feat(guardrails): add CI gate, push block, and auto-format hooks#102
Conversation
- Block direct push to main/master/develop/dev protected branches - Advisory CI check reminder on push/PR create operations - Auto-format suggestion after every 3 source edits Ref #98 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
This PR doesn't fully meet our contributing guidelines and PR template. What needs to be fixed:
Please edit this PR description to address the above within 2 hours, or it will be automatically closed. If you believe this was flagged incorrectly, please let a maintainer know. |
There was a problem hiding this comment.
Pull request overview
Adds new guardrail hook behaviors intended to (1) block direct pushes to protected branches, (2) remind users to verify CI status on push/PR creation, and (3) nudge formatting after repeated edits—extending the existing guardrail enforcement and reminder system.
Changes:
- Block
git pushattempts targeting protected branches (main/master/develop/dev). - Add an advisory CI reminder on
git pushandgh pr create. - Add an auto-format reminder every 3 edits/writes.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (/\b(git\s+push|gh\s+pr\s+create)\b/i.test(cmd) && !/\b(main|master|develop|dev)\b/i.test(cmd)) { | ||
| try { | ||
| const checks = await git(input.worktree, ["rev-parse", "--abbrev-ref", "HEAD"]) | ||
| if (checks) { |
There was a problem hiding this comment.
git() always returns an object ({ stdout, stderr }), so if (checks) is always truthy here and the advisory will always be appended whenever the regex matches. Also the rev-parse --abbrev-ref HEAD output isn’t used, so this call doesn’t currently verify anything about CI or even that the branch is non-empty. Consider checking stdout.trim() (and/or git exit status) and using a more meaningful condition (e.g., only emit when in a git repo / on a branch), or remove the git call entirely if it’s only a reminder.
| if (checks) { | |
| const branch = typeof checks.stdout === "string" ? checks.stdout.trim() : "" | |
| if (branch && branch !== "HEAD") { |
| // CI status advisory on push/PR create | ||
| if (/\b(git\s+push|gh\s+pr\s+create)\b/i.test(cmd) && !/\b(main|master|develop|dev)\b/i.test(cmd)) { | ||
| try { | ||
| const checks = await git(input.worktree, ["rev-parse", "--abbrev-ref", "HEAD"]) | ||
| if (checks) { | ||
| out.output = (out.output || "") + "\n⚠️ Remember to verify CI status after push: `gh pr checks`" | ||
| } |
There was a problem hiding this comment.
PR title/issue mention a “CI gate” (blocking push based on CI), but this implementation is only an advisory reminder and doesn’t check gh pr checks or enforce anything. If the intent is gating, this should actively query CI status (and decide what failures should block) before allowing git push/merge; otherwise, consider adjusting the PR title/description to avoid implying an enforcement gate.
| // Auto-format reminder after 3+ source edits | ||
| if (nextEditCount >= 3 && nextEditCount % 3 === 0) { | ||
| out.output = (out.output || "") + "\n🎨 " + nextEditCount + " source edits — consider running formatter (`prettier --write`, `biome format`, `go fmt`)." |
There was a problem hiding this comment.
This reminder triggers based on total edit/write operations, not “source edits”: it doesn’t check code(file), so edits to docs/config will still emit “N source edits”. Either gate this with code(file) (consistent with the doc reminder above) or adjust the message/counting so it reflects what’s actually being tracked.
| // Auto-format reminder after 3+ source edits | |
| if (nextEditCount >= 3 && nextEditCount % 3 === 0) { | |
| out.output = (out.output || "") + "\n🎨 " + nextEditCount + " source edits — consider running formatter (`prettier --write`, `biome format`, `go fmt`)." | |
| // Auto-format reminder after 3+ edit/write operations | |
| if (nextEditCount >= 3 && nextEditCount % 3 === 0) { | |
| out.output = (out.output || "") + "\n🎨 " + nextEditCount + " edit/write operations — consider running formatter (`prettier --write`, `biome format`, `go fmt`)." |
| // Direct push to protected branches — always blocked | ||
| if (/\bgit\s+push\s+(?:origin\s+)?(main|master|develop|dev)\b/i.test(cmd)) { | ||
| throw new Error(text("direct push to protected branch blocked — use a PR workflow")) | ||
| } |
There was a problem hiding this comment.
The direct-push block regex only matches git push origin main-style commands. It won’t block common direct pushes like git push while on main, or refspec pushes like git push origin HEAD:main / :main. Consider resolving the current branch for bare git push (e.g., via git rev-parse --abbrev-ref HEAD) and matching refspec destinations (e.g., :<branch>, HEAD:<branch>, refs/heads/<branch>), so protected branches can’t be updated via alternate syntaxes.
| } | ||
| // Direct push to protected branches — always blocked | ||
| if (/\bgit\s+push\s+(?:origin\s+)?(main|master|develop|dev)\b/i.test(cmd)) { | ||
| throw new Error(text("direct push to protected branch blocked — use a PR workflow")) |
There was a problem hiding this comment.
Unlike other bash-command blocks in this hook (e.g., shell access and merge blocked), this git push block throws without calling mark(...). That means the guardrail state won’t record last_block/last_command/last_reason for this denial. Record the block details before throwing so auditing and /review context stays consistent.
| throw new Error(text("direct push to protected branch blocked — use a PR workflow")) | |
| const err = "direct push to protected branch blocked — use a PR workflow" | |
| await mark({ last_block: "bash", last_command: cmd, last_reason: err }) | |
| throw new Error(text(err)) |
| if (/\b(git\s+push|gh\s+pr\s+create)\b/i.test(cmd) && !/\b(main|master|develop|dev)\b/i.test(cmd)) { | ||
| try { | ||
| const checks = await git(input.worktree, ["rev-parse", "--abbrev-ref", "HEAD"]) | ||
| if (checks) { | ||
| out.output = (out.output || "") + "\n⚠️ Remember to verify CI status after push: `gh pr checks`" | ||
| } |
There was a problem hiding this comment.
In tool.execute.before, the out parameter is typed/used as { args: ... } (and the core trigger passes only { args }). Writing to out.output here is not supported and will either be a TypeScript error or simply never surface to the user. If you want to emit an advisory message, it likely needs to be done in tool.execute.after (where out.output exists) or via a chat/event hook that can add message parts.
- P1: detect plain git push on protected branch + refspec forms - P2: use exact match instead of word boundary to avoid dev-fix false positives - P3: move CI advisory from tool.execute.before to tool.execute.after Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
Closes #98
Test plan
🤖 Generated with Claude Code