Repository navigation
fix(ssh): SSH environments connect over links with about a second of round trip - #35418
Conversation
ApprovabilityVerdict: Needs Human Approval Macroscope's review found this PR requires human approval — SSH tunnel readiness probes now default to 5 seconds instead of 1 second to accommodate high-latency links, while overall readiness budgets remain unchanged. A human must accept this change to default connection behavior. You can add or adjust custom eligibility rules. Learn more. |
|
No actionable comments were generated in the recent review. Walkthrough
Merge Risk:
|
Thread transfer impact✅ Thread transfer remains within every enforced ceiling.
Other scenariosAll 64 scenario measurements match All 64 scenario measurements
Baseline: Scenario and decoded snapshot size10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.
Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed. |
…round trip (pingdotgg#35418) (cherry picked from commit c58450f)
Each SSH tunnel readiness probe (an HTTP GET through the local ssh -L forward) was capped at SSH_READY_PROBE_TIMEOUT_MS = 1000ms. At about 1s RTT one request through the forward takes several round trips (3-4s measured), so every probe timed out inside the 20s budget and the environment never connected. The tunnel-side waitForHttpReady wrapper in packages/ssh/src/tunnel.ts now defaults to a new SSH_TUNNEL_READY_PROBE_TIMEOUT_MS = 5000. The overall budgets (20s first connect, 2s check of an existing tunnel) are unchanged, and a fast link still succeeds on its first probe. SSH_READY_PROBE_TIMEOUT_MS (1s) still drives the probe inside the remote launch script, which runs on the remote host and does not cross the link, and the launch-lock wait arithmetic. The shared httpReadiness default is untouched. Reproduced first with the repro branch test (fails with 'Backend readiness probe exceeded 1000ms'), which passes with the fix. No new regex or parser.
Fixes #7733
Reproduction on main
Commit f03db4f (origin/main). Built a real high-latency SSH host: a network namespace joined to the host by a veth pair, with tc netem adding 500ms each way on the veth ends only (ping RTT 1000ms), a dedicated sshd on 10.77.0.2:2222, a throwaway remote user with Node, and the published t3 0.1.9 linux-x64 release archive served from a mirror inside the namespace. A scratch driver ran the desktop's real SshEnvironmentManager.ensureEnvironment (NodeServices, NodeCrypto, NodeHttpClient.layerUndici, NetService, as the desktop main process wires it) with archiveVersion 0.1.9; the system ssh got its config through a PATH wrapper. Baseline with no added latency: connected, managed remote server, pairing token issued. At 1000ms RTT: failed after 56s with ssh.tunnel.ready.failed (processRunning: true, localPortListening: true) and SshReadinessError 'Timed out waiting 20000ms for backend readiness' caused by 'Backend readiness probe exceeded 1000ms' (19 attempts). The remote server stayed up and answered 200 locally in 2ms. curl through an identical manual ssh -L tunnel returned 200 in 4.0s, 3.0s and 3.0s. A unit test in packages/ssh/src/tunnel.test.ts (HTTP responses taking 2.5s through the tunnel) fails on main with the same timeout message, and passed with the per-probe cap raised to 5s (reverted).
Evidence
In each separately built Linux Electron desktop package, open Settings → Environments → Add environment → SSH, enter slow-link / t3latency7733 / port 2222, and click Add environment. Wait for the readiness error or the saved row to say Connected. Both runs used fresh synthetic userdata, the published remote t3 0.1.9 runtime, light theme, a 1100×780 window, and the same real OpenSSH host with 500 ms netem delay in each direction (measured RTT 1000.069 ms).
UI driver · SSH host setup · Reproduction details · Runtime summary · Redacted original SSH events
Control: the healthy remote returned HTTP 200 locally in 0.001629 s and through a separate real SSH tunnel in 3.003298 s. Base diagnostics showed processRunning=true, localPortListening=true, and probes exceeding the 1000 ms cap. Both final stills were identical 500 ms apart; both recordings were inspected through their ending.
Remaining limitation: head's existing-tunnel reuse check still has a 2000 ms total deadline. It rejected the healthy tunnel and recreated it before the client reached Connected. This proves eventual opening at this RTT, not seamless tunnel reuse.
AI-assisted; verified with the tests and tools listed above.
Not shown:
Tests
cd packages/ssh && vp test run src/tunnel.test.ts -t 'longer than a second': failed before the fix (probe exceeded 1000ms), passes after. vp test run src/ (packages/ssh): 45/46 pass; the one failure, 'concurrent launches on one remote home record the server that owns it' (5s test timeout), also fails on the base commit without this change and when run alone on this VM, and passed inside check:changed. vp run check:changed: format, lint, knip, typecheck (5 packages), tests (2 files, 25 tests) all pass.
Not in this change
AI-assisted; verified with the tests and tools listed above.