feat(testing): extend TestControllerBridge with per-tool seeded callbacks by bokelley · Pull Request #1754 · adcontextprotocol/adcp-client

bokelley · 2026-05-14T13:44:18Z

Summary

Extends TestControllerBridge<TAccount> with five new per-tool seeded callbacks so platform-proxy sellers (DSPs, walled gardens, retail-media networks) can inject fixtures into storyboard runs without live upstream OAuth or per-adapter stub code.

New callbacks (all optional, same post-handler merge pattern as the existing getSeededProducts):

Callback	Tool	Merge semantic
`getSeededCreatives`	`list_creatives`	append + dedup by `creative_id`, seeded wins
`getSeededMediaBuys`	`get_media_buys`	append + dedup by `media_buy_id`, seeded wins
`getSeededAccounts`	`list_accounts`	append + dedup by `account_id`, seeded wins
`getSeededAccountFinancials`	`get_account_financials`	deep-merge first entry onto handler response (`mergeSeed`)
`getSeededCreativeFormats`	`list_creative_formats`	append + dedup by composite `agent_url\0id`, seeded wins

bridgeFromSessionStore extended with matching optional selectSeeded* options so session-store adopters get all five bridges in one helper call.

New exports from @adcp/sdk/server: merge helpers (mergeSeededCreativesIntoResponse etc.), filter helpers (filterValidSeededCreatives etc.), and type aliases SeededCreative and SeededMediaBuy.

What was tested

59 new unit tests in test/lib/test-controller-bridge.test.js (all passing):
- bridgeFromSessionStore with each new selector (presence/absence, async, null return)
- Merge helpers: append, dedup/collision, empty-noop, sandbox stamp, sandbox: false preservation
- Filter helpers: valid entries, drops on missing required ID, non-array input
tsc --noEmit (main + lib tsconfig): clean
Prettier: clean
Pre-push hook (format check + typecheck + build): passing

Pre-PR review

Protocol review (ad-tech-protocol-expert): Approach is sound. get_account_financials deep-merge is the correct semantic for a single-object response. Sandbox gate matches the existing getSeededProducts pattern. mergeSeededAccountsIntoResponse and mergeSeededAccountFinancialsIntoResponse correctly omit the sandbox: true stamp — neither ListAccountsResponse nor GetAccountFinancialsSuccess carries a top-level sandbox field.

Code review (code-reviewer): Three issues found and fixed before this PR:

filterValidSeededAccountFinancials now also validates required period (DateRange) and timezone (string) fields, not just account and currency — prevents mergeSeed from deep-merging undefined onto required scalars.
JSDoc on both stampless merge functions now explains the omission (prevents bug reports).
bridgeFromSessionStore JSDoc clarified: five new select* callbacks are optional; selectSeededProducts remains required.

Also added a guard to scripts/generate-wire-spec-fields.ts to prevent the codegen from overwriting the committed generated file with an empty stub when schemas/cache/ hasn't been downloaded — a latent build-time footgun in fresh-clone environments.

This PR was drafted by the AdCP client triage agent in response to issue #1002.

https://claude.ai/code/session_01Jig6Zu57A749dCtjxQqbso

Generated by Claude Code

…acks Adds getSeededCreatives, getSeededMediaBuys, getSeededAccounts, getSeededAccountFinancials, and getSeededCreativeFormats to the TestControllerBridge interface. Each follows the existing getSeededProducts pattern: opt-in by presence, sandbox-gated, post-handler merge, warn-and-drop on invalid fixtures. Extends bridgeFromSessionStore with matching selectSeeded* options and wires all five dispatch blocks in the create-adcp-server dispatcher. Closes #1002 https://claude.ai/code/${CLAUDE_CODE_REMOTE_SESSION_ID}

https://claude.ai/code/${CLAUDE_CODE_REMOTE_SESSION_ID}

… version Prevents `generate-wire-spec-fields.ts` from overwriting the committed generated file with an empty stub when `schemas/cache/` hasn't been downloaded (fresh clone or gitignore wipe). The guard keeps the existing file unchanged when no schemas are found, avoiding a tsc breakage that blocked local builds before `sync-schemas` is run. Also picks up the `sync-version` output that bumped LIBRARY_VERSION to match the package.json 7.3.0 after the version sync ran. https://claude.ai/code/session_01Jig6Zu57A749dCtjxQqbso

…clarify no-sandbox-stamp JSDoc - filterValidSeededAccountFinancials now also rejects entries missing the required `period` (DateRange) or `timezone` (string) fields, preventing mergeSeed from deep-merging undefined onto required scalars. - JSDoc on mergeSeededAccountsIntoResponse and mergeSeededAccountFinancialsIntoResponse now explains that neither response type carries a top-level `sandbox` field, so no stamp is applied (pre-empts "copy-paste omission" bug reports). - bridgeFromSessionStore JSDoc now clarifies that the five new select* callbacks are optional while selectSeededProducts remains required. https://claude.ai/code/session_01Jig6Zu57A749dCtjxQqbso

The merge helpers for the five new per-tool seeded callbacks are exercised directly, but mergeSeededProductsIntoResponse was destructured without any call site. Removing the unused binding satisfies the github-code-quality lint and keeps the import list aligned with what the suite actually uses. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

A storyboard run that succeeds because seeded fixtures were merged into the response verifies protocol conformance against fixture data — wire shape, error envelopes, idempotency, signed-request handling, sandbox stamping. It does not verify that the seller's adapter against the real upstream (Snap, Meta, TikTok, Google Ads, etc.) works, because the post-handler merge bypasses the upstream code path. Spell that out in the TestControllerBridge JSDoc and the changeset so consumers don't read "conformance support for proxy sellers" and walk away thinking the storyboard exercises their real adapter. Sellers still need a live-OAuth runner to cover adapter health; the two together give wire conformance and adapter health respectively. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…nc envelope, dispatcher) Four expert reviews (protocol, code, security, product) converged on three real bugs and a handful of polish items. This commit addresses all of them plus the follow-up issues filed at #1775 / #1776. **Bugs fixed** - Pagination/count drift on list merge helpers. `mergeSeededCreativesIntoResponse` bumped `query_summary.returned` but left `total_matching` alone; the three other list helpers left `pagination.total_count` alone. A storyboard cursor-walk could land at `returned > total_matching` or terminate early. All four now bump the count by the number of genuinely-new seeded entries (collisions don't double-count). `cursor` / `has_more` left untouched — seeded entries land on the current page, the handler's cursor still points to its next page if any. - Async-envelope hybrid bug. `isErrorResponse` returns false for `{status:'submitted', task_id}` and `{status:'working'}` handoff envelopes the handler may return when deferring to a long-running task. Without a success-arm guard, the merge spread `creatives: [...]` (or `media_buys`, etc.) into the async envelope, producing an invalid hybrid wire shape. Each helper now short-circuits when the response isn't the success arm and returns the handler response reference-equal — the dispatcher reads that ref-equality to skip the re-wrap (re-wrapping the same payload can produce a subtly different `content[].text` summary). - `sandbox: true` stamp on `list_creative_formats`. The response schema has no `sandbox` property — we were inventing a wire field. Dropped to match the `list_accounts` / `get_account_financials` pattern. Sandbox semantics still flow through the dispatcher gate. **Dispatcher consolidation** Six near-identical per-tool branches in `createAdcpServer` collapse into a single table-driven `applySeededBridge` helper. Each row maps a tool name to its callback, validator, and merge helper. The helper also emits a `debug` log when the sandbox-account gate rejects a sandbox-flagged request — adopters chasing "why aren't my fixtures showing" now have a diagnostic surface (security review found this gap). **JSDoc tightening** Top-of-file JSDoc on `TestControllerBridge` now spells out two adopter responsibilities the SDK can't enforce: - `resolveAccount` is the trust boundary. Production bindings MUST configure it; otherwise the request-signal check is the only line of defense against fixture leakage. - Multi-tenant isolation is the adopter's job. Callbacks must key their fixture store on `ctx.account`; the SDK does no defensive cross-check. `mergeSeededAccountFinancialsIntoResponse` now accepts an optional logger and warns when the adopter passes more than one seeded entry (only the first is applied — product expert flagged this as a footgun in the type signature). **New exports** - `SeededAccount = ListAccountsResponse['accounts'][number]` for symmetry with `SeededCreative` and `SeededMediaBuy`. - `applySeededBridge` for direct unit testing of the dispatcher. **Tests** +24 tests covering: pagination/count bookkeeping for all four list helpers (with and without collisions, with and without pagination present); async-envelope guard on all six merge helpers; AccountFinancials multi-entry warning; applySeededBridge dispatcher (no-callback, error envelope, non-sandbox input, sandbox-gate-reject debug log, callback- throws degrade, non-array degrade, async-envelope no-re-wrap, happy path). 83/83 pass. **Follow-ups filed** - #1775 — runner-visible bridge marker (cross-repo: storyboard runner must surface bridge participation in the run record so compliance scores don't conflate wire conformance with adapter health). - #1776 — extend bridge to `get_signals`, `get_media_buy_delivery`, `list_property_lists`, `list_collection_lists`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…ool-callbacks Main shipped #1753 (the per-tool TestControllerBridge extension that closes #1002 — same intent as this branch) plus three follow-up phases adding bridges for media-buy-delivery, governance/property-lists/collection-lists/ content-standards, and signals/creative-delivery/creative-features. Main's implementation already includes the pagination/count drift fix (#1754 finding 1) and the dedup-on-collision semantics. Conflict resolution: took main's version of all four conflicted files (test-controller-bridge.ts, create-adcp-server.ts, server/index.ts, version.ts). Main's surface supersedes the slimmer 5-tool version this branch produced, so the bridge work itself is redundant. Dropped from this branch as duplicate work: - .changeset/bridge-per-tool-seeded-callbacks.md (main shipped equivalent changeset test-controller-bridge-per-tool.md via #1753) - 934-line test addition in test/lib/test-controller-bridge.test.js (main covers the same behavior in test/lib/seed-per-tool-wiring.test.js and reverted file matches main's surface) What remains net-additive vs main: the 11-line defensive guard in scripts/generate-wire-spec-fields.ts that keeps the codegen from overwriting the committed generated file with an empty stub on a fresh-clone build where schemas/cache/ hasn't been downloaded yet. The expert-review improvements that were applied to this branch (async-envelope guard, applySeededBridge dispatcher consolidation, adopter-responsibility JSDoc, debug log on sandbox-gate reject, mergeSeededAccountFinancials multi-entry warn) are NOT preserved here — main grew the bridge surface from 5 tools to 13, so reapplying those patterns is a different scope of work. Track as separate PR against post-#1753 main. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

bokelley · 2026-05-16T10:29:24Z

Closing as superseded.

While this PR was in review, #1753 shipped the same intent to main on 2026-05-14 (closing #1002 — the platform-proxy-seller TestControllerBridge per-tool extension). Three follow-up phases then extended the bridge surface from 5 tools to 13:

feat(testing): getSeededMediaBuyDelivery bridge callback + nit tests (#1755 phase 1) #1759 — getSeededMediaBuyDelivery + aggregated_totals recomputation + nit-test coverage on the existing bridges (phase 1).
feat(testing): governance/property-lists/content-standards/collection-lists seeded bridges (#1755 phase 2) #1761 — governance bridges: property-lists, collection-lists, content-standards (phase 2).
feat(testing): signals/creative-delivery/creative-features seeded bridges (#1755 phase 3) #1772 — signals, creative-delivery, creative-features bridges (phase 3).

Main's bridge surface now covers everything this PR targeted plus eight more tools. After merging origin/main into this branch and taking main's version of the conflicted files, the net-additive diff was just an 11-line scripts/generate-wire-spec-fields.ts build-time guard.

Carrying forward as separate PRs:

fix(codegen): guard wire-spec generator on empty schema cache — the 11-line script-only PR.
docs(bridge): trust-boundary + multi-tenant adopter responsibility — JSDoc paragraphs on TestControllerBridge naming resolveAccount as the trust boundary and multi-tenant keying as adopter responsibility. Security-review-driven; main has no public-surface warning about these.
fix(bridge): debug log on sandbox-gate reject across 13 dispatcher branches — logger.debug when isSandboxRequest(params) is true but ctx.account.sandbox !== true. Product-review-driven; cheapest fix for the AdCP Testing Framework with Authentication, Hierarchical Logging & Security Features #1 adopter support question.

Parked pending spec verification:

Async-envelope guard. Protocol expert flagged {status:'submitted', task_id} deferral via tasks/get as a real risk on get_media_buy_delivery / get_signals; security expert says the 3.0.11 response schemas don't carry an async arm on bridged read tools. Needs spec-side confirmation before shipping.

Dropped:

Table-driven applySeededBridge dispatcher. Clean at 6 branches; at 13 (with governance two-stage pick+replace + MBD aggregated-totals + creative-features oneOf) the abstraction no longer fits without per-row policy fields. Net regression risk.

The runner-visible bridge marker ask (#1775) is now load-bearing rather than nice-to-have — with 13 bridge-enabled tools, every storyboard pass that runs through fixture-merge reads identically to one that ran through a real adapter, and a compliance leaderboard will conflate the two.

…1779) * docs(bridge): name resolveAccount trust boundary + multi-tenant keying as adopter responsibility Adds two paragraphs to the top-of-file JSDoc on TestControllerBridge: 1. Scope of verification — a storyboard pass through this bridge proves wire conformance against fixture data, not adapter health against the real upstream. Sellers must still exercise adapters against a live-OAuth sandbox runner separately. Cross-references the runner-visible-bridge-marker ask at #1775. 2. Adopter responsibilities — names two patterns the SDK can't enforce: (a) resolveAccount is the trust boundary; production bindings MUST configure it or the request-signal check is the only line of defense, because the dispatcher gate falls through to permissive when ctx.account === undefined. (b) Multi-tenant keying is the adopter's job; the SDK does no defensive cross-check between fixture-entry account IDs and the resolved ctx.account. No code change. Security-review-driven during the post-merge review of #1754 — main shipped the bridge surface (#1753 + phases #1759/#1761/ #1772) but no public-surface warning about either trust pattern. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(bridge): docs-expert tightenings on JSDoc PR review - "bypassed by the post-handler merge" → "shadowed by the post-handler merge" (more accurate: the upstream is called, then its response is overridden). - "request-signal check is the only line of defense" → "buyer-supplied sandbox marker is the only gate" (flatter, more accurate). - Drop the "Snap, Meta, TikTok, Google Ads" brand list → "social / search / programmatic inventory APIs" (illustrative without pulling toward a specific adopter or violating fictional-names-only convention). - Echo a one-paragraph trust-boundary note on createAdcpServer's testController config field — that's where a wiring author lands when they hit autocomplete, and the last chance to warn before resolveAccount gets omitted. Cross-references the top-of-file JSDoc on TestControllerBridge. All four changes from the docs-expert review of PR #1779. No code behavior change; pure JSDoc. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…anches (#1780) * fix(bridge): emit logger.debug when sandbox-gate rejects on resolved-account mismatch When an adopter sets account.sandbox: true (or context.sandbox: true) on a request but resolveAccount returns sandbox: false, the dispatcher's TestControllerBridge gate rejects the merge silently. Adopters chasing "why aren't my fixtures showing in storyboards" had no diagnostic surface — the request looked sandbox-shaped but no fixtures appeared and no logs explained the gap. That was the #1 adopter support question after #1753 shipped. Adds a single logger.debug line inside create-adcp-server.ts covering all 13 dispatcher branches in one shot — the diagnostic fires after the request-sandbox check passes but before the resolved-account check. Production traffic (no sandbox marker on request) fails the first gate and never reaches this branch, so the log surface is dev-only. Three regression tests in seed-per-tool-wiring.test.js verify: gate-mismatch fires debug; production traffic does not fire debug; gate-pass does not fire debug and merge proceeds normally. Product-review-driven during the post-merge review of (now-closed) #1754. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(bridge): include resolved_account_id in sandbox-gate debug meta Product-expert review of PR #1780 flagged the log line as diagnostic but not prescriptive — it told you *what* (gate rejected) but not *what to change* (which tenant key resolved to sandbox: false). Adding resolved_account_id to the structured meta makes the line self- diagnostic for the common case: scope3-shaped adopters can match the rejected account against their resolveAccount source without correlating across log lines. account_ids appear in normal request logs already; no new PII surface. Regression test updated to assert hit.data.resolved_account_id. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

claude added 4 commits May 14, 2026 13:28

style: fix prettier formatting in test-controller-bridge files

eddd9d6

https://claude.ai/code/${CLAUDE_CODE_REMOTE_SESSION_ID}

bokelley mentioned this pull request May 14, 2026

feat(testing): extend TestControllerBridge with per-tool seeded callbacks (creatives, media-buys, accounts, financials, formats) #1002

Closed

5 tasks

github-code-quality Bot found potential problems May 14, 2026

View reviewed changes

Comment thread test/lib/test-controller-bridge.test.js Fixed

bokelley and others added 2 commits May 16, 2026 05:02

This was referenced May 16, 2026

Storyboard runner: surface bridge participation in run record #1775

Closed

Extend TestControllerBridge to remaining read tools (get_signals, get_media_buy_delivery, list_property_lists, list_collection_lists) #1776

Open

github-code-quality Bot found potential problems May 16, 2026

View reviewed changes

Comment thread src/lib/server/test-controller-bridge.ts Fixed

bokelley closed this May 16, 2026

github-actions Bot mentioned this pull request May 16, 2026

chore: release package #1749

Merged

bokelley mentioned this pull request May 16, 2026

feat(testing): bridge participation marker (#1775) #1786

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(testing): extend TestControllerBridge with per-tool seeded callbacks#1754

feat(testing): extend TestControllerBridge with per-tool seeded callbacks#1754
bokelley wants to merge 8 commits into
mainfrom
claude/issue-1002-testcontrollerbridge-per-tool-callbacks

bokelley commented May 14, 2026

Uh oh!

Uh oh!

Uh oh!

bokelley commented May 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

bokelley commented May 14, 2026

Summary

What was tested

Pre-PR review

Uh oh!

Uh oh!

Uh oh!

bokelley commented May 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants