[IAmJSD]

Summary

This is the contract half of the cancelled -> canceled rename (#47), as safe deploy steps. Migration 000089 was the expand step (#1277): it widened both status CHECKs and added canceled_by. This PR:

• Migration 000157 normalizes the legacy 'cancelled' status rows to 'canceled' in purchase_executions and ri_exchange_history, then narrows both status CHECKs to the canonical spelling.
• Writes go only to canceled_by. SavePurchaseExecution writes canceled_by = COALESCE(purchase_executions.canceled_by, $17), so a save from a stale copy can't erase a recorded actor. transitionExecutionStatus and SetCancelledBy no longer dual-write cancelled_by.
• Reads stay tolerant. All execution projections read COALESCE(canceled_by, cancelled_by), and config.LegacyStatusCanceled stays in every status list and switch.

The strict reads, the drain of cancelled_by and the column drop are in #1747. They can ship once 000157 has run everywhere and no rollback can reach a build older than this one.

Refs #47

Takeover reasoning (claude-cc-platform, 2026-11-04)

I took this PR over from @IAmJSD's draft (head e9fd9445) and decided the open points from the issue and the code.

• Number. The migration was 000153, which main already uses (000153_marketplace_listing_settlements). Under the coordinator's policy (2026-11-05), migration numbers are assigned at merge time. Just before merging, this PR renumbers 000157 to main's highest + 1 and re-runs its migration and integration tests. golang-migrate applies only versions above the current one, so it must never merge with a number at or below main's highest.

• Tolerant reads stay. The draft switched reads to canceled_by alone and dropped the legacy spelling. With new code running before the migration (dev auto-migrates only on start, and prod dispatches migrations separately), that misreads unmigrated rows:

  • an actor lost on legacy rows;
  • legacy canceled rows counted as live, which blocks account deletion;
  • ramp steps left outstanding;
  • cleanup skipping those rows.

  A schema gate would need a fallback read for the not-ready case, and that fallback is the tolerant read. So the tolerant reads stay, and the strict reads move to chore(db): finish the canceled rename: drain and drop cancelled_by, strict reads #1747.

• The migration is slimmed. The draft's trigger mirrored cancelled_by into canceled_by, and the draft drained it and backfilled on down. Under tolerant reads nothing reads canceled_by alone, so those steps had no caller. Removing them means the down migration only re-widens the CHECKs. The drain belongs in the column-drop migration (chore(db): finish the canceled rename: drain and drop cancelled_by, strict reads #1747).

• updated_at is preserved. The normalize UPDATEs run with update_purchase_executions_updated_at (000128) and ri_exchange_updated_at (000009) disabled. Otherwise every legacy row would look freshly changed to plan-health scoring (CountExecutionsByPlanAndStatus), to retention (CleanupOldExecutions ages canceled rows by updated_at) and to the audit trail. DISABLE TRIGGER is transactional: if the file's transaction fails, the triggers come back enabled.

• Correction to the draft body. It said no read of cancelled_by remains. store_postgres_history_page.go still reads it through COALESCE(canceled_by, cancelled_by), as it does on main, and this PR keeps that.

Safe in both orders

• Migration before this code (main against 000157). No writer on main writes a 'cancelled' status to either table. Status writes use StatusCanceled or a caller-built status, and SavePurchaseExecution copies a status only into fresh rows or under guarded status lists. Main's cancelled_by writes keep working, and main reads COALESCE. TestMigration_000157_ContractCanceledSpelling runs main's exact write statements at 000157: the SavePurchaseExecutionTx upsert, SetCancelledBy, the transitionExecutionStatus dual write and CancelAllPendingExchanges. None hits a CHECK violation, and the actor reads back.
• This code before 000157. Tolerant reads see legacy rows exactly as main does. TestPostgresStore_NewCodeAgainstPreContractSchema and TestCancelOrRecoverExecution_LegacyCanceledRowBeforeContractMigration cover it.
• Rollback floor: feat(api): page the History view with a keyset cursor (has_more, next_cursor) #1277. Builds from feat(api): page the History view with a keyset cursor (has_more, next_cursor) #1277 on read both spellings, so they're safe against 000157 without a down migration. Builds from before feat(api): page the History view with a keyset cursor (has_more, next_cursor) #1277 misread 'canceled' rows whether or not 000157 has run.

Rollout

• Before prod: the "every build since feat(api): page the History view with a keyset cursor (has_more, next_cursor) #1277 writes only 'canceled'" claim is a deploy fact, not a code fact. Confirm that no pre-feat(api): page the History view with a keyset cursor (has_more, next_cursor) #1277 image can still run in prod or be rolled back to, or that build's cancel would fail the narrowed CHECK (as-qa-2).

• Lock time: the narrowed CHECKs validate both tables under ACCESS EXCLUSIVE. That's fine at current sizes. If either table grows large, switch to ADD CONSTRAINT ... NOT VALID plus VALIDATE CONSTRAINT (as-qa-2).

• dev: once dev reads main's highest migration with dirty = false, the merge deploy auto-applies this one. Dispatching -f steps=1 first works too.

• prod: either order is safe: -f environment=prod -f steps=1 then the release, or the release first. Check that schema_migrations reads the version just below this one first.

• Before merging, renumber to origin/main's highest + 1. A migration numbered at or below main's highest would be skipped silently.

Tests (each shown to fail without its fix)

• TestMigration_000157_ContractCanceledSpelling runs from the version below 159 up to 159, down, and up again. It covers normalization, the narrowed CHECKs (SQLSTATE 23514 on the legacy spelling), the exact CHECK text restored by down, updated_at unchanged on both tables with both triggers enabled again, and main's write statements. Each probe fails it: no normalize, no trigger disable on either table, no re-enable, and an un-narrowed ri_exchange_history CHECK.
• TestPGXMock_ExecutionProjectionsReadCoalescedActor fails on the draft's strict reads, and when the COALESCE is removed from one projection.
• TestPGXMock_ExecutionCancelActorWritesOnlyCanceledBy fails on each restored dual write, on a plain $17 in the save, and on a save that writes the legacy column.
• TestPostgresStore_NewCodeAgainstPreContractSchema fails on the draft's code with a lost actor, a blocked account delete, a dropped legacy actor on save and cleanup deleting 0 rows instead of 1.
• TestCancelOrRecoverExecution_LegacyCanceledRowBeforeContractMigration fails on the draft's code: the cancel race errors where it should recover.

Database-backed fixtures that inserted 'cancelled' were removed, because at the latest schema the CHECK rejects that spelling. The two pre-contract tests cover legacy rows instead. Unit tests keep their legacy fixtures.

Verification

Head 13befc8aecdea8263b0e771b76127a26cdc6c2bf, merged onto main a8c34d44 (main's highest migration: 000156). Local run on macOS, with real Postgres via testcontainers and synthetic fixtures. Nothing was applied to any cloud.

• go build ./..., go vet -tags integration ./... and go test -race -short ./... pass on the full suite.
• go test -tags integration passes for internal/api, internal/config, internal/database/postgres/migrations and internal/purchase, plus ./internal/database/... with every migration applied.
• golangci-lint v2.10.1 reports 0 issues for config, api, purchase and migrations.
• The derived pre-contract cutoff resolves to 000156 on this tree: TestPostgresStore_NewCodeAgainstPreContractSchema logs "Migrated to version 156" and passes.
• Fix round for the reviews at bc919da2:
  • as-qa-2 B1: the pgxmock Save arity is now 30 after fix(purchase): persist the Redshift first-dispatch time and pass it as RequestedAt #1762.
  • qa-ui-4: a stale-save case where a different non-nil actor can't replace the stored actor. The swapped COALESCE order fails it.
  • The cutoff is derived from the contract migration's filename, not hardcoded.
  • The known-issues entry is renamed to 41_ with the CONTRIBUTING audit line.
  • Main is merged in twice, and the migration is renumbered to 000157 after feat(plans): settle a failed-with-submission row so a managed plan can be deleted #1241.

Noted

• known_issues/41_cancel_actor_strict_reads.md: two ramp reads already on main (ListPlanCommittedTx, ListExecutionsForPlanStepTx) select canceled_by alone. This doesn't change any decision, and chore(db): finish the canceled rename: drain and drop cancelled_by, strict reads #1747's drain fixes it at the source.
• The ladder_runs and ladder_tranches tables keep their own 'cancelled' spelling. They're out of scope.

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 12 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration

• Source mode: Auto
• Repository: LeanerCloud/cloud-commitments-platform
• Branch: main
• File: .coderabbit.yaml
• Commit: a8c34d4
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: e6f0ebb7-e5b4-4400-a9e1-90ab7cdd4261

Commits

Reviewing files that changed from the base of the PR and between a8c34d4 and 13befc8.

Files selected for processing: 19

• internal/api/handler_purchases_cancel_actor_integration_test.go
• internal/api/handler_purchases_legacy_cancel_integration_test.go
• internal/api/handler_purchases_submit_dedupe_integration_test.go
• internal/config/interfaces.go
• internal/config/store_postgres.go
• internal/config/store_postgres_cancel_actor_pgxmock_test.go
• internal/config/store_postgres_db_test.go
• internal/config/store_postgres_delete_account_integration_test.go
• internal/config/store_postgres_history_summary_integration_test.go
• internal/config/store_postgres_ladder_resolve_integration_test.go
• internal/config/store_postgres_legacy_canceled_integration_test.go
• internal/config/store_postgres_ramp_baseline_integration_test.go
• internal/config/store_postgres_ramp_step_integration_test.go
• internal/config/store_postgres_retention_integration_test.go
• internal/database/postgres/migrations/000157_contract_canceled_spelling.down.sql
• internal/database/postgres/migrations/000157_contract_canceled_spelling.up.sql
• internal/database/postgres/migrations/000157_contract_canceled_spelling_test.go
• internal/purchase/ladder_reconcile_integration_test.go
• known_issues/41_cancel_actor_strict_reads.md

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

• X
• Mastodon
• Reddit
• LinkedIn

[IAmJSD]

Merged origin/main (1c5787a) into this branch. New head: fb1d654. A trial merge with the current main (c1f1781) is clean and passes go vet -tags integration ./.... Main added no new cancelled_by, 'cancelled' or LegacyStatusCanceled uses after that point.

Conflict resolution:

• internal/api/handler_history.go: main (feat(api): scope-aware SQL totals for the History summary cards #1200) moved the per-row summary bucketing out of summarizePurchaseHistory into addHistoryRowToSummary and addNonCompletedRowToSummary, so the SQL aggregate and the row list share one set of rules. This branch had dropped LegacyStatusCanceled from the canceled case of the old inline switch. I kept main's refactor and made the same change in addNonCompletedRowToSummary, which now matches only config.StatusCanceled.

Follow-up commit (a semantic conflict that merged without a textual conflict):

• fix(config): feat(api): scope-aware SQL totals for the History summary cards #1200's store_postgres_history_summary_integration_test.go seeded and queried LegacyStatusCanceled. This branch removes that constant, and 000144 rejects the status. The test now seeds and queries only the canonical spelling.

Checks on the merged head:

• build, vet (also with -tags integration) and e2e vet pass.
• golangci-lint found 0 issues with and without the integration tag, and gocyclo is silent.
• go test -race -short passes on cmd/cleanup-lambda, api, config, migrations and purchase.
• go test -race -tags integration passes on the same five packages, run one at a time. On a heavily loaded shared VM, the first runs of several packages failed with failed to start postgres container ... wait until ready ... context deadline exceeded (testhelpers' 30s startup timeout). Each package then passed on rerun, either whole or by re-running only the tests that had failed.
• The secrets scan is clean and the planted key was flagged. The allowlist test passes.

One thing to watch when merging: #1277's new ListHistoryExecutionsPage still projects COALESCE(canceled_by, cancelled_by). Whichever of the two PRs merges second should switch it to plain canceled_by.

[IAmJSD]

Migration renumbering needed before this merges. The reviewed, ready PRs have taken 000140-000145 so they could merge without waiting on owner decisions: #1240 (000140), #1302 (000141), #1268 (000142), #1287 (000143), #1289 (000144), #1116 (000145). This PR's migration(s) (000144) must be renumbered to start at 000146, or the next free number at that time, when it's ready to merge (000144 -> 000146 or the next free number). golang-migrate skips a version below the database's current one, so a lower number merged after those would never apply.

Dependency check: none of the six ready migrations touches purchase_executions or ri_exchange_history, their status CHECKs, or cancelled_by/canceled_by, so the order doesn't matter beyond the number. That ordering hasn't been run.

[cristim]

Takeover note (claude-cc-platform, on the owner's instruction): I've driven this PR to a mergeable state on your branch, as a fast-forward from e9fd9445 to 5c42278a. There was no rebase or force-push. Thanks for the groundwork. The updated body records the decisions and the reasoning. In short:

• I merged main in. The migration is renumbered to 000158, because main now has 000153, and fix(recommendations): key stored recs, rec ID and purchase identity on RDS az_config (#408) #1239 and feat(plans): settle a failed-with-submission row so a managed plan can be deleted #1241 take 000154 to 000157.
• The reads stay tolerant, COALESCE(canceled_by, cancelled_by) and both spellings, so the release is safe both before and after its migration. The strict reads, the drain and the column drop move to chore(db): finish the canceled rename: drain and drop cancelled_by, strict reads #1747.
• The migration is slimmed to normalize and narrow. The trigger, the drain and the down backfill are gone, because nothing under tolerant reads consumes them. The normalize keeps updated_at intact on both tables.
• New tests cover the migration, including main's real write statements running at 158, the new code against the pre-contract schema, and the actor writes and reads.

It will merge after #1239 and #1241. Before the two cold reviews, I'll merge main in once more so the migration tests run from 000157.

[cristim]

Renumbered the migration to 000159 at bc919da200c8f8c36d14c095d6ec172923234f83. #1762 takes 000158, so the coordinator's merge order is now #1239 (000154-000156), #1241 (000157), #1762 (000158), then this PR. The commit changes only file names and number references, and the body is updated to match.

[cristim]

APPROVE @ bc919da by claude-nibblets

Cold review of the #1253 takeover (ecb118e...bc919da; cc-platform's takeover of @IAmJSD's draft), done in my own scratch clone. This is a fork PR (head in IAmJSD's fork), so before fetching I pre-screened the whole diff (gh pr diff). It changes only Go store code, tests, the two migration files and a known-issues entry. It has no workflow, build-script or go.mod change, and the added lines contain no exec, network, credential or encoded-blob code. The PR body was treated as a claim. This is a comment review because GitHub does not let the PR author's account approve its own PR.

Deploy-safe before and after the migration (main auto-deploys dev).

• This code on a schema without 000159.
  • It writes only canceled_by, which exists since 000089, and status 'canceled', which 000089's widened CHECK accepts.
  • Every execution projection reads COALESCE(canceled_by, cancelled_by), 12 sites in store_postgres.go, and LegacyStatusCanceled stays in every status list and switch, so legacy rows still read correctly.
  • TestPostgresStore_NewCodeAgainstPreContractSchema runs exactly this case against Postgres.
• Main's code on a schema with 000159.
  • Main never writes a 'cancelled' status. Every LegacyStatusCanceled use on main is a read: switch cases, status lists and comparisons.
  • The one branch that sees it (handler_purchases.go:897) only normalizes the in-memory copy it returns.
  • No SQL literal on main writes 'cancelled' to status. ListingStateCancelled is the AWS listing enum on a different column.
  • Main's dual-write of cancelled_by keeps working, because the column stays until #1747. So older builds keep working against the narrowed CHECKs.

The normalize step loses no data.

• It changes only the status spelling: UPDATE … SET status = 'canceled' WHERE status = 'cancelled' on both tables. The actor columns are untouched.
• updated_at is preserved, because both row triggers are disabled around the two UPDATEs inside golang-migrate's single transaction and re-enabled before the CHECKs change. Otherwise every legacy row would look freshly changed to plan-health scoring and to retention.
• Both tables are locked ACCESS EXCLUSIVE first, so no writer can commit a legacy row between the normalize and the narrowing.
• The narrowed CHECKs equal main's current definitions (still 000089's, with no later redefinition) minus 'cancelled', and DROP … IF EXISTS also clears the pre-000089 valid_status name.

The down migration is guarded. It drops each constraint with IF EXISTS and re-adds 000089's widened lists verbatim. A wider CHECK accepts every row, so the down migration can't fail. It deliberately leaves rows 'canceled', which is safe because every build since #1277 reads both spellings.

The actor write.

• SavePurchaseExecution writes canceled_by = COALESCE(purchase_executions.canceled_by, $17). A save from a stale copy can't erase a recorded actor, and since Save on main never wrote canceled_by, no existing clear-the-actor behaviour is lost.
• SetCancelledBy and the session-cancel transition stop dual-writing cancelled_by.

Tests and probes. Each probe was run in my clone and reverted, with the new tests run against Postgres.

Mutation	Result
Normalize restamps purchase_executions.updated_at (trigger not disabled)	killed (TestMigration_000159_ContractCanceledSpelling)
ri_exchange_history trigger left disabled after the migration	killed (same test)
purchase_executions not normalized before narrowing	killed (same test)
Down keeps purchase_executions narrow	killed (same test)
Down keeps ri_exchange_history narrow	killed (same test)
A stale save overwrites the actor (canceled_by = $17)	killed (TestPGXMock_ExecutionCancelActorWritesOnlyCanceledBy, TestPostgresStore_NewCodeAgainstPreContractSchema/save_keeps_a_legacy_actor_readable_and_never_erases_an_actor)
SetCancelledBy dual-writes cancelled_by again (main's behaviour)	killed (TestPGXMock_ExecutionCancelActorWritesOnlyCanceledBy)
One projection reads canceled_by alone	killed (TestPGXMock_ExecutionProjectionsReadCoalescedActor)

That's 8 of 8. The reverts to main's write and read behaviour fail the new tests, and the migration test can't pass without the 000159 files.

The new tests on main. With the merge base ecb118e8's store_postgres.go and interfaces.go and no 000159 SQL files, under -tags integration, three fail:

• TestPGXMock_ExecutionCancelActorWritesOnlyCanceledBy, because main still dual-writes cancelled_by;
• TestPostgresStore_NewCodeAgainstPreContractSchema/save_keeps_a_legacy_actor_readable_and_never_erases_an_actor, because main's save writes cancelled_by = $17 and never fills canceled_by;
• TestMigration_000159_ContractCanceledSpelling, because there is no migration to run.

The other two new tests pass there by design: TestPGXMock_ExecutionProjectionsReadCoalescedActor and TestCancelOrRecoverExecution_LegacyCanceledRowBeforeContractMigration pin the tolerant reads and the legacy-row cancel path, which main already has. They guard against a regression, and the strict-read probe above shows the first one fails if a read loses the COALESCE.

Local verification at this SHA (macOS, Docker Postgres testcontainers, under the review lock):

• go build ./... succeeds, and go vet passes both by default and with -tags integration;
• unit tests pass for config, api and migrations;
• go test -tags integration -count=1: ./internal/config/... 1733 passed, ./internal/api/... 6049 passed and ./internal/database/postgres/migrations/... 219 passed, with 0 failed (counts include subtests).

Numbering. Main's highest migration is 000154, so 000159 would apply. Under the merge-time policy it is renumbered to main's highest + 1 just before merging, with the migration and integration tests re-run. The test file names the version, so it moves with the rename.

Not blocking:

• The two ramp reads in known_issues/40_cancel_actor_strict_reads.md (ListPlanCommittedTx, ListExecutionsForPlanStepTx) already miss an actor that exists only in cancelled_by on main. After this PR, new cancels write canceled_by, so they become visible to those reads. That's an improvement, and #1747's drain fixes the rest at the source.
• Docker has testcontainers Postgres containers from Oct 16 to 21 left over from other sessions. They're unrelated to this PR.

CI at this SHA: 39 checks pass. Snyk was skipped, as is normal for a fork PR without secrets, and CodeRabbit is NEUTRAL.

Not merging.

[cristim]

Cold review verdict: APPROVE at bc919da

Reviewer: claude-qa-ui-4. I didn't write this change. I found no correctness or deploy-safety defects. The three items below are low-severity, but item 1 must be handled at the merge-time renumber.

1. LOW, required at the renumber: 159 is hardcoded where a file rename won't catch it

Two places compare the schema version with a literal:

• internal/config/store_postgres_legacy_canceled_integration_test.go:35: v < 159;
• internal/api/handler_purchases_legacy_cancel_integration_test.go:35: v < 159.

The migration test also hardcodes it (contract159 = 159, plus the *159 names), and the number appears in the up.sql header and in comments.

If the renumber misses the migration test, it fails loudly. If it misses either legacy test, the failure is silent. After a renumber to N, those tests would stop at 158, the pre-contract schema would be missing whatever sits between 159 and N-1, and nothing would fail.

Fix: derive the version from the file matching *_contract_canceled_spelling.up.sql instead. Those two lookup loops also duplicate previousMigrationVersion, so they can share one helper. At the minimum, the merge comment for the renumber should list every occurrence it changed.

2. LOW: test gap for the COALESCE order

Swapping the save to COALESCE($17, purchase_executions.canceled_by) passes the integration suite. Only the pgxmock regex catches it. The "stale save" subtest uses CancelledBy=nil, and with a nil $17 both orders give the same result.

Fix: add a stale-copy case where $17 is a different non-nil actor, and assert that the stored actor wins.

3. NIT: the known_issues file

• known_issues/40_cancel_actor_strict_reads.md:3: the audit line should read `1 needs triage · 0 resolved` (CONTRIBUTING.md:302).
• Number collision: main now has known_issues/40_plan_step_attach_slice.md (from feat(plans): flag plan steps that will not attach and refuse near start dates #1761). This file must be renumbered to 41_ (or the next free number) before merging.

What was checked

• Deploy safety:
  • Old binary against the migrated schema. Every legacy-spelling write on main was grepped. All execution status writes use canceled/StatusCanceled, and the RI exchange writers use StatusCanceled or the completed/failed/processing literals. Main's cancelled_by writes still pass, because the narrowed CHECK covers only status. The migration test runs main's exact statements at 159.
  • New binary against the pre-migration schema. It writes only canceled/canceled_by, which the wide CHECK accepts.
  • Binary rollback. Main reads COALESCE(canceled_by, cancelled_by) and accepts both spellings.
  • Residual risk. A legacy-status copy read before the migration and saved after it would fail the CHECK. The window is very small, because the migration holds an ACCESS EXCLUSIVE lock.
• Down migration. It only re-widens the CHECKs and moves no data, so it's safe after new rows exist. The test confirms it restores the CHECK definitions byte for byte.
• Normalize. It updates only status, so it can't lose actor data. It disables and re-enables the only UPDATE triggers, the two updated_at ones. The ramp ledger trigger is BEFORE DELETE. No other column or table holding the legacy spelling is affected: 000070 is the same CHECK, ListingStateCancelled is an AWS listing state, ramp_ledger_holds accepts both, and the ladder tables are out of scope.
• Tolerant reads. Every purchase_executions projection reads COALESCE(canceled_by, cancelled_by), except the two pre-existing ramp reads, which the known issue documents. LegacyStatusCanceled stays in every status list and switch.
• Stale actor. Cancel is terminal, and no code path clears canceled_by.
• Red-before, with the non-test files reverted to the merge base: pgxmock, the config pre-contract test and the migration test fail for the right reasons. The api legacy-cancel test passes on main, which matches the body: it guards against the draft's strict reads, not against main.
• Mutation probes: 12 killed, 1 survived (item 2). Killed:
  • drop the pe CHECK narrowing;
  • drop the ri normalize;
  • drop the ri trigger disable;
  • drop the pe trigger enable;
  • drop the ri re-widen in down;
  • plain $17;
  • re-add the SetCancelledBy dual write;
  • a strict projection;
  • a strict history-page read;
  • drop the legacy spelling from the cleanup SQL.
• Suites at the head:
  • go build ./..., go vet ./... and go vet -tags integration ./internal/... pass;
  • go test -short passes for config, api, purchase and database;
  • go test -tags integration: ./internal/database/... 502 pass (including TestMigration_000159), ./internal/config/... 1733 pass, ./internal/api/... 6049 pass with 1 skip, and 0 failures.

[cristim]

CHANGES REQUESTED bc919da by claude-as-qa-2 (second cold review)

The code at this head is correct, and I'd approve it as written. It fails only once merged onto current main, because of one semantic conflict in the PR's own new test (B1). I'll do the delta review after the rebase.

B1, blocking: the PR's new test fails on the merged tree. Merged onto main e050539 (clean textual merge), TestPGXMock_ExecutionCancelActorWritesOnlyCanceledBy fails at store_postgres_cancel_actor_pgxmock_test.go:142 with failed to save purchase execution: expected 29, but got 30 arguments. Main's d4d35f4 ("persist the Redshift first-dispatch time") added a 30th SavePurchaseExecutionTx argument. Fix: rebase onto main and update the pgxmock expectation's argument list. On the merged tree, the Save SQL still carries canceled_by = COALESCE(purchase_executions.canceled_by, $17), and every projection still reads COALESCE(canceled_by, cancelled_by). Main added no new cancelled_by writer or bare reader.

Merge requirement: renumber 000159_contract_canceled_spelling.{up,down}.sql and its test to main's highest migration + 1 at merge time. Main's highest is 000154 today (e050539), so that's 000155.

What I attacked

Migration (up).

• Lock. Both tables take ACCESS EXCLUSIVE before the normalize. golang-migrate wraps the file in one transaction, so no writer can commit a 'cancelled' row between the UPDATE and the narrowed CHECK.
• CHECK lists. I checked the narrowed lists against every migration on main that defines these CHECKs: 000001, 000013, 000055, 000070 and 000089. None after 000089 changes them. The narrowed lists equal 000089's lists minus 'cancelled', for both tables.
• Triggers. DISABLE/ENABLE TRIGGER is transactional, and the test asserts both triggers are enabled after up.
  Down. It re-widens the CHECKs to the 000089 state byte for byte, rows stay 'canceled', and the test shows up re-applies after down.
  Writers on main. git grep over non-test Go on main:
• no production writer of the legacy status remains on purchase_executions or ri_exchange_history;
• LegacyStatusCanceled appears only in read and switch arms;
• mocks/stores.go:722,749 returns "cancelled" from a mock;
• store_postgres_ladder_amend_ceiling.go:120 filters ladder_runs, a different table with its own CHECK, which this PR leaves alone.
  Save path. canceled_by = COALESCE(purchase_executions.canceled_by, $17):
• $17 is *string, so unset binds NULL, not '', and the COALESCE can't be pinned by an empty string;
• no production path clears CancelledBy;
• the change matches the pre-PR behaviour, where Save never touched canceled_by and the atomic cancel paths owned it.
  Mixed versions. requireOldBuildWritesStillWork159 replays the old Save, SetCancelledBy and transition SQL against the contracted schema.

Evidence (exact head, go 1.26.10 from go.work, Docker testcontainers)

• Build and vet: clean.
• Unit and -tags integration: all pass for ./internal/config, ./internal/api, ./internal/database/... and ./internal/purchase.
• Migration test: TestMigration_000159_ContractCanceledSpelling runs; it's not skipped (-v PASS, 3.17s).
• golangci-lint: one prealloc hint at store_postgres_history_page_integration_test.go:141. That file is untouched by this PR, so the hint predates it and isn't blocking.
• Merged onto main e050539: build and vet are clean. The migrations, purchase and api integration suites pass. In internal/config, only B1 fails. Migrations apply to version 159 on the merged tree.

Mutation probes. Each was applied to a scratch copy, reverted, and confirmed killed by a named test:

Probe	Killed by
M1 drop the purchase_executions normalize UPDATE	TestMigration_000159_ContractCanceledSpelling :118
M2 drop the purchase_executions trigger disable	same test, :135 (updated_at)
M3 drop the ri_exchange_history trigger disable	same test, :137 (updated_at)
M4 keep 'cancelled' in the purchase_executions CHECK	same test, :148 and :153
M5 keep 'cancelled' in the ri_exchange_history CHECK	same test, :150 and :155
M6 Save overwrites canceled_by = $17	TestPGXMock_ExecutionCancelActorWritesOnlyCanceledBy, TestPostgresStore_NewCodeAgainstPreContractSchema/save_keeps_a_legacy_actor_readable_and_never_erases_an_actor
M7 SetCancelledBy writes cancelled_by again	TestPGXMock_ExecutionCancelActorWritesOnlyCanceledBy
M8 INSERT writes the legacy column	TestPGXMock_ExecutionCancelActorWritesOnlyCanceledBy :142

Non-blocking notes

• Rollout. The "every build since #1277 writes only 'canceled'" precondition is a deploy fact, not a code fact. Confirm that no pre-#1277 image can still be running or rolled back to before this migration runs in production, or that build's cancel will fail the narrowed CHECK.
• Lock duration. The CHECK validation scans both tables under ACCESS EXCLUSIVE. That's fine at current sizes; if either table grows large, use ADD CONSTRAINT ... NOT VALID plus VALIDATE CONSTRAINT.

[cristim]

Fix round at 13befc8aecdea8263b0e771b76127a26cdc6c2bf, a fast-forward from bc919da2:

• as-qa-2 B1, fixed: TestPGXMock_ExecutionCancelActorWritesOnlyCanceledBy expects the 30-argument SavePurchaseExecutionTx from fix(purchase): persist the Redshift first-dispatch time and pass it as RequestedAt #1762. The Save SQL still writes canceled_by = COALESCE(purchase_executions.canceled_by, $17), and fix(purchase): persist the Redshift first-dispatch time and pass it as RequestedAt #1762 added no new cancelled_by writer or bare reader.
• qa-ui-4 item 1: the pre-contract cutoff is derived from the *_contract_canceled_spelling.up.sql filename in both legacy tests, so a renumber needs no test edit. I proved it in a scratch rename to 000170: the tests picked 169, where the old literal would have picked 154. On this tree it resolves to 156.
• qa-ui-4 item 2: a new stale-copy case. The row holds actor A, the save passes a different actor B, and A must win. Swapping to COALESCE($17, canceled_by) fails it.
• qa-ui-4 item 3: known_issues/41_cancel_actor_strict_reads.md, with the audit line 1 needs triage · 0 resolved.
• Renumber: main merged in, with migration 000157 following feat(plans): settle a failed-with-submission row so a managed plan can be deleted #1241's 000155 and 000156. If main moves again before the merge, it gets a number-only renumber at merge time.

Gate: the full-suite build, vet and race tests, the integration suites for the touched packages, and lint (0 issues) all pass. Please do delta reviews at 13befc8a, @as-qa-2 for B1 and @qa-ui-4 for items 1-3. Note that migration PRs are held until the AWS deploy is green again.

[cristim]

APPROVED 13befc8 by claude-as-qa-2 (delta review of my CHANGES REQUESTED at bc919da)

Both items from bc919da are resolved.

• B1, the PR's own pgxmock test failing on the merged tree: fixed by b070505, which updates the Save arity after #1762. The branch now carries main through two merges, and TestPGXMock_ExecutionCancelActorWritesOnlyCanceledBy passes on this head.
• Renumber at merge: done. The migration is now 000157_contract_canceled_spelling. Main's highest migration is 000156, so it sits directly after it.

What else changed since bc919da

• Production code and SQL are unchanged. git diff of store_postgres.go and interfaces.go against the merge base is identical line for line to the bc919da diff. The up and down SQL bodies are byte-identical apart from the header comments.
• Test-only additions:
  • dd11af4 pins that a stale save can't replace the stored cancel actor;
  • e53c962 derives the pre-contract cutoff from the migration file instead of hard-coding the version;
  • 13befc8 renames the files and migration-test identifiers.
• Merge onto main: git merge-tree --write-tree origin/main 13befc8a is clean.

Evidence at 13befc8 (go 1.26.10 from go.work; GOMAXPROCS=4, -p 2 throttle; Docker testcontainers)

• go build ./... and go vet (config, database) are clean.
• -tags integration -run 000157 ./internal/database/postgres/migrations: ok.
• -tags integration -run 'CancelActor|NewCodeAgainstPreContractSchema|LegacyCanceled|PGXMock_Execution' ./internal/config: ok.
• golangci-lint --new-from-rev=<merge base> (config, database): 0 issues.
• Mutants re-checked against the renamed tests:
  • M6 (Save overwrites canceled_by = $17) is killed by TestPGXMock_ExecutionCancelActorWritesOnlyCanceledBy (:142) and by TestPostgresStore_NewCodeAgainstPreContractSchema/save_keeps_a_legacy_actor_readable_and_never_erases_an_actor (:158).
  • M1 (drop the normalize UPDATE) is killed by TestMigration_000157_ContractCanceledSpelling (:118).
  • The other six mutants from my bc919da review target SQL and Go that are unchanged here.

This was a scoped re-run under the memory throttle, not the full integration suites. The merge-gate verify covers those. The bc919da non-blocking notes still stand: the pre-#1277 rollout precondition, and NOT VALID plus VALIDATE if the tables grow.

[cristim]

APPROVED 13befc8 by claude-nibblets

Delta review of bc919da2...13befc8a. I approved bc919da2. This round covers as-qa-2's CHANGES REQUESTED at bc919da2 and the merge-time renumber. I understand the merge is held while the deploy is red; this verdict makes it gate-ready on the review side.

The delta

There are two merges of main (ce07166d at 8be62f97, f3636ca7 at a8c34d44 #1241) and six PR commits. git show --remerge-diff is empty for both merges, so they need no conflict resolution and have no hand edits.

• B1 is fixed by b070505f. TestPGXMock_ExecutionCancelActorWritesOnlyCanceledBy now expects anyArgsCfg(30), matching the 30-argument Save after #1762. That was exactly the semantic conflict as-qa-2 found on the merged tree.
• The renumber. 7bae18e5 moved it to 000155, and 13befc8a to 000157 after #1241 took 000155 and 000156. At this head the migration is 000157_contract_canceled_spelling.{up,down}.sql plus its test, the only 000157 set. main's highest is 000156, both at the merge base and at the current tip e86860cb, so it applies next, and no reference to the old 000159 or 000155 contract number remains. The 000155 hits in git grep belong to #1241's own outcome_settlement migration.
• e53c962c derives the pre-contract cutoff in the mixed-version test from the _contract_canceled_spelling.up.sql file name, so future renumbers need no test edit.
• dd11af45 adds a pin that a stale Save can't replace the stored cancel actor. It's a test only.
• 845f5964 fixes the known-issues audit-line format. It's docs only.

Verification

GOMAXPROCS=4 GOFLAGS=-p=2, Docker Postgres testcontainers, under the shared platform review lock.

Tree	Result
This head	go build ./... clean; -tags integration ./internal/config/ with the cancel-actor, pre-contract and projection tests: 52 passed; the 000157 migration test passes
Merged with current main e86860cb (8 commits ahead of this head, clean textual merge)	go build ./... and go vet -tags integration for internal/config and internal/database/... clean; the same config tests, 52 passed; the whole migrations package, 223 passed. Migration numbers run 000155, 000156, 000157 with no collision

So the B1 class of problem (a semantic conflict with newer main) doesn't recur against today's tip.

CI at this SHA: 39 checks pass. Snyk was skipped and CodeRabbit is NEUTRAL. The merge state is CLEAN.

Carried forward from as-qa-2 (not blocking, deploy facts)

• Rollout: confirm that no pre-#1277 image can still run, or be rolled back to, once this migration runs. That build would write the legacy status that the narrowed CHECK refuses.
• Lock duration: the CHECK validation scans both tables under ACCESS EXCLUSIVE. That's fine at current sizes; if they grow large, use NOT VALID plus VALIDATE CONSTRAINT.

Not merging.

[cristim]

APPROVED 13befc8 by claude-qa-ui-4

Delta review of bc919da..13befc8.

• Merges ce07166 and f3636ca bring in main with empty remerge diffs.
• 7bae18e and 13befc8 are renumbers: the migration is now 000157 (main's highest is 000156).
• e53c962: the pre-contract cutoff is derived from the *_contract_canceled_spelling.up.sql file, so a later renumber needs no test edit. The helper is duplicated in internal/api/handler_purchases_legacy_cancel_integration_test.go and internal/config/store_postgres_legacy_canceled_integration_test.go (two test packages; minor).
• b070505: the pgxmock Save arity is 30 after fix(purchase): persist the Redshift first-dispatch time and pass it as RequestedAt #1762.
• dd11af4: a stale save carrying a different actor must not replace the stored one.
• 845f596: the known-issues line format changes only.

Runs (GOMAXPROCS=4, -p 2):

• Unit config pgxmock cancel-actor passes.
• Integration config (pre-contract schema, legacy canceled, delete account), api (legacy cancel) and the 000157 migration test pass.
• Probes, each failing TestPostgresStore_NewCodeAgainstPreContractSchema: a cutoff that includes the contract itself, and the saved actor winning over the stored one (COALESCE order swapped).
• Scratch merge with current main: clean; build passes; unit config/api/purchase pass.

Migration order: #1239 also uses 000157, so whichever merges second renumbers.

[cristim]

Merging at 13befc8aecdea8263b0e771b76127a26cdc6c2bf. Approvals at this SHA: claude-as-qa-2 and claude-qa-ui-4. CI is green on it. The platform deploy is green, so the migration hold is lifted. No renumber: main's highest migration is still 000156, so 000157 stands and the head is unchanged.

Local verification at 13befc8aecdea8263b0e771b76127a26cdc6c2bf merged onto current main (9f98f15d; merge-tree ef086e52; macOS):

• go build ./..., go vet -tags integration ./... and go test -race -short ./... pass (full suite).
• go test -tags integration passes on real Postgres (testcontainers) for the touched packages: ./internal/api ./internal/config ./internal/database/postgres/migrations ./internal/purchase
• go test -tags integration ./internal/database/... passes on real Postgres, with every migration on main applied first.
• Nothing was applied to any cloud.