fix: resolve hook paths for worktrees and restore env var#1036
fix: resolve hook paths for worktrees and restore env var#1036
Conversation
- Resolve .cs file paths with Join-Path $repoRoot in Invoke-PreCommit.ps1 so dotnet format --include works correctly in git worktrees - Save and restore DOTNET_ROLL_FORWARD in Invoke-PrePush.ps1 using a finally block to prevent environment variable leakage Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses two specific issues within the Git hooks. It corrects the path resolution for Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis pull request fixes two git hook bugs: the pre-commit hook now resolves C# file paths to absolute paths before passing them to dotnet format, and the pre-push hook preserves and restores the DOTNET_ROLL_FORWARD environment variable to prevent unintended global process state changes. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 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)
Comment |
|
|
Overall Grade |
Security Reliability Complexity Hygiene |
Code Review Summary
| Analyzer | Status | Updated (UTC) | Details |
|---|---|---|---|
| C# | Mar 7, 2026 10:56p.m. | Review ↗ |
There was a problem hiding this comment.
Code Review
This pull request introduces two fixes for the git hooks. The first fix correctly resolves file paths for dotnet format in Invoke-PreCommit.ps1, which is important for git worktree support. The second fix in Invoke-PrePush.ps1 ensures that the DOTNET_ROLL_FORWARD environment variable is properly restored after the script runs. While the implementation is correct, I've provided a suggestion to simplify the logic for restoring the environment variable, making it more concise and idiomatic to PowerShell.
Replace if/else block with single assignment since PowerShell removes an environment variable when assigned $null. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
c84b2a7
## Summary Fixes #996 - **Invoke-PreCommit.ps1**: Resolve `.cs` file paths using `Join-Path $repoRoot $_` before passing to `dotnet format --include`. Every other file type in the script already uses this pattern. Without it, `dotnet format` receives repo-relative paths that fail to resolve in git worktrees where `$PWD` differs from `$repoRoot`. - **Invoke-PrePush.ps1**: Save and restore `$env:DOTNET_ROLL_FORWARD` using a `finally` block. Prevents environment variable leakage to subprocesses spawned after the hook logic completes. Bug 2 from the issue (unquoted paths in `Invoke-AutoFix`) was evaluated and found to be a non-issue. PowerShell passes array elements as discrete arguments to native commands. The `--` separator is already present on all relevant `git diff` and `git add` calls. No change needed. ## Test plan - [ ] Verify `pwsh` parses both scripts without errors (confirmed via `[Parser]::ParseFile`) - [ ] `dotnet build Moq.Analyzers.sln` succeeds (confirmed, 0 warnings, 0 errors) - [ ] Run pre-commit hook from a git worktree with staged `.cs` files and confirm `dotnet format --include` resolves paths correctly 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Improved the C# code formatting process during the pre-commit phase by updating path handling to use absolute file paths for enhanced reliability when processing staged files. * Enhanced build and test environment variable management to properly preserve original settings and restore them after operations complete, reducing potential side effects. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Richard Murillo <rjmurillo@users.noreply.github.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Summary
Fixes #996
.csfile paths usingJoin-Path $repoRoot $_before passing todotnet format --include. Every other file type in the script already uses this pattern. Without it,dotnet formatreceives repo-relative paths that fail to resolve in git worktrees where$PWDdiffers from$repoRoot.$env:DOTNET_ROLL_FORWARDusing afinallyblock. Prevents environment variable leakage to subprocesses spawned after the hook logic completes.Bug 2 from the issue (unquoted paths in
Invoke-AutoFix) was evaluated and found to be a non-issue. PowerShell passes array elements as discrete arguments to native commands. The--separator is already present on all relevantgit diffandgit addcalls. No change needed.Test plan
pwshparses both scripts without errors (confirmed via[Parser]::ParseFile)dotnet build Moq.Analyzers.slnsucceeds (confirmed, 0 warnings, 0 errors).csfiles and confirmdotnet format --includeresolves paths correctly🤖 Generated with Claude Code
Summary by CodeRabbit