Conversation
…enarios
When a server returns an InputRequiredResult with an empty or missing
inputRequests object, `Object.keys(result.inputRequests!)[0]` evaluates
to undefined. This previously caused the test harness to send
`inputResponses: { "undefined": ... }` over the wire in subsequent
rounds instead of recognizing that the round-2/round-3 retry is
untestable.
Per the untestable convention (modelcontextprotocol#248, modelcontextprotocol#372), mark the completion check as
untestable when round 1 returns no inputRequests, and safely guard all 5
inputRequests extraction sites in input-required-result.ts.
Closes modelcontextprotocol#440
|
I verified this against main at There are nine Lines 270 (sampling) and 406 (list roots) run round 2 under So the five you guarded are exactly the ones whose round 1 check tolerates an absent or empty One thing I would still change. Those four surviving assertions are load bearing on error pushes dozens of lines away, with nothing in the code connecting the two. A later edit that relaxes a round 1 check, which is exactly what PR 442 did in a neighbouring scenario, turns one of them into a crash with no test failing first. Since you are already in this file, using The question I think is the real one here. PR 442 settled that a server naming no inputRequests has not violated anything, because the field is optional, and routed that through There is a defensible answer, that those two scenarios carry an explicit fixture contract so a server returning nothing has failed the prerequisite. But that is issue 248's case, which is what PR 442 cited when it chose One practical note for a maintainer passing by: the workflows here are still awaiting approval, so there is no CI signal on this yet. |
|
@itguruhaseeb Addressed both points in 75b6ead. The four remaining For the verdict question, empty or omitted The 32 new HTTP tests check the actual requests sent, including stopping before round 3 when round 2 requests nothing. Against Linux CI on this commit passes in the fork. The upstream workflows still need maintainer approval. |
Fixes #440.
An empty
inputRequestsmap made five retry sites sendinputResponsesunder the fabricated key"undefined". Those sites now stop before sending the retry and report the existing check asFAILUREwithdetails.untestable: true. Omitting the optional field follows the same path, including when round 2 leaves round 3 without input to answer.The guard uses
key === undefined, preserving server-assigned keys such as""and the literal"undefined". The remaining non-null assertions oninputRequestsare removed; sampling, roots, and multiple-input scenarios reuse their validated maps and keys.The sampling, roots, elicitation, and multiple-input fixtures also report an empty or omitted map as an untestable prerequisite, following #248 and #442. A response carrying only
requestStateis valid, but cannot exercise these fixtures. Wrong request methods still produce ordinary failures.Validation:
npm run checkandnpm run buildpass.npm test: 48 files, 655 tests pass.75b6eadpasses with Node 24.20.0: install, typecheck, lint, build, and the full test suite. The publish job was skipped. Upstream workflows still await maintainer approval."undefined"keys, and wrong-method failures. Againstmainat7169291, 20 fail; all pass with this change.b65426158ed9f29aea8ef3dc09ca22d7d9d6f970. Every CLI invocation exits 0 using Node 22.14.0 on Windows. Node 24.19.0 passed the wire checks but hit a libuv assertion during CLI shutdown; those runs are excluded from this result.AI assistance: Antigravity for the initial change; Codex for the revision and validation.