Repository navigation
fix(app): a script's new workspace does not take the person's keys from Home's composer - #30071
Open
lawrencecchen wants to merge 9 commits into
Open
lawrencecchen wants to merge 9 commits into
lawrencecchen wants to merge 9 commits into
Conversation
…ch the Chief message box; a script's new terminal never takes a typing person's keys (red) Ported from cp/fix-chief-composer-focus (4aefbf6, 228e8b0, c3b322c, abe55d9, f9ec8a6; perf merge 943ac39), adapted to feat-cmux-next: feat has no KeyRouter modes, so the hold is read from CreationInputCoordinator.isHolding(in:). Red on feat: a person's terminal-first newTab opens no key hold over Home, a script's newTab with focus shows over a person typing in a field, and a script's nested newTab opens the New Tab page hold.
…uxNextFocusUI A move only, so the fix after it reads alone. DecidedKeyEvents (key dispatch), ShortcutHintModifierPolicy and PaneNavigationHintPlan (the modifier-hold shortcut hints, beside FocusUI's hint overlay and chip fader), BrowserFocusModeEscapeExit (the focus-mode Escape gesture) and NSResponder.ownsTypedText (who has the keyboard) are focus code. The only edits are the access words a move across modules needs: `public` on the types and members the App and its tests use, and public memberwise inits. No logic changes. CmuxNextApp reaches them through its existing `@_exported import CmuxNextFocusUI` (FocusUIExport.swift).
…om Home's composer Dogfood v2.2.3 (P0 journey scripted-newtab-keeps-person-keys): with Home showing, keys typed for a new terminal went into Home's Chief message box and Return sent the shell command to the Chief as a message. A script's newTab with focus also took the window from a person typing in a field. - A person's terminal-first newTab shown in the window they type in opens a key hold (CreationInputCoordinator.beginWorkspace) that waits until the window shows that workspace; keys typed for a workspace that fails or never shows are dropped, never typed into what has the focus. - The window's keyboard stays claimed (KeyboardClaims) until the person's next intent there: a click, Escape, a focus move by mouse, keyboard or palette, or another of their actions. Never a timeout, never automation. While the claim holds, no primary input takes the keyboard by itself (NSWindow.pendingCreationOwnsKeyboard) and keys that run no binding are kept from the window. - A script's, the CLI's, MCP's or the socket's newTab with focus is created and listed in the sidebar without view change. Once it exists, the window shows it only if no text input has the person's keyboard then (WindowController.hasTextInputKeyboard: an editable text view or field, an input method composing, or an editable page element, in the active key window). Otherwise it stays listed and not shown. - The origin is the run's own (a nested run keeps its outer script origin): a script's nested newTab opens no New Tab page and no key hold. Ported from cp/fix-chief-composer-focus (f664686, 23248fd, 2be9a0c, a4c01b0, 8072355; perf merge 943ac39). The new code lives in CmuxNextFocusUI (KeyboardClaims, the CreationInputBuffer workspace tickets, NSWindow.firstResponderTypesText) and CmuxNextFocus (FocusEvent.isPersonsOwn). The move commit before this one moved five focus files out of CmuxNextApp, so the App does not grow over the PR (app-size rule).
lawrencecchen
temporarily deployed
to
ci-routing-agent
November 1, 2026 03:15 — with 
GitHub Actions
Inactive
GitHub Actions
Inactive
Contributor
|
cmux status (one comment, edited in place; each line is updated by the job that owns it) Review App of this head (app.zip, kept 1 day)
red: cmux-next checks (god files, concurrency, crash safety, l10n); base red: the same failure on the base 6eb6839; does not block Default changesThis pull request changes 5 defaults at
Needs a review. It passes with an independent review receipt on this commit: a comment or review whose first line is Regenerate the snapshot with |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
7 issues found across 19 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/AppServices.swift">
<violation number="1" location="Packages/macOS/CmuxNext/Sources/CmuxNextApp/AppServices.swift:742">
P2: This click handler ends the new terminal's keyboard claim for any mouse-down, including clicks the app posts for automation (`debug.mouse`, agent pane clicks). Skip the call when `currentEventIsSynthetic` is true, as the notifications call on the line above already does.</violation>
</file>
<file name="Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/HomePendingTerminalFocusTests.swift">
<violation number="1" location="Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/HomePendingTerminalFocusTests.swift:228">
P2: The result of the script's nested `newTab` is discarded, so `!scriptHolds` passes even when the action never runs. Assert the returned Bool, as the control call does with `#expect(control)`.</violation>
</file>
<file name="Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/PersonTerminalKeyboardClaimTests.swift">
<violation number="1" location="Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/PersonTerminalKeyboardClaimTests.swift:146">
P3: This test does not check its precondition or the action's result. Assert that the claim holds before the action and that `perform` returned true, so the final `!claimed` check actually tests the claim ending.</violation>
<violation number="2" location="Packages/macOS/CmuxNext/Tests/CmuxNextAppTests/PersonTerminalKeyboardClaimTests.swift:158">
P2: This test has no positive control. It discards the result of `perform("newTab")` and asserts only that no hold or claim exists, so it still passes if `newTab` never runs. Assert that the perform call returned true before checking the negatives.</violation>
</file>
<file name="Packages/macOS/CmuxNext/Sources/CmuxNextFocusUI/NSResponder+OwnsTypedText.swift">
<violation number="1" location="Packages/macOS/CmuxNext/Sources/CmuxNextFocusUI/NSResponder+OwnsTypedText.swift:27">
P2: The IME branch of `firstResponderTypesText` skips the visibility check that `ownsTypedText` applies. A hidden field that still holds the keyboard with uncommitted marked text counts as typing, so a script's workspace is not shown. Apply the same window and hidden checks before the `hasMarkedText()` test.</violation>
<violation number="2" location="Packages/macOS/CmuxNext/Sources/CmuxNextFocusUI/NSResponder+OwnsTypedText.swift:27">
P2: This only recognizes custom input surfaces while an IME preedit is active, so ordinary typing in `RemoteInputCaptureView` is treated as having no text-input keyboard and an automated `newTab` can steal focus. Include active custom keyboard owners in the ownership check, while retaining the intentional terminal exception.</violation>
</file>
<file name="Packages/macOS/CmuxNext/Sources/CmuxNextFocusUI/CreationInputBuffer.swift">
<violation number="1" location="Packages/macOS/CmuxNext/Sources/CmuxNextFocusUI/CreationInputBuffer.swift:100">
P1: A key arriving after a failed workspace reply but before the scheduled flush is captured as a normal post-creation key and replayed into the current responder, so it can still reach Home's composer. Keep the failed workspace ticket marked unshown until the buffer is flushed, so `take(shown:)` drops keys captured in this gap.</violation>
</file>
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
| if awaitsFocus, let workspace { | ||
| madeWorkspaces[ticket.id] = workspace | ||
| } else { | ||
| workspaceTickets.remove(ticket.id) |
There was a problem hiding this comment.
P1: A key arriving after a failed workspace reply but before the scheduled flush is captured as a normal post-creation key and replayed into the current responder, so it can still reach Home's composer. Keep the failed workspace ticket marked unshown until the buffer is flushed, so take(shown:) drops keys captured in this gap.
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/CmuxNextFocusUI/CreationInputBuffer.swift, line 100:
<comment>A key arriving after a failed workspace reply but before the scheduled flush is captured as a normal post-creation key and replayed into the current responder, so it can still reach Home's composer. Keep the failed workspace ticket marked unshown until the buffer is flushed, so `take(shown:)` drops keys captured in this gap.</comment>
<file context>
@@ -68,19 +84,29 @@ public final class CreationInputBuffer {
+ if awaitsFocus, let workspace {
+ madeWorkspaces[ticket.id] = workspace
+ } else {
+ workspaceTickets.remove(ticket.id)
+ events.removeAll { $0.creation == ticket.id }
+ }
</file context>
| self?.linkHints.cancel() | ||
| self?.windowHints.cancel() | ||
| self?.keyRouter.cancelChord() | ||
| self?.keyRouter.creationInputCoordinator.personActed(in: event.window, event: event) // a click: the person's next intent |
There was a problem hiding this comment.
P2: This click handler ends the new terminal's keyboard claim for any mouse-down, including clicks the app posts for automation (debug.mouse, agent pane clicks). Skip the call when currentEventIsSynthetic is true, as the notifications call on the line above already 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/Sources/CmuxNextApp/AppServices.swift, line 742:
<comment>This click handler ends the new terminal's keyboard claim for any mouse-down, including clicks the app posts for automation (`debug.mouse`, agent pane clicks). Skip the call when `currentEventIsSynthetic` is true, as the notifications call on the line above already does.</comment>
<file context>
@@ -739,6 +739,7 @@ final class AppServices {
self?.linkHints.cancel()
self?.windowHints.cancel()
self?.keyRouter.cancelChord()
+ self?.keyRouter.creationInputCoordinator.personActed(in: event.window, event: event) // a click: the person's next intent
}
(NSApp as? CmuxApplication)?.keyDownObserver = { [weak self] _ in self?.notifications.endViewHolds() }
</file context>
Suggested change
| self?.keyRouter.creationInputCoordinator.personActed(in: event.window, event: event) // a click: the person's next intent | |
| if (NSApp as? CmuxApplication)?.currentEventIsSynthetic != true { self?.keyRouter.creationInputCoordinator.personActed(in: event.window, event: event) } // a click: the person's next intent (not the app's own posted input) |
Comment on lines
+228
to
+231
| _ = ActionRunScope.carrying(script) { | ||
| services.registry.perform("newTab", invocation: ActionInvocation(arguments: ["name": .string("scripts-page")], origin: .user)) | ||
| } | ||
| let scriptHolds = input.isHolding(in: shell) |
There was a problem hiding this comment.
P2: The result of the script's nested newTab is discarded, so !scriptHolds passes even when the action never runs. Assert the returned Bool, as the control call does with #expect(control).
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/HomePendingTerminalFocusTests.swift, line 228:
<comment>The result of the script's nested `newTab` is discarded, so `!scriptHolds` passes even when the action never runs. Assert the returned Bool, as the control call does with `#expect(control)`.</comment>
<file context>
@@ -0,0 +1,235 @@
+ try await PasswordManagerRouteTests.until({ !input.isHolding(in: shell) }, within: .seconds(30), report: { "the person's page hold never ended: \(input.log)" })
+
+ let script = ActionRunScope(origin: .script, allowsViewChange: true)
+ _ = ActionRunScope.carrying(script) {
+ services.registry.perform("newTab", invocation: ActionInvocation(arguments: ["name": .string("scripts-page")], origin: .user))
+ }
</file context>
Suggested change
| _ = ActionRunScope.carrying(script) { | |
| services.registry.perform("newTab", invocation: ActionInvocation(arguments: ["name": .string("scripts-page")], origin: .user)) | |
| } | |
| let scriptHolds = input.isHolding(in: shell) | |
| #expect(ActionRunScope.carrying(script) { | |
| services.registry.perform("newTab", invocation: ActionInvocation(arguments: ["name": .string("scripts-page")], origin: .user)) | |
| }) |
| try await Self.withWindow { h in | ||
| let request = ActionInvocation(arguments: ["command": .string("/bin/sh"), "name": .string("nested")], origin: .user) | ||
| let script = ActionRunScope(origin: .script, allowsViewChange: true) | ||
| _ = ActionRunScope.carrying(script) { h.services.registry.perform("newTab", invocation: request) } |
There was a problem hiding this comment.
P2: This test has no positive control. It discards the result of perform("newTab") and asserts only that no hold or claim exists, so it still passes if newTab never runs. Assert that the perform call returned true before checking the negatives.
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/PersonTerminalKeyboardClaimTests.swift, line 158:
<comment>This test has no positive control. It discards the result of `perform("newTab")` and asserts only that no hold or claim exists, so it still passes if `newTab` never runs. Assert that the perform call returned true before checking the negatives.</comment>
<file context>
@@ -0,0 +1,208 @@
+ try await Self.withWindow { h in
+ let request = ActionInvocation(arguments: ["command": .string("/bin/sh"), "name": .string("nested")], origin: .user)
+ let script = ActionRunScope(origin: .script, allowsViewChange: true)
+ _ = ActionRunScope.carrying(script) { h.services.registry.perform("newTab", invocation: request) }
+ let holding = h.services.keyRouter.creationInputCoordinator.isHolding(in: h.window)
+ let claimed = h.window.pendingCreationOwnsKeyboard
</file context>
Suggested change
| _ = ActionRunScope.carrying(script) { h.services.registry.perform("newTab", invocation: request) } | |
| let ran = ActionRunScope.carrying(script) { h.services.registry.perform("newTab", invocation: request) } | |
| #expect(ran, "the nested newTab did not run, so the negatives below prove nothing") |
| @MainActor public func firstResponderTypesText(excluding excluded: (NSResponder) -> Bool) -> Bool { | ||
| guard let responder = firstResponder else { return false } | ||
| if responder.ownsTypedText { return true } | ||
| return !excluded(responder) && (responder as? any NSTextInputClient)?.hasMarkedText() == true |
There was a problem hiding this comment.
P2: The IME branch of firstResponderTypesText skips the visibility check that ownsTypedText applies. A hidden field that still holds the keyboard with uncommitted marked text counts as typing, so a script's workspace is not shown. Apply the same window and hidden checks before the hasMarkedText() test.
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/CmuxNextFocusUI/NSResponder+OwnsTypedText.swift, line 27:
<comment>The IME branch of `firstResponderTypesText` skips the visibility check that `ownsTypedText` applies. A hidden field that still holds the keyboard with uncommitted marked text counts as typing, so a script's workspace is not shown. Apply the same window and hidden checks before the `hasMarkedText()` test.</comment>
<file context>
@@ -0,0 +1,29 @@
+ @MainActor public func firstResponderTypesText(excluding excluded: (NSResponder) -> Bool) -> Bool {
+ guard let responder = firstResponder else { return false }
+ if responder.ownsTypedText { return true }
+ return !excluded(responder) && (responder as? any NSTextInputClient)?.hasMarkedText() == true
+ }
+}
</file context>
Suggested change
| return !excluded(responder) && (responder as? any NSTextInputClient)?.hasMarkedText() == true | |
| guard !excluded(responder), let view = responder as? NSView, view.window != nil, !view.isHiddenOrHasHiddenAncestor else { return false } | |
| return (responder as? any NSTextInputClient)?.hasMarkedText() == true |
| @MainActor public func firstResponderTypesText(excluding excluded: (NSResponder) -> Bool) -> Bool { | ||
| guard let responder = firstResponder else { return false } | ||
| if responder.ownsTypedText { return true } | ||
| return !excluded(responder) && (responder as? any NSTextInputClient)?.hasMarkedText() == true |
There was a problem hiding this comment.
P2: This only recognizes custom input surfaces while an IME preedit is active, so ordinary typing in RemoteInputCaptureView is treated as having no text-input keyboard and an automated newTab can steal focus. Include active custom keyboard owners in the ownership check, while retaining the intentional terminal exception.
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/CmuxNextFocusUI/NSResponder+OwnsTypedText.swift, line 27:
<comment>This only recognizes custom input surfaces while an IME preedit is active, so ordinary typing in `RemoteInputCaptureView` is treated as having no text-input keyboard and an automated `newTab` can steal focus. Include active custom keyboard owners in the ownership check, while retaining the intentional terminal exception.</comment>
<file context>
@@ -0,0 +1,29 @@
+ @MainActor public func firstResponderTypesText(excluding excluded: (NSResponder) -> Bool) -> Bool {
+ guard let responder = firstResponder else { return false }
+ if responder.ownsTypedText { return true }
+ return !excluded(responder) && (responder as? any NSTextInputClient)?.hasMarkedText() == true
+ }
+}
</file context>
| let ticket = try #require(coordinator.beginWorkspace(in: h.controller)) | ||
| coordinator.resolve(ticket, workspace: nil, in: h.window) | ||
| try await PasswordManagerRouteTests.until({ !coordinator.isHolding(in: h.window) }, report: { "the hold never ended" }) | ||
| _ = h.services.registry.perform("toggleSidebar", invocation: ActionInvocation(origin: .user)) |
There was a problem hiding this comment.
P3: This test does not check its precondition or the action's result. Assert that the claim holds before the action and that perform returned true, so the final !claimed check actually tests the claim ending.
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/PersonTerminalKeyboardClaimTests.swift, line 146:
<comment>This test does not check its precondition or the action's result. Assert that the claim holds before the action and that `perform` returned true, so the final `!claimed` check actually tests the claim ending.</comment>
<file context>
@@ -0,0 +1,208 @@
+ let ticket = try #require(coordinator.beginWorkspace(in: h.controller))
+ coordinator.resolve(ticket, workspace: nil, in: h.window)
+ try await PasswordManagerRouteTests.until({ !coordinator.isHolding(in: h.window) }, report: { "the hold never ended" })
+ _ = h.services.registry.perform("toggleSidebar", invocation: ActionInvocation(origin: .user))
+ let claimed = h.window.pendingCreationOwnsKeyboard
+ #expect(!claimed, "the claim outlived the person's next action")
</file context>
Suggested change
| _ = h.services.registry.perform("toggleSidebar", invocation: ActionInvocation(origin: .user)) | |
| #expect(h.window.pendingCreationOwnsKeyboard, "no claim before the person's action") | |
| #expect(h.services.registry.perform("toggleSidebar", invocation: ActionInvocation(origin: .user)), "toggleSidebar did not run") |
Contributor
Author
|
Review (review-agent-composer-focus): SHIP at bd887fa Scope: What I checked
Findings (none block)
|
…ttings popover panel it made Green step f28ce3bdca7354ac9eaa15b5 passed every test and failed the suite on the window-leak check: keysTypedForAPersonsNewTerminalOverHomeNeverReachTheChiefMessageBox left the conversation's NativePopupPanel open (the Home makes it up front and only hides it). Closed in the test's own teardown, as HomeOpenFocusesComposerTests does.
lawrencecchen
temporarily deployed
to
ci-routing-agent
November 1, 2026 03:30 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
lawrencecchen
temporarily deployed
to
ci-routing-agent
November 1, 2026 03:30 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
Contributor
Author
|
Review (review-agent-composer-focus-2): SHIP at af500ef Scope: Findings:
Evidence:
Verdict: SHIP. Land when 2549eaa093d29b23b5c0d47b is green with no window leak. |
lawrencecchen
added
the
ready-to-land
Conflicts with #29301 (373f346, held keys carry a lane: the creation whose tab the focus named): - CreationInputBuffer: open takes both `machine` (feat) and `newWorkspace` (this PR). A key whose focus names no creation's tab while a new workspace of the hold is on its way goes to that workspace's lane (Home showed, so the focus named no new tab). take(shown:) becomes dropKeys(ofWorkspacesOtherThan:) before feat's take(focused:isShortcut:); take(shown:) stays as drop + take. - CreationInputCoordinator: feat's CreationHoldLog (holdEnds) replaces the local failedCreation/lastHoldEnd; the flush drops unshown workspace keys, then takes the lanes as feat does. One comment line folded to keep the file at the 400-line limit. - DecidedKeyEvents: feat made the same move to CmuxNextFocusUI; feat's copy. - CreationInputHoldTests: both tests kept; this PR's captures name no focused tab.
A move only. KeyWindowRole says which window the key window acts for and its close rule (keyboard focus code; it reads WindowKindRegistry and isChromiumPageWindow, both already in CmuxNextDesign and CmuxNextFocusUI). The only edits are the access words a move across modules needs (public on the type, its fields and resolve; public imports) and dropping the import of its new module. KindlessWindowAudit stays internal beside it. Feat moved DecidedKeyEvents itself in #29301, so this PR needs one more move to keep CmuxNextApp from growing (app-size rule).
lawrencecchen
temporarily deployed
to
ci-routing-agent
November 1, 2026 05:22 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
lawrencecchen
temporarily deployed
to
ci-routing-agent
November 1, 2026 05:22 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
lawrencecchen
removed
the
ready-to-land
lawrencecchen
temporarily deployed
to
ci-routing-agent
November 1, 2026 05:25 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
Contributor
Author
|
Review (review-agent-composer-merge): SHIP at 7688e8b Scope: the merge of feat-cmux-next 2d8fbe0 (9591708) and the move 7688e8b, against the SHIP at af500ef and against #29301 (373f346). Merge scope
Keys for a new workspace that is not shown, or whose creation failed (CreationInputBuffer.swift, CreationInputCoordinator.swift)
The
Tests
Evidence read once
|
lawrencecchen
added
ready-to-land
Contributor
Author
DIRTY since the 07:02Z merge round: the merge with feat needs App-size work (ready-queue unblocker)I added the display proof What I found when I merged origin/feat-cmux-next (local only, not pushed):
So the owner of this branch must move about 26 lines of the new code out of CmuxNextApp (the |
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).
lawrencecchen
had a problem deploying
to
ci-routing-agent
November 1, 2026 07:27 — with
GitHub Actions
Error
GitHub Actions
Error
lawrencecchen
temporarily deployed
to
ci-routing-agent
November 1, 2026 07:27 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
One conflict: AppActions+Workspaces.swift newWorkspace, with #26078 (Run New Workspaces in Cloud). Feat's code is kept as is: the source window (the invocation's window target, else the active one) in place of windows.active, the Cloud route before the create, and the Cloud notice toast. This PR's rules sit on it: byPerson (the run's own origin), no New Tab page for automation, the person's keyboard claim in the source window, and a script's quiet create shown only when no text input has the keyboard; the Cloud notice shows after either show path.
lawrencecchen
temporarily deployed
to
ci-routing-agent
November 1, 2026 07:32 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
lawrencecchen
temporarily deployed
to
ci-routing-agent
November 1, 2026 07:48 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
|
Found 13 test failures on Blacksmith runners: Failures
...and 3 more test failures. View all on Blacksmith: run 41285212937, run 41285213049
|
![Fix with [code]smith](https://pr-comments-assets.blacksmith.sh/codesmith/fix-with-codesmith-light-3x.png)
lawrencecchen
temporarily deployed
to
ci-routing-agent
November 1, 2026 09:10 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
lawrencecchen
added a commit
that referenced
this pull request
Nov 1, 2026
…30476) * test(hooks): a merge commit registers none of the base's tests The pre-commit hook reads the cached diff against HEAD, which in a merge holds every test the base added since the branch point, so it wrote the base's unregistered test into the pull request (tests/test_nightly_dedicated_mini.py in the merge commits of #30071 and #30351). This test fails on the base: python3 tests/test_install_git_hooks.py InstallGitHooksTests.test_pre_commit_registers_no_test_in_a_merge_commit * fix(hooks): a merge commit registers no tests in tests/test-execution.toml In a merge, the cached diff against HEAD holds every test file and workflow line the other parent added since the branch point, so the pre-commit hook registered the base's tests in the pull request's merge commit (tests/test_nightly_dedicated_mini.py in #30071 and #30351). A merge adds no test of its own: the hook now detects MERGE_HEAD first and skips registration as it already skipped the formatters. Red on the base, green here: python3 tests/test_install_git_hooks.py InstallGitHooksTests.test_pre_commit_registers_no_test_in_a_merge_commit Whole file: python3 tests/test_install_git_hooks.py (41 tests OK).
Contributor
Author
|
Review (review-agent-composer-merge-2): SHIP at 9923d35 Scope: the two merges and 10f80bb since the SHIP at 7688e8b. Diffs are against the merged feat parent 6eb6839 (feat moved after the merge).
Cloud follow-up (not this PR's): the Cloud route still uses Evidence: compile 21e438d0d02304bb2cbd8a7e on this head is done, with receipts ok:true for CmuxNext (CmuxNextAppTests compiled), app and cmux-tui. Display green 03c125f17b5ba43ae7e38807 (HomePendingTerminalFocusTests, PersonTerminalKeyboardClaimTests, CreationInputHoldTests, SecondCreationKeysTests, CMUX_TEST_REQUIRE_GUI=1) is still queued (waiting_for_worker). Not run, not read. |
Collaborator
|
For awareness: this PR has a SHIP review at its head, but GitHub reports it as conflicting with |
This branch was previously deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
With Home showing, keys typed for a new terminal could land in Home's Chief message box, and Return sent the shell command to the Chief as a chat message (dogfood v2.2.3, P0 journey
scripted-newtab-keeps-person-keys). A script'snewTabwith focus could also take the window from a person typing in a field. Both are fixed:newTabfrom the person (menu, palette, key), shown in the window they type in, opens a key hold (CreationInputCoordinator.beginWorkspace). Keys wait until the window shows that workspace. If the create fails or the workspace never shows, those keys are dropped, never typed into whatever has the focus.KeyboardClaims). A timeout never ends it, and neither does automation. While the claim holds, no primary input takes the keyboard by itself (NSWindow.pendingCreationOwnsKeyboard), and keys that run no binding are kept from the window.newTabwith focus is created without view change. Once it exists, the window shows it only if no text input has the person's keyboard in the active key window (WindowController.hasTextInputKeyboard: an editable text view or field, an input method composing, or an editable page element). A person who runscmux open --focusfrom a terminal or from another app still gets the workspace shown.newTab, built with the default user invocation, opens no New Tab page and no key hold.Created but not shown (open behavior note,
big-changes.md). When a script's new workspace arrives while the person types in a field, the workspace is created and listed in the sidebar, and the window stays where the person is typing. The check runs when the workspace exists, not when the script asks for it, so a person who started typing meanwhile keeps their input. This PR adds no badge or toast for a workspace that is listed but not shown. Its sidebar row is the only sign. That stays an open UX question.Follow-up (lead decision). If the person's own claimed create fails, this PR keeps the claim until their next intent, so keys typed meanwhile are kept from the window and nothing says why (review finding 1). A follow-up PR, red first, ends the claim when its own create fails and shows the failure through the create-failure notice, which says the typed keys were not delivered. Keys held for that create still never reach an agent's composer.
Ported from
cp/fix-chief-composer-focus(perf merge 943ac39), changed to fit feat's code. Feat has noKeyRoutermodes, so the hold is read fromCreationInputCoordinator.isHolding(in:).App size. CmuxNextApp shrinks by 27 lines over the PR. The new types are in CmuxNextFocusUI (
KeyboardClaims, the new-workspace tickets ofCreationInputBuffer,NSWindow.firstResponderTypesText) and CmuxNextFocus (FocusEvent.isPersonsOwn). Commit b1fc9a3 only moves five focus and key files (DecidedKeyEvents,ShortcutHintModifierPolicy,PaneNavigationHintPlan,BrowserFocusModeEscapeExit,NSResponder.ownsTypedText) from CmuxNextApp to CmuxNextFocusUI. Its only edits are thepublicaccess words and the public inits that a move across modules needs. So the fix commit can be read on its own.Owner note.
Windows/WindowController.swiftis in the PL-switch area. The change is 4 lines: the focus coordinator'spersonIntenthook and ending the window's claim inteardown(). No switch path changes.Commits: c65b87a (red test), b1fc9a3 (move only), bd887fa (fix), af500ef (test only: the keys test closes the Home's Chief Settings popover panel it made).
Testing
HomePendingTerminalFocusTestson c65b87a (feat plus the test), fleet step9324962aac1d94d400bb65c2on aws-m4pro-9,CMUX_TEST_REQUIRE_GUI=1: failed with the tests run, showing the P0.keysTypedForAPersonsNewTerminalOverHomeNeverReachTheChiefMessageBoxfound no hold, Home's message box took the keyboard, and "echo ok" went into the Chief box.f28ce3bdca7354ac9eaa15b5on aws-m4pro-7. Every test passed (HomePendingTerminalFocusTests 5/5, PersonTerminalKeyboardClaimTests 8/8, CreationInputHoldTests 6/6, 0 skipped). The suite failed only on the window-leak check: the keys test left the Home's Chief SettingsNativePopupPanelopen. af500ef closes it in the test's teardown. The leak check is unchanged. Green rerun on af500ef: fleet step2549eaa093d29b23b5c0d47bon aws-m4pro-2,CMUX_TEST_REQUIRE_GUI=1: all 3 suites passed (HomePendingTerminalFocusTests, PersonTerminalKeyboardClaimTests, CreationInputHoldTests), 0 skipped, no window leak, no FLAKY, exit 0.cmux-ci compile-check --app --package CmuxNext --recipe-version 2on bd887fa, job7c2baf4ef55ccb7ebbec6281: CmuxNext, app and cmux-tui ok. On af500ef: job706ef390c8e5167d9d836c9e: CmuxNext, app and cmux-tui ok.HomePendingTerminalFocusTestsneeds this tree's cmux-tui. It used to skip without it (skips audit row 3). Its suite trait now also runs it whenCMUX_TEST_REQUIRE_GUI=1, so a run without the binary fails atwithApp's#require(SameTreeDaemonBinary.url)and does not skip.package-test-lane.shalready counts a "needs the same-tree cmux-tui" skip as no result.Observed / Not run
899cd57b1887b17c35461ea7on a gui-session-v1 worker (cmuxs-Mac-mini), in cmux's console session (launchctl asuser 501),CMUX_TEST_REQUIRE_GUI=1: HomePendingTerminalFocusTests 5/5, PersonTerminalKeyboardClaimTests 8/8, CreationInputHoldTests 7/7, SecondCreationKeysTests 11/11. All 4 suites passed, exit 0 (log read withcmux-ci log; added by the ready-queue unblocker).cmux-ci log --full), andprepush-gate.py("no new violations",app_lines -27).scripted-newtab-keeps-person-keyson a feat build, and a live check in a tagged build. No perf claim.Changelog
Fixed: Keys typed for a new terminal while Home shows no longer reach Home's message box, and a script's new workspace no longer takes the window from a person typing in a field
Proof
No capture: keyboard routing and focus behavior with no visual change. The evidence is the fleet test steps above.