From cf6034fc038ad54c8115b507245087dbe72e3f92 Mon Sep 17 00:00:00 2001 From: giswqs Date: Wed, 12 Aug 2026 16:51:49 -0400 Subject: [PATCH 1/2] fix(collab): ship the relay image's runtime dependencies The v2.5.0 collab-node image exited at startup with ERR_MODULE_NOT_FOUND for `ws`. The runtime stage copied only /app/node_modules, but that is not where npm puts this worker's `ws`: the root lockfile carries two versions (8.21.0, hoisted for geolibre-desktop's transitive deps, and ^8.21.3 for the relay), so `npm ci --workspace geolibre-collab-node` installs the relay's copy at the nested workers/collab-node/node_modules, which nothing copied. The same stage left node_modules/@geolibre/collab-core dangling, since packages/collab-core was never copied either. Assemble the runtime tree under /runtime in the build stage instead, and copy that: the nested node_modules when it exists (it disappears the moment a dependency bump lets npm hoist `ws` to the root, so the copy is guarded), packages/collab-core so the workspace symlink resolves, and a prod-only reinstall so esbuild and the rest of the dev tree stay behind. The staged bundle is then imported once at build time -- side-effect-free, since server.js only listens when it is process.argv[1] -- so a future layout regression fails the build rather than the container. publish-container.yml only ever built the web Dockerfile, which is how this shipped. Add a CI job that builds the relay image, runs it, and drives scripts/smoke.mjs against it: health, session creation, and a WebSocket join round-trip. A build alone would not have caught this, only starting the container does. Fixes #1866 --- .github/workflows/ci.yml | 34 ++++++++ workers/collab-node/Dockerfile | 32 ++++++- workers/collab-node/README.md | 14 +++ workers/collab-node/scripts/smoke.mjs | 117 ++++++++++++++++++++++++++ 4 files changed, 194 insertions(+), 3 deletions(-) create mode 100644 workers/collab-node/scripts/smoke.mjs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 47a8e9251..f6ad8ee86 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -126,6 +126,40 @@ jobs: exit 1 fi + collab-image: + name: Collab relay image + runs-on: ubuntu-22.04 + steps: + - name: Checkout repository + uses: actions/checkout@v7 + with: + persist-credentials: false + + - name: Set up Node.js + uses: actions/setup-node@v7 + with: + node-version: "22" + + # publish-container.yml only builds the web Dockerfile, so nothing in CI + # ever built the relay image and v2.5.0 shipped one that exited at startup + # on module resolution (GeoLibre#1866). A build alone would not have caught + # it either -- the missing dependency only surfaces when the container + # runs, so this job starts it and talks to it. + - name: Build the collaboration relay image + run: docker build -f workers/collab-node/Dockerfile -t geolibre-collab:ci . + + - name: Start the relay container + run: docker run -d --name collab -p 8787:8787 geolibre-collab:ci + + - name: Smoke-test the running relay + run: node workers/collab-node/scripts/smoke.mjs http://127.0.0.1:8787 + + # The container logs carry the module-resolution error that a failed smoke + # test only reports as "never answered GET /health". + - name: Show the container logs + if: ${{ always() }} + run: docker logs collab + checks: name: Build and test runs-on: ubuntu-22.04 diff --git a/workers/collab-node/Dockerfile b/workers/collab-node/Dockerfile index 8278a6338..1d66d7b1e 100644 --- a/workers/collab-node/Dockerfile +++ b/workers/collab-node/Dockerfile @@ -8,14 +8,40 @@ COPY packages/collab-core packages/collab-core COPY workers/collab-node workers/collab-node RUN npm run build -w geolibre-collab-node +# Assemble the runtime tree under /runtime so the final stage is one COPY of a +# layout that is known to resolve. esbuild bundles everything except `ws`, and +# `ws` does not always land in /app/node_modules: the root lockfile carries two +# versions of it (8.21.0 hoisted for the app's transitive deps, ^8.21.3 for this +# worker), so npm installs this workspace's copy at the nested +# workers/collab-node/node_modules. Copying only /app/node_modules dropped it and +# the container exited at startup with ERR_MODULE_NOT_FOUND (GeoLibre#1866), so +# take the nested tree too, guarded because it disappears whenever a dependency +# bump lets npm hoist `ws` to the root instead. packages/collab-core comes along +# so the workspace symlink under node_modules resolves rather than dangling. The +# reinstall drops esbuild and the other dev dependencies now that the bundle is +# built, which is also what keeps the copied tree small. +RUN npm ci --omit=dev --workspace geolibre-collab-node --workspace @geolibre/collab-core \ + && mkdir -p /runtime/workers/collab-node \ + && cp -a node_modules /runtime/node_modules \ + && cp -a packages /runtime/packages \ + && cp -a workers/collab-node/package.json workers/collab-node/dist /runtime/workers/collab-node/ \ + && if [ -d workers/collab-node/node_modules ]; then \ + cp -a workers/collab-node/node_modules /runtime/workers/collab-node/node_modules; \ + fi + +# Load the staged bundle from the path it will actually sit at, so a layout +# regression fails the build here instead of at container start. Importing it is +# side-effect-free: server.js only listens when it is process.argv[1]. +RUN node --input-type=module \ + -e "await import('/runtime/workers/collab-node/dist/server.js');" \ + && test -e /runtime/node_modules/@geolibre/collab-core + FROM node:22-bookworm-slim ENV NODE_ENV=production PORT=8787 COLLAB_DB_PATH=/data/collab.sqlite WORKDIR /app # --chown on the COPY rather than a recursive chown afterwards: chowning /app # rewrites every file, node_modules included, into a second copy in a new layer. -COPY --from=build --chown=node:node /app/node_modules ./node_modules -COPY --from=build --chown=node:node /app/workers/collab-node/package.json ./workers/collab-node/package.json -COPY --from=build --chown=node:node /app/workers/collab-node/dist ./workers/collab-node/dist +COPY --from=build --chown=node:node /runtime ./ # /data is created and owned here so a named volume mounted over it inherits the # ownership; Docker only copies image ownership onto a volume when the path # already exists, otherwise the mountpoint lands root-owned. diff --git a/workers/collab-node/README.md b/workers/collab-node/README.md index 3032576e7..7f50f1cdd 100644 --- a/workers/collab-node/README.md +++ b/workers/collab-node/README.md @@ -23,6 +23,20 @@ Endpoints are `POST /sessions`, `GET /sessions/:id/ws`, and `GET /health`. Persist the directory containing `COLLAB_DB_PATH`, and terminate TLS at the ingress so browsers can connect with `wss://`. +## Checking a deployment + +`scripts/smoke.mjs` exercises the three endpoints against a running relay — +health, session creation, and a WebSocket join round-trip — and needs nothing +beyond Node: + +```bash +node workers/collab-node/scripts/smoke.mjs http://127.0.0.1:8787 +``` + +CI runs it against the freshly built image, because the failure it guards +against (a runtime stage missing a dependency the bundle imports) is invisible +to `docker build` and only appears when the container starts. + ## Volume ownership The container runs as the unprivileged `node` user, and the image creates diff --git a/workers/collab-node/scripts/smoke.mjs b/workers/collab-node/scripts/smoke.mjs new file mode 100644 index 000000000..ce48fc6d5 --- /dev/null +++ b/workers/collab-node/scripts/smoke.mjs @@ -0,0 +1,117 @@ +#!/usr/bin/env node +// Smoke-test a *running* GeoLibre collaboration relay: health, session +// creation, and a WebSocket join round-trip. +// +// The relay's worst failure mode is an image that builds clean and then exits at +// startup because the runtime stage is missing a dependency the bundle imports +// (GeoLibre#1866). No unit test or type-check sees that, and neither does +// `docker build` -- only starting the container does. So CI builds the image, +// runs it, and points this script at it. It talks plain HTTP plus the global +// WebSocket, so it needs nothing installed beyond Node itself and can be aimed +// at any deployed relay: +// +// node workers/collab-node/scripts/smoke.mjs http://127.0.0.1:8787 + +const baseUrl = (process.argv[2] ?? "http://127.0.0.1:8787").replace(/\/+$/, ""); +// Generous: on a cold CI runner the container has to start Node and open the +// SQLite database before it listens. +const STARTUP_TIMEOUT_MS = 60_000; +const WS_TIMEOUT_MS = 15_000; + +function fail(message, detail) { + console.error(`FAIL: ${message}`); + if (detail !== undefined) console.error(detail); + process.exit(1); +} + +const sleep = (ms) => new Promise((resolve) => setTimeout(resolve, ms)); + +async function waitForHealth() { + const deadline = Date.now() + STARTUP_TIMEOUT_MS; + let lastError = "no response"; + while (Date.now() < deadline) { + try { + const response = await fetch(`${baseUrl}/health`); + const body = await response.json(); + if (response.ok && body?.ok) return body; + lastError = `HTTP ${response.status} ${JSON.stringify(body)}`; + } catch (error) { + lastError = error instanceof Error ? error.message : String(error); + } + await sleep(500); + } + fail(`the relay never answered GET /health at ${baseUrl}`, lastError); +} + +async function createSession() { + const response = await fetch(`${baseUrl}/sessions`, { + method: "POST", + headers: { "content-type": "application/json" }, + body: "{}", + }); + const body = await response.json().catch(() => null); + if (!response.ok || !body?.sessionId || !body?.hostToken) + fail( + "POST /sessions did not return a session", + `HTTP ${response.status} ${JSON.stringify(body)}`, + ); + return body; +} + +// The join round-trip is the part that actually exercises `ws`: the relay only +// reaches WebSocketServer.handleUpgrade here, so a missing or broken copy of it +// shows up as a failed upgrade rather than a passing health check. +function join(session) { + const wsUrl = `${baseUrl.replace(/^http/, "ws")}/sessions/${session.sessionId}/ws`; + return new Promise((resolve) => { + const socket = new WebSocket(wsUrl); + const timer = setTimeout(() => { + socket.close(); + fail(`no welcome frame within ${WS_TIMEOUT_MS}ms of joining ${wsUrl}`); + }, WS_TIMEOUT_MS); + + socket.addEventListener("open", () => { + socket.send( + JSON.stringify({ + type: "join", + clientId: "smoke-test", + displayName: "Smoke test", + color: "#2563eb", + hostToken: session.hostToken, + }), + ); + }); + socket.addEventListener("message", (event) => { + clearTimeout(timer); + let message; + try { + message = JSON.parse(String(event.data)); + } catch { + fail("the relay sent a frame that is not JSON", String(event.data).slice(0, 200)); + } + if (message.type !== "welcome") + fail( + `expected a welcome frame, got "${message.type}"`, + JSON.stringify(message).slice(0, 200), + ); + if (message.role !== "host") + fail(`the host token did not claim the host role (got "${message.role}")`); + socket.close(); + resolve(message); + }); + socket.addEventListener("error", () => { + clearTimeout(timer); + fail(`the WebSocket upgrade to ${wsUrl} failed`); + }); + }); +} + +const health = await waitForHealth(); +console.log(`ok GET /health -> ${JSON.stringify(health)}`); +const session = await createSession(); +console.log(`ok POST /sessions -> ${session.sessionId} (${session.mode})`); +const welcome = await join(session); +console.log( + `ok WebSocket join -> welcome as ${welcome.role}, ${welcome.participants.length} participant(s)`, +); +console.log(`PASS: the relay at ${baseUrl} is serving`); From 9f244d8fbf86d92cb20068f5815709dd380b7aa1 Mon Sep 17 00:00:00 2001 From: giswqs Date: Wed, 12 Aug 2026 17:06:05 -0400 Subject: [PATCH 2/2] Address review feedback - Correct the Dockerfile's framing of packages/collab-core: esbuild inlines @geolibre/collab-core, so the running server never imports it and it was not what broke the image. It is copied only so the symlink npm leaves in node_modules points at something, and the build-time `test` on it asserts the tree was copied whole rather than guarding startup. - Fix the README's stale claim that the failure is invisible to `docker build`: the Dockerfile now imports the staged bundle there, so the smoke test covers what only a running container shows (the process stays up and serves HTTP and a WebSocket upgrade). - Bound the smoke test's health probes with AbortSignal.timeout, capped by the remaining startup budget. A container that accepts the connection and never answers otherwise falls back on undici's minutes-long timeouts and stalls the job past the budget instead of failing at it. Verified against a black-hole listener: fails at the budget rather than hanging. POST /sessions is bounded the same way. --- workers/collab-node/Dockerfile | 11 ++++++++--- workers/collab-node/README.md | 8 +++++--- workers/collab-node/scripts/smoke.mjs | 14 +++++++++++++- 3 files changed, 26 insertions(+), 7 deletions(-) diff --git a/workers/collab-node/Dockerfile b/workers/collab-node/Dockerfile index 1d66d7b1e..2ef835cf3 100644 --- a/workers/collab-node/Dockerfile +++ b/workers/collab-node/Dockerfile @@ -17,9 +17,12 @@ RUN npm run build -w geolibre-collab-node # the container exited at startup with ERR_MODULE_NOT_FOUND (GeoLibre#1866), so # take the nested tree too, guarded because it disappears whenever a dependency # bump lets npm hoist `ws` to the root instead. packages/collab-core comes along -# so the workspace symlink under node_modules resolves rather than dangling. The +# for a weaker reason: esbuild inlines it, so the running server never imports it +# and it is not what broke the image. It is copied so that the symlink npm leaves +# at node_modules/@geolibre/collab-core points at something, since a dangling +# entry in node_modules trips anything that walks the tree in the container. The # reinstall drops esbuild and the other dev dependencies now that the bundle is -# built, which is also what keeps the copied tree small. +# built, which is what keeps the copied tree small. RUN npm ci --omit=dev --workspace geolibre-collab-node --workspace @geolibre/collab-core \ && mkdir -p /runtime/workers/collab-node \ && cp -a node_modules /runtime/node_modules \ @@ -31,7 +34,9 @@ RUN npm ci --omit=dev --workspace geolibre-collab-node --workspace @geolibre/col # Load the staged bundle from the path it will actually sit at, so a layout # regression fails the build here instead of at container start. Importing it is -# side-effect-free: server.js only listens when it is process.argv[1]. +# side-effect-free: server.js only listens when it is process.argv[1]. The +# `test` is the weaker half -- nothing at runtime resolves that symlink, it just +# asserts the tree was copied whole. RUN node --input-type=module \ -e "await import('/runtime/workers/collab-node/dist/server.js');" \ && test -e /runtime/node_modules/@geolibre/collab-core diff --git a/workers/collab-node/README.md b/workers/collab-node/README.md index 7f50f1cdd..54d5d5989 100644 --- a/workers/collab-node/README.md +++ b/workers/collab-node/README.md @@ -33,9 +33,11 @@ beyond Node: node workers/collab-node/scripts/smoke.mjs http://127.0.0.1:8787 ``` -CI runs it against the freshly built image, because the failure it guards -against (a runtime stage missing a dependency the bundle imports) is invisible -to `docker build` and only appears when the container starts. +CI runs it against the freshly built image. The Dockerfile already imports the +staged bundle during `docker build`, so a missing `ws` or a broken +`@geolibre/collab-core` target fails there; this script covers the rest, which +only a running container shows: that the process starts and stays up, and that +it actually serves HTTP and completes a WebSocket upgrade. ## Volume ownership diff --git a/workers/collab-node/scripts/smoke.mjs b/workers/collab-node/scripts/smoke.mjs index ce48fc6d5..551e8e8b3 100644 --- a/workers/collab-node/scripts/smoke.mjs +++ b/workers/collab-node/scripts/smoke.mjs @@ -17,6 +17,7 @@ const baseUrl = (process.argv[2] ?? "http://127.0.0.1:8787").replace(/\/+$/, "") // SQLite database before it listens. const STARTUP_TIMEOUT_MS = 60_000; const WS_TIMEOUT_MS = 15_000; +const PROBE_TIMEOUT_MS = 5_000; function fail(message, detail) { console.error(`FAIL: ${message}`); @@ -31,7 +32,15 @@ async function waitForHealth() { let lastError = "no response"; while (Date.now() < deadline) { try { - const response = await fetch(`${baseUrl}/health`); + // Bounded, because a container that accepts the connection and then never + // answers falls back on undici's own header/body timeouts, which are + // minutes long: the job would stall well past the startup budget instead + // of failing at it. Capped at whatever is left of that budget so a probe + // started near the deadline cannot overrun it either. + const signal = AbortSignal.timeout(Math.min(PROBE_TIMEOUT_MS, deadline - Date.now())); + const response = await fetch(`${baseUrl}/health`, { signal }); + // The same signal still covers this: aborting after the headers arrive + // errors the body stream rather than leaving the read hanging. const body = await response.json(); if (response.ok && body?.ok) return body; lastError = `HTTP ${response.status} ${JSON.stringify(body)}`; @@ -48,6 +57,9 @@ async function createSession() { method: "POST", headers: { "content-type": "application/json" }, body: "{}", + // Bounded for the same reason as the health probe: the relay has answered + // by now, so anything slower than this is a hang, not a slow start. + signal: AbortSignal.timeout(PROBE_TIMEOUT_MS), }); const body = await response.json().catch(() => null); if (!response.ok || !body?.sessionId || !body?.hostToken)