feat: Add local sandbox execution support with Docker and CLI tools #114
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdds a local sandbox subsystem: Convex schema and backend for tokens/connections/commands/results, a Local CLI and Docker image, frontend sandbox UI and persisted preference, Convex-backed ConvexSandbox and HybridSandboxManager, AnySandbox tooling, and made serviceKey required for many Convex endpoints. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant CLI as Local CLI
participant Convex as Convex Backend
participant Container as Docker / Host
CLI->>Convex: connect(token, metadata)
Convex-->>CLI: connectionId, connectionInfo
CLI->>Container: start container or use host
Container-->>CLI: containerId / osInfo
loop Heartbeat & Poll
CLI->>Convex: heartbeat(connectionId)
Convex-->>CLI: ack
CLI->>Convex: getPendingCommands(connectionId)
Convex-->>CLI: [commands]
end
CLI->>Convex: markCommandExecuting(commandId)
CLI->>Container: execute command
Container-->>CLI: stdout/stderr
CLI->>Convex: submitResult(commandId, stdout, stderr, exitCode)
Convex-->>CLI: ack
sequenceDiagram
autonumber
participant Agent as Agent flow
participant Hybrid as HybridSandboxManager
participant Convex as Convex Backend
participant Local as ConvexSandbox
participant Cloud as E2B Sandbox
Agent->>Hybrid: getSandbox()
alt preference == "e2b"
Hybrid->>Cloud: ensure/init E2B sandbox
Cloud-->>Hybrid: E2B sandbox
Hybrid-->>Agent: sandbox = E2B
else preference == connectionId
Hybrid->>Convex: listConnections(preferred)
Convex-->>Hybrid: connection info
Hybrid->>Local: instantiate ConvexSandbox(connectionInfo)
Local-->>Hybrid: ConvexSandbox ready
Hybrid-->>Agent: sandbox = Local
end
Agent->>Hybrid: commands.run(...)
alt local
Hybrid->>Local: route to ConvexSandbox
else e2b
Hybrid->>Cloud: route to E2B
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45–90 minutes Areas needing extra attention:
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
fkesheh
force-pushed
the
chat-to-local
branch
from
e14b7b2 to
4f784b2
Compare
- Add Convex schema tables for tokens, connections, commands, and results - Implement full Convex functions for local sandbox management: - Token management (get, regenerate, verify) - Connection management (connect, disconnect, heartbeat, list) - Command execution (enqueue, get pending, submit result) - Cleanup cron jobs for stale connections and old commands - Create ConvexSandbox wrapper with E2B-compatible interface - Create HybridSandboxManager for multi-connection support with E2B fallback - Add local client script supporting Docker and Dangerous modes - Add SandboxSelector UI component for chat-level sandbox selection - Add LocalSandboxTab settings component for token/connection management - Update utility files to support AnySandbox type (E2B + ConvexSandbox) - Add sandbox-types.ts with type guards for sandbox type checking
- Import api from convex/_generated/api - Use api.localSandbox.* instead of dynamic function objects - Remove @ts-expect-error comments (types now generated)
- Add dotenv to load environment variables from .env.local - Remove hardcoded default Convex URL (require env var) - Improve error handling with better error messages
- Add SandboxPreference type to chat types - Add sandboxPreference state to GlobalState (persisted to localStorage) - Show SandboxSelector next to mode selector when agent mode is selected - Allows switching between E2B cloud and local sandbox connections
- Add sandboxPreference to request body in chat handler - Pass sandboxPreference and serviceKey to createTools - Include sandboxPreference in all sendMessage/regenerate body payloads - Add sandboxPreferenceRef for queue auto-processing in chat.tsx
…files Local sandboxes may not have /home/user directory. Removed cwd parameter from all sandbox command executions to use default working directory.
…ruption Files downloaded from local sandbox were corrupted because binary data was piped through Convex commands as strings. Now files are uploaded directly from the sandbox to S3 using curl with presigned URLs, bypassing the Convex command pipeline entirely. - Add uploadToUrl method to ConvexSandbox for direct S3 uploads - Add downloadFromUrl method for downloading files into sandbox - Update sandbox-file-uploader to use direct S3 path for ConvexSandbox - Improve files.write to handle large binary files with chunking
Add Dockerfile and CI workflow to build and publish a pre-built sandbox image with 30+ pentesting tools (nmap, sqlmap, nuclei, hydra, etc.). - Add docker/Dockerfile with all pentesting tools pre-installed - Add GitHub Actions workflow to build and push to ghcr.io - Update local-sandbox-client to pull pre-built image by default - Add --build flag to build image locally instead of pulling
Replace Math.random() with Web Crypto API (crypto.getRandomValues) for generating local sandbox authentication tokens. This provides 256 bits of cryptographic randomness instead of a predictable PRNG.
fkesheh
force-pushed
the
chat-to-local
branch
from
440a2e6 to
44365ef
Compare
…b, update sandbox selector styling, and enhance command blocks
- Create standalone npm package for local sandbox client - Hardcode production Convex URL for published package - Use string-based Convex function paths instead of generated types - Remove dependency on dotenv and generated API types - Add README with usage documentation
- Update UI commands from pnpm local-sandbox to npx @hackerai/local - Add --convex-url flag for non-production environments - Remove old scripts/local-sandbox-client.ts - Remove local-sandbox npm script from package.json
Documentation content moved to knowledge graph: - SKILL: hackerai - SKILL: local-sandbox (subskill)
- Remove timeout on docker pull (allow slow connections) - Show live docker pull progress output - Add tip to pre-pull image beforehand
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
lib/ai/tools/utils/convex-sandbox.ts (1)
342-347: Add defensive error handling toisConnected()(same concern as prior review).If the Convex query fails (network, invalid
serviceKey, etc.),isConnected()will reject and surface as an unhandled rejection unless every caller wraps it. The prior review already suggested wrapping this in a try/catch; that still looks appropriate here.For example:
async isConnected(): Promise<boolean> { - const status = await this.convex.query(api.localSandbox.isConnected, { - serviceKey: this.serviceKey, - connectionId: this.connectionInfo.connectionId, - }); - return status.connected; + try { + const status = await this.convex.query(api.localSandbox.isConnected, { + serviceKey: this.serviceKey, + connectionId: this.connectionInfo.connectionId, + }); + return status.connected; + } catch (error) { + console.error( + `[ConvexSandbox] Failed to check connection (connectionId=${this.connectionInfo.connectionId}):`, + error, + ); + return false; + } }This keeps logs informative without leaking the
serviceKey.
🧹 Nitpick comments (1)
lib/ai/tools/utils/convex-sandbox.ts (1)
215-326: File ops look solid with escaping; minor polish: re-use helpers consistently and document size limits.
- Path handling via
ConvexSandbox.escapePath(Lines 210–213, 245, 267, 276, 281, 295, 319) and single‑quoted URL escaping (Lines 299–301, 316–320) is now in a good place; this substantially reduces shell‑injection risk for paths and URLs.- For maintainability, consider extracting a small
escapeSingleQuoted(value: string)helper and using it both for URLs and headers, mirroringescapePath, so future shell calls don’t accidentally bypass the escaping pattern.- The binary write chunking logic (Lines 233–252) is clearly tuned to Convex’s ~1MB limit. It might be worth adding a brief comment noting that large text writes (string
contentgoing through the heredoc path) are not chunked and can still hit Convex’s argument size limits if someone ever tries to write multi‑MB text files via this API.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
convex/localSandbox.ts(1 hunks)lib/ai/tools/utils/convex-sandbox.ts(1 hunks)packages/local/src/index.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
convex/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/convex_rules.mdc)
convex/**/*.{ts,tsx}: ALWAYS use the new function syntax for Convex functions with explicitargs,returns, andhandlerproperties
Usev.null()validator when returning null values from Convex functions
Usev.int64()instead of the deprecatedv.bigint()for representing signed 64-bit integers
Usev.record()for defining record types;v.map()andv.set()are not supported
Index fields must be queried in the same order they are defined; create separate indexes for different query orders
Use theId<'tableName'>TypeScript helper type from './_generated/dataModel' for strict typing of document IDs
Be strict with types around document IDs; useId<'tableName'>rather than genericstringtypes
Always useas constfor string literals in discriminated union types
Always define arrays asconst array: Array<T> = [...]with explicit type annotations
Always define records asconst record: Record<KeyType, ValueType> = {...}with explicit type annotations
UseinternalQuery,internalMutation, andinternalActionto register private functions; these are only callable by other Convex functions
Usequery,mutation, andactionto register public functions that are exposed to the public API
ALWAYS include argument and return validators for all Convex functions, includingreturns: v.null()if the function doesn't return anything
Usectx.runQueryto call a query from a query, mutation, or action
Usectx.runMutationto call a mutation from a mutation or action
Usectx.runActionto call an action from an action; only use this when crossing runtimes (e.g., V8 to Node)
When calling functions in the same file usingctx.runQuery,ctx.runMutation, orctx.runAction, specify a type annotation on the return value to work around TypeScript circularity limitations
Use theapiobject fromconvex/_generated/api.tsto call public functions registered withquery,mutation, oraction
Use theinternalobject fromconvex/_generated/api.ts...
Files:
convex/localSandbox.ts
🧠 Learnings (12)
📚 Learning: 2025-11-24T20:59:07.600Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-11-24T20:59:07.600Z
Learning: Applies to package.json : Add `types/node` to `package.json` when using any Node.js built-in modules in Convex functions
Applied to files:
packages/local/src/index.ts
📚 Learning: 2025-11-24T20:59:07.600Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-11-24T20:59:07.600Z
Learning: Applies to convex/**/*.{ts,tsx} : Always add `"use node";` to the top of files containing actions that use Node.js built-in modules
Applied to files:
packages/local/src/index.tslib/ai/tools/utils/convex-sandbox.ts
📚 Learning: 2025-11-24T20:59:07.600Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-11-24T20:59:07.600Z
Learning: Applies to convex/**/*.{ts,tsx} : Use the `internal` object from `convex/_generated/api.ts` to call internal functions registered with `internalQuery`, `internalMutation`, or `internalAction`
Applied to files:
convex/localSandbox.tslib/ai/tools/utils/convex-sandbox.ts
📚 Learning: 2025-11-24T20:59:07.600Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-11-24T20:59:07.600Z
Learning: Applies to convex/**/*.{ts,tsx} : Use `internalQuery`, `internalMutation`, and `internalAction` to register private functions; these are only callable by other Convex functions
Applied to files:
convex/localSandbox.ts
📚 Learning: 2025-11-24T20:59:07.600Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-11-24T20:59:07.600Z
Learning: Applies to convex/**/*.{ts,tsx} : Organize public query, mutation, or action functions thoughtfully within the `convex/` directory using file-based routing
Applied to files:
convex/localSandbox.tslib/ai/tools/utils/convex-sandbox.ts
📚 Learning: 2025-11-24T20:59:07.600Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-11-24T20:59:07.600Z
Learning: Applies to convex/**/*.{ts,tsx} : Do NOT use `filter` in queries; instead, define an index in the schema and use `withIndex`
Applied to files:
convex/localSandbox.ts
📚 Learning: 2025-11-24T20:59:07.600Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-11-24T20:59:07.600Z
Learning: Applies to convex/**/*.{ts,tsx} : Try to use as few calls from actions to queries and mutations as possible to minimize race condition risk
Applied to files:
convex/localSandbox.ts
📚 Learning: 2025-11-24T20:59:07.600Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-11-24T20:59:07.600Z
Learning: Applies to convex/**/*.{ts,tsx} : When using async iteration, don't use `.collect()` or `.take(n)` on query results; instead use `for await (const row of query)` syntax
Applied to files:
convex/localSandbox.ts
📚 Learning: 2025-11-24T20:59:07.600Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-11-24T20:59:07.600Z
Learning: Applies to convex/**/*.{ts,tsx} : ALWAYS use the new function syntax for Convex functions with explicit `args`, `returns`, and `handler` properties
Applied to files:
lib/ai/tools/utils/convex-sandbox.ts
📚 Learning: 2025-11-24T20:59:07.600Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-11-24T20:59:07.600Z
Learning: Applies to convex/**/*.{ts,tsx} : Pull out shared code into helper async functions and call them directly instead of using `ctx.runAction` for same-runtime calls
Applied to files:
lib/ai/tools/utils/convex-sandbox.ts
📚 Learning: 2025-11-24T20:59:07.600Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-11-24T20:59:07.600Z
Learning: Applies to convex/schema.ts : Always define schema in `convex/schema.ts` and import schema definition functions from `convex/server`
Applied to files:
lib/ai/tools/utils/convex-sandbox.ts
📚 Learning: 2025-11-24T20:59:07.600Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-11-24T20:59:07.600Z
Learning: Applies to convex/**/*.{ts,tsx} : Use the `api` object from `convex/_generated/api.ts` to call public functions registered with `query`, `mutation`, or `action`
Applied to files:
lib/ai/tools/utils/convex-sandbox.ts
🧬 Code graph analysis (2)
convex/localSandbox.ts (3)
convex/_generated/server.d.ts (1)
DatabaseReader(132-132)convex/tempStreams.ts (1)
validateServiceKey(4-8)convex/chats.ts (1)
validateServiceKey(6-10)
lib/ai/tools/utils/convex-sandbox.ts (1)
packages/local/src/index.ts (1)
cleanup(540-583)
🔇 Additional comments (11)
packages/local/src/index.ts (6)
56-136: LGTM!The
runShellCommandimplementation is well-designed:
- Proper timeout handling with SIGTERM escalation to SIGKILL
- Memory-safe output collection with intermediate truncation
- Correct use of the
killedflag to prevent race conditions- Standard exit code (124) for timeouts
627-630: LGTM on security documentation update!The updated security warning correctly explains the trade-offs of
--network host:
- Clarifies it provides process isolation but uses direct host network access
- Explains why this is necessary (pentesting tools need to scan network services)
- Distinguishes from DANGEROUS mode which has no isolation at all
This addresses the previous concern about misleading documentation.
414-433: Good defense-in-depth with shell escaping!The environment variable and working directory handling includes proper escaping:
- Escapes backslashes, quotes, dollar signs, and backticks in values
- Uses double quotes consistently
- While the developer noted that LLM-generated values don't require escaping for security (since the LLM could inject directly into
command), the escaping here is good defensive coding practice.
442-450: Excellent fix using single-quote escaping!The Docker exec implementation correctly uses single quotes with POSIX escaping (
'\\''), which prevents all shell interpretation of special characters ($, backticks, etc.). This addresses the previous concern about incomplete escaping.
483-520: Well-designed heartbeat with jitter!The heartbeat implementation includes good practices:
- ±5 second jitter to prevent thundering herd when multiple clients connect
- Uses
setTimeoutinstead ofsetIntervalto allow fresh jitter each cycle- Silently handles transient errors without killing the connection
- Checks
isShuttingDownbefore rescheduling
15-15: UseConvexHttpClientinstead ofConvexClientfor Node.js server-side operations.This file is a Node.js CLI tool (marked with
#!/usr/bin/env node). The import usesConvexClientfrom"convex/browser", which is designed for browser/real-time contexts. For Node.js server-side code, useConvexHttpClientinstead, as demonstrated in the codebase atlib/ai/tools/utils/sandbox-file-uploader.ts.Change line 15 from:
import { ConvexClient } from "convex/browser";To:
import { ConvexHttpClient } from "convex/browser";Then update line 228 accordingly:
this.convex = new ConvexHttpClient(config.convexUrl);⛔ Skipped due to learnings
Learnt from: CR Repo: hackerai-tech/hackerai PR: 0 File: .cursor/rules/convex_rules.mdc:0-0 Timestamp: 2025-11-24T20:59:07.600Z Learning: Applies to convex/**/*.{ts,tsx} : Convex storage stores items as `Blob` objects; convert all items to/from `Blob` when using Convex storageLearnt from: CR Repo: hackerai-tech/hackerai PR: 0 File: .cursor/rules/convex_rules.mdc:0-0 Timestamp: 2025-11-24T20:59:07.600Z Learning: Applies to convex/**/*.{ts,tsx} : Use the `api` object from `convex/_generated/api.ts` to call public functions registered with `query`, `mutation`, or `action`Learnt from: CR Repo: hackerai-tech/hackerai PR: 0 File: .cursor/rules/convex_rules.mdc:0-0 Timestamp: 2025-11-24T20:59:07.600Z Learning: Convex uses file-based routing; a public function defined in `convex/example.ts` named `f` has a function reference of `api.example.f`Learnt from: CR Repo: hackerai-tech/hackerai PR: 0 File: .cursor/rules/convex_rules.mdc:0-0 Timestamp: 2025-11-24T20:59:07.600Z Learning: Applies to convex/**/*.{ts,tsx} : Use the `internal` object from `convex/_generated/api.ts` to call internal functions registered with `internalQuery`, `internalMutation`, or `internalAction`Learnt from: CR Repo: hackerai-tech/hackerai PR: 0 File: .cursor/rules/convex_rules.mdc:0-0 Timestamp: 2025-11-24T20:59:07.600Z Learning: Applies to convex/**/*.{ts,tsx} : ALWAYS use the new function syntax for Convex functions with explicit `args`, `returns`, and `handler` propertiesLearnt from: CR Repo: hackerai-tech/hackerai PR: 0 File: .cursor/rules/convex_rules.mdc:0-0 Timestamp: 2025-11-24T20:59:07.600Z Learning: Applies to convex/schema.ts : Always define schema in `convex/schema.ts` and import schema definition functions from `convex/server`Learnt from: CR Repo: hackerai-tech/hackerai PR: 0 File: .cursor/rules/convex_rules.mdc:0-0 Timestamp: 2025-11-24T20:59:07.600Z Learning: Applies to convex/**/*.{ts,tsx} : Organize public query, mutation, or action functions thoughtfully within the `convex/` directory using file-based routingLearnt from: CR Repo: hackerai-tech/hackerai PR: 0 File: .cursor/rules/convex_rules.mdc:0-0 Timestamp: 2025-11-24T20:59:07.600Z Learning: Applies to convex/crons.ts : You can register Convex functions within `crons.ts` just like any other fileLearnt from: CR Repo: hackerai-tech/hackerai PR: 0 File: .cursor/rules/convex_rules.mdc:0-0 Timestamp: 2025-11-24T20:59:07.600Z Learning: Applies to convex/**/*.{ts,tsx} : Use `internalQuery`, `internalMutation`, and `internalAction` to register private functions; these are only callable by other Convex functionsconvex/localSandbox.ts (4)
9-142: LGTM on token management!The token management implementation follows Convex best practices:
- All functions have explicit
argsandreturnsvalidators- Uses
withIndexfor all queries (nofiltercalls)getTokenandregenerateTokenproperly validate user identityregenerateTokencorrectly disconnects all user connections for security- Token format (
hsb_prefix) is clear and identifiable
148-438: LGTM on connection management!The connection lifecycle is well-implemented:
- Proper token validation in all mutations
- Connection ownership checks prevent unauthorized access
listConnectionsForBackendandisConnectedcorrectly useserviceKeyfor backend-to-backend calls- All queries use
withIndexper coding guidelines- Post-query filtering for heartbeat timeout is acceptable (computed value can't be indexed)
444-729: LGTM on command execution!The command lifecycle implementation is secure and well-designed:
enqueueCommandvalidates connection exists and ownership (addresses past review)envparameter uses properv.record(v.string(), v.string())validator (addresses past review)markCommandExecutingprevents duplicate execution with status checksubmitResultvalidates command ownership before storing results (addresses past review)getPendingCommandsoptimization (skipping validation) is well-justified in comments and secure
735-840: LGTM on cleanup mutations!The cleanup implementation is well-designed:
- Both use
internalMutationfor cron job execution- All queries use
withIndexper coding guidelines (addresses past reviews)- Comment about timeout duration matches the actual constant (addresses past review)
- Batch processing with
.take(100)prevents overwhelming the database- Proper separation of concerns: stale connections vs. old commands/results
lib/ai/tools/utils/convex-sandbox.ts (1)
333-337: Based on my thorough verification, I have all the information needed to provide the final assessment.ConvexHttpClient does not have a close() method and does not require explicit resource cleanup.
The web search confirmed that:
- ConvexHttpClient uses HTTP (fetch) for each request with no persistent network connections
- Only WebSocket-based clients (ConvexClient, BaseConvexClient, ConvexReactClient) expose close() and require it
- The current implementation in convex-sandbox.ts is correct: it properly closes
realtimeClient(which is a ConvexClient using WebSocket)The review comment's suggestion to call
await this.convex.close()would result in a runtime error because ConvexHttpClient does not have this method. The reference to packages/local/src/index.ts was misleading—that file uses ConvexClient (WebSocket-based), not ConvexHttpClient.
The close() method correctly closes only the WebSocket-based realtimeClient, which is appropriate.
ConvexHttpClient (
this.convex) uses HTTP requests and does not maintain persistent connections requiring cleanup. Attempting to callawait this.convex.close()as suggested would fail at runtime since ConvexHttpClient has no close() method. The current implementation in lines 334-337 is correct and complete.Likely an incorrect or invalid review comment.
- Add closeCurrentSandbox() helper to properly close ConvexSandbox - Close WebSocket connection before replacing sandbox in: - setSandboxPreference() - getSandbox() when switching connections - getE2BSandbox() when switching to E2B - Remove unused isConnected() from ConvexSandbox - Remove unused getSandboxType() from HybridSandboxManager
- Remove unused exports from convex/localSandbox.ts: - verifyToken query (never called externally) - isConnected query (frontend uses listConnections) - getResult query (replaced by subscribeToResult) - Remove unused methods from ConvexSandbox: - getConnectionInfo() (never called) - getHost() (E2B compat stub never used) - Remove unused methods from HybridSandboxManager: - isUsingLocal() (never called) - getCurrentConnectionId() (never called) - Remove unused truncateOutput import from convex-sandbox.ts Total: 141 lines of dead code removed
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
lib/ai/tools/utils/convex-sandbox.ts (2)
94-144: Use an explicitrandomUUIDimport instead of the globalcryptoobject.
commands.runrelies oncrypto.randomUUID()without importingcrypto. This assumes a globalcryptois available and correctly typed, which can be brittle across runtimes/bundlers.Consider importing
randomUUIDfrom Node’scryptomodule (or your preferred UUID helper) and using that instead:-import { EventEmitter } from "events"; -import { ConvexHttpClient, ConvexClient } from "convex/browser"; -import { api } from "@/convex/_generated/api"; +import { EventEmitter } from "events"; +import { randomUUID } from "crypto"; +import { ConvexHttpClient, ConvexClient } from "convex/browser"; +import { api } from "@/convex/_generated/api"; @@ - const commandId = crypto.randomUUID(); + const commandId = randomUUID();This makes the dependency explicit and avoids surprises if global
cryptois not present or typed in some environments.
320-324:close()should also dispose the ConvexHttpClient to avoid leaking connections.
close()currently only callsthis.realtimeClient.close()but never closes theConvexHttpClient(this.convex). The Convex docs and your local CLI (packages/local/src/index.ts) show thatConvexHttpClient.close()should be called to clean up WebSocket/HTTP client resources.Update
close()to dispose both clients:// E2B-compatible interface: close() async close(): Promise<void> { - await this.realtimeClient.close(); - this.emit("close"); + // Close both HTTP and realtime Convex clients to free resources + await this.convex.close(); + await this.realtimeClient.close(); + this.emit("close"); }This prevents leaking Convex client connections for long‑lived servers or repeated sandbox lifecycle transitions.
lib/ai/tools/utils/hybrid-sandbox-manager.ts (1)
75-94: ConcurrentgetSandboxcalls can race and leakConvexSandboxinstances; add a shared in‑flight promise.
getSandboxhas no concurrency guard: multiple concurrent callers with a local preference can each create their ownConvexSandbox. Only the last one assigned tothis.sandboxis retained; earlier instances (with open WebSockets / Convex clients) are never closed. Similar races can occur when switching between local and E2B.This is a reliability/resource issue (leaked connections, extra load on Convex/local Docker), especially under concurrent tool usage.
Consider introducing a shared in‑flight promise and moving the current logic into a private helper:
export class HybridSandboxManager implements SandboxManager { private sandbox: SandboxType | null = null; private isLocal = false; private currentConnectionId: string | null = null; private convex: ConvexHttpClient; private convexUrl: string; + private sandboxPromise: Promise<{ sandbox: SandboxType }> | null = null; @@ - async getSandbox(): Promise<{ sandbox: SandboxType }> { - // If preference is E2B, always use E2B - if (this.sandboxPreference === "e2b") { - return this.getE2BSandbox(); - } - - // Check if the preferred connection is available - const connections = await this.listConnections(); - // ... existing body ... - return this.getE2BSandbox(); - } + async getSandbox(): Promise<{ sandbox: SandboxType }> { + if (this.sandboxPromise) { + return this.sandboxPromise; + } + + this.sandboxPromise = this.getSandboxInternal(); + try { + return await this.sandboxPromise; + } finally { + this.sandboxPromise = null; + } + } + + private async getSandboxInternal(): Promise<{ sandbox: SandboxType }> { + // If preference is E2B, always use E2B + if (this.sandboxPreference === "e2b") { + return this.getE2BSandbox(); + } + + // Check if the preferred connection is available + const connections = await this.listConnections(); + // ... move the rest of the current getSandbox logic here unchanged ... + }This ensures only one sandbox transition per manager at a time and avoids leaking extra ConvexSandbox instances created by overlapping calls.
Also applies to: 115-205
🧹 Nitpick comments (3)
convex/localSandbox.ts (3)
128-255: Connection management handles auth and ownership sensibly; minor reuse possible
connect,heartbeat, anddisconnectcorrectly gate access on tokens, enforceconnection.user_idalignment, and respect connection status transitions (connected→disconnected). This matches the intended capability model for the CLI.If you want to tighten consistency further, you could reuse the
validateTokenhelper inconnect(mirroringheartbeat/disconnect) so token validation logic and error semantics live in one place, but the current implementation is already correct.
560-618: Result subscription/deletion flow is coherent; consider tightening the commentThe
subscribeToResultquery keeps its reactive surface minimal by only touchinglocal_sandbox_resultsvia theby_command_idindex and then enforcingresult.user_id === userId, withcommandIdbeing a server-generated UUID.deleteResultis correctly gated byserviceKeyand double-checksuserIdbefore deleting, which limits accidental cross-user deletions from backend callers.One small nit: the comment “userId is validated” above
subscribeToResultcould be clarified to say that user ownership is validated against the storedresult.user_id(since there’s noctx.authcheck here). That would better reflect what the function actually does without changing behavior.
624-729: Cleanup cron mutations use indexes appropriately; you may want to bound stale-connection scansBoth
cleanupStaleConnectionsandcleanupOldCommands:
- Are correctly declared as
internalMutations.- Use dedicated indexes (
by_status_and_last_heartbeat,by_status_and_created_at,by_completed_at) instead of filters.- Limit deletions to small batches (
take(100)) for commands/results/connections, which aligns with Convex’s recommendations about write limits.The only place still using
.collect()is the stale connection scan. If you ever expect many thousands of concurrently “connected” rows, you might consider switching that to a bounded.take(100)loop (similar to the other sections) to keep each cron run within comfortable read/write limits; for typical usage with only a handful of connections per user, the current approach is fine.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
convex/localSandbox.ts(1 hunks)lib/ai/tools/utils/convex-sandbox.ts(1 hunks)lib/ai/tools/utils/hybrid-sandbox-manager.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
convex/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/convex_rules.mdc)
convex/**/*.{ts,tsx}: ALWAYS use the new function syntax for Convex functions with explicitargs,returns, andhandlerproperties
Usev.null()validator when returning null values from Convex functions
Usev.int64()instead of the deprecatedv.bigint()for representing signed 64-bit integers
Usev.record()for defining record types;v.map()andv.set()are not supported
Index fields must be queried in the same order they are defined; create separate indexes for different query orders
Use theId<'tableName'>TypeScript helper type from './_generated/dataModel' for strict typing of document IDs
Be strict with types around document IDs; useId<'tableName'>rather than genericstringtypes
Always useas constfor string literals in discriminated union types
Always define arrays asconst array: Array<T> = [...]with explicit type annotations
Always define records asconst record: Record<KeyType, ValueType> = {...}with explicit type annotations
UseinternalQuery,internalMutation, andinternalActionto register private functions; these are only callable by other Convex functions
Usequery,mutation, andactionto register public functions that are exposed to the public API
ALWAYS include argument and return validators for all Convex functions, includingreturns: v.null()if the function doesn't return anything
Usectx.runQueryto call a query from a query, mutation, or action
Usectx.runMutationto call a mutation from a mutation or action
Usectx.runActionto call an action from an action; only use this when crossing runtimes (e.g., V8 to Node)
When calling functions in the same file usingctx.runQuery,ctx.runMutation, orctx.runAction, specify a type annotation on the return value to work around TypeScript circularity limitations
Use theapiobject fromconvex/_generated/api.tsto call public functions registered withquery,mutation, oraction
Use theinternalobject fromconvex/_generated/api.ts...
Files:
convex/localSandbox.ts
🧠 Learnings (14)
📚 Learning: 2025-11-12T15:23:01.117Z
Learnt from: fkesheh
Repo: hackerai-tech/hackerai PR: 92
File: lib/ai/tools/utils/sandbox-file-uploader.ts:10-10
Timestamp: 2025-11-12T15:23:01.117Z
Learning: In the hackerai codebase, NEXT_PUBLIC_USE_S3_STORAGE is intentionally used in both client and server code (including server-only files like lib/ai/tools/utils/sandbox-file-uploader.ts) to ensure the frontend and backend stay in sync regarding which storage backend (S3 vs Convex) is being used. This consistency is important for file upload flows that span both environments.
Applied to files:
lib/ai/tools/utils/hybrid-sandbox-manager.tslib/ai/tools/utils/convex-sandbox.ts
📚 Learning: 2025-09-01T12:44:12.626Z
Learnt from: RostyslavManko
Repo: hackerai-tech/hackerai PR: 17
File: convex/fileStorage.ts:131-149
Timestamp: 2025-09-01T12:44:12.626Z
Learning: In convex/fileStorage.ts, serviceKey parameters are made optional (v.optional(v.string())) for queries like getFileTokensByFileIds to simplify local development workflow - developers don't need to configure service keys in the Convex dashboard when working locally. In production, the serviceKey is always provided, ensuring security. This pattern balances developer experience with production security requirements.
Applied to files:
lib/ai/tools/utils/hybrid-sandbox-manager.ts
📚 Learning: 2025-11-24T20:59:07.600Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-11-24T20:59:07.600Z
Learning: Applies to convex/**/*.{ts,tsx} : Use `v.null()` validator when returning null values from Convex functions
Applied to files:
lib/ai/tools/utils/hybrid-sandbox-manager.ts
📚 Learning: 2025-11-24T20:59:07.600Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-11-24T20:59:07.600Z
Learning: Applies to convex/**/*.{ts,tsx} : Use the `internal` object from `convex/_generated/api.ts` to call internal functions registered with `internalQuery`, `internalMutation`, or `internalAction`
Applied to files:
convex/localSandbox.ts
📚 Learning: 2025-11-24T20:59:07.600Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-11-24T20:59:07.600Z
Learning: Applies to convex/**/*.{ts,tsx} : Use `internalQuery`, `internalMutation`, and `internalAction` to register private functions; these are only callable by other Convex functions
Applied to files:
convex/localSandbox.ts
📚 Learning: 2025-11-24T20:59:07.600Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-11-24T20:59:07.600Z
Learning: Applies to convex/**/*.{ts,tsx} : Organize public query, mutation, or action functions thoughtfully within the `convex/` directory using file-based routing
Applied to files:
convex/localSandbox.ts
📚 Learning: 2025-11-24T20:59:07.600Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-11-24T20:59:07.600Z
Learning: Applies to convex/**/*.{ts,tsx} : Use the `api` object from `convex/_generated/api.ts` to call public functions registered with `query`, `mutation`, or `action`
Applied to files:
convex/localSandbox.ts
📚 Learning: 2025-11-24T20:59:07.600Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-11-24T20:59:07.600Z
Learning: Applies to convex/**/*.{ts,tsx} : Try to use as few calls from actions to queries and mutations as possible to minimize race condition risk
Applied to files:
convex/localSandbox.tslib/ai/tools/utils/convex-sandbox.ts
📚 Learning: 2025-11-24T20:59:07.600Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-11-24T20:59:07.600Z
Learning: Applies to convex/**/*.{ts,tsx} : Do NOT use `filter` in queries; instead, define an index in the schema and use `withIndex`
Applied to files:
convex/localSandbox.ts
📚 Learning: 2025-11-24T20:59:07.600Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-11-24T20:59:07.600Z
Learning: Applies to convex/**/*.{ts,tsx} : When using async iteration, don't use `.collect()` or `.take(n)` on query results; instead use `for await (const row of query)` syntax
Applied to files:
convex/localSandbox.ts
📚 Learning: 2025-11-24T20:59:07.600Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-11-24T20:59:07.600Z
Learning: Applies to convex/**/*.{ts,tsx} : ALWAYS use the new function syntax for Convex functions with explicit `args`, `returns`, and `handler` properties
Applied to files:
lib/ai/tools/utils/convex-sandbox.ts
📚 Learning: 2025-11-24T20:59:07.600Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-11-24T20:59:07.600Z
Learning: Applies to convex/**/*.{ts,tsx} : Pull out shared code into helper async functions and call them directly instead of using `ctx.runAction` for same-runtime calls
Applied to files:
lib/ai/tools/utils/convex-sandbox.ts
📚 Learning: 2025-11-24T20:59:07.600Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-11-24T20:59:07.600Z
Learning: Applies to convex/schema.ts : Always define schema in `convex/schema.ts` and import schema definition functions from `convex/server`
Applied to files:
lib/ai/tools/utils/convex-sandbox.ts
📚 Learning: 2025-11-24T20:59:07.600Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-11-24T20:59:07.600Z
Learning: Applies to convex/**/*.{ts,tsx} : Always add `"use node";` to the top of files containing actions that use Node.js built-in modules
Applied to files:
lib/ai/tools/utils/convex-sandbox.ts
🧬 Code graph analysis (3)
lib/ai/tools/utils/hybrid-sandbox-manager.ts (4)
lib/ai/tools/utils/convex-sandbox.ts (1)
ConvexSandbox(31-325)lib/ai/tools/index.ts (1)
SandboxPreference(128-128)types/agent.ts (1)
SandboxManager(13-16)lib/ai/tools/utils/sandbox.ts (1)
ensureSandboxConnection(34-136)
convex/localSandbox.ts (5)
convex/_generated/server.d.ts (1)
DatabaseReader(132-132)packages/local/src/index.ts (1)
connect(330-376)lib/ai/tools/utils/hybrid-sandbox-manager.ts (1)
listConnections(99-113)convex/chats.ts (1)
validateServiceKey(6-10)convex/tempStreams.ts (1)
validateServiceKey(4-8)
lib/ai/tools/utils/convex-sandbox.ts (1)
packages/local/src/index.ts (1)
cleanup(540-583)
🔇 Additional comments (5)
convex/localSandbox.ts (5)
9-31: Token helpers are clean and correctly indexed
generateTokenandvalidateTokennicely separate token creation from lookup, andvalidateToken’s narrow return type makes it easy to use safely across mutations/queries. Index-based lookup onby_tokenis aligned with the Convex best practices from the coding guidelines.
33-122: User-scoped token lifecycle looks solid
getToken/regenerateTokencorrectly tie tokens toidentity.subject, enforce authentication viaConvexError, reuse theby_user_idindex, and ensure only one active token per user while cleanly rotating on regenerate. The regeneration path also eagerly disconnects existing connections, which keeps the model consistent.
257-367: Connection listing APIs are well-scoped and efficient for expected cardinalityBoth
listConnectionsandlistConnectionsForBackend:
- Use an index on
(user_id, status)to fetch only “connected” rows.- Apply a 30s heartbeat cutoff in-memory to hide recently stale connections.
- Map to a narrow DTO suitable for UI/backend use.
The split between auth-based access for the user-facing query and
serviceKey-based access for the backend query is clear and consistent with the rest of the module.
373-477: Command enqueue + pending fetch enforce correct invariants and minimize reactivity
enqueueCommandnow:
- Validates the
serviceKey.- Ensures the connection exists, belongs to
args.userId, and is currentlyconnected,- Then writes a
pendingcommand with a strongly-typedenvmap.
getPendingCommandsintentionally skips token/service-key checks to reduce reactive dependencies and only depends onlocal_sandbox_commandsvia theby_connection_and_statusindex and a small.take(10)window. GivenconnectionIdis an unguessable UUID issued by the server, this is a reasonable trade-off between security and performance.
479-558: Command execution state transitions and result recording are guarded correctly
markCommandExecutingandsubmitResult:
- Use
validateTokento authenticate the caller.- Re-fetch the command by
command_idand enforcecommand.user_id === tokenResult.userId.- Restrict the status transition to
pending → executingbefore allowing results.- Persist results into
local_sandbox_resultsand then mark the commandcompleted.This keeps ownership and lifecycle transitions consistent and prevents another user’s token from “stealing” or completing a command.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
convex/schema.ts (1)
146-152: Hostname inos_infois potentially sensitive device metadataStoring
os_info.hostnameand detailed OS fields can make devices uniquely identifiable. If you want to reduce fingerprinting/PII surface, consider hashing or truncating hostname and/or storing only coarse-grained OS info (e.g., platform + arch).convex/localSandbox.ts (1)
373-435: Optionally deriveuserIdfrom the connection inenqueueCommandYou already validate that the connection exists and
connection.user_id === args.userId. At this point you could dropuserIdfrom the args and always populateuser_idfromconnection.user_idwhen inserting intolocal_sandbox_commands, which removes one caller-controlled field and avoids future mismatch bugs between args and DB.lib/ai/tools/utils/convex-sandbox.ts (2)
108-110: Prefer importingrandomUUIDfromcryptoinstead of using globalcrypto.randomUUID().Relying on the global
cryptocan be brittle across runtimes/bundlers. An explicit import from Node’scryptomodule improves clarity and type safety:-import { EventEmitter } from "events"; -import { ConvexHttpClient, ConvexClient } from "convex/browser"; +import { EventEmitter } from "events"; +import { randomUUID } from "crypto"; +import { ConvexHttpClient, ConvexClient } from "convex/browser"; @@ - const commandId = crypto.randomUUID(); + const commandId = randomUUID();This keeps the behavior the same while making the dependency explicit.
Node.js `crypto.randomUUID()` usage recommendations and TypeScript typing for global `crypto` vs imported `randomUUID`
321-325: ConvexHttpClient is never closed inclose(), causing a resource leak (regression).
close()currently only awaitsthis.realtimeClient.close()and emits"close". TheConvexHttpClientinstance (this.convex) remains open, which can leak WebSocket connections/resources over time. A prior review already recommended closing the HTTP client as well; this appears to have regressed.Update
close()to also closethis.convex:// E2B-compatible interface: close() async close(): Promise<void> { - await this.realtimeClient.close(); - this.emit("close"); + await this.convex.close(); + await this.realtimeClient.close(); + this.emit("close"); }ConvexHttpClient.close() behavior and recommendations for client lifecycle management
🧹 Nitpick comments (1)
packages/local/src/index.ts (1)
56-64: Hard‑coded/bin/bashlimits portability to Unix‑like environmentsThe CLI currently assumes
/bin/bashexists (inrunShellCommand, dangerous-mode execution, and background spawning). That’s fine if you officially target Linux/macOS only, but on Windows (without WSL) this will fail with spawn errors.If you want broader portability, consider either:
- failing fast with a clear error when
os.platform() === "win32"and no Bash is available, or- making the shell path configurable (env/flag) and defaulting to
/bin/bashon Unix.Also applies to: 458-462, 505-518
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
convex/localSandbox.ts(1 hunks)convex/schema.ts(1 hunks)lib/ai/tools/utils/convex-sandbox.ts(1 hunks)packages/local/src/index.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
convex/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/convex_rules.mdc)
convex/**/*.{ts,tsx}: ALWAYS use the new function syntax for Convex functions with explicitargs,returns, andhandlerproperties
Usev.null()validator when returning null values from Convex functions
Usev.int64()instead of the deprecatedv.bigint()for representing signed 64-bit integers
Usev.record()for defining record types;v.map()andv.set()are not supported
Index fields must be queried in the same order they are defined; create separate indexes for different query orders
Use theId<'tableName'>TypeScript helper type from './_generated/dataModel' for strict typing of document IDs
Be strict with types around document IDs; useId<'tableName'>rather than genericstringtypes
Always useas constfor string literals in discriminated union types
Always define arrays asconst array: Array<T> = [...]with explicit type annotations
Always define records asconst record: Record<KeyType, ValueType> = {...}with explicit type annotations
UseinternalQuery,internalMutation, andinternalActionto register private functions; these are only callable by other Convex functions
Usequery,mutation, andactionto register public functions that are exposed to the public API
ALWAYS include argument and return validators for all Convex functions, includingreturns: v.null()if the function doesn't return anything
Usectx.runQueryto call a query from a query, mutation, or action
Usectx.runMutationto call a mutation from a mutation or action
Usectx.runActionto call an action from an action; only use this when crossing runtimes (e.g., V8 to Node)
When calling functions in the same file usingctx.runQuery,ctx.runMutation, orctx.runAction, specify a type annotation on the return value to work around TypeScript circularity limitations
Use theapiobject fromconvex/_generated/api.tsto call public functions registered withquery,mutation, oraction
Use theinternalobject fromconvex/_generated/api.ts...
Files:
convex/schema.tsconvex/localSandbox.ts
convex/schema.ts
📄 CodeRabbit inference engine (.cursor/rules/convex_rules.mdc)
convex/schema.ts: Always define schema inconvex/schema.tsand import schema definition functions fromconvex/server
Include all index fields in the index name; for example, an index on ['field1', 'field2'] should be named 'by_field1_and_field2'
Files:
convex/schema.ts
🧠 Learnings (13)
📚 Learning: 2025-11-24T20:59:07.600Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-11-24T20:59:07.600Z
Learning: Applies to convex/schema.ts : Always define schema in `convex/schema.ts` and import schema definition functions from `convex/server`
Applied to files:
convex/schema.tsconvex/localSandbox.ts
📚 Learning: 2025-11-24T20:59:07.600Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-11-24T20:59:07.600Z
Learning: Applies to convex/**/*.{ts,tsx} : Organize public query, mutation, or action functions thoughtfully within the `convex/` directory using file-based routing
Applied to files:
convex/schema.tsconvex/localSandbox.ts
📚 Learning: 2025-09-01T12:44:12.626Z
Learnt from: RostyslavManko
Repo: hackerai-tech/hackerai PR: 17
File: convex/fileStorage.ts:131-149
Timestamp: 2025-09-01T12:44:12.626Z
Learning: In convex/fileStorage.ts, serviceKey parameters are made optional (v.optional(v.string())) for queries like getFileTokensByFileIds to simplify local development workflow - developers don't need to configure service keys in the Convex dashboard when working locally. In production, the serviceKey is always provided, ensuring security. This pattern balances developer experience with production security requirements.
Applied to files:
convex/schema.ts
📚 Learning: 2025-11-24T20:59:07.600Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-11-24T20:59:07.600Z
Learning: Applies to convex/**/*.{ts,tsx} : Use the `internal` object from `convex/_generated/api.ts` to call internal functions registered with `internalQuery`, `internalMutation`, or `internalAction`
Applied to files:
convex/localSandbox.ts
📚 Learning: 2025-11-24T20:59:07.600Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-11-24T20:59:07.600Z
Learning: Applies to convex/**/*.{ts,tsx} : Use `internalQuery`, `internalMutation`, and `internalAction` to register private functions; these are only callable by other Convex functions
Applied to files:
convex/localSandbox.ts
📚 Learning: 2025-11-24T20:59:07.600Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-11-24T20:59:07.600Z
Learning: Applies to convex/**/*.{ts,tsx} : Use the `api` object from `convex/_generated/api.ts` to call public functions registered with `query`, `mutation`, or `action`
Applied to files:
convex/localSandbox.ts
📚 Learning: 2025-11-24T20:59:07.600Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-11-24T20:59:07.600Z
Learning: Applies to convex/**/*.{ts,tsx} : Do NOT use `filter` in queries; instead, define an index in the schema and use `withIndex`
Applied to files:
convex/localSandbox.ts
📚 Learning: 2025-11-24T20:59:07.600Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-11-24T20:59:07.600Z
Learning: Applies to convex/**/*.{ts,tsx} : Try to use as few calls from actions to queries and mutations as possible to minimize race condition risk
Applied to files:
convex/localSandbox.tslib/ai/tools/utils/convex-sandbox.ts
📚 Learning: 2025-11-24T20:59:07.600Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-11-24T20:59:07.600Z
Learning: Applies to convex/**/*.{ts,tsx} : ALWAYS use the new function syntax for Convex functions with explicit `args`, `returns`, and `handler` properties
Applied to files:
lib/ai/tools/utils/convex-sandbox.ts
📚 Learning: 2025-11-24T20:59:07.600Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-11-24T20:59:07.600Z
Learning: Applies to convex/**/*.{ts,tsx} : Always add `"use node";` to the top of files containing actions that use Node.js built-in modules
Applied to files:
lib/ai/tools/utils/convex-sandbox.tspackages/local/src/index.ts
📚 Learning: 2025-11-24T20:59:07.600Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-11-24T20:59:07.600Z
Learning: Applies to convex/**/*.{ts,tsx} : Pull out shared code into helper async functions and call them directly instead of using `ctx.runAction` for same-runtime calls
Applied to files:
lib/ai/tools/utils/convex-sandbox.ts
📚 Learning: 2025-11-12T15:23:01.117Z
Learnt from: fkesheh
Repo: hackerai-tech/hackerai PR: 92
File: lib/ai/tools/utils/sandbox-file-uploader.ts:10-10
Timestamp: 2025-11-12T15:23:01.117Z
Learning: In the hackerai codebase, NEXT_PUBLIC_USE_S3_STORAGE is intentionally used in both client and server code (including server-only files like lib/ai/tools/utils/sandbox-file-uploader.ts) to ensure the frontend and backend stay in sync regarding which storage backend (S3 vs Convex) is being used. This consistency is important for file upload flows that span both environments.
Applied to files:
lib/ai/tools/utils/convex-sandbox.ts
📚 Learning: 2025-11-24T20:59:07.600Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-11-24T20:59:07.600Z
Learning: Applies to package.json : Add `types/node` to `package.json` when using any Node.js built-in modules in Convex functions
Applied to files:
packages/local/src/index.ts
🧬 Code graph analysis (2)
convex/localSandbox.ts (5)
convex/_generated/server.d.ts (1)
DatabaseReader(132-132)packages/local/src/index.ts (1)
connect(331-377)lib/ai/tools/utils/hybrid-sandbox-manager.ts (1)
listConnections(99-113)convex/chats.ts (1)
validateServiceKey(6-10)convex/tempStreams.ts (1)
validateServiceKey(4-8)
lib/ai/tools/utils/convex-sandbox.ts (1)
packages/local/src/index.ts (1)
cleanup(587-630)
🔇 Additional comments (8)
convex/schema.ts (1)
124-162: Local sandbox schema and indexes look consistent and query-friendlyThe four
local_sandbox_*tables and their indexes line up cleanly with how they’re queried inconvex/localSandbox.ts(lookup by token, user/status, command/connection IDs, and time-based cleanup). Names follow the “include all fields in index name” convention, and I don’t see correctness or performance red flags here.Also applies to: 164-198
convex/localSandbox.ts (1)
9-122: Token, connection, and command/result lifecycles are coherently enforcedThe flows for token issuance/rotation, connection validation (
heartbeat/disconnect), command state transitions (pending→executing→completed), and cleanup all look consistent and match the schema/index design. Ownership checks on commands/results and token validation before mutating state give you a solid security and data-integrity story here.Also applies to: 128-367, 483-737
packages/local/src/index.ts (1)
19-136: Robust command execution with bounded output and timeout handlingThe
truncateOutput+runShellCommandcombination, and how you thread it throughexecuteCommand, gives you good control over memory usage and LLM context size while still preserving useful head/tail output, and the SIGTERM→SIGKILL timeout path avoids hung processes. This is a solid foundation for the CLI’s execution loop.Also applies to: 399-502
lib/ai/tools/utils/convex-sandbox.ts (5)
32-86: ConvexSandbox structure and context helpers look good.Class shape, constructor wiring, and the mode-specific sandbox/OS context strings are clear and align with the intended “local vs docker vs dangerous” behavior. No issues here.
Also applies to: 88-94
199-207: File write implementation and path escaping look robust.
escapePathuses the standard single-quote pattern ('...'\''...') and is applied consistently in file writes. Chunked binary writes via base64 +printf | base64 -davoid interpretation of content, and the heredoc path for text writes uses a randomly-generated, single-quoted delimiter, which prevents content-based injection.Nice hardening of the shell interaction.
Also applies to: 208-256
258-282: Read/remove/list file operations use proper shell-escaping.
cat,rm -rf, andfindall run withescapePath(path), which eliminates earlier shell-injection risks while preserving expected behavior (e.g., handling spaces, quotes). Thelistcontract (files only,-maxdepth 1) is also clear and predictable.
147-197: Timeout and subscription cleanup logic is correct and follows Convex best practices.The
waitForResultmethod implements the idiomatic Convex one-shot Promise pattern properly: it correctly stores the unsubscribe function, calls it in all exit paths (timeout, result received, errors), maintains idempotent cleanup with proper existence checks, and deletes results asynchronously without blocking. The pattern is well-suited for managing real-time subscriptions with bounded waits, and the implementation properly integrates with the class-level resource cleanup viarealtimeClient.close().
284-318: Code escaping is correct, but upstream URL validation is missing.The
escapePath()function and URL escaping use proper shell-quote techniques ('\''pattern), which correctly prevent shell injection for standard URLs and file paths. However, the code processes URLs from user-provided file attachments without validating them against a URL format/scheme whitelist beforehand.Concern: While the escaping prevents shell injection for typical URLs, URLs should ideally be validated (e.g., must start with
https://, must be from allowed domains) before reachingdownloadFromUrlanduploadToUrl. The current approach relies solely on escaping rather than input validation.Suggestion: Add upstream validation in
applyModeSpecificTransforms(or equivalent) to ensure only valid, expected URL schemes and domains are added tosandboxFiles.
- Add is_complete and started_at as optional fields to local_sandbox_results - Add cleanupLegacyResults mutation to delete results with legacy fields - Add cleanup to cron job (runs every 5 min)
- Add 1-hour idle timeout (auto-terminates when no commands received) - Add --persist flag to reuse existing containers across restarts - Fix schema: use typed v.record() instead of v.any() for env - Add error logging for cleanup, heartbeat, and disconnect failures - Add exit code checking for file write/remove operations - Make setSandboxPreference async to properly await cleanup 🤖 Generated with [Claude Code](https://claude.com/claude-code)
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
lib/ai/tools/utils/convex-sandbox.ts (1)
32-47: Also close the Convex HTTP client inclose()to mirror local client cleanup.
ConvexSandboxconstructs both aConvexHttpClient(this.convex) and a realtimeConvexClient(this.realtimeClient), butclose()currently only closes the realtime client. To avoid leaking Convex client resources, consider also closingthis.convex:async close(): Promise<void> { // If your ConvexHttpClient version exposes close(), this is safe: await this.convex.close(); await this.realtimeClient.close(); this.emit("close"); }This matches the cleanup pattern used in
packages/local/src/index.tsand keeps both Convex clients well-behaved in long-lived processes.Also applies to: 333-336
🧹 Nitpick comments (2)
convex/schema.ts (1)
124-198: Local sandbox schema and indexes look solid; hostname can be privacy‑hardened if desired.The four
local_sandbox_*tables are well-structured, usev.recordappropriately forenv, and the composite index names (e.g.by_status_and_last_heartbeat,by_status_and_created_at) follow the “include all fields in the name” guideline. If you ever want to reduce host fingerprinting, you could optionally truncate/hashos_info.hostnameor store only coarse OS info, but that’s a policy choice rather than a blocker.lib/ai/tools/utils/hybrid-sandbox-manager.ts (1)
42-59: Consider adding a HybridSandboxManager cleanup that also closes the Convex client.You already close local
ConvexSandboxinstances viacloseCurrentSandbox(), but theConvexHttpClientinthis.convexis never disposed. Given Convex’s client APIs exposeclose()and your local CLI calls it to clean up connections, it would be safer to add adispose/closemethod onHybridSandboxManagerthat:
- calls
await this.closeCurrentSandbox(), and- then
await this.convex.close()(with a try/catch + log).Call this from whatever owns the manager (e.g. chat/session teardown) to avoid lingering Convex client resources in long-lived processes.
Also applies to: 75-81, 185-198
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
convex/schema.ts(1 hunks)lib/ai/tools/utils/convex-sandbox.ts(1 hunks)lib/ai/tools/utils/hybrid-sandbox-manager.ts(1 hunks)packages/local/src/index.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/local/src/index.ts
🧰 Additional context used
📓 Path-based instructions (2)
convex/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/convex_rules.mdc)
convex/**/*.{ts,tsx}: ALWAYS use the new function syntax for Convex functions with explicitargs,returns, andhandlerproperties
Usev.null()validator when returning null values from Convex functions
Usev.int64()instead of the deprecatedv.bigint()for representing signed 64-bit integers
Usev.record()for defining record types;v.map()andv.set()are not supported
Index fields must be queried in the same order they are defined; create separate indexes for different query orders
Use theId<'tableName'>TypeScript helper type from './_generated/dataModel' for strict typing of document IDs
Be strict with types around document IDs; useId<'tableName'>rather than genericstringtypes
Always useas constfor string literals in discriminated union types
Always define arrays asconst array: Array<T> = [...]with explicit type annotations
Always define records asconst record: Record<KeyType, ValueType> = {...}with explicit type annotations
UseinternalQuery,internalMutation, andinternalActionto register private functions; these are only callable by other Convex functions
Usequery,mutation, andactionto register public functions that are exposed to the public API
ALWAYS include argument and return validators for all Convex functions, includingreturns: v.null()if the function doesn't return anything
Usectx.runQueryto call a query from a query, mutation, or action
Usectx.runMutationto call a mutation from a mutation or action
Usectx.runActionto call an action from an action; only use this when crossing runtimes (e.g., V8 to Node)
When calling functions in the same file usingctx.runQuery,ctx.runMutation, orctx.runAction, specify a type annotation on the return value to work around TypeScript circularity limitations
Use theapiobject fromconvex/_generated/api.tsto call public functions registered withquery,mutation, oraction
Use theinternalobject fromconvex/_generated/api.ts...
Files:
convex/schema.ts
convex/schema.ts
📄 CodeRabbit inference engine (.cursor/rules/convex_rules.mdc)
convex/schema.ts: Always define schema inconvex/schema.tsand import schema definition functions fromconvex/server
Include all index fields in the index name; for example, an index on ['field1', 'field2'] should be named 'by_field1_and_field2'
Files:
convex/schema.ts
🧠 Learnings (10)
📓 Common learnings
Learnt from: fkesheh
Repo: hackerai-tech/hackerai PR: 114
File: lib/ai/tools/utils/convex-sandbox.ts:95-145
Timestamp: 2025-11-28T01:16:26.484Z
Learning: In the local sandbox architecture (lib/ai/tools/utils/convex-sandbox.ts), background command execution semantics are handled by the local sandbox client (packages/local/src/index.ts), not by the ConvexSandbox class. When background: true, the local client immediately submits a result with empty output and PID after spawning the process, causing waitForResult() to resolve immediately and maintaining E2B-compatible non-blocking behavior.
📚 Learning: 2025-11-24T20:59:07.600Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-11-24T20:59:07.600Z
Learning: Applies to convex/schema.ts : Always define schema in `convex/schema.ts` and import schema definition functions from `convex/server`
Applied to files:
convex/schema.ts
📚 Learning: 2025-09-01T12:44:12.626Z
Learnt from: RostyslavManko
Repo: hackerai-tech/hackerai PR: 17
File: convex/fileStorage.ts:131-149
Timestamp: 2025-09-01T12:44:12.626Z
Learning: In convex/fileStorage.ts, serviceKey parameters are made optional (v.optional(v.string())) for queries like getFileTokensByFileIds to simplify local development workflow - developers don't need to configure service keys in the Convex dashboard when working locally. In production, the serviceKey is always provided, ensuring security. This pattern balances developer experience with production security requirements.
Applied to files:
convex/schema.tslib/ai/tools/utils/hybrid-sandbox-manager.ts
📚 Learning: 2025-11-28T01:16:26.484Z
Learnt from: fkesheh
Repo: hackerai-tech/hackerai PR: 114
File: lib/ai/tools/utils/convex-sandbox.ts:95-145
Timestamp: 2025-11-28T01:16:26.484Z
Learning: In the local sandbox architecture (lib/ai/tools/utils/convex-sandbox.ts), background command execution semantics are handled by the local sandbox client (packages/local/src/index.ts), not by the ConvexSandbox class. When background: true, the local client immediately submits a result with empty output and PID after spawning the process, causing waitForResult() to resolve immediately and maintaining E2B-compatible non-blocking behavior.
Applied to files:
lib/ai/tools/utils/hybrid-sandbox-manager.tslib/ai/tools/utils/convex-sandbox.ts
📚 Learning: 2025-11-12T15:23:01.117Z
Learnt from: fkesheh
Repo: hackerai-tech/hackerai PR: 92
File: lib/ai/tools/utils/sandbox-file-uploader.ts:10-10
Timestamp: 2025-11-12T15:23:01.117Z
Learning: In the hackerai codebase, NEXT_PUBLIC_USE_S3_STORAGE is intentionally used in both client and server code (including server-only files like lib/ai/tools/utils/sandbox-file-uploader.ts) to ensure the frontend and backend stay in sync regarding which storage backend (S3 vs Convex) is being used. This consistency is important for file upload flows that span both environments.
Applied to files:
lib/ai/tools/utils/hybrid-sandbox-manager.tslib/ai/tools/utils/convex-sandbox.ts
📚 Learning: 2025-11-24T20:59:07.600Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-11-24T20:59:07.600Z
Learning: Applies to convex/**/*.{ts,tsx} : Use `v.null()` validator when returning null values from Convex functions
Applied to files:
lib/ai/tools/utils/hybrid-sandbox-manager.ts
📚 Learning: 2025-11-24T20:59:07.600Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-11-24T20:59:07.600Z
Learning: Applies to convex/**/*.{ts,tsx} : Try to use as few calls from actions to queries and mutations as possible to minimize race condition risk
Applied to files:
lib/ai/tools/utils/hybrid-sandbox-manager.tslib/ai/tools/utils/convex-sandbox.ts
📚 Learning: 2025-11-24T20:59:07.600Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-11-24T20:59:07.600Z
Learning: Applies to convex/**/*.{ts,tsx} : ALWAYS use the new function syntax for Convex functions with explicit `args`, `returns`, and `handler` properties
Applied to files:
lib/ai/tools/utils/convex-sandbox.ts
📚 Learning: 2025-11-24T20:59:07.600Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-11-24T20:59:07.600Z
Learning: Applies to convex/**/*.{ts,tsx} : Pull out shared code into helper async functions and call them directly instead of using `ctx.runAction` for same-runtime calls
Applied to files:
lib/ai/tools/utils/convex-sandbox.ts
📚 Learning: 2025-11-24T20:59:07.600Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-11-24T20:59:07.600Z
Learning: Applies to convex/**/*.{ts,tsx} : Always add `"use node";` to the top of files containing actions that use Node.js built-in modules
Applied to files:
lib/ai/tools/utils/convex-sandbox.ts
🧬 Code graph analysis (2)
lib/ai/tools/utils/hybrid-sandbox-manager.ts (3)
lib/ai/tools/utils/convex-sandbox.ts (1)
ConvexSandbox(32-337)types/agent.ts (1)
SandboxManager(13-16)lib/ai/tools/utils/sandbox.ts (1)
ensureSandboxConnection(34-136)
lib/ai/tools/utils/convex-sandbox.ts (1)
packages/local/src/index.ts (1)
cleanup(667-716)
Replace database lookups with cryptographic session verification in query handlers to reduce Convex reactive surface while maintaining security. Sessions are generated on connect(), refreshed via heartbeat(), and verified cryptographically in getPendingCommands() and subscribeToResult(). Key changes: - Add signSession() and verifySession() using crypto.subtle HMAC-SHA256 - Sessions expire after 10 minutes and refresh on heartbeat - Add authError field to query responses for explicit failure signaling - Validate signature format (64 hex chars) before crypto operations - Increase heartbeat interval to 60s and stale timeout to 5 minutes 🤖 Generated with [Claude Code](https://claude.com/claude-code)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
lib/ai/tools/utils/convex-sandbox.ts (1)
1-4: Consider explicit crypto import for better clarity.Line 115 uses
crypto.randomUUID()which works at runtime (Node.js 15.7+) but relies on the globalcryptoobject. While TypeScript'sesnextlib provides the types, an explicit import improves code clarity and aligns with modern ES module practices.Apply this diff:
import { EventEmitter } from "events"; +import { randomUUID } from "crypto"; import { ConvexHttpClient, ConvexClient } from "convex/browser"; import { api } from "@/convex/_generated/api"; - const commandId = crypto.randomUUID(); + const commandId = randomUUID();Also applies to: 115-115
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.env.local.example(1 hunks)convex/localSandbox.ts(1 hunks)lib/ai/tools/utils/convex-sandbox.ts(1 hunks)packages/local/src/index.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/local/src/index.ts
🧰 Additional context used
📓 Path-based instructions (1)
convex/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/convex_rules.mdc)
convex/**/*.{ts,tsx}: ALWAYS use the new function syntax for Convex functions with explicitargs,returns, andhandlerproperties
Usev.null()validator when returning null values from Convex functions
Usev.int64()instead of the deprecatedv.bigint()for representing signed 64-bit integers
Usev.record()for defining record types;v.map()andv.set()are not supported
Index fields must be queried in the same order they are defined; create separate indexes for different query orders
Use theId<'tableName'>TypeScript helper type from './_generated/dataModel' for strict typing of document IDs
Be strict with types around document IDs; useId<'tableName'>rather than genericstringtypes
Always useas constfor string literals in discriminated union types
Always define arrays asconst array: Array<T> = [...]with explicit type annotations
Always define records asconst record: Record<KeyType, ValueType> = {...}with explicit type annotations
UseinternalQuery,internalMutation, andinternalActionto register private functions; these are only callable by other Convex functions
Usequery,mutation, andactionto register public functions that are exposed to the public API
ALWAYS include argument and return validators for all Convex functions, includingreturns: v.null()if the function doesn't return anything
Usectx.runQueryto call a query from a query, mutation, or action
Usectx.runMutationto call a mutation from a mutation or action
Usectx.runActionto call an action from an action; only use this when crossing runtimes (e.g., V8 to Node)
When calling functions in the same file usingctx.runQuery,ctx.runMutation, orctx.runAction, specify a type annotation on the return value to work around TypeScript circularity limitations
Use theapiobject fromconvex/_generated/api.tsto call public functions registered withquery,mutation, oraction
Use theinternalobject fromconvex/_generated/api.ts...
Files:
convex/localSandbox.ts
🧠 Learnings (9)
📓 Common learnings
Learnt from: fkesheh
Repo: hackerai-tech/hackerai PR: 114
File: lib/ai/tools/utils/convex-sandbox.ts:95-145
Timestamp: 2025-11-28T01:16:26.484Z
Learning: In the local sandbox architecture (lib/ai/tools/utils/convex-sandbox.ts), background command execution semantics are handled by the local sandbox client (packages/local/src/index.ts), not by the ConvexSandbox class. When background: true, the local client immediately submits a result with empty output and PID after spawning the process, causing waitForResult() to resolve immediately and maintaining E2B-compatible non-blocking behavior.
📚 Learning: 2025-11-28T01:16:26.484Z
Learnt from: fkesheh
Repo: hackerai-tech/hackerai PR: 114
File: lib/ai/tools/utils/convex-sandbox.ts:95-145
Timestamp: 2025-11-28T01:16:26.484Z
Learning: In the local sandbox architecture (lib/ai/tools/utils/convex-sandbox.ts), background command execution semantics are handled by the local sandbox client (packages/local/src/index.ts), not by the ConvexSandbox class. When background: true, the local client immediately submits a result with empty output and PID after spawning the process, causing waitForResult() to resolve immediately and maintaining E2B-compatible non-blocking behavior.
Applied to files:
convex/localSandbox.tslib/ai/tools/utils/convex-sandbox.ts
📚 Learning: 2025-11-24T20:59:07.600Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-11-24T20:59:07.600Z
Learning: Applies to convex/**/*.{ts,tsx} : Do NOT use `filter` in queries; instead, define an index in the schema and use `withIndex`
Applied to files:
convex/localSandbox.ts
📚 Learning: 2025-11-24T20:59:07.600Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-11-24T20:59:07.600Z
Learning: Applies to convex/**/*.{ts,tsx} : Try to use as few calls from actions to queries and mutations as possible to minimize race condition risk
Applied to files:
convex/localSandbox.tslib/ai/tools/utils/convex-sandbox.ts
📚 Learning: 2025-11-24T20:59:07.600Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-11-24T20:59:07.600Z
Learning: Applies to convex/**/*.{ts,tsx} : When using async iteration, don't use `.collect()` or `.take(n)` on query results; instead use `for await (const row of query)` syntax
Applied to files:
convex/localSandbox.ts
📚 Learning: 2025-11-24T20:59:07.600Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-11-24T20:59:07.600Z
Learning: Applies to convex/**/*.{ts,tsx} : ALWAYS use the new function syntax for Convex functions with explicit `args`, `returns`, and `handler` properties
Applied to files:
lib/ai/tools/utils/convex-sandbox.ts
📚 Learning: 2025-11-24T20:59:07.600Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-11-24T20:59:07.600Z
Learning: Applies to convex/**/*.{ts,tsx} : Pull out shared code into helper async functions and call them directly instead of using `ctx.runAction` for same-runtime calls
Applied to files:
lib/ai/tools/utils/convex-sandbox.ts
📚 Learning: 2025-11-24T20:59:07.600Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-11-24T20:59:07.600Z
Learning: Applies to convex/**/*.{ts,tsx} : Always add `"use node";` to the top of files containing actions that use Node.js built-in modules
Applied to files:
lib/ai/tools/utils/convex-sandbox.ts
📚 Learning: 2025-11-12T15:23:01.117Z
Learnt from: fkesheh
Repo: hackerai-tech/hackerai PR: 92
File: lib/ai/tools/utils/sandbox-file-uploader.ts:10-10
Timestamp: 2025-11-12T15:23:01.117Z
Learning: In the hackerai codebase, NEXT_PUBLIC_USE_S3_STORAGE is intentionally used in both client and server code (including server-only files like lib/ai/tools/utils/sandbox-file-uploader.ts) to ensure the frontend and backend stay in sync regarding which storage backend (S3 vs Convex) is being used. This consistency is important for file upload flows that span both environments.
Applied to files:
lib/ai/tools/utils/convex-sandbox.ts
🧬 Code graph analysis (2)
convex/localSandbox.ts (5)
convex/_generated/server.d.ts (1)
DatabaseReader(132-132)packages/local/src/index.ts (1)
connect(402-449)lib/ai/tools/utils/hybrid-sandbox-manager.ts (1)
listConnections(99-113)convex/tempStreams.ts (1)
validateServiceKey(4-8)convex/chats.ts (1)
validateServiceKey(6-10)
lib/ai/tools/utils/convex-sandbox.ts (1)
packages/local/src/index.ts (1)
cleanup(702-751)
🔇 Additional comments (1)
.env.local.example (1)
62-69: Well-documented addition of LOCAL_SANDBOX configuration.The new section is clearly documented with proper instructions for generating the secret and setting it up in Convex. The placement after the E2B section as an alternative is logical, and the guidance is consistent with the existing patterns in the file.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
types/agent.ts (1)
18-21: Type mismatch:SandboxContext.setSandboxshould acceptAnySandboxinstead ofSandbox.
SandboxContext.setSandboxaccepts onlySandbox, butSandboxManager.setSandboxacceptsAnySandbox(union ofSandbox | ConvexSandbox). Inhybrid-sandbox-manager.ts(lines 147, 168, 203, 211), thesetSandboxcallback derived fromSandboxContextis called withConvexSandboxinstances, creating a type safety violation. Update line 20 to useAnySandbox:setSandbox: (sandbox: AnySandbox) => void;
♻️ Duplicate comments (1)
lib/ai/tools/utils/convex-sandbox.ts (1)
369-373: Missing ConvexHttpClient close.The
close()method only closesrealtimeClientbut not theconvexHTTP client. This was flagged in past reviews as addressed, but the code still doesn't close the HTTP client.The past review comment at lines 370-373 indicated this was addressed in commit 4510d15, but the current code still shows only
realtimeClient.close(). Please verify the fix was applied:async close(): Promise<void> { + await this.convex.close(); await this.realtimeClient.close(); this.emit("close"); }
🧹 Nitpick comments (7)
packages/local/src/index.ts (2)
295-310: Container name filter pattern may match unintended containers.The docker filter
name=^${containerName}$uses regex anchors, but special characters in the sanitized container name (like hyphens) have regex meaning. While the sanitization removes most problematic chars, the pattern could still behave unexpectedly.Consider using
--filter "name=${containerName}" --format "..."combined with exact string matching in code, or escape regex special characters:const result = await runShellCommand( - `docker ps -a --filter "name=^${containerName}$" --format "{{.ID}}|{{.State}}"`, + `docker ps -a --filter "name=${containerName}" --format "{{.Names}}|{{.ID}}|{{.State}}"`, { timeout: 5000 }, ); + // Then filter for exact name match in code
650-709: Heartbeat with jitter is a good pattern for distributed clients.The ±10s jitter prevents thundering herd issues. The idle timeout and session refresh logic are well-structured. However, consider adding a maximum consecutive failure count before giving up.
+ private heartbeatFailures = 0; + private readonly MAX_HEARTBEAT_FAILURES = 5; private scheduleNextHeartbeat(): void { // ... existing code ... try { const result = (await (this.convex as any).mutation( api.localSandbox.heartbeat, // ... )) as HeartbeatResult; if (!result.success) { // ... existing handling ... } + this.heartbeatFailures = 0; // Reset on success // ... rest ... } catch (error: unknown) { + this.heartbeatFailures++; + if (this.heartbeatFailures >= this.MAX_HEARTBEAT_FAILURES) { + console.log(chalk.red(`\n❌ Too many heartbeat failures (${this.heartbeatFailures})`)); + await this.cleanup(); + process.exit(1); + } const message = error instanceof Error ? error.message : String(error); console.debug(`Heartbeat error (will retry): ${message}`); }convex/localSandbox.ts (2)
21-22: Missing environment variable check could cause silent failures.
SESSION_SIGNING_SECRETis read at module load time. If not set,signSessionwill throw butverifySessionsilently returnsfalse. Consider failing fast on module load or adding consistent error handling.const SESSION_SIGNING_SECRET = process.env.LOCAL_SANDBOX_SESSION_SECRET; const SESSION_EXPIRY_MS = 10 * 60 * 1000; +// Fail fast if secret is missing in production +if (!SESSION_SIGNING_SECRET && process.env.NODE_ENV === "production") { + throw new Error("LOCAL_SANDBOX_SESSION_SECRET environment variable is required"); +}
419-430: In-memory filter after query could miss stale connections.The heartbeat timeout check is done in-memory after fetching all "connected" status connections. This works but relies on the client calling disconnect or the cleanup job running. Consider documenting this dependency.
Add a comment clarifying the cleanup dependency:
// Check heartbeat timeout (30 seconds) + // Note: Relies on cleanupStaleConnections cron job to mark stale connections as disconnected. + // This in-memory filter handles the window between missed heartbeats and cleanup runs. const now = Date.now(); const timeout = 30000;app/components/SandboxSelector.tsx (1)
82-83: Fallback to first option when selection not found.If
valuedoesn't match any option (e.g., connection disconnected), it falls back tooptions[0](Cloud). This is reasonable but could cause silent state mismatch if the parent doesn't know the selection changed.Consider notifying the parent when the selected connection is no longer available:
- const selectedOption = options.find((opt) => opt.id === value) || options[0]; + const selectedOption = options.find((opt) => opt.id === value); + + // If selected connection no longer exists, notify parent and fall back to cloud + useEffect(() => { + if (!selectedOption && value !== "e2b" && onChange) { + onChange("e2b"); + } + }, [selectedOption, value, onChange]); + + const displayOption = selectedOption || options[0];lib/ai/tools/index.ts (1)
39-42: Type guard relies on implementation detail.The
isE2BSandboxcheck uses"jupyterUrl" in swhich is fragile - if E2B changes their API, this breaks silently. Consider using a more explicit discriminator or the type guard fromsandbox-types.ts.- // Helper to check if sandbox is E2B Sandbox - const isE2BSandbox = (s: AnySandbox | null): s is Sandbox => { - return s !== null && "jupyterUrl" in s; - }; + // Import the shared type guard for consistency + import { isE2BSandbox } from "./utils/sandbox-types";This ensures consistency with the type guard already defined in
lib/ai/tools/utils/sandbox-types.ts.lib/ai/tools/run-terminal-cmd.ts (1)
14-16: Keep timeout comments and constants in sync
MAX_COMMAND_EXECUTION_TIMEis set to 7 minutes (Line 14), and the user-facing guidance (Line 54–55) also refers to keeping execution under 7 minutes. However, the comment in the timeout handler mentions “MAX_COMMAND_EXECUTION_TIME (10min)” (Line 252), which is now stale.To avoid confusion when adjusting timeouts in the future, it would be good to update that comment to match the 7-minute constant (or derive the human-readable minutes from the constant programmatically if you want to avoid future drift).
Also applies to: 252-265
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
app/components/ChatInput.tsx(4 hunks)app/components/SandboxSelector.tsx(1 hunks)convex/localSandbox.ts(1 hunks)lib/ai/tools/index.ts(5 hunks)lib/ai/tools/run-terminal-cmd.ts(4 hunks)lib/ai/tools/utils/convex-sandbox.ts(1 hunks)packages/local/package.json(1 hunks)packages/local/src/index.ts(1 hunks)types/agent.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/local/package.json
- app/components/ChatInput.tsx
🧰 Additional context used
📓 Path-based instructions (1)
convex/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/convex_rules.mdc)
convex/**/*.{ts,tsx}: ALWAYS use the new function syntax for Convex functions with explicitargs,returns, andhandlerproperties
Usev.null()validator when returning null values from Convex functions
Usev.int64()instead of the deprecatedv.bigint()for representing signed 64-bit integers
Usev.record()for defining record types;v.map()andv.set()are not supported
Index fields must be queried in the same order they are defined; create separate indexes for different query orders
Use theId<'tableName'>TypeScript helper type from './_generated/dataModel' for strict typing of document IDs
Be strict with types around document IDs; useId<'tableName'>rather than genericstringtypes
Always useas constfor string literals in discriminated union types
Always define arrays asconst array: Array<T> = [...]with explicit type annotations
Always define records asconst record: Record<KeyType, ValueType> = {...}with explicit type annotations
UseinternalQuery,internalMutation, andinternalActionto register private functions; these are only callable by other Convex functions
Usequery,mutation, andactionto register public functions that are exposed to the public API
ALWAYS include argument and return validators for all Convex functions, includingreturns: v.null()if the function doesn't return anything
Usectx.runQueryto call a query from a query, mutation, or action
Usectx.runMutationto call a mutation from a mutation or action
Usectx.runActionto call an action from an action; only use this when crossing runtimes (e.g., V8 to Node)
When calling functions in the same file usingctx.runQuery,ctx.runMutation, orctx.runAction, specify a type annotation on the return value to work around TypeScript circularity limitations
Use theapiobject fromconvex/_generated/api.tsto call public functions registered withquery,mutation, oraction
Use theinternalobject fromconvex/_generated/api.ts...
Files:
convex/localSandbox.ts
🧠 Learnings (12)
📓 Common learnings
Learnt from: fkesheh
Repo: hackerai-tech/hackerai PR: 114
File: lib/ai/tools/utils/convex-sandbox.ts:95-145
Timestamp: 2025-11-28T01:16:26.484Z
Learning: In the local sandbox architecture (lib/ai/tools/utils/convex-sandbox.ts), background command execution semantics are handled by the local sandbox client (packages/local/src/index.ts), not by the ConvexSandbox class. When background: true, the local client immediately submits a result with empty output and PID after spawning the process, causing waitForResult() to resolve immediately and maintaining E2B-compatible non-blocking behavior.
📚 Learning: 2025-11-28T01:16:26.484Z
Learnt from: fkesheh
Repo: hackerai-tech/hackerai PR: 114
File: lib/ai/tools/utils/convex-sandbox.ts:95-145
Timestamp: 2025-11-28T01:16:26.484Z
Learning: In the local sandbox architecture (lib/ai/tools/utils/convex-sandbox.ts), background command execution semantics are handled by the local sandbox client (packages/local/src/index.ts), not by the ConvexSandbox class. When background: true, the local client immediately submits a result with empty output and PID after spawning the process, causing waitForResult() to resolve immediately and maintaining E2B-compatible non-blocking behavior.
Applied to files:
types/agent.tslib/ai/tools/run-terminal-cmd.tsconvex/localSandbox.tslib/ai/tools/index.tslib/ai/tools/utils/convex-sandbox.tspackages/local/src/index.ts
📚 Learning: 2025-11-24T20:59:07.600Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-11-24T20:59:07.600Z
Learning: Applies to convex/**/*.{ts,tsx} : Organize public query, mutation, or action functions thoughtfully within the `convex/` directory using file-based routing
Applied to files:
convex/localSandbox.tslib/ai/tools/utils/convex-sandbox.ts
📚 Learning: 2025-11-24T20:59:07.600Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-11-24T20:59:07.600Z
Learning: Applies to convex/**/*.{ts,tsx} : Do NOT use `filter` in queries; instead, define an index in the schema and use `withIndex`
Applied to files:
convex/localSandbox.ts
📚 Learning: 2025-11-24T20:59:07.600Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-11-24T20:59:07.600Z
Learning: Applies to convex/**/*.{ts,tsx} : Try to use as few calls from actions to queries and mutations as possible to minimize race condition risk
Applied to files:
convex/localSandbox.tslib/ai/tools/utils/convex-sandbox.ts
📚 Learning: 2025-11-24T20:59:07.600Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-11-24T20:59:07.600Z
Learning: Applies to convex/**/*.{ts,tsx} : When using async iteration, don't use `.collect()` or `.take(n)` on query results; instead use `for await (const row of query)` syntax
Applied to files:
convex/localSandbox.ts
📚 Learning: 2025-11-24T20:59:07.600Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-11-24T20:59:07.600Z
Learning: Applies to convex/**/*.{ts,tsx} : ALWAYS use the new function syntax for Convex functions with explicit `args`, `returns`, and `handler` properties
Applied to files:
lib/ai/tools/utils/convex-sandbox.ts
📚 Learning: 2025-11-24T20:59:07.600Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-11-24T20:59:07.600Z
Learning: Applies to convex/schema.ts : Always define schema in `convex/schema.ts` and import schema definition functions from `convex/server`
Applied to files:
lib/ai/tools/utils/convex-sandbox.ts
📚 Learning: 2025-11-24T20:59:07.600Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-11-24T20:59:07.600Z
Learning: Applies to convex/**/*.{ts,tsx} : Always add `"use node";` to the top of files containing actions that use Node.js built-in modules
Applied to files:
lib/ai/tools/utils/convex-sandbox.tspackages/local/src/index.ts
📚 Learning: 2025-11-24T20:59:07.600Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-11-24T20:59:07.600Z
Learning: Applies to convex/**/*.{ts,tsx} : Pull out shared code into helper async functions and call them directly instead of using `ctx.runAction` for same-runtime calls
Applied to files:
lib/ai/tools/utils/convex-sandbox.ts
📚 Learning: 2025-11-12T15:23:01.117Z
Learnt from: fkesheh
Repo: hackerai-tech/hackerai PR: 92
File: lib/ai/tools/utils/sandbox-file-uploader.ts:10-10
Timestamp: 2025-11-12T15:23:01.117Z
Learning: In the hackerai codebase, NEXT_PUBLIC_USE_S3_STORAGE is intentionally used in both client and server code (including server-only files like lib/ai/tools/utils/sandbox-file-uploader.ts) to ensure the frontend and backend stay in sync regarding which storage backend (S3 vs Convex) is being used. This consistency is important for file upload flows that span both environments.
Applied to files:
lib/ai/tools/utils/convex-sandbox.ts
📚 Learning: 2025-11-24T20:59:07.600Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-11-24T20:59:07.600Z
Learning: Applies to package.json : Add `types/node` to `package.json` when using any Node.js built-in modules in Convex functions
Applied to files:
packages/local/src/index.ts
🧬 Code graph analysis (6)
types/agent.ts (1)
lib/ai/tools/utils/convex-sandbox.ts (1)
ConvexSandbox(39-374)
app/components/SandboxSelector.tsx (4)
__mocks__/convex-react.ts (1)
useQuery(7-7)components/ui/tooltip.tsx (3)
Tooltip(61-61)TooltipTrigger(61-61)TooltipContent(61-61)components/ui/popover.tsx (3)
Popover(48-48)PopoverTrigger(48-48)PopoverContent(48-48)components/ui/button.tsx (1)
Button(59-59)
lib/ai/tools/run-terminal-cmd.ts (1)
lib/ai/tools/utils/retry-with-backoff.ts (1)
retryWithBackoff(52-107)
convex/localSandbox.ts (3)
convex/_generated/server.d.ts (1)
DatabaseReader(132-132)convex/tempStreams.ts (1)
validateServiceKey(4-8)convex/chats.ts (1)
validateServiceKey(6-10)
lib/ai/tools/utils/convex-sandbox.ts (1)
packages/local/src/index.ts (1)
cleanup(734-796)
packages/local/src/index.ts (1)
lib/token-utils.ts (1)
truncateOutput(184-192)
🔇 Additional comments (17)
packages/local/src/index.ts (3)
33-50: Well-implemented truncation with head/tail strategy.The 25% head + 75% tail truncation strategy is a good approach for preserving both initial context and final results/errors. The implementation correctly handles the marker length calculation.
62-146: Solid shell command execution with proper timeout and output handling.The
runShellCommandimplementation correctly:
- Uses spawn with array args to avoid shell injection in the command itself
- Implements graceful SIGTERM with SIGKILL fallback after 2 seconds
- Caps output collection to prevent memory issues
- Handles timeout state properly to avoid race conditions
476-510: Good use of real-time subscriptions for command polling.Using
convex.onUpdatewith signed sessions for command subscriptions is efficient and secure. The auth error handling gracefully defers re-authentication to the heartbeat cycle.convex/localSandbox.ts (3)
56-98: Robust session verification implementation.Good practices observed:
- Cheap expiration check before expensive crypto operations
- Signature format validation before parsing
- Using
crypto.subtle.verifyfor constant-time comparison (timing-safe)
118-154: Token generation and retrieval follows Convex best practices.The mutation correctly uses
ctx.auth.getUserIdentity(), proper index queries withwithIndex, and explicit return validators as per coding guidelines.
809-835: Cleanup mutations properly use indexes and batch limits.Good implementation:
- Uses
withIndexfor all queries (per coding guidelines)- Batches deletes with
.take(100)to avoid transaction limits- Separates concerns (stale connections vs old commands)
types/agent.ts (2)
8-11: Clean union type for sandbox abstraction.The
AnySandboxtype provides a clean abstraction for both E2B and local sandbox implementations.
33-33: Good addition of isLocalSandbox flag to ToolContext.This flag enables tools to adapt behavior based on sandbox type, which is important for features that differ between E2B and local execution.
app/components/SandboxSelector.tsx (2)
56-80: Good defensive handling for undefined connections.The optional chaining and fallback to empty array handle the loading state gracefully.
155-190: Good use of semantic button element for options.Using
<button>instead of<div>provides proper keyboard navigation and screen reader support.lib/ai/tools/index.ts (1)
68-70: isLocalSandbox logic is clear and correct.The condition properly checks that a non-e2b preference is set AND serviceKey is provided.
lib/ai/tools/utils/convex-sandbox.ts (4)
39-54: Constructor properly initializes both HTTP and real-time clients.Good separation of concerns - HTTP client for mutations, real-time client for subscriptions.
59-93: Comprehensive sandbox context for AI guidance.The context strings provide appropriate warnings for dangerous mode and helpful information about available tools in docker mode.
170-234: Well-structured result subscription with proper cleanup.The implementation correctly:
- Uses a cleanup function to prevent leaks
- Handles auth errors gracefully
- Deletes results after reading (fire-and-forget with warning on failure)
- Uses signed sessions for security
240-243: Proper shell path escaping implementation.The
escapePathhelper correctly uses single quotes with the standard'\\''escape pattern for embedded quotes, preventing shell injection via$(), backticks, and other metacharacters.lib/ai/tools/run-terminal-cmd.ts (2)
324-343: Based on my verification of the code, I can now provide the rewritten review comment:Background run normalization is safe for process termination; no hooks are lost
The concern about
terminateProcessReliablylosing the ability to terminate background processes is not valid. The design correctly handles this case:
Normalized result has no
killmethod — This is intentional, not a problem. The normalized background result{stdout, stderr, exitCode, pid?}intentionally strips the execution handle.PID is properly captured —
processIdis extracted fromexec.pid(line 373) and passed separately toterminateProcessReliably.
terminateProcessReliablyis designed for this — The function (lines 100–108) explicitly handles the case whereexecutionlacks akillmethod butpidis available:if (pid && (!execution || !execution.kill)) { await forceKillProcess(sandbox, pid);
forceKillProcessworks with both sandbox types — It usessandbox.commands.kill(pid)for E2B andkill -9 ${pid}for ConvexSandbox, both requiring only the PID.No additional preservation of the execution handle is necessary.
18-28: Based on my verification, I've confirmed the key points raised in the review comment. Let me generate the rewritten comment:
Guidance recommends portable wait patterns for local sandboxes, but timeout behavior only extends to
tail --pidcommands; also update incorrect timeout commentThe new
waitForProcessInstruction/timeoutWaitInstructionvariants correctly differentiate local vs non-local sandbox guidance. However, there are two issues:
Timeout mismatch for portable wait patterns: For local sandboxes, the guidance recommends
while kill -0 <pid> 2>/dev/null; do sleep 1; done(POSIX-portable), but the code grants the extended 7-minute timeout only to commands starting withtail --pid(line 213). Sincetail --pidis a GNU coreutils extension unavailable on BSD/macOS, AI instructions recommending portable alternatives will timeout at 60 seconds instead of 7 minutes when executed on non-Linux local sandboxes.Incorrect timeout constant comment: Line 252 states "MAX_COMMAND_EXECUTION_TIME (10min)" but the constant is defined as
7 * 60 * 1000(7 minutes, line 14).Suggested fixes:
- Generalize
isWaitCommandto also recognize common portable wait patterns (e.g.,while kill -0), or- Update local sandbox guidance to recommend only
tail --pidif Linux is guaranteed, or- Make the wait-command timeout configurable per sandbox type
- Correct the timeout constant comment on line 252 to "7 minutes"
Also applies to: 42–43 (guidance interpolation), 211–228 (timeout logic)
| const sandboxManager = | ||
| sandboxPreference && serviceKey | ||
| ? new HybridSandboxManager( | ||
| userID, | ||
| (newSandbox) => { | ||
| sandbox = newSandbox; | ||
| }, | ||
| sandboxPreference, | ||
| serviceKey, | ||
| isE2BSandbox(sandbox) ? sandbox : null, | ||
| ) | ||
| : new DefaultSandboxManager( | ||
| userID, | ||
| (newSandbox) => { | ||
| sandbox = newSandbox; | ||
| }, | ||
| isE2BSandbox(sandbox) ? sandbox : null, | ||
| ); |
There was a problem hiding this comment.
Race condition: sandbox is null during manager initialization.
isE2BSandbox(sandbox) is called during manager construction when sandbox is always null. The existing E2B sandbox is never passed to the manager constructor.
const sandboxManager =
sandboxPreference && serviceKey
? new HybridSandboxManager(
userID,
(newSandbox) => {
sandbox = newSandbox;
},
sandboxPreference,
serviceKey,
- isE2BSandbox(sandbox) ? sandbox : null,
+ null, // Existing sandbox not available at construction time
)
: new DefaultSandboxManager(
userID,
(newSandbox) => {
sandbox = newSandbox;
},
- isE2BSandbox(sandbox) ? sandbox : null,
+ null, // Existing sandbox not available at construction time
);If passing an existing sandbox is intended, it should be a parameter to createTools.
📝 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 sandboxManager = | |
| sandboxPreference && serviceKey | |
| ? new HybridSandboxManager( | |
| userID, | |
| (newSandbox) => { | |
| sandbox = newSandbox; | |
| }, | |
| sandboxPreference, | |
| serviceKey, | |
| isE2BSandbox(sandbox) ? sandbox : null, | |
| ) | |
| : new DefaultSandboxManager( | |
| userID, | |
| (newSandbox) => { | |
| sandbox = newSandbox; | |
| }, | |
| isE2BSandbox(sandbox) ? sandbox : null, | |
| ); | |
| const sandboxManager = | |
| sandboxPreference && serviceKey | |
| ? new HybridSandboxManager( | |
| userID, | |
| (newSandbox) => { | |
| sandbox = newSandbox; | |
| }, | |
| sandboxPreference, | |
| serviceKey, | |
| null, // Existing sandbox not available at construction time | |
| ) | |
| : new DefaultSandboxManager( | |
| userID, | |
| (newSandbox) => { | |
| sandbox = newSandbox; | |
| }, | |
| null, // Existing sandbox not available at construction time | |
| ); |
🤖 Prompt for AI Agents
In lib/ai/tools/index.ts around lines 45 to 62, the code calls
isE2BSandbox(sandbox) while sandbox is still null during the manager
constructor, so the existing E2B sandbox never gets passed in; compute the
existing sandbox value before constructing the manager (e.g. determine
existingSandbox = isE2BSandbox(sandbox) ? sandbox : null or accept an
existingSandbox parameter to createTools) and pass that precomputed value into
the HybridSandboxManager/DefaultSandboxManager constructors instead of calling
isE2BSandbox(sandbox) inline.
Summary by CodeRabbit
New Features
Chores
Documentation
Style/Ignore
✏️ Tip: You can customize this high-level summary in your review settings.