diff --git a/.github/workflows/ci-macos.yml b/.github/workflows/ci-macos.yml index abcd0d19fe28..2c00c6a3aa2f 100644 --- a/.github/workflows/ci-macos.yml +++ b/.github/workflows/ci-macos.yml @@ -2052,8 +2052,10 @@ jobs: set -euo pipefail ./tests/test_bundled_ghostty_theme_picker_helper.sh + # The notification semantics step below resolves `bun` too, and it runs + # on the focused regression shard, so Bun must be set up there as well. - name: Set up Bun for Pi extension dispatch regression - if: ${{ matrix.shard == fromJSON(env.CMUX_APP_HOST_CLI_REGRESSION_SHARD) }} + if: ${{ matrix.shard == fromJSON(env.CMUX_APP_HOST_CLI_REGRESSION_SHARD) || matrix.shard == fromJSON(env.CMUX_APP_HOST_FOCUSED_REGRESSION_SHARD) || contains(inputs.unit_strict_steps, '|Run agent notification semantics|') }} uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2 with: bun-version: "1.3.6" diff --git a/.github/workflows/command-palette-search-benchmarks.yml b/.github/workflows/command-palette-search-benchmarks.yml new file mode 100644 index 000000000000..b9c165e97469 --- /dev/null +++ b/.github/workflows/command-palette-search-benchmarks.yml @@ -0,0 +1,152 @@ +name: command palette search benchmarks + +# The wall-clock command-palette search benchmarks are gated out of the sharded +# app-host unit suite by CMUX_COMMAND_PALETTE_SEARCH_BENCHMARKS (see +# skipUnlessCommandPaletteSearchBenchmarksAreEnabled in +# cmuxTests/CommandPaletteNucleoFixtures.swift). A gate with no caller at all is +# deletion with extra steps: the benchmarks keep compiling and stop running. +# This workflow is that caller -- dispatch-only while automatic CI is paused, so +# running the benchmarks is a deliberate act rather than a daily cost. It runs +# the one script that owns the gate, +# scripts/test-command-palette-nucleo-ffi.sh, which enables the env var and +# names both benchmark classes; the script's own BENCH assertions fail the job +# if any gated benchmark silently skipped. +on: + # Temporarily manual-only beginning 2026-07-13 to pause automatic CI. + # tmux-corpus.yml and perf-activation.yml -- the two closest macOS + # benchmark nightlies -- both carry this and have their crons removed. + # A new cron here would quietly reverse that reduction, so the gate's + # caller is dispatch-only until the pause lifts. + workflow_dispatch: + inputs: + runner: + description: macOS runner (auto follows MACOS_RUNNER_15) + required: false + default: auto + type: choice + options: + - auto + - blacksmith-6vcpu-macos-15 + - blacksmith-6vcpu-macos-26 + - blacksmith-6vcpu-macos-latest + +permissions: + contents: read + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: false + +jobs: + command-palette-search-benchmarks: + runs-on: ${{ github.repository_owner != 'manaflow-ai' && 'macos-26' || ((!inputs.runner || inputs.runner == 'auto') && (vars.CI_PAID_MACOS_OVERFLOW == '1' && vars.MACOS_RUNNER_15 || 'blacksmith-6vcpu-macos-15') || inputs.runner) }} + timeout-minutes: 60 + env: + # XCTest app-host crashes can leave xcodebuild waiting in Swift's crash + # backtracer until the job timeout. Keep crash handling non-interactive + # and cheap so xcodebuild can restart/finish the suite. + SWIFT_BACKTRACE: "interactive=no,timeout=0s,symbolicate=off,color=no" + # Declared here, not in a step, so the always() summary and upload steps + # still have a path when an earlier step fails. + CMUX_NUCLEO_FFI_LOG: ${{ github.workspace }}/command-palette-search-benchmarks.log + steps: + - name: Clear stale git locks (self-hosted reused workspace) + shell: bash + run: | + # Self-hosted macOS runners reuse the workspace. A job cancelled or + # killed mid-checkout can leave a stale .git/modules/*/index.lock that + # fails every later submodule checkout (e.g. ghostty). Clear them first. + ws="${GITHUB_WORKSPACE:-$PWD}" + rm -f "$ws/.git/index.lock" 2>/dev/null || true + if [ -d "$ws/.git/modules" ]; then + find "$ws/.git/modules" -type f -name "*.lock" -delete 2>/dev/null || true + fi + + - name: Checkout + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + persist-credentials: false + submodules: recursive + + - name: Select Xcode + run: | + set -euo pipefail + ./scripts/select-ci-xcode.sh + + - name: Setup Bun + uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2 + with: + bun-version: "1.3.14" + + - name: Cache GhosttyKit.xcframework + id: cache-ghosttykit + uses: actions/cache@27d5ce7f107fe9357f9df03efb73ab90386fccae # v5.0.5 + with: + path: GhosttyKit.xcframework + key: ghosttykit-sentry-off-v1-${{ hashFiles('.gitmodules', 'ghostty/**') }} + + - name: Download pre-built GhosttyKit.xcframework + if: steps.cache-ghosttykit.outputs.cache-hit != 'true' + run: ./scripts/download-prebuilt-ghosttykit.sh + + - name: Install zig + run: ./scripts/install-zig-ci.sh + + - name: Install Rust + run: ./scripts/install-rust-ci.sh + + - name: Cache Swift packages + uses: actions/cache@27d5ce7f107fe9357f9df03efb73ab90386fccae # v5.0.5 + with: + path: .ci-source-packages + key: command-palette-bench-spm-${{ hashFiles('cmux.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved') }} + restore-keys: command-palette-bench-spm- + + - name: Sanitize Swift package cache + run: python3 scripts/ci/sanitize-xcode-source-packages-cache.py .ci-source-packages + + - name: Prepare isolated DerivedData + run: | + set -euo pipefail + DERIVED_DATA_PATH="${RUNNER_TEMP}/cmux-derived-data-command-palette-bench-${GITHUB_RUN_ID}-${GITHUB_RUN_ATTEMPT}" + rm -rf "$DERIVED_DATA_PATH" + mkdir -p "$DERIVED_DATA_PATH" + echo "CMUX_NUCLEO_FFI_DERIVED_DATA=$DERIVED_DATA_PATH" >> "$GITHUB_ENV" + + - name: Resolve Swift packages + run: | + set -euo pipefail + SOURCE_PACKAGES_DIR="$PWD/.ci-source-packages" + mkdir -p "$SOURCE_PACKAGES_DIR" + xcodebuild -project cmux.xcodeproj -scheme cmux-unit -configuration Debug \ + -derivedDataPath "$CMUX_NUCLEO_FFI_DERIVED_DATA" \ + -clonedSourcePackagesDirPath "$SOURCE_PACKAGES_DIR" \ + -resolvePackageDependencies + + - name: Run command palette search benchmarks + run: | + set -euo pipefail + CMUX_NUCLEO_FFI_SOURCE_PACKAGES="$PWD/.ci-source-packages" \ + ./scripts/test-command-palette-nucleo-ffi.sh + + - name: Write benchmark summary + if: always() + run: | + set -euo pipefail + { + echo "## Command palette search benchmarks" + echo "" + if [ -f "$CMUX_NUCLEO_FFI_LOG" ]; then + grep 'BENCH cmd+' "$CMUX_NUCLEO_FFI_LOG" | sed 's/^/- `/; s/$/`/' || echo "No BENCH lines were emitted." + else + echo "The benchmark invocation produced no log." + fi + } >> "$GITHUB_STEP_SUMMARY" + + - name: Upload benchmark log + if: always() + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4 + with: + name: command-palette-search-benchmarks + path: ${{ env.CMUX_NUCLEO_FFI_LOG }} + if-no-files-found: ignore diff --git a/CLI/CMUXCLI+BrowserDownload.swift b/CLI/CMUXCLI+BrowserDownload.swift index a362a439e1dd..f8a90a3a963f 100644 --- a/CLI/CMUXCLI+BrowserDownload.swift +++ b/CLI/CMUXCLI+BrowserDownload.swift @@ -1,3 +1,4 @@ +import CmuxControlSocket import Foundation extension CMUXCLI { @@ -51,9 +52,11 @@ extension CMUXCLI { if let timeoutMs = wait.timeoutMs { params["timeout_ms"] = timeoutMs } - let requestedTimeoutMs = wait.timeoutMs ?? 10_000 - let effectiveTimeoutMs = min(requestedTimeoutMs, 120_000) - let responseTimeout = Double(max(1, effectiveTimeoutMs)) / 1000.0 + 5.0 + // The handler's own window plus reply slack, both owned by + // BrowserDownloadWaitTimeout so the client cannot give up while the + // app is still inside the window it is allowed to wait. + let responseTimeout = BrowserDownloadWaitTimeout.standard + .clientResponseTimeoutSeconds(requestedMilliseconds: wait.timeoutMs) let payload = try client.sendV2( method: "browser.download.wait", params: params, diff --git a/CLI/CMUXCLI+RestorePreflight.swift b/CLI/CMUXCLI+RestorePreflight.swift index f9ccd3254d86..ff89c71540ee 100644 --- a/CLI/CMUXCLI+RestorePreflight.swift +++ b/CLI/CMUXCLI+RestorePreflight.swift @@ -161,7 +161,9 @@ extension CMUXCLI { guard try waitForRestorePreflightExit( exitQueue, - timeout: 10 + timeout: AgentRestorePreflightInvocation.timeoutSeconds( + environment: ProcessInfo.processInfo.environment + ) ) else { terminateRestorePreflight(processID, exitQueue: exitQueue) throw loggedRestoreError( diff --git a/CLI/SocketClient+StartupWait.swift b/CLI/SocketClient+StartupWait.swift index 96a3fdb58a73..1521a62929e4 100644 --- a/CLI/SocketClient+StartupWait.swift +++ b/CLI/SocketClient+StartupWait.swift @@ -2,6 +2,14 @@ import CmuxControlSocket import Foundation extension SocketClient { + /// Budget the launch-capable commands give a starting app to bind its + /// control socket. ``SocketStartupWaiter`` owns both the default window and + /// the environment override that narrows it. + static let appStartupWaitTimeoutSeconds: TimeInterval = + SocketStartupWaiter.appStartupTimeoutSeconds( + environment: ProcessInfo.processInfo.environment + ) + static func waitForConnectableSocket(path: String, timeout: TimeInterval) throws -> SocketClient { try waitForConnectableSocket(resolvePath: { path }, timeout: timeout) } diff --git a/CLI/cmux.swift b/CLI/cmux.swift index 3c7b658062f9..5cb1f2aa3fdf 100644 --- a/CLI/cmux.swift +++ b/CLI/cmux.swift @@ -4197,19 +4197,11 @@ struct CMUXCLI { /// Restored terminals start the app and then race its listener bind. Keep /// the implicit restore connection alive long enough for that lifecycle, /// while explicit socket paths retain their immediate failure semantics. - private static let defaultRestoreSocketStartupTimeoutSeconds: TimeInterval = 45 - /// Tests that exercise the "still opening" failure set - /// `CMUX_RESTORE_SOCKET_STARTUP_TIMEOUT_SECONDS` so they do not wait out - /// the full startup budget. It can only shorten the default. - private static var restoreSocketStartupTimeoutSeconds: TimeInterval { - let key = "CMUX_RESTORE_SOCKET_STARTUP_TIMEOUT_SECONDS" - guard let raw = ProcessInfo.processInfo.environment[key]?.trimmingCharacters(in: .whitespacesAndNewlines), - let value = TimeInterval(raw), - value.isFinite else { - return defaultRestoreSocketStartupTimeoutSeconds - } - return min(max(value, 0.05), defaultRestoreSocketStartupTimeoutSeconds) - } + /// + /// ``SocketStartupWaiter`` owns the default window and its environment + /// override, so the CLI and the socket package cannot drift apart. + private static let restoreSocketStartupTimeoutSeconds: TimeInterval = + SocketClient.appStartupWaitTimeoutSeconds // Stable per-user slot for the pinned Cloud VM. This value is intentionally reused as // both the backend create idempotency key and the local daemon slot so every open, // reconnect, session restore, and mobile attach targets the same provider VM once diff --git a/Packages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentRestorePreflightInvocation.swift b/Packages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentRestorePreflightInvocation.swift index 33379ff9d8be..cf51e08f9d02 100644 --- a/Packages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentRestorePreflightInvocation.swift +++ b/Packages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentRestorePreflightInvocation.swift @@ -1,3 +1,5 @@ +import Foundation + /// A shell-free subprocess invocation run before the restored process. public struct AgentRestorePreflightInvocation: Equatable, Sendable { /// The executable token from ``arguments``. @@ -19,4 +21,35 @@ public struct AgentRestorePreflightInvocation: Equatable, Sendable { self.arguments = arguments self.environment = environment } + + /// Ceiling applied to one preflight invocation. + /// + /// The restore command runs the preflight before `execve`, so this is the + /// product-visible ceiling on "provider setup" before restore reports that + /// setup took too long. + public static let defaultTimeoutSeconds: Double = 10 + + /// Environment key that replaces ``defaultTimeoutSeconds``. + /// + /// A caller that drives restore non-interactively — a harness, or a + /// supervisor that retries on its own schedule — bounds the wait here + /// instead of holding the terminal for the full default window. + public static let timeoutEnvironmentKey = "CMUX_RESTORE_PREFLIGHT_TIMEOUT_SECONDS" + + /// Resolves the preflight budget for `environment`. + /// + /// - Parameter environment: Process environment to read the override from. + /// - Returns: The override when it parses to a finite positive number of + /// seconds no greater than the default, otherwise + /// ``defaultTimeoutSeconds``. + public static func timeoutSeconds(environment: [String: String]) -> Double { + guard let raw = environment[timeoutEnvironmentKey]? + .trimmingCharacters(in: .whitespacesAndNewlines), + let value = Double(raw), + value.isFinite, + value > 0 else { + return defaultTimeoutSeconds + } + return min(value, defaultTimeoutSeconds) + } } diff --git a/Packages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Policy/BrowserDownloadWaitTimeout.swift b/Packages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Policy/BrowserDownloadWaitTimeout.swift new file mode 100644 index 000000000000..b64ff6cb227f --- /dev/null +++ b/Packages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Policy/BrowserDownloadWaitTimeout.swift @@ -0,0 +1,71 @@ +public import Foundation + +/// Shared timeout window for the `browser.download.wait` control call. +/// +/// The app-side handler and the command-line client have to agree: the client +/// has to outwait the window the handler is allowed to spend, or a download +/// that the app reports on time still fails in the terminal. Both sides read +/// ``standard`` so the two windows cannot drift apart. +/// +/// The windows are values rather than fixed constants so a test can hold the +/// same agreement at a fraction of the wall-clock cost instead of waiting the +/// shipped window out. +public struct BrowserDownloadWaitTimeout: Equatable, Sendable { + /// The windows the app and the CLI ship with. + public static let standard = BrowserDownloadWaitTimeout() + + /// Window the handler waits when the caller sends no `timeout_ms`. + public let defaultTimeoutMilliseconds: Int + + /// Ceiling the handler applies to a caller-supplied `timeout_ms`. + public let maximumTimeoutMilliseconds: Int + + /// Slack the client adds on top of the handler's window to cover request + /// dispatch, the handler's own bookkeeping, and the reply hop. + public let clientResponseSlackSeconds: TimeInterval + + /// Creates a window pair the handler and the client both read. + /// + /// - Parameters: + /// - defaultTimeoutMilliseconds: Window for a caller that sends no + /// `timeout_ms`. + /// - maximumTimeoutMilliseconds: Ceiling for a caller-supplied + /// `timeout_ms`. + /// - clientResponseSlackSeconds: Slack the client adds to the handler's + /// window. + public init( + defaultTimeoutMilliseconds: Int = 10_000, + maximumTimeoutMilliseconds: Int = 120_000, + clientResponseSlackSeconds: TimeInterval = 5 + ) { + self.defaultTimeoutMilliseconds = defaultTimeoutMilliseconds + self.maximumTimeoutMilliseconds = maximumTimeoutMilliseconds + self.clientResponseSlackSeconds = clientResponseSlackSeconds + } + + /// Window the handler spends for `requestedMilliseconds`. + /// + /// - Parameter requestedMilliseconds: The caller's `timeout_ms`, or `nil` + /// when the caller sent none. + /// - Returns: The clamped handler window in milliseconds. + public func handlerTimeoutMilliseconds( + requestedMilliseconds: Int? + ) -> Int { + let requested = max(1, requestedMilliseconds ?? defaultTimeoutMilliseconds) + return min(requested, maximumTimeoutMilliseconds) + } + + /// Socket response timeout the client uses for `requestedMilliseconds`. + /// + /// - Parameter requestedMilliseconds: The caller's `--timeout-ms`, or `nil` + /// when the caller passed none. + /// - Returns: The handler window plus ``clientResponseSlackSeconds``. + public func clientResponseTimeoutSeconds( + requestedMilliseconds: Int? + ) -> TimeInterval { + let handlerWindow = handlerTimeoutMilliseconds( + requestedMilliseconds: requestedMilliseconds + ) + return TimeInterval(handlerWindow) / 1000.0 + clientResponseSlackSeconds + } +} diff --git a/Packages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Transport/SocketStartupWaiter.swift b/Packages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Transport/SocketStartupWaiter.swift index f2ef51666aca..010cbcc135f0 100644 --- a/Packages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Transport/SocketStartupWaiter.swift +++ b/Packages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Transport/SocketStartupWaiter.swift @@ -265,3 +265,35 @@ public struct SocketStartupWaiter { } } } + +extension SocketStartupWaiter { + /// Window a command-line client gives a launching cmux app to bind its + /// control socket before reporting that the app is still opening. + public static let appStartupTimeoutDefaultSeconds: TimeInterval = 45 + + /// Environment key that replaces ``appStartupTimeoutDefaultSeconds``. + /// + /// A caller that already knows how long the app may take — a supervised + /// relaunch, a harness that never launches the app at all — bounds the + /// wait here instead of holding the terminal for the full default window. + public static let appStartupTimeoutEnvironmentKey = "CMUX_APP_STARTUP_WAIT_TIMEOUT_SECONDS" + + /// Resolves the app-startup wait budget for `environment`. + /// + /// - Parameter environment: Process environment to read the override from. + /// - Returns: The override when it parses to a finite positive number of + /// seconds no greater than the default window, otherwise + /// ``appStartupTimeoutDefaultSeconds``. + public static func appStartupTimeoutSeconds( + environment: [String: String] + ) -> TimeInterval { + guard let raw = environment[appStartupTimeoutEnvironmentKey]? + .trimmingCharacters(in: .whitespacesAndNewlines), + let value = TimeInterval(raw), + value.isFinite, + value > 0 else { + return appStartupTimeoutDefaultSeconds + } + return min(value, appStartupTimeoutDefaultSeconds) + } +} diff --git a/Sources/KeyboardShortcutSettings.swift b/Sources/KeyboardShortcutSettings.swift index 0c44108ba244..3daa51736b2b 100644 --- a/Sources/KeyboardShortcutSettings.swift +++ b/Sources/KeyboardShortcutSettings.swift @@ -1109,9 +1109,21 @@ enum KeyboardShortcutSettings { static func clearShortcut(for action: Action) { setShortcut(.unbound, for: action) } + /// Clears every stored shortcut override. + /// + /// WHY the presence check: `removeObject(forKey:)` posts + /// `UserDefaults.didChangeNotification` even when the key was never + /// written, so an unguarded sweep over `Action.allCases` fans out one post + /// per action. Every post drives the live `ManagedPolicyEnforcementObserver` + /// through a full `reevaluate()` (dozens of forced-preference probes), and + /// `KeyboardShortcutSettingsFileStore` through + /// `reapplyManagedSettingsIfNeeded()`. Removing a key that is not stored is + /// a no-op, so skipping it keeps the reset identical while collapsing the + /// notification storm to the single `didChangeNotification` below. static func resetAll() { - for action in Action.allCases { - UserDefaults.standard.removeObject(forKey: action.defaultsKey) + let defaults = UserDefaults.standard + for action in Action.allCases where defaults.object(forKey: action.defaultsKey) != nil { + defaults.removeObject(forKey: action.defaultsKey) } postDidChangeNotification() } diff --git a/Sources/Panels/BrowserDesignModeScreenshotEvaluator.swift b/Sources/Panels/BrowserDesignModeScreenshotEvaluator.swift index 16c49dc0663e..e0b033ff75de 100644 --- a/Sources/Panels/BrowserDesignModeScreenshotEvaluator.swift +++ b/Sources/Panels/BrowserDesignModeScreenshotEvaluator.swift @@ -43,7 +43,11 @@ final class BrowserDesignModeScreenshotEvaluator { private var operationIDsByWebView: [ObjectIdentifier: UUID] = [:] private var webViewIDsByOperation: [UUID: ObjectIdentifier] = [:] - init(timeout: TimeInterval = 5, cleanupTimeout: TimeInterval = 2) { + init( + timeout: TimeInterval = 5, + cleanupTimeout: TimeInterval = 2, + scrollSettleTimeout: TimeInterval = BrowserScreenshotWebViewSnapshotter.defaultScrollSettleTimeout + ) { self.timeout = timeout self.cleanupTimeout = cleanupTimeout visibleViewportCapture = { webView, completion in @@ -56,6 +60,7 @@ final class BrowserDesignModeScreenshotEvaluator { try await BrowserScreenshotWebViewSnapshotter.captureBoundedFullPageOverview( from: webView, maximumPixelCount: BrowserScreenshotPasteboardWriter.maximumDesignModeArtifactPixelCount, + scrollSettleTimeout: scrollSettleTimeout, onProgress: onProgress ) } @@ -63,6 +68,7 @@ final class BrowserDesignModeScreenshotEvaluator { try await BrowserScreenshotWebViewSnapshotter.captureDocumentRect( rect, from: webView, + scrollSettleTimeout: scrollSettleTimeout, onProgress: onProgress ) } diff --git a/Sources/Panels/BrowserScreenshotSnapshotter.swift b/Sources/Panels/BrowserScreenshotSnapshotter.swift index 3f2a810a729d..4906831c87e4 100644 --- a/Sources/Panels/BrowserScreenshotSnapshotter.swift +++ b/Sources/Panels/BrowserScreenshotSnapshotter.swift @@ -84,9 +84,15 @@ enum BrowserScreenshotCaptureBounds { @MainActor enum BrowserScreenshotWebViewSnapshotter { + /// Upper bound on the per-tile scroll settle wait. A web view that is not + /// on screen never receives animation frames, so this timer — not the two + /// requested frames — decides how long each stitched tile waits. + nonisolated static let defaultScrollSettleTimeout: TimeInterval = 0.25 + static func captureFullPage( from webView: WKWebView, afterScreenUpdates: Bool = true, + scrollSettleTimeout: TimeInterval = defaultScrollSettleTimeout, onProgress: @escaping @MainActor () -> Void = {} ) async throws -> NSImage { try Task.checkCancellation() @@ -120,6 +126,7 @@ enum BrowserScreenshotWebViewSnapshotter { from: webView, metrics: metrics, afterScreenUpdates: afterScreenUpdates, + scrollSettleTimeout: scrollSettleTimeout, onProgress: onProgress ) } @@ -130,6 +137,7 @@ enum BrowserScreenshotWebViewSnapshotter { from webView: WKWebView, maximumPixelCount: Int, afterScreenUpdates: Bool = true, + scrollSettleTimeout: TimeInterval = defaultScrollSettleTimeout, onProgress: @escaping @MainActor () -> Void = {} ) async throws -> NSImage { try Task.checkCancellation() @@ -177,6 +185,7 @@ enum BrowserScreenshotWebViewSnapshotter { metrics: metrics, maximumPixelCount: maximumPixelCount, afterScreenUpdates: afterScreenUpdates, + scrollSettleTimeout: scrollSettleTimeout, onProgress: onProgress ) } @@ -199,6 +208,7 @@ enum BrowserScreenshotWebViewSnapshotter { _ rect: NSRect, from webView: WKWebView, afterScreenUpdates: Bool = true, + scrollSettleTimeout: TimeInterval = defaultScrollSettleTimeout, onProgress: @escaping @MainActor () -> Void = {} ) async throws -> NSImage { try Task.checkCancellation() @@ -225,6 +235,7 @@ enum BrowserScreenshotWebViewSnapshotter { metrics: metrics, maximumPixelCount: Int(BrowserScreenshotCaptureBounds.maximumSelectionPixels), afterScreenUpdates: afterScreenUpdates, + scrollSettleTimeout: scrollSettleTimeout, onProgress: onProgress ) } @@ -274,6 +285,7 @@ enum BrowserScreenshotWebViewSnapshotter { from webView: WKWebView, metrics: BrowserViewportContentMetrics, afterScreenUpdates: Bool, + scrollSettleTimeout: TimeInterval, onProgress: @escaping @MainActor () -> Void ) async throws -> NSImage { let contentSize = metrics.contentSize @@ -309,7 +321,11 @@ enum BrowserScreenshotWebViewSnapshotter { guard let origin = tilePlan.origin(column: column, row: row) else { throw BrowserScreenshotError.webContentMetricsUnavailable } - let actualOrigin = try await scroll(webView, to: origin) + let actualOrigin = try await scroll( + webView, + to: origin, + settleTimeout: scrollSettleTimeout + ) onProgress() try Task.checkCancellation() let tile = try await captureVisibleViewport( @@ -337,7 +353,11 @@ enum BrowserScreenshotWebViewSnapshotter { // must not leave the user's page scrolled to an intermediate tile. let restoration = Task { @MainActor [weak webView] in guard let webView else { return } - _ = try? await scroll(webView, to: metrics.scrollOffset) + _ = try? await scroll( + webView, + to: metrics.scrollOffset, + settleTimeout: scrollSettleTimeout + ) } await restoration.value onProgress() @@ -362,6 +382,7 @@ enum BrowserScreenshotWebViewSnapshotter { metrics: BrowserViewportContentMetrics, maximumPixelCount: Int, afterScreenUpdates: Bool, + scrollSettleTimeout: TimeInterval, onProgress: @escaping @MainActor () -> Void ) async throws -> NSImage { let pageRect = NSRect(origin: .zero, size: metrics.contentSize) @@ -399,7 +420,8 @@ enum BrowserScreenshotWebViewSnapshotter { to: NSPoint( x: captureRegion.minX + relativeOrigin.x, y: captureRegion.minY + relativeOrigin.y - ) + ), + settleTimeout: scrollSettleTimeout ) onProgress() try Task.checkCancellation() @@ -428,7 +450,11 @@ enum BrowserScreenshotWebViewSnapshotter { // never leaves the page at an intermediate stitched-capture offset. let restoration = Task { @MainActor [weak webView] in guard let webView else { return } - _ = try? await scroll(webView, to: metrics.scrollOffset) + _ = try? await scroll( + webView, + to: metrics.scrollOffset, + settleTimeout: scrollSettleTimeout + ) } await restoration.value onProgress() @@ -749,7 +775,11 @@ enum BrowserScreenshotWebViewSnapshotter { } @discardableResult - private static func scroll(_ webView: WKWebView, to point: NSPoint) async throws -> CGPoint { + private static func scroll( + _ webView: WKWebView, + to point: NSPoint, + settleTimeout: TimeInterval + ) async throws -> CGPoint { let value = try await webView.callAsyncJavaScript( """ const doc = document.documentElement; @@ -777,7 +807,7 @@ enum BrowserScreenshotWebViewSnapshotter { if (!settled) { settled = true; resolve(); } }; requestAnimationFrame(() => requestAnimationFrame(finish)); - setTimeout(finish, 250); + setTimeout(finish, settleTimeoutMilliseconds); }); return { x: window.scrollX || 0, @@ -789,6 +819,7 @@ enum BrowserScreenshotWebViewSnapshotter { arguments: [ "x": Double(point.x), "y": Double(point.y), + "settleTimeoutMilliseconds": max(0, settleTimeout * 1000), ], in: nil, contentWorld: .page diff --git a/Sources/PortScanner.swift b/Sources/PortScanner.swift index 27a1e573bb38..18f1bf69a514 100644 --- a/Sources/PortScanner.swift +++ b/Sources/PortScanner.swift @@ -80,7 +80,11 @@ final class PortScanner: @unchecked Sendable { /// Each scan fires at this absolute offset; the recursive scheduler /// converts to relative delays between consecutive scans. - private static let burstOffsets: [Double] = [0.5, 1.5, 3, 5, 7.5, 10] + static let defaultBurstOffsets: [TimeInterval] = [0.5, 1.5, 3, 5, 7.5, 10] + /// Quiet window that merges kicks from many shells into one burst. + static let defaultCoalesceDelay: TimeInterval = 0.2 + private let burstOffsets: [TimeInterval] + private let coalesceDelay: TimeInterval private static let panelMissingPortRetentionLimit = 2 private static let minimumScansPerKick = panelMissingPortRetentionLimit + 1 private static let agentRescanInterval: TimeInterval = 2 @@ -97,9 +101,13 @@ final class PortScanner: @unchecked Sendable { }, ttySessionIdentityProvider: @escaping @MainActor @Sendable (String) -> TerminalTTYSessionIdentity? = { TerminalTTYSessionIdentity(ttyName: $0) - } + }, + burstOffsets: [TimeInterval] = PortScanner.defaultBurstOffsets, + coalesceDelay: TimeInterval = PortScanner.defaultCoalesceDelay ) { self.commandRunner = commandRunner + self.burstOffsets = burstOffsets + self.coalesceDelay = coalesceDelay self.processIdentityProvider = processIdentityProvider self.processPresenceProvider = processPresenceProvider self.ttySessionIdentityProvider = ttySessionIdentityProvider @@ -227,7 +235,7 @@ final class PortScanner: @unchecked Sendable { private func startCoalesce() { coalesceTimer?.cancel() let timer = DispatchSource.makeTimerSource(queue: queue) - timer.schedule(deadline: .now() + 0.2) + timer.schedule(deadline: .now() + coalesceDelay) timer.setEventHandler { [weak self] in self?.coalesceTimerFired() } @@ -247,7 +255,7 @@ final class PortScanner: @unchecked Sendable { private func runBurst(index: Int, burstStart: DispatchTime? = nil, generation: UInt64) { // Already on `queue`. guard generation == burstGeneration else { return } - guard index < Self.burstOffsets.count else { + guard index < burstOffsets.count else { burstActive = false // If new kicks arrived during the burst, start a new coalesce cycle. if !pendingKicks.isEmpty { @@ -257,7 +265,7 @@ final class PortScanner: @unchecked Sendable { } let start = burstStart ?? .now() - let deadline = start + Self.burstOffsets[index] + let deadline = start + burstOffsets[index] let timerID = UUID() let timer = DispatchSource.makeTimerSource(queue: queue) timer.schedule(deadline: deadline) diff --git a/Sources/TerminalController.swift b/Sources/TerminalController.swift index 0e4ee66938dc..3d6323efd4da 100644 --- a/Sources/TerminalController.swift +++ b/Sources/TerminalController.swift @@ -242,8 +242,6 @@ class TerminalController { private nonisolated static var socketMainHopSignpostingActive: Bool { socketMainHopSignposter.isEnabled } - private nonisolated static let v2BrowserDownloadWaitDefaultTimeoutMs = 10_000 - private nonisolated static let v2BrowserDownloadWaitMaxTimeoutMs = 120_000 private nonisolated static let v2ConsumedBrowserDownloadIDLimit = 128 private struct MobileViewportReport { var columns: Int; var rows: Int; var updatedAt: Date; var generation: UInt64? = nil @@ -9784,13 +9782,16 @@ class TerminalController { } private nonisolated func v2BrowserDownloadWaitOnSocketWorker(params: [String: Any]) -> V2CallResult { + // Shared with the CLI client, which sizes its socket response timeout + // from the same window and clamp. + let downloadWait = BrowserDownloadWaitTimeout.standard let requestedTimeoutMs = max( 1, Self.v2WorkerInt(params, "timeout_ms") ?? Self.v2WorkerInt(params, "timeout") ?? - Self.v2BrowserDownloadWaitDefaultTimeoutMs + downloadWait.defaultTimeoutMilliseconds ) - let timeoutMs = min(requestedTimeoutMs, Self.v2BrowserDownloadWaitMaxTimeoutMs) + let timeoutMs = downloadWait.handlerTimeoutMilliseconds(requestedMilliseconds: requestedTimeoutMs) let timeout = Double(timeoutMs) / 1000.0 let path = Self.v2WorkerString(params, "path") diff --git a/cmuxTests/BrowserDesignModeScreenshotEvaluatorTests.swift b/cmuxTests/BrowserDesignModeScreenshotEvaluatorTests.swift index 785140e84644..9aacd8f0efcb 100644 --- a/cmuxTests/BrowserDesignModeScreenshotEvaluatorTests.swift +++ b/cmuxTests/BrowserDesignModeScreenshotEvaluatorTests.swift @@ -414,9 +414,16 @@ struct BrowserDesignModeScreenshotEvaluatorTests { guard didLoad else { return } webView.pageZoom = 2 + // Each stitched tile scrolls and then waits for two animation frames. + // This web view is never on screen in the test host, so no frame ever + // arrives and every one of the ~20 tiles waits out the settle bound + // instead. Shorten that bound; the tiling, stitching, and bounded + // output sizes under test are unchanged. + #expect(BrowserScreenshotWebViewSnapshotter.defaultScrollSettleTimeout == 0.25) let screenshotEvaluator = BrowserDesignModeScreenshotEvaluator( timeout: 10, - cleanupTimeout: 2 + cleanupTimeout: 2, + scrollSettleTimeout: 0.05 ) let overview = try await screenshotEvaluator.captureFullPage(from: webView) let selection = try await screenshotEvaluator.captureDocumentRect( diff --git a/cmuxTests/CLIGenericHookPersistenceTests.swift b/cmuxTests/CLIGenericHookPersistenceTests.swift index ea6b7d4e6963..13509dde9194 100644 --- a/cmuxTests/CLIGenericHookPersistenceTests.swift +++ b/cmuxTests/CLIGenericHookPersistenceTests.swift @@ -2326,7 +2326,13 @@ extension CLINotifyProcessIntegrationRegressionTests { /// classified wire name (`PostToolUse`) rather than the raw camelCase hook /// event — i.e. the suppression actually triggers for real Kiro events. func testKiroStandardLevelSuppressesReadOnlyToolFeedEvents() throws { - func feedPushCount(forTool tool: String) throws -> Int { + // A suppressed hook returns `{}` without ever opening the cmux socket, + // so the negative case is settled by the listener's empty accept queue + // once the hook process has exited — never by waiting out a timeout. + func runKiroPostToolUseHook( + forTool tool: String, + servesSocket: Bool + ) throws -> (feedPushCount: Int, openedSocket: Bool) { let cliPath = try bundledCLIPath() let socketPath = makeSocketPath("kiro-suppress") let listenerFD = try bindUnixSocket(at: socketPath) @@ -2339,11 +2345,14 @@ extension CLINotifyProcessIntegrationRegressionTests { unlink(socketPath) try? FileManager.default.removeItem(at: root) } - let serverHandled = startMockServer(listenerFD: listenerFD, state: state) { line in - guard let payload = self.jsonObject(line), let id = payload["id"] as? String else { - return self.malformedRequestResponse(raw: line) + var serverHandled: XCTestExpectation? + if servesSocket { + serverHandled = startMockServer(listenerFD: listenerFD, state: state) { line in + guard let payload = self.jsonObject(line), let id = payload["id"] as? String else { + return self.malformedRequestResponse(raw: line) + } + return self.v2Response(id: id, ok: true, result: ["status": "acknowledged"]) } - return self.v2Response(id: id, ok: true, result: ["status": "acknowledged"]) } let result = runProcess( executablePath: cliPath, @@ -2365,17 +2374,28 @@ extension CLINotifyProcessIntegrationRegressionTests { XCTAssertFalse(result.timedOut, "\(tool): \(result.stderr)") XCTAssertEqual(result.status, 0, "\(tool): \(result.stderr)") XCTAssertEqual(result.stdout, "{}\n", "\(tool) stdout") - // A non-suppressed event sends one feed.push, so wait for the - // server to record it (generous timeout to avoid flaking on the - // socket/process round-trip under CI load). A suppressed event - // sends nothing, so this wait simply times out silently. - _ = XCTWaiter().wait(for: [serverHandled], timeout: 5) - return state.commands.filter { $0.contains("feed.push") }.count + // A non-suppressed event sends one feed.push, so wait on the server + // recording it. The suppressed run serves no connection at all: the + // exited hook either left a connection queued on the listener or + // never dialed it, and poll answers that immediately. + if let serverHandled { + wait(for: [serverHandled], timeout: 10) + } + var listener = pollfd(fd: listenerFD, events: Int16(POLLIN), revents: 0) + let queuedConnection = Darwin.poll(&listener, 1, 0) > 0 + return ( + state.commands.filter { $0.contains("feed.push") }.count, + queuedConnection || !state.commands.isEmpty + ) } - XCTAssertEqual(try feedPushCount(forTool: "fs_read"), 0, + let suppressed = try runKiroPostToolUseHook(forTool: "fs_read", servesSocket: false) + XCTAssertFalse(suppressed.openedSocket, + "read-only kiro tool at standard level must be suppressed before it dials cmux") + XCTAssertEqual(suppressed.feedPushCount, 0, "read-only kiro tool at standard level must be suppressed") - XCTAssertGreaterThan(try feedPushCount(forTool: "fs_write"), 0, + let reported = try runKiroPostToolUseHook(forTool: "fs_write", servesSocket: true) + XCTAssertGreaterThan(reported.feedPushCount, 0, "mutating kiro tool at standard level must still emit telemetry") } diff --git a/cmuxTests/CMUXCLIErrorOutputRegressionTests.swift b/cmuxTests/CMUXCLIErrorOutputRegressionTests.swift index ac7ed9577246..793f1975e0b4 100644 --- a/cmuxTests/CMUXCLIErrorOutputRegressionTests.swift +++ b/cmuxTests/CMUXCLIErrorOutputRegressionTests.swift @@ -1,3 +1,5 @@ +import CMUXAgentLaunch +import CmuxControlSocket import CmuxSettings import Darwin import Foundation @@ -688,12 +690,23 @@ import Testing environment["HOME"] = root.path environment["CFFIXED_USER_HOME"] = root.path environment["HERMES_HOME"] = root.appendingPathComponent(".hermes", isDirectory: true).path + // The preflight child below never exits on its own, so restore has to reach its + // timeout for this test to observe the quiet failure. The size of that window is + // production policy, asserted as a value; the run itself narrows it. + #expect(AgentRestorePreflightInvocation.defaultTimeoutSeconds == 10) + #expect( + AgentRestorePreflightInvocation.timeoutSeconds(environment: [:]) + == AgentRestorePreflightInvocation.defaultTimeoutSeconds + ) + environment[AgentRestorePreflightInvocation.timeoutEnvironmentKey] = "0.5" let result = runProcess( executablePath: cliPath, arguments: ["restore", "hermes-agent", checkpointID], environment: environment, - timeout: 15 + // A deliberate cap, not a hang guard: the preflight window above is 0.5s, so + // the run has to finish well inside 5s. + timeout: 5 ) XCTAssertFalse(result.timedOut, result.diagnostics) @@ -2351,9 +2364,16 @@ import Testing } environment["CMUX_CLI_SENTRY_DISABLED"] = "1" environment["CFFIXED_USER_HOME"] = home.path - // restore and fork wait for the app's socket before reporting that - // cmux is still opening; the default 45 s wait made this test 90 s. - environment["CMUX_RESTORE_SOCKET_STARTUP_TIMEOUT_SECONDS"] = "1" + // restore and fork deliberately outwait a launching app before they report + // that cmux is still opening. This test is about which dispatch path the + // commands reach, not about the size of that window, so it narrows the + // window instead of spending the production default twice. + #expect(SocketStartupWaiter.appStartupTimeoutDefaultSeconds == 45) + #expect( + SocketStartupWaiter.appStartupTimeoutSeconds(environment: [:]) + == SocketStartupWaiter.appStartupTimeoutDefaultSeconds + ) + environment[SocketStartupWaiter.appStartupTimeoutEnvironmentKey] = "0.2" let cases: [(arguments: [String], expectedError: String)] = [ (["settings", "invalid-target"], "Unknown settings subcommand 'invalid-target'"), @@ -3296,10 +3316,43 @@ import Testing } @Test func testBrowserDownloadWaitDefaultTimeoutMatchesServerDefaultWindow() throws { + // The window itself is a value, not a latency: the app-side handler and the + // CLI client both take it from BrowserDownloadWaitTimeout.standard, so the + // client outwaits the handler by the reply slack rather than by coincidence. + // Spending the real window here would mean a >10s test that still could not + // tell 15s from a minute. + let window = BrowserDownloadWaitTimeout.standard + #expect(window.defaultTimeoutMilliseconds == 10_000) + #expect( + window.handlerTimeoutMilliseconds(requestedMilliseconds: nil) + == window.defaultTimeoutMilliseconds + ) + #expect( + window.clientResponseTimeoutSeconds(requestedMilliseconds: nil) + == TimeInterval(window.defaultTimeoutMilliseconds) / 1000.0 + + window.clientResponseSlackSeconds + ) + #expect(window.clientResponseSlackSeconds > 0) + // The agreement is a property of the pair, not of the shipped numbers: a + // narrower window the client still outwaits keeps the same invariant. + let narrow = BrowserDownloadWaitTimeout( + defaultTimeoutMilliseconds: 50, + maximumTimeoutMilliseconds: 200, + clientResponseSlackSeconds: 0.1 + ) + #expect( + narrow.clientResponseTimeoutSeconds(requestedMilliseconds: nil) + > TimeInterval(narrow.handlerTimeoutMilliseconds(requestedMilliseconds: nil)) + / 1000.0 + ) + + // And the default path really uses that window: with no --timeout-ms the CLI + // must ignore the generic response timeout below, which is short enough that + // a CLI falling back to it would give up before the responder answers. let cliPath = try bundledCLIPath() let socketPath = "/tmp/cmux-dw-\(UUID().uuidString.prefix(8)).sock" let response = #"{"ok":true,"result":{"downloaded":true}}"# - let responder = try UnixSocketResponder(path: socketPath, response: response, responseDelay: 10.5) + let responder = try UnixSocketResponder(path: socketPath, response: response, responseDelay: 0.4) defer { responder.stop() } var environment = ProcessInfo.processInfo.environment @@ -3319,12 +3372,9 @@ import Testing "wait", ], environment: environment, - // A deliberate cap, and the only upper bound that gives this test meaning: the - // responder answers after 10.5s, so waiting the server's default window has to - // land between there and 16s. Under the suite default a CLI that waited a full - // minute would still pass, and "matches the server default window" would stop - // being a claim about anything. - timeout: 16 + // A deliberate cap, not a hang guard: the responder answers after 0.4s, so + // this run has to finish well inside 3s. + timeout: 3 ) XCTAssertFalse(result.timedOut, result.diagnostics) diff --git a/cmuxTests/CMUXOpenCommandTests.swift b/cmuxTests/CMUXOpenCommandTests.swift index 3c340490099c..5ea65cc2bdf5 100644 --- a/cmuxTests/CMUXOpenCommandTests.swift +++ b/cmuxTests/CMUXOpenCommandTests.swift @@ -1,3 +1,4 @@ +import CryptoKit import Darwin import Foundation import XCTest @@ -1306,43 +1307,86 @@ final class CMUXOpenCommandTests: XCTestCase { XCTAssertFalse(html.contains("\\u001b"), html, file: file, line: line) } - try runGit(["init"], in: repoURL) - try runGit(["checkout", "-b", "main"], in: repoURL) - try runGit(["config", "user.name", "cmux tests"], in: repoURL) - try runGit(["config", "user.email", "cmux@example.invalid"], in: repoURL) - try runGit(["config", "color.ui", "always"], in: repoURL) - try runGit(["config", "color.diff", "always"], in: repoURL) - try runGit(["remote", "add", "origin", rootURL.appendingPathComponent("origin.git").path], in: repoURL) + // The fixture writes config, refs and branch heads directly instead of + // spawning one git process per setting: identical on-disk state, and + // the git subprocesses that remain are the ones that must be real + // (object creation and commits). Those handwritten loose refs and the + // in-process SHA-1 blob ids assume the classic repository layout, so + // pin it: a user's `init.defaultObjectFormat=sha256` or + // `init.defaultRefFormat=reftable` must not change what the fixture is. + try runGit(Self.classicLayoutGitInitArguments, in: repoURL) + // Same unborn-branch state `git checkout -b main` leaves behind. + try writeGitSymbolicRef("HEAD", target: "refs/heads/main", in: repoURL) + try appendGitConfig( + """ + [user] + \tname = cmux tests + \temail = cmux@example.invalid + [color] + \tui = always + \tdiff = always + [remote "origin"] + \turl = \(rootURL.appendingPathComponent("origin.git").path) + \tfetch = +refs/heads/*:refs/remotes/origin/* + + """, + in: repoURL + ) try "one\n".write(to: fileURL, atomically: true, encoding: .utf8) try runGit(["add", "story.txt"], in: repoURL) try runGit(["commit", "-m", "initial"], in: repoURL) let initialCommit = try runGitStdout(["rev-parse", "HEAD"], in: repoURL) - try runGit(["update-ref", "refs/remotes/origin/main", initialCommit], in: repoURL) - try runGit(["symbolic-ref", "refs/remotes/origin/HEAD", "refs/remotes/origin/main"], in: repoURL) + try writeGitRef("refs/remotes/origin/main", commit: initialCommit, in: repoURL) + try writeGitSymbolicRef("refs/remotes/origin/HEAD", target: "refs/remotes/origin/main", in: repoURL) let siblingRepoURL = rootURL.appendingPathComponent("other-repo", isDirectory: true) let siblingFileURL = siblingRepoURL.appendingPathComponent("other.txt") try FileManager.default.createDirectory(at: siblingRepoURL, withIntermediateDirectories: true) - try runGit(["init"], in: siblingRepoURL) - try runGit(["checkout", "-b", "main"], in: siblingRepoURL) - try runGit(["config", "user.name", "cmux tests"], in: siblingRepoURL) - try runGit(["config", "user.email", "cmux@example.invalid"], in: siblingRepoURL) + try runGit(Self.classicLayoutGitInitArguments, in: siblingRepoURL) + try writeGitSymbolicRef("HEAD", target: "refs/heads/main", in: siblingRepoURL) + try appendGitConfig( + """ + [user] + \tname = cmux tests + \temail = cmux@example.invalid + + """, + in: siblingRepoURL + ) try "base\n".write(to: siblingFileURL, atomically: true, encoding: .utf8) try runGit(["add", "other.txt"], in: siblingRepoURL) try runGit(["commit", "-m", "initial"], in: siblingRepoURL) let siblingInitialCommit = try runGitStdout(["rev-parse", "HEAD"], in: siblingRepoURL) - try runGit(["update-ref", "refs/remotes/origin/main", siblingInitialCommit], in: siblingRepoURL) - try runGit(["symbolic-ref", "refs/remotes/origin/HEAD", "refs/remotes/origin/main"], in: siblingRepoURL) - try runGit(["checkout", "-b", "feature/other"], in: siblingRepoURL) + try writeGitRef("refs/remotes/origin/main", commit: siblingInitialCommit, in: siblingRepoURL) + try writeGitSymbolicRef( + "refs/remotes/origin/HEAD", + target: "refs/remotes/origin/main", + in: siblingRepoURL + ) + // Same state `git checkout -b feature/other` leaves behind: the new + // head points at the current commit and the worktree is untouched. + try writeGitRef("refs/heads/feature/other", commit: siblingInitialCommit, in: siblingRepoURL) + try writeGitSymbolicRef("HEAD", target: "refs/heads/feature/other", in: siblingRepoURL) try "base\nchanged\n".write(to: siblingFileURL, atomically: true, encoding: .utf8) - try runGit(["checkout", "-b", "feature/diff-source"], in: repoURL) + try writeGitRef("refs/heads/feature/diff-source", commit: initialCommit, in: repoURL) + try writeGitSymbolicRef("HEAD", target: "refs/heads/feature/diff-source", in: repoURL) try "one\ntwo\n".write(to: fileURL, atomically: true, encoding: .utf8) try runGit(["add", "story.txt"], in: repoURL) try runGit(["commit", "-m", "add two"], in: repoURL) let featureCommit = try runGitStdout(["rev-parse", "HEAD"], in: repoURL) - try runGit(["update-ref", "refs/remotes/origin/feature/diff-source", featureCommit], in: repoURL) - try runGit(["branch", "--set-upstream-to=origin/feature/diff-source"], in: repoURL) + try writeGitRef("refs/remotes/origin/feature/diff-source", commit: featureCommit, in: repoURL) + // Same upstream `git branch --set-upstream-to=origin/feature/diff-source` + // records; the CLI reads it back through `@{upstream}`. + try appendGitConfig( + """ + [branch "feature/diff-source"] + \tremote = origin + \tmerge = refs/heads/feature/diff-source + + """, + in: repoURL + ) try "one\ntwo\nthree\n".write(to: fileURL, atomically: true, encoding: .utf8) let branch = try runDiffCLIAndReadHTML( @@ -2796,11 +2840,69 @@ final class CMUXOpenCommandTests: XCTestCase { return (try XCTUnwrap(attributes[.posixPermissions] as? NSNumber).intValue) & 0o777 } + /// `git init` for fixtures whose refs and object ids are written by hand: + /// SHA-1 objects and loose-file refs. The ref format goes through `-c` + /// rather than `--ref-format`, which git releases before 2.45 reject; those + /// releases only know loose-file refs anyway. `GIT_DEFAULT_REF_FORMAT` and + /// `GIT_DEFAULT_HASH` would override both choices, so `runGitProcess` drops + /// them from the inherited environment. + private static let classicLayoutGitInitArguments = [ + "-c", "init.defaultRefFormat=files", + "init", "-q", "--object-format=sha1" + ] + + /// Appends config text to a fixture repository's `.git/config`, producing + /// the same on-disk state as the equivalent `git config` / `git remote add` + /// invocations without a git subprocess per setting. + private func appendGitConfig(_ text: String, in directory: URL) throws { + let configURL = directory.appendingPathComponent(".git/config", isDirectory: false) + let existing = try String(contentsOf: configURL, encoding: .utf8) + let separator = existing.hasSuffix("\n") || existing.isEmpty ? "" : "\n" + try (existing + separator + text).write(to: configURL, atomically: true, encoding: .utf8) + } + + /// Writes the loose ref file `git update-ref` would write for a fixture + /// repository (no packed refs exist in these freshly created repos). + private func writeGitRef(_ ref: String, commit: String, in directory: URL) throws { + let refURL = directory.appendingPathComponent(".git", isDirectory: true) + .appendingPathComponent(ref, isDirectory: false) + try FileManager.default.createDirectory( + at: refURL.deletingLastPathComponent(), + withIntermediateDirectories: true + ) + try "\(commit)\n".write(to: refURL, atomically: true, encoding: .utf8) + } + + /// Writes the symbolic ref file `git symbolic-ref` would write. + private func writeGitSymbolicRef(_ ref: String, target: String, in directory: URL) throws { + let refURL = directory.appendingPathComponent(".git", isDirectory: true) + .appendingPathComponent(ref, isDirectory: false) + try FileManager.default.createDirectory( + at: refURL.deletingLastPathComponent(), + withIntermediateDirectories: true + ) + try "ref: \(target)\n".write(to: refURL, atomically: true, encoding: .utf8) + } + + /// The blob object id `git hash-object --no-filters` prints for a file, + /// computed in process (`sha1("blob \0" + contents)`) so baseline + /// fixtures do not pay a git subprocess per recorded untracked path. + private func gitBlobObjectID(forFileAt url: URL) throws -> String { + let contents = try Data(contentsOf: url) + var payload = Data("blob \(contents.count)\0".utf8) + payload.append(contents) + return Insecure.SHA1.hash(data: payload) + .map { String(format: "%02x", $0) } + .joined() + } + private func runGitProcess(_ arguments: [String], in directory: URL) -> ProcessRunResult { runProcess( executablePath: "/usr/bin/env", arguments: ["git"] + arguments, - environment: ProcessInfo.processInfo.environment, + environment: ProcessInfo.processInfo.environment.filter { key, _ in + key != "GIT_DEFAULT_REF_FORMAT" && key != "GIT_DEFAULT_HASH" + }, timeout: 30, currentDirectoryURL: directory ) @@ -2833,7 +2935,9 @@ final class CMUXOpenCommandTests: XCTestCase { .appendingPathComponent(snapshotId, isDirectory: true) .appendingPathComponent("files", isDirectory: true) for path in untrackedPaths { - let hash = try runGitStdout(["hash-object", "--no-filters", "--", path], in: repoURL) + let hash = try gitBlobObjectID( + forFileAt: repoURL.appendingPathComponent(path, isDirectory: false) + ) let snapshotURL = snapshotRoot.appendingPathComponent(path, isDirectory: false) try FileManager.default.createDirectory( at: snapshotURL.deletingLastPathComponent(), diff --git a/cmuxTests/CmuxWebViewKeyDownReentryTests.swift b/cmuxTests/CmuxWebViewKeyDownReentryTests.swift index d72d3250542e..c060f45802fe 100644 --- a/cmuxTests/CmuxWebViewKeyDownReentryTests.swift +++ b/cmuxTests/CmuxWebViewKeyDownReentryTests.swift @@ -10,7 +10,7 @@ import ObjectiveC.runtime @testable import cmux #endif -private var cmuxUnitTestCmuxWebViewKeyDownOverrideInstalled = false +private var cmuxUnitTestCmuxWebViewKeyDownOriginalIMP: IMP? private var cmuxUnitTestCmuxWebViewKeyDownHook: ((CmuxWebView, NSEvent) -> Bool)? private final class FakeWKInspectorUndoResponderView: NSView { @@ -26,28 +26,44 @@ private final class BrowserUndoMenuActionSpy: NSObject { } } -extension CmuxWebView { - @objc func cmuxUnitTest_keyDown(with event: NSEvent) { - if cmuxUnitTestCmuxWebViewKeyDownHook?(self, event) == true { - return - } - cmuxUnitTest_keyDown(with: event) - } -} - +/// Hooks `CmuxWebView.keyDown(with:)` for the duration of one test window. +/// +/// WHY scoped, not process-wide: other suites in the same app host (for +/// example `CmuxWebViewWebContentUndoTests`) swizzle the same key path on +/// `WKWebView` and exercise `CmuxWebView.keyDown` with no hook set. A +/// permanent swizzle left behind by this suite made those tests recurse until +/// the stack overflowed whenever the two suites shared a process. The hook +/// calls the captured original implementation directly and is removed again +/// in `uninstallCmuxUnitTestCmuxWebViewKeyDownOverride()`. private func installCmuxUnitTestCmuxWebViewKeyDownOverride() { - guard !cmuxUnitTestCmuxWebViewKeyDownOverrideInstalled else { return } + guard cmuxUnitTestCmuxWebViewKeyDownOriginalIMP == nil else { return } - let originalSelector = #selector(CmuxWebView.keyDown(with:)) - let swizzledSelector = #selector(CmuxWebView.cmuxUnitTest_keyDown(with:)) + let selector = #selector(CmuxWebView.keyDown(with:)) + guard let method = class_getInstanceMethod(CmuxWebView.self, selector) else { + fatalError("Unable to locate CmuxWebView keyDown method for swizzling") + } - guard let originalMethod = class_getInstanceMethod(CmuxWebView.self, originalSelector), - let swizzledMethod = class_getInstanceMethod(CmuxWebView.self, swizzledSelector) else { - fatalError("Unable to locate CmuxWebView keyDown methods for swizzling") + typealias KeyDownIMP = @convention(c) (AnyObject, Selector, NSEvent) -> Void + let originalIMP = method_getImplementation(method) + let original = unsafeBitCast(originalIMP, to: KeyDownIMP.self) + let hooked: @convention(block) (CmuxWebView, NSEvent) -> Void = { webView, event in + if cmuxUnitTestCmuxWebViewKeyDownHook?(webView, event) == true { + return + } + original(webView, selector, event) } + cmuxUnitTestCmuxWebViewKeyDownOriginalIMP = originalIMP + method_setImplementation(method, imp_implementationWithBlock(hooked)) +} - method_exchangeImplementations(originalMethod, swizzledMethod) - cmuxUnitTestCmuxWebViewKeyDownOverrideInstalled = true +private func uninstallCmuxUnitTestCmuxWebViewKeyDownOverride() { + guard let originalIMP = cmuxUnitTestCmuxWebViewKeyDownOriginalIMP, + let method = class_getInstanceMethod( + CmuxWebView.self, + #selector(CmuxWebView.keyDown(with:)) + ) else { return } + method_setImplementation(method, originalIMP) + cmuxUnitTestCmuxWebViewKeyDownOriginalIMP = nil } @Suite(.serialized) @@ -259,6 +275,7 @@ final class CmuxWebViewKeyDownReentryTests { defer { cmuxUnitTestCmuxWebViewKeyDownHook = nil window.orderOut(nil) + uninstallCmuxUnitTestCmuxWebViewKeyDownOverride() } #expect(window.makeFirstResponder(webView)) diff --git a/cmuxTests/CommandPaletteNucleoFFITests.swift b/cmuxTests/CommandPaletteNucleoFFITests.swift index f911c12b9932..2eb5bbf6dbfa 100644 --- a/cmuxTests/CommandPaletteNucleoFFITests.swift +++ b/cmuxTests/CommandPaletteNucleoFFITests.swift @@ -485,7 +485,37 @@ final class CommandPaletteNucleoFFITests: XCTestCase { ) } + /// Ranking half of `testNucleoFFIEdgeCaseTypingFrameBudgetComparison`. + /// + /// The frame-budget benchmark is env-gated out of the app-host unit suite, + /// but the edge-case ranking contract it also asserted (initialism, unread + /// command, open-folder command, exact generated workspace, diacritic title) + /// is cheap, so it stays here on a much smaller generated corpus. + func testNucleoFFIEdgeCaseQueriesRankExpectedTopResults() throws { + let entries = makeEdgeCasePaletteEntries(generatedWorkspaceCount: 60) + let corpus = searchCorpus(entries: entries) + guard let productionIndex = CommandPaletteNucleoSearchIndex(entries: corpus) else { + throw XCTSkip("Build the nucleo FFI dylib before running production wrapper tests") + } + + let expectedTopResults = [ + ("ims", "workspace.indigoMarkdownStudio"), + ("wunr", "palette.markWorkspaceUnread"), + ("open folder", "palette.openFolder"), + ("workspace 51", "workspace.large.51"), + ("cafe", "workspace.cafeUnicodeNotes"), + ] + for (query, expectedID) in expectedTopResults { + XCTAssertEqual( + productionIndex.search(query: query, resultLimit: 10)?.first?.payload, + expectedID, + "Unexpected top result for \(query)" + ) + } + } + func testNucleoFFIEdgeCaseTypingFrameBudgetComparison() throws { + try skipUnlessCommandPaletteSearchBenchmarksAreEnabled() let entries = makeEdgeCasePaletteEntries(generatedWorkspaceCount: 2_000) let corpus = searchCorpus(entries: entries) var index: CommandPaletteNucleoSearchIndex? diff --git a/cmuxTests/CommandPaletteNucleoFixtures.swift b/cmuxTests/CommandPaletteNucleoFixtures.swift index 6d886bf88b8c..c1121995b38d 100644 --- a/cmuxTests/CommandPaletteNucleoFixtures.swift +++ b/cmuxTests/CommandPaletteNucleoFixtures.swift @@ -1,6 +1,7 @@ import CmuxCommandPalette import Darwin import Foundation +import XCTest #if canImport(cmux_DEV) @testable import cmux_DEV @@ -344,3 +345,32 @@ func percentile(_ values: [Double], percentile: Double) -> Double { func repeatedQueries(_ baseQueries: [String], repetitions: Int) -> [String] { Array(repeating: baseQueries, count: repetitions).flatMap { $0 } } + +/// Gate for the command-palette search wall-clock benchmarks. +/// +/// These benchmarks measure pure search-engine code that has no app-host +/// dependency, and they cost roughly 85s of sharded app-host test time. Their +/// wall-clock ratios are also load-sensitive on a shared runner, so they belong +/// in a focused invocation rather than the unit suite. They stay runnable on +/// demand through the same env-gated skip shape the renderer-memory regression +/// uses (`CMUX_RENDERER_MEMORY_REGRESSION` in `TerminalAndGhosttyTests`): +/// +/// CMUX_COMMAND_PALETTE_SEARCH_BENCHMARKS=1 \ +/// scripts/ci/run-app-host-xcodebuild.sh ... \ +/// -only-testing:cmuxTests/CommandPaletteSearchEngineTests/ +/// +/// (`run-app-host-xcodebuild.sh` forwards driver variables to the app host +/// through the `TEST_RUNNER_` channel, so CI would set +/// `TEST_RUNNER_CMUX_COMMAND_PALETTE_SEARCH_BENCHMARKS=1`.) +/// +/// Whatever each benchmark asserted about *results* stays in the unit suite as +/// a small separate test, so behavior regressions are still caught when the +/// timing runs are skipped. +func skipUnlessCommandPaletteSearchBenchmarksAreEnabled() throws { + guard ProcessInfo.processInfo.environment["CMUX_COMMAND_PALETTE_SEARCH_BENCHMARKS"] == "1" else { + throw XCTSkip( + "Runs in the focused command-palette search benchmark invocation " + + "(CMUX_COMMAND_PALETTE_SEARCH_BENCHMARKS=1)" + ) + } +} diff --git a/cmuxTests/CommandPaletteSearchEngineTests.swift b/cmuxTests/CommandPaletteSearchEngineTests.swift index b7fbb835d414..a602d9d1c368 100644 --- a/cmuxTests/CommandPaletteSearchEngineTests.swift +++ b/cmuxTests/CommandPaletteSearchEngineTests.swift @@ -13,6 +13,25 @@ final class CommandPaletteSearchEngineTests: XCTestCase { let rank: Int let title: String let searchableTexts: [String] + /// Normalized title, as the engine prepares it. + let titleNormalizedText: String + /// Normalized title word text excluding symbol-only segments, which is + /// the text the engine's title-word ranking term is keyed on. + let titleSearchWordText: String + + /// Prepares the title texts once, outside the benchmark timing loops, + /// so the reference pipeline can model the engine's title-word term + /// without the preparation cost landing on the timed comparison. + init(id: String, rank: Int, title: String, searchableTexts: [String]) { + self.id = id + self.rank = rank + self.title = title + self.searchableTexts = searchableTexts + self.titleNormalizedText = CommandPaletteFuzzyMatcher.normalizeForSearch(title) + self.titleSearchWordText = CommandPaletteSearchCorpusEntry( + payload: id, rank: rank, title: title, searchableTexts: searchableTexts + ).normalizedTitleSearchWordText + } } private struct FixtureResult: Equatable { @@ -213,6 +232,7 @@ final class CommandPaletteSearchEngineTests: XCTestCase { query: String ) -> [FixtureResult] { let queryIsEmpty = query.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty + let preparedQuery = CommandPaletteFuzzyMatcher.preparedQuery(query) let results: [FixtureResult] = queryIsEmpty ? entries.map { entry in FixtureResult(id: entry.id, rank: entry.rank, title: entry.title, score: 0, titleMatchIndices: []) @@ -220,6 +240,7 @@ final class CommandPaletteSearchEngineTests: XCTestCase { : entries.compactMap { entry in guard let fuzzyScore = weightedReferenceScore( query: query, + preparedQuery: preparedQuery, entry: entry ) else { return nil @@ -260,6 +281,7 @@ final class CommandPaletteSearchEngineTests: XCTestCase { private func weightedReferenceScore( query: String, + preparedQuery: CommandPaletteFuzzyMatcher.PreparedQuery, entry: FixtureEntry ) -> Int? { guard let fuzzyScore = CommandPaletteFuzzyMatcher.score( @@ -274,7 +296,36 @@ final class CommandPaletteSearchEngineTests: XCTestCase { ) else { return fuzzyScore } - return max(fuzzyScore, titleScore + 2000) + return max( + fuzzyScore, + titleScore + 2000, + referenceTitleWordScore(preparedQuery: preparedQuery, entry: entry) ?? Int.min + ) + } + + /// Independently models the engine's title-word ranking term: a query that + /// is, or prefixes, a title's search words outranks the same query matched + /// fuzzily anywhere in the entry. Reimplemented here rather than called + /// through, because a reference pipeline that shared the engine's + /// implementation would assert nothing about it. + private func referenceTitleWordScore( + preparedQuery: CommandPaletteFuzzyMatcher.PreparedQuery, + entry: FixtureEntry + ) -> Int? { + guard !preparedQuery.isEmpty, + entry.titleSearchWordText != entry.titleNormalizedText else { + return nil + } + let scaledTitleMatchBonus = 2000 * max(1, preparedQuery.tokens.count) + if entry.titleSearchWordText == preparedQuery.normalizedTokenText { + return preparedQuery.tokens.reduce(0) { $0 + $1.scoreUpperBound } + scaledTitleMatchBonus + } + guard entry.titleSearchWordText.hasPrefix(preparedQuery.normalizedTokenText) else { + return nil + } + return preparedQuery.tokens.reduce(0) { + $0 + $1.scoreUpperBoundWithoutExactMatch + } + scaledTitleMatchBonus } private func benchmarkElapsedMs(operation: () -> Void) -> Double { @@ -303,6 +354,21 @@ final class CommandPaletteSearchEngineTests: XCTestCase { Array(repeating: baseQueries, count: repetitions).flatMap { $0 } } + /// Wall-clock search benchmarks do not belong in the sharded app-host unit + /// suite; see `skipUnlessCommandPaletteSearchBenchmarksAreEnabled()` in + /// `CommandPaletteNucleoFixtures.swift` for the gate and how to run them. + /// + /// Correctness of the benchmarked code paths remains in the unit suite: + /// `testOptimizedSearchMatchesReferencePipeline` and + /// `testBenchmarkCorporaMatchReferencePipelineOnSmallFixture` assert result + /// parity between the optimized engine and the legacy reference pipeline on + /// the same corpora and queries the benchmarks time, and + /// `testLimitedSearchReturnsSameTopResultsAsFullSearch` covers the capped / + /// preview paths the fast-typing benchmark times. + private func skipUnlessSearchBenchmarksAreEnabled() throws { + try skipUnlessCommandPaletteSearchBenchmarksAreEnabled() + } + func testOptimizedSearchMatchesReferencePipeline() { let commandEntries = makeCommandEntries(count: 96) let switcherEntries = makeSwitcherEntries(count: 64) @@ -332,6 +398,62 @@ final class CommandPaletteSearchEngineTests: XCTestCase { } } + /// Correctness half of the env-gated search benchmarks: the benchmarks time + /// the optimized engine against the legacy reference pipeline on the switcher + /// and large-workspace corpora, and the fast-typing benchmark times the + /// capped and visible-candidate preview paths over typed prefixes. This test + /// keeps the *result* contract for exactly those corpora, queries and paths + /// in the unit suite on small fixtures, so a behavior regression is still + /// caught when the timing runs are skipped. + func testBenchmarkCorporaMatchReferencePipelineOnSmallFixture() { + let switcherEntries = makeSwitcherEntries(count: 32) + let switcherQueries = ["workspace 12", "phoenix", "feature-18", "rename-tab", "3007", "9202", "switch", "worktrees"] + for query in switcherQueries { + XCTAssertEqual( + optimizedResults(entries: switcherEntries, query: query), + referenceResults(entries: switcherEntries, query: query), + "Switcher benchmark corpus mismatch for query \(query)" + ) + } + + let largeEntries = makeLargeWorkspaceSwitcherEntries(count: 32) + let largeQueries = [ + "workspace 31", + "palette latency", + "feature 21", + "cmd-p-search", + "project-17", + "4207", + "9204", + "Window 3", + ] + for query in largeQueries { + XCTAssertEqual( + optimizedResults(entries: largeEntries, query: query), + referenceResults(entries: largeEntries, query: query), + "Large workspace benchmark corpus mismatch for query \(query)" + ) + } + + // Fast-typing paths: the capped full-corpus search and the + // visible-candidate preview search must return the same ordered results + // (and highlights) the uncapped search would show for that corpus. + let previewEntries = Array(largeEntries.prefix(16)) + for query in fastTypingPrefixes("cmd-p-search").suffix(6) { + let fullResults = optimizedResults(entries: largeEntries, query: query) + XCTAssertEqual( + optimizedResults(entries: largeEntries, query: query, resultLimit: 8), + Array(fullResults.prefix(8)), + "Capped full-corpus search diverged from full search for prefix \(query)" + ) + XCTAssertEqual( + optimizedResults(entries: previewEntries, query: query, resultLimit: 8), + Array(optimizedResults(entries: previewEntries, query: query).prefix(8)), + "Visible-candidate preview search diverged from full search for prefix \(query)" + ) + } + } + func testMultiTokenSearchCanMatchAcrossTitleAndKeywordFields() { let entries = [ FixtureEntry( @@ -2259,7 +2381,8 @@ final class CommandPaletteSearchEngineTests: XCTestCase { XCTAssertNotEqual(base, changedSurfaceKind) } - func testCommandSearchBenchmarkBeatsLegacyPipeline() { + func testCommandSearchBenchmarkBeatsLegacyPipeline() throws { + try skipUnlessSearchBenchmarksAreEnabled() let entries = makeCommandEntries(count: 900) let corpus = entries.map { entry in CommandPaletteSearchCorpusEntry( @@ -2300,7 +2423,8 @@ final class CommandPaletteSearchEngineTests: XCTestCase { ) } - func testSwitcherSearchBenchmarkBeatsLegacyPipeline() { + func testSwitcherSearchBenchmarkBeatsLegacyPipeline() throws { + try skipUnlessSearchBenchmarksAreEnabled() let entries = makeSwitcherEntries(count: 400) let corpus = entries.map { entry in CommandPaletteSearchCorpusEntry( @@ -2341,7 +2465,8 @@ final class CommandPaletteSearchEngineTests: XCTestCase { ) } - func testLargeWorkspaceSwitcherSearchBenchmarkAvoidsPerQueryPreparationCost() { + func testLargeWorkspaceSwitcherSearchBenchmarkAvoidsPerQueryPreparationCost() throws { + try skipUnlessSearchBenchmarksAreEnabled() let entries = makeLargeWorkspaceSwitcherEntries(count: 800) let corpus = entries.map { entry in CommandPaletteSearchCorpusEntry( @@ -2391,7 +2516,8 @@ final class CommandPaletteSearchEngineTests: XCTestCase { ) } - func testFastTypingPreviewSearchBenchmarkReportsEstimatedDroppedFrames() { + func testFastTypingPreviewSearchBenchmarkReportsEstimatedDroppedFrames() throws { + try skipUnlessSearchBenchmarksAreEnabled() let entries = makeLargeWorkspaceSwitcherEntries(count: 800) let corpus = entries.map { entry in CommandPaletteSearchCorpusEntry( diff --git a/cmuxTests/GlobalSearchInputOwnershipTests.swift b/cmuxTests/GlobalSearchInputOwnershipTests.swift index 0a8ca1e27c91..2c680ed0ffe7 100644 --- a/cmuxTests/GlobalSearchInputOwnershipTests.swift +++ b/cmuxTests/GlobalSearchInputOwnershipTests.swift @@ -23,6 +23,11 @@ extension GlobalSearchShortcutBehaviorTests { startWatching: false ) KeyboardShortcutSettings.resetAll() + // The previous test dismisses the palette on its way out, but NSPopover + // animates the close, so `isShown` stays true until the run loop turns. + // Settle it here so no test starts with the last test's palette open. + GlobalSearchCoordinator.shared.dismissPalette() + _ = Self.waitUntilGlobalSearchCloses() } deinit { @@ -30,6 +35,20 @@ extension GlobalSearchShortcutBehaviorTests { KeyboardShortcutSettings.resetAll() } + private static func waitUntilGlobalSearchCloses(timeout: TimeInterval = 2) -> Bool { + let deadline = Date.now.addingTimeInterval(timeout) + repeat { + if !GlobalSearchCoordinator.shared.isPaletteVisible() { + return true + } + _ = RunLoop.main.run( + mode: .default, + before: min(deadline, Date.now.addingTimeInterval(0.01)) + ) + } while Date.now < deadline + return !GlobalSearchCoordinator.shared.isPaletteVisible() + } + @Test func browserFocusModeOwnsGlobalSearchShortcut() throws { #if DEBUG let appDelegate = try #require(AppDelegate.shared) diff --git a/cmuxTests/GlobalSearchShortcutPriorityTests.swift b/cmuxTests/GlobalSearchShortcutPriorityTests.swift index 9c062469cd30..fb4d365c2391 100644 --- a/cmuxTests/GlobalSearchShortcutPriorityTests.swift +++ b/cmuxTests/GlobalSearchShortcutPriorityTests.swift @@ -23,6 +23,11 @@ extension GlobalSearchShortcutBehaviorTests { startWatching: false ) KeyboardShortcutSettings.resetAll() + // The previous test dismisses the palette on its way out, but NSPopover + // animates the close, so `isShown` stays true until the run loop turns. + // Settle it here so no test starts with the last test's palette open. + GlobalSearchCoordinator.shared.dismissPalette() + _ = Self.waitUntilGlobalSearchCloses() } deinit { @@ -30,6 +35,20 @@ extension GlobalSearchShortcutBehaviorTests { KeyboardShortcutSettings.resetAll() } + private static func waitUntilGlobalSearchCloses(timeout: TimeInterval = 2) -> Bool { + let deadline = Date.now.addingTimeInterval(timeout) + repeat { + if !GlobalSearchCoordinator.shared.isPaletteVisible() { + return true + } + _ = RunLoop.main.run( + mode: .default, + before: min(deadline, Date.now.addingTimeInterval(0.01)) + ) + } while Date.now < deadline + return !GlobalSearchCoordinator.shared.isPaletteVisible() + } + @Test func rightSidebarModeOwnsOverlappingGlobalSearchShortcut() throws { #if DEBUG let appDelegate = try #require(AppDelegate.shared) diff --git a/cmuxTests/OpenCodeHookRegressionTests.swift b/cmuxTests/OpenCodeHookRegressionTests.swift index 3a59cb205eba..95db65577827 100644 --- a/cmuxTests/OpenCodeHookRegressionTests.swift +++ b/cmuxTests/OpenCodeHookRegressionTests.swift @@ -23,7 +23,11 @@ final class OpenCodeHookRegressionTests: XCTestCase { try fileManager.createDirectory(at: root, withIntermediateDirectories: true) defer { try? fileManager.removeItem(at: root) } - let socketPath = root.appendingPathComponent("cmux.sock").path + // WHY /tmp: a Unix socket path must fit sun_path (104 bytes). Under a + // runner's `/private/var/folders/.../T/` the temporary directory plus + // this UUID-named root overflows it and the harness `listen` fails. + let socketPath = "/tmp/cmux-oc-\(UUID().uuidString.prefix(8)).sock" + defer { unlink(socketPath) } let harnessURL = root.appendingPathComponent("harness.js") try Self.openCodeFeedEventHarness.write(to: harnessURL, atomically: true, encoding: .utf8) let bunURL = try Self.bunExecutableURL() diff --git a/cmuxTests/PortScannerTests.swift b/cmuxTests/PortScannerTests.swift index 9a024259d6fd..2ae7678e9dc5 100644 --- a/cmuxTests/PortScannerTests.swift +++ b/cmuxTests/PortScannerTests.swift @@ -1257,6 +1257,33 @@ struct PortScannerLsofBatchingTests { @Suite("Port scanner retirement end to end") struct PortScannerPortRetirementTests { + /// The production burst spans ten seconds, so these tests drive the scanner + /// on a compressed schedule of the same shape: six scans, one burst, the + /// same coalesce step. Only the wall-clock spacing shrinks; the scan count + /// and the ordering the reconciler depends on are unchanged. + private static let fastBurstOffsets: [TimeInterval] = [0.05, 0.15, 0.3, 0.45, 0.6, 0.75] + /// Same six-scan burst, but with the final scan left far enough behind the + /// fifth that a kick issued from inside the fifth scan's `lsof` reaches the + /// scanner queue while the burst still owes exactly one scan — the case the + /// late-burst test covers. The gap only has to outlast the scanner's own + /// hop from the fifth timer to that `lsof` call, not a test-task wakeup. + private static let fastLateBurstOffsets: [TimeInterval] = [0.05, 0.15, 0.3, 0.45, 0.6, 1.6] + /// The compressed stand-in for the production 200ms coalesce step. No test + /// here kicks repeatedly while it waits, so nothing is racing this window: + /// each kick is issued once and the scanner's own guarantee of + /// `minimumScansPerKick` scans per kick carries the rest. + private static let fastCoalesceDelay: TimeInterval = 0.01 + + /// The compressed schedules above only stand in for production if the + /// shipped cadence still has the shape they mimic. + @Test("The production scan schedule keeps its six-scan burst and coalesce window") + func productionScanScheduleMatchesCompressedShape() { + #expect(PortScanner.defaultBurstOffsets == [0.5, 1.5, 3, 5, 7.5, 10]) + #expect(PortScanner.defaultCoalesceDelay == 0.2) + #expect(Self.fastBurstOffsets.count == PortScanner.defaultBurstOffsets.count) + #expect(Self.fastLateBurstOffsets.count == PortScanner.defaultBurstOffsets.count) + } + /// Drives the whole scanner — TTY registration, kick, coalesce, burst, /// reconcile, publish — so a break anywhere in that chain surfaces even /// when every individual stage still passes its own test. @@ -1283,7 +1310,9 @@ struct PortScannerPortRetirementTests { let sessionIdentity = TerminalTTYSessionIdentity(processIdentity: listenerIdentity) let scanner = PortScanner( commandRunner: runner, - ttySessionIdentityProvider: { _ in sessionIdentity } + ttySessionIdentityProvider: { _ in sessionIdentity }, + burstOffsets: Self.fastBurstOffsets, + coalesceDelay: Self.fastCoalesceDelay ) let publishedPorts = OSAllocatedUnfairLock(initialState: [[Int]]()) @@ -1299,7 +1328,7 @@ struct PortScannerPortRetirementTests { let didPublishListeningPort = await Self.waitForPublication( in: publishedPorts, matching: { $0 == [listeningPort] }, - onKick: { scanner.kick(workspaceId: workspaceId, panelId: panelId) } + pollInterval: .milliseconds(25) ) try #require(didPublishListeningPort, "the listening port was never published") @@ -1307,12 +1336,16 @@ struct PortScannerPortRetirementTests { // retirement; an earlier empty publication is registration noise. let publicationsBeforeStop = publishedPorts.withLock { $0.count } await runner.stopListening() + // One kick, not one per poll: a kick guarantees `minimumScansPerKick` + // scans, which is exactly the number of complete misses the reconciler + // needs to retire the port. + scanner.kick(workspaceId: workspaceId, panelId: panelId) let didRetirePort = await Self.waitForPublication( in: publishedPorts, after: publicationsBeforeStop, matching: \.isEmpty, - onKick: { scanner.kick(workspaceId: workspaceId, panelId: panelId) } + pollInterval: .milliseconds(25) ) #expect(didRetirePort, "the port was never retired after its process stopped listening") @@ -1338,7 +1371,9 @@ struct PortScannerPortRetirementTests { let sessionIdentity = TerminalTTYSessionIdentity(processIdentity: listenerIdentity) let scanner = PortScanner( commandRunner: runner, - ttySessionIdentityProvider: { _ in sessionIdentity } + ttySessionIdentityProvider: { _ in sessionIdentity }, + burstOffsets: Self.fastLateBurstOffsets, + coalesceDelay: Self.fastCoalesceDelay ) let publishedPorts = OSAllocatedUnfairLock(initialState: [[Int]]()) @@ -1349,30 +1384,35 @@ struct PortScannerPortRetirementTests { } scanner.registerTTY(workspaceId: workspaceId, panelId: panelId, ttyName: ttyName) } + // The fifth scan leaves only the last scan of the six-scan burst. + // Stopping there means clearing the kick at that scan strands the port + // after only one complete miss, so the kick must survive the burst. + // The runner stops and kicks from inside the fifth `lsof` call, after + // that call reports the port, so the stop is tied to the scan itself + // rather than to when this task happens to observe it. + await runner.stopListening(afterLsofInvocation: 5) { + scanner.kick(workspaceId: workspaceId, panelId: panelId) + } scanner.kick(workspaceId: workspaceId, panelId: panelId) let didPublishListeningPort = await Self.waitForPublication( in: publishedPorts, matching: { $0 == [listeningPort] }, - onKick: {} + pollInterval: .milliseconds(10) ) try #require(didPublishListeningPort, "the listening port was never published") - - // The fifth scan is at 7.5 seconds in the six-scan burst. Stopping here - // leaves only the 10-second scan in the original burst, so clearing the - // kick at that scan strands the port after only one complete miss. - let reachedFifthScan = await runner.waitForLsofInvocation(5) - try #require(reachedFifthScan, "the scanner did not reach the fifth burst scan") - let publicationsBeforeStop = publishedPorts.withLock { $0.count } - await runner.stopListening() - scanner.kick(workspaceId: workspaceId, panelId: panelId) + // Retirement is the first empty publication after the port appeared; + // an earlier empty publication is registration noise. + let firstListeningPublication = try #require( + publishedPorts.withLock { $0.firstIndex(of: [listeningPort]) } + ) let didRetirePort = await Self.waitForPublication( in: publishedPorts, - after: publicationsBeforeStop, + after: firstListeningPublication + 1, matching: \.isEmpty, - onKick: {}, - timeout: .seconds(12) + timeout: .seconds(12), + pollInterval: .milliseconds(10) ) #expect(didRetirePort, "a late-burst kick did not schedule enough complete misses") @@ -1420,7 +1460,6 @@ struct PortScannerPortRetirementTests { let didPublishListeningPort = await Self.waitForPublication( in: publishedPorts, matching: { $0 == [listeningPort] }, - onKick: { scanner.kick(workspaceId: workspaceId, panelId: panelId) }, timeout: .seconds(6) ) @@ -1428,17 +1467,23 @@ struct PortScannerPortRetirementTests { } /// Polls rather than sleeping a fixed interval, since the scan burst runs - /// on real timers whose spacing shifts under load. + /// on real timers whose spacing shifts under load. The deadline bounds only + /// the failure path: a satisfied predicate returns immediately. /// - /// The interval must stay above the scanner's 200ms kick coalesce window: - /// each kick reschedules that timer, so polling faster than it starves the - /// burst and no scan ever runs. + /// This only observes; it never kicks. Kicking from the poll loop is a + /// flake vector, not a nudge: `PortScanner.kick()` re-arms the coalesce + /// timer whenever no burst is running, so on a loaded runner — where timer + /// jitter is the same order as the coalesce window — a stream of polls can + /// cancel that timer forever and no scan ever runs. Each caller kicks once + /// instead, which the scanner already answers with a guaranteed + /// `minimumScansPerKick` scans. That makes the poll interval a pure + /// latency/CPU tradeoff, independent of the coalesce delay. private static func waitForPublication( in publishedPorts: OSAllocatedUnfairLock<[[Int]]>, after startIndex: Int = 0, matching predicate: @Sendable ([Int]) -> Bool, - onKick: @Sendable () -> Void, - timeout: Duration = .seconds(20) + timeout: Duration = .seconds(20), + pollInterval: Duration = .milliseconds(500) ) async -> Bool { func isSatisfied() -> Bool { publishedPorts.withLock { $0.dropFirst(startIndex).contains(where: predicate) } @@ -1446,11 +1491,10 @@ struct PortScannerPortRetirementTests { let deadline = ContinuousClock.now + timeout while ContinuousClock.now < deadline { if isSatisfied() { return true } - onKick() // Cancellation makes the sleep throw immediately; without this the // poll would spin until the wall-clock deadline. do { - try await Task.sleep(for: .milliseconds(500)) + try await Task.sleep(for: pollInterval) } catch { break } @@ -1470,6 +1514,7 @@ private actor PortLifecycleCommandRunner: CommandRunning { private var isListening = true private(set) var lastLsofArguments: [String]? private var lsofInvocationCount = 0 + private var scheduledStop: (invocation: Int, action: @Sendable () -> Void)? private static let filesystemWarning = """ lsof: WARNING: can't stat() smbfs file system /Volumes/.timemachine/example @@ -1496,16 +1541,14 @@ private actor PortLifecycleCommandRunner: CommandRunning { isListening = false } - func waitForLsofInvocation(_ target: Int, timeout: Duration = .seconds(15)) async -> Bool { - let deadline = ContinuousClock.now + timeout - while lsofInvocationCount < target, ContinuousClock.now < deadline { - do { - try await Task.sleep(for: .milliseconds(50)) - } catch { - return false - } - } - return lsofInvocationCount >= target + /// Stops listening from inside the `target`th `lsof` call, after that call + /// has reported the port, then runs `action` while the call is still in + /// flight. Must be armed before that call happens. + func stopListening( + afterLsofInvocation target: Int, + then action: @escaping @Sendable () -> Void + ) { + scheduledStop = (target, action) } func run( @@ -1532,7 +1575,13 @@ private actor PortLifecycleCommandRunner: CommandRunning { // PID-scoped TCP socket query, but any stderr currently makes the // scanner globally incomplete and prevents stale ports from aging out. let stderr = arguments.contains("-w") ? "" : Self.filesystemWarning - guard isListening, Self.selection(for: "-p", in: arguments).contains(String(pid)) else { + let reportsPort = isListening && Self.selection(for: "-p", in: arguments).contains(String(pid)) + if let stop = scheduledStop, stop.invocation == lsofInvocationCount { + scheduledStop = nil + isListening = false + stop.action() + } + guard reportsPort else { return Self.noSelectedFiles(stderr: stderr) } return Self.output("p\(pid)\nf3\nn127.0.0.1:\(port)\n", stderr: stderr) diff --git a/cmuxTests/RemoteShellCWDRelayTests.swift b/cmuxTests/RemoteShellCWDRelayTests.swift index c8aa6d067181..5197a13d7219 100644 --- a/cmuxTests/RemoteShellCWDRelayTests.swift +++ b/cmuxTests/RemoteShellCWDRelayTests.swift @@ -118,8 +118,11 @@ struct RemoteShellCWDRelayTests { _CMUX_PORTS_LAST_RUN=$(_cmux_now) _CMUX_PWD_LAST_PWD="/tmp/local-launch" _cmux_precmd - repeat 20; do - [[ -s "\(logPath.path)" ]] && break + # _cmux_precmd reports shell state and the pwd from separate + # background calls, so wait for the pwd line itself rather than + # for the first line of any kind. + repeat 100; do + [[ -s "\(logPath.path)" && "$(<"\(logPath.path)")" == *surface.report_pwd* ]] && break sleep 0.05 done cat "\(logPath.path)" diff --git a/cmuxTests/SSHDeepSleepReattachTests.swift b/cmuxTests/SSHDeepSleepReattachTests.swift index f323911b601d..ba0879faf06d 100644 --- a/cmuxTests/SSHDeepSleepReattachTests.swift +++ b/cmuxTests/SSHDeepSleepReattachTests.swift @@ -1,5 +1,6 @@ import AppKit import CmuxCore +import CmuxFoundation import Foundation import Testing @@ -307,7 +308,55 @@ struct SSHDeepSleepReattachTests { #expect(restartedSnapshot.remotePTYSessionID == customSessionID) } - @Test(arguments: [(nil, Int32(255), "21", 20), ("2O", Int32(255), "21", 20)]) + /// The attach wrapper resolves its retry budget before the loop starts, so + /// the fallback, a well-formed operator budget, and the ceiling are all + /// provable without paying for a full budget of attach attempts. + @Test(arguments: [ + (nil, SSHReconnectBudget().fallbackLimit), + ("2O", SSHReconnectBudget().fallbackLimit), + ("0", SSHReconnectBudget().fallbackLimit), + ("021", 21), + ("21", 21), + (String(SSHReconnectBudget().maximumLimit + 1), SSHReconnectBudget().maximumLimit), + ] as [(String?, Int)]) + func foregroundAuthenticatedAttachResolvesRetryBudgetBeforeTheLoop( + reconnectLimit: String?, + expectedLimit: Int + ) throws { + let attachLines = SSHPTYAttachRetryScriptBuilder().lines( + command: "cmux_ssh_attach_attempt", + reauthenticates: true + ) + let budgetResolution = Array(attachLines.prefix { $0 != "while :; do" }) + #expect( + budgetResolution.count < attachLines.count, + "the attach retry loop marker moved; the budget probe would run the whole loop" + ) + // The generated startup command embeds these same lines, so the probe + // cannot drift away from what a real pane runs. + let startupCommand = SSHPTYAttachStartupCommandBuilder.command( + sessionID: "ssh-test-session", + foregroundAuth: Self.foregroundAuth() + ) + let clampLine = try #require(budgetResolution.first { $0.contains("CMUX_SSH_RECONNECT_LIMIT") }) + #expect(startupCommand.contains(clampLine)) + + var environment = ProcessInfo.processInfo.environment + environment["CMUX_SSH_RECONNECT_LIMIT"] = reconnectLimit + let result = Self.runProcessCapturingStandardOutput( + command: (budgetResolution + ["printf '%s' \"$cmux_ssh_attach_reconnect_limit\""]) + .joined(separator: "\n"), + environment: environment + ) + + #expect(!result.timedOut, Comment(rawValue: result.stderr)) + #expect(result.status == 0, Comment(rawValue: result.stderr)) + // Absent, malformed, and zero budgets fall back to the finite default; + // a well-formed budget is honored up to the shared ceiling (#13959). + #expect(result.stdout == String(expectedLimit), Comment(rawValue: result.stderr)) + } + + @Test(arguments: [(Optional("4"), Int32(255), "5", 4)]) func foregroundAuthenticatedAttachUsesConfiguredRetryBudget( reconnectLimit: String?, expectedStatus: Int32, expectedAttempts: String, expectedSleepCount: Int ) throws { @@ -532,6 +581,36 @@ struct SSHDeepSleepReattachTests { try (lines.joined(separator: "\n") + "\n").write(to: url, atomically: true, encoding: .utf8) } + /// Runs a short shell probe and keeps its stdout, for scripts that report a + /// resolved value instead of exercising a retry loop. + private static func runProcessCapturingStandardOutput( + command: String, + environment: [String: String] + ) -> (status: Int32, stdout: String, stderr: String, timedOut: Bool) { + let process = Process() + let stdoutPipe = Pipe() + let stderrPipe = Pipe() + process.executableURL = URL(fileURLWithPath: "/bin/sh") + process.arguments = ["-c", command] + process.environment = environment + process.standardInput = FileHandle.nullDevice + process.standardOutput = stdoutPipe + process.standardError = stderrPipe + do { + try process.run() + } catch { + return (-1, "", String(describing: error), false) + } + let timedOut = waitForProcessExit(process, timeout: 10) == .timedOut + if timedOut { + process.terminate() + _ = waitForProcessExit(process, timeout: 1) + } + let stdout = String(data: stdoutPipe.fileHandleForReading.readDataToEndOfFile(), encoding: .utf8) ?? "" + let stderr = String(data: stderrPipe.fileHandleForReading.readDataToEndOfFile(), encoding: .utf8) ?? "" + return (process.terminationStatus, stdout, stderr, timedOut) + } + private static func runProcess( command: String, environment: [String: String], diff --git a/cmuxTests/SSHStartupSignalLifecycleTests.swift b/cmuxTests/SSHStartupSignalLifecycleTests.swift index ffd845da6e61..0842d9435f71 100644 --- a/cmuxTests/SSHStartupSignalLifecycleTests.swift +++ b/cmuxTests/SSHStartupSignalLifecycleTests.swift @@ -895,19 +895,31 @@ extension CLINotifyProcessIntegrationRegressionTests { environment["CMUX_TEST_SLEEP_LOG"] = sleepLog.path environment["CMUX_SSH_RECONNECT_DELAY_SECONDS"] = "0" environment["CMUX_SSH_RECONNECT_LIMIT"] = "2" + environment["CMUX_CLI_SENTRY_DISABLED"] = "1" - let result = runProcess( + let child = try StreamingChildProcess( executablePath: "/bin/sh", arguments: ["-c", startupCommand], - environment: environment, - timeout: 1 + environment: environment ) + defer { child.terminate() } - XCTAssertTrue(result.timedOut, "closed stdin must not dismiss the terminal failure prompt") + // The prompt is the wrapper's own completion signal: it is printed only + // after the retry loop gave up and reported the session end. + XCTAssertTrue( + child.waitForStandardError( + containing: "[cmux] press Enter to close this pane.", + timeout: 20 + ), + child.standardError + ) + XCTAssertTrue( + child.waitUntilBlocked(timeout: 10), + "closed stdin must not dismiss the terminal failure prompt: \(child.standardError)" + ) XCTAssertEqual((try? String(contentsOf: attemptFile, encoding: .utf8))?.trimmingCharacters(in: .whitespacesAndNewlines), "3") XCTAssertEqual(try String(contentsOf: sleepLog, encoding: .utf8), "2\n2\n") - XCTAssertTrue(result.stderr.contains("[cmux] ssh exited with status 255."), result.stderr) - XCTAssertTrue(result.stderr.contains("[cmux] press Enter to close this pane."), result.stderr) + XCTAssertTrue(child.standardError.contains("[cmux] ssh exited with status 255."), child.standardError) let recordedCalls = (try? String(contentsOf: logFile, encoding: .utf8)) ?? "" let sessionEndCalls = recordedCalls .split(separator: "\n") @@ -954,15 +966,26 @@ extension CLINotifyProcessIntegrationRegressionTests { environment["CMUX_TEST_SESSION_END_LOG"] = logFile.path environment["CMUX_TEST_ATTEMPT_FILE"] = attemptFile.path environment["CMUX_SSH_RECONNECT_DELAY_SECONDS"] = "0" + environment["CMUX_CLI_SENTRY_DISABLED"] = "1" - let result = runProcess( + let child = try StreamingChildProcess( executablePath: "/bin/sh", arguments: ["-c", startupCommand], - environment: environment, - timeout: 1 + environment: environment ) + defer { child.terminate() } - XCTAssertTrue(result.timedOut, "closed stdin must not dismiss the terminal failure prompt") + XCTAssertTrue( + child.waitForStandardError( + containing: "[cmux] press Enter to close this pane.", + timeout: 20 + ), + child.standardError + ) + XCTAssertTrue( + child.waitUntilBlocked(timeout: 10), + "closed stdin must not dismiss the terminal failure prompt: \(child.standardError)" + ) XCTAssertEqual((try? String(contentsOf: attemptFile, encoding: .utf8))?.trimmingCharacters(in: .whitespacesAndNewlines), "1") let recordedCalls = (try? String(contentsOf: logFile, encoding: .utf8)) ?? "" let sessionEndCalls = recordedCalls @@ -1004,20 +1027,30 @@ extension CLINotifyProcessIntegrationRegressionTests { environment["CMUX_SURFACE_ID"] = "22222222-2222-2222-2222-222222222222" environment["CMUX_TEST_SESSION_END_LOG"] = logFile.path environment["CMUX_SSH_RECONNECT_DELAY_SECONDS"] = "0" + environment["CMUX_CLI_SENTRY_DISABLED"] = "1" - let result = runProcess( + let child = try StreamingChildProcess( executablePath: "/bin/sh", arguments: ["-c", startupCommand], - environment: environment, - timeout: 1 + environment: environment ) + defer { child.terminate() } // A child status is an ordinary session failure, unlike a signal sent // to the supervisor. Keep its status visible until a fresh Enter; EOF // must not dismiss the failure prompt (the #9966 contract). - XCTAssertTrue(result.timedOut, "closed stdin must not dismiss the terminal failure prompt") - XCTAssertTrue(result.stderr.contains("[cmux] ssh exited with status 130."), result.stderr) - XCTAssertTrue(result.stderr.contains("[cmux] press Enter to close this pane."), result.stderr) + XCTAssertTrue( + child.waitForStandardError( + containing: "[cmux] press Enter to close this pane.", + timeout: 20 + ), + child.standardError + ) + XCTAssertTrue( + child.waitUntilBlocked(timeout: 10), + "closed stdin must not dismiss the terminal failure prompt: \(child.standardError)" + ) + XCTAssertTrue(child.standardError.contains("[cmux] ssh exited with status 130."), child.standardError) let recordedCalls = (try? String(contentsOf: logFile, encoding: .utf8)) ?? "" let sessionEndCalls = recordedCalls .split(separator: "\n") @@ -1150,17 +1183,27 @@ extension CLINotifyProcessIntegrationRegressionTests { environment["CMUX_SURFACE_ID"] = "22222222-2222-2222-2222-222222222222" environment["CMUX_TEST_SESSION_END_LOG"] = logFile.path environment["CMUX_SSH_RECONNECT_DELAY_SECONDS"] = "0" + environment["CMUX_CLI_SENTRY_DISABLED"] = "1" - let result = runProcess( + let child = try StreamingChildProcess( executablePath: "/bin/sh", arguments: ["-c", startupCommand], - environment: environment, - timeout: 1 + environment: environment ) + defer { child.terminate() } - XCTAssertTrue(result.timedOut, "closed stdin must not dismiss the terminal failure prompt") - XCTAssertTrue(result.stderr.contains("[cmux] ssh exited with status 1."), result.stderr) - XCTAssertTrue(result.stderr.contains("[cmux] press Enter to close this pane."), result.stderr) + XCTAssertTrue( + child.waitForStandardError( + containing: "[cmux] press Enter to close this pane.", + timeout: 20 + ), + child.standardError + ) + XCTAssertTrue( + child.waitUntilBlocked(timeout: 10), + "closed stdin must not dismiss the terminal failure prompt: \(child.standardError)" + ) + XCTAssertTrue(child.standardError.contains("[cmux] ssh exited with status 1."), child.standardError) } func testSSHStartupForwardsStdinToBackgroundedSSH() throws { @@ -1562,3 +1605,207 @@ extension CLINotifyProcessIntegrationRegressionTests { return condition(contents) } } + +/// A child process whose output is streamed while it runs. +/// +/// Startup-wrapper tests that end at the terminal exit prompt used to prove +/// "the pane stays open" by letting a blocking `runProcess` call burn its whole +/// timeout (20s under CI). This type lets those tests wait on the real signals +/// instead: the prompt text the wrapper prints, and the prompt helper blocking +/// with its input at EOF. Both return the instant they hold, so the timeouts +/// bound only the failure path. +final class StreamingChildProcess: @unchecked Sendable { + let process = Process() + + private let stdoutPipe = Pipe() + private let stderrPipe = Pipe() + private let outputLock = NSLock() + private let drainGroup = DispatchGroup() + private var stdoutData = Data() + private var stderrData = Data() + + init( + executablePath: String, + arguments: [String], + environment: [String: String] + ) throws { + process.executableURL = URL(fileURLWithPath: executablePath) + process.arguments = arguments + process.environment = CLIChildEnvironment( + appHostEnvironment: ProcessInfo.processInfo.environment + ).normalizing(environment) + // The #9966 contract under test is "input already at EOF must not + // dismiss the failure prompt", so the child starts with a closed stdin. + process.standardInput = FileHandle.nullDevice + process.standardOutput = stdoutPipe + process.standardError = stderrPipe + try process.run() + + drain(stdoutPipe) { [weak self] data in + guard let self else { return } + self.outputLock.lock() + self.stdoutData.append(data) + self.outputLock.unlock() + } + drain(stderrPipe) { [weak self] data in + guard let self else { return } + self.outputLock.lock() + self.stderrData.append(data) + self.outputLock.unlock() + } + } + + var standardOutput: String { + outputLock.lock() + defer { outputLock.unlock() } + return String(data: stdoutData, encoding: .utf8) ?? "" + } + + var standardError: String { + outputLock.lock() + defer { outputLock.unlock() } + return String(data: stderrData, encoding: .utf8) ?? "" + } + + /// Returns as soon as `text` has been written to stderr. + /// + /// A child that exits before emitting `text` resolves immediately from its + /// drained output rather than waiting out `timeout`. + func waitForStandardError(containing text: String, timeout: TimeInterval) -> Bool { + let deadline = Date.now.addingTimeInterval(timeout) + while Date.now < deadline { + if standardError.contains(text) { return true } + if !process.isRunning { + _ = drainGroup.wait(timeout: .now() + 2) + return standardError.contains(text) + } + Thread.sleep(forTimeInterval: 0.005) + } + return standardError.contains(text) + } + + /// Returns true once the child's process tree is alive but consuming no + /// CPU, i.e. the prompt helper is parked in a blocking wait rather than on + /// its way to exiting. + /// + /// The startup script prints the prompt text and then execs the CLI + /// helper. Whether the helper keeps the launched pid depends on the shell: + /// `/bin/sh -c