diff --git a/CHANGELOG.md b/CHANGELOG.md index 3d09af88b..84a61fabe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,68 @@ # Changelog All notable changes to GBrain will be documented in this file. + +## [0.22.8] - 2026-04-28 + +## **Doctor stops timing out on Supabase. Integrity scan finishes in ~6s, multi-source brains get correct counts.** + +If you've been hitting the 60-second `gbrain doctor` timeout on Supabase or any pooled-connection deployment, this fixes it. The integrity check used to call `getPage()` 500 times sequentially through PgBouncer transaction-mode pooling. Each call required a full connection acquire/release cycle, which doctor couldn't finish before CI killed it. The new path batch-loads all 500 pages in a single SQL query, finishing in ~6s. + +While shipping the perf fix, codex review caught a correctness regression for multi-source brains: the batch SQL was scanning raw `(source_id, slug)` rows while the sequential path scanned unique slugs. Multi-source brains were getting inflated counts. `SELECT DISTINCT ON (slug)` mirrors the sequential path's `Set` semantics; parity tests against real Postgres pin both paths to the same output. + +Plus a Linux CI fix: `gbrain skillpack` lockfile checks were intermittently failing on ext4's sub-millisecond `mtimeMs` timestamps when `Date.now()` returned an integer ms behind the file's recorded mtime. Lock age now clamps to zero. + +### The numbers that matter + +Measured against the real failure mode on a Supabase PgBouncer deployment that hit the 60s CI timeout pre-fix. + +| Behavior | Before v0.22.8 | After v0.22.8 | +|---|---|---| +| `gbrain doctor` wall-clock (Postgres + PgBouncer) | 60s+ timeout (killed) | ~6s | +| `integrity_sample` query round-trips | ~500 (sequential `getPage`) | 1 (`SELECT DISTINCT ON`) | +| Multi-source brain scan accuracy | Overcounted by `source_id` | Exact per unique slug | + +### What this means for Supabase deployments + +If you've been avoiding `gbrain doctor` because it timed out, run it again. If you maintain a multi-source brain (imported pages from another gbrain deployment under a non-default `source_id`), the scan now treats each slug once instead of once-per-source — your output is exact, not inflated. Single-source users see no behavior change; PGLite users were never affected (the batch path is Postgres-only). + +## To take advantage of v0.22.8 + +`gbrain upgrade` should do this automatically. If it didn't, or if `gbrain doctor` warns about anything afterwards: + +1. **Run the upgrade:** + ```bash + gbrain upgrade + ``` +2. **Verify doctor finishes cleanly (especially relevant if you hit timeouts before):** + ```bash + gbrain doctor + ``` + On Postgres + PgBouncer deployments, you should see `integrity_sample` finish in ~6s instead of timing out at 60s. +3. **If `doctor` still times out or output looks wrong,** please file an issue: + https://github.com/garrytan/gbrain/issues with: + - output of `gbrain doctor` (full) + - which engine (Postgres vs PGLite) + - whether you use multi-source brains + +### Itemized changes + +#### Performance +- `gbrain doctor` integrity sample now batch-loads via a single SQL query on Postgres deployments (60s+ timeout → ~6s wall-clock, 500 round-trips → 1). +- Batch path explicitly gated to Postgres via `engine.kind` so PGLite never attempts it (clean fallback signal). + +#### Correctness +- `scanIntegrity` batch path uses `SELECT DISTINCT ON (slug)` to scope by unique slug, matching `engine.getAllSlugs()`'s `Set` semantics. Multi-source brains (UNIQUE(source_id, slug) since v0.18.0) now get correct counts instead of one-scan-per-source-row. +- `IntegrityScanResult.pagesScanned` now reflects unique slugs scanned, not raw row count. Single-source brains: unchanged. Multi-source brains: counts now match expected distinct-page semantics. +- Batch-path fallback narrowed: real Postgres errors (deadlock, connection drop, SQL bug) surface via `GBRAIN_DEBUG=1` instead of being silently swallowed. + +#### Tests +- New `test/e2e/integrity-batch.test.ts` — four parity cases (dedup, hits, validate, topPages) asserting batch ≡ sequential against real Postgres. Pinning the multi-source dedup case requires a raw-SQL fixture for the alt-source row since `engine.putPage` doesn't take a `source_id`. + +#### Infrastructure +- `src/core/skillpack/installer.ts` — clamp negative lock-age to 0, fixing intermittent Linux ext4 CI flakes from sub-millisecond `mtimeMs` precision (Date.now is integer ms; mtime can be ~0.3ms ahead). New regression test in `test/skillpack-install.test.ts` deterministically reproduces via `utimesSync`. +- `CLAUDE.md` test inventory updated for the new test files. + ## [0.22.7] - 2026-04-28 ## **Built-in HTTP transport with bearer auth for remote MCP.** diff --git a/CLAUDE.md b/CLAUDE.md index a7ce96a2e..736fb0c2b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -100,6 +100,7 @@ strict behavior when unset. - `src/commands/migrations/` — TS migration registry (compiled into the binary; no filesystem walk of `skills/migrations/*.md` needed at runtime). `index.ts` lists migrations in semver order. `v0_11_0.ts` = Minions adoption orchestrator (8 phases). `v0_12_0.ts` = Knowledge Graph auto-wire orchestrator (5 phases: schema → config check → backfill links → backfill timeline → verify). `phaseASchema` has a 600s timeout (bumped from 60s in v0.12.1 for duplicate-heavy brains). `v0_12_2.ts` = JSONB double-encode repair orchestrator (4 phases: schema → repair-jsonb → verify → record). `v0_14_0.ts` = shell-jobs + autopilot cooperative (2 phases: schema ALTER minion_jobs.max_stalled SET DEFAULT 3 — superseded by v0.14.3's schema-level DEFAULT 5 + UPDATE backfill; pending-host-work ping for skills/migrations/v0.14.0.md). All orchestrators are idempotent and resumable from `partial` status. As of v0.14.2 (Bug 3), the RUNNER owns all ledger writes — orchestrators return `OrchestratorResult` and `apply-migrations.ts` persists a canonical `{version, status, phases}` shape after return. Orchestrators no longer call `appendCompletedMigration` directly. `statusForVersion` prefers `complete` over `partial` (never regresses). 3 consecutive partials → wedged → `--force-retry ` writes a `'retry'` reset marker. v0.14.3 (fix wave) ships schema-only migrations v14 (`pages_updated_at_index`) + v15 (`minion_jobs_max_stalled_default_5` with UPDATE backfill) via the `MIGRATIONS` array in `src/core/migrate.ts` — no orchestrator phases needed. - `src/commands/repair-jsonb.ts` — `gbrain repair-jsonb [--dry-run] [--json]`: rewrites `jsonb_typeof='string'` rows in place across 5 affected columns (pages.frontmatter, raw_data.data, ingest_log.pages_updated, files.metadata, page_versions.frontmatter). Fixes v0.12.0 double-encode bug on Postgres; PGLite no-ops. Idempotent. - `src/commands/orphans.ts` — `gbrain orphans [--json] [--count] [--include-pseudo]`: surfaces pages with zero inbound wikilinks, grouped by domain. Auto-generated/raw/pseudo pages filtered by default. Also exposed as `find_orphans` MCP operation. Shipped in v0.12.3 (contributed by @knee5). +- `src/commands/integrity.ts` — `gbrain integrity check|auto|review|extract`: bare-tweet detection, dead-link detection, three-bucket repair (auto-repair / review-queue / skip). `scanIntegrity()` is the shared library function called from `gbrain doctor` (sampled at limit=500) and `cmdCheck` (full scan). v0.22.8: batch-load fast path on Postgres uses `SELECT DISTINCT ON (slug)` in a single SQL query to fix the PgBouncer round-trip timeout (60s → ~6s) while preserving `engine.getAllSlugs()`'s `Set` semantics on multi-source brains. Gated by `engine.kind === 'postgres'` at the call site so PGLite never enters batch; fallback `catch` logs at `GBRAIN_DEBUG=1` so real Postgres errors are diagnosable. - `src/commands/doctor.ts` — `gbrain doctor [--json] [--fast] [--fix] [--dry-run] [--index-audit]`: health checks. v0.12.3 added `jsonb_integrity` + `markdown_body_completeness` reliability checks. v0.14.1: `--fix` delegates inlined cross-cutting rules to `> **Convention:** see [path](path).` callouts (pipes DRY violations into `src/core/dry-fix.ts`); `--fix --dry-run` previews without writing. v0.14.2: `schema_version` check fails loudly when `version=0` (migrations never ran — the #218 `bun install -g` signature) and routes users to `gbrain apply-migrations --yes`; new opt-in `--index-audit` flag (Postgres-only) reports zero-scan indexes from `pg_stat_user_indexes` (informational only, no auto-drop). v0.15.2: every DB check is wrapped in a progress phase; `markdown_body_completeness` runs under a 1s heartbeat timer so 10+ min scans are observable on 50K-page brains. v0.19.1 added `queue_health` (Postgres-only) with two subchecks: stalled-forever active jobs (started_at > 1h) and waiting-depth-per-name > threshold (default 10, override via `GBRAIN_QUEUE_WAITING_THRESHOLD`). Worker-heartbeat subcheck intentionally deferred to follow-up B7 because it needs a `minion_workers` table to produce ground-truth signal. Fix hints point at `gbrain repair-jsonb`, `gbrain sync --force`, `gbrain apply-migrations`, and `gbrain jobs get/cancel `. - `src/core/migrate.ts` — schema-migration runner. Owns the `MIGRATIONS` array (source of truth for schema DDL). v0.14.2 extended the `Migration` interface with `sqlFor?: { postgres?, pglite? }` (engine-specific SQL overrides `sql`) and `transaction?: boolean` (set to false for `CREATE INDEX CONCURRENTLY`, which Postgres refuses inside a transaction; ignored on PGLite since it has no concurrent writers). Migration v14 (fix wave) uses a handler branching on `engine.kind` to run CONCURRENTLY on Postgres (with a pre-drop of any invalid remnant via `pg_index.indisvalid`) and plain `CREATE INDEX` on PGLite. v15 bumps `minion_jobs.max_stalled` default 1→5 and backfills existing non-terminal rows. v0.22.6.1: migration v24 (`rls_backfill_missing_tables`) uses `sqlFor: { pglite: '' }` to no-op on PGLite — PGLite has no RLS engine and is single-tenant by definition, and the v24 ALTERs target subagent tables that don't exist in pglite-schema.ts. Closes #395 (contributed by @jdcastro2). - `src/core/progress.ts` — Shared bulk-action progress reporter. Writes to stderr. Modes: `auto` (TTY: `\r`-rewriting; non-TTY: plain lines), `human`, `json` (JSONL), `quiet`. Rate-gated by `minIntervalMs` and `minItems`. `startHeartbeat(reporter, note)` helper for single long queries. `child()` composes phase paths. Singleton SIGINT/SIGTERM coordinator emits `abort` events for every live phase. EPIPE defense on both sync throws and stream `'error'` events. Zero dependencies. Introduced in v0.15.2. @@ -288,6 +289,7 @@ E2E tests (`test/e2e/`): Run against real Postgres+pgvector. Require `DATABASE_U - `test/e2e/search-quality.test.ts` runs search quality E2E against PGLite (no API keys, in-memory) - `test/e2e/graph-quality.test.ts` runs the v0.10.3 knowledge graph pipeline (auto-link via put_page, reconciliation, traversePaths) against PGLite in-memory - `test/e2e/postgres-jsonb.test.ts` — v0.12.2 regression test. Round-trips all 5 JSONB write sites (pages.frontmatter, raw_data.data, ingest_log.pages_updated, files.metadata, page_versions.frontmatter) against real Postgres and asserts `jsonb_typeof='object'` plus `->>'key'` returns the expected scalar. The test that should have caught the original double-encode bug. +- `test/e2e/integrity-batch.test.ts` (v0.22.8) — parity tests for `scanIntegrity`'s batch-load fast path vs sequential. Four cases (dedup, hits, validate, topPages) seed a fixture and assert both paths return identical results. Dedup case uses raw SQL via `getConn().unsafe()` to seed a `(test-source-2, people/alice)` row alongside the default-source row, since `engine.putPage` doesn't take a `source_id`. Pins the codex-caught multi-source overcounting regression. - `test/e2e/jsonb-roundtrip.test.ts` — v0.12.3 companion regression against the 4 doctor-scanned JSONB sites. Assertion-level overlap with `postgres-jsonb.test.ts` is intentional defense-in-depth: if doctor's scan surface ever drifts from the actual write surface, one of these tests catches it. - `test/e2e/upgrade.test.ts` runs check-update E2E against real GitHub API (network required) - `test/e2e/minions-shell-pglite.test.ts` (v0.20.4) exercises the PGLite `--follow` inline shell-job path (in-memory, no `DATABASE_URL` required) — the path the consolidated minion-orchestrator skill documents for dev use diff --git a/VERSION b/VERSION index 9e432c86f..eca6a4a43 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.22.7 +0.22.8 diff --git a/llms-full.txt b/llms-full.txt index 85f0e9f25..fd54666ff 100644 --- a/llms-full.txt +++ b/llms-full.txt @@ -179,6 +179,7 @@ strict behavior when unset. - `src/commands/migrations/` — TS migration registry (compiled into the binary; no filesystem walk of `skills/migrations/*.md` needed at runtime). `index.ts` lists migrations in semver order. `v0_11_0.ts` = Minions adoption orchestrator (8 phases). `v0_12_0.ts` = Knowledge Graph auto-wire orchestrator (5 phases: schema → config check → backfill links → backfill timeline → verify). `phaseASchema` has a 600s timeout (bumped from 60s in v0.12.1 for duplicate-heavy brains). `v0_12_2.ts` = JSONB double-encode repair orchestrator (4 phases: schema → repair-jsonb → verify → record). `v0_14_0.ts` = shell-jobs + autopilot cooperative (2 phases: schema ALTER minion_jobs.max_stalled SET DEFAULT 3 — superseded by v0.14.3's schema-level DEFAULT 5 + UPDATE backfill; pending-host-work ping for skills/migrations/v0.14.0.md). All orchestrators are idempotent and resumable from `partial` status. As of v0.14.2 (Bug 3), the RUNNER owns all ledger writes — orchestrators return `OrchestratorResult` and `apply-migrations.ts` persists a canonical `{version, status, phases}` shape after return. Orchestrators no longer call `appendCompletedMigration` directly. `statusForVersion` prefers `complete` over `partial` (never regresses). 3 consecutive partials → wedged → `--force-retry ` writes a `'retry'` reset marker. v0.14.3 (fix wave) ships schema-only migrations v14 (`pages_updated_at_index`) + v15 (`minion_jobs_max_stalled_default_5` with UPDATE backfill) via the `MIGRATIONS` array in `src/core/migrate.ts` — no orchestrator phases needed. - `src/commands/repair-jsonb.ts` — `gbrain repair-jsonb [--dry-run] [--json]`: rewrites `jsonb_typeof='string'` rows in place across 5 affected columns (pages.frontmatter, raw_data.data, ingest_log.pages_updated, files.metadata, page_versions.frontmatter). Fixes v0.12.0 double-encode bug on Postgres; PGLite no-ops. Idempotent. - `src/commands/orphans.ts` — `gbrain orphans [--json] [--count] [--include-pseudo]`: surfaces pages with zero inbound wikilinks, grouped by domain. Auto-generated/raw/pseudo pages filtered by default. Also exposed as `find_orphans` MCP operation. Shipped in v0.12.3 (contributed by @knee5). +- `src/commands/integrity.ts` — `gbrain integrity check|auto|review|extract`: bare-tweet detection, dead-link detection, three-bucket repair (auto-repair / review-queue / skip). `scanIntegrity()` is the shared library function called from `gbrain doctor` (sampled at limit=500) and `cmdCheck` (full scan). v0.22.8: batch-load fast path on Postgres uses `SELECT DISTINCT ON (slug)` in a single SQL query to fix the PgBouncer round-trip timeout (60s → ~6s) while preserving `engine.getAllSlugs()`'s `Set` semantics on multi-source brains. Gated by `engine.kind === 'postgres'` at the call site so PGLite never enters batch; fallback `catch` logs at `GBRAIN_DEBUG=1` so real Postgres errors are diagnosable. - `src/commands/doctor.ts` — `gbrain doctor [--json] [--fast] [--fix] [--dry-run] [--index-audit]`: health checks. v0.12.3 added `jsonb_integrity` + `markdown_body_completeness` reliability checks. v0.14.1: `--fix` delegates inlined cross-cutting rules to `> **Convention:** see [path](path).` callouts (pipes DRY violations into `src/core/dry-fix.ts`); `--fix --dry-run` previews without writing. v0.14.2: `schema_version` check fails loudly when `version=0` (migrations never ran — the #218 `bun install -g` signature) and routes users to `gbrain apply-migrations --yes`; new opt-in `--index-audit` flag (Postgres-only) reports zero-scan indexes from `pg_stat_user_indexes` (informational only, no auto-drop). v0.15.2: every DB check is wrapped in a progress phase; `markdown_body_completeness` runs under a 1s heartbeat timer so 10+ min scans are observable on 50K-page brains. v0.19.1 added `queue_health` (Postgres-only) with two subchecks: stalled-forever active jobs (started_at > 1h) and waiting-depth-per-name > threshold (default 10, override via `GBRAIN_QUEUE_WAITING_THRESHOLD`). Worker-heartbeat subcheck intentionally deferred to follow-up B7 because it needs a `minion_workers` table to produce ground-truth signal. Fix hints point at `gbrain repair-jsonb`, `gbrain sync --force`, `gbrain apply-migrations`, and `gbrain jobs get/cancel `. - `src/core/migrate.ts` — schema-migration runner. Owns the `MIGRATIONS` array (source of truth for schema DDL). v0.14.2 extended the `Migration` interface with `sqlFor?: { postgres?, pglite? }` (engine-specific SQL overrides `sql`) and `transaction?: boolean` (set to false for `CREATE INDEX CONCURRENTLY`, which Postgres refuses inside a transaction; ignored on PGLite since it has no concurrent writers). Migration v14 (fix wave) uses a handler branching on `engine.kind` to run CONCURRENTLY on Postgres (with a pre-drop of any invalid remnant via `pg_index.indisvalid`) and plain `CREATE INDEX` on PGLite. v15 bumps `minion_jobs.max_stalled` default 1→5 and backfills existing non-terminal rows. v0.22.6.1: migration v24 (`rls_backfill_missing_tables`) uses `sqlFor: { pglite: '' }` to no-op on PGLite — PGLite has no RLS engine and is single-tenant by definition, and the v24 ALTERs target subagent tables that don't exist in pglite-schema.ts. Closes #395 (contributed by @jdcastro2). - `src/core/progress.ts` — Shared bulk-action progress reporter. Writes to stderr. Modes: `auto` (TTY: `\r`-rewriting; non-TTY: plain lines), `human`, `json` (JSONL), `quiet`. Rate-gated by `minIntervalMs` and `minItems`. `startHeartbeat(reporter, note)` helper for single long queries. `child()` composes phase paths. Singleton SIGINT/SIGTERM coordinator emits `abort` events for every live phase. EPIPE defense on both sync throws and stream `'error'` events. Zero dependencies. Introduced in v0.15.2. @@ -367,6 +368,7 @@ E2E tests (`test/e2e/`): Run against real Postgres+pgvector. Require `DATABASE_U - `test/e2e/search-quality.test.ts` runs search quality E2E against PGLite (no API keys, in-memory) - `test/e2e/graph-quality.test.ts` runs the v0.10.3 knowledge graph pipeline (auto-link via put_page, reconciliation, traversePaths) against PGLite in-memory - `test/e2e/postgres-jsonb.test.ts` — v0.12.2 regression test. Round-trips all 5 JSONB write sites (pages.frontmatter, raw_data.data, ingest_log.pages_updated, files.metadata, page_versions.frontmatter) against real Postgres and asserts `jsonb_typeof='object'` plus `->>'key'` returns the expected scalar. The test that should have caught the original double-encode bug. +- `test/e2e/integrity-batch.test.ts` (v0.22.8) — parity tests for `scanIntegrity`'s batch-load fast path vs sequential. Four cases (dedup, hits, validate, topPages) seed a fixture and assert both paths return identical results. Dedup case uses raw SQL via `getConn().unsafe()` to seed a `(test-source-2, people/alice)` row alongside the default-source row, since `engine.putPage` doesn't take a `source_id`. Pins the codex-caught multi-source overcounting regression. - `test/e2e/jsonb-roundtrip.test.ts` — v0.12.3 companion regression against the 4 doctor-scanned JSONB sites. Assertion-level overlap with `postgres-jsonb.test.ts` is intentional defense-in-depth: if doctor's scan surface ever drifts from the actual write surface, one of these tests catches it. - `test/e2e/upgrade.test.ts` runs check-update E2E against real GitHub API (network required) - `test/e2e/minions-shell-pglite.test.ts` (v0.20.4) exercises the PGLite `--follow` inline shell-job path (in-memory, no `DATABASE_URL` required) — the path the consolidated minion-orchestrator skill documents for dev use diff --git a/package.json b/package.json index ffd929cbe..61f20235c 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "gbrain", - "version": "0.22.7", + "version": "0.22.8", "description": "Postgres-native personal knowledge brain with hybrid RAG search", "type": "module", "main": "src/core/index.ts", diff --git a/src/commands/integrity.ts b/src/commands/integrity.ts index b47cdb3ab..aecbb67f8 100644 --- a/src/commands/integrity.ts +++ b/src/commands/integrity.ts @@ -31,6 +31,7 @@ import { join, dirname } from 'path'; import { loadConfig, toEngineConfig } from '../core/config.ts'; import { createEngine } from '../core/engine-factory.ts'; import type { BrainEngine } from '../core/engine.ts'; +import * as db from '../core/db.ts'; import { BrainWriter } from '../core/output/writer.ts'; import { getDefaultRegistry, @@ -266,6 +267,12 @@ export interface IntegrityScanOptions { limit?: number; /** Slug prefix filter (e.g. "people") — matches slugs starting with `${typeFilter}/`. */ typeFilter?: string; + /** + * When true (default), batch-load pages via a single SQL query instead of + * sequential getPage() calls. Falls back to sequential on error (e.g. PGLite). + * Eliminates 500 round-trips through PgBouncer that caused doctor timeouts. + */ + batchLoad?: boolean; } export interface IntegrityScanResult { @@ -287,7 +294,29 @@ export async function scanIntegrity( engine: BrainEngine, opts: IntegrityScanOptions = {}, ): Promise { - const { limit = Infinity, typeFilter } = opts; + const { limit = Infinity, typeFilter, batchLoad = true } = opts; + + // Fast path: single SQL query instead of N sequential getPage() calls. + // Eliminates ~500 round-trips through PgBouncer that caused doctor to + // timeout on transaction-mode pooling. Postgres-only: PGLite has no + // postgres.js connection, so the gate keeps the GBRAIN_DEBUG fallback + // log clean for real Postgres errors instead of expected PGLite skips. + if (batchLoad && limit !== Infinity && engine.kind === 'postgres') { + try { + return await scanIntegrityBatch(limit, typeFilter); + } catch (err) { + // GBRAIN_DEBUG=1 surfaces real Postgres errors (deadlock, connection + // drop, SQL bug) that would otherwise vanish into the sequential + // fallback. Quiet by default since the fallback is harmless. + if (process.env.GBRAIN_DEBUG) { + console.error( + '[integrity] batch path failed, falling back to sequential:', + err instanceof Error ? err.message : err, + ); + } + } + } + const allSlugs = [...(await engine.getAllSlugs())].sort(); const bareHits: BareTweetHit[] = []; @@ -316,6 +345,52 @@ export async function scanIntegrity( return { pagesScanned, bareHits, externalHits, topPages }; } +/** + * Batch-load integrity scan: fetches all candidate pages in a single SQL + * query, then scans in-memory. Reduces PgBouncer round-trips from ~500 to 1. + */ +async function scanIntegrityBatch( + limit: number, + typeFilter?: string, +): Promise { + const sql = db.getConnection(); + const typeCondition = typeFilter ? sql`AND slug LIKE ${typeFilter + '/%'}` : sql``; + // Boolean validate is the documented contract; stringly-typed 'false' (quoted + // YAML) diverges from the sequential path's strict === false check. Intentional + // — gbrain lint should reject stringly-typed validate at write time. + const validateCondition = sql`AND (frontmatter->>'validate' IS NULL OR frontmatter->>'validate' != 'false')`; + + // DISTINCT ON (slug) mirrors getAllSlugs()'s Set semantics: multi-source + // brains can have the same slug under multiple source_ids (UNIQUE(source_id, slug) + // since v0.18.0); we want one scan per slug, not one per row. + const rows = await sql` + SELECT DISTINCT ON (slug) slug, compiled_truth, frontmatter + FROM pages + WHERE 1=1 ${typeCondition} ${validateCondition} + ORDER BY slug + LIMIT ${limit} + `; + + const bareHits: BareTweetHit[] = []; + const externalHits: ExternalLinkHit[] = []; + + for (const row of rows) { + const slug = row.slug as string; + const compiledTruth = row.compiled_truth as string; + bareHits.push(...findBareTweetHits(compiledTruth, slug)); + externalHits.push(...findExternalLinks(compiledTruth, slug)); + } + + const byPage = new Map(); + for (const h of bareHits) byPage.set(h.slug, (byPage.get(h.slug) ?? 0) + 1); + const topPages = [...byPage.entries()] + .sort((a, b) => b[1] - a[1]) + .slice(0, 10) + .map(([slug, count]) => ({ slug, count })); + + return { pagesScanned: rows.length, bareHits, externalHits, topPages }; +} + // --------------------------------------------------------------------------- // auto — three-bucket repair // --------------------------------------------------------------------------- diff --git a/src/core/skillpack/installer.ts b/src/core/skillpack/installer.ts index ab93968af..049f88472 100644 --- a/src/core/skillpack/installer.ts +++ b/src/core/skillpack/installer.ts @@ -183,7 +183,11 @@ function acquireLock(workspace: string, opts: InstallOptions): void { const existing = readLock(workspace); const staleMs = opts.lockStaleMs ?? DEFAULT_LOCK_STALE_MS; if (existing) { - const age = Date.now() - existing.mtimeMs; + // Clamp to 0. On Linux ext4, statSync().mtimeMs has sub-ms precision; + // Date.now() is integer ms. A file written microseconds ago can report + // a negative age here, which would break the staleMs:0 "any age is stale" + // contract the force-unlock path relies on (CI passes, local macOS masks it). + const age = Math.max(0, Date.now() - existing.mtimeMs); // `staleMs: 0` in tests means "any age counts as stale". Use >= // so a just-written lock qualifies when the threshold is 0. // Negative age (mtime in the future) happens on fast CI filesystems diff --git a/test/e2e/integrity-batch.test.ts b/test/e2e/integrity-batch.test.ts new file mode 100644 index 000000000..950346079 --- /dev/null +++ b/test/e2e/integrity-batch.test.ts @@ -0,0 +1,168 @@ +/** + * E2E parity tests — scanIntegrity batch path vs sequential path. + * + * The batch path (Postgres-only fast path added in v0.20.x) and the sequential + * path (engine.getAllSlugs + getPage loop) MUST return the same result for + * every supported case, otherwise gbrain doctor reports different numbers + * depending on engine type or whether batch was attempted. + * + * Codex review of the original perf commit caught a multi-source dedup + * regression: the batch SQL scanned raw (source_id, slug) rows while + * sequential's getAllSlugs() returned a Set. v0.22.7 adds + * SELECT DISTINCT ON (slug) to the batch SQL; these tests prove parity. + * + * Run: DATABASE_URL=... bun test test/e2e/integrity-batch.test.ts + */ + +import { describe, test, expect, beforeAll, afterAll, beforeEach } from 'bun:test'; +import { hasDatabase, setupDB, teardownDB, getEngine, getConn } from './helpers.ts'; +import { scanIntegrity } from '../../src/commands/integrity.ts'; + +const skip = !hasDatabase(); +const describeE2E = skip ? describe.skip : describe; + +if (skip) { + console.log('Skipping E2E integrity batch parity tests (DATABASE_URL not set)'); +} + +describeE2E('scanIntegrity batch parity (E2E, Postgres-only)', () => { + beforeAll(async () => { + await setupDB(); + }); + + afterAll(async () => { + await teardownDB(); + }); + + beforeEach(async () => { + // Clean slate per case so fixtures don't leak across describes. + const conn = getConn(); + await conn.unsafe(`TRUNCATE pages CASCADE`); + }); + + describe('dedup', () => { + test('multi-source duplicate slugs scan once, not once-per-source', async () => { + const engine = getEngine(); + const conn = getConn(); + + // Seed default-source page via the engine. + await engine.putPage('people/alice', { + type: 'person', + title: 'Alice', + compiled_truth: 'Alice writes about AI safety.', + timeline: '', + frontmatter: {}, + }); + + // Seed alt-source row via raw SQL — engine.putPage doesn't take a source_id, + // and we specifically need to test that DISTINCT ON (slug) collapses + // the multi-source rows into one scan. + await conn.unsafe(` + INSERT INTO sources (id, name) VALUES ('test-source-2', 'test-source-2') + ON CONFLICT DO NOTHING + `); + await conn.unsafe(` + INSERT INTO pages (source_id, slug, type, title, compiled_truth, timeline, frontmatter) + VALUES ('test-source-2', 'people/alice', 'person', 'Alice (alt source)', + 'Alice from another source.', '', '{}'::jsonb) + `); + + const batchResult = await scanIntegrity(engine, { limit: 100, batchLoad: true }); + const seqResult = await scanIntegrity(engine, { limit: 100, batchLoad: false }); + + // Both paths must report the same number of distinct slugs scanned. + // Pre-fix: batch reported 2 (one per source row), sequential reported 1. + expect(batchResult.pagesScanned).toBe(seqResult.pagesScanned); + expect(batchResult.pagesScanned).toBe(1); + }); + }); + + describe('hits', () => { + test('bareHits and externalHits arrays match between paths', async () => { + const engine = getEngine(); + + await engine.putPage('people/alice', { + type: 'person', + title: 'Alice', + compiled_truth: 'Alice tweeted about AI safety last week.', + timeline: '', + frontmatter: {}, + }); + await engine.putPage('people/bob', { + type: 'person', + title: 'Bob', + compiled_truth: 'Bob wrote at [example](https://example.com/bob).', + timeline: '', + frontmatter: {}, + }); + + const batchResult = await scanIntegrity(engine, { limit: 100, batchLoad: true }); + const seqResult = await scanIntegrity(engine, { limit: 100, batchLoad: false }); + + expect(batchResult.bareHits.length).toBe(seqResult.bareHits.length); + expect(batchResult.externalHits.length).toBe(seqResult.externalHits.length); + expect(batchResult.bareHits.map(h => h.slug).sort()).toEqual( + seqResult.bareHits.map(h => h.slug).sort(), + ); + expect(batchResult.externalHits.map(h => h.slug).sort()).toEqual( + seqResult.externalHits.map(h => h.slug).sort(), + ); + }); + }); + + describe('validate', () => { + test('validate:false (boolean) page is skipped on both paths', async () => { + const engine = getEngine(); + + await engine.putPage('people/alice', { + type: 'person', + title: 'Alice', + compiled_truth: 'Alice tweeted about something.', + timeline: '', + frontmatter: {}, + }); + await engine.putPage('people/legacy', { + type: 'person', + title: 'Legacy', + compiled_truth: 'Legacy tweeted about old stuff.', + timeline: '', + frontmatter: { validate: false }, + }); + + const batchResult = await scanIntegrity(engine, { limit: 100, batchLoad: true }); + const seqResult = await scanIntegrity(engine, { limit: 100, batchLoad: false }); + + expect(batchResult.pagesScanned).toBe(seqResult.pagesScanned); + expect(batchResult.pagesScanned).toBe(1); + expect(batchResult.bareHits.map(h => h.slug)).not.toContain('people/legacy'); + expect(seqResult.bareHits.map(h => h.slug)).not.toContain('people/legacy'); + }); + }); + + describe('topPages', () => { + test('topPages ordering matches between paths', async () => { + const engine = getEngine(); + + // Alice has 2 bare-tweet hits; Bob has 1. + await engine.putPage('people/alice', { + type: 'person', + title: 'Alice', + compiled_truth: 'Alice tweeted today. Alice tweeted yesterday too.', + timeline: '', + frontmatter: {}, + }); + await engine.putPage('people/bob', { + type: 'person', + title: 'Bob', + compiled_truth: 'Bob tweeted once.', + timeline: '', + frontmatter: {}, + }); + + const batchResult = await scanIntegrity(engine, { limit: 100, batchLoad: true }); + const seqResult = await scanIntegrity(engine, { limit: 100, batchLoad: false }); + + expect(batchResult.topPages).toEqual(seqResult.topPages); + }); + }); +}); diff --git a/test/skillpack-install.test.ts b/test/skillpack-install.test.ts index e27316704..753158bf2 100644 --- a/test/skillpack-install.test.ts +++ b/test/skillpack-install.test.ts @@ -11,6 +11,7 @@ import { mkdtempSync, readFileSync, rmSync, + utimesSync, writeFileSync, } from 'fs'; import { dirname, join } from 'path'; @@ -344,6 +345,31 @@ describe('planInstall + applyInstall', () => { expect(result.summary.wroteNew).toBeGreaterThan(0); }); + it('D-CX-11: --force-unlock works when lock mtime is sub-ms ahead of Date.now (Linux fs jitter)', () => { + // Regression guard: on Linux ext4, statSync().mtimeMs has sub-ms precision + // while Date.now() is integer ms, so a just-written lock can report a + // negative age. If acquireLock does not clamp, stale=false and the + // forceUnlock path is unreachable. Simulate deterministically by pushing + // the lock's mtime 10ms into the future. + const { gbrainRoot } = scratchGbrain(); + const { workspace, skillsDir } = scratchTarget(); + const lockFile = join(workspace, '.gbrain-skillpack.lock'); + writeFileSync(lockFile, '99999'); + const future = (Date.now() + 10) / 1000; + utimesSync(lockFile, future, future); + const opts = { + gbrainRoot, + targetWorkspace: workspace, + targetSkillsDir: skillsDir, + skillSlug: 'alpha', + forceUnlock: true, + lockStaleMs: 0, + }; + const plan = planInstall(opts); + const result = applyInstall(plan, opts); + expect(result.summary.wroteNew).toBeGreaterThan(0); + }); + it('managed block is written atomically (tmp then rename)', () => { const { gbrainRoot } = scratchGbrain(); const { workspace, skillsDir } = scratchTarget();