Ask Logue part 5: tool history and Deep Research on the island

CLAUDE.md

@@ -137,7 +137,16 @@ decided what Logue was.** These rules exist so that cannot happen again.
 - **Live run state belongs to a conversation, not to the app.** `AgentRunState` scopes
   `isProcessing`, `isStreaming`, `streamingText`, `activeToolCalls` and `lastError` to the
   conversation that started the run. A global flag means a run on one surface paints its
-  spinner, its streaming text and its tool cards onto the other's thread.
+  spinner, its streaming text and its tool cards onto the other's thread. This holds for
+  *every* pipeline, not only the agent loop: `DeepResearchCoordinator` runs one at a time
+  app-wide, which is precisely what made a global `isRunning` look sufficient for as long as
+  only one surface could start a run. It owns `runningConversationID` and is read through
+  `isRunning(in:)`.
+- **What the agent did is part of the answer.** Both surfaces render tool calls from
+  `AgentToolTimeline`, which pairs a call with the result that answers it and settles what to
+  show when the stored status and the result disagree. The island hid tool turns entirely,
+  which was fine while it had no tools and became the island claiming credit for work it
+  would not show.
 - **An error outlives its run but never its owner.** `lastError` persists after the run ends,
   so the banner survives until acknowledged — which is why `conversationID` survives
   `finish()` too. An error with no owner paints on every surface.

Logue/Agent/AgentCoordinator+Approval.swift

@@ -142,7 +142,8 @@ extension AgentCoordinator {
             )
         }
 
-        // Append the result message (invisible in UI; read by findResult in ToolExecutionCard)
+        // The result is its own message rather than a field on the call, so it is paired
+        // back up on read — see `AgentToolTimeline`. Never rendered as a row itself.
         let toolResult = AgentToolResult(toolCallID: toolCallID, output: output, isError: isError)
         store.appendMessage(
             AgentMessage(role: .toolResult, content: output, toolResult: toolResult),

Logue/Agent/AgentToolTimeline.swift

@@ -0,0 +1,107 @@
+import Foundation
+
+/// What the agent did during a turn, paired up and ready to render.
+///
+/// A tool call and its result are two separate messages in the conversation, linked only by
+/// an id — the call is appended when the model asks for it, the result arrives whenever the
+/// tool finishes. Rendering a card therefore means walking forward from a call looking for
+/// the message that answers it.
+///
+/// That walk lived in a `private func findResult` inside `AgentChatView+Messages`, along with
+/// the rule for what status to *show* when the stored status and the result disagree. Being
+/// private to a view is why the island could not show tool history at all: the island runs
+/// the same agent loop and stores the same messages, and the only thing it lacked was a way
+/// to read them back. #61's rule is that a feature is mounted by both surfaces rather than
+/// written twice, so the pairing moved here and both surfaces ask it.
+///
+/// Free of SwiftUI, so the pairing is testable without a view.
+enum AgentToolTimeline {
+    /// One tool call with whatever is known about how it went.
+    struct Entry: Identifiable, Equatable {
+        let call: AgentToolCall
+        let result: AgentToolResult?
+        /// What the card should show, which is not always `call.status` — see
+        /// `displayStatus(of:result:)`.
+        let status: AgentToolCallStatus
+
+        var id: UUID {
+            call.id
+        }
+
+        static func == (lhs: Self, rhs: Self) -> Bool {
+            lhs.call.id == rhs.call.id
+                && lhs.status == rhs.status
+                && lhs.result?.output == rhs.result?.output
+                && lhs.result?.isError == rhs.result?.isError
+        }
+    }
+
+    /// The result answering `toolCallID`, if one has arrived.
+    ///
+    /// Every message is searched rather than only those after the call. Ordering is how the
+    /// conversation is built, not something the store guarantees on read, and a result that
+    /// sorted oddly would otherwise render as a call that never finished.
+    static func result(for toolCallID: UUID, in messages: [AgentMessage]) -> AgentToolResult? {
+        for message in messages where message.role == .toolResult {
+            if let result = message.toolResult, result.toolCallID == toolCallID {
+                return result
+            }
+        }
+        return nil
+    }
+
+    /// What to show when the stored status and the result disagree.
+    ///
+    /// A call can be persisted as `.needsConfirmation` and still have a result: the user
+    /// answered the prompt on the other surface, or the run was resumed. The result is the
+    /// later fact, so it wins — otherwise a finished call keeps rendering Approve/Deny
+    /// buttons for something that has already happened, which is worse on the island than
+    /// anywhere else because the island is where the answer was given.
+    static func displayStatus(
+        of call: AgentToolCall,
+        result: AgentToolResult?
+    ) -> AgentToolCallStatus {
+        guard let result else { return call.status }
+        return result.isError ? .failed : .completed
+    }
+
+    /// The calls carried by `message`, each paired with its result.
+    static func entries(
+        in message: AgentMessage,
+        allMessages: [AgentMessage]
+    ) -> [Entry] {
+        message.toolCalls.map { call in
+            let result = result(for: call.id, in: allMessages)
+            return Entry(
+                call: AgentToolCall(
+                    id: call.id,
+                    toolName: call.toolName,
+                    arguments: call.arguments,
+                    status: displayStatus(of: call, result: result),
+                    clearance: call.clearance
+                ),
+                result: result,
+                status: displayStatus(of: call, result: result)
+            )
+        }
+    }
+
+    /// Every tool call in the conversation, in order, paired with its result.
+    ///
+    /// Used by the island, which renders tool history as its own rows rather than hanging
+    /// cards off the message that carried them.
+    static func entries(in messages: [AgentMessage]) -> [Entry] {
+        messages.flatMap { entries(in: $0, allMessages: messages) }
+    }
+
+    /// The calls still waiting on the user.
+    ///
+    /// A call whose result has arrived is never pending however it was stored, which is the
+    /// same disagreement `displayStatus` settles — stated once here so the approval strip and
+    /// the history cards cannot answer it differently.
+    static func awaitingApproval(in messages: [AgentMessage]) -> [AgentToolCall] {
+        entries(in: messages)
+            .filter { $0.status == .needsConfirmation }
+            .map(\.call)
+    }
+}

Logue/Agent/AskRoute.swift

@@ -87,7 +87,11 @@ enum AskRouter {
         // the agent is expected to read it.
         guard !trimmed.isEmpty || request.hasAttachments else { return nil }
 
-        if request.deepResearchRequested {
+        // Deep Research needs a question. The chip alone is not one: a send carrying only
+        // attachments passes the guard above, and routing that to Deep Research appended an
+        // empty user bubble and ran the whole seven-step pipeline on "" — with the files
+        // handed straight back, since research takes no attachments on either surface.
+        if request.deepResearchRequested, !trimmed.isEmpty {
             return .deepResearch
         }
 

Logue/Agent/AskStopTarget.swift

@@ -0,0 +1,32 @@
+import Foundation
+
+/// What the Stop button stops.
+///
+/// Two coordinators can be behind one button and they do not know about each other, so the
+/// decision has to be made somewhere — and it was made inside a `View`, where nothing could
+/// test it. That is the same privacy-inside-a-view problem `AgentToolTimeline` was extracted
+/// to fix, one button along.
+///
+/// The case that matters is `nothing`. The island's version fell back to cancelling the agent
+/// loop unconditionally, and `AgentCoordinator.cancel()` is *unscoped* — it kills the single
+/// global task and rejects every pending approval. So pressing "New" on an idle island
+/// stopped an answer that was streaming in the main window.
+enum AskStopTarget: Equatable {
+    case deepResearch
+    case agentLoop
+    /// Nothing this surface owns is running, so Stop stops nothing. Cancelling "just in case"
+    /// reaches into whatever the other surface is doing.
+    case nothing
+
+    /// - Parameters:
+    ///   - isResearchingHere: a Deep Research run owned by *this* conversation.
+    ///   - isAgentRunningHere: an agent-loop run owned by *this* conversation.
+    static func target(isResearchingHere: Bool, isAgentRunningHere: Bool) -> AskStopTarget {
+        // Deep Research first: it is the longer and more expensive of the two, and when both
+        // somehow read as running it is the one the user is waiting on.
+        if isResearchingHere {
+            return .deepResearch
+        }
+        return isAgentRunningHere ? .agentLoop : .nothing
+    }
+}

Logue/Agent/DeepResearch/DeepResearchCoordinator.swift

@@ -26,6 +26,27 @@ final class DeepResearchCoordinator {
     // MARK: - State
 
     private(set) var isRunning: Bool = false
+    /// The conversation this run belongs to, and it outlives the run.
+    ///
+    /// Deep Research is one-at-a-time app-wide, which made a single global `isRunning` look
+    /// sufficient while only the main window could start a run. The island can now start one
+    /// too, and a global flag means a run started in one window paints its progress — and
+    /// then its clarifying questions and its error — onto the other's thread. Same rule as
+    /// `AgentRunState`: live run state belongs to a conversation, not to the app, and the
+    /// owner survives the run because `lastError` and `clarifyingQuestions` do.
+    private(set) var runningConversationID: UUID?
+
+    /// Which run the coordinator's state currently describes.
+    ///
+    /// `cancel()` releases `isRunning` synchronously, but the cancelled task does not unwind
+    /// until whatever it is awaiting returns — and MLX generation is not preemptible, so that
+    /// can be tens of seconds. In that window a second run legitimately starts, and then the
+    /// first one finally throws `CancellationError` and writes *its* terminal state over the
+    /// live run's: the strip reads "Cancelled", `isRunning` goes false while the second run is
+    /// still working, its web-tool override is cleared underneath it, and nothing can stop it.
+    ///
+    /// Every write from inside `execute` is therefore gated on still owning this.
+    private var runGeneration = 0
     private(set) var currentStep: DeepResearchStep = .idle
     private(set) var sections: [ResearchSection] = []
     private(set) var currentSectionIdx: Int = 0
@@ -48,31 +69,80 @@ final class DeepResearchCoordinator {
 
     private init() {}
 
+    // MARK: - Scoped reads
+
+    /// Whether *this* conversation has a run in flight.
+    func isRunning(in conversationID: UUID) -> Bool {
+        isRunning && runningConversationID == conversationID
+    }
+
+    /// Whether this conversation has anything to show — a run, questions it came back with,
+    /// or how it failed. The progress panel is mounted on this rather than on `isRunning`, so
+    /// a finished run's questions stay readable on the surface that asked for them.
+    func hasActivity(in conversationID: UUID) -> Bool {
+        guard runningConversationID == conversationID else { return false }
+        return isRunning
+            || currentStep == .failed
+            || !clarifyingQuestions.isEmpty
+            || lastError != nil
+    }
+
     // MARK: - Public API
 
+    /// Puts the question in the conversation and starts a run on it.
+    ///
+    /// `run` expects the user message to be there already, which left each surface appending
+    /// it themselves — and the island, arriving second, would have had to rediscover that
+    /// contract by finding a run whose question was missing from the thread. #61's rule is
+    /// that both surfaces mount the same behaviour, so starting a run is one call.
+    ///
+    /// - Returns: the id of the appended question, so a caller that scrolls can scroll to it.
+    /// - Returns: the id of the appended question, or `nil` when a run was already in flight
+    ///   and this one was refused.
+    ///
+    /// The refusal is checked *before* the question is appended. It used to append first and
+    /// let `run` drop the request, which put the user's question in the thread with nothing
+    /// that would ever answer it: no spinner (run state is per conversation), no progress
+    /// strip (it belongs to the other conversation), no error, and the composer already
+    /// cleared. Returning `nil` is what lets a caller say so instead.
+    @discardableResult
+    func start(prompt: String, in conversationID: UUID, oneShotWebSearch: Bool = false) -> UUID? {
+        guard !isRunning else { return nil }
+        let question = AgentMessage(role: .user, content: prompt)
+        AgentConversationStore.shared.appendMessage(question, to: conversationID)
+        run(prompt: prompt, conversationID: conversationID, oneShotWebSearch: oneShotWebSearch)
+        return question.id
+    }
+
     /// Kicks off a Deep Research run for `prompt` and posts the final report to
     /// `conversationID`. The user message is expected to already be in the
-    /// conversation (the chat view appends it before calling).
+    /// conversation — prefer `start(prompt:in:oneShotWebSearch:)`, which does both.
     func run(prompt: String, conversationID: UUID, oneShotWebSearch: Bool = false) {
         guard !isRunning else { return }
         task?.cancel()
         resetState()
         isRunning = true
+        runningConversationID = conversationID
         // Mirror the per-send override into AgentCoordinator's tool registry so
         // `constrainedToolSpecs()` (which reads from there) sees web tools for
         // this run only. Cleanup happens in `execute()`'s defer block.
         if oneShotWebSearch {
             AgentCoordinator.shared.setOneShotIncludeWebTools(true)
         }
+        runGeneration += 1
+        let generation = runGeneration
         task = Task { [weak self] in
             guard let self else { return }
-            await execute(prompt: prompt, conversationID: conversationID)
+            await execute(prompt: prompt, conversationID: conversationID, generation: generation)
         }
     }
 
     func cancel() {
         task?.cancel()
         task = nil
+        // The cancelled task may still be unwinding; bumping the generation makes anything
+        // it writes afterwards a no-op, including its own "Cancelled." bookkeeping.
+        runGeneration += 1
         if isRunning {
             currentStep = .failed
             lastError = "Cancelled."
@@ -85,18 +155,24 @@ final class DeepResearchCoordinator {
     func dismiss() {
         guard !isRunning else { return }
         resetState()
+        runningConversationID = nil
     }
 
     // MARK: - Pipeline
 
     // swiftlint:disable:next function_body_length
-    private func execute(prompt: String, conversationID: UUID) async {
+    private func execute(prompt: String, conversationID: UUID, generation: Int) async {
         defer {
-            isRunning = false
-            // Match the AgentCoordinator cleanup contract — clear any per-run
-            // web-search override regardless of success/error/cancellation.
-            if AgentCoordinator.shared.oneShotIncludeWebTools {
-                AgentCoordinator.shared.setOneShotIncludeWebTools(false)
+            // Only if this is still the run the coordinator is describing. A cancelled run
+            // unwinding late must not release a *newer* run's `isRunning`, nor strip its
+            // web-tool override.
+            if generation == runGeneration {
+                isRunning = false
+                // Match the AgentCoordinator cleanup contract — clear any per-run
+                // web-search override regardless of success/error/cancellation.
+                if AgentCoordinator.shared.oneShotIncludeWebTools {
+                    AgentCoordinator.shared.setOneShotIncludeWebTools(false)
+                }
             }
         }
 
@@ -110,6 +186,7 @@ final class DeepResearchCoordinator {
             case .sufficient:
                 break
             case let .insufficient(questions):
+                guard generation == runGeneration else { return }
                 clarifyingQuestions = questions
                 currentStep = .failed
                 lastError = "Need more detail."
@@ -167,14 +244,18 @@ final class DeepResearchCoordinator {
             // posting throws (future refactor), we don't want the UI to claim
             // success while the conversation store has nothing.
             postReport(to: conversationID, report: finalReport)
+            guard generation == runGeneration else { return }
             currentStep = .completed
         } catch is CancellationError {
-            currentStep = .failed
-            lastError = "Cancelled."
+            // A cancelled run says nothing. `cancel()` already wrote the message, and by now
+            // a newer run may own the coordinator — repeating it here is how "Cancelled."
+            // landed on top of a run that was still going.
+            return
         } catch {
+            logger.error("Deep Research failed: \(error.localizedDescription, privacy: .public)")
+            guard generation == runGeneration else { return }
             currentStep = .failed
             lastError = error.localizedDescription
-            logger.error("Deep Research failed: \(error.localizedDescription, privacy: .public)")
             postFailure(to: conversationID, error: error.localizedDescription)
         }
     }

Logue/App/AppConstants.swift

@@ -71,6 +71,15 @@ enum AppConstants {
         static let oneShotWebSearch = "agent.oneShotWebSearch"
         /// Same idea for the per-send Deep Research toggle.
         static let oneShotDeepResearch = "agent.oneShotDeepResearch"
+        /// The island's own one-shot modes.
+        ///
+        /// Deliberately *not* the two keys above. The island clears its flags after every
+        /// send, so sharing the main window's keys meant a quick question here disarmed a
+        /// chip the user had armed over there and left on an unsent prompt — it then ran
+        /// without web tools and never said so. Separate keys keep the `@AppStorage`
+        /// binding a SwiftUI `Menu` needs without sharing the value.
+        static let islandOneShotWebSearch = "island.oneShotWebSearch"
+        static let islandOneShotDeepResearch = "island.oneShotDeepResearch"
         /// Optional override for the agent system prompt. Empty string = use the
         /// built-in default in `PromptRegistry.Agent`.
         static let agentSystemPromptOverride = "agent.systemPromptOverride"

Logue/App/AppDelegate.swift

@@ -650,8 +650,20 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent
         }
     }
 
+    /// Opens Settings on the AI tab, from either composer's `+` menu.
+    ///
+    /// Goes through `activateApp` because the island floats over another application:
+    /// posting the notification alone opens the Settings window *behind* whatever is
+    /// frontmost, which reads as the menu item doing nothing. From the main window the
+    /// activation is a no-op.
+    @objc
+    static func openToolSettings() {
+        SettingsNavigator.shared.pendingTab = .ai
+        (NSApp.delegate as? AppDelegate)?.showSettings()
+    }
+
     @objc
-    private func showSettings() {
+    fileprivate func showSettings() {
         activateApp {
             NotificationCenter.default.post(name: .openSettingsGeneral, object: nil)
         }