Skip to content

fix(ng-dev): disable git hooks when running git commands - #4006

Merged
alan-agius4 merged 2 commits into
angular:mainfrom
VenkatKwest:fix-ngdev-git-hooks
Sep 30, 2026
Merged

alan-agius4 merged 2 commits into
angular:mainfrom
VenkatKwest:fix-ngdev-git-hooks

Conversation

@VenkatKwest

@VenkatKwest VenkatKwest commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

ng-dev pr checkout and ng-dev pr rebase check out pull request content into the user's working
tree. Where husky is installed — any workspace after pnpm install, since package.json has
"prepare": "husky" — core.hooksPath is .husky/_, which lives inside the working tree and is
untracked upstream (husky writes .husky/_/.gitignore containing *). A pull request can therefore
add its own .husky/_/h, and checking that branch out replaces husky's generated wrapper, which git
then runs: post-checkout on both commands, plus pre-push on the rebase path.

mergePullRequest already sets HUSKY=0 for this reason, but that covers only the merge command and
relies on the wrapper being husky's own. runGraceful is a better place for it — it already injects
-c credential.helper= on every git invocation for a related reason, and config passed on the
command line cannot be replaced by checked out content.

ng-dev/utils/test/git-client.spec.ts uses a mock git script that shifts past the injected
arguments, so it moves from shift 2 to shift 4.

Verified against a948cdfd:

  • reproduction no longer fires the hook; ng-dev pr checkout otherwise behaves identically
  • ng-dev/utils/test/git-client.spec.ts — 14 specs, 0 failures
  • ng-dev/pr/merge/strategies/strategy.spec.ts + ng-dev/pr/merge/integration.spec.ts — 41 specs, 0 failures
  • tsc --noEmit -p tsconfig.json — pre-existing error count unchanged (1901 with and without)

Fixes #4005

🤖 Generated with Claude Code

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request configures Git to disable hooks during operations like checking out pull requests by setting core.hooksPath to devNull. The review feedback points out that using os.devNull will fail on Windows because Git's internal implementation strictly checks for the literal string '/dev/null'. It is recommended to use the literal string '/dev/null' directly and remove the unused devNull import from the os module.

Comment thread ng-dev/utils/git/git-client.ts Outdated
Comment thread ng-dev/utils/git/git-client.ts
@alan-agius4 alan-agius4 added the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label Sep 30, 2026

@alan-agius4 alan-agius4 left a comment

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.

The approach makes sense. Passing -c core.hooksPath=... globally in GitClient.runGraceful alongside -c credential.helper= ensures git hooks won't run during automated operations (checkout, rebase, etc.) and protects against untrusted hooks in PR branches.

Left a couple of comments below.

Comment thread ng-dev/utils/git/git-client.ts Outdated
Comment thread ng-dev/utils/test/git-client.spec.ts

@alan-agius4 alan-agius4 left a comment

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.

Please see the inline comments regarding using the literal '/dev/null' for cross-platform Git compatibility and adding a test to assert the injected flags.

@angular angular deleted a comment from google-cla Bot Sep 30, 2026
@alan-agius4 alan-agius4 added action: merge The PR is ready for merge by the caretaker and removed action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews labels Sep 30, 2026
VenkatKwest and others added 2 commits September 30, 2026 13:18
`pr checkout` and `pr rebase` check out pull request content into the working
tree. Where husky is installed `core.hooksPath` resolves to `.husky/_`, which
lives inside the working tree and is untracked upstream, so a pull request can
supply its own wrapper there and have it executed on checkout.

Pass `core.hooksPath` on the command line alongside the existing
`credential.helper` override, so hooks are disabled for every git invocation and
the setting cannot be replaced by checked out content. `HUSKY=0` covers only the
merge and release commands and relies on husky's own wrapper being present.

Fixes angular#4005
@alan-agius4
alan-agius4 merged commit 2a0ecc0 into angular:main Sep 30, 2026
17 checks passed
@alan-agius4

Copy link
Copy Markdown
Contributor

This PR was merged into the repository. The changes were merged into the following branches:

@VenkatKwest

Copy link
Copy Markdown
Contributor Author

Confirming identity for Google IssueTracker verification: report #559012933.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

action: merge The PR is ready for merge by the caretaker

Projects

None yet

Development

Successfully merging this pull request may close these issues.

angular/dev-infra — ng-dev pr checkout / pr rebase: git hooks supplied by the pull request are executed on checkout

2 participants