Prevent orphaned Harper processes after test runner crashes

[kriszyp]

A test runner that dies without running teardown — SIGKILL, a hard crash, a cancelled CI job — cannot reap the Harper instances it started, because those are deliberately detached into their own process groups so whole-tree teardown works. Left alone they hold their loopback address's fixed ports until reboot.

This adds a single shared monitor process that closes that gap. startHarper() publishes explicit metadata for each instance to an on-disk registry and makes sure exactly one monitor is running for that registry (per user on the machine by default). The monitor scans on an interval and terminates — SIGTERM, then SIGKILL after a grace period — the process group of any instance whose owning runner is gone, or which has outlived its lifetime budget, then shuts itself down once the registry has been empty for a while.

Because the registry is the only thing standing between an abrupt death and a stranded port, everything that touches it has to survive an abrupt death itself. That is what the review rounds on this PR converged on, and where the recent commits are:

• A record's lifetime is its process group's, not its leader's. The cleanup unit is the group; Harper exiting is not the end of it, because a child that ignored the SIGTERM stays in that group still holding the ports. Two writers used to end the record at the leader's exit — the monitor pruned any record whose leader was gone, and the runner deregistered on the exit event — and either one alone cancelled the pending SIGKILL escalation and every later chance to reap that group. Removal now belongs to the monitor alone, the half that can see the group, and it happens when the group has no members left rather than when a reap is due — a leader that exits on its own under a live runner is the case that makes those different. The runner only untracks its own child.
• A leaderless group is only reaped on the evidence that it is still ours. groupOutlivedLeader refuses a leader PID that ps still describes with a different start time: a group id is reserved only for the lifetime of the group that held it, so a reused PID's group is somebody else's. See For the human reviewer feat: output Harper logs on test failure #6 for what this does not establish.
• The registry is published by rename. writeRegistryFile writes a complete file beside the registry and renames it into place. An in-place write truncates first, so a writer killed in that window left torn JSON — and readRegistryFile read torn JSON as an empty registry, which the next writer persisted, discarding every reap target on the machine including other runners'. Exactly the event this PR exists to survive was the one that defeated it.
• A failed read never reports "empty". Only ENOENT is an empty registry; a permission error, unparseable content, or an instances that is not an array now propagates rather than being written back as emptiness.
• A lock holder releases only its own lock. withRegistryLock compares the lock's token before unlinking. A section that overran the 10s stale timeout used to delete the lock of the process that had superseded it, admitting a third process alongside it. Acquisition is also bounded at 30s, so a pathological holder fails a registration instead of hanging startHarper forever.
• Startup ends at readiness, not at resolution. Waiting for registration before resolving left settled false past readiness, and every startup guard keyed on it — so a post-readiness log line, which a real Harper emits constantly, re-armed the idle watchdog and let a registry-lock stall SIGKILL a healthy instance, while startupOutput kept accumulating past the snapshot its own comment promises. startupFinished() is now the boundary those guards always meant.
• An identity is a property of the process, and a reap target is never a broadcast. ps -o lstart= renders in the caller's timezone and locale, so two differently configured runners recorded different strings for one process and read each other's live records as PID reuse; the lookup now pins TZ=UTC and LC_ALL=C. And because the registry is on-disk state a corrupt or planted record can reach, signalProcessGroup refuses a group id of 1 or below — kill(-1) is a broadcast to every process the monitor may signal, and no instance we register is ever that.
• Instance ids are unique per module copy, not per PID. Worker threads share process.pid and each holds its own copy of the registry module, so both workers' first starts claimed <pid>-1; registration replaces same-id records, so one of two live instances was left with nothing to reap it. The id now carries a CSPRNG suffix.
• The registry directory is per-user, and nothing in it follows a symlink. Cross-user reaping could never work — signalling another user's process group returns EPERM, which reads as "still alive", so a foreign record pinned the monitor forever. The default is now ${TMPDIR}/harper-integration-test-monitor-${uid} at mode 0700, the pending file is created wx with a CSPRNG suffix, and monitor.log opens O_NOFOLLOW.

For the human reviewer

1. One monitor, not one sidecar per instance. An earlier revision of this PR paired every Harper instance with its own supervisor process watching a liveness pipe. That reaped faster (pipe EOF vs. one scan interval) but cost a process per instance and could only ever see the instances it was born with. The shared monitor costs one process per user per machine, is reused by every concurrent runner, and also cleans up stale instances left behind by earlier runs. Reversing this would mean going back to the spawn topology in the earlier revision.
2. No public API change. Harper is spawned directly, so ctx.harper.process is the Harper process itself and HarperContext is identical to main. Everything new is internal (harperInstanceRegistry.ts, harperMonitor.ts) and not re-exported from index.ts.
3. The runner no longer deregisters — the newest behaviour change here. deregisterHarperInstance is gone rather than fixed: the runner watches one process and the record covers a group, so it cannot tell when the group is finished. A record for a cleanly finished instance now lives until the monitor's next scan (one interval, 2s by default) instead of disappearing on the child's exit, and the registry can briefly hold records for instances that are already over. Framing-Verdict: chosen-approach-sound was not what the planning round returned — it returned better-alternative-exists, preferring exactly this monitor-owned removal over the two-writer version I had planned, and I adopted it.
4. startHarper still blocks on registration. Resolution waits for the registry write so a runner killed the instant startHarper returns still leaves a record. It is one line to flip to fire-and-forget if the sub-second window is not worth it. Two windows remain open by design and were re-raised this round: between spawn and the registry rename there is no record at all (milliseconds, up to 30s under lock contention), and the clean-exit branch resolves without that wait. Closing them properly means a monitor-owned launch or a child-side handoff protocol, which is a different PR.
5. Declined: fencing the stale-lock reclaim. Reclaim is mtime-only, so a holder stalled past 10s (swap, a paused container) can still complete its read-modify-write after someone else reclaimed the lock and wrote — the token check prevents the wrong unlink, not the wrong write. Raised in five rounds and left alone deliberately: real fencing means threading a token through every critical section and re-validating before each write, which is a design change rather than a fix to a review comment. The lost update drops one instance record, and the 4h backstop still reaps what it described.
6. Declined: replacing wall-clock start times as the identity. ps -o lstart= is derived from boot time on Linux, so an NTP step shifts the string for every process on the host, and the next scan reads live records as PID reuse. That is fail-safe — a mismatched record is dropped, never signalled — so the cost is instances that stop being monitored, not a wrong kill, and the same is true of the unobserved-gap case in bullet 2 above: a group of ours that ended while nothing was watching, whose id was reused by another detached group that then lost its own leader, is indistinguishable. Reaping is best-effort against PID reuse throughout, and the alternative (/proc/<pid>/stat starttime plus boot identity, with a portable fallback) replaces the identity mechanism rather than fixing this one.
7. Declined: validating a pre-existing registry directory. mkdir does not repair the mode of a directory that already exists, so a directory pre-created by another account keeps its permissions. With the pending file O_EXCL, the log O_NOFOLLOW, and now a signalling floor that refuses kill(-1), what remains there is denial of monitoring rather than an arbitrary write or a wide signal, and refusing to run on a directory we do not own is a behaviour change I would not make under a review comment.
8. Declined: a recovery path for an unreadable registry.json. Fail-closed is the decision above, so a corrupt registry stops monitoring for that user until the file is deleted (which the thrown message says), and a running monitor logs a scan failure every interval. Self-healing means deciding that unknown cleanup state is the same as no instances — the exact conflation that erased other runners' reap targets.
9. Declined: spawnSync('ps') on the registration path. Two ~10ms synchronous calls per Harper start, against a multi-second Harper boot. Raised as a blocker in one round and dismissed on the numbers in another. Relatedly, a ps wedged in uninterruptible sleep is unbounded by construction and a stuck filesystem operation cannot be cancelled — Node has no async cancellation for either.
10. Declined: moving the monitor liveness check outside the lock. It is inside the critical section on purpose: the check and the insert have to be atomic, or the monitor's idle exit races a new registration.
11. Pre-existing, filed rather than fixed. killHarper resolves on the leader's exit, so a SIGTERM-ignoring child can still be running when teardown returns; that predates this PR and is independent of the retention invariant. On Windows signalWindowsProcessTree shells out to taskkill asynchronously from a process.once('exit') handler where the loop is already stopped, and the SIGINT/SIGTERM handlers re-raise into listeners that are still attached. Windows keeps the runner-side handlers as its only protection, same as main — the comment documenting the first is restored.
12. A monitor already running keeps its old code. Registration reuses any live recorded monitor, so on a machine that has run an older build of this branch the fix reaches the monitor only after that process exits (idle timeout, or kill) — worth knowing when testing this locally rather than on a fresh box.

Verification

Every new test below was run against the code without its fix and observed failing, then against the fix and observed passing.

• the monitor finishes reaping a group whose leader dies before its children — a TERM-default leader with a TERM-ignoring descendant holding a listener; the runner is SIGKILLed, so only the monitor can act. Without the fix the descendant survives with its port held.
• a reap in flight is finished even though its runner is alive to see the leader exit — the same descendant under an expired lifetime budget with the runner alive, which is the route where the runner's own deregistration used to cancel the escalation.
• a group that outlives its leader stays reapable until its runner dies — nothing is due to be reaped: the leader is killed alone, and the record has to survive several scans so that killing the runner later still reaps the descendant.
• a record whose leader PID has been reused is dropped, not signalled — a seeded record naming an unrelated detached group leader; its group must not receive a signal.
• instances started from two worker threads of one runner are registered and reaped separately — two real workers in one process, each starting a fake Harper tree. Without the CSPRNG id one registration replaces the other and one tree survives the runner's death.
• a process identity does not depend on the environment that read it and a reap cannot be widened into a broadcast by a bad record.
• Earlier rounds' coverage retained: a registry write survives the abrupt death of its writer, a superseded lock holder does not release the lock that replaced it, an unreadable registry is reported rather than silently read as empty, neither startup watchdog can time out a Harper that already reported ready, runner SIGKILL/SIGHUP reaping with port release, the shared-monitor singleton, the lifetime backstop, idle shutdown, graceful killHarper ordering, startup watchdog, tree kill.
• npm run check, npm run build, npm test — 44/44 passed.
• Independent pre-push review across the rounds on this branch (codex + gemini + cursor-grok + cursor-composer + harper-domain), including a planning round whose better-alternative-exists verdict is adopted in docs: add CONTRIBUTING.md and rewrite AGENTS.md #3 above.

Refs #29

_{Review-Coverage: authored=claude; ran=gemini,codex; adjudicated=domain; declined=cursor-grok,cursor-composer; rounds=9 @ e7adea2}

_{Human-Review-Need: 4 (decisions: identity-source, monitor-only-removal, shared-singleton-monitor, resolve-after-registration, default-on, lifetime-backstop-4h, windows-uncovered) @ e7adea2}

[heskew]

Requesting changes at exact head 6388e032ea6dfbe73b571f32280b551f78cf9d40 for one new blocking issue found by Codex.

writeRegistryFile rewrites the shared registry in place, so an abrupt runner death during that write can leave truncated JSON. After the stale lock is reclaimed, readRegistryFile treats the torn file as an empty registry and the monitor overwrites the last recoverable state, losing every orphan-reaping target—including records belonging to other concurrent runners. Fault injection against the real monitor at this head reproduced the failure with a live detached target: {"victimAlive":true,"instancesAfter":0}.

Please write a complete temporary file in the registry directory and atomically rename it while holding the lock, then add a fault/recovery test proving a dead writer leaves either the old or new complete registry. This is blocking because it defeats the PR's headline guarantee under the precise SIGKILL/crash event it is intended to survive.

@gemini-code-assist's four earlier inline findings applied to the superseded per-instance supervisor architecture; all four threads are now outdated/resolved and are not being re-raised. Exact-head install, check, build, all 25 tests, diff checks, and the Ubuntu/Windows Node 22/24/26 matrix pass, but none injects a crash during registry rewrite.

🤖 Posted by Codex on behalf of @heskew

[heskew]

The original torn-registry-write finding is fixed at 76aecd0. Two reproduced P2 findings are attached inline. Type-check, build, all 37 tests, and all seven CI checks passed during the review.

🤖 Posted by Codex on behalf of @heskew; Codex review and runtime probes, with Claude synthesis.