diff --git a/Packages/macOS/CmuxNext/Sources/CmuxNextApp/Focus/CreationInputCoordinator.swift b/Packages/macOS/CmuxNext/Sources/CmuxNextApp/Focus/CreationInputCoordinator.swift index 489b0e3e4f76..f14717ab1da2 100644 --- a/Packages/macOS/CmuxNext/Sources/CmuxNextApp/Focus/CreationInputCoordinator.swift +++ b/Packages/macOS/CmuxNext/Sources/CmuxNextApp/Focus/CreationInputCoordinator.swift @@ -49,6 +49,7 @@ final class CreationInputCoordinator { /// script or agent run without `focus`), which leaves the keys where they are. func begin(in window: NSWindow?, generation: UInt64?, earlyTab: String? = nil, tab: String? = nil, machine: String? = nil) -> CreationInputBuffer.Ticket? { guard let window, let generation, ActionRunScope.viewChangeAllowed() else { return nil } + defer { router?.newTabInputCoordinator.creationTookKeys(in: window) } // the newer intent: a New Tab hold's keys come here, in order return buffer(in: window).open(generation: generation, earlyTab: earlyTab, tab: tab, machine: machine ?? router?.focus(for: window).0?.content?.daemon.machineID) } diff --git a/Packages/macOS/CmuxNext/Sources/CmuxNextApp/Focus/KeyRouter+NewTabInput.swift b/Packages/macOS/CmuxNext/Sources/CmuxNextApp/Focus/KeyRouter+NewTabInput.swift index abcb4d6bdf8e..12c59a7942fa 100644 --- a/Packages/macOS/CmuxNext/Sources/CmuxNextApp/Focus/KeyRouter+NewTabInput.swift +++ b/Packages/macOS/CmuxNext/Sources/CmuxNextApp/Focus/KeyRouter+NewTabInput.swift @@ -361,10 +361,7 @@ final class NewTabInputCoordinator { } /// Ends the window's hold and gives its keys, in typing order, to whatever has focus. - private func release(in window: NSWindow, why: String = "replaced") { - guard let entry = entry(for: window) else { return } - release(entry, why: why) - } + private func release(in window: NSWindow, why: String = "replaced") { if let entry = entry(for: window) { release(entry, why: why) } } private func release(_ entry: Entry, why: String) { remove(entry) @@ -390,10 +387,12 @@ final class NewTabInputCoordinator { } /// Whether `window` holds keys for an opening (debug and tests). - func isHolding(in window: NSWindow?) -> Bool { - guard let window else { return false } - return buffers[window.windowNumber] != nil - } + func isHolding(in window: NSWindow?) -> Bool { window.map { buffers[$0.windowNumber] != nil } ?? false } + + /// A creation that moves `window`'s focus opened its hold (``CreationInputCoordinator/begin``), the person's newer intent: this + /// hold ends, and its keys run again in typing order through the key router, into that hold. Before, keys typed after Cmd-N + /// then Ctrl-` waited for the New Tab page and went to its field (#30271 smoke, step 16). + func creationTookKeys(in window: NSWindow) { release(in: router?.focus(for: window).0?.window ?? window, why: "a creation took the keys") } } /// The shown native New Tab page under `view`, depth first. diff --git a/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/CmdNThenCreateKeysTests.swift b/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/CmdNThenCreateKeysTests.swift new file mode 100644 index 000000000000..60e83b353a07 --- /dev/null +++ b/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/CmdNThenCreateKeysTests.swift @@ -0,0 +1,107 @@ +import AppKit +import CmuxNextActions +@testable import CmuxNextApp +@testable import CmuxNextDaemon +import Darwin +import Foundation +import Testing + +/// Judged smoke of dogfood v2.2.13-opt, round 2 step 16 (PR #30271): after Cmd-N, Ctrl-` and a typed line, the line's +/// keys went to the New Tab page, not the new terminal. Cmd-N opens the window's New Tab hold (`beginIncoming`): every +/// key waits for the new page's field. A Ctrl-` (or Cmd-D) pressed then is the person's newer intent, but nothing ended +/// that hold, so the keys typed after it still waited for the page and went to its field. The pass condition is what +/// the person sees: every typed key reaches the NEW terminal, in order (its screen shows `echo held` and the shell's +/// answer `held`), and no other terminal gets them. Each key goes through the app's dispatch (`KeyRouter.interceptKeyDown`, +/// then the window), and a held key runs again the same way (the test window is never key). +@MainActor @Suite(.serialized, .timeLimit(.minutes(3)), .enabled(if: SameTreeDaemonBinary.url != nil, "needs the same-tree cmux-tui")) +struct CmdNThenCreateKeysTests { + @Test func keysTypedAfterCmdNThenCtrlBacktickReachTheNewTerminal() async throws { + try await Self.typeAfterCmdN(then: "newSurface", invocation: ActionInvocation(origin: .user)) + } + + /// The same seam for Cmd-D (a split that opens a terminal): its creation hold takes the keys from the page's hold. + @Test func keysTypedAfterCmdNThenCmdDReachTheNewTerminal() async throws { + var terminal = ActionInvocation(origin: .user) + terminal.opensTerminal = true + try await Self.typeAfterCmdN(then: "splitDown", invocation: terminal) + } + + /// Cmd-N with the daemon stopped (its workspace stays a stand-in), then `action`, then "echo held" Return, each key + /// through the app's dispatch; then the daemon runs, and the new terminal must show the line and its answer. + private static func typeAfterCmdN(then action: ActionID, invocation: ActionInvocation) async throws { + try await PasswordManagerRouteTests.withApp(heldKeyReplay: { event, window, router in + if !router.interceptKeyDown(event, in: window) { window.sendEvent(event) } + }) { services, connection in + services.agentTabs.localHost = AgentSessionRef.host(installID: "cn-cmdn-create-keys") + try #require(services.agentTabs.canHostChat, "this build has no New Tab page") + let pid = try #require(services.daemon.identity?.pid) + let store = services.daemon.store + let before = Set(store.workspaces.map(\.id)) + let router = services.keyRouter + let coordinator = router.creationInputCoordinator + // The page's hold keeps its keys as long as the page is opening, as for a person: their New Tab page reports + // its field ready and takes the keys (smoke step 16). This harness's page never reports ready, so its 4 s + // bound would hand the keys on and hide the loss (red 808acd14 passed on the base for that reason). + router.newTabInputCoordinator.incomingHoldLimit = .seconds(120) + + #expect(kill(pid, SIGSTOP) == 0) + var resumed = false + defer { if !resumed { kill(pid, SIGCONT) } } + var work = services.registry.capturingWork { + _ = services.registry.perform("newTab", invocation: ActionInvocation(origin: .user)) + } + let key = try #require(store.workspaces.first { !before.contains($0.id) }?.id, "Cmd-N showed no workspace in its turn") + try await PasswordManagerRouteTests.until({ + services.windows.active?.workspaceContent?.workspace.id == key + && services.windows.active?.workspaceContent?.focusedPane != nil + }, within: .seconds(2), report: { "the window does not show Cmd-N's workspace" }) + let content = try #require(services.windows.active?.workspaceContent) + let window = try #require(content.layoutView.window) + work += services.registry.capturingWork { _ = services.registry.perform(action, invocation: invocation) } + var paths: [String] = [] + for character in "echo held\r" { + let text = String(character) + let event = try #require(NSEvent.keyEvent(with: .keyDown, location: .zero, modifierFlags: [], + timestamp: ProcessInfo.processInfo.systemUptime, windowNumber: window.windowNumber, + context: nil, characters: text, charactersIgnoringModifiers: text, + isARepeat: false, keyCode: text == "\r" ? 36 : 0)) + if router.interceptKeyDown(event, in: window) { + paths.append("\(text.debugDescription): consumed (creation hold \(coordinator.isHolding(in: window)), " + + "New Tab hold \(router.newTabInputCoordinator.isHolding(in: window)))") + } else { + paths.append("\(text.debugDescription): to \(window.firstResponder.map { String(describing: type(of: $0)) } ?? "none")") + window.sendEvent(event) + } + } + #expect(kill(pid, SIGCONT) == 0) + resumed = true + + var failures: [String] = [] + for task in work { if let failure = await task.value { failures.append(failure.message) } } + #expect(failures.isEmpty, "\(action): the person's Cmd-N or \(action) failed: \(failures)") + try await PasswordManagerRouteTests.until({ !coordinator.isHolding(in: window) }, within: .seconds(10), + report: { "\(action): the creation hold never ended" }) + let selected = content.focusedPane?.selectedTab + let terminal = try #require(selected.flatMap { $0.kind == .pty && !ProvisionalTab.isProvisional(surface: $0.surface) ? $0.surface : nil }, + "\(action): the focused tab is not the new terminal (\(String(describing: selected?.kind))); key paths: \(paths)") + + // What the person sees: the new terminal shows the typed line and the shell ran it (every key, in order). + func ran(_ text: String) -> Bool { + text.contains("echo held") && text.components(separatedBy: "\n").contains { $0.trimmingCharacters(in: .whitespaces) == "held" } + } + var screen = "" + let deadline = ContinuousClock.now + .seconds(15) + while !ran(screen), ContinuousClock.now < deadline { + screen = (try? await connection.request(ReadScreenRequest(surface: terminal), timeout: .seconds(2)).text) ?? screen + if !ran(screen) { try await Task.sleep(for: .milliseconds(100)) } + } + #expect(ran(screen), "\(action): the new terminal did not get the typed line in order; its screen: \(screen.debugDescription); key paths: \(paths)") + // No other terminal got the line. + for other in store.panesByHandle.values.flatMap(\.tabs) where other.kind == .pty && other.surface != terminal + && !ProvisionalTab.isProvisional(surface: other.surface) { + let text = (try? await connection.request(ReadScreenRequest(surface: other.surface), timeout: .seconds(2)).text) ?? "" + #expect(!text.contains("echo held"), "\(action): terminal \(other.surface) also got the typed line: \(text.debugDescription)") + } + } + } +} diff --git a/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/PasswordManagerRouteTests.swift b/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/PasswordManagerRouteTests.swift index e716c0e44297..06fcaf20a806 100644 --- a/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/PasswordManagerRouteTests.swift +++ b/Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/PasswordManagerRouteTests.swift @@ -40,8 +40,10 @@ struct PasswordManagerRouteTests { } /// The app's services on a fresh same-tree daemon, ready once restore made its first terminal. - /// `home` is the daemon's HOME (where it reads its Ghostty config). + /// `home` is the daemon's HOME (where it reads its Ghostty config). `heldKeyReplay`: where a creation hold's keys + /// run again (the app's `NSApp.sendEvent` by default; a test whose window is never key passes its window's). static func withApp(home: String = NSHomeDirectory(), agentPaneEnvironment: [String: String]? = nil, + heldKeyReplay: @escaping HeldKeyReplay = { event, _, _ in NSApp?.sendEvent(event) }, _ body: (AppServices, DaemonConnection) async throws -> Void) async throws { let binary = try #require(SameTreeDaemonBinary.url) let root = FileManager.default.temporaryDirectory.appendingPathComponent("cn-pwroute-\(UUID().uuidString.prefix(8))") @@ -53,7 +55,7 @@ struct PasswordManagerRouteTests { environment: { ["HOME": home, "PATH": "/usr/bin:/bin:/usr/sbin:/sbin", "SHELL": "/bin/sh", "TERM": "xterm-256color"] }) let owner = AppTestDaemonOwner(launcher) _ = NSApplication.shared - let services = AppServices(environment: AppEnvironment.current([:]), agentPaneEnvironment: agentPaneEnvironment) + let services = AppServices(environment: AppEnvironment.current([:]), agentPaneEnvironment: agentPaneEnvironment, heldKeyReplay: heldKeyReplay) AppActions.bind(services) services.windows.ordersWindowsIn = false services.daemon.start {