fix(daemon): reseed id counter on restore + heal duplicate leaves
Root cause of the multi-focus/multi-search/linked-terminal bug: the in-memory id counter resets to 0 each daemon start, but restore() never advanced it past restored ids. After a restart new_surface_id() re-minted existing ids → the same surface_id appeared twice in a layout tree (rendered as two panels sharing focus, search bar, and output channel — one ends up blank). Session-persistence made restarts routine, surfacing the latent bug. - restore() now reseeds the counter to max(restored id)+1 - ops::dedupe_leaves heals an already-corrupted persisted tree on load Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -72,6 +72,32 @@ pub fn remove_leaf(root: LayoutNode, target: &SurfaceId) -> Option<LayoutNode> {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Drop duplicate leaves, keeping the first (left-to-right) occurrence of each
|
||||||
|
/// surface id; collapses now-single-child splits. Returns None if empty.
|
||||||
|
///
|
||||||
|
/// Heals a tree corrupted by a duplicate surface id (e.g. an id re-minted after
|
||||||
|
/// a daemon restart before the counter fix), which otherwise renders the same
|
||||||
|
/// panel twice and confuses focus/search/output routing keyed by surface id.
|
||||||
|
pub fn dedupe_leaves(root: LayoutNode) -> Option<LayoutNode> {
|
||||||
|
let mut seen = std::collections::HashSet::new();
|
||||||
|
dedupe(root, &mut seen)
|
||||||
|
}
|
||||||
|
fn dedupe(node: LayoutNode, seen: &mut std::collections::HashSet<SurfaceId>) -> Option<LayoutNode> {
|
||||||
|
match node {
|
||||||
|
LayoutNode::Leaf { surface_id } => {
|
||||||
|
if seen.insert(surface_id.clone()) { Some(LayoutNode::Leaf { surface_id }) } else { None }
|
||||||
|
}
|
||||||
|
LayoutNode::Split { orient, children, .. } => {
|
||||||
|
let kept: Vec<LayoutNode> = children.into_iter().filter_map(|c| dedupe(c, seen)).collect();
|
||||||
|
match kept.len() {
|
||||||
|
0 => None,
|
||||||
|
1 => Some(kept.into_iter().next().unwrap()),
|
||||||
|
n => Some(LayoutNode::Split { orient, ratios: even(n), children: kept }),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/// Set ratios on the split node addressed by `path` (child indices from root).
|
/// Set ratios on the split node addressed by `path` (child indices from root).
|
||||||
/// Normalizes to sum 1.0 and clamps each to >= MIN_RATIO. Returns false if the
|
/// Normalizes to sum 1.0 and clamps each to >= MIN_RATIO. Returns false if the
|
||||||
/// path is invalid or the length does not match the node's child count.
|
/// path is invalid or the length does not match the node's child count.
|
||||||
@@ -247,6 +273,27 @@ mod tests {
|
|||||||
assert_eq!(leaves(&after), vec![sid("s_2"), sid("s_1")]);
|
assert_eq!(leaves(&after), vec![sid("s_2"), sid("s_1")]);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn dedupe_removes_duplicate_leaf_keeping_first() {
|
||||||
|
// s_1 appears twice (the production corruption): heal to one occurrence.
|
||||||
|
let root = LayoutNode::Split {
|
||||||
|
orient: Orient::H, ratios: vec![1.0/3.0; 3],
|
||||||
|
children: vec![LayoutNode::leaf(sid("s_0")), LayoutNode::leaf(sid("s_1")), LayoutNode::leaf(sid("s_1"))],
|
||||||
|
};
|
||||||
|
let healed = dedupe_leaves(root).unwrap();
|
||||||
|
assert_eq!(leaves(&healed), vec![sid("s_0"), sid("s_1")]);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn dedupe_clean_tree_is_unchanged() {
|
||||||
|
let root = LayoutNode::Split {
|
||||||
|
orient: Orient::H, ratios: vec![0.5, 0.5],
|
||||||
|
children: vec![LayoutNode::leaf(sid("s_0")), LayoutNode::leaf(sid("s_1"))],
|
||||||
|
};
|
||||||
|
let out = dedupe_leaves(root.clone()).unwrap();
|
||||||
|
assert_eq!(leaves(&out), vec![sid("s_0"), sid("s_1")]);
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn move_onto_self_is_noop() {
|
fn move_onto_self_is_noop() {
|
||||||
let root = LayoutNode::leaf(sid("s_1"));
|
let root = LayoutNode::leaf(sid("s_1"));
|
||||||
|
|||||||
@@ -23,6 +23,12 @@ pub struct Registry {
|
|||||||
states: HashMap<SurfaceId, SurfaceState>,
|
states: HashMap<SurfaceId, SurfaceState>,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Parse the hex numeric suffix of an id (`"s_1f"` → `0x1f`). None if malformed.
|
||||||
|
/// All ids are minted as `format!("{prefix}_{n:x}")`, so the suffix is hex.
|
||||||
|
fn id_num(id: &str) -> Option<u64> {
|
||||||
|
id.rsplit_once('_').and_then(|(_, hex)| u64::from_str_radix(hex, 16).ok())
|
||||||
|
}
|
||||||
|
|
||||||
impl Registry {
|
impl Registry {
|
||||||
pub fn new() -> Self {
|
pub fn new() -> Self {
|
||||||
Self::default()
|
Self::default()
|
||||||
@@ -193,8 +199,29 @@ impl Registry {
|
|||||||
self.by_path.clear();
|
self.by_path.clear();
|
||||||
self.live.clear();
|
self.live.clear();
|
||||||
self.states.clear();
|
self.states.clear();
|
||||||
|
|
||||||
|
// Advance the id counter past every restored id. The in-memory counter
|
||||||
|
// resets to 0 on each daemon start; without this reseed, after a restart
|
||||||
|
// `new_surface_id()` re-mints ids that already exist — producing duplicate
|
||||||
|
// leaves in a workspace tree (same panel rendered twice, focus/search/
|
||||||
|
// output routing keyed by surface id all break) and cross-workspace id
|
||||||
|
// collisions.
|
||||||
|
let mut max_id = 0u64;
|
||||||
|
for gid in self.groups.keys() {
|
||||||
|
if let Some(n) = id_num(&gid.0) { max_id = max_id.max(n + 1); }
|
||||||
|
}
|
||||||
|
for w in &state.workspaces {
|
||||||
|
if let Some(n) = id_num(&w.id.0) { max_id = max_id.max(n + 1); }
|
||||||
|
for sid in w.surfaces.keys() {
|
||||||
|
if let Some(n) = id_num(&sid.0) { max_id = max_id.max(n + 1); }
|
||||||
|
}
|
||||||
|
}
|
||||||
|
self.counter.store(max_id, Ordering::Relaxed);
|
||||||
|
|
||||||
for w in state.workspaces {
|
for w in state.workspaces {
|
||||||
let mut w = w;
|
let mut w = w;
|
||||||
|
// Heal a tree already corrupted by a duplicate leaf (pre-fix state).
|
||||||
|
w.layout = w.layout.take().and_then(spacesh_core::ops::dedupe_leaves);
|
||||||
if let Some(z) = &w.zoomed {
|
if let Some(z) = &w.zoomed {
|
||||||
if !w.surfaces.contains_key(z) { w.zoomed = None; }
|
if !w.surfaces.contains_key(z) { w.zoomed = None; }
|
||||||
}
|
}
|
||||||
@@ -252,6 +279,52 @@ mod tests {
|
|||||||
assert_eq!(w.layout, Some(LN::leaf(s1))); // split collapsed
|
assert_eq!(w.layout, Some(LN::leaf(s1))); // split collapsed
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn restore_advances_counter_past_existing_ids() {
|
||||||
|
// After a daemon restart the counter must not re-mint a restored id.
|
||||||
|
let mut r = Registry::new();
|
||||||
|
let mut surfaces = HashMap::new();
|
||||||
|
surfaces.insert(SurfaceId("s_5".into()), spec());
|
||||||
|
let st = PersistState {
|
||||||
|
version: 1, groups: vec![],
|
||||||
|
workspaces: vec![Workspace {
|
||||||
|
id: WorkspaceId("w_2".into()), path: "/p".into(), name: "p".into(),
|
||||||
|
group_id: None, order: 0, unread: false, pinned: false,
|
||||||
|
layout: Some(LN::leaf(SurfaceId("s_5".into()))), zoomed: None, surfaces,
|
||||||
|
}],
|
||||||
|
};
|
||||||
|
r.restore(st);
|
||||||
|
// max restored id is s_5 (hex 5) → next minted must be s_6, no collision.
|
||||||
|
assert_eq!(r.new_surface_id(), SurfaceId("s_6".into()));
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn restore_heals_duplicate_leaf() {
|
||||||
|
// A persisted tree with s_1 twice (the production corruption) heals to one.
|
||||||
|
let mut r = Registry::new();
|
||||||
|
let mut surfaces = HashMap::new();
|
||||||
|
surfaces.insert(SurfaceId("s_0".into()), spec());
|
||||||
|
surfaces.insert(SurfaceId("s_1".into()), spec());
|
||||||
|
let tree = LN::Split {
|
||||||
|
orient: Orient::H, ratios: vec![1.0 / 3.0; 3],
|
||||||
|
children: vec![LN::leaf(SurfaceId("s_0".into())), LN::leaf(SurfaceId("s_1".into())), LN::leaf(SurfaceId("s_1".into()))],
|
||||||
|
};
|
||||||
|
let st = PersistState {
|
||||||
|
version: 1, groups: vec![],
|
||||||
|
workspaces: vec![Workspace {
|
||||||
|
id: WorkspaceId("w_0".into()), path: "/p".into(), name: "p".into(),
|
||||||
|
group_id: None, order: 0, unread: false, pinned: false,
|
||||||
|
layout: Some(tree), zoomed: None, surfaces,
|
||||||
|
}],
|
||||||
|
};
|
||||||
|
r.restore(st);
|
||||||
|
let w = r.workspace(&WorkspaceId("w_0".into())).unwrap();
|
||||||
|
assert_eq!(
|
||||||
|
spacesh_core::ops::leaves(w.layout.as_ref().unwrap()),
|
||||||
|
vec![SurfaceId("s_0".into()), SurfaceId("s_1".into())]
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn restore_round_trips_through_persist_state() {
|
fn restore_round_trips_through_persist_state() {
|
||||||
let mut r = Registry::new();
|
let mut r = Registry::new();
|
||||||
|
|||||||
Reference in New Issue
Block a user