Tests: stop the harness leaking live-server processes - #718
Merged
Conversation
Nothing owned a live server past the exit paths JavaScript can observe. The live unit tests spawn the server as a direct child and stop it with an HTTP /stop plus proc.kill() inside an after() hook; the e2e session and the target-context tests boot it through `live-server --background` / live.mjs, which spawns a detached, unref'd daemon that only the `stop` verb ever ends. A POSIX child does not die with its parent, and a detached daemon is orphaned to pid 1 from birth, so any exit that skipped teardown (a node:test timeout, a SIGKILL of the runner, a Ctrl-C, an assertion that threw before the hook) left the server listening on a fixed live-suite port for good. scripts/run-tests.mjs did not compensate: it used blocking spawnSync, so no signal handler could run; it left suite commands in its own process group with nothing that could kill that group; and it never checked afterwards whether anything survived. Days of local runs accumulated 197 orphans on one machine, the oldest four days old, until `bun run test:live` could not claim its ports. The fix is structural rather than a cleanup sweep bolted on the end, and it is deliberately implementation-agnostic so it holds for the Node scripts here and for the Rust `impeccable live-server` on rust-swap: - tests/lib/live-servers.mjs. armLiveServerReaper(), called once at module scope by every test file that starts a server, stamps the process env with a unique marker, installs exit and signal handlers, and spawns a detached reaper holding a pipe to the process. SIGKILL the process and the pipe closes, the reaper wakes on EOF and kills the servers carrying that marker. That is the one case no in-process cleanup can reach. trackServerChild() also registers direct children (live servers and fixture dev servers) so the ordinary exits are a cheap kill by handle. - scripts/lib/live-server-processes.mjs. The scan and kill primitives, shared by the reaper and the runner. Processes are matched by the environment marker the harness exported, never by name or port, so a sweep can only ever reach a server this repo's tests started. - scripts/run-tests.mjs. Each suite command now runs as its own process-group leader with SIGINT/SIGTERM/SIGHUP forwarded to the group, and after every suite the runner checks for live servers carrying that suite's run id. A survivor is killed and fails the run, so the next leak surfaces in the run that caused it instead of on a laptop days later. IMPECCABLE_SKIP_LEAK_CHECK=1 bypasses it. `bun run test:cleanup` sweeps leftovers from earlier runs. - tests/live-server-leak.test.mjs pins the guarantee: it boots a real server under a process it then SIGKILLs, and fails if the server outlives it. With IMPECCABLE_NO_TEST_REAPER=1 the test fails, which is what makes it a regression test rather than a tautology. Verified: bun run test:live green with zero survivors; scoped live-e2e (vite8-react-plain) matches pristine main test for test; the SIGKILL repro goes from 2 orphans to 0; SIGINT and SIGKILL of the runner itself both leave nothing behind; bun run build green. Fixes #717 AI assistance: prepared by Claude Code under pbakaus's direction. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Vau2X53xGTjjTCXWMVBoNY
Greptile SummaryThe PR prevents test-owned live servers and fixture development servers from surviving abnormal test-runner exits.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains; the previously reported cleanup ownership and marker-boundary issues are fixed by opaque checkout hashes, exact environment-entry matching, and removal of path-based fallback matching.
|
| Filename | Overview |
|---|---|
| scripts/lib/live-server-processes.mjs | Implements exact marker-based discovery and termination; the current opaque checkout hash and whole-entry matching resolve the previously reported ownership-boundary defects. |
| tests/lib/live-servers.mjs | Arms per-test cleanup, tracks direct children, and delegates unobservable-exit cleanup to the detached reaper. |
| scripts/lib/test-orphan-reaper.mjs | Watches the parent pipe and sweeps only live servers carrying the parent test process’s opaque marker. |
| scripts/lib/process-group.mjs | Encapsulates asynchronous process-group shutdown and second-signal escalation without blocking child reaping. |
| scripts/run-tests.mjs | Replaces blocking suite execution with isolated process groups, signal forwarding, exact run markers, leak detection, and checkout-scoped cleanup. |
| tests/live-server-leak.test.mjs | Covers SIGKILL cleanup, exact marker boundaries, adjacent checkout isolation, canonical checkout hashing, and invalid marker rejection. |
Sequence Diagram
sequenceDiagram
participant R as Test runner
participant T as Test process
participant S as Live server
participant O as Orphan reaper
R->>T: Spawn suite in isolated process group
T->>O: Spawn detached reaper with pipe
T->>S: Spawn with run/process/repository markers
alt Normal completion
T->>S: Stop tracked server
T->>O: Close pipe
R->>R: Verify no marked survivors
else Test process is SIGKILLed
T--xO: Pipe closes on process death
O->>S: Find exact process marker and terminate
R->>R: Verify no marked survivors
end
Reviews (5): Last reviewed commit: "Review fix: a second Ctrl-C must reach t..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The new leak regression test and signal handlers have minor but concrete correctness issues (Windows behavior and SIGHUP exit code) that should be fixed before merging.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR hardens the test harness and test runner to prevent orphaned live-server processes from surviving abnormal exits (e.g., SIGKILL, Ctrl-C, skipped teardown), addressing issue #717 by adding an env-marker-based reaper + post-suite leak checks.
Changes:
- Add a test-side reaper (
armLiveServerReaper()+ detached pipe-driven reaper) and child-process tracking for live servers and fixture dev servers. - Add process discovery/kill primitives scoped by harness env markers, and make
run-tests.mjsrun suites in their own process groups with signal forwarding and post-suite leak detection. - Add
bun run test:cleanup, a regression test to pin the no-orphans guarantee, and update suite wiring/docs.
File summaries
| File | Description |
|---|---|
| tests/live-target-context.test.mjs | Arms the test reaper for tests that boot detached live servers via live.mjs. |
| tests/live-server.test.mjs | Arms the reaper and tracks direct-child server processes for reliable teardown. |
| tests/live-server-leak.test.mjs | New regression test ensuring detached servers do not survive a SIGKILLed test process. |
| tests/live-poll-stream.test.mjs | Arms the reaper and tracks direct-child server processes for reliable teardown. |
| tests/live-e2e/session.mjs | Arms the reaper for detached live servers and tracks fixture dev servers as direct children. |
| tests/lib/live-servers.mjs | New test harness utilities: env stamping, signal/exit cleanup, detached reaper spawn, child tracking. |
| scripts/test-suites.mjs | Ensures the new infra files trigger appropriate suites; adds the new leak regression test to live. |
| scripts/run-tests.mjs | Replaces spawnSync runner with detached process groups, signal forwarding, leak checks, and --cleanup. |
| scripts/lib/test-orphan-reaper.mjs | New detached reaper process that kills marked live servers when parent dies (EOF on stdin pipe). |
| scripts/lib/live-server-processes.mjs | New shared primitives to find/kill live servers strictly scoped by harness markers (never by name/port alone). |
| package.json | Adds test:cleanup script. |
| CLAUDE.md | Documents the three-layer live-server leak guard and the new cleanup command. |
Review details
- Files reviewed: 12/12 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Five review findings on #718, all in the matching layer that decides which processes a sweep may touch. The repository-path fallback is gone (Greptile P1). `bun run test:cleanup` passed REPO_ROOT to findLiveServers, which then also matched any live-server command line under the checkout, marker or not. A developer running `impeccable live` in this repo has exactly that command line, so the cleanup could have killed their own session. The PR promised matching on the exported environment marker and nothing else; now it does. The cost is that a server from a run predating the marker is no longer found and has to be killed by hand, which is the right trade. Environment entries are compared whole on macOS and BSD (Greptile P1). `ps -E` flattens the environment into the command column, and that line was searched with a plain substring test, so IMPECCABLE_TEST_REPO=/work/impeccable also matched /work/impeccable-copy and one checkout's cleanup could reach a neighbouring checkout's servers. envLineHasEntry() now requires the marker to start an entry (line start or whitespace) and to end one (line end, or whitespace followed by the next KEY=), which is the same whole-entry comparison the Linux /proc branch already did. Six unit tests cover it, including the adjacent-path negative case, and a live probe against real `ps -E` output confirms an exact repo matches while /work/impeccable-copy and a run-id prefix do not. The SIGKILL regression test now skips on win32 with a stated reason (Copilot). The reaper is a POSIX mechanism and armLiveServerReaper() does not arm it there, so the test asserted a guarantee Windows does not make yet. Signal exits use the shell convention 128 + signum in both the runner and the test helper (Copilot, two threads). SIGHUP returned 143; it is 129. Read from os.constants.signals rather than a hand-written table. Verified: leak test 7/7 (2 guard, 5 matcher); bun run test:live 895 tests, 0 fail, 0 survivors; scoped live-e2e (vite8-react-plain) 3 pass / 1 fail, matching pristine main; SIGKILL repro 3 servers up, 0 after; bun run build green. AI assistance: prepared by Claude Code under pbakaus's direction. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Vau2X53xGTjjTCXWMVBoNY
… case Greptile's follow-up P1 on the parser was right, and the parser was the wrong place to answer it. envLineHasEntry ended an entry at "whitespace followed by the next KEY=", so a checkout path that extended another one with whitespace plus a KEY=-shaped token still defeated it, which is exactly the ambiguity the docblock admitted to. A format that cannot be parsed unambiguously should not be handed ambiguous input. So the fix is at the source: no marker value is a path any more. IMPECCABLE_TEST_REPO now carries repoMarker(), the first 16 hex characters of the sha256 of the checkout's real path, and the runner and the cleanup command both compute it the same way from REPO_ROOT. Two checkouts whose paths share a prefix get unrelated hashes, so a substring cannot arise in the first place, and every spelling of one checkout (trailing slash, `.` segment, symlink, /private prefix) resolves to one marker. The run id is now repoMarker plus 8 random bytes of hex, and the process id p<pid> plus the same, both from a whitespace-free alphabet. With every value fixed-alphabet, envLineHasEntry needs only "starts an entry and ends at whitespace or line end". The KEY= lookahead is gone and so is the documented unresolvable case. assertMarkerValue keeps the invariant honest: it refuses any value outside [A-Za-z0-9_-] with a message that says to hash it, so a future caller that passes a path gets a loud error instead of a silent mismatch. The readable path is still available for a human reading `ps -E` output, exported separately as IMPECCABLE_TEST_REPO_PATH, which nothing matches on and the docblock says so. Matcher tests: the space-in-value case is gone, since that value can no longer exist. Added a strict-prefix case (a longer hash-shaped value starting with the marker), an adjacent-checkout case asserting the two hashes do not even share a prefix, a symlink/trailing-slash case against real directories, an alphabet check on all three generators, and one asserting assertMarkerValue throws. Verified: leak test 10/10; bun run test:live 898 tests, 0 fail, 0 survivors; scoped live-e2e (vite8-react-plain) 3 pass / 1 fail, matching pristine main; SIGKILL repro 1 server up, 0 after; bun run build green. A probe against real `ps -E` output with a hashed marker: this checkout 1 match, its trailing-slash spelling 1, an adjacent checkout 0, exact run id 1, a run-id prefix 0. AI assistance: prepared by Claude Code under pbakaus's direction. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Vau2X53xGTjjTCXWMVBoNY
Two Cursor Bugbot findings, both real.
killCurrentGroup busy-waited on alive(child.pid) after sending SIGTERM, which
could never work. A dead child stays a zombie until its parent reaps it, the
parent here is the runner, and the runner reaps through libuv when the event
loop runs. The spin blocked the very loop that would have done the reaping and
then read the unreaped zombie as alive, so every SIGINT, SIGTERM and SIGHUP
burned the full 2s grace and ended in a needless SIGKILL. There is no waitpid
from JavaScript that sees through this, so the wait is now asynchronous and
keyed on the child's own exit event. The logic moved to
scripts/lib/process-group.mjs: trackChildExit exposes the exit as a flag and a
promise, stopGroup races that promise against the grace period and escalates to
SIGKILL only if it loses, and killGroupSync stays synchronous for
process.on('exit'), where nothing can be awaited, so it sends SIGTERM then
SIGKILL without pretending to wait. A second Ctrl-C now skips the grace period
entirely rather than queueing behind it.
Measured on a real SIGINT to a running live suite: 2027ms before, 34ms after.
tests/process-group.test.mjs pins both halves, including the escalation path
against a child that traps SIGTERM, which is not otherwise reachable from a
registered suite.
The repoMarker symlink test called symlinkSync with no type, which throws EPERM
on Windows without Developer Mode. It now passes 'junction' there and 'dir'
elsewhere, the same shape tests/concept-seed.test.mjs uses, and the
trailing-slash and dot-segment cases split into their own test so they keep
running on every platform regardless.
Merged origin/main (through #716) to re-level the branch.
Verified: leak and process-group tests 16/16; bun run test:live 900 tests, 0
fail, 0 survivors; scoped live-e2e (vite8-react-plain) now 4/4, with the
orphaned-session test that #716 fixed passing in 7.2s; bun run build green.
AI assistance: prepared by Claude Code under pbakaus's direction.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Vau2X53xGTjjTCXWMVBoNY
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit de31434. Configure here.
…pping
Cursor Bugbot caught a bug I introduced with the async shutdown, and it is the
same class of leak this PR exists to close. The signal handler cleared
currentChild before awaiting stopGroup, so a second Ctrl-C read a null handle:
killGroupSync did nothing, process.exit walked away from the SIGKILL escalation
still in flight, and because the suite is spawned detached it kept running
after the runner was gone. Impatience with a stuck suite produced exactly the
orphan the change is supposed to prevent.
The shutdown state machine moved into scripts/lib/process-group.mjs as
createGroupShutdown, which holds the group in `stopping` for as long as it is
being ended rather than dropping the only reference to it. A second signal
kills that handle and leaves; process.on('exit') looks at `current` or
`stopping`, so the last-resort path reaches a group mid-shutdown too. The
runner keeps no shutdown state of its own now, which is what made the bug
possible to write in the first place.
The extraction is what makes it testable: `exit` is injectable, so
tests/process-group.test.mjs can drive two signals at a stubborn child that
traps SIGTERM and assert the group dies in under 2s against a 30s grace. Point
that test at the old logic (killGroupSync on the cleared reference) and it
hangs out the full grace and fails, which is the check that it pins something
real. Five cases in all, including the exit-handler path and the no-child case.
Verified: process-group 10/10, live-server-leak 11/11; real double SIGINT to a
running live suite exits in 24ms with zero group members and zero servers left;
bun run test:live 900 tests, 0 fail, 0 survivors; scoped live-e2e
(vite8-react-plain) 4/4; bun run build green.
The core suite wedged twice locally in tests/build-phase.test.mjs, the
pre-existing unbounded-spawnSync hang noted in the PR description that
rust-swap's 47f1871 fixes. Unrelated to this change: CI is green on both Node
versions, and process-group.test.mjs passes inside that batch.
AI assistance: prepared by Claude Code under pbakaus's direction.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Vau2X53xGTjjTCXWMVBoNY
pbakaus
added a commit
that referenced
this pull request
Sep 4, 2026
Brings the leak guard from #718 onto the branch and makes its guarantee hold for the Rust engine instead of the Node scripts it was written against. Conflicts and how each was resolved: - tests/live-poll-stream.test.mjs, tests/live-server.test.mjs, tests/live-target-context.test.mjs (modify/delete): kept deleted. They drove skill/scripts/live-server.mjs, which does not exist here; the verb behavior they covered is the oracle's job now. Their entries came out of test-suites.mjs along with the rest of main's live list, which is Node-script coverage this branch already retired. - scripts/test-suites.mjs: took main's two new entries that still apply, process-group.test.mjs into core and live-server-leak.test.mjs into live, plus the infra trigger patterns for the three new scripts/lib modules. Dropped main's pin.test.mjs (no such file here). - package.json: kept test:cleanup, dropped test:cli-e2e (no cli-e2e suite here). - scripts/run-tests.mjs: rewritten to hold both sides rather than picking one. From #718: the createGroupShutdown state machine, the per-suite run-id marker env, the post-suite leak check, and --cleanup. From 47f1871: the per-command wall-clock cap with its per-suite wallClockMs override and IMPECCABLE_TEST_WALL_CLOCK_MS, plus the killed-by-signal report. The two agree on the detached process group, so they compose: the cap SIGKILLs that group when a command wedges, the shutdown handler ends it on a signal, and both now sweep for leaked servers before exiting. #718's handler replaces the old raw signal forwarding, which sent one signal and never escalated. - tests/live-e2e/session.mjs: kept both sides. The binary-driven boot (runEngineSync, requireEngineBin, engineEnv) stands, with armLiveServerReaper at module scope and trackServerChild around the fixture dev server. Ported to the rest of the branch: - tests/oracle/lib.mjs arms the reaper and tracks the daemon child. Its daemon steps spawn live-server detached, so a SIGKILLed oracle run used to strand one; buildInvocation already inherits process.env, so the marker reaches it. - tests/live-server-leak.test.mjs now boots the engine binary through tests/lib/engine-bin.mjs and skips cleanly without one. No crate change was needed. The daemon spawn does env_clear().envs(env) against Io::stdio()'s env, which is std::env::vars(), so the detached Rust process carries the parent environment and the markers reach it. Verified against a real --background daemon: found by run id and by repo marker, not found by an adjacent checkout's marker. CLAUDE.md now says so, since narrowing that env would make the guard silently blind. Verified with a fresh cargo build --release -p impeccable: - IMPECCABLE_BIN=... bun run test green end to end: core 90, oracle 1 (zero unreviewed differences), detector 1, live 159 (157 pass, 2 skipped), framework 186, plugin-e2e 4. Zero servers left. - SIGKILL repro against impeccable live-server --background: 1 daemon up, 0 after with the reaper, 1 surviving with parent pid 1 under IMPECCABLE_NO_TEST_REAPER=1. bun run test:cleanup then kills exactly that one. - The leak test fails under IMPECCABLE_NO_TEST_REAPER=1 and passes with it. - IMPECCABLE_E2E_ONLY=vite8-react-plain bun run test:live-e2e 4/4. - bun run build green. AI assistance: prepared by Claude Code under pbakaus's direction. Co-Authored-By: Claude Code <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Vau2X53xGTjjTCXWMVBoNY
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #717
Cause
Nothing in the harness owned a live server past the exit paths JavaScript can observe.
The live unit tests (
tests/live-server.test.mjs,tests/live-poll-stream.test.mjs) spawn the server as a direct child and stop it with an HTTP/stopplusproc.kill()inside anafter()hook. The e2e session (tests/live-e2e/session.mjs) andtests/live-target-context.test.mjsboot it throughlive-server --background/live.mjs, which spawns a detached, unref'd daemon that only thestopverb ever ends. A POSIX child does not die with its parent, and a detached daemon is orphaned to pid 1 from birth, so any exit that skipped teardown (anode:testtimeout, aSIGKILLof the runner, a Ctrl-C, an assertion that threw before the hook) left the server listening on a fixed live-suite port for good.scripts/run-tests.mjsdid not compensate: it used blockingspawnSync, so no signal handler could run; it left suite commands in its own process group with nothing that could kill that group; and it never checked afterwards whether anything survived. So a leak was invisible until a port was already wedged, days later.The fixture dev servers
startDevServer()spawns leak in exactly the same way, and are covered by the same change.Repro
On
mainatfcc271c1:Two orphans per hard kill, parent pid 1. On this branch the same repro reports
during: 3, after: 0.Fix
Structural, not a cleanup sweep bolted on the end, and deliberately implementation-agnostic so it holds for the Node scripts here and for the Rust
impeccable live-serveronrust-swap(#714). The harness drives whichever server it is given, so the mechanism keys on nothing the server has to implement.tests/lib/live-servers.mjs.armLiveServerReaper(), called once at module scope by every test file that starts a server, stamps the process environment with a unique marker, installs exit and signal handlers, and spawns a detached reaper holding a pipe to the process.SIGKILLthe process and the pipe closes; the reaper wakes on EOF and kills the servers carrying that marker. That is the one case no in-process cleanup can reach.trackServerChild()also registers direct children (live servers and fixture dev servers) so ordinary exits are a cheap kill by handle.scripts/lib/live-server-processes.mjs. The scan and kill primitives, shared by the reaper and the runner. Processes are matched by the environment marker the harness exported, never by name or port, so a sweep can only ever reach a server this repo's tests started. Never anything else on the machine.scripts/run-tests.mjs. Each suite command now runs as its own process-group leader withSIGINT/SIGTERM/SIGHUPforwarded to the group, and after every suite the runner checks for live servers carrying that suite's run id. A survivor is killed and fails the run, so the next leak surfaces in the run that caused it instead of on a laptop days later.IMPECCABLE_SKIP_LEAK_CHECK=1bypasses it.bun run test:cleanup. One-shot sweep for leftovers from earlier runs of this checkout.tests/live-server-leak.test.mjspins the guarantee: it boots a real server under a process it thenSIGKILLs and fails if the server outlives it. WithIMPECCABLE_NO_TEST_REAPER=1the test fails, which is what makes it a regression test rather than a tautology.Verification
bun run test(core, detector, live, framework, plugin-e2e)bun run test:livepgrep -f live-serverempty afterwardsIMPECCABLE_E2E_ONLY=vite8-react-plain bun run test:live-e2eorigin/maintest for test (see below)main, 0 on this branchSIGINTmid-run ofrun-tests.mjsSIGKILLofrun-tests.mjsitselfbun run buildTwo pre-existing failures on
mainare unchanged by this branch and are not addressed here:live-e2eself-discards an orphaned session when its wrapper is edited out of sourcefails on pristineorigin/maintoo. Verified in a clean worktree.bun run testcan hang forever intests/build-phase.test.mjs: itsrun()helper usesspawnSyncwith no timeout, which blocks the worker thread so--test-timeoutcannot interrupt a wedged child. I hit it once (12.9 minutes before I killed the child by hand) and it did not recur on the clean re-run. This is already fixed onrust-swapby commit47f18713, so I left it alone rather than duplicating that diff.Note on overlap:
47f18713onrust-swapindependently gaverun-tests.mjsthe same detached process-group shape plus a wall-clock cap. The two changes converge on the same approach and will need a small manual merge whichever lands second. I deliberately did not port the wall-clock cap or thebuild-phasetimeout here: the 197 orphans all had parent pid 1, so they came from dead parents rather than hung ones, and those belong to #714.AI assistance: prepared by Claude Code under pbakaus's direction.
🤖 Generated with Claude Code
https://claude.ai/code/session_01Vau2X53xGTjjTCXWMVBoNY
Note
Medium Risk
Changes how the test runner spawns and kills process trees on POSIX; kills are env-marker-scoped but mistaken matching could still affect unrelated processes if marker invariants were violated.
Overview
Stops live-server and fixture dev servers from outliving aborted test runs (timeouts, SIGKILL, Ctrl-C), which had been wedging fixed ports (#717).
Test harness: New
armLiveServerReaper()stamps opaque env markers, runs in-process cleanup, and spawns a detached pipe reaper so servers die even when the test process is killed without teardown. Direct children usetrackServerChild()for handle-based kills.Runner:
run-tests.mjsswitches from blockingspawnSyncto detached process groups with signal forwarding and async shutdown. After each suite it fails the run if any server still carries that suite’s run id (bypassIMPECCABLE_SKIP_LEAK_CHECK=1).bun run test:cleanupsweeps leftovers scoped to this checkout’s repo marker only—never by port or process name alone.Shared libs:
live-server-processes.mjsdiscovers/kills markedlive-serverprocesses (Linux/proc, BSDps -Ewith strict entry matching).process-group.mjsimplements group teardown without liveness polling (avoids zombie false positives).Live and e2e tests that start servers adopt the reaper;
live-server-leak.test.mjsandprocess-group.test.mjslock in the behavior. Docs inCLAUDE.mddescribe the three-layer model.Reviewed by Cursor Bugbot for commit ac947c5. Bugbot is set up for automated code reviews on this repo. Configure here.