feat(media-buy): add opportunity and proposal decline flow#5523
feat(media-buy): add opportunity and proposal decline flow#5523bokelley wants to merge 4 commits 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.
4b4edf4 to
12cea6d
Compare
There was a problem hiding this comment.
Clean, additive feature. The architecture is right: three distinct identifiers — idempotency_key (one mutating attempt), proposal_id + proposal_version (one offer state), opportunity_id (the planning cycle) — kept orthogonal instead of overloading one field to do three jobs.
ad-tech-protocol-expert: sound-with-caveats. code-reviewer: sound-with-caveats. No MUST-FIX in the diff.
Things I checked
- Semver is
minor, and that's correct. Every wire change is additive:openadded to thebuying_moderequest enum,declinetorefine[].action,opento thebuying_modesitems enum inget-adcp-capabilities-response.json:184, and the optionalproposal_version/opportunityfields. Adding an enum member only breaks a consumer doing closed-world validation that rejects unknowns — a posture this protocol explicitly forbids. Nomajorneeded. - oneOf gate passes without a baseline change.
get-products-request.jsonrefine[].reasonis 10 bare{const, description}branches.scripts/audit-oneof.mjsclassifies const-only variants asscalarand excludes them fromdiff()/writeBaseline(), so this never lands in the baseline.scripts/oneof-discriminators.baseline.jsoncorrectly stays untouched — the standard enum-with-descriptions idiom, same ascore/start-timing.json. - The decline conditional has no escape hatch.
get-products-request.jsonper-entryallOf: the not-decline branch isif not({action: const decline}) then not(reason or detail). Draft-07 does not applydefault: "include"before evaluatingif, so an absentactionis treated as not-decline andreason/detailare correctly forbidden. The decline branch requiresproposal_version+reason;reason: "other"requiresdetail. Three blocks compose without conflict. opportunity-context.json:965close-reason guard is correct.if not(status == closed) then not(close_reason or close_detail)— closure metadata is forbidden unless the opportunity is actually closed.additionalProperties: false,required: [opportunity_id], and the^[A-Za-z0-9_.:-]{1,255}$pattern are consistent with the docs table.create-media-buy-request.jsonclose-won rule is enforced on the wire. The top-levelallOfif opportunity.status == closed then close_reason ∈ [accepted_with_seller]uses an inline subschema inthen, so it does not re-imposeadditionalProperties: false, and a closed opportunity with noclose_reasonstays valid.dependencies: { proposal_version: [proposal_id] }correctly forcesproposal_id.proposal_decline.yaml:844closes the loop. The compliance scenario assertsINVALID_STATEoncreate_media_buyagainst a declinedproposal_id+proposal_version, plus duplicate-decline idempotency and therefinement_appliedecho — the cross-request state that JSON Schema can't model.example-validation-simple.test.cjs:1676and neighbors assert on the right substrings:expectInvalidmatches against message +JSON.stringify(params), so['accepted_with_seller'],['proposal_version'],['detail']land on enumallowedValues/missingProperty. No false-green.- error-code parity holds.
INVALID_STATEupdated in both the enum description (error-code.json:138) and theenumMetadata.suggestion(error-code.json:341). No new error code —CONFLICT,PROPOSAL_NOT_FOUND,INVALID_REQUESTall pre-exist. - Schema-vs-docs: no drift. Action enums, the four buying modes, the opportunity field table, and the
proposal_version"required when versioned" rule all match acrossget_products.mdx,create_media_buy.mdx,specification.mdx,refinement.mdx, and the source schemas.
Ship condition (blocking on the queue, not on the diff)
- Rebase the branch before merge. CI shows
TypeScript Build,Server integration tests, andNo duplicate migration numbersred. This diff contains zero migration files, zero.ts, zeroserver/changes — it cannot introduce a duplicate migration number. PR #5587 merged into main at 21:16 with all three green; this branch ran at 21:23 against a stale base. The failures are inherited, not yours. Merge is alreadyBLOCKEDuntil checks pass, so this can't land red — but rebase on current main to clear them rather than waiting.
Follow-ups (non-blocking — file as issues)
- The response-side
proposal_versionecho onrefinement_appliedis a prose MUST that JSON Schema can't enforce (it conditions a response field on a separate request payload). Conformance rests entirely onproposal_decline.yaml— confirm that scenario keeps asserting the echo as the spec evolves. - The "reject omitted
proposal_versionwhen the proposal is versioned →INVALID_REQUEST" rule is likewise seller-behavior-only; the request alone can't reveal whether the referenced proposal carried a version. Consistent with howPROPOSAL_NOT_COMMITTEDis already handled, but it's untestable at the schema layer.
Approving on the strength of two independent sound verdicts plus a clean schema/docs/compliance coherence pass. Ship it once the branch is rebased and CI is green.
12cea6d to
96b50a6
Compare
There was a problem hiding this comment.
Additive schema change, done right. New optional fields, two new enum values, one new optional core/ object — nothing flips required↔optional, nothing is removed or renamed, so minor is the correct changeset and the if/then blocks only constrain fields this PR introduces.
Things I checked
- Decline conditional logic (
get-products-request.json, refine proposalallOf). Three composed if/then members:action: "decline"⇒ requireproposal_version+reason;not(decline)⇒ forbidreason/detail;reason: "other"⇒ requiredetail. Thenot{properties+required}idiom correctly folds "action absent" and "action present non-decline" into the prohibited-reason case. Matches the prose inrefinement.mdxandspecification.mdx. - The
reasononeOf does not trip the discriminator gate. Every branch is{const, description}—scripts/audit-oneof.mjsclassifies that asscalar(const !== undefined), andwriteBaseline/diffskip scalar rows.get-products-request.jsonhas zero baseline entries; the existingrefine.items.oneOfalready classifies asdiscriminatedonscope. No baseline ratchet needed. This is the established enum-with-descriptions idiom, not a new undiscriminated oneOf. create-media-buy-request.json: closed-opportunity ⇒close_reason ∈ {accepted_with_seller}constrains the value only when present (doesn't force it to exist) — matches "the only valid close_reason is accepted_with_seller."dependencies.proposal_version: [proposal_id]enforces the version⇒id requirement.opportunity-context.json:60-92:close_reason/close_detailforbidden unlessstatus: "closed"viaif:not{status==closed} then:not{anyOf required}. Correct.- Test honesty on the one suspicious assertion.
'open mode rejects wholesale conditional version'expects a'wholesale'error string — that fires off the pre-existingif_wholesale_feed_version⇒buying_mode: "wholesale"rule, where AJV emits{"allowedValue":"wholesale"}. The rule exists; the assertion is real, not a tautology. - INVALID_STATE reuse is sound. A declined proposal version is a terminal lifecycle state — parallel to "updating a canceled media buy." No new error code warranted.
CONFLICT(stale version) andPROPOSAL_NOT_FOUND(unresolvable) reuses are consistent with the existing refinement error table. - Migration renames + CI gate.
517_*→519/520_*resolves the pre-existing517collision onmain; the newprefix_dupes()logic incheck-migration-numbers.ymlsuppresses the main-only517dupe so the fixing PR can merge, while still catching a PR that adds a fresh collision vialocal_dupes. Right shape. - Changeset present (
.changeset/proposal-decline-refinement.md,minor). Docs match schema acrossget_products.mdx,create_media_buy.mdx,refinement.mdx,specification.mdx.ad-tech-protocol-expert: sound-with-caveats.code-reviewer: sound-with-caveats.
Caveat on validation
Neither I nor the subagents could execute the suites — this sandbox gates git fetch/npm/node. The conclusions above are static traces of the schemas, tests, and audit classifier against the on-disk main tree, plus the PR's own claim that build:schemas, test:schemas, test:examples, test:json-schema, and test:oneof-discriminators were run. CI is the empirical gate. The newly schema-enforced brief-required / refine-forbidden / brief-forbidden rules mean any legacy get-products-request example that violated them only in prose would now fail test:examples — my scan of $schema-tagged doc blocks and the request-validating test files found none, but I did not run them. Watch test:json-schema and test:examples go green before merge.
Follow-ups (non-blocking — file as issues)
- The privacy guidance on
opportunity_id,close_detail, and declinedetail("MUST NOT include competitor identity / clearing prices / campaign IDs") is enforced only bymaxLength— the substance is out-of-band. Consider a$commentnoting enforcement is seller-side, so nobody mistakes the prose for a schema guarantee. - "MUST NOT decline an unversioned proposal" and "reject create after decline with INVALID_STATE" are stateful seller rules the schema can't express. Correctly pushed to
proposal_decline.yamlcompliance coverage — just noting the schema alone doesn't guard them.
Minor nits (non-blocking)
- Stale comment in the migration workflow.
.github/workflows/check-migration-numbers.yml:47-50still describes the old "union, hunt dupes in that union" approach; the new logic suppresses main-only dupes. Update the comment to match. - Decline scenario read steps.
proposal_decline.yamlomitsidempotency_keyon the twoget_productssteps — correct because the reads are non-mutating, but confirm the scenario runner doesn't require it forstateful: trueread steps.
The migration-readiness docs had been pointing sellers at a browse buying mode that never shipped; this PR quietly retires it for the real brief/wholesale/refine/open set. Good cleanup to fold in.
Approving on the strength of the additive-only schema delta plus two independent sound-with-caveats verdicts. Safe to merge once CI is green.
There was a problem hiding this comment.
One unregistered x-entity value breaks npm test. Everything else is clean, additive, and correctly versioned — fix the registry and this ships.
MUST FIX (blocking)
x-entity: "proposal" is not registered — fails test:storyboard-context-entity. create-media-buy-response.json adds the proposal_id echo field with "x-entity": "proposal" (the success-result block, ~L29). But proposal is absent from the enum in static/schemas/source/core/x-entity-types.json — this PR registers only opportunity (L13/L52). lint() in scripts/lint-storyboard-context-entity.cjs:616 walks [...lintRegistry(registry)] over every source schema and emits unknown_entity for any x-entity not in the registry (L336). tests/lint-storyboard-context-entity.test.cjs:35 asserts deepEqual(violations, []), and that test is wired into npm test (test:storyboard-context-entity). So the suite goes red on merge.
What breaks for adopters: the PR cannot land green — CI fails on the context-entity gate.
Fix is two lines: add \"proposal\" to the enum array and a matching x-entity-definitions entry in x-entity-types.json. Worth a glance at the request side too — create-media-buy-request.json's proposal_id carries no x-entity while the response echo now does; register proposal and the asymmetry at least resolves to a registered type.
Your validation list (build:schemas, test:schemas, test:examples, test:json-schema, test:oneof-discriminators) didn't include test:storyboard-context-entity — that's the gap that let this through. Run the full npm test before re-pushing.
Things I checked
- oneOf on
refine[].reason— scalarconstvariants, classifiedkind: scalar(STATUS_RANK 0) byaudit-oneof.mjs:111. Not baselined, not flagged, no--accept-new. The enum-with-descriptions idiom, not an undiscriminated-union regression. openis additive —[brief, wholesale, refine]→+openin bothget-products-request.jsonbuying_mode andget-adcp-capabilities-response.jsonbuying_modes. No enum values removed.actionlikewise+decline. No wire break.proposal_versioncross-schema parity —string / minLength 1 / maxLength 255identical across proposal.json, get-products-request refine entries, get-products-response refinement_applied, create-media-buy req + resp. Optional at property level; conditionally required via if/then (decline) anddependencies(proposal_version → proposal_id).- Schema conditionals —
get-products-request.jsonbrief/refine if/then blocks partition the enum with no overlap, andbuying_modebeing in top-levelrequireddefuses the draft-07 vacuous-iftrap.opportunity-context.jsonallOf[0]if: { not: { ... status const closed } }correctly reads as "status absent or ≠ closed." Both sound. - Changeset —
minoris right. New optional fields, two additively-extended enums, one new optional schema. The request-validation tightening (brief-required / refine-required, previously prose-only) only rejects payloads the docs already declared invalid — no conforming client relied on them.minorholds. - error-code.json parity — INVALID_STATE broadened in both
enumDescriptions(L138) and theenumMetadatasuggestion (L341). No new code minted; INVALID_STATE / CONFLICT / PROPOSAL_NOT_FOUND / INVALID_REQUEST all pre-exist. - Schema-vs-docs — get_products.mdx, create_media_buy.mdx, specification.mdx, refinement.mdx track the schema: field names, required/optional, the
reasonandclose_reasonenum lists, and the create→error mapping all match. - check-migration-numbers.yml —
prefix_dupes()+ union/main/local dedup fails on local dupes, skips pre-existing main-only dupes, and catches stale-merge collisions. The 519/520 renames produce a clean contiguous sequence.
Minor nits (non-blocking)
- Thin negative assertions. Several
expectInvalidcases intests/example-validation-simple.test.cjsassert only['must NOT be valid'](open-rejects-brief, omit-rejects-reason, create-closed-rejects-non-accepted). Asserting the offending field/keyword instead would stop an unrelated schema change from making them pass for the wrong reason.
Fix the registry, run npm test, and I'll approve.
There was a problem hiding this comment.
Clean, fully-additive extension to the media-buy flow. Right shape: opportunity is advisory buyer-reported context fenced off from idempotency, pricing, and buyability, while proposal_version does the load-bearing work of pinning the exact offer state a buyer accepts or declines.
code-reviewer, ad-tech-protocol-expert (sound-with-caveats), and security-reviewer (no High/Medium) all came back without a blocker.
Things I checked
- Changeset type is correct.
minorholds: every wire change is additive — new optional fields (proposal_version,opportunity, declinereason/detail), enum values appended to request/response/capability enums (openonbuying_mode+ capabilitiesbuying_modes,declineon proposalaction), and reused error codes. No removed/renamed fields, no required↔optional flips, no removed enum values.proposal_versionstays globally optional onproposal.jsonand is only conditionally required viadependencies.proposal_version: [proposal_id]plus the per-callif/then— the legacy unversioned-proposal path is preserved. - The conditional logic is right.
opportunity-context.json:74-105—if: not(status present AND status===closed) → then: not(required close_reason OR close_detail)correctly gates the close fields tostatus: "closed"only, andadditionalProperties: falsecoexists cleanly since both are declared properties.create-media-buy-request.json:191-223— thethenonly constrainsopportunity.close_reasonto["accepted_with_seller"], it does not require it, so a closed-on-create with no reason still validates; no contradiction with the context schema's own conditional (strict intersection, not conflict). openmode is fenced everywhere it needs to be.get-products-request.jsonforbidsbrief(brief-forbidden conditional includesopen) andrefine(the refine conditional'selsefires for any non-refine mode), andget-products-async-response-submitted.jsonexcludesopenfrom the Submitted arm — open is a synchronous state read, matchingasync-operations.mdx.- The
reasononeOf is the accepted enum-with-descriptions idiom, not a discriminator regression. Every variant is{const, description};audit-oneof.mjsclassifies all-constvariants as scalar and excludes them from the baseline and diff, so--checkdoes not regress. - error-code parity.
INVALID_STATEupdated in bothenumDescriptionsandenumMetadata.suggestion,recovery: "correctable"unchanged and still matching the prose. No new enum values —CONFLICT/PROPOSAL_NOT_FOUND/INVALID_REQUESTare reused. - x-entity registration is complete. Both
opportunityandproposalregistered inx-entity-types.json,proposal_id → proposaladded toscripts/x-entity-field-map.json, the doc table updated, and annotations placed on the request/response/proposal id sites. The PATCH-4 "register proposal entity annotation" commit is exactly the internal-consistency cleanup that would otherwise have shipped a half-wired entity. - Migration renames are safe.
maincurrently carries a triple collision at 517; this PR moves two to free 519/520 (highest prior was 518), and both renamed files are idempotent (ON CONFLICT DO UPDATE,IF NOT EXISTS, guardedADD CONSTRAINT). The newcheck-migration-numbers.ymlmain-only-duplicate reconciliation is what lets this fixing PR pass its own gate. - Compliance coverage is real.
proposal_decline.yamlexercises the full loop — versioned-proposal brief, decline-with-echo, idempotent duplicate decline, and theINVALID_STATEcreate-reject at the declined version — and the new negative tests inexample-validation-simple.test.cjscover open-mode-rejects-brief/refine and decline-required-fields.
Follow-ups (non-blocking — file as issues)
- Stale error row in
refinement.mdx. TheINVALID_REQUESTrow still reads "refine provided inbrieforwholesalemode" — not updated to addopen, even though the same PR addsopento the forbidden-refineset inspecification.mdxandget_products.mdxand the schema enforces it. Doc-only drift; tighten when convenient. - Receiver-side untrusted-input framing (
security-reviewerL1). The privacy prose onopportunity_id/detail/close_detailis thorough but entirely buyer-obligation-facing. For an agent-to-agent protocol where these free-text fields are explicitly routed into "CRM labels," "human-review prioritization," and "win-rate analytics," one normative sentence telling sellers to treat them as untrusted before placing them in LLM context would close the prompt-injection gap. Spec text, not code — hence a follow-up. - CONFLICT is slightly off-label. Its canonical recovery is
transient(retry as-is), but stale-proposal_versionrecovery is correctable (re-read for a new version). Defensible as the established re-read-then-retry model; a one-line note in the CONFLICT description would remove the ambiguity.
Minor nits (non-blocking)
opportunity_idper-seller scoping isSHOULD, and every example reuses the same literal.opp_01HX7W8S3Q9K2M4N6P0R5T1Vappears identically across the doc examples andproposal_decline.yaml, modeling the cross-seller-reuse anti-pattern the schema warns against. Vary the example IDs so the spec doesn't demonstrate the thing it tells buyers not to do.for prefix in $union_dupesis unquoted (check-migration-numbers.yml). Safe for numeric prefixes fromsed 's/_.*//', butgrep -Fqxover a quoted iterator would harden the dup verdict against an oddly-named.sqlfile. CI hygiene, not a bug.
Approving on the strength of the additive-only wire surface plus the in-schema enforcement of every new invariant. Follow-ups noted above.
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.