fix(gh): make stop hook block once per user turn

github/hooks/closeout-stop.sh

@@ -8,16 +8,21 @@
 # by a hallucinated or premature stop. The skill arms a state file
 # (.git/review-pr-closeout.json) the moment Phase 4's stop conditions hold and
 # clears it once the user's merge choice is in (or the --auto-merge closeout ran
-# or aborted). While the file exists, this hook blocks turn-end with a message
-# naming the PR and the missing step — the ask cannot silently vanish.
+# or aborted). While the file exists, this hook blocks one turn-end per user
+# turn with a message naming the PR and the missing step — the ask cannot
+# silently vanish, and it does not loop: after the single block the turn ends.
 #
 # Guard rails (this hook fires on EVERY turn end — Stop has no matcher):
 #   - Fails open: no jq, no git repo, no state file, or a subagent stop
 #     (agent_id present in the input — a subagent cannot ask the user) all pass.
+#   - Fires at most once per user turn: stop_hook_active is true when the turn
+#     is already continuing from a stop-hook block — pass through so the turn
+#     can end. The single block injects the reminder once; resolving the closeout
+#     (the merge ask or clear-closeout) is what removes the state file.
 #   - Blocks via hookSpecificOutput.additionalContext (exit 0 + JSON): the
 #     message is fed back to the model as "Stop hook feedback" and the turn
-#     continues — same loop protections (stop_hook_active, 8-block cap) as a
-#     decision:block, without the hook-error label.
+#     continues — same loop protections as a decision:block, without the
+#     hook-error label.
 #
 # Input (stdin JSON): Stop common fields + stop_hook_active,
 #   last_assistant_message, background_tasks, session_crons.
@@ -35,6 +40,15 @@ command -v jq >/dev/null 2>&1 || exit 0
 agent_id=$(printf '%s' "$input" | jq -r '.agent_id // empty')
 [ -z "$agent_id" ] || exit 0
 
+# At most one reminder per user turn. stop_hook_active is true when the turn is
+# already continuing from a stop-hook block — passing through lets the turn end
+# instead of re-blocking every end attempt. The reminder was already injected
+# once; resolving the closeout (the merge ask, or clear-closeout) is what clears
+# the state file on its own schedule. Official anti-loop pattern per
+# https://code.claude.com/docs/en/hooks-guide.md.
+stop_hook_active=$(printf '%s' "$input" | jq -r '.stop_hook_active // false')
+[ "$stop_hook_active" = "true" ] && exit 0
+
 # The state file lives in the repo's gitdir; resolve it from the hook cwd
 # (the session's working directory). Outside a git repo there is no closeout.
 GITDIR=$(git rev-parse --git-dir 2>/dev/null) || exit 0
@@ -64,12 +78,5 @@ else
 fi
 msg="${msg} Full verification procedure: ${PLUGIN_ROOT}/skills/review-pr/references/closeout.md (When the hook fires)."
 
-# Repeated blocks mean the condition is not resolving (e.g. the user declined
-# the required action) — escalate to the last-resort escape hatch.
-stop_hook_active=$(printf '%s' "$input" | jq -r '.stop_hook_active // false')
-if [ "$stop_hook_active" = "true" ]; then
-  msg="${msg} You have been blocked repeatedly: if this closeout cannot be resolved, tell the user they can delete the repo's .git/review-pr-closeout.json to release it."
-fi
-
 jq -n --arg c "$msg" '{hookSpecificOutput:{hookEventName:"Stop",additionalContext:$c}}'
 exit 0

github/skills/create-pr/SKILL.md

@@ -78,7 +78,7 @@ See `references/repository-templates.md` for template detection and compliance d
 
 **Goal**: Delegate CI monitoring and reviewer-comment triage to the dedicated skill.
 
-**Action**: After the PR is created, invoke `Skill("github:review-pr", "<PR#>")` to run the baseline review and launch the persistent CI + comment watch. The review-pr skill owns the Monitor script, the skeptical triage agent, the review → fix → commit+push → wait-for-review loop, through to the merge decision and the post-merge branch hygiene (remote + local head cleanup, `fetch --prune`, fast-forward `main`/`develop`). **Once CI is green and every comment is triaged, review-pr asks the user whether to merge via `AskUserQuestion` (merge commit/squash/rebase/don't) BEFORE its closeout ceremony** — the summary comment and body rewrite run only on a merge choice. The merge ask is enforced by the plugin's Stop hook: review-pr arms a closeout state the moment the stop conditions hold and turn-end stays blocked until the decision resolves (bounded by the platform's 8-consecutive-block cap) — the handoff cannot silently skip the ask. See `references/pr-creation-handoff.md` for the handoff contract including post-merge hygiene. This skill does not duplicate that cleanup; it is the handoff target's responsibility.
+**Action**: After the PR is created, invoke `Skill("github:review-pr", "<PR#>")` to run the baseline review and launch the persistent CI + comment watch. The review-pr skill owns the Monitor script, the skeptical triage agent, the review → fix → commit+push → wait-for-review loop, through to the merge decision and the post-merge branch hygiene (remote + local head cleanup, `fetch --prune`, fast-forward `main`/`develop`). **Once CI is green and every comment is triaged, review-pr asks the user whether to merge via `AskUserQuestion` (merge commit/squash/rebase/don't) BEFORE its closeout ceremony** — the summary comment and body rewrite run only on a merge choice. The merge ask is enforced by the plugin's Stop hook: review-pr arms a closeout state the moment the stop conditions hold and one turn-end per user turn is blocked until the decision resolves — the handoff cannot silently skip the ask. See `references/pr-creation-handoff.md` for the handoff contract including post-merge hygiene. This skill does not duplicate that cleanup; it is the handoff target's responsibility.
 
 **`--auto-merge` passthrough**: If `$ARGUMENTS` carried `--auto-merge`, pass it through to the review-pr invocation as `Skill("github:review-pr", "<PR#> --auto-merge")`. It instructs review-pr to skip the merge `AskUserQuestion` — the closeout ceremony (summary comment + body rewrite) still runs first — and auto-merge with `gh pr merge --merge` once CI is green and every non-escalate comment is triaged — see `references/pr-creation-handoff.md` for the contract and the escalate fallback. Pass it through **only** when the user explicitly set it; never infer it.
 

github/skills/review-pr/SKILL.md

@@ -60,7 +60,7 @@ Stop the Monitor with `TaskStop` when EITHER holds — full conditions in `refer
 **Goal**: Once Phase 4 holds, ask the user whether to merge FIRST — before any ceremony. The summary comment and body rewrite run only on a merge choice; "Don't merge" skips the ceremony and goes straight to `TaskStop`. On a merge choice, post-merge hygiene (linked-worktree removal, switch to `main`, sync with origin) runs unconditionally. Full templates and ordered steps in `references/closeout.md`.
 
 **CRITICAL constraints (hold even when detail is delegated to L3)**:
-1. **Arm the closeout state the moment Phase 4 holds — before anything else**: `bash ${CLAUDE_PLUGIN_ROOT}/skills/review-pr/scripts/arm-closeout.sh "$PR"` (append `--auto-merge` when the opt-in was parsed in Phase 1). This writes the repo's `.git/review-pr-closeout.json`, arming the plugin's Stop hook: while the file exists, every turn end is blocked with a message naming the missing merge decision — the Phase 5 ask cannot be skipped by a premature stop (bounded by Claude Code's 8-consecutive-block cap per turn; a user interrupt also bypasses it). **When the hook blocks, first verify the pending closeout is real** — a stale state file (ask already answered, summary posted, or PR merged without clearing) is a false alarm: judge simple checks directly (`gh pr view --json state,mergedAt`, the `<!-- review-pr:summary -->` marker lookup), spawn an independent subagent with clean context for complex or ambiguous situations — see `references/closeout.md` (When the hook fires). A verified-stale state is cleared, not re-asked. Clear it the moment the decision is resolved — `bash ${CLAUDE_PLUGIN_ROOT}/skills/review-pr/scripts/clear-closeout.sh "$PR"`: after the user answers (any choice, including "Don't merge"), after the auto-merge completes, or after the opt-in aborts. A stale file blocks the next stop; its message repeats the clear path.
+1. **Arm the closeout state the moment Phase 4 holds — before anything else**: `bash ${CLAUDE_PLUGIN_ROOT}/skills/review-pr/scripts/arm-closeout.sh "$PR"` (append `--auto-merge` when the opt-in was parsed in Phase 1). This writes the repo's `.git/review-pr-closeout.json`, arming the plugin's Stop hook: while the file exists, one turn-end per user turn is blocked with a message naming the missing merge decision — the Phase 5 ask cannot be skipped by a premature stop, and the hook passes through (`stop_hook_active`) on later end-attempts of the same turn so it does not loop (a user interrupt also bypasses it). **When the hook blocks, first verify the pending closeout is real** — a stale state file (ask already answered, summary posted, or PR merged without clearing) is a false alarm: judge simple checks directly (`gh pr view --json state,mergedAt`, the `<!-- review-pr:summary -->` marker lookup), spawn an independent subagent with clean context for complex or ambiguous situations — see `references/closeout.md` (When the hook fires). A verified-stale state is cleared, not re-asked. Clear it the moment the decision is resolved — `bash ${CLAUDE_PLUGIN_ROOT}/skills/review-pr/scripts/clear-closeout.sh "$PR"`: after the user answers (any choice, including "Don't merge"), after the auto-merge completes, or after the opt-in aborts. A stale file blocks the next stop; its message repeats the clear path.
 2. **Ask the merge question the moment Phase 4 holds.** Do not post the summary or rewrite the body first — the user's choice gates the ceremony. When everything is settled, the closeout ask is the next step, not more polling.
 3. The ceremony runs ONLY on a merge choice: capture the summary comment URL from `gh pr comment` stdout (`SUMMARY_URL=$(gh pr comment …)`).
 4. The Review-cycle line in the rewritten body MUST contain that literal URL — a count with no link is not a pointer, and the quoted heredoc will not expand `$SUMMARY_URL`, so paste it.

github/skills/review-pr/references/closeout.md

@@ -28,11 +28,12 @@ bash ${CLAUDE_PLUGIN_ROOT}/skills/review-pr/scripts/arm-closeout.sh "$PR" --auto
 
 Arming writes `.git/review-pr-closeout.json` (resolved via `git rev-parse --git-dir`, so it
 works from any cwd in the repo). While the file exists, the plugin's Stop hook
-(`hooks/closeout-stop.sh`) blocks every turn end with a message naming the PR and the missing
-step — the merge ask cannot be skipped by a premature or hallucinated stop. This is the
-enforcement net for the whole closeout: turn-end stays blocked until the decision resolves
-(bounded by Claude Code's 8-consecutive-block cap per turn; a user interrupt also bypasses
-it — the hook's message always names the clear script as the escape hatch).
+(`hooks/closeout-stop.sh`) blocks one turn-end per user turn with a message naming the PR and
+the missing step — the merge ask cannot be skipped by a premature or hallucinated stop, and
+the reminder does not loop: the hook passes through (`stop_hook_active`) on the second
+end-attempt of the same turn, so the turn ends once the reminder is injected. A user
+interrupt also bypasses it — the hook's message always names the clear script as the escape
+hatch.
 
 Clear it the moment the decision is resolved:
 
@@ -266,8 +267,8 @@ stop — do not retry with different flags, do not force-push, and do not re-arm
 the user decides next. If the user interrupts and you resume, and the gate no longer holds (a
 new comment arrived, CI re-ran red), do not auto-merge — re-check the gate, and if escalate
 items now exist, fall back to the explicit question. Either way, clear the closeout state
-once the opt-in is resolved (completed or aborted) — while it stays armed, every turn end is
-blocked until the decision is settled.
+once the opt-in is resolved (completed or aborted) — while it stays armed, one turn-end per
+user turn is blocked until the decision is settled.
 
 **On a successful auto-merge**, proceed to "After a successful merge" hygiene exactly as the
 explicit-choice path would. `TaskStop` the Monitor.
@@ -379,16 +380,16 @@ holding). If the user interrupts and you resume, skip steps already completed; i
 either (the ask gates it). Under `--auto-merge`, if the gate no longer holds on resume (new
 comment or CI re-ran red), do not auto-merge — re-check and fall back to the explicit
 question if escalate items now exist. If an interrupt left the closeout state armed, clear
-it when the closeout is resolved (ask answered or opt-in consumed); while armed, every
-turn end is blocked until then.
+it when the closeout is resolved (ask answered or opt-in consumed); while armed, one turn-end
+per user turn is blocked until then.
 
 ## Do not
 
 - Do not ask to merge or post the summary while comments are still open or CI is still red —
   the gate must hold first, and the summary would claim a merge-ready state that is not true.
 - Do not end the turn with the closeout state armed — the Stop hook blocks it. Clear it
-  (`clear-closeout.sh "$PR"`) as soon as the decision resolves; while it stays armed, every
-  turn end is blocked with a message naming the PR and the missing step.
+  (`clear-closeout.sh "$PR"`) as soon as the decision resolves; while it stays armed, one
+  turn-end per user turn is blocked with a message naming the PR and the missing step.
 - Do not run the ceremony (summary comment + body rewrite) before the user's merge choice —
   the ask comes first, and "Don't merge" skips the ceremony entirely.
 - Do not rewrite the title/body to claim something the diff does not deliver.