Skip to content

Commit f51f3d9

Browse files
committed
🤖 Fix SSH exec timeout to include connection establishment
The timeout in SSHRuntime.exec() was not comprehensive - it only killed the SSH process after the timeout, but didn't configure SSH itself to respect the timeout during connection establishment. This meant that if SSH was hanging during DNS lookup, TCP handshake, or authentication, the command could take much longer than the specified timeout. Changes: - Add ConnectTimeout SSH option set to the configured timeout value - Add ServerAliveInterval=5 and ServerAliveCountMax=2 to detect dead connections quickly (10 second detection window) - These SSH options ensure the timeout is respected from the moment the ssh command starts, not just after connection is established - Add integration test for timeout behavior The fix ensures: 1. Connection establishment can't hang indefinitely 2. Established connections that die are detected quickly 3. The overall timeout is respected comprehensively
1 parent 141f03b commit f51f3d9

File tree

2 files changed

+35
-0
lines changed

2 files changed

+35
-0
lines changed

src/runtime/SSHRuntime.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,22 @@ export class SSHRuntime implements Runtime {
122122
sshArgs.push("-o", `ControlPath=${this.controlPath}`);
123123
sshArgs.push("-o", "ControlPersist=60");
124124

125+
// Set comprehensive timeout options to ensure SSH respects the timeout
126+
// ConnectTimeout: Maximum time to wait for connection establishment (DNS, TCP handshake, SSH auth)
127+
// ServerAliveInterval: Send keepalive every 5 seconds to detect dead connections
128+
// ServerAliveCountMax: Consider connection dead after 2 missed keepalives (10 seconds total)
129+
// Together these ensure that:
130+
// 1. Connection establishment can't hang indefinitely
131+
// 2. Established connections that die are detected quickly
132+
// 3. The overall timeout is respected from the moment ssh command starts
133+
if (options.timeout !== undefined) {
134+
// Use the configured timeout for connection establishment
135+
sshArgs.push("-o", `ConnectTimeout=${Math.ceil(options.timeout)}`);
136+
// Set aggressive keepalives to detect dead connections
137+
sshArgs.push("-o", "ServerAliveInterval=5");
138+
sshArgs.push("-o", "ServerAliveCountMax=2");
139+
}
140+
125141
sshArgs.push(this.config.host, remoteCommand);
126142

127143
// Debug: log the actual SSH command being executed

tests/runtime/runtime.test.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,25 @@ describeIntegration("Runtime integration tests", () => {
142142

143143
expect(result.stdout.trim()).toContain(workspace.path);
144144
});
145+
test.concurrent("handles timeout correctly", async () => {
146+
const runtime = createRuntime();
147+
await using workspace = await TestWorkspace.create(runtime, type);
148+
149+
// Command that sleeps longer than timeout
150+
const startTime = performance.now();
151+
const result = await execBuffered(runtime, "sleep 10", {
152+
cwd: workspace.path,
153+
timeout: 1, // 1 second timeout
154+
});
155+
const duration = performance.now() - startTime;
156+
157+
// Exit code should be EXIT_CODE_TIMEOUT (-998)
158+
expect(result.exitCode).toBe(-998);
159+
// Should complete in around 1 second, not 10 seconds
160+
// Allow some margin for overhead (especially on SSH)
161+
expect(duration).toBeLessThan(3000); // 3 seconds max
162+
expect(duration).toBeGreaterThan(500); // At least 0.5 seconds
163+
});
145164
});
146165

147166
describe("readFile() - File reading", () => {

0 commit comments

Comments
 (0)