feat: adopt AdCP 3.2.0-beta.6 - #2667
Conversation
There was a problem hiding this comment.
Ladon verdict: Approve
Approve — mechanical AdCP 3.2.0-beta.5 → beta.6 adoption.
What I checked:
- Generated files (types/enums/tools/schemas/docs) track upstream faithfully; witness-not-translator holds, no fabricated fields or re-shaping at seams.
- Two new canonical format builders (seller_rendered_stateful_display, coordinated_placements) and a new
enforceElseForbiddenconformance arbitrary land cleanly with accompanying tests. src/lib/protocols/index.ts(high-risk, modified) — no medium-or-higher concern; presumed safe. No reimplemented HTTP/SSE.- Changeset present; the
patch-type note on dropped generated exports is a low/prerelease nit, not an understated-impact wire change.
Medium findings (non-blocking):
- src/lib/server/decisioning/runtime/from-platform.ts:4300 — Push operation_id version gates were generalized inconsistently across three call sites and diverge at the next GA/3.3 bump. Worth fixing before the GA bump, but not a current-run regression.
Decision path: no critical/high findings (row 1 n/a); gated_paths false (row 2 n/a); no deletions (row 3 n/a); the single medium is not data-loss/schema/infra category (row 4 n/a); high_risk file is (modified) but the medium finding is not on that file (row 5 n/a); no prior decision (row 6 n/a); no team gate (row 7 n/a); only 1 medium finding, fewer than three (row 8 n/a). Falls through to row 9 → approve.
Medium findings
- src/lib/server/decisioning/runtime/from-platform.ts:4300 — Push operation_id version gates diverge across three call sites at GA/3.3
There was a problem hiding this comment.
Ladon verdict: Approve
Approve — version-bump PR to AdCP 3.2.0-beta.6.
The bulk of this diff (~2500 lines) is generated types/schemas under src/lib/types/*.generated.ts and other .generated.ts files (exempt from wire-fidelity review). New hand-written code adds canonical format builders (seller_rendered_stateful_display, coordinated_placements), a conformance arbitrary handler for if/else discriminator forbidden fields, and generalizes three beta.5 push-operation-id predicates to "beta.5 or later." A changeset is present (patch, consistent with the library's 14.0.0-beta.7 prerelease line), so the changeset-vs-wire-impact gate is satisfied.
Medium findings
- src/lib/server/decisioning/runtime/from-platform.ts:4300 — push-operation-id predicate diverges from its two siblings for GA/future versions (beta-only regex).
Only 1 medium finding, not in data-loss/schema/infra category. high_risk is true because src/lib/protocols/index.ts was modified, but the reviewer surfaced no medium-or-higher concern on that file (row 5 requires a medium finding on a modified high-risk file — the sole medium is in from-platform.ts, not the high-risk-matched file). No no-auto-approve team match, gated_paths false, no prior decision. Falls through to row 9.
Medium findings
- src/lib/server/decisioning/runtime/from-platform.ts:4300 — from-platform push-operation-id predicate diverges from its two siblings for GA/future versions
There was a problem hiding this comment.
Ladon verdict: Approve
Approve — mechanical AdCP 3.2.0-beta.5 → 3.2.0-beta.6 pin bump.
The delta since the prior approve is a version pin bump across docs, one generated Zod file (wellknown-schemas.generated.ts), and test fixtures/assertions. No non-generated source behavior changed. Coordinator boundary tests are correctly re-anchored (buyer pin beta.6, seller advertises beta.7) so the fail-closed "newer than pin" assertions still hold; doc migration prose is updated consistently with no stale blob/main or tree/main links.
Checks:
- No critical/high findings (row 1 clear).
- gated_paths is false (row 2 N/A).
- high_risk is true only because src/lib/protocols/index.ts is (modified), but the reviewer surfaced no medium-or-higher concern on it — no (deleted) reason (rows 3–5 clear).
- Prior decision was approve, not escalate (row 6 N/A).
- No no-auto-approve team match (row 7 N/A).
- Zero medium findings — well under the ≥3 threshold (row 8 N/A).
The prior medium finding (from-platform.ts:4300) is outside this delta and does not carry forward. Falls through to row 9 → approve.
There was a problem hiding this comment.
Ladon verdict: Approve
Approve — subsequent review, no blocking findings.
This is a follow-up to a prior approve (head d1e9f77). The delta since that run is a single package.json change moving canonical-creatives-a2a-e2e.test.js and media-buy-lifecycle-release-gate.test.js from the fast Node test lanes to test:node:slow. The reviewer confirmed the find exclusions and explicit slow-lane paths are correct.
Findings
- [low] package.json:493 — test:lib:fast still runs media-buy-lifecycle-release-gate at 60s while test:node treats it as slow. Low-severity asymmetry; omitted from blocking consideration.
Decision trace
- No critical/high findings → row 1 not triggered.
- gated_paths is false → row 2 not applicable.
- high_risk is true (src/lib/protocols/index.ts modified), but no
(deleted)reasons → row 3 not triggered. - No medium findings at all → rows 4, 5 not triggered.
- Prior decision was
approve, notescalate→ row 6 not applicable. - No no-auto-approve team match → row 7 not applicable.
- Fewer than three medium findings (zero) → row 8 not applicable.
- Falls through to row 9 → approve.
The lone finding is low-severity (build/test-lane style) and does not block. Changeset present (.changeset/calm-canvases-coordinate.md); no hand-edited version line reported; no witness-not-translator or transport-client concerns raised.
|
Addressed all review feedback in 03e5599: unified the webhook operation_id release gate through the shared SemVer comparison, added GA/future-release regression coverage, and made media-buy-lifecycle-release-gate consistently slow in both test entry points. Local build, targeted test, typecheck, lint, and formatting pass. |
|
Ladon cannot review this PR until merge conflicts are resolved. |
…a6-sdk # Conflicts: # docs/TYPE-SUMMARY.md # docs/guides/MEDIA-BUY-3.2-COMPATIBILITY.md # docs/guides/PUSH-NOTIFICATION-CONFIG.md # docs/llms.txt # package.json # scripts/check-package-size.mjs # scripts/generate-zod-from-ts.ts # src/lib/conformance/schemaArbitrary.ts # src/lib/protocols/index.ts # src/lib/server/create-adcp-server.ts # src/lib/server/decisioning/runtime/from-platform.ts # src/lib/server/wire-spec-fields.generated.ts # src/lib/types/core.generated.ts # src/lib/types/inline-enums.generated.ts # src/lib/types/schemas.generated.ts # src/lib/types/tools.generated.ts # src/lib/types/wellknown-schemas.generated.ts # src/lib/version.ts # test/lib/canonical-format-builders.test.js # test/lib/cli-storyboard-show-specialism.test.js # test/lib/compact-lifecycle-storyboard-runner.test.js # test/lib/conformance-arbitrary.test.js # test/lib/conformance-seeder.test.js # test/lib/media-buy-lifecycle-coordinator.test.js # test/lib/media-buy-lifecycle-release-gate.test.js # test/lib/zod-schemas.test.js # test/request-signing-agent-integration.test.js # test/request-signing-runner-integration.test.js # test/server-create-adcp-server.test.js # test/server-decisioning-from-platform.test.js
There was a problem hiding this comment.
Ladon verdict: Approve
Approve — clean delta completing AdCP 3.2.0-beta.6 adoption.
No blocking or medium findings. The reviewer confirmed fail-closed runtime gating with authoritative-evidence guards, the isAdcpVersionAtLeast consolidation is a behavior improvement over the old range-only regexes, and beta.6 reporting-version gating is thoroughly tested (synthetic-v2, pre-discovery-pin, and dispatch cases). Regenerated types and codegen for the two promoted canonical formats are consistent with the witness-not-translator posture — no fabrication or re-shaping at a seam.
Decision-table check:
- No critical/high findings (row 1 n/a).
- gated_paths: false (row 2 n/a).
- high_risk: false; no deleted/modified sensitive paths (rows 3–5 n/a).
- Prior decision was
approve, notescalate(row 6 n/a). - No no-auto-approve team match (row 7 n/a).
- Zero medium findings (row 8 n/a).
- Falls through to row 9 → approve.
Changeset present (.changeset/calm-canvases-coordinate.md); package.json change accompanied. Doc renames (migration-12/13-to-14, MEDIA-BUY compatibility) surfaced no stale-link findings. Note: review_decision is REVIEW_REQUIRED, but with gated_paths false this does not force escalation.
Summary
mainmain, including its shared SemVer release gatesValidation
npm run build:libnpm run typechecknpm run format:checkRelated work
Completes the remaining adoption and verification portion of #2666. The reporting schema and compatibility implementation landed separately in #2668.