…oxlite-ai#523)

Two production code paths spawn shim subprocesses, send them SIGKILL,
and never `waitpid` — so a dead shim sits in `/proc` as `State: Z`
indefinitely, holding the PID slot and (in `boxlite serve` mode)
blocking init from reparenting because the daemon is still its parent.

**#1: async death (OOM / external SIGKILL/SIGTERM / internal panic).**
The shim dies without anyone in the runtime asking it to. The only
loop already polling shim liveness is health check (`box_impl.rs::
spawn_health_check`), but its `shim_died` branch did just one thing:
mark `Stopped` / `Unhealthy` in the DB and break. No `waitpid`. The
`is_process_alive` detector correctly treats `State: Z` as dead, so
the branch *fires* on a zombie — it just doesn't reap.

Fix: in the `shim_died` arm, `libc::waitpid(pid, _, WNOHANG)` in a
short bounded retry loop. SIGKILL → zombie transition is not always
synchronous with the wait queue: WNOHANG can briefly return 0 before
the kernel posts SIGCHLD/wait status, even when `/proc` already shows
`State: Z`. 500 ms ceiling with 10 ms backoff covers the race without
ever stalling the health-check loop.

The fix only takes effect if a health check task is *running*, so:

  - `AdvancedBoxOptions::default()` now spawns a health check with
    `HealthCheckOptions::default()` instead of `None`. Default
    interval 30 s, 60 s start period, 10 s ping timeout — the
    pre-existing defaults; no operator-visible cost beyond one ping
    per box per 30 s.
  - The decision is documented inline on the `health_check` field:
    health check is the *only* always-on shim-state watcher; explicit
    `health_check: None` opts out of both pings and zombie reaping
    (operator takes responsibility).

**#2: `boxlite serve` + REST `rm --force`.** Daemon's `rt_impl::
remove_box(force=true)` calls `kill_process(pid)` (SIGKILL by PID,
bypassing the canonical `stop()` path that *does* reap via
`Child::wait()`). The handler returns immediately and the box is
removed from the DB. The shim's `PPid` stays pointed at the daemon
(still alive), so init can't help.

Fix: after `kill_process`, run the same `WNOHANG`-with-deadline
polling loop already used in `vmm/controller/shim.rs::ShimHandler::
stop`'s SIGTERM path. 2 s deadline matches the existing
`GRACEFUL_SHUTDOWN_TIMEOUT_MS`; if SIGKILL doesn't yield a reapable
state in 2 s, the kernel is wedged and a longer wait won't help.

Three reproducer tests, each two-side verified (revert the relevant
production change → test goes red with the exact zombie signal;
restore → test goes green):

  - `boxlite::tests::health_check::
       health_check_becomes_unhealthy_when_shim_killed`
       (existing test on main, now actually passes — the
       `PerTestBoxHome::drop` zombie-leak panic is the load-bearing
       failure; this PR adds a pin on `/proc/<pid>/status` not being
       `State: Z` so the regression signal is inline)
  - `boxlite::tests::health_check::
       health_check_becomes_unhealthy_when_shim_sigtermed`
       (new — same shape, SIGTERM, covers k8s liveness initial
       signal / cgroup OOM initial / systemctl stop / plain kill)
  - `boxlite_cli::tests::serve_rm_force_zombie::
       boxlite_serve_rm_force_active_box_no_zombie_left_in_proc`
       (new — spawns real `boxlite serve` subprocess, drives REST
       `DELETE /v1/boxes/<id>?force=true` via CLI `--url`, watches
       `/proc/<shim_pid>/status` from *outside* the daemon. The
       failing-path panic captures `PPid:` against the daemon's
       PID — the load-bearing diagnostic that proves init isn't
       going to clean this up because the daemon is still alive
       and still the parent.)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

The `shim_died` arm sits inside the health check `tokio::spawn` task,
so `std::thread::sleep` between WNOHANG retries was blocking a tokio
worker thread for the duration of the 10 ms tick. Single-box workloads
never notice — the worst case is a 500 ms reap ceiling on one task.
But during a host-wide shim death event (host reboot, batch SIGKILL,
runtime tear-down), every box's `shim_died` arm fires at roughly the
same time. With tokio's default worker count = host CPU count, even
8–16 concurrent `shim_died` arms can saturate the runtime workers and
stall unrelated tasks (REST handlers, exec waits, snapshot work) for
up to the full 500 ms reap budget.

Swap to `tokio::time::sleep(...).await`. The retry now yields between
WNOHANG probes, the worker thread is free to drive other tasks, and
the 500 ms ceiling stays in place. No semantic change for the
single-event case the 3 reproducer tests exercise — all three still
green:

  - `health_check_becomes_unhealthy_when_shim_killed`  (21.8 s)
  - `health_check_becomes_unhealthy_when_shim_sigtermed`  (21.8 s)
  - `boxlite_serve_rm_force_active_box_no_zombie_left_in_proc`  (12.3 s)

Also: a small perf-measurement test alongside in
`src/boxlite/tests/health_check_perf.rs`. Spawns one box with
`health_check = None` and one with `health_check = Some(100ms tick)`,
measures `/proc/self/stat` CPU delta of the boxlite test process over
a 10 s window, and prints the per-tick cost so a reviewer can verify
the default-on decision empirically. On this host: ~900 us per tick
in the on-box minus the off-box baseline. Projected to the default
30 s interval: ~30 us/sec per box — 0.3 % of one core at 100 boxes,
3 % at 1000.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

… hatch

boxlite-ai#613 makes the per-box health check task default-on so the new
zombie-shim reaper (Issue boxlite-ai#523) has a watcher to piggy-back on. The
schema field `BoxOptions.advanced.health_check: Option<HealthCheckOptions>`
already lets Rust library users opt out via `None`, but CLI users
(`boxlite run` / `create`) and REST/SDK clients (Python, Node, Go all
route through `POST /v1/boxes`) had no way to express that choice —
exactly the gap boxlite-ai#597 hit for `cap_overrides`.

Surface plumbed end-to-end:

  - `ManagementFlags { no_health_check: bool }` — `--no-health-check`
    on `boxlite run` / `create`. When set, `apply_to` clears
    `opts.advanced.health_check = None`.
  - REST client-side `CreateBoxRequest` gets
    `health_check_disabled: Option<bool>`; `from_options` flips it to
    `Some(true)` only when the operator already disabled health check
    in the input options, so the wire form stays byte-identical for
    every other call.
  - Server-side `CreateBoxRequest` (in `cli/src/commands/serve/types.rs`,
    `#[serde(deny_unknown_fields)]`) gains the same field. The
    `build_box_options` mapper builds a manual `AdvancedBoxOptions`
    instead of falling through `..Default::default()`, then forces
    `health_check = None` iff the wire says so.

Three-state semantics for `health_check_disabled`:

  - `Some(true)` → explicit disable; box runs with no watcher, no
    zombie reaping (operator takes responsibility — documented inline).
  - `Some(false)` → "use the server default" (= currently `Some(...)`).
    Same effect as omitting the field, but documents the intent on
    the wire.
  - absent → server default. Pre-boxlite-ai#613 clients keep the legacy POST
    body shape and get the new default-on behaviour without code
    changes.

Seven unit tests cover the surface symmetrically:

  - CLI: `management_flags_no_health_check_clears_advanced_field` /
    `management_flags_no_health_check_unset_keeps_default_on` —
    flag-on clears, flag-off preserves; pre-asserts the baseline
    so a regression that broke the default-on schema would also fail.
  - REST client: `test_create_box_request_carries_health_check_disabled_on_the_wire` /
    `test_create_box_request_omits_health_check_disabled_when_default`
    — `None` in BoxOptions → `Some(true)` on the wire; `Some(default)`
    → field absent (backward-compat with pre-boxlite-ai#613 servers).
  - REST server: `build_box_options_health_check_disabled_true_clears_health_check` /
    `build_box_options_no_health_check_field_keeps_default_on` /
    `build_box_options_health_check_disabled_false_keeps_default_on`
    — three-state JSON → BoxOptions mapping, including the explicit
    "Some(false) means default" sentinel.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…p_force`

`rt_impl::remove_box(force=true)` historically reached for `kill_process(pid)`
+ inline `libc::waitpid` to clean up the shim — a PID-only shortcut that
worked, but ran a different kill path from `stop` / non-force `rm` /
`restart`, all of which already went through `ShimHandler` and held the
`Child` handle. Two patterns in the codebase for the same conceptual
operation; this commit collapses them into one.

Wiring, top-down:

  - `VmmHandler::stop_force` (trait): new sibling of `stop()`. Same
    return type, same `&mut self`, same sync inside-of-async pattern
    the existing `stop()` already uses.

  - `ShimHandler::stop_force` (impl on the trait): SIGKILL via
    `Child::kill()` followed by blocking `Child::wait()` when the
    handler owns a `Child` (the common case for boxes spawned by this
    runtime). Attached-mode fallback uses raw `libc::kill + waitpid`
    with the same 2 s deadline shape as the inline block previously
    held in `rt_impl::remove_box`.

  - `BoxBackend::stop_force` (trait): async counterpart on the higher-
    level box trait. Mirrors `stop()`.

  - `BoxImpl::stop_force` (impl on the trait): shares the entire
    teardown body with `BoxImpl::stop()` via a new `stop_impl(force:
    bool)` helper — health-check cancel, shutdown-token cancel, PID-
    file cleanup, state transition, DB sync, listener notification,
    metrics counter increment all stay identical between the two
    public methods. The only branches on `force`:
      * Skip the 10 s `guest_session.shutdown()` await (operator asked
        for `rm --force`, not a polite request).
      * Pick `handler.stop_force()` over `handler.stop()`.
      * Skip the trailing `auto_remove` self-call — the caller
        (`LocalRuntime::remove(force=true)`) is the remover; firing
        it again from inside `stop_force` would race.

  - `RestBox::stop_force` (impl on the trait): forwards to the existing
    graceful `stop` for now. The REST wire surface doesn't expose a
    force-stop verb today; clients reach the same outcome through
    `DELETE /v1/boxes/<id>?force=true`, which lands on
    `RuntimeBackend::remove(force=true)` server-side and follows the
    new 持-Child path there. Documented inline.

  - `LiteBox::stop_force` (public, in `litebox/mod.rs`): proxy over
    `box_backend.stop_force().await`. Public-facing API; what
    operators / SDK users would call directly.

  - `LocalRuntime::remove(id_or_name, force=true)` (the async backend
    impl in `rt_impl.rs`): looks up the LiteBox by id_or_name, calls
    `litebox.stop_force().await`, then delegates to the sync
    `RuntimeInnerImpl::remove` for DB / disk cleanup. After
    `stop_force()` returns, `mark_stop()` has already cleared
    `state.pid` to `None` AND `state.status` to `Stopped`, so the
    sync `remove_box`'s `if state.status.is_active() || state.pid
    .is_some()` guard short-circuits and the inline-waitpid block is
    not exercised on the async path. Production rm-force is now
    100 % 持-Child.

The inline `kill_process + libc::waitpid` block in `rt_impl::remove_box`
stays in place as a fallback for direct sync callers (Drop paths,
unit tests that exercise `RuntimeImpl::remove_box` without the async
surface). It's no longer the production code path; documenting it
inline as such would obscure the test-runner-only nature, so left
unannotated for the follow-up that strips it (if/when the 5 sync
tests are migrated to drive the async API). Without that migration,
killing the fallback would break the test suite for cosmetic gain.

Verification:

  - 3 reproducer tests (SIGKILL via health check + SIGTERM via health
    check + `boxlite serve` REST `rm --force`): all green, no
    regression on the zombie-reaping contract these tests pin.
  - 7 sync unit tests that exercise `RuntimeImpl::remove_box(force=
    true)` directly (`test_remove_box_*`, force_remove_*): all green
    — they hit the sync fallback path with state.pid still set.
  - 7 escape-hatch unit tests for `--no-health-check` / `health_check
    _disabled` plumbing: unchanged, all green.
  - `make fmt:check:rust` + workspace `cargo clippy` clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Seven findings, six fixes (two minor + four major), all motivated by
the same root issue: timed-out / non-child reap outcomes were being
treated as success, so the runtime moved on to destructive cleanup
while the shim could still be alive.

(1) `util/process.rs` reap helpers — `reap_pid_blocking` and
`reap_pid_async` returned `Reaped` unconditionally after `waitpid`,
even though `pidfd_open` succeeds for non-child PIDs and `waitpid`
then returns -1/ECHILD instead of reaping. Branch on the `waitpid`
return value and surface `NotOurChild` when it's not > 0.

(2) `vmm/controller/shim.rs:194` (`ShimHandler::stop_force` attached
mode) — discarded the reap outcome and returned `Ok(())`. Now
returns `Engine` error for `TimedOut` / `NotOurChild` so the
caller bails before clearing the PID / persisting `Stopped`.

(3) `runtime/rt_impl.rs:850` (force-remove sync fallback) — logged
the reap outcome and proceeded with home dir / disk deletion. Now
match-gated: only `Reaped` permits the destructive cleanup;
non-terminal outcomes return an `Engine` error pointing the
operator at the still-alive PID.

(4) `rest/litebox.rs:216` (REST `stop_force`) — silently downgraded
to graceful `stop()`. Returns `Unsupported` with a hint pointing
at `runtime.remove(.., force=true)` until REST gains a real
force-stop verb.

(5) REST `CreateBoxRequest` / `serve::types::CreateBoxRequest` /
`build_box_options` — previously only the disabled bit travelled on
the wire, so any custom `HealthCheckOptions` on the caller side
(non-default interval/timeout/retries/start_period) was silently
rewritten to defaults server-side. Added an optional
`health_check_settings: HealthCheckOptions` field; client emits it
only when the caller's settings differ from
`HealthCheckOptions::default()` (so the wire body is byte-identical
for pre-boxlite-ai#613 clients). Server honours it under
`AdvancedBoxOptions::default()` unless `health_check_disabled` is
also set.

(6) `tests/health_check_perf.rs:119` — wait-for-Healthy loop now
asserts the box actually reached `Healthy`. A timed-out wait used
to record CPU against startup / unhealthy behaviour and print it as
steady-state overhead.

(7) `tests/health_check_perf.rs:cpu_ms_from_ticks` — replaced the
`ticks * 10` hardcode (= 100 Hz assumption) with
`sysconf(_SC_CLK_TCK)` lookup. Falls back to 100 only if sysconf
returns something nonsensical.

## Tests added

- `test_create_box_request_carries_custom_health_check_settings_on_the_wire`
  — client puts the field on the wire when the caller's settings
  differ from default. Reverting either the new struct field or the
  `from_options` match arm flips it red.
- `test_create_box_request_omits_health_check_settings_when_default`
  — wire body stays byte-identical for default settings (back-compat).
- `build_box_options_honours_custom_health_check_settings` — server
  reads the field and produces the matching `HealthCheckOptions`.
- `build_box_options_disabled_wins_over_settings` — opt-out beats
  custom settings when both are present.
- The existing 2 client + 3 server health_check tests still pass.

`HealthCheckOptions` now derives `PartialEq + Eq` so the
`from_options` "are these the default?" check can do a simple
equality comparison.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Adds a single tokio task in `RuntimeImpl::new` that subscribes to
`SIGCHLD` and drains every available zombie via
`waitpid(-1, ..., WNOHANG)`. Health-check still calls `reap_pid_async`
per-pid for the boxes it knows about; this catches the cases health-
check never sees — operator disabled the watcher, the shim died
during init before the watcher spawned, or a dropped `Child` handle
left an unwatched pid.

Disabled under `cfg(test)` because cargo runs all lib unit tests in
one process and `waitpid(-1)` is process-wide — a reaper spawned by
a `#[tokio::test]` would race-steal `Child`ren that adjacent
`#[test]` cases are about to `try_wait()`. Production binaries and
integration-test binaries (one process per `tests/*.rs`) keep the
auto-spawn. The reaper's own contract is covered by the
`runtime_child_reaper` integration test, which lives in its own
binary precisely to avoid the same hazard.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>