diff --git a/.github/workflows/ci-macos-compat.yml b/.github/workflows/ci-macos-compat.yml index 4bcab6b0..463e5a56 100644 --- a/.github/workflows/ci-macos-compat.yml +++ b/.github/workflows/ci-macos-compat.yml @@ -94,6 +94,9 @@ jobs: sleep $((attempt * 5)) done + - name: Run issue #952 regression guard + run: python3 tests/test_issue_952_socket_listener_recovery.py + - name: Run unit tests run: | set -euo pipefail diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6bac7a85..c106a18a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -122,6 +122,9 @@ jobs: sleep $((attempt * 5)) done + - name: Run issue #952 regression guard + run: python3 tests/test_issue_952_socket_listener_recovery.py + - name: Run unit tests run: | set -euo pipefail @@ -244,6 +247,9 @@ jobs: sleep $((attempt * 5)) done + - name: Run issue #952 regression guard + run: python3 tests/test_issue_952_socket_listener_recovery.py + - name: Create virtual display run: | set -euo pipefail diff --git a/.github/workflows/test-depot.yml b/.github/workflows/test-depot.yml index ca636bf6..867479ad 100644 --- a/.github/workflows/test-depot.yml +++ b/.github/workflows/test-depot.yml @@ -122,6 +122,9 @@ jobs: sleep $((attempt * 5)) done + - name: Run issue #952 regression guard + run: python3 tests/test_issue_952_socket_listener_recovery.py + - name: Run unit tests if: ${{ !inputs.skip_unit_tests }} run: | diff --git a/CLAUDE.md b/CLAUDE.md index 18f06112..098b77ea 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -34,10 +34,16 @@ When reporting a tagged reload result in chat, use the format for your agent typ Never use `/tmp/cmux-/...` app links in chat output. If the expected DerivedData path is missing, resolve the real `.app` path and report that `file://` URL. -After making code changes, always run the build: +After making code changes, always use `reload.sh --tag` to build and launch. **Never run bare `xcodebuild` or `open` an untagged `cmux DEV.app`.** Untagged builds share the default debug socket and bundle ID with other agents, causing conflicts and stealing focus. ```bash -xcodebuild -project GhosttyTabs.xcodeproj -scheme cmux -configuration Debug -destination 'platform=macOS' build +./scripts/reload.sh --tag +``` + +If you only need to verify the build compiles (no launch), use a tagged derivedDataPath: + +```bash +xcodebuild -project GhosttyTabs.xcodeproj -scheme cmux -configuration Debug -destination 'platform=macOS' -derivedDataPath /tmp/cmux- build ``` When rebuilding GhosttyKit.xcframework, always use Release optimizations: @@ -115,6 +121,12 @@ tail -f "$(cat /tmp/cmux-last-debug-log-path 2>/dev/null || echo /tmp/cmux-debug - **Submodule safety:** When modifying a submodule (ghostty, vendor/bonsplit, etc.), always push the submodule commit to its remote `main` branch BEFORE committing the updated pointer in the parent repo. Never commit on a detached HEAD or temporary branch — the commit will be orphaned and lost. Verify with: `cd && git merge-base --is-ancestor HEAD origin/main`. - **All user-facing strings must be localized.** Use `String(localized: "key.name", defaultValue: "English text")` for every string shown in the UI (labels, buttons, menus, dialogs, tooltips, error messages). Keys go in `Resources/Localizable.xcstrings` with translations for all supported languages (currently English and Japanese). Never use bare string literals in SwiftUI `Text()`, `Button()`, alert titles, etc. +## Test quality policy + +- Do not add tests that only verify source code text, method signatures, AST fragments, or grep-style patterns. +- Tests must verify observable runtime behavior through executable paths (unit/integration/e2e/CLI), not implementation shape. +- If a behavior cannot be exercised end-to-end yet, add a small runtime seam or harness first, then test through that seam. + ## Socket command threading policy - Do not use `DispatchQueue.main.sync` for high-frequency socket telemetry commands (`report_*`, `ports_kick`, status/progress/log metadata updates). @@ -131,21 +143,14 @@ tail -f "$(cat /tmp/cmux-last-debug-log-path 2>/dev/null || echo /tmp/cmux-debug - Only explicit focus-intent commands may mutate in-app focus/selection (`window.focus`, `workspace.select/next/previous/last`, `surface.focus`, `pane.focus/last`, browser focus commands, and v1 focus equivalents). - All non-focus commands should preserve current user focus context while still applying data/model changes. -## E2E mac UI tests +## Testing policy -Run UI tests on the UTM macOS VM (never on the host machine). Always run e2e UI tests via `ssh cmux-vm`: +**Never run tests locally.** All tests (E2E, UI, python socket tests) run via GitHub Actions or on the VM. -```bash -ssh cmux-vm 'cd /Users/cmux/GhosttyTabs && xcodebuild -project GhosttyTabs.xcodeproj -scheme cmux -configuration Debug -destination "platform=macOS" -only-testing:cmuxUITests/UpdatePillUITests test' -``` - -## Basic tests - -Run basic automated tests on the UTM macOS VM (never on the host machine): - -```bash -ssh cmux-vm 'cd /Users/cmux/GhosttyTabs && xcodebuild -project GhosttyTabs.xcodeproj -scheme cmux -configuration Debug -destination "platform=macOS" build && pkill -x "cmux DEV" || true && APP=$(find /Users/cmux/Library/Developer/Xcode/DerivedData -path "*/Build/Products/Debug/cmux DEV.app" -print -quit) && open "$APP" --env CMUX_SOCKET_MODE=allowAll && for i in {1..20}; do [ -S /tmp/cmux-debug.sock ] && break; sleep 0.5; done && python3 tests/test_update_timing.py && python3 tests/test_signals_auto.py && python3 tests/test_ctrl_socket.py && python3 tests/test_notifications.py' -``` +- **E2E / UI tests:** trigger via `gh workflow run test-e2e.yml` (see cmuxterm-hq CLAUDE.md for details) +- **Unit tests:** `xcodebuild -scheme cmux-unit` is safe (no app launch), but prefer CI +- **Python socket tests (tests_v2/):** these connect to a running cmux instance's socket. Never launch an untagged `cmux DEV.app` to run them. If you must test locally, use a tagged build's socket (`/tmp/cmux-debug-.sock`) with `CMUX_SOCKET=/tmp/cmux-debug-.sock` +- **Never `open` an untagged `cmux DEV.app`** from DerivedData. It conflicts with the user's running debug instance. ## Ghostty submodule workflow diff --git a/CLI/cmux.swift b/CLI/cmux.swift index e9f8d40b..19837c3f 100644 --- a/CLI/cmux.swift +++ b/CLI/cmux.swift @@ -1736,6 +1736,45 @@ struct CMUXCLI { return (cwd as NSString).appendingPathComponent(expanded) } + private func sanitizedFilenameComponent(_ raw: String) -> String { + let sanitized = raw.replacingOccurrences( + of: #"[^\p{L}\p{N}._-]+"#, + with: "-", + options: .regularExpression + ) + let trimmed = sanitized.trimmingCharacters(in: CharacterSet(charactersIn: "-.")) + return trimmed.isEmpty ? "item" : trimmed + } + + private func bestEffortPruneTemporaryFiles( + in directoryURL: URL, + keepingMostRecent maxCount: Int = 50, + maxAge: TimeInterval = 24 * 60 * 60 + ) { + guard let entries = try? FileManager.default.contentsOfDirectory( + at: directoryURL, + includingPropertiesForKeys: [.isRegularFileKey, .contentModificationDateKey, .creationDateKey], + options: [.skipsHiddenFiles] + ) else { + return + } + + let now = Date() + let datedEntries = entries.compactMap { url -> (url: URL, date: Date)? in + guard let values = try? url.resourceValues(forKeys: [.isRegularFileKey, .contentModificationDateKey, .creationDateKey]), + values.isRegularFile == true else { + return nil + } + return (url, values.contentModificationDate ?? values.creationDate ?? .distantPast) + }.sorted { $0.date > $1.date } + + for (index, entry) in datedEntries.enumerated() { + if index >= maxCount || now.timeIntervalSince(entry.date) > maxAge { + try? FileManager.default.removeItem(at: entry.url) + } + } + } + // MARK: - Markdown Commands private func runMarkdownCommand( @@ -2650,6 +2689,29 @@ struct CMUXCLI { } } + func displaySnapshotText(_ payload: [String: Any]) -> String { + let snapshotText = (payload["snapshot"] as? String) ?? "Empty page" + guard snapshotText.contains("\n- (empty)") else { + return snapshotText + } + + let url = ((payload["url"] as? String) ?? "").trimmingCharacters(in: .whitespacesAndNewlines) + let readyState = ((payload["ready_state"] as? String) ?? "").trimmingCharacters(in: .whitespacesAndNewlines) + var lines = [snapshotText] + + if !url.isEmpty { + lines.append("url: \(url)") + } + if !readyState.isEmpty { + lines.append("ready_state: \(readyState)") + } + if url.isEmpty || url == "about:blank" { + lines.append("hint: run 'cmux browser get url' to verify navigation") + } + + return lines.joined(separator: "\n") + } + func displayBrowserValue(_ value: Any) -> String { if let dict = value as? [String: Any], let type = dict["__cmux_t"] as? String, @@ -2868,10 +2930,8 @@ struct CMUXCLI { let payload = try client.sendV2(method: "browser.snapshot", params: params) if effectiveJSONOutput { print(jsonString(formatIDs(payload, mode: effectiveIDFormat))) - } else if let text = payload["snapshot"] as? String { - print(text) } else { - print("Empty page") + print(displaySnapshotText(payload)) } return } @@ -3079,17 +3139,139 @@ struct CMUXCLI { if subcommand == "screenshot" { let sid = try requireSurface() let (outPathOpt, _) = parseOption(subArgs, name: "--out") - let payload = try client.sendV2(method: "browser.screenshot", params: ["surface_id": sid]) - if let outPathOpt, - let b64 = payload["png_base64"] as? String, - let data = Data(base64Encoded: b64) { - try data.write(to: URL(fileURLWithPath: outPathOpt)) + let localJSONOutput = hasFlag(subArgs, name: "--json") + let outputAsJSON = effectiveJSONOutput || localJSONOutput + var payload = try client.sendV2(method: "browser.screenshot", params: ["surface_id": sid]) + + func fileURL(fromPath rawPath: String) -> URL { + let resolvedPath = resolvePath(rawPath) + return URL(fileURLWithPath: resolvedPath).standardizedFileURL } - if effectiveJSONOutput { - print(jsonString(formatIDs(payload, mode: effectiveIDFormat))) + func writeScreenshot(_ data: Data, to destinationURL: URL) throws { + try FileManager.default.createDirectory( + at: destinationURL.deletingLastPathComponent(), + withIntermediateDirectories: true + ) + try data.write(to: destinationURL, options: .atomic) + } + + func hasText(_ value: String?) -> Bool { + guard let value else { return false } + return !value.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty + } + + var screenshotPath = payload["path"] as? String + var screenshotURL = payload["url"] as? String + + func syncScreenshotLocationFields() { + if !hasText(screenshotPath), + let rawURL = screenshotURL, + let fileURL = URL(string: rawURL), + fileURL.isFileURL, + !fileURL.path.isEmpty { + screenshotPath = fileURL.path + } + if !hasText(screenshotURL), + let screenshotPath, + hasText(screenshotPath) { + screenshotURL = URL(fileURLWithPath: screenshotPath).standardizedFileURL.absoluteString + } + if let screenshotPath, hasText(screenshotPath) { + payload["path"] = screenshotPath + } + if let screenshotURL, hasText(screenshotURL) { + payload["url"] = screenshotURL + } + } + + func persistPayloadScreenshot(to destinationURL: URL, allowFailure: Bool) throws -> Bool { + if let sourcePath = screenshotPath, hasText(sourcePath) { + let sourceURL = URL(fileURLWithPath: sourcePath).standardizedFileURL + do { + if sourceURL.path != destinationURL.path { + try FileManager.default.createDirectory( + at: destinationURL.deletingLastPathComponent(), + withIntermediateDirectories: true + ) + try? FileManager.default.removeItem(at: destinationURL) + try FileManager.default.copyItem(at: sourceURL, to: destinationURL) + } + return true + } catch { + if payload["png_base64"] == nil { + if allowFailure { + return false + } + throw error + } + } + } + + if let b64 = payload["png_base64"] as? String, + let data = Data(base64Encoded: b64) { + do { + try writeScreenshot(data, to: destinationURL) + return true + } catch { + if allowFailure { + return false + } + throw error + } + } + + return false + } + + if let outPathOpt { + let outputURL = fileURL(fromPath: outPathOpt) + guard try persistPayloadScreenshot(to: outputURL, allowFailure: false) else { + throw CLIError(message: "browser screenshot missing image data") + } + screenshotPath = outputURL.path + screenshotURL = outputURL.absoluteString + payload["path"] = screenshotPath + payload["url"] = screenshotURL + } else { + syncScreenshotLocationFields() + if !hasText(screenshotPath) && !hasText(screenshotURL) { + let outputDir = FileManager.default.temporaryDirectory + .appendingPathComponent("cmux-browser-screenshots-cli", isDirectory: true) + if (try? FileManager.default.createDirectory(at: outputDir, withIntermediateDirectories: true)) != nil { + bestEffortPruneTemporaryFiles(in: outputDir) + let timestampMs = Int(Date().timeIntervalSince1970 * 1000) + let safeSid = sanitizedFilenameComponent(sid) + let filename = "surface-\(safeSid)-\(timestampMs)-\(String(UUID().uuidString.prefix(8))).png" + let outputURL = outputDir.appendingPathComponent(filename, isDirectory: false) + if (try? persistPayloadScreenshot(to: outputURL, allowFailure: true)) == true { + screenshotPath = outputURL.path + screenshotURL = outputURL.absoluteString + payload["path"] = screenshotPath + payload["url"] = screenshotURL + } + } + } + } + + if outputAsJSON { + let formattedPayload = formatIDs(payload, mode: effectiveIDFormat) + if var outputPayload = formattedPayload as? [String: Any] { + if hasText(screenshotPath) || hasText(screenshotURL) { + outputPayload.removeValue(forKey: "png_base64") + } + print(jsonString(outputPayload)) + } else { + print(jsonString(formattedPayload)) + } } else if let outPathOpt { print("OK \(outPathOpt)") + } else if let screenshotURL, + hasText(screenshotURL) { + print("OK \(screenshotURL)") + } else if let screenshotPath, + hasText(screenshotPath) { + print("OK \(screenshotPath)") } else { print("OK") } @@ -5538,8 +5720,10 @@ struct CMUXCLI { } private func jsonString(_ object: Any) -> String { + var options: JSONSerialization.WritingOptions = [.prettyPrinted] + options.insert(.withoutEscapingSlashes) guard JSONSerialization.isValidJSONObject(object), - let data = try? JSONSerialization.data(withJSONObject: object, options: [.prettyPrinted]), + let data = try? JSONSerialization.data(withJSONObject: object, options: options), let output = String(data: data, encoding: .utf8) else { return "{}" } @@ -6824,6 +7008,7 @@ struct CMUXCLI { browser press|keydown|keyup [--snapshot-after] browser select [--snapshot-after] browser scroll [--selector ] [--dx ] [--dy ] [--snapshot-after] + browser screenshot [--out ] [--json] browser get [...] browser is browser find ... diff --git a/Sources/AppDelegate.swift b/Sources/AppDelegate.swift index 91625919..b015a5ea 100644 --- a/Sources/AppDelegate.swift +++ b/Sources/AppDelegate.swift @@ -1005,14 +1005,31 @@ func shouldDispatchBrowserReturnViaFirstResponderKeyDown( func shouldToggleMainWindowFullScreenForCommandControlFShortcut( flags: NSEvent.ModifierFlags, chars: String, - keyCode: UInt16 + keyCode: UInt16, + layoutCharacterProvider: (UInt16, NSEvent.ModifierFlags) -> String? = KeyboardLayout.character(forKeyCode:modifierFlags:) ) -> Bool { let normalizedFlags = flags .intersection(.deviceIndependentFlagsMask) .subtracting([.numericPad, .function, .capsLock]) guard normalizedFlags == [.command, .control] else { return false } let normalizedChars = chars.lowercased() - return normalizedChars == "f" || keyCode == 3 + if normalizedChars == "f" { + return true + } + let charsAreControlSequence = !normalizedChars.isEmpty + && normalizedChars.unicodeScalars.allSatisfy { CharacterSet.controlCharacters.contains($0) } + if !normalizedChars.isEmpty && !charsAreControlSequence { + return false + } + + // Fallback to layout translation only when characters are unavailable (for + // synthetic/key-equivalent paths that can report an empty string). + if let translatedCharacter = layoutCharacterProvider(keyCode, flags), !translatedCharacter.isEmpty { + return translatedCharacter == "f" + } + + // Keep ANSI fallback as a final safety net when layout translation is unavailable. + return keyCode == 3 } func commandPaletteSelectionDeltaForKeyboardNavigation( @@ -1449,6 +1466,7 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent weak var sidebarState: SidebarState? weak var fullscreenControlsViewModel: TitlebarControlsViewModel? weak var sidebarSelectionState: SidebarSelectionState? + var shortcutLayoutCharacterProvider: (UInt16, NSEvent.ModifierFlags) -> String? = KeyboardLayout.character(forKeyCode:modifierFlags:) private var workspaceObserver: NSObjectProtocol? private var lifecycleSnapshotObservers: [NSObjectProtocol] = [] private var windowKeyObserver: NSObjectProtocol? @@ -1559,7 +1577,8 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent private var isApplyingStartupSessionRestore = false private var sessionAutosaveTimer: DispatchSourceTimer? private var socketListenerHealthTimer: DispatchSourceTimer? - private static let socketListenerHealthCheckInterval: DispatchTimeInterval = .seconds(5) + private var socketListenerHealthCheckInFlight = false + private static let socketListenerHealthCheckInterval: DispatchTimeInterval = .seconds(2) private var lastSocketListenerUnhealthyCaptureAt: Date = .distantPast private static let socketListenerUnhealthyCaptureCooldown: TimeInterval = 60 private let sessionPersistenceQueue = DispatchQueue( @@ -1819,7 +1838,6 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent let tab = tabManager.tabs.first(where: { $0.id == tabId }) { tab.triggerNotificationFocusFlash(panelId: surfaceId, requiresSplit: false, shouldFocus: false) } - notificationStore.markRead(forTabId: tabId, surfaceId: surfaceId) } func applicationShouldTerminate(_ sender: NSApplication) -> NSApplication.TerminateReply { @@ -2491,25 +2509,57 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent private func stopSocketListenerHealthMonitor() { socketListenerHealthTimer?.cancel() socketListenerHealthTimer = nil + socketListenerHealthCheckInFlight = false } private func restartSocketListenerIfNeededForHealthCheck(source: String) { - guard let config = socketListenerConfigurationIfEnabled() else { return } - let health = TerminalController.shared.socketListenerHealth(expectedSocketPath: config.path) + guard !socketListenerHealthCheckInFlight, + let config = socketListenerConfigurationIfEnabled() else { return } + let expectedSocketPath = config.path + let terminalController = TerminalController.shared + socketListenerHealthCheckInFlight = true + Thread.detachNewThread { [weak self, expectedSocketPath, source, terminalController] in + let health = terminalController.socketListenerHealth(expectedSocketPath: expectedSocketPath) + Task { @MainActor [weak self, health] in + guard let self else { return } + self.socketListenerHealthCheckInFlight = false + self.handleSocketListenerHealthCheckResult( + health, + source: source, + expectedSocketPath: expectedSocketPath + ) + } + } + } + + private func handleSocketListenerHealthCheckResult( + _ health: TerminalController.SocketListenerHealth, + source: String, + expectedSocketPath: String + ) { + guard let config = socketListenerConfigurationIfEnabled(), + config.path == expectedSocketPath else { return } guard !health.isHealthy else { lastSocketListenerUnhealthyCaptureAt = .distantPast return } let failureSignals = health.failureSignals - let data: [String: Any] = [ + var data: [String: Any] = [ "source": source, "path": config.path, "isRunning": health.isRunning ? 1 : 0, "acceptLoopAlive": health.acceptLoopAlive ? 1 : 0, "socketPathMatches": health.socketPathMatches ? 1 : 0, "socketPathExists": health.socketPathExists ? 1 : 0, + "socketProbePerformed": health.socketProbePerformed ? 1 : 0, "failureSignals": failureSignals ] + if let socketConnectable = health.socketConnectable { + data["socketConnectable"] = socketConnectable ? 1 : 0 + } + if let socketConnectErrno = health.socketConnectErrno { + data["socketConnectErrno"] = Int(socketConnectErrno) + } sentryBreadcrumb("socket.listener.unhealthy", category: "socket", data: data) let now = Date() if now.timeIntervalSince(lastSocketListenerUnhealthyCaptureAt) >= Self.socketListenerUnhealthyCaptureCooldown { @@ -5840,6 +5890,11 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent titlebarAccessoryController.toggleNotificationsPopover(animated: animated, anchorView: anchorView) } + @discardableResult + func dismissNotificationsPopoverIfShown() -> Bool { + titlebarAccessoryController.dismissNotificationsPopoverIfShown() + } + func jumpToLatestUnread() { guard let notificationStore else { return } #if DEBUG @@ -6151,7 +6206,7 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent private func handleCustomShortcut(event: NSEvent) -> Bool { // `charactersIgnoringModifiers` can be nil for some synthetic NSEvents and certain special keys. - // Most shortcuts below use keyCode fallbacks, so treat nil as "" rather than bailing out. + // Treat nil as "" and rely on keyCode/layout-aware fallback logic where needed. let chars = (event.charactersIgnoringModifiers ?? "").lowercased() let flags = event.modifierFlags.intersection(.deviceIndependentFlagsMask) let hasControl = flags.contains(.control) @@ -6188,7 +6243,10 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent // Special-case: Cmd+D should confirm destructive close on alerts. // XCUITest key events often hit the app-level local monitor first, so forward the key // equivalent to the alert panel explicitly. - if flags == [.command], chars == "d", + if matchShortcut( + event: event, + shortcut: StoredShortcut(key: "d", command: true, shift: false, option: false, control: false) + ), let root = closeConfirmationPanel.contentView, let closeButton = findButton(in: root, titled: "Close") { closeButton.performClick(nil) @@ -6363,15 +6421,20 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent // focused omnibar in another window does not suppress Cmd+P here. let hasFocusedAddressBarInShortcutContext = focusedBrowserAddressBarPanelIdForShortcutEvent(event) != nil let isCommandP = !hasFocusedAddressBarInShortcutContext - && normalizedFlags == [.command] - && (chars == "p" || event.keyCode == 35) + && matchShortcut( + event: event, + shortcut: StoredShortcut(key: "p", command: true, shift: false, option: false, control: false) + ) if isCommandP { let targetWindow = commandPaletteTargetWindow ?? event.window ?? NSApp.keyWindow ?? NSApp.mainWindow requestCommandPaletteSwitcher(preferredWindow: targetWindow, source: "shortcut.cmdP") return true } - let isCommandShiftP = normalizedFlags == [.command, .shift] && (chars == "p" || event.keyCode == 35) + let isCommandShiftP = matchShortcut( + event: event, + shortcut: StoredShortcut(key: "p", command: true, shift: true, option: false, control: false) + ) if isCommandShiftP { let targetWindow = commandPaletteTargetWindow ?? event.window ?? NSApp.keyWindow ?? NSApp.mainWindow requestCommandPaletteCommands(preferredWindow: targetWindow, source: "shortcut.cmdShiftP") @@ -6387,11 +6450,16 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent return true } - if normalizedFlags == [.command], chars == "q" { + if matchShortcut( + event: event, + shortcut: StoredShortcut(key: "q", command: true, shift: false, option: false, control: false) + ) { return handleQuitShortcutWarning() } - if normalizedFlags == [.command, .shift], - (chars == "," || chars == "<" || event.keyCode == 43) { + if matchShortcut( + event: event, + shortcut: StoredShortcut(key: ",", command: true, shift: true, option: false, control: false) + ) { GhosttyApp.shared.reloadConfiguration(source: "shortcut.cmd_shift_comma") return true } @@ -6611,7 +6679,10 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent ) } - if normalizedFlags == [.command, .option], (chars == "t" || event.keyCode == 17) { + if matchShortcut( + event: event, + shortcut: StoredShortcut(key: "t", command: true, shift: false, option: true, control: false) + ) { if let targetWindow = event.window ?? NSApp.keyWindow ?? NSApp.mainWindow, targetWindow.identifier?.rawValue == "cmux.settings" { targetWindow.performClose(nil) @@ -6632,7 +6703,10 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent // Cmd+W must close the focused panel even if first-responder momentarily lags on a // browser NSTextView during split focus transitions. - if normalizedFlags == [.command], (chars == "w" || event.keyCode == 13) { + if matchShortcut( + event: event, + shortcut: StoredShortcut(key: "w", command: true, shift: false, option: false, control: false) + ) { if let targetWindow = event.window ?? NSApp.keyWindow ?? NSApp.mainWindow, targetWindow.identifier?.rawValue == "cmux.settings" { targetWindow.performClose(nil) @@ -6861,7 +6935,10 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent } // Focus browser address bar: Cmd+L - if flags == [.command] && chars == "l" { + if matchShortcut( + event: event, + shortcut: StoredShortcut(key: "l", command: true, shift: false, option: false, control: false) + ) { if let focusedPanel = tabManager?.focusedBrowserPanel { focusBrowserAddressBar(in: focusedPanel) return true @@ -7449,42 +7526,127 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent return false } - /// Match a shortcut against an event, handling normal keys + /// Match a shortcut against an event, handling normal keys. private func matchShortcut(event: NSEvent, shortcut: StoredShortcut) -> Bool { // Some keys can include extra flags (e.g. .function) depending on the responder chain. // Strip those for consistent matching across first responders (terminal, WebKit, etc). let flags = event.modifierFlags.intersection(.deviceIndependentFlagsMask) - .subtracting([.numericPad, .function]) + .subtracting([.numericPad, .function, .capsLock]) guard flags == shortcut.modifierFlags else { return false } - // NSEvent.charactersIgnoringModifiers preserves Shift for some symbol keys - // (e.g. Shift+] can yield "}" instead of "]"), so match brackets by keyCode. let shortcutKey = shortcut.key.lowercased() if shortcutKey == "\r" { return event.keyCode == 36 || event.keyCode == 76 } - if shortcutKey == "[" || shortcutKey == "]" { - switch event.keyCode { - case 33: // kVK_ANSI_LeftBracket - return shortcutKey == "[" - case 30: // kVK_ANSI_RightBracket - return shortcutKey == "]" - default: - return false - } - } - // Control-key combos can produce control characters (e.g. Ctrl+H => backspace), - // so fall back to keyCode matching for common printable keys. - if let chars = event.charactersIgnoringModifiers?.lowercased(), chars == shortcutKey { + let eventCharsIgnoringModifiers = event.charactersIgnoringModifiers + if shortcutCharacterMatches( + eventCharacter: eventCharsIgnoringModifiers, + shortcutKey: shortcutKey, + applyShiftSymbolNormalization: flags.contains(.shift), + eventKeyCode: event.keyCode + ) { return true } - if let expectedKeyCode = keyCodeForShortcutKey(shortcutKey) { + + // For command-based shortcuts, trust AppKit's layout-aware characters when present. + // Keep this strict for letter shortcuts to avoid physical-key collisions across layouts, + // while still allowing keyCode fallback for digit/punctuation shortcuts on non-US layouts. + let hasEventChars = !(eventCharsIgnoringModifiers?.isEmpty ?? true) + if hasEventChars, + flags.contains(.command), + !flags.contains(.control), + shouldRequireCharacterMatchForCommandShortcut(shortcutKey: shortcutKey) { + return false + } + + // Match using the current keyboard layout so Command shortcuts stay character-based + // across layouts (QWERTY, Dvorak, etc.) instead of being tied to ANSI physical keys. + let layoutCharacter = shortcutLayoutCharacterProvider(event.keyCode, event.modifierFlags) + if shortcutCharacterMatches( + eventCharacter: layoutCharacter, + shortcutKey: shortcutKey, + applyShiftSymbolNormalization: false, + eventKeyCode: event.keyCode + ) { + return true + } + + // Control-key combos can surface as ASCII control characters (e.g. Ctrl+H => backspace), + // so keep ANSI keyCode fallback for control-modified shortcuts. Also allow fallback for + // command punctuation shortcuts, since some non-US layouts report different characters + // for the same physical key even when menu-equivalent semantics should still apply. + let allowANSIKeyCodeFallback = flags.contains(.control) + || (flags.contains(.command) + && !flags.contains(.control) + && ( + !shouldRequireCharacterMatchForCommandShortcut(shortcutKey: shortcutKey) + || (!hasEventChars && (layoutCharacter?.isEmpty ?? true)) + )) + if allowANSIKeyCodeFallback, let expectedKeyCode = keyCodeForShortcutKey(shortcutKey) { return event.keyCode == expectedKeyCode } return false } + private func shouldRequireCharacterMatchForCommandShortcut(shortcutKey: String) -> Bool { + guard shortcutKey.count == 1, let scalar = shortcutKey.unicodeScalars.first else { + return false + } + return CharacterSet.letters.contains(scalar) + } + + private func shortcutCharacterMatches( + eventCharacter: String?, + shortcutKey: String, + applyShiftSymbolNormalization: Bool, + eventKeyCode: UInt16 + ) -> Bool { + guard let eventCharacter, !eventCharacter.isEmpty else { return false } + if normalizedShortcutEventCharacter( + eventCharacter, + applyShiftSymbolNormalization: applyShiftSymbolNormalization, + eventKeyCode: eventKeyCode + ) == shortcutKey { + return true + } + return false + } + + private func normalizedShortcutEventCharacter( + _ eventCharacter: String, + applyShiftSymbolNormalization: Bool, + eventKeyCode: UInt16 + ) -> String { + let lowered = eventCharacter.lowercased() + guard applyShiftSymbolNormalization else { return lowered } + + switch lowered { + case "{": return "[" + case "}": return "]" + case "<": return eventKeyCode == 43 ? "," : lowered // kVK_ANSI_Comma + case ">": return eventKeyCode == 47 ? "." : lowered // kVK_ANSI_Period + case "?": return "/" + case ":": return ";" + case "\"": return "'" + case "|": return "\\" + case "~": return "`" + case "+": return "=" + case "_": return "-" + case "!": return eventKeyCode == 18 ? "1" : lowered // kVK_ANSI_1 + case "@": return eventKeyCode == 19 ? "2" : lowered // kVK_ANSI_2 + case "#": return eventKeyCode == 20 ? "3" : lowered // kVK_ANSI_3 + case "$": return eventKeyCode == 21 ? "4" : lowered // kVK_ANSI_4 + case "%": return eventKeyCode == 23 ? "5" : lowered // kVK_ANSI_5 + case "^": return eventKeyCode == 22 ? "6" : lowered // kVK_ANSI_6 + case "&": return eventKeyCode == 26 ? "7" : lowered // kVK_ANSI_7 + case "*": return eventKeyCode == 28 ? "8" : lowered // kVK_ANSI_8 + case "(": return eventKeyCode == 25 ? "9" : lowered // kVK_ANSI_9 + case ")": return eventKeyCode == 29 ? "0" : lowered // kVK_ANSI_0 + default: return lowered + } + } + private func keyCodeForShortcutKey(_ key: String) -> UInt16? { // Matches macOS ANSI key codes. This is intentionally limited to keys we // support in StoredShortcut/ghostty trigger translation. @@ -7518,8 +7680,10 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent case "-": return 27 // kVK_ANSI_Minus case "8": return 28 // kVK_ANSI_8 case "0": return 29 // kVK_ANSI_0 + case "]": return 30 // kVK_ANSI_RightBracket case "o": return 31 // kVK_ANSI_O case "u": return 32 // kVK_ANSI_U + case "[": return 33 // kVK_ANSI_LeftBracket case "i": return 34 // kVK_ANSI_I case "p": return 35 // kVK_ANSI_P case "l": return 37 // kVK_ANSI_L @@ -7991,16 +8155,6 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent ) #endif - if let notificationId, let store = notificationStore { - markReadIfFocused( - notificationId: notificationId, - tabId: tabId, - surfaceId: surfaceId, - tabManager: context.tabManager, - notificationStore: store - ) - } - #if DEBUG recordMultiWindowNotificationFocusIfNeeded( windowId: context.windowId, @@ -8054,15 +8208,6 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent ) #endif - if let notificationId, let store = notificationStore { - markReadIfFocused( - notificationId: notificationId, - tabId: tabId, - surfaceId: surfaceId, - tabManager: tabManager, - notificationStore: store - ) - } #if DEBUG if ProcessInfo.processInfo.environment["CMUX_UI_TEST_JUMP_UNREAD_SETUP"] == "1" { writeJumpUnreadTestData(["jumpUnreadOpenInFallback": "1", "jumpUnreadOpenResult": "1"]) @@ -8122,22 +8267,6 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent NSRunningApplication.current.activate(options: [.activateAllWindows, .activateIgnoringOtherApps]) } - private func markReadIfFocused( - notificationId: UUID, - tabId: UUID, - surfaceId: UUID?, - tabManager: TabManager, - notificationStore: TerminalNotificationStore - ) { - DispatchQueue.main.asyncAfter(deadline: .now() + 0.05) { - guard tabManager.selectedTabId == tabId else { return } - if let surfaceId { - guard tabManager.focusedSurfaceId(for: tabId) == surfaceId else { return } - } - notificationStore.markRead(id: notificationId) - } - } - #if DEBUG private func recordMultiWindowNotificationOpenFailureIfNeeded( tabId: UUID, diff --git a/Sources/ContentView.swift b/Sources/ContentView.swift index f886eb22..a289f0eb 100644 --- a/Sources/ContentView.swift +++ b/Sources/ContentView.swift @@ -5632,7 +5632,7 @@ struct VerticalTabsSidebar: View { @Binding var selection: SidebarSelection @Binding var selectedTabIds: Set @Binding var lastSidebarSelectionIndex: Int? - @StateObject private var commandKeyMonitor = SidebarCommandKeyMonitor() + @StateObject private var modifierKeyMonitor = SidebarShortcutHintModifierMonitor() @StateObject private var dragAutoScrollController = SidebarDragAutoScrollController() @StateObject private var dragFailsafeMonitor = SidebarDragFailsafeMonitor() @State private var draggedTabId: UUID? @@ -5660,7 +5660,7 @@ struct VerticalTabsSidebar: View { selection: $selection, selectedTabIds: $selectedTabIds, lastSidebarSelectionIndex: $lastSidebarSelectionIndex, - showsCommandShortcutHints: commandKeyMonitor.isCommandPressed, + showsModifierShortcutHints: modifierKeyMonitor.isModifierPressed, dragAutoScrollController: dragAutoScrollController, draggedTabId: $draggedTabId, dropIndicator: $dropIndicator @@ -5716,12 +5716,12 @@ struct VerticalTabsSidebar: View { .background(SidebarBackdrop().ignoresSafeArea()) .background( WindowAccessor { window in - commandKeyMonitor.setHostWindow(window) + modifierKeyMonitor.setHostWindow(window) } .frame(width: 0, height: 0) ) .onAppear { - commandKeyMonitor.start() + modifierKeyMonitor.start() draggedTabId = nil dropIndicator = nil SidebarDragLifecycleNotification.postStateDidChange( @@ -5730,7 +5730,7 @@ struct VerticalTabsSidebar: View { ) } .onDisappear { - commandKeyMonitor.stop() + modifierKeyMonitor.stop() dragAutoScrollController.stop() dragFailsafeMonitor.stop() draggedTabId = nil @@ -5774,15 +5774,18 @@ struct VerticalTabsSidebar: View { } } -enum SidebarCommandHintPolicy { +enum ShortcutHintModifierPolicy { static let intentionalHoldDelay: TimeInterval = 0.30 static func shouldShowHints( for modifierFlags: NSEvent.ModifierFlags, defaults: UserDefaults = .standard ) -> Bool { - ShortcutHintDebugSettings.showHintsOnCommandHoldEnabled(defaults: defaults) && - modifierFlags.intersection(.deviceIndependentFlagsMask) == [.command] + let normalized = modifierFlags.intersection(.deviceIndependentFlagsMask) + guard normalized == [.command] else { + return false + } + return ShortcutHintDebugSettings.showHintsOnCommandHoldEnabled(defaults: defaults) } static func isCurrentWindow( @@ -5847,6 +5850,11 @@ enum ShortcutHintDebugSettings { } return defaults.bool(forKey: showHintsOnCommandHoldKey) } + + static func resetVisibilityDefaults(defaults: UserDefaults = .standard) { + defaults.set(defaultAlwaysShowHints, forKey: alwaysShowHintsKey) + defaults.set(defaultShowHintsOnCommandHold, forKey: showHintsOnCommandHoldKey) + } } enum SidebarDragLifecycleNotification { @@ -6064,8 +6072,8 @@ private struct SidebarExternalDropDelegate: DropDelegate { } @MainActor -private final class SidebarCommandKeyMonitor: ObservableObject { - @Published private(set) var isCommandPressed = false +private final class SidebarShortcutHintModifierMonitor: ObservableObject { + @Published private(set) var isModifierPressed = false private weak var hostWindow: NSWindow? private var hostWindowDidBecomeKeyObserver: NSObjectProtocol? @@ -6159,7 +6167,7 @@ private final class SidebarCommandKeyMonitor: ObservableObject { } private func isCurrentWindow(eventWindow: NSWindow?) -> Bool { - SidebarCommandHintPolicy.isCurrentWindow( + ShortcutHintModifierPolicy.isCurrentWindow( hostWindowNumber: hostWindow?.windowNumber, hostWindowIsKey: hostWindow?.isKeyWindow ?? false, eventWindowNumber: eventWindow?.windowNumber, @@ -6168,7 +6176,7 @@ private final class SidebarCommandKeyMonitor: ObservableObject { } private func update(from modifierFlags: NSEvent.ModifierFlags, eventWindow: NSWindow?) { - guard SidebarCommandHintPolicy.shouldShowHints( + guard ShortcutHintModifierPolicy.shouldShowHints( for: modifierFlags, hostWindowNumber: hostWindow?.windowNumber, hostWindowIsKey: hostWindow?.isKeyWindow ?? false, @@ -6183,31 +6191,31 @@ private final class SidebarCommandKeyMonitor: ObservableObject { } private func queueHintShow() { - guard !isCommandPressed else { return } + guard !isModifierPressed else { return } guard pendingShowWorkItem == nil else { return } let workItem = DispatchWorkItem { [weak self] in guard let self else { return } self.pendingShowWorkItem = nil - guard SidebarCommandHintPolicy.shouldShowHints( + guard ShortcutHintModifierPolicy.shouldShowHints( for: NSEvent.modifierFlags, hostWindowNumber: self.hostWindow?.windowNumber, hostWindowIsKey: self.hostWindow?.isKeyWindow ?? false, eventWindowNumber: nil, keyWindowNumber: NSApp.keyWindow?.windowNumber ) else { return } - self.isCommandPressed = true + self.isModifierPressed = true } pendingShowWorkItem = workItem - DispatchQueue.main.asyncAfter(deadline: .now() + SidebarCommandHintPolicy.intentionalHoldDelay, execute: workItem) + DispatchQueue.main.asyncAfter(deadline: .now() + ShortcutHintModifierPolicy.intentionalHoldDelay, execute: workItem) } private func cancelPendingHintShow(resetVisible: Bool) { pendingShowWorkItem?.cancel() pendingShowWorkItem = nil if resetVisible { - isCommandPressed = false + isModifierPressed = false } } @@ -6445,7 +6453,7 @@ private struct TabItemView: View { @Binding var selection: SidebarSelection @Binding var selectedTabIds: Set @Binding var lastSidebarSelectionIndex: Int? - let showsCommandShortcutHints: Bool + let showsModifierShortcutHints: Bool let dragAutoScrollController: SidebarDragAutoScrollController @Binding var draggedTabId: UUID? @Binding var dropIndicator: SidebarDropIndicator? @@ -6548,7 +6556,7 @@ private struct TabItemView: View { } private var showCloseButton: Bool { - isHovering && tabManager.tabs.count > 1 && !(showsCommandShortcutHints || alwaysShowShortcutHints) + isHovering && tabManager.tabs.count > 1 && !(showsModifierShortcutHints || alwaysShowShortcutHints) } private var workspaceShortcutLabel: String? { @@ -6557,7 +6565,7 @@ private struct TabItemView: View { } private var showsWorkspaceShortcutHint: Bool { - (showsCommandShortcutHints || alwaysShowShortcutHints) && workspaceShortcutLabel != nil + (showsModifierShortcutHints || alwaysShowShortcutHints) && workspaceShortcutLabel != nil } private var workspaceHintSlotWidth: CGFloat { @@ -6673,7 +6681,7 @@ private struct TabItemView: View { .transition(.opacity) } } - .animation(.easeInOut(duration: 0.14), value: showsCommandShortcutHints || alwaysShowShortcutHints) + .animation(.easeInOut(duration: 0.14), value: showsModifierShortcutHints || alwaysShowShortcutHints) .frame(width: workspaceHintSlotWidth, height: 16, alignment: .trailing) } diff --git a/Sources/KeyboardLayout.swift b/Sources/KeyboardLayout.swift index 392d0723..f7b7110a 100644 --- a/Sources/KeyboardLayout.swift +++ b/Sources/KeyboardLayout.swift @@ -1,3 +1,4 @@ +import AppKit import Carbon class KeyboardLayout { @@ -12,8 +13,12 @@ class KeyboardLayout { return nil } - /// Translate a physical keyCode to the unmodified character under the current keyboard layout. - static func character(forKeyCode keyCode: UInt16) -> String? { + /// Translate a physical keyCode to the character AppKit would use for shortcut matching, + /// preserving command-aware layouts such as "Dvorak - QWERTY Command". + static func character( + forKeyCode keyCode: UInt16, + modifierFlags: NSEvent.ModifierFlags = [] + ) -> String? { guard let source = TISCopyCurrentKeyboardInputSource()?.takeRetainedValue(), let layoutDataPointer = TISGetInputSourceProperty(source, kTISPropertyUnicodeKeyLayoutData) else { return nil @@ -31,7 +36,7 @@ class KeyboardLayout { keyboardLayout, keyCode, UInt16(kUCKeyActionDisplay), - 0, + translationModifierKeyState(for: modifierFlags), UInt32(LMGetKbdType()), UInt32(kUCKeyTranslateNoDeadKeysBit), &deadKeyState, @@ -43,4 +48,20 @@ class KeyboardLayout { guard status == noErr, length > 0 else { return nil } return String(utf16CodeUnits: chars, count: length).lowercased() } + + private static func translationModifierKeyState(for modifierFlags: NSEvent.ModifierFlags) -> UInt32 { + let normalized = modifierFlags + .intersection(.deviceIndependentFlagsMask) + .intersection([.shift, .command]) + + var carbonModifiers: Int = 0 + if normalized.contains(.shift) { + carbonModifiers |= shiftKey + } + if normalized.contains(.command) { + carbonModifiers |= cmdKey + } + + return UInt32((carbonModifiers >> 8) & 0xFF) + } } diff --git a/Sources/TabManager.swift b/Sources/TabManager.swift index abdb3ea6..528669a6 100644 --- a/Sources/TabManager.swift +++ b/Sources/TabManager.swift @@ -599,7 +599,7 @@ class TabManager: ObservableObject { self.focusSelectedTabPanel(previousTabId: previousTabId) self.updateWindowTitleForSelectedTab() if let selectedTabId = self.selectedTabId { - self.markFocusedPanelReadIfActive(tabId: selectedTabId) + self.flashFocusedPanelIfUnreadAndActive(tabId: selectedTabId) } #if DEBUG let dtMs = self.debugWorkspaceSwitchStartTime > 0 @@ -671,7 +671,7 @@ class TabManager: ObservableObject { guard let self else { return } guard let tabId = notification.userInfo?[GhosttyNotificationKey.tabId] as? UUID else { return } guard let surfaceId = notification.userInfo?[GhosttyNotificationKey.surfaceId] as? UUID else { return } - markPanelReadOnFocusIfActive(tabId: tabId, panelId: surfaceId) + flashPanelIfUnreadAndActive(tabId: tabId, panelId: surfaceId) } }) @@ -1596,16 +1596,16 @@ class TabManager: ObservableObject { selectedTabId != pendingTabId } - private func markFocusedPanelReadIfActive(tabId: UUID) { + private func flashFocusedPanelIfUnreadAndActive(tabId: UUID) { let shouldSuppressFlash = suppressFocusFlash suppressFocusFlash = false guard !shouldSuppressFlash else { return } guard AppFocusState.isAppActive() else { return } guard let panelId = focusedPanelId(for: tabId) else { return } - markPanelReadOnFocusIfActive(tabId: tabId, panelId: panelId) + flashPanelIfUnreadAndActive(tabId: tabId, panelId: panelId) } - private func markPanelReadOnFocusIfActive(tabId: UUID, panelId: UUID) { + private func flashPanelIfUnreadAndActive(tabId: UUID, panelId: UUID) { guard selectedTabId == tabId else { return } guard !suppressFocusFlash else { return } guard AppFocusState.isAppActive() else { return } @@ -1614,7 +1614,6 @@ class TabManager: ObservableObject { if let tab = tabs.first(where: { $0.id == tabId }) { tab.triggerNotificationFocusFlash(panelId: panelId, requiresSplit: false, shouldFocus: false) } - notificationStore.markRead(forTabId: tabId, surfaceId: panelId) } private func enqueuePanelTitleUpdate(tabId: UUID, panelId: UUID, title: String) { @@ -1740,7 +1739,6 @@ class TabManager: ObservableObject { guard let notificationStore = AppDelegate.shared?.notificationStore else { return } guard notificationStore.hasUnreadNotification(forTabId: tabId, surfaceId: targetPanelId) else { return } tab.triggerNotificationFocusFlash(panelId: targetPanelId, requiresSplit: false, shouldFocus: true) - notificationStore.markRead(forTabId: tabId, surfaceId: targetPanelId) } } diff --git a/Sources/TerminalController.swift b/Sources/TerminalController.swift index 56d7205b..8f8eadf1 100644 --- a/Sources/TerminalController.swift +++ b/Sources/TerminalController.swift @@ -13,6 +13,9 @@ class TerminalController { let acceptLoopAlive: Bool let socketPathMatches: Bool let socketPathExists: Bool + let socketProbePerformed: Bool + let socketConnectable: Bool? + let socketConnectErrno: Int32? var failureSignals: [String] { var signals: [String] = [] @@ -20,6 +23,9 @@ class TerminalController { if !acceptLoopAlive { signals.append("accept_loop_dead") } if !socketPathMatches { signals.append("socket_path_mismatch") } if !socketPathExists { signals.append("socket_missing") } + if socketProbePerformed && isRunning && acceptLoopAlive && socketPathMatches && socketPathExists && socketConnectable == false { + signals.append("socket_unreachable") + } return signals } @@ -51,6 +57,14 @@ class TerminalController { private nonisolated static let acceptFailureMaxBackoffMs = 5_000 private nonisolated static let acceptFailureMinimumRearmDelayMs = 100 private nonisolated static let acceptFailureRearmThreshold = 50 + private nonisolated static let socketProbePollTimeoutMs: Int32 = 100 + private nonisolated static let socketProbePollAttempts = 3 + private nonisolated static let socketProbePollRetryBackoffUs: useconds_t = 50_000 + private nonisolated static let unixSocketPathMaxLength: Int = { + var addr = sockaddr_un() + // Reserve one byte for the null terminator. + return MemoryLayout.size(ofValue: addr.sun_path) - 1 + }() private struct ListenerStateSnapshot { let socketPath: String @@ -508,6 +522,99 @@ class TerminalController { return !isRunning && activeGeneration == 0 } + private nonisolated static func unixSocketAddress(path: String) -> sockaddr_un? { + var addr = sockaddr_un() + addr.sun_family = sa_family_t(AF_UNIX) + + let maxLength = unixSocketPathMaxLength + 1 + var didFit = false + path.withCString { source in + let sourceLength = strlen(source) + guard sourceLength < maxLength else { return } + + _ = withUnsafeMutableBytes(of: &addr.sun_path) { buffer in + buffer.initializeMemory(as: UInt8.self, repeating: 0) + } + withUnsafeMutablePointer(to: &addr.sun_path) { pathPtr in + let destination = UnsafeMutableRawPointer(pathPtr).assumingMemoryBound(to: CChar.self) + strncpy(destination, source, maxLength - 1) + } + didFit = true + } + return didFit ? addr : nil + } + + private nonisolated static func bindUnixSocket(_ socket: Int32, path: String) -> Int32? { + guard var addr = unixSocketAddress(path: path) else { return nil } + return withUnsafePointer(to: &addr) { ptr in + ptr.withMemoryRebound(to: sockaddr.self, capacity: 1) { sockaddrPtr in + bind(socket, sockaddrPtr, socklen_t(MemoryLayout.size)) + } + } + } + + private nonisolated static func probeSocketConnectability(path: String) -> (isConnectable: Bool?, errnoCode: Int32?) { + let probeSocket = socket(AF_UNIX, SOCK_STREAM, 0) + guard probeSocket >= 0 else { + return (false, errno) + } + defer { close(probeSocket) } + + let existingFlags = fcntl(probeSocket, F_GETFL, 0) + if existingFlags >= 0 { + _ = fcntl(probeSocket, F_SETFL, existingFlags | O_NONBLOCK) + } + + guard var addr = unixSocketAddress(path: path) else { + return (false, ENAMETOOLONG) + } + let connectResult = withUnsafePointer(to: &addr) { ptr in + ptr.withMemoryRebound(to: sockaddr.self, capacity: 1) { sockaddrPtr in + connect(probeSocket, sockaddrPtr, socklen_t(MemoryLayout.size)) + } + } + if connectResult == 0 { + return (true, nil) + } + let connectErrno = errno + if connectErrno == EINPROGRESS { + var pollDescriptor = pollfd(fd: probeSocket, events: Int16(POLLOUT), revents: 0) + for attempt in 0.. 0 { + var socketError: Int32 = 0 + var socketErrorLength = socklen_t(MemoryLayout.size) + let status = getsockopt( + probeSocket, + SOL_SOCKET, + SO_ERROR, + &socketError, + &socketErrorLength + ) + if status == 0 && socketError == 0 { + return (true, nil) + } + if status == 0 { + return (false, socketError) + } + return (false, errno) + } + + let pollErrno = errno + if pollResult == 0 || pollErrno == EINTR { + if attempt + 1 < Self.socketProbePollAttempts { + usleep(Self.socketProbePollRetryBackoffUs) + continue + } + return (false, pollResult == 0 ? ETIMEDOUT : pollErrno) + } + return (false, pollErrno) + } + } + return (false, connectErrno) + } + func start(tabManager: TabManager, socketPath: String, accessMode: SocketControlMode) { self.tabManager = tabManager self.accessMode = accessMode @@ -556,19 +663,18 @@ class TerminalController { } // Bind to path - var addr = sockaddr_un() - addr.sun_family = sa_family_t(AF_UNIX) - socketPath.withCString { ptr in - withUnsafeMutablePointer(to: &addr.sun_path) { pathPtr in - let pathBuf = UnsafeMutableRawPointer(pathPtr).assumingMemoryBound(to: CChar.self) - strcpy(pathBuf, ptr) - } - } - - let bindResult = withUnsafePointer(to: &addr) { ptr in - ptr.withMemoryRebound(to: sockaddr.self, capacity: 1) { sockaddrPtr in - bind(newServerSocket, sockaddrPtr, socklen_t(MemoryLayout.size)) - } + guard let bindResult = Self.bindUnixSocket(newServerSocket, path: socketPath) else { + close(newServerSocket) + reportSocketListenerFailure( + message: "socket.listener.start.failed", + stage: "bind_path_too_long", + errnoCode: ENAMETOOLONG, + extra: [ + "pathLength": socketPath.utf8.count, + "maxPathLength": Self.unixSocketPathMaxLength + ] + ) + return } guard bindResult >= 0 else { @@ -653,12 +759,19 @@ class TerminalController { var st = stat() let exists = lstat(expectedSocketPath, &st) == 0 && (st.st_mode & S_IFMT) == S_IFSOCK + let shouldProbeConnection = snapshot.isRunning && snapshot.acceptLoopAlive && pathMatches && exists + let connectability = shouldProbeConnection + ? Self.probeSocketConnectability(path: expectedSocketPath) + : (isConnectable: nil, errnoCode: nil) return SocketListenerHealth( isRunning: snapshot.isRunning, acceptLoopAlive: snapshot.acceptLoopAlive, socketPathMatches: pathMatches, - socketPathExists: exists + socketPathExists: exists, + socketProbePerformed: shouldProbeConnection, + socketConnectable: connectability.isConnectable, + socketConnectErrno: connectability.errnoCode ) } @@ -5284,41 +5397,70 @@ class TerminalController { _ webView: WKWebView, script: String, timeout: TimeInterval = 5.0, - preferAsync: Bool = false + preferAsync: Bool = false, + contentWorld: WKContentWorld ) -> V2JavaScriptResult { + let timeoutSeconds = max(0.01, timeout) + let resultLock = NSLock() + let completionSignal = DispatchSemaphore(value: 0) var done = false var resultValue: Any? var resultError: String? - if preferAsync, #available(macOS 11.0, *) { - webView.callAsyncJavaScript(script, arguments: [:], in: nil, in: .page) { result in - switch result { - case .success(let value): - resultValue = value - case .failure(let error): - resultError = error.localizedDescription - } + let finish: (_ value: Any?, _ error: String?) -> Void = { value, error in + resultLock.lock() + if !done { done = true + resultValue = value + resultError = error + completionSignal.signal() + } + resultLock.unlock() + } + + let evaluator = { + if preferAsync, #available(macOS 11.0, *) { + webView.callAsyncJavaScript(script, arguments: [:], in: nil, in: contentWorld) { result in + switch result { + case .success(let value): + finish(value, nil) + case .failure(let error): + finish(nil, error.localizedDescription) + } + } + } else { + webView.evaluateJavaScript(script) { value, error in + if let error { + finish(nil, error.localizedDescription) + } else { + finish(value, nil) + } + } + } + } + + if Thread.isMainThread { + evaluator() + let deadline = Date().addingTimeInterval(timeoutSeconds) + while true { + resultLock.lock() + let isDone = done + resultLock.unlock() + if isDone { + break + } + if Date() >= deadline { + return .failure("Timed out waiting for JavaScript result") + } + _ = RunLoop.current.run(mode: .default, before: Date().addingTimeInterval(0.01)) } } else { - webView.evaluateJavaScript(script) { value, error in - if let error { - resultError = error.localizedDescription - } else { - resultValue = value - } - done = true + DispatchQueue.main.async(execute: evaluator) + if completionSignal.wait(timeout: .now() + timeoutSeconds) == .timedOut { + return .failure("Timed out waiting for JavaScript result") } } - let deadline = Date().addingTimeInterval(timeout) - while !done && Date() < deadline { - _ = RunLoop.current.run(mode: .default, before: Date().addingTimeInterval(0.01)) - } - - if !done { - return .failure("Timed out waiting for JavaScript result") - } if let resultError { return .failure(resultError) } @@ -5368,7 +5510,8 @@ class TerminalController { _ webView: WKWebView, surfaceId: UUID, script: String, - timeout: TimeInterval = 5.0 + timeout: TimeInterval = 5.0, + useEval: Bool = true ) -> V2JavaScriptResult { let scriptLiteral = v2JSONLiteral(script) let framePrelude: String @@ -5387,6 +5530,13 @@ class TerminalController { framePrelude = "const __cmuxDoc = document;" } + let executionBlock: String + if useEval { + executionBlock = "const __r = eval(\(scriptLiteral));" + } else { + executionBlock = "const __r = \(script);" + } + let asyncFunctionBody = """ \(framePrelude) @@ -5399,7 +5549,7 @@ class TerminalController { const __cmuxEvalInFrame = async function() { const document = __cmuxDoc; - const __r = eval(\(scriptLiteral)); + \(executionBlock) const __value = await __cmuxMaybeAwait(__r); return { __cmux_t: (typeof __value === 'undefined') ? 'undefined' : 'value', @@ -5410,16 +5560,40 @@ class TerminalController { return await __cmuxEvalInFrame(); """ - let rawResult: V2JavaScriptResult + var rawResult: V2JavaScriptResult if #available(macOS 11.0, *) { - rawResult = v2RunJavaScript(webView, script: asyncFunctionBody, timeout: timeout, preferAsync: true) + rawResult = v2RunJavaScript( + webView, + script: asyncFunctionBody, + timeout: timeout, + preferAsync: true, + contentWorld: .page + ) } else { let evaluateFallback = """ (async () => { \(asyncFunctionBody) })() """ - rawResult = v2RunJavaScript(webView, script: evaluateFallback, timeout: timeout) + rawResult = v2RunJavaScript(webView, script: evaluateFallback, timeout: timeout, contentWorld: .page) + } + + if !useEval, case .failure(let pageMessage) = rawResult, #available(macOS 11.0, *) { + let isolatedResult = v2RunJavaScript( + webView, + script: asyncFunctionBody, + timeout: timeout, + preferAsync: true, + contentWorld: .defaultClient + ) + switch isolatedResult { + case .success: + rawResult = isolatedResult + case .failure(let isolatedMessage): + if isolatedMessage != pageMessage { + rawResult = .failure("\(pageMessage) (isolated-world retry: \(isolatedMessage))") + } + } } switch rawResult { @@ -5520,38 +5694,41 @@ class TerminalController { } } - private func v2BrowserWaitForCondition( - _ conditionScript: String, - webView: WKWebView, - surfaceId: UUID? = nil, - timeout: TimeInterval = 5.0, - pollInterval: TimeInterval = 0.05 - ) -> Bool { - let deadline = Date().addingTimeInterval(timeout) - while Date() < deadline { - let wrapped = "(() => { try { return !!(\(conditionScript)); } catch (_) { return false; } })()" - let jsResult: V2JavaScriptResult - if let surfaceId { - jsResult = v2RunBrowserJavaScript(webView, surfaceId: surfaceId, script: wrapped, timeout: max(0.5, pollInterval + 0.25)) - } else { - jsResult = v2RunJavaScript(webView, script: wrapped, timeout: max(0.5, pollInterval + 0.25)) - } - if case let .success(value) = jsResult, - let ok = value as? Bool, - ok { - return true - } - _ = RunLoop.current.run(mode: .default, before: Date().addingTimeInterval(pollInterval)) - } - return false - } - private func v2PNGData(from image: NSImage) -> Data? { guard let tiff = image.tiffRepresentation, let rep = NSBitmapImageRep(data: tiff) else { return nil } return rep.representation(using: .png, properties: [:]) } + private func bestEffortPruneTemporaryFiles( + in directoryURL: URL, + keepingMostRecent maxCount: Int = 50, + maxAge: TimeInterval = 24 * 60 * 60 + ) { + guard let entries = try? FileManager.default.contentsOfDirectory( + at: directoryURL, + includingPropertiesForKeys: [.isRegularFileKey, .contentModificationDateKey, .creationDateKey], + options: [.skipsHiddenFiles] + ) else { + return + } + + let now = Date() + let datedEntries = entries.compactMap { url -> (url: URL, date: Date)? in + guard let values = try? url.resourceValues(forKeys: [.isRegularFileKey, .contentModificationDateKey, .creationDateKey]), + values.isRegularFile == true else { + return nil + } + return (url, values.contentModificationDate ?? values.creationDate ?? .distantPast) + }.sorted { $0.date > $1.date } + + for (index, entry) in datedEntries.enumerated() { + if index >= maxCount || now.timeIntervalSince(entry.date) > maxAge { + try? FileManager.default.removeItem(at: entry.url) + } + } + } + // MARK: - Markdown private func v2MarkdownOpen(params: [String: Any]) -> V2CallResult { @@ -5972,7 +6149,7 @@ class TerminalController { let retryAttempts = max(1, v2Int(params, "retry_attempts") ?? 3) for attempt in 1...retryAttempts { - switch v2RunBrowserJavaScript(browserPanel.webView, surfaceId: surfaceId, script: script) { + switch v2RunBrowserJavaScript(browserPanel.webView, surfaceId: surfaceId, script: script, useEval: false) { case .failure(let message): return .err(code: "js_error", message: message, data: ["action": actionName, "selector": selector]) case .success(let value): @@ -6230,7 +6407,7 @@ class TerminalController { })() """ - switch v2RunBrowserJavaScript(browserPanel.webView, surfaceId: surfaceId, script: script, timeout: 10.0) { + switch v2RunBrowserJavaScript(browserPanel.webView, surfaceId: surfaceId, script: script, timeout: 10.0, useEval: false) { case .failure(let message): return .err(code: "js_error", message: message, data: nil) case .success(let value): @@ -6327,42 +6504,120 @@ class TerminalController { private func v2BrowserWait(params: [String: Any]) -> V2CallResult { let timeoutMs = max(1, v2Int(params, "timeout_ms") ?? 5_000) let timeout = Double(timeoutMs) / 1000.0 + let selectorRaw = v2BrowserSelector(params) - return v2BrowserWithPanel(params: params) { _, ws, surfaceId, browserPanel in - let conditionScript: String = { - if let selector = v2BrowserSelector(params) { - let literal = v2JSONLiteral(selector) - return "document.querySelector(\(literal)) !== null" + let conditionScriptBase: String = { + if let urlContains = v2String(params, "url_contains") { + let literal = v2JSONLiteral(urlContains) + return "String(location.href || '').includes(\(literal))" + } + if let textContains = v2String(params, "text_contains") { + let literal = v2JSONLiteral(textContains) + return "(document.body && String(document.body.innerText || '').includes(\(literal)))" + } + if let loadState = v2String(params, "load_state") { + let normalizedLoadState = loadState.lowercased() + if normalizedLoadState == "interactive" { + return """ + (() => { + const __state = String(document.readyState || '').toLowerCase(); + return __state === 'interactive' || __state === 'complete'; + })() + """ } - if let urlContains = v2String(params, "url_contains") { - let literal = v2JSONLiteral(urlContains) - return "String(location.href || '').includes(\(literal))" - } - if let textContains = v2String(params, "text_contains") { - let literal = v2JSONLiteral(textContains) - return "(document.body && String(document.body.innerText || '').includes(\(literal)))" - } - if let loadState = v2String(params, "load_state") { - let literal = v2JSONLiteral(loadState.lowercased()) - return "String(document.readyState || '').toLowerCase() === \(literal)" - } - if let fn = v2String(params, "function") { - return "(() => { return !!(\(fn)); })()" - } - return "document.readyState === 'complete'" - }() + let literal = v2JSONLiteral(normalizedLoadState) + return "String(document.readyState || '').toLowerCase() === \(literal)" + } + if let fn = v2String(params, "function") { + return "(() => { return !!(\(fn)); })()" + } + return "document.readyState === 'complete'" + }() - let ok = v2BrowserWaitForCondition(conditionScript, webView: browserPanel.webView, surfaceId: surfaceId, timeout: timeout) - if !ok { + var setupResult: V2CallResult? + var workspaceId: UUID? + var surfaceIdOut: UUID? + var webView: WKWebView? + + v2MainSync { + guard let tabManager = self.v2ResolveTabManager(params: params) else { + setupResult = .err(code: "unavailable", message: "TabManager not available", data: nil) + return + } + guard let ws = self.v2ResolveWorkspace(params: params, tabManager: tabManager) else { + setupResult = .err(code: "not_found", message: "Workspace not found", data: nil) + return + } + let surfaceId = self.v2UUID(params, "surface_id") ?? ws.focusedPanelId + guard let surfaceId else { + setupResult = .err(code: "not_found", message: "No focused browser surface", data: nil) + return + } + guard let browserPanel = ws.browserPanel(for: surfaceId) else { + setupResult = .err(code: "invalid_params", message: "Surface is not a browser", data: ["surface_id": surfaceId.uuidString]) + return + } + workspaceId = ws.id + surfaceIdOut = surfaceId + webView = browserPanel.webView + } + + if let setupResult { + return setupResult + } + guard let workspaceId, let surfaceIdOut, let webView else { + return .err(code: "internal_error", message: "Failed to resolve browser surface", data: nil) + } + + 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]) + } + let literal = v2JSONLiteral(selector) + conditionScript = "document.querySelector(\(literal)) !== null" + } else { + conditionScript = conditionScriptBase + } + + let deadline = Date().addingTimeInterval(timeout) + let pollInterval = 0.05 + let wrappedScript = "(() => { try { return !!(\(conditionScript)); } catch (_) { return false; } })()" + + while true { + switch v2RunBrowserJavaScript( + webView, + surfaceId: surfaceIdOut, + script: wrappedScript, + timeout: max(0.5, pollInterval + 0.25), + useEval: false + ) { + case .success(let value): + if let b = value as? Bool, b { + return .ok([ + "workspace_id": workspaceId.uuidString, + "workspace_ref": self.v2Ref(kind: .workspace, uuid: workspaceId), + "surface_id": surfaceIdOut.uuidString, + "surface_ref": self.v2Ref(kind: .surface, uuid: surfaceIdOut), + "waited": true + ]) + } + case .failure(let message): + return .err( + code: "js_error", + message: message, + data: [ + "condition": conditionScript, + "timeout_ms": timeoutMs + ] + ) + } + + if Date() >= deadline { return .err(code: "timeout", message: "Condition not met before timeout", data: ["timeout_ms": timeoutMs]) } - return .ok([ - "workspace_id": ws.id.uuidString, - "workspace_ref": v2Ref(kind: .workspace, uuid: ws.id), - "surface_id": surfaceId.uuidString, - "surface_ref": v2Ref(kind: .surface, uuid: surfaceId), - "waited": true - ]) + + Thread.sleep(forTimeInterval: pollInterval) } } @@ -6707,13 +6962,31 @@ class TerminalController { return .err(code: "internal_error", message: "Failed to capture snapshot", data: nil) } - return .ok([ + var result: [String: Any] = [ "workspace_id": ws.id.uuidString, "workspace_ref": v2Ref(kind: .workspace, uuid: ws.id), "surface_id": surfaceId.uuidString, "surface_ref": v2Ref(kind: .surface, uuid: surfaceId), "png_base64": imageData.base64EncodedString() - ]) + ] + + // Best effort: keep screenshot data available even when temp-file writes fail. + let screenshotsDirectory = FileManager.default.temporaryDirectory + .appendingPathComponent("cmux-browser-screenshots", isDirectory: true) + if (try? FileManager.default.createDirectory(at: screenshotsDirectory, withIntermediateDirectories: true)) != nil { + bestEffortPruneTemporaryFiles(in: screenshotsDirectory) + let timestampMs = Int(Date().timeIntervalSince1970 * 1000) + let shortSurfaceId = String(surfaceId.uuidString.prefix(8)) + let shortRandomId = String(UUID().uuidString.prefix(8)) + let filename = "surface-\(shortSurfaceId)-\(timestampMs)-\(shortRandomId).png" + let imageURL = screenshotsDirectory.appendingPathComponent(filename, isDirectory: false) + if (try? imageData.write(to: imageURL, options: .atomic)) != nil { + result["path"] = imageURL.path + result["url"] = imageURL.absoluteString + } + } + + return .ok(result) } } @@ -7543,7 +7816,8 @@ class TerminalController { _ = v2RunJavaScript( browserPanel.webView, script: BrowserPanel.telemetryHookBootstrapScriptSource, - timeout: 5.0 + timeout: 5.0, + contentWorld: .page ) } @@ -7551,7 +7825,8 @@ class TerminalController { _ = v2RunJavaScript( browserPanel.webView, script: BrowserPanel.dialogTelemetryHookBootstrapScriptSource, - timeout: 5.0 + timeout: 5.0, + contentWorld: .page ) } @@ -7583,7 +7858,7 @@ class TerminalController { })() """ - switch v2RunJavaScript(browserPanel.webView, script: script, timeout: 5.0) { + switch v2RunJavaScript(browserPanel.webView, script: script, timeout: 5.0, contentWorld: .page) { case .failure(let message): return .err(code: "js_error", message: message, data: nil) case .success(let value): @@ -8178,7 +8453,7 @@ class TerminalController { return { ok: true, items }; })() """ - switch v2RunJavaScript(browserPanel.webView, script: script, timeout: 5.0) { + switch v2RunJavaScript(browserPanel.webView, script: script, timeout: 5.0, contentWorld: .page) { case .failure(let message): return .err(code: "js_error", message: message, data: nil) case .success(let value): @@ -8216,7 +8491,7 @@ class TerminalController { return { ok: true, items }; })() """ - switch v2RunJavaScript(browserPanel.webView, script: script, timeout: 5.0) { + switch v2RunJavaScript(browserPanel.webView, script: script, timeout: 5.0, contentWorld: .page) { case .failure(let message): return .err(code: "js_error", message: message, data: nil) case .success(let value): diff --git a/Sources/TerminalNotificationStore.swift b/Sources/TerminalNotificationStore.swift index 5bb768cb..1f1dac18 100644 --- a/Sources/TerminalNotificationStore.swift +++ b/Sources/TerminalNotificationStore.swift @@ -833,16 +833,9 @@ final class TerminalNotificationStore: ObservableObject { let isFocusedSurface = surfaceId == nil || focusedSurfaceId == surfaceId let isFocusedPanel = isActiveTab && isFocusedSurface let isAppFocused = AppFocusState.isAppFocused() - if isAppFocused && isFocusedPanel { - if !idsToClear.isEmpty { - notifications = updated - center.removeDeliveredNotificationsOffMain(withIdentifiers: idsToClear) - center.removePendingNotificationRequestsOffMain(withIdentifiers: idsToClear) - } - return - } + let suppressNativeDelivery = isAppFocused && isFocusedPanel - if WorkspaceAutoReorderSettings.isEnabled() { + if WorkspaceAutoReorderSettings.isEnabled() && !suppressNativeDelivery { AppDelegate.shared?.tabManager?.moveTabToTop(tabId) } @@ -862,7 +855,11 @@ final class TerminalNotificationStore: ObservableObject { center.removeDeliveredNotificationsOffMain(withIdentifiers: idsToClear) center.removePendingNotificationRequestsOffMain(withIdentifiers: idsToClear) } - scheduleUserNotification(notification) + if suppressNativeDelivery { + Self.runNotificationCustomCommand(notification) + } else { + scheduleUserNotification(notification) + } } func markRead(id: UUID) { @@ -993,10 +990,7 @@ final class TerminalNotificationStore: ObservableObject { guard let self, authorized else { return } let content = UNMutableNotificationContent() - let appName = Bundle.main.object(forInfoDictionaryKey: "CFBundleDisplayName") as? String - ?? Bundle.main.object(forInfoDictionaryKey: "CFBundleName") as? String - ?? "cmux" - content.title = notification.title.isEmpty ? appName : notification.title + content.title = Self.notificationDisplayTitle(notification) content.subtitle = notification.subtitle content.body = notification.body content.sound = NotificationSoundSettings.sound() @@ -1019,16 +1013,27 @@ final class TerminalNotificationStore: ObservableObject { if let error { NSLog("Failed to schedule notification: \(error)") } else { - NotificationSoundSettings.runCustomCommand( - title: content.title, - subtitle: content.subtitle, - body: content.body - ) + Self.runNotificationCustomCommand(notification) } } } } + nonisolated private static func notificationDisplayTitle(_ notification: TerminalNotification) -> String { + let appName = Bundle.main.object(forInfoDictionaryKey: "CFBundleDisplayName") as? String + ?? Bundle.main.object(forInfoDictionaryKey: "CFBundleName") as? String + ?? "cmux" + return notification.title.isEmpty ? appName : notification.title + } + + nonisolated private static func runNotificationCustomCommand(_ notification: TerminalNotification) { + NotificationSoundSettings.runCustomCommand( + title: notificationDisplayTitle(notification), + subtitle: notification.subtitle, + body: notification.body + ) + } + private func ensureAuthorization( origin: AuthorizationRequestOrigin, _ completion: @escaping (Bool) -> Void diff --git a/Sources/Update/UpdateTitlebarAccessory.swift b/Sources/Update/UpdateTitlebarAccessory.swift index c7a96e0e..12af22b6 100644 --- a/Sources/Update/UpdateTitlebarAccessory.swift +++ b/Sources/Update/UpdateTitlebarAccessory.swift @@ -237,7 +237,7 @@ struct TitlebarControlsView: View { @AppStorage(ShortcutHintDebugSettings.titlebarHintYKey) private var titlebarShortcutHintYOffset = ShortcutHintDebugSettings.defaultTitlebarHintY @AppStorage(ShortcutHintDebugSettings.alwaysShowHintsKey) private var alwaysShowShortcutHints = ShortcutHintDebugSettings.defaultAlwaysShowHints @State private var shortcutRefreshTick = 0 - @StateObject private var commandKeyMonitor = TitlebarCommandKeyMonitor() + @StateObject private var modifierKeyMonitor = TitlebarShortcutHintModifierMonitor() private let titlebarHintRightSafetyShift: CGFloat = 10 private let titlebarHintBaseXShift: CGFloat = -10 private let titlebarHintBaseYShift: CGFloat = 1 @@ -269,7 +269,7 @@ struct TitlebarControlsView: View { } private var shouldShowTitlebarShortcutHints: Bool { - alwaysShowShortcutHints || commandKeyMonitor.isCommandPressed + alwaysShowShortcutHints || modifierKeyMonitor.isModifierPressed } var body: some View { @@ -283,7 +283,7 @@ struct TitlebarControlsView: View { .padding(.trailing, titlebarHintTrailingInset) .background( WindowAccessor { window in - commandKeyMonitor.setHostWindow(window) + modifierKeyMonitor.setHostWindow(window) } .frame(width: 0, height: 0) ) @@ -291,10 +291,10 @@ struct TitlebarControlsView: View { shortcutRefreshTick &+= 1 } .onAppear { - commandKeyMonitor.start() + modifierKeyMonitor.start() } .onDisappear { - commandKeyMonitor.stop() + modifierKeyMonitor.stop() } } @@ -503,8 +503,8 @@ struct TitlebarControlsView: View { } @MainActor -private final class TitlebarCommandKeyMonitor: ObservableObject { - @Published private(set) var isCommandPressed = false +private final class TitlebarShortcutHintModifierMonitor: ObservableObject { + @Published private(set) var isModifierPressed = false private weak var hostWindow: NSWindow? private var hostWindowDidBecomeKeyObserver: NSObjectProtocol? @@ -598,7 +598,7 @@ private final class TitlebarCommandKeyMonitor: ObservableObject { } private func isCurrentWindow(eventWindow: NSWindow?) -> Bool { - SidebarCommandHintPolicy.isCurrentWindow( + ShortcutHintModifierPolicy.isCurrentWindow( hostWindowNumber: hostWindow?.windowNumber, hostWindowIsKey: hostWindow?.isKeyWindow ?? false, eventWindowNumber: eventWindow?.windowNumber, @@ -607,7 +607,7 @@ private final class TitlebarCommandKeyMonitor: ObservableObject { } private func update(from modifierFlags: NSEvent.ModifierFlags, eventWindow: NSWindow?) { - guard SidebarCommandHintPolicy.shouldShowHints( + guard ShortcutHintModifierPolicy.shouldShowHints( for: modifierFlags, hostWindowNumber: hostWindow?.windowNumber, hostWindowIsKey: hostWindow?.isKeyWindow ?? false, @@ -622,31 +622,31 @@ private final class TitlebarCommandKeyMonitor: ObservableObject { } private func queueHintShow() { - guard !isCommandPressed else { return } + guard !isModifierPressed else { return } guard pendingShowWorkItem == nil else { return } let workItem = DispatchWorkItem { [weak self] in guard let self else { return } self.pendingShowWorkItem = nil - guard SidebarCommandHintPolicy.shouldShowHints( + guard ShortcutHintModifierPolicy.shouldShowHints( for: NSEvent.modifierFlags, hostWindowNumber: self.hostWindow?.windowNumber, hostWindowIsKey: self.hostWindow?.isKeyWindow ?? false, eventWindowNumber: nil, keyWindowNumber: NSApp.keyWindow?.windowNumber ) else { return } - self.isCommandPressed = true + self.isModifierPressed = true } pendingShowWorkItem = workItem - DispatchQueue.main.asyncAfter(deadline: .now() + SidebarCommandHintPolicy.intentionalHoldDelay, execute: workItem) + DispatchQueue.main.asyncAfter(deadline: .now() + ShortcutHintModifierPolicy.intentionalHoldDelay, execute: workItem) } private func cancelPendingHintShow(resetVisible: Bool) { pendingShowWorkItem?.cancel() pendingShowWorkItem = nil if resetVisible { - isCommandPressed = false + isModifierPressed = false } } diff --git a/Sources/cmuxApp.swift b/Sources/cmuxApp.swift index ff392c1f..00cd360b 100644 --- a/Sources/cmuxApp.swift +++ b/Sources/cmuxApp.swift @@ -2825,6 +2825,8 @@ struct SettingsView: View { @AppStorage(QuitWarningSettings.warnBeforeQuitKey) private var warnBeforeQuitShortcut = QuitWarningSettings.defaultWarnBeforeQuit @AppStorage(CommandPaletteRenameSelectionSettings.selectAllOnFocusKey) private var commandPaletteRenameSelectAllOnFocus = CommandPaletteRenameSelectionSettings.defaultSelectAllOnFocus + @AppStorage(ShortcutHintDebugSettings.alwaysShowHintsKey) + private var alwaysShowShortcutHints = ShortcutHintDebugSettings.defaultAlwaysShowHints @AppStorage(WorkspacePlacementSettings.placementKey) private var newWorkspacePlacement = WorkspacePlacementSettings.defaultPlacement.rawValue @AppStorage(WorkspaceAutoReorderSettings.key) private var workspaceAutoReorder = WorkspaceAutoReorderSettings.defaultValue @AppStorage(SidebarBranchLayoutSettings.key) private var sidebarBranchVerticalLayout = SidebarBranchLayoutSettings.defaultVerticalLayout @@ -4107,6 +4109,8 @@ struct SettingsView: View { notificationDockBadgeEnabled = NotificationBadgeSettings.defaultDockBadgeEnabled warnBeforeQuitShortcut = QuitWarningSettings.defaultWarnBeforeQuit commandPaletteRenameSelectAllOnFocus = CommandPaletteRenameSelectionSettings.defaultSelectAllOnFocus + ShortcutHintDebugSettings.resetVisibilityDefaults() + alwaysShowShortcutHints = ShortcutHintDebugSettings.defaultAlwaysShowHints newWorkspacePlacement = WorkspacePlacementSettings.defaultPlacement.rawValue workspaceAutoReorder = WorkspaceAutoReorderSettings.defaultValue sidebarBranchVerticalLayout = SidebarBranchLayoutSettings.defaultVerticalLayout diff --git a/cmuxTests/AppDelegateShortcutRoutingTests.swift b/cmuxTests/AppDelegateShortcutRoutingTests.swift index 49dfa9ab..22b09b39 100644 --- a/cmuxTests/AppDelegateShortcutRoutingTests.swift +++ b/cmuxTests/AppDelegateShortcutRoutingTests.swift @@ -8,6 +8,39 @@ import XCTest @MainActor final class AppDelegateShortcutRoutingTests: XCTestCase { + private var savedShortcutsByAction: [KeyboardShortcutSettings.Action: StoredShortcut] = [:] + private var actionsWithPersistedShortcut: Set = [] + + override func setUp() { + super.setUp() + actionsWithPersistedShortcut = Set( + KeyboardShortcutSettings.Action.allCases.filter { + UserDefaults.standard.object(forKey: $0.defaultsKey) != nil + } + ) + savedShortcutsByAction = Dictionary( + uniqueKeysWithValues: actionsWithPersistedShortcut.map { action in + (action, KeyboardShortcutSettings.shortcut(for: action)) + } + ) + KeyboardShortcutSettings.resetAll() + } + + override func tearDown() { + AppDelegate.shared?.shortcutLayoutCharacterProvider = KeyboardLayout.character(forKeyCode:modifierFlags:) + AppDelegate.shared?.dismissNotificationsPopoverIfShown() + RunLoop.main.run(until: Date(timeIntervalSinceNow: 0.05)) + for action in KeyboardShortcutSettings.Action.allCases { + if actionsWithPersistedShortcut.contains(action), + let savedShortcut = savedShortcutsByAction[action] { + KeyboardShortcutSettings.setShortcut(savedShortcut, for: action) + } else { + KeyboardShortcutSettings.resetShortcut(for: action) + } + } + super.tearDown() + } + func testCmdNUsesEventWindowContextWhenActiveManagerIsStale() { guard let appDelegate = AppDelegate.shared else { XCTFail("Expected AppDelegate.shared") @@ -311,6 +344,910 @@ final class AppDelegateShortcutRoutingTests: XCTestCase { XCTAssertTrue(appDelegate.tabManager === secondManager, "Shortcut routing should retarget active manager to event window") } + func testCmdPhysicalIWithDvorakCharactersDoesNotTriggerShowNotifications() { + guard let appDelegate = AppDelegate.shared else { + XCTFail("Expected AppDelegate.shared") + return + } + + let windowId = appDelegate.createMainWindow() + defer { closeWindow(withId: windowId) } + + guard let window = window(withId: windowId) else { + XCTFail("Expected test window") + return + } + + withTemporaryShortcut(action: .showNotifications) { + // Dvorak: physical ANSI "I" key can produce the character "c". + // This should behave like Cmd+C (copy), not match the Cmd+I app shortcut. + guard let event = NSEvent.keyEvent( + with: .keyDown, + location: .zero, + modifierFlags: [.command], + timestamp: ProcessInfo.processInfo.systemUptime, + windowNumber: window.windowNumber, + context: nil, + characters: "c", + charactersIgnoringModifiers: "c", + isARepeat: false, + keyCode: 34 // kVK_ANSI_I + ) else { + XCTFail("Failed to construct Dvorak Cmd+C event on physical ANSI I key") + return + } + +#if DEBUG + XCTAssertFalse(appDelegate.debugHandleCustomShortcut(event: event)) +#else + XCTFail("debugHandleCustomShortcut is only available in DEBUG") +#endif + } + } + + func testCmdPhysicalPWithDvorakCharactersDoesNotTriggerCommandPaletteSwitcher() { + guard let appDelegate = AppDelegate.shared else { + XCTFail("Expected AppDelegate.shared") + return + } + + let windowId = appDelegate.createMainWindow() + defer { closeWindow(withId: windowId) } + + guard let window = window(withId: windowId) else { + XCTFail("Expected test window") + return + } + + let switcherExpectation = expectation(description: "Cmd+L should not request command palette switcher") + switcherExpectation.isInverted = true + let token = NotificationCenter.default.addObserver( + forName: .commandPaletteSwitcherRequested, + object: nil, + queue: nil + ) { _ in + switcherExpectation.fulfill() + } + defer { NotificationCenter.default.removeObserver(token) } + + // Dvorak: physical ANSI "P" key can produce "l". + // This should behave as Cmd+L, not as physical Cmd+P. + guard let event = NSEvent.keyEvent( + with: .keyDown, + location: .zero, + modifierFlags: [.command], + timestamp: ProcessInfo.processInfo.systemUptime, + windowNumber: window.windowNumber, + context: nil, + characters: "l", + charactersIgnoringModifiers: "l", + isARepeat: false, + keyCode: 35 // kVK_ANSI_P + ) else { + XCTFail("Failed to construct Dvorak Cmd+L event on physical ANSI P key") + return + } + +#if DEBUG + _ = appDelegate.debugHandleCustomShortcut(event: event) +#else + XCTFail("debugHandleCustomShortcut is only available in DEBUG") +#endif + + wait(for: [switcherExpectation], timeout: 0.15) + } + + func testCmdPWithCapsLockStillTriggersCommandPaletteSwitcher() { + guard let appDelegate = AppDelegate.shared else { + XCTFail("Expected AppDelegate.shared") + return + } + + let windowId = appDelegate.createMainWindow() + defer { closeWindow(withId: windowId) } + + guard let window = window(withId: windowId) else { + XCTFail("Expected test window") + return + } + + let switcherExpectation = expectation(description: "Cmd+P with Caps Lock should request command palette switcher") + let token = NotificationCenter.default.addObserver( + forName: .commandPaletteSwitcherRequested, + object: nil, + queue: nil + ) { _ in + switcherExpectation.fulfill() + } + defer { NotificationCenter.default.removeObserver(token) } + + guard let event = NSEvent.keyEvent( + with: .keyDown, + location: .zero, + modifierFlags: [.command, .capsLock], + timestamp: ProcessInfo.processInfo.systemUptime, + windowNumber: window.windowNumber, + context: nil, + characters: "p", + charactersIgnoringModifiers: "p", + isARepeat: false, + keyCode: 35 // kVK_ANSI_P + ) else { + XCTFail("Failed to construct Cmd+P + Caps Lock event") + return + } + +#if DEBUG + XCTAssertTrue(appDelegate.debugHandleCustomShortcut(event: event)) +#else + XCTFail("debugHandleCustomShortcut is only available in DEBUG") +#endif + + wait(for: [switcherExpectation], timeout: 0.15) + } + + func testCmdPFallsBackToANSIKeyCodeWhenCharactersAndLayoutTranslationAreUnavailable() { + guard let appDelegate = AppDelegate.shared else { + XCTFail("Expected AppDelegate.shared") + return + } + + let windowId = appDelegate.createMainWindow() + defer { closeWindow(withId: windowId) } + + guard let window = window(withId: windowId) else { + XCTFail("Expected test window") + return + } + + appDelegate.shortcutLayoutCharacterProvider = { _, _ in nil } + defer { + appDelegate.shortcutLayoutCharacterProvider = KeyboardLayout.character(forKeyCode:modifierFlags:) + } + + let switcherExpectation = expectation(description: "Cmd+P with unavailable characters should request command palette switcher") + let token = NotificationCenter.default.addObserver( + forName: .commandPaletteSwitcherRequested, + object: nil, + queue: nil + ) { _ in + switcherExpectation.fulfill() + } + defer { NotificationCenter.default.removeObserver(token) } + + guard let event = NSEvent.keyEvent( + with: .keyDown, + location: .zero, + modifierFlags: [.command], + timestamp: ProcessInfo.processInfo.systemUptime, + windowNumber: window.windowNumber, + context: nil, + characters: "", + charactersIgnoringModifiers: "", + isARepeat: false, + keyCode: 35 // kVK_ANSI_P + ) else { + XCTFail("Failed to construct Cmd+P event with unavailable characters") + return + } + + XCTAssertTrue(appDelegate.handleBrowserSurfaceKeyEquivalent(event)) + wait(for: [switcherExpectation], timeout: 0.15) + } + + func testCmdPDoesNotFallbackToANSIKeyCodeWhenLayoutTranslationProvidesDifferentLetter() { + guard let appDelegate = AppDelegate.shared else { + XCTFail("Expected AppDelegate.shared") + return + } + + let windowId = appDelegate.createMainWindow() + defer { closeWindow(withId: windowId) } + + guard let window = window(withId: windowId) else { + XCTFail("Expected test window") + return + } + + appDelegate.shortcutLayoutCharacterProvider = { _, _ in "b" } + defer { + appDelegate.shortcutLayoutCharacterProvider = KeyboardLayout.character(forKeyCode:modifierFlags:) + } + + let switcherExpectation = expectation(description: "Non-P layout translation should not request command palette switcher") + switcherExpectation.isInverted = true + let token = NotificationCenter.default.addObserver( + forName: .commandPaletteSwitcherRequested, + object: nil, + queue: nil + ) { _ in + switcherExpectation.fulfill() + } + defer { NotificationCenter.default.removeObserver(token) } + + guard let event = NSEvent.keyEvent( + with: .keyDown, + location: .zero, + modifierFlags: [.command], + timestamp: ProcessInfo.processInfo.systemUptime, + windowNumber: window.windowNumber, + context: nil, + characters: "", + charactersIgnoringModifiers: "", + isARepeat: false, + keyCode: 35 // kVK_ANSI_P + ) else { + XCTFail("Failed to construct Cmd+P event with unavailable characters") + return + } + + _ = appDelegate.handleBrowserSurfaceKeyEquivalent(event) + wait(for: [switcherExpectation], timeout: 0.15) + } + + func testCmdPFallsBackToCommandAwareLayoutTranslationWhenCharactersAreUnavailable() { + guard let appDelegate = AppDelegate.shared else { + XCTFail("Expected AppDelegate.shared") + return + } + + let windowId = appDelegate.createMainWindow() + defer { closeWindow(withId: windowId) } + + guard let window = window(withId: windowId) else { + XCTFail("Expected test window") + return + } + + appDelegate.shortcutLayoutCharacterProvider = { keyCode, modifierFlags in + guard keyCode == 35 else { return nil } // kVK_ANSI_P + return modifierFlags.contains(.command) ? "p" : "r" + } + defer { + appDelegate.shortcutLayoutCharacterProvider = KeyboardLayout.character(forKeyCode:modifierFlags:) + } + + let switcherExpectation = expectation(description: "Command-aware layout translation should request command palette switcher") + let token = NotificationCenter.default.addObserver( + forName: .commandPaletteSwitcherRequested, + object: nil, + queue: nil + ) { _ in + switcherExpectation.fulfill() + } + defer { NotificationCenter.default.removeObserver(token) } + + guard let event = NSEvent.keyEvent( + with: .keyDown, + location: .zero, + modifierFlags: [.command], + timestamp: ProcessInfo.processInfo.systemUptime, + windowNumber: window.windowNumber, + context: nil, + characters: "", + charactersIgnoringModifiers: "", + isARepeat: false, + keyCode: 35 // kVK_ANSI_P + ) else { + XCTFail("Failed to construct Cmd+P event with unavailable characters") + return + } + + XCTAssertTrue(appDelegate.handleBrowserSurfaceKeyEquivalent(event)) + wait(for: [switcherExpectation], timeout: 0.15) + } + + func testCmdShiftPhysicalPWithDvorakCharactersDoesNotTriggerCommandPalette() { + guard let appDelegate = AppDelegate.shared else { + XCTFail("Expected AppDelegate.shared") + return + } + + let windowId = appDelegate.createMainWindow() + defer { closeWindow(withId: windowId) } + + guard let window = window(withId: windowId) else { + XCTFail("Expected test window") + return + } + + let paletteExpectation = expectation(description: "Cmd+Shift+L should not request command palette") + paletteExpectation.isInverted = true + let token = NotificationCenter.default.addObserver( + forName: .commandPaletteRequested, + object: nil, + queue: nil + ) { _ in + paletteExpectation.fulfill() + } + defer { NotificationCenter.default.removeObserver(token) } + + // Dvorak: physical ANSI "P" key can produce "l". + // This should behave as Cmd+Shift+L, not as physical Cmd+Shift+P. + guard let event = NSEvent.keyEvent( + with: .keyDown, + location: .zero, + modifierFlags: [.command, .shift], + timestamp: ProcessInfo.processInfo.systemUptime, + windowNumber: window.windowNumber, + context: nil, + characters: "l", + charactersIgnoringModifiers: "l", + isARepeat: false, + keyCode: 35 // kVK_ANSI_P + ) else { + XCTFail("Failed to construct Dvorak Cmd+Shift+L event on physical ANSI P key") + return + } + +#if DEBUG + _ = appDelegate.debugHandleCustomShortcut(event: event) +#else + XCTFail("debugHandleCustomShortcut is only available in DEBUG") +#endif + + wait(for: [paletteExpectation], timeout: 0.15) + } + + func testCmdOptionPhysicalTWithDvorakCharactersDoesNotTriggerCloseOtherTabsShortcut() { + guard let appDelegate = AppDelegate.shared else { + XCTFail("Expected AppDelegate.shared") + return + } + + let windowId = appDelegate.createMainWindow() + defer { closeWindow(withId: windowId) } + + guard let window = window(withId: windowId) else { + XCTFail("Expected test window") + return + } + + // Dvorak: physical ANSI "T" key can produce "y". + // This should not match the Cmd+Option+T app shortcut. + guard let event = NSEvent.keyEvent( + with: .keyDown, + location: .zero, + modifierFlags: [.command, .option], + timestamp: ProcessInfo.processInfo.systemUptime, + windowNumber: window.windowNumber, + context: nil, + characters: "y", + charactersIgnoringModifiers: "y", + isARepeat: false, + keyCode: 17 // kVK_ANSI_T + ) else { + XCTFail("Failed to construct Dvorak Cmd+Option+Y event on physical ANSI T key") + return + } + +#if DEBUG + XCTAssertFalse(appDelegate.debugHandleCustomShortcut(event: event)) +#else + XCTFail("debugHandleCustomShortcut is only available in DEBUG") +#endif + } + + func testCmdPhysicalWWithDvorakCharactersDoesNotTriggerClosePanelShortcut() { + guard let appDelegate = AppDelegate.shared else { + XCTFail("Expected AppDelegate.shared") + return + } + + let windowId = appDelegate.createMainWindow() + defer { closeWindow(withId: windowId) } + + guard let window = window(withId: windowId), + let manager = appDelegate.tabManagerFor(windowId: windowId), + let workspace = manager.selectedWorkspace else { + XCTFail("Expected test window and workspace") + return + } + + let panelCountBefore = workspace.panels.count + + // Dvorak: physical ANSI "W" key can produce ",". + // This should not match the Cmd+W close-panel shortcut. + guard let event = NSEvent.keyEvent( + with: .keyDown, + location: .zero, + modifierFlags: [.command], + timestamp: ProcessInfo.processInfo.systemUptime, + windowNumber: window.windowNumber, + context: nil, + characters: ",", + charactersIgnoringModifiers: ",", + isARepeat: false, + keyCode: 13 // kVK_ANSI_W + ) else { + XCTFail("Failed to construct Dvorak Cmd+, event on physical ANSI W key") + return + } + +#if DEBUG + XCTAssertFalse(appDelegate.debugHandleCustomShortcut(event: event)) +#else + XCTFail("debugHandleCustomShortcut is only available in DEBUG") +#endif + XCTAssertEqual(workspace.panels.count, panelCountBefore) + } + + func testCmdIStillTriggersShowNotificationsShortcut() { + guard let appDelegate = AppDelegate.shared else { + XCTFail("Expected AppDelegate.shared") + return + } + + let windowId = appDelegate.createMainWindow() + defer { closeWindow(withId: windowId) } + + guard let window = window(withId: windowId) else { + XCTFail("Expected test window") + return + } + + withTemporaryShortcut(action: .showNotifications) { + guard let event = NSEvent.keyEvent( + with: .keyDown, + location: .zero, + modifierFlags: [.command], + timestamp: ProcessInfo.processInfo.systemUptime, + windowNumber: window.windowNumber, + context: nil, + characters: "i", + charactersIgnoringModifiers: "i", + isARepeat: false, + keyCode: 34 // kVK_ANSI_I + ) else { + XCTFail("Failed to construct Cmd+I event") + return + } + +#if DEBUG + XCTAssertTrue(appDelegate.debugHandleCustomShortcut(event: event)) +#else + XCTFail("debugHandleCustomShortcut is only available in DEBUG") +#endif + } + } + + func testCmdUnshiftedSymbolDoesNotMatchDigitShortcut() { + guard let appDelegate = AppDelegate.shared else { + XCTFail("Expected AppDelegate.shared") + return + } + + let windowId = appDelegate.createMainWindow() + defer { closeWindow(withId: windowId) } + + guard let window = window(withId: windowId) else { + XCTFail("Expected test window") + return + } + + withTemporaryShortcut( + action: .showNotifications, + shortcut: StoredShortcut(key: "8", command: true, shift: false, option: false, control: false) + ) { + // Some non-US layouts can produce "*" without Shift. + // This must not be coerced into "8" for a Cmd+8 shortcut match. + guard let event = NSEvent.keyEvent( + with: .keyDown, + location: .zero, + modifierFlags: [.command], + timestamp: ProcessInfo.processInfo.systemUptime, + windowNumber: window.windowNumber, + context: nil, + characters: "*", + charactersIgnoringModifiers: "*", + isARepeat: false, + keyCode: 30 // kVK_ANSI_RightBracket + ) else { + XCTFail("Failed to construct Cmd+* event") + return + } + +#if DEBUG + XCTAssertFalse(appDelegate.debugHandleCustomShortcut(event: event)) +#else + XCTFail("debugHandleCustomShortcut is only available in DEBUG") +#endif + } + } + + func testCmdDigitShortcutFallsBackByKeyCodeOnSymbolFirstLayouts() { + guard let appDelegate = AppDelegate.shared else { + XCTFail("Expected AppDelegate.shared") + return + } + + let windowId = appDelegate.createMainWindow() + defer { closeWindow(withId: windowId) } + + guard let window = window(withId: windowId) else { + XCTFail("Expected test window") + return + } + + withTemporaryShortcut( + action: .showNotifications, + shortcut: StoredShortcut(key: "1", command: true, shift: false, option: false, control: false) + ) { + // Symbol-first layouts (for example AZERTY) can report "&" for the ANSI 1 key. + // Cmd+1 shortcuts should still match via keyCode fallback in this case. + guard let event = NSEvent.keyEvent( + with: .keyDown, + location: .zero, + modifierFlags: [.command], + timestamp: ProcessInfo.processInfo.systemUptime, + windowNumber: window.windowNumber, + context: nil, + characters: "&", + charactersIgnoringModifiers: "&", + isARepeat: false, + keyCode: 18 // kVK_ANSI_1 + ) else { + XCTFail("Failed to construct Cmd+& event on ANSI 1 key") + return + } + +#if DEBUG + XCTAssertTrue(appDelegate.debugHandleCustomShortcut(event: event)) +#else + XCTFail("debugHandleCustomShortcut is only available in DEBUG") +#endif + } + } + + func testCmdShiftNonDigitKeySymbolDoesNotMatchShiftedDigitShortcut() { + guard let appDelegate = AppDelegate.shared else { + XCTFail("Expected AppDelegate.shared") + return + } + + let windowId = appDelegate.createMainWindow() + defer { closeWindow(withId: windowId) } + + guard let window = window(withId: windowId) else { + XCTFail("Expected test window") + return + } + + withTemporaryShortcut( + action: .showNotifications, + shortcut: StoredShortcut(key: "8", command: true, shift: true, option: false, control: false) + ) { + // Avoid unrelated default Cmd+Shift+] handling for this assertion. + withTemporaryShortcut( + action: .nextSurface, + shortcut: StoredShortcut(key: "x", command: true, shift: true, option: false, control: false) + ) { + // On some non-US layouts, Shift+RightBracket can produce "*". + // This must not be interpreted as Shift+8. + guard let event = NSEvent.keyEvent( + with: .keyDown, + location: .zero, + modifierFlags: [.command, .shift], + timestamp: ProcessInfo.processInfo.systemUptime, + windowNumber: window.windowNumber, + context: nil, + characters: "*", + charactersIgnoringModifiers: "*", + isARepeat: false, + keyCode: 30 // kVK_ANSI_RightBracket + ) else { + XCTFail("Failed to construct Cmd+Shift+* event from non-digit key") + return + } + +#if DEBUG + XCTAssertFalse(appDelegate.debugHandleCustomShortcut(event: event)) +#else + XCTFail("debugHandleCustomShortcut is only available in DEBUG") +#endif + } + } + } + + func testCmdShiftDigitShortcutMatchesShiftedDigitKey() { + guard let appDelegate = AppDelegate.shared else { + XCTFail("Expected AppDelegate.shared") + return + } + + let windowId = appDelegate.createMainWindow() + defer { closeWindow(withId: windowId) } + + guard let window = window(withId: windowId) else { + XCTFail("Expected test window") + return + } + + withTemporaryShortcut( + action: .showNotifications, + shortcut: StoredShortcut(key: "8", command: true, shift: true, option: false, control: false) + ) { + guard let event = NSEvent.keyEvent( + with: .keyDown, + location: .zero, + modifierFlags: [.command, .shift], + timestamp: ProcessInfo.processInfo.systemUptime, + windowNumber: window.windowNumber, + context: nil, + characters: "*", + charactersIgnoringModifiers: "*", + isARepeat: false, + keyCode: 28 // kVK_ANSI_8 + ) else { + XCTFail("Failed to construct Cmd+Shift+8 event") + return + } + +#if DEBUG + XCTAssertTrue(appDelegate.debugHandleCustomShortcut(event: event)) +#else + XCTFail("debugHandleCustomShortcut is only available in DEBUG") +#endif + } + } + + func testCmdShiftQuestionMarkMatchesSlashShortcut() { + guard let appDelegate = AppDelegate.shared else { + XCTFail("Expected AppDelegate.shared") + return + } + + let windowId = appDelegate.createMainWindow() + defer { closeWindow(withId: windowId) } + + guard let window = window(withId: windowId) else { + XCTFail("Expected test window") + return + } + + withTemporaryShortcut( + action: .triggerFlash, + shortcut: StoredShortcut(key: "/", command: true, shift: true, option: false, control: false) + ) { + guard let event = NSEvent.keyEvent( + with: .keyDown, + location: .zero, + modifierFlags: [.command, .shift], + timestamp: ProcessInfo.processInfo.systemUptime, + windowNumber: window.windowNumber, + context: nil, + characters: "?", + charactersIgnoringModifiers: "?", + isARepeat: false, + keyCode: 44 // kVK_ANSI_Slash + ) else { + XCTFail("Failed to construct Cmd+Shift+/ event") + return + } + +#if DEBUG + XCTAssertTrue(appDelegate.debugHandleCustomShortcut(event: event)) +#else + XCTFail("debugHandleCustomShortcut is only available in DEBUG") +#endif + } + } + + func testCmdShiftISOAngleBracketDoesNotMatchCommaShortcut() { + guard let appDelegate = AppDelegate.shared else { + XCTFail("Expected AppDelegate.shared") + return + } + + let windowId = appDelegate.createMainWindow() + defer { closeWindow(withId: windowId) } + + guard let window = window(withId: windowId) else { + XCTFail("Expected test window") + return + } + + withTemporaryShortcut( + action: .showNotifications, + shortcut: StoredShortcut(key: ",", command: true, shift: true, option: false, control: false) + ) { + guard let event = NSEvent.keyEvent( + with: .keyDown, + location: .zero, + modifierFlags: [.command, .shift], + timestamp: ProcessInfo.processInfo.systemUptime, + windowNumber: window.windowNumber, + context: nil, + characters: "<", + charactersIgnoringModifiers: "<", + isARepeat: false, + keyCode: 10 // kVK_ISO_Section + ) else { + XCTFail("Failed to construct Cmd+Shift+< event from ISO key") + return + } + +#if DEBUG + XCTAssertFalse(appDelegate.debugHandleCustomShortcut(event: event)) +#else + XCTFail("debugHandleCustomShortcut is only available in DEBUG") +#endif + } + } + + func testCmdShiftRightBracketCanFallbackByKeyCodeOnNonUSLayouts() { + guard let appDelegate = AppDelegate.shared else { + XCTFail("Expected AppDelegate.shared") + return + } + + let windowId = appDelegate.createMainWindow() + defer { closeWindow(withId: windowId) } + + guard let window = window(withId: windowId) else { + XCTFail("Expected test window") + return + } + + withTemporaryShortcut(action: .nextSurface) { + // Non-US layouts can report "*" (or other symbols) for kVK_ANSI_RightBracket with Shift. + // Shortcut matching should still allow Cmd+Shift+] via keyCode fallback. + guard let event = NSEvent.keyEvent( + with: .keyDown, + location: .zero, + modifierFlags: [.command, .shift], + timestamp: ProcessInfo.processInfo.systemUptime, + windowNumber: window.windowNumber, + context: nil, + characters: "*", + charactersIgnoringModifiers: "*", + isARepeat: false, + keyCode: 30 // kVK_ANSI_RightBracket + ) else { + XCTFail("Failed to construct non-US Cmd+Shift+] event") + return + } + +#if DEBUG + XCTAssertTrue(appDelegate.debugHandleCustomShortcut(event: event)) +#else + XCTFail("debugHandleCustomShortcut is only available in DEBUG") +#endif + } + } + + func testCmdPhysicalOWithDvorakCharactersTriggersRenameTabShortcut() { + guard let appDelegate = AppDelegate.shared else { + XCTFail("Expected AppDelegate.shared") + return + } + + let windowId = appDelegate.createMainWindow() + defer { closeWindow(withId: windowId) } + + guard let window = window(withId: windowId) else { + XCTFail("Expected test window") + return + } + + let renameTabExpectation = expectation(description: "Expected rename tab request for semantic Cmd+R") + var observedRenameTabWindow: NSWindow? + let renameTabToken = NotificationCenter.default.addObserver( + forName: .commandPaletteRenameTabRequested, + object: nil, + queue: nil + ) { notification in + observedRenameTabWindow = notification.object as? NSWindow + renameTabExpectation.fulfill() + } + defer { NotificationCenter.default.removeObserver(renameTabToken) } + + let switcherExpectation = expectation(description: "Cmd+R should not trigger command palette switcher") + switcherExpectation.isInverted = true + let switcherToken = NotificationCenter.default.addObserver( + forName: .commandPaletteSwitcherRequested, + object: nil, + queue: nil + ) { _ in + switcherExpectation.fulfill() + } + defer { NotificationCenter.default.removeObserver(switcherToken) } + + withTemporaryShortcut(action: .renameTab) { + // Dvorak: physical ANSI "O" key can produce "r". + // This should behave as semantic Cmd+R (rename tab), not Cmd+P. + guard let event = NSEvent.keyEvent( + with: .keyDown, + location: .zero, + modifierFlags: [.command], + timestamp: ProcessInfo.processInfo.systemUptime, + windowNumber: window.windowNumber, + context: nil, + characters: "r", + charactersIgnoringModifiers: "r", + isARepeat: false, + keyCode: 31 // kVK_ANSI_O + ) else { + XCTFail("Failed to construct Dvorak Cmd+R event on physical ANSI O key") + return + } + +#if DEBUG + XCTAssertTrue(appDelegate.debugHandleCustomShortcut(event: event)) +#else + XCTFail("debugHandleCustomShortcut is only available in DEBUG") +#endif + } + + wait(for: [renameTabExpectation, switcherExpectation], timeout: 1.0) + XCTAssertEqual(observedRenameTabWindow?.windowNumber, window.windowNumber) + } + + func testCmdPhysicalRWithDvorakCharactersTriggersCommandPaletteSwitcher() { + guard let appDelegate = AppDelegate.shared else { + XCTFail("Expected AppDelegate.shared") + return + } + + let windowId = appDelegate.createMainWindow() + defer { closeWindow(withId: windowId) } + + guard let window = window(withId: windowId) else { + XCTFail("Expected test window") + return + } + + let switcherExpectation = expectation(description: "Expected command palette switcher request for semantic Cmd+P") + var observedSwitcherWindow: NSWindow? + let switcherToken = NotificationCenter.default.addObserver( + forName: .commandPaletteSwitcherRequested, + object: nil, + queue: nil + ) { notification in + observedSwitcherWindow = notification.object as? NSWindow + switcherExpectation.fulfill() + } + defer { NotificationCenter.default.removeObserver(switcherToken) } + + let renameTabExpectation = expectation(description: "Physical R on Dvorak should not trigger rename tab") + renameTabExpectation.isInverted = true + let renameTabToken = NotificationCenter.default.addObserver( + forName: .commandPaletteRenameTabRequested, + object: nil, + queue: nil + ) { _ in + renameTabExpectation.fulfill() + } + defer { NotificationCenter.default.removeObserver(renameTabToken) } + + // Dvorak: physical ANSI "R" key can produce "p". + // This should behave as semantic Cmd+P (palette switcher), not Cmd+R. + guard let event = NSEvent.keyEvent( + with: .keyDown, + location: .zero, + modifierFlags: [.command], + timestamp: ProcessInfo.processInfo.systemUptime, + windowNumber: window.windowNumber, + context: nil, + characters: "p", + charactersIgnoringModifiers: "p", + isARepeat: false, + keyCode: 15 // kVK_ANSI_R + ) else { + XCTFail("Failed to construct Dvorak Cmd+P event on physical ANSI R key") + return + } + +#if DEBUG + XCTAssertTrue(appDelegate.debugHandleCustomShortcut(event: event)) +#else + XCTFail("debugHandleCustomShortcut is only available in DEBUG") +#endif + + wait(for: [switcherExpectation, renameTabExpectation], timeout: 1.0) + XCTAssertEqual(observedSwitcherWindow?.windowNumber, window.windowNumber) + } + func testCmdShiftRRequestsRenameWorkspaceInCommandPalette() { guard let appDelegate = AppDelegate.shared else { XCTFail("Expected AppDelegate.shared") @@ -684,6 +1621,9 @@ final class AppDelegateShortcutRoutingTests: XCTestCase { return } + window.makeKeyAndOrderFront(nil) + RunLoop.main.run(until: Date(timeIntervalSinceNow: 0.05)) + #if DEBUG XCTAssertTrue( appDelegate.debugSetCommandPalettePendingOpenAge(window: window, age: 20.0), @@ -1210,6 +2150,24 @@ final class AppDelegateShortcutRoutingTests: XCTestCase { ) } + private func withTemporaryShortcut( + action: KeyboardShortcutSettings.Action, + shortcut: StoredShortcut? = nil, + _ body: () -> Void + ) { + let hadPersistedShortcut = UserDefaults.standard.object(forKey: action.defaultsKey) != nil + let originalShortcut = KeyboardShortcutSettings.shortcut(for: action) + defer { + if hadPersistedShortcut { + KeyboardShortcutSettings.setShortcut(originalShortcut, for: action) + } else { + KeyboardShortcutSettings.resetShortcut(for: action) + } + } + KeyboardShortcutSettings.setShortcut(shortcut ?? action.defaultShortcut, for: action) + body() + } + private func assertEscapeKeyUpIsConsumedAfterCommandPaletteOpenRequest( _ openRequest: (_ appDelegate: AppDelegate, _ window: NSWindow) -> Void, file: StaticString = #filePath, diff --git a/cmuxTests/CmuxWebViewKeyEquivalentTests.swift b/cmuxTests/CmuxWebViewKeyEquivalentTests.swift index 2177f000..9e34690f 100644 --- a/cmuxTests/CmuxWebViewKeyEquivalentTests.swift +++ b/cmuxTests/CmuxWebViewKeyEquivalentTests.swift @@ -2758,6 +2758,52 @@ final class FullScreenShortcutTests: XCTestCase { shouldToggleMainWindowFullScreenForCommandControlFShortcut( flags: [.command, .control], chars: "", + keyCode: 3, + layoutCharacterProvider: { _, _ in nil } + ) + ) + } + + func testDoesNotFallbackToANSIWhenLayoutTranslationReturnsNonFCharacter() { + XCTAssertFalse( + shouldToggleMainWindowFullScreenForCommandControlFShortcut( + flags: [.command, .control], + chars: "", + keyCode: 3, + layoutCharacterProvider: { _, _ in "u" } + ) + ) + } + + func testMatchesCommandControlFWhenCommandAwareLayoutTranslationProvidesF() { + XCTAssertTrue( + shouldToggleMainWindowFullScreenForCommandControlFShortcut( + flags: [.command, .control], + chars: "", + keyCode: 3, + layoutCharacterProvider: { _, modifierFlags in + modifierFlags.contains(.command) ? "f" : "u" + } + ) + ) + } + + func testMatchesCommandControlFWhenCharsAreControlSequence() { + XCTAssertTrue( + shouldToggleMainWindowFullScreenForCommandControlFShortcut( + flags: [.command, .control], + chars: "\u{06}", + keyCode: 3, + layoutCharacterProvider: { _, _ in nil } + ) + ) + } + + func testRejectsPhysicalFWhenCharacterRepresentsDifferentLayoutKey() { + XCTAssertFalse( + shouldToggleMainWindowFullScreenForCommandControlFShortcut( + flags: [.command, .control], + chars: "u", keyCode: 3 ) ) @@ -3349,16 +3395,19 @@ final class CommandPaletteSelectionScrollBehaviorTests: XCTestCase { } } -final class SidebarCommandHintPolicyTests: XCTestCase { - func testCommandHintRequiresCommandOnlyModifier() { +final class ShortcutHintModifierPolicyTests: XCTestCase { + func testShortcutHintRequiresEnabledCommandOnlyModifier() { withDefaultsSuite { defaults in defaults.set(true, forKey: ShortcutHintDebugSettings.showHintsOnCommandHoldKey) - XCTAssertTrue(SidebarCommandHintPolicy.shouldShowHints(for: [.command], defaults: defaults)) - XCTAssertFalse(SidebarCommandHintPolicy.shouldShowHints(for: [], defaults: defaults)) - XCTAssertFalse(SidebarCommandHintPolicy.shouldShowHints(for: [.command, .shift], defaults: defaults)) - XCTAssertFalse(SidebarCommandHintPolicy.shouldShowHints(for: [.command, .option], defaults: defaults)) - XCTAssertFalse(SidebarCommandHintPolicy.shouldShowHints(for: [.command, .control], defaults: defaults)) + XCTAssertTrue(ShortcutHintModifierPolicy.shouldShowHints(for: [.command], defaults: defaults)) + XCTAssertFalse(ShortcutHintModifierPolicy.shouldShowHints(for: [.control], defaults: defaults)) + XCTAssertFalse(ShortcutHintModifierPolicy.shouldShowHints(for: [], defaults: defaults)) + XCTAssertFalse(ShortcutHintModifierPolicy.shouldShowHints(for: [.command, .shift], defaults: defaults)) + XCTAssertFalse(ShortcutHintModifierPolicy.shouldShowHints(for: [.control, .shift], defaults: defaults)) + XCTAssertFalse(ShortcutHintModifierPolicy.shouldShowHints(for: [.command, .option], defaults: defaults)) + XCTAssertFalse(ShortcutHintModifierPolicy.shouldShowHints(for: [.control, .option], defaults: defaults)) + XCTAssertFalse(ShortcutHintModifierPolicy.shouldShowHints(for: [.command, .control], defaults: defaults)) } } @@ -3366,7 +3415,8 @@ final class SidebarCommandHintPolicyTests: XCTestCase { withDefaultsSuite { defaults in defaults.set(false, forKey: ShortcutHintDebugSettings.showHintsOnCommandHoldKey) - XCTAssertFalse(SidebarCommandHintPolicy.shouldShowHints(for: [.command], defaults: defaults)) + XCTAssertFalse(ShortcutHintModifierPolicy.shouldShowHints(for: [.command], defaults: defaults)) + XCTAssertFalse(ShortcutHintModifierPolicy.shouldShowHints(for: [.control], defaults: defaults)) } } @@ -3374,17 +3424,18 @@ final class SidebarCommandHintPolicyTests: XCTestCase { withDefaultsSuite { defaults in defaults.removeObject(forKey: ShortcutHintDebugSettings.showHintsOnCommandHoldKey) - XCTAssertTrue(SidebarCommandHintPolicy.shouldShowHints(for: [.command], defaults: defaults)) + XCTAssertTrue(ShortcutHintModifierPolicy.shouldShowHints(for: [.command], defaults: defaults)) + XCTAssertFalse(ShortcutHintModifierPolicy.shouldShowHints(for: [.control], defaults: defaults)) } } - func testCommandHintUsesIntentionalHoldDelay() { - XCTAssertGreaterThanOrEqual(SidebarCommandHintPolicy.intentionalHoldDelay, 0.25) + func testShortcutHintUsesIntentionalHoldDelay() { + XCTAssertEqual(ShortcutHintModifierPolicy.intentionalHoldDelay, 0.30, accuracy: 0.001) } func testCurrentWindowRequiresHostWindowToBeKeyAndMatchEventWindow() { XCTAssertTrue( - SidebarCommandHintPolicy.isCurrentWindow( + ShortcutHintModifierPolicy.isCurrentWindow( hostWindowNumber: 42, hostWindowIsKey: true, eventWindowNumber: 42, @@ -3393,7 +3444,7 @@ final class SidebarCommandHintPolicyTests: XCTestCase { ) XCTAssertFalse( - SidebarCommandHintPolicy.isCurrentWindow( + ShortcutHintModifierPolicy.isCurrentWindow( hostWindowNumber: 42, hostWindowIsKey: true, eventWindowNumber: 7, @@ -3402,7 +3453,7 @@ final class SidebarCommandHintPolicyTests: XCTestCase { ) XCTAssertFalse( - SidebarCommandHintPolicy.isCurrentWindow( + ShortcutHintModifierPolicy.isCurrentWindow( hostWindowNumber: 42, hostWindowIsKey: false, eventWindowNumber: 42, @@ -3411,12 +3462,12 @@ final class SidebarCommandHintPolicyTests: XCTestCase { ) } - func testWindowScopedCommandHintsUseKeyWindowWhenNoEventWindowIsAvailable() { + func testWindowScopedShortcutHintsUseKeyWindowWhenNoEventWindowIsAvailable() { withDefaultsSuite { defaults in defaults.set(true, forKey: ShortcutHintDebugSettings.showHintsOnCommandHoldKey) XCTAssertTrue( - SidebarCommandHintPolicy.shouldShowHints( + ShortcutHintModifierPolicy.shouldShowHints( for: [.command], hostWindowNumber: 42, hostWindowIsKey: true, @@ -3427,7 +3478,7 @@ final class SidebarCommandHintPolicyTests: XCTestCase { ) XCTAssertFalse( - SidebarCommandHintPolicy.shouldShowHints( + ShortcutHintModifierPolicy.shouldShowHints( for: [.command], hostWindowNumber: 42, hostWindowIsKey: true, @@ -3436,11 +3487,33 @@ final class SidebarCommandHintPolicyTests: XCTestCase { defaults: defaults ) ) + + XCTAssertTrue( + ShortcutHintModifierPolicy.shouldShowHints( + for: [.command], + hostWindowNumber: 42, + hostWindowIsKey: true, + eventWindowNumber: nil, + keyWindowNumber: 42, + defaults: defaults + ) + ) + + XCTAssertFalse( + ShortcutHintModifierPolicy.shouldShowHints( + for: [.control], + hostWindowNumber: 42, + hostWindowIsKey: true, + eventWindowNumber: nil, + keyWindowNumber: 42, + defaults: defaults + ) + ) } } private func withDefaultsSuite(_ body: (UserDefaults) -> Void) { - let suiteName = "SidebarCommandHintPolicyTests-\(UUID().uuidString)" + let suiteName = "ShortcutHintModifierPolicyTests-\(UUID().uuidString)" guard let defaults = UserDefaults(suiteName: suiteName) else { XCTFail("Failed to create defaults suite") return @@ -3490,6 +3563,31 @@ final class ShortcutHintDebugSettingsTests: XCTestCase { defaults.set(true, forKey: ShortcutHintDebugSettings.showHintsOnCommandHoldKey) XCTAssertTrue(ShortcutHintDebugSettings.showHintsOnCommandHoldEnabled(defaults: defaults)) } + + func testResetVisibilityDefaultsRestoresAlwaysShowAndCommandHoldFlags() { + let suiteName = "ShortcutHintDebugSettingsTests-\(UUID().uuidString)" + guard let defaults = UserDefaults(suiteName: suiteName) else { + XCTFail("Failed to create defaults suite") + return + } + + defaults.removePersistentDomain(forName: suiteName) + defer { defaults.removePersistentDomain(forName: suiteName) } + + defaults.set(true, forKey: ShortcutHintDebugSettings.alwaysShowHintsKey) + defaults.set(false, forKey: ShortcutHintDebugSettings.showHintsOnCommandHoldKey) + + ShortcutHintDebugSettings.resetVisibilityDefaults(defaults: defaults) + + XCTAssertEqual( + defaults.object(forKey: ShortcutHintDebugSettings.alwaysShowHintsKey) as? Bool, + ShortcutHintDebugSettings.defaultAlwaysShowHints + ) + XCTAssertEqual( + defaults.object(forKey: ShortcutHintDebugSettings.showHintsOnCommandHoldKey) as? Bool, + ShortcutHintDebugSettings.defaultShowHintsOnCommandHold + ) + } } final class ShortcutHintLanePlannerTests: XCTestCase { @@ -6658,6 +6756,8 @@ final class NotificationDockBadgeTests: XCTestCase { } override func tearDown() { + AppFocusState.overrideIsFocused = nil + AppDelegate.shared = nil TerminalNotificationStore.shared.resetNotificationSettingsPromptHooksForTesting() TerminalNotificationStore.shared.replaceNotificationsForTesting([]) super.tearDown() @@ -7161,6 +7261,155 @@ final class NotificationDockBadgeTests: XCTestCase { XCTAssertEqual(store.latestNotification(forTabId: tabB)?.id, notificationBUnread.id) } + func testFocusedTabNotificationIsStoredWhenNativeDeliveryIsSuppressed() { + let store = TerminalNotificationStore.shared + store.replaceNotificationsForTesting([]) + + let appDelegate = AppDelegate() + let tabManager = TabManager() + appDelegate.tabManager = tabManager + AppDelegate.shared = appDelegate + AppFocusState.overrideIsFocused = true + + guard let tabId = tabManager.selectedTabId else { + XCTFail("Expected selected tab for notification test") + return + } + + store.addNotification( + tabId: tabId, + surfaceId: nil, + title: "Needs input", + subtitle: "", + body: "agent requires user action" + ) + + XCTAssertEqual(store.unreadCount(forTabId: tabId), 1) + guard let latest = store.latestNotification(forTabId: tabId) else { + XCTFail("Expected notification to be stored for focused tab") + return + } + XCTAssertEqual(latest.tabId, tabId) + XCTAssertEqual(latest.title, "Needs input") + XCTAssertEqual(latest.body, "agent requires user action") + XCTAssertFalse(latest.isRead) + } + + func testApplicationDidBecomeActiveDoesNotMarkFocusedNotificationRead() { + let store = TerminalNotificationStore.shared + let appDelegate = AppDelegate() + let tabManager = TabManager() + appDelegate.tabManager = tabManager + appDelegate.notificationStore = store + AppDelegate.shared = appDelegate + AppFocusState.overrideIsFocused = true + + guard let tabId = tabManager.selectedTabId, + let surfaceId = tabManager.focusedSurfaceId(for: tabId) else { + XCTFail("Expected selected tab and focused surface for activation test") + return + } + + let notification = TerminalNotification( + id: UUID(), + tabId: tabId, + surfaceId: surfaceId, + title: "Unread", + subtitle: "", + body: "should persist across app activation", + createdAt: Date(), + isRead: false + ) + store.replaceNotificationsForTesting([notification]) + + appDelegate.applicationDidBecomeActive( + Notification(name: NSApplication.didBecomeActiveNotification) + ) + + XCTAssertTrue(store.hasUnreadNotification(forTabId: tabId, surfaceId: surfaceId)) + XCTAssertFalse(store.notifications[0].isRead) + } + + func testSelectingWorkspaceDoesNotMarkFocusedNotificationRead() { + let store = TerminalNotificationStore.shared + let appDelegate = AppDelegate() + let tabManager = TabManager() + appDelegate.tabManager = tabManager + appDelegate.notificationStore = store + AppDelegate.shared = appDelegate + AppFocusState.overrideIsFocused = true + + guard let originalTabId = tabManager.selectedTabId, + let originalSurfaceId = tabManager.focusedSurfaceId(for: originalTabId) else { + XCTFail("Expected selected tab and focused surface for workspace selection test") + return + } + guard let originalWorkspace = tabManager.tabs.first(where: { $0.id == originalTabId }) else { + XCTFail("Expected original workspace for workspace selection test") + return + } + + let notification = TerminalNotification( + id: UUID(), + tabId: originalTabId, + surfaceId: originalSurfaceId, + title: "Unread", + subtitle: "", + body: "should persist across workspace selection", + createdAt: Date(), + isRead: false + ) + store.replaceNotificationsForTesting([notification]) + + _ = tabManager.addWorkspace(select: true) + tabManager.selectWorkspace(originalWorkspace) + + let drained = expectation(description: "workspace selection side effects drained") + DispatchQueue.main.async { drained.fulfill() } + wait(for: [drained], timeout: 1.0) + + XCTAssertEqual(tabManager.selectedTabId, originalTabId) + XCTAssertTrue(store.hasUnreadNotification(forTabId: originalTabId, surfaceId: originalSurfaceId)) + XCTAssertFalse(store.notifications[0].isRead) + } + + func testNotificationFocusNavigationDoesNotMarkNotificationRead() { + let store = TerminalNotificationStore.shared + let appDelegate = AppDelegate() + let tabManager = TabManager() + appDelegate.tabManager = tabManager + appDelegate.notificationStore = store + AppDelegate.shared = appDelegate + AppFocusState.overrideIsFocused = true + + guard let tabId = tabManager.selectedTabId, + let surfaceId = tabManager.focusedSurfaceId(for: tabId) else { + XCTFail("Expected selected tab and focused surface for notification focus test") + return + } + + let notification = TerminalNotification( + id: UUID(), + tabId: tabId, + surfaceId: surfaceId, + title: "Unread", + subtitle: "", + body: "should persist after notification focus", + createdAt: Date(), + isRead: false + ) + store.replaceNotificationsForTesting([notification]) + + tabManager.focusTabFromNotification(tabId, surfaceId: surfaceId) + + let drained = expectation(description: "notification focus drained") + DispatchQueue.main.async { drained.fulfill() } + wait(for: [drained], timeout: 1.0) + + XCTAssertTrue(store.hasUnreadNotification(forTabId: tabId, surfaceId: surfaceId)) + XCTAssertFalse(store.notifications[0].isRead) + } + func testNotificationIndexesUpdateAfterReadAndClearMutations() { let tab = UUID() let surfaceUnread = UUID() @@ -10483,6 +10732,7 @@ final class TerminalControllerSocketListenerHealthTests: XCTestCase { return fd } + @MainActor func testSocketListenerHealthRecognizesSocketPath() throws { let path = makeTempSocketPath() let fd = try bindUnixSocket(at: path) @@ -10496,6 +10746,7 @@ final class TerminalControllerSocketListenerHealthTests: XCTestCase { XCTAssertFalse(health.isHealthy) } + @MainActor func testSocketListenerHealthRejectsRegularFile() throws { let path = makeTempSocketPath() let url = URL(fileURLWithPath: path) @@ -10512,10 +10763,16 @@ final class TerminalControllerSocketListenerHealthTests: XCTestCase { isRunning: true, acceptLoopAlive: true, socketPathMatches: true, - socketPathExists: true + socketPathExists: true, + socketProbePerformed: true, + socketConnectable: true, + socketConnectErrno: nil ) XCTAssertTrue(health.isHealthy) - XCTAssertEqual(health.failureSignals, []) + XCTAssertTrue(health.failureSignals.isEmpty) + XCTAssertTrue(health.socketProbePerformed) + XCTAssertEqual(health.socketConnectable, true) + XCTAssertNil(health.socketConnectErrno) } func testSocketListenerHealthFailureSignalsIncludeAllDetectedProblems() { @@ -10523,9 +10780,15 @@ final class TerminalControllerSocketListenerHealthTests: XCTestCase { isRunning: false, acceptLoopAlive: false, socketPathMatches: false, - socketPathExists: false + socketPathExists: false, + socketProbePerformed: false, + socketConnectable: nil, + socketConnectErrno: nil ) XCTAssertFalse(health.isHealthy) + XCTAssertFalse(health.socketProbePerformed) + XCTAssertNil(health.socketConnectable) + XCTAssertNil(health.socketConnectErrno) XCTAssertEqual( health.failureSignals, ["not_running", "accept_loop_dead", "socket_path_mismatch", "socket_missing"] diff --git a/tests/test_focus_notification_dismiss.py b/tests/test_focus_notification_dismiss.py index d7569b65..14cef434 100755 --- a/tests/test_focus_notification_dismiss.py +++ b/tests/test_focus_notification_dismiss.py @@ -1,6 +1,6 @@ #!/usr/bin/env python3 """ -E2E: focusing a panel clears its notification and triggers a flash. +E2E: focusing a panel preserves its notification and triggers a flash. Note: This uses the socket focus command (no assistive access needed). """ @@ -74,8 +74,12 @@ def main() -> int: client.send("x") time.sleep(0.2) - if not wait_for_notification(client, surface_id, is_read=True, timeout=2.0): - print("FAIL: Notification did not become read after focus") + if wait_for_notification(client, surface_id, is_read=True, timeout=2.0): + print("FAIL: Notification became read after focus") + return 1 + items = client.list_notifications() + if not any(item["surface_id"] == surface_id and not item["is_read"] for item in items): + print("FAIL: Notification did not remain present and unread after focus") return 1 final_flash = client.flash_count(term_b) @@ -93,7 +97,7 @@ def main() -> int: except Exception: pass - print("PASS: Focus clears notification and flashes panel") + print("PASS: Focus preserves notification and flashes panel") return 0 except (cmuxError, RuntimeError) as exc: print(f"FAIL: {exc}") diff --git a/tests/test_issue_952_socket_listener_recovery.py b/tests/test_issue_952_socket_listener_recovery.py new file mode 100644 index 00000000..46a83644 --- /dev/null +++ b/tests/test_issue_952_socket_listener_recovery.py @@ -0,0 +1,162 @@ +#!/usr/bin/env python3 +"""Regression guard for issue #952 (flaky CLI socket connections).""" + +from __future__ import annotations + +import re +import shutil +import subprocess +from pathlib import Path + + +def get_repo_root() -> Path: + """Return the repository root for source inspections.""" + fallback_root = Path(__file__).resolve().parents[1] + git_path = shutil.which("git") + if git_path is None: + return fallback_root + + try: + result = subprocess.run( + [git_path, "rev-parse", "--show-toplevel"], + capture_output=True, + text=True, + check=False, + ) + except OSError: + return fallback_root + if result.returncode == 0: + return Path(result.stdout.strip()) + return fallback_root + + +def require(content: str, needle: str, message: str, failures: list[str], *, regex: bool = False) -> None: + """Record a failure when a required source pattern is missing.""" + matched = re.search(needle, content, re.MULTILINE) is not None if regex else needle in content + if not matched: + failures.append(message) + + +def collect_failures() -> list[str]: + """Collect missing source-level guards for the socket listener recovery fix.""" + repo_root = get_repo_root() + terminal_controller_path = repo_root / "Sources" / "TerminalController.swift" + app_delegate_path = repo_root / "Sources" / "AppDelegate.swift" + failures: list[str] = [] + + missing_paths = [ + str(path) for path in [terminal_controller_path, app_delegate_path] if not path.exists() + ] + if missing_paths: + for path in missing_paths: + failures.append(f"Missing expected file: {path}") + return failures + + terminal_controller = terminal_controller_path.read_text(encoding="utf-8") + app_delegate = app_delegate_path.read_text(encoding="utf-8") + + require( + terminal_controller, + "let socketProbePerformed: Bool", + "Socket health snapshot no longer tracks whether connectability was probed", + failures, + ) + require( + terminal_controller, + "let socketConnectable: Bool?", + "Socket health snapshot no longer distinguishes unprobed vs connectable sockets", + failures, + ) + require( + terminal_controller, + "let socketConnectErrno: Int32?", + "Socket health snapshot no longer preserves probe errno", + failures, + ) + require( + terminal_controller, + "signals.append(\"socket_unreachable\")", + "Socket health failures no longer flag unreachable listeners", + failures, + ) + require( + terminal_controller, + r"private\s+nonisolated\s+static\s+func\s+probeSocketConnectability\s*\(\s*path:\s*String\s*\)", + "Missing active socket connectability probe helper", + failures, + regex=True, + ) + require( + terminal_controller, + r"connect\s*\(\s*probeSocket\s*,\s*sockaddrPtr\s*,\s*socklen_t\s*\(\s*MemoryLayout\.size\s*\)\s*\)", + "Socket health probe no longer performs a real connect() check", + failures, + regex=True, + ) + require( + terminal_controller, + "stage: \"bind_path_too_long\"", + "Socket listener start no longer reports overlong Unix socket paths", + failures, + ) + require( + terminal_controller, + "Self.unixSocketPathMaxLength", + "Socket listener path-length telemetry was removed", + failures, + ) + + require( + app_delegate, + "private static let socketListenerHealthCheckInterval: DispatchTimeInterval = .seconds(2)", + "Socket health timer interval drifted from the fast recovery setting", + failures, + ) + require( + app_delegate, + "\"socketProbePerformed\": health.socketProbePerformed ? 1 : 0", + "Health telemetry no longer records whether a connectability probe ran", + failures, + ) + require( + app_delegate, + "if let socketConnectable = health.socketConnectable {", + "Health telemetry no longer gates connectability on an actual probe result", + failures, + ) + require( + app_delegate, + "data[\"socketConnectable\"] = socketConnectable ? 1 : 0", + "Health telemetry no longer includes connectability when a probe ran", + failures, + ) + require( + app_delegate, + "if let socketConnectErrno = health.socketConnectErrno {", + "Health telemetry no longer records connect probe errno when available", + failures, + ) + return failures + + +def test_issue_952_socket_listener_recovery() -> None: + """Keep the source-level recovery guards for issue #952 in CI.""" + failures = collect_failures() + assert not failures, "issue #952 regression(s) detected:\n- " + "\n- ".join(failures) + + +def main() -> int: + """Run the regression guard without requiring pytest to be installed.""" + failures = collect_failures() + if failures: + print("FAIL: issue #952 regression(s) detected") + for failure in failures: + print(f"- {failure}") + return 1 + + print("PASS: issue #952 socket listener recovery guards are present") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/tests/test_notifications.py b/tests/test_notifications.py index 1ac25c4b..23b4bf10 100644 --- a/tests/test_notifications.py +++ b/tests/test_notifications.py @@ -58,6 +58,15 @@ def wait_for_flash_count(client: cmux, surface: str, minimum: int = 1, timeout: return last +def wait_for_current_workspace(client: cmux, expected: str, timeout: float = 2.0) -> bool: + start = time.time() + while time.time() - start < timeout: + if client.current_workspace() == expected: + return True + time.sleep(0.05) + return client.current_workspace() == expected + + def ensure_two_surfaces(client: cmux) -> list[tuple[int, str, bool]]: surfaces = client.list_surfaces() if len(surfaces) < 2: @@ -215,8 +224,8 @@ def test_rxvt_notification_osc777(client: cmux) -> TestResult: return result -def test_mark_read_on_focus_change(client: cmux) -> TestResult: - result = TestResult("Mark Read On Panel Focus") +def test_preserve_unread_on_focus_change(client: cmux) -> TestResult: + result = TestResult("Preserve Unread On Panel Focus") try: client.clear_notifications() client.reset_flash_counts() @@ -229,81 +238,88 @@ def test_mark_read_on_focus_change(client: cmux) -> TestResult: client.set_app_focus(False) client.notify_surface(other[0], "focusread") - time.sleep(0.1) + items = wait_for_notifications(client, 1) + target = next((n for n in items if n["surface_id"] == other[1]), None) + if target is None or target["is_read"]: + result.failure("Expected unread notification for target surface before focus") + return result client.set_app_focus(True) client.focus_surface(other[0]) - time.sleep(0.1) + count = wait_for_flash_count(client, other[1], minimum=1, timeout=2.0) + if count < 1: + result.failure("Expected flash on panel focus") + return result items = client.list_notifications() target = next((n for n in items if n["surface_id"] == other[1]), None) if target is None: result.failure("Expected notification for target surface") - elif not target["is_read"]: - result.failure("Expected notification to be marked read on focus") + elif target["is_read"]: + result.failure("Expected notification to remain unread on focus") else: - count = wait_for_flash_count(client, other[1], minimum=1, timeout=2.0) - if count < 1: - result.failure("Expected flash on panel focus dismissal") - else: - result.success("Notification marked read on focus") + result.success("Notification persisted across panel focus") except Exception as e: result.failure(f"Exception: {e}") return result -def test_mark_read_on_app_active(client: cmux) -> TestResult: - result = TestResult("Mark Read On App Active") +def test_preserve_unread_on_app_active(client: cmux) -> TestResult: + result = TestResult("Preserve Unread On App Active") try: client.clear_notifications() client.set_app_focus(False) client.notify("activate") - time.sleep(0.1) - - items = client.list_notifications() + items = wait_for_notifications(client, 1) if not items or items[0]["is_read"]: result.failure("Expected unread notification before activation") return result client.simulate_app_active() - time.sleep(0.1) - - items = client.list_notifications() + items = wait_for_notifications(client, 1) if not items: result.failure("Expected notification to remain after activation") - elif not items[0]["is_read"]: - result.failure("Expected notification to be marked read on app active") + elif items[0]["is_read"]: + result.failure("Expected notification to remain unread on app active") else: - result.success("Notification marked read on app active") + result.success("Notification persisted across app activation") except Exception as e: result.failure(f"Exception: {e}") return result -def test_mark_read_on_tab_switch(client: cmux) -> TestResult: - result = TestResult("Mark Read On Tab Switch") +def test_preserve_unread_on_tab_switch(client: cmux) -> TestResult: + result = TestResult("Preserve Unread On Tab Switch") try: client.clear_notifications() client.set_app_focus(False) tab1 = client.current_workspace() client.notify("tabswitch") - time.sleep(0.1) + items = wait_for_notifications(client, 1) + target = next((n for n in items if n["workspace_id"] == tab1), None) + if target is None or target["is_read"]: + result.failure("Expected unread notification for original tab before switching") + return result tab2 = client.new_workspace() - time.sleep(0.1) + if not wait_for_current_workspace(client, tab2): + result.failure("Expected new workspace to become selected") + return result client.set_app_focus(True) client.select_workspace(tab1) - time.sleep(0.1) + if not wait_for_current_workspace(client, tab1): + result.failure("Expected original workspace to become selected again") + return result - items = client.list_notifications() + items = wait_for_notifications(client, 1) target = next((n for n in items if n["workspace_id"] == tab1), None) if target is None: result.failure("Expected notification for original tab") - elif not target["is_read"]: - result.failure("Expected notification to be marked read on tab switch") + elif target["is_read"]: + result.failure("Expected notification to remain unread on tab switch") else: - result.success("Notification marked read on tab switch") + result.success("Notification persisted across tab switch") except Exception as e: result.failure(f"Exception: {e}") return result @@ -371,11 +387,20 @@ def test_focus_on_notification_click(client: cmux) -> TestResult: result.failure("Expected notification surface to be focused") return result + items = client.list_notifications() + notification = next((n for n in items if n["surface_id"] == other[1]), None) + if notification is None: + result.failure("Expected notification to remain listed after notification click") + return result + if notification["is_read"]: + result.failure("Expected notification click to preserve unread state") + return result + count = wait_for_flash_count(client, other[1], minimum=1, timeout=2.0) if count < 1: result.failure(f"Expected flash count >= 1, got {count}") else: - result.success("Notification click focuses and flashes panel") + result.success("Notification click focuses, flashes, and preserves unread state") except Exception as e: result.failure(f"Exception: {e}") return result @@ -455,9 +480,9 @@ def run_tests() -> int: results.append(test_kitty_notification_simple(client)) results.append(test_kitty_notification_chunked(client)) results.append(test_rxvt_notification_osc777(client)) - results.append(test_mark_read_on_focus_change(client)) - results.append(test_mark_read_on_app_active(client)) - results.append(test_mark_read_on_tab_switch(client)) + results.append(test_preserve_unread_on_focus_change(client)) + results.append(test_preserve_unread_on_app_active(client)) + results.append(test_preserve_unread_on_tab_switch(client)) results.append(test_flash_on_tab_switch(client)) results.append(test_focus_on_notification_click(client)) results.append(test_restore_focus_on_tab_switch(client)) diff --git a/tests_v2/test_browser_cli_agent_port.py b/tests_v2/test_browser_cli_agent_port.py index 5e33e9b7..a4ca9a94 100644 --- a/tests_v2/test_browser_cli_agent_port.py +++ b/tests_v2/test_browser_cli_agent_port.py @@ -203,6 +203,12 @@ def main() -> int: snapshot_text = _run_cli_text(cli, ["browser", surface, "snapshot", "--interactive"]) _must("ref=e" in snapshot_text, f"Expected snapshot text with refs from CLI: {snapshot_text!r}") + blank_opened = _run_cli_json(cli, ["browser", "open", "about:blank", "--workspace", workspace]) + blank_surface = str(blank_opened.get("surface_ref") or blank_opened.get("surface_id") or "") + _must(bool(blank_surface), f"Expected about:blank browser open to return a surface: {blank_opened}") + blank_snapshot = _run_cli_text(cli, ["browser", blank_surface, "snapshot", "--interactive"]) + _must("about:blank" in blank_snapshot and "get url" in blank_snapshot, f"Expected empty snapshot diagnostics for about:blank: {blank_snapshot!r}") + opened_routed = _run_cli_json(cli, ["browser", "open", page_url, "--workspace", workspace]) routed_surface = str(opened_routed.get("surface_ref") or opened_routed.get("surface_id") or "") _must(bool(routed_surface), f"browser open --workspace returned no surface handle: {opened_routed}") diff --git a/tests_v2/test_browser_cli_wait_and_screenshot.py b/tests_v2/test_browser_cli_wait_and_screenshot.py new file mode 100644 index 00000000..fb4d2fb7 --- /dev/null +++ b/tests_v2/test_browser_cli_wait_and_screenshot.py @@ -0,0 +1,146 @@ +#!/usr/bin/env python3 +"""Regression: browser wait/snapshot and screenshot CLI return usable file locations.""" + +import glob +import json +import os +import subprocess +import sys +import tempfile +import urllib.parse +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).parent)) +from cmux import cmux, cmuxError + + +SOCKET_PATH = os.environ.get("CMUX_SOCKET", "/tmp/cmux-debug.sock") + + +def _must(cond: bool, msg: str) -> None: + if not cond: + raise cmuxError(msg) + + +def _find_cli_binary() -> str: + env_cli = os.environ.get("CMUXTERM_CLI") + if env_cli and os.path.isfile(env_cli) and os.access(env_cli, os.X_OK): + return env_cli + + fixed = os.path.expanduser( + "~/Library/Developer/Xcode/DerivedData/cmux-tests-v2/Build/Products/Debug/cmux" + ) + if os.path.isfile(fixed) and os.access(fixed, os.X_OK): + return fixed + + candidates = glob.glob( + os.path.expanduser("~/Library/Developer/Xcode/DerivedData/**/Build/Products/Debug/cmux"), + recursive=True, + ) + candidates += glob.glob("/tmp/cmux-*/Build/Products/Debug/cmux") + candidates = [p for p in candidates if os.path.isfile(p) and os.access(p, os.X_OK)] + if not candidates: + raise cmuxError("Could not locate cmux CLI binary; set CMUXTERM_CLI") + candidates.sort(key=lambda p: os.path.getmtime(p), reverse=True) + return candidates[0] + + +def _run_cli(cli: str, *args: str) -> subprocess.CompletedProcess[str]: + cmd = [cli, "--socket", SOCKET_PATH, *args] + proc = subprocess.run(cmd, capture_output=True, text=True, check=False) + if proc.returncode != 0: + merged = f"{proc.stdout}\n{proc.stderr}".strip() + raise cmuxError(f"CLI failed ({' '.join(cmd)}): {merged}") + return proc + + +def main() -> int: + cli = _find_cli_binary() + + with cmux(SOCKET_PATH) as c: + opened = c._call("browser.open_split", {"url": "about:blank"}) or {} + target = str(opened.get("surface_id") or opened.get("surface_ref") or "") + _must(target != "", f"browser.open_split returned no surface handle: {opened}") + + html = """ + + + cmux-browser-cli-regression + +
+

browser cli regression

+

ready

+
+ + +""".strip() + data_url = "data:text/html;charset=utf-8," + urllib.parse.quote(html) + c._call("browser.navigate", {"surface_id": target, "url": data_url}) + + wait_proc = _run_cli( + cli, + "browser", + target, + "wait", + "--load-state", + "interactive", + "--timeout-ms", + "5000", + ) + _must(wait_proc.stdout.strip() == "OK", f"Expected browser wait OK output: {wait_proc.stdout!r}") + + snapshot_payload = c._call("browser.snapshot", {"surface_id": target}) or {} + refs = snapshot_payload.get("refs") or {} + _must(isinstance(refs, dict) and len(refs) > 0, f"Expected snapshot refs for ref-based wait coverage: {snapshot_payload}") + ref_selector = str(next(iter(refs.keys()))) + ref_wait_proc = _run_cli( + cli, + "browser", + target, + "wait", + "--selector", + ref_selector, + "--timeout-ms", + "2000", + ) + _must(ref_wait_proc.stdout.strip() == "OK", f"Expected browser wait to resolve snapshot refs: {ref_wait_proc.stdout!r}") + + snapshot_proc = _run_cli(cli, "browser", target, "snapshot", "--compact") + _must( + snapshot_proc.stdout.strip().startswith("- document"), + f"Expected snapshot command to succeed with structured output: {snapshot_proc.stdout!r}", + ) + + screenshot_json_proc = _run_cli(cli, "browser", target, "screenshot", "--json") + screenshot_json_text = screenshot_json_proc.stdout.strip() + payload = json.loads(screenshot_json_text or "{}") + + _must("\\/" not in screenshot_json_text, f"Expected screenshot JSON without escaped slashes: {screenshot_json_text!r}") + _must("png_base64" not in payload, f"Expected screenshot JSON to omit png_base64 when file location is available: {payload}") + + screenshot_path = str(payload.get("path") or "") + screenshot_url = str(payload.get("url") or "") + _must(screenshot_path.startswith("/"), f"Expected screenshot path in JSON payload: {payload}") + _must(screenshot_url.startswith("file://"), f"Expected screenshot file URL in JSON payload: {payload}") + _must(Path(screenshot_path).is_file(), f"Expected screenshot file to exist: {payload}") + + out_dir = Path(tempfile.mkdtemp(prefix="cmux-browser-screenshot-cli-")) / "nested" / "dir" + out_path = out_dir / "capture.png" + screenshot_out_proc = _run_cli( + cli, + "browser", + target, + "screenshot", + "--out", + str(out_path), + ) + _must(screenshot_out_proc.stdout.strip() == f"OK {out_path}", f"Expected --out to print the requested path: {screenshot_out_proc.stdout!r}") + _must("file://" not in screenshot_out_proc.stdout, f"Expected --out to print a path, not a file URL: {screenshot_out_proc.stdout!r}") + _must(out_path.is_file(), f"Expected --out screenshot file to exist: {out_path}") + + print("PASS: browser CLI wait/snapshot and screenshot output work end-to-end") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/tests_v2/test_focus_notification_dismiss.py b/tests_v2/test_focus_notification_dismiss.py index d7569b65..14cef434 100755 --- a/tests_v2/test_focus_notification_dismiss.py +++ b/tests_v2/test_focus_notification_dismiss.py @@ -1,6 +1,6 @@ #!/usr/bin/env python3 """ -E2E: focusing a panel clears its notification and triggers a flash. +E2E: focusing a panel preserves its notification and triggers a flash. Note: This uses the socket focus command (no assistive access needed). """ @@ -74,8 +74,12 @@ def main() -> int: client.send("x") time.sleep(0.2) - if not wait_for_notification(client, surface_id, is_read=True, timeout=2.0): - print("FAIL: Notification did not become read after focus") + if wait_for_notification(client, surface_id, is_read=True, timeout=2.0): + print("FAIL: Notification became read after focus") + return 1 + items = client.list_notifications() + if not any(item["surface_id"] == surface_id and not item["is_read"] for item in items): + print("FAIL: Notification did not remain present and unread after focus") return 1 final_flash = client.flash_count(term_b) @@ -93,7 +97,7 @@ def main() -> int: except Exception: pass - print("PASS: Focus clears notification and flashes panel") + print("PASS: Focus preserves notification and flashes panel") return 0 except (cmuxError, RuntimeError) as exc: print(f"FAIL: {exc}") diff --git a/tests_v2/test_notifications.py b/tests_v2/test_notifications.py index 1ac25c4b..23b4bf10 100644 --- a/tests_v2/test_notifications.py +++ b/tests_v2/test_notifications.py @@ -58,6 +58,15 @@ def wait_for_flash_count(client: cmux, surface: str, minimum: int = 1, timeout: return last +def wait_for_current_workspace(client: cmux, expected: str, timeout: float = 2.0) -> bool: + start = time.time() + while time.time() - start < timeout: + if client.current_workspace() == expected: + return True + time.sleep(0.05) + return client.current_workspace() == expected + + def ensure_two_surfaces(client: cmux) -> list[tuple[int, str, bool]]: surfaces = client.list_surfaces() if len(surfaces) < 2: @@ -215,8 +224,8 @@ def test_rxvt_notification_osc777(client: cmux) -> TestResult: return result -def test_mark_read_on_focus_change(client: cmux) -> TestResult: - result = TestResult("Mark Read On Panel Focus") +def test_preserve_unread_on_focus_change(client: cmux) -> TestResult: + result = TestResult("Preserve Unread On Panel Focus") try: client.clear_notifications() client.reset_flash_counts() @@ -229,81 +238,88 @@ def test_mark_read_on_focus_change(client: cmux) -> TestResult: client.set_app_focus(False) client.notify_surface(other[0], "focusread") - time.sleep(0.1) + items = wait_for_notifications(client, 1) + target = next((n for n in items if n["surface_id"] == other[1]), None) + if target is None or target["is_read"]: + result.failure("Expected unread notification for target surface before focus") + return result client.set_app_focus(True) client.focus_surface(other[0]) - time.sleep(0.1) + count = wait_for_flash_count(client, other[1], minimum=1, timeout=2.0) + if count < 1: + result.failure("Expected flash on panel focus") + return result items = client.list_notifications() target = next((n for n in items if n["surface_id"] == other[1]), None) if target is None: result.failure("Expected notification for target surface") - elif not target["is_read"]: - result.failure("Expected notification to be marked read on focus") + elif target["is_read"]: + result.failure("Expected notification to remain unread on focus") else: - count = wait_for_flash_count(client, other[1], minimum=1, timeout=2.0) - if count < 1: - result.failure("Expected flash on panel focus dismissal") - else: - result.success("Notification marked read on focus") + result.success("Notification persisted across panel focus") except Exception as e: result.failure(f"Exception: {e}") return result -def test_mark_read_on_app_active(client: cmux) -> TestResult: - result = TestResult("Mark Read On App Active") +def test_preserve_unread_on_app_active(client: cmux) -> TestResult: + result = TestResult("Preserve Unread On App Active") try: client.clear_notifications() client.set_app_focus(False) client.notify("activate") - time.sleep(0.1) - - items = client.list_notifications() + items = wait_for_notifications(client, 1) if not items or items[0]["is_read"]: result.failure("Expected unread notification before activation") return result client.simulate_app_active() - time.sleep(0.1) - - items = client.list_notifications() + items = wait_for_notifications(client, 1) if not items: result.failure("Expected notification to remain after activation") - elif not items[0]["is_read"]: - result.failure("Expected notification to be marked read on app active") + elif items[0]["is_read"]: + result.failure("Expected notification to remain unread on app active") else: - result.success("Notification marked read on app active") + result.success("Notification persisted across app activation") except Exception as e: result.failure(f"Exception: {e}") return result -def test_mark_read_on_tab_switch(client: cmux) -> TestResult: - result = TestResult("Mark Read On Tab Switch") +def test_preserve_unread_on_tab_switch(client: cmux) -> TestResult: + result = TestResult("Preserve Unread On Tab Switch") try: client.clear_notifications() client.set_app_focus(False) tab1 = client.current_workspace() client.notify("tabswitch") - time.sleep(0.1) + items = wait_for_notifications(client, 1) + target = next((n for n in items if n["workspace_id"] == tab1), None) + if target is None or target["is_read"]: + result.failure("Expected unread notification for original tab before switching") + return result tab2 = client.new_workspace() - time.sleep(0.1) + if not wait_for_current_workspace(client, tab2): + result.failure("Expected new workspace to become selected") + return result client.set_app_focus(True) client.select_workspace(tab1) - time.sleep(0.1) + if not wait_for_current_workspace(client, tab1): + result.failure("Expected original workspace to become selected again") + return result - items = client.list_notifications() + items = wait_for_notifications(client, 1) target = next((n for n in items if n["workspace_id"] == tab1), None) if target is None: result.failure("Expected notification for original tab") - elif not target["is_read"]: - result.failure("Expected notification to be marked read on tab switch") + elif target["is_read"]: + result.failure("Expected notification to remain unread on tab switch") else: - result.success("Notification marked read on tab switch") + result.success("Notification persisted across tab switch") except Exception as e: result.failure(f"Exception: {e}") return result @@ -371,11 +387,20 @@ def test_focus_on_notification_click(client: cmux) -> TestResult: result.failure("Expected notification surface to be focused") return result + items = client.list_notifications() + notification = next((n for n in items if n["surface_id"] == other[1]), None) + if notification is None: + result.failure("Expected notification to remain listed after notification click") + return result + if notification["is_read"]: + result.failure("Expected notification click to preserve unread state") + return result + count = wait_for_flash_count(client, other[1], minimum=1, timeout=2.0) if count < 1: result.failure(f"Expected flash count >= 1, got {count}") else: - result.success("Notification click focuses and flashes panel") + result.success("Notification click focuses, flashes, and preserves unread state") except Exception as e: result.failure(f"Exception: {e}") return result @@ -455,9 +480,9 @@ def run_tests() -> int: results.append(test_kitty_notification_simple(client)) results.append(test_kitty_notification_chunked(client)) results.append(test_rxvt_notification_osc777(client)) - results.append(test_mark_read_on_focus_change(client)) - results.append(test_mark_read_on_app_active(client)) - results.append(test_mark_read_on_tab_switch(client)) + results.append(test_preserve_unread_on_focus_change(client)) + results.append(test_preserve_unread_on_app_active(client)) + results.append(test_preserve_unread_on_tab_switch(client)) results.append(test_flash_on_tab_switch(client)) results.append(test_focus_on_notification_click(client)) results.append(test_restore_focus_on_tab_switch(client))