[john-the-dev]

Draft until a 12-hour shadow trial reports. This PR ships the gate in shadow mode, which logs every merge decision and never denies. It stays a draft until a 12h trial on a live core has been summarised (python3 hooks/_merge_hold.py --summary) and the counts and latencies are posted here.

What changed, and why

A PR was merged during an announced re-read (#6300 → #6301). The re-read then found two defects, which needed the follow-up. hooks/github-evidence-guard.py already matches gh pr merge, but only adds advice after the command has run. It never denies.

This PR extends that hook instead of adding a new one. The core now registers the same file a second time, before Bash, with --merge-gate --repo <checkout>. That registration runs only the merge gate. Every existing registration and behaviour of the hook is unchanged.

Read first:

1. hooks/_merge_hold.py: the policy, a helper beside the hook in the _shell_scan.py style. It covers the merge-command parser, the single GraphQL read, holds, expiry, standing, mode, the shadow log, the override and the room-hold CLI.
2. hooks/github-evidence-guard.py: merge_gate() and the --merge-gate branch in decide(), about 20 lines.
3. src/agent/claude/cli/build-core-settings.mjs (arg 9) and session-launch.sh: the registration.

There is no src/ module, because the hook is the only consumer of this policy.

Behaviour

Merge commands it reads:

• gh pr merge <n|url> [--repo|-R o/r], except --disable-auto.
• gh api -X PUT repos/o/r/pulls/<n>/merge.
• gh api graphql carrying mergePullRequest or enqueuePullRequest. The body can be inline, -F query=@file or --input file. The pullRequestId can be a literal or a -f/-F variable, and the node id is resolved in the same query.

It also reads bash -c, sh -c and eval wrappers. Heredoc bodies are data, so a commit message that mentions gh pr merge is not gated.

The check. One gh api graphql call per merge, with a 5 s budget. It returns the head commit date, author, review requests, and the last 100 reviews and issue comments.

HOLD. A comment or review body posted since the head commit says one of: re-read, re-reading, reading again, will re-review, taking another look, hold merge, do not merge, please wait. It must come from a login with standing.

• These never hold: the PR author, bots, quoted (>) lines, and a re-read named as done ("after re-reading it", "I re-read the diff"). The last case was a false positive found on a live PR while building this.
• A hold resolves when that login posts any newer review, of any state. It also resolves on a newer comment that approves or says LGTM / done / no further / clear to merge without announcing again.

Room holds. A hold said in a chat room is recorded with python3 hooks/_merge_hold.py OWNER/REPO PR --add-hold --who LOGIN --note … [--url …] [--expires-at …] and cleared with --clear-hold. These live in <workspace>/state/merge-holds.json, with the path resolved through workspace_default.

UNKNOWN. Any of these: a failed, timed-out, malformed or truncated read (more than 100 comments or reviews since the head commit), an unreadable hold file, a bad TTL, or a merge whose PR it cannot name (no --repo, a $VAR id, an unreadable body). It is never CLEAR.

Modes. env.MERGE_HOLD_MODE is set in sutando.config.local.json, through config_get, the repo's convention for non-secret config.

• shadow is the default. It never denies.
• enforce denies on HOLD and UNKNOWN. A HOLD denial names the reviewer, the time, the URL and the expiry.
• Any other value enforces, so a typo cannot switch the gate off.

Logging. Both modes append one line per merge target to <workspace>/logs/merge-hold-shadow.jsonl. Each line has ts, repo, pr, verdict (HOLD / CLEAR / EXPIRED / UNKNOWN), holder, elapsed_ms, kind (pr-merge, rest-merge, graphql-enqueuePullRequest, …), mode and override. --summary prints counts by verdict and p50/p95 elapsed_ms.

Can a hold block merges forever?

No. There are four ways out, and none of them depends on the announcer coming back.

• Expiry. A hold blocks for env.MERGE_HOLD_TTL_HOURS (default 4) from the announcement. After that it passes, with the advisory line EXPIRED hold by <login> announced <t> (<age>); re-ask them <url>. That line is never a denial. Room holds expire the same way, or at their own expires_at.
• Standing. Only a requested reviewer, a login that has reviewed the PR, or an OWNER / MEMBER / COLLABORATOR author association can hold. The PR author and bots cannot. So a random comment cannot block a merge. The association is part of the same GraphQL read, so checking it costs no extra call.
• New commits. An announcement before the head commit does not count. The author pushing the fix moves the window.
• Override. SUTANDO_MERGE_HOLD_OVERRIDE='<reason>' gh pr merge … passes a live HOLD or an UNKNOWN in enforce mode. It works like the repo's existing inline-word overrides, for example SUTANDO_ALLOW_STALE_POST=1. The reason must be at least 8 characters. It is appended to <workspace>/state/guards/merge_hold_overrides.jsonl, next to the other guard logs under state/guards/, before the merge is allowed. If that write fails, the merge is denied.

An agent that says it will re-read and never does holds the PR for at most the TTL, and the next merge attempt tells the merger to re-ask them.

UNKNOWN is a separate case and still denies in enforce mode: a GitHub outage is rare, and the override covers it.

Cost

Only merge commands pay anything. The values below come from tests/github-evidence-guard-merge-gate.test.py (Cost), measured on a laptop.

Path	Subprocess calls	File reads	Time
Non-merge Bash command (6,000 decisions)	0	0	1.1 µs/command
Merge, stubbed gh	1 (gh api graphql)	the hold file	1.7 ms excluding the network
Merge, live GitHub (5 recent PRs, read-only)	1	the hold file	509–586 ms

A non-merge command returns on the hook's existing gh (pr|run) (view|checks|merge)|gh api string match. A merge past the 5 s budget is UNKNOWN.

The hook's PostToolUse evidence fetch (github-decision.py collect) runs after the command and makes several REST reads, so it is not reused before the tool.

What user behaviour or bug does this prove?

Live, read-only, against this repo:

• fix(core-limit-hold): the watchdog resolves the seat the launcher does, and an extended hold re-presents its card #6301 at first read as HOLD, from an APPROVED review saying "approved this exact head after re-reading it". That is the past-tense false positive. Now it reads CLEAR.
• fix(core-limit-hold): the watchdog reads the launcher's seat, and the card lasts the whole hold #6300's re-read was announced off GitHub, so its GitHub timeline has no announcement. It reads CLEAR, and only a recorded room hold would have stopped that merge. See Limits.

Tests

• python3 tests/merge-hold.test.py: 45 tests, OK (also under /usr/bin/python3 3.9.6).
  • Policy: announcement → HOLD. Same-author review of any state, or LGTM → CLEAR. Another author's LGTM → still HOLD. Announcement before the last commit is ignored. Author, bots and no-standing logins are ignored.
  • Expiry at the TTL. TTL from sutando.config, with a bad value → UNKNOWN.
  • Room hold add, clear, resolve and expire, including expires_at.
  • gh failure, timeout, malformed or truncated response → UNKNOWN. Corrupt state → UNKNOWN.
  • One GraphQL call per check. Parser edges. The CLI.
• python3 tests/github-evidence-guard-merge-gate.test.py: 31 tests, OK.
  • Held gh pr merge → deny, naming the reviewer, URL and expiry. Clear → allow.
  • Enqueue and merge GraphQL, with literal, variable and file bodies, are parsed and resolved. Unresolvable targets → deny. REST merge and wrapped merges are gated.
  • Non-merge commands and heredoc bodies pass with no gh call. UNKNOWN → deny with the override hint.
  • The override passes HOLD or UNKNOWN and is logged. Short or empty reasons are refused. An override that cannot be logged is refused.
  • An expired hold is advisory.
  • Shadow mode never denies and always logs, for HOLD, CLEAR, UNKNOWN, unparsed, EXPIRED, override and an unwritable log. The summary counts and percentiles are checked.
  • Mode comes from config, and a typo enforces.
  • The gate runs only before the tool, and the hook is unchanged without --merge-gate.
  • Delegation to _merge_hold.gate. Subprocess end to end in both modes, against a hermetic fake checkout. Registration.
• node --test tests/agent/claude/cli/build-core-settings.test.ts: 25/25.
• tests/pr-decision-evidence.test.py (25) and tests/pr-outcome-evidence.test.py (13), the existing evidence-guard suites: OK.
• tests/stale-head-post-guard.test.py, tests/shell-trap-guard.test.py and the launcher suites that source session-launch.sh: OK.
• Every new suite also passes under scripts/hermetic-workspace-guard.py run.
• bash scripts/review-checks.sh --diff: PASS. lint-workspace-resolution.sh --diff: clean. gen-src-map.py --check: up to date. ruff==0.15.22: clean. docs_audit.py: PASS.
• Diff coverage (diff-cover against origin/main): 99%. The one missed line is sys.exit(main()).

Feature/documentation consistency

hooks/README.md has a new section. docs/github-verification.md, the canonical doc for GitHub evidence, has a new paragraph, and its last_verified in docs/catalog.json is bumped.

Known limits

• It sees one command at a time. A merge run from a script, an alias, xargs or another tool is not seen.
• Phrase matching is English and literal.
• An announcement made only in a chat room is invisible to GitHub. It holds only if someone records it with --add-hold, which is what fix(core-limit-hold): the watchdog reads the launcher's seat, and the card lasts the whole hold #6300 needed.
• Write access is approximated by author association. A COLLABORATOR with read-only access counts as having standing.
• Codex-core sessions do not load Claude hooks.
• Concurrent --add-hold writers can lose an update. Writes are atomic, but there is no lock.

Trial plan

Run a live core on this branch's hook in shadow mode for 12 hours, then post the --summary output here: counts by verdict, and p50/p95. The plan is to switch to enforce only if HOLD and UNKNOWN look right on real merges.

Sutando-Task: 20261107-024

🤖 Generated with Claude Code

[sonichi-dot]

Reviewed exact head 58b2de635811dd9e85547476a4ea126f218ee6c4.

Recommendation: not ready to merge. Five reproduced correctness gaps below can release or bypass an active hold. Shadow mode mitigates immediate enforcement disruption, but records a false CLEAR or omits these merge attempts entirely, so the trial would not expose all of them. Please fix the cases and retain the planned live-core trial before enabling enforcement.

Validation: policy 45/45, gate 31/31, settings builder 25/25; adjacent evidence/outcome/stale-head suites 25/25, 13/13, 31/31; shell-trap 52 passed, 1 skipped. Python suites ran under the hermetic workspace guard; review-checks.sh passed. Additional synthetic probes exercised the failures with deny/HOLD controls, without performing merges. No live-core restart or 12-hour trial was run. At posting, CI remained in progress and CLA was successful. The CLI preflight could not query GitHub here; current state, reviews and comments were checked through the connector instead.

[sonichi-dot]

[P2] Keep holds when a follow-up explicitly says the review is unfinished. With the existing policy fixture, Alice's re-reading comment yields HOLD, but a newer I'm not done yet or I haven't approved this yet changes it to CLEAR. RESOLVE.search matches the negated word. Quoted > LGTM also releases the hold because only announcement detection strips quotes. Require an affirmative, unquoted release and add regression cases for these unfinished-status updates.

[sonichi-dot]

[P2] Parse attached short-option values before classifying API requests. gh api repos/o/r/pulls/7/merge -XPUT is valid gh syntax, but -XPUT is not recognized here, leaving method=None; the REST branch consequently returns no targets. With an enforced HOLD stub, -X PUT denies after one check while -XPUT allows with zero checks. Normalize attached values for the supported API flags instead of silently treating a merge as read-only.

[sonichi-dot]

[P2] Recognize heredoc operators outside quoted data only. For echo '<<EOF' followed by a newline and gh pr merge 7 --repo o/r, this regex incorrectly starts a heredoc inside the quoted echo argument and drops the actual merge line. Bash executes the merge, but merge_targets() returns an empty list and enforce mode allows it without checking. Reuse shell quote state when identifying heredoc declarations and test quoted/comment occurrences followed by real commands.

[sonichi-dot]

[P2] Do not discard valid merge spellings before the shell scanner runs. The new gate sits behind the old raw regex: "gh" pr merge 7 --repo o/r, or a backslash-newline between pr and merge, returns {} here even though merge_targets() correctly identifies the same merge. Bash confirms the expected argv, and an enforced HOLD probe makes zero checks for either spelling. Make the fast path conservative or normalize shell syntax before returning.

[sonichi-dot]

[P2] Recognize combined shell options when unwrapping command strings. bash -lc 'gh pr merge 7 --repo o/r' executes the merge normally, but this condition only recognizes a separate -c token, so the wrapper is discarded as a non-gh command. The enforced HOLD probe allows it with zero checks, unlike bash -c. Parse the shell's combined options (including common -lc/-ec forms) before locating its command argument.

[github-actions]

Coverage Gate

✅ Diff coverage PASSES the 95% bar. Whole-tree (informational): 93%.

Diff Coverage

Diff: origin/main...HEAD, staged and unstaged changes

• hooks/_merge_hold.py (100%)
• hooks/github-evidence-guard.py (100%)

Summary

• Total: 504 lines
• Missing: 0 lines
• Coverage: 100%