Repository navigation
fix(ssh): no probe stall on the first remote action after a pause - #29240
Conversation
…nswering A remote call after more than 5 s idle first ran a status.get probe on a new one-shot socket (SSH channel, WebSocket upgrade, hello, auth, request). A ready shared control socket that heard from the server in the last 20 s already proves it serves; a kill, reboot, rebind or stop closes that socket, so the probe still runs then.
📝 Walkthrough
Priority: ⬇️ Low Merge Risk: 🟡 Moderate · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 12.50% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 9 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Title check The title clearly and concisely describes the primary SSH behavior change: preventing a probe stall after a pause. Description check The description is complete and well aligned with the repository template. It explains the user impact, mechanism, rationale, alternatives, compatibility, testing, and checklist status. The N/A linked…
- Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 2
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🛠️ Fix failing CI checks: 💡
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
44792a09-62db-4bbd-b408-954e8f2b14f5
📒 Files selected for processing: 8
src/main/ipc/runtime-environment-request-connections.test.tssrc/main/ipc/runtime-environment-request-connections.tssrc/main/ssh/orcad-managed-serving.test.tssrc/main/ssh/orcad-managed-serving.tssrc/main/ssh/orcad-managed-tunnel.tssrc/shared/remote-runtime-shared-control-connection.tssrc/shared/remote-runtime-shared-control-open.tssrc/shared/remote-runtime-shared-control-types.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 3 remain after this review.
| if (answerAge !== null && answerAge >= 0 && answerAge < LIVE_ANSWER_PROOF_MS) { | ||
| return Promise.resolve({ state: 'serving' }) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🔎 Supported by static analysis
🏁 Script executed:
set -o pipefail
printf '%s\n' '--- diff ---'
git diff --find-renames 33759f91f5f5a113d9b2eff5bca6e3a30ec3636e 16033c1d1d38f2d4e5d1023d7b5c95657eb4111c -- src/main/ssh/orcad-managed-serving.ts src/main/ssh src/shared/remote-runtime-shared-control-connection.ts src/shared/remote-runtime-shared-control-test-server.ts src/main/ssh/orcad-managed-serving.test.ts
printf '%s\n' '--- relevant references ---'
rg -n -F --glob '*.ts' -e 'answeredAt' -e 'onAnswered' -e 'autoPong' -e 'status.get' -e 'LIVE_ANSWER_PROOF_MS' src/main/ssh src/shared || test "$?" -eq 1
printf '%s\n' '--- changed implementation ---'
nl -ba src/main/ssh/orcad-managed-serving.ts | sed -n '1,180p'
printf '%s\n' '--- shared connection ---'
nl -ba src/shared/remote-runtime-shared-control-connection.ts | sed -n '1,260p'
printf '%s\n' '--- test server ---'
nl -ba src/shared/remote-runtime-shared-control-test-server.ts | sed -n '1,280p'Repository: stablyai/orca
Length of output: 45010
🏁 Script executed:
set -o pipefail
printf '%s\n' '--- callback and answeredAt definitions ---'
rg -n -F --glob '*.ts' -e 'onActivity' -e 'getRemoteRuntimeSharedControlAnsweredAt' -e 'onAnswered' -e 'answeredAt' src/shared src/main/ipc
printf '%s\n' '--- socket opener and liveness ---'
nl -ba src/shared/remote-runtime-shared-control-open.ts | sed -n '1,280p'
nl -ba src/shared/remote-runtime-shared-control-types.ts | sed -n '50,110p'
rg -n -F --glob '*.ts' -e 'pong' -e 'ping' src/shared/remote-runtime-shared-control-*.ts src/shared/remote-runtime-socket*.ts || test "$?" -eq 1
printf '%s\n' '--- answeredAt owner and changed tests ---'
nl -ba src/main/ipc/runtime-environment-request-connections.ts | sed -n '1,280p'
git diff --find-renames 33759f91f5f5a113d9b2eff5bca6e3a30ec3636e 16033c1d1d38f2d4e5d1023d7b5c95657eb4111c -- src/main/ipc/runtime-environment-request-connections.ts src/main/ipc/runtime-environment-request-connections.test.ts src/shared/remote-runtime-shared-control-connection.test.ts src/shared/remote-runtime-shared-control-open.ts src/shared/remote-runtime-shared-control-types.tsRepository: stablyai/orca
Length of output: 30213
🏁 Script executed:
set -o pipefail
printf '%s\n' '--- repository-owned control server bindings ---'
rg -n -F --glob '*.ts' --glob '*.js' --glob '*.tsx' --glob '*.jsx' -e 'new WebSocketServer' -e 'WebSocketServer(' -e 'autoPong' -e 'status.get' -e 'silentMethods' -e 'createSharedControlTestServer' . || test "$?" -eq 1
printf '%s\n' '--- server/control-related files ---'
rg --files | rg 'remote-runtime|runtime-control|shared-control|websocket|ws' | sed -n '1,240p'Repository: stablyai/orca
Length of output: 41187
Do not treat a pong alone as proof that status.get works.
openSharedControlSocket records pongs as activity, and answeredAt records that activity. A recent timestamp then skips the status.get probe. A server can keep the WebSocket open, answer pings automatically, and stop sending application-level RPC responses. Require application-level response evidence before updating answeredAt, or run the probe when only control-frame activity is available.
orcad-managed-tunnel imported the request-connection cache, which (through the status owner) imports the serving gate back. Tests that mock that cache with an async factory deadlocked importing it. A leaf module now carries the answer source between them.
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
5ea8ba9b-0e21-4ce1-87c4-6568cb7031df
📒 Files selected for processing: 4
src/main/ipc/runtime-environment-request-connections.test.tssrc/main/ipc/runtime-environment-request-connections.tssrc/main/ipc/runtime-environment-shared-control-answers.tssrc/main/ssh/orcad-managed-tunnel.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review.
| setSharedControlAnswerSource(environmentId, () => | ||
| entry.connection.getDiagnostics().state === 'ready' ? entry.answeredAt() : null | ||
| ) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
sed -n '1,210p' src/main/ssh/orcad-managed-serving.ts
sed -n '1,130p' src/main/ssh/orcad-managed-tunnel.ts
rg -n 'ensureServing\\(|ensureManagedOrcadServing|machineNameReady\\(' src/main/ssh src/main/runtime/rpc/methods/status.tsRepository: stablyai/orca
Length of output: 11131
🏁 Script executed:
set -o pipefail
printf '%s\n' '--- probe and caller references ---'
rg -n -F --glob '*.ts' \
-e 'probeManagedOrcadTunnel' \
-e 'ensureServing' \
-e 'status.get' \
-e 'machineNameReady' \
-e 'ensureOrcadManagedTunnel' \
-e 'startOrcadManagedTunnel' \
src/main/ssh src/main/runtime/rpc src/main/ipc || test "$?" -eq 1
printf '%s\n' '--- probe implementation ---'
sed -n '1,240p' src/main/ssh/orcad-managed-tunnel-resume.ts
printf '%s\n' '--- tunnel manager serving/start flow ---'
sed -n '1,320p' src/main/ssh/orcad-managed-tunnel-manager.ts
printf '%s\n' '--- status RPC ---'
sed -n '1,180p' src/main/runtime/rpc/methods/status.ts
printf '%s\n' '--- websocket lifecycle ---'
sed -n '1,280p' src/main/runtime/rpc/node-websocket-lifecycle.tsRepository: stablyai/orca
Length of output: 42989
🏁 Script executed:
set -o pipefail
printf '%s\n' '--- probe and caller references ---'
rg -n -F --glob '*.ts' -e 'probeManagedOrcadTunnel' -e 'ensureServing' -e 'status.get' -e 'machineNameReady' -e 'ensureOrcadManagedTunnel' -e 'startOrcadManagedTunnel' src/main/ssh src/main/runtime/rpc src/main/ipc || test "$?" -eq 1
printf '%s\n' '--- probe implementation ---'
sed -n '1,240p' src/main/ssh/orcad-managed-tunnel-resume.ts
printf '%s\n' '--- tunnel manager serving/start flow ---'
sed -n '1,320p' src/main/ssh/orcad-managed-tunnel-manager.ts
printf '%s\n' '--- status RPC ---'
sed -n '1,180p' src/main/runtime/rpc/methods/status.ts
printf '%s\n' '--- websocket lifecycle ---'
sed -n '1,280p' src/main/runtime/rpc/node-websocket-lifecycle.tsRepository: stablyai/orca
Length of output: 42263
🏁 Script executed:
printf '%s\n' '--- probe implementation ---'
sed -n '248,295p' src/main/ssh/orcad-managed-tunnel-resume.ts
printf '%s\n' '--- serving check ---'
sed -n '1,130p' src/main/ssh/orcad-managed-tunnel-serving.ts
printf '%s\n' '--- immediate SSH action caller ---'
sed -n '110,195p' src/main/ssh/runtime-ssh-access.ts
printf '%s\n' '--- probe transport helper definitions ---'
rg -n -F --glob '*.ts' -e 'sendRemoteRuntimeRequest' -e 'callRuntimeEnvironment' -e 'probeManagedOrcadTunnel' src/main/ssh src/main/ipcRepository: stablyai/orca
Length of output: 32870
🏁 Script executed:
printf '%s\n' '--- request connection routing ---'
sed -n '35,85p' src/main/ipc/runtime-environment-request-connections.ts
printf '%s\n' '--- managed tunnel request handler ---'
sed -n '1,75p' src/main/ipc/runtime-environment-managed-tunnel.ts
printf '%s\n' '--- managed serving call site ---'
sed -n '1,65p' src/main/ssh/orcad-managed-tunnel-serving.tsRepository: stablyai/orca
Length of output: 7489
Keep status.get as the serving check.
answeredAt records transport activity, not an application response. A recent timestamp returns serving before probeManagedOrcadTunnel sends status.get. If that probe times out, it returns false and enters wakeIfStopped, which can start a stopped server or return unverifiable for a live server that does not answer. The timestamp shortcut bypasses both outcomes, so resolveManagedRuntimeEnvironment can continue with the remote call on an unresponsive runtime.
Use an authoritative serving response for this shortcut, or retain the status.get probe while answeredAt represents only socket activity.
1 participant
ELI5
Before Orca sends anything to a remote SSH host, it checks that the host's managed Orca server is running. It skipped the check only when the last one was under 5 seconds old. So the first click after any pause longer than 5 seconds waited for a whole separate check first. Orca already keeps an open line to that server, and that line hears from the server at least every 10 seconds. After this PR, a recent answer on that line counts as the check.
What Changed
Before: every remote call and subscription waits on
verifyOrcadManagedServing(runtime-environment-managed-tunnel.ts). After more than 5 s idle (VERDICT_REUSE_MS), the wait sends astatus.getprobe on a new one-shot socket: SSH channel, WebSocket upgrade, hello, auth, then the request. That is about 5 round trips before the user's own request goes out. At 150 ms latency, the repro rig measured Subscribe at 1251–1309 ms with a stale verdict, against 478–492 ms with a fresh one (about 800 ms added). On exec/stdio-bridge hosts each new socket is also a freshsshexec, and those count against the bridge's 8-socket limit.After: while the shared control socket for that environment is ready and heard from its server (any frame, ping or pong) in the last 20 s, the check answers "serving" with no probe and no new socket. In every other case the old check runs unchanged: no control socket, socket not ready, last answer older than 20 s (for example after a sleep), or a check already in flight on this transport.
Mechanism:
remote-runtime-shared-control-open.ts: the existing liveness hook (noteActivity) also calls a new optionalonActivitycallback.runtime-environment-request-connections.ts: each cached shared control connection stampsDate.now()on activity. It registers a source that returns that stamp only while the connection isready, and clears it on close.runtime-environment-shared-control-answers.ts(new, no imports): holds those sources so the serving gate can read them. The gate can't import the connection cache directly, because that cache imports the gate back (through the status owner). In tests that mock the cache with an async factory, that loop deadlocked module loading.orcad-managed-serving.ts:ensureManagedOrcadServingtakes an optionalansweredAtsource and treats an answer in the last 20 s as proof of serving (2× the 10 s ping interval). It joins an in-flight check first, so concurrent callers still share one probe.Why
The check (#24863, made blocking by #26847) exists so that a server that stopped is started before a call needs it. Stops include idling out, being killed, a host reboot, a move to another port, and another desktop stopping it. Every one of those ends the server process, and that closes the control socket. A ready socket that heard from the server seconds ago is direct evidence that the server is serving, which is what the probe was trying to learn.
What stays safe:
unverifiable. Losing contact is never read as the server or its terminals havingexited.Alternatives considered:
status.getandsession.tabs.listover the shared control socket: rejected.shouldRouteCallBySupportkeeps them off it on purpose, so snapshot recovery still works while that socket reconnects.Compatibility: nothing goes over the wire differently, so mixed client and server versions are unaffected. The change behaves the same on port-forward tunnels and exec/stdio-bridge hosts, and on every OS.
Linked Issue
N/A (internal SSH latency regression; maintainer PR).
Visual Proof
N/A. Nothing on screen changes. Only the delay before the first remote action after a pause changes; the measured timings are above.
Testing
pnpm test src/main/ssh/orcad-managed-serving.test.ts src/main/ipc/runtime-environment-request-connections.test.ts src/shared/remote-runtime-shared-control-connection.test.tspnpm tcandpnpm run check:code-quality:changedpass.Measured on a real orcad over SSH (Docker, 150 ms added latency). Each round: leave a parked worktree for 7 s, then switch back. The client was rebuilt from this branch and launched with
ORCA_BACKGROUND_LAUNCH=1.Automated tests added/updated
Agent skill upstream boundary
Notes
Ensure no issues in: Security, Cross-platoform support (Linux, Windows, Mac), Remote SSH, Mobile, general backwards compatibility, performance
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)