Skip to content

Route all surface shortcuts through focused Dock - #9566

Merged
austinywang merged 2 commits into
mainfrom
issue-9518-dock-focus-cmd-l-cmd-shift-t
Aug 5, 2026
Merged

austinywang merged 2 commits into
mainfrom
issue-9518-dock-focus-cmd-l-cmd-shift-t

Conversation

@austinywang

@austinywang austinywang commented Aug 4, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • classify every configurable shortcut as Dock-scoped, focus-resolved, or main-container-owned
  • route every surface-scoped shortcut through the focused Dock before any main-area fallback
  • add a per-Dock closed-panel history so Cmd+Shift+T restores the focused Dock most recently closed panel
  • add behavior regressions plus a CI routing guard for future shortcuts

Structural safeguard

KeyboardShortcutSettings.Action.dockShortcutRoutingDisposition exhaustively classifies all 143 actions with no default case: 44 Dock-scoped, 37 focus-resolved, and 62 main-container-owned. Adding an action now fails compilation until its ownership is explicit.

tests/test_dock_shortcut_routing_guard.py, run by workflow-guard-tests, independently scans the action-aware Dock gate calls in the dispatcher sources and requires exact equality with the Dock-scoped set. A new surface-scoped action therefore cannot pass CI with classification alone; its handler must also call focusedDockStoreForShortcut, performFocusedDockShortcut, routeCreateToFocusedDock, or routeSplitToFocusedDock. The routing helpers accept the action and assert if a non-Dock-scoped action attempts to use the gate.

Dispatcher audit

Already Dock-aware before this change:

  • triggerFlash
  • nextSurface, prevSurface, and legacy Ctrl+Tab surface navigation
  • moveSurfaceLeft, moveSurfaceRight
  • selectSurfaceByNumber
  • focusLeft, focusRight, focusUp, focusDown
  • toggleSplitZoom
  • splitRight, splitDown, splitBrowserRight, splitBrowserDown
  • newSurface, openBrowser
  • focusHistoryBack, focusHistoryForward
  • closeTab

Focus-resolved handlers already target the event responder or focused panel, including Dock panels. These include browser navigation, reload, zoom, developer tools, focus/design mode, Simulator commands, Markdown/file preview zoom, diff viewer navigation, and focused TextBox/checklist commands.

The audit found these missing or incomplete Dock routes, all addressed here:

  • focusBrowserAddressBar
  • reopenClosedBrowserPanel
  • focusPreviousPane, focusNextPane
  • equalizeSplits
  • closeOtherTabsInPane
  • renameTab
  • toggleTerminalCopyMode
  • focusTextBoxInput
  • attachTextBoxFile
  • sendCtrlFToTerminal
  • clearScreenKeepScrollback
  • find
  • findNext, findPrevious, hideFind, useSelectionForFind
  • toggleReactGrab
  • moveSurfaceToPreviousPane, moveSurfaceToNextPane
  • moveSurfaceToPaneLeft, moveSurfaceToPaneRight, moveSurfaceToPaneUp, moveSurfaceToPaneDown

App, window, workspace, sidebar, Canvas, workspace terminal-font-size, directory-search, and diff-viewer-opening actions remain intentionally main-container-owned.

Behavior details

Cmd+L focuses the focused Dock browser address bar. If the focused Dock panel is not a browser, the handler preserves the existing main-area fallback. The existing main-area behavior also remains when no Dock owns focus.

Cmd+Shift+T uses a real non-persisted ClosedItemHistoryStore owned by each DockSplitStore. User tab closes, pane closes, and Close Other Tabs stage snapshots before teardown. Configuration resets, runtime force-closes, transfers, and restore teardown are excluded. Restore prefers the original pane, then a same-pane anchor, then the former split placement, and finally the focused or root pane. Dock reopen never consumes the main-area shared history. If the focused Dock has no restorable item, the shortcut is consumed and beeps.

Transferred terminal history preserves both the live source-workspace identity and the snapshot workspace identity. A remote terminal fails closed if its live restore owner cannot be resolved, so reopen cannot silently create a local shell. Pane close and Close Other Tabs snapshot each batch once, reuse at most one agent index, and use indexed pane lookup for fallback anchors.

Dock rename writes Bonsplit custom-title state and prevents live terminal or browser title updates from overwriting it. The custom title round-trips through the existing Dock session snapshot fields.

Tests and validation

The first commit contains the failing behavior regressions and CI guard. The second commit contains the implementation.

Completed locally without invoking xcodebuild:

  • Swift parser checks for every changed Swift source and test file
  • python3 tests/test_dock_shortcut_routing_guard.py
  • python3 scripts/check-test-determinism.py --strict
  • python3 tests/test_ci_change_areas.py
  • git diff --check
  • ./scripts/check-pbxproj.sh
  • ./scripts/lint-pbxproj-test-wiring.sh
  • workspace package-group and Package.resolved policy checks
  • cmux architectural policy check
  • localization catalog validation for every locale, including Khmer and Ukrainian
  • cancellation regressions proving process-detected resume bindings survive cancelled tab and pane closes
  • placeholder-only fallback split restoration, without launching a temporary terminal

Remote validation on pre-rebase feature head faada6719b: DockShortcutRoutingTests and DockControlDefinitionDecodingTests passed, and every required PR check is green. A manually dispatched full CI run also passed the routing guard, Swift warning budget, app/test build, runtime regressions, and Swift package tests; its app-host matrix hit pre-existing baseline failures tracked by #8614 and #9574, so that non-required run was stopped rather than adding unrelated test repairs to this PR.

Local xcodebuild tests and a local tagged build were intentionally not run per the task instructions.

Closes #9518

Summary by CodeRabbit

  • New Features
    • Expanded keyboard shortcuts for Dock pane movement, focus cycling, split management, tab renaming, terminal and browser search, and related actions.
    • Added reopening of recently closed Dock panels and panes, restoring layout, focus, and session state when possible.
    • Browser address-bar shortcuts now target the focused Dock browser panel.
  • Bug Fixes
    • Closing and cancelling panel or tab closes now preserves state more reliably and avoids stale notifications.
    • Dock title updates better preserve custom tab names.
  • Localization
    • Added Khmer and Ukrainian translations for tab naming and cancellation actions.

Tagged dogfood

  • Built and user-tested feature head faada67 with CMUX_SKIP_ZIG_BUILD=1 /Users/austinwang/manaflow/cmuxterm-hq/scripts/reload-cloud.sh --tag issue-9518-dock-focus-cmd-l-cmd-shift-t --launch. Cloud build 30954884110 completed successfully on blacksmith-6vcpu-macos-26.
  • Launched the isolated tagged app and created a dedicated Dock Shortcut QA workspace with Dock terminals, a Dock browser, and a distinct main-area browser as a negative control.
  • Confirmed that live Settings bindings resolve to semantic actions before focus routing, so the same user-customized binding targets the main container or Dock solely according to keyboard focus. No shortcut behavior is hardcoded to its default key combination.
  • User dogfood confirmed the Dock shortcuts work in the tagged build. The tagged app was left running for follow-up testing.

@coderabbitai

coderabbitai Bot commented Aug 4, 2026 •

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Dock keyboard shortcuts now use explicit action routing through the focused Dock. Dock commands support pane operations, terminal and browser actions, React Grab, and closed-panel restoration. Dock close history preserves snapshots and workspace context.

Changes

Dock shortcut routing and command execution

Layer / File(s) Summary
Action-aware focused Dock routing
Sources/AppDelegate.swift, Sources/AppDelegate+DockShortcutRouting.swift, Sources/AppDelegate+AdjacentNavigationShortcut.swift
Shortcut actions are classified and routed through the focused Dock before existing fallbacks.
Dock shortcut command execution
Sources/DockSplitStore+ShortcutCommands.swift, Sources/DockSplitStore.swift
Dock commands handle pane movement, focus cycling, tab operations, terminal and browser find actions, React Grab, renaming, and closed-panel reopening.
Closed-panel history and restoration
Sources/ClosedItemHistory.swift, Sources/DockSplitStore*, Sources/Workspace+DockBrowserLookup.swift, Sources/TabManager.swift, Sources/AppDelegate+WindowDock.swift, Sources/SessionSplitContainerLayoutCodec.swift
Dock closures stage and commit history, restore panel snapshots, preserve workspace identifiers, and suppress history for runtime cleanup.
Focused Dock routing validation
cmuxTests/DockShortcutRoutingTests.swift, cmuxTests/DockControlDefinitionDecodingTests.swift, tests/test_dock_shortcut_routing_guard.py, .github/workflows/ci.yml, cmux.xcodeproj/project.pbxproj, Resources/Localizable.xcstrings
Tests, static checks, CI wiring, project references, and localization support cover the updated Dock behavior.

Estimated code review effort: 5 (Critical) | ~120 minutes

Possibly related issues

  • manaflow-ai/cmux-dev-artifacts#8435 — Concerns compilation in modified Dock shortcut command components.
  • manaflow-ai/cmux-dev-artifacts#8379 — Concerns build failures in modified Dock shortcut routing tests.
  • manaflow-ai/cmux-dev-artifacts#8341 — Concerns focused Dock shortcut routing and Dock ownership resolution.
  • manaflow-ai/cmux-dev-artifacts#8614 — Concerns closure compilation in the modified Dock close-confirmation code.

Possibly related PRs

Suggested reviewers: lawrencecchen


Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (2 warnings, 1 inconclusive)

Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning The PR includes broad shortcut routing, rename persistence, localization, CI guards, and other changes beyond #9518's two focused-Dock shortcuts. Split unrelated work into separate pull requests or link issues that define the broader routing, history, rename, localization, and CI objectives.
Docstring Coverage ⚠️ Warning Docstring coverage is 20.75% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Cmux Cache Substitution Correctness ❓ Inconclusive No result was produced after verification. Marking as INCONCLUSIVE. Re-run the check or adjust instructions to produce a final result.
✅ Passed checks (22 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The changes directly fix #9518 by routing Cmd+L and Cmd+Shift+T through the focused Dock and adding Dock-local closed-item history.
Cmux Swift Actor Isolation ✅ Passed PR introduces no Swift 6 actor isolation violations. DockSplitStore and ClosedItemHistoryStore are properly @MainActor-annotated; DockShortcutCommand is a synchronous-only value type on MainActor;...
Cmux Swift Blocking Runtime ✅ Passed No new blocking or timing-based synchronization patterns introduced. Changes focus on Dock routing, closed-panel history tracking, and parameter propagation without adding semaphores, blocking wait...
Cmux Browser Automation Off-Main ✅ Passed The PR does not modify Sources/TerminalController.swift or ControlCommandExecutionPolicy.swift, and its browser changes are Dock shortcut routing rather than socket automation.
Cmux Expensive Synchronous Load ✅ Passed The only new expensive load is cache-first at DockSplitStore+ClosedItemHistory.swift:198-199; the nil-guarded RestorableAgentSessionIndex.load() is an explicit documented cold-cache fallback.
Cmux No Hacky Sleeps ✅ Passed PR contains only Swift, YAML workflow, test, and localization changes. The Python test file (tests/test_dock_shortcut_routing_guard.py) contains no sleep/delay code. GitHub Actions workflow YAML...
Cmux Algorithmic Complexity ✅ Passed No algorithmic complexity violations found. All new code (1139 lines across 10 files) operates on tiny fixed-size collections: Dock panes (~10-20), tabs per pane (~1-100), and split tree depth O(lo...
Cmux Swift Concurrency ✅ Passed The only new runtime async work is stored reactGrabTask, cancelled on superseding commands, panel destruction, and reset; no new Dispatch, Combine, or internal completion-handler pattern was added.
Cmux Swift @Concurrent ✅ Passed The diff adds no new nonisolated async declarations or @concurrent annotations. The new React Grab task is explicitly @MainActor, while its network/hash loader uses Task.detached.
Cmux Swift Package Boundaries ✅ Passed Changed logic is Dock/AppDelegate composition and depends on AppKit, BonsplitController, panel/session types, AppDelegate.shared, and agent globals; it is not an app-independent reusable package fe...
Cmux Swiftpm Lockfiles ✅ Passed The full PR changes no Package.swift, Package.resolved, or .gitignore files; the pbxproj adds only source references, with no package-reference changes, and the root lockfile is unchanged.
Cmux Swift Logging ✅ Passed The changed Swift diff adds no print, debugPrint, dump, NSLog, Logger, or ad hoc diagnostic logging; existing Logger declarations and logs are unchanged.
Cmux User-Facing Error Privacy ✅ Passed The production diff adds only generic rename-tab dialog text and a developer assertion; no user-facing error exposes vendor names, internal identifiers, raw messages, credentials, or payloads.
Cmux Full Internationalization ✅ Passed New Dock rename UI uses localized keys already present in Localizable.xcstrings; all five touched keys have translated entries for 20 locales, and other added literals are internal or protocol tokens.
Cmux Swiftui State Layout ✅ Passed The PR adds no SwiftUI view or layout code. Existing ClosedItemHistoryStore ObservableObject/@published state is unchanged, and the new Task runs from a Dock command handler, not render time.
Cmux Architecture Rethink ✅ Passed The PR introduces exhaustive Dock-routing classification with no timing/dispatch patches, no new side channels or competing owners, and enforced gates that prevent architectural violations at compi...
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed The diff adds only an NSAlert rename dialog via runCmuxModal, not a standalone window; no new identifier assignment appears, and scripts/lint_auxiliary_window_close_shortcuts.py passes.
Cmux Source Artifacts ✅ Passed All 23 changed files are legitimate source code, tests, configs, or localization catalogs with no prohibited artifacts detected.
Cmux No Test Or Debug Seam In Production Source ✅ Passed The PR adds no DEBUG/test-build seam or test-named production member. The only visibility widening, dockCloseConfirmationManager, has a production caller in Dock shortcut handling.
Cmux No Ambient Global State ✅ Passed No new file-scope API functions, mutable globals, static-only namespaces, or singletons were added; new history and task state belongs to DockSplitStore with injectable ClosedItemHistoryStore.
Title check ✅ Passed The title clearly summarizes the primary change: routing surface shortcuts through the focused Dock.
Description check ✅ Passed The description provides a detailed summary, testing results, behavior details, validation scope, and dogfood results, but omits the template checklist and review-trigger section.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch issue-9518-dock-focus-cmd-l-cmd-shift-t

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@austinywang
austinywang force-pushed the issue-9518-dock-focus-cmd-l-cmd-shift-t branch from 296ec0d to 7928d9e Compare August 4, 2026 08:26

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
Sources/AppDelegate.swift (1)

14624-14639: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

Fail closed when the focused Dock panel is not a browser.

This is the only Dock branch in the diff that can fall through to the main area after the Dock gate passes. If the Dock owns keyboard focus and its focused panel is a terminal, dock.browserPanel(for: focusedPanelId) returns nil. Control then reaches line 14636 and focuses the main-area browser address bar, or line 14646 and creates a main-area browser. Issue #9518 requires Cmd+L to target the focused Dock browser instead of the main-area browser.

Every other Dock branch added in this PR consumes the event once focusedDockStoreForShortcut returns a store. Keep that contract here: when the Dock owns focus, resolve a browser inside the Dock, and fail closed instead of acting on the main area.

Note that the new test focusAddressBarTargetsFocusedDockBrowser seeds a Dock browser as the focused panel, so it does not cover this path.

🐛 Proposed fix to keep Cmd+L inside the focused Dock
         if matchConfiguredShortcut(event: event, action: .focusBrowserAddressBar) {
             if let dock = focusedDockStoreForShortcut(
                 action: .focusBrowserAddressBar,
                 preferredWindow: event.window
-            ),
-            let focusedPanelId = dock.focusedPanelId,
-            let focusedBrowser = dock.browserPanel(
-                for: focusedPanelId
-            ) {
-                focusBrowserAddressBar(in: focusedBrowser)
-                return true
+            ) {
+                // The Dock owns keyboard focus, so this shortcut must not reach
+                // the main area. Focus the Dock's focused browser, or create one
+                // in the Dock; never fall through.
+                if let focusedPanelId = dock.focusedPanelId,
+                   let focusedBrowser = dock.browserPanel(for: focusedPanelId) {
+                    focusBrowserAddressBar(in: focusedBrowser)
+                    return true
+                }
+                if routeCreateToFocusedDock(
+                    .browser,
+                    focusAddressBar: true,
+                    action: .focusBrowserAddressBar,
+                    preferredWindow: event.window
+                ) != nil {
+                    return true
+                }
+                NSSound.beep()
+                return true
             }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Sources/AppDelegate.swift` around lines 14624 - 14639, Update the
focusBrowserAddressBar shortcut branch around focusedDockStoreForShortcut so
that once a Dock store is returned, it handles the event exclusively within that
Dock: focus its browser when available, but return without falling through when
the focused panel is not a browser. Preserve the existing main-area fallback
only when no Dock owns focus.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@Sources/DockSplitStore`+SessionRestore.swift:
- Around line 89-94: Preserve the remote terminal’s source workspace identity
during closed-item restoration: in
Sources/DockSplitStore+ClosedItemHistory.swift:137-150, store
detachedSurfaceTransfersByPanelId[panelId]?.sessionRestoreWorkspaceId on each
ClosedPanelHistoryEntry; in Sources/DockSplitStore+SessionRestore.swift:89-94,
pass that stored identity to createSessionRestoredPanel instead of nil and fail
closed when it cannot be resolved.

In `@Sources/DockSplitStore`+SessionSnapshot.swift:
- Around line 95-97: Update the snapshot method containing the agentIndex
resolution in Sources/DockSplitStore+SessionSnapshot.swift: resolve the index
only for terminal snapshots and accept an optional already-resolved index from
batch callers, leaving browser snapshots without unnecessary index loading. In
Sources/DockSplitStore+ClosedItemHistory.swift, resolve the agent index once
before the tab iteration in stageDockClosedPaneHistory(_:) and pass that value
to each history-entry snapshot.

In `@Sources/DockSplitStore`+ShortcutCommands.swift:
- Around line 431-435: Update the close-warning check in the Dock shortcut
handling to construct CloseTabWarningStore with the injected
closeTabWarningDefaults source used by TabManager instead of
UserDefaults.standard. Preserve the existing shouldConfirmClose arguments and
behavior while routing the preference through the shared injectable owner.

---

Outside diff comments:
In `@Sources/AppDelegate.swift`:
- Around line 14624-14639: Update the focusBrowserAddressBar shortcut branch
around focusedDockStoreForShortcut so that once a Dock store is returned, it
handles the event exclusively within that Dock: focus its browser when
available, but return without falling through when the focused panel is not a
browser. Preserve the existing main-area fallback only when no Dock owns focus.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 90bf67f7-a3a6-45e5-8086-d3520b2b014f

📥 Commits

Reviewing files that changed from the base of the PR and between 197ede7 and 7928d9e.

📒 Files selected for processing (15)
  • Sources/AppDelegate+AdjacentNavigationShortcut.swift
  • Sources/AppDelegate+DockShortcutRouting.swift
  • Sources/AppDelegate+WindowDock.swift
  • Sources/AppDelegate.swift
  • Sources/DockSplitStore+CloseConfirmation.swift
  • Sources/DockSplitStore+ClosedItemHistory.swift
  • Sources/DockSplitStore+Reset.swift
  • Sources/DockSplitStore+SessionRestore.swift
  • Sources/DockSplitStore+SessionSnapshot.swift
  • Sources/DockSplitStore+ShortcutCommands.swift
  • Sources/DockSplitStore.swift
  • Sources/TabManager.swift
  • Sources/Workspace+DockBrowserLookup.swift
  • cmux.xcodeproj/project.pbxproj
  • cmuxTests/DockShortcutRoutingTests.swift

Comment thread Sources/DockSplitStore+SessionRestore.swift Outdated
Comment thread Sources/DockSplitStore+SessionSnapshot.swift Outdated
Comment thread Sources/DockSplitStore+ShortcutCommands.swift Outdated
@austinywang
austinywang force-pushed the issue-9518-dock-focus-cmd-l-cmd-shift-t branch from 7928d9e to 50ed762 Compare August 4, 2026 08:36

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@Sources/AppDelegate.swift`:
- Around line 14574-14594: Document in the legacy Ctrl+Tab shortcut handling
around matchesLegacyNextSurfaceShortcut and matchesLegacyPreviousSurfaceShortcut
that these strokes intentionally reuse the configurable .nextSurface and
.prevSurface actions when calling performFocusedDockShortcut. Keep the existing
routing unchanged and apply the same clarification to all corresponding legacy
shortcut branches.
- Around line 14624-14635: Update the Cmd+L handling around
focusedDockStoreForShortcut and focusBrowserAddressBar so the route returns true
as soon as the Dock store resolves, even when focusedPanelId or
browserPanel(for:) is unavailable; preserve focusing the browser when all nested
guards succeed and use the existing failure/beep behavior for the unavailable
focused Dock target, preventing fallthrough to the main-area browser fallback.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 4b584c7e-3751-4498-ab8f-cc45c986243c

📥 Commits

Reviewing files that changed from the base of the PR and between 7928d9e and 50ed762.

📒 Files selected for processing (14)
  • Sources/AppDelegate+AdjacentNavigationShortcut.swift
  • Sources/AppDelegate+DockShortcutRouting.swift
  • Sources/AppDelegate+WindowDock.swift
  • Sources/AppDelegate.swift
  • Sources/DockSplitStore+CloseConfirmation.swift
  • Sources/DockSplitStore+ClosedItemHistory.swift
  • Sources/DockSplitStore+Reset.swift
  • Sources/DockSplitStore+SessionRestore.swift
  • Sources/DockSplitStore+SessionSnapshot.swift
  • Sources/DockSplitStore+ShortcutCommands.swift
  • Sources/DockSplitStore.swift
  • Sources/TabManager.swift
  • Sources/Workspace+DockBrowserLookup.swift
  • cmux.xcodeproj/project.pbxproj

Comment thread Sources/AppDelegate.swift
Comment thread Sources/AppDelegate.swift
@austinywang
austinywang force-pushed the issue-9518-dock-focus-cmd-l-cmd-shift-t branch from 50ed762 to ae5ab54 Compare August 4, 2026 09:40

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
Sources/DockSplitStore+CloseConfirmation.swift (1)

87-94: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Stage only user-closed tabs in pane close history.

shouldClosePane immediately stages the pane with stageDockClosedPaneHistory(pane), before checking forceCloseDockTabIds. Forced tab closes via forceCloseDockTabIds return in shouldCloseTab, so the tabs are never staged through the tab path. shouldClosePane should also skip or discard history for tabs already in forceCloseDockTabIds, or stage only the confirmable/closeable tabs that are actually closing.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Sources/DockSplitStore`+CloseConfirmation.swift around lines 87 - 94, Update
splitTabBar(_:shouldClosePane:) so stageDockClosedPaneHistory(pane) records only
tabs that are actually user-closed, excluding any tab IDs in
forceCloseDockTabIds. Stage the filtered confirmable/closeable set after
evaluating controller.tabs(inPane:) and force-close status, while preserving the
existing confirmation behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@Sources/DockSplitStore`+ClosedItemHistory.swift:
- Around line 260-286: Update restoreDockClosedPanelInFallbackSplit to use a
placeholder-only split creation API, matching the main-area path’s
newTerminalSplit behavior, instead of newSplit(kind: .terminal). Add or reuse
the corresponding non-launching helper while preserving the existing placement,
restoration, cleanup, and failure behavior.

In `@Sources/DockSplitStore`+SessionSnapshot.swift:
- Around line 120-127: The snapshot capture in the close-history flow is
happening prematurely before the close confirmation completes. When
markDockCloseHistoryEligible(panelId:) is called, it immediately captures the
session snapshot via dockClosedPanelHistoryEntry(...) with detectedResumeBinding
set to nil, but the actual binding may still be needed if the user cancels the
close operation. Defer the snapshot creation to occur only after
confirmCloseDockPanel(...) succeeds, or preserve the actual binding value
instead of passing nil so it remains available if the close is cancelled. Ensure
pendingClosedPanelHistoryEntries is only populated after the close is confirmed,
or ensure surfaceResumeBindingsByPanelId retains the binding through the entire
close flow.

In `@Sources/DockSplitStore`+ShortcutCommands.swift:
- Around line 214-241: Add the five missing rename-alert string catalog
entries—alert.renameTab.title, alert.renameTab.message,
alert.renameTab.placeholder, alert.renameTab.rename, and alert.cancel—for the km
and uk locales in Resources/Localizable.xcstrings, using matching translated
values so the localized calls in the rename alert resolve correctly.

In `@tests/test_dock_shortcut_routing_guard.py`:
- Around line 43-61: The disposition_actions parser currently ends at a mutable
documentation comment, allowing unrelated source cases to be included if the
comment changes. Replace that end anchor with a stable code boundary such as
DockShortcutRoutingDisposition’s declaration, or add explicit validation that
both split anchors are found before parsing; preserve the existing disposition
extraction behavior.

---

Outside diff comments:
In `@Sources/DockSplitStore`+CloseConfirmation.swift:
- Around line 87-94: Update splitTabBar(_:shouldClosePane:) so
stageDockClosedPaneHistory(pane) records only tabs that are actually
user-closed, excluding any tab IDs in forceCloseDockTabIds. Stage the filtered
confirmable/closeable set after evaluating controller.tabs(inPane:) and
force-close status, while preserving the existing confirmation behavior.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: b7d7aa0c-5938-4eab-9c5c-d918651e4747

📥 Commits

Reviewing files that changed from the base of the PR and between 50ed762 and ae5ab54.

📒 Files selected for processing (19)
  • .github/workflows/ci.yml
  • Sources/AppDelegate+AdjacentNavigationShortcut.swift
  • Sources/AppDelegate+DockShortcutRouting.swift
  • Sources/AppDelegate+WindowDock.swift
  • Sources/AppDelegate.swift
  • Sources/ClosedItemHistory.swift
  • Sources/DockSplitStore+CloseConfirmation.swift
  • Sources/DockSplitStore+ClosedItemHistory.swift
  • Sources/DockSplitStore+PanelDestruction.swift
  • Sources/DockSplitStore+Reset.swift
  • Sources/DockSplitStore+SessionRestore.swift
  • Sources/DockSplitStore+SessionSnapshot.swift
  • Sources/DockSplitStore+ShortcutCommands.swift
  • Sources/DockSplitStore.swift
  • Sources/TabManager.swift
  • Sources/Workspace+DockBrowserLookup.swift
  • cmux.xcodeproj/project.pbxproj
  • cmuxTests/DockShortcutRoutingTests.swift
  • tests/test_dock_shortcut_routing_guard.py

Comment thread Sources/DockSplitStore+ClosedItemHistory.swift
Comment thread Sources/DockSplitStore+SessionSnapshot.swift
Comment thread Sources/DockSplitStore+ShortcutCommands.swift
Comment thread tests/test_dock_shortcut_routing_guard.py
@austinywang
austinywang force-pushed the issue-9518-dock-focus-cmd-l-cmd-shift-t branch from ae5ab54 to a7ca99c Compare August 4, 2026 10:48
@cursor

cursor Bot commented Aug 4, 2026

Copy link
Copy Markdown

Bugbot is paused — on-demand spend limit reached

Bugbot uses usage-based billing for this team and has hit its on-demand spend limit.

A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue.

@austinywang

Copy link
Copy Markdown
Contributor Author

Addressed the outside-diff pane-history finding in a7ca99c. Pane-close history now excludes every tab already present in forceCloseDockTabIds and, when confirmation is required, stages the filtered tab set only after confirmation succeeds. The cancellation regressions also verify that process-detected resume bindings remain intact when tab or pane close is cancelled.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@cmuxTests/DockControlDefinitionDecodingTests.swift`:
- Around line 479-482: Update the close-panel test around closePanel and
promptCount to register a completion signal before invoking closePanel, then
await that signal before asserting promptCount == 1. Remove the fixed
ten-iteration Task.yield loop, and ensure the signal is fulfilled by the
close-confirmation callback so the test waits for real completion.

In `@cmuxTests/DockShortcutRoutingTests.swift`:
- Around line 440-450: Strengthen the assertions in the Dock browser restoration
test by locating the Dock’s BrowserPanel and verifying its restored URL is
https://example.org before asserting main-area history remains available. Keep
the existing BrowserPanel presence and mainWorkspace exclusion checks, and use
the restored panel’s actual URL property rather than only checking its type.
- Around line 704-710: Strengthen the test around Self.dispatch in the Dock
shortcut routing case by asserting the existing Dock-browser observable that
changes when .toggleReactGrab succeeds, rather than relying only on the dispatch
return value and focusedPanelId checks. Ensure the assertion distinguishes a
successful command from performFocusedDockShortcut returning true after
performShortcutCommand fails.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 2141661c-b648-48f2-8a54-889f87ead6ef

📥 Commits

Reviewing files that changed from the base of the PR and between ae5ab54 and a7ca99c.

📒 Files selected for processing (23)
  • .github/workflows/ci.yml
  • Resources/Localizable.xcstrings
  • Sources/AppDelegate+AdjacentNavigationShortcut.swift
  • Sources/AppDelegate+DockShortcutRouting.swift
  • Sources/AppDelegate+WindowDock.swift
  • Sources/AppDelegate.swift
  • Sources/ClosedItemHistory.swift
  • Sources/DockShortcutRoutingDisposition.swift
  • Sources/DockSplitStore+CloseConfirmation.swift
  • Sources/DockSplitStore+ClosedItemHistory.swift
  • Sources/DockSplitStore+PanelDestruction.swift
  • Sources/DockSplitStore+Reset.swift
  • Sources/DockSplitStore+SessionRestore.swift
  • Sources/DockSplitStore+SessionSnapshot.swift
  • Sources/DockSplitStore+ShortcutCommands.swift
  • Sources/DockSplitStore.swift
  • Sources/SessionSplitContainerLayoutCodec.swift
  • Sources/TabManager.swift
  • Sources/Workspace+DockBrowserLookup.swift
  • cmux.xcodeproj/project.pbxproj
  • cmuxTests/DockControlDefinitionDecodingTests.swift
  • cmuxTests/DockShortcutRoutingTests.swift
  • tests/test_dock_shortcut_routing_guard.py

Comment thread cmuxTests/DockControlDefinitionDecodingTests.swift
Comment thread cmuxTests/DockShortcutRoutingTests.swift Outdated
Comment thread cmuxTests/DockShortcutRoutingTests.swift
@austinywang
austinywang force-pushed the issue-9518-dock-focus-cmd-l-cmd-shift-t branch 4 times, most recently from b304af5 to d3e6030 Compare August 4, 2026 11:47

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@cmuxTests/DockShortcutRoutingTests.swift`:
- Around line 390-391: The test is using ClosedItemHistoryStore.shared which
persists state to the application support directory and affects test isolation.
Replace the shared instance usage with a test-scoped store initialized with a
temporary file URL. Create a temporary file path for the test, instantiate
ClosedItemHistoryStore with that path instead of accessing .shared, and use the
test-scoped instance in both the initial removeAll() call and the defer block to
ensure proper cleanup without persisting to the real application support
directory.

In `@Sources/DockSplitStore`+CloseConfirmation.swift:
- Around line 48-51: Update the close-confirmation gates in the tab close flow
and splitTabBar(_:shouldClosePane:) to resolve the confirmation manager first,
then initialize CloseTabWarningStore with that manager’s closeTabWarningDefaults
instead of .standard. Keep closeOtherDockTabsInFocusedPane() aligned with the
same defaults source so every Dock close path uses one policy.

In `@tests/test_dock_shortcut_routing_guard.py`:
- Around line 84-139: Add a concise comment near balanced_call_bodies stating
that its scanner only supports single-line, non-raw string literals and does not
correctly parse Swift multiline or raw string forms. Do not change the scanning
logic or other behavior.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: fa4050e9-d3f2-47f6-bd4a-e51ffe0217df

📥 Commits

Reviewing files that changed from the base of the PR and between a7ca99c and d3e6030.

📒 Files selected for processing (23)
  • .github/workflows/ci.yml
  • Resources/Localizable.xcstrings
  • Sources/AppDelegate+AdjacentNavigationShortcut.swift
  • Sources/AppDelegate+DockShortcutRouting.swift
  • Sources/AppDelegate+WindowDock.swift
  • Sources/AppDelegate.swift
  • Sources/ClosedItemHistory.swift
  • Sources/DockShortcutRoutingDisposition.swift
  • Sources/DockSplitStore+CloseConfirmation.swift
  • Sources/DockSplitStore+ClosedItemHistory.swift
  • Sources/DockSplitStore+PanelDestruction.swift
  • Sources/DockSplitStore+Reset.swift
  • Sources/DockSplitStore+SessionRestore.swift
  • Sources/DockSplitStore+SessionSnapshot.swift
  • Sources/DockSplitStore+ShortcutCommands.swift
  • Sources/DockSplitStore.swift
  • Sources/SessionSplitContainerLayoutCodec.swift
  • Sources/TabManager.swift
  • Sources/Workspace+DockBrowserLookup.swift
  • cmux.xcodeproj/project.pbxproj
  • cmuxTests/DockControlDefinitionDecodingTests.swift
  • cmuxTests/DockShortcutRoutingTests.swift
  • tests/test_dock_shortcut_routing_guard.py

Comment thread cmuxTests/DockShortcutRoutingTests.swift
Comment thread Sources/DockSplitStore+CloseConfirmation.swift Outdated
Comment thread tests/test_dock_shortcut_routing_guard.py
@austinywang
austinywang force-pushed the issue-9518-dock-focus-cmd-l-cmd-shift-t branch 6 times, most recently from b2553de to faada67 Compare August 4, 2026 13:39
@austinywang
austinywang force-pushed the issue-9518-dock-focus-cmd-l-cmd-shift-t branch from faada67 to 314224f Compare August 4, 2026 22:18
@austinywang
austinywang merged commit 2f3d922 into main Aug 5, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Dock focus: Cmd+L (focus browser address bar) and Cmd+Shift+T (reopen closed browser panel) always target the main area, ignoring Dock focus

1 participant