From ca4f4b7c69a929a4498d1a2a385708882f3f3c19 Mon Sep 17 00:00:00 2001 From: Lawrence Chen <54008264+lawrencecchen@users.noreply.github.com> Date: Fri, 13 Mar 2026 21:04:48 -0700 Subject: [PATCH] Fix browser move and zsh bootstrap regressions --- CLI/cmux.swift | 13 ++-- Sources/Panels/BrowserPanel.swift | 65 ++++++++++++++++---- Sources/Workspace.swift | 10 +++- cmuxTests/CLIProcessRunnerTests.swift | 86 +++++++++++++++++++++++++++ cmuxTests/GhosttyConfigTests.swift | 74 +++++++++++++++++++++++ 5 files changed, 227 insertions(+), 21 deletions(-) diff --git a/CLI/cmux.swift b/CLI/cmux.swift index aac6ef2e..6df33c0c 100644 --- a/CLI/cmux.swift +++ b/CLI/cmux.swift @@ -3383,9 +3383,10 @@ struct CMUXCLI { return merged } - private func buildInteractiveRemoteShellCommand(remoteRelayPort: Int, shellFeatures: String) -> String { + func buildInteractiveRemoteShellCommand(remoteRelayPort: Int, shellFeatures: String) -> String { let remoteEnvExportLines = interactiveRemoteShellExportLines(shellFeatures: shellFeatures) let relaySocket = remoteRelayPort > 0 ? "127.0.0.1:\(remoteRelayPort)" : nil + let shellStateDir = "$HOME/.cmux/relay/\(max(remoteRelayPort, 0)).shell" let commonShellLines = remoteEnvExportLines + ["export PATH=\"$HOME/.cmux/bin:$PATH\""] + (relaySocket.map { ["export CMUX_SOCKET_PATH=\($0)"] } ?? []) @@ -3394,18 +3395,17 @@ struct CMUXCLI { "rehash >/dev/null 2>&1 || true", ] let zshEnvLines = [ - "export CMUX_REAL_ZDOTDIR=\"${CMUX_REAL_ZDOTDIR:-$HOME}\"", - "[ -f \"$HOME/.zshenv\" ] && source \"$HOME/.zshenv\"", + "[ -f \"$CMUX_REAL_ZDOTDIR/.zshenv\" ] && source \"$CMUX_REAL_ZDOTDIR/.zshenv\"", + "if [ -n \"${ZDOTDIR:-}\" ] && [ \"$ZDOTDIR\" != \"\(shellStateDir)\" ]; then export CMUX_REAL_ZDOTDIR=\"$ZDOTDIR\"; fi", + "export ZDOTDIR=\"\(shellStateDir)\"", ] let zshRCLines = [ - "export ZDOTDIR=\"${CMUX_REAL_ZDOTDIR:-$HOME}\"", - "[ -f \"$HOME/.zshrc\" ] && source \"$HOME/.zshrc\"", + "[ -f \"$CMUX_REAL_ZDOTDIR/.zshrc\" ] && source \"$CMUX_REAL_ZDOTDIR/.zshrc\"", ] + commonShellLines let bashRCLines = [ "[ -f \"$HOME/.bashrc\" ] && . \"$HOME/.bashrc\"", ] + commonShellLines let relayWarmupLines = interactiveRemoteRelayWarmupLines(remoteRelayPort: remoteRelayPort) - let shellStateDir = "$HOME/.cmux/relay/\(max(remoteRelayPort, 0)).shell" var outerLines: [String] = [ "CMUX_LOGIN_SHELL=\"${SHELL:-/bin/zsh}\"", @@ -3428,6 +3428,7 @@ struct CMUXCLI { ] outerLines.append(contentsOf: relayWarmupLines.map { " " + $0 }) outerLines += [ + " export CMUX_REAL_ZDOTDIR=\"${ZDOTDIR:-$HOME}\"", " export ZDOTDIR=\"$cmux_shell_dir\"", " exec \"$CMUX_LOGIN_SHELL\" -i", " ;;", diff --git a/Sources/Panels/BrowserPanel.swift b/Sources/Panels/BrowserPanel.swift index cf6ecbc6..b236a2e9 100644 --- a/Sources/Panels/BrowserPanel.swift +++ b/Sources/Panels/BrowserPanel.swift @@ -1421,7 +1421,7 @@ final class BrowserPanel: Panel, ObservableObject { /// The underlying web view private(set) var webView: WKWebView - private let websiteDataStore: WKWebsiteDataStore + private var websiteDataStore: WKWebsiteDataStore /// Monotonic identity for the current WKWebView instance. /// Incremented whenever we replace the underlying WKWebView after a process crash. @@ -1798,7 +1798,7 @@ final class BrowserPanel: Panel, ObservableObject { private let developerToolsRestoreRetryMaxAttempts: Int = 40 private var remoteProxyEndpoint: BrowserProxyEndpoint? @Published private(set) var remoteWorkspaceStatus: BrowserRemoteWorkspaceStatus? - private let usesRemoteWorkspaceProxy: Bool + private var usesRemoteWorkspaceProxy: Bool private struct PendingRemoteNavigation { let request: URLRequest let recordTypedNavigation: Bool @@ -2211,6 +2211,33 @@ final class BrowserPanel: Panel, ObservableObject { workspaceId = newWorkspaceId } + func reattachToWorkspace( + _ newWorkspaceId: UUID, + isRemoteWorkspace: Bool, + remoteWebsiteDataStoreIdentifier: UUID? = nil, + proxyEndpoint: BrowserProxyEndpoint?, + remoteStatus: BrowserRemoteWorkspaceStatus? + ) { + workspaceId = newWorkspaceId + usesRemoteWorkspaceProxy = isRemoteWorkspace + let targetStore = isRemoteWorkspace + ? WKWebsiteDataStore(forIdentifier: remoteWebsiteDataStoreIdentifier ?? newWorkspaceId) + : .default() + let needsStoreSwap = webView.configuration.websiteDataStore !== targetStore + websiteDataStore = targetStore + remoteProxyEndpoint = proxyEndpoint + remoteWorkspaceStatus = remoteStatus + if needsStoreSwap { + replaceWebViewPreservingState( + from: webView, + websiteDataStore: targetStore, + reason: "workspace_reattach" + ) + } + applyRemoteProxyConfigurationIfAvailable() + resumePendingRemoteNavigationIfNeeded() + } + func triggerFlash() { guard NotificationPaneFlashSettings.isEnabled() else { return } focusFlashToken &+= 1 @@ -2326,20 +2353,33 @@ final class BrowserPanel: Panel, ObservableObject { } private func replaceWebViewAfterContentProcessTermination(for terminatedWebView: WKWebView) { - guard terminatedWebView === webView else { return } + replaceWebViewPreservingState( + from: terminatedWebView, + websiteDataStore: websiteDataStore, + reason: "webcontent_process_terminated" + ) + } + + private func replaceWebViewPreservingState( + from oldWebView: WKWebView, + websiteDataStore: WKWebsiteDataStore, + reason: String + ) { + guard oldWebView === webView else { return } let wasRenderable = shouldRenderWebView - let restoreURL = Self.remoteProxyDisplayURL(for: terminatedWebView.url) ?? currentURL + let restoreURL = Self.remoteProxyDisplayURL(for: oldWebView.url) ?? currentURL let restoreURLString = restoreURL?.absoluteString let shouldRestoreURL = wasRenderable && restoreURLString != nil && restoreURLString != blankURLString let history = sessionNavigationHistorySnapshot() let historyCurrentURL = preferredURLStringForOmnibar() - let desiredZoom = max(minPageZoom, min(maxPageZoom, terminatedWebView.pageZoom)) + let desiredZoom = max(minPageZoom, min(maxPageZoom, oldWebView.pageZoom)) let restoreDevTools = preferredDeveloperToolsVisible #if DEBUG dlog( "browser.webview.replace.begin panel=\(id.uuidString.prefix(5)) " + + "reason=\(reason) " + "renderable=\(wasRenderable ? 1 : 0) restoreURL=\(restoreURLString ?? "nil") " + "restoreHistoryBack=\(history.backHistoryURLStrings.count) " + "restoreHistoryForward=\(history.forwardHistoryURLStrings.count)" @@ -2351,12 +2391,12 @@ final class BrowserPanel: Panel, ObservableObject { faviconTask?.cancel() faviconTask = nil faviconRefreshGeneration &+= 1 - BrowserWindowPortalRegistry.detach(webView: terminatedWebView) - terminatedWebView.stopLoading() - terminatedWebView.navigationDelegate = nil - terminatedWebView.uiDelegate = nil - if let terminatedCmuxWebView = terminatedWebView as? CmuxWebView { - terminatedCmuxWebView.onContextMenuDownloadStateChanged = nil + BrowserWindowPortalRegistry.detach(webView: oldWebView) + oldWebView.stopLoading() + oldWebView.navigationDelegate = nil + oldWebView.uiDelegate = nil + if let oldCmuxWebView = oldWebView as? CmuxWebView { + oldCmuxWebView.onContextMenuDownloadStateChanged = nil } let replacement = Self.makeWebView(websiteDataStore: websiteDataStore) @@ -2387,12 +2427,13 @@ final class BrowserPanel: Panel, ObservableObject { } if restoreDevTools { - requestDeveloperToolsRefreshAfterNextAttach(reason: "webcontent_process_terminated") + requestDeveloperToolsRefreshAfterNextAttach(reason: reason) } #if DEBUG dlog( "browser.webview.replace.end panel=\(id.uuidString.prefix(5)) " + + "reason=\(reason) " + "instance=\(webViewInstanceID.uuidString.prefix(6)) " + "restoreURL=\(restoreURLString ?? "nil") shouldRestore=\(shouldRestoreURL ? 1 : 0)" ) diff --git a/Sources/Workspace.swift b/Sources/Workspace.swift index 53b9487d..fd0e5ad4 100644 --- a/Sources/Workspace.swift +++ b/Sources/Workspace.swift @@ -6995,9 +6995,13 @@ final class Workspace: Identifiable, ObservableObject { if let terminalPanel = detached.panel as? TerminalPanel { terminalPanel.updateWorkspaceId(id) } else if let browserPanel = detached.panel as? BrowserPanel { - browserPanel.updateWorkspaceId(id) - browserPanel.setRemoteProxyEndpoint(remoteProxyEndpoint) - browserPanel.setRemoteWorkspaceStatus(browserRemoteWorkspaceStatusSnapshot()) + browserPanel.reattachToWorkspace( + id, + isRemoteWorkspace: isRemoteWorkspace, + remoteWebsiteDataStoreIdentifier: isRemoteWorkspace ? id : nil, + proxyEndpoint: remoteProxyEndpoint, + remoteStatus: browserRemoteWorkspaceStatusSnapshot() + ) installBrowserPanelSubscription(browserPanel) } diff --git a/cmuxTests/CLIProcessRunnerTests.swift b/cmuxTests/CLIProcessRunnerTests.swift index b4bc4dc5..d3831dee 100644 --- a/cmuxTests/CLIProcessRunnerTests.swift +++ b/cmuxTests/CLIProcessRunnerTests.swift @@ -16,5 +16,91 @@ final class CLIProcessRunnerTests: XCTestCase { XCTAssertEqual(result.status, 124) XCTAssertLessThan(Date().timeIntervalSince(startedAt), 2.0) } + + func testInteractiveRemoteShellCommandHonorsZDOTDIRFromRealZshenv() throws { + let fileManager = FileManager.default + let home = fileManager.temporaryDirectory.appendingPathComponent("cmux-cli-zdotdir-\(UUID().uuidString)") + let userZdotdir = home.appendingPathComponent("user-zdotdir") + let relayDir = home.appendingPathComponent(".cmux/relay") + let binDir = home.appendingPathComponent(".cmux/bin") + try fileManager.createDirectory(at: userZdotdir, withIntermediateDirectories: true) + try fileManager.createDirectory(at: relayDir, withIntermediateDirectories: true) + try fileManager.createDirectory(at: binDir, withIntermediateDirectories: true) + defer { try? fileManager.removeItem(at: home) } + + try "export ZDOTDIR=\"$HOME/user-zdotdir\"\n" + .write(to: home.appendingPathComponent(".zshenv"), atomically: true, encoding: .utf8) + try """ + precmd() { + print -r -- "REAL=$CMUX_REAL_ZDOTDIR ZDOTDIR=$ZDOTDIR SOCKET=$CMUX_SOCKET_PATH PATH=$PATH" + exit + } + """ + .write(to: userZdotdir.appendingPathComponent(".zshrc"), atomically: true, encoding: .utf8) + try "#!/bin/sh\nexit 0\n" + .write(to: binDir.appendingPathComponent("cmux"), atomically: true, encoding: .utf8) + try "".write( + to: relayDir.appendingPathComponent("64003.auth"), + atomically: true, + encoding: .utf8 + ) + try fileManager.setAttributes( + [.posixPermissions: 0o755], + ofItemAtPath: binDir.appendingPathComponent("cmux").path + ) + + let cli = CMUXCLI(args: []) + let command = cli.buildInteractiveRemoteShellCommand(remoteRelayPort: 64003, shellFeatures: "") + let result = CLIProcessRunner.runProcess( + executablePath: "/bin/sh", + arguments: ["-c", command], + timeout: 5 + ) + + XCTAssertFalse(result.timedOut, result.stderr) + XCTAssertEqual(result.status, 0, result.stderr) + XCTAssertTrue(result.stdout.contains("REAL=\(userZdotdir.path)"), result.stdout) + XCTAssertTrue(result.stdout.contains("SOCKET=127.0.0.1:64003"), result.stdout) + XCTAssertTrue(result.stdout.contains("PATH=\(binDir.path):"), result.stdout) + XCTAssertTrue(result.stdout.contains("ZDOTDIR=\(relayDir.appendingPathComponent("64003.shell").path)"), result.stdout) + } + + func testInteractiveRemoteShellCommandKeepsDefaultZDOTDIRWithoutRecursing() throws { + let fileManager = FileManager.default + let home = fileManager.temporaryDirectory.appendingPathComponent("cmux-cli-zdotdir-default-\(UUID().uuidString)") + let relayDir = home.appendingPathComponent(".cmux/relay") + let binDir = home.appendingPathComponent(".cmux/bin") + try fileManager.createDirectory(at: relayDir, withIntermediateDirectories: true) + try fileManager.createDirectory(at: binDir, withIntermediateDirectories: true) + defer { try? fileManager.removeItem(at: home) } + + try "precmd() { print -r -- \"REAL=$CMUX_REAL_ZDOTDIR ZDOTDIR=$ZDOTDIR\"; exit }\n" + .write(to: home.appendingPathComponent(".zshrc"), atomically: true, encoding: .utf8) + try "#!/bin/sh\nexit 0\n" + .write(to: binDir.appendingPathComponent("cmux"), atomically: true, encoding: .utf8) + try "".write( + to: relayDir.appendingPathComponent("64004.auth"), + atomically: true, + encoding: .utf8 + ) + try fileManager.setAttributes( + [.posixPermissions: 0o755], + ofItemAtPath: binDir.appendingPathComponent("cmux").path + ) + + let cli = CMUXCLI(args: []) + let command = cli.buildInteractiveRemoteShellCommand(remoteRelayPort: 64004, shellFeatures: "") + let result = CLIProcessRunner.runProcess( + executablePath: "/bin/sh", + arguments: ["-c", command], + timeout: 5 + ) + + XCTAssertFalse(result.timedOut, result.stderr) + XCTAssertEqual(result.status, 0, result.stderr) + XCTAssertFalse(result.stderr.contains("too many open files"), result.stderr) + XCTAssertTrue(result.stdout.contains("REAL=\(home.path)"), result.stdout) + XCTAssertTrue(result.stdout.contains("ZDOTDIR=\(relayDir.appendingPathComponent("64004.shell").path)"), result.stdout) + } } #endif diff --git a/cmuxTests/GhosttyConfigTests.swift b/cmuxTests/GhosttyConfigTests.swift index a77dcf78..72f064b9 100644 --- a/cmuxTests/GhosttyConfigTests.swift +++ b/cmuxTests/GhosttyConfigTests.swift @@ -931,6 +931,80 @@ final class BrowserPanelRemoteStoreTests: XCTestCase { XCTAssertEqual(panel.webView.url?.host, "localhost") } + func testBrowserMoveIntoRemoteWorkspaceRebuildsWebsiteDataStoreScope() throws { + let source = Workspace() + let sourcePaneId = try XCTUnwrap(source.bonsplitController.allPaneIds.first) + let sourceBrowser = try XCTUnwrap(source.newBrowserSurface(inPane: sourcePaneId, focus: false)) + let localStore = sourceBrowser.webView.configuration.websiteDataStore + XCTAssertTrue(localStore === WKWebsiteDataStore.default()) + + let destination = Workspace() + destination.configureRemoteConnection( + WorkspaceRemoteConfiguration( + destination: "cmux-macmini", + port: 22, + identityFile: nil, + sshOptions: [], + localProxyPort: nil, + relayPort: 64001, + relayID: "relay-store-dest", + relayToken: String(repeating: "a", count: 64), + localSocketPath: "/tmp/cmux-store-dest.sock", + terminalStartupCommand: "ssh cmux-macmini" + ), + autoConnect: false + ) + let destinationPaneId = try XCTUnwrap(destination.bonsplitController.allPaneIds.first) + let destinationBrowser = try XCTUnwrap(destination.newBrowserSurface(inPane: destinationPaneId, focus: false)) + let destinationStore = destinationBrowser.webView.configuration.websiteDataStore + XCTAssertFalse(destinationStore === WKWebsiteDataStore.default()) + + let detached = try XCTUnwrap(source.detachSurface(panelId: sourceBrowser.id)) + let attachedPanelId = try XCTUnwrap( + destination.attachDetachedSurface(detached, inPane: destinationPaneId, focus: false) + ) + let movedBrowser = try XCTUnwrap(destination.panels[attachedPanelId] as? BrowserPanel) + + XCTAssertTrue(movedBrowser.webView.configuration.websiteDataStore === destinationStore) + XCTAssertFalse(movedBrowser.webView.configuration.websiteDataStore === localStore) + } + + func testBrowserMoveOutOfRemoteWorkspaceRestoresDefaultWebsiteDataStore() throws { + let source = Workspace() + source.configureRemoteConnection( + WorkspaceRemoteConfiguration( + destination: "cmux-macmini", + port: 22, + identityFile: nil, + sshOptions: [], + localProxyPort: nil, + relayPort: 64002, + relayID: "relay-store-source", + relayToken: String(repeating: "b", count: 64), + localSocketPath: "/tmp/cmux-store-source.sock", + terminalStartupCommand: "ssh cmux-macmini" + ), + autoConnect: false + ) + let sourcePaneId = try XCTUnwrap(source.bonsplitController.allPaneIds.first) + let movedBrowser = try XCTUnwrap(source.newBrowserSurface(inPane: sourcePaneId, focus: false)) + let remainingRemoteBrowser = try XCTUnwrap(source.newBrowserSurface(inPane: sourcePaneId, focus: false)) + let remoteStore = remainingRemoteBrowser.webView.configuration.websiteDataStore + XCTAssertFalse(remoteStore === WKWebsiteDataStore.default()) + + let destination = Workspace() + let destinationPaneId = try XCTUnwrap(destination.bonsplitController.allPaneIds.first) + let detached = try XCTUnwrap(source.detachSurface(panelId: movedBrowser.id)) + let attachedPanelId = try XCTUnwrap( + destination.attachDetachedSurface(detached, inPane: destinationPaneId, focus: false) + ) + let attachedBrowser = try XCTUnwrap(destination.panels[attachedPanelId] as? BrowserPanel) + + XCTAssertTrue(attachedBrowser.webView.configuration.websiteDataStore === WKWebsiteDataStore.default()) + XCTAssertTrue(remainingRemoteBrowser.webView.configuration.websiteDataStore === remoteStore) + XCTAssertFalse(remainingRemoteBrowser.webView.configuration.websiteDataStore === attachedBrowser.webView.configuration.websiteDataStore) + } + func testNewTerminalSurfaceStaysRemoteWhileBrowserPanelsKeepWorkspaceRemote() throws { let workspace = Workspace() let paneId = try XCTUnwrap(workspace.bonsplitController.allPaneIds.first)