Repository navigation
Cmd+[ / Cmd+] traverse the global workspace focus history like the titlebar arrows - #9299
Closed
azooz2003-bit wants to merge 2 commits into
Closed
azooz2003-bit wants to merge 2 commits into
azooz2003-bit wants to merge 2 commits into
Conversation
📝 WalkthroughWalkthroughThe PR adds unbound previous- and next-focus shortcut actions, localized metadata, and schema entries. ChangesFocus history shortcuts
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant KeyboardEvent
participant AppDelegate
participant GhosttyGotoSplit
KeyboardEvent->>AppDelegate: Dispatch Cmd+[ or Cmd+]
AppDelegate->>AppDelegate: Check configured focus-history shortcut
AppDelegate->>GhosttyGotoSplit: Route to pane cycling when no focus-history match
Possibly related issues
Possibly related PRs
Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (2 errors, 1 warning)
✅ Passed checks (22 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
⚔️ Resolve merge conflicts 💡
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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 `@cmuxUITests/FocusHistoryShortcutUITests.swift`:
- Around line 251-267: Remove the count < buffer.count early-break condition
from the response-reading loop in the visible read logic. Continue accumulating
chunks until a newline is found, the deadline expires, or Darwin.read returns a
non-positive count, preserving the existing timeout and accumulator return
behavior.
In `@Sources/AppDelegate.swift`:
- Around line 786-796: Remove debugSetGhosttyGotoSplitCycleShortcuts from
Sources/AppDelegate.swift and relocate its test override behavior to a dedicated
debug support file or test-harness injection outside Sources/**/*.swift.
Preserve the ability of FocusHistoryBracketShortcutRoutingTests to configure the
Ghostty previous/next shortcuts without adding a test-only member to AppDelegate
production source.
In `@tests_v2/test_focus_history_shortcut_cross_workspace.py`:
- Around line 55-86: Replace fixed sleeps in
tests_v2/test_focus_history_shortcut_cross_workspace.py lines 55-86 after
activate_app, workspace creation/selection, and close_workspace with
deadline-bounded polls of _selected_workspace(c) or list_workspaces(), following
_press_and_wait; update cmuxUITests/FocusHistoryShortcutUITests.swift lines
115-123 to use waitForCurrentWorkspace(_:timeout:) instead of
RunLoop.current.run. Ensure all synchronization waits on real completion
predicates.
- Around line 55-56: Replace the fixed time.sleep calls in the focus-history
test with deadline-bounded polling of real state, following _press_and_wait.
After activate_app, poll c.ping(); after each select_workspace, poll until
_selected_workspace(c) equals the target workspace; and after close_workspace,
poll list_workspaces() until the workspace is absent. Preserve the existing test
sequence and assertions.
- Around line 32-48: Update _press_and_wait to tolerate transient cmuxError
exceptions from _selected_workspace during shortcut polling: catch the error
inside the deadline-bounded loop, retain the previous/current workspace value
when no workspace is selected, and continue retrying until expected_ws is
observed or the timeout expires. Preserve the existing final return behavior.
In `@web/data/cmux-shortcuts.ts`:
- Around line 315-332: Add complete locale coverage to the focusPreviousPane and
focusNextPane entries in the shortcut definitions, adding description and note
translations for zh-CN, zh-TW, ko, de, es, fr, it, da, pl, ru, bs, ar, no,
pt-BR, th, tr, km, and uk alongside en and ja. Keep each locale’s existing
translation structure and ensure both shortcuts receive matching fields.
🪄 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: b1e70f4e-cdc3-4cf9-86e5-ee0c78b66937
📒 Files selected for processing (14)
Packages/macOS/CmuxSettings/Sources/CmuxSettings/Values/ShortcutAction+Defaults.swiftPackages/macOS/CmuxSettings/Sources/CmuxSettings/Values/ShortcutAction+DisplayName.swiftPackages/macOS/CmuxSettings/Sources/CmuxSettings/Values/ShortcutAction+Group.swiftPackages/macOS/CmuxSettings/Sources/CmuxSettings/Values/ShortcutAction.swiftResources/Localizable.xcstringsSources/AppDelegate.swiftSources/KeyboardShortcutSettings.swiftcmux.xcodeproj/project.pbxprojcmuxTests/FocusHistoryBracketShortcutRoutingTests.swiftcmuxUITests/FocusHistoryShortcutUITests.swiftskills/cmux-settings/references/shortcut-actions.mdtests_v2/test_focus_history_shortcut_cross_workspace.pyweb/data/cmux-shortcuts.tsweb/data/cmux.schema.json
Comment on lines
+251
to
+267
| var buffer = [UInt8](repeating: 0, count: 4096) | ||
| var accumulator = "" | ||
| let deadline = Date().addingTimeInterval(responseTimeout) | ||
| while Date() < deadline { | ||
| let count = Darwin.read(fd, &buffer, buffer.count) | ||
| guard count > 0 else { break } | ||
| if let chunk = String(bytes: buffer[0..<count], encoding: .utf8) { | ||
| accumulator.append(chunk) | ||
| if let newline = accumulator.firstIndex(of: "\n") { | ||
| return String(accumulator[..<newline]) | ||
| } | ||
| if count < buffer.count { | ||
| break | ||
| } | ||
| } | ||
| } | ||
| return accumulator.isEmpty ? nil : accumulator.trimmingCharacters(in: .whitespacesAndNewlines) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
The early break on a partial read can truncate a slow multi-chunk response.
At line 262-264, the loop breaks as soon as count < buffer.count, even if no newline has been found yet. Darwin.read on a stream socket can legitimately return fewer bytes than requested while more of the line is still arriving, in which case this heuristic ends the loop prematurely and returns an incomplete response. SO_RCVTIMEO is already set on fd (lines 212-219), so the loop's Date() < deadline condition together with a count <= 0 timeout return is sufficient to bound the read; the partial-read heuristic is not needed and adds a truncation risk.
🔧 Proposed fix
if let chunk = String(bytes: buffer[0..<count], encoding: .utf8) {
accumulator.append(chunk)
if let newline = accumulator.firstIndex(of: "\n") {
return String(accumulator[..<newline])
}
- if count < buffer.count {
- break
- }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| var buffer = [UInt8](repeating: 0, count: 4096) | |
| var accumulator = "" | |
| let deadline = Date().addingTimeInterval(responseTimeout) | |
| while Date() < deadline { | |
| let count = Darwin.read(fd, &buffer, buffer.count) | |
| guard count > 0 else { break } | |
| if let chunk = String(bytes: buffer[0..<count], encoding: .utf8) { | |
| accumulator.append(chunk) | |
| if let newline = accumulator.firstIndex(of: "\n") { | |
| return String(accumulator[..<newline]) | |
| } | |
| if count < buffer.count { | |
| break | |
| } | |
| } | |
| } | |
| return accumulator.isEmpty ? nil : accumulator.trimmingCharacters(in: .whitespacesAndNewlines) | |
| var buffer = [UInt8](repeating: 0, count: 4096) | |
| var accumulator = "" | |
| let deadline = Date().addingTimeInterval(responseTimeout) | |
| while Date() < deadline { | |
| let count = Darwin.read(fd, &buffer, buffer.count) | |
| guard count > 0 else { break } | |
| if let chunk = String(bytes: buffer[0..<count], encoding: .utf8) { | |
| accumulator.append(chunk) | |
| if let newline = accumulator.firstIndex(of: "\n") { | |
| return String(accumulator[..<newline]) | |
| } | |
| } | |
| } | |
| return accumulator.isEmpty ? nil : accumulator.trimmingCharacters(in: .whitespacesAndNewlines) |
🤖 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 `@cmuxUITests/FocusHistoryShortcutUITests.swift` around lines 251 - 267, Remove
the count < buffer.count early-break condition from the response-reading loop in
the visible read logic. Continue accumulating chunks until a newline is found,
the deadline expires, or Darwin.read returns a non-positive count, preserving
the existing timeout and accumulator return behavior.
Comment on lines
+786
to
+796
|
|
||
| #if DEBUG | ||
| /// Test seam: unit tests can install the mirrored Ghostty | ||
| /// goto_split:previous/next triggers without loading a Ghostty config | ||
| /// (Ghostty's macOS defaults put them on ⌘[ / ⌘], colliding with the | ||
| /// focus-history defaults this dispatch must win). | ||
| func debugSetGhosttyGotoSplitCycleShortcuts(previous: StoredShortcut?, next: StoredShortcut?) { | ||
| ghosttyGotoSplitPreviousShortcut = previous | ||
| ghosttyGotoSplitNextShortcut = next | ||
| } | ||
| #endif |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Remove the test-only seam from the production source.
cmuxTests/FocusHistoryBracketShortcutRoutingTests.swift calls debugSetGhosttyGotoSplitCycleShortcuts, and the method exists only under #if DEBUG. This adds a test-only debug... member to Sources/AppDelegate.swift. Move the override to a dedicated debug support file or inject the Ghostty shortcut source through the test harness.
As per path instructions, **/Sources/**/*.swift must not add test-only or debug-only seams in production Swift source; isolate an unavoidable debug facility in a dedicated debug file or folder.
🤖 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 786 - 796, Remove
debugSetGhosttyGotoSplitCycleShortcuts from Sources/AppDelegate.swift and
relocate its test override behavior to a dedicated debug support file or
test-harness injection outside Sources/**/*.swift. Preserve the ability of
FocusHistoryBracketShortcutRoutingTests to configure the Ghostty previous/next
shortcuts without adding a test-only member to AppDelegate production source.
Source: Path instructions
Comment on lines
+32
to
+48
| def _selected_workspace(c: cmux) -> str: | ||
| for _idx, wsid, _title, selected in c.list_workspaces(): | ||
| if selected: | ||
| return wsid | ||
| raise cmuxError("no selected workspace") | ||
|
|
||
|
|
||
| def _press_and_wait(c: cmux, combo: str, expected_ws: str, timeout: float = 5.0) -> str: | ||
| c.simulate_shortcut(combo) | ||
| deadline = time.time() + timeout | ||
| current = _selected_workspace(c) | ||
| while time.time() < deadline: | ||
| current = _selected_workspace(c) | ||
| if current == expected_ws: | ||
| return current | ||
| time.sleep(0.1) | ||
| return current |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
A transient "no selection" moment during _press_and_wait crashes the test instead of retrying.
_selected_workspace raises cmuxError when no workspace is currently marked selected. _press_and_wait's poll loop calls it every iteration (line 44) while a shortcut-driven workspace switch is in flight. If the app reports no selection for even one poll (a plausible transient state during a switch), the raised exception propagates out of the loop and fails the test immediately, instead of letting the deadline-bounded retry handle it.
🔧 Proposed fix
def _press_and_wait(c: cmux, combo: str, expected_ws: str, timeout: float = 5.0) -> str:
c.simulate_shortcut(combo)
deadline = time.time() + timeout
- current = _selected_workspace(c)
+ current = ""
while time.time() < deadline:
- current = _selected_workspace(c)
+ try:
+ current = _selected_workspace(c)
+ except cmuxError:
+ time.sleep(0.05)
+ continue
if current == expected_ws:
return current
time.sleep(0.1)
return current📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def _selected_workspace(c: cmux) -> str: | |
| for _idx, wsid, _title, selected in c.list_workspaces(): | |
| if selected: | |
| return wsid | |
| raise cmuxError("no selected workspace") | |
| def _press_and_wait(c: cmux, combo: str, expected_ws: str, timeout: float = 5.0) -> str: | |
| c.simulate_shortcut(combo) | |
| deadline = time.time() + timeout | |
| current = _selected_workspace(c) | |
| while time.time() < deadline: | |
| current = _selected_workspace(c) | |
| if current == expected_ws: | |
| return current | |
| time.sleep(0.1) | |
| return current | |
| def _selected_workspace(c: cmux) -> str: | |
| for _idx, wsid, _title, selected in c.list_workspaces(): | |
| if selected: | |
| return wsid | |
| raise cmuxError("no selected workspace") | |
| def _press_and_wait(c: cmux, combo: str, expected_ws: str, timeout: float = 5.0) -> str: | |
| c.simulate_shortcut(combo) | |
| deadline = time.time() + timeout | |
| current = "" | |
| while time.time() < deadline: | |
| try: | |
| current = _selected_workspace(c) | |
| except cmuxError: | |
| time.sleep(0.05) | |
| continue | |
| if current == expected_ws: | |
| return current | |
| time.sleep(0.1) | |
| return current |
🧰 Tools
🪛 Ruff (0.16.0)
[warning] 36-36: Avoid specifying long messages outside the exception class
(TRY003)
🤖 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 `@tests_v2/test_focus_history_shortcut_cross_workspace.py` around lines 32 -
48, Update _press_and_wait to tolerate transient cmuxError exceptions from
_selected_workspace during shortcut polling: catch the error inside the
deadline-bounded loop, retain the previous/current workspace value when no
workspace is selected, and continue retrying until expected_ws is observed or
the timeout expires. Preserve the existing final return behavior.
Comment on lines
+55
to
+56
| c.activate_app() | ||
| time.sleep(0.3) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Replace fixed sleeps with polling for readiness, matching _press_and_wait's pattern.
Lines 56, 64, 69, and 86 use fixed time.sleep() calls to let activate_app, select_workspace, and close_workspace take effect, instead of polling for the real state change. _press_and_wait (lines 39-48) already shows the correct pattern: poll a real predicate on a deadline. Apply the same approach here: poll c.ping() or workspace state after activate_app, poll _selected_workspace(c) == wsid after each select_workspace, and poll list_workspaces() for the workspace's removal after close_workspace.
Based on path instructions: "Do not use fixed sleeps, measured wall-clock assertions, or hard absolute latency ceilings in correctness tests" and "await real completion signals or deadline-bounded polls of real predicates rather than fixed-duration waits."
Also applies to: 59-70, 85-86
🤖 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 `@tests_v2/test_focus_history_shortcut_cross_workspace.py` around lines 55 -
56, Replace the fixed time.sleep calls in the focus-history test with
deadline-bounded polling of real state, following _press_and_wait. After
activate_app, poll c.ping(); after each select_workspace, poll until
_selected_workspace(c) equals the target workspace; and after close_workspace,
poll list_workspaces() until the workspace is absent. Preserve the existing test
sequence and assertions.
Source: Path instructions
Comment on lines
+55
to
+86
| c.activate_app() | ||
| time.sleep(0.3) | ||
|
|
||
| created = [] | ||
| for index in range(3): | ||
| wsid = c.new_workspace() | ||
| c.select_workspace(wsid) | ||
| c.rename_workspace(f"fhist-ws{index + 1}", wsid) | ||
| created.append(wsid) | ||
| time.sleep(0.15) | ||
|
|
||
| # Visit ws1 -> ws2 -> ws3 so the focus-history stack is deterministic. | ||
| for wsid in created: | ||
| c.select_workspace(wsid) | ||
| time.sleep(0.15) | ||
| _must(_selected_workspace(c) == created[2], "expected focus on ws3 before navigating") | ||
|
|
||
| # Back across workspaces: ws3 -> ws2 -> ws1. | ||
| got = _press_and_wait(c, "cmd+[", created[1]) | ||
| _must(got == created[1], f"cmd+[ should land on ws2, got {got}") | ||
| got = _press_and_wait(c, "cmd+[", created[0]) | ||
| _must(got == created[0], f"second cmd+[ should land on ws1, got {got}") | ||
|
|
||
| # Forward again: ws1 -> ws2 -> ws3. | ||
| got = _press_and_wait(c, "cmd+]", created[1]) | ||
| _must(got == created[1], f"cmd+] should land on ws2, got {got}") | ||
| got = _press_and_wait(c, "cmd+]", created[2]) | ||
| _must(got == created[2], f"second cmd+] should land on ws3, got {got}") | ||
|
|
||
| # Closed workspaces are skipped, matching the arrow buttons' pruning. | ||
| c.close_workspace(created[1]) | ||
| time.sleep(0.3) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Fixed wall-clock sleeps used as synchronization in both regression suites. Both files use a fixed sleep to wait for a workspace-selection or app-state change to land, instead of polling the real state. The shared root cause is a "settle" delay standing in for a real completion signal, which the path instructions for these test directories explicitly forbid.
tests_v2/test_focus_history_shortcut_cross_workspace.py#L55-L86: replacetime.sleep(0.3)afteractivate_app(line 56),time.sleep(0.15)in the creation and visit loops (lines 64, 69), andtime.sleep(0.3)afterclose_workspace(line 86) with deadline-bounded polls of_selected_workspace(c)orlist_workspaces(), reusing the pattern already in_press_and_wait.cmuxUITests/FocusHistoryShortcutUITests.swift#L115-L123: replaceRunLoop.current.run(until: Date().addingTimeInterval(0.15))(line 122) with a call to the file's ownwaitForCurrentWorkspace(_:timeout:).
Based on path instructions: "Tests must await real completion signals or deadline-bounded polls of real predicates rather than fixed-duration waits before assertions" and "Do not use fixed sleeps, measured wall-clock assertions, or hard absolute latency ceilings in correctness tests."
📍 Affects 2 files
tests_v2/test_focus_history_shortcut_cross_workspace.py#L55-L86(this comment)cmuxUITests/FocusHistoryShortcutUITests.swift#L115-L123
🤖 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 `@tests_v2/test_focus_history_shortcut_cross_workspace.py` around lines 55 -
86, Replace fixed sleeps in
tests_v2/test_focus_history_shortcut_cross_workspace.py lines 55-86 after
activate_app, workspace creation/selection, and close_workspace with
deadline-bounded polls of _selected_workspace(c) or list_workspaces(), following
_press_and_wait; update cmuxUITests/FocusHistoryShortcutUITests.swift lines
115-123 to use waitForCurrentWorkspace(_:timeout:) instead of
RunLoop.current.run. Ensure all synchronization waits on real completion
predicates.
Source: Path instructions
Comment on lines
+315
to
+332
| { | ||
| id: "focusPreviousPane", | ||
| combos: [], | ||
| description: { en: "Focus previous pane (cycle)", ja: "前のペインにフォーカス（循環）" }, | ||
| note: { | ||
| en: "unbound by default; a Ghostty goto_split:previous keybind also cycles panes while Focus Back does not claim the same keys", | ||
| ja: "デフォルトでは未割り当て。Ghostty の goto_split:previous のキーバインドでも循環できます（Focus Back が同じキーを使っていない場合）", | ||
| }, | ||
| }, | ||
| { | ||
| id: "focusNextPane", | ||
| combos: [], | ||
| description: { en: "Focus next pane (cycle)", ja: "次のペインにフォーカス（循環）" }, | ||
| note: { | ||
| en: "unbound by default; a Ghostty goto_split:next keybind also cycles panes while Focus Forward does not claim the same keys", | ||
| ja: "デフォルトでは未割り当て。Ghostty の goto_split:next のキーバインドでも循環できます（Focus Forward が同じキーを使っていない場合）", | ||
| }, | ||
| }, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' "Configured web locales:"
rg -n -C 3 'locales|defaultLocale' web/i18n/routing.ts || true
printf '%s\n' "New shortcut metadata:"
rg -n -C 5 'focusPreviousPane|focusNextPane' web/data/cmux-shortcuts.ts web/messages || trueRepository: manaflow-ai/cmux
Length of output: 2732
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' "web/i18n/routing.ts:"
sed -n '1,120p' web/i18n/routing.ts
printf '%s\n' "Shortcut locale source/type validation:"
sed -n '1,80p' web/data/cmux-shortcuts.ts
printf '%s\n' rg -n 'description|note|supportedLocales|locales|type .*cmux|Shortcut|record' web -g '*.ts' -g '*.tsx' | head -n 220
printf '%s\n' "Message files:"
git ls-files web/messages | sed -n '1,120p'Repository: manaflow-ai/cmux
Length of output: 4705
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' "Shortcut metadata object shapes in web/data/cmux-shortcuts.ts (last 120 lines):"
sed -n '240,350p' web/data/cmux-shortcuts.ts
printf '%s\n' "All non-en/ja keys present in LocalizedText fields near new shortcuts:"
python3 - <<'PY'
import json, re
from pathlib import Path
src = Path("web/data/cmux-shortcuts.ts").read_text()
for name in ("focusPreviousPane","focusNextPane"):
idx = src.index(f"id: \"{name}\"")
slice_ = src[idx:src.index(" },", idx) + 5]
hits = re.findall(r'\b(?:description|note):\s*\{([^}]+)\}', slice_, re.S)
print(name)
for key in hits:
keys = sorted(re.findall(r'\b([A-z][A-z0-9-]*)\s*:', key))
print(" ", " ".join(keys))
PYRepository: manaflow-ai/cmux
Length of output: 8919
Add full locale coverage for the new shortcut text.
web/i18n/routing.ts supports additional locales beyond en and ja, including zh-CN, zh-TW, ko, de, es, fr, it, da, pl, ru, bs, ar, no, pt-BR, th, tr, km, and uk. Add description and note entries for these supported locales for focusPreviousPane and focusNextPane.
🤖 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 `@web/data/cmux-shortcuts.ts` around lines 315 - 332, Add complete locale
coverage to the focusPreviousPane and focusNextPane entries in the shortcut
definitions, adding description and note translations for zh-CN, zh-TW, ko, de,
es, fr, it, da, pl, ru, bs, ar, no, pt-BR, th, tr, km, and uk alongside en and
ja. Keep each locale’s existing translation structure and ensure both shortcuts
receive matching fields.
Source: Path instructions
…cus history Ghostty's macOS defaults bind goto_split:previous/next to cmd+[ / cmd+], the same keys as Focus Back/Forward. The shortcut dispatch mirrors those triggers to cycle pane focus and checks the mirror before the focus-history branch, so the keys cycle panes inside the current workspace (or do nothing) while the titlebar arrow buttons navigate across workspaces. Coverage added ahead of the fix so CI shows red then green: - cmuxTests/FocusHistoryBracketShortcutRoutingTests: dispatches real ⌘[ / ⌘] events through debugHandleCustomShortcut with the Ghostty mirror installed via a new DEBUG seam; expects workspace focus-history navigation. - cmuxUITests/FocusHistoryShortcutUITests: end-to-end over the control socket (simulate_shortcut uses the same matcher as the app-level monitor); walks back/forward across three workspaces and checks closed-workspace skipping. - tests_v2/test_focus_history_shortcut_cross_workspace.py: local socket verification against a tagged build. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ecomes rebindable The Ghostty goto_split:previous/next mirror in the shortcut dispatch now yields to a bound Focus Back/Forward shortcut (matchConfiguredShortcut, including shortcuts.when gating), so ⌘[ / ⌘] reach the focus-history branch and drive the exact same TabManager.navigateBack()/navigateForward() path as the titlebar arrow buttons: same history model, same closed-workspace pruning, same enable conditions. Unbinding Focus Back/Forward hands the keys back to the mirror, as the keyboard-shortcuts docs already promised. Pane cycling stays available two ways: the Ghostty goto_split trigger on any non-colliding key, and new cmux-owned rebindable actions focusPreviousPane / focusNextPane (default unbound) that share the same cyclePaneFocus body, per the shared-entrypoint policy. The window key-equivalent fallback route gets the same yield so both dispatch layers agree. The new actions follow the full shortcut policy: KeyboardShortcutSettings + CmuxSettings ShortcutAction (defaults, display names, panes group), Settings recorder rows, cmux.json shortcuts.bindings support, schema enum, web keyboard-shortcuts page (en+ja), and the shortcut-actions reference. Labels localized in Localizable.xcstrings for all catalog languages. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
azooz2003-bit
force-pushed
the
feat-cmd-bracket-workspace-history
branch
from
July 31, 2026 10:58
da9a0fe to
e73937d
Compare
Bugbot is paused — on-demand spend limit reachedBugbot 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. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@Resources/Localizable.xcstrings`:
- Around line 202980-203098: Add reviewed Khmer (km) stringUnit translations to
both shortcut.focusNextPane.label and shortcut.focusPreviousPane.label entries
in Resources/Localizable.xcstrings at 202980-203098 and 203575-203693,
respectively, ensuring both new shortcut labels cover every supported locale.
🪄 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: e72e84d0-0b12-40ad-a1c0-ec3a69add6f5
📒 Files selected for processing (14)
Packages/macOS/CmuxSettings/Sources/CmuxSettings/Values/ShortcutAction+Defaults.swiftPackages/macOS/CmuxSettings/Sources/CmuxSettings/Values/ShortcutAction+DisplayName.swiftPackages/macOS/CmuxSettings/Sources/CmuxSettings/Values/ShortcutAction+Group.swiftPackages/macOS/CmuxSettings/Sources/CmuxSettings/Values/ShortcutAction.swiftResources/Localizable.xcstringsSources/AppDelegate.swiftSources/KeyboardShortcutSettings.swiftcmux.xcodeproj/project.pbxprojcmuxTests/FocusHistoryBracketShortcutRoutingTests.swiftcmuxUITests/FocusHistoryShortcutUITests.swiftskills/cmux-settings/references/shortcut-actions.mdtests_v2/test_focus_history_shortcut_cross_workspace.pyweb/data/cmux-shortcuts.tsweb/data/cmux.schema.json
Comment on lines
+202980
to
+203098
| "shortcut.focusNextPane.label": { | ||
| "extractionState": "manual", | ||
| "localizations": { | ||
| "ar": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "التركيز على اللوحة التالية" | ||
| } | ||
| }, | ||
| "bs": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "Fokusiraj sljedeći panel" | ||
| } | ||
| }, | ||
| "da": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "Fokuser næste panel" | ||
| } | ||
| }, | ||
| "de": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "Nächsten Bereich fokussieren" | ||
| } | ||
| }, | ||
| "en": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "Focus Next Pane" | ||
| } | ||
| }, | ||
| "es": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "Enfocar panel siguiente" | ||
| } | ||
| }, | ||
| "fr": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "Activer le panneau suivant" | ||
| } | ||
| }, | ||
| "it": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "Sposta focus pannello successivo" | ||
| } | ||
| }, | ||
| "ja": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "次のペインにフォーカス" | ||
| } | ||
| }, | ||
| "ko": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "다음 패널로 포커스" | ||
| } | ||
| }, | ||
| "nb": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "Fokuser neste panel" | ||
| } | ||
| }, | ||
| "pl": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "Fokus na następny panel" | ||
| } | ||
| }, | ||
| "pt-BR": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "Focar Próximo Painel" | ||
| } | ||
| }, | ||
| "ru": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "Фокус на следующую панель" | ||
| } | ||
| }, | ||
| "th": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "โฟกัสบานหน้าต่างถัดไป" | ||
| } | ||
| }, | ||
| "tr": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "Sonraki Bölmeye Odaklan" | ||
| } | ||
| }, | ||
| "uk": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "Фокус на наступну панель" | ||
| } | ||
| }, | ||
| "zh-Hans": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "聚焦下一个面板" | ||
| } | ||
| }, | ||
| "zh-Hant": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "聚焦下一個面板" | ||
| } | ||
| } | ||
| } | ||
| }, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Add Khmer translations for both new shortcut labels.
Both entries omit the supported km locale. Add a reviewed Khmer translation to each entry.
Resources/Localizable.xcstrings#L202980-L203098: Addlocalizations.km.stringUnitforshortcut.focusNextPane.label.Resources/Localizable.xcstrings#L203575-L203693: Addlocalizations.km.stringUnitforshortcut.focusPreviousPane.label.
Based on learnings and path instructions, new catalog keys must cover every locale supported by Resources/Localizable.xcstrings, including km.
📍 Affects 1 file
Resources/Localizable.xcstrings#L202980-L203098(this comment)Resources/Localizable.xcstrings#L203575-L203693
🤖 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 `@Resources/Localizable.xcstrings` around lines 202980 - 203098, Add reviewed
Khmer (km) stringUnit translations to both shortcut.focusNextPane.label and
shortcut.focusPreviousPane.label entries in Resources/Localizable.xcstrings at
202980-203098 and 203575-203693, respectively, ensuring both new shortcut labels
cover every supported locale.
Sources: Path instructions, Learnings
Collaborator
Author
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
1 participant
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.
Fixes the default ⌘[ / ⌘] bindings never reaching focus history. Ghostty's macOS defaults bind
goto_split:previous/nextto the same keys, and the shortcut dispatch checked that mirror before the focus-history branch, consuming the key to cycle pane focus inside the current workspace (a no-op in single-pane workspaces). The titlebar arrows callTabManager.navigateBack()/navigateForward()directly, so the buttons crossed workspaces while the keys never did.The goto_split mirror now yields any event that matches a bound Focus Back/Forward shortcut (full
matchConfiguredShortcut, includingshortcuts.whenoverrides), in both the app-level monitor dispatch and the window key-equivalent fallback. ⌘[ / ⌘] therefore run the exact arrow-button path: same per-window history model, same closed-workspace pruning, same availability conditions.Default change: ⌘[ / ⌘] previously cycled panes through the Ghostty mirror; they now navigate global focus history, matching what the keyboard-shortcuts docs already claimed. The old behavior is preserved two ways: unbind Focus Back/Forward and the mirror gets the keys back, or bind the new rebindable actions
focusPreviousPane/focusNextPane(default unbound, samecyclePaneFocusbody). The new actions follow the full shortcut policy:KeyboardShortcutSettings+ShortcutAction(defaults, display names, panes group), Settings recorder rows,shortcuts.bindings.*in~/.config/cmux/cmux.json,cmux.schema.json, the web keyboard-shortcuts page (en+ja), andskills/cmux-settings/references/shortcut-actions.md. Labels are localized inResources/Localizable.xcstringsfor every catalog language.Regression coverage lands red then green across two commits:
FocusHistoryBracketShortcutRoutingTests(unit: real ⌘[ / ⌘] events throughdebugHandleCustomShortcutwith the mirror installed via a DEBUG seam),FocusHistoryShortcutUITests(e2e over the control socket;simulate_shortcutshares the monitor's matcher and dispatch order), andtests_v2/test_focus_history_shortcut_cross_workspace.pyfor tagged-build socket verification.Dictionary:
goto_splitis Ghostty's action for moving focus between terminal splits; themirroris cmux's app-level re-implementation of those Ghostty keybinds so they work across cmux panes;focus historyis the per-window stack behind the titlebar back/forward arrows; awhen clauseis the cmux.json predicate that scopes a shortcut to a focus context.🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by cubic
Cmd+[ / Cmd+] now navigate the global workspace focus history, matching the titlebar arrows. The Ghostty
goto_split:previous/nextmirror yields to Focus Back/Forward so these keys reach focus history; pane cycling is now rebindable.Bug Fixes
TabManager.navigateBack()/navigateForward()across workspaces and skip closed ones.shortcuts.when).Migration
focusPreviousPane/focusNextPane(default unbound).~/.config/cmux/cmux.jsonviashortcuts.bindings.focusPreviousPane/shortcuts.bindings.focusNextPane.Written for commit e73937d. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes
Tests