fix(server): skip Windows cleanup for exited child processes

[Alb11747]

On Windows, Effect launches taskkill /T /F after Node has already observed a child exit. This adds redundant work to failing Git commands and can target a reused PID. See #2537.

Patch the shared Windows cleanup helper to complete its callback without launching taskkill when exitCode or signalCode is set. Live process trees keep the existing cleanup. The patch covers both source and shipped JavaScript for the existing @effect/platform-node-shared@4.0.2 dependency; no dependency versions change.

Verification:

• Frozen-lockfile installation passed.
• Existing process runner and VCS process suites: 43 tests passed.
• A temporary probe using real child processes and simulated Windows taskkill dispatch passed for exited children, repeated kill, and live-child cleanup. Removing the guard makes the probe fail. This does not verify native Windows process-tree behavior.
• Separate simplification and adversarial agent reviews found no actionable issues. No new test files or mock scaffolding added.

Independent Windows benchmark of the equivalent guard reduced failing Git command time from 267–273 ms to 29–32 ms. Exact-head native Windows verification remains outstanding.

Implemented by the original author; takeover review and verification by GPT-6.1-Sol through the Codex harness in T3 Code.

[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: Advanced
• Run ID: 68e3e016-d273-401e-92fb-68b4f46f0831

---

---

---

📥 Commits

Reviewing files that changed from the base of the PR and between 0471bf9 and 8cee51e.

---

---

---

⛔ Files ignored due to path filters (1)

• pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml

---

---

---

📒 Files selected for processing (2)

• patches/@effect__platform-node-shared@4.0.2.patch
• pnpm-workspace.yaml

---

---

---

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

---

---

---

---

📝 Walkthrough

📝 Walkthrough

📝 Walkthrough

Walkthrough

The patch changes the Windows taskkill helpers to skip PID-based tree cleanup when Node has observed the child leader exit. It also updates the cleanup documentation and registers the patch for @effect/platform-node-shared@4.0.2.

Changes

Windows cleanup

Layer / File(s)	Summary
Guard cleanup by leader status patches/@effect__platform-node-shared@4.0.2.patch, pnpm-workspace.yaml	The JavaScript and TypeScript helpers call onExit(null) and skip taskkill when exitCode or signalCode is non-null. Otherwise, they retain tree termination. The patch documents this behavior and the workspace config applies the patch to version 4.0.2.

Priority: ➖ Normal

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

Change: Bug fix

Possibly related PRs

• pingdotgg/t3code#16850: Applies an equivalent Windows taskkill helper change to @effect/platform-node-shared version 4.0.1.

Suggested reviewers: juliusmarminge

---

---

---

---

Merge Risk: ⚪ Minimal · up to 8cee5

The cleanup change is mergeable after normal checks. Native Windows behavior for the rebuilt 4.0.2 package remains unverified, but no specific failure is established.

Architecture Summary

Architecture risk: 🔵 Low · up to 8cee5

The change affects 2 systems.

Changed systems: patches, pnpm-workspace.yaml

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

• observed — patches (service) was modified; 1 changed file maps to changed impact.
• observed — pnpm-workspace.yaml (service) was modified; 1 changed file maps to changed impact.

Before / after behavior

• observed — Modified behavior in patches/@effect__platform-node-shared@4.0.2.patch: The JavaScript taskkill helper now checks whether exitCode or signalCode is non-null; if either is, it calls onExit(null) and skips taskkill. Otherwise, it retains the existing tree-termination command.
• observed — Modified behavior in patches/@effect__platform-node-shared@4.0.2.patch: The Windows cleanup documentation now limits taskkill tree termination to when the leader is running and states that descendants outliving the leader are not reclaimed by PID-based cleanup.
• observed — Modified behavior in patches/@effect__platform-node-shared@4.0.2.patch: The TypeScript taskkill helper now checks whether exitCode or signalCode is non-null; if either is, it calls onExit(null) and skips taskkill. Otherwise, it retains the existing tree-termination command.
• observed — Modified behavior in pnpm-workspace.yaml: Adds a patched-dependency mapping for @effect/platform-node-shared@4.0.2 to its patch file.

---

---

---

Pre-merge checks | 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check		The description links the related investigation and explains why this focused fix qualifies without separate approval evidence.
Out of Scope Changes check		The changes remain within the stated scope: they patch Windows cleanup behavior, register the patch, and update related dependency metadata without changing POSIX behavior or dependency versions.
Title check		The title is concise, conventional, and accurately identifies the Windows cleanup change for exited child processes.
Description check		The description clearly covers the problem, implementation, verification results, limitations, issue reference, and agent attribution. It omits a separate Scope and approval heading, but it explains w…

---

✨ Finishing Touches

🧪 Generate unit tests (beta)

• 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.}

[macroscopeapp]

Approvability

Verdict: Approved at 8cee51e

Macroscope's review found this PR approvable — This is a small, localized Windows process-cleanup fix that avoids stale-PID taskkill attempts while preserving live-process cleanup and POSIX behavior. The dependency patch, workspace registration, and lockfile updates consistently activate only this targeted change.

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

[SunkenInTime]

Tested on Windows 11 (Ryzen 7 5800X, git 2.55, Node 24.13). This patch is line for line the same change as #16850. I benchmarked that one on a fresh boot: 60 failing git config calls through NodeChildProcessSpawner, alternating unpatched and patched runs.

run	taskkill launches	main thread blocked in launches	wall time per git command
unpatched	120	0.84 s	267 ms
patched	0	0.16 s	29 ms
unpatched	120	0.85 s	273 ms
patched	0	0.22 s	32 ms

Without the patch, each failing git command takes about 9 times as long, because the scope's release step waits for taskkill to finish. In 60 seconds of the desktop server's normal work, 64 of its 168 process launches were these taskkill calls.

My comment on #16850 said this PR targeted 4.0.0-rc.115. It now patches 4.0.2, which main uses, and it merges cleanly. #16850 still patches 4.0.1 and now conflicts, so this is the one to land. #17707 removed the git launcher, but a failing git command still starts 6 processes, and 4 of them come from these two taskkill calls.

Dismissing prior approval to re-evaluate 8cee51e