Skip to content

chore(docker): add Update-Codeman.sh for scripted major-update rebuilds - #465

Merged
Ark0N merged 4 commits into
Ark0N:masterfrom
opticon454:chore/docker-major-update-script
Sep 23, 2026
Merged

Ark0N merged 4 commits into
Ark0N:masterfrom
opticon454:chore/docker-major-update-script

Conversation

@opticon454

@opticon454 opticon454 commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • docker/README.md and docs/docker-self-update.md both already tell operators to stop the stack, rebuild, and restart for anything the in-app updater itself 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.
  • Adds docker/Update-Codeman.sh: 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 and risking drift.
  • 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: a new "Major updates" section in docker/README.md, and a pointer from docs/docker-self-update.md's existing "Resetting the build artefacts" entry.

Test plan

  • bash -n docker/Update-Codeman.sh (also verified against a real bash:3.2 container, matching this repo's own bash-3.2-compatibility bar for its other install scripts)
  • Extended test/docker-entrypoint.test.ts (the existing home for Start-Codeman.sh's own static checks): parse check, down-before-build-before-handoff ordering, the --volumes flag's effect (and that the default path stays a plain down), 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)
  • npm run typecheck — clean
  • npx eslint --config config/eslint.config.js "src/**/*.ts" — clean (no src/ changes, ran for completeness)
  • npx prettier --check on the changed .ts file — clean (docker/README.md/docs/docker-self-update.md are markdown, outside this repo's own npm run format glob — hand-formatted to match each file's existing style instead of running a bare prettier --write, which reflowed unrelated pre-existing content the first time I tried it)
  • Full test file run: only the 8 pre-existing failures in the neighbouring Start-Codeman.sh/cap_add describe blocks, reproduced identically on a clean upstream/master checkout — a CRLF-on-Windows artifact from this sandbox's core.autocrlf=true, unrelated to this change and invisible on the Linux CI runner

opticon454 and others added 2 commits September 21, 2026 13:57
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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N6eadpRyqpA9PD3i139cSD
…an.sh

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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N6eadpRyqpA9PD3i139cSD
@Ark0N

Ark0N commented Sep 21, 2026

Copy link
Copy Markdown
Owner

Thanks for this. Scripting the major-update path has been an obvious gap for a while, and the write-up in the script header plus the override-file byte-identity test are exactly the kind of care this deployment needs.

Three things before merge.

Blocker: the handoff fails on every checkout. docker/Update-Codeman.sh:108 runs exec "$script_dir/Start-Codeman.sh", but docker/Start-Codeman.sh is committed mode 100644, the point your second commit makes. So the exec gets EACCES and the script exits 126 with Permission denied, after the stack is already down and the full no-cache rebuild has finished. I reproduced it with a stub docker on PATH: down, build, then Permission denied, exit 126. Please change it to exec bash "$script_dir/Start-Codeman.sh". Related: the five new tests are all string matches over the script source, so nothing in them executes the handoff. A short smoke test (mkdtemp, copy the two scripts plus a minimal .env, put a stub docker that logs its arguments on PATH, run the script, assert the command sequence and a zero exit) would have caught this and would cover the ordering and the --volumes branch for real.

Build before taking the stack down. docker/Start-Codeman.sh:248 does it that way on purpose: "Build BEFORE taking the stack down: the image build is the slow part and needs no container stopped, so the deployment is offline only for the recreate." Here the down at line 85/87 comes first, so Codeman and every agent session it is running are offline for the whole no-cache rebuild, and a build failure leaves the stack stopped with nothing to bring it back (reproduced: exit 1, stack down). Nothing about the build needs the containers stopped, and down --volumes still does the right thing after it. Please reorder to build, then down, then hand off, and adjust the ordering assertion in test/docker-entrypoint.test.ts:183.

The default path can throw the rebuild away. codeman-node-modules and codeman-dist are seeded from the image only while empty, and on the default path neither this script nor the handoff clears them (Start-Codeman.sh only clears them when HEAD or package-lock.json moved). So bash docker/Update-Codeman.sh without a git pull prints "Building a fresh image (--no-cache)..." and then brings up a container still running the previous dist. Your script header states the mechanism exactly; docker/README.md:58 does not, and reads as though --volumes is optional tidying. Either make clearing those two volumes the default here (with a --keep-volumes opt-out) or add a sentence to the README saying --volumes is what makes a rebuild's application code actually run. Your call which, I am happy with either.

Smaller things, fine to fold into the same push or leave to me at merge:

  • docker/Update-Codeman.sh:97: the no-cache build gets Compose's default PUID/PGID of 1000:1000, because Start-Codeman.sh:78 derives them from the appdata owner and this script does not. On the Unraid layout in the README (99:100) the no-cache build therefore builds the wrong runtime account, and the handoff rebuilds those layers with the right values, so the image that ships is not the one the no-cache build produced.
  • docker/README.md:46: Start-Codeman.sh rebuilds on every start; only the volume clearing is conditional on HEAD or package-lock.json moving.
  • docker/Update-Codeman.sh:24 and :49: the usage strings omit the bash prefix that the README uses and that the non-executable mode requires. --help also lands in the unrecognised-argument branch.
  • docker/Update-Codeman.sh:30: "the ONLY named volumes this stack declares" is true of docker-compose.yaml, but the command also loads the user's docker-compose.override.yml, which the README recommends for host-specific changes.

Everything else checks out here: typecheck, lint, format, frontend syntax and the full npm test gate are all green (414 files, 7833 tests), the file parses under bash 3.2, and your reading of the Prettier scope for the two markdown files is right. Fix the exec and the ordering and I will merge.

… handoff, the build/down ordering, and default-clear the build volumes

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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N6eadpRyqpA9PD3i139cSD
@opticon454

opticon454 commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor Author

@Ark0N Thanks for reproducing it — all fixed in 5b4878d, and I verified the fix the same way you found the bug: ran the real script against a synthetic deployment with a stub docker on PATH, not just re-read the source.

  1. exec → exec bash "$script_dir/Start-Codeman.sh". Confirmed the old form dies with EACCES and the new one hands off cleanly (the process now fails, correctly, at Start-Codeman.sh's own DOCKER_SOCKET is not a Unix socket check in my harness, since I deliberately don't fake a real socket there).
  2. Reordered to build --no-cache, then down, then hand off — matches Start-Codeman.sh's own "build before taking the stack down" reasoning, now stated in this script's header too.
  3. Took the default-clear option: --keep-volumes replaces the old --volumes/-v flag. Default is now down --volumes.

Also fixed all four smaller items: PUID/PGID now derives from CODEMAN_APPDATA_PATH's owner via the identical owner_of() helper Start-Codeman.sh uses (before the build, so the --no-cache build actually gets the right args — parity-tested), the README's "only rebuilds when..." wording corrected to say only the volume-clearing is conditional, --help/-h now prints usage and exits 0, and the "only named volumes" line now says docker-compose.yaml specifically.

Per your note about the five tests being pure string matches: added a real end-to-end smoke test — synthetic deployment, stub docker on PATH logging every invocation, the actual script run as a real subprocess via execFileSync. It asserts the real command sequence and that a working handoff fails honestly at Start-Codeman.sh's own later check rather than with EACCES. It would have caught blocker #1 outright; I confirmed that by running it against the pre-fix script first.

typecheck/format clean, bash -n and a real bash:3.2 container both parse it, and the full test/docker-entrypoint.test.ts is green apart from the same 8 pre-existing CRLF-only failures already present on master in this sandbox.

🤖 Generated with Claude Code

…pdate-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
@Ark0N
Ark0N merged commit 5b5e932 into Ark0N:master Sep 23, 2026
2 checks passed
Ark0N pushed a commit that referenced this pull request Sep 23, 2026
- Remove exactly the codeman-node-modules/codeman-dist volumes by Compose
  label after a plain `down`, instead of `down --volumes` (which also takes
  any volume an override file declares while the message named two).
  `down --volumes` remains only as a warned fallback when the project name
  cannot be resolved.
- Report a failing first `docker compose config --format json` call with a
  clear error instead of exiting silently under `set -e`.
- Filter empty label lines in the collision guard so an unlabelled container
  cannot hide a real collision; name the moved-checkout exit in its error.
- Comments no longer cite a guard or incident in Start-Codeman.sh that does
  not exist; the README states the real gap (a Node base-image bump leaves
  codeman-node-modules stale because the lockfile did not move).
- docs: Update-Codeman.sh in the docker-self-update.md short-version table
  and a mention in docker-compose.md; "Major updates" moved under "Updating"
  in docker/README.md.
- test: smoke test covers the new sequence, the config failure and the
  empty-line case; quiet stdio; @fileoverview names the fourth concern.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@Ark0N

Ark0N commented Sep 23, 2026

Copy link
Copy Markdown
Owner

Merged, thanks @opticon454! This ships in 1.32.1.

The build-before-down ordering and the bash handoff from my first review are both in, and the smoke test with a stub docker is the part I like most: it would have caught the exec bug on its own.

I applied the rest at merge rather than doing another round:

  • down --volumes removed every named volume in the resolved project, override file included, while the message named two. The script now does a plain down and then docker volume rm on exactly codeman-node-modules and codeman-dist (found by project label). --volumes is only a fallback when the project name cannot be resolved, and the message says so.
  • A failing first docker compose config used to exit 1 with no output under set -e. It now shows docker's own error plus the likely causes.
  • The collision guard skips empty label lines before head -n1 (with a smoke case), and its error names the other exit: a moved or renamed checkout wants the old container removed, not a new COMPOSE_PROJECT_NAME.
  • Update-Codeman.sh is now in the short-version table in docs/docker-self-update.md and mentioned in docs/docker-compose.md; the smoke test no longer leaks stderr into the gate output.

Two of the fixes were to your own reasoning, so you should know which bits did not hold. The comments cited a collision guard and an incident in Start-Codeman.sh that do not exist there (grep finds only this script and its test), so they now describe this script's own guard. And the README said Start-Codeman.sh misses a Dockerfile-only change, but a released Dockerfile change arrives through git pull, which moves HEAD, and Start-Codeman.sh already refreshes codeman-dist for that. The real gap is a Node base-image bump that leaves the lockfile alone, so codeman-node-modules keeps the old node-pty build, plus the fact that Start-Codeman.sh never builds with --no-cache. The README says that now.

@github-actions github-actions Bot mentioned this pull request Sep 23, 2026
@opticon454
opticon454 deleted the chore/docker-major-update-script branch September 24, 2026 00:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants