Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 12 additions & 6 deletions docs/sandbox-integration-package.md
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,10 @@ class SandboxError extends Error {
#### ensureSandboxExists()

```typescript
async function ensureSandboxExists(): Promise<void>
async function ensureSandboxExists(requiredPaths: string[] = []): Promise<void>
```

Pre-flight check that the sandbox is running. Runs `sbx ls --quiet` and verifies `SANDBOX_NAME` exactly matches one output line. Throws `SandboxError` with `exitCode: null` if the sandbox is not found, with a message directing the user to run `./build/skillwalker sandbox create`.
Pre-flight check that the sandbox exists and mounts what the run needs. Runs `sbx ls --json`, finds the entry named `SANDBOX_NAME`, and checks that every path in `requiredPaths` is inside one of its workspaces (a trailing `:ro` on a listed workspace is ignored). `runEvals` and the SCIL and ACIL loops pass `[sandboxScriptsDir]`. A sandbox created before the scripts mount was added keeps its old workspaces, so this check fails it before any test runs instead of at the first `sbx exec`. Throws `SandboxError` with `exitCode: null` if the sandbox is not found, with a message directing the user to run `./build/skillwalker sandbox create`.

**Consumers:**
- `cli/src/commands/test-run.ts` -- before the per-eval test loop
Expand All @@ -98,17 +98,21 @@ Primary execution function. Builds and spawns the command `sbx exec claude-skill
- Both streams are fully captured regardless of the `debug` flag.
- Does not throw on non-zero exit codes; the caller inspects `SandboxResult.exitCode`.

`sbx exec` exits 0 even when it cannot start the command, printing `OCI runtime exec failed: ...` instead (for example, when the script's host path is outside every sandbox workspace). `execInSandbox` throws `SandboxError` when any stdout or stderr line starts with that message, pointing the user at `sandbox update`.

**Consumer:** `claude-integration/src/run-claude.ts` imports `execInSandbox` as the execution primitive for all Claude invocations inside the sandbox.

### lifecycle.ts -- Lifecycle Management

#### createSandbox()

```typescript
async function createSandbox(repoRoot: string): Promise<void>
async function createSandbox(repoRoot: string, extraWorkspaces: string[] = []): Promise<void>
```

Checks whether the sandbox already exists via an internal `sandboxExists()` helper (runs `sbx ls --quiet`). If found, prints a help message to stderr explaining how to recreate it, and returns early. Otherwise, spawns `sbx run --name claude-skills-skillwalker claude <repoRoot>` with inherited stdio for interactive OAuth login. Prints progress messages to stderr.
Checks whether the sandbox already exists via an internal `sandboxExists()` helper (runs `sbx ls --quiet`). If found, prints a help message to stderr explaining how to recreate it, and returns early. Otherwise, spawns `sbx run --name claude-skills-skillwalker claude <repoRoot> [<extraWorkspace>:ro ...]` with inherited stdio for interactive OAuth login. Prints progress messages to stderr.

`extraWorkspaces` are mounted read-only after `repoRoot` (`<path>:ro`); any already inside `repoRoot` are skipped. The CLI passes the directory holding `sandbox-run.sh` and `sandbox-extract.sh` (`sandboxScriptsDir` from `@testdouble/claude-integration`). `execInSandbox` runs those scripts by their host path, and the sandbox only sees host paths under a mounted workspace, so without this mount every test run fails whenever the target repo is not the skillwalker repo.

**Consumer:** `cli/src/commands/sandbox/create.ts`

Expand All @@ -125,14 +129,14 @@ Runs `sbx rm --force claude-skills-skillwalker`. Drains stdout and stderr in par
#### updateSandbox()

```typescript
async function updateSandbox(repoRoot: string): Promise<void>
async function updateSandbox(repoRoot: string, extraWorkspaces: string[] = []): Promise<void>
```

Replaces the sandbox with one built from the latest Claude Code template. `sbx` has no pull command and reuses a cached template image, so this function:

1. Removes the sandbox with `removeSandbox()`, if it exists.
2. Lists templates with `sbx template ls` and removes each cached image whose repository is `docker/sandbox-templates` and whose tag starts with `claude-code`, using `sbx template rm <image id>`.
3. Calls `createSandbox(repoRoot)`, which makes `sbx run` fetch the current template.
3. Calls `createSandbox(repoRoot, extraWorkspaces)`, which makes `sbx run` fetch the current template.

`sbx template ls` can list one image under several IDs, and removing the first ID removes them all. A later `rm` that reports `no template image` is therefore treated as already removed. Any other listing or removal failure throws `SandboxError`.

Expand Down Expand Up @@ -207,7 +211,9 @@ flowchart TB
| Scenario | Error Type | Behavior |
|----------|------------|----------|
| Sandbox not found by `ensureSandboxExists` | `SandboxError` (exitCode: `null`) | Thrown with message suggesting `./build/skillwalker sandbox create` |
| Required path not mounted, checked by `ensureSandboxExists` | `SandboxError` (exitCode: `null`) | Thrown naming the unmounted path, with a hint to run `skillwalker sandbox update` from the target repo |
| `sbx rm` fails | `SandboxError` (exitCode: process code) | Thrown with stdout+stderr in message |
| `sbx exec` prints `OCI runtime exec failed` (exits 0) | `SandboxError` (exitCode: process code) | Thrown with the sbx output and a hint to run `skillwalker sandbox update` from the target repo |
| Non-zero exit from `execInSandbox` | No error thrown | Returned in `SandboxResult.exitCode`; caller decides |
| `proc.exitCode` is null in `execInSandbox` | No error thrown | Defaults to `1` in `SandboxResult` |

Expand Down
22 changes: 16 additions & 6 deletions docs/sandbox-integration.md
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ export class SandboxError extends Error {

#### ensureSandboxExists

Pre-flight check that the sandbox is running. Runs `sbx ls --quiet` and verifies `SANDBOX_NAME` exactly matches one output line. Throws `SandboxError` with `exitCode: null` if not found.
Pre-flight check that the sandbox exists and mounts what the run needs. Runs `sbx ls --json`, finds the entry named `SANDBOX_NAME`, and checks that every path in `requiredPaths` is inside one of its workspaces (a trailing `:ro` on a listed workspace is ignored). `runEvals` and the SCIL and ACIL loops pass `[sandboxScriptsDir]`. A sandbox created before the scripts mount was added keeps its old workspaces, so this check fails it before any test runs instead of at the first `sbx exec`. Throws `SandboxError` with `exitCode: null` if not found.

Called by:
- `commands/test-run.ts` — before the per-eval test loop
Expand All @@ -124,6 +124,8 @@ export async function execInSandbox(
- stderr is drained in parallel via `new Response(stream).text()`. When `debug` is `true` and stderr is non-empty, it is written to `process.stderr`.
- Both streams are fully captured regardless of the `debug` flag.

`sbx exec` exits 0 even when it cannot start the command, printing `OCI runtime exec failed: ...` instead (for example, when the script's host path is outside every sandbox workspace). `execInSandbox` throws `SandboxError` when any stdout or stderr line starts with that message, pointing the user at `sandbox update`.

**Consumers and their claude args patterns:**

| Consumer | Key Args | Scaffold |
Expand Down Expand Up @@ -166,20 +168,22 @@ When a scaffold path is provided, it copies the scaffold into a fresh temp direc
#### createSandbox

```typescript
export async function createSandbox(repoRoot: string): Promise<void>
export async function createSandbox(repoRoot: string, extraWorkspaces: string[] = []): Promise<void>
```

Checks if the sandbox already exists via an internal `sandboxExists()` helper. If it does, prints a help message to stderr and returns. Otherwise, spawns `sbx run --name claude-skills-skillwalker claude <repoRoot>` with inherited stdio for interactive OAuth login.
Checks if the sandbox already exists via an internal `sandboxExists()` helper. If it does, prints a help message to stderr and returns. Otherwise, spawns `sbx run --name claude-skills-skillwalker claude <repoRoot> [<extraWorkspace>:ro ...]` with inherited stdio for interactive OAuth login.

`extraWorkspaces` are mounted read-only after `repoRoot` (`<path>:ro`); any already inside `repoRoot` are skipped. The CLI passes the directory holding `sandbox-run.sh` and `sandbox-extract.sh` (`sandboxScriptsDir` from `@testdouble/claude-integration`). `execInSandbox` runs those scripts by their host path, and the sandbox only sees host paths under a mounted workspace, so without this mount every test run fails whenever the target repo is not the skillwalker repo.

Called by `commands/sandbox/create.ts`.

#### updateSandbox

```typescript
export async function updateSandbox(repoRoot: string): Promise<void>
export async function updateSandbox(repoRoot: string, extraWorkspaces: string[] = []): Promise<void>
```

Removes the sandbox if it exists, then removes every cached `docker/sandbox-templates` image tagged `claude-code*` (found with `sbx template ls`). Finally it calls `createSandbox`, so `sbx run` fetches the latest Claude Code template. `sbx` has no pull command, so deleting the cached image is the only way to get a newer one. An `rm` that reports `no template image` counts as already removed, because `sbx template ls` can list one image under several IDs. Any other listing or removal failure throws `SandboxError`.
Removes the sandbox if it exists, then removes every cached `docker/sandbox-templates` image tagged `claude-code*` (found with `sbx template ls`). Finally it calls `createSandbox` with the same arguments, so `sbx run` fetches the latest Claude Code template. `sbx` has no pull command, so deleting the cached image is the only way to get a newer one. An `rm` that reports `no template image` counts as already removed, because `sbx template ls` can list one image under several IDs. Any other listing or removal failure throws `SandboxError`.

Called by `commands/sandbox/update.ts`, which catches `SandboxError` and re-throws as `SkillwalkerError`.

Expand Down Expand Up @@ -223,7 +227,9 @@ See [Cross-Runtime Meta Property Resolution](coding-standards/cross-runtime-meta
| Scenario | Error Type | Behavior |
|----------|------------|----------|
| Sandbox not found by `ensureSandboxExists` | `SandboxError` (exitCode: `null`) | Thrown with message suggesting `./build/skillwalker sandbox create` |
| Required path not mounted, checked by `ensureSandboxExists` | `SandboxError` (exitCode: `null`) | Thrown naming the unmounted path, with a hint to run `skillwalker sandbox update` from the target repo |
| `sbx rm` fails | `SandboxError` (exitCode: process code) | Thrown with stdout+stderr in message |
| `sbx exec` prints `OCI runtime exec failed` (exits 0) | `SandboxError` (exitCode: process code) | Thrown with the sbx output and a hint to run `skillwalker sandbox update` from the target repo |
| Non-zero exit code from `execInSandbox` | No error thrown | Returned in `SandboxResult.exitCode`; caller decides |
| `execInSandbox` with `proc.exitCode` null | No error thrown | `exitCode` defaults to `1` in `SandboxResult` |

Expand All @@ -232,7 +238,7 @@ See [Cross-Runtime Meta Property Resolution](coding-standards/cross-runtime-meta
| Layer | Pattern |
|-------|---------|
| CLI commands (`clean.ts`) | Catches `SandboxError`, re-throws as `SkillwalkerError` |
| Pre-flight checks (`test-run.ts`, `loop.ts`) | No catch — `SandboxError` propagates and crashes the process |
| Pre-flight checks (`test-run.ts`, `loop.ts`) and test runners | No catch — `SandboxError` propagates to `cli/index.ts`, which prints `Error: <message>` and exits 1 |
| Test runners (`prompt/`, `skill-call/`) | Checks `exitCode` on `SandboxResult`, increments failure counter |
| LLM judge (`step-3b`) | Catches all errors, records `status: 'infrastructure-error'` in results |
| SCIL step-5 | Catches errors per work-item, logs to stderr, continues |
Expand Down Expand Up @@ -281,6 +287,10 @@ If `ensureSandboxExists` throws `SandboxError`, run:
1. `./build/skillwalker sandbox create` — creates the sandbox and completes OAuth
2. Verify with `sbx ls --quiet` — should list `claude-skills-skillwalker`

### Sandbox does not mount a required path

If `ensureSandboxExists` reports that the sandbox does not mount a path, the sandbox predates that mount. From the target repo, run `./build/skillwalker sandbox update`, then verify with `sbx ls --json` that `claude-skills-skillwalker` lists both the target repo and the scripts directory.

### Sandbox already exists during setup

`createSandbox` returns early with a help message. To recreate:
Expand Down
1 change: 1 addition & 0 deletions packages/claude-integration/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,5 @@ export type { OutputFile } from './src/extract-output-files.js'
export { extractOutputFiles } from './src/extract-output-files.js'
export { resolvePluginDirs } from './src/plugin-flags.js'
export { runClaude } from './src/run-claude.js'
export { sandboxScriptsDir } from './src/sandbox-scripts.js'
export type { ClaudeRunOptions, ClaudeRunResult } from './src/types.js'
6 changes: 2 additions & 4 deletions packages/claude-integration/src/extract-output-files.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,13 @@
import { resolveRelativePath } from '@testdouble/bun-helpers'
import { execInSandbox } from '@testdouble/sandbox-integration'
import { sandboxExtractScript } from './sandbox-scripts.js'

export interface OutputFile {
path: string
content: string
}

const extractScript = resolveRelativePath(import.meta, '../sandbox-extract.sh', 'sandbox-extract.sh')

export async function extractOutputFiles(debug: boolean): Promise<OutputFile[]> {
const { stdout } = await execInSandbox(extractScript, [], null, debug)
const { stdout } = await execInSandbox(sandboxExtractScript, [], null, debug)

if (!stdout.trim()) return []

Expand Down
4 changes: 1 addition & 3 deletions packages/claude-integration/src/run-claude.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
import { resolveRelativePath } from '@testdouble/bun-helpers'
import { execInSandbox } from '@testdouble/sandbox-integration'
import { sandboxRunScript } from './sandbox-scripts.js'
import type { ClaudeRunOptions, ClaudeRunResult } from './types.js'

const sandboxRunScript = resolveRelativePath(import.meta, '../sandbox-run.sh', 'sandbox-run.sh')

export async function runClaude(options: ClaudeRunOptions): Promise<ClaudeRunResult> {
const { model, prompt, pluginDirs = [], scaffold = null, debug = false } = options

Expand Down
12 changes: 12 additions & 0 deletions packages/claude-integration/src/sandbox-scripts.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import path from 'node:path'
import { resolveRelativePath } from '@testdouble/bun-helpers'

export const sandboxRunScript = resolveRelativePath(import.meta, '../sandbox-run.sh', 'sandbox-run.sh')
export const sandboxExtractScript = resolveRelativePath(import.meta, '../sandbox-extract.sh', 'sandbox-extract.sh')

/**
* Directory holding the scripts `sbx exec` runs by their host path. The sandbox
* only sees host paths under a mounted workspace, so this directory must be
* mounted alongside the target repo.
*/
export const sandboxScriptsDir = path.dirname(sandboxRunScript)
12 changes: 10 additions & 2 deletions packages/cli/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#!/usr/bin/env bun
import { SandboxError } from '@testdouble/sandbox-integration'
import { SkillwalkerError } from '@testdouble/skillwalker-execution'
import yargs from 'yargs'
import { hideBin } from 'yargs/helpers'
Expand All @@ -14,10 +15,17 @@ try {
.command(await import('./src/commands/acil.js'))
.demandCommand(1)
.strict()
.showHelpOnFail(true)
// Rethrow handler errors to the catch below. Without this, yargs prints
// help and the raw error for them and exits before the catch runs.
.fail((message, error, cli) => {
if (error) throw error
cli.showHelp()
process.stderr.write(`\n${message}\n`)
process.exit(1)
})
.parseAsync()
} catch (err) {
if (err instanceof SkillwalkerError) {
if (err instanceof SkillwalkerError || err instanceof SandboxError) {
process.stderr.write(`Error: ${err.message}\n`)
process.exit(1)
}
Expand Down
10 changes: 10 additions & 0 deletions packages/cli/src/command-registration.integration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,3 +57,13 @@ describe('sandbox sub-command registration', () => {
expect(output).toContain('Unknown argument: bogus')
})
})

describe('command handler errors', () => {
it('prints a domain error as a single Error line without help text', () => {
const { status, output } = runCli('test-eval', 'no-such-run-id')
expect(status).toBe(1)
expect(output).toMatch(/^Error: Test run directory not found:/m)
expect(output).not.toContain('Options:')
expect(output).not.toContain('RunNotFoundError')
})
})
8 changes: 6 additions & 2 deletions packages/cli/src/commands/sandbox/create.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@ vi.mock('@testdouble/sandbox-integration', () => ({
createSandbox: vi.fn(),
}))

vi.mock('@testdouble/claude-integration', () => ({
sandboxScriptsDir: '/skillwalker/build',
}))

import { createSandbox } from '@testdouble/sandbox-integration'
import { builder, command, describe as commandDescribe, handler } from './create.js'

Expand Down Expand Up @@ -47,8 +51,8 @@ describe('sandbox create builder', () => {
})

describe('sandbox create handler', () => {
it('calls createSandbox with the resolved repo-root', async () => {
it('calls createSandbox with the resolved repo-root and the sandbox scripts directory', async () => {
await handler({ 'repo-root': '/repo/root' })
expect(vi.mocked(createSandbox)).toHaveBeenCalledWith('/repo/root')
expect(vi.mocked(createSandbox)).toHaveBeenCalledWith('/repo/root', ['/skillwalker/build'])
})
})
3 changes: 2 additions & 1 deletion packages/cli/src/commands/sandbox/create.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { sandboxScriptsDir } from '@testdouble/claude-integration'
import { createSandbox } from '@testdouble/sandbox-integration'
import type { Argv } from 'yargs'

Expand All @@ -13,5 +14,5 @@ export function builder(yargs: Argv): Argv {
}

export async function handler(argv: Record<string, unknown>): Promise<void> {
await createSandbox(argv['repo-root'] as string)
await createSandbox(argv['repo-root'] as string, [sandboxScriptsDir])
}
6 changes: 5 additions & 1 deletion packages/cli/src/commands/sandbox/update.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ vi.mock('@testdouble/sandbox-integration', () => ({
},
}))

vi.mock('@testdouble/claude-integration', () => ({
sandboxScriptsDir: '/skillwalker/build',
}))

import { SandboxError, updateSandbox } from '@testdouble/sandbox-integration'
import { SkillwalkerError } from '@testdouble/skillwalker-execution'
import { builder, command, describe as commandDescribe, handler } from './update.js'
Expand Down Expand Up @@ -58,7 +62,7 @@ describe('sandbox update builder', () => {
describe('sandbox update handler', () => {
it('calls updateSandbox with the resolved repo-root', async () => {
await handler({ 'repo-root': '/repo/root' })
expect(vi.mocked(updateSandbox)).toHaveBeenCalledWith('/repo/root')
expect(vi.mocked(updateSandbox)).toHaveBeenCalledWith('/repo/root', ['/skillwalker/build'])
})

it('throws SkillwalkerError when updateSandbox throws SandboxError', async () => {
Expand Down
3 changes: 2 additions & 1 deletion packages/cli/src/commands/sandbox/update.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { sandboxScriptsDir } from '@testdouble/claude-integration'
import { SandboxError, updateSandbox } from '@testdouble/sandbox-integration'
import { SkillwalkerError } from '@testdouble/skillwalker-execution'
import type { Argv } from 'yargs'
Expand All @@ -15,7 +16,7 @@ export function builder(yargs: Argv): Argv {

export async function handler(argv: Record<string, unknown>): Promise<void> {
try {
await updateSandbox(argv['repo-root'] as string)
await updateSandbox(argv['repo-root'] as string, [sandboxScriptsDir])
} catch (error) {
if (error instanceof SandboxError) {
throw new SkillwalkerError(error.message)
Expand Down
7 changes: 7 additions & 0 deletions packages/execution/src/acil/loop.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ vi.mock('node:readline/promises', () => ({
}))

import { createInterface } from 'node:readline/promises'
import { sandboxScriptsDir } from '@testdouble/claude-integration'
import { ensureSandboxExists } from '@testdouble/sandbox-integration'
import { getPhase } from '@testdouble/skillwalker-data'
import { generateRunId } from '../test-runners/steps/step-4-generate-run-id.js'
Expand Down Expand Up @@ -147,6 +148,12 @@ beforeEach(() => {
})

describe('runAcilLoop', () => {
it('requires the sandbox to mount the sandbox scripts directory', async () => {
await runAcilLoop(makeConfig({ maxIterations: 1, holdout: 0 }))

expect(ensureSandboxExists).toHaveBeenCalledWith([sandboxScriptsDir])
})

it('exits after one iteration when train accuracy is 1.0 and holdout is 0', async () => {
await runAcilLoop(makeConfig({ maxIterations: 5, holdout: 0 }))

Expand Down
Loading
Loading