Skip to content

fix(hooks): skip edit-log validation for in-progress merge commits#2100

Merged
carlos-alm merged 6 commits into
mainfrom
fix/guard-git-mergehead-skip
Jul 17, 2026
Merged

fix(hooks): skip edit-log validation for in-progress merge commits#2100
carlos-alm merged 6 commits into
mainfrom
fix/guard-git-mergehead-skip

Conversation

@carlos-alm

Copy link
Copy Markdown
Contributor

Summary

  • guard-git.sh's commit-time edit-log validation blocks commits whose staged files don't match this session's tracked edits — protection against cross-session interference.
  • During a merge (MERGE_HEAD present), git commit refuses a partial-pathspec commit, so the check's own suggested workaround (commit specific files) is structurally impossible.
  • A merge commit also stages every auto-merged file from the incoming branch, not just files this session edited via tool calls, so the check's premise doesn't hold during a merge anyway.
  • This skips the edit-log validation entirely when MERGE_HEAD exists, allowing legitimate merge commits through.

Test plan

  • Start a merge with conflicts, resolve them, run git commit with no pathspec — confirm it's no longer blocked by the edit-log check
  • Confirm the edit-log check still applies normally to non-merge commits

git commit refuses a partial-pathspec commit while MERGE_HEAD exists, so
the edit-log check's own suggested workaround (commit specific files) is
impossible during a merge. A merge commit also stages every auto-merged
file from the incoming branch, not just files this session edited via
tools, so the check's premise doesn't hold either. Skip validation when
MERGE_HEAD is present.
@greptile-apps

greptile-apps Bot commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes a false-positive block in guard-git.sh's edit-log validation that fired during in-progress merge commits. The fix detects MERGE_HEAD via git rev-parse --verify and wraps only the edit-log check in an if [ "$MERGE_IN_PROGRESS" = false ] guard, leaving any future commit-time checks outside that block unaffected.

  • MERGE_IN_PROGRESS is initialized to false before the worktree branch and set to true only when rev-parse --verify MERGE_HEAD resolves to a real object, preventing a crafted .git/MERGE_HEAD file from bypassing validation.
  • The previous review concern (an early exit 0 that would silently bypass future checks) has been addressed: the bypass is now scoped to just the edit-log validation if-block, not the entire script.

Confidence Score: 5/5

Safe to merge — the change is narrowly scoped to skip only the edit-log check during a merge commit, with no modifications to surrounding guards or push/branch validation logic.

The logic is correct: MERGE_IN_PROGRESS is initialized before the conditional branch so it is always defined, rev-parse --verify is a stronger test than a bare file-existence check, and the bypass is properly scoped to the edit-log if-block rather than the whole script. The earlier review concern about a top-level exit 0 has been fully addressed.

No files require special attention.

Important Files Changed

Filename Overview
.claude/hooks/guard-git.sh Adds MERGE_HEAD detection using rev-parse --verify and wraps only the edit-log validation block in a MERGE_IN_PROGRESS=false guard; previous exit 0 concern from review thread is resolved.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[git commit detected] --> B[detect_work_dir]
    B --> C{WORK_DIR set and exists?}
    C -- yes --> D[git -C WORK_DIR rev-parse --verify MERGE_HEAD]
    C -- no --> E[git rev-parse --verify MERGE_HEAD]
    D -- exit 0 --> F[MERGE_IN_PROGRESS=true]
    D -- exit non-0 --> G[MERGE_IN_PROGRESS=false]
    E -- exit 0 --> F
    E -- exit non-0 --> G
    F --> H{MERGE_IN_PROGRESS = false?}
    G --> H
    H -- no, merge in progress --> I[Skip edit-log check, allow commit through]
    H -- yes, normal commit --> J{LOG_FILE exists and non-empty?}
    J -- no --> K[exit 0 backward compat]
    J -- yes --> L{STAGED_FILES empty?}
    L -- yes --> M[exit 0 nothing staged]
    L -- no --> N[Compare staged files vs EDITED_FILES in log]
    N --> O{Unexpected files found?}
    O -- yes --> P[deny: BLOCKED cross-session files]
    O -- no --> Q[exit 0 commit allowed]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[git commit detected] --> B[detect_work_dir]
    B --> C{WORK_DIR set and exists?}
    C -- yes --> D[git -C WORK_DIR rev-parse --verify MERGE_HEAD]
    C -- no --> E[git rev-parse --verify MERGE_HEAD]
    D -- exit 0 --> F[MERGE_IN_PROGRESS=true]
    D -- exit non-0 --> G[MERGE_IN_PROGRESS=false]
    E -- exit 0 --> F
    E -- exit non-0 --> G
    F --> H{MERGE_IN_PROGRESS = false?}
    G --> H
    H -- no, merge in progress --> I[Skip edit-log check, allow commit through]
    H -- yes, normal commit --> J{LOG_FILE exists and non-empty?}
    J -- no --> K[exit 0 backward compat]
    J -- yes --> L{STAGED_FILES empty?}
    L -- yes --> M[exit 0 nothing staged]
    L -- no --> N[Compare staged files vs EDITED_FILES in log]
    N --> O{Unexpected files found?}
    O -- yes --> P[deny: BLOCKED cross-session files]
    O -- no --> Q[exit 0 commit allowed]
Loading

Reviews (5): Last reviewed commit: "Merge remote-tracking branch 'origin/fix..." | Re-trigger Greptile

Comment thread .claude/hooks/guard-git.sh Outdated
Comment on lines +216 to +218
if [ -n "$MERGE_HEAD_PATH" ] && [ -f "$MERGE_HEAD_PATH" ]; then
exit 0
fi

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 exit 0 skips any future commit checks added below this block

exit 0 terminates the entire script, so any commit-time validation added after the edit-log block in the future (lines 220–244) would also be silently bypassed during merge commits. Currently this is harmless because only the edit-log check remains, but it creates a latent footgun. Wrapping just the edit-log validation in its own if ! merge; then … fi guard, rather than exiting early, would make the bypass scope explicit and safe for future additions.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Fix in Claude Code

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch — reworked so the merge bypass only scopes the edit-log validation block instead of an early exit 0. The MERGE_IN_PROGRESS flag now gates an if [ "$MERGE_IN_PROGRESS" = false ]; then ... fi wrapper around just that check, so any commit-time validation added below it in the future will still run during merge commits. Pushed in a follow-up commit.

A bare file-existence check on MERGE_HEAD's path can be satisfied by any
stray or hand-crafted file at that location, letting it be faked to bypass
the edit-log validation added in the previous commit. Use git rev-parse
--verify instead, which requires MERGE_HEAD to resolve to an actual object
in this repo's object database.
carlos-alm and others added 4 commits July 16, 2026 02:31
Replace the early exit 0 for in-progress merges with an
if [ "$MERGE_IN_PROGRESS" = false ] wrapper around just the edit-log
validation, so any commit-time check added below it in the future
still runs during merge commits instead of being silently skipped.
…to fix/guard-git-mergehead-skip

Impact: 1 functions changed, 12 affected
@carlos-alm

Copy link
Copy Markdown
Contributor Author

@greptileai

@carlos-alm
carlos-alm merged commit cb88682 into main Jul 17, 2026
22 checks passed
@carlos-alm
carlos-alm deleted the fix/guard-git-mergehead-skip branch July 17, 2026 06:13
@github-actions github-actions Bot locked and limited conversation to collaborators Jul 17, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant