feat(protocol)!: stabilize multi-version SDK 8 beta - #1038
Conversation
Add version-scoped public models and MCP schemas, align request signing with negotiated AdCP versions, and restore a semantic BrandIdentity export. BREAKING CHANGE: Low-level sign_request and async_sign_request calls now require an explicit signing_profile_version.
There was a problem hiding this comment.
Ladon verdict: Approve
Approve — SDK 8 beta multi-version public models (adcp.types.v30/v31/v32), portable MCP tools/list schemas, negotiated-pin request signing, and a collision-safe BrandIdentity export.
Gate checks:
- No no-auto-approve team match.
- gated_paths: false — row 2 does not apply.
- high_risk: true but all reasons are
(modified)with no medium-or-higher findings; no(deleted)entries. Rows 3/5 do not fire. - Semver signal: the breaking low-level signer change ships under feat(protocol)! with a BREAKING CHANGE footer and migration notes (MIGRATION_v7_to_v8.md / request-signing-migration.md); the new BrandIdentity export is recorded in public_api_snapshot.json — the mandatory semver-signal requirement is satisfied.
- No Critical/High/Medium findings. Only three low-severity notes (schema_loader.py and versioned.py verified sound under largest-file coverage; a construction-time behavior change where a legacy server_version pin + SigningConfig now raises at ADCPClient construction).
No blocking findings and no escalation trigger — falls through to row 9.
Low-severity note for follow-up: client.py:682 — legacy server_version pin combined with SigningConfig now raises at construction; confirm this is documented in migration notes.
|
Addressed the automated review follow-up in 41df3a0: the migration guides now explicitly document pre-3.0 signed-client construction behavior, with a regression test confirming unsigned legacy clients remain unaffected. |
There was a problem hiding this comment.
Ladon verdict: Approve
Approve.
The delta since the prior approve is documentation clarifications (MIGRATION_v7_to_v8.md, docs/request-signing-migration.md) plus one new conformance test asserting a signed client pinned to legacy AdCP 2.5 fails closed at construction (DeprecationWarning fired in _resolve_server_version before signing_profile_for_adcp_version raises ValueError on the unsupported '2.5' profile). Docs accurately reflect 3.0–3.2 support and construction-time failure.
No critical, high, or medium findings in this run. high_risk is true only via (modified) entries with no accompanying medium-or-higher concern, so no escalation trigger applies (rows 3–5 do not fire). gated_paths is false; no no-auto-approve team match. Prior decision was approve, so sticky escalation (row 6) does not apply. Falls through to row 9.
Summary
Validation
Closes #1036
Closes #1037
Addresses #911
BREAKING CHANGE: Low-level sign_request and async_sign_request calls now require an explicit signing_profile_version.