From 91dafa8187982c14a57f9ff278c109282a02a34c Mon Sep 17 00:00:00 2001
From: axisrow <axisrow@users.noreply.github.com>
Date: Sun, 19 Jul 2026 14:43:49 +0800
Subject: [PATCH 1/4] feat(worktree): port worktree session core from
 openai#137 (step 3a)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Ports the isolated-git-worktree primitives that will back the rescue
--worktree flag (write-race fix, openai#135 / fork#10). This step lands the
library layer + tests; companion/markdown integration follows in 3b/3c.

From openai/codex-plugin-cc#137 (@peterdrier):
- plugins/codex/scripts/lib/worktree.mjs (NEW): createWorktreeSession /
  diffWorktreeSession / cleanupWorktreeSession — session lifecycle wrappers.
- plugins/codex/scripts/lib/git.mjs: createWorktree, removeWorktree,
  deleteWorktreeBranch, getWorktreeDiff, applyWorktreePatch (add .worktrees/
  to info/exclude, branch as codex/<ts>, diff captures untracked + uncommitted,
  keep applies patch via temp file with safe cleanup).
- plugins/codex/scripts/lib/process.mjs: suppress spurious spawnSync error on
  exit 0 (error: succeeded ? null : ...).
- plugins/codex/scripts/lib/render.mjs: renderWorktreeTaskResult +
  renderWorktreeCleanupResult (render jobId in keep/discard commands). Included
  here because tests/worktree.test.mjs imports them.
- tests/worktree.test.mjs (NEW, 12 tests): session creation, diff capture
  (uncommitted/committed/untracked), cleanup safety (keep/discard/conflict
  preservation), render output.

Fix one macOS-specific assertion (tests/worktree.test.mjs:52): git
canonicalizes repoRoot to /private/var while makeTempDir returns /var — compare
via realpathSync, same pattern as #497.

Verified: worktree.test.mjs 12/12 green; git/process/render suites green;
full npm test 153 pass / 4 pre-existing failures (unchanged, unrelated).

Refs fork#10, openai#135, openai#137.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0189cGaohsYdpkB4otKZnpAt
---
 plugins/codex/scripts/lib/git.mjs      |  67 +++++++
 plugins/codex/scripts/lib/process.mjs  |   4 +-
 plugins/codex/scripts/lib/render.mjs   |  60 ++++++
 plugins/codex/scripts/lib/worktree.mjs |  32 ++++
 tests/worktree.test.mjs                | 249 +++++++++++++++++++++++++
 5 files changed, 411 insertions(+), 1 deletion(-)
 create mode 100644 plugins/codex/scripts/lib/worktree.mjs
 create mode 100644 tests/worktree.test.mjs

diff --git a/plugins/codex/scripts/lib/git.mjs b/plugins/codex/scripts/lib/git.mjs
index c401f0ca..fe024f0d 100644
--- a/plugins/codex/scripts/lib/git.mjs
+++ b/plugins/codex/scripts/lib/git.mjs
@@ -297,6 +297,73 @@ function buildAdversarialCollectionGuidance(options = {}) {
   return "The repository context below is a lightweight summary. Inspect the target diff yourself with read-only git commands before finalizing findings.";
 }
 
+export function createWorktree(repoRoot) {
+  const ts = Date.now();
+  const worktreesDir = path.join(repoRoot, ".worktrees");
+  fs.mkdirSync(worktreesDir, { recursive: true });
+
+  // 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 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")) {
+    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();
+  gitChecked(repoRoot, ["worktree", "add", worktreePath, "-b", branch]);
+  return { worktreePath, branch, repoRoot, baseCommit, 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(worktreePath, baseCommit) {
+  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", "--cached", baseCommit]);
+  return { stat, patch: patchResult.stdout };
+}
+
+export function applyWorktreePatch(repoRoot, worktreePath, 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 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 });
+  }
+}
+
 export function collectReviewContext(cwd, target, options = {}) {
   const repoRoot = getRepoRoot(cwd);
   const currentBranch = getCurrentBranch(repoRoot);
diff --git a/plugins/codex/scripts/lib/process.mjs b/plugins/codex/scripts/lib/process.mjs
index 01fbbfe9..65419312 100644
--- a/plugins/codex/scripts/lib/process.mjs
+++ b/plugins/codex/scripts/lib/process.mjs
@@ -13,6 +13,8 @@ export function runCommand(command, args = [], options = {}) {
     windowsHide: true
   });
 
+  const succeeded = result.status === 0 && result.signal === null;
+
   return {
     command,
     args,
@@ -20,7 +22,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/plugins/codex/scripts/lib/render.mjs b/plugins/codex/scripts/lib/render.mjs
index 2ec18523..4394ed7c 100644
--- a/plugins/codex/scripts/lib/render.mjs
+++ b/plugins/codex/scripts/lib/render.mjs
@@ -445,6 +445,66 @@ export function renderStoredJobResult(job, storedJob) {
   return `${lines.join("\n").trimEnd()}\n`;
 }
 
+export function renderWorktreeTaskResult(execution, session, diff, { jobId = null } = {}) {
+  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("");
+  const resolvedJobId = jobId ?? "JOB_ID";
+  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`;
+}
+
+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 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("");
+      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}\`.`);
+  }
+
+  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 00000000..ad32c9ed
--- /dev/null
+++ b/plugins/codex/scripts/lib/worktree.mjs
@@ -0,0 +1,32 @@
+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.worktreePath, session.baseCommit);
+}
+
+export function cleanupWorktreeSession(session, { keep = false } = {}) {
+  if (keep) {
+    const result = applyWorktreePatch(session.repoRoot, session.worktreePath, session.baseCommit);
+    if (!result.applied && result.detail !== "No changes to apply.") {
+      return result;
+    }
+    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/tests/worktree.test.mjs b/tests/worktree.test.mjs
new file mode 100644
index 00000000..d7dee67a
--- /dev/null
+++ b/tests/worktree.test.mjs
@@ -0,0 +1,249 @@
+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 {
+    // macOS symlinks /var → /private/var; git canonicalizes repoRoot while
+    // makeTempDir returns the symlinked path. Compare via realpath like #497.
+    assert.equal(fs.realpathSync(session.repoRoot), fs.realpathSync(repoRoot));
+    assert.match(session.branch, /^codex\/\d+$/);
+    assert.equal(fs.realpathSync(session.worktreePath), fs.realpathSync(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 4d57e1c86ffa39973b074b6e52abfbbfdb39aaeb Mon Sep 17 00:00:00 2001
From: axisrow <axisrow@users.noreply.github.com>
Date: Sun, 19 Jul 2026 15:30:32 +0800
Subject: [PATCH 2/4] fix(worktree): close fail-open paths that lose the only
 copy of changes
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Cycle-review of PR #11 (Codex + Claude subagent) found three defects, all
leading to irreversible deletion of the only copy of worktree changes. All
verified empirically; all present in upstream openai#137 too.

1. process.mjs runCommand normalized a signal-terminated child to status:0.
   spawnSync returns status:null when the child is killed by a signal; the old
   `status: result.status ?? 0` turned that into 0, so a git-apply killed
   mid-run (or any signal-terminated git op) read as success. In worktree
   cleanup that means reporting "applied" then destroying the source worktree
   and branch even though the patch never landed. Now: signal != null => status 1.
   (Codex critical)

2. git.mjs getWorktreeDiff / applyWorktreePatch used unchecked `git add -A`. A
   failed snapshot (index lock held by a concurrent git op, disk full, perm) was
   classified identically to an empty diff => "No changes to apply" => keep-then-
   force-remove the only copy. Switched to gitChecked (fail-closed throw) and
   separated status!==0 from the genuine empty case. (Codex critical)

3. git.mjs removeWorktree matched git's "is not a working tree" stderr text.
   That message is locale-translated (ru_RU: "не является рабочим каталогом") —
   on non-English locales the guard failed and removeWorktree threw on an
   already-removed path, so cleanupWorktreeSession aborted before
   deleteWorktreeBranch and the codex/<ts> branch leaked forever. Now locale-
   safe: check `git worktree list` and no-op when the path isn't registered.
   (Claude subagent HIGH — confirmed on this machine LANG=ru_RU.UTF-8)

Regression tests:
- process.test.mjs: signal-terminated child => non-zero status.
- worktree.test.mjs: removeWorktree is a no-op when the worktree is already gone.

Verified: worktree 13/13, process 4/4, git 11/11 green; full npm test 155 pass /
4 pre-existing (unchanged, unrelated).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0189cGaohsYdpkB4otKZnpAt
---
 plugins/codex/scripts/lib/git.mjs     | 33 +++++++++++++++++++++++----
 plugins/codex/scripts/lib/process.mjs |  6 ++++-
 tests/process.test.mjs                | 14 +++++++++++-
 tests/worktree.test.mjs               | 16 ++++++++++++-
 4 files changed, 61 insertions(+), 8 deletions(-)

diff --git a/plugins/codex/scripts/lib/git.mjs b/plugins/codex/scripts/lib/git.mjs
index fe024f0d..04a6a75d 100644
--- a/plugins/codex/scripts/lib/git.mjs
+++ b/plugins/codex/scripts/lib/git.mjs
@@ -321,8 +321,22 @@ export function createWorktree(repoRoot) {
 }
 
 export function removeWorktree(repoRoot, worktreePath) {
+  // Locale-safe: git's "is not a working tree" message is translated on non-English
+  // locales (e.g. ru_RU: "не является рабочим каталогом"), so never match stderr text.
+  // If the worktree is already gone (prior cleanup, rm, crash), treat it as success.
+  const canonical = (p) => {
+    try { return fs.realpathSync(p); } catch { return path.resolve(p); }
+  };
+  const target = canonical(worktreePath);
+  const listed = git(repoRoot, ["worktree", "list", "--porcelain"]).stdout
+    .split("\n")
+    .filter((line) => line.startsWith("worktree "))
+    .map((line) => canonical(line.slice("worktree ".length).trim()));
+  if (!listed.includes(target)) {
+    return; // already removed — desired end-state reached
+  }
   const result = git(repoRoot, ["worktree", "remove", "--force", worktreePath]);
-  if (result.status !== 0 && !result.stderr.includes("is not a working tree")) {
+  if (result.status !== 0) {
     throw new Error(`Failed to remove worktree: ${result.stderr.trim()}`);
   }
 }
@@ -332,9 +346,15 @@ export function deleteWorktreeBranch(repoRoot, branch) {
 }
 
 export function getWorktreeDiff(worktreePath, baseCommit) {
-  git(worktreePath, ["add", "-A"]);
+  // gitChecked (not git): a failed snapshot (e.g. index lock held by a concurrent
+  // git op) must throw, not be classified as "no changes" — otherwise callers would
+  // treat real work as empty and discard the only copy.
+  gitChecked(worktreePath, ["add", "-A"]);
   const result = git(worktreePath, ["diff", "--cached", baseCommit, "--stat"]);
-  if (result.status !== 0 || !result.stdout.trim()) {
+  if (result.status !== 0) {
+    throw new Error(`Failed to diff worktree: ${result.stderr.trim()}`);
+  }
+  if (!result.stdout.trim()) {
     return { stat: "", patch: "" };
   }
   const stat = result.stdout.trim();
@@ -343,9 +363,12 @@ export function getWorktreeDiff(worktreePath, baseCommit) {
 }
 
 export function applyWorktreePatch(repoRoot, worktreePath, baseCommit) {
-  git(worktreePath, ["add", "-A"]);
+  gitChecked(worktreePath, ["add", "-A"]);
   const patchResult = git(worktreePath, ["diff", "--cached", baseCommit]);
-  if (patchResult.status !== 0 || !patchResult.stdout.trim()) {
+  if (patchResult.status !== 0) {
+    throw new Error(`Failed to snapshot worktree changes: ${patchResult.stderr.trim()}`);
+  }
+  if (!patchResult.stdout.trim()) {
     return { applied: false, detail: "No changes to apply." };
   }
   const patchPath = path.join(
diff --git a/plugins/codex/scripts/lib/process.mjs b/plugins/codex/scripts/lib/process.mjs
index 65419312..8e3ad341 100644
--- a/plugins/codex/scripts/lib/process.mjs
+++ b/plugins/codex/scripts/lib/process.mjs
@@ -14,11 +14,15 @@ export function runCommand(command, args = [], options = {}) {
   });
 
   const succeeded = result.status === 0 && result.signal === null;
+  // A child killed by signal has status: null. Treat that as a non-zero exit
+  // (not a success) so callers that check status===0 don't mistake a
+  // signal-terminated process (e.g. git apply killed mid-run) for success.
+  const status = result.signal !== null ? 1 : result.status ?? 0;
 
   return {
     command,
     args,
-    status: result.status ?? 0,
+    status,
     signal: result.signal ?? null,
     stdout: result.stdout ?? "",
     stderr: result.stderr ?? "",
diff --git a/tests/process.test.mjs b/tests/process.test.mjs
index c9b8390f..e2a9651b 100644
--- a/tests/process.test.mjs
+++ b/tests/process.test.mjs
@@ -1,7 +1,7 @@
 import test from "node:test";
 import assert from "node:assert/strict";
 
-import { terminateProcessTree } from "../plugins/codex/scripts/lib/process.mjs";
+import { terminateProcessTree, runCommand } from "../plugins/codex/scripts/lib/process.mjs";
 
 test("terminateProcessTree uses taskkill on Windows", () => {
   let captured = null;
@@ -84,3 +84,15 @@ test("terminateProcessTree recognizes a missing Windows process with localized o
   assert.equal(outcome.delivered, false);
   assert.equal(outcome.method, "taskkill");
 });
+
+// Regression: a child killed by signal has status:null from spawnSync. runCommand
+// must NOT normalize that to status:0 — callers checking status===0 would mistake a
+// signal-terminated process (e.g. git apply killed mid-run) for success, which for
+// worktree cleanup means deleting the only copy of unapplied changes (openai#137
+// review finding). Signal-terminated => status != 0.
+test("runCommand reports a signal-terminated child as failed (non-zero status)", () => {
+  // `node -e 'process.kill(process.pid, "SIGKILL")'` terminates itself with a signal.
+  const result = runCommand(process.execPath, ["-e", "process.kill(process.pid, 'SIGKILL')"]);
+  assert.ok(result.signal !== null, `expected a signal, got signal=${result.signal}`);
+  assert.notEqual(result.status, 0, `signal-terminated child must not report status 0, got ${result.status}`);
+});
diff --git a/tests/worktree.test.mjs b/tests/worktree.test.mjs
index d7dee67a..86057098 100644
--- a/tests/worktree.test.mjs
+++ b/tests/worktree.test.mjs
@@ -8,7 +8,7 @@ import {
   diffWorktreeSession,
   cleanupWorktreeSession
 } from "../plugins/codex/scripts/lib/worktree.mjs";
-import { getWorktreeDiff } from "../plugins/codex/scripts/lib/git.mjs";
+import { getWorktreeDiff, removeWorktree } from "../plugins/codex/scripts/lib/git.mjs";
 import { renderWorktreeTaskResult } from "../plugins/codex/scripts/lib/render.mjs";
 import { initGitRepo, makeTempDir, run } from "./helpers.mjs";
 
@@ -247,3 +247,17 @@ test("renderWorktreeTaskResult falls back to JOB_ID when jobId is null", () => {
   assert.match(output, /worktree-cleanup JOB_ID --action keep/);
   assert.match(output, /worktree-cleanup JOB_ID --action discard/);
 });
+
+// Regression: removeWorktree must not throw when the worktree is already gone
+// (prior cleanup, rm, crash). The old guard matched git's "is not a working tree"
+// stderr text, which is locale-translated (ru_RU: "не является рабочим каталогом")
+// — on non-English locales it threw, leaking the codex/<ts> branch forever.
+// Locale-safe: check git worktree list, no-op if the path is not registered.
+test("removeWorktree is a no-op (no throw) when the worktree is already removed", () => {
+  const { repoRoot } = createRepoWithInitialCommit();
+  const session = createWorktreeSession(repoRoot);
+  // Remove it once via the session helper path, then call removeWorktree again
+  // against the now-gone path — must not throw.
+  removeWorktree(repoRoot, session.worktreePath);
+  assert.doesNotThrow(() => removeWorktree(repoRoot, session.worktreePath));
+});

From 5748958d7dd9e6812ce746bf83249232d6c1fb1e Mon Sep 17 00:00:00 2001
From: axisrow <axisrow@users.noreply.github.com>
Date: Sun, 19 Jul 2026 15:58:25 +0800
Subject: [PATCH 3/4] fix(worktree): spawn-failure status + byte-preserving
 binary patch
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Cycle-2 review of PR #11 (Codex found two more fail-open paths; Claude subagent
verified the cycle-1 fixes clean but missed these). Both confirmed empirically.

1. process.mjs runCommand still normalized a spawn failure to status:0. The
   cycle-1 fix covered `signal !== null` but not `error` (ENOENT/EAGAIN/
   maxBuffer — spawnSync returns status:null, signal:null, error:set). A
   missing/failed git op still read as success (status ?? 0 → 0), so worktree
   cleanup could destroy the only copy. Now: `failed = signal != null || error`;
   failed => status 1. (Codex critical)

2. git.mjs applyWorktreePatch round-tripped the patch through a JS string.
   runCommand decodes stdout as UTF-8, replacing invalid bytes (0xff, NUL,
   legacy encodings) with U+FFFD; git apply then applied the corrupted patch
   as "success" and the source worktree was destroyed — silent data loss for
   any binary or non-UTF-8 file Codex touched. Now byte-preserving: write the
   patch via `git diff --cached --binary --output=<file>` (git→file, no JS
   string), detect empty by file size, apply. (Codex critical)

Regression tests:
- process.test.mjs: ENOENT spawn failure => non-zero status.
- worktree.test.mjs: keep round-trips a raw 0xff byte without U+FFFD corruption.

Verified: worktree 14/14, process 5/5, git 11/11 green; full npm test 157 pass /
4 pre-existing (unchanged, unrelated).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0189cGaohsYdpkB4otKZnpAt
---
 plugins/codex/scripts/lib/git.mjs     | 18 +++++++++--------
 plugins/codex/scripts/lib/process.mjs | 13 ++++++++-----
 tests/process.test.mjs                | 11 +++++++++++
 tests/worktree.test.mjs               | 28 +++++++++++++++++++++++++++
 4 files changed, 57 insertions(+), 13 deletions(-)

diff --git a/plugins/codex/scripts/lib/git.mjs b/plugins/codex/scripts/lib/git.mjs
index 04a6a75d..48a61886 100644
--- a/plugins/codex/scripts/lib/git.mjs
+++ b/plugins/codex/scripts/lib/git.mjs
@@ -364,19 +364,21 @@ export function getWorktreeDiff(worktreePath, baseCommit) {
 
 export function applyWorktreePatch(repoRoot, worktreePath, baseCommit) {
   gitChecked(worktreePath, ["add", "-A"]);
-  const patchResult = git(worktreePath, ["diff", "--cached", baseCommit]);
-  if (patchResult.status !== 0) {
-    throw new Error(`Failed to snapshot worktree changes: ${patchResult.stderr.trim()}`);
-  }
-  if (!patchResult.stdout.trim()) {
-    return { applied: false, detail: "No changes to apply." };
-  }
   const patchPath = path.join(
     repoRoot,
     ".codex-worktree-" + Date.now() + "-" + Math.random().toString(16).slice(2) + ".patch"
   );
   try {
-    fs.writeFileSync(patchPath, patchResult.stdout, "utf8");
+    // Write the patch via `git diff --output=<file>` so the bytes flow directly
+    // git→file, never through a JS string. runCommand decodes stdout as UTF-8,
+    // which would replace invalid bytes (0xff, NUL, legacy encodings) with
+    // U+FFFD and then apply a corrupted patch as "success" — destroying the
+    // only correct copy in the worktree. Detect empty via file size instead.
+    const diffResult = gitChecked(worktreePath, ["diff", "--cached", "--binary", baseCommit, "--output", patchPath]);
+    void diffResult;
+    if (!fs.existsSync(patchPath) || fs.statSync(patchPath).size === 0) {
+      return { applied: false, detail: "No changes to apply." };
+    }
     const applyResult = git(repoRoot, ["apply", "--index", patchPath]);
     if (applyResult.status !== 0) {
       return { applied: false, detail: applyResult.stderr.trim() || "Patch apply failed (conflicts?)." };
diff --git a/plugins/codex/scripts/lib/process.mjs b/plugins/codex/scripts/lib/process.mjs
index 8e3ad341..a52b94da 100644
--- a/plugins/codex/scripts/lib/process.mjs
+++ b/plugins/codex/scripts/lib/process.mjs
@@ -13,11 +13,14 @@ export function runCommand(command, args = [], options = {}) {
     windowsHide: true
   });
 
-  const succeeded = result.status === 0 && result.signal === null;
-  // A child killed by signal has status: null. Treat that as a non-zero exit
-  // (not a success) so callers that check status===0 don't mistake a
-  // signal-terminated process (e.g. git apply killed mid-run) for success.
-  const status = result.signal !== null ? 1 : result.status ?? 0;
+  const succeeded = result.status === 0 && result.signal === null && !result.error;
+  // A child that was killed by a signal OR failed to spawn (ENOENT, EAGAIN,
+  // maxBuffer) returns status:null. Treat any of those as a non-zero exit so
+  // callers that check status===0 don't mistake an abnormal git op (killed
+  // mid-run, missing binary) for success — for worktree cleanup that would mean
+  // deleting the only copy of unapplied changes.
+  const failed = result.signal !== null || result.error;
+  const status = failed ? 1 : result.status ?? 0;
 
   return {
     command,
diff --git a/tests/process.test.mjs b/tests/process.test.mjs
index e2a9651b..cc6367b6 100644
--- a/tests/process.test.mjs
+++ b/tests/process.test.mjs
@@ -96,3 +96,14 @@ test("runCommand reports a signal-terminated child as failed (non-zero status)",
   assert.ok(result.signal !== null, `expected a signal, got signal=${result.signal}`);
   assert.notEqual(result.status, 0, `signal-terminated child must not report status 0, got ${result.status}`);
 });
+
+// Regression: a child that fails to spawn (ENOENT — missing binary) returns
+// status:null, signal:null, error:ENOENT from spawnSync. runCommand must NOT
+// normalize that to status:0 — callers checking status===0 would mistake a
+// missing/failed git op for success, which for worktree cleanup destroys the
+// only copy of unapplied changes (openai#137 cycle-2 review finding).
+test("runCommand reports a spawn failure (ENOENT) as failed (non-zero status)", () => {
+  const result = runCommand("/nonexistent-codex-binary-xyz", []);
+  assert.ok(result.error, `expected an error, got ${result.error}`);
+  assert.notEqual(result.status, 0, `spawn failure must not report status 0, got ${result.status}`);
+});
diff --git a/tests/worktree.test.mjs b/tests/worktree.test.mjs
index 86057098..fa59f054 100644
--- a/tests/worktree.test.mjs
+++ b/tests/worktree.test.mjs
@@ -261,3 +261,31 @@ test("removeWorktree is a no-op (no throw) when the worktree is already removed"
   removeWorktree(repoRoot, session.worktreePath);
   assert.doesNotThrow(() => removeWorktree(repoRoot, session.worktreePath));
 });
+
+// Regression: the keep patch path must be byte-preserving. runCommand decodes
+// stdout as UTF-8, which corrupts invalid bytes (0xff, NUL, legacy encodings)
+// into U+FFFD. applyWorktreePatch now writes the patch via `git diff --output`
+// (git→file, no JS string), so binary/non-UTF-8 changes round-trip exactly —
+// otherwise keep would apply a corrupted patch as "success" then destroy the
+// only correct copy in the worktree (openai#137 cycle-2 review finding).
+test("cleanupWorktreeSession keep round-trips a non-UTF-8 byte (0xff) without corruption", () => {
+  const { repoRoot } = createRepoWithInitialCommit();
+  const session = createWorktreeSession(repoRoot);
+
+  try {
+    // Write a file containing a raw 0xff byte in the worktree.
+    fs.writeFileSync(path.join(session.worktreePath, "binary.bin"), Buffer.from([0xff, 0x00, 0x41, 0xff]));
+
+    const result = cleanupWorktreeSession(session, { keep: true });
+    assert.equal(result.applied, true);
+
+    const applied = fs.readFileSync(path.join(repoRoot, "binary.bin"));
+    assert.deepEqual(
+      Array.from(applied),
+      [0xff, 0x00, 0x41, 0xff],
+      `binary bytes must round-trip exactly; got ${Array.from(applied)} (U+FFFD corruption would show 0xEF 0xBF 0xBD)`
+    );
+  } finally {
+    cleanupSession(session);
+  }
+});

From 3e42783ac4159739551834e940d81240991505da Mon Sep 17 00:00:00 2001
From: axisrow <axisrow@users.noreply.github.com>
Date: Sun, 19 Jul 2026 20:13:45 +0800
Subject: [PATCH 4/4] fix(worktree): preserve ignored-only worktree changes (no
 silent loss)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Cycle-3 review (Codex, final cycle) found a fourth data-loss path in the same
fail-open class. `git add -A` skips git-ignored paths (.env, dist/, .output,
build artifacts) — so a worktree whose only changes landed in ignored files
produces an empty patch. cleanupWorktreeSession then returned "No changes to
apply" and force-removed the worktree + branch, irreversibly destroying the
artifacts. Confirmed empirically; this very repo ignores exactly .env/dist/
.output — paths Codex/rescue writes to routinely.

Fix: when keep produces an empty tracked patch, check hasIgnoredChanges()
(`git status --porcelain --ignored -uall`, !! entries). If ignored work exists,
preserve the worktree and branch and surface the situation in the result
detail instead of removing. Fail-safe: never lose work silently. If the user
wants those ignored files gone, they remove the worktree explicitly.

Regression test: worktree.test.mjs "keep preserves the worktree when only
ignored files changed" — creates dist/artifact.txt in an ignored worktree,
asserts keep returns applied:false + "ignored files" message AND the worktree
+ artifact survive.

Verified: worktree 15/15, process 5/5, git 11/11 green; full npm test 158 pass
/ 4 pre-existing (unchanged).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0189cGaohsYdpkB4otKZnpAt
---
 plugins/codex/scripts/lib/git.mjs      | 15 ++++++++++++
 plugins/codex/scripts/lib/worktree.mjs | 11 +++++++++
 tests/worktree.test.mjs                | 33 ++++++++++++++++++++++++++
 3 files changed, 59 insertions(+)

diff --git a/plugins/codex/scripts/lib/git.mjs b/plugins/codex/scripts/lib/git.mjs
index 48a61886..e25f884e 100644
--- a/plugins/codex/scripts/lib/git.mjs
+++ b/plugins/codex/scripts/lib/git.mjs
@@ -345,6 +345,21 @@ export function deleteWorktreeBranch(repoRoot, branch) {
   git(repoRoot, ["branch", "-D", branch]);
 }
 
+// Detect uncommitted work that `git add -A` skipped because it is git-ignored
+// (e.g. .env, dist/, build artifacts Codex wrote). A worktree whose only changes
+// are ignored would produce an empty patch and be mistaken for "no changes" —
+// force-removing it would irreversibly destroy that work. Returns true if any
+// ignored file in the worktree differs from the base worktree state.
+export function hasIgnoredChanges(worktreePath) {
+  // --porcelain: !! marks ignored entries. Only ignored files that exist as
+  // working-tree content count; an empty worktree (fresh from HEAD) lists none.
+  const result = git(worktreePath, ["status", "--porcelain", "--ignored", "-uall"]);
+  if (result.status !== 0) {
+    return false;
+  }
+  return result.stdout.split("\n").some((line) => line.startsWith("!! ") && line.length > 3);
+}
+
 export function getWorktreeDiff(worktreePath, baseCommit) {
   // gitChecked (not git): a failed snapshot (e.g. index lock held by a concurrent
   // git op) must throw, not be classified as "no changes" — otherwise callers would
diff --git a/plugins/codex/scripts/lib/worktree.mjs b/plugins/codex/scripts/lib/worktree.mjs
index ad32c9ed..9b6feab0 100644
--- a/plugins/codex/scripts/lib/worktree.mjs
+++ b/plugins/codex/scripts/lib/worktree.mjs
@@ -4,6 +4,7 @@ import {
   deleteWorktreeBranch,
   getWorktreeDiff,
   applyWorktreePatch,
+  hasIgnoredChanges,
   ensureGitRepository
 } from "./git.mjs";
 
@@ -22,6 +23,16 @@ export function cleanupWorktreeSession(session, { keep = false } = {}) {
     if (!result.applied && result.detail !== "No changes to apply.") {
       return result;
     }
+    // "No changes to apply" via `git add -A` skips git-ignored paths. If the
+    // worktree holds ignored-only work (e.g. Codex wrote .env, dist/, build
+    // artifacts), force-removing it would irreversibly destroy it. Preserve the
+    // worktree + branch and surface the situation instead. (openai#137 review.)
+    if (!result.applied && hasIgnoredChanges(session.worktreePath)) {
+      return {
+        applied: false,
+        detail: `No tracked changes to apply, but the worktree has ignored files (e.g. .env, dist/) at ${session.worktreePath}. Preserving the worktree and branch ${session.branch} — inspect and copy them manually before removing.`
+      };
+    }
     removeWorktree(session.repoRoot, session.worktreePath);
     deleteWorktreeBranch(session.repoRoot, session.branch);
     return result;
diff --git a/tests/worktree.test.mjs b/tests/worktree.test.mjs
index fa59f054..df47ddf9 100644
--- a/tests/worktree.test.mjs
+++ b/tests/worktree.test.mjs
@@ -289,3 +289,36 @@ test("cleanupWorktreeSession keep round-trips a non-UTF-8 byte (0xff) without co
     cleanupSession(session);
   }
 });
+
+// Regression: "No changes to apply" must not force-remove a worktree that holds
+// ignored-only work. `git add -A` skips git-ignored paths (.env, dist/, build
+// artifacts), so an ignored-only worktree produces an empty patch. Before the
+// fix, keep would force-remove it and lose the artifacts irreversibly. Now the
+// worktree + branch are preserved and the result surfaces the situation.
+test("cleanupWorktreeSession keep preserves the worktree when only ignored files changed", () => {
+  const { repoRoot } = createRepoWithInitialCommit();
+  // Make this test repo ignore dist/ so the worktree's artifact is ignored.
+  fs.writeFileSync(path.join(repoRoot, ".gitignore"), "dist/\n");
+  assert.equal(run("git", ["add", ".gitignore"], { cwd: repoRoot }).status, 0);
+  assert.equal(run("git", ["commit", "-m", "gitignore"], { cwd: repoRoot }).status, 0);
+
+  const session = createWorktreeSession(repoRoot);
+
+  try {
+    fs.mkdirSync(path.join(session.worktreePath, "dist"), { recursive: true });
+    fs.writeFileSync(path.join(session.worktreePath, "dist", "artifact.txt"), "precious\n");
+
+    const result = cleanupWorktreeSession(session, { keep: true });
+
+    assert.equal(result.applied, false);
+    assert.match(result.detail, /ignored files/);
+    // The worktree and its ignored artifact MUST be preserved.
+    assert.equal(fs.existsSync(session.worktreePath), true);
+    assert.equal(
+      fs.readFileSync(path.join(session.worktreePath, "dist", "artifact.txt"), "utf8"),
+      "precious\n"
+    );
+  } finally {
+    cleanupSession(session);
+  }
+});