Repository navigation
Add devbox.new creator - #5677
lawrencecchen wants to merge 3 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 5 minutes. View limit detailsLimit details: You’ve used all 10 included reviews currently available. Review configuration: ⚙️ Run configurationConfiguration used: Repository: manaflow-ai/cmux/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe PR introduces a new ChangesDevbox.new feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (2 errors, 1 warning)
✅ Passed checks (18 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 |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
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 1fed97b. Configure here.
|
|
||
| setIsCreating(true); | ||
| setError(null); | ||
| setCreated(null); |
There was a problem hiding this comment.
Duplicate create race on submit
Medium Severity
The create handler only blocks repeat submits with isCreating state, which updates on the next render. A second submit in the same tick (for example a quick double-click) can run before that update and send another POST /api/vm with a fresh idempotency-key, provisioning two devboxes for one action.
Reviewed by Cursor Bugbot for commit 1fed97b. Configure here.
![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: 1fed97bb9e
ℹ️ 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".
| <div className="w-full"> | ||
| <div className="mb-8 text-center"> | ||
| <h1 className="text-3xl font-semibold tracking-tight sm:text-5xl"> | ||
| Create a devbox |
There was a problem hiding this comment.
AGENTS.md requires every user-facing web string to be localized across the supported message catalogs (web/messages/en.json and web/messages/ja.json). This new devbox page hard-codes visible copy such as the heading, prompt text, labels, buttons, errors, and metadata, so the Japanese surface cannot be translated and the required localization audit cannot pass; please move the copy into localized messages and add both locales.
Useful? React with 👍 / 👎.
Greptile SummaryThis PR adds a standalone
Confidence Score: 4/5The routing and proxy wiring are solid, but the new page ships every user-visible string as hardcoded English with no next-intl integration, leaving all 20 non-English locales with an untranslated UI. The devbox-routing logic, the proxy rewrite ordering, idempotency key handling, and the export rename are all correct. The one concrete gap is that devbox-creator.tsx, layout.tsx, and page.tsx introduce a production user-facing surface with no entries in any of the 20 locale message files and no useTranslations calls — a pattern the project requires for every new web surface. web/app/devbox/devbox-creator.tsx and web/app/devbox/layout.tsx — all new user-visible strings and metadata need next-intl keys added across every locale in web/messages/. Important Files Changed
Sequence DiagramsequenceDiagram
participant Browser
participant Proxy as proxy.ts (Next.js 16 middleware)
participant DevboxPage as /devbox page
participant API as /api/vm
Browser->>Proxy: GET devbox.new/
Proxy->>Proxy: shouldRewriteToDevbox("devbox.new", "/") → true
Proxy-->>DevboxPage: rewrite to /devbox (host unchanged)
DevboxPage-->>Browser: Render DevboxCreator
Browser->>API: POST devbox.new/api/vm (excluded from matcher)
API-->>Browser: "200 { id, provider, ... } or 401"
Browser->>Browser: Display VM id + attach commands
Reviews (1): Last reviewed commit: "feat: add devbox.new creator" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| function userMessage(status: number, body: VmErrorBody | null) { | ||
| if (status === 401) { | ||
| return "Sign in first, then create the devbox again."; | ||
| } | ||
|
|
||
| const pieces = [body?.message, body?.action ?? body?.reason].filter( | ||
| Boolean, | ||
| ); | ||
| return pieces.join(" ") || `Devbox create failed with status ${status}.`; | ||
| } |
There was a problem hiding this comment.
Every user-visible string in this file — the heading ("Create a devbox"), the subheading, the textarea placeholder ("What should this devbox work on?"), the button labels ("Create devbox", "Creating..."), the sign-in link, the success section heading ("Devbox created"), the field labels ("ID", "Provider"), and the two error messages — are hardcoded in English with no call to useTranslations or any next-intl API. The project already supports 20 locales defined in web/i18n/routing.ts, and no corresponding keys were added to any file under web/messages/. The same applies to the <title> and description metadata in layout.tsx. A visitor whose browser language is Japanese, Arabic, or any non-English locale will receive a fully English UI, which violates the cmux-full-internationalization rule that applies to every new production user-facing surface.
Rule Used: Flag production user-facing text that is not fully... (source)
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!
![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 `@web/app/devbox/devbox-creator.tsx`:
- Around line 29-38: Replace hardcoded strings in userMessage with localized
messages: import and call useTranslations("devbox") in the component that calls
userMessage, change userMessage(status: number, body: VmErrorBody | null) to
accept a translator (or move its logic into the component) and map status/body
to locale keys (e.g. t("errors.unauthenticated") for 401,
t("errors.<mapped_key>") for known API error codes or body.action/body.reason),
avoid directly surface body.message/action/reason to users, and fall back to
t("errors.generic", { status }) when no mapping exists; ensure translation keys
(errors.unauthenticated, errors.generic, and specific mapped keys) are added to
the devbox namespace in your i18n messages and routing.
In `@web/app/devbox/layout.tsx`:
- Around line 15-29: The exported metadata object (metadata) and the hardcoded
lang="en" in web/app/devbox/layout.tsx must be made locale-aware: load the
current locale from your routing helper (web/i18n/routing.ts) or next/navigation
params, fetch localized title/description from web/messages/<locale> (or the app
i18n utility), and build metadata (title, description,
openGraph.url/siteName/alternates) and the html lang attribute dynamically using
those localized strings and the active locale; update the Layout component
(where metadata and lang are used) and ensure you populate
metadata.alternates/metadataBase per-locale and cover all locales listed in
web/i18n/routing.ts and corresponding web/messages/* entries.
In `@web/app/devbox/page.tsx`:
- Around line 8-18: The header currently hardcodes user-facing strings
("devbox.new" inside the Link and "cmux" inside the anchor) in
web/app/devbox/page.tsx; replace these literals by loading the locale messages
and using the appropriate message keys (e.g., a key for the page title and one
for the partner link) via your i18n helper used elsewhere in the app, update
web/i18n/routing.ts to include the new route’s message keys if required, and add
the corresponding translations in every file under web/messages/ for each locale
so the Link and anchor render locale-backed text instead of hardcoded strings.
In `@web/tests/devbox-proxy.test.ts`:
- Around line 5-15: The test suite for shouldRewriteToDevbox is missing coverage
for the explicit "/devbox" allowlist branch in web/devbox-routing.ts; add
positive assertions that shouldRewriteToDevbox("devbox.new", "/devbox") (and
similarly for "www.devbox.new" and "devbox.new:443") return true so the explicit
branch is exercised and protected from regression.
🪄 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: 1d597cbe-d71e-4bc9-bbee-6014583a7994
📒 Files selected for processing (6)
web/app/devbox/devbox-creator.tsxweb/app/devbox/layout.tsxweb/app/devbox/page.tsxweb/devbox-routing.tsweb/proxy.tsweb/tests/devbox-proxy.test.ts
| function userMessage(status: number, body: VmErrorBody | null) { | ||
| if (status === 401) { | ||
| return "Sign in first, then create the devbox again."; | ||
| } | ||
|
|
||
| const pieces = [body?.message, body?.action ?? body?.reason].filter( | ||
| Boolean, | ||
| ); | ||
| return pieces.join(" ") || `Devbox create failed with status ${status}.`; | ||
| } |
There was a problem hiding this comment.
Localize all form/error/success copy and map API errors to locale keys.
Line 31, Line 37, Line 79, and Line 89-Line 155 hardcode user-visible strings. Also, Line 34-Line 37 directly surfaces backend message/action/reason, which are not locale-resolved in this client flow. This route should read UI copy from next-intl (or equivalent) and map API error codes to localized keys for every supported locale.
Suggested direction
const t = useTranslations("devbox");
setError(t("errors.unauthenticated"));
// map body.error/status => t("errors.<key>")
<h1>{t("title")}</h1>
<textarea placeholder={t("promptPlaceholder")} />
<button>{isCreating ? t("creating") : t("create")}</button>As per coding guidelines: “Web UI, API response copy, and user-facing web data must use locale-specific sources and be represented across all locales in web/i18n/routing.ts and web/messages/.”
Also applies to: 79-79, 85-159
🤖 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 `@web/app/devbox/devbox-creator.tsx` around lines 29 - 38, Replace hardcoded
strings in userMessage with localized messages: import and call
useTranslations("devbox") in the component that calls userMessage, change
userMessage(status: number, body: VmErrorBody | null) to accept a translator (or
move its logic into the component) and map status/body to locale keys (e.g.
t("errors.unauthenticated") for 401, t("errors.<mapped_key>") for known API
error codes or body.action/body.reason), avoid directly surface
body.message/action/reason to users, and fall back to t("errors.generic", {
status }) when no mapping exists; ensure translation keys
(errors.unauthenticated, errors.generic, and specific mapped keys) are added to
the devbox namespace in your i18n messages and routing.
Source: Coding guidelines
| export const metadata: Metadata = { | ||
| title: "devbox.new", | ||
| description: "Create a new cmux devbox from a prompt.", | ||
| metadataBase: new URL("https://devbox.new"), | ||
| alternates: { | ||
| canonical: "https://devbox.new", | ||
| }, | ||
| openGraph: { | ||
| title: "devbox.new", | ||
| description: "Create a new cmux devbox from a prompt.", | ||
| url: "https://devbox.new", | ||
| siteName: "devbox.new", | ||
| type: "website", | ||
| }, | ||
| }; |
There was a problem hiding this comment.
Localize route metadata and language attributes.
Line 16-Line 27 and Line 37 hardcode English metadata and lang="en", so this route cannot reflect active locale. For web/** user-facing surfaces, metadata and language context should come from locale-specific sources and be covered across all locales configured in web/i18n/routing.ts and web/messages/*.
As per coding guidelines: “Web UI, API responses, and user-facing data must use locale-specific sources … update all locales listed in web/i18n/routing.ts and every matching file in web/messages/,” and “All user-facing strings must be localized.”
Also applies to: 37-37
🤖 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 `@web/app/devbox/layout.tsx` around lines 15 - 29, The exported metadata object
(metadata) and the hardcoded lang="en" in web/app/devbox/layout.tsx must be made
locale-aware: load the current locale from your routing helper
(web/i18n/routing.ts) or next/navigation params, fetch localized
title/description from web/messages/<locale> (or the app i18n utility), and
build metadata (title, description, openGraph.url/siteName/alternates) and the
html lang attribute dynamically using those localized strings and the active
locale; update the Layout component (where metadata and lang are used) and
ensure you populate metadata.alternates/metadataBase per-locale and cover all
locales listed in web/i18n/routing.ts and corresponding web/messages/* entries.
Source: Coding guidelines
| <header className="flex items-center justify-between gap-4 text-sm"> | ||
| <Link href="/" className="font-semibold tracking-tight"> | ||
| devbox.new | ||
| </Link> | ||
| <a | ||
| href="https://cmux.com" | ||
| className="text-muted transition-colors hover:text-foreground" | ||
| > | ||
| cmux | ||
| </a> | ||
| </header> |
There was a problem hiding this comment.
Move page header copy to locale-backed messages.
Line 10 and Line 16 introduce user-facing text as hardcoded literals in JSX. This bypasses the locale message source and leaves the new /devbox page incomplete for non-default locales.
As per coding guidelines: “Web UI … must use locale-specific sources … and update all locales listed in web/i18n/routing.ts and every matching file in web/messages/.”
🤖 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 `@web/app/devbox/page.tsx` around lines 8 - 18, The header currently hardcodes
user-facing strings ("devbox.new" inside the Link and "cmux" inside the anchor)
in web/app/devbox/page.tsx; replace these literals by loading the locale
messages and using the appropriate message keys (e.g., a key for the page title
and one for the partner link) via your i18n helper used elsewhere in the app,
update web/i18n/routing.ts to include the new route’s message keys if required,
and add the corresponding translations in every file under web/messages/ for
each locale so the Link and anchor render locale-backed text instead of
hardcoded strings.
Source: Coding guidelines
| test("rewrites the devbox.new homepage to the devbox creator", () => { | ||
| expect(shouldRewriteToDevbox("devbox.new", "/")).toBe(true); | ||
| expect(shouldRewriteToDevbox("www.devbox.new", "/")).toBe(true); | ||
| expect(shouldRewriteToDevbox("devbox.new:443", "/")).toBe(true); | ||
| }); | ||
|
|
||
| test("does not rewrite cmux.com or API paths", () => { | ||
| expect(shouldRewriteToDevbox("cmux.com", "/")).toBe(false); | ||
| expect(shouldRewriteToDevbox("devbox.new", "/api/vm")).toBe(false); | ||
| expect(shouldRewriteToDevbox("devbox.new", "/handler/sign-in")).toBe(false); | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Add coverage for the explicit /devbox allowlist branch.
Line 6-9 only verifies /, but shouldRewriteToDevbox also explicitly returns true for /devbox (web/devbox-routing.ts Line 6). Add a positive assertion for that branch to prevent silent regression.
Suggested test diff
describe("devbox.new host routing", () => {
test("rewrites the devbox.new homepage to the devbox creator", () => {
expect(shouldRewriteToDevbox("devbox.new", "/")).toBe(true);
expect(shouldRewriteToDevbox("www.devbox.new", "/")).toBe(true);
expect(shouldRewriteToDevbox("devbox.new:443", "/")).toBe(true);
+ expect(shouldRewriteToDevbox("devbox.new", "/devbox")).toBe(true);
});🤖 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 `@web/tests/devbox-proxy.test.ts` around lines 5 - 15, The test suite for
shouldRewriteToDevbox is missing coverage for the explicit "/devbox" allowlist
branch in web/devbox-routing.ts; add positive assertions that
shouldRewriteToDevbox("devbox.new", "/devbox") (and similarly for
"www.devbox.new" and "devbox.new:443") return true so the explicit branch is
exercised and protected from regression.
|
All contributors have signed the CLA ✍️ ✅ |
|
Fleet instruction update for head |
teamleaderleo
added
area: cloud
CI failure attributionCI passes on Written by |
|
This is red for a reason that has nothing to do with the change. Both this and #5341 fail only Those ten files and The cause is
This PR was updated today, so that is not staleness from sitting idle. And because the branch has merged main in since, Verified against the real module in a scratch repo, driving it from both inputs: So the validator is correct and is being handed a bad baseline. A fix is in flight on Note — Raindrop g2 🫧 / Run: run_worker_20260930_3fc64ba6 |
This branch was successfully deployed
2 participants
Summary
/devboxcreator page fordevbox.new.devbox.newandwww.devbox.newhomepage requests to/devboxwhile preserving existing cmux proxy behavior.POST /api/vmwith a source marker and show the created VM id plus attach commands.Verification
bun test tests/devbox-proxy.test.tsbun run lint(passes with existing warnings outside this change)VERCEL_ENV=preview bun run buildcurl -fsS -H 'Host: devbox.new' http://localhost:4117/rendersdevbox.newand the create formPOST /api/vmreturns401, which the page maps to sign-in guidanceNotes
The Cloud VM backend currently ignores
initialPrompt; this page sends it for forward compatibility, but the current shipped behavior is create-first and attach from cmux withcmux vm attach <id>orcmux vm ssh <id>.Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Note
Medium Risk
Touches request routing for a new production host and triggers authenticated VM provisioning via /api/vm; scope is limited but mis-routing could affect devbox.new traffic.
Overview
Adds a devbox.new landing experience: a standalone
/devboxroute with its own layout/metadata and a client DevboxCreator form that POSTs to the existing/api/vmendpoint (optionalinitialPrompt,source: "devbox.new", idempotency header), surfaces auth/errors, and shows attach/SSH CLI hints after success.Host routing is extended via
shouldRewriteToDevboxand an early rewrite inproxy: requests todevbox.new/www.devbox.newon/or/devboxare rewritten to/devbox, while/api/*and sign-in paths are left alone. Unit tests cover the routing helper.Reviewed by Cursor Bugbot for commit 1fed97b. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Adds a standalone
/devboxcreator and changesdevbox.newandwww.devbox.newhomepages from the localized cmux site to it, while leaving/api/*and sign-in paths untouched. Users can optionally provide a prompt, create an authenticated Cloud VM, and receive its ID with cmux attach and SSH commands.POST /api/vmwith an idempotency key andsource: "devbox.new"; 401 responses link to sign-in and other errors are shown in the form.initialPrompt, so the prompt is forwarded for compatibility but does not affect provisioning yet.Written for commit 592efd1. Summary will update on new commits.
Summary by CodeRabbit
Release Notes