Add an advisory agent hook reviewing the files a push adds - #2387
Merged
Conversation
A file can enter a branch without anyone deciding it should. It has happened twice recently: .git-exclude reached #2289 through a merge and survived until review, and a `git add -A` swept scheduling_problem.py into a commit that imported nothing from it. Both were additions. Modified files are nearly always deliberate; a brand-new file nobody mentioned is the one worth a second look. So the hook lists what a push adds relative to the merge-base, and names the ones that look unintended: dotfiles, .orig/.rej/.bak debris, scratch-looking names, build artifacts, and anything outside the usual directories. Advisory, exits 0. It cannot know intent -- it can only ask whether the addition was meant, which is a question an agent can answer and a gate cannot. The base is the tracked upstream when there is one, else origin/main, and it prints which base it used, so a wrong guess is visible rather than silent. That matters here: a stacked branch's base changes when its parent merges. Self-tested both directions. Against a synthetic commit adding .git-exclude, scratch/local_probe.py and a legitimate module, it flags the first two and leaves the module and its own files alone; with only a legitimate addition it lists it and flags nothing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X2jLjsQzwztDc7Nxz1ozmQ Signed-off-by: F.N. Claessen <claessen@seita.nl>
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.
Description
A file can enter a branch without anyone deciding it should. This has happened twice recently:
.git-exclude(containing a loneconftest.py) reached Balance internal commodity nodes (heat/steam networks) via first-class balance groups #2289 through a merge and survived until review caught it.git add -Asweptscheduling_problem.pyinto a commit that imported nothing from it — noticed only by auditing the commit afterwards.Both were additions. Modified files are nearly always deliberate; a brand-new file nobody mentioned is the one worth a second look.
.claude/hooks/push_file_review.py— lists what a push adds relative to the merge-base, and names the ones that look unintended.claude/settings.jsonas aPreToolUsehook ongit push, next to the existingtest-reminder.shWhat it flags
Dotfiles,
.orig/.rej/.bak/.swpdebris, scratch-looking names (local,scratch,probe,debug,tmp), build and runtime artifacts, and anything outsideflexmeasures/,documentation/,tests/,.github/,.claude/.Everything added is listed regardless; the flagged subset just gets called out.
Two deliberate choices
Advisory, exits 0. It cannot know intent — it can only ask whether an addition was meant. That is a question an agent can answer by looking, and a blocking gate cannot. Same reasoning as #2386.
It prints which base it used. The base is the tracked upstream when there is one, else
origin/main. Getting that wrong makes the added-file list either noisy or empty, and a stacked branch's base changes when its parent merges — #2289's went fromfeat/chptomainmid-flight. Printing it makes a wrong guess visible rather than silent.How to test
Proven capable of failing (per #2384). Against a synthetic commit adding
.git-exclude,scratch/local_probe.pyand a legitimate module:It flags the two junk files, and leaves the legitimate module and its own files alone. With only a legitimate addition, it lists it and flags nothing.
Limits
It can ask "did you mean to add this?" but not "does this belong in this commit" — it would have caught
.git-excludecleanly, and thescheduling_problem.pycase only partially.Related items
.git-excludesurfaced.