Skip to content

Conversation

@ammar-agent
Copy link
Collaborator

Problem

SSHRuntime generated random controlPaths for each instance, preventing SSH ControlMaster from multiplexing connections. This meant every workspace operation opened a new TCP connection to the remote host, even when targeting the same server.

Solution

Replaced random controlPath generation with deterministic hashing based on connection config (host, port, srcBaseDir, identityFile). Identical configs now produce identical controlPaths, enabling SSH ControlMaster to automatically multiplex connections.

Key changes:

  • New sshConnectionPool module with pure function for deterministic controlPath generation (~53 lines)
  • SSHRuntime constructor uses getControlPath(config) instead of random IDs
  • Added ControlMaster options to buildSSHArgs() so git bundle transfers also multiplex
  • Simplified dispose() to no-op since ControlPersist=60 handles cleanup automatically

Design approach:

  • Pure functions only - no state management, no reference counting, no cleanup timers
  • Filesystem is the state (socket files in /tmp managed by SSH)
  • Pool is private implementation detail of SSHRuntime (perfect SoC)

Impact

Performance:

  • 10× fewer SSH connections for bulk operations (e.g., 10 workspaces = 1 connection vs 10)
  • Saves 50-200ms per operation (eliminates SSH handshake overhead)
  • Prevents exhausting SSH MaxSessions or rate limits

Example:

// Before: Each runtime gets random controlPath
const r1 = new SSHRuntime({host: "dev.com", srcBaseDir: "/work"});
// controlPath: /tmp/cmux-ssh-a1b2c3d4 (random)

const r2 = new SSHRuntime({host: "dev.com", srcBaseDir: "/work"});
// controlPath: /tmp/cmux-ssh-e5f6g7h8 (different random!)
// Result: 2 separate TCP connections

// After: Same config produces same controlPath
const r1 = new SSHRuntime({host: "dev.com", srcBaseDir: "/work"});
// controlPath: /tmp/cmux-ssh-abc123def456 (deterministic hash)

const r2 = new SSHRuntime({host: "dev.com", srcBaseDir: "/work"});
// controlPath: /tmp/cmux-ssh-abc123def456 (same hash!)
// Result: 1 shared TCP connection, auto-cleanup after 60s idle

Testing

  • Added 8 unit tests for connection pool (all passing)
  • All 806 existing tests still pass
  • Verified SSHRuntime instances with same config share controlPath
  • Verified buildSSHArgs includes ControlMaster options

Generated with cmux

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

@ammar-agent ammar-agent force-pushed the ssh-connection-pooling branch from 5e66b73 to 6577b12 Compare October 27, 2025 15:20
@ammario ammario changed the title 🤖 Implement SSH connection pooling with pure functions 🤖 Implement SSH connection pooling Oct 27, 2025
Adds deterministic controlPath generation to enable SSH ControlMaster
multiplexing across SSHRuntime instances. Multiple runtimes with identical
configs now share the same TCP connection, reducing connections by ~10x
for bulk operations.

Key changes:
- New sshConnectionPool module with pure function for controlPath generation
- SSHRuntime constructor uses getControlPath() instead of random IDs
- Added ControlMaster options to buildSSHArgs() for bundle transfer multiplexing
- Simplified dispose() to no-op (ControlPersist handles cleanup)

Benefits:
- 10x fewer SSH connections for bulk operations
- 50-200ms saved per operation (no handshake overhead)
- Prevents exhausting SSH connection limits
- Simple design: ~53 lines of pure functions, no state management
…isions

On multi-user systems, different users connecting to the same remote would
generate identical socket paths, causing permission errors. Now includes
os.userInfo().username in the hash to ensure socket isolation per user.

Addresses Codex review comment.
Since ControlPersist=60 handles all socket cleanup automatically
and dispose() is not part of the Runtime interface or called anywhere,
there's no reason to keep an empty method.
@ammar-agent ammar-agent force-pushed the ssh-connection-pooling branch from 6577b12 to 8c80a9c Compare October 27, 2025 15:31
@ammario ammario merged commit 86323fa into main Oct 27, 2025
13 checks passed
@ammario ammario deleted the ssh-connection-pooling branch October 27, 2025 15:37
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