refactor: default SSH acquireConnection to wait

Author: ammar-agent

src/node/runtime/Runtime.ts

@@ -65,16 +65,6 @@ export interface ExecOptions {
   abortSignal?: AbortSignal;
   /** Force PTY allocation (SSH only - adds -t flag) */
   forcePTY?: boolean;
-
-  /**
-   * SSH only: if set (>0), wait through SSHConnectionPool backoff (bounded).
-   *
-   * If not set, SSH operations fail fast when the host is in backoff.
-   */
-  connectionMaxWaitMs?: number;
-
-  /** SSH only: invoked when waiting due to SSH backoff. */
-  onConnectionWait?: (waitMs: number) => void;
 }
 
 /**

src/node/runtime/SSHRuntime.ts

@@ -47,17 +47,6 @@ function logSSHBackoffWait(initLogger: InitLogger, waitMs: number): void {
 }
 
 // Re-export SSHRuntimeConfig from connection pool (defined there to avoid circular deps)
-const USER_INITIATED_SSH_MAX_WAIT_MS = 2 * 60 * 1000;
-
-function userInitiatedSSHWaitExecOptions(
-  initLogger: InitLogger
-): Pick<ExecOptions, "connectionMaxWaitMs" | "onConnectionWait"> {
-  return {
-    connectionMaxWaitMs: USER_INITIATED_SSH_MAX_WAIT_MS,
-    onConnectionWait: (waitMs) => logSSHBackoffWait(initLogger, waitMs),
-  };
-}
-
 export type { SSHRuntimeConfig } from "./sshConnectionPool";
 
 /**
@@ -140,15 +129,9 @@ export class SSHRuntime implements Runtime {
 
     // Ensure connection is healthy before executing.
     // This provides backoff protection and singleflighting for concurrent requests.
-    if (options.connectionMaxWaitMs !== undefined && options.connectionMaxWaitMs > 0) {
-      await sshConnectionPool.acquireConnection(this.config, {
-        maxWaitMs: options.connectionMaxWaitMs,
-        abortSignal: options.abortSignal,
-        onWait: options.onConnectionWait,
-      });
-    } else {
-      await sshConnectionPool.acquireConnection(this.config);
-    }
+    await sshConnectionPool.acquireConnection(this.config, {
+      abortSignal: options.abortSignal,
+    });
 
     // Build command parts
     const parts: string[] = [];
@@ -649,7 +632,6 @@ export class SSHRuntime implements Runtime {
 
       // Step 2: Ensure the SSH host is reachable before doing expensive local work
       await sshConnectionPool.acquireConnection(this.config, {
-        maxWaitMs: USER_INITIATED_SSH_MAX_WAIT_MS,
         abortSignal,
         onWait: (waitMs) => logSSHBackoffWait(initLogger, waitMs),
       });
@@ -718,7 +700,6 @@ export class SSHRuntime implements Runtime {
         cwd: "~",
         timeout: 300, // 5 minutes for clone
         abortSignal,
-        ...userInitiatedSSHWaitExecOptions(initLogger),
       });
 
       const [cloneStdout, cloneStderr, cloneExitCode] = await Promise.all([
@@ -741,7 +722,6 @@ export class SSHRuntime implements Runtime {
           cwd: "~",
           timeout: 30,
           abortSignal,
-          ...userInitiatedSSHWaitExecOptions(initLogger),
         }
       );
       await createTrackingBranchesStream.exitCode;
@@ -756,7 +736,6 @@ export class SSHRuntime implements Runtime {
             cwd: "~",
             timeout: 10,
             abortSignal,
-            ...userInitiatedSSHWaitExecOptions(initLogger),
           }
         );
 
@@ -775,7 +754,6 @@ export class SSHRuntime implements Runtime {
             cwd: "~",
             timeout: 10,
             abortSignal,
-            ...userInitiatedSSHWaitExecOptions(initLogger),
           }
         );
         await removeOriginStream.exitCode;
@@ -787,7 +765,6 @@ export class SSHRuntime implements Runtime {
         cwd: "~",
         timeout: 10,
         abortSignal,
-        ...userInitiatedSSHWaitExecOptions(initLogger),
       });
 
       const rmExitCode = await rmStream.exitCode;
@@ -803,7 +780,6 @@ export class SSHRuntime implements Runtime {
           cwd: "~",
           timeout: 10,
           abortSignal,
-          ...userInitiatedSSHWaitExecOptions(initLogger),
         });
         await rmStream.exitCode;
       } catch {
@@ -842,7 +818,6 @@ export class SSHRuntime implements Runtime {
       timeout: 3600, // 1 hour - generous timeout for init hooks
       abortSignal,
       env: muxEnv,
-      ...userInitiatedSSHWaitExecOptions(initLogger),
     });
 
     // Create line-buffered loggers
@@ -916,7 +891,6 @@ export class SSHRuntime implements Runtime {
           cwd: "/tmp",
           timeout: 10,
           abortSignal,
-          ...userInitiatedSSHWaitExecOptions(initLogger),
         });
         const mkdirExitCode = await mkdirStream.exitCode;
         if (mkdirExitCode !== 0) {
@@ -979,7 +953,6 @@ export class SSHRuntime implements Runtime {
         cwd: workspacePath, // Use the full workspace path for git operations
         timeout: 300, // 5 minutes for git checkout (can be slow on large repos)
         abortSignal,
-        ...userInitiatedSSHWaitExecOptions(initLogger),
       });
 
       const [stdout, stderr, exitCode] = await Promise.all([
@@ -1044,7 +1017,6 @@ export class SSHRuntime implements Runtime {
         cwd: workspacePath,
         timeout: 120, // 2 minutes for network operation
         abortSignal,
-        ...userInitiatedSSHWaitExecOptions(initLogger),
       });
 
       const fetchExitCode = await fetchStream.exitCode;
@@ -1064,7 +1036,6 @@ export class SSHRuntime implements Runtime {
         cwd: workspacePath,
         timeout: 60, // 1 minute for fast-forward merge
         abortSignal,
-        ...userInitiatedSSHWaitExecOptions(initLogger),
       });
 
       const [mergeStderr, mergeExitCode] = await Promise.all([

src/node/runtime/sshConnectionPool.test.ts

@@ -193,7 +193,9 @@ describe("SSHConnectionPool", () => {
       };
 
       // Trigger a failure via acquireConnection (will fail to connect)
-      await expect(pool.acquireConnection(config, 1000)).rejects.toThrow();
+      await expect(
+        pool.acquireConnection(config, { timeoutMs: 1000, maxWaitMs: 0 })
+      ).rejects.toThrow();
 
       // Verify we're now in backoff
       const healthBefore = pool.getConnectionHealth(config);
@@ -231,7 +233,7 @@ describe("SSHConnectionPool", () => {
       expect(elapsed).toBeLessThan(50);
     });
 
-    test("wait mode waits through backoff (bounded) instead of throwing", async () => {
+    test("waits through backoff (bounded) instead of throwing", async () => {
       const pool = new SSHConnectionPool();
       const config: SSHRuntimeConfig = {
         host: "test.example.com",
@@ -246,7 +248,6 @@ describe("SSHConnectionPool", () => {
       const onWaitCalls: number[] = [];
 
       await pool.acquireConnection(config, {
-        maxWaitMs: 60_000,
         onWait: (ms) => {
           onWaitCalls.push(ms);
         },
@@ -272,10 +273,12 @@ describe("SSHConnectionPool", () => {
       };
 
       // Trigger a failure to put connection in backoff
-      await expect(pool.acquireConnection(config, 1000)).rejects.toThrow();
+      await expect(
+        pool.acquireConnection(config, { timeoutMs: 1000, maxWaitMs: 0 })
+      ).rejects.toThrow();
 
       // Second call should throw immediately with backoff message
-      await expect(pool.acquireConnection(config)).rejects.toThrow(/in backoff/);
+      await expect(pool.acquireConnection(config, { maxWaitMs: 0 })).rejects.toThrow(/in backoff/);
     });
 
     test("getControlPath returns deterministic path", () => {
@@ -303,9 +306,9 @@ describe("SSHConnectionPool", () => {
 
       // All concurrent calls should share the same probe and get same result
       const results = await Promise.allSettled([
-        pool.acquireConnection(config, 1000),
-        pool.acquireConnection(config, 1000),
-        pool.acquireConnection(config, 1000),
+        pool.acquireConnection(config, { timeoutMs: 1000, maxWaitMs: 0 }),
+        pool.acquireConnection(config, { timeoutMs: 1000, maxWaitMs: 0 }),
+        pool.acquireConnection(config, { timeoutMs: 1000, maxWaitMs: 0 }),
       ]);
 
       // All should be rejected (connection fails)

src/node/runtime/sshConnectionPool.ts

@@ -65,14 +65,17 @@ const BACKOFF_SCHEDULE = [1, 5, 10, 20, 40, 60];
 const HEALTHY_TTL_MS = 15 * 1000; // 15 seconds
 
 const DEFAULT_PROBE_TIMEOUT_MS = 10_000;
+const DEFAULT_MAX_WAIT_MS = 2 * 60 * 1000; // 2 minutes
 
 export interface AcquireConnectionOptions {
   /** Timeout for the health check probe. */
   timeoutMs?: number;
 
   /**
-   * If set (>0), acquireConnection will wait through backoff (bounded by maxWaitMs)
-   * instead of throwing immediately.
+   * Max time to wait (ms) for a host to become healthy (waits + probes).
+   *
+   * - Omit to use the default (waits through backoff).
+   * - Set to 0 to fail fast.
    */
   maxWaitMs?: number;
 
@@ -136,12 +139,10 @@ export class SSHConnectionPool {
   /**
    * Ensure connection is healthy before proceeding.
    *
-   * The pool is intentionally conservative by default (fail-fast) to avoid turning
-   * an SSH outage into app-wide latency regressions.
+   * By default, acquireConnection waits through backoff (bounded) so user-facing
+   * actions don’t immediately fail during transient SSH outages.
    *
-   * For user-initiated flows where waiting is preferable to an immediate error
-   * (workspace init, terminal spawn, explicit user commands), callers can opt in
-   * to waiting through backoff by providing `maxWaitMs`.
+   * Callers can opt into fail-fast behavior by passing `{ maxWaitMs: 0 }`.
    */
   async acquireConnection(config: SSHRuntimeConfig, timeoutMs?: number): Promise<void>;
   async acquireConnection(
@@ -160,8 +161,8 @@ export class SSHConnectionPool {
     const timeoutMs = options.timeoutMs ?? DEFAULT_PROBE_TIMEOUT_MS;
     const sleep = options.sleep ?? sleepWithAbort;
 
-    const shouldWait = options.maxWaitMs !== undefined && options.maxWaitMs > 0;
-    const maxWaitMs = options.maxWaitMs ?? 0;
+    const maxWaitMs = options.maxWaitMs ?? DEFAULT_MAX_WAIT_MS;
+    const shouldWait = maxWaitMs > 0;
 
     const key = makeConnectionKey(config);
     const startTime = Date.now();

src/node/services/ptyService.ts

@@ -216,7 +216,7 @@ export class PTYService {
 
       // Ensure connection is healthy before spawning terminal
       // This provides backoff protection and singleflighting for concurrent requests
-      await sshConnectionPool.acquireConnection(sshConfig, { maxWaitMs: 30_000 });
+      await sshConnectionPool.acquireConnection(sshConfig);
 
       const sshArgs = buildSSHArgs(sshConfig, workspacePath);