Files
Codeman/test/agent-image-build-args-parity.test.ts
T
DevvynandClaude Sonnet 5 a0628a40e8 fix(cli-registry): address maintainer review on #380
Rebased onto current master (the one real conflict was the import line
in docker-hosts.ts Ark0N flagged; kept both), then addressed every
point from the review:

**1. Rebase.** Done — this branch now sits on current upstream/master.

**2. Agent-image special cases are data now, not an id-keyed table
outside stock.ts.** `AGENT_IMAGE_SPECIAL_CASE_IDS`/`AGENT_IMAGE_SPECIAL_CASES`
are gone. `CliDiscovery.install.agentImageLayer?: { kind: 'dedicated';
reason: string }` is a field on the registry entry itself (pi,
deepseek), `reason` is required by schema.ts, both producers
(docker-hosts.ts and cli-catalog.mjs) filter on its presence instead
of an id, and the coverage test reads it from the generated catalogue.
Also added the npm-package-name validation to the TS producer, which
only the .mjs one had — same SAFE_PACKAGE regex, duplicated
(necessarily, one side can't import the other) and now pinned
byte-identical by a new parity test.

**3. Changeset said five, it's eight.** (Not nine — see the DeepSeek
point below, which changes the true count.) Reworded to state it
structurally rather than pin a number that will go stale again.

Then the four behavior-changing findings:

- **DeepSeek was offered as a normal install option but can't actually
  drive a pane.** `npm install -g @deepseek-ai/dsh` installs the
  launcher only; DeepSeek ships no profile that can run standalone.
  The generator now emits an empty install command for any
  `launcherProfile` entry, so install.sh's menu (which requires a
  non-empty command) skips it and falls through to its docs URL hint
  instead — matching what the old hand-written code did before this
  PR replaced it.
- **wget-only hosts lost every automatic install, including the npm
  ones that never needed curl.** The menu-building loop now filters
  PER ENTRY (only a command starting with `curl ` is held back) rather
  than wiping the whole menu when DOWNLOADER != curl.
- **The DISPLAY/TRUSTED split and the catalogue refresh didn't hold up
  under review** (refresh's only real write was the label; it ran
  before the Node existence check; its own eval-detection test was
  tripped by the word "eval'd" in a comment). Dropped entirely per
  your own recommendation — embedded catalogue only, no network
  fetch, no second array. install-sh-invariants.test.ts now asserts
  the refresh/DISPLAY machinery does not exist rather than testing its
  internals.

The three take-or-leave items, applied:

- `dsh_banner_probe`'s bash 3.2 empty-array bug: `${runner[@]}` →
  `${runner[@]+"${runner[@]}"}`. Verified live in a real `bash:3.2.57`
  container with `timeout` removed from PATH — crashed before, clean
  now, full `detect_all_clis` path exercised end to end.
- `docker-agent-image-coverage.test.ts` now anchors on each layer's
  `<binary> --version` proof line instead of `Dockerfile.includes(binary)`,
  which stayed true if a layer were deleted but its comment survived.
- Doc drift: docs/docker-cases.md (four → five, and now describes the
  data field), docker/agent.Dockerfile's "other four CLIs" comment (no
  longer a magic number — CLI_NPM_PACKAGES is generated and can grow),
  CLAUDE.md's install.sh size (104KB → ~112KB) and its stale mention of
  the now-dropped refresh.

Verified: tsc clean, prettier clean, the full targeted suite (142
tests across the 8 affected files) green, and the full `npm test` gate
diffed BY TEST NAME against a clean upstream/master baseline run on
this same machine — identical 201-name failure set both sides (168
tests / 67 files, all pre-existing Windows-environment noise: symlinks,
PTY spawning, POSIX permission bits — none of it touching anything
this PR changes), zero new failures either side of the diff.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01R9ZSTEenc8soSu9bTi8Xru
2026-09-13 17:43:14 +08:00

99 lines
4.6 KiB
TypeScript

/**
* @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(' '));
});
it('validates an unsafe package name with the SAME regex on both sides', () => {
// Equal OUTPUT on today's catalogue (asserted above) does not prove equal VALIDATION — a
// looser regex on one side would only show up the day someone ships a hostile package name.
// The regex is duplicated rather than shared (the .mjs side cannot import the .ts side, the
// whole reason this file exists), so pin the literal PATTERN text is identical between the
// two source files rather than trusting the comment that says so.
const tsSource = readFileSync(fileURLToPath(new URL('../src/docker-hosts.ts', import.meta.url)), 'utf-8');
const mjsSource = readFileSync(fileURLToPath(new URL('../scripts/lib/cli-catalog.mjs', import.meta.url)), 'utf-8');
const extract = (source: string, file: string): string => {
// Non-greedy to `/;` deliberately: the pattern itself contains a `/` (inside the
// character class), so a naive `[^/]+` stops at the wrong slash.
const m = /const SAFE_PACKAGE = (\/.+?\/);/.exec(source);
expect(m, `could not find the SAFE_PACKAGE regex literal in ${file}`).toBeDefined();
return m![1];
};
expect(extract(tsSource, 'docker-hosts.ts')).toBe(extract(mjsSource, 'cli-catalog.mjs'));
});
});