feat: add critical-rules, PR reference expansion, and Claude Code hook recipes#23
Merged
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces critical Git workflow rules and provides Claude Code hook recipes to enforce them. Key additions include a non-negotiable "no squash" policy to preserve atomic commits and a requirement for SHA citations when resolving review threads. Feedback focuses on improving the robustness of the provided shell-based hooks, specifically regarding PR number extraction, path matching for relative files, and ensuring MultiEdit operations are correctly handled during auto-linting.
There was a problem hiding this comment.
Pull request overview
This PR codifies stricter PR/merge workflow guidance for the git-workflow skill by adding “non-negotiable” rules, expanding PR workflow references (atomic commits + review-thread resolution discipline), and providing Claude Code settings.json hook recipes to enforce these rules at tool-invocation time.
Changes:
- Adds a “Critical Rules (Non-Negotiable)” section to the skill and links to new/expanded reference material.
- Expands PR workflow documentation with atomic-commit defaults and a SHA-cited review-thread resolution pattern (plus GitHub-side thread-state verification).
- Introduces a new reference doc with Claude Code hook recipes (merge gating, cache-path write denial, squash warning, auto-lint, and a “verified claim” sentinel).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
skills/git-workflow/SKILL.md |
Adds critical rules and links to workflow/hook references. |
skills/git-workflow/references/pull-request-workflow.md |
Adds atomic-commit default guidance and a SHA-cited thread-resolution workflow with GitHub verification. |
skills/git-workflow/references/claude-code-hooks.md |
New hook recipe collection for enforcing workflow rules inside Claude Code. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Adds a concise 'Critical Rules (Non-Negotiable)' section to SKILL.md: - No direct push to main; no merge before threads resolved - No squash unless asked (atomic commits default) - No unverified success claims (paste command output) - No edits to installed skill/plugin cache paths - Force-push only with --force-with-lease Adds to references/pull-request-workflow.md: - 'Atomic Commits (Default — No Squash Unless Asked)' with strategy order - 'Review Thread Resolution (SHA Citation Required)' with banned lazy replies and GitHub-side verification pattern Codifies recurring high-cost friction patterns as non-negotiable rules. Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
Adds references/claude-code-hooks.md with settings.json hook recipes that enforce the critical-rules at tool-invocation time: - Recipe 1: Block gh pr merge when review threads are unresolved or mergeState is not CLEAN (denies via PreToolUse, surfaces reason in permission prompt) - Recipe 2: Reject Write/Edit/MultiEdit to installed skill/plugin cache paths and .bare/ directories - Recipe 3: Warn (not block) on --squash merge to allow squash-policy repos while prompting confirmation - Recipe 4: Auto-lint Go files via golangci-lint on PostToolUse (and template for PHP/JS equivalents) - Recipe 5: Stop-hook sentinel for 'verified/tested' claims without backing tool output (soft guardrail) Also documents scope/file-location table, deployment caveats (settings watcher only picks up .claude/ that existed at session start), and anti-patterns for writing hooks. Provides the hook-recipe content that a separate 'update-config' skill would have provided — update-config is a built-in Anthropic skill with no editable source, so operational hook recipes live here where the merge-gate rules they enforce are already documented. Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
CybotTM
force-pushed
the
feat/critical-rules-and-hooks
branch
from
e9e5f8b to
8cf0c98
Compare
Contributor
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
…KILL.md Reviewer-identified issues in Recipe 1: - Inline awk regex '/gh pr merge[[:space:]]+([0-9]+)/' only matched 'gh pr merge 123' — missed '--auto 123', full PR URL, and owner/repo#N forms (all valid for gh pr merge). Bypass = silent merge through the gate. - Large inline JSON string with nested escaping was unreadable and hard to test or evolve. Fix: move logic to ~/.claude/hooks/merge-gate.sh. The script parses all three PR-reference forms gh pr merge accepts (plain number via positional arg after optional flags, full URL, owner/repo#N), fetches state via gh pr view, and emits a PreToolUse deny JSON via jq -cn (safer than string concatenation). If parsing fails, the hook allows the call rather than producing false-positive denies. Also condenses Critical Release Rules prose to keep SKILL.md under the 500-word validator cap after adding the script body. Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
Member
Author
Review responseThanks for the review. Pushed fix commits addressing the raised issues. Summary commit: What was addressed: merge-gate regex now handles --auto/URL/owner#N; script extracted to ~/.claude/hooks/merge-gate.sh with safe JSON via jq -cn. Please re-review. If any thread still needs work, point at the specific file/line and I'll iterate. |
CybotTM
added a commit
that referenced
this pull request
Apr 18, 2026
SKILL.md Critical Rule 2 tells readers to 'run the merge gate in references/pull-request-workflow.md' but the reference had no section literally titled 'Merge Gate' — it was called 'Merge Requirements Checklist'. Reviewer flagged the inconsistency on PR #23. Renames the section to 'Merge Gate', retitles the inline check to 'Merge-Gate Command', adds reviewThreads to the gh pr view query and the isResolved-all-true criterion (making the gate behavior match the claude-code-hooks merge-gate.sh script), and cross-links that script for automated enforcement. Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds non-negotiable workflow rules and operational hook recipes derived from recurring high-cost friction patterns (see also sibling PRs across the skill-repo fleet).
Changes
skills/git-workflow/SKILL.mdskills/git-workflow/references/pull-request-workflow.mdskills/git-workflow/references/claude-code-hooks.md(new)Why
These rules codify the recurring process violations that have caused force-pushes, follow-up PRs, manual cache restorations, and untested-fix breakage in prior sessions. Making them explicit at the skill level — rather than re-teaching per project — closes the loop.
Test plan