Duke/cmux
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Make the topology own tab identity

Browse Source 
Tab identity was re-derived on every read from either the live surface or
the slot indexes, so each reader invented its own missing-data policy: the
projection failed hard, `rebuild_resource_indexes` silently dropped the tab,
and the browser branch required a live surface. A silent drop is the worst
of the three, because the next projection tombstones durable rows that are
still live.

`State::register_tab_identity` is now the writer every placement path uses,
including `insert_surface_checked` and the browser attach path, which
previously left identity to be harvested from the surface at the next index
rebuild. The reserved-placement install in `resource_project_terminal_selected`
still writes its own placement order, which `register_tab_identity` must not
reorder; it writes the same identity fields.

Every reader now takes identity from the topology: the index rebuild no
longer consults surfaces, the projection resolver reads the owner, and the
layout-undo token and active-tab lookup drop their surface fallbacks.
`State::ensure_tab_identity_coverage` runs at the projection boundary, so a
missing identity fails that mutation by name instead of erasing durable rows.

Browser tabs now project from their durable row when their runtime is gone,
matching terminals. That closes the same failure for a browser view whose
surface never materialized.
This commit is contained in:
Lawrence Chen
2026-08-17 17:15:07 -07:00
parent aa046ab0c7
commit 70f2e14f44
3 changed files with 161 additions and 69 deletions
Download Patch File Download Diff File Expand all files Collapse all files
cmux-tui/crates/cmux-tui-core/src/model.rs
+47 -15
View File
@@ -7,7 +7,7 @@ use std::sync::Arc;
use crate::resource::{
ContentPublicId, PanePublicId, PublicSlotIndexes, ScreenPublicId, TabPublicId,
TerminalPublicId, WorkspacePublicId,
TabResourceIdentity, TerminalPublicId, WorkspacePublicId,
};
use crate::{PaneId, ScreenId, SplitDir, SplitId, Surface, SurfaceId, WorkspaceId};
@@ -941,6 +941,43 @@ impl State {
}
}
/// Record the durable identity of one tab slot. This is the only writer
/// of tab identity, so a slot can never disagree with the topology it is
/// placed in.
pub(crate) fn register_tab_identity(
&mut self,
slot: SurfaceId,
identity: &TabResourceIdentity,
) {
self.resource_indexes.tabs.insert(identity.tab_id.clone(), slot);
self.resource_indexes.tab_ids.insert(slot, identity.tab_id.clone());
let placements = self
.resource_indexes
.content_placements
.entry(identity.content_id.clone())
.or_default();
if !placements.contains(&slot) {
placements.push(slot);
}
self.resource_indexes.content_ids.insert(slot, identity.content_id.clone());
}
/// Every placed tab must carry a durable identity. Losing one would make
/// the next projection tombstone live durable rows, so this fails the
/// mutation instead of silently dropping the tab.
pub(crate) fn ensure_tab_identity_coverage(&self) -> anyhow::Result<()> {
for pane in self.panes.values() {
for slot in &pane.tabs {
anyhow::ensure!(
self.resource_indexes.tab_ids.contains_key(slot)
&& self.resource_indexes.content_ids.contains_key(slot),
"tab slot {slot} has no durable identity"
);
}
}
Ok(())
}
pub(crate) fn rebuild_resource_indexes(&mut self) {
let mut indexes = PublicSlotIndexes::default();
let mut live_split_slots = self.split_screens.keys().copied().collect::<HashSet<_>>();
@@ -961,20 +998,15 @@ impl State {
indexes.pane_ids.insert(pane.id, pane.public_id.clone());
indexes.pane_screen.insert(pane.id, screen.id);
for surface_id in &pane.tabs {
let identity = self
.surfaces
.get(surface_id)
.and_then(|surface| surface.resource_identity())
.map(|identity| {
(identity.tab_id.clone(), identity.content_id.clone())
})
.or_else(|| {
Some((
self.resource_indexes.tab_ids.get(surface_id)?.clone(),
self.resource_indexes.content_ids.get(surface_id)?.clone(),
))
});
let Some((tab_id, content_id)) = identity else { continue };
// Tab identity is owned by the topology, never by
// the live surface. A restored or detached tab has
// no surface, and rebuilding must not lose it.
let (Some(tab_id), Some(content_id)) = (
self.resource_indexes.tab_ids.get(surface_id).cloned(),
self.resource_indexes.content_ids.get(surface_id).cloned(),
) else {
continue;
};
let old = indexes.tabs.insert(tab_id.clone(), *surface_id);
debug_assert!(old.is_none(), "duplicate tab public id");
indexes.tab_ids.insert(*surface_id, tab_id);
cmux-tui/crates/cmux-tui-core/src/mux.rs
+54 -7
View File
@@ -11662,6 +11662,9 @@ impl Mux {
pane.active_tab = pane.tabs.len() - 1;
pane.active_at = active_at;
fence_layout_undo_for_tab_membership(&mut state, &[pane_id]);
if let Some(identity) = surface.resource_identity().cloned() {
state.register_tab_identity(surface.id, &identity);
}
state.surfaces.insert(surface.id, surface.clone());
let delta = (|| {
let (wi, si) = state.screen_of(pane_id)?;
@@ -15136,6 +15139,12 @@ fn insert_surface_checked(state: &mut State, surface: Arc<Surface>) -> anyhow::R
);
}
register_terminal_runtime_checked(state, &surface)?;
// Surface insertion is the only way a tab placement enters live state, so
// it is also where the topology takes ownership of that tab's identity.
// Every later reader takes identity from the topology, never from here.
if let Some(identity) = surface.resource_identity().cloned() {
state.register_tab_identity(surface.id, &identity);
}
state.surfaces.insert(surface.id, surface);
Ok(())
}
@@ -16197,13 +16206,6 @@ fn layout_undo_confirmation_details(
.tab_ids
.get(surface)
.cloned()
.or_else(|| {
state
.surfaces
.get(surface)
.and_then(|surface| surface.resource_identity())
.map(|identity| identity.tab_id.clone())
})
.with_context(|| format!("tab {surface} has no public identity"))?;
update_layout_undo_token_part(&mut hasher, tab_id.to_string().as_bytes());
}
@@ -17183,6 +17185,51 @@ mod tests {
assert!(restored.next_id > 100);
}
#[test]
fn restored_tabs_keep_durable_identity_without_any_live_surface() {
let (snapshot, topology) = resource_restore_fixture();
let mut restored = restore_resource_state(snapshot, topology.clone()).unwrap();
assert!(restored.state.surfaces.is_empty(), "restore must not fabricate runtime");
restored.state.rebuild_resource_indexes();
restored.state.ensure_tab_identity_coverage().unwrap();
for tab in &topology.tabs {
let slot = restored
.state
.resource_indexes
.tabs
.get(&tab.public_id)
.copied()
.expect("restored tab lost its slot");
assert_eq!(
restored.state.resource_indexes.content_ids.get(&slot),
Some(&tab.content_id),
"restored tab lost its content identity"
);
}
}
#[test]
fn a_tab_without_durable_identity_fails_loudly_instead_of_vanishing() {
let (snapshot, topology) = resource_restore_fixture();
let mut restored = restore_resource_state(snapshot, topology).unwrap();
let slot = *restored
.state
.panes
.values()
.find(|pane| !pane.tabs.is_empty())
.expect("fixture has a placed tab")
.tabs
.first()
.expect("checked above");
restored.state.resource_indexes.tab_ids.remove(&slot);
let error = restored.state.ensure_tab_identity_coverage().unwrap_err();
assert!(error.to_string().contains("has no durable identity"), "unexpected error: {error}");
}
#[test]
fn restored_terminal_runtime_materializes_all_durable_views_and_survives_zero_views() {
let (snapshot, mut topology) = resource_restore_fixture();
cmux-tui/crates/cmux-tui-core/src/mux/resource_content.rs
+60 -47
View File
@@ -606,6 +606,7 @@ impl Mux {
// their reverse indexes are populated. Full projection is the
// reconciliation boundary, so rebuild from the live tree first.
state.rebuild_resource_indexes();
state.ensure_tab_identity_coverage()?;
ensure_split_public_ids(state)?;
let terminal_tab_order = ordered_terminal_tab_ids(state)?;
@@ -693,15 +694,10 @@ impl Mux {
.get(&pane_slot)
.with_context(|| format!("screen references missing pane {pane_slot}"))?;
live_panes.insert(pane.public_id.clone());
let active_tab = pane.tabs.get(pane.active_tab).and_then(|surface| {
state.resource_indexes.tab_ids.get(surface).cloned().or_else(|| {
state
.surfaces
.get(surface)
.and_then(|surface| surface.resource_identity())
.map(|identity| identity.tab_id.clone())
})
});
let active_tab = pane
.tabs
.get(pane.active_tab)
.and_then(|slot| state.resource_indexes.tab_ids.get(slot).cloned());
let creation_ordinal =
before_pane_ordinals.get(&pane.public_id).copied().unwrap_or(pane.id);
changes.push(ResourceChange::UpsertPane(RegistryPane {
@@ -762,29 +758,33 @@ impl Mux {
(None, Some(host_id), first_terminal_placement)
}
ContentPublicId::Browser(browser_id) => {
let surface = surface.with_context(|| {
format!("browser tab {surface_slot} has no live surface")
})?;
// A browser view can also outlive its runtime,
// so the durable row is the fallback rather
// than a hard requirement.
live_browsers.insert(browser_id.clone());
let durable = before_browsers.get(browser_id).cloned();
let url = surface
.browser_url()
.or_else(|| {
before_browsers
.get(browser_id)
.map(|browser| browser.url.clone())
})
.and_then(|surface| surface.browser_url())
.or_else(|| durable.as_ref().map(|browser| browser.url.clone()))
.or_else(|| before_tab.and_then(|tab| tab.browser_url.clone()))
.unwrap_or_else(|| "about:blank".to_string());
let (cols, rows) = surface.size();
let live_status = surface.browser_status();
let mut browser =
before_browsers.get(browser_id).cloned().unwrap_or_else(|| {
RegistryBrowser::recreate(
browser_id.clone(),
url.clone(),
cols.max(1),
rows.max(1),
)
});
let (cols, rows) = match surface {
Some(surface) => surface.size(),
None => durable
.as_ref()
.map(|browser| (browser.cols, browser.rows))
.unwrap_or((1, 1)),
};
let live_status =
surface.and_then(|surface| surface.browser_status());
let mut browser = durable.unwrap_or_else(|| {
RegistryBrowser::recreate(
browser_id.clone(),
url.clone(),
cols.max(1),
rows.max(1),
)
});
browser.url = url.clone();
browser.cols = cols.max(1);
browser.rows = rows.max(1);
@@ -794,10 +794,14 @@ impl Mux {
}
Some(BrowserStatus::Live) => RegistryBrowserStatus::Live,
Some(BrowserStatus::Failed(_)) => RegistryBrowserStatus::Failed,
None if surface.is_dead() => RegistryBrowserStatus::Failed,
None if surface.is_some_and(|surface| surface.is_dead()) => {
RegistryBrowserStatus::Failed
}
None => browser.status,
};
if let Some(source) = surface.browser_source() {
if let Some(source) =
surface.and_then(|surface| surface.browser_source())
{
browser.source = match source {
BrowserSource::External => RegistryBrowserSource::External,
BrowserSource::Launched => RegistryBrowserSource::Launched,
@@ -857,15 +861,28 @@ impl Mux {
}
ContentPublicId::Terminal(_) => {}
ContentPublicId::Browser(id) => {
let surface = surface.expect("browser surface validated above");
let (cols, rows) = surface.size();
let status = surface.browser_status();
let durable = before_browsers.get(id);
let (cols, rows) = match surface {
Some(surface) => surface.size(),
None => durable
.map(|browser| (browser.cols, browser.rows))
.unwrap_or((1, 1)),
};
let status = surface.and_then(|surface| surface.browser_status());
let status_name = status
.as_ref()
.map(|status| status.as_str())
.unwrap_or(if surface.is_dead() { "failed" } else { "live" });
.unwrap_or(match surface {
Some(surface) if surface.is_dead() => "failed",
Some(_) => "live",
None => match durable.map(|browser| &browser.status) {
Some(RegistryBrowserStatus::Starting) => "starting",
Some(RegistryBrowserStatus::Live) => "live",
Some(RegistryBrowserStatus::Failed) | None => "failed",
},
});
let source = surface
.browser_source()
.and_then(|surface| surface.browser_source())
.map(|source| source.as_str())
.or_else(|| {
before_browsers.get(id).map(|browser| {
@@ -891,12 +908,13 @@ impl Mux {
"id":id,
"tab_id":tab.public_id,
"url":tab.browser_url,
"title":surface.title(),
"title":surface.map(|surface| surface.title()),
"loading":status_name == "starting",
"source":source,
"status":status_name,
"error":status.and_then(|status| status.error()),
"frames_stalled":surface.browser_frames_stalled()
"frames_stalled":surface
.and_then(|surface| surface.browser_frames_stalled())
.unwrap_or(false),
"size":{
"cols":cols.max(1),
@@ -1061,16 +1079,11 @@ impl Mux {
}
}
/// Durable identity of one pane tab. Restored tabs exist in the topology
/// before their host runtime is adopted, and an unadoptable host never gets a
/// surface at all, so the durable indexes are the authority here. A live
/// surface is only the faster path to the same identity.
/// Durable identity of one pane tab. The topology owns this, written once by
/// `State::register_tab_identity`. Restored tabs exist before their host is
/// adopted and an unadoptable host never gets a surface at all, so identity
/// must never be read back out of live runtime state.
fn tab_resource_identity(state: &State, surface_slot: SurfaceId) -> Option<TabResourceIdentity> {
if let Some(identity) =
state.surfaces.get(&surface_slot).and_then(|surface| surface.resource_identity().cloned())
{
return Some(identity);
}
Some(TabResourceIdentity::new(
state.resource_indexes.tab_ids.get(&surface_slot)?.clone(),
state.resource_indexes.content_ids.get(&surface_slot)?.clone(),