test(e2e): cover tools/call without arguments

[claude]

Requested by Felix Weinberger · Slack thread

Motivation and Context

Before: main has accepted a tools/call request that omits the arguments field since the args ?? {} change in the McpServer tools/call path, but no e2e requirement covered it: mcpserver:tool:input-validation sends arguments: {}, which is a different wire shape.

After: the suite adds tools:call:omitted-args:all-optional and tools:call:omitted-args:required. The first registers a tool whose input schema fields are all optional, calls it with no arguments key (an outbound wire tap proves the key is absent), and sees the handler run with {} and return its result. The second registers a tool with a required argument, calls it with no arguments key, and sees a tool execution error (isError: true, content naming the argument: Input validation error: Invalid arguments for tool summarize: text: Invalid input: expected string, received undefined) without the handler running — the spec's Error Handling section lists input validation errors under tool execution errors. Both run on every transport and both entry arms (12 cells each); no knownFailures or exclusions.

With args ?? {} reverted the all-optional case fails on all 12 cells with AssertionError: expected true to be falsy (the SDK returns isError: true with Input validation error: Invalid arguments for tool greet: Invalid input: expected object, received undefined); with it, it passes.

How Has This Been Tested

pnpm --filter @modelcontextprotocol/test-e2e exec vitest run scenarios/tools.test.ts -t 'tools:call:omitted-args'   # 24 passed (12 cells × 2 ids)
pnpm --filter @modelcontextprotocol/test-e2e exec vitest run coverage.test.ts                                        # 6 passed
pnpm --filter @modelcontextprotocol/test-e2e test                                                                    # 44 files, 2667 passed | 145 expected fail
pnpm --filter @modelcontextprotocol/test-e2e check                                                                   # typecheck + lint clean
pnpm lint:all                                                                                                        # clean

Breaking Changes

None — tests only.

Types of changes

• Other: tests only

Checklist

• I have read the MCP Documentation
• My code follows the repository's style guidelines
• New and existing tests pass locally
• I have added a changeset (not needed: tests only)

🤖 Generated with Claude Code

https://claude.ai/code/session_012VRbFCp41otcScXE1YY3es

---

Generated by Claude Code

[changeset-bot]

⚠️ No Changeset found

Latest commit: 90ad5d6

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[pkg-pr-new]

Open in StackBlitz

@modelcontextprotocol/client

npm i https://pkg.pr.new/@modelcontextprotocol/client@2933

@modelcontextprotocol/codemod

npm i https://pkg.pr.new/@modelcontextprotocol/codemod@2933

@modelcontextprotocol/core

npm i https://pkg.pr.new/@modelcontextprotocol/core@2933

@modelcontextprotocol/server

npm i https://pkg.pr.new/@modelcontextprotocol/server@2933

@modelcontextprotocol/server-legacy

npm i https://pkg.pr.new/@modelcontextprotocol/server-legacy@2933

@modelcontextprotocol/express

npm i https://pkg.pr.new/@modelcontextprotocol/express@2933

@modelcontextprotocol/fastify

npm i https://pkg.pr.new/@modelcontextprotocol/fastify@2933

@modelcontextprotocol/hono

npm i https://pkg.pr.new/@modelcontextprotocol/hono@2933

@modelcontextprotocol/node

npm i https://pkg.pr.new/@modelcontextprotocol/node@2933

commit: 90ad5d6

[claude]

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Beyond the inline nits, I also checked that the outbound tx.send tap and the not.toHaveProperty('arguments') assertion hold on the entryModern arm — the client's per-request _meta envelope lands on params, not arguments, so the wire shape under test is unchanged there. The stringContaining('text') clause in the required-arg case is loose but is satisfied by the argument name in the validation message, and the send-restore skipping on assertion failure only affects a client that is torn down immediately afterward.

Extended reasoning...

Test-only change: two manifest entries in test/e2e/requirements.ts and two matching verifies() bodies in test/e2e/scenarios/tools.test.ts covering tools/call with the arguments field omitted; no SDK source, helpers, or coverage gates touched and no security-sensitive surface. The only reported findings are style nits, and the repository's catch-all CODEOWNERS entry means a human owner still signs off.

[claude]

I re-reviewed after the follow-up commit and found no bugs; the typed isJSONRPCRequest guard replacing the as cast reads cleanly. Since this adds two spec-conformance requirements to the e2e manifest, a maintainer look is still worthwhile.

What was reviewed:

• Both assertions against the SDK path: validateToolInput runs args ?? {} through the schema and the tools/call handler wraps the resulting ProtocolError into isError: true content via createToolError, so the all-optional and required cases match the message shape asserted (packages/server/src/server/mcp.ts:306-320, :370-375).
• Cross-arm state: received / handlerCalls are hoisted outside makeServer, and only the single tools/call is counted, so stateless re-creation and the initialize handshake do not perturb the [{}] / 0 expectations.
• Minor, non-blocking: the inline tx.send tap duplicates the existing tapWire(client) helper in test/e2e/helpers/index.ts:220 that the other scenario files use for the same purpose; switching to it would drop the manual restore line.

Extended reasoning...

The PR touches only test/e2e/requirements.ts (two new manifest entries) and test/e2e/scenarios/tools.test.ts (two verifies() bodies plus one added import); no SDK source changes and no security-sensitive surface. The follow-up commit replaced an unsafe as cast with the isJSONRPCRequest type guard and reworded one behavior string, which is the substantive change since the prior review. I verified the asserted behavior against the tools/call handler in packages/server/src/server/mcp.ts and that the hoisted state pattern follows test/e2e/CLAUDE.md. Deferred rather than approved because CODEOWNERS assigns every path to the typescript-sdk team and the manifest entries assert spec-conformance wording that maintainers own; the only observation is a small helper-reuse nit.

[claude]

The red pkg-publish check on a8db2d8 is the pkg.pr.new preview publish, not this PR: its "Publish preview packages" step got a 500 then a 404 from pkg.pr.new's /publish endpoint after the build succeeded, the twin pkg-publish run on the same commit passed, and both runs on the previous head passed. It is not a required check. The job has been re-run once.

---

Generated by Claude Code