fix(docker): never exit the server unless something is known to restart it

#373 restarts the Compose container by exiting the server, which is right for
the shipped deployment: `restart: unless-stopped` relaunches it. The updater
verified that policy through the Docker socket and, when it could not (no
socket mounted), failed open and exited anyway. Failing open is the correct
choice for the GATE, where refusing would block every install without a
socket, but not for the kill: a container the daemon does not restart goes
down for good, with no UI left to recover it from. That is exactly the case a
plain `docker run` of this image without `--restart` produces, and the image
sets CODEMAN_IN_CONTAINER=1 itself, so it takes the container path.

The decision now happens server-side, where both the socket and the Compose
env are reachable, and rides down to the script as `--restart-by-exit 0|1`.
It is 1 when the Compose file declared `CODEMAN_RESTART_BY_EXIT=1` (added there
and only there, since that file is what sets the restart policy; the image ENV
deliberately does not) or when the daemon confirmed an auto-restart policy.
Otherwise the build still lands, the status becomes
`completed-needs-manual-restart` with the `docker restart` hint, and the
server keeps running. The shipped deployment is unchanged in effect: with the
socket it was already confirmed, and without it the declaration now covers it.

Also: a root-run `Start-Codeman.sh` (common on Unraid) created the
fingerprint baseline's `.codeman` directory before the container's first start
and left it root-owned, which the unprivileged server could then never write
its own state into. It is chowned to PUID:PGID when running as root.

Verified with a real image build of the merged tree (classic builder; this
box's BuildKit lacks buildx): runs as uid 1000, tsc/esbuild and the toolchain
present, the four CLIs at their pins, docker/.env absent, and `docker inspect
$HOSTNAME` returns the restart policy through the mounted socket as that user.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Qg6bcATm1pNNY4kQWGwzgu
This commit is contained in:
Codeman maintainer
2026-09-04 14:36:35 +02:00
parent 823f56a243
commit 99ad9cb236
7 changed files with 91 additions and 4 deletions
+1 -1
View File
@@ -248,7 +248,7 @@ Codeman is a Claude Code session manager with web interface and autonomous Ralph
**Terminal scrollback strip + wheel/touch forwarding** (#205): codex/claude/gemini get the FULL strip (alt-screen, `3J`, mouse DECSETs); tmux-backed shell/opencode/antigravity/omp get a NARROW strip (alt-screen toggles only — it removes tmux's own attach-time `smcup`, which otherwise parks xterm in the scrollback-less alt buffer and turns the wheel into arrow keys). ⚠️ Gated on `useMux`: direct-PTY fallback sessions must keep the alt screen for vim/less/htop. Wheel AND touch forward to the CLI transcript for **claude ≥ 2.1.187 ONLY** at ANY scroll position (snap-to-bottom first); Shift+wheel and the `terminalWheelLocalScrollback` setting stay local. ⚠️ Codex was in that list and must never go back without a fresh measurement: codex-cli 0.147.0 ignores SGR wheel reports entirely (`mouse_any_flag=0`, inline viewport, transcript pushed into terminal scrollback), so forwarding produced a dead wheel (#227 follow-up). `_wheelScrollLines()` reads `ev.deltaMode` (Firefox = LINE units). ⚠️ When that gate is FALSE on a claude session whose local buffer is hollow (`baseY === 0`), the gesture becomes coalesced PageUp/PageDown key sends (`_maybePageCliTranscript`) instead of a no-op; ⚠️ and `getClaudeCliVersion()` must never cache a FAILED probe (one timeout used to disable forwarding process-wide until restart). ⚠️ **A click is hand-reported to the CLI only while the CLI actually has mouse tracking on.** The full strip removes the mouse DECSETs, so xterm's `mouseTrackingMode` is permanently `none` there and the browser hand-encodes SGR reports (`_sendSyntheticSgrTap`); without state it did that on EVERY click, so a stripped-mode pane running a plain shell (CLI exited, or a shell started inside a claude-mode session) received reports it never asked for and printed them as literal text (`[<0;88;20M`), garbling the next typed line. `_recordStrippedMouseMode()` (session.ts) records what the strip removes, `toState()` publishes `cliMouseTracking`, and `_shouldReportMouseToCli()` gates all three report sites on it. Only 1000/1001/1002/1003 count (1005/1006 are encodings, 1007 is alt-scroll), and the change broadcasts UNdebounced since a dialog can be clicked inside the 500ms window. `_logScrollRouting()` prints the routing decision and its inputs once per session — read it before diagnosing a scroll report. → [architecture-invariants#terminal-scrollback-strip-flavors-and-wheeltouch-forwarding](docs/architecture-invariants.md#terminal-scrollback-strip-flavors-and-wheeltouch-forwarding)
**Detached start + service install** (issue #231): `codeman web -d` relaunches the SAME entry script with `detached:true` (setsid), so there is no controlling terminal and no shell job entry. ⚠️ `nohup` is NOT what makes this work: Node re-arms SIGHUP to its default disposition even when it inherits "ignore", and `cli.ts` handles SIGHUP with a graceful shutdown, so a delivered HUP still stops the server. ⚠️ Both `-d` and `service install` must REFUSE when a server is already up on this data dir (pidfile check + `/api/status` probe): a second instance on the shared tmux socket attaches PTYs to the first one's live sessions. ⚠️ Neither may report success it has not observed — the parent polls `/api/status` until the child answers or dies, since `launchctl load` and a clean spawn are both silent about a server that starts and immediately exits. `--stop` verifies the pid still LOOKS like a Codeman server (`ps -o command=`) before signalling, because pids get recycled. Unit/label names live in `config/service-names.ts` so install.sh, `detectSupervisor()` and `service install` cannot drift into supervising two copies; they are instance-scoped, and identical to the historical names for the default instance. `service install` bakes the installing shell's PATH into the unit (launchd gives a job `/usr/bin:/bin:/usr/sbin:/sbin`, which finds neither a Homebrew/nvm `node` nor `tmux`/`claude`) and never writes `CODEMAN_PASSWORD` into it. → [architecture-invariants#detached-start-and-service-install](docs/architecture-invariants.md#detached-start-and-service-install)
**Self-update** (App Settings → System → Updates): in-app updater for git-clone installs supervised by systemd/launchd (`systemd`, `launchd`, `launchd-daemon`, `docker-compose`, else `none` → "restart manually"). The update restarts the very process running it, so the real work runs in a DETACHED `scripts/self-update.sh` that outlives the restart and writes progress to `update-status.json`, which the browser polls across the connection drop. `src/web/self-update.ts` splits pure helpers (unit-tested) from IO wrappers. npm installs report as non-updatable. ⚠️ **The Compose deployment is the one supervisor that does NOT outlive the restart**: there the restart IS the container exiting (`restart: unless-stopped` relaunches it), which kills the script too — safe only because the terminal `restarting` marker is written BEFORE the kill, so nothing may be appended after it. Two config facts make it work at all and both are load-bearing: the repo is a HOST BIND MOUNT over `/opt/codeman` (a pull into the baked image copy would land in the writable layer and be silently discarded by the next `up`), and the runtime image keeps devDependencies + a build toolchain (`npm run build` is tsc+esbuild, and node-pty has no Linux prebuild), which is why `npm prune --omit=dev` is gone and the updater passes `--include=dev` against `NODE_ENV=production`. ⚠️ An in-place container update applies CODE ONLY — a restart reuses the existing image and config — so `evaluateEnvironmentGate()` REFUSES a release that changes `server.Dockerfile`/`docker-compose.yaml` (sha256 vs the baseline `Start-Codeman.sh` writes to `docker-env-applied.json` on every start) or adds `.env.example` keys the user's `.env` lacks, and refuses when the restart policy would not bring the container back. That third check exists because **Compose resolves an unset `${VAR}` to the EMPTY STRING and starts anyway**, so a new required setting otherwise arrives as a silently blank env var. Every unknown fails OPEN (no baseline, unreadable `.env`, no socket): failing closed would permanently block containers created before the fingerprint file existed. The gate is re-evaluated on `POST /api/system/update`, so hiding the button is UX, not the control. ⚠️ The four global agent CLIs in `server.Dockerfile` are PINNED on purpose — unpinned, a user's CLI versions are a function of when their image was built rather than of any commit, which is the one environment change no diff-derived gate can see; pinning turns it into a Dockerfile change the gate already catches. `test/docker-compose-env-parity.test.ts` is the merge-side guard (every compose `${VAR}` ↔ an `.env.example` entry). → [docs/docker-self-update.md](docs/docker-self-update.md), [architecture-invariants#self-update](docs/architecture-invariants.md#self-update)
**Self-update** (App Settings → System → Updates): in-app updater for git-clone installs supervised by systemd/launchd (`systemd`, `launchd`, `launchd-daemon`, `docker-compose`, else `none` → "restart manually"). The update restarts the very process running it, so the real work runs in a DETACHED `scripts/self-update.sh` that outlives the restart and writes progress to `update-status.json`, which the browser polls across the connection drop. `src/web/self-update.ts` splits pure helpers (unit-tested) from IO wrappers. npm installs report as non-updatable. ⚠️ **The Compose deployment is the one supervisor that does NOT outlive the restart**: there the restart IS the container exiting (`restart: unless-stopped` relaunches it), which kills the script too — safe only because the terminal `restarting` marker is written BEFORE the kill, so nothing may be appended after it. Two config facts make it work at all and both are load-bearing: the repo is a HOST BIND MOUNT over `/opt/codeman` (a pull into the baked image copy would land in the writable layer and be silently discarded by the next `up`), and the runtime image keeps devDependencies + a build toolchain (`npm run build` is tsc+esbuild, and node-pty has no Linux prebuild), which is why `npm prune --omit=dev` is gone and the updater passes `--include=dev` against `NODE_ENV=production`. ⚠️ An in-place container update applies CODE ONLY — a restart reuses the existing image and config — so `evaluateEnvironmentGate()` REFUSES a release that changes `server.Dockerfile`/`docker-compose.yaml` (sha256 vs the baseline `Start-Codeman.sh` writes to `docker-env-applied.json` on every start) or adds `.env.example` keys the user's `.env` lacks, and refuses when the restart policy would not bring the container back. That third check exists because **Compose resolves an unset `${VAR}` to the EMPTY STRING and starts anyway**, so a new required setting otherwise arrives as a silently blank env var. Every unknown fails OPEN in the gate (no baseline, unreadable `.env`, no socket): failing closed would permanently block containers created before the fingerprint file existed. ⚠️ The KILL does not: the server exits only when `--restart-by-exit 1` was passed, i.e. the Compose file declared `CODEMAN_RESTART_BY_EXIT=1` (set ONLY there, since that file is what sets `restart: unless-stopped`; the image ENV deliberately does not) or the daemon reported an auto-restart policy; otherwise the build lands as `completed-needs-manual-restart`, because exiting blind takes a `docker run` container with no restart policy down with no UI left to recover it. The gate is re-evaluated on `POST /api/system/update`, so hiding the button is UX, not the control. ⚠️ The four global agent CLIs in `server.Dockerfile` are PINNED on purpose — unpinned, a user's CLI versions are a function of when their image was built rather than of any commit, which is the one environment change no diff-derived gate can see; pinning turns it into a Dockerfile change the gate already catches. `test/docker-compose-env-parity.test.ts` is the merge-side guard (every compose `${VAR}` ↔ an `.env.example` entry). → [docs/docker-self-update.md](docs/docker-self-update.md), [architecture-invariants#self-update](docs/architecture-invariants.md#self-update)
**Attachments** (live external document references; all wiring in `file-routes.ts`): a **registry** maps a stable `attachmentId` to a realpath-resolved, extension-allowlisted absolute path, so browser requests never carry arbitrary absolute paths. ⚠️ The **magic-link scanner** (`codeman://attach?...` in terminal output) is **prompt-injectable**, so its scan path is force-confined to the session workspace; a hostile prompt could otherwise exfiltrate arbitrary host files over SSE. The security gate is an extension **allowlist**, not a blocklist. `document-conversion-limiter.ts` caps converter spawns globally: without it, N large docs detected at once fork N multi-minute processes, which is a resource-exhaustion vector. → [architecture-invariants#attachments](docs/architecture-invariants.md#attachments)
+6
View File
@@ -116,6 +116,12 @@ if [[ -n "$dockerfile_sha" && -n "$compose_sha" ]]; then
printf '{\n "dockerfileSha256": "%s",\n "composeSha256": "%s"\n}\n' \
"$dockerfile_sha" "$compose_sha" >"$state_dir/docker-env-applied.json.tmp"
mv -- "$state_dir/docker-env-applied.json.tmp" "$state_dir/docker-env-applied.json"
# A root-run start (common on Unraid) would otherwise leave a root-owned
# `.codeman` on a FIRST start, before the container has created it as PUID,
# and the unprivileged server could then never write its own state there.
if [[ "$EUID" == '0' ]]; then
chown -- "$PUID:$PGID" "$state_dir" "$state_dir/docker-env-applied.json"
fi
else
printf 'Warning: no sha256 tool found; in-app updates will not detect environment changes.\n' >&2
fi
+6
View File
@@ -20,6 +20,12 @@ services:
# here. Also set in the image; repeated so a container started without the
# image default still self-identifies.
CODEMAN_IN_CONTAINER: "1"
# This file sets `restart: unless-stopped` below, so the updater may restart
# the server by EXITING. Declared here and only here, never in the image: a
# container started by plain `docker run` has no restart policy unless the
# operator gave it one, and there the updater asks the daemon instead and
# stages the update for a manual restart when it cannot get an answer.
CODEMAN_RESTART_BY_EXIT: "1"
CODEMAN_DOCKER_BRIDGE_HOOKS: ${CODEMAN_DOCKER_BRIDGE_HOOKS}
# Host-side equivalent of the runtime user's HOME. Docker case seed,
# credential and hook mounts are translated into the daemon namespace.
+13
View File
@@ -56,6 +56,7 @@ unchanged. The container path is a new `SupervisorKind`, not a new updater.
| `codeman-node-modules`, `codeman-dist` volumes | Container-owned build artefacts, layered over the bind mount. |
| `CODEMAN_IN_CONTAINER=1` | Tells `detectSupervisor()` to restart by exiting. |
| `restart: unless-stopped` | Turns that exit into a restart. Verified before every update. |
| `CODEMAN_RESTART_BY_EXIT=1` | The Compose file's declaration of that policy, so the updater may exit even with no Docker socket. |
| Toolchain + devDependencies in the image | Lets `npm install` and `npm run build` run inside the container. |
| `docker-env-applied.json` | Fingerprint baseline, written by `Start-Codeman.sh` on every start. |
@@ -118,6 +119,14 @@ Before signalling the server, the updater asks the Docker daemon for its own
container's restart policy. If it is `no`, the update is refused: applying it
would take Codeman down and leave no UI to recover from.
If the policy cannot be read at all (no Docker socket mounted) the update is
still allowed, but the final step changes: the server exits only when the
Compose file declared `CODEMAN_RESTART_BY_EXIT=1` (the shipped one does, because
it is the file that sets `restart: unless-stopped`) or the daemon confirmed an
auto-restart policy. Otherwise the build completes and the panel asks you to
restart the container by hand. A container started by plain `docker run` with no
restart policy therefore gets a staged update, never an outage.
### What the gate deliberately does not do
Every unknown fails **open**:
@@ -127,6 +136,10 @@ Every unknown fails **open**:
- An unreadable `.env`, an unreachable Docker socket, or a target tag whose files
cannot be read all yield "no blocker" rather than a refusal.
The one place an unknown does NOT fail open is the kill itself: with neither the
Compose declaration nor a daemon answer, the updater stages the build and asks
for a manual restart rather than exiting a server nothing may bring back.
The gate catches a specific, detectable class of mistake; it is not a last line of
defence. It is also re-evaluated server-side on `POST /api/system/update`, so
hiding the button in the UI is a courtesy rather than the control.
+16 -1
View File
@@ -24,7 +24,8 @@
# Args (all from the server, never user input — tag is validated server-side):
# --repo <dir> --tag <codeman@X.Y.Z> --supervisor <systemd|launchd|docker-compose|none>
# --status-file <path> --update-id <uuid> --from-version <ver> --node <path>
# --log <path> [--prev-sha <sha>] [--stash]
# --log <path> [--prev-sha <sha>] [--stash] [--server-pid <pid>]
# [--restart-by-exit 0|1] (docker-compose only: may we exit the server?)
#
set -uo pipefail
@@ -38,6 +39,7 @@ REPO=""
TAG=""
SUPERVISOR="none"
SERVER_PID=""
RESTART_BY_EXIT="0"
STATUS_FILE=""
UPDATE_ID=""
FROM_VERSION=""
@@ -58,6 +60,7 @@ while [[ $# -gt 0 ]]; do
--log) LOG="$2"; shift 2 ;;
--prev-sha) PREV_SHA="$2"; shift 2 ;;
--server-pid) SERVER_PID="$2"; shift 2 ;;
--restart-by-exit) RESTART_BY_EXIT="$2"; shift 2 ;;
--stash) DO_STASH=1; shift ;;
*) shift ;;
esac
@@ -224,6 +227,18 @@ case "$SUPERVISOR" in
# container's own Docker CLI talks to the HOST daemon, and a self-directed
# restart there races the client's own death. Exiting is the one path that
# needs no cooperation from anything outside the container.
#
# ⚠️ Only when the SERVER said the container comes back (`--restart-by-exit 1`:
# the Compose file declared it, or the daemon reported an auto-restart policy).
# An unknown policy stages the build and asks for a restart instead. Exiting
# blind would take a container the daemon does not restart down for good,
# with no UI left to recover it from.
if [[ "$RESTART_BY_EXIT" != "1" ]]; then
MANUAL_CMD="docker restart \$(hostname) # from the Docker host"
write_status "completed-needs-manual-restart" "Update built — restart the Codeman container to apply v$TO_VERSION."
echo "[self-update] docker-compose: restart-by-exit not confirmed — not exiting; manual restart required"
exit 0
fi
if [[ -n "$SERVER_PID" ]] && kill "$SERVER_PID" 2>/dev/null; then
: # container exit + restart policy take it from here
else
+28 -2
View File
@@ -271,6 +271,25 @@ export function isAutoRestartPolicy(name: string | null | undefined): boolean {
return name === 'always' || name === 'unless-stopped' || name === 'on-failure';
}
/**
* PURE: may the container updater restart the server by exiting? Yes when the
* Compose file declared it (`CODEMAN_RESTART_BY_EXIT=1`, set only there, since
* that file is what sets `restart: unless-stopped`) or when the daemon reports an
* auto-restart policy. Otherwise the answer is NO, and the updater stages the
* build and asks for a manual restart instead of exiting: an unknown policy is
* fine to fail open in the GATE (refusing would block installs with no socket),
* but the kill itself must not fail open, or a container the daemon would not
* bring back goes down with no UI left to recover it from.
*/
export function shouldRestartByExit(declared: boolean, restartPolicy: string | null): boolean {
return declared || isAutoRestartPolicy(restartPolicy);
}
/** The Compose file's declaration that exiting relaunches this container. */
export function restartByExitDeclared(): boolean {
return process.env.CODEMAN_RESTART_BY_EXIT === '1';
}
export interface EnvironmentGateInput {
/** sha256 of `docker/server.Dockerfile` the running container was built from. */
appliedDockerfileHash: string | null;
@@ -845,11 +864,18 @@ export async function startUpdate(): Promise<StartUpdateResult> {
process.execPath,
'--log',
logFile,
// For the launchd-daemon restart path: the updater kills this PID and the
// KeepAlive daemon respawns the server on the freshly built dist/.
// For the launchd-daemon and docker-compose restart paths: the updater kills
// this PID and the supervisor (KeepAlive daemon / Docker restart policy)
// respawns the server on the freshly built dist/.
'--server-pid',
String(process.pid),
];
if (info.supervisor === 'docker-compose') {
// Decided HERE, where the Docker socket and the Compose env are reachable;
// the updater only reads the answer. Without a yes it never exits the server.
const byExit = shouldRestartByExit(restartByExitDeclared(), detectOwnRestartPolicy());
args.push('--restart-by-exit', byExit ? '1' : '0');
}
if (prevSha) args.push('--prev-sha', prevSha);
if (info.dirty) args.push('--stash');
+21
View File
@@ -14,6 +14,7 @@ import {
diffRequiredEnvKeys,
isAutoRestartPolicy,
parseEnvKeys,
shouldRestartByExit,
type EnvironmentGateInput,
} from '../src/web/self-update.js';
@@ -86,6 +87,26 @@ describe('isAutoRestartPolicy', () => {
});
});
describe('shouldRestartByExit', () => {
it('exits when the Compose file declared it, whatever the daemon says', () => {
expect(shouldRestartByExit(true, null)).toBe(true);
expect(shouldRestartByExit(true, 'unless-stopped')).toBe(true);
});
it('exits when the daemon confirms an auto-restart policy', () => {
expect(shouldRestartByExit(false, 'unless-stopped')).toBe(true);
expect(shouldRestartByExit(false, 'always')).toBe(true);
});
// ⚠️ The gate fails open on an unknown policy; the KILL must not. A container
// nothing restarts would otherwise go down with no UI left to recover it.
it('does NOT exit on an unknown or non-restarting policy without the declaration', () => {
expect(shouldRestartByExit(false, null)).toBe(false);
expect(shouldRestartByExit(false, 'no')).toBe(false);
expect(shouldRestartByExit(false, '')).toBe(false);
});
});
describe('computeEnvironmentBlockers', () => {
it('allows a code-only release', () => {
expect(computeEnvironmentBlockers(CLEAN)).toEqual([]);