Fix CC todo tool parsing (#35721)

Agus Zubiaga created 3 months ago

It looks like the TODO tool call no longer requires a priority.

Release Notes:

- N/A

Change summary

crates/agent_servers/src/claude.rs       | 64 ++++++++++++++++++++++++++
crates/agent_servers/src/claude/tools.rs | 33 +++---------
2 files changed, 73 insertions(+), 24 deletions(-)

Detailed changes

crates/agent_servers/src/claude.rs

@@ -764,6 +764,8 @@ enum PermissionMode {
 #[cfg(test)]
 pub(crate) mod tests {
     use super::*;
+    use crate::e2e_tests;
+    use gpui::TestAppContext;
     use serde_json::json;
 
     crate::common_e2e_tests!(ClaudeCode, allow_option_id = "allow");
@@ -776,6 +778,68 @@ pub(crate) mod tests {
         }
     }
 
+    #[gpui::test]
+    #[cfg_attr(not(feature = "e2e"), ignore)]
+    async fn test_todo_plan(cx: &mut TestAppContext) {
+        let fs = e2e_tests::init_test(cx).await;
+        let project = Project::test(fs, [], cx).await;
+        let thread =
+            e2e_tests::new_test_thread(ClaudeCode, project.clone(), "/private/tmp", cx).await;
+
+        thread
+            .update(cx, |thread, cx| {
+                thread.send_raw(
+                    "Create a todo plan for initializing a new React app. I'll follow it myself, do not execute on it.",
+                    cx,
+                )
+            })
+            .await
+            .unwrap();
+
+        let mut entries_len = 0;
+
+        thread.read_with(cx, |thread, _| {
+            entries_len = thread.plan().entries.len();
+            assert!(thread.plan().entries.len() > 0, "Empty plan");
+        });
+
+        thread
+            .update(cx, |thread, cx| {
+                thread.send_raw(
+                    "Mark the first entry status as in progress without acting on it.",
+                    cx,
+                )
+            })
+            .await
+            .unwrap();
+
+        thread.read_with(cx, |thread, _| {
+            assert!(matches!(
+                thread.plan().entries[0].status,
+                acp::PlanEntryStatus::InProgress
+            ));
+            assert_eq!(thread.plan().entries.len(), entries_len);
+        });
+
+        thread
+            .update(cx, |thread, cx| {
+                thread.send_raw(
+                    "Now mark the first entry as completed without acting on it.",
+                    cx,
+                )
+            })
+            .await
+            .unwrap();
+
+        thread.read_with(cx, |thread, _| {
+            assert!(matches!(
+                thread.plan().entries[0].status,
+                acp::PlanEntryStatus::Completed
+            ));
+            assert_eq!(thread.plan().entries.len(), entries_len);
+        });
+    }
+
     #[test]
     fn test_deserialize_content_untagged_text() {
         let json = json!("Hello, world!");

crates/agent_servers/src/claude/tools.rs

@@ -143,25 +143,6 @@ impl ClaudeTool {
             Self::Grep(Some(params)) => vec![format!("`{params}`").into()],
             Self::WebFetch(Some(params)) => vec![params.prompt.clone().into()],
             Self::WebSearch(Some(params)) => vec![params.to_string().into()],
-            Self::TodoWrite(Some(params)) => vec![
-                params
-                    .todos
-                    .iter()
-                    .map(|todo| {
-                        format!(
-                            "- {} {}: {}",
-                            match todo.status {
-                                TodoStatus::Completed => "✅",
-                                TodoStatus::InProgress => "🚧",
-                                TodoStatus::Pending => "⬜",
-                            },
-                            todo.priority,
-                            todo.content
-                        )
-                    })
-                    .join("\n")
-                    .into(),
-            ],
             Self::ExitPlanMode(Some(params)) => vec![params.plan.clone().into()],
             Self::Edit(Some(params)) => vec![acp::ToolCallContent::Diff {
                 diff: acp::Diff {
@@ -193,6 +174,10 @@ impl ClaudeTool {
                     })
                     .unwrap_or_default()
             }
+            Self::TodoWrite(Some(_)) => {
+                // These are mapped to plan updates later
+                vec![]
+            }
             Self::Task(None)
             | Self::NotebookRead(None)
             | Self::NotebookEdit(None)
@@ -488,10 +473,11 @@ impl std::fmt::Display for GrepToolParams {
     }
 }
 
-#[derive(Deserialize, Serialize, JsonSchema, strum::Display, Debug)]
+#[derive(Default, Deserialize, Serialize, JsonSchema, strum::Display, Debug)]
 #[serde(rename_all = "snake_case")]
 pub enum TodoPriority {
     High,
+    #[default]
     Medium,
     Low,
 }
@@ -526,14 +512,13 @@ impl Into<acp::PlanEntryStatus> for TodoStatus {
 
 #[derive(Deserialize, Serialize, JsonSchema, Debug)]
 pub struct Todo {
-    /// Unique identifier
-    pub id: String,
     /// Task description
     pub content: String,
-    /// Priority level of the todo
-    pub priority: TodoPriority,
     /// Current status of the todo
     pub status: TodoStatus,
+    /// Priority level of the todo
+    #[serde(default)]
+    pub priority: TodoPriority,
 }
 
 impl Into<acp::PlanEntry> for Todo {