fix(server): Provider updates always fail on Windows#2781
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR propagates provider-specific environment variables into maintenance update commands, improves Windows compatibility for spawning update processes, and normalizes PATH handling on Windows to avoid casing-related issues.
Changes:
- Add optional
envto provider maintenance update actions and pass it through to spawned child processes (withshell: trueon Windows). - Update capability resolution to carry
envthrough to the resolved update command. - Normalize merged environment PATH casing on Windows and expand test coverage around env propagation.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| apps/server/src/provider/providerMaintenanceRunner.ts | Passes per-provider env into the spawned update process and enables Windows shell execution. |
| apps/server/src/provider/providerMaintenanceRunner.test.ts | Extends spawner mock to capture spawn options and asserts env/shell propagation. |
| apps/server/src/provider/providerMaintenance.ts | Adds env to maintenance command action and threads env through capability resolution. |
| apps/server/src/provider/providerMaintenance.test.ts | Adds test ensuring provider env is attached to resolved update commands. |
| apps/server/src/provider/ProviderInstanceEnvironment.ts | Normalizes PATH casing on Windows when merging env overrides. |
| apps/server/src/provider/ProviderInstanceEnvironment.test.ts | Adds Windows PATH-casing normalization test. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| .spawn( | ||
| ChildProcess.make(input.command, [...input.args], { | ||
| ...(input.env ? { env: input.env } : {}), | ||
| ...(process.platform === "win32" ? { shell: true } : {}), | ||
| }), | ||
| ) |
There was a problem hiding this comment.
This is the most minimal fix for update command resolution on Windows. This lets Windows resolve shims like npm.cmd, bun.cmd, and other package-manager/provider launchers.
The injection risk is limited, because command and args are generated internally in by providerMaintanance.ts, not accepted directly from user input. That said, the concern is valid as a hardening issue: using a shell broadly expands parsing/quoting semantics and increases risk if future update capabilities become externally influenced.
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.
Reviewed by Cursor Bugbot for commit 4c2ce06. Configure here.
ApprovabilityVerdict: Needs human review The fix adds You can customize Macroscope's approvability policy. Learn more. |

What Changed
Provider update commands now run with the provider instance’s resolved environment. On Windows, provider environments also canonicalize
PATHcasing and update commands run through the shell so.cmd/shim-based package manager commands resolve correctly.Tested on Windows 11 (a clean lab install + my local env) and Ubuntu 24.04.4 LTS. I don't have an OSX device to test.
Fixes #2765
Why
Windows can expose
PATHwith different casing, and provider-specific environments were used for detection but not passed through to the actual update command. That meant T3 Code could detect the right driver/update path but then fail to run the updater because the spawned process did not see the same environment or command resolution behavior.Checklist
No UI changes.
Note
Medium Risk
Changes provider update process spawning behavior on Windows by enabling
shell: true, which could affect command execution and quoting; non-Windows platforms are unchanged.Overview
Fixes provider updates on Windows by spawning update commands with
ChildProcess.make(..., { shell: true })whenprocess.platform === "win32", allowing.cmd/shim executables likenpmto resolve correctly.Updates the test harness to capture
ChildProcess.CommandOptions, adds awithProcessPlatformhelper to simulate Windows, and asserts the update command is invoked withshell: true.Reviewed by Cursor Bugbot for commit a08e55c. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
[!NOTE]
Fix provider update commands always failing on Windows by passing
shell: trueOn Windows,
npmis a batch script and cannot be spawned directly without a shell. providerMaintenanceRunner.ts now passes{ shell: true }asCommandOptionstoChildProcess.makewhenprocess.platform === 'win32', leaving other platforms unchanged. A new test uses awithProcessPlatformhelper to simulatewin32and asserts the shell option is set.Macroscope summarized a08e55c.