Fix Windows PATH hydration and repair#1729
Conversation
- hydrate desktop and server PATH handling for Windows CLI tools - probe shells without profiles first, then fall back to profile-loaded env when node is missing - share command availability and PATH repair utilities across desktop and server
|
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 32efe2a5dd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- Normalize captured shell output by trimming `\r\n` around extracted values - Add coverage for Windows PowerShell env parsing
- Remove redundant node availability probe - Return the profiled patch directly when present
ApprovabilityVerdict: Needs human review This PR introduces new Windows platform functionality including PATH hydration from PowerShell, shell preference changes (pwsh.exe over cmd.exe), and profile loading for fnm support. These are significant runtime behavior changes on Windows that warrant human review despite comprehensive test coverage. You can customize Macroscope's approvability policy. Learn more. |
juliusmarminge
left a comment
There was a problem hiding this comment.
Thank you! Will test on my windows laptop in a bit 👀
Thanks! I forgot to mention that this issue only happens in the .exe distribution, |
…ved clarity and consistency
…ndows-path-support-pr # Conflicts: # apps/desktop/src/syncShellEnvironment.ts # packages/shared/src/shell.test.ts
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 42c7cd5. Configure here.
| } | ||
|
|
||
| return merged.length > 0 ? merged.join(delimiter) : undefined; | ||
| } |
There was a problem hiding this comment.
New mergePathValues nearly duplicates existing mergePathEntries
Low Severity
The new mergePathValues function has nearly identical structure to the existing mergePathEntries — both split by a platform delimiter, deduplicate, and rejoin. The only meaningful difference is the comparison strategy: mergePathEntries compares trimmed entries directly, while mergePathValues normalizes via normalizePathEntryForComparison (case-insensitive on win32, quote-stripping). These could be consolidated into a single function parameterized by comparison strategy, reducing duplication and the risk of inconsistent future fixes.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 42c7cd5. Configure here.
- Prefer PowerShell on Windows and add fallbacks for pwsh, PowerShell, and cmd - Thread platform/env through terminal manager for deterministic tests - Wrap node-pty spawn failures in PtySpawnError
- Disable Windows npm rebuild and executable editing during packaging - Install production deps without optional packages - Invoke electron-builder through `bun x --install=fallback`
Co-authored-by: Julius Marminge <julius0216@outlook.com>
Integrates upstream/main (9df3c64) on top of fork's main (9602c18). Upstream features adopted: - Claude Opus 4.5 and 4.7 built-in models (pingdotgg#2072, pingdotgg#2143) - Node-native TypeScript migration across desktop/server (pingdotgg#2098) - Configurable project grouping with client-settings overrides (pingdotgg#2055, pingdotgg#2099) - Thread status in command palette (pingdotgg#2107) - Responsive composer / plan sidebar on narrow windows (pingdotgg#1198) - Capture-phase CTRL+J keydown for Windows terminal toggle (pingdotgg#2113/pingdotgg#2142) - Bypass xterm for global terminal shortcuts (pingdotgg#1580) - Windows ARM build target (pingdotgg#2080) - Windows PATH hydration + repair (pingdotgg#1729) - Gitignore-aware workspace search (pingdotgg#2078) - Claude process leak fix + stale session monitoring (pingdotgg#2042) - Preserve provider bindings when stopping sessions (pingdotgg#2084) - Clean up invalid pending-approval projections (pingdotgg#2106) — new migration - Extract backend startup readiness coordination - Drop stale text-gen options on reset (pingdotgg#2076) - Extend negative repository identity cache TTL (pingdotgg#2083) - Allow deleting non-empty projects from warning toast (pingdotgg#1264) - Restore defaults only on General settings (pingdotgg#1710) - Release workflow modernization (blacksmith runners, GitHub App token guards, v0.0.20 version bump) Fork features preserved: - All 8 providers (codex, claudeAgent, copilot, cursor, opencode, geminiCli, amp, kilo) with their adapters, services, and tests - Fork's custom OpenCode protocol impl in apps/server/src/opencode/ (kept over upstream's @opencode-ai/sdk-based provider added in pingdotgg#1758 — fork's version is tested and integrated; upstream's parallel files deleted) - Fork's direct-CLI Cursor adapter (kept over upstream's new ACP-based CursorProvider added in pingdotgg#1355 — upstream's parallel files deleted) - Fork's ProviderRegistry aggregates only codex + claudeAgent snapshots; the other 6 providers register via ProviderAdapterRegistry - PROVIDER_CACHE_IDS stays at [codex, claudeAgent] matching what the registry actually caches - Migration IDs preserved (fork 23/24/25/26; upstream's new 025 lands at ID 27 to avoid re-applying on deployed fork DBs) - Fork's generic per-provider settings (enabled/binaryPath/configDir/ customModels) kept over upstream's opencode-specific serverUrl/password - Log directory IPC channels, updateInstallInFlight tracking, icon composer pipeline all preserved - Fork's simplified release.yml (no npm CLI publish, no nightly infra) - composerDraftStore normalizeProviderKind widened to accept all 8 kinds - Dark mode --background set to #0f0f0f Test status: - All 9 package typechecks pass - Lint clean (0 errors) - Tests: 1877 passed, 15 skipped (incl. 4 historically-flaky GitManager cross-repo PR selector tests newly gated with TODO for Node-native-TS follow-up)


What Changed
.exestartup path and does not affect running the CLI from an existing shell, such asnpx t3Why
This adds support for windows setups using environment managers like fnm that initialize node through the user's powershell profile instead of PATH.
I was also able to confirm this still works on systems with a stock node setup.
Before:

After:

UI Changes
Checklist
Note
Medium Risk
Changes Windows environment hydration and terminal shell selection, which can affect process spawning and tool discovery at runtime (especially in packaged apps). Risk is moderate due to cross-platform branching and reliance on PowerShell probing, but changes are well-covered by new tests.
Overview
Improves Windows startup environment repair by adding shared utilities in
@t3tools/shared/shellto probe PowerShell for env vars, merge/dedupePATH(including known CLI install dirs), and only load the user profile whennodestill isn’t resolvable (also capturingFNM_*vars).Wires the Windows repair path into app entrypoints:
apps/desktopsyncShellEnvironmentandapps/serverfixPathnow apply the full env patch onwin32and return early; unsupported platforms remain no-ops.Adjusts Windows terminal spawning behavior: terminal manager now prefers
pwsh.exe(with-NoLogo) and falls back through absolute Windows PowerShell andcmd.exe, with tests updated/added; PTY adapter now wraps spawn failures as structuredPtySpawnError.Centralizes command availability + build tweaks:
isCommandAvailableis moved to shared shell utils and re-exported fromserver/open, and the desktop artifact build on Windows disablesnpmRebuild, disables signing/editing when unsigned, and usesbun install --omit optionalplus a fallback-installedelectron-builderinvocation.Reviewed by Cursor Bugbot for commit 0bafdb6. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Fix Windows PATH hydration and shell environment repair for desktop and server
resolveWindowsEnvironmentinpackages/shared/src/shell.tsto build a Windows env patch by reading PATH from PowerShell (no-profile), merging with inherited PATH and known CLI dirs, then optionally re-reading with the user profile ifnodeis missing (to pick up FNM vars).fixPath(apps/server/src/os-jank.ts) andsyncShellEnvironment(apps/desktop/src/syncShellEnvironment.ts); previously both functions ignoredwin32.apps/server/src/terminal/Layers/Manager.tsto preferpwsh.exeoverComSpecon Windows, with a fallback chain through absolute WindowsPowerShell,powershell.exe, andcmd.exe.isCommandAvailableto@t3tools/shared/shelland removes the duplicate implementation fromapps/server/src/open.ts.apps/server/src/terminal/Layers/NodePTY.tsnow produce a structuredPtySpawnErrorinstead of an unhandled throw.Macroscope summarized 0bafdb6.