Refactor ToolCallStatus enum to flat variants (#36356)

Ben Brandt created

Replace nested Allowed variant with distinct statuses for clearer status
handling.

Release Notes:

- N/A

Change summary

crates/acp_thread/src/acp_thread.rs    | 74 +++++++++++++++------------
crates/agent_servers/src/e2e_tests.rs  | 12 +++-
crates/agent_ui/src/acp/thread_view.rs | 56 ++++++++------------
3 files changed, 72 insertions(+), 70 deletions(-)

Detailed changes

crates/acp_thread/src/acp_thread.rs 🔗

@@ -223,7 +223,7 @@ impl ToolCall {
         }
 
         if let Some(status) = status {
-            self.status = ToolCallStatus::Allowed { status };
+            self.status = status.into();
         }
 
         if let Some(title) = title {
@@ -344,30 +344,48 @@ impl ToolCall {
 
 #[derive(Debug)]
 pub enum ToolCallStatus {
+    /// The tool call hasn't started running yet, but we start showing it to
+    /// the user.
+    Pending,
+    /// The tool call is waiting for confirmation from the user.
     WaitingForConfirmation {
         options: Vec<acp::PermissionOption>,
         respond_tx: oneshot::Sender<acp::PermissionOptionId>,
     },
-    Allowed {
-        status: acp::ToolCallStatus,
-    },
+    /// The tool call is currently running.
+    InProgress,
+    /// The tool call completed successfully.
+    Completed,
+    /// The tool call failed.
+    Failed,
+    /// The user rejected the tool call.
     Rejected,
+    /// The user cancelled generation so the tool call was cancelled.
     Canceled,
 }
 
+impl From<acp::ToolCallStatus> for ToolCallStatus {
+    fn from(status: acp::ToolCallStatus) -> Self {
+        match status {
+            acp::ToolCallStatus::Pending => Self::Pending,
+            acp::ToolCallStatus::InProgress => Self::InProgress,
+            acp::ToolCallStatus::Completed => Self::Completed,
+            acp::ToolCallStatus::Failed => Self::Failed,
+        }
+    }
+}
+
 impl Display for ToolCallStatus {
     fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
         write!(
             f,
             "{}",
             match self {
+                ToolCallStatus::Pending => "Pending",
                 ToolCallStatus::WaitingForConfirmation { .. } => "Waiting for confirmation",
-                ToolCallStatus::Allowed { status } => match status {
-                    acp::ToolCallStatus::Pending => "Pending",
-                    acp::ToolCallStatus::InProgress => "In Progress",
-                    acp::ToolCallStatus::Completed => "Completed",
-                    acp::ToolCallStatus::Failed => "Failed",
-                },
+                ToolCallStatus::InProgress => "In Progress",
+                ToolCallStatus::Completed => "Completed",
+                ToolCallStatus::Failed => "Failed",
                 ToolCallStatus::Rejected => "Rejected",
                 ToolCallStatus::Canceled => "Canceled",
             }
@@ -759,11 +777,7 @@ impl AcpThread {
                 AgentThreadEntry::UserMessage(_) => return false,
                 AgentThreadEntry::ToolCall(
                     call @ ToolCall {
-                        status:
-                            ToolCallStatus::Allowed {
-                                status:
-                                    acp::ToolCallStatus::InProgress | acp::ToolCallStatus::Pending,
-                            },
+                        status: ToolCallStatus::InProgress | ToolCallStatus::Pending,
                         ..
                     },
                 ) if call.diffs().next().is_some() => {
@@ -945,9 +959,7 @@ impl AcpThread {
         tool_call: acp::ToolCall,
         cx: &mut Context<Self>,
     ) -> Result<(), acp::Error> {
-        let status = ToolCallStatus::Allowed {
-            status: tool_call.status,
-        };
+        let status = tool_call.status.into();
         self.upsert_tool_call_inner(tool_call.into(), status, cx)
     }
 
@@ -1074,9 +1086,7 @@ impl AcpThread {
                 ToolCallStatus::Rejected
             }
             acp::PermissionOptionKind::AllowOnce | acp::PermissionOptionKind::AllowAlways => {
-                ToolCallStatus::Allowed {
-                    status: acp::ToolCallStatus::InProgress,
-                }
+                ToolCallStatus::InProgress
             }
         };
 
@@ -1097,7 +1107,10 @@ impl AcpThread {
             match &entry {
                 AgentThreadEntry::ToolCall(call) => match call.status {
                     ToolCallStatus::WaitingForConfirmation { .. } => return true,
-                    ToolCallStatus::Allowed { .. }
+                    ToolCallStatus::Pending
+                    | ToolCallStatus::InProgress
+                    | ToolCallStatus::Completed
+                    | ToolCallStatus::Failed
                     | ToolCallStatus::Rejected
                     | ToolCallStatus::Canceled => continue,
                 },
@@ -1290,10 +1303,9 @@ impl AcpThread {
             if let AgentThreadEntry::ToolCall(call) = entry {
                 let cancel = matches!(
                     call.status,
-                    ToolCallStatus::WaitingForConfirmation { .. }
-                        | ToolCallStatus::Allowed {
-                            status: acp::ToolCallStatus::InProgress
-                        }
+                    ToolCallStatus::Pending
+                        | ToolCallStatus::WaitingForConfirmation { .. }
+                        | ToolCallStatus::InProgress
                 );
 
                 if cancel {
@@ -1939,10 +1951,7 @@ mod tests {
             assert!(matches!(
                 thread.entries[1],
                 AgentThreadEntry::ToolCall(ToolCall {
-                    status: ToolCallStatus::Allowed {
-                        status: acp::ToolCallStatus::InProgress,
-                        ..
-                    },
+                    status: ToolCallStatus::InProgress,
                     ..
                 })
             ));
@@ -1981,10 +1990,7 @@ mod tests {
             assert!(matches!(
                 thread.entries[1],
                 AgentThreadEntry::ToolCall(ToolCall {
-                    status: ToolCallStatus::Allowed {
-                        status: acp::ToolCallStatus::Completed,
-                        ..
-                    },
+                    status: ToolCallStatus::Completed,
                     ..
                 })
             ));

crates/agent_servers/src/e2e_tests.rs 🔗

@@ -134,7 +134,9 @@ pub async fn test_tool_call(server: impl AgentServer + 'static, cx: &mut TestApp
             matches!(
                 entry,
                 AgentThreadEntry::ToolCall(ToolCall {
-                    status: ToolCallStatus::Allowed { .. },
+                    status: ToolCallStatus::Pending
+                        | ToolCallStatus::InProgress
+                        | ToolCallStatus::Completed,
                     ..
                 })
             )
@@ -212,7 +214,9 @@ pub async fn test_tool_call_with_permission(
         assert!(thread.entries().iter().any(|entry| matches!(
             entry,
             AgentThreadEntry::ToolCall(ToolCall {
-                status: ToolCallStatus::Allowed { .. },
+                status: ToolCallStatus::Pending
+                    | ToolCallStatus::InProgress
+                    | ToolCallStatus::Completed,
                 ..
             })
         )));
@@ -223,7 +227,9 @@ pub async fn test_tool_call_with_permission(
     thread.read_with(cx, |thread, cx| {
         let AgentThreadEntry::ToolCall(ToolCall {
             content,
-            status: ToolCallStatus::Allowed { .. },
+            status: ToolCallStatus::Pending
+                | ToolCallStatus::InProgress
+                | ToolCallStatus::Completed,
             ..
         }) = thread
             .entries()

crates/agent_ui/src/acp/thread_view.rs 🔗

@@ -1053,14 +1053,10 @@ impl AcpThreadView {
         let card_header_id = SharedString::from("inner-tool-call-header");
 
         let status_icon = match &tool_call.status {
-            ToolCallStatus::Allowed {
-                status: acp::ToolCallStatus::Pending,
-            }
-            | ToolCallStatus::WaitingForConfirmation { .. } => None,
-            ToolCallStatus::Allowed {
-                status: acp::ToolCallStatus::InProgress,
-                ..
-            } => Some(
+            ToolCallStatus::Pending
+            | ToolCallStatus::WaitingForConfirmation { .. }
+            | ToolCallStatus::Completed => None,
+            ToolCallStatus::InProgress => Some(
                 Icon::new(IconName::ArrowCircle)
                     .color(Color::Accent)
                     .size(IconSize::Small)
@@ -1071,16 +1067,7 @@ impl AcpThreadView {
                     )
                     .into_any(),
             ),
-            ToolCallStatus::Allowed {
-                status: acp::ToolCallStatus::Completed,
-                ..
-            } => None,
-            ToolCallStatus::Rejected
-            | ToolCallStatus::Canceled
-            | ToolCallStatus::Allowed {
-                status: acp::ToolCallStatus::Failed,
-                ..
-            } => Some(
+            ToolCallStatus::Rejected | ToolCallStatus::Canceled | ToolCallStatus::Failed => Some(
                 Icon::new(IconName::Close)
                     .color(Color::Error)
                     .size(IconSize::Small)
@@ -1146,15 +1133,23 @@ impl AcpThreadView {
                     tool_call.content.is_empty(),
                     cx,
                 )),
-            ToolCallStatus::Allowed { .. } | ToolCallStatus::Canceled => v_flex()
-                .w_full()
-                .children(tool_call.content.iter().map(|content| {
-                    div()
-                        .child(
-                            self.render_tool_call_content(entry_ix, content, tool_call, window, cx),
-                        )
-                        .into_any_element()
-                })),
+            ToolCallStatus::Pending
+            | ToolCallStatus::InProgress
+            | ToolCallStatus::Completed
+            | ToolCallStatus::Failed
+            | ToolCallStatus::Canceled => {
+                v_flex()
+                    .w_full()
+                    .children(tool_call.content.iter().map(|content| {
+                        div()
+                            .child(
+                                self.render_tool_call_content(
+                                    entry_ix, content, tool_call, window, cx,
+                                ),
+                            )
+                            .into_any_element()
+                    }))
+            }
             ToolCallStatus::Rejected => v_flex().size_0(),
         };
 
@@ -1467,12 +1462,7 @@ impl AcpThreadView {
 
         let tool_failed = matches!(
             &tool_call.status,
-            ToolCallStatus::Rejected
-                | ToolCallStatus::Canceled
-                | ToolCallStatus::Allowed {
-                    status: acp::ToolCallStatus::Failed,
-                    ..
-                }
+            ToolCallStatus::Rejected | ToolCallStatus::Canceled | ToolCallStatus::Failed
         );
 
         let output = terminal_data.output();