diff --git a/.claude/hooks/pr-handoff-stop.sh b/.claude/hooks/pr-handoff-stop.sh index 6f82a270ac..8cbb6017c4 100755 --- a/.claude/hooks/pr-handoff-stop.sh +++ b/.claude/hooks/pr-handoff-stop.sh @@ -47,9 +47,104 @@ set -uo pipefail mode="${1:-}" -payload="$(cat 2>/dev/null || true)" +case "$mode" in +post | pre) ;; +*) exit 0 ;; +esac + +# Read stdin with the `read` builtin instead of `$(cat)`: one fewer fork+exec on +# every Bash/PowerShell call, and not slower — measured from 1 KB to 2 MB it wins +# below 256 KB and draws above. `read -d ''` consumes to EOF and reports non-zero +# *there* having already filled the variable, so `|| true` is the expected path. +payload="" +IFS= read -r -d '' payload || true [ -z "$payload" ] && exit 0 +# --- fast reject, before any subprocess --------------------------------------- +# Both modes are registered on EVERY Bash/PowerShell call, and in almost all of +# them there is nothing to do. Reaching that conclusion used to cost four jq +# runs, a grep and a `git rev-parse` — on Windows/Git Bash that is roughly two +# seconds of pure process-spawn latency, twice per tool call. Everything under +# this banner uses shell builtins only. +# +# Each test below is a deliberate SUPERSET of the decision it stands in for, so +# it can only ever let MORE through, never less. Anything it rejects would have +# reached the same exit further down, just slower. + +# Post mode acts only when tool_response carries a `github.com/<…>/pull/` URL +# (the URL gate in the post branch). That match is case-sensitive, so the raw +# payload must contain the lowercase bytes `pull`; JSON escaping `/` as `\/` +# cannot hide them. The `\u` arm keeps the claim airtight against a hypothetical +# encoder emitting `\u0070ull`. +if [ "$mode" = post ]; then + case "$payload" in + *pull* | *\\u*) ;; + *) exit 0 ;; + esac +fi + +# Resolve the git directory the way `git rev-parse --absolute-git-dir` does, with +# builtins only. fast_git_dir is left EMPTY whenever the answer is not certain — +# any of git's own discovery controls being set, a `.git` file that does not +# resolve, a cwd that is itself a git dir, or no repository above cwd — and every +# one of those falls through to the authoritative `git rev-parse` in the marker +# section below. +# +# The discovery controls are load-bearing, not decoration. GIT_CEILING_DIRECTORIES +# in particular stops git ascending, so from a ceiling-excluded subdirectory git +# reports NO repository and the marker belongs in TMPDIR — while a naive upward +# walk finds the excluded checkout's `.git` and puts it there instead. Those two +# answers disagreeing across a post/pre pair is exactly how this guard would stop +# firing, so treat every discovery control as uncertain rather than guessing. +fast_git_dir="" +resolve_fast_git_dir() { + # Keep in sync with git's discovery controls; an unrecognised one must fail + # closed to `git rev-parse`, never be walked past. + [ -n "${GIT_DIR:-}${GIT_COMMON_DIR:-}${GIT_WORK_TREE:-}${GIT_CEILING_DIRECTORIES:-}${GIT_DISCOVERY_ACROSS_FILESYSTEM:-}" ] && return 0 + # cwd is itself a git dir (bare repo): walking up would find a different repo. + [ -f "$PWD/HEAD" ] && [ -d "$PWD/objects" ] && [ -d "$PWD/refs" ] && return 0 + local dir="$PWD" line target + while :; do + if [ -d "$dir/.git" ]; then + fast_git_dir="$dir/.git" + return 0 + fi + if [ -f "$dir/.git" ]; then + # Linked worktree: a one-line `gitdir: ` pointer. + IFS= read -r line <"$dir/.git" 2>/dev/null || return 0 + line="${line%$'\r'}" + case "$line" in + "gitdir: "*) + target="${line#gitdir: }" + case "$target" in + /* | [A-Za-z]:[/\\]*) ;; + *) target="$dir/$target" ;; + esac + [ -d "$target" ] && fast_git_dir="$target" + ;; + esac + return 0 + fi + case "$dir" in "" | /) break ;; esac + dir="${dir%/*}" + [ -z "$dir" ] && dir=/ + done + return 0 +} +resolve_fast_git_dir + +# Pre mode's entire job is gated on this session's marker existing (the first +# line of the pre branch), and no marker is written until this session opens a +# PR. The id is read with bash's own regex engine and accepted only in the same +# safe charset the full parse enforces, so a path-injection id matches nothing +# here and falls through to be rejected there. `-e`, not `-f`: anything present +# at that path defers to the slow path rather than short-circuiting it. +if [ "$mode" = pre ] && [ -n "$fast_git_dir" ]; then + if [[ "$payload" =~ \"session_id\"[[:space:]]*:[[:space:]]*\"([A-Za-z0-9_-]+)\" ]]; then + [ -e "$fast_git_dir/claude-pr-handoff-${BASH_REMATCH[1]}" ] || exit 0 + fi +fi + # Extract a JSON string value for key $1 from the raw payload (first match). # Handles only simple double-quoted values (no escapes). Empty on miss. # Callers that need shell-token matching must also scan $payload when @@ -148,8 +243,11 @@ fi # --- marker location ---------------------------------------------------------- # Absolute git dir so the marker path is valid from any cwd (and for linked -# worktrees). Falls back to TMPDIR outside a repo. -git_dir="$(git rev-parse --absolute-git-dir 2>/dev/null || true)" +# worktrees). The builtin resolver above answers this without spawning git in +# the ordinary cases; `git rev-parse` stays the authority everywhere it did not. +# Falls back to TMPDIR outside a repo. +git_dir="$fast_git_dir" +[ -z "$git_dir" ] && git_dir="$(git rev-parse --absolute-git-dir 2>/dev/null || true)" [ -z "$git_dir" ] && git_dir="${TMPDIR:-/tmp}" marker="$git_dir/claude-pr-handoff-$session_id" diff --git a/.claude/hooks/push-format-guard.sh b/.claude/hooks/push-format-guard.sh index ee5e40de0b..8a395f8e63 100755 --- a/.claude/hooks/push-format-guard.sh +++ b/.claude/hooks/push-format-guard.sh @@ -25,9 +25,34 @@ # still downstream. set -uo pipefail -payload="$(cat 2>/dev/null || true)" +# Read stdin with the `read` builtin instead of `$(cat)`. That is one fewer +# fork+exec on every single Bash/PowerShell tool call, and it is not slower: +# measured against payloads from 1 KB to 2 MB it is faster below 256 KB and +# level above. `read -d ''` consumes to EOF and reports non-zero *there* having +# already filled the variable, so the `|| true` is the expected path. +payload="" +IFS= read -r -d '' payload || true [ -z "$payload" ] && exit 0 +# --- cheapest possible discriminator, before any subprocess ------------------- +# This hook runs on EVERY Bash/PowerShell call but acts only on `git push`, and +# reaching "not a push" used to cost two jq runs, a grep and a subshell — on +# Windows/Git Bash that is well over a second of pure process-spawn latency per +# tool call. The test below uses shell builtins only. +# +# It is deliberately a SUPERSET of the real check further down +# (`git[[:space:]]+push`), so it can only ever let MORE through, never less: that +# regex cannot match unless the bytes `git` appear before the bytes `push`, +# whether the command reaches it jq-decoded, grep-extracted, or as the raw +# payload. The `\u` arm covers the one theoretical gap in that argument — an +# encoder emitting `\u0067it push` — at the cost of taking the slow path for the +# rare payload carrying a unicode escape. Anything rejected here would have +# exited at that regex anyway. +case "$payload" in +*git*push* | *\\u*) ;; +*) exit 0 ;; +esac + # --- 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)" diff --git a/tests/pr-handoff-stop.test.ts b/tests/pr-handoff-stop.test.ts index 039f0ba05d..8ac585d4ef 100644 --- a/tests/pr-handoff-stop.test.ts +++ b/tests/pr-handoff-stop.test.ts @@ -462,6 +462,50 @@ describe.skipIf(process.platform === "win32")("pr-babysit budget hook", () => { expect(out.stdout).toBe(""); }); + it("honours GIT_CEILING_DIRECTORIES instead of walking into an excluded checkout", () => { + // The builtin git-dir resolver added for hook latency must never find a repository + // `git rev-parse --absolute-git-dir` would refuse to discover. From a ceiling-excluded + // subdirectory git reports NO repository, so the marker belongs in TMPDIR — a naive + // upward walk instead lands it in the excluded checkout's .git. A post/pre pair + // straddling that disagreement is how the budget would silently stop being enforced. + const { root, gitDir } = freshRepo(); + const sub = join(root, "sub"); + mkdirSync(sub); + const tmp = mkdtempSync(join(tmpdir(), "pr-handoff-ceiling-")); + scratchRoots.push(tmp); + const env = { GIT_CEILING_DIRECTORIES: root, TMPDIR: tmp }; + + const post = spawnSync("bash", [hook, "post"], { + cwd: sub, + input: JSON.stringify({ + tool_name: "create_pull_request", + session_id: "sess-ceiling", + tool_response: "Opened https://github.com/BigSimmo/Database/pull/1649", + }), + encoding: "utf8", + env: { ...process.env, ...env }, + }); + expect(post.status).toBe(0); + expect( + existsSync(join(gitDir, "claude-pr-handoff-sess-ceiling")), + "must not write into a checkout git refuses to discover", + ).toBe(false); + expect( + existsSync(join(tmp, "claude-pr-handoff-sess-ceiling")), + "must fall back to TMPDIR exactly as `git rev-parse` finding nothing requires", + ).toBe(true); + + // And pre-mode must read back the same location, so the budget still bites. + const pre = spawnSync("bash", [hook, "pre"], { + cwd: sub, + input: JSON.stringify({ tool_name: "CronCreate", session_id: "sess-ceiling" }), + encoding: "utf8", + env: { ...process.env, ...env }, + }); + expect(pre.status).toBe(0); + expect(pre.stdout).toContain('"permissionDecision":"deny"'); + }); + it("does nothing at all when no PR was ever opened", () => { const { root } = freshRepo(); const out = runHook(