feat(worktree): port worktree session core from openai#137 (step 3a/3)

plugins/codex/scripts/lib/git.mjs

@@ -297,6 +297,113 @@ 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) {
+  // 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) {
+    throw new Error(`Failed to remove worktree: ${result.stderr.trim()}`);
+  }
+}
+
+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
+  // 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) {
+    throw new Error(`Failed to diff worktree: ${result.stderr.trim()}`);
+  }
+  if (!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) {
+  gitChecked(worktreePath, ["add", "-A"]);
+  const patchPath = path.join(
+    repoRoot,
+    ".codex-worktree-" + Date.now() + "-" + Math.random().toString(16).slice(2) + ".patch"
+  );
+  try {
+    // 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?)." };
+    }
+    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);

plugins/codex/scripts/lib/process.mjs

@@ -13,14 +13,23 @@ export function runCommand(command, args = [], options = {}) {
     windowsHide: true
   });
 
+  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,
     args,
-    status: result.status ?? 0,
+    status,
     signal: result.signal ?? null,
     stdout: result.stdout ?? "",
     stderr: result.stderr ?? "",
-    error: result.error ?? null
+    error: succeeded ? null : result.error ?? null
   };
 }
 

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",

plugins/codex/scripts/lib/worktree.mjs

@@ -0,0 +1,43 @@
+import {
+  createWorktree,
+  removeWorktree,
+  deleteWorktreeBranch,
+  getWorktreeDiff,
+  applyWorktreePatch,
+  hasIgnoredChanges,
+  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;
+    }
+    // "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;
+  }
+  removeWorktree(session.repoRoot, session.worktreePath);
+  deleteWorktreeBranch(session.repoRoot, session.branch);
+  return { applied: false, detail: "Worktree discarded." };
+}

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,26 @@ 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}`);
+});
+
+// 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}`);
+});