diff --git a/.changeset/steady-shell-cwd.md b/.changeset/steady-shell-cwd.md new file mode 100644 index 00000000000..b9821b5d514 --- /dev/null +++ b/.changeset/steady-shell-cwd.md @@ -0,0 +1,5 @@ +--- +"@moonshot-ai/kimi-code": patch +--- + +Keep Bash scripts in the requested working directory after backgrounded commands while preserving logical symlink paths. diff --git a/packages/agent-core-v2/src/agent/tools/os/bash/bashTool.ts b/packages/agent-core-v2/src/agent/tools/os/bash/bashTool.ts index 41090010ce9..4ba5a10500d 100644 --- a/packages/agent-core-v2/src/agent/tools/os/bash/bashTool.ts +++ b/packages/agent-core-v2/src/agent/tools/os/bash/bashTool.ts @@ -161,15 +161,15 @@ export class BashTool implements IBashTool { command: string, ): Promise { const shellCwd = getShellPathBridge(env).toShellPath(effectiveCwd); - const shellCommand = `cd ${shellQuote(shellCwd)} && ${command}`; - const noninteractiveEnv: Record = { + const shellEnv: Record = { NO_COLOR: '1', TERM: 'dumb', GIT_TERMINAL_PROMPT: process.env['GIT_TERMINAL_PROMPT'] ?? '0', + PWD: shellCwd, SHELL: env.shellPath, }; - return processService.spawn(env.shellPath, ['-c', shellCommand], { env: noninteractiveEnv }); + return processService.spawn(env.shellPath, ['-c', command], { cwd: effectiveCwd, env: shellEnv }); } private async execution( @@ -469,10 +469,6 @@ async function killSpawnedProcess(proc: IHostProcess): Promise { } } -function shellQuote(s: string): string { - return `'${s.replaceAll("'", "'\\''")}'`; -} - const WINDOWS_NUL_REDIRECT = /(\d?&?>+\s*)[Nn][Uu][Ll](?=\s|$|[|&;)\n])/g; function rewriteWindowsNullRedirect(command: string): string { diff --git a/packages/agent-core-v2/test/os/backends/node-local/tools/bash.test.ts b/packages/agent-core-v2/test/os/backends/node-local/tools/bash.test.ts index 4609e51b6cc..172206e5565 100644 --- a/packages/agent-core-v2/test/os/backends/node-local/tools/bash.test.ts +++ b/packages/agent-core-v2/test/os/backends/node-local/tools/bash.test.ts @@ -837,9 +837,11 @@ describe('BashTool', () => { expect(exec).toHaveBeenCalledTimes(1); const [command, args, execOptions] = exec.mock.calls[0]!; expect(command).toBe('/bin/bash'); - expect(args).toEqual(['-c', "cd '/workspace' && printf ok"]); + expect(args).toEqual(['-c', 'printf ok']); + expect(execOptions?.cwd).toBe('/workspace'); expect(execOptions?.env).toMatchObject({ NO_COLOR: '1', + PWD: '/workspace', TERM: 'dumb', }); expect(proc.stdin.end).toHaveBeenCalledTimes(1); @@ -856,7 +858,34 @@ describe('BashTool', () => { await executeTool(tool, context({ command: 'pwd', cwd: '/workspace/project', timeout: 60 })); expect(exec.mock.calls[0]?.[0]).toBe('/bin/bash'); - expect(exec.mock.calls[0]?.[1]).toEqual(['-c', "cd '/workspace/project' && pwd"]); + expect(exec.mock.calls[0]?.[1]).toEqual(['-c', 'pwd']); + expect(exec.mock.calls[0]?.[2]?.cwd).toBe('/workspace/project'); + expect(exec.mock.calls[0]?.[2]?.env).toMatchObject({ PWD: '/workspace/project' }); + }); + + it('resolves a relative args.cwd against the session cwd', async () => { + const { runner, exec } = createTestRunner(processWithOutput({ stdout: 'sub\n' })); + const tool = bashTool(runner, posixEnv, createTestCtx('/workspace/project')); + + await executeTool(tool, context({ command: 'pwd', cwd: 'packages/ui', timeout: 60 })); + + expect(exec.mock.calls[0]?.[0]).toBe('/bin/bash'); + expect(exec.mock.calls[0]?.[1]).toEqual(['-c', 'pwd']); + expect(exec.mock.calls[0]?.[2]?.cwd).toBe('/workspace/project/packages/ui'); + }); + + it('resolves a relative Windows args.cwd without Git Bash path conversion', async () => { + const { runner, exec } = createTestRunner(processWithOutput({ stdout: 'sub\n' })); + const tool = bashTool(runner, windowsBashEnv, createTestCtx('C:\\Users\\me\\project')); + + await executeTool(tool, context({ command: 'pwd', cwd: 'packages\\ui', timeout: 60 })); + + expect(exec.mock.calls[0]?.[0]).toBe('C:\\Program Files\\Git\\bin\\bash.exe'); + expect(exec.mock.calls[0]?.[1]).toEqual(['-c', 'pwd']); + expect(exec.mock.calls[0]?.[2]?.cwd).toBe('C:\\Users\\me\\project\\packages\\ui'); + expect(exec.mock.calls[0]?.[2]?.env).toMatchObject({ + PWD: '/c/Users/me/project/packages/ui', + }); }); it('uses the kaos cwd as the default working directory', async () => { @@ -866,7 +895,8 @@ describe('BashTool', () => { await executeTool(tool, context({ command: 'pwd', timeout: 60 })); expect(exec.mock.calls[0]?.[0]).toBe('/bin/bash'); - expect(exec.mock.calls[0]?.[1]).toEqual(['-c', "cd '/var/app' && pwd"]); + expect(exec.mock.calls[0]?.[1]).toEqual(['-c', 'pwd']); + expect(exec.mock.calls[0]?.[2]?.cwd).toBe('/var/app'); }); it('uses Git Bash semantics on Windows', async () => { @@ -879,8 +909,12 @@ describe('BashTool', () => { expect(exec).toHaveBeenCalledTimes(1); const [command, args, execOptions] = exec.mock.calls[0]!; expect(command).toBe('C:\\Program Files\\Git\\bin\\bash.exe'); - expect(args).toEqual(['-c', "cd '/c/Users/me/project' && echo ok 2>/dev/null"]); - expect(execOptions?.env).toMatchObject({ SHELL: 'C:\\Program Files\\Git\\bin\\bash.exe' }); + expect(args).toEqual(['-c', 'echo ok 2>/dev/null']); + expect(execOptions?.cwd).toBe('C:\\Users\\me\\project'); + expect(execOptions?.env).toMatchObject({ + PWD: '/c/Users/me/project', + SHELL: 'C:\\Program Files\\Git\\bin\\bash.exe', + }); expect(result).toMatchObject({ output: 'ok\n', isError: false, @@ -1283,7 +1317,8 @@ describe('BashTool', () => { await executeTool(tool, context({ command: 'ls 2>nul', timeout: 60 })); const args = exec.mock.calls[0]?.[1] as readonly string[]; - expect(args[1]).toBe("cd '/c/Users/me/project' && ls 2>/dev/null"); + expect(args[1]).toBe('ls 2>/dev/null'); + expect(exec.mock.calls[0]?.[2]?.cwd).toBe('C:\\Users\\me\\project'); }); it('passes nul-redirect through unchanged on Linux so the argv keeps the literal file target', async () => { @@ -1293,7 +1328,8 @@ describe('BashTool', () => { await executeTool(tool, context({ command: 'ls 2>nul', timeout: 60 })); const args = exec.mock.calls[0]?.[1] as readonly string[]; - expect(args[1]).toBe("cd '/workspace' && ls 2>nul"); + expect(args[1]).toBe('ls 2>nul'); + expect(exec.mock.calls[0]?.[2]?.cwd).toBe('/workspace'); }); it('exposes a shell description that documents /bin/bash, TaskOutput/TaskStop, safety and efficiency sections, and background semantics', () => { @@ -1715,7 +1751,8 @@ describe('BashTool background mode', () => { expect(exec).toHaveBeenCalledTimes(2); const [command, args, execOptions] = exec.mock.calls[0]!; expect(command).toBe('C:\\Program Files\\Git\\bin\\bash.exe'); - expect(args).toEqual(['-c', "cd '/c/Users/me/project' && echo ok 2>/dev/null"]); + expect(args).toEqual(['-c', 'echo ok 2>/dev/null']); + expect(execOptions?.cwd).toBe('C:\\Users\\me\\project'); expect(execOptions?.env).toMatchObject({ SHELL: 'C:\\Program Files\\Git\\bin\\bash.exe' }); expect(secondProc.kill).toHaveBeenCalledWith('SIGTERM'); expect(results).toContainEqual(expect.objectContaining({ isError: false })); diff --git a/packages/agent-core/src/tools/builtin/shell/bash.ts b/packages/agent-core/src/tools/builtin/shell/bash.ts index 26d56f0171d..f1dd6b918dd 100644 --- a/packages/agent-core/src/tools/builtin/shell/bash.ts +++ b/packages/agent-core/src/tools/builtin/shell/bash.ts @@ -29,6 +29,7 @@ import { ProcessBackgroundTask, type BackgroundManager } from '../../../agent/ba import type { BuiltinTool } from '../../../agent/tool'; import type { ExecutableToolResult, ToolExecution, ToolUpdate } from '../../../loop/types'; import { renderPrompt } from '../../../utils/render-prompt'; +import { canonicalizePath } from '../../policies/path-access'; import { toInputJsonSchema } from '../../support/input-schema'; import { literalRulePattern, matchesGlobRuleSubject } from '../../support/rule-match'; import { @@ -272,13 +273,9 @@ export class BashTool implements BuiltinTool { private spawn(effectiveCwd: string, command: string): Promise { const shellCwd = getShellPathBridge(this.kaos.osEnv).toShellPath(effectiveCwd); - const shellArgs = [ - this.kaos.osEnv.shellPath, - '-c', - `cd ${shellQuote(shellCwd)} && ${command}`, - ]; + const shellArgs = [this.kaos.osEnv.shellPath, '-c', command]; - const noninteractiveEnv: Record = { + const shellEnv: Record = { NO_COLOR: '1', TERM: 'dumb', // Default to '0' so git fails fast on private remotes if a TTY happens @@ -288,13 +285,16 @@ export class BashTool implements BuiltinTool { SHELL: this.kaos.osEnv.shellPath, }; - // Merge ambient env + noninteractive knobs so tools like git / node + // Merge ambient env + shell-specific overrides so tools like git / node // don't open a pager and paints don't colour the stream. const mergedEnv: Record = { ...(process.env as Record), - ...noninteractiveEnv, + ...shellEnv, }; - return this.kaos.execWithEnv(shellArgs, mergedEnv); + return this.kaos + .withCwd(effectiveCwd) + .withEnv({ PWD: shellCwd }) + .execWithEnv(shellArgs, mergedEnv); } /** @@ -319,7 +319,6 @@ export class BashTool implements BuiltinTool { const startsInBackground = args.run_in_background === true; const foregroundTimeoutMs = normalizeForegroundTimeoutMs(args.timeout); const command = this.isWindowsBash ? rewriteWindowsNullRedirect(args.command) : args.command; - const effectiveCwd = args.cwd ?? this.cwd; const description = startsInBackground ? args.description!.trim() : foregroundDescription(args); const timeoutMs = startsInBackground ? args.disable_timeout @@ -330,6 +329,10 @@ export class BashTool implements BuiltinTool { const builder = new ToolResultBuilder(); let proc: KaosProcess; try { + const effectiveCwd = + args.cwd === undefined + ? this.cwd + : canonicalizePath(args.cwd, this.cwd, this.kaos.pathClass()); proc = await this.spawn(effectiveCwd, command); } catch (error) { return { @@ -603,10 +606,6 @@ async function killSpawnedProcess(proc: KaosProcess): Promise { } } -function shellQuote(s: string): string { - return `'${s.replaceAll("'", "'\\''")}'`; -} - const WINDOWS_NUL_REDIRECT = /(\d?&?>+\s*)[Nn][Uu][Ll](?=\s|$|[|&;)\n])/g; function rewriteWindowsNullRedirect(command: string): string { diff --git a/packages/agent-core/test/tools/bash.test.ts b/packages/agent-core/test/tools/bash.test.ts index d4904813d7a..c2277404007 100644 --- a/packages/agent-core/test/tools/bash.test.ts +++ b/packages/agent-core/test/tools/bash.test.ts @@ -1,14 +1,22 @@ -import { mkdtempSync, readFileSync, rmSync } from 'node:fs'; +import { + mkdirSync, + mkdtempSync, + readFileSync, + realpathSync, + rmSync, + symlinkSync, +} from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { PassThrough, Readable, type Writable } from 'node:stream'; -import type { Environment, KaosProcess } from '@moonshot-ai/kaos'; +import type { Environment, Kaos, KaosProcess } from '@moonshot-ai/kaos'; import { describe, expect, it, vi } from 'vitest'; import { type BashInput, BashInputSchema, BashTool } from '../../src/tools/builtin/shell/bash'; import { createBackgroundManager, registerProcess } from '../agent/background/helpers'; -import { createFakeKaos } from './fixtures/fake-kaos'; +import { testKaos } from '../fixtures/test-kaos'; +import { createFakeKaos, toolContentString } from './fixtures/fake-kaos'; import { executeTool } from './fixtures/execute-tool'; const posixEnv: Environment = { @@ -411,9 +419,10 @@ describe('BashTool', () => { expect(execWithEnv).toHaveBeenCalledTimes(1); const [argv, env] = execWithEnv.mock.calls[0]!; - expect(argv).toEqual(['/bin/bash', '-c', "cd '/workspace' && printf ok"]); + expect(argv).toEqual(['/bin/bash', '-c', 'printf ok']); expect(env).toMatchObject({ NO_COLOR: '1', + PWD: '/workspace', TERM: 'dumb', }); expect(proc.stdin.end).toHaveBeenCalledTimes(1); @@ -426,17 +435,93 @@ describe('BashTool', () => { it('uses args.cwd when provided', async () => { const execWithEnv = vi.fn().mockResolvedValue(processWithOutput({ stdout: 'sub\n' })); - const tool = bashTool( - createFakeKaos({ execWithEnv, osEnv: posixEnv }), - '/workspace', - createBackgroundManager().manager, - ); + const withCwd = vi.fn().mockReturnValue(createFakeKaos({ execWithEnv, osEnv: posixEnv })); + const kaos: Kaos = { ...createFakeKaos({ osEnv: posixEnv }), withCwd }; + const tool = bashTool(kaos, '/workspace', createBackgroundManager().manager); await executeTool(tool, context({ command: 'pwd', cwd: '/tmp/project', timeout: 60 })); - expect(execWithEnv.mock.calls[0]?.[0]).toEqual(['/bin/bash', '-c', "cd '/tmp/project' && pwd"]); + expect(execWithEnv.mock.calls[0]?.[0]).toEqual(['/bin/bash', '-c', 'pwd']); + expect(withCwd).toHaveBeenCalledWith('/tmp/project'); + expect(execWithEnv.mock.calls[0]?.[1]).toMatchObject({ PWD: '/tmp/project' }); + }); + + it('resolves a relative args.cwd against the session cwd', async () => { + const execWithEnv = vi.fn().mockResolvedValue(processWithOutput({ stdout: 'sub\n' })); + const withCwd = vi.fn().mockReturnValue(createFakeKaos({ execWithEnv, osEnv: posixEnv })); + const kaos: Kaos = { ...createFakeKaos({ osEnv: posixEnv }), withCwd }; + const tool = bashTool(kaos, '/workspace/project', createBackgroundManager().manager); + + await executeTool(tool, context({ command: 'pwd', cwd: 'packages/ui', timeout: 60 })); + + expect(execWithEnv.mock.calls[0]?.[0]).toEqual(['/bin/bash', '-c', 'pwd']); + expect(withCwd).toHaveBeenCalledWith('/workspace/project/packages/ui'); + }); + + it('resolves a relative Windows args.cwd to a native process cwd', async () => { + const execWithEnv = vi.fn().mockResolvedValue(processWithOutput({ stdout: 'sub\n' })); + const withCwd = vi.fn().mockReturnValue(createFakeKaos({ execWithEnv, osEnv: windowsBashEnv })); + const kaos: Kaos = { ...createFakeKaos({ osEnv: windowsBashEnv }), withCwd }; + const tool = bashTool(kaos, 'C:\\Users\\me\\project', createBackgroundManager().manager); + + await executeTool(tool, context({ command: 'pwd', cwd: 'packages\\ui', timeout: 60 })); + + expect(execWithEnv.mock.calls[0]?.[0]).toEqual([ + 'C:\\Program Files\\Git\\bin\\bash.exe', + '-c', + 'pwd', + ]); + expect(withCwd).toHaveBeenCalledWith('C:/Users/me/project/packages/ui'); + expect(execWithEnv.mock.calls[0]?.[1]).toMatchObject({ + PWD: '/c/Users/me/project/packages/ui', + }); }); + it.skipIf(process.platform === 'win32')( + 'keeps later lines in cwd when the first command is backgrounded', + async () => { + const cwd = mkdtempSync(join(tmpdir(), 'kimi-bash-cwd-')); + try { + const tool = bashTool(testKaos, '/workspace'); + + const result = await executeTool( + tool, + context({ + command: 'true &\npwd -P\nwait', + cwd, + timeout: 60, + }), + ); + + expect(result).toMatchObject({ isError: false }); + expect(toolContentString(result).trim()).toBe(realpathSync(cwd)); + } finally { + rmSync(cwd, { recursive: true, force: true }); + } + }, + ); + + it.skipIf(process.platform === 'win32')( + 'preserves the logical cwd for a symlinked workspace over a stale kaos PWD', + async () => { + const root = mkdtempSync(join(tmpdir(), 'kimi-bash-cwd-')); + const target = join(root, 'target'); + const cwd = join(root, 'workspace'); + mkdirSync(target); + symlinkSync(target, cwd, 'dir'); + + try { + const tool = bashTool(testKaos.withEnv({ PWD: target }), '/workspace'); + const result = await executeTool(tool, context({ command: 'pwd', cwd, timeout: 60 })); + + expect(result).toMatchObject({ isError: false }); + expect(toolContentString(result).trim()).toBe(cwd); + } finally { + rmSync(root, { recursive: true, force: true }); + } + }, + ); + it('uses Git Bash semantics on Windows', async () => { const proc = processWithOutput({ stdout: 'ok\n' }); const execWithEnv = vi.fn().mockResolvedValue(proc); @@ -452,9 +537,12 @@ describe('BashTool', () => { expect(argv).toEqual([ 'C:\\Program Files\\Git\\bin\\bash.exe', '-c', - "cd '/c/Users/me/project' && echo ok 2>/dev/null", + 'echo ok 2>/dev/null', ]); - expect(env).toMatchObject({ SHELL: 'C:\\Program Files\\Git\\bin\\bash.exe' }); + expect(env).toMatchObject({ + PWD: '/c/Users/me/project', + SHELL: 'C:\\Program Files\\Git\\bin\\bash.exe', + }); expect(result).toMatchObject({ output: 'ok\n', isError: false, @@ -1028,7 +1116,7 @@ describe('BashTool', () => { expect(argv).toEqual([ 'C:\\Program Files\\Git\\bin\\bash.exe', '-c', - "cd '/c/Users/me/project' && echo ok 2>/dev/null", + 'echo ok 2>/dev/null', ]); expect(env).toMatchObject({ SHELL: 'C:\\Program Files\\Git\\bin\\bash.exe' }); expect(secondProc.kill).toHaveBeenCalledWith('SIGTERM'); @@ -1353,7 +1441,7 @@ describe('BashTool', () => { await executeTool(tool, context({ command: 'ls 2>nul', timeout: 60 })); const argv = execWithEnv.mock.calls[0]?.[0] as readonly string[]; - expect(argv[2]).toBe("cd '/c/Users/me/project' && ls 2>/dev/null"); + expect(argv[2]).toBe('ls 2>/dev/null'); }); it('passes nul-redirect through unchanged on Linux so the argv keeps the literal file target', async () => { @@ -1363,7 +1451,7 @@ describe('BashTool', () => { await executeTool(tool, context({ command: 'ls 2>nul', timeout: 60 })); const argv = execWithEnv.mock.calls[0]?.[0] as readonly string[]; - expect(argv[2]).toBe("cd '/workspace' && ls 2>nul"); + expect(argv[2]).toBe('ls 2>nul'); }); it('exposes a shell description that documents /bin/bash, TaskOutput/TaskStop, safety and efficiency sections, and background semantics', () => { diff --git a/packages/agent-core/test/tools/shell-quoting.test.ts b/packages/agent-core/test/tools/shell-quoting.test.ts index 62a17a7188b..96f4cceb2af 100644 --- a/packages/agent-core/test/tools/shell-quoting.test.ts +++ b/packages/agent-core/test/tools/shell-quoting.test.ts @@ -71,10 +71,7 @@ function captureCommandRewrite( signal, }).then(() => { const argv = execWithEnv.mock.calls[0]?.[0] as readonly string[]; - // The shell wrapper is "cd '' && "; isolate the rewrite. - const wrapped = argv[2]!; - const match = /^cd '[^']+' && (.*)$/.exec(wrapped)!; - return { rewritten: match[1]!, argv }; + return { rewritten: argv[2]!, argv }; }); }