Skip to content

tasks/mrtr: address PR 262 audit before review#9

Merged
panyam merged 1 commit into
feat/tasks-mrtr-extensionfrom
chore/conformance-262-audit-fixes
Jun 9, 2026
Merged

tasks/mrtr: address PR 262 audit before review#9
panyam merged 1 commit into
feat/tasks-mrtr-extensionfrom
chore/conformance-262-audit-fixes

Conversation

@panyam

@panyam panyam commented Jun 9, 2026

Copy link
Copy Markdown
Owner

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)

  • 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) now lives inside connectStateless: ConnectOptions.capabilities / clientInfo are folded into params._meta via the trailing _meta spread withRequestMeta already supports.
  • Doc-fix Connection.request's extraHeaders: 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.ts moves under tasks/ as tasks/mrtr-helpers.ts. All 10 importers live in tasks/.
  • Drop unused AnyResult export + zod import.
  • SEP-2322 spec reference re-exports MRTR_SPEC_REFERENCES[0] from input-required-result-helpers so there's one URL per SEP in the codebase.

Naming

  • All 10 tasks/*.ts switch source: ScenarioSource = {...} to readonly source = {...} as const to match caching.ts / resources.ts / http-standard-headers.ts.
  • Per-scenario connect-failure check IDs (tasks-lifecycle-bootstrap, tasks-headers-bootstrap, etc.) instead of a shared tasks-session-bootstrap. Matches the upstream caching.ts pattern (sep-2549-caching-connection).

Traceability

  • sep-2663.yaml gains the positive-path counterpart to the existing rejection requirement under Streamable HTTP: Routing Headers, so every sep-2663-* ID emitted from tasks/*.ts is declared in the manifest.

Reviewer's guide

What changed Where to look
Connection-layer capabilities/clientInfo injection (H1 surface revert) src/connection/stateless.ts connectStateless (new withConnectMeta)
extraHeaders stateful doc src/connection/index.ts
Helper relocation src/scenarios/server/tasks/mrtr-helpers.ts (rename + dead-code drop)
Per-scenario bootstrap IDs src/scenarios/server/tasks/*.ts — grep -bootstrap
Manifest update src/seps/sep-2663.yaml Streamable HTTP: Routing Headers section

Test plan

  • npm test — 270/270 pass
  • npm run build — clean
  • mcpkit testconf-tasks-v2 against this branch — 47 fork scenarios + 1 sentinel pass
  • mcpkit testconf-mrtr against this branch — 3 fork scenarios + 1 sentinel pass

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.
@panyam panyam merged commit 20fc9a5 into feat/tasks-mrtr-extension Jun 9, 2026
2 of 4 checks passed
@panyam panyam deleted the chore/conformance-262-audit-fixes branch June 9, 2026 07:30
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.

1 participant