starred/tty7
1
0
Fork 0
mirror of https://github.com/l0ng-ai/tty7.git synced 2026-09-22 00:02:23 +00:00
Code Issues Packages Projects Releases Wiki Activity

refactor(core): retire the client-side pane-identity defenses

Browse Source 
The machine tree made this whole family unnecessary, so it goes rather
than lingers: daemon_instance stamps (a restarted daemon's tree says
live=false about every pane — a fact, where the stamp was a heuristic),
forget_stale_pane_ids on both layers, dedupe_pane_ids (the daemon
refuses a pane appearing twice in its tree, so there is no duplicate to
mop up client-side), the claim/record instance plumbing, and the
whole-record halves of the storage split (to_remote_json,
apply_remote_json, REMOTE_OWNED_FIELDS, CLIENT_OWNED_FIELDS, and the
store's apply_remote / remote_payload), together with their tests.

forget_pane_ids stays for now: it clears the client's cached copy, which
still serves as the one-time import fallback until the view file slims
down to pure view state.
This commit is contained in:
l0ng-ai
2026-07-29 22:59:02 +08:00
parent 6feba961e0
commit 6bbf533d7e
4 changed files with 33 additions and 939 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/tty7-core/src/core/session.rs
-589
View File
@@ -408,32 +408,6 @@ pub struct Workspace {
/// workspace from the laptop at home.
#[serde(default, skip_serializing_if = "Option::is_none")]
pub host: Option<RemoteRef>,
/// Identity of the daemon *process* the pane ids in `session` refer to
/// (see `daemon::protocol::DaemonVersion::instance`). One field for the
/// whole workspace, not one per leaf, because a workspace's panes all live
/// in one daemon (one window, one machine).
///
/// This is what makes a saved pane id safe to trust: daemon ids restart
/// from 1, so after a reboot every saved id points at whatever unrelated
/// shell happens to hold the number now — and restore's aliveness check
/// cannot tell a survivor from a squatter. A claim whose instance differs
/// from the daemon now serving blanks its ids instead
/// ([`Workspace::forget_stale_pane_ids`]) and takes the fresh-spawn path,
/// agent resume included, which is the correct reading of "the daemon
/// those panes lived in is gone".
///
/// A remote workspace records its machine's `tty7-server` instance here,
/// for exactly the same reason and read by exactly the same check. The live
/// per-connection tracking on the client (`note_instance`) does not replace
/// this: that map is in memory, so it is empty on the launch where it would
/// matter most — the one after a client restart that spanned a server
/// replacement.
///
/// `None` for records written before the field, and whenever the serving
/// process cannot be named (an older peer, a machine not connected). `None`
/// disables the check, never fails it.
#[serde(default, skip_serializing_if = "Option::is_none")]
pub daemon_instance: Option<String>,
}
impl Default for Workspace {
@@ -446,7 +420,6 @@ impl Default for Workspace {
open: true,
last_active: now_secs(),
host: None,
daemon_instance: None,
}
}
}
@@ -552,29 +525,6 @@ impl Workspace {
forgotten
}
/// Blank every saved pane id if it was recorded against a *different*
/// daemon process than `current` — see [`Workspace::daemon_instance`] for
/// the id-reuse failure this closes. Answers how many ids were dropped.
///
/// Only a **known, differing** instance pair trips it. `None` on either
/// side means "cannot tell" (an old record, an old daemon), and treating
/// that as stale would respawn every pane on the first launch after an
/// upgrade — exactly the sessions persistence exists to keep.
///
/// The agent fields stay, deliberately: unlike a *duplicate* claim (see
/// `drop_duplicate_pane_ids`), a stale-instance claim means the pane is
/// genuinely gone with its daemon, nothing else is running the
/// conversation, and the fresh shell resuming it is the feature.
pub fn forget_stale_pane_ids(&mut self, current: Option<&str>) -> usize {
let (Some(recorded), Some(current)) = (self.daemon_instance.as_deref(), current) else {
return 0;
};
if recorded == current {
return 0;
}
self.forget_pane_ids()
}
/// Stamp this workspace as just-focused.
pub fn touch(&mut self) {
self.last_active = now_secs();
@@ -614,68 +564,6 @@ impl Workspace {
None => crate::host::HostId::LOCAL,
}
}
/// The record the **remote** owns, as the JSON that crosses the wire in a
/// [`WorkspacePut`](crate::daemon::control::ControlRequest::WorkspacePut).
///
/// The storage split, executable rather than aspirational: what
/// stays here is `window`, `open` and `host` — this client's view state —
/// and what goes over there is everything that is a fact about the machine.
/// [`REMOTE_OWNED_FIELDS`] pins the split, and a test fails if a new field
/// is added without a decision about which side it belongs to.
pub fn to_remote_json(&self) -> serde_json::Value {
let mut value = serde_json::to_value(self).unwrap_or(serde_json::Value::Null);
if let Some(obj) = value.as_object_mut() {
obj.retain(|k, _| REMOTE_OWNED_FIELDS.contains(&k.as_str()));
}
value
}
/// Merge an authoritative record pulled from a remote store into this entry.
///
/// Touches only the remote-owned fields. `id`, `host`, `window` and `open`
/// are left exactly as they were — the first two because the client's entry
/// is the thing being *pointed* by them, the last two because they are this
/// machine's view state and the remote has no opinion about them.
pub fn apply_remote_json(&mut self, value: &serde_json::Value) -> serde_json::Result<()> {
let record: RemoteRecord = serde_json::from_value(value.clone())?;
self.name = record.name;
self.session = record.session;
self.last_active = record.last_active;
Ok(())
}
}
/// The `Workspace` fields the **remote** is the authority for.
/// Everything else is client-side view state and never leaves this machine.
///
/// A `Workspace` field that is in neither list is a bug: it would be dropped by
/// [`Workspace::to_remote_json`] and silently lost on the next pull. The test
/// `the_storage_split_covers_every_workspace_field` is what makes that a red
/// build rather than a data-loss report.
pub const REMOTE_OWNED_FIELDS: &[&str] = &["id", "name", "session", "last_active"];
/// The client-side view state, which stays in this machine's `session.json`.
/// `daemon_instance` is client-owned because it records **which serving process
/// this client last saw** — an observation, not a property of the workspace. Two
/// clients open on one remote workspace each keep their own, and neither may
/// overwrite the other's; a remote record that carried it would do exactly that.
pub const CLIENT_OWNED_FIELDS: &[&str] = &["window", "open", "host", "daemon_instance"];
/// The remote-owned half of a [`Workspace`], for reading a record back.
///
/// Every field defaults: a record written by a *newer* client carries fields
/// this build has never heard of (serde ignores them), and one written by an
/// older client is missing fields this build expects. Neither may fail the pull
/// — a workspace that will not decode is a workspace the user cannot open.
#[derive(Deserialize)]
struct RemoteRecord {
#[serde(default)]
name: Option<String>,
#[serde(default)]
session: Session,
#[serde(default)]
last_active: u64,
}
/// The whole `session.json`: every workspace tty7 knows about, plus which one
@@ -780,48 +668,6 @@ impl Workspaces {
closed
}
/// Drop pane ids that appear in more than one workspace *on the same
/// machine*, keeping the claim of whichever workspace was active most
/// recently. A duplicate would have two windows attach the same daemon
/// pane, and the daemon's single subscriber means the loser's terminal goes
/// silently dead — so this runs on every load, before any window is built.
///
/// **Scoped per machine, because a pane id only means anything within one
/// daemon.** Every daemon hands out 1, 2, 3…, so a laptop and a build box
/// both having a pane 1 is the normal case, not a conflict. Deduping
/// globally would make the remote workspace forfeit a claim on a pane that
/// is alive and well on its own machine — orphaning a live session over a
/// collision that never existed.
///
/// Returns the number of claims dropped (0 in the healthy case).
pub fn dedupe_pane_ids(&mut self) -> usize {
let mut order: Vec<(usize, u64)> = self
.workspaces
.iter()
.enumerate()
.map(|(i, w)| (i, w.last_active))
.collect();
// Most recently active first: it keeps its claim, earlier ones yield.
order.sort_by(|a, b| b.1.cmp(&a.1));
// One `seen` set per machine. `HostId` is process-local, but this only
// has to be self-consistent within the single pass below.
let mut seen: std::collections::HashMap<
crate::host::HostId,
std::collections::HashSet<u64>,
> = std::collections::HashMap::new();
let mut dropped = 0;
for (index, _) in order {
let workspace = &mut self.workspaces[index];
let host = workspace.host_id();
let seen_here = seen.entry(host).or_default();
for tab in &mut workspace.session.tabs {
dropped += drop_duplicate_pane_ids(&mut tab.pane, seen_here);
}
}
dropped
}
/// Persist as JSON, creating the parent directory if needed. Any
/// IO/serialization error is logged and swallowed — the app must never
/// crash or stall over session bookkeeping.
@@ -904,46 +750,6 @@ pub fn blank_pane_ids(pane: &mut SessionPane) -> usize {
}
}
/// Blank any `pane_id` already claimed by an earlier-visited workspace. A
/// blanked leaf still restores — it just spawns a fresh shell in its saved cwd,
/// the same path a session from before the daemon existed takes.
///
/// The agent resume fields go with it. A blanked leaf takes restore's
/// spawn-fresh path, and that path auto-types the agent's resume command —
/// but the pane this claim duplicated is still running that very agent under
/// its winning workspace, so "recovering" the loser would start a second
/// process on the same agent session id. The duplicate claim is the evidence
/// of a corrupted record, not of a lost conversation; the conversation lives
/// with the winner.
fn drop_duplicate_pane_ids(
pane: &mut SessionPane,
seen: &mut std::collections::HashSet<u64>,
) -> usize {
match pane {
SessionPane::Leaf {
pane_id,
agent_session_id,
agent_launch_argv,
..
} => match *pane_id {
Some(id) if !seen.insert(id) => {
log::warn!(
"workspace claims pane {id} twice; dropping the duplicate claim \
(and its agent resume, which the winning claim still owns)"
);
*pane_id = None;
*agent_session_id = None;
*agent_launch_argv = None;
1
}
_ => 0,
},
SessionPane::Split { a, b, .. } => {
drop_duplicate_pane_ids(a, seen) + drop_duplicate_pane_ids(b, seen)
}
}
}
/// Helpers for every test that touches the on-disk `session.json`. The
/// config-dir pin is process-wide (`set_config_dir` is first-call-wins), so
/// the file is process-wide too — any test that reads or writes it must hold
@@ -1276,72 +1082,6 @@ mod tests {
);
}
/// The stale-instance check: ids recorded against a *different* daemon
/// process are blanked (they now name unrelated shells at best), ids
/// recorded against the *same* one are kept, and an unknown on either side
/// changes nothing — treating "cannot tell" as stale would respawn every
/// pane on the first launch after an upgrade.
#[test]
fn stale_instance_blanks_pane_ids_and_matching_or_unknown_keeps_them() {
let fresh = |instance: Option<&str>| {
let mut ws = workspace(vec![tab(leaf(Some("/work"), Some(7)), None)]);
ws.daemon_instance = instance.map(str::to_string);
ws
};
let mut ws = fresh(Some("daemon-a"));
assert_eq!(ws.forget_stale_pane_ids(Some("daemon-b")), 1);
assert!(ws.pane_ids().is_empty());
assert_eq!(
ws.first_cwd(),
Some(PathBuf::from("/work")),
"the layout survives; only the claims go"
);
let mut ws = fresh(Some("daemon-a"));
assert_eq!(ws.forget_stale_pane_ids(Some("daemon-a")), 0);
assert_eq!(ws.pane_ids(), vec![7], "same process, ids stay attachable");
let mut ws = fresh(None);
assert_eq!(ws.forget_stale_pane_ids(Some("daemon-b")), 0);
assert_eq!(ws.pane_ids(), vec![7], "an old record is not judged");
let mut ws = fresh(Some("daemon-a"));
assert_eq!(ws.forget_stale_pane_ids(None), 0);
assert_eq!(ws.pane_ids(), vec![7], "an unknown daemon is not judged");
}
/// Unlike a duplicate claim, a stale-instance claim keeps its agent resume:
/// the daemon those panes lived in is gone, nothing else runs the
/// conversation, and the fresh shell resuming it is the feature working.
#[test]
fn stale_instance_keeps_the_agent_resume() {
let mut ws = workspace(vec![tab(
SessionPane::Leaf {
cwd: Some(PathBuf::from("/work")),
pane_id: Some(7),
ssh_spec: None,
agent: Some(crate::core::cli_agent::CLIAgent::Claude),
agent_session_id: Some("sid".into()),
agent_launch_argv: None,
},
None,
)]);
ws.daemon_instance = Some("daemon-a".into());
assert_eq!(ws.forget_stale_pane_ids(Some("daemon-b")), 1);
match &ws.session.tabs[0].pane {
SessionPane::Leaf {
pane_id,
agent_session_id,
..
} => {
assert!(pane_id.is_none());
assert_eq!(agent_session_id.as_deref(), Some("sid"));
}
SessionPane::Split { .. } => panic!("leaf stays a leaf"),
}
}
#[test]
fn display_name_prefers_user_name_then_repo_then_cwd() {
// No name, no repo group: fall back to the first leaf's directory.
@@ -1398,181 +1138,6 @@ mod tests {
assert_eq!(ws.first_cwd(), Some(PathBuf::from("/a")));
}
#[test]
fn dedupe_pane_ids_keeps_the_most_recently_active_claim() {
// Two workspaces both claim pane 5 — the crash/hand-edit case. The
// stale one must yield, or its window silently steals the live one's
// stream when both attach (the daemon has a single subscriber).
let mut stale = workspace(vec![tab(leaf(Some("/old"), Some(5)), None)]);
stale.last_active = 100;
let mut fresh = workspace(vec![tab(leaf(Some("/new"), Some(5)), None)]);
fresh.last_active = 200;
let (stale_id, fresh_id) = (stale.id, fresh.id);
let mut all = Workspaces {
active: Some(fresh_id),
workspaces: vec![stale, fresh],
};
assert_eq!(all.dedupe_pane_ids(), 1);
// The recent one keeps pane 5; the stale one drops to a fresh spawn in
// its saved cwd (cwd is preserved — only the id is cleared).
assert_eq!(all.get(fresh_id).unwrap().pane_ids(), vec![5]);
assert!(all.get(stale_id).unwrap().pane_ids().is_empty());
assert_eq!(
all.get(stale_id).unwrap().first_cwd(),
Some(PathBuf::from("/old"))
);
}
/// The duplicate claim loses its agent resume along with its pane id.
/// Restore's spawn-fresh path auto-types the agent's resume command, and
/// the winning workspace's pane is still *running* that agent — a loser
/// that kept `agent_session_id` would come back as a second process on
/// the same conversation (double `claude --resume <id>`, both live).
#[test]
fn dedupe_pane_ids_disarms_the_duplicate_claims_agent_resume() {
let agent_leaf = |pane_id| SessionPane::Leaf {
cwd: Some(PathBuf::from("/work")),
pane_id: Some(pane_id),
ssh_spec: None,
agent: Some(crate::core::cli_agent::CLIAgent::Claude),
agent_session_id: Some("362f9261".into()),
agent_launch_argv: Some(vec!["claude".into(), "--continue".into()]),
};
let mut stale = workspace(vec![tab(agent_leaf(5), None)]);
stale.last_active = 100;
let mut fresh = workspace(vec![tab(agent_leaf(5), None)]);
fresh.last_active = 200;
let (stale_id, fresh_id) = (stale.id, fresh.id);
let mut all = Workspaces {
active: Some(fresh_id),
workspaces: vec![stale, fresh],
};
assert_eq!(all.dedupe_pane_ids(), 1);
let loser = &all.get(stale_id).unwrap().session.tabs[0].pane;
match loser {
SessionPane::Leaf {
pane_id,
cwd,
agent_session_id,
agent_launch_argv,
..
} => {
assert!(pane_id.is_none());
assert_eq!(
cwd.as_deref(),
Some(std::path::Path::new("/work")),
"the layout survives — only the claim and its resume go"
);
assert!(
agent_session_id.is_none(),
"no second resume of one conversation"
);
assert!(agent_launch_argv.is_none());
}
SessionPane::Split { .. } => panic!("the leaf must survive as a leaf"),
}
// The winner is untouched: its pane is the one actually running the agent.
match &all.get(fresh_id).unwrap().session.tabs[0].pane {
SessionPane::Leaf {
pane_id,
agent_session_id,
..
} => {
assert_eq!(*pane_id, Some(5));
assert_eq!(agent_session_id.as_deref(), Some("362f9261"));
}
SessionPane::Split { .. } => panic!("the leaf must survive as a leaf"),
}
}
/// A pane id is only unique within one daemon, so the same number on two
/// machines is not a collision. Deduping globally would make the remote
/// workspace forfeit a claim on a pane that is alive on its own box —
/// orphaning a live session over a conflict that never existed.
#[test]
fn dedupe_pane_ids_is_scoped_to_one_machine() {
let mut local = workspace(vec![tab(leaf(Some("/local"), Some(1)), None)]);
local.last_active = 200;
let mut remote = workspace(vec![tab(leaf(Some("/remote"), Some(1)), None)]);
remote.last_active = 100; // older, so a global dedupe would drop *this* one
remote.host = Some(RemoteRef {
target: RemoteTarget::Alias {
alias: "build-box".into(),
},
workspace: WorkspaceId::new(),
});
let (local_id, remote_id) = (local.id, remote.id);
let mut all = Workspaces {
active: Some(local_id),
workspaces: vec![local, remote],
};
assert_eq!(all.dedupe_pane_ids(), 0, "different machines never collide");
assert_eq!(all.get(local_id).unwrap().pane_ids(), vec![1]);
assert_eq!(
all.get(remote_id).unwrap().pane_ids(),
vec![1],
"the remote keeps its claim on its own daemon's pane 1"
);
// …and two workspaces on the *same* remote machine still dedupe.
let host = RemoteRef {
target: RemoteTarget::Alias {
alias: "build-box".into(),
},
workspace: WorkspaceId::new(),
};
let mut older = workspace(vec![tab(leaf(Some("/a"), Some(7)), None)]);
older.last_active = 100;
older.host = Some(host.clone());
let mut newer = workspace(vec![tab(leaf(Some("/b"), Some(7)), None)]);
newer.last_active = 200;
newer.host = Some(host);
let (older_id, newer_id) = (older.id, newer.id);
let mut same_box = Workspaces {
active: Some(newer_id),
workspaces: vec![older, newer],
};
assert_eq!(same_box.dedupe_pane_ids(), 1);
assert_eq!(same_box.get(newer_id).unwrap().pane_ids(), vec![7]);
assert!(same_box.get(older_id).unwrap().pane_ids().is_empty());
}
#[test]
fn dedupe_pane_ids_is_a_noop_on_healthy_sessions() {
let mut all = Workspaces {
active: None,
workspaces: vec![
workspace(vec![tab(leaf(Some("/a"), Some(1)), None)]),
workspace(vec![tab(leaf(Some("/b"), Some(2)), None)]),
],
};
assert_eq!(all.dedupe_pane_ids(), 0);
assert_eq!(all.workspaces[0].pane_ids(), vec![1]);
assert_eq!(all.workspaces[1].pane_ids(), vec![2]);
}
#[test]
fn dedupe_pane_ids_catches_a_duplicate_within_one_workspace() {
// Same guarantee inside a single workspace: a split that somehow ended
// up with the same pane in both halves would deadlock the same way.
let mut all = Workspaces {
active: None,
workspaces: vec![workspace(vec![
tab(leaf(Some("/a"), Some(1)), None),
tab(leaf(Some("/b"), Some(1)), None),
])],
};
assert_eq!(all.dedupe_pane_ids(), 1);
assert_eq!(all.workspaces[0].pane_ids(), vec![1]);
}
// ── Remote workspaces (M5) ──────────────────────────────────────────────
/// A real-shaped `session.json` from before `host` existed, written by the
@@ -1808,160 +1373,6 @@ mod tests {
);
}
#[test]
fn a_remote_workspace_survives_a_restart() {
let remote_id = WorkspaceId::new();
let mut ws = Workspace::on_remote(RemoteRef::new(
RemoteTarget::direct("me", "box.local", 2222),
remote_id,
));
ws.name = Some("api".into());
ws.open = false;
let all = Workspaces {
active: None,
workspaces: vec![ws],
};
let text = serde_json::to_string(&all).unwrap();
let back = Workspaces::decode(&text).unwrap();
let only = &back.workspaces[0];
assert!(only.is_remote());
let host = only.host.as_ref().unwrap();
assert_eq!(host.workspace, remote_id);
assert_eq!(host.target, RemoteTarget::direct("me", "box.local", 2222));
assert_eq!(host.target.connection_key(), "ssh-direct:me@box.local:2222");
}
/// Every `Workspace` field belongs to exactly one side of the storage
/// split. A new field that is in neither list would be silently dropped by
/// `to_remote_json` and lost on the next pull, which is data loss that no
/// other test would notice.
#[test]
fn the_storage_split_covers_every_workspace_field() {
let mut ws = workspace(vec![tab(leaf(Some("/w"), Some(1)), Some("/w"))]);
ws.name = Some("named".into());
ws.window = Some(crate::core::window_state::WindowState {
x: 0.0,
y: 0.0,
width: 800.0,
height: 600.0,
});
ws.host = Some(RemoteRef::new(
RemoteTarget::Alias {
alias: "devbox".into(),
},
WorkspaceId::new(),
));
// Every skip-when-`None` field must be populated here, or it never
// serializes and this census can't see it.
ws.daemon_instance = Some("daemon-uuid".into());
let value = serde_json::to_value(&ws).unwrap();
let mut present: Vec<String> = value
.as_object()
.unwrap()
.keys()
.map(String::from)
.collect();
present.sort();
let mut expected: Vec<String> = REMOTE_OWNED_FIELDS
.iter()
.chain(CLIENT_OWNED_FIELDS)
.map(|s| (*s).to_string())
.collect();
expected.sort();
assert_eq!(
present, expected,
"a Workspace field is on neither side of the storage split; decide which \
machine owns it and add it to REMOTE_OWNED_FIELDS or CLIENT_OWNED_FIELDS"
);
}
#[test]
fn the_remote_record_carries_the_layout_and_nothing_local() {
let mut ws = workspace(vec![tab(leaf(Some("/srv/app"), Some(7)), Some("/srv/app"))]);
ws.name = Some("app".into());
ws.last_active = 1_753_600_000;
ws.open = true;
ws.window = Some(crate::core::window_state::WindowState {
x: 1.0,
y: 2.0,
width: 800.0,
height: 600.0,
});
ws.host = Some(RemoteRef::new(
RemoteTarget::Alias {
alias: "devbox".into(),
},
WorkspaceId::new(),
));
let record = ws.to_remote_json();
let obj = record.as_object().unwrap();
// The machine's facts go over.
assert!(obj.contains_key("session"));
assert_eq!(obj["name"], "app");
assert_eq!(obj["last_active"], 1_753_600_000u64);
assert_eq!(obj["id"], ws.id.to_string());
// This client's view state does not — the point of the split.
for k in CLIENT_OWNED_FIELDS {
assert!(!obj.contains_key(*k), "`{k}` must not leave this machine");
}
// Pulling it back onto a *different* client's entry updates the layout
// and leaves that client's own view state alone.
let mut mine = Workspace::on_remote(RemoteRef::new(
RemoteTarget::Alias {
alias: "devbox".into(),
},
ws.id,
));
mine.open = false;
mine.window = None;
let my_id = mine.id;
mine.apply_remote_json(&record).unwrap();
assert_eq!(mine.session.tabs.len(), 1);
assert_eq!(mine.name.as_deref(), Some("app"));
assert_eq!(mine.last_active, 1_753_600_000);
assert_eq!(
mine.id, my_id,
"the client's own entry id is not overwritten"
);
assert!(
!mine.open,
"the remote has no opinion about my open windows"
);
assert!(mine.window.is_none());
assert!(mine.is_remote(), "and it is still a remote workspace");
}
/// A record from a newer client carries fields this build has never seen,
/// and one from an older client is missing fields it expects. Neither may
/// fail the pull.
#[test]
fn applying_a_record_tolerates_version_skew() {
let mut ws = Workspace::default();
ws.apply_remote_json(&serde_json::json!({
"id": "6a8f2a1e-1c1b-4f7a-9d3e-2b5c8e4a7f01",
"session": {"active": 0, "tabs": []},
"last_active": 5,
"something_from_2027": {"nested": true}
}))
.expect("unknown fields are ignored, not fatal");
assert_eq!(ws.last_active, 5);
let mut ws = Workspace {
name: Some("stale".into()),
..Workspace::default()
};
ws.apply_remote_json(&serde_json::json!({}))
.expect("a record missing every optional field still applies");
assert_eq!(
ws.name, None,
"the remote's answer wins, including 'no name'"
);
assert!(ws.session.tabs.is_empty());
}
#[test]
fn open_and_closed_partition_by_flag_and_recency() {
let mut open_one = workspace(vec![]);
crates/tty7-core/src/core/workspace_store.rs
+9 -1
View File
@@ -870,7 +870,15 @@ mod tests {
ws.name = Some("api".into());
ws.last_active = 1_753_600_000;
let id = ws.id.to_string();
store.put(&id, ws.to_remote_json(), None).unwrap();
// The remote-owned half of a record, spelled inline: the helper that
// used to derive it is retired with the whole-record write path.
let record = serde_json::json!({
"id": id,
"name": "api",
"session": { "active": 0, "tabs": [] },
"last_active": 1_753_600_000u64,
});
store.put(&id, record, None).unwrap();
let text = std::fs::read_to_string(dir.path().join(STORE_FILE)).unwrap();
let parsed = Workspaces::decode(&text).expect("the store's file is a Workspaces document");
src/core/session.rs
+23 -348
View File
@@ -30,14 +30,7 @@ impl WorkspaceStore {
/// duplicate pane claims, and install the result as the app global. Call
/// once, before the first window is built.
pub fn init(cx: &mut gpui::App) {
let mut workspaces = Workspaces::load().unwrap_or_default();
let dropped = workspaces.dedupe_pane_ids();
if dropped > 0 {
log::warn!(
"session.json claimed {dropped} pane(s) from more than one workspace; \
the stale claims will spawn fresh shells instead"
);
}
let workspaces = Workspaces::load().unwrap_or_default();
cx.set_global(Self { workspaces });
}
@@ -84,7 +77,6 @@ impl WorkspaceStore {
// workspace whose machine is unreachable must open empty. See
// [`claimable_session`].
let reachable = id.is_none_or(|id| Self::machine_is_connected(cx, id));
let instance = Self::serving_instance(cx, id);
let Some(store) = Self::try_store(cx) else {
// No store (tests): hand back a detached identity so the window
// still builds, but nothing is persisted.
@@ -100,10 +92,7 @@ impl WorkspaceStore {
};
workspace.open = true;
workspace.touch();
let claimed = (
workspace.id,
claimable_session(workspace, reachable, instance.as_deref()),
);
let claimed = (workspace.id, claimable_session(workspace, reachable));
store.workspaces.active = Some(claimed.0);
store.workspaces.save();
claimed
@@ -122,7 +111,6 @@ impl WorkspaceStore {
// describing that machine's layout, so it does not get to overwrite the
// copy we have of it — see [`record_session`].
let reachable = Self::machine_is_connected(cx, id);
let instance = Self::serving_instance(cx, Some(id));
let Some(store) = Self::try_store(cx) else {
return;
};
@@ -131,7 +119,7 @@ impl WorkspaceStore {
// tearing down); nothing to record.
return;
};
record_session(workspace, session, reachable, instance);
record_session(workspace, session, reachable);
if let Some(window) = window {
workspace.window = Some(window);
}
@@ -297,60 +285,6 @@ impl WorkspaceStore {
crate::ui::remote_connect::RemoteConnections::get(cx, host.host_id()).is_some()
}
/// The process whose pane ids this workspace's record is about: this
/// machine's daemon for a local workspace, the far machine's `tty7-server`
/// for a remote one. `None` when it cannot be named — an older peer, a
/// machine not connected right now, or a brand-new workspace with no host
/// yet — which every reader treats as "no instance check possible".
///
/// One function for both because [`Workspace::daemon_instance`] means the
/// same thing on both sides. It used to be local-only, on the reasoning
/// that a remote server's identity is tracked live per connection instead
/// — but that live map lives in memory, so it is empty on the launch that
/// matters most: the one where the client was closed while the remote
/// server was replaced.
pub fn serving_instance(cx: &mut gpui::App, id: Option<WorkspaceId>) -> Option<String> {
match id.and_then(|id| Self::remote_ref(cx, id)) {
Some(host) => crate::ui::remote_connect::RemoteConnections::get(cx, host.host_id())
.map(|h| h.peer().instance.clone())
.filter(|instance| !instance.is_empty()),
None => crate::daemon::spawn::local_daemon_instance(),
}
}
/// Blank `id`'s saved pane ids when they were recorded against a different
/// server process than `instance`, and persist that. Answers whether any
/// were dropped.
///
/// The remote counterpart of the check [`claimable_session`] runs for a
/// local workspace at claim time. It cannot run there for a remote one: at
/// claim time the machine is usually not connected yet, so there is no
/// instance to compare against. The reconnect is the first moment the
/// answer exists, which is where this is called from.
pub fn forget_stale_pane_ids(cx: &mut gpui::App, id: WorkspaceId, instance: &str) -> bool {
let current = (!instance.is_empty()).then_some(instance);
let Some(store) = Self::try_store(cx) else {
return false;
};
let Some(workspace) = store.workspaces.get_mut(id) else {
return false;
};
let dropped = workspace.forget_stale_pane_ids(current);
if dropped == 0 {
return false;
}
// Stamped now rather than left for the next save: the record has just
// been made to describe *this* server, and a crash before the window
// saves must not leave it claiming the old process again.
workspace.daemon_instance = current.map(str::to_string);
store.workspaces.save();
log::info!(
"workspace {id}: {dropped} saved pane id(s) belong to a previous \
tty7-server process; rebuilding from the layout"
);
true
}
/// The client-side entry for `host` — the existing one if this machine has
/// seen that workspace before, a fresh one otherwise.
///
@@ -387,47 +321,6 @@ impl WorkspaceStore {
store.workspaces.save();
id
}
/// Merge an authoritative record pulled from the remote into the client's
/// entry. Only the remote-owned fields move; `open`, `window` and `host`
/// stay as this machine left them (see [`Workspace::apply_remote_json`]).
///
/// A record that will not decode is dropped with a log line rather than
/// failing the open: the layout is recoverable on the next push, an
/// unopenable workspace is not.
pub fn apply_remote(cx: &mut gpui::App, id: WorkspaceId, record: &serde_json::Value) {
let Some(store) = Self::try_store(cx) else {
return;
};
let Some(workspace) = store.workspaces.get_mut(id) else {
return;
};
if let Err(e) = workspace.apply_remote_json(record) {
log::warn!("remote workspace {id} sent a record this build cannot read: {e}");
return;
}
store.workspaces.save();
}
/// What to send the remote for `id`: its store key and the remote-owned half
/// of the record. `None` for a local workspace — there is nobody to send to.
pub fn remote_payload(
cx: &gpui::App,
id: WorkspaceId,
) -> Option<(RemoteRef, String, serde_json::Value)> {
let workspace = Self::all(cx).get(id)?;
let host = workspace.host.clone()?;
let key = host.store_key();
// The record travels under the *remote's* id, not the client entry's:
// the remote store is keyed by its own ids, and a record whose `id`
// disagreed with its key would be a workspace that renames itself on
// every round trip.
let mut record = workspace.to_remote_json();
if let Some(obj) = record.as_object_mut() {
obj.insert("id".to_string(), serde_json::json!(key));
}
Some((host, key, record))
}
}
/// The machine a window showing `id` is bound to.
@@ -456,57 +349,21 @@ pub(crate) fn crosses_machines(previous: HostId, current: HostId) -> bool {
}
/// The layout a window opening on `workspace` may rebuild — the read-side twin
/// of [`record_session`].
/// of [`record_session`], and by now only the *fallback* source: the machine's
/// tree is the layout authority, and the hydration that follows a claim is
/// what actually fills the window. What this still decides is the shape the
/// window opens in.
///
/// A remote entry's `session` is this client's copy of a record the machine
/// owns: pulled on connect, refreshed on every `WorkspaceChanged`, pushed back
/// on every structural change. Rebuilding from it is the whole of "reconnecting
/// gets my tabs back", and it is safe to do because every pane it names is
/// routed by [`crate::ui::remote_workspace::pane_workspace_for`] — a leaf in a
/// remote workspace attaches or spawns *over there*, and a machine that cannot
/// be reached fails the spawn rather than falling back to a local shell.
///
/// `reachable` is what keeps that guarantee from being theoretical. With the
/// link down, `List` answers nothing, so every leaf would miss its live pane and
/// try to spawn a fresh one — either failing (an empty window, having thrown the
/// layout away) or, worse, landing a second shell next to the one still running
/// over there. So an unreachable remote workspace opens empty **without
/// touching the cached layout**, and
/// [`crate::ui::remote_workspace`]'s connect path rebuilds the window the moment
/// the machine answers.
/// `current_instance` is the identity of the process serving this workspace's
/// panes (see [`WorkspaceStore::serving_instance`]). A workspace whose saved ids
/// were recorded against a different one blanks them first — after a restart the
/// numbers begin again at 1, so a stale id would otherwise pass the aliveness
/// check by landing on whatever unrelated pane holds it now. Blanked in the
/// stored entry too, not just the returned copy, so the record stops claiming
/// panes that no longer exist even if the window never saves again.
///
/// A remote workspace usually reaches the early return above instead: at claim
/// time its machine is not connected yet, so there is no instance to compare and
/// no layout to hand back. `remote_workspace::finish_attempt` runs the same
/// check the moment the connect answers, which is the first point it can.
fn claimable_session(
workspace: &mut Workspace,
reachable: bool,
current_instance: Option<&str>,
) -> Session {
// A remote workspace *always* opens empty now, reachable or not: its
// machine's tree is the layout authority, and the hydration that follows
// the claim (`tree_sync::hydrate_window_from_tree`) is what fills the
// window — from the tree, not from this client's cache.
/// A remote workspace opens empty unconditionally — its layout lives on
/// another machine, and this client's cached copy is an import fallback, not
/// something to build panes from. A local one hands back the cached layout for
/// the paths that deliberately skip hydration (restore off, and the hydration
/// import itself).
fn claimable_session(workspace: &mut Workspace, reachable: bool) -> Session {
let _ = reachable;
if workspace.is_remote() {
return Session::default();
}
let dropped = workspace.forget_stale_pane_ids(current_instance);
if dropped > 0 {
log::info!(
"workspace {}: {dropped} saved pane id(s) belong to a previous serving \
process; restoring with fresh shells (and agent resume where recorded)",
workspace.id
);
}
workspace.session.clone()
}
@@ -515,29 +372,14 @@ fn claimable_session(
///
/// A window that cannot reach its machine is not describing that machine's
/// layout (its panes failed to restore, or are sitting there disconnected), so
/// it records nothing rather than replacing the copy we have with the wreckage.
/// The remote's own `workspaces.json` is still the authority; this entry is the
/// cache the next launch opens from.
/// The record is stamped with the process its pane ids came from (`instance`):
/// this machine's daemon for a local workspace, the far machine's
/// `tty7-server` for a remote one. That is what lets the next launch tell a
/// surviving process from a replaced one — see [`claimable_session`] and
/// [`WorkspaceStore::forget_stale_pane_ids`].
///
/// The unreachable early return doubles as the guard on that stamp: with the
/// machine down there is no instance to record, and writing `None` over a good
/// one would throw away the very comparison the next connect needs.
fn record_session(
workspace: &mut Workspace,
session: Session,
reachable: bool,
instance: Option<String>,
) {
/// it records nothing rather than replacing the copy we have with the
/// wreckage. The machine's tree is the authority; this entry is the cache the
/// hydration import falls back to.
fn record_session(workspace: &mut Workspace, session: Session, reachable: bool) {
if workspace.is_remote() && !reachable {
return;
}
workspace.session = session;
workspace.daemon_instance = instance;
}
#[cfg(test)]
@@ -580,7 +422,7 @@ mod tests {
#[test]
fn a_local_workspace_stores_its_own_layout() {
let mut workspace = Workspace::default();
record_session(&mut workspace, local_layout(), true, None);
record_session(&mut workspace, local_layout(), true);
assert_eq!(workspace.session.tabs.len(), 1);
assert_eq!(workspace.pane_ids(), vec![7]);
}
@@ -592,7 +434,7 @@ mod tests {
#[test]
fn a_connected_remote_workspace_stores_its_layout() {
let mut workspace = Workspace::on_remote(remote_ref());
record_session(&mut workspace, local_layout(), true, None);
record_session(&mut workspace, local_layout(), true);
assert_eq!(workspace.session.tabs.len(), 1);
assert_eq!(
workspace.pane_ids(),
@@ -601,35 +443,6 @@ mod tests {
);
}
/// A remote workspace's record is stamped with the **server's** instance,
/// not left blank. That stamp is the only part of "which process minted
/// these ids" that survives the client being closed, and it is what the
/// next connect compares against before re-attaching anything.
#[test]
fn a_connected_remote_workspace_records_the_serving_instance() {
let mut workspace = Workspace::on_remote(remote_ref());
record_session(
&mut workspace,
local_layout(),
true,
Some("server-a".to_string()),
);
assert_eq!(workspace.daemon_instance.as_deref(), Some("server-a"));
}
/// …and an unreachable machine does not un-stamp it. `None` there means
/// "nobody to ask", and writing it over a good value would disarm the very
/// check the next connect needs — the ids would look current again.
#[test]
fn an_unreachable_remote_window_does_not_erase_the_recorded_instance() {
let mut workspace = Workspace::on_remote(remote_ref());
workspace.session = local_layout();
workspace.daemon_instance = Some("server-a".to_string());
record_session(&mut workspace, Session::default(), false, None);
assert_eq!(workspace.daemon_instance.as_deref(), Some("server-a"));
assert_eq!(workspace.session.tabs.len(), 1, "and the layout stays too");
}
/// A local workspace opens on the layout it saved.
#[test]
fn a_local_workspace_reopens_its_saved_layout() {
@@ -637,48 +450,12 @@ mod tests {
session: local_layout(),
..Workspace::default()
};
let claimed = claimable_session(&mut workspace, true, None);
let claimed = claimable_session(&mut workspace, true);
assert_eq!(claimed.tabs.len(), 1);
// And the entry is left alone.
assert_eq!(workspace.session.tabs.len(), 1);
}
/// Claiming a local workspace whose ids were recorded against a *different*
/// daemon process blanks them — in the returned session **and** in the
/// stored entry. After a reboot the numbers restart from 1, so a stale id
/// passes the aliveness check by landing on whatever unrelated pane holds
/// it now; blanking is what turns that into an honest fresh spawn (with
/// the agent resume the leaf recorded).
#[test]
fn claiming_a_local_workspace_from_another_daemon_process_blanks_its_ids() {
let mut workspace = Workspace {
session: local_layout(),
daemon_instance: Some("previous-boot".into()),
..Workspace::default()
};
let leaf_id = |session: &Session| match &session.tabs[0].pane {
SessionPane::Leaf { pane_id, .. } => *pane_id,
SessionPane::Split { .. } => panic!("the fixture is a single leaf"),
};
let claimed = claimable_session(&mut workspace, true, Some("current-boot"));
assert_eq!(claimed.tabs.len(), 1, "the layout still restores");
assert_eq!(
leaf_id(&claimed),
None,
"but no leaf may attach by a number from a dead daemon"
);
assert!(workspace.pane_ids().is_empty(), "the entry agrees");
// Same process → the ids stay attachable.
let mut workspace = Workspace {
session: local_layout(),
daemon_instance: Some("current-boot".into()),
..Workspace::default()
};
let claimed = claimable_session(&mut workspace, true, Some("current-boot"));
assert_eq!(leaf_id(&claimed), Some(7));
}
/// A remote workspace opens empty even when its machine is connected: the
/// machine's tree is the layout authority now, and the hydration that
/// follows the claim fills the window from it. The cached copy stays —
@@ -687,7 +464,7 @@ mod tests {
fn a_connected_remote_workspace_still_opens_empty_for_the_tree_to_fill() {
let mut workspace = Workspace::on_remote(remote_ref());
workspace.session = local_layout();
let claimed = claimable_session(&mut workspace, true, None);
let claimed = claimable_session(&mut workspace, true);
assert!(claimed.tabs.is_empty());
assert_eq!(workspace.session.tabs.len(), 1, "the cache is kept");
}
@@ -702,7 +479,7 @@ mod tests {
let mut workspace = Workspace::on_remote(remote_ref());
workspace.session = local_layout();
let claimed = claimable_session(&mut workspace, false, None);
let claimed = claimable_session(&mut workspace, false);
assert!(claimed.tabs.is_empty(), "the window must open with no tabs");
assert_eq!(
workspace.session.tabs.len(),
@@ -718,7 +495,7 @@ mod tests {
fn an_unreachable_remote_window_does_not_overwrite_the_cached_layout() {
let mut workspace = Workspace::on_remote(remote_ref());
workspace.session = local_layout();
record_session(&mut workspace, Session::default(), false, None);
record_session(&mut workspace, Session::default(), false);
assert_eq!(workspace.session.tabs.len(), 1);
}
@@ -806,108 +583,6 @@ mod tests {
});
}
/// **The cold-launch half of the restart check.** `RemoteLinks::instances`
/// is in memory, so on the first connect after the client starts every
/// machine is a first sighting and nothing is judged a restart. A server
/// replaced while the client was closed would therefore sail through, and
/// its recycled ids — daemons number panes from 1 — would attach to
/// whatever unrelated shells hold those numbers now. The stamp on the
/// record is what closes that, so this is the test that has to hold.
#[gpui::test]
fn a_remote_workspace_drops_pane_ids_minted_by_a_previous_server(
cx: &mut gpui::TestAppContext,
) {
cx.update(|cx| {
crate::core::config::pin_test_config_dir();
let mut entry = Workspace::on_remote(remote_ref());
entry.session = local_layout();
entry.daemon_instance = Some("server-a".to_string());
let id = entry.id;
WorkspaceStore::install_for_test(
cx,
Workspaces {
workspaces: vec![entry],
active: None,
},
);
// Same process: these ids still name the panes they always did.
assert!(!WorkspaceStore::forget_stale_pane_ids(cx, id, "server-a"));
assert_eq!(WorkspaceStore::all(cx).get(id).unwrap().pane_ids(), vec![7]);
// An unknown instance is never judged — a peer too old to report
// one must not cost the user every pane on the machine.
assert!(!WorkspaceStore::forget_stale_pane_ids(cx, id, ""));
assert_eq!(WorkspaceStore::all(cx).get(id).unwrap().pane_ids(), vec![7]);
// Replaced: the claims go, the layout stays, and the stamp moves on.
assert!(WorkspaceStore::forget_stale_pane_ids(cx, id, "server-b"));
let after = WorkspaceStore::all(cx).get(id).unwrap();
assert!(after.pane_ids().is_empty());
assert_eq!(
after.session.tabs.len(),
1,
"the layout is exactly what the rebuild draws from"
);
assert_eq!(
after.daemon_instance.as_deref(),
Some("server-b"),
"stamped now, so a crash before the next save cannot re-arm the old claim"
);
// And the same server is not a restart twice over.
assert!(!WorkspaceStore::forget_stale_pane_ids(cx, id, "server-b"));
});
}
/// **Why clearing the ids locally is not enough.** The remote owns the
/// record, so reopening pulls its copy over the client's — and
/// a copy that still claims the killed panes puts them straight back. This
/// is the constraint `windows::forget_killed_panes` pushes to satisfy; if
/// this assertion ever flips, that push is dead weight.
#[test]
fn a_remote_record_reinstates_pane_ids_a_client_only_clear_dropped() {
let mut theirs = Workspace::on_remote(remote_ref());
theirs.session = local_layout();
let record = theirs.to_remote_json();
let mut ours = Workspace::on_remote(remote_ref());
ours.session = local_layout();
ours.forget_pane_ids();
assert!(ours.pane_ids().is_empty());
ours.apply_remote_json(&record).unwrap();
assert_eq!(
ours.pane_ids(),
vec![7],
"the machine's copy wins, so the clear has to reach it"
);
}
/// The remote-bound payload travels under the *remote's* id, so a record
/// pushed and pulled back names the same workspace both times.
#[test]
fn the_remote_payload_is_keyed_by_the_remote_id_not_the_client_entry() {
let host = remote_ref();
let workspace = Workspace::on_remote(host.clone());
let mut record = workspace.to_remote_json();
record
.as_object_mut()
.unwrap()
.insert("id".into(), serde_json::json!(host.store_key()));
assert_eq!(host.store_key(), host.workspace.to_string());
assert_ne!(host.store_key(), workspace.id.to_string());
assert_eq!(record["id"], serde_json::json!(host.store_key()));
// The client-owned half never crosses.
for client_only in tty7_core::core::session::CLIENT_OWNED_FIELDS {
assert!(
record.get(*client_only).is_none(),
"{client_only} must not be sent to the remote"
);
}
}
/// **The window/host invariant, as a test.**
///
/// A window is one machine. The inverse is listed under
src/ui/remote_connect.rs
+1 -1
View File
@@ -1260,7 +1260,7 @@ mod tests {
/// name derived the way a local workspace's would be when none is set.
#[test]
fn rows_from_the_tree_sort_newest_first_and_derive_names() {
use tty7_core::core::machine::{Machine, PaneNode, PaneRecord, Tab, Workspace};
use tty7_core::core::machine::{Machine, PaneRecord, Tab, Workspace};
let older = WorkspaceId::new();
let newer = WorkspaceId::new();
let machine = Machine {