Fix ACP v2 desktop review findings

Author: danielkov

fixtures/mock-acp.py

@@ -37,6 +37,7 @@ def argument_value(name):
 close_release = option("--close-release")
 close_release_session = option("--close-release-session")
 fail_close_session = option("--fail-close-session")
+prompt_capability_names = set((option("--prompt-capabilities") or "image,audio").split(","))
 model_ids = ["mock/default", "mock/requested"]
 
 if "--fail-start" in sys.argv:
@@ -86,6 +87,9 @@ def log_request(request):
         entry["cwd"] = params["cwd"]
     if request.get("method") == "session/prompt":
         entry["text"] = params["prompt"][0]["text"]
+    if request.get("method") is None and "result" in request:
+        entry["id"] = request.get("id")
+        entry["result"] = request["result"]
     with log_lock:
         with open(request_log, "a", encoding="utf-8") as log:
             log.write(json.dumps(entry, separators=(",", ":")) + "\n")
@@ -127,6 +131,11 @@ def prompt(request):
         return
     respond(request["id"], {})
     send({"jsonrpc": "2.0", "method": "session/update", "params": {"sessionId": session_id, "update": {"sessionUpdate": "state_update", "state": "running"}}})
+    if "MOCK_PERMISSION" in text:
+        send({
+            "jsonrpc": "2.0", "id": "permission-1", "method": "session/request_permission",
+            "params": {"sessionId": session_id, "title": "Approve?", "options": []},
+        })
     if "MOCK_HANG" in text:
         return
     should_gate = prompt_release is not None and (
@@ -147,6 +156,15 @@ def prompt(request):
         return
     if "MOCK_MEDIA" in text:
         text = ",".join(block.get("type", "unknown") for block in params["prompt"])
+    if not text and any(block.get("type") in ("image", "audio") for block in params["prompt"]):
+        send({
+            "jsonrpc": "2.0",
+            "method": "session/update",
+            "params": {"sessionId": session_id, "update": {
+                "sessionUpdate": "user_message", "messageId": "attachment-user",
+                "content": params["prompt"],
+            }},
+        })
     if "MOCK_ECHO" in text:
         send({
             "jsonrpc": "2.0",
@@ -157,6 +175,12 @@ def prompt(request):
                 "content": {"type": "text", "text": text},
             }},
         })
+    if "MOCK_EMPTY_REPLACEMENT" in text:
+        send({"jsonrpc": "2.0", "method": "session/update", "params": {"sessionId": session_id, "update": {"sessionUpdate": "user_message", "messageId": "empty-user", "content": []}}})
+    if "MOCK_NULL_REPLACEMENT" in text:
+        send({"jsonrpc": "2.0", "method": "session/update", "params": {"sessionId": session_id, "update": {"sessionUpdate": "user_message", "messageId": "null-user", "content": None}}})
+    if "MOCK_OMITTED_REPLACEMENT" in text:
+        send({"jsonrpc": "2.0", "method": "session/update", "params": {"sessionId": session_id, "update": {"sessionUpdate": "user_message", "messageId": "omitted-user"}}})
     if "MOCK_INTERRUPTION" in text:
         for update in [
             {"sessionUpdate": "agent_message_chunk", "messageId": "agent-1", "content": {"type": "text", "text": "stale response"}},
@@ -174,6 +198,14 @@ def prompt(request):
         send({"jsonrpc": "2.0", "method": "session/update", "params": {"sessionId": session_id, "update": {
             "sessionUpdate": "tool_call_update", "toolCallId": "background-1", "title": "Background task", "rawInput": {"background": True},
         }}})
+    if "MOCK_BACKGROUND_RECOMPUTE" in text:
+        for update in [
+            {"sessionUpdate": "tool_call_update", "toolCallId": "late-background", "title": "Late background"},
+            {"sessionUpdate": "tool_call_update", "toolCallId": "late-background", "rawInput": {"background": True}},
+            {"sessionUpdate": "tool_call_update", "toolCallId": "cleared-background", "title": "Cleared background", "rawInput": {"background": True}},
+            {"sessionUpdate": "tool_call_update", "toolCallId": "cleared-background", "rawInput": None},
+        ]:
+            send({"jsonrpc": "2.0", "method": "session/update", "params": {"sessionId": session_id, "update": update}})
     if "MOCK_RICH_OUTPUT" in text:
         updates = [
             {
@@ -195,6 +227,7 @@ def prompt(request):
                 "toolCallId": "call-1",
                 "title": "Inspect files",
             },
+            {"sessionUpdate": "tool_call_update", "toolCallId": "call-1"},
             {
                 "sessionUpdate": "tool_call_update",
                 "toolCallId": "call-1",
@@ -218,7 +251,7 @@ def prompt(request):
             if update["sessionUpdate"] == "tool_call_update" and "title" in update:
                 sys.stderr.write("\x01kit-runtime\x01" + json.dumps({"event": "child_started", "call": "call-1:compose:shell", "tool": "shell", "summary": "echo mock", "at": 1}) + "\n")
                 sys.stderr.flush()
-            elif update["sessionUpdate"] == "tool_call_update":
+            elif update["sessionUpdate"] == "tool_call_update" and "status" in update:
                 sys.stderr.write("\x01kit-runtime\x01" + json.dumps({"event": "child_finished", "call": "call-1:compose:shell", "tool": "shell", "ok": True, "summary": "done", "millis": 2}) + "\nmock diagnostic\n")
                 sys.stderr.flush()
         text = "rich done"
@@ -274,7 +307,7 @@ def close(request):
         respond(request["id"], {
             "protocolVersion": int(option("--protocol-version") or "2"),
             "info": {"name": "mock-acp", "version": "2.0.0"},
-            "capabilities": {"session": {"prompt": {"image": {}, "audio": {}}}},
+            "capabilities": {"session": {"prompt": {name: {} for name in prompt_capability_names if name}}},
         })
     elif method == "session/new":
         while new_release is not None and not os.path.exists(new_release):
@@ -291,7 +324,7 @@ def close(request):
             os._exit(0)
         if supports_models:
             result["configOptions"] = [{
-                "id": "model",
+                "configId": "model",
                 "name": "Model",
                 "category": "model",
                 "type": "select",
@@ -355,7 +388,7 @@ def close(request):
         result = {}
         if supports_models:
             result["configOptions"] = [{
-                "id": "model", "name": "Model", "category": "model", "type": "select",
+                "configId": "model", "name": "Model", "category": "model", "type": "select",
                 "currentValue": model_ids[0], "options": [{"value": value, "name": value} for value in model_ids],
             }]
         respond(request["id"], result)

macos/KitDesktop/Generated/ACPWireModels.generated.swift

@@ -181,6 +181,10 @@ struct ACPRequestPermissionRequest: Codable, Equatable {
     var options: [JSONValue]
 }
 
-struct ACPRequestPermissionResponse: Codable, Equatable {
+struct ACPRequestPermissionOutcome: Codable, Equatable {
     var outcome: String = "cancelled"
 }
+
+struct ACPRequestPermissionResponse: Codable, Equatable {
+    var outcome = ACPRequestPermissionOutcome()
+}

macos/KitDesktop/Models/ConversationController.swift

@@ -176,8 +176,8 @@ final class ConversationController: ObservableObject {
     }
 
     func send() {
-        let text = draft.trimmingCharacters(in: .whitespacesAndNewlines)
-        guard acceptsInput, !text.isEmpty || !attachments.isEmpty else { return }
+        let text = draft
+        guard acceptsInput, !text.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty || !attachments.isEmpty else { return }
         let files = attachments
         let turn = Turn.foreground(UUID())
         composerTransaction = ComposerTransaction(turn: turn, text: text, originalDraft: draft, attachments: files)
@@ -222,15 +222,24 @@ final class ConversationController: ObservableObject {
     }
 
     func choose(_ option: ConfigOption, value: String) {
-        let wireValue: ACPSessionConfigValue = option.valueType == "boolean" ? .boolean(value == "true") : .select(value)
+        let wireValue: ACPSessionConfigValue
+        switch option.valueType {
+        case "select": wireValue = .select(value)
+        case "boolean":
+            guard value == "true" || value == "false" else { return }
+            wireValue = .boolean(value == "true")
+        default: return
+        }
         client.setConfig(id: option.id, value: wireValue) { [weak self] result in
             guard let self else { return }
             switch result {
             case .failure(let error): self.fail(error)
             case .success(let payload):
-                let refreshed = Self.parseConfigOptions(payload["configOptions"])
-                if !refreshed.isEmpty { self.configOptions = refreshed }
-                else if let index = self.configOptions.firstIndex(where: { $0.id == option.id }) { self.configOptions[index].currentValue = value }
+                if let refreshed = payload["configOptions"] {
+                    self.configOptions = Self.parseConfigOptions(refreshed)
+                } else if let index = self.configOptions.firstIndex(where: { $0.id == option.id }) {
+                    self.configOptions[index].currentValue = value
+                }
                 self.publishCurrentConfig(userSelected: true)
             }
         }
@@ -297,7 +306,8 @@ final class ConversationController: ObservableObject {
         case .agentThought(let message): applyMessage(message, role: .thought)
         case .toolCall(let update), .toolCallUpdate(let update): updateTool(update)
         case .toolCallContent(let id, let content): appendToolContent(id: id, content: content)
-        case .plan(let id, let entries): applyPlan(id: id, entries: entries)
+        case .plan(let plan): applyPlan(plan)
+        case .planRemoved(let id): removePlan(id: id)
         case .usage(let usage): contextUsed = usage.used; contextSize = usage.size
         case .tokenUsage(let usage): tokenUsage = usage
         case .configOptions(let options): configOptions = Self.parseConfigOptions(options.anyValue); publishCurrentConfig()
@@ -319,18 +329,33 @@ final class ConversationController: ObservableObject {
     }
 
     private func applyMessage(_ message: DesktopMessageUpdate, role: TranscriptRole) {
-        if role == .user, composerTransaction != nil, message.content.allSatisfy({ consumeComposerEcho($0) }) {
-            if let optimistic = composerTransaction?.transcriptEntryID { messageEntryIDs[message.messageId] = optimistic }
+        if message.replace, !message.hasContent {
+            if role == .user, let optimistic = composerTransaction?.transcriptEntryID {
+                messageEntryIDs[message.messageId] = optimistic
+            }
             return
         }
-        if message.replace, !message.hasContent { return }
+        if role == .user, composerTransaction != nil {
+            if !message.content.isEmpty, consumeComposerEcho(message.content, requireComplete: message.replace) {
+                if let optimistic = composerTransaction?.transcriptEntryID { messageEntryIDs[message.messageId] = optimistic }
+                return
+            }
+            if message.replace, let optimistic = composerTransaction?.transcriptEntryID {
+                messageEntryIDs[message.messageId] = optimistic
+            }
+        }
         let blocks = message.content
         let text = blocks.map(Self.contentText).joined()
         if role == .assistant { latestAssistantSource = message.replace ? text : latestAssistantSource + text }
         if let entryID = messageEntryIDs[message.messageId], let index = entries.firstIndex(where: { $0.id == entryID }) {
             if message.replace {
                 entries[index].contentBlocks = blocks
                 entries[index].text = String(text.suffix(256 * 1024))
+                if role == .user {
+                    entries[index].presentation = .user(UserMessagePresentation(
+                        text: entries[index].text, media: blocks.flatMap { Self.userContent($0).media }
+                    ))
+                }
             } else {
                 entries[index].contentBlocks.append(contentsOf: blocks)
                 entries[index].text = String((entries[index].text + text).suffix(256 * 1024))
@@ -360,22 +385,29 @@ final class ConversationController: ObservableObject {
         }
     }
 
-    private func applyPlan(id: String, entries plan: [DesktopPlanEntry]?) {
-        if plan == nil {
-            if let entryID = planEntryIDs.removeValue(forKey: id) { entries.removeAll { $0.id == entryID } }
-            transcriptRevision += 1
-            return
-        }
-        let rows = plan!.map { "[" + ($0.status ?? "pending") + "] " + $0.content }.joined(separator: "\n")
-        if let entryID = planEntryIDs[id], let index = entries.firstIndex(where: { $0.id == entryID }) {
-            entries[index].text = rows; entries[index].formatted = Self.markdown(rows)
+    private func applyPlan(_ plan: DesktopPlanContent) {
+        let text: String
+        switch plan {
+        case .items(_, let entries):
+            text = entries.map { "[" + ($0.status ?? "pending") + "] " + $0.content }.joined(separator: "\n")
+        case .file(_, let uri): text = "[Plan file](" + uri + ")"
+        case .markdown(_, let content): text = content
+        case .unknown: return
+        }
+        if let entryID = planEntryIDs[plan.id], let index = entries.firstIndex(where: { $0.id == entryID }) {
+            entries[index].text = text; entries[index].formatted = Self.markdown(text)
         } else {
-            let entry = TranscriptEntry(role: .plan, title: "Plan", text: rows, formatted: Self.markdown(rows))
-            appendEntry(entry); planEntryIDs[id] = entry.id
+            let entry = TranscriptEntry(role: .plan, title: "Plan", text: text, formatted: Self.markdown(text))
+            appendEntry(entry); planEntryIDs[plan.id] = entry.id
         }
         transcriptRevision += 1
     }
 
+    private func removePlan(id: String) {
+        if let entryID = planEntryIDs.removeValue(forKey: id) { entries.removeAll { $0.id == entryID } }
+        transcriptRevision += 1
+    }
+
     private func applyTurnState(_ state: DesktopTurnState) {
         let turn = Turn.autonomous(state.turnId)
         if state.active { reduceSettlement(.started(turn, prompt: "")) }
@@ -482,21 +514,17 @@ final class ConversationController: ObservableObject {
 
     private func updateTool(_ patch: DesktopToolUpdate) {
         guard let id = patch.toolCallId else { return }
-        var update = toolStates[id] ?? DesktopToolUpdate(toolCallId: id)
-        func supplied(_ field: String) -> Bool { patch.present.isEmpty || patch.present.contains(field) }
-        if supplied("title") { update.title = patch.cleared.contains("title") ? nil : patch.title }
-        if supplied("kind") { update.kind = patch.cleared.contains("kind") ? nil : patch.kind }
-        if supplied("status") { update.status = patch.cleared.contains("status") ? nil : patch.status }
-        if supplied("content") { update.content = patch.cleared.contains("content") ? nil : patch.content }
-        if supplied("rawInput") { update.rawInput = patch.cleared.contains("rawInput") ? nil : patch.rawInput }
-        if supplied("rawOutput") { update.rawOutput = patch.cleared.contains("rawOutput") ? nil : patch.rawOutput }
+        let update = (toolStates[id] ?? DesktopToolUpdate(toolCallId: id)).merging(patch)
         toolStates[id] = update
         let dictionary = Self.toolDictionary(update)
         if let index = entries.lastIndex(where: { $0.role == .tool && $0.toolCallID == id }) {
             let tool = Self.toolPresentation(dictionary)
             entries[index].title = tool.title
             entries[index].text = tool.detail
             entries[index].presentation = .tool(tool)
+            if patch.present.contains("rawInput") {
+                entries[index].backgrounded = (update.rawInput?.anyValue as? [String: Any])?["background"] as? Bool == true
+            }
             entries[index].isStreaming = tool.status == .inProgress || tool.status == .pending
             if entries[index].isStreaming { streamingEntryIDs.insert(entries[index].id) }
             else { streamingEntryIDs.remove(entries[index].id) }
@@ -561,22 +589,33 @@ final class ConversationController: ObservableObject {
         transcriptRevision += 1
     }
 
-    private func consumeComposerEcho(_ content: DesktopContentBlock) -> Bool {
+    private func consumeComposerEcho(_ contents: [DesktopContentBlock], requireComplete: Bool) -> Bool {
         guard var transaction = composerTransaction else { return false }
-        switch content {
-        case .text(let text):
-            guard !transaction.echoedTextComplete else { return false }
-            let candidate = transaction.echoedText + text
-            guard !transaction.text.isEmpty, transaction.text.hasPrefix(candidate) else { return false }
-            transaction.echoedText = candidate
-            transaction.echoedTextComplete = candidate == transaction.text
-        case .image:
-            guard transaction.echoedAttachmentIndex < transaction.attachments.count, transaction.attachments[transaction.echoedAttachmentIndex].kind == .image else { return false }
-            transaction.echoedAttachmentIndex += 1
-        case .audio:
-            guard transaction.echoedAttachmentIndex < transaction.attachments.count, transaction.attachments[transaction.echoedAttachmentIndex].kind == .audio else { return false }
-            transaction.echoedAttachmentIndex += 1
-        default: return false
+        for content in contents {
+            switch content {
+            case .text(let text):
+                if transaction.text.isEmpty {
+                    guard text.isEmpty else { return false }
+                    transaction.echoedTextComplete = true
+                } else {
+                    guard !transaction.echoedTextComplete else { return false }
+                    let candidate = transaction.echoedText + text
+                    guard transaction.text.hasPrefix(candidate) else { return false }
+                    transaction.echoedText = candidate
+                    transaction.echoedTextComplete = candidate == transaction.text
+                }
+            case .image:
+                guard transaction.echoedAttachmentIndex < transaction.attachments.count, transaction.attachments[transaction.echoedAttachmentIndex].kind == .image else { return false }
+                transaction.echoedAttachmentIndex += 1
+            case .audio:
+                guard transaction.echoedAttachmentIndex < transaction.attachments.count, transaction.attachments[transaction.echoedAttachmentIndex].kind == .audio else { return false }
+                transaction.echoedAttachmentIndex += 1
+            default: return false
+            }
+        }
+        if requireComplete {
+            guard (transaction.text.isEmpty || transaction.echoedTextComplete),
+                  transaction.echoedAttachmentIndex == transaction.attachments.count else { return false }
         }
         composerTransaction = transaction
         return true
@@ -716,8 +755,9 @@ final class ConversationController: ObservableObject {
 
     static func parseConfigOptions(_ value: Any?) -> [ConfigOption] {
         (value as? [[String: Any]] ?? []).compactMap { item in
-            guard let id = (item["configId"] ?? item["id"]) as? String else { return nil }
-            let type = item["type"] as? String ?? "select"
+            guard let id = item["configId"] as? String,
+                  let type = item["type"] as? String,
+                  type == "select" || type == "boolean" else { return nil }
             let raw = item["options"] as? [[String: Any]] ?? []
             var groups: [ConfigGroup] = []
             var ungrouped: [ConfigChoice] = []