Skip to content

perf(wsl-host): a call made while the host connects joins that connect - #33180

Merged
nwparker merged 2 commits into
mainfrom
nwparker/wsl-host-call-joins-connect
Oct 28, 2026
Merged

nwparker merged 2 commits into
mainfrom
nwparker/wsl-host-call-joins-connect

Conversation

@nwparker

@nwparker nwparker commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 1 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​111 $\color{#cf222e}{\Huge{\mathbf{−}}}$​1 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​110
Prod 2 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​39 $\color{#cf222e}{\Huge{\mathbf{−}}}$​6 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​33

ELI5

When Orca starts and a WSL host is still connecting, a restored terminal asks the host to show it, and that request waits for the connection. It waited longer than needed: once the connection was up, the request also restarted the host's health monitor, which takes about a second on a slow PC. Now it waits only for the connection.

What Changed

  • Before: a runtime call for a WSL host that wasn't reachable yet ran a full liveness connect (connector.connect).
    • When a connect was already in flight, the bridge connect itself was shared, but the liveness supervisor bumped its generation, disarmed the watch and re-armed it. That's an identity read in the distro plus a new watch channel, and the call waited for it.
    • Measured on a slow Windows box: the restored pane's session.tabs.activate resolved its host check 1.15 s after the host was ready.
  • After: while a liveness connect is in flight for that distro, a plain connect (no Update or Force intent) returns the bridge if it's already up, or joins the connection manager's in-flight connect. Only the first connect arms liveness.
  • No-boot semantics (E9, fix(wsl-host): a runtime-call reconnect never boots a distro that stopped meanwhile (E9 follow-up) #34050/fix(wsl-host): no-boot connects stay quiet, and liveness recovery never boots (E9 follow-up) #34093) are kept; a joined call never changes what the in-flight connect may do:
    • A no-boot call joining a connect that may boot reaches the manager with its own running check, so it can only ride the boot the user's focus or Connect is already doing. It never starts one.
    • A connect that may boot never joins an in-flight no-boot connect. That connect may refuse a stopped distro, and nothing would arm liveness for a boot the joiner then made. It runs its own liveness connect, which the manager orders after the no-boot one.
    • Update and Force never join, and are never joined. An in-flight connect records whether it carries an intent, and a plain call never joins it, because its incumbent server is on its way out. The call goes through liveness, as on main.
    • A joiner never gets the outgoing server. While the connection manager is still connecting (isConnecting), get can name the server being replaced, so the joiner waits on the manager's connect (ensure) for the new bridge. Only once the bridge is up does it take get's answer at once. That's the step that skips the ~1 s liveness re-arm.
  • Mechanism: a per-distro map of in-flight liveness connects in createWslHostConnector, recording whether each is no-boot.
  • Flag off: no effect. This code runs only for WSL hosts, behind the internal ORCA_INTERNAL_WSL_HOSTS gate.

Why

The re-arm did nothing for the caller. The watch the first connect is arming is the same one, and repeating it churned the watch channel. Joining keeps one armed watch per connect and removes the wait. The join rule matches the connection manager's own: a no-boot connect may ride anything, and a booting one may not ride a no-boot one.

Linked Issue

Internal: WSL via orcad D3 startup gate (client-side activate gap); E9 connect policy (#33838, #34050, #34093).

Visual Proof

N/A. This is a startup timing change behind the internal ORCA_INTERNAL_WSL_HOSTS gate, with no visual difference.

Testing

  • Manual (first version), on win-lowspec with startup milestones: activate's host check went from 1.15 s after ready to about 0.04 s.
    • Caveat: under load, the terminal stream that opens next took longer, because it now overlaps the startup call burst instead of following it. The planned idle-box A/B on awin hasn't run.
  • wsl-host-connector.test.ts:
    • A plain connect during an in-flight one joins without a second liveness connect. Once the bridge is up it returns at once, and after the in-flight connect settles, liveness runs again.
    • A no-boot call joins a booting connect and reaches the manager with its running check.
    • A connect that may boot never rides a no-boot one, and an intent never joins.
    • An in-flight redeploy hands the joiner the new bridge, not the incumbent, and a call made during an in-flight Update isn't joined. Fails without the fix.
    • The first two fail without the change.
  • src/main/wsl tests pass. Two load-sensitive cost tests timed out under machine load and pass when run alone.
  • Checks: pnpm tc and check:code-quality:changed pass.

Review

Agent skill upstream boundary

  • Not applicable, or this change copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

  • WSL host only, gated. SSH is untouched, and there's no wire change.

Checklist

  • This PR is small and focused
  • I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
  • Before/after screenshots or videos attached for UI changes, or N/A with reason
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
  • pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

@nwparker
nwparker force-pushed the nwparker/wsl-host-call-joins-connect branch from 09b4630 to 992a6d4 Compare October 28, 2026 02:18
@nwparker
nwparker marked this pull request as ready for review October 28, 2026 02:18
@coderabbitai

coderabbitai Bot commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Walkthrough

The connector now tracks in-flight liveness connections per distro. Calls without an intent join an eligible pending connection. Calls with an intent, or boot-capable calls facing a pending no-boot connection, start a new liveness connection. Tests cover these conditions, connection-manager behavior, and replacement bridges.

Priority: Low

Merge Risk: Moderate · up to 842f2

This change speeds up WSL host reconnects, but it can skip the no-boot running check and can leave a reconnect without liveness supervision after a disconnect. Both cases are narrow and sit behind an internal flag. Fix them before merging.

Pre-merge checks: Passed 4 · Inconclusive 1

Failed Failed checks: 1 inconclusive

Check name Status Explanation Resolution
Description check Inconclusive The description follows the template sections (ELI5, What Changed, Why, Linked Issue, Visual Proof, Testing, AI Disclosure, Review, Agent skill upstream boundary, Notes, Checklist) and explains the be… Add a concrete issue link in the Linked Issue section (for example 'Fixes #NNNN') or state why none exists, since the template requires an actual issue for outside contributors. Confirm whether pnpm lint, typecheck, test, and build pass loc…
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title names the main behavior change: a plain connect made while the WSL host is connecting joins the in-flight connect instead of running a separate liveness connect. It uses the conventional per…
Docstring Coverage Passed No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Title check

Explanation

The title names the main behavior change: a plain connect made while the WSL host is connecting joins the in-flight connect instead of running a separate liveness connect. It uses the conventional perf(scope) prefix and is specific. It is 70 characters, which is within a normal title length.

Full details: Docstring Coverage

Explanation

No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 3 files.

Full details: Description check

Explanation

The description follows the template sections (ELI5, What Changed, Why, Linked Issue, Visual Proof, Testing, AI Disclosure, Review, Agent skill upstream boundary, Notes, Checklist) and explains the before/after behavior, mechanism, and rationale in detail. Gaps: the Linked Issue section cites only internal references ("Internal: WSL via orcad D3 startup gate...") and has no 'Fixes #' issue link, which the template says must always exist for outside contributors. The Testing section lists tests, but the PR checklist leaves 'pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass' unchecked, and the Testing text only mentions 'pnpm tc' and 'check:code-quality:changed'. The Testing checkbox 'I manually tested these changes locally' is also unchecked even though manual testing is described. The Review section is empty. The AI Disclosure section is empty.

Resolution

Add a concrete issue link in the Linked Issue section (for example 'Fixes #NNNN') or state why none exists, since the template requires an actual issue for outside contributors. Confirm whether pnpm lint, typecheck, test, and build pass locally, and check the matching checklist items or state which CI covers them. Tick the manual-testing checkbox if the manual test was done locally. Fill in the Review and AI Disclosure sections, or state that they do not apply.

Finishing Touches
📝 Generate docstrings
  • 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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1


Review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: a5b65aa3-cb9f-41ae-b5bb-88bca0c8aa93
Commits

Reviewing files that changed from the base of the PR and between 6dab169 and 992a6d4.

Files selected for processing: 2
  • src/main/wsl/wsl-host-connector.test.ts
  • src/main/wsl/wsl-host-connector.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.

Comment thread src/main/wsl/wsl-host-connector.ts Resolved

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2


Review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 98baa479-8dd8-40bd-9d26-0e5011af18b1
Commits

Reviewing files that changed from the base of the PR and between 992a6d4 and 842f2aa.

Files selected for processing: 3
  • src/main/wsl/wsl-host-connector.test.ts
  • src/main/wsl/wsl-host-connector.ts
  • src/main/wsl/wsl-orcad-connection.ts
Files skipped from review as they are similar to previous changes: 1
  • src/main/wsl/wsl-host-connector.test.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.

// While the bridge connect runs, `get` names the server it replaces: wait for the new one.
return manager.isConnecting(distro)
? manager.ensure(distro, options)
: Promise.resolve(manager.get(distro) ?? manager.ensure(distro, options))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Major · Functional Correctness · Quick win

Recheck the distro before returning a bridge to a no-boot call.

If the distro stops after the service’s reachability check while liveness is still arming, this branch returns the existing bridge without invoking requireRunning. The subsequent runtime call can dial that bridge and spawn wsl.exe, booting the stopped distro. Preserve the no-boot running check before returning the bridge. (raw.githubusercontent.com)


Major · Stability & Availability · Heavy lift

Invalidate pending connects when supervision stops.

If disconnect occurs while the first connect awaits its identity read, livenessConnects still contains that connect. A reconnect can then call manager.ensure here after liveness.stop has removed the supervisor. The new bridge has no watch or heartbeat, because the old supervisor generation cannot arm it. Clear the pending entry on every stop path and route the reconnect through liveness. (raw.githubusercontent.com)

@nwparker
nwparker merged commit c10110b into main Oct 28, 2026
60 of 61 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant