feat(training-agent): dogfood @adcp/client 5.13 seller helpers (closes #2889)#2992
Merged
Conversation
…#2889) Adopts the five helper families shipped for seller authors in `@adcp/client` 5.11–5.14: - `wrapEnvelope` for sibling-field emission in the framework server's response adapters - `bridgeFromSessionStore` wires `comply_test_controller.seed_product` fixtures through `get_products` on sandbox requests (new behavior — closes a latent gap where seeded products never reached the buyer tool) - `mergeSeedProduct` replaces the shallow-spread overlay in `overlaySeededProducts` - `mcpAcceptHeaderMiddleware` replaces the inline Accept header + rawHeaders rewrite on `/mcp` and `/mcp-strict` - `adcpError()` + dispatcher `sanitizeAdcpErrorEnvelope` make `conflict-envelope.ts` and the response-body wrapper redundant — both deleted; a framework-path regression locks in that `recovery` is stripped from `IDEMPOTENCY_CONFLICT` envelopes at both transport layers Follow-ups filed as adcontextprotocol/adcp-client#824, #825, #826, #827 landed in 5.14 (closed), so the adoption is complete rather than partial. Deferred: migrating `comply_test_controller` off the `customTools` path to `registerTestController` for its auto-emitted capability block. Net `+183 / -459` — 276 lines of bespoke glue removed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
# Conflicts: # package-lock.json # package.json # server/src/training-agent/framework-server.ts
@adcp/client 5.14 regressed the storyboard runner — with 5.14 the training agent drops from 52/52 clean to 39/52 (confirmed against both this branch and main, identical behavior). Filed upstream as adcontextprotocol/adcp-client#866. For this PR, pin to 5.13.0 and narrow the adoption to what works there: `wrapEnvelope`, `mergeSeedProduct`, and a hand-rolled session- scoped `testController.getSeededProducts` callback (new behavior — seeded products now flow through `get_products`, closing a latent gap). Put back until the 5.14 bump resolves adcp-client#866: - inline Accept-header + rawHeaders rewrite on `/mcp` and `/mcp-strict` - `conflict-envelope.ts` + `wrapResponseForConflictRedaction` - response-wrapper regression test file Drop the framework-path regression test for `recovery`-absence on `IDEMPOTENCY_CONFLICT` — that path exists now only at the HTTP-layer redactor, which `dispatchTestRequest` doesn't exercise. The legacy `conflict-envelope.test.ts` covers the redactor's unit contract. Storyboards local verify: - legacy (TRAINING_AGENT_USE_FRAMEWORK=0): 52/52 clean, 380 passed, 0 failed - framework (=1): 51/52 clean, 390 passed, 1 known residual Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bokelley
changed the title
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
Adopts
@adcp/client5.11–5.13 seller helpers in the reference training agent. Three helper families used:wrapEnvelopefor sibling-field emission (replayed/context/operation_id) inframework-server.ts'stoAdaptedResponse/serviceUnavailable/versionUnsupported.testController.getSeededProductscallback wirescomply_test_controller.seed_productfixtures throughget_productson sandbox requests. New behavior — the training agent previously surfaced seeded products only increate_media_buy's product lookup, so a buyer that seeded then calledget_productsnever saw the fixture.SEED_PRODUCT_DEFAULTSfills the response-schema required fields so sparse fixtures still validate.mergeSeedProductreplaces the shallow-spread merge inoverlaySeededProducts, gaining permissive-leaf semantics and by-idpricing_optionsoverlay.Why
5.13.0exact (not^5.14.0or^5.13.0)Originally shipped against 5.14 but 5.14 regressed the storyboard runner — strict AJV mode promotes unresolvable
$refs from lenient-warning to hard step failure, and downstream storyboards that depend on a×-flagged step cascade into false-positive agent failures. Measured this branch + main both drop from 52/52 → 39/52 clean storyboards under 5.14.Filed upstream as adcontextprotocol/adcp-client#866. Pinned to
5.13.0(exact) sonpm installdoesn't opportunistically upgrade past the regression. Bump to^5.14.xin a follow-up PR once #866 ships a fix; the follow-up will also adopt the three 5.14-only helpers:bridgeFromSessionStore— collapses our hand-rolled session-scoped callback to a one-liner.mcpAcceptHeaderMiddleware— 5.14 patchesrawHeaders(adcp-client#825); 5.13's version only mutatesreq.headers.accept, which doesn't propagate throughStreamableHTTPServerTransport's@hono/node-server-backed Fetch Request.conflict-envelope.ts— 5.14 shipssanitizeAdcpErrorEnvelopein the dispatcher (adcp-client#826), making the training agent's wire-layer redactor redundant.Also deferred (not blocked on 5.14): migrating
comply_test_controlleroffcustomToolsontoregisterTestControllerfor its auto-emittedcapabilities.compliance_testing.scenariosblock.Upstream follow-ups filed during dogfooding
Four DX-gap issues filed; all four merged upstream the same day in adcp-client#830 and #843:
bridgeFromSessionStore(loadSession, selectSeededProducts, defaults?)mcpAcceptHeaderMiddlewarenow patchesrawHeaderstooadcpError()consultsADCP_ERROR_FIELD_ALLOWLIST; dispatcher re-applies viasanitizeAdcpErrorEnvelopecreateDefaultTestControllerStorewas deleted, replaced with a worked seller examplePlus adcontextprotocol/adcp-client#866 blocking the 5.14 adoption (filed from this branch after finding the storyboard regression).
Security notes
Addressed during review:
resolveAccountwired,isSandboxRequest(params)is the only fence. Sessions are keyed bybrand.domain/account_id, not auth principal — cross-tenant fixture reads are possible for any authenticated caller that sendscontext.sandbox: trueplus another caller'sbrand.domain. Pre-existing behavior (legacyoverlaySeededProductshad the same shape); acceptable for this sandbox because fixture data is non-sensitive by design. Documented inline at the bridge wiring site.comply_test_controlleris registered viacustomTools, which bypasses the dispatcher'sfinalize()/sanitizeAdcpErrorEnvelope. Today the handler never emitsadcp_errorenvelopes (only{ success: false, error: '<code>' }), so no live leak. Guard-rail added as a module-level comment incomply-test-controller.ts.Test plan
npm run typecheckcleanAccept: application/json→ 200, framework dispatcher servesget_products,IDEMPOTENCY_CONFLICTflow stripsrecoveryfrom bothstructuredContent.adcp_errorANDcontent[0].text🤖 Generated with Claude Code