From 4ff030c73e6fe33f0d0704fdc04dc6ec20275b4c Mon Sep 17 00:00:00 2001 From: Kaede Akino Date: Tue, 25 Aug 2026 21:41:30 +0800 Subject: [PATCH] Give a conversation a real transcript, and make Stop stop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three defects in how a conversation drives and shows a run. Stop never reached the runtime. `Task { await runtime?.cancel() }` resolves the property when the task runs, which is after `stop()` has already set it to nil, so the run carried on executing tools with nobody listening — secrets undiscarded, SSH and SFTP connections uncleaned. `messages` was replayed as the next run's prior context, but it is a display projection: no tool calls, and no tool results at all, because those are drawn as cards. The model lost everything it had already done, and a tool-only turn came back as an assistant message with empty content, which Anthropic and Google both answer with a 400. The model-facing transcript is now kept beside the visible one and rebuilt from the same events. The runtime held the conversation through both its approval broker and its interaction handler while the conversation held the runtime, so every chat that had ever run an agent stayed alive for the session. All three edges are weak now, and a conversation that is gone denies and cancels. Alongside: messages and tool cards are merged into one ordered timeline rather than drawn as two lists, a tool-only turn no longer leaves an empty bubble, the transcript follows streaming text instead of only new rows, and conversations live in one app-wide store so two windows share instances rather than each building their own. Co-Authored-By: Claude Opus 5 (1M context) --- Termind/App/AppModel.swift | 34 ++- .../FeatureModels/AssistantLibraryModel.swift | 160 +++++++++---- Termind/Features/AI/ConversationModel.swift | 132 ++++++++++- .../ContextPanel/ContextPanelView.swift | 23 +- TermindTests/ChatComposerStateTests.swift | 223 ++++++++++++++++++ TermindTests/WindowStateTests.swift | 40 ++++ 6 files changed, 539 insertions(+), 73 deletions(-) diff --git a/Termind/App/AppModel.swift b/Termind/App/AppModel.swift index 022c35c0..8993617b 100644 --- a/Termind/App/AppModel.swift +++ b/Termind/App/AppModel.swift @@ -67,6 +67,9 @@ final class AppModel { /// One app-wide microphone owner. Keeping this above individual composers /// prevents two windows from installing competing audio taps. let speechTranscription: SpeechTranscriptionCoordinator + /// The one set of conversations every window navigates. See + /// `AssistantConversationStore`. + private let assistantConversations = AssistantConversationStore() private let agentRunRepository: any AgentRunPersisting private let agentHosts: AgentHostResolver private let agentConnections: AgentConnectionPool @@ -158,7 +161,7 @@ final class AppModel { func sendAgentDraft(_ conversation: ConversationModel) { let prompt = conversation.draft.trimmingCharacters(in: .whitespacesAndNewlines) guard !prompt.isEmpty else { return } - conversation.messages.append(ChatMessage(role: .user, text: prompt)) + conversation.appendUserTurn(prompt) conversation.draft = "" startAgentRun(conversation, prompt: prompt) } @@ -201,15 +204,25 @@ final class AppModel { reasoning: reasoning )) }() + // Weakly, all three times. The conversation owns the runtime, the + // runtime owns the broker and the interaction handler, and both of those + // used to own the conversation back — so every chat that had ever run an + // agent stayed alive, with its runtime, for the life of the process. + // A conversation that is gone denies and cancels: fail closed. let approval = AgentApprovalBroker( reviewer: reviewer, - manualApproval: { request in await conversation.requestApproval(request) }, - event: { [agentRunRepository] event in + manualApproval: { [weak conversation] request in + guard let conversation else { + return .deny(String(localized: "The conversation was closed before this action was approved.")) + } + return await conversation.requestApproval(request) + }, + event: { [agentRunRepository, weak conversation] event in Task { if let runID = event.associatedRunID { try? await agentRunRepository.record(event, runID: runID) } - await MainActor.run { conversation.handle(event) } + await MainActor.run { conversation?.handle(event) } } } ) @@ -224,21 +237,15 @@ final class AppModel { } let runtime = AgentRuntime( model: model, tools: tools, approval: approval, - repository: agentRunRepository, userInteraction: conversation, + repository: agentRunRepository, + userInteraction: WeakAgentUserInteraction(conversation), runCleanup: { [agentConnections] runID in await agentConnections.cleanup(runID: runID) } ) - let prior = conversation.messages.dropLast().map { message in - AgentTranscriptMessage( - id: message.id, - role: message.role == .user ? .user : .assistant, - text: message.text - ) - } conversation.start(runtime: runtime, request: AgentRunRequest( conversationID: conversation.id, prompt: userPrompt, - permissionMode: mode, priorMessages: prior + permissionMode: mode, priorMessages: conversation.pendingPriorMessages )) } @@ -246,6 +253,7 @@ final class AppModel { windows = windows.filter { $0.value.value != nil } windows[ObjectIdentifier(window)] = WeakWindow(window) window.commandHistoryStore = commandHistory + window.assistantLibrary.adopt(assistantConversations) window.assistantConversationDeletionHandler = { [weak self] id in self?.deleteAgentConversation(id) } diff --git a/Termind/Application/FeatureModels/AssistantLibraryModel.swift b/Termind/Application/FeatureModels/AssistantLibraryModel.swift index 5687bcb5..a4af1e28 100644 --- a/Termind/Application/FeatureModels/AssistantLibraryModel.swift +++ b/Termind/Application/FeatureModels/AssistantLibraryModel.swift @@ -1,65 +1,37 @@ import Foundation -/// Window-local navigation state for the fixed Assistant tab. +/// The app's conversations, and the single instance of each. /// -/// Conversations are no longer panes inside SSH/SFTP Workspaces. The model is -/// deliberately a separate feature boundary so persistence can later move to a -/// conversation repository without teaching WindowModel about message storage. +/// Conversations are app state, not window state: a run belongs to a +/// conversation, and a conversation opened in two windows is one conversation. +/// Building a `ConversationModel` per window meant a reply streaming in one +/// window was invisible in the other, and two windows could start concurrent +/// runs against the same stored history. @Observable -final class AssistantLibraryModel { - private(set) var conversations: [ConversationModel] - var selectedConversationID: ConversationModel.ID? +final class AssistantConversationStore { + private(set) var conversations: [ConversationModel] = [] init(conversations: [ConversationModel] = []) { self.conversations = conversations - selectedConversationID = conversations.first?.id - } - - var selectedConversation: ConversationModel? { - guard let selectedConversationID else { return nil } - return conversation(id: selectedConversationID) } func conversation(id: ConversationModel.ID) -> ConversationModel? { conversations.first { $0.id == id } } - @discardableResult - func createConversation() -> ConversationModel { - let conversation = ConversationModel() + func insert(_ conversation: ConversationModel) { + guard !conversations.contains(where: { $0.id == conversation.id }) else { return } conversations.insert(conversation, at: 0) - selectedConversationID = conversation.id - return conversation } - /// Promotes the sidebar's scratch conversation into history on its first - /// real send. Empty drafts remain ephemeral and never create blank rows. + /// Removes one conversation and hands back where it was, so each window can + /// move its own selection to whatever took its place. @discardableResult - func startConversation(_ conversation: ConversationModel) -> ConversationModel? { - let text = conversation.draft.trimmingCharacters(in: .whitespacesAndNewlines) - guard !text.isEmpty else { return nil } - guard !conversations.contains(where: { $0.id == conversation.id }) else { return nil } - - conversation.sendDraft() - conversations.insert(conversation, at: 0) - selectedConversationID = conversation.id - return conversation - } - - func select(_ id: ConversationModel.ID) { - guard conversations.contains(where: { $0.id == id }) else { return } - selectedConversationID = id - } - - func delete(_ id: ConversationModel.ID) { - guard let index = conversations.firstIndex(where: { $0.id == id }) else { return } + func remove(_ id: ConversationModel.ID) -> Int? { + guard let index = conversations.firstIndex(where: { $0.id == id }) else { return nil } conversations[index].stop() conversations.remove(at: index) - guard selectedConversationID == id else { return } - selectedConversationID = - conversations.indices.contains(index) - ? conversations[index].id - : conversations.first?.id + return index } func restore(_ stored: [StoredAgentConversation]) { @@ -69,11 +41,22 @@ final class AssistantLibraryModel { id: value.id, messages: value.messages.compactMap { message in switch message.role { - case .user: ChatMessage(id: message.id, role: .user, text: message.text) - case .assistant: ChatMessage(id: message.id, role: .assistant, text: message.text) + case .user: + ChatMessage( + id: message.id, role: .user, text: message.text, + createdAt: message.createdAt + ) + case .assistant: + ChatMessage( + id: message.id, role: .assistant, text: message.text, + createdAt: message.createdAt + ) case .tool: nil } }, + // Tool rows are dropped from the visible list because they are + // drawn as cards, but the model still needs them next turn. + transcript: value.messages, runState: value.lastRunState ) conversation.toolCards = value.toolCards.map { card in @@ -89,6 +72,91 @@ final class AssistantLibraryModel { } return conversation } - selectedConversationID = conversations.first?.id + } +} + +/// Window-local navigation over the app's conversations. +/// +/// The list is shared; only the selection is per window, which is what lets two +/// windows show different chats from the same set without duplicating any of +/// them. +@Observable +final class AssistantLibraryModel { + private var store: AssistantConversationStore + var selectedConversationID: ConversationModel.ID? + + init(store: AssistantConversationStore = AssistantConversationStore()) { + self.store = store + selectedConversationID = store.conversations.first?.id + } + + var conversations: [ConversationModel] { store.conversations } + + /// Joins the app-wide store, carrying over anything this window made before + /// it was registered. Windows are registered on appear, so in practice that + /// is nothing — but silently dropping a conversation would be worse than the + /// two lines it costs to keep it. + func adopt(_ shared: AssistantConversationStore) { + guard store !== shared else { return } + for conversation in store.conversations.reversed() { shared.insert(conversation) } + store = shared + if selectedConversationID.flatMap(shared.conversation(id:)) == nil { + selectedConversationID = shared.conversations.first?.id + } + } + + var selectedConversation: ConversationModel? { + guard let selectedConversationID else { return nil } + return conversation(id: selectedConversationID) + } + + func conversation(id: ConversationModel.ID) -> ConversationModel? { + store.conversation(id: id) + } + + @discardableResult + func createConversation() -> ConversationModel { + let conversation = ConversationModel() + store.insert(conversation) + selectedConversationID = conversation.id + return conversation + } + + /// Promotes the sidebar's scratch conversation into history on its first + /// real send. Empty drafts remain ephemeral and never create blank rows. + @discardableResult + func startConversation(_ conversation: ConversationModel) -> ConversationModel? { + let text = conversation.draft.trimmingCharacters(in: .whitespacesAndNewlines) + guard !text.isEmpty else { return nil } + guard !conversations.contains(where: { $0.id == conversation.id }) else { return nil } + + conversation.sendDraft() + store.insert(conversation) + selectedConversationID = conversation.id + return conversation + } + + func select(_ id: ConversationModel.ID) { + guard conversations.contains(where: { $0.id == id }) else { return } + selectedConversationID = id + } + + func delete(_ id: ConversationModel.ID) { + // `remove` returns nil when another window already took it out of the + // shared store; this window still has a stale selection to repair. + let index = store.remove(id) + guard selectedConversationID == id else { return } + let successor = index ?? 0 + selectedConversationID = + store.conversations.indices.contains(successor) + ? store.conversations[successor].id + : store.conversations.first?.id + } + + func restore(_ stored: [StoredAgentConversation]) { + store.restore(stored) + if selectedConversationID.flatMap(conversation(id:)) == nil { + selectedConversationID = conversations.first?.id + } } } diff --git a/Termind/Features/AI/ConversationModel.swift b/Termind/Features/AI/ConversationModel.swift index 53d933cb..67eabaf4 100644 --- a/Termind/Features/AI/ConversationModel.swift +++ b/Termind/Features/AI/ConversationModel.swift @@ -54,6 +54,10 @@ nonisolated struct ChatMessage: Identifiable, Hashable, Sendable { var commands: [SuggestedCommand] = [] /// True while tokens are still arriving, so the view can show a caret. var isStreaming: Bool = false + /// Orders this message against the tool cards, which are a separate list. + /// Restored conversations carry their stored time so history does not + /// re-sort itself behind every card the moment a window opens. + var createdAt: Date = .now } nonisolated struct AgentToolCard: Identifiable, Hashable, Sendable { @@ -69,6 +73,35 @@ nonisolated struct AgentToolCard: Identifiable, Hashable, Sendable { var review: SecurityReviewDecision? } +/// One row of a conversation, whichever of the two lists it came from. +nonisolated enum TimelineItem: Identifiable, Hashable, Sendable { + case message(ChatMessage) + case tool(AgentToolCard) + + var id: UUID { + switch self { + case .message(let message): message.id + case .tool(let card): card.id + } + } + + /// What the transcript scrolls to, and what a change to it must be visible + /// as: streaming text grows a message without changing any count. + var revision: Int { + var hasher = Hasher() + hasher.combine(id) + switch self { + case .message(let message): + hasher.combine(message.text.count) + hasher.combine(message.isStreaming) + case .tool(let card): + hasher.combine(card.state) + hasher.combine(card.result?.content.count ?? 0) + } + return hasher.finalize() + } +} + /// The approval posture selected for one conversation. /// /// This turn-level state does not yet alter command or MCP execution. Keeping @@ -112,6 +145,15 @@ final class ConversationModel: Identifiable { var pendingUserInput: AgentUserInputRequest? var userInputDraft = "" var secretInputDraft = "" + /// The model-facing history, which is not what `messages` holds. + /// + /// `messages` is a display projection: it has no tool calls and no tool + /// results, because those are drawn as cards. Replaying it as the next run's + /// prior context loses every tool the agent already ran, and turns a + /// tool-only assistant turn into an empty message — which Anthropic and + /// Google both reject. So the authoritative transcript is kept beside it and + /// rebuilt from the same events. + private(set) var transcript: [AgentTranscriptMessage] = [] private var runtime: AgentRuntime? private var eventTask: Task? private var approvalWaiters: [UUID: CheckedContinuation] = [:] @@ -142,10 +184,12 @@ final class ConversationModel: Identifiable { init( id: UUID = UUID(), messages: [ChatMessage] = [], + transcript: [AgentTranscriptMessage] = [], runState: AgentRunState = .idle ) { self.id = id self.messages = messages + self.transcript = transcript self.runState = runState } @@ -163,6 +207,31 @@ final class ConversationModel: Identifiable { var isEmpty: Bool { messages.isEmpty } + /// Messages and tool cards in the order they happened. + /// + /// Drawing the two lists one after the other put every tool card below + /// every message, so a run that alternated between talking and acting read + /// back as a wall of text followed by a wall of cards. Both lists are + /// already chronological, so this is a merge rather than a sort. + var timeline: [TimelineItem] { + var items: [TimelineItem] = [] + items.reserveCapacity(messages.count + toolCards.count) + var cards = toolCards[...] + for message in messages { + while let card = cards.first, card.invocation.createdAt < message.createdAt { + items.append(.tool(card)) + cards = cards.dropFirst() + } + // A turn that only called tools has no text to show, and an empty + // bubble reads as the model having said nothing. While it is still + // streaming it stays, because that is the "working" indicator. + guard message.role != .assistant || !message.text.isEmpty || message.isStreaming + else { continue } + items.append(.message(message)) + } + return items + cards.map(TimelineItem.tool) + } + func addChip(_ chip: ContextChip) { guard !contextChips.contains(where: { $0.kind == chip.kind && $0.label == chip.label }) else { return } contextChips.append(chip) @@ -177,10 +246,27 @@ final class ConversationModel: Identifiable { func sendDraft() { let text = draft.trimmingCharacters(in: .whitespacesAndNewlines) guard !text.isEmpty else { return } - messages.append(ChatMessage(role: .user, text: text)) + appendUserTurn(text) draft = "" } + /// Adds one user turn to both the visible transcript and the model-facing + /// one. The run request carries the prompt separately, so `AgentRuntime` + /// appends it to its own snapshot; `priorMessages` must therefore stop + /// short of it — see `pendingPriorMessages`. + func appendUserTurn(_ text: String) { + messages.append(ChatMessage(role: .user, text: text)) + transcript.append(AgentTranscriptMessage(role: .user, text: text)) + } + + /// The history to replay for a run whose prompt is the last user turn. + var pendingPriorMessages: [AgentTranscriptMessage] { + guard let last = transcript.lastIndex(where: { $0.role == .user }), + last == transcript.index(before: transcript.endIndex) + else { return transcript } + return Array(transcript[.. AgentUserInputResponse { + guard let conversation else { return .cancelled } + return await conversation.request(request) + } +} + +extension WeakAgentUserInteraction: AgentUserInteractionHandling {} + // MARK: - Demo data extension ConversationModel { diff --git a/Termind/Features/ContextPanel/ContextPanelView.swift b/Termind/Features/ContextPanel/ContextPanelView.swift index fee89692..a4ad18ef 100644 --- a/Termind/Features/ContextPanel/ContextPanelView.swift +++ b/Termind/Features/ContextPanel/ContextPanelView.swift @@ -627,14 +627,14 @@ struct ConversationView: View { if conversation.isEmpty { emptyState } else { - ForEach(conversation.messages) { message in - MessageView(message: message) - .id(message.id) - Divider().opacity(0.4) - } - ForEach(conversation.toolCards) { card in - AgentToolCardView(card: card) - .id(card.id) + ForEach(conversation.timeline) { item in + Group { + switch item { + case .message(let message): MessageView(message: message) + case .tool(let card): AgentToolCardView(card: card) + } + } + .id(item.id) Divider().opacity(0.4) } if conversation.runState == .interrupted { @@ -648,8 +648,11 @@ struct ConversationView: View { .frame(maxWidth: isCompact ? .infinity : 760) .frame(maxWidth: .infinity) } - .onChange(of: conversation.messages.count) { - guard let last = conversation.messages.last else { return } + // Keyed on the last row's revision, not on a count. Streaming text + // grows one message without adding a row, so a count never changed + // and the transcript stopped following the reply it was showing. + .onChange(of: conversation.timeline.last?.revision) { + guard let last = conversation.timeline.last else { return } withAnimation(Motion.tabSwitch) { proxy.scrollTo(last.id, anchor: .bottom) } } } diff --git a/TermindTests/ChatComposerStateTests.swift b/TermindTests/ChatComposerStateTests.swift index 5b92c097..3bc51f5c 100644 --- a/TermindTests/ChatComposerStateTests.swift +++ b/TermindTests/ChatComposerStateTests.swift @@ -30,6 +30,13 @@ private actor ConversationExecutionCounter { func increment() { value += 1 } } +private actor ConversationCancellationProbe { + var started = false + var cancelled = false + func markStarted() { started = true } + func markCancelled() { cancelled = true } +} + @Suite @MainActor struct ChatComposerStateTests { @@ -212,9 +219,225 @@ struct ChatComposerStateTests { }) } + /// `messages` is a display projection with no tool calls and no tool + /// results. Replaying it as the next run's prior context dropped everything + /// the agent had already done, and left an assistant turn with empty content + /// — the shape Anthropic and Google answer with a 400. + @Test + func modelFacingHistoryKeepsToolCallsAndResults() async { + let model = ImmediateConversationAgentModel(turns: [ + [ + .toolCallSnapshot( + id: "call-1", providerItemID: "item-1", name: "lookup", + arguments: #"{"query":"hosts"}"# + ), + .finished(.toolCalls), + ], + [.textDelta("host-a is up"), .finished(.completed)], + ]) + let tool = AnyAgentTool(descriptor: AgentToolDescriptor( + name: "lookup", summary: "lookup", + inputSchema: .object([ + "type": .string("object"), + "properties": .object(["query": .object(["type": .string("string")])]), + "required": .array([.string("query")]), + "additionalProperties": .bool(false), + ]), + target: .local, safety: .locallyReadOnly + )) { invocation, _ in + AgentToolResult(callID: invocation.call.id, content: "host-a") + } + let conversation = ConversationModel() + conversation.appendUserTurn("inspect") + // The prompt travels on the request, so it must not also be replayed. + #expect(conversation.pendingPriorMessages.isEmpty) + + conversation.start( + runtime: AgentRuntime( + model: model, tools: [tool], + approval: AgentApprovalBroker(reviewer: nil, manualApproval: { _ in .allow }) + ), + request: AgentRunRequest( + conversationID: conversation.id, prompt: "inspect", + permissionMode: .askForApproval + ) + ) + await waitForTerminal(conversation) + + conversation.appendUserTurn("connect to it") + let prior = conversation.pendingPriorMessages + + #expect(prior.map(\.role) == [.user, .assistant, .tool, .assistant]) + let call = prior.first { $0.role == .assistant }?.toolCalls.first + #expect(call?.name == "lookup") + #expect(call?.providerItemID == "item-1") + #expect(prior.contains { + $0.role == .tool && $0.toolCallID == "call-1" && $0.text == "host-a" + }) + #expect(!prior.contains { $0.role == .assistant && $0.text.isEmpty && $0.toolCalls.isEmpty }) + } + + /// Stop released the runtime reference before the cancelling task read it, + /// so the run carried on executing tools with nobody listening. + @Test + func stopCancelsTheRunAndNotOnlyItsEventStream() async { + let probe = ConversationCancellationProbe() + let tool = AnyAgentTool(descriptor: AgentToolDescriptor( + name: "wait", summary: "wait", + inputSchema: .object([ + "type": .string("object"), "properties": .object([:]), + "required": .array([]), "additionalProperties": .bool(false), + ]), + target: .local, safety: .locallyReadOnly + )) { invocation, _ in + await probe.markStarted() + do { + try await Task.sleep(for: .seconds(10)) + } catch { + await probe.markCancelled() + throw error + } + return AgentToolResult(callID: invocation.call.id, content: "never") + } + let conversation = ConversationModel() + conversation.appendUserTurn("wait for me") + conversation.start( + runtime: AgentRuntime( + model: ImmediateConversationAgentModel(turns: [[ + .toolCallDelta(id: "call-1", name: "wait", arguments: "{}"), + .finished(.toolCalls), + ]]), + tools: [tool], + approval: AgentApprovalBroker(reviewer: nil, manualApproval: { _ in .allow }) + ), + request: AgentRunRequest( + conversationID: conversation.id, prompt: "wait for me", + permissionMode: .askForApproval + ) + ) + await wait { await probe.started } + #expect(await probe.started) + + conversation.stop() + + await wait { await probe.cancelled } + #expect(await probe.cancelled) + #expect(conversation.runState == .cancelled) + #expect(!conversation.isStreaming) + } + + /// Messages and tool cards were two lists drawn one after the other, so a + /// run that alternated between talking and acting read back as all the text + /// followed by all the cards. + @Test + func timelineInterleavesToolCardsWithTheMessagesTheyBelongTo() async { + let conversation = ConversationModel() + conversation.appendUserTurn("inspect") + conversation.start( + runtime: AgentRuntime( + model: ImmediateConversationAgentModel(turns: [ + [ + .textDelta("Checking."), + .toolCallDelta(id: "call-1", name: "lookup", arguments: "{}"), + .finished(.toolCalls), + ], + [.textDelta("host-a is up"), .finished(.completed)], + ]), + tools: [AnyAgentTool(descriptor: AgentToolDescriptor( + name: "lookup", summary: "lookup", + inputSchema: .object([ + "type": .string("object"), "properties": .object([:]), + "required": .array([]), "additionalProperties": .bool(false), + ]), + target: .local, safety: .locallyReadOnly + )) { invocation, _ in + AgentToolResult(callID: invocation.call.id, content: "host-a") + }], + approval: AgentApprovalBroker(reviewer: nil, manualApproval: { _ in .allow }) + ), + request: AgentRunRequest( + conversationID: conversation.id, prompt: "inspect", + permissionMode: .askForApproval + ) + ) + await waitForTerminal(conversation) + + let kinds = conversation.timeline.map { item in + switch item { + case .message(let message): message.role == .user ? "user" : "assistant" + case .tool: "tool" + } + } + #expect(kinds == ["user", "assistant", "tool", "assistant"]) + } + + /// A turn that only called tools has nothing to say, and an empty bubble + /// reads as the model having said nothing at all. + @Test + func aSilentToolOnlyTurnDoesNotLeaveAnEmptyBubble() { + let conversation = ConversationModel() + conversation.appendUserTurn("go") + let id = UUID() + conversation.handle(.messageStarted(id)) + #expect(conversation.timeline.count == 2, "a streaming turn still shows progress") + + conversation.handle(.messageFinished(AgentTranscriptMessage( + id: id, role: .assistant, text: "", + toolCalls: [AgentToolCall(id: "c", name: "lookup", arguments: .object([:]))] + ))) + + #expect(conversation.timeline.count == 1) + #expect(conversation.messages.count == 2, "the turn is still in the history") + } + + /// The runtime held the conversation through both its approval broker and + /// its interaction handler, while the conversation held the runtime — so + /// every chat that ever ran an agent stayed alive for the whole session. + @Test + func aFinishedRunDoesNotKeepItsConversationAlive() async { + weak var leaked: ConversationModel? + do { + let conversation = ConversationModel() + leaked = conversation + conversation.appendUserTurn("hello") + conversation.start( + runtime: AgentRuntime( + model: ImmediateConversationAgentModel(turns: [[ + .textDelta("hi"), .finished(.completed), + ]]), + tools: [], + approval: AgentApprovalBroker( + reviewer: nil, + manualApproval: { [weak conversation] request in + guard let conversation else { return .deny("gone") } + return await conversation.requestApproval(request) + } + ), + userInteraction: WeakAgentUserInteraction(conversation) + ), + request: AgentRunRequest( + conversationID: conversation.id, prompt: "hello", + permissionMode: .askForApproval + ) + ) + await waitForTerminal(conversation) + } + // The event task holds the conversation until the stream ends; give the + // runloop a turn to release it. + await wait { leaked == nil } + #expect(leaked == nil) + } + private func waitForTerminal(_ conversation: ConversationModel) async { for _ in 0..<200 where !conversation.runState.isTerminal { try? await Task.sleep(for: .milliseconds(5)) } } + + private func wait(for condition: () async -> Bool) async { + for _ in 0..<400 { + if await condition() { return } + try? await Task.sleep(for: .milliseconds(5)) + } + } } diff --git a/TermindTests/WindowStateTests.swift b/TermindTests/WindowStateTests.swift index 23a45150..e2a45146 100644 --- a/TermindTests/WindowStateTests.swift +++ b/TermindTests/WindowStateTests.swift @@ -46,6 +46,46 @@ struct WindowStateTests { #expect(window.assistantLibrary.selectedConversationID == first.id) } + /// Each window used to build its own `ConversationModel` for the same + /// conversation id, so a reply streaming in one window was invisible in the + /// other and both could start a run against the same stored history. + @Test + func twoWindowsShareOneInstanceOfEachConversationButNotTheSelection() { + let shared = AssistantConversationStore() + let first = WindowModel() + let second = WindowModel() + first.assistantLibrary.adopt(shared) + second.assistantLibrary.adopt(shared) + + let conversation = first.openAssistant() + let other = first.openAssistant() + + #expect(second.assistantLibrary.conversations.map(\.id) == [other.id, conversation.id]) + #expect(second.assistantLibrary.conversation(id: conversation.id) === conversation) + + second.selectAssistant(conversation.id) + #expect(second.assistantLibrary.selectedConversationID == conversation.id) + #expect(first.assistantLibrary.selectedConversationID == other.id) + + // A delete in one window empties the other's stale selection too. + first.assistantLibrary.delete(conversation.id) + second.assistantLibrary.delete(conversation.id) + #expect(second.assistantLibrary.conversations.map(\.id) == [other.id]) + #expect(second.assistantLibrary.selectedConversationID == other.id) + } + + @Test + func aWindowThatAdoptsTheStoreLateKeepsWhatItAlreadyHad() { + let window = WindowModel() + let early = window.openAssistant() + + let shared = AssistantConversationStore() + window.assistantLibrary.adopt(shared) + + #expect(shared.conversations.map(\.id) == [early.id]) + #expect(window.assistantLibrary.selectedConversationID == early.id) + } + @Test func selectingAssistantDoesNotCreateAConversation() { let window = WindowModel()