Skip to content

fix: validate successful request-state completion - #534

Open
YS-OH-CORE wants to merge 1 commit into
modelcontextprotocol:mainfrom
YS-OH-CORE:fix/request-state-completion-505-20260928
Open

YS-OH-CORE wants to merge 1 commit into
modelcontextprotocol:mainfrom
YS-OH-CORE:fix/request-state-completion-505-20260928

Conversation

@YS-OH-CORE

Copy link
Copy Markdown

Reject unsuccessful round-2 results in the existing request-state diagnostic.

Motivation and Context

Closes #505.

sep-2322-request-state-complete currently accepts a tool-error result or a completion without the fixture's documented state-ok marker. The check can therefore report successful state validation when the diagnostic did not report success.

This patch rejects isError: true and requires a text content block containing state-ok. It keeps the existing check ID and protocol-level isCompleteResult helper, and extends the existing broken-server fixture and negative-test file. The marker is this fixture's existing contract, not a new requirement for arbitrary MCP tools.

How Has This Been Tested?

The linked executions were completed on 16 September 2026 KST (15 September UTC) against 7169291ec0b68eb370fddcd9947313ab0d5e4156. Current upstream main is still that exact commit. This branch applies the same published patch, SHA-256 6b54c4129d72691775a01c911515dee4bc59036262a957e3f9bf61c0bac7faa9.

Executed check Original check Candidate
Four existing plus eight new HTTP cases 8 pass, 4 targeted failures 12 pass
Full npm test suite Not repeated as a full baseline 630 pass; 0 failed, pending or todo
Built CLI, unchanged bundled server 3/3 checks pass 3/3 pass
Built CLI, three deliberately bad completions Target incorrectly succeeds, exit 0 Target fails, exit 1

Full-suite/build execution used Node 22.16.0, npm 10.9.2 and the upstream lockfile. Builds, typecheck, touched-file ESLint and diff checks passed. The 630 tests include the 12 focused cases.

A separate Go SDK execution used go-sdk@fbd36cb7870176bfc3e8e423df697cf110d0f927, Go 1.25.0, its native Streamable HTTP handler and the built conformance CLI. Across 12 before/after invocations, the unchanged request-state, multi-round and tampered-state controls stayed successful; three deliberately bad completion modes changed from false success to failure. The same raw target responses and successful round-1/wire checks were preserved.

These are controlled diagnostic cases, not defects claimed in the unmodified SDK. The marker does not itself prove state integrity; the separate tampered-state check remains necessary. Complete execution details and scope.

For this submission, source blobs, patch hash, clean application and resulting file bytes were rechecked. The executions above are prior evidence on the unchanged base and patch, not new runtime runs today.

Breaking Changes

No API or configuration change. This diagnostic now fails completions that violate its already documented successful-completion contract.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature
  • Breaking change
  • Documentation update

Checklist

  • Repository design guidance and the existing diagnostic contract reviewed.
  • Code follows the repository's style checks.
  • New and existing tests passed in the linked GitHub-hosted execution.
  • Invalid completion results produce an explicit failure diagnostic.
  • Existing scenario documentation already describes the required marker.

Additional context

This complements #498: its focus is round-1 prerequisite availability; this PR validates round-2 completion. Both modify the same diagnostic check and broken fixture, so both fixture behaviors will need to be retained when rebasing after either PR merges.

AI assistance: Zero used ChatGPT for the analysis, implementation, tests and this PR description.

Zero × Youngseok Oh

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

request-state-complete accepts tool-error results and completions missing its documented state-ok marker

1 participant