Repository navigation
Conversation
Adds a keyboard-shortcut action that toggles the pinned state of the focused workspace. The behavior already exists via the context menu and command palette (TabManager.togglePin); this exposes it as a first-class, user-bindable shortcut so pinning can be done from the keyboard. Ships unbound by default to avoid colliding with an existing default binding; users can assign any chord (e.g. Cmd+Shift+F) in Settings. - ShortcutAction: new case + navigation group + display label - ShortcutAction+Defaults: unbound default stroke - KeyboardShortcutSettings.Action: mirrored case, label, unbound default - AppDelegate: shortcut routing + handler (togglePin on focused workspace) - web: schema binding enum + shortcuts metadata entry Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T9GLqFQ3CfKcoTfVDqqd5w
|
Someone is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
|
To use Codex here, create a Codex account and connect to github. |
|
Important Review skippedWe couldn't safely recover the incremental review. No full review was started, and the last reviewed checkpoint was preserved. Retry later, or explicitly request a full review by commenting You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds the ChangesToggle pinned workspace shortcut
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant ShortcutEvent
participant AppDelegate
participant TabManager
ShortcutEvent->>AppDelegate: invoke togglePinnedWorkspace
AppDelegate->>TabManager: resolve focused window and selectedTabId
TabManager-->>AppDelegate: return selected tab or no target
AppDelegate->>TabManager: togglePin(tabId:)
AppDelegate-->>ShortcutEvent: consume event when handled
Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (23 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Greptile SummaryThis PR adds a bindable shortcut for pinning or unpinning the focused workspace. The main changes are:
Confidence Score: 4/5This is close, but the changed shortcut path still has issues to fix before merging.
Sources/AppDelegate.swift, web/data/cmux-shortcuts.ts Important Files Changed
Reviews (2): Last reviewed commit: "fix(shortcuts): guard stale pin toggle +..." | Re-trigger Greptile |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want higher recall? High effort reviews run extra passes and find more bugs. A team admin can switch effort levels in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit a0498f0. Configure here.
There was a problem hiding this comment.
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 14949-14951: Update the shortcut handler containing
tabManager.togglePin(tabId: focusedId) to verify that the selected tab still
exists and the pin mutation succeeds before consuming the shortcut; return false
when the ID is stale or no state change occurs, and true only after an actual
pin mutation.
- Around line 14942-14945: Remove the app-global self.tabManager fallback in
handleTogglePinnedWorkspaceShortcut. Resolve the TabManager exclusively from
contextForMainWindow(targetWindow), using the target window’s structured
context, and return false when that context or manager is unavailable so the
shortcut fails closed.
🪄 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
Run ID: 6cb86e02-040d-4ce8-8114-0375652315de
📒 Files selected for processing (6)
Packages/macOS/CmuxSettings/Sources/CmuxSettings/Values/ShortcutAction+Defaults.swiftPackages/macOS/CmuxSettings/Sources/CmuxSettings/Values/ShortcutAction.swiftSources/AppDelegate.swiftSources/KeyboardShortcutSettings.swiftweb/data/cmux-shortcuts.tsweb/data/cmux.schema.json
- handleTogglePinnedWorkspaceShortcut: only consume the chord when the selected id is a live workspace, matching the group-collapse handler, so a no-op togglePin no longer swallows the shortcut. - Add shortcut.togglePinnedWorkspace.label to Localizable.xcstrings across all 20 locales; it previously fell back to the English default value. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ya8UcURqMy5Rx984b8tJxv
There was a problem hiding this comment.
♻️ Duplicate comments (1)
Sources/AppDelegate.swift (1)
14943-14945:⚠️ Potential issue | 🟠 MajorDo not fall back to the app-global
self.tabManagerfor a missing window context.When the shortcut originates from a non-main window,
contextForMainWindow(targetWindow)can be nil, causing this fallback to toggle whichever workspace is selected inself.tabManagerrather than the workspace associated with the focused window. Resolve through the target window’s structured context and fail closed when unavailable. As per path instructions, correctness-critical shortcut handling and workspace state resolution must use a single authoritative source of truth and any degraded branches must fail closed (disable/no-op) rather than relying on an unreliable fallback branch.🐛 Proposed fix
- let targetWindow = preferredWindow ?? shortcutRoutingActiveWindow - let resolvedTabManager: TabManager? = contextForMainWindow(targetWindow)?.tabManager ?? self.tabManager - guard let tabManager = resolvedTabManager else { return false } + let targetWindow = preferredWindow ?? shortcutRoutingActiveWindow + guard let tabManager = contextForMainWindow(targetWindow)?.tabManager else { + return false + }🤖 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 14943 - 14945, Update the resolvedTabManager logic in the shortcut-handling path to remove the app-global self.tabManager fallback. Resolve the manager exclusively from targetWindow’s structured context, and return false when that context or its tabManager is unavailable so shortcut handling fails closed.Source: Path instructions
🤖 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.
Duplicate comments:
In `@Sources/AppDelegate.swift`:
- Around line 14943-14945: Update the resolvedTabManager logic in the
shortcut-handling path to remove the app-global self.tabManager fallback.
Resolve the manager exclusively from targetWindow’s structured context, and
return false when that context or its tabManager is unavailable so shortcut
handling fails closed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: e3c16f8b-a736-4903-b6d5-44f712eaf106
📒 Files selected for processing (2)
Resources/Localizable.xcstringsSources/AppDelegate.swift
|
@lawrencecchen — when you have a moment, would you be open to taking a look at this? No rush at all. For context, it mirrors your I've worked through the automated review feedback:
Happy to adjust anything if you'd prefer a different approach. Thanks for building cmux! |
|
This remains wanted, and current main needs a small shortcut-file refresh. Before we re-land your change, please comment exactly |
|
All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document v2.2 and I hereby sign the CLA |
|
recheck |
|
Taking this: refreshing the unbound Pin/Unpin Workspace shortcut for current main and checking all binding entrypoints.
|
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> # Conflicts: # Packages/macOS/CmuxSettings/Sources/CmuxSettings/Values/ShortcutAction.swift # web/data/cmux-shortcuts.ts
Regenerate the embedded config schema for the new action and reuse the contributor translations in all web locales. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Thanks @oyhoyhk, I updated this with main and kept the new action in the current split shortcut files :) I also regenerated the native config schema and reused your catalog translations for all 20 web locales. The shortcut stays unbound by default and uses the existing pin action. Independent review, schema/catalog checks, web lint, and test wiring passed. Native CI and shortcut dogfood are still pending.
|
CI failure attributionCI failed on
Matched log linesNot re-run automatically: Written by |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
I fixed the shortcut-action reference omission caught by CI.
|

▎ 🤖 Authored with AI assistance. This change was implemented with Claude (Anthropic) via Claude Code. A human author reviewed it and is responsible for the submission. Commits carry a Co-Authored-By: Claude trailer.
Summary
What changed? Adds a bindable keyboard-shortcut action, Pin or Unpin Focused Workspace, that toggles the pinned state of the currently focused workspace.
Why? The behavior already exists via the workspace context menu and the command palette (TabManager.togglePin), but there was no way to pin/unpin from the keyboard. This exposes it as a first-class, user-customizable shortcut action.
Design notes:
Files changed:
Testing
Demo Video
For UI or behavior changes, include a short demo video (GitHub upload, Loom, or other direct link).
Review Trigger (Copy/Paste as PR comment)
@codex review
@coderabbitai review
@greptile-apps review
@cubic-dev-ai review
Checklist
🤖 Generated with Claude Code
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Note
Low Risk
Shortcut plumbing only; no new pin logic beyond calling existing
togglePin, ships unbound so default behavior is unchanged.Overview
Adds a new customizable shortcut action Pin or Unpin Focused Workspace so users can toggle pin state from the keyboard; it reuses existing
TabManager.togglePinbehavior already available from the context menu and command palette.The action is wired end-to-end like other focused-workspace shortcuts: new
togglePinnedWorkspacecases in CmuxSettings andKeyboardShortcutSettings, unbound by default (to avoid default-key collisions such as ⌘⇧F), andAppDelegaterouting viahandleTogglePinnedWorkspaceShortcutthat only consumes the key when a focused workspace exists—otherwise the chord falls through to other bindings.Web shortcut metadata and
cmux.schema.jsonare updated so the action appears in docs and config validation.Reviewed by Cursor Bugbot for commit a0498f0. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Adds a bindable keyboard shortcut to pin or unpin the focused workspace, reusing the existing
TabManager.togglePinbehavior.togglePinnedWorkspaceaction, routed inAppDelegatethrough the main container to toggle the focused workspace.Written for commit 89b7647. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes