← Back to PRs

#15345: fix(daemon): doctor --fix pollutes service PATH with dirs that don't exist (#15316)

by yinghaosang open 2026-02-13 09:27 View on GitHub →
gateway stale size: S
## Summary `doctor --fix` writes nonexistent directories into the systemd unit's `Environment=PATH=...` line. On nvm setups without a `current` symlink, this includes `~/.nvm/current/bin` which doesn't exist — gateway starts fine (absolute node path in `ExecStart`), but spawned subprocesses can fail PATH resolution. Closes #15316 lobster-biscuit ## Root Cause `getMinimalServicePathParts` in `service-env.ts` collects candidate directories from `resolveLinuxUserBinDirs` (hardcoded paths like `${home}/.nvm/current/bin`) and dedupes them, but never checks whether they actually exist on disk. The full list gets written verbatim into the unit file. ## Changes - Before: all candidate dirs written to PATH regardless of existence - After: `fs.statSync` filters out dirs that don't exist before returning The filter sits in `getMinimalServicePathParts`, so both the service generation path (`buildServiceEnvironment`) and the audit path (`auditGatewayServicePath`) stay consistent. ## Tests - `service-env.test.ts` — added "excludes nonexistent directories from PATH (#15316)" test, fails before fix, passes after - Updated all existing tests to mock `fs.statSync` (the new filter would otherwise drop test-only paths) - All 116 daemon directory tests pass - `pnpm build && pnpm check` pass <!-- greptile_comment --> <h2>Greptile Overview</h2> <h3>Greptile Summary</h3> This PR updates the daemon’s minimal PATH generation to filter out non-existent directories when composing systemd/launchd service environments, preventing `doctor --fix` from writing dead entries like `~/.nvm/current/bin`. Core change is in `getMinimalServicePathParts`, which now `statSync`s each candidate dir and only keeps entries that exist and are directories. Tests in `service-env.test.ts` were updated to mock `fs.statSync` to keep them deterministic and to add coverage for excluding missing paths. One follow-up needed: an existing audit test (`service-audit.test.ts`) constructs a minimal PATH without mocking the filesystem, so it becomes environment-dependent and may no longer assert inclusion of user bin roots (e.g. `PNPM_HOME`). <h3>Confidence Score: 4/5</h3> - This PR is largely safe to merge, with one test needing adjustment to remain deterministic. - The production change is small and matches the stated root cause (filter nonexistent PATH entries). The main concern is that `service-audit.test.ts` now depends on the host filesystem due to the new filtering, reducing coverage and potentially causing CI variability. - src/daemon/service-audit.test.ts <sub>Last reviewed commit: 498edc2</sub> <!-- greptile_other_comments_section --> <!-- /greptile_comment -->

Most Similar PRs