Repository navigation
fix(app): keys typed right after Cmd-W go to the pane that gets the focus, never into the closing terminal (P0 close-then-type) - #29409
Conversation
…each the first terminal, and back (P0 1193) Tip watcher smoke, feat 5097a27 on cmux18, step 23: Cmd-D, a line at once, Cmd-D, a line at once: both lines ran in the FIRST new terminal and the second got nothing. SecondCreationKeysTests drives two creation holds on two real terminals: keys typed after the second creation began must wait for the second terminal; keys held for the first creation must reach the first terminal, also when the second one has the keyboard first. Red on this base.
…0 1193) After Cmd-D, a line, Cmd-D, a line typed at once, both lines ran in the first new terminal (tip watcher smoke on feat 5097a27, 2 of 3 rounds on cmux18). CreationInputBuffer.takesKeysEarly(tab:) was true for the early tab of ANY pending creation: until the second creation's focus landed, focus still named the first one's early view and the first creation was still pending (its reply is slower under load, and the second create waits for it), so CreationInputCoordinator.capture let the second line's keys, and the keys held before them, go to the first terminal. Each held key now carries a lane: the creation whose tab the focus named, or was on its way to, when it was typed (FocusState.typingTab). An early view takes only its own lane's keys, and only when its terminal view is the window's first responder; a key typed for another creation waits for that one. When the hold ends, a lane whose terminal is not the focused one gets its plain text by that terminal's input (TerminalSession.sendBytes, by the terminal id its create chose); the focused lane, keys of no lane and chords run again as typed, as before. The perf line's rule (050651b, "only the newest pending creation's early tab") is not ported as is: it sends keys held for the first creation to the second terminal when the second gets the keyboard first. CreationHoldEnd and CreationHoldLog move to CmuxNextFocus (as on the perf line), so CmuxNextApp gains no lines. Test: SecondCreationKeysTests (red on 0383621).
… as 03 and cursor-up (P0 1193) Review of ad8c348 (coordinator): held keys that are not plain text (Ctrl chords, arrows, Return, Option-as-Alt) for a terminal that is not focused still ran in the focused pane; a held Ctrl-C there stops another process. Byte readers in both terminals show which one gets the held Ctrl-C and Up.
… that terminal's own surface (P0 1193) Review of ad8c348 (coordinator): only plain-text held keys went to their own terminal; Ctrl chords, arrows, Return and Option-as-Alt keys still ran in the focused pane. Now every held key of a lane whose terminal is not focused goes to that terminal through TerminalSurfaceView.typeHeldKey: its Ghostty key encoder (modifiers, Option-as-Alt setting, cursor and keypad modes, Kitty protocol), written to that terminal as a typed key, with no input method. It never falls back to the focused pane; keys of a terminal whose view is gone are dropped with it (logged), as a closed creation's are. Command chords are app actions: they run again as typed, in the focus at the hold's end, as before. Test: SecondCreationKeysTests.heldControlAndArrowKeysReachOnlyTheirOwnTerminal.
…at its place in its own terminal (P0 1193) cmux-ad 11:15Z review of #29301: a lane holding a non-text key (an arrow, Ctrl-C) or a Command chord went back to the focused terminal as a whole. The new test needs held keys that run again to be visible in a test window, which is never key: CreationInputCoordinator.runKey is the seam (NSApplication.sendEvent by default, unchanged).
…y Command chords go through app dispatch (P0 1193) cmux-ad 11:15Z review: take(focused:writer:) sent a lane with any key that is not plain text back to "the rest", which ran in the focused terminal, and ran Command chords after every typed key. Now the hold's end is one ordered list (CreationInputBuffer.HeldKey): a key of a lane whose terminal is not focused is typed by that terminal's own Ghostty encoder (TerminalSurfaceView.typeHeldKey: arrows, Ctrl chords, Return, Option-as-Alt, keypad and cursor modes); a Command chord, the focused lane's keys and keys of no lane run again as typed (runKey), each at its typing position. A Command chord that starts another creation still takes the keys after it into that creation's hold. TerminalSession .hasKeyboard is the one "this terminal has the keyboard" check. Test: SecondCreationKeysTests.eachHeldKeyRunsAtItsPlaceInItsOwnTerminal (red at the seam commit).
…d keys survive a workspace switch; Cmd-W closes only its pending split (P0 1193) cmux-ad 11:2xZ review of #29301: a held Command chord ran in the focus at the hold's end (a held Cmd-D split the focused pane, not the first terminal's), and keys of a terminal whose view was gone were dropped. Cmd-W is never held (a close key acts at once, CreationInputBuffer.holds); its test guards that rule in the reported sequence.
…a hidden or far terminal still gets its held keys (P0 1193) cmux-ad 11:2xZ review of #29301: - A held Command chord of a lane whose terminal is not focused ran in the focus at the hold's end. Now an app shortcut runs with that terminal's tab as its target (RegistryKeyBindings.run target:) and that terminal's key context, as a key press there; handlers resolve a tab target to its pane (AppActionContext.paneController, daemonPane). A Command chord that is no app shortcut is that terminal's key (a TUI's Cmd keys). - Keys of a terminal whose view is gone were dropped. A hidden terminal's view (another workspace, an unselected tab) still exists in the content cache and types them; with no view (evicted, another machine's), the daemon types them with that terminal's own modes (send-key / send, HeldKeyInput), on the machine its creation was on. Only a closed terminal takes its keys with it. DecidedKeyEvents moves to CmuxNextFocusUI (pure AppKit, the key router's), so CmuxNextApp shrinks. Tests: SecondCreationKeysTests.eachHeldKeyRunsAtItsPlaceInItsOwnTerminal (split position), heldKeysReachTheirTerminalAfterItsWorkspaceWasSwitchedAway, cmdWTypedForAPendingSplitClosesThatSplitOnly.
… creation's daemon reply until released (P0 1193)
cmux-ad 11:25Z: the 1193 journey controls the race instead of chasing it with load or timestamps. debug.creation.hold
{n} arms the n-th creation (split, New Column, New Row, new terminal tab) that starts after it: the daemon makes the
pane, and the app applies the reply only at debug.creation.release {n} (no n: all, disarmed); a held reply nobody
releases goes on after 30 s. DEBUG and bench builds only (a release router drops debug.* methods); no product timing.
Registered on the existing debug.daemon line, so CmuxNextApp does not grow.
…-as-alt, as its view's encoder does (P0 1193) cmux-ad 11:3xZ: on the daemon path an Option key sent what macOS composed even when Ghostty's macos-option-as-alt (true, left, right) makes Option Alt, so another byte reached the terminal than a key press in its view sends. HeldKeyInput reads the same setting (GhosttyRuntime.optionAsAltValue) and sends alt+<base key> (ESC and the key). Test: HeldKeyInputTests (the mapping per setting and side, the config read, the order of text, arrows, Ctrl-C and Return). It needs the new API, so it is committed with the fix; it has no red commit of its own.
…ne in the control socket's JSON type (P0 1193) pa-d's Debug builds 8a86e9ae and c1cfa1dc: "the compiler is unable to type-check this expression in reasonable time" at the reply's one-expression literal, which also named a JSONValue that CmuxNextDaemon defines too. Build it as debug.daemon does: the call's own params type, one key per line.
…ions whose waiting reply it let go (P0 1193) pa-d's journey review: "released" lists every number a release named, so it cannot prove that creation 1 waited. "resumed" lists only the creations this call found waiting and resumed.
…n's stand-in out of the store until its release (P0 1193)
pa-d's rounds (cmux18, no hog): with only creation 1's reply held, the red ref PASSED: creation 2's stand-in landed
the focus within a frame, before d2's first key. The 1193 state needs creation 2's focus asked for (in its key's turn)
and not landed while d2 is typed, as when the main thread ran the keys before it laid the new pane out (the hog).
stand_in makes that state on purpose: the intent waits, the reply waits too, and the release shows the stand-in first
("shown" in its reply). DEBUG and bench builds only.
… every wait says what it waited for and what it saw (P0 1193) Red step 5e829f0c5c0ff1f0d9702c6a (red ref 25b488f) was no valid red: 5 tests stopped in setup at line 50 ("timed out: panes [3: conversation 2, 6: pty 5]"): the test app's first pane is an agent chat, not a terminal, so the wait for two terminal panes never held. The Cmd-W guard read the new pane's currentTabKey in the split's own turn (nil: the pane shows its stand-in in a later frame). Now two splits make both terminals, each focused with the keyboard before the test body; the Cmd-W test starts from a terminal and reads the stand-in's tab from its model.
…, not on the hold mechanism (P0 1193) Red step f78bf3a645e937c0a98ffb98 (red ref 656454a), coordinator's reading: - Four tests failed on "were the keys held": the setup took the keyboard away (makeFirstResponder(nil)) and opened an early tab, but the app's focus gives the terminal the keyboard back, so on the base the keys went straight to it. Now lane 1's creation has no early view and the keyboard stays: the keys are held on any build, and the builds differ only in where the hold delivers them. Every test checks first which terminal got which line, then how. - wait-for's timeout threw (line 141); a timeout is now the answer "the line did not run there". - The Cmd-W guard required a pane from the second Cmd-D; on the base that Cmd-D made none (the focus stayed on the closed stand-in, a defect of its own, reported apart). The guard now checks only what it guards: the closed split is gone, the pane it was typed from stays, and no key reached another terminal.
… gets the focus (P0 close-then-type, red) Journey r3-after-switch round s3 (cmux19, build a0f707a2, STATUS 1333/1353): Cmd-W on the focused right terminal, then a line typed at once; the left shell ran 'cho ...'. A person's Cmd-W on a live terminal first asks the daemon what it runs (TabCloseGuard), and until that read answers the closing terminal keeps the keyboard, so keys typed then go into it. CloseThenTypeKeysTests drives the real key path (key router, then the window's first responder) against a same-tree daemon: - aLineTypedRightAfterCmdWRunsWholeInThePaneThatGetsTheFocus: red on feat (the line runs in the closing terminal). - keysTypedWhileTheCloseWarningIsCancelledGoToTheTerminalThatStays: the cancel case, a guard. CreationInputCoordinator.runKey is a behavior-neutral seam (NSApp.sendEvent by default, the same line as #29301), so a held key that runs again reaches a test window that is never key.
…(no behavior change) The intentional-hold rule of the shortcut hints has no App dependency (AppKit only): it moves from the App to the focus UI package, public, with the same code. Lowers the App's lines (arch rule: the App is a thin shell); it pays for the close-then-type fix's App lines.
…behavior change) The pane-navigation hint plan is pure (no AppKit, no registry; ActionID only): it moves from the App to the focus UI package, public with an explicit init, the same code. Lowers the App's lines (arch rule: the App is a thin shell); it pays for the close-then-type fix's App lines.
…ocus, never into the closing terminal (P0 close-then-type) A person's Cmd-W on a live terminal waits for the daemon's read first (TabCloseGuard, the close warning; CloseUndoToasts.decideFirst when it asks the terminals itself), and the closing terminal kept the keyboard meanwhile: keys typed then went into it and closed with it (journey r3-after-switch s3: the left shell ran 'cho ...'). Now the read opens a close lane in the window's creation hold (CreationInputBuffer.openClose): keys typed while the focus names the closing tab wait, in order, as that close's keys. - The close ran: they go to the terminal focused after it (bound when the close ends, so no guess of the focus reducer's pick), through #29301's lanes. - The warning asks: they leave the hold (the question takes its own keys) and wait for the answer. Close: as above. Cancel: they go to the terminal that stays, by that terminal (its view's encoder, else the daemon), wherever the focus is then; no timeout can drop them. Never dropped, never into the closing terminal; nothing waits on the main thread. Test added: keysOfACancelledCloseGoToTheTerminalThatStaysWhenTheFocusMovedAway (the person clicks into the left pane while the warning shows, then cancels).
…se warning's daemon read (P0 close-then-type) DEBUG and bench builds only (a release router drops every debug.* method; closeReadStarts is a no-op there): a test arms the hold, presses Cmd-W, types, and releases, so the keys are typed inside the close warning's read in EVERY round, on any host (coordinator 11:5xZ; the race needs a key inside a read of a few ms). An armed hold nobody releases ends after 30 s. debug.close.release answers 'resumed', the reads that really waited for it.
…ch wait says what it waits for
Red step d10aec612b19 failed in the set-up on feat: the window's first pane shows an agent chat
('conversation'), not a terminal, so the wait for two terminal panes never held. Both
terminals are now new panes (Cmd-D, then Cmd-D from the first new terminal), and every wait's
failure names what it waited for and the state it saw.
…where the line went Red step a07f5c2a: the close test failed on a wait-for timeout (a thrown error), not on the bug's visible outcome. Now the test holds the close warning's read with the debug seam (DebugCloseReadHold.arm, the same seam the journey uses), types the line inside it, reads where the keys went, then releases (resumed must be 1). It fails with the outcome: 'the line ran in the closing pane', 'first key lost', or 'the line did not run whole in the left pane (<where it went>)'. A wait no longer throws (false on a timeout). The cancel test with the focus moved away (keysOfACancelledCloseGoToTheTerminalThatStaysWhenTheFocusMovedAway) is on the red ref too.
…t the substring "echo cw..." contains "cho cw" too, so the check fired on a correct run (green cb81d130 at 8bbdf97: the left pane showed "echo cw547478x$((6*7))" and "cw547478x42", the line ran whole). It now looks for the prompt line "$ cho cw..." or zsh/sh "command not found: cho".
…celled creation's stand-in (STATUS 1644) Cmd-D, then Cmd-W at once cancels the split's creation (TabLifecycle), and the close list can still name its stand-in (the Cmd-D-after-Cmd-W P0 owner removes it there). A stand-in has a provisional surface and no daemon read to wait for, so CreationInputCoordinator.beginClose now takes the closing TabModels and arms only for a live pty (not dead, not provisional): the next keys never wait for a pane that is already gone. Test: aCancelledStandInArmsNoCloseHoldAndTheNextLineRunsInTheLiveTerminal (the guard returns no ticket for the stand-in; after Cmd-D + Cmd-W no hold is left, and a line typed then runs whole in the focused live terminal; no hold ran out its 2 s bound).
|
cmux status (one comment, edited in place; each line is updated by the job that owns it) Not verified: macOS jobs did not run: compile and app tests were skipped on CI passes on Written by red: cmux-next checks (god files, concurrency, crash safety, l10n); base red: the same failure on the base aaab946; does not block Default changesThis pull request changes 3 defaults at
Signed off on this commit by Merge receipt: merged as Regenerate the snapshot with Shipped in:
|
…before it releases
Steps 08e73ca1 (red) and 258f0a0c (green) both failed this test on its own set-up check
("resumed 0"): the close warning's read starts in a later main-actor turn, and the test released
the hold in the same turn it typed, before any read waited. DebugCloseReadHold.held now says
how many reads wait, and the test waits for 1 before it releases. Test and DEBUG seam only.
|
Review (independent review subagent): SHIP at 32fb96b Scope: the 3 commits The earlier findings
Walks at the head (no blocking defect found)
Non-blocking notes (fix in a follow-up if you agree)
Evidence read
|
…e-then-type-v3 # Conflicts: # Packages/macOS/CmuxNext/Sources/CmuxNextApp/Control/AppControl.swift # Packages/macOS/CmuxNext/Sources/CmuxNextApp/Focus/CreationInputCoordinator.swift # Packages/macOS/CmuxNext/Sources/CmuxNextDaemonService/DaemonService.swift # Packages/macOS/CmuxNext/Sources/CmuxNextFocusUI/CreationInputBuffer.swift # Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/SecondCreationKeysTests.swift
…cus; #29301 moved them to CmuxNextFocusUI The merge of feat (with #29301 at 1d942e4) kept both copies: this branch's base (#29301 at 201b1e4) had them in CmuxNextFocus, #29301's final head moved them to CmuxNextFocusUI. Compile f24aceec at 748ab11: 'CreationHoldEnd' is ambiguous. The FocusUI copies are the live ones; no other code change.
|
Found 9 test failures on Blacksmith runners: Failures
|
|
Review (independent review subagent): SHIP at c1e6dc4 Scope: the merge 748ab11 (feat-cmux-next, with #29301 landed as 373f346), the fix-up c1e6dc4, and the PR diff over feat The merge resolution (5 conflicting files)
The close lane on #29301's final API
Walk of question 3
Non-blocking (the same as my last receipt, not changed by the merge)
Evidence read
|
There was a problem hiding this comment.
4 issues found across 18 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Packages/macOS/CmuxNext/Sources/CmuxNextApp/Control/AppControl.swift">
<violation number="1" location="Packages/macOS/CmuxNext/Sources/CmuxNextApp/Control/AppControl.swift:127">
P3: This registration line is now about 300 characters and chains four method lists with `+`, including an inline closure. Split it across lines so each list is readable and the expression is easier to type-check and review.</violation>
</file>
<file name="Packages/macOS/CmuxNext/Sources/CmuxNextApp/Handlers/CloseUndoToasts.swift">
<violation number="1" location="Packages/macOS/CmuxNext/Sources/CmuxNextApp/Handlers/CloseUndoToasts.swift:206">
P2: The `window` parameter defaults to nil, and the tab-group caller in TabGroupHandlers.swift:207 does not pass it. Without a window, `beginClose` returns nil, so no close hold opens for that close. Remove the default so the compiler requires a window, then pass `window` at the tab-group call site.</violation>
</file>
<file name="Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/CloseThenTypeKeysTests.swift">
<violation number="1" location="Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/CloseThenTypeKeysTests.swift:56">
P2: The fixture can start the second split while the first split still owns the window’s creation hold, making the setup depend on the daemon/focus timing and potentially combining both creations in one input buffer. Wait for the first creation hold to end before issuing the second split, as the other two-terminal fixture does.</violation>
</file>
<file name="Packages/macOS/CmuxNext/Sources/CmuxNextApp/Handlers/TabCloseGuard.swift">
<violation number="1" location="Packages/macOS/CmuxNext/Sources/CmuxNextApp/Handlers/TabCloseGuard.swift:45">
P1: `endClose` settles the key lane before every close has actually completed. A multi-tab close containing the focused terminal and a page-deciding tab returns from `pageDecidedCloses.begin` while the terminal remains focused, so held keys can replay into the terminal that will close. Keep the close ticket pending until the deferred page/daemon close completes, then settle it from the final focus.</violation>
</file>
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
| asking.subtract(ids) | ||
| return close(asked) | ||
| close(asked) | ||
| return coordinator.endClose(keys, in: window) |
There was a problem hiding this comment.
P1: endClose settles the key lane before every close has actually completed. A multi-tab close containing the focused terminal and a page-deciding tab returns from pageDecidedCloses.begin while the terminal remains focused, so held keys can replay into the terminal that will close. Keep the close ticket pending until the deferred page/daemon close completes, then settle it from the final focus.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At Packages/macOS/CmuxNext/Sources/CmuxNextApp/Handlers/TabCloseGuard.swift, line 45:
<comment>`endClose` settles the key lane before every close has actually completed. A multi-tab close containing the focused terminal and a page-deciding tab returns from `pageDecidedCloses.begin` while the terminal remains focused, so held keys can replay into the terminal that will close. Keep the close ticket pending until the deferred page/daemon close completes, then settle it from the final focus.</comment>
<file context>
@@ -34,16 +34,21 @@ enum TabCloseGuard {
asking.subtract(ids)
- return close(asked)
+ close(asked)
+ return coordinator.endClose(keys, in: window)
}
+ let parked = coordinator.parkClose(keys, in: window)
</file context>
| /// `asked` holds terminals the close's question already asked; when it | ||
| /// covers every terminal the rule asks about, the close runs at once. | ||
| func decideFirst(_ tabs: [TabModel], on daemon: DaemonService, asked: RunningPrograms = [:], | ||
| func decideFirst(_ tabs: [TabModel], on daemon: DaemonService, asked: RunningPrograms = [:], window: NSWindow? = nil, |
There was a problem hiding this comment.
P2: The window parameter defaults to nil, and the tab-group caller in TabGroupHandlers.swift:207 does not pass it. Without a window, beginClose returns nil, so no close hold opens for that close. Remove the default so the compiler requires a window, then pass window at the tab-group call site.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At Packages/macOS/CmuxNext/Sources/CmuxNextApp/Handlers/CloseUndoToasts.swift, line 206:
<comment>The `window` parameter defaults to nil, and the tab-group caller in TabGroupHandlers.swift:207 does not pass it. Without a window, `beginClose` returns nil, so no close hold opens for that close. Remove the default so the compiler requires a window, then pass `window` at the tab-group call site.</comment>
<file context>
@@ -195,24 +195,25 @@ final class CloseUndoToasts {
/// `asked` holds terminals the close's question already asked; when it
/// covers every terminal the rule asks about, the close runs at once.
- func decideFirst(_ tabs: [TabModel], on daemon: DaemonService, asked: RunningPrograms = [:],
+ func decideFirst(_ tabs: [TabModel], on daemon: DaemonService, asked: RunningPrograms = [:], window: NSWindow? = nil,
then run: @escaping @MainActor (_ costly: Bool) -> Void) {
switch services.map({ CloseToastRule(services: $0).decide(tabs) }) ?? .skip {
</file context>
| #expect(services.registry.perform("splitRight", invocation: terminal)) | ||
| try await PinnedPageTests.until({ terminals().count == 1 }, report: { "waiting for the first split's terminal pane: \(panes())" }) | ||
| let first = try #require(terminals().first) | ||
| PaneHandlers.focus(first.pane.layoutPaneID, in: content) | ||
| try await PinnedPageTests.until({ content.focus.state.resolved == .terminal(pane: first.pane.paneKey, tab: first.tab) }, | ||
| report: { "waiting for the focus on the first terminal: \(content.focus.state.resolved)" }) |
There was a problem hiding this comment.
P2: The fixture can start the second split while the first split still owns the window’s creation hold, making the setup depend on the daemon/focus timing and potentially combining both creations in one input buffer. Wait for the first creation hold to end before issuing the second split, as the other two-terminal fixture does.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/CloseThenTypeKeysTests.swift, line 56:
<comment>The fixture can start the second split while the first split still owns the window’s creation hold, making the setup depend on the daemon/focus timing and potentially combining both creations in one input buffer. Wait for the first creation hold to end before issuing the second split, as the other two-terminal fixture does.</comment>
<file context>
@@ -0,0 +1,302 @@
+ }
+ var terminal = ActionInvocation(origin: .user)
+ terminal.opensTerminal = true
+ #expect(services.registry.perform("splitRight", invocation: terminal))
+ try await PinnedPageTests.until({ terminals().count == 1 }, report: { "waiting for the first split's terminal pane: \(panes())" })
+ let first = try #require(terminals().first)
</file context>
| #expect(services.registry.perform("splitRight", invocation: terminal)) | |
| try await PinnedPageTests.until({ terminals().count == 1 }, report: { "waiting for the first split's terminal pane: \(panes())" }) | |
| let first = try #require(terminals().first) | |
| PaneHandlers.focus(first.pane.layoutPaneID, in: content) | |
| try await PinnedPageTests.until({ content.focus.state.resolved == .terminal(pane: first.pane.paneKey, tab: first.tab) }, | |
| report: { "waiting for the focus on the first terminal: \(content.focus.state.resolved)" }) | |
| let coordinator = services.keyRouter.creationInputCoordinator | |
| try await PinnedPageTests.until({ !coordinator.isHolding(in: content.layoutView.window) }, report: { "waiting for the first split's creation hold to end" }) | |
| #expect(services.registry.perform("splitRight", invocation: terminal)) | |
| try await PinnedPageTests.until({ terminals().count == 2 }, report: { "waiting for the second split's terminal pane: \(panes())" }) | |
| let second = try #require(terminals().first { $0.pane !== first.pane }) | |
| let window = try #require(content.layoutView.window) |
| service.router.register([budget.method(registry: registry)]) | ||
| service.router.register(DebugHibernationControl.methods(services)) | ||
| service.router.register(DebugWindowsControl.methods(services) + [services.daemon.debugControlMethod { [weak services] in services?.windows.controllers.map { (id: $0.state.id, shows: $0.recoveryShows) } ?? [] }] + services.daemon.creationHold.methods) | ||
| service.router.register(DebugWindowsControl.methods(services) + [services.daemon.debugControlMethod { [weak services] in services?.windows.controllers.map { (id: $0.state.id, shows: $0.recoveryShows) } ?? [] }] + services.daemon.creationHold.methods + services.daemon.closeReadHold.methods) |
There was a problem hiding this comment.
P3: This registration line is now about 300 characters and chains four method lists with +, including an inline closure. Split it across lines so each list is readable and the expression is easier to type-check and review.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At Packages/macOS/CmuxNext/Sources/CmuxNextApp/Control/AppControl.swift, line 127:
<comment>This registration line is now about 300 characters and chains four method lists with `+`, including an inline closure. Split it across lines so each list is readable and the expression is easier to type-check and review.</comment>
<file context>
@@ -124,7 +124,7 @@ final class AppControl {
service.router.register([budget.method(registry: registry)])
service.router.register(DebugHibernationControl.methods(services))
- service.router.register(DebugWindowsControl.methods(services) + [services.daemon.debugControlMethod { [weak services] in services?.windows.controllers.map { (id: $0.state.id, shows: $0.recoveryShows) } ?? [] }] + services.daemon.creationHold.methods)
+ service.router.register(DebugWindowsControl.methods(services) + [services.daemon.debugControlMethod { [weak services] in services?.windows.controllers.map { (id: $0.state.id, shows: $0.recoveryShows) } ?? [] }] + services.daemon.creationHold.methods + services.daemon.closeReadHold.methods)
service.router.register([
.mainActor("debug.frames") { [probe = frameProbe] call in .value(probe.handle(call.params)) },
</file context>
| service.router.register(DebugWindowsControl.methods(services) + [services.daemon.debugControlMethod { [weak services] in services?.windows.controllers.map { (id: $0.state.id, shows: $0.recoveryShows) } ?? [] }] + services.daemon.creationHold.methods + services.daemon.closeReadHold.methods) | |
| service.router.register(DebugWindowsControl.methods(services) | |
| + [services.daemon.debugControlMethod { [weak services] in services?.windows.controllers.map { (id: $0.state.id, shows: $0.recoveryShows) } ?? [] }] | |
| + services.daemon.creationHold.methods + services.daemon.closeReadHold.methods) |
One conflict: CmuxNextFocusUI/ShortcutHintModifierPolicy.swift. Feat's #29409 moved it (and DecidedKeyEvents, PaneNavigationHintPlan) out of CmuxNextApp too; the copies differ only in where public init() sits: feat's copy.
A move of pure data, beside the other key and focus types in CmuxNextFocusUI: the facts about the focused surface that KeyRouter reads besides the focus state (no App dependency, Foundation only). The edits are the access words a move across modules needs (public on the struct and its fields, a public import) and a public init with every field defaulted, in declaration order, which is what the App's memberwise init gave every call site (KeyRouter's Facts(...) with any subset of labels). No logic changes. Feat's #29409 moved three of the five focus files this PR moved, so the merge left CmuxNextApp +26 lines over feat (arch ratchet app_lines); this move pays for it (app-size rule).
…nd the held keys keep waiting; a person's close asks on no answer, hidden or not (review of #31875) With the hide, the focus is on the neighbour, so the coordinator's two-quick- Cmd-Ws guard (isClosing of the focused terminal) missed, and the close key flushed the first close's held keys into the neighbour (#29409 again). CreationInputBuffer.isClosing now also answers true while a hidden close takes every key, so the close key acts now and the keys wait for the answer. promptAndAnswers is confirmed for every person's close: a terminal that gives no answer asks (rule 4), also when the close empties its pane.
…rogram is read before the key) (SHORTCUT LATENCY SL-3) (#31875) * test(next): Cmd-W on an idle focused terminal leaves in the key's turn (red: it waits for the daemon's read) SHORTCUT LATENCY SL-3 (SHORTCUT FRAMES F6: Cmd-W first frame 4-6 refreshes after the key). TabCloseGuard asks the daemon what the terminal runs after the key, and the tab leaves only when that read answers. * perf(next): Cmd-W on an idle terminal closes in its key's turn (the program is read before the key) SHORTCUT LATENCY SL-3 (SHORTCUT FRAMES F6: Cmd-W's first changed frame came 4-6 refreshes after the key at 120 Hz). A user's close of a live terminal asked the daemon what it runs (process-info) after the key and closed only when the read answered, on a later turn, so the key's own commit showed nothing. TerminalProcessReadAhead reads the focused terminal's foreground program before the key: when it gets the focus and at each new prompt or command its shell reports (OSC 133 through the daemon: promptSinceMs, commandSinceMs). An answer counts only while the shell state is the one it was read in; a program's start is a new command, so it ends the answer, and a shell that reports neither has no answer. TabCloseGuard then decides a close in the key's turn when every closing terminal has such an answer and nothing asks (Chrome closes a tab at once unless its page registered beforeunload, which it knows before the key). A close with a question, an unknown answer, or a held read (the debug.close.hold test seam, now also DaemonService. closeReadIsHeld) reads at the key as before. The read waits on no thread (an awaited daemon request, 1 s bound). What stays: a program started in the milliseconds before the daemon's report of it reaches the store; that close goes without the question, and its undo toast brings the terminal back. Regression test: CmdWIdleTerminalClosesInItsTurnTests (same-tree daemon). Fleet red/green in the PR. * fix(next): public import for the moved close types; the test gives the store the prompt report an integrated shell sends (the harness's /bin/sh has no integration) * fix(next): the read-ahead follows the focus hook and watches the one focused tab (the window chain it observed is not @observable) WindowManager.active, WindowController and WorkspaceContentController are not @observable, so an ObservationStream over the active window's focused tab read nil at launch and never woke (fleet green step bb27ac78afe1: no answer in 10 s). FocusEffectApplier.publish, which runs at each focus change of the active window, now hands the focused tab to TerminalProcessReadAhead.focus, which reads it and observes that one TabModel's prompt and command state. * fix(next): the read-ahead runs at every focus effect of the active window (publish runs only when the focus context bits change, never between two terminals) * fix(next): the focus hook's read-ahead call as plain guards (the closure did not type-check) * fix(next): the read-ahead answers only at a prompt; a command's start always reads at the key (review of #31875) The review found that a read made between the shell's OSC 133 C and the command's exec still sees the shell, and a non-interactive command's busy stamp does not change until it ends: that 'no program' answer would have let Cmd-W close a running build with no question. Only a terminal at its prompt (promptSinceMs set, no commandSinceMs) is read ahead now, and only an answer for that same prompt counts; a read for an older prompt that answers late is dropped. The doc no longer claims an undo toast for the remaining window (the fast path passes the same answer to the toast decision, which then shows none; Reopen Closed brings the tab back). New test cmdWAfterACommandStartedReadsAtTheKeyAgain: after a fresh prompt answer, a command starts; Cmd-W must read at the key and keep the tab. * test(next): the Cmd-W tests drive real OSC 133 marks through the shell (A: at the prompt, C: a command starts with the shell still in front), not hand-set store fields that a re-lay wipes * docs(next): the read-ahead names every case it closes without the question (second review of #31875) * test(next): Cmd-W asks for fg from a key binding and for a command started just before it; an idle terminal leaves in the key's turn (red on #31875's read-ahead) Lead on #31875: a cached 'no program' answer closes a resumed editor with no question (data loss). (a) sleep, Ctrl-Z, the prompt's OSC 133 A, then fg with no OSC 133 C; (b) the prompt's A, then a command with no C. Both must ask, keep the tab and keep the program running. The idle test: the tab leaves the strip in the key's turn and the close completes after the read. * perf(next): Cmd-W hides the tab in its key's turn and ends the terminal only after the daemon confirms no program runs (lead, #31875) Replaces the read-ahead: a cached 'no program' answer could close a resumed editor (fg from a key binding) with no question, and a killed process is not brought back by Reopen Closed. Now a user's close of live terminals (close warning on) takes the tabs out of their strips in the key's turn with a close intent (HiddenClose: the same DaemonStore.intendClose a close uses, but nothing is sent, so the terminals stay alive). The daemon's process read runs off the main thread as before: - nothing runs: the hide is lifted in the turn the close's own intent takes the tabs out again (no flash), and the close goes to the daemon; - a program runs: the hide is rolled back (rejectIntent), the tabs come back at their places with each pane's selection, and the question shows. Only a confirmed 'no program' (or the person's Close) ends a process. A close that would leave a pane with no tab keeps the old path (the pane animates out after the read). The per-key input path is untouched. * test(next): a cancelled Cmd-W brings back the same terminal view and browser page; keys typed during the read wait for the answer and never press Close (red: review H1, H2 of #31875) * test(next): qualify the dialog helper in the new Cmd-W tests * test(next): the H2 test opens its own terminal neighbour (the pane's first tab may be an agent chat) * fix(next): a hidden Cmd-W keeps the tab's content and the person's keys until the daemon's answer (review H1, H2 of #31875) H1: the hide took the tabs out of the store's visible state, so GoneTabWatch, AgentTabStore and InternalPageTabStore released their terminal views and pages during the read; a Cancel brought back only the records. The store now keeps the tabs a pending HiddenTabClose holds (DaemonStore.holdTabs), and liveTreeLists(tab:), the one read all three watchers use, still lists them: their content stays until the close is sent. No watcher rule is copied. H2: the hide moved the focus to the neighbour, so keys typed during the read got no close lane and ran there, or after a rollback into the restored terminal or the question before the person answered. The close lane opens before the hide and, on the hide path, takes every key typed in the window until the answer (CreationInputBuffer.openClose takesEveryKey); the question then parks them as before (Close sends them to the neighbour, Cancel to the terminal that stays), so a typed Return never presses Close. Also: a terminal that gives no answer (the 1 s read bound, the daemon gone) asks on the hide path (DestructiveConfirmation.confirmedPrograms; only a confirmed 'no program' ends a process); tabs whose page decides its own close are never hidden (no flash); the rollback re-selects with applySelection (no opener reset, no focus taken from another pane). * refactor(next): runningPrograms' no-connection branch reads plainly * test(next): H2 checks that the keys typed during the read wait for the answer (the closing terminal has none while the question shows) * fix(next): the unanswered fallback is a closure literal (Sendable) * test(next): a second Cmd-W during a hidden close's read keeps the held keys for the answer (red: #29409 on the hide path, review of #31875) * fix(next): a second Cmd-W during a hidden close's read acts at once and the held keys keep waiting; a person's close asks on no answer, hidden or not (review of #31875) With the hide, the focus is on the neighbour, so the coordinator's two-quick- Cmd-Ws guard (isClosing of the focused terminal) missed, and the close key flushed the first close's held keys into the neighbour (#29409 again). CreationInputBuffer.isClosing now also answers true while a hidden close takes every key, so the close key acts now and the keys wait for the answer. promptAndAnswers is confirmed for every person's close: a terminal that gives no answer asks (rule 4), also when the close empties its pane. * fix(next): a stand-in terminal is never asked or counted as running; the Cmd-W view test waits for the view (5x race, 2 of 25) --------- Co-authored-by: cmux-bot[bot] <348531757+cmux-bot[bot]@users.noreply.github.com>
…st in #28295's merge) (#32238) #29409 registered the local daemon's creation-hold and close-read-hold methods (debug.creation.hold/release, debug.close.hold/release) beside debug.daemon in AppControl. #28295 (debug.daemon_requests) landed after it with its own version of that line, which dropped both, so a Debug build answers those methods with an unknown-method error (scripts/cmux-next/typeahead-gate.sh and the journey fuzz use them). This restores them next to debug.daemon_requests. Agent-Session: https://cmux.com/deeplink/agent?id=aid-9124da39adbff74677d9 Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
![Fix with [code]smith](https://pr-comments-assets.blacksmith.sh/codesmith/fix-with-codesmith-light-3x.png)
#29301 (P0 1193 lane code, which this fix builds on) merged at 373f346; this branch now holds feat with it, and the diff is only the close-then-type change.
Summary
P0 close-then-type (STATUS 1333, 1353): after Cmd-W closed the focused pane, the first typed key went into the closing terminal. Journey r3-after-switch round s3 (cmux19, build a0f707a2) ran
cho r3fc-…in the remaining shell.Cause: a person's Cmd-W on a live terminal first asks the daemon what the terminal runs. This is the close warning's read:
TabCloseGuard.close, andCloseUndoToasts.decideFirstwhen that function asks the terminals itself. The closing terminal kept the focus and the keyboard until that read answered. Keys typed during the read went into the closing terminal.Fix:
CreationInputBuffer.openClose). Keys typed while the focus names the closing tab wait as that close's keys, in typing order. Nothing waits on the main thread.TabModel.isLiveTerminal). A cancelled creation's stand-in has no read to wait for. This is a guard; the P0 1584 fix (branchfix/p0-close-pending-split-focus) also keeps stand-ins out of the close list.debug.close.hold/debug.close.release(DebugCloseReadHold, one perDaemonService). They hold the close read, so a test or journey types inside it every time.Moves out of the App, no behavior change, in their own commits:
ShortcutHintModifierPolicyandPaneNavigationHintPlanmove toCmuxNextFocusUI.TypeAheadQueuemoves toCmuxNextFocus, also in its own commit with no behavior change.app_lines−88 against fix(app): each held key goes only to the terminal it was typed for (P0 1193, Cmd-D twice) #29301's head.Testing
Red (feat + tests + hook), ref
1609450295aa9a11789eea4c765a875ef6b5a4de, fleet step7af5d31a3e1122be227bacae:aLineTypedRightAfterCmdWRunsWholeInThePaneThatGetsTheFocusFAILS: "the line ran in the closing pane (typed inside the close warning's read)".aSecondCmdWDuringTheCloseReadKeepsTheKeysOutOfTheClosingTerminalFAILS: "the line ran in the closing pane (a second Cmd-W during the close read let the held keys go)".keysTypedBeforeTheCloseWarningGoToThePaneThatGetsTheFocusOnCloseFAILS: "the keys typed before the warning did not run whole in the pane that got the focus on Close".Green at head
c1e6dc478886d0535f4eda6244cf6d34a60c4c74(after the merge of feat with #29301):675243e05d2ad437b3d5dc9c(CmuxNext, app, cmux-tui): ok.d7d6b94fbf6954cfb25a0d29(gui-session-v1, console session, "display step: this session has 1 screen(s)",CMUX_TEST_REQUIRE_GUI=1, no skipped test, no FLAKY notice):CloseThenTypeKeysTests6/6SecondCreationKeysTests11/11HeldKeyInputTests3/3PaneNavigationHintTests6/6TypeAheadQueueTests2/2TypeAheadDeleteTests6/608a46649bc2ca8634acfe635).On-screen journey
close-then-type(branchcp/journey-close-then-type4d43e46): whole-host capture lease, app launched active, 8 tries per round. Each try holds the close read, presses Cmd-W in the focused right pane, types a line, then releases the read. The try fails unless the release resumed 1 held read.handled_by "responder", so they went into the closing terminal.handled_by "app"(the close hold), and every line ran whole in the left pane. That build has the same product code as this head: the last commit changes only a DEBUG seam and a test.Review: the first review was FIX at 3da8089 (a second quick Cmd-W during the read let the held keys go, plus 4 smaller findings). All five are fixed in 62a0685..32fb96b. The re-review was SHIP at 32fb96b (#29409 (comment)); the merge of feat had conflicts, so a new review covers the merged head.
Non-blocking review notes, for a follow-up:
Observed / Not run
Changelog
Fixed: keys typed right after closing a terminal pane with Cmd-W now go to the pane that gets the focus, instead of the closing terminal.
No capture: keyboard routing only, no visual change. The evidence is the on-screen journey rounds above.
perf claim: none