fix(desktop): cancel backend pipe reads to avoid slow shutdown

[Yash-Singh1]

Problem

Quitting the desktop app leaves the local backend alive until the desktop's two-second force-kill timeout. The browser and telemetry channels read inherited pipes through fs.createReadStream; pending filesystem reads keep Node alive after service teardown while the desktop still holds the write ends open.

Change

Wrap both inherited input descriptors in read-only net.Socket streams. Their reads can be cancelled by the existing scoped destroy() finalizers, allowing the backend to exit without waiting for the desktop to close its pipe writers.

Add a subprocess regression test that receives browser attach/detach messages and the telemetry handshake, then exits while the parent keeps both input pipes open. Both readers need the fix: changing either reader alone still left shutdown waiting for the force-kill timeout in the isolated reproduction.

Scope and approval

This is a focused fix for an observed shutdown defect in the two desktop pipe readers. The production change replaces their stream construction and retains the existing ownership and teardown behavior. The two replacements address the same blocking-read problem; the fixture and test verify message delivery and process exit. No startup changes or UI changes are included.

Verification

The measurements compare baseline b707eeb052 with this branch at b1b7342a67. Values are medians of three launches per condition. Quit is timed from the quit request, after startup finishes.

Shell scenario	Milestone	Baseline	This branch	Reduction
Real login shell	Quit request to OS process exit	2.081 s	0.095 s	1.986 s / 95.5%
Real login shell	Quit request to Electron quit event	2.036 s	0.048 s	1.988 s / 97.7%
Fixed 1-second shell	Quit request to OS process exit	2.083 s	0.097 s	1.987 s / 95.4%
Fixed 1-second shell	Quit request to Electron quit event	2.038 s	0.049 s	1.989 s / 97.6%

With the real shell, OS process exit ranged from 2.074 to 2.092 s at baseline and 0.088 to 0.097 s after the fix. With the fixed-delay shell, it ranged from 2.077 to 2.087 s at baseline and 0.093 to 0.098 s after the fix. Backend SIGKILL fell from 3/3 to 0/3 runs in each scenario, or 6/6 to 0/6 overall.

Measurements used macOS Electron 44.4.5 development builds with a bundled renderer and no compile cache. Each launch had fresh isolated state copied from an 840 KB database with 10 projects, no threads, and no pending work. Runs were sequential, with no concurrent builds or tests. Windows stayed unfocused on the built-in laptop display, with background throttling disabled consistently. The fixed-delay shell sleeps for one second before executing the environment capture command. Shutdown with active provider turns, full production history, and native Windows/Linux launches was not measured.

• vp test run apps/server/src/preview/DesktopBrowserChannel.test.ts apps/server/src/resourceTelemetry/DesktopTelemetryReceiver.test.ts: 7 tests passed across two files. The subprocess regression verifies browser message delivery, healthy telemetry reception, and exit code 0 while the parent keeps the pipe writers open.
• Server typecheck passed.
• git diff --check origin/main...HEAD passed.
• Lint was blocked by the existing missing @oxlint/plugins dependency.

Individual shutdown samples, in seconds

Shell	Version	Trial	Electron quit event	OS process exit	Backend exit signal
Real	baseline	10	2.032	2.074	SIGKILL
Real	baseline	11	2.036	2.081	SIGKILL
Real	baseline	12	2.045	2.092	SIGKILL
Real	pipes	10	0.050	0.095	None
Real	pipes	11	0.048	0.097	None
Real	pipes	12	0.047	0.088	None
Fixed	baseline	10	2.033	2.077	SIGKILL
Fixed	baseline	11	2.038	2.087	SIGKILL
Fixed	baseline	12	2.040	2.083	SIGKILL
Fixed	pipes	10	0.053	0.097	None
Fixed	pipes	11	0.049	0.098	None
Fixed	pipes	12	0.047	0.093	None

Trials 10 through 12 are the final laptop-display batch; interrupted earlier samples are excluded. Electron's quit event and OS process exit are separate milestones.

Agent: gpt-6.1-sol via the Codex harness in T3 Code.

Closes #16932

[macroscopeapp]

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — The PR changes live desktop pipe-read lifecycles in two server components and adds a subprocess regression harness for shutdown behavior. It also introduces a file-level suppression for the nodeBuiltinImport diagnostic, so the change warrants human review.

Notes:

• No code objects were reviewed. Approvability was decided on eligibility alone.

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

[github-actions]

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

Provider	Metric	Main baseline	This PR	Impact	PR ceiling
Codex	Total thread wire	5.0 KiB	5.0 KiB	0 B (0.0%)	6.8 KiB	✅
Codex	Thread snapshot wire	3.8 KiB	3.8 KiB	0 B (0.0%)	4.9 KiB	✅
Codex	Live turn WebSocket wire	1.2 KiB	1.2 KiB	0 B (0.0%)	2.0 KiB	✅
Codex	Live turn WebSocket decoded	20.9 KiB	20.9 KiB	0 B (0.0%)	29.3 KiB	✅
Codex	Live turn messages	2	2	0 (0.0%)	8	✅
Claude	Total thread wire	5.0 KiB	5.0 KiB	0 B (0.0%)	6.8 KiB	✅
Claude	Thread snapshot wire	3.8 KiB	3.8 KiB	0 B (0.0%)	4.9 KiB	✅
Claude	Live turn WebSocket wire	1.2 KiB	1.2 KiB	0 B (0.0%)	2.0 KiB	✅
Claude	Live turn WebSocket decoded	21.2 KiB	21.2 KiB	0 B (0.0%)	29.3 KiB	✅
Claude	Live turn messages	2	2	0 (0.0%)	8	✅

Baseline: b707eeb · PR result: 2b61823 · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

• Codex decoded thread snapshot: 108.5 KiB
• Claude decoded thread snapshot: 108.8 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Path: .coderabbit.config.ts
• Review profile: CHILL
• Plan: Team
• Run ID: 1c1206cc-d6f6-473c-a87d-62ed4c803313

📥 Commits

Reviewing files that changed from the base of the PR and between b1b7342 and 2b61823.

📒 Files selected for processing (1)

• apps/server/src/preview/DesktopBrowserChannel.test.ts

🚧 Files skipped from review as they are similar to previous changes (1)

• apps/server/src/preview/DesktopBrowserChannel.test.ts

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

---

📝 Walkthrough

Walkthrough

The desktop browser and telemetry services now read input file descriptors through NodeNet.Socket. A subprocess fixture and test exercise browser attachment and detachment, telemetry health, and child-process completion.

Changes

Desktop pipe lifecycle

Layer / File(s)	Summary
Socket-based descriptor readers apps/server/src/preview/DesktopBrowserChannel.ts, apps/server/src/resourceTelemetry/DesktopTelemetryReceiver.ts	Both services replace filesystem read streams with readable NodeNet.Socket instances for input file descriptors. Managed cleanup remains in place.
Pipe lifecycle fixture and test apps/server/src/preview/testing/DesktopPipeLifecycle.fixture.ts, apps/server/src/preview/DesktopBrowserChannel.test.ts	The fixture checks attachment, telemetry health, and endpoint behavior after detachment. The test runs the fixture as a subprocess, exchanges lifecycle messages, and checks its output and exit status.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 2b618

The shutdown regression test covers the intended open-pipe behavior. No actionable merge-blocking issue is established, though native Windows launch behavior remains unmeasured.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check		The title clearly identifies the desktop shutdown fix and the cancellation of backend pipe reads.
Description check		The description includes all required sections. It clearly explains the problem, the implementation, scope, issue reference, detailed verification results, test outcomes, and the lint limitation.
Linked Issues check		Issue #16932 targets the inherited browser and telemetry desktop readers. DesktopBrowserChannel and DesktopTelemetryReceiver replace fs.createReadStream with read-only net.Socket streams. Both…
Out of Scope Changes check		The changed production files are the two desktop pipe readers named by issue #16932. The fixture and subprocess test verify message delivery and shutdown behavior for those readers. The available chan…

✨ Finishing Touches

📝 Generate docstrings

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

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