Skip to content

fix(schema): unify delivery_status enum across delivery report, webhook, and snapshot - #6104

Open
andybevan-scope3 wants to merge 3 commits into
adcontextprotocol:mainfrom
andybevan-scope3:create-github-issue-and-pr
Open

fix(schema): unify delivery_status enum across delivery report, webhook, and snapshot#6104
andybevan-scope3 wants to merge 3 commits into
adcontextprotocol:mainfrom
andybevan-scope3:create-github-issue-and-pr

Conversation

@andybevan-scope3

Copy link
Copy Markdown
Collaborator

Why

delivery_status was inlined independently in three schemas and had drifted: get-media-buys-response.json (the get_media_buys snapshot path) had 6 values including not_delivering, while get-media-buy-delivery-response.json and media-buy-delivery-webhook-result.json only had 5. A buyer polling get_media_buy_delivery or receiving a delivery webhook had no way to observe not_delivering, even though "zero delivery during flight" is exactly what those paths exist to surface.

Fixes #6103.

What Changed

  • Extracted static/schemas/source/enums/delivery-status.json (6 values, with enumDescriptions), following the existing enums/pricing-model.json extraction pattern.
  • Replaced the three independent inline enums with $ref to the shared enum, preserving each schema's existing per-field description override where one existed.
  • Added a minor changeset (new enum value permitted on two existing paths).
  • Updated docs/media-buy/task-reference/get_media_buy_delivery.mdx, which still listed only the old 5-value set — found during review since it's the doc for the exact endpoint this PR changes.

Test plan

  • npm run build:schemas — bundled output resolves the $ref to all 6 values on all three schemas
  • npm run test:schemas, test:json-schema, test:extension-schemas, test:build-schemas-hoist-enums — all pass
  • npm run typecheck, npm run lint:schema-links — clean
  • Changeset scope/status gates (check-changeset-protocol-scope.cjs, changesets status --since=origin/main) — pass
  • npm run test:docs-nav — pass

andybevan-scope3 and others added 2 commits July 30, 2026 11:20
…ok, and snapshot

get-media-buys-response.json, get-media-buy-delivery-response.json, and
media-buy-delivery-webhook-result.json each inlined their own copy of
delivery_status, and drifted: the newer get_media_buys snapshot path had
not_delivering, the older delivery-report and webhook paths didn't. Extract
enums/delivery-status.json (6 values) and $ref it from all three so a buyer
can observe not_delivering regardless of which path they poll.

Fixes adcontextprotocol#6103.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The delivery_status list in this doc predated the schema fix that added
not_delivering to get-media-buy-delivery-response.json. get_media_buys.mdx
already listed all 6 values; this brings the delivery-report doc in line.

@aao-secretariat aao-secretariat 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.

Ladon verdict: Escalate to human review

Escalate — gated schema paths require human/CODEOWNERS review.

This PR unifies delivery_status by extracting static/schemas/source/enums/delivery-status.json (6 values with enumDescriptions) and $ref-ing it from the get-media-buys, get-media-buy-delivery, and webhook-result schemas. The reviewer confirms it correctly closes real drift (#6103): the delivery-report and webhook paths previously inlined only 5 values, silently dropping not_delivering — the zero-delivery-during-flight signal those paths exist to surface. Extraction follows the enums/pacing.json convention, the changeset is a correct minor for an additive Normative enum value, schema↔docs coherence holds across all four sites, and no dist artifact, oneOf baseline, or index.json inline copy is touched.

The diff is mechanically clean and carries only one low finding (not_delivering timing MUST documented only on the get_media_buys field, not the two paths now gaining the value). There are no critical/high/medium findings.

However, this PR touches static/schemas/source/**, which the repo declares under a hard, deterministic approval gate (gated_paths: true), and review_decision is REVIEW_REQUIRED — not APPROVED. Per decision-table row 2, the gate is not satisfied and the outcome must be escalate. A human/CODEOWNERS approval will lift the gate and allow this clean, additive change to proceed.

Why human review

  • Gated protocol-schema paths modified without required review: static/schemas/source/enums/delivery-status.json (added), get-media-buy-delivery-response.json, get-media-buys-response.json, media-buy-delivery-webhook-result.json (modified) — review_decision is REVIEW_REQUIRED, not APPROVED; human/CODEOWNERS approval required.
  • This PR touches a path under a hard, non-overridable approval gate (static/schemas/source/enums/delivery-status.json (added) matches static/schemas/source/**; static/schemas/source/media-buy/get-media-buy-delivery-response.json (modified) matches static/schemas/source/**; static/schemas/source/media-buy/get-media-buys-response.json (modified) matches static/schemas/source/**; static/schemas/source/media-buy/media-buy-delivery-webhook-result.json (modified) matches static/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.

@aao-secretariat aao-secretariat Bot added the ladon/needs-human-review Ladon has escalated this PR for human review. label Jul 30, 2026
@aao-secretariat
aao-secretariat Bot requested a review from bokelley July 30, 2026 15:36
],
"enumDescriptions": {
"delivering": "Package is actively delivering impressions within its scheduled flight",
"not_delivering": "Package is within its scheduled flight but has delivered zero impressions for at least one full staleness cycle",

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.

the staleness guard from get-media-buys-response.json is missing here — suggest appending something like ", though implementers must not emit this value until at least staleness_seconds have elapsed since package activation". the delivery report and webhook schemas have no staleness_seconds field of their own, so this enum description is the only place consumers on those paths will see the constraint.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Good catch — the guard was genuinely missing on these two paths. Didn't copy this exact sentence in though: staleness_seconds doesn't exist on get-media-buy-delivery-response.json or the webhook schema (both are windowed reports with reporting_period, not live polls like the get_media_buys snapshot), so a rule referencing that field would dangle here. Used is_final instead — a mechanism both schemas already have for exactly this "is this number settled yet" question. Added to both field-level descriptions in 175e04e: "Sellers SHOULD only report 'not_delivering' once the package's data is_final for the period — a provisional (is_final: false) zero may still be measurement catching up, not genuine non-delivery."

get-media-buy-delivery-response.json and media-buy-delivery-webhook-result.json
are windowed reports (reporting_period), not live polls, so they have no
staleness_seconds to anchor a "how long before this counts" guard the way the
get_media_buys snapshot does. Point implementers at the mechanism these two
schemas already have instead: is_final. A provisional (is_final: false) zero
may still be measurement catching up, not genuine non-delivery.

Addresses review feedback on adcontextprotocol#6104.
"goal_met"
]
"$ref": "/schemas/enums/delivery-status.json",
"description": "System-reported operational state of this package. Reflects actual delivery state independent of buyer pause control. 'not_delivering' means zero impressions were recorded for the entire reporting_period while the package was in-flight. Sellers SHOULD only report 'not_delivering' once the package's data is_final for the period — a provisional (is_final: false) zero may still be measurement catching up, not genuine non-delivery."

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.

Medium: This delta grounds not_delivering in "zero impressions for the entire reporting_period ... once the package's data is_final" — but the docs page for this same task (docs/media-buy/task-reference/get_media_buy_delivery.mdx:666) defines it as "zero impressions for at least one full staleness cycle." Two different gating rules for the same signal on the same task. The staleness-cycle wording is inherited from the get_media_buys snapshot path, which has a staleness_seconds field; this response shape does not — it has reporting_period and is_final, which is exactly what your schema description now (correctly) keys off. The schema reads right; the docs entry describes machinery this task lacks. Align the docs to reporting_period/is_final so an implementer gating buyer alerts / price adjustments off not_delivering reads one definition, not two.

@aao-secretariat aao-secretariat 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.

Ladon verdict: Escalate to human review

Escalate — gated protocol-schema paths modified without required review.

This PR touches static/schemas/source/** (gated + high-risk paths) and review_decision is not APPROVED (unknown/none). Per the hard gated-paths rule (row 2), auto-approval is not possible until a human/CODEOWNERS approval is recorded.

Escalation reasons

  • Gated schema paths modified/added without required approval: static/schemas/source/enums/delivery-status.json (added), get-media-buy-delivery-response.json, get-media-buys-response.json, media-buy-delivery-webhook-result.json (modified). Human/CODEOWNERS review required — review_decision is not APPROVED.

Medium findings (surfaced for the human reviewer)

  • static/schemas/source/media-buy/get-media-buy-delivery-response.json:316not_delivering definition in schema diverges from its own task docs. The schema keys the signal off reporting_period/is_final, while the docs page defines it via a staleness_seconds "staleness cycle" (a field this task lacks). Schema↔docs coherence gap worth reconciling before merge.

The prior decision was also escalate for the same unsatisfied gate; a fresh medium coherence finding is present this run, so the escalation stands. No critical/high findings.

Medium findings

  • static/schemas/source/media-buy/get-media-buy-delivery-response.json:316 — not_delivering definition in schema diverges from its own task docs

Why human review

  • Gated protocol-schema paths modified/added without required review: static/schemas/source/enums/delivery-status.json (added), get-media-buy-delivery-response.json, get-media-buys-response.json, media-buy-delivery-webhook-result.json (modified) — review_decision is not APPROVED; human/CODEOWNERS approval required.
  • Schema↔docs coherence gap: not_delivering gating rule differs between get-media-buy-delivery-response.json:316 (reporting_period/is_final) and its task docs (staleness_seconds staleness cycle) — reviewer should reconcile.
  • This PR touches a path under a hard, non-overridable approval gate (static/schemas/source/enums/delivery-status.json (added) matches static/schemas/source/**; static/schemas/source/media-buy/get-media-buy-delivery-response.json (modified) matches static/schemas/source/**; static/schemas/source/media-buy/get-media-buys-response.json (modified) matches static/schemas/source/**; static/schemas/source/media-buy/media-buy-delivery-webhook-result.json (modified) matches static/schemas/source/**) and the current GitHub review decision is 'unknown', 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ladon/needs-human-review Ladon has escalated this PR for human review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

delivery_status enum inconsistent across get-media-buys-response, get-media-buy-delivery-response, and webhook schemas

2 participants