feat(legacypurchase): add durable legacy purchase continuation coordinator - #483
Conversation
…nator Implements the Go SDK-local coordinator for redeeming a deprecated AdCP 3.2 products_available legacy_create purchase continuation — the equivalent of the protocol's continueLegacyPurchase(CompatibilityPurchaseCoordinatorInput), per specs/legacy-compact-lifecycle-compatibility.md (adcp#6733, merged 2026-08-20) and the 3.2.0-beta.9 schema bundle this SDK pins. adcp/v3/legacypurchase adds: - Store.RegisterContinuation / Store.ContinueLegacyPurchase, modeled on the claim-once, pluggable-Backend shape already established by adcp/v3/idempotency and adcp/v3/signing/pgreplay, widened to an explicit three-state FSM (offered -> pending -> committed/failed) since redeeming a continuation is a two-phase operation whose crash-between-phases case must be observable rather than silently lost or fabricated. - Backend interface + MemoryBackend reference implementation. - Every binding check the spec states: principal, account (including the legacy_create_request account cross-check, with AdCP 2.5's no-account- field carve-out), expiry, exact loss-set acceptance, and selected- product-ID subset-and-equality against the request's explicit packages. - Atomic single-use claim proven under -race with concurrent distinct idempotency keys racing one token; deterministic replay on exact retry after success or terminal failure; fail-closed AmbiguousClaimError with recovery guidance for a claim stuck pending past its lease window. - The products-only-brief-compatibility vectors from the AdCP 3.2 bundle, run end to end, plus the negative mutations (product substitution, package-selection drift, incomplete/stale loss consent, wrong account, expiry) the vector bundle's own README documents as SDK-constructed. CompatibilityPurchaseCoordinatorInput is hand-written rather than generated: the schema is x-adcp-sdk-local and unreachable from any wire tool schema, so generate.py's auto-discovery does not produce a Go type for it (verified by regenerating types_gen.go against the pinned bundle). Deferred, disclosed rather than silently omitted, and tracked in adcp-go#482: a persistent (e.g. Postgres) Backend, full per-legacy-version create_media_buy request schema validation beyond the structural checks this package enforces, and the reverse compact-seller -> legacy-buyer server-side facade. See adcp/v3/legacypurchase/README.md and doc.go. Refs adcontextprotocol#466 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NFth3sHBrJbHuFeA8Cegdc
|
Could you take a look at four cases I reproduced locally? I saw the scope notes about the persistent backend and full legacy schema validation. These cases concern the identity checks, saved pricing, and replay behavior in the current coordinator.
The pricing check and the additional 2.5 declaration are both covered by the compatibility contract. Happy to clarify any of the reproduction steps if useful. |
|
All four cases reproduced and fixed in 844af1a. Here's what each fix does: 1. Principal check on replay ( 2. Pricing option validation ( 3. AdCP 2.5 risk declaration ( 4. MemoryBackend deep copy. All 35 tests pass with Generated by Claude Code |
| // that is StatePending, StateCommitted, or StateFailed — i.e. every case | ||
| // other than a fresh, winning claim. | ||
| func (s *Store) resolveNonOffered(ctx context.Context, rec *ContinuationRecord, input *CompatibilityPurchaseCoordinatorInput, principal, reqHash string, now time.Time) (*Result, error) { | ||
| if rec.ClaimantKey != input.IdempotencyKey { |
There was a problem hiding this comment.
Medium: resolveNonOffered never re-checks the principal — the principal parameter is threaded in (L219) and left unused. The confused-deputy guard fires only on the StateOffered claim path (validateBinding, L185). Every replay/pending/committed resolution keys off continuation_token alone.
Unlike adcp/v3/idempotency, which bakes the principal into the storage scope key so cross-principal collision is structurally impossible, this store keys records by a global continuation_token. A different authenticated principal presenting a StateCommitted token plus the original claimant's idempotency_key and matching payload hash reaches return &Result{Response: rec.Result, Replayed: true} (L244) and replays another principal's create_media_buy response — no principal check. The StatePending/AlreadyClaimedError branches likewise act on another principal's continuation without one.
Add if rec.Principal != principal { return nil, &PrincipalMismatchError{Token: rec.Token} } at the top of resolveNonOffered. The dead principal param is the tell the check was intended. Gated behind knowing the claimant's UUID today, but this is exactly the tenant-binding guard the package advertises.
There was a problem hiding this comment.
Ladon verdict: Approve
Approve — new additive package adcp/v3/legacypurchase (a durable claim-once coordinator for legacy_create purchase continuations).
What I checked:
- Purely additive PR (2579 additions, 0 deletions); 13 new files, all under the new
legacypurchasepackage. - No touches to
adcp/schemas/**,adcp/types_gen.go,tmproto/*,reference/identity-agent/**, orskills/**— none of the mandatory schema/TMP/TEE/protocol-skill gates apply. - No exported-symbol removal/rename on the wire/public-API path, so no conventional-commit breaking marker is required.
- Atomic FSM (Offered→Pending→Committed/Failed), single-use claim, exec-at-most-once, and fail-closed crash reconciliation are proven under
-race. gated_paths: false,high_risk: false, no no-auto-approve team match.
Medium findings:
store.go:220—resolveNonOffereddrops the principal re-check on the replay/pending/committed path; a different principal with the claimant's idempotency_key + matching payload could replay another principal's result.
Decision-table application: no critical/high findings (row 1 no); gated_paths false (row 2 no); no deletions (row 3 no); the single medium is not data-loss/schema/infra category and high_risk is false (rows 4/5 no); no prior escalation (row 6 no); no team gate (row 7 no); only 1 medium finding, fewer than 3 (row 8 no) → falls through to row 9: approve. The lone medium does not block. Worth a follow-up look at the principal re-check, but not a gate blocker.
There was a problem hiding this comment.
Please address the inline principal-binding finding before merge. resolveNonOffered must re-check rec.Principal before exposing pending or terminal state or replaying a committed result; continuation tokens cannot become a cross-principal response pinhole.
… pricing substitution, 2.5 loss, response aliasing
Addresses all four cases from the reviewer's reproduction on this PR:
1. resolveNonOffered received principal but never checked it against
rec.Principal, so a claimed/committed/failed continuation would replay
its result (or leak pending/terminal state) to any caller who reused
the idempotency_key, regardless of authenticated principal — the same
confused-deputy guard validateBinding already enforces for a fresh
claim was missing on every other path. Now checked first, before
ClaimantKey/RequestHash.
2. Redemption never validated a package's pricing_option_id against the
continuation's ObservedPayload, so a caller could substitute a pricing
option the seller never actually offered for that product — the spec's
"complete observed product/pricing payload" binding
(specs/legacy-compact-lifecycle-compatibility.md). Added
validatePricingSelection and PricingSelectionError.
3. RegisterContinuation only checked the two losses every source must
declare, not that a 2.5-sourced continuation also declares
mutation_idempotency_not_guaranteed (2.5 has no mutation replay
contract, per spec) — so an incompletely-consented 2.5 continuation
could be registered and later redeemed.
4. MemoryBackend's every accessor did `cp := *rec` — a shallow copy that
still shares ProductIDs/Losses/ObservedPayload/Result backing arrays
with the stored record. A caller mutating a byte in a returned replay
response mutated what the next retry would return. Added cloneRecord,
which deep-copies those fields, used by Put/Get/Claim/Complete/Fail.
store_test.go's validFixture also had ObservedPayload shaped as
{"products": [...]} with string entries — inconsistent with the real
compact_projection.products contract vectors_test.go uses (a bare array
of product objects) — corrected so validatePricingSelection has a
real shape to check against.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VPVi4oM4fC7MLRfcFW7KzE
|
Fixed in dcb8a11, all four:
Also had to fix Generated by Claude Code |
There was a problem hiding this comment.
Ladon verdict: Approve
Approve — clean delta pass, no blocking findings.
This PR adds a new adcp/v3/legacypurchase package (all files additions, 0 deletions) with tests. The reviewer found no Critical/High/Medium findings. The changes since the prior review are pure hardening with tests: cloneRecord deep-copies all four mutable slice fields (no store aliasing), resolveNonOffered now checks principal before disclosing claim state (closes a confused-deputy replay leak), validatePricingSelection fails closed on unoffered pricing options, and AdCP 2.5 mutation-idempotency loss is enforced at registration.
Gate checks:
- No no-auto-approve team match (row 7 does not fire).
- gated_paths: false — row 2 does not apply. (review_decision is CHANGES_REQUESTED, but the deterministic gated-paths gate only bites when gated_paths is true; the standalone review status does not force escalate.)
- high_risk: false — rows 3/5 do not fire.
- No schema/generated-type, TMP signing, TEE, or protocol-skill surfaces touched; no exported-symbol wire changes requiring a conventional-commit marker.
- Prior decision was approve, so sticky escalation (row 6) does not apply.
- Zero medium findings — rows 1, 4, 8 do not fire.
None of rows 1–8 match; falls through to row 9 → approve.
Summary
Implements the Go SDK-local durable coordinator for redeeming a deprecated
AdCP 3.2
products_availablelegacy_createpurchase continuation — theequivalent of the protocol's
continueLegacyPurchase(CompatibilityPurchaseCoordinatorInput), addressing#466.
adcp/v3/legacypurchase(
Store.RegisterContinuation,Store.ContinueLegacyPurchase,Backendinterface,MemoryBackendreference implementation).specs/legacy-compact-lifecycle-compatibility.mdfromadcp#6733 (merged
2026-08-20, resolving
adcp#6716) and
the
media-buy/legacy-purchase-continuation-input.jsonschema from the3.2.0-beta.9bundle this SDK already pins atadcp/v3/schemas/VERSION— confirmed byte-identical (formatting aside) between the bundle I
downloaded locally via
adcp/v3/schemas/download.shand the source repo.static/compliance/source/test-vectors/products-only-brief-compatibility/from the same bundle, vendored at
adcp/v3/legacypurchase/testdata/products-only-brief-compatibility/(see its
PROVENANCE.md) and run end to end invectors_test.go.Design
Modeled on the two existing "durable, pluggable, atomic-claim" packages in
this codebase —
adcp/v3/idempotency(PutIfAbsent-based replay) andadcp/v3/signing/pgreplay(atomic-insert replay-cap enforcement) — ratherthan inventing a new shape. Widened from
idempotency's singlePutIfAbsentinto an explicit three-state FSM(
StateOffered -> StatePending -> StateCommitted/StateFailed) becauseredeeming a continuation is a genuinely two-phase operation: claim the
token, then call an external legacy seller whose outcome isn't known at
claim time. A crash between those two phases must be observable as
StatePending, not silently lost (double-purchase) or silently fabricated —this is exactly the security-reviewer concern noted in #466's own triage
thread ("the three-state FSM requirement ... is semantically distinct from
PutIfAbsentreplay — conflating them would be a correctness footgun").CompatibilityPurchaseCoordinatorInputis hand-written rather thangenerated: the schema is
x-adcp-sdk-local: trueand is not reachable by$reffrom any AdCP tool's request/response schema, soadcp/v3/schemas/generate.py's auto-discovery does not produce a Go typefor it. Verified by downloading the pinned
3.2.0-beta.9bundle andregenerating
types_gen.golocally — noCompatibilityPurchaseCoordinatorInput/PurchaseContinuationtype isemitted, confirming this is by design, not a generator gap.
Scope decision (disclosed)
#466's full scope — durable pluggable store + a persistent backend + the
complete reverse compact-seller→legacy-buyer facade + full per-legacy-
version request-schema validation — is more than one well-verified PR
should carry. This PR ships a solid, fully-tested core: the coordinator
API, the durable-store interface, and a well-tested in-memory reference
backend, covering every one of #466's acceptance-criteria bullets an
in-memory backend can genuinely satisfy:
store_race_test.go,64 goroutines with distinct idempotency keys racing one continuation
token under
-race, exactly one wins,Executorcalled exactly once.exercised both directly (
TestContinueLegacyPurchase_RetryReturnsDeterministicPriorResult)and against every vector case.
AmbiguousClaimError, distinct from the ordinary in-flight case(
InFlightError), gated byOptions.PendingLeaseTimeout.every one has a typed error and a dedicated test.
vectors_test.gorunsevery
cases[]entry (AdCP 2.5/3.0/3.1) end to end, plus thenegative mutations (product substitution, package-selection drift,
incomplete/stale loss consent, wrong account, expiry) the vector
bundle's own README documents as SDK-suite-constructed (the upstream
bundle does not ship separate negative JSON fixtures).
adcp/v3/legacypurchase/README.md.This does not complete #466. Deferred and tracked in
#482:
Backendimplementation — this PR shipsthe interface +
MemoryBackendonly, mirroring howadcp/v3/idempotency's Postgres adapter andadcp/v3/signing/pgreplayshipped as their own follow-on work in this same PR sequence.
create_media_buyrequest schemavalidation. This PR enforces the structural rules tied directly to
atomicity/single-use-claim safety (explicit-package mode, exact
package-product-ID match, the account cross-check) — not a complete
replica of each legacy version's schema, which
adcp/v3(an AdCP3.x-only module) does not vendor. Disclosed in the README's migration
guidance as application-owned.
spec's "Established buyers against a compact-backed seller" section,
vectors.json'sreverse_compatibility_cases) — materially separate,seller-side adapter scope.
listed_purchaseis not a deferral: per the spec it passes seller-issuedfeed/pricing values straight into ordinary
buy_products, with no durablecontinuation state to claim — nothing for this coordinator to do there.
Verification
git statusclean; theadcp/v3/schemas/*.jsonfiles I downloaded locallyto inspect and cross-check the schema are gitignored and not part of this
diff.
Test plan for reviewers
cd adcp/v3 && go test -race -count=1 ./legacypurchase/...store_race_test.gofor the concurrency proof shape (mirrorsadcp/v3/idempotency/store_race_test.go's existing convention).testdata/products-only-brief-compatibility/PROVENANCE.mdforexactly which vector sections are and aren't exercised, and why.