bug(purchases): approve strands execution in 'approved' (purchase never runs, no error) on interrupted sync execution

[cristim]

Symptom

Approving a purchase repeatedly strands the execution in approved with the purchase never running. Three executions on 2026-05-20 (19:32, 22:07, 22:56, all cudly-web / cristi@leanercloud.com) are stuck:

status = approved
error = (empty)
completed_at = NULL
recommendations[].purchased = false

The AWS side confirms no commitment was created (Savings Plans / EC2-RDS-ElastiCache RIs all 0 on account 909626172446).

Root cause

Manager.ApproveAndExecute (internal/purchase/approvals.go) flips the row to approved before running the purchase synchronously inside the HTTP/Lambda request, and only finalizeExecution (internal/purchase/manager.go) sets completed/failed after executePurchase returns. If that synchronous run is interrupted (Lambda timeout, cold-start eviction, panic), the row is left persisted as approved with no error and purchased=false — the purchase never executed and there is no automatic recovery. PR #623 (#621) made these rows visible in History but did not address the stranding itself.

This is the structural problem behind #621: a financial action silently no-ops and the row requires manual DB intervention to clear.

Fix options (pick per architecture)

1. Roll back on interruption: wrap the approve+execute so that if execution does not finalize, the row returns to pending (or a dedicated approval_interrupted) rather than staying approved, so the scheduler / a retry can pick it up safely. Guard against double-execution (idempotency on commitment creation).
2. Durable queue: enqueue the approved execution to SQS and execute in a worker with visibility-timeout-based retry + a dead-letter queue, instead of synchronously in the request. The approved row then has a real owner that always finalizes it.
3. Recovery sweep: a scheduled job that finds approved rows older than N minutes with purchased=false and re-drives or fails them, so a strand can never be permanent.

Whichever path: an approved row must never be a terminal state without an owner. Add a regression test simulating an interrupted execution and asserting the row does not remain permanently approved.

Immediate cleanup (done separately)

The 3 stranded rows are being cancelled (nothing was purchased on AWS, so it is safe). This issue is the durable fix so it stops recurring.

Surfaced while diagnosing the stuck-purchase incident (#621 follow-up).

[cristim]

Independent fix path for #632 (does not wait on #721 / #639)

Re-walked Manager.ApproveAndExecute -> executeAndFinalize -> executePurchase and the existing mitigations (#635 RecoverStrandedApprovals, #678 ReapStuckExecutions). Below: the strand window, an option evaluation, and a recommendation that meaningfully reduces the operator-toil from #632 without needing #639 / #721.

The strand window (exact lines)

internal/purchase/approvals.go:110-129 (ApproveAndExecute):

1. TransitionExecutionStatus(... "approved") lands (line 111). Row is now durably approved.
2. Best-effort ApprovedBy stamp (lines 116-125).
3. executeAndFinalize(ctx, updated) runs synchronously (line 128).

internal/purchase/manager.go:156-176 (executeAndFinalize):

4. executePurchase (line 157): all cloud SDK calls fire here. Provider construction, credential resolution, per-rec fan-out, PurchaseCommitment for each rec, savePurchaseHistory per rec.
5. finalizeExecution (line 158): mutates exec.Status in memory only ("completed" / "partially_completed" / "failed").
6. SavePurchaseExecution (line 163): persists the terminal status.

The strand window is steps 4 to 6: the row is approved from the moment line 111 commits until step 6 writes a terminal status. Lambda timeout / panic / cold-start eviction inside steps 4-5, or a transient DB failure swallowing step 6, leaves the row in #632's exact "approved, error empty, completed_at NULL, purchased=false" shape. The two existing sweeps (#635 safe-fail and #678 reaper) eventually flip such rows to failed, but the operator still has to confirm the cloud state and Retry.

What is already idempotency-safe today (and what isn't)

Cataloguing the per-provider state of opts.IdempotencyToken (already plumbed through processPurchaseRecommendations at execution.go:428 via common.DeriveIdempotencyToken(exec.ExecutionID, i)):

Provider / service	Idempotency safety on re-drive
AWS Savings Plans	Native ClientToken (savingsplans/client.go:204)
AWS EC2 RI	Tag-guard via findRIByIdempotencyToken (ec2/client.go:122-137)
AWS RDS RI	idempotencyGuard short-circuit + deterministic IdempotentReservationID (rds/client.go:149,198)
AWS ElastiCache RI	idempotencyGuard + deterministic ID + recoverAlreadyExists (elasticache/client.go:134-159)
AWS OpenSearch RI	idempotencyGuard + deterministic ID + recoverAlreadyExists (opensearch/client.go:159-183)
AWS MemoryDB / Redshift	Idempotency-token paths exist in their *_test.go, present in client
GCP CUDs	Deterministic ID derived from opts.IdempotencyToken (per #641 patterns)
Azure (all 7 services via DoPurchaseTwoStep)	Token dropped (#721 regression)

This is the lever: #639's auto-re-drive is double-buy-safe on AWS-only executions today. The Azure regression (#721) is the only piece blocking the full #639. The strands in #632 were on AWS account 909626172446 (the AWS-only case is the one we actually observe in the field).

Option evaluation

Option	Effort	Helps #632?	New double-buy risk?	Depends on #721/#639
1. Write-ahead purchase_attempted_at / attempting interim status	Medium (new column or status, migration, sweep update)	Marginally: the sweep can already distinguish strands; this doesn't tell us whether the SDK call was made, only that we intended to call it	None	No
2. Async execution model (SQS worker)	Large (new infra, IAM, worker, deploy, retry contract, observability rewrite)	Yes, but it's a re-architecture	None new (existing idempotency still applies on the worker)	No
3. Resilient sync path (shorter per-phase timeouts + checkpoints)	Large (deep refactor of executor with no clear stopping point)	Partially, with new complexity	None	No
4. Cloud-side detection in the safe-fail sweep	Small to medium (per-provider commitment lookup by tag/ClientToken)	Yes, for the "actually succeeded but row never saved" case (operator no longer sees a misleading "failed" row)	None (read-only lookups, no purchase)	No
5 (recommended). AWS-only idempotent auto-re-drive in RecoverStrandedApprovals	Small (single file, no migration, gated by provider)	Yes, for the exact AWS strands in #632; converts operator-toil into automatic recovery for the common case	None (every AWS service is already idempotency-safe per the table above)	No

Option 5 is a strict subset of #639, sliced by provider: it does exactly what #639 proposes, but only for executions whose recs are all AWS. The Azure regression (#721) is sidestepped — Azure executions continue to fall through to today's safe-fail behaviour.

Recommended fix: option 5 (AWS-scoped auto-re-drive)

Design sketch

In internal/purchase/manager.go:

// allRecsAWS reports whether every selected rec in exec targets AWS. Used by
// RecoverStrandedApprovals to scope auto-re-drive to providers whose per-rec
// idempotency guards (Savings Plans ClientToken; EC2/RDS/ElastiCache/OpenSearch
// idempotency-tag short-circuits) make a re-drive double-buy-safe today.
// Azure is excluded until #721 restores token threading through
// DoPurchaseTwoStep; mixed-provider executions are excluded conservatively.
func allRecsAWS(exec *config.PurchaseExecution) bool {
    if len(exec.Recommendations) == 0 {
        return false
    }
    for i := range exec.Recommendations {
        p := exec.Recommendations[i].Provider
        if p != "" && p != "aws" {
            return false
        }
    }
    return true
}

In RecoverStrandedApprovals, between the stale SELECT and the CAS-to-failed, add an AWS-scoped re-drive branch. The CAS that already exists protects against the race where the original run finishes between SELECT and our re-drive: we keep the same atomic guard (WHERE status='approved') but transition to a fresh in-flight status (e.g. reuse approved with updated_at bumped, or introduce a sentinel; safest is reuse approved and rely on the executor's per-rec idempotency to handle the original run's residual writes if any).

for i := range stranded {
    exec := &stranded[i]

    if allRecsAWS(exec) {
        // Atomic re-drive: bump updated_at so the same row is not picked up
        // again by the next sweep tick while executeAndFinalize is mid-flight.
        // executeAndFinalize itself is idempotent on AWS (per the table above),
        // so the worst case if our re-drive crashes again is another sweep
        // tick later — each tick is double-buy-safe.
        logging.Warnf("Re-driving stranded AWS approved execution %s (idempotency-safe; #632)", exec.ExecutionID)
        if execErr := m.executeAndFinalize(ctx, exec); execErr != nil {
            logging.Errorf("Re-drive of stranded execution %s failed: %v", exec.ExecutionID, execErr)
            // fall through: leave the row in whatever state finalizeExecution wrote
            // (completed / partially_completed / failed). No additional sweep action.
        }
        recovered++
        continue
    }

    // Non-AWS (or mixed) executions: fall back to today's safe-fail path
    // (Azure DoPurchaseTwoStep loses IdempotencyToken per #721, so a re-drive
    // could double-buy until #721 is fixed). Operator confirms cloud state +
    // clicks Retry as today.
    // ... existing TransitionExecutionStatus + SavePurchaseExecution to "failed"
}

Why the existing AWS guards make this safe:

• If the original run never reached the cloud SDK call: re-drive purchases for the first time, normal happy path.
• If the original run created the commitment and crashed before the row saved: re-drive's findRIByIdempotencyToken / idempotencyGuard / ClientToken finds the existing commitment, returns its ID without buying again, savePurchaseHistory writes the (now-correct) history row, the execution stamps completed.
• If two sweep ticks race (one re-driving, the next picking it up via the SELECT before the first finished): the second sweep sees updated_at was just bumped (or runs after the first finalised) — and even if both somehow proceed, both re-drives hit the same idempotency guard and only one commitment exists. The aggregator writes one purchase_history row (the guard returns the same CommitmentID); a benign duplicate SavePurchaseHistory is the only failure mode and it is non-financial.

What option 5 does NOT fix (vs the full #639 path):

• Azure-only and mixed-provider strands stay on today's safe-fail-and-Retry path until bug(providers/azure): DoPurchaseTwoStep dropped IdempotencyToken threading; reintroduces double-purchase risk (regression of #641; blocks #639) #721 lands. This is acceptable: the field incident in bug(purchases): approve strands execution in 'approved' (purchase never runs, no error) on interrupted sync execution #632 is AWS, and the safe-fail behaviour for Azure is the same as today, not a regression.
• The underlying race (sync execution inside Lambda lifetime) still exists; we are eliminating the consequence (operator toil + misleading "failed" rows that secretly succeeded), not the cause. Option 2 (async / queue) is the only path that eliminates the cause; it is rightly deferred.

Effort estimate

• ~40 lines in manager.go (allRecsAWS + branch in RecoverStrandedApprovals).
• ~3-4 unit tests in manager_test.go:
  • AWS-only strand re-driven to completed; provider mock asserts the per-rec IdempotencyToken is the same on both attempts.
  • Azure strand still falls through to failed (regression-locks the bug(providers/azure): DoPurchaseTwoStep dropped IdempotencyToken threading; reintroduces double-purchase risk (regression of #641; blocks #639) #721 carve-out).
  • Mixed AWS+Azure strand falls through to failed.
  • Re-drive that crashes again leaves the row in a known state (either re-failed by the next sweep, or completed — never approved permanently).
• No DB migration. No new store method. No infra change.
• No dependency on bug(providers/azure): DoPurchaseTwoStep dropped IdempotencyToken threading; reintroduces double-purchase risk (regression of #641; blocks #639) #721 or feat(purchases): flip RecoverStrandedApprovals to idempotent re-drive (now that #636 makes it safe) #639.

Recommendation

STOP and let the user decide between option 5 (small, AWS-scoped, ships now) and waiting on #721 -> #639 (full fix, eventually). Option 5 is genuinely a strict subset of #639's eventual behaviour, so landing it does not commit us to a path different from the long-term design — when #721 lands and #639 lifts the Azure carve-out, allRecsAWS simply gets removed.

If the chosen path is option 5, I can implement on fix/632-aws-scoped-auto-redrive (single PR, single file + tests, no migration). If the preference is to keep waiting on #721 -> #639, no code change is needed and this comment is the record of why.