From 0b9ea33ce9b3d8b31f2459b003bb41dcd0f3f83a Mon Sep 17 00:00:00 2001 From: Sam & Claude Date: Thu, 25 Jun 2026 22:38:32 +0200 Subject: [PATCH] fix(tui): make the "All sessions" aggregated view reachable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Once any pane carried a session_id, rebuild_session_list() forced session_filter = Some(first), so the operator could never get back to the aggregated "All sessions" view — Tab only cycled individual sessions. Documented as a known bug in GLASSPANE-TUI-ENHANCEMENTS.md. Model the session cycle as [All, s1, s2, ...]: index 0 is a synthetic "All sessions" entry (filter = None), any other index scopes to sessions[i-1]. Two helpers encode the mapping: - session_count() = sessions.len() + 1 (All is always present) - apply_session_filter() maps session_idx -> filter (0 => None) Behavior changes: - On connect, the operator now lands on "All sessions" (was: the alphabetically-first session). The aggregated view is the more useful default and is always reachable via Tab/BackTab. - The position indicator now shows for any cycle > 1 item, so the "All (1 of 2)" hint appears even with a single session. self.sessions still holds real session ids only (no sentinel string), so the sorted/deduped invariant is unchanged. Tests: - rebuild_session_list_dedupes_and_sorts: updated for the new default + offset mapping (index 1 => s1, index 2 => s2). - all_sessions_view_is_reachable_with_sessions_present: new regression test covering connect-defaults-to-All and the Tab cycle All -> s1 -> s2 -> All. - attention_bar_ignores_other_session_panes: comment corrected (rebuild no longer selects the first session). 19/19 TUI tests pass; fmt + clippy (-D warnings) clean. (Sam & Claude) --- crates/colibri-glasspane-tui/src/main.rs | 124 ++++++++++++++++++----- 1 file changed, 98 insertions(+), 26 deletions(-) diff --git a/crates/colibri-glasspane-tui/src/main.rs b/crates/colibri-glasspane-tui/src/main.rs index bc5b8ec..73b4c9f 100644 --- a/crates/colibri-glasspane-tui/src/main.rs +++ b/crates/colibri-glasspane-tui/src/main.rs @@ -178,6 +178,23 @@ impl App { } } + /// Total items in the session cycle: the synthetic "All sessions" entry + /// plus one per real session id. Always >= 1 — "All" is always present. + fn session_count(&self) -> usize { + self.sessions.len() + 1 + } + + /// Map the current `session_idx` to a filter. Index 0 is the synthetic + /// "All sessions" aggregated view (filter = None); any other index scopes + /// to the session id at `index - 1`. Call after every `session_idx` change. + fn apply_session_filter(&mut self) { + self.session_filter = if self.session_idx == 0 { + None + } else { + self.sessions.get(self.session_idx - 1).cloned() + }; + } + fn rebuild_session_list(&mut self) { let snap = match &self.snapshot { Some(s) => s, @@ -195,17 +212,15 @@ impl App { .collect(); ids.sort(); ids.dedup(); - if ids.is_empty() { - self.sessions.clear(); + self.sessions = ids; + // The cycle is [All, s1, s2, ...] = sessions.len() + 1. Keep the + // previous selection when it still maps; otherwise fall back to "All". + // Default on first connect is "All sessions" (the aggregated view). + let count = self.session_count(); + if self.session_idx >= count { self.session_idx = 0; - self.session_filter = None; - } else { - self.sessions = ids; - if self.session_idx >= self.sessions.len() { - self.session_idx = self.sessions.len().saturating_sub(1); - } - self.session_filter = self.sessions.get(self.session_idx).cloned(); } + self.apply_session_filter(); } async fn refresh(&mut self) { @@ -312,12 +327,12 @@ impl App { Some(sid) => format!("Session: {sid}"), None => "All sessions".to_string(), }; - let session_span = if self.sessions.len() > 1 { + let session_span = if self.session_count() > 1 { Span::styled( format!( "{session_label} ({} of {})", self.session_idx + 1, - self.sessions.len() + self.session_count() ), Style::default().add_modifier(Modifier::BOLD), ) @@ -638,30 +653,24 @@ async fn run(socket_path: PathBuf) -> io::Result<()> { } } KeyCode::Tab | KeyCode::Char('\t') if !app.sessions.is_empty() => { - app.session_idx = (app.session_idx + 1) % app.sessions.len(); - app.session_filter = app.sessions.get(app.session_idx).cloned(); + let count = app.session_count(); + app.session_idx = (app.session_idx + 1) % count; + app.apply_session_filter(); app.table_state.select(Some(0)); app.detail_pane = None; - app.set_status(format!( - "session {}/{}", - app.session_idx + 1, - app.sessions.len() - )); + app.set_status(format!("session {}/{}", app.session_idx + 1, count)); } KeyCode::BackTab if !app.sessions.is_empty() => { + let count = app.session_count(); app.session_idx = if app.session_idx == 0 { - app.sessions.len() - 1 + count - 1 } else { app.session_idx - 1 }; - app.session_filter = app.sessions.get(app.session_idx).cloned(); + app.apply_session_filter(); app.table_state.select(Some(0)); app.detail_pane = None; - app.set_status(format!( - "session {}/{}", - app.session_idx + 1, - app.sessions.len() - )); + app.set_status(format!("session {}/{}", app.session_idx + 1, count)); } KeyCode::Down | KeyCode::Char('j') => { let count = app.filtered_panes().len(); @@ -846,8 +855,71 @@ mod tests { let mut app = App::new(PathBuf::from("/tmp/nonexistent.sock")); app.snapshot = Some(snap); app.rebuild_session_list(); + // Real session ids are sorted + deduped (s1 appeared twice). assert_eq!(app.sessions, vec!["s1", "s2"]); + // Default selection is "All sessions" (index 0) — the aggregated view + // stays reachable. Regression for the "All sessions unreachable" bug. + assert_eq!(app.session_idx, 0); + assert!(app.session_filter.is_none()); + assert_eq!(app.session_count(), 3); // [All, s1, s2] + // Index 1 -> s1, index 2 -> s2. + app.session_idx = 1; + app.apply_session_filter(); assert_eq!(app.session_filter.as_deref(), Some("s1")); + app.session_idx = 2; + app.apply_session_filter(); + assert_eq!(app.session_filter.as_deref(), Some("s2")); + } + + #[test] + fn all_sessions_view_is_reachable_with_sessions_present() { + // Regression for the pre-existing bug documented in + // GLASSPANE-TUI-ENHANCEMENTS.md: once any pane had a session_id, + // rebuild_session_list() forced session_filter = Some(first), making + // the aggregated "All sessions" view unreachable. It must now default + // to All and stay selectable via Tab. + let snap = GlasspaneSnapshot::new( + "osa", + "2026-06-25T12:00:00Z", + vec![ + colibri_glasspane::Pane { + id: "a".into(), + agent: "zot".into(), + state: AgentState::Working, + session_id: Some("s1".into()), + last_event_at: None, + cwd: None, + stalled: false, + }, + colibri_glasspane::Pane { + id: "b".into(), + agent: "zot".into(), + state: AgentState::Working, + session_id: Some("s2".into()), + last_event_at: None, + cwd: None, + stalled: false, + }, + ], + ); + let mut app = App::new(PathBuf::from("/tmp/nonexistent.sock")); + app.snapshot = Some(snap); + app.rebuild_session_list(); + // After connect, the operator lands on the aggregated view. + assert_eq!(app.session_idx, 0); + assert!(app.session_filter.is_none()); + assert_eq!(app.filtered_panes().len(), 2); + // Tab cycles All -> s1 -> s2 -> All. + let count = app.session_count(); + app.session_idx = (app.session_idx + 1) % count; + app.apply_session_filter(); + assert_eq!(app.session_filter.as_deref(), Some("s1")); + app.session_idx = (app.session_idx + 1) % count; + app.apply_session_filter(); + assert_eq!(app.session_filter.as_deref(), Some("s2")); + app.session_idx = (app.session_idx + 1) % count; + app.apply_session_filter(); + assert!(app.session_filter.is_none(), "wrap back to All sessions"); } // ── render tests (TestBackend) ── @@ -1341,7 +1413,7 @@ mod tests { let mut app = App::new(PathBuf::from("/tmp/nonexistent.sock")); app.snapshot = Some(snap); app.rebuild_session_list(); - // Navigate to session s1 (rebuild selects first alphabetically = s1). + // rebuild now defaults to "All sessions"; force the view to session s1. app.session_filter = Some("s1".into()); let text = render_text(&mut app, 80, 24);