Repository navigation
[v1.x] test(e2e): cover tools/call and prompts/get without arguments - #2931
Conversation
Add four requirement ids exercising a tools/call or prompts/get request
that omits the `arguments` field entirely (not `arguments: {}`): an
all-optional tool and prompt run with the handler receiving `{}`, and a
tool/prompt with a required argument is refused with a message naming it.
Tests only; locks in the behavior #2045 introduced on v1.x.
Co-Authored-By: Claude Fable 5.1 <[email protected]>
Claude-Session: https://claude.ai/code/session_012VRbFCp41otcScXE1YY3es
|
commit: |
![claude[bot]](/web-api/iframe-proxy?url=https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.
One optional note from this repository's REVIEW.md or CLAUDE.md checks was not posted as a comment, over this review's limit for such notes; it is on this commit's check card.
…error Also narrow the captured request with isJSONRPCRequest instead of a cast. Co-Authored-By: Claude Fable 5.1 <[email protected]> Claude-Session: https://claude.ai/code/session_012VRbFCp41otcScXE1YY3es
There was a problem hiding this comment.
I re-reviewed the follow-up commit and didn't find any bugs; the isJSONRPCRequest narrowing now replaces the cast at both capture sites. Because this adds four spec-sourced entries to the conformance manifest, a maintainer look at the wording is still worthwhile.
What was reviewed:
- Traced both required-arg assertions against
src/server/mcp.ts: tools go through the blanket catch intocreateToolError(soisError: truewith "Input validation error ... at query"), prompts throwMcpError(InvalidParams, "... at text")— both regexes andstringContainingchecks are satisfiable by the single text element. - Confirmed
tapWireis called afterwire()(it throws if the client has no transport) and thatreceived/handlerCallslive outside the factory for stateless hosting. - Confirmed
isJSONRPCRequestis exported fromsrc/types.tsand each new requirement id has a matchingverifies()call; I was not able to execute vitest in this environment, so the pass claims in the description are unverified here.
Extended reasoning...
The PR is test-only: four new entries in test/e2e/requirements.ts plus matching verifies() tests in test/e2e/scenarios/tools.test.ts and prompts.test.ts covering tools/call and prompts/get with the arguments field omitted. It touches no security-sensitive surface. The second commit addressed the earlier type-cast nit in code, but the open thread on whether 'tools:call:omitted-args:required' should be spec-sourced with knownFailures or moved to an sdk-sourced id is a manifest-classification judgment for maintainers, and all changed paths fall under the wildcard CODEOWNERS entry. I could not run the test suite in this sandbox, so correctness was verified by reading McpServer's validation and error-wrapping paths rather than by execution.
2 participants
Requested by Felix Weinberger · Slack thread
Motivation and Context
Refs #1869
Before: the v1.x e2e manifest covered only a schema-less prompt fetched without arguments (
prompts:get:no-args) and an empty-objectarguments: {}for a prompt with a required argument (prompts:get:missing-required-args) and for a typed tool (mcpserver:tool:input-validation). #2045 made a request that omits theargumentsfield entirely count as{}for bothtools/callandprompts/get, but nothing in the e2e suite exercised that wire shape, so a regression would not have been caught here.After: four new requirement ids, each run on every transport and entry arm the v1.x suite covers (inMemory, stdio, streamableHttp, streamableHttpStateless, sse), with the request verified on the wire to carry no
argumentskey:tools:call:omitted-args:all-optional— a tool whose inputs are all optional, called viacallTool({ name }), runs and its handler receives{}.tools:call:omitted-args:required— a tool with a required argument, called with noarguments, is refused as an input validation error (isError: trueresult whose text names the argument) and the handler does not run.prompts:get:omitted-args:all-optional— a prompt whoseargsSchemais all optional, fetched viagetPrompt({ name }), returns its messages and its handler receives{}.prompts:get:omitted-args:required— a prompt with a required argument, fetched with noarguments, rejects with-32602and a message naming the argument.Existing ids are untouched. This is the v1.x port of #2928 (main), extended to tools.
With #2045's
?? {}reverted the all-optional tool case fails with anisErrorresultInput validation error: Invalid arguments for tool list-files: Invalid input: expected object, received undefined; with it, it passes.With #2045's
?? {}reverted the all-optional prompt case fails withMCP error -32602: Invalid arguments for prompt code-review: Invalid input: expected object, received undefined; with it, it passes.(The two required-argument cases also fail under the revert, because the message then says
expected object, received undefinedinstead of naming the missing argument.)How Has This Been Tested
npx vitest run test/e2e/scenarios/tools.test.ts test/e2e/scenarios/prompts.test.ts -t omitted-args: 20/20 cells pass (4 ids x 5 transports).npm run test:e2e(what thetest-e2eCI job runs, including thecoverage.test.tsmanifest gates): 25 files, 1135 tests, all passing.?? {}insrc/server/mcp.tsfor tools and prompts, all 20 cells fail; restored, all 20 pass;git statusshows onlytest/e2efiles changed.npm run check(tsgo typecheck, eslint,prettier --check .): clean.Breaking Changes
None, test-only.
Types of changes
Checklist
🤖 Generated with Claude Code
https://claude.ai/code/session_012VRbFCp41otcScXE1YY3es
Generated by Claude Code