fix(input-required-result): report the capability check as untestable when nothing is requested - #442
Open
marcus-kepler-92 wants to merge 1 commit into
Conversation
… when nothing is requested `input-required-result-capability-check` guarded its elicitation scan with `isInputRequiredResult(result) && result.inputRequests`. When a server answered without `inputRequests`, that arm was skipped, and the following `isCompleteResult` arm is false by construction for an input_required result, so nothing was recorded and the check passed. An empty `inputRequests` object passed too: the arm was taken, but the loop had nothing to scan. `inputRequests` is optional — a result carrying only `requestState` still satisfies "at least one of inputRequests or requestState" — so such a server has not violated "servers MUST NOT send an inputRequests that the client has not declared support for in its capabilities". It has not exercised it either. Reporting SUCCESS makes an unverified MUST read as green, which is the failure mode modelcontextprotocol#248 and modelcontextprotocol#372 established should be reported as a failure instead. So report it through `notTestable()` with `details.untestable`, matching the convention in AGENTS.md and the call sites in stateless.ts, rather than claiming a violation the server did not commit. Servers that legitimately have nothing to ask for now go red on this check; per modelcontextprotocol#248 the expected-failures baseline is the escape hatch. Adds the case to sep-2322-mrtr-broken-server so the existing MRTR negative suite covers it. The fixture returns a conformant envelope (`requestState` present, no input requests) rather than a bare one, so it isolates this check rather than also tripping sep-2322-request-state-incomplete. Verified against --spec-version 2026-07-28: everything-server 2/2 passed sep-2322-mrtr-broken-server FAILURE Not testable: server returned no inputRequests, so the capability restriction was never exercised Refs modelcontextprotocol#439
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.
Narrows #439 item 3. The round-2 consequence of the same case is #440; this PR
does not touch it.
The bug
input-required-result-capability-checkguarded its elicitation scan with:With
inputRequestsabsent, that arm is skipped and the followingisCompleteResultarm is false by construction for aninput_requiredresult, sonothing is recorded and the check passes.
inputRequests: {}passes by a differentpath — the arm is taken (
{}is truthy), the loop just has nothing to scan.Either way the check reports SUCCESS without having exercised the requirement it
scores.
Why
notTestable()rather than a plain assertioninputRequestsis optional — a result carrying onlyrequestStatestill satisfies"at least one of
inputRequestsorrequestState" — so a server that names nonehas not violated
sep-2322-respect-client-capabilities("Servers MUST NOT send an inputRequests that the client has not declared support
for in its capabilities"). It has not exercised it either.
That is the #248 / #372 case, so the check now reports through
notTestable()withdetails.untestable, per AGENTS.md and the existing call sites instateless.ts.Status stays FAILURE — the severity follows the underlying MUST NOT — but the
message says the requirement could not be verified rather than claiming a violation
the server did not commit.
Does this fail a compliant implementation?
No. The scenario's description is an explicit fixture contract: implement
test_input_required_result_capabilities, read_meta['io.modelcontextprotocol/clientCapabilities'], and return inputRequests forthe declared capabilities. The probe declares
sampling: {}, so a server followingthat contract returns a
sampling/createMessagerequest and still scores SUCCESS(verified below).
What now goes red is a server that implements the tool but returns no input
request — i.e. the fixture prerequisite is missing, which is exactly what #248
says should be red, with the expected-failures baseline as the escape hatch. It is
still a behaviour change for anyone currently green on that basis.
Fixture
Adds the case to
sep-2322-mrtr-broken-server. The fixture returns a conformantenvelope —
requestStatepresent, no input requests — rather than a bare{"resultType":"input_required"}, so it isolates this check instead of alsotripping
sep-2322-request-state-incomplete.Verified
Against
--spec-version 2026-07-28:The negative test pins the slug, the
Not testable:prefix anddetails.untestablerather than just asserting a failure count.npm run lint,npm run typecheckand the fullvitest run(43 files / 505tests) pass.