From 787ec7dea5a59536f414ff3fd94ce3bf412f6753 Mon Sep 17 00:00:00 2001 From: Wintermute Date: Wed, 29 Apr 2026 22:56:56 +0000 Subject: [PATCH] fix: autopilot-cycle handler forwards job.data.phases to runCycle MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The autopilot-cycle handler always ran ALL_PHASES regardless of job data. This caused production stalls when the embed phase had a large backlog (17K+ stale chunks) that exceeded the 30-minute job timeout. Every 5-min cycle would start, hit the embed wall, stall, and get force-killed — creating an infinite stall loop that kept the queue perpetually unhealthy. The fix validates job.data.phases against ALL_PHASES (preventing injection) and forwards the selected phases to runCycle(). Callers can now submit fast cycles (lint+backlinks+sync+extract) on a 5-min cron and run embed separately with a longer timeout during off-peak hours. If phases is omitted, not an array, or filters to empty, behavior is unchanged (all phases run). Tests: 4 new cases covering phase restriction, invalid name filtering, empty array fallback, and non-array type safety. --- src/commands/jobs.ts | 9 ++++ test/handlers.test.ts | 102 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 111 insertions(+) diff --git a/src/commands/jobs.ts b/src/commands/jobs.ts index 7e8d97479..07f3746d2 100644 --- a/src/commands/jobs.ts +++ b/src/commands/jobs.ts @@ -948,10 +948,19 @@ export async function registerBuiltinHandlers(worker: MinionWorker, engine: Brai ? job.data.repoPath : (await engine.getConfig('sync.repo_path')) ?? '.'; + // 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'); + const validPhases = new Set(ALL_PHASES); + const requestedPhases = Array.isArray(job.data.phases) + ? (job.data.phases as string[]).filter(p => validPhases.has(p as any)) + : undefined; + const report = await runCycle(engine, { brainDir: repoPath, pull: true, // autopilot daemon opts into git pull signal: job.signal, // propagate abort so cycle bails on timeout/cancel + ...(requestedPhases && requestedPhases.length > 0 ? { phases: requestedPhases as any } : {}), yieldBetweenPhases: async () => { // Yield to the event loop so worker lock-renewal can fire. await new Promise(r => setImmediate(r)); diff --git a/test/handlers.test.ts b/test/handlers.test.ts index cc7eedc59..487e2a3eb 100644 --- a/test/handlers.test.ts +++ b/test/handlers.test.ts @@ -120,3 +120,105 @@ describe('autopilot-cycle handler — partial failure does NOT throw', () => { } }, 30_000); }); + +describe('autopilot-cycle handler — phase passthrough', () => { + test('job.data.phases restricts which phases run', async () => { + 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-phase-pass-')); + try { + 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'); + // Request only lint and sync — embed should NOT appear + const result = await handler({ + data: { repoPath: dir, phases: ['lint', 'sync'] }, + signal: { aborted: false } as any, + job: { id: 10, name: 'autopilot-cycle' } as any, + }); + + expect(result).toBeDefined(); + const report = (result as any).report; + expect(report).toBeDefined(); + const phaseNames = report.phases.map((p: any) => p.phase); + expect(phaseNames).toContain('lint'); + expect(phaseNames).toContain('sync'); + // Phases NOT requested must be absent + expect(phaseNames).not.toContain('embed'); + expect(phaseNames).not.toContain('extract'); + expect(phaseNames).not.toContain('backlinks'); + expect(phaseNames).not.toContain('orphans'); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }, 30_000); + + test('invalid phase names in job.data.phases are filtered out', async () => { + 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-phase-invalid-')); + try { + 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'); + // Mix valid and bogus names — only 'lint' should survive filtering + const result = await handler({ + data: { repoPath: dir, phases: ['lint', 'BOGUS', 'rm -rf /'] }, + signal: { aborted: false } as any, + job: { id: 11, name: 'autopilot-cycle' } as any, + }); + + const report = (result as any).report; + const phaseNames = report.phases.map((p: any) => p.phase); + expect(phaseNames).toContain('lint'); + expect(phaseNames).not.toContain('BOGUS'); + expect(phaseNames.length).toBe(1); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }, 30_000); + + test('empty phases array falls back to all phases (same as no phases)', async () => { + const handler = (worker as any).handlers.get('autopilot-cycle'); + // Empty array should fall through to ALL_PHASES (same as omitting phases) + const result = await handler({ + data: { repoPath: '/definitely-does-not-exist-for-phase-test', phases: [] }, + signal: { aborted: false } as any, + job: { id: 12, name: 'autopilot-cycle' } as any, + }); + + const report = (result as any).report; + // With all phases, filesystem phases fail on missing dir + const phaseNames = report.phases.map((p: any) => p.phase); + expect(phaseNames).toContain('lint'); + expect(phaseNames).toContain('backlinks'); + expect(phaseNames).toContain('sync'); + }, 30_000); + + test('non-array phases value is ignored (falls back to all)', async () => { + const handler = (worker as any).handlers.get('autopilot-cycle'); + // String instead of array — should be ignored + const result = await handler({ + data: { repoPath: '/definitely-does-not-exist-for-phase-test', phases: 'lint' }, + signal: { aborted: false } as any, + job: { id: 13, name: 'autopilot-cycle' } as any, + }); + + const report = (result as any).report; + const phaseNames = report.phases.map((p: any) => p.phase); + // Should have all phases since the string was ignored + expect(phaseNames).toContain('lint'); + expect(phaseNames).toContain('sync'); + expect(phaseNames).toContain('embed'); + }, 30_000); +});