diff --git a/server/__tests__/signal-processor.test.ts b/server/__tests__/signal-processor.test.ts index b910788e..3f4ddf2a 100644 --- a/server/__tests__/signal-processor.test.ts +++ b/server/__tests__/signal-processor.test.ts @@ -2,8 +2,30 @@ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; import { join } from 'path'; import { mkdirSync, rmSync } from 'fs'; import { tmpdir } from 'os'; +import { promisify } from 'util'; import { TaskStore } from '../task-store.js'; -import { SignalProcessor } from '../signal-processor.js'; +import { SignalProcessor, checkGate } from '../signal-processor.js'; + +// vi.hoisted runs before vi.mock hoisting, so these are available in the factory +const { execFilePromisified } = vi.hoisted(() => ({ + execFilePromisified: vi.fn<() => Promise<{ stdout: string; stderr: string }>>(), +})); + +vi.mock('child_process', async (importOriginal) => { + const actual = await importOriginal(); + const mockFn = Object.assign(vi.fn(), { + [promisify.custom]: execFilePromisified, + }); + return { ...actual, execFile: mockFn }; +}); + +function mockExecResult(stdout: string): void { + execFilePromisified.mockResolvedValue({ stdout, stderr: '' }); +} + +function mockExecError(err: Error): void { + execFilePromisified.mockRejectedValue(err); +} const TEST_DIR = join(tmpdir(), `mitzo-signal-test-${process.pid}`); @@ -380,6 +402,107 @@ describe('SignalProcessor', () => { }); }); + describe('checkGate — centaur_review', () => { + afterEach(() => { + execFilePromisified.mockReset(); + }); + + it('returns pass for LGTM review', async () => { + const reviewBody = '## Centaur Review\n\nLGTM — no issues found.'; + mockExecResult(JSON.stringify({ body: reviewBody, created_at: '2026-07-05T20:00:00Z' })); + + const result = await checkGate({ type: 'centaur_review', repo: 'org/repo', pr: 1 }); + expect(result.resolved).toBe(true); + expect(result.status).toBe('pass'); + }); + + it('returns fail for review with critical findings', async () => { + const reviewBody = + '## Centaur Review\n\nFound **3** issue(s) (1 critical, 2 warning).\n\n- details...'; + mockExecResult(JSON.stringify({ body: reviewBody, created_at: '2026-07-05T20:00:00Z' })); + + const result = await checkGate({ type: 'centaur_review', repo: 'org/repo', pr: 1 }); + expect(result.resolved).toBe(true); + expect(result.status).toBe('fail'); + expect(result.artifacts).toMatchObject({ hasCritical: true, hasWarning: true }); + }); + + it('returns fail for review with warnings only', async () => { + const reviewBody = '## Centaur Review\n\nFound **2** issue(s) (2 warning).\n\n- details...'; + mockExecResult(JSON.stringify({ body: reviewBody, created_at: '2026-07-05T20:00:00Z' })); + + const result = await checkGate({ type: 'centaur_review', repo: 'org/repo', pr: 1 }); + expect(result.resolved).toBe(true); + expect(result.status).toBe('fail'); + expect(result.artifacts).toMatchObject({ hasCritical: false, hasWarning: true }); + }); + + it('returns pass for info/style-only findings', async () => { + const reviewBody = '## Centaur Review\n\nFound **1** issue(s).\n\n- 🔵 style: minor nit'; + mockExecResult(JSON.stringify({ body: reviewBody, created_at: '2026-07-05T20:00:00Z' })); + + const result = await checkGate({ type: 'centaur_review', repo: 'org/repo', pr: 1 }); + expect(result.resolved).toBe(true); + expect(result.status).toBe('pass'); + }); + + it('checks severity before LGTM (prevents false pass)', async () => { + // A review that contains both LGTM text and critical findings + const reviewBody = + '## Centaur Review\n\nFound **1** issue(s) (1 critical).\n\n- LGTM overall but 1 critical issue'; + mockExecResult(JSON.stringify({ body: reviewBody, created_at: '2026-07-05T20:00:00Z' })); + + const result = await checkGate({ type: 'centaur_review', repo: 'org/repo', pr: 1 }); + expect(result.resolved).toBe(true); + expect(result.status).toBe('fail'); + }); + + it('returns not-resolved when no matching comment exists (empty output)', async () => { + mockExecResult(''); + + const result = await checkGate({ type: 'centaur_review', repo: 'org/repo', pr: 1 }); + expect(result.resolved).toBe(false); + }); + + it('returns not-resolved when jq returns null (no matching comment)', async () => { + mockExecResult('null'); + + const result = await checkGate({ type: 'centaur_review', repo: 'org/repo', pr: 1 }); + expect(result.resolved).toBe(false); + }); + + it('returns not-resolved when gh api fails', async () => { + mockExecError(new Error('gh api error')); + + const result = await checkGate({ type: 'centaur_review', repo: 'org/repo', pr: 1 }); + expect(result.resolved).toBe(false); + }); + + it('handles pr_url config format (backward compat)', async () => { + const reviewBody = '## Centaur Review\n\nLGTM — no issues found.'; + mockExecResult(JSON.stringify({ body: reviewBody, created_at: '2026-07-05T20:00:00Z' })); + + const result = await checkGate({ + type: 'centaur_review', + pr_url: 'https://github.com/dimakis/mitzo/pull/360', + } as unknown as Parameters[0]); + expect(result.resolved).toBe(true); + expect(result.status).toBe('pass'); + // Verify the promisified function was called with repo/pr extracted from pr_url + expect(execFilePromisified).toHaveBeenCalled(); + const callStr = JSON.stringify(execFilePromisified.mock.calls[0]); + expect(callStr).toContain('repos/dimakis/mitzo/issues/360/comments'); + }); + + it('returns not-resolved for pr_url that cannot be parsed', async () => { + const result = await checkGate({ + type: 'centaur_review', + pr_url: 'https://example.com/not-a-github-url', + } as unknown as Parameters[0]); + expect(result.resolved).toBe(false); + }); + }); + it('stores failure artifacts in annotations on retry', () => { const goal = store.create({ title: 'Goal' }); const agent = store.create({ diff --git a/server/signal-processor.ts b/server/signal-processor.ts index 0bc6f2e9..dc223f79 100644 --- a/server/signal-processor.ts +++ b/server/signal-processor.ts @@ -186,7 +186,9 @@ export async function checkGate(config: GateConfig): Promise { case 'gh_review': return checkGhReview(config as GateConfig & { repo: string; pr: number | string }); case 'centaur_review': - return checkCentaurReview(config as GateConfig & { pr_url: string }); + return checkCentaurReview( + config as GateConfig & { repo?: string; pr?: number | string; pr_url?: string }, + ); case 'compound': return checkCompound(config as GateConfig & { all: GateConfig[] }); case 'human_approval': @@ -260,23 +262,52 @@ async function checkGhReview(config: { repo: string; pr: number | string }): Pro } } -async function checkCentaurReview(config: { pr_url: string }): Promise { - try { - const res = await fetch( - `http://localhost:8642/api/reviews?pr=${encodeURIComponent(config.pr_url)}`, - ); - if (!res.ok) return { resolved: false, status: 'fail' }; - const data = (await res.json()) as { status?: string; review?: unknown }; - - if (data.status === 'approved') { - return { resolved: true, status: 'pass', artifacts: { review: data.review } }; +async function checkCentaurReview(config: { + repo?: string; + pr?: number | string; + pr_url?: string; +}): Promise { + // Support both repo+pr and legacy pr_url gate config formats + let repo = config.repo; + let pr = config.pr; + if (!repo && !pr && config.pr_url) { + const match = config.pr_url.match(/github\.com\/([^/]+\/[^/]+)\/pull\/(\d+)/); + if (match) { + repo = match[1]; + pr = match[2]; } - if (data.status === 'changes_requested') { - return { resolved: true, status: 'fail', artifacts: { review: data.review } }; + } + if (!repo || !pr) return { resolved: false, status: 'fail' }; + + try { + const { stdout } = await execFileAsync('gh', [ + 'api', + `repos/${repo}/issues/${pr}/comments`, + '--jq', + '[.[] | select(.body | startswith("## Centaur Review")) | {body: .body, created_at: .created_at}] | last', + ]); + if (!stdout.trim() || stdout.trim() === 'null') return { resolved: false, status: 'fail' }; + + const comment = JSON.parse(stdout) as { body: string; created_at: string }; + if (!comment) return { resolved: false, status: 'fail' }; + const body = comment.body; + + // Check severity first — a review with findings takes precedence even if "LGTM" appears + // Regexes match Centaur's summary format: "Found **N** issue(s) (X critical, Y warning)" + const hasCritical = /\d+\s+critical/.test(body); + const hasWarning = /\d+\s+warning/.test(body); + + if (hasCritical || hasWarning) { + return { + resolved: true, + status: 'fail', + artifacts: { review: body, hasCritical, hasWarning }, + }; } - return { resolved: false, status: 'fail' }; + + // LGTM or info/style only — pass + return { resolved: true, status: 'pass', artifacts: { review: body } }; } catch { - // Centaur might not be running — that's fine, just not resolved return { resolved: false, status: 'fail' }; } }