Skip to content

Commit 64b020f

Browse files
committed
Fix redundant CD detection for SSH runtime
- Add runtime-aware path normalization function - Detect SSH vs Local runtime using instanceof check - Handle POSIX paths for SSH (no filesystem access needed) - Handle local paths using Node.js path.resolve - Add 6 new tests for SSH runtime redundant CD detection - All 52 bash tool tests pass Fixes issue where redundant cd commands were not properly detected when using SSH runtime due to using local path.resolve() on remote paths.
1 parent 2ee355f commit 64b020f

File tree

2 files changed

+206
-5
lines changed

2 files changed

+206
-5
lines changed

src/services/tools/bash.test.ts

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1235,3 +1235,139 @@ fi
12351235
expect(remainingProcesses).toBe(0);
12361236
});
12371237
});
1238+
1239+
describe("SSH runtime redundant cd detection", () => {
1240+
// Helper to create bash tool with SSH runtime configuration
1241+
// Note: These tests check redundant cd detection logic only - they don't actually execute via SSH
1242+
function createTestBashToolWithSSH(cwd: string) {
1243+
const tempDir = new TestTempDir("test-bash-ssh");
1244+
const sshRuntime = createRuntime({
1245+
type: "ssh",
1246+
host: "test-host",
1247+
srcBaseDir: "/remote/base",
1248+
});
1249+
1250+
const tool = createBashTool({
1251+
cwd,
1252+
runtime: sshRuntime,
1253+
tempDir: tempDir.path,
1254+
});
1255+
1256+
return {
1257+
tool,
1258+
[Symbol.dispose]() {
1259+
tempDir[Symbol.dispose]();
1260+
},
1261+
};
1262+
}
1263+
1264+
it("should reject redundant cd to absolute path on SSH runtime", async () => {
1265+
const remoteCwd = "/home/user/project";
1266+
using testEnv = createTestBashToolWithSSH(remoteCwd);
1267+
const tool = testEnv.tool;
1268+
1269+
const args: BashToolArgs = {
1270+
script: `cd ${remoteCwd} && echo test`,
1271+
timeout_secs: 5,
1272+
};
1273+
1274+
const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult;
1275+
1276+
expect(result.success).toBe(false);
1277+
if (!result.success) {
1278+
expect(result.error).toContain("Redundant cd");
1279+
expect(result.error).toContain("already runs in");
1280+
}
1281+
});
1282+
1283+
it("should reject redundant cd with relative path (.) on SSH runtime", async () => {
1284+
const remoteCwd = "/home/user/project";
1285+
using testEnv = createTestBashToolWithSSH(remoteCwd);
1286+
const tool = testEnv.tool;
1287+
1288+
const args: BashToolArgs = {
1289+
script: "cd . && echo test",
1290+
timeout_secs: 5,
1291+
};
1292+
1293+
const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult;
1294+
1295+
expect(result.success).toBe(false);
1296+
if (!result.success) {
1297+
expect(result.error).toContain("Redundant cd");
1298+
}
1299+
});
1300+
1301+
it("should reject redundant cd with tilde path on SSH runtime", async () => {
1302+
const remoteCwd = "~/project";
1303+
using testEnv = createTestBashToolWithSSH(remoteCwd);
1304+
const tool = testEnv.tool;
1305+
1306+
const args: BashToolArgs = {
1307+
script: "cd ~/project && echo test",
1308+
timeout_secs: 5,
1309+
};
1310+
1311+
const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult;
1312+
1313+
expect(result.success).toBe(false);
1314+
if (!result.success) {
1315+
expect(result.error).toContain("Redundant cd");
1316+
}
1317+
});
1318+
1319+
it("should reject redundant cd with single tilde on SSH runtime", async () => {
1320+
const remoteCwd = "~";
1321+
using testEnv = createTestBashToolWithSSH(remoteCwd);
1322+
const tool = testEnv.tool;
1323+
1324+
const args: BashToolArgs = {
1325+
script: "cd ~ && echo test",
1326+
timeout_secs: 5,
1327+
};
1328+
1329+
const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult;
1330+
1331+
expect(result.success).toBe(false);
1332+
if (!result.success) {
1333+
expect(result.error).toContain("Redundant cd");
1334+
}
1335+
});
1336+
1337+
it("should handle trailing slashes in path comparison on SSH runtime", async () => {
1338+
const remoteCwd = "/home/user/project";
1339+
using testEnv = createTestBashToolWithSSH(remoteCwd);
1340+
const tool = testEnv.tool;
1341+
1342+
const args: BashToolArgs = {
1343+
script: "cd /home/user/project/ && echo test",
1344+
timeout_secs: 5,
1345+
};
1346+
1347+
const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult;
1348+
1349+
expect(result.success).toBe(false);
1350+
if (!result.success) {
1351+
expect(result.error).toContain("Redundant cd");
1352+
}
1353+
});
1354+
1355+
it("should handle cwd with trailing slash on SSH runtime", async () => {
1356+
const remoteCwd = "/home/user/project/";
1357+
using testEnv = createTestBashToolWithSSH(remoteCwd);
1358+
const tool = testEnv.tool;
1359+
1360+
const args: BashToolArgs = {
1361+
script: "cd /home/user/project && echo test",
1362+
timeout_secs: 5,
1363+
};
1364+
1365+
const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult;
1366+
1367+
expect(result.success).toBe(false);
1368+
if (!result.success) {
1369+
expect(result.error).toContain("Redundant cd");
1370+
}
1371+
});
1372+
});
1373+

src/services/tools/bash.ts

Lines changed: 70 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,70 @@ import { EXIT_CODE_ABORTED, EXIT_CODE_TIMEOUT } from "@/constants/exitCodes";
1616
import type { BashToolResult } from "@/types/tools";
1717
import type { ToolConfiguration, ToolFactory } from "@/utils/tools/tools";
1818
import { TOOL_DEFINITIONS } from "@/utils/tools/toolDefinitions";
19+
import { SSHRuntime } from "@/runtime/SSHRuntime";
20+
21+
/**
22+
* Normalize a path for comparison purposes.
23+
* Works for both local and remote (SSH) paths.
24+
* Handles:
25+
* - Relative paths (. and ..)
26+
* - Tilde expansion (~)
27+
* - Removes trailing slashes
28+
* - Resolves to absolute paths where possible
29+
*
30+
* For SSH runtime, uses POSIX path semantics (forward slashes).
31+
* For local runtime, uses Node.js path.resolve.
32+
*/
33+
function normalizePath(targetPath: string, basePath: string, isSSH: boolean): string {
34+
// Trim whitespace
35+
const target = targetPath.trim();
36+
const base = basePath.trim();
37+
38+
if (isSSH) {
39+
// For SSH, we need to handle paths in a POSIX-like manner
40+
// without actually accessing the remote filesystem
41+
42+
// Handle special case: current directory
43+
if (target === ".") {
44+
return base;
45+
}
46+
47+
// Handle tilde expansion
48+
let normalizedTarget = target;
49+
if (target === "~" || target.startsWith("~/")) {
50+
// Can't normalize ~ without knowing $HOME, but we can compare
51+
// if both use ~ or if base uses ~
52+
normalizedTarget = target;
53+
} else if (target.startsWith("/")) {
54+
// Absolute path - use as-is
55+
normalizedTarget = target;
56+
} else {
57+
// Relative path - resolve against base
58+
// Simple POSIX path joining
59+
normalizedTarget = base.endsWith("/") ? base + target : base + "/" + target;
60+
}
61+
62+
// Remove trailing slash for comparison (except for root "/")
63+
if (normalizedTarget.length > 1 && normalizedTarget.endsWith("/")) {
64+
normalizedTarget = normalizedTarget.slice(0, -1);
65+
}
66+
67+
// Handle base path normalization
68+
let normalizedBase = base;
69+
if (normalizedBase.length > 1 && normalizedBase.endsWith("/")) {
70+
normalizedBase = normalizedBase.slice(0, -1);
71+
}
72+
73+
return normalizedTarget;
74+
} else {
75+
// For local runtime, use Node.js path resolution
76+
// Handle special case: current directory
77+
if (target === ".") {
78+
return path.resolve(base);
79+
}
80+
return path.resolve(base, target);
81+
}
82+
}
1983

2084
/**
2185
* Bash execution tool factory for AI assistant
@@ -83,11 +147,12 @@ export const createBashTool: ToolFactory = (config: ToolConfiguration) => {
83147
const match = cdPattern.exec(script);
84148
if (match) {
85149
const targetPath = match[1].trim();
86-
// For SSH runtime, config.cwd might use $HOME - need to handle this
87-
// Normalize paths for comparison (resolve to absolute where possible)
88-
// Note: This check is best-effort - it won't catch all cases on SSH (e.g., ~/path vs $HOME/path)
89-
const normalizedTarget = path.resolve(config.cwd, targetPath);
90-
const normalizedCwd = path.resolve(config.cwd);
150+
// Determine if we're using SSH runtime to apply appropriate path normalization
151+
const isSSH = config.runtime instanceof SSHRuntime;
152+
153+
// Normalize both paths for comparison using runtime-appropriate logic
154+
const normalizedTarget = normalizePath(targetPath, config.cwd, isSSH);
155+
const normalizedCwd = isSSH ? config.cwd.replace(/\/$/, "") : path.resolve(config.cwd);
91156

92157
if (normalizedTarget === normalizedCwd) {
93158
return {

0 commit comments

Comments
 (0)