Conversation
Introduces the cloud workspaces feature for running Claude Code in remote Modal sandboxes with real-time event streaming. Control Plane (Cloudflare Workers): - Session Durable Objects with SQLite storage for session state - WebSocket support for real-time event streaming - REST API for session management and sandbox spawning - HMAC token authentication for Modal API Modal Sandbox (Python): - Sandbox execution environment in Modal containers - Git clone and branch management - Claude Code CLI execution with event streaming - Health, create, terminate, snapshot, restore endpoints Desktop App: - CloudWorkspaceRuntime for WebSocket connection - Cloud workspace sidebar section - Event streaming to terminal-like UI Web App: - Cloud workspace route with real-time event display Database: - Cloud workspaces schema with sessions, participants, events tRPC: - Cloud workspace router for CRUD operations Note: Migration will need to be generated with drizzle-kit.
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the
📝 WalkthroughWalkthroughIntroduces a comprehensive cloud workspaces feature spanning desktop, web, control plane, and sandbox infrastructure. Adds Cloudflare Workers-based control plane with Durable Objects for session state, Modal-based Python sandbox orchestration, cloud workspace database schema, TRPC API procedures, desktop client runtime and UI components, and web UI pages. Enables real-time WebSocket communication between desktop and cloud, sandbox pre-warming, git operations, and Claude Code execution. Changes
Sequence Diagram(s)sequenceDiagram
participant DC as Desktop Client
participant CP as Control Plane<br/>(Cloudflare Workers)
participant DO as SessionDO<br/>(Durable Object)
participant SB as Modal Sandbox
participant DB as Database
DC->>CP: POST /api/sessions<br/>(create session)
CP->>CP: Generate sessionId<br/>& sandboxToken
CP->>DB: Store token hash<br/>in KV
CP-->>DC: Return sessionId,<br/>sandboxToken
DC->>CP: GET /api/sessions/:sessionId/ws<br/>(WebSocket upgrade)
CP->>DO: Route to SessionDO
DO->>DO: Initialize SQLite schema
DO-->>DC: WebSocket established
DC->>DC: CloudTerminalRuntime<br/>ready
DC->>DO: Send prompt via WebSocket
DO->>SB: Forward prompt to sandbox
SB->>SB: Run Claude Code
SB->>DO: Emit events (tokens,<br/>tool calls, results)
DO->>DO: Persist events<br/>in SQLite
DO-->>DC: Broadcast state updates
DC->>DC: Update UI with<br/>real-time events
SB->>DO: POST /internal/sandbox-event<br/>(status update)
DO->>DB: Update sandbox_status
DO-->>DC: Notify status change
sequenceDiagram
participant Web as Web App
participant Auth as Auth Service
participant API as TRPC Server
participant DB as Database
participant CP as Control Plane
Web->>Auth: Get session from headers
Auth-->>Web: User authenticated
Web->>API: trpc.cloudWorkspace<br/>.getBySessionId(sessionId)
API->>DB: Query cloudWorkspaces<br/>by sessionId
DB-->>API: Return workspace record
API-->>Web: Return CloudWorkspace
Web->>Web: Render CloudWorkspaceContent
Web->>Web: Create iframe pointing to<br/>Cloud Control Plane
Web-->>User: Display workspace UI
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Important Action Needed: IP Allowlist UpdateIf your organization protects your Git platform with IP whitelisting, please add the new CodeRabbit IP address to your allowlist:
Reviews will stop working after February 8, 2026 if the new IP is not added to your allowlist. 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 |
🚀 Preview Deployment🔗 Preview Links
Preview updates automatically with new commits |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🤖 Fix all issues with AI agents
In `@apps/desktop/src/main/lib/workspace-runtime/cloud.ts`:
- Around line 105-109: The onclose handler always calls attemptReconnect,
causing intentional disconnects to auto-reconnect; add a boolean flag (e.g.,
this.manualDisconnect or this.shouldReconnect) on the class to indicate an
intentional/controlled disconnect, set it true in the disconnect() method, and
update the onclose handlers (the this.ws.onclose that currently calls
stopPingInterval(), emit("disconnect"), attemptReconnect()) to only call
attemptReconnect() when the flag is false; ensure disconnect() clears/resets the
flag appropriately and that other places creating or reopening sockets reset the
flag to false before connecting.
In `@packages/control-plane/src/index.ts`:
- Around line 47-52: The session routes currently only check for a "Bearer "
prefix in app.post("/api/sessions", ...) and leave all other /api/sessions/*
handlers (create, list/get, WS upgrade, events/messages, spawn, terminate)
unauthenticated; replace the naive prefix check with real token validation
(extract the bearer token, call the central verifyToken/verifyJWT function or
auth service to validate and decode it) and enforce session ownership on every
handler by comparing the decoded token subject/owner id to the session.ownerId
before returning or mutating session state; make sure the WS upgrade handler
performs the same token validation and ownership check prior to upgrading the
socket, and return proper 401/403 responses for missing/invalid token or
unauthorized access.
- Around line 99-101: The code currently forwards limit/offset as strings and
uses magic TTL numbers; parse limit and offset into integers (Number.parseInt or
+) and clamp them within named constants (e.g., DEFAULT_LIMIT, MAX_LIMIT,
DEFAULT_OFFSET, MAX_OFFSET), use DEFAULT_* when parsing fails or values are
negative, and replace literal TTL (86400 * 7) with a SESSION_TOKEN_TTL_SECONDS
constant (e.g., DAYS_TO_SECONDS * 7 or SESSION_TOKEN_TTL_SECONDS). Update all
usages that accept pagination (variables named limit/offset) and the
SESSION_TOKENS.put call (and the similar occurrences around the other noted
ranges) to use the parsed/clamped values and the new TTL constant. Ensure
constants are declared at the module top so they replace the magic numbers
across the file.
- Around line 340-361: You fetch sandboxTokenHash from SESSION_TOKENS but never
validate or persist the newly generated sandboxToken; either validate against
the stored hash or store the new token hash before calling
modalClient.createSandbox. Update the flow in the block around sandboxTokenHash,
generateSandboxToken, and modalClient.createSandbox: if you intend to reuse the
stored token, verify generateSandboxToken() (or the provided token) against
sandboxTokenHash; if you intend to rotate, hash the new sandboxToken and persist
it back to SESSION_TOKENS (e.g., using the same key
`session:${sessionId}:sandbox_token`) before invoking modalClient.createSandbox
so the control plane and modal can validate the token.
- Around line 22-35: The CORS origin check in the app.use("*", cors(...)) origin
callback is too permissive; update the origin handler to parse the incoming
origin with new URL(origin).hostname (catch invalid origins) and then validate
the hostname explicitly (e.g., hostname === "superset.sh" ||
hostname.endsWith(".superset.sh") for subdomains, and hostname === "localhost"
or hostname === "127.0.0.1" for local dev) instead of using includes; also
ensure you never return "*" when credentials: true — return the original origin
string when allowed or false/null when denied. Locate the origin callback in the
cors options in packages/control-plane/src/index.ts to implement this change.
In `@packages/control-plane/src/session/durable-object.ts`:
- Around line 252-275: The handleSubscribe function currently accepts every
client because the token is ignored; validate the provided token first (using
your project's auth/token verifier) and only create and set ClientInfo and add
to this.clients after successful validation; if validation fails, send an error
message via safeSend or close the WebSocket and do not expose session state.
Ensure you extract authenticated user fields (userId, userName, authenticatedAt,
source) from the token verification to populate ClientInfo instead of hardcoding
"anonymous", and only clear attachment.timeoutId after the client is
accepted/validated to avoid clearing timeouts for unauthenticated connections;
keep the existing getSessionState and safeSend usage but call them only after
validation succeeds.
In `@packages/control-plane/src/types.ts`:
- Line 33: SessionStatus in control-plane includes "paused" but the DB enum
cloudSessionStatusValues in packages/db/src/schema/enums.ts lacks it; make them
consistent by adding "paused" to cloudSessionStatusValues (or alternatively
remove "paused" from the SessionStatus type). Update cloudSessionStatusValues to
include "paused", then regenerate/type-check any schema artifacts and run tests
to ensure no enum mismatches remain; reference the SessionStatus type and
cloudSessionStatusValues symbol names when making the change.
In `@packages/db/src/schema/enums.ts`:
- Around line 44-75: The cloud model enum uses outdated identifiers—update the
array cloudModelValues and its derived enum cloudModelEnum (type CloudModel) to
the current Anthropic model IDs by replacing "claude-sonnet-4" →
"claude-sonnet-4-20250514", "claude-opus-4" → "claude-opus-4-20250514", and
"claude-haiku-3-5" → "claude-3-5-haiku-20241022"; if these values are only
internal keys rather than direct API IDs, instead add/adjust a clear mapping
layer and documentation that maps the enum values from cloudModelValues to the
actual API model strings used in requests.
In `@packages/sandbox/app.py`:
- Around line 553-558: The api_warm_sandbox endpoint (and the other internal
endpoints noted) lack the Bearer-token check used by api_create_sandbox; add the
same authentication logic to each internal function by extracting the
Authorization header, validating the "Bearer <token>" value against the same
secret or helper used by api_create_sandbox (or call the shared
validate_bearer_token(request) function if present), and return a 401/raise an
unauthorized error when validation fails so only authorized callers can invoke
api_warm_sandbox and the other internal endpoints.
- Around line 77-102: In verify_internal_token, currently an empty or missing
secret allows token verification to proceed; update the function to immediately
return False if the secret is falsy (e.g., empty string or None) before
computing or comparing HMACs. Locate the verify_internal_token function and add
a guard that checks the secret parameter and fails closed (return False) when
secret is not set, keeping the existing timestamp parsing, expiration check, and
HMAC verification (hmac.new, hashlib.sha256, hmac.compare_digest) unchanged.
In `@packages/sandbox/sandbox/app.py`:
- Around line 264-270: The internal endpoints (e.g., api_warm_sandbox and the
snapshot/restore/terminate endpoints referenced in the comment) currently skip
authentication; replicate the same Bearer token verification used in
api_create_sandbox: extract the Authorization header, validate the Bearer token
against the internal token check used by api_create_sandbox, and return a
401/unauthorized response on failure before proceeding; apply this change to
api_warm_sandbox and each internal endpoint in the ranges noted so they all
enforce identical token validation logic.
- Around line 52-76: The verify_internal_token function currently accepts an
empty secret which makes verification meaningless; update verify_internal_token
to immediately fail (return False) when the provided secret is empty or falsy
before attempting to parse or verify the token (i.e., check secret at the top of
the function), so that callers using MODAL_API_SECRET that is missing will be
rejected and misconfigurations are visible; keep the rest of the logic
(timestamp parsing, expiration check, HMAC compare via hmac.compare_digest)
unchanged.
In `@packages/sandbox/sandbox/runner.py`:
- Around line 29-123: The run_prompt method currently builds a fresh env dict
that sets "ANTHROPIC_API_KEY" to "" which discards Modal-injected secrets;
change it to start from os.environ.copy() and only set/override the specific
keys you need (e.g., "CLAUDE_CODE_NO_INTERACTIVE", "HOME", "PATH") so you do not
clobber ANTHROPIC_API_KEY; in run_prompt, after copying os.environ, optionally
check that os_env.get("ANTHROPIC_API_KEY") exists and emit an error (via
self.emitter.emit_error) or raise early if missing before launching subprocess
(self.process = subprocess.Popen(...)).
In `@packages/ui/src/components/ai-elements/live-preview.tsx`:
- Around line 87-123: The useEffect that syncs defaultUrl reads the state
variable url without including it in deps which Biome flags; fix by adding a ref
(e.g., latestUrlRef) to track the current url and update that ref inside setUrl
(and wherever url is updated, e.g., refresh and initial state setup) so the
useEffect only depends on defaultUrl and reads latestUrlRef.current instead of
url, and when defaultUrl !== latestUrlRef.current call setUrlState(defaultUrl)
and setState(defaultUrl ? "loading" : "empty") so behavior stays the same
without adding url to the effect deps.
- Around line 536-548: The SVG elements in the live-preview component are
missing accessible titles (causing Biome a11y rule noSvgWithoutTitle failures);
for each SVG (the image/placeholder icons in
packages/ui/src/components/ai-elements/live-preview.tsx) add a <title> with a
short descriptive string, give the title a unique id (e.g.,
"livePreviewImageTitle" / "livePreviewPlaceholderTitle") and reference it from
the <svg> using aria-labelledby plus role="img" (or aria-label if you prefer) so
screen readers pick up the description; ensure both SVG occurrences (the one in
the shown diff and the other similar SVG later) receive unique title ids and
aria-labelledby attributes.
🟡 Minor comments (12)
packages/ui/src/components/ai-elements/live-preview.tsx-182-195 (1)
182-195:⚠️ Potential issue | 🟡 MinorPropagate consumer
onOpenChangecallbacks.
LivePreviewCollapsibleoverridesonOpenChangebut doesn't call a user-supplied handler, which can break integrations. Forward it after updating context.💡 Suggested fix
export const LivePreviewCollapsible = ({ className, children, + onOpenChange, ...props }: LivePreviewCollapsibleProps) => { const { isCollapsed, setIsCollapsed } = useLivePreview(); return ( <Collapsible className={cn("border-t", className)} - onOpenChange={(open) => setIsCollapsed(!open)} + onOpenChange={(open) => { + setIsCollapsed(!open); + onOpenChange?.(open); + }} open={!isCollapsed} {...props} >apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/CloudWorkspacesSection/CloudWorkspaceListItem.tsx-48-52 (1)
48-52:⚠️ Potential issue | 🟡 MinorValidate URL protocol before opening external links.
window.openwith arbitrary URLs could be a security risk ifworkspace.prUrlis manipulated. Consider validating that the URL uses a safe protocol (https).🛡️ Suggested fix
const handleOpenPR = () => { - if (workspace.prUrl) { + if (workspace.prUrl && workspace.prUrl.startsWith("https://")) { window.open(workspace.prUrl, "_blank"); } };packages/control-plane/src/sandbox/client.ts-320-337 (1)
320-337:⚠️ Potential issue | 🟡 MinorLog failures and use a params object for snapshot lookup.
getLatestSnapshotsilently returnsnullon non‑OK or unsuccessful responses and uses positional params. Log with context and switch to a params object.As per coding guidelines, functions with 2+ parameters should accept a single params object, and errors must be logged with context using the `[domain/operation] message` prefix instead of silently returning null.🛠️ Proposed fix
- async getLatestSnapshot(repoOwner: string, repoName: string): Promise<SnapshotInfo | null> { - const url = `${this.snapshotUrl}?repo_owner=${encodeURIComponent(repoOwner)}&repo_name=${encodeURIComponent(repoName)}`; + async getLatestSnapshot(params: { repoOwner: string; repoName: string }): Promise<SnapshotInfo | null> { + const { repoOwner, repoName } = params; + const url = `${this.snapshotUrl}?repo_owner=${encodeURIComponent(repoOwner)}&repo_name=${encodeURIComponent(repoName)}`; @@ - if (!response.ok) { - return null; - } + if (!response.ok) { + console.warn("[sandbox/getLatestSnapshot] Modal API error:", response.status); + return null; + } @@ - if (!result.success) { - return null; - } + if (!result.success) { + console.warn("[sandbox/getLatestSnapshot] Modal API error:", result.error); + return null; + }packages/sandbox/app.py-505-505 (1)
505-505:⚠️ Potential issue | 🟡 MinorMove FastAPI imports to the top of the module.
Linting reports E402; keep all imports with the top-level import block.
🧹 Proposed fix
-import modal -from modal import Image, Secret, Volume, method, enter +import modal +from modal import Image, Secret, Volume, method, enter +from fastapi import Request, Response @@ -# ============================================================================= -# API Endpoints (using FastAPI) -# ============================================================================= - -from fastapi import Request, Response +# ============================================================================= +# API Endpoints (using FastAPI) +# =============================================================================apps/desktop/src/renderer/screens/main/components/CloudWorkspaceView/CloudWorkspaceView.tsx-49-69 (1)
49-69:⚠️ Potential issue | 🟡 MinorRemove unnecessary dependency from useEffect.
The static analysis tool correctly identifies that
workspace?.sessionIdis not needed in the dependency array. The event listeners only need to be attached once when the webview element is available, andwebviewRefdoesn't need to be in dependencies since refs are stable.🔧 Proposed fix
useEffect(() => { const webview = webviewRef.current; if (!webview) return; const handleDidFailLoad = (event: Event) => { const e = event as CustomEvent; console.error("[CloudWorkspaceView] Failed to load:", e.detail); }; const handleDidFinishLoad = () => { console.log("[CloudWorkspaceView] Finished loading"); }; webview.addEventListener("did-fail-load", handleDidFailLoad); webview.addEventListener("did-finish-load", handleDidFinishLoad); return () => { webview.removeEventListener("did-fail-load", handleDidFailLoad); webview.removeEventListener("did-finish-load", handleDidFinishLoad); }; - }, [workspace?.sessionId]); + }, []);packages/trpc/src/router/cloud-workspace/cloud-workspace.ts-114-116 (1)
114-116:⚠️ Potential issue | 🟡 MinorValidate
sessionIdas UUID.
Session IDs are generated viacrypto.randomUUID(), so input should enforce UUIDs.🛠️ Suggested fix
getBySessionId: protectedProcedure - .input(z.object({ sessionId: z.string() })) + .input(z.object({ sessionId: z.string().uuid() }))apps/web/src/app/cloud/[sessionId]/page.tsx-27-35 (1)
27-35:⚠️ Potential issue | 🟡 MinorDon’t swallow fetch errors—log with a prefixed message.
The catch block drops errors silently; add a prefixed log beforenotFound().🛠️ Suggested fix
- } catch { - notFound(); - } + } catch (error) { + console.error("[cloud-workspace/page] Failed to load workspace", { + sessionId, + error, + }); + notFound(); + }apps/desktop/docs/CLOUD_ARCHITECTURE.md-40-75 (1)
40-75:⚠️ Potential issue | 🟡 MinorAdd a language tag to the architecture ASCII fence (MD040).
This satisfies the fenced-code language requirement while keeping the diagram unchanged.🧹 Suggested fix
-``` +```textpackages/trpc/src/router/cloud-workspace/cloud-workspace.ts-35-41 (1)
35-41:⚠️ Potential issue | 🟡 MinorAvoid contradictory filters when status = "archived".
listalways excludes archived, but the schema allows it—requests will return empty results. Add a guard or adjust the schema.🛠️ Suggested guard
const { status, repositoryId, limit = 50, offset = 0 } = input ?? {}; + if (status === "archived") { + throw new TRPCError({ + code: "BAD_REQUEST", + message: "Use listArchived for archived workspaces", + }); + }apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/CloudWorkspacesSection/CloudWorkspacesSection.tsx-54-73 (1)
54-73:⚠️ Potential issue | 🟡 MinorLog query/mutation failures with prefixed context.
To meet the error-handling guideline, log failures in the list query and archive mutation.🛠️ Suggested fix
} = apiTrpc.cloudWorkspace.list.useQuery(undefined, { staleTime: 30_000, // 30 seconds retry: false, // Don't retry on failure refetchOnWindowFocus: false, // Prevent unnecessary refetches placeholderData: (prev) => prev, // Keep previous data while refetching + onError: (error) => { + console.error("[cloud-workspaces/list] Failed to load cloud workspaces", error); + }, }); const archiveMutation = apiTrpc.cloudWorkspace.archive.useMutation({ onSuccess: () => { toast.success("Workspace archived"); }, onError: (error) => { + console.error("[cloud-workspaces/archive] Failed to archive workspace", error); toast.error(`Failed to archive: ${error.message}`); }, });packages/control-plane/src/session/durable-object.ts-54-64 (1)
54-64:⚠️ Potential issue | 🟡 MinorLog send failures and standardize log prefixes.
safeSendswallows errors, and log prefixes don’t follow[domain/operation].🛠️ Suggested fix
private safeSend(ws: WebSocket, message: ServerMessage): boolean { try { if (ws.readyState !== WebSocket.OPEN) { return false; } ws.send(JSON.stringify(message)); return true; - } catch { + } catch (error) { + console.error("[control-plane/session-do/ws-send] Failed to send message", error); return false; } } ... - console.error("[SessionDO] WebSocket message error:", error); + console.error("[control-plane/session-do/websocket-message] Invalid message", error); ... - console.error("[SessionDO] WebSocket error:", error); + console.error("[control-plane/session-do/websocket-error] WebSocket error", error); ... - console.log("[SessionDO] Stop requested by client"); + console.info("[control-plane/session-do/stop] Stop requested by client");Also applies to: 223-225, 245-246, 339-340
apps/desktop/docs/CLOUD_ARCHITECTURE.md-19-31 (1)
19-31:⚠️ Potential issue | 🟡 MinorFix table separator spacing to satisfy markdownlint MD060.
The separator rows need spaces around the pipes in both tables.🧹 Suggested fix
-|-------|------------| +| --- | --- | ... -|-------|------------| +| --- | --- |
🧹 Nitpick comments (27)
packages/ui/src/components/ai-elements/live-preview.tsx (1)
1-35: Split Live Preview into per-component files.This module exports many components in one file, which makes navigation and tree-shaking harder. Consider moving each component into
ComponentName/ComponentName.tsxunder a folder and re-exporting viaindex.ts.As per coding guidelines,
**/components/**/*.tsx: Use folder structure with one component per file:ComponentName/ComponentName.tsxwith barrel export inindex.ts.packages/sandbox/README.md (1)
42-55: Add a language identifier to the fenced code block.The architecture diagram code block should have a language identifier for proper rendering. Use
textorplaintextfor ASCII diagrams.📝 Suggested fix
-``` +```text Control Plane (Cloudflare Workers)apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/CloudWorkspacesSection/CloudWorkspaceListItem.tsx (2)
27-35: Consider renaming"permission"mapping for clarity.Using
"permission"status to represent"failed"is semantically confusing—the comment helps but readers may still be misled. IfActivePaneStatussupports an"error"or"failed"variant, prefer that; otherwise, a brief inline comment at each usage site would improve readability.
93-108: Context menu content is duplicated.The "Open PR" and "Archive" menu items appear identically in both collapsed and expanded modes. Consider extracting a shared component or fragment to reduce duplication.
♻️ Example extraction
const ContextMenuItems = () => ( <> {workspace.prUrl && ( <> <ContextMenuItem onSelect={handleOpenPR}> <LuExternalLink className="size-4 mr-2" strokeWidth={STROKE_WIDTH} /> Open PR </ContextMenuItem> <ContextMenuSeparator /> </> )} {onArchive && ( <ContextMenuItem onSelect={onArchive}> <LuArchive className="size-4 mr-2" strokeWidth={STROKE_WIDTH} /> Archive </ContextMenuItem> )} </> );Then use
<ContextMenuItems />in both<ContextMenuContent>blocks.Also applies to: 193-208
packages/control-plane/src/auth/internal.ts (3)
40-40: Extract magic number to a named constant.The default
maxAgeMsvalue5 * 60 * 1000should be extracted to a module-level constant for clarity and easier configuration changes.Suggested fix
+const DEFAULT_TOKEN_MAX_AGE_MS = 5 * 60 * 1000; // 5 minutes + export async function verifyInternalToken( token: string, secret: string, - maxAgeMs = 5 * 60 * 1000, + maxAgeMs = DEFAULT_TOKEN_MAX_AGE_MS, ): Promise<boolean> {As per coding guidelines: "Avoid magic numbers by extracting them to named constants at module top".
54-58: Consider rejecting tokens with future timestamps.The verification only checks if the token is too old but doesn't reject tokens with timestamps in the future. An attacker with clock drift or manipulation could generate tokens that remain valid longer than intended.
Suggested fix
// Check if token is expired const now = Date.now(); + // Reject tokens from the future (with small grace for clock skew) + const clockSkewGraceMs = 30_000; // 30 seconds + if (timestamp > now + clockSkewGraceMs) { + return false; + } if (now - timestamp > maxAgeMs) { return false; }
70-72: Validate hex string format before parsing.The hex parsing silently falls back to an empty array if the regex match fails or contains invalid hex characters. Invalid hex chars like
"zz"would produceNaNviaparseInt, leading to signature bytes containingNaN. Consider adding explicit validation.Suggested fix
+ // Validate hex string format + if (!/^[0-9a-fA-F]{64}$/.test(signatureHex)) { + return false; + } + const signatureBytes = new Uint8Array( signatureHex.match(/.{2}/g)?.map((byte) => parseInt(byte, 16)) || [], );packages/db/src/schema/cloud-workspaces.ts (2)
42-42: Redundant unique constraint onsessionId.The
sessionIdcolumn has both a standalone.unique()constraint (line 42) and is part of a composite unique constraint withorganizationId(lines 83-86). Since the standalone unique already guarantees global uniqueness, the composite unique is redundant. Consider removing one based on your actual requirements:
- If
sessionIdshould be globally unique across all orgs: keep only.unique()on line 42- If
sessionIdonly needs to be unique within an org: remove.unique()from line 42 and keep only the compositeAlso applies to: 83-86
79-79: Index onsessionIdis redundant.The unique constraint on
sessionId(line 42) already creates an implicit index in PostgreSQL. The explicit index at line 79 is redundant and can be removed.Suggested removal
index("cloud_workspaces_user_id_idx").on(table.userId), - index("cloud_workspaces_session_id_idx").on(table.sessionId), index("cloud_workspaces_status_idx").on(table.status),packages/sandbox/sandbox/config.py (1)
24-33: Consider usingStrEnumfor better type safety.The
EventTypeclass uses string class attributes, which works but doesn't provide the same type safety as Python'sStrEnum. This is a minor improvement that would enable better static analysis.Suggested alternative using StrEnum
from enum import StrEnum class EventType(StrEnum): """Event types sent to the control plane.""" TOOL_CALL = "tool_call" TOOL_RESULT = "tool_result" TOKEN = "token" ERROR = "error" GIT_SYNC = "git_sync" EXECUTION_COMPLETE = "execution_complete" HEARTBEAT = "heartbeat"packages/sandbox/sandbox/events.py (2)
43-44: Improve error handling and logging.The error handler only prints to stdout, which may be lost in production environments. Consider using Python's
loggingmodule with appropriate log levels and context. Also, the current implementation silently continues after failures, which may hide persistent connectivity issues.Suggested improvement
+import logging + +logger = logging.getLogger(__name__) + class EventEmitter: ... except httpx.HTTPError as e: - print(f"[sandbox] Failed to emit event: {e}") + logger.warning( + "[sandbox/events] Failed to emit event", + extra={"event_type": event_type, "session_id": self.config.session_id, "error": str(e)}, + )
11-18: Consider implementing context manager protocol for resource cleanup.The
EventEmittercreates anhttpx.Clientthat must be explicitly closed viaclose(). Implementing__enter__and__exit__would allow usage withwithstatements, ensuring proper cleanup even if exceptions occur.Suggested addition
def __enter__(self) -> "EventEmitter": return self def __exit__(self, exc_type, exc_val, exc_tb) -> None: self.close()packages/control-plane/src/sandbox/client.ts (2)
71-87: Prefer params objects for multi-arg functions.Both the constructor and
createModalClienttake positional pairs; use a single params object to avoid order confusion and improve call-site clarity.As per coding guidelines, functions with 2+ parameters should accept a single params object with named properties instead of positional arguments.♻️ Proposed refactor
- constructor(secret: string, workspace: string) { + constructor({ secret, workspace }: { secret: string; workspace: string }) { @@ -export function createModalClient(secret: string, workspace: string): ModalClient { - return new ModalClient(secret, workspace); +export function createModalClient(params: { secret: string; workspace: string }): ModalClient { + return new ModalClient(params); }Also applies to: 343-344
114-115: Align console logs with the[domain/operation]prefix.Current logs use
[ModalClient]; please switch to the required[domain/operation] messagestyle.As per coding guidelines, use prefixed console logging with pattern `[domain/operation] message` for all logging.📝 Example
- console.log("[ModalClient] Creating sandbox:", request.sessionId); + console.log("[sandbox/create] Creating sandbox:", request.sessionId);Also applies to: 169-170, 206-207, 225-226, 257-258
packages/control-plane/src/session/schema.ts (1)
11-96: Consider enabling foreign key enforcement for SQLite.SQLite does not enforce foreign key constraints by default. To ensure referential integrity, add
PRAGMA foreign_keys = ON;before the schema creation statements.🔧 Proposed fix
export function initSchema(sql: SqlStorage): void { sql.exec(` + -- Enable foreign key enforcement + PRAGMA foreign_keys = ON; + -- Session metadata CREATE TABLE IF NOT EXISTS session (packages/control-plane/wrangler.toml (1)
19-21: Add environment-specific KV namespace overrides.The KV namespace
idis hardcoded. For staging and production deployments, use Wrangler environment overrides by adding[env.staging]and[env.production]sections with environment-specific namespace IDs, or manage them via the Cloudflare dashboard.packages/trpc/src/router/cloud-workspace/schema.ts (1)
31-35: Extract pagination limits into named constants.
Avoid magic numbers for limit/offset defaults and maxima.♻️ Suggested refactor
+const DEFAULT_LIST_LIMIT = 50; +const MAX_LIST_LIMIT = 100; +const DEFAULT_LIST_OFFSET = 0; + export const listCloudWorkspacesSchema = z.object({ status: z.enum(cloudSessionStatusValues).optional(), repositoryId: z.string().uuid().optional(), - limit: z.number().int().positive().max(100).default(50), - offset: z.number().int().min(0).default(0), + limit: z.number().int().positive().max(MAX_LIST_LIMIT).default(DEFAULT_LIST_LIMIT), + offset: z.number().int().min(DEFAULT_LIST_OFFSET).default(DEFAULT_LIST_OFFSET), });apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/CloudWorkspacesSection/CloudWorkspacesSection.tsx (1)
60-64: Extract staleTime and animation durations to constants.
Avoid magic numbers and keep the timings centralized.♻️ Suggested refactor
const CLOUD_SECTION_ID = "cloud-workspaces"; const STROKE_WIDTH = 1.5; +const CLOUD_WORKSPACES_STALE_TIME_MS = 30_000; +const CLOUD_WORKSPACES_ANIMATION_DURATION_S = 0.15; ... - staleTime: 30_000, // 30 seconds + staleTime: CLOUD_WORKSPACES_STALE_TIME_MS, // 30 seconds ... - transition={{ duration: 0.15, ease: "easeOut" }} + transition={{ duration: CLOUD_WORKSPACES_ANIMATION_DURATION_S, ease: "easeOut" }} ... - transition={{ duration: 0.15, ease: "easeOut" }} + transition={{ duration: CLOUD_WORKSPACES_ANIMATION_DURATION_S, ease: "easeOut" }}Also applies to: 124-127, 190-193
packages/control-plane/src/session/durable-object.ts (2)
281-306: Prefer params objects for multi-argument helpers.
handlePrompt(ws, content, authorId)uses positional args; per guideline, use a params object for clarity. (Same applies to other multi-arg helpers likesafeSend/broadcast.)
112-119: Clamp pagination inputs and extract thresholds.
limit/offsetare unbounded, and the online threshold uses a raw literal. Consider constants and caps to prevent heavy queries.♻️ Suggested refactor
-const WS_AUTH_TIMEOUT_MS = 30000; +const WS_AUTH_TIMEOUT_MS = 30_000; +const CLIENT_ONLINE_GRACE_MS = 60_000; +const DEFAULT_PAGE_LIMIT = 100; +const MAX_PAGE_LIMIT = 100; +const DEFAULT_PAGE_OFFSET = 0; ... - isOnline: Date.now() - p.last_seen_at < 60000, + isOnline: Date.now() - p.last_seen_at < CLIENT_ONLINE_GRACE_MS, ... - const limit = parseInt(url.searchParams.get("limit") || "100", 10); - const offset = parseInt(url.searchParams.get("offset") || "0", 10); + const limitParam = Number.parseInt( + url.searchParams.get("limit") ?? String(DEFAULT_PAGE_LIMIT), + 10, + ); + const offsetParam = Number.parseInt( + url.searchParams.get("offset") ?? String(DEFAULT_PAGE_OFFSET), + 10, + ); + const limit = Number.isFinite(limitParam) + ? Math.min(limitParam, MAX_PAGE_LIMIT) + : DEFAULT_PAGE_LIMIT; + const offset = Number.isFinite(offsetParam) + ? Math.max(offsetParam, DEFAULT_PAGE_OFFSET) + : DEFAULT_PAGE_OFFSET;Also applies to: 496-509, 528-541
packages/trpc/src/router/cloud-workspace/cloud-workspace.ts (2)
296-309: Reuse shared sandbox status enum to prevent drift.
Inline enums can diverge from the DB schema; usecloudSandboxStatusValues.♻️ Suggested refactor
-import { cloudWorkspaces } from "@superset/db/schema"; +import { cloudSandboxStatusValues, cloudWorkspaces } from "@superset/db/schema"; ... - sandboxStatus: z.enum([ - "pending", - "warming", - "syncing", - "ready", - "running", - "stopped", - "failed", - ]), + sandboxStatus: z.enum(cloudSandboxStatusValues),
33-34: Extract pagination defaults into constants.
Avoid magic numbers for limit/offset defaults and maxima.♻️ Suggested refactor
+const DEFAULT_PAGE_LIMIT = 50; +const MAX_PAGE_LIMIT = 100; +const DEFAULT_PAGE_OFFSET = 0; + ... - const { status, repositoryId, limit = 50, offset = 0 } = input ?? {}; + const { status, repositoryId, limit = DEFAULT_PAGE_LIMIT, offset = DEFAULT_PAGE_OFFSET } = + input ?? {}; ... .object({ - limit: z.number().int().positive().max(100).default(50), - offset: z.number().int().min(0).default(0), + limit: z.number().int().positive().max(MAX_PAGE_LIMIT).default(DEFAULT_PAGE_LIMIT), + offset: z.number().int().min(DEFAULT_PAGE_OFFSET).default(DEFAULT_PAGE_OFFSET), })Also applies to: 61-63, 72-72
packages/control-plane/src/index.ts (1)
90-91: Align log prefixes with[domain/operation]format.
Current logs only use[control-plane]. Use operation-specific prefixes to make filtering easier.🧭 Suggested fix
- console.error("[control-plane] Failed to initialize session:", error); + console.error("[control-plane/session-init] Failed to initialize session:", error); ... - console.error("[control-plane] Failed to spawn sandbox:", error); + console.error("[control-plane/sandbox-spawn] Failed to spawn sandbox:", error); ... - console.error("[control-plane] Failed to terminate sandbox:", error); + console.error("[control-plane/sandbox-terminate] Failed to terminate sandbox:", error);As per coding guidelines: Use prefixed console logging with pattern
[domain/operation] messagefor all logging.Also applies to: 370-371, 388-389
apps/desktop/src/main/lib/workspace-runtime/cloud.ts (4)
66-69: Extract reconnect/backoff/heartbeat constants.
Hard-coded retry/interval values are hard to tune and obscure intent. Promote them to named constants.♻️ Suggested fix
+const MAX_RECONNECT_ATTEMPTS = 5; +const RECONNECT_BASE_DELAY_MS = 1000; +const HEARTBEAT_INTERVAL_MS = 30_000; + class CloudWebSocketConnection extends EventEmitter { private ws: WebSocket | null = null; private reconnectAttempts = 0; - private maxReconnectAttempts = 5; - private reconnectDelay = 1000; + private maxReconnectAttempts = MAX_RECONNECT_ATTEMPTS; + private reconnectDelay = RECONNECT_BASE_DELAY_MS; ... this.pingInterval = setInterval(() => { this.send({ type: "ping" }); - }, 30000); + }, HEARTBEAT_INTERVAL_MS); }As per coding guidelines: Avoid magic numbers by extracting them to named constants at module top.
Also applies to: 166-169, 185-187
101-102: Log prefix should include operation.
Prefixes like[cloud-ws]don’t match the required[domain/operation]format. Use more specific prefixes for parse/error/reconnect paths.🧭 Suggested fix
- console.error("[cloud-ws] Failed to parse message:", e); + console.error("[cloud/ws-parse] Failed to parse message:", e); ... - console.error("[cloud-ws] WebSocket error:", error); + console.error("[cloud/ws-error] WebSocket error:", error); ... - console.error("[cloud-ws] Reconnect failed:", error); + console.error("[cloud/ws-reconnect] Reconnect failed:", error);As per coding guidelines: Use prefixed console logging with pattern
[domain/operation] messagefor all logging.Also applies to: 112-113, 189-191
158-160: Use a params object forsendPrompt.
This avoids positional arguments and follows the project guideline.♻️ Suggested fix
- sendPrompt(content: string, authorId: string): void { - this.send({ type: "prompt", content, authorId }); - } + sendPrompt(params: { content: string; authorId: string }): void { + this.send({ type: "prompt", ...params }); + } ... - this.connection.sendPrompt(params.data, "desktop-user"); + this.connection.sendPrompt({ content: params.data, authorId: "desktop-user" });As per coding guidelines: Functions with 2+ parameters should accept a single params object with named properties instead of positional arguments.
Also applies to: 286-287
409-410: Avoid bracket access to the privatesessionState.
["sessionState"]bypasses encapsulation and is brittle. Expose a getter onCloudTerminalRuntimeand use that instead.🧩 Suggested fix
class CloudTerminalRuntime extends EventEmitter implements TerminalRuntime { // ... getSession(paneId: string): { isAlive: boolean; cwd: string; lastActive: number } | null { // ... } + + getSessionState(): CloudSessionState | null { + return this.sessionState; + } } ... getState(): CloudSessionState | null { - return (this.terminalRuntime as CloudTerminalRuntime)["sessionState"]; + return this.terminalRuntime.getSessionState(); }
| this.ws.onclose = () => { | ||
| this.stopPingInterval(); | ||
| this.emit("disconnect"); | ||
| this.attemptReconnect(); | ||
| }; |
There was a problem hiding this comment.
disconnect() still triggers auto‑reconnect.
onclose always calls attemptReconnect, so kill/detach will reopen the socket. Add a flag to disable reconnect on intentional disconnects.
🛠️ Suggested fix
class CloudWebSocketConnection extends EventEmitter {
private ws: WebSocket | null = null;
+ private shouldReconnect = true;
...
async connect(): Promise<void> {
+ this.shouldReconnect = true;
const wsUrl = this.config.controlPlaneUrl
...
this.ws.onclose = () => {
this.stopPingInterval();
this.emit("disconnect");
- this.attemptReconnect();
+ if (this.shouldReconnect) {
+ this.attemptReconnect();
+ }
};
...
disconnect(): void {
+ this.shouldReconnect = false;
this.stopPingInterval();
if (this.ws) {
this.ws.close();
this.ws = null;
}
}Also applies to: 179-193, 195-201
🤖 Prompt for AI Agents
In `@apps/desktop/src/main/lib/workspace-runtime/cloud.ts` around lines 105 - 109,
The onclose handler always calls attemptReconnect, causing intentional
disconnects to auto-reconnect; add a boolean flag (e.g., this.manualDisconnect
or this.shouldReconnect) on the class to indicate an intentional/controlled
disconnect, set it true in the disconnect() method, and update the onclose
handlers (the this.ws.onclose that currently calls stopPingInterval(),
emit("disconnect"), attemptReconnect()) to only call attemptReconnect() when the
flag is false; ensure disconnect() clears/resets the flag appropriately and that
other places creating or reopening sockets reset the flag to false before
connecting.
| app.use( | ||
| "*", | ||
| cors({ | ||
| origin: (origin) => { | ||
| // Allow requests from our domains and localhost | ||
| if (!origin) return "*"; | ||
| if (origin.includes("superset.sh")) return origin; | ||
| if (origin.includes("localhost")) return origin; | ||
| return null; | ||
| }, | ||
| allowMethods: ["GET", "POST", "PUT", "DELETE", "OPTIONS"], | ||
| allowHeaders: ["Content-Type", "Authorization"], | ||
| credentials: true, | ||
| }), |
There was a problem hiding this comment.
CORS origin matching is too permissive (substring allows spoofed domains).
Using includes("superset.sh") / includes("localhost") will accept https://superset.sh.evil.com or https://localhost.evil.com. Parse the origin hostname and match exact hosts or a .superset.sh suffix; avoid "*" when credentials are enabled.
🛡️ Suggested fix
const app = new Hono<{ Bindings: Env }>();
+const ALLOWED_ORIGIN_HOSTS = new Set(["superset.sh", "localhost", "127.0.0.1"]);
// CORS middleware
app.use(
"*",
cors({
origin: (origin) => {
- // Allow requests from our domains and localhost
- if (!origin) return "*";
- if (origin.includes("superset.sh")) return origin;
- if (origin.includes("localhost")) return origin;
- return null;
+ if (!origin) return null;
+ try {
+ const { hostname } = new URL(origin);
+ if (hostname === "superset.sh" || hostname.endsWith(".superset.sh")) return origin;
+ if (ALLOWED_ORIGIN_HOSTS.has(hostname)) return origin;
+ } catch (error) {
+ console.error("[control-plane/cors] Invalid Origin header:", error);
+ }
+ return null;
},🤖 Prompt for AI Agents
In `@packages/control-plane/src/index.ts` around lines 22 - 35, The CORS origin
check in the app.use("*", cors(...)) origin callback is too permissive; update
the origin handler to parse the incoming origin with new URL(origin).hostname
(catch invalid origins) and then validate the hostname explicitly (e.g.,
hostname === "superset.sh" || hostname.endsWith(".superset.sh") for subdomains,
and hostname === "localhost" or hostname === "127.0.0.1" for local dev) instead
of using includes; also ensure you never return "*" when credentials: true —
return the original origin string when allowed or false/null when denied. Locate
the origin callback in the cors options in packages/control-plane/src/index.ts
to implement this change.
| app.post("/api/sessions", async (c) => { | ||
| const authHeader = c.req.header("Authorization"); | ||
| if (!authHeader?.startsWith("Bearer ")) { | ||
| return c.json({ error: "Unauthorized" }, 401); | ||
| } | ||
|
|
There was a problem hiding this comment.
Session endpoints are effectively unauthenticated.
Only the create route checks for a Bearer prefix (without validation), and the rest of the session endpoints—including WS, events/messages, and spawn/terminate—have no auth at all. Any caller can read or mutate sessions and spin up sandboxes. Please enforce token validation + session ownership on all /api/sessions/* routes (including the WS upgrade).
Also applies to: 113-214, 219-269, 319-391
🤖 Prompt for AI Agents
In `@packages/control-plane/src/index.ts` around lines 47 - 52, The session routes
currently only check for a "Bearer " prefix in app.post("/api/sessions", ...)
and leave all other /api/sessions/* handlers (create, list/get, WS upgrade,
events/messages, spawn, terminate) unauthenticated; replace the naive prefix
check with real token validation (extract the bearer token, call the central
verifyToken/verifyJWT function or auth service to validate and decode it) and
enforce session ownership on every handler by comparing the decoded token
subject/owner id to the session.ownerId before returning or mutating session
state; make sure the WS upgrade handler performs the same token validation and
ownership check prior to upgrading the socket, and return proper 401/403
responses for missing/invalid token or unauthorized access.
| await c.env.SESSION_TOKENS.put(`session:${sessionId}:sandbox_token`, sandboxTokenHash, { | ||
| expirationTtl: 86400 * 7, // 7 days | ||
| }); |
There was a problem hiding this comment.
Validate and clamp pagination params; extract defaults to constants.
limit/offset are forwarded as strings with no bounds; callers can request huge ranges and cause large reads. Parse to numbers, clamp to a max, and define constants (also reuse for the token TTL).
✅ Suggested fix
const app = new Hono<{ Bindings: Env }>();
+const SESSION_TOKEN_TTL_SECONDS = 60 * 60 * 24 * 7;
+const DEFAULT_PAGE_LIMIT = 100;
+const MAX_PAGE_LIMIT = 500;
+const DEFAULT_PAGE_OFFSET = 0;
+
+const parseIntParam = ({
+ value,
+ fallback,
+ max,
+}: {
+ value: string | null;
+ fallback: number;
+ max?: number;
+}): number => {
+ const parsed = Number.parseInt(value ?? "", 10);
+ if (!Number.isFinite(parsed) || parsed < 0) return fallback;
+ return typeof max === "number" ? Math.min(parsed, max) : parsed;
+};
+
...
await c.env.SESSION_TOKENS.put(`session:${sessionId}:sandbox_token`, sandboxTokenHash, {
- expirationTtl: 86400 * 7, // 7 days
+ expirationTtl: SESSION_TOKEN_TTL_SECONDS,
});
...
- const limit = c.req.query("limit") || "100";
- const offset = c.req.query("offset") || "0";
+ const limit = parseIntParam({
+ value: c.req.query("limit"),
+ fallback: DEFAULT_PAGE_LIMIT,
+ max: MAX_PAGE_LIMIT,
+ });
+ const offset = parseIntParam({ value: c.req.query("offset"), fallback: DEFAULT_PAGE_OFFSET });
...
- url.searchParams.set("limit", limit);
- url.searchParams.set("offset", offset);
+ url.searchParams.set("limit", String(limit));
+ url.searchParams.set("offset", String(offset));
...
- const limit = c.req.query("limit") || "100";
- const offset = c.req.query("offset") || "0";
+ const limit = parseIntParam({
+ value: c.req.query("limit"),
+ fallback: DEFAULT_PAGE_LIMIT,
+ max: MAX_PAGE_LIMIT,
+ });
+ const offset = parseIntParam({ value: c.req.query("offset"), fallback: DEFAULT_PAGE_OFFSET });
...
- url.searchParams.set("limit", limit);
- url.searchParams.set("offset", offset);
+ url.searchParams.set("limit", String(limit));
+ url.searchParams.set("offset", String(offset));As per coding guidelines: Avoid magic numbers by extracting them to named constants at module top.
Also applies to: 219-233, 246-259
🤖 Prompt for AI Agents
In `@packages/control-plane/src/index.ts` around lines 99 - 101, The code
currently forwards limit/offset as strings and uses magic TTL numbers; parse
limit and offset into integers (Number.parseInt or +) and clamp them within
named constants (e.g., DEFAULT_LIMIT, MAX_LIMIT, DEFAULT_OFFSET, MAX_OFFSET),
use DEFAULT_* when parsing fails or values are negative, and replace literal TTL
(86400 * 7) with a SESSION_TOKEN_TTL_SECONDS constant (e.g., DAYS_TO_SECONDS * 7
or SESSION_TOKEN_TTL_SECONDS). Update all usages that accept pagination
(variables named limit/offset) and the SESSION_TOKENS.put call (and the similar
occurrences around the other noted ranges) to use the parsed/clamped values and
the new TTL constant. Ensure constants are declared at the module top so they
replace the magic numbers across the file.
| // Get sandbox token from KV | ||
| const sandboxTokenHash = await c.env.SESSION_TOKENS.get(`session:${sessionId}:sandbox_token`); | ||
| if (!sandboxTokenHash) { | ||
| return c.json({ error: "Session token not found" }, 404); | ||
| } | ||
|
|
||
| // Generate a new sandbox token for this spawn | ||
| const sandboxToken = generateSandboxToken(); | ||
|
|
||
| // Create Modal client and spawn sandbox | ||
| const modalClient = createModalClient(c.env.MODAL_API_SECRET, c.env.MODAL_WORKSPACE); | ||
|
|
||
| try { | ||
| const result = await modalClient.createSandbox({ | ||
| sessionId, | ||
| repoOwner: state.repoOwner, | ||
| repoName: state.repoName, | ||
| branch: state.branch, | ||
| baseBranch: state.baseBranch, | ||
| controlPlaneUrl: c.env.CONTROL_PLANE_URL, | ||
| sandboxAuthToken: sandboxToken, | ||
| model: state.model, |
There was a problem hiding this comment.
Sandbox token rotation isn’t persisted or validated.
You fetch the existing hash but never compare it, then generate a new token without storing its hash. If sandbox auth depends on this token, verification will fail; if not, the KV check is dead code. Either reuse the stored token or persist the new hash when spawning.
🔐 Suggested fix
const sandboxTokenHash = await c.env.SESSION_TOKENS.get(`session:${sessionId}:sandbox_token`);
if (!sandboxTokenHash) {
return c.json({ error: "Session token not found" }, 404);
}
// Generate a new sandbox token for this spawn
const sandboxToken = generateSandboxToken();
+ const newSandboxTokenHash = await hashToken(sandboxToken);
+ await c.env.SESSION_TOKENS.put(`session:${sessionId}:sandbox_token`, newSandboxTokenHash, {
+ expirationTtl: 86400 * 7,
+ });🤖 Prompt for AI Agents
In `@packages/control-plane/src/index.ts` around lines 340 - 361, You fetch
sandboxTokenHash from SESSION_TOKENS but never validate or persist the newly
generated sandboxToken; either validate against the stored hash or store the new
token hash before calling modalClient.createSandbox. Update the flow in the
block around sandboxTokenHash, generateSandboxToken, and
modalClient.createSandbox: if you intend to reuse the stored token, verify
generateSandboxToken() (or the provided token) against sandboxTokenHash; if you
intend to rotate, hash the new sandboxToken and persist it back to
SESSION_TOKENS (e.g., using the same key `session:${sessionId}:sandbox_token`)
before invoking modalClient.createSandbox so the control plane and modal can
validate the token.
| def verify_internal_token(token: str, secret: str, max_age_ms: int = 5 * 60 * 1000) -> bool: | ||
| """Verify an HMAC-signed token.""" | ||
| try: | ||
| parts = token.split(".") | ||
| if len(parts) != 2: | ||
| return False | ||
|
|
||
| timestamp_str, signature = parts | ||
| timestamp = int(timestamp_str) | ||
|
|
||
| # Check expiration | ||
| now = int(time.time() * 1000) | ||
| if now - timestamp > max_age_ms: | ||
| return False | ||
|
|
||
| # Verify signature | ||
| expected = hmac.new( | ||
| secret.encode(), | ||
| timestamp_str.encode(), | ||
| hashlib.sha256, | ||
| ).hexdigest() | ||
|
|
||
| return hmac.compare_digest(signature, expected) | ||
| except Exception: | ||
| return False |
There was a problem hiding this comment.
Fail closed if MODAL_API_SECRET is missing.
With an empty secret, token verification becomes meaningless. Guard against empty secrets so requests are rejected and misconfigurations are visible.
🔒 Proposed fix
def verify_internal_token(token: str, secret: str, max_age_ms: int = 5 * 60 * 1000) -> bool:
"""Verify an HMAC-signed token."""
+ if not secret:
+ return False
try:
parts = token.split(".")🤖 Prompt for AI Agents
In `@packages/sandbox/sandbox/app.py` around lines 52 - 76, The
verify_internal_token function currently accepts an empty secret which makes
verification meaningless; update verify_internal_token to immediately fail
(return False) when the provided secret is empty or falsy before attempting to
parse or verify the token (i.e., check secret at the top of the function), so
that callers using MODAL_API_SECRET that is missing will be rejected and
misconfigurations are visible; keep the rest of the logic (timestamp parsing,
expiration check, HMAC compare via hmac.compare_digest) unchanged.
| @app.function(secrets=[Secret.from_name("superset-modal-secrets")]) | ||
| @modal.web_endpoint(method="POST") | ||
| def api_warm_sandbox(request: dict) -> dict: | ||
| """Pre-warm a sandbox for faster startup.""" | ||
| # For now, just return success - warming is handled by Modal's container lifecycle | ||
| return {"success": True, "data": {"sandbox_id": str(uuid.uuid4()), "status": "warming"}} | ||
|
|
There was a problem hiding this comment.
Missing auth checks on internal endpoints.
These endpoints skip the internal token verification used by api_create_sandbox, allowing unauthenticated calls to snapshot/restore/terminate. They should enforce the same Bearer token validation.
🔐 Suggested pattern (apply to each endpoint)
+def _require_internal_auth(request: dict) -> dict | None:
+ auth_header = request.get("headers", {}).get("authorization", "")
+ if not auth_header.startswith("Bearer "):
+ return {"success": False, "error": "Unauthorized"}
+ token = auth_header[7:]
+ modal_secret = os.environ.get("MODAL_API_SECRET", "")
+ if not verify_internal_token(token, modal_secret):
+ return {"success": False, "error": "Invalid token"}
+ return None
@@
def api_warm_sandbox(request: dict) -> dict:
"""Pre-warm a sandbox for faster startup."""
+ auth_error = _require_internal_auth(request)
+ if auth_error:
+ return auth_error
# For now, just return success - warming is handled by Modal's container lifecycle
return {"success": True, "data": {"sandbox_id": str(uuid.uuid4()), "status": "warming"}}Also applies to: 279-285, 287-297, 299-327, 329-334
🤖 Prompt for AI Agents
In `@packages/sandbox/sandbox/app.py` around lines 264 - 270, The internal
endpoints (e.g., api_warm_sandbox and the snapshot/restore/terminate endpoints
referenced in the comment) currently skip authentication; replicate the same
Bearer token verification used in api_create_sandbox: extract the Authorization
header, validate the Bearer token against the internal token check used by
api_create_sandbox, and return a 401/unauthorized response on failure before
proceeding; apply this change to api_warm_sandbox and each internal endpoint in
the ranges noted so they all enforce identical token validation logic.
| def run_prompt( | ||
| self, | ||
| prompt: str, | ||
| message_id: str | None = None, | ||
| on_event: Callable[[dict[str, Any]], None] | None = None, | ||
| ) -> dict[str, Any]: | ||
| """Execute a prompt through Claude Code.""" | ||
| self._stop_requested = False | ||
|
|
||
| # Build command | ||
| cmd = [ | ||
| CLAUDE_CODE_PATH, | ||
| "--print", # Output to stdout in JSON format | ||
| "--output-format", "stream-json", | ||
| "--model", self.config.model, | ||
| prompt, | ||
| ] | ||
|
|
||
| # Environment variables | ||
| env = { | ||
| "ANTHROPIC_API_KEY": "", # Will be set from Modal secrets | ||
| "CLAUDE_CODE_NO_INTERACTIVE": "1", | ||
| "HOME": "/root", | ||
| "PATH": "/usr/local/bin:/usr/bin:/bin", | ||
| } | ||
|
|
||
| try: | ||
| self.process = subprocess.Popen( | ||
| cmd, | ||
| cwd=self.workspace_path, | ||
| stdout=subprocess.PIPE, | ||
| stderr=subprocess.PIPE, | ||
| text=True, | ||
| env=env, | ||
| ) | ||
|
|
||
| # Start output processing in a thread | ||
| output_lines = [] | ||
| error_lines = [] | ||
|
|
||
| def read_stdout(): | ||
| for line in self.process.stdout: | ||
| if self._stop_requested: | ||
| break | ||
| line = line.strip() | ||
| if line: | ||
| output_lines.append(line) | ||
| self._process_output_line(line, message_id, on_event) | ||
|
|
||
| def read_stderr(): | ||
| for line in self.process.stderr: | ||
| if self._stop_requested: | ||
| break | ||
| line = line.strip() | ||
| if line: | ||
| error_lines.append(line) | ||
|
|
||
| stdout_thread = threading.Thread(target=read_stdout) | ||
| stderr_thread = threading.Thread(target=read_stderr) | ||
|
|
||
| stdout_thread.start() | ||
| stderr_thread.start() | ||
|
|
||
| # Wait for completion with timeout | ||
| try: | ||
| self.process.wait(timeout=CLAUDE_EXECUTION_TIMEOUT_SECONDS) | ||
| except subprocess.TimeoutExpired: | ||
| self.process.kill() | ||
| self.emitter.emit_error("Execution timed out", message_id) | ||
| return {"success": False, "error": "Execution timed out"} | ||
|
|
||
| stdout_thread.join(timeout=5) | ||
| stderr_thread.join(timeout=5) | ||
|
|
||
| success = self.process.returncode == 0 | ||
| self.emitter.emit_execution_complete( | ||
| success=success, | ||
| summary="Prompt completed" if success else "Prompt failed", | ||
| message_id=message_id, | ||
| ) | ||
|
|
||
| return { | ||
| "success": success, | ||
| "exit_code": self.process.returncode, | ||
| "output": "\n".join(output_lines), | ||
| "error": "\n".join(error_lines) if error_lines else None, | ||
| } | ||
|
|
||
| except Exception as e: | ||
| self.emitter.emit_error(str(e), message_id) | ||
| return {"success": False, "error": str(e)} | ||
|
|
||
| finally: | ||
| self.process = None | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "runner.py" -path "*/sandbox/*" | head -20Repository: superset-sh/superset
Length of output: 100
🏁 Script executed:
git ls-files | grep -i "runner.py" | grep -i sandboxRepository: superset-sh/superset
Length of output: 98
🏁 Script executed:
find . -type f -name "runner.py"Repository: superset-sh/superset
Length of output: 100
🏁 Script executed:
wc -l packages/sandbox/sandbox/runner.pyRepository: superset-sh/superset
Length of output: 102
🏁 Script executed:
sed -n '1,40p' packages/sandbox/sandbox/runner.pyRepository: superset-sh/superset
Length of output: 1099
🏁 Script executed:
sed -n '29,123p' packages/sandbox/sandbox/runner.pyRepository: superset-sh/superset
Length of output: 3212
🏁 Script executed:
grep -r "Modal" packages/sandbox/ --include="*.py" | head -20Repository: superset-sh/superset
Length of output: 1064
🏁 Script executed:
grep -r "ANTHROPIC_API_KEY" packages/sandbox/ --include="*.py"Repository: superset-sh/superset
Length of output: 169
🏁 Script executed:
grep -r "os.environ" packages/sandbox/ --include="*.py"Repository: superset-sh/superset
Length of output: 643
🏁 Script executed:
find packages/sandbox -name "*.py" -type f | xargs grep -l "subprocess\|Popen"Repository: superset-sh/superset
Length of output: 158
🏁 Script executed:
sed -n '1,50p' packages/sandbox/sandbox/app.pyRepository: superset-sh/superset
Length of output: 1296
🏁 Script executed:
grep -B5 -A10 "os.environ.copy()" packages/sandbox/sandbox/app.pyRepository: superset-sh/superset
Length of output: 46
🏁 Script executed:
grep -B10 -A10 "ANTHROPIC_API_KEY" packages/sandbox/sandbox/app.pyRepository: superset-sh/superset
Length of output: 46
🏁 Script executed:
cat -n packages/sandbox/sandbox/config.py | head -60Repository: superset-sh/superset
Length of output: 1540
🏁 Script executed:
sed -n '40,120p' packages/sandbox/sandbox/app.pyRepository: superset-sh/superset
Length of output: 2379
🏁 Script executed:
grep -B5 -A15 "Secret\|secret" packages/sandbox/sandbox/app.py | head -80Repository: superset-sh/superset
Length of output: 2073
🏁 Script executed:
cat packages/sandbox/sandbox/app.py | tail -100Repository: superset-sh/superset
Length of output: 3695
🏁 Script executed:
sed -n '120,178p' packages/sandbox/sandbox/app.pyRepository: superset-sh/superset
Length of output: 2025
🏁 Script executed:
grep -A30 "def run_prompt" packages/sandbox/sandbox/runner.py | grep -A10 "env\|environ"Repository: superset-sh/superset
Length of output: 409
🏁 Script executed:
grep -B5 -A20 "subprocess.Popen" packages/sandbox/sandbox/runner.pyRepository: superset-sh/superset
Length of output: 1642
Don't overwrite ANTHROPIC_API_KEY with an empty value.
Modal injects the ANTHROPIC_API_KEY into the parent process's os.environ (see Secret.from_name("superset-modal-secrets") in app.py). The custom env dict here discards those injected secrets and forces the API key to empty, causing the Claude Code subprocess to fail authentication. Use os.environ.copy() and only override what's needed; optionally fail fast when the key is missing.
Proposed fix
-import json
-import subprocess
+import json
+import os
+import subprocess
@@
- env = {
- "ANTHROPIC_API_KEY": "", # Will be set from Modal secrets
- "CLAUDE_CODE_NO_INTERACTIVE": "1",
- "HOME": "/root",
- "PATH": "/usr/local/bin:/usr/bin:/bin",
- }
+ env = os.environ.copy()
+ env.setdefault("CLAUDE_CODE_NO_INTERACTIVE", "1")
+ env.setdefault("HOME", "/root")
+ env.setdefault("PATH", "/usr/local/bin:/usr/bin:/bin")
+ if not env.get("ANTHROPIC_API_KEY"):
+ self.emitter.emit_error("Missing ANTHROPIC_API_KEY", message_id)
+ return {"success": False, "error": "Missing ANTHROPIC_API_KEY"}🤖 Prompt for AI Agents
In `@packages/sandbox/sandbox/runner.py` around lines 29 - 123, The run_prompt
method currently builds a fresh env dict that sets "ANTHROPIC_API_KEY" to ""
which discards Modal-injected secrets; change it to start from os.environ.copy()
and only set/override the specific keys you need (e.g.,
"CLAUDE_CODE_NO_INTERACTIVE", "HOME", "PATH") so you do not clobber
ANTHROPIC_API_KEY; in run_prompt, after copying os.environ, optionally check
that os_env.get("ANTHROPIC_API_KEY") exists and emit an error (via
self.emitter.emit_error) or raise early if missing before launching subprocess
(self.process = subprocess.Popen(...)).
| const [url, setUrlState] = useState(defaultUrl); | ||
| const [state, setState] = useState<LivePreviewState>( | ||
| defaultUrl ? "loading" : "empty", | ||
| ); | ||
| const [isExpanded, setIsExpandedState] = useState(false); | ||
| const [isCollapsed, setIsCollapsed] = useState(defaultCollapsed); | ||
| const [refreshKey, setRefreshKey] = useState(0); | ||
|
|
||
| const setUrl = useCallback( | ||
| (newUrl: string) => { | ||
| setUrlState(newUrl); | ||
| setState(newUrl ? "loading" : "empty"); | ||
| onUrlChange?.(newUrl); | ||
| }, | ||
| [onUrlChange], | ||
| ); | ||
|
|
||
| const setIsExpanded = useCallback( | ||
| (expanded: boolean) => { | ||
| setIsExpandedState(expanded); | ||
| onExpandChange?.(expanded); | ||
| }, | ||
| [onExpandChange], | ||
| ); | ||
|
|
||
| const refresh = useCallback(() => { | ||
| setRefreshKey((k) => k + 1); | ||
| if (url) setState("loading"); | ||
| }, [url]); | ||
|
|
||
| // Sync url when defaultUrl changes | ||
| useEffect(() => { | ||
| if (defaultUrl !== url) { | ||
| setUrlState(defaultUrl); | ||
| setState(defaultUrl ? "loading" : "empty"); | ||
| } | ||
| }, [defaultUrl]); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "live-preview.tsx" -type f | head -20Repository: superset-sh/superset
Length of output: 121
🏁 Script executed:
cat -n packages/ui/src/components/ai-elements/live-preview.tsx | sed -n '80,130p'Repository: superset-sh/superset
Length of output: 1677
🏁 Script executed:
# Check if there's a biome or eslint config
find . -name "biome.json" -o -name ".eslintrc*" -o -name "eslint.config.*" | head -10Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Check the broader file context for other issues
cat -n packages/ui/src/components/ai-elements/live-preview.tsx | head -150Repository: superset-sh/superset
Length of output: 4763
🏁 Script executed:
# Check for TypeScript types and any usage
rg "any" packages/ui/src/components/ai-elements/live-preview.tsxRepository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Check file structure - should be ComponentName/ComponentName.tsx
ls -la packages/ui/src/components/ai-elements/Repository: superset-sh/superset
Length of output: 2146
🏁 Script executed:
# Check if there's a barrel export for LivePreview
ls -la packages/ui/src/components/ai-elements/ | grep -E "index|__"Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Verify the complete end of the file to understand full structure
tail -50 packages/ui/src/components/ai-elements/live-preview.tsxRepository: superset-sh/superset
Length of output: 1244
Use a ref to track the latest URL and resolve the Biome dependency lint without breaking functionality.
The useEffect at lines 118-123 reads url without including it as a dependency, which Biome correctly flags. Adding url to the dependency array would reset user-edited URLs when the local state changes, and could leave the state stuck in loading. Use a ref to track the latest URL separately so the effect remains scoped to defaultUrl changes.
Suggested fix
const [url, setUrlState] = useState(defaultUrl);
+const urlRef = useRef(url);
const [state, setState] = useState<LivePreviewState>(
defaultUrl ? "loading" : "empty",
);
+
+useEffect(() => {
+ urlRef.current = url;
+}, [url]);
// Sync url when defaultUrl changes
useEffect(() => {
- if (defaultUrl !== url) {
+ if (defaultUrl !== urlRef.current) {
setUrlState(defaultUrl);
setState(defaultUrl ? "loading" : "empty");
}
}, [defaultUrl]);📝 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 [url, setUrlState] = useState(defaultUrl); | |
| const [state, setState] = useState<LivePreviewState>( | |
| defaultUrl ? "loading" : "empty", | |
| ); | |
| const [isExpanded, setIsExpandedState] = useState(false); | |
| const [isCollapsed, setIsCollapsed] = useState(defaultCollapsed); | |
| const [refreshKey, setRefreshKey] = useState(0); | |
| const setUrl = useCallback( | |
| (newUrl: string) => { | |
| setUrlState(newUrl); | |
| setState(newUrl ? "loading" : "empty"); | |
| onUrlChange?.(newUrl); | |
| }, | |
| [onUrlChange], | |
| ); | |
| const setIsExpanded = useCallback( | |
| (expanded: boolean) => { | |
| setIsExpandedState(expanded); | |
| onExpandChange?.(expanded); | |
| }, | |
| [onExpandChange], | |
| ); | |
| const refresh = useCallback(() => { | |
| setRefreshKey((k) => k + 1); | |
| if (url) setState("loading"); | |
| }, [url]); | |
| // Sync url when defaultUrl changes | |
| useEffect(() => { | |
| if (defaultUrl !== url) { | |
| setUrlState(defaultUrl); | |
| setState(defaultUrl ? "loading" : "empty"); | |
| } | |
| }, [defaultUrl]); | |
| const [url, setUrlState] = useState(defaultUrl); | |
| const urlRef = useRef(url); | |
| const [state, setState] = useState<LivePreviewState>( | |
| defaultUrl ? "loading" : "empty", | |
| ); | |
| useEffect(() => { | |
| urlRef.current = url; | |
| }, [url]); | |
| const [isExpanded, setIsExpandedState] = useState(false); | |
| const [isCollapsed, setIsCollapsed] = useState(defaultCollapsed); | |
| const [refreshKey, setRefreshKey] = useState(0); | |
| const setUrl = useCallback( | |
| (newUrl: string) => { | |
| setUrlState(newUrl); | |
| setState(newUrl ? "loading" : "empty"); | |
| onUrlChange?.(newUrl); | |
| }, | |
| [onUrlChange], | |
| ); | |
| const setIsExpanded = useCallback( | |
| (expanded: boolean) => { | |
| setIsExpandedState(expanded); | |
| onExpandChange?.(expanded); | |
| }, | |
| [onExpandChange], | |
| ); | |
| const refresh = useCallback(() => { | |
| setRefreshKey((k) => k + 1); | |
| if (url) setState("loading"); | |
| }, [url]); | |
| // Sync url when defaultUrl changes | |
| useEffect(() => { | |
| if (defaultUrl !== urlRef.current) { | |
| setUrlState(defaultUrl); | |
| setState(defaultUrl ? "loading" : "empty"); | |
| } | |
| }, [defaultUrl]); |
🧰 Tools
🪛 Biome (2.3.13)
[error] 118-118: This hook does not specify its dependency on url.
This dependency is being used here, but is not specified in the hook dependency list.
React relies on hook dependencies to determine when to re-compute Effects.
Failing to specify dependencies can result in Effects not updating correctly when state changes.
These "stale closures" are a common source of surprising bugs.
Unsafe fix: Add the missing dependency to the list.
(lint/correctness/useExhaustiveDependencies)
🤖 Prompt for AI Agents
In `@packages/ui/src/components/ai-elements/live-preview.tsx` around lines 87 -
123, The useEffect that syncs defaultUrl reads the state variable url without
including it in deps which Biome flags; fix by adding a ref (e.g., latestUrlRef)
to track the current url and update that ref inside setUrl (and wherever url is
updated, e.g., refresh and initial state setup) so the useEffect only depends on
defaultUrl and reads latestUrlRef.current instead of url, and when defaultUrl
!== latestUrlRef.current call setUrlState(defaultUrl) and setState(defaultUrl ?
"loading" : "empty") so behavior stays the same without adding url to the effect
deps.
| <svg | ||
| className="size-8 text-muted-foreground" | ||
| fill="none" | ||
| stroke="currentColor" | ||
| strokeWidth={1.5} | ||
| viewBox="0 0 24 24" | ||
| > | ||
| <path | ||
| d="M2.25 15.75l5.159-5.159a2.25 2.25 0 013.182 0l5.159 5.159m-1.5-1.5l1.409-1.409a2.25 2.25 0 013.182 0l2.909 2.909m-18 3.75h16.5a1.5 1.5 0 001.5-1.5V6a1.5 1.5 0 00-1.5-1.5H3.75A1.5 1.5 0 002.25 6v12a1.5 1.5 0 001.5 1.5zm10.5-11.25h.008v.008h-.008V8.25zm.375 0a.375.375 0 11-.75 0 .375.375 0 01.75 0z" | ||
| strokeLinecap="round" | ||
| strokeLinejoin="round" | ||
| /> | ||
| </svg> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "live-preview.tsx" | head -20Repository: superset-sh/superset
Length of output: 121
🏁 Script executed:
cat -n ./packages/ui/src/components/ai-elements/live-preview.tsx | sed -n '530,615p'Repository: superset-sh/superset
Length of output: 3005
🏁 Script executed:
find . -type f \( -name "biome.json" -o -name "biome.jsonc" -o -name ".biomerc*" \) | head -10Repository: superset-sh/superset
Length of output: 77
🏁 Script executed:
cat ./biome.jsonc | head -100Repository: superset-sh/superset
Length of output: 545
🏁 Script executed:
rg "noSvgWithoutTitle" . --type json --type jsoncRepository: superset-sh/superset
Length of output: 93
🏁 Script executed:
rg "noSvgWithoutTitle" . --type json -A 5 -B 5Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
rg "svg" ./biome.jsonc -iRepository: superset-sh/superset
Length of output: 46
🏁 Script executed:
rg "noSvgWithoutTitle" . -A 3 -B 3 --max-count 20Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
cat ./biome.jsoncRepository: superset-sh/superset
Length of output: 545
Add SVG titles for a11y lint compliance.
The SVGs at lines 536–548 and 596–608 lack <title> elements, which Biome's recommended a11y rules require via noSvgWithoutTitle. Add descriptive <title> elements to both SVGs for accessibility:
Suggested fix
<svg
className="size-8 text-muted-foreground"
fill="none"
stroke="currentColor"
strokeWidth={1.5}
viewBox="0 0 24 24"
>
+ <title>Preview placeholder</title>
<path <svg
className="size-8 text-red-500"
fill="none"
stroke="currentColor"
strokeWidth={1.5}
viewBox="0 0 24 24"
>
+ <title>Preview error</title>
<path📝 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.
| <svg | |
| className="size-8 text-muted-foreground" | |
| fill="none" | |
| stroke="currentColor" | |
| strokeWidth={1.5} | |
| viewBox="0 0 24 24" | |
| > | |
| <path | |
| d="M2.25 15.75l5.159-5.159a2.25 2.25 0 013.182 0l5.159 5.159m-1.5-1.5l1.409-1.409a2.25 2.25 0 013.182 0l2.909 2.909m-18 3.75h16.5a1.5 1.5 0 001.5-1.5V6a1.5 1.5 0 00-1.5-1.5H3.75A1.5 1.5 0 002.25 6v12a1.5 1.5 0 001.5 1.5zm10.5-11.25h.008v.008h-.008V8.25zm.375 0a.375.375 0 11-.75 0 .375.375 0 01.75 0z" | |
| strokeLinecap="round" | |
| strokeLinejoin="round" | |
| /> | |
| </svg> | |
| <svg | |
| className="size-8 text-muted-foreground" | |
| fill="none" | |
| stroke="currentColor" | |
| strokeWidth={1.5} | |
| viewBox="0 0 24 24" | |
| > | |
| <title>Preview placeholder</title> | |
| <path | |
| d="M2.25 15.75l5.159-5.159a2.25 2.25 0 013.182 0l5.159 5.159m-1.5-1.5l1.409-1.409a2.25 2.25 0 013.182 0l2.909 2.909m-18 3.75h16.5a1.5 1.5 0 001.5-1.5V6a1.5 1.5 0 00-1.5-1.5H3.75A1.5 1.5 0 002.25 6v12a1.5 1.5 0 001.5 1.5zm10.5-11.25h.008v.008h-.008V8.25zm.375 0a.375.375 0 11-.75 0 .375.375 0 01.75 0z" | |
| strokeLinecap="round" | |
| strokeLinejoin="round" | |
| /> | |
| </svg> |
🧰 Tools
🪛 Biome (2.3.13)
[error] 536-542: Alternative text title element cannot be empty
For accessibility purposes, SVGs should have an alternative text, provided via title element. If the svg element has role="img", you should add the aria-label or aria-labelledby attribute.
(lint/a11y/noSvgWithoutTitle)
🤖 Prompt for AI Agents
In `@packages/ui/src/components/ai-elements/live-preview.tsx` around lines 536 -
548, The SVG elements in the live-preview component are missing accessible
titles (causing Biome a11y rule noSvgWithoutTitle failures); for each SVG (the
image/placeholder icons in
packages/ui/src/components/ai-elements/live-preview.tsx) add a <title> with a
short descriptive string, give the title a unique id (e.g.,
"livePreviewImageTitle" / "livePreviewPlaceholderTitle") and reference it from
the <svg> using aria-labelledby plus role="img" (or aria-label if you prefer) so
screen readers pick up the description; ensure both SVG occurrences (the one in
the shown diff and the other similar SVG later) receive unique title ids and
aria-labelledby attributes.
- Add CloudHomePage with session list sidebar and stats cards - Add NewSessionForm for creating new cloud workspace sessions - Add useCloudSession hook with stable WebSocket connection management - Add chat history persistence via sendHistory() in SessionDO - Add user message display in CloudWorkspaceContent - Fix hydration mismatch by using static sparkline heights - Fix WebSocket reconnection stability with isCleaningUp ref - Add sandboxInfo property and getSessionState() getter to SessionDO - Add temp_modal_vibe to .gitignore - Use full Superset logo instead of just icon in cloud pages
- Add sidebar with session list matching home page design - Include search functionality for sessions - Group sessions into active/inactive - Highlight current session in sidebar - Add toggle button to collapse/expand sidebar
- Add collapsible tool call UI with icons and smart summaries - Group consecutive same-type tool calls for cleaner display - Auto-spawn sandbox when session opens with stopped status - Show spawning status in header badge - Fix duplicate React key warnings in event list
Use NEXT_PUBLIC_NGROK_URL when available for the GitHub App installation callback URL. This ensures the OAuth flow completes properly when testing locally with ngrok.
The install link must go to localhost where the session cookie is valid. Only the callback URL (set by the API) should use ngrok since GitHub needs to redirect to a public URL.
Added step 6 explaining that the GitHub App callback URL must be updated to the ngrok domain for local development. Also added a reminder to revert it back to production URL when done testing.
- Add server-side guard to prevent duplicate sandbox spawns - Add prompt_ack/stop_ack messages for proper client feedback - Fix participant duplication by reusing existing records per user - Fix modal_object_id return field in sandbox spawn response - Update client to handle queued prompts and skip spawn when active
- Create session when user starts typing with repo selected - Connect to WebSocket and send typing event to trigger sandbox warm-up - Navigate directly to pre-warmed session on submit - Add visual "Warming" indicator during pre-warm - Add push, snapshot, shutdown message handlers to control plane
Remove pre-warming complexity that was causing errors. The page now creates sessions on submit like NewSessionForm does, without WebSocket pre-warming. We can add fanciness back later once the basic flow works.
The useEffect that resets spawn state had a comment saying "when session changes" but the dependency array was empty. This mismatch could cause React errors about dependency array size changes, and prevented proper reset when navigating between sessions.
- Replace browser confirm() with proper AlertDialog for archive action - Add archived sessions section to CloudHomePage sidebar - Add collapsible archived section with session count - Add unarchive functionality with restore button - Add delete functionality for archived sessions with confirmation dialog - Show archived timestamp and repo info in archived items
… display - Extract CloudSidebar into dedicated component for better code organization - Add TextShimmer component for loading states - Improve ToolCallItem with better formatting and display - Enhance sandbox runner with git operations and event handling - Update control plane durable object for better session management
- Extract CloudWorkspaceHeader component from CloudWorkspaceContent - Add CloudPromptInput component with repo selector and model picker - Simplify CloudSidebar and CloudHomePage components - Remove unused TextShimmer component - Apply Biome lint auto-fixes across UI package - Fix TypeScript error in CloudPromptInput (undefined handling) - Update control-plane durable object event handling
|
Closing: stale cloud workspaces infrastructure PR from Jan 31. Architecture has likely evolved significantly. |
|
Hey — just a heads up, this was closed as part of an automated stale PR cleanup. If you think this was done in error, feel free to reopen it! |
|
Hey — this was closed by an automated cleanup of PRs with major merge conflicts that are 3+ weeks old. If you think this was done incorrectly, please feel free to reopen it. Sorry for any inconvenience! |
2 participants
Summary
Introduces the cloud workspaces infrastructure for running Claude Code in remote Modal sandboxes with real-time event streaming.
Control Plane (Cloudflare Workers)
Modal Sandbox (Python)
Desktop App
CloudWorkspaceRuntimefor WebSocket connection to control planegetSessionState()getter for session state accessWeb App
/cloud) with session list sidebar, stats cards, and Superset branding/cloud/new) for creating cloud workspace sessionsDatabase
drizzle-kit generatetRPC
Deployed endpoints (dev)
https://superset-control-plane.avi-6ac.workers.devhttps://superset--superset-cloud-api-health.modal.runTest plan
/cloud/cloud/new