mirror of
https://github.com/garrytan/gbrain.git
synced 2026-07-31 04:07:52 +00:00
feat(jobs): Tier 1 handlers + autopilot-cycle (the killer handler)
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) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.7
parent
73237d175b
commit
53d63b6380
+92
-18
@@ -360,35 +360,51 @@ USAGE
|
||||
}
|
||||
}
|
||||
|
||||
/** Register built-in job handlers from existing CLI commands. */
|
||||
async function registerBuiltinHandlers(worker: MinionWorker, engine: BrainEngine): Promise<void> {
|
||||
/**
|
||||
* 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<void> {
|
||||
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<string, unknown> = {};
|
||||
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 };
|
||||
});
|
||||
}
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
Reference in New Issue
Block a user