mirror of
https://github.com/garrytan/gbrain.git
synced 2026-07-28 14:59:47 +00:00
fix(autopilot): per-source cycle binds FS phases to source.local_path, not global repo (#2194 #2227)
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) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
0b4e27d1a1
commit
2996112053
+21
-4
@@ -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 } : {}),
|
||||
|
||||
@@ -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<Map<string, (job: any) => Promise<any>>> {
|
||||
const handlers = new Map<string, (job: any) => Promise<any>>();
|
||||
const fakeWorker = { register(name: string, fn: (job: any) => Promise<any>) { 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);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user