diff --git a/prompts/arbiter-paired-room.md b/prompts/arbiter-paired-room.md index 9811ddc..43a2f71 100644 --- a/prompts/arbiter-paired-room.md +++ b/prompts/arbiter-paired-room.md @@ -34,6 +34,7 @@ You may receive reference opinions from external models appended to your prompt. ## Rules - Base your verdict on evidence (code, test output, logs), not on who said what first +- When reading owner/reviewer summaries, treat **TASK_DONE** as full task completion, **STEP_DONE** as intermediate progress that should keep the owner flow alive, and **DONE** as a legacy alias for **TASK_DONE** - Distinguish reviewer snapshot limits from real product bugs. Reviewer workspaces may intentionally omit heavy artifacts like `node_modules`, `dist`, and `build`; inability to run direct local test/typecheck/build/lint there is not, by itself, a blocker if dedicated verification evidence exists - When verification evidence exists from the dedicated verification path, judge that evidence on its merits instead of requiring the reviewer to reproduce the same result from the lightweight reviewer snapshot - Your verdict is final for this deadlock cycle — after it, work resumes normally diff --git a/prompts/claude-paired-room.md b/prompts/claude-paired-room.md index 1d422fd..96d2cae 100644 --- a/prompts/claude-paired-room.md +++ b/prompts/claude-paired-room.md @@ -22,9 +22,11 @@ If you see a materially better design, debugging path, or scoping choice, propos ## Completion status -**Start your first line** with one of these four statuses. This is required. +**Start your first line** with one of these six statuses. This is required. -- **DONE** — Approved. The owner's response is correct and complete. Include the evidence +- **STEP_DONE** — The current step is acceptable, but the original requested task still has remaining work. Send the task back to the owner without escalating to the arbiter +- **TASK_DONE** — Approved. The owner's work satisfies the full requested task. Include the evidence +- **DONE** — Legacy alias for **TASK_DONE**. Prefer **TASK_DONE** for new turns - **DONE_WITH_CONCERNS** — Approved with concerns. List specific actions the owner must take. If the same concerns repeat for 2+ turns, escalate to BLOCKED - **BLOCKED** — Cannot proceed without user decision - **NEEDS_CONTEXT** — Missing information from user diff --git a/prompts/owner-common-paired-room.md b/prompts/owner-common-paired-room.md index 0368cb3..50a9365 100644 --- a/prompts/owner-common-paired-room.md +++ b/prompts/owner-common-paired-room.md @@ -18,16 +18,19 @@ Challenge the reviewer's reasoning. Point out logical gaps, over-engineering, sc ## Completion status -**Start your first line** with one of these four statuses. This is required. +**Start your first line** with one of these six statuses. This is required. -- **DONE** — All steps completed. Include the evidence (test output, build log, diff) +- **STEP_DONE** — A meaningful intermediate step is complete, but the original task still has remaining work. This keeps the task active and continues the owner flow without reviewer or arbiter intervention +- **TASK_DONE** — The original requested task is complete. Include the evidence (test output, build log, diff) +- **DONE** — Legacy alias for **TASK_DONE**. Prefer **TASK_DONE** for new turns - **DONE_WITH_CONCERNS** — Completed, but there are issues worth flagging. If the reviewer raises the same concerns again, fix them or escalate to BLOCKED - **BLOCKED** — Cannot proceed. State what is stopping you - **NEEDS_CONTEXT** — Missing information needed to continue ### Finalize semantics -- When the reviewer already approved and you are finalizing, **DONE** closes the paired turn +- When the reviewer already approved and you are finalizing, **TASK_DONE** closes the paired turn +- In that same finalize step, **STEP_DONE** keeps the task active and resumes the owner flow because the original request still has remaining work - In that same finalize step, **DONE_WITH_CONCERNS** does not close the turn — it intentionally reopens review - Use **DONE_WITH_CONCERNS** on finalize only when you are explicitly asking the reviewer loop to resume diff --git a/src/message-agent-executor-paired.ts b/src/message-agent-executor-paired.ts index 349bea2..7cbf0ea 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 { parseVisibleVerdict } 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'; @@ -438,6 +439,7 @@ export function createPairedExecutionLifecycle(args: { executionStatus: effectiveStatus, sawOutput: sawOutputForFollowUp, taskStatus: finishedTask?.status ?? null, + outputSummary: pairedExecutionSummary, }); if (queueAction !== 'pending' || !finishedTask) { return; @@ -452,6 +454,10 @@ export function createPairedExecutionLifecycle(args: { executionStatus: effectiveStatus, sawOutput: sawOutputForFollowUp, fallbackLastTurnOutputRole: sawOutputForFollowUp ? completedRole : null, + fallbackLastTurnOutputVerdict: + sawOutputForFollowUp && pairedExecutionSummary + ? parseVisibleVerdict(pairedExecutionSummary) + : null, enqueueMessageCheck, }); if (followUpResult.kind !== 'paired-follow-up') { diff --git a/src/message-agent-executor-rules.test.ts b/src/message-agent-executor-rules.test.ts index 74583ad..6fe983f 100644 --- a/src/message-agent-executor-rules.test.ts +++ b/src/message-agent-executor-rules.test.ts @@ -216,6 +216,18 @@ describe('message-agent-executor-rules', () => { ).toBe('none'); }); + it('does not request a duplicate executor-side follow-up after successful owner STEP_DONE output', () => { + expect( + resolvePairedFollowUpQueueAction({ + completedRole: 'owner', + executionStatus: 'succeeded', + sawOutput: true, + taskStatus: 'active', + outputSummary: 'STEP_DONE\nkeep going', + }), + ).toBe('none'); + }); + it('returns none after successful output when no next-turn action is needed', () => { expect( resolvePairedFollowUpQueueAction({ diff --git a/src/message-agent-executor-rules.ts b/src/message-agent-executor-rules.ts index 4fab503..134d6f5 100644 --- a/src/message-agent-executor-rules.ts +++ b/src/message-agent-executor-rules.ts @@ -2,6 +2,7 @@ import { resolveFollowUpDispatch, resolveNextTurnAction, } from './message-runtime-rules.js'; +import { parseVisibleVerdict } from './paired-execution-context-shared.js'; import type { PairedRoomRole, PairedTaskStatus } from './types.js'; export { isRetryableClaudeSessionFailureAttempt, @@ -20,10 +21,15 @@ export function resolvePairedFollowUpQueueAction(args: { executionStatus: 'succeeded' | 'failed'; sawOutput: boolean; taskStatus: PairedTaskStatus | null; + outputSummary?: string | null; }): PairedFollowUpQueueAction { const nextTurnAction = resolveNextTurnAction({ taskStatus: args.taskStatus, lastTurnOutputRole: args.sawOutput ? args.completedRole : null, + lastTurnOutputVerdict: + args.sawOutput && args.outputSummary + ? parseVisibleVerdict(args.outputSummary) + : null, }); const dispatch = resolveFollowUpDispatch({ source: 'executor-recovery', diff --git a/src/message-runtime-flow.ts b/src/message-runtime-flow.ts index 8058d63..5d4f46d 100644 --- a/src/message-runtime-flow.ts +++ b/src/message-runtime-flow.ts @@ -29,6 +29,7 @@ import { type ScheduledPairedFollowUpIntentKind, } from './paired-follow-up-scheduler.js'; import { hasReviewerLease } from './service-routing.js'; +import { parseVisibleVerdict } from './paired-execution-context-shared.js'; import type { Channel, NewMessage, @@ -127,6 +128,9 @@ export function buildPendingPairedTurn(args: { const nextTurnAction = resolveNextTurnAction({ taskStatus, lastTurnOutputRole: lastTurnOutput?.role ?? null, + lastTurnOutputVerdict: lastTurnOutput?.output_text + ? parseVisibleVerdict(lastTurnOutput.output_text) + : null, }); const recentMessages = getRecentChatMessages(chatJid, 20); const lastHumanMessage = getLastHumanMessageContent(chatJid); diff --git a/src/message-runtime-follow-up.test.ts b/src/message-runtime-follow-up.test.ts index 8eae6f7..4bf4dfc 100644 --- a/src/message-runtime-follow-up.test.ts +++ b/src/message-runtime-follow-up.test.ts @@ -83,6 +83,35 @@ describe('message-runtime-follow-up', () => { expect(enqueue).toHaveBeenCalledTimes(1); }); + it('uses the fallback STEP_DONE verdict to schedule owner follow-ups when the owner keeps the task active', () => { + const enqueue = vi.fn(); + const result = dispatchPairedFollowUpForEvent({ + chatJid: 'group@test', + runId: 'run-owner-step-done', + task: { + id: 'task-owner-step-done', + status: 'active', + round_trip_count: 1, + updated_at: '2026-03-30T00:00:00.000Z', + }, + source: 'owner-delivery-success', + completedRole: 'owner', + fallbackLastTurnOutputRole: 'owner', + fallbackLastTurnOutputVerdict: 'step_done', + enqueue, + }); + + expect(result).toMatchObject({ + kind: 'paired-follow-up', + intentKind: 'owner-follow-up', + scheduled: true, + taskStatus: 'active', + lastTurnOutputRole: 'owner', + lastTurnOutputVerdict: 'step_done', + }); + expect(enqueue).toHaveBeenCalledTimes(1); + }); + it('prefers the latest persisted turn output over the fallback delivery role when both are present', () => { const enqueue = vi.fn(); vi.mocked(getPairedTurnOutputs).mockReturnValue([ diff --git a/src/message-runtime-follow-up.ts b/src/message-runtime-follow-up.ts index e774021..97725eb 100644 --- a/src/message-runtime-follow-up.ts +++ b/src/message-runtime-follow-up.ts @@ -1,4 +1,8 @@ import { getPairedTurnOutputs } from './db.js'; +import { + parseVisibleVerdict, + type VisibleVerdict, +} from './paired-execution-context-shared.js'; import { matchesExpectedPairedFollowUpIntent, resolveFollowUpDispatch, @@ -20,6 +24,7 @@ export interface PairedFollowUpDecision { taskId: string | null; taskStatus: PairedTaskStatus | null; lastTurnOutputRole: PairedRoomRole | null; + lastTurnOutputVerdict: VisibleVerdict | null; nextTurnAction: NextTurnAction; dispatch: FollowUpDispatch; } @@ -37,15 +42,23 @@ export type PairedFollowUpDispatchResult = scheduled: boolean; }); -export function resolveLatestPairedTurnOutputRole(args: { +export function resolveLatestPairedTurnOutputContext(args: { task: Pick | null | undefined; fallbackLastTurnOutputRole?: PairedRoomRole | null; -}): PairedRoomRole | null { - return ( - (args.task ? getPairedTurnOutputs(args.task.id).at(-1)?.role : null) ?? - args.fallbackLastTurnOutputRole ?? - null - ); + fallbackLastTurnOutputVerdict?: VisibleVerdict | null; +}): { + role: PairedRoomRole | null; + verdict: VisibleVerdict | null; +} { + const latestOutput = args.task + ? getPairedTurnOutputs(args.task.id).at(-1) + : null; + return { + role: latestOutput?.role ?? args.fallbackLastTurnOutputRole ?? null, + verdict: latestOutput?.output_text + ? parseVisibleVerdict(latestOutput.output_text) + : (args.fallbackLastTurnOutputVerdict ?? null), + }; } export function resolvePairedFollowUpDecision(args: { @@ -55,14 +68,18 @@ export function resolvePairedFollowUpDecision(args: { executionStatus?: 'succeeded' | 'failed'; sawOutput?: boolean; fallbackLastTurnOutputRole?: PairedRoomRole | null; + fallbackLastTurnOutputVerdict?: VisibleVerdict | null; }): PairedFollowUpDecision { - const lastTurnOutputRole = resolveLatestPairedTurnOutputRole({ - task: args.task, - fallbackLastTurnOutputRole: args.fallbackLastTurnOutputRole, - }); + const { role: lastTurnOutputRole, verdict: lastTurnOutputVerdict } = + resolveLatestPairedTurnOutputContext({ + task: args.task, + fallbackLastTurnOutputRole: args.fallbackLastTurnOutputRole, + fallbackLastTurnOutputVerdict: args.fallbackLastTurnOutputVerdict, + }); const nextTurnAction = resolveNextTurnAction({ taskStatus: args.task?.status ?? null, lastTurnOutputRole, + lastTurnOutputVerdict, }); const dispatch = resolveFollowUpDispatch({ source: args.source, @@ -76,6 +93,7 @@ export function resolvePairedFollowUpDecision(args: { taskId: args.task?.id ?? null, taskStatus: args.task?.status ?? null, lastTurnOutputRole, + lastTurnOutputVerdict, nextTurnAction, dispatch, }; @@ -91,23 +109,31 @@ export function schedulePairedFollowUpIntent(args: { intentKind: ScheduledPairedFollowUpIntentKind; enqueue: () => void; fallbackLastTurnOutputRole?: PairedRoomRole | null; + fallbackLastTurnOutputVerdict?: VisibleVerdict | null; lastTurnOutputRole?: PairedRoomRole | null; + lastTurnOutputVerdict?: VisibleVerdict | null; }): boolean { if (!args.task) { return false; } - const lastTurnOutputRole = - args.lastTurnOutputRole ?? - resolveLatestPairedTurnOutputRole({ - task: args.task, - fallbackLastTurnOutputRole: args.fallbackLastTurnOutputRole, - }); + const latestOutputContext = + args.lastTurnOutputRole != null || args.lastTurnOutputVerdict != null + ? { + role: args.lastTurnOutputRole ?? null, + verdict: args.lastTurnOutputVerdict ?? null, + } + : resolveLatestPairedTurnOutputContext({ + task: args.task, + fallbackLastTurnOutputRole: args.fallbackLastTurnOutputRole, + fallbackLastTurnOutputVerdict: args.fallbackLastTurnOutputVerdict, + }); if ( !matchesExpectedPairedFollowUpIntent({ taskStatus: args.task.status, - lastTurnOutputRole, + lastTurnOutputRole: latestOutputContext.role, + lastTurnOutputVerdict: latestOutputContext.verdict, intentKind: args.intentKind, }) ) { @@ -133,7 +159,9 @@ export function schedulePairedFollowUpWithMessageCheck(args: { intentKind: ScheduledPairedFollowUpIntentKind; enqueueMessageCheck: () => void; fallbackLastTurnOutputRole?: PairedRoomRole | null; + fallbackLastTurnOutputVerdict?: VisibleVerdict | null; lastTurnOutputRole?: PairedRoomRole | null; + lastTurnOutputVerdict?: VisibleVerdict | null; }): boolean { return schedulePairedFollowUpIntent({ chatJid: args.chatJid, @@ -142,7 +170,9 @@ export function schedulePairedFollowUpWithMessageCheck(args: { intentKind: args.intentKind, enqueue: args.enqueueMessageCheck, fallbackLastTurnOutputRole: args.fallbackLastTurnOutputRole, + fallbackLastTurnOutputVerdict: args.fallbackLastTurnOutputVerdict, lastTurnOutputRole: args.lastTurnOutputRole, + lastTurnOutputVerdict: args.lastTurnOutputVerdict, }); } @@ -158,6 +188,7 @@ export function dispatchPairedFollowUpForEvent(args: { executionStatus?: 'succeeded' | 'failed'; sawOutput?: boolean; fallbackLastTurnOutputRole?: PairedRoomRole | null; + fallbackLastTurnOutputVerdict?: VisibleVerdict | null; enqueue: () => void; enqueueMessageCheck?: () => void; }): PairedFollowUpDispatchResult { @@ -168,6 +199,7 @@ export function dispatchPairedFollowUpForEvent(args: { executionStatus: args.executionStatus, sawOutput: args.sawOutput, fallbackLastTurnOutputRole: args.fallbackLastTurnOutputRole, + fallbackLastTurnOutputVerdict: args.fallbackLastTurnOutputVerdict, }); if ( @@ -193,6 +225,7 @@ export function dispatchPairedFollowUpForEvent(args: { intentKind: decision.nextTurnAction.kind, enqueue: args.enqueue, lastTurnOutputRole: decision.lastTurnOutputRole, + lastTurnOutputVerdict: decision.lastTurnOutputVerdict, }); return { @@ -221,6 +254,7 @@ export function enqueuePairedFollowUpAfterEvent(args: { executionStatus?: 'succeeded' | 'failed'; sawOutput?: boolean; fallbackLastTurnOutputRole?: PairedRoomRole | null; + fallbackLastTurnOutputVerdict?: VisibleVerdict | null; enqueueMessageCheck: () => void; }): PairedFollowUpDispatchResult { return dispatchPairedFollowUpForEvent({ @@ -232,6 +266,7 @@ export function enqueuePairedFollowUpAfterEvent(args: { executionStatus: args.executionStatus, sawOutput: args.sawOutput, fallbackLastTurnOutputRole: args.fallbackLastTurnOutputRole, + fallbackLastTurnOutputVerdict: args.fallbackLastTurnOutputVerdict, enqueue: args.enqueueMessageCheck, enqueueMessageCheck: args.enqueueMessageCheck, }); diff --git a/src/message-runtime-prompts.ts b/src/message-runtime-prompts.ts index 99a6380..263e3c0 100644 --- a/src/message-runtime-prompts.ts +++ b/src/message-runtime-prompts.ts @@ -212,7 +212,8 @@ export function buildFinalizePendingPrompt(args: { ? `\n\nReviewer's final assessment:\n${lastReviewerOutput.output_text.slice(0, 2000)}` : ''; - return `The reviewer approved your work (DONE). Finalize and report the result. -If you intend to close this paired turn now, your first line must be DONE. + return `The reviewer approved the current task scope (TASK_DONE / legacy DONE). Finalize and report the result. +If you intend to close this paired turn now, your first line must be TASK_DONE. +If the original request still has remaining work and the owner flow should continue, your first line may be STEP_DONE. If your first line is DONE_WITH_CONCERNS, the system will reopen review instead of finishing.${reviewerSummary}`; } diff --git a/src/message-runtime-queue.ts b/src/message-runtime-queue.ts index 698b444..1ceaee8 100644 --- a/src/message-runtime-queue.ts +++ b/src/message-runtime-queue.ts @@ -11,6 +11,7 @@ import { isBotOnlyPairedRoomTurn, } from './message-runtime-flow.js'; import { buildPairedTurnIdentity } from './paired-turn-identity.js'; +import { parseVisibleVerdict } from './paired-execution-context-shared.js'; import { advanceLastAgentCursor, resolveActiveRole, @@ -183,6 +184,11 @@ export async function runQueuedGroupTurn(args: { const lastTurnOutputRole = currentTask ? (turnOutputs.at(-1)?.role ?? null) : null; + const lastTurnOutputVerdict = currentTask + ? turnOutputs.at(-1)?.output_text + ? parseVisibleVerdict(turnOutputs.at(-1)?.output_text) + : null + : null; const turnRole = currentTask ? hasHumanMsg ? resolveQueuedTurnRole({ @@ -193,6 +199,7 @@ export async function runQueuedGroupTurn(args: { taskStatus, hasHumanMessage: false, lastTurnOutputRole, + lastTurnOutputVerdict, }) : 'owner'; if (!turnRole) { diff --git a/src/message-runtime-rules.test.ts b/src/message-runtime-rules.test.ts index 7c78c6b..db9b4bd 100644 --- a/src/message-runtime-rules.test.ts +++ b/src/message-runtime-rules.test.ts @@ -113,6 +113,14 @@ describe('message-runtime-rules', () => { intentKind: 'owner-follow-up', }), ).toBe(true); + expect( + matchesExpectedPairedFollowUpIntent({ + taskStatus: 'active', + lastTurnOutputRole: 'owner', + lastTurnOutputVerdict: 'step_done', + intentKind: 'owner-follow-up', + }), + ).toBe(true); }); it('maps active tasks with reviewer output to an owner follow-up', () => { @@ -124,6 +132,16 @@ describe('message-runtime-rules', () => { ).toEqual({ kind: 'owner-follow-up' }); }); + it('maps active tasks with owner STEP_DONE output to an owner follow-up', () => { + expect( + resolveNextTurnAction({ + taskStatus: 'active', + lastTurnOutputRole: 'owner', + lastTurnOutputVerdict: 'step_done', + }), + ).toEqual({ kind: 'owner-follow-up' }); + }); + it('returns none when an active task has no reviewer or arbiter handoff output', () => { expect( resolveNextTurnAction({ @@ -152,6 +170,15 @@ describe('message-runtime-rules', () => { kind: 'enqueue', queueKind: 'paired-follow-up', }); + expect( + resolveFollowUpDispatch({ + source: 'owner-delivery-success', + nextTurnAction: { kind: 'owner-follow-up' }, + }), + ).toEqual({ + kind: 'enqueue', + queueKind: 'paired-follow-up', + }); expect( resolveFollowUpDispatch({ source: 'owner-delivery-success', @@ -295,6 +322,14 @@ describe('message-runtime-rules', () => { lastTurnOutputRole: 'owner', }), ).toBe('reviewer'); + expect( + resolveQueuedPairedTurnRole({ + taskStatus: 'active', + hasHumanMessage: false, + lastTurnOutputRole: 'owner', + lastTurnOutputVerdict: 'step_done', + }), + ).toBe('owner'); }); it('resolves reviewer execution target from review_ready task status', () => { diff --git a/src/message-runtime-rules.ts b/src/message-runtime-rules.ts index 7d24a07..033abb1 100644 --- a/src/message-runtime-rules.ts +++ b/src/message-runtime-rules.ts @@ -4,6 +4,7 @@ import { normalizeStoredSeqCursor } from './message-cursor.js'; import { isTriggerAllowed, loadSenderAllowlist } from './sender-allowlist.js'; import { isTaskStatusControlMessage } from './task-watch-status.js'; import { ARBITER_AGENT_TYPE, REVIEWER_AGENT_TYPE } from './config.js'; +import type { VisibleVerdict } from './paired-execution-context-shared.js'; import { hasReviewerLease, resolveLeaseServiceId, @@ -79,6 +80,7 @@ export function resolveQueuedPairedTurnRole(args: { taskStatus?: PairedTaskStatus | null; hasHumanMessage: boolean; lastTurnOutputRole?: PairedRoomRole | null; + lastTurnOutputVerdict?: VisibleVerdict | null; }): 'owner' | 'reviewer' | 'arbiter' | null { if (args.hasHumanMessage) { return resolveQueuedTurnRole({ @@ -90,6 +92,7 @@ export function resolveQueuedPairedTurnRole(args: { const nextTurnAction = resolveNextTurnAction({ taskStatus: args.taskStatus, lastTurnOutputRole: args.lastTurnOutputRole, + lastTurnOutputVerdict: args.lastTurnOutputVerdict, }); switch (nextTurnAction.kind) { @@ -125,6 +128,7 @@ export type FollowUpDispatch = export function resolveNextTurnAction(args: { taskStatus?: PairedTaskStatus | null; lastTurnOutputRole?: PairedRoomRole | null; + lastTurnOutputVerdict?: VisibleVerdict | null; }): NextTurnAction { switch (args.taskStatus) { case 'review_ready': @@ -142,7 +146,8 @@ export function resolveNextTurnAction(args: { ? { kind: 'none' } : { kind: 'finalize-owner-turn' }; case 'active': - return args.lastTurnOutputRole === 'reviewer' || + return args.lastTurnOutputVerdict === 'step_done' || + args.lastTurnOutputRole === 'reviewer' || args.lastTurnOutputRole === 'arbiter' ? { kind: 'owner-follow-up' } : { kind: 'none' }; @@ -154,12 +159,14 @@ export function resolveNextTurnAction(args: { export function matchesExpectedPairedFollowUpIntent(args: { taskStatus?: PairedTaskStatus | null; lastTurnOutputRole?: PairedRoomRole | null; + lastTurnOutputVerdict?: VisibleVerdict | null; intentKind: ScheduledNextTurnActionKind; }): boolean { return ( resolveNextTurnAction({ taskStatus: args.taskStatus, lastTurnOutputRole: args.lastTurnOutputRole, + lastTurnOutputVerdict: args.lastTurnOutputVerdict, }).kind === args.intentKind ); } @@ -184,7 +191,8 @@ export function resolveFollowUpDispatch(args: { args.completedRole === 'owner' ) { return args.nextTurnAction.kind === 'reviewer-turn' || - args.nextTurnAction.kind === 'arbiter-turn' + args.nextTurnAction.kind === 'arbiter-turn' || + args.nextTurnAction.kind === 'owner-follow-up' ? { kind: 'enqueue', queueKind: 'paired-follow-up' } : { kind: 'none' }; } @@ -219,6 +227,7 @@ export function resolveFollowUpDispatch(args: { return { kind: 'none' }; } return args.nextTurnAction.kind === 'reviewer-turn' || + args.nextTurnAction.kind === 'owner-follow-up' || args.nextTurnAction.kind === 'arbiter-turn' || args.nextTurnAction.kind === 'finalize-owner-turn' ? { kind: 'enqueue', queueKind: 'paired-follow-up' } diff --git a/src/message-runtime.test.ts b/src/message-runtime.test.ts index e9c6d68..b5d82df 100644 --- a/src/message-runtime.test.ts +++ b/src/message-runtime.test.ts @@ -1632,8 +1632,9 @@ describe('createMessageRuntime', () => { vi.mocked(agentRunner.runAgentProcess).mockImplementation( async (_group, input, _onProcess, onOutput) => { expect(input.prompt).toBe( - `The reviewer approved your work (DONE). Finalize and report the result. -If you intend to close this paired turn now, your first line must be DONE. + `The reviewer approved the current task scope (TASK_DONE / legacy DONE). Finalize and report the result. +If you intend to close this paired turn now, your first line must be TASK_DONE. +If the original request still has remaining work and the owner flow should continue, your first line may be STEP_DONE. If your first line is DONE_WITH_CONCERNS, the system will reopen review instead of finishing.\n\nReviewer's final assessment:\n${truncatedReviewerOutput}`, ); expect(input.prompt).not.toContain(longReviewerOutput); @@ -1741,7 +1742,7 @@ If your first line is DONE_WITH_CONCERNS, the system will reopen review instead vi.mocked(agentRunner.runAgentProcess).mockImplementation( async (_group, input, _onProcess, onOutput) => { expect(input.prompt).toBe( - "The reviewer approved your work (DONE). Finalize and report the result.\nIf you intend to close this paired turn now, your first line must be DONE.\nIf your first line is DONE_WITH_CONCERNS, the system will reopen review instead of finishing.\n\nReviewer's final assessment:\n리뷰 승인 요약", + "The reviewer approved the current task scope (TASK_DONE / legacy DONE). Finalize and report the result.\nIf you intend to close this paired turn now, your first line must be TASK_DONE.\nIf the original request still has remaining work and the owner flow should continue, your first line may be STEP_DONE.\nIf your first line is DONE_WITH_CONCERNS, the system will reopen review instead of finishing.\n\nReviewer's final assessment:\n리뷰 승인 요약", ); expect(input.prompt).not.toContain('DONE\n승인합니다.'); await onOutput?.({ @@ -2004,6 +2005,110 @@ If your first line is DONE_WITH_CONCERNS, the system will reopen review instead expect(pending?.prompt).toContain('이전 reviewer 피드백'); }); + it('builds an owner follow-up pending turn when the latest owner output is STEP_DONE on an active task', () => { + const chatJid = 'group@test'; + const group = makeGroup('claude-code'); + const task = { + id: 'task-owner-step-done-pending', + chat_jid: chatJid, + group_folder: group.folder, + owner_service_id: 'claude', + reviewer_service_id: 'codex-main', + title: null, + source_ref: 'HEAD', + plan_notes: null, + review_requested_at: null, + round_trip_count: 1, + status: 'active', + arbiter_verdict: null, + arbiter_requested_at: null, + completion_reason: null, + created_at: '2026-03-30T00:00:10.000Z', + updated_at: '2026-03-30T00:00:10.000Z', + } as any; + + vi.mocked(db.getLatestPreviousPairedTaskForChat).mockReturnValue(undefined); + vi.mocked(db.getPairedTurnOutputs).mockReturnValue([ + { + id: 1, + task_id: task.id, + turn_number: 1, + role: 'owner', + output_text: 'STEP_DONE\n1단계는 끝났고 2단계로 이어가야 함', + created_at: '2026-03-30T00:00:11.000Z', + }, + ] as any); + + const pending = buildPendingPairedTurn({ + chatJid, + timezone: 'UTC', + task, + rawMissedMessages: [{ seq: 42, timestamp: '2026-03-30T00:00:12.000Z' }], + recentHumanMessages: [], + labeledRecentMessages: [], + resolveChannel: () => makeChannel(chatJid), + }); + + expect(pending).not.toBeNull(); + expect(pending).toMatchObject({ + role: 'owner', + intentKind: 'owner-follow-up', + taskId: task.id, + }); + }); + + it('builds a reviewer pending turn when a review_ready task follows owner TASK_DONE output', () => { + const chatJid = 'group@test'; + const group = makeGroup('claude-code'); + const task = { + id: 'task-owner-task-done-pending', + chat_jid: chatJid, + group_folder: group.folder, + owner_service_id: 'claude', + reviewer_service_id: 'codex-main', + title: null, + source_ref: 'HEAD', + plan_notes: null, + review_requested_at: '2026-03-30T00:00:10.000Z', + round_trip_count: 1, + status: 'review_ready', + arbiter_verdict: null, + arbiter_requested_at: null, + completion_reason: null, + created_at: '2026-03-30T00:00:10.000Z', + updated_at: '2026-03-30T00:00:10.000Z', + } as any; + + vi.mocked(db.getLatestPreviousPairedTaskForChat).mockReturnValue(undefined); + vi.mocked(db.getPairedTurnOutputs).mockReturnValue([ + { + id: 1, + task_id: task.id, + turn_number: 1, + role: 'owner', + output_text: 'TASK_DONE\n요청 범위 전체 완료', + created_at: '2026-03-30T00:00:11.000Z', + }, + ] as any); + + const pending = buildPendingPairedTurn({ + chatJid, + timezone: 'UTC', + task, + rawMissedMessages: [{ seq: 42, timestamp: '2026-03-30T00:00:12.000Z' }], + recentHumanMessages: [], + labeledRecentMessages: [], + resolveChannel: () => makeChannel(chatJid, 'discord-review', false), + }); + + expect(pending).not.toBeNull(); + expect(pending).toMatchObject({ + role: 'reviewer', + intentKind: 'reviewer-turn', + taskId: task.id, + }); + }); + it('re-enqueues reviewer after a successful owner delivery moves the task to review_ready', async () => { const chatJid = 'group@test'; const group = makeGroup('codex'); @@ -2109,6 +2214,133 @@ If your first line is DONE_WITH_CONCERNS, the system will reopen review instead ); }); + it('re-enqueues owner follow-up after a successful owner STEP_DONE delivery keeps the task active', async () => { + const chatJid = 'group@test'; + const group = makeGroup('codex'); + const ownerChannel: Channel = { + ...makeChannel(chatJid), + isOwnMessage: vi.fn((msg) => msg.sender === 'owner-bot@test'), + }; + const reviewerChannel = makeChannel(chatJid, 'discord-review', false); + const enqueueMessageCheck = vi.fn(); + const pairedTask = { + id: 'task-owner-step-done-follow-up', + chat_jid: chatJid, + group_folder: group.folder, + owner_service_id: 'claude', + reviewer_service_id: 'codex-main', + title: null, + source_ref: 'HEAD', + plan_notes: null, + review_requested_at: null, + round_trip_count: 1, + owner_failure_count: 2, + status: 'active', + arbiter_verdict: null, + arbiter_requested_at: null, + completion_reason: null, + created_at: '2026-03-30T00:00:00.000Z', + updated_at: '2026-03-30T00:00:00.000Z', + } as any; + let turnOutputs: any[] = []; + + vi.mocked(serviceRouting.hasReviewerLease).mockReturnValue(true); + vi.mocked(db.getLatestOpenPairedTaskForChat).mockImplementation( + () => pairedTask, + ); + vi.mocked(db.getPairedTurnOutputs).mockImplementation((taskId: string) => + taskId === pairedTask.id ? turnOutputs : [], + ); + vi.mocked(db.getMessagesSince).mockReturnValue([ + { + id: 'human-step-done-1', + chat_jid: chatJid, + sender: 'user@test', + sender_name: 'User', + content: '이 리팩토링 이어서 해줘', + timestamp: '2026-03-30T00:00:00.000Z', + seq: 1, + is_bot_message: false, + } as any, + ]); + vi.mocked(agentRunner.runAgentProcess).mockImplementation( + async (_group, _input, _onProcess, onOutput) => { + pairedTask.status = 'active'; + pairedTask.owner_failure_count = 0; + turnOutputs = [ + { + id: 1, + task_id: pairedTask.id, + turn_number: 1, + role: 'owner', + output_text: 'STEP_DONE\n리팩토링 1단계 완료, 다음 단계 진행', + created_at: '2026-03-30T00:00:01.000Z', + }, + ]; + await onOutput?.({ + status: 'success', + phase: 'final', + result: 'STEP_DONE\n리팩토링 1단계 완료, 다음 단계 진행', + newSessionId: 'session-owner-step-done-follow-up', + }); + return { + status: 'success', + result: 'STEP_DONE\n리팩토링 1단계 완료, 다음 단계 진행', + newSessionId: 'session-owner-step-done-follow-up', + }; + }, + ); + + const runtime = createMessageRuntime({ + assistantName: 'Andy', + idleTimeout: 1_000, + pollInterval: 1_000, + timezone: 'UTC', + triggerPattern: /^@Andy\b/i, + channels: [ownerChannel, reviewerChannel], + queue: { + registerProcess: vi.fn(), + closeStdin: vi.fn(), + notifyIdle: vi.fn(), + enqueueMessageCheck, + } as any, + getRoomBindings: () => ({ [chatJid]: group }), + getSessions: () => ({}), + getLastTimestamp: () => '', + setLastTimestamp: vi.fn(), + getLastAgentTimestamps: () => ({}), + saveState: vi.fn(), + persistSession: vi.fn(), + clearSession: vi.fn(), + }); + + const result = await runtime.processGroupMessages(chatJid, { + runId: 'run-owner-step-done-follow-up', + reason: 'messages', + }); + + expect(result).toBe(true); + expect(ownerChannel.sendMessage).toHaveBeenCalledWith( + chatJid, + 'STEP_DONE\n리팩토링 1단계 완료, 다음 단계 진행', + ); + expect(enqueueMessageCheck).toHaveBeenCalledWith(chatJid); + expect(pairedTask.status).toBe('active'); + expect(pairedTask.round_trip_count).toBe(1); + expect(pairedTask.owner_failure_count).toBe(0); + expect(logger.info).toHaveBeenCalledWith( + expect.objectContaining({ + chatJid, + runId: 'run-owner-step-done-follow-up', + completedRole: 'owner', + taskId: 'task-owner-step-done-follow-up', + taskStatus: 'active', + intentKind: 'owner-follow-up', + }), + 'Queued paired follow-up after successful owner delivery', + ); + }); + it('defers reviewer enqueue after owner delivery when a CI watcher is still active', async () => { const chatJid = 'group@test'; const group = makeGroup('codex'); diff --git a/src/paired-execution-context-owner.ts b/src/paired-execution-context-owner.ts index 9d2b7c4..c59ae8b 100644 --- a/src/paired-execution-context-owner.ts +++ b/src/paired-execution-context-owner.ts @@ -20,7 +20,7 @@ import { } from './paired-execution-context-shared.js'; import type { PairedTask } from './types.js'; -type OwnerFinalizeOutcome = 'stop' | 're_review'; +type OwnerFinalizeOutcome = 'stop' | 're_review' | 'continue_owner'; const OWNER_FAILURE_ESCALATION_THRESHOLD = 2; export function handleFailedOwnerExecution(args: { @@ -170,6 +170,28 @@ function handleOwnerFinalizeCompletion(args: { return 're_review'; } + if (signal.kind === 'request_owner_continue') { + transitionPairedTaskStatus({ + taskId, + currentStatus: task.status, + nextStatus: 'active', + expectedUpdatedAt: task.updated_at, + updatedAt: now, + patch: { + owner_failure_count: 0, + }, + }); + logger.info( + { + taskId, + ownerVerdict, + summary: summary?.slice(0, 100), + }, + 'Owner marked finalize output as an intermediate step — task returned to active without re-review', + ); + return 'continue_owner'; + } + transitionPairedTaskStatus({ taskId, currentStatus: task.status, @@ -290,6 +312,22 @@ export function handleOwnerCompletion(args: { return; } + if (signal.kind === 'request_owner_continue') { + applyPairedTaskPatch({ + taskId, + expectedUpdatedAt: task.updated_at, + updatedAt: now, + patch: { + owner_failure_count: 0, + }, + }); + logger.info( + { taskId, ownerVerdict, summary: summary?.slice(0, 100) }, + 'Owner marked the current output as an intermediate completed step — keeping task active for follow-up', + ); + return; + } + maybeAutoTriggerReviewerAfterOwnerCompletion({ task, taskId, diff --git a/src/paired-execution-context-shared.test.ts b/src/paired-execution-context-shared.test.ts index b19568b..3cb197a 100644 --- a/src/paired-execution-context-shared.test.ts +++ b/src/paired-execution-context-shared.test.ts @@ -9,6 +9,8 @@ import { describe('paired execution context shared verdict helpers', () => { it('parses visible verdicts from the first summary line only', () => { + expect(parseVisibleVerdict('STEP_DONE\nmore to do')).toBe('step_done'); + expect(parseVisibleVerdict('TASK_DONE\nall done')).toBe('task_done'); expect( parseVisibleVerdict( 'DONE_WITH_CONCERNS\n\nfollow-up detail that should not affect parsing', @@ -19,6 +21,15 @@ describe('paired execution context shared verdict helpers', () => { }); it('maps normal owner completion verdicts to reviewer or arbiter signals', () => { + expect( + resolveOwnerCompletionSignal({ + phase: 'normal', + visibleVerdict: 'step_done', + }), + ).toEqual({ + kind: 'request_owner_continue', + resetStatusToActive: false, + }); expect( resolveOwnerCompletionSignal({ phase: 'normal', @@ -37,6 +48,18 @@ describe('paired execution context shared verdict helpers', () => { }); it('maps finalize owner outcomes to complete, re-review, or arbiter', () => { + expect( + resolveOwnerCompletionSignal({ + phase: 'finalize', + visibleVerdict: 'step_done', + hasChangesSinceApproval: false, + roundTripCount: 0, + deadlockThreshold: 2, + }), + ).toEqual({ + kind: 'request_owner_continue', + resetStatusToActive: true, + }); expect( resolveOwnerCompletionSignal({ phase: 'finalize', @@ -70,6 +93,20 @@ describe('paired execution context shared verdict helpers', () => { }); it('maps reviewer completion verdicts to finalize, owner changes, or arbiter', () => { + expect( + resolveReviewerCompletionSignal({ + visibleVerdict: 'task_done', + roundTripCount: 0, + deadlockThreshold: 3, + }), + ).toEqual({ kind: 'request_owner_finalize' }); + expect( + resolveReviewerCompletionSignal({ + visibleVerdict: 'step_done', + roundTripCount: 0, + deadlockThreshold: 3, + }), + ).toEqual({ kind: 'request_owner_changes' }); expect( resolveReviewerCompletionSignal({ visibleVerdict: 'done', @@ -94,6 +131,11 @@ describe('paired execution context shared verdict helpers', () => { }); it('maps reviewer failure verdicts to explicit failure signals', () => { + expect( + resolveReviewerFailureSignal({ + visibleVerdict: 'task_done', + }), + ).toEqual({ kind: 'request_owner_finalize' }); expect( resolveReviewerFailureSignal({ visibleVerdict: 'done', diff --git a/src/paired-execution-context-shared.ts b/src/paired-execution-context-shared.ts index 18d3faa..8bf11fc 100644 --- a/src/paired-execution-context-shared.ts +++ b/src/paired-execution-context-shared.ts @@ -6,6 +6,8 @@ import { logger } from './logger.js'; import type { PairedTaskStatus } from './types.js'; export type VisibleVerdict = + | 'step_done' + | 'task_done' | 'done' | 'done_with_concerns' | 'blocked' @@ -15,6 +17,7 @@ export type VisibleVerdict = export type CompletionSignal = | { kind: 'request_reviewer'; resetStatusToActive: boolean } | { kind: 'request_owner_finalize' } + | { kind: 'request_owner_continue'; resetStatusToActive: boolean } | { kind: 'request_owner_changes' } | { kind: 'request_arbiter' } | { kind: 'complete'; completionReason: 'done' | 'escalated' } @@ -29,6 +32,8 @@ export function parseVisibleVerdict( const firstLine = cleaned.split('\n')[0].trim(); if (/^\*{0,2}BLOCKED\*{0,2}\b/i.test(firstLine)) return 'blocked'; if (/^\*{0,2}NEEDS_CONTEXT\*{0,2}\b/i.test(firstLine)) return 'needs_context'; + if (/^\*{0,2}STEP_DONE\*{0,2}\b/i.test(firstLine)) return 'step_done'; + if (/^\*{0,2}TASK_DONE\*{0,2}\b/i.test(firstLine)) return 'task_done'; if (/^\*{0,2}DONE_WITH_CONCERNS\*{0,2}\b/i.test(firstLine)) return 'done_with_concerns'; if (/^\*{0,2}DONE\*{0,2}\b/i.test(firstLine)) return 'done'; @@ -57,12 +62,25 @@ export function resolveOwnerCompletionSignal(args: { } if (phase === 'normal') { + if (visibleVerdict === 'step_done') { + return { + kind: 'request_owner_continue', + resetStatusToActive: false, + }; + } return { kind: 'request_reviewer', resetStatusToActive: false, }; } + if (visibleVerdict === 'step_done') { + return { + kind: 'request_owner_continue', + resetStatusToActive: true, + }; + } + const needsReReview = visibleVerdict === 'done_with_concerns' || hasChangesSinceApproval === true; @@ -90,8 +108,11 @@ export function resolveReviewerCompletionSignal(args: { const { visibleVerdict, roundTripCount, deadlockThreshold } = args; switch (visibleVerdict) { + case 'task_done': case 'done': return { kind: 'request_owner_finalize' }; + case 'step_done': + return { kind: 'request_owner_changes' }; case 'blocked': case 'needs_context': return { kind: 'request_arbiter' }; @@ -111,6 +132,7 @@ export function resolveReviewerFailureSignal(args: { const { visibleVerdict } = args; switch (visibleVerdict) { + case 'task_done': case 'done': return { kind: 'request_owner_finalize' }; case 'blocked': diff --git a/src/paired-execution-context.test.ts b/src/paired-execution-context.test.ts index bfee801..e3f29bc 100644 --- a/src/paired-execution-context.test.ts +++ b/src/paired-execution-context.test.ts @@ -742,6 +742,65 @@ describe('paired execution context', () => { ).not.toHaveBeenCalled(); }); + it('keeps an active task in owner follow-up mode when the owner reports STEP_DONE', () => { + vi.mocked(db.getPairedTaskById).mockReturnValue( + buildPairedTask({ + status: 'active', + round_trip_count: 1, + }), + ); + + completePairedExecutionContext({ + taskId: 'task-1', + role: 'owner', + status: 'succeeded', + summary: 'STEP_DONE\n1단계 완료, 후속 작업 계속', + }); + + expect(db.updatePairedTask).toHaveBeenCalledWith( + 'task-1', + expect.objectContaining({ + owner_failure_count: 0, + }), + ); + expect( + pairedWorkspaceManager.markPairedTaskReviewReady, + ).not.toHaveBeenCalled(); + expect(db.updatePairedTask).not.toHaveBeenCalledWith( + 'task-1', + expect.objectContaining({ + status: 'active', + }), + ); + }); + + it('returns merge_ready owner finalize output to active when the owner reports STEP_DONE', () => { + vi.mocked(db.getPairedTaskById).mockReturnValue( + buildPairedTask({ + status: 'merge_ready', + round_trip_count: 1, + }), + ); + + completePairedExecutionContext({ + taskId: 'task-1', + role: 'owner', + status: 'succeeded', + summary: 'STEP_DONE\n남은 범위가 있어서 계속 진행', + }); + + expect(db.updatePairedTask).toHaveBeenCalledWith( + 'task-1', + expect.objectContaining({ + status: 'active', + owner_failure_count: 0, + }), + ); + expect( + pairedWorkspaceManager.markPairedTaskReviewReady, + ).not.toHaveBeenCalled(); + }); + it('re-triggers review once when owner reports DONE_WITH_CONCERNS during finalize', () => { vi.mocked(db.getPairedTaskById).mockReturnValue( buildPairedTask({