fix(coding-agent): one zombie-aware process-liveness probe - #2041
Closed
snimu wants to merge 5 commits into
Closed
Conversation
Five modules hand-rolled bare kill(pid, 0) probes that count zombie processes as alive: the supervisor ownership registry, session leases, the worker-side supervisor launch lock, daemon-ps process stops, and update-restart identity checks. A zombie supervisor therefore kept its ownership record readable as a live owner, wedging every subsequent daemon start on that socket until manual cleanup, and a zombie lease owner kept a session file locked. Delete the local copies and import isProcessAlive from utils/child-process, which already exists and is zombie-aware.
…mbie checks Two consequences of unifying on the zombie-aware liveness probe: stopTrackedProcess signals a process GROUP, but its completion condition checked only the leader pid. With the zombie-aware probe, a leader that zombifies after the group SIGTERM read as stopped, skipping the SIGKILL escalation and letting the caller drop worker records while descendants in the group kept running (the pre-unification bare probe accidentally escalated in that case). Completion now requires the leader gone AND the group empty via the new processGroupExists helper, which also catches the pre-existing leak where a fully reaped leader left live descendants behind. assertDaemonSupervisorOwnerCurrent runs in the 250ms per-claim fence poll, and the zombie half of isProcessAlive spawns a synchronous ps on macOS/BSD, so unification turned every fence tick into a process spawn. The fence path now checks existence with kill(0) on every tick (reaped owners are caught immediately) and confirms non-zombie state at most once per 5s per owner pid, mirroring the supervisor's existing throttled-identity-probe pattern. Acquisition and admission paths keep the full-strength probe.
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes 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 06da0f8. Configure here.
…entity degradation, bounded confirmation cache Review follow-ups on the group-stop reshape: - An unreaped zombie member held processGroupExists true forever, so a stop whose targets had all exited burned the full escalation budget and reported failure. Completion now uses processGroupHasLiveMember: zombies have exited and only their parent can reap them, so they no longer block completion while running descendants still do. - A reaped leader failed the getProcessStartId identity gates and the stop bailed before signaling the group. The gates protect against pid reuse, which only applies while the leader process exists; with the leader gone, teardown degrades to the group checks (a pgid cannot be reused while members hold it). - The owner zombie-confirmation cache never dropped entries for supervisors nothing asserts anymore; expired entries are now pruned when a confirmation is recorded, keeping the cache bounded.
…comments One spawnZombieProcess fixture replaces three copies of the perl fork scaffold, and multi-line comment blocks collapse to one-line invariant guards. No behavior or coverage change.
…lding the pgid Degrading the identity gates when the leader is gone opened a reuse window the old refuse-to-signal behavior did not have: if every group member exits between the checks and the signal, the pgid can be recycled (a new process taking that pid as its session/group id) and the group SIGKILL lands on an unrelated same-user workload. signalProcessGroupIfHeld now re-checks at signal time: a leader process (even a zombie) anchors its pgid against reuse, and once the leader is gone a live member must hold the pgid — narrowing the residual exposure to the inherent kill() TOCTOU that every single-pid signal, including the old code's, already has. Empty groups are not signaled at all.
Contributor
sethkarten
added a commit
that referenced
this pull request
Sep 7, 2026
) * refactor(coding-agent): move the semantic-edge ledger onto the event-log substrate The recorder's private append/replay/repair IO is deleted; EventLog owns it, the same move #1987 made for the RLM spawn ledger. One durability rule is unified in the substrate rather than dropped: an unterminated final line is an uncommitted append, skipped on read and truncated before the next append — never newline-completed and never surfaced to a consumer whose next append destroys it. * fix(coding-agent): make the explicit ledger reader's ENOENT contract atomic readSemanticEdgeLedger probed with statSync before reading through EventLog, which swallows ENOENT; a ledger deleted between the two returned [] instead of throwing. The missing-file decision now lives at the single open (replaySync missingFileThrows), so no check-then-read window exists. * docs(coding-agent): state the event-log tail rule once The unterminated-tail contract was restated four times (module doc, replaySync doc, two test comments). It now lives once in the module doc; the method doc keeps only its own parse/missing-file semantics and the test comments reference the contract. * fix(coding-agent): write event-log appends fully and gate appends on tail repair writeSync may write short (ENOSPC after a prefix); appendSync now loops until the payload is fully on disk so write-before-action callers never act on a torn record reported as success. A tail-repair failure (e.g. append-only ACL permitting O_APPEND but not r+) now propagates instead of being swallowed: writing through an unrepaired torn tail would weld it to the new record as permanent interior corruption. ENOENT and the concurrent-writer instability path keep their existing semantics. * fix(coding-agent): reclaim short event-log writes instead of completing them The rlm spawn ledger is multi-writer by documented design (supervisor plus each worker over one file), so completing a short O_APPEND write with a second write could interleave with a rival append and weld two records. A short write now truncates its own torn prefix back off (only while this writer still owns the tail) and fails the append; a torn tail is read-tolerated, a weld is permanent corruption. The append fd opens a+ so the ownership check can read the tail. * fix(coding-agent): leave the torn tail on a short write instead of reclaiming it The tail-match reclaim could truncate a rival's committed record whose final bytes coincide with our torn prefix - committed-data loss, strictly worse than the torn tail it prevented. A short write now just fails the append: the torn tail is the one tolerated shape, skipped on read and truncated by any writer's next repair (verified for both topologies: a resumed single-writer recorder repairs on its first append; every rlm-ledger writer repairs before each append). * refactor(coding-agent): compress event-log comments * fix(ai): omit the default service tier, reprice cache writes from message_delta, repoint the zai default Incorporates #2032 at f82c7fa. * fix(tui,coding-agent): survive lone surrogates in table cells and terminate the WebP EXIF scan Incorporates #2033 at a3d1139. * fix(coding-agent): restart dead kernels on ensure() and read mcp>=2 tool schemas Incorporates #2034 at 749e216. * fix: one crash-safe owner for durable state writes Incorporates #2035 at f0f02d2. * fix(coding-agent): one zombie-aware process-liveness probe Incorporates #2041 at 92a0eac. * fix(coding-agent): snapshot transfer ids from the materialized cursor; mismatches settle the transfer, not the worker channel Incorporates #2044 at 5af3bbe. * fix(coding-agent): failed workers recover on touch; roster gaps answer a structured recovering error Incorporates #2047 at 77b747a. * fix(coding-agent): seven session and IO correctness defects Incorporates #2037 at 41b5d72. * fix(coding-agent): coalesce child-usage attribution and gate agent-status persistence on real changes Incorporates #2050 at 6b0af5d. * fix(coding-agent): incremental single-flight session metadata scans Incorporates #2043 at df032c1. * fix(coding-agent): memoize the passive RLM topology derivation Incorporates #2051 at 0ee114c. * fix(coding-agent): preserve accounting and metadata across deferred updates Keep durable child-usage aggregates separate from pending sibling usage. Retry optional topology metadata after transient reads. Completes #2050 and #2051 integration. * fix: preserve session accounting and read-only persistence boundaries --------- Co-authored-by: Seth <seth@primeintellect.ai>
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
2 participants
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.
Part of the worker-state single-truth program (Linear RES-1270); squashes discussion #1732.
Purpose
A zombie process passes a bare
kill(pid, 0)probe, andgetProcessStartIdstill reports its original start time, so identity checks pass too. Five modules hand-rolled that bare probe and therefore disagreed with the canonical zombie-awareisProcessAliveinutils/child-process.ts:daemon-supervisor-ownership.ts— a zombie supervisor's ownership record read as a live owner: every later daemon start on that socket throwsDaemonSupervisorAlreadyRunningErroruntil someone manually reaps the zombie (the [Bug] Daemon can never start again after its supervisor is left unreaped as a zombie #1732 report).core/session-lease.ts— a zombie lease owner kept a session file locked.daemon-mode.ts(worker-side supervisor launch lock) — a zombie lock owner suppressed supervisor relaunch.cli/daemon-ps.ts— stop/kill wait loops treated an exited-but-unreaped process as still running.cli/daemon-update-restart.ts— restart coordination treated a zombie predecessor as alive.Change
Pure consolidation: delete the five local
kill(pid, 0)copies and import the sharedisProcessAlive(zombie-aware, EPERM-tolerant). No new mechanism; net src LOC: -41 (+6/-47, all additions are import lines).The evidence sweep listed four copies;
daemon-mode.tshad a fifth identical private method (added in #383), deleted here for the same reason. Remainingkill(pid, 0)sites were left alone deliberately:daemon-ps.tsverifyHelloSupervisorPidchecks a pid that just answered a hello (cannot be a zombie mid-reply),daemon-launch.tshasProcessIdentityExitedis an inverted has-exited check where a zombie only extends a bounded wait, andkernel/bootstrap.tsbelongs to the atomic-persistence program.Tests
One new pin in
daemon-supervisor-ownership.test.ts: a real zombie (perl fork trick, same aschild-process.test.ts) written into a conflicting owner record must be reclaimed by a successor acquisition instead of throwingDaemonSupervisorAlreadyRunningError. Verified fail-unfixed: reverting the ownership hunk makes the test fail withAlreadyRunning. The shared helper's zombie semantics were already pinned inchild-process.test.ts.Ran locally: daemon-supervisor-ownership, session-lease, child-process, daemon-ps, proper-lockfile-compromise, daemon-supervisor-monitor, regressions 4606/4600/879 — 171/171 pass.
Note
Medium Risk
Changes ownership, lease, and shutdown liveness in the daemon path; a 5s owner-alive cache can briefly treat a just-zombied PID as live until re-probed.
Overview
Unreaped zombie processes no longer count as live owners across daemon supervision, session leases, supervisor launch locks,
daemon psworker stops, and update-restart coordination. Five duplicatedkill(pid, 0)helpers are removed in favor of the sharedisProcessAliveinchild-process.ts(existence plus non-zombie).child-process.tsgains process-group helpers:processGroupHasLiveMember(zombies do not block “stopped”),signalProcessGroupIfHeld, andprocessGroupExists.daemon-psstopTrackedProcesstreats a stop as complete only when the leader is not alive and no running group member remains.daemon-supervisor-ownershipusesisOwnerProcessAlivewith a 5s positive cache so 250ms fence polls stay cheap (kill(0)every tick,pszombie check at most once per interval). Tests add aspawnZombieProcessfixture and pins for zombie owners, group-stop behavior, and fence-poll caching.Reviewed by Cursor Bugbot for commit 92a0eac. Bugbot is set up for automated code reviews on this repo. Configure here.
LOC
Total src: +110/−58 (net +52); tests: +146/−24 (net +122).
Note
Replace local liveness checks with shared zombie-aware process probe
isProcessAlive,processGroupHasLiveMember, andsignalProcessGroupIfHeldto child-process.ts and replaces module-localisProcessAlivehelpers across daemon-ps, update-restart, session-lease, and daemon-mode with the shared probeps-backed zombie probes more than once per 5 seconds per pidtrackedProcessStoppedrequires both the leader to be non-alive and no live group member; a group with live descendants that cannot be listed bypswill conservatively report a live member, which may delay stop completion on platforms wherepslisting failsMacroscope summarized 92a0eac.