Skip to content

Commit ea205ab

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 86323fa commit ea205ab

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
@@ -121,6 +121,22 @@ export class SSHRuntime implements Runtime {
121121
sshArgs.push("-o", `ControlPath=${this.controlPath}`);
122122
sshArgs.push("-o", "ControlPersist=60");
123123

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

126142
// 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)