Skip to content

[codex] Add resource-scoped insights and warnings - #6212

Open
bokelley wants to merge 2 commits into
mainfrom
align-warnings-with-insights
Open

[codex] Add resource-scoped insights and warnings#6212
bokelley wants to merge 2 commits into
mainfrom
align-warnings-with-insights

Conversation

@bokelley

@bokelley bokelley commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds a joined AdCP 3.2 contract for structured success warnings and durable resource-scoped insights on media buys, packages, and creative assignments without introducing a new insights tool.
It defines seven standard insight types, publisher/placement-scoped approval outcomes, bounded sparse creative assignment discovery, signed prospective invalidations, capability-gated readback, and inline-seller compatibility.
The change includes source schemas, registered webhooks and enums, documentation, training and skill guidance, a minor changeset, and executable conformance vectors covering snapshot, warning, approval, and webhook semantics.
Validation includes SHIP reviews from protocol, product/workflow, and DX/training experts; focused, schema, docs, MCP, compliance, unit, server-unit, typecheck, and current/3.0 storyboard compatibility checks all pass.

@aao-secretariat

Copy link
Copy Markdown
Contributor

Ladon cannot review this PR until merge conflicts are resolved.

"properties": {
"supported": { "const": true }
},
"required": ["supported"]

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.

MUST FIX: This tightens required on a released capabilities field. Base webhook_signing required only ["supported"]; this now requires profile, algorithms, and legacy_hmac_fallback whenever supported: true. Any 3.1.x-conformant seller that advertised webhook_signing: {supported: true, profile, algorithms} and relied on legacy_hmac_fallback's default: false — or advertised bare {supported: true} — now fails validation against 3.2. get-adcp-capabilities-response.json ships in dist/schemas/3.1.x, so this is an optional→required flip on a published field, which is a breaking wire change. The minor changeset does not cover it. Either gate this behind a major/explicit escalation, or drop legacy_hmac_fallback from the newly-required set since it has a schema default.

}
}
],
"required": ["creative_id"],

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: required drops from ["creative_id", "name", "status", "created_date", "updated_date"] to ["creative_id"] on a schema that ships in dist/schemas/3.1.x. The sparse-fields motivation is sound, but this relaxes the guarantee unconditionally: even a full (non-sparse) list_creatives read is now schema-valid with name/status/created_date/updated_date absent, so buyers pinned to 3.1's always-present contract can no longer rely on it. The response schema can't see whether the request was sparse, so a conditional required isn't available — confirm this weakening of a released response contract is intended as minor rather than major.

@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: Request changes

Request changes — 1 blocking finding

Blocking findings

  • static/schemas/source/protocol/get-adcp-capabilities-response.json:1569 — webhook_signing tightens required on a released capabilities field. This is an optional↔required flip on a published wire surface (breaking) shipped under a minor changeset. Per the changeset-scope/type rule, a breaking wire change must carry a major changeset; shipping it under a minor is high.

Medium findings

  • static/schemas/source/creative/list-creatives-response.json:443 — list-creatives-response relaxes required to only creative_id on a released schema. Worth a look but non-blocking on its own.

The additive core (seven insight types, scoped approvals, prospective-invalidation webhooks, conformance vectors) is clean and well-tested, and schema↔docs coherence holds. The blocking issue is the required-tightening on a released capabilities field riding under a minor changeset — this either needs a major changeset (with the breaking-class human-ratification path) or the required set restored to non-breaking.

Note: this PR touches gated paths under static/schemas/source/** with review_decision: REVIEW_REQUIRED, and includes a breaking-class change — human/CODEOWNERS review is required regardless.

Blocking findings

  • static/schemas/source/protocol/get-adcp-capabilities-response.json:1569 — webhook_signing tightens required on a released capabilities field (breaking wire change under a minor changeset; requires major)

Medium findings

  • static/schemas/source/creative/list-creatives-response.json:443 — list-creatives-response relaxes required to only creative_id on a released schema

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