Skip to content

Conversation

@ammar-agent
Copy link
Collaborator

@ammar-agent ammar-agent commented Oct 26, 2025

Problem

The bash tool was not properly detecting redundant cd commands when using SSH runtime. The original code used path.resolve() which only works for local filesystem paths, causing it to fail when paths were on a remote SSH server.

Root Cause

path.resolve() is a Node.js API that only understands the local filesystem and can't normalize remote SSH paths correctly.

Solution

Delegate path normalization to the Runtime interface instead of having the bash tool understand different runtime types.

  1. Added normalizePath() method to Runtime interface: Each runtime can handle its own path semantics
  2. LocalRuntime implementation: Uses Node.js path.resolve() for local paths
  3. SSHRuntime implementation: Uses POSIX path semantics (no filesystem access required)
  4. Updated bash.ts: Simply delegates to config.runtime.normalizePath()

This maintains better separation of concerns - the bash tool doesn't need to know about SSH vs Local runtime specifics.

Test Coverage

Added 6 comprehensive tests specifically for SSH runtime:

  • ✅ Redundant cd to absolute path (cd /home/user/project)
  • ✅ Redundant cd with relative path (cd .)
  • ✅ Redundant cd with tilde path (cd ~/project)
  • ✅ Redundant cd with single tilde (cd ~)
  • ✅ Trailing slashes in target path (cd /path/)
  • ✅ Trailing slashes in cwd

All 52 tests pass (46 existing + 6 new)

Changes

  • src/runtime/Runtime.ts: Added normalizePath() interface method (+19 lines)
  • src/runtime/LocalRuntime.ts: Implemented for local paths (+10 lines)
  • src/runtime/SSHRuntime.ts: Implemented for remote POSIX paths (+35 lines)
  • src/services/tools/bash.ts: Simplified to delegate to runtime (-59 lines)
  • src/services/tools/bash.test.ts: Added SSH runtime test suite (+136 lines)

Before vs After

Before (Broken)

# SSH Runtime with cwd = "/home/user/project"
Tool Call: bash("cd /home/user/project && ls")
Result: ✗ Would execute with redundant cd (not detected)
Final command: cd "/home/user/project" && cd /home/user/project && ls

After (Fixed)

# SSH Runtime with cwd = "/home/user/project"
Tool Call: bash("cd /home/user/project && ls")
Result: ✅ Error: "Redundant cd to working directory detected"

Benefits

  • ✅ Consistent behavior between LocalRuntime and SSHRuntime
  • ✅ Prevents redundant cd commands in SSH environment
  • ✅ Better separation of concerns (runtime handles its own path logic)
  • ✅ bash.ts is simpler and doesn't need runtime-specific knowledge
  • ✅ No breaking changes to existing functionality

Delegate path normalization to Runtime interface instead of having bash
tool understand different runtime types.

- Add normalizePath() method to Runtime interface
- Implement in LocalRuntime using Node.js path.resolve()
- Implement in SSHRuntime using POSIX path semantics
- Update bash.ts to delegate to runtime.normalizePath()
- Add 6 comprehensive tests for SSH runtime
- All 52 bash tool tests pass

This approach maintains better separation of concerns - the bash tool
doesn't need to know about SSH vs Local runtime specifics.
@ammario ammario added this pull request to the merge queue Oct 26, 2025
Merged via the queue into main with commit 2dc1f47 Oct 26, 2025
13 checks passed
@ammario ammario deleted the rt-redundant-cd branch October 26, 2025 19:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants