Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
8bf675c
fix(worktrees): require human confirmation before clean-worktree.mjs …
BigSimmo Aug 21, 2026
d9f6721
fix(worktrees): detect a broken .git link at session start instead of…
BigSimmo Aug 21, 2026
a7cda27
docs: record review ledger entry for PR #2240
BigSimmo Aug 21, 2026
48b83fe
fix(worktrees): address Codex review — dry-run preview and symlink fa…
BigSimmo Aug 21, 2026
f9d64cd
Merge branch 'main' into claude/worktree-cleanup-guard
BigSimmo Aug 21, 2026
7952049
Merge branch 'main' sync into claude/worktree-cleanup-guard
BigSimmo Aug 21, 2026
a0a957c
fix(worktrees): exempt --dry-run from the removal confirmation gate
claude Aug 21, 2026
cd3ecb9
Merge branch 'main' into claude/worktree-cleanup-guard
BigSimmo Aug 21, 2026
08e2e78
Merge branch 'main' into claude/worktree-cleanup-guard
BigSimmo Aug 21, 2026
ca05fa3
Merge automated Autofix duplicate fix into claude/worktree-cleanup-guard
BigSimmo Aug 21, 2026
ced6892
Merge remote-tracking branch 'origin/main' into claude/worktree-clean…
claude Aug 21, 2026
6a62171
Merge branch 'main' into claude/worktree-cleanup-guard
BigSimmo Aug 21, 2026
ca8afd3
Merge branch 'main' into claude/worktree-cleanup-guard
BigSimmo Aug 21, 2026
89d2d19
Merge branch 'main' into claude/worktree-cleanup-guard
BigSimmo Aug 21, 2026
a57a3bd
fix(worktrees): close the programmatic bypass CodeRabbit found on PR …
BigSimmo Aug 21, 2026
3b16f53
docs: record ledger entry with decisive gate output for the CodeRabbi…
BigSimmo Aug 21, 2026
68c38b6
Merge latest origin/claude/worktree-cleanup-guard (main syncs + guard…
BigSimmo Aug 21, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion .claude/settings.json
Original file line numberDiff line numberDiff line change
Expand Up@@ -212,7 +212,11 @@
"Bash(gh auth status)",
"Bash(curl -s http://localhost:*)",
"Bash(node scripts/check-base-freshness.mjs:*)",
"Bash(node scripts/clean-worktree.mjs:*)",
"Bash(node scripts/clean-worktree.mjs --self-test)",
"Bash(node scripts/clean-worktree.mjs --merged)",
"Bash(node scripts/clean-worktree.mjs --merged --dry-run)",
"Bash(node scripts/clean-worktree.mjs --merged --squashed)",
"Bash(node scripts/clean-worktree.mjs --merged --squashed --dry-run)",
"Bash(tasklist)",
"Bash(tasklist /v)",
"mcp__Claude_Browser__navigate",
Expand Down
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
| 2026-08-21 | claude/worktree-cleanup-guard | d9f67213557eadda67e1cfc983e6948ecf57fe70 | scripts/clean-worktree.mjs, scripts/check-base-freshness.mjs, .claude/settings.json | author-implemented; PR #2240 opened | self-test, lint, typecheck, format(unchanged) |
Comment thread
BigSimmo marked this conversation as resolved.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
| 2026-08-21 | claude/worktree-cleanup-guard | a57a3bd52fdf25e5479e1e867cfbb2c7bccc9bb1 | scripts/clean-worktree.mjs, scripts/check-base-freshness.mjs | author-implemented; addressed CodeRabbit finding on PR #2240 (programmatic confirm-gate bypass, moved to assertRemovalConfirmed at both call sites) | self-test: [clean-worktree] Self-test passed successfully. \| lint: [gate-receipts] recorded a pass for lint:internal (3933 input files) \| typecheck: [gate-receipts] recorded a pass for typecheck:internal (3933 input files) \| manual bypass check: runMergedWorktreeReport({remove:true,dryRun:false}) with CLEAN_WORKTREE_CONFIRM unset now refuses before the removal loop instead of deleting |
42 changes: 42 additions & 0 deletions scripts/check-base-freshness.mjs
Original file line numberDiff line numberDiff line change
Expand Up@@ -47,6 +47,8 @@
* --strict exit 1 when the base ref cannot be resolved (default: exit 0)
*/
import { execFileSync } from "node:child_process";
import { realpathSync } from "node:fs";
import path from "node:path";

const threshold = Number.parseInt(process.env.STALE_BASE_THRESHOLD ?? "10", 10) || 10;
const asJson = process.argv.includes("--json");
Expand DownExpand Up@@ -110,6 +112,46 @@ function finish(result) {
process.exit(result.error && strict ? 1 : 0);
}

// Worktree-identity tripwire. A worktree's `.git` file can vanish (observed repeatedly
// against `.claude/worktrees/*` this week — see AGENTS.md "Worktree sweep destroys live
// work") without any command erroring: git just walks up the directory tree, finds the
// next real `.git` above it, and silently treats THAT repository as the target from then
// on — wrong branch, wrong working tree, commands that look like they succeeded. Detect
// it the same way a human would: ask git where it thinks the repo root is, and compare
// that to where Claude Code was actually told the project lives. CLAUDE_PROJECT_DIR is
// exported for every hook invocation and is the one signal available here that names the
// INTENDED worktree independent of whatever `.git` link git happens to resolve.
const expectedRoot = process.env.CLAUDE_PROJECT_DIR;
if (expectedRoot) {
const resolvedToplevel = tryGit(["rev-parse", "--show-toplevel"]);
// Canonicalize through realpath before comparing. `git rev-parse --show-toplevel` always
// returns the canonical filesystem path, but CLAUDE_PROJECT_DIR can name the same checkout
// through a symlink or junction (a container image's working-dir alias, a Dev Drive
// junction) — comparing the raw, uncanonicalized paths would then report a false "broken
// .git link" for a perfectly healthy worktree. realpath can fail (path deleted between
// hook invocation and here, permissions) — fall back to the lexical path rather than
// throwing out of an advisory tripwire.
const canonicalize = (p) => {
try {
return realpathSync(p);
} catch {
return path.resolve(p);
}
};
const normalize = (p) => (process.platform === "win32" ? canonicalize(p).toLowerCase() : canonicalize(p));
if (resolvedToplevel && normalize(resolvedToplevel) !== normalize(expectedRoot)) {
finish({
branch: "(unknown)",
error:
`this worktree's .git link is broken — git resolved the repo root as "${resolvedToplevel}" ` +
`instead of the expected "${expectedRoot}". Commands run here are silently operating on a ` +
`DIFFERENT checkout. Stop and recreate this worktree before doing any more work in it.`,
behind: 0,
ahead: 0,
});
}
}

const branch = tryGit(["rev-parse", "--abbrev-ref", "HEAD"]) ?? "(unknown)";

if (process.env.BASE_FRESHNESS_NO_FETCH !== "1") {
Expand Down
70 changes: 68 additions & 2 deletions scripts/clean-worktree.mjs
Original file line numberDiff line numberDiff line change
Expand Up@@ -364,6 +364,31 @@ export function verifyWorktreeSafetyBeforeRemove(
return { safe: true, confidence };
}

/**
* The actual confirmation gate. `--remove` is reachable from any agent session in any
* worktree on this machine (Claude Code, Codex, Antigravity/Gemini — only Claude Code's own
* permission config in .claude/settings.json can gate a Bash call before it runs, and that
* file governs nothing outside Claude Code). A worktree destroyed here has no local reflog of
* its own to recover from, so the actual deletion needs a second, tool-agnostic gate that only
* a human sets: CLEAN_WORKTREE_CONFIRM=1 in the invoking shell. No default flips this on.
*
* Called from BOTH `parseArgs` (so a CLI run without the confirm var fails fast, before it
* even lists candidates) AND `runMergedWorktreeReport` immediately before the deletion loop —
* the CLI is not the only caller: `runMergedWorktreeReport` is exported and any programmatic
* caller (a test, another script) can pass `{ remove: true, dryRun: false }` directly, bypassing
* `parseArgs` entirely. A single check living only in `parseArgs` would leave that path able to
* delete worktrees with no confirmation at all.
*/
function assertRemovalConfirmed(remove, dryRun) {
if (remove && !dryRun && process.env.CLEAN_WORKTREE_CONFIRM !== "1") {
throw new Error(
"--remove refused: set CLEAN_WORKTREE_CONFIRM=1 in your shell to confirm you (a human) reviewed the " +
"candidate list above and want these worktrees deleted. This gate exists because a worktree removed " +
"here has no reflog of its own — see AGENTS.md 'Worktree sweep destroys live work'.",
);
}
}

/**
* Parse CLI arguments for batch size and flags.
*
Expand DownExpand Up@@ -439,6 +464,11 @@ export function parseArgs(argv) {
if (remove && !merged) {
throw new Error("--remove is only valid together with --merged. Run `--merged` alone to list candidates first.");
}
// Fail fast at the CLI so a run without the confirm var doesn't even list candidates.
// `--dry-run` documents that it "wins over --remove" (see printHelp below) and never
// deletes anything, so `assertRemovalConfirmed` exempts it — the safe preflight preview an
// operator runs before setting CLEAN_WORKTREE_CONFIRM=1 for real must stay usable.
assertRemovalConfirmed(remove, dryRun);
if (squashed && !merged) {
throw new Error("--squashed is only valid together with --merged.");
}
Expand DownExpand Up@@ -691,9 +721,40 @@ export function selfTest() {
if (!args3.merged || args3.remove) {
throw new Error("selfTest failed: --merged must default to list-only (remove=false)");
}
const args4 = parseArgs(["--merged", "--remove"]);
// One outer try/finally for the whole CLEAN_WORKTREE_CONFIRM cycle: if any assertion in
// here throws (including the negative one right below), the real env var must still be
// restored before selfTest() exits, or a caller running selfTest() inside a longer-lived
// process (not a one-shot CLI invocation) inherits a deleted CLEAN_WORKTREE_CONFIRM.
const savedConfirm = process.env.CLEAN_WORKTREE_CONFIRM;
let args4;
try {
delete process.env.CLEAN_WORKTREE_CONFIRM;
let unconfirmedRemoveThrew = false;
try {
parseArgs(["--merged", "--remove"]);
} catch {
unconfirmedRemoveThrew = true;
}
if (!unconfirmedRemoveThrew) {
throw new Error("selfTest failed: --merged --remove without CLEAN_WORKTREE_CONFIRM=1 must refuse");
}

// `--dry-run` can delete nothing, so it must stay usable as a preflight preview without
// the confirm var — this was the actual P2 Codex found: the gate above fired before
// dry-run got a chance to win.
const dryRunArgs = parseArgs(["--merged", "--remove", "--dry-run"]);
if (!dryRunArgs.merged || !dryRunArgs.remove || !dryRunArgs.dryRun) {
throw new Error("selfTest failed: --merged --remove --dry-run without CLEAN_WORKTREE_CONFIRM=1 must still parse");
}

process.env.CLEAN_WORKTREE_CONFIRM = "1";
args4 = parseArgs(["--merged", "--remove"]);
} finally {
if (savedConfirm === undefined) delete process.env.CLEAN_WORKTREE_CONFIRM;
else process.env.CLEAN_WORKTREE_CONFIRM = savedConfirm;
}
if (!args4.merged || !args4.remove) {
throw new Error("selfTest failed: --merged --remove did not set both flags");
throw new Error("selfTest failed: --merged --remove (confirmed) did not set both flags");
}
let removeThrew = false;
try {
Expand DownExpand Up@@ -1043,6 +1104,11 @@ export function runMergedWorktreeReport(options = {}) {
return;
}

// Re-assert at the deletion boundary itself, not just at CLI parse time: this function is
// exported and a programmatic caller can reach here with `{ remove: true, dryRun: false }`
// without ever going through `parseArgs`.
assertRemovalConfirmed(remove, dryRun);

const batch = candidates.slice(0, batchSize);
console.log(
`\n[clean-worktree] Removing ${batch.length} of ${candidates.length} candidate(s) (batch size ${batchSize})...`,
Expand Down
Loading