fix: skip pre-push checks on branch deletion#1004
Conversation
The pre-push hook runs lint/typecheck/test/build checks on every push, including `git push origin --delete <branch>`. Branch deletion does not push any code, so verification checks are unnecessary and block the delete operation. Detect deletion pushes by reading stdin (local sha is all zeros for deletes) and exit early. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Greptile SummaryThis PR fixes the pre-push hook to skip lint/typecheck/test/build checks when the push is a branch deletion by reading Git's stdin ( Key changes:
Issues found:
Confidence Score: 4/5Safe to merge after addressing the empty-stdin bypass, which can silently skip all push checks on non-git invocations. The core logic is correct for the normal git use-case. However, the scripts/codex-git-hooks/pre-push — review the zero-OID constant and the empty-stdin default. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[git push triggered] --> B{ECC_SKIP_GIT_HOOKS\nor ECC_SKIP_PREPUSH set?}
B -- yes --> Z[exit 0 — skip all checks]
B -- no --> C{.ecc-hooks-disable\nfile present?}
C -- yes --> Z
C -- no --> D{Inside a git\nwork tree?}
D -- no --> Z
D -- yes --> E[Read stdin lines:\nlocal_ref local_sha remote_ref remote_sha]
E --> F{local_sha == zero OID\nfor ALL refs?}
F -- yes: deletion-only push --> Z
F -- no: at least one normal ref --> G[Run verification checks:\nlint · typecheck · test · build]
G --> H{Any check\nfailed?}
H -- yes --> X[exit 1 — block push]
H -- no --> Y[exit 0 — allow push]
style Z fill:#d4edda,stroke:#28a745
style X fill:#f8d7da,stroke:#dc3545
style Y fill:#d4edda,stroke:#28a745
Reviews (1): Last reviewed commit: "fix: skip pre-push checks on branch dele..." | Re-trigger Greptile |
| is_delete_only=true | ||
| while read -r _local_ref local_sha _remote_ref _remote_sha; do | ||
| if [[ "$local_sha" != "0000000000000000000000000000000000000000" ]]; then | ||
| is_delete_only=false | ||
| break | ||
| fi | ||
| done | ||
| if [[ "$is_delete_only" == "true" ]]; then | ||
| exit 0 |
There was a problem hiding this comment.
The hardcoded zero OID (0000... with 40 hex chars) only matches SHA-1 object names. Repositories initialized with git init --object-format=sha256 use a 64-character zero OID, so this check will never match and branch deletions will incorrectly run all verification checks.
A more robust approach is to test whether the sha is all-zero characters of any length using a regex:
| is_delete_only=true | |
| while read -r _local_ref local_sha _remote_ref _remote_sha; do | |
| if [[ "$local_sha" != "0000000000000000000000000000000000000000" ]]; then | |
| is_delete_only=false | |
| break | |
| fi | |
| done | |
| if [[ "$is_delete_only" == "true" ]]; then | |
| exit 0 | |
| is_delete_only=true | |
| while read -r _local_ref local_sha _remote_ref _remote_sha; do | |
| if [[ "$local_sha" =~ ^0+$ ]]; then | |
| : # deletion ref, continue | |
| else | |
| is_delete_only=false | |
| break | |
| fi | |
| done | |
| if [[ "$is_delete_only" == "true" ]]; then | |
| exit 0 | |
| fi |
This handles both SHA-1 (40 zeros) and SHA-256 (64 zeros) repositories.
| is_delete_only=true | ||
| while read -r _local_ref local_sha _remote_ref _remote_sha; do | ||
| if [[ "$local_sha" != "0000000000000000000000000000000000000000" ]]; then | ||
| is_delete_only=false | ||
| break | ||
| fi | ||
| done | ||
| if [[ "$is_delete_only" == "true" ]]; then | ||
| exit 0 | ||
| fi |
There was a problem hiding this comment.
is_delete_only is initialized to true before the while read loop. If stdin is empty (no lines are produced), the loop body never executes and the flag stays true, causing the hook to exit 0 and skip every verification check on the push.
While git's spec says the pre-push hook is only called when at least one ref is being pushed, there are real-world edge cases where stdin can be empty:
- Hook invoked manually or via a CI harness that doesn't supply stdin
- Certain git GUI clients or wrappers that call the hook without piping stdin correctly
- Future git behaviour changes
The fix is to guard against the empty-stdin case by tracking whether any line was actually read:
| is_delete_only=true | |
| while read -r _local_ref local_sha _remote_ref _remote_sha; do | |
| if [[ "$local_sha" != "0000000000000000000000000000000000000000" ]]; then | |
| is_delete_only=false | |
| break | |
| fi | |
| done | |
| if [[ "$is_delete_only" == "true" ]]; then | |
| exit 0 | |
| fi | |
| is_delete_only=true | |
| got_any_ref=false | |
| while read -r _local_ref local_sha _remote_ref _remote_sha; do | |
| got_any_ref=true | |
| if [[ "$local_sha" != "0000000000000000000000000000000000000000" ]]; then | |
| is_delete_only=false | |
| break | |
| fi | |
| done | |
| if [[ "$got_any_ref" == "true" && "$is_delete_only" == "true" ]]; then | |
| exit 0 | |
| fi |
This ensures the early-exit path is only taken when git actually reported deletion refs, not when stdin happened to be empty.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
scripts/codex-git-hooks/pre-push (1)
19-31: Logic is correct; consider guarding against empty stdin.The deletion detection correctly identifies pushes where all refs have the zero OID. However, if stdin is unexpectedly empty (e.g., the wrapper fails to pipe input),
is_delete_onlyremainstrueand all verification checks are silently skipped.This is unlikely in normal git operations, but adding a guard ensures checks only skip when deletion refs are actually present.
🛡️ Suggested defensive improvement
is_delete_only=true +has_refs=false while read -r _local_ref local_sha _remote_ref _remote_sha; do + has_refs=true if [[ "$local_sha" != "0000000000000000000000000000000000000000" ]]; then is_delete_only=false break fi done -if [[ "$is_delete_only" == "true" ]]; then +if [[ "$has_refs" == "true" && "$is_delete_only" == "true" ]]; then exit 0 fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/codex-git-hooks/pre-push` around lines 19 - 31, The current deletion-detection loop using is_delete_only and the while read -r _local_ref local_sha _remote_ref _remote_sha may exit early when stdin is empty and incorrectly treat the push as a delete; to fix this, add a guard that tracks whether any refs were actually read (e.g., set a read_any flag inside the loop when it iterates) and only treat is_delete_only as authoritative if read_any is true; implement by initializing read_any=false before the while, set read_any=true inside the loop, and change the final condition to exit 0 only when is_delete_only == "true" AND read_any == "true".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@scripts/codex-git-hooks/pre-push`:
- Around line 19-31: The current deletion-detection loop using is_delete_only
and the while read -r _local_ref local_sha _remote_ref _remote_sha may exit
early when stdin is empty and incorrectly treat the push as a delete; to fix
this, add a guard that tracks whether any refs were actually read (e.g., set a
read_any flag inside the loop when it iterates) and only treat is_delete_only as
authoritative if read_any is true; implement by initializing read_any=false
before the while, set read_any=true inside the loop, and change the final
condition to exit 0 only when is_delete_only == "true" AND read_any == "true".
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9ff88f72-8975-4f38-b54e-fd510e6ef335
📒 Files selected for processing (1)
scripts/codex-git-hooks/pre-push
There was a problem hiding this comment.
1 issue found across 1 file
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="scripts/codex-git-hooks/pre-push">
<violation number="1" location="scripts/codex-git-hooks/pre-push:24">
P2: Delete-only detection is SHA-1-specific due to hardcoded 40-zero OID and can fail in non-SHA-1 repositories.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| # For deletions, the local sha is the zero OID. | ||
| is_delete_only=true | ||
| while read -r _local_ref local_sha _remote_ref _remote_sha; do | ||
| if [[ "$local_sha" != "0000000000000000000000000000000000000000" ]]; then |
There was a problem hiding this comment.
P2: Delete-only detection is SHA-1-specific due to hardcoded 40-zero OID and can fail in non-SHA-1 repositories.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/codex-git-hooks/pre-push, line 24:
<comment>Delete-only detection is SHA-1-specific due to hardcoded 40-zero OID and can fail in non-SHA-1 repositories.</comment>
<file context>
@@ -16,6 +16,20 @@ if ! git rev-parse --is-inside-work-tree >/dev/null 2>&1; then
+# For deletions, the local sha is the zero OID.
+is_delete_only=true
+while read -r _local_ref local_sha _remote_ref _remote_sha; do
+ if [[ "$local_sha" != "0000000000000000000000000000000000000000" ]]; then
+ is_delete_only=false
+ break
</file context>
| if [[ "$local_sha" != "0000000000000000000000000000000000000000" ]]; then | |
| if [[ ! "$local_sha" =~ ^0+$ ]]; then |
…-branch-deletion fix: skip pre-push checks on branch deletion
…-branch-deletion fix: skip pre-push checks on branch deletion
Summary
git push origin --delete <branch>operationsProblem
The pre-push hook runs lint/typecheck/test/build checks on every push, including branch deletions (
git push origin --delete <branch>). Since branch deletion does not push any code, running verification checks is unnecessary and blocks the operation, forcing users to use workarounds likegh apito delete remote branches.Solution
Read the pre-push hook's stdin to detect deletion pushes. Git provides push info in the format
<local ref> <local sha> <remote ref> <remote sha>. For deletions, the local sha is the zero OID (0000000000000000000000000000000000000000). When all pushed refs are deletions, skip checks and exit early.Test plan
git push origin --delete <branch>completes without running checksgit pushstill runs lint/typecheck/test/build as expectedgit push origin <branch> --delete <other-branch>(mixed) still runs checks🤖 Generated with Claude Code
Summary by cubic
Skip pre-push checks when deleting remote branches to unblock
git push origin --delete <branch>. Normal and mixed pushes still run checks.Written for commit 0c166e1. Summary will update on new commits.
Summary by CodeRabbit