Repository navigation
Expose production read-screen capture-pane APIs - #219
Conversation
Preview Videos and ScreenshotsOpen Workspace (1 hr expiry) · Open Dev Browser (1 hr expiry) · Open Diff Heatmap Failed to capture screenshots for commit
Generated by manaflow preview system |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b163c2bbf3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let response = readTerminalTextBase64( | ||
| surfaceArg: v2String(params, "surface_id") ?? "", | ||
| includeScrollback: includeScrollback, |
There was a problem hiding this comment.
Resolve read_text against requested workspace
surface.read_text ignores workspace_id (and the workspace implied by surface_id) because it forwards only surface_id into readTerminalTextBase64, which always reads from tabManager.selectedTabId. In a multi-workspace session, calling surface.read_text (or cmux read-screen --workspace ...) for a non-selected workspace will read the wrong surface or return "Terminal surface not found" even when the target workspace/surface exists; this is a correctness regression compared with other surface.* v2 methods that resolve the workspace from params.
Useful? React with 👍 / 👎.
Addresses review feedback from #219 by resolving read-screen targets against requested workspace/surface instead of the selected workspace.
|
Follow-up fix is up in #220. It updates to resolve reads against the requested / instead of the selected workspace, and adds non-selected-workspace regression coverage in . |
|
Follow-up fix is up in #220. It updates the surface.read_text implementation to read from the requested workspace_id/surface_id context (not just the selected workspace) and adds non-selected-workspace regression coverage in tests_v2/test_read_screen_capture_pane_parity.py. |
Addresses review feedback from manaflow-ai/cmux#219 by resolving read-screen targets against requested workspace/surface instead of the selected workspace.
…d-screen-production Expose production read-screen capture-pane APIs
Summary
read_screenin production socket command routing and add--scrollback/--linesparsingsurface.read_textmethod and include it insystem.capabilitiesread-screencommand wired tosurface.read_textwith help/usage docstests_v2client text-read helper to prefersurface.read_text(with fallback)tests_v2/test_read_screen_capture_pane_parity.pycovering capabilities + CLI behaviorIssues
Validation
xcodebuild -project GhosttyTabs.xcodeproj -scheme cmux -configuration Debug -destination 'platform=macOS' buildCMUX_TAG=issue-152-153-read-screen CMUX_SOCKET=/tmp/cmux-debug-issue-152-153-read-screen.sock CMUX_SOCKET_PATH=/tmp/cmux-debug-issue-152-153-read-screen.sock CMUXTERM_CLI=/tmp/cmux-issue-152-153-read-screen/Build/Products/Debug/cmux python3 tests_v2/test_read_screen_capture_pane_parity.pyCMUX_TAG=issue-152-153-read-screen CMUX_SOCKET=/tmp/cmux-debug-issue-152-153-read-screen.sock CMUX_SOCKET_PATH=/tmp/cmux-debug-issue-152-153-read-screen.sock CMUXTERM_CLI=/tmp/cmux-issue-152-153-read-screen/Build/Products/Debug/cmux python3 tests/test_blank_screen.py