You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
PR #653 (merged) made Azure reservation purchases idempotent via deterministic reservationOrderId derived from opts.IdempotencyToken. PR #680 (commit 65ecbf81c, merged after #653) then switched all 7 Azure service executors (cache, compute, cosmosdb, database, search, managedredis, synapse) to reservations.DoPurchaseTwoStep (providers/azure/services/internal/reservations/purchase.go:106). This two-step flow:
Calls calculatePrice and Azure mints a fresh server-side reservationOrderId on every call.
Calls purchase with that fresh ID.
opts.IdempotencyToken is no longer threaded into either step. A second purchase attempt for the same (executionID, recIndex) produces a different reservationOrderId → Azure has no way to dedupe → double-purchase risk.
PR #680's commit body promised "caller-level deduplication uses the purchase-automation tag", but the caller-level guard was never implemented:
internal/purchase/execution.go:710 executeSinglePurchase calls PurchaseCommitment directly without any pre-flight lookup.
The only tag attached is cudly-purchase-source: web|cli — not unique per execution, so it can't serve as a dedupe key.
Why it matters now
Issue #639 (auto-re-drive on stranded approvals, unblocked by #641 closing) cannot proceed safely. The whole point of #641 was to make re-drive double-purchase-proof; #680 silently broke that invariant for Azure after #641 was already closed. A stranded Azure approval re-driven by the proposed #639 flip would create a second Azure reservation for every rec whose original attempt reached Azure but didn't save the row — exactly the scenario #639 is meant to make safer.
Implement the missing caller-level guard: before calling PurchaseCommitment, look up any existing reservation tagged with the deterministic per-execution token; if found, short-circuit (mirroring the AWS EC2 findRIByIdempotencyToken pattern in providers/aws/services/ec2/client.go:126-137). Tag every purchase with that token post-create.
Option 1 is cleaner if Azure supports client-supplied order IDs through the two-step flow. Option 2 mirrors the proven AWS EC2 pattern when client-supplied IDs aren't supported. Investigate which Azure actually accepts.
Tests required
A re-drive with the same IdempotencyToken on every Azure service does NOT create a second reservation (mock-asserted: either second call short-circuits, or Azure dedupes server-side because the order ID is identical).
This is a P1 blocker for #639 and a partial regression of #641. Mark priority/p1, severity/high — the financial-correctness invariant from #641 is broken.
Regression discovered while investigating #639
PR #653 (merged) made Azure reservation purchases idempotent via deterministic
reservationOrderIdderived fromopts.IdempotencyToken. PR #680 (commit65ecbf81c, merged after #653) then switched all 7 Azure service executors (cache, compute, cosmosdb, database, search, managedredis, synapse) toreservations.DoPurchaseTwoStep(providers/azure/services/internal/reservations/purchase.go:106). This two-step flow:calculatePriceand Azure mints a fresh server-sidereservationOrderIdon every call.purchasewith that fresh ID.opts.IdempotencyTokenis no longer threaded into either step. A second purchase attempt for the same(executionID, recIndex)produces a differentreservationOrderId→ Azure has no way to dedupe → double-purchase risk.PR #680's commit body promised "caller-level deduplication uses the purchase-automation tag", but the caller-level guard was never implemented:
internal/purchase/execution.go:710 executeSinglePurchasecallsPurchaseCommitmentdirectly without any pre-flight lookup.cudly-purchase-source: web|cli— not unique per execution, so it can't serve as a dedupe key.Why it matters now
Issue #639 (auto-re-drive on stranded approvals, unblocked by #641 closing) cannot proceed safely. The whole point of #641 was to make re-drive double-purchase-proof; #680 silently broke that invariant for Azure after #641 was already closed. A stranded Azure approval re-driven by the proposed #639 flip would create a second Azure reservation for every rec whose original attempt reached Azure but didn't save the row — exactly the scenario #639 is meant to make safer.
Fix direction
Either:
DoPurchaseTwoStep: derive the order ID deterministically fromopts.IdempotencyToken(the pattern PR feat(purchases): make Azure reservation purchases idempotent (refs #641) #653 used pre-fix(azure): switch 7 reservation clients to two-step calculatePrice->purchase flow (closes #677) #680); pass it into the two-step flow so the second step reuses the existing ID.PurchaseCommitment, look up any existing reservation tagged with the deterministic per-execution token; if found, short-circuit (mirroring the AWS EC2findRIByIdempotencyTokenpattern inproviders/aws/services/ec2/client.go:126-137). Tag every purchase with that token post-create.Option 1 is cleaner if Azure supports client-supplied order IDs through the two-step flow. Option 2 mirrors the proven AWS EC2 pattern when client-supplied IDs aren't supported. Investigate which Azure actually accepts.
Tests required
IdempotencyTokenon every Azure service does NOT create a second reservation (mock-asserted: either second call short-circuits, or Azure dedupes server-side because the order ID is identical).Blocking
This is a P1 blocker for #639 and a partial regression of #641. Mark
priority/p1,severity/high— the financial-correctness invariant from #641 is broken.Source of finding
implement-639investigation comment: #639 (comment)