Codex improvements (#195)

Author: gsabran

app/modules/coreui/CodePreview/Sources/FileChangeExpandablePill.swift

@@ -192,7 +192,6 @@ public struct FileChangeExpandablePill: View {
       }
     }
     .with(cornerRadius: Constants.cornerRadius, borderColor: colorScheme.textAreaBorderColor)
-    .padding(.horizontal, 4)
   }
 
   let change: FileDiffViewModel

app/modules/coreui/ToolUI/Sources/ToolErrorView.swift

@@ -17,31 +17,35 @@ public struct ToolErrorView: View {
   }
 
   public var body: some View {
-    if toolUseErrorDescription.count > 300 {
-      VStack(alignment: .leading, spacing: 8) {
-        HStack {
-          Text("Tool failed")
-            .foregroundColor(colorScheme.redError)
-
-          Button(action: {
-            isExpanded.toggle()
-          }) {
-            Image(systemName: isExpanded ? "chevron.up" : "chevron.down")
-              .foregroundColor(.secondary)
+    if error is CancellationError {
+      Text("Cancelled")
+        .foregroundColor(.secondary)
+    } else {
+      if toolUseErrorDescription.count > 300 {
+        VStack(alignment: .leading, spacing: 8) {
+          HStack {
+            Button(action: {
+              isExpanded.toggle()
+            }) {
+              Image(systemName: isExpanded ? "chevron.down" : "chevron.right")
+                .foregroundColor(.secondary)
+            }
+            .buttonStyle(.plain)
+            Text("Tool failed")
+              .foregroundColor(colorScheme.redError)
           }
-          .buttonStyle(.plain)
-        }
 
-        if isExpanded {
-          Text(toolUseErrorDescription.asPlainText(colorScheme: colorScheme))
-            .textSelection(.enabled)
-            .foregroundColor(.primary)
+          if isExpanded {
+            Text(toolUseErrorDescription.asPlainText(colorScheme: colorScheme))
+              .textSelection(.enabled)
+              .foregroundColor(.primary)
+          }
         }
+      } else {
+        Text(toolUseErrorDescription.asPlainText(colorScheme: colorScheme))
+          .textSelection(.enabled)
+          .foregroundColor(colorScheme.redError)
       }
-    } else {
-      Text(toolUseErrorDescription.asPlainText(colorScheme: colorScheme))
-        .textSelection(.enabled)
-        .foregroundColor(colorScheme.redError)
     }
   }
 

app/modules/features/Chat/ChatFeature/Tests/ChatViewModelTests.swift

@@ -1199,6 +1199,7 @@ extension DependencyValues {
     llmService = MockLLMService(activeModels: [.claudeSonnet, .gpt])
     self.settingsService = settingsService
     userDefaults = mockUserDefaults
+    xcodeObserver = MockXcodeObserver(AXState<XcodeState>.unknown)
   }
 }
 

app/modules/foundations/ToolFoundation/Sources/DefaultTool/UnknownTool.swift

@@ -33,7 +33,7 @@ public final class UnknownTool: Tool {
       self.input = input
       self.context = context
 
-      let (stream, updateStatus) = Status.makeStream(initial: initialStatus ?? .notStarted)
+      let (stream, updateStatus) = Status.makeStream(initial: initialStatus?.completedOrCancelled ?? .notStarted)
       if case .completed = stream.value { updateStatus.finish() }
       status = stream
       self.updateStatus = updateStatus

app/modules/foundations/ToolFoundation/Sources/TestTool.swift

@@ -49,7 +49,7 @@ public struct TestTool: Tool {
       isReadonly = callingTool.isReadonly
       output = callingTool.output
 
-      let (stream, updateStatus) = Status.makeStream(initial: initialStatus ?? .notStarted)
+      let (stream, updateStatus) = Status.makeStream(initial: initialStatus?.completedOrCancelled ?? .notStarted)
       if case .completed = stream.value { updateStatus.finish() }
       status = stream
       self.updateStatus = updateStatus

app/modules/foundations/ToolFoundation/Sources/Tool.swift

@@ -406,6 +406,18 @@ extension ToolUse {
   }
 }
 
+extension ToolUseExecutionStatus {
+  /// Converts in-flight status to cancelled, unless it is already completed or not started.
+  public var completedOrCancelled: Self {
+    switch self {
+    case .notStarted, .completed, .approvalRejected:
+      self
+    case .running, .pendingApproval:
+      .completed(.failure(CancellationError()))
+    }
+  }
+}
+
 extension AsyncStream.Continuation {
   public func complete<Output>(with result: Result<Output, Error>) where Element == ToolUseExecutionStatus<Output> {
     yield(.completed(result))

app/modules/foundations/ToolFoundation/Tests/UnknownToolTests.swift

@@ -105,6 +105,28 @@ struct UnknownToolTests {
     #expect(type(of: finalToolUse.callingTool) == type(of: originalToolUse.callingTool))
   }
 
+  @Test
+  func test_initialInFlightStatusCompletesAsCancelled() {
+    let tool = TestTool()
+    let toolUse = TestTool.Use(
+      callingTool: tool,
+      toolUseId: "tool-use-id",
+      input: .null,
+      context: toolExecutionContext,
+      internalState: nil,
+      initialStatus: .running)
+
+    if case .completed(let result) = toolUse.status.value {
+      if case .failure(let error) = result {
+        #expect(error is CancellationError)
+      } else {
+        #expect(false, "Expected failure result when initial status is non-terminal.")
+      }
+    } else {
+      #expect(false, "Expected completed status when initial status is non-terminal.")
+    }
+  }
+
   private let toolExecutionContext = ToolExecutionContext()
 }
 

app/modules/plugins/tools/ACPTool/Sources/ACPTool.swift

@@ -42,7 +42,7 @@ public final class ACPTool: Tool {
       self.input = input
       self.internalState = internalState
 
-      let (stream, updateStatus) = Status.makeStream(initial: initialStatus ?? .notStarted)
+      let (stream, updateStatus) = Status.makeStream(initial: initialStatus?.completedOrCancelled ?? .notStarted)
       if case .completed = stream.value { updateStatus.finish() }
       status = stream
       self.updateStatus = updateStatus

app/modules/plugins/tools/ACPTool/Sources/ACPToolView.swift

@@ -5,6 +5,7 @@ import CodePreview
 import Dependencies
 import DLS
 import FileDiffFoundation
+import LoggingServiceInterface
 import Markdown
 import SwiftUI
 import ToolFoundation
@@ -39,15 +40,15 @@ struct ToolUseView: View {
     static let hstackSpacing: CGFloat = 4
   }
 
-  @State private var isExpanded = false
+  @State private var isExpanded: Bool?
   @State private var isHovered = false
 
   @Environment(\.colorScheme) private var colorScheme
 
   @ViewBuilder
   private var pendingApprovalView: some View {
     VStack(alignment: .leading) {
-      contentView(content: toolUse.input.content)
+      contentView(content: toolUse.input.content, isExpandedByDefault: true)
 
       ThreeDotsLoadingAnimation(baseText: "Waiting for approval")
     }
@@ -104,11 +105,11 @@ struct ToolUseView: View {
   }
 
   @ViewBuilder
-  private func contentView(content: [ToolsSchema.ACPTool_Content]?) -> some View {
+  private func contentView(content: [ToolsSchema.ACPTool_Content]?, isExpandedByDefault: Bool = false) -> some View {
     if let content {
       VStack(alignment: .leading) {
         HStack(spacing: Constants.hstackSpacing) {
-          if isExpanded {
+          if isExpanded ?? isExpandedByDefault {
             Icon(systemName: "chevron.down")
               .forTool(with: foregroundColor)
           } else if isHovered {
@@ -125,9 +126,9 @@ struct ToolUseView: View {
             .foregroundColor(foregroundColor)
         }
         .tappableTransparentBackground()
-        .onTapGesture { isExpanded.toggle() }
+        .onTapGesture { isExpanded = !(isExpanded ?? isExpandedByDefault) }
         .acceptClickThrough()
-        if isExpanded {
+        if isExpanded ?? isExpandedByDefault {
           contentView(for: content)
         }
       }.onHover { isHovered = $0 }
@@ -146,14 +147,7 @@ struct ToolUseView: View {
   @ViewBuilder
   private func failureView(error: Error) -> some View {
     VStack(alignment: .leading) {
-      HStack(spacing: Constants.hstackSpacing) {
-        Icon(systemName: toolIconName)
-          .forTool(with: foregroundColor)
-        Text(title)
-          .lineLimit(1)
-          .truncationMode(.middle)
-          .foregroundColor(foregroundColor)
-      }
+      contentView(content: toolUse.input.content)
       ToolErrorView(error)
     }
   }
@@ -279,10 +273,9 @@ struct DiffContentView: View {
         do {
           try await xcodeController.open(file: file, line: nil, column: nil)
         } catch {
-          print("Failed to open file: \(error)")
+          defaultLogger.error("Failed to open file", error)
         }
       })
-      .padding(.leading, 11)
   }
 
   @State private var viewModel: FileDiffViewModel

app/modules/plugins/tools/AskFollowUpTool/Sources/AskFollowUpTool.swift

@@ -31,7 +31,7 @@ public final class AskFollowUpTool: Tool {
       self.input = input
       self.context = context
 
-      let (stream, updateStatus) = Status.makeStream(initial: initialStatus ?? .notStarted)
+      let (stream, updateStatus) = Status.makeStream(initial: initialStatus?.completedOrCancelled ?? .notStarted)
       if case .completed = stream.value { updateStatus.finish() }
       status = stream
       self.updateStatus = updateStatus