Repository navigation
feat(wsl): liveness, failure classification and recovery for WSL orcad (WSL-via-orcad C6) - #27916
Conversation
67bff99 to
5e76a1b
Compare
9ac0ba7 to
2b419a3
Compare
2b419a3 to
caf2132
Compare
3fb1697 to
4e55a67
Compare
caf2132 to
7d5d2c4
Compare
4e55a67 to
6837a61
Compare
7d5d2c4 to
4e4aa55
Compare
📝 Walkthrough
Priority: ➖ Normal Merge Risk: 🔵 Low · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 38.64% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 44 functions across 20 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 identifies the main change: WSL Orcad liveness monitoring, failure classification, and recovery. It is concise and related to the changeset. Description check The description is detailed and covers the required explanation, changes, rationale, testing, visual proof, checklist, and known limitations. The linked issue section names the internal C6 tracker but…
- 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: 2
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
a0a68e47-6eb6-4cc4-b3e4-07c01d2282dc
📒 Files selected for processing (20)
src/main/ssh/orcad-terminal-census-client.tssrc/main/wsl-background-distro-gate.tssrc/main/wsl/wsl-distro-holder.test.tssrc/main/wsl/wsl-distro-holder.tssrc/main/wsl/wsl-guest-process-identity.test.tssrc/main/wsl/wsl-guest-process-identity.tssrc/main/wsl/wsl-orcad-connection-status.tssrc/main/wsl/wsl-orcad-connection.test.tssrc/main/wsl/wsl-orcad-connection.tssrc/main/wsl/wsl-orcad-liveness-model.tssrc/main/wsl/wsl-orcad-liveness-recovery.tssrc/main/wsl/wsl-orcad-liveness-supervisor.test.tssrc/main/wsl/wsl-orcad-liveness-supervisor.tssrc/main/wsl/wsl-orcad-liveness-verdict.test.tssrc/main/wsl/wsl-orcad-liveness-verdict.tssrc/main/wsl/wsl-orcad-liveness-wiring.tssrc/main/wsl/wsl-orcad-reconnect-backoff.test.tssrc/main/wsl/wsl-orcad-reconnect-backoff.tssrc/main/wsl/wsl-orcad-watch.test.tssrc/main/wsl/wsl-orcad-watch.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.
| const connection = this.deps.manager.get(entry.distro) | ||
| await gatherWslLivenessEvidence(evidence, { | ||
| readIdentity: () => this.deps.readIdentity(entry.distro, connection?.pid ?? null), | ||
| // Opening a watch is the port probe; a held one already proved the port. | ||
| probePort: async () => | ||
| connection && !entry.watch ? this.attachWatch(entry, connection) : 'ready' | ||
| }) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Do not report the port as ready when no connection exists.
probePort returns 'ready' in two cases. One is when a held watch exists, which is correct. The other is when connection is null, which is not. The port probe runs only when expected.orcad is null, so no pid identity exists to compare. If manager.get returns null in that state, classifyWslOrcadLiveness returns live. This can happen after the manager closes the connection, or while a port-move replacement is in progress (connection.ts Line 222). act then publishes ready and schedules a heartbeat, although nothing proved that orcad answers. The supervisor reports a connection that does not exist. Return 'unverifiable' when no connection exists. The retry loop then checks again.
🐛 Proposed fix
// Opening a watch is the port probe; a held one already proved the port.
- probePort: async () =>
- connection && !entry.watch ? this.attachWatch(entry, connection) : 'ready'
+ probePort: async () => {
+ if (entry.watch) {
+ return 'ready'
+ }
+ return connection ? this.attachWatch(entry, connection) : 'unverifiable'
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const connection = this.deps.manager.get(entry.distro) | |
| await gatherWslLivenessEvidence(evidence, { | |
| readIdentity: () => this.deps.readIdentity(entry.distro, connection?.pid ?? null), | |
| // Opening a watch is the port probe; a held one already proved the port. | |
| probePort: async () => | |
| connection && !entry.watch ? this.attachWatch(entry, connection) : 'ready' | |
| }) | |
| const connection = this.deps.manager.get(entry.distro) | |
| await gatherWslLivenessEvidence(evidence, { | |
| readIdentity: () => this.deps.readIdentity(entry.distro, connection?.pid ?? null), | |
| // Opening a watch is the port probe; a held one already proved the port. | |
| probePort: async () => { | |
| if (entry.watch) { | |
| return 'ready' | |
| } | |
| return connection ? this.attachWatch(entry, connection) : 'unverifiable' | |
| } | |
| }) |
| function transportFor(distro: string, environmentId = defaultWslOrcadEnvironmentId(distro)) { | ||
| return createWslOrcadHostTransport({ | ||
| target: { kind: 'wsl', id: environmentId, distro, label: distro } | ||
| }) | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
#!/bin/bash
fd -i 'wsl-command-loop' src | xargs -I{} sh -c 'echo "== {}"; cat -n {}' | head -200
rg -nP -C3 '\bcreateWslCommandLoopPool\s*\(' src
rg -nP -C3 'detectRemoteHostPlatform\s*\(' src/main/ssh | head -40Repository: stablyai/orca
Length of output: 12583
🏁 Script executed:
set -o pipefail
printf '%s\n' '== wiring =='
nl -ba src/main/wsl/wsl-orcad-liveness-wiring.ts
printf '%s\n' '== host transport =='
nl -ba src/main/wsl/wsl-orcad-host-transport.ts
printf '%s\n' '== liveness references =='
rg -n -F --glob '*.ts' -e 'transportFor(' -e 'readIdentity' -e 'openWatch' -e 'watch.close' src/main/wsl src/main/ssh
printf '%s\n' '== watch implementation =='
rg -n -F --glob '*.ts' -e 'createWslOrcadConnectionWatch' -e 'openWslOrcadConnectionWatch' -e 'ConnectionWatch' src/main/wsl src/main/ssh
printf '%s\n' '== changed-file diff =='
git diff --no-ext-diff --unified=40 b978bc4dc266432b12bd06bb0d69d77308093597 4e4aa55f7d65d50468b737208805617f6815b8d4 -- src/main/wsl/wsl-orcad-liveness-wiring.tsRepository: stablyai/orca
Length of output: 15053
🏁 Script executed:
set -o pipefail
nl -ba src/main/wsl/wsl-orcad-liveness-wiring.ts
nl -ba src/main/wsl/wsl-orcad-host-transport.ts
rg -n -C 8 -F --glob '*.ts' -e 'transportFor(' -e 'openWatch(' -e 'readIdentity(' -e 'watch.close' src/main/wsl src/main/ssh
rg -n -C 12 -F --glob '*.ts' -e 'createWslOrcadConnectionWatch' -e 'openWslOrcadConnectionWatch' -e 'ConnectionWatch' src/main/wsl src/main/sshRepository: stablyai/orca
Length of output: 23536
🏁 Script executed:
set -o pipefail
printf '%s\n' '== WSL watch =='
nl -ba src/main/wsl/wsl-orcad-watch.ts
printf '%s\n' '== bridge entrypoints =='
rg -n -C 12 -F --glob '*.ts' -e 'openOrcadStdioBridgeChannel' -e 'resolveOrcadStdioBridge' -e 'watchChannel' src/main/ssh src/main/wsl
printf '%s\n' '== bridge implementations =='
rg -n --files src/main/ssh src/main/wsl | rg '(stdio|watch|channel)'Repository: stablyai/orca
Length of output: 30781
🏁 Script executed:
set -o pipefail
printf '%s\n' '== guest identity =='
nl -ba src/main/wsl/wsl-guest-process-identity.ts
printf '%s\n' '== execCommand =='
rg -n -C 12 -F --glob '*.ts' -e 'export async function execCommand' -e 'function execCommand' -e 'execWithClosedStdin' src/main/ssh/ssh-relay-exec-command.ts src/main/ssh
printf '%s\n' '== platform detection =='
nl -ba src/main/ssh/ssh-remote-platform-detection.tsRepository: stablyai/orca
Length of output: 27440
Disable pooled command loops for short-lived liveness probes.
transportFor creates a new pooled transport for each identity read and watch setup. A completed probe can leave its wsl.exe command loop alive for up to 5 seconds. Separate transports can retain overlapping loops.
The watch's long-lived bridge uses transport.exec and is closed by watch.close(). The issue is the idle probe loop, not the bridge lifetime.
🔧 Suggested fix
--- "a/src/main/wsl/wsl-orcad-liveness-wiring.ts"
+++ "b/src/main/wsl/wsl-orcad-liveness-wiring.ts"
@@ -22,11 +22,12 @@
daemonProtocolVersion: null
}
function transportFor(distro: string, environmentId = defaultWslOrcadEnvironmentId(distro)) {
return createWslOrcadHostTransport({
- target: { kind: 'wsl', id: environmentId, distro, label: distro }
+ target: { kind: 'wsl', id: environmentId, distro, label: distro },
+ commandLoops: false
})
}
/** Every session counts (activation time 0): liveness asks how many survived, not how new. */
function collectWslOrcadCensus(connection: WslOrcadConnection): Promise<OrcadTerminalCensus> {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function transportFor(distro: string, environmentId = defaultWslOrcadEnvironmentId(distro)) { | |
| return createWslOrcadHostTransport({ | |
| target: { kind: 'wsl', id: environmentId, distro, label: distro } | |
| }) | |
| } | |
| function transportFor(distro: string, environmentId = defaultWslOrcadEnvironmentId(distro)) { | |
| return createWslOrcadHostTransport({ | |
| target: { kind: 'wsl', id: environmentId, distro, label: distro }, | |
| commandLoops: false | |
| }) | |
| } |
Stacked on #27834 (C5: connecting to a WSL distro's orcad over
wsl.exestdio). Do not merge.ELI5
After C5, the desktop can talk to the Orca server (orcad) inside a WSL distro. It could not yet tell when that server or its distro went away, or bring it back. Now it notices within about 150 ms, works out what happened, and either reconnects or reports that the terminals are gone.
It only reports terminals as gone when Windows can prove the distro stopped or restarted. A check that fails or can't be read is reported as "unverifiable", never as "exited". It also never starts a stopped distro for background work.
What Changed
Watch (
wsl-orcad-watch.ts). A single bridge into orcad's port that stays open and never sends anything. When it ends, its exit code is the trigger: 0 means orcad closed the connection, 1 means the distro stopped. Opening a watch also works as a port probe, which answers READY or REFUSED.Classifier (
wsl-orcad-liveness-verdict.ts, pure). Exit codes from the bridge and the holder only start a check. They are never treated as proof. The verdict comes from:wsl --list --runningread, via the newreadConfirmedRunningWslDistros. Unlike the fail-closed list, a failed probe here comes back asunavailable;wsl-guest-process-identity.ts): the boot id, pid 1's start ticks, and orcad's pid plus start ticks, all from/proc/<pid>/stat. None of these use wall-clock time.unverifiabledistro-stoppeddistro-restartedorcad-exitedliveunverifiableThe classifier never runs a command inside a distro that is not confirmed running, because that would boot it. After a failed exit, it waits 500 ms and reads the list again, because a distro that is still stopping can show as running.
Recovery (
wsl-orcad-liveness-recovery.ts,wsl-orcad-liveness-supervisor.ts).failed. Before reportingready, it runs theorcad.terminalCensusRPC. While this runs, terminals stayunverifiableand are never marked exited.distro-stoppedand registers with the existingdeferUntilWslDistroRunninggate.idleand does not relaunch, so orcad's managed 15-minute idle exit isn't undone.connecttakes over from any recovery in progress.Timers. Heartbeats and backoff use
performance.now(). The heartbeat runs every 120 s and only reads the running list. A heartbeat that fires more than twice its interval late triggers a full re-check. On power resume (subscribeSystemPowerLifecycle, which is fed bypowerMonitor), it re-checks instead of trusting its timers.Status (
wsl-orcad-connection-status.ts). C5'sonChange(distro, connection, status)now has a third argument,status. Its states areconnecting,ready(with the census),reconnecting,distro-stopped,unverifiable,idle,failed, andclosed. Every non-ready state carries aterminalsverdict.Holder (C3's open question). If the recorded holder stays
unverifiablefor 5 minutes (monotonic time) while its distro is confirmed running,ensureHeldspawns a replacement. A process table that has failed for that long isn't a hiccup, and a distro running all that time means something is holding it, so waiting is free. A list read that fails still never doubles the holder. Boot and release deadlines are now monotonic too.onHolderExitreports when a holder we spawned exits.Census.
requestOrcadTerminalCensus(connection, …)is extracted from the SSH census client, and the SSH path calls it.Wiring.
createWslOrcadLivenessSupervisor({ manager, holder, hasUserVisibleWork })wires up the production probes. C7 supplieshasUserVisibleWorkand the app wiring.Why WSL can prove an exit when SSH cannot
An SSH host that goes quiet may still be running everything. WSL is different: the local WSL service answers
wsl --list --runningwith a positive report. A distro it lists as stopped has no processes left, and a changed boot id or pid 1 start time means a new incarnation. These are answers, not silence. Everything else, including probe failure, timeouts and unreadable output, staysunverifiable.Identity uses ticks since boot because the guest clock can jump after the host resumes (microsoft/WSL#4179, #10006).
Linked Issue
Internal WSL-via-orcad tracker, step C6.
Visual Proof
N/A: there is no UI here. C7 renders these statuses.
Testing
Real host (win-lowspec, WSL2). I used a throwaway Ubuntu 24.04 distro,
orca-c6-probe, withsystemd=falseset before its first boot (pid 1 wasinit). The branch code was bundled and run underOrca.exeas Node, with the daily-build orcad template. Times are measured from when the harness spawned the trigger command.wsl --terminate:distro-stoppedwith terminalsexitedcame at +296 ms.ready, including the distro boot and C4's relaunch.orcad-exitedcame at +507 ms.readyat +13.7 to +14.8 s.liveSessions: 1: the terminal survived the crash and was never marked exited.wsl --terminate.unverifiable/running-list-unavailableand kept re-checking at 1, 2 and 4 s.exitedfor 12 s, even though the distro really was Stopped.distro-stoppedverdict and recovered.wsl.exeitself.wsl -l -vmatches the starting state, and the other agents' distros weren't touched.Automated: I added tests for every classifier branch, identity parsing, the backoff bounds and stable reset, the supervisor, the 5-minute holder bound, and the watch (against the real bridge script and the C5 fake
wsl.exechildren). The supervisor tests cover:ready;idle,unverifiableand the give-up;Gates:
pnpm tc, rootoxlint, anti-slop,check:code-quality:changed, type-aware and native audits, the max-lines, ts-nocheck, focus-read and runtime-electron ratchets, reliability gates, dead classes, compat-boundary imports, process-host imports, the node pin, andpnpm formatall pass. Insrc/main/{ssh,wsl,orcad,ipc}, 8835 tests pass. Two fail and also fail when run alone, in code this PR doesn't touch:preflight-agent-detection(a known C4 item);orcad-state-snapshot-shell, which is about real-shell process groups on macOS.Two more passed when re-run alone.
AI Disclosure
Review
Agent skill upstream boundary
Notes
Open questions:
readytakes 13–17 s, against the tracker's target of under 2 s. C4's relaunch (about 40 sequentialwsl.execalls) is almost all of that. Batching C4's wake path is the fix.releaseonly ends the recorded holder.Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)