refactor(todo): switch to content-keyed incremental updates with cancelled status (#2606)

Author: amitksingh1490

crates/forge_app/src/fmt/snapshots/forge_app__fmt__todo_fmt__tests__todo_write_mixed_changes_snapshot.snap

@@ -4,6 +4,5 @@ expression: actual
 ---
 
   󰄵 Task 1
-  󰄱 Task 2
+  󰄗 Task 2
   󰄱 Task 3
-

crates/forge_app/src/fmt/snapshots/forge_app__fmt__todo_fmt__tests__todo_write_removed_in_progress_renders_with_in_progress_icon_in_raw_snapshot.snap

@@ -0,0 +1,6 @@
+---
+source: crates/forge_app/src/fmt/todo_fmt.rs
+expression: raw
+---
+
+  󰄗 Write migrations

crates/forge_app/src/fmt/snapshots/forge_app__fmt__todo_fmt__tests__todo_write_removed_pending_renders_with_pending_icon_in_raw_snapshot.snap

@@ -0,0 +1,6 @@
+---
+source: crates/forge_app/src/fmt/todo_fmt.rs
+expression: raw
+---
+
+  󰄱 Pending task

crates/forge_app/src/fmt/todo_fmt.rs

@@ -21,17 +21,22 @@ fn format_todo_line(todo: &Todo, line_style: TodoLineStyle) -> String {
         TodoStatus::Completed => "󰄵",
         TodoStatus::InProgress => "󰄗",
         TodoStatus::Pending => "󰄱",
+        TodoStatus::Cancelled => "󰅙",
     };
 
     let content = match todo.status {
-        TodoStatus::Completed => style(todo.content.as_str()).strikethrough().to_string(),
+        TodoStatus::Completed | TodoStatus::Cancelled => {
+            style(todo.content.as_str()).strikethrough().to_string()
+        }
         _ => todo.content.clone(),
     };
 
     let line = format!("  {checkbox} {content}");
     let styled = match (&todo.status, line_style) {
         (TodoStatus::Pending, TodoLineStyle::Bold) => style(line).white().bold().to_string(),
         (TodoStatus::Pending, TodoLineStyle::Dim) => style(line).white().dim().to_string(),
+        (TodoStatus::Cancelled, TodoLineStyle::Bold) => style(line).red().bold().to_string(),
+        (TodoStatus::Cancelled, TodoLineStyle::Dim) => style(line).red().dim().to_string(),
         (TodoStatus::InProgress, TodoLineStyle::Bold) => style(line).cyan().bold().to_string(),
         (TodoStatus::InProgress, TodoLineStyle::Dim) => style(line).cyan().dim().to_string(),
         (TodoStatus::Completed, TodoLineStyle::Bold) => style(line).green().bold().to_string(),
@@ -53,72 +58,56 @@ pub(crate) fn format_todos_diff(before: &[Todo], after: &[Todo]) -> String {
 
     let before_map: std::collections::HashMap<&str, &Todo> =
         before.iter().map(|todo| (todo.id.as_str(), todo)).collect();
-    let after_ids: std::collections::HashSet<&str> =
-        after.iter().map(|todo| todo.id.as_str()).collect();
+    let after_map: std::collections::HashMap<&str, &Todo> =
+        after.iter().map(|todo| (todo.id.as_str(), todo)).collect();
 
     let mut result = "\n".to_string();
 
-    enum DiffLine<'a> {
-        Current {
-            todo: &'a Todo,
-            line_style: TodoLineStyle,
-        },
-        Removed {
-            todo: &'a Todo,
-        },
-    }
-
-    impl DiffLine<'_> {
-        fn id(&self) -> &str {
-            match self {
-                DiffLine::Current { todo, .. } | DiffLine::Removed { todo } => todo.id.as_str(),
+    // Walk `before` in insertion order: emit the current version of surviving
+    // items, or the removed rendering for items that were dropped.
+    for before_todo in before {
+        if let Some(after_todo) = after_map.get(before_todo.id.as_str()).copied() {
+            // Item still exists — render with bold/dim based on whether it changed.
+            let is_changed = before_todo.status != after_todo.status
+                || before_todo.content != after_todo.content;
+            let line_style = if is_changed {
+                TodoLineStyle::Bold
+            } else {
+                TodoLineStyle::Dim
+            };
+            result.push_str(&format_todo_line(after_todo, line_style));
+        } else {
+            // Item was removed — render with status-aware styling.
+            let content = style(before_todo.content.as_str())
+                .strikethrough()
+                .to_string();
+            if before_todo.status == TodoStatus::Completed {
+                // Removed completed: dimmed white checkmark (historical done)
+                result.push_str(&format!(
+                    "  {}\n",
+                    style(format!("󰄵 {content}")).white().dim()
+                ));
+            } else {
+                // Removed non-completed: use the correct status icon in red
+                let checkbox = match before_todo.status {
+                    TodoStatus::InProgress => "󰄗",
+                    TodoStatus::Pending => "󰄱",
+                    TodoStatus::Cancelled => "󰅙",
+                    TodoStatus::Completed => "󰄵",
+                };
+                result.push_str(&format!(
+                    "  {}\n",
+                    style(format!("{checkbox} {content}")).red()
+                ));
             }
         }
     }
 
-    let mut lines: Vec<DiffLine<'_>> = Vec::new();
-
+    // Append newly-added items (present in `after` but not in `before`) in
+    // their original insertion order.
     for todo in after {
-        let previous = before_map.get(todo.id.as_str()).copied();
-        let is_new = previous.is_none();
-        let is_changed = previous
-            .map(|item| item.status != todo.status || item.content != todo.content)
-            .unwrap_or(false);
-
-        let line_style = if is_new || is_changed {
-            TodoLineStyle::Bold
-        } else {
-            TodoLineStyle::Dim
-        };
-
-        lines.push(DiffLine::Current { todo, line_style });
-    }
-
-    for todo in before {
-        if !after_ids.contains(todo.id.as_str()) {
-            lines.push(DiffLine::Removed { todo });
-        }
-    }
-
-    lines.sort_by(|left, right| left.id().cmp(right.id()));
-
-    for line in lines {
-        match line {
-            DiffLine::Current { todo, line_style } => {
-                result.push_str(&format_todo_line(todo, line_style));
-            }
-            DiffLine::Removed { todo } => {
-                let content = style(todo.content.as_str()).strikethrough().to_string();
-
-                if todo.status == TodoStatus::Completed {
-                    result.push_str(&format!(
-                        "  {}\n",
-                        style(format!("󰄵 {content}")).white().dim()
-                    ));
-                } else {
-                    result.push_str(&format!("  {}\n", style(format!("󰄱 {content}")).red()));
-                }
-            }
+        if !before_map.contains_key(todo.id.as_str()) {
+            result.push_str(&format_todo_line(todo, TodoLineStyle::Bold));
         }
     }
 
@@ -137,10 +126,7 @@ pub(crate) fn format_todos(todos: &[Todo]) -> String {
 
     let mut result = "\n".to_string();
 
-    let mut sorted_todos: Vec<&Todo> = todos.iter().collect();
-    sorted_todos.sort_by(|left, right| left.id.cmp(&right.id));
-
-    for todo in sorted_todos {
+    for todo in todos {
         result.push_str(&format_todo_line(todo, TodoLineStyle::Dim));
     }
 
@@ -149,14 +135,43 @@ pub(crate) fn format_todos(todos: &[Todo]) -> String {
 
 #[cfg(test)]
 mod tests {
-    use console::strip_ansi_codes;
+    use std::sync::Mutex;
+
+    use console::{
+        colors_enabled, colors_enabled_stderr, set_colors_enabled, set_colors_enabled_stderr,
+        strip_ansi_codes,
+    };
     use forge_domain::{ChatResponseContent, Environment, Todo, TodoStatus};
     use insta::assert_snapshot;
     use pretty_assertions::assert_eq;
 
     use crate::fmt::content::FormatContent;
     use crate::operation::ToolOperation;
 
+    static ANSI_STYLE_LOCK: Mutex<()> = Mutex::new(());
+
+    struct ColorStateGuard {
+        stdout: bool,
+        stderr: bool,
+    }
+
+    impl ColorStateGuard {
+        fn force_enabled() -> Self {
+            let stdout = colors_enabled();
+            let stderr = colors_enabled_stderr();
+            set_colors_enabled(true);
+            set_colors_enabled_stderr(true);
+            Self { stdout, stderr }
+        }
+    }
+
+    impl Drop for ColorStateGuard {
+        fn drop(&mut self) {
+            set_colors_enabled(self.stdout);
+            set_colors_enabled_stderr(self.stderr);
+        }
+    }
+
     fn fixture_environment() -> Environment {
         use fake::{Fake, Faker};
 
@@ -178,6 +193,58 @@ mod tests {
         Todo::new(content).id(id).status(status)
     }
 
+    fn fixture_todo_write_output_raw(before: Vec<Todo>, after: Vec<Todo>) -> String {
+        let _lock = ANSI_STYLE_LOCK
+            .lock()
+            .expect("ANSI style lock should not be poisoned");
+        let _colors = ColorStateGuard::force_enabled();
+        let setup = ToolOperation::TodoWrite { before, after };
+        let actual = setup.to_content(&fixture_environment());
+
+        if let Some(ChatResponseContent::ToolOutput(output)) = actual {
+            output
+        } else {
+            panic!("Expected ToolOutput content")
+        }
+    }
+
+    #[test]
+    fn test_todo_write_removed_in_progress_renders_with_in_progress_icon_in_raw_snapshot() {
+        // before: Write migrations is in_progress
+        // after:  empty (it was cancelled/removed)
+        let setup = (
+            vec![fixture_todo(
+                "Write migrations",
+                "1",
+                TodoStatus::InProgress,
+            )],
+            Vec::new(),
+        );
+
+        // Verify icon (strip color) — must be 󰄗, NOT 󰄱
+        let plain = fixture_todo_write_output(setup.0.clone(), setup.1.clone());
+        let expected_plain = "\n  󰄗 Write migrations\n";
+        assert_eq!(plain, expected_plain);
+
+        let raw = fixture_todo_write_output_raw(setup.0, setup.1);
+        assert_snapshot!(raw);
+    }
+
+    #[test]
+    fn test_todo_write_removed_pending_renders_with_pending_icon_in_raw_snapshot() {
+        let setup = (
+            vec![fixture_todo("Pending task", "1", TodoStatus::Pending)],
+            Vec::new(),
+        );
+
+        let plain = fixture_todo_write_output(setup.0.clone(), setup.1.clone());
+        let expected_plain = "\n  󰄱 Pending task\n";
+        assert_eq!(plain, expected_plain);
+
+        let raw = fixture_todo_write_output_raw(setup.0, setup.1);
+        assert_snapshot!(raw);
+    }
+
     fn fixture_todo_write_output(before: Vec<Todo>, after: Vec<Todo>) -> String {
         let setup = ToolOperation::TodoWrite { before, after };
         let actual = setup.to_content(&fixture_environment());
@@ -220,14 +287,16 @@ mod tests {
     }
 
     #[test]
-    fn test_format_todos_are_sorted_by_id() {
+    fn test_format_todos_preserves_insertion_order() {
+        // Items are given in insertion order: Second was added first, First second.
+        // Output must reflect that insertion order, not alphabetical or id-sorted.
         let setup = vec![
             fixture_todo("Second", "2", TodoStatus::Pending),
             fixture_todo("First", "1", TodoStatus::Pending),
         ];
 
         let actual = strip_ansi_codes(super::format_todos(&setup).as_str()).to_string();
-        let expected = "\n  󰄱 First\n  󰄱 Second\n";
+        let expected = "\n  󰄱 Second\n  󰄱 First\n";
 
         assert_eq!(actual, expected);
     }

crates/forge_app/src/operation.rs

@@ -655,7 +655,6 @@ impl ToolOperation {
 
                 for todo in added {
                     let todo_elm = Element::new("todo")
-                        .attr("id", &todo.id)
                         .attr("status", todo.status.to_string())
                         .attr("change", "added")
                         .text(&todo.content);
@@ -664,7 +663,6 @@ impl ToolOperation {
 
                 for (prev, todo) in updated {
                     let mut todo_elm = Element::new("todo")
-                        .attr("id", &todo.id)
                         .attr("status", todo.status.to_string())
                         .attr("change", "updated");
                     if prev.status != todo.status {
@@ -678,7 +676,6 @@ impl ToolOperation {
 
                 for todo in removed {
                     let todo_elm = Element::new("todo")
-                        .attr("id", &todo.id)
                         .attr("status", todo.status.to_string())
                         .attr("change", "removed")
                         .text(&todo.content);
@@ -692,7 +689,6 @@ impl ToolOperation {
 
                 for todo in output {
                     let todo_elm = Element::new("todo")
-                        .attr("id", &todo.id)
                         .attr("status", todo.status.to_string())
                         .text(&todo.content);
                     elm = elm.append(todo_elm);

crates/forge_app/src/snapshots/forge_app__tool_registry__all_rendered_tool_descriptions.snap

@@ -184,6 +184,22 @@ Fetches detailed information about a specific skill. Use this tool to load skill
 Use this tool to create and manage a structured task list for your current coding session. This helps you track progress, organize complex tasks, and demonstrate thoroughness to the user.
 It also helps the user understand the progress of the task and overall progress of their requests.
 
+## How It Works
+
+Each call sends only the items that changed — you do not need to repeat the whole list.
+
+Each item has two required fields:
+- `content`: The task description. This is the **unique key** — the server matches on content to decide whether to add or update.
+- `status`: One of `pending`, `in_progress`, `completed`, or `cancelled`.
+
+**Rules:**
+- Item with this `content` does **not** exist yet → **added** as a new task.
+- Item with this `content` already exists → its `status` is **updated**.
+- `status: cancelled` → the item is **removed** from the list entirely.
+- Items you do not mention are **left unchanged**.
+
+IDs are managed internally by the system and are never exposed to you.
+
 ## When to Use This Tool
 Use this tool proactively in these scenarios:
 
@@ -329,26 +345,24 @@ The assistant did not use the todo list because this is a single command executi
 ## Task States and Management
 
 1. **Task States**: Use these states to track progress:
-   - pending: Task not yet started
-   - in_progress: Currently working on (limit to ONE task at a time)
-   - completed: Task finished successfully
-
-   **IMPORTANT**: Task descriptions must have two forms:
-   - content: The imperative form describing what needs to be done (e.g., "Run tests", "Build the project")
-   - activeForm: The present continuous form shown during execution (e.g., "Running tests", "Building the project")
+   - `pending`: Task not yet started
+   - `in_progress`: Currently working on (limit to ONE task at a time)
+   - `completed`: Task finished successfully
+   - `cancelled`: Task is no longer relevant — this removes it from the list
 
 2. **Task Management**:
-   - Update task status in real-time as you work
-   - Mark tasks complete IMMEDIATELY after finishing (don't batch completions)
-   - Exactly ONE task must be in_progress at any time (not less, not more)
+   - Only send the items that changed — do not repeat unchanged items
+   - Mark tasks `in_progress` BEFORE beginning work
+   - Mark tasks `completed` IMMEDIATELY after finishing (don't batch completions)
+   - Exactly ONE task must be `in_progress` at any time
+   - Use `cancelled` to remove tasks that are no longer relevant
    - Complete current tasks before starting new ones
-   - Remove tasks that are no longer relevant from the list entirely
 
 3. **Task Completion Requirements**:
-   - ONLY mark a task as completed when you have FULLY accomplished it
-   - If you encounter errors, blockers, or cannot finish, keep the task as in_progress
+   - ONLY mark a task as `completed` when you have FULLY accomplished it
+   - If you encounter errors, blockers, or cannot finish, keep the task as `in_progress`
    - When blocked, create a new task describing what needs to be resolved
-   - Never mark a task as completed if:
+   - Never mark a task as `completed` if:
      - Tests are failing
      - Implementation is partial
      - You encountered unresolved errors
@@ -358,9 +372,6 @@ The assistant did not use the todo list because this is a single command executi
    - Create specific, actionable items
    - Break complex tasks into smaller, manageable steps
    - Use clear, descriptive task names
-   - Always provide both forms:
-     - content: "Fix authentication bug"
-     - activeForm: "Fixing authentication bug"
 
 When in doubt, use this tool. Being proactive with task management demonstrates attentiveness and ensures you complete all requirements successfully.