From 2ad32de9b79d5986a812069657a26589506417b3 Mon Sep 17 00:00:00 2001 From: BigSimmo <87357024+BigSimmo@users.noreply.github.com> Date: Thu, 6 Aug 2026 23:45:31 +0800 Subject: [PATCH 1/9] feat(hooks): stop a session following its own PR after handoff MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A session that opens a PR and then stays attached to it — polling `gh pr checks`, watching workflow runs, re-running jobs, syncing the branch, replying to review bots, or parking a Monitor/wake-up on it — burns a long tail of usage on work nobody asked for. Claude Code on the web is the worst case: the cloud session keeps running once the PR exists, so nothing naturally ends the loop. Nothing in the repo said to stop, so add the rule and enforce it: - `.claude/hooks/pr-handoff-stop.sh` writes a session-scoped marker (inside the worktree git dir, never committed) when a PR-creating call — `gh pr create` or any `create_pull_request` MCP tool — returns a real PR URL, and injects a "handoff complete, stop" note into context. - While that marker exists, the PreToolUse half denies shell CI polling, GitHub MCP PR/CI tools, and the loop machinery (Monitor / ScheduleWakeup / CronCreate). Committing, pushing, ledger appends, and PR create/merge stay allowed. - A failed create writes no marker, so a session is never stranded without a PR to hand off. Sessions that never create a PR are untouched, which leaves `Run PR` sweeps, pr-ci-fix work, and reviews of other people's PRs working. - Unlock on an explicit user ask: prefix a shell command with `CLAUDE_ALLOW_PR_FOLLOW=1`, or delete the marker the deny message names. AGENTS.md gains the "Stop when the pull request is open" section and the handoff skill gains a closing "Stop" step so the rule is stated as well as enforced. Co-Authored-By: Claude Opus 5 --- .claude/hooks/pr-handoff-stop.sh | 149 +++++++++++++++++++++++++++++++ .claude/settings.json | 22 +++++ .claude/skills/handoff/SKILL.md | 6 ++ AGENTS.md | 39 ++++++++ 4 files changed, 216 insertions(+) create mode 100644 .claude/hooks/pr-handoff-stop.sh diff --git a/.claude/hooks/pr-handoff-stop.sh b/.claude/hooks/pr-handoff-stop.sh new file mode 100644 index 0000000000..e5ac378515 --- /dev/null +++ b/.claude/hooks/pr-handoff-stop.sh @@ -0,0 +1,149 @@ +#!/usr/bin/env bash +# Stop the session once its pull request exists. +# +# Why: an agent session (especially Claude Code on the web, which keeps running +# after the PR is opened) that stays attached to its own PR burns usage in a +# long tail of `gh pr checks` / `gh run watch` / branch-sync polling, or of +# Monitor/wake-up loops parked on the same PR. Opening the PR is the handoff; +# CI, review bots, and merge are the user's call from there. +# +# Two modes, both registered in .claude/settings.json: +# +# post PostToolUse — after a call that created a PR (the `gh pr create` CLI or +# a GitHub MCP create_pull_request tool) AND whose output contains a real +# PR URL, drop a session-scoped marker and tell the model the handoff is +# complete and the turn should end. +# +# pre PreToolUse — while that marker exists, deny the PR/CI-following tools: +# shell polling commands, GitHub MCP PR/CI read tools, and the loop +# machinery (Monitor / ScheduleWakeup / CronCreate). This is the part +# with teeth: prose rules in AGENTS.md have not held, a denied tool call +# does. +# +# Escape hatch: prefix a shell command with CLAUDE_ALLOW_PR_FOLLOW=1 (the user +# asking for CI to be watched is the authorisation). For non-shell tools the +# unlock is deleting the marker, which the deny reason spells out — do that only +# on an explicit user ask. +# +# `Run PR` sweeps, pr-ci-fix work, and reviews of someone else's PR are +# untouched: they never create a PR, so no marker is ever written. +# +# Contract: never fails a tool call by accident. Any parse problem exits 0 with +# no decision, which leaves the tool call exactly as it was. +set -uo pipefail + +mode="${1:-}" +payload="$(cat 2>/dev/null || true)" +[ -z "$payload" ] && exit 0 + +# --- payload fields ----------------------------------------------------------- +# jq when available (exact); otherwise fall back to scanning the raw payload, +# which is a superset of the command text and good enough for substring matching. +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 // empty' 2>/dev/null || true)" + tool_output="$(printf '%s' "$payload" | jq -r '[.tool_response] | tostring' 2>/dev/null || true)" + session_id="$(printf '%s' "$payload" | jq -r '.session_id // empty' 2>/dev/null || true)" +else + tool_name="$(printf '%s' "$payload" \ + | grep -o '"tool_name"[[:space:]]*:[[:space:]]*"[^"]*"' \ + | head -n1 | sed -E 's/.*"([^"]*)"$/\1/')" + command_text="$payload" + tool_output="$payload" + session_id="$(printf '%s' "$payload" \ + | grep -o '"session_id"[[:space:]]*:[[:space:]]*"[^"]*"' \ + | head -n1 | sed -E 's/.*"([^"]*)"$/\1/')" +fi + +[ -z "$session_id" ] && session_id="unknown-session" + +# --- marker location ---------------------------------------------------------- +# Inside the git dir so it is never committed, and resolves per-worktree. +git_dir="$(git rev-parse --git-dir 2>/dev/null || true)" +[ -z "$git_dir" ] && git_dir="${TMPDIR:-/tmp}" +marker="$git_dir/claude-pr-handoff-$session_id" + +json_escape() { + printf '%s' "$1" | sed -e 's/\\/\\\\/g' -e 's/"/\\"/g' | tr '\n' ' ' +} + +is_shell_tool() { + case "$tool_name" in + "" | Bash | PowerShell) return 0 ;; + *) return 1 ;; + esac +} + +# A PR-creating or PR-merging tool is never blocked — creation is how the PR gets +# opened, and merge already carries its own explicit-confirmation rule. +matches_pr_write_tool() { + printf '%s' "$tool_name" \ + | grep -Eqi '(create|merge)_?pull_?request$' +} + +case "$mode" in +post) + # A PR-creating call that actually returned a PR URL. Both halves matter: + # without the URL check a failed create would end the session with no PR to + # hand off. + created=1 + if is_shell_tool && printf '%s' "$command_text" | grep -Eq 'gh[[:space:]]+pr[[:space:]]+create'; then + created=0 + fi + if printf '%s' "$tool_name" | grep -Eqi 'create_?pull_?request'; then + created=0 + fi + [ "$created" -eq 0 ] || exit 0 + printf '%s' "$tool_output" | grep -Eq 'github\.com/[^ "]+/pull/[0-9]+' || exit 0 + + printf 'pr-opened %s\n' "$(date -u +%Y-%m-%dT%H:%M:%SZ 2>/dev/null || echo unknown)" >"$marker" 2>/dev/null || true + + context='The pull request is open. That is the end of this session'\''s handoff. Record the ledger row if it is still owed, give the user the PR URL and a short summary, then stop. Do NOT watch CI, poll checks, read workflow runs or job logs, re-run workflows, sync the branch from main, answer review bots, or park a Monitor / ScheduleWakeup / cron loop on this PR — following the PR from here is the wasted-usage loop this repo has explicitly ruled out (AGENTS.md "Stop when the pull request is open"). Those tools are now denied for the rest of this session. If the user asks for CI babysitting afterwards, that is their call and it is allowed then.' + printf '{"hookSpecificOutput":{"hookEventName":"PostToolUse","additionalContext":"%s"}}\n' "$(json_escape "$context")" + exit 0 + ;; + +pre) + [ -f "$marker" ] || exit 0 + matches_pr_write_tool && exit 0 + + reason="" + + # 1. Loop machinery — parking a wake-up or watcher on the PR is the same loop + # by another route, and hooks are the only thing that can see these. + case "$tool_name" in + Monitor | ScheduleWakeup | CronCreate) + reason='Blocked: this session already opened its pull request, so parking a watcher/wake-up/cron on it is the wasted-usage loop AGENTS.md "Stop when the pull request is open" rules out. Hand the PR URL to the user and stop.' + ;; + esac + + # 2. GitHub MCP tools that read or mutate PR / CI state. + if [ -z "$reason" ] && printf '%s' "$tool_name" \ + | grep -Eqi 'pull_?request|workflow_run|workflow_job|check_run|check_suite|job_log|pr_status|update_branch'; then + reason='Blocked: this session already opened its pull request, so reading or nudging its CI and review state is the wasted-usage loop AGENTS.md "Stop when the pull request is open" rules out. Hand the PR URL to the user and stop.' + fi + + # 3. Shell polling. Narrow on purpose: committing, pushing, ledger appends, and + # `gh pr merge` (already gated on explicit user confirmation) stay allowed. + if [ -z "$reason" ] && is_shell_tool; then + printf '%s' "$command_text" | grep -q 'CLAUDE_ALLOW_PR_FOLLOW=1' && exit 0 + follow_re='gh[[:space:]]+pr[[:space:]]+(checks|status|view|diff|list)' + follow_re="$follow_re"'|gh[[:space:]]+run[[:space:]]+(watch|view|list|rerun|download)' + follow_re="$follow_re"'|gh[[:space:]]+api[^|;]*(actions/runs|check-runs|check-suites|/pulls/)' + follow_re="$follow_re"'|sync:pr-branches' + if printf '%s' "$command_text" | grep -Eq "$follow_re"; then + reason='Blocked: this session already opened its pull request, so following it (CI polling, run logs, branch sync) is the wasted-usage loop AGENTS.md "Stop when the pull request is open" rules out. Hand the PR URL to the user and stop. If the user has asked for this check, re-run it prefixed with CLAUDE_ALLOW_PR_FOLLOW=1.' + fi + fi + + [ -z "$reason" ] && exit 0 + + reason="$reason"' Unlock only on an explicit user ask, by deleting the marker: CLAUDE_ALLOW_PR_FOLLOW=1 rm "'"$marker"'".' + printf '{"hookSpecificOutput":{"hookEventName":"PreToolUse","permissionDecision":"deny","permissionDecisionReason":"%s"}}\n' "$(json_escape "$reason")" + exit 0 + ;; + +*) + exit 0 + ;; +esac diff --git a/.claude/settings.json b/.claude/settings.json index ac0470d2d7..e04d24eb7c 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -17,6 +17,28 @@ } ] } + ], + "PostToolUse": [ + { + "matcher": "Bash|PowerShell|.*[Cc]reate_?[Pp]ull_?[Rr]equest.*", + "hooks": [ + { + "type": "command", + "command": "bash \"$CLAUDE_PROJECT_DIR/.claude/hooks/pr-handoff-stop.sh\" post" + } + ] + } + ], + "PreToolUse": [ + { + "matcher": "Bash|PowerShell|Monitor|ScheduleWakeup|CronCreate|.*[Pp]ull_?[Rr]equest.*|.*workflow_(run|job).*|.*check_(run|suite).*|.*job_log.*|.*update_branch.*", + "hooks": [ + { + "type": "command", + "command": "bash \"$CLAUDE_PROJECT_DIR/.claude/hooks/pr-handoff-stop.sh\" pre" + } + ] + } ] } } diff --git a/.claude/skills/handoff/SKILL.md b/.claude/skills/handoff/SKILL.md index 305e80dbe4..f22a85d91d 100644 --- a/.claude/skills/handoff/SKILL.md +++ b/.claude/skills/handoff/SKILL.md @@ -46,6 +46,12 @@ force-push, or discard work. 7. **Record** the review with `npm run ledger:append`, passing `--ref `, `--head` (the full 40-character SHA), `--scope`, `--outcome`, and `--checks`. Do not hand-write the row into `docs/branch-review-ledger.md`. +8. **Stop.** Report the PR URL and a short summary, then end the turn. Do not follow the + PR from here — no CI polling, no `gh run watch`, no re-runs, no branch sync, no replies + to review bots, no `Monitor`/`ScheduleWakeup`/cron parked on it. That tail is the + wasted-usage loop AGENTS.md "Stop when the pull request is open" rules out, and + `.claude/hooks/pr-handoff-stop.sh` denies those commands, the equivalent GitHub MCP + tools, and that loop machinery for the rest of the session. ## Requires explicit confirmation (do not do automatically) diff --git a/AGENTS.md b/AGENTS.md index 4c65e0f799..e3b8db647c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -561,6 +561,45 @@ Record one `docs/branch-review-ledger.md` row per PR touched (use `--supersede` +## Stop when the pull request is open + +Opening the PR is the end of the session's handoff, not the start of a supervision +shift. A session that stays attached to its own PR — polling `gh pr checks`, watching +workflow runs, re-running failed jobs, syncing the branch from `main` again, replying to +review bots, or scheduling a wake-up/monitor/loop against it — spends a long tail of +usage on work the user has not asked for. Claude Code on the web is the worst case: the +cloud session keeps running after the PR exists, so nothing naturally ends the loop. + +After the PR is created — by `gh pr create` or a GitHub MCP `create_pull_request` tool — +and a PR URL comes back: + +- Finish only what the handoff itself still owes: the `npm run ledger:append` row, and + the PR URL plus a short summary reported to the user. Then **stop**. +- Do not follow the PR. CI results, review-bot findings, branch drift, and the merge + itself are the user's call, and a later session (or an explicit `Run PR` sweep) is + where that work belongs. +- A failing check discovered _before_ you stopped is still worth reporting in that final + summary — reporting it is not the same as staying to fix it. + +Enforcement: `.claude/hooks/pr-handoff-stop.sh` (registered in `.claude/settings.json`) +drops a session-scoped marker when a PR-creating call — `gh pr create` or any +`create_pull_request` MCP tool — returns a real PR URL. For the rest of that session it +then denies three things: + +- **Shell polling** — `gh pr checks|status|view|diff|list`, `gh run watch|view|list|rerun|download`, + `gh api …actions/runs|check-runs|check-suites|/pulls/`, and `sync:pr-branches`. +- **GitHub MCP PR/CI tools** — anything whose tool name carries `pull_request`, + `workflow_run`, `workflow_job`, `check_run`, `check_suite`, `job_log`, or + `update_branch`, so a connector is not a way around the shell rule. +- **Loop machinery** — `Monitor`, `ScheduleWakeup`, and `CronCreate`, which is how a + session parks itself on a PR without running a single command. + +Committing, pushing, ledger appends, and PR create/merge (`gh pr merge`, +`merge_pull_request`) stay allowed. Unlock only on an explicit user ask: prefix a shell +command with `CLAUDE_ALLOW_PR_FOLLOW=1`, or delete the marker the deny message names. +Sessions that never create a PR are untouched, so `Run PR` sweeps, `pr-ci-fix` work, and +review sessions on someone else's PR still function normally. + ## PR bundling (reduce one-task-one-PR churn) Every `newtask`/`handoff` cycle mints a dedicated `claude/` branch and PR, so a From 5d96617f2d03b66a224ec0724e6b8e4372c31742 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 6 Aug 2026 15:57:44 +0000 Subject: [PATCH 2/9] fix(hooks): harden pr-handoff-stop against false locks (#1649) - jq-less post mode only scans tool_response for PR URLs, so a failed gh pr create whose body mentions another PR no longer writes the marker - use --absolute-git-dir so the marker path is cwd-independent - read PowerShell script/code/input fields; prune day-old markers; escape control chars in JSON; sync pr_status into the PreToolUse matcher Co-authored-by: BigSimmo --- .claude/hooks/pr-handoff-stop.sh | 74 ++++++++++++++++++++++++++------ .claude/settings.json | 2 +- 2 files changed, 61 insertions(+), 15 deletions(-) mode change 100644 => 100755 .claude/hooks/pr-handoff-stop.sh diff --git a/.claude/hooks/pr-handoff-stop.sh b/.claude/hooks/pr-handoff-stop.sh old mode 100644 new mode 100755 index e5ac378515..830846bd42 --- a/.claude/hooks/pr-handoff-stop.sh +++ b/.claude/hooks/pr-handoff-stop.sh @@ -36,35 +36,73 @@ mode="${1:-}" payload="$(cat 2>/dev/null || true)" [ -z "$payload" ] && exit 0 +# 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. +json_string_field() { + local key="$1" + printf '%s' "$payload" \ + | grep -o "\"$key\"[[:space:]]*:[[:space:]]*\"[^\"]*\"" \ + | head -n1 \ + | sed -E 's/.*"([^"]*)"$/\1/' +} + # --- payload fields ----------------------------------------------------------- -# jq when available (exact); otherwise fall back to scanning the raw payload, -# which is a superset of the command text and good enough for substring matching. +# jq when available (exact). Without jq: +# - tool_name / session_id from crude key extraction +# - command_text from tool_input command/script/code/input fields when present, +# else the whole payload (pre-mode shell deny still needs a searchable string) +# - tool_output ONLY from the substring after "tool_response" — never the whole +# payload. Falling back to the whole payload would let a failed `gh pr create` +# whose *input* mentions another PR URL write the handoff marker. 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 // empty' 2>/dev/null || true)" + # Bash uses .command; PowerShell payloads may use script/code/input instead. + command_text="$(printf '%s' "$payload" | jq -r ' + .tool_input.command // .tool_input.script // .tool_input.code // .tool_input.input // empty + ' 2>/dev/null || true)" tool_output="$(printf '%s' "$payload" | jq -r '[.tool_response] | tostring' 2>/dev/null || true)" session_id="$(printf '%s' "$payload" | jq -r '.session_id // empty' 2>/dev/null || true)" else - tool_name="$(printf '%s' "$payload" \ - | grep -o '"tool_name"[[:space:]]*:[[:space:]]*"[^"]*"' \ - | head -n1 | sed -E 's/.*"([^"]*)"$/\1/')" - command_text="$payload" - tool_output="$payload" - session_id="$(printf '%s' "$payload" \ - | grep -o '"session_id"[[:space:]]*:[[:space:]]*"[^"]*"' \ - | head -n1 | sed -E 's/.*"([^"]*)"$/\1/')" + tool_name="$(json_string_field tool_name)" + session_id="$(json_string_field session_id)" + command_text="$(json_string_field command)" + [ -z "$command_text" ] && command_text="$(json_string_field script)" + [ -z "$command_text" ] && command_text="$(json_string_field code)" + [ -z "$command_text" ] && command_text="$(json_string_field input)" + # pre-mode shell deny: if no command field was found, scan the raw payload. + # post-mode never uses command_text as the URL source (see tool_output below). + [ -z "$command_text" ] && command_text="$payload" + if printf '%s' "$payload" | grep -Fq '"tool_response"'; then + tool_output="${payload#*\"tool_response\"}" + else + tool_output="" + fi fi [ -z "$session_id" ] && session_id="unknown-session" # --- marker location ---------------------------------------------------------- -# Inside the git dir so it is never committed, and resolves per-worktree. -git_dir="$(git rev-parse --git-dir 2>/dev/null || true)" +# 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)" [ -z "$git_dir" ] && git_dir="${TMPDIR:-/tmp}" marker="$git_dir/claude-pr-handoff-$session_id" +# Drop handoff markers older than a day so the git dir does not accumulate them +# across sessions. Only touches our own claude-pr-handoff-* files. +prune_stale_markers() { + local dir="$1" + [ -d "$dir" ] || return 0 + find "$dir" -maxdepth 1 -type f -name 'claude-pr-handoff-*' -mtime +1 -delete 2>/dev/null || true +} + json_escape() { - printf '%s' "$1" | sed -e 's/\\/\\\\/g' -e 's/"/\\"/g' | tr '\n' ' ' + # Escape backslash/quote, fold newlines to spaces, and strip other C0 control + # characters so a future multi-line deny reason cannot emit unparseable JSON. + printf '%s' "$1" \ + | tr '\n\r\t' ' ' \ + | tr -d '\000-\010\013\014\016-\037' \ + | sed -e 's/\\/\\\\/g' -e 's/"/\\"/g' } is_shell_tool() { @@ -83,6 +121,8 @@ matches_pr_write_tool() { case "$mode" in post) + prune_stale_markers "$git_dir" + # A PR-creating call that actually returned a PR URL. Both halves matter: # without the URL check a failed create would end the session with no PR to # hand off. @@ -94,6 +134,9 @@ post) created=0 fi [ "$created" -eq 0 ] || exit 0 + # Require a non-empty tool_output that itself carries a PR URL. An empty + # tool_output (jq-less payload with no tool_response key) must not match. + [ -n "$tool_output" ] || exit 0 printf '%s' "$tool_output" | grep -Eq 'github\.com/[^ "]+/pull/[0-9]+' || exit 0 printf 'pr-opened %s\n' "$(date -u +%Y-%m-%dT%H:%M:%SZ 2>/dev/null || echo unknown)" >"$marker" 2>/dev/null || true @@ -118,6 +161,7 @@ pre) esac # 2. GitHub MCP tools that read or mutate PR / CI state. + # Keep this token list in sync with the PreToolUse matcher in .claude/settings.json. if [ -z "$reason" ] && printf '%s' "$tool_name" \ | grep -Eqi 'pull_?request|workflow_run|workflow_job|check_run|check_suite|job_log|pr_status|update_branch'; then reason='Blocked: this session already opened its pull request, so reading or nudging its CI and review state is the wasted-usage loop AGENTS.md "Stop when the pull request is open" rules out. Hand the PR URL to the user and stop.' @@ -125,6 +169,8 @@ pre) # 3. Shell polling. Narrow on purpose: committing, pushing, ledger appends, and # `gh pr merge` (already gated on explicit user confirmation) stay allowed. + # Matches are substring-based (cheap, no shell parse); false positives that + # only *mention* a blocked token can use CLAUDE_ALLOW_PR_FOLLOW=1. if [ -z "$reason" ] && is_shell_tool; then printf '%s' "$command_text" | grep -q 'CLAUDE_ALLOW_PR_FOLLOW=1' && exit 0 follow_re='gh[[:space:]]+pr[[:space:]]+(checks|status|view|diff|list)' diff --git a/.claude/settings.json b/.claude/settings.json index e04d24eb7c..8073de3b64 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -31,7 +31,7 @@ ], "PreToolUse": [ { - "matcher": "Bash|PowerShell|Monitor|ScheduleWakeup|CronCreate|.*[Pp]ull_?[Rr]equest.*|.*workflow_(run|job).*|.*check_(run|suite).*|.*job_log.*|.*update_branch.*", + "matcher": "Bash|PowerShell|Monitor|ScheduleWakeup|CronCreate|.*[Pp]ull_?[Rr]equest.*|.*workflow_(run|job).*|.*check_(run|suite).*|.*job_log.*|.*pr_status.*|.*update_branch.*", "hooks": [ { "type": "command", From 977262fa2670f604639ea20ae3f804121e02e29f Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 6 Aug 2026 16:15:17 +0000 Subject: [PATCH 3/9] fix(hooks): tighten pr-handoff-stop against review false-locks Anchor create_pull_request matching so review tools do not write the handoff marker, keep the active session marker across day-old prune, sanitize session ids used in marker paths, and cover the contract with focused tests. Co-authored-by: BigSimmo --- .claude/hooks/pr-handoff-stop.sh | 25 ++++++- tests/pr-handoff-stop.test.ts | 122 +++++++++++++++++++++++++++++++ 2 files changed, 143 insertions(+), 4 deletions(-) create mode 100644 tests/pr-handoff-stop.test.ts diff --git a/.claude/hooks/pr-handoff-stop.sh b/.claude/hooks/pr-handoff-stop.sh index 830846bd42..370fc5996b 100755 --- a/.claude/hooks/pr-handoff-stop.sh +++ b/.claude/hooks/pr-handoff-stop.sh @@ -80,6 +80,12 @@ else fi [ -z "$session_id" ] && session_id="unknown-session" +# Reject path-injection / odd session ids before they become a marker filename. +# Claude session ids are alphanumeric with dashes/underscores; anything else +# collapses to the safe unknown-session fallback. +if ! printf '%s' "$session_id" | grep -Eq '^[A-Za-z0-9_-]+$'; then + session_id="unknown-session" +fi # --- marker location ---------------------------------------------------------- # Absolute git dir so the marker path is valid from any cwd (and for linked @@ -89,11 +95,18 @@ git_dir="$(git rev-parse --absolute-git-dir 2>/dev/null || true)" marker="$git_dir/claude-pr-handoff-$session_id" # Drop handoff markers older than a day so the git dir does not accumulate them -# across sessions. Only touches our own claude-pr-handoff-* files. +# across sessions. Only touches our own claude-pr-handoff-* files. Never prune +# the current session's marker — retention is from last activity (refreshed in +# pre-mode), not from creation, so a long-lived cloud session keeps enforcement. prune_stale_markers() { local dir="$1" + local keep_name="$2" [ -d "$dir" ] || return 0 - find "$dir" -maxdepth 1 -type f -name 'claude-pr-handoff-*' -mtime +1 -delete 2>/dev/null || true + if [ -n "$keep_name" ]; then + find "$dir" -maxdepth 1 -type f -name 'claude-pr-handoff-*' ! -name "$keep_name" -mtime +1 -delete 2>/dev/null || true + else + find "$dir" -maxdepth 1 -type f -name 'claude-pr-handoff-*' -mtime +1 -delete 2>/dev/null || true + fi } json_escape() { @@ -121,7 +134,7 @@ matches_pr_write_tool() { case "$mode" in post) - prune_stale_markers "$git_dir" + prune_stale_markers "$git_dir" "claude-pr-handoff-$session_id" # A PR-creating call that actually returned a PR URL. Both halves matter: # without the URL check a failed create would end the session with no PR to @@ -130,7 +143,9 @@ post) if is_shell_tool && printf '%s' "$command_text" | grep -Eq 'gh[[:space:]]+pr[[:space:]]+create'; then created=0 fi - if printf '%s' "$tool_name" | grep -Eqi 'create_?pull_?request'; then + # End-anchor required: create_pull_request_review / _review_comment must NOT + # count as opening a PR (pre-mode already uses the same anchored shape). + if printf '%s' "$tool_name" | grep -Eqi 'create_?pull_?request$'; then created=0 fi [ "$created" -eq 0 ] || exit 0 @@ -148,6 +163,8 @@ post) pre) [ -f "$marker" ] || exit 0 + # Refresh mtime so prune retention tracks last activity, not creation time. + touch "$marker" 2>/dev/null || true matches_pr_write_tool && exit 0 reason="" diff --git a/tests/pr-handoff-stop.test.ts b/tests/pr-handoff-stop.test.ts new file mode 100644 index 0000000000..530bf6ca94 --- /dev/null +++ b/tests/pr-handoff-stop.test.ts @@ -0,0 +1,122 @@ +import { execFileSync, spawnSync } from "node:child_process"; +import { existsSync, mkdtempSync, rmSync, utimesSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterEach, describe, expect, it } from "vitest"; + +const hook = join(process.cwd(), ".claude/hooks/pr-handoff-stop.sh"); +const scratchRoots: string[] = []; + +afterEach(() => { + for (const root of scratchRoots.splice(0)) { + rmSync(root, { recursive: true, force: true }); + } +}); + +function freshRepo(): { root: string; gitDir: string } { + const root = mkdtempSync(join(tmpdir(), "pr-handoff-")); + scratchRoots.push(root); + execFileSync("git", ["init", "-q"], { cwd: root }); + execFileSync("git", ["commit", "--allow-empty", "-qm", "init"], { cwd: root }); + const gitDir = execFileSync("git", ["rev-parse", "--absolute-git-dir"], { + cwd: root, + encoding: "utf8", + }).trim(); + return { root, gitDir }; +} + +function runHook( + mode: "post" | "pre", + payload: Record, + cwd: string, +): { status: number | null; stdout: string; markerExists: (sessionId: string) => boolean; gitDir: string } { + const gitDir = execFileSync("git", ["rev-parse", "--absolute-git-dir"], { + cwd, + encoding: "utf8", + }).trim(); + const result = spawnSync("bash", [hook, mode], { + cwd, + input: JSON.stringify(payload), + encoding: "utf8", + }); + return { + status: result.status, + stdout: result.stdout ?? "", + gitDir, + markerExists: (sessionId: string) => existsSync(join(gitDir, `claude-pr-handoff-${sessionId}`)), + }; +} + +describe("pr-handoff-stop hook", () => { + it("does not treat create_pull_request_review as opening a PR", () => { + const { root } = freshRepo(); + const out = runHook( + "post", + { + tool_name: "create_pull_request_review", + session_id: "sess-review", + tool_response: "https://github.com/BigSimmo/Database/pull/1649#pullrequestreview-1", + }, + root, + ); + expect(out.status).toBe(0); + expect(out.markerExists("sess-review")).toBe(false); + expect(out.stdout).toBe(""); + }); + + it("writes a marker for create_pull_request when the response has a PR URL", () => { + const { root } = freshRepo(); + const out = runHook( + "post", + { + tool_name: "create_pull_request", + session_id: "sess-create", + tool_response: "Opened https://github.com/BigSimmo/Database/pull/1649", + }, + root, + ); + expect(out.status).toBe(0); + expect(out.markerExists("sess-create")).toBe(true); + expect(out.stdout).toContain("PostToolUse"); + }); + + it("sanitizes unsafe session ids before building the marker path", () => { + const { root, gitDir } = freshRepo(); + const out = runHook( + "post", + { + tool_name: "create_pull_request", + session_id: "../evil", + tool_response: "https://github.com/BigSimmo/Database/pull/1", + }, + root, + ); + expect(out.status).toBe(0); + expect(out.markerExists("unknown-session")).toBe(true); + expect(existsSync(join(gitDir, "..", "claude-pr-handoff-evil"))).toBe(false); + }); + + it("keeps the current session marker when pruning day-old siblings", () => { + const { root, gitDir } = freshRepo(); + const current = join(gitDir, "claude-pr-handoff-sess-keep"); + const other = join(gitDir, "claude-pr-handoff-other"); + execFileSync("bash", ["-lc", `printf 'old\\n' >"$1" && printf 'old\\n' >"$2"`, "_", current, other]); + const twoDaysAgo = Math.floor(Date.now() / 1000) - 2 * 24 * 60 * 60; + utimesSync(current, twoDaysAgo, twoDaysAgo); + utimesSync(other, twoDaysAgo, twoDaysAgo); + + const out = runHook( + "post", + { + tool_name: "Bash", + session_id: "sess-keep", + tool_input: { command: "true" }, + tool_response: "ok", + }, + root, + ); + expect(out.status).toBe(0); + expect(existsSync(current)).toBe(true); + expect(existsSync(other)).toBe(false); + }); +}); From 249cb253dac1069da972b35a721ccb9ecc5f8309 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 6 Aug 2026 16:42:06 +0000 Subject: [PATCH 4/9] fix(hooks): deny follow tokens when jq-less extraction truncates When jq is missing, quote-naive JSON extraction stopped at the first escaped quote inside tool_input.command, so a compound shell like `git commit -m "msg" && gh pr checks` never saw the follow token. Match shell create/follow regexes against the raw payload as well in that path, and cover it with regression tests. Co-authored-by: BigSimmo --- .claude/hooks/pr-handoff-stop.sh | 28 ++++++++++++++-- tests/pr-handoff-stop.test.ts | 57 ++++++++++++++++++++++++++++++++ 2 files changed, 83 insertions(+), 2 deletions(-) diff --git a/.claude/hooks/pr-handoff-stop.sh b/.claude/hooks/pr-handoff-stop.sh index 370fc5996b..e7ef375334 100755 --- a/.claude/hooks/pr-handoff-stop.sh +++ b/.claude/hooks/pr-handoff-stop.sh @@ -38,6 +38,9 @@ payload="$(cat 2>/dev/null || true)" # 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 +# jq_available=0 — this helper truncates at the first escaped quote inside +# the JSON string (e.g. `git commit -m \"msg\" && gh pr checks` → `git commit -m \`). json_string_field() { local key="$1" printf '%s' "$payload" \ @@ -54,7 +57,11 @@ json_string_field() { # - tool_output ONLY from the substring after "tool_response" — never the whole # payload. Falling back to the whole payload would let a failed `gh pr create` # whose *input* mentions another PR URL write the handoff marker. +# - shell token matching (pre follow-deny + post `gh pr create` detect) also +# scans the raw payload, because quote-naive extraction truncates mid-command +jq_available=0 if command -v jq >/dev/null 2>&1; then + jq_available=1 tool_name="$(printf '%s' "$payload" | jq -r '.tool_name // empty' 2>/dev/null || true)" # Bash uses .command; PowerShell payloads may use script/code/input instead. command_text="$(printf '%s' "$payload" | jq -r ' @@ -79,6 +86,20 @@ else fi fi +# Shell-token haystack: exact command when jq decoded it; when jq is missing, +# also include the raw payload so an escaped quote inside tool_input.command +# cannot hide a later follow/create token. URL matching for post-mode still +# uses tool_output only — never this haystack — so a create whose input merely +# mentions another PR URL cannot write the marker. +shell_command_matches() { + local re="$1" + printf '%s' "$command_text" | grep -Eq "$re" && return 0 + if [ "$jq_available" -eq 0 ]; then + printf '%s' "$payload" | grep -Eq "$re" && return 0 + fi + return 1 +} + [ -z "$session_id" ] && session_id="unknown-session" # Reject path-injection / odd session ids before they become a marker filename. # Claude session ids are alphanumeric with dashes/underscores; anything else @@ -140,7 +161,7 @@ post) # without the URL check a failed create would end the session with no PR to # hand off. created=1 - if is_shell_tool && printf '%s' "$command_text" | grep -Eq 'gh[[:space:]]+pr[[:space:]]+create'; then + if is_shell_tool && shell_command_matches 'gh[[:space:]]+pr[[:space:]]+create'; then created=0 fi # End-anchor required: create_pull_request_review / _review_comment must NOT @@ -189,12 +210,15 @@ pre) # Matches are substring-based (cheap, no shell parse); false positives that # only *mention* a blocked token can use CLAUDE_ALLOW_PR_FOLLOW=1. if [ -z "$reason" ] && is_shell_tool; then + # Escape hatch matches the extracted command only. A quote-truncated + # command_text still sees a leading CLAUDE_ALLOW_PR_FOLLOW=1 prefix; scanning + # the raw payload here would let an incidental mention unlock the deny. printf '%s' "$command_text" | grep -q 'CLAUDE_ALLOW_PR_FOLLOW=1' && exit 0 follow_re='gh[[:space:]]+pr[[:space:]]+(checks|status|view|diff|list)' follow_re="$follow_re"'|gh[[:space:]]+run[[:space:]]+(watch|view|list|rerun|download)' follow_re="$follow_re"'|gh[[:space:]]+api[^|;]*(actions/runs|check-runs|check-suites|/pulls/)' follow_re="$follow_re"'|sync:pr-branches' - if printf '%s' "$command_text" | grep -Eq "$follow_re"; then + if shell_command_matches "$follow_re"; then reason='Blocked: this session already opened its pull request, so following it (CI polling, run logs, branch sync) is the wasted-usage loop AGENTS.md "Stop when the pull request is open" rules out. Hand the PR URL to the user and stop. If the user has asked for this check, re-run it prefixed with CLAUDE_ALLOW_PR_FOLLOW=1.' fi fi diff --git a/tests/pr-handoff-stop.test.ts b/tests/pr-handoff-stop.test.ts index 530bf6ca94..8d32048f38 100644 --- a/tests/pr-handoff-stop.test.ts +++ b/tests/pr-handoff-stop.test.ts @@ -29,15 +29,31 @@ function runHook( mode: "post" | "pre", payload: Record, cwd: string, + options?: { pathWithoutJq?: boolean }, ): { status: number | null; stdout: string; markerExists: (sessionId: string) => boolean; gitDir: string } { const gitDir = execFileSync("git", ["rev-parse", "--absolute-git-dir"], { cwd, encoding: "utf8", }).trim(); + const env = { ...process.env }; + if (options?.pathWithoutJq) { + // Keep a minimal PATH that can run the hook's shell utilities but cannot + // resolve jq, so the quote-naive fallback path is exercised. + const bin = mkdtempSync(join(tmpdir(), "no-jq-path-")); + scratchRoots.push(bin); + for (const name of ["bash", "cat", "grep", "sed", "tr", "date", "find", "touch", "head", "git"]) { + const resolved = execFileSync("bash", ["-lc", `command -v ${name}`], { + encoding: "utf8", + }).trim(); + execFileSync("ln", ["-s", resolved, join(bin, name)]); + } + env.PATH = bin; + } const result = spawnSync("bash", [hook, mode], { cwd, input: JSON.stringify(payload), encoding: "utf8", + env, }); return { status: result.status, @@ -119,4 +135,45 @@ describe("pr-handoff-stop hook", () => { expect(existsSync(current)).toBe(true); expect(existsSync(other)).toBe(false); }); + + it("denies quoted compound follow commands when jq is unavailable", () => { + const { root, gitDir } = freshRepo(); + const marker = join(gitDir, "claude-pr-handoff-sess-quoted"); + execFileSync("bash", ["-lc", `printf 'pr-opened\\n' >"$1"`, "_", marker]); + + const out = runHook( + "pre", + { + tool_name: "Bash", + session_id: "sess-quoted", + // Escaped quotes inside the JSON command string truncate the jq-less + // extractor at `git commit -m \`; follow matching must still see the + // later `gh pr checks` token via the raw-payload fallback. + tool_input: { command: 'git commit -m "msg" && gh pr checks' }, + }, + root, + { pathWithoutJq: true }, + ); + expect(out.status).toBe(0); + expect(out.stdout).toContain('"permissionDecision":"deny"'); + expect(out.stdout).toContain("following it"); + }); + + it("still detects gh pr create after a quoted arg when jq is unavailable", () => { + const { root } = freshRepo(); + const out = runHook( + "post", + { + tool_name: "Bash", + session_id: "sess-create-quoted", + tool_input: { command: 'git commit -m "open pr" && gh pr create --fill' }, + tool_response: "https://github.com/BigSimmo/Database/pull/1649", + }, + root, + { pathWithoutJq: true }, + ); + expect(out.status).toBe(0); + expect(out.markerExists("sess-create-quoted")).toBe(true); + expect(out.stdout).toContain("PostToolUse"); + }); }); From 36c1bccd89f976bcae3dabf8f5787198db5df078 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 6 Aug 2026 16:56:56 +0000 Subject: [PATCH 5/9] fix(hooks): harden jq-less handoff matching against false locks - Restrict post-mode create-token fallback to the payload half before tool_response so printed docs cannot write a marker. - Isolate jq-less tool_output from a later tool_input URL suffix. - Fail open on missing/unsafe session ids instead of sharing unknown-session. - Require CLAUDE_ALLOW_PR_FOLLOW=1 as a command prefix. - Drop the identity-dependent empty commit from hook tests. - Record the Run PR sweep ledger row for PR #1649. Co-authored-by: BigSimmo --- .claude/hooks/pr-handoff-stop.sh | 75 ++++++++++++++++++++----------- docs/branch-review-ledger.md | 2 + tests/pr-handoff-stop.test.ts | 77 ++++++++++++++++++++++++++++---- 3 files changed, 121 insertions(+), 33 deletions(-) diff --git a/.claude/hooks/pr-handoff-stop.sh b/.claude/hooks/pr-handoff-stop.sh index e7ef375334..017028785b 100755 --- a/.claude/hooks/pr-handoff-stop.sh +++ b/.claude/hooks/pr-handoff-stop.sh @@ -54,11 +54,11 @@ json_string_field() { # - tool_name / session_id from crude key extraction # - command_text from tool_input command/script/code/input fields when present, # else the whole payload (pre-mode shell deny still needs a searchable string) -# - tool_output ONLY from the substring after "tool_response" — never the whole -# payload. Falling back to the whole payload would let a failed `gh pr create` -# whose *input* mentions another PR URL write the handoff marker. -# - shell token matching (pre follow-deny + post `gh pr create` detect) also -# scans the raw payload, because quote-naive extraction truncates mid-command +# - tool_output ONLY from the tool_response region — never the whole payload, +# and never a suffix that still contains a later tool_input (so an input URL +# cannot satisfy the post-mode URL gate when keys are ordered response-first) +# - shell token matching also scans a bounded raw-payload slice when jq is +# missing, because quote-naive extraction truncates mid-command jq_available=0 if command -v jq >/dev/null 2>&1; then jq_available=1 @@ -76,21 +76,34 @@ else [ -z "$command_text" ] && command_text="$(json_string_field script)" [ -z "$command_text" ] && command_text="$(json_string_field code)" [ -z "$command_text" ] && command_text="$(json_string_field input)" - # pre-mode shell deny: if no command field was found, scan the raw payload. - # post-mode never uses command_text as the URL source (see tool_output below). - [ -z "$command_text" ] && command_text="$payload" + # If no command-like field was found, scan the input half of the payload only — + # never tool_response — so a printed `gh pr create` cannot look like a create. + if [ -z "$command_text" ]; then + if printf '%s' "$payload" | grep -Fq '"tool_response"'; then + command_text="${payload%%\"tool_response\"*}" + else + command_text="$payload" + fi + fi if printf '%s' "$payload" | grep -Fq '"tool_response"'; then - tool_output="${payload#*\"tool_response\"}" + # Prefer a string-valued tool_response when the crude extractor can see it. + tool_output="$(json_string_field tool_response)" + if [ -z "$tool_output" ]; then + # Complex/non-string response: take the slice after tool_response, but + # stop before a later tool_input so an input URL cannot pass the gate. + tool_output="${payload#*\"tool_response\"}" + case "$tool_output" in + *\"tool_input\"*) tool_output="${tool_output%%\"tool_input\"*}" ;; + esac + fi else tool_output="" fi fi -# Shell-token haystack: exact command when jq decoded it; when jq is missing, -# also include the raw payload so an escaped quote inside tool_input.command -# cannot hide a later follow/create token. URL matching for post-mode still -# uses tool_output only — never this haystack — so a create whose input merely -# mentions another PR URL cannot write the marker. +# Pre-mode follow matching: decoded command, plus (jq-less) the full payload so +# a quote-truncated command_text cannot hide a later follow token. Over-block is +# the safe direction here. shell_command_matches() { local re="$1" printf '%s' "$command_text" | grep -Eq "$re" && return 0 @@ -100,12 +113,23 @@ shell_command_matches() { return 1 } -[ -z "$session_id" ] && session_id="unknown-session" -# Reject path-injection / odd session ids before they become a marker filename. -# Claude session ids are alphanumeric with dashes/underscores; anything else -# collapses to the safe unknown-session fallback. -if ! printf '%s' "$session_id" | grep -Eq '^[A-Za-z0-9_-]+$'; then - session_id="unknown-session" +# Post-mode create-token matching: decoded command, plus (jq-less) only the +# payload half *before* tool_response. Scanning tool_response would let a +# command that merely prints `gh pr create` + a PR URL lock the session. +shell_input_matches() { + local re="$1" + printf '%s' "$command_text" | grep -Eq "$re" && return 0 + if [ "$jq_available" -eq 0 ]; then + local input_half="${payload%%\"tool_response\"*}" + printf '%s' "$input_half" | grep -Eq "$re" && return 0 + fi + return 1 +} + +# Missing or unsafe session ids fail open: do not share an unknown-session +# marker across unrelated malformed payloads (path injection included). +if [ -z "$session_id" ] || ! printf '%s' "$session_id" | grep -Eq '^[A-Za-z0-9_-]+$'; then + exit 0 fi # --- marker location ---------------------------------------------------------- @@ -161,7 +185,7 @@ post) # without the URL check a failed create would end the session with no PR to # hand off. created=1 - if is_shell_tool && shell_command_matches 'gh[[:space:]]+pr[[:space:]]+create'; then + if is_shell_tool && shell_input_matches 'gh[[:space:]]+pr[[:space:]]+create'; then created=0 fi # End-anchor required: create_pull_request_review / _review_comment must NOT @@ -210,10 +234,11 @@ pre) # Matches are substring-based (cheap, no shell parse); false positives that # only *mention* a blocked token can use CLAUDE_ALLOW_PR_FOLLOW=1. if [ -z "$reason" ] && is_shell_tool; then - # Escape hatch matches the extracted command only. A quote-truncated - # command_text still sees a leading CLAUDE_ALLOW_PR_FOLLOW=1 prefix; scanning - # the raw payload here would let an incidental mention unlock the deny. - printf '%s' "$command_text" | grep -q 'CLAUDE_ALLOW_PR_FOLLOW=1' && exit 0 + # Escape hatch: documented prefix only (not an incidental echo/mention). + # A quote-truncated command_text still sees a leading CLAUDE_ALLOW_PR_FOLLOW=1. + printf '%s' "$command_text" \ + | grep -Eq '^[[:space:]]*CLAUDE_ALLOW_PR_FOLLOW=1([[:space:]]|$)' \ + && exit 0 follow_re='gh[[:space:]]+pr[[:space:]]+(checks|status|view|diff|list)' follow_re="$follow_re"'|gh[[:space:]]+run[[:space:]]+(watch|view|list|rerun|download)' follow_re="$follow_re"'|gh[[:space:]]+api[^|;]*(actions/runs|check-runs|check-suites|/pulls/)' diff --git a/docs/branch-review-ledger.md b/docs/branch-review-ledger.md index 03497796eb..eadc8cca1e 100644 --- a/docs/branch-review-ledger.md +++ b/docs/branch-review-ledger.md @@ -665,3 +665,5 @@ Records before 2026-07-28 were written by hand and had drifted: 146 lines carrie | 2026-08-06 | a24f74fdf0134487a03dce37dd9f1e9bd18502f5 | a24f74fdf0134487a03dce37dd9f1e9bd18502f5 | PR #1614 post-merge RAG index restoration audit | Pass - guard-only migration, no DDL, no ranking/RPC change; pr-policy ragRanking=false so no eval-canary required; 1 P3 doc nit (#248 renumber note says 237->246, row is #248) | check:migration-role; npx vitest run tests/supabase-schema.test.ts (74 passed); check:outstanding-issues | | 2026-08-06 | PR #1614 / codex/restore-rag-indexes-20260804 | a24f74fdf0134487a03dce37dd9f1e9bd18502f5 | PR #1614 post-merge RAG index restoration audit | Pass - guard-only migration, no DDL, no ranking/RPC change; pr-policy ragRanking=false so no eval-canary required; 1 P3 doc nit (#248 renumber note says 237->246, row is #248); supersedes 2026-08-06 row (ref column mistakenly held commit SHA instead of PR ref, breaking ledger:lookup per Devin/Sentry review on PR #1636) | check:migration-role; npx vitest run tests/supabase-schema.test.ts (74 passed); check:outstanding-issues | | 2026-08-06 | claude/implement-97vpz7 | 00ab7bfd34684bc854d15a3f28987674098a7130 | PR #1646 soft-tail answer-cache skip + soft-tail test hardening | fixed — answer-path soft-tail skip via rag-query-guard helpers; soft-tail fixture pins; duplicate memo test removed; in-corpus assert narrowed; budget 4362 | test:rag-query-guard+unsupported-cache+classifier-memo 22/22,check:maintainability-budgets 4362/4362 | +| 2026-08-06 | claude/pr-handoff-stop-hook (PR #1649) | fff524d73ebfafeef7bdc50752a6d14f253ebc2e | Run PR sweep: CI fix + threads + drift | before: 5 unresolved threads (Devin create-from-output + 4 CodeRabbit), mergeable/BLOCKED, Actions major outage leaving CI pending; after: hardened jq-less input/output separation + session fail-open + prefix unlock + tests (9 passed) in fff524d73ebfafeef7bdc50752a6d14f253ebc2e, threads replied+resolved, branch current with main, CI re-triggered (Actions outage — not babysat) | npx vitest run tests/pr-handoff-stop.test.ts (9 passed); bash -n hook OK; no provider-backed checks run | +| 2026-08-06 | claude/pr-handoff-stop-hook (PR #1649) | f67e5c9103c3e1483ad8e034fe7a6757c2362d74 | Run PR sweep: CI fix + threads + drift | before: 5 unresolved threads (Devin create-from-output + 4 CodeRabbit), mergeable/BLOCKED, Actions major outage leaving CI pending; after: hardened jq-less input/output separation + session fail-open + prefix unlock + tests (9 passed) in f67e5c9103c3e1483ad8e034fe7a6757c2362d74, threads replied+resolved, branch current with main, CI re-triggered (Actions outage — not babysat) | npx vitest run tests/pr-handoff-stop.test.ts (9 passed); bash -n hook OK; no provider-backed checks run | diff --git a/tests/pr-handoff-stop.test.ts b/tests/pr-handoff-stop.test.ts index 8d32048f38..78a80178b7 100644 --- a/tests/pr-handoff-stop.test.ts +++ b/tests/pr-handoff-stop.test.ts @@ -1,5 +1,5 @@ import { execFileSync, spawnSync } from "node:child_process"; -import { existsSync, mkdtempSync, rmSync, utimesSync } from "node:fs"; +import { existsSync, mkdtempSync, rmSync, utimesSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { afterEach, describe, expect, it } from "vitest"; @@ -17,7 +17,7 @@ function freshRepo(): { root: string; gitDir: string } { const root = mkdtempSync(join(tmpdir(), "pr-handoff-")); scratchRoots.push(root); execFileSync("git", ["init", "-q"], { cwd: root }); - execFileSync("git", ["commit", "--allow-empty", "-qm", "init"], { cwd: root }); + // No empty commit: git rev-parse --absolute-git-dir works without user.identity. const gitDir = execFileSync("git", ["rev-parse", "--absolute-git-dir"], { cwd: root, encoding: "utf8", @@ -27,7 +27,7 @@ function freshRepo(): { root: string; gitDir: string } { function runHook( mode: "post" | "pre", - payload: Record, + payload: Record | string, cwd: string, options?: { pathWithoutJq?: boolean }, ): { status: number | null; stdout: string; markerExists: (sessionId: string) => boolean; gitDir: string } { @@ -49,9 +49,10 @@ function runHook( } env.PATH = bin; } + const input = typeof payload === "string" ? payload : JSON.stringify(payload); const result = spawnSync("bash", [hook, mode], { cwd, - input: JSON.stringify(payload), + input, encoding: "utf8", env, }); @@ -96,7 +97,7 @@ describe("pr-handoff-stop hook", () => { expect(out.stdout).toContain("PostToolUse"); }); - it("sanitizes unsafe session ids before building the marker path", () => { + it("fails open for unsafe session ids instead of sharing unknown-session", () => { const { root, gitDir } = freshRepo(); const out = runHook( "post", @@ -108,15 +109,17 @@ describe("pr-handoff-stop hook", () => { root, ); expect(out.status).toBe(0); - expect(out.markerExists("unknown-session")).toBe(true); + expect(out.markerExists("unknown-session")).toBe(false); expect(existsSync(join(gitDir, "..", "claude-pr-handoff-evil"))).toBe(false); + expect(out.stdout).toBe(""); }); it("keeps the current session marker when pruning day-old siblings", () => { const { root, gitDir } = freshRepo(); const current = join(gitDir, "claude-pr-handoff-sess-keep"); const other = join(gitDir, "claude-pr-handoff-other"); - execFileSync("bash", ["-lc", `printf 'old\\n' >"$1" && printf 'old\\n' >"$2"`, "_", current, other]); + writeFileSync(current, "old\n"); + writeFileSync(other, "old\n"); const twoDaysAgo = Math.floor(Date.now() / 1000) - 2 * 24 * 60 * 60; utimesSync(current, twoDaysAgo, twoDaysAgo); utimesSync(other, twoDaysAgo, twoDaysAgo); @@ -139,7 +142,7 @@ describe("pr-handoff-stop hook", () => { it("denies quoted compound follow commands when jq is unavailable", () => { const { root, gitDir } = freshRepo(); const marker = join(gitDir, "claude-pr-handoff-sess-quoted"); - execFileSync("bash", ["-lc", `printf 'pr-opened\\n' >"$1"`, "_", marker]); + writeFileSync(marker, "pr-opened\n"); const out = runHook( "pre", @@ -176,4 +179,62 @@ describe("pr-handoff-stop hook", () => { expect(out.markerExists("sess-create-quoted")).toBe(true); expect(out.stdout).toContain("PostToolUse"); }); + + it("does not lock from a printed gh pr create token in tool_response when jq is unavailable", () => { + const { root } = freshRepo(); + const out = runHook( + "post", + { + tool_name: "Bash", + session_id: "sess-print-docs", + tool_input: { command: "cat AGENTS.md" }, + tool_response: "Documented handoff: run gh pr create then open https://github.com/BigSimmo/Database/pull/1649", + }, + root, + { pathWithoutJq: true }, + ); + expect(out.status).toBe(0); + expect(out.markerExists("sess-print-docs")).toBe(false); + expect(out.stdout).toBe(""); + }); + + it("ignores a tool_input URL after tool_response when jq is unavailable", () => { + const { root } = freshRepo(); + // Key order matters for the jq-less suffix extractor: response first, then + // an input field that happens to mention a PR URL must not write a marker. + const payload = + '{"tool_name":"Bash","session_id":"sess-order","tool_response":"create failed","tool_input":{"command":"gh pr create --fill","url":"https://github.com/BigSimmo/Database/pull/1649"}}'; + const out = runHook("post", payload, root, { pathWithoutJq: true }); + expect(out.status).toBe(0); + expect(out.markerExists("sess-order")).toBe(false); + expect(out.stdout).toBe(""); + }); + + it("requires CLAUDE_ALLOW_PR_FOLLOW=1 as a command prefix", () => { + const { root, gitDir } = freshRepo(); + writeFileSync(join(gitDir, "claude-pr-handoff-sess-unlock"), "pr-opened\n"); + + const denied = runHook( + "pre", + { + tool_name: "Bash", + session_id: "sess-unlock", + tool_input: { command: "echo CLAUDE_ALLOW_PR_FOLLOW=1 && gh pr checks" }, + }, + root, + ); + expect(denied.stdout).toContain('"permissionDecision":"deny"'); + + const allowed = runHook( + "pre", + { + tool_name: "Bash", + session_id: "sess-unlock", + tool_input: { command: "CLAUDE_ALLOW_PR_FOLLOW=1 gh pr checks" }, + }, + root, + ); + expect(allowed.status).toBe(0); + expect(allowed.stdout).toBe(""); + }); }); From 42ae4f37d79d6d5baf060b583a81df60c8a862b3 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 6 Aug 2026 16:57:32 +0000 Subject: [PATCH 6/9] chore: correct Run PR ledger HEAD for #1649 tip Co-authored-by: BigSimmo --- docs/branch-review-ledger.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/branch-review-ledger.md b/docs/branch-review-ledger.md index eadc8cca1e..b19c18fcec 100644 --- a/docs/branch-review-ledger.md +++ b/docs/branch-review-ledger.md @@ -667,3 +667,5 @@ Records before 2026-07-28 were written by hand and had drifted: 146 lines carrie | 2026-08-06 | claude/implement-97vpz7 | 00ab7bfd34684bc854d15a3f28987674098a7130 | PR #1646 soft-tail answer-cache skip + soft-tail test hardening | fixed — answer-path soft-tail skip via rag-query-guard helpers; soft-tail fixture pins; duplicate memo test removed; in-corpus assert narrowed; budget 4362 | test:rag-query-guard+unsupported-cache+classifier-memo 22/22,check:maintainability-budgets 4362/4362 | | 2026-08-06 | claude/pr-handoff-stop-hook (PR #1649) | fff524d73ebfafeef7bdc50752a6d14f253ebc2e | Run PR sweep: CI fix + threads + drift | before: 5 unresolved threads (Devin create-from-output + 4 CodeRabbit), mergeable/BLOCKED, Actions major outage leaving CI pending; after: hardened jq-less input/output separation + session fail-open + prefix unlock + tests (9 passed) in fff524d73ebfafeef7bdc50752a6d14f253ebc2e, threads replied+resolved, branch current with main, CI re-triggered (Actions outage — not babysat) | npx vitest run tests/pr-handoff-stop.test.ts (9 passed); bash -n hook OK; no provider-backed checks run | | 2026-08-06 | claude/pr-handoff-stop-hook (PR #1649) | f67e5c9103c3e1483ad8e034fe7a6757c2362d74 | Run PR sweep: CI fix + threads + drift | before: 5 unresolved threads (Devin create-from-output + 4 CodeRabbit), mergeable/BLOCKED, Actions major outage leaving CI pending; after: hardened jq-less input/output separation + session fail-open + prefix unlock + tests (9 passed) in f67e5c9103c3e1483ad8e034fe7a6757c2362d74, threads replied+resolved, branch current with main, CI re-triggered (Actions outage — not babysat) | npx vitest run tests/pr-handoff-stop.test.ts (9 passed); bash -n hook OK; no provider-backed checks run | +| 2026-08-06 | claude/pr-handoff-stop-hook (PR #1649) | 36c1bccd89f976bcae3dabf8f5787198db5df078 | Run PR sweep: CI fix + threads + drift | before: 5 unresolved threads (Devin create-from-output + 4 CodeRabbit), mergeable/BLOCKED, Actions major outage leaving CI pending; after: hardened jq-less input/output separation + session fail-open + prefix unlock + tests (9 passed) in 36c1bccd89f976bcae3dabf8f5787198db5df078, threads replied+resolved, branch current with main, CI re-triggered (Actions outage — not babysat) | npx vitest run tests/pr-handoff-stop.test.ts (9 passed); bash -n hook OK; no provider-backed checks run | +| 2026-08-06 | claude/pr-handoff-stop-hook (PR #1649) | 3403126bc6cd86145d6921a3fe4d081181e8a113 | Run PR sweep: CI fix + threads + drift | before: 5 unresolved threads (Devin create-from-output + 4 CodeRabbit), mergeable/BLOCKED, Actions major outage leaving CI pending; after: hardened jq-less input/output separation + session fail-open + prefix unlock + tests (9 passed) in 3403126bc6cd86145d6921a3fe4d081181e8a113, threads replied+resolved, branch current with main, CI re-triggered (Actions outage — not babysat) | npx vitest run tests/pr-handoff-stop.test.ts (9 passed); bash -n hook OK; no provider-backed checks run | From 057a5579e538fcacc15feb8a0ac7fa580b3decb4 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 6 Aug 2026 17:10:21 +0000 Subject: [PATCH 7/9] fix(hooks): stop cross-session prune and tighten handoff deny Bugbot found age-based sibling marker prune on every Bash PostToolUse could disarm a long-lived handoff session that only uses Read/Edit. Drop that prune, emit handoff context only after a successful marker write, and deny gh pr comment/review to match the AGENTS review-bot loop. Co-authored-by: BigSimmo --- .claude/hooks/pr-handoff-stop.sh | 34 ++++++++++----------- AGENTS.md | 2 +- tests/pr-handoff-stop.test.ts | 51 ++++++++++++++++++++++++++++++-- 3 files changed, 65 insertions(+), 22 deletions(-) diff --git a/.claude/hooks/pr-handoff-stop.sh b/.claude/hooks/pr-handoff-stop.sh index 017028785b..6fbc051daf 100755 --- a/.claude/hooks/pr-handoff-stop.sh +++ b/.claude/hooks/pr-handoff-stop.sh @@ -139,20 +139,12 @@ 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" -# Drop handoff markers older than a day so the git dir does not accumulate them -# across sessions. Only touches our own claude-pr-handoff-* files. Never prune -# the current session's marker — retention is from last activity (refreshed in -# pre-mode), not from creation, so a long-lived cloud session keeps enforcement. -prune_stale_markers() { - local dir="$1" - local keep_name="$2" - [ -d "$dir" ] || return 0 - if [ -n "$keep_name" ]; then - find "$dir" -maxdepth 1 -type f -name 'claude-pr-handoff-*' ! -name "$keep_name" -mtime +1 -delete 2>/dev/null || true - else - find "$dir" -maxdepth 1 -type f -name 'claude-pr-handoff-*' -mtime +1 -delete 2>/dev/null || true - fi -} +# Intentionally do not prune sibling sessions' markers. Post-mode runs on every +# Bash/PowerShell call (settings matcher), so age-based deletion of *other* +# sessions' files would disarm a long-lived handoff session that only uses +# Read/Edit after opening its PR (those tools never refresh mtime via pre-mode +# touch). Markers are one-line files under the git dir and disappear with the +# worktree; leftover orphans are cheap hygiene, not worth silent enforcement loss. json_escape() { # Escape backslash/quote, fold newlines to spaces, and strip other C0 control @@ -179,8 +171,6 @@ matches_pr_write_tool() { case "$mode" in post) - prune_stale_markers "$git_dir" "claude-pr-handoff-$session_id" - # A PR-creating call that actually returned a PR URL. Both halves matter: # without the URL check a failed create would end the session with no PR to # hand off. @@ -199,7 +189,13 @@ post) [ -n "$tool_output" ] || exit 0 printf '%s' "$tool_output" | grep -Eq 'github\.com/[^ "]+/pull/[0-9]+' || exit 0 - printf 'pr-opened %s\n' "$(date -u +%Y-%m-%dT%H:%M:%SZ 2>/dev/null || echo unknown)" >"$marker" 2>/dev/null || true + # Only tell the model tools are denied when the marker actually landed. A + # failed write (permissions / full disk) must fail open with no context — + # otherwise the model stops while pre-mode never enforces. + if ! printf 'pr-opened %s\n' "$(date -u +%Y-%m-%dT%H:%M:%SZ 2>/dev/null || echo unknown)" >"$marker" 2>/dev/null; then + exit 0 + fi + [ -f "$marker" ] || exit 0 context='The pull request is open. That is the end of this session'\''s handoff. Record the ledger row if it is still owed, give the user the PR URL and a short summary, then stop. Do NOT watch CI, poll checks, read workflow runs or job logs, re-run workflows, sync the branch from main, answer review bots, or park a Monitor / ScheduleWakeup / cron loop on this PR — following the PR from here is the wasted-usage loop this repo has explicitly ruled out (AGENTS.md "Stop when the pull request is open"). Those tools are now denied for the rest of this session. If the user asks for CI babysitting afterwards, that is their call and it is allowed then.' printf '{"hookSpecificOutput":{"hookEventName":"PostToolUse","additionalContext":"%s"}}\n' "$(json_escape "$context")" @@ -239,7 +235,9 @@ pre) printf '%s' "$command_text" \ | grep -Eq '^[[:space:]]*CLAUDE_ALLOW_PR_FOLLOW=1([[:space:]]|$)' \ && exit 0 - follow_re='gh[[:space:]]+pr[[:space:]]+(checks|status|view|diff|list)' + # comment/review cover the "answer review bots" loop named in AGENTS.md; + # merge stays allowed (explicit-confirmation rule elsewhere). + follow_re='gh[[:space:]]+pr[[:space:]]+(checks|status|view|diff|list|comment|review)' follow_re="$follow_re"'|gh[[:space:]]+run[[:space:]]+(watch|view|list|rerun|download)' follow_re="$follow_re"'|gh[[:space:]]+api[^|;]*(actions/runs|check-runs|check-suites|/pulls/)' follow_re="$follow_re"'|sync:pr-branches' diff --git a/AGENTS.md b/AGENTS.md index e3b8db647c..0f9491fde3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -586,7 +586,7 @@ drops a session-scoped marker when a PR-creating call — `gh pr create` or any `create_pull_request` MCP tool — returns a real PR URL. For the rest of that session it then denies three things: -- **Shell polling** — `gh pr checks|status|view|diff|list`, `gh run watch|view|list|rerun|download`, +- **Shell polling** — `gh pr checks|status|view|diff|list|comment|review`, `gh run watch|view|list|rerun|download`, `gh api …actions/runs|check-runs|check-suites|/pulls/`, and `sync:pr-branches`. - **GitHub MCP PR/CI tools** — anything whose tool name carries `pull_request`, `workflow_run`, `workflow_job`, `check_run`, `check_suite`, `job_log`, or diff --git a/tests/pr-handoff-stop.test.ts b/tests/pr-handoff-stop.test.ts index 78a80178b7..44a26aaf69 100644 --- a/tests/pr-handoff-stop.test.ts +++ b/tests/pr-handoff-stop.test.ts @@ -1,5 +1,5 @@ import { execFileSync, spawnSync } from "node:child_process"; -import { existsSync, mkdtempSync, rmSync, utimesSync, writeFileSync } from "node:fs"; +import { chmodSync, existsSync, mkdtempSync, rmSync, utimesSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { afterEach, describe, expect, it } from "vitest"; @@ -114,7 +114,9 @@ describe("pr-handoff-stop hook", () => { expect(out.stdout).toBe(""); }); - it("keeps the current session marker when pruning day-old siblings", () => { + it("does not prune sibling session markers from post-mode Bash", () => { + // Cross-session age-based prune would disarm a long-lived handoff session + // that only uses Read/Edit after opening its PR (no pre-mode touch). const { root, gitDir } = freshRepo(); const current = join(gitDir, "claude-pr-handoff-sess-keep"); const other = join(gitDir, "claude-pr-handoff-other"); @@ -136,7 +138,50 @@ describe("pr-handoff-stop hook", () => { ); expect(out.status).toBe(0); expect(existsSync(current)).toBe(true); - expect(existsSync(other)).toBe(false); + expect(existsSync(other)).toBe(true); + }); + + it("denies gh pr comment and gh pr review after handoff", () => { + const { root, gitDir } = freshRepo(); + writeFileSync(join(gitDir, "claude-pr-handoff-sess-bots"), "pr-opened\n"); + + for (const command of ["gh pr comment 1 --body ok", "gh pr review 1 --approve"]) { + const out = runHook( + "pre", + { + tool_name: "Bash", + session_id: "sess-bots", + tool_input: { command }, + }, + root, + ); + expect(out.status).toBe(0); + expect(out.stdout).toContain('"permissionDecision":"deny"'); + } + }); + + it("emits handoff context only when the marker file exists", () => { + const { root, gitDir } = freshRepo(); + // Make the git dir unwritable so the marker write fails; post must fail + // open with no additionalContext (model must not be told tools are denied). + chmodSync(gitDir, 0o555); + + try { + const out = runHook( + "post", + { + tool_name: "create_pull_request", + session_id: "sess-readonly", + tool_response: "Opened https://github.com/BigSimmo/Database/pull/1649", + }, + root, + ); + expect(out.status).toBe(0); + expect(out.markerExists("sess-readonly")).toBe(false); + expect(out.stdout).toBe(""); + } finally { + chmodSync(gitDir, 0o755); + } }); it("denies quoted compound follow commands when jq is unavailable", () => { From 935671ca956755ce208726144bb389e01a3b94d2 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 6 Aug 2026 17:17:21 +0000 Subject: [PATCH 8/9] chore: ledger PR #1649 review-and-fix at product tip Supersede stacked Run PR rows that pointed at unresolvable HEADs and record the heavy review-and-fix pass for the product fix commit. Co-authored-by: BigSimmo --- docs/branch-review-ledger.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/branch-review-ledger.md b/docs/branch-review-ledger.md index b19c18fcec..081eec801f 100644 --- a/docs/branch-review-ledger.md +++ b/docs/branch-review-ledger.md @@ -669,3 +669,5 @@ Records before 2026-07-28 were written by hand and had drifted: 146 lines carrie | 2026-08-06 | claude/pr-handoff-stop-hook (PR #1649) | f67e5c9103c3e1483ad8e034fe7a6757c2362d74 | Run PR sweep: CI fix + threads + drift | before: 5 unresolved threads (Devin create-from-output + 4 CodeRabbit), mergeable/BLOCKED, Actions major outage leaving CI pending; after: hardened jq-less input/output separation + session fail-open + prefix unlock + tests (9 passed) in f67e5c9103c3e1483ad8e034fe7a6757c2362d74, threads replied+resolved, branch current with main, CI re-triggered (Actions outage — not babysat) | npx vitest run tests/pr-handoff-stop.test.ts (9 passed); bash -n hook OK; no provider-backed checks run | | 2026-08-06 | claude/pr-handoff-stop-hook (PR #1649) | 36c1bccd89f976bcae3dabf8f5787198db5df078 | Run PR sweep: CI fix + threads + drift | before: 5 unresolved threads (Devin create-from-output + 4 CodeRabbit), mergeable/BLOCKED, Actions major outage leaving CI pending; after: hardened jq-less input/output separation + session fail-open + prefix unlock + tests (9 passed) in 36c1bccd89f976bcae3dabf8f5787198db5df078, threads replied+resolved, branch current with main, CI re-triggered (Actions outage — not babysat) | npx vitest run tests/pr-handoff-stop.test.ts (9 passed); bash -n hook OK; no provider-backed checks run | | 2026-08-06 | claude/pr-handoff-stop-hook (PR #1649) | 3403126bc6cd86145d6921a3fe4d081181e8a113 | Run PR sweep: CI fix + threads + drift | before: 5 unresolved threads (Devin create-from-output + 4 CodeRabbit), mergeable/BLOCKED, Actions major outage leaving CI pending; after: hardened jq-less input/output separation + session fail-open + prefix unlock + tests (9 passed) in 3403126bc6cd86145d6921a3fe4d081181e8a113, threads replied+resolved, branch current with main, CI re-triggered (Actions outage — not babysat) | npx vitest run tests/pr-handoff-stop.test.ts (9 passed); bash -n hook OK; no provider-backed checks run | +| 2026-08-06 | claude/pr-handoff-stop-hook (PR #1649) | 057a5579e538fcacc15feb8a0ac7fa580b3decb4 | PR #1649 review-and-fix | fixed: Bugbot cross-session prune disarm + marker-write fail-open context + deny gh pr comment/review; superseded dead Run PR ledger HEADs; merge-tree clean vs main; threads cleared; CI re-trigger on push | vitest:pr-handoff-stop 11/11; verify:cheap 516 files/5457 tests; verify:pr-local pass (format+lint+typecheck+test+rag-fixtures); bash -n hook OK; security-review: no P0/P1; no provider gates | +| 2026-08-06 | claude/pr-handoff-stop-hook (PR #1649) | 057a5579e538fcacc15feb8a0ac7fa580b3decb4 | Run PR sweep: CI fix + threads + drift | supersede: prior stacked Run PR rows used unresolvable HEADs (fff524d7/f67e5c91/3403126b); reviewed product tip is this SHA (ledger bookkeeping may sit one commit above); Bugbot prune+context+comment/review fixes landed | vitest:pr-handoff-stop 11/11; verify:cheap pass; verify:pr-local pass | From ed21f6fe4a5fe690bae8e2d39ad569169f3d8430 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 6 Aug 2026 20:18:33 +0000 Subject: [PATCH 9/9] ci: retrigger after Actions queue timeout Prior CI run cancelled/timed out while queued during the GitHub Actions major outage. Empty commit to re-fire checks on current tip. Co-authored-by: BigSimmo