paired: carry forward latest owner final across superseded tasks

This commit is contained in:
ejclaw
2026-04-20 08:19:31 +09:00
parent a59813dee6
commit b6a24ad9fb
7 changed files with 154 additions and 15 deletions

View File

@@ -3,6 +3,7 @@
You are the **reviewer** in this paired room. 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 - 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 - 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 - 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 - 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? 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. 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 ## 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` - 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 - 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 - 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** - 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 - 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 - 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

View File

@@ -1246,6 +1246,40 @@ describe('paired task state', () => {
expect(getLatestTurnNumber('paired-task-turn-output')).toBe(2); 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', () => { it('fails init when paired task agent and service metadata conflict', () => {
const tempDir = fs.mkdtempSync('/tmp/ejclaw-paired-task-shadow-'); const tempDir = fs.mkdtempSync('/tmp/ejclaw-paired-task-shadow-');
const dbPath = path.join(tempDir, 'messages.db'); const dbPath = path.join(tempDir, 'messages.db');
@@ -2406,10 +2440,9 @@ describe('room assignment writes', () => {
folder: 'arbiter-only-capability', folder: 'arbiter-only-capability',
}); });
expect(getRegisteredAgentTypesForJid('dc:arbiter-only-capability').sort()).toEqual([ expect(
'claude-code', getRegisteredAgentTypesForJid('dc:arbiter-only-capability').sort(),
'codex', ).toEqual(['claude-code', 'codex']);
]);
expect( expect(
getAllRoomBindings('codex')['dc:arbiter-only-capability'], getAllRoomBindings('codex')['dc:arbiter-only-capability'],
).toMatchObject({ ).toMatchObject({

View File

@@ -11,6 +11,7 @@ export function insertPairedTurnOutputInDatabase(
turnNumber: number, turnNumber: number,
role: PairedRoomRole, role: PairedRoomRole,
outputText: string, outputText: string,
createdAt?: string,
): void { ): void {
if (outputText.length > MAX_TURN_OUTPUT_CHARS) { if (outputText.length > MAX_TURN_OUTPUT_CHARS) {
logger.warn( logger.warn(
@@ -36,7 +37,7 @@ export function insertPairedTurnOutputInDatabase(
turnNumber, turnNumber,
role, role,
outputText.slice(0, MAX_TURN_OUTPUT_CHARS), outputText.slice(0, MAX_TURN_OUTPUT_CHARS),
new Date().toISOString(), createdAt ?? new Date().toISOString(),
); );
} }

View File

@@ -300,6 +300,7 @@ export function insertPairedTurnOutput(
turnNumber: number, turnNumber: number,
role: PairedRoomRole, role: PairedRoomRole,
outputText: string, outputText: string,
createdAt?: string,
): void { ): void {
insertPairedTurnOutputInDatabase( insertPairedTurnOutputInDatabase(
requireDatabase(), requireDatabase(),
@@ -307,6 +308,7 @@ export function insertPairedTurnOutput(
turnNumber, turnNumber,
role, role,
outputText, outputText,
createdAt,
); );
} }

View File

@@ -18,7 +18,9 @@ vi.mock('./db.js', () => {
getLatestOpenPairedTaskForChat: vi.fn(), getLatestOpenPairedTaskForChat: vi.fn(),
getPairedTaskById: vi.fn(), getPairedTaskById: vi.fn(),
getPairedTurnById: vi.fn(), getPairedTurnById: vi.fn(),
getPairedTurnOutputs: vi.fn(() => []),
getPairedWorkspace: vi.fn(), getPairedWorkspace: vi.fn(),
insertPairedTurnOutput: vi.fn(),
updatePairedTask, updatePairedTask,
updatePairedTaskIfUnchanged: vi.fn((id, _expectedUpdatedAt, updates) => { updatePairedTaskIfUnchanged: vi.fn((id, _expectedUpdatedAt, updates) => {
updatePairedTask(id, updates); updatePairedTask(id, updates);
@@ -51,6 +53,7 @@ import * as config from './config.js';
import { import {
completePairedExecutionContext, completePairedExecutionContext,
preparePairedExecutionContext, preparePairedExecutionContext,
resolveOwnerTaskForHumanMessage,
} from './paired-execution-context.js'; } from './paired-execution-context.js';
import * as pairedWorkspaceManager from './paired-workspace-manager.js'; import * as pairedWorkspaceManager from './paired-workspace-manager.js';
import type { 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', () => { it('uses room role context agent overrides when creating a paired task', () => {
preparePairedExecutionContext({ preparePairedExecutionContext({
group, group,
@@ -626,7 +673,9 @@ describe('paired execution context', () => {
EJCLAW_PAIRED_ROLE: 'reviewer', EJCLAW_PAIRED_ROLE: 'reviewer',
EJCLAW_UNSAFE_HOST_PAIRED_MODE: '1', 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(); expect(result?.envOverrides.EJCLAW_REVIEWER_RUNTIME).toBeUndefined();
delete process.env.EJCLAW_UNSAFE_HOST_PAIRED_MODE; delete process.env.EJCLAW_UNSAFE_HOST_PAIRED_MODE;
}); });

View File

@@ -24,8 +24,10 @@ import {
getLatestOpenPairedTaskForChat, getLatestOpenPairedTaskForChat,
getPairedTaskById, getPairedTaskById,
getPairedTurnById, getPairedTurnById,
getPairedTurnOutputs,
getPairedWorkspace, getPairedWorkspace,
hasActiveCiWatcherForChat, hasActiveCiWatcherForChat,
insertPairedTurnOutput,
releasePairedTaskExecutionLease, releasePairedTaskExecutionLease,
upsertPairedProject, upsertPairedProject,
} from './db.js'; } from './db.js';
@@ -62,6 +64,7 @@ import type {
AgentType, AgentType,
PairedRoomRole, PairedRoomRole,
PairedTask, PairedTask,
PairedTurnOutput,
PairedWorkspace, PairedWorkspace,
RegisteredGroup, RegisteredGroup,
RoomRoleContext, RoomRoleContext,
@@ -115,8 +118,7 @@ function createActiveTaskForRoom(args: {
groupAgentType: roomRoleContext?.ownerAgentType ?? group.agentType, groupAgentType: roomRoleContext?.ownerAgentType ?? group.agentType,
configuredReviewer: configuredReviewer:
roomRoleContext?.reviewerAgentType ?? REVIEWER_AGENT_TYPE, roomRoleContext?.reviewerAgentType ?? REVIEWER_AGENT_TYPE,
configuredArbiter: configuredArbiter: roomRoleContext?.arbiterAgentType ?? ARBITER_AGENT_TYPE,
roomRoleContext?.arbiterAgentType ?? ARBITER_AGENT_TYPE,
}); });
const ownerServiceShadow = resolvePairedTaskServiceShadow( const ownerServiceShadow = resolvePairedTaskServiceShadow(
'owner', '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 { export interface ResolvedOwnerHumanTask {
task: PairedTask | null; task: PairedTask | null;
supersededTask: PairedTask | null; supersededTask: PairedTask | null;
@@ -247,13 +286,19 @@ export function resolveOwnerTaskForHumanMessage(args: {
cancelOutstandingFinalizeOwnerTurn(existing); cancelOutstandingFinalizeOwnerTurn(existing);
const newTask = createActiveTaskForRoom({
group: args.group,
chatJid: args.chatJid,
canonicalWorkDir,
roomRoleContext: args.roomRoleContext,
});
carryForwardLatestOwnerFinal({
sourceTask: existing,
targetTask: newTask,
});
return { return {
task: createActiveTaskForRoom({ task: newTask,
group: args.group,
chatJid: args.chatJid,
canonicalWorkDir,
roomRoleContext: args.roomRoleContext,
}),
supersededTask: existing, supersededTask: existing,
}; };
} }

View File

@@ -77,6 +77,12 @@ describe('platform-prompts', () => {
expect(codexPairedPrompt).toContain( expect(codexPairedPrompt).toContain(
'canonical verification root for this turn', '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'); expect(codexPairedPrompt).not.toContain('owner-side paired agent');
const failoverPlatformPrompt = fs.readFileSync( const failoverPlatformPrompt = fs.readFileSync(