Skip to content

feat: auto-create GitHub repo on first sandbox creation#210

Merged
sweetmantech merged 8 commits intotestfrom
sweetmantech/myc-4191-bash-no-supabase-info-create-a-github-repo
Feb 10, 2026
Merged

feat: auto-create GitHub repo on first sandbox creation#210
sweetmantech merged 8 commits intotestfrom
sweetmantech/myc-4191-bash-no-supabase-info-create-a-github-repo

Conversation

@sweetmantech
Copy link
Contributor

@sweetmantech sweetmantech commented Feb 9, 2026

Summary: auto-create GitHub repo on first sandbox creation

Summary by CodeRabbit

  • New Features

    • Sandbox setup now starts automatically in the background during creation.
    • Sandbox responses include a setup run identifier to track initialization.
    • Setup runs are asynchronous and do not block sandbox creation.
  • Bug Fixes / Reliability

    • Failures when triggering setup or subsequent commands are logged without aborting creation.
    • If a follow-up command runs, its run identifier may replace the setup run identifier; command trigger failures clear the run identifier.

When POST /api/sandboxes creates a sandbox for an account without a
GitHub repo, automatically creates a private repo in the recoupable
org and clones it into the sandbox. Returns github_repo in response.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@vercel
Copy link
Contributor

vercel bot commented Feb 9, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
recoup-api Ready Ready Preview Feb 10, 2026 4:45pm

Request Review

@coderabbitai
Copy link

coderabbitai bot commented Feb 9, 2026

📝 Walkthrough

Walkthrough

Adds a fire-and-forget setup-sandbox orchestration to sandbox creation: a new trigger helper dispatches a "setup-sandbox" task and the post handler captures its runId and includes it in the creation response; failures are logged and do not abort the request flow. (≤50 words)

Changes

Cohort / File(s) Summary
Setup Sandbox Trigger
lib/trigger/triggerSetupSandbox.ts
New module exporting triggerSetupSandbox(payload) which triggers a "setup-sandbox" task via the Trigger SDK using { sandboxId, accountId } and returns the task handle (setupRunId).
Sandbox Creation Handler
lib/sandbox/createSandboxPostHandler.ts
Invokes triggerSetupSandbox after sandbox creation as fire-and-forget, captures setupRunId and includes it in the sandbox response when present; retains existing command-run flow (command run may override runId), and logs errors without aborting the overall request.

Sequence Diagram

sequenceDiagram
    participant Client
    participant PostHandler as createSandboxPostHandler
    participant TriggerFn as triggerSetupSandbox
    participant TriggerSDK as Trigger SDK
    participant SetupTask as Setup-Sandbox Task
    participant CommandRunner as Command Run (optional)

    Client->>PostHandler: POST /sandboxes (create)
    PostHandler->>PostHandler: create sandbox record
    PostHandler->>TriggerFn: triggerSetupSandbox({sandboxId, accountId})
    TriggerFn->>TriggerSDK: request "setup-sandbox" task
    TriggerSDK-->>TriggerFn: return task handle (setupRunId)
    TriggerFn-->>PostHandler: setupRunId (fire-and-forget)
    alt command provided
        PostHandler->>CommandRunner: trigger command run
        CommandRunner-->>PostHandler: command run handle (overrides runId)
    end
    PostHandler-->>Client: respond with sandbox (runId if present)
    
    par Async setup execution
        TriggerSDK->>SetupTask: execute setup-sandbox
        SetupTask-->>TriggerSDK: complete / error (logged)
    end
Loading

Estimated Code Review Effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🏖️ Sandbox spun up, a spark in the sand,
A silent trigger sent off by hand,
RunId captured, then maybe replaced,
Errors just logged, no flow erased,
Async calm hums—automation grand. ✨

🚥 Pre-merge checks | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Solid & Clean Code ⚠️ Warning The implementation violates Single Responsibility Principle by accumulating orchestration logic in the handler, duplicates error handling patterns, and discards valid setup runId on command failure, compromising client tracking capabilities. Extract task orchestration into a dedicated abstraction, consolidate error handling patterns, and preserve setup runId as fallback when command execution fails to maintain consistent response semantics.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch sweetmantech/myc-4191-bash-no-supabase-info-create-a-github-repo

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

🤖 Fix all issues with AI agents
In `@lib/github/cloneRepoIntoSandbox.ts`:
- Around line 52-75: The fetchResult from sandbox.runCommand (git fetch origin)
is ignored when fetchResult.exitCode !== 0, causing cloneRepoIntoSandbox to
continue and return true even on fetch/network failure; update
cloneRepoIntoSandbox to check fetchResult.exitCode and on non-zero either log
the error (include fetchResult.stderr/stdout) and return false, or at minimum
return false so the caller can distinguish a failed fetch from an empty repo,
referencing the variables fetchResult and the subsequent refCheck/checkoutResult
logic to ensure early exit on fetch failure.
- Around line 30-34: The current approach embeds GITHUB_TOKEN into repoUrl
(constructed from githubRepo and githubToken) which leaves credentials in the
sandbox git config; update cloneRepoIntoSandbox (the code that builds repoUrl
and performs the fetch/clone) to avoid leaving the token: use a credential
helper or temporary authenticated remote only for the fetch, then immediately
replace the remote URL in the sandbox .git/config with the plain githubRepo (or
remove the remote and re-add it without credentials) after the fetch completes
(or on any error), ensuring no token remains in git config or refs; also ensure
any subprocess that used the token does not write it to logs or files.

In `@lib/github/createGithubRepo.ts`:
- Around line 24-25: The repoName construction (const repoName =
`${sanitizedName}-${accountId}`) can exceed GitHub's 100-character repo name
limit when accountId is a UUID; update createGithubRepo.ts to enforce truncation
of sanitizedName so that sanitizedName.length + 1 + accountId.length <= 100, or
derive the truncated base using sanitizeRepoName (or enhance
sanitizeRepoName.ts) to accept a maxLength parameter; ensure the final repoName
generation uses the truncated sanitizedName, preserves the hyphen and accountId,
and guarantees the 100-char max.
- Around line 46-70: In createGithubRepo, the fallback GET for an existing repo
uses existingResponse but if that fetch fails the code falls through and logs
the original creation response body (response.text()), which is misleading;
update the error handling so when response.status === 422 and
existingResponse.ok is false you capture and include the failing
existingResponse status and body (existingResponse.text()) in the error log
(along with the original response info) so the logged message reflects the real
failure for the GET of repoName.

In `@lib/github/ensureGithubRepo.ts`:
- Around line 24-27: The code in ensureGithubRepo (file
lib/github/ensureGithubRepo.ts) currently ignores the boolean result of
cloneRepoIntoSandbox when called for an existing repo (existingRepo) and after
creating a new repo, returning the repo URL even if cloning failed; update
ensureGithubRepo to check the return value of cloneRepoIntoSandbox and handle
failure by either logging a clear warning (include the repo URL and sandbox id)
and returning an error/nullable result or by throwing an error to propagate the
failure to the caller; specifically, after both calls to
cloneRepoIntoSandbox(reference existingRepo and the post-create clone), if the
call returns false then call the available logger (or console.warn if no logger
is available) with a descriptive message and do not return a success github_repo
value—instead return null or rethrow so the caller can detect the failed clone.

In `@lib/github/sanitizeRepoName.ts`:
- Around line 9-15: sanitizeRepoName currently returns an unchecked string and
can exceed GitHub's 100-char limit once createGithubRepo appends `-${accountId}`
(UUID ~37 chars); update the logic so the portion produced by sanitizeRepoName
is truncated to ensure final repo name <=100 chars: either add an optional
maxLength param to sanitizeRepoName or perform truncation in createGithubRepo
before appending `-${accountId}`, calculating maxPrefix = 100 -
(accountId.length + 1) and slicing the sanitized value to that length (preserve
existing sanitization, then truncate, then fallback to "account" if empty).

In `@lib/sandbox/createSandboxPostHandler.ts`:
- Around line 44-50: The ensureGithubRepo call is causing sequential latency;
change its usage so it runs concurrently or fire-and-forget instead of awaiting
it inline. Specifically, in createSandboxPostHandler replace the awaited call to
ensureGithubRepo with starting the promise without blocking the response (e.g.,
call ensureGithubRepo(validated.accountId, sandbox) and store the promise in a
variable or not at all), and attach a .catch to log errors; alternatively, if
github_repo is required for certain responses, run ensureGithubRepo concurrently
with triggerRunSandboxCommand (start both promises and await only what the
response must include). Update references around the githubRepo variable,
ensureGithubRepo, and triggerRunSandboxCommand to reflect the
non-blocking/concurrent handling and preserve error logging.
🧹 Nitpick comments (5)
lib/github/cloneRepoIntoSandbox.ts (2)

31-34: Fragile URL replacement — silently fails for non-standard GitHub URLs.

githubRepo.replace("https://github.com/", ...) only works for URLs starting with exactly https://github.com/. An Enterprise or trailing-slash variant would pass through unauthenticated, causing a silent auth failure at fetch time. A URL-based approach or at least a validation check would be more robust.


77-85: Unchecked git config commands — inconsistent error handling.

The git config calls on lines 78–85 don't check exitCode, unlike every other runCommand in this function. For consistency and to surface unexpected failures, consider checking or at least logging non-zero exits.

lib/sandbox/createSandboxPostHandler.ts (1)

69-81: github_repo is at the top level of the response, outside the sandboxes array.

The sandboxes array contains per-sandbox data, but github_repo sits alongside it at the root. If the API evolves to support batch sandbox creation, this structure may not scale. Consider nesting github_repo inside the sandbox object for consistency, or document that it's intentionally a top-level account-scoped field.

lib/github/ensureGithubRepo.ts (1)

16-51: Concurrent sandbox creation for the same account could trigger duplicate GitHub API calls.

If two POST /api/sandboxes requests arrive simultaneously for the same account with no existing repo, both will pass the existingRepo check (line 22), both will call createGithubRepo, and one will hit the 422 fallback. This is safe due to the idempotent fallback — just noting it for awareness. If this endpoint sees meaningful concurrency per-account, a distributed lock or check-and-set pattern would eliminate redundant API calls.

lib/github/createGithubRepo.ts (1)

28-39: No timeout on GitHub API requests — could block indefinitely.

Both fetch calls (creation on line 28 and fallback on line 48) have no AbortSignal timeout. If the GitHub API is slow or unresponsive, this will block the sandbox creation response indefinitely. Consider using AbortSignal.timeout() to cap the wait.

Proposed fix
     const response = await fetch(`https://api.github.com/orgs/${GITHUB_ORG}/repos`, {
       method: "POST",
       headers: {
         Accept: "application/vnd.github+json",
         Authorization: `Bearer ${token}`,
         "X-GitHub-Api-Version": "2022-11-28",
       },
       body: JSON.stringify({
         name: repoName,
         private: true,
       }),
+      signal: AbortSignal.timeout(10_000),
     });

Apply similarly to the fallback fetch on line 48.

Comment on lines 52 to 75
// Fetch from remote
const fetchResult = await sandbox.runCommand({
cmd: "git",
args: ["fetch", "origin"],
});

if (fetchResult.exitCode === 0) {
// Check if origin/main exists (won't for empty repos)
const refCheck = await sandbox.runCommand({
cmd: "git",
args: ["rev-parse", "--verify", "origin/main"],
});

if (refCheck.exitCode === 0) {
const checkoutResult = await sandbox.runCommand({
cmd: "git",
args: ["checkout", "-B", "main", "origin/main"],
});
if (checkoutResult.exitCode !== 0) {
console.error("Failed to checkout main branch");
return false;
}
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Fetch failure silently falls through — function returns true even when no content was pulled.

When fetchResult.exitCode !== 0 (line 58), execution skips the checkout block entirely and falls through to configure git user (line 77), ultimately returning true. The caller interprets true as "repo is cloned," which is misleading when the fetch actually failed.

If fetch fails, either return false or at least log a warning so the caller can distinguish "empty repo initialized" from "network/auth failure."

Proposed fix
   if (fetchResult.exitCode === 0) {
     // Check if origin/main exists (won't for empty repos)
     const refCheck = await sandbox.runCommand({
       cmd: "git",
       args: ["rev-parse", "--verify", "origin/main"],
     });

     if (refCheck.exitCode === 0) {
       const checkoutResult = await sandbox.runCommand({
         cmd: "git",
         args: ["checkout", "-B", "main", "origin/main"],
       });
       if (checkoutResult.exitCode !== 0) {
         console.error("Failed to checkout main branch");
         return false;
       }
     }
+  } else {
+    console.error("Failed to fetch from remote origin");
+    return false;
   }
📝 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.

Suggested change
// Fetch from remote
const fetchResult = await sandbox.runCommand({
cmd: "git",
args: ["fetch", "origin"],
});
if (fetchResult.exitCode === 0) {
// Check if origin/main exists (won't for empty repos)
const refCheck = await sandbox.runCommand({
cmd: "git",
args: ["rev-parse", "--verify", "origin/main"],
});
if (refCheck.exitCode === 0) {
const checkoutResult = await sandbox.runCommand({
cmd: "git",
args: ["checkout", "-B", "main", "origin/main"],
});
if (checkoutResult.exitCode !== 0) {
console.error("Failed to checkout main branch");
return false;
}
}
}
// Fetch from remote
const fetchResult = await sandbox.runCommand({
cmd: "git",
args: ["fetch", "origin"],
});
if (fetchResult.exitCode === 0) {
// Check if origin/main exists (won't for empty repos)
const refCheck = await sandbox.runCommand({
cmd: "git",
args: ["rev-parse", "--verify", "origin/main"],
});
if (refCheck.exitCode === 0) {
const checkoutResult = await sandbox.runCommand({
cmd: "git",
args: ["checkout", "-B", "main", "origin/main"],
});
if (checkoutResult.exitCode !== 0) {
console.error("Failed to checkout main branch");
return false;
}
}
} else {
console.error("Failed to fetch from remote origin");
return false;
}
🤖 Prompt for AI Agents
In `@lib/github/cloneRepoIntoSandbox.ts` around lines 52 - 75, The fetchResult
from sandbox.runCommand (git fetch origin) is ignored when fetchResult.exitCode
!== 0, causing cloneRepoIntoSandbox to continue and return true even on
fetch/network failure; update cloneRepoIntoSandbox to check fetchResult.exitCode
and on non-zero either log the error (include fetchResult.stderr/stdout) and
return false, or at minimum return false so the caller can distinguish a failed
fetch from an empty repo, referencing the variables fetchResult and the
subsequent refCheck/checkoutResult logic to ensure early exit on fetch failure.

Comment on lines 24 to 25
const sanitizedName = sanitizeRepoName(accountName);
const repoName = `${sanitizedName}-${accountId}`;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Repo name construction should account for GitHub's 100-character limit.

repoName is ${sanitizedName}-${accountId}. With a UUID accountId (36 chars) plus the hyphen, only 63 characters remain for the sanitized name. This ties into the truncation concern in sanitizeRepoName.ts — if that's addressed there, this line is fine. Flagging for cross-reference.

🤖 Prompt for AI Agents
In `@lib/github/createGithubRepo.ts` around lines 24 - 25, The repoName
construction (const repoName = `${sanitizedName}-${accountId}`) can exceed
GitHub's 100-character repo name limit when accountId is a UUID; update
createGithubRepo.ts to enforce truncation of sanitizedName so that
sanitizedName.length + 1 + accountId.length <= 100, or derive the truncated base
using sanitizeRepoName (or enhance sanitizeRepoName.ts) to accept a maxLength
parameter; ensure the final repoName generation uses the truncated
sanitizedName, preserves the hyphen and accountId, and guarantees the 100-char
max.

Comment on lines 46 to 70
// 422 means the repo already exists - fetch the existing one
if (response.status === 422) {
const existingResponse = await fetch(
`https://api.github.com/repos/${GITHUB_ORG}/${repoName}`,
{
headers: {
Accept: "application/vnd.github+json",
Authorization: `Bearer ${token}`,
"X-GitHub-Api-Version": "2022-11-28",
},
},
);

if (existingResponse.ok) {
const data = (await existingResponse.json()) as { html_url: string };
return data.html_url;
}
}

const errorText = await response.text();
console.error("Failed to create GitHub repo", {
status: response.status,
error: errorText,
});
return undefined;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

When the 422 fallback fetch fails, the logged error is misleading.

If response.status === 422 (line 47) but existingResponse.ok is false (line 59), execution falls through to line 65, which logs response.text() — the body of the original 422 creation response, not the failed GET. This obscures the real failure (e.g., a 404 or 403 on the existing repo lookup).

Proposed fix — log the actual failure
       if (existingResponse.ok) {
         const data = (await existingResponse.json()) as { html_url: string };
         return data.html_url;
       }
+
+      const existingErrorText = await existingResponse.text();
+      console.error("Repo exists but failed to fetch it", {
+        status: existingResponse.status,
+        error: existingErrorText,
+      });
+      return undefined;
     }

     const errorText = await response.text();
🤖 Prompt for AI Agents
In `@lib/github/createGithubRepo.ts` around lines 46 - 70, In createGithubRepo,
the fallback GET for an existing repo uses existingResponse but if that fetch
fails the code falls through and logs the original creation response body
(response.text()), which is misleading; update the error handling so when
response.status === 422 and existingResponse.ok is false you capture and include
the failing existingResponse status and body (existingResponse.text()) in the
error log (along with the original response info) so the logged message reflects
the real failure for the GET of repoName.

Comment on lines 24 to 27
if (existingRepo) {
await cloneRepoIntoSandbox(sandbox, existingRepo);
return existingRepo;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Clone failure is silently swallowed — function returns the repo URL regardless.

cloneRepoIntoSandbox returns false on failure, but the return value is discarded on both line 25 and line 48. The caller receives a valid repoUrl and has no indication the clone didn't actually succeed. This means the sandbox may not have the repo content, yet the response reports github_repo as if everything worked.

At minimum, log a warning when cloneRepoIntoSandbox returns false. Ideally, propagate the failure so the caller can make an informed decision.

Proposed fix
   if (existingRepo) {
-    await cloneRepoIntoSandbox(sandbox, existingRepo);
+    const cloned = await cloneRepoIntoSandbox(sandbox, existingRepo);
+    if (!cloned) {
+      console.error("Failed to clone existing repo into sandbox", { existingRepo });
+    }
     return existingRepo;
   }

And similarly for line 48:

   // Clone into sandbox
-  await cloneRepoIntoSandbox(sandbox, repoUrl);
+  const cloned = await cloneRepoIntoSandbox(sandbox, repoUrl);
+  if (!cloned) {
+    console.error("Failed to clone new repo into sandbox", { repoUrl });
+  }
📝 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.

Suggested change
if (existingRepo) {
await cloneRepoIntoSandbox(sandbox, existingRepo);
return existingRepo;
}
if (existingRepo) {
const cloned = await cloneRepoIntoSandbox(sandbox, existingRepo);
if (!cloned) {
console.error("Failed to clone existing repo into sandbox", { existingRepo });
}
return existingRepo;
}
🤖 Prompt for AI Agents
In `@lib/github/ensureGithubRepo.ts` around lines 24 - 27, The code in
ensureGithubRepo (file lib/github/ensureGithubRepo.ts) currently ignores the
boolean result of cloneRepoIntoSandbox when called for an existing repo
(existingRepo) and after creating a new repo, returning the repo URL even if
cloning failed; update ensureGithubRepo to check the return value of
cloneRepoIntoSandbox and handle failure by either logging a clear warning
(include the repo URL and sandbox id) and returning an error/nullable result or
by throwing an error to propagate the failure to the caller; specifically, after
both calls to cloneRepoIntoSandbox(reference existingRepo and the post-create
clone), if the call returns false then call the available logger (or
console.warn if no logger is available) with a descriptive message and do not
return a success github_repo value—instead return null or rethrow so the caller
can detect the failed clone.

Replace synchronous ensureGithubRepo call in POST /api/sandboxes with
fire-and-forget triggerSetupSandbox. This lets the endpoint return the
sandboxId faster while repo creation happens in a background task.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
GitHub repo setup now happens entirely via defense-in-depth in
runSandboxCommandTask's ensureGithubRepo call. Remove the fire-and-forget
trigger from the POST handler and its associated tests.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
These files are no longer imported anywhere — repo setup now lives
entirely in Recoup-Tasks. Removed:
- lib/github/ensureGithubRepo.ts
- lib/github/cloneRepoIntoSandbox.ts
- lib/github/createGithubRepo.ts
- lib/supabase/account_snapshots/updateAccountSnapshotGithubRepo.ts
- lib/trigger/triggerSetupSandbox.ts

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Wire up triggerSetupSandbox in createSandboxPostHandler so every new
sandbox fires the setup-sandbox Trigger.dev task (creates GitHub repo,
clones, snapshots). Fire-and-forget with graceful error handling.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Revert the SandboxCreateResult wrapper (sandbox + response) back to
the original flat SandboxCreatedResponse return. Delete unused
sanitizeRepoName. These changes are unrelated to the setup-sandbox
trigger goal.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When a command is provided, runId is the command task ID.
When no command, runId is the setup task ID.
Removes setupRunId for a consistent response shape.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@sweetmantech sweetmantech merged commit bf02fbb into test Feb 10, 2026
2 of 3 checks passed
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)
lib/sandbox/createSandboxPostHandler.ts (1)

11-21: ⚠️ Potential issue | 🟡 Minor

Stale JSDoc — runId semantics have changed.

Line 21 states runId is included "only if command was provided," but the new logic also returns runId from the setup task when no command is given. Update the docblock to reflect the current behavior.

📝 Suggested fix
- * `@returns` A NextResponse with sandbox creation result (includes runId only if command was provided)
+ * `@returns` A NextResponse with sandbox creation result (includes runId from setup task, or from command task if a command was provided)
🤖 Fix all issues with AI agents
In `@lib/sandbox/createSandboxPostHandler.ts`:
- Around line 67-70: The catch block that handles errors from
triggerRunSandboxCommand currently resets runId to undefined, which discards any
valid runId returned earlier by triggerSetupSandbox; remove the assignment runId
= undefined from the catch so that if triggerRunSandboxCommand fails the
previously obtained runId is preserved and can be returned to the client for
monitoring; ensure the catch still logs the triggerError (console.error("Failed
to trigger run-sandbox-command task:", triggerError)) but does not modify the
runId variable.
🧹 Nitpick comments (1)
lib/sandbox/createSandboxPostHandler.ts (1)

44-54: Comment says "fire-and-forget" but the call is awaited.

triggerSetupSandbox is awaited to capture setupHandle.id, so this is not fire-and-forget — it blocks the response until the trigger call resolves. The comment is misleading. If the intent is truly fire-and-forget, drop the await and use .then()/.catch(). If the intent is to capture the runId (which it clearly is), just fix the comment.

📝 Suggested comment fix
-    // Trigger the setup-sandbox task (fire-and-forget)
+    // Trigger the setup-sandbox task and capture its runId

Comment on lines 67 to 70
} catch (triggerError) {
console.error("Failed to trigger run-sandbox-command task:", triggerError);
runId = undefined;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Resetting runId to undefined on command failure discards a valid setup runId.

If triggerSetupSandbox succeeded and provided a runId, but triggerRunSandboxCommand then fails, the client loses all tracking information. The setup task is still running — returning its runId would let the client at least monitor that. Consider omitting the runId = undefined reset so the setup runId falls through as a useful fallback.

🐛 Proposed fix
      } catch (triggerError) {
        console.error("Failed to trigger run-sandbox-command task:", triggerError);
-        runId = undefined;
+        // Keep the setup runId so the client can still track sandbox initialization
      }
📝 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.

Suggested change
} catch (triggerError) {
console.error("Failed to trigger run-sandbox-command task:", triggerError);
runId = undefined;
}
} catch (triggerError) {
console.error("Failed to trigger run-sandbox-command task:", triggerError);
// Keep the setup runId so the client can still track sandbox initialization
}
🤖 Prompt for AI Agents
In `@lib/sandbox/createSandboxPostHandler.ts` around lines 67 - 70, The catch
block that handles errors from triggerRunSandboxCommand currently resets runId
to undefined, which discards any valid runId returned earlier by
triggerSetupSandbox; remove the assignment runId = undefined from the catch so
that if triggerRunSandboxCommand fails the previously obtained runId is
preserved and can be returned to the client for monitoring; ensure the catch
still logs the triggerError (console.error("Failed to trigger
run-sandbox-command task:", triggerError)) but does not modify the runId
variable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant