Skip to content

Commit cda315a

Browse files
committed
🤖 Fix redundant CD detection for tilde vs absolute path mismatch
The bash tool's redundant cd detection was failing when the working directory used tilde notation (~/path) but the agent used absolute notation (/home/user/path), or vice versa. **Problem:** - SSHRuntime.normalizePath() doesn't expand tildes (can't know remote HOME) - When cwd="~/workspace/project" and agent does "cd /home/user/workspace/project", the paths don't match even though they refer to the same directory - This caused redundant cd commands to slip through undetected **Solution:** Added heuristic matching in bash.ts to catch tilde vs absolute path mismatches: - If one path has tilde and the other is absolute, extract the suffix after tilde - Check if the absolute path ends with the same suffix as the tilde path - Example: cwd="~/project" matches target="/home/user/project" **Why this approach:** - Can't query remote HOME without async call (would break normalizePath interface) - Suffix matching is a reasonable heuristic that catches common cases - False negatives (missing some redundant cds) are better than false positives **Test Coverage:** Added 4 new test cases: - ✅ cwd has tilde, target is absolute (matching suffix) → rejected - ✅ cwd is absolute, target has tilde (matching suffix) → rejected - ✅ cwd has tilde, target is unrelated absolute path → allowed - ✅ Different paths with same notation → allowed All tests pass, type checking and linting clean.
1 parent c564e72 commit cda315a

File tree

3 files changed

+107
-2
lines changed

3 files changed

+107
-2
lines changed

src/runtime/SSHRuntime.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -317,7 +317,6 @@ export class SSHRuntime implements Runtime {
317317
isDirectory: fileType === "directory",
318318
};
319319
}
320-
321320
normalizePath(targetPath: string, basePath: string): string {
322321
// For SSH, handle paths in a POSIX-like manner without accessing the remote filesystem
323322
const target = targetPath.trim();

src/services/tools/bash.test.ts

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1369,4 +1369,86 @@ describe("SSH runtime redundant cd detection", () => {
13691369
expect(result.error).toContain("Redundant cd");
13701370
}
13711371
});
1372+
1373+
// Tests for tilde vs absolute path heuristic matching
1374+
it("should reject redundant cd when cwd has tilde but target is absolute (matching suffix)", async () => {
1375+
const remoteCwd = "~/workspace/project/branch";
1376+
using testEnv = createTestBashToolWithSSH(remoteCwd);
1377+
const tool = testEnv.tool;
1378+
1379+
// Agent uses absolute path that matches the tilde path suffix
1380+
const args: BashToolArgs = {
1381+
script: "cd /home/user/workspace/project/branch && echo test",
1382+
timeout_secs: 5,
1383+
};
1384+
1385+
const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult;
1386+
1387+
expect(result.success).toBe(false);
1388+
if (!result.success) {
1389+
expect(result.error).toContain("Redundant cd");
1390+
expect(result.error).toContain("already runs in");
1391+
}
1392+
});
1393+
1394+
it("should reject redundant cd when cwd is absolute but target has tilde (matching suffix)", async () => {
1395+
const remoteCwd = "/home/user/workspace/project/branch";
1396+
using testEnv = createTestBashToolWithSSH(remoteCwd);
1397+
const tool = testEnv.tool;
1398+
1399+
// Agent uses tilde path that matches the absolute path suffix
1400+
const args: BashToolArgs = {
1401+
script: "cd ~/workspace/project/branch && echo test",
1402+
timeout_secs: 5,
1403+
};
1404+
1405+
const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult;
1406+
1407+
expect(result.success).toBe(false);
1408+
if (!result.success) {
1409+
expect(result.error).toContain("Redundant cd");
1410+
expect(result.error).toContain("already runs in");
1411+
}
1412+
});
1413+
1414+
it("should allow cd when cwd has tilde but target is unrelated absolute path", async () => {
1415+
const remoteCwd = "~/workspace/project/branch";
1416+
using testEnv = createTestBashToolWithSSH(remoteCwd);
1417+
const tool = testEnv.tool;
1418+
1419+
// Agent uses absolute path that doesn't match the tilde path
1420+
const args: BashToolArgs = {
1421+
script: "cd /opt/other/path && echo test",
1422+
timeout_secs: 5,
1423+
};
1424+
1425+
const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult;
1426+
1427+
// Should NOT be rejected - this is a legitimate cd to a different directory
1428+
// We can't execute it in tests, so we just verify it's not rejected for redundancy
1429+
if (!result.success) {
1430+
expect(result.error).not.toContain("Redundant cd");
1431+
}
1432+
});
1433+
1434+
it("should allow cd when paths differ even with same notation", async () => {
1435+
const remoteCwd = "~/workspace/project/branch";
1436+
using testEnv = createTestBashToolWithSSH(remoteCwd);
1437+
const tool = testEnv.tool;
1438+
1439+
// Different path with same tilde notation
1440+
const args: BashToolArgs = {
1441+
script: "cd ~/workspace/project/other && echo test",
1442+
timeout_secs: 5,
1443+
};
1444+
1445+
const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult;
1446+
1447+
// Should NOT be rejected - different directory
1448+
if (!result.success) {
1449+
expect(result.error).not.toContain("Redundant cd");
1450+
}
1451+
});
1452+
1453+
13721454
});

src/services/tools/bash.ts

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,31 @@ export const createBashTool: ToolFactory = (config: ToolConfiguration) => {
8787
const normalizedTarget = config.runtime.normalizePath(targetPath, config.cwd);
8888
const normalizedCwd = config.runtime.normalizePath(".", config.cwd);
8989

90-
if (normalizedTarget === normalizedCwd) {
90+
// Direct comparison
91+
let isRedundant = normalizedTarget === normalizedCwd;
92+
93+
// If direct comparison fails, try heuristic for tilde vs absolute path mismatch
94+
// This handles cases like: cwd="~/project" vs target="/home/user/project"
95+
if (!isRedundant) {
96+
// Check if one has tilde and the other is absolute
97+
const cwdHasTilde = normalizedCwd.startsWith("~");
98+
const targetHasTilde = normalizedTarget.startsWith("~");
99+
100+
if (cwdHasTilde !== targetHasTilde) {
101+
// Extract the suffix after tilde or find common suffix
102+
const cwdSuffix = cwdHasTilde ? normalizedCwd.substring(1) : "";
103+
const targetSuffix = targetHasTilde ? normalizedTarget.substring(1) : "";
104+
105+
// If one has tilde, check if the other ends with the same relative path
106+
if (cwdHasTilde && normalizedTarget.endsWith(cwdSuffix)) {
107+
isRedundant = true;
108+
} else if (targetHasTilde && normalizedCwd.endsWith(targetSuffix)) {
109+
isRedundant = true;
110+
}
111+
}
112+
}
113+
114+
if (isRedundant) {
91115
return {
92116
success: false,
93117
error: `Redundant cd to working directory detected. The tool already runs in ${config.cwd} - no cd needed. Remove the 'cd ${targetPath}' prefix.`,

0 commit comments

Comments
 (0)