Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion src/runtime/SSHRuntime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
188 changes: 13 additions & 175 deletions src/services/tools/bash.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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");
});
});
27 changes: 7 additions & 20 deletions src/services/tools/bash.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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, {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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({
Expand Down
2 changes: 2 additions & 0 deletions src/types/tools.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down