fix: correct worktree path handling on Windows#1665
fix: correct worktree path handling on Windows#1665Valley-15 wants to merge 3 commits intogeneralaction:mainfrom
Conversation
|
@Valley-15 is attempting to deploy a commit to the General Action Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/services/ssh/SshService.ts`:
- Around line 371-372: The fallback path handling in executeCommandViaCli is
inconsistent with executeCommand: update executeCommandViaCli to normalize
Windows backslashes to forward slashes before composing innerCommand (i.e.,
compute safeCwd = cwd ? cwd.replace(/\\/g, '/') : undefined) and then build
innerCommand using quoteShellArg(safeCwd) like the pooled path code; do not
include the double-quote escaping used elsewhere. Locate executeCommandViaCli
and replace its current safeCwd/innerCommand logic accordingly so both
executeCommand and executeCommandViaCli normalize paths the same way.
- Line 371: The sanitization line creating safeCwd is incorrectly escaping
double quotes; remove the .replace(/"/g, '\\"') call so safeCwd only normalizes
backslashes and not insert literal backslashes before double quotes. Locate the
assignment to safeCwd in SshService (the const safeCwd = ... line) and change it
to only replace backslashes with forward slashes (and keep the existing ternary
handling), ensuring that downstream quoteShellArg receives the raw double-quote
characters unescaped.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: d94fc289-e798-4305-8d98-e239a7a45c08
📒 Files selected for processing (1)
src/main/services/ssh/SshService.ts
|
Thanks for the review! I’ve addressed the path normalization issues:
Please let me know if any further changes are needed. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 868 🏁 Script executed: Length of output: 18893
// executeCommandViaCli — around line ~475 — still has the old code:
const innerCommand = cwd ? `cd ${quoteShellArg(cwd)} && ${command}` : command;Since const safeCwd = cwd ? cwd.replace(/\\/g, '/') : undefined;
const innerCommand = safeCwd ? `cd ${quoteShellArg(safeCwd)} && ${command}` : command;Could you apply the same fix there? Once that's addressed, the path normalization will be truly consistent across both SSH execution paths. |
Summary
Fixes incorrect worktree path handling on Windows environments.
Previously, path normalization was inconsistent, which could lead to invalid or broken paths when executing commands via SSH or local shell. This change ensures paths are properly normalized before execution.
Fixes
Fixes #1641
Snapshot
N/A (backend/service-level fix, no UI changes)
Type of change
Mandatory Tasks
Checklist
Summary by CodeRabbit