fix(db): make cancelled->canceled rename truly dual-compatible during EXPAND

Address CodeRabbit review on PR #1277. The expand-contract rename must accept
both spellings/columns throughout the EXPAND phase; only the deferred contract
migration (#1278) removes the legacy spelling.

Down migration (CRITICAL rollback bug + actor loss):
- Reorder the DOWN path so data is converted 'canceled'->'cancelled' BEFORE the
  'cancelled'-only CHECK constraints are re-added. The old order re-added the
  narrowed constraint first, so any row still in the new state violated it and
  the whole rollback failed.
- Backfill canceled_by into cancelled_by BEFORE dropping canceled_by so actor
  attribution recorded by new code during the deploy window survives.

Reads stay dual-compatible:
- handler_history.go: accept both 'canceled' and 'cancelled' in the history
  fetch filter and in the KPI-summary / annotation switches so legacy rows
  written mid-deploy are neither dropped nor allowed to inflate KPIs.
- store_postgres.go: project COALESCE(canceled_by, cancelled_by) on every read
  so a late legacy write landing in cancelled_by is still surfaced.
- handler_purchases.go: the cancel-recovery idempotency check accepts both
  spellings.

Scheduled-cancel routing (doc + predicate reconciliation):
- interfaces.go: align the CancelExecutionAtomic doc to the implementation
  (pending/notified only; scheduled is handled by CancelScheduledExecutionAtomic).
- types.go: add IsImmediatelyCancelable (pending/notified) distinct from the
  broader IsCancelable (adds scheduled). The /cancel paths
  (cancelPurchaseViaSession, loadCancelableExecution) now gate on the narrow
  predicate so a scheduled row is routed to a clear "use the revoke endpoint"
  409 instead of misrouting into the pending/notified-only CAS and surfacing a
  misleading "concurrent operation" error.

Tests:
- New integration test 000078_..._test.go exercises up (both spellings accepted,
  legacy backfilled) and down (rollback succeeds with 'canceled' rows present,
  data converted back, actor attribution preserved, narrowed constraint restored).
- Unit tests for the cancelable predicates and the scheduled-routing behaviour
  on both cancel paths.

Author: cristim

internal/api/handler_history.go

@@ -114,7 +114,12 @@ func (h *Handler) getHistory(ctx context.Context, req *events.LambdaFunctionURLR
 // wave-2) appear in the History view with a Revoke button before the cloud SDK
 // call fires. Without this entry the row is invisible to the History UI, making
 // the Revoke button unreachable (issue #290, second-wave CR Finding E).
-var historyExecutionStatuses = []string{"pending", "notified", "scheduled", "approved", "running", "paused", "completed", "partially_completed", "failed", "expired", "canceled"}
+// Both "canceled" (US, current) and "cancelled" (British, legacy) are listed:
+// during the expand-contract rename (migration 000078) old code may still write
+// "cancelled" before the rolling deploy completes. The contract migration
+// (#1278) removes the legacy spelling once the deploy is verified stable; drop
+// "cancelled" here at the same time.
+var historyExecutionStatuses = []string{"pending", "notified", "scheduled", "approved", "running", "paused", "completed", "partially_completed", "failed", "expired", "canceled", "cancelled"}
 
 // approvalExpiryWindow is how long a pending approval stays actionable
 // before the History view flips it to "expired". Aligns with the
@@ -345,7 +350,10 @@ func annotateHistoryRowByStatus(row *config.PurchaseHistoryRecord, exec config.P
 		row.StatusDescription = exec.Error
 	case "expired":
 		row.StatusDescription = "approval link expired (not approved within 7 days)"
-	case "canceled":
+	case "canceled", "cancelled":
+		// "cancelled" (legacy British spelling) is still matched during the
+		// expand-contract rename (migration 000078) until the contract
+		// migration (#1278) backfills and drops it.
 		annotateCancelled(row, exec, approver)
 	default:
 		// In-flight (approved/running/scheduled/paused) and audit-gap
@@ -1022,10 +1030,14 @@ func summarizePurchaseHistory(purchases []config.PurchaseHistoryRecord) HistoryS
 		case "expired":
 			summary.TotalExpired++
 			continue
-		case "canceled":
+		case "canceled", "cancelled":
 			// A canceled purchase represents zero committed spend and zero
 			// realized savings (issue #736). Exclude from all dollar KPIs and
 			// from TotalCompleted -- the money was never committed.
+			// "cancelled" (legacy British spelling) is matched alongside
+			// "canceled" during the expand-contract rename (migration 000078)
+			// so legacy rows can't inflate KPIs mid-deploy; the contract
+			// migration (#1278) drops it once the deploy is stable.
 			continue
 		}
 		summary.TotalCompleted++

internal/api/handler_purchases.go

@@ -387,7 +387,11 @@ func (h *Handler) cancelOrRecoverExecution(ctx context.Context, executionID stri
 	if existing == nil {
 		return nil, NewClientError(404, fmt.Sprintf("execution %s not found", executionID))
 	}
-	if existing.Status != "canceled" {
+	// Accept both spellings: during the expand-contract rename (migration
+	// 000078) a concurrent legacy cancel may have written "cancelled" before
+	// the rolling deploy completes. The contract migration (#1278) backfills
+	// and drops the legacy spelling, after which "cancelled" can be removed.
+	if existing.Status != "canceled" && existing.Status != "cancelled" {
 		return nil, NewClientError(409, fmt.Sprintf(
 			"execution %s cannot be canceled (status=%s)",
 			executionID, existing.Status))
@@ -897,6 +901,24 @@ func (h *Handler) cancelPurchase(ctx context.Context, req *events.LambdaFunction
 	return h.cancelPurchaseViaSession(ctx, req, execution)
 }
 
+// guardImmediatelyCancelable returns a 409 ClientError unless the execution is
+// in a state the plain cancel CAS (CancelExecutionAtomic, pending/notified)
+// handles, and nil when it is cancelable. A "scheduled" row is cancelable but
+// only via the /revoke flow (CancelScheduledExecutionAtomic); routing it
+// through the cancel path would pass IsCancelable, fail the pending/notified-only
+// CAS, and surface a misleading "concurrent operation" 409 -- so it gets a
+// clear message directing it to the revoke endpoint instead (issue #290).
+// Shared by the session and email-token cancel gates so the policy can't drift.
+func guardImmediatelyCancelable(execution *config.PurchaseExecution) error {
+	if execution.Status == "scheduled" {
+		return NewClientError(409, fmt.Sprintf("execution %s is scheduled; use the revoke endpoint to cancel it before it fires", execution.ExecutionID))
+	}
+	if !execution.IsImmediatelyCancelable() {
+		return NewClientError(409, fmt.Sprintf("execution %s cannot be canceled (status=%s)", execution.ExecutionID, execution.Status))
+	}
+	return nil
+}
+
 // cancelPurchaseViaSession is the session-authed branch of cancelPurchase.
 // Enforces the cancel-any/cancel-own RBAC matrix, validates the execution
 // is in a cancelable state (pending|notified), atomically flips the row
@@ -924,8 +946,8 @@ func (h *Handler) cancelPurchaseViaSession(ctx context.Context, req *events.Lamb
 		return nil, err
 	}
 
-	if !execution.IsCancelable() {
-		return nil, NewClientError(409, fmt.Sprintf("execution %s cannot be canceled (status=%s)", execution.ExecutionID, execution.Status))
+	if err := guardImmediatelyCancelable(execution); err != nil {
+		return nil, err
 	}
 
 	if err := h.authorizeSessionCancel(ctx, session, execution); err != nil {

internal/api/handler_purchases_test.go

@@ -2420,6 +2420,39 @@ func TestHandler_cancelPurchase_Session_RejectsEachNonCancelableStatus(t *testin
 	}
 }
 
+// TestHandler_cancelPurchase_Session_ScheduledRoutedToRevoke is the regression
+// guard for CodeRabbit finding #6 on PR #1277. A "scheduled" row is cancelable
+// (IsCancelable returns true) but ONLY via the /revoke flow
+// (CancelScheduledExecutionAtomic). The /cancel path must NOT pass it to
+// CancelExecutionAtomic (pending/notified-only CAS) -- doing so would fail the
+// CAS and surface a misleading "concurrent operation already transitioned it"
+// 409. Instead the handler returns a clear 409 directing the caller to the
+// revoke endpoint, and never enters the cancel tx.
+func TestHandler_cancelPurchase_Session_ScheduledRoutedToRevoke(t *testing.T) {
+	creator := cancelCallerID
+	exec := &config.PurchaseExecution{
+		ExecutionID:     cancelExecID,
+		Status:          "scheduled",
+		CreatedByUserID: &creator,
+	}
+	session := &Session{UserID: cancelCallerID, Email: "admin@example.com"}
+
+	// Caller owns the row; cancel-own would authorise it if status allowed.
+	handler, mockConfig, mockAuth := buildSessionCancelHandler(exec, session, false, true)
+
+	_, err := handler.cancelPurchase(context.Background(), sessionCancelReq(), cancelExecID, "")
+	require.Error(t, err)
+	ce, ok := IsClientError(err)
+	require.True(t, ok, "scheduled-on-cancel must be a ClientError")
+	assert.Equal(t, 409, ce.code)
+	assert.Contains(t, err.Error(), "scheduled")
+	assert.Contains(t, err.Error(), "revoke", "error must direct the caller to the revoke endpoint")
+	// Must NOT misroute through the pending/notified-only CAS.
+	mockConfig.AssertNotCalled(t, "CancelExecutionAtomic", mock.Anything, mock.Anything, mock.Anything, mock.Anything)
+	mockConfig.AssertNotCalled(t, "WithTx")
+	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

internal/config/interfaces.go

@@ -82,9 +82,12 @@ type StoreInterface interface {
 	// When non-nil the actor is stamped onto transitioned_by + transitioned_at; when nil,
 	// transitioned_by is set to NULL and transitioned_at is still set to NOW() for ordering.
 	TransitionExecutionStatus(ctx context.Context, executionID string, fromStatuses []string, toStatus string, actor *string) (*PurchaseExecution, error)
-	// CancelExecutionAtomic atomically flips status from pending / notified /
-	// scheduled to canceled, setting canceled_by. The 'scheduled' status
-	// supports the Gmail-style pre-fire delay revoke path (issue #290).
+	// CancelExecutionAtomic atomically flips status from pending / notified to
+	// canceled, setting canceled_by. The 'scheduled' status is intentionally
+	// NOT accepted here -- the Gmail-style pre-fire delay revoke path (issue
+	// #290) has its own CancelScheduledExecutionAtomic so the two flows surface
+	// distinct CAS race outcomes (this path returns 409 on already-approved;
+	// scheduled-revoke returns 410 "window closed" on already-fired).
 	// Returns (true, "canceled", nil) on success and (false, currentStatus,
 	// nil) when zero rows were affected (the execution had already been
 	// approved or otherwise transitioned). Must be called inside a WithTx

internal/config/store_postgres.go

@@ -906,7 +906,7 @@ func (s *PostgresStore) TransitionExecutionStatus(ctx context.Context, execution
 		RETURNING plan_id, execution_id, status, step_number, scheduled_date,
 		          notification_sent, approval_token, recommendations,
 		          total_upfront_cost, estimated_savings, completed_at, error, expires_at,
-		          cloud_account_id, source, approved_by, canceled_by, capacity_percent,
+		          cloud_account_id, source, approved_by, COALESCE(canceled_by, cancelled_by) AS canceled_by, capacity_percent,
 		          created_by_user_id, retry_execution_id, retry_attempt_n,
 		          approval_token_expires_at,
 		          executed_by_user_id, executed_at, pre_approval_skip_reason,
@@ -1077,7 +1077,7 @@ func (s *PostgresStore) GetExecutionsByStatuses(ctx context.Context, statuses []
 		SELECT plan_id, execution_id, status, step_number, scheduled_date,
 		       notification_sent, approval_token, recommendations,
 		       total_upfront_cost, estimated_savings, completed_at, error, expires_at,
-		       cloud_account_id, source, approved_by, canceled_by, capacity_percent,
+		       cloud_account_id, source, approved_by, COALESCE(canceled_by, cancelled_by) AS canceled_by, capacity_percent,
 		       created_by_user_id, retry_execution_id, retry_attempt_n,
 		       approval_token_expires_at,
 		       executed_by_user_id, executed_at, pre_approval_skip_reason,
@@ -1118,7 +1118,7 @@ func (s *PostgresStore) GetPlannedExecutions(ctx context.Context, statuses []str
 		SELECT plan_id, execution_id, status, step_number, scheduled_date,
 		       notification_sent, approval_token, recommendations,
 		       total_upfront_cost, estimated_savings, completed_at, error, expires_at,
-		       cloud_account_id, source, approved_by, canceled_by, capacity_percent,
+		       cloud_account_id, source, approved_by, COALESCE(canceled_by, cancelled_by) AS canceled_by, capacity_percent,
 		       created_by_user_id, retry_execution_id, retry_attempt_n,
 		       approval_token_expires_at,
 		       executed_by_user_id, executed_at, pre_approval_skip_reason,
@@ -1143,7 +1143,7 @@ func (s *PostgresStore) GetStaleApprovedExecutions(ctx context.Context, olderTha
 		SELECT plan_id, execution_id, status, step_number, scheduled_date,
 		       notification_sent, approval_token, recommendations,
 		       total_upfront_cost, estimated_savings, completed_at, error, expires_at,
-		       cloud_account_id, source, approved_by, canceled_by, capacity_percent,
+		       cloud_account_id, source, approved_by, COALESCE(canceled_by, cancelled_by) AS canceled_by, capacity_percent,
 		       created_by_user_id, retry_execution_id, retry_attempt_n,
 		       approval_token_expires_at,
 		       executed_by_user_id, executed_at, pre_approval_skip_reason,
@@ -1185,7 +1185,7 @@ func (s *PostgresStore) ListStuckExecutions(ctx context.Context, statuses []stri
 		SELECT plan_id, execution_id, status, step_number, scheduled_date,
 		       notification_sent, approval_token, recommendations,
 		       total_upfront_cost, estimated_savings, completed_at, error, expires_at,
-		       cloud_account_id, source, approved_by, canceled_by, capacity_percent,
+		       cloud_account_id, source, approved_by, COALESCE(canceled_by, cancelled_by) AS canceled_by, capacity_percent,
 		       created_by_user_id, retry_execution_id, retry_attempt_n,
 		       approval_token_expires_at,
 		       executed_by_user_id, executed_at, pre_approval_skip_reason,
@@ -1206,7 +1206,7 @@ func (s *PostgresStore) GetPendingExecutions(ctx context.Context) ([]PurchaseExe
 		SELECT plan_id, execution_id, status, step_number, scheduled_date,
 		       notification_sent, approval_token, recommendations,
 		       total_upfront_cost, estimated_savings, completed_at, error, expires_at,
-		       cloud_account_id, source, approved_by, canceled_by, capacity_percent,
+		       cloud_account_id, source, approved_by, COALESCE(canceled_by, cancelled_by) AS canceled_by, capacity_percent,
 		       created_by_user_id, retry_execution_id, retry_attempt_n,
 		       approval_token_expires_at,
 		       executed_by_user_id, executed_at, pre_approval_skip_reason,
@@ -1230,7 +1230,7 @@ func (s *PostgresStore) GetPendingExecutionsTx(ctx context.Context, tx pgx.Tx) (
 		SELECT plan_id, execution_id, status, step_number, scheduled_date,
 		       notification_sent, approval_token, recommendations,
 		       total_upfront_cost, estimated_savings, completed_at, error, expires_at,
-		       cloud_account_id, source, approved_by, canceled_by, capacity_percent,
+		       cloud_account_id, source, approved_by, COALESCE(canceled_by, cancelled_by) AS canceled_by, capacity_percent,
 		       created_by_user_id, retry_execution_id, retry_attempt_n,
 		       approval_token_expires_at,
 		       executed_by_user_id, executed_at, pre_approval_skip_reason,
@@ -1260,7 +1260,7 @@ func (s *PostgresStore) GetExecutionByID(ctx context.Context, executionID string
 		SELECT plan_id, execution_id, status, step_number, scheduled_date,
 		       notification_sent, approval_token, recommendations,
 		       total_upfront_cost, estimated_savings, completed_at, error, expires_at,
-		       cloud_account_id, source, approved_by, canceled_by, capacity_percent,
+		       cloud_account_id, source, approved_by, COALESCE(canceled_by, cancelled_by) AS canceled_by, capacity_percent,
 		       created_by_user_id, retry_execution_id, retry_attempt_n,
 		       approval_token_expires_at,
 		       executed_by_user_id, executed_at, pre_approval_skip_reason,
@@ -1287,7 +1287,7 @@ func (s *PostgresStore) GetExecutionByPlanAndDate(ctx context.Context, planID st
 		SELECT plan_id, execution_id, status, step_number, scheduled_date,
 		       notification_sent, approval_token, recommendations,
 		       total_upfront_cost, estimated_savings, completed_at, error, expires_at,
-		       cloud_account_id, source, approved_by, canceled_by, capacity_percent,
+		       cloud_account_id, source, approved_by, COALESCE(canceled_by, cancelled_by) AS canceled_by, capacity_percent,
 		       created_by_user_id, retry_execution_id, retry_attempt_n,
 		       approval_token_expires_at,
 		       executed_by_user_id, executed_at, pre_approval_skip_reason,
@@ -1471,7 +1471,7 @@ func (s *PostgresStore) GetScheduledExecutionsDue(ctx context.Context) ([]Purcha
 		SELECT plan_id, execution_id, status, step_number, scheduled_date,
 		       notification_sent, approval_token, recommendations,
 		       total_upfront_cost, estimated_savings, completed_at, error, expires_at,
-		       cloud_account_id, source, approved_by, canceled_by, capacity_percent,
+		       cloud_account_id, source, approved_by, COALESCE(canceled_by, cancelled_by) AS canceled_by, capacity_percent,
 		       created_by_user_id, retry_execution_id, retry_attempt_n,
 		       approval_token_expires_at,
 		       executed_by_user_id, executed_at, pre_approval_skip_reason,

internal/config/types.go

@@ -362,6 +362,20 @@ func (e *PurchaseExecution) IsCancelable() bool {
 	return e.Status == "pending" || e.Status == "notified" || e.Status == "scheduled"
 }
 
+// IsImmediatelyCancelable reports whether an execution can be canceled by the
+// plain CancelExecutionAtomic CAS (WHERE status IN ('pending','notified')),
+// i.e. via the /cancel endpoints. The "scheduled" state is deliberately
+// excluded: it is cancelable (IsCancelable returns true) but only through the
+// Gmail-style pre-fire revoke flow (CancelScheduledExecutionAtomic, the
+// /revoke endpoint), which surfaces a distinct 410 "window closed" CAS race
+// outcome. Gating the /cancel paths on this predicate -- rather than the
+// broader IsCancelable -- prevents a "scheduled" row from passing the gate,
+// failing the pending/notified-only CAS, and surfacing the misleading
+// "concurrent operation already transitioned it" error (issue #290).
+func (e *PurchaseExecution) IsImmediatelyCancelable() 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/config/types_test.go

@@ -401,3 +401,38 @@ func TestGlobalConfig_GracePeriodFor(t *testing.T) {
 		assert.Equal(t, DefaultGracePeriodDays, cfg.GracePeriodFor("gcp"))
 	})
 }
+
+// TestPurchaseExecution_CancelablePredicates locks the distinction between the
+// two cancelability predicates (CodeRabbit finding #6, PR #1277):
+//   - IsCancelable is the broad policy predicate: pending/notified/scheduled.
+//   - IsImmediatelyCancelable is the narrow CAS predicate the /cancel paths
+//     gate on: pending/notified only. "scheduled" is cancelable but only via
+//     the Gmail-style revoke flow (CancelScheduledExecutionAtomic), so it must
+//     NOT be immediately cancelable -- otherwise it misroutes into the
+//     pending/notified-only CAS and surfaces a misleading "concurrent
+//     operation" error.
+func TestPurchaseExecution_CancelablePredicates(t *testing.T) {
+	cases := []struct {
+		status                string
+		cancelable            bool
+		immediatelyCancelable bool
+	}{
+		{"pending", true, true},
+		{"notified", true, true},
+		{"scheduled", true, false},
+		{"approved", false, false},
+		{"running", false, false},
+		{"paused", false, false},
+		{"completed", false, false},
+		{"canceled", false, false},
+		{"failed", false, false},
+		{"expired", false, false},
+	}
+	for _, tc := range cases {
+		t.Run(tc.status, func(t *testing.T) {
+			e := &PurchaseExecution{Status: tc.status}
+			assert.Equal(t, tc.cancelable, e.IsCancelable(), "IsCancelable(%q)", tc.status)
+			assert.Equal(t, tc.immediatelyCancelable, e.IsImmediatelyCancelable(), "IsImmediatelyCancelable(%q)", tc.status)
+		})
+	}
+}