Skip to content

bug(providers/azure): DoPurchaseTwoStep dropped IdempotencyToken threading; reintroduces double-purchase risk (regression of #641; blocks #639) #721

Description

@cristim

Regression discovered while investigating #639

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:

  1. Calls calculatePrice and Azure mints a fresh server-side reservationOrderId on every call.
  2. 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.

Fix direction

Either:

  1. Restore client-side idempotency in DoPurchaseTwoStep: derive the order ID deterministically from opts.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.
  2. 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

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-639 investigation comment: #639 (comment)

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions