Add CLI window targeting - #4211
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds ChangesWindow flag support across CLI and backend
Sequence Diagram(s)sequenceDiagram
participant CLI as cmux CLI
participant Parser as CLI Parser
participant Normalizer as normalize*Handle
participant Terminal as TerminalController
participant Store as MainWindowSummaries
CLI->>Parser: parse --window <id|ref|index>
Parser->>Normalizer: compute windowHandle, normalize surface/workspace/pane handles
Normalizer->>Terminal: RPC call with params.window_id (if present)
Terminal->>Store: enumerate main-window summaries (filter by window_id)
alt window found
Store->>Terminal: return window summary
Terminal->>CLI: scoped response
else not found
Terminal->>CLI: not_found (window_id/window_ref)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Poem
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error, 2 warnings)
✅ Passed checks (13 passed)
✨ 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 |
Contributor
Greptile SummaryThis PR extends
Confidence Score: 5/5Safe to merge; all previously-flagged routing and error-ordering regressions are addressed and covered by new tests. The window-not-found check now fires before workspace-not-found in both system.tree and system.top. notify/clear-notifications/sidebar-metadata fallback paths now fail explicitly via requireCurrentWorkspaceId. The cross-window pane guard in v2ResolveWorkspace closes a silent misrouting path. normalizeWindowHandle validates refs against window.list. Test coverage is broad across socket regressions, CLI contract, and multi-window UI e2e. No files require special attention. Important Files Changed
Sequence DiagramsequenceDiagram
participant CLI as CLI (cmux.swift)
participant WN as normalizeWindowHandle
participant WS as normalizeWorkspaceHandle
participant SF as normalizeSurfaceHandle
participant TC as TerminalController (RPC)
CLI->>WN: --window ref/index/uuid
WN->>TC: window.list (if ref or index)
TC-->>WN: windows[]
WN-->>CLI: resolved window UUID
CLI->>WS: --workspace + windowHandle
WS->>TC: workspace.current(window_id) if allowCurrent
TC-->>WS: workspace_id
WS-->>CLI: resolved workspace UUID
CLI->>SF: --surface + workspaceHandle + windowHandle
SF->>TC: surface.list(workspace_id, window_id) if index
TC-->>SF: surfaces[]
SF-->>CLI: resolved surface UUID
CLI->>TC: RPC(window_id, workspace_id, surface_id)
TC->>TC: parseV2WindowRouting validates window_id + all_windows
TC->>TC: loop summaries, skip non-matching windows
TC->>TC: check windowFound before workspaceFound
TC-->>CLI: result or not_found(window/workspace)
Reviews (33): 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}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
lawrencecchen
force-pushed
the
feature-window-routing-cli
branch
from
May 15, 2026 14:06
c2c93a4 to
01001b6
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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/TerminalController.swift`:
- Around line 3230-3259: Both v2SystemTree and v2SystemTopBasePayload duplicate
the same window_id/all_windows parsing, conflict checks, identifyParams
augmentation and windowFound bookkeeping; extract this into a shared helper
(e.g., a new function like parseV2WindowRouting) that accepts params and
workspaceFilter and returns a struct/dictionary containing includeAllWindows,
requestedWindowId, identifyParams (including caller and window_id string),
windowFound initial value, and the resolved defaultWindowId logic; replace the
duplicated blocks in both handlers to call parseV2WindowRouting and use its
returned fields (refer to existing helpers v2Bool, v2UUID, v2Identify, v2UUIDAny
and variables identifyParams, windowFound, defaultWindowId) so routing behavior
is centralized and reused.
- Around line 3232-3236: Update the error responses that use .err when
validating window selectors (the branches checking params["window_id"],
requestedWindowId, and includeAllWindows) to follow the repo error-copy
contract: produce a user-facing cmux message that explains the problem in cmux
terms and gives 1–2 concrete next steps (e.g., "Use --window <id> to target a
specific window or --all-windows to target all windows; run `cmux windows list`
to see available windows and retry"), and put only the minimal diagnostics the
caller provided (include details: { "window_id": <value> } when the caller
supplied window_id; do NOT synthesize or return window_ref). Apply the same fix
to the other similar validation sites referenced (the branches around the checks
at the other ranges) so all window selector errors are consistent.
🪄 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: a434c646-314f-49b8-904e-5e9317519b33
📒 Files selected for processing (8)
CLI/CMUXCLI+MoveTabToNewWorkspace.swiftCLI/cmux.swiftSources/TerminalController.swiftcmuxTests/CLINotifyProcessIntegrationRegressionTests.swiftcmuxTests/CMUXOpenCommandTests.swiftcmuxTests/VMSSHCommandTests.swiftcmuxTests/WorkspaceRemoteConnectionTests.swiftcmuxUITests/MultiWindowNotificationsUITests.swift
lawrencecchen
force-pushed
the
feature-window-routing-cli
branch
from
May 15, 2026 14:15
01001b6 to
c4a1051
Compare
lawrencecchen
force-pushed
the
feature-window-routing-cli
branch
from
May 15, 2026 14:24
c4a1051 to
67e5d7f
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
Sources/TerminalController.swift (2)
3233-3236:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
window_iderror responses don't follow the cmux user-facing error contract.These
invalid_params/not_foundmessages ("Missing or invalid window_id", "window_id cannot be combined with all_windows", "Window not found") are still too terse for a user-facing cmux API error and give the caller no recovery path. Two issues to address:
- Add a concrete next action in cmux terms (e.g. "Pass either
--window <id|ref|index>or--all-windows; runcmux windows listto see available windows and retry.").- In the
not_foundbranch (Lines 3320-3329 and 3578-3587), the payload synthesizeswindow_refeven though the caller only suppliedwindow_id— keep the diagnostics scoped to the selector the caller actually provided to avoid leaking an unrelated identifier shape back at them.As per coding guidelines (
.github/review-bot-rules/user-facing-errors.md): user-facing API error bodies must describe the issue in cmux terms, provide concrete next actions, and keep diagnostics minimal/safe.Also applies to: 3320-3329, 3486-3489, 3578-3587
🤖 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/TerminalController.swift` around lines 3233 - 3236, Update the user-facing error returns for window selection to follow the cmux error contract: replace terse messages in the branches that check window_id and includeAllWindows (where code returns .err(... "Missing or invalid window_id" and "window_id cannot be combined with all_windows") with a descriptive cmux next-action message such as "Pass either --window <id|ref|index> or --all-windows; run `cmux windows list` to see available windows and retry." and keep the error code as "invalid_params"; and in the not_found branches (the branches that return .err(code: "not_found", ...)) stop synthesizing a window_ref when the caller only provided window_id — only include the selector key that the caller actually supplied in the error payload (e.g., include window_id diagnostics when window_id was provided, include window_ref only when window_ref was provided), and ensure the message includes the concrete next action in cmux terms as above; update uses around the window selection logic (the checks referencing includeAllWindows and requestedWindowId) so all user-facing error bodies follow this pattern.
3230-3267: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winDuplicated
window_idrouting block acrossv2SystemTreeandv2SystemTopBasePayload.Both endpoints now repeat the same
window_idparse,all_windowsconflict check,identifyParams["window_id"]augmentation,windowFoundbookkeeping, anddefaultWindowIdresolution. Extracting a shared helper (e.g.parseV2WindowRouting(params:)returning(includeAllWindows, requestedWindowId, identifyParams, initialWindowFound)) would prevent the two paths from drifting next time.Also applies to: 3481-3521
🤖 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/TerminalController.swift` around lines 3230 - 3267, The block that parses window routing (includeAllWindows, requestedWindowId, window_id validation, all_windows conflict check, building identifyParams, and initial windowFound/defaultWindowId logic) is duplicated between v2SystemTree and v2SystemTopBasePayload; extract a helper like parseV2WindowRouting(params:) that returns (includeAllWindows: Bool, requestedWindowId: UUID?, identifyParams: [String:Any], initialWindowFound: Bool, defaultWindowIdCandidate: UUID?) and replace the duplicated code in both handlers with a call to that helper; ensure the helper preserves the current validations (returning the same error conditions when window_id is present but invalid, and when both all_windows and window_id are supplied), includes caller and window_id into identifyParams the same way, computes focusedWindowId/defaultWindowId the same, and update v2SystemTree and v2SystemTopBasePayload to use the helper’s returned values (and to still compute identifyPayload/focused/caller/focusedWindowId from identifyParams as before).
🤖 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/MultiWindowNotificationsUITests.swift`:
- Around line 605-607: The teardown silently ignores the result of
closeWorkspaceViaSocket; change the call that currently discards the return
value (when createdWorkspaceId and window2Id are used) to capture and assert the
result (e.g., assert success with XCTAssertTrue or XCTFail on false) so
workspace cleanup failures surface in the test instead of being ignored.
---
Duplicate comments:
In `@Sources/TerminalController.swift`:
- Around line 3233-3236: Update the user-facing error returns for window
selection to follow the cmux error contract: replace terse messages in the
branches that check window_id and includeAllWindows (where code returns .err(...
"Missing or invalid window_id" and "window_id cannot be combined with
all_windows") with a descriptive cmux next-action message such as "Pass either
--window <id|ref|index> or --all-windows; run `cmux windows list` to see
available windows and retry." and keep the error code as "invalid_params"; and
in the not_found branches (the branches that return .err(code: "not_found",
...)) stop synthesizing a window_ref when the caller only provided window_id —
only include the selector key that the caller actually supplied in the error
payload (e.g., include window_id diagnostics when window_id was provided,
include window_ref only when window_ref was provided), and ensure the message
includes the concrete next action in cmux terms as above; update uses around the
window selection logic (the checks referencing includeAllWindows and
requestedWindowId) so all user-facing error bodies follow this pattern.
- Around line 3230-3267: The block that parses window routing
(includeAllWindows, requestedWindowId, window_id validation, all_windows
conflict check, building identifyParams, and initial windowFound/defaultWindowId
logic) is duplicated between v2SystemTree and v2SystemTopBasePayload; extract a
helper like parseV2WindowRouting(params:) that returns (includeAllWindows: Bool,
requestedWindowId: UUID?, identifyParams: [String:Any], initialWindowFound:
Bool, defaultWindowIdCandidate: UUID?) and replace the duplicated code in both
handlers with a call to that helper; ensure the helper preserves the current
validations (returning the same error conditions when window_id is present but
invalid, and when both all_windows and window_id are supplied), includes caller
and window_id into identifyParams the same way, computes
focusedWindowId/defaultWindowId the same, and update v2SystemTree and
v2SystemTopBasePayload to use the helper’s returned values (and to still compute
identifyPayload/focused/caller/focusedWindowId from identifyParams as before).
🪄 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: f07a551a-d79f-40e2-b64d-1961da4be96e
📒 Files selected for processing (8)
CLI/CMUXCLI+MoveTabToNewWorkspace.swiftCLI/cmux.swiftSources/TerminalController.swiftcmuxTests/CLINotifyProcessIntegrationRegressionTests.swiftcmuxTests/CMUXOpenCommandTests.swiftcmuxTests/VMSSHCommandTests.swiftcmuxTests/WorkspaceRemoteConnectionTests.swiftcmuxUITests/MultiWindowNotificationsUITests.swift
lawrencecchen
force-pushed
the
feature-window-routing-cli
branch
from
May 15, 2026 14:33
67e5d7f to
12728fd
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
Sources/TerminalController.swift (1)
3232-3236:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAlign
window_iderrors with the cmux user-facing error contract.These responses are still too terse, and the
not_foundpayload includes synthesizedwindow_ref. Please add 1–2 concrete recovery actions inmessageand keep diagnostics minimal to caller-provided fields (e.g.,window_idonly when supplied).Suggested shape
- return .err(code: "invalid_params", message: "Missing or invalid window_id", data: nil) + return .err( + code: "invalid_params", + message: "Invalid --window value. Use a window UUID, or run `cmux windows list` and retry.", + data: ["details": ["window_id": params["window_id"] as Any]] + ) - return .err(code: "invalid_params", message: "window_id cannot be combined with all_windows", data: nil) + return .err( + code: "invalid_params", + message: "Choose one scope: use `--window <id>` for one window, or `--all-windows` for all windows.", + data: ["details": ["window_id": requestedWindowId.uuidString]] + ) - message: "Window not found", - data: [ - "window_id": requestedWindowId.uuidString, - "window_ref": v2Ref(kind: .window, uuid: requestedWindowId) - ] + message: "No cmux window matched --window. Run `cmux windows list` and retry, or use `--all-windows`.", + data: ["details": ["window_id": requestedWindowId.uuidString]]As per coding guidelines "Error copy states what happened in cmux terms and gives 1–2 concrete next actions" and "
detailscontains only safe, minimal diagnostics."Also applies to: 3320-3327, 3485-3490, 3578-3585
🤖 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/TerminalController.swift` around lines 3232 - 3236, The error handlers for the window_id checks (the branch where params["window_id"] != nil && requestedWindowId == nil and the branch where includeAllWindows && requestedWindowId != nil) need to be changed to match the cmux user-facing error contract: replace terse messages with one-to-two concrete recovery actions (e.g., "Provide a valid window_id" or "Remove all_windows when targeting a specific window"), set the error code to the appropriate cmux code (e.g., "not_found" or "invalid_params" as applicable), and move any diagnostics into a minimal details payload that only echoes caller-provided fields (include window_id only if it was supplied). Update the same pattern in the other occurrences you noted (around the checks at the other ranges) so all window-related errors use the same actionable message and minimal details structure.
🤖 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/TerminalController.swift`:
- Around line 3232-3236: The error handlers for the window_id checks (the branch
where params["window_id"] != nil && requestedWindowId == nil and the branch
where includeAllWindows && requestedWindowId != nil) need to be changed to match
the cmux user-facing error contract: replace terse messages with one-to-two
concrete recovery actions (e.g., "Provide a valid window_id" or "Remove
all_windows when targeting a specific window"), set the error code to the
appropriate cmux code (e.g., "not_found" or "invalid_params" as applicable), and
move any diagnostics into a minimal details payload that only echoes
caller-provided fields (include window_id only if it was supplied). Update the
same pattern in the other occurrences you noted (around the checks at the other
ranges) so all window-related errors use the same actionable message and minimal
details structure.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0f0f91bf-859f-4466-ba7e-56205e491d1b
📒 Files selected for processing (8)
CLI/CMUXCLI+MoveTabToNewWorkspace.swiftCLI/cmux.swiftSources/TerminalController.swiftcmuxTests/CLINotifyProcessIntegrationRegressionTests.swiftcmuxTests/CMUXOpenCommandTests.swiftcmuxTests/VMSSHCommandTests.swiftcmuxTests/WorkspaceRemoteConnectionTests.swiftcmuxUITests/MultiWindowNotificationsUITests.swift
lawrencecchen
force-pushed
the
feature-window-routing-cli
branch
from
May 15, 2026 14:44
12728fd to
ea43696
Compare
lawrencecchen
force-pushed
the
feature-window-routing-cli
branch
from
May 15, 2026 14:55
ea43696 to
f3dd79e
Compare
lawrencecchen
force-pushed
the
feature-window-routing-cli
branch
from
May 15, 2026 15:08
f3dd79e to
b0f653f
Compare
lawrencecchen
force-pushed
the
feature-window-routing-cli
branch
from
May 15, 2026 15:17
b0f653f to
1137efb
Compare
lawrencecchen
force-pushed
the
feature-window-routing-cli
branch
from
May 15, 2026 15:26
1137efb to
4b2df9a
Compare
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 84418fd. Configure here.
lawrencecchen
dismissed stale reviews from coderabbitai[bot] and coderabbitai[bot]
May 19, 2026 05:33
Stale CodeRabbit change request. The actionable threads were addressed or resolved in later commits, and the latest CodeRabbit check is passing/skipped.
…g-cli # Conflicts: # CLI/cmux.swift # cmuxTests/CLINotifyProcessIntegrationRegressionTests.swift
…g-cli # Conflicts: # CLI/cmux.swift # cmuxTests/CLINotifyProcessIntegrationRegressionTests.swift
Merged
This was referenced May 23, 2026
This was referenced Jun 19, 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.
Summary
--windowrouting to window-scoped CLI commands for workspace, pane, surface, SSH, VM, notifications, tree, and top flows.new-workspace --window.Tests
git diff --checkcmux-unitcoverage: 13 selected tests passed locally.test-e2e.yml: https://github.com/manaflow-ai/cmux/actions/runs/25951782066/tmp/cloud-mac-window-routing-demo-2d5a1c1c0/recording.mov./scripts/reload.sh --tag winrtNote
Medium Risk
Moderate risk: changes CLI argument parsing and handle normalization to be window-scoped, which can affect which surfaces/workspaces are mutated in multi-window setups if routing logic is wrong.
Overview
Adds explicit
--windowtargeting to window-scoped CLI flows inCMUXCLI+MoveTabToNewWorkspace.swift, including updatingmove-tab-to-new-workspacehelp/usage.Updates
move-surface,split-off, andreorder-surfaceto parse--window, resolve workspace/surface/pane/before/after handles within the targeted window, and includewindow_id/window-scoped context in the socket params sent to methods likesurface.split_offandsurface.reorder.Reviewed by Cursor Bugbot for commit edf08ea. Bugbot is set up for automated code reviews on this repo. Configure here.