Repository navigation
Add Roughdraft markdown review handoff - #4450
austinywang wants to merge 19 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:
📝 WalkthroughWalkthroughAdds a top-level ChangesRoughdraft command support
Sequence DiagramsequenceDiagram
participant User as User/Dispatcher
participant runRough as runRoughdraftCommand
participant urlHelper as roughdraftDocumentURL
participant extRough as External Roughdraft CLI
participant clientAPI as client.sendV2
User->>runRough: dispatch "cmux roughdraft open <path>" or "cmux roughdraft <path>"
runRough->>runRough: parse options & validate args (--workspace/--window/--surface/--focus)
runRough->>urlHelper: request document URL for <path>
urlHelper->>extRough: run "roughdraft open <path> --print-url --no-watch"
extRough-->>urlHelper: stdout with http(s) URL
urlHelper-->>runRough: return first http(s) URL
runRough->>clientAPI: sendV2(method: "browser.open_split", params including roughdraft_url, path, placement ids, focus)
clientAPI-->>runRough: pane/surface response (created_split, ids)
runRough-->>User: JSON or localized success message
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (19 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 |
Greptile SummaryAdds
Confidence Score: 5/5Safe to merge. The new roughdraft command is a thin integration layer: spawn an external CLI, validate its output, open a browser split. All previously identified review concerns have been resolved in the current commit. The implementation is well-scoped. Error sanitization is verified by a regression test that asserts sensitive subprocess output never reaches the user. Localization covers all 20 locale entries with genuine translations. The runProcess tuple extension adds a field without breaking existing named-field callers. URL validation correctly rejects non-http/https and host-less URLs. No actor isolation, blocking primitive, or unhandled-state issues were found in the changed code. No files require special attention. Important Files Changed
Sequence DiagramsequenceDiagram
actor User
participant cmux as cmux CLI
participant rd as roughdraft CLI
participant socket as cmux Socket
participant browser as Browser Pane
User->>cmux: cmux roughdraft open path.md
cmux->>rd: /usr/bin/env roughdraft open path.md --print-url --no-watch (30s timeout)
rd-->>cmux: stdout: http://127.0.0.1:PORT/roughdraft/path
Note over cmux: validate http/https + non-empty host
cmux->>socket: "browser.open_split(url, bypass_remote_proxy=true, workspace_id, ...)"
socket-->>cmux: surface_id, pane_id, created_split
cmux->>browser: opens localhost Roughdraft URL
cmux-->>User: "OK surface=... pane=... path=... url=..."
Reviews (14): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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/cmux.swift`:
- Around line 4328-4333: The current throw includes raw upstream stdout/stderr
from the roughdraft invocation (variables result, message, detail) which may
leak sensitive internals; instead replace the detailed inclusion with a
sanitized, user-friendly message: keep the CLIError text about installing
Roughdraft but drop raw result.stdout/result.stderr, or if you want minimal
diagnostics include only a sanitized/truncated snippet (e.g., first non-empty
line, stripped of paths/stack traces) and indicate it’s a truncated diagnostic.
Update the code around result, message, detail and the CLIError throw to use the
sanitized string.
🪄 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: 526e7a7f-7d89-41e0-b57a-165ece625ccc
📒 Files selected for processing (2)
CLI/cmux.swiftdocs/cli-contract.md
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 2 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
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/cmux.swift`:
- Line 4328: Replace the hardcoded, branded error thrown by CLIError in the
throw site (the throw CLIError(...) that currently references "Roughdraft", "npm
i -g roughdraft" and result.status) with a localized, generic message via your
app's localization helper (e.g. NSLocalizedString or the project's i18n
utility); keep the message provider-agnostic (e.g. "Failed to open required
tool. Please install it and retry.") and optionally append non-branded
machine-readable details like result.status separately or as metadata rather
than in the user-facing string; update the throw to use the localization key and
include result.status only where appropriate for logs or diagnostics.
🪄 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: 3e22cd54-c855-49f3-b7a8-0216c2fea442
📒 Files selected for processing (1)
CLI/cmux.swift
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
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 e4fbe67. 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 `@CLAUDE.md`:
- Around line 235-250: This review comment is providing positive feedback
acknowledging that the new Skills section in CLAUDE.md is a helpful and
well-organized addition for developers to locate task-specific documentation. No
fix is required; the implementation shown in the diff already successfully
addresses the organizational goal by providing a clear, structured skill map
with relevant references to different development areas (cmux-dev-workflow,
cmux-architecture, cmux-backend, cmux-testing, etc.).
🪄 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: 4d51be97-acda-454b-9409-6cbccd559ddb
📒 Files selected for processing (2)
CLAUDE.mdCLI/cmux.swift
💤 Files with no reviewable changes (1)
- CLI/cmux.swift
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
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 `@CLAUDE.md`:
- Around line 235-250: This review comment is providing positive feedback
acknowledging that the new Skills section in CLAUDE.md is a helpful and
well-organized addition for developers to locate task-specific documentation. No
fix is required; the implementation shown in the diff already successfully
addresses the organizational goal by providing a clear, structured skill map
with relevant references to different development areas (cmux-dev-workflow,
cmux-architecture, cmux-backend, cmux-testing, etc.).
🪄 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: 4d51be97-acda-454b-9409-6cbccd559ddb
📒 Files selected for processing (2)
CLAUDE.mdCLI/cmux.swift
💤 Files with no reviewable changes (1)
- CLI/cmux.swift
🛑 Comments failed to post (1)
CLAUDE.md (1)
235-250: 🧹 Nitpick | 🔵 Trivial | 💤 Low value
New Skills section provides helpful reference map for task-specific development areas.
The addition of a structured skill map at the end of CLAUDE.md helps developers quickly locate task-specific documentation for different areas of the codebase (e.g.,
cmux-localization,cmux-testing,cmux-ghostty). This is a useful organizational addition to the document.🤖 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 `@CLAUDE.md` around lines 235 - 250, This review comment is providing positive feedback acknowledging that the new Skills section in CLAUDE.md is a helpful and well-organized addition for developers to locate task-specific documentation. No fix is required; the implementation shown in the diff already successfully addresses the organizational goal by providing a clear, structured skill map with relevant references to different development areas (cmux-dev-workflow, cmux-architecture, cmux-backend, cmux-testing, etc.).
lawrencecchen
added
the
stale-revisit
This branch was successfully deployed
3 participants
Summary
Adds a
cmux roughdraft open <path>CLI handoff for Roughdraft, a local-first Markdown review app for commenting, suggesting edits, and collaborating with coding agents.Research
roughdraft.md,/docs,/api,/download,/signup,robots.txt,sitemap.xml, and the Vite bundle. The static routes all serve the same sparse React app, but the bundle exposes/setup.md,/spec/roughdraft-flavored-markdown.md, local/api/markdown-fileand/api/review-eventspaths, andhttps://github.com/Lex-Inc/roughdraft.https://roughdraft.md/setup.md, the Roughdraft README, CLI docs, and server source. Roughdraft has a published npm CLI (roughdraft@0.1.8) withroughdraft open <path> --print-url --no-watch, a local server on localhost, direct file-backed Markdown editing, CriticMarkup review state,watch, and experimental MCP support.cmux markdown), PR Add markdown surfaces to workspace layouts #3581 for markdown surface layouts, and PR feat: addnotesurface type withcmux noteCLI #4332 for note surfaces. This PR avoids duplicating those surface/editor efforts and builds on the existing browser-pane placement path.Chosen integration
Picked the smallest useful integration that works today without a Roughdraft account or hosted API: ask the installed Roughdraft CLI for a local document URL without opening an external browser, then open that URL inside a cmux browser pane.
This keeps Roughdraft as the source of truth for review semantics and uses cmux only for workspace placement.
User-facing surface area
cmux roughdraft open <path>.cmux roughdraft <path>.--workspace,--surface,--window,--focus.--json.Manual verification after HQ dev build launches
npm i -g roughdraft.README.md.cmux roughdraft open README.md --focus true.cmux roughdraft --helpand verify the command help documents the Roughdraft handoff.Verification
git diff --check.xcodebuild, or./scripts/reload.shper task instructions.Need help on this PR? Tag
@codesmithwith what you need. Autofix is disabled.Note
Medium Risk
Spawns an external CLI and loads its returned URL in an embedded browser pane; failures are contained with validation and sanitized errors, but depend on third-party tooling being installed and responsive.
Overview
Adds
cmux roughdraftto open a local Markdown file in a Roughdraft review UI inside a cmux browser split, instead of using the built-inmarkdownviewer.The CLI runs
roughdraft open <path> --print-url --no-watch(30s timeout), accepts only an http/https document URL from stdout, then callsbrowser.open_splitwith optional--workspace,--surface,--window, and--focus(default workspace from$CMUX_WORKSPACE_IDwhen no window is set). Shorthandcmux roughdraft <path>is supported; success output is plain text or--json.runProcessnow surfacestimedOutso Roughdraft failures can distinguish timeouts from non-zero exits without leaking subprocess output. User-facing errors are localized and sanitized (status code only on failure). Help, top-level command list,docs/cli-contract.md, and a test with a fakeroughdraftonPATHverify no socket traffic when the external CLI fails.Reviewed by Cursor Bugbot for commit 4703cf2. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Adds a CLI handoff to open local Markdown files in
Roughdraftinside a cmux browser split by calling theroughdraftCLI to fetch a document URL. Validates http/https URLs, bypasses the remote proxy for Roughdraft URLs, and opens in-app with localized help and sanitized errors.New Features
cmux roughdraft open <path>; shorthandcmux roughdraft <path>; plain text or--jsonoutput.--workspace,--surface,--window,--focus; defaults to$CMUX_WORKSPACE_IDwhen--windowisn’t set. 30s CLI timeout; accepts only valid http/https URLs. Localizedcmux roughdraft --help; top-level command list anddocs/cli-contract.mdupdated.Bug Fixes
bypass_remote_proxy: truefor Roughdraft URLs; test covers the proxy bypass.Written for commit b7bd864. Summary will update on new commits.
Summary by CodeRabbit
New Features
roughdraftCLI command to open a Markdown file in a Roughdraft browser pane, including shorthand andopenforms with--workspace,--surface,--window, and--focusoptions, returning either JSON or a formatted success message.Bug Fixes
Documentation
roughdraft, including detailed usage examples.Localization
roughdraftusage, success output, and error states.Tests