diff --git a/.claude/hooks/pr-handoff-stop.sh b/.claude/hooks/pr-handoff-stop.sh new file mode 100755 index 0000000000..6fbc051daf --- /dev/null +++ b/.claude/hooks/pr-handoff-stop.sh @@ -0,0 +1,259 @@ +#!/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 + +# 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" \ + | grep -o "\"$key\"[[:space:]]*:[[:space:]]*\"[^\"]*\"" \ + | head -n1 \ + | sed -E 's/.*"([^"]*)"$/\1/' +} + +# --- payload fields ----------------------------------------------------------- +# 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 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 + 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 ' + .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="$(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)" + # 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 + # 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 + +# 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 + if [ "$jq_available" -eq 0 ]; then + printf '%s' "$payload" | grep -Eq "$re" && return 0 + fi + return 1 +} + +# 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 ---------------------------------------------------------- +# 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" + +# 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 + # 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() { + 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 && shell_input_matches 'gh[[:space:]]+pr[[:space:]]+create'; then + created=0 + fi + # 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 + # 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 + + # 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")" + exit 0 + ;; + +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="" + + # 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. + # 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.' + fi + + # 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 + # 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 + # 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' + 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 + + [ -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..8073de3b64 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.*|.*pr_status.*|.*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..0f9491fde3 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|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 + `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 diff --git a/docs/branch-review-ledger.md b/docs/branch-review-ledger.md index daeac65955..19028ce378 100644 --- a/docs/branch-review-ledger.md +++ b/docs/branch-review-ledger.md @@ -665,4 +665,10 @@ 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 | +| 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 | | 2026-08-06 | temp-rebase | 868a8a2800351ce85a2ad13e14d80550cbc3e668 | Merge conflict resolution and CI fixes | Verified and ready for PR | verify:pr-local | diff --git a/tests/pr-handoff-stop.test.ts b/tests/pr-handoff-stop.test.ts new file mode 100644 index 0000000000..44a26aaf69 --- /dev/null +++ b/tests/pr-handoff-stop.test.ts @@ -0,0 +1,285 @@ +import { execFileSync, spawnSync } from "node:child_process"; +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"; + +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 }); + // 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", + }).trim(); + return { root, gitDir }; +} + +function runHook( + mode: "post" | "pre", + payload: Record | string, + 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 input = typeof payload === "string" ? payload : JSON.stringify(payload); + const result = spawnSync("bash", [hook, mode], { + cwd, + input, + encoding: "utf8", + env, + }); + 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("fails open for unsafe session ids instead of sharing unknown-session", () => { + 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(false); + expect(existsSync(join(gitDir, "..", "claude-pr-handoff-evil"))).toBe(false); + expect(out.stdout).toBe(""); + }); + + 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"); + 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); + + 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(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", () => { + const { root, gitDir } = freshRepo(); + const marker = join(gitDir, "claude-pr-handoff-sess-quoted"); + writeFileSync(marker, "pr-opened\n"); + + 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"); + }); + + 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(""); + }); +});