docs(compliance): SHOULD on runner session reuse and 404-as-terminal (#6204) - #6205
docs(compliance): SHOULD on runner session reuse and 404-as-terminal (#6204)#6205fgranata wants to merge 2 commits into
Conversation
Per the adcontextprotocol#6204 triage: one SHOULD in runner-output-contract.yaml closing the doc gap while the session-per-storyboard fix lands in adcp-client. Runners SHOULD reuse one MCP session across a storyboard's steps, close gracefully, and treat expired-session 404s as terminal — fresh-transport -per-call multiplies per-step wall time ~5x and orphaned reconnect loops generate silent 4xx volume misread as rate limiting. Patch-eligible per triage; no changeset.
| # capability v{expires_after_version}; | ||
| # promoted at v{runner_capability_version}]". | ||
|
|
||
| session_lifecycle: |
There was a problem hiding this comment.
MUST FIX: This adds a new normative session_lifecycle block (three SHOULDs on session reuse, graceful close, and 404-as-terminal) to a file under static/compliance/source/** — a protocol-scoped path per check-changeset-protocol-scope.cjs. The PR ships no changeset.
The changeset-check workflow does not skip here: --has-protocol-scoped-changes returns true for static/compliance/source/, the file is not in CHANGESET_STATUS_EXEMPT_MAINTENANCE_PATHS, so the job runs changeset status --since=origin/main and fails with no pending changeset. Beyond CI, omitting the changeset ships this normative addition untracked against the AdCP versioning surface — the compliance contract carries its own version: "2.6.0" and builds into dist/compliance/<semver>/.
The PR body cites #6204's "patch-eligible; no changeset." Patch-eligibility is the errata test — a clarification of prior ambiguity that every conformant implementation already satisfies. New session-reuse and 404-handling SHOULDs are new requirements, not errata; this is Normative-class and needs a patch/minor changeset. "Patch-eligible" would still mean a patch changeset, not none.
There was a problem hiding this comment.
Ladon verdict: Request changes
Request changes — 1 blocking finding.
Blocking findings
- static/compliance/source/universal/runner-output-contract.yaml:607 — Compliance-source change ships no changeset; changeset-check CI will fail and a Normative addition ships untracked.
This PR adds a normative session_lifecycle guidance block (session reuse, graceful close, 404-as-terminal SHOULDs) to the runner output contract under static/compliance/source/**. That path is part of the published protocol surface, so per the mandatory changeset-scope rule a PR touching it with no .changeset/*.md is high. The YAML itself is well-formed and introduces no lint or schema-parity failures, but the missing changeset is a hard blocker: it fails changeset-check CI and ships an untracked wire/normative change. Add a correctly-scoped (non-empty) changeset reflecting this Normative addition to unblock.
Blocking findings
- static/compliance/source/universal/runner-output-contract.yaml:607 — Compliance-source change ships no changeset; changeset-check will fail
Per Ladon's review: patch-eligible means a patch changeset, not none — the SHOULDs are a Normative addition to the compliance source surface.
|
Changeset added ( |
Implements the
runner-output-contract.yamlfollow-on from the #6204 triage ("Add one SHOULD on session reuse across steps and 404-as-terminal. Patch-eligible; no changeset.").Adds a
session_lifecycleguidance block: runners SHOULD establish one MCP session per storyboard and reuse it across steps (a fresh session per call spends 4–5 round trips on handshake, ~5× per-step wall time, and attributes runner overhead to the agent under test), SHOULD close sessions gracefully, and SHOULD treat expired-session 404s as terminal rather than retryable (the streamable-HTTP session layer answers them silently, so reconnect loops read as rate limiting).Evidence for the numbers is in #6204. YAML strict-parses clean (
yaml.safe_load). The core session-per-storyboard fix lands inadcp-clientper the triage's sibling-repo item.