feat(protocol): sync AdCP 3.2 beta.6 reporting - #2668
Conversation
There was a problem hiding this comment.
Ladon verdict: Approve
Approve — beta.5→beta.6 additive schema sync.
What I checked:
- Enums are strictly additive (no dropped values), no public export removed — witness-not-translator preserved.
- Hand-written surface correct:
reporting-version.tsfail-closed detection wired into three enforcement sites (client config pin, detected-capability evidence, negotiated-version coordinator). isAdcpVersionAtLeastgeneralization of hardcoded beta.5 checks holds under semver prerelease ordering.patchchangeset is defensible for an additive schema sync on a beta-track library (no removed/renamed exports, no required-param flips, no dropped enum values).- No reimplemented HTTP/SSE transport in
src/lib/protocols/.
High-risk flag: src/lib/protocols/index.ts (modified) matches a high-risk glob, but the reviewer surfaced no medium-or-higher concern on it — presumed safe per the change-kind rules.
Findings: Only one low finding (generate-zod-from-ts.ts:2615 — first-only String.replace without count guard), which is omitted from blocking consideration.
No critical/high/medium findings, gated_paths false, no no-auto-approve team match, no deletions in high-risk paths. Falls through to row 9 → approve.
There was a problem hiding this comment.
Ladon verdict: Approve
Approve — clean pass, no findings.
The delta since the prior clean pass is six test files migrating the last version-pinned compliance/schema fixture paths from 3.2.0-beta.5 to latest, plus an expected-version bump to 3.2.0-beta.6 in server-create-adcp-server.test.js. All changes align with the repo-wide latest convention already used across the suite. The added dereference: true on the storyboard cpSync is load-bearing (the test mutates and rewrites index.json) and correctly handled.
Checked against gates:
- Row 1: no critical/high findings.
- Row 2:
gated_pathsis false — no hard path gate. - Row 3–5:
high_riskis true (src/lib/protocols/index.ts modified), but the reviewer surfaced no medium-or-higher concerns on that file, so a modified high-risk file with no findings is presumed safe. - Row 6: prior decision was
approve, so sticky escalation does not apply. - Row 7: no no-auto-approve team match.
- Row 8: fewer than three medium findings (zero).
Falls through to row 9 — approve. A .changeset/*.md is present, consistent with the src/lib and package.json changes.
There was a problem hiding this comment.
Ladon verdict: Approve
Approve
Subsequent review after a prior approve. The only delta since the last approved head is a test-only formatting change in test/server-decisioning-from-platform.test.js (a multi-line path.resolve() collapsed to one line) — no behavior change, no new logic.
No findings surfaced (no critical/high/medium). Walking the decision table:
- Row 1: no critical/high findings — skip.
- Row 2:
gated_pathsis false — skip. - Row 3: no
(deleted)high-risk reasons (onlysrc/lib/protocols/index.ts (modified)) — skip. - Row 4/5: no medium findings — skip.
- Row 6: prior decision was
approve, notescalate— skip. - Row 7: no no-auto-approve team match — skip.
- Row 8: fewer than three medium findings — skip.
- Row 9: approve.
Note: high_risk is true only because src/lib/protocols/index.ts was modified, but the reviewer found no medium-or-higher concerns there, so the modification is presumed safe. review_decision: REVIEW_REQUIRED does not block auto-approve here because gated_paths is false.
There was a problem hiding this comment.
Ladon verdict: Approve
Approve — clean beta.6 reporting sync.
The delta since the prior approve (head 7efacaf) is entirely mechanical version-pin bumps (3.2.0-beta.5 → 3.2.0-beta.6) across test files, matching the PR's beta.6 reporting sync. Non-mechanical spots verified clean: release-gate derives the wire pin via version.replace('.0-','-') rather than a hardcoded literal; the coordinator build-metadata test intentionally keeps buildVersion at beta.5 to prove it does not enable compact tools; the request-signing runner points at the gitignored latest cache; and the HITL webhook loop correctly drops the retired beta.5 case.
Decision-table walk: no critical/high findings (row 1 n/a); gated_paths is false (row 2 n/a); no (deleted) high-risk reasons (row 3 n/a) — high_risk is true only from src/lib/protocols/index.ts (modified) with no finding attached; no medium findings at all (rows 4, 5, 8 n/a); prior decision was approve, not escalate (row 6 n/a); no no-auto-approve team match (row 7 n/a). Falls through to row 9 → approve.
Changeset present (.changeset/fuzzy-badgers-report.md). High-risk modified file carries no medium-or-higher concern, so per the high_risk guidance it is presumed safe.
There was a problem hiding this comment.
Ladon verdict: Approve
Approve — clean operational change since prior approve.
The delta since the prior approve (head 3d9933d) is a single package.json change: the canonical-creatives-a2a-e2e.test.js e2e test moves from the fast lane (60s timeout) to the slow lane (180s timeout). The exclusion is symmetric across test:node:fast and test:node:fast:shard, and the file is added to test:node:slow's explicit list. test:lib:fast is unaffected (globs test/lib only). No fabrication/witness-not-translator concerns, no transport-client reimplementation, and the changeset (.changeset/fuzzy-badgers-report.md) is present for the src/lib/** and package.json touches.
Decision path: No critical/high findings (row 1 no). gated_paths: false (row 2 no). high_risk is true but the only high-risk match is src/lib/protocols/index.ts (modified) — not a deletion (row 3 no), and there are no medium findings (rows 4/5 no). Prior decision was approve, not escalate (row 6 no). No no-auto-approve team match (row 7 no). Zero medium findings (row 8 no). Falls through to row 9 → approve.
Note: review_decision: REVIEW_REQUIRED does not force escalation here because gated_paths is false. high_risk on a modified protocols file with no findings is presumed safe per the flag-reading guidance.
Summary
Verification
npm run typechecknpm run build:libnpm run ci:docs-checkCloses #2666