perf: prebuild local sandbox images

[joshblack]

Follow up to #212.

This PR prebuilds a reusable local sandbox image through Dockerode before running trials. The configured Debian-based Node image remains the base layer, so compatible custom images with npm, apt-get, and a node user continue to work without publishing the derived sandbox image to a registry.

This is the third entry in stack 213. #199 adds the design system benchmark scenarios, #203 aligns the benchmark and experiment runner workflows, and #204 adds durable plan sharding.

Changelog

New

• Add a local multi-stage sandbox image build containing npm, Copilot CLI, Chromium, and the shared sandbox directories.
• Add deterministic local image tags and reuse in-process builds for matching base images.
• Add process-level tracking for active sandbox containers.

Changed

• Treat --docker-image as a Debian-based Node base image with npm, apt-get, and a node user for the generated sandbox layer.
• Create each trial container from the prebuilt local image instead of repeating sandbox setup.
• Keep agent-browser and its skill installation isolated to the walkthrough phase.
• Remove active sandbox containers before exiting on SIGINT or SIGTERM.

Removed

• Remove per-container image pulls and repeated npm, Copilot, and Chromium installation.

[Copilot]

Copilot review overview

🔵 Needs a closer look

It changes core sandbox container/image lifecycle behavior (Docker build + signal cleanup), which is operationally sensitive and is best validated with human review and real-run testing.

Review tier: Lite
Findings: None

What changed in this PR

This PR improves evaluation runtime performance by prebuilding and reusing a local “sandbox layer” Docker image (via Dockerode) that contains the common sandbox dependencies (npm, Copilot CLI, Chromium, and shared directories). It also adds process-level tracking and cleanup of active sandbox containers on termination signals.

Changes:

• Build a deterministic, reusable local sandbox image layered on top of --docker-image (now treated as the base image) and reuse the build within the process.
• Remove per-container bootstrapping (package installs / directory setup) and create trial containers from the prebuilt local image.
• Track active sandbox containers and force-remove them on SIGINT/SIGTERM, plus update CLI/workflow docs and add a changeset.

File	Description
script/​run-benchmark.sh	Passes --docker-image (base image) into benchmark runs, with a default.
packages/​agent-eval/​src/​trial.ts	Removes per-walkthrough Chromium installation from trial execution.
packages/​agent-eval/​src/​sandbox/​system.ts	Adds Dockerfile-based local image build, deterministic tagging, build reuse, and signal-driven container cleanup.
packages/​agent-eval/​src/​sandbox/​system.test.ts	Adds coverage for image build and termination cleanup behavior; updates lifecycle mocks.
packages/​agent-eval/​src/​cli.ts	Clarifies --docker-image semantics as a base layer rather than the final runtime image.
.github/​workflows/​experiment.yml	Updates workflow input documentation to match new docker-image semantics.
.github/​workflows/​benchmark.yml	Updates workflow input documentation to match new docker-image semantics.
.changeset/​puny-houses-arrive.md	Publishes a patch note about cleaning up active containers on termination.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Copilot review overview

🟡 Changes recommended

The updated --docker-image documentation in the CLI and workflows understates the requirement that the base image must include Node/npm, which can lead to misconfiguration and failed sandbox builds.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review tier: Lite
Findings: 3

New issues introduced by this change (3)

Severity	Finding
	.github/​workflows/​benchmark.yml — This workflow input description drops the requirement that the base image includes Node/npm, but…
	.github/​workflows/​experiment.yml — This workflow input description should continue to require a Debian-based Node image (with npm)…
	packages/​agent-eval/​src/​cli.ts — The --docker-image flag is now described as only requiring a Debian base with apt-get and a…

Issues resolved since last review (3)

Severity	Finding
	.changeset/​puny-houses-arrive.md — This changeset only mentions SIGINT/SIGTERM container cleanup, but the PR also changes how… View resolved comment
	packages/​agent-eval/​src/​sandbox/​system.ts — removeContainer treats any container.remove failure as fatal. With AutoRemove enabled, a container… View resolved comment
	packages/​agent-eval/​src/​sandbox/​system.ts — getDockerImageName only hashes the base image string + npm/copilot versions. If the Dockerfile… View resolved comment

Suppressed comments (1)

packages/agent-eval/src/cli.ts:92

• The usage text for --docker-image no longer mentions that the base must include Node/npm. Since the sandbox image build runs npm inside the base stage, passing a non-Node Debian image will fail; the help line should reflect the Node requirement to avoid misconfiguration.

  -b, --benchmark <file>     The file name of the benchmark to run
      --benchmarks <dir>     The directory containing local benchmark files (default: ./benchmarks)
  -c, --concurrency <num>    The number of treatments to run in parallel
      --docker-image <image> The Docker base image to layer the treatment environment on (default: node:26.5.0-slim)
  -e, --experiment <file>    The file name of the experiment to run

[Copilot]

Copilot review overview

🟡 Changes recommended

The SIGINT/SIGTERM cleanup relies on process.once, so a second signal during cleanup can terminate the process without handlers and leave active sandbox containers behind.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review tier: Lite
Findings: 1

New issues introduced by this change (1)

Severity	Finding
	packages/​agent-eval/​src/​sandbox/​system.ts — trackContainer registers SIGINT/SIGTERM handlers with process.once, which removes the listener…

Issues resolved since last review (3)

Severity	Finding
	packages/​agent-eval/​src/​cli.ts — The --docker-image flag is now described as only requiring a Debian base with apt-get and a… View resolved comment
	.github/​workflows/​experiment.yml — This workflow input description should continue to require a Debian-based Node image (with npm)… View resolved comment
	.github/​workflows/​benchmark.yml — This workflow input description drops the requirement that the base image includes Node/npm, but… View resolved comment

[Copilot]

Copilot review overview

🔵 Needs a closer look

Trial file read/write currently performs unbounded concurrent filesystem operations, which can fail or become unstable for large runs due to open-file limits and I/O contention.

Review tier: Lite
Findings: 1

Pre-existing issues (1)

Severity	Finding
	packages/​agent-eval/​src/​sandbox/​system.ts — trackContainer registers SIGINT/SIGTERM handlers with process.once, which removes the listener… View comment

Suppressed comments (2)

packages/agent-eval/src/trial.ts:187

• writeTrialFiles writes all trial JSON files using an unbounded Promise.all over the full trials map. For larger runs this can open many files concurrently and hit OS FD limits (EMFILE) or create significant I/O contention. Consider batching/limiting concurrency when writing trial files.

This issue also appears on line 200 of the same file.

  const entries = await Promise.all(
    [...trials].map(async ([trialId, trial]) => {
      if (trial.id !== trialId) {
        throw new Error(`Trial map key "${trialId}" does not match trial id "${trial.id}"`)
      }

      const artifactDirectory = path.isAbsolute(trial.artifacts.directory)
        ? trial.artifacts.directory
        : path.resolve(outputDirectory, trial.artifacts.directory)
      const trialFilePath = await preparePathWithinDirectory(
        host,
        outputDirectory,
        path.relative(outputDirectory, path.join(artifactDirectory, `${trialId}.json`)),
        `Trial file for "${trialId}"`,
      )
      await host.fs.writeFile(trialFilePath, JSON.stringify(trial), 'utf-8')
      const reference = path.relative(outputDirectory, trialFilePath).split(path.sep).join(path.posix.sep)
      return [trialId, reference] as const
    }),
  )

packages/agent-eval/src/trial.ts:222

• readTrialFiles reads all referenced trial files using an unbounded Promise.all across Object.entries(references). For runs with many trials this can trigger EMFILE/open-file limits or overwhelm the FS. Consider batching/limiting concurrency when loading the trial files.

  const entries = await Promise.all(
    Object.entries(references).map(async ([trialId, reference]) => {
      const trialFilePath = await resolveExistingPathWithinDirectory(
        host,
        outputDirectory,
        reference,
        `Trial reference for "${trialId}"`,
      )
      const contents = await host.fs.readFile(trialFilePath, 'utf-8')
      const trial = parse(JSON.parse(contents))
      if (
        typeof trial !== 'object' ||
        trial === null ||
        !('id' in trial) ||
        typeof trial.id !== 'string' ||
        trial.id !== trialId
      ) {
        throw new Error(`Trial file "${reference}" does not contain trial id "${trialId}"`)
      }

      return [trialId, trial] as const
    }),
  )

[Copilot]

Copilot review overview

🟡 Changes recommended

The new sandbox image build path no longer ensures the base image tag is refreshed, which can cause local runs to drift onto stale base layers unless the build explicitly pulls updated bases.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review tier: Lite
Findings: 2

New issues introduced by this change (1)

Severity	Finding
	packages/​agent-eval/​src/​sandbox/​system.ts — The sandbox image build no longer forces a refresh of the configured base image. Previously, the…

Pre-existing issues (1)

Severity	Finding
	packages/​agent-eval/​src/​sandbox/​system.ts — trackContainer registers SIGINT/SIGTERM handlers with process.once, which removes the listener… View comment