feat(media-buy): add opportunity and proposal decline flow#5523
feat(media-buy): add opportunity and proposal decline flow#5523bokelley wants to merge 1 commit into
Conversation
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
There was a problem hiding this comment.
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
allOfif/then blocks inget-products-request.json:483-549are correct. Block (b) is the load-bearing one:if: { not: { properties:{action:{const:"decline"}}, required:["action"] } }correctly fires whenactionis absent (defaults toinclude) as well as foromit/include/finalize, forbiddingreason/detailthere — and correctly does NOT fire fordecline. The naiveaction:{not:{const:"decline"}}would have wrongly passed whenactionis omitted. This avoids that footgun. oneOf-of-constreasonfollows the established AdCP idiom, not a divergence — same inline scalar pattern ascore/start-timing.json:6-17, used across ~88 source files. The audit walker classifies it asscalar(scripts/audit-oneof.mjs:145), so it's excluded from the baseline and--check. No undiscriminated-oneOf regression. The parent refine-entryoneOfstays discriminated onscope.- enumMetadata parity for INVALID_STATE. Both halves updated — the description map (
error-code.json:138) and the metadata block (error-code.json:341-344) — andrecoverystays"correctable"on both sides, honoring the metadata invariant. - Test assertions match AJV v8 output.
['proposal_version'],['reason'],['must NOT be valid'](thenotkeyword's exact message),['detail'], and['proposal_id'](the newdependencies.proposal_version:["proposal_id"]) all map to the right failing keyword. The seven cases cover the right invariants. additionalProperties:falseinteraction is safe. The new properties are declared in the variant'spropertiesblock, sofalsesees them; theallOfadds onlyrequired, 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-revieweranddocs-expertflagged it: schema block (b) hard-rejectsreason/detailon any non-declineproposal action, and there's a test for it (tests/example-validation-simple.test.cjs:628), but no prose says so. An agent attachingreasonto anincludeentry hits a schema rejection the docs never warned about. One line inspecification.mdxor theget_products.mdxreasonrow closes it. - Runtime obligations the schema can't express. The reject-omitted-version→
INVALID_REQUEST, stale→CONFLICT, declined→INVALID_STATEMUSTs inspecification.mdx/create_media_buy.mdxare seller-side runtime checks (correctly — JSON Schema only enforcesproposal_version→proposal_idco-presence). Worth confirming the conformance storyboards cover these rejections, since the schema gate can't.
Minor nits (non-blocking)
proposal_versionlength 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.N — minor is still the right declaration.
LGTM. Follow-ups noted below.
81aab72 to
24f9c4d
Compare
There was a problem hiding this comment.
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.jsonproposal entry.action: "decline"requiresproposal_version+reason; non-decline (including omitted action) forbidsreason/detail;reason: "other"requiresdetail. All three compose correctly — a valid decline passes blocks 2 and 3 vacuously. Confirmed againstcode-reviewer's case-by-case ajv v8 trace. opportunity-context.jsonclose 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. Testclose_reason requires closed statusexercises it.additionalProperties: falseis safe.proposal_version/reason/detailare declared in the variant's ownproperties; the newallOfif/then blocks add onlyrequired/not, no new property names — no additionalProperties strip footgun.- oneOf audit. The new
reasonfield isoneOfof pureconsts → classifiedscalarbyaudit-oneof.mjs:144and filtered from the baseline at L307/L322. No discriminator regression. The proposaloneOfstays discriminated onscope. - Error codes all pre-exist.
INVALID_STATE(broadened, bothenumDescriptionsandenumMetadata.suggestionupdated 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.mdxfiles. - Dependency
proposal_version → proposal_id(one-directional) — correct; unversionedproposal_idstill allowed.
Follow-ups (non-blocking — file as issues)
reasonmodeling breaksenumDescriptionsparity. It'stype: string+oneOfof{const, description}, which the build does not harvest intoenumDescriptions/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 sharedenums/*-reason.jsonwithenum+enumDescriptions(action-not-allowed-reason.json,makegood-remedy.json). Worth extractingenums/proposal-decline-reason.jsonand$refing it. Notable thatclose_reason/phase/statusin this same PR use plainenum—reasonis the one outlier.CONFLICTisrecovery: transient(concurrent-write). The PR overloads it for a staleproposal_version(optimistic-version mismatch). Defensible — re-read-and-retry is the transient contract — but extend theCONFLICTenumDescriptionsprose to name the stale-version case so SDKs don't treat it as a pure race.opportunity-context.jsonnot registered instatic/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)
refinement.mdxproposal-action table omits aproposal_versioncolumn.get_products.mdxlists it as a first-class conditional field;refinement.mdxfolds it into thedeclinerow's free text. Not a contradiction, just less scannable.- The central
proposal_versionMUST is runtime-only.specification.mdxandcreate_media_buy.mdxrequire sellers to reject a versioned-proposal execution that omitsproposal_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.
24f9c4d to
4b4edf4
Compare
There was a problem hiding this comment.
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
reasontaxonomy inget-products-request.jsonis aoneOfofconstbranches —scripts/audit-oneof.mjsclassifies all-const oneOf asscalar(variantInfo L111), so it's neither flagged nor baselined. No discriminator regression. - Decline gating fires correctly.
additionalProperties: falseon the proposal refine branch recognizes the newproposal_version/reason/detailbecause they're declared in that branch's ownproperties; theallOfonly addsrequired/not(required). Traced all four branches: decline requires version+reason (cond 1), non-decline forbids reason/detail even whenactionis omitted (cond 2,not-wrapped — no leak),reason: otherrequires detail (cond 3).code-reviewer: no blockers, manual semantic trace passes on every branch. - Close-won asymmetry is deliberate.
opportunity-context.jsonpermits all 7close_reasonvalues whenstatus: closed;create-media-buy-request.jsonnarrows toaccepted_with_selleronly on create. Loss reasons belong on aget_productsrefine close. Consistent with docs. - error-code parity holds.
INVALID_STATEupdated in both enumDescriptions (L885) and enumMetadata suggestion (L894),recovery: correctableunchanged. ReusingINVALID_STATEfor declined-then-executed is the right call — it already means "operation not permitted for terminal lifecycle state." - Changeset is right.
minor, purely additive:declineis 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) andclose_reason(7) match their schema enums exactly acrossget_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.yamlcovers acknowledge → idempotent duplicate → reject-create-after-decline withINVALID_STATE(L685-688), wired into bothsales-guaranteedandsales-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 includingproposal_version requires proposal_idandclosed opportunity rejects non-accepted close_reason.
Follow-ups (non-blocking — file as issues)
create-media-buy-response.jsondoesn't echoproposal_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 viaget_media_buys. Adding an optional echo staysminor. (ad-tech-protocol-expert.)CONFLICTrecovery class istransientbut a staleproposal_versionis correctable-by-re-read. This PR is the first to route a value-mismatch toCONFLICT. The docs route it correctly ("re-read or refine"), but an SDK trustingenumMetadata.CONFLICT.recovery = transientcould auto-retry the stale version and loop. Worth a one-line spec note that thisCONFLICTis correctable, not blindly retryable.- Response-side "MUST echo
proposal_versionon 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)
reasonusesoneOfof const; the parallelclose_reasonuses a plainenum. 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.
Adds optional
opportunitycontext acrossget_productsandcreate_media_buyso 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-scopedaction: "decline"toget_productsrefinement with requiredproposal_version, reason feedback, idempotent acknowledgement semantics, and privacy guidance. Carries proposal versions intocreate_media_buyso 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.