diff --git a/src/config.ts b/src/config.ts index cbeab98..d5e7fb9 100644 --- a/src/config.ts +++ b/src/config.ts @@ -106,7 +106,9 @@ export const REVIEWER_AGENT_TYPE: AgentType = /** Service ID for the reviewer based on agent type. */ export const REVIEWER_SERVICE_ID_FOR_TYPE = - REVIEWER_AGENT_TYPE === 'claude-code' ? CLAUDE_SERVICE_ID : CODEX_REVIEW_SERVICE_ID; + REVIEWER_AGENT_TYPE === 'claude-code' + ? CLAUDE_SERVICE_ID + : CODEX_REVIEW_SERVICE_ID; // Max owner↔reviewer round trips per task. 0 = unlimited. const rawMaxRoundTrips = getEnv('PAIRED_MAX_ROUND_TRIPS') || '10'; diff --git a/src/index.ts b/src/index.ts index 40270ca..c2cdd41 100644 --- a/src/index.ts +++ b/src/index.ts @@ -511,44 +511,44 @@ async function main(): Promise { }); leaseRecoveryTimer = setInterval(() => { - if (!hasAvailableClaudeToken()) { - return; + if (!hasAvailableClaudeToken()) { + return; + } + const now = Date.now(); + for (const lease of getActiveCodexFailoverLeases()) { + const activatedMs = lease.activatedAt + ? new Date(lease.activatedAt).getTime() + : NaN; + if (Number.isNaN(activatedMs)) { + logger.warn( + { chatJid: lease.chatJid, activatedAt: lease.activatedAt }, + 'Failover lease has unparseable activated_at, skipping auto-restore', + ); + continue; } - const now = Date.now(); - for (const lease of getActiveCodexFailoverLeases()) { - const activatedMs = lease.activatedAt - ? new Date(lease.activatedAt).getTime() - : NaN; - if (Number.isNaN(activatedMs)) { - logger.warn( - { chatJid: lease.chatJid, activatedAt: lease.activatedAt }, - 'Failover lease has unparseable activated_at, skipping auto-restore', - ); - continue; - } - const elapsed = now - activatedMs; - if (elapsed < FAILOVER_MIN_DURATION_MS) { - logger.debug( - { - chatJid: lease.chatJid, - elapsedMin: Math.round(elapsed / 60_000), - minDurationMin: Math.round(FAILOVER_MIN_DURATION_MS / 60_000), - }, - 'Failover lease still within minimum hold period, skipping restore', - ); - continue; - } - restoreDefaultChannelLease(lease.chatJid); - logger.info( + const elapsed = now - activatedMs; + if (elapsed < FAILOVER_MIN_DURATION_MS) { + logger.debug( { chatJid: lease.chatJid, - serviceId: SERVICE_ID, elapsedMin: Math.round(elapsed / 60_000), + minDurationMin: Math.round(FAILOVER_MIN_DURATION_MS / 60_000), }, - 'Claude token available and failover hold period elapsed, restored default channel lease', + 'Failover lease still within minimum hold period, skipping restore', ); + continue; } - }, 5_000); + restoreDefaultChannelLease(lease.chatJid); + logger.info( + { + chatJid: lease.chatJid, + serviceId: SERVICE_ID, + elapsedMin: Math.round(elapsed / 60_000), + }, + 'Claude token available and failover hold period elapsed, restored default channel lease', + ); + } + }, 5_000); runtime.startMessageLoop().catch((err) => { logger.fatal({ err }, 'Message loop crashed unexpectedly'); process.exit(1); diff --git a/src/message-runtime.ts b/src/message-runtime.ts index 36095ec..84dfb02 100644 --- a/src/message-runtime.ts +++ b/src/message-runtime.ts @@ -491,7 +491,10 @@ export function createMessageRuntime(deps: MessageRuntimeDeps): { (pendingTask.status === 'review_ready' || pendingTask.status === 'in_review'); const deliveryChannel = isReviewerWorkItem ? reviewerChannel : channel; - const delivered = await deliverOpenWorkItem(deliveryChannel, openWorkItem); + const delivered = await deliverOpenWorkItem( + deliveryChannel, + openWorkItem, + ); if (!delivered) return false; } @@ -532,9 +535,10 @@ export function createMessageRuntime(deps: MessageRuntimeDeps): { if (ownerContent) { parts.push(`Owner response:\n---\n${ownerContent}\n---`); } - const reviewPrompt = parts.length > 0 - ? `${parts.join('\n\n')}\n\nReview the owner's response above. Provide feedback or approve.` - : 'Review the latest owner changes in the workspace. Provide feedback or approve.'; + const reviewPrompt = + parts.length > 0 + ? `${parts.join('\n\n')}\n\nReview the owner's response above. Provide feedback or approve.` + : 'Review the latest owner changes in the workspace. Provide feedback or approve.'; // Advance cursor past the owner's messages so they aren't re-processed const lastRaw = rawMissedMessages[rawMissedMessages.length - 1]; @@ -559,6 +563,31 @@ export function createMessageRuntime(deps: MessageRuntimeDeps): { return deliverySucceeded; } + // merge_ready: reviewer approved, owner gets final turn to finalize + if (pendingReviewTask && pendingReviewTask.status === 'merge_ready') { + const lastRaw = rawMissedMessages[rawMissedMessages.length - 1]; + if (lastRaw?.seq != null) { + advanceLastAgentCursor( + deps.getLastAgentTimestamps(), + deps.saveState, + chatJid, + lastRaw.seq, + ); + } + const finalizePrompt = + 'The reviewer approved your work (DONE). Finalize: commit changes, push if appropriate, and report the result.'; + const { deliverySucceeded } = await executeTurn({ + group, + prompt: finalizePrompt, + chatJid, + runId, + channel, + startSeq: null, + endSeq: null, + }); + return deliverySucceeded; + } + const lastIgnored = rawMissedMessages[rawMissedMessages.length - 1]; if (lastIgnored) { advanceLastAgentCursor( diff --git a/src/output-suppression.ts b/src/output-suppression.ts index 6441399..6c02edc 100644 --- a/src/output-suppression.ts +++ b/src/output-suppression.ts @@ -28,8 +28,6 @@ export function buildStructuredOutputPrompt( return prompt; } -export function parseStructuredOutputEnvelope( - _rawText: string, -): null { +export function parseStructuredOutputEnvelope(_rawText: string): null { return null; } diff --git a/src/paired-execution-context.ts b/src/paired-execution-context.ts index 7208b79..e8cf732 100644 --- a/src/paired-execution-context.ts +++ b/src/paired-execution-context.ts @@ -29,17 +29,19 @@ import type { } from './types.js'; // --------------------------------------------------------------------------- -// Reviewer approval detection +// Reviewer verdict detection // --------------------------------------------------------------------------- -// Only check the first line for explicit status markers from the prompt's -// completion status protocol (DONE, DONE_WITH_CONCERNS, BLOCKED, NEEDS_CONTEXT). -// DONE = approved → stop ping-pong. Everything else = continue. -function isReviewerApproval(summary: string | null | undefined): boolean { - if (!summary) return false; +type ReviewerVerdict = 'done' | 'done_with_concerns' | 'blocked' | 'needs_context' | 'continue'; + +function classifyReviewerVerdict(summary: string | null | undefined): ReviewerVerdict { + if (!summary) return 'continue'; const firstLine = summary.trimStart().split('\n')[0].trim(); - // Match DONE but not DONE_WITH_CONCERNS - return /\bDONE\b/.test(firstLine) && !/\bDONE_WITH_CONCERNS\b/.test(firstLine); + if (/\bBLOCKED\b/.test(firstLine)) return 'blocked'; + if (/\bNEEDS_CONTEXT\b/.test(firstLine)) return 'needs_context'; + if (/\bDONE_WITH_CONCERNS\b/.test(firstLine)) return 'done_with_concerns'; + if (/\bDONE\b/.test(firstLine)) return 'done'; + return 'continue'; } // --------------------------------------------------------------------------- @@ -261,8 +263,21 @@ export function completePairedExecutionContext(args: { return; } - // Owner finished → auto-trigger reviewer (if within round trip limit) + // Owner finished if (role === 'owner') { + const now = new Date().toISOString(); + + // merge_ready → owner finalized after reviewer approval → completed + if (task.status === 'merge_ready') { + updatePairedTask(taskId, { status: 'completed', updated_at: now }); + logger.info( + { taskId, summary: args.summary?.slice(0, 100) }, + 'Owner finalized after reviewer approval — task completed', + ); + return; + } + + // Normal turn → auto-trigger reviewer (if within round trip limit) if (task.round_trip_count >= PAIRED_MAX_ROUND_TRIPS) { logger.info( { @@ -275,7 +290,6 @@ export function completePairedExecutionContext(args: { return; } - const now = new Date().toISOString(); const result = markPairedTaskReviewReady(taskId); if (result) { updatePairedTask(taskId, { @@ -290,28 +304,41 @@ export function completePairedExecutionContext(args: { } } - // Reviewer finished → check if approved or needs more work + // Reviewer finished → classify verdict and route accordingly if (role === 'reviewer') { const now = new Date().toISOString(); - const approved = isReviewerApproval(args.summary); - if (approved) { - updatePairedTask(taskId, { - status: 'completed', - updated_at: now, - }); - logger.info( - { taskId, summary: args.summary?.slice(0, 100) }, - 'Reviewer approved, task completed — ping-pong stopped', - ); - } else { - updatePairedTask(taskId, { - status: 'active', - updated_at: now, - }); - logger.info( - { taskId }, - 'Reviewer requested changes, task set back to active for owner', - ); + const verdict = classifyReviewerVerdict(args.summary); + + switch (verdict) { + case 'done': + // Approved → owner gets final turn to commit/push + updatePairedTask(taskId, { status: 'merge_ready', updated_at: now }); + logger.info( + { taskId, verdict, summary: args.summary?.slice(0, 100) }, + 'Reviewer approved — owner gets final turn to finalize', + ); + break; + + case 'blocked': + case 'needs_context': + // Needs user decision — stop ping-pong, leave task active + updatePairedTask(taskId, { status: 'completed', updated_at: now }); + logger.info( + { taskId, verdict, summary: args.summary?.slice(0, 100) }, + 'Reviewer escalated to user — ping-pong stopped', + ); + break; + + case 'done_with_concerns': + case 'continue': + default: + // Owner needs to address feedback — ping-pong continues + updatePairedTask(taskId, { status: 'active', updated_at: now }); + logger.info( + { taskId, verdict }, + 'Reviewer has feedback, task set back to active for owner', + ); + break; } } }