feat(protocol): draft account change feed - #6811
Conversation
There was a problem hiding this comment.
Ladon verdict: Escalate to human review
Escalate — gated schema paths require human/CODEOWNERS approval.
This PR modifies files under static/schemas/source/**, which are under a hard, path-based approval gate (gated_paths: true). The required review status is not APPROVED (unknown/none), so decision-table row 2 fires: the gate is not satisfied and the outcome must be escalate.
The reviewer surfaced no Critical/High/Medium findings — this is a clean additive 3.2 draft (new list_account_changes task, account.change_recorded notification, CURSOR_EXPIRED code, change_feed capability). Schema↔docs coherence, changeset scope/type (correctly additive/Normative), oneOf disjointness, and released-artifact immutability were all verified. Absent the gate, this would approve.
Human/CODEOWNERS review is required on the gated schema-source files before merge.
Escalation reasons:
- Gated paths touched without an APPROVED review:
static/schemas/source/account/list-account-changes-request.json,list-account-changes-response.json,core/account-change-recorded-webhook.json,core/account-change.json,core/account.json,core/notification-config.json,enums/error-code.json,enums/notification-type.json,index.json,protocol/get-adcp-capabilities-response.json— CODEOWNERS/human approval required.
Why human review
- Gated schema-source files changed (static/schemas/source/**) but review_decision is not APPROVED — human/CODEOWNERS approval required (decision rule row 2).
- Affected gated files include list-account-changes-request/response.json, account-change*.json, account.json, notification-config.json, error-code.json, notification-type.json, index.json, and get-adcp-capabilities-response.json.
- This PR touches a path under a hard, non-overridable approval gate (static/schemas/source/account/list-account-changes-request.json (added) matches
static/schemas/source/**; static/schemas/source/account/list-account-changes-response.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/account-change-recorded-webhook.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/account-change.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/account.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/notification-config.json (modified) matchesstatic/schemas/source/**; static/schemas/source/enums/error-code.json (modified) matchesstatic/schemas/source/**; static/schemas/source/enums/notification-type.json (modified) matchesstatic/schemas/source/**; static/schemas/source/index.json (modified) matchesstatic/schemas/source/**; static/schemas/source/protocol/get-adcp-capabilities-response.json (modified) matchesstatic/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.
|
SDK adopter follow-up filed: adcontextprotocol/adcp-client#2669. It covers generated types/enums, a typed |
|
Review at Overall: strong draft — the tree delivers what the RFC promises, and it's wired unusually completely. The notification enum's own meta-rule (anchor + Four findings: 1. blocker — the coverage matrix promises wholesale change records the schema cannot express
2. warning — cursor/filter mismatch has no defined disposition, and the silent branch loses data permanentlyThe cursor is "bound to the normalized filter" (request schema; task doc), but nothing defines what a seller MUST do when a cursor arrives with different filters than it was minted under. Two conformant implementations diverge: one rejects with 3. warning — the commit-boundary invariant is unimplementable for non-mediated changes as literally writtenInvariant item 2: "append one immutable account change record in the same commit boundary." For a seller whose authoritative reads pass through to the underlying ad server, a seller-UI or connected-platform change is visible on 4. warning — the ratified "durable seller-initiated subscription is 4.0 work" statements survive untouched and now misleadThree normative sentences this PR doesn't modify: Notes
|
|
Review round addressed in 8dad354. Thanks for the precise findings.
The follow-up also defines per-repairable-identity invalidation plus optional batch_id, authorization-scope epochs, mechanically testable stale_after_seconds coverage, and a combinator-free emitted MCP input schema regression test. RFC #6810 and the PR body now carry the same refinements. The seller remains responsible for durable feed records and retention. Buyer SDK/application work owns typed draining, local projection/checkpoint persistence, and rebootstrap helpers; that adopter work is tracked in adcontextprotocol/adcp-client#2669. CURSOR_EXPIRED transport recovery and durable reference deployment storage remain explicit pre-merge gates rather than being implied complete by this review round. |
|
Ladon cannot review this PR until merge conflicts are resolved. |
…f-truth # Conflicts: # docs.json # tests/docs-nav-validation.test.cjs
|
Main is merged at The SDK ownership split is now tracked explicitly:
The second item may be implemented in the server SDK, but the seller deployment still owns the configured durable backend and the advertised 90-day guarantee. The current process-local training store remains an explicit no-merge condition unless production advertisement is disabled. Post-merge local validation is green: focused feed tests (13/13), typecheck, schema audit, oneOf audit, docs navigation, compliance build/check, and |
There was a problem hiding this comment.
Ladon verdict: Escalate to human review
Escalate — gated schema-source files require human/CODEOWNERS approval.
This PR modifies static/schemas/source/** (a hard, deterministic approval gate) while review_decision is REVIEW_REQUIRED, not APPROVED. Per decision rule row 2, the outcome must be escalate until a human/CODEOWNERS approval is recorded.
The reviewer found no blocking or medium findings in the PR's own new work — the account change feed additions (list-account-changes-request/response.json, account-change.json, account.json, notification-config.json, error-code.json, notification-type.json, get-adcp-capabilities-response.json) are clean, additive, and schema↔docs coherent, with a correct minor changeset for an additive Normative task. The escalation is driven solely by the unsatisfied path gate, not by code defects.
Escalation reasons
- Gated schema-source files under
static/schemas/source/**changed butreview_decisionis REVIEW_REQUIRED, not APPROVED — human/CODEOWNERS approval required (rule row 2). - Affected gated files: list-account-changes-request.json, list-account-changes-response.json, account-change-recorded-webhook.json, account-change.json, account.json, notification-config.json, error-code.json, notification-type.json, index.json, get-adcp-capabilities-response.json.
This is a continuation of the prior escalate — the gate remains unsatisfied.
Why human review
- Gated schema-source files under static/schemas/source/** changed but review_decision is REVIEW_REQUIRED, not APPROVED — human/CODEOWNERS approval required (decision rule row 2).
- Affected gated files: list-account-changes-request.json, list-account-changes-response.json, account-change-recorded-webhook.json, account-change.json, account.json, notification-config.json, error-code.json, notification-type.json, index.json, get-adcp-capabilities-response.json.
- This PR touches a path under a hard, non-overridable approval gate (static/schemas/source/account/list-account-changes-request.json (added) matches
static/schemas/source/**; static/schemas/source/account/list-account-changes-response.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/account-change-recorded-webhook.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/account-change.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/account.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/notification-config.json (modified) matchesstatic/schemas/source/**; static/schemas/source/enums/error-code.json (modified) matchesstatic/schemas/source/**; static/schemas/source/enums/notification-type.json (modified) matchesstatic/schemas/source/**; static/schemas/source/index.json (modified) matchesstatic/schemas/source/**; static/schemas/source/protocol/get-adcp-capabilities-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.
|
@lukasz-pubx requesting your Working Group approval on this 3.2 account change-feed proposal. Your four protocol findings are addressed in This is an approval request for the protocol direction and wire contract. The PR still records separate pre-merge implementation gates for durable production seller storage, generated SDK support, transport conformance, and the RFC review window. |
|
Re-reviewed at
I also reviewed the same-round additions: the conditional requirement making Approve on the wire contract and implementation review, with the two pre-merge gates you're already holding (e2e The RFC-level Working Group decision I'll record on #6810 after today's WG meeting. |
|
Thanks, @lukasz-pubx. The two implementation gates from your re-review are now closed at head |
There was a problem hiding this comment.
Ladon verdict: Escalate to human review
Escalate — gated schema-source files changed and review is not yet APPROVED.
This PR modifies/adds multiple files under static/schemas/source/**, which is a hard, non-overridable approval gate (gated_paths: true). The current GitHub review_decision is REVIEW_REQUIRED, not APPROVED, so decision rule row 2 applies: the gate cannot be lifted until a human/CODEOWNERS approval is recorded, regardless of how clean the diff is.
Blocking findings: none. The reviewer surfaced no critical/high/medium findings. The account-change-feed cursor-expiry recovery and production advertisement gate are coherent across schema, implementation, compliance, spec, docs, and tests; the largest deltas are merged-from-main features. No spec/docs drift, no immutability violation, no undiscriminated oneOf, changeset present.
Escalation reasons:
- Gated files under
static/schemas/source/**changed butreview_decisionisREVIEW_REQUIRED, notAPPROVED— human/CODEOWNERS approval required (row 2). - Affected gated files: list-account-changes-request.json, list-account-changes-response.json, comply-test-controller-request.json, account-change-recorded-webhook.json, account-change.json, account.json, notification-config.json, error-code.json, notification-type.json, index.json, get-adcp-capabilities-response.json.
This sustains the prior escalate outcome; the underlying gate remains unsatisfied. Once a human/CODEOWNERS approval flips review_decision to APPROVED, this clean diff can fall through to approval.
Why human review
- Gated schema-source files under static/schemas/source/** changed but review_decision is REVIEW_REQUIRED, not APPROVED — human/CODEOWNERS approval required (decision rule row 2).
- Affected gated files: list-account-changes-request.json, list-account-changes-response.json, comply-test-controller-request.json, account-change-recorded-webhook.json, account-change.json, account.json, notification-config.json, error-code.json, notification-type.json, index.json, get-adcp-capabilities-response.json.
- This PR touches a path under a hard, non-overridable approval gate (static/schemas/source/account/list-account-changes-request.json (added) matches
static/schemas/source/**; static/schemas/source/account/list-account-changes-response.json (added) matchesstatic/schemas/source/**; static/schemas/source/compliance/comply-test-controller-request.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/account-change-recorded-webhook.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/account-change.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/account.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/notification-config.json (modified) matchesstatic/schemas/source/**; static/schemas/source/enums/error-code.json (modified) matchesstatic/schemas/source/**; static/schemas/source/enums/notification-type.json (modified) matchesstatic/schemas/source/**; static/schemas/source/index.json (modified) matchesstatic/schemas/source/**; static/schemas/source/protocol/get-adcp-capabilities-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.
There was a problem hiding this comment.
Ladon verdict: Escalate to human review
Escalate — gated schema-source files changed without an APPROVED review decision.
This PR modifies/adds files under static/schemas/source/**, which sit behind a hard, non-overridable approval gate (gated_paths: true). The current review_decision is REVIEW_REQUIRED, not APPROVED, so decision-table row 2 fires and the outcome must be escalate regardless of how clean the diff is.
The reviewer found no Critical/High/Medium findings — the account-change-feed epoch re-keying (accountScopeId → (principal, accountScopeId)), the authorizationEpoch → visibilityEpoch rename, and the supportsAccountChangeFeed version gating are well-tested, fail-closed, and schema↔docs coherent. This is a clean, well-executed change. The only blocker is the mechanical approval gate: a human/CODEOWNERS approval must be recorded before this can merge.
Escalation reasons
- Gated schema-source files under
static/schemas/source/**changed butreview_decisionisREVIEW_REQUIRED, notAPPROVED— human/CODEOWNERS approval required (decision rule row 2). - Affected gated files: list-account-changes-request.json, list-account-changes-response.json, comply-test-controller-request.json, account-change-recorded-webhook.json, account-change.json, account.json, notification-config.json, error-code.json, notification-type.json, index.json, get-adcp-capabilities-response.json.
Why human review
- Gated schema-source files under static/schemas/source/** changed but review_decision is REVIEW_REQUIRED, not APPROVED — human/CODEOWNERS approval required (decision rule row 2).
- Affected gated files: list-account-changes-request.json, list-account-changes-response.json, comply-test-controller-request.json, account-change-recorded-webhook.json, account-change.json, account.json, notification-config.json, error-code.json, notification-type.json, index.json, get-adcp-capabilities-response.json.
- This PR touches a path under a hard, non-overridable approval gate (static/schemas/source/account/list-account-changes-request.json (added) matches
static/schemas/source/**; static/schemas/source/account/list-account-changes-response.json (added) matchesstatic/schemas/source/**; static/schemas/source/compliance/comply-test-controller-request.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/account-change-recorded-webhook.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/account-change.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/account.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/notification-config.json (modified) matchesstatic/schemas/source/**; static/schemas/source/enums/error-code.json (modified) matchesstatic/schemas/source/**; static/schemas/source/enums/notification-type.json (modified) matchesstatic/schemas/source/**; static/schemas/source/index.json (modified) matchesstatic/schemas/source/**; static/schemas/source/protocol/get-adcp-capabilities-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.
There was a problem hiding this comment.
Ladon verdict: Escalate to human review
Escalate — gated schema-source files require human/CODEOWNERS approval.
This PR touches multiple files under static/schemas/source/**, which is a hard, non-overridable approval gate (gated_paths: true). The current review_decision is unknown/none — not APPROVED — so decision rule row 2 forces escalation regardless of how clean the diff is.
Escalation reasons
- Gated schema-source files changed but
review_decisionis notAPPROVED. Human/CODEOWNERS approval is required before merge. - Affected gated files: list-account-changes-request.json, list-account-changes-response.json, comply-test-controller-request.json, account-change-recorded-webhook.json, account-change.json, account.json, notification-config.json, error-code.json, notification-type.json, index.json, get-adcp-capabilities-response.json.
On the delta: The only code change since the prior review renames handleVerifyGovernanceToken → handleInspectGovernanceTokenFixture (definition + sole call site, no behavior change) to clarify it's a sandbox teaching fixture, not an auth check. Reviewer reports a clean pass with no critical/high/medium findings.
This is a net-clean iteration, but the schema-source approval gate stands until a human approval is recorded. No blocking findings exist; escalation is purely the gated-paths requirement.
Why human review
- Gated schema-source files under static/schemas/source/** changed but review_decision is not APPROVED — human/CODEOWNERS approval required (decision rule row 2).
- Affected gated files: list-account-changes-request.json, list-account-changes-response.json, comply-test-controller-request.json, account-change-recorded-webhook.json, account-change.json, account.json, notification-config.json, error-code.json, notification-type.json, index.json, get-adcp-capabilities-response.json.
- This PR touches a path under a hard, non-overridable approval gate (static/schemas/source/account/list-account-changes-request.json (added) matches
static/schemas/source/**; static/schemas/source/account/list-account-changes-response.json (added) matchesstatic/schemas/source/**; static/schemas/source/compliance/comply-test-controller-request.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/account-change-recorded-webhook.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/account-change.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/account.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/notification-config.json (modified) matchesstatic/schemas/source/**; static/schemas/source/enums/error-code.json (modified) matchesstatic/schemas/source/**; static/schemas/source/enums/notification-type.json (modified) matchesstatic/schemas/source/**; static/schemas/source/index.json (modified) matchesstatic/schemas/source/**; static/schemas/source/protocol/get-adcp-capabilities-response.json (modified) matchesstatic/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.
There was a problem hiding this comment.
Ladon verdict: Escalate to human review
Escalate — gated schema-source paths require human/CODEOWNERS approval.
This PR modifies and adds files under static/schemas/source/**, which is a hard, deterministic approval gate (gated_paths: true). The current review_decision is unknown/none — not APPROVED — so decision rule row 2 forces escalation regardless of how clean the diff is. Ladon cannot auto-approve gated schema-source changes until a human/CODEOWNERS approval is recorded.
The delta since the prior review is a single cosmetic rename in server/src/training-agent/comply-test-controller.ts (verifyGovernanceToken → alias inspectGovernanceTokenForTraining, import at L54 and call at L1497 both rewritten, no missed call site, no behavior change). The reviewer surfaced no critical/high/medium findings.
This is additionally a new protocol surface (account change feed) touching the published wire shape under static/schemas/source/** — schema additions and enum changes are protocol changes whose ratification is a human act. Prior decision was also escalate for the same gate; the gate remains unsatisfied.
Escalation reasons
- Gated files under
static/schemas/source/**changed butreview_decisionis notAPPROVED— human/CODEOWNERS approval required (rule row 2). - Affected gated files: list-account-changes-request.json, list-account-changes-response.json, comply-test-controller-request.json, account-change-recorded-webhook.json, account-change.json, account.json, notification-config.json, error-code.json, notification-type.json, index.json, get-adcp-capabilities-response.json.
Why human review
- Gated schema-source files under static/schemas/source/** changed but review_decision is not APPROVED — human/CODEOWNERS approval required (decision rule row 2).
- Affected gated files: list-account-changes-request.json, list-account-changes-response.json, comply-test-controller-request.json, account-change-recorded-webhook.json, account-change.json, account.json, notification-config.json, error-code.json, notification-type.json, index.json, get-adcp-capabilities-response.json.
- New account-change-feed protocol surface adds fields/enums to the published wire shape under static/schemas/source/** — protocol ratification is a human act; escalate unless review_decision is APPROVED.
- This PR touches a path under a hard, non-overridable approval gate (static/schemas/source/account/list-account-changes-request.json (added) matches
static/schemas/source/**; static/schemas/source/account/list-account-changes-response.json (added) matchesstatic/schemas/source/**; static/schemas/source/compliance/comply-test-controller-request.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/account-change-recorded-webhook.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/account-change.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/account.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/notification-config.json (modified) matchesstatic/schemas/source/**; static/schemas/source/enums/error-code.json (modified) matchesstatic/schemas/source/**; static/schemas/source/enums/notification-type.json (modified) matchesstatic/schemas/source/**; static/schemas/source/index.json (modified) matchesstatic/schemas/source/**; static/schemas/source/protocol/get-adcp-capabilities-response.json (modified) matchesstatic/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.
There was a problem hiding this comment.
Ladon verdict: Escalate to human review
Escalate — gated schema-source files require human/CODEOWNERS approval.
Gate: This PR touches files under the hard, non-overridable approval gate static/schemas/source/** and review_decision is REVIEW_REQUIRED (not APPROVED). Decision rule row 2 forces escalation.
Gated files: list-account-changes-request.json, list-account-changes-response.json, comply-test-controller-request.json, account-change-recorded-webhook.json, account-change.json, account.json, notification-config.json, error-code.json, notification-type.json, index.json, get-adcp-capabilities-response.json.
This PR introduces a new account-change-feed protocol surface (new fields, enums, error codes, and wire shapes). Ratification of new published protocol surface is a human act and cannot be auto-approved until a CODEOWNERS/human approval is recorded.
Prior decision was escalate. The current run surfaces one fresh Medium finding (broadened seeded fallback widens cross-session creative reads for account-less, ID-filtered requests in task-handlers.ts), so the escalation continues to be backed by fresh evidence in addition to the still-unsatisfied gate.
Medium findings
- server/src/training-agent/task-handlers.ts:13837 — Broadened seeded fallback widens cross-session creative reads for account-less requests
Human reviewers should confirm the wire-shape/protocol additions and evaluate the cross-session read scope of the seeded fallback change before merge.
Medium findings
- server/src/training-agent/task-handlers.ts:13837 — Broadened seeded fallback widens cross-session creative reads for account-less requests
Why human review
- Gated schema-source files under static/schemas/source/** changed but review_decision is REVIEW_REQUIRED, not APPROVED — human/CODEOWNERS approval required (decision rule row 2).
- Affected gated files: list-account-changes-request.json, list-account-changes-response.json, comply-test-controller-request.json, account-change-recorded-webhook.json, account-change.json, account.json, notification-config.json, error-code.json, notification-type.json, index.json, get-adcp-capabilities-response.json.
- New account-change-feed protocol surface adds fields/enums/error-codes to the published wire shape under static/schemas/source/** — protocol ratification is a human act; escalate unless review_decision is APPROVED.
- Prior decision was escalate and a fresh Medium finding (cross-session creative read widening in task-handlers.ts:13837) persists — sticky escalation backed by current evidence (row 6).
- This PR touches a path under a hard, non-overridable approval gate (static/schemas/source/account/list-account-changes-request.json (added) matches
static/schemas/source/**; static/schemas/source/account/list-account-changes-response.json (added) matchesstatic/schemas/source/**; static/schemas/source/compliance/comply-test-controller-request.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/account-change-recorded-webhook.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/account-change.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/account.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/notification-config.json (modified) matchesstatic/schemas/source/**; static/schemas/source/enums/error-code.json (modified) matchesstatic/schemas/source/**; static/schemas/source/enums/notification-type.json (modified) matchesstatic/schemas/source/**; static/schemas/source/index.json (modified) matchesstatic/schemas/source/**; static/schemas/source/protocol/get-adcp-capabilities-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 i
…(truncated)…
There was a problem hiding this comment.
Ladon verdict: Escalate to human review
Escalate — gated schema-source files changed and no APPROVED review recorded; protocol wire surface added.
Gated paths gate (decision rule row 2): This PR modifies/adds files under static/schemas/source/** (a hard, deterministic approval gate) and review_decision is not APPROVED (unknown/none). Ladon cannot auto-approve until a human/CODEOWNERS approval is recorded, regardless of how clean the diff is.
Affected gated files: list-account-changes-request.json, list-account-changes-response.json, comply-test-controller-request.json, account-change-recorded-webhook.json, account-change.json, account.json, notification-config.json, error-code.json, notification-type.json, index.json, get-adcp-capabilities-response.json.
New protocol surface: The account-change-feed feature adds new schemas, enum values (notification-type, error-code) and webhook shapes to the published wire surface. Protocol ratification of new fields/enums/error-codes is a human act — escalate unless review_decision is APPROVED.
Medium findings (non-blocking):
- server/src/training-agent/task-handlers.ts:13838 — frozenOpaqueFixtureBridge gates on account_id but ignores its value, widening cross-session controller-fixture reads. Isolation for real (non-controller-seeded) libraries remains intact. This is a fresh finding on the delta since the prior pass, sustaining the sticky escalation (row 6) alongside the gated-paths gate.
The prior escalation stands: the gated-paths gate is unsatisfied and a current medium finding persists. A human/CODEOWNERS approval is required before this can proceed.
Medium findings
- server/src/training-agent/task-handlers.ts:13838 — frozenOpaqueFixtureBridge gates on account_id but ignores its value, widening cross-session controller-fixture reads
Why human review
- Gated schema-source files under static/schemas/source/** changed but review_decision is not APPROVED (unknown/none) — human/CODEOWNERS approval required (decision rule row 2). Affected: list-account-changes-request.json, list-account-changes-response.json, comply-test-controller-request.json, account-change-recorded-webhook.json, account-change.json, account.json, notification-config.json, error-code.json, notification-type.json, index.json, get-adcp-capabilities-response.json.
- New account-change-feed protocol surface adds fields/enums/error-codes to the published wire shape under static/schemas/source/** — protocol ratification is a human act; escalate unless review_decision is APPROVED.
- Prior decision was escalate and a fresh medium finding persists (task-handlers.ts:13838 — frozenOpaqueFixtureBridge widens cross-session controller-fixture reads) — sticky escalation backed by current evidence (row 6).
- This PR touches a path under a hard, non-overridable approval gate (static/schemas/source/account/list-account-changes-request.json (added) matches
static/schemas/source/**; static/schemas/source/account/list-account-changes-response.json (added) matchesstatic/schemas/source/**; static/schemas/source/compliance/comply-test-controller-request.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/account-change-recorded-webhook.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/account-change.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/account.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/notification-config.json (modified) matchesstatic/schemas/source/**; static/schemas/source/enums/error-code.json (modified) matchesstatic/schemas/source/**; static/schemas/source/enums/notification-type.json (modified) matchesstatic/schemas/source/**; static/schemas/source/index.json (modified) matchesstatic/schemas/source/**; static/schemas/source/protocol/get-adcp-capabilities-response.json (modified) matchesstatic/schemas/source/**) and the current GitHub rev
…(truncated)…
There was a problem hiding this comment.
Ladon verdict: Escalate to human review
Escalate — gated schema-source paths require human/CODEOWNERS approval, and this PR introduces a new breaking-class protocol surface.
This PR adds the account-change-feed protocol surface: new schemas (list-account-changes-request.json, list-account-changes-response.json, account-change.json, account-change-recorded-webhook.json), new enum values (notification-type.json, error-code.json), and modifications to published wire shapes (account.json, notification-config.json, get-adcp-capabilities-response.json, index.json). All of these live under static/schemas/source/**, which is under a hard, deterministic approval gate.
Decision rule row 2 fires: gated_paths is true AND review_decision is REVIEW_REQUIRED (not APPROVED). Ladon cannot auto-approve gated schema-source changes until a human/CODEOWNERS approval is recorded, regardless of how clean the diff is.
Additionally, adding new fields, new enum values, and new error codes to the published wire shape is a protocol-surface expansion — ratification of new protocol surface is a human act per the constitution's breaking/normative decision classes. Escalate unless review_decision is APPROVED.
Reviewer delta assessment (positive): the prior medium finding (task-handlers.ts:13838 — frozenOpaqueFixtureBridge widening cross-session controller-fixture reads) has been RESOLVED. The read path now gates on an exact FROZEN_PAGINATION_ACCOUNT_ID plus three named fixture IDs, and needsSeededFallback requires req.account before borrowing. New tests confirm acct_runner_generated cannot read the pagination fixture and an account-less non-empty session returns []. No new findings surfaced in this run. The escalation is driven entirely by the gated-path / breaking-class governance gates, not by any outstanding code defect.
Escalation reasons
- Gated schema-source files under
static/schemas/source/**changed butreview_decisionisREVIEW_REQUIRED, notAPPROVED— human/CODEOWNERS approval required (row 2). Affected: list-account-changes-request.json, list-account-changes-response.json, comply-test-controller-request.json, account-change-recorded-webhook.json, account-change.json, account.json, notification-config.json, error-code.json, notification-type.json, index.json, get-adcp-capabilities-response.json. - New account-change-feed protocol surface adds fields, enum values, and error codes to the published wire shape — protocol ratification is a human act; escalate unless review_decision is APPROVED.
Why human review
- Gated schema-source files under static/schemas/source/** changed but review_decision is REVIEW_REQUIRED, not APPROVED — human/CODEOWNERS approval required (decision rule row 2). Affected: list-account-changes-request.json, list-account-changes-response.json, comply-test-controller-request.json, account-change-recorded-webhook.json, account-change.json, account.json, notification-config.json, error-code.json, notification-type.json, index.json, get-adcp-capabilities-response.json.
- New account-change-feed protocol surface adds fields, enum values, and error codes to the published wire shape under static/schemas/source/** — protocol ratification is a human act; escalate unless review_decision is APPROVED.
- This PR touches a path under a hard, non-overridable approval gate (static/schemas/source/account/list-account-changes-request.json (added) matches
static/schemas/source/**; static/schemas/source/account/list-account-changes-response.json (added) matchesstatic/schemas/source/**; static/schemas/source/compliance/comply-test-controller-request.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/account-change-recorded-webhook.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/account-change.json (added) matchesstatic/schemas/source/**; static/schemas/source/co
…(truncated)…
|
Final reviewer checkpoint at
The only two red checks are exact default-branch baseline defects, reproduced on
No floor was weakened and no baseline failure was waived in code. Remaining merge gates are governance/external: human CODEOWNERS approval for the gated schema surface, the RFC-level WG decision on #6810, and the generated-SDK notification/transport follow-up already listed in the PR body. Lukasz has approved the wire contract and implementation review; his RFC-level WG decision is still intentionally tracked on #6810. |
There was a problem hiding this comment.
Ladon verdict: Escalate to human review
Escalate — gated schema-source files changed without an APPROVED review.
This PR adds a new account-change-feed protocol surface: new fields, enum values (notification-type, error-code), and new schema files under static/schemas/source/**. Eleven gated files match the hard, non-overridable approval gate (static/schemas/source/**), and the current review_decision is REVIEW_REQUIRED, not APPROVED (decision rule row 2). Ladon cannot auto-approve until a human/CODEOWNERS approval is recorded, regardless of how clean the diff is.
The reviewer surfaced no Critical/High/Medium findings — wire schemas are internally consistent, the tracker-execution-selector oneOf is discriminated on asset_type, VAST/DAAST version matrices match IAB history, schema↔docs coherence holds, and the changeset is minor/additive with no removed identifiers or optional→required flips. That clean bill of health is credited, but it does not lift the gate.
Additionally, this is a new protocol surface adding fields/enums/error codes to the published wire shape — protocol ratification is a human act. The prior decision was also escalate; the current run continues to touch the gated surface with the gate unsatisfied, so the escalation stands.
Escalation reasons
- Gated files under
static/schemas/source/**changed butreview_decisionisREVIEW_REQUIRED, notAPPROVED— human/CODEOWNERS approval required (row 2). - New account-change-feed protocol surface (new fields, enum values, error codes) on the published wire shape — ratification is a human act.
No blocking code-level findings; the gate is the sole reason for escalation. Once a human/CODEOWNERS approval is recorded, a subsequent run can fall through to approve on this clean diff.
Why human review
- Gated schema-source files under static/schemas/source/** changed (list-account-changes-request.json, list-account-changes-response.json, comply-test-controller-request.json, account-change-recorded-webhook.json, account-change.json, account.json, notification-config.json, error-code.json, notification-type.json, index.json, get-adcp-capabilities-response.json) but review_decision is REVIEW_REQUIRED, not APPROVED — human/CODEOWNERS approval required (decision rule row 2).
- New account-change-feed protocol surface adds fields, enum values, and error codes to the published wire shape under static/schemas/source/** — protocol ratification is a human act; escalate unless review_decision is APPROVED.
- This PR touches a path under a hard, non-overridable approval gate (static/schemas/source/account/list-account-changes-request.json (added) matches
static/schemas/source/**; static/schemas/source/account/list-account-changes-response.json (added) matchesstatic/schemas/source/**; static/schemas/source/compliance/comply-test-controller-request.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/account-change-recorded-webhook.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/account-change.json (added) matchesstatic/schemas/source/**; static/schemas/source/core/account.json (modified) matchesstatic/schemas/source/**; static/schemas/source/core/notification-config.json (modified) matchesstatic/schemas/source/**; static/schemas/source/enums/error-code.json (modified) matchesstatic/schemas/source/**; static/schemas/source/enums/notification-type.json (modified) matchesstatic/schemas/source/**; static/schemas/source/index.json (modified) matchesstatic/schemas/source/**; static/schemas/source/protocol/get-adcp-capabilities-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.
|
Post-
The remaining gates are intentionally human/external: RFC acceptance after its review window and the Working Group decision on #6810, an APPROVED human/CODEOWNERS review for the gated schema surface, and the generated-SDK notification/transport work already tracked in the PR body and adcp-client#2669. |
Summary
list_account_changesdurable account feed,account.change_recordedwake-up, capability declaration, cursor-expiry recovery, privacy boundary, and 90-day retention floorRFC/epic: #6810
Why a new feed
webhook_activity[]is delivery-attempt diagnostics, task history covers AdCP operations, and snapshots only answer current state. None can durably enumerate changes made while the buyer is idle. The feed is ordered metadata plus a safe repair-read hint; authoritative reads remain current truth.Scope boundary
The completeness claim covers committed material changes to account-scoped control-plane fields recoverable through an advertised authoritative read. It excludes raw audience members/events, per-impression delivery accumulation, naturally moving forecasts, validation failures, dry runs, exact idempotency replays, and webhook delivery attempts.
Reviewer focus
This draft is ready for protocol review. The highest-value questions are:
an advertised authoritative read, but not high-volume data-plane events?
list_account_changesfeed preferable to per-resource feedsor notification-only convergence?
account.change_recordednotification to every active subscriber, includingchanges initiated synchronously through AdCP?
CURSOR_EXPIREDrebootstrap sufficient?
may advertise each resource family?
while 3.2 ships only source-neutral snapshots and invalidations?
The storage responsibilities split at the protocol boundary. The seller runtime
owns durable ordered records, the 90-day retention guarantee, authorization-
scoped cursors, and atomic seller-state/change commits. Buyer applications own
their local projection and last acknowledged cursor; SDK follow-up
adcontextprotocol/adcp-client#2669 covers typed drain, registration, parsing,
rebootstrap, and checkpoint helpers. An SDK storage adapter can make either
side easier to implement, but cannot turn process-local reference state into a
seller durability guarantee.
Draft / merge gates
Remaining before merge:
account.change_recordednotification value; the currently installed SDK rejects registration before regenerationCompleted in
8a0f887/ed2c09d:list_account_changestool while its store is process-local; local/test/compliance use remains available, and SDK server: durable account change feed store adcp-client#2681 tracks the durable server-SDK store needed before production can advertise the 90-day guarantee/sales/mcpintegration probe now validatesCURSOR_EXPIREDas a canonical payload error, acquires a fresh latest checkpoint, rebuilds the authoritative creative snapshot, and resumes draining from the replacement cursorThe training seller now advertises only
creativecoverage. Its shared-account fixture proves that a creative added through a connected platform while the buyer is idle appears both inlist_account_changesand the authoritativelist_creativesread. Account, media-buy, money, and reporting coverage remain explicitly unadvertised until their mutation paths and snapshots meet the same bar.Within creative coverage, the reference implementation now exercises
connected-platform create and status modification, direct
sync_creatives,inline media-buy create/update, purge, shared-account readback, no-op replay
suppression, cross-account isolation, and subscriber fanout. The learning lab
uses both external creation and external modification so this is visibly a
shared account, not a buyer-exclusive sandbox.
Security decisions
repair.taskis a closed allowlist of reads and is only a hint; buyers construct and validate requests locally and never dispatch feed-provided argumentsReview changes after
716c5d8runtime, with a regression assertion for Anthropic-compatible tool catalogs
batch_idgroupingcurrentmechanically testable withstale_after_secondsrejection, including a compliance probe
list_productsandget_signalsto the repair allowlistnotification language
Validation
npm run test:schemasnpm run typechecknpm run test:docs-navnpm run test:tenant-routingnpm run test:sdk-runner-capability-gatesnpm run test:error-code-driftnpx vitest run --config server/vitest.config.ts server/tests/unit/account-change-feed.test.tsnpx vitest run --config server/vitest.config.ts server/tests/unit/training-agent.test.tsnpm run test:oneof-discriminatorsnpm run test:error-codesgit diff --check