feat(cli): add hook migration suggestions - #169
Conversation
|
Warning Review limit reached
More reviews will be available in 54 minutes and 9 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses rolling per-developer review limits. Reviews become available again as older review attempts age out of the rolling limit window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds a migrate-hooks subcommand
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 218e93d784
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| lines.push(format!("[[{}]]", phase.as_str())); | ||
| lines.push(format!( | ||
| "command = \"{}\"", | ||
| toml_escape_basic_string(&format!(".git/hooks/{}", phase.as_str())) |
There was a problem hiding this comment.
For an existing .git/hooks/pre-commit, this suggestion emits command = ".git/hooks/pre-commit", but git smee install --force writes the managed wrapper back to that same path (install_hook uses hooks_dir.join(hook_name)). After copying the generated TOML, the wrapper calls git-smee run pre-commit, which executes .git/hooks/pre-commit again and recurses; if the user moves/deletes the old hook to install without --force, the suggested command is missing. Generate a command pointing at a path outside the effective hooks directory, or make the required move explicit in the generated TOML.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in 71ee6a7: migrate-hooks now suggests .git-smee/legacy/<phase> outside the managed hooks directory and the generated TODO tells users to move .git/hooks/<phase> there before installing. Local gates passed: fmt, clippy, full workspace tests.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/git-smee-cli/src/main.rs`:
- Around line 266-278: The path pattern in the command field for unmanaged hooks
is generating `.git/hooks/<phase>` which is the exact file that git-smee
overwrites during installation, causing infinite recursion. Change the path
pattern from `.git/hooks/{}` to `.git-smee/legacy/{}` in the format string
within the loop over self.unmanaged_hooks. Additionally, update the TODO comment
to instruct users to move their legacy hook script to the new
`.git-smee/legacy/<phase>` location before running git smee install, instead of
the current message about preserving or refactoring the legacy hook script.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: ad239c02-bb76-4d7c-ae31-f33cacec75c6
📒 Files selected for processing (3)
README.mdcrates/git-smee-cli/src/main.rscrates/git-smee-cli/tests/cli_integration.rs
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 71ee6a7650
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| lines.push(format!( | ||
| "command = \"{}\"", | ||
| toml_escape_basic_string(&format!(".git-smee/legacy/{}", phase.as_str())) | ||
| )); |
There was a problem hiding this comment.
When migrating any hook that receives positional arguments (for example commit-msg, prepare-commit-msg, post-checkout, or pre-push), this suggestion only runs the legacy script path. PlatformCommandRunner::run invokes Unix commands as sh -c <command> -- <hook-args>, and sh -c '.git-smee/legacy/commit-msg' -- .git/COMMIT_EDITMSG does not pass those args through to the script; the legacy hook sees zero arguments unless the command explicitly forwards "$@". This changes the behavior of common legacy hooks that expect Git's original arguments, so the generated command should include arg forwarding (and an equivalent Windows strategy) for preserved scripts.
Useful? React with 👍 / 👎.
|
|
||
| fn run_migrate_hooks() -> Result<(), Box<dyn std::error::Error>> { | ||
| repository::ensure_in_repo_root()?; | ||
| let installer = installer::FileSystemHookInstaller::from_default()?; |
There was a problem hiding this comment.
In repositories where core.hooksPath points at a directory that does not exist yet, constructing FileSystemHookInstaller here creates that directory before the scan (from_path_with_force calls create_dir_all). That contradicts the new migrate-hooks output/README promise that the command is read-only/dry-run, and a user checking a custom hooks path can be left with filesystem changes even when there are no hooks to migrate. Resolve Git's hooks path directly for this read-only command instead of using the installer constructor.
Useful? React with 👍 / 👎.
| if !hook_path.is_file() { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
On Unix repositories, Git ignores hook files that are not executable (git-scm githooks documents: “Hooks that don't have the executable bit set are ignored”), but this scan treats any regular file named after a phase as an unmanaged hook. Copying the generated TOML can therefore start running a previously disabled pre-commit or commit-msg file after git smee install, changing repository behavior during migration. Check executable permissions on Unix, or report disabled hooks separately, before adding them to unmanaged_hooks.
Useful? React with 👍 / 👎.
2 participants
Summary
git smee migrate-hooksas a read-only migration helper for existing unmanaged Git hooks.Fixes #147
Test plan
cargo fmt --all -- --checkcargo clippy --workspace --all-targets --all-features -- -D warningscargo test --workspace --all-targets --all-featuresSummary by CodeRabbit
New Features
git smee migrate-hookscommand that scans for unmanaged Git hooks and generates TOML-formatted suggestions for migration review.Documentation
Tests