From b6a24ad9fbcf49301eba358a7745894946fdc3f4 Mon Sep 17 00:00:00 2001 From: ejclaw Date: Mon, 20 Apr 2026 08:19:31 +0900 Subject: [PATCH] paired: carry forward latest owner final across superseded tasks --- prompts/claude-paired-room.md | 5 ++- src/db.test.ts | 41 +++++++++++++++++-- src/db/paired-turn-outputs.ts | 3 +- src/db/runtime-paired.ts | 2 + src/paired-execution-context.test.ts | 51 ++++++++++++++++++++++- src/paired-execution-context.ts | 61 ++++++++++++++++++++++++---- src/platform-prompts.test.ts | 6 +++ 7 files changed, 154 insertions(+), 15 deletions(-) diff --git a/prompts/claude-paired-room.md b/prompts/claude-paired-room.md index 0813804..87d9803 100644 --- a/prompts/claude-paired-room.md +++ b/prompts/claude-paired-room.md @@ -3,6 +3,7 @@ You are the **reviewer** in this paired room. - Your role: review, challenge, verify the owner's work. When you find issues, tell the owner exactly what to fix — the owner is the implementer, not you +- Do not stop at rebuttal. If the owner's approach is viable but clearly suboptimal, suggest 1-2 better alternatives with the reason and tradeoff for each - The owner's role: implement, execute, respond to user requests - Do not infer role from the visible bot name — use the paired-room role context for this turn - When the arbiter renders a verdict (PROCEED/REVISE/RESET), follow it — the arbiter's judgment is binding @@ -17,6 +18,7 @@ Before accepting any proposal, run it through: 4. **Hidden assumptions** — What are we taking for granted that could be wrong? Push back with evidence when the owner is wrong. Hold your ground when you are right. Point out logical gaps, missing edge cases, over-engineering. Agree when the owner is genuinely correct. +If you see a materially better design, debugging path, or scoping choice, propose it briefly. Distinguish blocking defects from optional improvements so the owner can prioritize correctly. ## Completion status @@ -34,7 +36,8 @@ Push back with evidence when the owner is wrong. Hold your ground when you are r - Treat `EJCLAW_WORK_DIR` as the canonical verification root for this turn. You may inspect other local paths for context, but final review findings must be re-checked against `EJCLAW_WORK_DIR` - Do not use a different clone, canonical repo path, or cached session path as the sole basis for `BLOCKED`, `DONE_WITH_CONCERNS`, or change requests. If another path disagrees with `EJCLAW_WORK_DIR`, prefer `EJCLAW_WORK_DIR` and explicitly call out the mismatch - When test/typecheck/build evidence is needed, prefer the dedicated verification path (`run_verification`) over assuming the reviewer workspace should execute the full project locally +- Separate correctness issues from improvement ideas. If something is only a better alternative, label it as optional instead of blocking the owner unnecessarily - Stagnation: **Spinning** (same error 3+), **Oscillation** (alternating approaches), **Diminishing returns** (shrinking improvement), **No progress** (discussion without change) — name the pattern and report: **Status**, **Attempted**, **Recommendation** - Implementation, commits, and pushes require agreement from both sides. Either can veto -- Keep reviews concise — approve quickly when there is nothing to critique +- Keep reviews concise — approve quickly when there is nothing to critique, and keep alternative proposals short and actionable - Never mention or tag the user (@username) during the owner↔reviewer loop — the system handles escalation automatically. User is only notified when all resolution paths (including arbiter) are exhausted diff --git a/src/db.test.ts b/src/db.test.ts index ceacaa9..0d687f5 100644 --- a/src/db.test.ts +++ b/src/db.test.ts @@ -1246,6 +1246,40 @@ describe('paired task state', () => { expect(getLatestTurnNumber('paired-task-turn-output')).toBe(2); }); + it('preserves explicit created_at when inserting a paired turn output', () => { + createPairedTask({ + id: 'paired-task-turn-output-created-at', + chat_jid: 'dc:paired', + group_folder: 'paired-room', + owner_service_id: 'codex-main', + reviewer_service_id: 'codex-review', + title: null, + source_ref: null, + plan_notes: null, + round_trip_count: 0, + review_requested_at: null, + 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', + }); + + insertPairedTurnOutput( + 'paired-task-turn-output-created-at', + 0, + 'owner', + 'carried forward owner final', + '2026-03-28T00:01:23.000Z', + ); + + const outputs = getPairedTurnOutputs('paired-task-turn-output-created-at'); + + expect(outputs).toHaveLength(1); + expect(outputs[0].created_at).toBe('2026-03-28T00:01:23.000Z'); + }); + it('fails init when paired task agent and service metadata conflict', () => { const tempDir = fs.mkdtempSync('/tmp/ejclaw-paired-task-shadow-'); const dbPath = path.join(tempDir, 'messages.db'); @@ -2406,10 +2440,9 @@ describe('room assignment writes', () => { folder: 'arbiter-only-capability', }); - expect(getRegisteredAgentTypesForJid('dc:arbiter-only-capability').sort()).toEqual([ - 'claude-code', - 'codex', - ]); + expect( + getRegisteredAgentTypesForJid('dc:arbiter-only-capability').sort(), + ).toEqual(['claude-code', 'codex']); expect( getAllRoomBindings('codex')['dc:arbiter-only-capability'], ).toMatchObject({ diff --git a/src/db/paired-turn-outputs.ts b/src/db/paired-turn-outputs.ts index 65cdfb3..9c89bb3 100644 --- a/src/db/paired-turn-outputs.ts +++ b/src/db/paired-turn-outputs.ts @@ -11,6 +11,7 @@ export function insertPairedTurnOutputInDatabase( turnNumber: number, role: PairedRoomRole, outputText: string, + createdAt?: string, ): void { if (outputText.length > MAX_TURN_OUTPUT_CHARS) { logger.warn( @@ -36,7 +37,7 @@ export function insertPairedTurnOutputInDatabase( turnNumber, role, outputText.slice(0, MAX_TURN_OUTPUT_CHARS), - new Date().toISOString(), + createdAt ?? new Date().toISOString(), ); } diff --git a/src/db/runtime-paired.ts b/src/db/runtime-paired.ts index fb875a4..807df33 100644 --- a/src/db/runtime-paired.ts +++ b/src/db/runtime-paired.ts @@ -300,6 +300,7 @@ export function insertPairedTurnOutput( turnNumber: number, role: PairedRoomRole, outputText: string, + createdAt?: string, ): void { insertPairedTurnOutputInDatabase( requireDatabase(), @@ -307,6 +308,7 @@ export function insertPairedTurnOutput( turnNumber, role, outputText, + createdAt, ); } diff --git a/src/paired-execution-context.test.ts b/src/paired-execution-context.test.ts index bd5c662..26556ba 100644 --- a/src/paired-execution-context.test.ts +++ b/src/paired-execution-context.test.ts @@ -18,7 +18,9 @@ vi.mock('./db.js', () => { getLatestOpenPairedTaskForChat: vi.fn(), getPairedTaskById: vi.fn(), getPairedTurnById: vi.fn(), + getPairedTurnOutputs: vi.fn(() => []), getPairedWorkspace: vi.fn(), + insertPairedTurnOutput: vi.fn(), updatePairedTask, updatePairedTaskIfUnchanged: vi.fn((id, _expectedUpdatedAt, updates) => { updatePairedTask(id, updates); @@ -51,6 +53,7 @@ import * as config from './config.js'; import { completePairedExecutionContext, preparePairedExecutionContext, + resolveOwnerTaskForHumanMessage, } from './paired-execution-context.js'; import * as pairedWorkspaceManager from './paired-workspace-manager.js'; import type { @@ -250,6 +253,50 @@ describe('paired execution context', () => { }); }); + it('carries forward the latest owner final when a merge_ready task is superseded by new human input', () => { + const supersededTask = buildPairedTask({ + id: 'task-superseded', + status: 'merge_ready', + updated_at: '2026-03-28T00:05:00.000Z', + }); + vi.mocked(db.getLatestOpenPairedTaskForChat).mockReturnValue(supersededTask); + vi.mocked(db.getPairedTurnOutputs).mockReturnValue([ + { + id: 1, + task_id: 'task-superseded', + turn_number: 1, + role: 'owner', + output_text: 'DONE_WITH_CONCERNS\n이전 task owner final', + created_at: '2026-03-28T00:01:00.000Z', + }, + { + id: 2, + task_id: 'task-superseded', + turn_number: 2, + role: 'reviewer', + output_text: 'DONE\nreview approved', + created_at: '2026-03-28T00:02:00.000Z', + }, + ]); + + const result = resolveOwnerTaskForHumanMessage({ + group, + chatJid: 'dc:test', + roomRoleContext: ownerContext, + existingTask: supersededTask, + }); + + expect(db.createPairedTask).toHaveBeenCalledTimes(1); + expect(result.supersededTask).toEqual(supersededTask); + expect(db.insertPairedTurnOutput).toHaveBeenCalledWith( + expect.any(String), + 0, + 'owner', + '[Carried forward context from the previous task: latest owner final]\nDONE_WITH_CONCERNS\n이전 task owner final', + '2026-03-28T00:01:00.000Z', + ); + }); + it('uses room role context agent overrides when creating a paired task', () => { preparePairedExecutionContext({ group, @@ -626,7 +673,9 @@ describe('paired execution context', () => { EJCLAW_PAIRED_ROLE: 'reviewer', EJCLAW_UNSAFE_HOST_PAIRED_MODE: '1', }); - expect(result?.envOverrides.EJCLAW_CLAUDE_REVIEWER_READONLY).toBeUndefined(); + expect( + result?.envOverrides.EJCLAW_CLAUDE_REVIEWER_READONLY, + ).toBeUndefined(); expect(result?.envOverrides.EJCLAW_REVIEWER_RUNTIME).toBeUndefined(); delete process.env.EJCLAW_UNSAFE_HOST_PAIRED_MODE; }); diff --git a/src/paired-execution-context.ts b/src/paired-execution-context.ts index 41ec9dc..ea847d5 100644 --- a/src/paired-execution-context.ts +++ b/src/paired-execution-context.ts @@ -24,8 +24,10 @@ import { getLatestOpenPairedTaskForChat, getPairedTaskById, getPairedTurnById, + getPairedTurnOutputs, getPairedWorkspace, hasActiveCiWatcherForChat, + insertPairedTurnOutput, releasePairedTaskExecutionLease, upsertPairedProject, } from './db.js'; @@ -62,6 +64,7 @@ import type { AgentType, PairedRoomRole, PairedTask, + PairedTurnOutput, PairedWorkspace, RegisteredGroup, RoomRoleContext, @@ -115,8 +118,7 @@ function createActiveTaskForRoom(args: { groupAgentType: roomRoleContext?.ownerAgentType ?? group.agentType, configuredReviewer: roomRoleContext?.reviewerAgentType ?? REVIEWER_AGENT_TYPE, - configuredArbiter: - roomRoleContext?.arbiterAgentType ?? ARBITER_AGENT_TYPE, + configuredArbiter: roomRoleContext?.arbiterAgentType ?? ARBITER_AGENT_TYPE, }); const ownerServiceShadow = resolvePairedTaskServiceShadow( 'owner', @@ -190,6 +192,43 @@ function cancelOutstandingFinalizeOwnerTurn(task: PairedTask): void { ); } +function getLatestTurnOutputByRole( + taskId: string, + role: PairedRoomRole, +): PairedTurnOutput | null { + return ( + [...getPairedTurnOutputs(taskId)] + .reverse() + .find((output) => output.role === role) ?? null + ); +} + +function carryForwardLatestOwnerFinal(args: { + sourceTask: PairedTask; + targetTask: PairedTask; +}): void { + const latestOwnerFinal = getLatestTurnOutputByRole(args.sourceTask.id, 'owner'); + if (!latestOwnerFinal) { + return; + } + + insertPairedTurnOutput( + args.targetTask.id, + 0, + 'owner', + `[Carried forward context from the previous task: latest owner final]\n${latestOwnerFinal.output_text}`, + latestOwnerFinal.created_at, + ); + logger.info( + { + sourceTaskId: args.sourceTask.id, + targetTaskId: args.targetTask.id, + carriedChars: latestOwnerFinal.output_text.length, + }, + 'Carried forward latest owner final into superseding paired task', + ); +} + export interface ResolvedOwnerHumanTask { task: PairedTask | null; supersededTask: PairedTask | null; @@ -247,13 +286,19 @@ export function resolveOwnerTaskForHumanMessage(args: { cancelOutstandingFinalizeOwnerTurn(existing); + const newTask = createActiveTaskForRoom({ + group: args.group, + chatJid: args.chatJid, + canonicalWorkDir, + roomRoleContext: args.roomRoleContext, + }); + carryForwardLatestOwnerFinal({ + sourceTask: existing, + targetTask: newTask, + }); + return { - task: createActiveTaskForRoom({ - group: args.group, - chatJid: args.chatJid, - canonicalWorkDir, - roomRoleContext: args.roomRoleContext, - }), + task: newTask, supersededTask: existing, }; } diff --git a/src/platform-prompts.test.ts b/src/platform-prompts.test.ts index fa7de2d..ea46f07 100644 --- a/src/platform-prompts.test.ts +++ b/src/platform-prompts.test.ts @@ -77,6 +77,12 @@ describe('platform-prompts', () => { expect(codexPairedPrompt).toContain( 'canonical verification root for this turn', ); + expect(codexPairedPrompt).toContain( + 'suggest 1-2 better alternatives with the reason and tradeoff for each', + ); + expect(codexPairedPrompt).toContain( + 'Separate correctness issues from improvement ideas', + ); expect(codexPairedPrompt).not.toContain('owner-side paired agent'); const failoverPlatformPrompt = fs.readFileSync(