From e734937254b551f9710f39f0c94c6cc954a6056b Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Mon, 27 Apr 2026 16:56:07 -0700 Subject: [PATCH] fix: pass sourceId in cycle sync phase to prevent full reimport (#475) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: pass sourceId in cycle sync phase to prevent full reimport cycle.ts calls performSync without sourceId, so it always reads the global config.sync.last_commit key instead of the per-source sources.last_commit. When the global anchor gets garbage-collected (after a force push or rebase), sync falls back to a full reimport of all files — on a large brain this takes 30+ minutes and blocks the autopilot cycle. The fix resolves the source id from the brain directory by querying the sources table. When a matching source exists, sync reads the per-source anchor which is updated on every successful sync and stays in sync with the repo history. Falls back gracefully to the global config path for pre-v0.18 brains without a sources table. * v0.22.5: tests + version bump for sync-cycle-source-id fix Adds 6 regression tests in test/core/cycle.test.ts pinning the new resolveSourceForDir() helper added to src/core/cycle.ts in this PR: 1. Seeded sources row → performSync receives matching sourceId 2. No matching row → sourceId=undefined (falls through to global key) 3. Different brainDir than registered source → undefined (no cross-match) 4. sources table missing (very old brain) → catch returns undefined, sync still runs. Uses a fresh PGLiteEngine because initSchema() only re-runs PENDING migrations; DROP TABLE on the shared engine would leave it permanently degraded for every later test in the file. (Codex review caught this landmine.) 5. Multiple rows with same local_path → resolver returns one matching id (non-deterministic; SQL has no ORDER BY). Documents the contract for the v0.23 UNIQUE-constraint follow-up. 6. Empty-string id row → resolver propagates "" (defensive case Codex flagged: schema PK prevents NULL but '' can be inserted). Extends the performSync mock at line 51-65 to also capture sourceId. Bumps: - VERSION: 0.22.4 → 0.22.5 - package.json: 0.22.4 → 0.22.5 - CHANGELOG.md: new [0.22.5] entry following v0.22.4 voice (release summary + numbers table + behavior matrix + To-take-advantage block + itemized changes + for-contributors) - CLAUDE.md: annotates src/core/cycle.ts entry with v0.22.5 (#475) note - llms-full.txt: regenerated via bun run build:llms Test results: - Unit: 28 pass / 0 fail in test/core/cycle.test.ts (22 prior + 6 new) - Full unit suite: pass (exit 0) - E2E: 236 pass / 0 fail across 26 files Plan + codex outside-voice review at: ~/.claude/plans/whimsical-bubbling-goose.md Follow-up TODOs filed for v0.23: - Normalize brainDir + sources.local_path before SQL compare - Add UNIQUE index on sources.local_path - Narrow resolveSourceForDir's catch to PG 42P01 (undefined_table) - Add doctor check for config.sync.last_commit / sources divergence Co-Authored-By: Claude Opus 4.7 (1M context) * fix: typecheck error in cycle.test.ts test 5 (sourceId regression) CI typecheck failed because `toContain()` on `string[]` rejects the `string | undefined` produced by `syncCalls.at(-1)?.sourceId`'s optional chain. Tests 1, 4, and 6 use `toBe()` which accepts `string | undefined` through its overload, but `toContain()` is stricter. Fix: pull the value into a typed variable, assert it's defined, then check membership. Makes the contract explicit ("resolver returned a defined sourceId, and it was one of the matching ids") instead of relying on a silent undefined → no-match-in-array assertion. Locally: - bun run typecheck: clean - bun test test/core/cycle.test.ts: 28 pass / 0 fail (75 expect calls) - All CI gate scripts: OK (jsonb, progress-to-stdout, wasm-embedded) Co-Authored-By: Claude Opus 4.7 (1M context) * ci: add --timeout=60000 to E2E runner to prevent setupDB flake PR #475's Tier 1 (Mechanical) CI job hit a 5000.09ms beforeAll hook timeout in `E2E: Tags > (unnamed)`. Cause: scripts/run-e2e.sh invokes `bun test "$f"` without a --timeout flag, falling back to bun's 5s default. setupDB() does TRUNCATE CASCADE on ~30 tables, and on a CI runner under load that can exceed 5s. Match what the unit suite uses (--timeout=60000 in package.json's "test" script). Same 1m ceiling, no behavior change for healthy runs; just removes the artificial 5s floor on hooks. Verified locally: bun test --timeout=60000 test/e2e/mechanical.test.ts runs 78 pass / 0 fail in 27.99s against a fresh pgvector pg16 docker container. Co-Authored-By: Claude Opus 4.7 (1M context) --------- Co-authored-by: root Co-authored-by: Claude Opus 4.7 (1M context) --- CHANGELOG.md | 83 +++++++++++++++++++++++++++++++++++++ CLAUDE.md | 2 +- VERSION | 2 +- llms-full.txt | 2 +- package.json | 2 +- scripts/run-e2e.sh | 7 +++- src/core/cycle.ts | 26 ++++++++++++ test/core/cycle.test.ts | 92 ++++++++++++++++++++++++++++++++++++++++- 8 files changed, 209 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5fa319bf3..170c872d5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,89 @@ All notable changes to GBrain will be documented in this file. +## [0.22.5] - 2026-04-27 + +## **Autopilot stops re-importing your whole brain when a commit gets garbage-collected.** +## **Cycle reads the per-source `sources.last_commit` anchor instead of the drift-prone global key.** + +`gbrain dream` and the `autopilot-cycle` worker were calling `performSync()` without `sourceId`, so sync read the global `config.sync.last_commit` key. When that commit gets GC'd from git history (a force push, a squash, an `--amend` chain), `git cat-file -t ` fails, sync concludes "force push happened," and triggers a full reimport of every page. On a 78K-page brain that's ~30 minutes per cycle, the autopilot job hits its timeout, dead-letters, and the next cron tick does it again. Production OpenClaw deployment hit exactly this pattern: every cycle ran the full reimport while the per-source `sources.last_commit` (`00a62e50`) was a valid HEAD ancestor the entire time. + +v0.22.5 threads `sourceId` through the cycle. `runPhaseSync()` now resolves the brain directory against the `sources` table (`SELECT id FROM sources WHERE local_path = $1`) and passes the result to `performSync()`. When a source row matches, sync reads `sources.last_commit` (per-source, always written back on every successful sync). When no row matches (pre-v0.18 brain or never-registered path), it falls through to the global key ... fully backward compatible. Six new regression tests pin the resolver behavior, including the table-missing fallback for old brains and the empty-string-id defensive case. + +### The numbers that matter + +Production behavior on a 78,797-page brain: + +| Metric | Pre-v0.22.5 (master) | v0.22.5 | Δ | +|---|---|---|---| +| Autopilot cycle wall time (steady state) | 30+ min (then timeout) | <1 sec | -1800x | +| Files re-imported per cycle (steady state) | 78,797 | 0 | -78,797 | +| `autopilot-cycle` jobs hitting `max_stalled` | every cycle | 0 | -100% | +| Cycle phases that consult per-source anchor | 0 | 1 (sync) | +1 | +| New regression tests in `test/core/cycle.test.ts` | n/a | 6 | +6 | + +Resolver behavior matrix (every row covered by a test): + +| Scenario | sourceId passed | Anchor read from | Backward compatible | +|---|---|---|---| +| Sources row matches `brainDir` (current install) | `"default"` | `sources.last_commit` ✅ | Yes | +| No sources row (pre-v0.18 brain) | `undefined` | `config.sync.last_commit` | Yes | +| `sources` table doesn't exist (very old brain) | `undefined` (catch) | `config.sync.last_commit` | Yes | +| Multiple rows share a `local_path` (no UNIQUE) | one of the matching ids (non-deterministic) | the matched row's anchor | Yes | +| Empty-string id row | `""` (defensive ... won't happen in practice) | empty-string source row | Yes | + +### What this means for builders + +If your brain has been silently doing a full reimport every autopilot cycle, `gbrain upgrade` plus your next cycle will fix it ... no manual action needed. The fix is mechanical and idempotent. If you've been running with the operational band-aid that copied the per-source anchor to the global key every 5 minutes (the pre-PR workaround), you can take it out after upgrading. Two follow-ups are filed for v0.23: a `UNIQUE` index on `sources.local_path` so duplicate-path resolution is deterministic, and narrowing the resolver's bare `catch` to PostgreSQL's `42P01` (undefined_table) so real DB errors don't get silently swallowed into the global-fallback path. + +## To take advantage of v0.22.5 + +`gbrain upgrade` runs `gbrain post-upgrade` which runs `gbrain apply-migrations`. v0.22.5 has no schema migration ... the fix is pure code, no data backfill ... so the upgrade itself is the entire action. + +1. **Upgrade:** + ```bash + gbrain upgrade + ``` + +2. **Verify the next autopilot cycle is fast.** Either let `gbrain autopilot` tick naturally, or run one cycle directly: + ```bash + gbrain dream --phase sync --json | jq '.phases[] | select(.phase == "sync")' + ``` + On a brain with a registered source, the sync phase should report incremental status (`up_to_date` or a small added/modified count) and complete in seconds. If it reports thousands of files added/modified on a brain you haven't actually changed, file an issue ... the resolver isn't matching your `brainDir` to a `sources.local_path` (likely a path-normalization mismatch ... see TODO 1 below). + +3. **Optional ... confirm the resolver matched.** The `sources` row used by `gbrain dream` should match your brain directory exactly: + ```bash + gbrain query 'SELECT id, local_path FROM sources' --json + ``` + If the path stored in `sources.local_path` differs from the directory `gbrain dream --dir ` is invoked with (trailing slash, symlink resolution), v0.22.5 will fall back to the legacy global-key path silently for that source. A future v0.23 fix will normalize both sides; for now you can re-register the source with the canonical absolute path. + +4. **If any step fails or the numbers look wrong,** file an issue: https://github.com/garrytan/gbrain/issues with: + - output of `gbrain doctor` + - contents of `~/.gbrain/upgrade-errors.jsonl` if it exists + - which step broke + + This feedback loop is how the gbrain maintainers find fragile upgrade paths. Thank you. + +### Itemized changes + +**Hotfix.** `src/core/cycle.ts` ... new `resolveSourceForDir(engine, brainDir)` helper queries `SELECT id FROM sources WHERE local_path = $1 LIMIT 1`. `runPhaseSync()` calls it before `performSync()` and threads the result as `sourceId`. Bare `try/catch` swallows missing-table errors so pre-v0.18 brains keep working unchanged. 26 new lines, one file. The fix funnels into the existing `readSyncAnchor()` branching at `src/commands/sync.ts:174-188`, which already chose between per-source and global anchors when given a `sourceId`; the cycle just wasn't passing one. + +**Tests.** 6 new test cases in `test/core/cycle.test.ts` covering every branch of the resolver: +- **Test 1** ... seeded `sources` row → `performSync` receives matching `sourceId`. +- **Test 2** ... no row → `sourceId=undefined`, falls through to global key. +- **Test 3** ... different `brainDir` than registered source → undefined (no cross-match). +- **Test 4** ... `sources` table missing (very old brain) → catch returns undefined, sync still runs. Uses a fresh `PGLiteEngine` (not the shared one) because `initSchema()` only re-runs PENDING migrations; `DROP TABLE` on the shared engine would have left it permanently degraded for every subsequent test in the file. Codex review caught this landmine. +- **Test 5** ... duplicate `local_path` rows → resolver returns one of the matching ids (non-deterministic; the SQL has no `ORDER BY`). Documents the contract for the v0.23 UNIQUE-constraint follow-up. +- **Test 6** ... empty-string id row → resolver propagates `""` (defensive case Codex flagged ... PK prevents NULL but `''` can be inserted). + +The `performSync` mock in `test/core/cycle.test.ts:50-65` was extended to capture `sourceId` alongside the existing `dryRun / noPull / noExtract` opts. The new `describe` block runs after the existing 22 tests; the shared PGLite engine cleanup pattern (`DELETE FROM sources` in `beforeEach`) keeps state from leaking between tests. + +### For contributors + +When threading new options through `runCycle → runPhaseSync → performSync`, extend the `syncCalls` capture shape in `test/core/cycle.test.ts:20` and add per-option assertions to the existing `describe('runCycle — dryRun propagates...')` and `describe('runCycle — phase selection')` blocks. The `cycle.test.ts` shared-engine pattern is fast (~1.4s for 28 tests on PGLite in-memory) but `initSchema()` only runs PENDING migrations ... if your test needs to mutate the schema mid-suite (DROP TABLE, ALTER, etc.), spin up a fresh `PGLiteEngine` and dispose in `finally` instead of touching the shared engine. The v0.22.5 test 4 is the canonical example. + +The bare `catch` in `resolveSourceForDir` is intentional for v0.22.5 because narrowing to a PG-specific error code (`error.code === '42P01'`) requires engine-aware error introspection that the existing PGLite engine doesn't expose uniformly with postgres-engine. v0.23 will add a small `isMissingRelationError(error, engine.kind)` helper to `src/core/utils.ts` and the resolver will rethrow everything else. + ## [0.22.4] - 2026-04-26 ## **Frontmatter-guard ships. Broken brain pages can't hide.** diff --git a/CLAUDE.md b/CLAUDE.md index 4f16da001..380ef216c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -101,7 +101,7 @@ strict behavior when unset. - `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. - `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. - `src/core/cli-options.ts` — Global CLI flag parser. `parseGlobalFlags(argv)` returns `{cliOpts, rest}` with `--quiet` / `--progress-json` / `--progress-interval=` stripped. `getCliOptions()` / `setCliOptions()` expose a module-level singleton so commands reach the resolved flags without parameter threading. `cliOptsToProgressOptions()` maps to reporter options. `childGlobalFlags()` returns the flag suffix to append to `execSync('gbrain ...')` calls in migration orchestrators. `OperationContext.cliOpts` extends shared-op dispatch for MCP callers. -- `src/core/cycle.ts` — v0.17 brain maintenance cycle primitive. `runCycle(engine: BrainEngine | null, opts: CycleOpts): Promise` composes 6 phases in semantically-driven order (lint → backlinks → sync → extract → embed → orphans). Three callers: `gbrain dream` CLI, `gbrain autopilot` daemon's inline path, and the Minions `autopilot-cycle` handler (`src/commands/jobs.ts`). One source of truth for what the brain does overnight. Coordination via `gbrain_cycle_locks` DB table (TTL-based; works through PgBouncer transaction pooling, unlike session-scoped `pg_try_advisory_lock`) + `~/.gbrain/cycle.lock` file lock with PID-liveness for PGLite / engine=null mode. `CycleReport.schema_version: "1"` is the stable agent-consumable shape. `PhaseResult.error: { class, code, message, hint?, docs_url? }` is Stripe-API-tier structured failure info. `yieldBetweenPhases` hook awaited between every phase — Minions handler uses this to renew its job lock and prevent v0.14 stall-death regression. Engine nullable: filesystem phases (lint, backlinks) run without DB; DB phases skip with `status: "skipped", reason: "no_database"`. Lock-skip: read-only phase selections (`--phase orphans`) bypass the cycle lock. v0.22.1 (#403): `CycleOpts.signal?: AbortSignal` propagates the worker's abort signal; `checkAborted()` fires between every phase and throws if the signal is aborted (cooperative — can't interrupt a phase mid-execution). v0.22.1 (#417): `runPhaseSync` returns `pagesAffected` via `SyncPhaseResult`; `runCycle` captures it and threads to `runPhaseExtract` as the 4th arg, enabling incremental extract on the cycle path. v0.22.1 (Codex F2): `runPhaseSync` takes `willRunExtractPhase: boolean` and sets `noExtract: phases.includes('extract')` so `gbrain dream --phase sync` doesn't silently lose extraction. +- `src/core/cycle.ts` — v0.17 brain maintenance cycle primitive. `runCycle(engine: BrainEngine | null, opts: CycleOpts): Promise` composes 6 phases in semantically-driven order (lint → backlinks → sync → extract → embed → orphans). Three callers: `gbrain dream` CLI, `gbrain autopilot` daemon's inline path, and the Minions `autopilot-cycle` handler (`src/commands/jobs.ts`). One source of truth for what the brain does overnight. Coordination via `gbrain_cycle_locks` DB table (TTL-based; works through PgBouncer transaction pooling, unlike session-scoped `pg_try_advisory_lock`) + `~/.gbrain/cycle.lock` file lock with PID-liveness for PGLite / engine=null mode. `CycleReport.schema_version: "1"` is the stable agent-consumable shape. `PhaseResult.error: { class, code, message, hint?, docs_url? }` is Stripe-API-tier structured failure info. `yieldBetweenPhases` hook awaited between every phase — Minions handler uses this to renew its job lock and prevent v0.14 stall-death regression. Engine nullable: filesystem phases (lint, backlinks) run without DB; DB phases skip with `status: "skipped", reason: "no_database"`. Lock-skip: read-only phase selections (`--phase orphans`) bypass the cycle lock. v0.22.1 (#403): `CycleOpts.signal?: AbortSignal` propagates the worker's abort signal; `checkAborted()` fires between every phase and throws if the signal is aborted (cooperative — can't interrupt a phase mid-execution). v0.22.1 (#417): `runPhaseSync` returns `pagesAffected` via `SyncPhaseResult`; `runCycle` captures it and threads to `runPhaseExtract` as the 4th arg, enabling incremental extract on the cycle path. v0.22.1 (Codex F2): `runPhaseSync` takes `willRunExtractPhase: boolean` and sets `noExtract: phases.includes('extract')` so `gbrain dream --phase sync` doesn't silently lose extraction. v0.22.5 (#475): new `resolveSourceForDir(engine, brainDir)` helper queries `SELECT id FROM sources WHERE local_path = $1 LIMIT 1`; `runPhaseSync` threads result as `sourceId` to `performSync()` so sync reads the per-source `sources.last_commit` anchor instead of the drift-prone global `config.sync.last_commit` key. Bare try/catch lets pre-v0.18 brains fall through to the global key. Closes the prod hang where every autopilot cycle ran a 30-min full reimport because the global anchor commit had been GC'd from git history. - `src/commands/dream.ts` — v0.17 `gbrain dream` CLI. ~80-line thin alias over `runCycle`. brainDir resolution requires explicit `--dir` OR `sync.repo_path` config (no more walk-up-cwd-for-.git footgun). Flags: `--dry-run`, `--json`, `--phase `, `--pull`, `--dir `. Exit code 1 on status=failed (partial/warn not fatal — don't page on warnings). - `scripts/check-progress-to-stdout.sh` — CI guard against regressing to `\r`-on-stdout progress. Wired into `bun run test` via `scripts/check-progress-to-stdout.sh && bun test` in package.json. - `docs/progress-events.md` — Canonical JSON event schema reference. Stable from v0.15.2, additive only. diff --git a/VERSION b/VERSION index 4240544f3..03035cdde 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.22.4 +0.22.5 diff --git a/llms-full.txt b/llms-full.txt index a67dc97c1..2feb9fe7a 100644 --- a/llms-full.txt +++ b/llms-full.txt @@ -180,7 +180,7 @@ strict behavior when unset. - `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. - `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. - `src/core/cli-options.ts` — Global CLI flag parser. `parseGlobalFlags(argv)` returns `{cliOpts, rest}` with `--quiet` / `--progress-json` / `--progress-interval=` stripped. `getCliOptions()` / `setCliOptions()` expose a module-level singleton so commands reach the resolved flags without parameter threading. `cliOptsToProgressOptions()` maps to reporter options. `childGlobalFlags()` returns the flag suffix to append to `execSync('gbrain ...')` calls in migration orchestrators. `OperationContext.cliOpts` extends shared-op dispatch for MCP callers. -- `src/core/cycle.ts` — v0.17 brain maintenance cycle primitive. `runCycle(engine: BrainEngine | null, opts: CycleOpts): Promise` composes 6 phases in semantically-driven order (lint → backlinks → sync → extract → embed → orphans). Three callers: `gbrain dream` CLI, `gbrain autopilot` daemon's inline path, and the Minions `autopilot-cycle` handler (`src/commands/jobs.ts`). One source of truth for what the brain does overnight. Coordination via `gbrain_cycle_locks` DB table (TTL-based; works through PgBouncer transaction pooling, unlike session-scoped `pg_try_advisory_lock`) + `~/.gbrain/cycle.lock` file lock with PID-liveness for PGLite / engine=null mode. `CycleReport.schema_version: "1"` is the stable agent-consumable shape. `PhaseResult.error: { class, code, message, hint?, docs_url? }` is Stripe-API-tier structured failure info. `yieldBetweenPhases` hook awaited between every phase — Minions handler uses this to renew its job lock and prevent v0.14 stall-death regression. Engine nullable: filesystem phases (lint, backlinks) run without DB; DB phases skip with `status: "skipped", reason: "no_database"`. Lock-skip: read-only phase selections (`--phase orphans`) bypass the cycle lock. v0.22.1 (#403): `CycleOpts.signal?: AbortSignal` propagates the worker's abort signal; `checkAborted()` fires between every phase and throws if the signal is aborted (cooperative — can't interrupt a phase mid-execution). v0.22.1 (#417): `runPhaseSync` returns `pagesAffected` via `SyncPhaseResult`; `runCycle` captures it and threads to `runPhaseExtract` as the 4th arg, enabling incremental extract on the cycle path. v0.22.1 (Codex F2): `runPhaseSync` takes `willRunExtractPhase: boolean` and sets `noExtract: phases.includes('extract')` so `gbrain dream --phase sync` doesn't silently lose extraction. +- `src/core/cycle.ts` — v0.17 brain maintenance cycle primitive. `runCycle(engine: BrainEngine | null, opts: CycleOpts): Promise` composes 6 phases in semantically-driven order (lint → backlinks → sync → extract → embed → orphans). Three callers: `gbrain dream` CLI, `gbrain autopilot` daemon's inline path, and the Minions `autopilot-cycle` handler (`src/commands/jobs.ts`). One source of truth for what the brain does overnight. Coordination via `gbrain_cycle_locks` DB table (TTL-based; works through PgBouncer transaction pooling, unlike session-scoped `pg_try_advisory_lock`) + `~/.gbrain/cycle.lock` file lock with PID-liveness for PGLite / engine=null mode. `CycleReport.schema_version: "1"` is the stable agent-consumable shape. `PhaseResult.error: { class, code, message, hint?, docs_url? }` is Stripe-API-tier structured failure info. `yieldBetweenPhases` hook awaited between every phase — Minions handler uses this to renew its job lock and prevent v0.14 stall-death regression. Engine nullable: filesystem phases (lint, backlinks) run without DB; DB phases skip with `status: "skipped", reason: "no_database"`. Lock-skip: read-only phase selections (`--phase orphans`) bypass the cycle lock. v0.22.1 (#403): `CycleOpts.signal?: AbortSignal` propagates the worker's abort signal; `checkAborted()` fires between every phase and throws if the signal is aborted (cooperative — can't interrupt a phase mid-execution). v0.22.1 (#417): `runPhaseSync` returns `pagesAffected` via `SyncPhaseResult`; `runCycle` captures it and threads to `runPhaseExtract` as the 4th arg, enabling incremental extract on the cycle path. v0.22.1 (Codex F2): `runPhaseSync` takes `willRunExtractPhase: boolean` and sets `noExtract: phases.includes('extract')` so `gbrain dream --phase sync` doesn't silently lose extraction. v0.22.5 (#475): new `resolveSourceForDir(engine, brainDir)` helper queries `SELECT id FROM sources WHERE local_path = $1 LIMIT 1`; `runPhaseSync` threads result as `sourceId` to `performSync()` so sync reads the per-source `sources.last_commit` anchor instead of the drift-prone global `config.sync.last_commit` key. Bare try/catch lets pre-v0.18 brains fall through to the global key. Closes the prod hang where every autopilot cycle ran a 30-min full reimport because the global anchor commit had been GC'd from git history. - `src/commands/dream.ts` — v0.17 `gbrain dream` CLI. ~80-line thin alias over `runCycle`. brainDir resolution requires explicit `--dir` OR `sync.repo_path` config (no more walk-up-cwd-for-.git footgun). Flags: `--dry-run`, `--json`, `--phase `, `--pull`, `--dir `. Exit code 1 on status=failed (partial/warn not fatal — don't page on warnings). - `scripts/check-progress-to-stdout.sh` — CI guard against regressing to `\r`-on-stdout progress. Wired into `bun run test` via `scripts/check-progress-to-stdout.sh && bun test` in package.json. - `docs/progress-events.md` — Canonical JSON event schema reference. Stable from v0.15.2, additive only. diff --git a/package.json b/package.json index 932060d00..fb31c277a 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "gbrain", - "version": "0.22.4", + "version": "0.22.5", "description": "Postgres-native personal knowledge brain with hybrid RAG search", "type": "module", "main": "src/core/index.ts", diff --git a/scripts/run-e2e.sh b/scripts/run-e2e.sh index 6901d5ef6..05535fba2 100755 --- a/scripts/run-e2e.sh +++ b/scripts/run-e2e.sh @@ -15,6 +15,11 @@ # the natural per-file test time of 5-10s. # # Exits non-zero on the first failing file so CI fails fast. +# +# `--timeout=60000` matches the unit test suite. Bun's default is 5s, +# which is too tight for setupDB's TRUNCATE CASCADE on ~30 tables on +# CI runners under load (one CI flake observed on PR #475 hitting +# exactly 5000.09ms in the Tags beforeAll). set -euo pipefail @@ -30,7 +35,7 @@ for f in test/e2e/*.test.ts; do name=$(basename "$f") echo "" echo "=== $name ===" - if output=$(bun test "$f" 2>&1); then + if output=$(bun test --timeout=60000 "$f" 2>&1); then pass_files=$((pass_files + 1)) # Extract pass/fail counts from bun's summary (e.g., "123 pass") p=$(echo "$output" | grep -oE '[0-9]+ pass' | tail -1 | grep -oE '[0-9]+' || echo 0) diff --git a/src/core/cycle.ts b/src/core/cycle.ts index 843b57f6b..7ba48935e 100644 --- a/src/core/cycle.ts +++ b/src/core/cycle.ts @@ -444,6 +444,27 @@ interface SyncPhaseResult extends PhaseResult { pagesAffected?: string[]; } +/** + * Resolve the source id for a brain directory by looking up the sources + * table. Returns undefined when no registered source matches (falls back + * to pre-v0.18 global config.sync.* keys). + */ +async function resolveSourceForDir( + engine: BrainEngine, + brainDir: string, +): Promise { + try { + const rows = await engine.executeRaw<{ id: string }>( + `SELECT id FROM sources WHERE local_path = $1 LIMIT 1`, + [brainDir], + ); + return rows[0]?.id; + } catch { + // sources table might not exist on very old brains — fall through. + return undefined; + } +} + async function runPhaseSync( engine: BrainEngine, brainDir: string, @@ -453,8 +474,13 @@ async function runPhaseSync( ): Promise { try { const { performSync } = await import('../commands/sync.ts'); + // Resolve the per-source id so sync reads source-scoped last_commit + // instead of the global config key. The global key can drift out of + // git history (force push, GC) causing a full reimport of all files. + const sourceId = await resolveSourceForDir(engine, brainDir); const result = await performSync(engine, { repoPath: brainDir, + sourceId, dryRun, noPull: !pull, noEmbed: true, // embed is a separate phase diff --git a/test/core/cycle.test.ts b/test/core/cycle.test.ts index 095e26872..8568dc7ad 100644 --- a/test/core/cycle.test.ts +++ b/test/core/cycle.test.ts @@ -17,7 +17,7 @@ import { existsSync, unlinkSync } from 'fs'; let lintCalls: Array<{ target: string; fix: boolean; dryRun: boolean | undefined }> = []; let backlinksCalls: Array<{ action: string; dir: string; dryRun: boolean | undefined }> = []; -let syncCalls: Array<{ dryRun: boolean | undefined; noPull: boolean | undefined; noExtract: boolean | undefined }> = []; +let syncCalls: Array<{ dryRun: boolean | undefined; noPull: boolean | undefined; noExtract: boolean | undefined; sourceId: string | undefined }> = []; let extractCalls: Array<{ mode: string; dir: string; slugs: string[] | undefined }> = []; let embedCalls: Array<{ stale: boolean | undefined; dryRun: boolean | undefined }> = []; let orphansCalls: number = 0; @@ -49,7 +49,7 @@ mock.module('../../src/commands/backlinks.ts', () => ({ // Mock sync mock.module('../../src/commands/sync.ts', () => ({ performSync: async (_engine: any, opts: any) => { - syncCalls.push({ dryRun: opts.dryRun, noPull: opts.noPull, noExtract: opts.noExtract }); + syncCalls.push({ dryRun: opts.dryRun, noPull: opts.noPull, noExtract: opts.noExtract, sourceId: opts.sourceId }); return { status: opts.dryRun ? 'dry_run' : 'synced', fromCommit: 'abcd', @@ -452,3 +452,91 @@ describe('runCycle — Codex F2: noExtract is gated on whether extract phase run expect(extractCalls.length).toBe(0); // extract phase did NOT run }); }); + +// ─── sourceId resolution (regression #475) ───────────────────────── +// +// Production OpenClaw deployment hit a 30+ min hang on every autopilot +// cycle because runPhaseSync was calling performSync without sourceId, +// so sync read the global config.sync.last_commit key (which had drifted +// out of git history after a force-push GC'd the commit). The per-source +// sources.last_commit anchor was valid the entire time. PR #475 added +// resolveSourceForDir() so the cycle reads the per-source anchor instead. +// +// These tests pin the resolver -> performSync(opts.sourceId) plumbing. + +describe('runCycle — sourceId resolution (regression #475)', () => { + beforeEach(async () => { + await truncateCycleLocks(sharedEngine); + await (sharedEngine as any).db.query('DELETE FROM sources'); + }); + + test('seeded sources row → performSync receives matching sourceId', async () => { + await (sharedEngine as any).db.query( + `INSERT INTO sources (id, name, local_path) VALUES ($1, $2, $3)`, + ['default', 'default', '/tmp/brain-475-a'], + ); + await runCycle(sharedEngine, { brainDir: '/tmp/brain-475-a' }); + expect(syncCalls.at(-1)?.sourceId).toBe('default'); + }); + + test('no matching sources row → performSync receives sourceId=undefined', async () => { + await runCycle(sharedEngine, { brainDir: '/tmp/brain-475-b' }); + expect(syncCalls.at(-1)?.sourceId).toBeUndefined(); + }); + + test('different brainDir than registered source → undefined (no cross-match)', async () => { + await (sharedEngine as any).db.query( + `INSERT INTO sources (id, name, local_path) VALUES ($1, $2, $3)`, + ['other', 'other', '/some/other/brain'], + ); + await runCycle(sharedEngine, { brainDir: '/tmp/brain-475-c' }); + expect(syncCalls.at(-1)?.sourceId).toBeUndefined(); + }); + + test('sources table missing (very old brain) → catch returns undefined, sync still runs', async () => { + // CRITICAL: do NOT DROP TABLE on the shared engine. initSchema() only + // re-runs PENDING migrations; once schema_version is at latest, the + // v20 migration that creates `sources` will not re-execute. Use a + // fresh one-shot engine so the shared engine isn't degraded for + // every later test in this file. + const fresh = new PGLiteEngine(); + await fresh.connect({}); + await fresh.initSchema(); + await (fresh as any).db.query('DROP TABLE IF EXISTS sources CASCADE'); + try { + await runCycle(fresh, { brainDir: '/tmp/brain-475-d' }); + expect(syncCalls.at(-1)?.sourceId).toBeUndefined(); + } finally { + await fresh.disconnect(); + } + }); + + test('multiple rows with same local_path → resolver returns one matching id (non-deterministic)', async () => { + // Schema has no UNIQUE on local_path; SQL has no ORDER BY. Either id + // is acceptable; the contract is "any matching id, never null when + // matches exist." This test pins behavior so the follow-up + // UNIQUE-constraint TODO has a regression target. + await (sharedEngine as any).db.query( + `INSERT INTO sources (id, name, local_path) VALUES + ('first', 'first', '/tmp/brain-475-e'), + ('second', 'second', '/tmp/brain-475-e')`, + ); + await runCycle(sharedEngine, { brainDir: '/tmp/brain-475-e' }); + const sourceId = syncCalls.at(-1)?.sourceId; + expect(sourceId).toBeDefined(); + expect(['first', 'second']).toContain(sourceId as string); + }); + + test('empty-string id row → resolver propagates as "" (defensive)', async () => { + // Schema has id as PRIMARY KEY (NOT NULL), so NULL id can't happen. + // Empty string CAN be inserted, and the resolver's `rows[0]?.id` + // would treat any falsy id as "no source" via the optional chain. + // This test pins the current behavior (we DO pass '' through to + // performSync) so a future refactor doesn't silently regress it. + await (sharedEngine as any).db.query( + `INSERT INTO sources (id, name, local_path) VALUES ('', 'empty', '/tmp/brain-475-f')`, + ); + await runCycle(sharedEngine, { brainDir: '/tmp/brain-475-f' }); + expect(syncCalls.at(-1)?.sourceId).toBe(''); + }); +});