fix(windows): restore shell environment variables - #588
Conversation
|
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 (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughWindows shell environment repair is centralized in an exported helper, applied during shell spawning, and covered by tests for repaired, default, and non-Windows environments. ChangesWindows shell environment repair
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant executeCommand
participant getRepairedWindowsShellEnvironment
participant SpawnedShell
executeCommand->>getRepairedWindowsShellEnvironment: build repaired shell environment
getRepairedWindowsShellEnvironment->>getRepairedWindowsShellEnvironment: repair PATHEXT and Windows variables
getRepairedWindowsShellEnvironment-->>executeCommand: return environment copy
executeCommand->>SpawnedShell: spawn with repaired environment
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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: 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 `@src/terminal-manager.ts`:
- Around line 49-60: Update getRepairedWindowsShellEnvironment() to resolve
WINDIR, SystemRoot, and PATHEXT from environment using case-insensitive key
matching before repairing them. Use those inherited values when present, then
assign the canonical keys and remove or normalize duplicate case variants so the
returned environment contains no conflicting duplicates.
🪄 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: a503f0ff-18d2-42bb-bd98-f461185f6c72
📒 Files selected for processing (2)
src/terminal-manager.tstest/test-windows-shell-environment.js
Summary
WINDIRandSystemRootfor Windows child shells when the host process supplies an empty or missing valueFixes #480
Root cause
TerminalManager.executeCommand()copied the inherited process environment directly into every child shell. Under the MSIX-packaged Claude Desktop,WINDIRcan be empty, which prevents PowerShell from loading CIM/CDXML modules even after PATHEXT has been repaired.Validation
npm run buildnode test/test-windows-shell-environment.jsgit diff --checkScope
This does not alter command policy or process execution. On Windows it preserves a non-empty configured
WINDIR, otherwise falls back toSystemRootand only thenC:\Windows; non-Windows environments are unchanged.Summary by CodeRabbit
WINDIR/SystemRootwith sensible defaults.PATHEXTbehavior unchanged on non-Windows systems.