fix(purchase): surface save error on failed-status execution record

[cristim]

Problem

COR-10 (#1184): in executeForAccount (internal/purchase/execution.go), the credential-resolution failure branch discarded the SavePurchaseExecution error (_ =), while the post-purchase save thirty lines later treats a save failure as AUDIT LOSS and returns it. If the DB write failed on the credential-failure path, the per-account "failed" row was silently lost with no log and no error, leaving a gap in the History audit trail.

Fix

Handle the failure-path save exactly like the success path: when the save fails, return an AUDIT LOSS: failed to save execution record ... error joined (via errors.Join) with the original credential-resolution error, so neither failure is masked and the underlying save error stays inspectable via errors.Is.

Test evidence

• New regression test TestExecuteForAccount_CredentialFailure_SaveErrorSurfaced replicates the real scenario (credential resolution fails AND the failed-row save fails) and asserts both the AUDIT LOSS error and the credential error surface.
• Confirmed it FAILS on pre-fix code (save error silently dropped) and passes with the fix.
• go build ./... clean; go test ./internal/purchase/... -count=1: 207 passed; go vet clean.
• Per the finding guidance, execution_test.go timing assertions were not touched (TEST-05 / TEST-05: Elapsed-wall-time parallelism assertion in purchase fan-out test is CI-load sensitive #1158 is separate).

Closes #1184

[coderabbitai]

Warning

Review limit reached

@cristim, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 43 minutes and 57 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 0f57ea17-0850-4b44-8bee-df285c5c4aed

📥 Commits

Reviewing files that changed from the base of the PR and between 451a70f and f0eccb8.

📒 Files selected for processing (2)

• internal/purchase/execution.go
• internal/purchase/execution_test.go

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/cor-10-fix

---

_{Comment @coderabbitai help to get the list of available commands.}

[cristim]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[cristim]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[cristim]

Adversarial review notes

Reviewed this as a money-path silent-failure surface (per-account "failed" row dropping silently leaves a re-approval / re-fire window). Findings:

Fixed in f0eccb8

• UNSTABLE was a real PR-caused gocyclo trip, not a flake. The new if saveErr != nil audit-loss branch pushed executeForAccount from cyclomatic complexity 10 to 11, tripping pre-commit's gocyclo -over 10 hook on all three retries. Lint / Integration / Security failures on this run match the same suite on origin/main@451a70f7 and are pre-existing (errcheck in cmd/, TestPostgresStore_* integration tests on main is red). Extracted the audit-loss save into persistFailedAccountExecution(ctx, acctExec, account, cause) so the function is back at 10 and the semantics (errors.Join, AUDIT LOSS prefix, errors.Is preservation, cause-unmasked-on-save-success) are identical. Confirmed the new regression test still passes.

Considered, no change needed

• Sibling save sites (grep SavePurchaseExecution across internal/purchase/ + internal/api/):
  • execution.go:277 post-purchase save: already returns AUDIT LOSS: ... (this PR's reference site).
  • manager.go:235 root-row finalize in executeAndFinalize: already wraps with ErrAuditLoss.
  • approvals.go:164 ApproveAndExecute ApprovedBy stamp: AUDIT GAP log-only by design - the atomic TransitionExecutionStatus to approved already landed, only attribution is lost (documented).
  • manager.go:421 safeFail explanatory-error save: AUDIT GAP log-only by design - TransitionExecutionStatus(approved → failed) already landed, only the human-readable error string is lost (documented).
  • handler_purchases.go:692 scheduleApprovedExecution: returns the save error.
  • handler_purchases.go:2004 direct-execute audit-fields stamp: AUDIT GAP log-only - pre-flight attribution, not status.
  • reaper.go:218 canonical-error annotation: log + bump Errored, status flip already landed (documented).
• Reaper interaction. The credential-failure save creates a brand-new row for the per-account fan-out unit. On save failure there is no DB row at all (not "stuck approved"), so the reaper has nothing to re-drive and the allRecsSafeToRedrive idempotency path is not triggered. No SDK call has fired (credential resolution was the failure), so no money has moved. The aggregate root row still gets finalized by executeAndFinalize (manager.go:235).
• Save-vs-SDK ordering. The fix is on the credential-failure branch, which runs before processPurchaseRecommendations (and therefore before any provider SDK call). No double-charge surface.
• Goroutine/error-return shape. executeForAccount is invoked synchronously by execution.RunForAccountsWithConcurrency, which collects (Value, Err) per account into results. The returned err is consumed by executeMultiAccount and surfaces as multiAccountPartialError or errAllAccountsFailed - no goroutine swallows it.
• Test invariant. TestExecuteForAccount_CredentialFailure_SaveErrorSurfaced asserts (a) errors.Is(err, saveErr) (would fail pre-fix - saveErr was discarded via _ =); (b) Contains(err.Error(), "AUDIT LOSS: ...") (would fail pre-fix - no such substring); (c) the credential cause still surfaces. Pinned the bug-replication invariant per feedback_go_ctx_aware_patterns.md.

Out-of-scope follow-ups

• finalizePurchaseStatus (handler_purchases.go:1594) mirrors the anti-pattern this PR fixes. When email-send fails, it tries to flip status=failed and on save-failure logs + returns "pending" to the API caller, masking the failed-failed state. Not on the money path (the row is still in pending so it won't fire), but identical shape. Worth a follow-up issue tracking the sweep.

CR re-requested.

[cristim]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.