fix(mcp): keep anticipated tool failures readable under mcp 2.1 - #452
Merged
Conversation
The mcp 2.1 SDK narrowed which exceptions reach the client: only a ToolError keeps its message, and everything else is treated as a crash and reported as a bare "Error executing tool <name>". The server raised ValueError/RuntimeError for failures it fully anticipated, so all of them were masked -- an agent asking for a mistyped REQ_TYPO got "Error executing tool get_requirement" and no way to see the typo. Raise ToolError at the eight anticipated-failure sites instead, which also drops them from ERROR-with-traceback to INFO in the server log. Reverting only the source change fails eight tests, but main caught this with one: the others asserted is_error without asserting that the reason survives. They now assert the message too, so a future SDK bump cannot re-mask them silently. Refs reqstool/.github#111 Signed-off-by: Jimisola Laursen <jimisola@jimisola.com>
get_urn_details and enrich_document raise ToolError like the rest, but neither had any error-path test, so the mcp 2.1 masking would have gone unnoticed there even with the rest of the suite strengthened. Both fail against the pre-fix server, so they bind the behaviour rather than describe it. refresh's reload-failure path is left uncovered -- provoking it needs a project that builds once and then cannot. Refs reqstool/.github#111 Signed-off-by: Jimisola Laursen <jimisola@jimisola.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
mainis red. Its HEAD is the merge commit of #450 (mcp2.0.0 → 2.1.0), whosebuildfailed — one of the three PRs that landed without waiting for CI in reqstool/.github#111.Cause
mcp 2.1 narrowed which exceptions reach the client. From the SDK's own docstring:
Only a
ToolErrorkeeps its message. The server raisedValueError/RuntimeErrorfor failures it fully anticipated — "RequirementXnot found", "sources changed but reloading them failed" — so every one of them was masked. An agent asking for a mistypedREQ_TYPOgotError executing tool get_requirementand no way to see the typo.This fix raises
ToolErrorat the eight anticipated-failure sites. As a side effect they also drop from ERROR-with-traceback to INFO in the server log, which is where an anticipated failure belongs.Nothing else changes: a snapshot that cannot be reloaded is still an error and never a fallback to superseded data (bug #437), and startup failures stay
RuntimeErrorbecause they abort the server rather than answer a tool call.The regression was six times wider than the test that caught it
Reverting only
server.pyand keeping the tests fails 8 tests, spanningget_requirement,get_svc,get_mvr,get_requirement_statusandget_status.maincaught it with one.The other seven passed on
mainbecause they assertedresult.is_errorand stopped there — true, but blind to whether the model can tell why the call failed. They now assert the message survives too, so a future SDK bump cannot re-mask every error message behind a still-green suite.Verification
987 passed, 2 skipped(mainis986 passed, 1 failed);black --checkandflake8 src testsclean.statusagainsttest_basic/baseline/ms-101.mcp2.1.1 carries the identical branch intools/base.py, so this also unblocks fix(pypi): update dependency mcp to v2.1.1 #451, which is currently sitting open and red for the same reason.Note
#451 being open-and-red is the reqstool/.github#111 fix working:
autoMergeRequestis nowNone,renovate-approve[bot]still supplies the approving review, and Renovate is holding the merge becausebuildis failing. Its three predecessors merged in 4–28 seconds with red builds.Refs reqstool/.github#111