diff --git a/src/runtime/SSHRuntime.ts b/src/runtime/SSHRuntime.ts index d4be4d433..f3b860226 100644 --- a/src/runtime/SSHRuntime.ts +++ b/src/runtime/SSHRuntime.ts @@ -317,7 +317,6 @@ export class SSHRuntime implements Runtime { isDirectory: fileType === "directory", }; } - normalizePath(targetPath: string, basePath: string): string { // For SSH, handle paths in a POSIX-like manner without accessing the remote filesystem const target = targetPath.trim(); diff --git a/src/services/tools/bash.test.ts b/src/services/tools/bash.test.ts index 0542280b2..d2d0a9d99 100644 --- a/src/services/tools/bash.test.ts +++ b/src/services/tools/bash.test.ts @@ -697,95 +697,6 @@ describe("bash tool", () => { } }); - it("should reject redundant cd to working directory with &&", async () => { - using testEnv = createTestBashTool(); - const tool = testEnv.tool; - const cwd = process.cwd(); - - const args: BashToolArgs = { - script: `cd ${cwd} && echo test`, - timeout_secs: 5, - }; - - const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; - - expect(result.success).toBe(false); - if (!result.success) { - expect(result.error).toContain("Redundant cd"); - expect(result.error).toContain("already runs in"); - } - }); - - it("should reject redundant cd to working directory with semicolon", async () => { - using testEnv = createTestBashTool(); - const tool = testEnv.tool; - const cwd = process.cwd(); - - const args: BashToolArgs = { - script: `cd ${cwd}; echo test`, - timeout_secs: 5, - }; - - const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; - - expect(result.success).toBe(false); - if (!result.success) { - expect(result.error).toContain("Redundant cd"); - } - }); - - it("should reject redundant cd with relative path (.)", async () => { - using testEnv = createTestBashTool(); - const tool = testEnv.tool; - - const args: BashToolArgs = { - script: "cd . && echo test", - timeout_secs: 5, - }; - - const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; - - expect(result.success).toBe(false); - if (!result.success) { - expect(result.error).toContain("Redundant cd"); - } - }); - - it("should reject redundant cd with quoted path", async () => { - using testEnv = createTestBashTool(); - const tool = testEnv.tool; - const cwd = process.cwd(); - - const args: BashToolArgs = { - script: `cd '${cwd}' && echo test`, - timeout_secs: 5, - }; - - const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; - - expect(result.success).toBe(false); - if (!result.success) { - expect(result.error).toContain("Redundant cd"); - } - }); - - it("should allow cd to a different directory", async () => { - using testEnv = createTestBashTool(); - const tool = testEnv.tool; - - const args: BashToolArgs = { - script: "cd /tmp && pwd", - timeout_secs: 5, - }; - - const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; - - expect(result.success).toBe(true); - if (result.success) { - expect(result.output).toContain("/tmp"); - } - }); - it("should allow commands that don't start with cd", async () => { using testEnv = createTestBashTool(); const tool = testEnv.tool; @@ -1261,112 +1172,39 @@ describe("SSH runtime redundant cd detection", () => { }; } - it("should reject redundant cd to absolute path on SSH runtime", async () => { - const remoteCwd = "/home/user/project"; - using testEnv = createTestBashToolWithSSH(remoteCwd); - const tool = testEnv.tool; - - const args: BashToolArgs = { - script: `cd ${remoteCwd} && echo test`, - timeout_secs: 5, - }; - - const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; - - expect(result.success).toBe(false); - if (!result.success) { - expect(result.error).toContain("Redundant cd"); - expect(result.error).toContain("already runs in"); - } - }); - - it("should reject redundant cd with relative path (.) on SSH runtime", async () => { - const remoteCwd = "/home/user/project"; - using testEnv = createTestBashToolWithSSH(remoteCwd); - const tool = testEnv.tool; - - const args: BashToolArgs = { - script: "cd . && echo test", - timeout_secs: 5, - }; - - const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; - - expect(result.success).toBe(false); - if (!result.success) { - expect(result.error).toContain("Redundant cd"); - } - }); - - it("should reject redundant cd with tilde path on SSH runtime", async () => { - const remoteCwd = "~/project"; - using testEnv = createTestBashToolWithSSH(remoteCwd); - const tool = testEnv.tool; - - const args: BashToolArgs = { - script: "cd ~/project && echo test", - timeout_secs: 5, - }; - - const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; - - expect(result.success).toBe(false); - if (!result.success) { - expect(result.error).toContain("Redundant cd"); - } - }); - - it("should reject redundant cd with single tilde on SSH runtime", async () => { - const remoteCwd = "~"; + it("should add educational note when command starts with cd", async () => { + const remoteCwd = "~/workspace/project/branch"; using testEnv = createTestBashToolWithSSH(remoteCwd); const tool = testEnv.tool; const args: BashToolArgs = { - script: "cd ~ && echo test", + script: "cd ~/workspace/project/branch && echo test", timeout_secs: 5, }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; - expect(result.success).toBe(false); - if (!result.success) { - expect(result.error).toContain("Redundant cd"); + // Command should execute (not blocked) + // But should include a note about cd behavior + if (result.success && "note" in result) { + expect(result.note).toContain("bash command starts in"); + expect(result.note).toContain("do not persist"); } }); - it("should handle trailing slashes in path comparison on SSH runtime", async () => { - const remoteCwd = "/home/user/project"; + it("should not add note when command does not start with cd", async () => { + const remoteCwd = "~/workspace/project/branch"; using testEnv = createTestBashToolWithSSH(remoteCwd); const tool = testEnv.tool; const args: BashToolArgs = { - script: "cd /home/user/project/ && echo test", - timeout_secs: 5, - }; - - const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; - - expect(result.success).toBe(false); - if (!result.success) { - expect(result.error).toContain("Redundant cd"); - } - }); - - it("should handle cwd with trailing slash on SSH runtime", async () => { - const remoteCwd = "/home/user/project/"; - using testEnv = createTestBashToolWithSSH(remoteCwd); - const tool = testEnv.tool; - - const args: BashToolArgs = { - script: "cd /home/user/project && echo test", + script: "echo test", timeout_secs: 5, }; const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; - expect(result.success).toBe(false); - if (!result.success) { - expect(result.error).toContain("Redundant cd"); - } + // Should not have a note field + expect(result).not.toHaveProperty("note"); }); }); diff --git a/src/services/tools/bash.ts b/src/services/tools/bash.ts index 8def72db6..782e72b96 100644 --- a/src/services/tools/bash.ts +++ b/src/services/tools/bash.ts @@ -76,26 +76,11 @@ export const createBashTool: ToolFactory = (config: ToolConfiguration) => { let displayTruncated = false; // Hit 16KB display limit let fileTruncated = false; // Hit 100KB file limit - // Detect redundant cd to working directory - // Delegate path normalization to the runtime for proper handling of local vs remote paths - const cdPattern = /^\s*cd\s+['"]?([^'";\\&|]+)['"]?\s*[;&|]/; - const match = cdPattern.exec(script); - if (match) { - const targetPath = match[1].trim(); - - // Use runtime's normalizePath method to handle path comparison correctly - const normalizedTarget = config.runtime.normalizePath(targetPath, config.cwd); - const normalizedCwd = config.runtime.normalizePath(".", config.cwd); - - if (normalizedTarget === normalizedCwd) { - return { - success: false, - error: `Redundant cd to working directory detected. The tool already runs in ${config.cwd} - no cd needed. Remove the 'cd ${targetPath}' prefix.`, - exitCode: -1, - wall_duration_ms: 0, - }; - } - } + // Detect if command starts with cd - we'll add an educational note for the agent + const scriptStartsWithCd = /^\s*cd\s/.test(script); + const cdNote = scriptStartsWithCd + ? `Note: Each bash command starts in ${config.cwd}. Directory changes (cd) do not persist between commands.` + : undefined; // Execute using runtime interface (works for both local and SSH) const execStream = await config.runtime.exec(script, { @@ -392,6 +377,7 @@ export const createBashTool: ToolFactory = (config: ToolConfiguration) => { output, exitCode: 0, wall_duration_ms, + ...(cdNote && { note: cdNote }), truncated: { reason: overflowReason ?? "unknown reason", totalLines: lines.length, @@ -476,6 +462,7 @@ File will be automatically cleaned up when stream ends.`; output: lines.join("\n"), exitCode: 0, wall_duration_ms, + ...(cdNote && { note: cdNote }), }); } else { resolveOnce({ diff --git a/src/types/tools.ts b/src/types/tools.ts index 03264e7e1..bc5169adb 100644 --- a/src/types/tools.ts +++ b/src/types/tools.ts @@ -20,6 +20,7 @@ export type BashToolResult = success: true; output: string; exitCode: 0; + note?: string; // Agent-only message (not displayed in UI) truncated?: { reason: string; totalLines: number; @@ -30,6 +31,7 @@ export type BashToolResult = output?: string; exitCode: number; error: string; + note?: string; // Agent-only message (not displayed in UI) truncated?: { reason: string; totalLines: number;