feat(supervisor): re-adopt orphaned daemons by default after a crash

[jdx]

Context

Implements the re-adoption half of #631 (rebased onto main now that #632 is merged): the reporter asked that a restarted supervisor either re-adopt orphaned daemons or stop them before considering them startable. #632 made the kill path safe; this PR makes re-adoption the default, built on its fail-closed identity machinery.

Behavior

On supervisor startup, for each state daemon whose recorded PID is alive and whose identity (PID + kernel start time) is verified:

• orphan_policy = "adopt" (new default): the process keeps running and supervision resumes — state, resolved ports, and proxy routing stay intact. Since the process is no longer a child of the supervisor, child.wait() monitoring is impossible; a poll monitor (2s) takes over, anchored to the verified start time so a later PID recycle is never mistaken for the daemon. It mirrors the regular monitor's exit path: stop transitions to stopped and fires on_stop/on_exit (even when stop() finalizes the record before the poll monitor wakes); unexpected death marks errored with unknown exit code (exit codes of non-children are unobservable), feeding the existing retry machinery — a retried daemon respawns as a normal child again. Legacy title-matched records get their start_time persisted at adoption.
• orphan_policy = "kill": pre-adoption behavior — the pinned, fail-closed termination from fix(supervisor): verify process identity before killing orphaned daemons #632.
• Unverifiable identity retains running state untouched (fail closed, both policies); dead or recycled PIDs reset state. cleanup_orphans = false still skips orphan handling entirely (legacy escape hatch; docs updated to point at orphan_policy).

Interval-watcher reconciliation

A new monitored registry (daemon → PID under a live monitor task) lets the 10s interval watcher catch state-running daemons that nothing is watching, applying the same fail-closed checks and orphan policy as the startup scan: dead or recycled PIDs are marked errored(-1) (previously a stale running record could persist forever when startup cleanup was skipped), live verified ones are adopted or terminated per policy. Registration happens synchronously before monitor spawn, with restart-overlap-safe RAII removal, so supervised daemons never appear unmonitored. The poll monitor re-verifies record ownership before mutating state, so a stale monitor can never clobber a successor process's state.

Known limitation (follow-up planned)

Log capture cannot be restored for an adopted process — its stdout/stderr pipes died with the old supervisor. Output capture resumes on the daemon's next restart. A follow-up will explore re-attaching via /proc/<pid>/fd on Linux (reopening the surviving pipe inode recovers capture and up to 64KB of gap output), and possibly FIFO-based capture for a cross-platform fix; connected-socket approaches don't work here since a broken stream fd can't be re-pointed at a new listener.

Tests

• crashed supervisor re-adopts running daemon by default — same PID survives crash+restart, still running, start refuses a duplicate, stop terminates the adopted process and completes the transition
• adopted daemon death is detected and marked errored — external kill of the adopted process is caught by the poll monitor
• stopping an adopted daemon fires on_stop and on_exit hooks — regression for the stop-hooks race found in review
• the kill test is pinned to PITCHFORK_ORPHAN_POLICY=kill; the recycled-PID bystander test is policy-independent and unchanged

Review feedback from Bugbot (stop hooks, kill policy on the interval path) and Greptile (successor-state clobbering, P1) is addressed in the current revision.

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Supervisors can now reconcile orphaned daemon processes after restarting.
  • By default, verified orphaned daemons are re-adopted and continue running under supervision.
  • Added an option to terminate orphaned daemons instead.
  • Re-adopted daemons are monitored for termination and process identity changes.

• Bug Fixes

  • Improved handling when daemon monitoring is unexpectedly lost.
  • Unverifiable or mismatched processes now fail safely without being terminated.

• Documentation

  • Expanded configuration guidance for orphan verification, adoption, termination, and fail-closed behavior.

---

Note

High Risk
Changes core supervisor lifecycle around process identity, concurrent monitors, and startup/interval reconciliation; mistakes could kill wrong processes, duplicate daemons, or corrupt state/hooks.

Overview
After an unclean supervisor exit, verified live daemon processes are reconciled instead of always being killed. New supervisor.orphan_policy defaults to adopt: keep the process, preserve state/ports/proxy routing, and resume supervision via a 2s poll monitor (no restored stdio; unknown exit codes → errored and retries). kill preserves the prior terminate-on-restart behavior.

A monitored registry with tokenized MonitoredGuard registration (before Running state is written on normal starts) distinguishes supervised daemons from orphans. The interval watcher runs the same fail-closed identity checks and policy for unmonitored running records (dead/recycled → errored(-1)).

Docs/schema add orphan_policy; cleanup_orphans is described as reconciliation. Bats cover default adoption, adopted death/hooks, and pin the old kill test with PITCHFORK_ORPHAN_POLICY=kill.

^{Reviewed by Cursor Bugbot for commit 4fa3618. Bugbot is set up for automated code reviews on this repo. Configure here.}

[coderabbitai]

📝 Walkthrough

Walkthrough

Supervisor orphan handling now verifies process identity and applies an adopt or kill policy. Adopted daemons receive poll-based monitoring, state updates, and lifecycle hooks. Monitor registration and reconciliation ordering prevent unmonitored running daemons from being missed.

Changes

Orphan reconciliation

Layer / File(s)	Summary
Orphan policy contract settings.toml, docs/public/schema.json	Documents verified orphan reconciliation and adds supervisor.orphan_policy with adopt and kill values.
Monitor ownership tracking src/supervisor/mod.rs, src/supervisor/lifecycle.rs	Tracks active daemon monitors and registers monitor ownership before spawning monitoring tasks.
Reconciliation and adoption flow src/supervisor/adopt.rs, src/supervisor/mod.rs	Reconciles unmonitored daemons, validates PID and start time, adopts or terminates verified processes, and marks unverifiable deaths as errored. Adopted processes are polled and lifecycle state and hooks are updated on exit.
Adoption behavior validation test/supervisor.bats	Tests default adoption, adopted-process failure detection, lifecycle hooks, and explicit kill-policy behavior.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Supervisor
  participant OrphanedDaemon
  participant StateStore
  participant LifecycleHooks
  Supervisor->>StateStore: Find running daemon without monitor
  Supervisor->>OrphanedDaemon: Verify PID and start time
  Supervisor->>OrphanedDaemon: Adopt and poll liveness
  OrphanedDaemon-->>Supervisor: Terminate or fail identity check
  Supervisor->>StateStore: Record stopped or errored state
  Supervisor->>LifecycleHooks: Run applicable lifecycle hooks

Loading

Possibly related PRs

• jdx/pitchfork#632: Shares verified PID and kernel start-time checks for orphan cleanup.
• jdx/pitchfork#620: Overlaps with monitor-task lifecycle and guard registration.
• jdx/pitchfork#635: Intersects with stale monitor callbacks and daemon state handling.

Poem

A bunny found a daemon astray,
And checked its start time before the day.
“Adopt,” said the burrow, “or safely depart,”
While monitors watched with a careful heart.
Hooks chimed softly when processes ceased—
Orphaned no longer, the supervisor feasted!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: orphaned daemons are re-adopted by default after a supervisor crash.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[greptile-apps]

Greptile Summary

Re-adopts verified orphaned daemons by default after supervisor crashes.

• Adds configurable adopt and kill orphan policies for startup and interval reconciliation.
• Tracks monitor generations to prevent stale monitors and reconciliation passes from mutating successor state.
• Adds polling-based lifecycle handling for adopted processes, including terminal state and hook processing.
• Updates generated settings schema, documentation, and supervisor integration tests.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failures remain in the fixes associated with the previous ownership and successor-state threads.

Important Files Changed

Filename	Overview
src/supervisor/adopt.rs	Implements orphan reconciliation, monitor-generation ownership, adopted-process polling, and guarded terminal-state handling; the displayed ownership fixes appear complete.
src/supervisor/lifecycle.rs	Registers each child monitor before exposing its running state, closing the reconciliation window discussed in the previous threads.
src/supervisor/mod.rs	Integrates monitor tracking, periodic reconciliation, startup adoption, and orphan-policy selection into the supervisor.
settings.toml	Defines and documents the new adopt-by-default orphan policy and updated cleanup semantics.
docs/public/schema.json	Publishes the generated schema entry for the new orphan policy.
test/supervisor.bats	Covers explicit kill behavior, default re-adoption, adopted-process death detection, and stop-hook execution.

_{Reviews (8): Last reviewed commit: "fix(supervisor): register monitors befor..." | Re-trigger Greptile}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

docs/public/schema.json (1)

1084-1090: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win

Constrain orphan_policy to supported values.

The runtime only accepts adopt and kill; schema validation currently accepts any string and tooling can validate a config that silently falls back to adopt.

Proposed schema constraint

 "orphan_policy": {
   "description": "What to do with live orphaned daemons on supervisor startup: adopt or kill",
   "type": [
     "string",
     "null"
-  ]
+  ],
+  "enum": ["adopt", "kill", null]
 }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/public/schema.json` around lines 1084 - 1090, Constrain the
orphan_policy property in the schema to the supported values “adopt” and “kill”
by adding an enum constraint while preserving its nullable behavior.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/supervisor/mod.rs`:
- Around line 1349-1356: Update the supervisor decision flow around
process_identity_matches() so adopt and kill actions require a persisted
daemon.start_time that matches the current process start time; title-only legacy
records must retain their running state and receive neither action. Apply the
same start-time requirement in the reconciliation logic in adopt.rs, using its
existing identity and state-handling symbols.

---

Nitpick comments:
In `@docs/public/schema.json`:
- Around line 1084-1090: Constrain the orphan_policy property in the schema to
the supported values “adopt” and “kill” by adding an enum constraint while
preserving its nullable behavior.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Central YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: 21b355e5-ee10-4d97-9537-96b72eb2d5f2

📥 Commits

Reviewing files that changed from the base of the PR and between fa85337 and f93d8a1.

📒 Files selected for processing (6)

• docs/public/schema.json
• settings.toml
• src/supervisor/adopt.rs
• src/supervisor/lifecycle.rs
• src/supervisor/mod.rs
• test/supervisor.bats

[cursor]

Startup dead PID skips retries

Medium Severity

After a supervisor crash, startup orphan cleanup clears a still-running record whose PID is already dead to stopped, while the new interval reconciliation marks the same situation errored(-1). Because cleanup runs first on every restart, an adopted daemon that exits before the poll monitor persists state never becomes errored, so configured retries and fail hooks may not run.

Additional Locations (1)

• src/supervisor/adopt.rs#L174-L182

 

^{Reviewed by Cursor Bugbot for commit 98df44c. Configure here.}

[jdx]

The divergence is real but deliberate, and I'd keep it in this PR:

• Startup's dead-PID → stopped is merged fix(supervisor): verify process identity before killing orphaned daemons #632 behavior (and predates it) — changing it here would silently alter semantics jdx already shipped, and the merged test suite pins it (orphaned daemons are cleaned up on supervisor restart asserts stopped).
• The two paths answer different questions. At startup, a dead PID means "this daemon did not survive the supervisor outage" — historically treated as a clean slate. At runtime, the reconciler finding a dead unmonitored PID means "a supervised daemon died and nobody recorded it" — errored(-1) is what feeds retries there.
• The scenario where this bites (adopted daemon dies, then the supervisor crashes again within the poll monitor's 2s window, so the death is only ever seen by the next startup scan) is a narrow double-crash race, and its outcome — stopped, no retry — matches what every pre-adoption version did for all crash-window deaths.

That said, I think there's a fair argument that startup should also mark previously-running dead daemons errored so retries fire after a supervisor crash — that would be a behavior change best made deliberately (and tested) in a follow-up rather than folded into this one. Happy to file it.

This comment was generated by Claude Code.

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

There are 2 total unresolved issues (including 1 from previous review).

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 0f4bf18. Configure here.}