Skip to content

docs: update roadmap for AdCP 3.1 parity#23

Open
bokelley wants to merge 5 commits into
mainfrom
github-pr-4145-comment
Open

docs: update roadmap for AdCP 3.1 parity#23
bokelley wants to merge 5 commits into
mainfrom
github-pr-4145-comment

Conversation

@bokelley
Copy link
Copy Markdown
Contributor

Summary

  • add D22 for tenant-aware signing key selection from the ADvendio feedback thread
  • add D23 and refresh the roadmap baseline to AdCP 3.1.0-beta.5, TS @adcp/sdk@8.1.0-beta.13, and Python adcp@6.3.0b4
  • tighten v0.1-v0.4 gates and track milestones for 3.1 beta parity, version negotiation, multi-bundle validators, SSRF/auth parity, webhook proof-of-control, and conformance runner semantics
  • clarify source-of-truth precedence between the signed protocol bundle and the pinned TS mock-server CI oracle

Review

  • protocol reviewer: no blockers after milestone/runner fixes
  • docs reviewer: final clarity findings addressed

Checks

  • git diff --check -- ROADMAP.md
  • JAVA_HOME=/opt/homebrew/opt/openjdk@21/libexec/openjdk.jdk/Contents/Home ./gradlew check
  • changeset not added: docs-only change per .changeset/README.md

@bokelley bokelley requested a review from MichielDean as a code owner May 26, 2026 23:09
github-actions[bot]
github-actions Bot previously approved these changes May 26, 2026
Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Follow-ups noted below. Governance update is internally coherent and the protocol claims hold up to spec citation in 9 of 12 places, with three spots where the SDK's internal abstraction overreaches the normative spec.

Things I checked

  • D-coherence audit on D1–D21: only D8 touched, and that's a precedence sharpening (signed bundle > mock-server pin for schema/API truth) not a drift. D22 and D23 are additive.
  • D22 motivation grounds out in JOSE convention (RFC 7517 §4.5 kid) — one JWKS, distinct kid per tenant under shared adcp_use is a real adopter pattern.
  • Upstream version pairings verified: AdCP v3.1.0-beta.5 (2026-05-26), @adcp/sdk@8.1.0-beta.13 (2026-05-26), Python v6.3.0-beta.4 (2026-05-26). "TS aligned, Python one beta behind" holds.
  • 3.1 surface claims verified verbatim against beta.2/beta.4 changelogs and whats-new-in-3-1.mdx: envelope status required (incl. sync reads), media_buy_status split with legacy status deprecated, governance verdict/outcome_state/entries[].verdict, signal_ref + scopes + signal_targeting_* + targeting_overlay.signal_targeting_groups, beta.5 format_option_* rename (beta.2 capability_ids write path removed), read-tool idempotency_key and the read_tool_idempotency storyboard, adcp_major_version mirror through 3.x.
  • CI: commitlint, changeset-check, IPR all green. Changeset omission is correct per .changeset/README.md (docs-only).

Follow-ups (non-blocking — file as issues)

  1. D23 ↔ D15 cross-ref is broken. D23 (ROADMAP.md:35) claims to update D15 spec-rev SLO but D15 (ROADMAP.md:27) is unedited. Either edit D15 to acknowledge the bundle-pin / wire-token split, or drop the claim from D23's Supersedes column.

  2. Three-value wire-version framing overreaches the spec. Diff line 49 presents bundle pin / stable wire 3.1 / prerelease wire token 3.1-beta.5 as three first-class concepts. The actual spec is two-tier: one adcp_version wire string at release precision (\"3.1\" or \"3.1-beta\"), plus the bundle pin as a separate build/cache key. The normative SDK rule is normalize \"3.1.0-beta.1\"\"3.1-beta.1\" before wire emission — one wire token, not two. Reframe the Java internal model (AdcpVersion.wire() + .bundle() with a prerelease flag on .wire()) rather than promoting the prerelease distinction to a third spec value. Say explicitly the prerelease handling collapses on 3.1 GA.

  3. Open-error-code claim should soften from MUST to SDK practice. Diff line 54 reads as if "must not fail closed on unknown error.code" is a 3.1 normative MUST. It's implementation guidance from the open-enum direction (and the STALE_RESPONSE / IDEMPOTENCY_IN_FLIGHT / PROPOSAL_NOT_FOUND adds), not a quotable spec MUST. Label as SDK forward-compatibility practice unless someone surfaces the normative passage.

  4. D22 selector context shape is the v0.2 freeze surface and isn't named. "Freeze the API shape without assuming a singleton key per use" leaves v0.2 contributors without a context type to build against. Natural Java shape given D2's record/sealed-type stance: sealed SigningContext with AdcpUse use(), @Nullable TenantId tenant(), @Nullable PrincipalRef principal(). Per D16, land specs/signing-context.md before v0.2 opens for claim. Worth a note that today's SigningProvider.forUse(AdcpUse.WEBHOOK) single-arg signature is retired by this decision. Also pick ScopedValue vs explicit param for tenant propagation — D2 lands the primitive, D22 should commit the mechanism.

  5. Envelope-stamping seam is unnamed. Diff lines 51, 159, and Track 3 scope commit to central envelope status stamping but never name where the seam is — ResponseEnvelope<P> record around adopter P, MCP transport interceptor, or handler base class method. Matters day-1 for transport and signing because outbound signing canonicalizes the envelope; ordering depends on where it materializes. Pick before transport opens for claim.

  6. Unknown-safe enum shape punted at the ROADMAP row. Three viable Java idioms (sealed Known(KnownErrorCode) + Unknown(String); enum + UNKNOWN sentinel + sidecar rawValue(); plain String + helper). Punt is fine at the row; a specs/codegen-open-enums.md should be a Track 2 deliverable before the first generated ErrorCode lands. Worth promoting alongside the *Request / *Response rule as a generator invariant, since some enums become non-exhaustive — easy for a future generator contributor to miss in the current delta-bullet placement.

  7. D22 JWKS / kid pattern wants a signing-spec citation. ADvendio-sourced framing is JOSE-conventional and plausibly fits the L1 signing surface, but cite static/schemas/source/signing/ or downgrade to "convention compatible with the L1 signing surface" — current wording reads as established spec behavior.

  8. v0.1 gate scope creep. Original gate was "L0 surface compiles, storyboards green." New gate adds bundle ingestion + multi-bundle validators + wire-version negotiation (stable AND prerelease) + envelope stamping + SSRF baseline + WWW-Authenticate parsing + Basic auth + runner output splits. Multi-bundle validators and wire-version negotiation are architectural and rewrite-expensive — keep in v0.1. SSRF baseline + WWW-Authenticate + Basic auth are additive surfaces that fit v0.2 cleanly. Honest call: defer the additive items to v0.2 or accept M+2 slips to M+3.

  9. Track 2 / Track 9 naming drift. Track 2 (ROADMAP.md:255) says "3.0 + 3.1 validator selection"; Track 9 says "Version-aware compliance cache selection." Same capability, two names — future contributors will build two registries. Pick one ("multi-bundle validator selection" reads cleanest) and cross-reference.

  10. D8 doom-loop escape valve is undefined. New D8 clause says mock-server divergence is release-blocking with "fix is to bump/patch the pin or document a temporary exception in the same PR." Honest framing, but it puts Java's bundle bumps on TS's release cadence by construction. Define who approves the exception and whether the storyboard CI gate still has to pass — otherwise the escape valve gets used silently the first time it bites.

  11. No mention of bundle-pin bump cadence vs D6 (Maven Central at v0.3). If 3.1 GA cuts between v0.2 and v0.3, does Java cut an off-cycle v0.2.1 to track it, or take a documented D15-SLO exception until v0.3? The new D15 framing makes this load-bearing.

Minor nits (non-blocking)

  1. Status row overstates D22's lock state. ROADMAP.md:585 reads "Confirmed decisions D1–D23 locked: ✅" while D22 explicitly defers the selector shape to v0.2 design work. Soften to "D1–D21 locked; D22 + D23 framing locked, shape/value pending."

  2. D22 + D23 milestone cross-refs aren't symmetric. v0.2 row cites D22 by ID; v0.1 row cites 3.1.0-beta.5 by value but not D23 by ID. Make every decision-driven gate item cite its D-number, like D22 does.

  3. Open Question 11 naming ADvendio. Naming the only concrete reviewer candidate in an open roadmap before they've signed an LOI risks chilling outreach to competitors. Consider segment-archetype framing ("EU ad-server vendor on Spring Boot 3.x") with ADvendio as parenthetical. Non-blocking — they're already giving D22-level feedback, so the call is defensible either way.

  4. decomposeUpdateMediaBuy helper (Track 7 line 375) is borrowed from TS too literally. In Java with sealed types + records the request itself is a sealed hierarchy of mutations; switch is the language-native decomposition. Reposition decompose() as a fallback for legacy non-sealed shapes, not the primary API. Tighten the wording or leave as-is and revisit at the codegen PR.

Approving on the strength of the verified protocol claims plus the clean D1–D21 coherence audit. The follow-ups are tightening, not blocking.

@bokelley bokelley requested a review from bmilekic May 26, 2026 23:18
github-actions[bot]
github-actions Bot previously approved these changes May 26, 2026
Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Follow-ups noted below. D22 and D23 are well-framed governance — D22 narrows the signing SPI before any adopter ships against it, D23 makes the signed bundle (not TS or Python) the precedence root, and both specs/<topic>.md files match D16.

Things I checked

  • D-decision coherence. D22 + D23 don't drift D1–D21. D8 was correctly updated to subordinate to D23 ("CI conformance oracle for executable behavior, but never overrides D23's signed protocol bundle for schema/API truth" — ROADMAP.md:20), D15 was refined to track the bundle pin for build/cache inputs while emitting the release-precision wire token (ROADMAP.md:27), Track 4 scope (ROADMAP.md:292-293) and Status table (ROADMAP.md:586) both updated to match. No row is now self-contradictory.
  • D23 + D8 precedence. ad-tech-protocol-expert: JOSE-correct. Signed bundle with cosign verify-blob (D4) is the only Sigstore-rooted artifact; mock-server is necessarily downstream. The four-tier precedence at ROADMAP.md:39 is right.
  • Wire-version normalization. Two-tier split — bundle pin (3.1.0-beta.5, full semver, build/cache key) vs. release-precision wire token (3.1 or 3.1-beta.N) — matches how TS @adcp/sdk@8.1 emits adcp_version on the wire. ROADMAP.md:56 is the load-bearing sentence.
  • Open-enum shape for ErrorCode. specs/codegen-open-enums.md correctly forbids a generic UNKNOWN sentinel and preserves rawValue() for forward-compat. Matches core/error.json enumMetadata (PR #3738) where recovery is the classifier, not the literal code. Strict Java enums would fail-closed on unknown codes; this doesn't.
  • D22 multi-tenant kid pattern. Same adcp_use, distinct kid per publisher tenant on one JWKS is RFC 7517 §4.5 baseline. specs/signing-context.md:44 preserves the JWK adcp_use purpose check. Nothing in AdCP §L1 signing forces a one-key-per-adcp_use invariant.
  • *Request / *Response invariant. specs/signing-context.md:22-28 follows it (SigningContext is a context record, not a *Request; the SPI takes SigningInput/SignedInput). No new builder on a response shape.
  • Changeset exemption. Docs-only governance change. .changeset/README.md exempts docs: commits with no public-API impact. Consistent.

Follow-ups (non-blocking — file as issues)

  • SigningContext verify path is backwards. agentic-product-architect: on inbound, the kid header is what tells you the tenant — so verify(SigningContext, SignedInput) at specs/signing-context.md:33 implies the caller already resolved tenant before verifying, which inverts the dependency. Worth splitting into a KeyResolver (context → KeyHandle) for the sign path and threading inbound kid → tenant for verify. D22 freezes the direction (explicit-parameter SPI, no forUse(AdcpUse)), which is right; the byte-level shape can still be refined before v0.2.
  • specs/codegen-open-enums.md has no Jackson binding section. Sealed-interface deserialization through mcp-json-jackson2:1.1.2 (D9) doesn't work out of the box — the generator needs a custom JsonDeserializer<ErrorCode> per open vocabulary plus a JsonSerializer that writes rawValue() flat. The @JsonTypeInfo reference at ROADMAP.md:241 is about polymorphic envelopes and shouldn't be conflated. Codegen track owner will rediscover this on day one; better to add a "Jackson binding" subsection now.
  • Track-size drift. adtech-product-expert: Track 2 (2.0 PM, ROADMAP.md:234) absorbed multi-bundle registry + open-enum codegen + the 3.1 envelope/governance/SignalRef/format-option/wholesale-feed/vendor-metric surface (ROADMAP.md:243-247). Track 3 (1.5 PM, ROADMAP.md:262) absorbed wire-version negotiation + envelope stamping + SSRF + WWW-Authenticate + Basic + advisory-errors. Track 6 (2.0 PM, ROADMAP.md:339) absorbed IDEMPOTENCY_IN_FLIGHT + universal idempotency_key + durable account-level subscriptions + wholesale feed webhooks + proof-of-control. None of the person-month estimates moved. First TBD claimant on track 2 or 6 will under-claim; recommend resizing in a fast-follow before the next round of track-claim issues opens.
  • ADvendio named in Decisions Wanted #11. ROADMAP.md:570 names ADvendio in public roadmap text with the hedge "has not committed engineering time or an LOI." Two governance-uncomfortable angles: (a) AdCP is positioning itself as venue-neutral and naming one EU ad-server vendor creates an unearned association presumption, (b) it sets a precedent — every reviewer candidate now expects equal namecheck. Either downgrade to the category ("an EU ad-server vendor on Spring Boot 3.x / OAuth 2.1 / multi-tenant signing shape") or move the named candidate to a private outreach tracker. The hedge doesn't undo the public visibility cost.
  • TS-vs-Python port semantics. ROADMAP.md:79-80 lists the TS helper surface and Python helper surface verbatim with no Java-idiom translation rule. Track 7 (ROADMAP.md:376) correctly demotes decomposeUpdateMediaBuy to compatibility fallback; every other helper is listed by JS name. One line ("TS helper names are reference, not normative; Java idiom — *Request builders, instance methods over free functions, @Nullable over Optional — overrides camelCase fidelity") would prevent contributors from porting EnsureGetProductsCacheScope as a static instead of CacheScope.require(response).

Minor nits (non-blocking)

  1. v0.1 gate is eight items in one sentence. ROADMAP.md:199 packs target-bundle ingestion, multi-bundle validators, wire-version negotiation for stable and prerelease tokens, SSRF/auth baseline, and storyboards-green into a single cell. Adopters/contributors will read this as one item. Bullet list would set expectations honestly.
  2. BILLING_OUT_OF_BAND is mis-grouped. ROADMAP.md:74 lists it under "Delivery and billing finality" alongside reach_window and viewability.viewed_seconds. It's an error code in core/error.json, not a delivery field. One clarifying parenthetical would prevent a codegen contributor from looking for it in the wrong schema.
  3. Webhook bullet conflates two workstreams. ROADMAP.md:64 collapses "typed webhook token round-trip" and "endpoint proof-of-control" into one line. Token echo is a v0.2 gate item (ROADMAP.md:131); proof-of-control is v0.4 (ROADMAP.md:133). Splitting into two bullets keeps ownership unambiguous.

Approved. The substantive items (KeyResolver split, Jackson binding section, track resizing) belong in the next governance PR — they refine D22's frozen direction without contradicting it.

github-actions[bot]
github-actions Bot previously approved these changes May 26, 2026
Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Governance lands clean. The bundle-pin → mock-server → TS/Python precedence (D8 refined + D23 new) is the right shape: signed protocol bundle is schema truth, the pinned @adcp/sdk/mock-server is the CI executable oracle and can never override the bundle.

Things I checked

  • D22 + D23 added as new rows; D8 + D15 refined coherently. ROADMAP.md:25-26 cross-reference each other and specs/signing-context.md matches the D22 SPI shape verbatim.
  • D16 respected — longer-form docs land in specs/<topic>.md. No docs/adr/.
  • D18 respected — three docs: commits, Conventional Commits clean.
  • D6 unchanged — first Maven Central publish still v0.3; the v0.1 M+2 → M+3 slip absorbs 3.1 beta scope into local/SNAPSHOT, not Central.
  • No changeset present — correct per .changeset/README.md (docs commits that don't touch generated docs are exempt; ROADMAP / specs aren't on the 8-artifact list).
  • 3.1 surface inventory matches changesets at adcontextprotocol/adcp@3.1.0-beta.5: envelope status required, media_buy_status split, governance verdict / outcome_state, format_option_refs, cache_scope fail-closed on get_products, SignalRef, VERSION_UNSUPPORTED + supported_versions, advisory errors[] non-promotion, open ErrorCode decoding.
  • specs/signing-context.md is consistent with the AdCP JWKS multi-tenant pattern — one operator JWKS, distinct kid per tenant under same adcp_use, inbound verify starts from kid and only then maps to tenant.
  • v0.1 gate at ROADMAP.md:136 makes the M+2 → M+3 slip explicit instead of burying it; v0.2 / v0.3 / v0.4 gates name 3.1 surfaces by spec field, not vague capabilities.

Follow-ups (non-blocking — file as issues)

  • Wire-token round-trip vs commit. ROADMAP.md:56 correctly normalizes the full prerelease pin (3.1.0-beta.5) to the wire token, but ROADMAP.md:186 commits to 3.1-beta.N as the prerelease wire form. The version-envelope schema permits bare 3.1-beta too. SDK must round-trip what the seller emits — word this as "release-precision token, normalized per spec" rather than 3.1-beta.N.
  • adcp_major_version receiver rule. ROADMAP.md:56 says emit both adcp_version and legacy adcp_major_version through 3.x. Per the version-negotiation spec the receiver also returns VERSION_UNSUPPORTED when the two disagree at the major level. Worth naming on the v0.1 gate.
  • VERSION_UNSUPPORTED error.data shape. supported_majors is in the payload and deprecated through 3.x alongside supported_versions. Codegen will need to model both.
  • SigningContext as record locks the dimensions early. specs/signing-context.md:18-22 freezes a 3-component record at v0.2 (M+4). D22 commits to growing the surface (tenant resolution in v0.3) and the v0.4 upstream-recorder SPI will plausibly add attribution fields. The repo's *Request invariant says request-shaped inputs build; SigningContext is request-shaped. Either give it a builder before v0.2 freeze, or soften D22 so the v0.2 freeze targets the parameter set, not the concrete type, until the signing track has an owner.
  • signing track is still unclaimed. ROADMAP.md:321 shows 3/14 tracks claimed; signing is not one of them, yet D22 freezes its SPI at v0.2. Add "(shape only; tenant resolution lands v0.3)" to the v0.2 gate at ROADMAP.md:137 so a v0.2 implementer reading "tenant-aware per-adcp_use key selector per D22" knows the resolver wires in v0.3.
  • Buy-side tenant vs principal ambiguity. Sell-side reading is clean. Agency/DSP callers signing on behalf of N advertiser principals won't know whether to encode advertiser identity into tenant, principal, or out-of-band. One line in specs/signing-context.md saying tenant is operator-side and principal is on-behalf-of identity covers it.
  • VerificationKeyResolver.resolve miss shape. specs/signing-context.md:36-44 returns VerificationKey synchronously; no typed shape for "kid unknown" or KMS-cache miss. @Nullable VerificationKey or a sealed return reads better than try/catch on a hot lookup path.
  • Open-enum vocabulary list. specs/codegen-open-enums.md correctly says the schema post-processor classifies open vs closed (not value count), but only names ErrorCode. By the same forward-compat logic the generator will also need open shape for media_buy_status, creative_status, recovery, and the action enums in allowed_actions[] / available_actions[]. Naming them in the spec stops a contributor from inferring closed-by-default and breaking forward-compat on a 3.1.x dot release.
  • Cold-start adcp_version. What does the SDK emit on first call when it has no prior knowledge of server capability? Worth a sentence in the version-negotiation roadmap entry.

Minor nits (non-blocking)

  1. Verify the TS / Python version claims at codegen-PR time. ROADMAP.md:41-42 asserts @adcp/sdk@8.1.0-beta.13 declares adcp_version=3.1.0-beta.5 and adcp@6.3.0b4 declares 3.1.0-beta.4. Both are verifiable against npm / PyPI; doesn't block the roadmap, blocks the v0.1 codegen pin.

Third pass on this roadmap — the prior protocol and docs reviewers already shrank the obvious follow-up list. LGTM. Follow-ups noted above.

github-actions[bot]
github-actions Bot previously approved these changes May 26, 2026
Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving. D22 and D23 land cleanly in the table, the specs sit in specs/ per D16, and the parity baseline matches what's actually published in adcontextprotocol/adcp@3.1.0-beta.5.

Things I checked

  • D-row coherence. D22 and D23 append correctly; D8 refinement ("pinned mock-server is CI conformance oracle, never overrides signed bundle for schema/API truth") and D15 refinement ("track the signed bundle pin... refined by D23") cross-reference properly. No drift on D2, D7, D9, D14, D16.
  • Parity baseline (ROADMAP.md L29-43). 3.1.0-beta.5 matches the published release dated 2026-05-26. TS @adcp/sdk@8.1.0-beta.13 aligned to that bundle. Python 6.3.0b4 on beta.4 — correctly flagged "reference, not blocker."
  • Envelope / status splits (ROADMAP.md L58-60). Required envelope status, media_buy_status additive-deprecate, governance verdict / outcome_state rename all verify against 4832-envelope-status-required, 4895-media-buy-status-additive-deprecate, 4897-governance-body-status-rename in the spec repo.
  • Universal idempotency_key (ROADMAP.md L63). "Every request, period" framing matches 4399b-universal-idempotency-key.md verbatim.
  • SigningContext SPI shape (specs/signing-context.md L20-27, L46-50). Verification ordering — start from inbound kid, look up key, then check signature and adcp_use — matches dist/compliance/3.1.0-beta.5/test-vectors/request-signing/keys.json and standard JOSE flow. Multi-kid-per-JWKS under same adcp_use is JOSE-canonical.
  • Open-enum sealed Known/Unknown (specs/codegen-open-enums.md L16-28). Right shape for forward-compatibility on ErrorCode. Pattern-matching switch + rawValue() accessor degrades gracefully — agent consumers can log/forward Unknown(rawValue) without throwing.
  • D8 source-of-truth precedence. Signed bundle > release notes > mock-server CI > TS/Python is the right ordering for AdCP; the escape hatch (bump pin or document exception with WG approval, same-PR) is the right shape.
  • M+3 slip on v0.1. Given the v0.1 gate now carries D23 codegen + multi-bundle validators + version negotiation (stable + prerelease) + envelope stamping + SSRF baseline + WWW-Authenticate/Basic + storyboard splits, M+2 was no longer realistic. Honest slip beats pre-committed slip.
  • D-decision audit. No *Async mirrors (D2), no javax / Spring Boot 2.x (D7), no MCP SDK switch (D9 still on mcp-core:1.1.2), no Kotlin parallel SDK (D14), specs in specs/ not docs/adr/ (D16). Clean.

Follow-ups (non-blocking — file as issues)

  • Tighten kid-lookup constraints in specs/signing-context.md. `security-reviewer` flagged that the spec doesn't say kid is opaque or that resolver implementations MUST NOT dereference attacker-controlled URLs from the inbound signed object. Add a §Selection rules paragraph: (a) kid is an opaque lookup key into a pre-provisioned set; (b) any HTTP fetch the resolver performs to populate that set goes through the SSRF-policy-strict HttpClient; (c) resolver MUST NOT trust URL fields from the inbound signed object. The signing track hasn't been claimed yet — close this before the v0.2 SPI freeze.
  • Constrain Found.tenant() == null handling. Same review path: when verification returns Found(key, null, null), receivers MUST either treat the request as untenanted and reject tenant-scoped operations, OR require the resolver to have populated tenant — never fall back to body/header/query for tenant. Without this in the spec, every adopter reinvents the bug.
  • Tighten prerelease wire-token normalization (ROADMAP.md L56). `ad-tech-protocol-expert` cross-checked against specs/version-negotiation.md: full pin 3.1.0-beta.5 normalizes to wire 3.1-beta.5 (drop patch, keep prerelease), not 3.1. The current text says 3.1 for stable releases (correct) and "beta wire tokens round-trip what the seller emits after normalizing full prerelease pins such as 3.1.0-beta.5" — the target form isn't shown explicitly and invites a generator bug. State it.
  • Name enumMetadata as the open/closed signal in specs/codegen-open-enums.md. The doc correctly delegates classification to the schema post-processor but doesn't say the spec marks open vocabularies via enumMetadata. Add the reference and broaden the initial list with notification-type (wholesale-feed webhook events) and task-status (added entries through 3.x). Worth pinning so contributors don't ship a generator that infers open/closed from value count.
  • MCP tool descriptions / A2A skill IDs / tool annotations belong in D23. D23 pins schemas, validators, codegen, and envelope shape but is silent on how generated tool surfaces present to MCP clients and A2A registries. Specifically: (a) generated MCP tool input descriptions consume spec text verbatim; (b) A2A skill IDs match a cross-SDK convention so Java and TS dedupe in registries; (c) with universal idempotency_key, the generator has the signal to emit idempotentHint: true on every MCP tool. Consider specs/agent-surface-emission.md as the third sibling spec.
  • v0.2 CHANGELOG note for SigningContext.tenant() semantics. Adopters who implement SigningProvider against v0.2 will see tenant() always null (resolver wiring lands at v0.3 per D22). Add a line warning that single-tenant assumption baked at v0.2 will break at v0.3.

Minor nits (non-blocking)

  1. D22 "retires any single-arg SigningProvider.forUse(AdcpUse) API before release" (ROADMAP.md L25). No such API exists yet; the wording reads as a retire when it's a never-ship. Rewording to "the v0.2 shipped API is SigningContext-based; no forUse(AdcpUse) short form ships" closes the loop. A notable line of text to retire something that hasn't been written.

LGTM. Follow-ups noted below. The follow-ups on specs/signing-context.md should close before the signing track is opened for claim — they're the kind of spec-text gap that gets implemented around if it isn't tightened first.

Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving. D22 and D23 land as one coherent governance bump: tenant-aware signing freezes the right SPI shape (one operator, multiple publisher tenants, distinct kid under same adcp_use is the standard JOSE/JWKS pattern), and pinning to 3.1.0-beta.5 with the signed-bundle-first precedence is the only ordering that survives @adcp/sdk and adcp shipping at different beta cadences.

Things I checked

  • D-decision coherence — no drift. D8 precedence language correctly subordinates the mock-server pin to D23's signed bundle. D15 refinement preserves the ≤2-week SLO via an off-cycle SNAPSHOT clause. D16 honored: new design docs live in specs/, not docs/adr/.
  • Wire-protocol claims in the 7.x/8.x deltas, verified by ad-tech-protocol-expert against live schemas at adcontextprotocol/adcp@main: enumMetadata exists, recovery classifies correctable/terminal/transient, VERSION_UNSUPPORTED is in the enum, error-code spec text explicitly mandates unknown-code recovery-fallback. media_buy_status / envelope status split, cache_scope fail-closed, SignalRef, universal idempotency_key, advisory errors[] on success, STALE_RESPONSE — all consistent with how TS @adcp/sdk@8.x shipped these.
  • D22 signing model: specs/signing-context.md:54-58 correctly inverts inbound verification to start from kid before mapping tenant — avoids attacker-driven tenant spoofing pre-signature-check. Explicit prohibition on parsing tenancy out of kid at signing-context.md:69 is the right constraint.
  • specs/codegen-open-enums.md initial open list (ErrorCode, media_buy_status, creative_status, recovery, action-discovery) — correct per spec semantics.
  • Largest-file rule N/A — ROADMAP.md is 136 net lines, under the 200 threshold; the three new specs files all under 80 lines each.
  • Changeset correctly omitted: governance change with no adopter-visible code surface.

Follow-ups (non-blocking — file as issues)

  • v0.1 M+2 → M+3 absorption is tight. D23 target bundle codegen + wire-version negotiation (stable + prerelease) + multi-bundle validators + SSRF/auth baseline + envelope stamping in one 1-month delta on the founder pair. Track 2 went 2.0→2.5 PM, Track 3 went 1.5→2.0 PM. Honest accounting per adtech-product-expert is v0.1 at M+4, or split v0.1a (codegen + version negotiation) / v0.1b (SSRF/auth/envelope). Worth resolving before contributors claim against the gate.
  • D22 freeze-then-wire silently regresses KMS providers. SigningContext.tenant() is null in v0.2; a KMS provider that ignores it ships and regresses the day v0.3 populates it. The specs/signing-context.md:500 "release notes must warn" mitigation is weak — consider a sealed Untenanted / Tenanted split, or mark the v0.2 SPI @Experimental so the freeze isn't load-bearing for external implementers.
  • specs/agent-surface-emission.md:13 idempotentHint: true mapping is wrong direction. MCP's idempotentHint means "repeated calls with same args produce no additional effect," not "key-scoped dedupe." A create_media_buy whose schema accepts idempotency_key but is sent without one creates twice — hint would be false. Per agentic-product-architect: gate the hint on "tool is read-only OR caller is required to populate `idempotency_key`," not on schema acceptance. Same spec should take a position on readOnlyHint / destructiveHint / openWorldHint (free correctness on AdCP's read/write split) and on MCP Resources vs. Tools for read-heavy catalogs — 30+ tools is a real context-budget problem.
  • specs/codegen-open-enums.md:63 lists task-status as open. A2A TaskStatus is canonically closed upstream; if Java treats it as open and TS treats it as closed, deserializer behavior diverges on malformed input. Verify against static/schemas/source/enums/task-status.json before the generator ships.
  • ROADMAP.md:56 prerelease wire-token normalization (3.1.0-beta.53.1-beta.5) needs cross-SDK confirmation. TS appears to round-trip the full SemVer prerelease tag as-emitted. If Java collapses on the wire, mock-server CI catches but adopters integrating non-mock sellers won't. Confirm against @adcp/sdk@8.1.0-beta.13 normalizeAdcpVersion() before the codegen PR.
  • D8's "WG maintainer approval in same PR" exception is self-approving with a founder pair. Either never used (people fork mock-server patches privately) or always approved. Make exceptions self-expiring: require the exception to file an issue against the mock-server repo with a deadline, and fail the next release if still open.
  • D23 bundles a governance rule and a version pin in one row. "Signed bundle is source of truth" is permanent; 3.1.0-beta.5 will be wrong in 6 weeks. The version pin already has a home in the parity baseline table at ROADMAP.md:40. D-rows without expiry metadata accelerate index rot — cleanup pass after 3.1 GA cuts.

Minor nits (non-blocking)

  1. specs/agent-surface-emission.md:14 defers A2A skill ID convention to "the protocol bundle or a follow-up cross-SDK spec." That follow-up doesn't exist yet, so "cross-SDK stable" is an assertion, not a contract. Block v0.1 codegen snapshot tests on TS/Python publishing the canonical skill-ID grammar — otherwise Java ships first and forces the others to retrofit.

Approving on the strength of ad-tech-protocol-expert sound on every wire claim and agentic-product-architect sound-with-caveats, with the product-expert milestone concerns and the idempotentHint semantic correction filed as follow-ups. The third "docs:" commit in a row carrying real surface decisions is notable — D22 and D23 are governance, not roadmap polish, and a future grep for the decision boundary would benefit from a feat(roadmap): or decision: type on rows that introduce new D-numbers.

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