Test that the real docker is found once at load and that shims refuse without one (#189)

Review round 47: a unit test loads the fixture with docker on PATH, then puts
another first, and checks REAL_DOCKER keeps the first; loaded with no docker,
requireRealDocker refuses. A docker found through a relative PATH entry is
resolved to an absolute path. The never-started agent container's comment gives
the real reason it is tracked.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Author: mchwang

test/agent-container.test.ts

@@ -983,8 +983,8 @@ describe('real Docker agent isolation', () => {
 
   it('never removes an agent container found by name after a killed create when a label is missing', async () => {
     const data = fixture(), live = await profile(data, 'planning', 'must-not-run');
-    // Tracked for afterAll should the test time out: never started and without the runner label, it is no holder a
-    // sweep finds once its network is gone (#189).
+    // Tracked for afterAll should the test time out: it has no runner label, and until it goes the task volumes it
+    // mounts cannot be removed (#189).
     containers.add(live.name);
     // The create lands without the runner label, and its client is killed before it reports the ID. Only this
     // container's: should the test time out with the shim still on PATH, other tests' creates pass through.

test/docker-sweep.test.ts

@@ -1,4 +1,4 @@
-import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs';
+import { mkdirSync, mkdtempSync, readFileSync, rmSync, symlinkSync, writeFileSync } from 'node:fs';
 import { tmpdir } from 'node:os';
 import { join } from 'node:path';
 import { spawnSync } from 'node:child_process';
@@ -173,3 +173,26 @@ it.each([
   expect(warn.mock.calls.filter(call => String(call[0]).endsWith('failed:'))).toEqual(warnings);
   expect(readFileSync(calls, 'utf8').split('\n')).toContain(`rm --force ${a}`);
 });
+
+it('finds the real docker once, at load, and refuses a shim when none was found', async () => {
+  const root = mkdtempSync(join(tmpdir(), 'codeboost-sweep-')); roots.push(root);
+  // Two directories, each with a docker and the sh the lookup runs, and one with only sh.
+  const bin = (name: string, docker: boolean) => {
+    const dir = join(root, name); mkdirSync(dir);
+    symlinkSync(spawnSync('sh', ['-c', 'command -v sh'], { encoding: 'utf8' }).stdout.trim(), join(dir, 'sh'));
+    if (docker) writeFileSync(join(dir, 'docker'), '#!/bin/sh\nexit 0\n', { mode: 0o755 });
+    return dir;
+  };
+  const first = bin('first', true), second = bin('second', true), none = bin('none', false);
+  vi.stubEnv('PATH', first);
+  vi.resetModules();
+  const loaded = await import('./fixtures/docker-sweep.ts') as typeof Sweep;
+  // A shim put on PATH later does not change it.
+  vi.stubEnv('PATH', second);
+  expect(loaded.REAL_DOCKER).toBe(join(first, 'docker'));
+  expect(loaded.requireRealDocker()).toBe(join(first, 'docker'));
+  vi.stubEnv('PATH', none);
+  vi.resetModules();
+  const missing = await import('./fixtures/docker-sweep.ts') as typeof Sweep;
+  expect(() => missing.requireRealDocker()).toThrow('No docker on PATH when the test files loaded.');
+});

test/fixtures/docker-sweep.ts

@@ -1,12 +1,15 @@
 import { spawnSync, type SpawnSyncReturns } from 'node:child_process';
+import { isAbsolute, resolve } from 'node:path';
 import { RUNNER_LABEL } from '../../agents/labels.ts';
 
 /**
  * The real `docker`, found once when this module loads, before any test puts a shim on PATH. A sweep that runs while a
  * shim is still on PATH (after a test timed out inside one) would otherwise wait on the shim's own child, which the
  * call's SIGKILL does not reach.
  */
-export const REAL_DOCKER = spawnSync('sh', ['-c', 'command -v docker'], { encoding: 'utf8' }).stdout?.trim() || 'docker';
+const FOUND_DOCKER = spawnSync('sh', ['-c', 'command -v docker'], { encoding: 'utf8' }).stdout?.trim();
+// Resolved, as a PATH entry may be relative.
+export const REAL_DOCKER = FOUND_DOCKER ? resolve(FOUND_DOCKER) : 'docker';
 /**
  * The daemon `docker` talks to when this module loads. A test that times out may leave DOCKER_HOST or DOCKER_CONTEXT
  * changed (agent-supervisor points DOCKER_HOST at a missing socket until its handle settles), so every call here
@@ -20,7 +23,7 @@ const dockerEnv = () => {
 };
 /** The real `docker` for a shim to call; refuses when none was found, since a shim calling `docker` would call itself. */
 export function requireRealDocker(): string {
-  if (!REAL_DOCKER.startsWith('/')) throw new Error('No docker on PATH when the test files loaded.');
+  if (!isAbsolute(REAL_DOCKER)) throw new Error('No docker on PATH when the test files loaded.');
   return REAL_DOCKER;
 }