From 0e8136b554be4d34a67c1a6356302b0654bf7c73 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 21 Aug 2026 17:31:58 -0700 Subject: [PATCH] fix(macos): prevent duplicate New Chat sessions and stale navigation (#127693) * fix(macos): fence new chat creation * chore(i18n): refresh native source inventory --- apps/.i18n/native-source.json | 4 - .../ChatViewModel+SessionActions.swift | 18 +- .../OpenClawChatUI/ChatViewModel.swift | 7 +- .../ChatViewModelSessionActionTests.swift | 198 +++++++++++++++++- 4 files changed, 211 insertions(+), 16 deletions(-) diff --git a/apps/.i18n/native-source.json b/apps/.i18n/native-source.json index d0790a611b60..e02defb1c77c 100644 --- a/apps/.i18n/native-source.json +++ b/apps/.i18n/native-source.json @@ -40394,10 +40394,6 @@ { "kind": "ui-localized-call", "path": "apps/shared/OpenClawKit/Sources/OpenClawChatUI/ChatViewModel+Attachments.swift" - }, - { - "kind": "ui-localized-call", - "path": "apps/shared/OpenClawKit/Sources/OpenClawChatUI/ChatViewModel+SessionActions.swift" } ] }, diff --git a/apps/shared/OpenClawKit/Sources/OpenClawChatUI/ChatViewModel+SessionActions.swift b/apps/shared/OpenClawKit/Sources/OpenClawChatUI/ChatViewModel+SessionActions.swift index 6ec644a0bee5..8760683f27c2 100644 --- a/apps/shared/OpenClawKit/Sources/OpenClawChatUI/ChatViewModel+SessionActions.swift +++ b/apps/shared/OpenClawKit/Sources/OpenClawChatUI/ChatViewModel+SessionActions.swift @@ -35,11 +35,10 @@ extension OpenClawChatViewModel { worktreeBaseRef: String? = nil, routeLease: OpenClawChatNewSessionRouteLease? = nil) async -> Bool { - guard !self.blocksAttachmentOwnerChange else { - self.errorText = String( - localized: "Remove attachments or wait for delivery to resolve before starting a new chat.") - return false - } + guard !self.isCreatingSession, self.canCreateSessionForImmediateSwitch() else { return false } + self.isCreatingSession = true + defer { self.isCreatingSession = false } + let initiatingSession = self.currentSessionSnapshot() let normalizedAgentID = agentID? .trimmingCharacters(in: .whitespacesAndNewlines) .lowercased() @@ -77,6 +76,7 @@ extension OpenClawChatViewModel { let createdKey = created.key.trimmingCharacters(in: .whitespacesAndNewlines) next = createdKey.isEmpty ? requested : createdKey } catch { + guard self.isCurrentSession(initiatingSession) else { return false } if Self.isUnsupportedCreateSessionError(error) { // Reset only mimics a plain new chat; agent/worktree selections were // not honored, so advanced requests surface the error instead of @@ -86,17 +86,17 @@ extension OpenClawChatViewModel { self.errorText = error.localizedDescription return false } + guard self.canCreateSessionForImmediateSwitch() else { return false } chatUILogger.info("sessions.create unsupported; falling back to sessions.reset") await self.performReset() - return true + return self.isCurrentSession(initiatingSession) } chatUILogger.error("sessions.create failed \(error.localizedDescription, privacy: .public)") self.errorText = error.localizedDescription return false } - guard !self.blocksAttachmentOwnerChange else { - self.errorText = String( - localized: "Remove attachments or wait for delivery to resolve before starting a new chat.") + guard self.isCurrentSession(initiatingSession), self.canCreateSessionForImmediateSwitch() else { + if !self.sessions.contains(where: { $0.key == next }) { self.refreshSessions() } return false } self.adoptCreatedSession(next) diff --git a/apps/shared/OpenClawKit/Sources/OpenClawChatUI/ChatViewModel.swift b/apps/shared/OpenClawKit/Sources/OpenClawChatUI/ChatViewModel.swift index 5e3bd285b26f..786181788f98 100644 --- a/apps/shared/OpenClawKit/Sources/OpenClawChatUI/ChatViewModel.swift +++ b/apps/shared/OpenClawKit/Sources/OpenClawChatUI/ChatViewModel.swift @@ -71,6 +71,8 @@ public final class OpenClawChatViewModel { private var deferredExternalSessionKey: String? private var deferredDeliveryIdentity: DeferredDeliveryIdentity? var isSubmittingDraft = false + @ObservationIgnored + var isCreatingSession = false var attachmentStagingCount = 0 public private(set) var isAborting = false public var errorText: String? @@ -1273,18 +1275,21 @@ extension OpenClawChatViewModel { } func performReset() async { + let session = self.currentSessionSnapshot() self.isLoading = true self.errorText = nil do { - try await self.transport.resetSession(sessionKey: self.sessionKey) + try await self.transport.resetSession(sessionKey: session.key) } catch { + guard self.isCurrentSession(session) else { return } self.isLoading = false self.errorText = error.localizedDescription chatUILogger.error("session reset failed \(error.localizedDescription, privacy: .public)") return } + guard self.isCurrentSession(session) else { return } self.replyTarget = nil self.runMessageScopesByRunID.removeAll() self.provisionalFinalMessagesByID.removeAll() diff --git a/apps/shared/OpenClawKit/Tests/OpenClawKitTests/ChatViewModelSessionActionTests.swift b/apps/shared/OpenClawKit/Tests/OpenClawKitTests/ChatViewModelSessionActionTests.swift index 21e8fcc581d9..73a4697b4026 100644 --- a/apps/shared/OpenClawKit/Tests/OpenClawKitTests/ChatViewModelSessionActionTests.swift +++ b/apps/shared/OpenClawKit/Tests/OpenClawKitTests/ChatViewModelSessionActionTests.swift @@ -79,8 +79,11 @@ private actor SessionActionTransportState { var patchIdentities: [(key: String, expectedSessionID: String?)] = [] var deletedKeys: [String] = [] var groupPuts: [[String]] = [] + var createdKeys: [String] = [] var createdAgentIDs: [String?] = [] var createdParentKeys: [String?] = [] + var resetSessionKeys: [String] = [] + var sessionListRequestCount = 0 func recordFork(_ key: String, fromLastCompleted: Bool) { self.forkedParentKeys.append(key) @@ -128,9 +131,20 @@ private actor SessionActionTransportState { self.deletedKeys.append(key) } - func recordCreate(agentID: String?, parentKey: String?) { + func recordCreate(key: String, agentID: String?, parentKey: String?) -> Int { + let index = self.createdKeys.count + self.createdKeys.append(key) self.createdAgentIDs.append(agentID) self.createdParentKeys.append(parentKey) + return index + } + + func recordReset(_ sessionKey: String) { + self.resetSessionKeys.append(sessionKey) + } + + func recordSessionListRequest() { + self.sessionListRequestCount += 1 } } @@ -169,6 +183,9 @@ private struct SessionActionCompletionGate: Sendable { private final class SessionActionTransport: @unchecked Sendable, OpenClawChatTransport { private let state = SessionActionTransportState() + private let createGate: SessionActionCompletionGate? + private let resetGate: SessionActionCompletionGate? + private let createIsUnsupported: Bool private let forkGate: SessionActionCompletionGate? private let rewindGate: SessionActionCompletionGate? private let forkAtMessageGate: SessionActionCompletionGate? @@ -187,6 +204,9 @@ private final class SessionActionTransport: @unchecked Sendable, OpenClawChatTra private let sendSucceeds: Bool init( + createGate: SessionActionCompletionGate? = nil, + resetGate: SessionActionCompletionGate? = nil, + createIsUnsupported: Bool = false, forkGate: SessionActionCompletionGate? = nil, rewindGate: SessionActionCompletionGate? = nil, forkAtMessageGate: SessionActionCompletionGate? = nil, @@ -204,6 +224,9 @@ private final class SessionActionTransport: @unchecked Sendable, OpenClawChatTra historyFailureIndices: Set = [], sendSucceeds: Bool = false) { + self.createGate = createGate + self.resetGate = resetGate + self.createIsUnsupported = createIsUnsupported self.forkGate = forkGate self.rewindGate = rewindGate self.forkAtMessageGate = forkAtMessageGate @@ -342,6 +365,8 @@ private final class SessionActionTransport: @unchecked Sendable, OpenClawChatTra func acquireNewSessionRouteLease() async -> OpenClawChatNewSessionRouteLease? { let state = self.state + let createGate = self.createGate + let createIsUnsupported = self.createIsUnsupported return OpenClawChatNewSessionRouteLease( listAgents: { OpenClawChatAgentsListResponse( @@ -349,11 +374,37 @@ private final class SessionActionTransport: @unchecked Sendable, OpenClawChatTra agents: [OpenClawChatAgentChoice(id: "worker", workspaceGit: true)]) }, createSession: { key, _, agentID, parentKey, _, _ in - await state.recordCreate(agentID: agentID, parentKey: parentKey) + let index = await state.recordCreate(key: key, agentID: agentID, parentKey: parentKey) + if index == 0 { + await createGate?.suspendCompletion() + } + if createIsUnsupported { + throw NSError( + domain: "OpenClawChatTransport", + code: 0, + userInfo: [NSLocalizedDescriptionKey: "sessions.create not supported by this transport"]) + } return OpenClawChatCreateSessionResponse(ok: true, key: key, sessionId: nil) }) } + func resetSession(sessionKey: String) async throws { + await self.state.recordReset(sessionKey) + await self.resetGate?.suspendCompletion() + } + + func listSessions( + limit _: Int?, + search _: String?, + archived _: Bool) async throws -> OpenClawChatSessionsListResponse + { + await self.state.recordSessionListRequest() + throw NSError( + domain: "OpenClawChatTransport", + code: 0, + userInfo: [NSLocalizedDescriptionKey: "sessions.list not supported by this transport"]) + } + func deleteSession(key: String) async throws { await self.state.recordDelete(key) } @@ -418,9 +469,21 @@ private final class SessionActionTransport: @unchecked Sendable, OpenClawChatTra await self.state.createdAgentIDs } + func createdKeys() async -> [String] { + await self.state.createdKeys + } + func createdParentKeys() async -> [String?] { await self.state.createdParentKeys } + + func resetSessionKeys() async -> [String] { + await self.state.resetSessionKeys + } + + func sessionListRequestCount() async -> Int { + await self.state.sessionListRequestCount + } } private actor BatchMutationProbe { @@ -556,6 +619,137 @@ struct ChatViewModelSessionActionTests { #expect(await transport.createdAgentIDs() == ["worker"]) } + @Test func `new session creation rejects a duplicate while its gateway mutation is in flight`() async throws { + let createGate = SessionActionCompletionGate() + let transport = SessionActionTransport(createGate: createGate) + var observedSelections: [String] = [] + let viewModel = OpenClawChatViewModel( + sessionKey: "main", + transport: transport, + onSessionChanged: { observedSelections.append($0) }) + let lease = try await viewModel.newSessionRouteLease() + + let firstCreate = Task { + await viewModel.startNewSession(agentID: "", worktree: false, worktreeBaseRef: nil, using: lease) + } + guard await self.waitForForkStart(createGate) else { + createGate.release() + firstCreate.cancel() + Issue.record("timed out waiting for session creation start signal") + return + } + + let duplicateCreated = await viewModel.startNewSession( + agentID: "", + worktree: false, + worktreeBaseRef: nil, + using: lease) + + #expect(duplicateCreated == false) + #expect(await transport.createdKeys().count == 1) + createGate.release() + #expect(await firstCreate.value) + #expect(await viewModel.sessionKey == (transport.createdKeys()).first) + #expect(observedSelections == [viewModel.sessionKey]) + } + + @Test(arguments: [false, true]) + func `stale new session completion cannot replace or reset newer navigation`( + createIsUnsupported: Bool) async throws + { + let createGate = SessionActionCompletionGate() + let transport = SessionActionTransport( + createGate: createGate, + createIsUnsupported: createIsUnsupported) + let viewModel = OpenClawChatViewModel(sessionKey: "main", transport: transport) + let lease = try await viewModel.newSessionRouteLease() + + let create = Task { + await viewModel.startNewSession(agentID: "", worktree: false, worktreeBaseRef: nil, using: lease) + } + guard await self.waitForForkStart(createGate) else { + createGate.release() + create.cancel() + Issue.record("timed out waiting for session creation start signal") + return + } + viewModel.switchSession(to: "other") + let newerSession = viewModel.currentSessionSnapshot() + let newerHistoryGeneration = viewModel.lastIssuedHistoryRequestID + + createGate.release() + let created = await create.value + + #expect(created == false) + #expect(viewModel.currentSessionSnapshot() == newerSession) + #expect(viewModel.lastIssuedHistoryRequestID == newerHistoryGeneration) + #expect(viewModel.errorText == nil) + #expect(await transport.createdKeys().count == 1) + #expect(await transport.resetSessionKeys().isEmpty) + } + + @Test(arguments: [false, true]) + func `new session completion preserves attachment ownership acquired during creation`( + createIsUnsupported: Bool) async throws + { + let createGate = SessionActionCompletionGate() + let transport = SessionActionTransport( + createGate: createGate, + createIsUnsupported: createIsUnsupported) + let viewModel = OpenClawChatViewModel(sessionKey: "main", transport: transport) + let lease = try await viewModel.newSessionRouteLease() + + let create = Task { + await viewModel.startNewSession(agentID: "", worktree: false, worktreeBaseRef: nil, using: lease) + } + #expect(await self.waitForForkStart(createGate)) + viewModel.beginAttachmentStaging() + defer { viewModel.endAttachmentStaging() } + + createGate.release() + #expect(await create.value == false) + #expect(viewModel.sessionKey == "main" && viewModel.isAttachmentOwnerPinned) + #expect(viewModel.errorText == + "Remove attachments or wait for delivery to resolve before starting a new chat.") + #expect(await transport.createdKeys().count == 1) + #expect(await transport.resetSessionKeys().isEmpty) + + if !createIsUnsupported { + try await waitUntil("committed session is discoverable after attachment ownership changes") { + await transport.sessionListRequestCount() == 1 + } + } + } + + @Test func `navigation during unsupported new session reset preserves the newer selection`() async throws { + let resetGate = SessionActionCompletionGate() + let transport = SessionActionTransport(resetGate: resetGate, createIsUnsupported: true) + let viewModel = OpenClawChatViewModel(sessionKey: "main", transport: transport) + let lease = try await viewModel.newSessionRouteLease() + + let create = Task { + await viewModel.startNewSession(agentID: "", worktree: false, worktreeBaseRef: nil, using: lease) + } + guard await self.waitForForkStart(resetGate) else { + resetGate.release() + create.cancel() + Issue.record("timed out waiting for fallback reset start signal") + return + } + viewModel.switchSession(to: "other") + let newerSession = viewModel.currentSessionSnapshot() + let newerHistoryGeneration = viewModel.lastIssuedHistoryRequestID + + resetGate.release() + let created = await create.value + + #expect(created == false) + #expect(viewModel.currentSessionSnapshot() == newerSession) + #expect(viewModel.lastIssuedHistoryRequestID == newerHistoryGeneration) + #expect(viewModel.errorText == nil) + #expect(await transport.resetSessionKeys() == ["main"]) + } + @Test func `unsupported create with advanced options fails without resetting`() async { // SessionActionTransport relies on the protocol's default createSession, // which throws the canonical unsupported error; the worktree request must