Never let a reconcile empty a live sandbox slice

docs/recovering-lost-sandbox-sessions.md

@@ -0,0 +1,97 @@
+# Recovering sandbox sessions that vanished from the TUI
+
+Live sessions disappear from the dashboard while their processes keep running.
+This has happened repeatedly. Every recovery so far has been improvised. This is
+the procedure.
+
+**Nothing is ever lost when this happens.** The processes are alive, the
+transcripts are intact, and the terminals still work. What is lost is one file's
+contents: this sandbox's slice of `sandbox-sessions.json`.
+
+## Why it happens
+
+The registry slice for a sandbox is written by `record_hook_event`, which
+reconciles **only when a hook fires inside that sandbox**. Three properties
+combine badly:
+
+1. **A reconcile is wholesale.** It replaces the sandbox's entire slice with
+   whatever the live scan returned.
+2. **A live scan can return empty for reasons that are not "no sessions"** — a
+   failed process-table read, pointers already gone, a hook firing during
+   teardown. Before the `EmptySlice::Freeze` guard, that emptied the slice.
+3. **Idle sessions fire no hooks.** Once a slice is empty, nothing rewrites it
+   until someone interacts with a session in that sandbox. Sessions idle for
+   days stay invisible for days.
+
+The pid cannot be recovered from outside the sandbox. It exists only as the
+filename of `~/.claude/sessions/<pid>.json`, inside the user+mount namespace
+each `sc` session creates for itself (`sandbox-bootstrap`, `unshare --user
+--mount`). Each session gets its *own* namespace with exactly one pointer in it,
+so even `sbx exec` into the right sandbox reaches at most one of them.
+
+**A missing pid is fine.** `from_registry_entry` maps it to `0`, and
+`collector_says_gone` explicitly treats an absent pid as "no evidence" — which is
+what keeps the row on screen. Tab-to-session keys on `host_terminal_id` and
+`host_tty`, not the pid. Do not invent a pid to make the column look right: a
+fabricated pid makes `collector_says_gone` trust the collector's zero reading and
+filters every row straight back off.
+
+## Diagnose (30 seconds, host)
+
+```bash
+# 1. What does the registry think?
+python3 -c "import json;d=json.load(open('$HOME/.local/share/claudectl/sandbox-sessions.json'));[print(k,len(v)) for k,v in d['sandboxes'].items()]"
+
+# 2. What is actually running? This is the ground truth.
+ps -eo pid,ppid,lstart,args | grep -E '[c]laude|[s]c '
+
+# 3. Are the sandboxes up?
+sbx ls
+```
+
+If (2) shows more sessions than (1), the registry lost them. Proceed.
+
+## Recover (host)
+
+Every field needed to rebuild an entry is in the `sbx exec` argv that (2)
+printed. For each session:
+
+| Registry field     | Where it comes from                       |
+| ------------------ | ----------------------------------------- |
+| `session_id`       | `--resume <id>`                           |
+| sandbox key        | `-e SANDBOX_NAME=linera-agent-…`          |
+| `host_tty`         | `-e SANDBOX_HOST_TTY=/dev/ttysNNN`        |
+| `host_terminal_id` | `-e SANDBOX_HOST_TERMINAL_ID=…`           |
+| `transcript`       | `~/.claude/projects/*/<session_id>.jsonl` |
+| `cwd`              | `-w /Users/<you>`                         |
+| `pid`              | leave `null` — see above                  |
+
+A session launched with an initial prompt rather than `--resume` has no id in
+its argv. Those are new sessions, so their `SessionStart` hook has already
+registered them; they are not the ones that go missing.
+
+Back up first, then write only the sandboxes you are repairing — other slices
+must be left alone:
+
+```bash
+cp ~/.local/share/claudectl/sandbox-sessions.json{,.bak-$(date +%F-%H%M)}
+```
+
+## Rules that make recovery safe
+
+These are the mistakes that turned previous recoveries into long ones.
+
+- **Never delete an entry because a probe found nothing.** "I could not see it"
+  is not "it is not running". Repair scripts must be additive-only.
+- **Never key entries to a sandbox name `sbx` does not report.** `foreign_sessions`
+  filters every row through `running_sandbox_filter`, so an invented key renders
+  nothing at all.
+- **Never use `ps` from inside a sandbox to judge another sandbox.** Sandboxes
+  are separate VMs; that `ps` cannot see them, and its silence means nothing.
+- **Ask for host `ps` output first.** It is the only complete view, and it
+  carries sandbox, session id, tty and terminal id in one line.
+
+## Verify
+
+Rows appear in the TUI, and tab-to-session reaches the right window. The pid
+column showing `0` for foreign rows is expected, not a failure.

src/sandbox_registry.rs

@@ -704,15 +704,36 @@ where
     })
 }
 
+/// What an empty `entries` means to [`replace_sandbox_slice_with`].
+#[derive(Clone, Copy, Debug, Eq, PartialEq)]
+pub enum EmptySlice {
+    /// Keep the previous slice. For reconciles, where empty is a *measurement*
+    /// and a wrong one costs sessions that cannot be recovered from the file.
+    Freeze,
+    /// Drop the key. Only for a deliberate close, where empty is a *decision*.
+    Remove,
+}
+
 /// Reconcile one sandbox's slice (keyed by `sandbox` name) of
 /// `sandbox-sessions.json` to its current live set.
 ///
 /// Unlike the local file ([`update_local`]) this stays a plain mirror: `sbx rm`
 /// is abrupt and fires no further hooks, so the slice freezes at its last live
 /// state on its own — exactly what `--restore-sbx-sessions` restores — and
 /// container pids say nothing about which host terminal owned anything.
-/// Empty `entries` removes the key; other sandboxes' slices are untouched.
-pub fn replace_sandbox_slice(sandbox: &str, mut entries: Vec<SessionEntry>) -> io::Result<()> {
+/// Other sandboxes' slices are untouched, and an empty `entries` freezes this
+/// one rather than deleting it: see [`EmptySlice`] for why a reconcile is never
+/// allowed to make that call.
+pub fn replace_sandbox_slice(sandbox: &str, entries: Vec<SessionEntry>) -> io::Result<()> {
+    replace_sandbox_slice_with(sandbox, entries, EmptySlice::Freeze)
+}
+
+/// [`replace_sandbox_slice`] with an explicit choice of what empty means.
+pub fn replace_sandbox_slice_with(
+    sandbox: &str,
+    mut entries: Vec<SessionEntry>,
+    on_empty: EmptySlice,
+) -> io::Result<()> {
     let path = sandbox_registry_path();
     with_lock(&path, || {
         // Never `load()` here: this write serialises the *whole* file, so a
@@ -725,6 +746,12 @@ pub fn replace_sandbox_slice(sandbox: &str, mut entries: Vec<SessionEntry>) -> i
         // sandbox at once. Backfill before the unchanged-comparison so a
         // no-op-after-backfill write is still skipped.
         if let Some(existing) = registry.sandboxes.get(sandbox) {
+            // The backfill below repairs a degraded reading's session *titles*;
+            // this repairs a fully empty one's *existence*, which the backfill
+            // cannot reach because there is no entry left to backfill into.
+            if entries.is_empty() && on_empty == EmptySlice::Freeze && !existing.is_empty() {
+                return Ok(());
+            }
             backfill_missing_identity(existing, &mut entries);
         }
         let unchanged = match (registry.sandboxes.get(sandbox), entries.is_empty()) {
@@ -766,7 +793,7 @@ pub fn forget_session(session_id: &str) -> io::Result<()> {
                 .into_iter()
                 .filter(|entry| entry.session_id != session_id)
                 .collect();
-            replace_sandbox_slice(&sandbox, remaining)
+            replace_sandbox_slice_with(&sandbox, remaining, EmptySlice::Remove)
         }
         None => update_local(|current| {
             current
@@ -1447,10 +1474,25 @@ pub(crate) mod tests {
     }
 
     #[test]
-    fn replace_sandbox_slice_empty_removes_the_sandbox() {
-        let _guard = TempRegistry::new("empty");
+    fn a_reconcile_that_measures_zero_never_empties_a_live_slice() {
+        let _guard = TempRegistry::new("freeze-on-empty");
         replace_sandbox_slice("linera-agent", vec![entry("aaa", "/a")]).unwrap();
+        // A live scan can come back empty for reasons that are not "every
+        // session ended": the process table read failed, the pointer files are
+        // gone, the hook fired during teardown. Losing the slice to any of
+        // those is unrecoverable, so a measured zero must freeze, not delete.
         replace_sandbox_slice("linera-agent", vec![]).unwrap();
+        assert_eq!(
+            ids(load().sandboxes.get("linera-agent").expect("slice kept")),
+            ["aaa"]
+        );
+    }
+
+    #[test]
+    fn a_deliberate_close_of_the_last_session_still_removes_the_sandbox() {
+        let _guard = TempRegistry::new("empty");
+        replace_sandbox_slice("linera-agent", vec![entry("aaa", "/a")]).unwrap();
+        replace_sandbox_slice_with("linera-agent", vec![], EmptySlice::Remove).unwrap();
         assert!(!load().sandboxes.contains_key("linera-agent"));
     }
 
@@ -1459,7 +1501,7 @@ pub(crate) mod tests {
         let _guard = TempRegistry::new("independent");
         replace_sandbox_slice("linera-agent", vec![entry("aaa", "/a")]).unwrap();
         replace_sandbox_slice("pm-task", vec![entry("bbb", "/b")]).unwrap();
-        replace_sandbox_slice("linera-agent", vec![]).unwrap();
+        replace_sandbox_slice_with("linera-agent", vec![], EmptySlice::Remove).unwrap();
         let registry = load();
         assert!(!registry.sandboxes.contains_key("linera-agent"));
         assert_eq!(registry.sandboxes.get("pm-task").unwrap().len(), 1);