Supervisor crash orphans daemons; restarted supervisor doesn't re-adopt and can start duplicates

[chirayuk]

While evaluating pitchfork (installed via mise) I tested what happens to daemons when the supervisor crashes, and found that a crash orphans running daemons, and a subsequently restarted supervisor neither re-adopts nor kills them — so a later pitchfork start runs a duplicate instance alongside the still-running orphan.

Environment: pitchfork 2.16.0, macOS (arm64), installed via mise use -g pitchfork.

Repro:

1. pitchfork.toml with a trivial daemon:

   [daemons.heartbeat]
   run = "sh -c 'while true; do date >> hb.log; sleep 1; done'"

2. pitchfork start heartbeat — daemon runs as a direct child of the supervisor.
3. kill -9 <supervisor pid> (simulating a supervisor crash/panic).
4. The daemon keeps running, reparented to PID 1. Expected so far — SIGKILL leaves no chance to clean up, and macOS has no PR_SET_PDEATHSIG equivalent.
5. Run any pitchfork command so the supervisor auto-restarts. At this point state.toml still has the correct record from before the crash (pid = <orphan pid>, status = "running"), and that pid is alive — but pitchfork list reports the daemon as available.
6. pitchfork start heartbeat → starts a second instance. Both processes now run concurrently (verified: both appending to the same log).

Expected (or at least hoped): on startup, the supervisor could consult the persisted state — it already records pid, status, and the command — and for each daemon marked running with a live pid either re-adopt it (resume monitoring) or, if adoption isn't feasible, stop the orphan before considering the daemon startable. Either would prevent the silent-duplicate outcome; today the stale-but-accurate state entry is discarded.

For contrast, the graceful path is clean: pitchfork supervisor stop takes its child daemons down with it, and normal restarts leave nothing behind. This only bites on a genuine crash.

Happy to provide more detail or test a fix. And thanks for pitchfork — the mise integration and the cron retrigger = "always" semantics are exactly what I needed to replace a hand-rolled supervisor.

[jdx]

Thanks for testing and the report! We'll get on this

[jdx]

my clanker is still clanking but it appears this was a bug which was fixed in 2.17.0

[jdx]

Thanks again for the excellent report and repro — I dug into this and can confirm exactly what you saw. The good news: upgrading to v2.17.0 fixes the core bug.

Root cause: in 2.16.0, the supervisor never loaded state.toml on startup — Supervisor::new() initialized an empty in-memory state instead of reading the file. Pitchfork actually already has startup orphan cleanup (supervisor.cleanup_orphans, on by default) designed for precisely your crash scenario, but with the state never loaded it saw zero daemons and silently did nothing. That's also why pitchfork list showed available: the restarted supervisor rewrote state.toml from its empty state, erasing the (accurate) pre-crash record, so the daemon appeared config-only.

This was fixed in v2.17.0 (#585 changed the supervisor to load persisted state on startup) — though it landed inside a test-infrastructure PR, so the changelog doesn't call it out. I verified your exact repro against 2.17.0: after kill -9 + auto-restart, the orphaned daemon is terminated during supervisor startup, and a subsequent pitchfork start runs a single instance.

Follow-ups in flight:

1. A regression test for the crash → restart → no-duplicate path (the fix deserves a guardrail), plus a fix for a related edge: the safety check meant to prevent killing an unrelated process when the OS recycles a daemon's PID doesn't currently work. We'll record process identity (start time) at spawn so cleanup can tell a genuine orphan from a recycled PID.
2. Your re-adopt suggestion: on restart, the supervisor will re-adopt live orphans whose identity matches — resuming monitoring by polling, since they're no longer child processes — instead of killing them. One inherent caveat: log capture can't be restored for an adopted process (its stdout/stderr pipes died with the old supervisor), so its output is captured again only after that daemon's next restart.

Thanks for the detailed writeup — it made this straightforward to track down.

This comment was generated by Claude Code.

[jdx]

Follow-ups are up:

• fix(supervisor): verify process identity before killing orphaned daemons #632 — regression tests for the crash → restart → no-duplicate path, plus a fix making the PID-recycle safety check actually work (process identity is now recorded as PID + kernel start time at spawn).
• feat(supervisor): re-adopt orphaned daemons by default after a crash #633 (stacked on fix(supervisor): verify process identity before killing orphaned daemons #632) — re-adoption as the default: a restarted supervisor now resumes supervising live orphans whose identity matches instead of killing them (supervisor.orphan_policy = "adopt" | "kill"). Adopted daemons are poll-monitored; the one gap is stdout/stderr capture, which cannot be re-attached to a non-child process — log capture resumes on the daemon's next restart. We have a design sketched for re-attaching via /proc/<pid>/fd on Linux as a further follow-up.

This comment was generated by Claude Code.