fix(testing): get_media_buys / get_media_buy_delivery skip 'unknown' fallback (#983)

[bokelley]

Summary

Closes #983.

request-builder.get_media_buys and get_media_buy_delivery always returned { media_buy_ids: [context.media_buy_id ?? 'unknown'] }. When a storyboard's sample_request omitted media_buy_ids — e.g. testing the broad-list / pagination path — the merge { ...enriched, ...fixture } kept the ["unknown"] placeholder because the fixture didn't override it. Agents received a placeholder ID, returned 0 matches, and pagination assertions failed.

Fix

Per get-media-buys-request.json the field is optional (minItems: 1). Omitting it asks for "a paginated set of accessible media buys matching status_filter" — a conformant broad-list query. Both enrichers now return {} when context lacks a real media_buy_id; the fixture's sample_request becomes authoritative.

This mirrors peer paginated-read enrichers (list_creatives, list_creative_formats, list_accounts, get_signals) — none of which fabricate IDs.

For get_media_buy_delivery (which requires media_buy_ids per the spec), the same change converts the silent NOT_FOUND failure mode into surfacing INVALID_REQUEST errors that authors can debug.

Behavior

Context	Fixture	Result
has real media_buy_id	omits the field	injects [context.media_buy_id] (unchanged)
has real media_buy_id	declares media_buy_ids	fixture wins (unchanged)
no real id	omits the field	media_buy_ids absent on the wire (broad-list path)
no real id	declares media_buy_ids	fixture is authoritative

Test plan

• 8 new tests in test/lib/request-builder.test.js (62/62)
• 2528/2532 in the storyboard regression sweep (4 intentional skips)
• tsc --noEmit clean, prettier --check clean, eslint 0 errors
• CI green

Unblocks

adcontextprotocol/adcp#3122 — once this fix ships, the get-media-buys-pagination-integrity storyboard returns to its multi-page walk against the broad-list path.

🤖 Generated with Claude Code

[bokelley]

Expert review applied

Both reviewers (code-reviewer + ad-tech-protocol-expert) converged on the same blocker: my JSDoc + changeset claim that get_media_buy_delivery "requires media_buy_ids per the spec" is factually wrong — neither get-media-buys-request.json nor get-media-buy-delivery-request.json has a top-level required[] array. media_buy_ids is optional on both.

Fixed in commit d78b454:

• JSDoc on get_media_buy_delivery — rewrote to drop the spec-violation claim and explain the actual semantics (placeholder produced phantom NOT_FOUND; empty enricher lets fixtures be authoritative).
• JSDoc on get_media_buys — added a one-line audit reference to the peer paginated-read enrichers (list_creatives, list_creative_formats, list_accounts, get_signals) — none fabricate ids.
• Changeset — dropped the spec-violation framing; added a Migration paragraph for storyboards that previously relied on the phantom-NOT_FOUND path.
• Test parity — added fixture sample_request wins over context injection for get_media_buys (the symmetric test already existed for get_media_buy_delivery).
• Non-string context types — parametric test covering null, undefined, 42, ['mb_x'], {id: 'mb_x'}, false to lock the guard's behavior against future refactors.

Filed for follow-up

#989 — audit other request-builder enrichers for the same ?? 'unknown' placeholder pattern. Code-reviewer flagged 12 other call sites; some are mutating (the placeholder is acceptable — surfaces NOT_FOUND on context-wiring gaps), some are paginated reads (potentially same bug as #983). Out of scope here; tracked separately.

Test count

64 tests in request-builder.test.js (was 56). All pass; 2528/2532 in the wider storyboard regression sweep (4 intentional skips, none introduced).

Latest commit: d78b454.

[bokelley]

Approved

ad-tech-protocol-expert review: approve.

Schema verification — schemas/cache/3.0.0/bundled/media-buy/get-media-buys-request.json and …/get-media-buy-delivery-request.json both lack a top-level required array; media_buy_ids is optional in both (minItems: 1 only constrains presence-when-supplied). get_media_buys explicitly documents the broad-list path ("When omitted, returns a paginated set…"). The fabricated "unknown" was wire-non-conformant.

Returning {} and letting sample_request win is the right invariant: builder fills only what context actually knows, fixture remains authoritative. Peer enrichers (list_creatives, list_creative_formats, list_accounts) confirm the pattern.

Caveat (non-blocking): get-media-buy-delivery-request.json is silent on broad-list semantics. Sellers could legitimately reject id-less calls with INVALID_REQUEST. That's a spec gap to file against adcontextprotocol/adcp — not a PR defect.

[bokelley]

Closing as superseded — the same fix landed via PR #987 (commit fc70b9a3).

Verified on origin/main at src/lib/testing/storyboard/request-builder.ts:

• get_media_buys (line 325): if (context.media_buy_id == null) return {} — same posture as this PR
• get_media_buy_delivery (line 332): same

The functional fix is identical. #987 used a slightly more concise expression; this PR's longer JSDoc explanation and broader test coverage (broad-list intent, fixture-wins, empty-string handling) are nice-to-have but not essential — the core 4 tests on main cover both enricher behaviors.

The expert review summary documented for posterity in #988's comment thread.

Closes adcp-client#983 (already closed via #987).