feat: inline setup for fresh sandbox onboarding#252
Closed
sweetmantech wants to merge 4 commits intotestfrom
Closed
feat: inline setup for fresh sandbox onboarding#252sweetmantech wants to merge 4 commits intotestfrom
sweetmantech wants to merge 4 commits intotestfrom
Conversation
…ming
When a brand new account triggers sandbox_prompt, the sandbox is created fresh
(no snapshot). Previously this resulted in OpenClaw failing because nothing
was set up. Now promptSandboxStreaming detects fresh sandboxes (created=true,
fromSnapshot=false) and runs the full setup pipeline inline before executing
the user's prompt.
Setup pipeline (mirrors runSandboxCommandTask):
1. Install OpenClaw CLI
2. Onboard + configure OpenClaw with env vars + start gateway
3. Create/clone GitHub repo
4. Write README
5. Create/clone org repos
6. Install skills + run setup-sandbox + setup-artist
After prompt completes, pushes to GitHub and snapshots so subsequent calls
use the fast snapshot path.
New files:
- api/lib/sandbox/setup/ - DRY setup functions with dependency injection
- api/lib/github/{sanitizeRepoName,getExistingGithubRepo,createGithubRepo,createOrgGithubRepo}.ts
Modified files:
- createSandboxFromSnapshot returns { sandbox, fromSnapshot }
- getOrCreateSandbox propagates fromSnapshot
- promptSandboxStreaming runs inline setup + post-prompt snapshot
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds GitHub repo utilities and a generator-driven sandbox setup pipeline that installs/configures OpenClaw, creates/clones repos (with tokenized remotes and submodules), pushes sandbox content to GitHub, and propagates snapshot origin metadata. Changes
Sequence DiagramsequenceDiagram
participant User
participant Prompt as promptSandboxStreaming
participant SetupGen as setupFreshSandbox
participant Sandbox
participant OpenClaw
participant GitHub
participant OC_Agent as OpenClawAgent
User->>Prompt: submit prompt (account)
Prompt->>SetupGen: start generator if isFreshSandbox
SetupGen->>Sandbox: installOpenClaw()
SetupGen->>OpenClaw: setupOpenClaw(onboard & inject env)
SetupGen->>GitHub: ensureGithubRepo(account)
GitHub-->>SetupGen: returns repo URL
SetupGen->>Sandbox: git fetch/checkout + submodules (ensureGithubRepo)
SetupGen->>Prompt: yield setup logs
SetupGen->>GitHub: ensureOrgRepos(orgs)
SetupGen->>OC_Agent: runOpenClawAgent(clone instructions)
SetupGen-->>Prompt: return githubRepo URL
Prompt->>Sandbox: pushSandboxToGithub()
Sandbox->>GitHub: push --force origin HEAD:main (tokenized)
Prompt->>Sandbox: upsertAccountSnapshot(snapshot + github_repo)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1✅ 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: 16
🧹 Nitpick comments (8)
lib/github/getExistingGithubRepo.ts (1)
1-24: Extract shared GitHub request config into a common helper.
GITHUB_ORG, auth headers, and API version handling are duplicated here and inlib/github/createGithubRepo.ts; centralizing them will reduce drift and simplify future changes.As per coding guidelines, "**/*.{ts,tsx}: Extract shared logic into reusable utilities following Don't Repeat Yourself (DRY) principle."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/github/getExistingGithubRepo.ts` around lines 1 - 24, The GITHUB_ORG constant and duplicated request config (auth header and API version) used in getExistingGithubRepo are duplicated in createGithubRepo; extract them into a shared utility (e.g., export a GITHUB_ORG and a helper like buildGithubHeaders or getGithubRequestOptions) and replace the inline headers/ORG usage in getExistingGithubRepo and createGithubRepo to use that shared helper; update getExistingGithubRepo to import and call the helper when building the fetch call so org, Authorization, Accept, and X-GitHub-Api-Version are centralized.lib/github/createOrgGithubRepo.ts (1)
22-24: Consider acceptingSetupDepsfor consistent logging.This function uses
console.errordirectly, while similar setup functions useSetupDepsfor logging. If this utility is primarily called from setup contexts, accepting an optional deps parameter would provide consistent logging behavior.Also applies to: 50-52, 59-62
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/github/createOrgGithubRepo.ts` around lines 22 - 24, createOrgGithubRepo currently uses console.error in the token-check and other error paths; update the function signature to accept an optional SetupDeps (e.g., deps?: SetupDeps) and replace direct console.error calls with deps?.logger.error (or deps?.log) falling back to console.error if deps is not provided; locate all occurrences inside createOrgGithubRepo (the token check and the other two error-return branches around lines 50-52 and 59-62) and use the provided deps for consistent logging behavior while preserving the same return/flow.lib/sandbox/setup/ensureSetupSandbox.ts (1)
33-35: Consider extracting skill installation to reduce function length.The function exceeds the 50-line guideline. Extracting the skill installation loop into a small helper would improve readability and bring the function within guidelines.
♻️ Proposed extraction
+const REQUIRED_SKILLS = [ + "recoupable/setup-sandbox", + "recoupable/setup-artist", + "recoupable/release-management", +] as const; + +async function installRequiredSkills(sandbox: Sandbox, deps: SetupDeps): Promise<void> { + deps.log("Installing skills"); + for (const skill of REQUIRED_SKILLS) { + await installSkill(sandbox, skill, deps); + } +} + export async function ensureSetupSandbox( // ... ): Promise<void> { // ... idempotency check ... - deps.log("Installing skills"); - - await installSkill(sandbox, "recoupable/setup-sandbox", deps); - await installSkill(sandbox, "recoupable/setup-artist", deps); - await installSkill(sandbox, "recoupable/release-management", deps); + await installRequiredSkills(sandbox, deps); // ... rest of function ... }As per coding guidelines: "Keep functions under 50 lines".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/sandbox/setup/ensureSetupSandbox.ts` around lines 33 - 35, The ensureSetupSandbox function is over the 50-line guideline because it directly calls installSkill multiple times; extract the repeated installation logic into a small helper (e.g., add a new async function installSetupSkills(sandbox, deps) or installSkillsList(sandbox, deps, skills)) that encapsulates the loop over ["recoupable/setup-sandbox","recoupable/setup-artist","recoupable/release-management"] and calls installSkill for each entry, then replace the three installSkill calls in ensureSetupSandbox with a single await call to that helper; keep helper name unique (installSetupSkills or installSkillsList) so it’s easy to find and test.lib/sandbox/setup/ensureOrgRepos.ts (1)
67-81: Consider extracting message construction for readability.The multi-line message array construction is detailed and could be extracted into a helper function, which would also help bring the main function closer to the 50-line guideline.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/sandbox/setup/ensureOrgRepos.ts` around lines 67 - 81, The multi-line construction of the instructions string assigned to message should be extracted into a small helper to improve readability and shorten the main function (e.g., createCloneInstructions(repoList) or buildRepoCloneMessage(repoList)); move the array join logic into that helper, accept repoList as a parameter, return the joined string, and replace the original inline array/ join with a call to the helper where message is assigned (referencing the message variable and repoList in ensureOrgRepos.ts).lib/sandbox/setup/setupFreshSandbox.ts (1)
30-33: Inconsistent logging:dataparameter ignored in error handler.The
errorcallback receives adataparameter but only logs the message, discarding structured error details. This differs from theSetupDepsinterface contract and reduces observability for debugging.🔧 Proposed fix to include data in error logging
const deps: SetupDeps = { log: msg => console.log(`[Setup] ${msg}`), - error: (msg, data) => console.error(`[Setup] ${msg}`, data), + error: (msg, data) => console.error(`[Setup] ${msg}`, data ?? ""), };Or, to match the
logsignature more closely while still outputting data when present:const deps: SetupDeps = { - log: msg => console.log(`[Setup] ${msg}`), - error: (msg, data) => console.error(`[Setup] ${msg}`, data), + log: (msg, data) => console.log(`[Setup] ${msg}`, data ?? ""), + error: (msg, data) => console.error(`[Setup] ${msg}`, data ?? ""), };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/sandbox/setup/setupFreshSandbox.ts` around lines 30 - 33, The error handler in the deps constant (const deps: SetupDeps) ignores the second parameter; update the error: (msg, data) implementation so it includes the provided data when logging (e.g., pass data to console.error or serialize it) to honor the SetupDeps contract and preserve structured error details; ensure the log signature remains consistent with log and that data is only omitted when undefined.lib/sandbox/promptSandboxStreaming.ts (1)
85-88: DRY opportunity:SetupDepscreation is duplicated.The same
SetupDepsconstruction pattern appears insetupFreshSandbox.tsand here. Consider extracting a shared factory function.♻️ Proposed shared utility
Create a helper in
lib/sandbox/setup/createSetupDeps.ts:import type { SetupDeps } from "./types"; export function createSetupDeps(prefix: string): SetupDeps { return { log: (msg, data) => console.log(`[${prefix}] ${msg}`, data ?? ""), error: (msg, data) => console.error(`[${prefix}] ${msg}`, data ?? ""), }; }Then use it:
-const deps: SetupDeps = { - log: msg => console.log(`[PostSetup] ${msg}`), - error: (msg, data) => console.error(`[PostSetup] ${msg}`, data), -}; +const deps = createSetupDeps("PostSetup");As per coding guidelines: "Extract shared logic into reusable utilities following Don't Repeat Yourself (DRY) principle".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/sandbox/promptSandboxStreaming.ts` around lines 85 - 88, The SetupDeps object construction is duplicated; create a shared factory function createSetupDeps(prefix: string): SetupDeps that returns { log: (msg, data) => console.log(`[${prefix}] ${msg}`, data ?? ""), error: (msg, data) => console.error(`[${prefix}] ${msg}`, data ?? "") }, then replace the inline deps = { ... } in promptSandboxStreaming (the deps variable) and the similar construction in setupFreshSandbox to call createSetupDeps("PostSetup") (or appropriate prefix) so both modules reuse the same utility.lib/sandbox/setup/setupOpenClaw.ts (1)
13-113: Function exceeds 50-line guideline and handles multiple responsibilities.This function handles onboarding check, running onboard, injecting environment variables, and starting the gateway—four distinct responsibilities. Consider extracting into smaller, focused functions.
♻️ Suggested decomposition
async function runOnboardIfNeeded(sandbox: Sandbox, deps: SetupDeps): Promise<void> { /* ... */ } async function injectEnvVars(sandbox: Sandbox, apiKey: string, accountId: string, deps: SetupDeps): Promise<void> { /* ... */ } async function startGateway(sandbox: Sandbox, deps: SetupDeps): Promise<void> { /* ... */ } export async function setupOpenClaw( sandbox: Sandbox, accountId: string, apiKey: string, deps: SetupDeps, ): Promise<void> { await runOnboardIfNeeded(sandbox, deps); await injectEnvVars(sandbox, apiKey, accountId, deps); await startGateway(sandbox, deps); }As per coding guidelines: "Keep functions under 50 lines" and "Single responsibility per function".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/sandbox/setup/setupOpenClaw.ts` around lines 13 - 113, setupOpenClaw is over 50 lines and mixes four responsibilities (onboard check/run, env injection, gateway start); extract each into its own helper and have setupOpenClaw orchestrate them. Create runOnboardIfNeeded(sandbox, deps) to perform the configCheck (using sandbox.runCommand and onboardArgs including process.env.VERCEL_AI_GATEWAY_API_KEY) and the onboard logging/error handling; create injectEnvVars(sandbox, apiKey, accountId, deps) to run the node -e JSON edit (preserve GITHUB_TOKEN handling) and the injectEnv exitCode/error handling; create startGateway(sandbox, deps) to run the nohup openclaw gateway run command and associated logging/error throws; finally replace the body of setupOpenClaw with sequential calls to runOnboardIfNeeded, injectEnvVars, and startGateway.lib/sandbox/setup/ensureGithubRepo.ts (1)
18-140: Split this orchestration into smaller helpers.
ensureGithubRepocurrently combines snapshot lookup, repo provisioning, persistence, clone/bootstrap, and checkout/submodule flow in one function. Extracting private helpers will improve readability/testability and keep function size manageable.As per coding guidelines, “For domain functions, ensure: ... Keep functions under 50 lines” and “Apply Single Responsibility Principle (SRP): one exported function per file; each file should do one thing well.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/sandbox/setup/ensureGithubRepo.ts` around lines 18 - 140, The ensureGithubRepo function is doing too many things; split its logic into small private helpers to meet SRP and the 50-line guideline: extract (1) fetchSnapshotAndRepo(sandbox/accountId) that calls selectAccountSnapshots and returns {githubRepo, snapshotId}, (2) provisionRepoIfMissing(accountId, snapshotId) that wraps selectAccounts, createGithubRepo, upsertAccountSnapshot and returns the repo URL, and (3) cloneAndInitRepo(sandbox, githubRepo, githubToken) that contains the git init/remote/fetch/checkout/submodule logic and uses runGitCommand and sandbox.runCommand; keep ensureGithubRepo as the exported orchestrator that uses these helpers and returns the repo URL. Use the existing function names (ensureGithubRepo, selectAccountSnapshots, selectAccounts, createGithubRepo, upsertAccountSnapshot, runGitCommand, sandbox.runCommand) to locate code and move code blocks into the new private functions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/github/createGithubRepo.ts`:
- Around line 41-44: When response.ok is false and response.status === 422,
parse the response.json() body and inspect its errors array for an entry where
error.code === "already_exists"; only in that case call
getExistingGithubRepo(repoName) and return its result; otherwise log the parsed
error details (including message and errors) and return a failure (e.g., throw
or return a rejected result) so other validation issues aren't masked. Ensure
you reference response, repoName, and getExistingGithubRepo when making the
change.
In `@lib/sandbox/promptSandboxStreaming.ts`:
- Around line 83-98: The post-prompt sequence under isFreshSandbox (calls to
pushSandboxToGithub, sandbox.snapshot, and upsertAccountSnapshot) can throw and
abort the whole generator; wrap the whole block in a try/catch so any errors are
caught, logged using the existing deps logger (or processLogger) and not
rethrown, ensuring failures don’t prevent returning the user's prompt results
(stdout/stderr/exitCode); keep the existing deps object and its log/error usage
for messages and include contextual info (e.g., sandbox id, githubRepo) when
logging the caught error.
In `@lib/sandbox/setup/ensureGithubRepo.ts`:
- Around line 65-74: When .git exists (gitCheck) call sandbox.runCommand({cmd:
"git", args: ["remote", "get-url", "origin"]}) to read the configured origin and
compare the returned stdout (trimmed) to githubRepo; if they match, return
githubRepo as before, but if they differ, update the remote to the expected URL
by running sandbox.runCommand({cmd: "git", args: ["remote", "set-url", "origin",
githubRepo]}) (or fail with an error/log) before returning; reference gitCheck,
sandbox.runCommand, githubRepo and deps.log when making the change.
- Around line 99-133: The code currently hardcodes checking out "origin/main"
using sandbox.runCommand and runGitCommand; replace that logic in
ensureGithubRepo.ts to detect the remote default branch by running "git
ls-remote --symref origin HEAD" (via sandbox.runCommand), parse the symref
target (e.g., "refs/heads/<default>"), and then use runGitCommand to checkout
"-B <default>" from "origin/<default>" instead of "origin/main"; if detection
fails, fall back to the existing behavior but log a clear fallback via deps.log;
keep the subsequent URL rewrite (git config url.https://x-access-token:...) and
submodule initialization unchanged.
- Around line 79-82: The current code constructs repoUrl with the GITHUB_TOKEN
(repoUrl = githubRepo.replace(...)) and later persists it into .git/config via
the git remote add / git config blocks, which leaks secrets; instead, stop
embedding githubToken into repoUrl and remove any code that writes the token
into git config (the repoUrl construction and the git config / remote add that
persists credentials). Modify ensureGithubRepo to use per-command authentication
by invoking git commands with the -c url.<base>.insteadOf=<replacement> flag
(using githubRepo and githubToken) for the temporary mapping when running
clone/fetch/push, and keep githubRepo/gatsby variables unchanged so no token is
ever written to disk or .git/config.
In `@lib/sandbox/setup/ensureOrgRepos.ts`:
- Around line 83-90: The call to runOpenClawAgent in ensureOrgRepos currently
ignores the returned result.exitCode so failures still allow execution to
continue and log "Org repo setup complete"; update the ensureOrgRepos flow to
capture the result from runOpenClawAgent, check result.exitCode, and if non-zero
log an error including the exitCode and any returned output/message (identify
the agent call by label "Cloning org repos"), then throw or return an error to
stop further processing so failures are propagated instead of being reported as
success.
In `@lib/sandbox/setup/helpers.ts`:
- Around line 13-132: The file exports three helpers that violate SRP; split
runGitCommand, runOpenClawAgent, and installSkill into their own files (one
exported function per file) and update imports/exports accordingly: create files
(e.g., runGitCommand.ts, runOpenClawAgent.ts, installSkill.ts) that export the
corresponding function (preserve signatures and types like Sandbox and
SetupDeps), move related interfaces
(RunOpenClawAgentOptions/RunOpenClawAgentResult) into the runOpenClawAgent file
or a shared types file if needed, and modify any callers to import the functions
from their new module locations.
- Around line 101-121: The shell interpolation of skillName into a single "sh
-c" string is vulnerable to command injection; before using skill or skillName
(extracted via skill.split("/").pop()), validate and sanitize it (reject or
escape values containing metacharacters like ; & | $ ` \n or leading dashes and
spaces) or better yet avoid "sh -c" entirely: run safe commands via
sandbox.runCommand with explicit argv arrays (e.g., separate calls for mkdir -p,
rm -rf and cp -r) passing the sanitized source and destination as args so no
shell interpolation occurs; update the block that constructs the copy command
(the sandbox.runCommand with cmd "sh") to use validated skillName and
non-shell-joined arguments instead.
In `@lib/sandbox/setup/installOpenClaw.ts`:
- Around line 24-27: The npm install call currently uses the non-reproducible
selector "openclaw@latest" in the sandbox.runCommand invocation (see installCLI
and runCommand usage); change this to pin a concrete OpenClaw version (e.g.,
"openclaw@<MAJOR.MINOR.PATCH>") so the args array passed to sandbox.runCommand
is ["install","-g","openclaw@<VERSION>"], ensuring reproducible installs and
stable onboarding.
In `@lib/sandbox/setup/pushSandboxToGithub.ts`:
- Around line 11-22: copyOpenClawToRepo and addOrgSubmodules currently fire
sandbox.runCommand for critical shell/git steps but ignore failures, allowing
orchestration to proceed on error; update both functions (copyOpenClawToRepo and
addOrgSubmodules) to await and verify the result of sandbox.runCommand (or catch
thrown errors) and on any non-successful exit throw an Error or propagate the
failure so the caller stops; ensure you surface error details (command,
stdout/stderr) in the thrown Error or processLogger so callers can diagnose the
failure before continuing.
- Around line 100-108: The sanitization command in stripGitmodulesTokens
currently suppresses failures with "|| true", allowing tokenized URLs to slip
through; update the sandbox.runCommand call in stripGitmodulesTokens to remove
the "|| true" suffixes so that both the sed invocation and the subsequent git
add (the command string passed to sandbox.runCommand) return non-zero and cause
the Promise to reject on failure, ensuring failures surface and prevent
committing tokenized URLs; keep any harmless redirections like 2>/dev/null if
desired but do not swallow exit codes.
- Around line 170-237: pushSandboxToGithub is too large and mixes multiple
responsibilities (git identity setup, org repo push, workspace sync, submodule
changes, commit, and push); refactor by extracting smaller helper functions and
orchestrating them from pushSandboxToGithub. Create private/inner functions like
configureGitIdentity(sandbox, deps) that calls runGitCommand for user.email and
user.name, prepareWorkspace(sandbox, deps) that runs pushOrgRepos,
copyOpenClawToRepo, addOrgSubmodules, and stripGitmodulesTokens,
stageAndCommitIfNeeded(sandbox, deps) that runs git add -A, checks git diff
--cached --quiet via sandbox.runCommand and commits with runGitCommand, and
pushHeadToMain(sandbox, deps) that does the rebase abort and push with
runGitCommand; then make pushSandboxToGithub call these helpers in sequence and
return their aggregated boolean result to keep each function under 50 lines and
single-responsibility.
In `@lib/sandbox/setup/setupOpenClaw.ts`:
- Around line 67-82: The code in injectEnv calling sandbox.runCommand builds a
shell-invoked node -e string by interpolating apiKey, accountId and githubToken
directly, risking shell/JS injection; fix it by avoiding direct
interpolation—encode those values with JSON.stringify (or pass them via
stdin/temp file) and construct the injected script using the JSON-escaped
strings so special characters are safely escaped before embedding into the node
-e payload, updating the args payload logic around sandbox.runCommand and the
constructed command string accordingly.
- Around line 34-35: The code passes process.env.VERCEL_AI_GATEWAY_API_KEY!
directly which can crash or pass undefined; in the setupOpenClaw function
validate the environment variable first (e.g., const apiKey =
process.env.VERCEL_AI_GATEWAY_API_KEY), if missing log an explicit error and
throw or process.exit(1), otherwise use the validated apiKey when building the
args array instead of the non-null assertion; ensure the change touches the
argument list construction where "--ai-gateway-api-key" and
process.env.VERCEL_AI_GATEWAY_API_KEY! are used so the runtime always has a
defined string or fails fast.
In `@lib/sandbox/setup/writeReadme.ts`:
- Around line 14-102: The writeReadme function is doing three responsibilities
(existence check, content construction, and file write) and exceeds the
50-line/domain-function guideline; refactor by extracting the grep check into a
new async function (e.g., readmeExists or checkReadmeContains) that calls
sandbox.runCommand, the string assembly into a buildReadmeContent(sandboxId,
accountId, githubRepo) helper that returns the README text, and the file write
into a writeReadmeFile(sandbox, content) helper that calls sandbox.writeFiles;
then reduce writeReadme to orchestrate: call readmeExists, bail if true, call
buildReadmeContent and writeReadmeFile, and keep logging via deps.log as before.
- Around line 21-24: The idempotency check uses sandbox.runCommand(...) assigned
to check to grep "README.md" in the current working directory while the code
later writes to "/vercel/sandbox/README.md", causing repeated rewrites; update
the grep target to the same absolute path used for writing (i.e., grep -q
"Recoup Sandbox" "/vercel/sandbox/README.md") or derive the target from the same
variable/path used when writing the file so the check and write both reference
the same file (locate the call to sandbox.runCommand in writeReadme.ts and align
its args to the write path).
---
Nitpick comments:
In `@lib/github/createOrgGithubRepo.ts`:
- Around line 22-24: createOrgGithubRepo currently uses console.error in the
token-check and other error paths; update the function signature to accept an
optional SetupDeps (e.g., deps?: SetupDeps) and replace direct console.error
calls with deps?.logger.error (or deps?.log) falling back to console.error if
deps is not provided; locate all occurrences inside createOrgGithubRepo (the
token check and the other two error-return branches around lines 50-52 and
59-62) and use the provided deps for consistent logging behavior while
preserving the same return/flow.
In `@lib/github/getExistingGithubRepo.ts`:
- Around line 1-24: The GITHUB_ORG constant and duplicated request config (auth
header and API version) used in getExistingGithubRepo are duplicated in
createGithubRepo; extract them into a shared utility (e.g., export a GITHUB_ORG
and a helper like buildGithubHeaders or getGithubRequestOptions) and replace the
inline headers/ORG usage in getExistingGithubRepo and createGithubRepo to use
that shared helper; update getExistingGithubRepo to import and call the helper
when building the fetch call so org, Authorization, Accept, and
X-GitHub-Api-Version are centralized.
In `@lib/sandbox/promptSandboxStreaming.ts`:
- Around line 85-88: The SetupDeps object construction is duplicated; create a
shared factory function createSetupDeps(prefix: string): SetupDeps that returns
{ log: (msg, data) => console.log(`[${prefix}] ${msg}`, data ?? ""), error:
(msg, data) => console.error(`[${prefix}] ${msg}`, data ?? "") }, then replace
the inline deps = { ... } in promptSandboxStreaming (the deps variable) and the
similar construction in setupFreshSandbox to call createSetupDeps("PostSetup")
(or appropriate prefix) so both modules reuse the same utility.
In `@lib/sandbox/setup/ensureGithubRepo.ts`:
- Around line 18-140: The ensureGithubRepo function is doing too many things;
split its logic into small private helpers to meet SRP and the 50-line
guideline: extract (1) fetchSnapshotAndRepo(sandbox/accountId) that calls
selectAccountSnapshots and returns {githubRepo, snapshotId}, (2)
provisionRepoIfMissing(accountId, snapshotId) that wraps selectAccounts,
createGithubRepo, upsertAccountSnapshot and returns the repo URL, and (3)
cloneAndInitRepo(sandbox, githubRepo, githubToken) that contains the git
init/remote/fetch/checkout/submodule logic and uses runGitCommand and
sandbox.runCommand; keep ensureGithubRepo as the exported orchestrator that uses
these helpers and returns the repo URL. Use the existing function names
(ensureGithubRepo, selectAccountSnapshots, selectAccounts, createGithubRepo,
upsertAccountSnapshot, runGitCommand, sandbox.runCommand) to locate code and
move code blocks into the new private functions.
In `@lib/sandbox/setup/ensureOrgRepos.ts`:
- Around line 67-81: The multi-line construction of the instructions string
assigned to message should be extracted into a small helper to improve
readability and shorten the main function (e.g.,
createCloneInstructions(repoList) or buildRepoCloneMessage(repoList)); move the
array join logic into that helper, accept repoList as a parameter, return the
joined string, and replace the original inline array/ join with a call to the
helper where message is assigned (referencing the message variable and repoList
in ensureOrgRepos.ts).
In `@lib/sandbox/setup/ensureSetupSandbox.ts`:
- Around line 33-35: The ensureSetupSandbox function is over the 50-line
guideline because it directly calls installSkill multiple times; extract the
repeated installation logic into a small helper (e.g., add a new async function
installSetupSkills(sandbox, deps) or installSkillsList(sandbox, deps, skills))
that encapsulates the loop over
["recoupable/setup-sandbox","recoupable/setup-artist","recoupable/release-management"]
and calls installSkill for each entry, then replace the three installSkill calls
in ensureSetupSandbox with a single await call to that helper; keep helper name
unique (installSetupSkills or installSkillsList) so it’s easy to find and test.
In `@lib/sandbox/setup/setupFreshSandbox.ts`:
- Around line 30-33: The error handler in the deps constant (const deps:
SetupDeps) ignores the second parameter; update the error: (msg, data)
implementation so it includes the provided data when logging (e.g., pass data to
console.error or serialize it) to honor the SetupDeps contract and preserve
structured error details; ensure the log signature remains consistent with log
and that data is only omitted when undefined.
In `@lib/sandbox/setup/setupOpenClaw.ts`:
- Around line 13-113: setupOpenClaw is over 50 lines and mixes four
responsibilities (onboard check/run, env injection, gateway start); extract each
into its own helper and have setupOpenClaw orchestrate them. Create
runOnboardIfNeeded(sandbox, deps) to perform the configCheck (using
sandbox.runCommand and onboardArgs including
process.env.VERCEL_AI_GATEWAY_API_KEY) and the onboard logging/error handling;
create injectEnvVars(sandbox, apiKey, accountId, deps) to run the node -e JSON
edit (preserve GITHUB_TOKEN handling) and the injectEnv exitCode/error handling;
create startGateway(sandbox, deps) to run the nohup openclaw gateway run command
and associated logging/error throws; finally replace the body of setupOpenClaw
with sequential calls to runOnboardIfNeeded, injectEnvVars, and startGateway.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (16)
lib/github/__tests__/createGithubRepo.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/github/__tests__/createOrgGithubRepo.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/github/__tests__/getExistingGithubRepo.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/github/__tests__/sanitizeRepoName.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/sandbox/__tests__/createSandboxFromSnapshot.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/sandbox/__tests__/getOrCreateSandbox.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/sandbox/__tests__/promptSandboxStreaming.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/sandbox/setup/__tests__/ensureGithubRepo.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/sandbox/setup/__tests__/ensureOrgRepos.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/sandbox/setup/__tests__/ensureSetupSandbox.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/sandbox/setup/__tests__/helpers.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/sandbox/setup/__tests__/installOpenClaw.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/sandbox/setup/__tests__/pushSandboxToGithub.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/sandbox/setup/__tests__/setupFreshSandbox.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/sandbox/setup/__tests__/setupOpenClaw.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**lib/sandbox/setup/__tests__/writeReadme.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**
📒 Files selected for processing (17)
lib/github/createGithubRepo.tslib/github/createOrgGithubRepo.tslib/github/getExistingGithubRepo.tslib/github/sanitizeRepoName.tslib/sandbox/createSandboxFromSnapshot.tslib/sandbox/getOrCreateSandbox.tslib/sandbox/promptSandboxStreaming.tslib/sandbox/setup/ensureGithubRepo.tslib/sandbox/setup/ensureOrgRepos.tslib/sandbox/setup/ensureSetupSandbox.tslib/sandbox/setup/helpers.tslib/sandbox/setup/installOpenClaw.tslib/sandbox/setup/pushSandboxToGithub.tslib/sandbox/setup/setupFreshSandbox.tslib/sandbox/setup/setupOpenClaw.tslib/sandbox/setup/types.tslib/sandbox/setup/writeReadme.ts
lib/github/createGithubRepo.ts
Outdated
Comment on lines
+41
to
+44
| if (!response.ok) { | ||
| if (response.status === 422) { | ||
| return getExistingGithubRepo(repoName); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
GitHub REST API POST /orgs/{org}/repos 422 response body validation error fields
💡 Result:
For POST /orgs/{org}/repos, a 422 Unprocessable Entity from the GitHub REST API generally means “Validation Failed” (or sometimes “the endpoint has been spammed”). The response body is typically:
{
"message": "Validation Failed",
"errors": [
{
"resource": "Repository",
"field": "name",
"code": "missing_field",
"message": "..." // sometimes omitted
}
],
"documentation_url": "..."
}Response body fields you can expect/use:
message(string): usually"Validation Failed". [1]errors(array): list of specific validation problems. Each item commonly includes:resource(string): what kind of object failed (e.g.,"Repository"). [2]field(string|null): which input field is invalid (e.g.,"name","private", etc.). [2]code(string): machine-friendly reason. GitHub documents these common codes:missing,missing_field,invalid,already_exists,unprocessable,custom(use the per-errormessagefor details). [1]
message(string, optional): human detail (often present, but not guaranteed). [1], [2]
documentation_url(string): link to the relevant docs page. [2]
Repo-creation-specific 422 gotcha:
- If you set
private/visibilityin a way that violates org policy (e.g., org restricts visibility changes to org owners), GitHub explicitly notes you can get a 422 for that. [3]
Sources
- GitHub Docs – “Troubleshooting the REST API” (Validation Failed + error
codemeanings). (docs.github.com) - Example 422 body showing
message,errors[].resource/field/code/message,documentation_url. (github.com) - GitHub Docs – “Create an organization repository” notes 422 for org visibility restrictions. (docs.github.com)
Citations:
- 1: https://docs.github.com/enterprise-cloud%40latest/rest/using-the-rest-api/troubleshooting-the-rest-api
- 2: https://github.com/orgs/community/discussions/139773?utm_source=openai
- 3: https://docs.github.com/rest/repos/repos?utm_source=openai
🏁 Script executed:
cat -n lib/github/createGithubRepo.tsRepository: recoupable/api
Length of output: 2144
Parse the 422 response body to check for "already_exists" error code before falling back to existing repo lookup.
GitHub returns 422 for multiple validation failures (missing fields, org policy violations, etc.). Only when errors[].code === "already_exists" should you call getExistingGithubRepo(); otherwise log the actual error and return failure to avoid masking legitimate validation issues.
Current problematic code (lines 41-44)
if (!response.ok) {
if (response.status === 422) {
return getExistingGithubRepo(repoName);
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/github/createGithubRepo.ts` around lines 41 - 44, When response.ok is
false and response.status === 422, parse the response.json() body and inspect
its errors array for an entry where error.code === "already_exists"; only in
that case call getExistingGithubRepo(repoName) and return its result; otherwise
log the parsed error details (including message and errors) and return a failure
(e.g., throw or return a rejected result) so other validation issues aren't
masked. Ensure you reference response, repoName, and getExistingGithubRepo when
making the change.
Comment on lines
+83
to
+98
| // Post-prompt: push to GitHub and snapshot for fresh sandboxes | ||
| if (isFreshSandbox) { | ||
| const deps: SetupDeps = { | ||
| log: msg => console.log(`[PostSetup] ${msg}`), | ||
| error: (msg, data) => console.error(`[PostSetup] ${msg}`, data), | ||
| }; | ||
|
|
||
| await pushSandboxToGithub(sandbox, deps); | ||
|
|
||
| const snapshotResult = await sandbox.snapshot(); | ||
| await upsertAccountSnapshot({ | ||
| account_id: accountId, | ||
| snapshot_id: snapshotResult.snapshotId, | ||
| github_repo: githubRepo, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Missing error handling for post-prompt steps could lose user's prompt results.
If pushSandboxToGithub, sandbox.snapshot(), or upsertAccountSnapshot throws, the entire generator throws and the user loses their prompt results (stdout, stderr, exitCode). These post-prompt steps are secondary to the user's actual work—failures should be logged but not prevent returning results.
🛡️ Proposed fix with defensive error handling
// Post-prompt: push to GitHub and snapshot for fresh sandboxes
if (isFreshSandbox) {
const deps: SetupDeps = {
log: msg => console.log(`[PostSetup] ${msg}`),
error: (msg, data) => console.error(`[PostSetup] ${msg}`, data),
};
- await pushSandboxToGithub(sandbox, deps);
-
- const snapshotResult = await sandbox.snapshot();
- await upsertAccountSnapshot({
- account_id: accountId,
- snapshot_id: snapshotResult.snapshotId,
- github_repo: githubRepo,
- });
+ try {
+ await pushSandboxToGithub(sandbox, deps);
+
+ const snapshotResult = await sandbox.snapshot();
+ await upsertAccountSnapshot({
+ account_id: accountId,
+ snapshot_id: snapshotResult.snapshotId,
+ github_repo: githubRepo,
+ });
+ } catch (err) {
+ deps.error("Post-prompt setup failed, continuing with result", {
+ error: err instanceof Error ? err.message : String(err),
+ });
+ }
}📝 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
| // Post-prompt: push to GitHub and snapshot for fresh sandboxes | |
| if (isFreshSandbox) { | |
| const deps: SetupDeps = { | |
| log: msg => console.log(`[PostSetup] ${msg}`), | |
| error: (msg, data) => console.error(`[PostSetup] ${msg}`, data), | |
| }; | |
| await pushSandboxToGithub(sandbox, deps); | |
| const snapshotResult = await sandbox.snapshot(); | |
| await upsertAccountSnapshot({ | |
| account_id: accountId, | |
| snapshot_id: snapshotResult.snapshotId, | |
| github_repo: githubRepo, | |
| }); | |
| } | |
| // Post-prompt: push to GitHub and snapshot for fresh sandboxes | |
| if (isFreshSandbox) { | |
| const deps: SetupDeps = { | |
| log: msg => console.log(`[PostSetup] ${msg}`), | |
| error: (msg, data) => console.error(`[PostSetup] ${msg}`, data), | |
| }; | |
| try { | |
| await pushSandboxToGithub(sandbox, deps); | |
| const snapshotResult = await sandbox.snapshot(); | |
| await upsertAccountSnapshot({ | |
| account_id: accountId, | |
| snapshot_id: snapshotResult.snapshotId, | |
| github_repo: githubRepo, | |
| }); | |
| } catch (err) { | |
| deps.error("Post-prompt setup failed, continuing with result", { | |
| error: err instanceof Error ? err.message : String(err), | |
| }); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/sandbox/promptSandboxStreaming.ts` around lines 83 - 98, The post-prompt
sequence under isFreshSandbox (calls to pushSandboxToGithub, sandbox.snapshot,
and upsertAccountSnapshot) can throw and abort the whole generator; wrap the
whole block in a try/catch so any errors are caught, logged using the existing
deps logger (or processLogger) and not rethrown, ensuring failures don’t prevent
returning the user's prompt results (stdout/stderr/exitCode); keep the existing
deps object and its log/error usage for messages and include contextual info
(e.g., sandbox id, githubRepo) when logging the caught error.
Comment on lines
+65
to
+74
| // Check if repo is already cloned | ||
| const gitCheck = await sandbox.runCommand({ | ||
| cmd: "test", | ||
| args: ["-d", ".git"], | ||
| }); | ||
|
|
||
| if (gitCheck.exitCode === 0) { | ||
| deps.log("GitHub repo already cloned in sandbox"); | ||
| return githubRepo; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n lib/sandbox/setup/ensureGithubRepo.tsRepository: recoupable/api
Length of output: 5022
🏁 Script executed:
rg "git.*remote" -t ts --max-count 20Repository: recoupable/api
Length of output: 283
🏁 Script executed:
rg "git.*config" -t ts --max-count 20Repository: recoupable/api
Length of output: 983
🏁 Script executed:
head -150 lib/sandbox/setup/__tests__/ensureGithubRepo.test.tsRepository: recoupable/api
Length of output: 4850
Validate origin remote URL before returning early.
When .git already exists (lines 65-74), the function returns without verifying that the origin remote points to the expected githubRepo. This creates a critical asymmetry: the clone-from-scratch path (lines 76-90) properly sets origin to the correct URL, but the early-return path silently reuses any existing repository. If a different repository was previously cloned, the function would incorrectly return success with the wrong origin configured.
Add a git remote get-url origin check before returning at line 73, comparing it against githubRepo to ensure consistency.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/sandbox/setup/ensureGithubRepo.ts` around lines 65 - 74, When .git exists
(gitCheck) call sandbox.runCommand({cmd: "git", args: ["remote", "get-url",
"origin"]}) to read the configured origin and compare the returned stdout
(trimmed) to githubRepo; if they match, return githubRepo as before, but if they
differ, update the remote to the expected URL by running
sandbox.runCommand({cmd: "git", args: ["remote", "set-url", "origin",
githubRepo]}) (or fail with an error/log) before returning; reference gitCheck,
sandbox.runCommand, githubRepo and deps.log when making the change.
Comment on lines
+79
to
+82
| const repoUrl = githubRepo.replace( | ||
| "https://github.com/", | ||
| `https://x-access-token:${githubToken}@github.com/`, | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n lib/sandbox/setup/ensureGithubRepo.tsRepository: recoupable/api
Length of output: 5022
Remove GITHUB_TOKEN from persisted git config; use per-command auth instead.
Lines 79–82 and 117–123 persist the token in .git/config, which leaks credentials through sandbox snapshots and git config output.
The git -c url.<base>.insteadOf=<replacement> flag applies authentication per-command without writing to config, which is the correct approach for sensitive secrets.
🔐 Fix: Replace remote add and git config with per-command authentication
const repoUrl = githubRepo.replace(
"https://github.com/",
- `https://x-access-token:${githubToken}@github.com/`,
+ "https://github.com/",
);
- if (!(await runGitCommand(sandbox, ["remote", "add", "origin", repoUrl], "add remote", deps))) {
+ if (!(await runGitCommand(sandbox, ["remote", "add", "origin", githubRepo], "add remote", deps))) {
return undefined;
}
// Fetch and checkout only if the remote has content
const fetchResult = await sandbox.runCommand({
cmd: "git",
- args: ["fetch", "origin"],
+ args: [
+ "-c",
+ `url.https://x-access-token:${githubToken}@github.com/.insteadOf=https://github.com/`,
+ "fetch",
+ "origin",
+ ],
});
- // Set up URL rewriting for submodule clones
- await sandbox.runCommand({
- cmd: "git",
- args: [
- "config",
- `url.https://x-access-token:${githubToken}@github.com/.insteadOf`,
- "https://github.com/",
- ],
- });
// Initialize submodules if they exist
await sandbox.runCommand({
cmd: "git",
- args: ["submodule", "update", "--init", "--recursive"],
+ args: [
+ "-c",
+ `url.https://x-access-token:${githubToken}@github.com/.insteadOf=https://github.com/`,
+ "submodule",
+ "update",
+ "--init",
+ "--recursive",
+ ],
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/sandbox/setup/ensureGithubRepo.ts` around lines 79 - 82, The current code
constructs repoUrl with the GITHUB_TOKEN (repoUrl = githubRepo.replace(...)) and
later persists it into .git/config via the git remote add / git config blocks,
which leaks secrets; instead, stop embedding githubToken into repoUrl and remove
any code that writes the token into git config (the repoUrl construction and the
git config / remote add that persists credentials). Modify ensureGithubRepo to
use per-command authentication by invoking git commands with the -c
url.<base>.insteadOf=<replacement> flag (using githubRepo and githubToken) for
the temporary mapping when running clone/fetch/push, and keep githubRepo/gatsby
variables unchanged so no token is ever written to disk or .git/config.
Comment on lines
+99
to
+133
| const refCheck = await sandbox.runCommand({ | ||
| cmd: "git", | ||
| args: ["rev-parse", "--verify", "origin/main"], | ||
| }); | ||
|
|
||
| if (refCheck.exitCode === 0) { | ||
| if ( | ||
| !(await runGitCommand( | ||
| sandbox, | ||
| ["checkout", "-B", "main", "origin/main"], | ||
| "checkout main branch", | ||
| deps, | ||
| )) | ||
| ) { | ||
| return undefined; | ||
| } | ||
|
|
||
| // Set up URL rewriting for submodule clones | ||
| await sandbox.runCommand({ | ||
| cmd: "git", | ||
| args: [ | ||
| "config", | ||
| `url.https://x-access-token:${githubToken}@github.com/.insteadOf`, | ||
| "https://github.com/", | ||
| ], | ||
| }); | ||
|
|
||
| // Initialize submodules if they exist | ||
| await sandbox.runCommand({ | ||
| cmd: "git", | ||
| args: ["submodule", "update", "--init", "--recursive"], | ||
| }); | ||
| } else { | ||
| deps.log("Empty remote repo, skipping checkout"); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's see the full file to understand the context
wc -l lib/sandbox/setup/ensureGithubRepo.tsRepository: recoupable/api
Length of output: 99
🏁 Script executed:
# Read the full file to understand the context
cat -n lib/sandbox/setup/ensureGithubRepo.tsRepository: recoupable/api
Length of output: 5022
🏁 Script executed:
# Search for any other default branch handling in the codebase
rg "origin/main|origin/master|default.*branch|symbolic-ref" --type ts --type tsx -B 2 -A 2Repository: recoupable/api
Length of output: 85
🏁 Script executed:
# Check if there's a git command to detect the actual default branch
# Look for any patterns that fetch or detect remote default branch
rg "ls-remote|symbolic-ref|HEAD" --type ts --type tsxRepository: recoupable/api
Length of output: 85
🏁 Script executed:
# Check if there are other patterns in the codebase for detecting default branches
rg "checkout.*-B|main|master|HEAD" --type ts -B 2 -A 2 | head -50Repository: recoupable/api
Length of output: 3885
🏁 Script executed:
# Search for any git.symbolic-ref or similar patterns that might detect default branch
rg "symbolic-ref|ls-remote|HEAD" --type tsRepository: recoupable/api
Length of output: 383
🏁 Script executed:
# Let's verify what git commands can detect the default branch
# The standard way is: git symbolic-ref refs/remotes/origin/HEAD
# This returns something like: refs/remotes/origin/main or refs/remotes/origin/master
# Or: git ls-remote --symref origin HEAD
# Check if there's any documentation or examples of how to handle this
cat -n lib/sandbox/setup/helpers.ts 2>/dev/null || echo "File not found, searching for helper files"Repository: recoupable/api
Length of output: 4476
🏁 Script executed:
# Find helper files in the sandbox setup directory
fd ".*\.ts" lib/sandbox/setup/Repository: recoupable/api
Length of output: 875
🏁 Script executed:
# Read the helpers file to understand the pattern
cat -n lib/sandbox/setup/helpers.tsRepository: recoupable/api
Length of output: 4476
Detect and use the actual default branch instead of hardcoding origin/main.
The code assumes the default branch is main (line 101), but repositories using origin/master or other default branches will fail the ref check silently and skip checkout with the misleading log message "Empty remote repo, skipping checkout" (line 132).
Use git ls-remote --symref origin HEAD to detect the actual default branch after fetch, then checkout that branch instead of hardcoding the name.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/sandbox/setup/ensureGithubRepo.ts` around lines 99 - 133, The code
currently hardcodes checking out "origin/main" using sandbox.runCommand and
runGitCommand; replace that logic in ensureGithubRepo.ts to detect the remote
default branch by running "git ls-remote --symref origin HEAD" (via
sandbox.runCommand), parse the symref target (e.g., "refs/heads/<default>"), and
then use runGitCommand to checkout "-B <default>" from "origin/<default>"
instead of "origin/main"; if detection fails, fall back to the existing behavior
but log a clear fallback via deps.log; keep the subsequent URL rewrite (git
config url.https://x-access-token:...) and submodule initialization unchanged.
Comment on lines
+170
to
+237
| export async function pushSandboxToGithub(sandbox: Sandbox, deps: SetupDeps): Promise<boolean> { | ||
| deps.log("Pushing sandbox files to GitHub"); | ||
|
|
||
| if ( | ||
| !(await runGitCommand( | ||
| sandbox, | ||
| ["config", "user.email", "agent@recoupable.com"], | ||
| "configure git email", | ||
| deps, | ||
| )) | ||
| ) { | ||
| return false; | ||
| } | ||
|
|
||
| if ( | ||
| !(await runGitCommand( | ||
| sandbox, | ||
| ["config", "user.name", "Recoup Agent"], | ||
| "configure git name", | ||
| deps, | ||
| )) | ||
| ) { | ||
| return false; | ||
| } | ||
|
|
||
| await pushOrgRepos(sandbox, deps); | ||
| await copyOpenClawToRepo(sandbox); | ||
| await addOrgSubmodules(sandbox); | ||
| await stripGitmodulesTokens(sandbox); | ||
|
|
||
| if (!(await runGitCommand(sandbox, ["add", "-A"], "stage files", deps))) { | ||
| return false; | ||
| } | ||
|
|
||
| const diffResult = await sandbox.runCommand({ | ||
| cmd: "git", | ||
| args: ["diff", "--cached", "--quiet"], | ||
| }); | ||
|
|
||
| if (diffResult.exitCode !== 0) { | ||
| if ( | ||
| !(await runGitCommand( | ||
| sandbox, | ||
| ["commit", "-m", "Update sandbox files"], | ||
| "commit changes", | ||
| deps, | ||
| )) | ||
| ) { | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| await sandbox.runCommand({ cmd: "git", args: ["rebase", "--abort"] }); | ||
|
|
||
| if ( | ||
| !(await runGitCommand( | ||
| sandbox, | ||
| ["push", "--force", "origin", "HEAD:main"], | ||
| "push to remote", | ||
| deps, | ||
| )) | ||
| ) { | ||
| return false; | ||
| } | ||
|
|
||
| deps.log("Sandbox files pushed to GitHub successfully"); | ||
| return true; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Decompose pushSandboxToGithub into smaller orchestration steps.
The function currently exceeds 50 lines and combines git identity setup, org push, workspace sync, submodule mutation, commit, and push in one block.
As per coding guidelines, "For domain functions, ensure: Single responsibility per function" and "Keep functions under 50 lines."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/sandbox/setup/pushSandboxToGithub.ts` around lines 170 - 237,
pushSandboxToGithub is too large and mixes multiple responsibilities (git
identity setup, org repo push, workspace sync, submodule changes, commit, and
push); refactor by extracting smaller helper functions and orchestrating them
from pushSandboxToGithub. Create private/inner functions like
configureGitIdentity(sandbox, deps) that calls runGitCommand for user.email and
user.name, prepareWorkspace(sandbox, deps) that runs pushOrgRepos,
copyOpenClawToRepo, addOrgSubmodules, and stripGitmodulesTokens,
stageAndCommitIfNeeded(sandbox, deps) that runs git add -A, checks git diff
--cached --quiet via sandbox.runCommand and commits with runGitCommand, and
pushHeadToMain(sandbox, deps) that does the rebase abort and push with
runGitCommand; then make pushSandboxToGithub call these helpers in sequence and
return their aggregated boolean result to keep each function under 50 lines and
single-responsibility.
Comment on lines
+67
to
+82
| const injectEnv = await sandbox.runCommand({ | ||
| cmd: "sh", | ||
| args: [ | ||
| "-c", | ||
| `node -e " | ||
| const fs = require('fs'); | ||
| const p = require('os').homedir() + '/.openclaw/openclaw.json'; | ||
| const c = JSON.parse(fs.readFileSync(p, 'utf8')); | ||
| c.env = c.env || {}; | ||
| c.env.RECOUP_API_KEY = '${apiKey}'; | ||
| c.env.RECOUP_ACCOUNT_ID = '${accountId}'; | ||
| ${githubToken ? `c.env.GITHUB_TOKEN = '${githubToken}';` : ""} | ||
| fs.writeFileSync(p, JSON.stringify(c, null, 2)); | ||
| "`, | ||
| ], | ||
| }); |
There was a problem hiding this comment.
Security concern: Shell injection risk from string interpolation.
The apiKey, accountId, and githubToken values are interpolated directly into a shell command string using template literals. If any of these values contain shell metacharacters (e.g., ', $, `), the command could break or potentially execute unintended code.
🔒 Proposed fix using JSON.stringify for safe escaping
const injectEnv = await sandbox.runCommand({
cmd: "sh",
args: [
"-c",
`node -e "
const fs = require('fs');
const p = require('os').homedir() + '/.openclaw/openclaw.json';
const c = JSON.parse(fs.readFileSync(p, 'utf8'));
c.env = c.env || {};
- c.env.RECOUP_API_KEY = '${apiKey}';
- c.env.RECOUP_ACCOUNT_ID = '${accountId}';
- ${githubToken ? `c.env.GITHUB_TOKEN = '${githubToken}';` : ""}
+ c.env.RECOUP_API_KEY = ${JSON.stringify(apiKey)};
+ c.env.RECOUP_ACCOUNT_ID = ${JSON.stringify(accountId)};
+ ${githubToken ? `c.env.GITHUB_TOKEN = ${JSON.stringify(githubToken)};` : ""}
fs.writeFileSync(p, JSON.stringify(c, null, 2));
"`,
],
});JSON.stringify will properly escape any special characters, making the string safe for embedding in JavaScript code.
📝 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
| const injectEnv = await sandbox.runCommand({ | |
| cmd: "sh", | |
| args: [ | |
| "-c", | |
| `node -e " | |
| const fs = require('fs'); | |
| const p = require('os').homedir() + '/.openclaw/openclaw.json'; | |
| const c = JSON.parse(fs.readFileSync(p, 'utf8')); | |
| c.env = c.env || {}; | |
| c.env.RECOUP_API_KEY = '${apiKey}'; | |
| c.env.RECOUP_ACCOUNT_ID = '${accountId}'; | |
| ${githubToken ? `c.env.GITHUB_TOKEN = '${githubToken}';` : ""} | |
| fs.writeFileSync(p, JSON.stringify(c, null, 2)); | |
| "`, | |
| ], | |
| }); | |
| const injectEnv = await sandbox.runCommand({ | |
| cmd: "sh", | |
| args: [ | |
| "-c", | |
| `node -e " | |
| const fs = require('fs'); | |
| const p = require('os').homedir() + '/.openclaw/openclaw.json'; | |
| const c = JSON.parse(fs.readFileSync(p, 'utf8')); | |
| c.env = c.env || {}; | |
| c.env.RECOUP_API_KEY = ${JSON.stringify(apiKey)}; | |
| c.env.RECOUP_ACCOUNT_ID = ${JSON.stringify(accountId)}; | |
| ${githubToken ? `c.env.GITHUB_TOKEN = ${JSON.stringify(githubToken)};` : ""} | |
| fs.writeFileSync(p, JSON.stringify(c, null, 2)); | |
| "`, | |
| ], | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/sandbox/setup/setupOpenClaw.ts` around lines 67 - 82, The code in
injectEnv calling sandbox.runCommand builds a shell-invoked node -e string by
interpolating apiKey, accountId and githubToken directly, risking shell/JS
injection; fix it by avoiding direct interpolation—encode those values with
JSON.stringify (or pass them via stdin/temp file) and construct the injected
script using the JSON-escaped strings so special characters are safely escaped
before embedding into the node -e payload, updating the args payload logic
around sandbox.runCommand and the constructed command string accordingly.
Comment on lines
+14
to
+102
| export async function writeReadme( | ||
| sandbox: Sandbox, | ||
| sandboxId: string, | ||
| accountId: string, | ||
| githubRepo: string | undefined, | ||
| deps: SetupDeps, | ||
| ): Promise<void> { | ||
| const check = await sandbox.runCommand({ | ||
| cmd: "grep", | ||
| args: ["-q", "Recoup Sandbox", "README.md"], | ||
| }); | ||
|
|
||
| if (check.exitCode === 0) { | ||
| deps.log("README.md already has sandbox details, skipping"); | ||
| return; | ||
| } | ||
|
|
||
| const repoLine = githubRepo | ||
| ? `- **GitHub Repo**: ${githubRepo}` | ||
| : "- **GitHub Repo**: Not configured"; | ||
|
|
||
| const content = `# Recoup Sandbox | ||
|
|
||
| This is an isolated Linux microVM sandbox environment managed by [Recoup](https://recoupable.com). | ||
|
|
||
| ## Sandbox Details | ||
|
|
||
| - **Sandbox ID**: \`${sandboxId}\` | ||
| - **Account ID**: \`${accountId}\` | ||
| ${repoLine} | ||
|
|
||
| ## What is a Sandbox? | ||
|
|
||
| Sandboxes are ephemeral, isolated Linux microVM environments created via the [Recoup API](https://developers.recoupable.com/api-reference/sandboxes/create). They provide a safe space to execute code, run AI agent tasks, and evaluate generated output. | ||
|
|
||
| ### Key Features | ||
|
|
||
| - **Isolated execution** -- each sandbox runs in its own microVM | ||
| - **Snapshot support** -- sandbox state can be saved and restored from snapshots | ||
| - **GitHub integration** -- sandboxes can be linked to a GitHub repo for persistent file storage | ||
| - **Command execution** -- run any command with optional arguments and working directory | ||
| - **Automatic timeout** -- sandboxes stop automatically after a configurable timeout period | ||
|
|
||
| ### API | ||
|
|
||
| Create a sandbox via \`POST /api/sandboxes\`: | ||
|
|
||
| \`\`\`json | ||
| { | ||
| "command": "ls", | ||
| "args": ["-la", "/home"], | ||
| "cwd": "/home/user" | ||
| } | ||
| \`\`\` | ||
|
|
||
| The response includes sandbox status, snapshot ID, and GitHub repo if configured: | ||
|
|
||
| \`\`\`json | ||
| { | ||
| "status": "success", | ||
| "sandboxes": [ | ||
| { | ||
| "sandboxId": "sbx_abc123", | ||
| "sandboxStatus": "running", | ||
| "timeout": 300000, | ||
| "createdAt": "2024-01-15T10:30:00.000Z" | ||
| } | ||
| ], | ||
| "snapshot_id": "snap_abc123", | ||
| "github_repo": "https://github.com/org/repo" | ||
| } | ||
| \`\`\` | ||
|
|
||
| ### Docs | ||
|
|
||
| Full API documentation: https://developers.recoupable.com | ||
| `; | ||
|
|
||
| deps.log("Writing README.md to sandbox"); | ||
|
|
||
| await sandbox.writeFiles([ | ||
| { | ||
| path: "/vercel/sandbox/README.md", | ||
| content: Buffer.from(content), | ||
| }, | ||
| ]); | ||
|
|
||
| deps.log("README.md written successfully"); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Split writeReadme into smaller focused functions.
This function currently handles existence check, content construction, and file write in one block and exceeds the 50-line guideline for domain functions.
As per coding guidelines, "For domain functions, ensure: Single responsibility per function" and "Keep functions under 50 lines."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/sandbox/setup/writeReadme.ts` around lines 14 - 102, The writeReadme
function is doing three responsibilities (existence check, content construction,
and file write) and exceeds the 50-line/domain-function guideline; refactor by
extracting the grep check into a new async function (e.g., readmeExists or
checkReadmeContains) that calls sandbox.runCommand, the string assembly into a
buildReadmeContent(sandboxId, accountId, githubRepo) helper that returns the
README text, and the file write into a writeReadmeFile(sandbox, content) helper
that calls sandbox.writeFiles; then reduce writeReadme to orchestrate: call
readmeExists, bail if true, call buildReadmeContent and writeReadmeFile, and
keep logging via deps.log as before.
Comment on lines
+21
to
+24
| const check = await sandbox.runCommand({ | ||
| cmd: "grep", | ||
| args: ["-q", "Recoup Sandbox", "README.md"], | ||
| }); |
There was a problem hiding this comment.
Idempotency check targets a different path than the write path.
The check reads README.md in current CWD, but writes /vercel/sandbox/README.md. This can cause repeated rewrites on subsequent runs.
🔧 Suggested change
+const README_PATH = "/vercel/sandbox/README.md";
+
const check = await sandbox.runCommand({
cmd: "grep",
- args: ["-q", "Recoup Sandbox", "README.md"],
+ args: ["-q", "Recoup Sandbox", README_PATH],
});
...
await sandbox.writeFiles([
{
- path: "/vercel/sandbox/README.md",
+ path: README_PATH,
content: Buffer.from(content),
},
]);📝 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
| const check = await sandbox.runCommand({ | |
| cmd: "grep", | |
| args: ["-q", "Recoup Sandbox", "README.md"], | |
| }); | |
| const README_PATH = "/vercel/sandbox/README.md"; | |
| const check = await sandbox.runCommand({ | |
| cmd: "grep", | |
| args: ["-q", "Recoup Sandbox", README_PATH], | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/sandbox/setup/writeReadme.ts` around lines 21 - 24, The idempotency check
uses sandbox.runCommand(...) assigned to check to grep "README.md" in the
current working directory while the code later writes to
"/vercel/sandbox/README.md", causing repeated rewrites; update the grep target
to the same absolute path used for writing (i.e., grep -q "Recoup Sandbox"
"/vercel/sandbox/README.md") or derive the target from the same variable/path
used when writing the file so the check and write both reference the same file
(locate the call to sandbox.runCommand in writeReadme.ts and align its args to
the write path).
- Upgrade @vercel/sandbox from 1.3.1 to 1.8.0 to expose Snapshot.expiresAt - Add type assertion for setupFreshSandbox generator yield type narrowing - Add required expires_at field to upsertAccountSnapshot calls - Fix runOpenClawAgent command options type for stricter SDK types Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
♻️ Duplicate comments (6)
lib/sandbox/setup/helpers.ts (2)
13-129: 🛠️ Refactor suggestion | 🟠 MajorSplit this file into one exported function per module.
This file currently exports three functions, which breaks the repository SRP rule for
lib/**/*.ts. Please moverunGitCommand,runOpenClawAgent, andinstallSkillinto separate files and keep shared types in a smalltypesmodule if needed.As per coding guidelines, "
lib/**/*.ts: Apply Single Responsibility Principle (SRP): one exported function per file; each file should do one thing well."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/sandbox/setup/helpers.ts` around lines 13 - 129, This file exports three top-level functions (runGitCommand, runOpenClawAgent, installSkill) which violates the SRP rule; split each exported function into its own module (e.g., runGitCommand.ts, runOpenClawAgent.ts, installSkill.ts), move shared types (RunOpenClawAgentOptions, RunOpenClawAgentResult and any SetupDeps/Sandbox types referenced) into a small types module (e.g., types.ts), update imports/exports where these functions are consumed to reference the new files, and ensure each new file keeps the original function signature and error/logging behavior so callers need only update import paths.
98-118:⚠️ Potential issue | 🔴 CriticalPrevent command injection in skill copy step.
Line 117 interpolates
skillNameinto ash -ccommand. Without strict validation, crafted input can alter command execution.🔒 Minimal hardening diff
export async function installSkill( sandbox: Sandbox, skill: string, deps: SetupDeps, ): Promise<void> { + if (!/^[a-z0-9][a-z0-9-]*\/[a-z0-9][a-z0-9-]*$/i.test(skill)) { + throw new Error(`Invalid skill identifier: ${skill}`); + } const skillName = skill.split("/").pop()!;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/sandbox/setup/helpers.ts` around lines 98 - 118, The copy step is vulnerable to command injection because skillName is interpolated into a sh -c string passed to sandbox.runCommand; update the logic in the block that builds the copy command (the sandbox.runCommand call that currently uses cmd: "sh", args: ["-c", `...${skillName}...`]) to avoid shell interpolation—either validate/sanitize skillName to a strict allowed pattern (e.g., alphanumeric, hyphen/underscore) and reject unsafe values, or better, call sandbox.runCommand with a safe argv form (e.g., use cmd: "cp" and pass each path as a separate arg or use mkdir/rm/cp separately) so no untrusted data is embedded inside a shell string; ensure you reference skillName and the sandbox.runCommand invocation (the "copy" variable) when making the change.lib/sandbox/setup/ensureGithubRepo.ts (3)
83-86:⚠️ Potential issue | 🔴 CriticalDo not persist
GITHUB_TOKENin git remote/config.
Line 83-Line 86andLine 120-Line 128write tokenized auth into repo config paths that can be snapshotted. Use per-command auth (git -c url....insteadOf=...) and keeporiginas the plaingithubRepoURL.🔐 Suggested fix
- const repoUrl = githubRepo.replace( - "https://github.com/", - `https://x-access-token:${githubToken}@github.com/`, - ); @@ - if (!(await runGitCommand(sandbox, ["remote", "add", "origin", repoUrl], "add remote", deps))) { + if (!(await runGitCommand(sandbox, ["remote", "add", "origin", githubRepo], "add remote", deps))) { return undefined; } @@ - const fetchResult = await sandbox.runCommand({ - cmd: "git", - args: ["fetch", "origin"], - }); + const fetchResult = await sandbox.runCommand({ + cmd: "git", + args: [ + "-c", + `url.https://x-access-token:${githubToken}@github.com/.insteadOf=https://github.com/`, + "fetch", + "origin", + ], + }); @@ - // Set up URL rewriting for submodule clones - await sandbox.runCommand({ - cmd: "git", - args: [ - "config", - `url.https://x-access-token:${githubToken}@github.com/.insteadOf`, - "https://github.com/", - ], - }); - // Initialize submodules if they exist await sandbox.runCommand({ cmd: "git", - args: ["submodule", "update", "--init", "--recursive"], + args: [ + "-c", + `url.https://x-access-token:${githubToken}@github.com/.insteadOf=https://github.com/`, + "submodule", + "update", + "--init", + "--recursive", + ], });Also applies to: 92-93, 120-128
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/sandbox/setup/ensureGithubRepo.ts` around lines 83 - 86, The code currently builds a tokenized repo URL (repoUrl) and writes it into git config/remotes (affecting githubRepo, repoUrl and code paths around ensureGithubRepo.ts and the remote/checkout logic), which persists GITHUB_TOKEN; instead, stop replacing origin with a tokenized URL and keep origin set to the plain githubRepo, and run git operations with per-command auth using git -c url."https://x-access-token:${GITHUB_TOKEN}@github.com/".insteadOf="https://github.com/" (apply this pattern where repoUrl or tokenized URLs are used, including the spots around the repoUrl assignment and later remote/config updates referenced near lines 92-93 and 120-128) so the token is injected only for that git invocation and never written into repo config or remotes.
75-78:⚠️ Potential issue | 🟠 MajorValidate
originwhen.gitalready exists before early return.At
Line 75-Line 78, the function returns without confirmingoriginmatchesgithubRepo. A stale clone can silently target the wrong repository.🧩 Suggested fix
if (gitCheck.exitCode === 0) { + const originResult = await sandbox.runCommand({ + cmd: "git", + args: ["remote", "get-url", "origin"], + }); + if (originResult.exitCode !== 0) { + deps.error("Failed to read git origin remote"); + return undefined; + } + const currentOrigin = originResult.stdout.trim(); + if (currentOrigin !== githubRepo) { + if ( + !(await runGitCommand( + sandbox, + ["remote", "set-url", "origin", githubRepo], + "set origin remote", + deps, + )) + ) { + return undefined; + } + } deps.log("GitHub repo already cloned in sandbox"); return githubRepo; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/sandbox/setup/ensureGithubRepo.ts` around lines 75 - 78, The early return when gitCheck.exitCode === 0 should validate that the existing repository's origin matches the expected githubRepo URL; inside the same branch before calling deps.log/return, run a git remote get-url origin (e.g., via the same shell/exec helper used elsewhere) against the sandbox path and compare it to the expected githubRepo clone URL (the variable githubRepo); if they differ, either set the remote with git remote set-url origin <githubRepo> or remove/reclone the directory so it points to the correct repo, otherwise proceed to log and return as currently implemented.
103-113:⚠️ Potential issue | 🟠 MajorAvoid hardcoding
origin/main; detect remote default branch.
Line 105andLine 112assumemain. Repos using another default branch will skip checkout with a misleading “empty remote” path.🌿 Suggested fix
- const refCheck = await sandbox.runCommand({ - cmd: "git", - args: ["rev-parse", "--verify", "origin/main"], - }); + const headRef = await sandbox.runCommand({ + cmd: "git", + args: ["ls-remote", "--symref", "origin", "HEAD"], + }); + const defaultBranch = + headRef.exitCode === 0 + ? (headRef.stdout.match(/refs\/heads\/([^\s]+)/)?.[1] ?? "main") + : "main"; + const refCheck = await sandbox.runCommand({ + cmd: "git", + args: ["rev-parse", "--verify", `origin/${defaultBranch}`], + }); @@ - ["checkout", "-B", "main", "origin/main"], - "checkout main branch", + ["checkout", "-B", defaultBranch, `origin/${defaultBranch}`], + `checkout ${defaultBranch} branch`, deps, ))Also applies to: 135-137
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/sandbox/setup/ensureGithubRepo.ts` around lines 103 - 113, The code hardcodes "origin/main" when checking and checking out the remote branch; update ensureGithubRepo.ts to detect the remote's default branch instead of assuming "main": call sandbox.runCommand or runGitCommand to resolve the remote HEAD (e.g. use "git rev-parse --abbrev-ref origin/HEAD" or "git symbolic-ref refs/remotes/origin/HEAD") and parse the returned ref to derive the remote branch name (like origin/<branch>), then replace the literal "origin/main" usages in the refCheck, the runGitCommand checkout call and the similar occurrences around lines 135-137 to use the detected remote branch variable so checkout and verification work for repos whose default branch is not main.lib/sandbox/promptSandboxStreaming.ts (1)
84-101:⚠️ Potential issue | 🟠 MajorGuard post-prompt side effects so prompt results are never dropped.
At Line 90-Line 100, failures in GitHub push/snapshot/upsert can still throw after successful prompt execution and abort the generator. Wrap this block in
try/catchand log context instead of rethrowing.🛡️ Proposed fix
if (isFreshSandbox) { const deps: SetupDeps = { log: msg => console.log(`[PostSetup] ${msg}`), error: (msg, data) => console.error(`[PostSetup] ${msg}`, data), }; - await pushSandboxToGithub(sandbox, deps); - - const snapshotResult = await sandbox.snapshot(); - await upsertAccountSnapshot({ - account_id: accountId, - snapshot_id: snapshotResult.snapshotId, - expires_at: ( - snapshotResult.expiresAt ?? new Date(Date.now() + 7 * 24 * 60 * 60 * 1000) - ).toISOString(), - github_repo: githubRepo, - }); + try { + await pushSandboxToGithub(sandbox, deps); + + const snapshotResult = await sandbox.snapshot(); + await upsertAccountSnapshot({ + account_id: accountId, + snapshot_id: snapshotResult.snapshotId, + expires_at: ( + snapshotResult.expiresAt ?? new Date(Date.now() + 7 * 24 * 60 * 60 * 1000) + ).toISOString(), + github_repo: githubRepo, + }); + } catch (error) { + deps.error("Post-prompt setup failed; returning prompt result", { + sandboxId, + githubRepo, + error: error instanceof Error ? error.message : String(error), + }); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/sandbox/promptSandboxStreaming.ts` around lines 84 - 101, The post-prompt side-effect block guarded by isFreshSandbox (calling pushSandboxToGithub, sandbox.snapshot(), and upsertAccountSnapshot) must be wrapped in a try/catch so failures in pushSandboxToGithub, sandbox.snapshot, or upsertAccountSnapshot are caught and logged rather than allowed to propagate and abort the generator; update the code around isFreshSandbox to try { await pushSandboxToGithub(...); const snapshotResult = await sandbox.snapshot(); await upsertAccountSnapshot({...}); } catch (err) { log the error and relevant context (e.g., accountId, githubRepo, snapshotResult if available) via the existing deps/error or process logger but do not rethrow } so prompt results are preserved even if side effects fail.
🧹 Nitpick comments (2)
lib/sandbox/setup/ensureGithubRepo.ts (1)
18-144: SplitensureGithubRepointo focused units (SRP + size guideline).The function currently mixes account/snapshot reads, repo creation persistence, and git workspace orchestration in one long flow. This is hard to test and maintain.
As per coding guidelines:
lib/**/*.tsdomain functions should keep functions under 50 lines and have single responsibility.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/sandbox/setup/ensureGithubRepo.ts` around lines 18 - 144, ensureGithubRepo is too large and mixes snapshot/account lookup, repo creation/persistence, and git workspace orchestration; split it into focused helper functions to satisfy SRP and size limits. Extract: 1) snapshot/account read logic into a function like getAccountSnapshot(accountId) returning {snapshot, githubRepo, snapshotId, expiresAt}; 2) repo creation + persistence into createAndPersistGithubRepo(account, accountId, snapshotId, expiresAt) which calls createGithubRepo and upsertAccountSnapshot; and 3) git workspace steps into initializeGitWorkspace(sandbox, githubRepo, githubToken, deps) that handles git init, remote add, fetch/checkout, config and submodule update (use runGitCommand and sandbox.runCommand inside). Update ensureGithubRepo to orchestrate these helpers and keep it under ~50 lines.lib/sandbox/promptSandboxStreaming.ts (1)
31-101: Split this orchestrator into focused helpers to improve maintainability.
promptSandboxStreamingnow handles sandbox acquisition, setup streaming, command execution, and post-prompt persistence in one large path. Consider extracting these into small internal helpers to reduce complexity and make tests more targeted.As per coding guidelines,
lib/**/*.ts: “Apply Single Responsibility Principle (SRP): one exported function per file; each file should do one thing well” and “Keep functions under 50 lines.”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/sandbox/promptSandboxStreaming.ts` around lines 31 - 101, The function promptSandboxStreaming is doing too many things; split it into focused helpers: implement acquireSandbox(accountId) that wraps getOrCreateSandbox and returns {sandbox, sandboxId, created, fromSnapshot, isFreshSandbox}, implement streamSetupForFreshSandbox({sandbox, accountId, apiKey}) that drives setupFreshSandbox generator and yields setup log chunks and returns githubRepo, implement executeCommandStreaming({sandbox, prompt, apiKey, abortSignal}) that starts sandbox.runCommand, aggregates stdout/stderr while yielding cmd.logs and returns {exitCode, stdout, stderr}, and implement postPromptPersistence({sandbox, accountId, created, fromSnapshot, githubRepo}) that calls pushSandboxToGithub, sandbox.snapshot, and upsertAccountSnapshot; then refactor promptSandboxStreaming to orchestrate these helpers (calling acquireSandbox, conditionally streamSetupForFreshSandbox, call executeCommandStreaming, then call postPromptPersistence) so each helper is <50 lines and only promptSandboxStreaming remains exported from the file.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@lib/sandbox/promptSandboxStreaming.ts`:
- Around line 84-101: The post-prompt side-effect block guarded by
isFreshSandbox (calling pushSandboxToGithub, sandbox.snapshot(), and
upsertAccountSnapshot) must be wrapped in a try/catch so failures in
pushSandboxToGithub, sandbox.snapshot, or upsertAccountSnapshot are caught and
logged rather than allowed to propagate and abort the generator; update the code
around isFreshSandbox to try { await pushSandboxToGithub(...); const
snapshotResult = await sandbox.snapshot(); await upsertAccountSnapshot({...}); }
catch (err) { log the error and relevant context (e.g., accountId, githubRepo,
snapshotResult if available) via the existing deps/error or process logger but
do not rethrow } so prompt results are preserved even if side effects fail.
In `@lib/sandbox/setup/ensureGithubRepo.ts`:
- Around line 83-86: The code currently builds a tokenized repo URL (repoUrl)
and writes it into git config/remotes (affecting githubRepo, repoUrl and code
paths around ensureGithubRepo.ts and the remote/checkout logic), which persists
GITHUB_TOKEN; instead, stop replacing origin with a tokenized URL and keep
origin set to the plain githubRepo, and run git operations with per-command auth
using git -c
url."https://x-access-token:${GITHUB_TOKEN}@github.com/".insteadOf="https://github.com/"
(apply this pattern where repoUrl or tokenized URLs are used, including the
spots around the repoUrl assignment and later remote/config updates referenced
near lines 92-93 and 120-128) so the token is injected only for that git
invocation and never written into repo config or remotes.
- Around line 75-78: The early return when gitCheck.exitCode === 0 should
validate that the existing repository's origin matches the expected githubRepo
URL; inside the same branch before calling deps.log/return, run a git remote
get-url origin (e.g., via the same shell/exec helper used elsewhere) against the
sandbox path and compare it to the expected githubRepo clone URL (the variable
githubRepo); if they differ, either set the remote with git remote set-url
origin <githubRepo> or remove/reclone the directory so it points to the correct
repo, otherwise proceed to log and return as currently implemented.
- Around line 103-113: The code hardcodes "origin/main" when checking and
checking out the remote branch; update ensureGithubRepo.ts to detect the
remote's default branch instead of assuming "main": call sandbox.runCommand or
runGitCommand to resolve the remote HEAD (e.g. use "git rev-parse --abbrev-ref
origin/HEAD" or "git symbolic-ref refs/remotes/origin/HEAD") and parse the
returned ref to derive the remote branch name (like origin/<branch>), then
replace the literal "origin/main" usages in the refCheck, the runGitCommand
checkout call and the similar occurrences around lines 135-137 to use the
detected remote branch variable so checkout and verification work for repos
whose default branch is not main.
In `@lib/sandbox/setup/helpers.ts`:
- Around line 13-129: This file exports three top-level functions
(runGitCommand, runOpenClawAgent, installSkill) which violates the SRP rule;
split each exported function into its own module (e.g., runGitCommand.ts,
runOpenClawAgent.ts, installSkill.ts), move shared types
(RunOpenClawAgentOptions, RunOpenClawAgentResult and any SetupDeps/Sandbox types
referenced) into a small types module (e.g., types.ts), update imports/exports
where these functions are consumed to reference the new files, and ensure each
new file keeps the original function signature and error/logging behavior so
callers need only update import paths.
- Around line 98-118: The copy step is vulnerable to command injection because
skillName is interpolated into a sh -c string passed to sandbox.runCommand;
update the logic in the block that builds the copy command (the
sandbox.runCommand call that currently uses cmd: "sh", args: ["-c",
`...${skillName}...`]) to avoid shell interpolation—either validate/sanitize
skillName to a strict allowed pattern (e.g., alphanumeric, hyphen/underscore)
and reject unsafe values, or better, call sandbox.runCommand with a safe argv
form (e.g., use cmd: "cp" and pass each path as a separate arg or use
mkdir/rm/cp separately) so no untrusted data is embedded inside a shell string;
ensure you reference skillName and the sandbox.runCommand invocation (the "copy"
variable) when making the change.
---
Nitpick comments:
In `@lib/sandbox/promptSandboxStreaming.ts`:
- Around line 31-101: The function promptSandboxStreaming is doing too many
things; split it into focused helpers: implement acquireSandbox(accountId) that
wraps getOrCreateSandbox and returns {sandbox, sandboxId, created, fromSnapshot,
isFreshSandbox}, implement streamSetupForFreshSandbox({sandbox, accountId,
apiKey}) that drives setupFreshSandbox generator and yields setup log chunks and
returns githubRepo, implement executeCommandStreaming({sandbox, prompt, apiKey,
abortSignal}) that starts sandbox.runCommand, aggregates stdout/stderr while
yielding cmd.logs and returns {exitCode, stdout, stderr}, and implement
postPromptPersistence({sandbox, accountId, created, fromSnapshot, githubRepo})
that calls pushSandboxToGithub, sandbox.snapshot, and upsertAccountSnapshot;
then refactor promptSandboxStreaming to orchestrate these helpers (calling
acquireSandbox, conditionally streamSetupForFreshSandbox, call
executeCommandStreaming, then call postPromptPersistence) so each helper is <50
lines and only promptSandboxStreaming remains exported from the file.
In `@lib/sandbox/setup/ensureGithubRepo.ts`:
- Around line 18-144: ensureGithubRepo is too large and mixes snapshot/account
lookup, repo creation/persistence, and git workspace orchestration; split it
into focused helper functions to satisfy SRP and size limits. Extract: 1)
snapshot/account read logic into a function like getAccountSnapshot(accountId)
returning {snapshot, githubRepo, snapshotId, expiresAt}; 2) repo creation +
persistence into createAndPersistGithubRepo(account, accountId, snapshotId,
expiresAt) which calls createGithubRepo and upsertAccountSnapshot; and 3) git
workspace steps into initializeGitWorkspace(sandbox, githubRepo, githubToken,
deps) that handles git init, remote add, fetch/checkout, config and submodule
update (use runGitCommand and sandbox.runCommand inside). Update
ensureGithubRepo to orchestrate these helpers and keep it under ~50 lines.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
lib/sandbox/__tests__/promptSandboxStreaming.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**package.jsonis excluded by none and included by nonepnpm-lock.yamlis excluded by!**/pnpm-lock.yamland included by none
📒 Files selected for processing (3)
lib/sandbox/promptSandboxStreaming.tslib/sandbox/setup/ensureGithubRepo.tslib/sandbox/setup/helpers.ts
Catch APIError from Sandbox.create() and extract the actual API response body (json/text) so the real error reason is logged and included in the thrown error message. Previously only the generic "Status code 400 is not ok" was visible, losing the diagnostic details from the Vercel API. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
lib/sandbox/createSandbox.ts (1)
33-73: Extract error-shaping/logging into helpers to keepcreateSandboxfocused.
createSandboxnow handles defaults, source branching, SDK invocation, API error parsing, and logging in one block. Consider extracting error detail parsing + safe logging helpers to keep this domain function under the guideline size/complexity threshold.As per coding guidelines, "Keep functions under 50 lines" and "DRY: Consolidate similar logic into shared utilities."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/sandbox/createSandbox.ts` around lines 33 - 73, Extract the API error parsing and logging out of createSandbox into two helpers (e.g., parseApiError(error: unknown): string and safeLogApiError(error: APIError, context: { params: unknown }) : void); move the logic that recognizes APIError, extracts apiJson, builds detail ((apiJson?.error as Record<string, unknown>)?.message ?? error.text ?? error.message) and throws new Error(`Sandbox creation failed: ${detail}`) into parseApiError/throw flow, and move the console.error call (with status, json, text, params) into safeLogApiError; then simplify createSandbox to call Sandbox.create(createParams) inside try/catch that delegates to these helpers when error instanceof APIError and rethrows otherwise so createSandbox focuses only on defaults, branching, and SDK invocation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@lib/sandbox/createSandbox.ts`:
- Around line 33-73: Extract the API error parsing and logging out of
createSandbox into two helpers (e.g., parseApiError(error: unknown): string and
safeLogApiError(error: APIError, context: { params: unknown }) : void); move the
logic that recognizes APIError, extracts apiJson, builds detail ((apiJson?.error
as Record<string, unknown>)?.message ?? error.text ?? error.message) and throws
new Error(`Sandbox creation failed: ${detail}`) into parseApiError/throw flow,
and move the console.error call (with status, json, text, params) into
safeLogApiError; then simplify createSandbox to call
Sandbox.create(createParams) inside try/catch that delegates to these helpers
when error instanceof APIError and rethrows otherwise so createSandbox focuses
only on defaults, branching, and SDK invocation.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
lib/sandbox/__tests__/createSandbox.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**
📒 Files selected for processing (1)
lib/sandbox/createSandbox.ts
Comment on lines
+63
to
+68
| console.error("Sandbox.create failed", { | ||
| status: error.response?.status, | ||
| json: apiJson, | ||
| text: error.text, | ||
| params: createParams, | ||
| }); |
There was a problem hiding this comment.
Avoid logging raw sandbox create parameters.
Line 67 logs full createParams. These params can contain sensitive values (for example, credentials/secrets in env-like fields). Please log a redacted shape instead of raw input.
🔐 Suggested redaction patch
+ const redactedParams = JSON.parse(
+ JSON.stringify(createParams, (key, value) =>
+ /(token|secret|password|key)/i.test(key) ? "[REDACTED]" : value,
+ ),
+ );
+
console.error("Sandbox.create failed", {
status: error.response?.status,
json: apiJson,
text: error.text,
- params: createParams,
+ params: redactedParams,
});📝 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
| console.error("Sandbox.create failed", { | |
| status: error.response?.status, | |
| json: apiJson, | |
| text: error.text, | |
| params: createParams, | |
| }); | |
| const redactedParams = JSON.parse( | |
| JSON.stringify(createParams, (key, value) => | |
| /(token|secret|password|key)/i.test(key) ? "[REDACTED]" : value, | |
| ), | |
| ); | |
| console.error("Sandbox.create failed", { | |
| status: error.response?.status, | |
| json: apiJson, | |
| text: error.text, | |
| params: redactedParams, | |
| }); |
… repo creation - setupOpenClaw was using VERCEL_AI_GATEWAY_API_KEY which doesn't exist in the API environment, causing openclaw onboard to silently fail with "undefined" as the gateway key. Changed to AI_GATEWAY_API_KEY and added early validation that throws if missing. - Extracted shared createRepoInOrg() from createGithubRepo and createOrgGithubRepo to eliminate duplicate GitHub API fetch logic. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
♻️ Duplicate comments (1)
lib/sandbox/setup/setupOpenClaw.ts (1)
73-88:⚠️ Potential issue | 🟠 MajorSecurity concern: Shell injection risk remains unaddressed.
The
apiKey,accountId, andgithubTokenvalues are interpolated directly into the shell command string. If any of these contain shell metacharacters (e.g.,',$, backticks), the command could break or execute unintended code.The suggested fix using
JSON.stringifyfor safe escaping should be applied:🔒 Proposed fix using JSON.stringify
const injectEnv = await sandbox.runCommand({ cmd: "sh", args: [ "-c", `node -e " const fs = require('fs'); const p = require('os').homedir() + '/.openclaw/openclaw.json'; const c = JSON.parse(fs.readFileSync(p, 'utf8')); c.env = c.env || {}; - c.env.RECOUP_API_KEY = '${apiKey}'; - c.env.RECOUP_ACCOUNT_ID = '${accountId}'; - ${githubToken ? `c.env.GITHUB_TOKEN = '${githubToken}';` : ""} + c.env.RECOUP_API_KEY = ${JSON.stringify(apiKey)}; + c.env.RECOUP_ACCOUNT_ID = ${JSON.stringify(accountId)}; + ${githubToken ? `c.env.GITHUB_TOKEN = ${JSON.stringify(githubToken)};` : ""} fs.writeFileSync(p, JSON.stringify(c, null, 2)); "`, ], });,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/sandbox/setup/setupOpenClaw.ts` around lines 73 - 88, The current injectEnv command builds a shell string with apiKey, accountId, and githubToken directly interpolated, creating shell-injection risk; modify the sandbox.runCommand call that creates injectEnv so the sensitive values are serialized safely (e.g., use JSON.stringify(apiKey), JSON.stringify(accountId) and JSON.stringify(githubToken) inside the node -e payload) or pass a JSON payload via stdin instead of interpolating into the shell command, and keep the existing conditional for githubToken but embed it using the serialized value (refer to injectEnv and sandbox.runCommand to locate the change).
🧹 Nitpick comments (1)
lib/sandbox/setup/setupOpenClaw.ts (1)
13-119: Consider extracting phases into smaller helper functions.At ~106 lines, this function exceeds the 50-line guideline from the coding standards. The function has three distinct phases that could be extracted:
- Onboarding phase (lines 19-68): Check config and run
openclaw onboard- Env injection phase (lines 70-97): Inject environment variables into config
- Gateway startup phase (lines 99-118): Start the gateway process
This would improve readability and testability while adhering to the KISS principle.
♻️ Suggested structure
// lib/sandbox/setup/onboardOpenClaw.ts export async function onboardOpenClaw(sandbox: Sandbox, deps: SetupDeps): Promise<void> { ... } // lib/sandbox/setup/injectOpenClawEnv.ts export async function injectOpenClawEnv( sandbox: Sandbox, accountId: string, apiKey: string, deps: SetupDeps ): Promise<void> { ... } // lib/sandbox/setup/startOpenClawGateway.ts export async function startOpenClawGateway(sandbox: Sandbox, deps: SetupDeps): Promise<void> { ... } // lib/sandbox/setup/setupOpenClaw.ts - orchestrator export async function setupOpenClaw(...): Promise<void> { await onboardOpenClaw(sandbox, deps); await injectOpenClawEnv(sandbox, accountId, apiKey, deps); await startOpenClawGateway(sandbox, deps); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/sandbox/setup/setupOpenClaw.ts` around lines 13 - 119, The setupOpenClaw function is too large and should be split into three focused helpers: extract the onboarding phase into onboardOpenClaw(sandbox: Sandbox, deps: SetupDeps), the env injection phase into injectOpenClawEnv(sandbox: Sandbox, accountId: string, apiKey: string, deps: SetupDeps), and the gateway startup into startOpenClawGateway(sandbox: Sandbox, deps: SetupDeps); update setupOpenClaw to call these helpers in order, move the relevant sandbox.runCommand logic and logging/error handling into each new function, and keep existing behaviors and environment checks intact (use the same arg names and deps for easy replacement).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@lib/sandbox/setup/setupOpenClaw.ts`:
- Around line 73-88: The current injectEnv command builds a shell string with
apiKey, accountId, and githubToken directly interpolated, creating
shell-injection risk; modify the sandbox.runCommand call that creates injectEnv
so the sensitive values are serialized safely (e.g., use JSON.stringify(apiKey),
JSON.stringify(accountId) and JSON.stringify(githubToken) inside the node -e
payload) or pass a JSON payload via stdin instead of interpolating into the
shell command, and keep the existing conditional for githubToken but embed it
using the serialized value (refer to injectEnv and sandbox.runCommand to locate
the change).
---
Nitpick comments:
In `@lib/sandbox/setup/setupOpenClaw.ts`:
- Around line 13-119: The setupOpenClaw function is too large and should be
split into three focused helpers: extract the onboarding phase into
onboardOpenClaw(sandbox: Sandbox, deps: SetupDeps), the env injection phase into
injectOpenClawEnv(sandbox: Sandbox, accountId: string, apiKey: string, deps:
SetupDeps), and the gateway startup into startOpenClawGateway(sandbox: Sandbox,
deps: SetupDeps); update setupOpenClaw to call these helpers in order, move the
relevant sandbox.runCommand logic and logging/error handling into each new
function, and keep existing behaviors and environment checks intact (use the
same arg names and deps for easy replacement).
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
lib/sandbox/setup/__tests__/setupOpenClaw.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included bylib/**
📒 Files selected for processing (4)
lib/github/createGithubRepo.tslib/github/createOrgGithubRepo.tslib/github/createRepoInOrg.tslib/sandbox/setup/setupOpenClaw.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/github/createGithubRepo.ts
Contributor
Author
|
Closing in favor of a simpler approach: delegate to existing setup-sandbox task for fresh sandboxes instead of duplicating the setup pipeline inline. |
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
promptSandboxStreamingand runs the full setup pipeline inline before executing the user's promptapi/lib/sandbox/setup/with dependency injection for logging (reusable by tasks/ in a follow-up PR)api/lib/github/(sanitizeRepoName, createGithubRepo, createOrgGithubRepo, getExistingGithubRepo)Setup Pipeline (mirrors
runSandboxCommandTask)installOpenClawsetupOpenClawensureGithubRepowriteReadmeensureOrgReposensureSetupSandboxpushSandboxToGithubsandbox.snapshot()+upsertAccountSnapshot()Key Changes
createSandboxFromSnapshotnow returns{ sandbox, fromSnapshot }instead of justSandboxgetOrCreateSandboxpropagatesfromSnapshotbooleanpromptSandboxStreamingcheckscreated && !fromSnapshotto trigger inline setupTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit