[IAmJSD]

Root cause

purchase_history.ramp_step (INTEGER) and purchase_executions.error (TEXT) have been nullable since migration 000001, but the Go readers scan them into plain values: ramp_step into PurchaseHistoryRecord.RampStep int (scanPurchaseHistoryRow and the other history readers), error into a string (tolerated through sql.NullString since #1473). No Go writer stores NULL in either column (both are bound from a Go int / string), so a NULL can only come from a legacy or hand-written row. One such purchase_history row fails the scan of the whole listing (GetAllPurchaseHistory returns a scan error).

Fix

Migration 000146_history_ramp_step_execution_error_not_null:

• LOCK TABLE purchase_executions, purchase_history IN ACCESS EXCLUSIVE MODE first, in one statement;
• backfills ramp_step NULL to 0 and error NULL to '' (the values the Go zero values already mean);
• SET DEFAULT 0 / SET DEFAULT '' and SET NOT NULL on both columns.

Down drops the NOT NULL and the default on both columns. The backfilled 0 / '' stay, which is what the readers already returned for NULL.

Two details the reviewers will want to check:

• Concurrency. The file runs as one implicit transaction. Without the up-front lock, a writer still on an older build (or a hand-written INSERT) with an uncommitted NULL insert is invisible to the backfill UPDATE (both take ROW EXCLUSIVE, so nothing blocks). SET NOT NULL then waits for that writer, sees the committed NULL, and fails the whole migration, leaving schema_migrations dirty. With the lock taken first, the migration waits for the writer, then backfills its row. Taking ACCESS EXCLUSIVE up front also avoids upgrading the lock from ROW EXCLUSIVE to ACCESS EXCLUSIVE partway through. TestMigration146_ConcurrentNullWriter covers this race on real Postgres (see the probe below).
• updated_at. update_purchase_executions_updated_at would stamp NOW() on every backfilled execution. updated_at is a lifecycle clock: plan health windows failed/canceled counts on it, and CleanupOldExecutions uses it to age canceled rows (see 000128). So the trigger is disabled for the backfill statement only, the same pattern as 000127 and 000145. Because the file is one transaction, a failure rolls back the disable too. purchase_history has no UPDATE trigger.

The existing #1465 regression test (TestIntegration_ExecutionReaders_NullErrorColumn) seeded a NULL with UPDATE ... SET error = NULL, which the new constraint rejects (23502). It now runs ALTER COLUMN error DROP NOT NULL in its own per-test database first, so it still covers the scanner's NULL tolerance on a schema rolled back below 000146.

Closes #1484

Verification

All commands were run in the worktree with AWS_CONFIG_FILE=/dev/null AWS_SHARED_CREDENTIALS_FILE=/dev/null AWS_EC2_METADATA_DISABLED=true. Integration evidence comes from real PostgreSQL 16 (testcontainers, shared container with a per-test database). No cloud APIs were called.

• go build ./..., go vet ./..., go vet -tags integration ./...: clean.
• go test -count=1 ./internal/database/postgres/migrations/ (unit, including the transactional-safety scan): ok.
• go test -count=1 ./internal/config/: ok. go test ./internal/mocks/ -count=1: ok.
• golangci-lint run --allow-parallel-runners --build-tags integration ./internal/database/postgres/migrations/ ./internal/config/ (v2.10.1): 0 issues. No non-test Go files changed, so gocyclo has nothing to check.
• go test -tags integration -count=1 -timeout 20m -run 'TestMigration146_' ./internal/database/postgres/migrations/: both PASS.
  • TestMigration146_BackfillsAndEnforcesNotNull: at version 145 it seeds a NULL ramp_step row, a ramp_step = 3 row, a NULL-error execution and an error = 'boom' execution. Fail-before (asserted as a precondition): at 145, GetAllPurchaseHistory returns a scan error. After 146: NULL becomes 0 / '', the set values are unchanged, the backfilled execution's updated_at is unchanged, GetAllPurchaseHistory returns both rows, both columns are NOT NULL with defaults 0 / ''::text, an insert that omits the columns gets the defaults, an explicit NULL fails with 23502, the trigger is enabled again (tgenabled = 'O') and stamps a later update, and re-running the up file succeeds. Down to 145 makes both columns nullable with no default, NULL inserts work again and existing rows survive.
  • TestMigration146_ConcurrentNullWriter: an open transaction inserts NULL into both tables, the migration starts and blocks (a non-granted pg_locks entry on one of the tables), the writer commits, and the migration succeeds with the row backfilled.
• go test -tags integration -count=1 -timeout 20m -run 'TestIntegration_ExecutionReaders_NullErrorColumn' ./internal/config/: PASS.

Mutation probes (each change reverted afterwards):

Probe	Result
Remove the LOCK TABLE line	ConcurrentNullWriter FAILS: failed to migrate to version 146: migration failed: column "ramp_step" of relation "purchase_history" contains null values
Remove the trigger DISABLE/ENABLE	BackfillsAndEnforcesNotNull FAILS: the backfill must not stamp updated_at: got 19:14:05, want 18:14:05
Leave the trigger disabled (no ENABLE)	BackfillsAndEnforcesNotNull FAILS: the updated_at trigger is enabled after the migration / a real update stamps updated_at again
Revert the #1465 test edit	TestIntegration_ExecutionReaders_NullErrorColumn FAILS with SQLSTATE 23502 on SET error = NULL

Combined-tree verification at local queue commit 89fe5df6 = origin/main 3bfc2fe5 + fix/issue-1484, fix/issue-1498, fix/issue-1489 and fix/issue-1494 merged in that order. Passed: go build ./...; go vet with and without -tags integration; go test -race -short -count=1 ./...; golangci-lint v2.10.1 with and without the integration tag; gocyclo -over 10; go test -race -tags integration on internal/api, internal/config and internal/database/postgres/migrations; the git-secrets scan and scripts/test-git-secrets-allowlist.sh.

Out of scope

• The issue suggests scanning ramp_step through sql.NullInt32 only until the migration lands. This PR lands the migration, so the history scanners are unchanged; after 000146 they can no longer see a NULL. The error scanner keeps its fix(config): tolerate a NULL error when scanning purchase executions #1473 NULL tolerance.
• Other nullable columns read into plain Go values were not audited. This PR covers only the two columns named in the issue.

Frontend follow-up for the UI owner

None. API responses are unchanged: ramp_step was already 0 / omitted and error "" / omitted for these rows.

Maintainer: please mirror the issue's triage labels on this PR: priority/p3, severity/low, urgency/this-quarter, impact/few, effort/xs, type/bug (the issue has no triaged label).

🤖 Generated with Claude Code

[coderabbitai]

Caution

Review failed

The pull request is closed.

Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

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

_{Comment @coderabbitai help to get the list of available commands.}

[IAmJSD]

Independent review (IAmJSD's agent, fresh context, given only the diff, the issue and the PR text): VERDICT: APPROVE e0f53a0bde7d11c575c09943518b265462c03811

Blocking:

• none

Non-blocking:

• 000146 up.sql:45, lock order. LOCK TABLE purchase_executions, purchase_history takes the locks in that order. An app transaction that writes purchase_history and then purchase_executions while the migration sits between its two locks would deadlock. Postgres aborts one side, and if the migration is the one aborted, schema_migrations is left dirty. The common pattern (execution first, then history) matches the migration's order, and the window is only the start of a rolling deploy, so this is a theoretical risk, not a defect. Mentioned so whoever runs the deploy knows a dirty 146 after a deadlock is safe to force and re-run: the file is idempotent.
• The new ACCESS EXCLUSIVE holds both tables for one SET NOT NULL scan each. That is fine at today's table sizes. On a very large purchase_executions, ADD CONSTRAINT ... CHECK (error IS NOT NULL) NOT VALID + VALIDATE would avoid the long lock. Not needed now.

Attacked:

• Numbering: 000146 is the number reserved for fix(db): make purchase_history.ramp_step and purchase_executions.error NOT NULL #1484 (main is at 000145).
• Rolling deploy: I checked every Go writer of these two columns (store_postgres_history_save.go binds record.RampStep int; SavePurchaseExecution, ramp_target_met and the others bind a Go string; the error = $N writes in ri_exchange_* are on another table). None binds NULL, so old binaries keep working after the constraint. No SQL function or trigger in the migrations writes NULL either.
• Grepped every test that seeds these tables with NULL. Only the fix(history): one execution row with a NULL error hides every actionable execution #1465 test does, and the PR adjusts it.
• ramp_step = 0 is what non-plan writers already store, and nothing in SQL filters on ramp_step, so the backfill value doesn't change any meaning.
• Ran: go build ./...; go vet with and without -tags integration on internal/database/... and internal/config; go test -race -short -count=1 on migrations and config (ok).
• Integration runs:
  • the full migrations package (go test -race -tags integration ./internal/database/postgres/migrations/, ok, so every older migration test still passes on top of 146)
  • the full config package (go test -race -tags integration ./internal/config/, ok)
  • the full purchase package (go test -race -tags integration ./internal/purchase/, ok)
• Fail-before: the 145 precondition in BackfillsAndEnforcesNotNull asserts that GetAllPurchaseHistory fails to scan. I also deleted the LOCK TABLE line and the trigger DISABLE/ENABLE in a scratch copy. ConcurrentNullWriter then fails with "column ramp_step ... contains null values", and BackfillsAndEnforcesNotNull fails with "the backfill must not stamp updated_at". Both pass on the real file.

Body-stale: none found.

_{Reviewer: rF1. Bar while the maintainers are away (set by IAmJSD as acting owner): one approving independent review on the head, two for money, auth and security paths.}

[IAmJSD]

Local verification at e0f53a0bde7d11c575c09943518b265462c03811 merged onto main (30f5602b), together with #1505, #1508 in that order (combined tree 7b4062ee, Linux, Go 1.26.10): go build ./..., go vet with and without -tags integration, the full go test -race -short -count=1 ./..., and go test -race -tags integration -count=1 on every changed package, against Docker Postgres. All pass. CI Success and pre-commit are green on this head, with 1 approving independent review(s) on it. Nothing was run against any cloud.

[IAmJSD]

Merged by IAmJSD's agent at IAmJSD's request (IAmJSD is acting owner while the maintainers are away), under the bar IAmJSD set: 1 approving independent review(s) on e0f53a0b, green CI, and the combined-tree verification above. Squash-merged at exactly e0f53a0bde7d11c575c09943518b265462c03811.