fix(daemon): sweep a dead daemon's leavings on the writer's tick, not at startup

Review follow-ups on this branch.

`history::sweep` still ran at startup, three lines under a new comment
explaining why sweeping there is wrong. The reasoning transfers exactly, and
worse than by analogy: a restore carries the dead pane's commands to its
successor via `history::carry`, so sweeping before the window can ask deletes
the file the request is about. Same shape as the scrollback bug, one file over.
Both sweeps now run on the writer's tick off one shared id set, and the writer
is named for what it does.

`pane_attachable` lost its only caller when the restore path moved to
`pane_free_for`, leaving a function kept alive by the test asserting on it. The
attach site does not need to predict the listing: it tries the attach, and a
pane that is gone falls through to the fresh spawn on its own. Gone, with its
tests folded into `pane_free_for`'s.

`restored_screen` now drops the snapshot in both directions. Keeping the file
when it decoded to nothing left it to be re-read and re-rejected by every later
restore, and swept never, for a pane the tree still names.

Also: the module doc still said scrollback was off unless asked for, which is
what this branch reverses; and #449 landed the whole feature with no CHANGELOG
entry, so nothing told anyone that pane output now lives on disk.

CHANGELOG.md

@@ -48,6 +48,21 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   shows through the whole workspace, and the settings panel stays opaque.
   macOS and Linux keep the existing blur toggle.
 
+- **Panes come back showing what was on them after the background service dies
+  unexpectedly** — a crash, a `kill -9` or a reboot takes the shells with it
+  either way, but the screens no longer go with them. A capped tail of each
+  pane's output is kept at `<config>/scrollback/*.bin` (0600 on unix, behind
+  the config directory's ACL on Windows; 256 KiB per pane, written at most
+  every 30s and only for panes whose output moved), and a pane that reopens on
+  a dead predecessor's id is handed it. A planned restart already carried the
+  live ptys across untouched; this covers the deaths nothing gets to prepare
+  for. There is no switch: the moment anyone learns they wanted this is the
+  moment a service has already died, so it is on for everyone. The bytes are
+  dropped as soon as nothing can ask for them — closing a pane deletes its
+  file at once, a restore consumes it, and a periodic pass collects the rest.
+  A pane's shell is recorded alongside, so a git bash pane no longer comes
+  back as PowerShell.
+
 ### Fixed
 
 - **An SFTP upload no longer sits in the browser under its temporary name** —

crates/tty7-core/src/daemon/scrollback.rs

@@ -22,10 +22,15 @@
 //!   screen or two, not the ring's whole 8 MiB. The value of scrollback decays
 //!   steeply with distance from the bottom, and every byte here is a byte of
 //!   someone's terminal sitting on disk.
-//! - **It is off unless asked for.** These bytes include whatever was echoed
-//!   into the pane: tokens, `env` output, an agent's transcript. In memory
-//!   they die with the daemon. On disk they outlive it, which is the whole
-//!   point and also the whole risk, so the choice is the user's.
+//! - **It is not asked for.** These bytes include whatever was echoed into the
+//!   pane: tokens, `env` output, an agent's transcript. In memory they die
+//!   with the daemon; on disk they outlive it, which is the whole point and
+//!   also the whole risk. It was a setting once, defaulting to off. That was
+//!   wrong about *when* the choice gets made: the moment anyone learns they
+//!   wanted this is the moment a daemon has already died, and by then the
+//!   switch could only be flipped for next time. A feature that exists to
+//!   survive an unscheduled event cannot be opt-in. So the cost is paid for
+//!   everyone, and the two bullets below are what keep it small.
 //! - **It is dropped as soon as it is meaningless.** A pane that is closed, or
 //!   that no workspace refers to any more, has its file removed. Retention is
 //!   by relevance, not by calendar: a snapshot of a pane nobody will reopen is

crates/tty7-core/src/daemon/server.rs

@@ -178,12 +178,19 @@ fn restorable_pane_ids(registry: &Registry) -> std::collections::HashSet<u64> {
     ids
 }
 
-/// Keep each pane's stored screen roughly current.
+/// Keep each pane's stored screen roughly current, and collect what no pane
+/// can be asked about any more.
 ///
 /// Only panes whose ring has moved are written, so an idle machine does no IO
 /// at all, and the busy pane that most needs a fresh copy is the one that gets
 /// it.
-fn spawn_scrollback_writer(registry: Arc<Registry>) {
+///
+/// The two sweeps ride along here rather than at startup because this is where
+/// the question they ask can be answered: by now the registry holds this
+/// daemon's panes and the windows have had time to put their trees back. Both
+/// take the same set, because a pane whose screen is still worth restoring is
+/// exactly a pane whose commands are still worth carrying.
+fn spawn_snapshot_keeper(registry: Arc<Registry>) {
     let spawned = std::thread::Builder::new()
         .name("tty7-scrollback".into())
         .spawn(move || {
@@ -198,7 +205,9 @@ fn spawn_scrollback_writer(registry: Arc<Registry>) {
                     crate::daemon::scrollback::save(pane.id, &segments);
                     marks.insert(pane.id, mark);
                 }
-                crate::daemon::scrollback::sweep(&restorable_pane_ids(&registry));
+                let restorable = restorable_pane_ids(&registry);
+                crate::daemon::scrollback::sweep(&restorable);
+                crate::daemon::history::sweep(&restorable);
                 marks.retain(|id, _| registry.get(*id).is_some());
             }
         });
@@ -230,13 +239,15 @@ fn restored_screen(
     request: crate::daemon::protocol::RestoreFrom,
 ) -> Option<crate::daemon::pane::Restore> {
     let segments = crate::daemon::scrollback::load(request.pane_id)?;
+    // Dropped either way — this is the one request that will ever be made about
+    // this pane, so nothing is served by keeping the file past it. What the
+    // emptiness check decides is whether a *restore* happened, not whether the
+    // file stays: a snapshot holding nothing is not a screen to hand over, but
+    // it is still a file nobody will read again.
+    crate::daemon::scrollback::forget(request.pane_id);
     if segments.is_empty() {
         return None;
     }
-    // After the emptiness check, not before it: dropping the file is how a
-    // screen that *was* handed out stops being handed out twice, and a
-    // snapshot that turned out to hold nothing was never handed out at all.
-    crate::daemon::scrollback::forget(request.pane_id);
     log::info!(
         "pane {} is gone; its last screen is restored into a fresh pane",
         request.pane_id
@@ -549,25 +560,26 @@ fn run_with(registry: Arc<Registry>) -> anyhow::Result<()> {
     }
 
     spawn_orphan_sweep(registry.clone());
-    let restorable = restorable_pane_ids(&registry);
-    // No scrollback sweep here, deliberately. Startup is the one moment this
-    // process knows least: it owns no panes yet, and the windows that know
-    // which screens are still wanted cannot say so until the endpoint below is
-    // listening. Answering "is anyone going to ask for this?" here answers it
-    // when nobody can — and the answer deletes. A tree that failed to parse
-    // makes it worse, because `read_machine` quarantines it and hands back an
-    // empty `Machine`, so one bad file would take every pane's screen with it.
+    // No sweeping here, deliberately — of either kind. Startup is the one
+    // moment this process knows least: it owns no panes yet, and the windows
+    // that know which of a dead daemon's files are still wanted cannot say so
+    // until the endpoint below is listening. Answering "is anyone going to ask
+    // for this?" here answers it when nobody can — and the answer deletes. A
+    // tree that failed to parse makes it worse, because `read_machine`
+    // quarantines it and hands back an empty `Machine`, so one bad file would
+    // take every pane's screen and every pane's history with it.
     //
-    // The periodic sweep asks the same question a tick later, with the registry
-    // filled in and the tree caught up, and that is soon enough: nothing here
-    // is serving a request in the meantime.
+    // History was swept here until it was noticed that a restore carries the
+    // dead pane's commands to its successor (`history::carry`, at the top of
+    // the `Spawn` handler): sweeping before the window can ask deletes the very
+    // file the request is about. Same shape as the scrollback bug, one file
+    // over.
     //
-    // A daemon that was killed outright never retired anything, so the files of
-    // panes that died with it are still here. Their commands cannot be
-    // recovered — the mark saying which were new belongs to a shell that is
-    // gone — so what is left is not to hoard them.
-    crate::daemon::history::sweep(&restorable);
-    spawn_scrollback_writer(registry.clone());
+    // Both sweeps run on the writer's tick instead, with the registry filled in
+    // and the tree caught up. That is soon enough: nothing here is serving a
+    // request in the meantime, and a daemon killed outright left its files for
+    // exactly this pass to collect.
+    spawn_snapshot_keeper(registry.clone());
 
     for stream in listener.incoming() {
         match stream {

crates/tty7-server/tests/scrollback_restore.rs

@@ -332,9 +332,11 @@ fn the_tree_records_the_shell_a_pane_is_running() {
 
     // A pane reaches the tree by being put in a tab, which is what the window
     // does right after it spawns one — carrying the same seed it spawned with.
-    // Until then the daemon has nothing to record its facts against, so the
-    // seed is the pane's first and only chance to say what it is running.
-    let mut control = ControlClient::connect_at(
+    // Until then the daemon has nothing to record its facts against, so this is
+    // the pane's first chance to say what it is running, and for a pane that
+    // then sits at a prompt it is the only one: the daemon's own observation
+    // rides on a fact *changing*, and a pane's shell never does.
+    let control = ControlClient::connect_at(
         &instance.control_endpoint(),
         &ControlHello::host_rpc("probe", "probe"),
     )

src/ui/app.rs

@@ -6633,16 +6633,6 @@ pub(crate) fn alive_panes_on(
     }
 }
 
-fn pane_attachable(
-    alive: Option<&std::collections::HashMap<u64, Option<String>>>,
-    id: u64,
-    owner: crate::core::session::WorkspaceId,
-) -> bool {
-    // Listed at all, and then whose it is. A pane missing from the listing is
-    // one there is nothing to attach to.
-    alive.is_none_or(|listed| listed.contains_key(&id)) && pane_free_for(alive, id, owner)
-}
-
 /// Whether this window may stand on `id` — as the pane it attaches to, or as
 /// the dead predecessor whose screen a fresh pane opens showing.
 ///
@@ -6652,8 +6642,13 @@ fn pane_attachable(
 /// Ruling the id out there threw away the only thing that could ask: the
 /// window spawned a pane that had never heard of a predecessor, so no attach
 /// was tried, no restore was requested, and the screen the daemon still had on
-/// disk was swept a tick later, unread. Attaching is still tried first and
-/// still fails harmlessly when the pane really is gone.
+/// disk was swept a tick later, unread.
+///
+/// There used to be a second predicate here that also required the id to be
+/// listed, and the attach site consulted it. Nothing does now: the attach is
+/// simply tried, and a pane that really is gone fails it and falls through to
+/// the fresh spawn — the same outcome the listing was consulted to predict,
+/// reached by asking the daemon instead of guessing ahead of it.
 ///
 /// Ownership does come into it. Another workspace's pane is not this window's
 /// to attach to, and its screen is not this window's to show.
@@ -7389,8 +7384,8 @@ mod window_drag_tests {
 mod tests {
     use super::{
         CloseReason, TabAgentSession, clear_window_override_values, close_prompt,
-        leaf_shares_the_window_daemon, mru_order, pane_attachable, pane_free_for,
-        parse_ssh_connect_input, parse_ssh_option_words,
+        leaf_shares_the_window_daemon, mru_order, pane_free_for, parse_ssh_connect_input,
+        parse_ssh_option_words,
     };
 
     #[test]
@@ -7502,28 +7497,24 @@ mod tests {
         .collect();
 
         assert!(
-            pane_attachable(Some(&alive), 1, ours),
+            pane_free_for(Some(&alive), 1, ours),
             "our own pane attaches"
         );
         assert!(
-            !pane_attachable(Some(&alive), 2, ours),
+            !pane_free_for(Some(&alive), 2, ours),
             "another workspace's pane must spawn fresh instead"
         );
         assert!(
-            pane_attachable(Some(&alive), 3, ours),
+            pane_free_for(Some(&alive), 3, ours),
             "an unowned pane is legacy"
         );
         assert!(
-            pane_attachable(Some(&alive), 5, ours),
+            pane_free_for(Some(&alive), 5, ours),
             "an owner that names no workspace is not a rival's claim: older CLIs \
              wrote their own name there, and respawning strands the live pane"
         );
         assert!(
-            !pane_attachable(Some(&alive), 4, ours),
-            "a dead id never attaches"
-        );
-        assert!(
-            pane_attachable(None, 4, ours),
+            pane_free_for(None, 4, ours),
             "a failed List says nothing about pane 4; the attach itself must decide, \
              because respawning on a transient RPC error destroys a live session"
         );
@@ -7546,10 +7537,6 @@ mod tests {
             pane_free_for(Some(&alive), 4, ours),
             "a dead pane's id has to survive; the restore is keyed on it"
         );
-        assert!(
-            !pane_attachable(Some(&alive), 4, ours),
-            "attaching to it is still hopeless, and that stays true"
-        );
 
         // What being free does not mean: helping yourself to a pane that is
         // alive and belongs to another workspace, whose screen is not this