Skip to content

refactor(wsl-hosts): add flow in wsl-host-registration; liveness split into state, evidence and monitor - #29506

Merged
nwparker merged 3 commits into
mainfrom
wsl-host-registration-split
Oct 21, 2026
Merged

nwparker merged 3 commits into
mainfrom
wsl-host-registration-split

Conversation

@nwparker

@nwparker nwparker commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 3 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​313 $\color{#cf222e}{\Huge{\mathbf{−}}}$​1 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​312
Prod 8 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​328 $\color{#cf222e}{\Huge{\mathbf{−}}}$​282 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​46

ELI5

Two WSL-host files had grown to do too many things. One mostly handled day-to-day hosts but also held the whole "add a new WSL host" procedure. The other was called a "model" but held timers, wake-up handling and evidence gathering. This PR moves code into files named for what they do. The code itself doesn't change. WSL hosts stay behind an internal switch that is off, so no user can see any of this.

What Changed

Nothing a user can see. Under the hood:

  • Add flow: the add procedure moves out of wsl-host-service.ts into the new wsl-host-registration.ts as registerWslHost. That procedure is: refuse, admit, check for an orcad this desktop already reaches, connect, save, and undo the connection if the save fails. The service keeps everything else and still makes sure two adds for the same distro share one in-flight attempt. The service file drops from 351 to 289 lines.
  • Liveness: wsl-orcad-liveness-model.ts is split three ways. -state holds the dependencies, the per-distro record, creating it and disarming it. -evidence holds exit classification, the running-list check and evidence gathering. -monitor holds timers, the power-resume wake, the holder re-ensure and the held watch. The function bodies are identical: I compared the old file's lines against the three new files' lines and they match as a set.

Why

Smaller files with concrete names are easier to review. They also keep the active WSL work from colliding in one large file. I chose plain moves over redesigning the supervisor so that the change stays mechanical and easy to check.

Linked Issue

N/A (maintainer cleanup; internal plan items 10 and 11)

Visual Proof

N/A: no visual or interaction change. Code moves only.

Testing

  • The first commit only adds tests, and they pass against the old code:

    • Add flow: every branch, namely an unsupported WSL 1, a generic connect failure, and a failed save that disconnects and then releases the holder even when the disconnect throws. Also that concurrent adds of the same distro (any casing) share one attempt, and the exact call order: list, shared-orcad check, connect, publish ready, then report ready.
    • Liveness: each of its building blocks on its own.
  • The same tests pass after the move.

  • tsc (node project), vitest for all of src/main/wsl and the WSL IPC tests (433 passed), and pnpm run check:code-quality:changed (0 findings).

  • E2E: I started the WSL fake-transport spec and the SSH-on-managed-orcad specs by hand. This PR also triggers the real-WSL-distro lane through its path filter.

  • Automated tests added/updated, or explained why not below

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

SSH: untouched. Windows/WSL: the gate stays closed, and the WSL commands and their order are unchanged. Wire and on-disk formats: no 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)

Risk rating (final): LOW

  • Starting rating: MEDIUM (internal plan items 10 and 11).
  • Rebased onto main (rebased 2026-10-20) after upstream changed the add flow:
    • an add now notes how many times the experiment has been turned off, and drops the new host if that count changes while it connects;
    • the connect step now lives in wsl-host-first-connect.ts, and a failed connect now releases the connection and the distro holder.
      The add flow was re-extracted from main's current code. Those upstream changes are in it unchanged; the registration module reads the turned-off count through a getter that the service provides.
  • Characterization tests: updated to pin main's new release-on-failed-connect behaviour, and verified against main's unmodified service before the move.
  • Liveness split: main's new importer wsl-host-connector.ts now imports from -evidence. The function bodies still match the old module line for line.
  • Adversarial review (twice, before and after the rebase): no behaviour change.
  • CI: all PR checks pass, including WSL2 distros (systemd off and on) on real WSL and real WSL terminal.
  • E2E (started by hand, run 39501412825): WSL fake-transport spec and both managed-SSH-on-Docker shards all pass.
  • Not merged: held for owner review.

@coderabbitai

coderabbitai Bot commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 31dad2fa-e279-45fe-88e8-1fa37da4efac
📥 Commits

Reviewing files that changed from the base of the PR and between 78849b2 and 6e54d36263fe5cdd72cb6955142389ce2eaddd1e.

📒 Files selected for processing: 10
  • src/main/wsl/wsl-host-registration.ts
  • src/main/wsl/wsl-host-service.test.ts
  • src/main/wsl/wsl-host-service.ts
  • src/main/wsl/wsl-orcad-liveness-building-blocks.test.ts
  • src/main/wsl/wsl-orcad-liveness-evidence.ts
  • src/main/wsl/wsl-orcad-liveness-model.ts
  • src/main/wsl/wsl-orcad-liveness-monitor.ts
  • src/main/wsl/wsl-orcad-liveness-state.ts
  • src/main/wsl/wsl-orcad-liveness-supervisor.test.ts
  • src/main/wsl/wsl-orcad-liveness-supervisor.ts
💤 Files with no reviewable changes: 1
  • src/main/wsl/wsl-orcad-liveness-model.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 2 remain after this review.

📝 Walkthrough

The WSL host service now delegates registration to registerWslHost, which handles admission, connection, persistence, cleanup, and readiness. New tests cover registration errors, cleanup, concurrent adds, and status ordering. The Orcad liveness helpers now reside in separate state, evidence, and monitor modules. The supervisor imports from those modules, and tests cover the helper behavior.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to 6e54d

No concrete issue remains that should block merging after normal checks.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 41.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 9 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Linked Issues check Passed The description states that this is maintainer cleanup and references internal plan items. The template permits maintainers to avoid opening needless issues.
Out of Scope Changes check Passed The changes match the stated objectives: extract WSL host registration, split liveness helpers, and add characterization tests. No unrelated product or format changes are described.
Title check Passed The title clearly identifies both primary refactors: extracting WSL host registration and splitting the liveness module. It is specific and related to the changeset.
Description check Passed The description is detailed and covers the required explanation, changes, rationale, testing, visual proof, review notes, and checklist. The maintainer-specific Linked Issue exception is addressed wit…
Full details: Description check

Explanation

The description is detailed and covers the required explanation, changes, rationale, testing, visual proof, review notes, and checklist. The maintainer-specific Linked Issue exception is addressed with N/A. Some standard command checklist items remain unchecked, but the description reports substantial test and CI validation.

✨ 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.

❤️ Share

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

@nwparker
nwparker force-pushed the wsl-host-registration-split branch 2 times, most recently from 484a1b1 to 4eb5bf8 Compare October 16, 2026 07:51
@nwparker
nwparker force-pushed the wsl-host-registration-split branch from 4eb5bf8 to bab0c15 Compare October 20, 2026 21:33
@nwparker
nwparker merged commit 9e04a98 into main Oct 21, 2026
49 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