feat(training-agent): add list_accounts handler with pagination conformance#3110
Conversation
|
Independent review by ad-tech-protocol-expert + code-reviewer (the prior internal approval was bot-resident and missed the build-time lint surface). Three merge blockers, two nice-to-haves. Merge blockers1. CI: `idempotency_key` missing on the setup phaseThe storyboard's `phase=setup step=sync_three_accounts` step omits `idempotency_key` on a mutating `sync_accounts` call. `scripts/build-compliance.cjs:251-262` keys off `schema_ref` matching a mutating schema and fails the build. Convention is: ```yaml The alias resolves to a stable UUID per run via the storyboard runner. Working precedent: `static/compliance/source/protocols/media-buy/index.yaml:167`. 2. Cursor-codec duplication with #3109This PR ships private `encodeAccountCursor` / `decodeAccountCursor` in `account-handlers.ts:104-122` with format `offset:N` (no kind prefix). #3109 (now in flight, expected to merge before this) generalized the codec into `encodeOffsetCursor(kind, offset)` / `decodeOffsetCursor(kind, cursor)` at `server/src/training-agent/task-handlers.ts:2087-2127`, with format `:offset:N`. The kind prefix prevents a `list_creatives` cursor from decoding to a meaningful offset on `list_accounts`. Replace the local helpers with: ```typescript Or move the helpers to a shared module if `account-handlers.ts` shouldn't depend on `task-handlers.ts`. Either way, single source of truth. 3. Bootstrap primitive drifts from the seeded-storyboard precedent`universal/pagination-integrity.yaml` (the canonical model from #3095/#3100) uses `controller_seeding: true` with a `fixtures.creatives:` block. This PR uses a mutating `sync_accounts` setup phase instead. The seed-DAG documented at `static/compliance/source/universal/storyboard-schema.yaml:79-112` enumerates products / pricing_options / creatives / plans / media_buys — accounts are not in the DAG. Two paths forward:
(a) is the consistent answer — without it, every list-* storyboard authored against a tool that lacks a seed scenario will keep choosing between bespoke setup phases or new seed scenarios case by case. (b) ships faster. Nice-to-haves
What's good
RecommendationThree blockers above. Prefer path (a) for the bootstrap primitive (extends the seed-DAG cleanly) but path (b) is acceptable if you want to ship sooner — just be explicit about the deviation in the storyboard's prereq description. Worth flagging upstream: the triage agent's internal review apparently doesn't run `node scripts/build-compliance.cjs` before declaring approval, which is why the idempotency_key lint failure slipped through. That's a process gap worth filing separately. |
… codec Addresses three blockers from the independent expert review on #3110: 1. **Idempotency_key.** The `sync_three_accounts` setup step on the new storyboard omitted `idempotency_key`, failing the build-time storyboard-idempotency lint. Adds the conventional `$generate:uuid_v4#<storyboard>_<phase>_<step>` alias. 2. **Cursor codec duplication.** Triage shipped private `encodeAccountCursor` / `decodeAccountCursor` in account-handlers.ts with format `offset:N`. PR #3095 (now merged) generalized the codec to a kind-prefixed pair. Moves the shared codec to `server/src/training-agent/pagination.ts` (avoids the task-handlers ↔ account-handlers circular import the in-place re-export would create) and replaces the local helpers with `encodeOffsetCursor('accounts', n)` / `decodeOffsetCursor('accounts', c)`. list_creatives and get_signals handlers move to the shared module with no behavior change. 3. **Bootstrap-pattern deviation note.** The seed-DAG documented at storyboard-schema.yaml does not include accounts; this storyboard uses `sync_accounts` as setup rather than `controller_seeding: true`. Adds explicit narrative explaining why and pointing at a future `seed_account` controller scenario. Also fixes: - **Wire-shape leak.** `accountStateToWire()` was passing `account.brand` through verbatim, including a `name` operational hint that brand-ref.json forbids (additionalProperties: false). Strips to declared fields only (`domain`, optional `brand_id`); display name and advertiser fields are still derived from the operational hint but never round-trip on the wire. Compliance fixture pool's brand entries cleaned up to match. - **Framework registration.** Triage added `handleListAccounts` to `HANDLER_MAP` but never registered the tool through the framework-server's `accounts:` block, so the SDK never advertised `list_accounts` and the storyboard's pagination steps were skipped (no-such-tool). Wires it alongside `sync_accounts`. - **Doc-parity.** Adds the new storyboard to both `docs/building/conformance.mdx` and `docs/building/compliance-catalog.mdx` per the lint that gates the build. Verified: negative-test flips `has_more` to false on the agent — first-page assertion fires with `Expected true, got false`. Reverted, clean run restored. 4/4 storyboard steps pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
0f46360 to
76c0fed
Compare
…obal pool, lint registration, doc parity Triage's #3114 implementation had four gaps the build/run surface revealed: 1. **TypeScript compile error.** `Array.from(seeded.values()) as ReturnType<typeof getFormats>` was a same-shape cast across non-overlapping types (TrainingFormat has required `name`/`description`/`renders`/`assets`; the seeded entry is `Record<string, unknown>`). Cast through `unknown` to match the contract that storyboards seed complete fixtures. 2. **Contradiction-lint registration.** `creative_formats` was added as a fixture category in storyboard-schema.yaml but missed in `scripts/lint-storyboard-contradictions.cjs`'s `FIXTURE_CATEGORY_PRIMARY_ID` map, failing the build with `unknown fixture category "creative_formats"`. Add `creative_formats: 'format_id'` alongside the other categories. 3. **Doc-parity.** Same lint that bit #3109/#3110 — adds rows for `pagination_integrity_creative_formats` to both `docs/building/conformance.mdx` and `docs/building/compliance-catalog.mdx`. 4. **Seed pool scope (the load-bearing fix).** The original implementation kept the seeded formats in `session.complyExtensions.seededCreativeFormats`, mirroring the per-session shape used by `seed_creative` / `seed_media_buy`. But `list_creative_formats` is a global catalog read with no tenant identity in its request schema — the seed call (which carries identity) and the listing call (which does not) land in different sessions, and the listing falls through to the static 37-format catalog, failing the `total_count: 2` assertion. Move the seed pool to a process-global Map (`SEEDED_CREATIVE_FORMATS`) — the test controller is sandbox-only and process-scoped anyway, so global scope is correct here. Other `seed_*` scenarios stay session-scoped because the listing calls they pair with carry identity. Mirror into session state so any test reading `complyExtensions.seededCreativeFormats` still observes it. Verified: 3/3 pagination-integrity storyboards (list_creatives, get_signals, list_creative_formats) pass clean against the training agent — 14/14 steps. Negative-test on `list_creative_formats` (flip `has_more` to false) trips the page-1 assertion as expected. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…obal pool, lint registration, doc parity Triage's #3114 implementation had four gaps the build/run surface revealed: 1. **TypeScript compile error.** `Array.from(seeded.values()) as ReturnType<typeof getFormats>` was a same-shape cast across non-overlapping types (TrainingFormat has required `name`/`description`/`renders`/`assets`; the seeded entry is `Record<string, unknown>`). Cast through `unknown` to match the contract that storyboards seed complete fixtures. 2. **Contradiction-lint registration.** `creative_formats` was added as a fixture category in storyboard-schema.yaml but missed in `scripts/lint-storyboard-contradictions.cjs`'s `FIXTURE_CATEGORY_PRIMARY_ID` map, failing the build with `unknown fixture category "creative_formats"`. Add `creative_formats: 'format_id'` alongside the other categories. 3. **Doc-parity.** Same lint that bit #3109/#3110 — adds rows for `pagination_integrity_creative_formats` to both `docs/building/conformance.mdx` and `docs/building/compliance-catalog.mdx`. 4. **Seed pool scope (the load-bearing fix).** The original implementation kept the seeded formats in `session.complyExtensions.seededCreativeFormats`, mirroring the per-session shape used by `seed_creative` / `seed_media_buy`. But `list_creative_formats` is a global catalog read with no tenant identity in its request schema — the seed call (which carries identity) and the listing call (which does not) land in different sessions, and the listing falls through to the static 37-format catalog, failing the `total_count: 2` assertion. Move the seed pool to a process-global Map (`SEEDED_CREATIVE_FORMATS`) — the test controller is sandbox-only and process-scoped anyway, so global scope is correct here. Other `seed_*` scenarios stay session-scoped because the listing calls they pair with carry identity. Mirror into session state so any test reading `complyExtensions.seededCreativeFormats` still observes it. Verified: 3/3 pagination-integrity storyboards (list_creatives, get_signals, list_creative_formats) pass clean against the training agent — 14/14 steps. Negative-test on `list_creative_formats` (flip `has_more` to false) trips the page-1 assertion as expected. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…gination (#3114) * feat(compliance): add seed_creative_format + list_creative_formats pagination (#3108) Adds `seed_creative_format` to `comply_test_controller` so the compliance harness can pre-populate a deterministic, size-controlled set of creative formats for pagination-integrity storyboards. **Schema changes:** `seed_creative_format` added to the `scenario` enum in both `comply-test-controller-request.json` and `comply-test-controller-response.json`. Request schema gains `params.format_id` (string, required for the new scenario) and a matching allOf conditional. Both files are updated atomically. **Implementation:** `seed_creative_format` is handled in `handleComplyTestController` before the SDK dispatcher (avoids UNKNOWN_SCENARIO). Idempotency enforced inline (same-fixture replay succeeds; different-fixture replay returns INVALID_STATE, matching the SEED_CACHE contract of other seed_* scenarios). `agent_url` stamped at write time so stored entries are schema-valid without a read-time patch. **Handler:** `handleListCreativeFormats` is now session-aware. When seeded formats are present they replace the static catalog, giving storyboards a knowable total_count. Cursor-based pagination added (reusing the offset-encoded cursor helpers from `handleListCreatives`). Static-catalog path is unchanged. **Storyboard:** `pagination-integrity-creative-formats.yaml` seeds 2 formats, walks pages at `max_results=1`, asserts `has_more`/`cursor`/`total_count` invariants. No `query_summary` assertions (field absent from response schema). Non-breaking: additive enum value + optional param. Existing `list_creative_formats` callers receive pagination fields in addition to `formats`; pagination block is additive per `additionalProperties: true` on the response schema. https://claude.ai/code/session_01VVBirzqi8AzidsW646iypJ * fix(compliance): resolve pre-PR review blockers on seed_creative_format - comply-test-controller-response.json: add SeedSuccess oneOf branch so seed_* responses pass schema validation (plain { success: true } failed all five existing branches which require scenarios/previous_state/ simulated/forced/error as discriminating fields) - comply-test-controller.ts: add message field to both success return paths (idempotent replay + first seed) to match other seed_* scenarios - task-handlers.ts: use `args` instead of `req` in sessionKeyFromArgs call to avoid an unsound cast (args is ToolArgs which is the expected shape; req carries ListCreativeFormatsRequest extras not needed for session key derivation) https://claude.ai/code/session_01VVBirzqi8AzidsW646iypJ * fix(compliance): unblock seed_creative_format pagination — process-global pool, lint registration, doc parity Triage's #3114 implementation had four gaps the build/run surface revealed: 1. **TypeScript compile error.** `Array.from(seeded.values()) as ReturnType<typeof getFormats>` was a same-shape cast across non-overlapping types (TrainingFormat has required `name`/`description`/`renders`/`assets`; the seeded entry is `Record<string, unknown>`). Cast through `unknown` to match the contract that storyboards seed complete fixtures. 2. **Contradiction-lint registration.** `creative_formats` was added as a fixture category in storyboard-schema.yaml but missed in `scripts/lint-storyboard-contradictions.cjs`'s `FIXTURE_CATEGORY_PRIMARY_ID` map, failing the build with `unknown fixture category "creative_formats"`. Add `creative_formats: 'format_id'` alongside the other categories. 3. **Doc-parity.** Same lint that bit #3109/#3110 — adds rows for `pagination_integrity_creative_formats` to both `docs/building/conformance.mdx` and `docs/building/compliance-catalog.mdx`. 4. **Seed pool scope (the load-bearing fix).** The original implementation kept the seeded formats in `session.complyExtensions.seededCreativeFormats`, mirroring the per-session shape used by `seed_creative` / `seed_media_buy`. But `list_creative_formats` is a global catalog read with no tenant identity in its request schema — the seed call (which carries identity) and the listing call (which does not) land in different sessions, and the listing falls through to the static 37-format catalog, failing the `total_count: 2` assertion. Move the seed pool to a process-global Map (`SEEDED_CREATIVE_FORMATS`) — the test controller is sandbox-only and process-scoped anyway, so global scope is correct here. Other `seed_*` scenarios stay session-scoped because the listing calls they pair with carry identity. Mirror into session state so any test reading `complyExtensions.seededCreativeFormats` still observes it. Verified: 3/3 pagination-integrity storyboards (list_creatives, get_signals, list_creative_formats) pass clean against the training agent — 14/14 steps. Negative-test on `list_creative_formats` (flip `has_more` to false) trips the page-1 assertion as expected. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
…, list_collection_lists, list_property_lists (#3147) * feat(training-agent): add cursor pagination to governance list handlers (#3112) Adds cursor-based pagination to list_content_standards, list_collection_lists, and list_property_lists — the fifth batch in the rolling pagination conformance series (#3095, #3100, #3109, #3110). https://claude.ai/code/session_018AkoeaWmnrsnbXBtK8FLD2 * fix(compliance): scope identity + idempotency_key + valid channel on governance pagination storyboards Triage's #3147 hit two build-time lints CI surfaced: 1. **Storyboard scoping** — 15 sample_request blocks (3 storyboards × 5 steps each, minus capability_discovery) omitted brand/account identity. The create_* and list_* tasks for governance lists are tenant-scoped per `lint-storyboard-scoping`. Adds the canonical `account: { brand: { domain: 'acmeoutdoor.example' }, operator: 'pinnacle-agency.example' }` to all 15. 2. **Idempotency_key on mutating setup steps** — 9 create_* sample_requests (3 per storyboard) on `create_collection_list`/`create_content_standards`/ `create_property_list` omitted `idempotency_key`, which their request schemas mark as required. Adds `$generate:uuid_v4#<storyboard>_setup_<step>` per the established convention. 3. **Invalid channel value** — `create_standards_2` used `channels_any: ["video"]` which isn't in the channels enum. Replace with `["olv"]` (the standardized value for online video advertising outside CTV per `static/schemas/source/enums/channels.json`). Verified: 8/8 pagination storyboards pass against the training agent (list_creatives, get_signals, list_creative_formats, list_accounts, get_media_buys, content_standards, collection_lists, property_lists) — 41/41 steps clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
Implements Brian's Option A from #3121 — three coordinated edits to .agents/routines/triage-prompt.md: 1. **Pre-PR gate now runs `npm run build && npm run typecheck`** instead of `npm run precommit`. The full `build` chains build:schemas, build:compliance (storyboard idempotency_key, contradictions, pagination invariants, doc-parity rows), and build:protocol-tarball — every lint CI runs. precommit skips the compliance build entirely, which is what bit #3110 (missing idempotency_key), #3114 (FIXTURE_CATEGORY_PRIMARY_ID gap), #3122 (cursor codec duplication), and three doc-parity gaps. The bot's "approved" verdict next to red CI was a trust-eroding signal; this fixes it. 2. **PR body now carries a "Triage-managed PR" block** documenting that the bot does not iterate on review comments — push fixup commits directly or re-trigger via /triage on the source issue. Reviewers no longer have to guess whether to wait or push. 3. **`claude-triaged` label is applied to the PR after creation** (mirrors the issue label). Searchable from PR list views; complements the claude/issue-NNNN-* branch pattern as a PR-ownership signal. Option B from the issue (build a PR review-comment iteration loop) is deferred to 4.x roadmap per Brian's decision. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Closes #3106
Summary
Implements
handleListAccountsin the training agent to unblock pagination conformance gating forlist_accounts, following thehandleListCreativespattern shipped in #3095/#3100.Changes
server/src/training-agent/account-handlers.ts— addsencodeAccountCursor/decodeAccountCursor(offset-based, same encoding as creative cursors),AccountWireShapeinterface (explicit camelCase→snake_case mapping soAccountStatefields are not blindly spread onto the wire),accountStateToWire()mapper,getComplianceAccounts()fixture pool (3 accounts for empty-session fallback),list_accountstool definition inACCOUNT_TOOLS, andhandleListAccountshandler.server/src/training-agent/task-handlers.ts— addshandleListAccountsimport andlist_accounts: handleListAccountsentry inHANDLER_MAP.static/compliance/source/universal/pagination-integrity-list-accounts.yaml— new storyboard that bootstraps 3 accounts via async_accountssetup phase, then walks the cursor↔has_more invariant withmax_results=2(first page non-terminal, second page terminal,total_counthonesty when volunteered).Non-breaking justification
Adds a new handler and endpoint registration. Existing consumers are unaffected:
handleListAccountsis a net-new export,list_accountswas previously an unregistered tool name, and the storyboard is an additive YAML file. No schema fields removed or renamed.Pre-PR review
synced_atdropped from wire shape (not incore/account.json),||→??nit appliedNits (not fixed, noted for reviewer)
payment_termsis always emitted (defaults to'net_30'); the schema marks it optional but emitting a valid value is not incorrectaccount_scope: 'brand'; the storyboard always runssync_accountsfirst so the fixture is never consulted during the compliance walkSession: https://claude.ai/code/session_01N6Bgto9hbbN5pepe9uhgxp
Generated by Claude Code