From 2996112053fd27ddb982c68d03fa48cfd60b6ca5 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Tue, 16 Jun 2026 22:37:38 -0700 Subject: [PATCH] fix(autopilot): per-source cycle binds FS phases to source.local_path, not global repo (#2194 #2227) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A per-source autopilot-cycle inherited the global sync.repo_path as brainDir while stamping DB freshness for source_id — mixed scope. FS phases (sync/lint/extract) ran against the wrong tree, so the failure-cooldown and freshness gates would attribute work to the wrong source. Resolve the source's local_path in the handler (reuse the archive-recheck SELECT) and bind brainDir to it; a pure-DB source gets null (FS phases skip) instead of falling through to the global checkout. Legacy no-source dispatch keeps the global repoPath. Prerequisite for the cooldown/split commits (codex outside-voice #8). Resolves TODOS:634. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/commands/jobs.ts | 25 ++++- test/autopilot-cycle-source-checkout.test.ts | 108 +++++++++++++++++++ 2 files changed, 129 insertions(+), 4 deletions(-) create mode 100644 test/autopilot-cycle-source-checkout.test.ts diff --git a/src/commands/jobs.ts b/src/commands/jobs.ts index a2d7db0a6..cec90bb55 100644 --- a/src/commands/jobs.ts +++ b/src/commands/jobs.ts @@ -1594,6 +1594,13 @@ export async function registerBuiltinHandlers( // archived between fan-out and worker claim, skip cleanly. const rawSourceId = job.data.source_id; let sourceId: string | undefined; + // issue #2227/#2194 (TODOS:634, codex #8): a per-source cycle must run its + // FILESYSTEM phases (sync/lint/extract) against the SOURCE's own checkout, + // not the global brain's. Pre-fix it inherited `repoPath` (the default + // checkout) while writing DB freshness for `source_id` — mixed scope that + // made cooldown/freshness attribute to the wrong source. We resolve the + // source's `local_path` here and use it as the cycle's brainDir below. + let sourceLocalPath: string | null = null; if (rawSourceId !== undefined && rawSourceId !== null) { if (typeof rawSourceId !== 'string') { throw new Error(`autopilot-cycle: invalid source_id (not a string): ${JSON.stringify(rawSourceId)}`); @@ -1607,9 +1614,10 @@ export async function registerBuiltinHandlers( } // Archive recheck (codex r1 P1-5): cheap pre-cycle lookup. Returns // immediately if source is gone or archived; runCycle never even - // acquires a lock. - const rows = await engine.executeRaw<{ archived: boolean | null }>( - `SELECT archived FROM sources WHERE id = $1`, + // acquires a lock. Also fetches local_path so FS phases bind to the + // source's own checkout (the #2227/#2194 mixed-scope fix). + const rows = await engine.executeRaw<{ archived: boolean | null; local_path: string | null }>( + `SELECT archived, local_path FROM sources WHERE id = $1`, [rawSourceId], ); if (rows.length === 0) { @@ -1627,8 +1635,17 @@ export async function registerBuiltinHandlers( }; } sourceId = rawSourceId; + sourceLocalPath = typeof rows[0].local_path === 'string' && rows[0].local_path.length > 0 + ? rows[0].local_path + : null; } + // Effective checkout for FS phases. For a per-source cycle, bind to the + // SOURCE's local_path (or null → skip FS phases for a pure-DB source); + // NEVER fall through to the global repoPath, which would run sync/lint + // against the wrong tree. Legacy (no source_id) keeps the global repoPath. + const effectiveBrainDir: string | null = sourceId ? sourceLocalPath : repoPath; + // Allow callers to select phases via job data (e.g. skip embed for // fast cycles). Validates against ALL_PHASES to prevent injection. const { ALL_PHASES } = await import('../core/cycle.ts'); @@ -1641,7 +1658,7 @@ export async function registerBuiltinHandlers( const pull = typeof job.data.pull === 'boolean' ? job.data.pull : true; const report = await runCycle(engine, { - brainDir: repoPath, + brainDir: effectiveBrainDir, pull, signal: job.signal, // propagate abort so cycle bails on timeout/cancel ...(sourceId ? { sourceId } : {}), diff --git a/test/autopilot-cycle-source-checkout.test.ts b/test/autopilot-cycle-source-checkout.test.ts new file mode 100644 index 000000000..00387eed8 --- /dev/null +++ b/test/autopilot-cycle-source-checkout.test.ts @@ -0,0 +1,108 @@ +/** + * issue #2227/#2194 (TODOS:634, codex #8) — a per-source `autopilot-cycle` + * binds its filesystem phases to the SOURCE's own checkout (`local_path`), + * never the global brain's `sync.repo_path`. + * + * Pre-fix the handler fed `repoPath` (the global checkout) into runCycle even + * when `source_id` was set, so FS phases (sync/lint/extract) ran against the + * wrong tree while DB freshness was stamped for `source_id` — mixed scope. + * That made the failure-cooldown and freshness gates attribute work to the + * wrong source, the prerequisite codex flagged before the storm-breaker could + * be trusted. + * + * Drives the REAL handler captured from registerBuiltinHandlers (not a + * source-grep) so a reintroduced repoPath fallthrough fails here. PGLite + * in-memory. The report's `brain_dir` mirrors the cycle's effective brainDir + * (cycle.ts:2324), so it's the observable proxy for "which checkout did FS + * phases bind to". + */ + +import { describe, test, expect, beforeAll, afterAll, beforeEach } from 'bun:test'; +import { mkdtempSync } from 'fs'; +import { tmpdir } from 'os'; +import { join } from 'path'; +import { PGLiteEngine } from '../src/core/pglite-engine.ts'; +import { registerBuiltinHandlers } from '../src/commands/jobs.ts'; +import { resetPgliteState } from './helpers/reset-pglite.ts'; + +let engine: PGLiteEngine; + +beforeAll(async () => { + engine = new PGLiteEngine(); + await engine.connect({}); + await engine.initSchema(); +}); + +afterAll(async () => { + await engine.disconnect(); +}); + +beforeEach(async () => { + await resetPgliteState(engine); +}); + +async function captureHandlers(): Promise Promise>> { + const handlers = new Map Promise>(); + const fakeWorker = { register(name: string, fn: (job: any) => Promise) { handlers.set(name, fn); } }; + await registerBuiltinHandlers(fakeWorker as never, engine); + return handlers; +} + +describe('autopilot-cycle handler — per-source checkout binding (#2227/#2194)', () => { + test('source_id with local_path → brainDir is the SOURCE checkout, not the global repo', async () => { + const sourceDir = mkdtempSync(join(tmpdir(), 'gbrain-src-')); + // A DIFFERENT global checkout must NOT win for a per-source job. + await engine.setConfig('sync.repo_path', '/some/global/brain/checkout'); + await engine.executeRaw( + `INSERT INTO sources (id, name, local_path, config, archived, created_at) + VALUES ('repo-a', 'Repo A', $1, '{}'::jsonb, false, now())`, + [sourceDir], + ); + + const handlers = await captureHandlers(); + const handler = handlers.get('autopilot-cycle')!; + // DB-only phase keeps the test cheap; brain_dir is stamped from opts.brainDir + // regardless of which phases run, so it still proves the binding. + const result = await handler({ + data: { source_id: 'repo-a', phases: ['resolve_symbol_edges'] }, + signal: undefined, + }); + + expect(result.report.brain_dir).toBe(sourceDir); + expect(result.report.brain_dir).not.toBe('/some/global/brain/checkout'); + }); + + test('source_id with NULL local_path → brainDir is null (FS phases skip), never the global repo', async () => { + // The mixed-scope bug: a pure-DB source must NOT fall through to repoPath. + await engine.setConfig('sync.repo_path', '/some/global/brain/checkout'); + await engine.executeRaw( + `INSERT INTO sources (id, name, local_path, config, archived, created_at) + VALUES ('db-only', 'DB Only', NULL, '{}'::jsonb, false, now())`, + [], + ); + + const handlers = await captureHandlers(); + const handler = handlers.get('autopilot-cycle')!; + const result = await handler({ + data: { source_id: 'db-only', phases: ['resolve_symbol_edges'] }, + signal: undefined, + }); + + expect(result.report.brain_dir).toBeNull(); + expect(result.report.brain_dir).not.toBe('/some/global/brain/checkout'); + }); + + test('legacy (no source_id) keeps the global repoPath — back-compat', async () => { + const globalDir = mkdtempSync(join(tmpdir(), 'gbrain-global-')); + await engine.setConfig('sync.repo_path', globalDir); + + const handlers = await captureHandlers(); + const handler = handlers.get('autopilot-cycle')!; + const result = await handler({ + data: { phases: ['resolve_symbol_edges'] }, + signal: undefined, + }); + + expect(result.report.brain_dir).toBe(globalDir); + }); +});