From 7eaace58c9caf277bdd7a090f7634dcecdc09aa8 Mon Sep 17 00:00:00 2001 From: Peter Drier Date: Fri, 3 Apr 2026 18:25:26 +0200 Subject: [PATCH 1/7] feat: add --worktree flag for isolated write-capable rescue tasks When /codex:rescue runs with --write, it edits files directly in the working directory. This creates conflicts with concurrent uncommitted work or parallel agent editing. The new --worktree flag creates an isolated git worktree so Codex operates on a separate copy of the repo. After completion, the user sees a diff summary and can choose to apply changes to their working tree or discard them. A new worktree-cleanup subcommand handles the keep/discard lifecycle. Changes: - git.mjs: worktree create/remove/diff/apply primitives - worktree.mjs: session lifecycle module (create, diff, cleanup) - render.mjs: worktree result and cleanup renderers - codex-companion.mjs: --worktree flag parsing, executeTaskRunWithWorktree, handleWorktreeCleanup subcommand, background task worker support - rescue.md, codex-rescue.md, codex-cli-runtime SKILL.md: flag routing Closes #135 Co-Authored-By: Claude Opus 4.6 (1M context) --- plugins/codex/agents/codex-rescue.md | 1 + plugins/codex/commands/rescue.md | 3 +- plugins/codex/scripts/codex-companion.mjs | 87 +++++++++++++++++-- plugins/codex/scripts/lib/git.mjs | 44 ++++++++++ plugins/codex/scripts/lib/render.mjs | 56 ++++++++++++ plugins/codex/scripts/lib/worktree.mjs | 29 +++++++ .../codex/skills/codex-cli-runtime/SKILL.md | 1 + 7 files changed, 213 insertions(+), 8 deletions(-) create mode 100644 plugins/codex/scripts/lib/worktree.mjs diff --git a/plugins/codex/agents/codex-rescue.md b/plugins/codex/agents/codex-rescue.md index 971bb29..1cad898 100644 --- a/plugins/codex/agents/codex-rescue.md +++ b/plugins/codex/agents/codex-rescue.md @@ -30,6 +30,7 @@ Forwarding rules: - If the user asks for `spark`, map that to `--model gpt-5.3-codex-spark`. - If the user asks for a concrete model name such as `gpt-5.4-mini`, pass it through with `--model`. - Treat `--effort ` and `--model ` as runtime controls and do not include them in the task text you pass through. +- If the user includes `--worktree`, add `--worktree` to the forwarded `task` call. Do not include it in the task text. This runs Codex in an isolated git worktree. - Default to a write-capable Codex run by adding `--write` unless the user explicitly asks for read-only behavior or only wants review, diagnosis, or research without edits. - Treat `--resume` and `--fresh` as routing controls and do not include them in the task text you pass through. - `--resume` means add `--resume-last`. diff --git a/plugins/codex/commands/rescue.md b/plugins/codex/commands/rescue.md index c92a289..f51516c 100644 --- a/plugins/codex/commands/rescue.md +++ b/plugins/codex/commands/rescue.md @@ -1,6 +1,6 @@ --- description: Delegate investigation, an explicit fix request, or follow-up rescue work to the Codex rescue subagent -argument-hint: "[--background|--wait] [--resume|--fresh] [--model ] [--effort ] [what Codex should investigate, solve, or continue]" +argument-hint: "[--background|--wait] [--worktree] [--resume|--fresh] [--model ] [--effort ] [what Codex should investigate, solve, or continue]" context: fork allowed-tools: Bash(node:*), AskUserQuestion --- @@ -18,6 +18,7 @@ Execution mode: - If neither flag is present, default to foreground. - `--background` and `--wait` are execution flags for Claude Code. Do not forward them to `task`, and do not treat them as part of the natural-language task text. - `--model` and `--effort` are runtime-selection flags. Preserve them for the forwarded `task` call, but do not treat them as part of the natural-language task text. +- `--worktree` is an isolation flag. Preserve it for the forwarded `task` call, but do not treat it as part of the natural-language task text. When present, Codex runs in an isolated git worktree instead of editing the working directory in-place. - If the request includes `--resume`, do not ask whether to continue. The user already chose. - If the request includes `--fresh`, do not ask whether to continue. The user already chose. - Otherwise, before starting Codex, check for a resumable rescue thread from this Claude session by running: diff --git a/plugins/codex/scripts/codex-companion.mjs b/plugins/codex/scripts/codex-companion.mjs index 201d1c7..83288bd 100644 --- a/plugins/codex/scripts/codex-companion.mjs +++ b/plugins/codex/scripts/codex-companion.mjs @@ -21,7 +21,8 @@ import { runAppServerTurn } from "./lib/codex.mjs"; import { readStdinIfPiped } from "./lib/fs.mjs"; -import { collectReviewContext, ensureGitRepository, resolveReviewTarget } from "./lib/git.mjs"; +import { collectReviewContext, ensureGitRepository, getRepoRoot, resolveReviewTarget } from "./lib/git.mjs"; +import { createWorktreeSession, diffWorktreeSession, cleanupWorktreeSession } from "./lib/worktree.mjs"; import { binaryAvailable, terminateProcessTree } from "./lib/process.mjs"; import { loadPromptTemplate, interpolateTemplate } from "./lib/prompts.mjs"; import { @@ -59,7 +60,9 @@ import { renderJobStatusReport, renderSetupReport, renderStatusReport, - renderTaskResult + renderTaskResult, + renderWorktreeTaskResult, + renderWorktreeCleanupResult } from "./lib/render.mjs"; const ROOT_DIR = path.resolve(fileURLToPath(new URL("..", import.meta.url))); @@ -77,7 +80,8 @@ function printUsage() { " node scripts/codex-companion.mjs setup [--enable-review-gate|--disable-review-gate] [--json]", " node scripts/codex-companion.mjs review [--wait|--background] [--base ] [--scope ]", " node scripts/codex-companion.mjs adversarial-review [--wait|--background] [--base ] [--scope ] [focus text]", - " node scripts/codex-companion.mjs task [--background] [--write] [--resume-last|--resume|--fresh] [--model ] [--effort ] [prompt]", + " node scripts/codex-companion.mjs task [--background] [--write] [--worktree] [--resume-last|--resume|--fresh] [--model ] [--effort ] [prompt]", + " node scripts/codex-companion.mjs worktree-cleanup --action [--cwd ]", " node scripts/codex-companion.mjs status [job-id] [--all] [--json]", " node scripts/codex-companion.mjs result [job-id] [--json]", " node scripts/codex-companion.mjs cancel [job-id] [--json]" @@ -498,6 +502,32 @@ async function executeTaskRun(request) { }; } +async function executeTaskRunWithWorktree(request) { + ensureGitRepository(request.cwd); + const session = createWorktreeSession(request.cwd); + + try { + const execution = await executeTaskRun({ + ...request, + cwd: session.worktreePath + }); + + const diff = diffWorktreeSession(session); + const rendered = renderWorktreeTaskResult(execution, session, diff); + return { + ...execution, + rendered, + payload: { + ...execution.payload, + worktreeSession: session + } + }; + } catch (error) { + cleanupWorktreeSession(session, { keep: false }); + throw error; + } +} + function buildReviewJobMetadata(reviewName, target) { return { kind: reviewName === "Adversarial Review" ? "adversarial-review" : "review", @@ -570,13 +600,14 @@ function buildTaskJob(workspaceRoot, taskMetadata, write) { }); } -function buildTaskRequest({ cwd, model, effort, prompt, write, resumeLast, jobId }) { +function buildTaskRequest({ cwd, model, effort, prompt, write, worktree, resumeLast, jobId }) { return { cwd, model, effort, prompt, write, + worktree, resumeLast, jobId }; @@ -704,7 +735,7 @@ async function handleReview(argv) { async function handleTask(argv) { const { options, positionals } = parseCommandInput(argv, { valueOptions: ["model", "effort", "cwd", "prompt-file"], - booleanOptions: ["json", "write", "resume-last", "resume", "fresh", "background"], + booleanOptions: ["json", "write", "worktree", "resume-last", "resume", "fresh", "background"], aliasMap: { m: "model" } @@ -722,11 +753,14 @@ async function handleTask(argv) { throw new Error("Choose either --resume/--resume-last or --fresh."); } const write = Boolean(options.write); + const worktree = Boolean(options.worktree); const taskMetadata = buildTaskRunMetadata({ prompt, resumeLast }); + const runTask = write && worktree ? executeTaskRunWithWorktree : executeTaskRun; + if (options.background) { ensureCodexReady(cwd); requireTaskRequest(prompt, resumeLast); @@ -738,6 +772,7 @@ async function handleTask(argv) { effort, prompt, write, + worktree, resumeLast, jobId: job.id }); @@ -750,12 +785,13 @@ async function handleTask(argv) { await runForegroundCommand( job, (progress) => - executeTaskRun({ + runTask({ cwd, model, effort, prompt, write, + worktree, resumeLast, jobId: job.id, onProgress: progress @@ -794,6 +830,7 @@ async function handleTaskWorker(argv) { logFile: storedJob.logFile ?? null } ); + const runTask = request.write && request.worktree ? executeTaskRunWithWorktree : executeTaskRun; await runTrackedJob( { ...storedJob, @@ -801,7 +838,7 @@ async function handleTaskWorker(argv) { logFile }, () => - executeTaskRun({ + runTask({ ...request, onProgress: progress }), @@ -958,6 +995,39 @@ async function handleCancel(argv) { outputCommandResult(payload, renderCancelReport(nextJob), options.json); } +function handleWorktreeCleanup(argv) { + const { options, positionals } = parseCommandInput(argv, { + valueOptions: ["action", "cwd"], + booleanOptions: ["json"] + }); + + const action = options.action; + if (action !== "keep" && action !== "discard") { + throw new Error("Required: --action keep or --action discard."); + } + + const cwd = resolveCommandCwd(options); + const workspaceRoot = resolveCommandWorkspace(options); + const jobId = positionals[0]; + if (!jobId) { + throw new Error("Required: job ID as positional argument."); + } + + const storedJob = readStoredJob(workspaceRoot, jobId); + if (!storedJob) { + throw new Error(`No stored job found for ${jobId}.`); + } + + const session = storedJob.result?.worktreeSession ?? storedJob.payload?.worktreeSession; + if (!session || !session.worktreePath || !session.branch || !session.repoRoot) { + throw new Error(`Job ${jobId} does not have worktree session data. Was it run with --worktree?`); + } + + const result = cleanupWorktreeSession(session, { keep: action === "keep" }); + const rendered = renderWorktreeCleanupResult(action, result, session); + outputCommandResult({ jobId, action, result, session }, rendered, options.json); +} + async function main() { const [subcommand, ...argv] = process.argv.slice(2); if (!subcommand || subcommand === "help" || subcommand === "--help") { @@ -995,6 +1065,9 @@ async function main() { case "cancel": await handleCancel(argv); break; + case "worktree-cleanup": + handleWorktreeCleanup(argv); + break; default: throw new Error(`Unknown subcommand: ${subcommand}`); } diff --git a/plugins/codex/scripts/lib/git.mjs b/plugins/codex/scripts/lib/git.mjs index 1c0529a..a782a66 100644 --- a/plugins/codex/scripts/lib/git.mjs +++ b/plugins/codex/scripts/lib/git.mjs @@ -187,6 +187,50 @@ function collectBranchContext(cwd, baseRef) { }; } +export function createWorktree(repoRoot) { + const ts = Date.now(); + const worktreesDir = path.join(repoRoot, ".worktrees"); + fs.mkdirSync(worktreesDir, { recursive: true }); + + const worktreePath = path.join(worktreesDir, `codex-${ts}`); + const branch = `codex/${ts}`; + gitChecked(repoRoot, ["worktree", "add", worktreePath, "-b", branch]); + return { worktreePath, branch, repoRoot, timestamp: ts }; +} + +export function removeWorktree(repoRoot, worktreePath) { + const result = git(repoRoot, ["worktree", "remove", "--force", worktreePath]); + if (result.status !== 0 && !result.stderr.includes("is not a working tree")) { + throw new Error(`Failed to remove worktree: ${result.stderr.trim()}`); + } +} + +export function deleteWorktreeBranch(repoRoot, branch) { + git(repoRoot, ["branch", "-D", branch]); +} + +export function getWorktreeDiff(repoRoot, branch) { + const result = git(repoRoot, ["diff", `HEAD...${branch}`, "--stat"]); + if (result.status !== 0 || !result.stdout.trim()) { + return { stat: "", patch: "" }; + } + const stat = result.stdout.trim(); + const patchResult = gitChecked(repoRoot, ["diff", `HEAD...${branch}`]); + return { stat, patch: patchResult.stdout }; +} + +export function applyWorktreePatch(repoRoot, branch) { + const patchResult = git(repoRoot, ["diff", `HEAD...${branch}`]); + if (patchResult.status !== 0 || !patchResult.stdout.trim()) { + return { applied: false, detail: "No changes to apply." }; + } + const applyResult = git(repoRoot, ["apply", "--index"], { input: patchResult.stdout }); + if (applyResult.status !== 0) { + return { applied: false, detail: applyResult.stderr.trim() || "Patch apply failed (conflicts?)." }; + } + return { applied: true, detail: "Changes applied and staged." }; +} + export function collectReviewContext(cwd, target) { const repoRoot = getRepoRoot(cwd); const state = getWorkingTreeState(cwd); diff --git a/plugins/codex/scripts/lib/render.mjs b/plugins/codex/scripts/lib/render.mjs index 2ec1852..1432f25 100644 --- a/plugins/codex/scripts/lib/render.mjs +++ b/plugins/codex/scripts/lib/render.mjs @@ -445,6 +445,62 @@ export function renderStoredJobResult(job, storedJob) { return `${lines.join("\n").trimEnd()}\n`; } +export function renderWorktreeTaskResult(execution, session, diff) { + const lines = []; + + if (execution.rendered) { + lines.push(execution.rendered.trimEnd()); + lines.push(""); + } + + lines.push("---"); + lines.push(""); + lines.push("## Worktree"); + lines.push(""); + lines.push(`Branch: \`${session.branch}\``); + lines.push(`Path: \`${session.worktreePath}\``); + lines.push(""); + + if (diff.stat) { + lines.push("### Changes"); + lines.push(""); + lines.push("```"); + lines.push(diff.stat); + lines.push("```"); + lines.push(""); + } else { + lines.push("Codex made no file changes in the worktree."); + lines.push(""); + } + + lines.push("### Actions"); + lines.push(""); + lines.push("Apply these changes to your working tree, or discard them:"); + lines.push(""); + lines.push(`- **Keep**: \`node "\${CLAUDE_PLUGIN_ROOT}/scripts/codex-companion.mjs" worktree-cleanup ${execution.payload?.jobId ?? "JOB_ID"} --action keep\``); + lines.push(`- **Discard**: \`node "\${CLAUDE_PLUGIN_ROOT}/scripts/codex-companion.mjs" worktree-cleanup ${execution.payload?.jobId ?? "JOB_ID"} --action discard\``); + + return `${lines.join("\n").trimEnd()}\n`; +} + +export function renderWorktreeCleanupResult(action, result, session) { + const lines = ["# Worktree Cleanup", ""]; + + if (action === "keep") { + if (result.applied) { + lines.push(`Applied changes from \`${session.branch}\` and cleaned up.`); + } else { + lines.push(`Failed to apply changes: ${result.detail}`); + lines.push(""); + lines.push(`The branch \`${session.branch}\` may still be available for manual merge.`); + } + } else { + lines.push(`Discarded worktree \`${session.worktreePath}\` and branch \`${session.branch}\`.`); + } + + return `${lines.join("\n").trimEnd()}\n`; +} + export function renderCancelReport(job) { const lines = [ "# Codex Cancel", diff --git a/plugins/codex/scripts/lib/worktree.mjs b/plugins/codex/scripts/lib/worktree.mjs new file mode 100644 index 0000000..4071ad1 --- /dev/null +++ b/plugins/codex/scripts/lib/worktree.mjs @@ -0,0 +1,29 @@ +import { + createWorktree, + removeWorktree, + deleteWorktreeBranch, + getWorktreeDiff, + applyWorktreePatch, + ensureGitRepository +} from "./git.mjs"; + +export function createWorktreeSession(cwd) { + const repoRoot = ensureGitRepository(cwd); + return createWorktree(repoRoot); +} + +export function diffWorktreeSession(session) { + return getWorktreeDiff(session.repoRoot, session.branch); +} + +export function cleanupWorktreeSession(session, { keep = false } = {}) { + if (keep) { + const result = applyWorktreePatch(session.repoRoot, session.branch); + removeWorktree(session.repoRoot, session.worktreePath); + deleteWorktreeBranch(session.repoRoot, session.branch); + return result; + } + removeWorktree(session.repoRoot, session.worktreePath); + deleteWorktreeBranch(session.repoRoot, session.branch); + return { applied: false, detail: "Worktree discarded." }; +} diff --git a/plugins/codex/skills/codex-cli-runtime/SKILL.md b/plugins/codex/skills/codex-cli-runtime/SKILL.md index 0e91bfb..a9c3b76 100644 --- a/plugins/codex/skills/codex-cli-runtime/SKILL.md +++ b/plugins/codex/skills/codex-cli-runtime/SKILL.md @@ -33,6 +33,7 @@ Command selection: - `--resume`: always use `task --resume-last`, even if the request text is ambiguous. - `--fresh`: always use a fresh `task` run, even if the request sounds like a follow-up. - `--effort`: accepted values are `none`, `minimal`, `low`, `medium`, `high`, `xhigh`. +- If the forwarded request includes `--worktree`, pass `--worktree` through to `task`. This runs Codex in an isolated git worktree instead of editing the working directory in-place. - `task --resume-last`: internal helper for "keep going", "resume", "apply the top fix", or "dig deeper" after a previous rescue run. Safety rules: From 309c2d41dea78f11cd540ca87cb9b6c8b8f855d0 Mon Sep 17 00:00:00 2001 From: Peter Drier Date: Fri, 3 Apr 2026 18:47:01 +0200 Subject: [PATCH 2/7] fix: worktree diff capture, cleanup safety, resume-last state, and jobId rendering - Diff from baseCommit within the worktree path to capture uncommitted changes - Preserve worktree and branch when patch apply fails instead of destroying them - Use original repo cwd for state/thread lookups so --resume-last works in worktrees - Pass jobId explicitly to renderWorktreeTaskResult so cleanup commands are usable Co-Authored-By: Claude Opus 4.6 (1M context) --- plugins/codex/scripts/codex-companion.mjs | 8 +++++--- plugins/codex/scripts/lib/git.mjs | 13 +++++++------ plugins/codex/scripts/lib/render.mjs | 9 +++++---- plugins/codex/scripts/lib/worktree.mjs | 7 +++++-- 4 files changed, 22 insertions(+), 15 deletions(-) diff --git a/plugins/codex/scripts/codex-companion.mjs b/plugins/codex/scripts/codex-companion.mjs index 83288bd..5334d20 100644 --- a/plugins/codex/scripts/codex-companion.mjs +++ b/plugins/codex/scripts/codex-companion.mjs @@ -433,6 +433,7 @@ async function executeReviewRun(request) { async function executeTaskRun(request) { const workspaceRoot = resolveWorkspaceRoot(request.cwd); + const stateRoot = request.stateRoot ? resolveWorkspaceRoot(request.stateRoot) : workspaceRoot; ensureCodexReady(request.cwd); const taskMetadata = buildTaskRunMetadata({ @@ -442,7 +443,7 @@ async function executeTaskRun(request) { let resumeThreadId = null; if (request.resumeLast) { - const latestThread = await resolveLatestTrackedTaskThread(workspaceRoot, { + const latestThread = await resolveLatestTrackedTaskThread(stateRoot, { excludeJobId: request.jobId }); if (!latestThread) { @@ -509,11 +510,12 @@ async function executeTaskRunWithWorktree(request) { try { const execution = await executeTaskRun({ ...request, - cwd: session.worktreePath + cwd: session.worktreePath, + stateRoot: request.cwd }); const diff = diffWorktreeSession(session); - const rendered = renderWorktreeTaskResult(execution, session, diff); + const rendered = renderWorktreeTaskResult(execution, session, diff, { jobId: request.jobId }); return { ...execution, rendered, diff --git a/plugins/codex/scripts/lib/git.mjs b/plugins/codex/scripts/lib/git.mjs index a782a66..187ca39 100644 --- a/plugins/codex/scripts/lib/git.mjs +++ b/plugins/codex/scripts/lib/git.mjs @@ -194,8 +194,9 @@ export function createWorktree(repoRoot) { const worktreePath = path.join(worktreesDir, `codex-${ts}`); const branch = `codex/${ts}`; + const baseCommit = gitChecked(repoRoot, ["rev-parse", "HEAD"]).stdout.trim(); gitChecked(repoRoot, ["worktree", "add", worktreePath, "-b", branch]); - return { worktreePath, branch, repoRoot, timestamp: ts }; + return { worktreePath, branch, repoRoot, baseCommit, timestamp: ts }; } export function removeWorktree(repoRoot, worktreePath) { @@ -209,18 +210,18 @@ export function deleteWorktreeBranch(repoRoot, branch) { git(repoRoot, ["branch", "-D", branch]); } -export function getWorktreeDiff(repoRoot, branch) { - const result = git(repoRoot, ["diff", `HEAD...${branch}`, "--stat"]); +export function getWorktreeDiff(worktreePath, baseCommit) { + const result = git(worktreePath, ["diff", baseCommit, "--stat"]); if (result.status !== 0 || !result.stdout.trim()) { return { stat: "", patch: "" }; } const stat = result.stdout.trim(); - const patchResult = gitChecked(repoRoot, ["diff", `HEAD...${branch}`]); + const patchResult = gitChecked(worktreePath, ["diff", baseCommit]); return { stat, patch: patchResult.stdout }; } -export function applyWorktreePatch(repoRoot, branch) { - const patchResult = git(repoRoot, ["diff", `HEAD...${branch}`]); +export function applyWorktreePatch(repoRoot, worktreePath, baseCommit) { + const patchResult = git(worktreePath, ["diff", baseCommit]); if (patchResult.status !== 0 || !patchResult.stdout.trim()) { return { applied: false, detail: "No changes to apply." }; } diff --git a/plugins/codex/scripts/lib/render.mjs b/plugins/codex/scripts/lib/render.mjs index 1432f25..72d45cd 100644 --- a/plugins/codex/scripts/lib/render.mjs +++ b/plugins/codex/scripts/lib/render.mjs @@ -445,7 +445,7 @@ export function renderStoredJobResult(job, storedJob) { return `${lines.join("\n").trimEnd()}\n`; } -export function renderWorktreeTaskResult(execution, session, diff) { +export function renderWorktreeTaskResult(execution, session, diff, { jobId = null } = {}) { const lines = []; if (execution.rendered) { @@ -477,8 +477,9 @@ export function renderWorktreeTaskResult(execution, session, diff) { lines.push(""); lines.push("Apply these changes to your working tree, or discard them:"); lines.push(""); - lines.push(`- **Keep**: \`node "\${CLAUDE_PLUGIN_ROOT}/scripts/codex-companion.mjs" worktree-cleanup ${execution.payload?.jobId ?? "JOB_ID"} --action keep\``); - lines.push(`- **Discard**: \`node "\${CLAUDE_PLUGIN_ROOT}/scripts/codex-companion.mjs" worktree-cleanup ${execution.payload?.jobId ?? "JOB_ID"} --action discard\``); + const resolvedJobId = jobId ?? "JOB_ID"; + lines.push(`- **Keep**: \`node "\${CLAUDE_PLUGIN_ROOT}/scripts/codex-companion.mjs" worktree-cleanup ${resolvedJobId} --action keep\``); + lines.push(`- **Discard**: \`node "\${CLAUDE_PLUGIN_ROOT}/scripts/codex-companion.mjs" worktree-cleanup ${resolvedJobId} --action discard\``); return `${lines.join("\n").trimEnd()}\n`; } @@ -492,7 +493,7 @@ export function renderWorktreeCleanupResult(action, result, session) { } else { lines.push(`Failed to apply changes: ${result.detail}`); lines.push(""); - lines.push(`The branch \`${session.branch}\` may still be available for manual merge.`); + lines.push(`The worktree and branch \`${session.branch}\` have been preserved at \`${session.worktreePath}\` for manual recovery.`); } } else { lines.push(`Discarded worktree \`${session.worktreePath}\` and branch \`${session.branch}\`.`); diff --git a/plugins/codex/scripts/lib/worktree.mjs b/plugins/codex/scripts/lib/worktree.mjs index 4071ad1..cecfab1 100644 --- a/plugins/codex/scripts/lib/worktree.mjs +++ b/plugins/codex/scripts/lib/worktree.mjs @@ -13,12 +13,15 @@ export function createWorktreeSession(cwd) { } export function diffWorktreeSession(session) { - return getWorktreeDiff(session.repoRoot, session.branch); + return getWorktreeDiff(session.worktreePath, session.baseCommit); } export function cleanupWorktreeSession(session, { keep = false } = {}) { if (keep) { - const result = applyWorktreePatch(session.repoRoot, session.branch); + const result = applyWorktreePatch(session.repoRoot, session.worktreePath, session.baseCommit); + if (!result.applied) { + return result; + } removeWorktree(session.repoRoot, session.worktreePath); deleteWorktreeBranch(session.repoRoot, session.branch); return result; From 6b6472f9c7987072bff225191e718a714fb23b1a Mon Sep 17 00:00:00 2001 From: Peter Drier Date: Fri, 3 Apr 2026 19:10:43 +0200 Subject: [PATCH 3/7] fix: capture untracked files in worktree diff and add worktree tests Stage all changes (git add -A) before diffing so new files created in the worktree are included in the patch. Use a temp file for git apply to avoid stdin hang. Suppress spurious error in runCommand when exit status is 0. Adds 12 worktree tests covering session creation, diff capture (uncommitted, committed, untracked), cleanup safety, and render output. Co-Authored-By: Claude Opus 4.6 (1M context) --- plugins/codex/scripts/lib/git.mjs | 25 ++- plugins/codex/scripts/lib/process.mjs | 4 +- tests/worktree.test.mjs | 247 ++++++++++++++++++++++++++ 3 files changed, 268 insertions(+), 8 deletions(-) create mode 100644 tests/worktree.test.mjs diff --git a/plugins/codex/scripts/lib/git.mjs b/plugins/codex/scripts/lib/git.mjs index 187ca39..24d4efc 100644 --- a/plugins/codex/scripts/lib/git.mjs +++ b/plugins/codex/scripts/lib/git.mjs @@ -211,25 +211,36 @@ export function deleteWorktreeBranch(repoRoot, branch) { } export function getWorktreeDiff(worktreePath, baseCommit) { - const result = git(worktreePath, ["diff", baseCommit, "--stat"]); + git(worktreePath, ["add", "-A"]); + const result = git(worktreePath, ["diff", "--cached", baseCommit, "--stat"]); if (result.status !== 0 || !result.stdout.trim()) { return { stat: "", patch: "" }; } const stat = result.stdout.trim(); - const patchResult = gitChecked(worktreePath, ["diff", baseCommit]); + const patchResult = gitChecked(worktreePath, ["diff", "--cached", baseCommit]); return { stat, patch: patchResult.stdout }; } export function applyWorktreePatch(repoRoot, worktreePath, baseCommit) { - const patchResult = git(worktreePath, ["diff", baseCommit]); + git(worktreePath, ["add", "-A"]); + const patchResult = git(worktreePath, ["diff", "--cached", baseCommit]); if (patchResult.status !== 0 || !patchResult.stdout.trim()) { return { applied: false, detail: "No changes to apply." }; } - const applyResult = git(repoRoot, ["apply", "--index"], { input: patchResult.stdout }); - if (applyResult.status !== 0) { - return { applied: false, detail: applyResult.stderr.trim() || "Patch apply failed (conflicts?)." }; + const patchPath = path.join( + repoRoot, + ".codex-worktree-" + Date.now() + "-" + Math.random().toString(16).slice(2) + ".patch" + ); + try { + fs.writeFileSync(patchPath, patchResult.stdout, "utf8"); + const applyResult = git(repoRoot, ["apply", "--index", patchPath]); + if (applyResult.status !== 0) { + return { applied: false, detail: applyResult.stderr.trim() || "Patch apply failed (conflicts?)." }; + } + return { applied: true, detail: "Changes applied and staged." }; + } finally { + fs.rmSync(patchPath, { force: true }); } - return { applied: true, detail: "Changes applied and staged." }; } export function collectReviewContext(cwd, target) { diff --git a/plugins/codex/scripts/lib/process.mjs b/plugins/codex/scripts/lib/process.mjs index 0948dbd..5e76c68 100644 --- a/plugins/codex/scripts/lib/process.mjs +++ b/plugins/codex/scripts/lib/process.mjs @@ -12,6 +12,8 @@ export function runCommand(command, args = [], options = {}) { windowsHide: true }); + const succeeded = result.status === 0 && result.signal === null; + return { command, args, @@ -19,7 +21,7 @@ export function runCommand(command, args = [], options = {}) { signal: result.signal ?? null, stdout: result.stdout ?? "", stderr: result.stderr ?? "", - error: result.error ?? null + error: succeeded ? null : result.error ?? null }; } diff --git a/tests/worktree.test.mjs b/tests/worktree.test.mjs new file mode 100644 index 0000000..81817fe --- /dev/null +++ b/tests/worktree.test.mjs @@ -0,0 +1,247 @@ +import fs from "node:fs"; +import path from "node:path"; +import test from "node:test"; +import assert from "node:assert/strict"; + +import { + createWorktreeSession, + diffWorktreeSession, + cleanupWorktreeSession +} from "../plugins/codex/scripts/lib/worktree.mjs"; +import { getWorktreeDiff } from "../plugins/codex/scripts/lib/git.mjs"; +import { renderWorktreeTaskResult } from "../plugins/codex/scripts/lib/render.mjs"; +import { initGitRepo, makeTempDir, run } from "./helpers.mjs"; + +function gitStdout(cwd, args) { + const result = run("git", args, { cwd }); + assert.equal(result.status, 0, result.stderr); + return result.stdout.trim(); +} + +function commitFile(cwd, fileName = "app.js", contents = "export const value = 1;\n") { + fs.writeFileSync(path.join(cwd, fileName), contents); + assert.equal(run("git", ["add", fileName], { cwd }).status, 0); + const commit = run("git", ["commit", "-m", "init"], { cwd }); + assert.equal(commit.status, 0, commit.stderr); +} + +function createRepoWithInitialCommit() { + const repoRoot = makeTempDir(); + initGitRepo(repoRoot); + commitFile(repoRoot); + return { repoRoot }; +} + +function cleanupSession(session) { + if (!session || !fs.existsSync(session.worktreePath)) { + return; + } + + try { + cleanupWorktreeSession(session, { keep: false }); + } catch { + // Best-effort cleanup for temp test repositories. + } +} + +test("createWorktreeSession returns session with worktreePath, branch, repoRoot, baseCommit", () => { + const { repoRoot } = createRepoWithInitialCommit(); + const session = createWorktreeSession(repoRoot); + + try { + assert.equal(session.repoRoot, repoRoot); + assert.match(session.branch, /^codex\/\d+$/); + assert.equal(session.worktreePath, path.join(repoRoot, ".worktrees", `codex-${session.timestamp}`)); + assert.ok(session.baseCommit); + assert.ok(fs.existsSync(session.worktreePath)); + } finally { + cleanupSession(session); + } +}); + +test("createWorktreeSession baseCommit matches repo HEAD at creation time", () => { + const { repoRoot } = createRepoWithInitialCommit(); + const headAtCreation = gitStdout(repoRoot, ["rev-parse", "HEAD"]); + const session = createWorktreeSession(repoRoot); + + try { + fs.writeFileSync(path.join(repoRoot, "app.js"), "export const value = 2;\n"); + assert.equal(run("git", ["add", "app.js"], { cwd: repoRoot }).status, 0); + const commit = run("git", ["commit", "-m", "repo-root change"], { cwd: repoRoot }); + assert.equal(commit.status, 0, commit.stderr); + + const newHead = gitStdout(repoRoot, ["rev-parse", "HEAD"]); + assert.equal(session.baseCommit, headAtCreation); + assert.notEqual(newHead, session.baseCommit); + } finally { + cleanupSession(session); + } +}); + +test("diffWorktreeSession captures uncommitted changes in the worktree", () => { + const { repoRoot } = createRepoWithInitialCommit(); + const session = createWorktreeSession(repoRoot); + + try { + fs.writeFileSync(path.join(session.worktreePath, "app.js"), "export const value = 2;\n"); + + const diff = diffWorktreeSession(session); + + assert.deepEqual(diff, getWorktreeDiff(session.worktreePath, session.baseCommit)); + assert.notEqual(diff.stat, ""); + assert.match(diff.stat, /app\.js/); + } finally { + cleanupSession(session); + } +}); + +test("diffWorktreeSession captures committed changes in the worktree", () => { + const { repoRoot } = createRepoWithInitialCommit(); + const session = createWorktreeSession(repoRoot); + + try { + fs.writeFileSync(path.join(session.worktreePath, "app.js"), "export const value = 2;\n"); + assert.equal(run("git", ["add", "app.js"], { cwd: session.worktreePath }).status, 0); + const commit = run("git", ["commit", "-m", "worktree change"], { cwd: session.worktreePath }); + assert.equal(commit.status, 0, commit.stderr); + + const diff = diffWorktreeSession(session); + + assert.deepEqual(diff, getWorktreeDiff(session.worktreePath, session.baseCommit)); + assert.notEqual(diff.stat, ""); + assert.match(diff.stat, /app\.js/); + } finally { + cleanupSession(session); + } +}); + +test("diffWorktreeSession returns empty when no changes made", () => { + const { repoRoot } = createRepoWithInitialCommit(); + const session = createWorktreeSession(repoRoot); + + try { + const diff = diffWorktreeSession(session); + + assert.deepEqual(diff, { stat: "", patch: "" }); + assert.deepEqual(diff, getWorktreeDiff(session.worktreePath, session.baseCommit)); + } finally { + cleanupSession(session); + } +}); + +test("diffWorktreeSession captures new untracked files in the worktree", () => { + const { repoRoot } = createRepoWithInitialCommit(); + const session = createWorktreeSession(repoRoot); + + try { + fs.writeFileSync(path.join(session.worktreePath, "newfile.js"), "export const added = true;\n"); + + const diff = diffWorktreeSession(session); + + assert.notEqual(diff.stat, ""); + assert.match(diff.stat, /newfile\.js/); + assert.match(diff.patch, /added = true/); + } finally { + cleanupSession(session); + } +}); + +test("cleanupWorktreeSession with keep=true applies new untracked files to repoRoot", () => { + const { repoRoot } = createRepoWithInitialCommit(); + const session = createWorktreeSession(repoRoot); + + try { + fs.writeFileSync(path.join(session.worktreePath, "newfile.js"), "export const added = true;\n"); + + const result = cleanupWorktreeSession(session, { keep: true }); + + assert.equal(result.applied, true); + assert.ok(fs.existsSync(path.join(repoRoot, "newfile.js"))); + assert.match(fs.readFileSync(path.join(repoRoot, "newfile.js"), "utf8"), /added = true/); + } finally { + cleanupSession(session); + } +}); + +test("cleanupWorktreeSession with keep=true applies uncommitted worktree changes to repoRoot as staged changes", () => { + const { repoRoot } = createRepoWithInitialCommit(); + const session = createWorktreeSession(repoRoot); + + try { + fs.writeFileSync(path.join(session.worktreePath, "app.js"), "export const value = 2;\n"); + + const result = cleanupWorktreeSession(session, { keep: true }); + const stagedStat = gitStdout(repoRoot, ["diff", "--cached", "--stat"]); + + assert.equal(result.applied, true); + assert.match(stagedStat, /app\.js/); + assert.equal(fs.existsSync(session.worktreePath), false); + } finally { + cleanupSession(session); + } +}); + +test("cleanupWorktreeSession with keep=false discards worktree and returns applied:false", () => { + const { repoRoot } = createRepoWithInitialCommit(); + const session = createWorktreeSession(repoRoot); + + try { + fs.writeFileSync(path.join(session.worktreePath, "app.js"), "export const value = 2;\n"); + + const result = cleanupWorktreeSession(session, { keep: false }); + const stagedStat = gitStdout(repoRoot, ["diff", "--cached", "--stat"]); + + assert.equal(result.applied, false); + assert.match(result.detail, /Worktree discarded\./); + assert.equal(stagedStat, ""); + assert.equal(fs.existsSync(session.worktreePath), false); + } finally { + cleanupSession(session); + } +}); + +test("cleanupWorktreeSession with keep=true preserves worktree when apply fails", () => { + const { repoRoot } = createRepoWithInitialCommit(); + const session = createWorktreeSession(repoRoot); + + try { + fs.writeFileSync(path.join(session.worktreePath, "app.js"), "export const value = 2;\n"); + fs.writeFileSync(path.join(repoRoot, "app.js"), "export const value = 3;\n"); + assert.equal(run("git", ["add", "app.js"], { cwd: repoRoot }).status, 0); + + const result = cleanupWorktreeSession(session, { keep: true }); + const branchList = gitStdout(repoRoot, ["branch", "--list", session.branch]); + + assert.equal(result.applied, false); + assert.ok(result.detail); + assert.equal(fs.existsSync(session.worktreePath), true); + assert.match(branchList, new RegExp(session.branch)); + } finally { + cleanupSession(session); + } +}); + +test("renderWorktreeTaskResult includes jobId in cleanup commands when provided", () => { + const output = renderWorktreeTaskResult( + { rendered: "# Task Result\n" }, + { branch: "codex/123", worktreePath: "/tmp/worktree-123" }, + { stat: " app.js | 1 +", patch: "" }, + { jobId: "job-123" } + ); + + assert.match(output, /worktree-cleanup job-123 --action keep/); + assert.match(output, /worktree-cleanup job-123 --action discard/); + assert.doesNotMatch(output, /worktree-cleanup JOB_ID/); +}); + +test("renderWorktreeTaskResult falls back to JOB_ID when jobId is null", () => { + const output = renderWorktreeTaskResult( + { rendered: "" }, + { branch: "codex/123", worktreePath: "/tmp/worktree-123" }, + { stat: "", patch: "" }, + { jobId: null } + ); + + assert.match(output, /worktree-cleanup JOB_ID --action keep/); + assert.match(output, /worktree-cleanup JOB_ID --action discard/); +}); From 4bca062f807d7d39e8b5605dd49345176c77fe84 Mon Sep 17 00:00:00 2001 From: Peter Drier Date: Sat, 4 Apr 2026 00:39:00 +0200 Subject: [PATCH 4/7] =?UTF-8?q?fix:=20address=20PR=20review=20=E2=80=94=20?= =?UTF-8?q?clean=20up=20no-op=20worktrees=20and=20exclude=20.worktrees=20f?= =?UTF-8?q?rom=20target=20repos?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit cleanupWorktreeSession now distinguishes "no changes" from "apply failed" so keep=true with no edits still removes the worktree and branch instead of leaving them behind. createWorktree writes .worktrees/ to .git/info/exclude so the directory doesn't dirty the target repo's working tree. Co-Authored-By: Claude Opus 4.6 (1M context) --- plugins/codex/scripts/lib/git.mjs | 8 ++++++++ plugins/codex/scripts/lib/worktree.mjs | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/plugins/codex/scripts/lib/git.mjs b/plugins/codex/scripts/lib/git.mjs index 24d4efc..159fb4c 100644 --- a/plugins/codex/scripts/lib/git.mjs +++ b/plugins/codex/scripts/lib/git.mjs @@ -192,6 +192,14 @@ export function createWorktree(repoRoot) { const worktreesDir = path.join(repoRoot, ".worktrees"); fs.mkdirSync(worktreesDir, { recursive: true }); + // Ensure .worktrees/ is excluded from the target repo without modifying tracked files + const excludePath = path.join(repoRoot, ".git", "info", "exclude"); + const excludeContent = fs.existsSync(excludePath) ? fs.readFileSync(excludePath, "utf8") : ""; + if (!excludeContent.includes(".worktrees")) { + fs.mkdirSync(path.dirname(excludePath), { recursive: true }); + fs.appendFileSync(excludePath, `${excludeContent.endsWith("\n") || !excludeContent ? "" : "\n"}.worktrees/\n`); + } + const worktreePath = path.join(worktreesDir, `codex-${ts}`); const branch = `codex/${ts}`; const baseCommit = gitChecked(repoRoot, ["rev-parse", "HEAD"]).stdout.trim(); diff --git a/plugins/codex/scripts/lib/worktree.mjs b/plugins/codex/scripts/lib/worktree.mjs index cecfab1..ad32c9e 100644 --- a/plugins/codex/scripts/lib/worktree.mjs +++ b/plugins/codex/scripts/lib/worktree.mjs @@ -19,7 +19,7 @@ export function diffWorktreeSession(session) { export function cleanupWorktreeSession(session, { keep = false } = {}) { if (keep) { const result = applyWorktreePatch(session.repoRoot, session.worktreePath, session.baseCommit); - if (!result.applied) { + if (!result.applied && result.detail !== "No changes to apply.") { return result; } removeWorktree(session.repoRoot, session.worktreePath); From 70727cfbdbb029d25dcaae1f379f698c3676032f Mon Sep 17 00:00:00 2001 From: Peter Drier Date: Sun, 5 Apr 2026 18:00:55 +0200 Subject: [PATCH 5/7] fix: handle linked-worktree .git layout and render no-op keep as success - Use `git rev-parse --git-dir` instead of hardcoded `.git/` path for the exclude file, fixing ENOTDIR when repo is a linked worktree. - Render no-op keep cleanup as success instead of misleading "failed" message when there were simply no changes to apply. Co-Authored-By: Claude Opus 4.6 (1M context) --- plugins/codex/scripts/lib/git.mjs | 6 ++++-- plugins/codex/scripts/lib/render.mjs | 2 ++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/plugins/codex/scripts/lib/git.mjs b/plugins/codex/scripts/lib/git.mjs index 159fb4c..8402ee7 100644 --- a/plugins/codex/scripts/lib/git.mjs +++ b/plugins/codex/scripts/lib/git.mjs @@ -192,8 +192,10 @@ export function createWorktree(repoRoot) { const worktreesDir = path.join(repoRoot, ".worktrees"); fs.mkdirSync(worktreesDir, { recursive: true }); - // Ensure .worktrees/ is excluded from the target repo without modifying tracked files - const excludePath = path.join(repoRoot, ".git", "info", "exclude"); + // Ensure .worktrees/ is excluded from the target repo without modifying tracked files. + // Use git rev-parse to resolve the real git dir (handles linked worktrees where .git is a file). + const gitDir = gitChecked(repoRoot, ["rev-parse", "--git-dir"]).stdout.trim(); + const excludePath = path.join(gitDir, "info", "exclude"); const excludeContent = fs.existsSync(excludePath) ? fs.readFileSync(excludePath, "utf8") : ""; if (!excludeContent.includes(".worktrees")) { fs.mkdirSync(path.dirname(excludePath), { recursive: true }); diff --git a/plugins/codex/scripts/lib/render.mjs b/plugins/codex/scripts/lib/render.mjs index 72d45cd..e75f5f9 100644 --- a/plugins/codex/scripts/lib/render.mjs +++ b/plugins/codex/scripts/lib/render.mjs @@ -490,6 +490,8 @@ export function renderWorktreeCleanupResult(action, result, session) { if (action === "keep") { if (result.applied) { lines.push(`Applied changes from \`${session.branch}\` and cleaned up.`); + } else if (result.detail === "No changes to apply.") { + lines.push(`No changes to apply. Worktree and branch \`${session.branch}\` cleaned up.`); } else { lines.push(`Failed to apply changes: ${result.detail}`); lines.push(""); From 256df3a6e9031d0332da4088c9452cdda3056708 Mon Sep 17 00:00:00 2001 From: Peter Drier Date: Sun, 5 Apr 2026 18:18:48 +0200 Subject: [PATCH 6/7] fix: resolve relative git-dir path against repoRoot git rev-parse --git-dir returns a relative path (e.g. ".git") in normal repos. Resolve it against repoRoot so the exclude file path is correct regardless of working directory. Co-Authored-By: Claude Opus 4.6 (1M context) --- plugins/codex/scripts/lib/git.mjs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/plugins/codex/scripts/lib/git.mjs b/plugins/codex/scripts/lib/git.mjs index 8402ee7..0784945 100644 --- a/plugins/codex/scripts/lib/git.mjs +++ b/plugins/codex/scripts/lib/git.mjs @@ -194,7 +194,8 @@ export function createWorktree(repoRoot) { // Ensure .worktrees/ is excluded from the target repo without modifying tracked files. // Use git rev-parse to resolve the real git dir (handles linked worktrees where .git is a file). - const gitDir = gitChecked(repoRoot, ["rev-parse", "--git-dir"]).stdout.trim(); + const rawGitDir = gitChecked(repoRoot, ["rev-parse", "--git-dir"]).stdout.trim(); + const gitDir = path.resolve(repoRoot, rawGitDir); const excludePath = path.join(gitDir, "info", "exclude"); const excludeContent = fs.existsSync(excludePath) ? fs.readFileSync(excludePath, "utf8") : ""; if (!excludeContent.includes(".worktrees")) { From 4fe80773b13955fad069068028eb390aed10edc5 Mon Sep 17 00:00:00 2001 From: Peter Drier Date: Sun, 5 Apr 2026 19:41:57 +0200 Subject: [PATCH 7/7] fix: include --cwd in rendered worktree cleanup commands Ensures cleanup works regardless of the caller's current directory by passing the original repo root via --cwd. Co-Authored-By: Claude Opus 4.6 (1M context) --- plugins/codex/scripts/lib/render.mjs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/plugins/codex/scripts/lib/render.mjs b/plugins/codex/scripts/lib/render.mjs index e75f5f9..4394ed7 100644 --- a/plugins/codex/scripts/lib/render.mjs +++ b/plugins/codex/scripts/lib/render.mjs @@ -478,8 +478,9 @@ export function renderWorktreeTaskResult(execution, session, diff, { jobId = nul lines.push("Apply these changes to your working tree, or discard them:"); lines.push(""); const resolvedJobId = jobId ?? "JOB_ID"; - lines.push(`- **Keep**: \`node "\${CLAUDE_PLUGIN_ROOT}/scripts/codex-companion.mjs" worktree-cleanup ${resolvedJobId} --action keep\``); - lines.push(`- **Discard**: \`node "\${CLAUDE_PLUGIN_ROOT}/scripts/codex-companion.mjs" worktree-cleanup ${resolvedJobId} --action discard\``); + const cwdFlag = session.repoRoot ? ` --cwd "${session.repoRoot}"` : ""; + lines.push(`- **Keep**: \`node "\${CLAUDE_PLUGIN_ROOT}/scripts/codex-companion.mjs" worktree-cleanup ${resolvedJobId} --action keep${cwdFlag}\``); + lines.push(`- **Discard**: \`node "\${CLAUDE_PLUGIN_ROOT}/scripts/codex-companion.mjs" worktree-cleanup ${resolvedJobId} --action discard${cwdFlag}\``); return `${lines.join("\n").trimEnd()}\n`; }