[iFwu]

ELI5

On Windows, Orca looks in a few well-known folders for Node when it was launched from the Start Menu and can't see your shell's PATH. For fnm it was looking in the macOS/Linux folder layout, so fnm users on Windows got "node not found" style failures. Orca now looks where fnm actually puts Node on Windows.

What Changed

• Before: On Windows, the Node fallback probed ~\.fnm\aliases\default\bin, a POSIX-only layout. fnm on Windows keeps its data under %FNM_DIR% or %APPDATA%\fnm, and node.exe sits directly in aliases\default (no bin). Agent CLIs installed via fnm's default Node (e.g. codex.cmd) stayed unresolved.
• After: New src/shared/fnm-default-alias-dirs.ts returns fnm's default alias dir per platform. On Windows: FNM_DIR\aliases\default if FNM_DIR is set, else %APPDATA%\fnm\aliases\default (falling back to <home>\AppData\Roaming when APPDATA is unset) plus the legacy ~\.fnm\aliases\default. macOS/Linux keep ~/.fnm/aliases/default/bin, unchanged.
• The shared resolver (node-cli-command-resolution.ts) uses those dirs. The packaged-startup PATH patch (configure-process.ts) and the CLI agent env (account.ts) append the Windows fnm default dir instead of prepending it, so it only fills in a missing Node and never shadows an inherited system, nvm-windows, or fnm use Node.

Why

fnm's default alias is a global fallback. Prepending it would override whichever Node the user picked for their session, so appending fixes the missing-Node case without hijacking an existing one. Putting the dir logic in one shared module keeps the resolver, startup PATH patch, and CLI env in agreement.

Linked Issue

Fixes #11356

Original fix by @iFwu in #11359; final version reworked during the bug bash.

Reported by @iFwu

Visual Proof

N/A: this changes Windows PATH/command resolution only. Nothing in the UI changes.

Testing

• Repro (on origin/main sources with the new tests): both new assertions fail. The Windows fnm %APPDATA% alias dir is missing from getVersionManagerBinPaths, and the fnm alias is never on the packaged PATH. resolveCliCommand('codex', { platform: 'win32' }) with codex.cmd + node.exe in <home>\AppData\Roaming\fnm\aliases\default returns the bare codex (unresolved).

• On this branch: src/main/codex-cli/command.test.ts and src/main/startup/configure-process.test.ts pass (82/82). An earlier run of the related resolver, startup, account, and hydrate-shell-path suites passed 190/190. The extra probes (APPDATA then legacy order, POSIX path unchanged and ignores FNM_DIR, codex.cmd resolves) pass.

• oxlint (anti-slop, code-quality native plugins), ast-grep, changed-code quality gate, and oxfmt --check are clean on all changed files.

• Platforms: simulated Windows (platform: 'win32') on macOS. POSIX behavior is unchanged. Local only: SSH/remote hosts don't inherit this PATH.

• I manually tested these changes locally

• Automated tests added/updated, or explained why not below

Review

Agent skill upstream boundary

• Not applicable, or this change copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

Ensure no issues in: Security, Cross-platform support (Linux, Windows, Mac), Remote SSH, Mobile, general backwards compatibility, performance

No new subprocesses or filesystem scans. The paths come only from FNM_DIR, APPDATA, or the home dir. Remote/SSH and mobile are unaffected.

Checklist

• This PR is small and focused
• I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
• Before/after screenshots or videos attached for UI changes, or N/A with reason
• Self-reviewed for correctness, security, and performance
• Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
• pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

[greptile-apps]

Greptile Summary

This PR fixes Windows fnm (Fast Node Manager) Node directory resolution in Orca's GUI process PATH. The original code used ~/.fnm/aliases/default/bin for all platforms, which is a POSIX-only path layout — on Windows fnm uses %FNM_DIR%\aliases\default (no \bin subdirectory). The fix correctly resolves the path via FNM_DIR env var → APPDATA\fnm → <home>\AppData\Roaming\fnm in priority order.

• Adds normalizeSingleWindowsPathEntry in src/shared/windows-path-entry.ts to sanitize env-sourced path values, blocking semicolons, extended namespaces, device paths with forward slashes, and named pipes before they can be injected into PATH.
• Adds windows-fnm-node-fallback.ts implementing a runtime cmd.exe-based probe that validates node.exe is reachable from the fnm directory before prepending it to the agent launch PATH, with a 3-second timeout and process-tree termination on expiry.
• Extends command.ts with getFnmDefaultDirectory (platform-branching, Windows-safe), and threads the process env into getBaseVersionManagerDirectories so the resolver can read FNM_DIR/APPDATA without touching process.env directly.

Confidence Score: 5/5

Safe to merge. The change is Windows-only in its behavioral surface — macOS and Linux paths are untouched — and the new Windows code is guarded behind both a platform check and a route-classification gate before any subprocess is spawned.

The core fix (removing the erroneous /bin suffix on the Windows fnm directory and adding the correct FNM_DIR/APPDATA priority chain) is straightforward and well-validated by the test suite. Path injection is prevented by normalizeSingleWindowsPathEntry before any candidate enters PATH. The probe-before-prepend design avoids modifying the spawn environment unless node is confirmed reachable. The two observations flagged are quality notes — an observable startup latency on every agent launch and an allowed .. traversal in validated paths — neither of which causes incorrect behaviour in the current call sites.

Files Needing Attention: windows-fnm-node-fallback.ts warrants a second look if agent-launch latency metrics are collected on Windows, since it always forks cmd.exe before the agent starts.

Important Files Changed

Filename	Overview
src/shared/windows-path-entry.ts	New shared utility for validating Windows path entries — rejects semicolons (PATH injection), extended namespace prefixes, device paths with forward slashes, named pipes, relative paths, and paths with leading/trailing whitespace. Regex coverage is thorough.
src/main/codex-cli/command.ts	Adds getFnmDefaultDirectory with correct platform branching (POSIX keeps /bin, Windows drops it and resolves via FNM_DIR → APPDATA → home fallback). Exports getWindowsFnmDefaultDirectory for the fallback module. Case-insensitive env lookup for Windows is correct.
src/main/pty/windows-fnm-node-fallback.ts	New module implementing probe-before-prepend Windows fnm fallback. Probe uses cmd.exe with 3 s timeout and process-tree termination. Correctly canonicalizes PATH key spelling, deduplicates entries, and validates the fnm directory through normalizeSingleWindowsPathEntry before using it. Added latency per agent launch is worth tracking.
src/main/ipc/pty.ts	Adds two withWindowsFnmNodeFallback call sites (fresh agent launch and resume launch). Both are correctly gated by shouldApplyWindowsFnmNodeFallback. The resume path has an additional nativeWindowsConptySpawn guard, which appears intentional.
src/main/providers/local-pty-provider.ts	Adds canonicalizeWindowsPathKey call after the full spawn env is built, ensuring PATH key casing is normalized to the inherited env's spelling. Captures inheritedEnv before merging so the base reference is stable.
src/main/daemon/pty-subprocess.ts	Parallel change to local-pty-provider.ts: adds canonicalizeWindowsPathKey before the PTY spawn in the daemon subprocess path, stabilizing PATH key casing.

_{Reviews (2): Last reviewed commit: "fix(windows): deduplicate PATH at PTY sp..." | Re-trigger Greptile}

[coderabbitai]

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Repository UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: f8f8dfa2-5af6-4fee-accb-1c4a6c59c40a

Commits

Reviewing files that changed from the base of the PR and between 5a1f353 and 01d2ee7.

Files selected for processing: 2

• src/main/codex-cli/command.test.ts
• src/main/startup/configure-process.test.ts

Files skipped from review as they are similar to previous changes: 2

• src/main/startup/configure-process.test.ts
• src/main/codex-cli/command.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 1 remain after this review.

Walkthrough

The change adds platform-aware fnm default-alias directory selection. On Windows, it uses FNM_DIR when set, or checks the APPDATA and home-directory locations. Version-manager resolution uses the selected directories. The account handler and packaged-process PATH logic append missing Windows fnm default-alias directories after existing PATH entries. Tests cover directory selection and PATH ordering.

Priority: Normal

Severity of issue fixed: Medium

Merge Risk: Minimal · up to 01d2e

The Windows fnm fallback appears mergeable after normal checks.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within surv…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Title check		The title clearly and concisely describes the primary Windows fnm default-alias resolution fix.
Description check		The description includes all required sections, links issue #11356, explains the user impact and implementation, documents testing and platform scope, and marks the relevant checklist items complete. …
Linked Issues check		Issue #11356 requires Windows fnm default-alias discovery through FNM_DIR, with %APPDATA%\\fnm or the home AppData\\Roaming\\fnm fallback. getFnmDefaultAliasDirectories implements these paths and…
Out of Scope Changes check		The changes remain within issue #11356. The shared resolver supports the required fnm paths. The PATH-order changes prevent fnm from overriding an inherited Node selection. The added tests validate th…

Full details: Description check

Explanation

The description includes all required sections, links issue #11356, explains the user impact and implementation, documents testing and platform scope, and marks the relevant checklist items complete. The visual proof requirement is addressed with N/A because there is no UI change.

Full details: Linked Issues check

Explanation

Issue #11356 requires Windows fnm default-alias discovery through FNM_DIR, with %APPDATA%\fnm or the home AppData\Roaming\fnm fallback. getFnmDefaultAliasDirectories implements these paths and keeps the POSIX ~/.fnm/aliases/default/bin path for non-Windows platforms. Command resolution, packaged startup PATH setup, and agent PATH setup use the shared directories. The tests cover the Windows fallback, FNM_DIR override, POSIX-path exclusion, and PATH append order.

Full details: Out of Scope Changes check

Explanation

The changes remain within issue #11356. The shared resolver supports the required fnm paths. The PATH-order changes prevent fnm from overriding an inherited Node selection. The added tests validate the required Windows behavior. The legacy Windows .fnm candidate supports backward compatibility for the same fallback feature.

Full details: Docstring Coverage

Explanation

Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 5 functions across 6 files.

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

src/main/codex-cli/command.test.ts (1)

316-353: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Cover FNM_DIR precedence over APPDATA.

These tests exercise each fallback independently, but not the required ordering. Add a case with both variables set and assert that only the FNM_DIR-derived directory is selected.

Suggested coverage

+  it('prefers FNM_DIR over APPDATA on Windows', () => {
+    const root = mkdtempSync(join(tmpdir(), 'orca-vm-paths-'))
+    const fnmDir = join(root, 'fnm-data')
+    const appData = join(root, 'Roaming')
+    const paths = getVersionManagerBinPaths({
+      platform: 'win32',
+      pathEnv: '',
+      homePath: root,
+      env: { FNM_DIR: fnmDir, APPDATA: appData }
+    })
+
+    expect(paths).toContain(join(fnmDir, 'aliases', 'default'))
+    expect(paths).not.toContain(join(appData, 'fnm', 'aliases', 'default'))
+  })

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 4749f929-9de8-4a2a-9874-f42622a308b3

📥 Commits

Reviewing files that changed from the base of the PR and between 4543bb6 and 74ba63355d5d0e5edc6a513cdc2950b2c6ce708e.

📒 Files selected for processing (2)

• src/main/codex-cli/command.test.ts
• src/main/codex-cli/command.ts