From 5a71d644f54ae4f44efa8d15d818c50bf04ca218 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 11 Jun 2026 02:13:30 +0000 Subject: [PATCH] =?UTF-8?q?fix(tui):=20#3030=20audit=20fixes=20=E2=80=94?= =?UTF-8?q?=20nickname=20beats=20generated=20Agent-N=20label;=20status=20b?= =?UTF-8?q?ar=20uses=20stable=20labels=20(with=20raw-id=20fallback)=20for?= =?UTF-8?q?=20spawn/progress/complete;=20drop=20truncated=20raw=20id=20fro?= =?UTF-8?q?m=20compact=20detail=20line;=20add=20label/turn/step-counter=20?= =?UTF-8?q?tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude https://claude.ai/code/session_018zaP8vUfTAsrE38L6h6fw5 --- crates/tui/src/tools/subagent/tests.rs | 15 +++ crates/tui/src/tui/app.rs | 22 +++++ crates/tui/src/tui/sidebar.rs | 123 +++++++++++++++++++++++-- crates/tui/src/tui/ui.rs | 21 +++-- 4 files changed, 162 insertions(+), 19 deletions(-) diff --git a/crates/tui/src/tools/subagent/tests.rs b/crates/tui/src/tools/subagent/tests.rs index 113ea47a..37241283 100644 --- a/crates/tui/src/tools/subagent/tests.rs +++ b/crates/tui/src/tools/subagent/tests.rs @@ -3173,3 +3173,18 @@ fn model_catalog_only_advertises_canonical_subagent_tools() { ); } } + +// ── #3030: step-counter formatting ────────────────────────────────────────── + +#[test] +fn format_step_counter_hides_unbounded_sentinel() { + // DEFAULT_MAX_STEPS is u32::MAX, meaning "unbounded" — rendering the + // sentinel as a denominator produced "step 16/4294967295". + assert_eq!(format_step_counter(16, u32::MAX), "step 16"); +} + +#[test] +fn format_step_counter_keeps_concrete_budgets() { + assert_eq!(format_step_counter(3, 25), "step 3/25"); + assert_eq!(format_step_counter(0, 1), "step 0/1"); +} diff --git a/crates/tui/src/tui/app.rs b/crates/tui/src/tui/app.rs index a2d597ec..e7cc6d2f 100644 --- a/crates/tui/src/tui/app.rs +++ b/crates/tui/src/tui/app.rs @@ -2740,6 +2740,28 @@ impl App { self.collapsed_cell_map.clear(); } + /// #3030: return the stable user-facing label for an agent id + /// ("Agent 3"), assigning the next sequential label on first sight. + pub(crate) fn ensure_agent_label(&mut self, agent_id: &str) -> String { + if let Some(label) = self.agent_label_map.get(agent_id) { + return label.clone(); + } + self.agent_counter = self.agent_counter.saturating_add(1); + let label = format!("Agent {}", self.agent_counter); + self.agent_label_map + .insert(agent_id.to_string(), label.clone()); + label + } + + /// #3030: read-only label lookup with raw-id fallback for agents the + /// label map has never seen. + pub(crate) fn agent_display_label(&self, agent_id: &str) -> String { + self.agent_label_map + .get(agent_id) + .cloned() + .unwrap_or_else(|| agent_id.to_string()) + } + pub fn mark_history_updated(&mut self) { self.history_version = self.history_version.wrapping_add(1); // Resync per-cell revisions to history.len(). This is the diff --git a/crates/tui/src/tui/sidebar.rs b/crates/tui/src/tui/sidebar.rs index 198fbffb..7eb26db0 100644 --- a/crates/tui/src/tui/sidebar.rs +++ b/crates/tui/src/tui/sidebar.rs @@ -1835,12 +1835,13 @@ fn sidebar_agent_rows(app: &App) -> Vec { .map(summarize_tool_output) .filter(|summary| !summary.trim().is_empty()) }); - // #3030: Prefer stable label ("Agent 1") > nickname > raw name. - let display_name = app - .agent_label_map - .get(&agent.agent_id) - .cloned() - .or_else(|| agent.nickname.clone()) + // #3030: Prefer the user-assigned nickname > stable label + // ("Agent 1") > raw name. Every spawned agent gets a label-map + // entry, so the generated label must not shadow nicknames. + let display_name = agent + .nickname + .clone() + .or_else(|| app.agent_label_map.get(&agent.agent_id).cloned()) .unwrap_or_else(|| agent.name.clone()); SidebarAgentRow { id: agent.agent_id.clone(), @@ -1981,8 +1982,9 @@ pub fn subagent_panel_lines( if lines.len() >= max_rows { break; } + // #3030: keep raw agent ids out of the compact detail line — the + // full id remains available in the hover text. let mut detail_parts = Vec::new(); - detail_parts.push(truncate_line_to_width(&row.id, 10)); if row.steps_taken > 0 { detail_parts.push(format!("{} step(s)", row.steps_taken)); } @@ -1997,6 +1999,9 @@ pub fn subagent_panel_lines( if let Some(duration) = row.duration_ms { detail_parts.push(format_duration_ms(duration)); } + if detail_parts.is_empty() { + detail_parts.push(row.status.clone()); + } lines.push(Line::from(Span::styled( format!( " {}", @@ -2371,8 +2376,8 @@ mod tests { SidebarSubagentSummary, SidebarToolRow, SidebarWorkChecklistItem, SidebarWorkStrategyStep, SidebarWorkSummary, ToolRowOrder, auto_sidebar_panels, editorial_tool_rows, normalize_activity_text, sidebar_hover_rows, sidebar_work_summary, - subagent_panel_hover_texts, subagent_panel_lines, task_panel_lines, work_panel_empty_hint, - work_panel_hover_texts, work_panel_lines, + subagent_panel_hover_texts, subagent_panel_lines, task_panel_hover_texts, task_panel_lines, + work_panel_empty_hint, work_panel_hover_texts, work_panel_lines, }; use crate::config::Config; use crate::palette; @@ -3530,4 +3535,104 @@ mod tests { "hover text should include the full progress before popover wrapping: {hover:?}" ); } + + // ── #3030: stable labels instead of raw internal ids ─────────────────── + + #[test] + fn tasks_panel_shows_stable_turn_label_not_uuid() { + let mut app = create_test_app(); + app.runtime_turn_id = Some("0196f0a3-1111-2222-3333-444455556666".to_string()); + app.runtime_turn_status = Some("in_progress".to_string()); + app.turn_counter = 3; + + let text = lines_to_text(&task_panel_lines(&app, 64, 8)); + assert!( + text[0].contains("Turn 3 (in_progress)"), + "compact row must show the stable turn label: {text:?}" + ); + assert!( + !text[0].contains("0196f0a3"), + "raw turn UUID must stay out of the compact row: {text:?}" + ); + + let hover = task_panel_hover_texts(&app, 8); + assert!( + hover[0].contains("0196f0a3-1111-2222-3333-444455556666"), + "full turn UUID must remain available in hover text: {hover:?}" + ); + } + + #[test] + fn tasks_panel_turn_label_falls_back_before_first_counted_turn() { + let mut app = create_test_app(); + app.runtime_turn_id = Some("0196f0a3-1111-2222-3333-444455556666".to_string()); + app.runtime_turn_status = Some("in_progress".to_string()); + app.turn_counter = 0; + + let text = lines_to_text(&task_panel_lines(&app, 64, 8)); + assert!( + text[0].contains("Current turn (in_progress)"), + "zero counter falls back to a generic label: {text:?}" + ); + } + + #[test] + fn ensure_agent_label_assigns_stable_sequential_labels() { + let mut app = create_test_app(); + assert_eq!(app.ensure_agent_label("agent_aaa111"), "Agent 1"); + assert_eq!(app.ensure_agent_label("agent_bbb222"), "Agent 2"); + // Re-seeing a known agent keeps its original label. + assert_eq!(app.ensure_agent_label("agent_aaa111"), "Agent 1"); + assert_eq!(app.agent_counter, 2); + // Read-only lookup falls back to the raw id for unknown agents. + assert_eq!(app.agent_display_label("agent_bbb222"), "Agent 2"); + assert_eq!(app.agent_display_label("agent_zzz999"), "agent_zzz999"); + } + + fn cached_agent( + agent_id: &str, + nickname: Option<&str>, + ) -> crate::tools::subagent::SubAgentResult { + crate::tools::subagent::SubAgentResult { + name: "implementation-worker".to_string(), + agent_id: agent_id.to_string(), + context_mode: "fresh".to_string(), + fork_context: false, + workspace: None, + git_branch: None, + agent_type: crate::tools::subagent::SubAgentType::General, + assignment: crate::tools::subagent::SubAgentAssignment { + objective: "task".to_string(), + role: Some("worker".to_string()), + }, + model: String::new(), + nickname: nickname.map(str::to_string), + status: crate::tools::subagent::SubAgentStatus::Running, + result: None, + steps_taken: 1, + checkpoint: None, + duration_ms: 100, + from_prior_session: false, + } + } + + #[test] + fn sidebar_agent_rows_prefer_nickname_over_generated_label() { + let mut app = create_test_app(); + let agent_id = "agent_cafe0123"; + app.ensure_agent_label(agent_id); + app.subagent_cache + .push(cached_agent(agent_id, Some("doc-fixer"))); + + let rows = super::sidebar_agent_rows(&app); + assert_eq!( + rows[0].name, "doc-fixer", + "user nickname must beat the generated Agent-N label" + ); + + // Without a nickname the generated label is used. + app.subagent_cache[0].nickname = None; + let rows = super::sidebar_agent_rows(&app); + assert_eq!(rows[0].name, "Agent 1"); + } } diff --git a/crates/tui/src/tui/ui.rs b/crates/tui/src/tui/ui.rs index 8ed830cb..e1ff54d2 100644 --- a/crates/tui/src/tui/ui.rs +++ b/crates/tui/src/tui/ui.rs @@ -2302,14 +2302,10 @@ async fn run_event_loop( if app.agent_activity_started_at.is_none() { app.agent_activity_started_at = Some(Instant::now()); } - // #3030: Assign a stable user-facing label for this agent. - if !app.agent_label_map.contains_key(&id) { - app.agent_counter = app.agent_counter.saturating_add(1); - app.agent_label_map - .insert(id.clone(), format!("Agent {}", app.agent_counter)); - } - app.status_message = - Some(format!("Sub-agent {id} starting: {prompt_summary}")); + // #3030: Assign a stable user-facing label for this + // agent and keep the raw id out of the status bar. + let label = app.ensure_agent_label(&id); + app.status_message = Some(format!("{label} starting: {prompt_summary}")); let _ = engine_handle.send(Op::ListSubAgents).await; } EngineEvent::AgentProgress { id, status } => { @@ -2324,7 +2320,10 @@ async fn run_event_loop( if app.agent_activity_started_at.is_none() { app.agent_activity_started_at = Some(Instant::now()); } - app.status_message = Some(format!("Sub-agent {id}: {display}")); + // #3030: progress can arrive before AgentSpawned is + // observed — assign the stable label on first sight. + let label = app.ensure_agent_label(&id); + app.status_message = Some(format!("{label}: {display}")); } EngineEvent::AgentComplete { id, result } => { execute_subagent_observer_hook( @@ -2346,8 +2345,10 @@ async fn run_event_loop( && matches!(agent.status, SubAgentStatus::Running) }); app.agent_progress.remove(&id); + // #3030: stable label with raw-id fallback. + let label = app.agent_display_label(&id); app.status_message = Some(format!( - "Sub-agent {id} completed: {}", + "{label} completed: {}", summarize_tool_output(&result) )); let should_recapture_terminal =