Add unified surface selection context API - #10022
Open
austinywang wants to merge 17 commits into
Open
Conversation
|
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 Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe PR implements ChangesSurface selection implementation
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant SocketWorker
participant TerminalController
participant Panel
CLI->>SocketWorker: read-selection selectors
SocketWorker->>TerminalController: surface.read_selection
TerminalController->>Panel: readSurfaceSelection()
Panel-->>TerminalController: SurfaceSelectionReadResult
TerminalController-->>SocketWorker: selection payload
SocketWorker-->>CLI: plain or JSON output
Possibly related issues
Possibly related PRs
Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (3 errors, 2 warnings)
✅ Passed checks (20 passed)
✨ Finishing Touches 💡 1📝 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 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`+SurfaceSelection.swift:
- Around line 74-82: Update the selection handling in the relevant CLI
read-selection flow so a valid has_selection: false response returns success
instead of throwing CLIError. Define a plain-output representation for the
no-selection case that clearly distinguishes it from selection text, while
preserving existing behavior when a selection is present.
In `@cmuxTests/CLIExplicitSurfaceRoutingTests.swift`:
- Around line 136-144: Expand the assertions in the CLI JSON response test
around jsonObject so they validate every field in the no-selection payload,
including text, base64, workspace_id, workspace_ref, and surface_id alongside
the existing fields. Ensure the expected values match the complete socket
response returned by the explicit surface routing command.
In `@cmuxTests/SurfaceSelectionTests.swift`:
- Around line 81-82: Remove the fixed .timeLimit(.minutes(1)) attribute from
browserAndMarkdownPreviewReadLiveDOMSelection, leaving the correctness test’s
existing navigation-completion synchronization unchanged.
- Around line 100-101: Add positive selection coverage in
SurfaceSelectionTests.swift alongside the existing password-input cases: create
a non-password text input or textarea with selected content, then assert
hasSelection is true and the selected text matches the expected value. Keep the
existing password-selection assertions unchanged.
In `@Resources/Localizable.xcstrings`:
- Around line 102-106: Update the socket selection error localizations around
socket.surfaceSelection.tabManagerUnavailable to use stable product-facing
wording instead of internal identifiers such as TabManager or
panel.panelType.rawValue. Use wording equivalent to “Selection reading is
currently unavailable” and “This surface does not support selection reads,”
while retaining raw values only in structured diagnostics or internal logs.
In `@Sources/Panels/SurfaceSelectionSnapshot.swift`:
- Around line 12-26: Make SurfaceSelectionSnapshot construction enforce valid
selected and no-selection states rather than accepting an unrestricted
hasSelection Boolean; update Sources/Panels/SurfaceSelectionSnapshot.swift lines
12-26 accordingly. In SurfaceSelectionLineRange’s initializer at
Sources/Panels/SurfaceSelectionLineRange.swift lines 8-11, require start >= 1
and end >= start, returning or propagating an invalid conversion result so
callers handle failure before creating a snapshot.
In `@Sources/TerminalController`+SurfaceSelection.swift:
- Around line 74-75: Update the surface-selection routing around v2UUID and the
target-resolution logic near lines 157-160 to distinguish an absent pane_id from
a present but unparseable one. When pane_id is supplied and v2UUID returns nil,
return not_found or the established invalid-selector error instead of falling
back to the focused surface; preserve normal resolution for valid or omitted
pane_id values.
🪄 Autofix
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: 08a13cfd-26ea-4e56-a60d-ad1c5a8433d5
📒 Files selected for processing (27)
CLI/CMUXCLI+CommandSuggestions.swiftCLI/CMUXCLI+SurfaceSelection.swiftCLI/cmux.swiftPackages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Wire/ControlCommandExecutionPolicy.swiftResources/Localizable.xcstringsSources/Panels/BrowserPanel.swiftSources/Panels/FilePreviewPanel.swiftSources/Panels/MarkdownPanel.swiftSources/Panels/MarkdownWebRenderer.swiftSources/Panels/MarkdownWebSupport.swiftSources/Panels/NativeTextSurfaceSelectionReader.swiftSources/Panels/Panel.swiftSources/Panels/SurfaceSelectionLineRange.swiftSources/Panels/SurfaceSelectionReadResult.swiftSources/Panels/SurfaceSelectionSnapshot.swiftSources/Panels/TerminalPanel.swiftSources/Panels/WebSurfaceSelectionReader.swiftSources/SurfaceSelectionSocketCapture.swiftSources/SurfaceSelectionSocketCaptureOutcome.swiftSources/SurfaceSelectionSocketFailure.swiftSources/TerminalController+SurfaceSelection.swiftSources/TerminalController.swiftcmux.xcodeproj/project.pbxprojcmuxTests/CLIExplicitSurfaceRoutingTests.swiftcmuxTests/SurfaceSelectionTests.swiftcmuxTests/TerminalControllerSocketSecurityTests.swiftdocs/cli-contract.md
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/cli-contract.md (1)
243-243: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse a UUID for
surface_idin the raw RPC example.
surface.read_selectionvalidatessurface_idas a UUID.surface:2returnsinvalid_params, so this documented command fails. Use a UUID value in the JSON parameters.🤖 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 `@docs/cli-contract.md` at line 243, Update the surface.read_selection raw RPC example to use a valid UUID for the surface_id JSON parameter instead of surface:2, while preserving the command structure and parameter name.
🤖 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.
Outside diff comments:
In `@docs/cli-contract.md`:
- Line 243: Update the surface.read_selection raw RPC example to use a valid
UUID for the surface_id JSON parameter instead of surface:2, while preserving
the command structure and parameter name.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 35c5cc92-eaf1-4c39-8bb8-3a463dc939ef
📒 Files selected for processing (14)
CLI/CMUXCLI+SurfaceSelection.swiftResources/Localizable.xcstringsSources/Panels/Panel.swiftSources/Panels/SurfaceSelectionLineRange.swiftSources/Panels/SurfaceSelectionReadResult.swiftSources/Panels/SurfaceSelectionSnapshot.swiftSources/Sidebar/SidebarWorkspaceDescriptionText.swiftSources/TerminalController+ControlSurfaceDock.swiftSources/TerminalController+SurfaceSelection.swiftSources/Workspace+RemoteTmuxControlTopology.swiftcmuxTests/CLIExplicitSurfaceRoutingTests.swiftcmuxTests/SurfaceSelectionTests.swiftcmuxTests/TerminalControllerSocketSecurityTests.swiftdocs/cli-contract.md
Contributor
Author
|
Fixed the latest CodeRabbit documentation finding in 64428da. The raw |
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. |
6 tasks
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 #10020
Summary
This PR adds one discoverable selection capability across selectable cmux surfaces:
surface.read_selectionfor socket clientscmux read-selection --surface <ref>for source-aware CLI outputcmux read-screen --selectionas a text-only terminal-compatible aliassurface.read_selectioninsystem.capabilities/cmux capabilitiesThe implementation is read-only: it does not focus a window, activate a workspace, move a pane, or mutate selection state.
Uniform selection model
Every supported panel implements the same async
Panel.readSurfaceSelection()capability and returns an immutable Sendable snapshot. The socket response always includes:has_selectionkindtextbase64nullIt conditionally includes
file_path, one-based inclusiveline_range, andurlwhen that source context exists. A supported surface with no selection succeeds withhas_selection: falseand empty text/base64. Unsupported panel kinds returnnot_supported; a supported surface whose live backing object is unavailable returnsunavailable.Snapshot construction makes selected and no-selection states distinct, and line ranges reject zero, negative, or reversed bounds. Explicit window/group/workspace/surface/pane selectors are validated before routing and fail closed instead of degrading to the focused surface. Pane-to-surface resolution lives in the shared workspace/Dock topology helpers rather than in the selection command.
Surface providers
ghostty_surface_read_selection, without requiring a mountedGhosttySurfaceScrollView.NSTextView.selectedRange(), extracts UTF-16-safe text, normalizes the file path, and maps the range to one-based inclusive source lines, including CRLF and selected line terminators.WKWebViewand returns the Markdown file path.Selectionobject into isolated content worlds, so the tracker deliberately lives in the same world that observes the selection. It owns an immutable last-selection snapshot so native focus can move to the neighboring agent surface without WebKit erasing the context before the socket read. Real page selection/focus/input changes and navigation reconcile or clear that snapshot; password controls clear it immediately, and cross-origin frame contents remain inaccessible by design. The response reports the live user-visible URL and returns immediately for an uncommitted document.Socket and concurrency design
surface.read_selectionis a worker-lane method withmainThreadCallable: false. The typed request remains intact until the async handler resolves the target. AppKit, WebKit, workspace routing, Ghostty access, and ref minting remain on the main actor. Only the immutable Sendable capture crosses back to the socket worker, where the common payload and base64 are encoded.The async payload builder has an explicit compiler-compatible
@concurrentboundary. The existing transport processes each connection on its own dedicated OS thread specifically because synchronous request/reply commands can wait on main-actor or async bridges; selection reuses that contract without blocking the listener or Swift cooperative executor. It introduces no polling, sleeps, accessibility scraping, or per-kind socket methods.CLI and discoverability
cmux read-selectionprints kind plus any file/line/URL context followed by the selected text.--jsonpreserves the complete common response, including a successful no-selection result. Plain output also exits zero for this valid state and prints the explicitHas selection: falsemarker.cmux read-screen --selectionuses the same RPC and prints selected text or the no-selection marker without source metadata. It rejects--scrollbackand--lines. Both read commands avoid global--windowprefocus so querying another surface does not steal focus.The CLI contract, command help, command suggestions, and capabilities output make the method discoverable.
Related terminal work
This includes the terminal socket lane and
read-screen --selectionbehavior requested by #7500. It intentionally does not close that issue because this PR does not add its DEBUGselect_allend-to-end fixture. The PR provides the production terminal read capability and alias while leaving any remaining test-seam work to that issue.Selection-change events
Push events are intentionally separated from this authoritative read path. AppKit, Ghostty, and WebKit expose selection changes through different owners and replacement lifecycles; adding per-panel listeners here would fragment ownership and risk leaks or accidental default disclosure of selected text.
The opt-in, debounced, polling-free event design is tracked at #10027. It will reuse this common result shape while owning observer and WebView teardown centrally.
Tests and validation
Behavior coverage includes:
The first commit contains only the regression tests; the second contains the production implementation and expanded behavior coverage.
Permitted local validation completed successfully: Swift frontend parsing, project normalization/checking, test-wiring lint, JSON validation, localization audit, file-length audit, and
git diff --check. Per repository guidance, no localxcodebuild, XCUITest, build, launch, or dogfood run was performed on this machine. Required GitHub checks are the compilation and test source of truth.Localization and budgets
All new CLI help, output labels, and socket error strings use
String(localized:defaultValue:)and have English and Japanese catalog entries. The touched user-facing strings and docs were audited. Every new Swift file is below 500 lines. A clean Xcode 26 compile exposed one existing unbudgeted deprecatedonChangeoverload inSidebarWorkspaceDescriptionText; the source was migrated to the modern two-argument callback instead of increasing the warning budget. No warning-budget or file-length-budget TSV was edited or regenerated.Summary by CodeRabbit
read-selectionCLI command for retrieving selected text from terminals, browsers, files, and Markdown surfaces.read-screen --selectionas a compatible alias with workspace, window, surface, and pane targeting.surface.read_selectionsocket capability with structured routing and error responses.