Repository navigation
refactor(agents): replace the gate's shell parser with plain checks - #815
Conversation
The agent PR gate carried a ~1,000-line bash tokenizer to follow cd, wrappers, substitutions and heredocs. It now matches regexes over the whole command string: gh PR actions and their API forms are checked as plain text, and a git push must be a plain `git [-C <worktree>] push` on its own before it is judged against the review markers. Only plain forms are caught; a command that merely mentions a guarded name is blocked too, and long text goes in a file instead. Supersedes the open fixes for #797 and #809. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 Summary
Merge Risk: 🔵 Low · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Linked Issues check |  |
The plain-text scan covers the three #797 forms and the first two #809 forms. It detects guarded text in -R/--repo placements, nested or quoted text, same-line heredoc input, $[…] commands, and … |
Implement the remaining #809 behavior without weakening the required human-only action blocks. Allow the valid parameter-expansion, GraphQL-variable, and thread-reply cases. Add regression tests for all three cases. |
-
Check name Status Explanation Out of Scope Changes check The hook rewrite, push checks, tests, AGENTS.md guidance, Claude Code documentation, and changelog changes all support the agent PR gate objectives in #797and#809. No unrelated product or repository…Docstring Coverage Docstring coverage is 87.50% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 2 files. (3 skipped: 3 u… Title check The title clearly and concisely describes the main change: replacing the gate's shell parser with plain checks. Description check The description directly explains the parser replacement, affected files, behavior changes, limitations, testing, and stated objectives.
Full details: Linked Issues check
-
Explanation
The plain-text scan covers the three
#797forms and the first two#809forms. It detects guarded text in-R/--repoplacements, nested or quoted text, same-line heredoc input,$[…]commands, and here-string input. It does not satisfy the remaining#809requirements. The scan still blocks the valid quoted parameter-expansion case, GraphQL queries with variables, andaddPullRequestReviewThreadReplytext that containsAPPROVE. The PR description explicitly accepts these false positives.
✨ Finishing Touches
-
📝 Generate docstrings
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
✨ Simplify code
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
github-actions
Bot
added
documentation
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
📚 Docs preview is live → https://851b2935-wavehouse-docs.wave-rf.workers.dev
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Organization UI
- Review profile: ASSERTIVE
- Plan: Advanced
- Run ID:
bb72c555-3ba6-415c-83fd-d319ee9ded9b
📒 Files selected for processing (5)
.claude/hooks/agent-bash-gate.sh.claude/hooks/review-gate.test.shAGENTS.mdCHANGELOG.mddocs/src/content/docs/claude-code.md
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.
📜 Review details
⏰ Context from checks skipped due to timeout. (7)
- GitHub Check: Docs build
- GitHub Check: Lint
- GitHub Check: Analyze (actions)
- GitHub Check: Analyze (go)
- GitHub Check: Analyze (go)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📚 Code guidelines (2)
CONTRIBUTING.md — auto-discovered
.github/copilot-instructions.md — auto-discovered
📓 Path-based instructions (3)
Source excerpt: Update the documentation and `CHANGELOG.md` (under `## Unreleased`) that your change affects.
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
docs/src/content/docs/claude-code.mdCHANGELOG.md
See [AGENTS.md](../AGENTS.md) for project conventions, architecture notes, and AI agent instructions.
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
AGENTS.md
Source excerpt: **Docs prose**: never hard-wrap Markdown — one paragraph is one line.
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
docs/src/content/docs/claude-code.mdCHANGELOG.mdAGENTS.md
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-10-12T15:55:58.554Z
Learning: Source excerpt:
# AGENTS.md — AI Agent Instructions for WaveHouse
## Operating Rules
3. **Every code change updates its docs + `CHANGELOG.md` in the same PR** — a code change without its doc update is incomplete ([§Documentation Sync](#documentation-sync)).
🪛 LanguageTool
CHANGELOG.md
[style] ~15-~15: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... regexes over the whole command string. It blocks a non-draft gh pr create / `gh...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~15-~15: The serial comma (Oxford comma, Harvard comma) is missing.
Context: ... pr new, a malformed literal PR title, reviewer and assignee adds, gh pr review --approve`...
(SERIAL_COMMA_ON)
[style] ~15-~15: Since ownership is already implied, this phrasing may be redundant.
Context: ...fused with a request to run the push as its own command. The price is a few false posit...
(PRP_OWN)
AGENTS.md
[style] ~251-~251: This sentence is over 40 words long. Consider splitting it up, as shorter sentences make the text easier to read.
Context: ...alid; it prints the reason on failure). .claude/hooks/agent-bash-gate.sh runs the same check on gh pr create / gh pr new / gh pr edit when the title is written literally, and on the title= field of a gh api call that opens a PR, so a malformed title is caught locally before the PR exists. The rule has a single source of truth —...
(TOO_LONG_SENTENCE)
[style] ~277-~277: Since ownership is already implied, this phrasing may be redundant.
Context: ...fused with a request to run the push as its own command. --all, --mirror, :, glob...
(PRP_OWN)
🔇 Additional comments (3)
.claude/hooks/review-gate.test.sh (1)
262-331: LGTM!Also applies to: 412-487
AGENTS.md (1)
249-251: LGTM!Also applies to: 277-277
docs/src/content/docs/claude-code.md (1)
27-27: LGTM!Also applies to: 55-55, 109-110
github-project-automation
Bot
moved this from Backlog
to In review
in WaveHouse Task Board
A leading + on a refspec force-updates the remote ref, so the gate now refuses it like the force flags instead of stripping it. The docs no longer say the short -r / -a flags on PR creation are caught, and two Unreleased changelog entries no longer describe the parser this branch removes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@coderabbitai review |
❌ Action failedReview failed.
|
|
@coderabbitai review |
1 similar comment
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to GitHub limitations.
🟡 Minor · Match repository flags after gh pr. · agent-bash-gate.sh:60-133
.claude/hooks/agent-bash-gate.sh:60-133
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winMatch repository flags after
gh pr.GitHub CLI accepts both
gh pr -R owner/repo createandgh pr --repo owner/repo create. The currentgh_repattern only allows these flags beforepr, so neither form reachescreate_re. An agent can therefore create a non-draft PR without--draft, contrary toAGENTS.md.Suggested fix
gh_re='(^|[^[:alnum:]_./-])gh[[:space:]]+((-R|--repo)(=|[[:space:]]*)[^[:space:]]+[[:space:]]+)*' +gh_pr_re="${gh_re}pr[[:space:]]+((-R|--repo)(=|[[:space:]]*)[^[:space:]]+[[:space:]]+)*" end='([[:space:]]|$)' -if has "${gh_re}(pr|api)${end}"; then - create_re="${gh_re}pr[[:space:]]+(create|new)${end}" +if has "${gh_re}(pr|api)${end}" || has "${gh_pr_re}(create|new)${end}"; then + create_re="${gh_pr_re}(create|new)${end}"
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Organization UI
- Review profile: ASSERTIVE
- Plan: Advanced
- Run ID:
1fea0a35-70b4-41ac-ae96-7ffdc374cde8
📒 Files selected for processing (5)
.claude/hooks/agent-bash-gate.sh.claude/hooks/review-gate.test.shAGENTS.mdCHANGELOG.mddocs/src/content/docs/claude-code.md
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.
📜 Review details
🧰 Additional context used
📚 Code guidelines (2)
CONTRIBUTING.md — auto-discovered
.github/copilot-instructions.md — auto-discovered
📓 Path-based instructions (3)
Source excerpt: Update the documentation and `CHANGELOG.md` (under `## Unreleased`) that your change affects.
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
CHANGELOG.mddocs/src/content/docs/claude-code.md
See [AGENTS.md](../AGENTS.md) for project conventions, architecture notes, and AI agent instructions.
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
AGENTS.md
Source excerpt: **Docs prose**: never hard-wrap Markdown — one paragraph is one line.
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
AGENTS.mdCHANGELOG.mddocs/src/content/docs/claude-code.md
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-10-12T21:50:27.322Z
Learning: Source excerpt:
# AGENTS.md — AI Agent Instructions for WaveHouse
## Agent PR Discipline
### Drafts only
Agents must create PRs with `gh pr create --draft`. Only humans transition draft → ready-for-review (`gh pr ready` is blocked for agents). Only humans approve or request changes (`gh pr review --approve` / `--request-changes` are blocked).
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-10-12T21:50:27.322Z
Learning: Source excerpt:
# AGENTS.md — AI Agent Instructions for WaveHouse
## Agent PR Discipline
### Drafts only
**PR title format.** The title becomes the squash-merge subject on `main` and is gated by the required `CI` check's `PR title` job — a bad title blocks merge, so don't discover it from CI. It must be Conventional Commits — `<type>(optional-scope)(optional-!): <subject>` — **≤ 72 chars**, subject **lowercase-first** with **no trailing period**. Types: `feat fix docs refactor test chore ci deps build perf revert style`. Validate before creating: `scripts/lint-pr-title.sh "<title>"` (exit 0 = valid; it prints the reason on failure). `.claude/hooks/agent-bash-gate.sh` runs the same check on `gh pr create` / `gh pr new` / `gh pr edit` when the title is written literally, and on the `title=` field of a `gh api` call that opens a PR, so a malformed title is caught locally before the PR exists. The rule has a single source of truth — `scripts/lint-pr-title.sh` — called by this local gate...
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-10-12T21:50:27.322Z
Learning: Source excerpt:
# AGENTS.md — AI Agent Instructions for WaveHouse
## Operating Rules
1. **Validate locally before every push** — run `make ci` the documented way, here or on a configured host with `make ci-remote` ([§Running `make ci`](#running-make-ci-for-agents)). Don't use CI as your first feedback loop.
🪛 LanguageTool
AGENTS.md
[style] ~255-~255: Consider using the typographical ellipsis character here instead.
Context: ...e/gh pr new, any gh apiwrite to/repos/.../pulls//requested_reviewers`, or the ...
(ELLIPSIS)
CHANGELOG.md
[typographical] ~130-~130: Consider using an em dash and a white space in dialogues and enumerations.
Context: ...eviews -f event=APPROVEapproved a PR,gh api …/pulls -f head=… -f title=…opened a non-draft one,gh a...
(DASH_RULE)
[uncategorized] ~162-~162: The official name of this software platform is spelled with a capital “H”.
Context: ...ude/skills/pr-review-locally/SKILL.md, .github/prompts/docs-review.md, scripts/{skip...
(GITHUB)
[style] ~162-~162: This sentence is over 40 words long. Consider splitting it up, as shorter sentences make the text easier to read.
Context: ...thub.com//issues/659). review-marker.sh keyed a reviewer's marker to whatever HEAD the session's launch directory had when the reviewer stopped, so a commit made while a reviewer ran inherited its ship_it, and a review of a sibling worktree marked the launch directory's branch instead. Reviewers now end with `REVIEWED: ...
(TOO_LONG_SENTENCE)
[style] ~162-~162: This sentence is over 40 words long. Consider splitting it up, as shorter sentences make the text easier to read.
Context: ... the launch directory's branch instead. Reviewers now end with REVIEWED: <sha>, the commit they pinned before reading anything (an unambiguous prefix will do), and the hook writes the marker into each worktree on that commit, and only when that worktree's HEAD reflog shows it on the commit from the start of the review to its end (git reset or git stash on the same commit doesn't count as a move). When last_assistant_message has no ...
(TOO_LONG_SENTENCE)
[typographical] ~162-~162: The word ‘When’ starts a question. Add a question mark (“?”) at the end of the sentence.
Context: ...ockstands, whatever the hand-back said. Each decision goes totmp/review-marke...
(WRB_QUESTION_MARK)
[style] ~162-~162: ‘by mistake’ might be wordy. Consider a shorter alternative.
Context: ...s none, so commits made on local main by mistake and pushed on a new branch still need r...
(EN_WORDINESS_PREMIUM_BY_MISTAKE)
[style] ~162-~162: This word has been used in one of the immediately preceding sentences. Using a synonym could make your text more interesting to read, unless the repetition is intentional.
Context: ...fygainstest-review-gate(so it now needsjq`, listed in the development prerequ...
(EN_REPEATEDWORDS_NEED)
🔇 Additional comments (5)
.claude/hooks/agent-bash-gate.sh (1)
259-259: LGTM!.claude/hooks/review-gate.test.sh (1)
303-303: LGTM!AGENTS.md (1)
255-255: LGTM!CHANGELOG.md (1)
130-130: LGTM!Also applies to: 162-162
docs/src/content/docs/claude-code.md (1)
111-111: LGTM!
…-gate # Conflicts: # CHANGELOG.md
github-project-automation
Bot
moved this from In review
to Done
in WaveHouse Task Board
Replaces the agent PR gate's bash tokenizer with plain text checks over the command string.
.claude/hooks/agent-bash-gate.shgoes from 1,032 lines to 304, and its tests from 684 lines to 493.The gate exists to catch accidental breaches of the human-only PR actions and of the pre-push review rule. It doesn't try to stop deliberate evasion. Teaching it to read bash had become the larger problem: each review round found narrower forms it misread, harmless edits that mentioned a guarded command were blocked, and on macOS bash 3.2 a large command could outlast the hook's timeout and pass unchecked. It now catches the plain, ordinary forms. The
denyrules in.claude/settings.json, AGENTS.md and Claude Code's own permission checks cover the rest.Blocks
gh pr create/gh pr newwithout--draft, and a literal title the PR-title lint rejects;gh pr ready, andgh pr review --approve/--request-changes;gh apiforms of those, and of a merge or auto-merge;git pushthat isn't a plaingit [-C <worktree>] push …on its own (optionally with2>&1and one| filter);scripts/pre-push-reviewers.sh. That marker check (gate_commit) is unchanged.Accepted false positives. The text is searched, not parsed. So a command that only mentions a guarded name is blocked too: a
--bodynaming the ready-for-review mutation, a commit message containinggit push, or a GET ofrequested_reviewers. The workaround is to put the text in a file (--body-file,git commit -F <file>,gh api -F body=@file). Unusual forms aren't caught, for example a quoted-Cpath or a multi-lineghcommand. AGENTS.md and the Claude Code guide say only plain forms are checked and that every other form is still against the rules.This supersedes #810 and #812, which hardened the tokenizer this removes.
Verified
review-gate.test.shpasses under Homebrew bash 5.3 and macOS/bin/bash3.2. A 108 KB command gets a verdict in about 0.07 s.hook-commands.test.shpasses, and shellcheck is clean.make verifyandmake cipass.Closes #797
Closes #809
🤖 Generated with Claude Code