From 9ba90a674ad3b4e6d38373c814321b1ffbb6fe2e Mon Sep 17 00:00:00 2001 From: Devvyn <22340871+opticon454@users.noreply.github.com> Date: Mon, 21 Sep 2026 13:57:54 +0800 Subject: [PATCH 1/4] chore(docker): add Update-Codeman.sh for scripted major-update rebuilds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit docker/README.md and docs/docker-self-update.md both already point operators at "stop the stack, rebuild, restart" for anything the in-app updater refuses to apply (a changed server.Dockerfile, a changed docker-compose.yaml, or a new required .env key) — but that was a manual, hand-typed procedure with no script of its own, unlike every other start/update path this deployment has. docker/Update-Codeman.sh scripts it: `docker compose down`, then an unconditional `docker compose build --no-cache` (a major update should be certain of what actually ships, not reuse whatever layers happened to be cached), then hands off to the existing Start-Codeman.sh for the same careful PUID/PGID, override-file and fingerprint handling every other start already goes through — rather than reimplementing any of that by hand and risking it drifting out of step. An optional --volumes/-v flag also removes the codeman-node-modules/ codeman-dist named volumes, the scripted form of the "Resetting the build artefacts" procedure docs/docker-self-update.md already documents by hand. Safe: those two are the only named volumes this stack declares; application data and case workspaces are host bind mounts, never touched by `docker compose down` either way. Docs updated: a "Major updates" section in docker/README.md, and a pointer from docs/docker-self-update.md's existing "Resetting the build artefacts" troubleshooting entry. Tests: extended test/docker-entrypoint.test.ts (the existing home for Start-Codeman.sh's own static checks) with a bash -n parse check, the down-before-build-before-handoff ordering, the --volumes flag's effect, unrecognised-argument handling, and byte-for-byte agreement with Start-Codeman.sh's own override-file resolution logic (so `down` here and `up` there can never target different Compose files). Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01N6eadpRyqpA9PD3i139cSD --- docker/README.md | 20 ++++++ docker/Update-Codeman.sh | 108 +++++++++++++++++++++++++++++++++ docs/docker-self-update.md | 4 +- test/docker-entrypoint.test.ts | 56 ++++++++++++++++- 4 files changed, 186 insertions(+), 2 deletions(-) create mode 100644 docker/Update-Codeman.sh diff --git a/docker/README.md b/docker/README.md index cdd169f5..91fded40 100644 --- a/docker/README.md +++ b/docker/README.md @@ -41,6 +41,26 @@ Releases that change `server.Dockerfile`, `docker-compose.yaml`, or add a key to changed, and asks you to run `Start-Codeman.sh` here on the host instead. Details: [`../docs/docker-self-update.md`](../docs/docker-self-update.md). +### 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: + +```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 +[`../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. + ## Local customisation Compose merges `docker-compose.override.yml` on top of `docker-compose.yaml`. Keep host-specific changes there rather than editing `docker-compose.yaml`, so this repository can be updated without losing them. Both `docker-compose.override.yml` and `docker-compose.override.yaml` are ignored by Git. diff --git a/docker/Update-Codeman.sh b/docker/Update-Codeman.sh new file mode 100644 index 00000000..63cdf497 --- /dev/null +++ b/docker/Update-Codeman.sh @@ -0,0 +1,108 @@ +#!/usr/bin/env bash +# +# The scripted major-update path for the Docker Compose deployment. +# +# docker/README.md and docs/docker-self-update.md both point operators here for +# anything the in-app updater itself refuses to apply: a changed +# `server.Dockerfile`, a changed `docker-compose.yaml`, or a new required +# `.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 +# 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". +# +# 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. + +set -euo pipefail + +script_dir=$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd) +env_file="$script_dir/.env" +compose_file="$script_dir/docker-compose.yaml" + +remove_volumes=0 +for arg in "$@"; do + case "$arg" in + --volumes | -v) + remove_volumes=1 + ;; + *) + printf 'Error: unrecognised argument: %s\n' "$arg" >&2 + printf 'Usage: %s [--volumes]\n' "$0" >&2 + exit 1 + ;; + esac +done + +if [[ ! -f "$env_file" ]]; then + printf 'Error: Docker environment file is missing: %s\n' "$env_file" >&2 + printf 'Create it from %s/.env.example before running this script.\n' "$script_dir" >&2 + exit 1 +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). +override_yml="$script_dir/docker-compose.override.yml" +override_yaml="$script_dir/docker-compose.override.yaml" +if [[ -f "$override_yml" && -f "$override_yaml" ]]; then + printf 'Warning: both %s and %s exist; Compose uses .yml and ignores .yaml.\n' \ + "$override_yml" "$override_yaml" >&2 +fi +compose_files=(-f "$compose_file") +for override_file in "$override_yml" "$override_yaml"; do + if [[ -f "$override_file" ]]; then + compose_files+=(-f "$override_file") + printf 'Using Compose override file: %s\n' "$override_file" + break + fi +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 +fi + +# --no-cache, always: a plain `build` reuses cached layers (npm install, apt +# packages, the CLI installs baked into the image) and can silently keep them +# 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). +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 'Handing off to Start-Codeman.sh...\n' +exec "$script_dir/Start-Codeman.sh" diff --git a/docs/docker-self-update.md b/docs/docker-self-update.md index 6f41505b..42bd6fc0 100644 --- a/docs/docker-self-update.md +++ b/docs/docker-self-update.md @@ -224,7 +224,9 @@ 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. +image. `docker/Update-Codeman.sh --volumes` scripts exactly this (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`. ## Disabling it diff --git a/test/docker-entrypoint.test.ts b/test/docker-entrypoint.test.ts index 94823c21..b4a19792 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, statSync } from 'node:fs'; import { execFileSync } from 'node:child_process'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; @@ -33,6 +33,7 @@ const compose = read('docker/docker-compose.yaml'); const entrypoint = read('docker/entrypoint.sh'); const dockerfile = read('docker/server.Dockerfile'); const startScript = read('docker/Start-Codeman.sh'); +const updateScript = read('docker/Update-Codeman.sh'); /** The `- NAME` entries under `cap_add:` (the block ends at the next key at the same indent). */ function composeCapAdd(text: string): string[] { @@ -174,6 +175,59 @@ describe('Start-Codeman.sh', () => { }); }); +describe('Update-Codeman.sh (the scripted major-update path — docker/README.md "Major updates")', () => { + it('parses under bash -n', () => { + execFileSync('bash', ['-n', join(ROOT, 'docker/Update-Codeman.sh')]); + }); + + it('is executable, like every other script this deployment runs directly', () => { + // Windows checkouts (this repo is developed on both) do not carry a real + // execute bit, so this only meaningfully asserts on POSIX — matching how + // docker/README.md documents running it (`bash docker/Update-Codeman.sh`, + // not `./docker/Update-Codeman.sh`) either way. + if (process.platform === 'win32') return; + const mode = statSync(join(ROOT, 'docker/Update-Codeman.sh')).mode; + expect(mode & 0o111).not.toBe(0); + }); + + it('stops the stack, THEN force-rebuilds with --no-cache, THEN hands off to Start-Codeman.sh', () => { + const down = updateScript.indexOf('"${compose_command[@]}" down'); + 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); + }); + + 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/); + 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/); + }); + + it('rejects an unrecognised argument rather than silently ignoring it', () => { + expect(updateScript).toMatch(/Error: unrecognised argument/); + expect(updateScript).toMatch(/exit 1/); + }); + + it('resolves the override file exactly like Start-Codeman.sh, so `down` and `up` never target different Compose files', () => { + // \r stripped before comparing: git's autocrlf normalises the COMMITTED blob to LF + // either way, but a Windows checkout can have already converted one file's line + // 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 overrideBlock = (script: string) => + script + .slice(script.indexOf('override_yml='), script.indexOf('compose_command=(docker compose')) + .replace(/\r\n/g, '\n'); + expect(overrideBlock(updateScript)).toBe(overrideBlock(startScript)); + }); +}); + describe('git_head_commit resolves every ref layout a checkout can have', () => { let base: string; const git = (cwd: string, ...args: string[]) => From 3b714446b4bbba4d07b329e4a3e675bafd546843 Mon Sep 17 00:00:00 2001 From: Devvyn <22340871+opticon454@users.noreply.github.com> Date: Mon, 21 Sep 2026 13:58:45 +0800 Subject: [PATCH 2/4] fix(docker): drop the wrong executable-bit assertion for Update-Codeman.sh MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Start-Codeman.sh, its sibling and the script it hands off to, is itself committed non-executable (100644) upstream — it's documented and invoked as `bash docker/Start-Codeman.sh`, never `./docker/Start-Codeman.sh`. The "is executable" test I'd added for Update-Codeman.sh asserted the opposite convention, which the file correctly does not follow. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01N6eadpRyqpA9PD3i139cSD --- test/docker-entrypoint.test.ts | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/test/docker-entrypoint.test.ts b/test/docker-entrypoint.test.ts index b4a19792..a71e5d48 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, statSync } from 'node:fs'; +import { readFileSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; import { execFileSync } from 'node:child_process'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; @@ -180,16 +180,6 @@ describe('Update-Codeman.sh (the scripted major-update path — docker/README.md execFileSync('bash', ['-n', join(ROOT, 'docker/Update-Codeman.sh')]); }); - it('is executable, like every other script this deployment runs directly', () => { - // Windows checkouts (this repo is developed on both) do not carry a real - // execute bit, so this only meaningfully asserts on POSIX — matching how - // docker/README.md documents running it (`bash docker/Update-Codeman.sh`, - // not `./docker/Update-Codeman.sh`) either way. - if (process.platform === 'win32') return; - const mode = statSync(join(ROOT, 'docker/Update-Codeman.sh')).mode; - expect(mode & 0o111).not.toBe(0); - }); - it('stops the stack, THEN force-rebuilds with --no-cache, THEN hands off to Start-Codeman.sh', () => { const down = updateScript.indexOf('"${compose_command[@]}" down'); const build = updateScript.indexOf('"${compose_command[@]}" build --no-cache'); 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 3/4] =?UTF-8?q?fix(docker):=20address=20Ark0N's=20PR=20rev?= =?UTF-8?q?iew=20on=20Update-Codeman.sh=20=E2=80=94=20fix=20the=20handoff,?= =?UTF-8?q?=20the=20build/down=20ordering,=20and=20default-clear=20the=20b?= =?UTF-8?q?uild=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', () => { From a2dcc91ddf3d876f4f5000869b3d20d65115613c Mon Sep 17 00:00:00 2001 From: Devvyn <22340871+opticon454@users.noreply.github.com> Date: Mon, 21 Sep 2026 21:23:39 +0800 Subject: [PATCH 4/4] fix(docker): add and correct the cross-checkout collision guard for Update-Codeman.sh MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Same guard as Start-Codeman.sh's own (docs/docker-self-update.md-adjacent incident, 2026-09-21): docker-compose.yaml hard-codes `name: codeman`, so a second checkout run without COMPOSE_PROJECT_NAME resolves to the SAME Compose project as any other checkout on the host. It has to live here too, not just in Start-Codeman.sh: this script's own --no-cache build and its `down`/`down --volumes` both run BEFORE the handoff at the bottom of the file, so Start-Codeman.sh's copy of the guard would only fire after this script's own destructive calls already ran — and its default `down --volumes` is more destructive than Start-Codeman.sh's own targeted refresh, clearing every named volume the resolved project has. Also fixes a real bug the same guard shipped with: under `set -o pipefail`, `grep -v` legitimately exits 1 when nothing survives the filter (the ordinary, no-collision case), and without `|| true` on the pipeline that non-zero status propagates through the command substitution and `set -e` aborts the WHOLE script at the guard — every time, collision or not. Caught only by actually executing the guard end-to-end against a stub `docker` (the existing smoke-test harness), never by a static text/regex check on the source; the stub's `config --format json` response was also fixed to pretty-print like real Compose does, since a compact one-liner silently resolved project_name to empty and exercised neither script's guard the way production output does. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01GuHtuPiHXdykq9T6rKQJ9n --- docker/Update-Codeman.sh | 47 +++++++++++++++++++++++++++++ test/docker-entrypoint.test.ts | 54 ++++++++++++++++++++++++++++++++-- 2 files changed, 98 insertions(+), 3 deletions(-) diff --git a/docker/Update-Codeman.sh b/docker/Update-Codeman.sh index ba9fb2fd..cc318ed0 100644 --- a/docker/Update-Codeman.sh +++ b/docker/Update-Codeman.sh @@ -88,6 +88,53 @@ for override_file in "$override_yml" "$override_yaml"; do done compose_command=(docker compose --env-file "$env_file" "${compose_files[@]}") +# Same collision guard as Start-Codeman.sh, and load-bearing HERE rather than +# left to that script's own copy: this script's --no-cache build and its +# `down`/`down --volumes` (below) both run BEFORE the handoff at the bottom of +# this file, so Start-Codeman.sh's guard would only fire after the damage this +# one exists to prevent has already happened. docker-compose.yaml hard-codes +# `name: codeman`, so a second checkout run without COMPOSE_PROJECT_NAME +# resolves to the SAME Compose project as any other checkout on the host and +# operates on ITS containers and volumes — this script's default `down +# --volumes` makes that worse than Start-Codeman.sh's own targeted refresh, +# since it clears every named volume the resolved project has, not just the +# two this script means to. See Start-Codeman.sh's own guard for the full +# incident this is written against (2026-09-21). +project_name=$( + "${compose_command[@]}" config --format json 2>/dev/null | + sed -n 's/^[[:space:]]*"name":[[:space:]]*"\([^"]*\)".*$/\1/p' | head -n1 +) +if [[ -n "$project_name" ]]; then + # `|| true` on the pipeline's LAST command: under `set -o pipefail`, `grep -v` + # exits 1 when nothing survives the filter — the ordinary, no-collision case, + # since `docker ps` finds nothing at all on a first-ever deployment or a + # single matching (own) working_dir gets filtered out. Without it, that exit + # status propagates through the command substitution and `set -e` aborts the + # WHOLE script right here, every time, regardless of whether a collision + # actually exists — caught only by actually running this end-to-end (a + # static text/regex check on the source cannot see it). + other_working_dir=$( + docker ps -a --filter "label=com.docker.compose.project=$project_name" \ + --format '{{.Label "com.docker.compose.project.working_dir"}}' 2>/dev/null | + grep -v -F -x -- "$script_dir" | head -n1 || true + ) + if [[ -n "$other_working_dir" ]]; then + printf 'Error: Compose project "%s" is already in use by a DIFFERENT checkout:\n' "$project_name" >&2 + printf ' %s\n' "$other_working_dir" >&2 + printf 'This checkout is:\n' >&2 + printf ' %s\n' "$script_dir" >&2 + printf '\n' >&2 + printf 'docker-compose.yaml hard-codes `name: %s`, so two checkouts on the same host\n' "$project_name" >&2 + printf 'collide unless each one sets a distinct COMPOSE_PROJECT_NAME. Continuing would\n' >&2 + printf 'rebuild and stop the OTHER checkout'"'"'s running container and, by default,\n' >&2 + printf 'delete ALL of its named volumes.\n' >&2 + printf '\n' >&2 + printf 'Fix: export COMPOSE_PROJECT_NAME= before\n' >&2 + printf 'running this script, then retry.\n' >&2 + exit 1 + fi +fi + # 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 diff --git a/test/docker-entrypoint.test.ts b/test/docker-entrypoint.test.ts index 8058ba34..c4a8cabe 100644 --- a/test/docker-entrypoint.test.ts +++ b/test/docker-entrypoint.test.ts @@ -192,6 +192,24 @@ describe('Update-Codeman.sh (the scripted major-update path — docker/README.md expect(updateScript).not.toMatch(/exec "\$script_dir\/Start-Codeman\.sh"/); }); + it('resolves the collision guard BEFORE the --no-cache build and the down, not after', () => { + // This script's own build/down run before the handoff to Start-Codeman.sh, + // so its copy of the guard has to be early here too - Start-Codeman.sh's + // copy alone would only catch the collision after this script's own + // destructive calls already ran. + const projectName = updateScript.indexOf('project_name=$('); + const guard = updateScript.indexOf('other_working_dir=$('); + const build = updateScript.indexOf('"${compose_command[@]}" build --no-cache'); + const down = updateScript.indexOf('"${compose_command[@]}" down'); + expect(projectName).toBeGreaterThan(-1); + expect(guard).toBeGreaterThan(projectName); + expect(guard).toBeLessThan(build); + expect(guard).toBeLessThan(down); + expect(updateScript).toMatch(/label=com\.docker\.compose\.project=\$project_name/); + expect(updateScript).toMatch(/\{\{\.Label "com\.docker\.compose\.project\.working_dir"\}\}/); + expect(updateScript).toMatch(/grep -v -F -x -- "\$script_dir"/); + }); + 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/); @@ -270,7 +288,10 @@ describe('Update-Codeman.sh (the scripted major-update path — docker/README.md // 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[] } { + function runSmokeTest( + args: string[], + extraEnv: Record = {} + ): { status: number; stderr: string; log: string[] } { const dir = mkdtempSync(join(tmpdir(), 'codeman-update-smoke-')); try { const dockerDir = join(dir, 'docker'); @@ -333,11 +354,26 @@ describe('Update-Codeman.sh (the scripted major-update path — docker/README.md ' exit 0', ' fi', ' if [[ " $* " == *" config "* && " $* " == *" --format json "* ]]; then', - ' echo \'{"name":"codeman"}\'', + // Real `docker compose config --format json` pretty-prints, so + // `"name"` starts its OWN line rather than sharing one with `{` - + // the sed extraction both scripts use anchors on that, and a + // compact one-liner here would silently resolve project_name to + // empty, exercising neither script's guard the way real Compose + // output does. + ' printf \'{\\n "name": "codeman"\\n}\\n\'', ' exit 0', ' fi', ' exit 0', 'fi', + // Mirrors the guard's own `docker ps -a --filter ... --format + // '{{.Label "com.docker.compose.project.working_dir"}}'` call. + // Empty by default (no collision) so the existing smoke tests above + // see no output here and proceed exactly as before; a test that + // wants to exercise the guard itself sets STUB_PS_WORKING_DIR. + 'if [[ "$1" == "ps" && -n "${STUB_PS_WORKING_DIR:-}" ]]; then', + ' echo "$STUB_PS_WORKING_DIR"', + ' exit 0', + 'fi', 'exit 0', ].join('\n'); const stubPath = join(binDir, 'docker'); @@ -351,7 +387,7 @@ describe('Update-Codeman.sh (the scripted major-update path — docker/README.md let stderr = ''; try { execFileSync('bash', [join(dockerDir, 'Update-Codeman.sh'), ...args], { - env: { ...process.env, PATH: `${binDir}:${process.env.PATH}`, CMDLOG: logPath }, + env: { ...process.env, PATH: `${binDir}:${process.env.PATH}`, CMDLOG: logPath, ...extraEnv }, encoding: 'utf-8', }); } catch (err) { @@ -399,6 +435,18 @@ describe('Update-Codeman.sh (the scripted major-update path — docker/README.md expect(downLine).toBeDefined(); expect(downLine).not.toContain('--volumes'); }); + + it('refuses BEFORE the --no-cache build when the resolved project belongs to a different checkout', () => { + // The whole reason this guard lives here rather than only in + // Start-Codeman.sh: this script's own build/down run before the handoff + // ever reaches that script's copy of the same check. + const { status, stderr, log } = runSmokeTest([], { STUB_PS_WORKING_DIR: '/some/other/checkout/docker' }); + expect(status).toBe(1); + expect(stderr).toMatch(/already in use by a DIFFERENT checkout/); + expect(stderr).toContain('/some/other/checkout/docker'); + expect(log.some((l) => l.includes('build --no-cache'))).toBe(false); + expect(log.some((l) => / down(\s|$)/.test(l))).toBe(false); + }); }); });