fix(purchase): approve and direct-execute map every failure to 409, partial runs mint no revocation token, and approved-row claim races #532

Activity

1. cristim

   added

   priority/p2Backlog-worthyBacklog-worthy

   severity/mediumModerate harmModerate harm

   effort/lWeeksWeeks

   triagedItem has been triagedItem has been triaged

   urgency/this-quarterWithin the quarterWithin the quarter

   impact/fewLimited audienceLimited audience

   type/bugDefectDefect

   on Oct 5, 2026
2. cristim commented on Oct 6, 2026

   

   cristim

   on Oct 6, 2026

   MemberAuthor

   More actions

   Plan for #532 (checked against main feec673, 2026-10-06). #386 items 2/3 (runNowError, run-now stamping) are merged and are the pattern to follow; #584/#589/#585/#594 fixed the token-mode error mapping only. #415 is untouched.

   Failure modes

   • [STILL PRESENT] Session approve maps every ApproveAndExecute error to 409: internal/api/handler_purchases.go:851 (approvePurchaseViaSession). ApproveAndExecute (internal/purchase/approvals.go:316) discards the row (_, token, err), so the handler cannot tell a lost claim from a provider failure after money moved.
   • [STILL PRESENT] directExecutePurchase maps every error to 409 (handler_purchases.go:3179) and stamps ExecutedAt/ExecutedByUserID/PreApprovalSkipReason and saves them (:3158-3167) before the claim, so a 4-eyes denial or lost claim leaves executed_at on a row that never ran. Run-now already stamps after the CAS (approvals.go:~380).
   • [PARTLY FIXED] Token approve (approveViaToken, :760): tokenActionError maps invalid/expired/wrong-status to 403/410/409 (fix(api): answer 4xx for bad approval tokens and a missing revoke execution #584/fix(api): validate the emailed approval token on the delayed-approval path #589/fix(purchase): refuse mismatched client discriminators when execute omits the recommendation id #585). Post-claim execution failures fall through as a raw 500, not 502 with the row status, because ApproveExecution returns no row.
   • [STILL PRESENT] Partial runs (partially_completed): executeAndFinalize saves the row, but transitionApproveAndExecute returns before rotateApprovalToken (approvals.go:410 vs :419), so no revocation token and no executed email. The row itself exists, so there is no missing-record problem.
   • [STILL PRESENT, needs a test] approved-row claim: sync approve/run-now execute while the row is "approved"; claimAndRedrive (manager.go:452) claims stale "approved" rows. Window is only a sync run longer than the stale threshold.

   Target mapping (follows runNowError, handler_purchases.go:370), applied to session approve, direct execute (token mode in slice 3)

   • ErrFourEyesDenied -> 403
   • no row returned and ErrExecutionNotInExpectedStatus/ErrNotFound -> 409 (lost claim)
   • no row returned, other error -> 500 (not started)
   • row returned and ErrAuditLoss -> 500
   • row returned, any other error (provider failure, partial) -> 502 with details {execution_id, status}

   PR split (each <= ~400 lines, by money impact)

   1. Approve-session + direct-execute error mapping, ApproveAndExecute returns the row, direct-execute stamps after the claim (this PR, size M).
   2. Partial runs mint a revocation token and send the executed email; response reports partially_completed (size M).
   3. Token-mode ApproveExecution returns the row and maps post-claim failures to 502 (SQS caller adjusted) (size S/M).
   4. approved-row claim race: test the stale-approved reclaim vs an in-flight sync run, or document the single claimer (size S).
      Order note: slice 2 builds on the row return from slice 1, so mapping goes first; no money-moved-without-record case exists on main (the row is always saved).