fix(controller): run Registry::reconcile under a single lock

[AmirF194]

Summary

Registry::reconcile() took K + 4 separate registry-mutex acquisitions to prune K stale sandboxes: one lock per removed id, plus one each for the initial stale scan, the live-id snapshot, and the workspace walk. As you noted in the issue, nothing stops an HTTP handler from inserting or removing a sandbox between the scan and the per-id removal, which can make the reported prune count diverge from what was actually removed.

This runs the whole in-memory pass (stale scan, removal, live-id snapshot, workspace walk) under a single self.inner.lock() acquisition, matching the shape from the issue. pid_alive is a /proc stat and takes no lock the registry cares about, so holding the mutex across it is fine. flush() behavior is unchanged: still called once, after the lock is dropped, only when pruned > 0 || stale_ws_changed.

Why not a one-liner

The function reads sandboxes then mutates workspaces from the same MutexGuard; the live-id set has to stay an owned HashSet<String> (not borrowed) so the borrow checker allows the later mutable walk over g.workspaces through the same guard.

Verification

• New test reconcile_prunes_dead_sandbox_and_marks_its_workspace_stale: a sandbox with a dead pid is pruned and its Running workspace flips to Stale with live_sandbox_id cleared; a sandbox with no recorded pid (pid: None) is left alone and its workspace stays Running; a Suspended workspace is never touched even though its sandbox is gone; reloading from disk confirms flush() ran.
• New test reconcile_no_op_does_not_flush: a reconcile pass that prunes nothing leaves state.json byte-identical.
• All four rust CI steps run clean in a rust:1-bookworm container against this branch (94 passed, 0 failed, in the forkd-controller unit crate):

  cargo fmt --all -- --check
  cargo clippy --all-targets --all-features -- -D warnings
  cargo build --all
  cargo test --all
  

• Not verified: the actual interleaving/lost-accounting scenario from the issue. As you said, reconcile() only runs at startup today before the HTTP listener accepts requests, so it is not reachable through any current call path; the two new tests cover the prune-count and workspace-transition behavior the single-lock refactor must preserve, not a live race.

Note on #299

#299 is open on the same function (extracts the workspace-marking half into mark_stale_workspaces()); it does not touch the stale-id removal loop this issue is about, so it is not a competing fix, but merging either PR first will need a small rebase of the other.

Fixes #310

[WaylandYang]

The single-lock refactor matches the accepted shape from #310: stale discovery/removal, the live-id snapshot, and Running→Stale workspace transitions now occur under one registry guard, while persistence remains outside the lock and conditional on an actual change. The focused tests cover prune accounting, workspace transitions, suspended/no-PID preservation, and reload persistence. DCO and all required CI checks are green.

Approved. Non-blocking: the byte-equality no-op test confirms stable output but cannot by itself prove no rewrite occurred; the explicit if pruned > 0 || stale_ws_changed control flow is clear enough here.

[AmirF194]

Thanks for the quick review and merge. Glad the single-lock shape matched what you had in mind for #310.

[WaylandYang]

Thanks for the contribution, Amir — welcome aboard!