test(mcp): negotiate the non-task revision for real in the required-tool unit test (#369 follow-up) - #553
Conversation
…ool unit test The test pinning the Codex P1 fix (required tools stay callable on a session without the core Tasks utility) narrowed the server's negotiated version with Object.defineProperty. The SDK client can offer an older revision through supportedProtocolVersions, so the session is now negotiated for real at 2025-06-18 through the initialize handshake and the server's own getNegotiatedProtocolVersion() is asserted. A 2026-07-28 session is opened only by the SDK's serving entries (stdio/HTTP), never by a hand-connected in-memory pair, and takes the same branch: anything but 2025-11-25 serves every tool on the ordinary contract.
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
commit: |
1 participant
Follow-up to #550 (merged as
415583176), answering the Codex P1 thread onmcp-tasks.ts("Keep required tools callable on non-task revisions", discussion) with a stronger pin. #550 auto-merged while this commit was being gated, so it lands here rather than on that branch.What changes
Test-only. The unit test pinning the fix —
tests/mcp-tasks.test.ts, "serves every tool as an ordinary request on a revision without the core Tasks utility, required ones included" — narrowed the server's negotiated version withObject.defineProperty(tasks, 'getNegotiatedProtocolVersion', …). It now negotiates the session for real through the SDK'sinitializehandshake: the client is constructed withsupportedProtocolVersions: ['2025-06-18'](a revision whose core has no Tasks utility), the server counter-offers it, and the test asserts the server's owngetNegotiatedProtocolVersion()is2025-06-18before checking that an ordinary call to ataskSupport: 'required'tool is served and thatparams.taskis ignored (no task record is created).Why
2025-06-18and not2026-07-28: the SDK opens a2026-07-28session only through its serving entries (the stdio entry andcreateMcpHandler), which mark the instance modern with a package-internal write; a hand-connectedInMemoryTransportpair cannot negotiate it ({ pin: '2026-07-28' }fails,'auto'falls back to legacy — verified against@modelcontextprotocol/{client,server}@2.0.0). The gate under test is "negotiated revision ≠2025-11-25", so any other genuinely negotiated revision drives the same branch, and a real handshake is stronger evidence than a stubbed accessor.Verified the test fails on the pre-fix
mcp-tasks.tswith exactly the-32601the thread describes, and passes onmain.Changeset
None: the only change is under
packages/agent-bundle/tests/**, which.changeset/config.jsonexempts (pnpm changeset status --since=origin/mainreports no package to bump).Verification
pnpm typecheck,pnpm lint,pnpm test:uniton the parent tree (3341 passed),rstest packages/agent-bundle/tests/mcp-tasks.test.ts(11 passed) on this branch.Self-review
Reviewer:
change-risk-reviewersubagent on GPT-5.6 Sol, diff vsorigin/mainat4945ad4f5. No findings: the gate inmcp-tasks.tsis strict equality on2025-11-25, so2025-06-18drives the same ordinary-request branch a2026-07-28session would;@modelcontextprotocol/client@2.0.0declaressupportedProtocolVersionsand its legacy handshake negotiates2025-06-18over anInMemoryTransportpair against server2.0.0; the test still fails pre-fix with-32601;tests/**is exempt from changesets andChangeset presentpassed.