From dbccb1c41c529d597cb2133ae780b518e2a54f23 Mon Sep 17 00:00:00 2001 From: "haozhe.yang" Date: Wed, 8 Jul 2026 16:54:16 +0800 Subject: [PATCH] fix(agent-core-v2): enrich PATH from the user's login shell at startup - port probeLoginShellPath/mergeLoginShellPath/applyLoginShellPath into _base/execEnv/loginShellPath.ts as a pure helper (no DI) - export execFileText from environmentProbe for reuse by the probe - run applyLoginShellPathFromNode concurrently with the host probe in HostEnvironmentService, mirroring kaos LocalKaos.create() Aligns agent-core-v2 with kaos 021786f5 so the Bash tool finds user-installed tools (e.g. Homebrew's gh) when kimi-code is launched from a GUI or non-login shell. --- .../src/_base/execEnv/environmentProbe.ts | 2 +- .../src/_base/execEnv/loginShellPath.ts | 152 +++++++++++ .../node-local/hostEnvironmentService.ts | 25 +- .../test/_base/loginShellPath.test.ts | 239 ++++++++++++++++++ 4 files changed, 410 insertions(+), 8 deletions(-) create mode 100644 packages/agent-core-v2/src/_base/execEnv/loginShellPath.ts create mode 100644 packages/agent-core-v2/test/_base/loginShellPath.test.ts diff --git a/packages/agent-core-v2/src/_base/execEnv/environmentProbe.ts b/packages/agent-core-v2/src/_base/execEnv/environmentProbe.ts index d9b236092..f813c29d4 100644 --- a/packages/agent-core-v2/src/_base/execEnv/environmentProbe.ts +++ b/packages/agent-core-v2/src/_base/execEnv/environmentProbe.ts @@ -318,7 +318,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/agent-core-v2/src/_base/execEnv/loginShellPath.ts b/packages/agent-core-v2/src/_base/execEnv/loginShellPath.ts new file mode 100644 index 000000000..5930011d1 --- /dev/null +++ b/packages/agent-core-v2/src/_base/execEnv/loginShellPath.ts @@ -0,0 +1,152 @@ +/** + * `_base/execEnv` (L0) — login-shell PATH probe. + * + * Enriches `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, so the probe falls + * back to the OS account's login shell from the user database before giving up. + * + * Like `probeHostEnvironment`, 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. + * + * Vendored from `@moonshot-ai/kaos` `login-shell-path.ts` — kept as a pure + * helper with no DI dependencies. + */ + +import { userInfo } from 'node:os'; + +import { execFileText } from './environmentProbe'; + +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 — + * the host 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; +} + +/** + * 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; + +/** + * Production convenience — apply the probe to `process.env` once per process. + * Memoised like `probeHostEnvironmentFromNode`: the login-shell PATH does not + * change for the lifetime of the process, and repeated calls must not re-spawn + * the shell. + */ +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/agent-core-v2/src/os/backends/node-local/hostEnvironmentService.ts b/packages/agent-core-v2/src/os/backends/node-local/hostEnvironmentService.ts index 7c129330a..7d6143bcf 100644 --- a/packages/agent-core-v2/src/os/backends/node-local/hostEnvironmentService.ts +++ b/packages/agent-core-v2/src/os/backends/node-local/hostEnvironmentService.ts @@ -1,15 +1,17 @@ /** * `hostEnvironment` domain (L1) — `IHostEnvironment` implementation. * - * Kicks off the OS / shell probe (`probeHostEnvironmentFromNode`) at - * construction time; the sync fields become populated once `ready` resolves. - * Reads before `ready` throws with a clear message so misuse fails loudly - * instead of returning stale zeros. Bound at App scope. + * Kicks off the OS / shell probe (`probeHostEnvironmentFromNode`) and the + * login-shell PATH enrichment (`applyLoginShellPathFromNode`) at construction + * time; the sync fields become populated once `ready` resolves. Reads before + * `ready` throws with a clear message so misuse fails loudly instead of + * returning stale zeros. Bound at App scope. */ import { InstantiationType } from '#/_base/di/extensions'; import { LifecycleScope, registerScopedService } from '#/_base/di/scope'; import { probeHostEnvironmentFromNode } from '#/_base/execEnv/environmentProbe'; +import { applyLoginShellPathFromNode } from '#/_base/execEnv/loginShellPath'; import { type HostEnvironmentInfo, @@ -26,9 +28,18 @@ export class HostEnvironmentService implements IHostEnvironment { readonly ready: Promise; constructor() { - this.ready = probeHostEnvironmentFromNode().then((info) => { - this._info = info; - }); + // 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: the login-shell probe is a no-op on + // win32 (where probeHostEnvironment reads PATH to locate Git Bash), and on + // POSIX probeHostEnvironment does not consult PATH. + this.ready = Promise.all([ + probeHostEnvironmentFromNode().then((info) => { + this._info = info; + }), + applyLoginShellPathFromNode(), + ]).then(() => {}); } private require(field: keyof HostEnvironmentInfo): never | HostEnvironmentInfo[typeof field] { diff --git a/packages/agent-core-v2/test/_base/loginShellPath.test.ts b/packages/agent-core-v2/test/_base/loginShellPath.test.ts new file mode 100644 index 000000000..0a19ff9f2 --- /dev/null +++ b/packages/agent-core-v2/test/_base/loginShellPath.test.ts @@ -0,0 +1,239 @@ +/** + * 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. + * + * `HostEnvironmentService` 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 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. + * + * Ported from `packages/kaos/test/login-shell-path.test.ts`; the e2e block + * exercises `applyLoginShellPathFromNode()` (the v2 entry wired into + * `HostEnvironmentService`) instead of v1's `LocalKaos.create()`. + */ + +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 '#/_base/execEnv/loginShellPath'; +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 — the host 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')('applyLoginShellPathFromNode', () => { + let tempDir: string; + let originalPath: string | undefined; + let originalShell: string | undefined; + + beforeEach(async () => { + tempDir = await mkdtemp(join(tmpdir(), 'v2-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: its shebang runs under /bin/sh, so + // the trailing `-l -c /usr/bin/env` land as positional args and the script + // just 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; + + // Drop any memoised probe from prior tests so this call probes the stub + // shell instead of returning a cached result. + vi.resetModules(); + const { applyLoginShellPathFromNode } = await import('#/_base/execEnv/loginShellPath'); + await applyLoginShellPathFromNode(); + + 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; + } +}