fix(purchase): user-facing Retry ignores recIsSafeToRedrive, so retrying a landed Azure savings-plan buys a second one

[cristim]

Surfaced during the adversarial review of #1655 (fix for #1537). Pre-existing on main; #1655 does not cause it and does not fix it.

What

purchase.recIsSafeToRedrive (internal/purchase/manager.go:290-307) already encodes the fact that an Azure savings-plans purchase is not safe to re-drive:

case "azure":
    // Azure savings-plans uses a timestamp-based alias name and has no
    // server-side idempotency key, so a re-drive would create a duplicate.
    // All other Azure services use DoIdempotentPurchaseTwoStep (#729).
    return rec.Service != "savingsplans" && rec.Service != "savings-plans"

Its wrapper allRecsSafeToRedrive is called from exactly two places, both in the reaper:

• internal/purchase/reaper.go:224 — deciding whether to append "; safe to retry" to the failure note
• internal/purchase/manager.go:469 — RecoverStrandedApprovals, gating an in-place re-drive

It is never called from the user-facing retry path. loadAndValidateRetryRequest / checkRetryRateGates / persistRetryExecution (internal/api/handler_purchases.go) gate on status, RBAC, already-retried, resolveOpsHint, and the retry-attempt threshold — nothing consults provider re-drive safety.

Why it costs money

Every other provider path reproduces a deterministic provider token from the execution's lineage key, so a retry of an execution whose commitment actually landed short-circuits at the provider:

provider / service	dedupe mechanism
AWS, all services	ClientToken (Savings Plans) or the EC2 RI tag-guard
Azure reservations	DoIdempotentPurchaseTwoStep lookup (#729)
GCP compute CUDs	RequestId + deterministic name derived from the token (#654)
Azure savings-plans	none — order alias named from time.Now().UnixNano()

So for an Azure SP row that failed after the order landed (a timeout, a post-purchase history write failure, a lost response), the operator sees a failed row with a Retry button, clicks it, and gets a second savings plan. Unlike every other path there is no server-side guard behind it, and a savings plan cannot be cancelled — it is a multi-year commitment.

Nothing warns the operator either: the History UI hides Retry only when resolveOpsHint matches a known-persistent failure string, and none of those strings relate to provider re-drive safety.

Suggested fix

Consult the existing helper on the user-facing path, rather than adding a second notion of re-drive safety:

• In loadAndValidateRetryRequest (or checkRetryRateGates, next to the ops-hint gate), reject with a 409 + a specific reason when any selected rec fails recIsSafeToRedrive — the same shape as the existing ops_hint 409, so the frontend already renders it in place of the Retry button.
• Decide explicitly whether ?force=true may override it. Given a savings plan cannot be cancelled, defaulting to not overridable seems right; if it is overridable it should require a distinct confirm, not the existing generic threshold confirm.
• recIsSafeToRedrive's default: return false also means an unknown provider is currently retryable from the API while being un-redrivable by the reaper — worth aligning in the same change.

Regression test

Assert on purchase counts, not statuses: an azure/savingsplans failed row must be refused by the retry endpoint with zero successor rows created, while an azure/compute row with the same shape is still retryable. internal/api/handler_purchases_retry_fanout_test.go (added in #1655) has a harness that drives the real retry handler into a real purchase.Manager and counts commitments reaching the provider; the same shape fits here.

Related

• fix(purchase): retrying a per-account fan-out row re-fans-out and double-buys every account that already succeeded #1537 / fix(purchase): keep account scope on per-account retry successors #1655 — per-account retry re-fan-out (different defect, same retry path). fix(purchase): keep account scope on per-account retry successors #1655's persistRetryExecution comment was corrected to state that scope propagation is necessary for dedupe everywhere and sufficient everywhere except Azure SP, and to point at this issue.
• Azure Savings Plan purchase is non-idempotent (double-buy on re-drive) #1079 — Azure-SP-specific non-idempotency, the underlying provider-side gap.
• fix(providers/azure): restore IdempotencyToken threading in DoPurchaseTwoStep #729 — DoIdempotentPurchaseTwoStep, which covers the other Azure services.

[cristim]

Re-triaged p1 -> p0, urgency/this-sprint -> urgency/now. The severity/critical label was already correct; the derived priority was not consistent with it.

Reasoning, so the call is auditable rather than just asserted:

• The loss is irreversible. An Azure savings plan cannot be cancelled. Every other money-path defect triaged p1 this session had a recoverable end state; this one does not. Irreversibility should dominate the derived priority even when impact/few would otherwise temper it.
• The trigger is a user clicking Retry, not an internal code path reachable only under an unusual sequence. No operator error and no malice is required — the button is doing what it appears to do.
• impact/few is accurate but misleading here. It counts affected users, not per-user magnitude. A small number of customers each buying a second uncancellable commitment is a worse outcome than a large number seeing a cosmetic defect, and the priority should reflect the product of the two rather than the count alone.

Flagging one limit honestly: I have not re-verified reachability against current main in this pass, so if the Retry path has changed since the issue was filed, the priority should be revisited rather than taken as settled.

Found during a proportionality sweep of the backlog, which also surfaced a cluster of p3/severity/low roll-ups carrying security and money-path mediums inside them (#1607, #1613, LeanerCloud/cloud-commitments-platform#132, LeanerCloud/cloud-commitments-platform#134, LeanerCloud/cloud-commitments-platform#136, LeanerCloud/cloud-commitments-platform#111, LeanerCloud/cloud-commitments-go#53) and issues with missing or absent labels (#1528, #1478). Those are being handled separately.