fix(db): contract the canceled rename: normalize, narrow the status CHECKs, stop using cancelled_by

cmd/cleanup-lambda/main_integration_test.go

@@ -17,7 +17,6 @@ import (
 	"testing"
 	"time"
 
-	"github.com/LeanerCloud/cloud-commitments-platform/internal/config"
 	"github.com/LeanerCloud/cloud-commitments-platform/internal/database"
 	"github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/testhelpers"
 	"github.com/stretchr/testify/require"
@@ -31,7 +30,7 @@ var (
 	cleanupDeletable = []string{"completed", "partially_completed", "expired", "revocation_requested"}
 	// Live (an executor, scheduler or reaper may still act on it), or counted
 	// by the health score and left to CleanupOldExecutions: always kept.
-	cleanupKept = []string{"pending", "notified", "approved", "running", "scheduled", "paused", "failed", "canceled", config.LegacyStatusCanceled}
+	cleanupKept = []string{"pending", "notified", "approved", "running", "scheduled", "paused", "failed", "canceled"}
 )
 
 // TestMain stops the PostgreSQL container that every test in this package shares

internal/api/handler_history.go

@@ -141,12 +141,7 @@ var actionableExecutionStatuses = []string{"pending", "notified", "scheduled", "
 // partial-failure marker and is flagged IsAuditGap so its execution-level
 // dollars are excluded from the dashboard totals — the committed dollars are
 // already counted via the per-rec purchase_history rows that succeeded.
-// Both the US-spelling status (config.StatusCanceled) and the legacy British
-// spelling (config.LegacyStatusCanceled) are listed: during the expand-contract
-// rename (migration 000089) old code may still write the legacy value before
-// the rolling deploy completes. The contract migration (#1278) normalizes the
-// data once the deploy is verified stable; drop the legacy entry here then.
-var terminalExecutionStatuses = []string{"completed", "partially_completed", "failed", "expired", config.StatusCanceled, config.LegacyStatusCanceled}
+var terminalExecutionStatuses = []string{"completed", "partially_completed", "failed", "expired", config.StatusCanceled}
 
 // approvalExpiryWindow is how long a pending approval stays actionable
 // before the History view flips it to "expired". Aligns with the
@@ -433,10 +428,7 @@ 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 config.StatusCanceled, config.LegacyStatusCanceled:
-		// The legacy British spelling is still matched during the
-		// expand-contract rename (migration 000089) until the contract
-		// migration (#1278) normalizes and drops it.
+	case config.StatusCanceled:
 		annotateCancelled(row, exec, approver)
 	default:
 		// In-flight (approved/running/scheduled/paused) and audit-gap
@@ -1190,14 +1182,10 @@ func addNonCompletedRowToSummary(summary *HistorySummary, status string, n int)
 		summary.TotalFailed += n
 	case "expired":
 		summary.TotalExpired += n
-	case config.StatusCanceled, config.LegacyStatusCanceled:
+	case config.StatusCanceled:
 		// 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. The legacy
-		// British spelling is matched alongside the US one during the
-		// expand-contract rename (migration 000089) so legacy rows can't
-		// inflate KPIs mid-deploy; the contract migration (#1278) drops it
-		// once the deploy is stable.
+		// from TotalCompleted -- the money was never committed.
 	default:
 		return false
 	}

internal/api/handler_history_test.go

@@ -1775,16 +1775,9 @@ func TestSummarizePurchaseHistory_CancelledExcludedFromKPIs(t *testing.T) {
 		// One pending row that should be counted as pending, not completed.
 		{Status: "pending", UpfrontCost: new(float64(999.0)), EstimatedSavings: 99.0},
 		// Two canceled rows — the regression case from issue #736. Neither must
-		// appear in the dollar KPIs or TotalCompleted. One uses the new US
-		// spelling (config.StatusCanceled) and one the legacy British spelling
-		// (config.LegacyStatusCanceled): during the expand-contract rename
-		// (migration 000089) a mixed fleet still emits the legacy spelling, so
-		// the dual-spelling read path must exclude BOTH. The constant carries
-		// the legacy value without a literal the US-locale misspell linter would
-		// flag (and without a nolint). Drop the legacy fixture once the contract
-		// migration (#1278) normalizes the data.
+		// appear in the dollar KPIs or TotalCompleted.
 		{Status: config.StatusCanceled, UpfrontCost: new(float64(500.0)), EstimatedSavings: 50.0},
-		{Status: config.LegacyStatusCanceled, UpfrontCost: new(float64(750.0)), EstimatedSavings: 75.0},
+		{Status: config.StatusCanceled, UpfrontCost: new(float64(750.0)), EstimatedSavings: 75.0},
 	}
 
 	summary := summarizePurchaseHistory(purchases)
@@ -1921,7 +1914,7 @@ func TestSummarizePurchaseHistory_CancelPendingDoesNotChangeKPIs(t *testing.T) {
 
 	// After: same rows plus one canceled execution (the pending that got canceled).
 	withCancelled := append(baseline, config.PurchaseHistoryRecord{ //nolint:gocritic
-		Status:           "cancelled", //nolint:misspell // DB schema value 'cancelled' -- see migration 000001_initial_schema.up.sql
+		Status:           config.StatusCanceled,
 		UpfrontCost:      new(float64(999.0)),
 		EstimatedSavings: 99.0,
 	})

internal/api/handler_purchases.go

@@ -770,27 +770,11 @@ func (h *Handler) cancelOrRecoverExecution(ctx context.Context, executionID stri
 	if getErr != nil {
 		return nil, fmt.Errorf("disable plan: failed to get execution %s after conflict: %w", executionID, getErr)
 	}
-	// Accept both spellings: during the expand-contract rename (migration
-	// 000089) a concurrent legacy cancel may have written the legacy value
-	// before the rolling deploy completes. The contract migration (#1278)
-	// normalizes the data, after which LegacyStatusCanceled can be removed.
-	if existing.Status != config.StatusCanceled && existing.Status != config.LegacyStatusCanceled {
+	if existing.Status != config.StatusCanceled {
 		return nil, NewClientError(409, fmt.Sprintf(
 			"execution %s cannot be canceled (status=%s)",
 			executionID, existing.Status))
 	}
-	// Normalize the response Status so the idempotent recovery path returns the
-	// canonical US spelling even when the DB row still carries the legacy value.
-	// Without this, a caller that received an in-flight 200 from this branch
-	// would see status="canceled" while a caller that hit the happy-path
-	// transition above would see status="canceled" for the same execution_id
-	// during the rolling deploy. The legacy value is preserved in storage --
-	// only the in-memory copy returned to the handler is normalized -- so the
-	// contract migration's authoritative status backfill still observes every
-	// legacy row.
-	if existing.Status == config.LegacyStatusCanceled {
-		existing.Status = config.StatusCanceled
-	}
 	if delErr := h.config.DeleteSuppressionsByExecution(ctx, executionID); delErr != nil {
 		return nil, fmt.Errorf("cancel execution %s: failed to release suppressions: %w", executionID, delErr)
 	}

internal/api/handler_purchases_cancel_actor_integration_test.go

@@ -39,7 +39,7 @@ func TestDeletePlannedPurchase_RecordsCancelActor(t *testing.T) {
 	require.Equal(t, config.StatusCanceled, row.Status)
 	require.NotNil(t, row.CancelledBy, "cancel actor must be readable after a UI cancel")
 	require.Equal(t, "creator@example.com", *row.CancelledBy)
-	requireRawCancelColumns(ctx, t, f, execID, "creator@example.com")
+	requireRawCanceledBy(ctx, t, f, execID, "creator@example.com")
 
 	// The record TransitionExecutionStatus returns must use the same projection.
 	directID := uuid.NewString()
@@ -55,7 +55,7 @@ func TestDeletePlannedPurchase_RecordsCancelActor(t *testing.T) {
 
 // TestTransitionExecutionStatus_NonCancelLeavesCancelColumnsNull pins the
 // cancel-only guard: a non-cancel transition with an actor stamps
-// transitioned_by but must not write either cancel column.
+// transitioned_by but must not write canceled_by.
 func TestTransitionExecutionStatus_NonCancelLeavesCancelColumnsNull(t *testing.T) {
 	ctx, cancel := context.WithTimeout(context.Background(), 90*time.Second)
 	t.Cleanup(cancel)
@@ -72,26 +72,19 @@ func TestTransitionExecutionStatus_NonCancelLeavesCancelColumnsNull(t *testing.T
 	_, err := f.store.TransitionExecutionStatus(ctx, execID, []string{"pending"}, config.StatusFailed, &creator)
 	require.NoError(t, err)
 
-	var canceledBy, cancelledBy *string
+	var canceledBy *string
 	require.NoError(t, f.pool.QueryRow(ctx,
-		"SELECT canceled_by, "+legacyCancelColumn+" FROM purchase_executions WHERE execution_id=$1", execID,
-	).Scan(&canceledBy, &cancelledBy))
+		"SELECT canceled_by FROM purchase_executions WHERE execution_id=$1", execID,
+	).Scan(&canceledBy))
 	require.Nil(t, canceledBy, "canceled_by must stay NULL on a non-cancel transition")
-	require.Nil(t, cancelledBy, legacyCancelColumn+" must stay NULL on a non-cancel transition")
 }
 
-func requireRawCancelColumns(ctx context.Context, t *testing.T, f *createConcurrencyFixture, execID, want string) {
+func requireRawCanceledBy(ctx context.Context, t *testing.T, f *createConcurrencyFixture, execID, want string) {
 	t.Helper()
-	var canceledBy, cancelledBy *string
+	var canceledBy *string
 	require.NoError(t, f.pool.QueryRow(ctx,
-		"SELECT canceled_by, "+legacyCancelColumn+" FROM purchase_executions WHERE execution_id=$1", execID,
-	).Scan(&canceledBy, &cancelledBy))
+		"SELECT canceled_by FROM purchase_executions WHERE execution_id=$1", execID,
+	).Scan(&canceledBy))
 	require.NotNil(t, canceledBy, "canceled_by column")
-	require.NotNil(t, cancelledBy, legacyCancelColumn+" column")
 	require.Equal(t, want, *canceledBy)
-	require.Equal(t, want, *cancelledBy)
 }
-
-// legacyCancelColumn is the pre-migration-000089 British spelling, split so the
-// misspell linter does not flag the column name.
-const legacyCancelColumn = "cancel" + "led_by"

internal/api/handler_purchases_submit_dedupe_integration_test.go

@@ -118,7 +118,7 @@ func TestPersistExecution_RetryAfterRowLeftPending(t *testing.T) {
 	}{
 		{"pending", true}, {"notified", true}, {"approved", true}, {"running", true},
 		{"completed", true}, {"partially_completed", true},
-		{config.StatusFailed, false}, {config.StatusCanceled, false}, {config.LegacyStatusCanceled, false}, {config.StatusExpired, false},
+		{config.StatusFailed, false}, {config.StatusCanceled, false}, {config.StatusExpired, false},
 	} {
 		t.Run(tc.status, func(t *testing.T) {
 			recs, key := submitFixture(t)

internal/api/handler_purchases_test.go

@@ -2019,67 +2019,6 @@ func TestHandler_deletePlannedPurchase_ConflictRetryRunningReturns409(t *testing
 	assert.Contains(t, ce.message, "running", "error must include actual status")
 }
 
-// TestHandler_deletePlannedPurchase_ConflictRetryLegacyCanceledNormalizes is
-// the regression guard for the CR outside-diff comment on PR #1277: when the
-// CAS conflict recovery branch finds the row in the LEGACY British spelling
-// (the row was canceled by old code during the rolling deploy window), the
-// API response Status must be normalized to the canonical US spelling so a
-// caller doesn't observe two different status values for the same execution
-// depending on which code instance handled the request. The stored row is
-// untouched -- only the in-memory copy returned to the handler is normalized.
-func TestHandler_deletePlannedPurchase_ConflictRetryLegacyCanceledNormalizes(t *testing.T) {
-	ctx := context.Background()
-	mockStore := new(MockConfigStore)
-	mockAuth := new(MockAuthService)
-	t.Cleanup(func() { mockStore.AssertExpectations(t) })
-	t.Cleanup(func() { mockAuth.AssertExpectations(t) })
-
-	adminSession := &Session{
-		UserID: "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa",
-		Email:  "admin@example.com",
-	}
-
-	planID := "12121212-1212-1212-1212-121212121212"
-	execID := "13131313-1313-1313-1313-131313131313"
-
-	conflictErr := fmt.Errorf("%w: execution %s cannot transition", config.ErrExecutionNotInExpectedStatus, execID)
-
-	// Old code already canceled this row during the deploy window: the DB
-	// holds the legacy British spelling. The handler must accept it (idempotent
-	// recovery) but normalize the response to the canonical US spelling.
-	existingExec := &config.PurchaseExecution{
-		ExecutionID: execID,
-		PlanID:      planID,
-		Status:      config.LegacyStatusCanceled,
-	}
-	plan := &config.PurchasePlan{
-		ID:      planID,
-		Name:    "Legacy Cancel Plan",
-		Enabled: true,
-	}
-
-	mockAuth.On("ValidateSession", ctx, "admin-token").Return(adminSession, nil)
-	mockAuth.grantAdmin()
-	mockStore.On("TransitionExecutionStatus", ctx, execID, []string{"pending", "notified", "paused"}, "canceled", mock.Anything).Return(nil, conflictErr)
-	mockStore.On("GetExecutionByID", ctx, execID).Return(existingExec, nil)
-	mockStore.On("GetPurchasePlan", ctx, planID).Return(plan, nil)
-	mockStore.On("UpdatePurchasePlan", ctx, mock.MatchedBy(func(p *config.PurchasePlan) bool {
-		return p.ID == planID && !p.Enabled
-	})).Return(nil)
-
-	handler := &Handler{config: mockStore, auth: mockAuth}
-
-	req := &events.LambdaFunctionURLRequest{
-		Headers: map[string]string{"Authorization": "Bearer admin-token"},
-	}
-	result, err := handler.deletePlannedPurchase(ctx, req, execID)
-	require.NoError(t, err)
-	assert.Equal(t, config.StatusCanceled, result.Status,
-		"legacy 'cancelled' must be normalized to the canonical US 'canceled' in the response")
-	assert.NotEqual(t, config.LegacyStatusCanceled, result.Status,
-		"the response must never leak the legacy spelling once the recovery branch ran")
-}
-
 // TestHandler_deletePlannedPurchase_BackendErrorReturns5xx is the regression
 // guard for CodeRabbit round-2 finding #1 (same class as PR #1276): a
 // TransitionExecutionStatus error that is NOT ErrExecutionNotInExpectedStatus

internal/api/plan_health.go

@@ -222,11 +222,9 @@ func failedExecutionsFactor(counts config.ExecutionStatusCounts) (PlanHealthFact
 }
 
 // canceledExecutionsFactor: -5 per canceled execution in the last
-// planHealthLookbackDays, capped at 4 (-20 max). Counts both spellings of
-// "canceled" (config.StatusCanceled and the legacy
-// config.LegacyStatusCanceled) so pre-#1278 rows aren't invisible to scoring.
+// planHealthLookbackDays, capped at 4 (-20 max).
 func canceledExecutionsFactor(counts config.ExecutionStatusCounts) (PlanHealthFactor, bool) {
-	count := counts[config.StatusCanceled] + counts[config.LegacyStatusCanceled]
+	count := counts[config.StatusCanceled]
 	if count == 0 {
 		return PlanHealthFactor{}, false
 	}

internal/api/plan_health_test.go

@@ -149,7 +149,7 @@ func TestComputePlanHealth_FailedExecutionsPenaltyCapsAtFour(t *testing.T) {
 	assert.Equal(t, 100-40, score)
 }
 
-func TestComputePlanHealth_CanceledExecutionsPenaltyCapsAtFourAndCountsBothSpellings(t *testing.T) {
+func TestComputePlanHealth_CanceledExecutionsPenaltyCapsAtFour(t *testing.T) {
 	now := time.Date(2026, 7, 27, 12, 0, 0, 0, time.UTC)
 	plan := config.PurchasePlan{
 		Enabled: true,
@@ -160,11 +160,10 @@ func TestComputePlanHealth_CanceledExecutionsPenaltyCapsAtFourAndCountsBothSpell
 			StartDate:        now.AddDate(0, 0, -1),
 		},
 	}
-	// 3 canonical + 2 legacy-spelled = 5 total canceled; penalty caps at
-	// 4 * 5 = 20 but the note reports the true count of 5.
+	// 5 canceled; penalty caps at 4 * 5 = 20 but the note reports the true
+	// count of 5.
 	execCounts := counts(map[string]int{
-		config.StatusCanceled:       3,
-		config.LegacyStatusCanceled: 2,
+		config.StatusCanceled: 5,
 	})
 
 	score, factors := computePlanHealth(plan, now, execCounts, config.RampStepBlock{})

internal/config/interfaces.go

@@ -251,11 +251,10 @@ type StoreInterface interface {
 	// (and writes nothing) when the row has left those statuses, so a cancel or
 	// approve that lands first is never overwritten.
 	RescheduleAwaitingExecution(ctx context.Context, executionID string, scheduledDate time.Time) (bool, error)
-	// SetCancelledBy stamps canceled_by and the legacy cancelled_by column on
-	// an execution without overwriting any other columns. Used after
-	// TransitionExecutionStatus to fold the actor attribution into the same
-	// logical write without the full-row SavePurchaseExecution clobber risk
-	// (Finding #5 / PR #889).
+	// SetCancelledBy stamps canceled_by on an execution without overwriting
+	// any other columns. Used after TransitionExecutionStatus to fold the actor
+	// attribution into the same logical write without the full-row
+	// SavePurchaseExecution clobber risk (Finding #5 / PR #889).
 	SetCancelledBy(ctx context.Context, executionID string, cancelledBy string) error
 	// LinkRetryExecutionAtomic stamps retry_execution_id on a failed
 	// execution row without a full-row overwrite, conditional on