From 1645ef5f5c930d68581f0e6a0c386ccb0071656a Mon Sep 17 00:00:00 2001 From: Codeman maintainer Date: Mon, 28 Sep 2026 16:25:33 +0200 Subject: [PATCH] fix(docker): merge-time fixes for #492 - test: the complete-identity case now checks the combined agentImageBuildArgPairs() argv on both producers, so the manual build-agent-image.mjs path cannot drop the identity unnoticed - both producers: GIT_IDENTITY_BUILD_ARGS carries the mirror/parity warning its gh/az neighbour has - the partial-identity error names CODEMAN_AGENT_IMAGE_GIT_USER_NAME and CODEMAN_AGENT_IMAGE_GIT_USER_EMAIL; test regex follows - wiki Docker-Cases: mention the identity variables next to the gh/az switches Co-Authored-By: Claude Opus 5.5 (1M context) --- docs/wiki/Docker-Cases.md | 6 ++++++ scripts/lib/cli-catalog.mjs | 8 ++++++-- src/docker-hosts.ts | 9 +++++++-- test/agent-image-build-args-parity.test.ts | 8 ++++++-- 4 files changed, 25 insertions(+), 6 deletions(-) diff --git a/docs/wiki/Docker-Cases.md b/docs/wiki/Docker-Cases.md index 5298bd11..30812cca 100644 --- a/docs/wiki/Docker-Cases.md +++ b/docs/wiki/Docker-Cases.md @@ -147,6 +147,12 @@ in `docker-compose.override.yml`), then rebuild the image with `--no-cache`. `docker/README.md` ("Private repositories") has the details and the matching switches for the server image. +To give agents a fixed Git commit identity, set `CODEMAN_AGENT_IMAGE_GIT_USER_NAME` and +`CODEMAN_AGENT_IMAGE_GIT_USER_EMAIL` together in that same environment (in the Docker +deployment, set `GIT_USER_NAME` and `GIT_USER_EMAIL` in `docker/.env` instead, which feeds +both images). An existing `codeman/agent:base` only picks it up after a `--no-cache` rebuild +and recreated case containers; `docker/README.md` ("Git commit identity") has the details. + ## Isolation Every container runs hardened by default: diff --git a/scripts/lib/cli-catalog.mjs b/scripts/lib/cli-catalog.mjs index fd9c53f6..614a56fb 100644 --- a/scripts/lib/cli-catalog.mjs +++ b/scripts/lib/cli-catalog.mjs @@ -64,7 +64,10 @@ export const GIT_HOST_CLI_BUILD_ARGS = [ ['CODEMAN_AGENT_IMAGE_INSTALL_AZ', 'CODEMAN_INSTALL_AZ'], ]; -/** Environment variables passed through to the agent image's system Git configuration. */ +/** + * Environment variable → Dockerfile ARG for the image's system Git identity. + * ⚠️ Mirrored by `GIT_IDENTITY_BUILD_ARGS` in `src/docker-hosts.ts`; the parity test pins them. + */ export const GIT_IDENTITY_BUILD_ARGS = [ ['CODEMAN_AGENT_IMAGE_GIT_USER_NAME', 'GIT_USER_NAME'], ['CODEMAN_AGENT_IMAGE_GIT_USER_EMAIL', 'GIT_USER_EMAIL'], @@ -94,7 +97,8 @@ export function gitIdentityBuildArgPairs(env) { const configured = pairs.filter(([, value]) => value !== ''); if (configured.length === 0) return []; if (configured.length !== pairs.length) { - throw new Error('Git user name and email must both be set when configuring Git identity'); + const names = GIT_IDENTITY_BUILD_ARGS.map(([envName]) => envName).join(' and '); + throw new Error(`${names} must both be set when configuring Git identity`); } return pairs; } diff --git a/src/docker-hosts.ts b/src/docker-hosts.ts index 3b6f7744..fbafb460 100644 --- a/src/docker-hosts.ts +++ b/src/docker-hosts.ts @@ -615,7 +615,10 @@ export const GIT_HOST_CLI_BUILD_ARGS: ReadonlyArray = ['CODEMAN_AGENT_IMAGE_INSTALL_AZ', 'CODEMAN_INSTALL_AZ'], ]; -/** Environment variables passed through to the agent image's system Git configuration. */ +/** + * Environment variable → Dockerfile ARG for the image's system Git identity. + * ⚠️ Mirrors `GIT_IDENTITY_BUILD_ARGS` in `scripts/lib/cli-catalog.mjs`; the parity test pins them. + */ export const GIT_IDENTITY_BUILD_ARGS: ReadonlyArray = [ ['CODEMAN_AGENT_IMAGE_GIT_USER_NAME', 'GIT_USER_NAME'], ['CODEMAN_AGENT_IMAGE_GIT_USER_EMAIL', 'GIT_USER_EMAIL'], @@ -642,13 +645,15 @@ export function gitHostCliBuildArgPairs(env: NodeJS.ProcessEnv): Array<[string, /** * The `--build-arg` pairs for a configured Git identity. An absent pair leaves * Git unconfigured, preserving existing deployments; a partial pair is refused. + * ⚠️ Mirrors `gitIdentityBuildArgPairs()` in `scripts/lib/cli-catalog.mjs`; the parity test pins them. */ export function gitIdentityBuildArgPairs(env: NodeJS.ProcessEnv): Array<[string, string]> { const pairs = GIT_IDENTITY_BUILD_ARGS.map(([envName, argName]) => [argName, env[envName] ?? ''] as [string, string]); const configured = pairs.filter(([, value]) => value !== ''); if (configured.length === 0) return []; if (configured.length !== pairs.length) { - throw new Error('Git user name and email must both be set when configuring Git identity'); + const names = GIT_IDENTITY_BUILD_ARGS.map(([envName]) => envName).join(' and '); + throw new Error(`${names} must both be set when configuring Git identity`); } return pairs; } diff --git a/test/agent-image-build-args-parity.test.ts b/test/agent-image-build-args-parity.test.ts index 3ca358d0..bbf315c6 100644 --- a/test/agent-image-build-args-parity.test.ts +++ b/test/agent-image-build-args-parity.test.ts @@ -172,6 +172,9 @@ describe('Git identity in the agent image: both producers pass the same settings expect(mjsGitIdentityPairs(identity)).toEqual(expected); expect(tsGitIdentityPairs({})).toEqual([]); expect(mjsGitIdentityPairs({})).toEqual([]); + // The combined argv, not just the helper: the manual build path could drop the identity otherwise. + expect(tsPairs(identity)).toEqual(mjsPairs(CATALOG, identity)); + expect(tsPairs(identity)).toEqual(expect.arrayContaining(expected)); }); it('refuses a partial identity in both build paths', () => { @@ -179,8 +182,9 @@ describe('Git identity in the agent image: both producers pass the same settings { CODEMAN_AGENT_IMAGE_GIT_USER_NAME: 'Ada Lovelace' }, { CODEMAN_AGENT_IMAGE_GIT_USER_EMAIL: 'ada@example.com' }, ]) { - expect(() => tsGitIdentityPairs(identity)).toThrow(/Git user name and email/); - expect(() => mjsGitIdentityPairs(identity)).toThrow(/Git user name and email/); + const named = /CODEMAN_AGENT_IMAGE_GIT_USER_NAME and CODEMAN_AGENT_IMAGE_GIT_USER_EMAIL must both be set/; + expect(() => tsGitIdentityPairs(identity)).toThrow(named); + expect(() => mjsGitIdentityPairs(identity)).toThrow(named); } });