Skip to content

feat(media-buy): add opportunity and proposal decline flow#5523

Open
bokelley wants to merge 1 commit into
mainfrom
feature-feedback
Open

feat(media-buy): add opportunity and proposal decline flow#5523
bokelley wants to merge 1 commit into
mainfrom
feature-feedback

Conversation

@bokelley

@bokelley bokelley commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

Adds optional opportunity context across get_products and create_media_buy so buyers can identify a planning cycle, carry phase/horizon/deadline context, and close the opportunity without conflating it with idempotency or proposal versioning. Adds proposal-scoped action: "decline" to get_products refinement with required proposal_version, reason feedback, idempotent acknowledgement semantics, and privacy guidance. Carries proposal versions into create_media_buy so sellers can verify the accepted offer version and reject omitted, stale, unresolved, or previously declined proposal versions with the documented errors. Updates docs, source schemas, error-code guidance, changeset metadata, and regression tests for the combined opportunity/proposal-resolution flow.

Validation: npm run build:schemas, npm run test:schemas, npm run test:examples, npm run test:json-schema, npm run test:oneof-discriminators, git diff --check, and precommit hook.

@mintlify

mintlify Bot commented Jun 14, 2026

Copy link
Copy Markdown

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated (UTC)
adcp 🟢 Ready View Preview Jun 14, 2026, 2:27 PM

💡 Tip: Enable Workflows to automatically generate PRs for you.

@bokelley bokelley marked this pull request as ready for review June 14, 2026 13:26
aao-release-bot[bot]
aao-release-bot Bot previously approved these changes Jun 14, 2026

@aao-release-bot aao-release-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clean additive change. New optional fields, one new enum value (decline), and conditional required that only fires on the new action — textbook minor, and every conformant 3.0 payload validates unchanged.

Things I checked

  • Conditional logic is sound. The three allOf if/then blocks in get-products-request.json:483-549 are correct. Block (b) is the load-bearing one: if: { not: { properties:{action:{const:"decline"}}, required:["action"] } } correctly fires when action is absent (defaults to include) as well as for omit/include/finalize, forbidding reason/detail there — and correctly does NOT fire for decline. The naive action:{not:{const:"decline"}} would have wrongly passed when action is omitted. This avoids that footgun.
  • oneOf-of-const reason follows the established AdCP idiom, not a divergence — same inline scalar pattern as core/start-timing.json:6-17, used across ~88 source files. The audit walker classifies it as scalar (scripts/audit-oneof.mjs:145), so it's excluded from the baseline and --check. No undiscriminated-oneOf regression. The parent refine-entry oneOf stays discriminated on scope.
  • enumMetadata parity for INVALID_STATE. Both halves updated — the description map (error-code.json:138) and the metadata block (error-code.json:341-344) — and recovery stays "correctable" on both sides, honoring the metadata invariant.
  • Test assertions match AJV v8 output. ['proposal_version'], ['reason'], ['must NOT be valid'] (the not keyword's exact message), ['detail'], and ['proposal_id'] (the new dependencies.proposal_version:["proposal_id"]) all map to the right failing keyword. The seven cases cover the right invariants.
  • additionalProperties:false interaction is safe. The new properties are declared in the variant's properties block, so false sees them; the allOf adds only required, never new properties. A decline payload matches only the proposal variant. No regression.
  • Changeset present (proposal-decline-refinement.md), minor, correct for additive optional fields. No generated files touched.

Follow-ups (non-blocking — file as issues)

  • Docs don't state the non-decline rejection rule. Both code-reviewer and docs-expert flagged it: schema block (b) hard-rejects reason/detail on any non-decline proposal action, and there's a test for it (tests/example-validation-simple.test.cjs:628), but no prose says so. An agent attaching reason to an include entry hits a schema rejection the docs never warned about. One line in specification.mdx or the get_products.mdx reason row closes it.
  • Runtime obligations the schema can't express. The reject-omitted-version→INVALID_REQUEST, stale→CONFLICT, declined→INVALID_STATE MUSTs in specification.mdx/create_media_buy.mdx are seller-side runtime checks (correctly — JSON Schema only enforces proposal_versionproposal_id co-presence). Worth confirming the conformance storyboards cover these rejections, since the schema gate can't.

Minor nits (non-blocking)

  1. proposal_version length bounds are consistent. core/proposal.json (minLength:1), create-media-buy-request.json (minLength:1,maxLength:255), and the request-side field all agree. Good.

ad-tech-protocol-expert: sound-with-caveats (caveats are docs/runtime division-of-labor, not schema defects). code-reviewer: clean. docs-expert: clean, one optional polish. Note main is in pre-mode (rc tag) so this ships as 3.1.0-rc.Nminor is still the right declaration.

LGTM. Follow-ups noted below.

@bokelley bokelley changed the title feat(media-buy): add proposal decline refinement feat(media-buy): add opportunity and proposal decline flow Jun 15, 2026
aao-release-bot[bot]
aao-release-bot Bot previously approved these changes Jun 15, 2026

@aao-release-bot aao-release-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additive across the board — new optional opportunity, proposal_version, and a fourth proposal action — so minor is the right changeset and nothing on the wire flips from valid to invalid. The decline/version lifecycle is the right shape: proposal_id + proposal_version identifies one offer state, opportunity_id spans the planning cycle, and the schema is explicit that neither is an idempotency key.

Things I checked

  • Conditional logic, three blocks in get-products-request.json proposal entry. action: "decline" requires proposal_version + reason; non-decline (including omitted action) forbids reason/detail; reason: "other" requires detail. All three compose correctly — a valid decline passes blocks 2 and 3 vacuously. Confirmed against code-reviewer's case-by-case ajv v8 trace.
  • opportunity-context.json close gate (L466-497). if not(status===closed present) then not(close_reason or close_detail) correctly forbids close fields on absent/open status and allows them only on closed. Test close_reason requires closed status exercises it.
  • additionalProperties: false is safe. proposal_version/reason/detail are declared in the variant's own properties; the new allOf if/then blocks add only required/not, no new property names — no additionalProperties strip footgun.
  • oneOf audit. The new reason field is oneOf of pure consts → classified scalar by audit-oneof.mjs:144 and filtered from the baseline at L307/L322. No discriminator regression. The proposal oneOf stays discriminated on scope.
  • Error codes all pre-exist. INVALID_STATE (broadened, both enumDescriptions and enumMetadata.suggestion updated in parallel — parity held), CONFLICT, PROPOSAL_NOT_FOUND, INVALID_REQUEST. No new code, no error-code lint surface.
  • Changeset present (.changeset/proposal-decline-refinement.md, minor), wire-touching PR, correct type. ad-tech-protocol-expert: sound-with-caveats; docs-expert: no enum/phase/close_reason value drift between schema and the five .mdx files.
  • Dependency proposal_version → proposal_id (one-directional) — correct; unversioned proposal_id still allowed.

Follow-ups (non-blocking — file as issues)

  • reason modeling breaks enumDescriptions parity. It's type: string + oneOf of {const, description}, which the build does not harvest into enumDescriptions/enumMetadata — so SDK and doc generators get nothing for the decline taxonomy. The wire shape is identical (same 10 strings), so this is internal polish, not a block. The convention is a shared enums/*-reason.json with enum + enumDescriptions (action-not-allowed-reason.json, makegood-remedy.json). Worth extracting enums/proposal-decline-reason.json and $refing it. Notable that close_reason/phase/status in this same PR use plain enumreason is the one outlier.
  • CONFLICT is recovery: transient (concurrent-write). The PR overloads it for a stale proposal_version (optimistic-version mismatch). Defensible — re-read-and-retry is the transient contract — but extend the CONFLICT enumDescriptions prose to name the stale-version case so SDKs don't treat it as a pure race.
  • opportunity-context.json not registered in static/schemas/source/index.json. Won't fail CI (registry check doesn't enforce completeness), but every sibling core schema is listed. Add the entry.

Minor nits (non-blocking)

  1. refinement.mdx proposal-action table omits a proposal_version column. get_products.mdx lists it as a first-class conditional field; refinement.mdx folds it into the decline row's free text. Not a contradiction, just less scannable.
  2. The central proposal_version MUST is runtime-only. specification.mdx and create_media_buy.mdx require sellers to reject a versioned-proposal execution that omits proposal_version (INVALID_REQUEST), but the request schema can't know whether the referenced proposal was versioned, so it stays optional. Docs frame this correctly as a seller-side rule — flagging so reviewers know it's not schema-caught.

LGTM. Follow-ups noted below.

@aao-release-bot aao-release-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clean additive change. The opportunity/proposal-decline split is the right shape: opportunity_id spans the planning cycle, proposal_id + proposal_version pins one offer state, and neither is conflated with idempotency_key. The conditional logic is load-bearing and correct.

Things I checked

  • oneOf audit is clean. The new reason taxonomy in get-products-request.json is a oneOf of const branches — scripts/audit-oneof.mjs classifies all-const oneOf as scalar (variantInfo L111), so it's neither flagged nor baselined. No discriminator regression.
  • Decline gating fires correctly. additionalProperties: false on the proposal refine branch recognizes the new proposal_version/reason/detail because they're declared in that branch's own properties; the allOf only adds required/not(required). Traced all four branches: decline requires version+reason (cond 1), non-decline forbids reason/detail even when action is omitted (cond 2, not-wrapped — no leak), reason: other requires detail (cond 3). code-reviewer: no blockers, manual semantic trace passes on every branch.
  • Close-won asymmetry is deliberate. opportunity-context.json permits all 7 close_reason values when status: closed; create-media-buy-request.json narrows to accepted_with_seller only on create. Loss reasons belong on a get_products refine close. Consistent with docs.
  • error-code parity holds. INVALID_STATE updated in both enumDescriptions (L885) and enumMetadata suggestion (L894), recovery: correctable unchanged. Reusing INVALID_STATE for declined-then-executed is the right call — it already means "operation not permitted for terminal lifecycle state."
  • Changeset is right. minor, purely additive: decline is an added enum value, every new field is optional or conditionally-required on a brand-new branch. No required↔optional flip, no removal. ad-tech-protocol-expert: sound-with-caveats, nothing here is a breaking wire change.
  • No schema-vs-docs drift. reason (10 values) and close_reason (7) match their schema enums exactly across get_products.mdx, specification.mdx, refinement.mdx; the two taxonomies are intentionally distinct (proposal-version-level vs opportunity-level), not drift. Changeset present.
  • Compliance scenario. proposal_decline.yaml covers acknowledge → idempotent duplicate → reject-create-after-decline with INVALID_STATE (L685-688), wired into both sales-guaranteed and sales-proposal-mode. end_time: 2027-... dodges the concrete-date lint.
  • Tests. 13 new assertions in example-validation-simple.test.cjs (L1188-1400) exercise every positive and negative branch including proposal_version requires proposal_id and closed opportunity rejects non-accepted close_reason.

Follow-ups (non-blocking — file as issues)

  • create-media-buy-response.json doesn't echo proposal_version. The request-side contract is fully specified, but the response is silent on which offer version the seller executed. When versions race, the buyer can't reconcile what it bought from the response alone — it has to re-read via get_media_buys. Adding an optional echo stays minor. (ad-tech-protocol-expert.)
  • CONFLICT recovery class is transient but a stale proposal_version is correctable-by-re-read. This PR is the first to route a value-mismatch to CONFLICT. The docs route it correctly ("re-read or refine"), but an SDK trusting enumMetadata.CONFLICT.recovery = transient could auto-retry the stale version and loop. Worth a one-line spec note that this CONFLICT is correctable, not blindly retryable.
  • Response-side "MUST echo proposal_version on decline ack" is prose-only. The response schema can't see the request action, so validation alone won't catch a seller that drops the echo — only the compliance scenario does. Expected, just don't assume the schema guarantees it.

Minor nits (non-blocking)

  1. reason uses oneOf of const; the parallel close_reason uses a plain enum. Both validate identically. The per-const form buys per-value descriptions — fine — but the inconsistency between two sibling taxonomies in the same PR is gratuitous.

Approving on the strength of two clean expert verdicts plus a full trace of the conditional gates. Ship it once CI confirms test:examples / test:json-schema / test:oneof-discriminators green.

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.

1 participant