feat: grader MCP transport mode (#612)#617
Merged
Merged
Conversation
Closes #612. Adds transport-aware grading so the conformance grader works against MCP agents, not just raw-HTTP AdCP endpoints. GradeOptions.transport: 'raw' | 'mcp' (default 'raw'). In 'mcp' mode: - URL becomes baseUrl as-is (no path-join); MCP agents have one endpoint. - Body wraps in a JSON-RPC tools/call envelope; operation comes from the vector URL's last path segment. - Accept: application/json, text/event-stream added so MCP Streamable HTTP servers don't 406 the probe. Signature covers the envelope body. Applied uniformly through sign(), signWithParamOverride(), signWithComponents(), and the 001 mutation — every mutation path produces MCP-shaped requests when transport === 'mcp'. CLI: --transport <raw|mcp> on `adcp grade request-signing`. Validated; unknown values exit 2 with a clear error. Test agent: test-agents/seller-agent-signed-mcp.ts uses createAdcpServer (with request_signing + specialisms advertised) + serve({ preTransport }) with the verifier middleware. Valid requests flow into createMediaBuy; invalid requests get 401 + WWW-Authenticate before MCP dispatch. E2E test: test/request-signing-grader-mcp.test.js spawns the MCP agent, grades it in MCP mode, asserts 25/25 non-profile vectors pass + envelope structural invariants. All 3 subtests green. Full suite: 3953/3955 pass, 0 fail, 2 skipped (pre-existing). With this change + #600 (grader) + adcp#2368 (deploy verifier on the production test agent), the live-agent smoke test path is complete. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Addresses expert review findings on PR #617. Must-fix: - Add build:test-agents script to package.json. The file headers of both test-agents/seller-agent-signed{,-mcp}.ts told operators to run it; it didn't exist. First-contact path now works. - Auto-skip canonicalization-edge positive vectors 005-008 in MCP mode. MCP flattens every vector URL to the same MCP endpoint, neutralizing the port/path/query/encoding edges. Skipping with a distinct skip_reason prevents the report from claiming coverage it didn't deliver. MCP_FLATTENED_VECTORS is an explicit list so future canonicalization vectors have to opt in. - Update seller-agent-signed.ts stale header — used to say MCP-aware grader was "future work (issue TBD)"; now cross-links to the MCP variant + --transport mcp. Should-fix: - wrapMcpEnvelope now lets JSON.parse throw on non-JSON vector bodies instead of silently degrading to a string. Vectors ship JSON by spec contract; a parse failure is a vector drift worth surfacing. - mcpRequestIdCounter removed — JSON-RPC ids now come from crypto.randomUUID(). Removes module-level mutable state that could leak between concurrent graders in the same process. - MCP e2e startup detection rejects on timeout OR on child exit before ready, with captured stderr/stdout. Previous 3s "assume started" fallback hid crashes behind 20s of connect errors in the grader. - Added negative-vector MCP-shape matrix test — iterates all 20 mutations, asserts each produces url === baseUrl + JSON-RPC tools/call envelope. Locks the applyTransport invariant at every mutation site. Follow-up: - CLI hint: when every graded vector fails with a 404/405/fetch-error shape AND --transport wasn't explicitly set to mcp, surface a 💡 hint suggesting --transport mcp. Based on the common mistake of grading an MCP agent in the default raw mode. Also: subprocess orphan-reaper via process.on('exit') in MCP e2e test — prevents EADDRINUSE on the next CI run when the parent crashes abnormally. All 4 MCP tests pass; full suite 3911/3913 pass (2 pre-existing skips). Refs #617. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CI runs `npm test` without `npm run build:test-agents`, so the MCP e2e test's subprocess agent (test-agents/dist/seller-agent-signed-mcp.js) was missing on fresh runs and all 4 subtests cancelled with "Cannot find module". My round-1 startup-reject change helpfully surfaced the cause instead of hanging for 20s. Fix: before() hook calls tsc on test-agents/tsconfig.json if dist is missing. Cost is one-time per CI run (~1–2s tsc). No workflow file edit needed; the test self-builds. Bumped startup timeout to 10s for cold-start headroom. Attempted a tsx-based approach first but tsx doesn't honor the root tsconfig's `paths` mapping for `structured-headers`, so ParseError imports become undefined and the verifier crashes on instanceof. Dist path + auto-build is simpler and matches how other test files invoke compiled agents. All 4 MCP tests pass from a cold (no dist) start in 2.7s locally; full suite 3954/3956 pass (2 pre-existing skips). Refs #617. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
Author
Follow-ups deferred from this PRCaptured here so they don't get lost — neither is a merge blocker per the expert reviews.
Happy to do either in a follow-up PR after this lands. |
bokelley
added a commit
that referenced
this pull request
Apr 19, 2026
…reasons (#617) The request-signing grader's MCP-transport mode (#617) adds a new grader-specific skip reason. Adopt it into RunnerDetailedSkipReason and route it through DETAILED_SKIP_TO_CANONICAL as `not_applicable` — the vector is skipped because the transport doesn't exercise URL-edge semantics, which is a scope gap (agent didn't opt into the vector) not a contract failure. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bokelley
added a commit
that referenced
this pull request
Apr 19, 2026
Two follow-ups deferred from #617. 1. Operation-name allowlist in extractOperationFromVectorUrl. Throws if the vector URL's last path segment isn't an AdCP operation identifier (lowercase_snake_case). No exploit today — fixtures are spec-published — but blocks arbitrary bytes from reaching params.name if a compliance cache gets corrupted or swapped. 2. MCP rate-abuse subtest. Dedicated MCP agent with ADCP_REPLAY_CAP=10 + grader configured with matching cap + onlyVectors=['020-rate-abuse'] + allowLiveSideEffects=true. Validates the 11-request flood trips request_signature_rate_abuse under MCP transport. Adds ADCP_REPLAY_CAP env override to seller-agent-signed-mcp.ts so tests can tune the cap without touching agent code. All 5 MCP tests pass; full suite 3947/3949 (2 pre-existing skips). Refs #617 review thread. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
5 tasks
bokelley
added a commit
that referenced
this pull request
Apr 19, 2026
* chore: grader MCP follow-ups — operation allowlist + rate-abuse subtest Two follow-ups deferred from #617. 1. Operation-name allowlist in extractOperationFromVectorUrl. Throws if the vector URL's last path segment isn't an AdCP operation identifier (lowercase_snake_case). No exploit today — fixtures are spec-published — but blocks arbitrary bytes from reaching params.name if a compliance cache gets corrupted or swapped. 2. MCP rate-abuse subtest. Dedicated MCP agent with ADCP_REPLAY_CAP=10 + grader configured with matching cap + onlyVectors=['020-rate-abuse'] + allowLiveSideEffects=true. Validates the 11-request flood trips request_signature_rate_abuse under MCP transport. Adds ADCP_REPLAY_CAP env override to seller-agent-signed-mcp.ts so tests can tune the cap without touching agent code. All 5 MCP tests pass; full suite 3947/3949 (2 pre-existing skips). Refs #617 review thread. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix: address review nits on #620 Three low-severity items from the expert-review round: 1. NaN guard on ADCP_REPLAY_CAP. A typo (ADCP_REPLAY_CAP=abc) turns Number.parseInt into NaN; InMemoryReplayStore's `size >= NaN` check is always false, silently disabling the rate-abuse guard. Now falls back to the default on any non-positive int. 2. PORT clobber fix in startMcpAgent. Spread overrides BEFORE PORT so a caller passing { PORT: '9999' } in overrides can't override the explicit port argument. Prophylactic — no caller does this today. 3. Aspirational comment reference in builder.ts operation allowlist. The reviewer noted `static/schemas/source/enums/operation.json` isn't in the local sync (it lives in the adcp spec repo). Reworded to reference the actual observable source — task values in compliance/cache/{version}/protocols/**/*.yaml. Bonus: documented the port-stride requirement (≥2) for parallel runs of request-signing-grader-mcp.test.js, since the rate-abuse subtest binds PORT+1. All 5 MCP tests still pass. Refs #620 review thread. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Summary
Closes #612. Makes the request-signing conformance grader work against MCP agents, not just raw-HTTP AdCP endpoints.
GradeOptions.transport: 'raw' | 'mcp'(default'raw'). In'mcp'mode, each vector is wrapped in a JSON-RPCtools/callenvelope and POSTed to the agent's single MCP mount path; operation name comes from the vector URL's last path segment.--transport <mode>onadcp grade request-signingwith validation.test-agents/seller-agent-signed-mcp.ts—createAdcpServer(using therequest_signing/specialismscapability fields added in feat: RFC 9421 request-signing conformance grader (#585) #600) +serve({ preTransport })(the pre-MCP middleware hook from feat: RFC 9421 request-signing conformance grader (#585) #600).test/request-signing-grader-mcp.test.jsspawns the agent, grades in MCP mode, asserts 25/25 non-profile vectors pass.The raw-HTTP path is unchanged. Canonicalization-edge vectors (005–008) fold into plain POSTs under MCP mode (their URL-level edges don't apply) — a documented trade-off, not a regression. Operators wanting those edges tested use
--transport raw.Dependency graph complete
With this PR + #600 (grader) + adcontextprotocol/adcp#2368 (deploy verifier on the production test agent),
adcp grade request-signing https://test-agent.adcontextprotocol.org/mcp --transport mcpproduces a meaningful grade against the live reference.Test plan
npx tsc --noEmitcleannpx tsc -p test-agents/tsconfig.json --rootDir test-agentscleannpm run format:checkcleannpm run build:libcleannpm test— 3953/3955 pass, 0 fail, 2 skipped (pre-existing)test/request-signing-grader-vectors.test.js— 30 (loader + builder + preflight)test/request-signing-grader-e2e.test.js— 6 (raw vs reference verifier)test/request-signing-runner-integration.test.js— 10 (synthesis + dispatch)test/request-signing-grader-mcp.test.js— 3 (MCP transport path)Related
serve.preTransporthook)🤖 Generated with Claude Code