From 53d63b6380e2df5f006aacbaf40a7c9031dcee3c Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Sat, 18 Apr 2026 13:28:12 +0800 Subject: [PATCH] feat(jobs): Tier 1 handlers + autopilot-cycle (the killer handler) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit registerBuiltinHandlers now handlers every operation autopilot needs to dispatch via Minions + the single autopilot-cycle handler the autopilot loop actually submits each interval. Existing handlers (sync, embed, lint) rewired to call library-level Core functions directly instead of the CLI wrappers. CLI wrappers call process.exit(1) on validation errors; if a worker claimed a badly-formed job, the WORKER PROCESS would die — killing every in-flight job. Cores throw, so one bad job fails one job. New handlers: - extract → runExtractCore (mode: links|timeline|all, dir) - backlinks → runBacklinksCore (action: check|fix, dir) - autopilot-cycle → THE killer handler. Runs sync → extract → embed → backlinks inline. Each step wrapped in try/catch; returns { partial: true, failed_steps: [...] } when any step fails. Does NOT throw on partial failure — that would trigger Minion retry, and an intermittent extract bug would block every future cycle. Replaces the 4-job parent-child DAG proposed in early plan drafts (Codex H3/H4: parent/child is NOT a depends_on primitive in Minions). import.ts handler still uses the CLI wrapper (runImport) — import's one process.exit fires only on a missing dir arg and the handler always passes a dir; Core extraction deferred to v0.12.0 when Tier 2 refactors happen. registerBuiltinHandlers promoted from private to exported for testability. test/handlers.test.ts: 4 tests. Asserts every expected handler name registers. Asserts autopilot-cycle against a nonexistent repo returns { partial: true, failed_steps: ['sync', 'extract', 'backlinks'] } — does NOT throw. Asserts autopilot-cycle against an empty (but real) git repo returns a result with a steps map, never throws. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/commands/jobs.ts | 110 +++++++++++++++++++++++++++++++++++------- test/handlers.test.ts | 109 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 201 insertions(+), 18 deletions(-) create mode 100644 test/handlers.test.ts diff --git a/src/commands/jobs.ts b/src/commands/jobs.ts index 3f6447e73..244b6c485 100644 --- a/src/commands/jobs.ts +++ b/src/commands/jobs.ts @@ -360,35 +360,51 @@ USAGE } } -/** Register built-in job handlers from existing CLI commands. */ -async function registerBuiltinHandlers(worker: MinionWorker, engine: BrainEngine): Promise { +/** + * Register built-in job handlers. + * + * Handlers call library-level Core functions (runSyncCore via performSync, + * runExtractCore, runEmbedCore, runBacklinksCore) directly — NOT the CLI + * wrappers. CLI wrappers call process.exit(1) on validation errors; if a + * worker claimed a badly-formed job and ran one, the WORKER PROCESS would + * die and every in-flight job would go stalled. Library Cores throw + * instead, so one bad job fails one job — not the worker. + * + * Per the v0.11.1 plan (Codex architecture #5 — tension 3). + */ +export async function registerBuiltinHandlers(worker: MinionWorker, engine: BrainEngine): Promise { worker.register('sync', async (job) => { - const { runSync } = await import('./sync.ts'); - const result = await runSync(engine, Object.entries(job.data).flatMap(([k, v]) => [`--${k}`, String(v)])); - return result ?? { synced: true }; + const { performSync } = await import('./sync.ts'); + const repoPath = typeof job.data.repoPath === 'string' ? job.data.repoPath : undefined; + const noPull = !!job.data.noPull; + const noEmbed = job.data.noEmbed !== false; + const result = await performSync(engine, { repoPath, noPull, noEmbed }); + return result; }); worker.register('embed', async (job) => { - const { runEmbed } = await import('./embed.ts'); - const embedArgs: string[] = []; - if (job.data.slug) embedArgs.push(String(job.data.slug)); - else if (job.data.all) embedArgs.push('--all'); - else if (job.data.stale) embedArgs.push('--stale'); - else embedArgs.push('--stale'); - await runEmbed(engine, embedArgs); + const { runEmbedCore } = await import('./embed.ts'); + await runEmbedCore(engine, { + slug: typeof job.data.slug === 'string' ? job.data.slug : undefined, + slugs: Array.isArray(job.data.slugs) ? (job.data.slugs as string[]) : undefined, + all: !!job.data.all, + stale: job.data.all ? false : (job.data.stale !== false), + }); return { embedded: true }; }); worker.register('lint', async (job) => { - const { runLint } = await import('./lint.ts'); - const lintArgs: string[] = []; - if (job.data.dir) lintArgs.push(String(job.data.dir)); - if (job.data.fix) lintArgs.push('--fix'); - await runLint(lintArgs); - return { linted: true }; + const { runLintCore } = await import('./lint.ts'); + const target = typeof job.data.dir === 'string' ? job.data.dir : '.'; + const result = await runLintCore({ target, fix: !!job.data.fix, dryRun: !!job.data.dryRun }); + return result; }); worker.register('import', async (job) => { + // import.ts Core extraction deferred to v0.12.0 (import has parallel + // workers + checkpointing). Keep the CLI wrapper call but note the + // worker-kill risk is bounded: import's only process.exit fires on + // a missing dir arg, which this handler always passes. const { runImport } = await import('./import.ts'); const importArgs: string[] = []; if (job.data.dir) importArgs.push(String(job.data.dir)); @@ -396,4 +412,62 @@ async function registerBuiltinHandlers(worker: MinionWorker, engine: BrainEngine await runImport(engine, importArgs); return { imported: true }; }); + + worker.register('extract', async (job) => { + const { runExtractCore } = await import('./extract.ts'); + const mode = (typeof job.data.mode === 'string' && ['links', 'timeline', 'all'].includes(job.data.mode)) + ? (job.data.mode as 'links' | 'timeline' | 'all') + : 'all'; + const dir = typeof job.data.dir === 'string' + ? job.data.dir + : (await engine.getConfig('sync.repo_path')) ?? '.'; + return await runExtractCore(engine, { mode, dir, dryRun: !!job.data.dryRun }); + }); + + worker.register('backlinks', async (job) => { + const { runBacklinksCore } = await import('./backlinks.ts'); + const action: 'check' | 'fix' = job.data.action === 'check' ? 'check' : 'fix'; + const dir = typeof job.data.dir === 'string' + ? job.data.dir + : (await engine.getConfig('sync.repo_path')) ?? '.'; + return await runBacklinksCore({ action, dir, dryRun: !!job.data.dryRun }); + }); + + // The killer handler. Autopilot submits ONE `autopilot-cycle` per cycle + // (idempotency_key on cycle slot) instead of a 4-job parent-child DAG, + // because Minions' parent/child is NOT a depends_on primitive (Codex + // H3/H4). Each step is wrapped in its own try/catch; the handler returns + // `{ partial: true, failed_steps: [...] }` when any step fails. It does + // NOT throw on partial failure — that would cause the Minion to retry, + // and an intermittent extract bug would block every future cycle. + worker.register('autopilot-cycle', async (job) => { + const { performSync } = await import('./sync.ts'); + const { runExtractCore } = await import('./extract.ts'); + const { runEmbedCore } = await import('./embed.ts'); + const { runBacklinksCore } = await import('./backlinks.ts'); + + const repoPath = typeof job.data.repoPath === 'string' + ? job.data.repoPath + : (await engine.getConfig('sync.repo_path')) ?? '.'; + + const steps: Record = {}; + const failed: string[] = []; + + try { steps.sync = await performSync(engine, { repoPath, noEmbed: true }); } + catch (e) { steps.sync = { error: e instanceof Error ? e.message : String(e) }; failed.push('sync'); } + + try { steps.extract = await runExtractCore(engine, { mode: 'all', dir: repoPath }); } + catch (e) { steps.extract = { error: e instanceof Error ? e.message : String(e) }; failed.push('extract'); } + + try { await runEmbedCore(engine, { stale: true }); steps.embed = { embedded: true }; } + catch (e) { steps.embed = { error: e instanceof Error ? e.message : String(e) }; failed.push('embed'); } + + try { steps.backlinks = await runBacklinksCore({ action: 'fix', dir: repoPath }); } + catch (e) { steps.backlinks = { error: e instanceof Error ? e.message : String(e) }; failed.push('backlinks'); } + + if (failed.length > 0) { + return { partial: true, failed_steps: failed, steps }; + } + return { partial: false, steps }; + }); } diff --git a/test/handlers.test.ts b/test/handlers.test.ts new file mode 100644 index 000000000..e0d7e0a28 --- /dev/null +++ b/test/handlers.test.ts @@ -0,0 +1,109 @@ +/** + * Tests for registerBuiltinHandlers in src/commands/jobs.ts. + * + * Covers: + * - Every expected handler name is registered. + * - autopilot-cycle handler returns { partial: true, failed_steps: [...] } + * when any step throws — does NOT throw itself (critical for preventing + * intermittent extract bugs from blocking every future cycle via retry). + */ + +import { describe, test, expect, beforeAll, afterAll, mock } from 'bun:test'; +import { PGLiteEngine } from '../src/core/pglite-engine.ts'; +import { MinionWorker } from '../src/core/minions/worker.ts'; +import { registerBuiltinHandlers } from '../src/commands/jobs.ts'; + +let engine: PGLiteEngine; +let worker: MinionWorker; + +beforeAll(async () => { + engine = new PGLiteEngine(); + await engine.connect({}); + await engine.initSchema(); + worker = new MinionWorker(engine, { queue: 'test' }); + await registerBuiltinHandlers(worker, engine); +}); + +afterAll(async () => { + await engine.disconnect(); +}); + +describe('registerBuiltinHandlers', () => { + test('registers all built-in handler names', () => { + const names = worker.registeredNames; + // Existing handlers from pre-v0.11.1 + expect(names).toContain('sync'); + expect(names).toContain('embed'); + expect(names).toContain('lint'); + expect(names).toContain('import'); + // New in v0.11.1 (Tier 1 + autopilot-cycle) + expect(names).toContain('extract'); + expect(names).toContain('backlinks'); + expect(names).toContain('autopilot-cycle'); + }); + + test('total handler count includes all 7 names', () => { + expect(worker.registeredNames.length).toBeGreaterThanOrEqual(7); + }); +}); + +describe('autopilot-cycle handler — partial failure does NOT throw', () => { + test('step failure returns partial:true + failed_steps, no throw', async () => { + // Call the handler directly with a context that points at a nonexistent + // repo. Every step will fail (sync throws on missing .git, extract + // throws on missing dir, embed tries to list pages which is fine against + // the test engine, backlinks throws on missing dir). The handler should + // STILL return successfully — never throw. + // + // This is the critical invariant: an intermittent bug in one step must + // not cause the Minion to retry + block every future cycle. + const handler = (worker as any).handlers.get('autopilot-cycle'); + expect(handler).toBeDefined(); + + const result = await handler({ + data: { repoPath: '/definitely-does-not-exist-for-autopilot-test' }, + signal: { aborted: false } as any, + job: { id: 1, name: 'autopilot-cycle' } as any, + }); + + expect(result).toBeDefined(); + expect((result as any).partial).toBe(true); + expect(Array.isArray((result as any).failed_steps)).toBe(true); + // sync + extract + backlinks all fail on missing repo (embed operates + // on the DB directly and doesn't touch the repo path, so it doesn't fail). + expect((result as any).failed_steps).toContain('sync'); + expect((result as any).failed_steps).toContain('extract'); + expect((result as any).failed_steps).toContain('backlinks'); + }); + + test('all steps succeed → partial:false', async () => { + // Smoke: invoke against a real (if empty) brain dir. If every step + // completes, partial is false. + const fs = await import('fs'); + const { execSync } = await import('child_process'); + const { tmpdir } = await import('os'); + const { join } = await import('path'); + const dir = fs.mkdtempSync(join(tmpdir(), 'gbrain-autopilot-cycle-')); + try { + // Initialize as a git repo so sync doesn't fail on .git lookup. + execSync('git init', { cwd: dir, stdio: 'pipe' }); + execSync('git config user.email test@example.com', { cwd: dir, stdio: 'pipe' }); + execSync('git config user.name Test', { cwd: dir, stdio: 'pipe' }); + execSync('git commit --allow-empty -m init', { cwd: dir, stdio: 'pipe' }); + + const handler = (worker as any).handlers.get('autopilot-cycle'); + const result = await handler({ + data: { repoPath: dir }, + signal: { aborted: false } as any, + job: { id: 2, name: 'autopilot-cycle' } as any, + }); + // Empty repo: some steps may still fail (backlinks needs .md files) + // but the handler MUST return a result object, never throw. + expect(result).toBeDefined(); + expect(typeof (result as any).partial).toBe('boolean'); + expect('steps' in (result as any)).toBe(true); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }, 30_000); +});