add cloud vm integration with yolocode e2b sandboxes #60
Conversation
WalkthroughAdds cloud sandbox support: a new cloud API client, IPC handlers for creating/opening/deleting sandboxes, CloudSandbox types and IPC channel entries, and UI integrations for creating, opening, and killing cloud worktrees/tabs. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant UI as Renderer (Sidebar/TaskTabs)
participant Main as Main Process
participant GH as GitHub CLI
participant API as Cloud API
User->>UI: Click "New Cloud Worktree"
UI->>Main: IPC: worktree-create-cloud-sandbox
Main->>GH: gh auth token request
GH-->>Main: GH token
Main->>API: POST /sandbox (repo, envVars, template)
API-->>Main: sandbox { id, claudeHost, websshHost, ... }
Main->>Main: persist worktree.cloudSandbox
Main-->>UI: { success, sandbox }
UI->>UI: open preview tab (claudeHost)
sequenceDiagram
actor User
participant UI as Renderer (TabItem)
participant Main as Main Process
participant API as Cloud API
User->>UI: "Kill VM" or Remove Worktree
UI->>Main: IPC: cloud-sandbox-delete-by-id (sandboxId)
Main->>API: DELETE /sandbox/{id}
API-->>Main: { success }
Main->>Main: clear worktree.cloudSandbox
Main->>Main: IPC: worktree-remove
Main-->>UI: worktree removed
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![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
🧹 Nitpick comments (7)
apps/desktop/src/renderer/screens/main/components/NewLayout/TaskTabs.tsx (1)
70-421: Gate the “Open Cloud” button on a ready endpointRight now the “Open Cloud” action renders as soon as any
cloudSandboxobject exists, even while the backend is still provisioning and hasn’t provided aclaudeHost. Clicking it in that state immediately trips the IPC handler’s “No cloud sandbox found” error path and forces the user through an alert loop. Guard onclaudeHost(or show a disabled “Provisioning…” button) so we don’t lead people into a guaranteed failure.- const hasCloudSandbox = selectedWorktree && selectedWorktree.cloudSandbox; + const cloudSandbox = selectedWorktree?.cloudSandbox; + const hasCloudSandbox = Boolean(cloudSandbox); + const canOpenCloudSandbox = Boolean(cloudSandbox?.claudeHost); … - {hasCloudSandbox && onOpenCloudSandbox ? ( + {canOpenCloudSandbox && onOpenCloudSandbox ? ( <Tooltip> <TooltipTrigger asChild> <Button variant="secondary" size="sm" onClick={onOpenCloudSandbox} className="h-7" > <Cloud size={14} className="mr-1.5" /> Open Cloud </Button> </TooltipTrigger> <TooltipContent side="bottom"> <p>Open cloud sandbox in browser</p> </TooltipContent> </Tooltip> - ) : onCreateCloudSandbox && selectedWorktree ? ( + ) : hasCloudSandbox ? ( + <Tooltip> + <TooltipTrigger asChild> + <Button + variant="secondary" + size="sm" + disabled + className="h-7" + > + <Loader2 size={14} className="mr-1.5 animate-spin" /> + Provisioning… + </Button> + </TooltipTrigger> + <TooltipContent side="bottom"> + <p>Sandbox is still provisioning; Claude endpoint not ready yet</p> + </TooltipContent> + </Tooltip> + ) : onCreateCloudSandbox && selectedWorktree ? (apps/desktop/src/main/lib/cloud-api-client.ts (4)
37-37: Consider making the API URL configurable.The staging URL is hardcoded, which makes it difficult to switch environments or deploy to production.
Apply this diff to make it configurable:
- private baseUrl = "https://staging.yolocode.ai/api/e2b-sandboxes"; + private baseUrl = process.env.YOLOCODE_API_URL || "https://staging.yolocode.ai/api/e2b-sandboxes";
109-110: Clarify or configure the port override to 7030.The port replacement to 7030 appears to be a magic number with minimal documentation ("for web UI"). Consider adding more explanation or making this configurable.
Apply this diff to improve clarity:
- // Override claudeHost to use port 7030 for web UI + // Override claudeHost to use port 7030 for the Claude web UI proxy + // This port is expected by the desktop app's renderer process const claudeHost = data.claudeHost?.replace(/:\d+/, ":7030") || data.claudeHost;
86-93: Add timeout to fetch calls.The fetch call has no timeout configured, which could cause the application to hang indefinitely if the API is unresponsive.
Apply this diff to add a timeout:
+ const controller = new AbortController(); + const timeoutId = setTimeout(() => controller.abort(), 30000); // 30 second timeout + const response = await fetch(this.baseUrl, { method: "POST", headers: { Authorization: `Bearer ${token}`, "Content-Type": "application/json", }, body: JSON.stringify(requestBody), + signal: controller.signal, }); + + clearTimeout(timeoutId);
148-153: Add timeout to fetch call.Similar to the
createSandboxmethod, this fetch call lacks a timeout and could hang indefinitely.Apply this diff to add a timeout:
+ const controller = new AbortController(); + const timeoutId = setTimeout(() => controller.abort(), 30000); + const response = await fetch(`${this.baseUrl}/${sandboxId}`, { method: "DELETE", headers: { Authorization: `Bearer ${token}`, }, + signal: controller.signal, }); + + clearTimeout(timeoutId);apps/desktop/src/renderer/screens/main/components/Sidebar/components/CreateWorktreeButton/CreateWorktreeButton.tsx (2)
27-34: Consider showing disabled state more clearly.The primary button is disabled when
isCreatingCloudis true (line 23), but still displays the normal Plus icon and "New Worktree" text (lines 27-34). Users may not understand why the button is disabled.This might be acceptable if the cloud button provides sufficient visual feedback, but consider whether the UX could be clearer.
37-53: Consider adding accessibility attributes.The cloud button (and the primary button) lack ARIA attributes to indicate loading state or provide clearer labels for screen readers.
Consider adding:
<Button variant="ghost" size="sm" onClick={onCreateCloud} disabled={isCreating || isCreatingCloud} + aria-busy={isCreatingCloud} + aria-label="Create new cloud worktree" className="flex-1 h-7 px-2.5 font-normal text-xs border border-dashed border-blue-700/50 hover:bg-blue-800/40 hover:border-blue-600 text-blue-400 hover:text-blue-300 gap-1.5" style={{ justifyContent: "flex-start" }} >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (8)
apps/desktop/src/main/lib/cloud-api-client.ts(1 hunks)apps/desktop/src/main/lib/workspace-ipcs.ts(2 hunks)apps/desktop/src/renderer/screens/main/components/NewLayout/NewLayoutMain.tsx(3 hunks)apps/desktop/src/renderer/screens/main/components/NewLayout/TaskTabs.tsx(8 hunks)apps/desktop/src/renderer/screens/main/components/Sidebar/Sidebar.tsx(3 hunks)apps/desktop/src/renderer/screens/main/components/Sidebar/components/CreateWorktreeButton/CreateWorktreeButton.tsx(1 hunks)apps/desktop/src/shared/ipc-channels.ts(2 hunks)apps/desktop/src/shared/types.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
apps/desktop/src/main/lib/workspace-ipcs.ts (2)
apps/desktop/src/main/lib/workspace-manager.ts (1)
workspaceManager(481-481)apps/desktop/src/main/lib/cloud-api-client.ts (1)
cloudApiClient(173-173)
apps/desktop/src/main/lib/cloud-api-client.ts (1)
apps/desktop/src/shared/types.ts (1)
CloudSandbox(59-67)
apps/desktop/src/renderer/screens/main/components/Sidebar/Sidebar.tsx (1)
apps/desktop/src/renderer/screens/main/components/Sidebar/components/CreateWorktreeButton/CreateWorktreeButton.tsx (1)
CreateWorktreeButton(11-56)
apps/desktop/src/renderer/screens/main/components/NewLayout/TaskTabs.tsx (1)
apps/desktop/src/renderer/screens/main/components/NewLayout/StatusIndicator.tsx (1)
StatusIndicator(53-113)
| private getGithubToken(): string | null { | ||
| try { | ||
| const token = execSync("gh auth token", { | ||
| encoding: "utf-8", | ||
| stdio: ["pipe", "pipe", "pipe"], | ||
| }).trim(); | ||
| return token; | ||
| } catch (error) { | ||
| console.error("Failed to get GitHub token:", error); | ||
| return null; | ||
| } | ||
| } |
There was a problem hiding this comment.
Use async alternatives to avoid blocking the main thread.
execSync blocks the Electron main process, freezing the UI while waiting for the gh CLI command to complete. In desktop applications, this creates a poor user experience.
Refactor to use async exec from node:child_process/promises:
+import { exec } from "node:child_process";
+import { promisify } from "node:util";
+
+const execAsync = promisify(exec);
+
/**
* Get GitHub token from gh CLI
*/
- private getGithubToken(): string | null {
+ private async getGithubToken(): Promise<string | null> {
try {
- const token = execSync("gh auth token", {
+ const { stdout } = await execAsync("gh auth token", {
encoding: "utf-8",
- stdio: ["pipe", "pipe", "pipe"],
- }).trim();
- return token;
+ });
+ return stdout.trim();
} catch (error) {
console.error("Failed to get GitHub token:", error);
return null;
}
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/desktop/src/main/lib/cloud-api-client.ts around lines 42 to 53, the sync
execSync call blocks the Electron main thread; change getGithubToken to an async
function that uses exec from node:child_process/promises, await exec("gh auth
token"), read stdout.trim() and return it (or null on error), catch and log
errors, and update any callers to await the new Promise-returning getGithubToken
so the main process is non-blocking.
| const sandbox: CloudSandbox = { | ||
| id: data.id, | ||
| name: data.name, | ||
| status: "running", | ||
| websshHost: data.websshHost, | ||
| claudeHost: claudeHost, | ||
| createdAt: data.createdAt, | ||
| }; |
There was a problem hiding this comment.
Use actual API response status instead of hardcoding "running".
Line 115 hardcodes status: "running" regardless of the actual status returned by the API (data.status). This could mask sandbox creation failures or delays.
Apply this diff to use the actual status:
const sandbox: CloudSandbox = {
id: data.id,
name: data.name,
- status: "running",
+ status: data.status as CloudSandbox["status"],
websshHost: data.websshHost,
claudeHost: claudeHost,
createdAt: data.createdAt,
};📝 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 sandbox: CloudSandbox = { | |
| id: data.id, | |
| name: data.name, | |
| status: "running", | |
| websshHost: data.websshHost, | |
| claudeHost: claudeHost, | |
| createdAt: data.createdAt, | |
| }; | |
| const sandbox: CloudSandbox = { | |
| id: data.id, | |
| name: data.name, | |
| status: data.status as CloudSandbox["status"], | |
| websshHost: data.websshHost, | |
| claudeHost: claudeHost, | |
| createdAt: data.createdAt, | |
| }; |
🤖 Prompt for AI Agents
In apps/desktop/src/main/lib/cloud-api-client.ts around lines 112 to 119, the
sandbox object incorrectly hardcodes status: "running"; change it to use the API
response by assigning status: data.status (and optionally fallback to a safe
value like 'unknown' if data.status is missing) so the client reflects the real
sandbox state returned by the server.
| const handleCreateCloudWorktree = async () => { | ||
| if (!currentWorkspace) return; | ||
|
|
||
| // Generate random two-word name | ||
| const adjectives = [ | ||
| "happy", | ||
| "sleepy", | ||
| "brave", | ||
| "clever", | ||
| "gentle", | ||
| "bright", | ||
| "calm", | ||
| "bold", | ||
| "swift", | ||
| "quiet", | ||
| ]; | ||
| const nouns = [ | ||
| "cat", | ||
| "fox", | ||
| "owl", | ||
| "bear", | ||
| "wolf", | ||
| "deer", | ||
| "hawk", | ||
| "lynx", | ||
| "seal", | ||
| "dove", | ||
| ]; | ||
| const randomAdj = adjectives[Math.floor(Math.random() * adjectives.length)]; | ||
| const randomNoun = nouns[Math.floor(Math.random() * nouns.length)]; | ||
| const randomName = `${randomAdj}-${randomNoun}`; | ||
|
|
||
| // For now, create a simple worktree and immediately create a cloud sandbox for it | ||
| const title = `Cloud ${randomAdj} ${randomNoun}`; | ||
| const branch = `cloud-dev-${randomName}`; | ||
|
|
||
| try { | ||
| setIsCreatingCloudWorktree(true); | ||
|
|
||
| // Create worktree | ||
| const result = await window.ipcRenderer.invoke("worktree-create", { | ||
| workspaceId: currentWorkspace.id, | ||
| title, | ||
| branch, | ||
| createBranch: true, | ||
| description: "Cloud development environment", | ||
| }); | ||
|
|
||
| if (result.success && result.worktree) { | ||
| // Immediately create cloud sandbox for this worktree | ||
| const sandboxResult = await window.ipcRenderer.invoke( | ||
| "worktree-create-cloud-sandbox", | ||
| { | ||
| workspaceId: currentWorkspace.id, | ||
| worktreeId: result.worktree.id, | ||
| }, | ||
| ); | ||
|
|
||
| if (sandboxResult.success && sandboxResult.sandbox?.claudeHost) { | ||
| // Create a preview tab with the claude host URL | ||
| const claudeUrl = sandboxResult.sandbox.claudeHost.startsWith("http") | ||
| ? sandboxResult.sandbox.claudeHost | ||
| : `https://${sandboxResult.sandbox.claudeHost}`; | ||
|
|
||
| const tabResult = await window.ipcRenderer.invoke("tab-create", { | ||
| workspaceId: currentWorkspace.id, | ||
| worktreeId: result.worktree.id, | ||
| name: "Cloud IDE", | ||
| type: "preview", | ||
| url: claudeUrl, | ||
| }); | ||
|
|
||
| console.log("Tab creation result:", tabResult); | ||
|
|
||
| // Expand the worktree and select the newly created tab | ||
| if (tabResult.success && tabResult.tab && result.worktree) { | ||
| setExpandedWorktrees((prev) => { | ||
| const next = new Set(prev); | ||
| next.add(result.worktree!.id); | ||
| return next; | ||
| }); | ||
| onTabSelect(result.worktree.id, tabResult.tab.id); | ||
| } | ||
| } | ||
|
|
||
| // Refresh UI after everything is created | ||
| onWorktreeCreated(); | ||
| } else { | ||
| alert( | ||
| `Failed to create cloud worktree: ${result.error || "Unknown error"}`, | ||
| ); | ||
| } | ||
| } catch (error) { | ||
| const errorMessage = | ||
| error instanceof Error ? error.message : String(error); | ||
| alert(`Failed to create cloud worktree: ${errorMessage}`); | ||
| } finally { | ||
| setIsCreatingCloudWorktree(false); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Surface sandbox creation failures and roll back the optimistic worktree
If worktree-create-cloud-sandbox fails (network, quota, auth, etc.), we skip the if (sandboxResult.success …) block but still fall through as if everything worked: the brand-new worktree sticks around, the user gets no feedback, and there’s no sandbox attached. That’s a high-impact UX bug—“New Cloud Worktree” completes with nothing cloud about it, and we leave behind an extra local branch.
Treat the sandbox call as part of the transactional flow: surface the error and undo the optimistic worktree so the UI state matches reality.
const sandboxResult = await window.ipcRenderer.invoke(
"worktree-create-cloud-sandbox",
{
workspaceId: currentWorkspace.id,
worktreeId: result.worktree.id,
},
);
+ if (!sandboxResult.success) {
+ alert(
+ `Failed to create cloud sandbox: ${
+ sandboxResult.error || "Unknown error"
+ }`,
+ );
+ await window.ipcRenderer.invoke("worktree-remove", {
+ workspaceId: currentWorkspace.id,
+ worktreeId: result.worktree.id,
+ });
+ onWorktreeCreated();
+ return;
+ }
+
if (sandboxResult.success && sandboxResult.sandbox?.claudeHost) {
// Create a preview tab with the claude host URL
const claudeUrl = sandboxResult.sandbox.claudeHost.startsWith("http")
? sandboxResult.sandbox.claudeHost
: `https://${sandboxResult.sandbox.claudeHost}`;📝 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 handleCreateCloudWorktree = async () => { | |
| if (!currentWorkspace) return; | |
| // Generate random two-word name | |
| const adjectives = [ | |
| "happy", | |
| "sleepy", | |
| "brave", | |
| "clever", | |
| "gentle", | |
| "bright", | |
| "calm", | |
| "bold", | |
| "swift", | |
| "quiet", | |
| ]; | |
| const nouns = [ | |
| "cat", | |
| "fox", | |
| "owl", | |
| "bear", | |
| "wolf", | |
| "deer", | |
| "hawk", | |
| "lynx", | |
| "seal", | |
| "dove", | |
| ]; | |
| const randomAdj = adjectives[Math.floor(Math.random() * adjectives.length)]; | |
| const randomNoun = nouns[Math.floor(Math.random() * nouns.length)]; | |
| const randomName = `${randomAdj}-${randomNoun}`; | |
| // For now, create a simple worktree and immediately create a cloud sandbox for it | |
| const title = `Cloud ${randomAdj} ${randomNoun}`; | |
| const branch = `cloud-dev-${randomName}`; | |
| try { | |
| setIsCreatingCloudWorktree(true); | |
| // Create worktree | |
| const result = await window.ipcRenderer.invoke("worktree-create", { | |
| workspaceId: currentWorkspace.id, | |
| title, | |
| branch, | |
| createBranch: true, | |
| description: "Cloud development environment", | |
| }); | |
| if (result.success && result.worktree) { | |
| // Immediately create cloud sandbox for this worktree | |
| const sandboxResult = await window.ipcRenderer.invoke( | |
| "worktree-create-cloud-sandbox", | |
| { | |
| workspaceId: currentWorkspace.id, | |
| worktreeId: result.worktree.id, | |
| }, | |
| ); | |
| if (sandboxResult.success && sandboxResult.sandbox?.claudeHost) { | |
| // Create a preview tab with the claude host URL | |
| const claudeUrl = sandboxResult.sandbox.claudeHost.startsWith("http") | |
| ? sandboxResult.sandbox.claudeHost | |
| : `https://${sandboxResult.sandbox.claudeHost}`; | |
| const tabResult = await window.ipcRenderer.invoke("tab-create", { | |
| workspaceId: currentWorkspace.id, | |
| worktreeId: result.worktree.id, | |
| name: "Cloud IDE", | |
| type: "preview", | |
| url: claudeUrl, | |
| }); | |
| console.log("Tab creation result:", tabResult); | |
| // Expand the worktree and select the newly created tab | |
| if (tabResult.success && tabResult.tab && result.worktree) { | |
| setExpandedWorktrees((prev) => { | |
| const next = new Set(prev); | |
| next.add(result.worktree!.id); | |
| return next; | |
| }); | |
| onTabSelect(result.worktree.id, tabResult.tab.id); | |
| } | |
| } | |
| // Refresh UI after everything is created | |
| onWorktreeCreated(); | |
| } else { | |
| alert( | |
| `Failed to create cloud worktree: ${result.error || "Unknown error"}`, | |
| ); | |
| } | |
| } catch (error) { | |
| const errorMessage = | |
| error instanceof Error ? error.message : String(error); | |
| alert(`Failed to create cloud worktree: ${errorMessage}`); | |
| } finally { | |
| setIsCreatingCloudWorktree(false); | |
| } | |
| }; | |
| const handleCreateCloudWorktree = async () => { | |
| if (!currentWorkspace) return; | |
| // Generate random two-word name | |
| const adjectives = [ | |
| "happy", | |
| "sleepy", | |
| "brave", | |
| "clever", | |
| "gentle", | |
| "bright", | |
| "calm", | |
| "bold", | |
| "swift", | |
| "quiet", | |
| ]; | |
| const nouns = [ | |
| "cat", | |
| "fox", | |
| "owl", | |
| "bear", | |
| "wolf", | |
| "deer", | |
| "hawk", | |
| "lynx", | |
| "seal", | |
| "dove", | |
| ]; | |
| const randomAdj = adjectives[Math.floor(Math.random() * adjectives.length)]; | |
| const randomNoun = nouns[Math.floor(Math.random() * nouns.length)]; | |
| const randomName = `${randomAdj}-${randomNoun}`; | |
| // For now, create a simple worktree and immediately create a cloud sandbox for it | |
| const title = `Cloud ${randomAdj} ${randomNoun}`; | |
| const branch = `cloud-dev-${randomName}`; | |
| try { | |
| setIsCreatingCloudWorktree(true); | |
| // Create worktree | |
| const result = await window.ipcRenderer.invoke("worktree-create", { | |
| workspaceId: currentWorkspace.id, | |
| title, | |
| branch, | |
| createBranch: true, | |
| description: "Cloud development environment", | |
| }); | |
| if (result.success && result.worktree) { | |
| // Immediately create cloud sandbox for this worktree | |
| const sandboxResult = await window.ipcRenderer.invoke( | |
| "worktree-create-cloud-sandbox", | |
| { | |
| workspaceId: currentWorkspace.id, | |
| worktreeId: result.worktree.id, | |
| }, | |
| ); | |
| if (!sandboxResult.success) { | |
| alert( | |
| `Failed to create cloud sandbox: ${ | |
| sandboxResult.error || "Unknown error" | |
| }`, | |
| ); | |
| await window.ipcRenderer.invoke("worktree-remove", { | |
| workspaceId: currentWorkspace.id, | |
| worktreeId: result.worktree.id, | |
| }); | |
| onWorktreeCreated(); | |
| return; | |
| } | |
| if (sandboxResult.success && sandboxResult.sandbox?.claudeHost) { | |
| // Create a preview tab with the claude host URL | |
| const claudeUrl = sandboxResult.sandbox.claudeHost.startsWith("http") | |
| ? sandboxResult.sandbox.claudeHost | |
| : `https://${sandboxResult.sandbox.claudeHost}`; | |
| const tabResult = await window.ipcRenderer.invoke("tab-create", { | |
| workspaceId: currentWorkspace.id, | |
| worktreeId: result.worktree.id, | |
| name: "Cloud IDE", | |
| type: "preview", | |
| url: claudeUrl, | |
| }); | |
| console.log("Tab creation result:", tabResult); | |
| // Expand the worktree and select the newly created tab | |
| if (tabResult.success && tabResult.tab && result.worktree) { | |
| setExpandedWorktrees((prev) => { | |
| const next = new Set(prev); | |
| next.add(result.worktree!.id); | |
| return next; | |
| }); | |
| onTabSelect(result.worktree.id, tabResult.tab.id); | |
| } | |
| } | |
| // Refresh UI after everything is created | |
| onWorktreeCreated(); | |
| } else { | |
| alert( | |
| `Failed to create cloud worktree: ${result.error || "Unknown error"}`, | |
| ); | |
| } | |
| } catch (error) { | |
| const errorMessage = | |
| error instanceof Error ? error.message : String(error); | |
| alert(`Failed to create cloud worktree: ${errorMessage}`); | |
| } finally { | |
| setIsCreatingCloudWorktree(false); | |
| } | |
| }; |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/desktop/src/renderer/screens/main/components/Sidebar/components/WorktreeList/components/WorktreeItem/components/TabItem/TabItem.tsx (2)
122-152: Replace blocking dialogs with proper UI components.The function uses
window.confirmandalert(), which block the UI thread and provide a dated user experience. Consider using a modal dialog component (e.g., from@superset/ui) for the confirmation prompt and toast notifications for error feedback.Example pattern using a hypothetical dialog component:
const handleKillVM = async () => { if (!workspaceId || !worktreeId) return; // Show non-blocking confirmation dialog const confirmed = await showConfirmDialog({ title: "Delete Cloud Sandbox", message: "Are you sure you want to delete this cloud sandbox? This cannot be undone.", confirmText: "Delete", variant: "destructive" }); if (!confirmed) return; try { const result = await window.ipcRenderer.invoke( "worktree-delete-cloud-sandbox", { workspaceId, worktreeId } ); if (result.success) { onTabRemove?.(tab.id); } else { showToast({ title: "Failed to delete cloud sandbox", description: result.error || "Unknown error", variant: "error" }); } } catch (error) { const errorMessage = error instanceof Error ? error.message : String(error); showToast({ title: "Failed to delete cloud sandbox", description: errorMessage, variant: "error" }); } };
159-163: Extract "Cloud IDE" to a named constant for maintainability.The hard-coded string appears in exactly two places: creation in
Sidebar.tsx:205and the check inTabItem.tsx:162. Without a constant, changing the tab name in one location will silently break the condition in the other, creating a maintenance risk.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/desktop/src/renderer/screens/main/components/Sidebar/components/WorktreeList/components/WorktreeItem/components/TabItem/TabItem.tsx(3 hunks)
🔇 Additional comments (2)
apps/desktop/src/renderer/screens/main/components/Sidebar/components/WorktreeList/components/WorktreeItem/components/TabItem/TabItem.tsx (2)
9-9: LGTM!The Cloud icon import is correctly added and used in the Kill VM menu item.
239-244: LGTM!The Kill VM context menu item is well implemented. The conditional rendering based on
isCloudIDETabis correct, and thetext-red-400styling appropriately signals the destructive nature of this action. The Cloud icon and label are clear and consistent with the existing menu item patterns.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/desktop/src/renderer/screens/main/components/Sidebar/components/WorktreeList/components/WorktreeItem/components/TabItem/TabItem.tsx (1)
159-163: Consider more precise URL matching for e2b.app.The current check uses
includes("e2b.app")which could match unintended URLs like "fake2b.app.com". Consider using a more precise check:const isCloudIDETab = tab.type === "preview" && - tab.url?.includes("e2b.app") && + (tab.url?.includes("://e2b.app") || tab.url?.includes(".e2b.app")) && worktree?.cloudSandbox;This ensures the domain is either the main e2b.app domain or a subdomain of it.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/desktop/src/renderer/screens/main/components/Sidebar/components/WorktreeList/components/WorktreeItem/components/TabItem/TabItem.tsx(3 hunks)
🔇 Additional comments (2)
apps/desktop/src/renderer/screens/main/components/Sidebar/components/WorktreeList/components/WorktreeItem/components/TabItem/TabItem.tsx (2)
9-9: LGTM!The Cloud icon import is properly added and used in the Kill VM context menu item.
239-244: LGTM!The Kill VM context menu item is properly implemented with appropriate styling for a destructive action and correct conditional rendering.
| const handleKillVM = async () => { | ||
| if (!workspaceId || !worktreeId) return; | ||
|
|
||
| const confirmed = window.confirm( | ||
| "Are you sure you want to delete this cloud sandbox? This cannot be undone.", | ||
| ); | ||
| if (!confirmed) return; | ||
|
|
||
| try { | ||
| const result = await window.ipcRenderer.invoke( | ||
| "worktree-delete-cloud-sandbox", | ||
| { | ||
| workspaceId, | ||
| worktreeId, | ||
| }, | ||
| ); | ||
|
|
||
| if (result.success) { | ||
| // Close the tab after successful deletion | ||
| onTabRemove?.(tab.id); | ||
| } else { | ||
| alert( | ||
| `Failed to delete cloud sandbox: ${result.error || "Unknown error"}`, | ||
| ); | ||
| } | ||
| } catch (error) { | ||
| const errorMessage = | ||
| error instanceof Error ? error.message : String(error); | ||
| alert(`Failed to delete cloud sandbox: ${errorMessage}`); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Replace blocking dialogs with proper UI components and add loading state.
The implementation has several UX and reliability concerns:
window.confirmandalertare blocking and provide poor user experience- No loading/disabled state during the async operation allows users to trigger multiple concurrent deletions
- The IPC channel name should be imported from
ipc-channels.tsinstead of hardcoded - Missing TypeScript types for the IPC response structure
Consider this refactor:
+import { IPC_CHANNELS } from 'shared/ipc-channels';
+
+// At the component level, add loading state:
+const [isDeleting, setIsDeleting] = useState(false);
+
const handleKillVM = async () => {
if (!workspaceId || !worktreeId) return;
+ if (isDeleting) return; // Prevent concurrent operations
- const confirmed = window.confirm(
- "Are you sure you want to delete this cloud sandbox? This cannot be undone.",
- );
- if (!confirmed) return;
+ // TODO: Replace with a proper modal component
+ // For now, keeping window.confirm but noting the limitation
+ const confirmed = window.confirm(
+ "Are you sure you want to delete this cloud sandbox? This will also close this tab and cannot be undone.",
+ );
+ if (!confirmed) return;
+ setIsDeleting(true);
try {
- const result = await window.ipcRenderer.invoke(
- "worktree-delete-cloud-sandbox",
+ const result: { success: boolean; error?: string } = await window.ipcRenderer.invoke(
+ IPC_CHANNELS.WORKTREE_DELETE_CLOUD_SANDBOX,
{
workspaceId,
worktreeId,
},
);
if (result.success) {
onTabRemove?.(tab.id);
} else {
- alert(
- `Failed to delete cloud sandbox: ${result.error || "Unknown error"}`,
- );
+ // TODO: Replace with a proper toast/notification component
+ console.error('Failed to delete cloud sandbox:', result.error);
+ alert(`Failed to delete cloud sandbox: ${result.error || "Unknown error"}`);
}
} catch (error) {
const errorMessage =
error instanceof Error ? error.message : String(error);
+ console.error('Failed to delete cloud sandbox:', error);
alert(`Failed to delete cloud sandbox: ${errorMessage}`);
+ } finally {
+ setIsDeleting(false);
}
};Also update the Kill VM button to show loading state:
-<ContextMenuItem onClick={handleKillVM} className="text-red-400">
+<ContextMenuItem onClick={handleKillVM} className="text-red-400" disabled={isDeleting}>
<Cloud size={14} className="mr-2" />
- Kill VM
+ {isDeleting ? 'Deleting...' : 'Kill VM'}
</ContextMenuItem>Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
apps/desktop/src/renderer/screens/main/components/Sidebar/components/WorktreeList/components/WorktreeItem/components/TabItem/TabItem.tsx
around lines 122-152, replace the blocking window.confirm/window.alert flows
with non-blocking UI (open a confirm modal and show a toast/snackbar for
errors/success), add a local isDeleting state to disable the Kill VM button and
show a spinner while the IPC call is in progress, import the IPC channel
constant from ipc-channels.ts instead of using the hardcoded
"worktree-delete-cloud-sandbox", and add proper TypeScript types for the IPC
response (e.g., { success: boolean; error?: string }) so you can check
result.success, call onTabRemove(tab.id) on success, and ensure isDeleting is
reset in a finally block while catching and displaying any error message in the
UI toast.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/desktop/src/renderer/screens/main/components/Sidebar/components/WorktreeList/components/WorktreeItem/components/TabItem/TabItem.tsx (1)
122-152: Previous review feedback on this function has not been addressed.The implementation still has the UX and reliability concerns flagged in the earlier review: blocking dialogs (window.confirm/alert), no loading state to prevent concurrent deletions, hardcoded IPC channel string, and missing TypeScript types for the IPC response.
Please refer to the comprehensive fix provided in the previous review comment.
🧹 Nitpick comments (1)
apps/desktop/src/renderer/screens/main/components/Sidebar/components/WorktreeList/components/WorktreeItem/components/TabItem/TabItem.tsx (1)
159-162: Cloud IDE tab detection looks good.The logic correctly identifies cloud sandbox tabs using type and URL pattern matching. The optional chaining safely handles potentially undefined URLs.
If this URL pattern is used elsewhere or expected to change, consider extracting "e2b.app" to a shared constant for maintainability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/desktop/src/renderer/screens/main/components/Sidebar/components/WorktreeList/components/WorktreeItem/components/TabItem/TabItem.tsx(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/desktop/src/renderer/screens/main/components/Sidebar/components/WorktreeList/components/WorktreeItem/components/TabItem/TabItem.tsx (1)
packages/ui/src/components/context-menu.tsx (1)
ContextMenuItem(238-238)
🔇 Additional comments (1)
apps/desktop/src/renderer/screens/main/components/Sidebar/components/WorktreeList/components/WorktreeItem/components/TabItem/TabItem.tsx (1)
238-243: Kill VM context menu item is well-implemented.The conditional rendering, destructive action styling (red text), and Cloud icon provide clear UX for the VM deletion action.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/desktop/src/main/lib/workspace-ipcs.ts (1)
655-724: Consider preventing duplicate sandbox creation.The handler doesn't check if
worktree.cloudSandboxalready exists before creating a new one. Creating multiple sandboxes for the same worktree could lead to orphaned cloud resources and confusion.Add a check before creating the sandbox:
if (!worktree) { return { success: false, error: "Worktree not found", }; } + + // Prevent duplicate sandboxes + if (worktree.cloudSandbox) { + return { + success: false, + error: "Cloud sandbox already exists for this worktree. Delete it first or use the existing one.", + }; + } // Get GitHub repo URL
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/desktop/src/main/lib/workspace-ipcs.ts(2 hunks)apps/desktop/src/renderer/screens/main/components/Sidebar/components/WorktreeList/components/WorktreeItem/components/TabItem/TabItem.tsx(3 hunks)apps/desktop/src/shared/ipc-channels.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/desktop/src/shared/ipc-channels.ts
- apps/desktop/src/renderer/screens/main/components/Sidebar/components/WorktreeList/components/WorktreeItem/components/TabItem/TabItem.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
apps/desktop/src/main/lib/workspace-ipcs.ts (2)
apps/desktop/src/main/lib/workspace-manager.ts (1)
workspaceManager(481-481)apps/desktop/src/main/lib/cloud-api-client.ts (1)
cloudApiClient(173-173)
🔇 Additional comments (4)
apps/desktop/src/main/lib/workspace-ipcs.ts (4)
561-563: LGTM!The debug logging is clear and helpful for tracking PR URL storage.
726-773: LGTM!The handler correctly validates the sandbox exists, constructs a proper URL, and opens it in the browser. Error handling is comprehensive.
775-829: LGTM!The handler correctly validates inputs, deletes the sandbox via API, and only removes the local reference if the deletion succeeds. This prevents config drift if the API call fails.
832-847: LGTM!This standalone deletion handler is useful for cleanup operations and follows the same error handling pattern as the other handlers.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
apps/desktop/src/renderer/screens/main/components/Sidebar/components/WorktreeList/components/WorktreeItem/components/TabItem/TabItem.tsx (1)
122-177: Harden the Kill VM flow before shipping.The Kill VM path still repeats the issues from the earlier review: without a guard/loading state the user can trigger concurrent deletions, the IPC channel is hard-coded, the IPC result is untyped, and we still fall back to blocking
window.confirm/alert. This shows up as double deletions and spurious “failed to delete worktree” alerts when the first call actually succeeded, so we need to keep the existing feedback open. Please add anisKillingSandboxstate to short-circuit concurrent clicks, switch to the shared IPC channel constant, and type the IPC response while replacing the blocking dialogs with the same non-blocking UI pattern we use elsewhere.- const handleKillVM = async () => { - if (!tab.url || !workspaceId || !worktreeId) return; + const handleKillVM = async () => { + if (!tab.url || !workspaceId || !worktreeId || isKillingSandbox) return; + setIsKillingSandbox(true); const confirmed = window.confirm( "Are you sure you want to delete this cloud sandbox and worktree? This cannot be undone.", ); if (!confirmed) return; try { // Extract sandbox ID from URL (format: https://7030-SANDBOX_ID.e2b.app/) const urlMatch = tab.url.match(/\/\/\d+-([^.]+)\.e2b\.app/); const sandboxId = urlMatch?.[1]; if (!sandboxId) { alert("Could not extract sandbox ID from URL"); return; } // Delete the cloud sandbox - const sandboxResult = await window.ipcRenderer.invoke( - "cloud-sandbox-delete-by-id", - { sandboxId }, - ); + const sandboxResult: { success: boolean; error?: string } = + await window.ipcRenderer.invoke( + IPC_CHANNELS.CLOUD_SANDBOX_DELETE_BY_ID, + { sandboxId }, + );Also add
const [isKillingSandbox, setIsKillingSandbox] = useState(false);next to the other local state, replace the blocking confirm/alert calls with our modal/toast components, and disable the menu item whileisKillingSandboxis true so the UI communicates progress.apps/desktop/src/main/lib/cloud-api-client.ts (2)
42-53: Stop blocking the main process when fetching the GitHub token.
execSync("gh auth token")executes on the Electron main thread, freezing the UI whenever we touch the cloud client. Please switch to the async API and await it from call sites.-import { execSync } from "node:child_process"; +import { exec } from "node:child_process/promises"; ... - private getGithubToken(): string | null { + private async getGithubToken(): Promise<string | null> { try { - const token = execSync("gh auth token", { - encoding: "utf-8", - stdio: ["pipe", "pipe", "pipe"], - }).trim(); - return token; + const { stdout } = await exec("gh auth token", { encoding: "utf-8" }); + return stdout.trim(); } catch (error) { console.error("Failed to get GitHub token:", error); return null; } }Remember to
await this.getGithubToken()inside bothcreateSandboxanddeleteSandbox.
119-126: Preserve the actual sandbox status from the API.We’re currently hardcoding every new sandbox to
"running", masking creation errors and pending states. Use the response value (fall back only if the API omitted it).- status: "running", + status: (data.status as CloudSandbox["status"]) ?? "running",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/desktop/src/main/lib/cloud-api-client.ts(1 hunks)apps/desktop/src/renderer/screens/main/components/Sidebar/components/WorktreeList/components/WorktreeItem/WorktreeItem.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/Sidebar/components/WorktreeList/components/WorktreeItem/components/TabItem/TabItem.tsx(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
apps/desktop/src/renderer/screens/main/components/Sidebar/components/WorktreeList/components/WorktreeItem/components/TabItem/TabItem.tsx (1)
packages/ui/src/components/context-menu.tsx (1)
ContextMenuItem(238-238)
apps/desktop/src/main/lib/cloud-api-client.ts (1)
apps/desktop/src/shared/types.ts (1)
CloudSandbox(59-67)
...ens/main/components/Sidebar/components/WorktreeList/components/WorktreeItem/WorktreeItem.tsx
Show resolved
Hide resolved
caffeinum
force-pushed
the
feat/cloud-yolocode
branch
from
3e49082 to
482fca9
Compare
enables one-click creation of cloud coding environments (with claude code) for any worktree via yolocode api
creates worktree + cloud sandbox + preview tab in one click
ensures worktree shows with cloud ide tab already loaded
- add support for passing CLAUDE_CODE_OAUTH_TOKEN from .env to VMs - cloud sandboxes now receive ANTHROPIC_AUTH_TOKEN env var - enables claude code in VMs to authenticate with user credentials - see docs: https://gist.github.com/caffeinum/c1e68a6f8fe66f7bd06b9509559d701a
- kill vm now also deletes the worktree - removing worktree now also kills the cloud sandbox - ensures cleanup is complete in both directions
- extract sandbox ID from tab URLs when cloudSandbox property not set - mask ANTHROPIC_AUTH_TOKEN in logs to prevent leaking credentials - fix env var name: use ANTHROPIC_AUTH_TOKEN instead of CLAUDE_CODE_OAUTH_TOKEN
prevents 'branch already exists' error by appending base36 timestamp
- now passes both CLAUDE_CODE_OAUTH_TOKEN and ANTHROPIC_AUTH_TOKEN - ensures compatibility with different claude code versions
caffeinum
force-pushed
the
feat/cloud-yolocode
branch
from
482fca9 to
da53f04
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop/src/renderer/screens/main/components/Layout/NewLayoutMain.tsx (1)
723-779: Unresolved merge conflict marker will break compilation.Line 758 contains a raw
<<<<<<< HEAD:apps/desktop/src/renderer/screens/main/components/Layout/NewLayoutMain.tsx, which is a leftover merge marker and makes this file invalid TypeScript. Resolve the conflict (choose the correct branch of the logic), remove all<<<<<<<,=======, and>>>>>>>markers, and re-run the build.
♻️ Duplicate comments (5)
apps/desktop/src/main/lib/cloud-api-client.ts (3)
42-52: Avoid blockingexecSyncin the Electron main process for GitHub token retrieval.
getGithubTokenrunsgh auth tokenviaexecSync, and bothcreateSandboxanddeleteSandboxcall it on the main thread; this will freeze the UI whenever the CLI is slow or blocked. Refactor this to use an async child process API and await it from the existing async methods.A possible refactor:
-import { execSync } from "node:child_process"; +import { exec } from "node:child_process"; +import { promisify } from "node:util"; + +const execAsync = promisify(exec); @@ - private getGithubToken(): string | null { + private async getGithubToken(): Promise<string | null> { try { - const token = execSync("gh auth token", { - encoding: "utf-8", - stdio: ["pipe", "pipe", "pipe"], - }).trim(); - return token; + const { stdout } = await execAsync("gh auth token"); + return stdout.trim(); @@ - const token = this.getGithubToken(); + const token = await this.getGithubToken(); @@ - const token = this.getGithubToken(); + const token = await this.getGithubToken();Also applies to: 58-62, 147-151
71-83: Send the Claude token to the VM asANTHROPIC_AUTH_TOKEN, notCLAUDE_CODE_OAUTH_TOKEN.The PR description says the desktop’s
CLAUDE_CODE_OAUTH_TOKENshould be exposed inside the VM asANTHROPIC_AUTH_TOKEN, but the request currently forwards it under the sameCLAUDE_CODE_OAUTH_TOKENkey (and logs that key as well). This means the sandbox won’t see the expected env var.Suggested change:
- const claudeAuthToken = process.env.CLAUDE_CODE_OAUTH_TOKEN; + const claudeAuthToken = process.env.CLAUDE_CODE_OAUTH_TOKEN; @@ - envVars: { - ...params.envVars, - ...(claudeAuthToken && { - CLAUDE_CODE_OAUTH_TOKEN: claudeAuthToken, - }), - }, + envVars: { + ...params.envVars, + ...(claudeAuthToken && { + ANTHROPIC_AUTH_TOKEN: claudeAuthToken, + }), + }, @@ - envVars: claudeAuthToken - ? { CLAUDE_CODE_OAUTH_TOKEN: "***" } - : undefined, + envVars: claudeAuthToken + ? { ANTHROPIC_AUTH_TOKEN: "***" } + : undefined,Also applies to: 86-95
117-130: Use the API’s sandbox status instead of hardcoding"running".The sandbox object always reports
status: "running"regardless of whatCreateSandboxResponse.statuscontains, which can hide creation failures or queued states and mislead the rest of the app.For example:
- const sandbox: CloudSandbox = { - id: data.id, - name: data.name, - status: "running", + const rawStatus = data.status; + const status = + rawStatus === "creating" || + rawStatus === "running" || + rawStatus === "stopped" || + rawStatus === "error" + ? (rawStatus as CloudSandbox["status"]) + : "running"; + + const sandbox: CloudSandbox = { + id: data.id, + name: data.name, + status,apps/desktop/src/renderer/screens/main/components/Sidebar/components/WorktreeList/components/WorktreeItem/components/TabItem/TabItem.tsx (1)
53-53: Kill VM flow is functionally sound but still relies on blocking dialogs and hardcoded IPC channel names.
handleKillVMcorrectly guards against missing ids, tracksisKillingVM, extracts the sandbox id, and sequencescloud-sandbox-delete-by-idfollowed byworktree-remove, with the context menu item disabled and showing a spinner while in progress. However, it still useswindow.confirm/alertand string literals for IPC channel names, and the IPC responses are handled as untypedany, which limits type safety and UX.Consider over time:
- Replacing
window.confirm/alertwith a non-blocking modal + toast/notification pattern.- Importing IPC channel constants (e.g., from
shared/ipc-channels/worktree) instead of"cloud-sandbox-delete-by-id"/"worktree-remove"string literals.- Adding an explicit TypeScript type for the expected IPC response (
{ success: boolean; error?: string }) and casting the results once, instead of runtimetypeofchecks.Also applies to: 130-188, 195-199, 272-284
apps/desktop/src/renderer/screens/main/components/Sidebar/Sidebar.tsx (1)
182-282: Handle cloud sandbox creation failures transactionally with the new worktree.If
worktree-create-cloud-sandboxfails (quota, auth, network, etc.), the code currently skips the happy path but still leaves the freshly created worktree and branch in place, with no cloud sandbox attached and only a generic alert on worktree failures. That mismatches the “New Cloud Worktree” UX and repeats the earlier concern about surfacing sandbox errors and rolling back optimistic state.A more robust flow:
if (result.success && result.worktree) { // Immediately create cloud sandbox for this worktree const sandboxResult = await window.ipcRenderer.invoke( "worktree-create-cloud-sandbox", @@ - if (sandboxResult.success && sandboxResult.sandbox?.claudeHost) { + if (!sandboxResult.success) { + alert( + `Failed to create cloud sandbox: ${ + sandboxResult.error || "Unknown error" + }`, + ); + // Roll back the just-created worktree so UI and git state stay in sync + await window.ipcRenderer.invoke("worktree-remove", { + workspaceId: currentWorkspace.id, + worktreeId: result.worktree.id, + }); + onWorktreeCreated(); + return; + } + + if (sandboxResult.sandbox?.claudeHost) { // Create a preview tab with the claude host URL const claudeUrl = sandboxResult.sandbox.claudeHost.startsWith("http") ? sandboxResult.sandbox.claudeHost @@ - // Refresh UI after everything is created - onWorktreeCreated(); + // Refresh UI after everything is created + onWorktreeCreated();(Feel free to adapt rollback behavior if you want to keep the local worktree but clearly mark the sandbox failure.)
🧹 Nitpick comments (1)
apps/desktop/src/renderer/screens/main/components/Layout/NewLayoutMain.tsx (1)
856-901: Cloud sandbox create/open handlers work, but could benefit from basic guards and better UX.
handleCreateCloudSandboxandhandleOpenCloudSandboxcorrectly validate workspace/worktree, call the new IPC handlers, refresh the workspace, and surface errors, and wiring them intoTaskTabslooks coherent. As a follow-up, consider (a) disabling the “Create cloud sandbox” action while a create is in flight and/or whenselectedWorktree.cloudSandboxalready exists and (b) replacingalert-based messaging with your standard non-blocking notification pattern to avoid modal interruptions.Also applies to: 903-925, 1043-1051
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
apps/desktop/src/main/lib/cloud-api-client.ts(1 hunks)apps/desktop/src/main/lib/workspace-ipcs.ts(1 hunks)apps/desktop/src/renderer/screens/main/components/Layout/NewLayoutMain.tsx(10 hunks)apps/desktop/src/renderer/screens/main/components/Sidebar/Sidebar.tsx(5 hunks)apps/desktop/src/renderer/screens/main/components/Sidebar/components/CreateWorktreeButton/CreateWorktreeButton.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/Sidebar/components/WorktreeList/components/WorktreeItem/WorktreeItem.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/Sidebar/components/WorktreeList/components/WorktreeItem/components/TabItem/TabItem.tsx(4 hunks)apps/desktop/src/shared/ipc-channels.ts(0 hunks)apps/desktop/src/shared/ipc-channels/worktree.ts(1 hunks)apps/desktop/src/shared/types.ts(2 hunks)
💤 Files with no reviewable changes (1)
- apps/desktop/src/shared/ipc-channels.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/desktop/src/renderer/screens/main/components/Sidebar/components/WorktreeList/components/WorktreeItem/WorktreeItem.tsx
- apps/desktop/src/shared/types.ts
🧰 Additional context used
🧬 Code graph analysis (5)
apps/desktop/src/main/lib/workspace-ipcs.ts (2)
apps/desktop/src/main/lib/workspace-manager.ts (1)
workspaceManager(481-481)apps/desktop/src/main/lib/cloud-api-client.ts (1)
cloudApiClient(184-184)
apps/desktop/src/main/lib/cloud-api-client.ts (1)
apps/desktop/src/shared/types.ts (1)
CloudSandbox(59-67)
apps/desktop/src/renderer/screens/main/components/Sidebar/Sidebar.tsx (5)
apps/desktop/src/renderer/contexts/WorkspaceContext.tsx (1)
useWorkspaceContext(48-56)apps/desktop/src/renderer/contexts/TabContext.tsx (1)
useTabContext(55-61)apps/desktop/src/renderer/contexts/WorktreeOperationsContext.tsx (1)
useWorktreeOperationsContext(46-52)apps/desktop/src/renderer/contexts/SidebarContext.tsx (1)
useSidebarContext(34-40)apps/desktop/src/renderer/screens/main/components/Sidebar/components/CreateWorktreeButton/CreateWorktreeButton.tsx (1)
CreateWorktreeButton(11-56)
apps/desktop/src/shared/ipc-channels/worktree.ts (2)
apps/desktop/src/shared/ipc-channels/index.ts (1)
SuccessResponse(22-22)apps/desktop/src/shared/ipc-channels/types.ts (1)
SuccessResponse(27-27)
apps/desktop/src/renderer/screens/main/components/Sidebar/components/WorktreeList/components/WorktreeItem/components/TabItem/TabItem.tsx (1)
packages/ui/src/components/context-menu.tsx (1)
ContextMenuItem(244-244)
🔇 Additional comments (4)
apps/desktop/src/main/lib/workspace-ipcs.ts (1)
847-894: Cloud sandbox open/delete IPC handlers look consistent with the client and types.The
worktree-open-cloud-sandbox,worktree-delete-cloud-sandbox, andcloud-sandbox-delete-by-idhandlers have clear workspace/worktree validation, return structured{ success, error? }responses, and keep config in sync when deleting. They align with theCloudSandboxshape and the cloud API client’s semantics.Also applies to: 896-950, 952-968
apps/desktop/src/renderer/screens/main/components/Sidebar/Sidebar.tsx (1)
30-36: Sidebar context wiring and CreateWorktreeButton integration look good.Using the workspace/tab/worktree operation contexts keeps this component lean, and the new
CreateWorktreeButtonwiring correctly disables both buttons when either worktree or cloud-worktree creation is in progress.Also applies to: 40-42, 499-506
apps/desktop/src/shared/ipc-channels/worktree.ts (1)
185-208: Cloud sandbox IPC channel contracts look consistent; confirm delete semantics across call sitesThe four new cloud sandbox channels follow the existing IPC typing patterns (worktree‑scoped request payloads, SuccessResponse / inline success+error contracts, and the CloudSandbox result on creation). One thing to double‑check at the call‑site level is the distinction between:
"worktree-delete-cloud-sandbox"— scoped by{ workspaceId, worktreeId }, presumably responsible for also keepingWorktree.cloudSandboxin sync, and"cloud-sandbox-delete-by-id"— scoped only by{ sandboxId }, which may not touch worktree state.Making sure these two are used intentionally (e.g., worktree removal vs. global cleanup) will help avoid cases where a sandbox is deleted but the associated worktree still thinks it has an attached
cloudSandbox, or vice versa.apps/desktop/src/renderer/screens/main/components/Sidebar/components/CreateWorktreeButton/CreateWorktreeButton.tsx (1)
2-55: Cloud worktree button behavior is sound; ensure callers keepisCreatingandisCreatingCloudmutually exclusiveThe dual‑button layout and disabled logic look correct:
- Existing behavior is preserved when
onCreateCloudis not passed (single “New Worktree” button, full width viaflex-1in a one‑child flex row).- When
onCreateCloudis provided, the two buttons share a clear split of responsibilities, anddisabled={isCreating || isCreatingCloud}on both prevents overlapping actions.- Icon/text switching for each button is coherent as long as only one of
isCreatingorisCreatingCloudistrueat a time.It would be good to verify that callers never set both flags
truesimultaneously; if they did, both buttons would be disabled (which is OK) but the primary button would show the idle “New Worktree” state while the cloud button shows the spinner, which may be slightly confusing. If this can’t be guaranteed by convention, a single “mode” prop (e.g.,creatingMode?: "local" | "cloud") might be a cleaner future refactor, but the current API is fine for now.
| // Cloud sandbox operations | ||
| ipcMain.handle( | ||
| "worktree-create-cloud-sandbox", | ||
| async (_event, input: { workspaceId: string; worktreeId: string }) => { | ||
| try { | ||
| const workspace = await workspaceManager.getWorkspace( | ||
| input.workspaceId, | ||
| ); | ||
| if (!workspace) { | ||
| return { | ||
| success: false, | ||
| error: "Workspace not found", | ||
| }; | ||
| } | ||
|
|
||
| const worktree = workspace.worktrees.find( | ||
| (wt) => wt.id === input.worktreeId, | ||
| ); | ||
| if (!worktree) { | ||
| return { | ||
| success: false, | ||
| error: "Worktree not found", | ||
| }; | ||
| } | ||
|
|
||
| // Get GitHub repo URL | ||
| let githubRepo: string | undefined; | ||
| try { | ||
| const { execSync } = await import("node:child_process"); | ||
| const remoteUrl = execSync("git remote get-url origin", { | ||
| cwd: workspace.repoPath, | ||
| encoding: "utf-8", | ||
| }).trim(); | ||
|
|
||
| // Convert git URL to repo format (owner/repo) | ||
| const match = remoteUrl.match(/github\.com[:/](.+?)(?:\.git)?$/); | ||
| if (match?.[1]) { | ||
| githubRepo = match[1]; | ||
| } | ||
| } catch (error) { | ||
| console.warn("Could not determine GitHub repo:", error); | ||
| } | ||
|
|
||
| // Import cloud API client | ||
| const { cloudApiClient } = await import("./cloud-api-client"); | ||
|
|
||
| // Create sandbox | ||
| const result = await cloudApiClient.createSandbox({ | ||
| name: `${workspace.name}-${worktree.branch}`, | ||
| githubRepo, | ||
| taskDescription: worktree.description || `Work on ${worktree.branch}`, | ||
| }); | ||
|
|
||
| if (!result.success || !result.sandbox) { | ||
| return result; | ||
| } | ||
|
|
||
| // Store sandbox info in worktree config | ||
| worktree.cloudSandbox = result.sandbox; | ||
| await workspaceManager.saveConfig(); | ||
|
|
||
| return result; | ||
| } catch (error) { | ||
| console.error("Failed to create cloud sandbox:", error); | ||
| return { | ||
| success: false, | ||
| error: error instanceof Error ? error.message : String(error), | ||
| }; | ||
| } | ||
| }, | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
Guard against duplicate sandboxes per worktree and avoid execSync in the main process.
This handler looks solid overall (good workspace/worktree validation and config persistence), but:
- It will happily create multiple cloud sandboxes for the same worktree if called repeatedly, only tracking the last one in
worktree.cloudSandbox. - It shells out with
execSync("git remote get-url origin")on the main thread, which can briefly freeze the UI on slow repos.
Consider:
const worktree = workspace.worktrees.find(
(wt) => wt.id === input.worktreeId,
);
if (!worktree) {
@@
};
}
+
+ if (worktree.cloudSandbox) {
+ return {
+ success: false,
+ error: "Cloud sandbox already exists for this worktree",
+ };
+ }
@@
- try {
- const { execSync } = await import("node:child_process");
- const remoteUrl = execSync("git remote get-url origin", {
- cwd: workspace.repoPath,
- encoding: "utf-8",
- }).trim();
+ try {
+ const { exec } = await import("node:child_process");
+ const { stdout } = await new Promise<{
+ stdout: string;
+ stderr: string;
+ }>((resolve, reject) =>
+ exec(
+ "git remote get-url origin",
+ { cwd: workspace.repoPath },
+ (err, stdout, stderr) =>
+ err ? reject(err) : resolve({ stdout, stderr }),
+ ),
+ );
+ const remoteUrl = stdout.trim();(Or similar async child-process usage that fits your existing patterns.)
I'll verify the concerns raised in this review by examining the codebase. Let me gather information about the duplicate sandbox guard, the blocking execSync usage, and existing async patterns.
🏁 Script executed:
#!/bin/bash
# Check full context of cloud sandbox handlers and search for existing guards/patterns
cd /tmp/repo
# 1. Get full handler implementation to check for duplicate guards
rg -n -A 100 "worktree-create-cloud-sandbox" apps/desktop/src/main/lib/workspace-ipcs.ts | head -120
# 2. Search for how cloudSandbox is used elsewhere
rg -n "cloudSandbox" apps/desktop/src/main/lib/workspace-ipcs.ts
# 3. Look for existing async child_process patterns in the file
rg -n -B 2 -A 5 "exec\(" apps/desktop/src/main/lib/workspace-ipcs.ts | head -60
# 4. Check if execSync is used elsewhere and how
rg -n "execSync" apps/desktop/src/main/lib/workspace-ipcs.tsLength of output: 395
Let me search the codebase with proper cloning enabled:
🏁 Script executed:
#!/bin/bash
# Check full context of cloud sandbox handlers and existing patterns
# 1. Get full handler implementation to check for duplicate guards
rg -n -A 100 "worktree-create-cloud-sandbox" apps/desktop/src/main/lib/workspace-ipcs.ts | head -120
# 2. Search for how cloudSandbox is used elsewhere
rg -n "cloudSandbox" apps/desktop/src/main/lib/workspace-ipcs.ts
# 3. Look for existing async child_process patterns in the file
rg -n -B 2 -A 5 "exec\(" apps/desktop/src/main/lib/workspace-ipcs.ts | head -80
# 4. Check if execSync is used elsewhere and how
rg -n "execSync" apps/desktop/src/main/lib/workspace-ipcs.ts
# 5. Check other ipcMain.handle for duplicate state guards
rg -n -B 5 -A 10 "already exists" apps/desktop/src/main/lib/workspace-ipcs.ts | head -40Length of output: 4194
Guard against duplicate sandboxes and replace execSync with async exec to prevent UI blocking.
The handler creates multiple cloud sandboxes for the same worktree without checking if one already exists, which leaks VMs since only the latest is tracked in worktree.cloudSandbox. Additionally, execSync("git remote get-url origin") blocks the main thread on line 804, freezing the UI during the git operation. Other handlers in the same file guard state checks (e.g., worktree-open-cloud-sandbox at line 871), establishing the pattern you should follow.
Add a guard check and switch to async exec:
const worktree = workspace.worktrees.find(
(wt) => wt.id === input.worktreeId,
);
if (!worktree) {
return {
success: false,
error: "Worktree not found",
};
}
+
+ if (worktree.cloudSandbox) {
+ return {
+ success: false,
+ error: "Cloud sandbox already exists for this worktree",
+ };
+ }
// Get GitHub repo URL
let githubRepo: string | undefined;
try {
- const { execSync } = await import("node:child_process");
- const remoteUrl = execSync("git remote get-url origin", {
- cwd: workspace.repoPath,
- encoding: "utf-8",
- }).trim();
+ const { exec } = await import("node:child_process");
+ const remoteUrl = await new Promise<string>((resolve, reject) =>
+ exec(
+ "git remote get-url origin",
+ { cwd: workspace.repoPath },
+ (err, stdout) =>
+ err ? reject(err) : resolve(stdout.trim()),
+ ),
+ );- use ipcRenderer.off instead of removeListener for worktree-setup-progress - removes trailing whitespace
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
apps/desktop/src/renderer/screens/main/components/Sidebar/Sidebar.tsx (1)
229-265: Surface sandbox creation failures and roll back the optimistic worktree.This is the same issue flagged in the previous review. If
worktree-create-cloud-sandboxfails (network, quota, auth, etc.), the code skips theif (sandboxResult.success …)block but still falls through to line 265 whereonWorktreeCreated()is called. The brand-new worktree persists without an attached sandbox, leaving the user with a "cloud worktree" that has nothing cloud about it.Apply this diff to handle the failure case:
const sandboxResult = await window.ipcRenderer.invoke( "worktree-create-cloud-sandbox", { workspaceId: currentWorkspace.id, worktreeId: result.worktree.id, }, ); + if (!sandboxResult.success) { + alert( + `Failed to create cloud sandbox: ${ + sandboxResult.error || "Unknown error" + }`, + ); + await window.ipcRenderer.invoke("worktree-remove", { + workspaceId: currentWorkspace.id, + worktreeId: result.worktree.id, + }); + handleWorktreeCreated(); + return; + } + if (sandboxResult.success && sandboxResult.sandbox?.claudeHost) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/desktop/src/renderer/screens/main/components/Layout/NewLayoutMain.tsx(5 hunks)apps/desktop/src/renderer/screens/main/components/Sidebar/Sidebar.tsx(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/desktop/src/renderer/screens/main/components/Sidebar/Sidebar.tsx (1)
apps/desktop/src/renderer/screens/main/components/Sidebar/components/CreateWorktreeButton/CreateWorktreeButton.tsx (1)
CreateWorktreeButton(11-56)
🔇 Additional comments (5)
apps/desktop/src/renderer/screens/main/components/Sidebar/Sidebar.tsx (1)
495-502: LGTM!The CreateWorktreeButton is correctly wired with both the standard worktree creation handler and the new cloud worktree handler. The loading states are properly managed with separate flags for each operation.
apps/desktop/src/renderer/screens/main/components/Layout/NewLayoutMain.tsx (4)
841-886: LGTM — Cloud sandbox creation handler is well-structured.The handler guards against missing workspace/worktree, refreshes state after creation, and handles errors gracefully. The auto-open logic is conditional and safe.
888-910: LGTM — Cloud sandbox open handler is clean and defensive.Proper guards, error handling, and user feedback via alerts.
718-718: Good refactor — using.off()instead of.removeListener().This aligns with modern IPC cleanup patterns and improves readability.
Also applies to: 762-762
1034-1035: LGTM — Handlers correctly wired to TaskTabs.The new cloud sandbox handlers are properly threaded into the TaskTabs component.
| alert("Cloud sandbox created! Opening in browser..."); | ||
|
|
||
| // Auto-open the sandbox | ||
| if (result.sandbox?.claudeHost) { | ||
| await window.ipcRenderer.invoke("worktree-open-cloud-sandbox", { | ||
| workspaceId: currentWorkspace.id, | ||
| worktreeId: selectedWorktreeId, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Alert fires before confirming the auto-open succeeded.
The success alert at line 867 says "Opening in browser..." but fires before the auto-open IPC call (lines 871-874) completes. If the open operation fails silently, the user sees a misleading success message.
Move the alert after confirming the open succeeded:
if (refreshedWorkspace) {
setCurrentWorkspace(refreshedWorkspace);
}
- alert("Cloud sandbox created! Opening in browser...");
// Auto-open the sandbox
if (result.sandbox?.claudeHost) {
- await window.ipcRenderer.invoke("worktree-open-cloud-sandbox", {
+ const openResult = await window.ipcRenderer.invoke("worktree-open-cloud-sandbox", {
workspaceId: currentWorkspace.id,
worktreeId: selectedWorktreeId,
});
+
+ if (openResult.success) {
+ alert("Cloud sandbox created and opened in browser!");
+ } else {
+ alert(`Cloud sandbox created, but failed to open: ${openResult.error || "Unknown error"}`);
+ }
+ } else {
+ alert("Cloud sandbox created!");
}
} else {Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/desktop/src/renderer/screens/main/components/Layout/NewLayoutMain.tsx
around lines 867 to 875, the "Cloud sandbox created! Opening in browser..."
alert is shown before the auto-open IPC call completes, which can mislead users
if the open fails; move the alert to after awaiting
window.ipcRenderer.invoke("worktree-open-cloud-sandbox", ...) and only show it
when that call resolves successfully, wrapping the invoke in a try/catch to
handle errors and show an error alert or log on failure so the user isn’t
misinformed.
| next.add(result.worktree!.id); | ||
| return next; | ||
| }); | ||
| onTabSelect(result.worktree.id, tabResult.tab.id); |
There was a problem hiding this comment.
Fix undefined variable reference.
Line 260 calls onTabSelect which is not defined in the function scope. The correct handler is handleTabSelect (defined at line 416).
Apply this diff:
- onTabSelect(result.worktree.id, tabResult.tab.id);
+ handleTabSelect(result.worktree.id, tabResult.tab.id);📝 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.
| onTabSelect(result.worktree.id, tabResult.tab.id); | |
| handleTabSelect(result.worktree.id, tabResult.tab.id); |
🤖 Prompt for AI Agents
In apps/desktop/src/renderer/screens/main/components/Sidebar/Sidebar.tsx around
line 260, the code calls onTabSelect which is undefined in this scope; replace
that call with the correct handler handleTabSelect (the function defined at line
416) so the existing parameters (result.worktree.id, tabResult.tab.id) are
passed to handleTabSelect instead, ensuring the variable name matches the
defined function.
|
re-created in #176 |
Summary
Features
CLAUDE_CODE_OAUTH_TOKENto VMs asANTHROPIC_AUTH_TOKENDocs
see API documentation: https://gist.github.com/caffeinum/c1e68a6f8fe66f7bd06b9509559d701a
Summary by CodeRabbit