paired: resync reviewer workspace to owner path
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -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) {
|
||||
// If an old reviewer record still points elsewhere, resync it first.
|
||||
const reviewerWorkspace = syncReviewerWorkspaceToOwnerWorkspace({
|
||||
taskId: task.id,
|
||||
ownerWorkspace,
|
||||
});
|
||||
return { workspace: reviewerWorkspace, autoRefreshed: false };
|
||||
}
|
||||
|
||||
// No reviewer workspace registered yet — register owner dir
|
||||
const now = new Date().toISOString();
|
||||
const newReviewerWorkspace = makeWorkspaceRecord({
|
||||
taskId: task.id,
|
||||
role: 'reviewer',
|
||||
workspaceDir: ownerWorkspace.workspace_dir,
|
||||
snapshotSourceDir: ownerWorkspace.workspace_dir,
|
||||
snapshotRefreshedAt: now,
|
||||
});
|
||||
upsertPairedWorkspace(newReviewerWorkspace);
|
||||
return { workspace: newReviewerWorkspace, autoRefreshed: false };
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user