Skip to content

fix(testing): storyboard request-builders emit spec shapes for log_event + create_media_buy (#793)#794

Merged
bokelley merged 1 commit into
mainfrom
bokelley/issue-793
Apr 22, 2026
Merged

fix(testing): storyboard request-builders emit spec shapes for log_event + create_media_buy (#793)#794
bokelley merged 1 commit into
mainfrom
bokelley/issue-793

Conversation

@bokelley

Copy link
Copy Markdown
Contributor

Summary

Fixes #793 — two storyboard REQUEST_BUILDERS in src/lib/testing/storyboard/request-builder.ts emit non-spec payloads. Framework-dispatch agents running zod at the MCP boundary reject them with -32602 invalid_type; legacy-dispatch permissively lets them through. Surfaced while diagnosing adcontextprotocol/adcp#2872.

Bug A — log_event

  • Fallback emitted timestamp (spec: event_time) and value: { amount, currency } (spec: custom_data.{value,currency} per core/event.json + core/event-custom-data.json).
  • Builder never consulted step.sample_request, so storyboards (sales_catalog_driven, sales_social) that author spec-conformant events with event_time / content_ids / siblings were silently discarded.

Fix: honor step.sample_request first (same convention as sync_catalogs, report_usage, update_media_buy, sync_creatives); correct the synthetic fallback's field names.

Bug B — create_media_buy

  • Builder always emitted packages: [pkg] regardless of how many packages the storyboard's sample_request authored. Multi-package storyboards (sales_non_guaranteed) lost packages[1+], leaving context_outputs like second_package_id unresolved — the next step (update_media_buy using $context.second_package_id) was then skipped with "unresolved context variables from prior steps". Reproduced under both framework and legacy dispatch.

Fix: emit every authored package. First package still receives context-derived product_id / pricing_option_id (so single-package storyboards against arbitrary sellers keep working); additional packages pass through with context injection only, preserving per-package product_id, bid_price, pricing_option_id, and creative_assignments.

Tests

Four new regression tests in test/lib/request-builder.test.js:

  • create_media_buy emits every package when sample_request authors multiple.
  • create_media_buy injects context into additional packages.
  • log_event fallback uses spec field names (event_time, custom_data).
  • log_event honors step.sample_request with context injection.

All 37 request-builder tests and 1396 storyboard-completeness tests pass.

Deeper structural issues (filing as follow-ups)

Expert review (code-reviewer + ad-tech-protocol-expert) flagged a broader structural issue and several additional spec-violating builders that are out of scope for this PR:

  • Structural: the runner's default is inverted. REQUEST_BUILDERS entries take precedence over sample_request unless the builder explicitly opts in with if (step.sample_request) return injectContext(...) — a pattern that's now duplicated in ~12 builders. The correct default is "sample_request is authoritative; builder fills discovery-derived identifiers." Will file as a separate refactor.
  • Additional spec violations verified against schemas/cache/latest/:
    • si_get_offering emits context as a string, but core/context.json is an object ref.
    • si_initiate_session stuffs the intent string into context; schema requires intent + identity per si-identity.json.
    • sync_governance emits credentials: "test-governance-token" (21 chars) but schema requires minLength: 32; also never honors step.sample_request.
    • sync_event_sources, sync_audiences (narrow delegation), list_content_standards, sync_accounts — same "ignores sample_request" class as Bug A.

Both experts also suggested a schema-driven invariant test that round-trips every builder's output through the generated Zod schema. Worth adding alongside the structural refactor.

Test plan

  • node --test test/lib/request-builder.test.js — 37 pass
  • node --test test/lib/storyboard-completeness.test.js — 1396 pass
  • npm run build — clean
  • tsc --noEmit — clean
  • CI green on this PR

🤖 Generated with Claude Code

…ent + create_media_buy (#793)

`log_event` now honors `step.sample_request` and uses spec field names
(`event_time` + `custom_data.{value,currency}`) in the synthetic
fallback. `create_media_buy` emits every authored package instead of
dropping `packages[1+]`; the first package keeps context-override for
discovery-derived identifiers, additional packages pass through with
context injection only.

Framework-dispatch agents running zod at the MCP boundary previously
rejected these with -32602 invalid_type; legacy-dispatch let them
through. Surfaced while diagnosing adcontextprotocol/adcp#2872.
@bokelley
bokelley merged commit 5e52efa into main Apr 22, 2026
9 checks passed
bokelley added a commit that referenced this pull request Apr 22, 2026
…803) (#807)

* test(testing): schema round-trip invariant for every request builder (#803)

Iterate every task in `TOOL_REQUEST_SCHEMAS` (plus the two brand-rights
schemas that ship outside the MCP tool surface) with a matching storyboard
request builder, build its fallback request, and assert it parses cleanly
against the generated Zod schema. Catches the #789 / #792 / #793 / #794
class of drift before it ships — any builder that emits off-spec field
names or types now fails CI.

Also fixes eight pre-existing fallbacks the invariant surfaced:

- `update_media_buy.packages[0].package_id` defaulted
- `update_rights` + `creative_approval` use spec `rights_id` (not
  `rights_grant_id`); `creative_approval` emits `creative_url` + `creative_id`
- `sync_creatives` + `buildAssetsForFormat` emit the `asset_type`
  discriminator and spec-correct video fields
- `calibrate_content` + `validate_content_delivery` artifact `assets` is
  an array of typed assets, not an object map
- `activate_signal` defaults `destinations` to a placeholder agent entry
- `create_content_standards` + `update_content_standards` align with the
  current `scope` + `policies` shape
- `si_get_offering` + `si_initiate_session` pass `options.si_context`
  through `intent` (string) instead of `context` (ContextObject);
  `si_initiate_session` emits the required `intent`

Closes #803.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* test(testing): address review - MUTATING_TASKS, enumerated coverage, si intent default

Expert review feedback on PR #807:

- Use exported `MUTATING_TASKS` from `src/lib/utils/idempotency.ts` instead
  of re-implementing Zod shape introspection in the test. Handles the two
  brand-rights tasks (creative_approval, update_rights) via a small
  EXTRA_MUTATING set — both are mutating per their generated schemas but
  intentionally live outside TOOL_REQUEST_SCHEMAS.

- Replace ">= 30 covered" floor with an enumerated EXPECTED_COVERED_TASKS
  list. Adding a new builder/schema pair now forces a conscious list
  update rather than drifting past a magic number.

- `si_initiate_session` intent default is now "Browse available offerings"
  (a real user-ask) instead of "E2E test session" (test metadata). Intent
  dispatches meaningfully on SI agents; storyboards override via
  sample_request for intent-specific paths.

- Regenerate docs/llms.txt and docs/TYPE-SUMMARY.md (upstream compliance-
  cache update landed while this branch was open).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* chore: sync version.ts + package-lock.json to 5.11.0

`sync-version` regenerated `src/lib/version.ts` and the package-lock to
match `package.json` (5.11.0). The branch's version.ts had been carried
over from a 5.10.0-era rebase point.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Storyboard request-builders emit out-of-spec shapes for log_event / create_media_buy (drops packages[1+], wrong field names)

1 participant