From 274e234365e7ece8cd27c9b1793cae030f002ea8 Mon Sep 17 00:00:00 2001 From: ejclaw Date: Wed, 29 Apr 2026 10:05:26 +0900 Subject: [PATCH] Extract paired verdict signal helpers --- src/index.ts | 2 +- src/ipc-outbound-delivery.ts | 2 +- src/message-agent-executor-paired.ts | 2 +- src/message-agent-executor-rules.ts | 2 +- src/message-runtime-follow-up.ts | 6 +- src/message-runtime-rules.ts | 2 +- src/paired-completion-signals.ts | 109 +++++++++++++++ src/paired-execution-context-arbiter.ts | 6 +- src/paired-execution-context-owner.ts | 4 +- src/paired-execution-context-reviewer.ts | 8 +- src/paired-execution-context-shared.test.ts | 15 +- src/paired-execution-context-shared.ts | 145 ++------------------ src/paired-verdict.test.ts | 34 +++++ src/paired-verdict.ts | 23 ++++ 14 files changed, 196 insertions(+), 164 deletions(-) create mode 100644 src/paired-completion-signals.ts create mode 100644 src/paired-verdict.test.ts diff --git a/src/index.ts b/src/index.ts index 1137ee0..007afd0 100644 --- a/src/index.ts +++ b/src/index.ts @@ -73,7 +73,7 @@ import { isBotMessageSourceKind, resolveInjectedMessageSourceKind, } from './message-source.js'; -import { parseVisibleVerdict } from './paired-execution-context-shared.js'; +import { parseVisibleVerdict } from './paired-verdict.js'; export function isTerminalStatusMessage(text: string): boolean { return parseVisibleVerdict(text) !== 'continue'; diff --git a/src/ipc-outbound-delivery.ts b/src/ipc-outbound-delivery.ts index 1c00b1f..328d8bc 100644 --- a/src/ipc-outbound-delivery.ts +++ b/src/ipc-outbound-delivery.ts @@ -1,7 +1,7 @@ import { createProducedWorkItem } from './db.js'; import type { WorkItem } from './db/work-items.js'; import { deliverOpenWorkItem } from './message-runtime-delivery.js'; -import { parseVisibleVerdict } from './paired-execution-context-shared.js'; +import { parseVisibleVerdict } from './paired-verdict.js'; import { resolveChannelForDeliveryRole } from './router.js'; import type { Channel, diff --git a/src/message-agent-executor-paired.ts b/src/message-agent-executor-paired.ts index 7cbf0ea..eb76773 100644 --- a/src/message-agent-executor-paired.ts +++ b/src/message-agent-executor-paired.ts @@ -14,7 +14,7 @@ import { completePairedExecutionContext, type PreparedPairedExecutionContext, } from './paired-execution-context.js'; -import { parseVisibleVerdict } from './paired-execution-context-shared.js'; +import { parseVisibleVerdict } from './paired-verdict.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'; diff --git a/src/message-agent-executor-rules.ts b/src/message-agent-executor-rules.ts index 134d6f5..7d0f8f5 100644 --- a/src/message-agent-executor-rules.ts +++ b/src/message-agent-executor-rules.ts @@ -2,7 +2,7 @@ import { resolveFollowUpDispatch, resolveNextTurnAction, } from './message-runtime-rules.js'; -import { parseVisibleVerdict } from './paired-execution-context-shared.js'; +import { parseVisibleVerdict } from './paired-verdict.js'; import type { PairedRoomRole, PairedTaskStatus } from './types.js'; export { isRetryableClaudeSessionFailureAttempt, diff --git a/src/message-runtime-follow-up.ts b/src/message-runtime-follow-up.ts index 0bed522..5e9784e 100644 --- a/src/message-runtime-follow-up.ts +++ b/src/message-runtime-follow-up.ts @@ -1,6 +1,8 @@ import { getPairedTurnOutputs } from './db.js'; -import { type VisibleVerdict } from './paired-execution-context-shared.js'; -import { resolveStoredVisibleVerdict } from './paired-verdict.js'; +import { + resolveStoredVisibleVerdict, + type VisibleVerdict, +} from './paired-verdict.js'; import { matchesExpectedPairedFollowUpIntent, resolveFollowUpDispatch, diff --git a/src/message-runtime-rules.ts b/src/message-runtime-rules.ts index c7582e2..9725e3c 100644 --- a/src/message-runtime-rules.ts +++ b/src/message-runtime-rules.ts @@ -4,7 +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 type { VisibleVerdict } from './paired-verdict.js'; import { hasReviewerLease, resolveLeaseServiceId, diff --git a/src/paired-completion-signals.ts b/src/paired-completion-signals.ts new file mode 100644 index 0000000..e8573a1 --- /dev/null +++ b/src/paired-completion-signals.ts @@ -0,0 +1,109 @@ +import type { VisibleVerdict } from './paired-verdict.js'; + +export type CompletionSignal = + | { kind: 'request_reviewer'; resetStatusToActive: boolean } + | { kind: 'request_owner_finalize' } + | { kind: 'request_owner_changes' } + | { kind: 'request_arbiter' } + | { kind: 'complete'; completionReason: 'done' | 'escalated' } + | { kind: 'preserve_review_ready' }; + +export function resolveOwnerCompletionSignal(args: { + phase: 'normal' | 'finalize'; + visibleVerdict: VisibleVerdict; + hasChangesSinceApproval?: boolean | null; + roundTripCount?: number; + deadlockThreshold?: number; +}): CompletionSignal { + const { + phase, + visibleVerdict, + hasChangesSinceApproval = false, + roundTripCount = 0, + deadlockThreshold = Number.POSITIVE_INFINITY, + } = args; + + if (visibleVerdict === 'blocked' || visibleVerdict === 'needs_context') { + return { kind: 'request_arbiter' }; + } + + if (phase === 'normal') { + return { + kind: 'request_reviewer', + resetStatusToActive: false, + }; + } + + if (visibleVerdict === 'step_done') { + return { + kind: 'request_reviewer', + resetStatusToActive: true, + }; + } + + const needsReReview = + visibleVerdict === 'done_with_concerns' || hasChangesSinceApproval === true; + + if (needsReReview) { + if (roundTripCount >= deadlockThreshold) { + return { kind: 'request_arbiter' }; + } + return { + kind: 'request_reviewer', + resetStatusToActive: true, + }; + } + + return { + kind: 'complete', + completionReason: 'done', + }; +} + +export function resolveReviewerCompletionSignal(args: { + visibleVerdict: VisibleVerdict; + roundTripCount: number; + deadlockThreshold: number; +}): CompletionSignal { + 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' }; + case 'done_with_concerns': + case 'continue': + default: + if (roundTripCount >= deadlockThreshold) { + return { kind: 'request_arbiter' }; + } + return { kind: 'request_owner_changes' }; + } +} + +export function resolveReviewerFailureSignal(args: { + visibleVerdict: VisibleVerdict; +}): CompletionSignal { + const { visibleVerdict } = args; + + switch (visibleVerdict) { + case 'task_done': + case 'done': + return { kind: 'request_owner_finalize' }; + case 'blocked': + case 'needs_context': + return { + kind: 'complete', + completionReason: 'escalated', + }; + case 'done_with_concerns': + case 'continue': + default: + return { kind: 'preserve_review_ready' }; + } +} diff --git a/src/paired-execution-context-arbiter.ts b/src/paired-execution-context-arbiter.ts index 9f52b22..3c58810 100644 --- a/src/paired-execution-context-arbiter.ts +++ b/src/paired-execution-context-arbiter.ts @@ -1,9 +1,7 @@ import { ARBITER_DEADLOCK_THRESHOLD } from './config.js'; import { logger } from './logger.js'; -import { - classifyArbiterVerdict, - transitionPairedTaskStatus, -} from './paired-execution-context-shared.js'; +import { transitionPairedTaskStatus } from './paired-execution-context-shared.js'; +import { classifyArbiterVerdict } from './paired-verdict.js'; import type { PairedTask } from './types.js'; export function handleFailedArbiterExecution(args: { diff --git a/src/paired-execution-context-owner.ts b/src/paired-execution-context-owner.ts index beb958a..03fa0d4 100644 --- a/src/paired-execution-context-owner.ts +++ b/src/paired-execution-context-owner.ts @@ -12,12 +12,12 @@ import { markPairedTaskReviewReady } from './paired-workspace-manager.js'; import { applyPairedTaskPatch, hasCodeChangesSinceRef, - parseVisibleVerdict, requestArbiterOrEscalate, - resolveOwnerCompletionSignal, resolveCanonicalSourceRef, transitionPairedTaskStatus, } from './paired-execution-context-shared.js'; +import { resolveOwnerCompletionSignal } from './paired-completion-signals.js'; +import { parseVisibleVerdict } from './paired-verdict.js'; import type { PairedTask } from './types.js'; type OwnerFinalizeOutcome = 'stop' | 're_review'; diff --git a/src/paired-execution-context-reviewer.ts b/src/paired-execution-context-reviewer.ts index 939b639..2bacb76 100644 --- a/src/paired-execution-context-reviewer.ts +++ b/src/paired-execution-context-reviewer.ts @@ -2,13 +2,15 @@ import { ARBITER_DEADLOCK_THRESHOLD } from './config.js'; import { getPairedWorkspace } from './db.js'; import { logger } from './logger.js'; import { - parseVisibleVerdict, requestArbiterOrEscalate, - resolveReviewerCompletionSignal, - resolveReviewerFailureSignal, resolveCanonicalSourceRef, transitionPairedTaskStatus, } from './paired-execution-context-shared.js'; +import { + resolveReviewerCompletionSignal, + resolveReviewerFailureSignal, +} from './paired-completion-signals.js'; +import { parseVisibleVerdict } from './paired-verdict.js'; import type { PairedTask } from './types.js'; export function handleFailedReviewerExecution(args: { diff --git a/src/paired-execution-context-shared.test.ts b/src/paired-execution-context-shared.test.ts index c6ceddf..d8b0962 100644 --- a/src/paired-execution-context-shared.test.ts +++ b/src/paired-execution-context-shared.test.ts @@ -1,25 +1,12 @@ import { describe, expect, it } from 'vitest'; import { - parseVisibleVerdict, resolveOwnerCompletionSignal, resolveReviewerCompletionSignal, resolveReviewerFailureSignal, -} from './paired-execution-context-shared.js'; +} from './paired-completion-signals.js'; 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', - ), - ).toBe('done_with_concerns'); - expect(parseVisibleVerdict('BLOCKED\nextra detail')).toBe('blocked'); - expect(parseVisibleVerdict('random prose')).toBe('continue'); - }); - 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 1ede0b1..def652a 100644 --- a/src/paired-execution-context-shared.ts +++ b/src/paired-execution-context-shared.ts @@ -3,142 +3,19 @@ import { execFileSync } from 'child_process'; import { isArbiterEnabled } from './config.js'; import { updatePairedTaskIfUnchanged } from './db.js'; import { logger } from './logger.js'; -import { parseVisibleVerdict, type VisibleVerdict } from './paired-verdict.js'; import type { PairedTaskStatus } from './types.js'; -export type CompletionSignal = - | { kind: 'request_reviewer'; resetStatusToActive: boolean } - | { kind: 'request_owner_finalize' } - | { kind: 'request_owner_changes' } - | { kind: 'request_arbiter' } - | { kind: 'complete'; completionReason: 'done' | 'escalated' } - | { kind: 'preserve_review_ready' }; - -export { parseVisibleVerdict }; -export type { VisibleVerdict }; - -export function resolveOwnerCompletionSignal(args: { - phase: 'normal' | 'finalize'; - visibleVerdict: VisibleVerdict; - hasChangesSinceApproval?: boolean | null; - roundTripCount?: number; - deadlockThreshold?: number; -}): CompletionSignal { - const { - phase, - visibleVerdict, - hasChangesSinceApproval = false, - roundTripCount = 0, - deadlockThreshold = Number.POSITIVE_INFINITY, - } = args; - - if (visibleVerdict === 'blocked' || visibleVerdict === 'needs_context') { - return { kind: 'request_arbiter' }; - } - - if (phase === 'normal') { - return { - kind: 'request_reviewer', - resetStatusToActive: false, - }; - } - - if (visibleVerdict === 'step_done') { - return { - kind: 'request_reviewer', - resetStatusToActive: true, - }; - } - - const needsReReview = - visibleVerdict === 'done_with_concerns' || hasChangesSinceApproval === true; - - if (needsReReview) { - if (roundTripCount >= deadlockThreshold) { - return { kind: 'request_arbiter' }; - } - return { - kind: 'request_reviewer', - resetStatusToActive: true, - }; - } - - return { - kind: 'complete', - completionReason: 'done', - }; -} - -export function resolveReviewerCompletionSignal(args: { - visibleVerdict: VisibleVerdict; - roundTripCount: number; - deadlockThreshold: number; -}): CompletionSignal { - 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' }; - case 'done_with_concerns': - case 'continue': - default: - if (roundTripCount >= deadlockThreshold) { - return { kind: 'request_arbiter' }; - } - return { kind: 'request_owner_changes' }; - } -} - -export function resolveReviewerFailureSignal(args: { - visibleVerdict: VisibleVerdict; -}): CompletionSignal { - const { visibleVerdict } = args; - - switch (visibleVerdict) { - case 'task_done': - case 'done': - return { kind: 'request_owner_finalize' }; - case 'blocked': - case 'needs_context': - return { - kind: 'complete', - completionReason: 'escalated', - }; - case 'done_with_concerns': - case 'continue': - default: - return { kind: 'preserve_review_ready' }; - } -} - -export type ArbiterVerdictResult = - | 'proceed' - | 'revise' - | 'reset' - | 'escalate' - | 'unknown'; - -export function classifyArbiterVerdict( - summary: string | null | undefined, -): ArbiterVerdictResult { - if (!summary) return 'unknown'; - const cleaned = summary.replace(/[\s\S]*?<\/internal>/g, '').trim(); - if (!cleaned) return 'unknown'; - const firstLine = cleaned.split('\n')[0].trim(); - const verdictMatch = firstLine.match( - /\*{0,2}(?:VERDICT\s*[:—-]\s*)?(PROCEED|REVISE|RESET|ESCALATE)\*{0,2}/i, - ); - if (verdictMatch) { - return verdictMatch[1].toLowerCase() as ArbiterVerdictResult; - } - return 'unknown'; -} +export { + resolveOwnerCompletionSignal, + resolveReviewerCompletionSignal, + resolveReviewerFailureSignal, +} from './paired-completion-signals.js'; +export type { CompletionSignal } from './paired-completion-signals.js'; +export { + classifyArbiterVerdict, + parseVisibleVerdict, +} from './paired-verdict.js'; +export type { ArbiterVerdictResult, VisibleVerdict } from './paired-verdict.js'; export function resolveCanonicalSourceRef(workDir: string): string { const treeHash = resolveCanonicalTreeHash(workDir); diff --git a/src/paired-verdict.test.ts b/src/paired-verdict.test.ts new file mode 100644 index 0000000..7ba6e02 --- /dev/null +++ b/src/paired-verdict.test.ts @@ -0,0 +1,34 @@ +import { describe, expect, it } from 'vitest'; + +import { + classifyArbiterVerdict, + parseVisibleVerdict, +} from './paired-verdict.js'; + +describe('paired verdict parser', () => { + 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', + ), + ).toBe('done_with_concerns'); + expect(parseVisibleVerdict('BLOCKED\nextra detail')).toBe('blocked'); + expect(parseVisibleVerdict('random prose')).toBe('continue'); + }); + + it('classifies arbiter verdicts from the first visible line', () => { + expect(classifyArbiterVerdict('PROCEED\ncontinue')).toBe('proceed'); + expect(classifyArbiterVerdict('**VERDICT: REVISE**\nfix it')).toBe( + 'revise', + ); + expect( + classifyArbiterVerdict( + 'private notes\nVERDICT — RESET\nrestart', + ), + ).toBe('reset'); + expect(classifyArbiterVerdict('ESCALATE\nblocked')).toBe('escalate'); + expect(classifyArbiterVerdict('no verdict here')).toBe('unknown'); + }); +}); diff --git a/src/paired-verdict.ts b/src/paired-verdict.ts index a2eef37..c464df9 100644 --- a/src/paired-verdict.ts +++ b/src/paired-verdict.ts @@ -7,6 +7,13 @@ export type VisibleVerdict = | 'needs_context' | 'continue'; +export type ArbiterVerdictResult = + | 'proceed' + | 'revise' + | 'reset' + | 'escalate' + | 'unknown'; + export function parseVisibleVerdict( summary: string | null | undefined, ): VisibleVerdict { @@ -26,6 +33,22 @@ export function parseVisibleVerdict( return 'continue'; } +export function classifyArbiterVerdict( + summary: string | null | undefined, +): ArbiterVerdictResult { + if (!summary) return 'unknown'; + const cleaned = summary.replace(/[\s\S]*?<\/internal>/g, '').trim(); + if (!cleaned) return 'unknown'; + const firstLine = cleaned.split('\n')[0].trim(); + const verdictMatch = firstLine.match( + /\*{0,2}(?:VERDICT\s*[:—-]\s*)?(PROCEED|REVISE|RESET|ESCALATE)\*{0,2}/i, + ); + if (verdictMatch) { + return verdictMatch[1].toLowerCase() as ArbiterVerdictResult; + } + return 'unknown'; +} + export function resolveStoredVisibleVerdict(args: { verdict?: VisibleVerdict | null; outputText?: string | null;