fix: prevent main-branch-guard from blocking non-repository files#36
fix: prevent main-branch-guard from blocking non-repository files#36JacobPEvans merged 1 commit intomainfrom
Conversation
Summary of ChangesHello @JacobPEvans, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a critical bug in the Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed everything up to 97ef327 in 11 seconds. Click for details.
- Reviewed
377lines of code in5files - Skipped
0files when reviewing. - Skipped posting
0draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
Workflow ID: wflow_ijaPlFYvEiPNbMXm
You can customize 
![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 effectively resolves an issue where the main-branch-guard was incorrectly blocking edits on files outside of a git repository. The replacement of the Python script with a more concise and context-aware shell script is a solid improvement. The accompanying documentation updates are clear and helpful. I have one suggestion to make the new script even more robust by checking if a file is tracked by git, rather than just existing within a git repository's directory, to better handle untracked files.
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug in the main-branch-guard plugin where files outside git repositories (like ~/.claude/plans/) were incorrectly blocked when the current working directory was on the main branch. The fix involves replacing a 110-line Python implementation with a 60-line shell script that checks repository context from the file's directory rather than the process's working directory.
Changes:
- Replaced Python implementation with shell script that runs git commands from the file's directory context
- Updated hooks.json to reference the new shell script
- Added guidance for hook implementation language selection to AGENTS.md
- Added comprehensive implementation summary documentation
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| git-guards/scripts/main-branch-guard.sh | New shell implementation that checks git repository status from file's directory instead of PWD |
| git-guards/scripts/main-branch-guard.py | Removed Python implementation that had the PWD bug |
| git-guards/hooks/hooks.json | Updated to reference .sh instead of .py script |
| IMPLEMENTATION_SUMMARY.md | Added detailed documentation of the problem, solution, and testing |
| AGENTS.md | Added language selection guidance for hook implementations |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Re: gemini-code-assist's feedback on git-guards/scripts/main-branch-guard.sh:22-27 Excellent suggestion! Implemented in commit fce4a3a. Changed from |
|
Re: copilot-pull-request-reviewer's feedback on AGENTS.md:147 Fixed in commit fce4a3a. Moved git-permission-guard from Shell examples to Python examples where it belongs (it's a 140-line .py file). |
|
Thank you for the update, @JacobPEvans! I appreciate you incorporating the suggestion regarding |
JacobPEvans
left a comment
JacobPEvans
left a comment
There was a problem hiding this comment.
All review feedback has been addressed:
- ✅ Thread 1: Improved file tracking with git ls-files (commit fce4a3a)
- ✅ Thread 2: Fixed documentation example (commit fce4a3a)
- ✅ Threads 3 & 4: Safe JSON construction with jq (commits fce4a3a, 67772df)
Please review the changes and mark threads as resolved if satisfied.
Replace Python implementation with shell script that correctly handles
files outside git repositories.
ROOT CAUSE
----------
The Python implementation ran branch checks from PWD without validating
whether the target file was actually in a git repository. This caused
legitimate operations on non-repo files (like ~/.claude/plans/) to be
blocked when the current directory was on the main branch.
SOLUTION
--------
Replaced 110-line Python script with ~60-line shell script that:
1. Extracts file path from JSON input (handles file_path and notebook_path)
2. Checks if file is in a git repository from file's directory context
3. Only applies branch checks if file is actually in a repository
4. Blocks edits to tracked files on main branch
Key fix:
# NEW: Check git repo from file's directory, not PWD
if ! (cd "$file_dir" && git rev-parse --git-dir >/dev/null 2>&1); then
exit 0 # Not in repo, allow operation
fi
TEST RESULTS
------------
All manual tests pass:
- ✅ Allows edits to non-repo files (even when CWD is main)
- ✅ Blocks tracked files in main worktree
- ✅ Allows all edits in feature branches
- ✅ Allows untracked files everywhere
FILES CHANGED
-------------
- git-guards/scripts/main-branch-guard.sh (NEW) - Shell implementation
- git-guards/scripts/main-branch-guard.py (REMOVED) - Python implementation
- git-guards/hooks/hooks.json - Updated script reference (.py → .sh)
- git-guards/README.md (NEW) - Plugin documentation
- content-guards/README.md - Simplified for AI-only repo
- AGENTS.md - Added hook language selection guidance
BENEFITS
--------
1. Fixes plan mode and other non-repo file operations
2. 45% code reduction (110 → 60 lines)
3. Simpler implementation (shell vs Python subprocess handling)
4. Same functionality preserved
5. Better guidance for future hook development
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
JacobPEvans
force-pushed
the
fix/main-branch-guard-over-blocking
branch
from
d610813 to
0d0ab9c
Compare
…ator
Simplified array expansion syntax in validate-markdown.sh to prevent
"unbound variable" error when running with set -euo pipefail.
ROOT CAUSE
----------
Line 111 used complex array expansion syntax ${config_flag[@]+"${config_flag[@]}"}
which triggered unbound variable errors with set -u, even though config_flag
was properly initialized on line 50.
SOLUTION
--------
Replaced with simpler "${config_flag[@]}" syntax which works correctly:
- Empty array expands to no arguments
- Non-empty array expands to its values
- No unbound variable error with set -u
This was a follow-up fix from PR #36.
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…ator (#39) * fix(content-guards): resolve unbound variable error in markdown validator Simplified array expansion syntax in validate-markdown.sh to prevent "unbound variable" error when running with set -euo pipefail. ROOT CAUSE ---------- Line 111 used complex array expansion syntax ${config_flag[@]+"${config_flag[@]}"} which triggered unbound variable errors with set -u, even though config_flag was properly initialized on line 50. SOLUTION -------- Replaced with simpler "${config_flag[@]}" syntax which works correctly: - Empty array expands to no arguments - Non-empty array expands to its values - No unbound variable error with set -u This was a follow-up fix from PR #36. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * fix(markdown-validator): add jq dependency check and simplify config discovery - Add jq availability check before JSON parsing to fail gracefully when unavailable - Replace 10-file config discovery loop with glob-based compgen check - Keep array expansion fix from PR #39 with safe expansion syntax - Reduce script from 137 to 130 lines while maintaining all functionality Fixes unbound variable crashes and improves robustness when tools are unavailable. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * fix: use nullglob for safer config detection Address glob injection and false-positive concerns from review: - Use nullglob to safely expand .markdownlint* pattern - Avoids compgen external command and glob-injection risks - Trusts markdownlint-cli2 native discovery for config validation - Maintains simplicity while addressing security concerns Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
1 participant
Summary
Root Cause
The Python implementation ran branch checks from PWD without verifying if the target file was actually in a git repository. This caused legitimate operations on non-repo files (like
~/.claude/plans/) to be blocked when the current directory was on the main branch.Solution
Replaced 110-line Python script with ~60-line shell script that:
Test Results
All manual tests pass:
Files Changed
git-guards/scripts/main-branch-guard.sh(new) - Shell implementationgit-guards/scripts/main-branch-guard.py(removed) - Python implementationgit-guards/hooks/hooks.json- Updated script referencegit-guards/README.md(new) - Plugin documentationcontent-guards/README.md- Simplified for AI-only repoAGENTS.md- Added language selection guidance🤖 Generated with Claude Code