doc: add cross-workspace targeting note for send/send-key/read-screen#5004
doc: add cross-workspace targeting note for send/send-key/read-screen#5004v2nic wants to merge 1 commit into
Conversation
|
@v2nic is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughDocumentation updates across two reference files clarify how ChangesCross-workspace targeting clarification
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 18✅ Passed checks (18 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
These commands default --workspace to $CMUX_WORKSPACE_ID. When targeting a surface outside the caller's workspace, omitting --workspace produces the misleading error 'Surface is not a terminal' — the surface exists and is a terminal, but cmux resolves it in the wrong workspace context.
ba5f2c3 to
2b98135
Compare
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 (2)
skills/cmux-workspace/references/commands.md (1)
62-70:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winInconsistency between PR description and implementation.
The PR description states that the "Input" section examples were updated to "always include --workspace," but the examples at lines 62-67 do not include
--workspaceand show no changes (~markers). The note at lines 69-70 correctly explains when--workspaceis required, but if the intention was to update all examples to demonstrate explicit--workspaceusage, that change is missing.📝 If examples should always include --workspace, apply this change
```bash -cmux send "echo hello\n" -cmux send-key enter -cmux send --surface "$CMUX_SURFACE_ID" "git status\n" -cmux send-key --surface "$CMUX_SURFACE_ID" enter -cmux read-screen --surface "$CMUX_SURFACE_ID" +cmux send --workspace "$CMUX_WORKSPACE_ID" "echo hello\n" +cmux send-key --workspace "$CMUX_WORKSPACE_ID" enter +cmux send --workspace "$CMUX_WORKSPACE_ID" --surface "$CMUX_SURFACE_ID" "git status\n" +cmux send-key --workspace "$CMUX_WORKSPACE_ID" --surface "$CMUX_SURFACE_ID" enter +cmux read-screen --workspace "$CMUX_WORKSPACE_ID" --surface "$CMUX_SURFACE_ID"</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@skills/cmux-workspace/references/commands.mdaround lines 62 - 70, Update
the example invocations so they consistently pass an explicit workspace flag:
replace the local examples of cmux send, cmux send-key and cmux read-screen with
versions that include --workspace "$CMUX_WORKSPACE_ID" (and also include
--surface "$CMUX_SURFACE_ID" where applicable) so the examples match the PR
description; edit the examples for the commands send, send-key and read-screen
to always include --workspace and ensure the third/fourth examples also include
--surface to mirror the note about cross-workspace targeting.</details> </blockquote></details> <details> <summary>skills/cmux/references/panes-surfaces.md (1)</summary><blockquote> `39-40`: _🧹 Nitpick_ | _🔵 Trivial_ | _💤 Low value_ **Minor wording inconsistency between files.** This note uses "Cross-workspace input" while `skills/cmux-workspace/references/commands.md` line 69 uses "Cross-workspace targeting." Both are reasonable, but consistent terminology across documentation would be clearer. <details> <summary>♻️ Align terminology with commands.md</summary> ```diff -**Cross-workspace input:** `send`, `send-key`, and `read-screen` resolve `--workspace` from `$CMUX_WORKSPACE_ID`. When targeting a surface outside the caller's workspace, you must pass `--workspace` explicitly. Omitting it produces the misleading `Error: invalid_params: Surface is not a terminal`. +**Cross-workspace targeting:** `send`, `send-key`, and `read-screen` resolve `--workspace` from `$CMUX_WORKSPACE_ID`. When targeting a surface outside the caller's workspace, you must pass `--workspace` explicitly. Omitting it produces the misleading `Error: invalid_params: Surface is not a terminal`.🤖 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 `@skills/cmux/references/panes-surfaces.md` around lines 39 - 40, Change the phrasing in panes-surfaces.md to match the terminology used in commands.md: replace "Cross-workspace input" with "Cross-workspace targeting" (or vice versa if you prefer the other term) so both docs use the exact same phrase; update the note that mentions `send`, `send-key`, `read-screen` and the `$CMUX_WORKSPACE_ID` behavior to use "Cross-workspace targeting" to mirror the wording in commands.md.
🤖 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 `@skills/cmux-workspace/references/commands.md`:
- Around line 62-70: Update the example invocations so they consistently pass an
explicit workspace flag: replace the local examples of cmux send, cmux send-key
and cmux read-screen with versions that include --workspace "$CMUX_WORKSPACE_ID"
(and also include --surface "$CMUX_SURFACE_ID" where applicable) so the examples
match the PR description; edit the examples for the commands send, send-key and
read-screen to always include --workspace and ensure the third/fourth examples
also include --surface to mirror the note about cross-workspace targeting.
In `@skills/cmux/references/panes-surfaces.md`:
- Around line 39-40: Change the phrasing in panes-surfaces.md to match the
terminology used in commands.md: replace "Cross-workspace input" with
"Cross-workspace targeting" (or vice versa if you prefer the other term) so both
docs use the exact same phrase; update the note that mentions `send`,
`send-key`, `read-screen` and the `$CMUX_WORKSPACE_ID` behavior to use
"Cross-workspace targeting" to mirror the wording in commands.md.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3481a008-9de9-4e99-b375-68ab6c34071c
📒 Files selected for processing (2)
skills/cmux-workspace/references/commands.mdskills/cmux/references/panes-surfaces.md
Greptile SummaryThis PR documents the cross-workspace targeting requirement for
Confidence Score: 4/5Documentation-only change; no production code is modified and the added guidance is factually correct. The note content is accurate and directly addresses the agent regression. The two gaps — missing --workspace in the code examples and missing section header/examples in panes-surfaces.md — are polish issues that leave the docs slightly inconsistent with the stated intent but don't introduce any incorrect information. Both changed files would benefit from the small follow-up fixes noted in comments, but neither has correctness problems that would block merging. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Agent calls send/send-key/read-screen] --> B{--workspace provided?}
B -- No --> C[Resolves workspace from $CMUX_WORKSPACE_ID]
B -- Yes --> D[Uses provided workspace]
C --> E{Surface in same workspace?}
E -- Yes --> F[✅ Command succeeds]
E -- No --> G[❌ Error: invalid_params: Surface is not a terminal]
D --> F
|
|
|
||
| Surface identity is stable across move/reorder/split-off operations. Layout commands are focus-neutral by default; pass `--focus true` only when you want the moved or created surface selected. | ||
|
|
||
| **Cross-workspace input:** `send`, `send-key`, and `read-screen` resolve `--workspace` from `$CMUX_WORKSPACE_ID`. When targeting a surface outside the caller's workspace, you must pass `--workspace` explicitly. Omitting it produces the misleading `Error: invalid_params: Surface is not a terminal`. |
There was a problem hiding this comment.
Note orphaned without section header or examples
The cross-workspace note is appended to a file that covers only layout operations (split, move, focus, reorder) and contains no send/send-key/read-screen examples. Without a section heading anchoring it, an agent or developer reading panes-surfaces.md for surface layout guidance won't naturally encounter the input section, and anyone specifically seeking input usage has no example to copy. The PR description mentioned adding an "Input and Screen Reading" section with examples — that section header and at least one --workspace example were not included in the diff. Adding a heading like ## Input and Screen Reading with a representative example (e.g. cmux read-screen --workspace workspace:46 --surface surface:266) above the bold note would make the guidance more discoverable and actionable.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Problem
cmux send,send-key, andread-screendefault--workspaceto$CMUX_WORKSPACE_ID. When targeting a surface outside the caller's workspace, omitting--workspaceproduces a misleading error:The surface is a terminal — cmux just resolved it in the wrong workspace context. This caused an agent script to classify ~40 live, interactive surfaces as "zombie/orphaned terminals" and skip them entirely.
Changes
skills/cmux/references/panes-surfaces.md— Add "Input and Screen Reading" section with examples showing--workspaceand a bold note about the cross-workspace requirement and the misleading error message.skills/cmux-workspace/references/commands.md— Update the "Input" section examples to always include--workspace, matching the pattern used for pane/surface commands throughout the doc. Add the same cross-workspace targeting note.Test plan
cmux read-screen --surface surface:266fails with "Surface is not a terminal" when the surface is in a different workspacecmux read-screen --workspace workspace:46 --surface surface:266succeeds and returns the terminal contentssendandsend-keybehave the same way--workspacepatternsNeed help on this PR? Tag
@codesmithwith what you need. Autofix is disabled.Summary by cubic
Clarified cross-workspace behavior for
send,send-key, andread-screen: these commands default--workspaceto$CMUX_WORKSPACE_ID, so you must pass--workspacewhen targeting surfaces in other workspaces to avoid the misleadingError: invalid_params: Surface is not a terminal.Added a bold cross-workspace note to
skills/cmux/references/panes-surfaces.mdandskills/cmux-workspace/references/commands.md.Written for commit 2b98135. Summary will update on new commits.
Summary by CodeRabbit
send,send-key, andread-screencommands. The workspace parameter is automatically inferred from the environment for same-workspace operations but must be explicitly provided when targeting surfaces in different workspaces.