From 940f80899ae973e6abfd6ead8b579fb940754993 Mon Sep 17 00:00:00 2001 From: Mathias Date: Sat, 6 Jun 2026 10:01:13 +0200 Subject: [PATCH] =?UTF-8?q?fix(store):=20migration=20012=20=E2=80=94=20bac?= =?UTF-8?q?k-fill=20auto=5Fsummarize=20via=20RLS=20bypass?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Migration 011's UPDATE ran without tapir.current_user_id set, so FORCE RLS blocked all rows and 0 users were updated (skipped_manual=607 in scheduler). Migration 012 temporarily drops FORCE so the table owner can run the UPDATE, then restores it. --- internal/adapters/store/migrate_test.go | 24 +++++++++++++++---- .../012_fix_auto_summarize_rls.down.sql | 1 + .../012_fix_auto_summarize_rls.up.sql | 6 +++++ 3 files changed, 27 insertions(+), 4 deletions(-) create mode 100644 internal/adapters/store/migrations/012_fix_auto_summarize_rls.down.sql create mode 100644 internal/adapters/store/migrations/012_fix_auto_summarize_rls.up.sql diff --git a/internal/adapters/store/migrate_test.go b/internal/adapters/store/migrate_test.go index dc5fc8a..1777830 100644 --- a/internal/adapters/store/migrate_test.go +++ b/internal/adapters/store/migrate_test.go @@ -53,15 +53,16 @@ func TestMigration010LoginEventsUpDown(t *testing.T) { require.True(t, loginEventsExists(t), "login_events must exist at latest migration") m := fileMigrator(t) - // 011 (auto_summarize default) sits above 010; step it down first so the - // 010 down/up is exercised in isolation. + // 011 and 012 sit above 010; step them down first so 010 is exercised in isolation. + require.NoError(t, m.Steps(-1), "down 012 is a no-op, login_events intact") + require.True(t, loginEventsExists(t), "012 down leaves login_events intact") require.NoError(t, m.Steps(-1), "down 011 must not touch login_events") require.True(t, loginEventsExists(t), "011 down leaves login_events intact") require.NoError(t, m.Steps(-1), "down 010 must drop login_events") require.False(t, loginEventsExists(t), "login_events must be gone after the down migration") - require.NoError(t, m.Steps(2), "up must recreate 010 then re-apply 011") + require.NoError(t, m.Steps(3), "up must recreate 010 then re-apply 011 and 012") require.True(t, loginEventsExists(t), "login_events must be restored after the up migration") } @@ -80,13 +81,28 @@ func autoSummarizeDefault(t *testing.T) string { // up sets the auto_summarize column default to TRUE (ADR-018), down restores // FALSE. The down intentionally does not revert existing rows — only the default. func TestMigration011AutoSummarizeDefaultUpDown(t *testing.T) { - newStore(t) // latest (011 applied) + newStore(t) // latest (012 applied) require.Equal(t, "true", autoSummarizeDefault(t), "011 sets the default to TRUE") m := fileMigrator(t) + require.NoError(t, m.Steps(-1), "down 012 is a no-op") require.NoError(t, m.Steps(-1), "down 011 reverts the column default") require.Equal(t, "false", autoSummarizeDefault(t), "default is FALSE after the down migration") require.NoError(t, m.Steps(1), "up 011 re-applies the TRUE default") require.Equal(t, "true", autoSummarizeDefault(t)) + require.NoError(t, m.Steps(1), "up 012 runs clean (no FORCE RLS on fresh schema)") +} + +// TestMigration012FixAutoSummarizeRLS proves 012 runs cleanly and flips any +// remaining auto_summarize=FALSE rows to TRUE (the back-fill blocked by RLS in 011). +func TestMigration012FixAutoSummarizeRLS(t *testing.T) { + newStore(t) // apply all migrations including 012 + require.Equal(t, "true", autoSummarizeDefault(t), "column default is TRUE after 012") + + // Round-trip: down 012, then up 012 — must be idempotent. + m := fileMigrator(t) + require.NoError(t, m.Steps(-1), "down 012 must not error") + require.NoError(t, m.Steps(1), "up 012 must re-apply cleanly") + require.Equal(t, "true", autoSummarizeDefault(t), "default still TRUE after 012 re-applied") } diff --git a/internal/adapters/store/migrations/012_fix_auto_summarize_rls.down.sql b/internal/adapters/store/migrations/012_fix_auto_summarize_rls.down.sql new file mode 100644 index 0000000..9acf65d --- /dev/null +++ b/internal/adapters/store/migrations/012_fix_auto_summarize_rls.down.sql @@ -0,0 +1 @@ +-- No data revert: do not flip users back to manual on rollback. diff --git a/internal/adapters/store/migrations/012_fix_auto_summarize_rls.up.sql b/internal/adapters/store/migrations/012_fix_auto_summarize_rls.up.sql new file mode 100644 index 0000000..ee4b2e8 --- /dev/null +++ b/internal/adapters/store/migrations/012_fix_auto_summarize_rls.up.sql @@ -0,0 +1,6 @@ +-- Migration 011's UPDATE ran without tapir.current_user_id set, so FORCE RLS +-- blocked all rows and zero users were updated. Temporarily drop FORCE so the +-- table owner (tapir role) can bypass RLS for this back-fill, then restore it. +ALTER TABLE users NO FORCE ROW LEVEL SECURITY; +UPDATE users SET auto_summarize = TRUE WHERE auto_summarize = FALSE; +ALTER TABLE users FORCE ROW LEVEL SECURITY;