Repository navigation
Add right-sidebar CLI command parity - #3810
Merged
Merged
Conversation
The CLI had no tests proving keyboard-only right-sidebar actions were reachable through the socket surface, so add regression coverage before wiring the implementation. Constraint: Do not run local tests for this repo; CI owns verification. Confidence: medium Scope-risk: narrow Tested: Not run locally per repository policy. Not-tested: Expected to fail before the right_sidebar command family exists.
Keyboard shortcuts could already drive every right-sidebar action, but automation had no matching socket command family. Add a single right_sidebar IPC path, one app-level dispatcher over the existing sidebar handlers, and one CLI branch that forwards the full quiet command surface with optional targeting. Constraint: Preserve the hand-rolled CLI and V1 socket style already used by sidebar commands. Constraint: Do not run local tests or direct xcodebuild in this repo; CI and tagged reload own verification. Rejected: Add ArgumentParser for this command family | inconsistent with the existing CLI dispatcher. Confidence: medium Scope-risk: moderate Directive: Keep future right-sidebar CLI actions routed through RightSidebarRemoteCommand instead of adding one-off socket branches. Tested: python3 -m json.tool Resources/Localizable.xcstrings Tested: git diff --check Not-tested: Local unit/UI tests and local xcodebuild, per repository and task policy.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds a complete cmux right-sidebar CLI and socket remote-control surface: domain types and parser, AppDelegate apply handler and context resolution, TerminalController socket wiring and focus-policy checks, CLI dispatcher with argument parsing and handle resolution, Xcode/localization wiring, a minor UI declaration change, tests, and docs. ChangesRight Sidebar CLI and Remote Control
🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 14 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (14 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
The PR branch needs the latest mainline changes before CI can provide useful signal, so merge origin/main after opening the PR and preserve the existing right-sidebar commits on top of that sync. Constraint: origin/main advanced substantially after the branch point. Constraint: Do not run local tests or direct xcodebuild in this repo; CI owns executable verification. Confidence: medium Scope-risk: broad Directive: Treat this merge as baseline synchronization; behavioral intent remains in the two right-sidebar commits below it. Tested: git status --short after submodule checkout update. Not-tested: Local unit/UI tests and local xcodebuild, per repository and task policy.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
Greptile SummaryThis PR adds a
Confidence Score: 5/5Safe to merge. All changed paths are well-bounded: new IPC types are nonisolated Sendable value types, visibility-increasing paths route through existing window-activation helpers, and targeted commands fail cleanly on stale refs rather than mutating state silently. The three focus/visibility correctness issues surfaced in prior review rounds have each been explicitly closed out in earlier commits. The new applyRightSidebarRemoteCommand dispatcher is consistent across all six command variants, the focus allowance policy is parsed per-command before the V1 body executes, and the CLI validates subcommands before attempting handle resolution. Integration tests exercise the full CLI→socket→app round-trip including ref/index resolution, quiet output, and stale-target failure paths. No files require special attention. Important Files Changed
Reviews (9): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Remote review caught two edge-path divergences: invalid subcommands could trigger target resolution first, and show used a different hidden-to-visible path than toggle. Validate the subcommand before socket lookups and route show through the existing toggle handler when it actually reveals the sidebar. Constraint: Keep the CLI quiet and preserve the existing V1 forwarding style. Constraint: Do not run local tests or direct xcodebuild in this repo; CI owns executable verification. Rejected: Add a separate show-specific app handler | it would duplicate the toggle path the feature is meant to mirror. Confidence: medium Scope-risk: narrow Directive: Keep target resolution after subcommand validation so invalid commands report command errors before socket lookup errors. Tested: python3 -m json.tool Resources/Localizable.xcstrings Tested: git diff --check Not-tested: Local unit/UI tests and local xcodebuild, per repository and task policy.
The previous follow-up still left a stale-target fallback that made show visible by mutating FileExplorerState directly. Fail stale targeted show requests instead, and keep every hidden-to-visible show path on the existing toggle handler. Constraint: Preserve keyboard-shortcut parity for the visible-sidebar transition. Rejected: Keep a stale-context direct mutation fallback | it preserves the split behavior review flagged. Confidence: medium Scope-risk: narrow Directive: Do not add direct show-only visibility mutation unless there is a documented target-without-window use case. Tested: git diff --check Not-tested: Local unit/UI tests and local xcodebuild, per repository and task policy.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
The right-sidebar CLI was a small command surface, but it crosses CLI parsing, socket forwarding, AppDelegate targeting, and persisted sidebar state. Add behavior-level coverage for handle resolution, forwarding failure, parser validation, target scoping, and the no-socket help contract while keeping the IPC parser in its own file. Constraint: Local test execution is routed through CI/VM by repo policy; tagged reload build is queued behind an existing shared Xcode build lock. Rejected: Source-text or project-grep tests | repo test policy requires observable runtime behavior instead. Confidence: medium Scope-risk: narrow Directive: Keep right-sidebar CLI tests at the socket/behavior boundary so future parser rewrites preserve command contracts. Tested: git diff --check; plutil -lint GhosttyTabs.xcodeproj/project.pbxproj; existing PR CI was green before this commit Not-tested: Local unit tests and local tagged build completion; ./scripts/reload.sh --tag issue-3808-right-sidebar-cli remains queued behind another Xcode build
The right-sidebar socket parser now has its protocol types in a dedicated file, and CI review asked that these pure values follow the repo's explicit concurrency boundary convention. Marking the request, target, state, result, and mode values as nonisolated/Sendable keeps the IPC surface safe to pass across the socket-to-main-actor boundary without relying on implicit defaults. Constraint: Local tests run in CI for this repo; keep local verification to static checks and tagged builds. Rejected: Leave Sendable implicit | this would diverge from the existing SocketLineProcessingResult pattern. Confidence: medium Scope-risk: narrow Directive: Keep right-sidebar IPC values pure and explicitly sendable if the command family grows. Tested: git diff --check Not-tested: Local unit tests, per repository testing policy
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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/TerminalControllerSocketSecurityTests.swift`:
- Around line 311-340: The test creates a new AppDelegate instance but doesn't
restore the global singleton, which can leak state; in
testRightSidebarRemoteCommandsCanTargetRegisteredWindowOrWorkspaceWithoutFocus
capture the current AppDelegate.shared before creating AppDelegate(), set the
new instance for the test, and ensure you restore the original
AppDelegate.shared in a defer block (alongside the existing
unregisterMainWindowContextForTesting calls) so the singleton is returned to its
prior value after the test; update references around
registerMainWindowContextForTesting/unregisterMainWindowContextForTesting to use
the temporary shared value during the test.
In `@Resources/Localizable.xcstrings`:
- Around line 111074-111093: Usage strings for rightSidebar remote commands
reference only --tab even though RightSidebarRemoteCommand accepts --workspace
as an alias; update the localized values for the keys
rightSidebar.remote.error.usage, rightSidebar.remote.error.usage.toggle,
rightSidebar.remote.error.usage.show, rightSidebar.remote.error.usage.hide,
rightSidebar.remote.error.usage.focus, rightSidebar.remote.error.usage.mode, and
rightSidebar.remote.error.usage.set to show both flag names (e.g.
"[--workspace/--tab=<workspace-id>]") or replace --tab with --workspace for
consistency with the CLI parsing in RightSidebarRemoteCommand (the alias
handling around the --workspace/--tab option).
In `@Sources/AppDelegate.swift`:
- Around line 5530-5536: The current guard uses context?.fileExplorerState ??
fileExplorerState which lets explicit non-active targets fall back to the global
active-window state; instead, ensure non-active targets cannot use that
fallback: in the rightSidebarRemoteContext/handling logic, if
target.isActiveTarget is true allow the fallback to fileExplorerState, but if
target.isActiveTarget is false require context?.fileExplorerState to be non-nil
and return the "stateUnavailable" failure otherwise; update the guard around
state (the state variable and its resolution) to implement that conditional
behavior.
In `@Sources/RightSidebarRemoteCommand.swift`:
- Around line 3-164: This file contains lifecycle-independent models and parsing
logic (RightSidebarRemoteTarget, RightSidebarRemoteCommand,
RightSidebarRemoteRequest, RightSidebarRemoteState, RightSidebarRemoteParseError
and parseTargetOption/parse(tokens:)) and should be moved out of the app root
into the shared/core module: move the file to the shared/core target, update the
Swift package target or Xcode target membership to that core module, remove any
app-only imports if present, and ensure any symbols that need cross-target
visibility have appropriate access levels (make types/functions internal/public
as needed). After moving, run build/tests and update any import paths where
RightSidebarRemoteRequest.parse(...) and related types are referenced from the
app target so they import the new core module.
- Around line 98-129: The usage/error messages in RightSidebarRemoteCommand (the
localized strings like "rightSidebar.remote.error.usage",
"rightSidebar.remote.error.usage.toggle",
"rightSidebar.remote.error.usage.show", "rightSidebar.remote.error.usage.hide",
"rightSidebar.remote.error.usage.focus", "rightSidebar.remote.error.usage.mode",
and "rightSidebar.remote.error.usage.set") list only "--tab=<workspace-id>"
while the CLI parser also accepts "--workspace"; update each defaultValue usage
string to include "--workspace=<workspace-id>" (e.g., "[--tab=<workspace-id> |
--workspace=<workspace-id>]" or include both flags) so the printed usage matches
the accepted flags for actions toggle, show, hide, focus, mode/state, set, and
the general usage message.
🪄 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: f28dd790-d1ca-494e-81de-9efe1c11325e
📒 Files selected for processing (11)
CLI/cmux.swiftGhosttyTabs.xcodeproj/project.pbxprojResources/Localizable.xcstringsSources/AppDelegate.swiftSources/RightSidebarMode+Availability.swiftSources/RightSidebarRemoteCommand.swiftSources/TerminalController.swiftcmuxTests/CLINotifyProcessIntegrationRegressionTests.swiftcmuxTests/FileExplorerStateModePersistenceTests.swiftcmuxTests/TerminalControllerSocketSecurityTests.swiftdocs/cli-contract.md
The right-sidebar CLI needs UUIDs for the remote command, but it does not need its own copy of window/workspace index resolution. Compose the existing handle normalizers with one small UUID extraction helper so right-sidebar targeting follows the same handle rules as the rest of the CLI while still forwarding concrete UUIDs over the V1 socket command. Constraint: right_sidebar remote parser only accepts UUID target IDs, while public CLI flags accept UUIDs, refs, and indexes. Rejected: Keep bespoke right-sidebar index helpers | duplicates the established CLI normalization path and drew review feedback. Confidence: medium Scope-risk: narrow Directive: Add future target syntaxes to the shared normalizers first, then compose them here. Tested: git diff --check Not-tested: Local unit tests, per repository testing policy
Review found two edge cases in the right-sidebar CLI/socket path: invalid set modes could still proceed to target resolution, and explicit remote targets could fall back to active sidebar state when their registered context lacked one. Validate set modes before resolution, require explicit targets to own sidebar state, and keep usage strings aligned with the public --workspace flag. Constraint: right_sidebar IPC still accepts --tab as an internal alias, but user-facing usage should prefer --workspace. Rejected: Move the parser to a core module in this PR | the thread is already outdated and the type still depends on app-target RightSidebarMode; doing that cleanly would require a broader module-boundary change. Confidence: medium Scope-risk: narrow Directive: Keep validation before socket/target work for every right-sidebar CLI branch. Tested: git diff --check; jq empty Resources/Localizable.xcstrings Not-tested: Local unit tests, per repository testing policy
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 `@Sources/AppDelegate.swift`:
- Around line 5573-5583: The .show branch is rejecting targets that already own
fileExplorerState but lack a live NSWindow; replace the strict guard that checks
target.isActiveTarget || preferredWindow != nil with a check that allows a
resolved file-explorer state (e.g., target.fileExplorerState != nil) or a
preferredWindow; in other words, in the case .show block update the second guard
to permit targets that have an existing fileExplorerState (or equivalent
resolved state) even when isActiveTarget is false and preferredWindow is nil,
then let toggleRightSidebarInActiveMainWindow(preferredWindow:) run as before
and return failures only if neither a resolved fileExplorerState nor a
preferredWindow exists.
🪄 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: c7bc8a86-3b09-4928-921c-8f6e6f96b1e6
📒 Files selected for processing (7)
CLI/cmux.swiftResources/Localizable.xcstringsSources/AppDelegate.swiftSources/RightSidebarPanelView.swiftSources/RightSidebarRemoteCommand.swiftcmuxTests/CLINotifyProcessIntegrationRegressionTests.swiftcmuxTests/TerminalControllerSocketSecurityTests.swift
A targeted remote toggle should not bypass the same window-selection path used by show/focus when the target sidebar is hidden. Return target-not-found when a non-active target has no concrete window instead of mutating FileExplorerState directly, and lock the no-window workspace case with a regression assertion. Constraint: Right-sidebar visibility commands must preserve the socket focus policy and avoid silently acting on a different window. Rejected: Keep direct toggle mutation for no-window targets | it recreates the split-path behavior already rejected for show. Confidence: medium Scope-risk: narrow Directive: Visibility commands that can reveal UI should go through the shared window-aware path for explicit targets. Tested: git diff --check; jq empty Resources/Localizable.xcstrings Not-tested: Local unit tests, per repository testing policy
The V1 socket policy was still treating every right_sidebar command as focus-capable, even though mode, hide, invalid requests, and set --no-focus should preserve focus. Parse the right_sidebar request before installing the socket policy so only show, toggle, focus, and focused mode switches can mutate in-app focus, and add debug-level coverage for the command matrix. Constraint: V1 socket policy normally only sees the command key, so right_sidebar passes its raw args into the policy params for granular classification. Rejected: Keep right_sidebar in the broad focus-intent set | it violates the socket focus policy for state reads and no-focus writes. Confidence: medium Scope-risk: narrow Directive: New right_sidebar subcommands must update the command-specific focus policy test. Tested: git diff --check; jq empty Resources/Localizable.xcstrings Not-tested: Local unit tests, per repository testing policy
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit a1ce268. Configure here.
lawrencecchen
dismissed stale reviews from coderabbitai[bot] and coderabbitai[bot]
May 12, 2026 04:31
Superseded by later commits and a later CodeRabbit approval; the referenced inline threads are resolved or outdated.
This was referenced May 21, 2026
This branch was successfully 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
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.
Closes #3808
Design notes
right_sidebar, parsed intoRightSidebarRemoteCommandbefore dispatch.FileExplorerStateinstead of duplicating keyboard shortcut behavior.cmux right-sidebar, with quiet default output and JSON output only formode.RightSidebarMode.from(cliArgument:)helper;vaultandsessionsboth map to.sessions.--workspace/--windowtargeting by resolving refs or indexes to UUIDs before forwarding.set --no-focusswitches modes without requesting sidebar focus.CLI surface
cmux right-sidebar modeprints JSON, for example:{"visible":true,"mode":"find"}Test plan output
python3 -m json.tool Resources/Localizable.xcstringspassed.git diff --checkpassed.99e0d0c5cd7cadac591776c7bbb12471931622deare green: CircleCI macOS debug build, release build, unit tests, GitHub remote-daemon-tests, workflow-guard-tests, web typecheck, web DB migrations, Cursor Bugbot, Greptile, CodeRabbit, Socket Security, Vercel deployments.Manual verification output
Tagged build launched with:
The tagged dev app has isolated defaults, so I enabled the existing Feed/Dock beta flags for
com.cmuxterm.app.debug.issue.3808.right.sidebar.clibefore verifying those two keyboard-reachable modes.Manual CLI dogfood used the tagged CLI and socket:
Observed results:
right-sidebar hideexited 0;modereturned{"mode":"feed","visible":false}.right-sidebar showexited 0;modereturned visible true.right-sidebar toggleflipped false, then true on the second toggle.right-sidebar set findexited 0;modereturnedfind.right-sidebar set vaultandright-sidebar set sessionsboth exited 0;modereturnedsessions.right-sidebar focusexited 0.right-sidebar files,find,vault,feed, anddockall exited 0 and landed onfiles,find,sessions,feed, anddockrespectively.right-sidebar set find --no-focusexited 0 and left state at{"mode":"find","visible":true}.right-sidebar mode --window E8D4A759-9725-400F-9A9C-8A15FF38D561exited 0.right-sidebar set files --workspace 83E4A1DD-277C-4369-8B33-85CDCD16BC31 --no-focusexited 0; targetedmodereturnedfiles.Note
Medium Risk
Adds a new V1 IPC command (
right_sidebar) that can mutate UI focus and sidebar state, plus handle-resolution logic that depends on window/workspace listing; mistakes could affect targeting or focus behavior but changes are well-scoped and covered by new tests.Overview
Adds
cmux right-sidebarCLI support (toggle/show/hide/focus/set/mode plus mode aliases) that forwards a new V1 socket command familyright_sidebar, stays quiet on success, and prints JSON only formode.Implements a shared
RightSidebarRemoteRequestparser +RightSidebarRemoteCommandmodel, wiresTerminalControllerto dispatchright_sidebar(including a command-specific focus-mutation policy), and addsAppDelegate.applyRightSidebarRemoteCommandto apply remote commands to the existing right-sidebarFileExplorerState, including optional targeting by--window/--workspace.Updates localized usage/error strings, CLI contract docs, Xcode project sources, and adds regression/security tests covering CLI validation, target resolution, command parsing, and state application.
Reviewed by Cursor Bugbot for commit 6c66304. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
New Features
cmux right-sidebarCLI (toggle/show/hide/focus/set + mode aliases) with--workspace,--window, and--no-focus; forwards to the app and returns JSON for mode queries.Bug Fixes / Reliability
Documentation
Localization
Tests