hooks: refuse a write whose effect you cannot see

[EdbertChan]

Summary

New Claude hook, gh-write-verification. Four guards, one principle: a command's report is not the state it claims.

gh pr edit is broken in this repo on every flag. It eagerly queries a sunset Projects-classic GraphQL field and exits 1 before writing anything.

Three separate sessions hit that in one day. One of them ran it with >/dev/null 2>&1 attached.

So the error went to /dev/null, and a stale read-back reported success.

Four PRs then merged. GitHub reported MERGED for all four, because each merged into its own un-retargeted stack branch. Recovering them cost a cherry-pick PR.

So the hook refuses gh pr edit and names the gh api call that works. It refuses a mutation with both streams discarded and no exit-code check.

And it stops a turn that merged a PR without checking where the commit landed.

The fourth guard points that at a read. pgrep -f compares full command lines, so it matches the shell asking the question.

A wait negated on it never exits. One agent armed three, yielded, and was never resumed; two were still spinning 45 minutes later.

Review Claim

One hook with four detectors — PreToolUse blocks on gh pr edit, on an unchecked both-streams-discarded mutation, and on a pgrep -f/pkill -f that matches its own command line, plus a Stop block on a gh pr merge with no landing check — each proven to fire on the real incident payload and stay silent on the legitimate neighbours it is most likely to be confused with.

Review Lane

behavior

Review Unit

engine-runtime

Safety Invariant

Both entrypoints are advisory-to-the-model only: they write to stderr and exit 2, and have no other write path — no file mutation, no network, no state file. The PreToolUse half positive-lists shell-like tool names, so a Write/Edit whose content merely mentions these shapes is never blocked. The Stop half returns early on stop_hook_active and on a missing, unreadable, or malformed transcript, and every uncaught exception in either entrypoint is trapped and reported as "allowing", so a detector bug can only under-block.

Detector 2 is an explicit allowlist of state-changing commands rather than a blocklist of safe ones, so every read-only command with discarded output — the common, legitimate, expensive-to-break case — is silent by construction, not by exclusion rule. Detector 3 decides statically from the command text alone, runs no process lookup of its own, and stays silent whenever the pattern is a variable or command substitution it cannot resolve. GH_WRITE_VERIFICATION_TRUST_PR_EDIT=1 lifts detector 1 once the CLI stops erroring.

Assumptions: this Safety Invariant is unconfirmed — drafted under a non-interactive delegation with no user available to confirm it, per draft-pr's headless mode.

Slice Rationale

The first, second and fourth detectors are one causal chain from a single incident, not independent ideas: the broken command produced the failure, the discarded output hid it, and the trusted MERGED report let it through. The third is the same class caught in the recovery session itself. Splitting them would ship a guard whose motivating evidence lives in another PR.

install.sh and tests/test_install.py cannot be split off — a hook whose installer is never called and never asserted is dead code, which is the failure mode this repo has already hit.

lint-diff-atomicity.mjs raises one advisory unrelated-areas warning for that spread (engine, install.sh, tests). Confirmed intentional: it is the standard shape of every hook under engine/hooks/.

Non-goals

• Does not extend pr-schema-gate, which was the first candidate. That hook returns early unless repo_root_with_create_pr_tool() finds scripts/create-pr.mjs; catstack has none, so it fails open in the very repo where all three failures happened. Its gh pr edit --body block stays as-is — it survives a gh fix, this one does not.
• Does not fix gh pr edit. It refuses it and names the working REST call.
• Does not undo a bad merge, and cannot: Stop fires after the fact. It blocks the turn from ending until the landing is checked.
• Does not assert the landing check passed, only that it ran. verify_pr_landed_on_trunk.sh exits non-zero on a bad landing, and detector 2 stops that exit code being discarded.
• No Cursor or Codex wiring in this slice. Claude only.
• Does not add the hook's row to the root README.md hooks table — that path classifies as docs and cannot ship alongside a behavior / engine-runtime slice.
• Does not write prose rules. A sibling change owns the rule text.
• Does not rename the hook directory, though gh-write-verification is now narrower than its scope — detector 3 is not a gh write. Renaming would rewrite every file plus the install.sh wiring and the settings marker on an otherwise green branch; recommended as a follow-up, noted in the README.
• Does not change wait-needs-wakeup, which is what pushes agents into these polling loops in the first place. Detector 3 only makes the loop terminate; deciding whether the loop should exist stays with that hook.

Test Plan

Test Plan

Repro before, with the guard absent:

• gh pr edit 295 --base main → GraphQL: Projects (classic) is being deprecated ... (repository.pullRequest.projectCards), exit 1 — while gh api repos/EdbertChan/catstack/pulls/295 --jq .base.ref → main, exit 0
• every existing Bash PreToolUse hook fed the five incident payloads (gh pr edit --add-label, gh pr edit --base, gh pr edit "$1" --base main >/dev/null 2>&1, git push ... >/dev/null 2>&1, gh pr merge 291 --squash --admin) → all exit 0, none caught
• pr-schema-gate fed its own gh pr edit --body-file case with cwd in catstack → exit 0 (no scripts/create-pr.mjs); the identical payload with cwd in a repo that has one → exit 2
• PRs hook: ui-input-guard blocks synthetic input at the user's live session #291/docs: prove UI work on a disposable surface, not the user's session #292/gate: an unchecked file is not a clean file #296/hook(wrong-check-reflect): catch a concession that the user found the miss #298 all report merged=true; git merge-base --is-ancestor <merge_commit> origin/main fails for all four, and git branch -r --contains shows each on its own origin/stack/... branch

Repro after:

• gh pr edit 295 --add-label admin-bypass and gh pr edit 295 --base main → exit 2, message names gh api -X PATCH repos/<owner>/<repo>/pulls/<n> and gh api -X POST repos/<owner>/<repo>/issues/<n>/labels
• gh pr edit "$1" --base main >/dev/null 2>&1, git push origin HEAD >/dev/null 2>&1, gh pr merge 291 --squash --admin &>/dev/null → exit 2
• grep -q ... 2>/dev/null, command -v x >/dev/null 2>&1, git cat-file -e ... 2>/dev/null, git status --porcelain >/dev/null 2>&1, git init/commit/npm ci/rm -rf/pkill ... >/dev/null 2>&1, ... || exit 1, if ! ...; then, 2>&1 >/dev/null, grep -q ... 2>/dev/null && git push ..., gh api -X PATCH ... → all exit 0
• transcript with gh pr merge 291 + gh pr merge 292 and no check → exit 2 naming both; same transcript plus verify_pr_landed_on_trunk.sh 291 → silent for hook: ui-input-guard blocks synthetic input at the user's live session #291
• bash engine/hooks/gh-write-verification/verify_pr_landed_on_trunk.sh 291 → FAIL: PR #291 reports MERGED but 314f0447... is not on origin/main, exit 1
• bash engine/hooks/gh-write-verification/verify_pr_landed_on_trunk.sh 318 → OK: f19319b2... is an ancestor of origin/main, exit 0

Failure 4 (self-matching process wait), repro before:

• pgrep -f zzz_selfmatch_demo_ghwv.sh for a token no process on the box uses → RUNNING; pgrep -af showed it matched the argv of the shell asking
• a token appearing exactly once, as the pgrep argument only → still RUNNING, so a "pattern appears elsewhere in the command" test would miss the canonical loop
• pkill -f vvvv_selfkill_ghwv inside bash -c → the following echo SURVIVED never ran; the wrapper was killed, harness reported exit 144
• the stuck watcher on this box (PID 1429941): its log sentinel had already matched 35 times, so only the ! pgrep half kept it alive — killed, confirmed gone
• bracket form in isolation → absent; the same bracket form in a command that also spells the token plainly → RUNNING, which is why the detector re-checks for a plain mention

Failure 4, repro after:

• fires: the incident loop, pkill -f run_all_tests.sh, pgrep -af my_worker.py, pgrep --full …, pgrep -f -u edbert …, and a bracket pattern undone by a plain mention
• silent: bracket idiom alone, a log-sentinel wait, pgrep run_all_tests.sh / pgrep -x bash / pkill -x node (no -f), kill -0 "$PID", pgrep -f "$PATTERN", backtick-substituted pattern

Gates:

• python3 -m unittest discover -s engine/hooks/gh-write-verification/tests -v — 34 tests, OK
• python3 engine/skills/make-pr/scripts/preflight.py --base origin/main — unit engine-runtime: 9 file(s), neutral 1 file(s): tests/test_install.py, ok preflight passed
• bash scripts/run_all_tests.sh — 37 suites, EXIT4=0, no FAILED/ERROR: lines
• python3 scripts/check_hook_test_coverage.py — OK (26 hook(s) checked)
• python3 scripts/check_no_new_comments.py --base origin/main — ok no new comments
• python3 scripts/check_no_dated_provenance.py --base origin/main — ok no dated provenance
• python3 scripts/check_no_tracked_local_artifacts.py, check_skills_three_harnesses.py, check_ecosystem_boundaries.py, check_skill_file_refs.py, check_skill_test_coverage.py, check_skill_test_debt_no_growth.py, check_skill_trigger_mechanism.py, check_dora_baseline.py — all OK
• ruff check . --select E9,F — All checks passed
• shellcheck install.sh and shellcheck engine/hooks/gh-write-verification/verify_pr_landed_on_trunk.sh — clean
• node engine/skills/draft-pr/scripts/validate-pr-body.mjs and lint-diff-atomicity.mjs --base origin/main --review-lane behavior — both passed

Revert Plan

Revert Plan

• Safe to revert? Yes
• Revert command: git revert <sha> then ./install.sh
• Post-revert steps: ./install.sh rewrites ~/.claude/settings.json; scripts/prune_dead_hook_entries.py runs inside it and drops the now-dangling gh-write-verification entries. A stale entry left behind is inert — the hook file is gone, so the harness logs a missing-script error and the tool call proceeds.
• Data migration? No. The hook writes no state file.

🤖 Generated with Claude Code

https://claude.ai/code/session_01F43CBUnsDEs6J2zEC1r8a8

[cursor]

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_37d0f9c1-f5aa-47b2-a652-6d51c6afde3b)