fix(purchases): align token cancel path with session path and guard in-flight rows (closes #645)

internal/api/handler_purchases.go

@@ -509,7 +509,7 @@ func (h *Handler) cancelPurchaseViaSession(ctx context.Context, req *events.Lamb
 		return nil, err
 	}
 
-	if execution.Status != "pending" && execution.Status != "notified" {
+	if !execution.IsCancelable() {
 		return nil, NewClientError(409, fmt.Sprintf("execution %s cannot be cancelled (status=%s)", execution.ExecutionID, execution.Status))
 	}
 

internal/api/handler_purchases_test.go

@@ -1721,6 +1721,57 @@ func TestHandler_cancelPurchase_Session_RejectsTerminalStatus(t *testing.T) {
 	mockAuth.AssertExpectations(t)
 }
 
+// TestHandler_cancelPurchase_Session_RejectsEachNonCancelableStatus is the
+// session-path companion to the token-path #645 regression guard: every
+// status outside pending/notified must be rejected with a 409 and no write,
+// for parity with purchase.Manager.CancelExecution. The admin session keeps
+// the focus on the status guard (which fires before authorizeSessionCancel)
+// rather than the RBAC matrix, already covered by the matrix tests above.
+func TestHandler_cancelPurchase_Session_RejectsEachNonCancelableStatus(t *testing.T) {
+	rejected := []string{"approved", "running", "paused", "failed", "expired", "completed", "cancelled"}
+	for _, status := range rejected {
+		t.Run(status, func(t *testing.T) {
+			creator := cancelCallerID
+			exec := &config.PurchaseExecution{
+				ExecutionID:     cancelExecID,
+				Status:          status,
+				CreatedByUserID: &creator,
+			}
+			session := &Session{UserID: cancelCallerID, Role: "admin"}
+
+			handler, mockConfig, mockAuth := buildSessionCancelHandler(exec, session, false, false)
+
+			_, err := handler.cancelPurchase(context.Background(), sessionCancelReq(), cancelExecID, "")
+			require.Error(t, err)
+			assert.Contains(t, err.Error(), "cannot be cancelled")
+			assert.Contains(t, err.Error(), status)
+			mockConfig.AssertNotCalled(t, "WithTx")
+			mockConfig.AssertNotCalled(t, "SavePurchaseExecution")
+			mockAuth.AssertExpectations(t)
+		})
+	}
+}
+
+// TestHandler_cancelPurchase_Session_AllowsEachCancelableStatus confirms the
+// inverse: pending and notified rows remain cancelable on the session path,
+// guarding against an over-restriction that would break the dashboard cancel
+// of a row awaiting approval.
+func TestHandler_cancelPurchase_Session_AllowsEachCancelableStatus(t *testing.T) {
+	allowed := []string{"pending", "notified"}
+	for _, status := range allowed {
+		t.Run(status, func(t *testing.T) {
+			creator := cancelCallerID
+			exec := &config.PurchaseExecution{
+				ExecutionID:     cancelExecID,
+				Status:          status,
+				CreatedByUserID: &creator,
+			}
+			session := &Session{UserID: cancelCallerID, Role: "admin", Email: "admin@example.com"}
+			runSessionCancelAllowed(t, exec, session, false, false)
+		})
+	}
+}
+
 func TestHandler_cancelPurchase_Session_LegacyNullCreator_NonAdminRejected(t *testing.T) {
 	// Pre-migration row: created_by_user_id is NULL. cancel-own can't
 	// match a NULL creator, so a non-admin must be rejected. The email

internal/config/types.go

@@ -266,6 +266,18 @@ type PurchaseExecution struct {
 	ApprovalTokenExpiresAt *time.Time `json:"approval_token_expires_at,omitempty" dynamodbav:"approval_token_expires_at,omitempty"`
 }
 
+// IsCancelable reports whether an execution may still be cancelled. Only the
+// pre-purchase states ("pending"/"notified") qualify: once a row reaches
+// "approved" or "running" the AWS commitment is being or has been created, so
+// cancelling would leave the DB and the cloud out of sync; "cancelled",
+// "completed", "failed", "expired", and "paused" are likewise non-cancelable.
+// Both cancel paths (purchase.Manager.CancelExecution on the email-token flow
+// and the session-authed cancelPurchaseViaSession) call this single predicate
+// so the policy can never drift between them (issue #645).
+func (e *PurchaseExecution) IsCancelable() bool {
+	return e.Status == "pending" || e.Status == "notified"
+}
+
 // RecommendationRecord stores a recommendation with purchase status
 type RecommendationRecord struct {
 	ID           string `json:"id" dynamodbav:"id"`

internal/purchase/approvals.go

@@ -188,7 +188,17 @@ func (m *Manager) loadCancelableExecution(ctx context.Context, executionID, toke
 		return nil, fmt.Errorf("approval token has expired")
 	}
 
-	if execution.Status == "completed" || execution.Status == "cancelled" {
+	// Only pending/notified rows are cancelable — shares the single
+	// PurchaseExecution.IsCancelable predicate with the session path in
+	// cancelPurchaseViaSession so the policy can never drift between the two
+	// flows (issue #645). The previous predicate rejected only
+	// completed/cancelled, which let an email-link holder cancel an
+	// approved/running/paused/failed/expired execution that the dashboard
+	// user cannot. Restricting to the pre-purchase states is also the
+	// in-flight guard: approved/running rows are mid-execution (the AWS
+	// commitment is being or has been created), so cancelling them would
+	// leave the DB and the cloud out of sync.
+	if !execution.IsCancelable() {
 		return nil, fmt.Errorf("execution cannot be cancelled, current status: %s", execution.Status)
 	}
 	return execution, nil

internal/purchase/approvals_test.go

@@ -374,6 +374,92 @@ func TestManager_CancelExecution_AlreadyCompleted(t *testing.T) {
 	mockStore.AssertExpectations(t)
 }
 
+// TestManager_CancelExecution_RejectsNonCancelableStatus is the regression
+// guard for issue #645: the token/email cancel path previously rejected only
+// completed/cancelled, so an email-link holder could cancel an
+// approved/running/paused/failed/expired execution that the dashboard
+// (session) path refuses. Each non-pending/notified status must now be
+// rejected with no write to the store — approved/running rows in particular
+// are mid-execution and cancelling them would desync the DB from the cloud.
+func TestManager_CancelExecution_RejectsNonCancelableStatus(t *testing.T) {
+	rejected := []string{"approved", "running", "paused", "failed", "expired", "completed", "cancelled"}
+	for _, status := range rejected {
+		t.Run(status, func(t *testing.T) {
+			ctx := context.Background()
+			mockStore := new(MockConfigStore)
+			mockEmail := new(MockEmailSender)
+
+			execution := &config.PurchaseExecution{
+				ExecutionID:   "exec-123",
+				PlanID:        "plan-456",
+				Status:        status,
+				ApprovalToken: "valid-token",
+			}
+			mockStore.On("GetExecutionByID", ctx, "exec-123").Return(execution, nil)
+
+			manager := &Manager{
+				config:       mockStore,
+				email:        mockEmail,
+				dashboardURL: "https://dashboard.example.com",
+			}
+
+			err := manager.CancelExecution(ctx, "exec-123", "valid-token", status)
+			require.Error(t, err)
+			assert.Contains(t, err.Error(), "execution cannot be cancelled")
+			assert.Contains(t, err.Error(), status)
+			// Status guard must fire before any persistence — a rejected
+			// cancel must not flip the row or drop suppressions.
+			// SavePurchaseExecutionTx forwards to SavePurchaseExecution in
+			// the mock, so this single assertion covers both the tx and
+			// non-tx write paths.
+			mockStore.AssertNotCalled(t, "SavePurchaseExecution", mock.Anything, mock.Anything)
+			mockStore.AssertExpectations(t)
+		})
+	}
+}
+
+// TestManager_CancelExecution_AllowsCancelableStatus confirms the inverse of
+// the #645 guard: genuinely-pending and notified rows are still cancelable on
+// the token path, and the cancel commits (status flip + suppression cleanup)
+// in a single tx. Without this the alignment fix could silently over-restrict
+// and break the legitimate email-link cancel of a row awaiting approval.
+func TestManager_CancelExecution_AllowsCancelableStatus(t *testing.T) {
+	allowed := []string{"pending", "notified"}
+	for _, status := range allowed {
+		t.Run(status, func(t *testing.T) {
+			ctx := context.Background()
+			mockStore := new(MockConfigStore)
+			mockEmail := new(MockEmailSender)
+
+			execution := &config.PurchaseExecution{
+				ExecutionID:   "exec-123",
+				PlanID:        "plan-456",
+				Status:        status,
+				ApprovalToken: "valid-token",
+			}
+			mockStore.On("GetExecutionByID", ctx, "exec-123").Return(execution, nil)
+			var saved *config.PurchaseExecution
+			mockStore.On("SavePurchaseExecution", ctx, mock.AnythingOfType("*config.PurchaseExecution")).
+				Run(func(args mock.Arguments) {
+					saved = args.Get(1).(*config.PurchaseExecution)
+				}).
+				Return(nil)
+
+			manager := &Manager{
+				config:       mockStore,
+				email:        mockEmail,
+				dashboardURL: "https://dashboard.example.com",
+			}
+
+			err := manager.CancelExecution(ctx, "exec-123", "valid-token", "")
+			require.NoError(t, err)
+			require.NotNil(t, saved, "cancel should persist the execution")
+			assert.Equal(t, "cancelled", saved.Status)
+			mockStore.AssertExpectations(t)
+		})
+	}
+}
+
 func TestManager_CancelExecution_NotFound(t *testing.T) {
 	ctx := context.Background()
 	mockStore := new(MockConfigStore)