feat: inline setup for fresh sandbox onboarding (simplified)#256
Conversation
When promptSandboxStreaming detects a fresh sandbox (no snapshot), it delegates to the existing runSandboxCommandTask background task which handles full setup (OpenClaw, GitHub, etc.) instead of duplicating the setup pipeline inline. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe changes implement snapshot-origin tracking throughout the sandbox creation and streaming pipeline. A new Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant GetOrCreateSandbox
participant CreateSandboxFromSnapshot
participant TriggerRunSandboxCommand
participant DirectStreaming
Client->>GetOrCreateSandbox: getOrCreateSandbox(accountId)
alt Existing Sandbox Found
GetOrCreateSandbox-->>Client: {sandbox, sandboxId, created: false, fromSnapshot: true}
else Fresh Sandbox Created
GetOrCreateSandbox->>CreateSandboxFromSnapshot: createSandboxFromSnapshot(accountId)
CreateSandboxFromSnapshot-->>GetOrCreateSandbox: {sandbox, fromSnapshot: false}
GetOrCreateSandbox->>TriggerRunSandboxCommand: trigger background run
TriggerRunSandboxCommand-->>GetOrCreateSandbox: runId
GetOrCreateSandbox-->>Client: {sandbox, sandboxId, created: true, fromSnapshot: false}
end
Client->>DirectStreaming: promptSandboxStreaming(sandbox, fromSnapshot)
alt Fresh (fromSnapshot: false)
DirectStreaming->>TriggerRunSandboxCommand: initiate background execution
DirectStreaming-->>Client: {runId, fromSnapshot: false, stdout: "", stderr: ""}
else Snapshot-based (fromSnapshot: true)
DirectStreaming->>DirectStreaming: execute & stream directly
DirectStreaming-->>Client: {stdout, stderr, fromSnapshot: true}
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ❌ 1❌ Failed checks (1 warning)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…hotResult Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
lib/sandbox/getOrCreateSandbox.ts (1)
9-9:fromSnapshotis semantically overloaded in the existing-sandbox path.On Line 26,
fromSnapshot: truemeans “already has an active sandbox,” not strictly “originated from snapshot.” Consider renaming this flag (e.g.,hasExistingSetup) to reduce future misuse.As per coding guidelines, "**/*.{ts,tsx}: ... use specific names like 'artist', 'workspace', 'organization' when referring to specific types".
Also applies to: 26-26
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/sandbox/getOrCreateSandbox.ts` at line 9, The flag fromSnapshot is overloaded in getOrCreateSandbox (used to mean "already has an active sandbox" in the existing-sandbox path) — rename that boolean to a specific, unambiguous name (e.g., hasExistingSetup) and update all places that construct or read this property (the object created in the existing-sandbox branch where fromSnapshot: true is set and any consumer code that checks the flag) to use the new name; ensure the exported/returned type signature and any references in functions or classes that expect fromSnapshot are updated accordingly to avoid breaking type checks and follow the naming guideline for specific, semantic property names.lib/sandbox/promptSandboxStreaming.ts (1)
31-37: Split this function into focused helpers to reduce complexity.
promptSandboxStreamingnow orchestrates sandbox retrieval, fresh-background execution, and direct streaming in one flow. Extracting the fresh path and direct path into helpers will improve readability/testability and keep this under the recommended size.As per coding guidelines,
lib/**/*.ts: "Single responsibility per function", "Keep functions under 50 lines", and "DRY: Consolidate similar logic into shared utilities".Also applies to: 43-70, 72-102
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/sandbox/promptSandboxStreaming.ts` around lines 31 - 37, The promptSandboxStreaming function is doing too much—split its responsibilities into focused helpers: extract sandbox retrieval into getOrCreateSandbox(sandboxId|input) and separate the two execution flows into handleFreshExecution(input, sandbox) and handleDirectStreaming(input, sandbox) (or similarly named functions referenced from promptSandboxStreaming) so the main function only routes to the appropriate helper and yields their AsyncGenerator output; move any duplicated logic (e.g., stream event mapping, error handling, logger usage) into a shared utility (e.g., streamEventMapper or emitStreamChunk) so duplicate code in the fresh-path and direct-path sections is consolidated, keep each helper under ~50 lines, update promptSandboxStreaming to call these helpers and forward their results, and adjust tests to call the helpers directly for focused unit tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/sandbox/promptSandboxStreaming.ts`:
- Around line 56-61: The code assumes pollTaskRun(handle.id) returns a run with
a non-null structured output and force-casts it; update the logic around
pollTaskRun and the local output variable to first check that run and run.output
exist and have the expected shape (stdout, stderr, exitCode) before casting or
accessing properties, and throw or return a clear structured error if missing or
malformed; specifically adjust the block using pollTaskRun, the const run =
await pollTaskRun(handle.id) and the subsequent use of output (and the handling
around lines 63–69) to validate types/fields (or use a safe parser/guard)
instead of force-casting.
- Around line 43-56: The generator currently awaits pollTaskRun(handle.id)
unconditionally and can outlive an aborted caller; update the created &&
!fromSnapshot branch to respect the provided abortSignal by (1) passing the
abortSignal into triggerRunSandboxCommand and/or pollTaskRun if those helpers
accept it (use the abortSignal parameter), and (2) if they don't, check
abortSignal.aborted immediately after obtaining handle (and subscribe to
abortSignal) to cancel the run and throw or return early; ensure you also call
any available cancellation API for the run (e.g., a cancelTaskRun or
handle.cancel) before exiting to avoid orphaned background tasks.
---
Nitpick comments:
In `@lib/sandbox/getOrCreateSandbox.ts`:
- Line 9: The flag fromSnapshot is overloaded in getOrCreateSandbox (used to
mean "already has an active sandbox" in the existing-sandbox path) — rename that
boolean to a specific, unambiguous name (e.g., hasExistingSetup) and update all
places that construct or read this property (the object created in the
existing-sandbox branch where fromSnapshot: true is set and any consumer code
that checks the flag) to use the new name; ensure the exported/returned type
signature and any references in functions or classes that expect fromSnapshot
are updated accordingly to avoid breaking type checks and follow the naming
guideline for specific, semantic property names.
In `@lib/sandbox/promptSandboxStreaming.ts`:
- Around line 31-37: The promptSandboxStreaming function is doing too much—split
its responsibilities into focused helpers: extract sandbox retrieval into
getOrCreateSandbox(sandboxId|input) and separate the two execution flows into
handleFreshExecution(input, sandbox) and handleDirectStreaming(input, sandbox)
(or similarly named functions referenced from promptSandboxStreaming) so the
main function only routes to the appropriate helper and yields their
AsyncGenerator output; move any duplicated logic (e.g., stream event mapping,
error handling, logger usage) into a shared utility (e.g., streamEventMapper or
emitStreamChunk) so duplicate code in the fresh-path and direct-path sections is
consolidated, keep each helper under ~50 lines, update promptSandboxStreaming to
call these helpers and forward their results, and adjust tests to call the
helpers directly for focused unit tests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8e40e5c5-5e51-4b7f-a2db-867fafac8b06
⛔ Files ignored due to path filters (4)
lib/sandbox/__tests__/createSandboxFromSnapshot.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/sandbox/__tests__/getOrCreateSandbox.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/sandbox/__tests__/promptSandboxStreaming.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/trigger/__tests__/pollTaskRun.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**
📒 Files selected for processing (4)
lib/sandbox/createSandboxFromSnapshot.tslib/sandbox/getOrCreateSandbox.tslib/sandbox/promptSandboxStreaming.tslib/trigger/pollTaskRun.ts
| if (created && !fromSnapshot) { | ||
| yield { | ||
| data: "Setting up your sandbox for the first time...\n", | ||
| stream: "stderr" as const, | ||
| }; | ||
|
|
||
| const handle = await triggerRunSandboxCommand({ | ||
| command: "openclaw", | ||
| args: ["agent", "--agent", "main", "--message", prompt], | ||
| sandboxId, | ||
| accountId, | ||
| }); | ||
|
|
||
| const run = await pollTaskRun(handle.id); |
There was a problem hiding this comment.
Background-task path currently ignores abortSignal.
Line 56 awaits polling unconditionally; if the caller aborts, this generator can continue waiting and outlive the request lifecycle.
Proposed cancellation wiring
+ if (abortSignal?.aborted) {
+ throw new DOMException("Operation aborted", "AbortError");
+ }
+
const handle = await triggerRunSandboxCommand({
command: "openclaw",
args: ["agent", "--agent", "main", "--message", prompt],
sandboxId,
accountId,
});
- const run = await pollTaskRun(handle.id);
+ const runPromise = pollTaskRun(handle.id);
+ const run = abortSignal
+ ? await Promise.race([
+ runPromise,
+ new Promise<never>((_, reject) => {
+ abortSignal.addEventListener(
+ "abort",
+ () => reject(new DOMException("Operation aborted", "AbortError")),
+ { once: true },
+ );
+ }),
+ ])
+ : await runPromise;As per coding guidelines, lib/**/*.ts: "For domain functions, ensure: ... Proper error handling".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/sandbox/promptSandboxStreaming.ts` around lines 43 - 56, The generator
currently awaits pollTaskRun(handle.id) unconditionally and can outlive an
aborted caller; update the created && !fromSnapshot branch to respect the
provided abortSignal by (1) passing the abortSignal into
triggerRunSandboxCommand and/or pollTaskRun if those helpers accept it (use the
abortSignal parameter), and (2) if they don't, check abortSignal.aborted
immediately after obtaining handle (and subscribe to abortSignal) to cancel the
run and throw or return early; ensure you also call any available cancellation
API for the run (e.g., a cancelTaskRun or handle.cancel) before exiting to avoid
orphaned background tasks.
…oxes Avoids API function timeouts by triggering the background task and returning the runId immediately. The caller can track progress without blocking the request. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Chat frontend will show the same tool component as run sandbox command, so no need to yield a status message from the streaming generator. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/sandbox/getOrCreateSandbox.ts`:
- Around line 21-27: getOrCreateSandbox currently returns fromSnapshot: true
when reusing an existing Sandbox returned by getActiveSandbox(accountId), but
that branch has no snapshot-origin metadata so it can misreport origin; update
the Sandbox persistence/retrieval to include origin metadata (e.g., add an
origin/fromSnapshot field when creating and storing sandboxes) and change
getActiveSandbox and getOrCreateSandbox to read and propagate that real origin
value (use the Sandbox object's origin/fromSnapshot property instead of
hardcoding true) so callers receive the correct origin flag for
sandboxId/sandbox.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: efb2d170-e9c3-4470-b661-7ff1d1bf8375
📒 Files selected for processing (1)
lib/sandbox/getOrCreateSandbox.ts
| if (existing) { | ||
| return { | ||
| sandbox: existing, | ||
| sandboxId: existing.sandboxId, | ||
| created: false, | ||
| fromSnapshot: true, | ||
| }; |
There was a problem hiding this comment.
fromSnapshot is hardcoded for reused sandboxes and can misreport origin.
On Line 26, fromSnapshot: true is returned even though this branch only has Sandbox from getActiveSandbox(accountId) and no snapshot-origin metadata. That can leak incorrect state to callers if an active sandbox was originally created fresh.
Recommend persisting/retrieving sandbox origin metadata (e.g., alongside sandbox_id) and returning the real value here, rather than inferring it.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/sandbox/getOrCreateSandbox.ts` around lines 21 - 27, getOrCreateSandbox
currently returns fromSnapshot: true when reusing an existing Sandbox returned
by getActiveSandbox(accountId), but that branch has no snapshot-origin metadata
so it can misreport origin; update the Sandbox persistence/retrieval to include
origin metadata (e.g., add an origin/fromSnapshot field when creating and
storing sandboxes) and change getActiveSandbox and getOrCreateSandbox to read
and propagate that real origin value (use the Sandbox object's
origin/fromSnapshot property instead of hardcoding true) so callers receive the
correct origin flag for sandboxId/sandbox.
Allows the chat tool UI component to determine which UI to render based on whether the sandbox was created from a snapshot or fresh. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The complete status now includes fromSnapshot and runId when a fresh sandbox triggers a background setup task. This allows the chat frontend to show the appropriate UI component (run sandbox command view vs streaming view). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
lib/sandbox/promptSandboxStreaming.ts (1)
45-51: Refactor shared command construction and split execution paths for maintainability.This function now handles two execution strategies plus output aggregation, and the OpenClaw args are duplicated in Line 48 and Line 67. Extracting helpers will reduce branching noise and keep this domain function tighter.
♻️ Proposed refactor
+function buildOpenClawAgentArgs(prompt: string): string[] { + return ["agent", "--agent", "main", "--message", prompt]; +} + +async function triggerFreshSandboxRun(params: { + sandboxId: string; + accountId: string; + prompt: string; + created: boolean; + fromSnapshot: boolean; +}): Promise<PromptSandboxStreamingResult> { + const handle = await triggerRunSandboxCommand({ + command: "openclaw", + args: buildOpenClawAgentArgs(params.prompt), + sandboxId: params.sandboxId, + accountId: params.accountId, + }); + + return { + sandboxId: params.sandboxId, + stdout: "", + stderr: "", + exitCode: 0, + created: params.created, + fromSnapshot: params.fromSnapshot, + runId: handle.id, + }; +} + export async function* promptSandboxStreaming( input: PromptSandboxStreamingInput, ): AsyncGenerator< @@ // Fresh sandbox: trigger background task for full setup + prompt execution if (created && !fromSnapshot) { - const handle = await triggerRunSandboxCommand({ - command: "openclaw", - args: ["agent", "--agent", "main", "--message", prompt], - sandboxId, - accountId, - }); - - return { - sandboxId, - stdout: "", - stderr: "", - exitCode: 0, - created, - fromSnapshot, - runId: handle.id, - }; + return triggerFreshSandboxRun({ + sandboxId, + accountId, + prompt, + created, + fromSnapshot, + }); } @@ const cmd = await sandbox.runCommand({ cmd: "openclaw", - args: ["agent", "--agent", "main", "--message", prompt], + args: buildOpenClawAgentArgs(prompt),As per coding guidelines,
lib/**/*.ts: "For domain functions, ensure: Single responsibility per function ... Keep functions under 50 lines ... DRY: Consolidate similar logic into shared utilities" and**/*.{ts,tsx}: "Extract shared logic into reusable utilities following Don't Repeat Yourself (DRY) principle".Also applies to: 65-67
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/sandbox/promptSandboxStreaming.ts` around lines 45 - 51, The OpenClaw command args are duplicated in promptSandboxStreaming (used in the triggerRunSandboxCommand call and again in the alternate execution path); extract a small helper like buildOpenClawArgs(prompt) and a runOpenClaw(sandboxId, accountId, prompt) wrapper that calls triggerRunSandboxCommand with the constructed args, then replace the two inline arg arrays and direct calls with those helpers, and refactor the two execution branches to both call the same run helper and a single output-aggregation routine so promptSandboxStreaming remains focused and DRY.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@lib/sandbox/promptSandboxStreaming.ts`:
- Around line 45-51: The OpenClaw command args are duplicated in
promptSandboxStreaming (used in the triggerRunSandboxCommand call and again in
the alternate execution path); extract a small helper like
buildOpenClawArgs(prompt) and a runOpenClaw(sandboxId, accountId, prompt)
wrapper that calls triggerRunSandboxCommand with the constructed args, then
replace the two inline arg arrays and direct calls with those helpers, and
refactor the two execution branches to both call the same run helper and a
single output-aggregation routine so promptSandboxStreaming remains focused and
DRY.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 56e6743e-7a4e-414e-b9f8-e352e551d53b
⛔ Files ignored due to path filters (2)
lib/chat/tools/__tests__/createPromptSandboxStreamingTool.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/sandbox/__tests__/promptSandboxStreaming.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**
📒 Files selected for processing (2)
lib/chat/tools/createPromptSandboxStreamingTool.tslib/sandbox/promptSandboxStreaming.ts
Summary
promptSandboxStreamingdetects a fresh sandbox (no snapshot), it delegates to the existingrunSandboxCommandTaskbackground task instead of duplicating the entire setup pipeline inlinecreateSandboxFromSnapshotnow returnsfromSnapshotbooleangetOrCreateSandboxpropagatesfromSnapshotto callerspollTaskRunutility wrapsruns.poll()from Trigger.dev SDKHow it works
runSandboxCommandTask(handles full setup + runs prompt), poll for completionWhy this is simpler than PR #252
PR #252 duplicated the entire setup pipeline (3000+ lines, 38 files) inline in
promptSandboxStreaming. This PR reuses the existingrunSandboxCommandTaskwhich already handles OpenClaw install, GitHub repo setup, org repos, README, skills, push, and snapshot — all in ~8 changed files and ~250 net new lines.Test plan
fromSnapshotincreateSandboxFromSnapshotfromSnapshotpropagation ingetOrCreateSandboxpromptSandboxStreamingpollTaskRunutility🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes