fix: support seed_account comply controller#2288
Conversation
There was a problem hiding this comment.
Clean additive wiring of seed_account across all five touch points. Right shape: the dispatcher fails closed — UNKNOWN_SCENARIO when the store omits seedAccount, INVALID_PARAMS when params.account_id is absent — and the storyboard seeder lifts account_id into params with the rest unwrapped into fixture, byte-for-byte the established seed_* contract.
Things I checked
- Spec-faithful, not fabricated.
seed_accountis named in the spec request schema (src/lib/types/tools.generated.ts:25479— "Account to ... seed. Used by ... seed_account"). The SDK is wiring a defined scenario, not inventing one.ad-tech-protocol-expert: sound. - The
as unknown as ControllerScenariocast (comply-controller.ts:655) is correct and necessary —seed_accountis absent from the v3-1-betaListScenariosSuccess.scenariosenum (src/lib/types/v3-1-beta/tools.generated.ts:23492-23512, which listsseed_productbut notseed_account). The enum docstring normatively mandates open-extension ("Runners and sellers MUST accept unknown scenario strings"), so this is the house convention forseed_buyer_agent/force_audience_status, not a new smell. - Fail-closed dispatch at
test-controller.ts:904-916—account_idchecked beforestore.seedAccount, cache keyseed_account:${accountId}matches the per-(kind,id) convention, sharedfixturederivation inherits the prototype-pollution guard. - No mock-data injection.
fixture ?? {}passes straight through to the registered adapter (comply-controller.ts:519-521). Witness, not translator — preserved. - Changeset present (
.changeset/seed-account-controller.md),patch. Both experts confirm patch is right for an additive opt-in testing scenario. - Tests cover routing,
INVALID_PARAMS,UNKNOWN_SCENARIO, fixture unwrapping (nested{account_id, fixture}and flat-field shapes), ordering, and capability projection throughfrom-platform.ts.
Follow-ups (non-blocking — file as issues)
- The v3-1-beta
ListScenariosSuccess.scenariosenum omitsseed_accountwhile the request schema names it. A buyer running closed-world schema validation against the beta schema could reject alist_scenariosresponse that advertisesseed_account. This is an upstream schema lag, not an SDK bug — file an issue onadcontextprotocol/adcpto addseed_accountto the response enum, then the cast can be dropped when codegen catches up. Triage order (spec → mock → SDK) confirms the spec defines the contract; the enum is the lagging artifact.
Minor nits (non-blocking)
- Cast line placement.
comply-controller.ts:641puts the newseed_accountpush (which needs the cast) directly above the un-castseed_productgroup, rather than below the comment at line 648 that explains the extension-cast convention alongsideseed_buyer_agent/force_audience_status. Reads slightly anomalous — a cast line in the no-cast block. Cosmetic.
Approving on the strength of the fail-closed dispatch plus both experts returning sound.
There was a problem hiding this comment.
Clean additive change. seed_account mirrors the existing seed_product/seed_buyer_agent extension pattern at every layer — store, dispatcher, typed facade, capability projection, and storyboard seeding — so the new scenario inherits every existing guard rather than carving a new path.
Things I checked
seed_accountis real protocol surface, not fabricated.ad-tech-protocol-expert: sound —seed_accountis enumerated in the generated schema (tools.generated.ts:25467), withparams.account_iddocumented (:25479) andscenarioan openstringfield. The PR is filling a genuine SDK enumeration/dispatch gap, exactly as the body claims.- Dispatch correctness (
test-controller.ts:906-916): missingaccount_id→INVALID_PARAMS; absentstore.seedAccount→UNKNOWN_SCENARIO. Param-check-then-store-check ordering matches every sibling case. The prototype-pollution guard (:882-892) and fixture-must-be-object guard run before the switch, so the account fixture inherits both. - The
as unknown as ControllerScenariocast (comply-controller.ts:651) is the established escape hatch for scenarios not yet widened into the narrowerListScenariosSuccess['scenarios']response union — identical toseed_buyer_agent/force_audience_statuson the adjacent lines. Witness, not translator:seedFixtureFromFieldspasses fixture fields through verbatim, only coercingfixture ?? {}for an absent fixture. - No new auth surface.
security-reviewer: the sandbox-authority gate (from-platform.ts:1799-1897) keys on the resolved-accountmode, not the scenario string — adding a switch arm cannot widen it.seed_accounthas no auto-seed fallback (auto-seed only fillsproduct/pricing_option), so nothing writes account fixtures unless the adopter explicitly wiresseed.account. No fixture logging intest-controller.ts;safeEchonever reflectsparams/fixture. buildSeedCallsaccount_id-lifting (seeding.ts:80-98): destructuresaccount_id, routes the rest through the sharedseedFixtureFromFields, handles both nested-fixtureand direct-field storyboard shapes, and emits anauthoring_errorwithout issuing the seed on an empty/non-string id. Tests cover both shapes plus ordering and the authoring-error case.- Changeset present,
Changeset Checkwill pass.
Follow-ups (non-blocking — file as issues)
- The
list_scenariosresponse union (ControllerScenario) lags the open scenario set, so each new extension scenario accrues anas unknown ascast inadvertisedScenarios. Worth widening the codegen source so the cast pile stops growing —ad-tech-protocol-expertflagged the same thing. patchis defensible (the repo treats these test-controller extension adapters as patch-level additive surface, andSeedAccountParams+seed.accountare purely additive with no behavior change). Strictly, a new exported type leansminor— note it for the next version-bump audit, but it doesn't understate impact in a way that breaks semver consumers.
Minor nits (non-blocking)
SeedAccountParamsJSDoc invites secrets into fixtures. The "billing/rate-card metadata, authorization" wording (comply-controller.ts:97-103) could nudge adopters toward live credentials in sandbox fixtures. A one-line pointer todocs/guides/CTX-METADATA-SAFETY.mdwould close that. Docs-only.
LGTM. Follow-ups noted below.
Summary
seed_accountsupport to the lower-levelcomply_test_controllerstore/dispatcher andcreateComplyControllertyped facade.seed_accountcalls fromfixtures.accounts[], including the current{ account_id, fixture }storyboard shape.seed.accountin platform capability projection soget_adcp_capabilities.compliance_testing.scenariosmatcheslist_scenarios.Root Cause
seed_accountwas present in the protocol schema, but the SDK helper surfaces did not enumerate or dispatch it. Storyboard seeding also ignoredfixtures.accounts[], so pagination-integrity account fixtures were never seeded.Validation
npm run build:libNODE_ENV=test node --test-timeout=60000 --test-force-exit --test test/comply-controller.test.jsNODE_ENV=test node --test-timeout=60000 --test-force-exit --test test/server-test-controller.test.jsNODE_ENV=test node --test-timeout=60000 --test-force-exit --test test/lib/storyboard-controller-seeding.test.jsNODE_ENV=test node --test-timeout=60000 --test-force-exit --test test/server-decisioning-comply-test.test.jsgit diff --checkReview
Ran parallel expert review (protocol, code review, Node.js testing). Initial findings on account fixture unwrapping and capability projection were fixed; re-checks reported no remaining actionable issues.
Local Repro
Against
origin/main,createComplyController({ seed: { account } })returnsUNKNOWN_SCENARIOforseed_accountand advertises no seed scenario. This branch advertisesseed_account, dispatches to the adapter, and emits the threeseed_accountcalls from the cachedpagination-integrity-list-accountsstoryboard.