diff --git a/GhosttyTabs.xcodeproj/project.pbxproj b/GhosttyTabs.xcodeproj/project.pbxproj index d160ee3f..7922603c 100644 --- a/GhosttyTabs.xcodeproj/project.pbxproj +++ b/GhosttyTabs.xcodeproj/project.pbxproj @@ -15,6 +15,7 @@ THTM0002 /* TerminalThemeStore.swift in Sources */ = {isa = PBXBuildFile; fileRef = THTM0001 /* TerminalThemeStore.swift */; }; THTM0003 /* TerminalThemeStore.swift in Sources */ = {isa = PBXBuildFile; fileRef = THTM0001 /* TerminalThemeStore.swift */; }; REND0002A1B2C3D4E5F60719 /* RendererRealization.swift in Sources */ = {isa = PBXBuildFile; fileRef = REND0001A1B2C3D4E5F60719 /* RendererRealization.swift */; }; + WKTR0002A1B2C3D4E5F60719 /* WebKitSubviewTransfer.swift in Sources */ = {isa = PBXBuildFile; fileRef = WKTR0001A1B2C3D4E5F60719 /* WebKitSubviewTransfer.swift */; }; A5FF0007 /* SettingDefinition.swift in Sources */ = {isa = PBXBuildFile; fileRef = A5FF0017 /* SettingDefinition.swift */; }; A5001002 /* ContentView.swift in Sources */ = {isa = PBXBuildFile; fileRef = A5001012 /* ContentView.swift */; }; NRSP0084A1B2C3D4E5F60719 /* WorkspaceSidebarModels.swift in Sources */ = {isa = PBXBuildFile; fileRef = NRSP0083A1B2C3D4E5F60719 /* WorkspaceSidebarModels.swift */; }; @@ -439,6 +440,7 @@ NRPA00010 /* SettingsView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SettingsView.swift; sourceTree = ""; }; THTM0001 /* TerminalThemeStore.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TerminalThemeStore.swift; sourceTree = ""; }; REND0001A1B2C3D4E5F60719 /* RendererRealization.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RendererRealization.swift; sourceTree = ""; }; + WKTR0001A1B2C3D4E5F60719 /* WebKitSubviewTransfer.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WebKitSubviewTransfer.swift; sourceTree = ""; }; A5FF0017 /* SettingDefinition.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SettingDefinition.swift; sourceTree = ""; }; A5001012 /* ContentView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ContentView.swift; sourceTree = ""; }; NRSP0083A1B2C3D4E5F60719 /* WorkspaceSidebarModels.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WorkspaceSidebarModels.swift; sourceTree = ""; }; @@ -912,6 +914,7 @@ NRPA00010 /* SettingsView.swift */, THTM0001 /* TerminalThemeStore.swift */, REND0001A1B2C3D4E5F60719 /* RendererRealization.swift */, + WKTR0001A1B2C3D4E5F60719 /* WebKitSubviewTransfer.swift */, A5FF0017 /* SettingDefinition.swift */, A5001012 /* ContentView.swift */, B10A1CE5 /* RenderableSystemSymbol.swift */, @@ -1474,6 +1477,7 @@ NRPA00009 /* SettingsView.swift in Sources */, THTM0002 /* TerminalThemeStore.swift in Sources */, REND0002A1B2C3D4E5F60719 /* RendererRealization.swift in Sources */, + WKTR0002A1B2C3D4E5F60719 /* WebKitSubviewTransfer.swift in Sources */, A5FF0007 /* SettingDefinition.swift in Sources */, A5001002 /* ContentView.swift in Sources */, B10A1CE6 /* RenderableSystemSymbol.swift in Sources */, diff --git a/Resources/Info.plist b/Resources/Info.plist index 97ae3a79..f6f1c352 100644 --- a/Resources/Info.plist +++ b/Resources/Info.plist @@ -133,7 +133,7 @@ SUAutomaticallyUpdate - + SUEnableAutomaticChecks SUFeedURL diff --git a/Sources/AppDelegate.swift b/Sources/AppDelegate.swift index 469ec5fa..23a56d70 100644 --- a/Sources/AppDelegate.swift +++ b/Sources/AppDelegate.swift @@ -1659,7 +1659,7 @@ final class AppDelegate: NSObject, NSApplicationDelegate, @preconcurrency UNUser SessionPersistenceStore.rotateIntoHistory() guard SessionRestorePolicy.shouldAttemptRestore() else { return } Self.removeLegacyPersistedWindowGeometry() - startupSessionSnapshot = SessionPersistenceStore.load() + startupSessionSnapshot = SessionPersistenceStore.loadWithHistoryFallback() } private func persistedWindowGeometry( diff --git a/Sources/BrowserWindowPortal.swift b/Sources/BrowserWindowPortal.swift index 4d0b6a03..60827c52 100644 --- a/Sources/BrowserWindowPortal.swift +++ b/Sources/BrowserWindowPortal.swift @@ -514,50 +514,6 @@ final class WindowBrowserPortal: HostedViewPortalRegistry { }) ?? reference } - private func directTransferChild(of container: NSView, containing descendant: NSView) -> NSView? { - var current: NSView? = descendant - var directChild: NSView? - while let view = current, view !== container { - directChild = view - current = view.superview - } - guard current === container else { return nil } - return directChild - } - - private func relatedWebKitTransferSubviews( - from sourceSuperview: NSView, - primaryWebView: WKWebView - ) -> [NSView] { - var relatedSubviews: [NSView] = [] - var seen = Set() - - func append(_ candidate: NSView?) { - guard let candidate, candidate !== sourceSuperview else { return } - let id = ObjectIdentifier(candidate) - guard seen.insert(id).inserted else { return } - relatedSubviews.append(candidate) - } - - append(directTransferChild(of: sourceSuperview, containing: primaryWebView) ?? primaryWebView) - - if let inspectorFrontend = primaryWebView.programaInspectorFrontendWebView() { - append(directTransferChild(of: sourceSuperview, containing: inspectorFrontend) ?? inspectorFrontend) - } - - for view in sourceSuperview.subviews { - if view === primaryWebView { continue } - let className = String(describing: type(of: view)) - guard className.contains("WK") else { continue } - if InspectorDock.isInspectorView(view) && !InspectorDock.isVisibleCandidate(view) { - continue - } - append(view) - } - - return relatedSubviews - } - private func appendHostedWebKitSubviews( in root: NSView, to result: inout [WKWebView], @@ -826,46 +782,6 @@ final class WindowBrowserPortal: HostedViewPortalRegistry { } } - private func moveWebKitRelatedSubviewsIfNeeded( - from sourceSuperview: NSView, - to containerView: WindowBrowserSlotView, - primaryWebView: WKWebView, - reason: String - ) { - guard sourceSuperview !== containerView else { return } - // When Web Inspector is docked, WebKit can inject companion WK* subviews - // next to the primary WKWebView. Move those with the web view so inspector - // UI state does not get orphaned in the old host during split churn. - let relatedSubviews = relatedWebKitTransferSubviews( - from: sourceSuperview, - primaryWebView: primaryWebView - ) - guard !relatedSubviews.isEmpty else { return } -#if DEBUG - dlog( - "browser.portal.reparent.batch reason=\(reason) source=\(browserPortalDebugToken(sourceSuperview)) " + - "container=\(browserPortalDebugToken(containerView)) count=\(relatedSubviews.count) " + - "sourceType=\(String(describing: type(of: sourceSuperview))) targetType=\(String(describing: type(of: containerView))) " + - "sourceFlipped=\(sourceSuperview.isFlipped ? 1 : 0) targetFlipped=\(containerView.isFlipped ? 1 : 0) " + - "sourceBounds=\(browserPortalDebugFrame(sourceSuperview.bounds)) targetBounds=\(browserPortalDebugFrame(containerView.bounds))" - ) -#endif - for view in relatedSubviews { - let frameInWindow = sourceSuperview.convert(view.frame, to: nil) - let className = String(describing: type(of: view)) - view.removeFromSuperview() - containerView.addSubview(view, positioned: .above, relativeTo: nil) - let convertedFrame = containerView.convert(frameInWindow, from: nil) - view.frame = convertedFrame -#if DEBUG - dlog( - "browser.portal.reparent.batch.item reason=\(reason) class=\(className) " + - "view=\(browserPortalDebugToken(view)) frameInWindow=\(browserPortalDebugFrame(frameInWindow)) " + - "converted=\(browserPortalDebugFrame(convertedFrame))" - ) -#endif - } - } func detachWebView(withId webViewId: ObjectIdentifier) { cancelPendingHostedWebViewRefreshes(for: webViewId) @@ -1138,12 +1054,22 @@ final class WindowBrowserPortal: HostedViewPortalRegistry { ) #endif if let sourceSuperview = webView.superview { - moveWebKitRelatedSubviewsIfNeeded( + // When Web Inspector is docked, WebKit can inject companion WK* + // subviews next to the primary WKWebView. Move those with the web + // view so inspector UI state does not get orphaned in the old + // host during split churn. Shared with WebViewRepresentable's + // local-inline reparenting; see WebKitSubviewTransfer's doc + // comment for the unified fast-path/window-relative contract. + WebKitSubviewTransfer.move( from: sourceSuperview, to: containerView, primaryWebView: webView, reason: "bind.attachContainer" - ) + ) { message in +#if DEBUG + dlog(message) +#endif + } } else { containerView.addSubview(webView, positioned: .above, relativeTo: nil) } @@ -1459,12 +1385,18 @@ final class WindowBrowserPortal: HostedViewPortalRegistry { ) #endif if let sourceSuperview = webView.superview { - moveWebKitRelatedSubviewsIfNeeded( + // See WebKitSubviewTransfer's doc comment for the unified + // fast-path/window-relative reparenting contract. + WebKitSubviewTransfer.move( from: sourceSuperview, to: containerView, primaryWebView: webView, reason: "sync.attachContainer" - ) + ) { message in +#if DEBUG + dlog(message) +#endif + } } else { containerView.addSubview(webView, positioned: .above, relativeTo: nil) } diff --git a/Sources/Panels/BrowserPanel.swift b/Sources/Panels/BrowserPanel.swift index d500534d..0a313b16 100644 --- a/Sources/Panels/BrowserPanel.swift +++ b/Sources/Panels/BrowserPanel.swift @@ -990,6 +990,12 @@ final class BrowserPanel: Panel, ObservableObject { guard let self, self.isCurrentWebView(webView, instanceID: boundWebViewInstanceID) else { return } self.hasPromptedPasskeyHandoffForCurrentNavigation = false } + navigationDelegate.didCommit = { [weak self] webView in + guard let self, self.isCurrentWebView(webView, instanceID: boundWebViewInstanceID) else { return } + // Invalidate element refs (@eN) allocated on the previous page (M6a) — this is the + // single choke point for a committed main-frame navigation. + TerminalController.shared.v2BrowserBumpNavigationGeneration(forSurface: self.id) + } navigationDelegate.didFinish = { [weak self] webView in Task { @MainActor [weak self] in guard let self, self.isCurrentWebView(webView, instanceID: boundWebViewInstanceID) else { return } @@ -2163,12 +2169,23 @@ final class BrowserPanel: Panel, ObservableObject { ) } + /// BY DESIGN (audit 2026-08-20, M4 — decided 2026-08-20): callers of this + /// bypass fall into two classes, both deliberate. Back/forward/reload skip + /// the insecure-HTTP prompt because the user already accepted the page. + /// Session/profile restore and content-process-crash replacement ALSO skip + /// it — prompting N times at launch for tabs the user left open was judged + /// hostile UX. The trade: a plaintext http:// tab left open reloads over + /// plaintext on relaunch with no prompt. Restore-class bypasses are logged + /// to the release diagnostics channel below so the behavior is auditable. func navigateWithoutInsecureHTTPPrompt( request: URLRequest, recordTypedNavigation: Bool, preserveRestoredSessionHistory: Bool = false ) { guard let url = request.url else { return } + if preserveRestoredSessionHistory, browserShouldBlockInsecureHTTPURL(url) { + dilog("browser.restore", "insecure_http_reload_without_prompt host=\(url.host ?? "-")") + } if usesRemoteWorkspaceProxy, remoteProxyEndpoint == nil { pendingRemoteNavigation = PendingRemoteNavigation( request: request, diff --git a/Sources/Panels/BrowserPanelWebDelegates.swift b/Sources/Panels/BrowserPanelWebDelegates.swift index 59d4cf31..35193411 100644 --- a/Sources/Panels/BrowserPanelWebDelegates.swift +++ b/Sources/Panels/BrowserPanelWebDelegates.swift @@ -66,6 +66,10 @@ func browserNavigationShouldFallbackNilTargetToNewTab( class BrowserNavigationDelegate: NSObject, WKNavigationDelegate { var didStartProvisionalNavigation: ((WKWebView) -> Void)? + /// Fires once a main-frame navigation commits and the web view begins showing new content + /// (WKNavigationDelegate contract — this is the single choke point for "the page changed"). + /// Wired to invalidate stale browser-automation element refs (M6a). + var didCommit: ((WKWebView) -> Void)? var didFinish: ((WKWebView) -> Void)? var didFailNavigation: ((WKWebView, String) -> Void)? var didTerminateWebContentProcess: ((WKWebView) -> Void)? @@ -83,6 +87,10 @@ class BrowserNavigationDelegate: NSObject, WKNavigationDelegate { didStartProvisionalNavigation?(webView) } + func webView(_ webView: WKWebView, didCommit navigation: WKNavigation!) { + didCommit?(webView) + } + func webView(_ webView: WKWebView, didFinish navigation: WKNavigation!) { didFinish?(webView) } diff --git a/Sources/Panels/WebViewRepresentable.swift b/Sources/Panels/WebViewRepresentable.swift index 8be309ab..2be82f44 100644 --- a/Sources/Panels/WebViewRepresentable.swift +++ b/Sources/Panels/WebViewRepresentable.swift @@ -1606,106 +1606,6 @@ struct WebViewRepresentable: NSViewRepresentable { return last ?? webView.superview } - private static func directTransferChild(of container: NSView, containing descendant: NSView) -> NSView? { - var current: NSView? = descendant - var directChild: NSView? - while let view = current, view !== container { - directChild = view - current = view.superview - } - guard current === container else { return nil } - return directChild - } - - private static func relatedWebKitTransferSubviews( - from sourceSuperview: NSView, - primaryWebView: WKWebView - ) -> [NSView] { - var relatedSubviews: [NSView] = [] - var seen = Set() - - func append(_ candidate: NSView?) { - guard let candidate, candidate !== sourceSuperview else { return } - let id = ObjectIdentifier(candidate) - guard seen.insert(id).inserted else { return } - relatedSubviews.append(candidate) - } - - append(directTransferChild(of: sourceSuperview, containing: primaryWebView) ?? primaryWebView) - - if let inspectorFrontend = primaryWebView.programaInspectorFrontendWebView() { - append(directTransferChild(of: sourceSuperview, containing: inspectorFrontend) ?? inspectorFrontend) - } - - for view in sourceSuperview.subviews { - if view === primaryWebView { continue } - let className = String(describing: type(of: view)) - guard className.contains("WK") else { continue } - if InspectorDock.isInspectorView(view) && !InspectorDock.isVisibleCandidate(view) { - continue - } - append(view) - } - - return relatedSubviews - } - - private static func moveWebKitRelatedSubviewsIntoHostIfNeeded( - from sourceSuperview: NSView, - to container: WindowBrowserSlotView, - primaryWebView: WKWebView, - reason: String - ) { - let relatedSubviews = relatedWebKitTransferSubviews( - from: sourceSuperview, - primaryWebView: primaryWebView - ) - guard !relatedSubviews.isEmpty else { return } - let preserveSlotLocalFrames = sourceSuperview is WindowBrowserSlotView - let sourceSlotBoundsSize = sourceSuperview.bounds.size - var movedSubviewCount = 0 - var reusedSourceLocalFrames = false -#if DEBUG - dlog( - "browser.localHost.reparent.batch reason=\(reason) source=\(Self.objectID(sourceSuperview)) " + - "container=\(Self.objectID(container)) count=\(relatedSubviews.count) " + - "sourceType=\(String(describing: type(of: sourceSuperview))) targetType=\(String(describing: type(of: container)))" - ) -#endif - for view in relatedSubviews { - if view === container || view.isDescendant(of: container) { - continue - } - let className = String(describing: type(of: view)) - let targetFrame: NSRect - let currentSuperview = view.superview - if preserveSlotLocalFrames && currentSuperview === sourceSuperview { - targetFrame = view.frame - reusedSourceLocalFrames = true - } else { - let frameInWindow = currentSuperview?.convert(view.frame, to: nil) - ?? sourceSuperview.convert(view.frame, to: nil) - targetFrame = container.convert(frameInWindow, from: nil) - } - view.removeFromSuperview() - container.addSubview(view, positioned: .above, relativeTo: nil) - view.frame = targetFrame - movedSubviewCount += 1 -#if DEBUG - dlog( - "browser.localHost.reparent.batch.item reason=\(reason) class=\(className) " + - "view=\(Self.objectID(view))" - ) -#endif - } - guard movedSubviewCount > 0 else { return } - if reusedSourceLocalFrames, sourceSlotBoundsSize != container.bounds.size { - container.resizeSubviews(withOldSize: sourceSlotBoundsSize) - container.needsLayout = true - container.layoutSubtreeIfNeeded() - } - } - private static func installPortalAnchorView(_ anchorView: NSView, in host: NSView) { // SwiftUI can keep transient replacement hosts alive off-window during split // reparenting. Never let those hosts steal the shared portal anchor, or the @@ -1819,12 +1719,18 @@ struct WebViewRepresentable: NSViewRepresentable { if didAttachWebViewToLocalHost { if let sourceSuperview = Self.localInlineTransferRoot(for: webView) { - Self.moveWebKitRelatedSubviewsIntoHostIfNeeded( + // See WebKitSubviewTransfer's doc comment for the unified + // slot-local-frame-fast-path / window-relative-conversion contract. + WebKitSubviewTransfer.move( from: sourceSuperview, to: slotView, primaryWebView: webView, reason: "attachLocalHost" - ) + ) { message in +#if DEBUG + dlog(message) +#endif + } } else { slotView.addSubview(webView, positioned: .above, relativeTo: nil) } @@ -1852,14 +1758,18 @@ struct WebViewRepresentable: NSViewRepresentable { panel.restoreDeveloperToolsAfterAttachIfNeeded() if let sourceSuperview = Self.localInlineTransferRoot(for: webView), didAttachWebViewToLocalHost || sourceSuperview === slotView { - Self.moveWebKitRelatedSubviewsIntoHostIfNeeded( + WebKitSubviewTransfer.move( from: sourceSuperview, to: slotView, primaryWebView: webView, reason: didAttachWebViewToLocalHost ? "localInline.reconcile.immediate" : "localInline.reconcile.existingHost" - ) + ) { message in +#if DEBUG + dlog(message) +#endif + } } host.setHostedInspectorFrontendWebView(webView.programaInspectorFrontendWebView()) let didRevealDeveloperToolsAfterAttach = @@ -1888,12 +1798,16 @@ struct WebViewRepresentable: NSViewRepresentable { guard let host, let webView else { return } if let sourceSuperview = Self.localInlineTransferRoot(for: webView), sourceSuperview === slotView { - Self.moveWebKitRelatedSubviewsIntoHostIfNeeded( + WebKitSubviewTransfer.move( from: sourceSuperview, to: slotView, primaryWebView: webView, reason: "localInline.reconcile.async" - ) + ) { message in +#if DEBUG + dlog(message) +#endif + } } host.setHostedInspectorFrontendWebView(webView.programaInspectorFrontendWebView()) host.refreshHostedWebKitPresentation( diff --git a/Sources/SessionPersistence.swift b/Sources/SessionPersistence.swift index 48f2a301..100a5464 100644 --- a/Sources/SessionPersistence.swift +++ b/Sources/SessionPersistence.swift @@ -395,6 +395,63 @@ enum SessionPersistenceStore { return snapshot } + /// Like `load(fileURL:)`, but when the primary snapshot exists and fails to decode or is on + /// an unrecognized schema version, falls back to the newest usable archive in + /// `session-history/` (see `rotateIntoHistory`, which -- run just before this at startup -- + /// already guarantees the previous launch's intact snapshot lives there) instead of dropping + /// the whole session. Deliberately does NOT migrate an old-version history entry forward: a + /// history entry that also fails the version check is treated the same as no history at all, + /// and this returns nil exactly like `load(fileURL:)` would. Every outcome -- a used fallback + /// or an exhausted one -- is reported to the release diagnostics log so a version-bump-after- + /// update drop is distinguishable from real data loss (a silent primary-file-missing case, + /// e.g. first launch, is neither -- no diagnostics line, matching `load(fileURL:)`). + static func loadWithHistoryFallback(fileURL: URL? = nil, historyLookupLimit: Int = 5) -> AppSessionSnapshot? { + guard let fileURL = fileURL ?? defaultSnapshotFileURL() else { return nil } + guard let data = try? Data(contentsOf: fileURL) else { return nil } + + guard let snapshot = try? JSONDecoder().decode(AppSessionSnapshot.self, from: data) else { + return fallbackAfterPrimarySnapshotFailure(reason: "decode", fileURL: fileURL, limit: historyLookupLimit) + } + guard snapshot.version == SessionSnapshotSchema.currentVersion else { + return fallbackAfterPrimarySnapshotFailure(reason: "version", fileURL: fileURL, limit: historyLookupLimit) + } + guard !snapshot.windows.isEmpty else { return nil } + return snapshot + } + + private static func fallbackAfterPrimarySnapshotFailure( + reason: String, + fileURL: URL, + limit: Int + ) -> AppSessionSnapshot? { + guard let fallback = newestRestorableHistorySnapshot(fileURL: fileURL, limit: limit) else { + dilog("session.restore", "primary snapshot unusable reason=\(reason) fallback=none") + return nil + } + dilog("session.restore", "primary snapshot unusable reason=\(reason) fallback=\(fallback.filename)") + return fallback.snapshot + } + + /// Scans the newest `limit` archives (newest-first, per `historyFileURLs`) for the first one + /// that decodes at the current schema version with at least one window. Capped rather than + /// unbounded: a long-neglected `session-history/` directory should not turn a startup restore + /// into an unbounded disk scan. + private static func newestRestorableHistorySnapshot( + fileURL: URL, + limit: Int + ) -> (snapshot: AppSessionSnapshot, filename: String)? { + let candidates = historyFileURLs(fileURL: fileURL).prefix(max(0, limit)) + for entry in candidates { + guard let data = try? Data(contentsOf: entry), + let snapshot = decodeSnapshot(from: data), + snapshot.version == SessionSnapshotSchema.currentVersion, + !snapshot.windows.isEmpty + else { continue } + return (snapshot, entry.lastPathComponent) + } + return nil + } + @discardableResult static func save(_ snapshot: AppSessionSnapshot, fileURL: URL? = nil) -> Bool { guard let fileURL = fileURL ?? defaultSnapshotFileURL() else { return false } diff --git a/Sources/TerminalController+BrowserAutomation.swift b/Sources/TerminalController+BrowserAutomation.swift index 43428c74..7ca95752 100644 --- a/Sources/TerminalController+BrowserAutomation.swift +++ b/Sources/TerminalController+BrowserAutomation.swift @@ -322,16 +322,44 @@ extension TerminalController { .err(code: "not_supported", message: "\(method) is not supported on WKWebView", data: ["details": details]) } + /// Current navigation generation for a surface. Bumped by `v2BrowserBumpNavigationGeneration` + /// on every committed main-frame navigation (see `BrowserPanel.configureNavigationDelegateCallbacks`). + func v2BrowserNavigationGeneration(forSurface surfaceId: UUID) -> UInt64 { + v2BrowserNavigationGenerationBySurface[surfaceId] ?? 0 + } + + /// Invalidates every element ref allocated on the surface's previous page by advancing its + /// navigation generation (M6a). Call from the single main-frame-commit choke point only — + /// do not call per-subframe or per-provisional-navigation event. + func v2BrowserBumpNavigationGeneration(forSurface surfaceId: UUID) { + v2BrowserNavigationGenerationBySurface[surfaceId, default: 0] += 1 + } + func v2BrowserAllocateElementRef(surfaceId: UUID, selector: String) -> String { let ref = "@e\(v2BrowserNextElementOrdinal)" v2BrowserNextElementOrdinal += 1 - v2BrowserElementRefs[ref] = V2BrowserElementRefEntry(surfaceId: surfaceId, selector: selector) + v2BrowserElementRefs[ref] = V2BrowserElementRefEntry( + surfaceId: surfaceId, + selector: selector, + navigationGeneration: v2BrowserNavigationGeneration(forSurface: surfaceId) + ) return ref } - func v2BrowserResolveSelector(_ rawSelector: String, surfaceId: UUID) -> String? { + private enum V2BrowserSelectorLookup { + case literal(String) + case notFound + case stale + } + + /// Single shared resolve helper backing both `v2BrowserResolveSelector` (used by every + /// click/type/query consumer) and `v2BrowserSelectorResolutionError` (the structured error + /// to surface when resolution fails). Distinguishes a ref that once resolved but whose + /// surface has since navigated (`.stale`) from any other miss (`.notFound`) so callers can + /// report `stale_element` instead of a generic `not_found` (M6a). + private func v2BrowserLookupSelector(_ rawSelector: String, surfaceId: UUID) -> V2BrowserSelectorLookup { let trimmed = rawSelector.trimmingCharacters(in: .whitespacesAndNewlines) - guard !trimmed.isEmpty else { return nil } + guard !trimmed.isEmpty else { return .notFound } let refKey: String? = { if trimmed.hasPrefix("@e") { return trimmed } @@ -339,11 +367,39 @@ extension TerminalController { return nil }() - if let refKey { - guard let entry = v2BrowserElementRefs[refKey], entry.surfaceId == surfaceId else { return nil } - return entry.selector + guard let refKey else { return .literal(trimmed) } + + guard let entry = v2BrowserElementRefs[refKey], entry.surfaceId == surfaceId else { + return .notFound + } + guard entry.navigationGeneration == v2BrowserNavigationGeneration(forSurface: surfaceId) else { + return .stale + } + return .literal(entry.selector) + } + + func v2BrowserResolveSelector(_ rawSelector: String, surfaceId: UUID) -> String? { + if case .literal(let selector) = v2BrowserLookupSelector(rawSelector, surfaceId: surfaceId) { + return selector + } + return nil + } + + /// Error to return when `v2BrowserResolveSelector` returns nil for `rawSelector` against + /// `surfaceId`. Every consumer of the element-ref maps (resolve/click/type/query paths) + /// should surface this instead of hand-rolling a generic not_found, so a stale ref reports + /// `stale_element` rather than being indistinguishable from "never allocated". + func v2BrowserSelectorResolutionError(_ rawSelector: String, surfaceId: UUID) -> V2CallResult { + switch v2BrowserLookupSelector(rawSelector, surfaceId: surfaceId) { + case .stale: + return .err( + code: "stale_element", + message: "element ref was captured on a previous page", + data: ["ref": rawSelector] + ) + case .notFound, .literal: + return .err(code: "not_found", message: "Element reference not found", data: ["selector": rawSelector]) } - return trimmed } func v2BrowserCurrentFrameSelector(surfaceId: UUID) -> String? { @@ -912,7 +968,7 @@ extension TerminalController { return v2BrowserWithPanel(params: params) { _, ws, surfaceId, browserPanel in guard let selector = v2BrowserResolveSelector(selectorRaw, surfaceId: surfaceId) else { - return .err(code: "not_found", message: "Element reference not found", data: ["selector": selectorRaw]) + return v2BrowserSelectorResolutionError(selectorRaw, surfaceId: surfaceId) } let script = scriptBuilder(v2JSONLiteral(selector)) let retryAttempts = max(1, v2Int(params, "retry_attempts") ?? 3) @@ -1355,7 +1411,7 @@ extension TerminalController { let conditionScript: String if let selectorRaw { guard let selector = v2BrowserResolveSelector(selectorRaw, surfaceId: surfaceIdOut) else { - return .err(code: "not_found", message: "Element reference not found", data: ["selector": selectorRaw]) + return v2BrowserSelectorResolutionError(selectorRaw, surfaceId: surfaceIdOut) } let literal = v2JSONLiteral(selector) conditionScript = "document.querySelector(\(literal)) !== null" @@ -1655,8 +1711,8 @@ extension TerminalController { return v2BrowserWithPanel(params: params) { _, ws, surfaceId, browserPanel in let selector = selectorRaw.flatMap { v2BrowserResolveSelector($0, surfaceId: surfaceId) } - if selectorRaw != nil && selector == nil { - return .err(code: "not_found", message: "Element reference not found", data: ["selector": selectorRaw ?? ""]) + if let selectorRaw, selector == nil { + return v2BrowserSelectorResolutionError(selectorRaw, surfaceId: surfaceId) } let script: String @@ -1838,7 +1894,7 @@ extension TerminalController { } return v2BrowserWithPanel(params: params) { _, ws, surfaceId, browserPanel in guard let selector = v2BrowserResolveSelector(selectorRaw, surfaceId: surfaceId) else { - return .err(code: "not_found", message: "Element reference not found", data: ["selector": selectorRaw]) + return v2BrowserSelectorResolutionError(selectorRaw, surfaceId: surfaceId) } let selectorLiteral = v2JSONLiteral(selector) let script = "document.querySelectorAll(\(selectorLiteral)).length" @@ -2423,7 +2479,7 @@ extension TerminalController { } return v2BrowserWithPanel(params: params) { _, ws, surfaceId, browserPanel in guard let selector = v2BrowserResolveSelector(selectorRaw, surfaceId: surfaceId) else { - return .err(code: "not_found", message: "Element reference not found", data: ["selector": selectorRaw]) + return v2BrowserSelectorResolutionError(selectorRaw, surfaceId: surfaceId) } let selectorLiteral = v2JSONLiteral(selector) let script = """ @@ -2463,7 +2519,7 @@ extension TerminalController { } return v2BrowserWithPanel(params: params) { _, ws, surfaceId, browserPanel in guard let selector = v2BrowserResolveSelector(selectorRaw, surfaceId: surfaceId) else { - return .err(code: "not_found", message: "Element reference not found", data: ["selector": selectorRaw]) + return v2BrowserSelectorResolutionError(selectorRaw, surfaceId: surfaceId) } let selectorLiteral = v2JSONLiteral(selector) let script = """ @@ -2512,7 +2568,7 @@ extension TerminalController { return v2BrowserWithPanel(params: params) { _, ws, surfaceId, browserPanel in guard let selector = v2BrowserResolveSelector(selectorRaw, surfaceId: surfaceId) else { - return .err(code: "not_found", message: "Element reference not found", data: ["selector": selectorRaw]) + return v2BrowserSelectorResolutionError(selectorRaw, surfaceId: surfaceId) } let selectorLiteral = v2JSONLiteral(selector) let script = """ @@ -2562,7 +2618,7 @@ extension TerminalController { return v2BrowserWithPanel(params: params) { _, ws, surfaceId, browserPanel in guard let selector = v2BrowserResolveSelector(selectorRaw, surfaceId: surfaceId) else { - return .err(code: "not_found", message: "Element reference not found", data: ["selector": selectorRaw]) + return v2BrowserSelectorResolutionError(selectorRaw, surfaceId: surfaceId) } let selectorLiteral = v2JSONLiteral(selector) let script = """ diff --git a/Sources/TerminalController.swift b/Sources/TerminalController.swift index 524a91a0..16f53470 100644 --- a/Sources/TerminalController.swift +++ b/Sources/TerminalController.swift @@ -169,6 +169,11 @@ class TerminalController { struct V2BrowserElementRefEntry { let surfaceId: UUID let selector: String + /// The surface's navigation generation (see `v2BrowserNavigationGenerationBySurface`) + /// at the moment this ref was allocated. Resolving the ref against a surface whose + /// generation has since advanced (i.e. it navigated) is a stale-ref error rather than + /// silently re-resolving the selector against the new page's DOM. + let navigationGeneration: UInt64 } final class V2BrowserUndefinedSentinel {} @@ -181,6 +186,11 @@ class TerminalController { var v2BrowserNextElementOrdinal: Int = 1 var v2BrowserElementRefs: [String: V2BrowserElementRefEntry] = [:] var v2BrowserFrameSelectorBySurface: [UUID: String] = [:] + /// Bumped on every committed main-frame navigation of a browser surface. Element refs + /// (`v2BrowserElementRefs`) capture the generation at allocation time so a ref from a + /// previous page can be rejected instead of silently re-resolving against the new DOM + /// (M6a). Main-thread only, same discipline as the other v2Browser state above. + var v2BrowserNavigationGenerationBySurface: [UUID: UInt64] = [:] var v2BrowserInitScriptsBySurface: [UUID: [String]] = [:] var v2BrowserInitStylesBySurface: [UUID: [String]] = [:] var v2BrowserDownloadEventsBySurface: [UUID: [[String: Any]]] = [:] @@ -2262,21 +2272,75 @@ class TerminalController { private func v2RefreshKnownRefs() { guard let app = AppDelegate.shared else { return } + var liveWindowIds = Set() + var liveWorkspaceIds = Set() + var livePaneIds = Set() + var liveSurfaceIds = Set() + let windows = app.listMainWindowSummaries() for item in windows { + liveWindowIds.insert(item.windowId) _ = v2EnsureHandleRef(kind: .window, uuid: item.windowId) if let tm = app.tabManagerFor(windowId: item.windowId) { for ws in tm.tabs { + liveWorkspaceIds.insert(ws.id) _ = v2EnsureHandleRef(kind: .workspace, uuid: ws.id) for paneId in ws.bonsplitController.allPaneIds { + livePaneIds.insert(paneId.id) _ = v2EnsureHandleRef(kind: .pane, uuid: paneId.id) } for panelId in ws.panels.keys { + liveSurfaceIds.insert(panelId) _ = v2EnsureHandleRef(kind: .surface, uuid: panelId) } } } } + + v2PruneDeadHandleRefs( + liveWindowIds: liveWindowIds, + liveWorkspaceIds: liveWorkspaceIds, + livePaneIds: livePaneIds, + liveSurfaceIds: liveSurfaceIds + ) + } + + /// Drops `v2RefByUUID`/`v2UUIDByRef` entries for UUIDs no longer present in the live + /// object graph (M8). Runs as a sweep at refresh time rather than adding teardown hooks + /// at every window/workspace/pane/surface destruction site. + /// + /// Never touches `v2NextHandleOrdinal`: the per-kind ordinal counter stays monotonic, so a + /// pruned-then-reappearing UUID gets a brand-new ref rather than reusing a number that + /// might still be cached by a client as pointing at the old object. Only the map entries + /// (the thing that actually grows unbounded) are dropped. + /// Internal (not private) so the unit-test target can exercise the never-reissue invariant + /// directly, bypassing the AppDelegate-dependent enumeration in `v2RefreshKnownRefs`. + func v2PruneDeadHandleRefs( + liveWindowIds: Set, + liveWorkspaceIds: Set, + livePaneIds: Set, + liveSurfaceIds: Set + ) { + let liveByKind: [V2HandleKind: Set] = [ + .window: liveWindowIds, + .workspace: liveWorkspaceIds, + .pane: livePaneIds, + .surface: liveSurfaceIds, + ] + + v2HandleRefStateLock.lock() + defer { v2HandleRefStateLock.unlock() } + + for kind in V2HandleKind.allCases { + guard let live = liveByKind[kind], var byUUID = v2RefByUUID[kind] else { continue } + var byRef = v2UUIDByRef[kind] ?? [:] + for (uuid, ref) in byUUID where !live.contains(uuid) { + byUUID.removeValue(forKey: uuid) + byRef.removeValue(forKey: ref) + } + v2RefByUUID[kind] = byUUID + v2UUIDByRef[kind] = byRef + } } // MARK: - V2 Param Parsing diff --git a/Sources/Update/UpdateController.swift b/Sources/Update/UpdateController.swift index d3a88833..0d219a9d 100644 --- a/Sources/Update/UpdateController.swift +++ b/Sources/Update/UpdateController.swift @@ -19,17 +19,20 @@ enum UpdateSettings { static let scheduledCheckIntervalKey = "SUScheduledCheckInterval" static let sendProfileInfoKey = "SUSendProfileInfo" static let migrationKey = "programa.sparkle.automaticChecksMigration.v2" + static let autoInstallMigrationKey = "programa.sparkle.autoInstallMigration.v3" static let previousDefaultScheduledCheckInterval: TimeInterval = 60 * 60 * 24 static let scheduledCheckInterval: TimeInterval = 60 * 60 static func apply(to defaults: UserDefaults) { defaults.register(defaults: [ automaticChecksKey: true, - automaticallyUpdateKey: false, + automaticallyUpdateKey: true, scheduledCheckIntervalKey: scheduledCheckInterval, sendProfileInfoKey: false, ]) + applyAutoInstallMigration(to: defaults) + guard !defaults.bool(forKey: migrationKey) else { return } // Repair older installs that may have ended up with automatic checks disabled @@ -46,15 +49,30 @@ enum UpdateSettings { defaults.set(scheduledCheckInterval, forKey: scheduledCheckIntervalKey) } - if defaults.object(forKey: automaticallyUpdateKey) == nil { - defaults.set(false, forKey: automaticallyUpdateKey) - } + // automaticallyUpdateKey is deliberately NOT written here anymore: the + // registered default (true, since the 2026-08-20 auto-install flip) + // covers the nil case, and writing a concrete value would defeat the + // v3 migration below for installs that reach v2 first. if defaults.object(forKey: sendProfileInfoKey) == nil { defaults.set(false, forKey: sendProfileInfoKey) } defaults.set(true, forKey: migrationKey) } + + /// 2026-08-20 default flip (audit follow-up): silent auto-install is now + /// the default — the single-lane, fix-forward release model only works if + /// fixes actually reach users, and a crash-looping user never clicks the + /// pill. The v2 migration wrote a concrete `false` into every existing + /// install's defaults, so flipping the registered default alone would + /// never reach them: this one-time migration clears that stored value so + /// the registered default (true) takes effect. The Settings toggle writes + /// a concrete value afterwards and wins permanently — this runs once. + private static func applyAutoInstallMigration(to defaults: UserDefaults) { + guard !defaults.bool(forKey: autoInstallMigrationKey) else { return } + defaults.removeObject(forKey: automaticallyUpdateKey) + defaults.set(true, forKey: autoInstallMigrationKey) + } } /// Controller for managing Sparkle updates in cmux. @@ -121,6 +139,7 @@ class UpdateController { defaults.removeObject(forKey: UpdateSettings.scheduledCheckIntervalKey) defaults.removeObject(forKey: UpdateSettings.sendProfileInfoKey) defaults.removeObject(forKey: UpdateSettings.migrationKey) + defaults.removeObject(forKey: UpdateSettings.autoInstallMigrationKey) defaults.synchronize() UpdateLogStore.shared.append("reset sparkle permission defaults (ui test)") } diff --git a/Sources/WebKitSubviewTransfer.swift b/Sources/WebKitSubviewTransfer.swift new file mode 100644 index 00000000..bc2b178c --- /dev/null +++ b/Sources/WebKitSubviewTransfer.swift @@ -0,0 +1,153 @@ +import AppKit +import WebKit + +/// Shared subview-selection and reparenting logic for moving a browser +/// `WKWebView`'s docked-inspector companion views into a new host container. +/// +/// This used to be two near-identical copy/paste implementations: +/// `BrowserWindowPortal.moveWebKitRelatedSubviewsIfNeeded` (external-window +/// portal reparenting) and `WebViewRepresentable.moveWebKitRelatedSubviewsIntoHostIfNeeded` +/// (local-inline host reparenting). The subview-selection half +/// (`relatedSubviews`/`directTransferChild`) was byte-identical between the +/// two files, but the move/frame-conversion half had diverged: +/// `WebViewRepresentable` picked up a `preserveSlotLocalFrames` fast path in +/// commit efc759ecb9 ("fix: DevTools pane breaks after workspace switch +/// round-trips") that reuses a related subview's existing local frame +/// directly when it is still parented under the same `WindowBrowserSlotView` +/// as the primary web view — skipping the window-relative frame conversion — +/// then reconciles via `resizeSubviews(withOldSize:)` if the slot's bounds +/// size changed across the move. `BrowserWindowPortal`'s copy never received +/// that fix and always performed the window-relative +/// `convert(to: nil)`/`convert(from: nil)` round trip, even when both +/// endpoints were slot views. +/// +/// This type unifies both call sites onto the newer, deliberate behavior: the +/// fast path applies whenever it qualifies (source is a `WindowBrowserSlotView` +/// and a given related view's current superview still matches `sourceSuperview` +/// at the moment it is processed); everything else falls back to the +/// window-relative conversion, matching both originals' non-slot behavior. +/// +/// Behavioral notes vs. the two originals: +/// - `removeFromSuperview()` -> `addSubview(_:positioned:.above,relativeTo:nil)` +/// call order and `.above` z-position are preserved exactly, so z-order and +/// first-responder side effects match both originals (neither original did +/// anything special with first responder, and AppKit does not resign first +/// responder on an in-window reparent). +/// - The per-view `view === destination || view.isDescendant(of: destination)` +/// skip (from the same April 2026 fix) is kept as the sole no-op guard for +/// the `sourceSuperview === destination` case. `BrowserWindowPortal`'s old +/// copy had a redundant top-level `guard sourceSuperview !== containerView` +/// early return, but that guard was dead code at both of its call sites +/// (each only calls the move helper when `webView.superview !== containerView` +/// holds), so dropping it changes nothing there. `WebViewRepresentable` +/// deliberately calls this with `sourceSuperview === destination` from its +/// "reconcile" call sites (`localInline.reconcile.existingHost` / +/// `.async`) to safely re-run the same no-op path; keeping only the +/// per-view skip (and no top-level early return) preserves that call +/// pattern unchanged. +/// - Debug log tokens are unified to a single `browser.webKitTransfer.reparent.*` +/// prefix instead of each call site's own prefix +/// (`browser.portal.reparent.batch*` / `browser.localHost.reparent.batch*`). +/// This is DEBUG-log-only; no runtime behavior reads these strings. +enum WebKitSubviewTransfer { + static func directTransferChild(of container: NSView, containing descendant: NSView) -> NSView? { + var current: NSView? = descendant + var directChild: NSView? + while let view = current, view !== container { + directChild = view + current = view.superview + } + guard current === container else { return nil } + return directChild + } + + static func relatedSubviews( + from sourceSuperview: NSView, + primaryWebView: WKWebView + ) -> [NSView] { + var relatedSubviews: [NSView] = [] + var seen = Set() + + func append(_ candidate: NSView?) { + guard let candidate, candidate !== sourceSuperview else { return } + let id = ObjectIdentifier(candidate) + guard seen.insert(id).inserted else { return } + relatedSubviews.append(candidate) + } + + append(directTransferChild(of: sourceSuperview, containing: primaryWebView) ?? primaryWebView) + + if let inspectorFrontend = primaryWebView.programaInspectorFrontendWebView() { + append(directTransferChild(of: sourceSuperview, containing: inspectorFrontend) ?? inspectorFrontend) + } + + for view in sourceSuperview.subviews { + if view === primaryWebView { continue } + let className = String(describing: type(of: view)) + guard className.contains("WK") else { continue } + if InspectorDock.isInspectorView(view) && !InspectorDock.isVisibleCandidate(view) { + continue + } + append(view) + } + + return relatedSubviews + } + + /// Moves `primaryWebView`'s related WebKit companion subviews (e.g. a + /// docked Web Inspector frontend) from `sourceSuperview` into + /// `destination`, using the slot-local fast path when both endpoints + /// qualify and falling back to a window-relative frame conversion + /// otherwise. See the type-level doc comment for the full behavioral + /// contract vs. the two call sites this replaces. + static func move( + from sourceSuperview: NSView, + to destination: WindowBrowserSlotView, + primaryWebView: WKWebView, + reason: String, + debugLog: ((String) -> Void)? = nil + ) { + let related = relatedSubviews(from: sourceSuperview, primaryWebView: primaryWebView) + guard !related.isEmpty else { return } + + let preserveSlotLocalFrames = sourceSuperview is WindowBrowserSlotView + let sourceSlotBoundsSize = sourceSuperview.bounds.size + var movedCount = 0 + var reusedSourceLocalFrames = false + + debugLog?( + "browser.webKitTransfer.reparent.batch reason=\(reason) count=\(related.count) " + + "sourceType=\(String(describing: type(of: sourceSuperview))) " + + "targetType=\(String(describing: type(of: destination)))" + ) + + for view in related { + if view === destination || view.isDescendant(of: destination) { + continue + } + let className = String(describing: type(of: view)) + let targetFrame: NSRect + let currentSuperview = view.superview + if preserveSlotLocalFrames && currentSuperview === sourceSuperview { + targetFrame = view.frame + reusedSourceLocalFrames = true + } else { + let frameInWindow = currentSuperview?.convert(view.frame, to: nil) + ?? sourceSuperview.convert(view.frame, to: nil) + targetFrame = destination.convert(frameInWindow, from: nil) + } + view.removeFromSuperview() + destination.addSubview(view, positioned: .above, relativeTo: nil) + view.frame = targetFrame + movedCount += 1 + debugLog?("browser.webKitTransfer.reparent.item reason=\(reason) class=\(className)") + } + + guard movedCount > 0 else { return } + if reusedSourceLocalFrames, sourceSlotBoundsSize != destination.bounds.size { + destination.resizeSubviews(withOldSize: sourceSlotBoundsSize) + destination.needsLayout = true + destination.layoutSubtreeIfNeeded() + } + } +} diff --git a/Sources/WorkspaceRemoteSessionController+DaemonInstall.swift b/Sources/WorkspaceRemoteSessionController+DaemonInstall.swift index 4a69df87..51d26715 100644 --- a/Sources/WorkspaceRemoteSessionController+DaemonInstall.swift +++ b/Sources/WorkspaceRemoteSessionController+DaemonInstall.swift @@ -34,7 +34,7 @@ extension WorkspaceRemoteSessionController { debugLog("remote.bootstrap.binaryExists remotePath=\(remotePath) exists=\(hadExistingBinary ? 1 : 0)") if forceExplicitOverrideInstall || !hadExistingBinary { let localBinary = try buildLocalDaemonBinary(goOS: platform.goOS, goArch: platform.goArch, version: version) - try uploadRemoteDaemonBinaryLocked(localBinary: localBinary, remotePath: remotePath) + try uploadRemoteDaemonBinaryLocked(localBinary: localBinary, remotePath: remotePath, currentVersion: version) } var hello: DaemonHello @@ -49,13 +49,13 @@ extension WorkspaceRemoteSessionController { "detail=\(error.localizedDescription)" ) let localBinary = try buildLocalDaemonBinary(goOS: platform.goOS, goArch: platform.goArch, version: version) - try uploadRemoteDaemonBinaryLocked(localBinary: localBinary, remotePath: remotePath) + try uploadRemoteDaemonBinaryLocked(localBinary: localBinary, remotePath: remotePath, currentVersion: version) hello = try helloRemoteDaemonLocked(remotePath: remotePath) } if hadExistingBinary, !hello.capabilities.contains(WorkspaceRemoteDaemonRPCClient.requiredProxyStreamCapability) { debugLog("remote.bootstrap.capabilityMissing remotePath=\(remotePath) capabilities=\(hello.capabilities.joined(separator: ","))") let localBinary = try buildLocalDaemonBinary(goOS: platform.goOS, goArch: platform.goArch, version: version) - try uploadRemoteDaemonBinaryLocked(localBinary: localBinary, remotePath: remotePath) + try uploadRemoteDaemonBinaryLocked(localBinary: localBinary, remotePath: remotePath, currentVersion: version) hello = try helloRemoteDaemonLocked(remotePath: remotePath) } @@ -454,7 +454,7 @@ extension WorkspaceRemoteSessionController { return output } - func uploadRemoteDaemonBinaryLocked(localBinary: URL, remotePath: String) throws { + func uploadRemoteDaemonBinaryLocked(localBinary: URL, remotePath: String, currentVersion: String) throws { let remoteDirectory = (remotePath as NSString).deletingLastPathComponent let remoteTempPath = "\(remotePath).tmp-\(UUID().uuidString.prefix(8))" debugLog( @@ -484,9 +484,22 @@ extension WorkspaceRemoteSessionController { ]) } + // Prune runs only once chmod+mv have confirmed the current version's + // binary is in place, and its own failure must never fail the install: + // it is wrapped in a subshell with `|| true`, which is also the last + // command in the `then` branch, so a successful install always exits 0 + // regardless of what pruning does. A failed chmod/mv skips pruning + // entirely and exits 1 via the `else` branch, so a prune bug can never + // be mistaken for (or mask) an install failure. let finalizeScript = """ - chmod 755 \(RemoteSSHConnectionPolicy.shellSingleQuoted(remoteTempPath)) && \ - mv \(RemoteSSHConnectionPolicy.shellSingleQuoted(remoteTempPath)) \(RemoteSSHConnectionPolicy.shellSingleQuoted(remotePath)) + if chmod 755 \(RemoteSSHConnectionPolicy.shellSingleQuoted(remoteTempPath)) && \ + mv \(RemoteSSHConnectionPolicy.shellSingleQuoted(remoteTempPath)) \(RemoteSSHConnectionPolicy.shellSingleQuoted(remotePath)); then + ( + \(Self.remoteDaemonPruneStaleVersionsScript(currentVersion: currentVersion)) + ) || true + else + exit 1 + fi """ let finalizeCommand = "sh -c \(RemoteSSHConnectionPolicy.shellSingleQuoted(finalizeScript))" let finalizeResult = try sshExec(arguments: sshCommonArguments(batchMode: true) + [configuration.destination, finalizeCommand], timeout: 12) diff --git a/Sources/WorkspaceRemoteSessionController+ScriptBuilders.swift b/Sources/WorkspaceRemoteSessionController+ScriptBuilders.swift index b5910451..d08023ea 100644 --- a/Sources/WorkspaceRemoteSessionController+ScriptBuilders.swift +++ b/Sources/WorkspaceRemoteSessionController+ScriptBuilders.swift @@ -208,6 +208,62 @@ extension WorkspaceRemoteSessionController { ".programa/bin/programad-remote/\(version)/\(goOS)-\(goArch)/programad-remote" } + /// Prunes stale `programad-remote` version install directories under + /// `$HOME/.programa/bin/programad-remote/`, keeping the current version + /// (just confirmed present by the caller, after a successful install) + /// plus the most-recently-used other version directory (audit finding + /// M12: version-scoped installs otherwise accumulate forever on remote + /// hosts, since each app version only probes/uploads its own directory + /// and never touches others). + /// + /// Version strings are `major.minor.patch` where `patch` is a CI run + /// number (e.g. "0.4.9" vs "0.4.100"), so a lexical sort of version + /// directory names would misorder them. There is no existing + /// version-compare helper among this file's POSIX sh script builders, + /// so retention is decided by directory mtime (newest = most recently + /// installed/used) instead of parsing/comparing version strings in + /// shell -- simpler and correct without a numeric-segment parser. + static func remoteDaemonPruneStaleVersionsScript(currentVersion: String) -> String { + let trimmedVersion = currentVersion.trimmingCharacters(in: .whitespacesAndNewlines) + let quotedVersion = RemoteSSHConnectionPolicy.shellSingleQuoted(trimmedVersion) + return """ + programa_daemon_base="$HOME/.programa/bin/programad-remote" + programa_current_version=\(quotedVersion) + if [ -n "$programa_daemon_base" ] && [ -d "$programa_daemon_base" ]; then + programa_keep_other="" + programa_keep_other_mtime=0 + for programa_version_dir in "$programa_daemon_base"/*/; do + [ -d "$programa_version_dir" ] || continue + [ -L "${programa_version_dir%/}" ] && continue + programa_version_name="$(basename "$programa_version_dir")" + [ "$programa_version_name" = "$programa_current_version" ] && continue + programa_dir_mtime="$(stat -f '%m' "$programa_version_dir" 2>/dev/null || stat -c '%Y' "$programa_version_dir" 2>/dev/null || echo 0)" + case "$programa_dir_mtime" in + ''|*[!0-9]*) programa_dir_mtime=0 ;; + esac + if [ "$programa_dir_mtime" -gt "$programa_keep_other_mtime" ]; then + programa_keep_other_mtime="$programa_dir_mtime" + programa_keep_other="$programa_version_name" + fi + done + for programa_version_dir in "$programa_daemon_base"/*/; do + [ -d "$programa_version_dir" ] || continue + [ -L "${programa_version_dir%/}" ] && continue + programa_version_name="$(basename "$programa_version_dir")" + [ "$programa_version_name" = "$programa_current_version" ] && continue + if [ -n "$programa_keep_other" ] && [ "$programa_version_name" = "$programa_keep_other" ]; then + continue + fi + case "$programa_version_dir" in + "$programa_daemon_base"/*) + rm -rf -- "$programa_version_dir" || true + ;; + esac + done + fi + """ + } + static func orphanedCMUXRemoteSSHPIDs( psOutput: String, destination: String, diff --git a/docs/remote-daemon-spec.md b/docs/remote-daemon-spec.md index baa746c3..5baea932 100644 --- a/docs/remote-daemon-spec.md +++ b/docs/remote-daemon-spec.md @@ -198,6 +198,7 @@ Before declaring browser proxying complete: 1. Proxy auth policy for local broker (`none` vs optional credentials). 2. Reconnect backoff profile and max retry budget. +3. `DONE` version-scoped `programad-remote` installs under `$HOME/.programa/bin/programad-remote/` are pruned on every fresh install (audit M12): retention is the current version plus the most-recently-used other version, decided by directory mtime rather than version-string comparison. ## 10. Socket API Contract Notes diff --git a/programaTests/SessionPersistenceTests.swift b/programaTests/SessionPersistenceTests.swift index 6bed0ea3..2a4154d6 100644 --- a/programaTests/SessionPersistenceTests.swift +++ b/programaTests/SessionPersistenceTests.swift @@ -425,6 +425,69 @@ final class SessionPersistenceTests: XCTestCase { XCTAssertNil(SessionPersistenceStore.load(fileURL: snapshotURL)) } + func testLoadWithHistoryFallbackReturnsHistoryCopyWhenPrimaryIsCorrupt() throws { + let tempDir = FileManager.default.temporaryDirectory + .appendingPathComponent("cmux-session-tests-\(UUID().uuidString)", isDirectory: true) + try FileManager.default.createDirectory(at: tempDir, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: tempDir) } + + let snapshotURL = tempDir.appendingPathComponent("session.json", isDirectory: false) + var snapshot = makeSnapshot(version: SessionSnapshotSchema.currentVersion) + snapshot.windows[0].tabManager.workspaces[0].customTitle = "Intact History Copy" + XCTAssertTrue(SessionPersistenceStore.save(snapshot, fileURL: snapshotURL)) + + // Archives the just-saved (intact) snapshot into session-history/ via the store's own + // rotation seam, exactly like startup does before overwriting the primary file. + XCTAssertTrue(SessionPersistenceStore.rotateIntoHistory(fileURL: snapshotURL)) + XCTAssertEqual(SessionPersistenceStore.historyFileURLs(fileURL: snapshotURL).count, 1) + + // Simulate a crash mid-write / disk corruption: truncate the primary file so it no + // longer decodes. + try Data("{\"version\":".utf8).write(to: snapshotURL) + XCTAssertNil(SessionPersistenceStore.load(fileURL: snapshotURL), "Strict load must stay nil on corrupt primary data") + + let restored = SessionPersistenceStore.loadWithHistoryFallback(fileURL: snapshotURL) + XCTAssertEqual( + restored?.windows.first?.tabManager.workspaces.first?.customTitle, + "Intact History Copy", + "A corrupt primary snapshot should fall back to the archived history copy" + ) + } + + func testLoadWithHistoryFallbackReturnsHistoryCopyWhenPrimaryVersionMismatches() throws { + let tempDir = FileManager.default.temporaryDirectory + .appendingPathComponent("cmux-session-tests-\(UUID().uuidString)", isDirectory: true) + try FileManager.default.createDirectory(at: tempDir, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: tempDir) } + + let snapshotURL = tempDir.appendingPathComponent("session.json", isDirectory: false) + let snapshot = makeSnapshot(version: SessionSnapshotSchema.currentVersion) + XCTAssertTrue(SessionPersistenceStore.save(snapshot, fileURL: snapshotURL)) + XCTAssertTrue(SessionPersistenceStore.rotateIntoHistory(fileURL: snapshotURL)) + + // A future schema-bumped primary must not shadow the still-current-version archive. + XCTAssertTrue( + SessionPersistenceStore.save(makeSnapshot(version: SessionSnapshotSchema.currentVersion + 1), fileURL: snapshotURL) + ) + + let restored = SessionPersistenceStore.loadWithHistoryFallback(fileURL: snapshotURL) + XCTAssertEqual(restored?.version, SessionSnapshotSchema.currentVersion) + XCTAssertEqual(restored?.windows.count, 1) + } + + func testLoadWithHistoryFallbackReturnsNilWhenHistoryIsAlsoUnusable() throws { + let tempDir = FileManager.default.temporaryDirectory + .appendingPathComponent("cmux-session-tests-\(UUID().uuidString)", isDirectory: true) + try FileManager.default.createDirectory(at: tempDir, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: tempDir) } + + let snapshotURL = tempDir.appendingPathComponent("session.json", isDirectory: false) + // No history/ directory has ever been created for this snapshot path. + try Data("not json".utf8).write(to: snapshotURL) + + XCTAssertNil(SessionPersistenceStore.loadWithHistoryFallback(fileURL: snapshotURL)) + } + func testDefaultSnapshotPathSanitizesBundleIdentifier() { let tempDir = FileManager.default.temporaryDirectory .appendingPathComponent("cmux-session-tests-\(UUID().uuidString)", isDirectory: true) diff --git a/programaTests/ShortcutAndCommandPaletteTests.swift b/programaTests/ShortcutAndCommandPaletteTests.swift index a34fe4e9..4f96a47d 100644 --- a/programaTests/ShortcutAndCommandPaletteTests.swift +++ b/programaTests/ShortcutAndCommandPaletteTests.swift @@ -1077,11 +1077,33 @@ final class UpdateSettingsTests: XCTestCase { XCTAssertTrue(defaults.bool(forKey: UpdateSettings.automaticChecksKey)) XCTAssertEqual(defaults.double(forKey: UpdateSettings.scheduledCheckIntervalKey), UpdateSettings.scheduledCheckInterval) - XCTAssertFalse(defaults.bool(forKey: UpdateSettings.automaticallyUpdateKey)) + // Silent auto-install became the default 2026-08-20 (audit follow-up). + XCTAssertTrue(defaults.bool(forKey: UpdateSettings.automaticallyUpdateKey)) XCTAssertFalse(defaults.bool(forKey: UpdateSettings.sendProfileInfoKey)) XCTAssertTrue(defaults.bool(forKey: UpdateSettings.migrationKey)) } + func testAutoInstallMigrationFlipsStoredFalseOnceAndRespectsLaterUserChoice() { + let defaults = makeDefaults() + // The v2 migration wrote a concrete false into existing installs. + defaults.set(false, forKey: UpdateSettings.automaticallyUpdateKey) + + UpdateSettings.apply(to: defaults) + XCTAssertTrue( + defaults.bool(forKey: UpdateSettings.automaticallyUpdateKey), + "the v3 migration must clear the v2-era stored false so the new registered default applies" + ) + XCTAssertTrue(defaults.bool(forKey: UpdateSettings.autoInstallMigrationKey)) + + // A user turning it off AFTER the migration is a real choice and must survive. + defaults.set(false, forKey: UpdateSettings.automaticallyUpdateKey) + UpdateSettings.apply(to: defaults) + XCTAssertFalse( + defaults.bool(forKey: UpdateSettings.automaticallyUpdateKey), + "the migration must run once — a post-migration user choice wins" + ) + } + func testApplyRepairsLegacyDisabledAutomaticChecksOnce() { let defaults = makeDefaults() defaults.set(false, forKey: UpdateSettings.automaticChecksKey) diff --git a/programaTests/TerminalAndGhosttyTests.swift b/programaTests/TerminalAndGhosttyTests.swift index 72611e9f..0f4d7bad 100644 --- a/programaTests/TerminalAndGhosttyTests.swift +++ b/programaTests/TerminalAndGhosttyTests.swift @@ -4350,3 +4350,93 @@ final class TerminalControllerSocketListenerHealthTests: XCTestCase { ) } } + +// MARK: - V2 Browser Automation Ref Invariants (audit M6a / M8) + +@MainActor +final class TerminalControllerV2RefInvariantTests: XCTestCase { + /// M6a: an element ref (@eN) allocated before a navigation must not silently re-resolve + /// against the new page's DOM after the surface navigates — it should report a structured + /// `stale_element` error instead. + func testElementRefResolvesUntilSurfaceNavigatesThenReportsStale() { + let surfaceId = UUID() + let ref = TerminalController.shared.v2BrowserAllocateElementRef(surfaceId: surfaceId, selector: "#foo") + + // Before any navigation, the ref resolves normally. + XCTAssertEqual(TerminalController.shared.v2BrowserResolveSelector(ref, surfaceId: surfaceId), "#foo") + + // Simulate a committed main-frame navigation on that surface (BrowserPanel's + // navigationDelegate.didCommit calls this in production). + TerminalController.shared.v2BrowserBumpNavigationGeneration(forSurface: surfaceId) + + // The pre-navigation ref must no longer resolve... + XCTAssertNil(TerminalController.shared.v2BrowserResolveSelector(ref, surfaceId: surfaceId)) + + // ...and the error surfaced must specifically be stale_element, not a generic not_found, + // so callers can distinguish "this ref is dead" from "this ref never existed". + switch TerminalController.shared.v2BrowserSelectorResolutionError(ref, surfaceId: surfaceId) { + case .err(let code, _, let data): + XCTAssertEqual(code, "stale_element") + XCTAssertEqual(data as? [String: String], ["ref": ref]) + case .ok: + XCTFail("expected an error result for a stale ref") + } + + // A ref allocated *after* the navigation on the same surface resolves normally again. + let freshRef = TerminalController.shared.v2BrowserAllocateElementRef(surfaceId: surfaceId, selector: "#bar") + XCTAssertEqual(TerminalController.shared.v2BrowserResolveSelector(freshRef, surfaceId: surfaceId), "#bar") + } + + /// M6a: a ref allocated on one surface must never resolve against a different surface, with + /// or without a navigation — unrelated to staleness, but exercised on the same shared helper. + func testElementRefDoesNotResolveAgainstAnotherSurface() { + let surfaceA = UUID() + let surfaceB = UUID() + let ref = TerminalController.shared.v2BrowserAllocateElementRef(surfaceId: surfaceA, selector: "#foo") + + XCTAssertNil(TerminalController.shared.v2BrowserResolveSelector(ref, surfaceId: surfaceB)) + switch TerminalController.shared.v2BrowserSelectorResolutionError(ref, surfaceId: surfaceB) { + case .err(let code, _, _): + XCTAssertEqual(code, "not_found") + case .ok: + XCTFail("expected an error result") + } + } + + /// M8: pruning dead handle-ref map entries must never let a ref string get reissued for a + /// different UUID. The per-kind ordinal counter is untouched by pruning — a + /// pruned-then-reappearing UUID gets a brand-new ref, not its old one back, and no other + /// UUID can ever receive a ref that used to point at it. + func testPruningDeadHandleRefsNeverReissuesARefForADifferentUUID() { + let uuidA = UUID() + let uuidB = UUID() + + guard let refA1 = TerminalController.shared.v2Ref(kind: .surface, uuid: uuidA) as? String else { + return XCTFail("expected a ref string") + } + + // Simulate uuidA no longer being part of the live object graph (window/workspace/pane/ + // surface all empty) — the sweep v2RefreshKnownRefs performs after enumerating the live + // windows/workspaces/panes/surfaces. + TerminalController.shared.v2PruneDeadHandleRefs( + liveWindowIds: [], + liveWorkspaceIds: [], + livePaneIds: [], + liveSurfaceIds: [] + ) + + // A different UUID allocated after the prune must never land on uuidA's old ref. + guard let refB = TerminalController.shared.v2Ref(kind: .surface, uuid: uuidB) as? String else { + return XCTFail("expected a ref string") + } + XCTAssertNotEqual(refB, refA1) + + // uuidA reappearing gets a brand-new ref (the ordinal counter is never rewound by + // pruning) — not its old ref, and not uuidB's ref either. + guard let refA2 = TerminalController.shared.v2Ref(kind: .surface, uuid: uuidA) as? String else { + return XCTFail("expected a ref string") + } + XCTAssertNotEqual(refA2, refA1) + XCTAssertNotEqual(refA2, refB) + } +} diff --git a/programaTests/WorkspaceRemoteConnectionTests.swift b/programaTests/WorkspaceRemoteConnectionTests.swift index d6468654..8bbe5663 100644 --- a/programaTests/WorkspaceRemoteConnectionTests.swift +++ b/programaTests/WorkspaceRemoteConnectionTests.swift @@ -203,6 +203,90 @@ final class WorkspaceRemoteConnectionTests: XCTestCase { XCTAssertFalse(fileManager.fileExists(atPath: ttyURL.path)) } + func testRemoteDaemonPruneStaleVersionsScriptKeepsCurrentAndNewestOtherByMtime() throws { + let fileManager = FileManager.default + let home = fileManager.temporaryDirectory.appendingPathComponent("cmux-daemon-prune-\(UUID().uuidString)") + let daemonBase = home.appendingPathComponent(".programa/bin/programad-remote") + defer { try? fileManager.removeItem(at: home) } + + let currentVersion = "0.4.213" + // "0.4.100" is deliberately the most-recently-used "other" version by mtime + // while being lexically *smaller* than "0.4.99" -- this proves retention is + // decided by directory mtime, not by comparing version strings, since patch + // is a CI run number (0.4.9 vs 0.4.100 sorts backwards lexically). + let baseDate = Date(timeIntervalSince1970: 1_700_000_000) + let mtimeOffsetByVersion: [String: TimeInterval] = [ + "0.4.9": 100, + "0.4.99": 200, + "0.4.100": 300, + currentVersion: 400, + ] + + var versionDirectories: [String: URL] = [:] + for (version, offset) in mtimeOffsetByVersion { + let platformDir = daemonBase.appendingPathComponent(version).appendingPathComponent("darwin-arm64") + try fileManager.createDirectory(at: platformDir, withIntermediateDirectories: true) + try "binary".write(to: platformDir.appendingPathComponent("programad-remote"), atomically: true, encoding: .utf8) + let versionDir = daemonBase.appendingPathComponent(version) + try fileManager.setAttributes( + [.modificationDate: baseDate.addingTimeInterval(offset)], + ofItemAtPath: versionDir.path + ) + versionDirectories[version] = versionDir + } + + // Sibling outside programad-remote/ that must never be touched, even though + // it shares the parent "bin" directory the prune script also lives under. + let decoySibling = home.appendingPathComponent(".programa/bin/decoy-outside-programad-remote") + try fileManager.createDirectory(at: decoySibling, withIntermediateDirectories: true) + + let script = WorkspaceRemoteSessionController.remoteDaemonPruneStaleVersionsScript(currentVersion: currentVersion) + + // Generated-artifact assertion: the script is the runtime behavior here, so + // assert the deletion is anchored to the literal expanded base directory and + // that no unanchored `rm -rf` on the base (or anything broader) exists. + XCTAssertTrue(script.contains(#"case "$programa_version_dir" in"#)) + XCTAssertTrue(script.contains(#""$programa_daemon_base"/*)"#)) + XCTAssertFalse(script.contains("rm -rf \"$programa_daemon_base\"")) + XCTAssertFalse(script.contains("rm -rf -- \"$programa_daemon_base\"\n")) + + let result = runProcess( + executablePath: "/usr/bin/env", + arguments: [ + "HOME=\(home.path)", + "/bin/sh", + "-c", + script, + ], + // See timeout comment in runRelayZshHistfile above. + timeout: 90 + ) + + XCTAssertFalse(result.timedOut, result.stderr) + XCTAssertEqual(result.status, 0, result.stderr) + + XCTAssertTrue( + fileManager.fileExists(atPath: try XCTUnwrap(versionDirectories[currentVersion]).path), + "current version directory must survive" + ) + XCTAssertTrue( + fileManager.fileExists(atPath: try XCTUnwrap(versionDirectories["0.4.100"]).path), + "most-recently-used other version directory must survive" + ) + XCTAssertFalse( + fileManager.fileExists(atPath: try XCTUnwrap(versionDirectories["0.4.99"]).path), + "stale version directory must be pruned" + ) + XCTAssertFalse( + fileManager.fileExists(atPath: try XCTUnwrap(versionDirectories["0.4.9"]).path), + "stale version directory must be pruned" + ) + XCTAssertTrue( + fileManager.fileExists(atPath: decoySibling.path), + "prune must never touch anything outside programad-remote/" + ) + } + func testRelayZshBootstrapUsesRealHomeHistoryByDefault() throws { let histfile = try runRelayZshHistfile { home in try ":\n".write(to: home.appendingPathComponent(".zshenv"), atomically: true, encoding: .utf8)