From abc8bbbe1fb1fd0d4654341599b0c18250b5557e Mon Sep 17 00:00:00 2001 From: ejclaw Date: Wed, 8 Apr 2026 06:16:30 +0900 Subject: [PATCH 1/7] =?UTF-8?q?docs:=20clean=20up=20README=20=E2=80=94=20r?= =?UTF-8?q?emove=20.env=20duplication,=20move=20acknowledgments=20to=20bot?= =?UTF-8?q?tom?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- README.md | 52 ++++++---------------------------------------------- 1 file changed, 6 insertions(+), 46 deletions(-) diff --git a/README.md b/README.md index 0c54364..4cedc4c 100644 --- a/README.md +++ b/README.md @@ -8,10 +8,6 @@ Tribunal multi-agent AI assistant (Owner + Reviewer + Arbiter) over Discord with autonomous paired review and Mixture of Agents. -Originally derived from [qwibitai/nanoclaw](https://github.com/qwibitai/nanoclaw), now independently maintained as EJClaw. -Prompt design inspired by [Q00/ouroboros](https://github.com/Q00/ouroboros) and [garrytan/gstack](https://github.com/garrytan/gstack), adapted for EJClaw's Discord and dual-service workflow. -Tribunal arbiter system inspired by multi-agent consensus architectures. - ## Overview A single unified service (`ejclaw`) runs a **Tribunal** of three roles while @@ -159,47 +155,6 @@ bun run build - [Architecture](docs/architecture.md) — Data flow, room model, verification execution, key files - [Configuration](docs/configuration.md) — Full `.env` reference, debugging paths -### Environment - -All configuration in a single `.env` file: - -```bash -# Discord bots (canonical role-fixed names) -DISCORD_OWNER_BOT_TOKEN= # Owner bot -DISCORD_REVIEWER_BOT_TOKEN= # Reviewer bot -DISCORD_ARBITER_BOT_TOKEN= # Arbiter bot - -# Old service-based token names are no longer accepted. -# Rename existing values to the canonical role-based keys above. - -# Agent types -OWNER_AGENT_TYPE=codex # codex | claude-code -REVIEWER_AGENT_TYPE=claude-code # claude-code | codex -ARBITER_AGENT_TYPE=codex # codex | claude-code (optional, enables 3rd agent) - -# Per-role model overrides -OWNER_MODEL=gpt-5.4 -REVIEWER_MODEL=claude-opus-4-6 -ARBITER_MODEL=gpt-5.4 - -# API keys -CLAUDE_CODE_OAUTH_TOKEN= # Claude Code OAuth token -CLAUDE_CODE_OAUTH_TOKENS= # Comma-separated for multi-account rotation -GROQ_API_KEY= # Voice transcription (Groq Whisper) - -# Mixture of Agents (MoA) -MOA_ENABLED=true -MOA_REF_MODELS=kimi,glm -MOA_KIMI_MODEL=kimi-k2.5 -MOA_KIMI_BASE_URL=https://api.kimi.com/coding -MOA_KIMI_API_KEY=sk-kimi-xxx -MOA_KIMI_API_FORMAT=anthropic -MOA_GLM_MODEL=glm-5.1 -MOA_GLM_BASE_URL=https://open.bigmodel.cn/api/anthropic -MOA_GLM_API_KEY=xxx -MOA_GLM_API_FORMAT=anthropic -``` - ### Deploy ```bash @@ -215,6 +170,11 @@ bun run dev # Dev mode with hot reload bun test # Run tests ``` +## Acknowledgments + +- Originally derived from [qwibitai/nanoclaw](https://github.com/qwibitai/nanoclaw) +- Prompt design inspired by [Q00/ouroboros](https://github.com/Q00/ouroboros) and [garrytan/gstack](https://github.com/garrytan/gstack) + ## License -MIT — Originally derived from [qwibitai/nanoclaw](https://github.com/qwibitai/nanoclaw) +MIT From 126ca85bdf09846a1f8dd2a6d20495809b476588 Mon Sep 17 00:00:00 2001 From: ejclaw Date: Fri, 10 Apr 2026 14:06:41 +0900 Subject: [PATCH 2/7] Repair startup FK recovery and paired turn runtime fixes --- src/db.test.ts | 59 ++++++++++++++++++++++++ src/db/bootstrap.ts | 21 +++++++++ src/message-agent-executor.test.ts | 62 +++++++++++++++++++++++++ src/message-agent-executor.ts | 10 ++-- src/message-runtime-handoffs.test.ts | 69 +++++++++++++--------------- src/message-runtime-handoffs.ts | 5 +- src/message-runtime-rules.test.ts | 7 ++- src/message-runtime-rules.ts | 18 +++++--- src/paired-execution-context.test.ts | 1 + src/paired-execution-context.ts | 3 ++ 10 files changed, 203 insertions(+), 52 deletions(-) diff --git a/src/db.test.ts b/src/db.test.ts index f0793b4..85942c7 100644 --- a/src/db.test.ts +++ b/src/db.test.ts @@ -698,6 +698,65 @@ describe('task CRUD', () => { expect(getTaskById('task-3')).toBeUndefined(); }); + it('prunes orphan task_run_logs rows during database initialization', () => { + const tempDir = fs.mkdtempSync('/tmp/ejclaw-task-log-fk-repair-'); + const dbPath = path.join(tempDir, 'messages.db'); + const legacyDb = new Database(dbPath); + + try { + initializeDatabaseSchema(legacyDb); + legacyDb + .prepare( + `INSERT INTO scheduled_tasks ( + id, group_folder, chat_jid, prompt, schedule_type, schedule_value, created_at + ) VALUES (?, ?, ?, ?, ?, ?, ?)`, + ) + .run( + 'task-legacy-orphan', + 'infra-auto', + 'dc:legacy', + 'legacy task', + 'once', + '2026-03-20T19:00:00.000Z', + '2026-03-20T18:59:00.000Z', + ); + legacyDb + .prepare( + `INSERT INTO task_run_logs (task_id, run_at, duration_ms, status, result, error) + VALUES (?, ?, ?, ?, ?, ?)`, + ) + .run( + 'task-legacy-orphan', + '2026-03-20T19:00:58.882Z', + 0, + 'error', + null, + 'Group not found: infra-auto', + ); + legacyDb.exec('PRAGMA foreign_keys = OFF'); + legacyDb + .prepare('DELETE FROM scheduled_tasks WHERE id = ?') + .run('task-legacy-orphan'); + } finally { + legacyDb.close(); + } + + expect(() => _initTestDatabaseFromFile(dbPath)).not.toThrow(); + + const migratedDb = new Database(dbPath, { readonly: true }); + try { + const remainingLogs = migratedDb + .prepare(`SELECT COUNT(*) AS count FROM task_run_logs WHERE task_id = ?`) + .get('task-legacy-orphan') as { count: number }; + const fkViolations = migratedDb.prepare('PRAGMA foreign_key_check').all(); + + expect(remainingLogs.count).toBe(0); + expect(fkViolations).toHaveLength(0); + } finally { + migratedDb.close(); + } + }); + it('deletes task-scoped IPC and session directories when removing a task', () => { const taskId = 'task-cleanup'; const groupFolder = 'cleanup-group'; diff --git a/src/db/bootstrap.ts b/src/db/bootstrap.ts index d836f7e..f4f8c17 100644 --- a/src/db/bootstrap.ts +++ b/src/db/bootstrap.ts @@ -3,6 +3,7 @@ import fs from 'fs'; import path from 'path'; import { ASSISTANT_NAME, STORE_DIR } from '../config.js'; +import { logger } from '../logger.js'; import { applyBaseSchema } from './base-schema.js'; import { applySchemaMigrations } from './schema.js'; @@ -10,6 +11,19 @@ function setForeignKeys(database: Database, enabled: boolean): void { database.exec(`PRAGMA foreign_keys = ${enabled ? 'ON' : 'OFF'}`); } +function pruneOrphanTaskRunLogs(database: Database): number { + const result = database + .prepare( + `DELETE FROM task_run_logs + WHERE NOT EXISTS ( + SELECT 1 FROM scheduled_tasks + WHERE scheduled_tasks.id = task_run_logs.task_id + )`, + ) + .run(); + return result.changes; +} + function assertNoForeignKeyViolations(database: Database): void { const violations = database .prepare('PRAGMA foreign_key_check') @@ -35,8 +49,15 @@ export function initializeDatabaseSchema(database: Database): void { applySchemaMigrations(database, { assistantName: ASSISTANT_NAME, }); + const prunedTaskRunLogs = pruneOrphanTaskRunLogs(database); setForeignKeys(database, true); assertNoForeignKeyViolations(database); + if (prunedTaskRunLogs > 0) { + logger.warn( + { count: prunedTaskRunLogs }, + 'Pruned orphan task_run_logs rows during database initialization', + ); + } } export function openPersistentDatabase(): Database { diff --git a/src/message-agent-executor.test.ts b/src/message-agent-executor.test.ts index 6a1f9c7..0bd37bb 100644 --- a/src/message-agent-executor.test.ts +++ b/src/message-agent-executor.test.ts @@ -743,6 +743,68 @@ describe('runAgentForGroup room memory', () => { expect(agentRunner.runAgentProcess).not.toHaveBeenCalled(); }); + it('allows a claimed paired turn revision when preparation advances the task before execution', async () => { + const group = { ...makeGroup(), folder: 'test-group' }; + + vi.mocked( + pairedExecutionContext.preparePairedExecutionContext, + ).mockReturnValue({ + task: { + id: 'paired-task-prep-advance', + chat_jid: 'group@test', + group_folder: 'test-group', + owner_service_id: 'claude', + reviewer_service_id: 'codex-review', + title: null, + source_ref: 'HEAD', + plan_notes: null, + round_trip_count: 0, + review_requested_at: '2026-04-10T00:00:00.000Z', + status: 'in_review', + arbiter_verdict: null, + arbiter_requested_at: null, + completion_reason: null, + created_at: '2026-04-10T00:00:00.000Z', + updated_at: '2026-04-10T01:00:00.000Z', + }, + claimedTaskUpdatedAt: '2026-04-10T00:00:00.000Z', + workspace: null, + envOverrides: {}, + }); + + await expect( + runAgentForGroup(makeDeps(), { + group, + prompt: 'hello', + chatJid: 'group@test', + runId: 'run-claimed-paired-turn-revision', + forcedRole: 'reviewer', + pairedTurnIdentity: { + turnId: + 'paired-task-prep-advance:2026-04-10T00:00:00.000Z:reviewer-turn', + taskId: 'paired-task-prep-advance', + taskUpdatedAt: '2026-04-10T00:00:00.000Z', + intentKind: 'reviewer-turn', + role: 'reviewer', + }, + }), + ).resolves.toBe('success'); + + expect(agentRunner.runAgentProcess).toHaveBeenCalledWith( + group, + expect.any(Object), + expect.any(Function), + expect.any(Function), + expect.objectContaining({ + EJCLAW_PAIRED_TURN_ID: + 'paired-task-prep-advance:2026-04-10T00:00:00.000Z:reviewer-turn', + EJCLAW_PAIRED_TURN_ROLE: 'reviewer', + EJCLAW_PAIRED_TURN_INTENT: 'reviewer-turn', + EJCLAW_PAIRED_TASK_UPDATED_AT: '2026-04-10T00:00:00.000Z', + }), + ); + }); + it('fails closed when a persisted paired turn revision mismatches the latest paired task fallback', async () => { const group = { ...makeGroup(), folder: 'test-group' }; diff --git a/src/message-agent-executor.ts b/src/message-agent-executor.ts index 0b924c3..4b935a2 100644 --- a/src/message-agent-executor.ts +++ b/src/message-agent-executor.ts @@ -195,12 +195,17 @@ export async function runAgentForGroup( roomRoleContext, hasHumanMessage: args.hasHumanMessage, }); + const preparedTurnTaskUpdatedAt = pairedExecutionContext + ? (pairedExecutionContext.claimedTaskUpdatedAt ?? + pairedExecutionContext.task.updated_at) + : undefined; const runtimePairedTurnIdentity = args.pairedTurnIdentity ?? (pairedExecutionContext ? resolveRuntimePairedTurnIdentity({ taskId: pairedExecutionContext.task.id, - taskUpdatedAt: pairedExecutionContext.task.updated_at, + taskUpdatedAt: + preparedTurnTaskUpdatedAt ?? pairedExecutionContext.task.updated_at, role: activeRole, taskStatus: pairedExecutionContext.task.status, hasHumanMessage: args.hasHumanMessage, @@ -230,8 +235,7 @@ export async function runAgentForGroup( } if ( pairedExecutionContext && - runtimePairedTurnIdentity.taskUpdatedAt !== - pairedExecutionContext.task.updated_at + runtimePairedTurnIdentity.taskUpdatedAt !== preparedTurnTaskUpdatedAt ) { throw new Error( `Paired turn ${runtimePairedTurnIdentity.turnId} task_updated_at does not match the prepared execution context`, diff --git a/src/message-runtime-handoffs.test.ts b/src/message-runtime-handoffs.test.ts index fa28129..855aeac 100644 --- a/src/message-runtime-handoffs.test.ts +++ b/src/message-runtime-handoffs.test.ts @@ -1,16 +1,12 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; -vi.mock('./config.js', () => ({ - SERVICE_SESSION_SCOPE: 'codex-main', -})); - vi.mock('./db.js', () => ({ claimServiceHandoff: vi.fn(() => true), claimPairedTurnReservation: vi.fn(() => true), completeServiceHandoffAndAdvanceTargetCursor: vi.fn(() => null), failServiceHandoff: vi.fn(), + getAllPendingServiceHandoffs: vi.fn(() => []), getPairedTurnOutputs: vi.fn(() => []), - getPendingServiceHandoffs: vi.fn(() => []), reservePairedTurnReservation: vi.fn(() => true), _clearPairedTurnReservationsForTests: vi.fn(), })); @@ -57,35 +53,30 @@ describe('message-runtime-handoffs', () => { vi.clearAllMocks(); }); - it('does not expose reviewer-targeted handoffs to the codex-main poller', () => { - vi.mocked(db.getPendingServiceHandoffs).mockImplementation( - (targetServiceId?: string) => - targetServiceId === 'codex-main' - ? [] - : ([ - { - id: 1, - chat_jid: 'group@test', - group_folder: 'test-group', - source_service_id: 'claude', - target_service_id: 'codex-review', - source_role: 'reviewer', - source_agent_type: 'claude-code', - target_role: 'reviewer', - target_agent_type: 'codex', - prompt: 'review retry', - status: 'pending', - start_seq: 1, - end_seq: 2, - reason: 'reviewer-auth-failure', - intended_role: 'reviewer', - created_at: '2026-04-10T00:00:00.000Z', - claimed_at: null, - completed_at: null, - last_error: null, - }, - ] as any), - ); + it('exposes reviewer-targeted handoffs to the unified runtime poller', () => { + vi.mocked(db.getAllPendingServiceHandoffs).mockReturnValue([ + { + id: 1, + chat_jid: 'group@test', + group_folder: 'test-group', + source_service_id: 'claude', + target_service_id: 'codex-review', + source_role: 'reviewer', + source_agent_type: 'claude-code', + target_role: 'reviewer', + target_agent_type: 'codex', + prompt: 'review retry', + status: 'pending', + start_seq: 1, + end_seq: 2, + reason: 'reviewer-auth-failure', + intended_role: 'reviewer', + created_at: '2026-04-10T00:00:00.000Z', + claimed_at: null, + completed_at: null, + last_error: null, + }, + ] as any); const enqueueTask = vi.fn(); @@ -94,9 +85,13 @@ describe('message-runtime-handoffs', () => { processClaimedHandoff: vi.fn(), }); - expect(db.getPendingServiceHandoffs).toHaveBeenCalledWith('codex-main'); - expect(db.claimServiceHandoff).not.toHaveBeenCalled(); - expect(enqueueTask).not.toHaveBeenCalled(); + expect(db.getAllPendingServiceHandoffs).toHaveBeenCalled(); + expect(db.claimServiceHandoff).toHaveBeenCalledWith(1); + expect(enqueueTask).toHaveBeenCalledWith( + 'group@test', + 'handoff:1', + expect.any(Function), + ); }); it('fails a claimed handoff closed when its intended role cannot be resolved', async () => { diff --git a/src/message-runtime-handoffs.ts b/src/message-runtime-handoffs.ts index 990dea5..2a3cb07 100644 --- a/src/message-runtime-handoffs.ts +++ b/src/message-runtime-handoffs.ts @@ -1,10 +1,9 @@ -import { SERVICE_SESSION_SCOPE } from './config.js'; import { getErrorMessage } from './utils.js'; import { claimServiceHandoff, completeServiceHandoffAndAdvanceTargetCursor, failServiceHandoff, - getPendingServiceHandoffs, + getAllPendingServiceHandoffs, type ServiceHandoff, } from './db.js'; import { findChannel, findChannelByName } from './router.js'; @@ -29,7 +28,7 @@ export function enqueuePendingHandoffs(args: { ) => void; processClaimedHandoff: (handoff: ServiceHandoff) => Promise; }): void { - for (const handoff of getPendingServiceHandoffs(SERVICE_SESSION_SCOPE)) { + for (const handoff of getAllPendingServiceHandoffs()) { if (!claimServiceHandoff(handoff.id)) { continue; } diff --git a/src/message-runtime-rules.test.ts b/src/message-runtime-rules.test.ts index f31c406..5e64534 100644 --- a/src/message-runtime-rules.test.ts +++ b/src/message-runtime-rules.test.ts @@ -23,6 +23,7 @@ import { resolveLeaseServiceId, type EffectiveChannelLease, } from './service-routing.js'; +import { resolveRoleServiceShadow } from './role-service-shadow.js'; const baseLease: EffectiveChannelLease = { chat_jid: 'chat-1', @@ -354,7 +355,7 @@ describe('message-runtime-rules', () => { ); }); - it('applies forced agent type overrides without changing the configured role target', () => { + it('applies forced agent type overrides and switches to the forced role service shadow', () => { const resolution = resolveExecutionTarget({ lease: baseLease, pairedTaskStatus: 'review_ready', @@ -368,7 +369,9 @@ describe('message-runtime-rules', () => { configuredAgentType: 'claude-code', effectiveAgentType: 'codex', }); - expect(resolution.effectiveServiceId).toBe(resolution.reviewerServiceId); + expect(resolution.effectiveServiceId).toBe( + resolveRoleServiceShadow('reviewer', 'codex'), + ); }); it('always gives arbiter a dedicated session folder', () => { diff --git a/src/message-runtime-rules.ts b/src/message-runtime-rules.ts index 3e152da..96b9af0 100644 --- a/src/message-runtime-rules.ts +++ b/src/message-runtime-rules.ts @@ -9,6 +9,7 @@ import { resolveLeaseServiceId, type EffectiveChannelLease, } from './service-routing.js'; +import { resolveRoleServiceShadow } from './role-service-shadow.js'; import { resolveAgentTypeForRole, resolveRoleAgentPlan, @@ -307,13 +308,6 @@ export function resolveExecutionTarget(args: { (args.forcedRole === 'arbiter' && args.lease.arbiter_agent_type), ); const activeRole = canHonorForcedRole ? args.forcedRole! : inferredRole; - const effectiveServiceId = resolveLeaseServiceId(args.lease, activeRole); - if (!effectiveServiceId) { - throw new Error(`Missing runtime service id for ${activeRole} lease`); - } - - const reviewerServiceId = resolveLeaseServiceId(args.lease, 'reviewer'); - const arbiterServiceId = resolveLeaseServiceId(args.lease, 'arbiter'); const roleAgentPlan = resolveConfiguredRoleAgentPlan( args.lease.reviewer_agent_type != null, args.groupAgentType, @@ -323,6 +317,16 @@ export function resolveExecutionTarget(args: { args.groupAgentType, ); const effectiveAgentType = args.forcedAgentType ?? configuredAgentType; + const effectiveServiceId = + (args.forcedAgentType + ? resolveRoleServiceShadow(activeRole, effectiveAgentType) + : null) ?? resolveLeaseServiceId(args.lease, activeRole); + if (!effectiveServiceId) { + throw new Error(`Missing runtime service id for ${activeRole} lease`); + } + + const reviewerServiceId = resolveLeaseServiceId(args.lease, 'reviewer'); + const arbiterServiceId = resolveLeaseServiceId(args.lease, 'arbiter'); return { inferredRole, diff --git a/src/paired-execution-context.test.ts b/src/paired-execution-context.test.ts index af608ec..d6071dd 100644 --- a/src/paired-execution-context.test.ts +++ b/src/paired-execution-context.test.ts @@ -327,6 +327,7 @@ describe('paired execution context', () => { 'task-1', expect.objectContaining({ status: 'in_review' }), ); + expect(result?.claimedTaskUpdatedAt).toBe('2026-03-28T00:00:00.000Z'); expect(result?.envOverrides).toMatchObject({ EJCLAW_WORK_DIR: '/tmp/paired/task-1/reviewer', EJCLAW_REVIEWER_RUNTIME: '1', diff --git a/src/paired-execution-context.ts b/src/paired-execution-context.ts index 07e972f..cdef09e 100644 --- a/src/paired-execution-context.ts +++ b/src/paired-execution-context.ts @@ -171,6 +171,7 @@ function ensureActiveTask( export interface PreparedPairedExecutionContext { task: PairedTask; + claimedTaskUpdatedAt?: string; workspace: PairedWorkspace | null; envOverrides: Record; gateTurnKind?: string | null; @@ -209,6 +210,7 @@ export function preparePairedExecutionContext(args: { } const latestTask = getPairedTaskById(task.id) ?? task; + const claimedTaskUpdatedAt = latestTask.updated_at; let workspace: PairedWorkspace | null = null; let blockMessage: string | undefined; const now = new Date().toISOString(); @@ -349,6 +351,7 @@ export function preparePairedExecutionContext(args: { return { task: getPairedTaskById(task.id) ?? task, + claimedTaskUpdatedAt, workspace, envOverrides, blockMessage, From c0ec9fa8ef60651bd2b5dfaa063de9f86d378493 Mon Sep 17 00:00:00 2001 From: ejclaw Date: Fri, 10 Apr 2026 14:07:04 +0900 Subject: [PATCH 3/7] style: format db.test.ts --- src/db.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/db.test.ts b/src/db.test.ts index 85942c7..fbf8233 100644 --- a/src/db.test.ts +++ b/src/db.test.ts @@ -746,7 +746,9 @@ describe('task CRUD', () => { const migratedDb = new Database(dbPath, { readonly: true }); try { const remainingLogs = migratedDb - .prepare(`SELECT COUNT(*) AS count FROM task_run_logs WHERE task_id = ?`) + .prepare( + `SELECT COUNT(*) AS count FROM task_run_logs WHERE task_id = ?`, + ) .get('task-legacy-orphan') as { count: number }; const fkViolations = migratedDb.prepare('PRAGMA foreign_key_check').all(); From f0d061073f81e993b3be11f5796aaffbf4ee1882 Mon Sep 17 00:00:00 2001 From: ejclaw Date: Fri, 10 Apr 2026 15:22:45 +0900 Subject: [PATCH 4/7] Audit outbound delivery and fix reviewer turn routing --- src/channels/discord.ts | 33 +++- src/message-agent-executor.ts | 2 +- src/message-runtime.ts | 12 +- src/message-turn-controller.test.ts | 224 ++++++++++++++++++++++++++++ src/message-turn-controller.ts | 80 ++++++++++ src/room-role-context.test.ts | 26 ++++ src/room-role-context.ts | 7 +- src/types.ts | 8 + 8 files changed, 385 insertions(+), 7 deletions(-) create mode 100644 src/message-turn-controller.test.ts diff --git a/src/channels/discord.ts b/src/channels/discord.ts index e689e45..c84aad3 100644 --- a/src/channels/discord.ts +++ b/src/channels/discord.ts @@ -601,6 +601,8 @@ export class DiscordChannel implements Channel { attachmentCount: files.length, messageId: sentMessageIds[0] ?? null, messageIds: sentMessageIds, + botUserId: this.client.user?.id ?? null, + botUsername: this.client.user?.username ?? null, }, 'Discord message sent', ); @@ -689,6 +691,18 @@ export class DiscordChannel implements Channel { const channel = await this.client.channels.fetch(channelId); if (!channel || !('send' in channel)) return null; const msg = await (channel as TextChannel).send(text); + logger.info( + { + jid, + channelName: this.name, + deliveryMode: 'tracked-send', + messageId: msg.id, + botUserId: this.client.user?.id ?? null, + botUsername: this.client.user?.username ?? null, + length: text.length, + }, + 'Discord tracked message sent', + ); return msg.id; } catch (err) { logger.error({ jid, err }, 'Failed to send tracked Discord message'); @@ -696,6 +710,14 @@ export class DiscordChannel implements Channel { } } + getOutboundAuditMeta() { + return { + channelName: this.name, + botUserId: this.client?.user?.id ?? null, + botUsername: this.client?.user?.username ?? null, + }; + } + async getChannelMeta(jids: string[]): Promise> { const result = new Map(); if (!this.client) return result; @@ -813,12 +835,21 @@ export class DiscordChannel implements Channel { deliveryMode: 'edit', messageId, length: text.length, + botUserId: this.client.user?.id ?? null, + botUsername: this.client.user?.username ?? null, }, 'Discord message edited', ); } catch (err) { logger.debug( - { jid, channelName: this.name, messageId, err }, + { + jid, + channelName: this.name, + messageId, + botUserId: this.client?.user?.id ?? null, + botUsername: this.client?.user?.username ?? null, + err, + }, 'Failed to edit Discord message', ); throw err; // Re-throw so callers (e.g. dashboard) can reset message ID diff --git a/src/message-agent-executor.ts b/src/message-agent-executor.ts index 4b935a2..970d004 100644 --- a/src/message-agent-executor.ts +++ b/src/message-agent-executor.ts @@ -397,7 +397,7 @@ export async function runAgentForGroup( const pairedExecutionLifecycle = createPairedExecutionLifecycle({ pairedExecutionContext, pairedTurnIdentity: runtimePairedTurnIdentity, - completedRole: roomRoleContext?.role ?? 'owner', + completedRole: runtimePairedTurnIdentity?.role ?? activeRole, chatJid, runId, enqueueMessageCheck: () => deps.queue.enqueueMessageCheck(chatJid), diff --git a/src/message-runtime.ts b/src/message-runtime.ts index 8f35bcb..00b5f92 100644 --- a/src/message-runtime.ts +++ b/src/message-runtime.ts @@ -268,6 +268,10 @@ export function createMessageRuntime(deps: MessageRuntimeDeps): { const pairedRoom = hasReviewerLease(chatJid); const resolvedDeliveryRole = args.deliveryRole ?? args.forcedRole ?? (pairedRoom ? 'owner' : null); + const resolvedDeliveryServiceId = resolveLeaseServiceId( + getEffectiveChannelLease(chatJid), + resolvedDeliveryRole ?? 'owner', + ); const turnController = new MessageTurnController({ chatJid, group, @@ -279,13 +283,13 @@ export function createMessageRuntime(deps: MessageRuntimeDeps): { clearSession: () => deps.clearSession(group.folder), requestClose: (reason) => deps.queue.closeStdin(chatJid, { runId, reason }), + deliveryRole: resolvedDeliveryRole, + deliveryServiceId: resolvedDeliveryServiceId, + pairedTurnIdentity: args.pairedTurnIdentity ?? null, deliverFinalText: async (text) => { try { const persistedDeliveryRole = resolvedDeliveryRole; - const persistedDeliveryServiceId = resolveLeaseServiceId( - getEffectiveChannelLease(chatJid), - persistedDeliveryRole ?? 'owner', - ); + const persistedDeliveryServiceId = resolvedDeliveryServiceId; if ( (persistedDeliveryRole === 'reviewer' || persistedDeliveryRole === 'arbiter') && diff --git a/src/message-turn-controller.test.ts b/src/message-turn-controller.test.ts new file mode 100644 index 0000000..2eafd85 --- /dev/null +++ b/src/message-turn-controller.test.ts @@ -0,0 +1,224 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +vi.mock('./logger.js', () => { + const mockLogger = { + debug: vi.fn(), + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + child: (_bindings?: Record) => mockLogger, + }; + return { + logger: mockLogger, + createScopedLogger: (_bindings?: Record) => mockLogger, + }; +}); + +import { MessageTurnController } from './message-turn-controller.js'; +import { logger } from './logger.js'; +import type { Channel, RegisteredGroup } from './types.js'; +import type { PairedTurnIdentity } from './paired-turn-identity.js'; + +function makeGroup(): RegisteredGroup { + return { + name: 'Test Group', + folder: 'test-group', + trigger: '@Andy', + added_at: new Date().toISOString(), + requiresTrigger: false, + agentType: 'claude-code', + }; +} + +function makeChannel(): Channel { + return { + name: 'discord-review', + connect: vi.fn().mockResolvedValue(undefined), + disconnect: vi.fn().mockResolvedValue(undefined), + isConnected: vi.fn(() => true), + ownsJid: vi.fn(() => true), + sendMessage: vi.fn().mockResolvedValue(undefined), + sendAndTrack: vi.fn().mockResolvedValue('progress-1'), + editMessage: vi.fn().mockResolvedValue(undefined), + setTyping: vi.fn().mockResolvedValue(undefined), + getOutboundAuditMeta: vi.fn(() => ({ + channelName: 'discord-review', + botUserId: 'bot-review', + botUsername: 'reviewer-bot', + })), + }; +} + +function makeTurnIdentity(): PairedTurnIdentity { + return { + turnId: 'task-1:2026-04-10T14:22:00.000Z:reviewer-turn', + taskId: 'task-1', + taskUpdatedAt: '2026-04-10T14:22:00.000Z', + intentKind: 'reviewer-turn', + role: 'reviewer', + }; +} + +async function flushAsync(): Promise { + await new Promise((resolve) => setTimeout(resolve, 0)); +} + +function getAuditEntries(): Array> { + return vi + .mocked(logger.info) + .mock.calls.map(([payload]) => payload) + .filter( + (payload): payload is Record => + !!payload && typeof payload === 'object' && 'auditEvent' in payload, + ); +} + +describe('MessageTurnController outbound audit logging', () => { + beforeEach(() => { + vi.resetAllMocks(); + }); + + it('logs outbound progress and final audit context with role/service/turn metadata', async () => { + const channel = makeChannel(); + const deliverFinalText = vi.fn().mockResolvedValue(true); + const controller = new MessageTurnController({ + chatJid: 'dc:test-room', + group: makeGroup(), + runId: 'run-review-1', + channel, + idleTimeout: 1_000, + failureFinalText: '실패', + isClaudeCodeAgent: true, + clearSession: vi.fn(), + requestClose: vi.fn(), + deliverFinalText, + deliveryRole: 'reviewer', + deliveryServiceId: 'codex-review', + pairedTurnIdentity: makeTurnIdentity(), + }); + + await controller.start(); + await controller.handleOutput({ + status: 'success', + phase: 'progress', + result: '첫 진행 상황', + } as any); + await controller.handleOutput({ + status: 'success', + phase: 'progress', + result: '둘째 진행 상황', + } as any); + await flushAsync(); + await controller.handleOutput({ + status: 'success', + phase: 'final', + result: '최종 답변', + } as any); + await controller.finish('success'); + + expect(channel.sendAndTrack).toHaveBeenCalledTimes(1); + expect(channel.editMessage).toHaveBeenCalledWith( + 'dc:test-room', + 'progress-1', + expect.stringContaining('둘째 진행 상황'), + ); + expect(deliverFinalText).toHaveBeenCalledWith('최종 답변'); + + expect(getAuditEntries()).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + auditEvent: 'progress-create', + chatJid: 'dc:test-room', + runId: 'run-review-1', + deliveryRole: 'reviewer', + serviceId: 'codex-review', + turnId: 'task-1:2026-04-10T14:22:00.000Z:reviewer-turn', + turnRole: 'reviewer', + channelName: 'discord-review', + botUserId: 'bot-review', + botUsername: 'reviewer-bot', + messageId: 'progress-1', + }), + expect.objectContaining({ + auditEvent: 'progress-edit', + deliveryRole: 'reviewer', + serviceId: 'codex-review', + turnId: 'task-1:2026-04-10T14:22:00.000Z:reviewer-turn', + messageId: 'progress-1', + }), + expect.objectContaining({ + auditEvent: 'final-delivery-attempt', + deliveryRole: 'reviewer', + serviceId: 'codex-review', + turnId: 'task-1:2026-04-10T14:22:00.000Z:reviewer-turn', + }), + expect.objectContaining({ + auditEvent: 'final-delivery-result', + deliveryRole: 'reviewer', + serviceId: 'codex-review', + turnId: 'task-1:2026-04-10T14:22:00.000Z:reviewer-turn', + delivered: true, + }), + ]), + ); + }); + + it('logs fallback progress audit when tracked message creation fails', async () => { + const channel = makeChannel(); + const deliverFinalText = vi.fn().mockResolvedValue(true); + vi.mocked(channel.sendAndTrack!).mockRejectedValueOnce( + new Error('send failed'), + ); + + const controller = new MessageTurnController({ + chatJid: 'dc:test-room', + group: makeGroup(), + runId: 'run-review-fallback', + channel, + idleTimeout: 1_000, + failureFinalText: '실패', + isClaudeCodeAgent: true, + clearSession: vi.fn(), + requestClose: vi.fn(), + deliverFinalText, + deliveryRole: 'reviewer', + deliveryServiceId: 'codex-review', + pairedTurnIdentity: makeTurnIdentity(), + }); + + await controller.start(); + await controller.handleOutput({ + status: 'success', + phase: 'progress', + result: '첫 진행 상황', + } as any); + await controller.handleOutput({ + status: 'success', + phase: 'progress', + result: '둘째 진행 상황', + } as any); + await flushAsync(); + await controller.finish('success'); + + expect(channel.sendMessage).toHaveBeenCalledWith( + 'dc:test-room', + expect.stringContaining('첫 진행 상황'), + ); + expect(getAuditEntries()).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + auditEvent: 'progress-fallback-send', + chatJid: 'dc:test-room', + runId: 'run-review-fallback', + deliveryRole: 'reviewer', + serviceId: 'codex-review', + turnId: 'task-1:2026-04-10T14:22:00.000Z:reviewer-turn', + channelName: 'discord-review', + botUserId: 'bot-review', + messageId: null, + fallbackReason: 'tracked-send-error', + }), + ]), + ); + }); +}); diff --git a/src/message-turn-controller.ts b/src/message-turn-controller.ts index e396e47..7a5665c 100644 --- a/src/message-turn-controller.ts +++ b/src/message-turn-controller.ts @@ -5,11 +5,13 @@ import { formatOutbound } from './router.js'; import { shouldResetSessionOnAgentFailure } from './session-recovery.js'; import { TASK_STATUS_MESSAGE_PREFIX } from './task-watch-status.js'; import { formatElapsedKorean } from './utils.js'; +import type { PairedTurnIdentity } from './paired-turn-identity.js'; import { normalizeAgentOutputPhase, toVisiblePhase, type AgentOutputPhase, type Channel, + type PairedRoomRole, type RegisteredGroup, type VisiblePhase, } from './types.js'; @@ -32,6 +34,9 @@ interface MessageTurnControllerOptions { clearSession: () => void; requestClose: (reason: string) => void; deliverFinalText: (text: string) => Promise; + deliveryRole?: PairedRoomRole | null; + deliveryServiceId?: string | null; + pairedTurnIdentity?: PairedTurnIdentity | null; } export class MessageTurnController { @@ -66,6 +71,41 @@ export class MessageTurnController { }); } + private buildOutboundAuditContext( + extra: Record = {}, + ): Record { + const outboundMeta = this.options.channel.getOutboundAuditMeta?.(); + return { + chatJid: this.options.chatJid, + runId: this.options.runId, + deliveryRole: + this.options.deliveryRole ?? + this.options.pairedTurnIdentity?.role ?? + null, + serviceId: this.options.deliveryServiceId ?? null, + turnId: this.options.pairedTurnIdentity?.turnId ?? null, + turnRole: this.options.pairedTurnIdentity?.role ?? null, + intentKind: this.options.pairedTurnIdentity?.intentKind ?? null, + channelName: outboundMeta?.channelName ?? this.options.channel.name, + botUserId: outboundMeta?.botUserId ?? null, + botUsername: outboundMeta?.botUsername ?? null, + ...extra, + }; + } + + private logOutboundAudit( + auditEvent: string, + extra: Record = {}, + ): void { + this.log.info( + this.buildOutboundAuditContext({ + auditEvent, + ...extra, + }), + 'Outbound message audit', + ); + } + private async setTyping( isTyping: boolean, source: string, @@ -481,6 +521,11 @@ export class MessageTurnController { ); this.latestProgressRendered = rendered; this.progressEditFailCount = 0; + this.logOutboundAudit('progress-edit', { + messageId: this.progressMessageId, + textLength: this.latestProgressText.length, + renderedLength: rendered.length, + }); } catch (err) { this.progressEditFailCount++; this.log.warn( @@ -529,7 +574,16 @@ export class MessageTurnController { private async deliverFinalText(text: string): Promise { await this.activateTyping('turn:deliver-final'); this.visiblePhase = toVisiblePhase('final'); + this.logOutboundAudit('final-delivery-attempt', { + messageId: this.progressMessageId, + textLength: text.length, + }); const delivered = await this.options.deliverFinalText(text); + this.logOutboundAudit('final-delivery-result', { + messageId: this.progressMessageId, + textLength: text.length, + delivered, + }); if (!delivered) { this.producedDeliverySucceeded = false; } @@ -581,6 +635,12 @@ export class MessageTurnController { if (!this.options.channel.sendAndTrack) { this.latestProgressRendered = rendered; await this.options.channel.sendMessage(this.options.chatJid, rendered); + this.logOutboundAudit('progress-fallback-send', { + messageId: null, + tracked: false, + textLength: text.length, + renderedLength: rendered.length, + }); this.visiblePhase = toVisiblePhase('progress'); return; } @@ -594,6 +654,13 @@ export class MessageTurnController { this.log.warn({ err }, 'Failed to send tracked progress message'); this.latestProgressRendered = rendered; await this.options.channel.sendMessage(this.options.chatJid, rendered); + this.logOutboundAudit('progress-fallback-send', { + messageId: null, + tracked: false, + fallbackReason: 'tracked-send-error', + textLength: text.length, + renderedLength: rendered.length, + }); this.visiblePhase = toVisiblePhase('progress'); return; } @@ -607,6 +674,12 @@ export class MessageTurnController { 'Created tracked progress message', ); this.latestProgressRendered = rendered; + this.logOutboundAudit('progress-create', { + messageId: this.progressMessageId, + tracked: true, + textLength: text.length, + renderedLength: rendered.length, + }); this.ensureProgressTicker(); this.visiblePhase = toVisiblePhase('progress'); return; @@ -614,6 +687,13 @@ export class MessageTurnController { this.latestProgressRendered = rendered; await this.options.channel.sendMessage(this.options.chatJid, rendered); + this.logOutboundAudit('progress-fallback-send', { + messageId: null, + tracked: false, + fallbackReason: 'tracked-send-returned-null', + textLength: text.length, + renderedLength: rendered.length, + }); this.visiblePhase = toVisiblePhase('progress'); } diff --git a/src/room-role-context.test.ts b/src/room-role-context.test.ts index 717ca3d..5589586 100644 --- a/src/room-role-context.test.ts +++ b/src/room-role-context.test.ts @@ -137,6 +137,32 @@ describe('buildRoomRoleContext', () => { }); }); + it('keeps the preferred reviewer role when fallback execution shares the arbiter service shadow', () => { + expect( + buildRoomRoleContext( + { + chat_jid: 'group@test', + owner_service_id: 'codex-main', + reviewer_service_id: 'claude', + arbiter_service_id: 'codex-review', + owner_failover_active: false, + activated_at: null, + reason: null, + explicit: false, + }, + 'codex-review', + 'reviewer', + ), + ).toEqual({ + serviceId: 'codex-review', + role: 'reviewer', + ownerServiceId: 'codex-main', + reviewerServiceId: 'claude', + failoverOwner: false, + arbiterServiceId: 'codex-review', + }); + }); + it('returns undefined for a non-paired room', () => { expect( buildRoomRoleContext( diff --git a/src/room-role-context.ts b/src/room-role-context.ts index 49581d5..e242f45 100644 --- a/src/room-role-context.ts +++ b/src/room-role-context.ts @@ -26,8 +26,13 @@ export function buildRoomRoleContext( arbiter: arbiterServiceId === normalizedServiceId, }; + const canHonorPreferredRole = + preferredRole === 'owner' || + (preferredRole === 'reviewer' && reviewerServiceId !== null) || + (preferredRole === 'arbiter' && arbiterServiceId !== undefined); + const role = - preferredRole && matches[preferredRole] + preferredRole && canHonorPreferredRole ? preferredRole : matches.arbiter ? 'arbiter' diff --git a/src/types.ts b/src/types.ts index 1ae44fc..2a8ce0e 100644 --- a/src/types.ts +++ b/src/types.ts @@ -209,6 +209,12 @@ export interface ChannelMeta { categoryPosition: number; } +export interface ChannelOutboundAuditMeta { + channelName: string; + botUserId: string | null; + botUsername: string | null; +} + export interface Channel { name: string; connect(): Promise; @@ -229,6 +235,8 @@ export interface Channel { getChannelMeta?(jids: string[]): Promise>; // Optional: delete all messages in a channel (used for dashboard cleanup). purgeChannel?(jid: string): Promise; + // Optional: expose runtime sender identity for outbound audit logs. + getOutboundAuditMeta?(): ChannelOutboundAuditMeta; } // Callback type that channels use to deliver inbound messages From 8f8cfac58fb2d62511f60aa4dec160930be9e52d Mon Sep 17 00:00:00 2001 From: ejclaw Date: Fri, 10 Apr 2026 16:20:23 +0900 Subject: [PATCH 5/7] Backfill paired agent metadata for raw service ids --- src/config.ts | 2 ++ src/db.test.ts | 29 +++++++++++++++++++++++++++++ src/db/canonical-role-metadata.ts | 13 +++++++++++-- src/db/paired-state.ts | 7 ++++++- 4 files changed, 48 insertions(+), 3 deletions(-) diff --git a/src/config.ts b/src/config.ts index e77b3a7..ae928b9 100644 --- a/src/config.ts +++ b/src/config.ts @@ -22,6 +22,8 @@ export function normalizeServiceId(serviceId: string): string { } export const SERVICE_SESSION_SCOPE = normalizeServiceId(SERVICE_ID); +export const CURRENT_RUNTIME_AGENT_TYPE = + SERVICE_SESSION_SCOPE === CLAUDE_SERVICE_ID ? 'claude-code' : 'codex'; export function isClaudeService(serviceId: string = SERVICE_ID): boolean { return normalizeServiceId(serviceId) === CLAUDE_SERVICE_ID; diff --git a/src/db.test.ts b/src/db.test.ts index fbf8233..701ce63 100644 --- a/src/db.test.ts +++ b/src/db.test.ts @@ -92,6 +92,7 @@ import { CLAUDE_SERVICE_ID, CODEX_MAIN_SERVICE_ID, CODEX_REVIEW_SERVICE_ID, + OWNER_AGENT_TYPE, SERVICE_ID, SERVICE_SESSION_SCOPE, normalizeServiceId, @@ -1152,6 +1153,34 @@ describe('paired task state', () => { }); }); + it('backfills configured owner agent type when creating a paired task with a raw non-shadow owner service id', () => { + createPairedTask({ + id: 'paired-task-raw-owner-service', + chat_jid: 'dc:paired-raw-owner-service', + group_folder: 'paired-raw-owner-service', + owner_service_id: 'andy', + reviewer_service_id: CLAUDE_SERVICE_ID, + title: null, + source_ref: 'HEAD', + plan_notes: null, + review_requested_at: null, + round_trip_count: 0, + status: 'active', + arbiter_verdict: null, + arbiter_requested_at: null, + completion_reason: null, + created_at: '2026-03-28T00:00:00.000Z', + updated_at: '2026-03-28T00:00:00.000Z', + }); + + expect(getPairedTaskById('paired-task-raw-owner-service')).toMatchObject({ + owner_service_id: 'andy', + reviewer_service_id: CLAUDE_SERVICE_ID, + owner_agent_type: OWNER_AGENT_TYPE, + reviewer_agent_type: 'claude-code', + }); + }); + it('preserves raw legacy paired task service ids during init when registered group metadata is present', () => { const tempDir = fs.mkdtempSync('/tmp/ejclaw-paired-task-legacy-groups-'); const dbPath = path.join(tempDir, 'messages.db'); diff --git a/src/db/canonical-role-metadata.ts b/src/db/canonical-role-metadata.ts index 4915f80..14fd2ee 100644 --- a/src/db/canonical-role-metadata.ts +++ b/src/db/canonical-role-metadata.ts @@ -1,4 +1,9 @@ -import { ARBITER_AGENT_TYPE, SERVICE_SESSION_SCOPE } from '../config.js'; +import { + ARBITER_AGENT_TYPE, + OWNER_AGENT_TYPE, + REVIEWER_AGENT_TYPE, + SERVICE_SESSION_SCOPE, +} from '../config.js'; import { inferAgentTypeFromServiceShadow, inferRoleFromServiceShadow, @@ -12,6 +17,7 @@ interface RequiredRoleMetadataInput { role: PairedRoomRole; storedAgentType?: string | null; storedServiceId?: string | null; + fallbackAgentType?: AgentType | null; } interface OptionalRoleMetadataInput extends RequiredRoleMetadataInput {} @@ -31,7 +37,8 @@ function resolveRequiredAgentType(input: RequiredRoleMetadataInput): AgentType { ); } - const agentType = persistedAgentType ?? inferredAgentType; + const agentType = + persistedAgentType ?? inferredAgentType ?? input.fallbackAgentType ?? null; if (agentType) { return agentType; } @@ -100,12 +107,14 @@ export function canonicalizePairedTaskMetadata(input: { role: 'owner', storedAgentType: input.owner_agent_type, storedServiceId: input.owner_service_id, + fallbackAgentType: OWNER_AGENT_TYPE, }); const reviewerAgentType = resolveRequiredAgentType({ context, role: 'reviewer', storedAgentType: input.reviewer_agent_type, storedServiceId: input.reviewer_service_id, + fallbackAgentType: REVIEWER_AGENT_TYPE, }); return { diff --git a/src/db/paired-state.ts b/src/db/paired-state.ts index 5daaf0c..2f2584b 100644 --- a/src/db/paired-state.ts +++ b/src/db/paired-state.ts @@ -1,6 +1,10 @@ import { Database } from 'bun:sqlite'; -import { SERVICE_ID, normalizeServiceId } from '../config.js'; +import { + CURRENT_RUNTIME_AGENT_TYPE, + SERVICE_ID, + normalizeServiceId, +} from '../config.js'; import { buildPairedTurnIdentity, resolvePairedTurnRole, @@ -602,6 +606,7 @@ export function claimPairedTurnReservationInDatabase( const currentAttempt = markPairedTurnRunningInDatabase(database, { turnIdentity, executorServiceId: CURRENT_SERVICE_ID, + executorAgentType: CURRENT_RUNTIME_AGENT_TYPE, runId: args.runId, }); if (!currentAttempt) { From 446223e3c845b95f6878eda832fa1e6eee48a3fb Mon Sep 17 00:00:00 2001 From: ejclaw Date: Fri, 10 Apr 2026 18:45:19 +0900 Subject: [PATCH 6/7] Fix paired reviewer finalize flow --- src/message-agent-executor-paired.ts | 51 ++++++++--- src/message-agent-executor.test.ts | 93 +++++++++++++++++++++ src/message-agent-executor.ts | 1 + src/message-turn-controller.test.ts | 62 ++++++++++++++ src/message-turn-controller.ts | 30 +++++-- src/paired-execution-context-shared.test.ts | 28 +++++++ src/paired-execution-context-shared.ts | 21 ++++- src/paired-execution-context.test.ts | 46 ++++++++++ src/paired-execution-context.ts | 15 +++- 9 files changed, 330 insertions(+), 17 deletions(-) diff --git a/src/message-agent-executor-paired.ts b/src/message-agent-executor-paired.ts index f782d09..aa4a0f1 100644 --- a/src/message-agent-executor-paired.ts +++ b/src/message-agent-executor-paired.ts @@ -14,6 +14,7 @@ import { completePairedExecutionContext, type PreparedPairedExecutionContext, } from './paired-execution-context.js'; +import { hasRecognizedPairedTerminalSummary } from './paired-execution-context-shared.js'; import { resolvePairedFollowUpQueueAction } from './message-agent-executor-rules.js'; import { enqueuePairedFollowUpAfterEvent } from './message-runtime-follow-up.js'; import type { PairedTurnIdentity } from './paired-turn-identity.js'; @@ -60,7 +61,8 @@ export function createPairedExecutionLifecycle(args: { let pairedExecutionStatus: 'succeeded' | 'failed' = 'failed'; let pairedExecutionSummary: string | null = null; - let pairedFinalOutput: string | null = null; + let pairedTerminalSummary: string | null = null; + let pairedTerminalOutput: string | null = null; let pairedExecutionCompleted = false; let pairedExecutionDelegated = false; let pairedSawOutput = false; @@ -131,12 +133,20 @@ export function createPairedExecutionLifecycle(args: { leaseHeartbeatTimer.unref?.(); } + const rememberTerminalOutputIfRecognized = (outputText: string) => { + if (!hasRecognizedPairedTerminalSummary(completedRole, outputText)) { + return; + } + pairedTerminalSummary = outputText.slice(0, 500); + pairedTerminalOutput = outputText; + }; + const persistPairedTurnOutputIfNeeded = () => { if ( !pairedExecutionContext || pairedTurnOutputPersisted || - !pairedFinalOutput || - pairedFinalOutput.length === 0 + !pairedTerminalOutput || + pairedTerminalOutput.length === 0 ) { return; } @@ -146,7 +156,7 @@ export function createPairedExecutionLifecycle(args: { pairedExecutionContext.task.id, turnNumber, completedRole, - pairedFinalOutput, + pairedTerminalOutput, ); pairedTurnOutputPersisted = true; }; @@ -156,8 +166,8 @@ export function createPairedExecutionLifecycle(args: { completedRole !== 'owner' || !pairedExecutionContext || pairedExecutionCompleted || - !pairedFinalOutput || - pairedFinalOutput.length === 0 + !pairedTerminalOutput || + pairedTerminalOutput.length === 0 ) { return; } @@ -171,7 +181,7 @@ export function createPairedExecutionLifecycle(args: { role: completedRole, status: 'succeeded', runId, - summary: pairedExecutionSummary, + summary: pairedTerminalSummary ?? pairedExecutionSummary, }); pairedExecutionCompleted = true; }; @@ -180,6 +190,7 @@ export function createPairedExecutionLifecycle(args: { updateSummary({ outputText, errorText }) { if (outputText && outputText.length > 0) { pairedExecutionSummary = outputText.slice(0, 500); + rememberTerminalOutputIfRecognized(outputText); return; } @@ -189,7 +200,7 @@ export function createPairedExecutionLifecycle(args: { }, recordFinalOutputBeforeDelivery(outputText) { - pairedFinalOutput = outputText; + rememberTerminalOutputIfRecognized(outputText); completeSuccessfulOwnerTurnBeforeDeliveryIfNeeded(); persistPairedTurnOutputIfNeeded(); }, @@ -254,12 +265,29 @@ export function createPairedExecutionLifecycle(args: { return; } + const missingReviewerOrArbiterVerdict = + (completedRole === 'reviewer' || completedRole === 'arbiter') && + pairedExecutionStatus === 'succeeded' && + !pairedTerminalSummary; const effectiveStatus = completedRole === 'owner' && pairedExecutionStatus === 'succeeded' && !pairedSawOutput ? 'failed' - : pairedExecutionStatus; + : missingReviewerOrArbiterVerdict + ? 'failed' + : pairedExecutionStatus; + + if (missingReviewerOrArbiterVerdict) { + log.warn( + { + pairedTaskId: pairedExecutionContext?.task.id ?? null, + role: completedRole, + summary: pairedExecutionSummary, + }, + 'Downgraded paired reviewer/arbiter completion to failed because no explicit terminal verdict was emitted', + ); + } if (pairedExecutionContext && !pairedExecutionCompleted) { if (effectiveStatus === 'succeeded') { @@ -278,7 +306,10 @@ export function createPairedExecutionLifecycle(args: { role: completedRole, status: effectiveStatus, runId, - summary: pairedExecutionSummary, + summary: + effectiveStatus === 'succeeded' + ? (pairedTerminalSummary ?? pairedExecutionSummary) + : (pairedExecutionSummary ?? pairedTerminalSummary), }); pairedExecutionCompleted = true; } diff --git a/src/message-agent-executor.test.ts b/src/message-agent-executor.test.ts index 0bd37bb..7e3041f 100644 --- a/src/message-agent-executor.test.ts +++ b/src/message-agent-executor.test.ts @@ -1236,6 +1236,99 @@ describe('runAgentForGroup room memory', () => { ]); }); + it('fails a paired reviewer turn when the run ends after progress-only output without an explicit reviewer verdict', async () => { + const group = { + ...makeGroup(), + folder: 'test-group', + workDir: '/repo/canonical', + }; + + vi.mocked( + pairedExecutionContext.preparePairedExecutionContext, + ).mockReturnValue({ + task: { + id: 'paired-task-reviewer-progress-only', + chat_jid: 'group@test', + group_folder: 'test-group', + owner_service_id: 'codex-main', + reviewer_service_id: 'codex-review', + title: null, + source_ref: 'HEAD', + plan_notes: null, + round_trip_count: 0, + review_requested_at: '2026-04-10T00:00:00.000Z', + status: 'in_review', + arbiter_verdict: null, + arbiter_requested_at: null, + completion_reason: null, + created_at: '2026-04-10T00:00:00.000Z', + updated_at: '2026-04-10T00:00:00.000Z', + }, + workspace: null, + envOverrides: {}, + }); + vi.mocked(agentRunner.runAgentProcess).mockImplementation( + async (_group, _input, _onProcess, onOutput) => { + await onOutput?.({ + status: 'success', + phase: 'progress', + result: + '요청 범위가 실제로 지워졌는지와 검증 근거가 맞는지 확인하겠습니다.', + output: { + visibility: 'public', + text: '요청 범위가 실제로 지워졌는지와 검증 근거가 맞는지 확인하겠습니다.', + }, + } as AgentOutput); + await onOutput?.({ + status: 'success', + phase: 'progress', + result: + '우선 변경 파일과 현재 계약 노출 지점을 직접 읽고, 필요하면 전용 검증도 다시 돌리겠습니다.', + output: { + visibility: 'public', + text: '우선 변경 파일과 현재 계약 노출 지점을 직접 읽고, 필요하면 전용 검증도 다시 돌리겠습니다.', + }, + } as AgentOutput); + return { + status: 'success', + result: null, + } as AgentOutput; + }, + ); + + await expect( + runAgentForGroup(makeDeps(), { + group, + prompt: 'review please', + chatJid: 'group@test', + runId: 'run-reviewer-progress-only', + forcedRole: 'reviewer', + forcedAgentType: 'codex', + pairedTurnIdentity: { + turnId: + 'paired-task-reviewer-progress-only:2026-04-10T00:00:00.000Z:reviewer-turn', + taskId: 'paired-task-reviewer-progress-only', + taskUpdatedAt: '2026-04-10T00:00:00.000Z', + intentKind: 'reviewer-turn', + role: 'reviewer', + }, + }), + ).resolves.toBe('success'); + + expect( + pairedExecutionContext.completePairedExecutionContext, + ).toHaveBeenCalledWith( + expect.objectContaining({ + taskId: 'paired-task-reviewer-progress-only', + role: 'reviewer', + status: 'failed', + summary: expect.stringContaining( + '우선 변경 파일과 현재 계약 노출 지점을', + ), + }), + ); + }); + it('logs streamed activity with resolved execution attribution', async () => { const group = { ...makeGroup(), diff --git a/src/message-agent-executor.ts b/src/message-agent-executor.ts index 970d004..7626324 100644 --- a/src/message-agent-executor.ts +++ b/src/message-agent-executor.ts @@ -194,6 +194,7 @@ export async function runAgentForGroup( runId, roomRoleContext, hasHumanMessage: args.hasHumanMessage, + pairedTurnIdentity: args.pairedTurnIdentity, }); const preparedTurnTaskUpdatedAt = pairedExecutionContext ? (pairedExecutionContext.claimedTaskUpdatedAt ?? diff --git a/src/message-turn-controller.test.ts b/src/message-turn-controller.test.ts index 2eafd85..0b33f2d 100644 --- a/src/message-turn-controller.test.ts +++ b/src/message-turn-controller.test.ts @@ -59,6 +59,28 @@ function makeTurnIdentity(): PairedTurnIdentity { }; } +function makeTurnIdentityWithRole( + role: PairedTurnIdentity['role'], +): PairedTurnIdentity { + return { + ...makeTurnIdentity(), + role, + intentKind: + role === 'arbiter' + ? 'arbiter-turn' + : role === 'owner' + ? 'owner-turn' + : 'reviewer-turn', + turnId: `task-1:2026-04-10T14:22:00.000Z:${ + role === 'arbiter' + ? 'arbiter-turn' + : role === 'owner' + ? 'owner-turn' + : 'reviewer-turn' + }`, + }; +} + async function flushAsync(): Promise { await new Promise((resolve) => setTimeout(resolve, 0)); } @@ -221,4 +243,44 @@ describe('MessageTurnController outbound audit logging', () => { ]), ); }); + + it('does not promote paired reviewer progress-only output to a final message when no explicit verdict was emitted', async () => { + const channel = makeChannel(); + const deliverFinalText = vi.fn().mockResolvedValue(true); + + const controller = new MessageTurnController({ + chatJid: 'dc:test-room', + group: makeGroup(), + runId: 'run-reviewer-progress-only', + channel, + idleTimeout: 1_000, + failureFinalText: '실패', + isClaudeCodeAgent: false, + clearSession: vi.fn(), + requestClose: vi.fn(), + deliverFinalText, + deliveryRole: 'reviewer', + deliveryServiceId: 'codex-review', + pairedTurnIdentity: makeTurnIdentityWithRole('reviewer'), + }); + + await controller.start(); + await controller.handleOutput({ + status: 'success', + phase: 'progress', + result: + '요청 범위가 실제로 지워졌는지와 검증 근거가 맞는지 확인하겠습니다.', + } as any); + await controller.handleOutput({ + status: 'success', + phase: 'progress', + result: + '우선 변경 파일과 현재 계약 노출 지점을 직접 읽고, 필요하면 전용 검증도 다시 돌리겠습니다.', + } as any); + await flushAsync(); + await controller.finish('success'); + + expect(channel.sendAndTrack).toHaveBeenCalledTimes(1); + expect(deliverFinalText).not.toHaveBeenCalled(); + }); }); diff --git a/src/message-turn-controller.ts b/src/message-turn-controller.ts index 7a5665c..7b09542 100644 --- a/src/message-turn-controller.ts +++ b/src/message-turn-controller.ts @@ -5,6 +5,7 @@ import { formatOutbound } from './router.js'; import { shouldResetSessionOnAgentFailure } from './session-recovery.js'; import { TASK_STATUS_MESSAGE_PREFIX } from './task-watch-status.js'; import { formatElapsedKorean } from './utils.js'; +import { hasRecognizedPairedTerminalSummary } from './paired-execution-context-shared.js'; import type { PairedTurnIdentity } from './paired-turn-identity.js'; import { normalizeAgentOutputPhase, @@ -338,11 +339,30 @@ export class MessageTurnController { !this.hadError && this.latestProgressTextForFinal ) { - this.log.info( - 'Sending a separate final message from the last progress output after agent completion', - ); - await this.finalizeProgressMessage(); - await this.deliverFinalText(this.latestProgressTextForFinal); + const pairedRole = this.options.pairedTurnIdentity?.role ?? null; + const shouldPromoteToFinal = + !pairedRole || + hasRecognizedPairedTerminalSummary( + pairedRole, + this.latestProgressTextForFinal, + ); + if (shouldPromoteToFinal) { + this.log.info( + 'Sending a separate final message from the last progress output after agent completion', + ); + await this.finalizeProgressMessage(); + await this.deliverFinalText(this.latestProgressTextForFinal); + } else { + this.log.info( + { + pairedRole, + turnId: this.options.pairedTurnIdentity?.turnId ?? null, + }, + 'Skipped separate final message promotion because paired progress did not contain an explicit terminal verdict', + ); + await this.finalizeProgressMessage(); + this.latestProgressTextForFinal = null; + } } else if ( this.visiblePhase === 'progress' && !this.terminalObserved() && diff --git a/src/paired-execution-context-shared.test.ts b/src/paired-execution-context-shared.test.ts index b19568b..9c791c6 100644 --- a/src/paired-execution-context-shared.test.ts +++ b/src/paired-execution-context-shared.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from 'vitest'; import { + hasRecognizedPairedTerminalSummary, parseVisibleVerdict, resolveOwnerCompletionSignal, resolveReviewerCompletionSignal, @@ -18,6 +19,33 @@ describe('paired execution context shared verdict helpers', () => { expect(parseVisibleVerdict('random prose')).toBe('continue'); }); + it('recognizes paired terminal summaries only when the role-specific verdict is explicit', () => { + expect( + hasRecognizedPairedTerminalSummary('owner', 'plain owner update'), + ).toBe(true); + expect( + hasRecognizedPairedTerminalSummary( + 'reviewer', + 'DONE_WITH_CONCERNS\nfollow-up detail', + ), + ).toBe(true); + expect( + hasRecognizedPairedTerminalSummary( + 'reviewer', + '요청 범위를 실제로 지워졌는지 확인하겠습니다.', + ), + ).toBe(false); + expect( + hasRecognizedPairedTerminalSummary('arbiter', 'PROCEED\n승인합니다.'), + ).toBe(true); + expect( + hasRecognizedPairedTerminalSummary( + 'arbiter', + '대화와 현재 코드 상태를 대조해 먼저 확인하겠습니다.', + ), + ).toBe(false); + }); + it('maps normal owner completion verdicts to reviewer or arbiter signals', () => { expect( resolveOwnerCompletionSignal({ diff --git a/src/paired-execution-context-shared.ts b/src/paired-execution-context-shared.ts index 58d2f95..9066afd 100644 --- a/src/paired-execution-context-shared.ts +++ b/src/paired-execution-context-shared.ts @@ -3,7 +3,7 @@ import { execFileSync } from 'child_process'; import { isArbiterEnabled } from './config.js'; import { updatePairedTaskIfUnchanged } from './db.js'; import { logger } from './logger.js'; -import type { PairedTaskStatus } from './types.js'; +import type { PairedRoomRole, PairedTaskStatus } from './types.js'; export type VisibleVerdict = | 'done' @@ -149,6 +149,25 @@ export function classifyArbiterVerdict( return 'unknown'; } +export function hasRecognizedPairedTerminalSummary( + role: PairedRoomRole, + summary: string | null | undefined, +): boolean { + if (!summary || summary.trim().length === 0) { + return false; + } + + if (role === 'owner') { + return true; + } + + if (role === 'reviewer') { + return parseVisibleVerdict(summary) !== 'continue'; + } + + return classifyArbiterVerdict(summary) !== 'unknown'; +} + export function resolveCanonicalSourceRef(workDir: string): string { const treeHash = resolveCanonicalTreeHash(workDir); return treeHash || 'HEAD'; diff --git a/src/paired-execution-context.test.ts b/src/paired-execution-context.test.ts index d6071dd..c3409c0 100644 --- a/src/paired-execution-context.test.ts +++ b/src/paired-execution-context.test.ts @@ -46,6 +46,7 @@ import { preparePairedExecutionContext, } from './paired-execution-context.js'; import * as pairedWorkspaceManager from './paired-workspace-manager.js'; +import type { PairedTurnIdentity } from './paired-turn-identity.js'; import type { PairedTask, PairedWorkspace, @@ -335,6 +336,51 @@ describe('paired execution context', () => { }); }); + it('reuses the persisted reviewer turn revision when a claimed handoff re-enters an already in_review task', () => { + const pairedTurnIdentity: PairedTurnIdentity = { + turnId: 'task-1:2026-03-28T00:00:00.000Z:reviewer-turn', + taskId: 'task-1', + taskUpdatedAt: '2026-03-28T00:00:00.000Z', + intentKind: 'reviewer-turn', + role: 'reviewer', + }; + + vi.mocked(db.getLatestOpenPairedTaskForChat).mockReturnValue( + buildPairedTask({ + status: 'in_review', + review_requested_at: '2026-03-28T00:00:00.000Z', + updated_at: '2026-03-28T00:00:05.000Z', + }), + ); + vi.mocked(db.getPairedTaskById).mockReturnValue( + buildPairedTask({ + status: 'in_review', + review_requested_at: '2026-03-28T00:00:00.000Z', + updated_at: '2026-03-28T00:00:05.000Z', + }), + ); + vi.mocked( + pairedWorkspaceManager.prepareReviewerWorkspaceForExecution, + ).mockReturnValue({ + workspace: buildWorkspace('reviewer', '/tmp/paired/task-1/reviewer'), + autoRefreshed: false, + }); + + const result = preparePairedExecutionContext({ + group, + chatJid: 'dc:test', + runId: 'run-reviewer-handoff-reentry', + roomRoleContext: reviewerContext, + pairedTurnIdentity, + }); + + expect(result?.claimedTaskUpdatedAt).toBe('2026-03-28T00:00:00.000Z'); + expect(db.updatePairedTask).not.toHaveBeenCalledWith( + 'task-1', + expect.objectContaining({ status: 'in_review' }), + ); + }); + it('does not change task state when the reviewer snapshot is not ready', () => { vi.mocked(db.getLatestOpenPairedTaskForChat).mockReturnValue({ id: 'task-1', diff --git a/src/paired-execution-context.ts b/src/paired-execution-context.ts index cdef09e..9b6f6f8 100644 --- a/src/paired-execution-context.ts +++ b/src/paired-execution-context.ts @@ -46,6 +46,7 @@ import { prepareReviewerWorkspaceForExecution, provisionOwnerWorkspaceForPairedTask, } from './paired-workspace-manager.js'; +import type { PairedTurnIdentity } from './paired-turn-identity.js'; import type { AgentType, PairedRoomRole, @@ -193,6 +194,7 @@ export function preparePairedExecutionContext(args: { runId: string; roomRoleContext?: RoomRoleContext; hasHumanMessage?: boolean; + pairedTurnIdentity?: PairedTurnIdentity; }): PreparedPairedExecutionContext | undefined { const { group, chatJid, roomRoleContext } = args; if (!roomRoleContext || !group.workDir) { @@ -210,7 +212,18 @@ export function preparePairedExecutionContext(args: { } const latestTask = getPairedTaskById(task.id) ?? task; - const claimedTaskUpdatedAt = latestTask.updated_at; + const claimedTaskUpdatedAt = + args.pairedTurnIdentity && + args.pairedTurnIdentity.taskId === latestTask.id && + args.pairedTurnIdentity.role === roomRoleContext.role && + ((roomRoleContext.role === 'reviewer' && + (latestTask.status === 'review_ready' || + latestTask.status === 'in_review')) || + (roomRoleContext.role === 'arbiter' && + (latestTask.status === 'arbiter_requested' || + latestTask.status === 'in_arbitration'))) + ? args.pairedTurnIdentity.taskUpdatedAt + : latestTask.updated_at; let workspace: PairedWorkspace | null = null; let blockMessage: string | undefined; const now = new Date().toISOString(); From bc4251177158650725d146bf3eee95399c3d4155 Mon Sep 17 00:00:00 2001 From: ejclaw Date: Fri, 10 Apr 2026 23:41:41 +0900 Subject: [PATCH 7/7] fix: persist direct terminal verdicts for paired reviewers --- src/group-queue.test.ts | 43 +++++ src/group-queue.ts | 28 ++- src/index.ts | 2 +- src/message-agent-executor-paired.ts | 140 +++++++++------ src/message-agent-executor.test.ts | 249 ++++++++++++++++++++++++++- src/message-agent-executor.ts | 31 +++- 6 files changed, 431 insertions(+), 62 deletions(-) diff --git a/src/group-queue.test.ts b/src/group-queue.test.ts index fc72e6f..4c45386 100644 --- a/src/group-queue.test.ts +++ b/src/group-queue.test.ts @@ -124,6 +124,49 @@ describe('GroupQueue', () => { await vi.advanceTimersByTimeAsync(10); }); + it('stores direct terminal text for the active run and clears it when the run ends', async () => { + let releaseRun!: (value: boolean) => void; + let runId: string | undefined; + const blocker = new Promise((resolve) => { + releaseRun = resolve; + }); + + queue.setProcessMessagesFn( + async (_groupJid: string, context: GroupRunContext) => { + runId = context.runId; + return await blocker; + }, + ); + + queue.enqueueMessageCheck('group1@g.us'); + await vi.advanceTimersByTimeAsync(10); + + expect(runId).toEqual(expect.any(String)); + + queue.noteDirectTerminalDelivery( + 'group1@g.us', + 'reviewer', + 'DONE_WITH_CONCERNS\n리뷰 완료', + ); + + expect( + queue.hasDirectTerminalDeliveryForRun('group1@g.us', runId!, 'reviewer'), + ).toBe(true); + expect( + queue.getDirectTerminalDeliveryForRun('group1@g.us', runId!, 'reviewer'), + ).toBe('DONE_WITH_CONCERNS\n리뷰 완료'); + + releaseRun(true); + await vi.advanceTimersByTimeAsync(10); + + expect( + queue.hasDirectTerminalDeliveryForRun('group1@g.us', runId!, 'reviewer'), + ).toBe(false); + expect( + queue.getDirectTerminalDeliveryForRun('group1@g.us', runId!, 'reviewer'), + ).toBeNull(); + }); + it('force-terminates a lingering process after output was delivered', async () => { class StubbornProcess extends EventEmitter { exitCode: number | null = null; diff --git a/src/group-queue.ts b/src/group-queue.ts index 81b8d39..6a441e0 100644 --- a/src/group-queue.ts +++ b/src/group-queue.ts @@ -48,7 +48,7 @@ interface GroupState { runPhase: RunPhase; runningTaskId: string | null; currentRunId: string | null; - directTerminalDeliveryRoles: Set; + directTerminalDeliveries: Map; pendingMessages: boolean; pendingTasks: QueuedTask[]; process: ChildProcess | null; @@ -114,7 +114,7 @@ function resetRunState(state: GroupState, groupJid: string): void { state.process = null; state.processName = null; state.ipcDir = null; - state.directTerminalDeliveryRoles.clear(); + state.directTerminalDeliveries.clear(); transitionRunPhase(state, groupJid, 'idle'); } @@ -199,7 +199,7 @@ export class GroupQueue { runPhase: 'idle', runningTaskId: null, currentRunId: null, - directTerminalDeliveryRoles: new Set(), + directTerminalDeliveries: new Map(), pendingMessages: false, pendingTasks: [], process: null, @@ -427,21 +427,25 @@ export class GroupQueue { noteDirectTerminalDelivery( groupJid: string, senderRole?: string | null, + text?: string | null, ): void { const state = this.getGroup(groupJid); if ( state.runPhase !== 'running_messages' || !state.currentRunId || - !senderRole + !senderRole || + !text || + text.length === 0 ) { return; } - state.directTerminalDeliveryRoles.add(senderRole); + state.directTerminalDeliveries.set(senderRole, text); logger.info( { groupJid, runId: state.currentRunId, senderRole, + textLength: text.length, }, 'Recorded direct terminal delivery for active run', ); @@ -456,7 +460,19 @@ export class GroupQueue { if (state.currentRunId !== runId || !senderRole) { return false; } - return state.directTerminalDeliveryRoles.has(senderRole); + return state.directTerminalDeliveries.has(senderRole); + } + + getDirectTerminalDeliveryForRun( + groupJid: string, + runId: string, + senderRole?: string | null, + ): string | null { + const state = this.getGroup(groupJid); + if (state.currentRunId !== runId || !senderRole) { + return null; + } + return state.directTerminalDeliveries.get(senderRole) ?? null; } private clearPostCloseTimers(state: GroupState): void { diff --git a/src/index.ts b/src/index.ts index b73b2f7..a238e7c 100644 --- a/src/index.ts +++ b/src/index.ts @@ -472,7 +472,7 @@ async function main(): Promise { (senderRole === 'reviewer' || senderRole === 'arbiter') && isTerminalStatusMessage(text) ) { - queue.noteDirectTerminalDelivery(jid, senderRole); + queue.noteDirectTerminalDelivery(jid, senderRole, text); } }, nudgeScheduler: nudgeSchedulerLoop, diff --git a/src/message-agent-executor-paired.ts b/src/message-agent-executor-paired.ts index aa4a0f1..40cdcbd 100644 --- a/src/message-agent-executor-paired.ts +++ b/src/message-agent-executor-paired.ts @@ -14,7 +14,6 @@ import { completePairedExecutionContext, type PreparedPairedExecutionContext, } from './paired-execution-context.js'; -import { hasRecognizedPairedTerminalSummary } from './paired-execution-context-shared.js'; import { resolvePairedFollowUpQueueAction } from './message-agent-executor-rules.js'; import { enqueuePairedFollowUpAfterEvent } from './message-runtime-follow-up.js'; import type { PairedTurnIdentity } from './paired-turn-identity.js'; @@ -45,6 +44,7 @@ export function createPairedExecutionLifecycle(args: { chatJid: string; runId: string; enqueueMessageCheck: () => void; + getDirectTerminalDeliveryText?: () => string | null; onOutput?: (output: AgentOutput) => Promise; log: ExecutorLog; }): PairedExecutionLifecycle { @@ -55,20 +55,25 @@ export function createPairedExecutionLifecycle(args: { chatJid, runId, enqueueMessageCheck, + getDirectTerminalDeliveryText, onOutput, log, } = args; let pairedExecutionStatus: 'succeeded' | 'failed' = 'failed'; let pairedExecutionSummary: string | null = null; - let pairedTerminalSummary: string | null = null; - let pairedTerminalOutput: string | null = null; + let pairedFinalOutput: string | null = null; + let pairedSummaryLocked = false; let pairedExecutionCompleted = false; let pairedExecutionDelegated = false; let pairedSawOutput = false; let pairedTurnOutputPersisted = false; let pairedTurnStateFinalized = false; let leaseHeartbeatTimer: ReturnType | null = null; + const requiresVisibleVerdict = + pairedExecutionContext?.requiresVisibleVerdict === true; + const missingVisibleVerdictSummary = + 'Execution completed without a visible terminal verdict.'; const finalizePairedTurnState = ( status: 'succeeded' | 'failed', @@ -133,20 +138,12 @@ export function createPairedExecutionLifecycle(args: { leaseHeartbeatTimer.unref?.(); } - const rememberTerminalOutputIfRecognized = (outputText: string) => { - if (!hasRecognizedPairedTerminalSummary(completedRole, outputText)) { - return; - } - pairedTerminalSummary = outputText.slice(0, 500); - pairedTerminalOutput = outputText; - }; - const persistPairedTurnOutputIfNeeded = () => { if ( !pairedExecutionContext || pairedTurnOutputPersisted || - !pairedTerminalOutput || - pairedTerminalOutput.length === 0 + !pairedFinalOutput || + pairedFinalOutput.length === 0 ) { return; } @@ -156,7 +153,7 @@ export function createPairedExecutionLifecycle(args: { pairedExecutionContext.task.id, turnNumber, completedRole, - pairedTerminalOutput, + pairedFinalOutput, ); pairedTurnOutputPersisted = true; }; @@ -166,8 +163,8 @@ export function createPairedExecutionLifecycle(args: { completedRole !== 'owner' || !pairedExecutionContext || pairedExecutionCompleted || - !pairedTerminalOutput || - pairedTerminalOutput.length === 0 + !pairedFinalOutput || + pairedFinalOutput.length === 0 ) { return; } @@ -181,16 +178,55 @@ export function createPairedExecutionLifecycle(args: { role: completedRole, status: 'succeeded', runId, - summary: pairedTerminalSummary ?? pairedExecutionSummary, + summary: pairedExecutionSummary, }); pairedExecutionCompleted = true; }; + const lockVisibleVerdict = (outputText: string) => { + if (outputText.length === 0) { + return; + } + if (!pairedFinalOutput || pairedFinalOutput.length === 0) { + pairedFinalOutput = outputText; + } + if (!pairedSummaryLocked) { + pairedExecutionSummary = outputText.slice(0, 500); + pairedSummaryLocked = true; + } + pairedSawOutput = true; + }; + + const adoptDirectTerminalDeliveryIfNeeded = () => { + const outputText = getDirectTerminalDeliveryText?.(); + if (!outputText || outputText.length === 0) { + return null; + } + if (!pairedFinalOutput || pairedFinalOutput.length === 0) { + lockVisibleVerdict(outputText); + log.info( + { + pairedTaskId: pairedExecutionContext?.task.id ?? null, + role: completedRole, + runId, + }, + 'Adopted direct terminal delivery as paired final output', + ); + } else if (!pairedSummaryLocked) { + pairedExecutionSummary = pairedFinalOutput.slice(0, 500); + pairedSummaryLocked = true; + } + return outputText; + }; + return { updateSummary({ outputText, errorText }) { + if (pairedSummaryLocked) { + return; + } + if (outputText && outputText.length > 0) { pairedExecutionSummary = outputText.slice(0, 500); - rememberTerminalOutputIfRecognized(outputText); return; } @@ -200,7 +236,7 @@ export function createPairedExecutionLifecycle(args: { }, recordFinalOutputBeforeDelivery(outputText) { - rememberTerminalOutputIfRecognized(outputText); + lockVisibleVerdict(outputText); completeSuccessfulOwnerTurnBeforeDeliveryIfNeeded(); persistPairedTurnOutputIfNeeded(); }, @@ -265,29 +301,33 @@ export function createPairedExecutionLifecycle(args: { return; } - const missingReviewerOrArbiterVerdict = - (completedRole === 'reviewer' || completedRole === 'arbiter') && - pairedExecutionStatus === 'succeeded' && - !pairedTerminalSummary; + const directTerminalOutput = adoptDirectTerminalDeliveryIfNeeded(); + + const missingVisibleVerdict = + requiresVisibleVerdict && + (!pairedFinalOutput || pairedFinalOutput.length === 0); + if (missingVisibleVerdict) { + pairedExecutionSummary = missingVisibleVerdictSummary; + log.warn( + { + pairedTaskId: pairedExecutionContext?.task.id ?? null, + role: completedRole, + runId, + }, + 'Treating paired execution as failed because it ended without a visible terminal verdict', + ); + } const effectiveStatus = completedRole === 'owner' && pairedExecutionStatus === 'succeeded' && !pairedSawOutput ? 'failed' - : missingReviewerOrArbiterVerdict + : missingVisibleVerdict && pairedExecutionStatus === 'succeeded' ? 'failed' : pairedExecutionStatus; - - if (missingReviewerOrArbiterVerdict) { - log.warn( - { - pairedTaskId: pairedExecutionContext?.task.id ?? null, - role: completedRole, - summary: pairedExecutionSummary, - }, - 'Downgraded paired reviewer/arbiter completion to failed because no explicit terminal verdict was emitted', - ); - } + const sawOutputForFollowUp = missingVisibleVerdict + ? false + : pairedSawOutput; if (pairedExecutionContext && !pairedExecutionCompleted) { if (effectiveStatus === 'succeeded') { @@ -306,15 +346,15 @@ export function createPairedExecutionLifecycle(args: { role: completedRole, status: effectiveStatus, runId, - summary: - effectiveStatus === 'succeeded' - ? (pairedTerminalSummary ?? pairedExecutionSummary) - : (pairedExecutionSummary ?? pairedTerminalSummary), + summary: pairedExecutionSummary, }); pairedExecutionCompleted = true; } - finalizePairedTurnState(effectiveStatus); + finalizePairedTurnState( + effectiveStatus, + effectiveStatus === 'failed' ? pairedExecutionSummary : null, + ); if (!pairedExecutionContext) { return; @@ -342,12 +382,16 @@ export function createPairedExecutionLifecycle(args: { } } - const queueAction = resolvePairedFollowUpQueueAction({ - completedRole, - executionStatus: effectiveStatus, - sawOutput: pairedSawOutput, - taskStatus: finishedTask?.status ?? null, - }); + const queueAction = + directTerminalOutput && + (completedRole === 'reviewer' || completedRole === 'arbiter') + ? 'none' + : resolvePairedFollowUpQueueAction({ + completedRole, + executionStatus: effectiveStatus, + sawOutput: sawOutputForFollowUp, + taskStatus: finishedTask?.status ?? null, + }); if (queueAction !== 'pending' || !finishedTask) { return; } @@ -359,8 +403,8 @@ export function createPairedExecutionLifecycle(args: { source: 'executor-recovery', completedRole, executionStatus: effectiveStatus, - sawOutput: pairedSawOutput, - fallbackLastTurnOutputRole: pairedSawOutput ? completedRole : null, + sawOutput: sawOutputForFollowUp, + fallbackLastTurnOutputRole: sawOutputForFollowUp ? completedRole : null, enqueueMessageCheck, }); if (followUpResult.kind !== 'paired-follow-up') { diff --git a/src/message-agent-executor.test.ts b/src/message-agent-executor.test.ts index 7e3041f..b5da7ef 100644 --- a/src/message-agent-executor.test.ts +++ b/src/message-agent-executor.test.ts @@ -981,6 +981,250 @@ describe('runAgentForGroup room memory', () => { ]); }); + it('fails paired reviewer turns that finish without a visible terminal verdict', async () => { + const group = { + ...makeGroup(), + folder: 'test-group', + workDir: '/repo', + agentType: 'codex' as const, + }; + vi.mocked(serviceRouting.getEffectiveChannelLease).mockReturnValue({ + chat_jid: 'group@test', + owner_agent_type: 'claude-code', + reviewer_agent_type: 'codex', + arbiter_agent_type: null, + owner_service_id: 'claude', + reviewer_service_id: 'codex-review', + arbiter_service_id: null, + activated_at: null, + reason: null, + explicit: false, + }); + vi.mocked( + pairedExecutionContext.preparePairedExecutionContext, + ).mockReturnValue({ + task: { + id: 'paired-task-review-no-verdict', + chat_jid: 'group@test', + group_folder: 'test-group', + owner_service_id: 'claude', + reviewer_service_id: 'codex-review', + title: null, + source_ref: 'HEAD', + review_requested_at: '2026-04-10T00:00:00.000Z', + plan_notes: null, + round_trip_count: 0, + status: 'in_review', + arbiter_verdict: null, + arbiter_requested_at: null, + completion_reason: null, + created_at: '2026-04-10T00:00:00.000Z', + updated_at: '2026-04-10T00:00:00.000Z', + }, + claimedTaskUpdatedAt: '2026-04-10T00:00:00.000Z', + workspace: null, + envOverrides: {}, + requiresVisibleVerdict: true, + }); + vi.mocked(agentRunner.runAgentProcess).mockImplementation( + async (_group, _input, _onProcess, onOutput) => { + await onOutput?.({ + status: 'success', + phase: 'progress', + result: '검토 중입니다.', + output: { visibility: 'public', text: '검토 중입니다.' }, + } as any); + return { + status: 'success', + result: null, + }; + }, + ); + + const result = await runAgentForGroup(makeDeps(), { + group, + prompt: 'hello', + chatJid: 'group@test', + runId: 'run-review-no-verdict', + forcedRole: 'reviewer', + pairedTurnIdentity: { + turnId: + 'paired-task-review-no-verdict:2026-04-10T00:00:00.000Z:reviewer-turn', + taskId: 'paired-task-review-no-verdict', + taskUpdatedAt: '2026-04-10T00:00:00.000Z', + intentKind: 'reviewer-turn', + role: 'reviewer', + }, + }); + + expect(result).toBe('error'); + expect( + pairedExecutionContext.completePairedExecutionContext, + ).toHaveBeenCalledWith( + expect.objectContaining({ + taskId: 'paired-task-review-no-verdict', + role: 'reviewer', + status: 'failed', + summary: 'Execution completed without a visible terminal verdict.', + }), + ); + expect(db.failPairedTurn).toHaveBeenCalledWith({ + turnIdentity: { + turnId: + 'paired-task-review-no-verdict:2026-04-10T00:00:00.000Z:reviewer-turn', + taskId: 'paired-task-review-no-verdict', + taskUpdatedAt: '2026-04-10T00:00:00.000Z', + intentKind: 'reviewer-turn', + role: 'reviewer', + }, + error: 'Execution completed without a visible terminal verdict.', + }); + expect(db.completePairedTurn).not.toHaveBeenCalled(); + expect(db.insertPairedTurnOutput).not.toHaveBeenCalled(); + }); + + it('adopts a direct terminal reviewer delivery as the paired final output and avoids requeue', async () => { + const group = { + ...makeGroup(), + folder: 'test-group', + workDir: '/repo', + agentType: 'codex' as const, + }; + const deps = { + ...makeDeps(), + queue: { + registerProcess: vi.fn(), + enqueueMessageCheck: vi.fn(), + getDirectTerminalDeliveryForRun: vi.fn( + (groupJid: string, runId: string, senderRole?: string | null) => + groupJid === 'group@test' && + runId === 'run-review-direct-terminal' && + senderRole === 'reviewer' + ? 'DONE_WITH_CONCERNS\n핵심 concern이 있습니다.' + : null, + ), + }, + }; + + vi.mocked(serviceRouting.getEffectiveChannelLease).mockReturnValue({ + chat_jid: 'group@test', + owner_agent_type: 'claude-code', + reviewer_agent_type: 'codex', + arbiter_agent_type: null, + owner_service_id: 'claude', + reviewer_service_id: 'codex-review', + arbiter_service_id: null, + activated_at: null, + reason: null, + explicit: false, + }); + vi.mocked( + pairedExecutionContext.preparePairedExecutionContext, + ).mockReturnValue({ + task: { + id: 'paired-task-review-direct-terminal', + chat_jid: 'group@test', + group_folder: 'test-group', + owner_service_id: 'claude', + reviewer_service_id: 'codex-review', + title: null, + source_ref: 'HEAD', + review_requested_at: '2026-04-10T00:00:00.000Z', + plan_notes: null, + round_trip_count: 1, + status: 'in_review', + arbiter_verdict: null, + arbiter_requested_at: null, + completion_reason: null, + created_at: '2026-04-10T00:00:00.000Z', + updated_at: '2026-04-10T00:00:00.000Z', + }, + claimedTaskUpdatedAt: '2026-04-10T00:00:00.000Z', + workspace: null, + envOverrides: {}, + requiresVisibleVerdict: true, + }); + vi.mocked(db.getPairedTaskById).mockReturnValue({ + id: 'paired-task-review-direct-terminal', + chat_jid: 'group@test', + group_folder: 'test-group', + owner_service_id: 'claude', + reviewer_service_id: 'codex-review', + title: null, + source_ref: 'HEAD', + plan_notes: null, + round_trip_count: 1, + review_requested_at: '2026-04-10T00:00:00.000Z', + status: 'review_ready', + arbiter_verdict: null, + arbiter_requested_at: null, + completion_reason: null, + created_at: '2026-04-10T00:00:00.000Z', + updated_at: '2026-04-10T00:00:00.000Z', + }); + vi.mocked(agentRunner.runAgentProcess).mockImplementation( + async (_group, _input, _onProcess, onOutput) => { + await onOutput?.({ + status: 'success', + phase: 'progress', + result: '리뷰 완료했습니다. 핵심 concern을 정리 중입니다.', + output: { + visibility: 'public', + text: '리뷰 완료했습니다. 핵심 concern을 정리 중입니다.', + }, + } as any); + return { + status: 'success', + result: null, + }; + }, + ); + + const result = await runAgentForGroup(deps, { + group, + prompt: 'please review', + chatJid: 'group@test', + runId: 'run-review-direct-terminal', + forcedRole: 'reviewer', + pairedTurnIdentity: { + turnId: + 'paired-task-review-direct-terminal:2026-04-10T00:00:00.000Z:reviewer-turn', + taskId: 'paired-task-review-direct-terminal', + taskUpdatedAt: '2026-04-10T00:00:00.000Z', + intentKind: 'reviewer-turn', + role: 'reviewer', + }, + onOutput: async () => {}, + }); + + expect(result).toBe('success'); + expect( + pairedExecutionContext.completePairedExecutionContext, + ).toHaveBeenCalledWith( + expect.objectContaining({ + taskId: 'paired-task-review-direct-terminal', + role: 'reviewer', + status: 'succeeded', + summary: 'DONE_WITH_CONCERNS\n핵심 concern이 있습니다.', + }), + ); + expect(db.insertPairedTurnOutput).toHaveBeenCalledWith( + 'paired-task-review-direct-terminal', + 1, + 'reviewer', + 'DONE_WITH_CONCERNS\n핵심 concern이 있습니다.', + ); + expect(db.completePairedTurn).toHaveBeenCalledWith({ + turnId: + 'paired-task-review-direct-terminal:2026-04-10T00:00:00.000Z:reviewer-turn', + taskId: 'paired-task-review-direct-terminal', + taskUpdatedAt: '2026-04-10T00:00:00.000Z', + intentKind: 'reviewer-turn', + role: 'reviewer', + }); + expect(db.failPairedTurn).not.toHaveBeenCalled(); + expect(deps.queue.enqueueMessageCheck).not.toHaveBeenCalled(); + }); it('passes paired workspace env overrides into the runner when execution metadata exists', async () => { const group = { ...makeGroup(), @@ -1266,6 +1510,7 @@ describe('runAgentForGroup room memory', () => { }, workspace: null, envOverrides: {}, + requiresVisibleVerdict: true, }); vi.mocked(agentRunner.runAgentProcess).mockImplementation( async (_group, _input, _onProcess, onOutput) => { @@ -1322,9 +1567,7 @@ describe('runAgentForGroup room memory', () => { taskId: 'paired-task-reviewer-progress-only', role: 'reviewer', status: 'failed', - summary: expect.stringContaining( - '우선 변경 파일과 현재 계약 노출 지점을', - ), + summary: 'Execution completed without a visible terminal verdict.', }), ); }); diff --git a/src/message-agent-executor.ts b/src/message-agent-executor.ts index 7626324..7bae286 100644 --- a/src/message-agent-executor.ts +++ b/src/message-agent-executor.ts @@ -68,7 +68,8 @@ import type { AgentType, PairedRoomRole, RegisteredGroup } from './types.js'; export interface MessageAgentExecutorDeps { assistantName: string; - queue: Pick; + queue: Pick & + Partial>; getRegisteredGroups: () => Record; getSessions: () => Record; persistSession: (groupFolder: string, sessionId: string) => void; @@ -402,9 +403,21 @@ export async function runAgentForGroup( chatJid, runId, enqueueMessageCheck: () => deps.queue.enqueueMessageCheck(chatJid), + getDirectTerminalDeliveryText: () => + deps.queue.getDirectTerminalDeliveryForRun?.( + chatJid, + runId, + runtimePairedTurnIdentity?.role ?? activeRole, + ) ?? null, onOutput, log, }); + const hasDirectTerminalDelivery = (): boolean => + !!deps.queue.getDirectTerminalDeliveryForRun?.( + chatJid, + runId, + runtimePairedTurnIdentity?.role ?? activeRole, + ); const maybeHandoffToCodex = ( reason: AgentTriggerReason, @@ -917,7 +930,11 @@ export async function runAgentForGroup( }); } - if (!attempt.sawOutput && output.status !== 'error') { + if ( + !attempt.sawOutput && + !hasDirectTerminalDelivery() && + output.status !== 'error' + ) { const claudeRetryResult = await retryClaudeAttemptIfNeeded(attempt); if (claudeRetryResult) { return claudeRetryResult; @@ -973,7 +990,11 @@ export async function runAgentForGroup( // success-null-result with no visible output — agent returned nothing useful. // But if output was already delivered to Discord (sawOutput), treat as success. - if (attempt.sawSuccessNullResultWithoutOutput && !attempt.sawOutput) { + if ( + attempt.sawSuccessNullResultWithoutOutput && + !attempt.sawOutput && + !hasDirectTerminalDelivery() + ) { log.error( 'Agent returned success with null result and no visible output', ); @@ -981,7 +1002,9 @@ export async function runAgentForGroup( } pairedExecutionLifecycle.markStatus('succeeded'); - pairedExecutionLifecycle.markSawOutput(attempt.sawOutput); + pairedExecutionLifecycle.markSawOutput( + attempt.sawOutput || hasDirectTerminalDelivery(), + ); return 'success'; };