[cross-repo] Implement Basilica Compute Sandboxes (backend + frontend) - #413
Open
machine-god-deus wants to merge 12 commits into
Open
Conversation
…pute-sandboxes-backend--frontend--subtask-frontend-sandbox-ui' into agent/issue-271-cross-repo-implement-basilica-compute-sandboxes-backend--frontend
…pute-sandboxes-backend--frontend--subtask-fix-frontend-cargo-test' into agent/issue-271-cross-repo-implement-basilica-compute-sandboxes-backend--frontend
…pute-sandboxes-backend--frontend--subtask-frontend-sandbox-ui' into agent/issue-271-cross-repo-implement-basilica-compute-sandboxes-backend--frontend
…pute-sandboxes-backend--frontend--subtask-add-sdk-sandbox-tests' into agent/issue-271-cross-repo-implement-basilica-compute-sandboxes-backend--frontend
Contributor
WalkthroughThis pull request adds Basilica Compute Sandbox support to the CLI and SDK, including new sandbox lifecycle commands (create, list, status, delete) and WebSocket-based exec connectivity for running sandboxes, alongside SDK types and client methods for sandbox API operations. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant CLI as CLI Handler
participant Client as BasilicaClient
participant API as Sandbox API
participant Router as sandbox-router
participant Agent as exec-agent
User->>CLI: sandbox connect --id=sb-xxx --secret=...
CLI->>Client: get_sandbox(sandbox_id)
Client->>API: GET /v1/sandboxes/{id}
API-->>Client: SandboxResponse {domain, status}
alt Phase == Pending
loop Poll Until Running
CLI->>Client: get_sandbox(sandbox_id)
Client->>API: GET /v1/sandboxes/{id}
API-->>Client: SandboxResponse
Note over CLI: Check phase, wait if Pending
end
end
CLI->>Router: Connect to wss://sb-xxx.sandboxes.basilica.ai/ws
Note over CLI,Router: X-Exec-Secret header
Router->>Agent: Forward WebSocket connection
Agent-->>Router: WebSocket established
Router-->>CLI: WebSocket connection ready
alt Interactive Mode
loop Per stdin line
User->>CLI: Enter command
CLI->>Agent: Send SandboxWsRequest {op: "exec", args: {...}}
Agent->>Agent: Execute command
Agent-->>CLI: SandboxWsResponse {data: stdout/stderr}
CLI->>User: Display output
end
else Single Command Mode
CLI->>Agent: Send SandboxWsRequest {op: "exec", args: {cmd}}
Agent->>Agent: Execute single command
Agent-->>CLI: SandboxWsResponse {exit_code, stdout, stderr}
CLI->>CLI: Exit process with exit code
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
crates/basilica-cli/Cargo.toml (1)
33-33: Gatetokio-tungstenitebehind thesandboxfeature.The dependency is currently unconditional in every build. The
sandbox = []feature gates only the code, not the dependency itself. To make sandbox support truly optional, marktokio-tungsteniteas optional and wire it to the feature:-tokio-tungstenite = { workspace = true } +tokio-tungstenite = { workspace = true, optional = true } [features] -sandbox = [] +sandbox = ["dep:tokio-tungstenite"]Also applies to lines 78–79.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/basilica-cli/Cargo.toml` at line 33, The tokio-tungstenite dependency is unconditional; mark it optional and wire it to the sandbox feature so the dependency is only pulled when sandbox is enabled: change occurrences of tokio-tungstenite = { workspace = true } to tokio-tungstenite = { workspace = true, optional = true } and then add the dependency to the sandbox feature (e.g. in the [features] section make sandbox = ["tokio-tungstenite", ...existing entries]) so enabling the sandbox feature also enables the optional tokio-tungstenite dependency; apply this change for the other occurrences referenced (lines 78–79).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/basilica-cli/src/cli/commands.rs`:
- Around line 390-397: The Connect enum variant exposes exec secret via a plain
--secret flag which can leak; change the CLI to accept the secret
non-interactively from stdin or a file instead of a regular flag: remove or stop
using #[arg(long)] on the existing secret: Option<String> in the Connect variant
and add a new Option<PathBuf> (e.g. secret_file) with #[arg(long =
"secret-file")] (accept "-" to mean read from stdin), then update the connect
handler that currently prompts for secret to read from secret_file (reading file
contents or stdin) before falling back to the secure interactive prompt; ensure
functions handling the secret (the Connect match arm / handler) use the new
secret_file path and do not log or echo the secret.
In `@crates/basilica-cli/src/cli/handlers/sandbox.rs`:
- Around line 471-550: interactive_session is blocking on stdin and never sends
WebSocket activity, so idle sandboxes are reaped; fix by ensuring the socket is
kept alive while waiting for user input: either spawn a background task (or use
tokio::select!) that periodically sends a ping/text heartbeat on the ws (e.g.,
every 30s) and/or polls the socket for incoming frames while the REPL waits,
using the existing connect_ws and read_op_response functions to detect
disconnects and trigger reconnect logic; reference interactive_session,
connect_ws, read_op_response, and parse_interactive_command when adding the
heartbeat/polling logic so the session maintains activity during user idle
periods.
- Around line 75-85: The special-case match for
basilica_sdk::ApiError::ApiResponse { status, message } in the
client.create_sandbox error handling is invalid because the current SDK enum has
no ApiResponse variant; update the error handler in the create_sandbox call (the
map_err closure around client.create_sandbox) to remove that non-existent
pattern and simply convert all errors to CliError::Api(e) after calling
complete_spinner_error(spinner.clone(), "Failed to create sandbox"); if you do
need a 402/payment-specific path later, instead add a status-carrying error
variant to the SDK and then reintroduce a match for that new variant (or provide
a helper on basilica_sdk::ApiError to inspect HTTP status).
- Around line 377-382: The code builds SandboxWsRequest for op "exec" by
splitting the command with split_whitespace(), which mangles quoted args; update
the handler(s) where SandboxWsRequest is created (the blocks using op "exec"
around the uses of command and op_id) to send shell-aware argv instead: either
parse the command with a shell-aware parser (e.g., use shell_words::split on the
command string and pass that Vec) or, more simply and reliably for shell
semantics, set args to a single argv array that invokes the shell (e.g., ["sh",
"-lc", command]) so commands like python -c "print(1)" and paths with spaces are
preserved. Ensure both occurrences mentioned (the one-shot --exec path and the
interactive exec fallback) are changed consistently.
- Around line 396-452: The loop currently returns success when the websocket
stream ends without receiving an "exit" response; add a boolean flag (e.g.,
seen_exit) initialized false, set seen_exit = true inside the "exit" arm (where
exit_code is assigned), and after the read loop (before closing ws) check if
seen_exit is false and return an Err(CliError::Internal(eyre!("WebSocket closed
before exit frame"))) so dropped/died exec sessions are treated as errors; apply
the same check/update for the similar block referenced around the other
occurrence (lines ~462-466) and reference SandboxWsResponse, ws.next(), the
"exit" arm, and exit_code when locating changes.
---
Nitpick comments:
In `@crates/basilica-cli/Cargo.toml`:
- Line 33: The tokio-tungstenite dependency is unconditional; mark it optional
and wire it to the sandbox feature so the dependency is only pulled when sandbox
is enabled: change occurrences of tokio-tungstenite = { workspace = true } to
tokio-tungstenite = { workspace = true, optional = true } and then add the
dependency to the sandbox feature (e.g. in the [features] section make sandbox =
["tokio-tungstenite", ...existing entries]) so enabling the sandbox feature also
enables the optional tokio-tungstenite dependency; apply this change for the
other occurrences referenced (lines 78–79).
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ee7fdc07-c146-4a4d-a69a-d7c5ab3f6bd4
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
Cargo.tomlcrates/basilica-cli/Cargo.tomlcrates/basilica-cli/src/cli/args.rscrates/basilica-cli/src/cli/commands.rscrates/basilica-cli/src/cli/handlers/mod.rscrates/basilica-cli/src/cli/handlers/sandbox.rscrates/basilica-sdk-python/src/lib.rscrates/basilica-sdk/src/client.rscrates/basilica-sdk/src/types.rs
Comment on lines
+390
to
+397
| /// Connect to a running sandbox via WebSocket | ||
| Connect { | ||
| /// Sandbox ID | ||
| id: String, | ||
|
|
||
| /// exec-agent secret (from create response). Prompted if not provided. | ||
| #[arg(long)] | ||
| secret: Option<String>, |
Contributor
There was a problem hiding this comment.
Don't expose the exec secret as a normal CLI argument.
This is a bearer credential, and --secret ... will leak into shell history and often process listings. The secure prompt path already exists in the handler, so the non-interactive path should be stdin/secret-file based instead of a plain flag.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/basilica-cli/src/cli/commands.rs` around lines 390 - 397, The Connect
enum variant exposes exec secret via a plain --secret flag which can leak;
change the CLI to accept the secret non-interactively from stdin or a file
instead of a regular flag: remove or stop using #[arg(long)] on the existing
secret: Option<String> in the Connect variant and add a new Option<PathBuf>
(e.g. secret_file) with #[arg(long = "secret-file")] (accept "-" to mean read
from stdin), then update the connect handler that currently prompts for secret
to read from secret_file (reading file contents or stdin) before falling back to
the secure interactive prompt; ensure functions handling the secret (the Connect
match arm / handler) use the new secret_file path and do not log or echo the
secret.
Comment on lines
+75
to
+85
| let response: CreateSandboxResponse = client.create_sandbox(request).await.map_err(|e| { | ||
| complete_spinner_error(spinner.clone(), "Failed to create sandbox"); | ||
| match &e { | ||
| basilica_sdk::ApiError::ApiResponse { status, message } if *status == 402 => { | ||
| CliError::Internal( | ||
| eyre!("Insufficient balance: {}", message) | ||
| .wrap_err("Top up your account with: basilica fund"), | ||
| ) | ||
| } | ||
| _ => CliError::Api(e), | ||
| } |
Contributor
There was a problem hiding this comment.
This 402 special-case doesn't compile against the current SDK error enum.
basilica_sdk::ApiError doesn't have an ApiResponse { status, message } variant, so this match arm fails at compile time. If you want payment-required handling here, the SDK needs a status-carrying error first; otherwise this should just fall back to CliError::Api(e).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/basilica-cli/src/cli/handlers/sandbox.rs` around lines 75 - 85, The
special-case match for basilica_sdk::ApiError::ApiResponse { status, message }
in the client.create_sandbox error handling is invalid because the current SDK
enum has no ApiResponse variant; update the error handler in the create_sandbox
call (the map_err closure around client.create_sandbox) to remove that
non-existent pattern and simply convert all errors to CliError::Api(e) after
calling complete_spinner_error(spinner.clone(), "Failed to create sandbox"); if
you do need a 402/payment-specific path later, instead add a status-carrying
error variant to the SDK and then reintroduce a match for that new variant (or
provide a helper on basilica_sdk::ApiError to inspect HTTP status).
Comment on lines
+377
to
+382
| let request = SandboxWsRequest { | ||
| id: op_id.clone(), | ||
| op: "exec".to_string(), | ||
| args: Some(serde_json::json!({ | ||
| "command": command.split_whitespace().collect::<Vec<&str>>() | ||
| })), |
Contributor
There was a problem hiding this comment.
Quoted commands are being split into the wrong argv.
Both the one-shot --exec path and the interactive exec fallback use split_whitespace(), so commands like python -c "print(1)" or paths with spaces arrive mangled. Use a shell-aware parser here, or send the raw string through /bin/sh -lc if the exec-agent expects shell semantics.
Also applies to: 637-640
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/basilica-cli/src/cli/handlers/sandbox.rs` around lines 377 - 382, The
code builds SandboxWsRequest for op "exec" by splitting the command with
split_whitespace(), which mangles quoted args; update the handler(s) where
SandboxWsRequest is created (the blocks using op "exec" around the uses of
command and op_id) to send shell-aware argv instead: either parse the command
with a shell-aware parser (e.g., use shell_words::split on the command string
and pass that Vec) or, more simply and reliably for shell semantics, set args to
a single argv array that invokes the shell (e.g., ["sh", "-lc", command]) so
commands like python -c "print(1)" and paths with spaces are preserved. Ensure
both occurrences mentioned (the one-shot --exec path and the interactive exec
fallback) are changed consistently.
Comment on lines
+396
to
+452
| // Read frames until we get an exit or error for this op | ||
| while let Some(frame) = ws.next().await { | ||
| let frame = frame.map_err(|e| CliError::Internal(eyre!("WebSocket read error: {}", e)))?; | ||
|
|
||
| match frame { | ||
| tungstenite::Message::Text(text) => { | ||
| let resp: SandboxWsResponse = serde_json::from_str(&text) | ||
| .map_err(|e| CliError::Internal(eyre!("Invalid response frame: {}", e)))?; | ||
|
|
||
| if resp.id != op_id { | ||
| continue; | ||
| } | ||
|
|
||
| match resp.response_type.as_str() { | ||
| "stdout" => { | ||
| if let Some(data) = &resp.data { | ||
| let s = data.as_str().unwrap_or(""); | ||
| if json { | ||
| stdout_buf.push_str(s); | ||
| } else { | ||
| print!("{}", s); | ||
| } | ||
| } | ||
| } | ||
| "stderr" => { | ||
| if let Some(data) = &resp.data { | ||
| let s = data.as_str().unwrap_or(""); | ||
| if json { | ||
| stderr_buf.push_str(s); | ||
| } else { | ||
| eprint!("{}", s); | ||
| } | ||
| } | ||
| } | ||
| "exit" => { | ||
| exit_code = resp.code; | ||
| break; | ||
| } | ||
| "error" => { | ||
| let msg = resp.error.unwrap_or_else(|| "Unknown error".to_string()); | ||
| let code = resp.error_code.unwrap_or_default(); | ||
| return Err(CliError::Internal(eyre!( | ||
| "exec-agent error [{}]: {}", | ||
| code, | ||
| msg | ||
| ))); | ||
| } | ||
| _ => {} | ||
| } | ||
| } | ||
| tungstenite::Message::Close(_) => break, | ||
| _ => {} | ||
| } | ||
| } | ||
|
|
||
| // Close WebSocket | ||
| let _ = ws.close(None).await; |
Contributor
There was a problem hiding this comment.
Don't return success when the socket dies before an exit frame.
If the agent closes the connection mid-command, we break out of the loop and still return Ok(()), which makes scripts treat a dropped exec session as a successful run. Return an error whenever the stream ends without an exit response.
🐛 Suggested fix
while let Some(frame) = ws.next().await {
let frame = frame.map_err(|e| CliError::Internal(eyre!("WebSocket read error: {}", e)))?;
match frame {
@@
tungstenite::Message::Close(_) => break,
_ => {}
}
}
+ if exit_code.is_none() {
+ return Err(CliError::Internal(eyre!(
+ "WebSocket closed before the exec operation returned an exit status"
+ )));
+ }
+
// Close WebSocket
let _ = ws.close(None).await;Also applies to: 462-466
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/basilica-cli/src/cli/handlers/sandbox.rs` around lines 396 - 452, The
loop currently returns success when the websocket stream ends without receiving
an "exit" response; add a boolean flag (e.g., seen_exit) initialized false, set
seen_exit = true inside the "exit" arm (where exit_code is assigned), and after
the read loop (before closing ws) check if seen_exit is false and return an
Err(CliError::Internal(eyre!("WebSocket closed before exit frame"))) so
dropped/died exec sessions are treated as errors; apply the same check/update
for the similar block referenced around the other occurrence (lines ~462-466)
and reference SandboxWsResponse, ws.next(), the "exit" arm, and exit_code when
locating changes.
Comment on lines
+471
to
+550
| /// Interactive sandbox session — read commands, send exec ops, display output | ||
| async fn interactive_session(domain: &str, secret: &str) -> Result<(), CliError> { | ||
| let (mut ws, _) = connect_ws(domain, secret).await?; | ||
|
|
||
| println!( | ||
| "{} Connected to {}", | ||
| style("●").green(), | ||
| style(domain).cyan() | ||
| ); | ||
| println!( | ||
| "{}", | ||
| style("Type commands to execute. Ctrl+C or 'exit' to disconnect.").dim() | ||
| ); | ||
| println!( | ||
| "{}", | ||
| style("File ops: :read <path>, :write <path> <content>, :ls [path], :stat <path>, :mv <old> <new>").dim() | ||
| ); | ||
| println!(); | ||
|
|
||
| let stdin = tokio::io::stdin(); | ||
| let reader = tokio::io::BufReader::new(stdin); | ||
| let mut lines = tokio::io::AsyncBufReadExt::lines(reader); | ||
|
|
||
| loop { | ||
| // Print prompt | ||
| eprint!("{} ", style("sandbox>").green().bold()); | ||
|
|
||
| let line = match lines.next_line().await { | ||
| Ok(Some(line)) => line, | ||
| Ok(None) => break, // EOF | ||
| Err(_) => break, | ||
| }; | ||
|
|
||
| let trimmed = line.trim(); | ||
| if trimmed.is_empty() { | ||
| continue; | ||
| } | ||
| if trimmed == "exit" || trimmed == "quit" { | ||
| break; | ||
| } | ||
|
|
||
| // Check for file op shortcuts | ||
| let (op, args) = parse_interactive_command(trimmed); | ||
|
|
||
| let op_id = uuid::Uuid::new_v4().to_string(); | ||
| let request = SandboxWsRequest { | ||
| id: op_id.clone(), | ||
| op, | ||
| args: Some(args), | ||
| }; | ||
|
|
||
| let msg = serde_json::to_string(&request) | ||
| .map_err(|e| CliError::Internal(eyre!("Failed to serialize: {}", e)))?; | ||
|
|
||
| if ws.send(tungstenite::Message::Text(msg)).await.is_err() { | ||
| eprintln!("{} Connection lost. Attempting reconnect...", style("●").red()); | ||
| match connect_ws(domain, secret).await { | ||
| Ok((new_ws, _)) => { | ||
| ws = new_ws; | ||
| println!("{} Reconnected", style("●").green()); | ||
| continue; | ||
| } | ||
| Err(e) => { | ||
| eprintln!("Reconnect failed: {}", e); | ||
| eprintln!("Create a new sandbox with: basilica sandbox create"); | ||
| return Err(e); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Read response frames for this op | ||
| let done = read_op_response(&mut ws, &op_id).await?; | ||
| if done { | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| let _ = ws.close(None).await; | ||
| println!("Disconnected."); | ||
| Ok(()) |
Contributor
There was a problem hiding this comment.
The interactive REPL currently self-destructs after ~120s of user idle time.
This loop waits on stdin forever but never emits protocol pings or polls the socket between commands, and the same file documents that idle sandboxes are reaped after 120 seconds of no WebSocket activity. An open session where the user pauses to read output or think will get torn down underneath them.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/basilica-cli/src/cli/handlers/sandbox.rs` around lines 471 - 550,
interactive_session is blocking on stdin and never sends WebSocket activity, so
idle sandboxes are reaped; fix by ensuring the socket is kept alive while
waiting for user input: either spawn a background task (or use tokio::select!)
that periodically sends a ping/text heartbeat on the ws (e.g., every 30s) and/or
polls the socket for incoming frames while the REPL waits, using the existing
connect_ws and read_op_response functions to detect disconnects and trigger
reconnect logic; reference interactive_session, connect_ws, read_op_response,
and parse_interactive_command when adding the heartbeat/polling logic so the
session maintains activity during user idle periods.
epappas
approved these changes
Apr 8, 2026
epappas
left a comment
epappas
left a comment
Contributor
There was a problem hiding this comment.
needs testing, but i don't see an obvious issue
epappas
reviewed
Apr 8, 2026
Contributor
There was a problem hiding this comment.
sdk doesn't implement how to integrate with the sandbox agent
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
basilica-exec-agentcrate — in-pod binary handling WebSocket connections, command execution, filesystem ops (read_file,write_file,list_dir), and R2 upload/download, authenticated viaX-Exec-Secretheaderbasilica-sandbox-operatorcrate — K8s operator reconcilingBasilicaSandboxCRD with TTL-based cleanup, billing bridge (credits deducted per-second), and Prometheus metricssandbox-routercrate — Envoy GatewayBackendRefthat resolvesHost: sb-{id}.sandboxes.basilica.aitosandbox-{id}.u-{user_id}.svc.cluster.local:9999using an in-memory CRD informer cachebasilica-api(POST /v1/sandboxes,GET /v1/sandboxes/{id},DELETE /v1/sandboxes/{id},GET /v1/sandboxes) withMultiClusterK8sClientfor cross-cluster CRD management (fail-hard, no silent fallback per D-2)CloudType::Sandboxvariant tobasilica-billingwith migration046_sandbox_cloud_type.sqlBasilicaSandboxCRD, operator deployment/RBAC, sandbox-router deployment/service/RBAC, Envoy Gateway config (wildcard HTTPRoute*.sandboxes.basilica.ai), image pre-pull DaemonSetsandbox_k3s_server,sandbox_k3s_agent,sandbox_gateway,sandbox_operator,sandbox_networking,sandbox_prepullersandbox_k8s_integrationtest inintegration-testscrate covering CRD creation, status transitions, and cleanupcargo testValidation
cargo test --workspacepasses: 493+ tests (123 aggregator, 357 api, 8 verda, 5 masscompute) with 0 failuressandbox_k8s_integrationvalidates CRD lifecycle against mock K8s APIcargo vetregressionsRisks
basilica-sandbox-operatorcontroller at 777 LOC exceeds the 300 LOC guideline in the workspace CLAUDE.md; may warrant splitting before further iterationMultiClusterK8sClientfail-hard behavior (D-2) means sandbox API routes return 503 if sandbox cluster kubeconfig is missing or invalid at startupCloses one-covenant/basilica-backend#271
Summary by CodeRabbit