fix(docker): add and correct the cross-checkout collision guard for Update-Codeman.sh

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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GuHtuPiHXdykq9T6rKQJ9n
2026-10-03 05:59:43 +02:00 · 2026-09-21 21:23:39 +08:00
parent 5b4878df3b
commit a2dcc91ddf
2 changed files with 98 additions and 3 deletions
@@ -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=<something-unique-to-this-checkout> 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
@@ -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<string, string> = {}
+    ): { 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);
+    });
  });
 });