diff --git a/.changeset/fix-login-shell-path.md b/.changeset/fix-login-shell-path.md new file mode 100644 index 000000000..80695da31 --- /dev/null +++ b/.changeset/fix-login-shell-path.md @@ -0,0 +1,5 @@ +--- +"@moonshot-ai/kimi-code": patch +--- + +Enrich PATH from the user's login shell at startup, so shell commands find user-installed tools (e.g. Homebrew's `gh`) even when kimi-code was launched without the full profile PATH. diff --git a/packages/kaos/src/environment.ts b/packages/kaos/src/environment.ts index 9b08baff2..0fc1b3f84 100644 --- a/packages/kaos/src/environment.ts +++ b/packages/kaos/src/environment.ts @@ -284,7 +284,7 @@ async function findExecutablesOnPath( return platform === 'win32' ? dedupeWindowsPaths(paths) : paths; } -async function execFileText( +export async function execFileText( file: string, args: readonly string[], timeoutMs: number, diff --git a/packages/kaos/src/local.ts b/packages/kaos/src/local.ts index e0cc17b19..92ff99b67 100644 --- a/packages/kaos/src/local.ts +++ b/packages/kaos/src/local.ts @@ -18,6 +18,7 @@ import { detectEnvironmentFromNode, type Environment } from './environment'; import { KaosFileExistsError } from './errors'; import { BufferedReadable, decodeTextWithErrors, globPatternToRegex } from './internal'; import type { Kaos } from './kaos'; +import { applyLoginShellPathFromNode } from './login-shell-path'; import type { KaosProcess } from './process'; import type { StatResult } from './types'; @@ -212,7 +213,11 @@ export class LocalKaos implements Kaos { * without polluting one another. */ static async create(): Promise { - const osEnv = await detectEnvironmentFromNode(); + // Enrich process.env.PATH from the user's login shell so spawned + // commands find user-installed tools (e.g. Homebrew's gh) even when + // kimi-code itself was launched without the full profile PATH. Both + // probes are memoised, independent, and run concurrently. + const [osEnv] = await Promise.all([detectEnvironmentFromNode(), applyLoginShellPathFromNode()]); return new LocalKaos(osEnv); } diff --git a/packages/kaos/src/login-shell-path.ts b/packages/kaos/src/login-shell-path.ts new file mode 100644 index 000000000..62f51e573 --- /dev/null +++ b/packages/kaos/src/login-shell-path.ts @@ -0,0 +1,162 @@ +/** + * Login-shell PATH probe — enrich `process.env.PATH` with entries from the + * user's login shell. + * + * 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 can't find tools the user has in their interactive shell (e.g. + * `gh`). We run the user's login shell once (`$SHELL -l -c /usr/bin/env`), + * extract its PATH, and append the entries the current PATH lacks. Existing + * entries keep their order and priority; failures (no resolvable shell, + * hung or broken profile) silently leave PATH untouched. + * + * launchd/daemon launches can leave `$SHELL` unset or blank (see + * `defaultShell()` in agent-core's terminalService for the same case), so + * the probe falls back to the OS account's login shell from the user + * database before giving up. + * + * Like `detectEnvironment`, the probe is a pure function of injected deps + * so the suite runs identically on any host. Windows is skipped: the + * problem is specific to POSIX login-shell profiles. + */ + +import { userInfo } from 'node:os'; + +import { execFileText } from './environment'; + +export interface LoginShellPathDeps { + readonly platform: string; + readonly env: Record; + /** Login shell from the OS user database; fallback when $SHELL is unset. */ + readonly userShell: () => string | undefined; + readonly execFileText: ( + file: string, + args: readonly string[], + timeoutMs: number, + ) => Promise; +} + +const LOGIN_SHELL_ENV_TIMEOUT_MS = 5_000; + +/** + * Run the user's login shell and return its PATH, or `undefined` when the + * probe does not apply (Windows, no resolvable shell) or fails (spawn + * error, timeout, no PATH in the output). + */ +export async function probeLoginShellPath(deps: LoginShellPathDeps): Promise { + if (deps.platform === 'win32') return undefined; + // A set-but-blank $SHELL (some daemon/launchd envs) must also fall back. + const envShell = deps.env['SHELL']?.trim(); + const shell = envShell === undefined || envShell.length === 0 ? deps.userShell() : envShell; + if (shell === undefined || shell.length === 0) return undefined; + + // `env` prints the resolved environment in every shell dialect, unlike + // `echo $PATH`, which fish would join with spaces. Invoke it by absolute + // path: a bare `env` resolves through the inherited PATH — which may + // carry cwd-dependent components — from the workspace cwd, so a + // repo-planted `env` binary could run at session startup and feed us an + // arbitrary PATH. The absolute path also bypasses profile function + // 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, + ['-l', '-c', '/usr/bin/env'], + LOGIN_SHELL_ENV_TIMEOUT_MS, + ); + if (stdout === undefined) return undefined; + + // Profile output lands on stdout before `env` runs, so keep the last + // PATH= line. + let path: string | undefined; + for (const line of stdout.split('\n')) { + if (line.startsWith('PATH=')) { + path = line.slice('PATH='.length).trim(); + } + } + if (path === undefined || path.length === 0) return undefined; + return path; +} + +/** + * Union of the current PATH and the login-shell PATH: the current PATH + * string is kept verbatim — including empty components, which POSIX + * command lookup treats as the current directory — and login-shell + * entries the current PATH lacks are appended in their own order. When + * nothing is missing the current string is returned unchanged. Only + * absolute login-shell entries are imported: empty, `.`, and relative + * components are all cwd-dependent lookup, and appending one the user + * did not already have would widen their search path — LocalKaos runs + * commands from arbitrary workspace directories. + */ +export function mergeLoginShellPath( + currentPath: string | undefined, + loginShellPath: string, +): string { + const current = currentPath ?? ''; + const seen = new Set(current.split(':').filter((entry) => entry.length > 0)); + const additions: string[] = []; + for (const entry of loginShellPath.split(':')) { + // The probe only runs on POSIX (win32 bails before merging), so a + // leading slash is a sufficient absoluteness test. Empty components + // fail it too. + if (!entry.startsWith('/') || seen.has(entry)) continue; + seen.add(entry); + additions.push(entry); + } + if (additions.length === 0) return current; + // `undefined` means "no PATH at all", so the additions stand alone; '' + // is a real (cwd-only) PATH whose empty component must survive as a + // leading colon. + if (currentPath === undefined) return additions.join(':'); + return `${current}:${additions.join(':')}`; +} + +/** Probe the login shell and merge its PATH into `deps.env['PATH']`. */ +export async function applyLoginShellPath(deps: LoginShellPathDeps): Promise { + const loginShellPath = await probeLoginShellPath(deps); + if (loginShellPath === undefined) return; + const currentPath = deps.env['PATH']; + const merged = mergeLoginShellPath(currentPath, loginShellPath); + // Only write when something was appended — an unset PATH must stay + // unset (assigning '' would turn "implementation default search path" + // into "cwd-only lookup"), and a set PATH must not be rewritten. + if (merged === (currentPath ?? '')) return; + deps.env['PATH'] = merged; +} + +/** + * Production convenience — apply the probe to `process.env` once per + * process. Memoised like `detectEnvironmentFromNode`: the login-shell PATH + * does not change for the lifetime of the process, and repeated + * `LocalKaos.create()` calls must not re-spawn the shell. + */ +/** + * Login shell from the OS user database (`/etc/passwd` via getpwuid on + * Linux, Directory Services on macOS). `userInfo()` throws when the uid + * has no database entry (e.g. containers running an arbitrary uid), and + * service accounts may carry `/usr/sbin/nologin` — the latter needs no + * special casing here because probing it simply fails and degrades + * silently. + */ +function userShellFromNode(): string | undefined { + try { + const shell = userInfo().shell; + return shell === null || shell.length === 0 ? undefined : shell; + } catch { + return undefined; + } +} + +let appliedLoginShellPath: Promise | undefined; + +export function applyLoginShellPathFromNode(): Promise { + if (appliedLoginShellPath !== undefined) return appliedLoginShellPath; + appliedLoginShellPath = applyLoginShellPath({ + platform: process.platform, + env: process.env as Record, + userShell: userShellFromNode, + execFileText, + }); + return appliedLoginShellPath; +} diff --git a/packages/kaos/test/login-shell-path.test.ts b/packages/kaos/test/login-shell-path.test.ts new file mode 100644 index 000000000..1b2d6fbde --- /dev/null +++ b/packages/kaos/test/login-shell-path.test.ts @@ -0,0 +1,236 @@ +/** + * Login-shell PATH enrichment. + * + * Reproduces the "Bash tool can't find local `gh`" report: when kimi-code + * is launched from a context that skipped the user's shell profile (GUI + * launcher, non-login parent shell), `process.env.PATH` misses entries + * like `/opt/homebrew/bin`, so every command spawned by the Bash tool + * inherits the impoverished PATH. + * + * `LocalKaos.create()` must probe the user's login shell (`$SHELL -l -c + * /usr/bin/env`, falling back to the OS account's login shell when $SHELL + * is unset or blank) once and append the missing PATH entries to + * `process.env.PATH` — without reordering or overriding what is already + * there. Probe failures (no resolvable shell, hung or broken profile) + * must leave PATH untouched. + * + * The probe/merge unit tests are pure (injected deps) and run on every + * platform. The end-to-end LocalKaos suite spawns a stub shell and is + * skipped on Windows: the problem is specific to POSIX login-shell + * profiles, and the probe must not run there. + */ + +import { chmod, mkdtemp, rm, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; + +import { + applyLoginShellPath, + type LoginShellPathDeps, + mergeLoginShellPath, + probeLoginShellPath, +} from '#/login-shell-path'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +interface StubOpts { + readonly platform?: string; + readonly env?: Record; + readonly execFileResult?: string | undefined; + readonly execFileText?: LoginShellPathDeps['execFileText']; + readonly userShell?: string | undefined; +} + +/** Build a stub deps bag; records `execFileText` invocations in `calls`. */ +function stubDeps(opts: StubOpts): { deps: LoginShellPathDeps; calls: unknown[][] } { + const calls: unknown[][] = []; + return { + calls, + deps: { + platform: opts.platform ?? 'darwin', + env: opts.env ?? { SHELL: '/bin/zsh' }, + userShell: () => opts.userShell, + execFileText: + opts.execFileText ?? + (async (file, args, timeoutMs) => { + calls.push([file, args, timeoutMs]); + return opts.execFileResult; + }), + }, + }; +} + +describe('probeLoginShellPath', () => { + it('runs $SHELL -l -c /usr/bin/env and returns its PATH', async () => { + const { deps, calls } = stubDeps({ + execFileResult: 'HOME=/Users/u\nPATH=/opt/homebrew/bin:/usr/bin:/bin\nTERM=dumb\n', + }); + await expect(probeLoginShellPath(deps)).resolves.toBe('/opt/homebrew/bin:/usr/bin:/bin'); + // env must be invoked by absolute path: a bare `env` resolves through + // the inherited (possibly cwd-dependent) PATH from the workspace cwd, + // so a repo-planted `env` binary could run at session startup. + expect(calls).toEqual([['/bin/zsh', ['-l', '-c', '/usr/bin/env'], 5_000]]); + }); + + it('keeps the last PATH= line, ignoring profile noise printed earlier', async () => { + const { deps } = stubDeps({ + execFileResult: 'PATH=/from-profile-echo\nsome profile banner\nPATH=/real/bin:/usr/bin\n', + }); + await expect(probeLoginShellPath(deps)).resolves.toBe('/real/bin:/usr/bin'); + }); + + it('returns undefined on Windows without spawning anything', async () => { + const { deps, calls } = stubDeps({ platform: 'win32', execFileResult: 'PATH=/x' }); + await expect(probeLoginShellPath(deps)).resolves.toBeUndefined(); + expect(calls).toEqual([]); + }); + + it('falls back to the account login shell when SHELL is unset or blank', async () => { + // launchd/daemon launches can leave $SHELL unset or blank (the very + // contexts whose PATH is impoverished); the probe must then use the + // OS account's login shell instead of giving up. + for (const env of [{}, { SHELL: '' }, { SHELL: ' ' }]) { + const { deps, calls } = stubDeps({ + env, + userShell: '/bin/zsh', + execFileResult: 'PATH=/opt/homebrew/bin:/usr/bin\n', + }); + await expect(probeLoginShellPath(deps)).resolves.toBe('/opt/homebrew/bin:/usr/bin'); + expect(calls).toEqual([['/bin/zsh', ['-l', '-c', '/usr/bin/env'], 5_000]]); + } + }); + + it('returns undefined when SHELL is unset and no account shell is available', async () => { + for (const env of [{}, { SHELL: '' }, { SHELL: ' ' }]) { + const { deps, calls } = stubDeps({ env, execFileResult: 'PATH=/x' }); + await expect(probeLoginShellPath(deps)).resolves.toBeUndefined(); + expect(calls).toEqual([]); + } + }); + + it('returns undefined when the shell fails or times out', async () => { + const { deps } = stubDeps({ execFileResult: undefined }); + await expect(probeLoginShellPath(deps)).resolves.toBeUndefined(); + }); + + it('returns undefined when the output has no PATH line', async () => { + const { deps } = stubDeps({ execFileResult: 'HOME=/Users/u\nTERM=dumb\n' }); + await expect(probeLoginShellPath(deps)).resolves.toBeUndefined(); + }); +}); + +describe('mergeLoginShellPath', () => { + it('appends entries the current PATH lacks, keeping current priority', () => { + expect(mergeLoginShellPath('/usr/bin:/bin', '/opt/homebrew/bin:/usr/bin:/extra')).toBe( + '/usr/bin:/bin:/opt/homebrew/bin:/extra', + ); + }); + + it('returns the current PATH string verbatim when nothing is missing', () => { + // Strict identity, including empty components and duplicates the user + // already has — a no-op merge must not normalize anything. + expect(mergeLoginShellPath('/a::/b:/a:', '/b:/a')).toBe('/a::/b:/a:'); + }); + + it('preserves empty components (cwd lookup) in the current PATH while appending', () => { + // POSIX treats a leading colon, trailing colon, or double colon as + // "search the current directory"; merging must not strip that. + expect(mergeLoginShellPath(':/usr/bin', '/new')).toBe(':/usr/bin:/new'); + expect(mergeLoginShellPath('/usr/bin:', '/new')).toBe('/usr/bin::/new'); + expect(mergeLoginShellPath('/a::/b', '/c')).toBe('/a::/b:/c'); + // A set-but-empty PATH is cwd-only lookup; the empty component stays first. + expect(mergeLoginShellPath('', '/a')).toBe(':/a'); + }); + + it('handles an unset current PATH', () => { + expect(mergeLoginShellPath(undefined, '/a:/b')).toBe('/a:/b'); + }); + + it('skips empty and duplicate login-shell entries', () => { + // Empty login-shell components are never imported: appending a cwd + // lookup the user did not already have would widen their search path. + expect(mergeLoginShellPath('/a', ':/b::/a:')).toBe('/a:/b'); + }); + + it('skips relative login-shell entries', () => { + // `.` and relative components are cwd-dependent lookup with another + // spelling — LocalKaos runs commands from arbitrary workspace + // directories, so importing one would let a command name resolve from + // an untrusted project cwd. Only absolute entries may be appended. + expect(mergeLoginShellPath('/a', '.:bin:../x:/b')).toBe('/a:/b'); + }); +}); + +describe('applyLoginShellPath', () => { + it('merges the probed PATH into the env bag', async () => { + const env: Record = { SHELL: '/bin/zsh', PATH: '/usr/bin' }; + const { deps } = stubDeps({ env, execFileResult: 'PATH=/opt/homebrew/bin:/usr/bin\n' }); + await applyLoginShellPath(deps); + expect(env['PATH']).toBe('/usr/bin:/opt/homebrew/bin'); + }); + + it('leaves PATH untouched when the probe fails', async () => { + const env: Record = { SHELL: '/bin/zsh', PATH: '/usr/bin' }; + const { deps } = stubDeps({ env, execFileResult: undefined }); + await applyLoginShellPath(deps); + expect(env['PATH']).toBe('/usr/bin'); + }); + + it('does not set an unset PATH when the login shell contributes nothing', async () => { + // Pathological but possible: the login-shell PATH holds only empty + // components. Writing '' back would turn "unset" (implementation + // default search path) into "cwd-only lookup". + const env: Record = { SHELL: '/bin/zsh' }; + const { deps } = stubDeps({ env, execFileResult: 'PATH=:::\n' }); + await applyLoginShellPath(deps); + expect('PATH' in env).toBe(false); + }); +}); + +describe.skipIf(process.platform === 'win32')('LocalKaos login-shell PATH enrichment', () => { + let tempDir: string; + let originalPath: string | undefined; + let originalShell: string | undefined; + + beforeEach(async () => { + tempDir = await mkdtemp(join(tmpdir(), 'kaos-login-path-')); + originalPath = process.env['PATH']; + originalShell = process.env['SHELL']; + }); + + afterEach(async () => { + restoreEnv('PATH', originalPath); + restoreEnv('SHELL', originalShell); + await rm(tempDir, { recursive: true, force: true }); + }); + + it('appends login-shell PATH entries missing from process.env.PATH', async () => { + const extraDir = join(tempDir, 'login-only-bin'); + const stubShell = join(tempDir, 'stub-shell.sh'); + // Stands in for the user's login shell: whatever flags it is invoked + // with, it reports an environment whose PATH carries an entry the + // kimi-code process does not have. + await writeFile(stubShell, `#!/bin/sh\necho "HOME=$HOME"\necho "PATH=${extraDir}:/usr/bin:/bin"\n`); + await chmod(stubShell, 0o755); + process.env['SHELL'] = stubShell; + + // The suite's setup.ts already ran LocalKaos.create() with the real + // $SHELL, consuming the memoised probe. Import a fresh module graph so + // this create() probes the stub shell instead. + vi.resetModules(); + const { LocalKaos } = await import('#/local'); + await LocalKaos.create(); + + const entries = (process.env['PATH'] ?? '').split(':'); + expect(entries).toContain(extraDir); + // Existing entries keep priority: the login-shell extras are appended. + expect(process.env['PATH']?.startsWith(originalPath ?? '')).toBe(true); + }); +}); + +function restoreEnv(key: string, value: string | undefined): void { + if (value === undefined) { + delete process.env[key]; + } else { + process.env[key] = value; + } +}