diff --git a/docker/agent.Dockerfile b/docker/agent.Dockerfile index 26885c50..18dd9563 100644 --- a/docker/agent.Dockerfile +++ b/docker/agent.Dockerfile @@ -26,13 +26,25 @@ RUN apt-get update \ openssh-client \ && rm -rf /var/lib/apt/lists/* -# The npm-published agent CLIs. Pinning is left to the rebuild cadence (see -# docs/docker-cases-plan.md, user-decision 2). -RUN npm install -g \ - @anthropic-ai/claude-code \ - @openai/codex \ - @google/gemini-cli \ - opencode-ai \ +# The npm-published agent CLIs, supplied by scripts/build-agent-image.mjs from +# config/clis.stock.json so a new stock CLI needs no edit here. The default is +# today's literal list, so a bare `docker build` still produces the same image. +# +# ⚠️ Expanded UNQUOTED on purpose: word splitting is what turns the list into +# several arguments. Every token is validated against +# ^[@A-Za-z0-9][@A-Za-z0-9/._-]*$ on the producing side +# (scripts/lib/cli-catalog.mjs) precisely because of that. +# +# ⚠️ Filtered on each entry's `enabled` flag, so a CLI that ships disabled is +# never baked into every image. +# +# Pinning is left to the rebuild cadence (see docs/docker-cases-plan.md, +# user-decision 2). +# ⚠️ The default is in REGISTRY order, byte-identical to what the generator emits. +# A different order is a different RUN string, which is a different layer hash and +# so a needless cache miss between a bare `docker build` and a scripted one. +ARG CLI_NPM_PACKAGES="@anthropic-ai/claude-code opencode-ai @openai/codex @google/gemini-cli" +RUN npm install -g ${CLI_NPM_PACKAGES} \ && npm cache clean --force # Antigravity (`agy`) is NOT on npm — Google ships a standalone binary through its diff --git a/scripts/build-agent-image.mjs b/scripts/build-agent-image.mjs index 00602d24..271b2465 100644 --- a/scripts/build-agent-image.mjs +++ b/scripts/build-agent-image.mjs @@ -12,6 +12,7 @@ import { spawn, spawnSync } from 'node:child_process'; import { fileURLToPath } from 'node:url'; import { dirname, join } from 'node:path'; +import { agentImageBuildArgPairs, readCatalog } from './lib/cli-catalog.mjs'; const __dirname = dirname(fileURLToPath(import.meta.url)); const REPO_ROOT = join(__dirname, '..'); @@ -58,6 +59,13 @@ if (args.help) { const engine = resolveEngine(args.engine); const buildArgs = ['build', '-f', DOCKERFILE, '-t', args.image]; if (args.noCache) buildArgs.push('--no-cache'); +// The CLI list comes from the generated catalogue rather than the Dockerfile, so adding a +// stock CLI needs no edit in either. `src/docker-hosts.ts` assembles the same argv for the +// in-app auto-build; test/agent-image-build-args-parity.test.ts pins the two together, since +// two independent producers of one command line is exactly how they drift. +for (const [name, value] of agentImageBuildArgPairs(readCatalog())) { + buildArgs.push('--build-arg', `${name}=${value}`); +} buildArgs.push(REPO_ROOT); console.log(`[build-agent-image] ${engine} ${buildArgs.join(' ')}`); diff --git a/scripts/lib/cli-catalog.mjs b/scripts/lib/cli-catalog.mjs new file mode 100644 index 00000000..348efa90 --- /dev/null +++ b/scripts/lib/cli-catalog.mjs @@ -0,0 +1,63 @@ +/** + * @fileoverview Reads the generated CLI catalogue for the Docker build. + * + * `scripts/build-agent-image.mjs` is a `.mjs` and cannot import the TypeScript registry, so it + * reads `config/clis.stock.json` (generated by `scripts/generate-cli-catalog.mts`) instead. + * The pure half lives here so `src/docker-hosts.ts`'s programmatic mirror of the same build + * command can be pinned against it by a test — those two produce the docker argv independently + * and must not drift. + */ +import { readFileSync } from 'node:fs'; +import { fileURLToPath } from 'node:url'; + +const CATALOG_PATH = fileURLToPath(new URL('../../config/clis.stock.json', import.meta.url)); + +/** + * npm package names the AGENT image installs in its shared `npm install -g` layer. + * + * PURE: takes the parsed catalogue, returns a sorted-by-registry-order list. + * + * ⚠️ Filters on `enabled`. That is the field the earlier attempt's export omitted, which is + * how a CLI that ships disabled still had its package baked into every image. + * + * ⚠️ SPECIAL_CASES are excluded here and installed by their own hand-written Dockerfile + * layers, because the registry cannot express what makes them special — a flag, a companion + * package, or not being on npm at all. `test/docker-agent-image-coverage.test.ts` requires + * every one of them to carry a reason and to still be present in the Dockerfile, so an + * exclusion cannot quietly become an omission. + */ +export const AGENT_IMAGE_SPECIAL_CASES = { + pi: 'installed with --ignore-scripts in its own layer, so the flag cannot leak to the shared block', + deepseek: 'needs pnpm alongside it (dsh plugin, issue #352) and a dsh-tui profile install', +}; + +/** Tokens allowed in an npm package name reaching a Dockerfile build arg unquoted. */ +const SAFE_PACKAGE = /^[@A-Za-z0-9][@A-Za-z0-9/._-]*$/; + +export function agentImageNpmPackages(catalog) { + const packages = []; + for (const entry of catalog) { + if (!entry.enabled) continue; + if (entry.id in AGENT_IMAGE_SPECIAL_CASES) continue; + const pkg = entry.discovery?.install?.npmPackage; + if (!pkg) continue; // antigravity/grok/omp ship standalone installers, not npm + if (!SAFE_PACKAGE.test(pkg)) { + // The value is interpolated into a Dockerfile ARG that is expanded UNQUOTED (word + // splitting is how the list becomes several arguments), so a token with whitespace or + // shell metacharacters would change what the RUN line means. + throw new Error(`Refusing unsafe npm package name for "${entry.id}": ${JSON.stringify(pkg)}`); + } + packages.push(pkg); + } + return packages; +} + +/** The `--build-arg` pairs the agent image takes. PURE. */ +export function agentImageBuildArgPairs(catalog) { + return [['CLI_NPM_PACKAGES', agentImageNpmPackages(catalog).join(' ')]]; +} + +/** Read the committed catalogue. IO. */ +export function readCatalog(path = CATALOG_PATH) { + return JSON.parse(readFileSync(path, 'utf-8')); +} diff --git a/src/docker-hosts.ts b/src/docker-hosts.ts index 2cfa6502..68719abb 100644 --- a/src/docker-hosts.ts +++ b/src/docker-hosts.ts @@ -25,6 +25,7 @@ import { existsSync, mkdirSync, readFileSync, writeFileSync } from 'node:fs'; import fs from 'node:fs/promises'; import { dirname, isAbsolute, join, relative, resolve } from 'node:path'; import { enabledCliIds, getCli } from './config/cli-registry/registry.js'; +import { STOCK_CLIS } from './config/cli-registry/stock.js'; import { fileURLToPath } from 'node:url'; import { homedir } from 'node:os'; import { createHash } from 'node:crypto'; @@ -488,8 +489,52 @@ export function buildDockerCreateArgs(ctx: DockerCreateContext): string[] { * scripts/build-agent-image.mjs): `build -f -t [--no-cache] * `. Kept pure + unit-testable; the caller prepends the engine binary. */ -export function agentImageBuildArgs(dockerfile: string, image: string, contextDir: string, noCache = false): string[] { - return ['build', '-f', dockerfile, '-t', image, ...(noCache ? ['--no-cache'] : []), contextDir]; +export function agentImageBuildArgs( + dockerfile: string, + image: string, + contextDir: string, + noCache = false, + buildArgs: Array<[string, string]> = [] +): string[] { + return [ + 'build', + '-f', + dockerfile, + '-t', + image, + ...(noCache ? ['--no-cache'] : []), + ...buildArgs.flatMap(([name, value]) => ['--build-arg', `${name}=${value}`]), + contextDir, + ]; +} + +/** + * npm packages the agent image installs in its shared layer, from the STOCK catalogue. + * + * ⚠️ Stock, deliberately, NOT the merged registry. A user's `~/.codeman/clis.json` must not + * change what lands inside an image tagged `codeman/agent:base`, or two machines holding that + * same tag hold different images and every cache-hit decision downstream is a lie. + * + * ⚠️ This mirrors `agentImageNpmPackages()` in `scripts/lib/cli-catalog.mjs`, which the CLI + * build path uses because a `.mjs` cannot import TypeScript. Two producers of one command + * line drift; `test/agent-image-build-args-parity.test.ts` is what stops them. + */ +export function agentImageNpmPackages(): string[] { + return STOCK_CLIS.filter((entry) => entry.enabled && !AGENT_IMAGE_SPECIAL_CASE_IDS.has(entry.id as string)) + .map((entry) => entry.discovery.install.npmPackage) + .filter((pkg): pkg is string => Boolean(pkg)); +} + +/** + * CLIs the agent image installs in their OWN Dockerfile layer rather than the shared npm one. + * The registry cannot express what makes each special, so the layers stay hand-written and + * `test/docker-agent-image-coverage.test.ts` requires each to carry a reason and still exist. + */ +const AGENT_IMAGE_SPECIAL_CASE_IDS = new Set(['pi', 'deepseek']); + +/** The `--build-arg` pairs the agent image takes. */ +export function agentImageBuildArgPairs(): Array<[string, string]> { + return [['CLI_NPM_PACKAGES', agentImageNpmPackages().join(' ')]]; } // ========== Credential mount resolution (IO) ========== @@ -1020,7 +1065,7 @@ function buildAgentImage( const argv = dockerEngineArgv(docker); const args = [ ...argv.slice(1), - ...agentImageBuildArgs(resolved.dockerfile, image, resolved.contextDir, opts.noCache), + ...agentImageBuildArgs(resolved.dockerfile, image, resolved.contextDir, opts.noCache, agentImageBuildArgPairs()), ]; return new Promise((resolve) => { // async spawn (NEVER spawnSync) so a multi-minute build never wedges the event loop. diff --git a/src/web/public/index.html b/src/web/public/index.html index 5ddb00fa..54b1558f 100644 --- a/src/web/public/index.html +++ b/src/web/public/index.html @@ -2905,7 +2905,7 @@
- Build it once with node scripts/build-agent-image.mjs. Contains node + claude/codex/gemini/opencode/agy/pi/grok/dsh + tmux. + Build it once with node scripts/build-agent-image.mjs. Contains node + claude/opencode/codex/gemini/agy/pi/grok/dsh/omp + tmux.
diff --git a/test/agent-image-build-args-parity.test.ts b/test/agent-image-build-args-parity.test.ts new file mode 100644 index 00000000..f3a1d695 --- /dev/null +++ b/test/agent-image-build-args-parity.test.ts @@ -0,0 +1,80 @@ +/** + * @fileoverview The two producers of the agent-image `docker build` command line must agree. + * + * There are two, and there have to be: `scripts/build-agent-image.mjs` is what a human runs + * and is a `.mjs`, so it cannot import the TypeScript registry and reads the generated + * `config/clis.stock.json` instead; `src/docker-hosts.ts` builds the same command for the + * in-app auto-build on the first Docker case, from `STOCK_CLIS` directly. + * + * Two independent producers of one command line is exactly the shape that drifts, and the + * failure would be quiet and confusing: an image built by hand and an image built by the app + * would hold different CLIs under the SAME `codeman/agent:base` tag, so which CLIs a container + * has would depend on who built it. + * + * Port: none (pure). + */ + +import { describe, expect, it } from 'vitest'; +import { readFileSync } from 'node:fs'; +import { fileURLToPath } from 'node:url'; +import { + agentImageBuildArgPairs as mjsPairs, + agentImageNpmPackages as mjsPackages, +} from '../scripts/lib/cli-catalog.mjs'; +import { + agentImageBuildArgPairs as tsPairs, + agentImageBuildArgs, + agentImageNpmPackages as tsPackages, +} from '../src/docker-hosts.js'; + +const CATALOG = JSON.parse(readFileSync(fileURLToPath(new URL('../config/clis.stock.json', import.meta.url)), 'utf-8')); + +describe('agent-image build args: the .mjs and the TS mirror agree', () => { + it('resolve the same npm package list, in the same order', () => { + // Order matters as well as membership: a different order is a different RUN string, hence + // a different layer hash, hence a cache miss between the two build paths. + expect(tsPackages()).toEqual(mjsPackages(CATALOG)); + }); + + it('produce the same --build-arg pairs', () => { + expect(tsPairs()).toEqual(mjsPairs(CATALOG)); + }); + + it('render the same argv', () => { + // What the .mjs assembles by hand around its pairs, spelled out here so a change to + // either side's argv SHAPE (not just its values) fails too. + const pairs = tsPairs(); + const expected = [ + 'build', + '-f', + '/repo/docker/agent.Dockerfile', + '-t', + 'codeman/agent:base', + '--no-cache', + ...pairs.flatMap(([name, value]) => ['--build-arg', `${name}=${value}`]), + '/repo', + ]; + expect(agentImageBuildArgs('/repo/docker/agent.Dockerfile', 'codeman/agent:base', '/repo', true, pairs)).toEqual( + expected + ); + }); + + it('keeps --build-arg out of the argv when nothing is passed', () => { + // The parameter defaults to empty, so an existing caller that has not been updated still + // produces exactly the command it produced before. + expect(agentImageBuildArgs('/d', 'i', '/c')).toEqual(['build', '-f', '/d', '-t', 'i', '/c']); + }); + + it('resolves a non-empty list (anti-vacuity)', () => { + // Two empty lists compare equal very happily. + expect(tsPackages().length).toBeGreaterThan(3); + expect(tsPairs()[0][1].length).toBeGreaterThan(20); + }); + + it('matches the Dockerfile ARG default, so a bare `docker build` is cache-identical', () => { + const dockerfile = readFileSync(fileURLToPath(new URL('../docker/agent.Dockerfile', import.meta.url)), 'utf-8'); + const declared = /^ARG CLI_NPM_PACKAGES="([^"]*)"$/m.exec(dockerfile)?.[1]; + expect(declared, 'the Dockerfile no longer declares CLI_NPM_PACKAGES').toBeDefined(); + expect(declared).toBe(tsPackages().join(' ')); + }); +}); diff --git a/test/docker-agent-image-coverage.test.ts b/test/docker-agent-image-coverage.test.ts new file mode 100644 index 00000000..18ffeefa --- /dev/null +++ b/test/docker-agent-image-coverage.test.ts @@ -0,0 +1,148 @@ +/** + * @fileoverview Every shipped CLI reaches the Docker agent image, and no unshipped one does. + * + * The image's npm layer is now a build arg fed from the generated catalogue, but four CLIs + * still install through hand-written layers because the registry cannot describe what makes + * them special — a flag, a companion package, or not being on npm at all. That mix is fine; + * what is not fine is a CLI landing in `stock.ts` and reaching NEITHER, which is upstream + * `b6d0f1fa` (omp shipped with no installer wiring) in the image instead of the installer. + * + * So this asserts total coverage rather than checking the arg alone, and requires every + * special case to carry a written reason. + * + * Port: none (pure, over two Dockerfiles, the catalogue and the registry). + */ + +import { describe, expect, it } from 'vitest'; +import { readFileSync } from 'node:fs'; +import { fileURLToPath } from 'node:url'; +import { AGENT_IMAGE_SPECIAL_CASES, agentImageNpmPackages } from '../scripts/lib/cli-catalog.mjs'; +import { STOCK_CLIS } from '../src/config/cli-registry/stock.js'; + +const read = (rel: string): string => readFileSync(fileURLToPath(new URL(`../${rel}`, import.meta.url)), 'utf-8'); + +const AGENT_DOCKERFILE = read('docker/agent.Dockerfile'); +const SERVER_DOCKERFILE = read('docker/server.Dockerfile'); +const INDEX_HTML = read('src/web/public/index.html'); +const CATALOG = JSON.parse(read('config/clis.stock.json')) as Array<{ + id: string; + enabled: boolean; + discovery: { binaries: string[]; install: { npmPackage?: string } }; +}>; + +const enabledAgents = CATALOG.filter((e) => e.enabled && e.discovery.binaries.length > 0); + +describe('docker agent image covers the catalogue', () => { + it('installs every enabled npm CLI, via the build arg or a documented special case', () => { + const inBuildArg = new Set(agentImageNpmPackages(CATALOG)); + const missing: string[] = []; + for (const entry of enabledAgents) { + const pkg = entry.discovery.install.npmPackage; + if (!pkg) continue; // standalone installer, checked below + if (inBuildArg.has(pkg)) continue; + if (entry.id in AGENT_IMAGE_SPECIAL_CASES) continue; + missing.push(`${entry.id} (${pkg})`); + } + expect( + missing, + `npm CLI reaches neither the build arg nor a special case:\n ${missing.join('\n ')}\n` + + 'Add it to the arg (it is automatic) or give it a Dockerfile layer AND a reason in AGENT_IMAGE_SPECIAL_CASES.' + ).toEqual([]); + }); + + it('gives every special case a reason and a real layer', () => { + for (const [id, reason] of Object.entries(AGENT_IMAGE_SPECIAL_CASES)) { + expect(reason.length, `${id} has an empty reason`).toBeGreaterThan(20); + const entry = CATALOG.find((e) => e.id === id); + expect(entry, `${id} is a special case but not in the catalogue`).toBeDefined(); + const pkg = entry?.discovery.install.npmPackage; + // Excluded from the shared arg, so it MUST appear in a hand-written layer, or it is + // simply not installed at all — an exclusion silently becoming an omission. + expect( + AGENT_DOCKERFILE.includes(pkg ?? id), + `${id} is excluded from the arg but absent from the Dockerfile` + ).toBe(true); + } + }); + + it('installs every enabled non-npm CLI in its own layer', () => { + for (const entry of enabledAgents) { + if (entry.discovery.install.npmPackage) continue; + const binary = entry.discovery.binaries[0]; + expect( + AGENT_DOCKERFILE.includes(binary), + `${entry.id} ships no npm package and no Dockerfile layer mentions "${binary}"` + ).toBe(true); + } + }); + + it('bakes in nothing from a DISABLED entry', () => { + // The maintainer's finding: the earlier export carried no `enabled` field, so a CLI that + // ships disabled still had its package installed into every image. + for (const entry of CATALOG) { + if (entry.enabled) continue; + const pkg = entry.discovery.install.npmPackage; + if (!pkg) continue; + expect(AGENT_DOCKERFILE.includes(pkg), `disabled ${entry.id} is still baked into the image`).toBe(false); + } + }); + + it('excludes a disabled entry from the build arg (unit, since none ships disabled today)', () => { + // Every stock entry is enabled right now, so the assertion above passes vacuously. Feed + // the pure helper a fabricated disabled entry so the fix is genuinely covered TODAY + // rather than the first time someone ships one. + const fabricated = [ + ...CATALOG, + { id: 'ghost', enabled: false, discovery: { binaries: ['ghost'], install: { npmPackage: '@ghost/cli' } } }, + ]; + expect(agentImageNpmPackages(fabricated)).not.toContain('@ghost/cli'); + const enabledTwin = fabricated.map((e) => (e.id === 'ghost' ? { ...e, enabled: true } : e)); + expect(agentImageNpmPackages(enabledTwin)).toContain('@ghost/cli'); + }); + + it('refuses an npm package name that would not survive unquoted expansion', () => { + // The Dockerfile expands ${CLI_NPM_PACKAGES} unquoted so word splitting makes the list. + // A token with a space or a metacharacter would therefore change what the RUN line means. + const hostile = [ + { id: 'x', enabled: true, discovery: { binaries: ['x'], install: { npmPackage: 'a && rm -rf /' } } }, + ]; + expect(() => agentImageNpmPackages(hostile)).toThrow(/unsafe npm package name/i); + }); +}); + +describe('docker server image divergence is declared, not accidental', () => { + // server.Dockerfile deliberately ships a NARROWER list than the agent image, and is left + // untouched by this change because two other open PRs already modify it. Asserting the + // omissions here makes the divergence reviewable without editing the file: if someone adds + // a CLI there, or the intent changes, this fails and the list has to be restated. + const SERVER_INTENTIONAL_OMISSIONS = new Set(['antigravity', 'pi', 'grok', 'deepseek', 'omp']); + + it('installs exactly the CLIs it declares, and no more', () => { + for (const entry of enabledAgents) { + const pkg = entry.discovery.install.npmPackage; + if (!pkg) continue; + const present = SERVER_DOCKERFILE.includes(pkg); + if (SERVER_INTENTIONAL_OMISSIONS.has(entry.id)) { + expect(present, `${entry.id} is listed as an intentional omission but IS in server.Dockerfile`).toBe(false); + } else { + expect(present, `${entry.id} is missing from server.Dockerfile and not declared as omitted`).toBe(true); + } + } + }); +}); + +describe('the in-app agent-image hint stays accurate', () => { + it('names every enabled CLI binary the image contains', () => { + // index.html tells the user what the image holds. It was stale (it omitted omp), which is + // the same drift one layer out: prose describing a list nobody re-checks. + const hint = INDEX_HTML.split('\n').find((l) => l.includes('build-agent-image.mjs')); + expect(hint, 'the agent-image hint disappeared from index.html').toBeDefined(); + for (const entry of enabledAgents) { + expect(hint, `the hint does not mention ${entry.discovery.binaries[0]}`).toContain(entry.discovery.binaries[0]); + } + }); + + it('is checked against the registry, not a copy of itself (anti-vacuity)', () => { + expect(STOCK_CLIS.filter((e) => e.enabled && e.discovery.binaries.length > 0).length).toBeGreaterThan(5); + }); +});