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 how provider update commands are spawned by conditionally enabling shell execution on Windows; this can alter command resolution/quoting behavior and may affect update execution across platforms.
Overview
Fixes Windows provider updates by spawning provider update commands with
shell: truewhenprocess.platform === "win32", improving resolution ofnpm/shim executables.Updates the test harness to capture
ChildProcesscommand options, adds a helper to temporarily overrideprocess.platform, and adds a dedicated test asserting Windows shell execution behavior.Reviewed by Cursor Bugbot for commit 1d3e368. Bugbot is set up for automated code reviews on this repo. Configure here.
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 1d3e368.