diff --git a/runners/agent-runner/src/index.ts b/runners/agent-runner/src/index.ts index b6b8287..ebfc342 100644 --- a/runners/agent-runner/src/index.ts +++ b/runners/agent-runner/src/index.ts @@ -54,6 +54,7 @@ interface ContainerOutput { output?: { visibility: 'public' | 'silent'; text?: string; + verdict?: 'done' | 'done_with_concerns' | 'blocked' | 'silent'; }; newSessionId?: string; error?: string; @@ -217,14 +218,27 @@ function normalizeStructuredOutput(result: string | null): { const trimmed = result.trim(); try { const parsed = JSON.parse(trimmed) as { - ejclaw?: { visibility?: unknown; text?: unknown }; + ejclaw?: { visibility?: unknown; text?: unknown; verdict?: unknown }; }; const envelope = parsed?.ejclaw; if (envelope && typeof envelope === 'object' && !Array.isArray(envelope)) { if (envelope.visibility === 'silent') { + if ( + envelope.verdict !== undefined && + envelope.verdict !== 'silent' + ) { + return { + result, + output: { visibility: 'public', text: result }, + }; + } return { result: null, - output: { visibility: 'silent' }, + output: { + visibility: 'silent', + verdict: + envelope.verdict === 'silent' ? ('silent' as const) : undefined, + }, }; } if ( @@ -232,9 +246,30 @@ function normalizeStructuredOutput(result: string | null): { typeof envelope.text === 'string' && envelope.text.length > 0 ) { + if ( + envelope.verdict !== undefined && + envelope.verdict !== 'done' && + envelope.verdict !== 'done_with_concerns' && + envelope.verdict !== 'blocked' + ) { + return { + result, + output: { visibility: 'public', text: result }, + }; + } return { result: envelope.text, - output: { visibility: 'public', text: envelope.text }, + output: { + visibility: 'public', + text: envelope.text, + verdict: + typeof envelope.verdict === 'string' + ? (envelope.verdict as + | 'done' + | 'done_with_concerns' + | 'blocked') + : undefined, + }, }; } } diff --git a/runners/codex-runner/src/index.ts b/runners/codex-runner/src/index.ts index 4fa3e5d..e9d2a5b 100644 --- a/runners/codex-runner/src/index.ts +++ b/runners/codex-runner/src/index.ts @@ -48,6 +48,7 @@ interface ContainerOutput { output?: { visibility: 'public' | 'silent'; text?: string; + verdict?: 'done' | 'done_with_concerns' | 'blocked' | 'silent'; }; phase?: 'progress' | 'final'; newSessionId?: string; @@ -92,14 +93,27 @@ function normalizeStructuredOutput(result: string | null): { const trimmed = result.trim(); try { const parsed = JSON.parse(trimmed) as { - ejclaw?: { visibility?: unknown; text?: unknown }; + ejclaw?: { visibility?: unknown; text?: unknown; verdict?: unknown }; }; const envelope = parsed?.ejclaw; if (envelope && typeof envelope === 'object' && !Array.isArray(envelope)) { if (envelope.visibility === 'silent') { + if ( + envelope.verdict !== undefined && + envelope.verdict !== 'silent' + ) { + return { + result, + output: { visibility: 'public', text: result }, + }; + } return { result: null, - output: { visibility: 'silent' }, + output: { + visibility: 'silent', + verdict: + envelope.verdict === 'silent' ? ('silent' as const) : undefined, + }, }; } if ( @@ -107,9 +121,30 @@ function normalizeStructuredOutput(result: string | null): { typeof envelope.text === 'string' && envelope.text.length > 0 ) { + if ( + envelope.verdict !== undefined && + envelope.verdict !== 'done' && + envelope.verdict !== 'done_with_concerns' && + envelope.verdict !== 'blocked' + ) { + return { + result, + output: { visibility: 'public', text: result }, + }; + } return { result: envelope.text, - output: { visibility: 'public', text: envelope.text }, + output: { + visibility: 'public', + text: envelope.text, + verdict: + typeof envelope.verdict === 'string' + ? (envelope.verdict as + | 'done' + | 'done_with_concerns' + | 'blocked') + : undefined, + }, }; } } diff --git a/src/agent-output.ts b/src/agent-output.ts index 5be30b4..3a40820 100644 --- a/src/agent-output.ts +++ b/src/agent-output.ts @@ -1,3 +1,4 @@ +import { parseStructuredOutputEnvelope } from './output-suppression.js'; import type { StructuredAgentOutput } from './types.js'; export function stringifyLegacyAgentResult( @@ -13,15 +14,29 @@ export function stringifyLegacyAgentResult( } } +export function getStructuredAgentOutput(output: { + output?: StructuredAgentOutput; + result?: string | object | null; +}): StructuredAgentOutput | null { + if (output.output) { + return output.output; + } + if (typeof output.result !== 'string') { + return null; + } + return parseStructuredOutputEnvelope(output.result.trim()); +} + export function getAgentOutputText(output: { output?: StructuredAgentOutput; result?: string | object | null; }): string | null { - if (output.output?.visibility === 'silent') { + const structured = getStructuredAgentOutput(output); + if (structured?.visibility === 'silent') { return null; } - if (output.output?.visibility === 'public') { - return output.output.text; + if (structured?.visibility === 'public') { + return structured.text; } return stringifyLegacyAgentResult(output.result); } @@ -38,6 +53,7 @@ export function hasAgentOutputPayload(output: { export function isSilentAgentOutput(output: { output?: StructuredAgentOutput; + result?: string | object | null; }): boolean { - return output.output?.visibility === 'silent'; + return getStructuredAgentOutput(output)?.visibility === 'silent'; } diff --git a/src/db.test.ts b/src/db.test.ts index 0b13912..6e525dd 100644 --- a/src/db.test.ts +++ b/src/db.test.ts @@ -635,6 +635,12 @@ describe('paired task state', () => { expect(getPairedTaskById('paired-task-1')?.plan_status).toBe( 'not_requested', ); + expect(getPairedTaskById('paired-task-1')?.gate_turn_kind ?? null).toBe( + null, + ); + expect(getPairedTaskById('paired-task-1')?.reviewer_verdict ?? null).toBe( + null, + ); expect(getPairedExecutionById('paired-exec-1')?.status).toBe('pending'); expect(getPairedWorkspace('paired-task-1', 'owner')?.workspace_dir).toBe( '/tmp/paired-room/owner', @@ -960,6 +966,10 @@ describe('paired task state', () => { task_policy: 'autonomous', risk_level: 'low', plan_status: 'approved', + gate_turn_kind: 'implementation_start', + reviewer_verdict: 'done_with_concerns', + reviewer_verdict_at: '2026-03-28T00:09:00.000Z', + reviewer_verdict_note: '**DONE_WITH_CONCERNS** okay to proceed', status: 'review_pending', review_requested_at: '2026-03-28T00:10:00.000Z', updated_at: '2026-03-28T00:10:00.000Z', @@ -969,6 +979,13 @@ describe('paired task state', () => { started_at: '2026-03-28T00:11:00.000Z', }); + expect(getPairedTaskById('paired-task-2')?.gate_turn_kind).toBe( + 'implementation_start', + ); + expect(getPairedTaskById('paired-task-2')?.reviewer_verdict).toBe( + 'done_with_concerns', + ); + upsertPairedWorkspace({ id: 'paired-task-2:reviewer', task_id: 'paired-task-2', diff --git a/src/db.ts b/src/db.ts index 9e9cdcb..5d3d4d6 100644 --- a/src/db.ts +++ b/src/db.ts @@ -258,6 +258,10 @@ function createSchema(database: Database.Database): void { risk_level TEXT NOT NULL DEFAULT 'low', plan_status TEXT NOT NULL DEFAULT 'not_requested', review_requested_at TEXT, + gate_turn_kind TEXT, + reviewer_verdict TEXT, + reviewer_verdict_at TEXT, + reviewer_verdict_note TEXT, status TEXT NOT NULL DEFAULT 'active', created_at TEXT NOT NULL, updated_at TEXT NOT NULL, @@ -271,6 +275,14 @@ function createSchema(database: Database.Database): void { 'changes_requested' ) ), + CHECK ( + gate_turn_kind IS NULL OR + gate_turn_kind IN ('implementation_start', 'commit', 'push') + ), + CHECK ( + reviewer_verdict IS NULL OR + reviewer_verdict IN ('done', 'done_with_concerns', 'blocked', 'silent') + ), CHECK ( status IN ( 'active', @@ -690,7 +702,9 @@ function createSchema(database: Database.Database): void { !pairedTasksSql.includes("'plan_review_pending'") || !pairedTasksSql.includes('task_policy TEXT') || !pairedTasksSql.includes('risk_level TEXT') || - !pairedTasksSql.includes('plan_status TEXT')); + !pairedTasksSql.includes('plan_status TEXT') || + !pairedTasksSql.includes('gate_turn_kind TEXT') || + !pairedTasksSql.includes('reviewer_verdict TEXT')); if (pairedTasksNeedsMigration) { database.exec(` CREATE TABLE paired_tasks_new ( @@ -705,6 +719,10 @@ function createSchema(database: Database.Database): void { risk_level TEXT NOT NULL DEFAULT 'low', plan_status TEXT NOT NULL DEFAULT 'not_requested', review_requested_at TEXT, + gate_turn_kind TEXT, + reviewer_verdict TEXT, + reviewer_verdict_at TEXT, + reviewer_verdict_note TEXT, status TEXT NOT NULL DEFAULT 'active', created_at TEXT NOT NULL, updated_at TEXT NOT NULL, @@ -718,6 +736,14 @@ function createSchema(database: Database.Database): void { 'changes_requested' ) ), + CHECK ( + gate_turn_kind IS NULL OR + gate_turn_kind IN ('implementation_start', 'commit', 'push') + ), + CHECK ( + reviewer_verdict IS NULL OR + reviewer_verdict IN ('done', 'done_with_concerns', 'blocked', 'silent') + ), CHECK ( status IN ( 'active', @@ -748,6 +774,10 @@ function createSchema(database: Database.Database): void { risk_level, plan_status, review_requested_at, + gate_turn_kind, + reviewer_verdict, + reviewer_verdict_at, + reviewer_verdict_note, status, created_at, updated_at @@ -774,6 +804,10 @@ function createSchema(database: Database.Database): void { ELSE 'not_requested' END, review_requested_at, + NULL, + NULL, + NULL, + NULL, status, created_at, updated_at @@ -2042,11 +2076,15 @@ export function createPairedTask(task: PairedTask): void { risk_level, plan_status, review_requested_at, + gate_turn_kind, + reviewer_verdict, + reviewer_verdict_at, + reviewer_verdict_note, status, created_at, updated_at ) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) `, ).run( task.id, @@ -2060,6 +2098,10 @@ export function createPairedTask(task: PairedTask): void { task.risk_level, task.plan_status, task.review_requested_at, + task.gate_turn_kind ?? null, + task.reviewer_verdict ?? null, + task.reviewer_verdict_at ?? null, + task.reviewer_verdict_note ?? null, task.status, task.created_at, task.updated_at, @@ -2100,6 +2142,10 @@ export function updatePairedTask( | 'risk_level' | 'plan_status' | 'review_requested_at' + | 'gate_turn_kind' + | 'reviewer_verdict' + | 'reviewer_verdict_at' + | 'reviewer_verdict_note' | 'status' | 'updated_at' > @@ -2132,6 +2178,22 @@ export function updatePairedTask( fields.push('review_requested_at = ?'); values.push(updates.review_requested_at); } + if (updates.gate_turn_kind !== undefined) { + fields.push('gate_turn_kind = ?'); + values.push(updates.gate_turn_kind); + } + if (updates.reviewer_verdict !== undefined) { + fields.push('reviewer_verdict = ?'); + values.push(updates.reviewer_verdict); + } + if (updates.reviewer_verdict_at !== undefined) { + fields.push('reviewer_verdict_at = ?'); + values.push(updates.reviewer_verdict_at); + } + if (updates.reviewer_verdict_note !== undefined) { + fields.push('reviewer_verdict_note = ?'); + values.push(updates.reviewer_verdict_note); + } if (updates.status !== undefined) { fields.push('status = ?'); values.push(updates.status); diff --git a/src/message-agent-executor.test.ts b/src/message-agent-executor.test.ts index 19d47c8..76851c3 100644 --- a/src/message-agent-executor.test.ts +++ b/src/message-agent-executor.test.ts @@ -115,6 +115,7 @@ vi.mock('./paired-execution-context.js', () => ({ })); import * as agentRunner from './agent-runner.js'; +import type { AgentOutput } from './agent-runner.js'; import * as codexTokenRotation from './codex-token-rotation.js'; import * as db from './db.js'; import { buildRoomMemoryBriefing } from './memento-client.js'; @@ -303,6 +304,344 @@ describe('runAgentForGroup room memory', () => { ); }); + it('requires a visible structured verdict in reviewer gate turns', async () => { + const group = { ...makeGroup(), folder: 'test-group', workDir: '/repo' }; + vi.mocked(serviceRouting.getEffectiveChannelLease).mockReturnValue({ + chat_jid: 'group@test', + owner_service_id: 'codex-main', + reviewer_service_id: 'claude', + activated_at: null, + reason: null, + explicit: false, + }); + vi.mocked( + pairedExecutionContext.preparePairedExecutionContext, + ).mockReturnValue({ + task: { + id: 'paired-task-gate', + chat_jid: 'group@test', + group_folder: 'test-group', + owner_service_id: 'codex-main', + reviewer_service_id: 'claude', + title: null, + source_ref: 'HEAD', + task_policy: 'autonomous', + risk_level: 'low', + plan_status: 'not_requested', + review_requested_at: null, + gate_turn_kind: 'implementation_start', + reviewer_verdict: null, + reviewer_verdict_at: null, + reviewer_verdict_note: null, + status: 'active', + created_at: '2026-03-29T00:00:00.000Z', + updated_at: '2026-03-29T00:00:00.000Z', + }, + execution: { + id: 'run-gate:claude', + task_id: 'paired-task-gate', + service_id: 'claude', + role: 'reviewer', + workspace_id: null, + status: 'running', + summary: null, + created_at: '2026-03-29T00:00:00.000Z', + started_at: '2026-03-29T00:00:00.000Z', + completed_at: null, + }, + workspace: null, + envOverrides: {}, + gateTurnKind: 'implementation_start', + requiresVisibleVerdict: true, + }); + + await runAgentForGroup(makeDeps(), { + group, + prompt: 'hello', + chatJid: 'group@test', + runId: 'run-review-gate-prompt', + }); + + expect(agentRunner.runAgentProcess).toHaveBeenCalledWith( + group, + expect.objectContaining({ + prompt: expect.stringContaining( + 'This turn is a paired-room gate turn for implementation_start. Silent output is forbidden.', + ), + }), + expect.any(Function), + undefined, + {}, + ); + }); + + it('blocks silent reviewer output on gate turns and records a silent verdict', async () => { + const group = { ...makeGroup(), folder: 'test-group', workDir: '/repo' }; + vi.mocked(serviceRouting.getEffectiveChannelLease).mockReturnValue({ + chat_jid: 'group@test', + owner_service_id: 'codex-main', + reviewer_service_id: 'claude', + activated_at: null, + reason: null, + explicit: false, + }); + vi.mocked( + pairedExecutionContext.preparePairedExecutionContext, + ).mockReturnValue({ + task: { + id: 'paired-task-gate', + chat_jid: 'group@test', + group_folder: 'test-group', + owner_service_id: 'codex-main', + reviewer_service_id: 'claude', + title: null, + source_ref: 'HEAD', + task_policy: 'autonomous', + risk_level: 'low', + plan_status: 'not_requested', + review_requested_at: null, + gate_turn_kind: 'implementation_start', + reviewer_verdict: null, + reviewer_verdict_at: null, + reviewer_verdict_note: null, + status: 'active', + created_at: '2026-03-29T00:00:00.000Z', + updated_at: '2026-03-29T00:00:00.000Z', + }, + execution: { + id: 'run-gate:claude', + task_id: 'paired-task-gate', + service_id: 'claude', + role: 'reviewer', + workspace_id: null, + status: 'running', + summary: null, + created_at: '2026-03-29T00:00:00.000Z', + started_at: '2026-03-29T00:00:00.000Z', + completed_at: null, + }, + workspace: null, + envOverrides: {}, + gateTurnKind: 'implementation_start', + requiresVisibleVerdict: true, + }); + vi.mocked(agentRunner.runAgentProcess).mockImplementation( + async (_group, _input, _onProcess, onOutput) => { + await onOutput?.({ + status: 'success', + result: null, + output: { visibility: 'silent', verdict: 'silent' }, + phase: 'final', + }); + return { + status: 'success', + result: null, + }; + }, + ); + const outputs: AgentOutput[] = []; + + const result = await runAgentForGroup(makeDeps(), { + group, + prompt: 'hello', + chatJid: 'group@test', + runId: 'run-review-gate-silent', + onOutput: async (output) => { + outputs.push(output); + }, + }); + + expect(result).toBe('error'); + expect(outputs.at(-1)?.output).toEqual({ + visibility: 'public', + text: expect.stringContaining( + 'Reviewer gate turn requires a visible structured verdict.', + ), + }); + expect( + pairedExecutionContext.completePairedExecutionContext, + ).toHaveBeenCalledWith( + expect.objectContaining({ + executionId: 'run-gate:claude', + status: 'failed', + reviewerVerdict: 'silent', + }), + ); + }); + + it('allows structured reviewer verdicts on gate turns', async () => { + const group = { ...makeGroup(), folder: 'test-group', workDir: '/repo' }; + vi.mocked(serviceRouting.getEffectiveChannelLease).mockReturnValue({ + chat_jid: 'group@test', + owner_service_id: 'codex-main', + reviewer_service_id: 'claude', + activated_at: null, + reason: null, + explicit: false, + }); + vi.mocked( + pairedExecutionContext.preparePairedExecutionContext, + ).mockReturnValue({ + task: { + id: 'paired-task-gate', + chat_jid: 'group@test', + group_folder: 'test-group', + owner_service_id: 'codex-main', + reviewer_service_id: 'claude', + title: null, + source_ref: 'HEAD', + task_policy: 'autonomous', + risk_level: 'low', + plan_status: 'not_requested', + review_requested_at: null, + gate_turn_kind: 'implementation_start', + reviewer_verdict: null, + reviewer_verdict_at: null, + reviewer_verdict_note: null, + status: 'active', + created_at: '2026-03-29T00:00:00.000Z', + updated_at: '2026-03-29T00:00:00.000Z', + }, + execution: { + id: 'run-gate:claude', + task_id: 'paired-task-gate', + service_id: 'claude', + role: 'reviewer', + workspace_id: null, + status: 'running', + summary: null, + created_at: '2026-03-29T00:00:00.000Z', + started_at: '2026-03-29T00:00:00.000Z', + completed_at: null, + }, + workspace: null, + envOverrides: {}, + gateTurnKind: 'implementation_start', + requiresVisibleVerdict: true, + }); + vi.mocked(agentRunner.runAgentProcess).mockImplementation( + async (_group, _input, _onProcess, onOutput) => { + await onOutput?.({ + status: 'success', + result: null, + output: { + visibility: 'public', + verdict: 'done_with_concerns', + text: '**DONE_WITH_CONCERNS**\n문제는 없지만 확인 포인트는 있습니다.', + }, + phase: 'final', + }); + return { + status: 'success', + result: null, + }; + }, + ); + + const result = await runAgentForGroup(makeDeps(), { + group, + prompt: 'hello', + chatJid: 'group@test', + runId: 'run-review-gate-done', + onOutput: async () => {}, + }); + + expect(result).toBe('success'); + expect( + pairedExecutionContext.completePairedExecutionContext, + ).toHaveBeenCalledWith( + expect.objectContaining({ + executionId: 'run-gate:claude', + status: 'succeeded', + reviewerVerdict: 'done_with_concerns', + }), + ); + }); + + it('still allows silent reviewer outputs on non-gate turns', async () => { + const group = { ...makeGroup(), folder: 'test-group', workDir: '/repo' }; + vi.mocked(serviceRouting.getEffectiveChannelLease).mockReturnValue({ + chat_jid: 'group@test', + owner_service_id: 'codex-main', + reviewer_service_id: 'claude', + activated_at: null, + reason: null, + explicit: false, + }); + vi.mocked( + pairedExecutionContext.preparePairedExecutionContext, + ).mockReturnValue({ + task: { + id: 'paired-task-gate', + chat_jid: 'group@test', + group_folder: 'test-group', + owner_service_id: 'codex-main', + reviewer_service_id: 'claude', + title: null, + source_ref: 'HEAD', + task_policy: 'autonomous', + risk_level: 'low', + plan_status: 'not_requested', + review_requested_at: null, + gate_turn_kind: null, + reviewer_verdict: null, + reviewer_verdict_at: null, + reviewer_verdict_note: null, + status: 'active', + created_at: '2026-03-29T00:00:00.000Z', + updated_at: '2026-03-29T00:00:00.000Z', + }, + execution: { + id: 'run-gate:claude', + task_id: 'paired-task-gate', + service_id: 'claude', + role: 'reviewer', + workspace_id: null, + status: 'running', + summary: null, + created_at: '2026-03-29T00:00:00.000Z', + started_at: '2026-03-29T00:00:00.000Z', + completed_at: null, + }, + workspace: null, + envOverrides: {}, + gateTurnKind: null, + requiresVisibleVerdict: false, + }); + vi.mocked(agentRunner.runAgentProcess).mockImplementation( + async (_group, _input, _onProcess, onOutput) => { + await onOutput?.({ + status: 'success', + result: null, + output: { visibility: 'silent', verdict: 'silent' }, + phase: 'final', + }); + return { + status: 'success', + result: null, + }; + }, + ); + const outputs: AgentOutput[] = []; + + const result = await runAgentForGroup(makeDeps(), { + group, + prompt: 'hello', + chatJid: 'group@test', + runId: 'run-review-non-gate-silent', + onOutput: async (output) => { + outputs.push(output); + }, + }); + + expect(result).toBe('success'); + expect(outputs).toEqual([ + expect.objectContaining({ + output: { visibility: 'silent', verdict: 'silent' }, + }), + ]); + }); + it('passes paired workspace env overrides into the runner when execution metadata exists', async () => { const group = { ...makeGroup(), @@ -384,11 +723,13 @@ describe('runAgentForGroup room memory', () => { ); expect( pairedExecutionContext.completePairedExecutionContext, - ).toHaveBeenCalledWith({ - executionId: 'run-room-role:claude', - status: 'succeeded', - summary: 'ok', - }); + ).toHaveBeenCalledWith( + expect.objectContaining({ + executionId: 'run-room-role:claude', + status: 'succeeded', + summary: 'ok', + }), + ); }); it('blocks reviewer execution when an in-review snapshot became stale and does not spawn the runner', async () => { diff --git a/src/message-agent-executor.ts b/src/message-agent-executor.ts index db8b605..4a91b8b 100644 --- a/src/message-agent-executor.ts +++ b/src/message-agent-executor.ts @@ -1,6 +1,6 @@ import { getErrorMessage } from './utils.js'; -import { getAgentOutputText } from './agent-output.js'; +import { getAgentOutputText, getStructuredAgentOutput } from './agent-output.js'; import { AgentOutput, runAgentProcess, @@ -51,7 +51,7 @@ import { } from './codex-token-rotation.js'; import type { CodexRotationReason } from './agent-error-detection.js'; import { getTokenCount } from './token-rotation.js'; -import type { RegisteredGroup } from './types.js'; +import type { PairedReviewerVerdict, RegisteredGroup } from './types.js'; // ── Main executor ───────────────────────────────────────────────── @@ -138,9 +138,26 @@ export async function runAgentForGroup( }); const effectivePrompt = buildStructuredOutputPrompt(prompt, { reviewerMode, + gateTurnKind: pairedExecutionContext?.gateTurnKind, + requiresVisibleVerdict: pairedExecutionContext?.requiresVisibleVerdict, }); let pairedExecutionStatus: 'succeeded' | 'failed' = 'failed'; let pairedExecutionSummary: string | null = null; + let pairedReviewerVerdict: PairedReviewerVerdict | null = null; + let pairedReviewerVerdictNote: string | null = null; + let reviewerGateValidationFailed = false; + + const formatReviewerGateFailureMessage = (): string => { + const taskId = pairedExecutionContext?.task.id ?? 'unknown-task'; + const gateTurnKind = + pairedExecutionContext?.gateTurnKind ?? 'implementation_start'; + return [ + 'Reviewer gate turn requires a visible structured verdict.', + `- Task: ${taskId}`, + `- Gate: ${gateTurnKind}`, + '- Allowed verdicts: done, done_with_concerns, blocked', + ].join('\n'); + }; const shouldHandoffToCodex = ( reason: AgentTriggerReason, @@ -369,6 +386,41 @@ export async function runAgentForGroup( sawVisibleOutput: true, }; } + if ( + pairedExecutionContext?.requiresVisibleVerdict && + (output.phase === undefined || output.phase === 'final') + ) { + const structuredOutput = getStructuredAgentOutput(output); + const reviewerVerdict = structuredOutput?.verdict ?? null; + if ( + structuredOutput?.visibility === 'silent' || + reviewerVerdict === null + ) { + reviewerGateValidationFailed = true; + pairedReviewerVerdict = + structuredOutput?.visibility === 'silent' ? 'silent' : null; + pairedReviewerVerdictNote = null; + const failureMessage = formatReviewerGateFailureMessage(); + pairedExecutionSummary = failureMessage.slice(0, 500); + await onOutput({ + status: 'success', + result: null, + output: { + visibility: 'public', + text: failureMessage, + }, + phase: 'final', + }); + return; + } + if (structuredOutput) { + pairedReviewerVerdict = reviewerVerdict; + pairedReviewerVerdictNote = + structuredOutput.visibility === 'public' + ? structuredOutput.text + : null; + } + } await onOutput(output); } : undefined; @@ -863,6 +915,26 @@ export async function runAgentForGroup( return 'error'; } + if ( + pairedExecutionContext?.requiresVisibleVerdict && + (reviewerGateValidationFailed || pairedReviewerVerdict === null) + ) { + if (!reviewerGateValidationFailed && onOutput) { + const failureMessage = formatReviewerGateFailureMessage(); + pairedExecutionSummary = failureMessage.slice(0, 500); + await onOutput({ + status: 'success', + result: null, + output: { + visibility: 'public', + text: failureMessage, + }, + phase: 'final', + }); + } + return 'error'; + } + pairedExecutionStatus = 'succeeded'; return 'success'; } finally { @@ -871,6 +943,8 @@ export async function runAgentForGroup( executionId: pairedExecutionContext.execution.id, status: pairedExecutionStatus, summary: pairedExecutionSummary, + reviewerVerdict: pairedReviewerVerdict, + reviewerVerdictNote: pairedReviewerVerdictNote, }); } } diff --git a/src/output-suppression.test.ts b/src/output-suppression.test.ts index 6ae63fc..076c7f4 100644 --- a/src/output-suppression.test.ts +++ b/src/output-suppression.test.ts @@ -85,6 +85,18 @@ describe('parseStructuredOutputEnvelope', () => { ), ).toEqual({ visibility: 'public', text: 'hello' }); }); + + it('parses a public envelope with a reviewer verdict', () => { + expect( + parseStructuredOutputEnvelope( + '{"ejclaw":{"visibility":"public","verdict":"done_with_concerns","text":"**DONE_WITH_CONCERNS**"}}', + ), + ).toEqual({ + visibility: 'public', + verdict: 'done_with_concerns', + text: '**DONE_WITH_CONCERNS**', + }); + }); }); describe('buildStructuredOutputPrompt', () => { @@ -104,4 +116,25 @@ describe('buildStructuredOutputPrompt', () => { 'If you have not already emitted any visible progress, status update, or partial answer in this turn and you are only agreeing, mirroring, or restating without adding a concrete correction, risk, missing prerequisite, test gap, or code change, output only the JSON object.', ); }); + + it('requires a visible structured verdict on reviewer gate turns', () => { + expect( + buildStructuredOutputPrompt('hello', { + reviewerMode: true, + gateTurnKind: 'implementation_start', + requiresVisibleVerdict: true, + }), + ).toContain( + 'This turn is a paired-room gate turn for implementation_start. Silent output is forbidden.', + ); + expect( + buildStructuredOutputPrompt('hello', { + reviewerMode: true, + gateTurnKind: 'implementation_start', + requiresVisibleVerdict: true, + }), + ).toContain( + 'Allowed verdict values are: "done", "done_with_concerns", "blocked".', + ); + }); }); diff --git a/src/output-suppression.ts b/src/output-suppression.ts index b24e0d5..11542ed 100644 --- a/src/output-suppression.ts +++ b/src/output-suppression.ts @@ -5,7 +5,7 @@ import { CODEX_REVIEW_SERVICE_ID, normalizeServiceId, } from './config.js'; -import type { StructuredAgentOutput } from './types.js'; +import type { PairedGateTurnKind, StructuredAgentOutput } from './types.js'; const ANY_SUPPRESS_TOKEN_PATTERN = /__EJ_SUPPRESS_[a-f0-9]{24,}(?:__)?/g; const EXACT_ANY_SUPPRESS_TOKEN_PATTERN = /^__EJ_SUPPRESS_[a-f0-9]{24,}(?:__)?$/; @@ -13,6 +13,12 @@ const STRUCTURED_SILENT_OUTPUT_PREFIX_PATTERN = /^\s*\{\s*"ejclaw"\s*:\s*\{\s*"visibility"\s*:\s*"silent"/; export const STRUCTURED_SILENT_OUTPUT_ENVELOPE = '{"ejclaw":{"visibility":"silent"}}'; +const STRUCTURED_PUBLIC_VERDICTS = new Set([ + 'done', + 'done_with_concerns', + 'blocked', +]); +const STRUCTURED_SILENT_VERDICTS = new Set(['silent']); export function createSuppressToken(): string { return `__EJ_SUPPRESS_${randomBytes(12).toString('hex')}__`; @@ -58,21 +64,46 @@ export function parseStructuredOutputEnvelope( ): StructuredAgentOutput | null { try { const parsed = JSON.parse(rawText) as { - ejclaw?: { visibility?: unknown; text?: unknown }; + ejclaw?: { visibility?: unknown; text?: unknown; verdict?: unknown }; }; const envelope = parsed?.ejclaw; if (!envelope || typeof envelope !== 'object' || Array.isArray(envelope)) { return null; } if (envelope.visibility === 'silent') { - return { visibility: 'silent' }; + if ( + envelope.verdict !== undefined && + !STRUCTURED_SILENT_VERDICTS.has(String(envelope.verdict)) + ) { + return null; + } + return { + visibility: 'silent', + verdict: + envelope.verdict === 'silent' + ? ('silent' as const) + : undefined, + }; } if ( envelope.visibility === 'public' && typeof envelope.text === 'string' && envelope.text.length > 0 ) { - return { visibility: 'public', text: envelope.text }; + if ( + envelope.verdict !== undefined && + !STRUCTURED_PUBLIC_VERDICTS.has(String(envelope.verdict)) + ) { + return null; + } + return { + visibility: 'public', + text: envelope.text, + verdict: + typeof envelope.verdict === 'string' + ? (envelope.verdict as 'done' | 'done_with_concerns' | 'blocked') + : undefined, + }; } } catch { return null; @@ -85,6 +116,8 @@ export function buildStructuredOutputPrompt( prompt: string, options?: { reviewerMode?: boolean; + gateTurnKind?: PairedGateTurnKind | null; + requiresVisibleVerdict?: boolean; }, ): string { const lines = [ @@ -101,5 +134,17 @@ export function buildStructuredOutputPrompt( ); } + if (options?.reviewerMode && options.requiresVisibleVerdict) { + lines.push( + `This turn is a paired-room gate turn for ${options.gateTurnKind ?? 'implementation_start'}. Silent output is forbidden.`, + ); + lines.push( + 'Your final answer must be a structured public JSON envelope with a reviewer verdict: {"ejclaw":{"visibility":"public","verdict":"done","text":"**DONE** ..."}}', + ); + lines.push( + 'Allowed verdict values are: "done", "done_with_concerns", "blocked".', + ); + } + return `${lines.join('\n')}\n\n${prompt}`; } diff --git a/src/paired-execution-context.test.ts b/src/paired-execution-context.test.ts index 216979a..e275d8b 100644 --- a/src/paired-execution-context.test.ts +++ b/src/paired-execution-context.test.ts @@ -248,6 +248,79 @@ describe('paired execution context', () => { }); }); + it('blocks owner execution when a gate turn lacks a visible reviewer verdict', () => { + vi.mocked(db.getLatestOpenPairedTaskForChat).mockReturnValue({ + id: 'task-1', + chat_jid: 'dc:test', + group_folder: group.folder, + owner_service_id: 'codex-main', + reviewer_service_id: 'codex-review', + title: null, + source_ref: 'HEAD', + task_policy: 'autonomous', + risk_level: 'low', + plan_status: 'not_requested', + review_requested_at: null, + gate_turn_kind: 'implementation_start', + reviewer_verdict: null, + reviewer_verdict_at: null, + reviewer_verdict_note: null, + status: 'active', + created_at: '2026-03-28T00:00:00.000Z', + updated_at: '2026-03-28T00:00:00.000Z', + }); + + const result = preparePairedExecutionContext({ + group, + chatJid: 'dc:test', + runId: 'run-gate-block', + roomRoleContext: ownerContext, + }); + + expect(result?.blockMessage).toContain( + 'A visible reviewer verdict is required before the owner can proceed with this gate.', + ); + expect(result?.blockMessage).toContain('- Gate: implementation_start'); + expect( + pairedWorkspaceManager.provisionOwnerWorkspaceForPairedTask, + ).not.toHaveBeenCalled(); + }); + + it('allows owner execution when the current gate has an approved reviewer verdict', () => { + vi.mocked(db.getLatestOpenPairedTaskForChat).mockReturnValue({ + id: 'task-1', + chat_jid: 'dc:test', + group_folder: group.folder, + owner_service_id: 'codex-main', + reviewer_service_id: 'codex-review', + title: null, + source_ref: 'HEAD', + task_policy: 'autonomous', + risk_level: 'low', + plan_status: 'not_requested', + review_requested_at: null, + gate_turn_kind: 'implementation_start', + reviewer_verdict: 'done_with_concerns', + reviewer_verdict_at: '2026-03-28T00:01:00.000Z', + reviewer_verdict_note: '**DONE_WITH_CONCERNS** gate okay', + status: 'active', + created_at: '2026-03-28T00:00:00.000Z', + updated_at: '2026-03-28T00:01:00.000Z', + }); + + const result = preparePairedExecutionContext({ + group, + chatJid: 'dc:test', + runId: 'run-gate-allowed', + roomRoleContext: ownerContext, + }); + + expect(result?.blockMessage).toBeUndefined(); + expect( + pairedWorkspaceManager.provisionOwnerWorkspaceForPairedTask, + ).toHaveBeenCalledWith('task-1'); + }); + it('does not self-cancel when the same execution resumes', () => { vi.mocked(db.getLatestOpenPairedTaskForChat).mockReturnValue({ id: 'task-1', @@ -294,6 +367,57 @@ describe('paired execution context', () => { }); }); + it('records the latest structured reviewer verdict on a fresh gate turn completion', () => { + vi.mocked(db.getPairedExecutionById).mockReturnValue({ + id: 'run-review:codex-review', + task_id: 'task-1', + service_id: 'codex-review', + role: 'reviewer', + workspace_id: 'task-1:reviewer', + checkpoint_fingerprint: null, + status: 'running', + summary: null, + created_at: '2026-03-28T00:00:00.000Z', + started_at: '2026-03-28T00:00:00.000Z', + completed_at: null, + }); + vi.mocked(db.getPairedTaskById).mockReturnValue({ + id: 'task-1', + chat_jid: 'dc:test', + group_folder: group.folder, + owner_service_id: 'codex-main', + reviewer_service_id: 'codex-review', + title: null, + source_ref: 'HEAD', + task_policy: 'autonomous', + risk_level: 'low', + plan_status: 'not_requested', + review_requested_at: null, + gate_turn_kind: 'implementation_start', + reviewer_verdict: null, + reviewer_verdict_at: null, + reviewer_verdict_note: null, + status: 'active', + created_at: '2026-03-28T00:00:00.000Z', + updated_at: '2026-03-28T00:00:00.000Z', + }); + + completePairedExecutionContext({ + executionId: 'run-review:codex-review', + status: 'succeeded', + reviewerVerdict: 'done_with_concerns', + reviewerVerdictNote: '**DONE_WITH_CONCERNS** gate okay', + }); + + expect(db.updatePairedTask).toHaveBeenCalledWith( + 'task-1', + expect.objectContaining({ + reviewer_verdict: 'done_with_concerns', + reviewer_verdict_note: '**DONE_WITH_CONCERNS** gate okay', + }), + ); + }); + it('plans reviewer recovery from the latest review checkpoint', () => { vi.mocked(db.getLatestOpenPairedTaskForChat).mockReturnValue({ id: 'task-1', diff --git a/src/paired-execution-context.ts b/src/paired-execution-context.ts index 46a0ef7..b41a23c 100644 --- a/src/paired-execution-context.ts +++ b/src/paired-execution-context.ts @@ -32,12 +32,51 @@ import type { PairedArtifactType, PairedEventType, PairedExecution, + PairedGateTurnKind, + PairedReviewerVerdict, PairedTask, PairedWorkspace, RegisteredGroup, RoomRoleContext, } from './types.js'; +const APPROVED_REVIEWER_GATE_VERDICTS = new Set([ + 'done', + 'done_with_concerns', +]); +const VISIBLE_REVIEWER_GATE_VERDICTS = new Set([ + 'done', + 'done_with_concerns', + 'blocked', +]); +const REVIEWER_GATE_REQUIRED_MESSAGE = + 'A visible reviewer verdict is required before the owner can proceed with this gate.'; + +function getGateTurnKind(task: PairedTask): PairedGateTurnKind | null { + return task.gate_turn_kind ?? null; +} + +function hasVisibleReviewerGateVerdict( + verdict: PairedReviewerVerdict | null | undefined, +): boolean { + return verdict ? VISIBLE_REVIEWER_GATE_VERDICTS.has(verdict) : false; +} + +function allowsOwnerToProceedForGate( + verdict: PairedReviewerVerdict | null | undefined, +): boolean { + return verdict ? APPROVED_REVIEWER_GATE_VERDICTS.has(verdict) : false; +} + +function formatOwnerGateBlockedMessage(task: PairedTask): string { + return [ + REVIEWER_GATE_REQUIRED_MESSAGE, + `- Task: ${task.id}`, + `- Gate: ${task.gate_turn_kind ?? 'implementation_start'}`, + `- Reviewer verdict: ${task.reviewer_verdict ?? 'missing'}`, + ].join('\n'); +} + function resolveCanonicalSourceRef(workDir: string): string { try { const head = execFileSync('git', ['rev-parse', 'HEAD'], { @@ -99,6 +138,10 @@ function ensureActiveTask( risk_level: 'low', plan_status: 'not_requested', review_requested_at: null, + gate_turn_kind: null, + reviewer_verdict: null, + reviewer_verdict_at: null, + reviewer_verdict_note: null, status: 'active', created_at: now, updated_at: now, @@ -258,6 +301,8 @@ export interface PreparedPairedExecutionContext { execution: PairedExecution; workspace: PairedWorkspace | null; envOverrides: Record; + gateTurnKind?: PairedGateTurnKind | null; + requiresVisibleVerdict?: boolean; blockMessage?: string; } @@ -285,19 +330,31 @@ export function preparePairedExecutionContext(args: { return undefined; } + const latestTask = getPairedTaskById(task.id) ?? task; + const gateTurnKind = getGateTurnKind(latestTask); + const requiresVisibleVerdict = + roomRoleContext.role === 'reviewer' && + !!gateTurnKind && + !hasVisibleReviewerGateVerdict(latestTask.reviewer_verdict); let workspace: PairedWorkspace | null = null; let blockMessage: string | undefined; const now = new Date().toISOString(); - if (roomRoleContext.role === 'owner') { - workspace = provisionOwnerWorkspaceForPairedTask(task.id); + if ( + roomRoleContext.role === 'owner' && + gateTurnKind && + !allowsOwnerToProceedForGate(latestTask.reviewer_verdict) + ) { + blockMessage = formatOwnerGateBlockedMessage(latestTask); + } else if (roomRoleContext.role === 'owner') { + workspace = provisionOwnerWorkspaceForPairedTask(latestTask.id); } else { - const reviewerWorkspace = prepareReviewerWorkspaceForExecution(task); + const reviewerWorkspace = prepareReviewerWorkspaceForExecution(latestTask); workspace = reviewerWorkspace.workspace; blockMessage = reviewerWorkspace.blockMessage; - const latestTask = getPairedTaskById(task.id) ?? task; - if (workspace && latestTask.status === 'review_ready') { - updatePairedTask(task.id, { + const refreshedTask = getPairedTaskById(latestTask.id) ?? latestTask; + if (workspace && refreshedTask.status === 'review_ready') { + updatePairedTask(latestTask.id, { status: 'in_review', updated_at: now, }); @@ -307,16 +364,16 @@ export function preparePairedExecutionContext(args: { const execution = ensureExecutionRecord({ runId, roomRoleContext, - task, + task: latestTask, workspace: workspace ?? undefined, checkpointFingerprint: resolveExecutionCheckpointFingerprint({ - taskId: task.id, + taskId: latestTask.id, role: roomRoleContext.role, workspace, }), }); const envOverrides: Record = { - EJCLAW_PAIRED_TASK_ID: task.id, + EJCLAW_PAIRED_TASK_ID: latestTask.id, EJCLAW_PAIRED_EXECUTION_ID: execution.id, EJCLAW_PAIRED_ROLE: roomRoleContext.role, }; @@ -326,13 +383,18 @@ export function preparePairedExecutionContext(args: { } if (roomRoleContext.role === 'reviewer') { envOverrides.EJCLAW_REVIEWER_RUNTIME = '1'; + if (requiresVisibleVerdict && gateTurnKind) { + envOverrides.EJCLAW_PAIRED_GATE_TURN_KIND = gateTurnKind; + } } return { - task: getPairedTaskById(task.id) ?? task, + task: getPairedTaskById(latestTask.id) ?? latestTask, execution, workspace, envOverrides, + gateTurnKind, + requiresVisibleVerdict, blockMessage, }; } @@ -728,6 +790,8 @@ export function completePairedExecutionContext(args: { executionId: string; status: 'succeeded' | 'failed'; summary?: string | null; + reviewerVerdict?: PairedReviewerVerdict | null; + reviewerVerdictNote?: string | null; }): void { const execution = getPairedExecutionById(args.executionId); if (!execution) { @@ -764,6 +828,21 @@ export function completePairedExecutionContext(args: { return; } + const task = getPairedTaskById(completedExecution.task_id); + if ( + task && + completedExecution.role === 'reviewer' && + task.gate_turn_kind && + args.reviewerVerdict + ) { + updatePairedTask(task.id, { + reviewer_verdict: args.reviewerVerdict, + reviewer_verdict_at: completedAt, + reviewer_verdict_note: args.reviewerVerdictNote ?? null, + updated_at: completedAt, + }); + } + maybeAutoRequestReviewForCompletedExecution({ execution: completedExecution, status: args.status, diff --git a/src/types.ts b/src/types.ts index 1870462..653fcfc 100644 --- a/src/types.ts +++ b/src/types.ts @@ -50,6 +50,14 @@ export type PairedPlanStatus = | 'approved' | 'changes_requested'; +export type PairedGateTurnKind = 'implementation_start' | 'commit' | 'push'; + +export type PairedReviewerVerdict = + | 'done' + | 'done_with_concerns' + | 'blocked' + | 'silent'; + export type PairedExecutionStatus = | 'pending' | 'running' @@ -114,6 +122,10 @@ export interface PairedTask { risk_level: PairedRiskLevel; plan_status: PairedPlanStatus; review_requested_at: string | null; + gate_turn_kind?: PairedGateTurnKind | null; + reviewer_verdict?: PairedReviewerVerdict | null; + reviewer_verdict_at?: string | null; + reviewer_verdict_note?: string | null; status: PairedTaskStatus; created_at: string; updated_at: string; @@ -184,9 +196,11 @@ export type StructuredAgentOutput = | { visibility: 'public'; text: string; + verdict?: Exclude; } | { visibility: 'silent'; + verdict?: 'silent'; }; export function normalizeAgentOutputPhase(