fix(doctor,migrate): detect and repair a missing pages(source_id, slug) unique index (#550)

put_page upserts with ON CONFLICT (source_id, slug) on both engines. When
the arbiter unique index is missing, every write fails with "no unique or
exclusion constraint matching the ON CONFLICT specification" while reads
stay green — and there was no recovery path: initSchema() is additive-only
and apply-migrations --force-schema re-runs from the current version (a
no-op at head). The root cause is that migration v23 (Postgres) / v21
(PGLite) guarded the ADD CONSTRAINT on pg_constraint.conname — a NAME
match, not a shape match.

Fix:
- src/core/pages-unique-repair.ts: shared column-shape probe over pg_index
  (non-partial, non-expression unique index with key columns exactly
  {source_id, slug}, either order, constraint or bare index — what ON
  CONFLICT inference actually accepts), check + repair helpers, and the
  shape-guarded DDL shared by the migrations and the self-heal. Falls back
  to a different constraint name when pages_source_slug_key is taken by a
  wrong-shaped relation; never auto-deletes duplicate pages.
- src/core/migrate.ts: v23/v21 guards now use the shared column-based DDL;
  runMigrations runs the repair on EVERY pass (same always-run slot as the
  #2038 timeline heal), so a wedged brain heals on the next init/migrate.
- src/commands/doctor.ts: new pages_unique_index check (BRAIN category,
  wired into both buildChecks and doctorReportRemote) that FAILs with the
  literal repair SQL; OK/skip when pages or source_id doesn't exist yet.
- test/pages-unique-repair.test.ts: reproduction, migrate-pass heal
  (fails behaviorally on master), shape-not-name detection, partial-index
  rejection, name-collision fallback, idempotency, fresh-brain skip.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Garry Tan
2026-07-28 13:11:43 -07:00
co-authored by Claude Opus 5
parent 6920744dd8
commit e7083aaca2
7 changed files with 411 additions and 17 deletions
File diff suppressed because one or more lines are too long
+65
View File
@@ -3,6 +3,7 @@ import { REPAIR_SOURCE_CONFIG_SQL } from '../core/source-config-sql.ts';
import { setCliExitVerdict } from '../core/cli-force-exit.ts';
import * as db from '../core/db.ts';
import { LATEST_VERSION, getIdleBlockers } from '../core/migrate.ts';
import { checkPagesUniqueIndex, PAGES_UNIQUE_REPAIR_SQL } from '../core/pages-unique-repair.ts';
import { checkResolvable } from '../core/check-resolvable.ts';
import { autoFixDryViolations, type AutoFixReport, type FixOutcome } from '../core/dry-fix.ts';
import { autoDetectSkillsDirReadOnly } from '../core/repo-root.ts';
@@ -532,6 +533,59 @@ export async function childTableOrphansCheck(engine: BrainEngine): Promise<Check
};
}
/**
* #550 put_page writability probe.
*
* Both engines upsert pages with `ON CONFLICT (source_id, slug)`. Inference
* needs a NON-PARTIAL unique index (constraint-backed or plain unique index)
* whose key columns are exactly {source_id, slug}. Migration v23's original
* guard matched the constraint NAME (`pg_constraint.conname`), so a brain
* stamped past v23 without that DDL executing is permanently write-dead
* every put_page throws while reads stay green, and neither `initSchema()`
* nor `apply-migrations --force-schema` used to bring the index back.
*
* Probes BY COLUMNS (shape), never by name, and accepts either a unique
* constraint or a bare unique index ON CONFLICT accepts both. FAILs loudly
* with the literal repair SQL. OK (not fail) when the pages table or
* source_id column doesn't exist yet: a fresh/pre-v21 brain is not broken.
*/
export async function pagesUniqueIndexCheck(engine: BrainEngine): Promise<Check> {
try {
const status = await checkPagesUniqueIndex(engine);
if (!status.tablePresent) {
return { name: 'pages_unique_index', status: 'ok', message: 'no pages table yet' };
}
if (!status.sourceIdPresent) {
return {
name: 'pages_unique_index',
status: 'ok',
message: 'pages.source_id not present yet (pre-v21 schema) — pending migrations own this',
};
}
if (status.satisfied) {
return {
name: 'pages_unique_index',
status: 'ok',
message: `put_page upsert arbiter present (${status.indexName})`,
};
}
return {
name: 'pages_unique_index',
status: 'fail',
message:
'No non-partial UNIQUE index on pages(source_id, slug) — every put_page write fails with ' +
'"no unique or exclusion constraint matching the ON CONFLICT specification" (#550). ' +
`Fix (run against the brain database): ${PAGES_UNIQUE_REPAIR_SQL}`,
};
} catch (e) {
return {
name: 'pages_unique_index',
status: 'warn',
message: `Could not check pages unique index: ${e instanceof Error ? e.message : String(e)}`,
};
}
}
/**
* Raw-source persistence guarantee (#1978, warn-only v1).
*
@@ -701,6 +755,11 @@ export async function doctorReportRemote(engine: BrainEngine): Promise<DoctorRep
checks.push({ name: 'timeline_dedup_index', status: 'warn', message: 'Could not check idx_timeline_dedup shape' });
}
// 2c. #550: put_page writability — cross-surface parity with buildChecks.
// Same drift class as 2b, but on pages(source_id, slug): missing arbiter
// index means every put_page fails while reads look healthy.
checks.push(await pagesUniqueIndexCheck(engine));
// v0.42.x — Life Chronicle (#2390): orphaned event projections. Reads already
// hide projections whose event page is soft-deleted (read-time correctness);
// this always-run probe surfaces the cleanup backlog. Keyed off the real
@@ -6549,6 +6608,12 @@ export async function buildChecks(
progress.heartbeat('child_table_orphans');
checks.push(await childTableOrphansCheck(engine));
// 10c'. put_page writability (#550). A missing pages(source_id, slug)
// unique index kills EVERY write with an ON CONFLICT inference error while
// reads stay green — the one failure mode a version counter can't see.
progress.heartbeat('pages_unique_index');
checks.push(await pagesUniqueIndexCheck(engine));
// 10d. Raw-source persistence guarantee (#1978, warn-only v1).
// Every synthesized/derived page must carry a raw trace or an explicit
// exemption. Warn-only in v1 — surfaces violations, blocks nothing.
+1
View File
@@ -98,6 +98,7 @@ export const BRAIN_CHECK_NAMES: ReadonlySet<string> = new Set([
'ocr_health',
'orphan_ratio',
'oversized_pages',
'pages_unique_index',
'quarantined_pages',
'raw_provenance',
'flagged_pages',
+21 -16
View File
@@ -2,6 +2,7 @@ import type { BrainEngine } from './engine.ts';
import { slugifyPath } from './sync.ts';
import { getFtsLanguage } from './fts-language.ts';
import { hnswMaxDimsForType } from './vector-index.ts';
import { PAGES_SOURCE_SLUG_UNIQUE_DDL } from './pages-unique-repair.ts';
/**
* Schema migrations run automatically on initSchema().
@@ -618,16 +619,12 @@ export const MIGRATIONS: Migration[] = [
// 0b. Swap pages.UNIQUE(slug) → UNIQUE(source_id, slug).
// Deferred from v21 so PR #356 closes the integrity
// window. PGLite already did this swap in its v21 path.
// #550: guarded by index SHAPE (columns), not constraint NAME —
// the old conname guard let a wrong-shaped or missing index pass
// as "done", leaving put_page's ON CONFLICT permanently broken.
await tx.runMigration(23, `
ALTER TABLE pages DROP CONSTRAINT IF EXISTS pages_slug_key;
DO $$ BEGIN
IF NOT EXISTS (
SELECT 1 FROM pg_constraint WHERE conname = 'pages_source_slug_key'
) THEN
ALTER TABLE pages ADD CONSTRAINT pages_source_slug_key
UNIQUE (source_id, slug);
END IF;
END $$;
${PAGES_SOURCE_SLUG_UNIQUE_DDL}
`);
// 1a. source_id with DEFAULT 'default' (idempotent)
@@ -775,14 +772,7 @@ export const MIGRATIONS: Migration[] = [
CREATE INDEX IF NOT EXISTS idx_pages_source_id ON pages(source_id);
ALTER TABLE pages DROP CONSTRAINT IF EXISTS pages_slug_key;
DO $$ BEGIN
IF NOT EXISTS (
SELECT 1 FROM pg_constraint WHERE conname = 'pages_source_slug_key'
) THEN
ALTER TABLE pages ADD CONSTRAINT pages_source_slug_key
UNIQUE (source_id, slug);
END IF;
END $$;
${PAGES_SOURCE_SLUG_UNIQUE_DDL}
`,
},
},
@@ -6082,6 +6072,21 @@ export async function runMigrations(engine: BrainEngine): Promise<{ applied: num
}
} catch { /* best-effort; doctor reports the drift if this couldn't run */ }
// #550: same drift class for pages(source_id, slug). v23's old guard matched
// the constraint NAME, so a brain stamped >= v23 whose constraint never
// materialized (or was later dropped) is permanently write-dead — every
// put_page ON CONFLICT (source_id, slug) fails while reads look healthy.
// Shape-keyed, every-pass, idempotent; skips pre-v21 schemas (no source_id).
try {
const { repairPagesUniqueIndex } = await import('./pages-unique-repair.ts');
const r = await repairPagesUniqueIndex(engine);
if (r.repaired) {
console.error(
`[migrate] healed missing pages(source_id, slug) unique index (#550) — put_page writes restored`,
);
}
} catch { /* best-effort; the pages_unique_index doctor check reports it */ }
if (pending.length === 0) {
return { applied: 0, current };
}
+135
View File
@@ -0,0 +1,135 @@
/**
* #550 — pages(source_id, slug) unique-index drift detection + self-heal.
*
* Both engines' `putPage` upsert with `ON CONFLICT (source_id, slug)`. That
* inference only succeeds when a NON-PARTIAL unique index (constraint-backed
* or plain `CREATE UNIQUE INDEX`) exists whose key columns are exactly
* {source_id, slug}. When it's missing, EVERY put_page write fails with
* "there is no unique or exclusion constraint matching the ON CONFLICT
* specification" while reads keep working — the brain looks healthy and is
* write-dead.
*
* How the index goes missing: migration v23 (Postgres) / v21 (PGLite) guarded
* the `ADD CONSTRAINT pages_source_slug_key` on `pg_constraint.conname` — a
* NAME match, not a shape match. Any brain stamped >= v23 without that DDL
* actually executing (fresh-init blob stamped to head, externally-created
* `pages`, a later drop/rename) never gets it back: `runMigrations` early
* returns at head and `initSchema()` is additive-only. Same class as #2038
* (idx_timeline_dedup), so the repair follows the same pattern: keyed off the
* actual index SHAPE, run on every migrate pass, idempotent.
*/
import type { BrainEngine } from './engine.ts';
/**
* Selects the name of every index that satisfies `ON CONFLICT (source_id,
* slug)` inference on `pages`:
* - unique
* - non-partial (a partial unique index does NOT satisfy inference without
* a matching WHERE clause, which putPage doesn't emit)
* - plain-column (no expression keys)
* - key columns exactly {source_id, slug}, in any order
*
* Matched BY COLUMNS, never by name — a correctly-named but wrong-shaped
* index must not count. Shared verbatim by the doctor probe, the repair
* helper, and the v21/v23 migration guards (embedded in `EXISTS (...)`).
* `i.indkey` also lists INCLUDE columns, so with `indnkeyatts = 2` the
* aggregate has exactly 2 names only when there are no INCLUDE columns —
* conservative: an INCLUDE-bearing index is treated as not satisfying.
*/
export const PAGES_SOURCE_SLUG_UNIQUE_PROBE_SQL = `
SELECT c.relname
FROM pg_index i
JOIN pg_class t ON t.oid = i.indrelid
JOIN pg_class c ON c.oid = i.indexrelid
WHERE t.relname = 'pages'
AND i.indisunique
AND i.indpred IS NULL
AND i.indexprs IS NULL
AND i.indnkeyatts = 2
AND (
SELECT array_agg(a.attname::text ORDER BY a.attname)
FROM pg_attribute a
WHERE a.attrelid = t.oid
AND a.attnum = ANY (i.indkey)
) = ARRAY['slug', 'source_id']
`;
/** The literal operator repair — doctor prints this verbatim on FAIL. */
export const PAGES_UNIQUE_REPAIR_SQL =
'ALTER TABLE pages ADD CONSTRAINT pages_source_slug_key UNIQUE (source_id, slug);';
/**
* The guard DDL shared by migration v23 (Postgres) and v21 (PGLite) and by
* the every-pass self-heal. Column-shape-guarded, so:
* - a satisfying index under ANY name → no-op (no duplicate constraint)
* - the canonical name taken by a wrong-shaped relation → falls back to
* `pages_source_slug_uniq` instead of failing (ON CONFLICT inference is
* by shape; the name is not load-bearing)
* Does NOT dedupe pages first: if unconstrained duplicates snuck in, ADD
* CONSTRAINT fails naming the duplicated key — deleting pages automatically
* would be data loss, so that call stays with the operator.
*/
export const PAGES_SOURCE_SLUG_UNIQUE_DDL = `
DO $$ BEGIN
IF NOT EXISTS (${PAGES_SOURCE_SLUG_UNIQUE_PROBE_SQL}) THEN
IF EXISTS (SELECT 1 FROM pg_class WHERE relname = 'pages_source_slug_key') THEN
ALTER TABLE pages ADD CONSTRAINT pages_source_slug_uniq UNIQUE (source_id, slug);
ELSE
ALTER TABLE pages ADD CONSTRAINT pages_source_slug_key UNIQUE (source_id, slug);
END IF;
END IF;
END $$;
`;
export interface PagesUniqueStatus {
/** The pages table exists (nothing to check on a fresh brain). */
tablePresent: boolean;
/** pages.source_id exists (pre-v21 schemas are the migration chain's job). */
sourceIdPresent: boolean;
/** A satisfying unique index exists — put_page's ON CONFLICT works. */
satisfied: boolean;
/** Name of the first satisfying index (null when unsatisfied). */
indexName: string | null;
}
export async function checkPagesUniqueIndex(engine: BrainEngine): Promise<PagesUniqueStatus> {
const tbl = await engine.executeRaw<{ reg: string | null }>(
`SELECT to_regclass('pages')::text AS reg`,
);
if (!tbl[0]?.reg) {
return { tablePresent: false, sourceIdPresent: false, satisfied: false, indexName: null };
}
const col = await engine.executeRaw<{ n: number }>(
`SELECT count(*)::int AS n FROM information_schema.columns
WHERE table_name = 'pages' AND column_name = 'source_id'`,
);
if (Number(col[0]?.n ?? 0) === 0) {
return { tablePresent: true, sourceIdPresent: false, satisfied: false, indexName: null };
}
const rows = await engine.executeRaw<{ relname: string }>(PAGES_SOURCE_SLUG_UNIQUE_PROBE_SQL);
return {
tablePresent: true,
sourceIdPresent: true,
satisfied: rows.length > 0,
indexName: rows[0]?.relname ?? null,
};
}
export interface PagesUniqueRepairResult {
repaired: boolean;
reason: 'already_correct' | 'no_table' | 'pre_source_id' | 'added';
}
/**
* Heal the missing unique index. Idempotent; a no-op on healthy brains and on
* pre-v21 schemas (no source_id yet — the ordinary migration chain owns those).
*/
export async function repairPagesUniqueIndex(engine: BrainEngine): Promise<PagesUniqueRepairResult> {
const status = await checkPagesUniqueIndex(engine);
if (!status.tablePresent) return { repaired: false, reason: 'no_table' };
if (!status.sourceIdPresent) return { repaired: false, reason: 'pre_source_id' };
if (status.satisfied) return { repaired: false, reason: 'already_correct' };
await engine.executeRaw(PAGES_SOURCE_SLUG_UNIQUE_DDL);
return { repaired: true, reason: 'added' };
}
+4 -1
View File
@@ -333,7 +333,10 @@ describe('migrate v23 — files_source_id_page_id_ledger', () => {
expect(body).toContain('engine.transaction');
expect(body).toContain('files_page_slug_fkey');
expect(body).toContain('pages_slug_key');
expect(body).toContain('pages_source_slug_key');
// #550: the UNIQUE(source_id, slug) swap now routes through the shared
// column-shape-guarded DDL (src/core/pages-unique-repair.ts), so the
// handler body carries the interpolation marker instead of the literal.
expect(body).toContain('PAGES_SOURCE_SLUG_UNIQUE_DDL');
});
test('v23 backfills files.page_id scoped to default source (Codex fix)', () => {
+184
View File
@@ -0,0 +1,184 @@
/**
* #550 — pages(source_id, slug) unique-index drift: detection + self-heal.
*
* A brain stamped >= v23 whose `pages_source_slug_key` constraint never
* materialized (or was later dropped) fails EVERY put_page with "no unique or
* exclusion constraint matching the ON CONFLICT specification" while reads
* stay green. The old v21/v23 guards matched the constraint NAME, so
* re-running migrations could never repair it, and `initSchema()` was a
* no-op. These tests pin: the reproduction, the every-pass migrate self-heal
* (the behavioral fix — fails on master), shape-not-name detection (partial
* indexes rejected, differently-named satisfying indexes accepted), the
* name-collision fallback, and the doctor check's literal repair SQL.
*/
import { describe, test, expect, beforeAll, afterAll } from 'bun:test';
import { PGLiteEngine } from '../src/core/pglite-engine.ts';
import { runMigrations } from '../src/core/migrate.ts';
import {
checkPagesUniqueIndex,
repairPagesUniqueIndex,
PAGES_UNIQUE_REPAIR_SQL,
} from '../src/core/pages-unique-repair.ts';
import type { BrainEngine } from '../src/core/engine.ts';
// Dynamic so the module loads on a master checkout too — the discrimination
// proof for #550 runs this file against unmodified master, where doctor.ts
// has no pagesUniqueIndexCheck export and runMigrations has no heal.
async function doctorCheck(eng: BrainEngine) {
const doctor = await import('../src/commands/doctor.ts');
if (typeof doctor.pagesUniqueIndexCheck !== 'function') {
throw new Error('doctor.ts does not export pagesUniqueIndexCheck (#550 check missing)');
}
return doctor.pagesUniqueIndexCheck(eng);
}
let engine: PGLiteEngine;
/** Schema-less engine (no initSchema) — the fresh-brain / no-pages-table case. */
let bare: PGLiteEngine;
let n = 0;
beforeAll(async () => {
engine = new PGLiteEngine();
await engine.connect({});
await engine.initSchema();
bare = new PGLiteEngine();
await bare.connect({});
});
afterAll(async () => {
await engine.disconnect();
await bare.disconnect();
});
/** One put_page upsert — the exact write path #550 kills. */
function put() {
n++;
return engine.putPage(
`notes/issue-550-${n}`,
{ type: 'note', title: `t${n}`, compiled_truth: `body ${n}` },
{ sourceId: 'default' },
);
}
/** Wedge the brain: drop every arbiter the repair may have added. */
async function dropArbiters() {
await engine.executeRaw(`ALTER TABLE pages DROP CONSTRAINT IF EXISTS pages_source_slug_key`);
await engine.executeRaw(`ALTER TABLE pages DROP CONSTRAINT IF EXISTS pages_source_slug_uniq`);
await engine.executeRaw(`DROP INDEX IF EXISTS pages_ss_other_name`);
await engine.executeRaw(`DROP INDEX IF EXISTS pages_source_slug_key`);
}
describe('#550 pages(source_id, slug) unique-index drift', () => {
test('reproduction: dropping the constraint kills put_page; a plain migrate pass heals it', async () => {
await put(); // baseline — fresh brain writes fine
await dropArbiters();
expect(put()).rejects.toThrow(/unique or exclusion constraint|ON CONFLICT/i);
// The brain is stamped at head, so nothing is "pending" — on master this
// pass is a no-op and the brain stays write-dead forever. The #550 fix
// heals on every pass, shape-keyed.
const res = await runMigrations(engine);
expect(res.applied).toBe(0); // proves the heal is NOT a pending migration
await put(); // writes restored
const status = await checkPagesUniqueIndex(engine);
expect(status.satisfied).toBe(true);
expect(status.indexName).toBe('pages_source_slug_key');
});
test('detection is by columns: a satisfying unique index under a different name counts', async () => {
await dropArbiters();
await engine.executeRaw(
`CREATE UNIQUE INDEX pages_ss_other_name ON pages(slug, source_id)`, // reversed order also satisfies inference
);
const status = await checkPagesUniqueIndex(engine);
expect(status.satisfied).toBe(true);
expect(status.indexName).toBe('pages_ss_other_name');
// Repair must be a no-op — no duplicate constraint piled on top.
const r = await repairPagesUniqueIndex(engine);
expect(r.repaired).toBe(false);
expect(r.reason).toBe('already_correct');
await put(); // ON CONFLICT (source_id, slug) accepts the reversed-column arbiter
await dropArbiters();
await repairPagesUniqueIndex(engine); // restore canonical state
});
test('a partial unique index does NOT satisfy ON CONFLICT and is not accepted', async () => {
await dropArbiters();
await engine.executeRaw(
`CREATE UNIQUE INDEX pages_ss_other_name ON pages(source_id, slug) WHERE slug <> ''`,
);
expect(put()).rejects.toThrow(/unique or exclusion constraint|ON CONFLICT/i);
const status = await checkPagesUniqueIndex(engine);
expect(status.satisfied).toBe(false);
const r = await repairPagesUniqueIndex(engine);
expect(r.repaired).toBe(true);
await put();
await engine.executeRaw(`DROP INDEX IF EXISTS pages_ss_other_name`);
});
test('canonical name taken by a wrong-shaped index: repair falls back instead of failing', async () => {
await dropArbiters();
// Correctly named, wrong shape (partial) — must not block the repair.
await engine.executeRaw(
`CREATE UNIQUE INDEX pages_source_slug_key ON pages(source_id, slug) WHERE slug <> ''`,
);
const r = await repairPagesUniqueIndex(engine);
expect(r.repaired).toBe(true);
const status = await checkPagesUniqueIndex(engine);
expect(status.satisfied).toBe(true);
expect(status.indexName).toBe('pages_source_slug_uniq');
await put();
// Restore canonical state for later tests.
await dropArbiters();
await repairPagesUniqueIndex(engine);
});
test('repair is idempotent', async () => {
const first = await repairPagesUniqueIndex(engine);
expect(first.reason).toBe('already_correct');
const second = await repairPagesUniqueIndex(engine);
expect(second.reason).toBe('already_correct');
});
test('doctor check FAILS loudly with the literal repair SQL (and never mentions --force-schema)', async () => {
await dropArbiters();
const check = await doctorCheck(engine);
expect(check.name).toBe('pages_unique_index');
expect(check.status).toBe('fail');
expect(check.message).toContain(PAGES_UNIQUE_REPAIR_SQL);
expect(check.message).toContain(
'ALTER TABLE pages ADD CONSTRAINT pages_source_slug_key UNIQUE (source_id, slug);',
);
expect(check.message).not.toContain('force-schema');
await repairPagesUniqueIndex(engine);
const healthy = await doctorCheck(engine);
expect(healthy.status).toBe('ok');
});
test('missing pages table is OK/skip, not FAIL — a fresh brain is not broken', async () => {
const status = await checkPagesUniqueIndex(bare);
expect(status.tablePresent).toBe(false);
expect(status.satisfied).toBe(false);
const r = await repairPagesUniqueIndex(bare);
expect(r.repaired).toBe(false);
expect(r.reason).toBe('no_table');
const check = await doctorCheck(bare);
expect(check.status).toBe('ok');
});
});