Conversation
…oks with absolute paths to the entire binary Entire-Checkpoint: 43cdf4753c06
There was a problem hiding this comment.
Pull request overview
This PR addresses issue #489 by adding an entire enable flag to generate git hook scripts that invoke the entire binary via an absolute path (to support GUI git clients like Xcode that don’t inherit a full PATH).
Changes:
- Add
--absolute-git-hook-pathflag and persist it asabsolute_git_hook_pathin.entire/settings*.json. - Update hook installation/warning code paths to support an absolute-path command prefix (with safe shell quoting).
- Add unit tests for absolute-path hook installation and
shellQuote.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| cmd/entire/cli/strategy/hooks.go | Extend hook installation to support absolute binary paths; add shellQuote; load hook-related settings. |
| cmd/entire/cli/strategy/hooks_test.go | Update hook installer calls for new signature; add tests for absolute-path hooks and shell quoting. |
| cmd/entire/cli/strategy/hook_managers.go | Include absolute-path prefix in hook manager warnings. |
| cmd/entire/cli/strategy/hook_managers_test.go | Update tests for new hook manager warning signature. |
| cmd/entire/cli/strategy/common.go | Use persisted hook settings when ensuring hooks are installed. |
| cmd/entire/cli/setup.go | Add EnableOptions struct and --absolute-git-hook-path flag; plumb through enable flows. |
| cmd/entire/cli/setup_test.go | Update hook installer call signature. |
| cmd/entire/cli/settings/settings.go | Add AbsoluteGitHookPath setting and merge behavior for absolute_git_hook_path. |
| cmd/entire/cli/bench_enable_test.go | Update benchmark helper call to use EnableOptions. |
PR SummaryMedium Risk Overview Refactors enable/setup code to pass a single Written by Cursor Bugbot for commit 4a08b56. Configure here. |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
Comment @cursor review or bugbot run to trigger another review on this PR
Entire-Checkpoint: 4b25d19a41b7
toothbrush
left a comment
There was a problem hiding this comment.
I think this looks good! Only concern i have is the flag's name. The setting influences both Git hooks as well as agent hooks, so should we call it --absolute-hook-paths?
…k-path-option # Conflicts: # cmd/entire/cli/bench_enable_test.go # cmd/entire/cli/setup.go # cmd/entire/cli/setup_test.go # cmd/entire/cli/strategy/common.go # cmd/entire/cli/strategy/hook_managers.go # cmd/entire/cli/strategy/hook_managers_test.go # cmd/entire/cli/strategy/hooks.go # cmd/entire/cli/strategy/hooks_test.go Entire-Checkpoint: 1c7d96957309
|
@toothbrush no this is really only changing the git hooks. I felt for the agent hooks this doesn't make sense since relative paths are always better I think and the agent is started from an interactive shell so paths should be i.O. for these hooks. |
Ah yes, i looked again, carefully 👍
That surprises me, is that also true for e.g. Cursor IDE? Typically (in macOS at least) GUI apps have pretty bare-bones environments. |
|
@toothbrush yes, cursor IDE works, and if we run into an issue with another IDE I would change it only there. |
Addresses #489
This adds an optional setting to setup git hooks with absolute paths. I considered doing this always but I feel there is a risk path changes and then it just stops working when the most common use case is you have a full path available.
Entire-Checkpoint: 43cdf4753c06