feat: add run_sandbox_command and get_task_run_status MCP tools#224
Conversation
Extract shared sandbox creation logic into processCreateSandbox.ts for DRY reuse between the POST /api/sandboxes handler and the new MCP tools. Co-Authored-By: Claude Opus 4.6 <[email protected]>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughAdds sandbox and task-status MCP tools and centralizes sandbox creation into a new processCreateSandbox module; the POST /api/sandboxes handler now delegates to that module and tool registrations call the new sandbox/task registration functions. All changes are internal wiring and new registrations. Changes
Sequence DiagramsequenceDiagram
participant Client
participant MCPServer
participant Handler as registerRunSandboxCommandTool
participant ProcessCreate as processCreateSandbox
participant SnapshotDB as AccountSnapshotDB
participant SandboxSvc as SandboxService
participant TaskQueue as TaskQueue
Client->>MCPServer: run_sandbox_command(command, args, cwd)
MCPServer->>Handler: invoke handler (with extra.authInfo)
Handler->>Handler: validate input (Zod) & extract accountId
alt missing accountId
Handler-->>MCPServer: return auth error
MCPServer-->>Client: error
else accountId present
Handler->>ProcessCreate: processCreateSandbox(accountId, command, args, cwd)
ProcessCreate->>SnapshotDB: fetch latest snapshot for account
SnapshotDB-->>ProcessCreate: snapshot (or none)
ProcessCreate->>SandboxSvc: create sandbox (from snapshot or fresh)
SandboxSvc-->>ProcessCreate: SandboxCreatedResponse
ProcessCreate->>SnapshotDB: insert account↔sandbox mapping
alt command provided
ProcessCreate->>TaskQueue: enqueue run-sandbox-command task
TaskQueue-->>ProcessCreate: runId (or error logged)
end
ProcessCreate-->>Handler: SandboxCreatedResponse (+ optional runId)
Handler-->>MCPServer: success result
MCPServer-->>Client: sandbox info (+ runId)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@lib/mcp/tools/sandbox/registerGetTaskRunStatusTool.ts`:
- Around line 38-43: The registerGetTaskRunStatusTool extracts accountId but
doesn't pass it to retrieveTaskRun, allowing cross-account access; update
registerGetTaskRunStatusTool to call retrieveTaskRun(args.runId, accountId) and
change retrieveTaskRun's signature to accept (runId, accountId) and enforce
account-scoped lookup (e.g., include accountId in the query/filter). Update any
other callers of retrieveTaskRun to the new two-arg signature and ensure
tests/usage reflect the account-scoped authorization.
In `@lib/mcp/tools/sandbox/registerRunSandboxCommandTool.ts`:
- Around line 31-32: The code currently extracts accountId directly from
extra.authInfo (extra.authInfo as McpAuthInfo and authInfo?.extra?.accountId) in
registerRunSandboxCommandTool; replace that manual extraction by invoking the
centralized n() resolver (imported from lib/mcp/resolveAccountId.ts) with the
authInfo and an accountIdOverride (undefined or derived from schema), await its
result, check for error and return getToolResultError(error) if present, and
then use the returned accountId; this ensures registerRunSandboxCommandTool uses
the same validation/override logic as other tools like
registerCreateNewArtistTool and registerUpdatePulseTool.
In `@lib/sandbox/processCreateSandbox.ts`:
- Around line 36-39: The call to insertAccountSandbox in processCreateSandbox
silently ignores its result so a created sandbox can become orphaned if the DB
insert fails; update processCreateSandbox to capture the return (e.g., const res
= await insertAccountSandbox(...)), check res.error (or equivalent) and if
present either throw an error or log and perform cleanup (delete the created
sandbox using result.sandboxId) to avoid orphaning; ensure the error path
includes the sandbox id and accountId in the message and that any thrown error
bubbles up from processCreateSandbox.
🧹 Nitpick comments (2)
lib/sandbox/processCreateSandbox.ts (1)
6-13: Consider exporting the input/result types for consumers.
ProcessCreateSandboxInputandProcessCreateSandboxResultare only accessible within this module. If the MCP tool or handler tests need to reference these types, they'll need to be exported. This is minor now but good hygiene for a shared utility.lib/sandbox/createSandboxPostHandler.ts (1)
40-45: All errors map to HTTP 400 — consider 500 for internal failures.
processCreateSandboxcan fail due to sandbox service errors or database issues, which are server-side faults (5xx), not client errors (4xx). A blanket400may mislead API consumers and monitoring systems.
| try { | ||
| const result = await retrieveTaskRun(args.runId); | ||
|
|
||
| if (!result) { | ||
| return getToolResultError(`Task run with ID "${args.runId}" not found.`); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n lib/mcp/tools/sandbox/registerGetTaskRunStatusTool.tsRepository: recoupable/api
Length of output: 2375
🏁 Script executed:
rg -n "retrieveTaskRun" --type=ts -A10 -B2Repository: recoupable/api
Length of output: 28433
🏁 Script executed:
fd "\.ts$" lib/mcp/tools/sandbox -type fRepository: recoupable/api
Length of output: 228
🏁 Script executed:
fd -e ts lib/mcp/tools | head -20Repository: recoupable/api
Length of output: 415
🏁 Script executed:
cat -n lib/mcp/tools/sandbox/__tests__/registerGetTaskRunStatusTool.test.ts | head -60Repository: recoupable/api
Length of output: 2298
🏁 Script executed:
rg -n "accountId" lib/mcp/tools --type=ts -A2 -B2 | head -100Repository: recoupable/api
Length of output: 8285
🏁 Script executed:
cat -n lib/trigger/retrieveTaskRun.tsRepository: recoupable/api
Length of output: 2975
🏁 Script executed:
rg -n "accountId" lib/trigger --type=ts -B2 -A5 | head -80Repository: recoupable/api
Length of output: 4331
🏁 Script executed:
rg -n "retrieveTaskRun" lib/tasks --type=ts -B5 -A10Repository: recoupable/api
Length of output: 10797
🏁 Script executed:
rg -n "Trigger" lib --type=ts | grep -i "config\|auth\|token\|credential" | head -20Repository: recoupable/api
Length of output: 40
🏁 Script executed:
cat -n lib/trigger/retrieveTaskRun.tsRepository: recoupable/api
Length of output: 2975
🏁 Script executed:
rg -n "@trigger.dev" lib --type=ts -B2 -A2 | head -50Repository: recoupable/api
Length of output: 2880
Pass accountId to retrieveTaskRun() to enforce account-scoped authorization.
The tool verifies authentication but doesn't scope the task run query to the authenticated account. Unlike other MCP tools in the codebase (e.g., registerRunSandboxCommandTool, registerGetPulsesTool), the accountId is extracted but never used. Update retrieveTaskRun() to accept an accountId parameter and apply it during retrieval, ensuring each account can only access their own task runs.
🤖 Prompt for AI Agents
In `@lib/mcp/tools/sandbox/registerGetTaskRunStatusTool.ts` around lines 38 - 43,
The registerGetTaskRunStatusTool extracts accountId but doesn't pass it to
retrieveTaskRun, allowing cross-account access; update
registerGetTaskRunStatusTool to call retrieveTaskRun(args.runId, accountId) and
change retrieveTaskRun's signature to accept (runId, accountId) and enforce
account-scoped lookup (e.g., include accountId in the query/filter). Update any
other callers of retrieveTaskRun to the new two-arg signature and ensure
tests/usage reflect the account-scoped authorization.
| await insertAccountSandbox({ | ||
| account_id: accountId, | ||
| sandbox_id: result.sandboxId, | ||
| }); |
There was a problem hiding this comment.
insertAccountSandbox error is silently ignored — orphaned sandbox risk.
The return value of insertAccountSandbox includes an error field (per insertAccountSandbox.ts), but it's never checked here. If the insert fails, you've created a sandbox that isn't associated with the account — it becomes orphaned and unmanageable. At minimum, throw or log on failure.
🛡️ Proposed fix
- await insertAccountSandbox({
+ const { error: insertError } = await insertAccountSandbox({
account_id: accountId,
sandbox_id: result.sandboxId,
});
+
+ if (insertError) {
+ console.error("Failed to associate sandbox with account:", insertError);
+ throw new Error("Failed to save sandbox association");
+ }📝 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.
| await insertAccountSandbox({ | |
| account_id: accountId, | |
| sandbox_id: result.sandboxId, | |
| }); | |
| const { error: insertError } = await insertAccountSandbox({ | |
| account_id: accountId, | |
| sandbox_id: result.sandboxId, | |
| }); | |
| if (insertError) { | |
| console.error("Failed to associate sandbox with account:", insertError); | |
| throw new Error("Failed to save sandbox association"); | |
| } |
🤖 Prompt for AI Agents
In `@lib/sandbox/processCreateSandbox.ts` around lines 36 - 39, The call to
insertAccountSandbox in processCreateSandbox silently ignores its result so a
created sandbox can become orphaned if the DB insert fails; update
processCreateSandbox to capture the return (e.g., const res = await
insertAccountSandbox(...)), check res.error (or equivalent) and if present
either throw an error or log and perform cleanup (delete the created sandbox
using result.sandboxId) to avoid orphaning; ensure the error path includes the
sandbox id and accountId in the message and that any thrown error bubbles up
from processCreateSandbox.
The tool is general-purpose (works for any Trigger.dev run), not sandbox-specific, so it belongs with the other task tools. Co-Authored-By: Claude Opus 4.6 <[email protected]>
Replace manual authInfo extraction with the centralized resolveAccountId resolver, consistent with registerCreateNewArtistTool and registerUpdatePulseTool. Co-Authored-By: Claude Opus 4.6 <[email protected]>
Add explicit warning against manual authInfo extraction and include the full error-handling pattern with both error and null-accountId checks. Co-Authored-By: Claude Opus 4.6 <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
Before applying any fix, first verify the finding against the current code and
decide whether a code change is actually needed. If the finding is not valid or
no change is required, do not modify code for that item and briefly explain why
it was skipped.
In `@lib/mcp/tools/tasks/registerGetTaskRunStatusTool.ts`:
- Around line 29-30: The code manually extracts accountId by casting
extra.authInfo to McpAuthInfo and reading authInfo.extra.accountId; replace this
with a call to resolveAccountId(extra.authInfo) so the shared helper handles
resolution and keeps behavior consistent. Update registerGetTaskRunStatusTool to
import/use resolveAccountId and remove the manual cast/dig pattern (references:
extra.authInfo, McpAuthInfo, resolveAccountId, registerGetTaskRunStatusTool) so
account IDs are derived via the helper used elsewhere.
Replace manual authInfo extraction with the centralized resolveAccountId resolver, consistent with all other MCP tools. Co-Authored-By: Claude Opus 4.6 <[email protected]>
Co-Authored-By: Claude Opus 4.6 <[email protected]>
Replace duplicate ProcessCreateSandboxInput interface with the existing SandboxBody type from validateSandboxBody. Co-Authored-By: Claude Opus 4.6 <[email protected]>
SandboxBody includes AuthContext fields (orgId, authToken) that MCP tool callers don't have. Pick only the fields the function actually uses. Co-Authored-By: Claude Opus 4.6 <[email protected]>
1 participant
Summary
processCreateSandbox.tsfor DRY reuse between REST handler and MCP toolsrun_sandbox_commandMCP tool that creates a sandbox and runs a command in itget_task_run_statusMCP tool that retrieves Trigger.dev task run status by run IDcreateSandboxPostHandler.tsto use the sharedprocessCreateSandbox()functionTest plan
processCreateSandbox(creation with/without command, snapshot usage, error handling)registerRunSandboxCommandTool(registration, auth, success, error)registerGetTaskRunStatusTool(registration, auth, success, not found, error)createSandboxPostHandlertests still pass after refactor🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements