Mount the sandbox scripts directory and fail loudly when sbx exec can't start a command - #12
Merged
Merged
Conversation
…t repo sbx exec runs sandbox-run.sh and sandbox-extract.sh by their host path, but the sandbox only sees host paths under a mounted workspace. sandbox create mounted only the target repo, so every test run failed whenever the target repo was not the skillwalker repo. sandbox create and sandbox update now also mount the scripts directory read-only, skipping it when it is already inside the target repo. Co-Authored-By: Claude <noreply@anthropic.com>
sbx exec prints "OCI runtime exec failed" and still exits 0 when the command cannot be started, so a test run reported [DONE] with zero tokens instead of failing. execInSandbox now throws SandboxError when it sees that message, and the CLI prints SandboxError as a clean error instead of a stack trace. Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
With no fail handler, yargs catches a rejected command handler itself: it prints help and the raw error (which Bun renders with a source snippet and stack trace), then exits before the catch in index.ts runs. The clean "Error: ..." output for SkillwalkerError and SandboxError never appeared. A .fail() handler now rethrows handler errors to that catch. Parse errors still print help and the message. Co-Authored-By: Claude <noreply@anthropic.com>
… run A sandbox created before the scripts mount was added keeps its original workspaces, so it passed "Checking sandbox..." and failed only at the first sbx exec. ensureSandboxExists now reads `sbx ls --json` and throws a SandboxError naming any required path the sandbox does not mount, with a hint to run `skillwalker sandbox update`. runEvals and the SCIL and ACIL loops require sandboxScriptsDir. Co-Authored-By: Claude <noreply@anthropic.com>
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
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.
Problem
Running an eval from a repo other than skillwalker (for example
han) failed on every test:There were two bugs:
sbx execrunssandbox-run.shandsandbox-extract.shby their host path, but the sandbox only sees host paths under a mounted workspace.sandbox createmounted only--repo-root, which is the target repo. So this failed whenever the target repo wasn't the skillwalker repo.sbx execcan't start the command, it printsOCI runtime exec failedand still exits 0. The test runner's exit-code check never fired, so the test showed[DONE]with zero tokens.Changes
createSandboxandupdateSandboxtake optionalextraWorkspaces, mounted read-only after the repo root (sbx run ... claude <repoRoot> <dir>:ro). A workspace already inside the repo root is skipped.sandbox createandsandbox updatepass the scripts directory:build/for the compiled binary,packages/claude-integration/from source. It's exported assandboxScriptsDirfrom a newclaude-integration/src/sandbox-scripts.ts, whichrun-claude.tsandextract-output-files.tsnow also use.execInSandboxthrowsSandboxErrorwhen any stdout or stderr line starts withOCI runtime exec failed. The error includes a hint to runskillwalker sandbox update.cli/index.tsprintsSandboxErrorasError: <message>and exits 1, instead of printing a stack trace.docs/sandbox-integration.mdanddocs/sandbox-integration-package.md.After merging
Existing sandboxes still have the old single mount. Run
skillwalker sandbox updatefrom the target repo once to recreate the sandbox with both mounts.Testing
make test: all 1022 tests pass, including new tests for the mount args, skipping nested workspaces, and detecting the exec failure. Detection matches only at the start of a line, so Claude output that merely mentions the message doesn't trigger it.make buildpass. Biome is clean on the changed files.sbx execexits 0 on a missing executable. Also confirmed that the newexecInSandboxthrows the expectedSandboxErroragainst a sandbox that's missing the mount.sandbox update. That step needs the interactive OAuth login.