refactor(daemon): give a pane's takeover a seam a test can reach

Deleting `history::carry` from the pane-create path left the suite green,
and what it costs is what the function's own comment names: the commands
the predecessor ran, which are the ones a reader reaches for first after a
restore. `carry` and `restored_screen` are each held by their own tests;
nothing held that a pane replacing a dead one calls either.

Rather than another source-reading guard, the two statements move into
`take_over_from`. They belong together anyway -- they are one decision,
that this pane continues that one, and a caller doing half of it hands
back the scrollback with an empty Up key, or the reverse. Both have to run
before the spawn, because the spawn is what hands the shell the name of
its history file.

`handle_conn` cannot be entered by a test; this can, and the test holds
both halves plus the no-predecessor case. Each half was deleted in turn to
check it fails.

Two neighbours came out of the same sweep needing nothing: the
stale-endpoint removal on startup is held by two tests, and the one in
`on_shutdown` is belt and braces for it -- a socket left by an unclean
exit is what the guarded one already handles.
This commit is contained in:
l0ng-ai
2026-08-16 14:18:07 +08:00
parent 16732d10af
commit f7ab6dacec
+79 -7
View File
@@ -258,6 +258,27 @@ fn restored_screen(
})
}
/// Everything a new pane inherits from the one it replaces, before it spawns.
///
/// Both halves have to happen here rather than at the spawn: the spawn is what
/// hands the shell the name of its history file, so the carry has to have put
/// the predecessor's commands behind that name already, and the screen has to
/// be in hand to seed the ring with.
///
/// They are one function because they are one decision — this pane is the
/// continuation of that one — and because a caller that did half of it would
/// give the reader back their scrollback with an empty Up key, or the reverse.
/// `handle_conn` cannot be entered by a test, so this is the seam that can.
fn take_over_from(
dead: Option<crate::daemon::protocol::RestoreFrom>,
id: u64,
) -> Option<crate::daemon::pane::Restore> {
if let Some(pane_id) = dead.as_ref().map(|r| r.pane_id) {
crate::daemon::history::carry(pane_id, id);
}
dead.and_then(restored_screen)
}
/// Close a pane for good: stop it, drop it from the registry, and drop the copy
/// of its screen.
///
@@ -700,13 +721,7 @@ fn handle_conn(stream: Stream, registry: Arc<Registry>) -> anyhow::Result<()> {
restore,
} => {
let id = registry.alloc_id();
if let Some(dead) = restore.as_ref().map(|r| r.pane_id) {
// Before the spawn, because the spawn is what hands the shell
// the name of its history file — and this is what puts the
// predecessor's commands behind that name.
crate::daemon::history::carry(dead, id);
}
let restore = restore.and_then(restored_screen);
let restore = take_over_from(restore, id);
let on_dead = {
let registry = registry.clone();
move || {
@@ -1257,6 +1272,63 @@ fn spawn_writer(
#[cfg(test)]
mod tests {
/// A replacement pane inherits both halves, or neither is worth having.
///
/// `history::carry` and `restored_screen` are each held by their own
/// tests; nothing held that a pane taking over from a dead one calls them.
/// Deleting the carry left the suite green, and what it costs is the thing
/// its own comment names — the commands the predecessor ran, which are the
/// ones a reader reaches for first after a restore.
#[test]
fn a_pane_taking_over_inherits_the_history_and_the_screen() {
let dir = std::env::temp_dir().join(format!("tty7-takeover-{}", std::process::id()));
std::fs::create_dir_all(&dir).ok();
crate::core::config::set_config_dir(dir);
let (dead, fresh) = (93_001, 93_002);
crate::daemon::scrollback::save(
dead,
&[crate::daemon::scrollback::Segment {
size: crate::daemon::protocol::WinSize {
cols: 80,
rows: 24,
cell_w: 8,
cell_h: 16,
},
bytes: b"what the dead pane had on it".to_vec(),
}],
);
let history = crate::daemon::history::path_for(dead).expect("a history path");
std::fs::create_dir_all(history.parent().expect("a parent")).ok();
std::fs::write(
&history,
"cargo test
",
)
.expect("seed the predecessor's history");
let restore = take_over_from(
Some(crate::daemon::protocol::RestoreFrom {
pane_id: dead,
banner: None,
}),
fresh,
)
.expect("the screen comes across");
assert_eq!(restore.segments.len(), 1, "the screen came across");
let carried = crate::daemon::history::path_for(fresh).expect("a history path");
assert_eq!(
std::fs::read_to_string(&carried).unwrap_or_default(),
"cargo test\n",
"the replacement pane's Up key cannot reach what its predecessor ran"
);
assert!(!history.exists(), "the old name was left behind as a copy");
// No predecessor: nothing to inherit, and nothing to go wrong.
assert!(take_over_from(None, 93_003).is_none());
}
/// A restore consumes the snapshot it was handed, either way.
///
/// The privacy page says a restore consumes the file, and