diff --git a/.claude/hooks/precompact-issues-capture.sh b/.claude/hooks/precompact-issues-capture.sh new file mode 100755 index 0000000000..9b9af56fcb --- /dev/null +++ b/.claude/hooks/precompact-issues-capture.sh @@ -0,0 +1,65 @@ +#!/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." + +# 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/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..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 @@ -45,15 +47,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 +91,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/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 | 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/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"); + } + } + }); +}); diff --git a/tests/session-start-hook.test.ts b/tests/session-start-hook.test.ts index 49a32b1df4..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"); @@ -158,3 +166,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); + }, + ); +});