fix(kaos): enrich PATH from the user's login shell at startup#1339
Conversation
When kimi-code is launched from a context that skipped the user's shell profile (GUI launchers, non-login parent shells), process.env.PATH misses entries like /opt/homebrew/bin, so commands spawned by the Bash tool cannot find user-installed tools such as gh. LocalKaos.create() now probes the user's login shell once ($SHELL -l -c env, 5s timeout, memoised) and appends the missing PATH entries to process.env.PATH. Existing entries keep their order and priority; probe failures silently leave PATH untouched. Windows is skipped: the problem is specific to POSIX login-shell profiles.
🦋 Changeset detectedLatest commit: 4df7123 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
commit: |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 123d6f9f4d
ℹ️ 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".
| const shell = deps.env['SHELL']?.trim(); | ||
| if (shell === undefined || shell.length === 0) return undefined; |
There was a problem hiding this comment.
In launchd/daemon launches, $SHELL can be unset or blank (the repo already handles that case in packages/agent-core/src/services/terminal/terminalService.ts:267-270). This early return means those GUI/service startup paths never probe a login shell, so PATH remains the launchd/system default and the Bash tool still won't find user-installed tools such as Homebrew gh; please fall back to the OS account shell (for example os.userInfo().shell) before giving up.
Useful? React with 👍 / 👎.
launchd/daemon launches can leave $SHELL unset or blank — the very contexts whose PATH is impoverished — so the login-shell PATH probe would give up exactly where it matters most. Resolve the shell from the OS user database (os.userInfo().shell) before giving up; lookups that throw (uid without a database entry) or yield nologin shells degrade silently as before.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fcf381cedd
ℹ️ 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".
| currentPath: string | undefined, | ||
| loginShellPath: string, | ||
| ): string { | ||
| const merged = (currentPath ?? '').split(':').filter((entry) => entry.length > 0); |
There was a problem hiding this comment.
When the existing PATH contains an empty component such as :/usr/bin, /usr/bin:, or ::, POSIX command lookup treats that component as the current directory. This filter removes those entries before writing the merged value back to process.env.PATH, so the first LocalKaos.create() silently changes lookup semantics for all later Bash/exec calls instead of only appending login-shell entries; users who intentionally rely on the current directory in PATH lose it. Please preserve empty components from the current PATH while appending only the missing login-shell entries.
Useful? React with 👍 / 👎.
POSIX command lookup treats an empty PATH component (leading colon, trailing colon, or double colon) as the current directory. The merge previously filtered those out of the current PATH and rewrote the value even when nothing was appended, silently dropping cwd lookup for users who rely on it. Keep the current PATH string verbatim as the prefix, append only the missing login-shell entries, and skip the env write entirely when the login shell contributes nothing — an unset PATH stays unset, a set PATH is never rewritten. Empty login-shell components are still never imported.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b1358fbfab
ℹ️ 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".
| const seen = new Set(current.split(':').filter((entry) => entry.length > 0)); | ||
| const additions: string[] = []; | ||
| for (const entry of loginShellPath.split(':')) { | ||
| if (entry.length === 0 || seen.has(entry)) continue; |
There was a problem hiding this comment.
When the login-shell PATH contains . or another relative component such as bin and the current PATH does not, this merge appends it to the global process.env.PATH. LocalKaos later runs commands from arbitrary workspace directories, so a command name missing from earlier PATH entries can now resolve from the project cwd, importing the same cwd-dependent lookup that the empty-component guard is trying to avoid. Consider skipping ./relative entries here, or only importing absolute login-shell paths.
Useful? React with 👍 / 👎.
A `.` or relative component in the login-shell PATH is cwd-dependent lookup with another spelling, and LocalKaos runs commands from arbitrary workspace directories — importing one would let a command name resolve from an untrusted project cwd. Tighten the merge's skip condition from "empty" to "not absolute", which subsumes the empty-component check.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b7684c37ce
ℹ️ 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".
|
|
||
| // `env` prints the resolved environment in every shell dialect, unlike | ||
| // `echo $PATH`, which fish would join with spaces. | ||
| const stdout = await deps.execFileText(shell, ['-l', '-c', 'env'], LOGIN_SHELL_ENV_TIMEOUT_MS); |
There was a problem hiding this comment.
When Kimi is launched from a workspace with a cwd-dependent PATH component (PATH=:/usr/bin, PATH=.:..., etc.), the new startup probe runs the bare command env inside $SHELL -l -c ..., so the shell resolves env through that PATH before mergeLoginShellPath has a chance to filter relative login entries. That means an executable named env in the project can be run automatically during LocalKaos.create() and can return an arbitrary PATH; use an absolute system env path or another non-PATH lookup for the probe.
Useful? React with 👍 / 👎.
A bare `env` inside `$SHELL -l -c` resolves through the inherited PATH from the workspace cwd. If that PATH carries a cwd-dependent component (which the merge deliberately preserves), a repo-planted `env` binary would run automatically at session startup and could feed the probe an arbitrary PATH. /usr/bin/env is guaranteed on mainstream POSIX systems and also bypasses profile function shadowing.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4df71234c0
ℹ️ 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".
| // shadowing, and /usr/bin/env is guaranteed on every mainstream POSIX | ||
| // system (it is the canonical shebang interpreter path). | ||
| const stdout = await deps.execFileText( | ||
| shell, |
There was a problem hiding this comment.
When a launcher provides SHELL as a bare name or relative path (for example SHELL=bash) and the inherited PATH contains a cwd-dependent component such as . or an empty entry, this execFile call resolves the shell itself through that untrusted PATH. In a workspace with a checked-in executable named bash, LocalKaos.create() can run it automatically before the /usr/bin/env safeguard applies and then accept its forged PATH; please require an absolute shell path (or fall back to the account shell) before spawning.
Useful? React with 👍 / 👎.
1 participant
Problem
User report: the Bash tool cannot find locally installed tools like
gh.When kimi-code is launched from a context that skipped the user's shell profile (GUI launchers, non-login parent shells),
process.env.PATHmisses entries like/opt/homebrew/bin. The Bash tool spawns commands via a non-interactive, non-login shell that inheritsprocess.envdirectly, so every spawned command inherits the impoverished PATH.Fix
LocalKaos.create()now probes the user's login shell once and merges the result intoprocess.env.PATH:$SHELL -l -c envwith a 5s timeout and extracts itsPATH. Usingenv(rather thanecho $PATH) keeps the probe correct across shell dialects — fish would join$PATHwith spaces — and taking the lastPATH=line filters out profile stdout noise.$SHELL, spawn failure, timeout, or noPATHin the output leave PATH untouched.detectEnvironmentFromNode(), and runs concurrently with it, so repeatedcreate()calls never re-spawn the shell.Testing
packages/kaos/test/login-shell-path.test.ts: 12 unit tests over the pure probe/merge functions (injected deps, run on every platform) plus an end-to-end test that points$SHELLat a stub shell and assertsLocalKaos.create()appends the missing entry.test/cli/update/preflight.test.tsrollout-gating failures that reproduce identically onmainwithout this change.