fix(supervisor): make child-monitor exit finalization atomic

[jdx]

Context

Second in a stack; based on #658 (merge that first). Found while hardening the adopted-daemon path in #657 — the bots never flagged it because lifecycle.rs wasn't in any of those diffs.

The race

The child monitoring task takes an ownership snapshot:

let current_daemon = SUPERVISOR.get_daemon(&id).await;   // lock acquired and released

…then, some lines later, writes the terminal state with a separate upsert_daemon call that acquires the lock again. Nothing holds it across both. On the multi-threaded runtime a concurrent start() / restart can install a successor in that gap, and the write then clears the successor's PID and replaces its Running status — orphaning a live process and silently undoing the restart. That is the same silent-duplicate failure mode #631 was about, reached from the other direction.

The window is narrow (no .await between the two, so only a genuine cross-thread interleaving hits it), but it is real, and it is exactly the window I closed on the adopted path in #657. That left the adopted poll monitor strictly safer than the child monitor it was modelled on — an asymmetry worth removing.

Change

Both monitors now share finalize_monitored_exit, which revalidates two things inside the same state-lock critical section as the write:

• the registry entry still carries this monitor's token (so a successor that recycled the same numeric PID is still detected), and
• the record still names this monitor's PID.

If either fails, the monitor stands down and logs at debug instead of writing. This also removes the poll monitor's hand-rolled copy of the same logic, so there is one implementation rather than two.

Behavior is otherwise unchanged: the existing already_stopped / pre_drain_is_stopping gates still decide whether to write, and the recorded status and last_exit_success values are identical.

Tests

Existing coverage exercises the paths this touches — restart triggers on_stop and on_exit hooks, restart all includes ad-hoc daemons, and the adoption suite all pass (61/61 across supervisor.bats + basic.bats, 519/519 unit). I did not add a test for the race itself: reproducing a cross-thread interleaving with no await point between the two operations is not something a bats test can do deterministically, and a flaky test would be worse than none.

🤖 Generated with Claude Code

---

Note

Medium Risk
Touches core supervisor state on every monitored exit; behavior is intended to be equivalent when no race, but incorrect stand-down would leave stale state and wrong stand-down would still be a serious bug.

Overview
Daemon exit finalization is now one shared, lock-held path for both the child monitor and the adopted poll monitor via finalize_monitored_exit.

That helper re-checks the monitor token and that the state file still records the exiting PID, then clears runtime fields and writes terminal status and last_exit_success in a single state-lock section. If a concurrent start/restart installed a successor, the stale monitor skips the write and logs at debug instead of clearing a live PID or undoing a restart.

The child monitor no longer uses a separate upsert_daemon after an unlocked get_daemon snapshot; it keeps the token from MonitoredGuard::register and calls the same helper. The poll monitor drops its duplicated inline finalization in favor of that call.

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

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

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

Review profile: CHILL

Plan: Pro Plus

Run ID: 5dccda4d-acf9-4ad5-a08a-1baa40e647d7

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

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

Makes monitored-exit finalization atomic.

• Adds a shared token-and-PID-validated terminal-state update for child and adopted-daemon monitors.
• Prevents stale monitors from overwriting a concurrently installed successor.
• Replaces manual timeout ceiling arithmetic with div_ceil.

Confidence Score: 5/5

The PR appears safe to merge, with monitored-exit state updates now protected against successor races.

The shared finalizer validates both monitor generation and PID while holding the state lock, preserves the prior terminal-state field behavior, and introduces no supported-toolchain incompatibility.

Important Files Changed

Filename	Overview
src/supervisor/adopt.rs	Introduces shared lock-held exit finalization and reuses it in the adopted-daemon poll monitor without identifying a concrete regression.
src/supervisor/lifecycle.rs	Carries the monitor generation token into child-exit handling and atomically validates ownership before writing terminal state.
src/procs.rs	Replaces equivalent manual ceiling division with a compiler-compatible div_ceil call.

_{Reviews (1): Last reviewed commit: "fix(supervisor): make child-monitor exit..." | Re-trigger Greptile}