From 5b4878df3b7ea54965e1e072e1f4211c774b5aeb Mon Sep 17 00:00:00 2001 From: Devvyn <22340871+opticon454@users.noreply.github.com> Date: Mon, 21 Sep 2026 15:19:00 +0800 Subject: [PATCH] =?UTF-8?q?fix(docker):=20address=20Ark0N's=20PR=20review?= =?UTF-8?q?=20on=20Update-Codeman.sh=20=E2=80=94=20fix=20the=20handoff,=20?= =?UTF-8?q?the=20build/down=20ordering,=20and=20default-clear=20the=20buil?= =?UTF-8?q?d=20volumes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three blockers, all fixed and verified by actually running the script (not just string-matching it): 1. `exec "$script_dir/Start-Codeman.sh"` failed EACCES/exit 126 on every checkout, since Start-Codeman.sh is committed non-executable (100644) — the same fact my own second commit on this branch established. Fixed to `exec bash "$script_dir/Start-Codeman.sh"`. 2. `down` ran before `build --no-cache`, so Codeman and every session it was running were offline for the entire rebuild, and a build failure left the stack down with nothing to bring it back — the exact ordering mistake Start-Codeman.sh's own "Build BEFORE taking the stack down" comment exists to prevent. Reordered to build, then down, then hand off. 3. The default path could throw the rebuild away: codeman-node-modules/ codeman-dist only re-seed from the image while EMPTY, Start-Codeman.sh only clears them when it detects the checkout's HEAD or package-lock.json moved, and neither condition is true for the Dockerfile-only change this script exists for — so a plain `bash docker/Update-Codeman.sh` rebuilt an image whose fresh node_modules/dist then sat unused behind the old volumes. Made clearing them the default; `--keep-volumes` opts out (replaces the old `--volumes`/`-v` flag, which is no longer needed since clearing is now the default). Smaller items from the same review, also fixed: - The --no-cache build now derives PUID/PGID from CODEMAN_APPDATA_PATH's owner first, via the identical owner_of() helper Start-Codeman.sh uses (parity-tested) — without it, the build used Compose's default 1000:1000 regardless of the real appdata owner (99:100 on the Unraid layout docker/README.md documents), and Start-Codeman.sh's own correctly-PUID'd build during the handoff would then rebuild those layers anyway, so the --no-cache image never actually shipped. - docker/README.md's "rebuilds ... only when it detects ... moved" wrongly described BOTH the rebuild and the volume-clearing as conditional; Start-Codeman.sh rebuilds on every start, only the volume-clearing is conditional. Corrected, and reworded around the new default. - --help/-h now prints usage and exits 0 instead of falling into the unrecognised-argument branch. - "the ONLY named volumes this stack declares" now says docker-compose.yaml specifically, since a docker-compose.override.yml could add more. New tests: PUID/PGID derivation parity with Start-Codeman.sh's owner_of(), --help handling, and — the one that actually catches blocker #1, which five source-string-matching tests did not — a real end-to-end smoke test: a synthetic deployment, a stub `docker` on PATH logging every invocation, the real script executed via a real subprocess. Confirms the real command sequence (build --no-cache, then down --volumes or plain down, then evidence the handoff genuinely ran Start-Codeman.sh) and that a working handoff fails honestly at Start-Codeman.sh's own later check rather than with EACCES. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01N6eadpRyqpA9PD3i139cSD --- docker/README.md | 25 ++-- docker/Update-Codeman.sh | 143 ++++++++++++++------- docs/docker-self-update.md | 2 +- test/docker-entrypoint.test.ts | 218 ++++++++++++++++++++++++++++++--- 4 files changed, 316 insertions(+), 72 deletions(-) diff --git a/docker/README.md b/docker/README.md index 91fded40..0047625f 100644 --- a/docker/README.md +++ b/docker/README.md @@ -43,23 +43,26 @@ changed, and asks you to run `Start-Codeman.sh` here on the host instead. Detail ### Major updates -`Start-Codeman.sh` rebuilds the image and clears the build-artefact volumes on -its own, but only when it detects the checkout's HEAD or `package-lock.json` -moved — exactly right for an ordinary `git pull`, too conservative when a -release note (or the updater's own blocker message) calls for starting over. -For that case, `docker/Update-Codeman.sh` stops the stack, force-rebuilds the -image with no layer cache, then hands off to `Start-Codeman.sh` for the usual -start: +`Start-Codeman.sh` rebuilds the image on every start, but only clears the +`codeman-node-modules`/`codeman-dist` build-artefact volumes when it detects +the checkout's HEAD or `package-lock.json` moved — exactly right for an +ordinary `git pull`, too narrow when a release note (or the updater's own +blocker message) calls for starting over on a Dockerfile-only change, which +touches neither. For that case, `docker/Update-Codeman.sh` force-rebuilds the +image with no layer cache, clears those two volumes, stops the stack, then +hands off to `Start-Codeman.sh` for the usual start: ```sh bash docker/Update-Codeman.sh ``` -Add `--volumes` to also clear the `codeman-node-modules`/`codeman-dist` -volumes — the scripted form of "Resetting the build artefacts" in +Pass `--keep-volumes` to skip clearing them (safe only if you know the +rebuilt image's `node_modules`/`dist` did not change) — the scripted default +is the "Resetting the build artefacts" procedure in [`../docs/docker-self-update.md`](../docs/docker-self-update.md). Those two -are the only named volumes this stack declares; application data and case -workspaces are host bind mounts and are never touched either way. +are the only named volumes `docker-compose.yaml` itself declares; a +`docker-compose.override.yml` could add more, and application data and case +workspaces are host bind mounts, never touched either way. ## Local customisation diff --git a/docker/Update-Codeman.sh b/docker/Update-Codeman.sh index 63cdf497..ba9fb2fd 100644 --- a/docker/Update-Codeman.sh +++ b/docker/Update-Codeman.sh @@ -8,29 +8,34 @@ # `.env.example` key. None of those can be applied by a container restarting # itself — a restart reuses the existing image and configuration (see "The # environment gate" in docs/docker-self-update.md) — so this script does the -# three things an in-place update cannot: stop the stack, force a real image -# rebuild with no layer cache, then hand off to Start-Codeman.sh for the same +# three things an in-place update cannot: force a real image rebuild with no +# layer cache, stop the stack, then hand off to Start-Codeman.sh for the same # careful PUID/PGID, override-file and fingerprint handling every other start # goes through. # -# This is the scripted form of "Resetting the build artefacts" in -# docs/docker-self-update.md (`docker compose down -v`, then -# `Start-Codeman.sh`), plus the unconditional `--no-cache` a major update -# warrants: `Start-Codeman.sh` on its own only rebuilds without the cache flag, -# and only clears the two build-artefact volumes when it detects the checkout's -# HEAD or `package-lock.json` moved — exactly right for an ordinary `git pull`, -# too conservative when the ask is "start over, certain of what ships". +# ⚠️ Build BEFORE stopping the stack, deliberately, same reasoning as +# Start-Codeman.sh's own build-then-down ordering: the build needs nothing +# stopped, so a slow --no-cache rebuild costs no downtime, and a build failure +# (a bad Dockerfile edit, a network blip pulling a base image) leaves the +# ALREADY-RUNNING stack untouched instead of stopped with nothing to bring it +# back. # -# Usage: docker/Update-Codeman.sh [--volumes] -# --volumes, -v Also remove the codeman-node-modules/codeman-dist named -# volumes, so the fresh image's own node_modules/dist are -# what actually run instead of sitting unused behind a -# Docker-seeded volume's old content (Docker only seeds a -# named volume from the image while that volume is EMPTY). -# Safe: those two are the ONLY named volumes this stack -# declares (`docker-compose.yaml`) — application data and -# case workspaces are host bind mounts, never touched by -# `docker compose down`, with or without this flag. +# ⚠️ Clears the codeman-node-modules/codeman-dist named volumes by DEFAULT. +# Docker seeds a named volume from the image only while that volume is EMPTY, +# so a rebuilt image's fresh node_modules/dist otherwise sit unused behind a +# volume's old content and the container comes back up looking unchanged — +# exactly wrong for a script whose whole point is "be certain of what ships". +# Start-Codeman.sh only clears them when it detects the checkout's HEAD or +# `package-lock.json` moved, which is right for its own ordinary-start case but +# too narrow here: nothing about a Dockerfile-only change (the case that sends +# people to this script in the first place) touches either of those. Pass +# --keep-volumes to opt out and reuse whatever is already in them. +# +# Usage: docker/Update-Codeman.sh [--keep-volumes] +# --keep-volumes Do not clear codeman-node-modules/codeman-dist. Safe to +# combine with a source change Start-Codeman.sh's own +# detection would have cleared anyway; unsafe if the reason +# you are here is a change to server.Dockerfile alone. set -euo pipefail @@ -38,15 +43,19 @@ script_dir=$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd) env_file="$script_dir/.env" compose_file="$script_dir/docker-compose.yaml" -remove_volumes=0 +keep_volumes=0 for arg in "$@"; do case "$arg" in - --volumes | -v) - remove_volumes=1 + --keep-volumes) + keep_volumes=1 + ;; + --help | -h) + printf 'Usage: bash %s [--keep-volumes]\n' "$0" + exit 0 ;; *) printf 'Error: unrecognised argument: %s\n' "$arg" >&2 - printf 'Usage: %s [--volumes]\n' "$0" >&2 + printf 'Usage: bash %s [--keep-volumes]\n' "$0" >&2 exit 1 ;; esac @@ -59,10 +68,10 @@ if [[ ! -f "$env_file" ]]; then fi # Same override-file discovery as Start-Codeman.sh, and deliberately kept in -# step with it: a stack started through one script and updated through the -# other must resolve to the exact same Compose files, or `down` here and `up` -# there could target different configurations. Compose's own precedence -# (measured on v5.5.0 with both present: it uses .yml and ignores .yaml). +# step with it: a stack built here and started there must resolve to the exact +# same Compose files, or this script's build could target a configuration the +# handoff's own `up` never actually uses. Compose's own precedence (measured on +# v5.5.0 with both present: it uses .yml and ignores .yaml). override_yml="$script_dir/docker-compose.override.yml" override_yaml="$script_dir/docker-compose.override.yaml" if [[ -f "$override_yml" && -f "$override_yaml" ]]; then @@ -79,12 +88,47 @@ for override_file in "$override_yml" "$override_yaml"; do done compose_command=(docker compose --env-file "$env_file" "${compose_files[@]}") -printf 'Stopping the stack...\n' -if [[ "$remove_volumes" == '1' ]]; then - printf 'Also removing the codeman-node-modules/codeman-dist volumes (--volumes).\n' - "${compose_command[@]}" down --volumes -else - "${compose_command[@]}" down +# Same owner-detection Start-Codeman.sh uses to derive PUID/PGID for its own +# build — without it, the --no-cache build below gets Compose's untouched +# default of 1000:1000, and on any host whose appdata owner differs (99:100 on +# Unraid, per docker/README.md's chown example), Start-Codeman.sh's own +# correctly-PUID'd build during the handoff then rebuilds those layers with the +# right values anyway — so the "no cache, certain of what ships" image this +# script produces is not the one that actually ends up running. +# +# Deliberately NOT the same as Start-Codeman.sh's own handling of a MISSING +# appdata directory (which creates it): this script updates an EXISTING +# deployment, so a missing appdata path means there is nothing here yet to +# update, and creating one would just be this script quietly doing +# Start-Codeman.sh's first-run job worse. +appdata_path=$( + "${compose_command[@]}" config --environment | + awk -F= '$1 == "CODEMAN_APPDATA_PATH" { sub(/^[^=]*=/, ""); print; exit }' +) +if [[ -z "$appdata_path" || ! -d "$appdata_path" ]]; then + printf 'Error: CODEMAN_APPDATA_PATH is not set or does not exist: %s\n' "${appdata_path:-}" >&2 + printf 'Run docker/Start-Codeman.sh first to set up a new deployment.\n' >&2 + exit 1 +fi + +# `stat -c` is GNU, `stat -f` is BSD/macOS; the bind source lives on the Docker +# host, so both need to work. Identical to Start-Codeman.sh's own helper. +owner_of() { + stat -c '%u:%g' -- "$1" 2>/dev/null || stat -f '%u:%g' "$1" 2>/dev/null +} + +if ! owner_ids=$(owner_of "$appdata_path"); then + printf 'Error: Cannot determine the owner of CODEMAN_APPDATA_PATH: %s\n' "$appdata_path" >&2 + exit 1 +fi + +export PUID=${owner_ids%%:*} +export PGID=${owner_ids##*:} + +if [[ "$PUID" == '0' ]]; then + printf 'Error: CODEMAN_APPDATA_PATH is owned by root: %s\n' "$appdata_path" >&2 + printf 'Change the directory ownership to the unprivileged account that should run Codeman.\n' >&2 + exit 1 fi # --no-cache, always: a plain `build` reuses cached layers (npm install, apt @@ -92,17 +136,30 @@ fi # frozen at whatever they were the day the cache was populated — exactly wrong # for a major update, whose whole point is being certain of what actually # ships. `scripts/build-agent-image.mjs` makes the same call for the same -# reason (see its entry in CLAUDE.md's Additional Commands table). +# reason (see its entry in CLAUDE.md's Additional Commands table). Runs BEFORE +# the stack is stopped — see the header comment for why. printf 'Building a fresh image (--no-cache)...\n' "${compose_command[@]}" build --no-cache -# Start-Codeman.sh does everything a plain `up -d` does not: resolves -# PUID/PGID from CODEMAN_APPDATA_PATH's owner, pre-creates CODEMAN_CASES_PATH -# with the right ownership, resolves DOCKER_SOCKET_GID, records the -# server.Dockerfile/docker-compose.yaml fingerprint the in-app updater's gate -# reads on every future update, and clears the build-artefact volumes itself -# if it finds the checkout's source moved since the last start. Reimplementing -# any of that here would only risk drifting out of step with it — hand off -# instead, exactly as docs/docker-self-update.md's own reset procedure does. +printf 'Stopping the stack...\n' +if [[ "$keep_volumes" == '1' ]]; then + "${compose_command[@]}" down +else + printf 'Also clearing the codeman-node-modules/codeman-dist volumes (pass --keep-volumes to skip).\n' + "${compose_command[@]}" down --volumes +fi + +# Start-Codeman.sh does everything a plain `up -d` does not: re-derives +# PUID/PGID, pre-creates CODEMAN_CASES_PATH with the right ownership, resolves +# DOCKER_SOCKET_GID, records the server.Dockerfile/docker-compose.yaml +# fingerprint the in-app updater's gate reads on every future update, and +# starts the (already freshly built) image. Reimplementing any of that here +# would only risk drifting out of step with it — hand off instead, exactly as +# docs/docker-self-update.md's own reset procedure does. +# +# ⚠️ `bash`, not a bare exec of the path: Start-Codeman.sh is committed +# non-executable (100644), the same as this script, and is documented +# everywhere as `bash docker/Start-Codeman.sh` rather than +# `./docker/Start-Codeman.sh` — execing the bare path fails with EACCES. printf 'Handing off to Start-Codeman.sh...\n' -exec "$script_dir/Start-Codeman.sh" +exec bash "$script_dir/Start-Codeman.sh" diff --git a/docs/docker-self-update.md b/docs/docker-self-update.md index 42bd6fc0..d66592f3 100644 --- a/docs/docker-self-update.md +++ b/docs/docker-self-update.md @@ -224,7 +224,7 @@ the host and the in-app path works from then on. **Resetting the build artefacts** — `docker compose down -v`, then `Start-Codeman.sh`. This discards the named volumes and re-seeds them from a fresh -image. `docker/Update-Codeman.sh --volumes` scripts exactly this (plus an +image. `docker/Update-Codeman.sh` scripts exactly this by default (plus an unconditional `--no-cache` rebuild, which a plain `Start-Codeman.sh` run does not force on its own) — see "Major updates" in `docker/README.md`. diff --git a/test/docker-entrypoint.test.ts b/test/docker-entrypoint.test.ts index a71e5d48..8058ba34 100644 --- a/test/docker-entrypoint.test.ts +++ b/test/docker-entrypoint.test.ts @@ -21,7 +21,7 @@ */ import { describe, it, expect, beforeAll, afterAll } from 'vitest'; -import { readFileSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { readFileSync, mkdtempSync, rmSync, writeFileSync, mkdirSync } from 'node:fs'; import { execFileSync } from 'node:child_process'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; @@ -180,23 +180,32 @@ describe('Update-Codeman.sh (the scripted major-update path — docker/README.md execFileSync('bash', ['-n', join(ROOT, 'docker/Update-Codeman.sh')]); }); - it('stops the stack, THEN force-rebuilds with --no-cache, THEN hands off to Start-Codeman.sh', () => { - const down = updateScript.indexOf('"${compose_command[@]}" down'); + it('force-rebuilds with --no-cache BEFORE stopping the stack, THEN hands off to Start-Codeman.sh via `bash`', () => { const build = updateScript.indexOf('"${compose_command[@]}" build --no-cache'); - const handoff = updateScript.indexOf('exec "$script_dir/Start-Codeman.sh"'); - expect(down).toBeGreaterThan(-1); - expect(build).toBeGreaterThan(down); - expect(handoff).toBeGreaterThan(build); + const down = updateScript.indexOf('"${compose_command[@]}" down'); + const handoff = updateScript.indexOf('exec bash "$script_dir/Start-Codeman.sh"'); + expect(build).toBeGreaterThan(-1); + expect(down).toBeGreaterThan(build); + expect(handoff).toBeGreaterThan(down); + // A bare `exec "$script_dir/Start-Codeman.sh"` fails EACCES — Start-Codeman.sh + // is committed non-executable (100644), same as this script. + expect(updateScript).not.toMatch(/exec "\$script_dir\/Start-Codeman\.sh"/); }); - it('--volumes (or -v) removes the named volumes on the way down; the default path does not', () => { - expect(updateScript).toMatch(/--volumes \| -v\)\s*\n\s*remove_volumes=1/); + it('clears the named volumes by DEFAULT; --keep-volumes opts out to a plain `down`', () => { + expect(updateScript).toMatch(/--keep-volumes\)\s*\n\s*keep_volumes=1/); expect(updateScript).toMatch(/"\$\{compose_command\[@\]\}" down --volumes/); - // The unconditional call further down (the else branch) must stay a plain - // `down` — accidentally merging the two branches would silently start - // wiping the build-artefact volumes on every major update, not just when - // the flag is passed. - expect(updateScript).toMatch(/else\s*\n\s*"\$\{compose_command\[@\]\}" down\s*\n\s*fi/); + // The keep_volumes branch must stay a plain `down` — merging the two would + // silently start wiping the build-artefact volumes even when asked not to. + expect(updateScript).toMatch( + /if \[\[ "\$keep_volumes" == '1' \]\]; then\s*\n\s*"\$\{compose_command\[@\]\}" down\s*\n\s*else/ + ); + }); + + it('--help/-h prints usage and exits 0, rather than falling into the unrecognised-argument branch', () => { + expect(updateScript).toMatch(/--help \| -h\)\s*\n\s*printf 'Usage:/); + const helpBlock = updateScript.slice(updateScript.indexOf('--help | -h)'), updateScript.indexOf('*)')); + expect(helpBlock).toMatch(/exit 0/); }); it('rejects an unrecognised argument rather than silently ignoring it', () => { @@ -210,12 +219,187 @@ describe('Update-Codeman.sh (the scripted major-update path — docker/README.md // endings on disk and not the other's (e.g. Start-Codeman.sh checked out before this // script existed), which would fail a raw byte comparison for a reason that has // nothing to do with the two scripts actually agreeing. + const normalise = (s: string) => s.replace(/\r\n/g, '\n'); const overrideBlock = (script: string) => - script - .slice(script.indexOf('override_yml='), script.indexOf('compose_command=(docker compose')) - .replace(/\r\n/g, '\n'); + normalise(script.slice(script.indexOf('override_yml='), script.indexOf('compose_command=(docker compose'))); expect(overrideBlock(updateScript)).toBe(overrideBlock(startScript)); }); + + it('derives PUID/PGID from the SAME owner_of() helper Start-Codeman.sh uses, so the --no-cache build gets the right build args', () => { + const normalise = (s: string) => s.replace(/\r\n/g, '\n'); + const ownerOfBlock = (script: string) => { + const start = script.indexOf('owner_of() {'); + const end = script.indexOf('\n}', start) + '\n}'.length; + return normalise(script.slice(start, end)); + }; + expect(ownerOfBlock(updateScript)).toBe(ownerOfBlock(startScript)); + expect(updateScript).toMatch(/export PUID=\$\{owner_ids%%:\*\}/); + expect(updateScript).toMatch(/export PGID=\$\{owner_ids##\*:\}/); + // The build must come AFTER PUID/PGID are resolved and exported, or Compose + // falls back to its own default of 1000:1000 for the build args. + const puidExport = updateScript.indexOf('export PUID='); + const build = updateScript.indexOf('"${compose_command[@]}" build --no-cache'); + expect(puidExport).toBeGreaterThan(-1); + expect(build).toBeGreaterThan(puidExport); + }); + + describe('end-to-end smoke test (a stub `docker` on PATH, logging every invocation)', () => { + /** + * Reproduces the exact scenario the review on PR #465 caught by hand: a bare + * `exec` of a non-executable script exits 126 with no further `docker` calls + * at all. Runs the REAL Update-Codeman.sh against a synthetic deployment, + * asserting the actual command sequence a shell would issue — string-matching + * the source (the tests above) cannot tell a working `exec bash "…"` apart + * from a silently-broken bare `exec "…"` the way actually running it can. + * + * The harness intentionally does NOT create a real Unix socket for + * DOCKER_SOCKET (net.createServer().listen(path) is unreliable off Linux — + * measured EACCES on this Windows sandbox even outside any container). So the + * handoff to Start-Codeman.sh is real and fully exercises this script's own + * build/down/handoff sequence, but Start-Codeman.sh's OWN socket check is + * expected to then fail — which is itself the proof the handoff worked: a + * process that failed to exec would never reach a Start-Codeman.sh-only error + * message, and would exit 126, not 1. + */ + // Windows join()/mkdtempSync() paths carry backslashes, which the stub + // `docker`'s naive `source "$envfile"` (a shortcut for `docker compose + // config --environment`'s own real parsing, which handles this fine) reads + // as bash ESCAPE characters and silently drops — `C:\Users\x` becomes + // `C:Usersx`. Forward slashes are accepted by git-bash/MSYS on Windows and + // by every POSIX shell, so normalising once here sidesteps a harness + // artifact that has nothing to do with the scripts under test. + const posix = (p: string) => p.replace(/\\/g, '/'); + + function runSmokeTest(args: string[]): { status: number; stderr: string; log: string[] } { + const dir = mkdtempSync(join(tmpdir(), 'codeman-update-smoke-')); + try { + const dockerDir = join(dir, 'docker'); + mkdirSync(dockerDir); + writeFileSync(join(dockerDir, 'Update-Codeman.sh'), updateScript); + writeFileSync(join(dockerDir, 'Start-Codeman.sh'), startScript); + writeFileSync(join(dockerDir, 'docker-compose.yaml'), compose); + + const appdataPath = join(dir, 'appdata'); + const casesPath = join(dir, 'cases'); + const socketPath = join(dir, 'docker.sock'); // deliberately NOT a real socket — see above + mkdirSync(appdataPath); + mkdirSync(casesPath); + writeFileSync(socketPath, ''); + + writeFileSync( + join(dockerDir, '.env'), + [ + `CODEMAN_APPDATA_PATH=${posix(appdataPath)}`, + `CODEMAN_CASES_PATH=${posix(casesPath)}`, + `DOCKER_SOCKET=${posix(socketPath)}`, + 'CODEMAN_RUNTIME_USER=codeman', + 'CODEMAN_PORT=3000', + 'CODEMAN_HOST=127.0.0.1', + 'CODEMAN_PASSWORD=x', + 'CODEMAN_USERNAME=admin', + 'GEMINI_API_KEY=', + 'CODEMAN_DOCKER_BRIDGE_HOOKS=', + 'CODEMAN_DOCKER_DISABLE_SWAP_LIMIT=', + 'TZ=UTC', + 'CODEMAN_IMAGE=codeman:test', + '', + ].join('\n') + ); + + // A stub `docker` that only understands the two `compose config` shapes + // both scripts actually issue, and logs every invocation verbatim — + // written and chmod+x'd from WITHIN one bash invocation (not + // fs.chmodSync, whose Win32 backing does not reliably set the bit this + // MSYS bash's own PATH lookup honours — measured, differs from a plain + // `chmod +x` issued by bash itself). + const binDir = join(dir, 'bin'); + mkdirSync(binDir); + const stub = [ + '#!/usr/bin/env bash', + 'echo "docker $*" >> "$CMDLOG"', + 'if [[ "$1" == "compose" ]]; then', + ' shift', + ' prev=""', + ' envfile=""', + ' for a in "$@"; do', + ' if [[ "$prev" == "--env-file" ]]; then envfile="$a"; fi', + ' prev="$a"', + ' done', + ' if [[ " $* " == *" config "* && " $* " == *" --environment "* ]]; then', + ' source "$envfile"', + ' echo "CODEMAN_APPDATA_PATH=$CODEMAN_APPDATA_PATH"', + ' echo "CODEMAN_CASES_PATH=$CODEMAN_CASES_PATH"', + ' echo "DOCKER_SOCKET=$DOCKER_SOCKET"', + ' exit 0', + ' fi', + ' if [[ " $* " == *" config "* && " $* " == *" --format json "* ]]; then', + ' echo \'{"name":"codeman"}\'', + ' exit 0', + ' fi', + ' exit 0', + 'fi', + 'exit 0', + ].join('\n'); + const stubPath = join(binDir, 'docker'); + writeFileSync(stubPath, stub); + execFileSync('bash', ['-c', `chmod +x '${stubPath}'`]); + + const logPath = join(dir, 'cmdlog.txt'); + writeFileSync(logPath, ''); + + let status = 0; + let stderr = ''; + try { + execFileSync('bash', [join(dockerDir, 'Update-Codeman.sh'), ...args], { + env: { ...process.env, PATH: `${binDir}:${process.env.PATH}`, CMDLOG: logPath }, + encoding: 'utf-8', + }); + } catch (err) { + const e = err as { status?: number; stderr?: string }; + status = e.status ?? 1; + stderr = e.stderr ?? ''; + } + + const log = readFileSync(logPath, 'utf-8') + .split('\n') + .filter((l) => l.trim()); + return { status, stderr, log }; + } finally { + rmSync(dir, { recursive: true, force: true }); + } + } + + it('default: build --no-cache, THEN down --volumes, THEN the handoff genuinely runs Start-Codeman.sh', () => { + const { status, stderr, log } = runSmokeTest([]); + + const buildIdx = log.findIndex((l) => l.includes('build --no-cache')); + const downIdx = log.findIndex((l) => l.includes(' down --volumes') || l.endsWith(' down')); + expect(buildIdx).toBeGreaterThan(-1); + expect(downIdx).toBeGreaterThan(buildIdx); + expect(log[downIdx]).toContain('down --volumes'); + + // Proof the handoff really executed Start-Codeman.sh rather than dying + // with EACCES right after printing "Handing off...": more `docker` + // invocations appear AFTER the down, which only Start-Codeman.sh's own + // config-resolution lines would produce. + const configCallsAfterDown = log.slice(downIdx + 1).filter((l) => l.includes('config')); + expect(configCallsAfterDown.length).toBeGreaterThan(0); + + // A working handoff fails HONESTLY at Start-Codeman.sh's own socket + // check (this harness deliberately supplies no real Unix socket) — never + // with an EACCES/126 from a broken `exec`. + expect(status).toBe(1); + expect(stderr).toMatch(/DOCKER_SOCKET is not a Unix socket/); + expect(stderr).not.toMatch(/permission denied/i); + }); + + it('--keep-volumes: a plain `down`, with no --volumes flag', () => { + const { log } = runSmokeTest(['--keep-volumes']); + const downLine = log.find((l) => / down(\s|$)/.test(l)); + expect(downLine).toBeDefined(); + expect(downLine).not.toContain('--volumes'); + }); + }); }); describe('git_head_commit resolves every ref layout a checkout can have', () => {