Repository navigation
Add remote surface listing commands - #4759
austinywang wants to merge 22 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (5)
💤 Files with no reviewable changes (3)
📝 WalkthroughWalkthroughAdds two CLI subcommands, ChangesList Surfaces and Terminals Commands
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 20 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (20 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 |
Greptile SummaryAdds
Confidence Score: 5/5Read-only CLI listing over an existing RPC with no state mutations; safe to merge. The change is purely additive and read-only: it calls the existing system.tree endpoint, flattens the result, and prints it. All validation fires before any socket connection. Localization is complete across every supported locale. The one issue found — a misleading error message when a global --window override triggers the --all conflict guard — does not affect correctness or safety. CLI/CMUXCLI+ListSurfaces.swift lines 107–113: the --all conflict guard labels the scoped option as --window even when only the global flag was set, which can produce a confusing error message for users. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["cmux list-surfaces / list-terminals\n(commandArgs, windowOverride)"] --> B["validateListSurfacesCommandSyntax\n(early, before socket)"]
B --> C{Parse --workspace\n--window --all --json}
C -- "missing handle value" --> E1["CLIError: requires handle"]
C -- "unknown flag / extra arg" --> E2["CLIError: unknown flag / unexpected arg"]
C -- "--all + scoped option" --> E3["CLIError: cannot combine --all with scope"]
C -- "no scope given" --> D["includeAllWindows = true (default)"]
C -- "scoped" --> D2["includeAllWindows = false"]
D & D2 --> F["buildListSurfacesPayload\n-> buildTreePayload (system.tree RPC)"]
F --> G["treePayloadWithMarkers\n(normalises active surface_ref -> surface.active)"]
G --> H["flattenTreeSurfaces\n(terminalOnly filter, active matching)"]
H --> I{--json?}
I -- yes --> J["print(jsonString)"]
I -- no --> K["renderListSurfacesText\n(active marker, context, tty, URL)"]
Reviews (14): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile |
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 `@CLI/cmux.swift`:
- Around line 12388-12421: The one-line usage synopses for the "list-surfaces"
and "list-terminals" cases (localized keys "cli.help.listSurfaces" and
"cli.help.listTerminals" in the switch handling those commands) are missing the
--json flag; update the triple-quoted defaultValue usage lines in those case
blocks to include " --json" in the Usage: synopsis so the short help matches the
detailed Flags section and accepts the --json option in the summary.
- Around line 14073-14085: The code builds a flattened row but never sets the
"active" key, yet later logic checks row["active"] to render the active marker;
update the flattening to populate that field from an existing tree-state field
(e.g., use pane["focused"] or surface["selected"] / surface["focused"] depending
on which upstream field represents the active surface) by calling
copyTreeListValue(into: &row, key: "active", from: pane["focused"] ??
surface["selected"] ?? surface["focused"]); ensure the "active" key is set on
the same row variable that is appended to result so the text renderer can mark
the active surface.
🪄 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: fdda1b97-1469-49a8-8f3f-a32283ef6139
📒 Files selected for processing (4)
CLI/cmux.swiftResources/Localizable.xcstringscmuxTests/CLINotifyProcessIntegrationRegressionTests.swiftdocs/cli-contract.md
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 8451793. Configure here.
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 `@CLI/CMUXCLI`+ListSurfaces.swift:
- Around line 46-59: The current check only looks for the literal flag in
rem0/rem1; instead validate the parsed option values returned by parseOption:
after let (workspaceOpt, rem0) = parseOption(...) throw the missing-handle
CLIError if workspaceOpt is nil OR workspaceOpt is an empty string OR
workspaceOpt starts(with: "-") (same for windowOpt/rem1). Update the checks for
workspaceOpt and windowOpt (not rem0/rem1.contains) to detect empty or flag-like
values and raise the existing CLIError with commandName and the flag name
("--workspace"/"--window").
🪄 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: 520c5fdf-d738-4ca1-856a-a8829413c942
📒 Files selected for processing (2)
CLI/CMUXCLI+ListSurfaces.swiftCLI/cmux.swift
💤 Files with no reviewable changes (1)
- CLI/cmux.swift
# Conflicts: # cmux.xcodeproj/project.pbxproj
Dismissed after CodeRabbit marked the scoped-handle validation thread addressed on head commit 19b6a93; fix is committed and thread has an acknowledgement reply.
# Conflicts: # .github/swift-file-length-budget.tsv

Summary
cmux list-surfacesandcmux list-terminals, defaulting to all windows and backed bysystem.treeScope
This is a self-contained first increment for #1037. It does not add
cmux attach <terminal-id>yet; that needs a separate PTY attach/streaming design for existing local terminal surfaces.Test plan
git diff --checkjq empty Resources/Localizable.xcstringsNeed help on this PR? Tag
@codesmithwith what you need. Autofix is disabled.Note
Low Risk
Read-only CLI listing over existing
system.tree; no app RPC or attach/streaming behavior changes.Overview
Adds
cmux list-surfacesandcmux list-terminalsso you can inspect surfaces from an SSH shell without a current window context. Both call existingsystem.treevia sharedbuildTreePayload, default to all windows when no--workspace/--windowis set, and flatten the tree into rows with window/workspace/pane handles, type, title, tty, browser URL, and anactivemarker from the tree’s global active surface.list-terminalsis the same pipeline withterminal_onlyfiltering. Output supports global or per-command--json, plus scoped flags and validation (missing handles, unknown flags,--allvs scoped options). Help,docs/cli-contract.md, localization, Xcode wiring, and mocked-socket integration tests are included.Reviewed by Cursor Bugbot for commit 19b6a93. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Adds SSH-friendly
cmux list-surfacesandcmux list-terminalsto inspect surfaces across windows, workspaces, and panes with text or JSON fromsystem.tree. This is the first step toward remote access in #1037; attach/streaming is not included.New Features
cmux list-surfacesandcmux list-terminalsdefault to all windows when unscoped; flags:--workspace,--window,--all,--json.countandterminal_only; localized help anddocs/cli-contract.mdupdated; internals reuse widenedbuildTreePayload/TreeCommandOptions.Bug Fixes
--windowoverride, and validate flags before any socket connection.--allconflicts.Written for commit 4a60613. Summary will update on new commits.
Summary by CodeRabbit
cmux list-surfacesandcmux list-terminalscommands with--workspace,--window,--all, terminal-only filtering, and--jsonor human-readable output.--workspacewith--all.--help/usage text, including new empty-state and error messages.