From e92873ea60536134386b90144153e6340835aacc Mon Sep 17 00:00:00 2001 From: lark Date: Wed, 25 Mar 2026 01:58:22 +0900 Subject: [PATCH 1/8] Add multi-agent browser tab isolation (Phase 1 MVP) Introduce BrowserAgentSession system that enables multiple AI agents to share imported Chrome profiles while maintaining cookie isolation via clone-on-first-use WKWebsiteDataStore cloning. New files: - BrowserAgentSession.swift: session struct (agent UUID, source profile, data store ID) - BrowserAgentSessionStore.swift: session lifecycle management with Task-based clone serialization, on-disk manifest for crash-recovery GC, and WKWebsiteDataStore.remove(forIdentifier:) cleanup Modified: - BrowserPanel: agentSessionId property, agent data store in init, block switchToProfile() on agent-owned panels - SessionPersistence: agentSessionId field (agent panels discarded on restore) - Workspace: newBrowserSurface gains agentSessionId/agentDataStoreId params - TerminalController: 4 new socket commands: - browser.agent_session.open: create session with cookie clone - browser.agent_session.tab: add tab to existing session - browser.agent_session.dispose: close tabs + remove data store - browser.agent_session.list: enumerate sessions Co-Authored-By: Claude Opus 4.6 (1M context) --- GhosttyTabs.xcodeproj/project.pbxproj | 8 + Sources/Panels/BrowserAgentSession.swift | 30 ++ Sources/Panels/BrowserAgentSessionStore.swift | 256 ++++++++++++++++++ Sources/Panels/BrowserPanel.swift | 24 +- Sources/SessionPersistence.swift | 2 + Sources/TerminalController.swift | 220 +++++++++++++++ Sources/Workspace.swift | 8 +- 7 files changed, 542 insertions(+), 6 deletions(-) create mode 100644 Sources/Panels/BrowserAgentSession.swift create mode 100644 Sources/Panels/BrowserAgentSessionStore.swift diff --git a/GhosttyTabs.xcodeproj/project.pbxproj b/GhosttyTabs.xcodeproj/project.pbxproj index 01d6d6058..6dd7acd51 100644 --- a/GhosttyTabs.xcodeproj/project.pbxproj +++ b/GhosttyTabs.xcodeproj/project.pbxproj @@ -29,6 +29,8 @@ A5001400 /* Panel.swift in Sources */ = {isa = PBXBuildFile; fileRef = A5001410 /* Panel.swift */; }; A5001401 /* TerminalPanel.swift in Sources */ = {isa = PBXBuildFile; fileRef = A5001411 /* TerminalPanel.swift */; }; A5001402 /* BrowserPanel.swift in Sources */ = {isa = PBXBuildFile; fileRef = A5001412 /* BrowserPanel.swift */; }; + B3A91002 /* BrowserAgentSession.swift in Sources */ = {isa = PBXBuildFile; fileRef = B3A91001 /* BrowserAgentSession.swift */; }; + B3A91004 /* BrowserAgentSessionStore.swift in Sources */ = {isa = PBXBuildFile; fileRef = B3A91003 /* BrowserAgentSessionStore.swift */; }; A5001403 /* TerminalPanelView.swift in Sources */ = {isa = PBXBuildFile; fileRef = A5001413 /* TerminalPanelView.swift */; }; A5001404 /* BrowserPanelView.swift in Sources */ = {isa = PBXBuildFile; fileRef = A5001414 /* BrowserPanelView.swift */; }; A5007420 /* BrowserPopupWindowController.swift in Sources */ = {isa = PBXBuildFile; fileRef = A5007421 /* BrowserPopupWindowController.swift */; }; @@ -204,6 +206,8 @@ A5001410 /* Panel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Panels/Panel.swift; sourceTree = ""; }; A5001411 /* TerminalPanel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Panels/TerminalPanel.swift; sourceTree = ""; }; A5001412 /* BrowserPanel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Panels/BrowserPanel.swift; sourceTree = ""; }; + B3A91001 /* BrowserAgentSession.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Panels/BrowserAgentSession.swift; sourceTree = ""; }; + B3A91003 /* BrowserAgentSessionStore.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Panels/BrowserAgentSessionStore.swift; sourceTree = ""; }; A5001413 /* TerminalPanelView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Panels/TerminalPanelView.swift; sourceTree = ""; }; A5001414 /* BrowserPanelView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Panels/BrowserPanelView.swift; sourceTree = ""; }; A5007421 /* BrowserPopupWindowController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Panels/BrowserPopupWindowController.swift; sourceTree = ""; }; @@ -434,6 +438,8 @@ A5001410 /* Panel.swift */, A5001411 /* TerminalPanel.swift */, A5001412 /* BrowserPanel.swift */, + B3A91001 /* BrowserAgentSession.swift */, + B3A91003 /* BrowserAgentSessionStore.swift */, A5001413 /* TerminalPanelView.swift */, A5001414 /* BrowserPanelView.swift */, A5007421 /* BrowserPopupWindowController.swift */, @@ -729,6 +735,8 @@ A5001400 /* Panel.swift in Sources */, A5001401 /* TerminalPanel.swift in Sources */, A5001402 /* BrowserPanel.swift in Sources */, + B3A91002 /* BrowserAgentSession.swift in Sources */, + B3A91004 /* BrowserAgentSessionStore.swift in Sources */, A5001403 /* TerminalPanelView.swift in Sources */, A5001404 /* BrowserPanelView.swift in Sources */, A5007420 /* BrowserPopupWindowController.swift in Sources */, diff --git a/Sources/Panels/BrowserAgentSession.swift b/Sources/Panels/BrowserAgentSession.swift new file mode 100644 index 000000000..495c0e994 --- /dev/null +++ b/Sources/Panels/BrowserAgentSession.swift @@ -0,0 +1,30 @@ +import Foundation + +/// Represents one agent's isolated browser session against a specific profile. +/// +/// Each `BrowserAgentSession` maps an agent terminal surface to a cloned +/// `WKWebsiteDataStore` derived from a source `BrowserProfileDefinition`. +/// Multiple agents sharing the same imported profile each get their own +/// session with independent cookie state (clone-on-first-use). +/// +/// ``` +/// Agent -> BrowserAgentSession -> BrowserProfileDefinition -> WKWebsiteDataStore +/// ``` +struct BrowserAgentSession: Codable, Identifiable, Sendable { + /// Unique identifier for this agent session. + let id: UUID + + /// Internal UUID of the agent's terminal surface that owns this session. + let agentSurfaceUUID: UUID + + /// The original imported profile this session was cloned from. + let sourceProfileId: UUID + + /// Agent-specific `WKWebsiteDataStore` identifier (used with + /// `WKWebsiteDataStore(forIdentifier:)`). Cookies from `sourceProfileId` + /// are cloned into this store on first use. + let dataStoreId: UUID + + /// When this session was created. + let createdAt: Date +} diff --git a/Sources/Panels/BrowserAgentSessionStore.swift b/Sources/Panels/BrowserAgentSessionStore.swift new file mode 100644 index 000000000..7cfef0e1e --- /dev/null +++ b/Sources/Panels/BrowserAgentSessionStore.swift @@ -0,0 +1,256 @@ +#if canImport(WebKit) +import Foundation +import WebKit + +// MARK: - Cookie Cloning Extension + +extension WKWebsiteDataStore { + /// Clone all cookies from this store to the destination. + /// + /// **Must** be called (and awaited) before any navigation in the + /// destination's `WKWebView`. The clone completes before the WebView + /// loads its first URL. + func cloneCookies(to destination: WKWebsiteDataStore) async { + let cookies = await self.httpCookieStore.allCookies() + for cookie in cookies { + await destination.httpCookieStore.setCookie(cookie) + } + } +} + +// MARK: - On-Disk Manifest + +/// Lightweight JSON manifest persisted to disk so that orphaned +/// `WKWebsiteDataStore` files can be garbage-collected after a crash. +private struct AgentSessionManifest: Codable { + var dataStoreIds: [UUID] +} + +// MARK: - BrowserAgentSessionStore + +/// Manages `BrowserAgentSession` instances, providing clone-on-first-use +/// cookie isolation per agent + profile pair. +/// +/// Follows the same singleton / `@MainActor` / `ObservableObject` pattern +/// as `BrowserProfileStore`. +@MainActor +final class BrowserAgentSessionStore: ObservableObject { + static let shared = BrowserAgentSessionStore() + + /// Upper bound on concurrent agent sessions to limit disk usage. + static let maxConcurrentSessions = 10 + + // MARK: Published State + + @Published private(set) var sessions: [BrowserAgentSession] = [] + + // MARK: Private State + + /// Per-agent-profile serialization: if a cookie clone is already in + /// progress for a given `(agentUUID, profileId)` pair, subsequent + /// callers await the existing `Task` instead of starting a duplicate. + /// Key format: `"\(agentUUID)-\(profileId)"`. + private var cloneInFlight: [String: Task] = [:] + + /// On-disk manifest of active session `dataStoreId`s for crash-recovery GC. + /// Located at `~/Library/Application Support/{bundleId}/agent_sessions_manifest.json`. + let manifestURL: URL + + // MARK: Init + + init() { + let fm = FileManager.default + let appSupport = fm.urls(for: .applicationSupportDirectory, in: .userDomainMask).first! + let bundleId = Bundle.main.bundleIdentifier ?? "cmux" + let container = appSupport.appendingPathComponent(bundleId, isDirectory: true) + self.manifestURL = container.appendingPathComponent( + "agent_sessions_manifest.json", + isDirectory: false + ) + } + + // MARK: - Get or Create + + /// Return an existing session for the agent + profile pair, or create a + /// new one with a fresh cookie clone. + /// + /// Thread-safe against concurrent calls for the same pair: uses + /// `cloneInFlight` to deduplicate in-flight clones. + func getOrCreate(agentSurfaceUUID: UUID, profileId: UUID) async -> BrowserAgentSession { + // Fast path: return existing session. + if let existing = sessions.first(where: { + $0.agentSurfaceUUID == agentSurfaceUUID && $0.sourceProfileId == profileId + }) { + return existing + } + + // Enforce session limit. + guard sessions.count < Self.maxConcurrentSessions else { + // If at capacity, still check for a matching in-flight clone. + let key = "\(agentSurfaceUUID)-\(profileId)" + if let inFlight = cloneInFlight[key] { + return await inFlight.value + } + // Return a placeholder error session — callers should check + // session count before calling. In practice the socket handler + // will return a `session_limit_exceeded` error before reaching + // here. Create a degenerate session so the compiler is happy. + let degenerate = BrowserAgentSession( + id: UUID(), + agentSurfaceUUID: agentSurfaceUUID, + sourceProfileId: profileId, + dataStoreId: UUID(), + createdAt: Date() + ) + return degenerate + } + + let key = "\(agentSurfaceUUID)-\(profileId)" + + // If a clone is already in flight for this pair, await it. + if let inFlight = cloneInFlight[key] { + return await inFlight.value + } + + // Start a new clone task. + let task = Task { @MainActor in + let session = BrowserAgentSession( + id: UUID(), + agentSurfaceUUID: agentSurfaceUUID, + sourceProfileId: profileId, + dataStoreId: UUID(), + createdAt: Date() + ) + + // Clone cookies from the source profile's data store into the + // new agent-specific store. + let sourceStore = BrowserProfileStore.shared.websiteDataStore(for: profileId) + let agentStore = WKWebsiteDataStore(forIdentifier: session.dataStoreId) + await sourceStore.cloneCookies(to: agentStore) + + self.sessions.append(session) + self.cloneInFlight.removeValue(forKey: key) + self.persistManifest() + + return session + } + + cloneInFlight[key] = task + return await task.value + } + + // MARK: - Dispose + + /// Dispose a session: remove from the in-memory list and delete the + /// `WKWebsiteDataStore` from disk. + /// + /// Callers are responsible for closing owned `BrowserPanel`s (skipping + /// the undo stack) before or after calling this method. + func dispose(sessionId: UUID) async { + guard let index = sessions.firstIndex(where: { $0.id == sessionId }) else { return } + let session = sessions.remove(at: index) + + // Remove the cloned data store from disk. + await withCheckedContinuation { (continuation: CheckedContinuation) in + WKWebsiteDataStore.remove(forIdentifier: session.dataStoreId) { _ in + continuation.resume() + } + } + + persistManifest() + } + + // MARK: - Queries + + /// All sessions owned by a specific agent terminal surface. + func sessionsForAgent(_ agentUUID: UUID) -> [BrowserAgentSession] { + sessions.filter { $0.agentSurfaceUUID == agentUUID } + } + + /// All sessions cloned from a specific source profile. + func sessionsForProfile(_ profileId: UUID) -> [BrowserAgentSession] { + sessions.filter { $0.sourceProfileId == profileId } + } + + /// Computed tab count for a session. + /// + /// Derived by counting live `BrowserPanel` instances whose + /// `agentSessionId` matches. This avoids storing a stale count. + /// + /// - Note: The actual panel enumeration depends on the workspace / + /// tab-manager layer providing a way to iterate panels. For now + /// this returns 0 as a placeholder — wired up in Phase 1 socket + /// integration when `BrowserPanel.agentSessionId` is added. + func tabCount(for sessionId: UUID) -> Int { + // TODO: Wire to workspace panel enumeration once BrowserPanel + // gains agentSessionId property. + return 0 + } + + // MARK: - Agent Disconnect + + /// Called when a socket connection drops (agent crash / exit). + /// Disposes all sessions owned by the disconnected agent surface. + func handleAgentDisconnect(agentSurfaceUUID: UUID) async { + let orphaned = sessionsForAgent(agentSurfaceUUID) + for session in orphaned { + await dispose(sessionId: session.id) + } + } + + // MARK: - Garbage Collection + + /// Called on app launch: remove orphaned `WKWebsiteDataStore` files + /// left behind by sessions that were not cleanly disposed (e.g. crash). + /// + /// Reads the on-disk manifest and removes any data store whose ID is + /// not referenced by a live session. + func garbageCollect() async { + let manifest = loadManifest() + guard !manifest.dataStoreIds.isEmpty else { return } + + let liveIds = Set(sessions.map(\.dataStoreId)) + + for storeId in manifest.dataStoreIds where !liveIds.contains(storeId) { + await withCheckedContinuation { (continuation: CheckedContinuation) in + WKWebsiteDataStore.remove(forIdentifier: storeId) { _ in + continuation.resume() + } + } + } + + // Rewrite manifest with only live entries (if any). + persistManifest() + } + + // MARK: - Manifest Persistence + + /// Persist the current set of `dataStoreId`s to disk so they can be + /// cleaned up after a crash. + private func persistManifest() { + let manifest = AgentSessionManifest(dataStoreIds: sessions.map(\.dataStoreId)) + do { + let data = try JSONEncoder().encode(manifest) + let directory = manifestURL.deletingLastPathComponent() + try FileManager.default.createDirectory( + at: directory, + withIntermediateDirectories: true + ) + try data.write(to: manifestURL, options: .atomic) + } catch { + #if DEBUG + print("[BrowserAgentSessionStore] Failed to persist manifest: \(error)") + #endif + } + } + + /// Load the on-disk manifest. Returns an empty manifest on any error. + private func loadManifest() -> AgentSessionManifest { + guard let data = try? Data(contentsOf: manifestURL), + let manifest = try? JSONDecoder().decode(AgentSessionManifest.self, from: data) else { + return AgentSessionManifest(dataStoreIds: []) + } + return manifest + } +} +#endif diff --git a/Sources/Panels/BrowserPanel.swift b/Sources/Panels/BrowserPanel.swift index f0bb3778a..41f43c3c3 100644 --- a/Sources/Panels/BrowserPanel.swift +++ b/Sources/Panels/BrowserPanel.swift @@ -1865,6 +1865,10 @@ final class BrowserPanel: Panel, ObservableObject { @Published private(set) var profileID: UUID @Published private(set) var historyStore: BrowserHistoryStore + /// If non-nil, this panel is owned by an agent session and subject to + /// tab-level isolation rules. See `BrowserAgentSessionStore`. + private(set) var agentSessionId: UUID? + /// The underlying web view private(set) var webView: WKWebView private var websiteDataStore: WKWebsiteDataStore @@ -2566,10 +2570,13 @@ final class BrowserPanel: Panel, ObservableObject { bypassInsecureHTTPHostOnce: String? = nil, proxyEndpoint: BrowserProxyEndpoint? = nil, isRemoteWorkspace: Bool = false, - remoteWebsiteDataStoreIdentifier: UUID? = nil + remoteWebsiteDataStoreIdentifier: UUID? = nil, + agentSessionId: UUID? = nil, + agentDataStoreId: UUID? = nil ) { self.id = UUID() self.workspaceId = workspaceId + self.agentSessionId = agentSessionId let requestedProfileID = profileID ?? BrowserProfileStore.shared.effectiveLastUsedProfileID let resolvedProfileID = BrowserProfileStore.shared.profileDefinition(id: requestedProfileID) != nil ? requestedProfileID @@ -2580,9 +2587,14 @@ final class BrowserPanel: Panel, ObservableObject { self.remoteProxyEndpoint = proxyEndpoint self.usesRemoteWorkspaceProxy = isRemoteWorkspace self.browserThemeMode = BrowserThemeSettings.mode() - self.websiteDataStore = isRemoteWorkspace - ? WKWebsiteDataStore(forIdentifier: remoteWebsiteDataStoreIdentifier ?? workspaceId) - : BrowserProfileStore.shared.websiteDataStore(for: resolvedProfileID) + if let agentDataStoreId { + // Agent session: use the agent-specific cloned data store. + self.websiteDataStore = WKWebsiteDataStore(forIdentifier: agentDataStoreId) + } else if isRemoteWorkspace { + self.websiteDataStore = WKWebsiteDataStore(forIdentifier: remoteWebsiteDataStoreIdentifier ?? workspaceId) + } else { + self.websiteDataStore = BrowserProfileStore.shared.websiteDataStore(for: resolvedProfileID) + } let webView = Self.makeWebView( profileID: resolvedProfileID, @@ -2782,6 +2794,10 @@ final class BrowserPanel: Panel, ObservableObject { @discardableResult func switchToProfile(_ requestedProfileID: UUID) -> Bool { + // Agent-owned panels cannot switch profiles — agents must dispose and + // recreate to change profile. + guard agentSessionId == nil else { return false } + let resolvedProfileID = BrowserProfileStore.shared.profileDefinition(id: requestedProfileID) != nil ? requestedProfileID : BrowserProfileStore.shared.builtInDefaultProfileID diff --git a/Sources/SessionPersistence.swift b/Sources/SessionPersistence.swift index 188833d40..11988a0f3 100644 --- a/Sources/SessionPersistence.swift +++ b/Sources/SessionPersistence.swift @@ -234,6 +234,8 @@ struct SessionBrowserPanelSnapshot: Codable, Sendable { var developerToolsVisible: Bool var backHistoryURLStrings: [String]? var forwardHistoryURLStrings: [String]? + /// Agent-owned panels are discarded on restore (agents must reconnect). + var agentSessionId: UUID? } struct SessionMarkdownPanelSnapshot: Codable, Sendable { diff --git a/Sources/TerminalController.swift b/Sources/TerminalController.swift index c6509f464..ac8993b16 100644 --- a/Sources/TerminalController.swift +++ b/Sources/TerminalController.swift @@ -2341,6 +2341,16 @@ class TerminalController { case "browser.input_touch": return v2Result(id: id, self.v2BrowserInputTouch(params: params)) + // Agent browser sessions + case "browser.agent_session.open": + return v2Result(id: id, self.v2BrowserAgentSessionOpen(params: params)) + case "browser.agent_session.tab": + return v2Result(id: id, self.v2BrowserAgentSessionTab(params: params)) + case "browser.agent_session.dispose": + return v2Result(id: id, self.v2BrowserAgentSessionDispose(params: params)) + case "browser.agent_session.list": + return v2Result(id: id, self.v2BrowserAgentSessionList(params: params)) + // Markdown case "markdown.open": return v2Result(id: id, self.v2MarkdownOpen(params: params)) @@ -9797,6 +9807,216 @@ class TerminalController { return result } + // MARK: - Agent Browser Session Commands + + /// Create a new browser tab with an agent session (clone-on-first-use cookie isolation). + /// Params: profile_id (UUID), url? (String), pane_id? (UUID) + /// Caller identity resolved from params["caller"]["surface_id"]. + private func v2BrowserAgentSessionOpen(params: [String: Any]) -> V2CallResult { + guard let tabManager = v2ResolveTabManager(params: params) else { + return .err(code: "unavailable", message: "TabManager not available", data: nil) + } + guard let profileId = v2UUID(params, "profile_id") else { + return .err(code: "invalid_params", message: "Missing profile_id", data: nil) + } + // Resolve caller identity from the existing caller parameter infrastructure. + guard let callerObj = params["caller"] as? [String: Any], + let callerSurfaceUUID = v2UUIDAny(callerObj["surface_id"]) else { + return .err(code: "unauthorized", message: "Missing or invalid caller surface identity", data: nil) + } + guard BrowserProfileStore.shared.profileDefinition(id: profileId) != nil else { + return .err(code: "profile_not_found", message: "Profile not found", data: nil) + } + guard BrowserAgentSessionStore.shared.sessions.count < BrowserAgentSessionStore.maxConcurrentSessions else { + return .err(code: "session_limit_exceeded", + message: "Maximum \(BrowserAgentSessionStore.maxConcurrentSessions) concurrent agent sessions", + data: nil) + } + + let url = v2String(params, "url").flatMap(URL.init(string:)) + var result: V2CallResult = .err(code: "internal_error", message: "Failed to create agent browser session", data: nil) + + // Cookie clone is async; we need to bridge to sync for the socket handler. + let semaphore = DispatchSemaphore(value: 0) + var session: BrowserAgentSession? + + Task { @MainActor in + session = await BrowserAgentSessionStore.shared.getOrCreate( + agentSurfaceUUID: callerSurfaceUUID, + profileId: profileId + ) + semaphore.signal() + } + semaphore.wait() + + guard let session else { + return .err(code: "internal_error", message: "Failed to create agent session", data: nil) + } + + v2MainSync { + guard let ws = v2ResolveWorkspace(params: params, tabManager: tabManager) else { + result = .err(code: "not_found", message: "Workspace not found", data: nil) + return + } + let paneUUID = v2UUID(params, "pane_id") + ?? ws.bonsplitController.focusedPaneId?.id + guard let paneUUID, + let pane = ws.bonsplitController.allPaneIds.first(where: { $0.id == paneUUID }) else { + result = .err(code: "not_found", message: "Pane not found", data: nil) + return + } + + guard let panel = ws.newBrowserSurface( + inPane: pane, + url: url, + focus: false, + preferredProfileID: profileId, + agentSessionId: session.id, + agentDataStoreId: session.dataStoreId + ) else { + result = .err(code: "internal_error", message: "Failed to create tab", data: nil) + return + } + + result = .ok([ + "surface_id": panel.id.uuidString, + "surface_ref": v2Ref(kind: .surface, uuid: panel.id), + "agent_session_id": session.id.uuidString, + "workspace_id": ws.id.uuidString, + "workspace_ref": v2Ref(kind: .workspace, uuid: ws.id), + "pane_id": pane.id.uuidString, + "pane_ref": v2Ref(kind: .pane, uuid: pane.id), + ]) + } + return result + } + + /// Open an additional tab in an existing agent session. + /// Params: session_id (UUID), url (String), pane_id? (UUID) + private func v2BrowserAgentSessionTab(params: [String: Any]) -> V2CallResult { + guard let tabManager = v2ResolveTabManager(params: params) else { + return .err(code: "unavailable", message: "TabManager not available", data: nil) + } + guard let sessionId = v2UUID(params, "session_id") else { + return .err(code: "invalid_params", message: "Missing session_id", data: nil) + } + guard let callerObj = params["caller"] as? [String: Any], + let callerSurfaceUUID = v2UUIDAny(callerObj["surface_id"]) else { + return .err(code: "unauthorized", message: "Missing caller identity", data: nil) + } + + guard let session = BrowserAgentSessionStore.shared.sessions.first(where: { $0.id == sessionId }) else { + return .err(code: "session_not_found", message: "Agent session not found", data: nil) + } + guard session.agentSurfaceUUID == callerSurfaceUUID else { + return .err(code: "unauthorized", message: "Session owned by different agent", data: nil) + } + + let url = v2String(params, "url").flatMap(URL.init(string:)) + var result: V2CallResult = .err(code: "internal_error", message: "Failed to create tab", data: nil) + + v2MainSync { + guard let ws = v2ResolveWorkspace(params: params, tabManager: tabManager) else { + result = .err(code: "not_found", message: "Workspace not found", data: nil) + return + } + let paneUUID = v2UUID(params, "pane_id") + ?? ws.bonsplitController.focusedPaneId?.id + guard let paneUUID, + let pane = ws.bonsplitController.allPaneIds.first(where: { $0.id == paneUUID }) else { + result = .err(code: "not_found", message: "Pane not found", data: nil) + return + } + + guard let panel = ws.newBrowserSurface( + inPane: pane, + url: url, + focus: false, + preferredProfileID: session.sourceProfileId, + agentSessionId: session.id, + agentDataStoreId: session.dataStoreId + ) else { + result = .err(code: "internal_error", message: "Failed to create tab", data: nil) + return + } + + result = .ok([ + "surface_id": panel.id.uuidString, + "surface_ref": v2Ref(kind: .surface, uuid: panel.id), + "workspace_id": ws.id.uuidString, + "workspace_ref": v2Ref(kind: .workspace, uuid: ws.id), + "pane_id": pane.id.uuidString, + "pane_ref": v2Ref(kind: .pane, uuid: pane.id), + ]) + } + return result + } + + /// Dispose an agent session: close all owned tabs and remove the data store from disk. + /// Params: session_id (UUID) + private func v2BrowserAgentSessionDispose(params: [String: Any]) -> V2CallResult { + guard let sessionId = v2UUID(params, "session_id") else { + return .err(code: "invalid_params", message: "Missing session_id", data: nil) + } + guard let callerObj = params["caller"] as? [String: Any], + let callerSurfaceUUID = v2UUIDAny(callerObj["surface_id"]) else { + return .err(code: "unauthorized", message: "Missing caller identity", data: nil) + } + guard let session = BrowserAgentSessionStore.shared.sessions.first(where: { $0.id == sessionId }) else { + return .err(code: "session_not_found", message: "Agent session not found", data: nil) + } + guard session.agentSurfaceUUID == callerSurfaceUUID else { + return .err(code: "unauthorized", message: "Session owned by different agent", data: nil) + } + + // Close all browser panels owned by this session (skip undo stack). + v2MainSync { + guard let tabManager = v2ResolveTabManager(params: params) else { return } + for ws in tabManager.tabs { + let ownedPanelIds = ws.panels.values + .compactMap { $0 as? BrowserPanel } + .filter { $0.agentSessionId == sessionId } + .map(\.id) + for panelId in ownedPanelIds { + ws.closePanel(panelId, force: true) + } + } + } + + // Dispose the session (removes data store from disk). + let semaphore = DispatchSemaphore(value: 0) + Task { @MainActor in + await BrowserAgentSessionStore.shared.dispose(sessionId: sessionId) + semaphore.signal() + } + semaphore.wait() + + return .ok(["disposed": true, "session_id": sessionId.uuidString]) + } + + /// List agent sessions, optionally filtered by agent surface. + /// Params: agent_surface_id? (UUID) + private func v2BrowserAgentSessionList(params: [String: Any]) -> V2CallResult { + let agentFilter = v2UUID(params, "agent_surface_id") + let sessions: [BrowserAgentSession] + if let agentFilter { + sessions = BrowserAgentSessionStore.shared.sessionsForAgent(agentFilter) + } else { + sessions = BrowserAgentSessionStore.shared.sessions + } + + let entries: [[String: Any]] = sessions.map { session in + [ + "session_id": session.id.uuidString, + "agent_surface_id": session.agentSurfaceUUID.uuidString, + "source_profile_id": session.sourceProfileId.uuidString, + "profile_name": BrowserProfileStore.shared.displayName(for: session.sourceProfileId), + "created_at": ISO8601DateFormatter().string(from: session.createdAt), + ] + } + return .ok(["sessions": entries]) + } + private func v2BrowserTabSwitch(params: [String: Any]) -> V2CallResult { guard let tabManager = v2ResolveTabManager(params: params) else { return .err(code: "unavailable", message: "TabManager not available", data: nil) diff --git a/Sources/Workspace.swift b/Sources/Workspace.swift index 476204643..0ff1e70e3 100644 --- a/Sources/Workspace.swift +++ b/Sources/Workspace.swift @@ -7281,7 +7281,9 @@ final class Workspace: Identifiable, ObservableObject { focus: Bool? = nil, insertAtEnd: Bool = false, preferredProfileID: UUID? = nil, - bypassInsecureHTTPHostOnce: String? = nil + bypassInsecureHTTPHostOnce: String? = nil, + agentSessionId: UUID? = nil, + agentDataStoreId: UUID? = nil ) -> BrowserPanel? { let shouldFocusNewTab = focus ?? (bonsplitController.focusedPaneId == paneId) let sourcePanelId = effectiveSelectedPanelId(inPane: paneId) @@ -7298,7 +7300,9 @@ final class Workspace: Identifiable, ObservableObject { bypassInsecureHTTPHostOnce: bypassInsecureHTTPHostOnce, proxyEndpoint: remoteProxyEndpoint, isRemoteWorkspace: isRemoteWorkspace, - remoteWebsiteDataStoreIdentifier: isRemoteWorkspace ? id : nil + remoteWebsiteDataStoreIdentifier: isRemoteWorkspace ? id : nil, + agentSessionId: agentSessionId, + agentDataStoreId: agentDataStoreId ) panels[browserPanel.id] = browserPanel panelTitles[browserPanel.id] = browserPanel.displayTitle From b8ac0fc8b837171f91ddf5f0a38a164f64cf7166 Mon Sep 17 00:00:00 2001 From: lark Date: Wed, 25 Mar 2026 04:49:33 +0900 Subject: [PATCH 2/8] Fix review findings: deadlock, snapshot, restore, auth, GC wiring Address all must-fix issues from 3-agent code review: - Fix caller identity: verify surface UUID exists in live workspace before trusting params['caller']['surface_id'] - Fix session snapshot: include agentSessionId in persistence - Fix session restore: skip agent-owned panels on app restart - Fix degenerate session: getOrCreate returns nil at capacity instead of untracked orphan session - Fix session limit: remove premature guard that blocked reuse of existing sessions at capacity - Fix list command: wrap in v2MainSync, restrict to caller's own sessions by default - Wire GC: call garbageCollect() on app launch in AppDelegate - Wire disconnect: track agent surfaces per connection, call handleAgentDisconnect on socket close - Implement tabCount: enumerate workspace panels for real count Co-Authored-By: Claude Opus 4.6 (1M context) --- Sources/AppDelegate.swift | 7 + Sources/Panels/BrowserAgentSessionStore.swift | 36 +++-- Sources/TerminalController.swift | 143 +++++++++++++++--- Sources/Workspace.swift | 8 +- 4 files changed, 152 insertions(+), 42 deletions(-) diff --git a/Sources/AppDelegate.swift b/Sources/AppDelegate.swift index e17b02090..596900ff9 100644 --- a/Sources/AppDelegate.swift +++ b/Sources/AppDelegate.swift @@ -2326,6 +2326,13 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent NSWindow.allowsAutomaticWindowTabbing = false disableNativeTabbingShortcut() ensureApplicationIcon() + + // Garbage-collect orphaned agent session data stores left by unclean shutdown. + #if canImport(WebKit) + Task { @MainActor in + await BrowserAgentSessionStore.shared.garbageCollect() + } + #endif if !isRunningUnderXCTest { configureUserNotifications() installMenuBarVisibilityObserver() diff --git a/Sources/Panels/BrowserAgentSessionStore.swift b/Sources/Panels/BrowserAgentSessionStore.swift index 7cfef0e1e..c0c92c899 100644 --- a/Sources/Panels/BrowserAgentSessionStore.swift +++ b/Sources/Panels/BrowserAgentSessionStore.swift @@ -1,4 +1,5 @@ #if canImport(WebKit) +import AppKit import Foundation import WebKit @@ -76,7 +77,7 @@ final class BrowserAgentSessionStore: ObservableObject { /// /// Thread-safe against concurrent calls for the same pair: uses /// `cloneInFlight` to deduplicate in-flight clones. - func getOrCreate(agentSurfaceUUID: UUID, profileId: UUID) async -> BrowserAgentSession { + func getOrCreate(agentSurfaceUUID: UUID, profileId: UUID) async -> BrowserAgentSession? { // Fast path: return existing session. if let existing = sessions.first(where: { $0.agentSurfaceUUID == agentSurfaceUUID && $0.sourceProfileId == profileId @@ -91,18 +92,9 @@ final class BrowserAgentSessionStore: ObservableObject { if let inFlight = cloneInFlight[key] { return await inFlight.value } - // Return a placeholder error session — callers should check - // session count before calling. In practice the socket handler - // will return a `session_limit_exceeded` error before reaching - // here. Create a degenerate session so the compiler is happy. - let degenerate = BrowserAgentSession( - id: UUID(), - agentSurfaceUUID: agentSurfaceUUID, - sourceProfileId: profileId, - dataStoreId: UUID(), - createdAt: Date() - ) - return degenerate + // At capacity with no in-flight clone — return nil so callers + // can report a proper error instead of leaking an untracked store. + return nil } let key = "\(agentSurfaceUUID)-\(profileId)" @@ -182,9 +174,21 @@ final class BrowserAgentSessionStore: ObservableObject { /// this returns 0 as a placeholder — wired up in Phase 1 socket /// integration when `BrowserPanel.agentSessionId` is added. func tabCount(for sessionId: UUID) -> Int { - // TODO: Wire to workspace panel enumeration once BrowserPanel - // gains agentSessionId property. - return 0 + guard let app = AppDelegate.shared else { return 0 } + var count = 0 + let windows = app.listMainWindowSummaries() + for item in windows { + guard let tm = app.tabManagerFor(windowId: item.windowId) else { continue } + for ws in tm.tabs { + for panel in ws.panels.values { + if let browser = panel as? BrowserPanel, + browser.agentSessionId == sessionId { + count += 1 + } + } + } + } + return count } // MARK: - Agent Disconnect diff --git a/Sources/TerminalController.swift b/Sources/TerminalController.swift index ac8993b16..a541f517a 100644 --- a/Sources/TerminalController.swift +++ b/Sources/TerminalController.swift @@ -1605,6 +1605,9 @@ class TerminalController { var buffer = [UInt8](repeating: 0, count: 4096) var pending = "" var authenticated = false + // Track agent surface UUIDs that opened sessions on this connection + // so we can clean up on disconnect. + var agentSurfaceUUIDs = Set() while withListenerState({ isRunning }) { let bytesRead = read(socket, &buffer, buffer.count - 1) @@ -1624,10 +1627,33 @@ class TerminalController { continue } + // Extract agent surface UUID from agent session commands for disconnect cleanup. + if trimmed.hasPrefix("{"), trimmed.contains("browser.agent_session.open") { + if let data = trimmed.data(using: .utf8), + let json = try? JSONSerialization.jsonObject(with: data) as? [String: Any], + let params = json["params"] as? [String: Any], + let caller = params["caller"] as? [String: Any], + let surfaceIdStr = caller["surface_id"] as? String, + let surfaceUUID = UUID(uuidString: surfaceIdStr) { + agentSurfaceUUIDs.insert(surfaceUUID) + } + } + let response = processCommand(trimmed) writeSocketResponse(response, to: socket) } } + + // On connection close, clean up any agent sessions owned by surfaces + // that were active on this connection. + if !agentSurfaceUUIDs.isEmpty { + let uuids = agentSurfaceUUIDs + Task { @MainActor in + for uuid in uuids { + await BrowserAgentSessionStore.shared.handleAgentDisconnect(agentSurfaceUUID: uuid) + } + } + } } private func processCommand(_ command: String) -> String { @@ -3123,6 +3149,22 @@ class TerminalController { return nil } + /// Verify that a caller-provided surface UUID exists as a live panel in some workspace. + /// Must be called on main thread (inside v2MainSync). + private func v2VerifyCallerSurface(_ surfaceUUID: UUID) -> Bool { + guard let app = AppDelegate.shared else { return false } + let windows = app.listMainWindowSummaries() + for item in windows { + guard let tm = app.tabManagerFor(windowId: item.windowId) else { continue } + for ws in tm.tabs { + if ws.panels[surfaceUUID] != nil { + return true + } + } + } + return false + } + private func v2LocatePane(_ paneUUID: UUID) -> (windowId: UUID, tabManager: TabManager, workspace: Workspace, paneId: PaneID)? { guard let app = AppDelegate.shared else { return nil } let windows = app.listMainWindowSummaries() @@ -9824,19 +9866,29 @@ class TerminalController { let callerSurfaceUUID = v2UUIDAny(callerObj["surface_id"]) else { return .err(code: "unauthorized", message: "Missing or invalid caller surface identity", data: nil) } - guard BrowserProfileStore.shared.profileDefinition(id: profileId) != nil else { - return .err(code: "profile_not_found", message: "Profile not found", data: nil) + + // Verify the caller surface_id exists in a live workspace. + var callerVerified = false + v2MainSync { + callerVerified = v2VerifyCallerSurface(callerSurfaceUUID) } - guard BrowserAgentSessionStore.shared.sessions.count < BrowserAgentSessionStore.maxConcurrentSessions else { - return .err(code: "session_limit_exceeded", - message: "Maximum \(BrowserAgentSessionStore.maxConcurrentSessions) concurrent agent sessions", - data: nil) + guard callerVerified else { + return .err(code: "unauthorized", message: "Caller surface not found in any workspace", data: nil) + } + + var profileExists = false + v2MainSync { + profileExists = BrowserProfileStore.shared.profileDefinition(id: profileId) != nil + } + guard profileExists else { + return .err(code: "profile_not_found", message: "Profile not found", data: nil) } let url = v2String(params, "url").flatMap(URL.init(string:)) var result: V2CallResult = .err(code: "internal_error", message: "Failed to create agent browser session", data: nil) - // Cookie clone is async; we need to bridge to sync for the socket handler. + // Cookie clone is async; bridge to sync for the socket handler. + // handleClient runs on a background queue so semaphore.wait() does NOT block main. let semaphore = DispatchSemaphore(value: 0) var session: BrowserAgentSession? @@ -9850,7 +9902,9 @@ class TerminalController { semaphore.wait() guard let session else { - return .err(code: "internal_error", message: "Failed to create agent session", data: nil) + return .err(code: "session_limit_exceeded", + message: "Maximum \(BrowserAgentSessionStore.maxConcurrentSessions) concurrent agent sessions", + data: nil) } v2MainSync { @@ -9905,7 +9959,17 @@ class TerminalController { return .err(code: "unauthorized", message: "Missing caller identity", data: nil) } - guard let session = BrowserAgentSessionStore.shared.sessions.first(where: { $0.id == sessionId }) else { + // Verify the caller surface_id exists in a live workspace. + var callerVerified = false + var session: BrowserAgentSession? + v2MainSync { + callerVerified = v2VerifyCallerSurface(callerSurfaceUUID) + session = BrowserAgentSessionStore.shared.sessions.first(where: { $0.id == sessionId }) + } + guard callerVerified else { + return .err(code: "unauthorized", message: "Caller surface not found in any workspace", data: nil) + } + guard let session else { return .err(code: "session_not_found", message: "Agent session not found", data: nil) } guard session.agentSurfaceUUID == callerSurfaceUUID else { @@ -9962,7 +10026,18 @@ class TerminalController { let callerSurfaceUUID = v2UUIDAny(callerObj["surface_id"]) else { return .err(code: "unauthorized", message: "Missing caller identity", data: nil) } - guard let session = BrowserAgentSessionStore.shared.sessions.first(where: { $0.id == sessionId }) else { + + // Verify the caller surface_id exists in a live workspace and validate session ownership. + var callerVerified = false + var session: BrowserAgentSession? + v2MainSync { + callerVerified = v2VerifyCallerSurface(callerSurfaceUUID) + session = BrowserAgentSessionStore.shared.sessions.first(where: { $0.id == sessionId }) + } + guard callerVerified else { + return .err(code: "unauthorized", message: "Caller surface not found in any workspace", data: nil) + } + guard let session else { return .err(code: "session_not_found", message: "Agent session not found", data: nil) } guard session.agentSurfaceUUID == callerSurfaceUUID else { @@ -9984,6 +10059,7 @@ class TerminalController { } // Dispose the session (removes data store from disk). + // handleClient runs on a background queue so semaphore.wait() does NOT block main. let semaphore = DispatchSemaphore(value: 0) Task { @MainActor in await BrowserAgentSessionStore.shared.dispose(sessionId: sessionId) @@ -9997,24 +10073,41 @@ class TerminalController { /// List agent sessions, optionally filtered by agent surface. /// Params: agent_surface_id? (UUID) private func v2BrowserAgentSessionList(params: [String: Any]) -> V2CallResult { + // Resolve caller identity — when no explicit agent_surface_id filter is provided, + // only return sessions owned by the verified caller to prevent information leakage. + let callerSurfaceUUID: UUID? = { + guard let callerObj = params["caller"] as? [String: Any] else { return nil } + return v2UUIDAny(callerObj["surface_id"]) + }() + let agentFilter = v2UUID(params, "agent_surface_id") - let sessions: [BrowserAgentSession] - if let agentFilter { - sessions = BrowserAgentSessionStore.shared.sessionsForAgent(agentFilter) - } else { - sessions = BrowserAgentSessionStore.shared.sessions - } - let entries: [[String: Any]] = sessions.map { session in - [ - "session_id": session.id.uuidString, - "agent_surface_id": session.agentSurfaceUUID.uuidString, - "source_profile_id": session.sourceProfileId.uuidString, - "profile_name": BrowserProfileStore.shared.displayName(for: session.sourceProfileId), - "created_at": ISO8601DateFormatter().string(from: session.createdAt), - ] + var result: V2CallResult = .err(code: "internal_error", message: "Unexpected error", data: nil) + v2MainSync { + // When no explicit filter, use the caller's own surface UUID to scope results. + let effectiveFilter = agentFilter ?? callerSurfaceUUID + let sessions: [BrowserAgentSession] + if let effectiveFilter { + sessions = BrowserAgentSessionStore.shared.sessionsForAgent(effectiveFilter) + } else { + // No caller and no filter — return empty to avoid leaking all sessions. + sessions = [] + } + + let store = BrowserAgentSessionStore.shared + let entries: [[String: Any]] = sessions.map { session in + [ + "session_id": session.id.uuidString, + "agent_surface_id": session.agentSurfaceUUID.uuidString, + "source_profile_id": session.sourceProfileId.uuidString, + "profile_name": BrowserProfileStore.shared.displayName(for: session.sourceProfileId), + "tab_count": store.tabCount(for: session.id), + "created_at": ISO8601DateFormatter().string(from: session.createdAt), + ] + } + result = .ok(["sessions": entries]) } - return .ok(["sessions": entries]) + return result } private func v2BrowserTabSwitch(params: [String: Any]) -> V2CallResult { diff --git a/Sources/Workspace.swift b/Sources/Workspace.swift index 0ff1e70e3..92f009813 100644 --- a/Sources/Workspace.swift +++ b/Sources/Workspace.swift @@ -419,7 +419,8 @@ extension Workspace { pageZoom: Double(browserPanel.currentPageZoomFactor()), developerToolsVisible: browserPanel.isDeveloperToolsVisible(), backHistoryURLStrings: historySnapshot.backHistoryURLStrings, - forwardHistoryURLStrings: historySnapshot.forwardHistoryURLStrings + forwardHistoryURLStrings: historySnapshot.forwardHistoryURLStrings, + agentSessionId: browserPanel.agentSessionId ) markdownSnapshot = nil case .markdown: @@ -596,6 +597,11 @@ extension Workspace { applySessionPanelMetadata(snapshot, toPanelId: terminalPanel.id) return terminalPanel.id case .browser: + // Agent-owned browser panels are not restored — agents must reconnect + // and create new sessions with fresh cookie clones. + if snapshot.browser?.agentSessionId != nil { + return nil + } let initialURL = snapshot.browser?.urlString.flatMap { URL(string: $0) } guard let browserPanel = newBrowserSurface( inPane: paneId, From fa30359b0c5714c0b56f0de7197db0990616260b Mon Sep 17 00:00:00 2001 From: lark Date: Wed, 25 Mar 2026 04:55:59 +0900 Subject: [PATCH 3/8] Fix cross-review security findings: list scoping, disconnect handler - Restrict browser.agent_session.list to caller's own sessions only; explicit agent_surface_id filter must match caller UUID - Move disconnect handler tracking after processCommand succeeds and verify method == "browser.agent_session.open" via parsed JSON (prevents string-injection DoS) - Update tabCount documentation Co-Authored-By: Claude Opus 4.6 (1M context) --- Sources/Panels/BrowserAgentSessionStore.swift | 6 +-- Sources/TerminalController.swift | 38 +++++++++++-------- 2 files changed, 25 insertions(+), 19 deletions(-) diff --git a/Sources/Panels/BrowserAgentSessionStore.swift b/Sources/Panels/BrowserAgentSessionStore.swift index c0c92c899..9b30cfe24 100644 --- a/Sources/Panels/BrowserAgentSessionStore.swift +++ b/Sources/Panels/BrowserAgentSessionStore.swift @@ -169,10 +169,8 @@ final class BrowserAgentSessionStore: ObservableObject { /// Derived by counting live `BrowserPanel` instances whose /// `agentSessionId` matches. This avoids storing a stale count. /// - /// - Note: The actual panel enumeration depends on the workspace / - /// tab-manager layer providing a way to iterate panels. For now - /// this returns 0 as a placeholder — wired up in Phase 1 socket - /// integration when `BrowserPanel.agentSessionId` is added. + /// Enumerates all windows → tab managers → workspaces → panels + /// to count `BrowserPanel` instances matching the given session. func tabCount(for sessionId: UUID) -> Int { guard let app = AppDelegate.shared else { return 0 } var count = 0 diff --git a/Sources/TerminalController.swift b/Sources/TerminalController.swift index a541f517a..48548cf36 100644 --- a/Sources/TerminalController.swift +++ b/Sources/TerminalController.swift @@ -1627,20 +1627,23 @@ class TerminalController { continue } - // Extract agent surface UUID from agent session commands for disconnect cleanup. - if trimmed.hasPrefix("{"), trimmed.contains("browser.agent_session.open") { - if let data = trimmed.data(using: .utf8), - let json = try? JSONSerialization.jsonObject(with: data) as? [String: Any], - let params = json["params"] as? [String: Any], - let caller = params["caller"] as? [String: Any], - let surfaceIdStr = caller["surface_id"] as? String, - let surfaceUUID = UUID(uuidString: surfaceIdStr) { - agentSurfaceUUIDs.insert(surfaceUUID) - } - } - let response = processCommand(trimmed) writeSocketResponse(response, to: socket) + + // Track agent surface UUIDs from successful agent session opens for + // disconnect cleanup. Only register after processCommand succeeds + // (response contains "result", not "error") and method is verified. + if trimmed.hasPrefix("{"), + response.contains("\"agent_session_id\""), + let data = trimmed.data(using: .utf8), + let json = try? JSONSerialization.jsonObject(with: data) as? [String: Any], + (json["method"] as? String) == "browser.agent_session.open", + let params = json["params"] as? [String: Any], + let caller = params["caller"] as? [String: Any], + let surfaceIdStr = caller["surface_id"] as? String, + let surfaceUUID = UUID(uuidString: surfaceIdStr) { + agentSurfaceUUIDs.insert(surfaceUUID) + } } } @@ -10084,13 +10087,18 @@ class TerminalController { var result: V2CallResult = .err(code: "internal_error", message: "Unexpected error", data: nil) v2MainSync { - // When no explicit filter, use the caller's own surface UUID to scope results. - let effectiveFilter = agentFilter ?? callerSurfaceUUID + // Always scope to caller's own sessions. Explicit agent_surface_id filter + // is only allowed if it matches the caller (prevents cross-agent enumeration). + let effectiveFilter: UUID? + if let agentFilter, let callerSurfaceUUID, agentFilter == callerSurfaceUUID { + effectiveFilter = agentFilter + } else { + effectiveFilter = callerSurfaceUUID + } let sessions: [BrowserAgentSession] if let effectiveFilter { sessions = BrowserAgentSessionStore.shared.sessionsForAgent(effectiveFilter) } else { - // No caller and no filter — return empty to avoid leaking all sessions. sessions = [] } From d9bc5378dedafe41a6a612e4a5968e257b2ce7a4 Mon Sep 17 00:00:00 2001 From: lark Date: Wed, 25 Mar 2026 04:57:58 +0900 Subject: [PATCH 4/8] Add CLI wrappers for browser agent-session commands cmux browser agent-session open --profile [--url ] cmux browser agent-session tab --session [--url ] cmux browser agent-session dispose --session cmux browser agent-session list [--json] Caller context (CMUX_SURFACE_ID, CMUX_WORKSPACE_ID) is automatically included from environment variables for agent identity resolution. Co-Authored-By: Claude Opus 4.6 (1M context) --- CLI/cmux.swift | 82 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 82 insertions(+) diff --git a/CLI/cmux.swift b/CLI/cmux.swift index 17279ef63..157942220 100644 --- a/CLI/cmux.swift +++ b/CLI/cmux.swift @@ -5839,6 +5839,88 @@ struct CMUXCLI { return } + // Agent session commands + func agentCallerContext() -> [String: Any] { + var caller: [String: Any] = [:] + if let sid = ProcessInfo.processInfo.environment["CMUX_SURFACE_ID"] { + caller["surface_id"] = sid + } + if let wid = ProcessInfo.processInfo.environment["CMUX_WORKSPACE_ID"] { + caller["workspace_id"] = wid + } + return caller + } + + if subcommand == "agent-session" { + guard let verb = subArgs.first?.lowercased() else { + throw CLIError(message: "browser agent-session requires: open|tab|dispose|list") + } + let verbArgs = Array(subArgs.dropFirst()) + + switch verb { + case "open": + guard let profileId = optionValue(verbArgs, name: "--profile") else { + throw CLIError(message: "browser agent-session open requires --profile ") + } + let url = optionValue(verbArgs, name: "--url") + let paneId = optionValue(verbArgs, name: "--pane") + var params: [String: Any] = ["profile_id": profileId] + if let url { params["url"] = url } + if let paneId { params["pane_id"] = paneId } + params["caller"] = agentCallerContext() + let payload = try client.sendV2(method: "browser.agent_session.open", params: params) + output(payload, fallback: "OK") + return + + case "tab": + guard let sessionId = optionValue(verbArgs, name: "--session") else { + throw CLIError(message: "browser agent-session tab requires --session ") + } + let url = optionValue(verbArgs, name: "--url") + let paneId = optionValue(verbArgs, name: "--pane") + var params: [String: Any] = ["session_id": sessionId] + if let url { params["url"] = url } + if let paneId { params["pane_id"] = paneId } + params["caller"] = agentCallerContext() + let payload = try client.sendV2(method: "browser.agent_session.tab", params: params) + output(payload, fallback: "OK") + return + + case "dispose": + guard let sessionId = optionValue(verbArgs, name: "--session") else { + throw CLIError(message: "browser agent-session dispose requires --session ") + } + var params: [String: Any] = ["session_id": sessionId] + params["caller"] = agentCallerContext() + let payload = try client.sendV2(method: "browser.agent_session.dispose", params: params) + output(payload, fallback: "OK") + return + + case "list": + var params: [String: Any] = [:] + params["caller"] = agentCallerContext() + let payload = try client.sendV2(method: "browser.agent_session.list", params: params) + if effectiveJSONOutput { + print(jsonString(formatIDs(payload, mode: effectiveIDFormat))) + } else if let sessions = payload["sessions"] as? [[String: Any]] { + if sessions.isEmpty { + print("No active agent sessions") + } else { + for s in sessions { + let sid = s["session_id"] as? String ?? "?" + let profile = s["profile_name"] as? String ?? "?" + let tabs = s["tab_count"] as? Int ?? 0 + print("\(sid) profile=\(profile) tabs=\(tabs)") + } + } + } + return + + default: + throw CLIError(message: "Unknown agent-session verb: \(verb). Use: open|tab|dispose|list") + } + } + throw CLIError(message: "Unsupported browser subcommand: \(subcommand)") } From 47cc0d1f63f393e43fc4196155096ed2767f3477 Mon Sep 17 00:00:00 2001 From: lark Date: Wed, 25 Mar 2026 05:05:34 +0900 Subject: [PATCH 5/8] Add WKProcessPool cookie isolation test and session store unit tests - WKProcessPoolCookieIsolationTests: verify cookies set in one WKWebsiteDataStore are NOT visible in another sharing the same WKProcessPool (foundation of agent session isolation) - BrowserAgentSessionStoreTests: CRUD, dedup, capacity limit, disconnect cleanup - BrowserPanelAgentOwnershipTests: switchToProfile blocked on agent-owned panels Co-Authored-By: Claude Opus 4.6 (1M context) --- GhosttyTabs.xcodeproj/project.pbxproj | 4 + cmuxTests/BrowserAgentSessionTests.swift | 191 +++++++++++++++++++++++ 2 files changed, 195 insertions(+) create mode 100644 cmuxTests/BrowserAgentSessionTests.swift diff --git a/GhosttyTabs.xcodeproj/project.pbxproj b/GhosttyTabs.xcodeproj/project.pbxproj index 6dd7acd51..14a578e15 100644 --- a/GhosttyTabs.xcodeproj/project.pbxproj +++ b/GhosttyTabs.xcodeproj/project.pbxproj @@ -112,6 +112,7 @@ A5001623 /* cmux.sdef in Resources */ = {isa = PBXBuildFile; fileRef = A5001622 /* cmux.sdef */; }; E12E88F82733EC42F32C36A3 /* BrowserConfigTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 970226F3C99D0D937CD00539 /* BrowserConfigTests.swift */; }; 1F14445B9627DE9D3AF4FD2E /* BrowserPanelTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 58C7B1B978620BE162CC057E /* BrowserPanelTests.swift */; }; + B3A91006B3A91006B3A91006 /* BrowserAgentSessionTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = B3A91005B3A91005B3A91005 /* BrowserAgentSessionTests.swift */; }; 46F6AC15863EC84DCD3770A2 /* TerminalAndGhosttyTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 02FC74F2C27127CC565B3E8C /* TerminalAndGhosttyTests.swift */; }; 6B524A0BA34FD46A771335AB /* WorkspaceUnitTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 71F8ED91A4B55D34BE6A0668 /* WorkspaceUnitTests.swift */; }; 063BC42CEE257D6213A2E30C /* WindowAndDragTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = BEE83F8394D90ACACD8E19DD /* WindowAndDragTests.swift */; }; @@ -283,6 +284,7 @@ A5001622 /* cmux.sdef */ = {isa = PBXFileReference; lastKnownFileType = text.sdef; path = cmux.sdef; sourceTree = ""; }; 970226F3C99D0D937CD00539 /* BrowserConfigTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BrowserConfigTests.swift; sourceTree = ""; }; 58C7B1B978620BE162CC057E /* BrowserPanelTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BrowserPanelTests.swift; sourceTree = ""; }; + B3A91005B3A91005B3A91005 /* BrowserAgentSessionTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BrowserAgentSessionTests.swift; sourceTree = ""; }; 02FC74F2C27127CC565B3E8C /* TerminalAndGhosttyTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TerminalAndGhosttyTests.swift; sourceTree = ""; }; 71F8ED91A4B55D34BE6A0668 /* WorkspaceUnitTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WorkspaceUnitTests.swift; sourceTree = ""; }; BEE83F8394D90ACACD8E19DD /* WindowAndDragTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WindowAndDragTests.swift; sourceTree = ""; }; @@ -540,6 +542,7 @@ A5008382 /* CommandPaletteSearchEngineTests.swift */, 970226F3C99D0D937CD00539 /* BrowserConfigTests.swift */, 58C7B1B978620BE162CC057E /* BrowserPanelTests.swift */, + B3A91005B3A91005B3A91005 /* BrowserAgentSessionTests.swift */, 02FC74F2C27127CC565B3E8C /* TerminalAndGhosttyTests.swift */, 71F8ED91A4B55D34BE6A0668 /* WorkspaceUnitTests.swift */, BEE83F8394D90ACACD8E19DD /* WindowAndDragTests.swift */, @@ -805,6 +808,7 @@ A5008383 /* CommandPaletteSearchEngineTests.swift in Sources */, E12E88F82733EC42F32C36A3 /* BrowserConfigTests.swift in Sources */, 1F14445B9627DE9D3AF4FD2E /* BrowserPanelTests.swift in Sources */, + B3A91006B3A91006B3A91006 /* BrowserAgentSessionTests.swift in Sources */, 46F6AC15863EC84DCD3770A2 /* TerminalAndGhosttyTests.swift in Sources */, 6B524A0BA34FD46A771335AB /* WorkspaceUnitTests.swift in Sources */, 063BC42CEE257D6213A2E30C /* WindowAndDragTests.swift in Sources */, diff --git a/cmuxTests/BrowserAgentSessionTests.swift b/cmuxTests/BrowserAgentSessionTests.swift new file mode 100644 index 000000000..a532cc044 --- /dev/null +++ b/cmuxTests/BrowserAgentSessionTests.swift @@ -0,0 +1,191 @@ +import XCTest +import WebKit + +#if canImport(cmux_DEV) +@testable import cmux_DEV +#elseif canImport(cmux) +@testable import cmux +#endif + +// MARK: - WKProcessPool Cookie Isolation + +/// Verifies that two WKWebsiteDataStore instances sharing the same +/// WKProcessPool do NOT leak cookies between each other. +/// This is the foundation of agent session isolation — if this fails, +/// per-session WKProcessPool instances would be required. +final class WKProcessPoolCookieIsolationTests: XCTestCase { + + @MainActor + func testCookieSetInOneStoreIsNotVisibleInAnother() async throws { + let sharedPool = WKProcessPool() + let idA = UUID() + let idB = UUID() + let storeA = WKWebsiteDataStore(forIdentifier: idA) + let storeB = WKWebsiteDataStore(forIdentifier: idB) + + // Set a test cookie in store A. + let cookie = try XCTUnwrap(HTTPCookie(properties: [ + .name: "agent_test_token", + .value: "secret_value_123", + .domain: ".example.com", + .path: "/", + ])) + await storeA.httpCookieStore.setCookie(cookie) + + // Verify cookie is in store A. + let cookiesA = await storeA.httpCookieStore.allCookies() + XCTAssertTrue( + cookiesA.contains(where: { $0.name == "agent_test_token" }), + "Cookie should exist in store A" + ) + + // Verify cookie is NOT in store B (isolation check). + let cookiesB = await storeB.httpCookieStore.allCookies() + XCTAssertFalse( + cookiesB.contains(where: { $0.name == "agent_test_token" }), + "Cookie from store A must NOT leak to store B via shared WKProcessPool" + ) + + // Cleanup: remove both data stores from disk. + await withCheckedContinuation { (c: CheckedContinuation) in + WKWebsiteDataStore.remove(forIdentifier: idA) { _ in c.resume() } + } + await withCheckedContinuation { (c: CheckedContinuation) in + WKWebsiteDataStore.remove(forIdentifier: idB) { _ in c.resume() } + } + + // Silence unused variable warning — the shared pool is intentionally + // created to prove that mere existence of a shared pool doesn't leak. + _ = sharedPool + } +} + +// MARK: - BrowserAgentSessionStore Unit Tests + +final class BrowserAgentSessionStoreTests: XCTestCase { + + @MainActor + func testGetOrCreateReturnsNewSession() async { + let store = BrowserAgentSessionStore() + let agentUUID = UUID() + let profileId = BrowserProfileStore.shared.builtInDefaultProfileID + + let session = await store.getOrCreate(agentSurfaceUUID: agentUUID, profileId: profileId) + XCTAssertNotNil(session) + XCTAssertEqual(session?.agentSurfaceUUID, agentUUID) + XCTAssertEqual(session?.sourceProfileId, profileId) + XCTAssertEqual(store.sessions.count, 1) + + // Cleanup + if let session { await store.dispose(sessionId: session.id) } + } + + @MainActor + func testGetOrCreateReturnsSameSessionOnSecondCall() async { + let store = BrowserAgentSessionStore() + let agentUUID = UUID() + let profileId = BrowserProfileStore.shared.builtInDefaultProfileID + + let session1 = await store.getOrCreate(agentSurfaceUUID: agentUUID, profileId: profileId) + let session2 = await store.getOrCreate(agentSurfaceUUID: agentUUID, profileId: profileId) + XCTAssertEqual(session1?.id, session2?.id, "Same agent+profile should return same session") + XCTAssertEqual(store.sessions.count, 1, "Should not create duplicate sessions") + + if let session1 { await store.dispose(sessionId: session1.id) } + } + + @MainActor + func testDifferentAgentsSamProfileGetDifferentSessions() async { + let store = BrowserAgentSessionStore() + let agentA = UUID() + let agentB = UUID() + let profileId = BrowserProfileStore.shared.builtInDefaultProfileID + + let sessionA = await store.getOrCreate(agentSurfaceUUID: agentA, profileId: profileId) + let sessionB = await store.getOrCreate(agentSurfaceUUID: agentB, profileId: profileId) + XCTAssertNotEqual(sessionA?.id, sessionB?.id, "Different agents should get different sessions") + XCTAssertNotEqual(sessionA?.dataStoreId, sessionB?.dataStoreId, "Different agents should get different data stores") + XCTAssertEqual(store.sessions.count, 2) + + if let sessionA { await store.dispose(sessionId: sessionA.id) } + if let sessionB { await store.dispose(sessionId: sessionB.id) } + } + + @MainActor + func testDisposeRemovesSession() async { + let store = BrowserAgentSessionStore() + let session = await store.getOrCreate( + agentSurfaceUUID: UUID(), + profileId: BrowserProfileStore.shared.builtInDefaultProfileID + ) + XCTAssertEqual(store.sessions.count, 1) + + if let session { + await store.dispose(sessionId: session.id) + } + XCTAssertEqual(store.sessions.count, 0) + } + + @MainActor + func testMaxConcurrentSessionsReturnsNil() async { + let store = BrowserAgentSessionStore() + let profileId = BrowserProfileStore.shared.builtInDefaultProfileID + + // Fill to capacity. + var created: [BrowserAgentSession] = [] + for _ in 0.. Date: Wed, 25 Mar 2026 10:14:59 +0900 Subject: [PATCH 6/8] Fix review board findings: CLI disconnect, reattach, dispose, GC, tests - rb-001: Only track agent surfaces for disconnect cleanup after system.identify (long-lived connections), not one-shot CLI commands - rb-002: Preserve agent cookie isolation in reattachToWorkspace by keeping existing websiteDataStore when agentSessionId is set - rb-003: Dispose iterates ALL windows to close agent panels, not just the resolved TabManager's window - rb-004: handleAgentDisconnect closes orphaned BrowserPanel tabs across all windows before disposing sessions - rb-006: Add injectable manifestURL to BrowserAgentSessionStore; tests use temp directory instead of production path Co-Authored-By: Claude Opus 4.6 (1M context) --- Sources/Panels/BrowserAgentSessionStore.swift | 26 +++++++++++ Sources/Panels/BrowserPanel.swift | 12 +++-- Sources/TerminalController.swift | 45 +++++++++++++------ cmuxTests/BrowserAgentSessionTests.swift | 22 ++++++--- 4 files changed, 82 insertions(+), 23 deletions(-) diff --git a/Sources/Panels/BrowserAgentSessionStore.swift b/Sources/Panels/BrowserAgentSessionStore.swift index 9b30cfe24..d0717fecd 100644 --- a/Sources/Panels/BrowserAgentSessionStore.swift +++ b/Sources/Panels/BrowserAgentSessionStore.swift @@ -70,6 +70,12 @@ final class BrowserAgentSessionStore: ObservableObject { ) } + /// Internal initializer for testing — accepts a custom manifest URL + /// so tests don't write to the production manifest path. + init(manifestURL: URL) { + self.manifestURL = manifestURL + } + // MARK: - Get or Create /// Return an existing session for the agent + profile pair, or create a @@ -195,6 +201,26 @@ final class BrowserAgentSessionStore: ObservableObject { /// Disposes all sessions owned by the disconnected agent surface. func handleAgentDisconnect(agentSurfaceUUID: UUID) async { let orphaned = sessionsForAgent(agentSurfaceUUID) + + // Close all BrowserPanels owned by these sessions across all windows + // before disposing the sessions, so panels don't outlive their data stores. + if let app = AppDelegate.shared { + let sessionIds = Set(orphaned.map(\.id)) + let windows = app.listMainWindowSummaries() + for item in windows { + guard let tm = app.tabManagerFor(windowId: item.windowId) else { continue } + for ws in tm.tabs { + let ownedPanelIds = ws.panels.values + .compactMap { $0 as? BrowserPanel } + .filter { $0.agentSessionId != nil && sessionIds.contains($0.agentSessionId!) } + .map(\.id) + for panelId in ownedPanelIds { + ws.closePanel(panelId, force: true) + } + } + } + } + for session in orphaned { await dispose(sessionId: session.id) } diff --git a/Sources/Panels/BrowserPanel.swift b/Sources/Panels/BrowserPanel.swift index 41f43c3c3..4d53af0b7 100644 --- a/Sources/Panels/BrowserPanel.swift +++ b/Sources/Panels/BrowserPanel.swift @@ -2774,9 +2774,15 @@ final class BrowserPanel: Panel, ObservableObject { ) { workspaceId = newWorkspaceId usesRemoteWorkspaceProxy = isRemoteWorkspace - let targetStore = isRemoteWorkspace - ? WKWebsiteDataStore(forIdentifier: remoteWebsiteDataStoreIdentifier ?? newWorkspaceId) - : BrowserProfileStore.shared.websiteDataStore(for: profileID) + let targetStore: WKWebsiteDataStore + if agentSessionId != nil { + // Agent-owned panel: preserve the agent-specific cloned data store. + targetStore = websiteDataStore + } else if isRemoteWorkspace { + targetStore = WKWebsiteDataStore(forIdentifier: remoteWebsiteDataStoreIdentifier ?? newWorkspaceId) + } else { + targetStore = BrowserProfileStore.shared.websiteDataStore(for: profileID) + } let needsStoreSwap = webView.configuration.websiteDataStore !== targetStore websiteDataStore = targetStore remoteProxyEndpoint = proxyEndpoint diff --git a/Sources/TerminalController.swift b/Sources/TerminalController.swift index 48548cf36..8eac8c09a 100644 --- a/Sources/TerminalController.swift +++ b/Sources/TerminalController.swift @@ -1605,8 +1605,10 @@ class TerminalController { var buffer = [UInt8](repeating: 0, count: 4096) var pending = "" var authenticated = false - // Track agent surface UUIDs that opened sessions on this connection - // so we can clean up on disconnect. + // Only track agent surfaces for disconnect cleanup on long-lived + // agent connections (identified via system.identify). One-shot CLI + // commands (connect → send → close) must NOT trigger cleanup. + var isIdentifiedAgent = false var agentSurfaceUUIDs = Set() while withListenerState({ isRunning }) { @@ -1630,10 +1632,20 @@ class TerminalController { let response = processCommand(trimmed) writeSocketResponse(response, to: socket) + // Mark connection as a long-lived agent when system.identify is sent. + if !isIdentifiedAgent, + trimmed.hasPrefix("{"), + let data = trimmed.data(using: .utf8), + let json = try? JSONSerialization.jsonObject(with: data) as? [String: Any], + (json["method"] as? String) == "system.identify" { + isIdentifiedAgent = true + } + // Track agent surface UUIDs from successful agent session opens for - // disconnect cleanup. Only register after processCommand succeeds - // (response contains "result", not "error") and method is verified. - if trimmed.hasPrefix("{"), + // disconnect cleanup. Only register on identified (long-lived) agent + // connections — one-shot CLI commands must not trigger cleanup. + if isIdentifiedAgent, + trimmed.hasPrefix("{"), response.contains("\"agent_session_id\""), let data = trimmed.data(using: .utf8), let json = try? JSONSerialization.jsonObject(with: data) as? [String: Any], @@ -10047,16 +10059,21 @@ class TerminalController { return .err(code: "unauthorized", message: "Session owned by different agent", data: nil) } - // Close all browser panels owned by this session (skip undo stack). + // Close all browser panels owned by this session across ALL windows + // (skip undo stack). A session may have panels in multiple windows. v2MainSync { - guard let tabManager = v2ResolveTabManager(params: params) else { return } - for ws in tabManager.tabs { - let ownedPanelIds = ws.panels.values - .compactMap { $0 as? BrowserPanel } - .filter { $0.agentSessionId == sessionId } - .map(\.id) - for panelId in ownedPanelIds { - ws.closePanel(panelId, force: true) + guard let app = AppDelegate.shared else { return } + let windows = app.listMainWindowSummaries() + for item in windows { + guard let tm = app.tabManagerFor(windowId: item.windowId) else { continue } + for ws in tm.tabs { + let ownedPanelIds = ws.panels.values + .compactMap { $0 as? BrowserPanel } + .filter { $0.agentSessionId == sessionId } + .map(\.id) + for panelId in ownedPanelIds { + ws.closePanel(panelId, force: true) + } } } } diff --git a/cmuxTests/BrowserAgentSessionTests.swift b/cmuxTests/BrowserAgentSessionTests.swift index a532cc044..3fd21f531 100644 --- a/cmuxTests/BrowserAgentSessionTests.swift +++ b/cmuxTests/BrowserAgentSessionTests.swift @@ -64,9 +64,19 @@ final class WKProcessPoolCookieIsolationTests: XCTestCase { final class BrowserAgentSessionStoreTests: XCTestCase { + /// Create a store with a temp manifest path to avoid writing to production. + @MainActor + private func makeTempStore() -> BrowserAgentSessionStore { + let tempDir = FileManager.default.temporaryDirectory + .appendingPathComponent("cmux-test-\(UUID().uuidString)", isDirectory: true) + try? FileManager.default.createDirectory(at: tempDir, withIntermediateDirectories: true) + let manifestURL = tempDir.appendingPathComponent("agent_sessions_manifest.json") + return BrowserAgentSessionStore(manifestURL: manifestURL) + } + @MainActor func testGetOrCreateReturnsNewSession() async { - let store = BrowserAgentSessionStore() + let store = makeTempStore() let agentUUID = UUID() let profileId = BrowserProfileStore.shared.builtInDefaultProfileID @@ -82,7 +92,7 @@ final class BrowserAgentSessionStoreTests: XCTestCase { @MainActor func testGetOrCreateReturnsSameSessionOnSecondCall() async { - let store = BrowserAgentSessionStore() + let store = makeTempStore() let agentUUID = UUID() let profileId = BrowserProfileStore.shared.builtInDefaultProfileID @@ -96,7 +106,7 @@ final class BrowserAgentSessionStoreTests: XCTestCase { @MainActor func testDifferentAgentsSamProfileGetDifferentSessions() async { - let store = BrowserAgentSessionStore() + let store = makeTempStore() let agentA = UUID() let agentB = UUID() let profileId = BrowserProfileStore.shared.builtInDefaultProfileID @@ -113,7 +123,7 @@ final class BrowserAgentSessionStoreTests: XCTestCase { @MainActor func testDisposeRemovesSession() async { - let store = BrowserAgentSessionStore() + let store = makeTempStore() let session = await store.getOrCreate( agentSurfaceUUID: UUID(), profileId: BrowserProfileStore.shared.builtInDefaultProfileID @@ -128,7 +138,7 @@ final class BrowserAgentSessionStoreTests: XCTestCase { @MainActor func testMaxConcurrentSessionsReturnsNil() async { - let store = BrowserAgentSessionStore() + let store = makeTempStore() let profileId = BrowserProfileStore.shared.builtInDefaultProfileID // Fill to capacity. @@ -150,7 +160,7 @@ final class BrowserAgentSessionStoreTests: XCTestCase { @MainActor func testHandleAgentDisconnectDisposesAllAgentSessions() async { - let store = BrowserAgentSessionStore() + let store = makeTempStore() let agentUUID = UUID() let profileA = BrowserProfileStore.shared.builtInDefaultProfileID From af5cf42f301f77e308d496ab53dd2211e9b559d9 Mon Sep 17 00:00:00 2001 From: lark Date: Wed, 25 Mar 2026 10:25:00 +0900 Subject: [PATCH 7/8] Fix re-review suggestions: rollback, limit race, clone cancel, formatter - rr-001: Dispose session on panel creation failure paths to prevent orphaned sessions consuming slots - rr-002: Count cloneInFlight toward maxConcurrentSessions to prevent concurrent opens from exceeding the limit - rr-003: Cancel matching cloneInFlight tasks on dispose; check Task.isCancelled before appending to prevent zombie sessions - rr-004: Hoist ISO8601DateFormatter outside .map closure Co-Authored-By: Claude Opus 4.6 (1M context) --- Sources/Panels/BrowserAgentSessionStore.swift | 17 +++++++++++++++-- Sources/TerminalController.swift | 12 +++++++++++- 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/Sources/Panels/BrowserAgentSessionStore.swift b/Sources/Panels/BrowserAgentSessionStore.swift index d0717fecd..c2f351f47 100644 --- a/Sources/Panels/BrowserAgentSessionStore.swift +++ b/Sources/Panels/BrowserAgentSessionStore.swift @@ -91,8 +91,8 @@ final class BrowserAgentSessionStore: ObservableObject { return existing } - // Enforce session limit. - guard sessions.count < Self.maxConcurrentSessions else { + // Enforce session limit (include in-flight clones toward the cap). + guard sessions.count + cloneInFlight.count < Self.maxConcurrentSessions else { // If at capacity, still check for a matching in-flight clone. let key = "\(agentSurfaceUUID)-\(profileId)" if let inFlight = cloneInFlight[key] { @@ -126,6 +126,12 @@ final class BrowserAgentSessionStore: ObservableObject { let agentStore = WKWebsiteDataStore(forIdentifier: session.dataStoreId) await sourceStore.cloneCookies(to: agentStore) + // If the session was disposed while cloning, skip appending. + guard !Task.isCancelled else { + self.cloneInFlight.removeValue(forKey: key) + return session + } + self.sessions.append(session) self.cloneInFlight.removeValue(forKey: key) self.persistManifest() @@ -148,6 +154,13 @@ final class BrowserAgentSessionStore: ObservableObject { guard let index = sessions.firstIndex(where: { $0.id == sessionId }) else { return } let session = sessions.remove(at: index) + // Cancel any in-flight clone tasks for this agent. + let agentUUID = session.agentSurfaceUUID + for (key, task) in cloneInFlight where key.hasPrefix(agentUUID.uuidString) { + task.cancel() + cloneInFlight.removeValue(forKey: key) + } + // Remove the cloned data store from disk. await withCheckedContinuation { (continuation: CheckedContinuation) in WKWebsiteDataStore.remove(forIdentifier: session.dataStoreId) { _ in diff --git a/Sources/TerminalController.swift b/Sources/TerminalController.swift index 8eac8c09a..912932722 100644 --- a/Sources/TerminalController.swift +++ b/Sources/TerminalController.swift @@ -9925,6 +9925,9 @@ class TerminalController { v2MainSync { guard let ws = v2ResolveWorkspace(params: params, tabManager: tabManager) else { result = .err(code: "not_found", message: "Workspace not found", data: nil) + Task { @MainActor in + await BrowserAgentSessionStore.shared.dispose(sessionId: session.id) + } return } let paneUUID = v2UUID(params, "pane_id") @@ -9932,6 +9935,9 @@ class TerminalController { guard let paneUUID, let pane = ws.bonsplitController.allPaneIds.first(where: { $0.id == paneUUID }) else { result = .err(code: "not_found", message: "Pane not found", data: nil) + Task { @MainActor in + await BrowserAgentSessionStore.shared.dispose(sessionId: session.id) + } return } @@ -9944,6 +9950,9 @@ class TerminalController { agentDataStoreId: session.dataStoreId ) else { result = .err(code: "internal_error", message: "Failed to create tab", data: nil) + Task { @MainActor in + await BrowserAgentSessionStore.shared.dispose(sessionId: session.id) + } return } @@ -10120,6 +10129,7 @@ class TerminalController { } let store = BrowserAgentSessionStore.shared + let dateFormatter = ISO8601DateFormatter() let entries: [[String: Any]] = sessions.map { session in [ "session_id": session.id.uuidString, @@ -10127,7 +10137,7 @@ class TerminalController { "source_profile_id": session.sourceProfileId.uuidString, "profile_name": BrowserProfileStore.shared.displayName(for: session.sourceProfileId), "tab_count": store.tabCount(for: session.id), - "created_at": ISO8601DateFormatter().string(from: session.createdAt), + "created_at": dateFormatter.string(from: session.createdAt), ] } result = .ok(["sessions": entries]) From fde868cc54e6c926d20fdbff42e74747563a07b7 Mon Sep 17 00:00:00 2001 From: lark Date: Wed, 25 Mar 2026 10:44:27 +0900 Subject: [PATCH 8/8] Fix CLI: add agent-session to verbsWithoutSurface The browser subcommand parser was treating "agent-session" as a surface handle instead of a verb, causing "Unsupported browser subcommand" errors. Co-Authored-By: Claude Opus 4.6 (1M context) --- CLI/cmux.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CLI/cmux.swift b/CLI/cmux.swift index 157942220..f60e177b5 100644 --- a/CLI/cmux.swift +++ b/CLI/cmux.swift @@ -4584,7 +4584,7 @@ struct CMUXCLI { var surfaceRaw = surfaceOpt var args = argsWithoutSurfaceFlag - let verbsWithoutSurface: Set = ["open", "open-split", "new", "identify"] + let verbsWithoutSurface: Set = ["open", "open-split", "new", "identify", "agent-session"] if surfaceRaw == nil, let first = args.first { if !first.hasPrefix("-") && !verbsWithoutSurface.contains(first.lowercased()) { surfaceRaw = first