From 9e902272cf3f0cfd9072cfd49fed97157acea2c1 Mon Sep 17 00:00:00 2001 From: Chad Miller Date: Tue, 18 Aug 2026 22:17:46 -0700 Subject: [PATCH] fix(git-ci): survive a state file the daemon cannot write A bind mount arrives owned by root however the image left its mount point, so the daemon could not write /data/state.json. The write threw out of the firehose handler, which dropped the event and left the cursor where it was, so every reconnect resumed from the live head and lost whatever landed while the socket was down. A durability problem became a correctness one. The store now warns once and carries on in memory, and the image takes the mount as root before dropping to the unprivileged user, so the mounted case persists again. Co-Authored-By: Claude Opus 5 (1M context) --- packages/git-ci/Dockerfile | 9 ++++++--- packages/git-ci/docker-entrypoint.sh | 11 +++++++++++ packages/git-ci/src/cli.js | 2 +- packages/git-ci/src/state.js | 22 +++++++++++++++++++--- packages/git-ci/test/daemon.test.js | 24 ++++++++++++++++++++++++ 5 files changed, 61 insertions(+), 7 deletions(-) create mode 100755 packages/git-ci/docker-entrypoint.sh diff --git a/packages/git-ci/Dockerfile b/packages/git-ci/Dockerfile index 48db84b..889ad36 100644 --- a/packages/git-ci/Dockerfile +++ b/packages/git-ci/Dockerfile @@ -30,22 +30,25 @@ WORKDIR /app # git clones the repository under test from the PDS over https. RUN apt-get update \ - && apt-get install -y --no-install-recommends git ca-certificates \ + && apt-get install -y --no-install-recommends git ca-certificates gosu \ && rm -rf /var/lib/apt/lists/* # Runs unprivileged. node:22-slim ships a "node" user for exactly this. +# The entrypoint starts as root only to take the state mount, then drops. RUN mkdir -p /data && chown node:node /data -USER node COPY --from=builder --chown=node:node /app /app +COPY --chmod=755 packages/git-ci/docker-entrypoint.sh /usr/local/bin/ ENV NODE_ENV=production # The cursor and the ref map, which decide what replays after a restart. # Mount a volume at /data to keep them. There is no VOLUME instruction: some # hosts reject one, and `docker run -v` needs no declaration here. Without a -# mount the daemon still runs, and a restart replays from the live head. +# mount, or with one the daemon cannot write, it still runs; a restart then +# replays from the live head. ENV PDSJS_CI_STATE=/data/state.json # PDSJS_CI_IDENTIFIER, PDSJS_CI_PASSWORD and PDSJS_CI_REPOS are deliberately # unset. The CLI prints its usage and exits rather than inventing them. +ENTRYPOINT ["docker-entrypoint.sh"] CMD ["node", "packages/git-ci/src/cli.js", "watch"] diff --git a/packages/git-ci/docker-entrypoint.sh b/packages/git-ci/docker-entrypoint.sh new file mode 100755 index 0000000..f0f29bb --- /dev/null +++ b/packages/git-ci/docker-entrypoint.sh @@ -0,0 +1,11 @@ +#!/bin/sh +# The state directory is a mount point on most hosts, and a bind mount arrives +# owned by root however the image left it. Take ownership as root, then drop to +# the unprivileged user for the daemon itself. +set -e + +state_dir=$(dirname "${PDSJS_CI_STATE:-/data/state.json}") +mkdir -p "$state_dir" +chown -R node:node "$state_dir" || true + +exec gosu node "$@" diff --git a/packages/git-ci/src/cli.js b/packages/git-ci/src/cli.js index 04a21d3..c2c4cf9 100644 --- a/packages/git-ci/src/cli.js +++ b/packages/git-ci/src/cli.js @@ -82,7 +82,7 @@ async function main() { await watch({ repos, runCheck, - state: createStateStore(statePath(env)), + state: createStateStore(statePath(env), onNotice), signal: controller.signal, onNotice, }); diff --git a/packages/git-ci/src/state.js b/packages/git-ci/src/state.js index 0602065..33da16f 100644 --- a/packages/git-ci/src/state.js +++ b/packages/git-ci/src/state.js @@ -47,13 +47,29 @@ function readState(path) { /** * @param {string} path + * @param {(message: string) => void} [onWarn] */ -export function createStateStore(path) { +export function createStateStore(path, onWarn = () => {}) { const state = readState(path); + let unwritable = false; + // A state file that cannot be written costs durability, not the run. The + // daemon keeps the cursor and the ref map in memory and carries on, so an + // unwritable path degrades to "replays from the live head after a restart" + // rather than stopping the events that reach the runner. function flush() { - mkdirSync(dirname(path), { recursive: true }); - writeFileSync(path, `${JSON.stringify(state, null, 2)}\n`, { mode: 0o600 }); + if (unwritable) return; + try { + mkdirSync(dirname(path), { recursive: true }); + writeFileSync(path, `${JSON.stringify(state, null, 2)}\n`, { + mode: 0o600, + }); + } catch (err) { + unwritable = true; + onWarn( + `state file ${path} is not writable (${err instanceof Error ? err.message : err}); continuing without saving progress`, + ); + } } return { diff --git a/packages/git-ci/test/daemon.test.js b/packages/git-ci/test/daemon.test.js index 167ef3d..9e8c80b 100644 --- a/packages/git-ci/test/daemon.test.js +++ b/packages/git-ci/test/daemon.test.js @@ -1,3 +1,4 @@ +import { chmodSync, mkdirSync } from 'node:fs'; import { mkdtemp, rm } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; @@ -137,6 +138,29 @@ describe('createStateStore', () => { }); }); + it('keeps working in memory when the file cannot be written', () => { + // A bind mount the daemon does not own reads this way, and losing + // durability must not stop the events reaching the runner. + const readOnly = join(dir, 'locked'); + mkdirSync(readOnly); + chmodSync(readOnly, 0o500); + /** @type {string[]} */ + const warnings = []; + const store = createStateStore(join(readOnly, 'state.json'), (message) => + warnings.push(message), + ); + + expect(() => store.setCursor(SERVICE, 7)).not.toThrow(); + expect(() => store.setRefs('did:plc:abc/proj', { a: 'b' })).not.toThrow(); + expect(store.cursor(SERVICE)).toBe(7); + expect(store.refs('did:plc:abc/proj')).toEqual({ a: 'b' }); + // Warned once, not once per write. + expect(warnings).toHaveLength(1); + expect(warnings[0]).toMatch(/not writable/); + + chmodSync(readOnly, 0o700); + }); + it('starts empty when the file is absent or unreadable', () => { const store = createStateStore(join(dir, 'missing', 'state.json')); expect(store.cursor(SERVICE)).toBeUndefined(); -- 2.51.2