fix(db): write canonical 'canceled' on all cancel paths + cleanup filter (follow-up to #1277)

internal/api/coverage_extras_test.go

@@ -331,8 +331,9 @@ func TestHandler_rejectRIExchange_AlreadyProcessed(t *testing.T) {
 	mockStore := new(MockConfigStore)
 	mockStore.On("GetRIExchangeRecord", ctx, "11111111-1111-1111-1111-111111111111").Return(
 		&config.RIExchangeRecord{ID: "11111111-1111-1111-1111-111111111111", ApprovalToken: "tok"}, nil)
-	// Transition returns nil indicating already processed
-	mockStore.On("TransitionRIExchangeStatus", ctx, "11111111-1111-1111-1111-111111111111", "pending", "cancelled", mock.Anything).
+	// Transition returns nil indicating already processed.
+	// Canonical spelling used (#1277 follow-up).
+	mockStore.On("TransitionRIExchangeStatus", ctx, "11111111-1111-1111-1111-111111111111", "pending", config.StatusCanceled, mock.Anything).
 		Return(nil, nil)
 
 	h := &Handler{config: mockStore}

internal/api/handler_ri_exchange.go

@@ -1369,15 +1369,15 @@ func (h *Handler) rejectRIExchange(ctx context.Context, id, token string) (any,
 	}
 
 	// Token-based rejection: no session user, so transitioned_by = NULL.
-	transitioned, err := h.config.TransitionRIExchangeStatus(ctx, id, "pending", "cancelled", nil)
+	transitioned, err := h.config.TransitionRIExchangeStatus(ctx, id, "pending", config.StatusCanceled, nil)
 	if err != nil {
 		return nil, fmt.Errorf("failed to transition exchange status: %w", err)
 	}
 	if transitioned == nil {
-		return nil, NewClientError(409, "exchange already processed, expired, or was cancelled")
+		return nil, NewClientError(409, "exchange already processed, expired, or was canceled")
 	}
 
-	return map[string]string{"status": "cancelled"}, nil
+	return map[string]string{"status": config.StatusCanceled}, nil
 }
 
 // RIExchangeConfigResponse is the response for GET /api/ri-exchange/config.

internal/api/handler_ri_exchange_test.go

@@ -262,8 +262,9 @@ func TestRejectRIExchange_AlreadyCompleted(t *testing.T) {
 		ExchangeID:    "exch-already-done",
 	}, nil)
 
-	// Transition from pending→canceled fails (record is not pending)
-	mockStore.On("TransitionRIExchangeStatus", ctx, id, "pending", "cancelled", mock.Anything).
+	// Transition from pending->canceled fails (record is not pending).
+	// Uses canonical spelling; see config.StatusCanceled.
+	mockStore.On("TransitionRIExchangeStatus", ctx, id, "pending", config.StatusCanceled, mock.Anything).
 		Return((*config.RIExchangeRecord)(nil), nil)
 
 	_, err := h.rejectRIExchange(ctx, id, token)
@@ -1577,10 +1578,10 @@ func TestRejectRIExchange_TokenPathActorIsNil(t *testing.T) {
 	mockStore.On("GetRIExchangeRecord", ctx, id).Return(&config.RIExchangeRecord{
 		ID: id, Status: "pending", ApprovalToken: "tok",
 	}, nil)
-	// Token path: actor must be nil.
-	mockStore.On("TransitionRIExchangeStatus", ctx, id, "pending", "cancelled",
+	// Token path: actor must be nil. Canonical spelling used (#1277 follow-up).
+	mockStore.On("TransitionRIExchangeStatus", ctx, id, "pending", config.StatusCanceled,
 		(*string)(nil),
-	).Return(&config.RIExchangeRecord{ID: id, Status: "cancelled"}, nil)
+	).Return(&config.RIExchangeRecord{ID: id, Status: config.StatusCanceled}, nil)
 
 	_, err := (&Handler{config: mockStore}).rejectRIExchange(ctx, id, "tok")
 	require.NoError(t, err)

internal/api/router_handlers_test.go

@@ -784,18 +784,18 @@ func TestHandler_rejectRIExchange_ValidTokenAndRecord(t *testing.T) {
 			ApprovalToken: "tok",
 			Status:        "pending",
 		}, nil)
-	// rejectRIExchange transitions to "cancelled" (UK spelling, matches DB canonical)
-	mockStore.On("TransitionRIExchangeStatus", ctx, "11111111-1111-1111-1111-111111111111", "pending", "cancelled", mock.Anything).
+	// rejectRIExchange transitions to "canceled" (US canonical spelling, #1277 follow-up).
+	mockStore.On("TransitionRIExchangeStatus", ctx, "11111111-1111-1111-1111-111111111111", "pending", config.StatusCanceled, mock.Anything).
 		Return(&config.RIExchangeRecord{
 			ID:     "11111111-1111-1111-1111-111111111111",
-			Status: "cancelled",
+			Status: config.StatusCanceled,
 		}, nil)
 
 	h := &Handler{config: mockStore}
 	result, err := h.rejectRIExchange(ctx, "11111111-1111-1111-1111-111111111111", "tok")
 	require.NoError(t, err)
 	m := result.(map[string]string)
-	assert.Equal(t, "cancelled", m["status"])
+	assert.Equal(t, config.StatusCanceled, m["status"])
 }
 
 // ---------------------------------------------------------------------------

internal/config/store_postgres.go

@@ -1044,12 +1044,13 @@ func (s *PostgresStore) SetCancelledBy(ctx context.Context, executionID, cancell
 }
 
 // CancelExecutionAtomic atomically transitions an execution from
-// pending or notified to 'cancelled', setting cancelled_by to the supplied
-// actor (NULL when actor is nil). The UPDATE is conditional on
+// pending or notified to 'canceled' (canonical US spelling), setting
+// canceled_by to the supplied actor (NULL when actor is nil). The UPDATE
+// is conditional on
 // status IN ('pending','notified') so a concurrent approve that has
 // already transitioned the row to 'approved' causes zero rows to be
 // affected and the method returns (false, currentStatus, nil) with the
-// live status fetched via a follow-up SELECT. Returns (true, "cancelled",
+// live status fetched via a follow-up SELECT. Returns (true, "canceled",
 // nil) on success and (false, "", err) on a real DB error.
 //
 // The 'scheduled' status is intentionally NOT accepted here -- the
@@ -1064,16 +1065,17 @@ func (s *PostgresStore) SetCancelledBy(ctx context.Context, executionID, cancell
 // status guard is inside the UPDATE rather than checked optimistically
 // before entering the tx.
 func (s *PostgresStore) CancelExecutionAtomic(ctx context.Context, tx pgx.Tx, executionID string, cancelledBy *string) (canceled bool, currentStatus string, err error) {
+	// StatusCanceled is the canonical US-spelling value; see types.go.
 	q := `
 		UPDATE purchase_executions
-		   SET status       = 'cancelled',
-		       cancelled_by = $2,
+		   SET status       = $3,
+		       canceled_by  = $2,
 		       updated_at   = NOW()
 		 WHERE execution_id = $1
 		   AND status IN ('pending', 'notified')
 		RETURNING status
 	`
-	rows, err := tx.Query(ctx, q, executionID, cancelledBy)
+	rows, err := tx.Query(ctx, q, executionID, cancelledBy, StatusCanceled)
 	if err != nil {
 		return false, "", fmt.Errorf("failed to cancel execution: %w", err)
 	}
@@ -1104,11 +1106,11 @@ func (s *PostgresStore) CancelExecutionAtomic(ctx context.Context, tx pgx.Tx, ex
 }
 
 // CancelScheduledExecutionAtomic atomically transitions an execution from
-// 'scheduled' to 'cancelled', setting cancelled_by to the supplied actor
-// (NULL when actor is nil). Used by the Gmail-style pre-fire delay revoke
-// path (issue #290 / #291 wave-2): an approved-but-not-yet-fired execution
-// can be revoked at $0 by flipping it to cancelled before the scheduler
-// fires the cloud SDK call.
+// 'scheduled' to 'canceled' (canonical US spelling), setting canceled_by to
+// the supplied actor (NULL when actor is nil). Used by the Gmail-style
+// pre-fire delay revoke path (issue #290 / #291 wave-2): an
+// approved-but-not-yet-fired execution can be revoked at $0 by flipping it
+// to canceled before the scheduler fires the cloud SDK call.
 //
 // The 'scheduled' status is the only accepted source. A concurrent
 // scheduler tick that already transitioned the row to 'approved' or
@@ -1117,20 +1119,21 @@ func (s *PostgresStore) CancelExecutionAtomic(ctx context.Context, tx pgx.Tx, ex
 // ("revocation window has closed") so the frontend can fall through to
 // the post-execution Azure direct-cancel API path.
 //
-// Returns (true, "cancelled", nil) on success and (false, "", err) on a
+// Returns (true, StatusCanceled, nil) on success and (false, "", err) on a
 // real DB error. Must be called inside a WithTx block so the suppression
 // cleanup commits atomically with the status flip.
 func (s *PostgresStore) CancelScheduledExecutionAtomic(ctx context.Context, tx pgx.Tx, executionID string, cancelledBy *string) (canceled bool, currentStatus string, err error) {
+	// StatusCanceled is the canonical US-spelling value; see types.go.
 	q := `
 		UPDATE purchase_executions
-		   SET status       = 'cancelled',
-		       cancelled_by = $2,
+		   SET status       = $3,
+		       canceled_by  = $2,
 		       updated_at   = NOW()
 		 WHERE execution_id = $1
 		   AND status = 'scheduled'
 		RETURNING status
 	`
-	rows, err := tx.Query(ctx, q, executionID, cancelledBy)
+	rows, err := tx.Query(ctx, q, executionID, cancelledBy, StatusCanceled)
 	if err != nil {
 		return false, "", fmt.Errorf("failed to cancel scheduled execution: %w", err)
 	}
@@ -1598,9 +1601,11 @@ func (s *PostgresStore) GetScheduledExecutionsDue(ctx context.Context) ([]Purcha
 // Two independent cleanup branches, each with its own retention window so
 // that a row far in one dimension doesn't block cleanup in the other:
 //
-//  1. Terminal-state cleanup: `status IN ('completed', 'cancelled') AND
-//     scheduled_date < NOW() - retention`. Keeps recent completions
-//     visible in the UI for at least `retention` days before purging.
+//  1. Terminal-state cleanup: `status IN ('completed', 'cancelled', 'canceled') AND
+//     scheduled_date < NOW() - retention`. Both spellings are included to
+//     cover legacy rows ('cancelled') and new code rows ('canceled', canonical
+//     US spelling per StatusCanceled / migration 000089). Keeps recent
+//     completions visible in the UI for at least `retention` days before purging.
 //
 //  2. Expired-execution cleanup: `expires_at IS NOT NULL AND expires_at <
 //     NOW() - retention`. A row whose approval token has been expired
@@ -1620,10 +1625,14 @@ func (s *PostgresStore) GetScheduledExecutionsDue(ctx context.Context) ([]Purcha
 // NULL `expires_at` is excluded from branch 2 so rows that never had an
 // expiration deadline are safe from expiry-based cleanup.
 func (s *PostgresStore) CleanupOldExecutions(ctx context.Context, retentionDays int) (int64, error) {
+	// Both spellings are included: 'cancelled' (legacy, pre-migration 000089
+	// rows) and 'canceled' (canonical, written by new code after the
+	// follow-up to #1277). #1278 will drop 'cancelled' once all rows are
+	// normalized.
 	query := `
 		DELETE FROM purchase_executions
 		WHERE (
-		        status IN ('completed', 'cancelled')
+		        status IN ('completed', 'cancelled', 'canceled')
 		    AND scheduled_date < NOW() - INTERVAL '1 day' * $1
 		      )
 		   OR (
@@ -2585,11 +2594,11 @@ func (s *PostgresStore) GetRIExchangeDailySpend(ctx context.Context, date time.T
 func (s *PostgresStore) CancelAllPendingExchanges(ctx context.Context) (int64, error) {
 	query := `
 		UPDATE ri_exchange_history
-		SET status = 'cancelled'
+		SET status = $1, updated_at = NOW()
 		WHERE status = 'pending'
 	`
 
-	result, err := s.db.Exec(ctx, query)
+	result, err := s.db.Exec(ctx, query, StatusCanceled)
 	if err != nil {
 		return 0, fmt.Errorf("failed to cancel pending exchanges: %w", err)
 	}
@@ -2622,20 +2631,20 @@ func (s *PostgresStore) CancelPendingExchangesByOrigin(ctx context.Context, orig
 	case common.ExchangeOriginLadder:
 		query = `
 			UPDATE ri_exchange_history
-			SET status = 'cancelled', updated_at = NOW()
+			SET status = $1, updated_at = NOW()
 			WHERE status = 'pending'
 			  AND ladder_run_id IS NOT NULL
 		`
 	default: // common.ExchangeOriginStandalone (validated non-unknown above)
 		query = `
 			UPDATE ri_exchange_history
-			SET status = 'cancelled', updated_at = NOW()
+			SET status = $1, updated_at = NOW()
 			WHERE status = 'pending'
 			  AND ladder_run_id IS NULL
 		`
 	}
 
-	result, err := s.db.Exec(ctx, query)
+	result, err := s.db.Exec(ctx, query, StatusCanceled)
 	if err != nil {
 		return 0, fmt.Errorf("failed to cancel pending exchanges by origin %q: %w", origin, err)
 	}

internal/config/store_postgres_pgxmock_test.go

@@ -1968,7 +1968,8 @@ func TestPGXMock_CancelAllPendingExchanges_Success(t *testing.T) {
 	store := storeWith(mock)
 	ctx := context.Background()
 
-	mock.ExpectExec("UPDATE").WillReturnResult(pgxmock.NewResult("UPDATE", 3))
+	// Now parameterized: $1 = status value.
+	mock.ExpectExec("UPDATE").WithArgs(pgxmock.AnyArg()).WillReturnResult(pgxmock.NewResult("UPDATE", 3))
 
 	n, err := store.CancelAllPendingExchanges(ctx)
 	require.NoError(t, err)
@@ -1987,7 +1988,8 @@ func TestPGXMock_CancelPendingExchangesByOrigin_Standalone(t *testing.T) {
 	ctx := context.Background()
 
 	// Standalone origin must target WHERE ladder_run_id IS NULL only.
-	mock.ExpectExec("ladder_run_id IS NULL").WillReturnResult(pgxmock.NewResult("UPDATE", 2))
+	// Now parameterized: $1 = status value.
+	mock.ExpectExec("ladder_run_id IS NULL").WithArgs(pgxmock.AnyArg()).WillReturnResult(pgxmock.NewResult("UPDATE", 2))
 
 	n, err := store.CancelPendingExchangesByOrigin(ctx, common.ExchangeOriginStandalone)
 	require.NoError(t, err)
@@ -2001,7 +2003,8 @@ func TestPGXMock_CancelPendingExchangesByOrigin_Ladder(t *testing.T) {
 	ctx := context.Background()
 
 	// Ladder origin must target WHERE ladder_run_id IS NOT NULL only.
-	mock.ExpectExec("ladder_run_id IS NOT NULL").WillReturnResult(pgxmock.NewResult("UPDATE", 1))
+	// Now parameterized: $1 = status value.
+	mock.ExpectExec("ladder_run_id IS NOT NULL").WithArgs(pgxmock.AnyArg()).WillReturnResult(pgxmock.NewResult("UPDATE", 1))
 
 	n, err := store.CancelPendingExchangesByOrigin(ctx, common.ExchangeOriginLadder)
 	require.NoError(t, err)
@@ -2521,3 +2524,136 @@ func TestPGXMock_GetExecutionByPlanAndDate_NotFoundWrapsErrNotFound(t *testing.T
 		"zero-row GetExecutionByPlanAndDate must wrap ErrNotFound so getOrCreateExecution can create a new execution; got: %v", err)
 	assert.NoError(t, mock.ExpectationsWereMet())
 }
+
+// ─── Canonical-cancel write regression tests (#1277 follow-up) ───────────────
+//
+// These tests assert that every cancel write path emits the canonical US
+// spelling 'canceled' (StatusCanceled) rather than the legacy 'cancelled'.
+// They fail on the pre-fix code (which hardcoded 'cancelled') and pass only
+// when the parameterized $N arg receives StatusCanceled = "canceled".
+
+// TestPGXMock_CancelExecutionAtomic_WritesCanonicalStatus verifies that
+// CancelExecutionAtomic sends StatusCanceled ("canceled") as the status
+// parameter and sets canceled_by (not cancelled_by).
+// Regression guard for the #1277 follow-up fix.
+func TestPGXMock_CancelExecutionAtomic_WritesCanonicalStatus(t *testing.T) {
+	mock := newMock(t)
+	store := storeWith(mock)
+	ctx := context.Background()
+
+	actor := "user@example.com"
+
+	// The mock uses regexp matching. We pin on:
+	//   - "canceled_by" to confirm the new column is targeted (not cancelled_by)
+	//   - WithArgs: $1=execID, $2=&actor, $3=StatusCanceled
+	// The RETURNING row also carries "canceled" so the caller's returned
+	// status is the canonical spelling end-to-end.
+	rows := pgxmock.NewRows([]string{"status"}).AddRow(StatusCanceled)
+	mock.ExpectQuery("canceled_by").
+		WithArgs("exec-1", &actor, StatusCanceled).
+		WillReturnRows(rows)
+
+	// Pass mock directly as pgx.Tx (PgxPoolIface embeds pgx.Tx).
+	ok, status, err := store.CancelExecutionAtomic(ctx, mock, "exec-1", &actor)
+	require.NoError(t, err)
+	assert.True(t, ok, "row should have been updated")
+	assert.Equal(t, StatusCanceled, status, "returned status must be canonical 'canceled'")
+	assert.NoError(t, mock.ExpectationsWereMet())
+}
+
+// TestPGXMock_CancelScheduledExecutionAtomic_WritesCanonicalStatus mirrors the
+// above for the scheduled-execution cancel path.
+func TestPGXMock_CancelScheduledExecutionAtomic_WritesCanonicalStatus(t *testing.T) {
+	mock := newMock(t)
+	store := storeWith(mock)
+	ctx := context.Background()
+
+	actor := "scheduler@system"
+
+	rows := pgxmock.NewRows([]string{"status"}).AddRow(StatusCanceled)
+	mock.ExpectQuery("canceled_by").
+		WithArgs("exec-sched", &actor, StatusCanceled).
+		WillReturnRows(rows)
+
+	ok, status, err := store.CancelScheduledExecutionAtomic(ctx, mock, "exec-sched", &actor)
+	require.NoError(t, err)
+	assert.True(t, ok, "row should have been updated")
+	assert.Equal(t, StatusCanceled, status, "returned status must be canonical 'canceled'")
+	assert.NoError(t, mock.ExpectationsWereMet())
+}
+
+// TestPGXMock_CancelAllPendingExchanges_WritesCanonicalStatus pins
+// CancelAllPendingExchanges to emit StatusCanceled as the $1 argument.
+// A pre-fix version of the code sent no parameter (the value was inlined as
+// the literal 'cancelled'), so this test double-checks that the parameterized
+// form is used and the value is the canonical spelling.
+func TestPGXMock_CancelAllPendingExchanges_WritesCanonicalStatus(t *testing.T) {
+	mock := newMock(t)
+	store := storeWith(mock)
+	ctx := context.Background()
+
+	mock.ExpectExec("UPDATE ri_exchange_history").
+		WithArgs(StatusCanceled).
+		WillReturnResult(pgxmock.NewResult("UPDATE", 2))
+
+	n, err := store.CancelAllPendingExchanges(ctx)
+	require.NoError(t, err)
+	assert.Equal(t, int64(2), n)
+	assert.NoError(t, mock.ExpectationsWereMet())
+}
+
+// TestPGXMock_CancelPendingExchangesByOrigin_Standalone_WritesCanonicalStatus
+// asserts the standalone branch passes StatusCanceled.
+func TestPGXMock_CancelPendingExchangesByOrigin_Standalone_WritesCanonicalStatus(t *testing.T) {
+	mock := newMock(t)
+	store := storeWith(mock)
+	ctx := context.Background()
+
+	mock.ExpectExec("ladder_run_id IS NULL").
+		WithArgs(StatusCanceled).
+		WillReturnResult(pgxmock.NewResult("UPDATE", 1))
+
+	n, err := store.CancelPendingExchangesByOrigin(ctx, common.ExchangeOriginStandalone)
+	require.NoError(t, err)
+	assert.Equal(t, int64(1), n)
+	assert.NoError(t, mock.ExpectationsWereMet())
+}
+
+// TestPGXMock_CancelPendingExchangesByOrigin_Ladder_WritesCanonicalStatus
+// asserts the ladder branch passes StatusCanceled.
+func TestPGXMock_CancelPendingExchangesByOrigin_Ladder_WritesCanonicalStatus(t *testing.T) {
+	mock := newMock(t)
+	store := storeWith(mock)
+	ctx := context.Background()
+
+	mock.ExpectExec("ladder_run_id IS NOT NULL").
+		WithArgs(StatusCanceled).
+		WillReturnResult(pgxmock.NewResult("UPDATE", 3))
+
+	n, err := store.CancelPendingExchangesByOrigin(ctx, common.ExchangeOriginLadder)
+	require.NoError(t, err)
+	assert.Equal(t, int64(3), n)
+	assert.NoError(t, mock.ExpectationsWereMet())
+}
+
+// TestPGXMock_CleanupOldExecutions_IncludesCanonicalStatus verifies that the
+// cleanup DELETE includes 'canceled' (canonical) in the terminal-status filter,
+// so Plans-canceled rows written by new code are not missed.
+// Regression guard for the #1277 follow-up Defect 2 fix.
+func TestPGXMock_CleanupOldExecutions_IncludesCanonicalStatus(t *testing.T) {
+	mock := newMock(t)
+	store := storeWith(mock)
+	ctx := context.Background()
+
+	// The regex pins on both 'cancelled' (legacy) and 'canceled' (canonical)
+	// appearing together in the SQL. If either is missing the mock won't
+	// match, causing an "unexpected query" error from ExpectationsWereMet.
+	mock.ExpectExec(`'cancelled'.*'canceled'`).
+		WithArgs(pgxmock.AnyArg()).
+		WillReturnResult(pgxmock.NewResult("DELETE", 4))
+
+	n, err := store.CleanupOldExecutions(ctx, 90)
+	require.NoError(t, err)
+	assert.Equal(t, int64(4), n)
+	assert.NoError(t, mock.ExpectationsWereMet())
+}