tasks/mrtr: address PR 262 audit before review#9
Merged
panyam merged 1 commit intoJun 9, 2026
Conversation
Pre-emptive cleanup against the upstream patterns established by recent merges (modelcontextprotocol#318 Connection abstraction, modelcontextprotocol#319 draft-spec consistency, modelcontextprotocol#303 SEP-2243 check IDs, modelcontextprotocol#321 MockServer surface). No behavior change for scenarios — every check is still emitted; only the shape of how they're emitted moves toward upstream's house style. Surface - Revert withRequestMeta / sendStatelessRequest signatures to match upstream byte-for-byte. The need that pushed me to widen those (per-request _meta.clientCapabilities for extension declaration on the SEP-2575 stateless wire) is now handled inside connectStateless: ConnectOptions.capabilities / clientInfo are folded into params._meta via the trailing _meta spread that withRequestMeta already supports. This keeps the new "extension declaration" concept on the Connection layer only — the lower-level helpers stay shared with every other scenario in the suite. - Document that Connection.request's extraHeaders throws on the stateful wire instead of "ignored with a warning" — the SDK transport manages headers internally, so silent drop in a conformance harness would mask test correctness. Layout - src/scenarios/server/tasks-mrtr-helpers.ts moves under tasks/ as tasks/mrtr-helpers.ts. All 10 importers live in tasks/; the file's prior placement next to input-required-result-helpers.ts no longer applies once those scenarios moved into tasks/. - Drop the unused AnyResult export + zod import. The Connection wrapper doesn't take a zod schema, so the helper was dead on arrival. - SEP-2322 spec reference now re-exports MRTR_SPEC_REFERENCES[0] from input-required-result-helpers so the codebase holds one URL per SEP instead of two for SEP-2322. Naming - All 10 tasks/*.ts switch to `readonly source = { ... } as const` to match the convention used by caching.ts, resources.ts, http-standard-headers.ts. Drops the ScenarioSource import where now unused. - Scenario-scope each scenario's connect-failure check ID (tasks-lifecycle-bootstrap, tasks-headers-bootstrap, …) instead of sharing tasks-session-bootstrap. Matches the upstream caching.ts pattern (sep-2549-caching-connection) where each scenario owns its own bootstrap-failure ID. Traceability - sep-2663.yaml gains the positive-path counterpart to the existing rejection requirement under Streamable HTTP: Routing Headers, so every sep-2663-* check ID emitted by tasks/*.ts is declared in the manifest. Verification - Conformance vitest: 270/270. - Build: clean. - mcpkit testconf-tasks-v2 (47 fork scenarios + 1 sentinel): pass. - mcpkit testconf-mrtr (3 fork scenarios + 1 sentinel): pass.
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.
Pre-emptive cleanup against the upstream patterns established by recent merges (PR 318 Connection abstraction, PR 319 draft-spec consistency, PR 303 SEP-2243 check IDs, PR 321 MockServer surface). No behavior change for scenarios — every check is still emitted; only the shape of how they're emitted moves toward upstream's house style.
This PR stacks on top of
feat/tasks-mrtr-extension(PR 262 in upstream). Merge here first; PR 262 absorbs the changes on its next rebase.Summary
Surface (the H1 of the audit)
withRequestMeta/sendStatelessRequestsignatures to match upstream byte-for-byte. The need that pushed me to widen those (per-request_meta.clientCapabilitiesfor extension declaration on the SEP-2575 stateless wire) now lives insideconnectStateless:ConnectOptions.capabilities/clientInfoare folded intoparams._metavia the trailing_metaspreadwithRequestMetaalready supports.Connection.request'sextraHeaders: throws on the stateful wire (matches code); upstream stateful's SDK transport owns headers, so silent drop would mask test correctness.Layout
src/scenarios/server/tasks-mrtr-helpers.tsmoves undertasks/astasks/mrtr-helpers.ts. All 10 importers live intasks/.AnyResultexport +zodimport.MRTR_SPEC_REFERENCES[0]frominput-required-result-helpersso there's one URL per SEP in the codebase.Naming
tasks/*.tsswitchsource: ScenarioSource = {...}toreadonly source = {...} as constto matchcaching.ts/resources.ts/http-standard-headers.ts.tasks-lifecycle-bootstrap,tasks-headers-bootstrap, etc.) instead of a sharedtasks-session-bootstrap. Matches the upstreamcaching.tspattern (sep-2549-caching-connection).Traceability
sep-2663.yamlgains the positive-path counterpart to the existing rejection requirement under Streamable HTTP: Routing Headers, so everysep-2663-*ID emitted fromtasks/*.tsis declared in the manifest.Reviewer's guide
src/connection/stateless.tsconnectStateless(newwithConnectMeta)extraHeadersstateful docsrc/connection/index.tssrc/scenarios/server/tasks/mrtr-helpers.ts(rename + dead-code drop)src/scenarios/server/tasks/*.ts— grep-bootstrapsrc/seps/sep-2663.yamlStreamable HTTP: Routing Headers sectionTest plan
npm test— 270/270 passnpm run build— cleantestconf-tasks-v2against this branch — 47 fork scenarios + 1 sentinel passtestconf-mrtragainst this branch — 3 fork scenarios + 1 sentinel pass