diff --git a/docs/branch-review-records/6a3785a84afec6df46b90f552bb0b3bcb3555d1c1b80156ecbf1c4ce17a2d357.record.md b/docs/branch-review-records/6a3785a84afec6df46b90f552bb0b3bcb3555d1c1b80156ecbf1c4ce17a2d357.record.md new file mode 100644 index 0000000000..949c7ab080 --- /dev/null +++ b/docs/branch-review-records/6a3785a84afec6df46b90f552bb0b3bcb3555d1c1b80156ecbf1c4ce17a2d357.record.md @@ -0,0 +1 @@ +| 2026-08-19 | claude/migration-history-drift-allowlist-37444c | aceb66fc936821397175aead919a47b54ee455ad | Phase 6.2 (#Q5JHBJ): six validation guard migrations 20260819110000-110500 + fifteen migration_history allowlist entries; guard test predicate refinement; forensics/board/drift-doc; production+staging applied in the authorised window; PR #2185 | Drift zero on production (live-drift 32251326536 compare step: No unexpected schema drift, all 20 history rows allowed) and staging; chain replay 210/210 CHAIN == MANIFEST; seven mutants raise; production dry-runs green and a mutant fails there; job red only on the Phase 0 Align-migration-history step (PGRST106) queued as its own P2 | verify:pr-local exit 0 (682 files / 7398 tests passed, failed none); vitest schema set 113/113; check:migration-role; check:drift --self-test; local whole-chain Docker replay + compareDriftSnapshots; production guard dry-runs + mutant; staging md5-matched Phase 2 apply + offline drift comparison | diff --git a/supabase/migrations/20260819110100_validate_history_comments_and_retention.sql b/supabase/migrations/20260819110100_validate_history_comments_and_retention.sql index f39c2fd95a..b01a41aa1a 100644 --- a/supabase/migrations/20260819110100_validate_history_comments_and_retention.sql +++ b/supabase/migrations/20260819110100_validate_history_comments_and_retention.sql @@ -26,7 +26,8 @@ declare mismatched_objects text[] := array[]::text[]; function_oid regprocedure; comment_text text; - cron_job_present boolean; + cron_schedule text; + cron_command text; begin -- 20260702100000: claim_ingestion_jobs function comment function_oid := to_regprocedure('public.claim_ingestion_jobs(text,integer,integer)'); @@ -67,11 +68,15 @@ begin -- 20260702120000: the nightly purge job, only where pg_cron is installed if to_regnamespace('cron') is not null then - execute 'select exists (select 1 from cron.job where jobname = $1)' - into cron_job_present + execute 'select schedule, command from cron.job where jobname = $1 limit 1' + into cron_schedule, cron_command using 'purge-rag-retrieval-logs'; - if not coalesce(cron_job_present, false) then + if cron_schedule is null and cron_command is null then missing_objects := array_append(missing_objects, 'cron job purge-rag-retrieval-logs'); + elsif cron_schedule <> '0 3 * * *' + or position('delete from public.rag_retrieval_logs' in cron_command) = 0 + or position('where created_at < now() - interval ''90 days''' in cron_command) = 0 then + mismatched_objects := array_append(mismatched_objects, 'cron job purge-rag-retrieval-logs'); end if; end if; diff --git a/supabase/migrations/20260819110200_validate_history_document_foreign_keys.sql b/supabase/migrations/20260819110200_validate_history_document_foreign_keys.sql index 04aa04f702..146a416580 100644 --- a/supabase/migrations/20260819110200_validate_history_document_foreign_keys.sql +++ b/supabase/migrations/20260819110200_validate_history_document_foreign_keys.sql @@ -9,7 +9,7 @@ -- Each must be a FOREIGN KEY on (document_id) referencing public.documents(id) with ON DELETE SET -- NULL -- the shape supabase/schema.sql declares and supabase/drift-manifest.json pins -- ('FOREIGN KEY (document_id) REFERENCES public.documents(id) ON DELETE SET NULL'). The check reads --- pg_constraint columns (contype / confrelid / confdeltype / conkey) rather than comparing +-- pg_constraint columns (contype / confrelid / confdeltype / conkey / confkey) rather than comparing -- pg_get_constraintdef text, so the result does not depend on the session search_path rendering -- 'documents' versus 'public.documents'. -- @@ -28,7 +28,14 @@ declare table_oid regclass; con record; document_column smallint; + documents_id_column smallint; begin + select a.attnum + into documents_id_column + from pg_attribute as a + where a.attrelid = to_regclass('public.documents') + and a.attname = 'id' + and not a.attisdropped; for required in select * from ( @@ -47,7 +54,7 @@ begin continue; end if; - select c.contype, c.confrelid, c.confdeltype, c.conkey + select c.contype, c.confrelid, c.confdeltype, c.conkey, c.confkey into con from pg_constraint as c where c.conrelid = table_oid @@ -68,7 +75,8 @@ begin if con.contype <> 'f' or con.confrelid is distinct from to_regclass('public.documents')::oid or con.confdeltype <> 'n' - or con.conkey is distinct from array[document_column]::smallint[] then + or con.conkey is distinct from array[document_column]::smallint[] + or con.confkey is distinct from array[documents_id_column]::smallint[] then mismatched_constraints := array_append(mismatched_constraints, required.constraint_name); end if; end loop; diff --git a/tests/migration-history-guards.test.ts b/tests/migration-history-guards.test.ts index dcd3f5eb5b..b0bb47d0db 100644 --- a/tests/migration-history-guards.test.ts +++ b/tests/migration-history-guards.test.ts @@ -64,16 +64,96 @@ function stripSql(sql: string): string { } /** - * Executable SQL only: comments and single-quoted string literals removed. A - * validation guard pins the canonical `create index … on …` text of the objects - * it proves as a string literal (the 20260804110240 pattern), which is data, not - * a statement — the create-index check must not mistake it for a build. + * Executable SQL only: comments and single-quoted string literals removed, except + * when passed dynamically to EXECUTE. A validation guard pins the canonical + * `create index … on …` text of the objects it proves as a data literal (the + * 20260804110240 pattern), which must not be mistaken for a build; but dynamic + * EXECUTE of CREATE INDEX statements must still be caught. */ function executableSql(sql: string): string { - return stripSql(sql).replace(/'(?:[^']|'')*'/g, "''"); + const stripped = stripSql(sql); + let out = ""; + let inExecute = false; + let i = 0; + + while (i < stripped.length) { + // Check for string literal + if (stripped[i] === "'") { + let literal = ""; + i++; // skip opening quote + while (i < stripped.length) { + if (stripped[i] === "'") { + if (stripped[i + 1] === "'") { + literal += "'"; + i += 2; + } else { + i++; // skip closing quote + break; + } + } else { + literal += stripped[i]; + i++; + } + } + + if (inExecute) { + out += " " + literal + " "; + } else { + out += "''"; + } + continue; + } + + // Check for dollar-quoted string opening $tag$ + if (stripped[i] === "$") { + const match = stripped.slice(i).match(/^\$[a-zA-Z0-9_]*\$/); + if (match) { + const tag = match[0]; + inExecute = false; + out += tag; + i += tag.length; + continue; + } + } + + // Check for dynamic EXECUTE keyword (excluding GRANT/REVOKE EXECUTE and EXECUTE FUNCTION/PROCEDURE) + const rest = stripped.slice(i); + const executeMatch = rest.match(/^execute\b(?!\s+(?:function|procedure)\b)/i); + if (executeMatch) { + const prefix = stripped.slice(Math.max(0, i - 15), i); + if (!/\b(?:grant|revoke)\s+$/i.test(prefix)) { + inExecute = true; + } + out += rest.slice(0, executeMatch[0].length); + i += executeMatch[0].length; + continue; + } + + // If inside EXECUTE, check for clause terminators or string concatenation + if (inExecute) { + if (rest.match(/^\|\|/)) { + out += " "; + i += 2; + continue; + } + const termMatch = rest.match(/^(?:into\b|using\b|;|end\b)/i); + if (termMatch) { + inExecute = false; + out += rest.slice(0, termMatch[0].length); + i += termMatch[0].length; + continue; + } + } + + out += stripped[i]; + i++; + } + + return out; } -const CREATE_INDEX_STATEMENT = /create\s+(?:unique\s+)?index\s+(?:concurrently\s+)?(?:if\s+not\s+exists\s+)?[a-z_]/i; +const CREATE_INDEX_STATEMENT = + /create\s+(?:unique\s+)?index\s+(?:concurrently\s+)?(?:if\s+not\s+exists\s+)?(?:[a-z0-9_%"]|\$)/i; describe("migration-history probe and guard-migration contract", () => { it("the v2 snapshot migration exists and check:drift knows its name", () => { @@ -159,6 +239,21 @@ describe("migration-history probe and guard-migration contract", () => { expect(executableSql(`${reference}\ncreate index if not exists oops_idx on public.documents(id);`)).toMatch( CREATE_INDEX_STATEMENT, ); + expect( + executableSql( + `${reference}\ndo $$ begin execute 'create index if not exists oops_idx on public.documents(id);'; end $$;`, + ), + ).toMatch(CREATE_INDEX_STATEMENT); + expect( + executableSql( + `${reference}\ndo $$ begin execute format('create index %I on public.documents(id)', 'oops_idx'); end $$;`, + ), + ).toMatch(CREATE_INDEX_STATEMENT); + expect( + executableSql( + `${reference}\ndo $$ begin execute 'create ' || 'unique index concurrently if not exists oops_idx on public.documents(id);'; end $$;`, + ), + ).toMatch(CREATE_INDEX_STATEMENT); }); it("no pre-contract class is used for a version at or after the contract date", () => {