paired: repair safe owner workspace branch mismatches
This commit is contained in:
@@ -194,7 +194,7 @@ export async function runAgentForGroup(
|
||||
roomRoleServiceId: roomRoleContext?.serviceId,
|
||||
roomRole: roomRoleContext?.role,
|
||||
},
|
||||
'Blocked reviewer execution before review-ready snapshot was available',
|
||||
'Blocked paired execution before runner start',
|
||||
);
|
||||
await onOutput?.({
|
||||
status: 'success',
|
||||
|
||||
@@ -29,6 +29,7 @@ vi.mock('./db.js', () => {
|
||||
});
|
||||
|
||||
vi.mock('./paired-workspace-manager.js', () => ({
|
||||
isOwnerWorkspaceRepairNeededError: vi.fn(() => false),
|
||||
markPairedTaskReviewReady: vi.fn(),
|
||||
prepareReviewerWorkspaceForExecution: vi.fn(),
|
||||
provisionOwnerWorkspaceForPairedTask: vi.fn(),
|
||||
@@ -448,6 +449,34 @@ describe('paired execution context', () => {
|
||||
);
|
||||
});
|
||||
|
||||
it('blocks owner execution when the owner workspace needs branch repair', () => {
|
||||
const repairError = new Error(
|
||||
'BLOCKED\nOwner workspace needs repair.\nOwner workspace branch mismatch: `codex/owner/paired-room-sync` -> expected `codex/owner/paired-room`.',
|
||||
);
|
||||
vi.mocked(
|
||||
pairedWorkspaceManager.provisionOwnerWorkspaceForPairedTask,
|
||||
).mockImplementation(() => {
|
||||
throw repairError;
|
||||
});
|
||||
vi.mocked(
|
||||
pairedWorkspaceManager.isOwnerWorkspaceRepairNeededError,
|
||||
).mockReturnValue(true);
|
||||
|
||||
const result = preparePairedExecutionContext({
|
||||
group,
|
||||
chatJid: 'dc:test',
|
||||
runId: 'run-owner-repair-needed',
|
||||
roomRoleContext: ownerContext,
|
||||
hasHumanMessage: true,
|
||||
});
|
||||
|
||||
expect(result?.blockMessage).toContain('Owner workspace needs repair.');
|
||||
expect(result?.blockMessage).toContain(
|
||||
'`codex/owner/paired-room-sync` -> expected `codex/owner/paired-room`',
|
||||
);
|
||||
expect(result?.envOverrides.EJCLAW_WORK_DIR).toBeUndefined();
|
||||
});
|
||||
|
||||
it('blocks reviewer execution when an in-review snapshot became stale', () => {
|
||||
vi.mocked(db.getLatestOpenPairedTaskForChat).mockReturnValue({
|
||||
id: 'task-1',
|
||||
|
||||
@@ -47,6 +47,7 @@ import {
|
||||
transitionPairedTaskStatus,
|
||||
} from './paired-execution-context-shared.js';
|
||||
import {
|
||||
isOwnerWorkspaceRepairNeededError,
|
||||
markPairedTaskReviewReady,
|
||||
prepareReviewerWorkspaceForExecution,
|
||||
provisionOwnerWorkspaceForPairedTask,
|
||||
@@ -273,7 +274,15 @@ export function preparePairedExecutionContext(args: {
|
||||
}
|
||||
// Use a stable per-channel worktree (not per-task) so the Claude SDK
|
||||
// session persists across tasks. Different channels still get isolation.
|
||||
workspace = provisionOwnerWorkspaceForPairedTask(latestTask.id);
|
||||
try {
|
||||
workspace = provisionOwnerWorkspaceForPairedTask(latestTask.id);
|
||||
} catch (error) {
|
||||
if (isOwnerWorkspaceRepairNeededError(error)) {
|
||||
blockMessage = error.blockMessage || error.message;
|
||||
} else {
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
// Update source_ref from workspace HEAD so change detection compares
|
||||
// against the correct repo. At task creation, source_ref is from the
|
||||
// canonical workDir which may differ from the workspace clone.
|
||||
|
||||
@@ -990,6 +990,123 @@ describe('paired workspace manager', () => {
|
||||
).toBe('keep me\n');
|
||||
});
|
||||
|
||||
it('auto-repairs a clean named owner workspace onto the expected new channel branch', async () => {
|
||||
const { db, manager } = await loadModules();
|
||||
db._initTestDatabase();
|
||||
|
||||
const canonicalDir = path.join(tempRoot, 'canonical');
|
||||
initCanonicalRepo(canonicalDir);
|
||||
seedPairedTask(db, canonicalDir, {
|
||||
taskId: 'paired-task-named-branch-repair',
|
||||
groupFolder: 'named-repair-room',
|
||||
});
|
||||
|
||||
const workspaceDir = ownerWorkspacePath('named-repair-room');
|
||||
fs.mkdirSync(path.dirname(workspaceDir), { recursive: true });
|
||||
runGit(
|
||||
[
|
||||
'worktree',
|
||||
'add',
|
||||
'-b',
|
||||
'codex/owner/named-repair-room-sync',
|
||||
workspaceDir,
|
||||
'HEAD',
|
||||
],
|
||||
canonicalDir,
|
||||
);
|
||||
|
||||
const ownerWorkspace = manager.provisionOwnerWorkspaceForPairedTask(
|
||||
'paired-task-named-branch-repair',
|
||||
);
|
||||
|
||||
expect(
|
||||
runGit(['branch', '--show-current'], ownerWorkspace.workspace_dir),
|
||||
).toBe(ownerBranchName('named-repair-room'));
|
||||
expect(
|
||||
fs.readFileSync(
|
||||
path.join(ownerWorkspace.workspace_dir, 'README.md'),
|
||||
'utf-8',
|
||||
),
|
||||
).toBe('original\n');
|
||||
});
|
||||
|
||||
it('auto-repairs a clean named owner workspace onto an existing matching channel branch', async () => {
|
||||
const { db, manager } = await loadModules();
|
||||
db._initTestDatabase();
|
||||
|
||||
const canonicalDir = path.join(tempRoot, 'canonical');
|
||||
initCanonicalRepo(canonicalDir);
|
||||
seedPairedTask(db, canonicalDir, {
|
||||
taskId: 'paired-task-named-existing-branch-repair',
|
||||
groupFolder: 'named-existing-room',
|
||||
});
|
||||
|
||||
runGit(
|
||||
['branch', ownerBranchName('named-existing-room'), 'HEAD'],
|
||||
canonicalDir,
|
||||
);
|
||||
const workspaceDir = ownerWorkspacePath('named-existing-room');
|
||||
fs.mkdirSync(path.dirname(workspaceDir), { recursive: true });
|
||||
runGit(
|
||||
[
|
||||
'worktree',
|
||||
'add',
|
||||
'-b',
|
||||
'codex/owner/named-existing-room-sync',
|
||||
workspaceDir,
|
||||
'HEAD',
|
||||
],
|
||||
canonicalDir,
|
||||
);
|
||||
|
||||
const ownerWorkspace = manager.provisionOwnerWorkspaceForPairedTask(
|
||||
'paired-task-named-existing-branch-repair',
|
||||
);
|
||||
|
||||
expect(
|
||||
runGit(['branch', '--show-current'], ownerWorkspace.workspace_dir),
|
||||
).toBe(ownerBranchName('named-existing-room'));
|
||||
});
|
||||
|
||||
it('blocks provisioning when a named owner workspace branch mismatch has local changes', async () => {
|
||||
const { db, manager } = await loadModules();
|
||||
db._initTestDatabase();
|
||||
|
||||
const canonicalDir = path.join(tempRoot, 'canonical');
|
||||
initCanonicalRepo(canonicalDir);
|
||||
seedPairedTask(db, canonicalDir, {
|
||||
taskId: 'paired-task-dirty-named-branch',
|
||||
groupFolder: 'dirty-named-room',
|
||||
});
|
||||
|
||||
const workspaceDir = ownerWorkspacePath('dirty-named-room');
|
||||
fs.mkdirSync(path.dirname(workspaceDir), { recursive: true });
|
||||
runGit(
|
||||
[
|
||||
'worktree',
|
||||
'add',
|
||||
'-b',
|
||||
'codex/owner/dirty-named-room-sync',
|
||||
workspaceDir,
|
||||
'HEAD',
|
||||
],
|
||||
canonicalDir,
|
||||
);
|
||||
fs.writeFileSync(
|
||||
path.join(workspaceDir, 'README.md'),
|
||||
'dirty named branch\n',
|
||||
);
|
||||
|
||||
expect(() =>
|
||||
manager.provisionOwnerWorkspaceForPairedTask(
|
||||
'paired-task-dirty-named-branch',
|
||||
),
|
||||
).toThrow(/Owner workspace needs repair/);
|
||||
expect(runGit(['branch', '--show-current'], workspaceDir)).toBe(
|
||||
'codex/owner/dirty-named-room-sync',
|
||||
);
|
||||
});
|
||||
|
||||
it('blocks provisioning when the channel branch is already checked out in another worktree', async () => {
|
||||
const { db, manager } = await loadModules();
|
||||
db._initTestDatabase();
|
||||
|
||||
@@ -20,6 +20,7 @@ const REVIEWER_SNAPSHOT_STALE_BLOCK_MESSAGE =
|
||||
'Review snapshot is stale after owner changes. Retry the review once to refresh 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.';
|
||||
const OWNER_WORKSPACE_REPAIR_PREFIX = 'BLOCKED\nOwner workspace needs repair.';
|
||||
const REVIEWER_SNAPSHOT_DENY_SEGMENTS = new Set([
|
||||
'.git',
|
||||
'.claude',
|
||||
@@ -108,6 +109,22 @@ function runGitWithInput(args: string[], cwd: string, input: string): string {
|
||||
}).trim();
|
||||
}
|
||||
|
||||
export class OwnerWorkspaceRepairNeededError extends Error {
|
||||
readonly blockMessage: string;
|
||||
|
||||
constructor(blockMessage: string) {
|
||||
super(blockMessage);
|
||||
this.name = 'OwnerWorkspaceRepairNeededError';
|
||||
this.blockMessage = blockMessage;
|
||||
}
|
||||
}
|
||||
|
||||
export function isOwnerWorkspaceRepairNeededError(
|
||||
error: unknown,
|
||||
): error is OwnerWorkspaceRepairNeededError {
|
||||
return error instanceof OwnerWorkspaceRepairNeededError;
|
||||
}
|
||||
|
||||
function ensureGitRepository(repoDir: string): void {
|
||||
const insideWorkTree = runGit(
|
||||
['rev-parse', '--is-inside-work-tree'],
|
||||
@@ -118,6 +135,82 @@ function ensureGitRepository(repoDir: string): void {
|
||||
}
|
||||
}
|
||||
|
||||
function isGitWorktreeClean(repoDir: string): boolean {
|
||||
return runGit(['status', '--short'], repoDir).length === 0;
|
||||
}
|
||||
|
||||
function buildOwnerWorkspaceRepairBlockMessage(args: {
|
||||
workspaceDir: string;
|
||||
currentBranch: string;
|
||||
targetBranch: string;
|
||||
reason: string;
|
||||
}): string {
|
||||
return `${OWNER_WORKSPACE_REPAIR_PREFIX}
|
||||
Owner workspace branch mismatch: \`${args.currentBranch}\` -> expected \`${args.targetBranch}\`.
|
||||
${args.reason}
|
||||
Workspace: \`${args.workspaceDir}\`
|
||||
Repair the owner workspace branch, then retry the task.`;
|
||||
}
|
||||
|
||||
function maybeRepairNamedOwnerWorkspaceBranch(args: {
|
||||
canonicalWorkDir: string;
|
||||
workspaceDir: string;
|
||||
currentBranch: string;
|
||||
targetBranch: string;
|
||||
targetBranchCommit: string | null;
|
||||
}): boolean {
|
||||
const {
|
||||
canonicalWorkDir,
|
||||
workspaceDir,
|
||||
currentBranch,
|
||||
targetBranch,
|
||||
targetBranchCommit,
|
||||
} = args;
|
||||
const currentHeadCommit = resolveCommit(workspaceDir, 'HEAD');
|
||||
if (!currentHeadCommit) {
|
||||
throw new Error(
|
||||
`Unable to resolve owner workspace HEAD for ${workspaceDir}.`,
|
||||
);
|
||||
}
|
||||
|
||||
if (!isGitWorktreeClean(workspaceDir)) {
|
||||
throw new OwnerWorkspaceRepairNeededError(
|
||||
buildOwnerWorkspaceRepairBlockMessage({
|
||||
workspaceDir,
|
||||
currentBranch,
|
||||
targetBranch,
|
||||
reason:
|
||||
'Automatic repair was skipped because the workspace has local changes.',
|
||||
}),
|
||||
);
|
||||
}
|
||||
|
||||
if (!targetBranchCommit) {
|
||||
runGit(['switch', '-c', targetBranch], workspaceDir);
|
||||
return true;
|
||||
}
|
||||
|
||||
if (targetBranchCommit !== currentHeadCommit) {
|
||||
throw new OwnerWorkspaceRepairNeededError(
|
||||
buildOwnerWorkspaceRepairBlockMessage({
|
||||
workspaceDir,
|
||||
currentBranch,
|
||||
targetBranch,
|
||||
reason:
|
||||
'Automatic repair was skipped because the expected branch points at a different commit.',
|
||||
}),
|
||||
);
|
||||
}
|
||||
|
||||
ensureBranchNotCheckedOutElsewhere(
|
||||
canonicalWorkDir,
|
||||
workspaceDir,
|
||||
targetBranch,
|
||||
);
|
||||
runGit(['switch', targetBranch], workspaceDir);
|
||||
return true;
|
||||
}
|
||||
|
||||
function ensureCleanDirectory(targetDir: string): void {
|
||||
fs.mkdirSync(targetDir, { recursive: true });
|
||||
}
|
||||
@@ -629,9 +722,25 @@ export function provisionOwnerWorkspaceForPairedTask(
|
||||
if (currentBranch === targetBranch) {
|
||||
// Stable owner workspace is already attached to the channel branch.
|
||||
} else if (currentBranch) {
|
||||
throw new Error(
|
||||
`Owner workspace ${workspaceDir} is on unexpected branch ${currentBranch}; expected ${targetBranch}.`,
|
||||
);
|
||||
const repaired = maybeRepairNamedOwnerWorkspaceBranch({
|
||||
canonicalWorkDir,
|
||||
workspaceDir,
|
||||
currentBranch,
|
||||
targetBranch,
|
||||
targetBranchCommit,
|
||||
});
|
||||
if (repaired) {
|
||||
logger.warn(
|
||||
{
|
||||
taskId,
|
||||
workspaceDir,
|
||||
previousBranch: currentBranch,
|
||||
targetBranch,
|
||||
targetBranchCommit,
|
||||
},
|
||||
'Auto-repaired owner workspace branch mismatch',
|
||||
);
|
||||
}
|
||||
} else {
|
||||
const currentHeadCommit = resolveCommit(workspaceDir, 'HEAD');
|
||||
if (!currentHeadCommit) {
|
||||
|
||||
Reference in New Issue
Block a user