perf: prebuild local sandbox images

.changeset/puny-houses-arrive.md

@@ -0,0 +1,5 @@
+---
+'@primer/agent-eval': patch
+---
+
+Prebuild and reuse a local sandbox image from the configured `--docker-image` base, and remove active sandbox containers when an evaluation process receives SIGINT or SIGTERM.

.github/workflows/benchmark.yml

@@ -11,8 +11,8 @@ on:
         type: string
       docker-image:
         description: >-
-          Docker container image to use for running trials.
-          Must be a Debian-based Node image with apt-get and a node user (e.g. node:26.5.0-slim).
+          Docker base image to layer the trial environment on.
+          Must be a Debian-based Node image with npm, apt-get, and a node user.
         required: false
         default: 'node:26.5.0-slim'
         type: string

.github/workflows/experiment.yml

@@ -13,8 +13,8 @@ on:
         type: string
       docker-image:
         description: >-
-          Docker container image to use for running treatments.
-          Must be a Debian-based Node image with apt-get and a node user (e.g. node:26.5.0-slim).
+          Docker base image to layer the treatment environment on.
+          Must be a Debian-based Node image with npm, apt-get, and a node user.
         required: false
         default: 'node:26.5.0-slim'
         type: string

packages/agent-eval/src/cli.ts

@@ -40,7 +40,7 @@ const {values} = parseArgs({
     'docker-image': {
       type: 'string',
       description:
-        'The Docker container image to use for running treatments (must be a Debian-based Node image with apt-get and a node user, e.g. node:26.5.0-slim)',
+        'The Docker base image to layer the treatment environment on (must be a Debian-based Node image with npm, apt-get, and a node user, default: node:26.5.0-slim)',
     },
     experiment: {
       type: 'string',
@@ -87,7 +87,7 @@ Options:
   -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 container image to use for running treatments (must be a Debian-based Node image with apt-get and a node user, e.g. node:26.5.0-slim)
+      --docker-image <image> The Docker base image to layer the treatment environment on (must be a Debian-based Node image with npm, apt-get, and a node user; default: node:26.5.0-slim)
   -e, --experiment <file>    The file name of the experiment to run
       --experiments <dir>    The directory containing local experiment files (default: ./experiments)
   -h, --help                 Learn more about the command and its options

packages/agent-eval/src/sandbox/system.test.ts

@@ -2,10 +2,17 @@ import Docker from 'dockerode'
 import {beforeEach, describe, expect, test, vi} from 'vitest'
 import {VirtualHost} from '../host'
 import {MCP_CONFIG_PATH, NODE_USER, SKILLS_DIR} from './constants'
-import {createContainer, SandboxSchema, SystemSandbox} from './system'
+import {
+  buildDockerImage,
+  cleanupActiveContainers,
+  createContainer,
+  getDockerImageName,
+  SandboxSchema,
+  SystemSandbox,
+} from './system'
 import {VirtualSandbox} from './virtual'
 
-function createSandbox(container = {remove: vi.fn()}) {
+function createSandbox(container = {remove: vi.fn().mockResolvedValue(undefined)}) {
   // @ts-expect-error This test only exercises methods whose container operations are mocked.
   return new SystemSandbox(VirtualHost.create(), new Docker(), container)
 }
@@ -24,9 +31,47 @@ describe('SandboxSchema', () => {
 })
 
 describe('SystemSandbox lifecycle', () => {
+  test('builds the local sandbox image', async () => {
+    const stream = {}
+    const docker = {
+      buildImage: vi.fn().mockResolvedValue(stream),
+      modem: {
+        followProgress: vi.fn((_stream: unknown, onFinished: (error: Error | null) => void) => {
+          onFinished(null)
+        }),
+      },
+    }
+
+    // @ts-expect-error This test only exercises the Docker methods used to build the image.
+    const image = await buildDockerImage(docker, 'custom-node:local')
+
+    expect(docker.buildImage).toHaveBeenCalledWith(
+      expect.anything(),
+      expect.objectContaining({
+        buildargs: {
+          BASE_IMAGE: 'custom-node:local',
+          COPILOT_CLI_VERSION: '1.0.82',
+          NPM_VERSION: '12.0.2',
+        },
+        dockerfile: 'Dockerfile',
+        t: expect.stringMatching(/^agent-eval-sandbox:[a-f0-9]{16}$/),
+        target: 'sandbox',
+      }),
+    )
+    expect(docker.modem.followProgress).toHaveBeenCalledWith(stream, expect.any(Function))
+    expect(image).toMatch(/^agent-eval-sandbox:[a-f0-9]{16}$/)
+  })
+
+  test('includes the Dockerfile contents in the local image tag', () => {
+    const firstImage = getDockerImageName('custom-node:local', 'FROM custom-node:local\nRUN echo first')
+    const secondImage = getDockerImageName('custom-node:local', 'FROM custom-node:local\nRUN echo second')
+
+    expect(firstImage).not.toBe(secondImage)
+  })
+
   test('force removes the container when disposed', async () => {
     const container = {
-      remove: vi.fn(),
+      remove: vi.fn().mockResolvedValue(undefined),
     }
     const sandbox = createSandbox(container)
 
@@ -43,20 +88,63 @@ describe('SystemSandbox lifecycle', () => {
     }
     const docker = {
       createContainer: vi.fn().mockResolvedValue(container),
-      pull: vi.fn((_name: string, callback: (error: Error | null, stream: NodeJS.ReadableStream) => void) => {
-        callback(null, {} as NodeJS.ReadableStream)
-      }),
-      modem: {
-        followProgress: vi.fn((_stream: NodeJS.ReadableStream, onFinished: (error: Error | null) => void) => {
-          onFinished(null)
-        }),
-      },
     }
 
     // @ts-expect-error This test only exercises the Docker methods used before container initialization.
     await expect(createContainer(docker, 'test-image')).rejects.toBe(initializationError)
     expect(container.remove).toHaveBeenCalledWith({force: true})
   })
+
+  test('removes active containers when the process is terminated', async () => {
+    const container = {
+      start: vi.fn(),
+      remove: vi.fn().mockResolvedValue(undefined),
+    }
+    const docker = {
+      createContainer: vi.fn().mockResolvedValue(container),
+    }
+    const once = vi.spyOn(process, 'once')
+    const off = vi.spyOn(process, 'off')
+
+    // @ts-expect-error This test only exercises the Docker methods used to create and remove the container.
+    const initializedContainer = await createContainer(docker, 'test-image')
+
+    expect(once).toHaveBeenCalledWith('SIGINT', expect.any(Function))
+    expect(once).toHaveBeenCalledWith('SIGTERM', expect.any(Function))
+
+    await cleanupActiveContainers()
+
+    expect(container.remove).toHaveBeenCalledWith({force: true})
+    expect(off).toHaveBeenCalledWith('SIGINT', expect.any(Function))
+    expect(off).toHaveBeenCalledWith('SIGTERM', expect.any(Function))
+
+    const sandbox = new SystemSandbox(VirtualHost.create(), new Docker(), initializedContainer)
+    await sandbox[Symbol.asyncDispose]()
+
+    expect(container.remove).toHaveBeenCalledTimes(1)
+  })
+
+  test('untracks containers that Docker already removed', async () => {
+    const notFoundError = Object.assign(new Error('No such container'), {statusCode: 404})
+    const container = {
+      start: vi.fn(),
+      remove: vi.fn().mockRejectedValue(notFoundError),
+    }
+    const docker = {
+      createContainer: vi.fn().mockResolvedValue(container),
+    }
+    const off = vi.spyOn(process, 'off')
+
+    // @ts-expect-error This test only exercises the Docker methods used to create and remove the container.
+    const initializedContainer = await createContainer(docker, 'test-image')
+    const sandbox = new SystemSandbox(VirtualHost.create(), new Docker(), initializedContainer)
+
+    await expect(sandbox[Symbol.asyncDispose]()).resolves.toBeUndefined()
+
+    expect(off).toHaveBeenCalledWith('SIGINT', expect.any(Function))
+    expect(off).toHaveBeenCalledWith('SIGTERM', expect.any(Function))
+    await expect(cleanupActiveContainers()).resolves.toBeUndefined()
+  })
 })
 
 describe('SystemSandbox configuration helpers', () => {