From 3dbc919d4e5ef6fda0cab073edb6706b4be1d3ba Mon Sep 17 00:00:00 2001 From: cmux reload-cloud Date: Sun, 12 Jul 2026 18:31:20 -0700 Subject: [PATCH 1/2] test(ios): cover stable terminal picker menu values --- .../TerminalPickerMenuValueTests.swift | 72 +++++++++++++++++++ 1 file changed, 72 insertions(+) create mode 100644 Packages/iOS/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/TerminalPickerMenuValueTests.swift diff --git a/Packages/iOS/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/TerminalPickerMenuValueTests.swift b/Packages/iOS/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/TerminalPickerMenuValueTests.swift new file mode 100644 index 000000000000..ccf30c3716f4 --- /dev/null +++ b/Packages/iOS/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/TerminalPickerMenuValueTests.swift @@ -0,0 +1,72 @@ +import CmuxMobileShellModel +import Testing +@testable import CmuxMobileShellUI + +@Suite struct TerminalPickerMenuValueTests { + @Test func previewChurnDoesNotChangeSeededMenuValueButMembershipDoes() { + let terminal = MobileTerminalPreview(id: "terminal-1", name: "Build") + let snapshotRows = [TerminalPickerMenuRow(terminal)] + let baseline = menuValue(liveTerminals: [terminal], snapshotRows: snapshotRows) + + var churnedTerminal = terminal + churnedTerminal.name = "Build output" + churnedTerminal.isFocused = true + churnedTerminal.viewportFit = MobileTerminalViewportFit( + effective: MobileTerminalViewportSize(columns: 80, rows: 24), + client: MobileTerminalViewportSize(columns: 100, rows: 30), + isCurrentClientLimiting: false + ) + let previewChurn = menuValue(liveTerminals: [churnedTerminal], snapshotRows: snapshotRows) + + let addedTerminal = MobileTerminalPreview(id: "terminal-2", name: "Tests") + let membershipRows = snapshotRows + [TerminalPickerMenuRow(addedTerminal)] + let membershipChange = menuValue( + liveTerminals: [churnedTerminal, addedTerminal], + snapshotRows: membershipRows + ) + + #expect(previewChurn == baseline) + #expect(membershipChange != baseline) + } + + @Test func selectionIsResolvedFromTheRowsDisplayedByTheMenu() { + let liveTerminals = [ + MobileTerminalPreview(id: "terminal-live", name: "Live") + ] + let snapshotRows = [ + TerminalPickerMenuRow(MobileTerminalPreview(id: "terminal-snapshot", name: "Snapshot")), + TerminalPickerMenuRow(MobileTerminalPreview(id: "terminal-selected", name: "Selected")), + ] + + let selected = menuValue( + liveTerminals: liveTerminals, + snapshotRows: snapshotRows, + selectedID: "terminal-selected" + ) + let staleSelection = menuValue( + liveTerminals: liveTerminals, + snapshotRows: snapshotRows, + selectedID: "terminal-live" + ) + + #expect(selected.selectedID == MobileTerminalPreview.ID(rawValue: "terminal-selected")) + #expect(selected.selectedName == "Selected") + #expect(staleSelection.selectedID == MobileTerminalPreview.ID(rawValue: "terminal-snapshot")) + #expect(staleSelection.selectedName == "Snapshot") + } + + private func menuValue( + liveTerminals: [MobileTerminalPreview], + snapshotRows: [TerminalPickerMenuRow], + selectedID: MobileTerminalPreview.ID? = "terminal-1" + ) -> TerminalPickerMenuValue { + TerminalPickerMenuValue( + liveTerminals: liveTerminals, + snapshotRows: snapshotRows, + selectedID: selectedID, + canCreateWorkspace: true, + hasActiveBrowser: false, + isChatMode: false + ) + } +} From 74daa96dcab2b8553815274b702dc916d7010ac0 Mon Sep 17 00:00:00 2001 From: cmux reload-cloud Date: Sun, 12 Jul 2026 18:44:16 -0700 Subject: [PATCH 2/2] fix(ios): isolate terminal picker from preview churn --- .../TerminalPickerMenu.swift | 114 ++++++++++++++++++ .../TerminalPickerMenuActions.swift | 12 ++ .../TerminalPickerMenuDiagnostics.swift | 41 +++++++ .../TerminalPickerMenuValue.swift | 30 +++++ .../WorkspaceDetailView+MenuState.swift | 12 ++ .../WorkspaceDetailView.swift | 112 ++++------------- .../TerminalPickerMenuValueTests.swift | 34 ++++-- 7 files changed, 259 insertions(+), 96 deletions(-) create mode 100644 Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalPickerMenu.swift create mode 100644 Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalPickerMenuActions.swift create mode 100644 Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalPickerMenuDiagnostics.swift create mode 100644 Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalPickerMenuValue.swift diff --git a/Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalPickerMenu.swift b/Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalPickerMenu.swift new file mode 100644 index 000000000000..b53aa71988d2 --- /dev/null +++ b/Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalPickerMenu.swift @@ -0,0 +1,114 @@ +import CmuxMobileSupport +import SwiftUI + +/// Snapshot-isolated native menu for switching the active workspace surface. +struct TerminalPickerMenu: View, Equatable { + let value: TerminalPickerMenuValue + let actions: TerminalPickerMenuActions + #if DEBUG + private let diagnostics = TerminalPickerMenuDiagnostics() + #endif + + nonisolated static func == (lhs: Self, rhs: Self) -> Bool { + lhs.value == rhs.value + } + + var body: some View { + Menu { + instrumentedMenuContent + } label: { + Label( + value.selectedName ?? L10n.string("mobile.terminal.select", defaultValue: "Terminal"), + systemImage: "rectangle.stack" + ) + .labelStyle(.iconOnly) + } + .foregroundStyle(TerminalPalette.foreground) + .accessibilityLabel(L10n.string("mobile.terminal.picker.title", defaultValue: "Terminals")) + .accessibilityIdentifier("MobileTerminalDropdown") + .accessibilityValue(value.selectedName ?? "") + } + + @ViewBuilder + private var instrumentedMenuContent: some View { + #if DEBUG + let _ = diagnostics.recordContentBuilderEvaluation(rowCount: value.rows.count) + #endif + menuContent + } + + @ViewBuilder + private var menuContent: some View { + Section(L10n.string("mobile.terminal.picker.title", defaultValue: "Terminals")) { + ForEach(value.rows) { terminal in + Button { + actions.selectTerminal(terminal.id) + } label: { + Label( + terminal.name, + systemImage: terminal.id == value.selectedID && !value.hasActiveBrowser + ? "checkmark.circle.fill" + : "terminal" + ) + } + .accessibilityIdentifier("MobileTerminalMenuItem-\(terminal.id.rawValue)") + } + } + + Section { + Button(action: actions.createWorkspace) { + Label( + L10n.string("mobile.workspace.new", defaultValue: "New Workspace"), + systemImage: "plus.square.on.square" + ) + } + .disabled(!value.canCreateWorkspace) + .accessibilityIdentifier("MobileNewWorkspaceMenuItem") + + Button(action: actions.createTerminal) { + Label(L10n.string("mobile.terminal.new", defaultValue: "New Terminal"), systemImage: "plus") + } + .accessibilityIdentifier("MobileNewTerminalMenuItem") + + Button(action: actions.openBrowser) { + Label( + L10n.string("mobile.browser.new", defaultValue: "New Browser"), + systemImage: value.hasActiveBrowser ? "checkmark.circle.fill" : "globe" + ) + } + .accessibilityIdentifier("MobileNewBrowserMenuItem") + } + + #if canImport(UIKit) + Section { + if !value.hasActiveBrowser && !value.isChatMode { + Button(action: actions.openTextSheet) { + Label( + L10n.string("mobile.terminal.viewAsText", defaultValue: "View as Text"), + systemImage: "doc.plaintext" + ) + } + .accessibilityIdentifier("MobileViewAsTextMenuItem") + } + + #if DEBUG + Button(action: actions.copyDebugLogs) { + Label( + L10n.string("mobile.debug.copyLogs", defaultValue: "Copy Debug Logs"), + systemImage: "doc.on.clipboard" + ) + } + .accessibilityIdentifier("MobileCopyDebugLogsMenuItem") + #endif + + Button(action: actions.sendFeedback) { + Label( + L10n.string("mobile.feedback.send", defaultValue: "Send Feedback"), + systemImage: "paperplane" + ) + } + .accessibilityIdentifier("MobileSendFeedbackMenuItem") + } + #endif + } +} diff --git a/Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalPickerMenuActions.swift b/Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalPickerMenuActions.swift new file mode 100644 index 000000000000..d067993e9e70 --- /dev/null +++ b/Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalPickerMenuActions.swift @@ -0,0 +1,12 @@ +import CmuxMobileShellModel + +/// User actions emitted by ``TerminalPickerMenu`` without exposing mutable stores to its row subtree. +struct TerminalPickerMenuActions { + let selectTerminal: (MobileTerminalPreview.ID) -> Void + let createWorkspace: () -> Void + let createTerminal: () -> Void + let openBrowser: () -> Void + let openTextSheet: () -> Void + let copyDebugLogs: () -> Void + let sendFeedback: () -> Void +} diff --git a/Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalPickerMenuDiagnostics.swift b/Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalPickerMenuDiagnostics.swift new file mode 100644 index 000000000000..655f19595ce6 --- /dev/null +++ b/Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalPickerMenuDiagnostics.swift @@ -0,0 +1,41 @@ +#if DEBUG +import Foundation +import OSLog + +/// DEBUG-only events for counting terminal-picker menu evaluation and snapshot writes. +struct TerminalPickerMenuDiagnostics { + private let logger = Logger( + subsystem: Bundle.main.bundleIdentifier ?? "com.cmuxterm.app", + category: "TerminalPickerMenu" + ) + private let signpostLog = OSLog( + subsystem: Bundle.main.bundleIdentifier ?? "com.cmuxterm.app", + category: "TerminalPickerMenu" + ) + + func recordContentBuilderEvaluation(rowCount: Int) { + logger.debug("content-builder evaluated rows=\(rowCount, privacy: .public)") + os_signpost( + .event, + log: signpostLog, + name: "ContentBuilderEvaluation", + "rows=%{public}d", + rowCount + ) + } + + func recordRowsWrite(rowCount: Int, includesTitleChanges: Bool) { + logger.debug( + "snapshot rows write rows=\(rowCount, privacy: .public) includeTitles=\(includesTitleChanges, privacy: .public)" + ) + os_signpost( + .event, + log: signpostLog, + name: "SnapshotRowsWrite", + "rows=%{public}d includeTitles=%{public}d", + rowCount, + includesTitleChanges ? 1 : 0 + ) + } +} +#endif diff --git a/Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalPickerMenuValue.swift b/Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalPickerMenuValue.swift new file mode 100644 index 000000000000..caf7497ad87b --- /dev/null +++ b/Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalPickerMenuValue.swift @@ -0,0 +1,30 @@ +import CmuxMobileShellModel + +/// Immutable state that determines the native terminal picker's presented menu. +struct TerminalPickerMenuValue: Equatable { + let rows: [TerminalPickerMenuRow] + let selectedID: MobileTerminalPreview.ID? + let selectedName: String? + let canCreateWorkspace: Bool + let hasActiveBrowser: Bool + let isChatMode: Bool + + init( + liveTerminals: [MobileTerminalPreview], + snapshotRows: [TerminalPickerMenuRow], + selectedID: MobileTerminalPreview.ID?, + canCreateWorkspace: Bool, + hasActiveBrowser: Bool, + isChatMode: Bool + ) { + rows = snapshotRows.isEmpty + ? liveTerminals.map(TerminalPickerMenuRow.init) + : snapshotRows + let selection = rows.resolvedTerminalPickerSelection(selectedID: selectedID) + self.selectedID = selection?.id + selectedName = selection?.name + self.canCreateWorkspace = canCreateWorkspace + self.hasActiveBrowser = hasActiveBrowser + self.isChatMode = isChatMode + } +} diff --git a/Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView+MenuState.swift b/Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView+MenuState.swift index 6db4c5e18120..beea8b180b0e 100644 --- a/Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView+MenuState.swift +++ b/Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView+MenuState.swift @@ -45,12 +45,24 @@ extension WorkspaceDetailView { let rows = terminalPickerLiveRows if includeTitleChanges { guard terminalPickerRows != rows else { return } + #if DEBUG + TerminalPickerMenuDiagnostics().recordRowsWrite( + rowCount: rows.count, + includesTitleChanges: true + ) + #endif terminalPickerRows = rows return } guard terminalPickerRows.isEmpty || TerminalPickerMenuMembership(terminalPickerRows) != TerminalPickerMenuMembership(rows) else { return } + #if DEBUG + TerminalPickerMenuDiagnostics().recordRowsWrite( + rowCount: rows.count, + includesTitleChanges: false + ) + #endif terminalPickerRows = rows } diff --git a/Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView.swift b/Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView.swift index 1d808f07ba82..c435c22c2023 100644 --- a/Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView.swift +++ b/Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceDetailView.swift @@ -350,101 +350,35 @@ struct WorkspaceDetailView: View { // Native menu keeps press-drag-release selection and routes through // `selectTerminalFromPicker`; keyboard-dismiss-on-open is unavailable. var terminalPickerToolbarButton: some View { - let rows = terminalPickerRows.isEmpty ? terminalPickerLiveRows : terminalPickerRows - let selection = terminalPickerLiveRows.resolvedTerminalPickerSelection(selectedID: store.selectedTerminalID) - - return Menu { - terminalPickerMenuContent(rows: rows, selectedID: selection?.id) - } label: { - Label( - selection?.name ?? L10n.string("mobile.terminal.select", defaultValue: "Terminal"), - systemImage: "rectangle.stack" + TerminalPickerMenu( + value: TerminalPickerMenuValue( + liveTerminals: workspace.terminals, + snapshotRows: terminalPickerRows, + selectedID: store.selectedTerminalID, + canCreateWorkspace: canCreateWorkspace, + hasActiveBrowser: activeBrowser != nil, + isChatMode: isChatMode + ), + actions: TerminalPickerMenuActions( + selectTerminal: selectTerminalFromPicker, + createWorkspace: createWorkspaceFromToolbar, + createTerminal: createTerminalFromToolbar, + openBrowser: openBrowserFromToolbar, + openTextSheet: openTextSheetFromMenu, + copyDebugLogs: { + #if DEBUG + copyDebugLogsFromMenu() + #endif + }, + sendFeedback: openFeedbackComposerFromMenu ) - .labelStyle(.iconOnly) - } - .foregroundStyle(TerminalPalette.foreground) - .accessibilityLabel(L10n.string("mobile.terminal.picker.title", defaultValue: "Terminals")) - .accessibilityIdentifier("MobileTerminalDropdown") - .accessibilityValue(selection?.name ?? "") + ) + .equatable() .simultaneousGesture(TapGesture().onEnded { syncTerminalPickerRows(includeTitleChanges: true) }) .onAppear { syncTerminalPickerRows(includeTitleChanges: true) } .onChange(of: terminalPickerLiveMembership) { _, _ in syncTerminalPickerRows() } } - @ViewBuilder - private func terminalPickerMenuContent( - rows: [TerminalPickerMenuRow], - selectedID: MobileTerminalPreview.ID? - ) -> some View { - Section(L10n.string("mobile.terminal.picker.title", defaultValue: "Terminals")) { - ForEach(rows) { terminal in - Button { - selectTerminalFromPicker(terminal.id) - } label: { - Label( - terminal.name, - systemImage: terminal.id == selectedID && activeBrowser == nil - ? "checkmark.circle.fill" - : "terminal" - ) - } - .accessibilityIdentifier("MobileTerminalMenuItem-\(terminal.id.rawValue)") - } - } - - Section { - Button(action: createWorkspaceFromToolbar) { - Label(L10n.string("mobile.workspace.new", defaultValue: "New Workspace"), systemImage: "plus.square.on.square") - } - .disabled(!canCreateWorkspace) - .accessibilityIdentifier("MobileNewWorkspaceMenuItem") - - Button(action: createTerminalFromToolbar) { - Label(L10n.string("mobile.terminal.new", defaultValue: "New Terminal"), systemImage: "plus") - } - .accessibilityIdentifier("MobileNewTerminalMenuItem") - - Button(action: openBrowserFromToolbar) { - Label( - L10n.string("mobile.browser.new", defaultValue: "New Browser"), - systemImage: activeBrowser == nil ? "globe" : "checkmark.circle.fill" - ) - } - .accessibilityIdentifier("MobileNewBrowserMenuItem") - } - - #if canImport(UIKit) - Section { - // Only while the terminal pane is showing: browser and chat modes - // do not mount a terminal surface for text capture. - if activeBrowser == nil && !isChatMode { - Button(action: openTextSheetFromMenu) { - Label( - L10n.string("mobile.terminal.viewAsText", defaultValue: "View as Text"), - systemImage: "doc.plaintext" - ) - } - .accessibilityIdentifier("MobileViewAsTextMenuItem") - } - - #if DEBUG - Button(action: copyDebugLogsFromMenu) { - Label(L10n.string("mobile.debug.copyLogs", defaultValue: "Copy Debug Logs"), systemImage: "doc.on.clipboard") - } - .accessibilityIdentifier("MobileCopyDebugLogsMenuItem") - #endif - - Button(action: openFeedbackComposerFromMenu) { - Label( - L10n.string("mobile.feedback.send", defaultValue: "Send Feedback"), - systemImage: "paperplane" - ) - } - .accessibilityIdentifier("MobileSendFeedbackMenuItem") - } - #endif - } - #if canImport(UIKit) #if DEBUG private func copyDebugLogsFromMenu() { diff --git a/Packages/iOS/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/TerminalPickerMenuValueTests.swift b/Packages/iOS/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/TerminalPickerMenuValueTests.swift index ccf30c3716f4..518085bed429 100644 --- a/Packages/iOS/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/TerminalPickerMenuValueTests.swift +++ b/Packages/iOS/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/TerminalPickerMenuValueTests.swift @@ -8,24 +8,27 @@ import Testing let snapshotRows = [TerminalPickerMenuRow(terminal)] let baseline = menuValue(liveTerminals: [terminal], snapshotRows: snapshotRows) - var churnedTerminal = terminal - churnedTerminal.name = "Build output" - churnedTerminal.isFocused = true - churnedTerminal.viewportFit = MobileTerminalViewportFit( + var titleOnlyTerminal = terminal + titleOnlyTerminal.name = "Build output" + let titleOnlyChange = menuValue(liveTerminals: [titleOnlyTerminal], snapshotRows: snapshotRows) + + var viewportOnlyTerminal = terminal + viewportOnlyTerminal.viewportFit = MobileTerminalViewportFit( effective: MobileTerminalViewportSize(columns: 80, rows: 24), client: MobileTerminalViewportSize(columns: 100, rows: 30), isCurrentClientLimiting: false ) - let previewChurn = menuValue(liveTerminals: [churnedTerminal], snapshotRows: snapshotRows) + let viewportOnlyChange = menuValue(liveTerminals: [viewportOnlyTerminal], snapshotRows: snapshotRows) let addedTerminal = MobileTerminalPreview(id: "terminal-2", name: "Tests") let membershipRows = snapshotRows + [TerminalPickerMenuRow(addedTerminal)] let membershipChange = menuValue( - liveTerminals: [churnedTerminal, addedTerminal], + liveTerminals: [viewportOnlyTerminal, addedTerminal], snapshotRows: membershipRows ) - #expect(previewChurn == baseline) + #expect(titleOnlyChange == baseline) + #expect(viewportOnlyChange == baseline) #expect(membershipChange != baseline) } @@ -55,6 +58,23 @@ import Testing #expect(staleSelection.selectedName == "Snapshot") } + @Test func emptySnapshotUsesLiveRowsAndHandlesNoTerminals() { + let liveTerminal = MobileTerminalPreview(id: "terminal-live", name: "Live") + let firstOpen = menuValue( + liveTerminals: [liveTerminal], + snapshotRows: [], + selectedID: "missing" + ) + let noTerminals = menuValue(liveTerminals: [], snapshotRows: [], selectedID: "missing") + + #expect(firstOpen.rows == [TerminalPickerMenuRow(liveTerminal)]) + #expect(firstOpen.selectedID == liveTerminal.id) + #expect(firstOpen.selectedName == liveTerminal.name) + #expect(noTerminals.rows.isEmpty) + #expect(noTerminals.selectedID == nil) + #expect(noTerminals.selectedName == nil) + } + private func menuValue( liveTerminals: [MobileTerminalPreview], snapshotRows: [TerminalPickerMenuRow],