From 649d3990d61503e3c13cb38c6b251150e35b925a Mon Sep 17 00:00:00 2001 From: G1 Agent Date: Sat, 6 Jun 2026 19:06:38 +0800 Subject: [PATCH 1/3] feat(tui): add multi-tab system core (manager + persistence) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sister PR to #2753, scoped to the narrow tab-core/persistence slice Hmbown asked for in the v0.9 stewardship review. Adds the `tab` module under `crates/tui/src/tui/` and a one-line module registration in `tui/mod.rs`. Nothing else in the host changes here — the switcher / picker / meeting UI pass and the host wiring (`App::tab_manager`, keyboard shortcuts, mouse menu, tab-bar layout in `ui.rs`) live on #2753 and land in a follow-up PR. Scope: * `tab::TabManager` with monotonic `next_tab_id`, max 9 tabs by default, snapshot/restore round-trip, group assignment, cross-tab event/links, and persistence integration * `tab::delegator::TaskDelegator` — bounded pending queue with `MAX_COMPLETED_RESULTS` auto-prune; `take_pending_for_tab` marks InProgress in place and returns a clone so subsequent `start_task` / `complete` / `fail_task` / `cancel_task` can still find the task (the previous `swap_remove` would have dropped it on the first call) * `tab::meeting::MeetingManager` — participants, messages by type (Regular / Question / Answer / Proposal / Agreement / Objection / Summary), decisions * `tab::cross_tab` — `CrossTabEvent` (TaskDelegation / ReviewRequest / MeetingInvite / ContextSync / ResultReturn) and `SharedContext` * `tab::group` — `TabGroup` / `TabGroupManager` / `GroupColor` (Red/Orange/Yellow/Green/Cyan/Blue/Magenta/Gray) * `tab::mention` — `@Tab`, `@N`, `@tab` (case-insensitive) parser, with `resolve_tab_mention` for 1-indexed tab lookups * `tab::persistence` — JSON file in the user's data dir, `PersistedTabState` / `PersistedTab` / `PersistedDelegation` / `PersistedGroup` with schema-version header, atomic save, bounded file size, corruption-tolerant load * `tab::benches` and `tab::key_e2e` regression suites (roundtrip, save/load, end-to-end save→load with keymap) Lint posture: * `#![allow(dead_code, unused_imports)]` on `tab/mod.rs` because the collab/UI pass is not on this branch; the public surface is intentionally exposed for the follow-up wiring in #2753 * One pre-existing `tools/shell.rs` fix piggy-backed: drop the redundant `as *mut c_void` cast on `child.as_raw_handle()` (the return type is already `*mut c_void`). Pre-existing on `codex/v0.9.0-stewardship`; promoted to `clippy::unnecessary_cast` by rust 1.95 Local CI matrix on this branch (Windows runner, same flags as `.github/workflows/ci.yml`): * `cargo fmt --all -- --check` — pass * `cargo clippy --workspace --all-features --locked -- -D warnings` — pass, 0 errors * `cargo test --bin codewhale-tui tab::` — 63/63 pass * `git diff --exit-code -- Cargo.lock` — clean GitHub Actions will run on the standard fork-PR approval gate; the `on.pull_request` branch filter on this repo matches `codex/v0.9.0-stewardship` so the same matrix will run on ubuntu-latest, macos-latest, and windows-latest. Co-Authored-By: Claude Opus 4.7 --- crates/tui/src/tools/shell.rs | 2 +- crates/tui/src/tui/mod.rs | 1 + crates/tui/src/tui/tab/benches.rs | 218 +++++++ crates/tui/src/tui/tab/cross_tab.rs | 189 ++++++ crates/tui/src/tui/tab/delegator.rs | 542 ++++++++++++++++ crates/tui/src/tui/tab/group.rs | 350 ++++++++++ crates/tui/src/tui/tab/key_e2e.rs | 348 ++++++++++ crates/tui/src/tui/tab/manager.rs | 876 ++++++++++++++++++++++++++ crates/tui/src/tui/tab/meeting.rs | 377 +++++++++++ crates/tui/src/tui/tab/mention.rs | 219 +++++++ crates/tui/src/tui/tab/mod.rs | 148 +++++ crates/tui/src/tui/tab/persistence.rs | 375 +++++++++++ 12 files changed, 3644 insertions(+), 1 deletion(-) create mode 100644 crates/tui/src/tui/tab/benches.rs create mode 100644 crates/tui/src/tui/tab/cross_tab.rs create mode 100644 crates/tui/src/tui/tab/delegator.rs create mode 100644 crates/tui/src/tui/tab/group.rs create mode 100644 crates/tui/src/tui/tab/key_e2e.rs create mode 100644 crates/tui/src/tui/tab/manager.rs create mode 100644 crates/tui/src/tui/tab/meeting.rs create mode 100644 crates/tui/src/tui/tab/mention.rs create mode 100644 crates/tui/src/tui/tab/mod.rs create mode 100644 crates/tui/src/tui/tab/persistence.rs diff --git a/crates/tui/src/tools/shell.rs b/crates/tui/src/tools/shell.rs index c8eacca91..278134ec3 100644 --- a/crates/tui/src/tools/shell.rs +++ b/crates/tui/src/tools/shell.rs @@ -271,7 +271,7 @@ impl WindowsJob { ) .map_err(windows_io_error)?; - let process_handle = HANDLE(child.as_raw_handle() as *mut core::ffi::c_void); + let process_handle = HANDLE(child.as_raw_handle()); AssignProcessToJobObject(job.handle, process_handle).map_err(windows_io_error)?; } diff --git a/crates/tui/src/tui/mod.rs b/crates/tui/src/tui/mod.rs index 804b4b73d..823f1df41 100644 --- a/crates/tui/src/tui/mod.rs +++ b/crates/tui/src/tui/mod.rs @@ -62,6 +62,7 @@ pub mod slash_menu; pub mod streaming; pub mod streaming_thinking; mod subagent_routing; +pub mod tab; pub mod theme_picker; mod tool_routing; pub mod transcript; diff --git a/crates/tui/src/tui/tab/benches.rs b/crates/tui/src/tui/tab/benches.rs new file mode 100644 index 000000000..3c63650f6 --- /dev/null +++ b/crates/tui/src/tui/tab/benches.rs @@ -0,0 +1,218 @@ +//! Performance benchmarks for the tab system. +//! +//! These tests are not assertions — they print timing info to stderr and +//! return success. Run with `--nocapture` to see the numbers. +//! +//! Run with: `cargo test tui::tab::benches -- --nocapture --test-threads=1` +//! +//! These benchmarks guard against performance regressions in the +//! critical-path operations of the multi-tab system: +//! +//! - TabManager creation and tab creation (startup overhead) +//! - Tab switching (interactive latency) +//! - Delegation queue operations (background processing) +//! - Persistence save/load (startup + shutdown overhead) +//! - Group color rendering (per-frame work in tab bar) + +#![allow(unused_imports)] + +#[cfg(test)] +mod benches { + use std::time::Instant; + + use crate::tui::tab::{ + Priority, TabId, TabManager, TabType, group::GroupColor, persistence::PersistedTab, + persistence::PersistedTabState, + }; + + /// Helper: print a timing result + fn report(label: &str, dur: std::time::Duration, ops: usize) { + let per_op_ns = if ops > 0 { + dur.as_nanos() / ops as u128 + } else { + 0 + }; + eprintln!( + "[bench] {:50} total={:>8.2?} ops={:>6} per_op={:>7} ns", + label, dur, ops, per_op_ns + ); + } + + #[test] + fn bench_create_tabs() { + let start = Instant::now(); + let mut manager = TabManager::new(); + for i in 0..9 { + manager + .create_tab(format!("Bench Tab {}", i), TabType::Chat) + .expect("create_tab should succeed"); + } + report("create 9 tabs", start.elapsed(), 9); + } + + #[test] + fn bench_switch_tabs() { + let mut manager = TabManager::new(); + for i in 0..9 { + manager + .create_tab(format!("Tab {}", i), TabType::Chat) + .expect("create_tab"); + } + let start = Instant::now(); + for _ in 0..1000 { + manager.switch_to_next(); + } + report("1000 tab switches (next)", start.elapsed(), 1000); + } + + #[test] + fn bench_delegate_many_tasks() { + let mut manager = TabManager::new(); + let from = manager + .create_tab("Source".to_string(), TabType::Chat) + .expect("create"); + let to = manager + .create_tab("Target".to_string(), TabType::Chat) + .expect("create"); + + let start = Instant::now(); + for i in 0..1000 { + manager.delegate_task(from, to, format!("Task {}", i), Priority::Normal); + } + report("1000 delegations", start.elapsed(), 1000); + } + + #[test] + fn bench_take_pending_priority() { + let mut manager = TabManager::new(); + let from = manager.create_tab("S".to_string(), TabType::Chat).unwrap(); + let to = manager.create_tab("T".to_string(), TabType::Chat).unwrap(); + + // Create 100 tasks with mixed priorities + let priorities = [ + Priority::Low, + Priority::Normal, + Priority::High, + Priority::Urgent, + ]; + for i in 0..100 { + manager.delegate_task(from, to, format!("Task {}", i), priorities[i % 4]); + } + + let start = Instant::now(); + let mut count = 0; + while manager.take_next_delegation(to).is_some() { + count += 1; + } + report( + &format!("drain {} priority-sorted tasks", count), + start.elapsed(), + count, + ); + } + + #[test] + fn bench_persistence_roundtrip() { + let mut manager = TabManager::new(); + for i in 0..9 { + manager + .create_tab(format!("Tab {}", i), TabType::Chat) + .expect("create"); + } + + // Add some delegations + let from = manager.active_id().unwrap(); + let to = manager.all_tabs()[1].id; + for i in 0..20 { + manager.delegate_task(from, to, format!("Task {}", i), Priority::Normal); + } + + let start = Instant::now(); + let state = manager.snapshot(); + let snap_dur = start.elapsed(); + + let json = serde_json::to_string(&state).unwrap(); + let ser_dur = start.elapsed() - snap_dur; + + let start2 = Instant::now(); + let _loaded: PersistedTabState = serde_json::from_str(&json).unwrap(); + let de_dur = start2.elapsed(); + + eprintln!( + "[bench] {:50} snap={:>8.2?} ser={:>8.2?} de={:>8.2?} json_size={} bytes", + "9 tabs + 20 delegations persistence", + snap_dur, + ser_dur, + de_dur, + json.len() + ); + } + + #[test] + fn bench_group_operations() { + let mut manager = TabManager::new(); + for i in 0..9 { + manager + .create_tab(format!("Tab {}", i), TabType::Chat) + .expect("create"); + } + let tabs: Vec = manager.all_tabs().iter().map(|t| t.id).collect(); + + // Create 3 groups + let start = Instant::now(); + let g1 = manager.create_group("Frontend".to_string(), GroupColor::Blue); + let g2 = manager.create_group("Backend".to_string(), GroupColor::Red); + let g3 = manager.create_group("Misc".to_string(), GroupColor::Green); + report("create 3 groups", start.elapsed(), 3); + + // Assign all 9 tabs to groups + let start = Instant::now(); + for (i, tab) in tabs.iter().enumerate() { + let group = if i % 3 == 0 { + &g1 + } else if i % 3 == 1 { + &g2 + } else { + &g3 + }; + manager.assign_tab_to_group(*tab, group); + } + report("assign 9 tabs to groups", start.elapsed(), 9); + + // Lookup + let start = Instant::now(); + for tab in &tabs { + let _ = manager.tab_group(*tab); + } + report("9 group lookups", start.elapsed(), 9); + } + + #[test] + fn bench_render_at_widths() { + // Smoke test: ensure rendering at various widths completes quickly + for width in [20, 40, 80, 120, 200] { + let mut manager = TabManager::new(); + for i in 0..9 { + manager + .create_tab(format!("Tab {}", i), TabType::Chat) + .expect("create"); + } + + let start = Instant::now(); + // Simulate the per-frame work the tab bar does: iterate all tabs + // and gather metadata for display. Real rendering also iterates. + let mut count = 0; + for tab in manager.all_tabs() { + // Touch a few fields to simulate display work + let _ = tab.title.len(); + let _ = tab.id.0; + count += 1; + } + report( + &format!("iterate {} tabs at width {}", count, width), + start.elapsed(), + count, + ); + } + } +} diff --git a/crates/tui/src/tui/tab/cross_tab.rs b/crates/tui/src/tui/tab/cross_tab.rs new file mode 100644 index 000000000..8a155a3ef --- /dev/null +++ b/crates/tui/src/tui/tab/cross_tab.rs @@ -0,0 +1,189 @@ +//! Cross-tab collaboration events + +// WIP collaboration surface — narrow harvest. See `tab/mod.rs` for the +// PR #2753 context. +#![allow(dead_code)] + +use super::{Priority, TabId}; +use chrono::{DateTime, Utc}; +use serde::{Deserialize, Serialize}; +use std::collections::HashMap; + +/// A link between two tabs for collaboration +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct CrossTabLink { + pub from: TabId, + pub to: TabId, + pub created_at: DateTime, +} + +/// Cross-tab event types +#[derive(Debug, Clone, Serialize, Deserialize)] +pub enum CrossTabEvent { + /// Task delegation request + TaskDelegation { + task_id: String, + from_tab: TabId, + to_tab: TabId, + description: String, + priority: Priority, + created_at: DateTime, + }, + /// Review request + ReviewRequest { + request_id: String, + from_tab: TabId, + to_tab: TabId, + content_ref: String, + criteria: Vec, + created_at: DateTime, + }, + /// Meeting invitation + MeetingInvite { + meeting_id: String, + from_tab: TabId, + participants: Vec, + topic: String, + created_at: DateTime, + }, + /// Result returned from delegation + ResultReturn { + task_id: String, + from_tab: TabId, + to_tab: TabId, + result: String, + created_at: DateTime, + }, + /// Context sync between tabs + ContextSync { + tab_ids: Vec, + changes: Vec, + created_at: DateTime, + }, +} + +impl CrossTabEvent { + /// Get the sender tab ID + #[allow(clippy::wrong_self_convention)] // getter, not a constructor + pub fn from_tab(&self) -> TabId { + match self { + Self::TaskDelegation { from_tab, .. } => *from_tab, + Self::ReviewRequest { from_tab, .. } => *from_tab, + Self::MeetingInvite { from_tab, .. } => *from_tab, + Self::ResultReturn { from_tab, .. } => *from_tab, + // ContextSync has no single sender; return the first participant + // if any, otherwise a sentinel `TabId(0)`. Callers that need to + // distinguish "no sender" should match on the variant directly. + Self::ContextSync { tab_ids, .. } => tab_ids.first().copied().unwrap_or(TabId(0)), + } + } + + /// Get the target tab ID (if applicable) + pub fn to_tab(&self) -> Option { + match self { + Self::TaskDelegation { to_tab, .. } => Some(*to_tab), + Self::ReviewRequest { to_tab, .. } => Some(*to_tab), + Self::MeetingInvite { participants, .. } => participants.first().copied(), + Self::ResultReturn { to_tab, .. } => Some(*to_tab), + Self::ContextSync { .. } => None, + } + } + + /// Get creation timestamp + pub fn created_at(&self) -> DateTime { + match self { + Self::TaskDelegation { created_at, .. } => *created_at, + Self::ReviewRequest { created_at, .. } => *created_at, + Self::MeetingInvite { created_at, .. } => *created_at, + Self::ResultReturn { created_at, .. } => *created_at, + Self::ContextSync { created_at, .. } => *created_at, + } + } +} + +/// Type of context change +#[derive(Debug, Clone, Serialize, Deserialize)] +pub enum ContextChangeType { + VariableSet(String), + VariableRemoved(String), + MessageAdded, + FileModified(String), + StateUpdate, +} + +/// A change in shared context +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct ContextChange { + pub change_type: ContextChangeType, + pub value: Option, + pub timestamp: DateTime, +} + +impl ContextChange { + pub fn variable_set(name: &str, value: &str) -> Self { + Self { + change_type: ContextChangeType::VariableSet(name.to_string()), + value: Some(value.to_string()), + timestamp: Utc::now(), + } + } + + pub fn file_modified(path: &str) -> Self { + Self { + change_type: ContextChangeType::FileModified(path.to_string()), + value: None, + timestamp: Utc::now(), + } + } +} + +/// Shared context between linked tabs +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct SharedContext { + pub participants: Vec, + pub shared_variables: HashMap, + pub shared_messages: Vec, + pub meeting_notes: Vec, + pub created_at: DateTime, +} + +impl SharedContext { + pub fn new(participants: Vec) -> Self { + Self { + participants, + shared_variables: HashMap::new(), + shared_messages: Vec::new(), + meeting_notes: Vec::new(), + created_at: Utc::now(), + } + } + + pub fn add_variable(&mut self, name: &str, value: &str) { + self.shared_variables + .insert(name.to_string(), value.to_string()); + } + + pub fn get_variable(&self, name: &str) -> Option<&String> { + self.shared_variables.get(name) + } + + pub fn add_message(&mut self, sender: TabId, content: &str) { + self.shared_messages.push(SharedMessage { + sender, + content: content.to_string(), + timestamp: Utc::now(), + }); + } + + pub fn add_meeting_note(&mut self, note: &str) { + self.meeting_notes.push(note.to_string()); + } +} + +/// A message in shared context +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct SharedMessage { + pub sender: TabId, + pub content: String, + pub timestamp: DateTime, +} diff --git a/crates/tui/src/tui/tab/delegator.rs b/crates/tui/src/tui/tab/delegator.rs new file mode 100644 index 000000000..806a55a3a --- /dev/null +++ b/crates/tui/src/tui/tab/delegator.rs @@ -0,0 +1,542 @@ +//! Task delegation system + +// WIP collaboration surface — narrow harvest. See `tab/mod.rs` for the +// PR #2753 context. +#![allow(dead_code)] + +use super::{Priority, TabId}; +use chrono::{DateTime, Utc}; +use serde::{Deserialize, Serialize}; +use std::collections::VecDeque; + +/// Status of a delegation task +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +pub enum DelegationStatus { + Pending, + InProgress, + Completed, + Failed, + Cancelled, +} + +/// A delegation task +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct DelegationTask { + pub task_id: String, + pub from_tab: TabId, + pub to_tab: TabId, + pub description: String, + pub priority: Priority, + pub status: DelegationStatus, + pub created_at: DateTime, + pub deadline: Option>, + pub completed_at: Option>, + pub result: Option, +} + +impl DelegationTask { + /// Create a new pending delegation task. + /// + /// # Arguments + /// * `task_id` - Unique identifier (e.g. "delegation_42") + /// * `from` - Tab that originated the task + /// * `to` - Tab that should execute the task + /// * `description` - Human-readable description of what to do + /// * `priority` - Priority level (Low/Normal/High/Urgent) + pub fn new( + task_id: String, + from: TabId, + to: TabId, + description: String, + priority: Priority, + ) -> Self { + Self { + task_id, + from_tab: from, + to_tab: to, + description, + priority, + status: DelegationStatus::Pending, + created_at: Utc::now(), + deadline: None, + completed_at: None, + result: None, + } + } + + /// Builder method: set the deadline for this task. Returns self for chaining. + pub fn with_deadline(mut self, deadline: DateTime) -> Self { + self.deadline = Some(deadline); + self + } + + /// Transition status to InProgress. Idempotent. + pub fn start(&mut self) { + self.status = DelegationStatus::InProgress; + } + + /// Mark as completed with the given result string. + /// Records completion timestamp. + pub fn complete(&mut self, result: String) { + self.status = DelegationStatus::Completed; + self.result = Some(result); + self.completed_at = Some(Utc::now()); + } + + /// Mark as failed (no result). Records completion timestamp. + pub fn fail(&mut self) { + self.status = DelegationStatus::Failed; + self.completed_at = Some(Utc::now()); + } + + /// Cancel the task. Records completion timestamp. + pub fn cancel(&mut self) { + self.status = DelegationStatus::Cancelled; + self.completed_at = Some(Utc::now()); + } + + /// Returns true if the task is still pending (not yet started). + pub fn is_pending(&self) -> bool { + self.status == DelegationStatus::Pending + } + + /// Returns true if the task completed successfully. + pub fn is_completed(&self) -> bool { + self.status == DelegationStatus::Completed + } + + /// Returns true if the task is pending or in progress (i.e., not terminal). + pub fn is_active(&self) -> bool { + matches!( + self.status, + DelegationStatus::Pending | DelegationStatus::InProgress + ) + } +} + +/// Result of a completed delegation +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct DelegationResult { + pub task_id: String, + pub from_tab: TabId, + pub to_tab: TabId, + pub result: String, + pub completed_at: DateTime, + pub was_successful: bool, +} + +/// Task delegator managing cross-tab task distribution +pub struct TaskDelegator { + /// Active tasks (pending + in-progress). Terminal-state tasks + /// (completed / failed / cancelled) are removed from this vec and + /// recorded in `completed_results`. `pub(crate)` so the persistence + /// layer can restore from snapshot. + pub(crate) pending_tasks: Vec, + /// Bounded ring buffer of completed results. + /// Using VecDeque so O(1) front removal when pruning old entries. + /// Bounded to MAX_COMPLETED_RESULTS to prevent unbounded growth. + completed_results: VecDeque, + next_id: u64, +} + +/// Maximum number of completed results to keep in memory. +/// At this size, prune_results is a no-op for the common case. +const MAX_COMPLETED_RESULTS: usize = 256; + +impl TaskDelegator { + pub fn new() -> Self { + Self { + pending_tasks: Vec::new(), + completed_results: VecDeque::new(), + next_id: 1, + } + } + + /// Create a new delegation + pub fn create_delegation( + &mut self, + from: TabId, + to: TabId, + description: String, + priority: Priority, + ) -> Option { + let task_id = self.generate_task_id(); + let task = DelegationTask::new(task_id.clone(), from, to, description, priority); + self.pending_tasks.push(task); + Some(task_id) + } + + /// Create a delegation with a deadline + pub fn create_delegation_with_deadline( + &mut self, + from: TabId, + to: TabId, + description: String, + priority: Priority, + deadline: DateTime, + ) -> Option { + let task_id = self.generate_task_id(); + let task = DelegationTask::new(task_id.clone(), from, to, description, priority) + .with_deadline(deadline); + self.pending_tasks.push(task); + Some(task_id) + } + + /// Get pending tasks for a tab + pub fn pending_for_tab(&self, tab_id: TabId) -> Vec<&DelegationTask> { + self.pending_tasks + .iter() + .filter(|t| t.to_tab == tab_id && t.is_pending()) + .collect() + } + + /// Get active tasks for a tab (pending or in progress) + pub fn active_for_tab(&self, tab_id: TabId) -> Vec<&DelegationTask> { + self.pending_tasks + .iter() + .filter(|t| t.to_tab == tab_id && t.is_active()) + .collect() + } + + /// Get all pending tasks + pub fn all_pending(&self) -> &[DelegationTask] { + &self.pending_tasks + } + + /// Get pending tasks sorted by priority (highest first) + pub fn pending_sorted_by_priority(&self) -> Vec<&DelegationTask> { + let mut tasks: Vec<&DelegationTask> = self + .pending_tasks + .iter() + .filter(|t| t.is_pending()) + .collect(); + tasks.sort_by_key(|t| std::cmp::Reverse(t.priority)); + tasks + } + + /// Start working on a task + pub fn start_task(&mut self, task_id: &str) -> bool { + if let Some(task) = self.pending_tasks.iter_mut().find(|t| t.task_id == task_id) { + task.start(); + true + } else { + false + } + } + + /// Take the highest-priority pending task for a tab. + /// Marks the task as `InProgress` in place and returns a clone; the task + /// is only removed from the queue when it reaches a terminal state via + /// `complete` / `fail_task` / `cancel_task`. Higher priority wins; on tie, + /// earlier `created_at` wins. + pub fn take_pending_for_tab(&mut self, tab_id: TabId) -> Option { + // Find the highest priority pending task for this tab + let mut best_idx: Option = None; + for (i, task) in self.pending_tasks.iter().enumerate() { + if task.to_tab != tab_id || !task.is_pending() { + continue; + } + match best_idx { + None => best_idx = Some(i), + Some(b) => { + let best = &self.pending_tasks[b]; + // Higher priority wins; if equal, earlier created_at wins + if task.priority > best.priority + || (task.priority == best.priority && task.created_at < best.created_at) + { + best_idx = Some(i); + } + } + } + } + + // Mark as in-progress in place and return a clone; do NOT remove. + best_idx.map(|i| { + self.pending_tasks[i].start(); + self.pending_tasks[i].clone() + }) + } + + /// Peek at the next pending task for a tab without removing it + pub fn peek_pending_for_tab(&self, tab_id: TabId) -> Option<&DelegationTask> { + self.pending_tasks + .iter() + .filter(|t| t.to_tab == tab_id && t.is_pending()) + .max_by(|a, b| { + // Higher priority first; on tie, earlier created_at first + a.priority + .cmp(&b.priority) + .then_with(|| b.created_at.cmp(&a.created_at)) + }) + } + + /// Complete a task + pub fn complete(&mut self, task_id: &str, result: String) { + let pos = self.pending_tasks.iter().position(|t| t.task_id == task_id); + let Some(pos) = pos else { return }; + let mut task = self.pending_tasks.swap_remove(pos); + let from = task.from_tab; + let to = task.to_tab; + task.complete(result.clone()); + + self.completed_results.push_back(DelegationResult { + task_id: task_id.to_string(), + from_tab: from, + to_tab: to, + result, + completed_at: Utc::now(), + was_successful: true, + }); + // Auto-prune to bound memory + if self.completed_results.len() > MAX_COMPLETED_RESULTS { + self.completed_results.pop_front(); + } + } + + /// Fail a task + pub fn fail_task(&mut self, task_id: &str) { + let pos = self.pending_tasks.iter().position(|t| t.task_id == task_id); + let Some(pos) = pos else { return }; + let mut task = self.pending_tasks.swap_remove(pos); + let from = task.from_tab; + let to = task.to_tab; + task.fail(); + + self.completed_results.push_back(DelegationResult { + task_id: task_id.to_string(), + from_tab: from, + to_tab: to, + result: String::new(), + completed_at: Utc::now(), + was_successful: false, + }); + // Auto-prune to bound memory + if self.completed_results.len() > MAX_COMPLETED_RESULTS { + self.completed_results.pop_front(); + } + } + + /// Cancel a task + pub fn cancel_task(&mut self, task_id: &str) -> bool { + let Some(pos) = self.pending_tasks.iter().position(|t| t.task_id == task_id) else { + return false; + }; + let mut task = self.pending_tasks.swap_remove(pos); + task.cancel(); + true + } + + /// Get results for a tab + pub fn results_for_tab(&self, tab_id: TabId) -> Vec<&DelegationResult> { + self.completed_results + .iter() + .filter(|r| r.to_tab == tab_id) + .collect() + } + + /// Get pending count for a tab + pub fn pending_count(&self, tab_id: TabId) -> usize { + self.pending_tasks + .iter() + .filter(|t| t.to_tab == tab_id && t.is_pending()) + .count() + } + + /// Clean up old completed results (keep last N) + /// O(N) where N is the number of items to remove, but much faster than + /// the previous drain() implementation because VecDeque supports + /// O(1) front removal. + pub fn prune_results(&mut self, keep_last: usize) { + while self.completed_results.len() > keep_last { + self.completed_results.pop_front(); + } + } + + /// Get completed results sorted by completion time (most recent first) + pub fn recent_results(&self, limit: usize) -> Vec<&DelegationResult> { + let mut results: Vec<&DelegationResult> = self.completed_results.iter().collect(); + results.sort_by_key(|r| std::cmp::Reverse(r.completed_at)); + results.into_iter().take(limit).collect() + } + + fn generate_task_id(&mut self) -> String { + let id = self.next_id; + self.next_id += 1; + format!("delegation_{}", id) + } +} + +impl Default for TaskDelegator { + fn default() -> Self { + Self::new() + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_create_and_complete_delegation() { + let mut delegator = TaskDelegator::new(); + let from = TabId::new(1); + let to = TabId::new(2); + + let task_id = delegator + .create_delegation(from, to, "Fix the bug".to_string(), Priority::High) + .unwrap(); + + assert_eq!(delegator.pending_count(to), 1); + + delegator.complete(&task_id, "Fixed successfully".to_string()); + + let results = delegator.results_for_tab(from); + assert!(results.is_empty()); + + let results_to = delegator.results_for_tab(to); + assert_eq!(results_to.len(), 1); + assert_eq!(results_to[0].was_successful, true); + } + + #[test] + fn test_priority_ordering() { + let mut delegator = TaskDelegator::new(); + let from = TabId::new(1); + let to = TabId::new(2); + + delegator.create_delegation(from, to, "Low priority".to_string(), Priority::Low); + delegator.create_delegation(from, to, "Urgent".to_string(), Priority::Urgent); + delegator.create_delegation(from, to, "Normal".to_string(), Priority::Normal); + delegator.create_delegation(from, to, "High".to_string(), Priority::High); + + let sorted = delegator.pending_sorted_by_priority(); + assert_eq!(sorted[0].description, "Urgent"); + assert_eq!(sorted[1].description, "High"); + assert_eq!(sorted[2].description, "Normal"); + assert_eq!(sorted[3].description, "Low priority"); + } + + #[test] + fn test_take_pending_priority_order() { + let mut delegator = TaskDelegator::new(); + let from = TabId::new(1); + let to = TabId::new(2); + + delegator.create_delegation(from, to, "Low task".to_string(), Priority::Low); + delegator.create_delegation(from, to, "Urgent task".to_string(), Priority::Urgent); + delegator.create_delegation(from, to, "Normal task".to_string(), Priority::Normal); + + // Should return Urgent first + let task = delegator.take_pending_for_tab(to).unwrap(); + assert_eq!(task.description, "Urgent task"); + assert_eq!(task.priority, Priority::Urgent); + + // Then Normal + let task = delegator.take_pending_for_tab(to).unwrap(); + assert_eq!(task.description, "Normal task"); + assert_eq!(task.priority, Priority::Normal); + + // Then Low + let task = delegator.take_pending_for_tab(to).unwrap(); + assert_eq!(task.description, "Low task"); + assert_eq!(task.priority, Priority::Low); + + // Then nothing + assert!(delegator.take_pending_for_tab(to).is_none()); + } + + #[test] + fn test_take_pending_filters_by_tab() { + let mut delegator = TaskDelegator::new(); + let from = TabId::new(1); + let to_a = TabId::new(2); + let to_b = TabId::new(3); + + delegator.create_delegation(from, to_a, "For A".to_string(), Priority::High); + delegator.create_delegation(from, to_b, "For B".to_string(), Priority::High); + + let task = delegator.take_pending_for_tab(to_a).unwrap(); + assert_eq!(task.description, "For A"); + + let task = delegator.take_pending_for_tab(to_b).unwrap(); + assert_eq!(task.description, "For B"); + + // Both should be drained now + assert!(delegator.take_pending_for_tab(to_a).is_none()); + assert!(delegator.take_pending_for_tab(to_b).is_none()); + } + + #[test] + fn test_peek_pending_does_not_remove() { + let mut delegator = TaskDelegator::new(); + let from = TabId::new(1); + let to = TabId::new(2); + + delegator.create_delegation(from, to, "Task".to_string(), Priority::High); + + // Peek multiple times + assert!(delegator.peek_pending_for_tab(to).is_some()); + assert!(delegator.peek_pending_for_tab(to).is_some()); + assert_eq!(delegator.pending_count(to), 1); + + // Take should still work + let task = delegator.take_pending_for_tab(to).unwrap(); + assert_eq!(task.description, "Task"); + + // Now should be empty + assert!(delegator.peek_pending_for_tab(to).is_none()); + } + + #[test] + fn test_auto_prune_bounded_results() { + // Verify auto-prune keeps the queue bounded under heavy load. + let mut delegator = TaskDelegator::new(); + let from = TabId::new(1); + let to = TabId::new(2); + + // Create and complete 1000 tasks (more than MAX_COMPLETED_RESULTS=256) + for i in 0..1000 { + let task_id = delegator + .create_delegation(from, to, format!("Task {}", i), Priority::Normal) + .unwrap(); + delegator.complete(&task_id, format!("Result {}", i)); + } + + // Should be bounded at MAX_COMPLETED_RESULTS + let results = delegator.results_for_tab(to); + assert!( + results.len() <= 256, + "Results should be bounded, got {}", + results.len() + ); + } + + #[test] + fn test_prune_results_o1() { + let mut delegator = TaskDelegator::new(); + let from = TabId::new(1); + let to = TabId::new(2); + + // Complete many tasks + for i in 0..100 { + let task_id = delegator + .create_delegation(from, to, format!("Task {}", i), Priority::Normal) + .unwrap(); + delegator.complete(&task_id, format!("Result {}", i)); + } + + assert_eq!(delegator.results_for_tab(to).len(), 100); + + // Prune to keep only 5 + delegator.prune_results(5); + assert_eq!(delegator.results_for_tab(to).len(), 5); + + // Pruning further works + delegator.prune_results(3); + assert_eq!(delegator.results_for_tab(to).len(), 3); + + // Pruning to a larger count is a no-op + delegator.prune_results(10); + assert_eq!(delegator.results_for_tab(to).len(), 3); + } +} diff --git a/crates/tui/src/tui/tab/group.rs b/crates/tui/src/tui/tab/group.rs new file mode 100644 index 000000000..715214d88 --- /dev/null +++ b/crates/tui/src/tui/tab/group.rs @@ -0,0 +1,350 @@ +//! Tab groups for organizing related tabs +//! +//! A TabGroup is a named collection of tabs (e.g. "Frontend Refactor", +//! "Backend Bug Hunt"). Groups help users manage 9 tabs by clustering +//! them by project/topic. +//! +//! Groups are purely organizational - they don't change delegation, +//! meeting, or any other tab behavior. They just provide: +//! - Visual separation in the tab bar (color/icon) +//! - Filtering in the tab switcher +//! - Quick group switching (next/prev group) + +// WIP collaboration surface — narrow harvest. See `tab/mod.rs` for the +// PR #2753 context. +#![allow(dead_code)] + +use chrono::{DateTime, Utc}; +use serde::{Deserialize, Serialize}; +use std::collections::HashMap; + +use super::TabId; + +/// Visual style/color identifier for a group +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize, Default)] +pub enum GroupColor { + Red, + Orange, + Yellow, + Green, + Cyan, + #[default] + Blue, + Magenta, + Gray, +} + +impl GroupColor { + /// Short name for the color (1-3 chars) + pub fn short(&self) -> &'static str { + match self { + GroupColor::Red => "Rd", + GroupColor::Orange => "Or", + GroupColor::Yellow => "Yl", + GroupColor::Green => "Gn", + GroupColor::Cyan => "Cy", + GroupColor::Blue => "Bl", + GroupColor::Magenta => "Mg", + GroupColor::Gray => "Gy", + } + } + + /// Cycle to the next color (used by the group cycle command) + pub fn next(&self) -> Self { + match self { + GroupColor::Red => GroupColor::Orange, + GroupColor::Orange => GroupColor::Yellow, + GroupColor::Yellow => GroupColor::Green, + GroupColor::Green => GroupColor::Cyan, + GroupColor::Cyan => GroupColor::Blue, + GroupColor::Blue => GroupColor::Magenta, + GroupColor::Magenta => GroupColor::Gray, + GroupColor::Gray => GroupColor::Red, + } + } +} + +/// A named collection of tabs +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct TabGroup { + pub id: String, + pub name: String, + pub color: GroupColor, + pub tab_ids: Vec, + pub created_at: DateTime, +} + +impl TabGroup { + pub fn new(name: String, color: GroupColor) -> Self { + let id = format!("group_{}", chrono::Utc::now().timestamp_millis()); + Self { + id, + name, + color, + tab_ids: Vec::new(), + created_at: Utc::now(), + } + } + + /// Add a tab to this group + pub fn add_tab(&mut self, tab_id: TabId) { + if !self.tab_ids.contains(&tab_id) { + self.tab_ids.push(tab_id); + } + } + + /// Remove a tab from this group + pub fn remove_tab(&mut self, tab_id: TabId) -> bool { + if let Some(pos) = self.tab_ids.iter().position(|t| *t == tab_id) { + self.tab_ids.swap_remove(pos); + true + } else { + false + } + } + + /// Number of tabs in this group + pub fn len(&self) -> usize { + self.tab_ids.len() + } + + /// Check if the group is empty + pub fn is_empty(&self) -> bool { + self.tab_ids.is_empty() + } + + /// Check if this group contains a tab + pub fn contains(&self, tab_id: TabId) -> bool { + self.tab_ids.contains(&tab_id) + } +} + +/// Manager for tab groups +pub struct TabGroupManager { + pub(crate) groups: HashMap, + /// Maps tab_id -> group_id for quick lookup + pub(crate) tab_to_group: HashMap, + next_id: u64, +} + +impl TabGroupManager { + pub fn new() -> Self { + Self { + groups: HashMap::new(), + tab_to_group: HashMap::new(), + next_id: 1, + } + } + + /// Create a new group + pub fn create_group(&mut self, name: String, color: GroupColor) -> String { + let id = self.generate_group_id(); + let group = TabGroup { + id: id.clone(), + name, + color, + tab_ids: Vec::new(), + created_at: Utc::now(), + }; + self.groups.insert(id.clone(), group); + id + } + + /// Delete a group (tabs themselves are not deleted) + pub fn delete_group(&mut self, group_id: &str) -> bool { + if let Some(group) = self.groups.remove(group_id) { + for tab_id in &group.tab_ids { + self.tab_to_group.remove(tab_id); + } + true + } else { + false + } + } + + /// Assign a tab to a group + pub fn assign_tab(&mut self, tab_id: TabId, group_id: &str) -> bool { + // Remove from any previous group first + self.unassign_tab(tab_id); + + if let Some(group) = self.groups.get_mut(group_id) { + group.add_tab(tab_id); + self.tab_to_group.insert(tab_id, group_id.to_string()); + true + } else { + false + } + } + + /// Remove a tab from its group (if any) + pub fn unassign_tab(&mut self, tab_id: TabId) { + if let Some(prev_group_id) = self.tab_to_group.remove(&tab_id) + && let Some(group) = self.groups.get_mut(&prev_group_id) + { + group.remove_tab(tab_id); + } + } + + /// Get the group a tab is assigned to + pub fn group_of(&self, tab_id: TabId) -> Option<&TabGroup> { + self.tab_to_group + .get(&tab_id) + .and_then(|id| self.groups.get(id)) + } + + /// Get all groups + pub fn all_groups(&self) -> Vec<&TabGroup> { + let mut groups: Vec<&TabGroup> = self.groups.values().collect(); + // Sort by name for stable display + groups.sort_by(|a, b| a.name.cmp(&b.name)); + groups + } + + /// Get tabs in a specific group + pub fn tabs_in_group(&self, group_id: &str) -> Option<&Vec> { + self.groups.get(group_id).map(|g| &g.tab_ids) + } + + /// Get the number of groups + pub fn group_count(&self) -> usize { + self.groups.len() + } + + /// Iterate over groups + pub fn iter(&self) -> impl Iterator { + self.groups.iter() + } + + /// Cycle a tab to the next group (or unassign if at the end) + pub fn cycle_tab_group(&mut self, tab_id: TabId) { + let group_ids: Vec = self.all_groups().iter().map(|g| g.id.clone()).collect(); + + if let Some(current_group_id) = self.tab_to_group.get(&tab_id).cloned() { + if let Some(pos) = group_ids.iter().position(|id| id == ¤t_group_id) { + if pos + 1 < group_ids.len() { + let next = group_ids[pos + 1].clone(); + self.assign_tab(tab_id, &next); + } else { + self.unassign_tab(tab_id); + } + } + } else if !group_ids.is_empty() { + let first = group_ids[0].clone(); + self.assign_tab(tab_id, &first); + } + } + + fn generate_group_id(&mut self) -> String { + let id = self.next_id; + self.next_id += 1; + format!("group_{}", id) + } +} + +impl Default for TabGroupManager { + fn default() -> Self { + Self::new() + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_create_and_delete_group() { + let mut mgr = TabGroupManager::new(); + let id = mgr.create_group("Frontend".to_string(), GroupColor::Blue); + assert_eq!(mgr.group_count(), 1); + assert!(mgr.delete_group(&id)); + assert_eq!(mgr.group_count(), 0); + } + + #[test] + fn test_assign_tab() { + let mut mgr = TabGroupManager::new(); + let group_id = mgr.create_group("Backend".to_string(), GroupColor::Green); + let tab1 = TabId::new(1); + let tab2 = TabId::new(2); + + assert!(mgr.assign_tab(tab1, &group_id)); + assert!(mgr.assign_tab(tab2, &group_id)); + + let group = mgr.group_of(tab1).unwrap(); + assert_eq!(group.len(), 2); + assert!(group.contains(tab1)); + } + + #[test] + fn test_unassign_tab() { + let mut mgr = TabGroupManager::new(); + let group_id = mgr.create_group("Test".to_string(), GroupColor::Red); + let tab1 = TabId::new(1); + mgr.assign_tab(tab1, &group_id); + + mgr.unassign_tab(tab1); + assert!(mgr.group_of(tab1).is_none()); + + let group = mgr.groups.get(&group_id).unwrap(); + assert_eq!(group.len(), 0); + } + + #[test] + fn test_reassign_tab() { + let mut mgr = TabGroupManager::new(); + let g1 = mgr.create_group("G1".to_string(), GroupColor::Blue); + let g2 = mgr.create_group("G2".to_string(), GroupColor::Red); + let tab1 = TabId::new(1); + + mgr.assign_tab(tab1, &g1); + assert_eq!(mgr.group_of(tab1).unwrap().id, g1); + + mgr.assign_tab(tab1, &g2); + assert_eq!(mgr.group_of(tab1).unwrap().id, g2); + // G1 should now be empty + let g1_ref = mgr.groups.get(&g1).unwrap(); + assert_eq!(g1_ref.len(), 0); + } + + #[test] + fn test_color_cycle() { + let c = GroupColor::Red; + assert_eq!(c.next(), GroupColor::Orange); + assert_eq!(c.next().next(), GroupColor::Yellow); + // Cycle back to Red after Gray + let gray = GroupColor::Gray; + assert_eq!(gray.next(), GroupColor::Red); + } + + #[test] + fn test_cycle_tab_group() { + let mut mgr = TabGroupManager::new(); + let g1 = mgr.create_group("G1".to_string(), GroupColor::Blue); + let g2 = mgr.create_group("G2".to_string(), GroupColor::Red); + let tab1 = TabId::new(1); + + // Not assigned yet -> assign to first + mgr.cycle_tab_group(tab1); + assert_eq!(mgr.group_of(tab1).unwrap().id, g1); + + // Cycle to g2 + mgr.cycle_tab_group(tab1); + assert_eq!(mgr.group_of(tab1).unwrap().id, g2); + + // Cycle past end -> unassign + mgr.cycle_tab_group(tab1); + assert!(mgr.group_of(tab1).is_none()); + } + + #[test] + fn test_delete_group_clears_assignments() { + let mut mgr = TabGroupManager::new(); + let g1 = mgr.create_group("G1".to_string(), GroupColor::Blue); + let tab1 = TabId::new(1); + mgr.assign_tab(tab1, &g1); + + mgr.delete_group(&g1); + assert!(mgr.group_of(tab1).is_none()); + assert!(mgr.tab_to_group.get(&tab1).is_none()); + } +} diff --git a/crates/tui/src/tui/tab/key_e2e.rs b/crates/tui/src/tui/tab/key_e2e.rs new file mode 100644 index 000000000..dbd9098d2 --- /dev/null +++ b/crates/tui/src/tui/tab/key_e2e.rs @@ -0,0 +1,348 @@ +//! End-to-end keyboard event tests for tab system +//! +//! These tests simulate the keyboard event handling that happens in +//! `ui.rs` when the user presses tab-related shortcuts. They verify that +//! the TabManager state transitions correctly in response to key events. +//! +//! The actual key event dispatch lives in `ui.rs` (which is hard to +//! test in isolation due to the engine/App dependencies), so these +//! tests exercise the underlying state transitions that the key handlers +//! would trigger. +//! +//! Run with: `cargo test tui::tab::key_e2e -- --nocapture` + +#[cfg(test)] +mod tests { + use crate::tui::tab::{Priority, TabId, TabManager, TabType}; + + /// Simulate the sequence of key events the user would press + /// to: create a new tab, switch to it, type a message, and submit. + fn simulate_create_and_switch(manager: &mut TabManager) { + // Ctrl+Shift+N: create new tab + manager + .create_tab(format!("Tab {}", manager.len() + 1), TabType::Chat) + .expect("Ctrl+Shift+N should create tab"); + + // The new tab is automatically active after creation, + // simulating the key handler that updates active_tab. + } + + /// Simulate Ctrl+1..9 key press + fn simulate_ctrl_number(manager: &mut TabManager, n: u8) { + if n == 0 || n as usize > manager.len() { + return; + } + manager.switch_to((n - 1) as usize); + } + + /// Simulate Ctrl+Tab / Ctrl+Shift+Tab + fn simulate_ctrl_tab(manager: &mut TabManager, forward: bool) { + if forward { + manager.switch_to_next(); + } else { + manager.switch_to_prev(); + } + } + + /// Simulate Ctrl+` to open switcher (we just verify the manager + /// can list its tabs as the switcher would) + fn simulate_switcher_list(manager: &TabManager) -> Vec<(usize, String)> { + manager + .all_tabs() + .iter() + .enumerate() + .map(|(i, t)| (i, t.title.clone())) + .collect() + } + + /// Simulate Ctrl+Shift+D: process pending delegations + fn simulate_process_delegation(manager: &mut TabManager) -> Option { + let tab_id = manager.active_id()?; + manager.take_next_delegation(tab_id).map(|t| t.task_id) + } + + // === Tab creation tests === + + #[test] + fn test_e2e_create_first_tab() { + let mut manager = TabManager::new(); + assert!(manager.is_empty()); + + simulate_create_and_switch(&mut manager); + assert_eq!(manager.len(), 1); + assert_eq!(manager.active_index(), Some(0)); + } + + #[test] + fn test_e2e_create_max_tabs() { + let mut manager = TabManager::new(); + for _ in 0..9 { + simulate_create_and_switch(&mut manager); + } + assert_eq!(manager.len(), 9); + + // 10th should fail + let result = manager.create_tab("10th".to_string(), TabType::Chat); + assert!(result.is_none()); + } + + // === Tab switching tests === + + #[test] + fn test_e2e_ctrl_number_switches() { + let mut manager = TabManager::new(); + for i in 1..=5 { + manager + .create_tab(format!("Tab {}", i), TabType::Chat) + .expect("create"); + } + + simulate_ctrl_number(&mut manager, 3); + assert_eq!(manager.active_index(), Some(2)); + + simulate_ctrl_number(&mut manager, 1); + assert_eq!(manager.active_index(), Some(0)); + + simulate_ctrl_number(&mut manager, 5); + assert_eq!(manager.active_index(), Some(4)); + } + + #[test] + fn test_e2e_ctrl_number_out_of_range() { + let mut manager = TabManager::new(); + for i in 1..=3 { + manager + .create_tab(format!("Tab {}", i), TabType::Chat) + .unwrap(); + } + + // Out-of-range should be a no-op + simulate_ctrl_number(&mut manager, 9); + assert_eq!(manager.active_index(), Some(2)); // last created + } + + #[test] + fn test_e2e_ctrl_tab_cycles() { + let mut manager = TabManager::new(); + for i in 1..=3 { + manager + .create_tab(format!("Tab {}", i), TabType::Chat) + .unwrap(); + } + // Initially at last (2) + assert_eq!(manager.active_index(), Some(2)); + + simulate_ctrl_tab(&mut manager, true); + assert_eq!(manager.active_index(), Some(0)); // wrap + + simulate_ctrl_tab(&mut manager, true); + assert_eq!(manager.active_index(), Some(1)); + + simulate_ctrl_tab(&mut manager, false); + assert_eq!(manager.active_index(), Some(0)); + } + + // === Switcher listing tests === + + #[test] + fn test_e2e_switcher_lists_all_tabs() { + let mut manager = TabManager::new(); + manager.create_tab("A".to_string(), TabType::Chat).unwrap(); + manager + .create_tab("B".to_string(), TabType::Review) + .unwrap(); + manager + .create_tab("C".to_string(), TabType::Meeting) + .unwrap(); + + let listed = simulate_switcher_list(&manager); + assert_eq!(listed.len(), 3); + assert_eq!(listed[0].1, "A"); + assert_eq!(listed[1].1, "B"); + assert_eq!(listed[2].1, "C"); + } + + // === Delegation tests === + + #[test] + fn test_e2e_delegate_and_process() { + let mut manager = TabManager::new(); + let from = manager + .create_tab("Source".to_string(), TabType::Chat) + .unwrap(); + let to = manager + .create_tab("Target".to_string(), TabType::Chat) + .unwrap(); + + // User delegates a task + let task_id = manager + .delegate_task(from, to, "Review PR".to_string(), Priority::High) + .expect("delegate"); + + // User switches to target tab + manager.switch_to_by_id(to); + + // User presses Ctrl+Shift+D + let processed = simulate_process_delegation(&mut manager); + assert_eq!(processed, Some(task_id)); + } + + #[test] + fn test_e2e_no_pending_delegation_returns_none() { + let mut manager = TabManager::new(); + let _ = manager + .create_tab("Solo".to_string(), TabType::Chat) + .unwrap(); + + let processed = simulate_process_delegation(&mut manager); + assert_eq!(processed, None); + } + + #[test] + fn test_e2e_delegation_priority_drain() { + let mut manager = TabManager::new(); + let from = manager.create_tab("S".to_string(), TabType::Chat).unwrap(); + let to = manager.create_tab("T".to_string(), TabType::Chat).unwrap(); + + manager.delegate_task(from, to, "Low".to_string(), Priority::Low); + manager.delegate_task(from, to, "Urgent".to_string(), Priority::Urgent); + manager.delegate_task(from, to, "High".to_string(), Priority::High); + manager.delegate_task(from, to, "Normal".to_string(), Priority::Normal); + + manager.switch_to_by_id(to); + + // Press Ctrl+Shift+D 4 times + assert_eq!(simulate_process_delegation(&mut manager).is_some(), true); + assert_eq!(simulate_process_delegation(&mut manager).is_some(), true); + assert_eq!(simulate_process_delegation(&mut manager).is_some(), true); + // 4th should be the last task + assert_eq!(simulate_process_delegation(&mut manager).is_some(), true); + // 5th should be none + assert_eq!(simulate_process_delegation(&mut manager), None); + } + + // === Tab close tests === + + #[test] + fn test_e2e_close_active_tab() { + let mut manager = TabManager::new(); + let id_a = manager.create_tab("A".to_string(), TabType::Chat).unwrap(); + let _id_b = manager.create_tab("B".to_string(), TabType::Chat).unwrap(); + let id_c = manager.create_tab("C".to_string(), TabType::Chat).unwrap(); + + // Switch to B (index 1) + manager.switch_to(1); + + // Ctrl+Shift+W: close current + manager.close_tab(1); + assert_eq!(manager.len(), 2); + // Active should now be C (index 1) since B was removed. + // C is the previously-created 3rd tab. + assert_eq!(manager.active_id().unwrap(), id_c); + assert!(id_a != id_c); + } + + #[test] + fn test_e2e_close_only_tab_clears_active() { + let mut manager = TabManager::new(); + manager + .create_tab("Solo".to_string(), TabType::Chat) + .unwrap(); + manager.close_tab(0); + assert!(manager.is_empty()); + assert_eq!(manager.active_index(), None); + } + + // === Group management tests === + + #[test] + fn test_e2e_cycle_through_groups() { + use crate::tui::tab::group::GroupColor; + + let mut manager = TabManager::new(); + let tab1 = manager.create_tab("T1".to_string(), TabType::Chat).unwrap(); + let _g1 = manager.create_group("A".to_string(), GroupColor::Blue); + let _g2 = manager.create_group("B".to_string(), GroupColor::Red); + + // Cycle: not assigned -> first group (A) + manager.cycle_tab_group(tab1); + assert!(manager.tab_group(tab1).is_some()); + + // Cycle: A -> B + manager.cycle_tab_group(tab1); + let group = manager.tab_group(tab1).unwrap(); + assert_eq!(group.name, "B"); + + // Cycle: B -> unassigned + manager.cycle_tab_group(tab1); + assert!(manager.tab_group(tab1).is_none()); + } + + // === Persistence e2e === + + #[test] + fn test_e2e_full_workflow_save_load() { + use std::path::Path; + + let dir = std::env::temp_dir().join("codewhale_e2e_persist"); + let path = dir.join("tabs.json"); + std::fs::create_dir_all(&dir).unwrap(); + let _ = std::fs::remove_file(&path); + + // Session 1: User creates tabs and groups + let mut manager = TabManager::new(); + manager + .create_tab("Work".to_string(), TabType::Chat) + .unwrap(); + manager + .create_tab("Personal".to_string(), TabType::Chat) + .unwrap(); + let group_id = manager.create_group( + "Default".to_string(), + crate::tui::tab::group::GroupColor::Blue, + ); + manager.assign_tab_to_group(manager.all_tabs()[0].id, &group_id); + manager.switch_to(1); + + // App saves on shutdown + manager.save_to_file(&path).unwrap(); + + // Session 2: Restore + let mut restored = TabManager::new(); + restored.restore_from_file(&path).unwrap(); + assert_eq!(restored.len(), 2); + assert_eq!(restored.active_index(), Some(1)); + assert!(restored.tab_group(restored.all_tabs()[0].id).is_some()); + + // Cleanup + let _ = std::fs::remove_file(&path); + let _ = std::fs::remove_dir(&dir); + } + + // === Edge case tests === + + #[test] + fn test_e2e_rapid_create_close() { + let mut manager = TabManager::new(); + for i in 0..9 { + manager + .create_tab(format!("T{}", i), TabType::Chat) + .expect("create"); + } + // Close all + for _ in 0..9 { + manager.close_tab(manager.active_index().unwrap()); + } + assert!(manager.is_empty()); + } + + #[test] + fn test_e2e_switch_empty_manager() { + let mut manager = TabManager::new(); + simulate_ctrl_tab(&mut manager, true); + // Should be a no-op + assert!(manager.is_empty()); + simulate_ctrl_number(&mut manager, 1); + assert!(manager.is_empty()); + } +} diff --git a/crates/tui/src/tui/tab/manager.rs b/crates/tui/src/tui/tab/manager.rs new file mode 100644 index 000000000..736c16a7b --- /dev/null +++ b/crates/tui/src/tui/tab/manager.rs @@ -0,0 +1,876 @@ +//! Tab manager for handling multiple agent sessions + +// WIP collaboration surface. The collab/UI pass lives in PR #2753; this +// file's public items are part of the narrow tab-core harvest. The +// `dead_code` allow at the file root makes that explicit. +#![allow(dead_code)] + +use super::cross_tab::CrossTabLink; +use super::delegator::{DelegationResult, TaskDelegator}; +use super::meeting::{Meeting, MeetingDecision, MeetingManager, MeetingMessage}; +use super::{Priority, TabId, TabMetadata, TabStatus, TabType}; +use std::collections::HashMap; + +/// Maximum number of tabs allowed +pub const MAX_TABS: usize = 9; + +/// Tab state including metadata and status +#[derive(Debug, Clone)] +pub struct TabState { + pub metadata: TabMetadata, + pub status: TabStatus, + pub pending_tasks: Vec, +} + +impl TabState { + pub fn new(id: TabId, title: String, tab_type: TabType) -> Self { + Self { + metadata: TabMetadata::new(id, title, tab_type), + status: TabStatus::Idle, + pending_tasks: Vec::new(), + } + } +} + +/// Manages multiple tabs and their interactions +pub struct TabManager { + tabs: Vec, + active_tab: Option, + max_tabs: usize, + cross_tab_links: HashMap>, + delegator: TaskDelegator, + meeting_manager: MeetingManager, + groups: super::group::TabGroupManager, + /// Monotonic counter for assigning unique tab IDs. Independent of wall + /// clock so two `create_tab` calls in the same nanosecond on fast + /// machines still get distinct IDs. + next_tab_id: u64, +} + +impl TabManager { + /// Create a new tab manager + pub fn new() -> Self { + Self { + tabs: Vec::new(), + active_tab: None, + max_tabs: MAX_TABS, + cross_tab_links: HashMap::new(), + delegator: TaskDelegator::new(), + meeting_manager: MeetingManager::new(), + groups: super::group::TabGroupManager::new(), + next_tab_id: 1, + } + } + + /// Get the number of tabs + /// Returns the number of tabs currently open. + pub fn len(&self) -> usize { + self.tabs.len() + } + + /// Returns true if no tabs are open. + pub fn is_empty(&self) -> bool { + self.tabs.is_empty() + } + + /// Returns the index of the currently active tab, or `None` if no tabs exist. + pub fn active_index(&self) -> Option { + self.active_tab + } + + /// Returns the ID of the currently active tab, or `None` if no tabs exist. + pub fn active_id(&self) -> Option { + self.active_tab + .and_then(|i| self.tabs.get(i)) + .map(|t| t.metadata.id) + } + + /// Returns metadata for all tabs, in tab order. + pub fn all_tabs(&self) -> Vec<&TabMetadata> { + self.tabs.iter().map(|t| &t.metadata).collect() + } + + /// Returns the tab at `index`, or `None` if out of range. + pub fn get(&self, index: usize) -> Option<&TabState> { + self.tabs.get(index) + } + + /// Returns a mutable reference to the tab at `index`, or `None` if out of range. + pub fn get_mut(&mut self, index: usize) -> Option<&mut TabState> { + self.tabs.get_mut(index) + } + + /// Iterate over tabs by `(index, &TabState)` without allocating. + /// Cheaper than [`Self::all_tabs`] for hot paths like the tab bar renderer. + pub fn iter(&self) -> impl Iterator { + self.tabs.iter().enumerate() + } + + /// Create a new tab. The new tab becomes active. + /// + /// # Arguments + /// * `title` - Display title shown in the tab bar + /// * `tab_type` - Type of tab (Chat/Delegation/Review/Meeting) + /// + /// # Returns + /// The new tab's `TabId`, or `None` if `max_tabs` (default 9) is reached. + pub fn create_tab(&mut self, title: String, tab_type: TabType) -> Option { + if self.tabs.len() >= self.max_tabs { + return None; + } + + let id = TabId::new(self.next_tab_id); + self.next_tab_id += 1; + let tab = TabState::new(id, title, tab_type); + self.tabs.push(tab); + self.active_tab = Some(self.tabs.len() - 1); + Some(id) + } + + /// Create a new chat tab with a default `"Tab N"` title where `N` is the + /// 1-indexed slot for the new tab. Returns `None` if `max_tabs` is reached. + pub fn create_default_chat_tab(&mut self) -> Option { + let title = format!("Tab {}", self.tabs.len() + 1); + self.create_tab(title, TabType::Chat) + } + + /// Get mutable access to the group manager + pub fn groups_mut(&mut self) -> &mut super::group::TabGroupManager { + &mut self.groups + } + + /// Get immutable access to the group manager + pub fn groups(&self) -> &super::group::TabGroupManager { + &self.groups + } + + /// Create a new tab group + pub fn create_group(&mut self, name: String, color: super::group::GroupColor) -> String { + self.groups.create_group(name, color) + } + + /// Delete a tab group + pub fn delete_group(&mut self, group_id: &str) -> bool { + self.groups.delete_group(group_id) + } + + /// Assign a tab to a group + pub fn assign_tab_to_group(&mut self, tab_id: TabId, group_id: &str) -> bool { + self.groups.assign_tab(tab_id, group_id) + } + + /// Get the group a tab is assigned to + pub fn tab_group(&self, tab_id: TabId) -> Option<&super::group::TabGroup> { + self.groups.group_of(tab_id) + } + + /// List all groups + pub fn all_groups(&self) -> Vec<&super::group::TabGroup> { + self.groups.all_groups() + } + + /// Cycle a tab to the next group + pub fn cycle_tab_group(&mut self, tab_id: TabId) { + self.groups.cycle_tab_group(tab_id); + } + + /// Snapshot the current manager state for persistence + pub fn snapshot(&self) -> super::persistence::PersistedTabState { + use super::persistence::PersistedTab; + let tabs: Vec = self + .tabs + .iter() + .map(|t| super::persistence::from_metadata(&t.metadata)) + .collect(); + let delegations: Vec = self + .delegator + .all_pending() + .iter() + .map(|t| super::persistence::PersistedDelegation { + task_id: t.task_id.clone(), + from_tab: t.from_tab.0, + to_tab: t.to_tab.0, + description: t.description.clone(), + priority: t.priority, + created_at: t.created_at, + completed_at: t.completed_at, + result: t.result.clone(), + was_successful: None, + }) + .collect(); + let groups: Vec = self + .groups + .all_groups() + .into_iter() + .map(|g| super::persistence::PersistedGroup { + id: g.id.clone(), + name: g.name.clone(), + color: g.color, + tab_ids: g.tab_ids.clone(), + created_at: g.created_at, + }) + .collect(); + super::persistence::PersistedTabState { + version: 1, + saved_at: chrono::Utc::now(), + active_tab_index: self.active_tab, + tabs, + delegations, + groups, + } + } + + /// Save the current state to a file + pub fn save_to_file(&self, path: &std::path::Path) -> std::io::Result<()> { + let state = self.snapshot(); + super::persistence::save_to_file(&state, path) + } + + /// Restore state from a persisted snapshot. Existing tabs are cleared. + pub fn restore_from_snapshot(&mut self, state: &super::persistence::PersistedTabState) { + self.tabs.clear(); + self.active_tab = None; + self.cross_tab_links.clear(); + self.delegator = super::delegator::TaskDelegator::new(); + self.meeting_manager = super::meeting::MeetingManager::new(); + self.groups = super::group::TabGroupManager::new(); + self.next_tab_id = 1; + + let mut max_seen_id: u64 = 0; + for p in &state.tabs { + let meta = super::persistence::to_metadata(p); + // Advance the monotonic counter past any restored ID so freshly + // created tabs can never collide with restored ones. + if meta.id.0 > max_seen_id { + max_seen_id = meta.id.0; + } + let tab = TabState { + metadata: meta, + status: TabStatus::Idle, + pending_tasks: Vec::new(), + }; + self.tabs.push(tab); + } + self.next_tab_id = max_seen_id + 1; + + // Restore active delegations so cross-tab work survives restarts. + for d in &state.delegations { + let task = super::delegator::DelegationTask { + task_id: d.task_id.clone(), + from_tab: TabId(d.from_tab), + to_tab: TabId(d.to_tab), + description: d.description.clone(), + priority: d.priority, + status: super::delegator::DelegationStatus::Pending, + created_at: d.created_at, + deadline: None, + completed_at: d.completed_at, + result: d.result.clone(), + }; + self.delegator.pending_tasks.push(task); + } + + // Restore groups AFTER tabs so tab_ids can reference real tabs + for g in &state.groups { + let group = super::group::TabGroup { + id: g.id.clone(), + name: g.name.clone(), + color: g.color, + tab_ids: g.tab_ids.clone(), + created_at: g.created_at, + }; + self.groups.groups.insert(group.id.clone(), group); + for tab_id in &g.tab_ids { + self.groups.tab_to_group.insert(*tab_id, g.id.clone()); + } + } + + if let Some(idx) = state.active_tab_index { + if idx < self.tabs.len() { + self.active_tab = Some(idx); + } else if !self.tabs.is_empty() { + self.active_tab = Some(self.tabs.len() - 1); + } + } else if !self.tabs.is_empty() { + self.active_tab = Some(0); + } + } + + /// Restore state from a file. Missing file is treated as empty. + pub fn restore_from_file(&mut self, path: &std::path::Path) -> std::io::Result<()> { + let state = super::persistence::load_from_file(path)?; + self.restore_from_snapshot(&state); + Ok(()) + } + + /// Close a tab by index + pub fn close_tab(&mut self, index: usize) -> bool { + if index >= self.tabs.len() { + return false; + } + + // Unassign from any group before removing + let tab_id = self.tabs[index].metadata.id; + self.groups.unassign_tab(tab_id); + + self.tabs.remove(index); + + // Adjust active tab index + if let Some(active) = self.active_tab { + if index < active { + self.active_tab = Some(active - 1); + } else if index == active { + self.active_tab = if self.tabs.is_empty() { + None + } else if active >= self.tabs.len() { + Some(self.tabs.len() - 1) + } else { + Some(active) + }; + } + } + + true + } + + /// Close a tab by ID + pub fn close_tab_by_id(&mut self, id: TabId) -> bool { + if let Some(index) = self.tabs.iter().position(|t| t.metadata.id == id) { + self.close_tab(index) + } else { + false + } + } + + /// Switch to a tab by index + pub fn switch_to(&mut self, index: usize) -> bool { + if index < self.tabs.len() { + self.active_tab = Some(index); + if let Some(tab) = self.tabs.get_mut(index) { + tab.metadata.touch(); + tab.metadata.clear_unread(); + } + true + } else { + false + } + } + + /// Switch to the next tab + pub fn switch_to_next(&mut self) -> bool { + if self.tabs.is_empty() { + return false; + } + + let next = match self.active_tab { + Some(i) => (i + 1) % self.tabs.len(), + None => 0, + }; + self.switch_to(next) + } + + /// Switch to the previous tab + pub fn switch_to_prev(&mut self) -> bool { + if self.tabs.is_empty() { + return false; + } + + let prev = match self.active_tab { + Some(i) => { + if i == 0 { + self.tabs.len() - 1 + } else { + i - 1 + } + } + None => self.tabs.len() - 1, + }; + self.switch_to(prev) + } + + /// Switch to tab by ID + pub fn switch_to_by_id(&mut self, id: TabId) -> bool { + if let Some(index) = self.tabs.iter().position(|t| t.metadata.id == id) { + self.switch_to(index) + } else { + false + } + } + + /// Update tab title + pub fn update_title(&mut self, index: usize, title: &str) -> bool { + if let Some(tab) = self.tabs.get_mut(index) { + tab.metadata.title = title.to_string(); + true + } else { + false + } + } + + /// Update tab status + pub fn update_status(&mut self, index: usize, status: TabStatus) -> bool { + if let Some(tab) = self.tabs.get_mut(index) { + tab.status = status; + true + } else { + false + } + } + + /// Mark a tab as having unread content + pub fn mark_unread(&mut self, index: usize) -> bool { + if let Some(tab) = self.tabs.get_mut(index) { + if self.active_tab != Some(index) { + tab.metadata.increment_unread(); + } + true + } else { + false + } + } + + /// Get pending tasks for a tab + pub fn pending_tasks(&self, id: TabId) -> Vec<&DelegationResult> { + self.delegator.results_for_tab(id) + } + + /// Create a link between tabs for cross-tab events + pub fn create_link(&mut self, from: TabId, to: TabId) { + let link = CrossTabLink { + from, + to, + created_at: chrono::Utc::now(), + }; + self.cross_tab_links.entry(from).or_default().push(link); + } + + /// Remove link between tabs + pub fn remove_link(&mut self, from: TabId, to: TabId) -> bool { + if let Some(links) = self.cross_tab_links.get_mut(&from) { + let initial_len = links.len(); + links.retain(|l| l.to != to); + links.len() < initial_len + } else { + false + } + } + + /// Get all links from a tab + pub fn get_links(&self, from: TabId) -> Vec<&CrossTabLink> { + self.cross_tab_links + .get(&from) + .map(|l| l.iter().collect()) + .unwrap_or_default() + } + + /// Delegate a task from one tab to another + pub fn delegate_task( + &mut self, + from: TabId, + to: TabId, + description: String, + priority: Priority, + ) -> Option { + self.delegator + .create_delegation(from, to, description, priority) + } + + /// Complete a delegated task + pub fn complete_delegation(&mut self, task_id: &str, result: String) { + self.delegator.complete(task_id, result); + } + + /// Get pending delegations for a tab + pub fn pending_delegations(&self, tab_id: TabId) -> Vec<&super::delegator::DelegationTask> { + self.delegator.pending_for_tab(tab_id) + } + + /// Take the highest-priority pending delegation for a tab. + /// The task is marked as in-progress in place (it is not removed until + /// `complete_delegation` / `fail_delegation` / `cancel_delegation` is + /// called). Returns the task. + pub fn take_next_delegation( + &mut self, + tab_id: TabId, + ) -> Option { + self.delegator.take_pending_for_tab(tab_id) + } + + /// Peek at the next pending delegation for a tab + pub fn peek_next_delegation(&self, tab_id: TabId) -> Option<&super::delegator::DelegationTask> { + self.delegator.peek_pending_for_tab(tab_id) + } + + /// Check if a tab has any pending delegations + pub fn has_pending_delegations(&self, tab_id: TabId) -> bool { + self.delegator.peek_pending_for_tab(tab_id).is_some() + } + + /// Start a meeting + pub fn start_meeting(&mut self, topic: String, participants: Vec) -> Option { + self.meeting_manager.start_meeting(topic, participants) + } + + /// End a meeting + pub fn end_meeting(&mut self, meeting_id: &str) -> Option { + self.meeting_manager.end_meeting(meeting_id) + } + + /// Add a message to a meeting + pub fn add_meeting_message(&mut self, meeting_id: &str, msg: MeetingMessage) { + self.meeting_manager.add_message(meeting_id, msg); + } + + /// Add a decision to a meeting + pub fn add_meeting_decision(&mut self, meeting_id: &str, decision: MeetingDecision) { + self.meeting_manager.add_decision(meeting_id, decision); + } + + /// Get active meeting for a tab + pub fn active_meeting(&self, tab_id: TabId) -> Option<&Meeting> { + self.meeting_manager.active_meeting_for(tab_id) + } + + /// Get an active meeting by ID. + pub fn meeting(&self, meeting_id: &str) -> Option<&Meeting> { + self.meeting_manager.get_meeting(meeting_id) + } +} + +impl Default for TabManager { + fn default() -> Self { + Self::new() + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_create_and_close_tab() { + let mut manager = TabManager::new(); + assert!(manager.is_empty()); + + let id1 = manager.create_tab("Tab 1".to_string(), TabType::Chat); + assert!(id1.is_some()); + assert_eq!(manager.len(), 1); + assert_eq!(manager.active_index(), Some(0)); + + let id2 = manager.create_tab("Tab 2".to_string(), TabType::Chat); + assert!(id2.is_some()); + assert_eq!(manager.len(), 2); + assert_eq!(manager.active_index(), Some(1)); + + // Close second tab + assert!(manager.close_tab(1)); + assert_eq!(manager.len(), 1); + + // Close first tab + assert!(manager.close_tab(0)); + assert!(manager.is_empty()); + } + + #[test] + fn test_switch_tabs() { + let mut manager = TabManager::new(); + + manager.create_tab("Tab 1".to_string(), TabType::Chat); + manager.create_tab("Tab 2".to_string(), TabType::Chat); + manager.create_tab("Tab 3".to_string(), TabType::Chat); + + assert_eq!(manager.active_index(), Some(2)); + + // Switch to next (wraps around) + assert!(manager.switch_to_next()); + assert_eq!(manager.active_index(), Some(0)); + + // Switch to prev + assert!(manager.switch_to_prev()); + assert_eq!(manager.active_index(), Some(2)); + } + + #[test] + fn test_max_tabs() { + let mut manager = TabManager::new(); + + // Create max tabs + for i in 0..MAX_TABS { + let result = manager.create_tab(format!("Tab {}", i), TabType::Chat); + assert!(result.is_some(), "Tab {} should be created", i); + } + + // Try to create one more + let result = manager.create_tab("Extra".to_string(), TabType::Chat); + assert!(result.is_none(), "Should not exceed max tabs"); + } + + #[test] + fn test_delegation_take_priority_order() { + use super::super::Priority; + + let mut manager = TabManager::new(); + let from = manager + .create_tab("Source".to_string(), TabType::Chat) + .unwrap(); + let to = manager + .create_tab("Target".to_string(), TabType::Chat) + .unwrap(); + + // Delegate with different priorities + manager.delegate_task(from, to, "Low task".to_string(), Priority::Low); + manager.delegate_task(from, to, "Urgent task".to_string(), Priority::Urgent); + manager.delegate_task(from, to, "Normal task".to_string(), Priority::Normal); + + // Take should return Urgent first + let task = manager.take_next_delegation(to).unwrap(); + assert_eq!(task.description, "Urgent task"); + assert_eq!(task.priority, Priority::Urgent); + + // has_pending_delegations should be true + assert!(manager.has_pending_delegations(to)); + + // Take should return Normal next + let task = manager.take_next_delegation(to).unwrap(); + assert_eq!(task.description, "Normal task"); + assert_eq!(task.priority, Priority::Normal); + + // Take should return Low last + let task = manager.take_next_delegation(to).unwrap(); + assert_eq!(task.description, "Low task"); + assert_eq!(task.priority, Priority::Low); + + // No more delegations + assert!(manager.take_next_delegation(to).is_none()); + assert!(!manager.has_pending_delegations(to)); + } + + #[test] + fn test_delegation_peek() { + use super::super::Priority; + + let mut manager = TabManager::new(); + let from = manager + .create_tab("Source".to_string(), TabType::Chat) + .unwrap(); + let to = manager + .create_tab("Target".to_string(), TabType::Chat) + .unwrap(); + + manager.delegate_task(from, to, "Test task".to_string(), Priority::High); + + // Peek multiple times should not consume + let peek1 = manager.peek_next_delegation(to).unwrap(); + let peek2 = manager.peek_next_delegation(to).unwrap(); + assert_eq!(peek1.task_id, peek2.task_id); + assert_eq!(manager.delegator.pending_count(to), 1); + } + + #[test] + fn test_tab_type_icons() { + use super::super::TabType; + + // Each tab type has a unique icon + assert_eq!(TabType::Chat.icon(), "💬"); + assert_eq!(TabType::Delegation.icon(), "📤"); + assert_eq!(TabType::Review.icon(), "🔍"); + assert_eq!(TabType::Meeting.icon(), "👥"); + + // ASCII fallback + assert_eq!(TabType::Chat.ascii_icon(), "[C]"); + assert_eq!(TabType::Delegation.ascii_icon(), "[D]"); + assert_eq!(TabType::Review.ascii_icon(), "[R]"); + assert_eq!(TabType::Meeting.ascii_icon(), "[M]"); + + // Display names + assert_eq!(TabType::Chat.display_name(), "Chat"); + assert_eq!(TabType::Delegation.display_name(), "Delegation"); + assert_eq!(TabType::Review.display_name(), "Review"); + assert_eq!(TabType::Meeting.display_name(), "Meeting"); + } + + #[test] + fn test_snapshot_and_restore() { + let mut manager = TabManager::new(); + let id1 = manager + .create_tab("Tab A".to_string(), TabType::Chat) + .unwrap(); + let _id2 = manager + .create_tab("Tab B".to_string(), TabType::Review) + .unwrap(); + assert!(manager.switch_to_by_id(id1)); + + // Snapshot + let snapshot = manager.snapshot(); + assert_eq!(snapshot.tabs.len(), 2); + assert_eq!(snapshot.active_tab_index, Some(0)); + assert_eq!(snapshot.tabs[0].title, "Tab A"); + assert_eq!(snapshot.tabs[1].tab_type, TabType::Review); + + // Mutate original + manager.close_tab(0); + assert_eq!(manager.len(), 1); + + // Restore + manager.restore_from_snapshot(&snapshot); + assert_eq!(manager.len(), 2); + assert_eq!(manager.active_index(), Some(0)); + assert_eq!(manager.all_tabs()[0].title, "Tab A"); + } + + #[test] + fn test_snapshot_restore_preserves_delegations() { + use super::super::Priority; + + let mut manager = TabManager::new(); + let from = manager + .create_tab("Source".to_string(), TabType::Chat) + .unwrap(); + let to = manager + .create_tab("Target".to_string(), TabType::Chat) + .unwrap(); + + // Create a pending delegation that should survive a restart. + manager.delegate_task(from, to, "review the patch".to_string(), Priority::High); + assert_eq!(manager.pending_delegations(to).len(), 1); + + let snapshot = manager.snapshot(); + assert_eq!(snapshot.delegations.len(), 1); + assert_eq!(snapshot.delegations[0].description, "review the patch"); + + // Fresh manager restores the delegation. + let mut restored = TabManager::new(); + restored.restore_from_snapshot(&snapshot); + assert_eq!(restored.pending_delegations(to).len(), 1); + assert_eq!( + restored.pending_delegations(to)[0].description, + "review the patch" + ); + assert_eq!(restored.pending_delegations(to)[0].priority, Priority::High); + } + + #[test] + fn test_create_tab_id_does_not_collide_after_restore() { + let mut manager = TabManager::new(); + let _a = manager.create_tab("A".to_string(), TabType::Chat).unwrap(); + let _b = manager.create_tab("B".to_string(), TabType::Chat).unwrap(); + let _c = manager.create_tab("C".to_string(), TabType::Chat).unwrap(); + let max_id_before = manager.all_tabs().iter().map(|t| t.id.0).max().unwrap(); + + let snapshot = manager.snapshot(); + manager.close_tab(0); + manager.close_tab(0); + manager.close_tab(0); + assert!(manager.is_empty()); + + manager.restore_from_snapshot(&snapshot); + let d = manager.create_tab("D".to_string(), TabType::Chat).unwrap(); + assert!( + d.0 > max_id_before, + "new tab id {} must exceed restored max {}", + d.0, + max_id_before + ); + // The new tab id must not collide with any *restored* tab. + for tab in manager.all_tabs() { + if tab.title == "D" { + continue; + } + assert_ne!(tab.id, d, "newly created tab collided with restored tab"); + } + } + + #[test] + fn test_persistence_roundtrip() { + let dir = std::env::temp_dir().join("codewhale_tabs_roundtrip"); + let path = dir.join("tabs.json"); + let _ = std::fs::remove_file(&path); + + let mut manager = TabManager::new(); + manager + .create_tab("First".to_string(), TabType::Chat) + .unwrap(); + manager + .create_tab("Second".to_string(), TabType::Meeting) + .unwrap(); + manager.switch_to(1); + + // Save + manager.save_to_file(&path).unwrap(); + assert!(path.exists()); + + // Create fresh manager and restore + let mut restored = TabManager::new(); + restored.restore_from_file(&path).unwrap(); + + assert_eq!(restored.len(), 2); + assert_eq!(restored.active_index(), Some(1)); + assert_eq!(restored.all_tabs()[0].title, "First"); + assert_eq!(restored.all_tabs()[1].tab_type, TabType::Meeting); + + // Cleanup + let _ = std::fs::remove_file(&path); + let _ = std::fs::remove_dir(&dir); + } + + #[test] + fn test_manager_group_operations() { + use super::super::group::GroupColor; + + let mut manager = TabManager::new(); + let tab1 = manager + .create_tab("Tab 1".to_string(), TabType::Chat) + .unwrap(); + let tab2 = manager + .create_tab("Tab 2".to_string(), TabType::Review) + .unwrap(); + + // Create group + let group_id = manager.create_group("Frontend".to_string(), GroupColor::Blue); + assert_eq!(manager.all_groups().len(), 1); + + // Assign tabs + assert!(manager.assign_tab_to_group(tab1, &group_id)); + assert!(manager.assign_tab_to_group(tab2, &group_id)); + + let group = manager.tab_group(tab1).unwrap(); + assert_eq!(group.name, "Frontend"); + assert_eq!(group.len(), 2); + + // Cycle: unassign when at the end of the sorted group list + manager.cycle_tab_group(tab1); + // With only one group, cycling should unassign + assert!(manager.tab_group(tab1).is_none()); + + // Re-assign and create a second group + manager.assign_tab_to_group(tab1, &group_id); + manager.create_group("Alpha".to_string(), GroupColor::Red); + // Alpha comes before Frontend alphabetically + // Cycling from Frontend (last) should unassign + manager.cycle_tab_group(tab1); + assert!(manager.tab_group(tab1).is_none()); + } + + #[test] + fn test_close_tab_unassigns_group() { + use super::super::group::GroupColor; + + let mut manager = TabManager::new(); + let tab1 = manager + .create_tab("Tab 1".to_string(), TabType::Chat) + .unwrap(); + let group_id = manager.create_group("Test".to_string(), GroupColor::Cyan); + manager.assign_tab_to_group(tab1, &group_id); + + // Verify assignment + assert!(manager.tab_group(tab1).is_some()); + + // Close the tab + manager.close_tab(0); + + // Group should still exist but be empty + let group = manager.groups().all_groups(); + assert_eq!(group.len(), 1); + assert_eq!(group[0].len(), 0); + } +} diff --git a/crates/tui/src/tui/tab/meeting.rs b/crates/tui/src/tui/tab/meeting.rs new file mode 100644 index 000000000..d969c6bd2 --- /dev/null +++ b/crates/tui/src/tui/tab/meeting.rs @@ -0,0 +1,377 @@ +//! Meeting manager for multi-agent discussions + +// WIP collaboration surface — narrow harvest. See `tab/mod.rs` for the +// PR #2753 context. +#![allow(dead_code)] + +use super::TabId; +use chrono::{DateTime, Utc}; +use serde::{Deserialize, Serialize}; +use std::collections::HashMap; + +/// Status of a meeting +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +pub enum MeetingStatus { + Active, + Paused, + Ended, +} + +/// Type of meeting message +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +pub enum MeetingMessageType { + Regular, + Question, + Answer, + Proposal, + Agreement, + Objection, + Summary, +} + +/// A message in a meeting +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct MeetingMessage { + pub id: u64, + pub sender: TabId, + pub content: String, + pub message_type: MeetingMessageType, + pub timestamp: DateTime, +} + +impl MeetingMessage { + pub fn new(id: u64, sender: TabId, content: String) -> Self { + Self { + id, + sender, + content, + message_type: MeetingMessageType::Regular, + timestamp: Utc::now(), + } + } + + pub fn question(id: u64, sender: TabId, content: String) -> Self { + Self { + id, + sender, + content, + message_type: MeetingMessageType::Question, + timestamp: Utc::now(), + } + } + + pub fn proposal(id: u64, sender: TabId, content: String) -> Self { + Self { + id, + sender, + content, + message_type: MeetingMessageType::Proposal, + timestamp: Utc::now(), + } + } +} + +/// A decision made in a meeting +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct MeetingDecision { + pub id: u64, + pub description: String, + pub proposer: TabId, + pub supporters: Vec, + pub timestamp: DateTime, +} + +impl MeetingDecision { + pub fn new(id: u64, description: String, proposer: TabId) -> Self { + Self { + id, + description, + proposer, + supporters: vec![proposer], + timestamp: Utc::now(), + } + } + + pub fn add_supporter(&mut self, tab_id: TabId) { + if !self.supporters.contains(&tab_id) { + self.supporters.push(tab_id); + } + } + + pub fn support_count(&self) -> usize { + self.supporters.len() + } +} + +/// A meeting session +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct Meeting { + pub id: String, + pub topic: String, + pub participants: Vec, + pub messages: Vec, + pub decisions: Vec, + pub status: MeetingStatus, + pub created_at: DateTime, + pub ended_at: Option>, +} + +impl Meeting { + pub fn new(id: String, topic: String, participants: Vec) -> Self { + Self { + id, + topic, + participants, + messages: Vec::new(), + decisions: Vec::new(), + status: MeetingStatus::Active, + created_at: Utc::now(), + ended_at: None, + } + } + + pub fn add_message(&mut self, msg: MeetingMessage) { + self.messages.push(msg); + } + + pub fn add_decision(&mut self, decision: MeetingDecision) { + self.decisions.push(decision); + } + + pub fn end(&mut self) { + self.status = MeetingStatus::Ended; + self.ended_at = Some(Utc::now()); + } + + pub fn duration(&self) -> chrono::Duration { + let end = self.ended_at.unwrap_or_else(Utc::now); + end - self.created_at + } + + pub fn message_count(&self) -> usize { + self.messages.len() + } + + pub fn decision_count(&self) -> usize { + self.decisions.len() + } +} + +/// Summary of a completed meeting +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct MeetingSummary { + pub id: String, + pub topic: String, + pub participant_count: usize, + pub message_count: usize, + pub decision_count: usize, + pub duration_seconds: i64, + pub decisions: Vec, +} + +/// Meeting history entry +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct MeetingHistory { + pub summary: MeetingSummary, + pub created_at: DateTime, +} + +/// Manager for meeting sessions +pub struct MeetingManager { + active_meetings: HashMap, + history: Vec, + next_meeting_id: u64, + next_message_id: u64, + next_decision_id: u64, +} + +impl MeetingManager { + pub fn new() -> Self { + Self { + active_meetings: HashMap::new(), + history: Vec::new(), + next_meeting_id: 1, + next_message_id: 1, + next_decision_id: 1, + } + } + + /// Start a new meeting + pub fn start_meeting(&mut self, topic: String, participants: Vec) -> Option { + if participants.len() < 2 { + return None; + } + + let meeting_id = self.generate_meeting_id(); + let meeting = Meeting::new(meeting_id.clone(), topic, participants); + self.active_meetings.insert(meeting_id.clone(), meeting); + Some(meeting_id) + } + + /// Get an active meeting by ID + pub fn get_meeting(&self, meeting_id: &str) -> Option<&Meeting> { + self.active_meetings.get(meeting_id) + } + + /// Get a mutable meeting by ID + pub fn get_meeting_mut(&mut self, meeting_id: &str) -> Option<&mut Meeting> { + self.active_meetings.get_mut(meeting_id) + } + + /// Add a message to a meeting + pub fn add_message(&mut self, meeting_id: &str, msg: MeetingMessage) { + if let Some(meeting) = self.active_meetings.get_mut(meeting_id) { + meeting.add_message(msg); + } + } + + /// Create and add a new message + pub fn create_message( + &mut self, + meeting_id: &str, + sender: TabId, + content: String, + ) -> Option { + let msg_id = self.next_message_id; + self.next_message_id += 1; + + if let Some(meeting) = self.active_meetings.get_mut(meeting_id) { + let msg = MeetingMessage::new(msg_id, sender, content); + meeting.add_message(msg); + Some(msg_id) + } else { + None + } + } + + /// Add a decision to a meeting + pub fn add_decision(&mut self, meeting_id: &str, decision: MeetingDecision) { + if let Some(meeting) = self.active_meetings.get_mut(meeting_id) { + meeting.add_decision(decision); + } + } + + /// Create and add a new decision + pub fn create_decision( + &mut self, + meeting_id: &str, + description: String, + proposer: TabId, + ) -> Option { + let decision_id = self.next_decision_id; + self.next_decision_id += 1; + + if let Some(meeting) = self.active_meetings.get_mut(meeting_id) { + let decision = MeetingDecision::new(decision_id, description, proposer); + meeting.add_decision(decision); + Some(decision_id) + } else { + None + } + } + + /// End a meeting + pub fn end_meeting(&mut self, meeting_id: &str) -> Option { + if let Some(mut meeting) = self.active_meetings.remove(meeting_id) { + meeting.end(); + let duration = meeting.duration(); + let summary = MeetingSummary { + id: meeting.id.clone(), + topic: meeting.topic.clone(), + participant_count: meeting.participants.len(), + message_count: meeting.message_count(), + decision_count: meeting.decision_count(), + duration_seconds: duration.num_seconds(), + decisions: meeting + .decisions + .iter() + .map(|d| d.description.clone()) + .collect(), + }; + self.history.push(MeetingHistory { + summary: summary.clone(), + created_at: meeting.created_at, + }); + Some(summary) + } else { + None + } + } + + /// Get active meeting for a specific tab + pub fn active_meeting_for(&self, tab_id: TabId) -> Option<&Meeting> { + self.active_meetings + .values() + .find(|m| m.participants.contains(&tab_id)) + } + + /// Get all active meetings + pub fn active_meetings(&self) -> Vec<&Meeting> { + self.active_meetings.values().collect() + } + + /// Get meeting history + pub fn history(&self) -> &[MeetingHistory] { + &self.history + } + + /// Get recent meetings + pub fn recent_meetings(&self, limit: usize) -> Vec<&MeetingHistory> { + let mut history: Vec<&MeetingHistory> = self.history.iter().collect(); + history.sort_by_key(|m| std::cmp::Reverse(m.created_at)); + history.into_iter().take(limit).collect() + } + + /// Check if a tab is in any active meeting + pub fn is_in_meeting(&self, tab_id: TabId) -> bool { + self.active_meetings + .values() + .any(|m| m.participants.contains(&tab_id)) + } + + fn generate_meeting_id(&mut self) -> String { + let id = self.next_meeting_id; + self.next_meeting_id += 1; + format!("meeting_{}", id) + } +} + +impl Default for MeetingManager { + fn default() -> Self { + Self::new() + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_meeting_lifecycle() { + let mut manager = MeetingManager::new(); + let tab1 = TabId::new(1); + let tab2 = TabId::new(2); + + let meeting_id = manager + .start_meeting("Discuss design".to_string(), vec![tab1, tab2]) + .unwrap(); + + assert!(manager.is_in_meeting(tab1)); + assert!(manager.is_in_meeting(tab2)); + assert!(!manager.is_in_meeting(TabId::new(3))); + + manager.create_message(&meeting_id, tab1, "Let's start".to_string()); + manager.create_message(&meeting_id, tab2, "Agreed".to_string()); + + let meeting = manager.get_meeting(&meeting_id).unwrap(); + assert_eq!(meeting.message_count(), 2); + + manager.create_decision(&meeting_id, "Use component pattern".to_string(), tab1); + + let summary = manager.end_meeting(&meeting_id).unwrap(); + assert_eq!(summary.topic, "Discuss design"); + assert_eq!(summary.participant_count, 2); + assert_eq!(summary.message_count, 2); + assert_eq!(summary.decision_count, 1); + } +} diff --git a/crates/tui/src/tui/tab/mention.rs b/crates/tui/src/tui/tab/mention.rs new file mode 100644 index 000000000..e2825bcbc --- /dev/null +++ b/crates/tui/src/tui/tab/mention.rs @@ -0,0 +1,219 @@ +//! Smart @-mention parsing for cross-tab references +//! +//! When a user writes a message like "Hey @Tab2 can you review this?", +//! the mention parser extracts the referenced tab and suggests +//! automatically switching to it (or routing the message). +//! +//! Supported mention forms: +//! - `@Tab2` - by tab number (1-indexed) +//! - `@2` - shorthand for tab number +//! - `@tab2` - case-insensitive +//! +//! Examples: +//! ``` +//! use crate::tui::tab::mention::extract_tab_mention; +//! assert_eq!(extract_tab_mention("Hello @Tab2!"), Some(2)); +//! assert_eq!(extract_tab_mention("see @3"), Some(3)); +//! assert_eq!(extract_tab_mention("no mention here"), None); +//! ``` + +// WIP collaboration surface — narrow harvest. See `tab/mod.rs` for the +// PR #2753 context. +#![allow(dead_code)] + +/// Parse a message and extract the first tab mention (1-indexed number). +/// Returns `None` if no valid mention is found. +/// +/// Recognized patterns: +/// - `@Tab` (e.g. `@Tab2`, `@tab3`) +/// - `@` at the start of a word (e.g. `@2`, `@3`) +/// +/// The mention must be a single token (preceded by start-of-string or +/// whitespace, followed by whitespace, punctuation, or end-of-string). +pub fn extract_tab_mention(message: &str) -> Option { + let bytes = message.as_bytes(); + let mut i = 0; + while i < bytes.len() { + if bytes[i] == b'@' { + // Must be at start or after whitespace + if i > 0 && !is_mention_boundary(bytes[i - 1]) { + i += 1; + continue; + } + // Try `@Tab` + if i + 4 <= bytes.len() + && bytes[i..i + 4].eq_ignore_ascii_case(b"@tab") + && i + 4 < bytes.len() + && bytes[i + 4].is_ascii_digit() + { + let num_start = i + 4; + let num_end = scan_digits(bytes, num_start); + if let Some(num) = parse_usize(&bytes[num_start..num_end]) + && num > 0 + && is_mention_terminator(bytes, num_end) + { + return Some(num); + } + } + // Try `@` shorthand + if i + 1 < bytes.len() && bytes[i + 1].is_ascii_digit() { + let num_start = i + 1; + let num_end = scan_digits(bytes, num_start); + if let Some(num) = parse_usize(&bytes[num_start..num_end]) + && num > 0 + && is_mention_terminator(bytes, num_end) + { + return Some(num); + } + } + } + i += 1; + } + None +} + +/// Extract all tab mentions in order +pub fn extract_all_tab_mentions(message: &str) -> Vec { + let mut mentions = Vec::new(); + let bytes = message.as_bytes(); + let mut i = 0; + while i < bytes.len() { + if bytes[i] == b'@' && (i == 0 || is_mention_boundary(bytes[i - 1])) { + // Determine the mention token length + let token_start = i; + // @Tab or @ + let mut j = i + 1; + if j + 3 <= bytes.len() && bytes[j..j + 3].eq_ignore_ascii_case(b"tab") { + j += 3; + } + let num_start = j; + while j < bytes.len() && bytes[j].is_ascii_digit() { + j += 1; + } + let num_end = j; + if num_start < num_end + && is_mention_terminator(bytes, num_end) + && let Some(num) = parse_usize(&bytes[num_start..num_end]) + && num > 0 + { + mentions.push(num); + // Also skip past the terminator + i = num_end; + continue; + } + let _ = token_start; // suppress unused + } + i += 1; + } + mentions +} + +fn is_mention_boundary(b: u8) -> bool { + b == b' ' || b == b'\t' || b == b'\n' || b == b'\r' || b == b',' || b == b';' +} + +fn is_mention_terminator(bytes: &[u8], pos: usize) -> bool { + pos >= bytes.len() + || bytes[pos] == b' ' + || bytes[pos] == b'\t' + || bytes[pos] == b'\n' + || bytes[pos] == b'\r' + || bytes[pos] == b',' + || bytes[pos] == b'.' + || bytes[pos] == b'!' + || bytes[pos] == b'?' + || bytes[pos] == b';' + || bytes[pos] == b':' +} + +fn scan_digits(bytes: &[u8], start: usize) -> usize { + let mut end = start; + while end < bytes.len() && bytes[end].is_ascii_digit() { + end += 1; + } + end +} + +fn parse_usize(s: &[u8]) -> Option { + if s.is_empty() { + return None; + } + let mut result: usize = 0; + for &b in s { + if !b.is_ascii_digit() { + return None; + } + result = result.checked_mul(10)?; + result = result.checked_add((b - b'0') as usize)?; + } + Some(result) +} + +/// Given a tab number (1-indexed) and the list of tab IDs, return the +/// matching TabId. Returns `None` if the index is out of range. +pub fn resolve_tab_mention<'a, I>(tab_number: usize, tab_ids: I) -> Option +where + I: IntoIterator, +{ + let mut sorted: Vec = tab_ids.into_iter().copied().collect(); + sorted.sort_unstable(); + if tab_number == 0 || tab_number > sorted.len() { + return None; + } + Some(sorted[tab_number - 1]) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_extract_tab_mention() { + assert_eq!(extract_tab_mention("Hello @Tab2!"), Some(2)); + assert_eq!(extract_tab_mention("see @3 please"), Some(3)); + assert_eq!(extract_tab_mention("@Tab1 help"), Some(1)); + assert_eq!(extract_tab_mention("no mention here"), None); + assert_eq!(extract_tab_mention("email@2 is wrong"), None); // not at boundary + assert_eq!(extract_tab_mention("@0 invalid"), None); // 0 not allowed + assert_eq!(extract_tab_mention(""), None); + } + + #[test] + fn test_extract_case_insensitive() { + assert_eq!(extract_tab_mention("Hello @TAB2!"), Some(2)); + assert_eq!(extract_tab_mention("see @tab3 please"), Some(3)); + } + + #[test] + fn test_extract_with_punctuation() { + assert_eq!(extract_tab_mention("Please @Tab2, review"), Some(2)); + assert_eq!(extract_tab_mention("Ask @Tab2."), Some(2)); + assert_eq!(extract_tab_mention("Hey @Tab2!"), Some(2)); + assert_eq!(extract_tab_mention("What about @Tab2?"), Some(2)); + } + + #[test] + fn test_extract_all_mentions() { + let mentions = extract_all_tab_mentions("Hey @Tab2 and @3, also @Tab1"); + assert_eq!(mentions, vec![2, 3, 1]); + } + + #[test] + fn test_extract_mention_at_start() { + assert_eq!(extract_tab_mention("@Tab1 hi"), Some(1)); + } + + #[test] + fn test_resolve_tab_mention() { + let tab_ids = vec![100, 50, 200]; + // Tab 1 = smallest id (50) + assert_eq!(resolve_tab_mention(1, tab_ids.iter()), Some(50)); + // Tab 2 = middle id (100) + assert_eq!(resolve_tab_mention(2, tab_ids.iter()), Some(100)); + // Tab 3 = largest id (200) + assert_eq!(resolve_tab_mention(3, tab_ids.iter()), Some(200)); + // Out of range + assert_eq!(resolve_tab_mention(4, tab_ids.iter()), None); + assert_eq!(resolve_tab_mention(0, tab_ids.iter()), None); + } +} diff --git a/crates/tui/src/tui/tab/mod.rs b/crates/tui/src/tui/tab/mod.rs new file mode 100644 index 000000000..8e264d59a --- /dev/null +++ b/crates/tui/src/tui/tab/mod.rs @@ -0,0 +1,148 @@ +//! Multi-tab system for CodeWhale TUI +//! +//! This module provides support for multiple concurrent agent sessions +//! in a tabbed interface, similar to Claude Code Windows. + +// Cross-tab collaboration APIs (delegator, meeting, cross_tab, group, +// mention) are intentionally exposed here as a public surface for the +// narrow tab-core harvest. They are not yet wired into the TUI host +// (that lands in a follow-up UI pass) and therefore trip `dead_code` +// inside the binary crate. The `pub use manager::TabManager` re-export +// is the public entry point for that follow-up wiring, so it is also +// marked `unused_imports`-tolerated in the meantime. +#![allow(dead_code, unused_imports)] + +#[cfg(test)] +mod benches; +mod cross_tab; +mod delegator; +pub mod group; +#[cfg(test)] +mod key_e2e; +mod manager; +pub mod meeting; +pub mod mention; +pub mod persistence; + +pub use manager::TabManager; + +use chrono::{DateTime, Utc}; +use serde::{Deserialize, Serialize}; +use std::path::PathBuf; + +/// Unique identifier for a tab +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)] +pub struct TabId(pub u64); + +impl TabId { + pub fn new(id: u64) -> Self { + Self(id) + } +} + +/// Tab type determining the session mode +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, Default)] +pub enum TabType { + /// Regular conversation session + #[default] + Chat, + /// Task delegation session + Delegation, + /// Code review session + Review, + /// Multi-agent meeting session + Meeting, +} + +impl TabType { + /// Short single-character icon for display in tight UI (tab bar, picker) + pub fn icon(&self) -> &'static str { + match self { + TabType::Chat => "💬", + TabType::Delegation => "📤", + TabType::Review => "🔍", + TabType::Meeting => "👥", + } + } + + /// ASCII fallback icon (for terminals without emoji support) + pub fn ascii_icon(&self) -> &'static str { + match self { + TabType::Chat => "[C]", + TabType::Delegation => "[D]", + TabType::Review => "[R]", + TabType::Meeting => "[M]", + } + } + + /// Display name for the tab type + pub fn display_name(&self) -> &'static str { + match self { + TabType::Chat => "Chat", + TabType::Delegation => "Delegation", + TabType::Review => "Review", + TabType::Meeting => "Meeting", + } + } +} + +/// Metadata for a tab +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct TabMetadata { + pub id: TabId, + pub title: String, + pub tab_type: TabType, + pub created_at: DateTime, + pub last_active: DateTime, + pub unread_count: usize, + pub agent_name: Option, + pub session_path: Option, +} + +impl TabMetadata { + pub fn new(id: TabId, title: String, tab_type: TabType) -> Self { + let now = Utc::now(); + Self { + id, + title, + tab_type, + created_at: now, + last_active: now, + unread_count: 0, + agent_name: None, + session_path: None, + } + } + + pub fn touch(&mut self) { + self.last_active = Utc::now(); + } + + pub fn increment_unread(&mut self) { + self.unread_count += 1; + } + + pub fn clear_unread(&mut self) { + self.unread_count = 0; + } +} + +/// Priority levels for task delegation +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize, Default)] +pub enum Priority { + Low = 0, + #[default] + Normal = 1, + High = 2, + Urgent = 3, +} + +/// Status of a tab +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, Default)] +pub enum TabStatus { + Active, + #[default] + Idle, + Loading, + Error, +} diff --git a/crates/tui/src/tui/tab/persistence.rs b/crates/tui/src/tui/tab/persistence.rs new file mode 100644 index 000000000..fc8aa6f4d --- /dev/null +++ b/crates/tui/src/tui/tab/persistence.rs @@ -0,0 +1,375 @@ +//! Tab state persistence +//! +//! Saves the TabManager's tab list (titles, types, IDs, creation time) to a +//! JSON file in the user's data directory. On startup, the file is loaded +//! to restore the previous session's tabs. +//! +//! Note: messages and conversation history are NOT persisted here - those +//! live in the session_manager. This file only stores the tab metadata. + +// WIP collaboration surface — narrow harvest. See `tab/mod.rs` for the +// PR #2753 context. +#![allow(dead_code)] + +use std::path::{Path, PathBuf}; + +use chrono::{DateTime, Utc}; +use serde::{Deserialize, Serialize}; + +use super::{Priority, TabId, TabMetadata, TabType}; + +/// Current schema version. Bump when making breaking changes to the +/// on-disk format. Older versions are detected on load so we can +/// give a useful error message rather than silently dropping data. +pub const CURRENT_SCHEMA_VERSION: u32 = 1; + +/// Maximum size of the tab state file we'll attempt to load (1 MB). +/// Past this, the file is treated as corrupted and ignored. This +/// prevents a malicious or accidental huge file from OOM-ing the TUI +/// on startup. +pub const MAX_FILE_SIZE: u64 = 1024 * 1024; + +/// On-disk representation of a single tab +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct PersistedTab { + pub id: u64, + pub title: String, + pub tab_type: TabType, + pub created_at: DateTime, + pub last_active: DateTime, +} + +/// On-disk representation of a tab group +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct PersistedGroup { + pub id: String, + pub name: String, + pub color: super::group::GroupColor, + pub tab_ids: Vec, + pub created_at: DateTime, +} + +/// On-disk representation of a single delegation task +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct PersistedDelegation { + pub task_id: String, + pub from_tab: u64, + pub to_tab: u64, + pub description: String, + pub priority: Priority, + pub created_at: DateTime, + pub completed_at: Option>, + pub result: Option, + pub was_successful: Option, +} + +/// On-disk representation of the tab manager state +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct PersistedTabState { + pub version: u32, + pub saved_at: DateTime, + pub active_tab_index: Option, + pub tabs: Vec, + pub delegations: Vec, + #[serde(default)] + pub groups: Vec, +} + +impl Default for PersistedTabState { + fn default() -> Self { + Self { + version: 1, + saved_at: Utc::now(), + active_tab_index: None, + tabs: Vec::new(), + delegations: Vec::new(), + groups: Vec::new(), + } + } +} + +/// Get the default path for the tab state file +/// `~/.codewhale/tabs.json` +pub fn default_tab_state_path() -> Option { + let home = std::env::var_os("HOME").or_else(|| std::env::var_os("USERPROFILE"))?; + Some(PathBuf::from(home).join(".codewhale").join("tabs.json")) +} + +/// Save the tab state to a file. +/// Atomically writes via temp file + rename to prevent corruption +/// from interrupted writes. +pub fn save_to_file(state: &PersistedTabState, path: &Path) -> std::io::Result<()> { + if let Some(parent) = path.parent() { + std::fs::create_dir_all(parent)?; + } + let json = serde_json::to_string_pretty(state) + .map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidData, e))?; + // Write to temp file first, then rename for atomicity + let tmp_path = path.with_extension("json.tmp"); + std::fs::write(&tmp_path, json)?; + std::fs::rename(&tmp_path, path)?; + Ok(()) +} + +/// Load the tab state from a file. Returns default state if file doesn't exist. +/// Refuses to load files larger than MAX_FILE_SIZE to prevent OOM. +/// Detects schema version mismatches and returns a specific error. +pub fn load_from_file(path: &Path) -> std::io::Result { + if !path.exists() { + return Ok(PersistedTabState::default()); + } + + // Size check + let metadata = std::fs::metadata(path)?; + if metadata.len() > MAX_FILE_SIZE { + tracing::warn!( + size = metadata.len(), + max = MAX_FILE_SIZE, + path = %path.display(), + "Tab state file too large, ignoring" + ); + return Ok(PersistedTabState::default()); + } + + let content = std::fs::read_to_string(path)?; + let state: PersistedTabState = serde_json::from_str(&content).map_err(|e| { + tracing::error!( + ?e, + path = %path.display(), + "Failed to parse tab state file" + ); + std::io::Error::new(std::io::ErrorKind::InvalidData, e) + })?; + + // Schema version check + if state.version > CURRENT_SCHEMA_VERSION { + tracing::warn!( + file_version = state.version, + current = CURRENT_SCHEMA_VERSION, + "Tab state file is from a newer version; some data may be ignored" + ); + } else if state.version < CURRENT_SCHEMA_VERSION { + tracing::info!( + file_version = state.version, + current = CURRENT_SCHEMA_VERSION, + "Migrating tab state from older schema" + ); + // Future: implement migration logic here + } + + Ok(state) +} + +/// Convert a TabMetadata to its persisted form +pub fn from_metadata(meta: &TabMetadata) -> PersistedTab { + PersistedTab { + id: meta.id.0, + title: meta.title.clone(), + tab_type: meta.tab_type, + created_at: meta.created_at, + last_active: meta.last_active, + } +} + +/// Convert a persisted tab to a TabMetadata +pub fn to_metadata(persisted: &PersistedTab) -> TabMetadata { + let mut meta = TabMetadata::new( + TabId::new(persisted.id), + persisted.title.clone(), + persisted.tab_type, + ); + meta.created_at = persisted.created_at; + meta.last_active = persisted.last_active; + meta +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_roundtrip() { + let state = PersistedTabState { + version: 1, + saved_at: Utc::now(), + active_tab_index: Some(0), + tabs: vec![ + PersistedTab { + id: 1, + title: "Tab 1".to_string(), + tab_type: TabType::Chat, + created_at: Utc::now(), + last_active: Utc::now(), + }, + PersistedTab { + id: 2, + title: "Tab 2".to_string(), + tab_type: TabType::Meeting, + created_at: Utc::now(), + last_active: Utc::now(), + }, + ], + delegations: vec![PersistedDelegation { + task_id: "delegation_1".to_string(), + from_tab: 1, + to_tab: 2, + description: "Review code".to_string(), + priority: Priority::High, + created_at: Utc::now(), + completed_at: None, + result: None, + was_successful: None, + }], + groups: vec![], + }; + + let json = serde_json::to_string(&state).unwrap(); + let parsed: PersistedTabState = serde_json::from_str(&json).unwrap(); + + assert_eq!(parsed.tabs.len(), 2); + assert_eq!(parsed.tabs[0].title, "Tab 1"); + assert_eq!(parsed.tabs[1].tab_type, TabType::Meeting); + assert_eq!(parsed.delegations.len(), 1); + assert_eq!(parsed.delegations[0].priority, Priority::High); + } + + #[test] + fn test_metadata_conversion() { + let meta = TabMetadata::new(TabId::new(42), "Test".to_string(), TabType::Review); + let persisted = from_metadata(&meta); + assert_eq!(persisted.id, 42); + assert_eq!(persisted.title, "Test"); + assert_eq!(persisted.tab_type, TabType::Review); + + let restored = to_metadata(&persisted); + assert_eq!(restored.id, TabId::new(42)); + assert_eq!(restored.title, "Test"); + assert_eq!(restored.tab_type, TabType::Review); + } + + #[test] + fn test_load_missing_file() { + let result = load_from_file(Path::new("/nonexistent/path/tabs.json")); + assert!(result.is_ok()); + let state = result.unwrap(); + assert!(state.tabs.is_empty()); + assert!(state.delegations.is_empty()); + } + + #[test] + fn test_save_and_load() { + let dir = std::env::temp_dir().join("codewhale_tab_test"); + let path = dir.join("tabs.json"); + + // Clean up any leftover + let _ = std::fs::remove_file(&path); + + let state = PersistedTabState { + version: 1, + saved_at: Utc::now(), + active_tab_index: Some(1), + tabs: vec![PersistedTab { + id: 1, + title: "Test".to_string(), + tab_type: TabType::Delegation, + created_at: Utc::now(), + last_active: Utc::now(), + }], + delegations: vec![], + groups: vec![], + }; + + save_to_file(&state, &path).unwrap(); + let loaded = load_from_file(&path).unwrap(); + assert_eq!(loaded.active_tab_index, Some(1)); + assert_eq!(loaded.tabs.len(), 1); + assert_eq!(loaded.tabs[0].tab_type, TabType::Delegation); + + // Cleanup + let _ = std::fs::remove_file(&path); + let _ = std::fs::remove_dir(&dir); + } + + #[test] + fn test_load_oversized_file_rejected() { + // Create a file that exceeds MAX_FILE_SIZE + let dir = std::env::temp_dir().join("codewhale_tab_oversize"); + let path = dir.join("tabs.json"); + std::fs::create_dir_all(&dir).unwrap(); + let _ = std::fs::remove_file(&path); + + // Write a small header followed by enough junk to exceed 1MB + let mut content = String::from( + r#"{"version":1,"saved_at":"2026-01-01T00:00:00Z","active_tab_index":null,"tabs":[],"delegations":[]}"#, + ); + while content.len() < (MAX_FILE_SIZE as usize) + 100 { + content.push(' '); + } + std::fs::write(&path, content).unwrap(); + + // Should return default state, not panic + let loaded = load_from_file(&path).unwrap(); + assert!(loaded.tabs.is_empty()); + + let _ = std::fs::remove_file(&path); + let _ = std::fs::remove_dir(&dir); + } + + #[test] + fn test_load_corrupted_file() { + let dir = std::env::temp_dir().join("codewhale_tab_corrupt"); + let path = dir.join("tabs.json"); + std::fs::create_dir_all(&dir).unwrap(); + let _ = std::fs::remove_file(&path); + + // Write invalid JSON + std::fs::write(&path, "{ not valid json :::").unwrap(); + + // Should return error + let result = load_from_file(&path); + assert!(result.is_err()); + + let _ = std::fs::remove_file(&path); + let _ = std::fs::remove_dir(&dir); + } + + #[test] + fn test_save_is_atomic() { + // Verify that save_to_file uses a temp + rename pattern. + // The test ensures the final file exists and no .tmp file remains. + let dir = std::env::temp_dir().join("codewhale_tab_atomic"); + let path = dir.join("tabs.json"); + let tmp_path = path.with_extension("json.tmp"); + std::fs::create_dir_all(&dir).unwrap(); + let _ = std::fs::remove_file(&path); + let _ = std::fs::remove_file(&tmp_path); + + let state = PersistedTabState::default(); + save_to_file(&state, &path).unwrap(); + + assert!(path.exists(), "Final file should exist"); + assert!(!tmp_path.exists(), "Temp file should be cleaned up"); + + let _ = std::fs::remove_file(&path); + let _ = std::fs::remove_dir(&dir); + } + + #[test] + fn test_load_newer_schema_logs_warning_but_loads() { + // Simulate a file from a future version + let dir = std::env::temp_dir().join("codewhale_tab_newer"); + let path = dir.join("tabs.json"); + std::fs::create_dir_all(&dir).unwrap(); + let _ = std::fs::remove_file(&path); + + let json = r#"{"version":99,"saved_at":"2027-01-01T00:00:00Z","active_tab_index":null,"tabs":[],"delegations":[]}"#; + std::fs::write(&path, json).unwrap(); + + // Should still load successfully (graceful degradation) + let loaded = load_from_file(&path).unwrap(); + assert_eq!(loaded.version, 99); + + let _ = std::fs::remove_file(&path); + let _ = std::fs::remove_dir(&dir); + } +} From 7fcd7d7469ec4b8d80884063f0e7a27bc9a04003 Mon Sep 17 00:00:00 2001 From: G1 Agent Date: Sat, 6 Jun 2026 20:23:51 +0800 Subject: [PATCH 2/3] chore(tui): address Phase 2 bot review on narrow tab-core harvest MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Six fixes for the bot review comments landed on PR #2864 head 649d3990. See phase2-playbook.md §7 for the triage rationale. * persistence.rs: oversized state file now surfaces an InvalidData error instead of silently returning a default. The old behaviour would let the next save overwrite the oversized file and destroy the user's data. Test updated to expect the error. * persistence.rs: PersistedDelegation gains a `status` field so in-flight `InProgress` delegations aren't silently demoted to `Pending` on restart. The snapshot now writes the live status and restore_from_snapshot honours it. Adds a regression test. * mention.rs: resolve_tab_mention no longer sorts its input — tab mentions (@Tab2) must map to the visual order in the tab bar, not to an arbitrary ID sort. Test updated. * manager.rs: `pending_tasks` renamed to `completed_delegations` because the getter returns completed DelegationResults, not in-flight tasks. Docstring points to the in-flight getter `pending_delegations` to avoid the same confusion recurring. * manager.rs: delegate_task and start_meeting now validate that the from/to tab IDs (or all participant IDs) currently exist in the manager. Returns `None` on any unknown ID, preventing orphaned tasks / meetings with stale tab references. Two new regression tests cover both methods. Local CI matrix (Windows runner, flags matching ci.yml): - cargo fmt --all -- --check: exit 0 - cargo clippy --workspace --all-features --locked -- -D warnings: exit 0 - cargo test --workspace --all-features --locked: 4282 pass, 6 fail (the 6 failures are pre-existing on the baseline; not caused by this PR) - git diff --exit-code -- Cargo.lock: exit 0 The three deferred threads (#1 close_tab cleanup, #5 cross_tab_links snapshot, #8 TabGroup::new collision risk) are explicitly out of scope here; they belong to the follow-up collab/UI PR per the narrow-harvest promise to Hmbown. Co-Authored-By: Claude Opus 4.7 --- crates/tui/src/tui/tab/manager.rs | 134 +++++++++++++++++++++++++- crates/tui/src/tui/tab/mention.rs | 23 +++-- crates/tui/src/tui/tab/persistence.rs | 31 ++++-- 3 files changed, 168 insertions(+), 20 deletions(-) diff --git a/crates/tui/src/tui/tab/manager.rs b/crates/tui/src/tui/tab/manager.rs index 736c16a7b..61f280bdd 100644 --- a/crates/tui/src/tui/tab/manager.rs +++ b/crates/tui/src/tui/tab/manager.rs @@ -192,6 +192,7 @@ impl TabManager { to_tab: t.to_tab.0, description: t.description.clone(), priority: t.priority, + status: t.status, created_at: t.created_at, completed_at: t.completed_at, result: t.result.clone(), @@ -254,6 +255,8 @@ impl TabManager { self.next_tab_id = max_seen_id + 1; // Restore active delegations so cross-tab work survives restarts. + // We honour the persisted status (`InProgress` is preserved) so + // work-in-progress isn't silently demoted to `Pending` on restart. for d in &state.delegations { let task = super::delegator::DelegationTask { task_id: d.task_id.clone(), @@ -261,7 +264,7 @@ impl TabManager { to_tab: TabId(d.to_tab), description: d.description.clone(), priority: d.priority, - status: super::delegator::DelegationStatus::Pending, + status: d.status, created_at: d.created_at, deadline: None, completed_at: d.completed_at, @@ -429,8 +432,13 @@ impl TabManager { } } - /// Get pending tasks for a tab - pub fn pending_tasks(&self, id: TabId) -> Vec<&DelegationResult> { + /// Get completed delegation results for a tab. + /// + /// Despite the historical name, this returns **completed** results + /// (via `delegator.results_for_tab`), not in-flight tasks. Use + /// [`Self::pending_delegations`] for tasks that are still pending or + /// in progress. + pub fn completed_delegations(&self, id: TabId) -> Vec<&DelegationResult> { self.delegator.results_for_tab(id) } @@ -463,7 +471,12 @@ impl TabManager { .unwrap_or_default() } - /// Delegate a task from one tab to another + /// Delegate a task from one tab to another. + /// + /// Returns `None` if either the `from` or `to` tab does not currently + /// exist in the manager. This defensive check prevents orphaned + /// delegations from being created with stale tab IDs after a tab + /// has been closed. pub fn delegate_task( &mut self, from: TabId, @@ -471,6 +484,11 @@ impl TabManager { description: String, priority: Priority, ) -> Option { + let has_from = self.tabs.iter().any(|t| t.metadata.id == from); + let has_to = self.tabs.iter().any(|t| t.metadata.id == to); + if !has_from || !has_to { + return None; + } self.delegator .create_delegation(from, to, description, priority) } @@ -506,8 +524,17 @@ impl TabManager { self.delegator.peek_pending_for_tab(tab_id).is_some() } - /// Start a meeting + /// Start a meeting. + /// + /// Returns `None` if any participant tab does not currently exist in + /// the manager. This defensive check prevents meetings from being + /// created with stale tab IDs after a tab has been closed. pub fn start_meeting(&mut self, topic: String, participants: Vec) -> Option { + for p in &participants { + if !self.tabs.iter().any(|t| t.metadata.id == *p) { + return None; + } + } self.meeting_manager.start_meeting(topic, participants) } @@ -873,4 +900,101 @@ mod tests { assert_eq!(group.len(), 1); assert_eq!(group[0].len(), 0); } + + #[test] + fn test_delegate_task_rejects_unknown_tabs() { + use super::super::Priority; + + let mut manager = TabManager::new(); + let from = manager + .create_tab("Source".to_string(), TabType::Chat) + .unwrap(); + + // Unknown `to` tab — should be rejected. + let bogus_to = TabId(9999); + assert!( + manager + .delegate_task(from, bogus_to, "x".to_string(), Priority::Normal) + .is_none() + ); + + // Unknown `from` tab — should be rejected. + let bogus_from = TabId(9998); + let to = manager + .create_tab("Target".to_string(), TabType::Chat) + .unwrap(); + assert!( + manager + .delegate_task(bogus_from, to, "x".to_string(), Priority::Normal) + .is_none() + ); + + // Known tabs — should succeed. + let id = manager.delegate_task(from, to, "real".to_string(), Priority::Normal); + assert!(id.is_some()); + } + + #[test] + fn test_start_meeting_rejects_unknown_participants() { + let mut manager = TabManager::new(); + let a = manager.create_tab("A".to_string(), TabType::Chat).unwrap(); + let _b = manager.create_tab("B".to_string(), TabType::Chat).unwrap(); + let bogus = TabId(4242); + + // Any unknown participant — should be rejected. + assert!( + manager + .start_meeting("topic".to_string(), vec![a, bogus]) + .is_none() + ); + assert!( + manager + .start_meeting("topic".to_string(), vec![bogus]) + .is_none() + ); + + // All-known participants — should succeed. + let meeting_id = manager.start_meeting("topic".to_string(), vec![a, _b]); + assert!(meeting_id.is_some()); + } + + #[test] + fn test_snapshot_restore_preserves_in_progress_status() { + use super::super::Priority; + use super::super::delegator::DelegationStatus; + + let mut manager = TabManager::new(); + let from = manager + .create_tab("Source".to_string(), TabType::Chat) + .unwrap(); + let to = manager + .create_tab("Target".to_string(), TabType::Chat) + .unwrap(); + + // Create a delegation and mark it InProgress (simulating a worker + // that has taken the task but not yet completed it). + let task_id = manager + .delegate_task(from, to, "work".to_string(), Priority::High) + .unwrap(); + manager.delegator.start_task(&task_id); + assert_eq!( + manager.delegator.all_pending()[0].status, + DelegationStatus::InProgress + ); + + // Snapshot and restore on a fresh manager. + let snapshot = manager.snapshot(); + let mut restored = TabManager::new(); + restored.restore_from_snapshot(&snapshot); + + // The in-flight `InProgress` status must survive the restart — + // demoting it to `Pending` would lose work-in-progress. Query + // through `all_pending` (not `pending_delegations`) so the test + // is not coupled to the public read-filter on `pending_for_tab`, + // which intentionally only surfaces not-yet-started tasks. + let restored_tasks = restored.delegator.all_pending(); + assert_eq!(restored_tasks.len(), 1); + assert_eq!(restored_tasks[0].status, DelegationStatus::InProgress); + assert_eq!(restored_tasks[0].task_id, task_id); + } } diff --git a/crates/tui/src/tui/tab/mention.rs b/crates/tui/src/tui/tab/mention.rs index e2825bcbc..eadfb2d4d 100644 --- a/crates/tui/src/tui/tab/mention.rs +++ b/crates/tui/src/tui/tab/mention.rs @@ -151,16 +151,20 @@ fn parse_usize(s: &[u8]) -> Option { /// Given a tab number (1-indexed) and the list of tab IDs, return the /// matching TabId. Returns `None` if the index is out of range. +/// +/// The caller is expected to pass the IDs in **visual order** (i.e. the +/// order they appear in the tab bar). We deliberately do not sort the +/// list here — tab mentions like `@Tab2` should map to the second tab the +/// user sees, not the second-smallest ID. pub fn resolve_tab_mention<'a, I>(tab_number: usize, tab_ids: I) -> Option where I: IntoIterator, { - let mut sorted: Vec = tab_ids.into_iter().copied().collect(); - sorted.sort_unstable(); - if tab_number == 0 || tab_number > sorted.len() { + let ids: Vec = tab_ids.into_iter().copied().collect(); + if tab_number == 0 || tab_number > ids.len() { return None; } - Some(sorted[tab_number - 1]) + Some(ids[tab_number - 1]) } #[cfg(test)] @@ -205,12 +209,13 @@ mod tests { #[test] fn test_resolve_tab_mention() { + // Tab IDs in the visual order they appear in the tab bar. let tab_ids = vec![100, 50, 200]; - // Tab 1 = smallest id (50) - assert_eq!(resolve_tab_mention(1, tab_ids.iter()), Some(50)); - // Tab 2 = middle id (100) - assert_eq!(resolve_tab_mention(2, tab_ids.iter()), Some(100)); - // Tab 3 = largest id (200) + // Tab 1 = first in visual order (100) + assert_eq!(resolve_tab_mention(1, tab_ids.iter()), Some(100)); + // Tab 2 = second in visual order (50) + assert_eq!(resolve_tab_mention(2, tab_ids.iter()), Some(50)); + // Tab 3 = third in visual order (200) assert_eq!(resolve_tab_mention(3, tab_ids.iter()), Some(200)); // Out of range assert_eq!(resolve_tab_mention(4, tab_ids.iter()), None); diff --git a/crates/tui/src/tui/tab/persistence.rs b/crates/tui/src/tui/tab/persistence.rs index fc8aa6f4d..efeecb38c 100644 --- a/crates/tui/src/tui/tab/persistence.rs +++ b/crates/tui/src/tui/tab/persistence.rs @@ -16,6 +16,7 @@ use std::path::{Path, PathBuf}; use chrono::{DateTime, Utc}; use serde::{Deserialize, Serialize}; +use super::delegator::DelegationStatus; use super::{Priority, TabId, TabMetadata, TabType}; /// Current schema version. Bump when making breaking changes to the @@ -57,6 +58,10 @@ pub struct PersistedDelegation { pub to_tab: u64, pub description: String, pub priority: Priority, + /// Status of the delegation when it was snapshotted. Without this field, + /// an in-flight `InProgress` task is silently demoted to `Pending` on + /// restart, losing work-in-progress state. + pub status: DelegationStatus, pub created_at: DateTime, pub completed_at: Option>, pub result: Option, @@ -122,13 +127,23 @@ pub fn load_from_file(path: &Path) -> std::io::Result { // Size check let metadata = std::fs::metadata(path)?; if metadata.len() > MAX_FILE_SIZE { - tracing::warn!( + // Silently returning `default()` would let the next save overwrite + // the oversized file and destroy the user's data. Surface the error + // so the application can refuse to save and preserve the file. + tracing::error!( size = metadata.len(), max = MAX_FILE_SIZE, path = %path.display(), - "Tab state file too large, ignoring" + "Tab state file too large, refusing to load" ); - return Ok(PersistedTabState::default()); + return Err(std::io::Error::new( + std::io::ErrorKind::InvalidData, + format!( + "Tab state file size {} exceeds maximum allowed size {}", + metadata.len(), + MAX_FILE_SIZE + ), + )); } let content = std::fs::read_to_string(path)?; @@ -215,6 +230,7 @@ mod tests { to_tab: 2, description: "Review code".to_string(), priority: Priority::High, + status: DelegationStatus::Pending, created_at: Utc::now(), completed_at: None, result: None, @@ -307,9 +323,12 @@ mod tests { } std::fs::write(&path, content).unwrap(); - // Should return default state, not panic - let loaded = load_from_file(&path).unwrap(); - assert!(loaded.tabs.is_empty()); + // Should return an error rather than silently overwriting the file + // on next save. Silently returning a default would destroy the + // user's data. + let result = load_from_file(&path); + assert!(result.is_err()); + assert_eq!(result.unwrap_err().kind(), std::io::ErrorKind::InvalidData); let _ = std::fs::remove_file(&path); let _ = std::fs::remove_dir(&dir); From ffaf110957c3061cd8c7f8bd91dd11818ef3dec9 Mon Sep 17 00:00:00 2001 From: Hunter B Date: Sat, 6 Jun 2026 10:34:49 -0700 Subject: [PATCH 3/3] fix(tui): advance tab restore counters --- crates/tui/src/tui/tab/delegator.rs | 35 +++++++++++++++ crates/tui/src/tui/tab/group.rs | 33 ++++++++++++++ crates/tui/src/tui/tab/manager.rs | 67 +++++++++++++++++++++++++++++ 3 files changed, 135 insertions(+) diff --git a/crates/tui/src/tui/tab/delegator.rs b/crates/tui/src/tui/tab/delegator.rs index 806a55a3a..9b88557be 100644 --- a/crates/tui/src/tui/tab/delegator.rs +++ b/crates/tui/src/tui/tab/delegator.rs @@ -364,6 +364,17 @@ impl TaskDelegator { self.next_id += 1; format!("delegation_{}", id) } + + pub(crate) fn advance_next_id_past_existing_tasks(&mut self) { + let max_seen = self + .pending_tasks + .iter() + .filter_map(|task| task.task_id.strip_prefix("delegation_")) + .filter_map(|suffix| suffix.parse::().ok()) + .max() + .unwrap_or(0); + self.next_id = self.next_id.max(max_seen + 1); + } } impl Default for TaskDelegator { @@ -539,4 +550,28 @@ mod tests { delegator.prune_results(10); assert_eq!(delegator.results_for_tab(to).len(), 3); } + + #[test] + fn test_advance_next_id_after_restore() { + let mut delegator = TaskDelegator::new(); + delegator.pending_tasks.push(DelegationTask::new( + "delegation_42".to_string(), + TabId::new(1), + TabId::new(2), + "restored".to_string(), + Priority::Normal, + )); + + delegator.advance_next_id_past_existing_tasks(); + let new_id = delegator + .create_delegation( + TabId::new(1), + TabId::new(2), + "fresh".to_string(), + Priority::Normal, + ) + .unwrap(); + + assert_eq!(new_id, "delegation_43"); + } } diff --git a/crates/tui/src/tui/tab/group.rs b/crates/tui/src/tui/tab/group.rs index 715214d88..14095dcee 100644 --- a/crates/tui/src/tui/tab/group.rs +++ b/crates/tui/src/tui/tab/group.rs @@ -239,6 +239,17 @@ impl TabGroupManager { self.next_id += 1; format!("group_{}", id) } + + pub(crate) fn advance_next_id_past_existing_groups(&mut self) { + let max_seen = self + .groups + .keys() + .filter_map(|id| id.strip_prefix("group_")) + .filter_map(|suffix| suffix.parse::().ok()) + .max() + .unwrap_or(0); + self.next_id = self.next_id.max(max_seen + 1); + } } impl Default for TabGroupManager { @@ -347,4 +358,26 @@ mod tests { assert!(mgr.group_of(tab1).is_none()); assert!(mgr.tab_to_group.get(&tab1).is_none()); } + + #[test] + fn test_advance_next_id_after_restore() { + let mut mgr = TabGroupManager::new(); + mgr.groups.insert( + "group_7".to_string(), + TabGroup { + id: "group_7".to_string(), + name: "Restored".to_string(), + color: GroupColor::Blue, + tab_ids: Vec::new(), + created_at: Utc::now(), + }, + ); + + mgr.advance_next_id_past_existing_groups(); + let new_id = mgr.create_group("Fresh".to_string(), GroupColor::Green); + + assert_eq!(new_id, "group_8"); + assert!(mgr.groups.contains_key("group_7")); + assert!(mgr.groups.contains_key("group_8")); + } } diff --git a/crates/tui/src/tui/tab/manager.rs b/crates/tui/src/tui/tab/manager.rs index 61f280bdd..7bf7fb2f7 100644 --- a/crates/tui/src/tui/tab/manager.rs +++ b/crates/tui/src/tui/tab/manager.rs @@ -272,6 +272,7 @@ impl TabManager { }; self.delegator.pending_tasks.push(task); } + self.delegator.advance_next_id_past_existing_tasks(); // Restore groups AFTER tabs so tab_ids can reference real tabs for g in &state.groups { @@ -287,6 +288,7 @@ impl TabManager { self.groups.tab_to_group.insert(*tab_id, g.id.clone()); } } + self.groups.advance_next_id_past_existing_groups(); if let Some(idx) = state.active_tab_index { if idx < self.tabs.len() { @@ -997,4 +999,69 @@ mod tests { assert_eq!(restored_tasks[0].status, DelegationStatus::InProgress); assert_eq!(restored_tasks[0].task_id, task_id); } + + #[test] + fn test_restore_advances_delegation_ids() { + use super::super::Priority; + + let mut manager = TabManager::new(); + let from = manager + .create_tab("Source".to_string(), TabType::Chat) + .unwrap(); + let to = manager + .create_tab("Target".to_string(), TabType::Chat) + .unwrap(); + + let restored_id = manager + .delegate_task(from, to, "restored".to_string(), Priority::Normal) + .unwrap(); + assert_eq!(restored_id, "delegation_1"); + + let snapshot = manager.snapshot(); + let mut restored = TabManager::new(); + restored.restore_from_snapshot(&snapshot); + + let fresh_id = restored + .delegate_task(from, to, "fresh".to_string(), Priority::Normal) + .unwrap(); + assert_eq!(fresh_id, "delegation_2"); + } + + #[test] + fn test_restore_advances_group_ids() { + use super::super::group::GroupColor; + + let mut manager = TabManager::new(); + let tab = manager + .create_tab("Grouped".to_string(), TabType::Chat) + .unwrap(); + let restored_group = manager.create_group("Restored".to_string(), GroupColor::Blue); + assert_eq!(restored_group, "group_1"); + assert!(manager.assign_tab_to_group(tab, &restored_group)); + + let snapshot = manager.snapshot(); + let mut restored = TabManager::new(); + restored.restore_from_snapshot(&snapshot); + + let fresh_group = restored.create_group("Fresh".to_string(), GroupColor::Green); + assert_eq!(fresh_group, "group_2"); + assert!( + restored + .groups() + .all_groups() + .iter() + .any(|g| g.id == "group_1") + ); + assert!( + restored + .groups() + .all_groups() + .iter() + .any(|g| g.id == "group_2") + ); + assert_eq!( + restored.tab_group(tab).map(|g| g.id.as_str()), + Some("group_1") + ); + } }