Skip to content

.claude/settings.json: skip tests hook when no files changed#21640

Merged
MikeMcQuaid merged 4 commits intoHomebrew:mainfrom
zbeekman:fix-stop-hook-no-changed-files
Mar 1, 2026
Merged

.claude/settings.json: skip tests hook when no files changed#21640
MikeMcQuaid merged 4 commits intoHomebrew:mainfrom
zbeekman:fix-stop-hook-no-changed-files

Conversation

@zbeekman
Copy link
Contributor

@zbeekman zbeekman commented Feb 27, 2026

EDIT: I understand that this is a pretty minimal annoyance, but in the interest of token efficiency and maybe speed, it might be worth considering. It is a very small change. One down side is that the behavior of brew tests --changed is sort of encoded in the hook, so it's possible that it could become out of sync. If you think this is a dumb PR, I won't be offended, but I was sick of looking at the error message myself, so I thought I'd see if anyone else thought this was a good idea.

Problem

The Stop hook runs ./bin/brew tests --changed unconditionally. When a session ends without editing any files (e.g. read-only/analysis work), this exits non-zero:

Error: Invalid usage: No files have been changed from the `main` branch!

This surfaces as a spurious hook error at the end of every read-only session.

Fix

Guard the command so it only runs when git diff --name-only main...HEAD returns results. Test failures still propagate normally when the command does run — this is not a silent ignore.

changed=$(git diff --name-only main...HEAD); [ -z "$changed" ] || ./bin/brew tests --changed

Disclosure

This fix was identified and implemented with assistance from Claude Code (claude-sonnet-4-6) and reviewed/tested by me before submission.

`brew tests --changed` exits non-zero with "No files have been changed
from the `main` branch!" when a session ends without editing any files
(e.g. read-only/analysis sessions). Guard the Stop hook so it only
invokes `brew tests --changed` when there are actually changed files,
while still propagating test failures when it does run.
Copilot AI review requested due to automatic review settings February 27, 2026 18:47
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR modifies the Claude Stop hook to conditionally run ./bin/brew tests --changed only when there are changed files, preventing spurious errors in read-only sessions where no files have been modified.

Changes:

  • Added a guard condition using git diff to check for changed files before running the tests command

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

fix stupid oversight on my part

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@MikeMcQuaid
Copy link
Member

The Stop hook runs ./bin/brew tests --changed unconditionally. When a session ends without editing any files (e.g. read-only/analysis work), this exits non-zero:

Instead: I'd be 👍🏻 on just making this not an error (like it isn't for brew style --changed).

@carlocab's idea

Co-authored-by: Carlo Cabrera <github@carlo.cab>
@zbeekman
Copy link
Contributor Author

The Stop hook runs ./bin/brew tests --changed unconditionally. When a session ends without editing any files (e.g. read-only/analysis work), this exits non-zero:

Instead: I'd be 👍🏻 on just making this not an error (like it isn't for brew style --changed).

@MikeMcQuaid i.e. changing the return status of brew tests --changed when there are no changes? Want me to try my hand at that instead of the hook? That definitely feels like the correct approach to me now that I think about it.

@MikeMcQuaid
Copy link
Member

@MikeMcQuaid i.e. changing the return status of brew tests --changed when there are no changes? Want me to try my hand at that instead of the hook? That definitely feels like the correct approach to me now that I think about it.

@zbeekman Yes please!

@zbeekman zbeekman marked this pull request as draft February 28, 2026 21:37
@zbeekman
Copy link
Contributor Author

OK, I implemented the change. LMK if you want me to squash and/or rebase from main.

The only downside is ruby may be slower than git so, claude hook may be slightly slower. will test. But this way we won't get out of sync with code base.

LMK your thoughts @carlocab and team

@zbeekman zbeekman marked this pull request as ready for review February 28, 2026 22:08
@zbeekman
Copy link
Contributor Author

zbeekman commented Feb 28, 2026

time git diff --quiet --no-ext-diff main 0.038 s on my Intel i9 MBP

vs

time ./bin/brew tests --changed 2.211 s

Worth adding the hook gaurd back? And if so should we match behavior of brew tests --changed (compare vs main) or just git diff --quiet --noo-ext-diff without main?

I'm inclined to have claude get those ~ 2 seconds back

Copy link
Member

@MikeMcQuaid MikeMcQuaid left a comment

Choose a reason for hiding this comment

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

Thanks!

@MikeMcQuaid
Copy link
Member

Worth adding the hook gaurd back? And if so should we match behavior of brew tests --changed (compare vs main) or just git diff --quiet --noo-ext-diff without main?

I think it'd be nicer to e.g. add something to brew.sh to handle those --changed arguments in there to make the command itself a no-op if there's nothing from git diff rather than limiting it entirely to Claude Code configuration.

@MikeMcQuaid MikeMcQuaid added this pull request to the merge queue Mar 1, 2026
Merged via the queue into Homebrew:main with commit 3ad8be1 Mar 1, 2026
36 checks passed
@zbeekman zbeekman deleted the fix-stop-hook-no-changed-files branch March 1, 2026 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants