fix(purchase): finalizePurchaseStatus silently swallows save-failure (mirrors COR-10)

[cristim]

Problem

handler_purchases.go:1594 finalizePurchaseStatus mirrors the COR-10 (LeanerCloud/cloud-commitments-cli#1184 / PR LeanerCloud/cloud-commitments-cli#1237) anti-pattern. When the approval email fails to send, it stamps Status="failed" on the row and calls SavePurchaseExecution. If that save fails, the function logs an error and returns "pending" to the API caller:

execution.Status = "failed"
execution.Error = emailReason
if err := h.config.SavePurchaseExecution(ctx, execution); err != nil {
    logging.Errorf("failed to mark execution %s as failed after email send error: %v", execution.ExecutionID, err)
    return "pending"
}
return "failed"

The row remains in whatever its previous status was (typically pending) and the API call appears successful to the caller, while:

• the failed-email reason is not persisted onto the row,
• the dashboard/history UI sees no signal that the approval email failed,
• the row sits in pending until the History expiry sweep (LeanerCloud/cloud-commitments-cli#1158) reaps it, with no breadcrumb explaining why.

This is not on the money path: the email never went out so no human approval link can be clicked, and the row will expire rather than fire. But it is the same silent-failure shape COR-10 closed on the credential-resolution path, so the audit-trail loss is identical.

Suggested fix

Mirror PR LeanerCloud/cloud-commitments-cli#1237: bubble the save error so the caller can act on it (or at minimum return "failed_send_audit_loss" instead of "pending", plus an explicit error response). The two known shapes are:

1. Return the save error from finalizePurchaseStatus. Change the signature to (string, error) and surface an AUDIT LOSS: error to the HTTP handler so the response carries a 5xx with the audit-loss reason. Callers at handler_purchases.go:1120 and :1927 would propagate the error to the existing error path.
2. Persist via the atomic TransitionExecutionStatus(["pending","notified"], "failed", ...) CAS. This is what approvals.go / manager.go use elsewhere and gives an atomic guard against a concurrent approve clobbering the failed-state write.

Either approach removes the silent-failure window. (1) is the minimal change; (2) aligns the email-send-failed path with the rest of the failed-status state machine.

Sweep target

This file (internal/api/handler_purchases.go) and handler_purchases.go:2004 (direct-execute audit-fields stamp) are the two remaining log-only SavePurchaseExecution error sites in the handler_purchases package. The latter is best-effort attribution; this one is a real status-write.

References

• PR fix(purchase): surface save error on failed-status execution record cloud-commitments-cli#1237 (the credential-resolution sibling, just fixed).
• Issue COR-10: Failed-status purchase execution save is unchecked (_ =) while the success-path save treats failure as audit loss cloud-commitments-cli#1184 (COR-10) for the underlying audit-loss requirement.

Acceptance

• finalizePurchaseStatus's save-failure no longer returns "pending" silently; the API caller sees either an error or a failed_send_audit_loss status.
• Regression test that asserts the new contract: with SavePurchaseExecution returning an error, the response carries the save-failure (and the test fails against the current code).

[cristim]

Reviewed commit: be11bdcb5. Note: origin/main moved to 3e9660d06 during the review; re-verify against current main before changing code, since a finding may have been fixed or moved.

Re-derived independently during the 2026-07-28 full review of internal/api and confirmed still live on be11bdcb5. Same defect, same function. Recording as evidence, but contesting the severity rationale, because the review found two consequences this issue's write-up explicitly rules out.

Where, on current line numbers: internal/api/handler_purchases.go:2218-2229, called from executePurchase:2558 and retryPurchase:1642.

This issue states:

This is not on the money path: the email never went out so no human approval link can be clicked, and the row will expire rather than fire.

Both halves of that are weaker than they look on be11bdcb5:

1. The row still holds a live approval token. The failed save means the row keeps status = pending with an empty error column and its approval token intact and valid until the 7-day approvalExpiryWindow sweep. The premise "the email never went out, so nobody can click the link" holds only if the send failure was total and no token was ever transmitted or logged anywhere. Anyone who does hold that token can approve a real purchase whose requester was told the request was merely "pending" delivery. "The row will expire rather than fire" is the best case, not the guaranteed one.

2. The returned status matches neither the in-memory object nor the DB row. The function sets execution.Status = "failed" in memory, fails to persist it, and then returns "pending". The API response is {"status":"pending","email_sent":false}. The persisted row is a normal-looking pending execution with no failure reason, so the History view offers nothing to explain it and resolveOpsHint (handler_purchases.go:1560) has nothing to match against. The operator's only signal is a server-side log line.

Failure scenario as re-derived: SES is misconfigured and the SavePurchaseExecution write races a connection reset. The requester sees pending, no approval email exists, the row is indistinguishable from a healthy pending execution, and it sits with a live token until the 7-day sweep expires it.

On the fix: either of the two shapes already sketched in this issue closes it. The review's phrasing of the requirement: never report the pre-mutation status after a failed state write; return an explicit error or a distinct unknown / failed_send_audit_loss status. If option (1) (bubble the error) is chosen, the response should be a 5xx so the caller retries or alerts rather than recording a phantom pending.

Triage note: currently priority/p3 / severity/low / impact/internal, justified by the "not on the money path" reading above. If the live-token point stands, that reading is too generous and this deserves a re-look. At minimum the "will expire rather than fire" claim should be edited out of the issue body so the next reader does not inherit it.

Related: LeanerCloud/cloud-commitments-cli#1310 (silent save-failure after an irreversible Execute). This issue's own "Sweep target" section names directExecutePurchase's audit-stamp site as the sibling; the 2026-07-28 review records that one in the grouped medium/low issue for internal/api, since it is pre-execute and therefore cleanly recoverable with a 500.