diff --git a/TODOS.md b/TODOS.md index fc74cfaf6..aa668e823 100644 --- a/TODOS.md +++ b/TODOS.md @@ -1799,11 +1799,6 @@ Three items deferred: self-heals via stale-reclaim). The common sync SUCCESS path already drains via handleCliOnly's finally. Convert for graceful drain on sync error exits. -- [ ] **(v0.42.20.0 follow-up) Decouple the op-dispatch force-exit timer** so it - wraps `engine.disconnect()` only (it's armed before the handler today, doubling - as a blanket handler watchdog) and fix its misleading "engine.disconnect() did - not return…" message that fires even when the handler (not disconnect) was slow. - - [ ] **(v0.42.20.0 follow-up) Gateway idle-timeout (vs absolute) for streaming chat.** `withDefaultTimeout` uses an absolute `AbortSignal.timeout`; a streaming generation actively producing tokens past the chat default (300s) would abort. @@ -3573,6 +3568,18 @@ keeping both skills' triggers intact for chaining. ## Completed +### ~~(v0.42.20.0 follow-up) Decouple the op-dispatch force-exit timer~~ +**Completed:** v0.42.39.0 (2026-06-10) + +The timer now arms at teardown entry (inside the op-dispatch finally, before +drain + disconnect) so it bounds ONLY disconnect — no longer doubling as a +blanket handler watchdog that killed slow-but-healthy ops at 10s with exit 0 +and empty stdout. Its "engine.disconnect() did not return…" message is now +accurate by construction (it can only fire during teardown). Read-scope +handlers + context build got their own explicit wallclock bound (180s default, +`--timeout=Ns`, exit 124, hard-exit after teardown) in the same wave. Pinned by +`test/cli-force-exit-teardown-arming.test.ts`. + ### ~~Checks 5 + 6 for check-resolvable~~ **Completed:** v0.19.0 (2026-04-22) diff --git a/docs/embedding-migrations.md b/docs/embedding-migrations.md index e9c425128..49e08765b 100644 --- a/docs/embedding-migrations.md +++ b/docs/embedding-migrations.md @@ -43,8 +43,8 @@ genuinely has to change. Switching dimensions requires: 1. Dropping the HNSW vector index (pgvector won't survive an `ALTER COLUMN TYPE`). -2. Altering the column type (Postgres only — PGLite cannot do this). -3. Wiping every existing embedding (the old vectors are unusable in the new space). +2. Wiping every existing embedding (the old vectors are unusable in the new space — and pgvector refuses to cast them across dimensions, so this must happen before the alter). +3. Altering the column type (Postgres only — PGLite cannot do this). 4. Re-embedding the entire corpus (can take hours on a 50K-page brain and costs $1-100 in API calls depending on model). 5. Conditionally recreating the index (HNSW supports up to 2000 dimensions per pgvector; above that you must use exact scans). @@ -115,12 +115,17 @@ BEGIN; -- 1. Drop the HNSW index. It can't survive the column type change. DROP INDEX IF EXISTS idx_chunks_embedding; --- 2. Alter the column type. -ALTER TABLE content_chunks ALTER COLUMN embedding TYPE vector(); - --- 3. Clear stale embeddings so they don't survive into the new space. +-- 2. Clear stale embeddings FIRST. This must happen BEFORE the column +-- alter: pgvector refuses to cast existing vectors across dimensions +-- ("expected dimensions, not "), so altering a +-- column that still holds old-width vectors aborts the transaction. +-- NULLs cast fine. (The old vectors are unusable in the new space +-- anyway — this is the wipe step from the rationale above.) UPDATE content_chunks SET embedding = NULL, embedded_at = NULL; +-- 3. Alter the column type (all rows are NULL now, so the cast succeeds). +ALTER TABLE content_chunks ALTER COLUMN embedding TYPE vector(); + -- 4. Recreate the HNSW index ONLY IF dims <= 2000. Above that, leave it -- indexless and rely on exact scans (gbrain searchVector handles this -- automatically — search just gets slower, not broken). diff --git a/src/cli.ts b/src/cli.ts index 09d187cea..e80d7e4b8 100755 --- a/src/cli.ts +++ b/src/cli.ts @@ -360,30 +360,83 @@ async function main() { // // Defense-in-depth (adversarial-review C13): `engine.disconnect()` itself // can hang on PGLite (db.close() or releaseLock racing OS-level FS state). - // Install an unref'd setTimeout hard-exit fallback BEFORE entering the - // try/catch/finally so a hung disconnect cannot defeat the force-exit - // contract. Daemons (`serve`) are excluded so they stay alive. + // The unref'd hard-exit fallback is armed inside the `finally` below, so it + // bounds ONLY the teardown phase (drain + disconnect) — the same placement + // as the fall-through owner-disconnect site later in this file. It used to + // be armed HERE, before the try, which silently killed any op whose BODY ran + // past the deadline: on a slow Postgres pooler (6-10s per fresh connection) + // a healthy `gbrain search` was force-exited mid-handler with code 0 and + // ZERO stdout — an empty "success" indistinguishable from no results. The + // exitCode honor (v0.42.20.0) can't help there: a mid-op kill fires before + // any error path sets exitCode. Op-body wallclock bounds are the read-scope + // withTimeout wrap inside the try below, not this teardown backstop. + // Daemons (`serve`) are excluded so they stay alive. const DISCONNECT_HARD_DEADLINE_MS = 10_000; + // Wallclock bound for READ-scope op handlers. With the hard-deadline timer + // correctly scoped to teardown, a genuinely WEDGED read handler (hung pooler + // connection mid-query) would otherwise hang the CLI forever — the #1633 + // zombie class the old (buggy) pre-try timer accidentally bounded at 10s. + // 180s sits far above any healthy slow-pooler run (6-10s/connection); + // --timeout=Ns overrides. Writes/admin stay unbounded: a long import/embed + // must never be killed by a default deadline. + const READ_OP_TIMEOUT_MS = 180_000; let forceExitTimer: ReturnType | undefined; - if (shouldForceExitAfterMain()) { - forceExitTimer = setTimeout(() => { - console.warn( - `[cli] engine.disconnect() did not return within ${DISCONNECT_HARD_DEADLINE_MS}ms — force-exiting`, - ); - // v0.42.20.0 (codex): honor an exit code an errored op already set — - // a bare process.exit(0) here would mask a failed op as success if the - // drain/disconnect then hangs. - process.exit(process.exitCode ?? 0); - }, DISCONNECT_HARD_DEADLINE_MS); - // unref so the timer itself doesn't keep the event loop alive — only - // the actual pending work (PGLite WASM handle) does. Without unref, - // we'd block a clean exit by 10s on every successful CLI run. - forceExitTimer.unref?.(); - } + // Set when a wallclock bound fired. The abandoned (timed-out but still + // running) handler can hold ref'd sockets/timers that keep Bun's event loop + // alive after main() returns — so the finally must hard-exit after teardown + // on this path, or the timeout print is followed by an immortal process: + // the same zombie class, resurrected through the timeout door (adversarial + // review finding). + let wallclockTimedOut = false; try { - const ctx = await makeContext(engine, params); - const rawResult = await op.handler(ctx, params); + const { withTimeout, OperationTimeoutError } = await import('./core/timeout.ts'); + const wallclockMs = getCliOptions().timeoutMs ?? READ_OP_TIMEOUT_MS; + const onWallclockTimeout = (e: InstanceType) => { + const hint = getCliOptions().timeoutMs + ? '' + : ` (default ${e.ms}ms; pass --timeout=Ns to override)`; + console.error(`${e.label} timed out${hint}.`); + process.exitCode = 124; + wallclockTimedOut = true; + }; + + // Context build does DB I/O (resolveSourceId) and runs for EVERY op — + // a wedged pooler connection here would otherwise hang reads, writes, + // and admin alike with no bound at all (adversarial review finding). + let ctx: Awaited>; + try { + ctx = await withTimeout( + makeContext(engine, params), + wallclockMs, + `gbrain ${command}: context`, + ); + } catch (e: unknown) { + if (e instanceof OperationTimeoutError) { + onWallclockTimeout(e); + return; // the finally below still drains + disconnects, then exits + } + throw e; + } + + let rawResult: unknown; + if (op.scope === 'read') { + try { + rawResult = await withTimeout( + op.handler(ctx, params), + wallclockMs, + `gbrain ${command}`, + ); + } catch (e: unknown) { + if (e instanceof OperationTimeoutError) { + onWallclockTimeout(e); + return; // the finally below still drains + disconnects, then exits + } + throw e; + } + } else { + rawResult = await op.handler(ctx, params); + } // ENG-2 (renderer parity by data shape): JSON-round-trip the local-engine // path's return value so renderers see the same shape they'd see on the // routed path. Date → ISO string; bigint → string (postgres.js shape); @@ -396,7 +449,8 @@ async function main() { // STILL runs (drains every background-work sink + disconnects). A bare // process.exit(1) here would skip the finally → skip the drain + disconnect // (leaves facts/cache/eval-capture writes racing teardown). The finally's - // drain bounds teardown; the outer hard-deadline timer bounds a hung one. + // drain bounds teardown; the hard-deadline timer armed at teardown entry + // bounds a hung one. if (e instanceof OperationError) { console.error(`Error [${e.code}]: ${e.message}`); if (e.suggestion) console.error(` Fix: ${e.suggestion}`); @@ -412,11 +466,36 @@ async function main() { // DB logIngest gets the freshest live-engine window. 1s per-sink timeout: // read paths with no pending work pay the ~0ms fast path; capture/import // that DO enqueue pay up to 1s (+ facts shutdown grace) while in-flight - // Haiku finishes. The unref'd hard-deadline timer above is the backstop if - // disconnect or a lingering socket keeps Bun's loop alive. + // Haiku finishes. The unref'd hard-deadline timer armed here is the + // backstop if disconnect or a lingering socket keeps Bun's loop alive — + // armed at teardown entry (NOT before the op body; see the C13 comment + // above) so a slow-but-progressing op handler is never killed mid-flight. + if (shouldForceExitAfterMain()) { + forceExitTimer = setTimeout(() => { + console.warn( + `[cli] engine.disconnect() did not return within ${DISCONNECT_HARD_DEADLINE_MS}ms — force-exiting`, + ); + // v0.42.20.0 (codex): honor an exit code an errored op already set — + // a bare process.exit(0) here would mask a failed op as success if the + // drain/disconnect then hangs. + process.exit(process.exitCode ?? 0); + }, DISCONNECT_HARD_DEADLINE_MS); + // unref so the timer itself doesn't keep the event loop alive — only + // the actual pending work (PGLite WASM handle) does. Without unref, + // we'd block a clean exit by 10s on every successful CLI run. + forceExitTimer.unref?.(); + } await drainAllBackgroundWorkForCliExit({ timeoutMs: 1000 }); await engine.disconnect(); if (forceExitTimer) clearTimeout(forceExitTimer); + // Wallclock-timeout path: teardown is done, but the ABANDONED handler + // (withTimeout races, it does not cancel) can still hold ref'd sockets / + // SDK retry timers that keep Bun's event loop alive indefinitely. With + // the hard-deadline timer just cleared, nothing else bounds that — exit + // explicitly. Safe: drain + disconnect completed on the lines above. + if (wallclockTimedOut) { + process.exit(process.exitCode ?? 124); + } } } diff --git a/src/core/embedding-dim-check.ts b/src/core/embedding-dim-check.ts index 4ca542da2..220bcca05 100644 --- a/src/core/embedding-dim-check.ts +++ b/src/core/embedding-dim-check.ts @@ -217,8 +217,10 @@ export function embeddingMismatchMessage(opts: EmbeddingMismatchOpts): string { ``, ` BEGIN;`, ` DROP INDEX IF EXISTS idx_chunks_embedding;`, - ` ALTER TABLE content_chunks ALTER COLUMN embedding TYPE vector(${requestedDims});`, + ` -- NULL embeddings BEFORE the alter: pgvector refuses to cast existing`, + ` -- vectors across dimensions and aborts the transaction. NULLs cast fine.`, ` UPDATE content_chunks SET embedding = NULL, embedded_at = NULL;`, + ` ALTER TABLE content_chunks ALTER COLUMN embedding TYPE vector(${requestedDims});`, ` ${reindexLine.split('\n').join('\n ')}`, ` COMMIT;`, ``, @@ -573,11 +575,25 @@ export function buildFactsAlterRecipe( ): string { const opclass = columnType === 'halfvec' ? 'halfvec_cosine_ops' : 'vector_cosine_ops'; const targetType = columnType === 'halfvec' ? `halfvec(${configuredDims})` : `vector(${configuredDims})`; + const dimsChanged = columnDims !== configuredDims; return [ `-- ALTER ${columnType}(${columnDims}) → ${columnType}(${configuredDims}) on indexed column.`, `-- HOLD a maintenance window: this rewrites every row's embedding.`, `-- Coordinate with any active extract-conversation-facts backfill.`, `DROP INDEX IF EXISTS idx_facts_embedding_hnsw;`, + // Same-dim type swaps (halfvec <-> vector) cast row data losslessly and + // MUST keep it. Cross-dimension changes are different: pgvector refuses + // to cast existing vectors across dimensions ("expected N dimensions, + // not M") and aborts the transaction — and the old-space vectors are + // unusable at the new width anyway. NULL them first; the facts pipeline + // re-embeds on the next write. + ...(dimsChanged + ? [ + `-- Dimension change: NULL embeddings BEFORE the alter — pgvector`, + `-- refuses cross-dimension casts and aborts the transaction.`, + `UPDATE facts SET embedding = NULL;`, + ] + : []), `ALTER TABLE facts ALTER COLUMN embedding TYPE ${targetType}`, ` USING embedding::${targetType};`, `CREATE INDEX idx_facts_embedding_hnsw`, diff --git a/src/core/import-file.ts b/src/core/import-file.ts index fb9190ee4..cbf2d03a0 100644 --- a/src/core/import-file.ts +++ b/src/core/import-file.ts @@ -1193,7 +1193,15 @@ export async function importCodeFile( // chunk IDs are stable. if (extractedEdges.length > 0 && chunks.length > 0) { try { - const persistedChunks = await engine.getChunks(slug, sourceId ? { sourceId } : undefined); + // Normalize ONCE: '' and undefined both mean the schema-default source + // (pages.source_id DEFAULT 'default'). Using the normalized value for + // BOTH the chunk lookup and the edge stamp keeps them in lockstep — + // an unscoped getChunks here could fan out to same-slug chunks from + // another source, and a '' stamp would FK-violate against sources(id) + // and silently drop the file's whole call graph in the best-effort + // catch below (adversarial review findings). + const edgeSourceId = sourceId || 'default'; + const persistedChunks = await engine.getChunks(slug, { sourceId: edgeSourceId }); const byIndex = new Map(); for (const pc of persistedChunks) { byIndex.set(pc.chunk_index, pc); @@ -1231,6 +1239,17 @@ export async function importCodeFile( from_symbol_qualified: from.symbol_name_qualified, to_symbol_qualified: e.toSymbol, edge_type: e.edgeType, + // Stamp the source: getCallersOf/getCalleesOf add + // `AND source_id = ` whenever a worktree pin / --source is + // in play, and a NULL here never matches that filter — so every + // scoped call-graph query silently returned 0 rows on + // multi-source brains even though the edges existed. The fallback + // is 'default', NOT null: an unscoped import lands its pages under + // the schema default (pages.source_id DEFAULT 'default'), so a + // NULL-stamped edge would be invisible to the matching scoped + // query getCallersOf(sym, { sourceId: 'default' }) — the same bug + // through the other door. + source_id: edgeSourceId, }); } diff --git a/src/core/migrate.ts b/src/core/migrate.ts index 106b1e015..31d3a10c1 100644 --- a/src/core/migrate.ts +++ b/src/core/migrate.ts @@ -5186,6 +5186,52 @@ export const MIGRATIONS: Migration[] = [ ); `, }, + { + version: 116, + name: 'code_edges_source_backfill_and_callee_index', + // Repair + index pass for the call graph: + // + // 1. BACKFILL: importCodeFile built CodeEdgeInput rows without source_id, + // so every extracted edge landed NULL. getCallersOf/getCalleesOf add + // `AND source_id = ` whenever a worktree pin / --source is in + // play — NULL never matches, so scoped call-graph queries silently + // returned 0 rows on multi-source brains even though the edges + // existed. The write path now stamps `sourceId ?? 'default'`; this + // backfill repairs rows written before the fix by deriving each + // edge's source from its own from_chunk's page (pages.source_id is + // NOT NULL DEFAULT 'default', so COALESCE is belt-and-braces only). + // + // 2. INDEXES: getCalleesOf filters BOTH edge tables on + // from_symbol_qualified, which had no index anywhere — every callee + // lookup was a sequential scan, amplified per-BFS-node by the + // recursive code walk (one getCalleesOf per frontier node, up to + // maxNodes). With NULL edges repaired, scoped walks actually expand, + // so the latent seq-scan cost becomes real. Plain CREATE INDEX (not + // CONCURRENTLY): edge tables are modest (mirrors the v58 resolver + // index). Keep in sync with src/schema.sql. + idempotent: true, + sql: ` + UPDATE code_edges_symbol e + SET source_id = COALESCE(p.source_id, 'default') + FROM content_chunks c + JOIN pages p ON p.id = c.page_id + WHERE c.id = e.from_chunk_id + AND e.source_id IS NULL; + + UPDATE code_edges_chunk e + SET source_id = COALESCE(p.source_id, 'default') + FROM content_chunks c + JOIN pages p ON p.id = c.page_id + WHERE c.id = e.from_chunk_id + AND e.source_id IS NULL; + + CREATE INDEX IF NOT EXISTS idx_code_edges_symbol_from_symbol + ON code_edges_symbol (from_symbol_qualified); + + CREATE INDEX IF NOT EXISTS idx_code_edges_chunk_from_symbol + ON code_edges_chunk (from_symbol_qualified); + `, + }, ]; export const LATEST_VERSION = MIGRATIONS.length > 0 diff --git a/src/core/pglite-engine.ts b/src/core/pglite-engine.ts index 74bc88ab3..8fca74c3c 100644 --- a/src/core/pglite-engine.ts +++ b/src/core/pglite-engine.ts @@ -4943,7 +4943,7 @@ export class PGLiteEngine implements BrainEngine { e.from_chunk_id, e.to_chunk_id, e.from_symbol_qualified, e.to_symbol_qualified, e.edge_type, JSON.stringify(e.edge_metadata ?? {}), - e.source_id ?? null, + e.source_id ?? 'default', ); } const res = await this.db.query( @@ -4964,7 +4964,7 @@ export class PGLiteEngine implements BrainEngine { params.push( e.from_chunk_id, e.from_symbol_qualified, e.to_symbol_qualified, e.edge_type, JSON.stringify(e.edge_metadata ?? {}), - e.source_id ?? null, + e.source_id ?? 'default', ); } const res = await this.db.query( diff --git a/src/core/postgres-engine.ts b/src/core/postgres-engine.ts index b41408bdf..c6e3ad9f4 100644 --- a/src/core/postgres-engine.ts +++ b/src/core/postgres-engine.ts @@ -5169,7 +5169,7 @@ export class PostgresEngine implements BrainEngine { const toQual = resolved.map(e => e.to_symbol_qualified); const edgeTypes = resolved.map(e => e.edge_type); const metas = resolved.map(e => JSON.stringify(e.edge_metadata ?? {})); - const sources = resolved.map(e => e.source_id ?? null); + const sources = resolved.map(e => e.source_id ?? 'default'); const res = await sql` INSERT INTO code_edges_chunk (from_chunk_id, to_chunk_id, from_symbol_qualified, to_symbol_qualified, edge_type, edge_metadata, source_id) SELECT * FROM unnest( @@ -5189,7 +5189,7 @@ export class PostgresEngine implements BrainEngine { const toQual = unresolved.map(e => e.to_symbol_qualified); const edgeTypes = unresolved.map(e => e.edge_type); const metas = unresolved.map(e => JSON.stringify(e.edge_metadata ?? {})); - const sources = unresolved.map(e => e.source_id ?? null); + const sources = unresolved.map(e => e.source_id ?? 'default'); const res = await sql` INSERT INTO code_edges_symbol (from_chunk_id, from_symbol_qualified, to_symbol_qualified, edge_type, edge_metadata, source_id) SELECT * FROM unnest( diff --git a/src/core/schema-embedded.ts b/src/core/schema-embedded.ts index dc288eabd..2e30b49f1 100644 --- a/src/core/schema-embedded.ts +++ b/src/core/schema-embedded.ts @@ -390,6 +390,11 @@ CREATE INDEX IF NOT EXISTS idx_code_edges_chunk_to ON code_edges_chunk(to_chunk_id, edge_type); CREATE INDEX IF NOT EXISTS idx_code_edges_chunk_to_symbol ON code_edges_chunk(to_symbol_qualified, edge_type); +-- getCalleesOf filters on from_symbol_qualified; without this index every +-- callee lookup is a sequential scan, amplified per-BFS-node by the +-- recursive code walk. Mirrors migration v116. +CREATE INDEX IF NOT EXISTS idx_code_edges_chunk_from_symbol + ON code_edges_chunk(from_symbol_qualified); CREATE TABLE IF NOT EXISTS code_edges_symbol ( id SERIAL PRIMARY KEY, @@ -407,16 +412,32 @@ CREATE INDEX IF NOT EXISTS idx_code_edges_symbol_from ON code_edges_symbol(from_chunk_id, edge_type); CREATE INDEX IF NOT EXISTS idx_code_edges_symbol_to ON code_edges_symbol(to_symbol_qualified, edge_type); +-- getCalleesOf companion to idx_code_edges_chunk_from_symbol above. +-- Mirrors migration v116. +CREATE INDEX IF NOT EXISTS idx_code_edges_symbol_from_symbol + ON code_edges_symbol(from_symbol_qualified); -- ============================================================ -- links: cross-references between pages -- ============================================================ --- Provenance model (v0.13, extended issue #972): --- link_source — 'markdown' | 'frontmatter' | 'manual' | 'wikilink-resolved' | NULL --- 'wikilink-resolved' is the opt-in --- (link_resolution.global_basename) basename-match --- provenance — see issue #972 / migration v113. --- (NULL = legacy row written before v0.13; unknown source) +-- Provenance model (v0.13; opened to kebab provenance in v114 / issue #1941): +-- link_source — open kebab-case provenance tag, NOT a closed allowlist. +-- Format gate (CHECK): ^[a-z][a-z0-9]*(-[a-z0-9]+)*\$ and +-- char_length <= 64. NULL = legacy row (pre-v0.13). +-- Reconciliation-managed built-ins written internally: +-- 'markdown' — body markdown links +-- 'frontmatter' — YAML frontmatter edges (see origin_*) +-- 'mentions' — auto-linked body-text mentions +-- 'wikilink-resolved'— opt-in global-basename [[name]] (#972) +-- User/tool-facing: +-- 'manual' — hand- or tool-created edges (the +-- add_link op default + CLI link-add) +-- '' — external derivers, e.g. 'citation-graph', +-- stamp their own kebab tag (no migration). +-- The add_link OP forbids callers from passing the four +-- managed built-ins (they imply reconciliation semantics a +-- hand-created row can't honor); the DB CHECK still admits +-- them because internal writers use them. See operations.ts. -- origin_page_id — for link_source='frontmatter', the page whose YAML -- frontmatter created this edge; scopes reconciliation -- origin_field — the frontmatter field name (e.g. 'key_people') @@ -424,21 +445,21 @@ CREATE INDEX IF NOT EXISTS idx_code_edges_symbol_to -- The unique constraint includes link_source + origin_page_id so a manual edge -- and a frontmatter-derived edge with the same (from, to, type) tuple coexist. -- Reconciliation on put_page filters by (link_source='frontmatter' AND --- origin_page_id = written_page) — never touches other pages' edges. +-- origin_page_id = written_page) — never touches other pages' edges. (This is +-- exactly why a CLI-forged 'frontmatter' row with NULL origin would be a phantom +-- edge reconciliation never cleans — hence the op-layer guard.) CREATE TABLE IF NOT EXISTS links ( id SERIAL PRIMARY KEY, from_page_id INTEGER NOT NULL REFERENCES pages(id) ON DELETE CASCADE, to_page_id INTEGER NOT NULL REFERENCES pages(id) ON DELETE CASCADE, link_type TEXT NOT NULL DEFAULT '', context TEXT NOT NULL DEFAULT '', - -- v114 (#1941): link_source is an open kebab-case provenance, not a closed - -- allowlist — external derivers (e.g. 'citation-graph') stamp their own tag - -- without a gbrain migration. Format gate only: lowercase kebab, <=64 chars. - -- The reconciliation-managed built-ins ('markdown','frontmatter','mentions', - -- 'wikilink-resolved') still satisfy this and are written internally; the - -- add_link op forbids CALLERS from forging them (see operations.ts). 'manual' - -- is the user-facing default for hand/tool-created edges. - link_source TEXT CHECK (link_source IS NULL OR (link_source ~ '^[a-z][a-z0-9]*(-[a-z0-9]+)*$' AND char_length(link_source) <= 64)), + -- v0.41.18.0: 'mentions' added for auto-linked body-text mentions + -- (gbrain extract links --by-mention). Filtered OUT of backlink-count + -- for search ranking; only counts toward orphan-ratio + graph traversal. + -- v0.40.8.2 (#972): 'wikilink-resolved' added for opt-in global-basename + -- wikilink resolution (bare [[name]] resolved by slug tail). + link_source TEXT CHECK (link_source IS NULL OR (link_source ~ '^[a-z][a-z0-9]*(-[a-z0-9]+)*\$' AND char_length(link_source) <= 64)), -- v0.41.18.0: nullable link_kind distinguishes "plain body mention" from -- "verb-pattern-derived typed link" within link_source='mentions'. -- Codex finding #12 design: keep link_source stable; add link_kind @@ -675,8 +696,11 @@ CREATE TABLE IF NOT EXISTS op_checkpoints ( CREATE INDEX IF NOT EXISTS op_checkpoints_updated_at_idx ON op_checkpoints (updated_at); --- #1794: append-only delta storage (one row per completed path). FK cascade --- drops children with the parent. Mirrors migration v115 + src/schema.sql. +-- #1794: append-only delta storage. One row per completed path; sync's +-- appendCompleted INSERTs only the delta instead of rewriting the whole +-- completed_keys JSONB array each flush (O(N^2) -> O(delta)). FK cascade drops +-- children with the parent (clearOpCheckpoint + 7-day purge). PK prefix +-- (op,fingerprint) serves all reads; no separate index. Mirrors migration v115. CREATE TABLE IF NOT EXISTS op_checkpoint_paths ( op TEXT NOT NULL, fingerprint TEXT NOT NULL, diff --git a/src/schema.sql b/src/schema.sql index a900c6d1a..3a5f32ef7 100644 --- a/src/schema.sql +++ b/src/schema.sql @@ -386,6 +386,11 @@ CREATE INDEX IF NOT EXISTS idx_code_edges_chunk_to ON code_edges_chunk(to_chunk_id, edge_type); CREATE INDEX IF NOT EXISTS idx_code_edges_chunk_to_symbol ON code_edges_chunk(to_symbol_qualified, edge_type); +-- getCalleesOf filters on from_symbol_qualified; without this index every +-- callee lookup is a sequential scan, amplified per-BFS-node by the +-- recursive code walk. Mirrors migration v116. +CREATE INDEX IF NOT EXISTS idx_code_edges_chunk_from_symbol + ON code_edges_chunk(from_symbol_qualified); CREATE TABLE IF NOT EXISTS code_edges_symbol ( id SERIAL PRIMARY KEY, @@ -403,6 +408,10 @@ CREATE INDEX IF NOT EXISTS idx_code_edges_symbol_from ON code_edges_symbol(from_chunk_id, edge_type); CREATE INDEX IF NOT EXISTS idx_code_edges_symbol_to ON code_edges_symbol(to_symbol_qualified, edge_type); +-- getCalleesOf companion to idx_code_edges_chunk_from_symbol above. +-- Mirrors migration v116. +CREATE INDEX IF NOT EXISTS idx_code_edges_symbol_from_symbol + ON code_edges_symbol(from_symbol_qualified); -- ============================================================ -- links: cross-references between pages diff --git a/test/cli-force-exit-teardown-arming.test.ts b/test/cli-force-exit-teardown-arming.test.ts new file mode 100644 index 000000000..ff9743e30 --- /dev/null +++ b/test/cli-force-exit-teardown-arming.test.ts @@ -0,0 +1,56 @@ +/** + * Structural regression — the DISCONNECT_HARD_DEADLINE_MS force-exit timer in + * cli.ts main() must be armed at TEARDOWN ENTRY (inside the finally, before + * the drain + disconnect), never before the op-dispatch try block. + * + * Pre-fix bug: the 10s unref'd setTimeout was armed BEFORE the try, so any op + * whose handler ran past 10s wall-clock was killed mid-flight with + * process.exit(0) and ZERO stdout — an empty "success" indistinguishable from + * no results (a healthy `gbrain search` on a slow Postgres pooler hit this on + * every run). Armed in the finally, the timer still bounds a hung + * drain/disconnect (the C13 contract) but can no longer kill a + * slow-but-progressing op body. + * + * Source-grep is the right tool here (same rationale as + * fix-wave-structural.test.ts): the rule is "this arming must stay at this + * location". A behavioral test would need >10s of real wall-clock plus a + * deliberately slow op handler in a spawned CLI — slow and flaky by + * construction. + */ + +import { describe, test, expect } from 'bun:test'; +import { readFileSync } from 'fs'; + +describe('cli.ts — disconnect hard-deadline armed at teardown entry, not before the op body', () => { + test('forceExitTimer setTimeout lives inside the finally, gated on the daemon guard, before the drain', () => { + const src = readFileSync('src/cli.ts', 'utf8'); + + const decl = src.indexOf('const DISCONNECT_HARD_DEADLINE_MS'); + expect(decl).toBeGreaterThan(-1); + const tryIdx = src.indexOf('try {', decl); + expect(tryIdx).toBeGreaterThan(-1); + const finallyIdx = src.indexOf('} finally {', tryIdx); + expect(finallyIdx).toBeGreaterThan(-1); + const armIdx = src.indexOf('forceExitTimer = setTimeout', decl); + expect(armIdx).toBeGreaterThan(-1); + const drainIdx = src.indexOf('drainAllBackgroundWorkForCliExit', finallyIdx); + expect(drainIdx).toBeGreaterThan(-1); + + // NO arming between the deadline declaration and the op-body try — a + // pre-try timer kills slow-but-progressing op handlers mid-flight with + // exit 0 and empty stdout. (`setTimeout(` matches only a call site; the + // `ReturnType` type annotation stays allowed.) + expect(src.slice(decl, tryIdx)).not.toContain('setTimeout('); + + // The arming sits AFTER the finally opens (teardown entry) and BEFORE the + // drain + disconnect it exists to bound. + expect(armIdx).toBeGreaterThan(finallyIdx); + expect(armIdx).toBeLessThan(drainIdx); + + // Still gated on the daemon-survival guard so `serve` stays alive, and + // still unref'd + cleared on clean teardown. + expect(src.slice(finallyIdx, drainIdx)).toMatch(/if \(shouldForceExitAfterMain\(\)\)/); + expect(src.slice(finallyIdx, drainIdx)).toContain('forceExitTimer.unref?.()'); + expect(src.slice(drainIdx)).toContain('if (forceExitTimer) clearTimeout(forceExitTimer)'); + }); +}); diff --git a/test/embedding-dim-check-facts.test.ts b/test/embedding-dim-check-facts.test.ts index d152e6fd4..2b0a9a8fc 100644 --- a/test/embedding-dim-check-facts.test.ts +++ b/test/embedding-dim-check-facts.test.ts @@ -142,6 +142,27 @@ describe('buildFactsAlterRecipe', () => { const recipe = buildFactsAlterRecipe(1536, 1280, 'halfvec'); expect(recipe).toMatch(/DROP INDEX[\s\S]*ALTER TABLE[\s\S]*CREATE INDEX/); }); + + test('dimension change NULLs embeddings BEFORE the alter (pgvector refuses cross-dim casts)', () => { + // Same defect class fixed for content_chunks in embeddingMismatchMessage: + // pgvector aborts a cross-dimension ALTER while rows still hold old-width + // vectors. The dims-change recipe must wipe first; order pinned. + const recipe = buildFactsAlterRecipe(1536, 1280, 'halfvec'); + const nullIdx = recipe.indexOf('UPDATE facts SET embedding = NULL'); + const alterIdx = recipe.indexOf('ALTER TABLE facts ALTER COLUMN embedding TYPE'); + expect(nullIdx).toBeGreaterThan(-1); + expect(alterIdx).toBeGreaterThan(-1); + expect(nullIdx).toBeLessThan(alterIdx); + }); + + test('same-dim type swap PRESERVES embeddings (no NULL wipe)', () => { + // halfvec(1536) <-> vector(1536): the USING cast is lossless and the + // whole point is keeping the data. A wipe here would destroy valid + // embeddings for no reason. + const recipe = buildFactsAlterRecipe(1536, 1536, 'vector'); + expect(recipe).not.toContain('UPDATE facts SET embedding = NULL'); + expect(recipe).toContain('USING embedding::vector(1536)'); + }); }); describe('FactsEmbeddingDimMismatchError', () => { diff --git a/test/embedding-dim-check.test.ts b/test/embedding-dim-check.test.ts index f74ebc750..6a945b98c 100644 --- a/test/embedding-dim-check.test.ts +++ b/test/embedding-dim-check.test.ts @@ -103,6 +103,26 @@ describe('embeddingMismatchMessage', () => { expect(msg).toContain('docs/embedding-migrations.md'); }); + test('Postgres recipe NULLs embeddings BEFORE the column alter (pgvector refuses cross-dim casts)', () => { + // pgvector aborts `ALTER COLUMN TYPE vector(N)` with "expected N + // dimensions, not M" while rows still hold old-width vectors — which is + // every brain running this recipe. The UPDATE must precede the ALTER + // (NULLs cast fine). Order pinned so the printed recipe can't drift from + // the corrected docs/embedding-migrations.md again. + const msg = embeddingMismatchMessage({ + currentDims: 1536, + requestedDims: 768, + requestedModel: 'nomic-embed-text', + source: 'init', + engineKind: 'postgres', + }); + const nullIdx = msg.indexOf('UPDATE content_chunks SET embedding = NULL'); + const alterIdx = msg.indexOf('ALTER TABLE content_chunks ALTER COLUMN embedding TYPE vector(768)'); + expect(nullIdx).toBeGreaterThan(-1); + expect(alterIdx).toBeGreaterThan(-1); + expect(nullIdx).toBeLessThan(alterIdx); + }); + test('Postgres branch skips HNSW recreate when requested dims exceed pgvector cap', () => { // Codex finding #8: 2048d (Voyage 4 Large) cannot be HNSW-indexed in pgvector. // The recipe must NOT instruct a CREATE INDEX HNSW for that dim. diff --git a/test/import-code-edges-source-id.test.ts b/test/import-code-edges-source-id.test.ts new file mode 100644 index 000000000..69c46be0f --- /dev/null +++ b/test/import-code-edges-source-id.test.ts @@ -0,0 +1,112 @@ +/** + * Regression — importCodeFile stamps source_id on extracted call-graph edges. + * + * Pre-fix bug: importCodeFile built CodeEdgeInput rows WITHOUT source_id, so + * every extracted edge landed NULL in code_edges_symbol. getCallersOf / + * getCalleesOf add `AND source_id = ` whenever a worktree pin or + * --source is in play — NULL never matches that filter, so scoped call-graph + * queries silently returned 0 rows on multi-source brains even though the + * edges existed. Pre-existing coverage (cathedral-ii-brainbench.test.ts, + * code-edges.test.ts) only ever queried with { allSources: true }, which + * bypasses the filter — exactly why the NULL never surfaced. + * + * This test imports a caller/callee pair under a non-default source and + * asserts (a) the persisted code_edges_symbol rows carry the source_id, and + * (b) the SCOPED getCallersOf/getCalleesOf — the user-visible path that + * returned 0 — now find the edge, while a different source scope does not. + */ + +import { describe, test, expect, beforeAll, afterAll } from 'bun:test'; +import { PGLiteEngine } from '../src/core/pglite-engine.ts'; +import { importCodeFile } from '../src/core/import-file.ts'; +import { runSources } from '../src/commands/sources.ts'; + +let engine: PGLiteEngine; + +beforeAll(async () => { + engine = new PGLiteEngine(); + await engine.connect({}); + await engine.initSchema(); + await runSources(engine, ['add', 'testsrc', '--no-federated']); + + // Same fixture shape as cathedral-ii-brainbench: runner() calls helper(), + // Layer 5 edge extraction captures the unresolved 'calls' edge — but here + // the import is pinned to a non-default source. + await importCodeFile( + engine, + 'src/a.ts', + 'export function runner() { return helper(); }\n', + { noEmbed: true, sourceId: 'testsrc' }, + ); + await importCodeFile( + engine, + 'src/b.ts', + 'export function helper() { return 42; }\n', + { noEmbed: true, sourceId: 'testsrc' }, + ); +}, 60_000); + +afterAll(async () => { + if (engine) await engine.disconnect(); +}, 30_000); + +describe('importCodeFile — source_id stamped on extracted call-graph edges', () => { + test('edges land with the import source_id and scoped caller/callee queries match', async () => { + // (a) The persisted unresolved edge rows carry the source, not NULL. + const rows = await engine.executeRaw<{ source_id: string | null }>( + `SELECT source_id FROM code_edges_symbol WHERE from_symbol_qualified = $1`, + ['runner'], + ); + expect(rows.length).toBeGreaterThanOrEqual(1); + for (const r of rows) { + expect(r.source_id).toBe('testsrc'); + } + + // (b) The scoped queries — the path that silently returned 0 pre-fix. + const callers = await engine.getCallersOf('helper', { sourceId: 'testsrc' }); + const fromRunner = callers.find(r => r.from_symbol_qualified === 'runner'); + expect(fromRunner).toBeDefined(); + expect(fromRunner!.edge_type).toBe('calls'); + expect(fromRunner!.source_id).toBe('testsrc'); + + const callees = await engine.getCalleesOf('runner', { sourceId: 'testsrc' }); + expect(callees.some(r => r.to_symbol_qualified === 'helper')).toBe(true); + + // Source isolation still holds: a different scope must NOT see the edge. + const otherScope = await engine.getCallersOf('helper', { sourceId: 'default' }); + expect(otherScope.find(r => r.from_symbol_qualified === 'runner')).toBeUndefined(); + }); + + test('UNSCOPED import stamps edges with the schema-default source, not NULL', async () => { + // The other door of the same bug: an import WITHOUT opts.sourceId (legacy + // unscoped callers — `gbrain reindex --code` with no --source) lands its + // pages under the schema default (pages.source_id DEFAULT 'default'). + // If its edges were stamped NULL, the matching scoped query + // getCallersOf(sym, { sourceId: 'default' }) — a worktree pinned to + // default, --source default, GBRAIN_SOURCE=default — would miss them. + await importCodeFile( + engine, + 'src/c.ts', + 'export function unscopedRunner() { return unscopedHelper(); }\n', + { noEmbed: true }, + ); + await importCodeFile( + engine, + 'src/d.ts', + 'export function unscopedHelper() { return 7; }\n', + { noEmbed: true }, + ); + + const rows = await engine.executeRaw<{ source_id: string | null }>( + `SELECT source_id FROM code_edges_symbol WHERE from_symbol_qualified = $1`, + ['unscopedRunner'], + ); + expect(rows.length).toBeGreaterThanOrEqual(1); + for (const r of rows) { + expect(r.source_id).toBe('default'); + } + + const callers = await engine.getCallersOf('unscopedHelper', { sourceId: 'default' }); + expect(callers.some(r => r.from_symbol_qualified === 'unscopedRunner')).toBe(true); + }); +});