fix(purchase): return the row from token approve and answer post-claim failures 502

internal/api/authpublic_csrf_guard_test.go

@@ -267,7 +267,7 @@ func purchaseCSRFGuardFixture(action string, viaToken bool) func(t *testing.T) c
 
 		pm := new(MockPurchaseManager)
 		pm.On("ApproveAndExecute", mock.Anything, csrfGuardExecID, mock.Anything, mock.Anything).Return(exec, "", nil).Maybe()
-		pm.On("ApproveExecution", mock.Anything, csrfGuardExecID, csrfGuardRawToken, mock.Anything).Return("", nil).Maybe()
+		pm.On("ApproveExecution", mock.Anything, csrfGuardExecID, csrfGuardRawToken, mock.Anything).Return(nil, "", nil).Maybe()
 		pm.On("CancelExecution", mock.Anything, csrfGuardExecID, csrfGuardRawToken, mock.Anything).Return(nil).Maybe()
 
 		var m *MockAuthService

internal/api/coverage_extras_test.go

@@ -101,7 +101,7 @@ func TestHandler_approvePurchase_PurchaseError(t *testing.T) {
 
 	mockPurchase := new(MockPurchaseManager)
 	mockPurchase.On("ApproveExecution", ctx, execID, "tok", approver).
-		Return("", errors.New("approval failed"))
+		Return(nil, "", errors.New("approval failed"))
 
 	h := &Handler{purchase: mockPurchase, config: mockConfig, auth: mockAuth}
 	req := &events.LambdaFunctionURLRequest{

internal/api/executed_notification_flow_test.go

@@ -114,7 +114,7 @@ func TestExecutedNotification_TokenApprovePath(t *testing.T) {
 	mockAuth.On("HasPermissionAPI", ctx, "", "approve-own", "purchases").Return(false, nil).Maybe()
 
 	mockPurchase := new(MockPurchaseManager)
-	mockPurchase.On("ApproveExecution", ctx, execID, "valid-token", contact).Return(freshToken, nil)
+	mockPurchase.On("ApproveExecution", ctx, execID, "valid-token", contact).Return(nil, freshToken, nil)
 
 	notifier := &recordingExecutedNotifier{}
 	handler := &Handler{
@@ -187,7 +187,7 @@ func TestExecutedNotification_TokenApprovePath_RefetchFailureStillEmailsToken(t
 	mockAuth.On("HasPermissionAPI", ctx, "", "approve-own", "purchases").Return(false, nil).Maybe()
 
 	mockPurchase := new(MockPurchaseManager)
-	mockPurchase.On("ApproveExecution", ctx, execID, "valid-token", contact).Return(freshToken, nil)
+	mockPurchase.On("ApproveExecution", ctx, execID, "valid-token", contact).Return(nil, freshToken, nil)
 
 	notifier := &recordingExecutedNotifier{}
 	handler := &Handler{

internal/api/executed_notification_partial_test.go

@@ -8,6 +8,7 @@ import (
 	"time"
 
 	"github.com/LeanerCloud/cloud-commitments-platform/internal/config"
+	"github.com/LeanerCloud/cloud-commitments-platform/internal/purchase"
 	"github.com/aws/aws-lambda-go/events"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
@@ -139,19 +140,15 @@ func TestExecutedNotification_PartialDirectExecuteEmailsRevocationToken(t *testi
 	assertPartialRunEmailed(t, notifier, row)
 }
 
-// The token approve returns no row yet, so a token alongside the error is what
-// marks a committed partial run; the email comes from the re-fetched row. The
-// status mapping of this path is unchanged here.
-func TestExecutedNotification_PartialTokenApproveEmailsRevocationToken(t *testing.T) {
-	ctx := context.Background()
-	execID := "89012345-8901-8901-8901-89012345abcd"
-	contact := "contact@example.com"
-	row := partialRunRow(execID)
-
+// tokenApproveHandler wires the email-link approve: the caller is the
+// account's contact mailbox with no approve-* permission, so approvePurchase
+// takes the token branch, whose ApproveExecution returns final, token and
+// runErr. The execution is read once, for the request itself: a failed run
+// answers from the returned row, never from a re-fetch.
+func tokenApproveHandler(ctx context.Context, execID, contact string, final *config.PurchaseExecution, token string, runErr error) (*Handler, *recordingExecutedNotifier, *MockConfigStore) {
 	mockConfig := new(MockConfigStore)
 	exec := approvalTestExec(execID, contact, mockConfig)
 	mockConfig.On("GetExecutionByID", ctx, execID).Return(exec, nil).Once()
-	mockConfig.On("GetExecutionByID", ctx, execID).Return(row, nil).Once()
 	mockConfig.On("GetGlobalConfig", ctx).Return(&config.GlobalConfig{NotificationEmail: &contact}, nil)
 	mockAuth := new(MockAuthService)
 	mockAuth.On("GetAllowedAccountsAPI", mock.Anything, mock.Anything).Return(nil, nil).Maybe()
@@ -160,16 +157,69 @@ func TestExecutedNotification_PartialTokenApproveEmailsRevocationToken(t *testin
 	mockAuth.On("HasPermissionAPI", ctx, "", "approve-any", "purchases").Return(false, nil).Maybe()
 	mockAuth.On("HasPermissionAPI", ctx, "", "approve-own", "purchases").Return(false, nil).Maybe()
 	mockPurchase := new(MockPurchaseManager)
-	mockPurchase.On("ApproveExecution", ctx, execID, "valid-token", contact).Return(partialRunToken, errPartialRun)
+	mockPurchase.On("ApproveExecution", ctx, execID, "valid-token", contact).Return(final, token, runErr)
 	notifier := &recordingExecutedNotifier{}
-	handler := &Handler{purchase: mockPurchase, config: mockConfig, auth: mockAuth, emailNotifier: notifier}
+	return &Handler{purchase: mockPurchase, config: mockConfig, auth: mockAuth, emailNotifier: notifier}, notifier, mockConfig
+}
+
+// The token approve now gets the row back, so a partial run answers like the
+// session approve: emailed from the returned row, then a 502 with its status.
+func TestExecutedNotification_PartialTokenApproveEmailsRevocationToken(t *testing.T) {
+	ctx := context.Background()
+	execID := "89012345-8901-8901-8901-89012345abcd"
+	row := partialRunRow(execID)
+	handler, notifier, mockConfig := tokenApproveHandler(ctx, execID, "contact@example.com", row, partialRunToken, errPartialRun)
 
 	_, err := handler.approvePurchase(ctx, &events.LambdaFunctionURLRequest{Headers: map[string]string{"authorization": "Bearer sess-tok"}}, execID, "valid-token")
-	require.ErrorIs(t, err, errPartialRun)
+	assertPartialRun502(t, err)
 	assertPartialRunEmailed(t, notifier, row)
 	mockConfig.AssertExpectations(t)
 }
 
+// Issue #532 slice 3: a token approve maps its failures by cause, as the
+// session approve does. Past the claim (a row came back) a provider failure
+// is a 502 with the row's status and an audit loss a 500; before it, the
+// 4-eyes denial is a 403 and the token and lost-claim refusals keep their
+// 4xx. Nothing here committed, so nothing is emailed.
+func TestTokenApprove_FailureMappedByCause(t *testing.T) {
+	ctx := context.Background()
+	execID := "a1234567-a123-a123-a123-a1234567abcd"
+	failed := partialRunRow(execID)
+	failed.Status = "failed"
+	cases := map[string]struct {
+		final  *config.PurchaseExecution
+		err    error
+		code   int // 0: a plain (500) error, not a client error
+		status string
+	}{
+		"provider failure after the claim": {failed, errors.New("AWS RI purchase failed"), 502, "failed"},
+		"final status not saved":           {partialRunRow(execID), fmt.Errorf("%w: terminal save failed; original execution error: %w", config.ErrAuditLoss, errPartialRun), 500, ""},
+		"4-eyes denial before the claim":   {nil, fmt.Errorf("%w: approver is the requester", purchase.ErrFourEyesDenied), 403, ""},
+		"lost claim":                       {nil, fmt.Errorf("approve: %w", config.ErrExecutionNotInExpectedStatus), 409, ""},
+		"token rotated before the claim":   {nil, fmt.Errorf("approve: %w", config.ErrApprovalTokenChanged), 403, ""},
+		"not started, store error":         {nil, errors.New("failed to get execution: db down"), 0, ""},
+	}
+	for name, tc := range cases {
+		t.Run(name, func(t *testing.T) {
+			handler, notifier, _ := tokenApproveHandler(ctx, execID, "contact@example.com", tc.final, "", tc.err)
+			_, err := handler.approvePurchase(ctx, &events.LambdaFunctionURLRequest{Headers: map[string]string{"authorization": "Bearer sess-tok"}}, execID, "valid-token")
+			require.Error(t, err)
+			assert.Zero(t, notifier.calls)
+			ce, ok := IsClientError(err)
+			if tc.code == 0 {
+				assert.False(t, ok, "a failure before the run started stays a plain error, got %v", err)
+				return
+			}
+			require.True(t, ok, "expected a client error, got %v", err)
+			assert.Equal(t, tc.code, ce.code)
+			if tc.status != "" {
+				assert.Equal(t, execID, ce.Details()["execution_id"])
+				assert.Equal(t, tc.status, ce.Details()["status"])
+			}
+		})
+	}
+}
+
 func TestExecutedNotification_TokenApproveFailureWithoutTokenSendsNoEmail(t *testing.T) {
 	ctx := context.Background()
 	execID := "90123456-9012-9012-9012-90123456abcd"
@@ -186,7 +236,7 @@ func TestExecutedNotification_TokenApproveFailureWithoutTokenSendsNoEmail(t *tes
 	mockAuth.On("HasPermissionAPI", ctx, "", "approve-any", "purchases").Return(false, nil).Maybe()
 	mockAuth.On("HasPermissionAPI", ctx, "", "approve-own", "purchases").Return(false, nil).Maybe()
 	mockPurchase := new(MockPurchaseManager)
-	mockPurchase.On("ApproveExecution", ctx, execID, "valid-token", contact).Return("", errors.New("AWS RI purchase failed"))
+	mockPurchase.On("ApproveExecution", ctx, execID, "valid-token", contact).Return(nil, "", errors.New("AWS RI purchase failed"))
 	notifier := &recordingExecutedNotifier{}
 	handler := &Handler{purchase: mockPurchase, config: mockConfig, auth: mockAuth, emailNotifier: notifier}
 

internal/api/execution_scope_test.go

@@ -389,7 +389,7 @@ func TestApprove_OutOfScopeSessionWithTokenUsesContactGate(t *testing.T) {
 	store := new(MockConfigStore)
 	mockPurchase := new(MockPurchaseManager)
 	h := newScopeTestHandler(t, exec, []string{scopeTestProd.ID}, store, mockPurchase)
-	mockPurchase.On("ApproveExecution", mock.Anything, scopeTestExecID, "email-token", "operator@example.com").Return("revocation-token", nil)
+	mockPurchase.On("ApproveExecution", mock.Anything, scopeTestExecID, "email-token", "operator@example.com").Return(nil, "revocation-token", nil)
 	store.On("GetGlobalConfig", mock.Anything).Return(&config.GlobalConfig{}, nil)
 
 	res, err := h.approvePurchase(context.Background(), scopeTestRequest(), scopeTestExecID, "email-token")

internal/api/handler_purchases.go

@@ -1094,25 +1094,32 @@ func (h *Handler) executeTokenApprove(ctx context.Context, req *events.LambdaFun
 	// the hash.
 	// approveConstraintsForSession above read a snapshot; the funnel re-runs
 	// it on the row its claim returns, the row it buys (issue #415).
-	revocationToken, approveErr := h.purchase.ApproveExecution(ctx, execution.ExecutionID, token, actor, h.tokenApproveClaimCheck(tokenSession))
-	// A token returned with an error is a partially_completed run: part of it
-	// committed and is revocable, so it is emailed like a completed run
-	// (issue #532). Any other error has nothing to email.
-	if approveErr != nil && revocationToken == "" {
-		return nil, tokenActionError(approveErr)
-	}
-	h.emailTokenApproveOutcome(ctx, req, execution, revocationToken, actor)
+	final, revocationToken, approveErr := h.purchase.ApproveExecution(ctx, execution.ExecutionID, token, actor, h.tokenApproveClaimCheck(tokenSession))
 	if approveErr != nil {
-		return nil, tokenActionError(approveErr)
+		return nil, h.tokenApproveFailure(ctx, req, execution.ExecutionID, final, revocationToken, actor, approveErr)
 	}
+	h.emailTokenApproveOutcome(ctx, req, execution, revocationToken, actor)
 	return map[string]string{"status": "completed"}, nil
 }
 
+// tokenApproveFailure answers a failed token approve (issue #532). Once the
+// claim was won (a row came back) it answers like the session approve: a
+// partially_completed run gets its executed email, a provider failure is a
+// 502 carrying the row's status, an audit loss a 500. A 4-eyes denial is a
+// 403 on every path. Any other refusal before the claim (bad, rotated or
+// expired token, lost claim) keeps the tokenActionError mapping.
+func (h *Handler) tokenApproveFailure(ctx context.Context, req *events.LambdaFunctionURLRequest, executionID string, final *config.PurchaseExecution, revocationToken, actor string, err error) error {
+	if final == nil && !errors.Is(err, purchase.ErrFourEyesDenied) {
+		return tokenActionError(err)
+	}
+	return h.executeFailure(ctx, req, executionID, "approved", final, revocationToken, actor, err)
+}
+
 // emailTokenApproveOutcome sends the executed email for a token approve that
-// committed, in full or in part. It re-fetches the execution to pick up the
-// FINAL state (status, completed_at, per-rec purchase results)
-// ApproveAndExecute wrote -- the pre-approve `execution` struct is stale on
-// those fields. Falling back to the stale struct on a re-fetch failure is a
+// completed (tokenApproveFailure emails a partial run from the returned row).
+// It re-fetches the execution to pick up the FINAL state (status,
+// completed_at, per-rec purchase results) ApproveAndExecute wrote -- the
+// pre-approve `execution` struct is stale on those fields. Falling back to the stale struct on a re-fetch failure is a
 // cosmetic degradation of the email content only: revocationToken is already
 // in hand either way, so the Revoke link is always valid. Best-effort (issue
 // #291): errors are logged inside sendPurchaseExecutedEmail and never

internal/api/handler_purchases_plan_step_approve_test.go

@@ -97,7 +97,7 @@ func newTokenApprove(t *testing.T, perms []auth.Permission, plan *config.Purchas
 	mp := new(MockPurchaseManager)
 	// A reached ApproveExecution proves the approval was not refused up front;
 	// it answers with a conflict so the test needs no email or re-fetch wiring.
-	mp.On("ApproveExecution", mock.Anything, stepExecID, "valid-token", stepContact).Return("", config.ErrExecutionNotInExpectedStatus).Maybe()
+	mp.On("ApproveExecution", mock.Anything, stepExecID, "valid-token", stepContact).Return(nil, "", config.ErrExecutionNotInExpectedStatus).Maybe()
 	return &tokenApproveFixture{
 		handler:  &Handler{purchase: mp, config: mockConfig, auth: mockAuth},
 		purchase: mp,

internal/api/handler_purchases_test.go

@@ -74,7 +74,7 @@ func TestHandler_approvePurchase(t *testing.T) {
 	mockAuth.On("HasPermissionAPI", ctx, "", "approve-own", "purchases").Return(false, nil).Maybe()
 
 	mockPurchase := new(MockPurchaseManager)
-	mockPurchase.On("ApproveExecution", ctx, execID, "valid-token", approver).Return("", nil)
+	mockPurchase.On("ApproveExecution", ctx, execID, "valid-token", approver).Return(nil, "", nil)
 
 	handler := &Handler{purchase: mockPurchase, config: mockConfig, auth: mockAuth}
 
@@ -291,7 +291,7 @@ func TestHandler_approvePurchase_AcceptsContactEmailSession(t *testing.T) {
 	mockAuth.On("HasPermissionAPI", ctx, "", "approve-own", "purchases").Return(false, nil).Maybe()
 
 	mockPurchase := new(MockPurchaseManager)
-	mockPurchase.On("ApproveExecution", ctx, execID, "valid-token", contactEmail).Return("", nil)
+	mockPurchase.On("ApproveExecution", ctx, execID, "valid-token", contactEmail).Return(nil, "", nil)
 
 	handler := &Handler{purchase: mockPurchase, config: mockConfig, auth: mockAuth}
 

internal/api/handler_test.go

@@ -877,7 +877,7 @@ func TestHandler_HandleRequest_ApprovePurchase(t *testing.T) {
 	mockAuth.On("HasPermissionAPI", mock.Anything, "", "approve-own", "purchases").Return(false, nil).Maybe()
 
 	mockPurchase := new(MockPurchaseManager)
-	mockPurchase.On("ApproveExecution", mock.Anything, execID, "token123", approver).Return("", nil)
+	mockPurchase.On("ApproveExecution", mock.Anything, execID, "token123", approver).Return(nil, "", nil)
 
 	handler := &Handler{purchase: mockPurchase, config: mockConfig, auth: mockAuth}
 

internal/api/middleware_test.go

@@ -726,7 +726,7 @@ func TestTokenApprove_SessionRequiresCSRF(t *testing.T) {
 	t.Run("valid CSRF header", func(t *testing.T) {
 		mockPurchase := new(MockPurchaseManager)
 		handler := newHandler(new(MockConfigStore), mockPurchase)
-		mockPurchase.On("ApproveExecution", ctx, execID, "email-token", contactEmail).Return("", nil)
+		mockPurchase.On("ApproveExecution", ctx, execID, "email-token", contactEmail).Return(nil, "", nil)
 		req := &events.LambdaFunctionURLRequest{
 			Headers: map[string]string{"authorization": "Bearer sess-tok", "x-csrf-token": "csrf-ok"},
 		}

internal/api/mocks_test.go

@@ -78,12 +78,13 @@ func (m *MockPurchaseManager) claimedRowRefusal(ctx context.Context, check purch
 	return nil, nil
 }
 
-func (m *MockPurchaseManager) ApproveExecution(ctx context.Context, execID, token, actor string, check purchase.ClaimedRowCheck) (string, error) {
-	if _, err := m.claimedRowRefusal(ctx, check); err != nil {
-		return "", err
+func (m *MockPurchaseManager) ApproveExecution(ctx context.Context, execID, token, actor string, check purchase.ClaimedRowCheck) (*config.PurchaseExecution, string, error) {
+	if refused, err := m.claimedRowRefusal(ctx, check); err != nil {
+		return refused, "", err
 	}
 	args := m.Called(ctx, execID, token, actor)
-	return args.String(0), args.Error(1)
+	exec, _ := args.Get(0).(*config.PurchaseExecution)
+	return exec, args.String(1), args.Error(2)
 }
 
 func (m *MockPurchaseManager) ApproveAndExecute(ctx context.Context, execID, actor string, transitionedBy *string, check purchase.ClaimedRowCheck) (*config.PurchaseExecution, string, error) {

internal/api/openapi.yaml

@@ -991,7 +991,7 @@ paths:
             The execution cannot be approved: a non-AWS execution without a
             resolvable cloud account, a status other than pending or notified
             (in token mode the status check is the atomic transition, so a
-            concurrent change answers the same way, and in session mode a
+            concurrent change answers the same way, and in either mode a
             lost claim answers 409 as well).
           content:
             application/json:
@@ -1015,9 +1015,9 @@ paths:
                 $ref: '#/components/schemas/Error'
         '502':
           description: >
-            Session mode only. The claim was won and the purchase ran, but it
-            failed or only partly completed (a failed or partially_completed
-            run). Money may have moved; the body details carry execution_id
+            Session and token mode. The claim was won and the purchase ran,
+            but it failed or only partly completed (a failed or
+            partially_completed run). Money may have moved; the body details carry execution_id
             and the row's actual status. A partially_completed run is
             revocable: the executed email with its revocation link is sent as
             for a completed run. Check the execution in History before

internal/api/types.go

@@ -136,15 +136,17 @@ type BreakdownValue struct {
 // ApproveExecution, ApproveAndExecute and RunPlannedPurchaseNow return the
 // raw revocation token minted on a successful execute (issue #103: only its
 // hash is stored, so callers can no longer recover a raw, emailable token by
-// re-reading the execution from the store). Empty on failure.
+// re-reading the execution from the store). Empty on failure. All three
+// also return the row with its final status, also on an execution error, and
+// a nil row when the run never started (issue #532).
 //
 // `check` (every execute entry point) re-runs the caller's permission
 // Constraints against the row the claim CAS returned, the row that is
 // actually bought, before any provider call (issue #415). The handler's own
 // pre-CAS check reads an earlier snapshot, which a repricing or a
 // recommendation attach can change before the CAS.
 type PurchaseManagerInterface interface {
-	ApproveExecution(ctx context.Context, execID, token, actor string, check purchase.ClaimedRowCheck) (string, error)
+	ApproveExecution(ctx context.Context, execID, token, actor string, check purchase.ClaimedRowCheck) (*config.PurchaseExecution, string, error)
 	ApproveAndExecute(ctx context.Context, execID, actor string, transitionedBy *string, check purchase.ClaimedRowCheck) (*config.PurchaseExecution, string, error)
 	// DirectExecute is ApproveAndExecute for the direct-execute permission; it
 	// stamps the executed_* audit fields only after the claim is won.