Skip to content

feat(gh): gate PR and issue bodies on visual approval - #354

Merged
twistedmelonman merged 1 commit into
mainfrom
claude/feat-gh-wrapper-gate-5e415a2f
Sep 19, 2026
Merged

twistedmelonman merged 1 commit into
mainfrom
claude/feat-gh-wrapper-gate-5e415a2f

Conversation

@twistedmelonman

Copy link
Copy Markdown
Owner

Closes the manual-gh gap left by claude-config#546. The hook armed there
only sees Claude Code's Bash tool, so a PR or issue body written at an
interactive prompt still reached GitHub without being read. This is the
other half of that deliberately redundant pair.

What changes

gh pr create|comment|edit and gh issue create|comment|edit now refuse a
body unless its exact bytes are in gate-review's approved/ directory. The
wrapper asks the same gate-review.sh check the hook asks, so one approval
satisfies both halves and there is one rule to learn:

form result
--body-file with an absolute path checked against approvals
--body/-b inline refused, nothing on disk to hash
relative, ~/..., $VAR/..., - refused, gh and the gate resolve these differently
no body flag at all passes untouched

Titles and labels carry no prose, so gh pr edit --add-label and
gh pr review --approve are unaffected.

Two things to accept consciously

This adds friction to your own hand-typed gh. The hook only fires
inside Claude Code; this wrapper fires in your interactive shell too. After
this merges, gh pr create --body "fix typo" at your terminal is blocked
until you write a file, stage it, approve it, and re-run. Interactive
gh pr create with no body flag still drops you in the editor and passes,
and --web is unaffected. The stashed version had a PERSONIFY_OK opt-out;
this has none, which follows the #546 design but is a real change to your
own workflow.

CI does not exercise the approval path. GitHub runners have no
~/.claude/scripts/gate-review.sh, so 14 approval cases skip there and the
suite is green without having tested them. They run locally, where the
pre-push hook is the actual gate. CI still covers the fail-closed and
no-body paths.

Why not the stashed design

The stash read PERSONIFY_OK from the environment. That channel is dead,
not merely strict: the Bash tool runs in a process that does not inherit the
interactive shell's environment, so no human could ever satisfy it. Measured
2026-09-18. Approval is read from disk instead.

Fails closed

If gate-review.sh is missing, the gate blocks rather than passes. A
redundant pair whose halves disagree about the unverifiable case is not
redundant. A machine without claude-config installed cannot write PR bodies
through this wrapper, which is intended.

Testing

34 cases in bash/tests/test-gh-wrapper-approval-gate.sh, driving the real
gate-review.sh against a sandboxed GATE_REVIEW_DIR rather than a stub —
claude-config has three incidents of a green suite over broken code from
exactly that substitution.

Six mutants confirm the suite detects a gate that stops gating. One survived
the first pass and is worth naming: the relative-path cases were blocking
because the file did not exist in the test's cwd, not because the path was
relative, so deleting the absolute-path rule left them green. They now
resolve to a file whose bytes are approved, so the path rule is the only
thing that can block them, and the mutant dies.

Full dotfiles suite passes, core.hooksPath intact afterward, shellcheck
-S info clean with no disable directives.

https://claude.ai/code/session_011awg91UvzUos9YoXHJ2e8B

The gh wrapper now refuses to write a PR or issue body unless those exact
bytes are in gate-review's approved/ directory. This is the manual-gh half
of the pair armed in claude-config#546; the hook half only sees Claude
Code's Bash tool, so until now a body typed at an interactive prompt
reached GitHub ungated.

The stashed version of this change read PERSONIFY_OK from the environment.
That channel is dead: the Bash tool runs in a process that does not inherit
the interactive shell's environment, so an env-var acknowledgement is
unsatisfiable by a human rather than merely strict. Approval is read from
disk instead, by the same gate-review.sh the hook calls, so one approval
satisfies both halves.

The rule enforced here is deliberately identical to the hook's: an absolute
--body-file is verifiable and checked, an inline --body has nothing to hash
and is refused, and a relative or unexpanded path is refused because gh and
the gate would resolve it against different directories. Titles and labels
carry no prose and pass untouched.

Fails closed when gate-review.sh is absent. A redundant pair whose halves
disagree about the unverifiable case is not redundant.

Tests drive the real gate-review.sh against a sandboxed approvals directory
rather than a stub, and six mutants confirm the suite detects a gate that
stops gating. One of those mutants initially survived: the relative-path
cases were blocking because the file did not exist, not because the path
was relative, so they passed with the rule deleted. They now resolve to an
approved file, which leaves the path rule as the only thing that can block.

Claude-Session: https://claude.ai/code/session_011awg91UvzUos9YoXHJ2e8B
@claude

claude Bot commented Sep 19, 2026

Copy link
Copy Markdown

The PR adds _gh_wrapper_approval_gate to bash/gh-wrapper.sh -- a gate that blocks gh pr/issue body writes unless the body bytes have been pre-approved via gate-review.sh.

No BLOCK-level issues found:

  • Arg parsing handles all flag spellings correctly.
  • Fails closed when gate-review.sh is absent (returns 1).
  • No unvalidated input reaches a privileged operation.
  • Integration into both code paths is consistent; export -f is updated.
  • Test suite covers all edge cases.

VERDICT: PASS

@claude

claude Bot commented Sep 19, 2026

Copy link
Copy Markdown

Argument parsing correctly extracts subcommand and body-file path through flag-spelling variations and skip logic. Gate logic blocks inline bodies (unverifiable), relative paths (resolution ambiguity), missing files, and missing gate-review.sh. Approval check defers to gate-review.sh. Test suite is comprehensive with proper HOME sandboxing and edge-case coverage.

Gate fails closed (line 672) when gate-review.sh is absent, matching documented design. No unhandled async operations, data loss, or silent failures identified.

VERDICT: PASS

@twistedmelonman
twistedmelonman merged commit 97747c1 into main Sep 19, 2026
5 checks passed
@twistedmelonman
twistedmelonman deleted the claude/feat-gh-wrapper-gate-5e415a2f branch September 19, 2026 02:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant