fix: make pane input descriptions harm-first - #356
Conversation
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_d24f2bf1-d5c3-4879-8d54-40758210a60e) |
|
Warning Review limit reached
Next review available in: 47 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThis PR adds two documentation files describing a design and implementation plan for tool description safety, focused on harm-first pane-writing warnings and a unified spawn prompt validator. It also updates two test files to validate this wording and spawn-path guard behavior. ChangesTool Description Safety
Estimated code review effort: 2 (Simple) | ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| return; | ||
| } | ||
|
|
||
| const overrideGuidance = |
There was a problem hiding this comment.
🟡 Medium src/server.ts:1306
When assertInteractiveMultilineInputAllowed rejects a multi-paragraph prompt from new_worktree_split or spawn_in_workspace, the error message tells the caller to pass boot_prompt_path, but neither tool exposes that argument, so the remediation is unusable. The message is built unconditionally for every tool, but the boot_prompt_path guidance only applies to the original launchers. Consider branching the guidance so these legacy tools are directed to use a short Read and follow <path> prompt instead, or document why they are intentionally given a non-actionable path.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @src/server.ts around line 1306:
When `assertInteractiveMultilineInputAllowed` rejects a multi-paragraph `prompt` from `new_worktree_split` or `spawn_in_workspace`, the error message tells the caller to pass `boot_prompt_path`, but neither tool exposes that argument, so the remediation is unusable. The message is built unconditionally for every tool, but the `boot_prompt_path` guidance only applies to the original launchers. Consider branching the guidance so these legacy tools are directed to use a short `Read and follow <path>` prompt instead, or document why they are intentionally given a non-actionable path.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 50be412497
ℹ️ 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".
| - Lead every pane-writing tool description with that guidance: `send_to`, | ||
| `send_input`, `send_command`, `broadcast`, `spawn_agent`, `new_split`, | ||
| `new_surface`, `new_worktree_split`, `spawn_in_workspace`, and the deprecated | ||
| `send_to_agent` alias. |
There was a problem hiding this comment.
The new “every pane-writing tool” list omits the public interact tool. In src/server.ts, interact(action="send") forwards arbitrary text directly to deliverAgentInput without the inline, dense-line, or multiline guards, and tests/enter-reliability.test.ts still asserts that a 2,000-character message succeeds. A caller using this API therefore receives none of the warning and can bypass the cap, reproducing the pane-wedging case this change is intended to prevent; include the interact description/field and guard its send action.
Useful? React with 👍 / 👎.
| opts.arg === "prompt" || opts.tool === "send_command" | ||
| ? " For launcher boot prompts, put the full prompt in a file and pass boot_prompt_path." |
There was a problem hiding this comment.
When an over-cap prompt is passed to new_worktree_split or spawn_in_workspace, this shared branch tells the caller to pass boot_prompt_path, but neither tool exposes that argument in its schema. Following the new error therefore cannot work—the unknown field is not a supported delivery path—so make this guidance conditional on tools that actually accept boot_prompt_path and direct these deprecated tools only to submit a short Read and follow <path> prompt.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@docs/plans/2026-08-04-tool-description-safety.md`:
- Line 13: Update the task headings in the document, including “Task 1” and the
corresponding task headings at the other referenced locations, from H3 (###) to
H2 (##). Keep the step-level headings as H3 (###).
In `@src/server.ts`:
- Around line 1306-1311: Update the shared over-cap and multi-paragraph error
guidance to be tool-specific: for new_worktree_split and spawn_in_workspace,
recommend a short “Read and follow <path>” prompt or redirect callers to
spawn_agent instead of advertising boot_prompt_path; retain boot_prompt_path
guidance only for tools that define it. Apply this consistently across the
affected error-message branches, including the logic around overrideGuidance.
- Around line 341-342: Update the text constant PANE_INPUT_BREAKAGE_GUIDANCE so
its first sentence warns that longer payloads break the receiving pane, then
state the 2–3 short-line limit and retain the existing file-writing instruction.
In `@tests/server.test.ts`:
- Line 1012: Strengthen the assertion for verboseDescription in the relevant
server test so it requires wording that explicitly conveys increased token
usage, such as “costs more tokens” or “increases token usage,” rather than
merely matching the word “token.”
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 4394dcdc-aade-4f92-b2c2-7dc950c7a0ed
📒 Files selected for processing (5)
docs/plans/2026-08-04-tool-description-safety-design.mddocs/plans/2026-08-04-tool-description-safety.mdsrc/server.tstests/pointer-discipline.test.tstests/server.test.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (4)
src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.ts: Build the project with TypeScript (tsc) and keep source code compatible with Node 20+ and Zod-based typing.
Use theok(data)anderr(error)helpers for consistent MCP tool responses.
All MCP tool handlers must return{ content: TextContent[], structuredContent?, isError? }.
Files:
src/server.ts
src/server.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Register all MCP tools in
server.ts, including the 33 tool handlers, and conditionally skip agent-lifecycle tools whenskipAgentLifecycle: true.
Files:
src/server.ts
tests/**/*.test.ts
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.test.ts: Mirror source layout in tests (src/foo.ts->tests/foo.test.ts).
Do not add integration tests that require a running cmux instance; tests should be fully mocked.
Files:
tests/server.test.tstests/pointer-discipline.test.ts
tests/server.test.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Server tests should mock the cmux client via
createServer({ exec, skipAgentLifecycle }).
Files:
tests/server.test.ts
🧠 Learnings (1)
📚 Learning: 2026-03-15T10:42:35.917Z
Learnt from: EtanHey
Repo: EtanHey/cmuxlayer PR: 1
File: tests/quality-tracking.test.ts:171-200
Timestamp: 2026-03-15T10:42:35.917Z
Learning: In tests/quality-tracking.test.ts for the cmuxlayer project, ensure that at or above 80% context quality degradation, behavior depends on depth: depth-0 agents receive a /compact command; depth > 0 agents are killed and logged (kill + log). Respawn of non-root agents is out of scope for v1. Treat the design doc quality tracking section as the authoritative source for this behavior, and align test expectations accordingly.
Applied to files:
tests/server.test.tstests/pointer-discipline.test.ts
🪛 markdownlint-cli2 (0.23.1)
docs/plans/2026-08-04-tool-description-safety.md
[warning] 13-13: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
🔇 Additional comments (7)
docs/plans/2026-08-04-tool-description-safety-design.md (1)
1-70: LGTM!tests/pointer-discipline.test.ts (1)
786-906: LGTM!Also applies to: 1200-1271
src/server.ts (5)
1359-1397: LGTM!Also applies to: 1400-1429
5517-5528: LGTM!Also applies to: 6025-6058, 6370-6388
6550-6627: LGTM!Also applies to: 6740-6799
8655-8782: LGTM!
10249-10282: LGTM!Also applies to: 11024-11133, 11173-11228
|
|
||
| --- | ||
|
|
||
| ### Task 1: Pin pane-input description wording |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use H2 headings for the tasks.
Lines 13, 52, and 81 use ### directly after the document H1. Change each task heading to ##. Keep the step headings at ###.
Based on static analysis, this violates MD001.
Also applies to: 52-52, 81-81
🧰 Tools
🪛 markdownlint-cli2 (0.23.1)
[warning] 13-13: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
🤖 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/plans/2026-08-04-tool-description-safety.md` at line 13, Update the task
headings in the document, including “Task 1” and the corresponding task headings
at the other referenced locations, from H3 (###) to H2 (##). Keep the step-level
headings as H3 (###).
Source: Linters/SAST tools
| const PANE_INPUT_BREAKAGE_GUIDANCE = | ||
| "Max 2-3 short lines. Longer payloads BREAK the receiving pane — write the payload to a file and send one line: `Read and follow <path>`."; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Put the harm-first warning first.
PANE_INPUT_BREAKAGE_GUIDANCE starts with Max 2-3 short lines.. The PR requirement is for pane-writing descriptions to begin with the pane-breakage warning. Move Longer payloads BREAK the receiving pane to the first sentence, then keep the numeric limit.
Suggested wording
- "Max 2-3 short lines. Longer payloads BREAK the receiving pane — write the payload to a file and send one line: `Read and follow <path>`.";
+ "Longer payloads BREAK the receiving pane. Keep payloads to 2-3 short lines; write longer payloads to a file and send one line: `Read and follow <path>`.";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const PANE_INPUT_BREAKAGE_GUIDANCE = | |
| "Max 2-3 short lines. Longer payloads BREAK the receiving pane — write the payload to a file and send one line: `Read and follow <path>`."; | |
| const PANE_INPUT_BREAKAGE_GUIDANCE = | |
| "Longer payloads BREAK the receiving pane. Keep payloads to 2-3 short lines; write longer payloads to a file and send one line: `Read and follow <path>`."; |
🤖 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 `@src/server.ts` around lines 341 - 342, Update the text constant
PANE_INPUT_BREAKAGE_GUIDANCE so its first sentence warns that longer payloads
break the receiving pane, then state the 2–3 short-line limit and retain the
existing file-writing instruction.
| expect(tool.description).toMatch(/condensed by default/i); | ||
| const verboseDescription = tool.inputSchema.shape.verbose.description; | ||
| expect(verboseDescription).toMatch(/raw cmux fields/i); | ||
| expect(verboseDescription).toMatch(/token/i); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Assert the token-cost relationship.
Line 1012 only requires the word token. A description can mention tokens without stating that verbose increases token usage and still pass. Assert costs more tokens or increases token usage.
Based on the implementation plan, verbose must state that it costs more tokens.
🤖 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 `@tests/server.test.ts` at line 1012, Strengthen the assertion for
verboseDescription in the relevant server test so it requires wording that
explicitly conveys increased token usage, such as “costs more tokens” or
“increases token usage,” rather than merely matching the word “token.”
50be412 to
0e26c55
Compare
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_10fe109b-6ddd-4569-a61e-f323067de923) |
1 participant
Summary
new_worktree_split/spawn_in_workspacebypass found during review.list_surfacescondensed output unchanged and make the cost and rarity ofverbose:trueexplicit.Design decisions
submit_verified:nullDo not blanket-fail null. Normal interactive
send_toalready attempts submit verification and converts attempted-but-unconfirmed delivery tosubmit_verified:false, which returns non-ok. A successful null means verification was intentionally not attempted, includingpress_enter:falseandallow_busy:true. PR #326, now merged, complements this by hardening spawn/boot settlement; it does not require a second generic relay mechanism. The recommendation was posted in the #326 thread.list_surfacesLive measurement on 11 surfaces:
The condensed pane, column, and real-CWD fields are required for deterministic placement and routing, so this is case (a): callers should avoid
verbose:trueunless a specific raw field is needed.Review disposition
Verification
npx vitest run tests/pointer-discipline.test.ts tests/server.test.ts --reporter=dot— 229 passednpm run typecheck— passednpm run build— passednpm run pre-pr— 63 passednpm test -- --reporter=dot— 106 files, 2,393 passedrun_tests.shexit 0list_surfacesreturned 3,779 / 9,262 bytes (2.45x)Scope
No release was run. This PR is ready for the lead-routed Codex review and is intentionally not merged by the worker.
Note
Medium Risk
Touches central MCP input validation and spawn entry points; mis-wiring could block legitimate spawns or leave unsafe prompts accepted, though behavior is heavily regression-tested and mostly aligns existing guards with previously missing paths.
Overview
MCP tool descriptions now lead with a shared harm-first rule (max 2–3 short lines; longer inline payloads break the receiving pane; use file-backed
Read and follow <path>) instead of byte-threshold / dense-routing prose. That prefix is applied to all pane-writing tools (send_input,send_command,send_to,broadcast, spawn/split/surface tools, andsend_to_agent) and to inline text/command/prompt andboot_prompt_pathfield descriptions.Runtime behavior adds
assertSpawnPromptInputAllowedsospawn_agent,new_worktree_split, andspawn_in_workspaceall run the same over-cap, dense-line, and multi-paragraph checks before workspace/pane mutation. Legacynew_worktree_split/spawn_in_workspacepaths previously skipped these guards; they also omitallow_long_inlineoverride messaging.list_surfacestool andverbosearg descriptions now state condensed-by-default output and that verbose raw fields cost materially more tokens and are rarely needed (response shape unchanged).Tests pin the description contract and six pre-mutation rejection cases; design/implementation plan docs are added.
Reviewed by Cursor Bugbot for commit 0e26c55. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Make pane-writing tool descriptions and field descriptions lead with harm-first input guidance
PANE_BREAKAGE_GUIDANCEconstant to server.ts and prepends it to the description and relevant field descriptions of all pane-writing tools (send_input,send_command,spawn_agent,new_split,new_surface,broadcast,send_to,send_to_agent,new_worktree_split,spawn_in_workspace).assertSpawnPromptInputAllowedto centralize prompt validation logic, replacing three separate inline/dense/multiline guard calls inspawn_agentand introducing pre-mutation guards innew_worktree_splitandspawn_in_workspace.new_worktree_splitandspawn_in_workspacenow reject over-cap, dense, or multi-paragraph inline prompts before any workspace or pane mutation; neither tool supports theallow_long_inlineoverride.list_surfacesdescription to document that results are condensed by default and thatverboseincreases token usage and is rarely needed.📊 Macroscope summarized 0e26c55. 3 files reviewed, 0 issues evaluated, 0 issues filtered, 0 comments posted
🗂️ Filtered Issues
No issues evaluated.
Summary by CodeRabbit
Documentation
list_surfacesoutput by default, with optional verbose details.Tests