fix(server): discover editors on the hydrated PATH - #12501
Christos-Ioannou wants to merge 1 commit into
Conversation
Editor discovery read PATH through Effect Config. The default ConfigProvider copies process.env the first time it is used, which happens while ServerConfig resolves - before fixPath() hydrates PATH from the login shell. Discovery therefore kept looking at the bare launchd PATH (/usr/bin:/bin:/usr/sbin:/sbin) of a Dock/Finder launch, where only `open` resolves, so the Open-in picker offered Finder and nothing else. Launching the app from a terminal hid the bug because the inherited PATH was already complete. Read the launcher's env from HostProcessEnvironment instead, which is the object fixPath() mutates. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
ApprovabilityVerdict: Approved at Macroscope's review found this PR approvable — This is a small, focused server bug fix that makes existing editor discovery use the hydrated PATH without changing APIs, schemas, deployment, or product defaults. The accompanying regression test covers the startup-versus-hydration timing issue. You can add or adjust custom eligibility rules. Learn more. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe external launcher now reads environment variables through ChangesExternal launcher environment
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Suggested reviewers: Merge Risk: ⚪ Minimal · up to The launcher now observes the hydrated host PATH, preserving editor discovery for desktop launches. No actionable current-head risk remains. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
From pingdotgg#12501 by @Christos-Ioannou. Environment reads go through HostProcessEnvironment (hydrated by fixPath) for the fork's macOS bundle lookup and pingdotgg#12439's install-root lookup too. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
What Changed
externalLaunchernow reads its env (PATH/PATHEXTand the browser-launch vars) fromHostProcessEnvironmentinstead of EffectConfig. Tests provide the env throughHostProcessEnvironmentaccordingly, plus one regression test.Two files, server only:
apps/server/src/process/externalLauncher.tsand its test.Why
On macOS, when the desktop app is launched from the Dock/Finder, the Open in picker only offers Finder. Cursor, VS Code, Antigravity, IntelliJ etc. are all missing even though their CLIs are installed.
Root cause:
/usr/bin:/bin:/usr/sbin:/sbin.fixPath()correctly hydratesprocess.env.PATHfrom the login shell (provider child processes do get the full PATH).process.envthough - it readConfig.String("PATH"). The defaultConfigProviderisfromEnv(), which copies{ ...process.env }the first time it is used. That happens whileServerConfigresolves, which is beforefixPath()runs inmakeServerLayer. So discovery kept seeing the pre-hydration PATH forever.open(in/usr/bin) resolves, sofile-manageris the single available editor.The snapshot behaviour is easy to confirm in isolation:
Launching the same build from a terminal (
"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/Applications/T3 Code (Alpha).app/Contents/MacOS/T3 Code (Alpha)") makes every editor show up, because the inherited PATH is already complete and the stale snapshot happens to be right. That is also why this is easy to miss in dev.HostProcessEnvironmentis the objectfixPath()mutates, so reading it at call time is the smallest change that makes discovery and hydration agree. Empty values are still treated as absent, matching the previousConfigbehaviour.The new test fails on
mainand passes here. It asserts an exact editor list on purpose: onmainthe lookup falls through to the ambient process PATH and reports whatever editors the host machine has installed.Verified locally:
vp test run src/process/externalLauncher.test.ts(25 passed, 1 skipped),tsc --noEmitclean for the touched files,vp fmtandvp lintclean.Not in this PR, to keep it focused:
apps/server/src/provider/providerMaintenance.tshas the sameConfig.String("PATH")reader and is likely affected the same way. Happy to follow up if wanted.Checklist
🤖 Generated with Claude Code
Summary by CodeRabbit
PATH, become available after application startup.