diff --git a/CLAUDE.md b/CLAUDE.md index bf589e17..73f14534 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -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) diff --git a/docker/Start-Codeman.sh b/docker/Start-Codeman.sh index 3055ecfc..c0294fe4 100644 --- a/docker/Start-Codeman.sh +++ b/docker/Start-Codeman.sh @@ -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 diff --git a/docker/docker-compose.yaml b/docker/docker-compose.yaml index 9103ad46..ca61796a 100644 --- a/docker/docker-compose.yaml +++ b/docker/docker-compose.yaml @@ -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. diff --git a/docs/docker-self-update.md b/docs/docker-self-update.md index b8a4af1c..6521abd0 100644 --- a/docs/docker-self-update.md +++ b/docs/docker-self-update.md @@ -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. diff --git a/scripts/self-update.sh b/scripts/self-update.sh index 9ad786b8..6ac3f80d 100755 --- a/scripts/self-update.sh +++ b/scripts/self-update.sh @@ -24,7 +24,8 @@ # Args (all from the server, never user input — tag is validated server-side): # --repo --tag --supervisor # --status-file --update-id --from-version --node -# --log [--prev-sha ] [--stash] +# --log [--prev-sha ] [--stash] [--server-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 diff --git a/src/web/self-update.ts b/src/web/self-update.ts index 2e821d29..d25ec186 100644 --- a/src/web/self-update.ts +++ b/src/web/self-update.ts @@ -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 { 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'); diff --git a/test/docker-self-update.test.ts b/test/docker-self-update.test.ts index 86b35b71..1cd429a7 100644 --- a/test/docker-self-update.test.ts +++ b/test/docker-self-update.test.ts @@ -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([]);