From c984d972e1e20771e2968d7bb4de906fdabacf57 Mon Sep 17 00:00:00 2001 From: ejclaw Date: Sat, 11 Apr 2026 20:12:53 +0900 Subject: [PATCH] paired: resync reviewer workspace to owner path --- prompts/claude-paired-room.md | 4 +- src/paired-workspace-manager.test.ts | 77 ++++++++++++++++++++++++++++ src/paired-workspace-manager.ts | 71 ++++++++++++++++--------- 3 files changed, 125 insertions(+), 27 deletions(-) diff --git a/prompts/claude-paired-room.md b/prompts/claude-paired-room.md index 8205400..97c3d31 100644 --- a/prompts/claude-paired-room.md +++ b/prompts/claude-paired-room.md @@ -30,8 +30,8 @@ Push back with evidence when the owner is wrong. Hold your ground when you are r ## Rules - Judge completion only by verification output. "It should work now" means run it. "I'm confident" means nothing — confidence is not evidence. "I tested earlier" means test again if code changed since. "It's a trivial change" means verify anyway -- Reviewer workspaces may intentionally exclude heavy runtime artifacts such as `node_modules`, `dist`, and `build`. Do not treat the inability to run direct local test/typecheck/build from the reviewer snapshot as a product bug by itself -- When test/typecheck/build evidence is needed, prefer the dedicated verification path (`run_verification`) over assuming the reviewer snapshot can execute the full project locally +- Reviewer runs against the owner's current workspace in read-only mode. Do not treat the inability to run direct local test/typecheck/build from the reviewer workspace as a product bug by itself +- 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 - 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 diff --git a/src/paired-workspace-manager.test.ts b/src/paired-workspace-manager.test.ts index e98d70a..d1a4e24 100644 --- a/src/paired-workspace-manager.test.ts +++ b/src/paired-workspace-manager.test.ts @@ -799,6 +799,83 @@ describe('paired workspace manager', () => { ).toBe('owner changed again\n'); }); + it('resyncs a stale reviewer workspace record to the current owner workspace', async () => { + const { db, manager } = await loadModules(); + db._initTestDatabase(); + + const canonicalDir = path.join(tempRoot, 'canonical'); + fs.mkdirSync(canonicalDir, { recursive: true }); + runGit(['init'], canonicalDir); + runGit(['config', 'user.email', 'test@example.com'], canonicalDir); + runGit(['config', 'user.name', 'EJClaw Test'], canonicalDir); + fs.writeFileSync(path.join(canonicalDir, 'README.md'), 'base\n'); + runGit(['add', 'README.md'], canonicalDir); + runGit(['commit', '-m', 'initial'], canonicalDir); + + const now = '2026-03-28T00:00:00.000Z'; + db.upsertPairedProject({ + chat_jid: 'dc:test', + group_folder: 'paired-room', + canonical_work_dir: canonicalDir, + created_at: now, + updated_at: now, + }); + db.createPairedTask({ + id: 'paired-task-6b', + chat_jid: 'dc:test', + group_folder: 'paired-room', + owner_service_id: 'codex-main', + reviewer_service_id: 'codex-review', + title: null, + source_ref: 'HEAD', + plan_notes: null, + round_trip_count: 0, + review_requested_at: '2026-03-28T00:01:00.000Z', + status: 'review_ready', + arbiter_verdict: null, + arbiter_requested_at: null, + completion_reason: null, + created_at: now, + updated_at: '2026-03-28T00:01:00.000Z', + }); + + const ownerWorkspace = + manager.provisionOwnerWorkspaceForPairedTask('paired-task-6b'); + db.upsertPairedWorkspace({ + id: 'paired-task-6b:reviewer', + task_id: 'paired-task-6b', + role: 'reviewer', + workspace_dir: canonicalDir, + snapshot_source_dir: canonicalDir, + snapshot_ref: 'stale-fingerprint', + status: 'ready', + snapshot_refreshed_at: '2026-03-28T00:01:00.000Z', + created_at: '2026-03-28T00:01:00.000Z', + updated_at: '2026-03-28T00:01:00.000Z', + }); + + const result = manager.prepareReviewerWorkspaceForExecution( + db.getPairedTaskById('paired-task-6b')!, + ); + + expect(result.autoRefreshed).toBe(false); + expect(result.blockMessage).toBeUndefined(); + expect(result.workspace?.workspace_dir).toBe(ownerWorkspace.workspace_dir); + expect(result.workspace?.snapshot_source_dir).toBe( + ownerWorkspace.workspace_dir, + ); + expect( + db.getPairedWorkspace('paired-task-6b', 'reviewer')?.workspace_dir, + ).toBe(ownerWorkspace.workspace_dir); + expect( + db.getPairedWorkspace('paired-task-6b', 'reviewer')?.snapshot_source_dir, + ).toBe(ownerWorkspace.workspace_dir); + expect( + db.getPairedWorkspace('paired-task-6b', 'reviewer') + ?.snapshot_refreshed_at, + ).not.toBe('2026-03-28T00:01:00.000Z'); + }); + it('bases a new owner branch on the canonical repo HEAD without requiring a remote', async () => { const { db, manager } = await loadModules(); db._initTestDatabase(); diff --git a/src/paired-workspace-manager.ts b/src/paired-workspace-manager.ts index 5e377f4..2cb88af 100644 --- a/src/paired-workspace-manager.ts +++ b/src/paired-workspace-manager.ts @@ -17,9 +17,9 @@ import type { PairedTask, PairedWorkspace } from './types.js'; import { ensureWorkspaceDependenciesInstalled } from './workspace-package-manager.js'; const REVIEWER_SNAPSHOT_STALE_BLOCK_MESSAGE = - 'Review snapshot is stale after owner changes. Retry the review once to refresh against the latest owner workspace.'; + 'Review workspace is out of date after owner changes. Retry the review once to resync against the latest owner workspace.'; const REVIEWER_SNAPSHOT_NOT_READY_BLOCK_MESSAGE = - 'Review snapshot is not ready yet. Wait for the owner to complete a turn so the reviewer snapshot can be prepared.'; + 'Review workspace is not ready yet. Wait for the owner to complete a turn so the reviewer workspace can be prepared.'; const OWNER_WORKSPACE_REPAIR_PREFIX = 'BLOCKED\nOwner workspace needs repair.'; const REVIEWER_SNAPSHOT_DENY_SEGMENTS = new Set([ '.git', @@ -887,16 +887,11 @@ export function markPairedTaskReviewReady(taskId: string): { ); } - // Reviewer uses the owner workspace directly in read-only mode. - // No file-level snapshot copy needed — just register the path. - const reviewerWorkspace = makeWorkspaceRecord({ + const reviewerWorkspace = syncReviewerWorkspaceToOwnerWorkspace({ taskId, - role: 'reviewer', - workspaceDir: ownerWorkspace.workspace_dir, - snapshotSourceDir: ownerWorkspace.workspace_dir, - snapshotRefreshedAt: requestedAt, + ownerWorkspace, + syncedAt: requestedAt, }); - upsertPairedWorkspace(reviewerWorkspace); logger.info( { taskId, ownerDir: ownerWorkspace.workspace_dir }, 'Reviewer will mount owner workspace directly', @@ -926,6 +921,43 @@ export interface PreparedReviewerWorkspace { autoRefreshed: boolean; } +function syncReviewerWorkspaceToOwnerWorkspace(args: { + taskId: string; + ownerWorkspace: PairedWorkspace; + syncedAt?: string; +}): PairedWorkspace { + const existingReviewerWorkspace = getPairedWorkspace(args.taskId, 'reviewer'); + if ( + existingReviewerWorkspace && + existingReviewerWorkspace.workspace_dir === + args.ownerWorkspace.workspace_dir && + existingReviewerWorkspace.snapshot_source_dir === + args.ownerWorkspace.workspace_dir + ) { + return existingReviewerWorkspace; + } + + const reviewerWorkspace = makeWorkspaceRecord({ + taskId: args.taskId, + role: 'reviewer', + workspaceDir: args.ownerWorkspace.workspace_dir, + snapshotSourceDir: args.ownerWorkspace.workspace_dir, + snapshotRefreshedAt: args.syncedAt ?? new Date().toISOString(), + }); + upsertPairedWorkspace(reviewerWorkspace); + if (existingReviewerWorkspace) { + logger.info( + { + taskId: args.taskId, + previousReviewerDir: existingReviewerWorkspace.workspace_dir, + ownerDir: args.ownerWorkspace.workspace_dir, + }, + 'Resynced reviewer workspace to the current owner workspace', + ); + } + return reviewerWorkspace; +} + export function prepareReviewerWorkspaceForExecution( task: PairedTask, ): PreparedReviewerWorkspace { @@ -939,21 +971,10 @@ export function prepareReviewerWorkspaceForExecution( } // Reviewer uses the owner workspace directly in read-only mode. - // No snapshot copy needed — just return the owner workspace as reviewer workspace. - const reviewerWorkspace = getPairedWorkspace(task.id, 'reviewer') ?? null; - if (reviewerWorkspace) { - return { workspace: reviewerWorkspace, autoRefreshed: false }; - } - - // No reviewer workspace registered yet — register owner dir - const now = new Date().toISOString(); - const newReviewerWorkspace = makeWorkspaceRecord({ + // If an old reviewer record still points elsewhere, resync it first. + const reviewerWorkspace = syncReviewerWorkspaceToOwnerWorkspace({ taskId: task.id, - role: 'reviewer', - workspaceDir: ownerWorkspace.workspace_dir, - snapshotSourceDir: ownerWorkspace.workspace_dir, - snapshotRefreshedAt: now, + ownerWorkspace, }); - upsertPairedWorkspace(newReviewerWorkspace); - return { workspace: newReviewerWorkspace, autoRefreshed: false }; + return { workspace: reviewerWorkspace, autoRefreshed: false }; }