Conversation
…r ownership record During a real incident a fully wedged supervisor (socket alive, ownership record deleted) was reported by doctor as status "current" and the only recovery was `shutdown`, which kills ALL daemons. - doctor now cross-checks each current daemon's hello identity against the supervisor ownership registry (purely local reads) and reports status "ownership-lost" with a one-line remedy; young/stale/foreign daemons are never falsely flagged (owner record is written before listen()). - doctor --fix repairs exactly the affected daemon: verifies it belongs to the doctor's agent dir via its snapshot-cache state dir, holds shutdown admission across kill/socket cleanup/startup-fence wait, confirms process exit after SIGKILL, then relaunches on the same socket; workers/sessions are re-adopted by the existing adoption path. Foreign-agent-dir daemons are declined safely. - ownership-lost errors now name the cause (released vs record missing vs record replaced) and ELOCKED socket-lease errors name the incumbent supervisor (pid + generation). fixes ENG-5302
# Conflicts: # packages/coding-agent/CHANGELOG.md
…ills against pid reuse
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 676e577. Configure here.
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.

During a real incident, a daemon supervisor got fully wedged: its socket was still alive and answering the version handshake, but its ownership record had been deleted out from under it. Every actual command it received failed with an ownership-lost error, yet
prime-agent doctorreported it as status"current"— and the only way out was hand-replicating internal spawn code orprime-agent shutdown, which kills ALL daemons on the machine.What doctor now says and does
Detection (
doctor,ps): doctor cross-checks each healthy-looking daemon's hello identity (generation + pid + process start id) against the supervisor ownership registry, using purely local reads — no daemon commands, since a wedged daemon can't serve them anyway. A daemon whose owner record is missing or was replaced is reported with the new statusownership-lost(human table +--json), plus a remedy footer:No false positives on daemons that are starting up: the owner record is written before
listen(), so a daemon that hasn't written its record yet can never answer a probe. Stopping daemons fail a final pid/start-id liveness re-check; replaced daemons fail the generation match; old/foreign daemons without a hello generation are skipped.Repair (
doctor --fix): restarts only the affected daemon, not the whole fleet:If the kill or relaunch fails, doctor reports honestly and leaves the system no worse (nothing is unlinked or spawned after a failed kill).
Messages: ownership-lost errors now name the cause (
releasedvsrecord missing on diskvsrecord replaced by another owner), and the raw proper-lockfileLock file is already being heldon the socket lease is wrapped to name the incumbent supervisor (pid + generation).Not included (deliberately): self-heal in
assertCurrent(rewriting a deleted owner record). Post-#1449 the registry lives in~/.prime/supervisor-ownersand is not OS-reaped, so external deletion is implausible; self-heal would also fightdoctor --fix(a supervisor rewriting its record between detection and repair) and let a supervisor resurrect ownership an operator deliberately revoked. Both the plan and the reviewer signed off on leaving it out.Checks
npm run check: passdaemon-ps,daemon-supervisor-ownership,public-command,daemon-ps-format,daemon-socket— 5 files, 85 tests, all pass (new coverage: detection true/false cases incl. young-daemon and shutdown-race guards, repair call order under admission, foreign-agent-dir decline, survive-SIGKILL abort, ELOCKED contract, ownership-reason texts)--daemon-socket): started daemon + session, deleted its owner record,doctorreportedownership-lostwith remedy,doctor --fixkilled pid 11323 and relaunched on the same socket (new pid 12570), owner record re-created, the session survived with the same worker pid (re-adoption confirmed), and a second healthy control daemon was untouched (same pid before/after). Evidence bundle: /tmp/prime-agent-features/eng-5302-e2e/LOC: +660/−29 (feature code +290/−26 in daemon-ps, daemon-ps-format, daemon-supervisor, daemon-supervisor-ownership; tests +340/−3; changelog +1).
Fixes ENG-5302 (https://linear.app/primeintellect/issue/ENG-5302/doctor-must-detect-and-repair-daemon-ownership-loss)
Note
Medium Risk
Repair force-kills a live supervisor and unlinks its socket. Guards (admission, pid/start-id recheck, agent-dir match) reduce TOCTOU risk, but a failed relaunch still leaves the daemon down until autostart.
Overview
doctor/psnow flag daemons whose supervisor is still answering hello but whose on-disk ownership record is missing or replaced (ownership-lost), instead of reporting them as healthy.doctor --fixrestarts only those daemons on the same socket so workers/sessions can be re-adopted.Detection compares hello generation + pid + start id against the local ownership registry (no daemon commands). Repair holds shutdown admission across kill, socket cleanup, and startup-fence wait, then relaunches; it declines daemons whose snapshot-cache lives under a different agent dir, and will not unlink the socket if SIGKILL does not confirm exit. Ownership-lost and socket-lock errors now name the cause and incumbent owner.
Reviewed by Cursor Bugbot for commit b4c652d. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Detect and repair daemons that lost their supervisor ownership record in
doctorownership-lostdaemon status, detected by cross-checking the daemon's reported supervisor generation against the on-disk ownership registry viadetectDaemonOwnershipLostindaemon-ps.ts.repairOwnershipLostDaemon, a session-preserving restart flow that force-kills the wedged supervisor, removes the socket, and relaunches — with PID-reuse fencing and admission scoping throughout.planReapnow schedules arestartaction forownership-lostdaemons, including the default daemon, instead of skipping them.forceKillDaemonnow returns a boolean confirming the target is gone and accepts an optionalexpectedProcessStartIdto avoid signaling a reused PID.psoutput appends a per-daemon repair hint (prime-agent doctor --fix) for eachownership-lostentry, rendered in red.Macroscope summarized b4c652d.