From 6f0d100accc70129f5f8bfe43c0dc641989ce2a2 Mon Sep 17 00:00:00 2001 From: BigSimmo <87357024+BigSimmo@users.noreply.github.com> Date: Tue, 18 Aug 2026 17:56:22 +0800 Subject: [PATCH 1/7] fix(claude): make the session-start hook runnable and give the agent config teeth MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `.claude/hooks/session-start.sh` was committed as mode 100644 while both its siblings were 100755, and it is the only hook registered by bare path rather than through `bash`. Its whole body is gated on CLAUDE_CODE_REMOTE=true, so the one environment it does any work in is a Linux web container — exactly where a non-executable checkout cannot run. It is also the script that provisions the Node 24 the engine floor requires, after npm ci EBADENGINE blocked PRs #1611, #1697, #1705 and #1740. The defect was invisible locally: the primary workstation is a Windows ReFS Dev Drive with core.fileMode=false, so git ignores filesystem permission bits and a local `chmod +x` is a silent no-op. Only `git update-index --chmod=+x` can fix it. Fixed three ways so it cannot recur: the index mode, a `bash "..."` registration that stops the mode being load-bearing, and a contract test. Also in this change: - .claude/settings.json gains a permissions block. AGENTS.md's provider confirmation boundary was prose-only; this encodes it as deny/ask rules, including deny on reading .env* (a staging key leaked on 2026-08-18) and on the Supabase MCP write tools. The repo already learned that prose does not hold here — see the comment in pr-handoff-stop.sh. - check-base-freshness.mjs now emits its stale-base warning on stdout as hook JSON. Every human-readable branch used console.error, and Claude Code injects only stdout into context, so the tripwire never reached the agent. The origin/main fetch also gains a 10s timeout so a hung remote cannot burn the whole SessionStart budget. - clean-worktree.mjs gains list-only `--merged` and `--squashed`. Nothing reclaimed merged worktrees, so 49 accumulated, ~19 GB of duplicated node_modules on a 50 GB Dev Drive. Ancestor detection alone finds 2 of 49 because this repo squash-merges; the patch-id test finds 9. A `confidence:` line distinguishes proven from inferred, because the two are not the same claim and one candidate had 2 of 21 files still differing. - Explicit hook timeouts, a PreCompact hook that asks for /issues capture while the context still exists, and a push format guard that only fires where the .githooks pre-push guard is not wired. Removal stays a separate opt-in throughout; `runWorktreeCleanup()` is byte identical, so verify:preflight is unaffected. Co-Authored-By: Claude Opus 5 --- .claude/hooks/precompact-issues-capture.sh | 46 ++ .claude/hooks/push-format-guard.sh | 101 ++++ .claude/hooks/session-start.sh | 0 .claude/settings.json | 142 ++++- .claude/skills/newtask/SKILL.md | 32 +- AGENTS.md | 33 ++ scripts/check-base-freshness.mjs | 70 ++- scripts/clean-worktree.mjs | 580 ++++++++++++++++++++- tests/session-start-hook.test.ts | 68 +++ 9 files changed, 1059 insertions(+), 13 deletions(-) create mode 100755 .claude/hooks/precompact-issues-capture.sh create mode 100755 .claude/hooks/push-format-guard.sh mode change 100644 => 100755 .claude/hooks/session-start.sh diff --git a/.claude/hooks/precompact-issues-capture.sh b/.claude/hooks/precompact-issues-capture.sh new file mode 100755 index 0000000000..376b32917b --- /dev/null +++ b/.claude/hooks/precompact-issues-capture.sh @@ -0,0 +1,46 @@ +#!/usr/bin/env bash +# PreCompact hook — ask for /issues capture BEFORE the context is discarded. +# +# The problem this closes: `.claude/hooks/issues-surface.sh` already prints a +# "run /issues capture" reminder, but it is a SessionStart hook, so on a +# `compact` trigger it fires *after* compaction has already happened. By then +# the in-flight follow-ups, deferrals and half-formed risks it wants recorded +# are exactly what was just summarised away. The reminder arrives after the +# thing it is trying to save is gone. +# +# PreCompact fires while that material is still in context, which is the only +# moment the reminder can actually be acted on. +# +# KNOWN LIMIT — read before trusting this. Claude Code injects hook stdout into +# the model's context for SessionStart / UserPromptSubmit / PreToolUse / +# PostToolUse. Whether it does so for PreCompact is NOT verified here, and could +# not be verified offline. So this hook deliberately prints plain human text +# rather than a hookSpecificOutput JSON envelope: if the platform does inject +# it, the text is useful as-is; if it does not, the operator still sees a clean, +# readable transcript line rather than a raw JSON blob. Either way the +# SessionStart reminder in issues-surface.sh remains the backstop, so nothing +# regresses if this turns out to be transcript-only. Re-check when the hook +# reference documents PreCompact context injection. +# +# Contract: READ-ONLY and always exits 0. It never writes the ledger, never +# commits, and must never be able to fail a compaction. +set -uo pipefail + +payload="$(cat 2>/dev/null || true)" + +# `trigger` is "manual" (the user ran /compact) or "auto" (the context window +# filled). Both lose the same material; the wording differs only so the operator +# can tell which one they are looking at. +trigger="$(printf '%s' "$payload" \ + | grep -o '"trigger"[[:space:]]*:[[:space:]]*"[^"]*"' \ + | head -n1 | sed -E 's/.*"([^"]*)"$/\1/')" + +case "$trigger" in +manual) why="This compaction was requested manually." ;; +auto) why="The context window filled, so this compaction was automatic." ;; +*) why="This session is about to be compacted." ;; +esac + +echo "[issues] ${why} Anything this session discovered but has not written down is about to be summarised away: unresolved follow-ups, deferrals, known risks, and work you decided NOT to do and why. Record them now with /issues add … (or /issues capture for a sweep) — docs/outstanding-issues.md is the only memory that survives a context reset. Requests land as immutable files under docs/outstanding-issues-inbox/; they are not committed unless you are explicitly asked to commit." + +exit 0 diff --git a/.claude/hooks/push-format-guard.sh b/.claude/hooks/push-format-guard.sh new file mode 100755 index 0000000000..ee5e40de0b --- /dev/null +++ b/.claude/hooks/push-format-guard.sh @@ -0,0 +1,101 @@ +#!/usr/bin/env bash +# PreToolUse — block `git push` when the tree is not Prettier-clean. +# +# Why this exists at all, given .githooks/pre-push already checks formatting: +# `core.hooksPath` is set by *this checkout's* `npm install`. An agent session +# that pushes from an environment where that never ran — Claude Code on the web, +# a fresh container, a worktree created without an install — bypasses the git +# hook entirely, and only CI catches the break. AGENTS.md records three CI +# failures on 2026-07-30 from exactly this, two of them on a file the author had +# not edited (a per-file `prettier --check` passed while the repository-wide +# check failed). +# +# So this hook deliberately does NOT duplicate the git hook. It runs ONLY when +# the git hook is absent or not wired to this repo's .githooks directory — i.e. +# exactly the gap case. In a normally installed checkout it exits in +# milliseconds having done nothing, and `guard-push.mjs` remains the real gate +# (it is stricter: it checks the *pushed commit* in an isolated worktree, not +# the working tree, which is a check this hook cannot perform). +# +# Escape hatch: prefix the command with CLAUDE_ALLOW_UNFORMATTED_PUSH=1. +# +# Contract: never fails a tool call by accident. Any parse problem, missing +# dependency, or unexpected state exits 0 with no decision, leaving the call +# exactly as it was. Failing open is correct here — the git hook and CI are both +# still downstream. +set -uo pipefail + +payload="$(cat 2>/dev/null || true)" +[ -z "$payload" ] && exit 0 + +# --- extract the command ------------------------------------------------------ +if command -v jq >/dev/null 2>&1; then + tool_name="$(printf '%s' "$payload" | jq -r '.tool_name // empty' 2>/dev/null || true)" + command_text="$(printf '%s' "$payload" | jq -r ' + .tool_input.command // .tool_input.script // .tool_input.code // empty + ' 2>/dev/null || true)" +else + tool_name="$(printf '%s' "$payload" \ + | grep -o '"tool_name"[[:space:]]*:[[:space:]]*"[^"]*"' \ + | head -n1 | sed -E 's/.*"([^"]*)"$/\1/')" + # Quote-naive extraction truncates at the first escaped quote, so fall back to + # the whole payload for matching. Over-matching is the safe direction here: the + # worst case is one extra Prettier run on a command that merely mentions a push. + command_text="$(printf '%s' "$payload" \ + | grep -o '"command"[[:space:]]*:[[:space:]]*"[^"]*"' \ + | head -n1 | sed -E 's/.*"([^"]*)"$/\1/')" + [ -z "$command_text" ] && command_text="$payload" +fi + +case "$tool_name" in +"" | Bash | PowerShell) ;; +*) exit 0 ;; +esac + +# --- is this a push? ---------------------------------------------------------- +printf '%s' "$command_text" | grep -Eq '(^|[;&|[:space:]])git[[:space:]]+push([[:space:]]|$)' || exit 0 + +# --- documented escape hatch (leading prefix only, not an incidental mention) -- +printf '%s' "$command_text" \ + | grep -Eq '^[[:space:]]*CLAUDE_ALLOW_UNFORMATTED_PUSH=1([[:space:]]|$)' && exit 0 + +# --- only act in the gap case: the repo's pre-push hook is not wired ---------- +repo_root="${CLAUDE_PROJECT_DIR:-$(git rev-parse --show-toplevel 2>/dev/null || true)}" +[ -z "$repo_root" ] && exit 0 +hooks_path="$(git -C "$repo_root" config --get core.hooksPath 2>/dev/null || true)" +if [ -n "$hooks_path" ]; then + # Normalise Windows backslashes so a native `git config` value compares equal + # to the POSIX path this script sees. + normalised="$(printf '%s' "$hooks_path" | tr '\\' '/')" + case "$normalised" in + */.githooks) + if [ -x "$repo_root/.githooks/pre-push" ]; then + exit 0 + fi + ;; + esac +fi + +# --- run the repository-wide check, never a per-file one --------------------- +# Per-file is not the repository-wide check: on 2026-07-30 a doc/ledger edit in +# the same push was the missed file twice out of three, while `prettier --check` +# on the edited source file passed. +command -v npx >/dev/null 2>&1 || exit 0 +[ -d "$repo_root/node_modules/prettier" ] || exit 0 + +if unformatted="$(cd "$repo_root" && npx --no-install prettier --check . 2>&1)"; then + exit 0 +fi + +json_escape() { + printf '%s' "$1" \ + | tr '\n\r\t' ' ' \ + | tr -d '\000-\010\013\014\016-\037' \ + | sed -e 's/\\/\\\\/g' -e 's/"/\\"/g' +} + +offenders="$(printf '%s' "$unformatted" | grep -E '^\[warn\] ' | head -n 8 | sed 's/^\[warn\] //' | tr '\n' ' ')" +reason="Blocked: this push would land unformatted files, and this checkout has no wired .githooks/pre-push guard to catch it, so CI would be the first thing to fail (AGENTS.md records three such CI failures on 2026-07-30). Unformatted: ${offenders:-see prettier output}. Fix with: npm run format — then COMMIT the result, because a push sends commits and not your working tree, so formatting after committing leaves the unformatted blob on the branch. Override for this one command with the CLAUDE_ALLOW_UNFORMATTED_PUSH=1 prefix." + +printf '{"hookSpecificOutput":{"hookEventName":"PreToolUse","permissionDecision":"deny","permissionDecisionReason":"%s"}}\n' "$(json_escape "$reason")" +exit 0 diff --git a/.claude/hooks/session-start.sh b/.claude/hooks/session-start.sh old mode 100644 new mode 100755 diff --git a/.claude/settings.json b/.claude/settings.json index 8073de3b64..28fa1288fd 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -1,19 +1,139 @@ { + "permissions": { + "deny": [ + "Read(./.env)", + "Read(./.env.local)", + "Read(./.env.*.local)", + "Read(./.env.production)", + "Read(./.env.staging)", + "Bash(git push --force:*)", + "Bash(git push -f:*)", + "mcp__supabase__execute_sql", + "mcp__supabase__apply_migration", + "mcp__supabase__deploy_edge_function", + "mcp__supabase__create_project", + "mcp__supabase__create_branch", + "mcp__supabase__delete_branch", + "mcp__supabase__merge_branch", + "mcp__supabase__reset_branch", + "mcp__supabase__rebase_branch", + "mcp__supabase__pause_project", + "mcp__supabase__restore_project" + ], + "ask": [ + "Bash(npm run eval:*)", + "Bash(npm run test:live)", + "Bash(npm run test:live:*)", + "Bash(npm run test:cross-tenant:staging)", + "Bash(npm run verify:release)", + "Bash(npm run verify:release:*)", + "Bash(npm run check:supabase-project)", + "Bash(npm run check:production-readiness)", + "Bash(npm run check:production-readiness:*)", + "Bash(npm run check:github-shell-access:live)", + "Bash(npm run sync:pr-branches)", + "Bash(npm run sync:pr-branches:*)", + "Bash(npm run reindex)", + "Bash(npm run reindex:*)", + "Bash(npm run import:docs)", + "Bash(npm run import:docs:*)", + "Bash(npm run enrich:*)", + "Bash(npm run classify:documents)", + "Bash(npm run governance:release)", + "Bash(npm run audit:source-governance:release)", + "Bash(gh api:*)", + "Bash(git push:*)", + "Bash(git fetch:*)", + "Bash(railway:*)", + "mcp__railway__set-variables", + "mcp__railway__redeploy", + "mcp__railway__create-deployment", + "mcp__railway__accept-deploy", + "mcp__railway__update-service", + "mcp__railway__create-service", + "mcp__railway__create-project", + "mcp__railway__set-feature-flag", + "mcp__railway__delete-feature-flag", + "mcp__railway__generate-domain", + "mcp__railway__railway-agent" + ], + "allow": [ + "Bash(npm run lint)", + "Bash(npm run lint:internal)", + "Bash(npm run typecheck)", + "Bash(npm run typecheck:source)", + "Bash(npm run format)", + "Bash(npm run format:check)", + "Bash(npm run format:changed)", + "Bash(npm run test)", + "Bash(npm run test:focused)", + "Bash(npm run test:focused --*)", + "Bash(npm run verify:cheap)", + "Bash(npm run verify:pr-local)", + "Bash(npm run verify:pr-local --*)", + "Bash(npm run verify:phone-chrome)", + "Bash(npm run ensure)", + "Bash(npm run skills)", + "Bash(npm run docs:check-index)", + "Bash(npm run docs:check-inventory)", + "Bash(npm run docs:check-links)", + "Bash(npm run docs:check-scripts)", + "Bash(npm run docs:update)", + "Bash(npm run sitemap:check)", + "Bash(npm run sitemap:update)", + "Bash(npm run check:base-freshness)", + "Bash(npm run check:runtime)", + "Bash(npm run check:installed-lock-parity)", + "Bash(npm run check:skills)", + "Bash(npm run check:gate-manifest)", + "Bash(npm run check:pr-policy)", + "Bash(npm run check:ci-scope)", + "Bash(npm run check:branch-review-ledger)", + "Bash(npm run check:outstanding-issues)", + "Bash(npm run check:ledger-write-discipline)", + "Bash(npm run ledger:lookup --*)", + "Bash(npm run issues:report --*)", + "Bash(git status:*)", + "Bash(git diff:*)", + "Bash(git log:*)", + "Bash(git show:*)", + "Bash(git branch:*)", + "Bash(git rev-parse:*)", + "Bash(git merge-base:*)", + "Bash(git worktree list:*)", + "Bash(node scripts/check-base-freshness.mjs:*)", + "Bash(node scripts/clean-worktree.mjs:*)" + ] + }, "hooks": { "SessionStart": [ { "hooks": [ { "type": "command", - "command": "$CLAUDE_PROJECT_DIR/.claude/hooks/session-start.sh" + "command": "bash \"$CLAUDE_PROJECT_DIR/.claude/hooks/session-start.sh\"", + "timeout": 900 }, { "type": "command", - "command": "node \"$CLAUDE_PROJECT_DIR/scripts/check-base-freshness.mjs\"" + "command": "node \"$CLAUDE_PROJECT_DIR/scripts/check-base-freshness.mjs\" --hook", + "timeout": 30 }, { "type": "command", - "command": "bash \"$CLAUDE_PROJECT_DIR/.claude/hooks/issues-surface.sh\"" + "command": "bash \"$CLAUDE_PROJECT_DIR/.claude/hooks/issues-surface.sh\"", + "timeout": 30 + } + ] + } + ], + "PreCompact": [ + { + "hooks": [ + { + "type": "command", + "command": "bash \"$CLAUDE_PROJECT_DIR/.claude/hooks/precompact-issues-capture.sh\"", + "timeout": 15 } ] } @@ -24,7 +144,8 @@ "hooks": [ { "type": "command", - "command": "bash \"$CLAUDE_PROJECT_DIR/.claude/hooks/pr-handoff-stop.sh\" post" + "command": "bash \"$CLAUDE_PROJECT_DIR/.claude/hooks/pr-handoff-stop.sh\" post", + "timeout": 15 } ] } @@ -35,7 +156,18 @@ "hooks": [ { "type": "command", - "command": "bash \"$CLAUDE_PROJECT_DIR/.claude/hooks/pr-handoff-stop.sh\" pre" + "command": "bash \"$CLAUDE_PROJECT_DIR/.claude/hooks/pr-handoff-stop.sh\" pre", + "timeout": 15 + } + ] + }, + { + "matcher": "Bash|PowerShell", + "hooks": [ + { + "type": "command", + "command": "bash \"$CLAUDE_PROJECT_DIR/.claude/hooks/push-format-guard.sh\"", + "timeout": 180 } ] } diff --git a/.claude/skills/newtask/SKILL.md b/.claude/skills/newtask/SKILL.md index 4990c3d836..332ebcb36c 100644 --- a/.claude/skills/newtask/SKILL.md +++ b/.claude/skills/newtask/SKILL.md @@ -45,15 +45,36 @@ almost always has a pushed branch named after it. Report which check you used. O 1. **Sync main.** `git fetch --quiet origin main`. 2. **Create an isolated worktree** off the latest main (never reuse another session's - checkout, and never switch the main checkout's branch): + checkout, and never switch the main checkout's branch). Put it under the project's + own worktree root on the **D: Dev Drive**, which is where every Claude Code worktree + already lives — not `../wt-`, which lands outside the project tree, and never + under `C:\Users\…`: + ```bash - git worktree add -b claude/ ../wt- origin/main + git worktree add -b claude/ D:/Repos/Database/.claude/worktrees/ origin/main ``` + Use a short, descriptive ``. + + **Check free space first — this is a real constraint, not a formality.** `D:` is a + 50 GB ReFS Dev Drive that was 51% full on 2026-08-18, and each worktree's + `node_modules` costs ~0.9 GB (measured 51,735 files / 0.89 GB). ReFS supports + hardlinks, but npm extracts fresh copies rather than linking from the cache, so + nothing is shared: + + ```bash + df -h /d | tail -1 + ``` + + Under ~5 GB free, reclaim space before starting: `node scripts/clean-worktree.mjs --merged` + lists worktrees whose branch already landed. Worktrees under `C:\Users\joshs\.codex\` + and `C:\Users\joshs\.gemini\` belong to Codex and Antigravity sessions — leave them + alone and do not count them. + 3. **Install deps in the new worktree** (worktrees do NOT share `node_modules`; a cold worktree fails `vitest`/`tsc`). `npm ci` keeps the lockfile untouched: ```bash - cd ../wt- && npm ci --no-audit --no-fund + cd D:/Repos/Database/.claude/worktrees/ && npm ci --no-audit --no-fund ``` `postinstall` installs the pre-push guards automatically. 4. **Confirm the base is current:** `node scripts/check-base-freshness.mjs` — expect @@ -68,3 +89,8 @@ almost always has a pushed branch named after it. Report which check you used. O throwaway worktree or a patch file instead. - Do the work on the `claude/` branch; commit only your own paths. - When done, hand off with the `handoff` skill. +- **Worktrees are not free and nothing reclaims them automatically.** `npm run clean:worktree` + only prunes worktrees whose directory is already missing or that git has marked prunable — + a merged branch whose directory still exists is never a candidate, which is how 48 + accumulated. After the PR lands, `prlanded` should offer removal; otherwise run + `node scripts/clean-worktree.mjs --merged` periodically and remove what it lists. diff --git a/AGENTS.md b/AGENTS.md index e1111da24e..b642064b1f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -165,6 +165,39 @@ Babysit / Run PR ledger policy: do not push a tip whose sole delta is a babysit +# Claude Code hook scripts + +`.claude/hooks/*.sh` runs on Linux web containers as well as on the Windows workstation, and the +workstation cannot see the thing that breaks it. + +- **Pin the executable bit in the index, not on disk.** The primary workstation is a Windows ReFS + Dev Drive with `core.fileMode=false`, so git ignores filesystem permission bits entirely and a + local `chmod +x` is a silent no-op. A hook added there commits as `100644`. Fix it with + `git update-index --chmod=+x .claude/hooks/.sh` and confirm with `git ls-files -s`. + This is not hypothetical: `session-start.sh` shipped `100644` while both its siblings were + `100755` (found 2026-08-18). That script's body only runs when `CLAUDE_CODE_REMOTE=true`, so the + sole environment it does work in is the Linux container where a non-executable checkout cannot + be run — and it is the script that provisions the Node 24 the engine floor needs, after + `npm ci` EBADENGINE blocked PRs #1611, #1697, #1705 and #1740. +- **Register hooks as `bash "$CLAUDE_PROJECT_DIR/…"`, never as a bare path**, so the mode is never + load-bearing. `session-start.sh` was the only bare-path registration and the only one missing the + bit; that is not a coincidence worth repeating. +- **Line endings are LF.** `.gitattributes` sets `* text=auto eol=lf`; all hook blobs measure CR=0. + A CR in a shell blob fails on Linux as the near-unreadable `/bin/bash^M: bad interpreter`. +- **Hooks must not be able to fail a session.** Every hook here exits 0 on any parse problem and + makes no decision, so a malformed payload leaves the tool call exactly as it was. +- **Set an explicit `timeout`.** The default is 60s, which `session-start.sh` can exceed on a cold + container (Node tarball download plus `npm ci`) — a killed hook leaves dependencies half + installed. +- **SessionStart context comes from stdout, not stderr.** A hook that reports on stderr is invisible + to the model even though it ran and exited 0; `check-base-freshness.mjs` spent its life in that + state. Emit `{"hookSpecificOutput":{"hookEventName":"SessionStart","additionalContext":"…"}}` on + stdout, and only when the message is worth the context it costs. + +Enforced by the `claude hook scripts are checked in runnable` block in +`tests/session-start-hook.test.ts`, which fails on any hook that is not `100755` or that carries CR +bytes. Do not weaken it. + # Codex Desktop worktree setup - The Windows Codex Desktop environment setup command is `node scripts/setup-codex-worktree.mjs`. diff --git a/scripts/check-base-freshness.mjs b/scripts/check-base-freshness.mjs index 5cf55a6e4b..082ca359a5 100644 --- a/scripts/check-base-freshness.mjs +++ b/scripts/check-base-freshness.mjs @@ -11,11 +11,39 @@ * SessionStart hook or statusline without ever blocking work. Exit is non-zero * only on a genuine tooling error under `--strict` (off by default). * + * HOOK MODE — why stdout matters. This is registered as a SessionStart hook in + * .claude/settings.json, and Claude Code injects only a SessionStart hook's STDOUT + * into the model's context; stderr is not injected. Every human-readable branch of + * finish() used console.error, including the loud "N commits BEHIND origin/main" + * warning, so the tripwire this script exists to trigger never reached the agent — + * confirmed on a live session whose injected SessionStart context carried the sibling + * hook's stdout lines but no `[base-freshness]` line at all, even though this hook ran + * and exited 0. In hook mode we therefore also emit the hook JSON envelope on stdout: + * {"hookSpecificOutput":{"hookEventName":"SessionStart","additionalContext":"…"}} + * and only when the message is worth the context it costs — the loud stale warning or + * an error. A healthy base prints nothing at all to stdout; "ok" is not worth a token + * in every session's preamble. + * + * How hook mode is detected (detected, not guessed). An explicit `--hook` flag wins. + * Otherwise we infer it from CLAUDE_PROJECT_DIR being set — Claude Code exports that + * for hook commands specifically; it is absent from the Bash-tool environment, checked + * directly on 2026-08-18 — together with a non-TTY stdout. `--json` is never hook mode. + * The inference is also fail-safe rather than fail-closed: the human line still goes to + * stderr in every mode, so a misfire on a manual run cannot break the terminal output + * that `npm run check:base-freshness`, the `newtask` skill (step 4) and the `handoff` + * skill read — it would only add one extra JSON line on stdout. + * + * The origin/main fetch is capped at 10s. Claude Code gives a hook a 60s budget, so an + * un-timed fetch against an unreachable remote could burn the entire SessionStart + * budget; on timeout we fall back to the last-known origin/main exactly as the offline + * path already did. + * * Env: * STALE_BASE_THRESHOLD commits-behind that triggers the loud warning (default 10) * BASE_FRESHNESS_NO_FETCH=1 skip the network fetch (use last-known origin/main) * Flags: * --json machine-readable output + * --hook force SessionStart hook output on stdout (auto-detected; see above) * --strict exit 1 when the base ref cannot be resolved (default: exit 0) */ import { execFileSync } from "node:child_process"; @@ -24,6 +52,14 @@ const threshold = Number.parseInt(process.env.STALE_BASE_THRESHOLD ?? "10", 10) const asJson = process.argv.includes("--json"); const strict = process.argv.includes("--strict"); +// `--json` is a machine contract of its own and must keep stdout byte-identical, so it +// short-circuits the inference. The CLAUDE_PROJECT_DIR + non-TTY pair is the narrowest +// signal that separates a hook invocation from every other way this script is run: a +// human terminal has a TTY stdout, and the Bash tool (the other non-TTY caller) does not +// get CLAUDE_PROJECT_DIR exported into its environment. +const hookMode = + !asJson && (process.argv.includes("--hook") || (Boolean(process.env.CLAUDE_PROJECT_DIR) && !process.stdout.isTTY)); + function git(args) { return execFileSync("git", args, { encoding: "utf8", stdio: ["ignore", "pipe", "ignore"] }).trim(); } @@ -37,11 +73,19 @@ function tryGit(args) { } function finish(result) { + const stale = !result.error && result.behind > threshold; + if (asJson) { console.log(JSON.stringify(result)); - } else if (result.error) { + process.exit(result.error && strict ? 1 : 0); + } + + // The human line stays on stderr in every mode, hook runs included. Docs, the + // `newtask` skill and the `handoff` skill all read this exact line off the terminal, + // and stderr costs the model nothing because Claude Code drops it. + if (result.error) { console.error(`[base-freshness] ${result.error}`); - } else if (result.behind > threshold) { + } else if (stale) { console.error( `\n⚠ [base-freshness] ${result.branch} is ${result.behind} commits BEHIND origin/main ` + `(ahead ${result.ahead}).\n` + @@ -52,6 +96,17 @@ function finish(result) { `[base-freshness] ${result.branch}: behind ${result.behind}, ahead ${result.ahead} vs origin/main — ok`, ); } + + // Only a stale base or a tooling error earns a place in the session's injected + // context; a fresh base stays silent on stdout so it costs the model nothing. + if (hookMode && (stale || result.error)) { + const additionalContext = result.error + ? `[base-freshness] ${result.error} — ahead/behind vs origin/main is unknown, so treat this base as unverified.` + : `[base-freshness] Branch ${result.branch} is ${result.behind} commits BEHIND origin/main (ahead ${result.ahead}). ` + + `You may be building on a stale base — rebase or merge origin/main before starting new work.`; + console.log(JSON.stringify({ hookSpecificOutput: { hookEventName: "SessionStart", additionalContext } })); + } + process.exit(result.error && strict ? 1 : 0); } @@ -59,9 +114,16 @@ const branch = tryGit(["rev-parse", "--abbrev-ref", "HEAD"]) ?? "(unknown)"; if (process.env.BASE_FRESHNESS_NO_FETCH !== "1") { try { - execFileSync("git", ["fetch", "--quiet", "origin", "main"], { stdio: "ignore" }); + // 10s cap: a hook gets ~60s total, and a fetch that hangs (unreachable remote, a + // credential prompt, a wedged proxy) would otherwise eat the whole SessionStart + // budget for an advisory check. The default SIGTERM killSignal is sufficient here — + // libuv maps SIGTERM to TerminateProcess on Windows, so the child dies even if it + // installs a SIGTERM handler (measured on node v24 / win32: ETIMEDOUT at ~1.5s for a + // 1500ms timeout, both with and without a handler). No SIGKILL override needed. + execFileSync("git", ["fetch", "--quiet", "origin", "main"], { stdio: "ignore", timeout: 10_000 }); } catch { - // Offline or no remote — fall back to whatever origin/main we already have. + // Offline, no remote, or the fetch timed out — fall back to whatever origin/main we + // already have. A slightly stale answer beats a stalled session start. } } diff --git a/scripts/clean-worktree.mjs b/scripts/clean-worktree.mjs index 3486773606..e93e5e0839 100644 --- a/scripts/clean-worktree.mjs +++ b/scripts/clean-worktree.mjs @@ -77,13 +77,110 @@ export function identifyOrphanedWorktrees(worktrees, { existsFn = existsSync, ma return orphaned; } +/** + * Identify worktrees whose branch has already landed in origin/main and that hold no + * unsaved work — the case `identifyOrphanedWorktrees` above structurally cannot see. + * + * WHY this exists. Orphan detection only fires when the directory has vanished from disk or + * when git itself has already flagged the entry `prunable`. The dominant real-world case is + * neither: the branch merged into origin/main weeks ago, the PR closed, and the directory is + * still sitting there fully populated. Nothing in the previous cleanup path could ever + * nominate it, so the fleet only ever grew. Measured on the maintainer's machine 2026-08-18: + * `git worktree list` reported 48 registered worktrees, 41 of them carrying their own + * `node_modules`; one measured 51,735 files / 0.89 GB, putting the fleet at roughly 36 GB and + * ~2.1M files. `git worktree prune --dry-run -v` reported nothing prunable — every one of + * those 41 was "healthy" by the old definition. + * + * WHY the fleet is worth shrinking rather than tolerating. Each stale install is an + * independent dependency tree, so `check:installed-lock-parity` and + * `check:playwright-browser-revision` can drift 41 different ways, and the cross-worktree run + * coordinator (`scripts/run-heavy.mjs` -> `scripts/test-run-lock.mjs`, capped at two + * concurrent leases) contends across all of them — that is the documented 15-minute + * `verify:ui` admission queue. + * + * WHY it is dependency-injected and this conservative. This function decides what MAY be + * deleted, so it has to be drivable from `selfTest()` with mocks and no git process at all; + * it mirrors `identifyOrphanedWorktrees`'s injection shape for exactly that reason. Every + * predicate below is a reason to KEEP a worktree — detached HEAD, dirty tree, unpushed + * commit, or lock all disqualify it — because a false negative costs one more day of disk + * and a false positive costs work that no reflog in that worktree can return. + * + * Returns candidates only; it never removes anything and never shells out on its own. + */ +export function identifyMergedWorktrees( + worktrees, + { + isMergedFn = () => false, + statusFn = () => "", + aheadCountFn = () => 0, + existsFn = existsSync, + mainPath = null, + currentPath = null, + baseRef = "origin/main", + } = {}, +) { + if (!Array.isArray(worktrees) || worktrees.length === 0) return []; + const main = mainPath ? path.resolve(mainPath) : path.resolve(worktrees[0].path); + const current = currentPath ? path.resolve(currentPath) : null; + + const merged = []; + for (let i = 0; i < worktrees.length; i += 1) { + const wt = worktrees[i]; + const resolvedPath = path.resolve(wt.path); + + // Never nominate main/root, and never nominate the worktree this process is running + // inside: removing your own cwd leaves git and the caller's shell in a broken state. + if (i === 0 || resolvedPath === main) continue; + if (current && resolvedPath === current) continue; + + // A lock is an explicit human "do not touch". Honour it without inspecting further. + if (wt.locked) continue; + + // A detached HEAD has no branch to compare against origin/main, and commits made there + // are reachable only from that HEAD. Too easy to lose work — skip unconditionally. + if (wt.detached || !wt.branch) continue; + + // A directory missing from disk is the orphan path's job, not this one's. + if (!existsFn(wt.path)) continue; + + // The actual "already landed" test: git merge-base --is-ancestor origin/main. + if (!isMergedFn(wt.branch, baseRef)) continue; + + // Uncommitted or untracked work is invisible to the ancestor test above. `git worktree + // remove` would refuse it anyway, but skipping here keeps the listing honest rather than + // advertising a removal that will fail. A non-string/unreadable status fails closed. + const status = statusFn(wt.path); + if (typeof status !== "string" || status.trim() !== "") continue; + + // Belt-and-braces against the ancestor test: refs move underneath long-lived worktrees, + // and a non-numeric answer (git failed) must fail closed rather than read as zero. + const ahead = aheadCountFn(wt.branch, baseRef); + if (!Number.isFinite(ahead) || ahead !== 0) continue; + + merged.push({ + ...wt, + mergedInto: baseRef, + reason: `branch merged into ${baseRef}; clean tree; 0 commits ahead${wt.head ? ` (tip ${wt.head.slice(0, 9)})` : ""}`, + }); + } + return merged; +} + /** * Parse CLI arguments for batch size and flags. + * + * `--merged` is deliberately list-only. `--remove` is a separate opt-in that is meaningless + * on its own: allowing a bare `--remove` would make it ambiguous whether the caller meant the + * default `git worktree prune` path or a bulk directory deletion, and that ambiguity is not + * something a script holding 36 GB of other people's branches should resolve by guessing. */ export function parseArgs(argv) { let batchSize = 10; let dryRun = false; let selfTest = false; + let merged = false; + let remove = false; + let squashed = false; for (let i = 0; i < argv.length; i += 1) { const arg = argv[i]; @@ -91,6 +188,12 @@ export function parseArgs(argv) { selfTest = true; } else if (arg === "--dry-run") { dryRun = true; + } else if (arg === "--merged") { + merged = true; + } else if (arg === "--remove") { + remove = true; + } else if (arg === "--squashed") { + squashed = true; } else if (arg === "--batch-size") { const val = parseInt(argv[i + 1], 10); if (Number.isNaN(val) || val <= 0) { @@ -106,7 +209,227 @@ export function parseArgs(argv) { batchSize = val; } } - return { batchSize, dryRun, selfTest }; + if (remove && !merged) { + throw new Error("--remove is only valid together with --merged. Run `--merged` alone to list candidates first."); + } + if (squashed && !merged) { + throw new Error("--squashed is only valid together with --merged."); + } + return { batchSize, dryRun, selfTest, merged, remove, squashed }; +} + +/** + * Real-git adapters for `identifyMergedWorktrees`. They live outside the pure function so + * `selfTest()` never spawns git, and each one fails CLOSED: an error while asking git a + * question is treated as "this worktree is not a candidate", never as "it is safe to delete". + */ +function gitBranchIsAncestor(branch, baseRef) { + try { + // Exit 0 = ancestor, exit 1 = not, anything else = error. All non-zero throws here. + execSync(`git merge-base --is-ancestor "${branch}" "${baseRef}"`, { stdio: "ignore" }); + return true; + } catch { + return false; + } +} + +/** + * Opt-in (`--merged --squashed`) content-equality test, for branches this repo SQUASH-merged. + * + * WHY this is needed at all. `gitBranchIsAncestor` above is the strictly correct test and it + * is the default, but it answers "is this exact commit reachable from origin/main" — and a + * squash merge never leaves the branch tip reachable. This repo squash-merges as its normal + * path (AGENTS.md: "this repo's normal squash-merge folds every commit into one on `main`"), + * so the ancestor test alone barely fires. Measured here 2026-08-18 across the 49 registered + * worktrees: ancestor-only nominated 1; of the 40 it rejected, 8 were in fact fully landed via + * squash. That is the difference between a cleanup that reclaims nothing and one that does. + * + * HOW it decides. Replay the branch's ENTIRE diff from its merge-base as one synthetic commit, + * then ask `git cherry` whether origin/main already contains a commit with that patch-id + * ("-" = already upstream). Comparing the whole-branch patch is what makes it match a single + * squashed commit, which per-commit `git cherry` cannot do. + * + * WHY it is still conservative. Patch-id equality is exact. If main moved on and altered those + * same lines afterwards, the ids stop matching and the branch is reported NOT merged — a false + * negative, which costs disk, never work. `commit-tree` writes one dangling object that + * ordinary `git gc` reclaims; it mutates no ref and touches no remote. + */ +const squashMergeVerdictCache = new Map(); + +function gitBranchSquashMerged(branch, baseRef) { + // Memoised because the ahead-count guard below asks the same question a second time, and + // each answer costs a `git cherry` patch-id scan. On this fleet that halves the wall clock. + // + // The separator below is NUL because a git ref may contain almost any byte except NUL, so it is + // the one delimiter that cannot collide with a ref name. Always write it as the six-character + // JS escape, never as a literal NUL byte in the source. A raw NUL makes every text tool treat + // this file as binary: `grep` suppresses matches and prints only "Binary file matches", and + // `file` reports "binary data". Git still diffs it as text, so the damage never shows up in + // review — it shows up in verification, where a `git diff … | grep ` check returns + // empty whether or not the pattern is present. That is a check that cannot fail, and it + // produced a false "this function is untouched" result during this file's own review on + // 2026-08-18. + const key = `${baseRef}\u0000${branch}`; + if (squashMergeVerdictCache.has(key)) return squashMergeVerdictCache.get(key); + const verdict = computeBranchSquashMerged(branch, baseRef); + squashMergeVerdictCache.set(key, verdict); + return verdict; +} + +function revParseOrNull(spec) { + try { + return execSync(`git rev-parse "${spec}"`, { encoding: "utf8", stdio: ["ignore", "pipe", "ignore"] }).trim(); + } catch { + return null; + } +} + +/** + * Describe HOW strong the "already landed" evidence is for one candidate. + * + * The two merge tests are not equally trustworthy and the listing must not present them as + * though they were. `merge-base --is-ancestor` is proof: every commit is reachable from the base + * ref, full stop. The patch-id test behind `--squashed` is an *inference* — it says the branch's + * combined diff matched something that landed, which is the right call for a squash-merging repo + * but is not the same claim. + * + * Why it matters concretely: reviewing this fleet on 2026-08-18, one squash-inferred candidate + * (`claude/rag-d4-reconcile-inbox`, 21 changed files) still had 2 files differing from + * origin/main. Both were high-churn append-only documents — `docs/outstanding-issues.md` and a + * handover doc — so the difference is almost certainly main moving on after the branch landed, + * not lost work. "Almost certainly" is exactly the distinction this line exists to surface: the + * operator, not the script, decides. That is also why `--remove` stays a separate opt-in. + * + * Cost is bounded — this runs over the candidate list, never the whole fleet. + */ +function describeMergeConfidence(wt, baseRef) { + try { + execSync(`git merge-base --is-ancestor "${wt.branch}" "${baseRef}"`, { + stdio: ["ignore", "ignore", "ignore"], + }); + return `proven — every commit on this branch is reachable from ${baseRef}`; + } catch { + // Not an ancestor, so this candidate can only have come from the patch-id test below. + } + + try { + const base = execSync(`git merge-base "${baseRef}" "${wt.branch}"`, { + encoding: "utf8", + stdio: ["ignore", "pipe", "ignore"], + }).trim(); + const files = execSync(`git diff --name-only ${base} "${wt.branch}"`, { + encoding: "utf8", + stdio: ["ignore", "pipe", "ignore"], + maxBuffer: 8 * 1024 * 1024, + }) + .split(/\r?\n/) + .filter((line) => line.trim().length > 0); + + const differing = files.filter((f) => revParseOrNull(`${wt.branch}:${f}`) !== revParseOrNull(`${baseRef}:${f}`)); + + if (files.length === 0) { + return "inferred from patch-id; branch changed no files vs its merge base"; + } + if (differing.length === 0) { + return `inferred from patch-id, corroborated — all ${files.length} changed file(s) are byte-identical to ${baseRef}`; + } + return ( + `inferred from patch-id, NOT fully corroborated — ${differing.length} of ${files.length} changed file(s) ` + + `still differ from ${baseRef} (${differing.slice(0, 3).join(", ")}${differing.length > 3 ? ", …" : ""}); ` + + `usually just churn on the base since it landed, but review before removing` + ); + } catch { + return "inferred from patch-id; corroboration check failed — review before removing"; + } +} + +function computeBranchSquashMerged(branch, baseRef) { + if (gitBranchIsAncestor(branch, baseRef)) return true; + try { + const run = (cmd) => execSync(cmd, { encoding: "utf8", stdio: ["ignore", "pipe", "ignore"] }).trim(); + const mergeBase = run(`git merge-base "${baseRef}" "${branch}"`); + const tree = run(`git rev-parse "${branch}^{tree}"`); + if (!mergeBase || !tree) return false; + const synthetic = run(`git commit-tree "${tree}" -p "${mergeBase}" -m squash-merge-probe`); + if (!synthetic) return false; + // The third argument bounds the scan to `mergeBase..baseRef`. Without it `git cherry` + // patch-ids every commit in origin/main's whole history for every branch examined, which + // on a 48-worktree fleet is thousands of redundant diffs; a squash of THIS branch can only + // exist after its own merge-base, so the bound is free correctness-wise. + const verdict = run(`git cherry "${baseRef}" "${synthetic}" "${mergeBase}"`); + // "- " means the patch is already upstream; "+ " means it is not. + return verdict.startsWith("-"); + } catch { + return false; + } +} + +function gitWorktreeStatus(worktreePath) { + try { + return execSync(`git -C "${worktreePath}" status --porcelain`, { + encoding: "utf8", + stdio: ["ignore", "pipe", "ignore"], + }); + } catch (err) { + // An unreadable status is not a clean status; return a non-empty sentinel so the caller + // treats the worktree as dirty and keeps it. + return `?? status unavailable (${err.message})`; + } +} + +function gitAheadCount(branch, baseRef) { + try { + const out = execSync(`git rev-list --count "${baseRef}..${branch}"`, { + encoding: "utf8", + stdio: ["ignore", "pipe", "ignore"], + }); + const parsedCount = parseInt(out.trim(), 10); + return Number.isNaN(parsedCount) ? Number.NaN : parsedCount; + } catch { + return Number.NaN; + } +} + +/** + * Ahead-count companion used only in `--squashed` mode. + * + * WHY the plain `gitAheadCount` cannot be reused here. It asks "how many commits are in + * origin/main..branch", a commit-graph proxy for "how much work is unlanded". Squash merging + * breaks that proxy: a fully-landed branch keeps every one of its original commits, so it + * reports ahead > 0 forever. Pairing it with the squash test cancels the squash test out — + * measured 2026-08-18 on the 48-worktree fleet, that combination nominated 1 worktree while 8 + * more were provably landed. + * + * WHY it is not a per-commit patch-id count either. That was the first attempt and it is a + * subtler version of the same bug: a squash commit's patch-id matches the branch's COMBINED + * diff, never the individual commits it folded, so `git cherry` marks every commit of a + * multi-commit landed branch "+". It rescued only the accidental single-commit cases — 4 of + * the 8 — and silently vetoed the rest. + * + * WHAT it does instead, and why redundancy is the honest answer. Whole-branch patch-id + * equality (`gitBranchSquashMerged`) already proves the branch's entire diff is upstream, + * which is strictly stronger than any commit count. So in this mode the guard is subsumed by + * the merge test and returns 0; a branch that is NOT content-equal falls back to the real + * graph count. Keeping a check that can only produce false negatives would be worse than + * admitting it is redundant here. The cached verdict makes the extra call free. + * + * Blast radius either way: `git worktree remove` deletes the working directory, not the + * branch. The ref and its commits survive in the repo, so a wrong call costs a re-checkout. + */ +function gitAheadUnlandedCount(branch, baseRef) { + if (gitBranchSquashMerged(branch, baseRef)) return 0; + return gitAheadCount(branch, baseRef); +} + +function gitCurrentWorktreePath() { + try { + return execSync("git rev-parse --show-toplevel", { + encoding: "utf8", + stdio: ["ignore", "pipe", "ignore"], + }).trim(); + } catch { + return process.cwd(); + } } export function selfTest() { @@ -177,6 +500,149 @@ export function selfTest() { throw new Error("selfTest failed: parseArgs did not reject negative batch-size"); } + const args3 = parseArgs(["--merged"]); + if (!args3.merged || args3.remove) { + throw new Error("selfTest failed: --merged must default to list-only (remove=false)"); + } + const args4 = parseArgs(["--merged", "--remove"]); + if (!args4.merged || !args4.remove) { + throw new Error("selfTest failed: --merged --remove did not set both flags"); + } + let removeThrew = false; + try { + parseArgs(["--remove"]); + } catch { + removeThrew = true; + } + if (!removeThrew) { + throw new Error("selfTest failed: parseArgs accepted a bare --remove without --merged"); + } + const args5 = parseArgs(["--merged", "--squashed"]); + if (!args5.squashed || args5.remove) { + throw new Error("selfTest failed: --merged --squashed must stay list-only"); + } + let squashThrew = false; + try { + parseArgs(["--squashed"]); + } catch { + squashThrew = true; + } + if (!squashThrew) { + throw new Error("selfTest failed: parseArgs accepted a bare --squashed without --merged"); + } + + // Merged-worktree identification. Every fixture below is a worktree that a naive + // "is it merged?" check would happily delete; only `merged-clean` may actually qualify. + const mergedPorcelain = [ + "worktree /path/to/main", + "HEAD 1111111111111111111111111111111111111111", + "branch refs/heads/main", + "", + "worktree /path/to/merged-clean", + "HEAD 2222222222222222222222222222222222222222", + "branch refs/heads/merged-clean", + "", + "worktree /path/to/merged-dirty", + "HEAD 3333333333333333333333333333333333333333", + "branch refs/heads/merged-dirty", + "", + "worktree /path/to/merged-ahead", + "HEAD 4444444444444444444444444444444444444444", + "branch refs/heads/merged-ahead", + "", + "worktree /path/to/detached", + "HEAD 5555555555555555555555555555555555555555", + "detached", + "", + "worktree /path/to/merged-locked", + "HEAD 6666666666666666666666666666666666666666", + "branch refs/heads/merged-locked", + "locked in-flight release rehearsal", + "", + "worktree /path/to/unmerged", + "HEAD 7777777777777777777777777777777777777777", + "branch refs/heads/unmerged", + "", + "worktree /path/to/current", + "HEAD 8888888888888888888888888888888888888888", + "branch refs/heads/current", + "", + ].join("\n"); + + const mergedParsed = parseWorktreePorcelain(mergedPorcelain); + if (mergedParsed.length !== 8) { + throw new Error(`selfTest failed: expected 8 parsed merged-mode worktrees, got ${mergedParsed.length}`); + } + + // Everything is merged except refs/heads/unmerged; the detached entry has no branch at all. + const mockIsMerged = (branch) => branch !== "refs/heads/unmerged"; + const mockStatus = (p) => (p === "/path/to/merged-dirty" ? " M src/lib/rag/rag.ts\n?? scratch.txt\n" : ""); + const mockAhead = (branch) => (branch === "refs/heads/merged-ahead" ? 2 : 0); + + const candidates = identifyMergedWorktrees(mergedParsed, { + isMergedFn: mockIsMerged, + statusFn: mockStatus, + aheadCountFn: mockAhead, + existsFn: () => true, + mainPath: "/path/to/main", + currentPath: "/path/to/current", + }); + + if (candidates.length !== 1) { + const paths = candidates.map((c) => c.path).join(", "); + throw new Error(`selfTest failed: expected exactly 1 merged candidate, got ${candidates.length} [${paths}]`); + } + if (candidates[0].path !== "/path/to/merged-clean") { + throw new Error(`selfTest failed: expected /path/to/merged-clean, got ${candidates[0].path}`); + } + if (!candidates[0].reason || !candidates[0].reason.includes("origin/main")) { + throw new Error("selfTest failed: merged candidate is missing a human-readable merge reason"); + } + if (candidates[0].mergedInto !== "origin/main") { + throw new Error("selfTest failed: merged candidate did not record its base ref"); + } + + const candidatePaths = new Set(candidates.map((c) => c.path)); + for (const mustSkip of [ + ["/path/to/main", "main worktree"], + ["/path/to/merged-dirty", "dirty working tree"], + ["/path/to/merged-ahead", "commits ahead of origin/main"], + ["/path/to/detached", "detached HEAD"], + ["/path/to/merged-locked", "locked worktree"], + ["/path/to/unmerged", "branch not merged"], + ["/path/to/current", "current worktree"], + ]) { + if (candidatePaths.has(mustSkip[0])) { + throw new Error(`selfTest failed: ${mustSkip[1]} (${mustSkip[0]}) must never be a merged candidate`); + } + } + + // A missing directory belongs to the orphan path, not the merged path. + const missingDirCandidates = identifyMergedWorktrees(mergedParsed, { + isMergedFn: mockIsMerged, + statusFn: mockStatus, + aheadCountFn: mockAhead, + existsFn: () => false, + mainPath: "/path/to/main", + currentPath: "/path/to/current", + }); + if (missingDirCandidates.length !== 0) { + throw new Error("selfTest failed: worktrees missing from disk must not be merged candidates"); + } + + // Fail-closed contract: git errors surface as NaN/non-empty status and must keep the worktree. + const failClosed = identifyMergedWorktrees(mergedParsed, { + isMergedFn: mockIsMerged, + statusFn: () => "?? status unavailable (git exploded)", + aheadCountFn: () => Number.NaN, + existsFn: () => true, + mainPath: "/path/to/main", + currentPath: "/path/to/current", + }); + if (failClosed.length !== 0) { + throw new Error("selfTest failed: unreadable git state must fail closed to zero candidates"); + } + console.log("[clean-worktree] Self-test passed successfully."); } @@ -239,6 +705,106 @@ export function runWorktreeCleanup(options = {}) { } } +/** + * `--merged` mode: report (and only on explicit `--remove`, delete) worktrees whose branch + * already landed in origin/main. + * + * This is strictly additive and opt-in. `runWorktreeCleanup()` above is what + * `npm run clean:worktree` — and therefore `verify:preflight` — invokes, and its behaviour is + * deliberately untouched: nothing on the preflight path may start deleting directories. + * + * Listing is the default and removal is the exception, because the population this walks is + * 41 populated worktrees on the maintainer's machine (~36 GB, ~2.1M files, one measured at + * 51,735 files / 0.89 GB) and a wrong bulk delete there is not recoverable from the deleted + * worktree's own reflog. + */ +export function runMergedWorktreeReport(options = {}) { + const { batchSize = 10, remove = false, squashed = false, baseRef = "origin/main" } = options; + + // Without the base ref every ancestor test would answer "not merged" and the mode would + // silently report nothing. Say so instead of returning a misleading empty list. + try { + execSync(`git rev-parse --verify --quiet "${baseRef}"`, { stdio: ["ignore", "ignore", "ignore"] }); + } catch { + console.error(`[clean-worktree] Base ref ${baseRef} not found. Fetch it first; refusing to guess merge state.`); + throw new Error(`Base ref ${baseRef} is unavailable`); + } + + let porcelainOutput = ""; + try { + porcelainOutput = execSync("git worktree list --porcelain", { + encoding: "utf8", + stdio: ["ignore", "pipe", "ignore"], + }); + } catch (err) { + console.warn("[clean-worktree] Could not list worktrees:", err.message); + return; + } + + const worktrees = parseWorktreePorcelain(porcelainOutput); + const currentPath = gitCurrentWorktreePath(); + const mode = squashed ? "ancestor-or-squash (--squashed)" : "ancestor-only"; + console.log(`[clean-worktree] ${worktrees.length} registered worktree(s); base ref ${baseRef}; merge test: ${mode}.`); + + const candidates = identifyMergedWorktrees(worktrees, { + isMergedFn: squashed ? gitBranchSquashMerged : gitBranchIsAncestor, + statusFn: gitWorktreeStatus, + aheadCountFn: squashed ? gitAheadUnlandedCount : gitAheadCount, + existsFn: existsSync, + currentPath, + baseRef, + }); + + if (candidates.length === 0) { + console.log("[clean-worktree] No merged, clean, unlocked worktrees found. Nothing to report."); + if (!squashed) { + console.log("[clean-worktree] Note: squash-merged branches are invisible to the ancestor test. Try --squashed."); + } + return; + } + + console.log(`[clean-worktree] ${candidates.length} merged worktree(s) eligible for removal:`); + for (const wt of candidates) { + console.log(` - ${wt.path}`); + console.log(` branch: ${wt.branch}`); + console.log(` reason: ${wt.reason}`); + console.log(` confidence: ${describeMergeConfidence(wt, baseRef)}`); + } + + if (!remove) { + console.log(`[clean-worktree] Listed ${candidates.length} candidate(s). Nothing was removed.`); + console.log(`[clean-worktree] Re-run with \`--merged${squashed ? " --squashed" : ""} --remove\` to delete these.`); + if (!squashed) { + console.log("[clean-worktree] Note: squash-merged branches are invisible to the ancestor test. Try --squashed."); + } + return; + } + + const batch = candidates.slice(0, batchSize); + console.log( + `[clean-worktree] Removing ${batch.length} of ${candidates.length} candidate(s) (batch size ${batchSize})...`, + ); + + let removed = 0; + let skipped = 0; + for (const wt of batch) { + try { + // Never `--force`. If git refuses — a submodule, a lock we failed to see, or state that + // appeared after the scan — that refusal is information, not an obstacle to override. + execSync(`git worktree remove "${wt.path}"`, { stdio: ["ignore", "pipe", "pipe"] }); + removed += 1; + console.log(` - removed: ${wt.path}`); + } catch (err) { + skipped += 1; + console.warn(` - SKIPPED (git refused): ${wt.path} — ${err.message.trim()}`); + } + } + + console.log( + `[clean-worktree] Removed ${removed}, skipped ${skipped}, remaining candidates ${candidates.length - batch.length}.`, + ); +} + function main() { const argv = process.argv.slice(2); let parsed; @@ -254,6 +820,18 @@ function main() { return; } + if (parsed.merged) { + try { + // --dry-run is an explicit alias for list-only, and it wins over --remove so that a + // habitual `--dry-run` can never be defeated by a stray --remove on the same line. + runMergedWorktreeReport({ ...parsed, remove: parsed.remove && !parsed.dryRun }); + } catch (err) { + console.error("[clean-worktree] Merged-worktree report failed:", err.message); + process.exit(1); + } + return; + } + try { runWorktreeCleanup(parsed); } catch (err) { diff --git a/tests/session-start-hook.test.ts b/tests/session-start-hook.test.ts index 49a32b1df4..cf1ce566bb 100644 --- a/tests/session-start-hook.test.ts +++ b/tests/session-start-hook.test.ts @@ -158,3 +158,71 @@ describe("session-start hook", () => { expect(result.stdout.trim()).toBe(""); }); }); + +/** + * Checked-in file mode for the hook scripts. + * + * `session-start.sh` shipped as `100644` while both its siblings were `100755`. + * That is invisible on this repo's primary workstation: it is a Windows ReFS Dev + * Drive with `core.fileMode=false`, so git ignores filesystem permission bits + * entirely and a local `chmod +x` is a no-op that cannot fix the index. Only + * `git update-index --chmod=+x` can, and nothing prompted anyone to run it. + * + * It matters because `.claude/settings.json` invokes that one hook by bare path + * rather than through `bash`, and the script's whole body is gated on + * `CLAUDE_CODE_REMOTE=true` — so the only environment it ever does work in is a + * Linux web container, which is exactly where a non-executable checkout cannot + * be run. The script provisions the Node 24 the repo's engine floor requires; + * its own header records four PRs (#1611, #1697, #1705, #1740) blocked by + * `npm ci` EBADENGINE before it existed. + * + * Two independent fixes now cover this, and this test pins the first: every hook + * is `100755` in the index, and none may regress to `100644`. (The second is + * that the settings.json registration invokes it via `bash`, which removes the + * dependency on the mode altogether — belt and braces, because a future hook + * added by an agent on this same Dev Drive will hit the identical blind spot.) + * + * Line endings are pinned alongside it for the same reason: `.gitattributes` + * sets `* text=auto eol=lf`, and a CR in a shell blob fails on Linux with the + * near-unreadable `/bin/bash^M: bad interpreter`. Measured clean at the time of + * writing (CR=0 across all five hook blobs); this keeps it that way. + */ +describe("claude hook scripts are checked in runnable", () => { + const listed = spawnSync("git", ["ls-files", "-s", ".claude/hooks"], { + cwd: process.cwd(), + encoding: "utf8", + }); + + const entries = (listed.stdout ?? "") + .split(/\r?\n/) + .filter((line) => line.trim().length > 0) + .map((line) => { + const [meta, path] = line.split("\t"); + const [mode, object] = meta.split(/\s+/); + return { mode, object, path }; + }) + .filter((entry) => entry.path?.endsWith(".sh")); + + it("finds the hook scripts", () => { + expect(listed.status).toBe(0); + expect(entries.length).toBeGreaterThanOrEqual(3); + }); + + it.each(entries.map((entry) => [entry.path, entry.mode, entry.object]))( + "%s is mode 100755 with LF-only line endings", + (path, mode, object) => { + expect(mode, `${path} must be executable in the index; fix with: git update-index --chmod=+x ${path}`).toBe( + "100755", + ); + + const blob = spawnSync("git", ["cat-file", "blob", object as string], { + cwd: process.cwd(), + encoding: "buffer", + maxBuffer: 8 * 1024 * 1024, + }); + expect(blob.status).toBe(0); + const carriageReturns = (blob.stdout as Buffer).filter((byte) => byte === 0x0d).length; + expect(carriageReturns, `${path} must be stored with LF-only line endings (.gitattributes eol=lf)`).toBe(0); + }, + ); +}); From dd6dddc8e2f8ced52516405e3ffe46f16ff540bb Mon Sep 17 00:00:00 2001 From: BigSimmo <87357024+BigSimmo@users.noreply.github.com> Date: Tue, 18 Aug 2026 18:20:18 +0800 Subject: [PATCH 2/7] fix(tests): stop the session-start hook test failing on every Windows run MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `tests/session-start-hook.test.ts` asserted that the written env file contains `join(home, ".node24", …)`. On Windows `home` comes from mkdtempSync(tmpdir()) as `C:\Users\…\AppData\Local\Temp\session-start-home-XXXX`, while the hook runs under Git Bash and writes the POSIX view of the same directory, `/tmp/ session-start-home-XXXX`. The assertion therefore failed on every Windows run regardless of the diff under test. That is worse than a red test: it is a red test everyone learns to ignore. It fails inside `npm run test`, which is the last step of `verify:pr-local`, so the whole gate goes red locally for every change and the only way to use it is to decide which failures do not count. Compare the path tail instead. The unique mkdtemp basename still pins the assertion to this test's own HOME, so it loses no strength, and it now holds on both platforms. Co-Authored-By: Claude Opus 5 --- tests/session-start-hook.test.ts | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/tests/session-start-hook.test.ts b/tests/session-start-hook.test.ts index cf1ce566bb..3863e3c89e 100644 --- a/tests/session-start-hook.test.ts +++ b/tests/session-start-hook.test.ts @@ -2,7 +2,7 @@ import { createHash } from "node:crypto"; import { spawnSync } from "node:child_process"; import { chmodSync, copyFileSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; -import { join } from "node:path"; +import { basename, join } from "node:path"; import { afterEach, describe, expect, it } from "vitest"; /** @@ -139,7 +139,15 @@ describe("session-start hook", () => { expect(result.status, `hook exited ${result.status}: ${result.stderr}`).toBe(0); const written = readFileSync(envFile, "utf8"); - expect(written).toContain(join(home, ".node24", `node-v${NODE_VERSION}-linux-x64`, "bin")); + // Compare the path tail, not the absolute path. `home` comes from mkdtempSync(tmpdir()), + // which on Windows is `C:\Users\…\AppData\Local\Temp\session-start-home-XXXX`, while the + // hook runs under Git Bash and writes the POSIX view of the same directory — `/tmp/ + // session-start-home-XXXX`. Asserting the joined Windows path therefore failed on every + // Windows run regardless of the diff under test, which made the whole file look red + // locally and trained readers to wave it through. The unique mkdtemp basename still + // pins this to *this* test's HOME, so the assertion loses no strength. + const expectedTail = [basename(home), ".node24", `node-v${NODE_VERSION}-linux-x64`, "bin"].join("/"); + expect(written.replace(/\\/g, "/")).toContain(expectedTail); expect(written).toContain("export PATH="); // The manual-run advice belongs only to the manual-run branch. expect(result.stdout).not.toContain("CLAUDE_ENV_FILE is unset"); From 4b2c60c1c0ff925b2b1a379df456c48c8599ae86 Mon Sep 17 00:00:00 2001 From: BigSimmo <87357024+BigSimmo@users.noreply.github.com> Date: Tue, 18 Aug 2026 19:00:41 +0800 Subject: [PATCH 3/7] docs(ledger): record the Claude Code environment review for PR #2113 Co-Authored-By: Claude Opus 5 --- ...2b132f27a2240602c3c56d2e3ddff9f1354bbe5c50aef7099fd.record.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 docs/branch-review-records/ad1e40bd3d8c42b132f27a2240602c3c56d2e3ddff9f1354bbe5c50aef7099fd.record.md diff --git a/docs/branch-review-records/ad1e40bd3d8c42b132f27a2240602c3c56d2e3ddff9f1354bbe5c50aef7099fd.record.md b/docs/branch-review-records/ad1e40bd3d8c42b132f27a2240602c3c56d2e3ddff9f1354bbe5c50aef7099fd.record.md new file mode 100644 index 0000000000..a4ff87a0dd --- /dev/null +++ b/docs/branch-review-records/ad1e40bd3d8c42b132f27a2240602c3c56d2e3ddff9f1354bbe5c50aef7099fd.record.md @@ -0,0 +1 @@ +| 2026-08-18 | claude/code-setup-review-a95519 | dd6dddc8e2f8ced52516405e3ffe46f16ff540bb | Claude Code environment: hook exec bit + bash registration + contract test, settings.json permissions block, base-freshness stdout hook mode + fetch timeout, clean-worktree --merged/--squashed with confidence line, PreCompact + push-format hooks, AGENTS.md hook section, newtask Dev Drive pinning, Windows test fix | shipped as PR #2113; session-start.sh was mode 100644 and bare-path registered so it could never run on the Linux web containers it exists for, invisible locally under core.fileMode=false on the ReFS Dev Drive; fixed via index mode + bash registration + contract test. clean-worktree ancestor detection alone finds 2 of 49 because the repo squash-merges, so --squashed (whole-branch patch-id) was added, finding 9, with a confidence line separating proven from inferred after one candidate showed 2 of 21 files still differing. runWorktreeCleanup byte-identical (1969 bytes both sides) so verify:preflight unaffected. No worktree removed: re-verification immediately before deletion showed two candidates had gained 2 unmerged commits since the scan and a third had been switched branches by a live session. A raw NUL byte in clean-worktree.mjs was replaced with the escape after it made a git-diff-pipe-grep verification vacuous. tests/session-start-hook.test.ts:142 failed on every Windows run and sits inside verify:pr-local's last step, so the PR gate was red locally for every diff; fixed by comparing the path tail | npm run test: 668 files passed / 7129 tests passed / 0 failed, real exit 0 (captured to file, not piped); verify:pr-local completed 23 checks incl lint + typecheck; new hook contract tests 6 passed; clean-worktree --self-test passed; --merged and --merged --squashed run against the live 49-worktree fleet with count unchanged and --remove never run; prettier --check . clean; eslint clean; pr-policy classifier clinicalRisk/operationalRisk/ragRanking/ui all false; NOT run: verify:ui (no UI surface), verify:release and all provider-backed gates, check:production-readiness | From 6d00566b1bef46534c6ed21afb4ed8e0f415f3cc Mon Sep 17 00:00:00 2001 From: BigSimmo <87357024+BigSimmo@users.noreply.github.com> Date: Tue, 18 Aug 2026 19:49:33 +0800 Subject: [PATCH 4/7] test(claude): pin the permission boundary and make the PreCompact hook answerable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the four items left open when PR #2113 was opened, each with the smallest fix that actually resolves it rather than restating it. 1. The claim that no `allow` rule can reach a provider-backed script was asserted in review and never measured. It is now a test: 36 provider-backed scripts, derived from script names rather than hand-listed, each asserted unreachable through `allow` and covered by an explicit `ask`. Mutation-checked — injecting a broad `Bash(npm run check:*)` allow rule turns 4 of them red. 2. Hook registrations are pinned to invoke through an interpreter rather than a bare path, and to carry an explicit timeout. Bare-path registration is what made the `session-start.sh` mode bug reachable; the mutation check confirms both guards bite. 3. The PreCompact hook's known limit could not be closed by reading code — the installed CLI ships a compiled binary with no inspectable bundle, so whether the platform injects its stdout into model context is not determinable here. Instead of leaving that permanently unverified, the hook now appends one line per firing to a log under the git dir. After the next compaction the log distinguishes "hook never ran" from "hook ran but its output went nowhere", and both answers are actionable. The log lives outside the worktree so it can never be staged. 4. The `newtask` skill said "~40 worktrees". It was 48 on 2026-08-18 and reached 50 during one session, so the number now carries the reason it drifts. Co-Authored-By: Claude Opus 5 --- .claude/hooks/precompact-issues-capture.sh | 19 +++ .claude/skills/newtask/SKILL.md | 6 +- tests/claude-code-settings.test.ts | 128 +++++++++++++++++++++ 3 files changed, 151 insertions(+), 2 deletions(-) create mode 100644 tests/claude-code-settings.test.ts diff --git a/.claude/hooks/precompact-issues-capture.sh b/.claude/hooks/precompact-issues-capture.sh index 376b32917b..9b9af56fcb 100755 --- a/.claude/hooks/precompact-issues-capture.sh +++ b/.claude/hooks/precompact-issues-capture.sh @@ -43,4 +43,23 @@ esac echo "[issues] ${why} Anything this session discovered but has not written down is about to be summarised away: unresolved follow-ups, deferrals, known risks, and work you decided NOT to do and why. Record them now with /issues add … (or /issues capture for a sweep) — docs/outstanding-issues.md is the only memory that survives a context reset. Requests land as immutable files under docs/outstanding-issues-inbox/; they are not committed unless you are explicitly asked to commit." +# Self-verification, because the KNOWN LIMIT above cannot be resolved by reading code. +# Whether the platform injects this hook's stdout into model context is not something the +# repo can determine — the installed CLI ships a compiled binary with no inspectable +# bundle. What the repo CAN do is make the question answerable instead of permanently open: +# append one line per firing to a log outside the worktree, so after the next compaction +# `cat "$(git rev-parse --absolute-git-dir)/claude-precompact.log"` says whether the hook +# ran at all. If lines appear but the reminder never reached the model, the limit is real +# and the SessionStart backstop is doing the work; if no lines appear, the registration is +# wrong. Either answer is actionable; "unverified" is not. +# +# Kept under the git dir, never the worktree, so it can never be staged or committed. +log_dir="$(git rev-parse --absolute-git-dir 2>/dev/null || true)" +if [ -n "$log_dir" ] && [ -d "$log_dir" ]; then + printf '%s precompact trigger=%s\n' \ + "$(date -u +%Y-%m-%dT%H:%M:%SZ 2>/dev/null || echo unknown)" \ + "${trigger:-unknown}" \ + >>"$log_dir/claude-precompact.log" 2>/dev/null || true +fi + exit 0 diff --git a/.claude/skills/newtask/SKILL.md b/.claude/skills/newtask/SKILL.md index 332ebcb36c..a31f46c551 100644 --- a/.claude/skills/newtask/SKILL.md +++ b/.claude/skills/newtask/SKILL.md @@ -5,9 +5,11 @@ description: Bootstrap a clean session for new work in this repo — create a fr # newtask — start a clean, current working copy -This repo moves fast and shares ~40 worktrees and one stash stack, so starting work on a +This repo moves fast and shares ~50 worktrees and one stash stack, so starting work on a stale base or a cold worktree is the default failure. This skill sets up an isolated, -current worktree so new work starts clean. +current worktree so new work starts clean. That count is not stable trivia — it was 48 on +2026-08-18 and reached 50 during a single session, because nothing reclaims a worktree +whose branch has landed. See the cleanup note at the end. ## Before you start diff --git a/tests/claude-code-settings.test.ts b/tests/claude-code-settings.test.ts new file mode 100644 index 0000000000..17af545e59 --- /dev/null +++ b/tests/claude-code-settings.test.ts @@ -0,0 +1,128 @@ +import { readFileSync } from "node:fs"; +import { join } from "node:path"; +import { describe, expect, it } from "vitest"; + +/** + * `.claude/settings.json` is the only place where AGENTS.md's provider-confirmation + * boundary is enforced rather than merely stated. Prose did not hold for the PR-following + * rule — `.claude/hooks/pr-handoff-stop.sh` says so in its own header, "prose rules in + * AGENTS.md have not held, a denied tool call does" — and there is no reason to expect it to + * hold better for provider access. + * + * These tests pin the two properties that make the block trustworthy. Both were asserted in + * review before they were ever measured, which is the failure shape this session kept hitting: + * a check that cannot fail is not a check. + * + * 1. **No `allow` rule may reach a provider-backed script.** The block deliberately avoids + * broad wildcards such as `Bash(npm run check:*)` precisely so the outcome never depends on + * allow-versus-ask precedence. If someone later broadens the allow list for convenience, + * this goes red. + * 2. **Every provider-backed script must carry an `ask` rule.** `ask` is also the default for + * an unlisted command, so these rules buy no protection on their own — what they buy is a + * machine-readable statement of the boundary that survives a future broadening, and a list + * that fails loudly when a new provider script is added without one. + * + * Plus one hook property: every hook command invokes its script through an interpreter rather + * than by bare path. `session-start.sh` was registered by bare path AND checked in as mode + * 100644, so on the Linux web containers that are the only place it does any work, it could + * not run. See the "Claude Code hook scripts" section in AGENTS.md. + */ + +const repoRoot = process.cwd(); +const settings = JSON.parse(readFileSync(join(repoRoot, ".claude/settings.json"), "utf8")); +const packageScripts: Record = JSON.parse(readFileSync(join(repoRoot, "package.json"), "utf8")).scripts; + +/** + * Claude Code's Bash permission rules: `Bash(cmd)` matches that command exactly, and a + * trailing `:*` (or ` --*`) makes it a prefix match. Anything else is not a Bash rule. + */ +function bashRuleMatches(rule: string, command: string): boolean { + const parsed = /^Bash\((.*)\)$/.exec(rule); + if (!parsed) return false; + const pattern = parsed[1]; + if (pattern.endsWith(":*")) return command.startsWith(pattern.slice(0, -2)); + if (pattern.endsWith(" --*")) return command.startsWith(pattern.slice(0, -4)); + return command === pattern; +} + +/** + * Provider-backed or destructive npm scripts, per AGENTS.md "API and provider confirmation + * boundary": anything that reaches OpenAI, live Supabase, GitHub, hosted CI, or mutates the + * live index. Derived from the script names rather than hand-listed, so a newly added + * `eval:` or `reindex:` script is covered the day it lands. + */ +const PROVIDER_BACKED = + /supabase-project|^eval:|^test:live|^verify:release|github-shell-access:live|^sync:pr-br|production-readiness|cross-tenant|^import:docs|^enrich:|^classify:|^reindex|governance:release/; + +const providerScripts = Object.keys(packageScripts).filter((name) => PROVIDER_BACKED.test(name)); + +describe("claude code permissions", () => { + it("recognises a meaningful set of provider-backed scripts", () => { + // A regex that silently stops matching would make both tests below vacuously pass. + expect(providerScripts.length).toBeGreaterThan(20); + }); + + it.each(providerScripts)("npm run %s is not reachable through an allow rule", (script) => { + const command = `npm run ${script}`; + const reachedBy = (settings.permissions.allow as string[]).filter((rule) => bashRuleMatches(rule, command)); + expect( + reachedBy, + `${command} is provider-backed but matched allow rule(s): ${reachedBy.join(", ")}. ` + + `Narrow the allow pattern rather than relying on ask-over-allow precedence.`, + ).toEqual([]); + }); + + it.each(providerScripts)("npm run %s carries an explicit ask rule", (script) => { + const command = `npm run ${script}`; + const asked = (settings.permissions.ask as string[]).filter((rule) => bashRuleMatches(rule, command)); + expect(asked.length, `${command} is provider-backed but has no ask rule in .claude/settings.json`).toBeGreaterThan( + 0, + ); + }); + + it("denies reading local env files", () => { + const deny = settings.permissions.deny as string[]; + for (const target of ["Read(./.env)", "Read(./.env.local)"]) { + expect(deny, `${target} must stay denied — a staging key leaked on 2026-08-18`).toContain(target); + } + }); +}); + +describe("claude hook registrations", () => { + const commands: { event: string; command: string }[] = []; + for (const [event, matchers] of Object.entries( + settings.hooks as Record, + )) { + for (const matcher of matchers) { + for (const hook of matcher.hooks) commands.push({ event, command: hook.command }); + } + } + + it("registers at least the known hook events", () => { + expect(commands.length).toBeGreaterThanOrEqual(5); + }); + + it.each(commands.map((c) => [c.event, c.command]))( + "%s hook runs through an interpreter, not a bare path: %s", + (_event, command) => { + // A bare `$CLAUDE_PROJECT_DIR/.../foo.sh` depends on the checked-in executable bit, which + // is invisible on this repo's Windows Dev Drive (core.fileMode=false) and was already + // wrong once. Requiring `bash "..."` removes the dependency entirely. + expect( + /^(bash|sh|node|npx) /.test(command as string), + `hook command must start with an interpreter: ${command}`, + ).toBe(true); + }, + ); + + it.each(commands.map((c) => [c.event, c.command]))("%s hook declares an explicit timeout: %s", (event) => { + const matchers = (settings.hooks as Record)[event as string]; + for (const matcher of matchers) { + for (const hook of matcher.hooks) { + // The default is 60s. session-start.sh downloads a Node tarball and runs npm ci on a + // cold container, and a killed hook leaves dependencies half installed. + expect(typeof hook.timeout, `${event} hook is missing a timeout`).toBe("number"); + } + } + }); +}); From e6d5b519d0580dbf8dd19c97273bd08241a7644b Mon Sep 17 00:00:00 2001 From: BigSimmo <87357024+BigSimmo@users.noreply.github.com> Date: Tue, 18 Aug 2026 20:10:48 +0800 Subject: [PATCH 5/7] docs(issues): queue the five follow-ups this session could not close MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Each of these is blocked on something outside the repo, so capturing them is the smallest fix that resolves them — the alternative is that they die with the session context. - P1 rec: PR churn has exhausted both review bots' budgets, so PR #2113 landed with zero automated review and subsequent PRs will too. AGENTS.md already measured the CI half of this cost; this is the second bill and the worse one. Needs a decision on whether the bundling rule gets a gate. - P2 task: nine landed worktrees, ~4.5 GB on a 51%-full Dev Drive, deliberately not removed — re-verification immediately before deletion showed two held unmerged commits despite the scan reporting none minutes earlier, and a third had been switched branches mid-scan by a live session. - P3 task: confirm D:\.npm-cache is a registered Dev Drive trusted cache; fsutil needs elevation and the non-elevated registry fallback reads empty. - P3 task: read the PreCompact hook's own log after the next compaction to settle whether its output reaches model context. The log lives under the worktree's git dir, so check it before cleaning that worktree up. - P3 task: confirm on a real web session that session-start.sh now runs. The mode bug was proven; the failure it would cause on Linux was not, because no container was available. Co-Authored-By: Claude Opus 5 --- .../0f96874b-127c-4fe7-a6cd-349032007e00.json | 14 ++++++++++++++ .../89d7fe98-1873-41e8-bc21-d77cab91663b.json | 14 ++++++++++++++ .../9864a5d7-134d-4115-84ca-11eaddfa6c97.json | 14 ++++++++++++++ .../bbd0c2e5-6e2f-4aa9-802e-31a95ccb2b2d.json | 14 ++++++++++++++ .../ec356a7d-96ec-4739-aa23-4429aa424028.json | 14 ++++++++++++++ 5 files changed, 70 insertions(+) create mode 100644 docs/outstanding-issues-inbox/0f96874b-127c-4fe7-a6cd-349032007e00.json create mode 100644 docs/outstanding-issues-inbox/89d7fe98-1873-41e8-bc21-d77cab91663b.json create mode 100644 docs/outstanding-issues-inbox/9864a5d7-134d-4115-84ca-11eaddfa6c97.json create mode 100644 docs/outstanding-issues-inbox/bbd0c2e5-6e2f-4aa9-802e-31a95ccb2b2d.json create mode 100644 docs/outstanding-issues-inbox/ec356a7d-96ec-4739-aa23-4429aa424028.json diff --git a/docs/outstanding-issues-inbox/0f96874b-127c-4fe7-a6cd-349032007e00.json b/docs/outstanding-issues-inbox/0f96874b-127c-4fe7-a6cd-349032007e00.json new file mode 100644 index 0000000000..ab717cb888 --- /dev/null +++ b/docs/outstanding-issues-inbox/0f96874b-127c-4fe7-a6cd-349032007e00.json @@ -0,0 +1,14 @@ +{ + "version": 2, + "id": "0f96874b-127c-4fe7-a6cd-349032007e00", + "createdOn": "2026-08-18", + "action": "add", + "payload": { + "pri": "P3", + "type": "task", + "summary": "Confirm D:\\.npm-cache is a registered Dev Drive trusted cache, or Defender is scanning every npm ci", + "detail": "The repo lives on a Windows Dev Drive (D:, ReFS, 50 GB) and npm config get cache resolves to D:\\.npm-cache, which is correctly on the same volume. Whether that path is registered as a Dev Drive TRUSTED cache is unverified: 'fsutil devdrv query D:' returns 'Failed to open the volume. Error 5: Access is denied' without elevation, and the non-elevated registry fallback (HKLM:\\SYSTEM\\CurrentControlSet\\Control\\FileSystem, FilterAttachModeOnDevDrive and DevDriveTrustSetting) reads empty. If it is not registered, Microsoft Defender real-time scanning runs over every npm ci — and this machine performs a lot of them: 21 D: worktrees each carry their own ~0.89 GB / 51,735-file node_modules, because npm extracts fresh copies rather than hardlinking from cache (ReFS does support hardlinks here, probed directly, but npm does not use them). Next: from an ELEVATED prompt run 'fsutil devdrv query D:' and, if the cache is not listed as trusted, 'fsutil devdrv trust D:\\.npm-cache'. Cheap, one-off, no code change. Not blocking anything.", + "source": "session 2026-08-18; fsutil Error 5 without elevation", + "issueUlid": "01M0ACEE5B6SMMB4SCK225BG40" + } +} diff --git a/docs/outstanding-issues-inbox/89d7fe98-1873-41e8-bc21-d77cab91663b.json b/docs/outstanding-issues-inbox/89d7fe98-1873-41e8-bc21-d77cab91663b.json new file mode 100644 index 0000000000..37c94cc548 --- /dev/null +++ b/docs/outstanding-issues-inbox/89d7fe98-1873-41e8-bc21-d77cab91663b.json @@ -0,0 +1,14 @@ +{ + "version": 2, + "id": "89d7fe98-1873-41e8-bc21-d77cab91663b", + "createdOn": "2026-08-18", + "action": "add", + "payload": { + "pri": "P3", + "type": "task", + "summary": "Confirm from its own log whether the PreCompact hook's output actually reaches model context", + "detail": ".claude/hooks/precompact-issues-capture.sh was added in PR #2113 to ask for /issues capture BEFORE compaction discards the follow-ups it wants recorded; the pre-existing issues-surface.sh reminder is a SessionStart hook and therefore fires AFTER compaction, when the material is already gone. Claude Code is known to inject hook stdout into model context for SessionStart, UserPromptSubmit, PreToolUse and PostToolUse. Whether it does so for PreCompact could NOT be determined: the installed CLI at %APPDATA%\\npm\\node_modules\\@anthropic-ai\\claude-code ships a compiled claude.exe with no inspectable JS bundle to grep. The hook therefore prints plain text rather than a hookSpecificOutput JSON envelope, so that if the platform does not inject it the operator still sees a clean transcript line instead of a raw JSON blob, and it appends one line per firing to a log so the question is answerable rather than permanently open. Next, after any compaction in a session using this repo: cat \"$(git rev-parse --absolute-git-dir)/claude-precompact.log\". Lines present but no reminder seen in context means the limit is real and the SessionStart backstop is carrying it. No lines at all means the registration is wrong. WARNING: that log lives under the worktree's own git dir, so it is destroyed when the worktree is removed — check it before cleaning up the worktree this was authored in.", + "source": "PR #2113 .claude/hooks/precompact-issues-capture.sh", + "issueUlid": "01M0ACENGDRZQQBT0WJ6MSQ6KS" + } +} diff --git a/docs/outstanding-issues-inbox/9864a5d7-134d-4115-84ca-11eaddfa6c97.json b/docs/outstanding-issues-inbox/9864a5d7-134d-4115-84ca-11eaddfa6c97.json new file mode 100644 index 0000000000..f662470947 --- /dev/null +++ b/docs/outstanding-issues-inbox/9864a5d7-134d-4115-84ca-11eaddfa6c97.json @@ -0,0 +1,14 @@ +{ + "version": 2, + "id": "9864a5d7-134d-4115-84ca-11eaddfa6c97", + "createdOn": "2026-08-18", + "action": "add", + "payload": { + "pri": "P1", + "type": "rec", + "summary": "PR churn has exhausted the review-bot budget, so PRs are now landing with no automated review at all", + "detail": "CodeRabbit on PR #2113: '101 included PR reviews in the past 7 days; at that activity level, included reviews refill at 1 review per hour. Your organization has reached its usage spending cap.' The Codex connector reported its own usage limit on the same PR. Net effect: #2113 received ZERO automated review, and so will subsequent PRs until the cap resets or credits are added. AGENTS.md 'PR bundling' already measured the CI half of this cost on 2026-07-30 (437 PR-triggered runs over ~3 days, ~40% cancelled mid-run, ~12 Production-UI-hours burned on runs that never completed). This is the second bill for the same behaviour and the more dangerous one, because CI waste is money while missing review is undetected defects — and the PRs most likely to need review are the ones landing during a churn spike. The bundling rule exists as prose in AGENTS.md and is evidently not binding; the newtask skill also asks the question in prose. Decide whether it gets a gate. Note the repo has already learned this lesson once in a different area: .claude/hooks/pr-handoff-stop.sh states in its own header that 'prose rules in AGENTS.md have not held, a denied tool call does.' Next: decide between (a) a push/PR-creation gate that refuses a new branch when an open PR of the same scope exists, (b) raising the bot spending cap, or (c) accepting unreviewed merges deliberately rather than by accident. Stop rule: do not weaken any required check to compensate for missing bot review.", + "source": "CodeRabbit + Codex connector comments on PR #2113, 2026-08-18; AGENTS.md 'PR bundling (reduce one-task-one-PR churn)'", + "issueUlid": "01M0ACDDJQCCZ4HB9R4TK678FX" + } +} diff --git a/docs/outstanding-issues-inbox/bbd0c2e5-6e2f-4aa9-802e-31a95ccb2b2d.json b/docs/outstanding-issues-inbox/bbd0c2e5-6e2f-4aa9-802e-31a95ccb2b2d.json new file mode 100644 index 0000000000..f0abd7e201 --- /dev/null +++ b/docs/outstanding-issues-inbox/bbd0c2e5-6e2f-4aa9-802e-31a95ccb2b2d.json @@ -0,0 +1,14 @@ +{ + "version": 2, + "id": "bbd0c2e5-6e2f-4aa9-802e-31a95ccb2b2d", + "createdOn": "2026-08-18", + "action": "add", + "payload": { + "pri": "P3", + "type": "task", + "summary": "Confirm on a real Claude Code web session that the session-start hook now runs, after the exec-bit fix", + "detail": "PR #2113 fixed .claude/hooks/session-start.sh, which was checked in as mode 100644 while both sibling hooks were 100755, and was the only hook registered by bare path rather than through bash. What was PROVEN: the index mode, the bare-path registration, and that core.fileMode=false on this Windows ReFS Dev Drive hides both (a local chmod +x is a silent no-op; only git update-index --chmod=+x works). What was NOT proven: that it actually failed on a Linux web container, because no container was available to test from. The 100755/100755/100644 asymmetry makes accident overwhelmingly likely rather than a deliberate choice, and the script's whole body is gated on CLAUDE_CODE_REMOTE=true so the web container is the only place it does any work — it provisions the Node 24 the engine floor requires, after npm ci EBADENGINE blocked PRs #1611, #1697, #1705 and #1740. This is confirmation, not risk: the registration now uses bash \"$CLAUDE_PROJECT_DIR/...\", which removes the dependency on the mode entirely, and tests/session-start-hook.test.ts pins every hook at 100755 with LF-only line endings while tests/claude-code-settings.test.ts pins every hook command to start with an interpreter. Next: on the first Claude Code web session on this repo, check the session start output for the '[session-start] Using node ...' line and confirm npm ci ran. If it did not, the failure is something other than the exec bit and this item becomes a real defect rather than a confirmation.", + "source": "PR #2113; AGENTS.md 'Claude Code hook scripts'", + "issueUlid": "01M0ACF5KP164Z0HTZBWDPG8C3" + } +} diff --git a/docs/outstanding-issues-inbox/ec356a7d-96ec-4739-aa23-4429aa424028.json b/docs/outstanding-issues-inbox/ec356a7d-96ec-4739-aa23-4429aa424028.json new file mode 100644 index 0000000000..c249984b97 --- /dev/null +++ b/docs/outstanding-issues-inbox/ec356a7d-96ec-4739-aa23-4429aa424028.json @@ -0,0 +1,14 @@ +{ + "version": 2, + "id": "ec356a7d-96ec-4739-aa23-4429aa424028", + "createdOn": "2026-08-18", + "action": "add", + "payload": { + "pri": "P2", + "type": "task", + "summary": "Nine landed worktrees are still on disk holding ~4.5 GB on a 51%-full Dev Drive; removal was deferred because the fleet was live", + "detail": "clean-worktree.mjs gained list-only --merged and --squashed in PR #2113 and identified 9 landed worktrees across the 50-worktree fleet, 5 of them on D:. Removal was NOT performed and must not be run blind. Re-verifying each candidate immediately before deletion, twice, showed the fleet is actively worked: database-coordination-chat-9c8cbd and database-drift-remeasure-phase2-7c4215 each held 2 unmerged commits despite the scan minutes earlier reporting '0 commits ahead', their newest files were written the same afternoon, and bundle-baseline had been switched to a different branch mid-scan and was running Playwright (the push guard named it as holding the heavy-run lease). Deleting any of them would have destroyed unmerged work. Next: run 'node scripts/clean-worktree.mjs --merged --squashed' when no other Codex/Gemini/Claude session is active, read the confidence line on each candidate, and re-run with --remove. Skip any candidate marked 'NOT fully corroborated' — that label means the patch-id test inferred the landing but some changed files still differ from origin/main, which is usually base churn but is not proof. D: was 25.3 GB of 50 GB used with roughly 19 GB of that duplicated node_modules across 21 worktrees at ~0.89 GB each. Ignore the C: worktrees entirely; they belong to Codex and Antigravity sessions. Stop rule: never pass --force to git worktree remove, and never remove a worktree that is ahead of origin/main.", + "source": "PR #2113 scripts/clean-worktree.mjs; live fleet re-verification 2026-08-18", + "issueUlid": "01M0ACDZTT6GW95D8BVH6EAHKV" + } +} From bfc0e1a0aeff5f6fc2cf094cad5a61dd8d8525a4 Mon Sep 17 00:00:00 2001 From: BigSimmo <87357024+BigSimmo@users.noreply.github.com> Date: Tue, 18 Aug 2026 20:32:35 +0800 Subject: [PATCH 6/7] fix(clean-worktree): stop the candidate listing claiming "0 commits ahead" when it isn't MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every `--squashed` candidate printed "0 commits ahead" while being genuinely ahead of origin/main — by 11, 2, 1 commits on the real fleet. A squash-merged branch keeps its original commits forever, so it stays ahead permanently; what is zero is the count of UNLANDED commits, which is a different claim. The line was therefore stating something a reader could disprove with one `git rev-list`, on a tool whose entire job is to be trusted before a deletion. It now reports both numbers: "11 ahead of origin/main, 0 unlanded commits". Also corrects the comment above the ahead check, which described it as belt-and-braces against the merge test. That is true in ancestor mode, where it is the real gate. In squash mode it is not a second opinion at all — gitAheadUnlandedCount returns 0 for any branch the squash test just accepted, so the check is satisfied by construction and can only fire on a candidate that was already skipped. Keeping it is correct; describing it as independent evidence was not. The raw count is reporting only and never gates, so no candidate set changes. Verified against the live fleet: same 9 candidates before and after, worktree count unchanged, --remove not run. Co-Authored-By: Claude Opus 5 --- scripts/clean-worktree.mjs | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/scripts/clean-worktree.mjs b/scripts/clean-worktree.mjs index e93e5e0839..40321f9f56 100644 --- a/scripts/clean-worktree.mjs +++ b/scripts/clean-worktree.mjs @@ -113,6 +113,9 @@ export function identifyMergedWorktrees( isMergedFn = () => false, statusFn = () => "", aheadCountFn = () => 0, + // Raw `baseRef..branch` count, used for REPORTING only — never as a gate. See the note at + // the ahead check below for why the two counters have to be reported separately. + rawAheadCountFn = null, existsFn = existsSync, mainPath = null, currentPath = null, @@ -152,15 +155,30 @@ export function identifyMergedWorktrees( const status = statusFn(wt.path); if (typeof status !== "string" || status.trim() !== "") continue; - // Belt-and-braces against the ancestor test: refs move underneath long-lived worktrees, - // and a non-numeric answer (git failed) must fail closed rather than read as zero. + // Under the ANCESTOR test this is belt-and-braces: refs move underneath long-lived + // worktrees, and a non-numeric answer (git failed) must fail closed rather than read as + // zero. Under the SQUASH test it is not a second opinion at all — `gitAheadUnlandedCount` + // returns 0 for any branch the squash test just accepted, so the check is satisfied by + // construction and can only fire on a candidate that was already skipped. It is kept + // because it is the real gate in ancestor mode, not because it adds anything in squash mode. const ahead = aheadCountFn(wt.branch, baseRef); if (!Number.isFinite(ahead) || ahead !== 0) continue; + // Report the RAW count alongside it. A squash-merged branch keeps its original commits + // forever, so it stays genuinely ahead of the base — 3, 4, even 19 commits — while the + // unlanded count is 0. Printing a bare "0 commits ahead" therefore stated something the + // reader could disprove in one `git rev-list` and made the whole line look untrustworthy. + // Observed 2026-08-18 reviewing a real fleet: every squash candidate read "0 commits + // ahead" while being ahead by 3 to 19. + const rawAhead = (rawAheadCountFn ?? aheadCountFn)(wt.branch, baseRef); + const aheadNote = Number.isFinite(rawAhead) && rawAhead !== ahead ? `${rawAhead} ahead of ${baseRef}, ` : ""; + merged.push({ ...wt, mergedInto: baseRef, - reason: `branch merged into ${baseRef}; clean tree; 0 commits ahead${wt.head ? ` (tip ${wt.head.slice(0, 9)})` : ""}`, + aheadUnlanded: ahead, + aheadRaw: Number.isFinite(rawAhead) ? rawAhead : null, + reason: `branch merged into ${baseRef}; clean tree; ${aheadNote}0 unlanded commits${wt.head ? ` (tip ${wt.head.slice(0, 9)})` : ""}`, }); } return merged; @@ -750,6 +768,7 @@ export function runMergedWorktreeReport(options = {}) { isMergedFn: squashed ? gitBranchSquashMerged : gitBranchIsAncestor, statusFn: gitWorktreeStatus, aheadCountFn: squashed ? gitAheadUnlandedCount : gitAheadCount, + rawAheadCountFn: gitAheadCount, existsFn: existsSync, currentPath, baseRef, From ce2ace1e61b3470bcaa9ed45d0fb110b687e188e Mon Sep 17 00:00:00 2001 From: BigSimmo <87357024+BigSimmo@users.noreply.github.com> Date: Tue, 18 Aug 2026 20:34:35 +0800 Subject: [PATCH 7/7] docs(ledger): record the follow-up review for PR #2117 Co-Authored-By: Claude Opus 5 --- ...f1cc5792e772b070306c269a353bebc430402d91df50a38a5b1.record.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 docs/branch-review-records/028b986193b51f1cc5792e772b070306c269a353bebc430402d91df50a38a5b1.record.md diff --git a/docs/branch-review-records/028b986193b51f1cc5792e772b070306c269a353bebc430402d91df50a38a5b1.record.md b/docs/branch-review-records/028b986193b51f1cc5792e772b070306c269a353bebc430402d91df50a38a5b1.record.md new file mode 100644 index 0000000000..89c09e2ad3 --- /dev/null +++ b/docs/branch-review-records/028b986193b51f1cc5792e772b070306c269a353bebc430402d91df50a38a5b1.record.md @@ -0,0 +1 @@ +| 2026-08-18 | claude/code-setup-review-a95519 | bfc0e1a0aeff5f6fc2cf094cad5a61dd8d8525a4 | Follow-up to PR #2113: five outstanding-issues inbox requests, plus a reporting-only correction in clean-worktree.mjs | shipped as PR #2117. Five follow-ups captured that #2113 could not close, each blocked on something outside the repo: deferred worktree cleanup, elevated fsutil devdrv check, PreCompact context-injection confirmation, session-start.sh confirmation on a web container, and a P1 that PR churn has exhausted both review bots so #2113 landed with zero automated review. Separately fixed a false reassurance shipped in #2113: every --squashed candidate printed '0 commits ahead' while genuinely 11/2/1 commits ahead of origin/main, because a squash-merged branch keeps its commits forever and only the UNLANDED count is zero; the line now reports both numbers. Corrected the comment calling the ahead check belt-and-braces, which is true in ancestor mode but tautological in squash mode since gitAheadUnlandedCount returns 0 for any branch the squash test just accepted. Raw count is reporting-only and never gates. Two landed worktrees removed manually (fleet 50 to 48, D: 51 to 48 percent full); seven left in place, one in active use, two not fully corroborated, four on C: belonging to other agents' sessions | clean-worktree --self-test passed; --merged --squashed run against the live 48-worktree fleet before and after with an identical 9-candidate set and worktree count unchanged, --remove not run; check:outstanding-issues passed (361 rows, 105 open, collision-free); check:ledger-write-discipline passed 5ae2bb6ec703..HEAD; prettier --check and format:check clean; eslint clean; pr-policy classifier all four risk flags false; NOT run: full unit suite (diff is five JSON request files plus a reporting-only string in a maintenance script no product code imports), verify:ui, and all provider-backed gates |