From 2e73634ab07c2c64c9538242c762802128557332 Mon Sep 17 00:00:00 2001
From: Hunter Bown <hmbown@gmail.com>
Date: Wed, 27 May 2026 04:45:40 -0500
Subject: [PATCH] fix(tui): make sidebar failure clearing order-aware

---
 crates/tui/src/tui/sidebar.rs | 139 ++++++++++++++++++++++++++++------
 1 file changed, 114 insertions(+), 25 deletions(-)

diff --git a/crates/tui/src/tui/sidebar.rs b/crates/tui/src/tui/sidebar.rs
index c28c5a83..da99f31f 100644
--- a/crates/tui/src/tui/sidebar.rs
+++ b/crates/tui/src/tui/sidebar.rs
@@ -770,7 +770,7 @@ fn active_tool_rows(app: &App) -> Vec<SidebarToolRow> {
     if !stale_running.is_empty() {
         rows.push(collapsed_stale_running_row(stale_running));
     }
-    editorial_tool_rows(rows, usize::MAX)
+    editorial_tool_rows(rows, usize::MAX, ToolRowOrder::OldestFirst)
 }
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
@@ -837,7 +837,7 @@ fn recent_tool_rows(app: &App, limit: usize) -> Vec<SidebarToolRow> {
         .filter_map(sidebar_tool_row_from_cell)
         .take(RECENT_TOOL_SCAN_LIMIT)
         .collect();
-    editorial_tool_rows(rows, limit)
+    editorial_tool_rows(rows, limit, ToolRowOrder::NewestFirst)
 }
 
 fn push_tool_rows(
@@ -1142,7 +1142,17 @@ fn background_task_duplicates_live_tool(
         })
 }
 
-fn editorial_tool_rows(rows: Vec<SidebarToolRow>, limit: usize) -> Vec<SidebarToolRow> {
+#[derive(Clone, Copy, Debug, PartialEq, Eq)]
+enum ToolRowOrder {
+    OldestFirst,
+    NewestFirst,
+}
+
+fn editorial_tool_rows(
+    rows: Vec<SidebarToolRow>,
+    limit: usize,
+    order_mode: ToolRowOrder,
+) -> Vec<SidebarToolRow> {
     #[derive(Clone)]
     struct Candidate {
         rank: u8,
@@ -1155,7 +1165,7 @@ fn editorial_tool_rows(rows: Vec<SidebarToolRow>, limit: usize) -> Vec<SidebarTo
     let mut ci_poll_groups: Vec<(usize, SidebarToolRow, usize)> = Vec::new();
     let mut shell_wait_groups: Vec<(usize, SidebarToolRow, usize, String)> = Vec::new();
     let mut seen_success: Vec<String> = Vec::new();
-    let mut seen_tool_names_succeeded: Vec<String> = Vec::new();
+    let mut seen_success_tool_names: Vec<String> = Vec::new();
     let mut seen_failures: Vec<String> = Vec::new();
     let mut visible_failure_count: usize = 0;
     const MAX_VISIBLE_FAILURES: usize = 2;
@@ -1166,6 +1176,11 @@ fn editorial_tool_rows(rows: Vec<SidebarToolRow>, limit: usize) -> Vec<SidebarTo
             // recent failure per tool. Fixes #1884 — stale failures from
             // tools that have since succeeded no longer crowd the sidebar.
             let fail_key = row.name.trim().to_ascii_lowercase();
+            if order_mode == ToolRowOrder::NewestFirst
+                && seen_success_tool_names.contains(&fail_key)
+            {
+                continue;
+            }
             if seen_failures.contains(&fail_key) {
                 continue;
             }
@@ -1226,20 +1241,34 @@ fn editorial_tool_rows(rows: Vec<SidebarToolRow>, limit: usize) -> Vec<SidebarTo
         if row.status == ToolStatus::Success {
             seen_success.push(key);
             let normalized = row.name.trim().to_ascii_lowercase();
-            if !seen_tool_names_succeeded.contains(&normalized) {
-                seen_tool_names_succeeded.push(normalized);
+            if !seen_success_tool_names.contains(&normalized) {
+                seen_success_tool_names.push(normalized.clone());
             }
-        }
 
-        // When a tool succeeds, remove any prior failure for the same
-        // tool name. Prevents stale `gh issue create (failed)` from
-        // lingering after a successful retry. (#1884)
-        if row.status == ToolStatus::Success {
-            let normalized = row.name.trim().to_ascii_lowercase();
-            candidates.retain(|c| {
-                c.row.name.trim().to_ascii_lowercase() != normalized
-                    || c.row.status != ToolStatus::Failed
-            });
+            // Active rows are oldest-first, so a success means any candidate
+            // failure for the same tool is stale. Recent history rows are
+            // newest-first; in that path the success is older than any
+            // already-seen failure and must not remove it.
+            if order_mode == ToolRowOrder::OldestFirst {
+                let mut removed_visible_failures = 0usize;
+                let mut removed_any_failure = false;
+                candidates.retain(|c| {
+                    let remove = c.row.status == ToolStatus::Failed
+                        && c.row.name.trim().eq_ignore_ascii_case(&normalized);
+                    if remove {
+                        removed_any_failure = true;
+                        if c.rank == 0 {
+                            removed_visible_failures += 1;
+                        }
+                    }
+                    !remove
+                });
+                if removed_any_failure {
+                    seen_failures.retain(|seen| seen != &normalized);
+                    visible_failure_count =
+                        visible_failure_count.saturating_sub(removed_visible_failures);
+                }
+            }
         }
 
         // Cap visible failures at MAX_VISIBLE_FAILURES. Excess failures
@@ -1247,7 +1276,6 @@ fn editorial_tool_rows(rows: Vec<SidebarToolRow>, limit: usize) -> Vec<SidebarTo
         // sidebar. (#1884)
         let rank = if row.status == ToolStatus::Failed {
             if visible_failure_count >= MAX_VISIBLE_FAILURES {
-                visible_failure_count += 1;
                 3
             } else {
                 visible_failure_count += 1;
@@ -1257,11 +1285,7 @@ fn editorial_tool_rows(rows: Vec<SidebarToolRow>, limit: usize) -> Vec<SidebarTo
             tool_row_rank(&row)
         };
 
-        candidates.push(Candidate {
-            rank,
-            order,
-            row,
-        });
+        candidates.push(Candidate { rank, order, row });
     }
 
     for (order, mut row, count) in ci_poll_groups {
@@ -1925,9 +1949,9 @@ mod tests {
     use super::{
         ACTIVE_TOOL_COMPLETED_ROW_TTL, ACTIVE_TOOL_STALE_RUNNING_ROW_TTL, AutoSidebarPanel,
         AutoSidebarState, SidebarAgentRow, SidebarHoverSection, SidebarHoverState,
-        SidebarSubagentSummary, SidebarWorkChecklistItem, SidebarWorkStrategyStep,
-        SidebarWorkSummary, auto_sidebar_panels, subagent_panel_lines, task_panel_lines,
-        work_panel_empty_hint, work_panel_lines,
+        SidebarSubagentSummary, SidebarToolRow, SidebarWorkChecklistItem, SidebarWorkStrategyStep,
+        SidebarWorkSummary, ToolRowOrder, auto_sidebar_panels, editorial_tool_rows,
+        subagent_panel_lines, task_panel_lines, work_panel_empty_hint, work_panel_lines,
     };
     use crate::config::Config;
     use crate::palette::PaletteMode;
@@ -1967,6 +1991,15 @@ mod tests {
         App::new(options, &Config::default())
     }
 
+    fn sidebar_tool_row(name: &str, status: ToolStatus) -> SidebarToolRow {
+        SidebarToolRow {
+            name: name.to_string(),
+            status,
+            summary: String::new(),
+            duration_ms: None,
+        }
+    }
+
     fn lines_to_text(lines: &[Line<'static>]) -> Vec<String> {
         lines
             .iter()
@@ -1979,6 +2012,62 @@ mod tests {
             .collect()
     }
 
+    #[test]
+    fn editorial_rows_keep_newer_failure_when_older_success_is_seen_later() {
+        let rows = vec![
+            sidebar_tool_row("gh issue create", ToolStatus::Failed),
+            sidebar_tool_row("gh issue create", ToolStatus::Success),
+        ];
+
+        let rendered = editorial_tool_rows(rows, 4, ToolRowOrder::NewestFirst);
+
+        assert!(
+            rendered
+                .iter()
+                .any(|row| row.name == "gh issue create" && row.status == ToolStatus::Failed),
+            "newest-first rows must keep a failure newer than a later-seen success: {rendered:?}"
+        );
+    }
+
+    #[test]
+    fn editorial_rows_hide_older_failure_after_newer_success() {
+        let rows = vec![
+            sidebar_tool_row("gh issue create", ToolStatus::Success),
+            sidebar_tool_row("gh issue create", ToolStatus::Failed),
+        ];
+
+        let rendered = editorial_tool_rows(rows, 4, ToolRowOrder::NewestFirst);
+
+        assert!(
+            !rendered
+                .iter()
+                .any(|row| row.name == "gh issue create" && row.status == ToolStatus::Failed),
+            "newest-first rows should hide stale failures older than success: {rendered:?}"
+        );
+    }
+
+    #[test]
+    fn editorial_rows_reclaim_failure_slot_after_oldest_first_success() {
+        let rows = vec![
+            sidebar_tool_row("gh issue create", ToolStatus::Failed),
+            sidebar_tool_row("grep_files", ToolStatus::Failed),
+            sidebar_tool_row("gh issue create", ToolStatus::Success),
+            sidebar_tool_row("cargo test", ToolStatus::Failed),
+        ];
+
+        let rendered = editorial_tool_rows(rows, 2, ToolRowOrder::OldestFirst);
+
+        assert_eq!(
+            rendered
+                .iter()
+                .filter(|row| row.status == ToolStatus::Failed)
+                .map(|row| row.name.as_str())
+                .collect::<Vec<_>>(),
+            vec!["grep_files", "cargo test"],
+            "success should clear its stale failure and free a visible failure slot"
+        );
+    }
+
     #[test]
     fn auto_sidebar_does_not_reserve_empty_work_when_other_panels_are_active() {
         let panels = auto_sidebar_panels(AutoSidebarState {