feat(media-buy): remove deprecated response status - #6570
Conversation
![aao-secretariat[bot]](https://avatars.githubusercontent.com/u/221690821?s=60&v=4){kind=small}
There was a problem hiding this comment.
Ladon verdict: Request changes
Request changes — 3 blocking finding(s)
This PR cleanly removes the deprecated top-level status: MediaBuyStatus from CreateMediaBuySuccess/UpdateMediaBuySuccess. The schema change itself is sound: the property is dropped, the provisional-buy not.anyOf guard collapses correctly, oneOf discrimination stays intact, the new test is valid, and the major changeset is the correct class for a breaking removal. However, the doc surfaces carry a version-narrative that no longer matches the wire change — a schema↔docs coherence defect (docs asserting a shape/version the schema/changeset does not encode).
Blocking findings
- docs/media-buy/task-reference/create_media_buy.mdx:1428 — Docs assert removal in "3.2" but a major changeset bumps 3.1.1 → 4.0.0
- docs/reference/migration/media-buy-status.mdx:75 — Migration reference doc still describes the removed field as a live
deprecated: truefield - docs/media-buy/task-reference/update_media_buy.mdx:648 — Same version-label drift on update_media_buy migration prose
Row 1 fires (three high findings). Fix the version narrative (major → 4.0) and update the migration reference docs to describe the field as removed rather than deprecated.
Note for merge: this is also a Breaking-class protocol change touching gated static/schemas/source/** with review_decision: REVIEW_REQUIRED, so even once the doc drift is resolved it requires explicit human/CODEOWNERS ratification before it can proceed — it is not auto-approvable.
Blocking findings
- docs/media-buy/task-reference/create_media_buy.mdx:1428 — Docs assert removal in "3.2" but breaking removal ships major (→ 4.0)
- docs/reference/migration/media-buy-status.mdx:75 — Migration reference doc still describes the removed field as live deprecation
- docs/media-buy/task-reference/update_media_buy.mdx:648 — Same version-label drift on update_media_buy migration prose
205b1b2 to
3d1f71a
Compare
There was a problem hiding this comment.
Ladon verdict: Request changes
Request changes — 4 blocking finding(s)
The schema removal of the deprecated body-level status field is clean and coherent, and the new schema-validation test covers it. However, a version-label contradiction persists across the entire changeset and remains unresolved from the prior review.
Blocking findings
.changeset/remove-deprecated-media-buy-status.md:5— Amajorchangeset against3.1.1in beta pre mode produces4.0.0, not3.2. The claim "protocol target remains 3.2" is mechanically false — spec/changeset drift.docs/reference/migration/media-buy-status.mdx:84— Migration doc prose entrenches the 3.2-vs-major contradiction rather than resolving it (new rebuttal text added this pass).docs/media-buy/task-reference/create_media_buy.mdx:1428— Still asserts the removal lands in AdCP 3.2; contradicts the major version bump.docs/media-buy/task-reference/update_media_buy.mdx:648— Same version-label drift on the update_media_buy migration prose.
Row 1 of the decision table fires: high-severity findings are present, so this is request-changes. The version-label mismatch between the changeset (which resolves to 4.0.0) and the docs (asserting 3.2) is a customer-facing spec-coherence break and must be reconciled before merge. Note also gated_paths: true with review_decision: CHANGES_REQUESTED — the hard schema-source approval gate is unsatisfied, so this could not have approved regardless.
Blocking findings
- ‘.changeset/remove-deprecated-media-buy-status.md:5’ — major changeset resolves to 4.0.0 not 3.2; “protocol target remains 3.2” is false
- docs/reference/migration/media-buy-status.mdx:84 — migration prose entrenches the 3.2-vs-major contradiction
- docs/media-buy/task-reference/create_media_buy.mdx:1428 — still asserts removal lands in AdCP 3.2
- docs/media-buy/task-reference/update_media_buy.mdx:648 — same 3.2 version-label drift on update_media_buy prose
3d1f71a to
c532500
Compare
There was a problem hiding this comment.
Ladon verdict: Request changes
Request changes — 1 blocking finding.
Blocking findings
- .changeset/remove-deprecated-media-buy-status.md:2 — Minor changeset ships a breaking wire change: removal of the top-level
statusfield published across the released 3.1.x line. Per repo policy, a breaking wire change (removed field) must bemajor, notminor. This ishigh.
Context / iteration credit
This is a subsequent review. The prior pass's four high findings — the changeset/docs contradiction over the 3.2-vs-major version target — have all been resolved. Schema↔docs coherence now holds consistently across the schema, migration doc, both task-reference docs, and the compliance index, and Test 11 validates the schema-level removal. The oneOf disjointness also improves (the ambiguous status is removed from the success branch). That is real progress.
However, the fix over-corrected: flipping the changeset to minor mis-types a breaking removal. Removing a top-level field that shipped across 3.1.x breaks buyer/seller agents pinned to that shape, and breaking wire changes take major. The changeset type must be corrected before merge.
Row 1 of the decision table fires on the high finding → request-changes. (Note: this PR also touches gated static/schemas/source/** paths with review_decision: CHANGES_REQUESTED, and constitutes a Breaking-class protocol change — both of which independently require human/CODEOWNERS ratification and preclude auto-approval regardless.)
Blocking findings
- [
- "
- \
- .
- c
- h
- a
- n
- g
- e
- s
- e
- t
- /
- r
- e
- m
- o
- v
- e
-
- d
- e
- p
- r
- e
- c
- a
- t
- e
- d
-
- m
- e
- d
- i
- a
-
- b
- u
- y
-
- s
- t
- a
- t
- u
- s
- .
- m
- d
- :
- 2
- —
- m
- i
- n
- o
- r
- c
- h
- a
- n
- g
- e
- s
- e
- t
- s
- h
- i
- p
- s
- b
- r
- e
- a
- k
- i
- n
- g
- r
- e
- m
- o
- v
- a
- l
- o
- f
- p
- u
- b
- l
- i
- s
- h
- e
- d
- t
- o
- p
-
- l
- e
- v
- e
- l
- `
- s
- t
- a
- t
- u
- s
- `
- f
- i
- e
- l
- d
- ;
- b
- r
- e
- a
- k
- i
- n
- g
- w
- i
- r
- e
- c
- h
- a
- n
- g
- e
- s
- r
- e
- q
- u
- i
- r
- e
- a
- m
- a
- j
- o
- r
- c
- h
- a
- n
- g
- e
- s
- e
- t
- "
- ]
|
The release-mechanics contradiction is resolved: the current minor changeset calculates AdCP 3.2.0-beta.0, and all CI checks pass. The remaining Ladon objection is the underlying policy exception, not an implementation defect. The two mechanically possible outcomes are:
I have kept the branch aligned with #4906 and requested human schema review for the deliberate same-major deprecation exception. The separate nested-status cascade remains reserved for 4.0 in #4905. |
c532500 to
7553634
Compare
There was a problem hiding this comment.
Ladon verdict: Escalate to human review
Escalate — breaking-class protocol change on gated paths without human approval.
This PR removes body-level lifecycle status from create/update media-buy success payloads — a breaking wire-shape change to the published protocol surface under static/schemas/source/**. The reviewer found no code-correctness, schema-drift, or docs-coherence defects in the delta (findings list is empty), and the prior changeset finding is now backed by the self-introduced DR-0011 breaking-class exception. So there are no blocking reviewer findings.
However, two hard gates independently require escalation:
-
Gated paths (decision row 2):
gated_paths: true(threestatic/schemas/source/media-buy/*.jsonfiles modified) ANDreview_decision: CHANGES_REQUESTED(≠ APPROVED). The hard approval gate is not satisfied. -
Breaking-class governance gate: This is a removed published field — a Breaking-class change per the constitution. Breaking-class changes are never auto-approved; ratification is a human act. Escalate unless
review_decisionis APPROVED, which it is not.
Escalation reasons:
- Modifies gated schema files under
static/schemas/source/**whilereview_decisionis CHANGES_REQUESTED — human/CODEOWNERS approval required. - Breaking-class removal of published
statusfield from create/update success payloads — human ratification required (DR-0011 self-introduced; needs a human on the thread).
The delta itself is clean and the developer resolved the prior changeset concern via DR-0011, but the breaking removal and unsatisfied gate require explicit human review.
Why human review
- Gated paths: static/schemas/source/media-buy/create-media-buy-response.json, update-media-buy-response.json, and create-media-buy-async-response-submitted.json are under a hard approval gate and review_decision is CHANGES_REQUESTED (not APPROVED) — human/CODEOWNERS approval required.
- Breaking-class protocol change: removal of published body-level
statusfield from create/update media-buy success payloads. Breaking-class changes are never auto-approved; human ratification is required (DR-0011 is a self-introduced exception needing a human sign-off). - This PR touches a path under a hard, non-overridable approval gate (static/schemas/source/media-buy/create-media-buy-async-response-submitted.json (modified) matches
static/schemas/source/**; static/schemas/source/media-buy/create-media-buy-response.json (modified) matchesstatic/schemas/source/**; static/schemas/source/media-buy/update-media-buy-response.json (modified) matchesstatic/schemas/source/**) and the current GitHub review decision is 'CHANGES_REQUESTED', not APPROVED. This is a hard gate enforced in code — Ladon cannot auto-approve until a human/CODEOWNERS approval is recorded, regardless of how clean the diff is.
aao-secretariat
Bot
added
the
ladon/needs-human-review
|
Ladon cannot review this PR until merge conflicts are resolved. |
c766741 to
9442965
Compare
There was a problem hiding this comment.
Ladon verdict: Escalate to human review
Escalate — hard approval gate unsatisfied + breaking-class protocol change requires human ratification.
This PR removes the deprecated top-level status (MediaBuyStatus) field from CreateMediaBuySuccess and UpdateMediaBuySuccess for AdCP 3.2, ratified by new DR-0011. The reviewer found no code-correctness issues: schema, docs (create/update task refs, migration guide), compliance storyboard, and DR-0011 all move in lockstep with no drift; the removal is well-formed (protocol-envelope constrains root status to TaskStatus, asserted by new Test 11) and the provisional-buy not.anyOf constraint collapses cleanly. Clean diff, no findings.
However, two hard gates apply and neither can be waived by a clean diff:
Escalation reasons
- Gated paths: static/schemas/source/media-buy/create-media-buy-response.json, update-media-buy-response.json, and create-media-buy-async-response-submitted.json are under a hard, non-overridable approval gate (
static/schemas/source/**) andreview_decisionis REVIEW_REQUIRED — not APPROVED. Human/CODEOWNERS approval is required before merge (decision-table row 2). - Breaking-class protocol change: removal of a published body-level field from create/update media-buy success payloads. Per the constitution's Breaking-class hard gate, ratification is a human act and is never auto-approved unless
review_decisionis APPROVED — even with a correct changeset and a self-introduced exception record (DR-0011). This is a prose-based hard gate that stands becausereview_decisionis not APPROVED.
Prior decision was escalate for the same reasons; both gate conditions still hold on this head. No blocking (critical/high) findings. Requires explicit human/CODEOWNERS sign-off to proceed.
Why human review
- Gated paths under hard approval gate (static/schemas/source/**): create-media-buy-response.json, update-media-buy-response.json, create-media-buy-async-response-submitted.json — review_decision is REVIEW_REQUIRED, not APPROVED. Human/CODEOWNERS approval required.
- Breaking-class protocol change: removal of the published body-level
status(MediaBuyStatus) field from create/update media-buy success payloads. Breaking-class changes are never auto-approved; human ratification required (DR-0011 is a self-introduced exception needing human sign-off), and review_decision is not APPROVED. - This PR touches a path under a hard, non-overridable approval gate (static/schemas/source/media-buy/create-media-buy-async-response-submitted.json (modified) matches
static/schemas/source/**; static/schemas/source/media-buy/create-media-buy-response.json (modified) matchesstatic/schemas/source/**; static/schemas/source/media-buy/update-media-buy-response.json (modified) matchesstatic/schemas/source/**) and the current GitHub review decision is 'REVIEW_REQUIRED', not APPROVED. This is a hard gate enforced in code — Ladon cannot auto-approve until a human/CODEOWNERS approval is recorded, regardless of how clean the diff is.
|
Maintainer direction confirmed during the 3.2 backlog sweep: proceed with the scoped create/update response removal in 3.2. The 3.1 certification gate already requires @ohalushchak-exadel this is ready for schema approval: all 46 checks are green, Ladon reports no correctness findings, and there are no unresolved review threads. Auto-merge is armed. |
1 participant
Summary
statusproperty from create/update success branchesstatusexclusively as the task-envelope discriminatorValidation
Closes #4906