fix(desktop): stop WSL backends when they are disabled

[yashranaway]

Disabling the WSL backend removes its pool entry but leaves the server running. Its supervisor can restart it afterward, even with WSL disabled, and compete with another service in the same distro for remote connectivity.

Registered instances accidentally inherit the pool's application scope from the captured Effect context. Apply the instance scope inside that context so unregistering an instance runs its stop finalizer, releases its process, and cancels scheduled restarts. The primary backend stays running, and the same WSL instance can be registered again.

Fixes #12642.

Validation:

• Two regression tests fail on main before the fix: running-instance teardown and pending-restart cancellation.
• All 42 focused backend pool, backend manager, WSL reconciliation, and WSL IPC tests pass after the fix.
• Coverage includes primary-process isolation, re-enabling the secondary, and application shutdown.
• Desktop typecheck and targeted lint pass.

The tests exercise the real pool and supervisor with scoped process doubles on Linux. A Windows/WSL end-to-end run was not performed. This changes desktop process ownership only; it does not touch the server provider or orchestration layers being rewritten for V2.

Model: GPT-6
Harness: Codex in T3 Code

Summary by CodeRabbit

• Bug Fixes

  • Improved desktop backend lifecycle handling during shutdown and restart scenarios.
  • Prevented pending secondary backend restarts from continuing after cancellation.
  • Ensured backend processes are cleaned up correctly when the desktop application shuts down.

• Tests

  • Expanded coverage for backend registration, re-registration, cancellation, process cleanup, and successful HTTP responses.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: pingdotgg/t3code/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 531e78d9-2f40-49ab-aed7-1a194da33e56

📥 Commits

Reviewing files that changed from the base of the PR and between 7445aa7 and eb65f3e.

📒 Files selected for processing (2)

• apps/desktop/src/backend/DesktopBackendPool.test.ts
• apps/desktop/src/backend/DesktopBackendPool.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.

---

📝 Walkthrough

Walkthrough

The pool now constructs backend instances with unregister-owned child scopes. Tests add deterministic process and HTTP infrastructure and cover secondary unregister, restart cancellation, re-registration, and shutdown cleanup.

Changes

Backend lifecycle

Layer / File(s)	Summary
Instance scope ownership apps/desktop/src/backend/DesktopBackendPool.ts	register now applies the child scope before the captured factory context when creating a backend instance.
Process lifecycle validation apps/desktop/src/backend/DesktopBackendPool.test.ts	Tests add process, timing, filesystem, and HTTP stubs. They verify secondary unregister and re-registration, cancellation of scheduled restarts, and cleanup of primary and secondary processes during shutdown.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~15 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: t3dotgg

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the primary fix: stopping WSL backends when they are disabled.
Description check	✅ Passed	The description explains what changed, why it changed, validation results, test scope, and limitations. It does not use the template headings or include the checklist, but the required technical infor…
Linked Issues check	✅ Passed	Issue #12642 requires live WSL teardown to stop the runtime process, remove the instance from the pool, and prevent restart while disabled. The change applies the child scope inside the captured facto…
Out of Scope Changes check	✅ Passed	The changes stay within issue #12642. The source change fixes scope ownership during backend registration. The added process harness and regression tests verify teardown, restart cancellation, re-regi…

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

---

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

[macroscopeapp]

Approvability

Verdict: Approved at eb65f3e

Macroscope's review found this PR approvable — This is a narrowly scoped desktop lifecycle bug fix that makes disabled WSL backends release their process and cancel restarts through the correct instance scope. Focused tests cover teardown, re-registration, restart cancellation, primary isolation, and application shutdown, with no API, default, deployment, or static-analysis changes.

^{You can add or adjust custom eligibility rules. Learn more.}

[NawzBlaze]

I hit a scope issue while working on the same fix independently and wanted to share a finding that may help with the test approach here.

Problem: \Layer.effect's inner \Scope.Scope\ closes as soon as its build effect completes under per-test \Effect.provide. This means \pool.register\ (which calls \Scope.fork(layerScope)) always forks a closed scope in \it.effect\ tests — so the forked process never spawns, making it impossible to test
egister/\unregister\ against the real pool layer.

In production this doesn't happen because the pool layer survives via the massive shared \provideMerge\ build in \main.ts\ (multiple consumers hold observers > 0, keeping the layer scope alive).

Solution: Use \it.layer\ from @effect/vitest\ instead of \Effect.provide:

\\ s
it.layer(poolLayerWithFakes)('suite-name', (it) => {
it.effect('test', () => Effect.gen(function* () {
const pool = yield* DesktopBackendPool.DesktopBackendPool;
const instance = yield* pool.register(spec);
yield* instance.start;
// ... assert spawnCount, desiredRunning, etc.
}));
});
\\

\it.layer\ builds the layer once into a suite-lifetime \Scope.make()\ (kept open until the last test in the block runs), exactly mirroring production's shared-layer semantics. The \pool.register\ fork gets a live scope, processes spawn, and \pool.unregister\ correctly runs the auto-stop finalizer.

I confirmed this empirically: a \Layer.effect\ with \�ddFinalizer\ only prints its close log when the \it.layer\ suite finishes — not when the test gen completes.

This lets the regression test exercise the real \pool.register\ / \makeBackendInstance\ / \pool.unregister\ path with a stubbed \ChildProcessSpawner\ (counting \spawnCount), rather than mirroring the pipe externally.

[yashranaway]

Thanks @NawzBlaze; here Effect.provide wraps the entire register/start/unregister operation, keeping the real pool alive throughout, and I reverified that reverting the scope fix fails two regressions while the current PR passes all six pool tests.

[juliusmarminge]

Note

This comment is posted by Julius' dot

The scope regression tests establish that unregister stops the supervisor. Do you have a Windows/WSL result showing the Linux t3 process also exits after disabling the backend? The triage in #12642 called out that separate process-boundary check, and the PR says it was not performed. Please provide that result or explain how the existing shutdown path establishes it under verification.

[yashranaway]

The existing tests prove supervisor shutdown, but they do not prove that the Linux t3 process exits across the wsl.exe boundary. I do not have a Windows/WSL host to perform that check, so this verification gap remains open. I have kept the PR description explicit about that limitation.