fix(hooks): pass --allow-unknown-hook-name to git hook run#724
fix(hooks): pass --allow-unknown-hook-name to git hook run#724tgandrews wants to merge 1 commit intoaviator-co:masterfrom
Conversation
Git 2.54 tightened `git hook run` to reject non-native hook names
unless `--allow-unknown-hook-name` is passed. Since av invokes
`git hook run pre-av-pr` and `git hook run pre-av-sync` — both
custom names — every `av pr` and `av sync` now fails on git 2.54
with:
error: unknown hook event 'pre-av-pr';
use --allow-unknown-hook-name to allow non-native hook names
Add `--allow-unknown-hook-name` at both call sites. The flag is
accepted by `git hook run` on older versions as well, so no version
gating is needed.
Verified by building locally and running `av pr` against a repo on
git 2.54.0 — the hook invocation now succeeds and the command
proceeds normally.
Co-Authored-By: Claude Opus 4.7 <[email protected]>
Current Aviator status
This PR is currently in state
See the real-time status of this PR on the
Aviator webapp.
Use the Aviator Chrome Extension
to see the status of your PR within GitHub.
|
🔃 FlexReview StatusCommon Owner:
Review SLO: |
![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 adds the --allow-unknown-hook-name flag to Git hook commands in cmd/av/pr.go and cmd/av/sync.go. Feedback highlights that this flag was introduced in Git 2.44.0 and will cause errors on older versions, suggesting that version gating may be necessary if older Git versions are supported.
| "hook", | ||
| "run", | ||
| "--ignore-missing", | ||
| "--allow-unknown-hook-name", |
There was a problem hiding this comment.
The --allow-unknown-hook-name flag was introduced in Git 2.44.0. Using it will cause an unknown option error on older Git versions (2.36–2.43). While the PR description mentions that no version gating is needed, this is only true for Git 2.44 and later. If the project intends to support older Git versions, consider checking the Git version before passing this flag.
There was a problem hiding this comment.
This is correct, we get an error on older versions:
error: unknown option `allow-unknown-hook-name'
| "hook", | ||
| "run", | ||
| "--ignore-missing", | ||
| "--allow-unknown-hook-name", |
There was a problem hiding this comment.
The --allow-unknown-hook-name flag was introduced in Git 2.44.0. Using it will cause an unknown option error on older Git versions (2.36–2.43). While the PR description mentions that no version gating is needed, this is only true for Git 2.44 and later. If the project intends to support older Git versions, consider checking the Git version before passing this flag.
## Summary Git 2.54 tightened `git hook run` to reject non-native hook names unless `--allow-unknown-hook-name` is passed. Since av invokes `git hook run pre-av-pr` and `git hook run pre-av-sync` — both custom names — every `av pr` and `av sync` fails on git 2.54+ with: ``` error: unknown hook event 'pre-av-pr'; use --allow-unknown-hook-name to allow non-native hook names ``` This is closely related to #724, but that PR passes the flag unconditionally. `--allow-unknown-hook-name` was only introduced in git 2.44, so on older gits the flag itself would be rejected. This PR gates the flag on a parsed `git --version`. ## Changes - `internal/git/version.go`: adds `Repo.Version(ctx)` which parses `git --version` into `(major, minor)`, and `Repo.HookRunArgs(ctx, hookName)` which builds the `git hook run` arg list and conditionally appends `--allow-unknown-hook-name` when git >= 2.44. If version detection fails, the flag is still included so modern git keeps working. - `internal/git/version_test.go`: unit tests covering stable, patch, Apple-suffixed, newline-trailed, and malformed version strings. - `cmd/av/pr.go`: `runPRHook` uses `repo.HookRunArgs(ctx, "pre-av-pr")`. - `cmd/av/sync.go`: `preAvSyncHookModel.Init` uses `m.repo.HookRunArgs(ctx, "pre-av-sync")`. Behavior by git version: - `< 2.44` — flag omitted (avoids "unknown option" errors on old git). - `2.44`–`2.53` — flag included, accepted but not required. - `>= 2.54` — flag included and required; prevents the "unknown hook event" failure. ## Test plan - [x] `go build ./...` - [x] `go tool golangci-lint run ./internal/git/... ./cmd/av/...` - [x] `go test --vet=all ./internal/git/... ./cmd/av/...` - [x] CI green
|
Fixed with #727 |
Thanks for fixing it. I only just saw the review comments. |
2 participants
Summary
Git 2.54 tightened
git hook runto reject non-native hook names unless--allow-unknown-hook-nameis passed. Since av invokesgit hook run pre-av-prandgit hook run pre-av-sync— both custom names — everyav prandav synccurrently fails on git 2.54 with:…which av treats as a hook failure, aborting the command before any push or PR creation happens (even when the user has no such hook configured, because
--ignore-missingno longer suppresses the unknown-name error).This PR adds
--allow-unknown-hook-nameat the two call sites:cmd/av/pr.go—runPRHookinvokingpre-av-prcmd/av/sync.go—preAvSyncHookModel.Initinvokingpre-av-syncThe flag is also accepted by
git hook runon pre-2.54 versions, so no version gating is needed.Reproduction (before the fix)
Test plan
go build ./...go tool golangci-lint rungo test ./cmd/av/...av prnow passes the hook stage and proceeds to push / PR creation as expected (debug log showsgit [hook run --ignore-missing --allow-unknown-hook-name pre-av-pr]succeeding).🤖 Generated with Claude Code