feat(compliance): pagination integrity for get_media_buys#3122
Merged
Conversation
Add cursor-based pagination to handleGetMediaBuys and a new
get-media-buys-pagination-integrity conformance storyboard.
- Skip pagination when media_buy_ids is present (direct lookup semantics
per the request schema; callers expect all requested IDs back)
- Broad-scope queries: read max_results (default 50, cap 100), decode a
namespaced mb:offset:<n> base64url cursor, slice post-filter buys, emit
pagination: { has_more, total_count, cursor? }
- mb: prefix prevents cross-endpoint cursor reuse with list_creatives
- Return INVALID_REQUEST on malformed cursor
- Storyboard: 3 active media_buy fixtures, capability-discovery + 2-page
walk (first page has_more=true+cursor, terminal page has_more=false+no cursor)
- Three unit tests: pagination walk, malformed cursor, ID-lookup bypass
Closes #3111
https://claude.ai/code/session_01YQwRdKHrxUzQrYEGk2dMDT
…_ids conflict When media_buy_ids and pagination.cursor are both present, the cursor is intentionally ignored — ID-lookup semantics win. Adds explicit comment to make this design decision undeniable (flagged in pre-PR protocol review). https://claude.ai/code/session_01YQwRdKHrxUzQrYEGk2dMDT
…storyboard for SDK gap, fix test data
843321c to
afd99fa
Compare
bokelley
added a commit
to adcontextprotocol/adcp-client
that referenced
this pull request
Apr 25, 2026
…fallback (#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 (testing the broad-list / pagination path), the merge kept the placeholder because the fixture didn't override it. Agents received a placeholder ID, returned 0 matches, and pagination assertions failed. Per get-media-buys-request.json the field is optional (minItems: 1). Omitting it asks for a paginated set of accessible media buys — a conformant broad-list query. Mirroring peer paginated-read enrichers (list_creatives, list_creative_formats, list_accounts, get_signals) — none of which fabricate IDs — both enrichers now return empty when context lacks a real media_buy_id. The fixture's sample_request becomes authoritative on that path. For get_media_buy_delivery (which requires media_buy_ids per the spec), the same change turns silent NOT_FOUND responses into surfacing INVALID_REQUEST errors that authors can debug. 8 new tests in test/lib/request-builder.test.js. Unblocks adcontextprotocol/adcp#3122 (get_media_buys pagination conformance storyboard). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bokelley
added a commit
that referenced
this pull request
Apr 25, 2026
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #3111
Summary
Fourth in the rolling pagination conformance series (#3095, #3100, #3109). Adds cursor-based pagination to
handleGetMediaBuysand a newget-media-buys-pagination-integrity.yamlconformance storyboard.Non-breaking justification:
paginationwas already an optional field inget-media-buys-response.json($ref: core/pagination-response.json). Existing callers receive an additional field they can safely ignore; themedia_buysarray shape is unchanged.Changes
server/src/training-agent/task-handlers.tspagination?: { max_results?: number; cursor?: string }toGetMediaBuysArgs(widening in case the generatedGetMediaBuysRequesttype omits it)media_buy_idsis present → direct lookup, pagination skipped, all matching buys returned (per request-schema semantics: "When omitted, returns a paginated set")media_buy_idsis absent → readsmax_results(default 50, cap 100), decodes cursor, slices post-filter buys, emitspagination: { has_more, total_count, cursor? }INVALID_REQUESTon malformed cursorencodeMediaBuyCursor/decodeMediaBuyCursorusingmb:offset:<n>prefix — namespaced so aget_media_buyscursor is rejected bydecodeCreativeCursor(and vice versa), preventing silent wrong-offset reads. Will unify with sharedencodeOffsetCursoronce feat(compliance): get_signals pagination cursor integrity #3109 mergesDesign note: when both
media_buy_idsandpagination.cursorare provided, the cursor is intentionally ignored — ID-lookup semantics win (documented in code comment; flagged and accepted in pre-PR protocol review).static/compliance/source/universal/get-media-buys-pagination-integrity.yamlmedia_buyfixtures viacontroller_seeding: truefirst_page(max_results=2, assert has_more=true + cursor present) →terminal_page(follow cursor, assert has_more=false + cursor absent)query_summaryassertions —get-media-buys-response.jsondoes not define that fieldserver/tests/unit/training-agent.test.ts— 3 new tests:INVALID_REQUESTmedia_buy_idsbypasses pagination → all 3 returned, nopaginationblockPre-PR review
encodeCreativeCursornamespace tech-debt inherited from pre-existing code)total_countsemantics correct (post-filter, pre-slice); storyboard correctly omitsquery_summary; silent-ignore contract for cursor+ID-lookup conflict is documented in code commentNits noted (not fixing in this PR)
InMemoryStateStore— this is correct behavior, a comment would help future readersGetMediaBuysArgs.paginationuses a local type literal rather than the canonicalPaginationRequesttype from@adcp/client— low blast radius now, will matter more once feat(compliance): get_signals pagination cursor integrity #3109 ships the shared codecSession: https://claude.ai/code/session_01YQwRdKHrxUzQrYEGk2dMDT
Generated by Claude Code