Ask Logue part 6: the island as glass, with starters, a level meter and a Return hint

Logue/Agent/AgentCoordinator.swift

@@ -120,12 +120,13 @@ final class AgentCoordinator {
     /// when either the Settings master toggle is on or a per-send override
     /// (`oneShotIncludeWebTools`) is set.
     private func buildToolRegistry() -> [any AgentTool] {
-        var tools = Self.readOnlyTools()
-            + Self.writeTools()
-            + Self.aiContentTools()
-            + Self.appleNativeTools()
-            + Self.computeAndDialogTools()
-            + Self.fileSystemTools()
+        // Web search tools — registered when the Settings master toggle is on OR when a
+        // per-send override (the input bar's one-shot Search toggle) is active for this run.
+        let userOptedIn = UserDefaults.standard.bool(forKey: AppConstants.UserDefaultsKeys.webSearchEnabled)
+        let includeWebTools = userOptedIn || oneShotIncludeWebTools
+        let webNames = Set(Self.webTools().map(\.name))
+
+        var tools = Self.allKnownTools().filter { includeWebTools || !webNames.contains($0.name) }
 
         // Phase A: per-tool enable/disable filter. The AISettingsTab persists
         // a set of tool names the user has explicitly turned off (e.g. "I
@@ -135,17 +136,30 @@ final class AgentCoordinator {
         if !disabledNames.isEmpty {
             tools.removeAll { disabledNames.contains($0.name) }
         }
-        // Web search tools — included when the Settings master toggle is on
-        // OR when a per-send override (the input bar's one-shot Search toggle)
-        // is active for this run.
-        let userOptedIn = UserDefaults.standard.bool(forKey: AppConstants.UserDefaultsKeys.webSearchEnabled)
-        if userOptedIn || oneShotIncludeWebTools {
-            tools.append(WebSearchTool())
-            tools.append(FetchWebPageTool())
-        }
         return tools
     }
 
+    /// Every tool the app can build, before any user filter.
+    ///
+    /// Exists so the registry has one definition rather than a second list kept in step by
+    /// hand — `ToolApprovalPromptTests` walks this to prove nothing that asks for approval
+    /// lacks a sentence explaining what it is about to do. A separate list would go stale
+    /// exactly when it mattered: the day someone adds a destructive tool.
+    static func allKnownTools() -> [any AgentTool] {
+        readOnlyTools()
+            + writeTools()
+            + aiContentTools()
+            + appleNativeTools()
+            + computeAndDialogTools()
+            + fileSystemTools()
+            + webTools()
+    }
+
+    /// Reaching off the machine, so registered only on an explicit opt-in.
+    static func webTools() -> [any AgentTool] {
+        [WebSearchTool(), FetchWebPageTool()]
+    }
+
     // MARK: - Tool registry shards
 
     //

Logue/Agent/AgentThinkingState.swift

@@ -0,0 +1,44 @@
+import Foundation
+
+/// Whether a surface should say it is working, rather than showing an answer.
+///
+/// There is a gap between a send and the first token — the model is loading, the context is
+/// being built, the loop has not produced anything yet — and it is the moment a user is most
+/// likely to conclude nothing happened and press the button again. The main window filled it
+/// with a pulsing dot and a status line. The island filled it with a literal `"..."` rendered
+/// as markdown, and before the assistant message existed at all it filled it with nothing.
+///
+/// The wording already had one definition in `UICopy.Status.describe(toolName:)`. This is the
+/// other half — *when* to show it — which was written out longhand at each place that needed
+/// it, and therefore came out differently at each place.
+///
+/// Free of SwiftUI so the matrix is testable without a view.
+enum AgentThinkingState {
+    /// - Parameters:
+    ///   - isProcessing: a run is in flight for this conversation.
+    ///   - isStreaming: tokens are being delivered for this conversation.
+    ///   - pendingAnswerText: what has arrived of the answer being produced *now*. Empty
+    ///     while nothing has. Deliberately not "the last assistant message", which is the
+    ///     previous answer and is non-empty for the whole of the next gap.
+    ///   - hasActiveToolCard: a tool card is on screen saying what is happening. Two things
+    ///     claiming to explain the same pause is worse than one.
+    static func showsThinking(
+        isProcessing: Bool,
+        isStreaming: Bool,
+        pendingAnswerText: String,
+        hasActiveToolCard: Bool
+    ) -> Bool {
+        guard isProcessing || isStreaming else { return false }
+        guard !hasActiveToolCard else { return false }
+        return pendingAnswerText.isEmpty
+    }
+
+    /// What the row should say, given whatever the agent is doing.
+    ///
+    /// Delegates rather than restating: the strings are `UICopy`'s, and a second copy of the
+    /// mapping is how one surface starts saying "Thinking…" while the other says "Searching
+    /// the web…" about the same run.
+    static func label(activeToolName: String?) -> String {
+        UICopy.Status.describe(toolName: activeToolName)
+    }
+}

Logue/Agent/ComposerChipRow.swift

@@ -0,0 +1,52 @@
+import Foundation
+
+/// How many chips a composer shows before it starts counting the rest.
+///
+/// The island floated its chips in an overlay offset above the pill, which meant two things.
+/// They took no layout space, so with a conversation on screen they drew over the bottom of
+/// the transcript; and nothing bounded them, so attaching six files ran the row past the
+/// island's width and off both ends.
+///
+/// The ordering rule is the part worth stating: **modes are never hidden.** A mode chip says
+/// what the send is about to *do* — search the web, spend minutes on Deep Research — and a
+/// hidden one is a send the user did not know they were making. An attachment chip only says
+/// what is going with it, and "+3 more" loses nothing that matters, because the files are
+/// still attached and still listed the moment one is removed.
+///
+/// Free of SwiftUI so the arithmetic is testable without a bar to put it in.
+enum ComposerChipRow {
+    /// What to draw.
+    struct Layout: Equatable {
+        /// Mode chips to render. Always every one of them.
+        let modes: Int
+        /// Attachment chips to render, oldest first.
+        let attachments: Int
+        /// Attachments not rendered, summarised by a single counter chip.
+        let hidden: Int
+
+        var showsOverflow: Bool {
+            hidden > 0
+        }
+    }
+
+    /// Chips the island's single row can hold before it looks like a list.
+    static let islandLimit = 4
+
+    static func layout(modeCount: Int, attachmentCount: Int, limit: Int = islandLimit) -> Layout {
+        let modes = max(0, modeCount)
+        let attachments = max(0, attachmentCount)
+
+        // Modes first, and they are not subject to the limit — see the note above. A limit
+        // smaller than the number of modes is a layout that cannot be honoured, and dropping
+        // a mode is the wrong way to honour it.
+        let remaining = max(0, limit - modes)
+        guard attachments > remaining else {
+            return Layout(modes: modes, attachments: attachments, hidden: 0)
+        }
+
+        // One of the remaining slots goes to the counter itself, so the row does not grow by
+        // adding the thing that says the row is full.
+        let shown = max(0, remaining - 1)
+        return Layout(modes: modes, attachments: shown, hidden: attachments - shown)
+    }
+}

Logue/Agent/DisplayText.swift

@@ -0,0 +1,49 @@
+import Foundation
+
+/// Turning a string that came from somewhere else into one line it is safe to show.
+///
+/// Two places need exactly this — the sentence on an approval card and the argument line on
+/// a tool card — and both show text the app did not write: a document title, a filesystem
+/// path, an argument a model produced. They had a copy each, which is one copy too many for
+/// something that is partly a security control.
+enum DisplayText {
+    /// One line, with everything that could make that line lie taken out.
+    ///
+    /// **Control and format characters go first**, and that is the half that matters. A title
+    /// is not always something the user typed: it can arrive in a `.md` file dropped into the
+    /// markdown folder, or from a call a prompt-injected model made. A bidirectional override
+    /// (U+202E) reverses the display of everything after it, so `Delete “report.txt”` can be
+    /// made to read as a different file — and on an approval card that sits directly above a
+    /// Touch ID prompt. `CharacterSet.controlCharacters` is Unicode categories Cc *and* Cf,
+    /// which is what makes it the right set rather than merely a plausible one.
+    ///
+    /// Whitespace is then collapsed, so a multi-paragraph value looks as truncated as it is
+    /// rather than like a short one, and a newline cannot split a sentence in half and leave
+    /// the verb sitting alone above Approve.
+    static func singleLine(_ value: String) -> String {
+        // Whitespace is exempted from the strip and handled by the split below, because a
+        // newline is itself a control character (U+000A is Cc). Removing it here rather than
+        // splitting on it would run the words either side of it together — `first\nsecond`
+        // becoming `firstsecond`, which reads as a different value rather than a flattened
+        // one. So: take out the controls that carry no meaning, then let the split turn the
+        // ones that do into a single space.
+        let scrubbed = value.unicodeScalars.filter { scalar in
+            !CharacterSet.controlCharacters.contains(scalar)
+                || CharacterSet.whitespacesAndNewlines.contains(scalar)
+        }
+        return String(String.UnicodeScalarView(scrubbed))
+            .components(separatedBy: .whitespacesAndNewlines)
+            .filter { !$0.isEmpty }
+            .joined(separator: " ")
+    }
+
+    /// Cuts to `limit`, marking that something was cut.
+    ///
+    /// The ellipsis is inside the budget rather than added to it, so the result is never
+    /// longer than the caller asked for.
+    static func clamp(_ value: String, to limit: Int) -> String {
+        guard value.count > limit else { return value }
+        guard limit > 1 else { return String(value.prefix(limit)) }
+        return String(value.prefix(limit - 1)) + "…"
+    }
+}

Logue/Agent/ToolApprovalPrompt.swift

@@ -0,0 +1,160 @@
+import Foundation
+
+/// What an approval card says is about to happen, and to what.
+///
+/// The card used to answer this with five hand-written sentences and a fallback of
+/// "Agent wants to run \(toolName)". None of them named the thing being acted on, which is
+/// the half that matters: every destructive tool here takes a **UUID**, so "Agent wants to
+/// delete a document" is the whole of what the user was told before being asked for Touch ID.
+/// Which document was not knowable from the card at all.
+///
+/// Pure, so the wording and the target extraction are testable without a view or a store. The
+/// names themselves have to be looked up, which is what `resolve` is for — a caller on the
+/// main actor asks the stores; a test passes a stub.
+enum ToolApprovalPrompt {
+    /// What kind of thing an id points at, so a caller knows which store to ask.
+    enum TargetKind: Equatable {
+        case document
+        case space
+        case reminder
+        case calendarEvent
+    }
+
+    /// An id carried in the arguments, and what it points at.
+    struct Reference: Equatable {
+        let kind: TargetKind
+        let id: UUID
+    }
+
+    /// Where the name of the thing being acted on comes from.
+    private enum TargetSource {
+        /// Nothing identifies a target — the action is the whole sentence.
+        case none
+        /// An argument holding a name, path, address or query, usable as written.
+        case literal(String)
+        /// An argument holding a UUID, which has to be resolved to something a person
+        /// recognises before it is worth showing.
+        case reference(TargetKind, String)
+    }
+
+    private struct Rule {
+        let action: String
+        let target: TargetSource
+    }
+
+    /// Longest target we will show. A path or a title can be arbitrarily long, and a prompt
+    /// that wraps to five lines is one people stop reading — which is the failure mode an
+    /// approval prompt can least afford.
+    static let maxTargetLength = 64
+
+    /// One entry per tool that can ask for approval.
+    ///
+    /// `ToolApprovalPromptTests` walks the registry and fails when a tool needing approval has
+    /// no entry here, so adding a destructive tool without saying what it does is a red build
+    /// rather than a card reading "Agent wants to run delete_everything".
+    private static let rules: [String: Rule] = [
+        // Documents
+        "create_document": Rule(action: "Create a document", target: .literal("title")),
+        "update_document": Rule(action: "Edit", target: .reference(.document, "documentID")),
+        "delete_document": Rule(action: "Delete", target: .reference(.document, "documentID")),
+        "move_document": Rule(action: "Move", target: .reference(.document, "documentID")),
+        "add_document_tag": Rule(action: "Tag", target: .reference(.document, "documentID")),
+        "export_document_pdf": Rule(action: "Export as PDF", target: .reference(.document, "documentID")),
+        "create_document_from_template": Rule(action: "Create a document", target: .literal("title")),
+        // Spaces
+        "create_space": Rule(action: "Create a space", target: .literal("name")),
+        "rename_space": Rule(action: "Rename", target: .reference(.space, "spaceID")),
+        "delete_space": Rule(
+            action: "Delete, with everything in it,",
+            target: .reference(.space, "spaceID")
+        ),
+        // Calendar
+        "create_calendar_event": Rule(action: "Create a calendar event", target: .literal("title")),
+        "update_calendar_event": Rule(action: "Change", target: .reference(.calendarEvent, "eventID")),
+        "delete_calendar_event": Rule(action: "Delete", target: .reference(.calendarEvent, "eventID")),
+        // Reminders
+        "add_reminder": Rule(action: "Add a reminder", target: .literal("title")),
+        "update_reminder": Rule(action: "Change", target: .reference(.reminder, "reminderID")),
+        "delete_reminder": Rule(action: "Delete", target: .reference(.reminder, "reminderID")),
+        // The filesystem, where the path is the target and is already readable
+        "list_directory": Rule(action: "List", target: .literal("path")),
+        "read_file_at_path": Rule(action: "Read", target: .literal("path")),
+        "write_text_to_file": Rule(action: "Write to", target: .literal("path")),
+        "delete_file_at_path": Rule(action: "Delete the file", target: .literal("path")),
+        // The user's own data, held by macOS rather than by Logue. Neither takes an id and
+        // neither has a target worth naming — what matters is that the card says plainly
+        // which private thing is about to be read, which "Agent wants to run get_location"
+        // did not. Found by the coverage test below, not by hand.
+        "fetch_contacts": Rule(action: "Read your contacts", target: .literal("name")),
+        "get_location": Rule(action: "Read your current location", target: .none),
+        // Off the machine
+        "draft_email": Rule(action: "Draft an email to", target: .literal("to")),
+        "web_search": Rule(action: "Search the web for", target: .literal("query")),
+        "fetch_web_page": Rule(action: "Open", target: .literal("url")),
+    ]
+
+    /// Whether this tool has a prompt written for it.
+    static func knows(toolNamed name: String) -> Bool {
+        rules[name] != nil
+    }
+
+    /// The id this call will act on, if it acts on one that has to be looked up.
+    static func reference(toolNamed name: String, arguments: String) -> Reference? {
+        guard case let .reference(kind, key)? = rules[name]?.target,
+              let raw = value(of: key, in: arguments),
+              let id = UUID(uuidString: raw)
+        else { return nil }
+        return Reference(kind: kind, id: id)
+    }
+
+    /// The sentence to show above Approve and Reject.
+    ///
+    /// - Parameter resolve: turns a `Reference` into something a person recognises. Returning
+    ///   `nil` — the object is gone, or the id was invented — leaves the action standing on
+    ///   its own rather than showing a UUID, which tells the user nothing and looks like a
+    ///   bug at the exact moment they are deciding whether to trust the agent.
+    static func sentence(
+        toolNamed name: String,
+        arguments: String,
+        resolve: (Reference) -> String?
+    ) -> String {
+        guard let rule = rules[name] else {
+            // An unknown tool is still asking for permission, so say so plainly rather than
+            // inventing a description of something we do not have a rule for.
+            return "Run \(clamp(flatten(name)))"
+        }
+
+        let target: String? = switch rule.target {
+        case .none:
+            nil
+        case let .literal(key):
+            value(of: key, in: arguments).map { clamp(flatten($0)) }
+        case .reference:
+            reference(toolNamed: name, arguments: arguments)
+                .flatMap(resolve)
+                .map { clamp(flatten($0)) }
+        }
+
+        guard let target, !target.isEmpty else { return rule.action }
+        return "\(rule.action) “\(target)”"
+    }
+
+    // MARK: - Reading arguments
+
+    private static func value(of key: String, in json: String) -> String? {
+        guard let data = json.data(using: .utf8),
+              let dict = try? JSONSerialization.jsonObject(with: data) as? [String: Any],
+              let raw = dict[key]
+        else { return nil }
+        let string = String(describing: raw).trimmingCharacters(in: .whitespacesAndNewlines)
+        return string.isEmpty ? nil : string
+    }
+
+    private static func flatten(_ value: String) -> String {
+        DisplayText.singleLine(value)
+    }
+
+    private static func clamp(_ value: String) -> String {
+        DisplayText.clamp(value, to: maxTargetLength)
+    }
+}