From 501a51f80e05ff5eaa9e67a6b5e4d7bf0a2d4f8c Mon Sep 17 00:00:00 2001 From: Developer Date: Sun, 2 Aug 2026 23:01:15 -0500 Subject: [PATCH 01/16] fix(update): the updater failed silently and waited forever MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reported symptom: a Docker instance running for some time sits on WAITING FOR SERVER and never updates. Every step of the update discarded its output. The route answered "Update started" before checking anything could work, and the pull handler was `if (code !== 0) return` — so a failed pull meant the helper never ran, nothing changed, and nothing was written down. The client polled every three seconds forever with no deadline, so a stack that could not update looked exactly like one still working. The likeliest cause for a long-running container is the compose file's self-mount at /tmp/docker-compose.yml, which the update reads. Containers started before that line existed do not have it, so the pull fails, and everything above turns that into an indefinite wait. Which is to say the instances least able to update in place are precisely the ones that have been running longest. POST /api/update now preflights the mount, the docker socket and the compose project labels, and returns 409 naming what is missing and what to do. Both steps append to backend/data/update.log on the data volume, so it survives the container being replaced. GET /api/update/status reports phase and error. The modal renders those and gives up after six minutes. Two further faults found while in there. The client waited for the reported version to change, but a build without APP_VERSION reports 'dev' before and after, so a successful update hung too — /api/version now carries a boot id and the client waits on the restart. And hasUpdate was `latest !== current`, so a published tag trailing the running one counted as an update, which is why a 1.8.0 instance was offered 1.7.4. The logic moved out of the route into backend/updater.js; none of it was reachable from a test where it was, which is much of why three separate faults sat in it unnoticed. 20 backend tests and 3 modal tests. No automation added — the trigger stays a button. --- CHANGELOG.md | 18 ++ README.md | 3 +- backend/__tests__/updater.test.js | 208 ++++++++++++++++ backend/routes/admin.js | 79 ++---- backend/updater.js | 230 ++++++++++++++++++ frontend/package.json | 2 +- frontend/src/components/UpdateModal.tsx | 95 ++++++-- .../components/__tests__/UpdateModal.test.tsx | 38 ++- package.json | 2 +- 9 files changed, 598 insertions(+), 77 deletions(-) create mode 100644 backend/__tests__/updater.test.js create mode 100644 backend/updater.js diff --git a/CHANGELOG.md b/CHANGELOG.md index 69aa43c..b64657f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,24 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). --- +## [1.8.1] - 2026-08-02 + +### Fixed + +- **The in-app updater could sit on `WAITING FOR SERVER` indefinitely.** Every step failed silently — both child processes discarded their output, the route answered "Update started" before checking anything could work, and a non-zero exit from `docker compose pull` simply returned — so a stack that *could not* update was indistinguishable from one still working, and the client polled every three seconds forever with no deadline. + + The likeliest cause on a long-running instance is the compose file's own self-mount at `/tmp/docker-compose.yml`, which the update reads. A container started before that line existed does not have it, the pull fails, and everything above turns that into an endless wait — so the instances least able to update in place are exactly the ones that have been running longest. + + `POST /api/update` now checks the mount, the Docker socket and the compose project labels *before* answering, and returns `409` naming what is missing and what to do about it. Both steps append to `backend/data/update.log`, which lives on the data volume and so survives the container being replaced. `GET /api/update/status` reports phase and error, and the modal shows them and gives up after six minutes. +- **A successful update could hang too.** The client waited for the reported version to change, but a build without `APP_VERSION` reports `dev` before and after. `/api/version` now carries a boot id and the client waits for the restart itself. +- **The update check offered downgrades.** `hasUpdate` was `latest !== current`, so a published tag trailing the running one counted as an update — a 1.8.0 instance was offered 1.7.4. It is a numeric version comparison now, and anything unparseable (`dev`, `latest`) is never offered. + +### Technical + +- The update logic moved out of the admin route into `backend/updater.js`. None of it was reachable from a test where it was, which is a large part of why three separate faults sat in it unnoticed. + +--- + ## [1.8.0] - 2026-08-01 ### Added diff --git a/README.md b/README.md index cad37ea..73633ed 100644 --- a/README.md +++ b/README.md @@ -323,10 +323,11 @@ CITY_NET/ ├── backend/ │ ├── server.js # Express entrypoint — mounts routes, starts Socket.IO │ ├── db.js # SQLite schema and migrations +│ ├── updater.js # In-app self-update — preflight (compose file mounted, docker socket, compose project labels) so a stack that cannot update says why instead of hanging; upgrade-only semver check; update log on the data volume; boot id so a restart is detectable without a version change │ ├── middleware/ │ │ └── auth.js # JWT verify middleware (admin + elevated users) │ ├── routes/ -│ │ ├── admin.js # Admin-only REST endpoints; undo covers locations, roads, signs; POST /water marks generated water so a regenerate can clear its own river without touching a lake the GM drew +│ │ ├── admin.js # Admin-only REST endpoints; undo covers locations, roads, signs; POST /update preflights and returns 409 naming what is missing, GET /update/status reports phase, error and log tail; POST /water marks generated water so a regenerate can clear its own river without touching a lake the GM drew │ │ ├── locations.js # Location CRUD; JOIN→CUSTOM classification upserts roots + child parts to custom_structure_library; serves GET /custom-library (CUSTOM-only); GET / includes sheet_data for NPC initiative rolls; POST /purge-region clears one region's generated content in a single transaction, keeping GM-named structures, tokens, battle-map content and hand-drawn water │ │ ├── battle_maps.js # Battle map image upload/management │ │ ├── maps.js # Saved map snapshots (locations, districts, roads, overpasses, water bodies); preserves only rhombus tokens on load/clear; records active_map_name in global_settings so exports can name their files diff --git a/backend/__tests__/updater.test.js b/backend/__tests__/updater.test.js new file mode 100644 index 0000000..381ed37 --- /dev/null +++ b/backend/__tests__/updater.test.js @@ -0,0 +1,208 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { createRequire } from 'module'; + +const require = createRequire(import.meta.url); +const updater = require('../updater.js'); + +/** + * In-app self-update. + * + * The failure these guard is not "the update broke" but "the update broke silently". + * Every step used to discard its output and the route reported success before checking + * anything, so a stack that could not update was indistinguishable from a slow one and + * the client waited for a restart that was never coming. + */ + +describe('isNewerVersion', () => { + it('accepts a genuinely newer version', () => { + expect(updater.isNewerVersion('1.8.1', '1.8.0')).toBe(true); + expect(updater.isNewerVersion('1.9.0', '1.8.9')).toBe(true); + expect(updater.isNewerVersion('2.0.0', '1.99.99')).toBe(true); + }); + + it('refuses a downgrade', () => { + // The check this replaces was `latest !== current`, which called any difference an + // update — so a published tag trailing the running one offered 1.8.0 → 1.7.4. + expect(updater.isNewerVersion('1.7.4', '1.8.0')).toBe(false); + expect(updater.isNewerVersion('1.8.0', '1.8.1')).toBe(false); + }); + + it('refuses an identical version', () => { + expect(updater.isNewerVersion('1.8.0', '1.8.0')).toBe(false); + }); + + it('compares numerically, not as text', () => { + // '1.10.0' sorts before '1.9.0' as a string and after it as a version. + expect(updater.isNewerVersion('1.10.0', '1.9.0')).toBe(true); + expect(updater.isNewerVersion('1.9.0', '1.10.0')).toBe(false); + }); + + it('treats a missing segment as zero', () => { + expect(updater.isNewerVersion('1.9', '1.8.7')).toBe(true); + expect(updater.isNewerVersion('1.8', '1.8.0')).toBe(false); + }); + + it('refuses anything it cannot parse rather than guessing', () => { + // 'dev' is what a build without APP_VERSION reports; offering it an update to + // whatever the registry has would be acting on nothing. + expect(updater.isNewerVersion('latest', '1.8.0')).toBe(false); + expect(updater.isNewerVersion('1.8.1', 'dev')).toBe(false); + expect(updater.isNewerVersion('', '1.0.0')).toBe(false); + }); +}); + +describe('preflight', () => { + const labels = { projectName: 'citynet', configFile: '/srv/citynet/docker-compose.yml', workingDir: '/srv/citynet' }; + const allPresent = () => true; + + it('passes when the mount, the socket and the labels are all there', () => { + const res = updater.preflight({ existsSync: allPresent, labels }); + expect(res.ok).toBe(true); + expect(res.labels).toBe(labels); + }); + + it('refuses when the compose file is not mounted, and says how to fix it', () => { + // The commonest case by far: a container started before the compose file mounted + // itself, which is to say a long-running one — exactly those most needing an update. + const res = updater.preflight({ + existsSync: (p) => p !== updater.COMPOSE_FILE, + labels, + }); + expect(res.ok).toBe(false); + expect(res.error).toContain(updater.COMPOSE_FILE); + expect(res.error).toContain('docker compose up -d'); + }); + + it('refuses when the docker socket is missing', () => { + const res = updater.preflight({ + existsSync: (p) => p !== '/var/run/docker.sock', + labels, + }); + expect(res.ok).toBe(false); + expect(res.error).toMatch(/socket/i); + }); + + it('refuses when the compose project labels are missing', () => { + // Without these the helper would be handed undefined as a mount source, fail + // instantly, and report nothing. + const res = updater.preflight({ + existsSync: allPresent, + labels: { projectName: null, configFile: null, workingDir: null }, + }); + expect(res.ok).toBe(false); + expect(res.error).toMatch(/docker run/); + }); + + it('refuses when only the working directory is missing', () => { + const res = updater.preflight({ + existsSync: allPresent, + labels: { ...labels, workingDir: null }, + }); + expect(res.ok).toBe(false); + }); +}); + +describe('buildUpdateHelperArgs', () => { + it('mounts the host project directory at its own absolute path', () => { + // Mounting it at an alias made the daemon look for a path that does not exist on the + // host, silently create an empty directory, and wipe the bind-mounted data. + const args = updater.buildUpdateHelperArgs('/srv/citynet', '/srv/citynet/docker-compose.yml', ['-p', 'citynet']); + expect(args).toContain('/srv/citynet:/srv/citynet'); + }); + + it('runs compose against the mounted config with the project name', () => { + const args = updater.buildUpdateHelperArgs('/srv/citynet', '/srv/citynet/docker-compose.yml', ['-p', 'citynet']); + const cmd = args[args.length - 1]; + expect(cmd).toContain('--project-directory "/srv/citynet"'); + expect(cmd).toContain('-p citynet'); + expect(cmd).toContain('up -d'); + }); +}); + +describe('readComposeLabels', () => { + it('returns nulls rather than throwing when docker is unavailable', () => { + const labels = updater.readComposeLabels({ + readFileSync: () => 'abc123', + execSync: () => { throw new Error('docker: not found'); }, + }); + expect(labels).toEqual({ projectName: null, configFile: null, workingDir: null }); + }); + + it('reads the compose labels off the running container', () => { + const labels = updater.readComposeLabels({ + readFileSync: () => 'abc123\\n', + execSync: () => JSON.stringify({ + 'com.docker.compose.project': 'citynet', + 'com.docker.compose.project.config_files': '/srv/citynet/docker-compose.yml', + 'com.docker.compose.project.working_dir': '/srv/citynet', + }), + }); + expect(labels.projectName).toBe('citynet'); + expect(labels.workingDir).toBe('/srv/citynet'); + }); +}); + +describe('runUpdate', () => { + const labels = { projectName: 'citynet', configFile: '/srv/citynet/docker-compose.yml', workingDir: '/srv/citynet' }; + + /** A fake child process whose lifecycle the test drives. */ + const fakeChild = () => { + const handlers = {}; + return { + unref: vi.fn(), + on: (evt, fn) => { handlers[evt] = fn; }, + emit: (evt, arg) => handlers[evt]?.(arg), + }; + }; + + beforeEach(() => updater.resetState()); + + it('reports the phase while it works instead of leaving the client to guess', () => { + const pull = fakeChild(); + updater.runUpdate(labels, { spawn: () => pull, openSync: () => { throw new Error('no log'); } }); + expect(updater.getState().phase).toBe('pulling'); + + pull.emit('close', 0); + expect(updater.getState().phase).toBe('restarting'); + }); + + it('records a failed pull rather than returning silently', () => { + // The old code did `if (code !== 0) return`, so the helper never ran, nothing + // changed, and the client polled for a restart forever. + const pull = fakeChild(); + updater.runUpdate(labels, { spawn: () => pull, openSync: () => { throw new Error('no log'); } }); + pull.emit('close', 1); + + const state = updater.getState(); + expect(state.phase).toBe('failed'); + expect(state.error).toContain('exited with code 1'); + }); + + it('records docker being missing entirely', () => { + const pull = fakeChild(); + updater.runUpdate(labels, { spawn: () => pull, openSync: () => { throw new Error('no log'); } }); + pull.emit('error', new Error('spawn docker ENOENT')); + + expect(updater.getState().phase).toBe('failed'); + expect(updater.getState().error).toContain('ENOENT'); + }); + + it('spawns the helper only after a successful pull', () => { + const pull = fakeChild(); + const helper = fakeChild(); + const spawn = vi.fn().mockReturnValueOnce(pull).mockReturnValueOnce(helper); + updater.runUpdate(labels, { spawn, openSync: () => { throw new Error('no log'); } }); + + expect(spawn).toHaveBeenCalledTimes(1); + pull.emit('close', 0); + expect(spawn).toHaveBeenCalledTimes(2); + expect(spawn.mock.calls[1][1]).toContain('run'); + }); + + it('exposes a boot id so a restart can be detected without a version change', () => { + // A build without APP_VERSION reports 'dev' before and after, so waiting on the + // version alone hangs even when the update succeeded. + expect(updater.getState().bootId).toBe(updater.BOOT_ID); + expect(updater.BOOT_ID).toBeTruthy(); + }); +}); diff --git a/backend/routes/admin.js b/backend/routes/admin.js index 8f04603..e9789b0 100644 --- a/backend/routes/admin.js +++ b/backend/routes/admin.js @@ -6,27 +6,7 @@ const { authenticate } = require('../middleware/auth'); const SECRET = process.env.JWT_SECRET; let currentController = 'GM'; -/** - * Build the docker-run argument list for the self-update helper container. - * - * The host project directory MUST be mounted at its own absolute path, not at - * an alias like /project. Compose passes volume host-paths straight to the - * Docker daemon; if those paths don't exist on the host the daemon silently - * creates a new empty directory, wiping existing bind-mount data (issue that - * caused data loss on in-app updates prior to 1.6.3). - */ -function buildUpdateHelperArgs(hostWorkingDir, hostConfigFile, projectArgs) { - const projectArgsStr = projectArgs.join(' '); - return [ - 'run', '--rm', - '-v', '/var/run/docker.sock:/var/run/docker.sock', - '-v', `${hostWorkingDir}:${hostWorkingDir}`, - '-v', `${hostConfigFile}:/tmp/docker-compose.yml:ro`, - 'over2take/citynet-backend:latest', - 'sh', '-c', - `docker compose --project-directory "${hostWorkingDir}" -f /tmp/docker-compose.yml ${projectArgsStr} up -d`, - ]; -} +const updater = require('../updater'); module.exports = (db, io, { emitUpdate, recordAction }) => { const router = express.Router(); @@ -232,7 +212,10 @@ module.exports = (db, io, { emitUpdate, recordAction }) => { const { execSync } = require('child_process'); let isDocker = false; try { execSync('docker info', { stdio: 'ignore' }); isDocker = true; } catch {} - res.json({ version: process.env.APP_VERSION || 'dev', isDocker }); + // bootId changes on every process start. The client waits for *that*, not for the + // version to differ: a build without APP_VERSION reports 'dev' before and after, so + // waiting on the version hangs even when the update worked. + res.json({ version: process.env.APP_VERSION || 'dev', isDocker, bootId: updater.BOOT_ID }); }); // --- Version Check (Docker Hub) --- @@ -267,7 +250,9 @@ module.exports = (db, io, { emitUpdate, recordAction }) => { return 0; }) || []; const latestTag = versionTags[0] || 'unknown'; - const hasUpdate = latestTag !== 'unknown' && latestTag !== currentVersion; + // Strictly newer, not merely different. Comparing with !== offers a + // downgrade whenever the published tag trails the running one. + const hasUpdate = latestTag !== 'unknown' && updater.isNewerVersion(latestTag, currentVersion); res.json({ current: currentVersion, @@ -294,44 +279,20 @@ module.exports = (db, io, { emitUpdate, recordAction }) => { router.post('/update', authenticate, (req, res) => { if (req.user.isTemporary) return res.status(403).json({ error: 'Primary admin only' }); - const { spawn, execSync } = require('child_process'); - const fs = require('fs'); + // Checked before answering, so a stack that cannot update says why instead of + // reporting success and leaving the client polling for a restart that never comes. + const check = updater.preflight(); + if (!check.ok) return res.status(409).json({ error: check.error }); + updater.runUpdate(check.labels); res.json({ message: 'Update started' }); + }); - // Read compose project name and host paths from this container's own Docker labels - let projectArgs = []; - let hostConfigFile = null; - let hostWorkingDir = null; - try { - const containerId = fs.readFileSync('/etc/hostname', 'utf8').trim(); - const labels = JSON.parse(execSync( - `docker inspect ${containerId} --format '{{json .Config.Labels}}'`, - { encoding: 'utf8' } - ).trim()); - const projectName = labels['com.docker.compose.project']; - if (projectName) projectArgs = ['-p', projectName]; - hostConfigFile = labels['com.docker.compose.project.config_files']; - hostWorkingDir = labels['com.docker.compose.project.working_dir']; - } catch (_) {} - - const composeArgs = ['compose', '-f', '/tmp/docker-compose.yml', ...projectArgs]; - - const pull = spawn('docker', [...composeArgs, 'pull'], { detached: true, stdio: 'ignore' }); - pull.unref(); - pull.on('close', (code) => { - if (code !== 0) return; - // Spawn a temporary helper container to run up -d so it survives - // the backend container being replaced mid-execution. - // Mount the host project dir at its *own host path* (not /project) so - // that when compose resolves relative paths like ./backend/data, the - // resulting absolute path matches what the host Docker daemon sees. - // Using a different mountpoint (e.g. /project) caused the daemon to - // look for /project/backend/data on the host, which doesn't exist, - // creating a new empty bind mount and wiping data on every update. - const helper = spawn('docker', buildUpdateHelperArgs(hostWorkingDir, hostConfigFile, projectArgs), { detached: true, stdio: 'ignore' }); - helper.unref(); - }); + // --- Update progress --- + // Unauthenticated on purpose: it carries no secrets, and the client needs to keep + // reading it across the restart that drops its session. + router.get('/update/status', (req, res) => { + res.json(updater.getState()); }); // --- Chat --- @@ -350,4 +311,4 @@ module.exports = (db, io, { emitUpdate, recordAction }) => { return router; }; -module.exports.buildUpdateHelperArgs = buildUpdateHelperArgs; +module.exports.buildUpdateHelperArgs = updater.buildUpdateHelperArgs; diff --git a/backend/updater.js b/backend/updater.js new file mode 100644 index 0000000..0d3f957 --- /dev/null +++ b/backend/updater.js @@ -0,0 +1,230 @@ +const fs = require('fs'); +const path = require('path'); +const { spawn, execSync } = require('child_process'); + +/** + * In-app self-update. + * + * The pieces here were previously inline in the admin route, where every failure was + * silent: both child processes ran with `stdio: 'ignore'`, the route answered "Update + * started" before knowing whether anything would work, and a non-zero exit from the + * pull simply returned. The client polled for a version change forever, so a stack that + * could not update looked exactly like one that was taking a while — which is how an + * instance sits on WAITING FOR SERVER indefinitely. + * + * So: check first and say why not, record what happened, and let the client ask. + */ + +/** Where the compose file is mounted inside the backend container. */ +const COMPOSE_FILE = '/tmp/docker-compose.yml'; + +/** Identifies this process. A restart is what the client is really waiting for. */ +const BOOT_ID = `${Date.now()}-${Math.random().toString(36).slice(2, 10)}`; + +/** Update log, on the data volume so it survives the container being replaced. */ +function logPath() { + const dbPath = process.env.DB_PATH || '/app/data/city.db'; + return path.join(path.dirname(dbPath), 'update.log'); +} + +/** + * Compare two dotted versions. True when `candidate` is strictly newer than `current`. + * + * The check this replaces was `latest !== current`, which treats any difference as an + * update — so a host publishing an older tag than the one running offers a downgrade, + * and the modal duly reported "1.8.0 → 1.7.4". Harmless to ignore by hand, not harmless + * to act on. + */ +function isNewerVersion(candidate, current) { + const parse = (v) => String(v).split('.').map((n) => parseInt(n, 10)); + const a = parse(candidate); + const b = parse(current); + if (a.some(Number.isNaN) || b.some(Number.isNaN)) return false; + for (let i = 0; i < Math.max(a.length, b.length); i++) { + const x = a[i] ?? 0; + const y = b[i] ?? 0; + if (x !== y) return x > y; + } + return false; +} + +/** + * Build the docker-run argument list for the self-update helper container. + * + * The host project directory MUST be mounted at its own absolute path, not at + * an alias like /project. Compose passes volume host-paths straight to the + * Docker daemon; if those paths don't exist on the host the daemon silently + * creates a new empty directory, wiping existing bind-mount data (issue that + * caused data loss on in-app updates prior to 1.6.3). + */ +function buildUpdateHelperArgs(hostWorkingDir, hostConfigFile, projectArgs) { + const projectArgsStr = projectArgs.join(' '); + return [ + 'run', '--rm', + '-v', '/var/run/docker.sock:/var/run/docker.sock', + '-v', `${hostWorkingDir}:${hostWorkingDir}`, + '-v', `${hostConfigFile}:${COMPOSE_FILE}:ro`, + 'over2take/citynet-backend:latest', + 'sh', '-c', + `docker compose --project-directory "${hostWorkingDir}" -f ${COMPOSE_FILE} ${projectArgsStr} up -d`, + ]; +} + +/** + * Read the compose project name and host paths from this container's own labels. + * + * A stack started with plain `docker run`, or by a compose old enough not to set these, + * has none of them — and the helper would then be handed `undefined` as a mount source, + * fail instantly, and report nothing. + */ +function readComposeLabels(deps = {}) { + const read = deps.readFileSync || fs.readFileSync; + const exec = deps.execSync || execSync; + try { + const containerId = read('/etc/hostname', 'utf8').trim(); + const labels = JSON.parse(exec( + `docker inspect ${containerId} --format '{{json .Config.Labels}}'`, + { encoding: 'utf8' } + ).trim()); + return { + projectName: labels['com.docker.compose.project'] || null, + configFile: labels['com.docker.compose.project.config_files'] || null, + workingDir: labels['com.docker.compose.project.working_dir'] || null, + }; + } catch { + return { projectName: null, configFile: null, workingDir: null }; + } +} + +/** + * Everything that must be true before an update can possibly work. + * + * Each failure names the thing that is missing and what to do about it, because the + * commonest cause is a container started from an older compose file that predates the + * self-mount — which is to say, exactly the long-running instances most in need of + * updating. + */ +function preflight(deps = {}) { + const exists = deps.existsSync || fs.existsSync; + const labels = deps.labels || readComposeLabels(deps); + + if (!exists(COMPOSE_FILE)) { + return { + ok: false, + error: `${COMPOSE_FILE} is not mounted in this container, so the update cannot read your compose file. ` + + 'This container predates that mount. Run "docker compose pull && docker compose up -d" ' + + 'on the host once; in-app updates will work from then on.', + }; + } + + if (!exists('/var/run/docker.sock')) { + return { + ok: false, + error: 'The Docker socket is not mounted, so this container cannot manage the stack. ' + + 'Add /var/run/docker.sock to the backend volumes and recreate it.', + }; + } + + if (!labels.workingDir || !labels.configFile) { + return { + ok: false, + error: 'This container is missing its compose project labels, so the update cannot locate ' + + 'the project on the host. That happens when the stack was started with "docker run" ' + + 'rather than "docker compose up". Start it with compose and try again.', + }; + } + + return { ok: true, labels }; +} + +/** Update progress, so the client can be told rather than left guessing. */ +const state = { + phase: 'idle', + error: null, + startedAt: null, + finishedAt: null, +}; + +function getState() { + let log = ''; + try { + const raw = fs.readFileSync(logPath(), 'utf8'); + log = raw.split('\n').slice(-40).join('\n'); + } catch { /* no log yet */ } + return { ...state, bootId: BOOT_ID, log }; +} + +function resetState() { + state.phase = 'idle'; + state.error = null; + state.startedAt = null; + state.finishedAt = null; +} + +function fail(error) { + state.phase = 'failed'; + state.error = error; + state.finishedAt = Date.now(); +} + +/** + * Pull the new images, then hand off to a helper container to recreate the stack. + * + * The helper exists because `up -d` replaces this very container, so whatever runs it + * has to outlive it. Both steps append to the update log; previously both discarded + * their output, which left nothing to diagnose when an update quietly did nothing. + */ +function runUpdate(labels, deps = {}) { + const spawnFn = deps.spawn || spawn; + const open = deps.openSync || fs.openSync; + + state.phase = 'pulling'; + state.error = null; + state.startedAt = Date.now(); + state.finishedAt = null; + + let out; + try { + out = open(logPath(), 'a'); + } catch { + out = 'ignore'; + } + const stdio = out === 'ignore' ? 'ignore' : ['ignore', out, out]; + + const projectArgs = labels.projectName ? ['-p', labels.projectName] : []; + const composeArgs = ['compose', '-f', COMPOSE_FILE, ...projectArgs]; + + const pull = spawnFn('docker', [...composeArgs, 'pull'], { detached: true, stdio }); + pull.unref(); + + pull.on('error', (err) => fail(`Could not run docker: ${err.message}`)); + + pull.on('close', (code) => { + if (code !== 0) { + return fail(`"docker compose pull" exited with code ${code}. See update.log for the reason.`); + } + state.phase = 'restarting'; + const helper = spawnFn( + 'docker', + buildUpdateHelperArgs(labels.workingDir, labels.configFile, projectArgs), + { detached: true, stdio } + ); + helper.unref(); + helper.on('error', (err) => fail(`Could not start the update helper: ${err.message}`)); + }); + + return pull; +} + +module.exports = { + COMPOSE_FILE, + BOOT_ID, + isNewerVersion, + buildUpdateHelperArgs, + readComposeLabels, + preflight, + runUpdate, + getState, + resetState, + logPath, +}; diff --git a/frontend/package.json b/frontend/package.json index ce94df9..9800d79 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -1,7 +1,7 @@ { "name": "frontend", "private": true, - "version": "1.8.0", + "version": "1.8.1", "type": "module", "scripts": { "dev": "vite --host", diff --git a/frontend/src/components/UpdateModal.tsx b/frontend/src/components/UpdateModal.tsx index b9eaa5d..01a9879 100644 --- a/frontend/src/components/UpdateModal.tsx +++ b/frontend/src/components/UpdateModal.tsx @@ -11,8 +11,9 @@ interface Props { } export function UpdateModal({ current, latest, message, token, isDocker, onDismiss, onSkip }: Props) { - const [phase, setPhase] = useState<'idle' | 'updating' | 'done'>('idle'); + const [phase, setPhase] = useState<'idle' | 'updating' | 'failed' | 'done'>('idle'); const [statusMsg, setStatusMsg] = useState(''); + const [detail, setDetail] = useState(''); // Draggable const modalRef = useRef(null); @@ -39,27 +40,82 @@ export function UpdateModal({ current, latest, message, token, isDocker, onDismi }; }, []); + /** + * How long to wait for the server to come back before calling it a failure. + * + * A pull and a container recreate is minutes, not seconds, on a slow connection. But + * it is bounded: the previous version polled every three seconds forever, so a stack + * that could not update looked identical to one still working, and sat on + * "WAITING FOR SERVER" indefinitely. + */ + const DEADLINE_MS = 6 * 60 * 1000; + const handleUpdate = async () => { setPhase('updating'); setStatusMsg('UPDATE IN PROGRESS — WAITING FOR SERVER...'); + setDetail(''); + + let bootId = ''; + try { + const before = await (await fetch('/api/version')).json(); + bootId = before.bootId ?? ''; + } catch { /* carry on; the restart check falls back to the version */ } + try { - await fetch('/api/update', { method: 'POST', headers: { Authorization: `Bearer ${token}` } }); - const poll = async () => { - try { - const res = await fetch('/api/version'); - if (!res.ok) throw new Error(); + const res = await fetch('/api/update', { method: 'POST', headers: { Authorization: `Bearer ${token}` } }); + if (!res.ok) { + // Preflight refused it and said why — much the commonest case being a container + // started before the compose file mounted itself. + const body = await res.json().catch(() => ({})); + setPhase('failed'); + setStatusMsg('UPDATE CANNOT RUN'); + setDetail(body.error || `Server returned ${res.status}.`); + return; + } + } catch (e) { + setPhase('failed'); + setStatusMsg('UPDATE FAILED TO START'); + setDetail(e instanceof Error ? e.message : 'The server could not be reached.'); + return; + } + + const deadline = Date.now() + DEADLINE_MS; + const poll = async () => { + // The server reports its own failures now, so ask before assuming it is just slow. + try { + const st = await (await fetch('/api/update/status')).json(); + if (st.phase === 'failed') { + setPhase('failed'); + setStatusMsg('UPDATE FAILED'); + setDetail(st.error || 'No reason given.'); + return; + } + } catch { /* the server is restarting, which is the point */ } + + try { + const res = await fetch('/api/version'); + if (res.ok) { const data = await res.json(); - if (data.version !== current) { + // A restart is what matters. Waiting on the version alone hangs forever on a + // build without APP_VERSION, which reports 'dev' before and after. + const restarted = bootId ? data.bootId && data.bootId !== bootId : data.version !== current; + if (restarted) { window.location.href = `/?v=${Date.now()}`; return; } - } catch { /* server restarting */ } - setTimeout(poll, 3000); - }; - setTimeout(poll, 10000); - } catch { - setStatusMsg('Update failed — try manually from the nav panel'); - } + } + } catch { /* server restarting */ } + + if (Date.now() > deadline) { + setPhase('failed'); + setStatusMsg('UPDATE TIMED OUT'); + setDetail('The server did not come back within six minutes. It may still be pulling — ' + + 'check "docker compose ps" on the host, and backend/data/update.log for what happened.'); + return; + } + setTimeout(poll, 3000); + }; + setTimeout(poll, 10000); }; const panelStyle: React.CSSProperties = { @@ -160,6 +216,17 @@ export function UpdateModal({ current, latest, message, token, isDocker, onDismi {phase === 'updating' && (
{statusMsg}
)} + + {phase === 'failed' && ( + <> +
{statusMsg}
+
{detail}
+
+ + +
+ + )} ); diff --git a/frontend/src/components/__tests__/UpdateModal.test.tsx b/frontend/src/components/__tests__/UpdateModal.test.tsx index 64efa37..42473c2 100644 --- a/frontend/src/components/__tests__/UpdateModal.test.tsx +++ b/frontend/src/components/__tests__/UpdateModal.test.tsx @@ -145,7 +145,43 @@ describe('UpdateModal — Update Now', () => { render(); await userEvent.click(screen.getByText('UPDATE NOW')); await waitFor(() => { - expect(screen.getByText(/Update failed/)).toBeInTheDocument(); + expect(screen.getByText(/UPDATE FAILED TO START/)).toBeInTheDocument(); }); + expect(screen.getByText(/network error/)).toBeInTheDocument(); + }); + + it('reports why the server refused, rather than waiting for a restart', async () => { + // Preflight rejects a container that cannot possibly update — most often one started + // before the compose file mounted itself. Previously the client ignored the response + // entirely and polled for a version change that was never coming, which is how an + // instance sits on WAITING FOR SERVER indefinitely. + vi.stubGlobal('fetch', vi.fn(async (url: string) => { + if (String(url).includes('/api/version')) { + return { ok: true, json: async () => ({ version: '1.8.0', bootId: 'boot-1' }) }; + } + return { + ok: false, + status: 409, + json: async () => ({ error: '/tmp/docker-compose.yml is not mounted in this container' }), + }; + })); + + render(); + await userEvent.click(screen.getByText('UPDATE NOW')); + + await waitFor(() => { + expect(screen.getByText(/UPDATE CANNOT RUN/)).toBeInTheDocument(); + }); + expect(screen.getByText(/docker-compose.yml is not mounted/)).toBeInTheDocument(); + }); + + it('offers a way back after a failure instead of stranding the modal', async () => { + vi.stubGlobal('fetch', vi.fn().mockRejectedValue(new Error('network error'))); + render(); + await userEvent.click(screen.getByText('UPDATE NOW')); + await waitFor(() => expect(screen.getByText('BACK')).toBeInTheDocument()); + + await userEvent.click(screen.getByText('BACK')); + expect(screen.getByText('UPDATE NOW')).toBeInTheDocument(); }); }); diff --git a/package.json b/package.json index 6c12b9c..5bf5f50 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "mapsystem", - "version": "1.8.0", + "version": "1.8.1", "description": "", "main": "index.js", "scripts": { From 89968077e5848c57fa6d78d3b7529d8f9ee99270 Mon Sep 17 00:00:00 2001 From: Developer Date: Sun, 2 Aug 2026 23:07:00 -0500 Subject: [PATCH 02/16] feat(update): tell a stale container it cannot update itself MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A container from before the self-checking backend answers POST /api/update with "Update started" and then does nothing — the failure this branch exists to stop being silent. The hardening only helps once that build is running, which the stuck instance by definition is not. So the client asks first. GET /api/update/status exists only in the new build; if it is missing, the container predates the fix, and the modal says so and shows the command to run on the host instead of posting an update that will be swallowed. That turns a six-minute wait into an immediate, actionable answer, and it works from a frontend newer than its backend — which is exactly the shape of a partly-failed update. The response shape is checked, not just the status code: a setup serving index.html for unknown paths answers 200 with a page and would otherwise read as modern. There is a test for that specifically. Also staged the waiting message — at 45 seconds it says a pull genuinely takes a few minutes, so a working update does not look stalled — and the host command is now shown on the timeout path too, as selectable text. Two existing tests stubbed every response as {}, which the probe correctly reads as a stale server; their stubs now describe a modern one. --- CHANGELOG.md | 3 +- frontend/src/components/UpdateModal.tsx | 68 ++++++++++++++-- .../components/__tests__/UpdateModal.test.tsx | 80 ++++++++++++++++++- 3 files changed, 141 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b64657f..e553f3b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,7 +17,8 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). The likeliest cause on a long-running instance is the compose file's own self-mount at `/tmp/docker-compose.yml`, which the update reads. A container started before that line existed does not have it, the pull fails, and everything above turns that into an endless wait — so the instances least able to update in place are exactly the ones that have been running longest. - `POST /api/update` now checks the mount, the Docker socket and the compose project labels *before* answering, and returns `409` naming what is missing and what to do about it. Both steps append to `backend/data/update.log`, which lives on the data volume and so survives the container being replaced. `GET /api/update/status` reports phase and error, and the modal shows them and gives up after six minutes. + `POST /api/update` now checks the mount, the Docker socket and the compose project labels *before* answering, and returns `409` naming what is missing and what to do about it. Both steps append to `backend/data/update.log`, which lives on the data volume and so survives the container being replaced. `GET /api/update/status` reports phase and error, and the modal shows them, reassures at 45 seconds that a pull legitimately takes minutes, and gives up after six with the host command to fall back to. +- **A container too old to update itself now says so immediately.** Such a container answers `POST /api/update` with "Update started" and then does nothing, so the client used to wait out the full deadline to learn what could be known at once. It is asked for `GET /api/update/status` first — a route that only exists in the self-checking build — and if that is missing the modal says the container predates it and shows the command to run on the host. The response shape is checked rather than just the status code, since a setup serving `index.html` for unknown paths answers `200` with a page. Nothing is POSTed to a server that cannot act on it. - **A successful update could hang too.** The client waited for the reported version to change, but a build without `APP_VERSION` reports `dev` before and after. `/api/version` now carries a boot id and the client waits for the restart itself. - **The update check offered downgrades.** `hasUpdate` was `latest !== current`, so a published tag trailing the running one counted as an update — a 1.8.0 instance was offered 1.7.4. It is a numeric version comparison now, and anything unparseable (`dev`, `latest`) is never offered. diff --git a/frontend/src/components/UpdateModal.tsx b/frontend/src/components/UpdateModal.tsx index 01a9879..a0c945b 100644 --- a/frontend/src/components/UpdateModal.tsx +++ b/frontend/src/components/UpdateModal.tsx @@ -14,6 +14,7 @@ export function UpdateModal({ current, latest, message, token, isDocker, onDismi const [phase, setPhase] = useState<'idle' | 'updating' | 'failed' | 'done'>('idle'); const [statusMsg, setStatusMsg] = useState(''); const [detail, setDetail] = useState(''); + const [command, setCommand] = useState(''); // Draggable const modalRef = useRef(null); @@ -50,11 +51,52 @@ export function UpdateModal({ current, latest, message, token, isDocker, onDismi */ const DEADLINE_MS = 6 * 60 * 1000; + /** Long enough that a normal pull has not finished, short enough to reassure. */ + const REASSURE_MS = 45 * 1000; + + /** What to run on the host when the container cannot update itself. */ + const MANUAL_COMMAND = 'docker compose pull && docker compose up -d'; + + /** + * Does the server behind this page have the self-checking updater? + * + * A container from before it has no `/api/update/status`, and asking is the one + * reliable way to find out — its `/api/update` cheerfully answers "Update started" + * and then does nothing, which is the whole failure being guarded against here. + * + * The shape is checked, not just the status code: a setup that serves index.html for + * unknown paths would otherwise answer 200 with a page and look modern. + */ + const hasModernUpdater = async () => { + try { + const res = await fetch('/api/update/status'); + if (!res.ok) return false; + const data = await res.json(); + return typeof data?.phase === 'string'; + } catch { + return false; + } + }; + const handleUpdate = async () => { setPhase('updating'); - setStatusMsg('UPDATE IN PROGRESS — WAITING FOR SERVER...'); + setStatusMsg('CHECKING SERVER...'); setDetail(''); + if (!(await hasModernUpdater())) { + // Told immediately rather than after a six-minute wait for a restart that this + // container was never going to perform. + setPhase('failed'); + setStatusMsg('THIS CONTAINER CANNOT UPDATE ITSELF'); + setDetail('It was built before the self-updating backend, so the in-app update would ' + + 'report success and then do nothing. Run this on the host, in the folder holding ' + + 'docker-compose.yml — after that, in-app updates work.'); + setCommand(MANUAL_COMMAND); + return; + } + + setStatusMsg('UPDATE IN PROGRESS — WAITING FOR SERVER...'); + let bootId = ''; try { const before = await (await fetch('/api/version')).json(); @@ -79,8 +121,12 @@ export function UpdateModal({ current, latest, message, token, isDocker, onDismi return; } - const deadline = Date.now() + DEADLINE_MS; + const started = Date.now(); + const deadline = started + DEADLINE_MS; const poll = async () => { + if (Date.now() - started > REASSURE_MS) { + setStatusMsg('STILL WORKING — PULLING IMAGES, THIS CAN TAKE A FEW MINUTES...'); + } // The server reports its own failures now, so ask before assuming it is just slow. try { const st = await (await fetch('/api/update/status')).json(); @@ -109,8 +155,9 @@ export function UpdateModal({ current, latest, message, token, isDocker, onDismi if (Date.now() > deadline) { setPhase('failed'); setStatusMsg('UPDATE TIMED OUT'); - setDetail('The server did not come back within six minutes. It may still be pulling — ' - + 'check "docker compose ps" on the host, and backend/data/update.log for what happened.'); + setDetail('The server did not come back within six minutes. Check backend/data/update.log ' + + 'for what happened, then recreate the stack from the host:'); + setCommand(MANUAL_COMMAND); return; } setTimeout(poll, 3000); @@ -221,8 +268,19 @@ export function UpdateModal({ current, latest, message, token, isDocker, onDismi <>
{statusMsg}
{detail}
+ {command && ( +
+ {command} +
+ )}
- +
diff --git a/frontend/src/components/__tests__/UpdateModal.test.tsx b/frontend/src/components/__tests__/UpdateModal.test.tsx index 42473c2..4b44e37 100644 --- a/frontend/src/components/__tests__/UpdateModal.test.tsx +++ b/frontend/src/components/__tests__/UpdateModal.test.tsx @@ -122,7 +122,12 @@ describe('UpdateModal — non-docker install', () => { describe('UpdateModal — Update Now', () => { it('shows updating status message after clicking UPDATE NOW', async () => { - vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ ok: true, json: async () => ({}) })); + vi.stubGlobal('fetch', vi.fn(async (url: string) => ({ + ok: true, + json: async () => (String(url).includes('/api/update/status') + ? { phase: 'idle' } + : { version: '1.8.0', bootId: 'boot-1' }), + }))); render(); await userEvent.click(screen.getByText('UPDATE NOW')); await waitFor(() => { @@ -131,7 +136,12 @@ describe('UpdateModal — Update Now', () => { }); it('hides action buttons while updating', async () => { - vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ ok: true, json: async () => ({}) })); + vi.stubGlobal('fetch', vi.fn(async (url: string) => ({ + ok: true, + json: async () => (String(url).includes('/api/update/status') + ? { phase: 'idle' } + : { version: '1.8.0', bootId: 'boot-1' }), + }))); render(); await userEvent.click(screen.getByText('UPDATE NOW')); await waitFor(() => { @@ -141,7 +151,11 @@ describe('UpdateModal — Update Now', () => { }); it('shows failure message if update fetch throws', async () => { - vi.stubGlobal('fetch', vi.fn().mockRejectedValue(new Error('network error'))); + vi.stubGlobal('fetch', vi.fn(async (url: string, opts: any) => { + if (String(url).includes('/api/update/status')) return { ok: true, json: async () => ({ phase: 'idle' }) }; + if (opts?.method === 'POST') throw new Error('network error'); + return { ok: true, json: async () => ({ version: '1.8.0', bootId: 'boot-1' }) }; + })); render(); await userEvent.click(screen.getByText('UPDATE NOW')); await waitFor(() => { @@ -150,12 +164,66 @@ describe('UpdateModal — Update Now', () => { expect(screen.getByText(/network error/)).toBeInTheDocument(); }); + it('detects a container too old to update itself and gives the host command', async () => { + // A stale backend has no /api/update/status. Its /api/update answers "Update + // started" and then does nothing, so without this probe the client waits six + // minutes to learn what can be known immediately. + vi.stubGlobal('fetch', vi.fn(async (url: string) => { + if (String(url).includes('/api/update/status')) return { ok: false, status: 404, json: async () => ({}) }; + return { ok: true, json: async () => ({ version: '1.7.0' }) }; + })); + + render(); + await userEvent.click(screen.getByText('UPDATE NOW')); + + await waitFor(() => { + expect(screen.getByText(/CANNOT UPDATE ITSELF/)).toBeInTheDocument(); + }); + expect(screen.getByText('docker compose pull && docker compose up -d')).toBeInTheDocument(); + }); + + it('treats an index.html fallback as a stale server, not a modern one', async () => { + // A setup that serves the SPA for unknown paths answers 200 with a page, which a + // status-code check alone would read as "this server has the new updater". + vi.stubGlobal('fetch', vi.fn(async (url: string) => { + if (String(url).includes('/api/update/status')) { + return { ok: true, json: async () => { throw new Error('not json'); } }; + } + return { ok: true, json: async () => ({ version: '1.7.0' }) }; + })); + + render(); + await userEvent.click(screen.getByText('UPDATE NOW')); + + await waitFor(() => { + expect(screen.getByText(/CANNOT UPDATE ITSELF/)).toBeInTheDocument(); + }); + }); + + it('does not send the update to a server that cannot run it', async () => { + const fetchMock = vi.fn(async (url: string) => { + if (String(url).includes('/api/update/status')) return { ok: false, status: 404, json: async () => ({}) }; + return { ok: true, json: async () => ({ version: '1.7.0' }) }; + }); + vi.stubGlobal('fetch', fetchMock); + + render(); + await userEvent.click(screen.getByText('UPDATE NOW')); + await waitFor(() => expect(screen.getByText(/CANNOT UPDATE ITSELF/)).toBeInTheDocument()); + + const posted = fetchMock.mock.calls.some(([u, o]: any[]) => String(u).endsWith('/api/update') && o?.method === 'POST'); + expect(posted).toBe(false); + }); + it('reports why the server refused, rather than waiting for a restart', async () => { // Preflight rejects a container that cannot possibly update — most often one started // before the compose file mounted itself. Previously the client ignored the response // entirely and polled for a version change that was never coming, which is how an // instance sits on WAITING FOR SERVER indefinitely. vi.stubGlobal('fetch', vi.fn(async (url: string) => { + if (String(url).includes('/api/update/status')) { + return { ok: true, json: async () => ({ phase: 'idle' }) }; + } if (String(url).includes('/api/version')) { return { ok: true, json: async () => ({ version: '1.8.0', bootId: 'boot-1' }) }; } @@ -176,7 +244,11 @@ describe('UpdateModal — Update Now', () => { }); it('offers a way back after a failure instead of stranding the modal', async () => { - vi.stubGlobal('fetch', vi.fn().mockRejectedValue(new Error('network error'))); + vi.stubGlobal('fetch', vi.fn(async (url: string, opts: any) => { + if (String(url).includes('/api/update/status')) return { ok: true, json: async () => ({ phase: 'idle' }) }; + if (opts?.method === 'POST') throw new Error('network error'); + return { ok: true, json: async () => ({ version: '1.8.0', bootId: 'boot-1' }) }; + })); render(); await userEvent.click(screen.getByText('UPDATE NOW')); await waitFor(() => expect(screen.getByText('BACK')).toBeInTheDocument()); From d21b9f12db6c2b18666df7e5150dcee68a788960 Mon Sep 17 00:00:00 2001 From: Developer Date: Sun, 2 Aug 2026 23:28:29 -0500 Subject: [PATCH 03/16] feat(update): optional dev channel, off by default MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DEV=false in backend/.env. Stable releases are all anyone sees unless they deliberately ask for otherwise, which is the right default for unreleased code. Dev builds are tagged X.Y.Z-dev with an optional counter, so 1.9.0-dev and 1.9.0-dev.7 both parse and publishing counters later is a change to the build workflow rather than to this code — which removes the one decision that had to be made up front. Ordering follows the three rules that matter: a dev build of a newer release is offered to someone on an older release; the release supersedes its own dev builds when it lands, so a dev user is carried onto it; and a release user is never dragged back onto a dev build of the same version. Two settings rather than one, because they do different jobs. DEV decides what is offered, IMAGE_TAG=dev decides what is pulled — docker-compose.yml now reads ${IMAGE_TAG:-latest} rather than hardcoding latest. Setting only DEV would offer a dev version and then install the stable one, since the compose file is what decides the image. Both are documented together in .env.example with that trap spelled out. This also fixes a latent fault that would have bitten on the first dev tag published, and would have hurt stable users rather than dev ones: the tag filter was unanchored, so 1.9.0-dev passed it, parsed to NaN, and made the sort comparator return NaN. With the ordering undefined a prerelease could surface as the newest tag, which the version check would then correctly refuse — leaving stable users told there was no update when there was. --- CHANGELOG.md | 7 +++ README.md | 2 +- backend/.env.example | 12 ++++ backend/__tests__/updater.test.js | 91 ++++++++++++++++++++++++++++++- backend/routes/admin.js | 17 +++--- backend/updater.js | 70 ++++++++++++++++++++---- docker-compose.yml | 7 ++- 7 files changed, 180 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e553f3b..90f77ef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,8 +11,15 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ## [1.8.1] - 2026-08-02 +### Added + +- **An optional development channel.** Off by default — `DEV=false` in `backend/.env`, and stable releases are all anyone sees unless they ask otherwise. Setting `DEV=true` makes the update check consider `X.Y.Z-dev` builds alongside releases, with a counter accepted but not required, so `1.9.0-dev` and `1.9.0-dev.7` both work and adding counters later is a build-workflow change rather than a code one. A dev build of a newer release is offered to someone on an older release, the release itself supersedes its own dev builds when it lands, and a release user is never dragged back onto a dev build of the same version. + + It takes two settings, not one, because they do different jobs: `DEV` decides what is *offered*, and `IMAGE_TAG=dev` decides what is *pulled* — `docker-compose.yml` now reads `${IMAGE_TAG:-latest}` instead of hardcoding `latest`. Setting only `DEV` would offer a dev version and then install the stable one. `.env.example` says so where both are defined. + ### Fixed +- **One dev tag on the registry would have silenced update notices for everyone.** The tag filter was `/^\d+\.\d+\.\d+/`, unanchored, so `1.9.0-dev` passed it and then parsed to `NaN` — which made the sort comparator return `NaN`, leaving the ordering undefined and letting a prerelease surface as the newest tag, whereupon the version check correctly refused it and reported no update at all. Version tags are matched strictly now, and sorted by a comparator that understands them. - **The in-app updater could sit on `WAITING FOR SERVER` indefinitely.** Every step failed silently — both child processes discarded their output, the route answered "Update started" before checking anything could work, and a non-zero exit from `docker compose pull` simply returned — so a stack that *could not* update was indistinguishable from one still working, and the client polled every three seconds forever with no deadline. The likeliest cause on a long-running instance is the compose file's own self-mount at `/tmp/docker-compose.yml`, which the update reads. A container started before that line existed does not have it, the pull fails, and everything above turns that into an endless wait — so the instances least able to update in place are exactly the ones that have been running longest. diff --git a/README.md b/README.md index 73633ed..e6693c3 100644 --- a/README.md +++ b/README.md @@ -323,7 +323,7 @@ CITY_NET/ ├── backend/ │ ├── server.js # Express entrypoint — mounts routes, starts Socket.IO │ ├── db.js # SQLite schema and migrations -│ ├── updater.js # In-app self-update — preflight (compose file mounted, docker socket, compose project labels) so a stack that cannot update says why instead of hanging; upgrade-only semver check; update log on the data volume; boot id so a restart is detectable without a version change +│ ├── updater.js # In-app self-update — release channels (DEV=false by default; X.Y.Z-dev tags with an optional counter, ordered so a release supersedes its own dev builds); preflight (compose file mounted, docker socket, compose project labels) so a stack that cannot update says why instead of hanging; upgrade-only semver check; update log on the data volume; boot id so a restart is detectable without a version change │ ├── middleware/ │ │ └── auth.js # JWT verify middleware (admin + elevated users) │ ├── routes/ diff --git a/backend/.env.example b/backend/.env.example index 77ed05c..a593089 100644 --- a/backend/.env.example +++ b/backend/.env.example @@ -17,3 +17,15 @@ DUCKDNS_TOKEN=your-duckdns-token # Timezone for DuckDNS container (e.g. America/New_York) TZ=America/Chicago + +# ── Development builds (optional) ──────────────────────────────────────────── +# Off by default. Stable releases only, which is what almost everyone wants. +# +# Set BOTH to follow development builds — they do different jobs and must agree: +# DEV=true the update check offers X.Y.Z-dev versions as well as releases +# IMAGE_TAG=dev docker compose actually pulls the dev images +# +# DEV alone would offer you a dev version and then pull the stable one, since the +# compose file decides what is fetched. Dev builds are unreleased and may break. +DEV=false +IMAGE_TAG=latest diff --git a/backend/__tests__/updater.test.js b/backend/__tests__/updater.test.js index 381ed37..bfd7075 100644 --- a/backend/__tests__/updater.test.js +++ b/backend/__tests__/updater.test.js @@ -37,9 +37,13 @@ describe('isNewerVersion', () => { expect(updater.isNewerVersion('1.9.0', '1.10.0')).toBe(false); }); - it('treats a missing segment as zero', () => { - expect(updater.isNewerVersion('1.9', '1.8.7')).toBe(true); - expect(updater.isNewerVersion('1.8', '1.8.0')).toBe(false); + it('requires all three segments rather than guessing at a partial version', () => { + // Deliberately strict. Every tag this project publishes comes from package.json and + // has three parts, and a loose parser is what let '1.9.0-dev' through the filter and + // then produce NaN in the sort comparator. + expect(updater.isNewerVersion('1.9', '1.8.7')).toBe(false); + expect(updater.isNewerVersion('1.9.0', '1.8')).toBe(false); + expect(updater.parseVersion('1.9')).toBeNull(); }); it('refuses anything it cannot parse rather than guessing', () => { @@ -51,6 +55,87 @@ describe('isNewerVersion', () => { }); }); +describe('dev channel', () => { + it('is off unless explicitly turned on', () => { + // A dev build is unreleased code. Nobody should land on one by leaving a value out. + expect(updater.devChannelEnabled({})).toBe(false); + expect(updater.devChannelEnabled({ DEV: 'false' })).toBe(false); + expect(updater.devChannelEnabled({ DEV: '' })).toBe(false); + expect(updater.devChannelEnabled({ DEV: '1' })).toBe(false); + expect(updater.devChannelEnabled({ DEV: 'yes' })).toBe(false); + }); + + it('turns on for true, whatever the casing or padding', () => { + expect(updater.devChannelEnabled({ DEV: 'true' })).toBe(true); + expect(updater.devChannelEnabled({ DEV: 'TRUE' })).toBe(true); + expect(updater.devChannelEnabled({ DEV: ' true ' })).toBe(true); + }); + + it('hides dev tags from the stable channel', () => { + expect(updater.isVersionTag('1.9.0', false)).toBe(true); + expect(updater.isVersionTag('1.9.0-dev', false)).toBe(false); + expect(updater.isVersionTag('1.9.0-dev.7', false)).toBe(false); + }); + + it('shows dev tags to the dev channel', () => { + expect(updater.isVersionTag('1.9.0-dev', true)).toBe(true); + expect(updater.isVersionTag('1.9.0-dev.7', true)).toBe(true); + expect(updater.isVersionTag('1.9.0', true)).toBe(true); + }); + + it('rejects tags that are not versions on either channel', () => { + // The filter this replaces was unanchored, so '1.9.0-dev' passed it, parsed to NaN, + // made the sort comparator return NaN, and left the ordering undefined — a single + // dev tag on the registry could stop stable users hearing about releases at all. + for (const tag of ['latest', 'dev', '1.9.0-rc1', '1.9', 'v1.9.0', '']) { + expect(updater.isVersionTag(tag, false), tag).toBe(false); + expect(updater.isVersionTag(tag, true), tag).toBe(false); + } + }); + + it('sorts a mixed tag list without NaN poisoning the comparator', () => { + const tags = ['1.8.0', '1.9.0-dev.2', '1.10.0', '1.9.0', '1.9.0-dev.10']; + const sorted = [...tags].sort((a, b) => + updater.compareVersions(updater.parseVersion(b), updater.parseVersion(a))); + expect(sorted[0]).toBe('1.10.0'); + expect(sorted).toEqual(['1.10.0', '1.9.0', '1.9.0-dev.10', '1.9.0-dev.2', '1.8.0']); + }); +}); + +describe('dev version ordering', () => { + it('offers a dev build of a newer release to someone on an older release', () => { + expect(updater.isNewerVersion('1.9.0-dev', '1.8.1')).toBe(true); + }); + + it('carries a dev user onto the release when it lands', () => { + expect(updater.isNewerVersion('1.9.0', '1.9.0-dev')).toBe(true); + expect(updater.isNewerVersion('1.9.0', '1.9.0-dev.7')).toBe(true); + }); + + it('never drags a release user back onto a dev build of the same version', () => { + expect(updater.isNewerVersion('1.9.0-dev', '1.9.0')).toBe(false); + expect(updater.isNewerVersion('1.9.0-dev.7', '1.9.0')).toBe(false); + }); + + it('orders dev builds by their counter when there is one', () => { + expect(updater.isNewerVersion('1.9.0-dev.7', '1.9.0-dev.3')).toBe(true); + expect(updater.isNewerVersion('1.9.0-dev.3', '1.9.0-dev.7')).toBe(false); + // Ten after two, not before it — the old comparison was textual. + expect(updater.isNewerVersion('1.9.0-dev.10', '1.9.0-dev.2')).toBe(true); + }); + + it('accepts a counter without requiring one', () => { + // So publishing counters later is a change to the build workflow, not to this code. + expect(updater.parseVersion('1.9.0-dev')).toEqual({ core: [1, 9, 0], dev: 0 }); + expect(updater.parseVersion('1.9.0-dev.7')).toEqual({ core: [1, 9, 0], dev: 7 }); + expect(updater.parseVersion('1.9.0')).toEqual({ core: [1, 9, 0], dev: null }); + }); + + it('offers no update between identical dev builds', () => { + expect(updater.isNewerVersion('1.9.0-dev', '1.9.0-dev')).toBe(false); + }); +}); + describe('preflight', () => { const labels = { projectName: 'citynet', configFile: '/srv/citynet/docker-compose.yml', workingDir: '/srv/citynet' }; const allPresent = () => true; diff --git a/backend/routes/admin.js b/backend/routes/admin.js index e9789b0..b7dd3bc 100644 --- a/backend/routes/admin.js +++ b/backend/routes/admin.js @@ -238,17 +238,16 @@ module.exports = (db, io, { emitUpdate, recordAction }) => { try { const data = JSON.parse(body); // Find version tags (skip 'latest'), sort and get highest version + // Dev builds are published as X.Y.Z-dev and ignored unless DEV=true. The + // previous filter was unanchored, so a tag like 1.9.0-dev passed it and then + // parsed to NaN, which made the comparator return NaN and left the sort + // order undefined — one dev tag on the registry could stop stable users + // being told about releases at all. + const allowDev = updater.devChannelEnabled(); const versionTags = data.results - ?.filter(tag => tag.name !== 'latest' && /^\d+\.\d+\.\d+/.test(tag.name)) + ?.filter(tag => updater.isVersionTag(tag.name, allowDev)) .map(tag => tag.name) - .sort((a, b) => { - const aParts = a.split('.').map(Number); - const bParts = b.split('.').map(Number); - for (let i = 0; i < 3; i++) { - if (aParts[i] !== bParts[i]) return bParts[i] - aParts[i]; - } - return 0; - }) || []; + .sort((a, b) => updater.compareVersions(updater.parseVersion(b), updater.parseVersion(a))) || []; const latestTag = versionTags[0] || 'unknown'; // Strictly newer, not merely different. Comparing with !== offers a // downgrade whenever the published tag trails the running one. diff --git a/backend/updater.js b/backend/updater.js index 0d3f957..11ed0ae 100644 --- a/backend/updater.js +++ b/backend/updater.js @@ -28,7 +28,52 @@ function logPath() { } /** - * Compare two dotted versions. True when `candidate` is strictly newer than `current`. + * A released version, or a dev build of one. + * + * Dev builds are tagged `X.Y.Z-dev`, optionally with a counter — `1.9.0-dev.7`. The + * counter is accepted but not required, so publishing one later is a change to the + * build workflow and not to this code. + * + * Returns null for anything else, `dev` and `latest` included. That matters: a build + * without APP_VERSION reports `dev`, and there is no honest way to say whether some + * numbered release is newer than an unknown. + */ +function parseVersion(v) { + const m = /^(\d+)\.(\d+)\.(\d+)(?:-dev(?:\.(\d+))?)?$/.exec(String(v).trim()); + if (!m) return null; + return { + core: [Number(m[1]), Number(m[2]), Number(m[3])], + // null means a release; a number means a dev build of that release. + dev: m[4] !== undefined ? Number(m[4]) : (/-dev/.test(v) ? 0 : null), + }; +} + +/** True when this tag is one the given channel should consider at all. */ +function isVersionTag(tag, allowDev) { + const parsed = parseVersion(tag); + if (!parsed) return false; + return allowDev || parsed.dev === null; +} + +/** + * Order two parsed versions. Negative when `a` is older. + * + * `1.9.0-dev` precedes `1.9.0`, which is the rule that lets a dev user be carried onto + * the release when it lands, and stops a release user being dragged back onto a dev + * build of the same version. + */ +function compareVersions(a, b) { + for (let i = 0; i < 3; i++) { + if (a.core[i] !== b.core[i]) return a.core[i] - b.core[i]; + } + if (a.dev === null && b.dev === null) return 0; + if (a.dev === null) return 1; + if (b.dev === null) return -1; + return a.dev - b.dev; +} + +/** + * True when `candidate` is strictly newer than `current`. * * The check this replaces was `latest !== current`, which treats any difference as an * update — so a host publishing an older tag than the one running offers a downgrade, @@ -36,16 +81,15 @@ function logPath() { * to act on. */ function isNewerVersion(candidate, current) { - const parse = (v) => String(v).split('.').map((n) => parseInt(n, 10)); - const a = parse(candidate); - const b = parse(current); - if (a.some(Number.isNaN) || b.some(Number.isNaN)) return false; - for (let i = 0; i < Math.max(a.length, b.length); i++) { - const x = a[i] ?? 0; - const y = b[i] ?? 0; - if (x !== y) return x > y; - } - return false; + const a = parseVersion(candidate); + const b = parseVersion(current); + if (!a || !b) return false; + return compareVersions(a, b) > 0; +} + +/** Whether this deployment has opted into dev builds. Off unless explicitly on. */ +function devChannelEnabled(env = process.env) { + return String(env.DEV ?? '').trim().toLowerCase() === 'true'; } /** @@ -219,6 +263,10 @@ function runUpdate(labels, deps = {}) { module.exports = { COMPOSE_FILE, BOOT_ID, + parseVersion, + compareVersions, + isVersionTag, + devChannelEnabled, isNewerVersion, buildUpdateHelperArgs, readComposeLabels, diff --git a/docker-compose.yml b/docker-compose.yml index 0f83905..89574ad 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -1,6 +1,9 @@ +# IMAGE_TAG selects the release channel and defaults to the stable images. Set it to +# "dev" in backend/.env, alongside DEV=true, to run development builds — the flag alone +# only changes what the update check *offers*, while this is what is actually pulled. services: backend: - image: over2take/citynet-backend:latest + image: over2take/citynet-backend:${IMAGE_TAG:-latest} build: context: . dockerfile: Dockerfile.backend @@ -18,7 +21,7 @@ services: restart: unless-stopped frontend: - image: over2take/citynet-frontend:latest + image: over2take/citynet-frontend:${IMAGE_TAG:-latest} build: context: . dockerfile: Dockerfile.frontend From cd7692a06ca5e49e294a34c09a7e83374eddf0f5 Mon Sep 17 00:00:00 2001 From: Developer Date: Sun, 2 Aug 2026 23:32:11 -0500 Subject: [PATCH 04/16] docs: list updater.test.js and the compose channel in the structure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two gaps from this branch. The backend test listing named every other suite but not the new one, and docker-compose.yml sat in the root list with no note — worth one now that its image tags read ${IMAGE_TAG:-latest} and the release channel is a setting rather than a file edit. --- README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index e6693c3..e77611d 100644 --- a/README.md +++ b/README.md @@ -364,6 +364,7 @@ CITY_NET/ │ ├── helpers/ │ │ └── testDb.js # In-memory SQLite factory for isolated test DBs │ ├── admin.test.js # Admin endpoints (auth, settings, undo access) +│ ├── updater.test.js # Version ordering including X.Y.Z-dev, tag filtering per channel, preflight refusals, and an update that records its failures instead of returning silently │ ├── battle_maps.test.js # Battle map upload/list/delete │ ├── locations.test.js # Location CRUD and classification │ ├── locations.global.test.js # Custom structure global persistence tests @@ -565,7 +566,7 @@ CITY_NET/ ├── docs/ # Reference docs (deployment plans, feature notes) ├── Dockerfile.backend ├── Dockerfile.frontend -├── docker-compose.yml +├── docker-compose.yml # Image tags read ${IMAGE_TAG:-latest}, so the release channel is a setting rather than an edit ├── nginx.conf └── .env.example ``` From 133805f94029d355c00397859e84718116d69fdb Mon Sep 17 00:00:00 2001 From: Developer Date: Sun, 2 Aug 2026 23:38:12 -0500 Subject: [PATCH 05/16] test: cover the update wiring and the compose channel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The module had 35 tests and the seam around it had none, which is the wrong way round for this branch: every fault fixed here lived in that seam. A correct updater.js is no use if the route ignores it. Route level, in admin.test.js: POST /update answers 409 with a reason instead of a false success, /update/status needs no auth because the restart it reports drops the caller's session, /version carries a boot id, and the update is refused without a usable token. Config level, in docker_config.test.js: image tags read IMAGE_TAG rather than a bare :latest — hardcoding it is what would make DEV cosmetic, since compose decides what is actually pulled — neither service is left pinned so a channel cannot half-switch, the compose self-mount the updater reads is still present, and .env.example ships DEV=false. Checked these fail against main's versions of docker-compose.yml and .env.example rather than assuming: four of them go red. One assertion named the wrong layer and was corrected — a temporary admin is turned away at 401 by the middleware, so the route's own isTemporary guard is the second line, reachable only by an elevated temporary. --- README.md | 3 +- backend/__tests__/admin.test.js | 59 +++++++++++++++++++++++++ backend/__tests__/docker_config.test.js | 40 +++++++++++++++++ 3 files changed, 101 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index e77611d..92dfb0e 100644 --- a/README.md +++ b/README.md @@ -363,8 +363,9 @@ CITY_NET/ │ └── __tests__/ │ ├── helpers/ │ │ └── testDb.js # In-memory SQLite factory for isolated test DBs -│ ├── admin.test.js # Admin endpoints (auth, settings, undo access) +│ ├── admin.test.js # Admin endpoints (auth, settings, undo access); update routes — 409 with a reason rather than a false success, unauthenticated status, boot id on /version │ ├── updater.test.js # Version ordering including X.Y.Z-dev, tag filtering per channel, preflight refusals, and an update that records its failures instead of returning silently +│ ├── docker_config.test.js # Deployment invariants — DB_PATH baked in, data excluded from the image, image tags parameterised by IMAGE_TAG, compose file mounted for the updater, dev channel shipped off │ ├── battle_maps.test.js # Battle map upload/list/delete │ ├── locations.test.js # Location CRUD and classification │ ├── locations.global.test.js # Custom structure global persistence tests diff --git a/backend/__tests__/admin.test.js b/backend/__tests__/admin.test.js index 1230273..55bbd37 100644 --- a/backend/__tests__/admin.test.js +++ b/backend/__tests__/admin.test.js @@ -239,3 +239,62 @@ describe('DELETE /api/admin/water (purge all)', () => { expect(rows).toHaveLength(0); }); }); + +// ─── update routes ──────────────────────────────────────────────────────────── + +/** + * These test the wiring, not the logic — updater.test.js covers the logic. + * + * The gap they close is real: every fault this branch fixed lived in the seam between + * the route and what it called, and a module can be correct while the route ignores it. + */ +describe('update routes', () => { + it('GET /version carries a boot id, so a restart is detectable', async () => { + // The client waits on this rather than a version change, because a build without + // APP_VERSION reports 'dev' before and after and would hang on a working update. + const res = await request(app).get('/api/admin/version'); + expect(res.status).toBe(200); + expect(typeof res.body.bootId).toBe('string'); + expect(res.body.bootId.length).toBeGreaterThan(0); + }); + + it('GET /update/status needs no auth, since the restart drops the session', async () => { + const res = await request(app).get('/api/admin/update/status'); + expect(res.status).toBe(200); + expect(res.body).toHaveProperty('phase'); + expect(res.body).toHaveProperty('bootId'); + }); + + it('POST /update refuses with a reason instead of reporting success', async () => { + // Nothing about this test environment is a Docker stack, so preflight must refuse. + // The route previously answered 200 "Update started" regardless, which is what left + // a client polling for a restart that was never going to happen. + const res = await request(app) + .post('/api/admin/update') + .set('Authorization', `Bearer ${ADMIN_TOKEN}`); + + expect(res.status).toBe(409); + expect(typeof res.body.error).toBe('string'); + expect(res.body.error.length).toBeGreaterThan(0); + expect(res.body.message).toBeUndefined(); + }); + + it('POST /update refuses a temporary admin', async () => { + // Rejected at 401 by the middleware, which turns away a temporary token unless the + // user has been elevated — the route's own isTemporary guard is the second line, + // for an elevated temporary who gets past that. + const tempToken = jwt.sign( + { id: 2, username: 'temp', role: 'admin', isTemporary: true }, + 'test-secret' + ); + const res = await request(app) + .post('/api/admin/update') + .set('Authorization', `Bearer ${tempToken}`); + expect(res.status).toBe(401); + }); + + it('POST /update refuses with no token at all', async () => { + const res = await request(app).post('/api/admin/update'); + expect(res.status).toBe(401); + }); +}); diff --git a/backend/__tests__/docker_config.test.js b/backend/__tests__/docker_config.test.js index 5a70101..8999c4d 100644 --- a/backend/__tests__/docker_config.test.js +++ b/backend/__tests__/docker_config.test.js @@ -61,3 +61,43 @@ describe('db.js DB_PATH resolution', () => { expect(src).toMatch(/DB_PATH.*city\.db/); }); }); + +describe('docker-compose.yml release channel', () => { + const compose = () => readRoot('docker-compose.yml'); + + it('reads the image tag from IMAGE_TAG and defaults to latest', () => { + // Hardcoding :latest is what made the DEV flag cosmetic — the check would offer a + // dev version and compose would then pull the stable one, because the compose file + // is what decides the image. + const yml = compose(); + expect(yml).toMatch(/citynet-backend:\$\{IMAGE_TAG:-latest\}/); + expect(yml).toMatch(/citynet-frontend:\$\{IMAGE_TAG:-latest\}/); + }); + + it('pins no image to a bare :latest', () => { + // Leaving one service pinned would half-switch a channel: a dev backend against a + // stable frontend, or the reverse. + expect(compose()).not.toMatch(/citynet-(backend|frontend):latest/); + }); + + it('still mounts the compose file into the backend, which the updater reads', () => { + // Its absence is the single likeliest reason an in-app update does nothing, since a + // container started before this line was added does not have it. + expect(compose()).toMatch(/\.\/docker-compose\.yml:\/tmp\/docker-compose\.yml:ro/); + }); +}); + +describe('.env.example release channel', () => { + const env = () => readRoot('backend/.env.example'); + + it('ships the dev channel switched off', () => { + // Dev builds are unreleased code; nobody should arrive on one by default. + expect(env()).toMatch(/^DEV=false$/m); + }); + + it('documents both settings, since one without the other misleads', () => { + const text = env(); + expect(text).toMatch(/^IMAGE_TAG=latest$/m); + expect(text).toMatch(/IMAGE_TAG=dev/); + }); +}); From 0d7ff034f6a0c73c51ad3f2fad936c2eb0a23a18 Mon Sep 17 00:00:00 2001 From: Developer Date: Sun, 2 Aug 2026 23:46:30 -0500 Subject: [PATCH 06/16] fix(update): refuse to offer dev builds a stable tag cannot install MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DEV=true with IMAGE_TAG=latest is a state a user can reach in one edit, and it does not fail cleanly. The check offers 1.9.0-dev, compose pulls the stable image because compose is what decides that, and if latest has not moved then `up -d` is a no-op, nothing restarts, and the modal waits out its timeout. The next check sees the same dev version as newer and offers it again — a nag loop that can never resolve, the same shape as the 1.8.0 → 1.7.4 one this branch already fixed. Capability now wins over intent: dev versions are ignored until DEV and IMAGE_TAG agree. Offering something that cannot be installed is worse than offering nothing, because it never settles. Reported in three places, because the obvious one is unreliable. At startup in the server log, which is where a config error belongs. In the check-update response. And in the modal — which needed App.tsx to show the modal for a warning even with no update available, since a suppressed dev channel is usually the reason there is no update to show. Also corrects the .env.example note, which was wrong: compose interpolates ${IMAGE_TAG} from the .env beside docker-compose.yml, not from env_file, so setting it only in backend/.env silently falls back to latest. That is a second route into the same contradiction and the warning names it. --- backend/.env.example | 9 ++++- backend/__tests__/updater.test.js | 52 +++++++++++++++++++++++++ backend/routes/admin.js | 5 ++- backend/server.js | 3 ++ backend/updater.js | 47 ++++++++++++++++++++++ frontend/src/App.tsx | 17 ++++++-- frontend/src/components/UpdateModal.tsx | 14 ++++++- 7 files changed, 140 insertions(+), 7 deletions(-) diff --git a/backend/.env.example b/backend/.env.example index a593089..cfcac2b 100644 --- a/backend/.env.example +++ b/backend/.env.example @@ -26,6 +26,13 @@ TZ=America/Chicago # IMAGE_TAG=dev docker compose actually pulls the dev images # # DEV alone would offer you a dev version and then pull the stable one, since the -# compose file decides what is fetched. Dev builds are unreleased and may break. +# compose file decides what is fetched — so the app ignores dev versions until the two +# agree, and says so in the server log at startup. +# +# IMAGE_TAG must also reach the .env beside docker-compose.yml: compose interpolates +# ${IMAGE_TAG} from that file, not from env_file. Re-copy after changing it, exactly as +# the setup steps describe for the DuckDNS values. +# +# Dev builds are unreleased and may break. DEV=false IMAGE_TAG=latest diff --git a/backend/__tests__/updater.test.js b/backend/__tests__/updater.test.js index bfd7075..9760d28 100644 --- a/backend/__tests__/updater.test.js +++ b/backend/__tests__/updater.test.js @@ -136,6 +136,58 @@ describe('dev version ordering', () => { }); }); +describe('channel mismatch', () => { + it('is silent when the channel is consistent', () => { + expect(updater.channelMismatch({ DEV: 'false', IMAGE_TAG: 'latest' })).toBeNull(); + expect(updater.channelMismatch({ DEV: 'true', IMAGE_TAG: 'dev' })).toBeNull(); + expect(updater.channelMismatch({})).toBeNull(); + }); + + it('catches DEV=true against a stable image tag', () => { + // The state a user can easily land in: told about 1.9.0-dev, handed the stable + // image, and offered the same dev version again on the next check for ever, since + // compose is what decides which image is actually pulled. + const problem = updater.channelMismatch({ DEV: 'true', IMAGE_TAG: 'latest' }); + expect(problem).toContain('IMAGE_TAG'); + expect(problem).toContain('latest'); + }); + + it('catches DEV=true with IMAGE_TAG left unset, which defaults to latest', () => { + expect(updater.channelMismatch({ DEV: 'true' })).not.toBeNull(); + }); + + it('mentions the root .env, which is what compose actually reads', () => { + // Setting IMAGE_TAG only in backend/.env is the second way into this state: + // compose interpolates from the file beside docker-compose.yml, not from env_file. + const problem = updater.channelMismatch({ DEV: 'true', IMAGE_TAG: 'latest' }); + expect(problem).toMatch(/docker-compose\.yml/); + }); + + it('stops offering dev versions until intent and capability agree', () => { + // Capability wins over intent. Offering something that cannot be installed is worse + // than not offering it, because it never resolves. + expect(updater.shouldOfferDev({ DEV: 'true', IMAGE_TAG: 'dev' })).toBe(true); + expect(updater.shouldOfferDev({ DEV: 'true', IMAGE_TAG: 'latest' })).toBe(false); + expect(updater.shouldOfferDev({ DEV: 'true' })).toBe(false); + expect(updater.shouldOfferDev({ DEV: 'false', IMAGE_TAG: 'dev' })).toBe(false); + }); + + it('warns at boot, where a config error belongs', () => { + // The update modal only appears when there is an update, and a suppressed dev + // channel is usually exactly why there is not one. + const logged = []; + updater.warnOnChannelMismatch((m) => logged.push(m), { DEV: 'true', IMAGE_TAG: 'latest' }); + expect(logged).toHaveLength(1); + expect(logged[0]).toContain('[update]'); + }); + + it('says nothing at boot when the channel is consistent', () => { + const logged = []; + updater.warnOnChannelMismatch((m) => logged.push(m), { DEV: 'true', IMAGE_TAG: 'dev' }); + expect(logged).toHaveLength(0); + }); +}); + describe('preflight', () => { const labels = { projectName: 'citynet', configFile: '/srv/citynet/docker-compose.yml', workingDir: '/srv/citynet' }; const allPresent = () => true; diff --git a/backend/routes/admin.js b/backend/routes/admin.js index b7dd3bc..921aea7 100644 --- a/backend/routes/admin.js +++ b/backend/routes/admin.js @@ -243,7 +243,7 @@ module.exports = (db, io, { emitUpdate, recordAction }) => { // parsed to NaN, which made the comparator return NaN and left the sort // order undefined — one dev tag on the registry could stop stable users // being told about releases at all. - const allowDev = updater.devChannelEnabled(); + const allowDev = updater.shouldOfferDev(); const versionTags = data.results ?.filter(tag => updater.isVersionTag(tag.name, allowDev)) .map(tag => tag.name) @@ -260,6 +260,9 @@ module.exports = (db, io, { emitUpdate, recordAction }) => { message: hasUpdate ? `Update available: ${currentVersion} → ${latestTag}` : `You're up to date (${currentVersion})`, + // Surfaced even when there is no update, since a suppressed dev channel is + // usually why there isn't one. + warning: updater.channelMismatch(), }); } catch (e) { res.status(500).json({ error: 'Failed to parse Docker Hub response' }); diff --git a/backend/server.js b/backend/server.js index 1ddd599..f985abc 100644 --- a/backend/server.js +++ b/backend/server.js @@ -61,6 +61,9 @@ require('./sockets')(io, db, { elevatedUsers, ...helpers }); server.listen(PORT, '0.0.0.0', () => { console.log(`Server running on port ${PORT}`); + // A contradictory release channel is a config error, and the log is where a config + // error belongs — the update modal only shows when there is an update to show. + require('./updater').warnOnChannelMismatch(); if (process.env.ADMIN_PASS === 'cyberpunk_password' || !process.env.ADMIN_PASS) { console.warn('\x1b[33m⚠️ WARNING: Default admin password in use. Set ADMIN_PASS in your .env file.\x1b[0m'); } diff --git a/backend/updater.js b/backend/updater.js index 11ed0ae..928cc39 100644 --- a/backend/updater.js +++ b/backend/updater.js @@ -92,6 +92,49 @@ function devChannelEnabled(env = process.env) { return String(env.DEV ?? '').trim().toLowerCase() === 'true'; } +/** The image tag this deployment actually pulls. */ +function imageTag(env = process.env) { + return String(env.IMAGE_TAG ?? '').trim() || 'latest'; +} + +/** + * Why `DEV` and `IMAGE_TAG` disagree, or null when they do not. + * + * `DEV` states an intention and `IMAGE_TAG` is the capability: compose decides what is + * actually fetched, so `DEV=true` with a stable tag offers a dev version and then + * installs the release. Worse, it cannot settle — the next check sees the same dev + * version as newer and offers it again, for ever. + * + * There is a second way to land here even with both set correctly in `backend/.env`: + * compose interpolates `${IMAGE_TAG}` from the project `.env` beside the compose file, + * not from `env_file:`, so the value has to reach the root copy as well. + */ +function channelMismatch(env = process.env) { + if (!devChannelEnabled(env)) return null; + const tag = imageTag(env); + if (tag === 'dev') return null; + return `DEV=true asks for development builds, but IMAGE_TAG is "${tag}", which is what ` + + 'docker compose actually pulls — so a dev version would be offered and the stable ' + + 'image installed instead. Dev versions are being ignored until they agree. Set ' + + 'IMAGE_TAG=dev in backend/.env *and* in the .env beside docker-compose.yml, since ' + + 'compose reads the root copy rather than env_file.'; +} + +/** Whether dev versions should be offered — intent and capability must agree. */ +function shouldOfferDev(env = process.env) { + return devChannelEnabled(env) && channelMismatch(env) === null; +} + +/** + * Say so at boot, because the update modal only appears when there is an update — which + * is precisely what a suppressed dev channel means there is not. + */ +function warnOnChannelMismatch(log = console.warn, env = process.env) { + const problem = channelMismatch(env); + if (problem) log(`[update] ${problem}`); + return problem; +} + /** * Build the docker-run argument list for the self-update helper container. * @@ -267,6 +310,10 @@ module.exports = { compareVersions, isVersionTag, devChannelEnabled, + imageTag, + channelMismatch, + shouldOfferDev, + warnOnChannelMismatch, isNewerVersion, buildUpdateHelperArgs, readComposeLabels, diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index 4502070..0097f22 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -143,7 +143,7 @@ function App() { }, [token, isAdmin]); // Silent update check on admin login - const [updateInfo, setUpdateInfo] = useState<{ current: string; latest: string; message: string; isDocker: boolean } | null>(null); + const [updateInfo, setUpdateInfo] = useState<{ current: string; latest: string; message: string; warning: string; isDocker: boolean } | null>(null); useEffect(() => { if (!token || !isAdmin) return; const skipped = localStorage.getItem('citynet_skipped_version'); @@ -154,9 +154,17 @@ function App() { fetch('/api/version').then(r => r.json()), ]) .then(([updateData, versionData]) => { - if (!updateData.hasUpdate) return; - if (skipped === updateData.latest) return; - setUpdateInfo({ current: updateData.current, latest: updateData.latest, message: updateData.message, isDocker: versionData.isDocker ?? false }); + // A release-channel contradiction is shown even with no update available, since + // suppressing the dev channel is usually the reason there isn't one. + if (!updateData.hasUpdate && !updateData.warning) return; + if (updateData.hasUpdate && skipped === updateData.latest) return; + setUpdateInfo({ + current: updateData.current, + latest: updateData.latest, + message: updateData.message, + warning: updateData.warning ?? '', + isDocker: versionData.isDocker ?? false, + }); }) .catch(() => {}); }, [token, isAdmin]); @@ -1462,6 +1470,7 @@ function App() { current={updateInfo.current} latest={updateInfo.latest} message={updateInfo.message} + warning={updateInfo.warning} token={token} isDocker={updateInfo.isDocker} onDismiss={() => { diff --git a/frontend/src/components/UpdateModal.tsx b/frontend/src/components/UpdateModal.tsx index a0c945b..f0d4bad 100644 --- a/frontend/src/components/UpdateModal.tsx +++ b/frontend/src/components/UpdateModal.tsx @@ -4,13 +4,15 @@ interface Props { current: string; latest: string; message: string; + /** A release-channel contradiction, shown regardless of whether an update exists. */ + warning?: string; token: string; isDocker: boolean; onDismiss: () => void; onSkip: () => void; } -export function UpdateModal({ current, latest, message, token, isDocker, onDismiss, onSkip }: Props) { +export function UpdateModal({ current, latest, message, warning, token, isDocker, onDismiss, onSkip }: Props) { const [phase, setPhase] = useState<'idle' | 'updating' | 'failed' | 'done'>('idle'); const [statusMsg, setStatusMsg] = useState(''); const [detail, setDetail] = useState(''); @@ -212,6 +214,16 @@ export function UpdateModal({ current, latest, message, token, isDocker, onDismi
{message}
+ {warning && ( +
+ {warning} +
+ )}
running: {current} {' → '} From 6828753578f48a959faa5c149ea5b3a0db3c3828 Mon Sep 17 00:00:00 2001 From: Developer Date: Sun, 2 Aug 2026 23:47:14 -0500 Subject: [PATCH 07/16] docs: record the channel-mismatch guard in the changelog The edit was in the previous commit's command but ran from the wrong directory and was silently skipped, so the code landed without it. --- CHANGELOG.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 90f77ef..98a9085 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,7 +15,9 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - **An optional development channel.** Off by default — `DEV=false` in `backend/.env`, and stable releases are all anyone sees unless they ask otherwise. Setting `DEV=true` makes the update check consider `X.Y.Z-dev` builds alongside releases, with a counter accepted but not required, so `1.9.0-dev` and `1.9.0-dev.7` both work and adding counters later is a build-workflow change rather than a code one. A dev build of a newer release is offered to someone on an older release, the release itself supersedes its own dev builds when it lands, and a release user is never dragged back onto a dev build of the same version. - It takes two settings, not one, because they do different jobs: `DEV` decides what is *offered*, and `IMAGE_TAG=dev` decides what is *pulled* — `docker-compose.yml` now reads `${IMAGE_TAG:-latest}` instead of hardcoding `latest`. Setting only `DEV` would offer a dev version and then install the stable one. `.env.example` says so where both are defined. + It takes two settings, not one, because they do different jobs: `DEV` decides what is *offered*, and `IMAGE_TAG=dev` decides what is *pulled* — `docker-compose.yml` now reads `${IMAGE_TAG:-latest}` instead of hardcoding `latest`. Setting only `DEV` would offer a dev version and then install the stable one, and would never settle: the next check sees the same dev version as newer and offers it again. So the two are checked against each other — dev versions are ignored until they agree, the reason is logged at startup, and the update modal shows it even when there is no update, since a suppressed dev channel is usually why there isn't one. + + `IMAGE_TAG` has to reach the `.env` beside `docker-compose.yml` as well as `backend/.env`: compose interpolates from the project file, not from `env_file`, so setting it in only one place silently falls back to `latest`. That is a second route into the same contradiction, and the warning names it. ### Fixed From 4b5b7cd1a88b31d0c19f0e5c4a1789bb29e7e349 Mon Sep 17 00:00:00 2001 From: Developer Date: Sun, 2 Aug 2026 23:51:06 -0500 Subject: [PATCH 08/16] fix(update): guard the reverse channel mismatch too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit IMAGE_TAG=dev with DEV=false was unguarded, and it is the more dangerous of the two directions. DEV=false filters dev tags out of the check, so a stable release is offered — but compose pulls :dev, so whatever that currently points at is installed under the release's name. The version changes and the boot id changes, so the poll reports success. The operator is told they are on 1.9.0 stable and is in fact running 1.10.0-dev.1, with nothing anywhere saying otherwise. The two directions need different remedies, which is why one check was not enough. With DEV=true against a stable tag, a stable offer still installs correctly, so only dev offers are suppressed. Here every offer would be a lie, so updates are suspended entirely until the two agree — and the message names both ways out, since either is a legitimate intention. The three failing combinations are now enumerated in one place rather than inferred, which is what let the reverse case go unnoticed while the forward one was being fixed. --- CHANGELOG.md | 4 +- backend/__tests__/updater.test.js | 27 +++++++++++ backend/routes/admin.js | 6 ++- backend/updater.js | 75 ++++++++++++++++++++++++------- 4 files changed, 93 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 98a9085..97df936 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,7 +15,9 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - **An optional development channel.** Off by default — `DEV=false` in `backend/.env`, and stable releases are all anyone sees unless they ask otherwise. Setting `DEV=true` makes the update check consider `X.Y.Z-dev` builds alongside releases, with a counter accepted but not required, so `1.9.0-dev` and `1.9.0-dev.7` both work and adding counters later is a build-workflow change rather than a code one. A dev build of a newer release is offered to someone on an older release, the release itself supersedes its own dev builds when it lands, and a release user is never dragged back onto a dev build of the same version. - It takes two settings, not one, because they do different jobs: `DEV` decides what is *offered*, and `IMAGE_TAG=dev` decides what is *pulled* — `docker-compose.yml` now reads `${IMAGE_TAG:-latest}` instead of hardcoding `latest`. Setting only `DEV` would offer a dev version and then install the stable one, and would never settle: the next check sees the same dev version as newer and offers it again. So the two are checked against each other — dev versions are ignored until they agree, the reason is logged at startup, and the update modal shows it even when there is no update, since a suppressed dev channel is usually why there isn't one. + It takes two settings, not one, because they do different jobs: `DEV` decides what is *offered*, and `IMAGE_TAG=dev` decides what is *pulled* — `docker-compose.yml` now reads `${IMAGE_TAG:-latest}` instead of hardcoding `latest`. Setting only `DEV` would offer a dev version and then install the stable one, and would never settle: the next check sees the same dev version as newer and offers it again. So the two are checked against each other, and the two directions fail differently. `DEV=true` with a stable tag suppresses dev offers only, since a stable offer still installs correctly. The reverse — `IMAGE_TAG=dev` with `DEV=false` — is quieter and worse: dev tags are filtered out of the check, so a *release* is offered and whatever `:dev` points at is installed under its name, with the version and boot id both changing so the update reports success. Nothing can be offered honestly there, so updates are suspended until the two agree. + + Either way the reason is logged at startup and shown in the modal even when there is no update, since a suppressed channel is usually why there isn't one. `IMAGE_TAG` has to reach the `.env` beside `docker-compose.yml` as well as `backend/.env`: compose interpolates from the project file, not from `env_file`, so setting it in only one place silently falls back to `latest`. That is a second route into the same contradiction, and the warning names it. diff --git a/backend/__tests__/updater.test.js b/backend/__tests__/updater.test.js index 9760d28..2397192 100644 --- a/backend/__tests__/updater.test.js +++ b/backend/__tests__/updater.test.js @@ -172,6 +172,33 @@ describe('channel mismatch', () => { expect(updater.shouldOfferDev({ DEV: 'false', IMAGE_TAG: 'dev' })).toBe(false); }); + it('catches the reverse mismatch, which is the quieter of the two', () => { + // IMAGE_TAG=dev with DEV=false filters dev tags out of the check, so a *release* is + // offered and whatever ":dev" currently points at gets installed under its name. + // Version and boot id both change, so the update reports success — and the operator + // believes they are on stable while running dev. + const problem = updater.channelMismatch({ DEV: 'false', IMAGE_TAG: 'dev' }); + expect(problem).not.toBeNull(); + expect(problem).toMatch(/DEV=true/); + expect(problem).toMatch(/IMAGE_TAG=latest/); + }); + + it('suspends updates entirely when nothing can be offered honestly', () => { + // Suppressing only dev offers is enough the other way round, because a stable offer + // does install correctly from a stable tag. Here every offer would be a lie. + expect(updater.shouldOfferUpdates({ DEV: 'false', IMAGE_TAG: 'dev' })).toBe(false); + expect(updater.shouldOfferUpdates({ DEV: 'true', IMAGE_TAG: 'latest' })).toBe(true); + expect(updater.shouldOfferUpdates({ DEV: 'true', IMAGE_TAG: 'dev' })).toBe(true); + expect(updater.shouldOfferUpdates({ DEV: 'false', IMAGE_TAG: 'latest' })).toBe(true); + }); + + it('names both ways out of the reverse mismatch', () => { + // Either direction is a legitimate intention; the point is only that they agree. + const problem = updater.channelMismatch({ DEV: 'false', IMAGE_TAG: 'dev' }); + expect(problem).toContain('DEV=true to follow dev builds'); + expect(problem).toContain('IMAGE_TAG=latest to return to stable'); + }); + it('warns at boot, where a config error belongs', () => { // The update modal only appears when there is an update, and a suppressed dev // channel is usually exactly why there is not one. diff --git a/backend/routes/admin.js b/backend/routes/admin.js index 921aea7..9d5c17b 100644 --- a/backend/routes/admin.js +++ b/backend/routes/admin.js @@ -251,7 +251,11 @@ module.exports = (db, io, { emitUpdate, recordAction }) => { const latestTag = versionTags[0] || 'unknown'; // Strictly newer, not merely different. Comparing with !== offers a // downgrade whenever the published tag trails the running one. - const hasUpdate = latestTag !== 'unknown' && updater.isNewerVersion(latestTag, currentVersion); + // A contradictory channel installs something other than what is offered, so + // in the direction where nothing can be offered honestly, nothing is. + const hasUpdate = updater.shouldOfferUpdates() + && latestTag !== 'unknown' + && updater.isNewerVersion(latestTag, currentVersion); res.json({ current: currentVersion, diff --git a/backend/updater.js b/backend/updater.js index 928cc39..5081dce 100644 --- a/backend/updater.js +++ b/backend/updater.js @@ -98,31 +98,70 @@ function imageTag(env = process.env) { } /** - * Why `DEV` and `IMAGE_TAG` disagree, or null when they do not. + * How `DEV` and `IMAGE_TAG` relate, and what can safely be offered. * - * `DEV` states an intention and `IMAGE_TAG` is the capability: compose decides what is - * actually fetched, so `DEV=true` with a stable tag offers a dev version and then - * installs the release. Worse, it cannot settle — the next check sees the same dev - * version as newer and offers it again, for ever. + * `DEV` states an intention; `IMAGE_TAG` is the capability, because compose is what + * decides which image is actually fetched. When they disagree the update offers one + * thing and installs another, and the two directions fail differently: * - * There is a second way to land here even with both set correctly in `backend/.env`: - * compose interpolates `${IMAGE_TAG}` from the project `.env` beside the compose file, - * not from `env_file:`, so the value has to reach the root copy as well. + * `DEV=true` with a stable tag offers a dev version and installs the release. It cannot + * settle either — the next check sees the same dev version as newer and offers it + * again, for ever. Stable offers still install correctly, so only dev offers are + * suppressed. + * + * `DEV=false` with `IMAGE_TAG=dev` is worse and quieter. Dev tags are filtered out, so a + * *release* is offered, and pulling installs whatever `:dev` currently points at. The + * version and the boot id both change, so the update reports success — and the operator + * believes they are on stable while running dev. Nothing can be offered honestly here, + * so nothing is. + * + * There is a second way into either state with both values set correctly in + * `backend/.env`: compose interpolates `${IMAGE_TAG}` from the project `.env` beside the + * compose file, not from `env_file:`, so the value has to reach the root copy as well. */ -function channelMismatch(env = process.env) { - if (!devChannelEnabled(env)) return null; +function channelState(env = process.env) { + const dev = devChannelEnabled(env); const tag = imageTag(env); - if (tag === 'dev') return null; - return `DEV=true asks for development builds, but IMAGE_TAG is "${tag}", which is what ` - + 'docker compose actually pulls — so a dev version would be offered and the stable ' - + 'image installed instead. Dev versions are being ignored until they agree. Set ' - + 'IMAGE_TAG=dev in backend/.env *and* in the .env beside docker-compose.yml, since ' - + 'compose reads the root copy rather than env_file.'; + const rootEnvNote = 'Set them in backend/.env *and* in the .env beside docker-compose.yml, ' + + 'since compose reads the root copy rather than env_file.'; + + if (dev && tag !== 'dev') { + return { + problem: `DEV=true asks for development builds, but IMAGE_TAG is "${tag}", which is what ` + + 'docker compose actually pulls — so a dev version would be offered and the stable ' + + `image installed instead. Dev versions are being ignored until they agree. ${rootEnvNote}`, + offerDev: false, + offerAny: true, + }; + } + + if (!dev && tag === 'dev') { + return { + problem: `IMAGE_TAG is "dev", so this deployment pulls development images, but DEV is not ` + + 'true — so a stable release would be offered and a development build installed under ' + + 'its name, with the update reporting success. Updates are suspended until they agree: ' + + `set DEV=true to follow dev builds, or IMAGE_TAG=latest to return to stable. ${rootEnvNote}`, + offerDev: false, + offerAny: false, + }; + } + + return { problem: null, offerDev: dev, offerAny: true }; +} + +/** Why the release channel is contradictory, or null when it is not. */ +function channelMismatch(env = process.env) { + return channelState(env).problem; } /** Whether dev versions should be offered — intent and capability must agree. */ function shouldOfferDev(env = process.env) { - return devChannelEnabled(env) && channelMismatch(env) === null; + return channelState(env).offerDev; +} + +/** Whether any update can be offered honestly at all. */ +function shouldOfferUpdates(env = process.env) { + return channelState(env).offerAny; } /** @@ -312,7 +351,9 @@ module.exports = { devChannelEnabled, imageTag, channelMismatch, + channelState, shouldOfferDev, + shouldOfferUpdates, warnOnChannelMismatch, isNewerVersion, buildUpdateHelperArgs, From 7d4978f4168580b89fc244a267538f4a70feca01 Mon Sep 17 00:00:00 2001 From: Developer Date: Sun, 2 Aug 2026 23:53:27 -0500 Subject: [PATCH 09/16] fix(update): do not claim up to date when nothing was compared MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Suspending updates on a contradictory channel left the panel reporting "You're up to date (1.9.0-dev.7)" beside a red warning saying updates were suspended. Of the two, the reassuring half is the one people read — and it was untrue: nothing had been compared, because nothing could be offered. It now says updates are suspended and the channel needs attention. A wart introduced by the guard two commits ago, found by asking what the previous answer actually looks like on screen rather than in the response body. --- CHANGELOG.md | 2 +- backend/__tests__/admin.test.js | 23 +++++++++++++++++++++++ backend/routes/admin.js | 6 +++++- 3 files changed, 29 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 97df936..d66a295 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,7 +17,7 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). It takes two settings, not one, because they do different jobs: `DEV` decides what is *offered*, and `IMAGE_TAG=dev` decides what is *pulled* — `docker-compose.yml` now reads `${IMAGE_TAG:-latest}` instead of hardcoding `latest`. Setting only `DEV` would offer a dev version and then install the stable one, and would never settle: the next check sees the same dev version as newer and offers it again. So the two are checked against each other, and the two directions fail differently. `DEV=true` with a stable tag suppresses dev offers only, since a stable offer still installs correctly. The reverse — `IMAGE_TAG=dev` with `DEV=false` — is quieter and worse: dev tags are filtered out of the check, so a *release* is offered and whatever `:dev` points at is installed under its name, with the version and boot id both changing so the update reports success. Nothing can be offered honestly there, so updates are suspended until the two agree. - Either way the reason is logged at startup and shown in the modal even when there is no update, since a suppressed channel is usually why there isn't one. + Either way the reason is logged at startup and shown in the modal even when there is no update, since a suppressed channel is usually why there isn't one — and with updates suspended the panel says so rather than reporting "you're up to date", which would be a claim rather than a fact when nothing is being compared. `IMAGE_TAG` has to reach the `.env` beside `docker-compose.yml` as well as `backend/.env`: compose interpolates from the project file, not from `env_file`, so setting it in only one place silently falls back to `latest`. That is a second route into the same contradiction, and the warning names it. diff --git a/backend/__tests__/admin.test.js b/backend/__tests__/admin.test.js index 55bbd37..04a2e7b 100644 --- a/backend/__tests__/admin.test.js +++ b/backend/__tests__/admin.test.js @@ -293,6 +293,29 @@ describe('update routes', () => { expect(res.status).toBe(401); }); + it('POST /check-update does not claim you are up to date when it cannot tell', async () => { + // With a contradictory channel nothing is compared, because nothing can be offered. + // Reporting "up to date" beside a warning that updates are suspended is a + // contradiction, and the reassuring half is the one people read. + const prev = { DEV: process.env.DEV, IMAGE_TAG: process.env.IMAGE_TAG }; + process.env.DEV = 'false'; + process.env.IMAGE_TAG = 'dev'; + try { + const res = await request(app) + .post('/api/admin/check-update') + .set('Authorization', `Bearer ${ADMIN_TOKEN}`); + // Reaching Docker Hub is not this test's business; a network failure is fine. + if (res.status === 200) { + expect(res.body.hasUpdate).toBe(false); + expect(res.body.message).toMatch(/suspended/i); + expect(res.body.warning).toBeTruthy(); + } + } finally { + process.env.DEV = prev.DEV; + process.env.IMAGE_TAG = prev.IMAGE_TAG; + } + }); + it('POST /update refuses with no token at all', async () => { const res = await request(app).post('/api/admin/update'); expect(res.status).toBe(401); diff --git a/backend/routes/admin.js b/backend/routes/admin.js index 9d5c17b..88e39a2 100644 --- a/backend/routes/admin.js +++ b/backend/routes/admin.js @@ -261,9 +261,13 @@ module.exports = (db, io, { emitUpdate, recordAction }) => { current: currentVersion, latest: latestTag, hasUpdate, + // "Up to date" would be a claim, not a fact, when the channel is + // contradictory: nothing is being compared because nothing can be offered. message: hasUpdate ? `Update available: ${currentVersion} → ${latestTag}` - : `You're up to date (${currentVersion})`, + : updater.shouldOfferUpdates() + ? `You're up to date (${currentVersion})` + : `Updates suspended — release channel needs attention (${currentVersion})`, // Surfaced even when there is no update, since a suppressed dev channel is // usually why there isn't one. warning: updater.channelMismatch(), From 5820c9bd8f3fd6080cad97db7ec6d43946245435 Mon Sep 17 00:00:00 2001 From: Developer Date: Mon, 3 Aug 2026 00:02:22 -0500 Subject: [PATCH 10/16] refactor(update): one setting selects the release channel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DEV and IMAGE_TAG said the same thing, and two settings saying one thing produced three contradictory states — each of which I then wrote a guard for. Offering a dev version and installing stable. Offering a release and installing dev under its name, with the update reporting success. A nag loop that could never settle because the thing offered was never the thing installed. None of them is expressible now. IMAGE_TAG is what compose interpolates to decide which images are pulled, so it is already the authority on what a deployment is; the check reads that rather than a separate declaration of intent. DEV is gone, and with it channelState, channelMismatch, shouldOfferDev, shouldOfferUpdates, warnOnChannelMismatch, the boot warning, the warning plumbed through App.tsx into the modal, and the "updates suspended" message — about a hundred lines whose only job was to defend a contradiction that should not have existed. This is also the conventional shape: a single variable naming the tag, as Immich, Paperless, Nextcloud and the LinuxServer images all do. The alternative I was part-way through — the app injecting IMAGE_TAG into the compose child process so DEV could stay authoritative — would have made `docker compose up -d` by hand and the in-app update pull different images, which is the same class of bug in a new place. Tests follow the code: the mismatch suite is deleted, the channel suite is rewritten around the one setting, and a guard asserts .env.example does not reintroduce a second switch. A pinned version tag now counts as stable, which is new and correct — pinning 1.8.1 is not a dev channel. --- CHANGELOG.md | 8 +- README.md | 4 +- backend/.env.example | 24 +++--- backend/__tests__/admin.test.js | 23 ----- backend/__tests__/docker_config.test.js | 17 ++-- backend/__tests__/updater.test.js | 108 ++++-------------------- backend/routes/admin.js | 17 +--- backend/server.js | 3 - backend/updater.js | 100 ++++------------------ frontend/src/App.tsx | 10 +-- frontend/src/components/UpdateModal.tsx | 14 +-- 11 files changed, 67 insertions(+), 261 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d66a295..e1ef759 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,13 +13,11 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ### Added -- **An optional development channel.** Off by default — `DEV=false` in `backend/.env`, and stable releases are all anyone sees unless they ask otherwise. Setting `DEV=true` makes the update check consider `X.Y.Z-dev` builds alongside releases, with a counter accepted but not required, so `1.9.0-dev` and `1.9.0-dev.7` both work and adding counters later is a build-workflow change rather than a code one. A dev build of a newer release is offered to someone on an older release, the release itself supersedes its own dev builds when it lands, and a release user is never dragged back onto a dev build of the same version. +- **An optional development channel, selected by one setting.** `IMAGE_TAG=latest` by default, which is stable; `IMAGE_TAG=dev` follows development builds. That is the same variable `docker-compose.yml` interpolates to decide which images are pulled, so what the update check offers and what it installs cannot disagree — the check reads the deployment's own tag rather than a separate declaration of intent. - It takes two settings, not one, because they do different jobs: `DEV` decides what is *offered*, and `IMAGE_TAG=dev` decides what is *pulled* — `docker-compose.yml` now reads `${IMAGE_TAG:-latest}` instead of hardcoding `latest`. Setting only `DEV` would offer a dev version and then install the stable one, and would never settle: the next check sees the same dev version as newer and offers it again. So the two are checked against each other, and the two directions fail differently. `DEV=true` with a stable tag suppresses dev offers only, since a stable offer still installs correctly. The reverse — `IMAGE_TAG=dev` with `DEV=false` — is quieter and worse: dev tags are filtered out of the check, so a *release* is offered and whatever `:dev` points at is installed under its name, with the version and boot id both changing so the update reports success. Nothing can be offered honestly there, so updates are suspended until the two agree. + Development builds are tagged `X.Y.Z-dev`, with a counter accepted but not required, so `1.9.0-dev` and `1.9.0-dev.7` both work and introducing counters later is a build-workflow change rather than a code one. A dev build of a newer release is offered to someone on an older release, the release itself supersedes its own dev builds when it lands, and a release user is never dragged back onto a dev build of the same version. A pinned version tag counts as stable, since pinning is not a channel. - Either way the reason is logged at startup and shown in the modal even when there is no update, since a suppressed channel is usually why there isn't one — and with updates suspended the panel says so rather than reporting "you're up to date", which would be a claim rather than a fact when nothing is being compared. - - `IMAGE_TAG` has to reach the `.env` beside `docker-compose.yml` as well as `backend/.env`: compose interpolates from the project file, not from `env_file`, so setting it in only one place silently falls back to `latest`. That is a second route into the same contradiction, and the warning names it. + `IMAGE_TAG` must also reach the `.env` beside `docker-compose.yml`, which the setup steps already cover by copying `backend/.env` to the project root: compose interpolates from the project file rather than from `env_file`. ### Fixed diff --git a/README.md b/README.md index 92dfb0e..baa3b48 100644 --- a/README.md +++ b/README.md @@ -323,7 +323,7 @@ CITY_NET/ ├── backend/ │ ├── server.js # Express entrypoint — mounts routes, starts Socket.IO │ ├── db.js # SQLite schema and migrations -│ ├── updater.js # In-app self-update — release channels (DEV=false by default; X.Y.Z-dev tags with an optional counter, ordered so a release supersedes its own dev builds); preflight (compose file mounted, docker socket, compose project labels) so a stack that cannot update says why instead of hanging; upgrade-only semver check; update log on the data volume; boot id so a restart is detectable without a version change +│ ├── updater.js # In-app self-update — release channels selected by IMAGE_TAG alone, the same variable compose pulls with (X.Y.Z-dev tags with an optional counter, ordered so a release supersedes its own dev builds); preflight (compose file mounted, docker socket, compose project labels) so a stack that cannot update says why instead of hanging; upgrade-only semver check; update log on the data volume; boot id so a restart is detectable without a version change │ ├── middleware/ │ │ └── auth.js # JWT verify middleware (admin + elevated users) │ ├── routes/ @@ -365,7 +365,7 @@ CITY_NET/ │ │ └── testDb.js # In-memory SQLite factory for isolated test DBs │ ├── admin.test.js # Admin endpoints (auth, settings, undo access); update routes — 409 with a reason rather than a false success, unauthenticated status, boot id on /version │ ├── updater.test.js # Version ordering including X.Y.Z-dev, tag filtering per channel, preflight refusals, and an update that records its failures instead of returning silently -│ ├── docker_config.test.js # Deployment invariants — DB_PATH baked in, data excluded from the image, image tags parameterised by IMAGE_TAG, compose file mounted for the updater, dev channel shipped off +│ ├── docker_config.test.js # Deployment invariants — DB_PATH baked in, data excluded from the image, image tags parameterised by IMAGE_TAG, compose file mounted for the updater, channel shipped pointing at stable │ ├── battle_maps.test.js # Battle map upload/list/delete │ ├── locations.test.js # Location CRUD and classification │ ├── locations.global.test.js # Custom structure global persistence tests diff --git a/backend/.env.example b/backend/.env.example index cfcac2b..85aabb8 100644 --- a/backend/.env.example +++ b/backend/.env.example @@ -18,21 +18,17 @@ DUCKDNS_TOKEN=your-duckdns-token # Timezone for DuckDNS container (e.g. America/New_York) TZ=America/Chicago -# ── Development builds (optional) ──────────────────────────────────────────── -# Off by default. Stable releases only, which is what almost everyone wants. +# ── Release channel (optional) ─────────────────────────────────────────────── +# Which images this deployment runs. Stable unless you change it. # -# Set BOTH to follow development builds — they do different jobs and must agree: -# DEV=true the update check offers X.Y.Z-dev versions as well as releases -# IMAGE_TAG=dev docker compose actually pulls the dev images +# IMAGE_TAG=latest stable releases (default) +# IMAGE_TAG=dev development builds — unreleased, and they may break # -# DEV alone would offer you a dev version and then pull the stable one, since the -# compose file decides what is fetched — so the app ignores dev versions until the two -# agree, and says so in the server log at startup. +# One setting, because it is one decision: docker compose pulls this tag, and the +# update check offers versions from the same channel. Development builds are tagged +# X.Y.Z-dev and are only ever offered when this is set to dev. # -# IMAGE_TAG must also reach the .env beside docker-compose.yml: compose interpolates -# ${IMAGE_TAG} from that file, not from env_file. Re-copy after changing it, exactly as -# the setup steps describe for the DuckDNS values. -# -# Dev builds are unreleased and may break. -DEV=false +# It must also be present in the .env beside docker-compose.yml — compose interpolates +# ${IMAGE_TAG} from that file rather than from env_file — which the setup steps already +# cover by copying backend/.env to the project root. IMAGE_TAG=latest diff --git a/backend/__tests__/admin.test.js b/backend/__tests__/admin.test.js index 04a2e7b..55bbd37 100644 --- a/backend/__tests__/admin.test.js +++ b/backend/__tests__/admin.test.js @@ -293,29 +293,6 @@ describe('update routes', () => { expect(res.status).toBe(401); }); - it('POST /check-update does not claim you are up to date when it cannot tell', async () => { - // With a contradictory channel nothing is compared, because nothing can be offered. - // Reporting "up to date" beside a warning that updates are suspended is a - // contradiction, and the reassuring half is the one people read. - const prev = { DEV: process.env.DEV, IMAGE_TAG: process.env.IMAGE_TAG }; - process.env.DEV = 'false'; - process.env.IMAGE_TAG = 'dev'; - try { - const res = await request(app) - .post('/api/admin/check-update') - .set('Authorization', `Bearer ${ADMIN_TOKEN}`); - // Reaching Docker Hub is not this test's business; a network failure is fine. - if (res.status === 200) { - expect(res.body.hasUpdate).toBe(false); - expect(res.body.message).toMatch(/suspended/i); - expect(res.body.warning).toBeTruthy(); - } - } finally { - process.env.DEV = prev.DEV; - process.env.IMAGE_TAG = prev.IMAGE_TAG; - } - }); - it('POST /update refuses with no token at all', async () => { const res = await request(app).post('/api/admin/update'); expect(res.status).toBe(401); diff --git a/backend/__tests__/docker_config.test.js b/backend/__tests__/docker_config.test.js index 8999c4d..1be691f 100644 --- a/backend/__tests__/docker_config.test.js +++ b/backend/__tests__/docker_config.test.js @@ -90,14 +90,19 @@ describe('docker-compose.yml release channel', () => { describe('.env.example release channel', () => { const env = () => readRoot('backend/.env.example'); - it('ships the dev channel switched off', () => { + it('ships pointed at stable', () => { // Dev builds are unreleased code; nobody should arrive on one by default. - expect(env()).toMatch(/^DEV=false$/m); + expect(env()).toMatch(/^IMAGE_TAG=latest$/m); }); - it('documents both settings, since one without the other misleads', () => { - const text = env(); - expect(text).toMatch(/^IMAGE_TAG=latest$/m); - expect(text).toMatch(/IMAGE_TAG=dev/); + it('documents the dev value on the same setting', () => { + // One setting, because it is one decision. A second one alongside it produced three + // contradictory states — offering a dev version and installing stable, offering a + // release and installing dev under its name, and a nag loop that never settled. + expect(env()).toMatch(/IMAGE_TAG=dev/); + }); + + it('does not reintroduce a second channel switch', () => { + expect(env()).not.toMatch(/^DEV=/m); }); }); diff --git a/backend/__tests__/updater.test.js b/backend/__tests__/updater.test.js index 2397192..0c600a5 100644 --- a/backend/__tests__/updater.test.js +++ b/backend/__tests__/updater.test.js @@ -55,20 +55,25 @@ describe('isNewerVersion', () => { }); }); -describe('dev channel', () => { - it('is off unless explicitly turned on', () => { - // A dev build is unreleased code. Nobody should land on one by leaving a value out. - expect(updater.devChannelEnabled({})).toBe(false); - expect(updater.devChannelEnabled({ DEV: 'false' })).toBe(false); - expect(updater.devChannelEnabled({ DEV: '' })).toBe(false); - expect(updater.devChannelEnabled({ DEV: '1' })).toBe(false); - expect(updater.devChannelEnabled({ DEV: 'yes' })).toBe(false); +describe('release channel', () => { + it('is stable unless the operator points it elsewhere', () => { + // Dev builds are unreleased code. Nobody should arrive on one by omission. + expect(updater.imageTag({})).toBe('latest'); + expect(updater.imageTag({ IMAGE_TAG: '' })).toBe('latest'); + expect(updater.allowsDevBuilds({})).toBe(false); + expect(updater.allowsDevBuilds({ IMAGE_TAG: 'latest' })).toBe(false); }); - it('turns on for true, whatever the casing or padding', () => { - expect(updater.devChannelEnabled({ DEV: 'true' })).toBe(true); - expect(updater.devChannelEnabled({ DEV: 'TRUE' })).toBe(true); - expect(updater.devChannelEnabled({ DEV: ' true ' })).toBe(true); + it('follows dev builds only when pointed at the dev images', () => { + // The same setting compose resolves, so what is offered and what is installed + // cannot disagree. A separate boolean alongside this produced three contradictory + // states, each needing a guard; none of them is expressible now. + expect(updater.allowsDevBuilds({ IMAGE_TAG: 'dev' })).toBe(true); + }); + + it('treats a pinned version tag as stable', () => { + // Pinning 1.8.1 is a legitimate thing to do and is not a dev channel. + expect(updater.allowsDevBuilds({ IMAGE_TAG: '1.8.1' })).toBe(false); }); it('hides dev tags from the stable channel', () => { @@ -136,85 +141,6 @@ describe('dev version ordering', () => { }); }); -describe('channel mismatch', () => { - it('is silent when the channel is consistent', () => { - expect(updater.channelMismatch({ DEV: 'false', IMAGE_TAG: 'latest' })).toBeNull(); - expect(updater.channelMismatch({ DEV: 'true', IMAGE_TAG: 'dev' })).toBeNull(); - expect(updater.channelMismatch({})).toBeNull(); - }); - - it('catches DEV=true against a stable image tag', () => { - // The state a user can easily land in: told about 1.9.0-dev, handed the stable - // image, and offered the same dev version again on the next check for ever, since - // compose is what decides which image is actually pulled. - const problem = updater.channelMismatch({ DEV: 'true', IMAGE_TAG: 'latest' }); - expect(problem).toContain('IMAGE_TAG'); - expect(problem).toContain('latest'); - }); - - it('catches DEV=true with IMAGE_TAG left unset, which defaults to latest', () => { - expect(updater.channelMismatch({ DEV: 'true' })).not.toBeNull(); - }); - - it('mentions the root .env, which is what compose actually reads', () => { - // Setting IMAGE_TAG only in backend/.env is the second way into this state: - // compose interpolates from the file beside docker-compose.yml, not from env_file. - const problem = updater.channelMismatch({ DEV: 'true', IMAGE_TAG: 'latest' }); - expect(problem).toMatch(/docker-compose\.yml/); - }); - - it('stops offering dev versions until intent and capability agree', () => { - // Capability wins over intent. Offering something that cannot be installed is worse - // than not offering it, because it never resolves. - expect(updater.shouldOfferDev({ DEV: 'true', IMAGE_TAG: 'dev' })).toBe(true); - expect(updater.shouldOfferDev({ DEV: 'true', IMAGE_TAG: 'latest' })).toBe(false); - expect(updater.shouldOfferDev({ DEV: 'true' })).toBe(false); - expect(updater.shouldOfferDev({ DEV: 'false', IMAGE_TAG: 'dev' })).toBe(false); - }); - - it('catches the reverse mismatch, which is the quieter of the two', () => { - // IMAGE_TAG=dev with DEV=false filters dev tags out of the check, so a *release* is - // offered and whatever ":dev" currently points at gets installed under its name. - // Version and boot id both change, so the update reports success — and the operator - // believes they are on stable while running dev. - const problem = updater.channelMismatch({ DEV: 'false', IMAGE_TAG: 'dev' }); - expect(problem).not.toBeNull(); - expect(problem).toMatch(/DEV=true/); - expect(problem).toMatch(/IMAGE_TAG=latest/); - }); - - it('suspends updates entirely when nothing can be offered honestly', () => { - // Suppressing only dev offers is enough the other way round, because a stable offer - // does install correctly from a stable tag. Here every offer would be a lie. - expect(updater.shouldOfferUpdates({ DEV: 'false', IMAGE_TAG: 'dev' })).toBe(false); - expect(updater.shouldOfferUpdates({ DEV: 'true', IMAGE_TAG: 'latest' })).toBe(true); - expect(updater.shouldOfferUpdates({ DEV: 'true', IMAGE_TAG: 'dev' })).toBe(true); - expect(updater.shouldOfferUpdates({ DEV: 'false', IMAGE_TAG: 'latest' })).toBe(true); - }); - - it('names both ways out of the reverse mismatch', () => { - // Either direction is a legitimate intention; the point is only that they agree. - const problem = updater.channelMismatch({ DEV: 'false', IMAGE_TAG: 'dev' }); - expect(problem).toContain('DEV=true to follow dev builds'); - expect(problem).toContain('IMAGE_TAG=latest to return to stable'); - }); - - it('warns at boot, where a config error belongs', () => { - // The update modal only appears when there is an update, and a suppressed dev - // channel is usually exactly why there is not one. - const logged = []; - updater.warnOnChannelMismatch((m) => logged.push(m), { DEV: 'true', IMAGE_TAG: 'latest' }); - expect(logged).toHaveLength(1); - expect(logged[0]).toContain('[update]'); - }); - - it('says nothing at boot when the channel is consistent', () => { - const logged = []; - updater.warnOnChannelMismatch((m) => logged.push(m), { DEV: 'true', IMAGE_TAG: 'dev' }); - expect(logged).toHaveLength(0); - }); -}); - describe('preflight', () => { const labels = { projectName: 'citynet', configFile: '/srv/citynet/docker-compose.yml', workingDir: '/srv/citynet' }; const allPresent = () => true; diff --git a/backend/routes/admin.js b/backend/routes/admin.js index 88e39a2..a99677a 100644 --- a/backend/routes/admin.js +++ b/backend/routes/admin.js @@ -243,7 +243,7 @@ module.exports = (db, io, { emitUpdate, recordAction }) => { // parsed to NaN, which made the comparator return NaN and left the sort // order undefined — one dev tag on the registry could stop stable users // being told about releases at all. - const allowDev = updater.shouldOfferDev(); + const allowDev = updater.allowsDevBuilds(); const versionTags = data.results ?.filter(tag => updater.isVersionTag(tag.name, allowDev)) .map(tag => tag.name) @@ -251,26 +251,15 @@ module.exports = (db, io, { emitUpdate, recordAction }) => { const latestTag = versionTags[0] || 'unknown'; // Strictly newer, not merely different. Comparing with !== offers a // downgrade whenever the published tag trails the running one. - // A contradictory channel installs something other than what is offered, so - // in the direction where nothing can be offered honestly, nothing is. - const hasUpdate = updater.shouldOfferUpdates() - && latestTag !== 'unknown' - && updater.isNewerVersion(latestTag, currentVersion); + const hasUpdate = latestTag !== 'unknown' && updater.isNewerVersion(latestTag, currentVersion); res.json({ current: currentVersion, latest: latestTag, hasUpdate, - // "Up to date" would be a claim, not a fact, when the channel is - // contradictory: nothing is being compared because nothing can be offered. message: hasUpdate ? `Update available: ${currentVersion} → ${latestTag}` - : updater.shouldOfferUpdates() - ? `You're up to date (${currentVersion})` - : `Updates suspended — release channel needs attention (${currentVersion})`, - // Surfaced even when there is no update, since a suppressed dev channel is - // usually why there isn't one. - warning: updater.channelMismatch(), + : `You're up to date (${currentVersion})`, }); } catch (e) { res.status(500).json({ error: 'Failed to parse Docker Hub response' }); diff --git a/backend/server.js b/backend/server.js index f985abc..1ddd599 100644 --- a/backend/server.js +++ b/backend/server.js @@ -61,9 +61,6 @@ require('./sockets')(io, db, { elevatedUsers, ...helpers }); server.listen(PORT, '0.0.0.0', () => { console.log(`Server running on port ${PORT}`); - // A contradictory release channel is a config error, and the log is where a config - // error belongs — the update modal only shows when there is an update to show. - require('./updater').warnOnChannelMismatch(); if (process.env.ADMIN_PASS === 'cyberpunk_password' || !process.env.ADMIN_PASS) { console.warn('\x1b[33m⚠️ WARNING: Default admin password in use. Set ADMIN_PASS in your .env file.\x1b[0m'); } diff --git a/backend/updater.js b/backend/updater.js index 5081dce..28cfeb8 100644 --- a/backend/updater.js +++ b/backend/updater.js @@ -87,91 +87,30 @@ function isNewerVersion(candidate, current) { return compareVersions(a, b) > 0; } -/** Whether this deployment has opted into dev builds. Off unless explicitly on. */ -function devChannelEnabled(env = process.env) { - return String(env.DEV ?? '').trim().toLowerCase() === 'true'; -} - -/** The image tag this deployment actually pulls. */ -function imageTag(env = process.env) { - return String(env.IMAGE_TAG ?? '').trim() || 'latest'; -} - /** - * How `DEV` and `IMAGE_TAG` relate, and what can safely be offered. - * - * `DEV` states an intention; `IMAGE_TAG` is the capability, because compose is what - * decides which image is actually fetched. When they disagree the update offers one - * thing and installs another, and the two directions fail differently: + * The image tag this deployment runs, and the only thing that selects a channel. * - * `DEV=true` with a stable tag offers a dev version and installs the release. It cannot - * settle either — the next check sees the same dev version as newer and offers it - * again, for ever. Stable offers still install correctly, so only dev offers are - * suppressed. + * Compose resolves `${IMAGE_TAG:-latest}` to decide which image is pulled, so this is + * already the authority on what a deployment *is* — and anything else claiming to + * select a channel can only agree with it or contradict it. * - * `DEV=false` with `IMAGE_TAG=dev` is worse and quieter. Dev tags are filtered out, so a - * *release* is offered, and pulling installs whatever `:dev` currently points at. The - * version and the boot id both change, so the update reports success — and the operator - * believes they are on stable while running dev. Nothing can be offered honestly here, - * so nothing is. - * - * There is a second way into either state with both values set correctly in - * `backend/.env`: compose interpolates `${IMAGE_TAG}` from the project `.env` beside the - * compose file, not from `env_file:`, so the value has to reach the root copy as well. + * An earlier version had a separate `DEV` boolean alongside it. Two settings saying the + * same thing produced three contradictory states, each needing its own guard: offering + * a dev version and installing stable, offering a release and installing dev under its + * name, and a nag loop that could never settle. None of them is expressible now. */ -function channelState(env = process.env) { - const dev = devChannelEnabled(env); - const tag = imageTag(env); - const rootEnvNote = 'Set them in backend/.env *and* in the .env beside docker-compose.yml, ' - + 'since compose reads the root copy rather than env_file.'; - - if (dev && tag !== 'dev') { - return { - problem: `DEV=true asks for development builds, but IMAGE_TAG is "${tag}", which is what ` - + 'docker compose actually pulls — so a dev version would be offered and the stable ' - + `image installed instead. Dev versions are being ignored until they agree. ${rootEnvNote}`, - offerDev: false, - offerAny: true, - }; - } - - if (!dev && tag === 'dev') { - return { - problem: `IMAGE_TAG is "dev", so this deployment pulls development images, but DEV is not ` - + 'true — so a stable release would be offered and a development build installed under ' - + 'its name, with the update reporting success. Updates are suspended until they agree: ' - + `set DEV=true to follow dev builds, or IMAGE_TAG=latest to return to stable. ${rootEnvNote}`, - offerDev: false, - offerAny: false, - }; - } - - return { problem: null, offerDev: dev, offerAny: true }; -} - -/** Why the release channel is contradictory, or null when it is not. */ -function channelMismatch(env = process.env) { - return channelState(env).problem; -} - -/** Whether dev versions should be offered — intent and capability must agree. */ -function shouldOfferDev(env = process.env) { - return channelState(env).offerDev; -} - -/** Whether any update can be offered honestly at all. */ -function shouldOfferUpdates(env = process.env) { - return channelState(env).offerAny; +function imageTag(env = process.env) { + return String(env.IMAGE_TAG ?? '').trim() || 'latest'; } /** - * Say so at boot, because the update modal only appears when there is an update — which - * is precisely what a suppressed dev channel means there is not. + * Whether this deployment follows development builds. + * + * Stable unless the operator has deliberately pointed it at the dev images, which is + * the same decision as which images get pulled, made once. */ -function warnOnChannelMismatch(log = console.warn, env = process.env) { - const problem = channelMismatch(env); - if (problem) log(`[update] ${problem}`); - return problem; +function allowsDevBuilds(env = process.env) { + return imageTag(env) === 'dev'; } /** @@ -348,13 +287,8 @@ module.exports = { parseVersion, compareVersions, isVersionTag, - devChannelEnabled, imageTag, - channelMismatch, - channelState, - shouldOfferDev, - shouldOfferUpdates, - warnOnChannelMismatch, + allowsDevBuilds, isNewerVersion, buildUpdateHelperArgs, readComposeLabels, diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index 0097f22..aaedcf9 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -143,7 +143,7 @@ function App() { }, [token, isAdmin]); // Silent update check on admin login - const [updateInfo, setUpdateInfo] = useState<{ current: string; latest: string; message: string; warning: string; isDocker: boolean } | null>(null); + const [updateInfo, setUpdateInfo] = useState<{ current: string; latest: string; message: string; isDocker: boolean } | null>(null); useEffect(() => { if (!token || !isAdmin) return; const skipped = localStorage.getItem('citynet_skipped_version'); @@ -154,15 +154,12 @@ function App() { fetch('/api/version').then(r => r.json()), ]) .then(([updateData, versionData]) => { - // A release-channel contradiction is shown even with no update available, since - // suppressing the dev channel is usually the reason there isn't one. - if (!updateData.hasUpdate && !updateData.warning) return; - if (updateData.hasUpdate && skipped === updateData.latest) return; + if (!updateData.hasUpdate) return; + if (skipped === updateData.latest) return; setUpdateInfo({ current: updateData.current, latest: updateData.latest, message: updateData.message, - warning: updateData.warning ?? '', isDocker: versionData.isDocker ?? false, }); }) @@ -1470,7 +1467,6 @@ function App() { current={updateInfo.current} latest={updateInfo.latest} message={updateInfo.message} - warning={updateInfo.warning} token={token} isDocker={updateInfo.isDocker} onDismiss={() => { diff --git a/frontend/src/components/UpdateModal.tsx b/frontend/src/components/UpdateModal.tsx index f0d4bad..a0c945b 100644 --- a/frontend/src/components/UpdateModal.tsx +++ b/frontend/src/components/UpdateModal.tsx @@ -4,15 +4,13 @@ interface Props { current: string; latest: string; message: string; - /** A release-channel contradiction, shown regardless of whether an update exists. */ - warning?: string; token: string; isDocker: boolean; onDismiss: () => void; onSkip: () => void; } -export function UpdateModal({ current, latest, message, warning, token, isDocker, onDismiss, onSkip }: Props) { +export function UpdateModal({ current, latest, message, token, isDocker, onDismiss, onSkip }: Props) { const [phase, setPhase] = useState<'idle' | 'updating' | 'failed' | 'done'>('idle'); const [statusMsg, setStatusMsg] = useState(''); const [detail, setDetail] = useState(''); @@ -214,16 +212,6 @@ export function UpdateModal({ current, latest, message, warning, token, isDocker
{message}
- {warning && ( -
- {warning} -
- )}
running: {current} {' → '} From c7b4d1cbd7887e1e4faa699f619aa924f92b8e14 Mon Sep 17 00:00:00 2001 From: Developer Date: Mon, 3 Aug 2026 00:12:13 -0500 Subject: [PATCH 11/16] docs: document IMAGE_TAG and the compose-file caveat in UPGRADE.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The env-var section stopped at 1.2.3, so a release adding a setting had nowhere to say so. IMAGE_TAG is optional and existing installs need to change nothing — absent resolves to latest, which is what they already run — but that is worth stating plainly rather than leaving to be inferred. Also records something the guide never said: the in-app update pulls images, not repository files. docker-compose.yml is where the tag became a variable, so the channel is unavailable until a git pull, even on an otherwise fully updated instance. Nothing breaks meanwhile; the capability is simply absent, which is a confusing thing to hit undocumented. And a line on the updater now reporting failures, with the log path, since the section previously promised it "will pull, restart and reload" with no acknowledgement that it might not. --- UPGRADE.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/UPGRADE.md b/UPGRADE.md index 8df2562..90b321a 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -10,6 +10,10 @@ How to update CITY_NET to the latest version. Log in as admin, open the nav panel, and click **CLICK TO UPDATE (docker only)**. The app will pull the latest image, restart all containers, and reload automatically. +If it cannot update, it now says why rather than waiting — the commonest reason being a container started before `docker-compose.yml` mounted itself into the backend, which is to say a long-running one. Recreating the stack once with the manual steps below fixes that permanently. Details of any failure are appended to `backend/data/update.log`. + +**The in-app update pulls images, not files.** `docker-compose.yml`, `nginx.conf` and the rest come from the repository, so a release that changes one of them needs a `git pull` as well. Everything keeps working without it; only the new capability is missing. + ### Manual Docker update ```bash @@ -43,6 +47,15 @@ pm2 restart citynet-backend ## Environment variable changes by version +### [1.8.1] +- **`IMAGE_TAG`** — Optional, defaults to `latest`. Selects the release channel: `latest` for stable releases, `dev` for development builds, or a pinned version such as `1.8.1`. + + **Existing installs need to change nothing.** An absent `IMAGE_TAG` resolves to `latest`, which is the behaviour you already have. + + If you do set it, put it in the `.env` beside `docker-compose.yml` as well as `backend/.env` — compose interpolates it from the project file rather than from `env_file`, and the setup steps already cover this by copying one to the other. It also requires the `docker-compose.yml` from 1.8.1 or later, since that is where the tag became a variable; see the note above about the in-app update not updating repository files. + + Development builds are unreleased and may break. They are only ever offered when `IMAGE_TAG=dev`. + ### [1.2.3] No new required vars. `WATCHTOWER_API_TOKEN` is no longer required — you can remove it from your `.env` if present. From 0bbec0d2c7fb45194f6a993a630ef702be1636d1 Mon Sep 17 00:00:00 2001 From: Developer Date: Mon, 3 Aug 2026 00:16:06 -0500 Subject: [PATCH 12/16] test: cover the check-update route, where the faults actually shipped MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit check-update had no test anywhere, and it is the route that carried three of this branch's four bugs: an unanchored tag filter, a comparator returning NaN, and a "different" test that counted a downgrade as an update. Each is tested in updater.js — but the module was correct in isolation the whole time, and it was the route's use of it that shipped broken. Same gap I closed for /update and left open here. Seven tests against a stubbed registry: a newer release is offered, the 1.8.0 → 1.7.4 downgrade is not, dev builds are ignored on the stable channel and offered on the dev one, a dev deployment is carried onto the release when it lands, and a registry with no version tags offers nothing. The one that matters most asserts a prerelease on the registry does not hide a stable release — the fault that would have appeared on the first dev tag published and would have hurt stable users, not dev ones. Verified by restoring the old filter, sort and comparison and re-running: three go red. Without that check they would only have proved the current code agrees with itself. --- README.md | 4 +- backend/__tests__/admin.test.js | 126 ++++++++++++++++++++++++++++++++ 2 files changed, 128 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index baa3b48..e71f402 100644 --- a/README.md +++ b/README.md @@ -327,7 +327,7 @@ CITY_NET/ │ ├── middleware/ │ │ └── auth.js # JWT verify middleware (admin + elevated users) │ ├── routes/ -│ │ ├── admin.js # Admin-only REST endpoints; undo covers locations, roads, signs; POST /update preflights and returns 409 naming what is missing, GET /update/status reports phase, error and log tail; POST /water marks generated water so a regenerate can clear its own river without touching a lake the GM drew +│ │ ├── admin.js # Admin-only REST endpoints; undo covers locations, roads, signs; POST /update preflights and returns 409 naming what is missing, GET /update/status reports phase, error and log tail, POST /check-update offers only genuine upgrades from the deployment's own channel; POST /water marks generated water so a regenerate can clear its own river without touching a lake the GM drew │ │ ├── locations.js # Location CRUD; JOIN→CUSTOM classification upserts roots + child parts to custom_structure_library; serves GET /custom-library (CUSTOM-only); GET / includes sheet_data for NPC initiative rolls; POST /purge-region clears one region's generated content in a single transaction, keeping GM-named structures, tokens, battle-map content and hand-drawn water │ │ ├── battle_maps.js # Battle map image upload/management │ │ ├── maps.js # Saved map snapshots (locations, districts, roads, overpasses, water bodies); preserves only rhombus tokens on load/clear; records active_map_name in global_settings so exports can name their files @@ -363,7 +363,7 @@ CITY_NET/ │ └── __tests__/ │ ├── helpers/ │ │ └── testDb.js # In-memory SQLite factory for isolated test DBs -│ ├── admin.test.js # Admin endpoints (auth, settings, undo access); update routes — 409 with a reason rather than a false success, unauthenticated status, boot id on /version +│ ├── admin.test.js # Admin endpoints (auth, settings, undo access); update routes — 409 with a reason rather than a false success, unauthenticated status, boot id on /version; check-update against a stubbed registry — upgrades only, dev tags per channel, and a prerelease not hiding a stable release │ ├── updater.test.js # Version ordering including X.Y.Z-dev, tag filtering per channel, preflight refusals, and an update that records its failures instead of returning silently │ ├── docker_config.test.js # Deployment invariants — DB_PATH baked in, data excluded from the image, image tags parameterised by IMAGE_TAG, compose file mounted for the updater, channel shipped pointing at stable │ ├── battle_maps.test.js # Battle map upload/list/delete diff --git a/backend/__tests__/admin.test.js b/backend/__tests__/admin.test.js index 55bbd37..a0275e8 100644 --- a/backend/__tests__/admin.test.js +++ b/backend/__tests__/admin.test.js @@ -3,6 +3,8 @@ import express from 'express'; import request from 'supertest'; import jwt from 'jsonwebtoken'; import bcrypt from 'bcrypt'; +import https from 'https'; +import { vi } from 'vitest'; import { makeTestDb, get, all, run } from './helpers/testDb.js'; import adminRouteFactory from '../routes/admin.js'; @@ -298,3 +300,127 @@ describe('update routes', () => { expect(res.status).toBe(401); }); }); + +// ─── check-update ───────────────────────────────────────────────────────────── + +/** + * The route where three of this branch's faults lived: an unanchored tag filter, a + * comparator that returned NaN, and a "different" test that counted a downgrade as an + * update. Each was fixed in the module and each is tested there — but the module was + * always correct in isolation, and it was the route's use of it that shipped broken. + */ +describe('POST /api/admin/check-update', () => { + /** Stand in for the Docker Hub tag listing. */ + const withTags = (names) => { + const body = JSON.stringify({ results: names.map((name) => ({ name })) }); + return vi.spyOn(https, 'request').mockImplementation((options, cb) => { + const handlers = {}; + const upstream = { on: (evt, fn) => { handlers[evt] = fn; return upstream; } }; + const req = { + on: () => req, + end: () => { + cb(upstream); + process.nextTick(() => { handlers.data?.(body); handlers.end?.(); }); + }, + }; + return req; + }); + }; + + const check = () => request(app) + .post('/api/admin/check-update') + .set('Authorization', `Bearer ${ADMIN_TOKEN}`); + + const withChannel = async (tag, fn) => { + const prev = process.env.IMAGE_TAG; + const prevVersion = process.env.APP_VERSION; + if (tag === undefined) delete process.env.IMAGE_TAG; + else process.env.IMAGE_TAG = tag; + try { + return await fn(); + } finally { + if (prev === undefined) delete process.env.IMAGE_TAG; + else process.env.IMAGE_TAG = prev; + if (prevVersion === undefined) delete process.env.APP_VERSION; + else process.env.APP_VERSION = prevVersion; + } + }; + + afterEach(() => vi.restoreAllMocks()); + + it('offers a newer stable release', async () => { + withTags(['1.8.0', '1.8.1', 'latest']); + await withChannel(undefined, async () => { + process.env.APP_VERSION = '1.8.0'; + const res = await check(); + expect(res.body.hasUpdate).toBe(true); + expect(res.body.latest).toBe('1.8.1'); + }); + }); + + it('does not offer a downgrade when the registry trails the running build', async () => { + // The reported symptom: a 1.8.0 instance told "Update available: 1.8.0 → 1.7.4". + withTags(['1.7.4', '1.7.3', 'latest']); + await withChannel(undefined, async () => { + process.env.APP_VERSION = '1.8.0'; + const res = await check(); + expect(res.body.hasUpdate).toBe(false); + expect(res.body.message).toMatch(/up to date/i); + }); + }); + + it('a dev tag on the registry does not hide a stable release', async () => { + // The fault that would have appeared on the first dev tag published, and would have + // hurt stable users: the unanchored filter let 1.9.0-dev through, it parsed to NaN, + // the comparator returned NaN, the sort order became undefined, and a prerelease + // could surface as "latest" — which the version check then correctly refused, + // reporting no update when there was one. + withTags(['1.8.0', '1.9.0-dev.3', '1.8.1', 'latest']); + await withChannel(undefined, async () => { + process.env.APP_VERSION = '1.8.0'; + const res = await check(); + expect(res.body.hasUpdate).toBe(true); + expect(res.body.latest).toBe('1.8.1'); + }); + }); + + it('ignores dev builds on the stable channel', async () => { + withTags(['1.8.1', '1.9.0-dev', 'latest']); + await withChannel('latest', async () => { + process.env.APP_VERSION = '1.8.1'; + const res = await check(); + expect(res.body.hasUpdate).toBe(false); + }); + }); + + it('offers dev builds when the deployment runs the dev images', async () => { + // Same registry, same running version, different channel — and the channel comes + // from the tag compose pulls, so what is offered cannot diverge from what installs. + withTags(['1.8.1', '1.9.0-dev', 'latest']); + await withChannel('dev', async () => { + process.env.APP_VERSION = '1.8.1'; + const res = await check(); + expect(res.body.hasUpdate).toBe(true); + expect(res.body.latest).toBe('1.9.0-dev'); + }); + }); + + it('carries a dev deployment onto the release when it lands', async () => { + withTags(['1.9.0', '1.9.0-dev.7', 'latest']); + await withChannel('dev', async () => { + process.env.APP_VERSION = '1.9.0-dev.7'; + const res = await check(); + expect(res.body.hasUpdate).toBe(true); + expect(res.body.latest).toBe('1.9.0'); + }); + }); + + it('offers nothing when the registry has no version tags at all', async () => { + withTags(['latest', 'dev']); + await withChannel(undefined, async () => { + process.env.APP_VERSION = '1.8.1'; + const res = await check(); + expect(res.body.hasUpdate).toBe(false); + }); + }); +}); From cc2d036fec86b58cfad793a41c67f6009123e7fd Mon Sep 17 00:00:00 2001 From: Developer Date: Mon, 3 Aug 2026 00:23:37 -0500 Subject: [PATCH 13/16] ci: publish development builds to Docker Hub MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The dev channel had nothing to find. This publishes it, on manual dispatch or a push to a dev branch, with the tests run first. Two tags per build, and both are needed. `dev` is the moving pointer that IMAGE_TAG=dev pulls. `X.Y.Z-dev.N` is immutable and is the only form the update check can see, since `dev` is not a version and is filtered out of the tag listing — publishing only the moving tag would mean a dev deployment never being offered anything. The run number supplies N, so each build is a distinct version rather than the same one repeatedly. APP_VERSION is baked in as the same string. Without it the container reports 'dev', which parses as nothing, and the check can neither offer it an update nor confirm one landed. It never writes `latest`, which is the one thing that must not happen: a dev build there would reach every stable deployment. And it refuses to run while package.json still holds an already-released version. X.Y.Z-dev sorts below X.Y.Z, so dev builds of a released version are older than what is already out and would be offered to nobody — a confusing silence to debug, and cheap to catch here instead. Verified the comparison against 1.9.0/1.8.1/1.8.0. Five config tests guard the invariants that are easy to lose in a later edit: no latest, both tags, APP_VERSION baked, a distinct version per build, tests before publish. --- .github/workflows/dev-release.yml | 114 ++++++++++++++++++++++++ CHANGELOG.md | 2 + README.md | 1 + backend/__tests__/docker_config.test.js | 37 ++++++++ 4 files changed, 154 insertions(+) create mode 100644 .github/workflows/dev-release.yml diff --git a/.github/workflows/dev-release.yml b/.github/workflows/dev-release.yml new file mode 100644 index 0000000..e7e709e --- /dev/null +++ b/.github/workflows/dev-release.yml @@ -0,0 +1,114 @@ +name: Dev Build to Docker Hub + +# Publishes a development build. Deliberately separate from Release, and deliberately +# never touching `latest` — a stable deployment must not be able to reach these images. +# +# Two tags go out for each build, and both are needed: +# dev the moving pointer that IMAGE_TAG=dev pulls +# X.Y.Z-dev.N an immutable build, and the only form the update check can see, +# since `dev` is not a version and is filtered out of the tag listing +# +# The version comes from frontend/package.json, so dev builds must run *ahead* of the +# last release: with package.json at 1.9.0 the builds are 1.9.0-dev.N, which supersede +# 1.8.1 and are in turn superseded by 1.9.0 when it ships. + +on: + workflow_dispatch: + push: + branches: [dev] + +jobs: + test: + name: Tests + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-node@v4 + with: + node-version: '20' + + - name: Backend tests + working-directory: ./backend + run: npm ci && npm test + + - name: Frontend tests + working-directory: ./frontend + run: npm ci && npm test + + publish: + name: Build & Push + needs: test + runs-on: ubuntu-latest + + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Compose the dev version + id: version + run: | + BASE=$(jq -r '.version' frontend/package.json) + # The run number gives each build a distinct, increasing identity, so a dev + # deployment is offered the next one. A single moving X.Y.Z-dev tag would be + # the same version every time and would never be seen as an update. + echo "VERSION=${BASE}-dev.${{ github.run_number }}" >> "$GITHUB_OUTPUT" + echo "BASE=${BASE}" >> "$GITHUB_OUTPUT" + + - name: Refuse to publish a dev build older than the latest release + env: + BASE: ${{ steps.version.outputs.BASE }} + REPO: ${{ secrets.DOCKERHUB_USERNAME }}/citynet-frontend + run: | + set -euo pipefail + LATEST=$(curl -fsSL "https://hub.docker.com/v2/repositories/${REPO}/tags?page_size=100" \ + | jq -r '[.results[].name | select(test("^[0-9]+[.][0-9]+[.][0-9]+$"))] + | sort_by(split(".") | map(tonumber)) | last // "0.0.0"') + echo "package.json: ${BASE} latest release: ${LATEST}" + # X.Y.Z-dev sorts *below* X.Y.Z, so a dev build of an already-released version + # is older than what is already out and would never be offered to anyone. + # Catching that here is far cheaper than working out later why a dev + # deployment is being told it is up to date. + NEWEST=$(printf '%s\n%s\n' "${BASE}" "${LATEST}" | sort -V | tail -1) + if [ "${BASE}" = "${LATEST}" ] || [ "${NEWEST}" != "${BASE}" ]; then + echo "::error::Dev builds must be ahead of the last release. Bump package.json and frontend/package.json past ${LATEST} first." + exit 1 + fi + + - name: Log in to Docker Hub + uses: docker/login-action@v3 + with: + username: ${{ secrets.DOCKERHUB_USERNAME }} + password: ${{ secrets.DOCKERHUB_TOKEN }} + + - name: Set up Docker Buildx + uses: docker/setup-buildx-action@v3 + + - name: Build & push backend + uses: docker/build-push-action@v5 + with: + context: . + file: Dockerfile.backend + push: true + build-args: | + APP_VERSION=${{ steps.version.outputs.VERSION }} + tags: | + ${{ secrets.DOCKERHUB_USERNAME }}/citynet-backend:dev + ${{ secrets.DOCKERHUB_USERNAME }}/citynet-backend:${{ steps.version.outputs.VERSION }} + + - name: Build & push frontend + uses: docker/build-push-action@v5 + with: + context: . + file: Dockerfile.frontend + push: true + tags: | + ${{ secrets.DOCKERHUB_USERNAME }}/citynet-frontend:dev + ${{ secrets.DOCKERHUB_USERNAME }}/citynet-frontend:${{ steps.version.outputs.VERSION }} + + - name: Summary + run: | + { + echo "Published \`${{ steps.version.outputs.VERSION }}\` and \`:dev\`." + echo "" + echo "A deployment with \`IMAGE_TAG=dev\` will be offered this build." + } >> "$GITHUB_STEP_SUMMARY" diff --git a/CHANGELOG.md b/CHANGELOG.md index e1ef759..a278b91 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,8 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). Development builds are tagged `X.Y.Z-dev`, with a counter accepted but not required, so `1.9.0-dev` and `1.9.0-dev.7` both work and introducing counters later is a build-workflow change rather than a code one. A dev build of a newer release is offered to someone on an older release, the release itself supersedes its own dev builds when it lands, and a release user is never dragged back onto a dev build of the same version. A pinned version tag counts as stable, since pinning is not a channel. + A `Dev Build to Docker Hub` workflow publishes them, on manual dispatch or a push to a `dev` branch. It runs the test suites first, never touches `latest`, and pushes two tags per build: `dev`, which is what `IMAGE_TAG=dev` pulls, and `X.Y.Z-dev.N`, which is the only form the update check can see — `dev` is not a version and is filtered out of the tag listing. It refuses to run when `package.json` still holds an already-released version, since `X.Y.Z-dev` sorts *below* `X.Y.Z` and such a build could never be offered to anyone. + `IMAGE_TAG` must also reach the `.env` beside `docker-compose.yml`, which the setup steps already cover by copying `backend/.env` to the project root: compose interpolates from the project file rather than from `env_file`. ### Fixed diff --git a/README.md b/README.md index e71f402..e1a3181 100644 --- a/README.md +++ b/README.md @@ -567,6 +567,7 @@ CITY_NET/ ├── docs/ # Reference docs (deployment plans, feature notes) ├── Dockerfile.backend ├── Dockerfile.frontend +├── .github/workflows/ # CI Tests on PRs and main; Release to Docker Hub on green main; Dev Build to Docker Hub on dispatch or a push to dev ├── docker-compose.yml # Image tags read ${IMAGE_TAG:-latest}, so the release channel is a setting rather than an edit ├── nginx.conf └── .env.example diff --git a/backend/__tests__/docker_config.test.js b/backend/__tests__/docker_config.test.js index 1be691f..a78345e 100644 --- a/backend/__tests__/docker_config.test.js +++ b/backend/__tests__/docker_config.test.js @@ -106,3 +106,40 @@ describe('.env.example release channel', () => { expect(env()).not.toMatch(/^DEV=/m); }); }); + +describe('dev release workflow', () => { + const workflow = () => readRoot('.github/workflows/dev-release.yml'); + + it('never publishes to latest', () => { + // The one thing that must not happen. `latest` is what every stable deployment + // pulls, so a dev build landing there would push unreleased code to everybody. + expect(workflow()).not.toMatch(/:latest/); + }); + + it('publishes both the moving tag and an immutable version', () => { + // Both are needed: `dev` is what IMAGE_TAG=dev pulls, and X.Y.Z-dev.N is the only + // form the update check can see, since `dev` is not a version and is filtered out + // of the tag listing. + const yml = workflow(); + expect(yml).toMatch(/citynet-backend:dev$/m); + expect(yml).toMatch(/citynet-frontend:dev$/m); + expect(yml).toMatch(/citynet-backend:\$\{\{ steps\.version\.outputs\.VERSION \}\}/); + expect(yml).toMatch(/citynet-frontend:\$\{\{ steps\.version\.outputs\.VERSION \}\}/); + }); + + it('bakes the dev version into the image as APP_VERSION', () => { + // Without it the container reports 'dev', which parses as nothing, and the update + // check can neither offer it an update nor confirm one landed. + expect(workflow()).toMatch(/APP_VERSION=\$\{\{ steps\.version\.outputs\.VERSION \}\}/); + }); + + it('gives each build a distinct version', () => { + // A single moving X.Y.Z-dev would be the same version every time, so a dev + // deployment would never see a newer one. + expect(workflow()).toMatch(/-dev\.\$\{\{ github\.run_number \}\}/); + }); + + it('runs the tests before publishing', () => { + expect(workflow()).toMatch(/needs: test/); + }); +}); From 89e091d22d523525c9f15729cc76ff693ead9696 Mon Sep 17 00:00:00 2001 From: Developer Date: Mon, 3 Aug 2026 08:43:42 -0500 Subject: [PATCH 14/16] ci: run tests on pull requests into dev dev is where unreleased work integrates and where development images are published from, so a PR into it should get the same test feedback as one into main. Without this a fault would surface at publish time rather than at review time. Only the pull_request trigger. A push to dev already runs both suites inside Dev Build to Docker Hub before it publishes, so adding push here would run the same tests twice on every dev commit for no extra signal. --- .github/workflows/ci.yml | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 01869d4..bbdee29 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,12 +1,18 @@ name: CI Tests on: - # Runs tests whenever someone opens or updates a Pull Request targeting 'main' + # Runs tests whenever someone opens or updates a Pull Request targeting 'main' or + # 'dev' — the integration branch that publishes development images. pull_request: branches: - main - - # Runs tests when code is merged or pushed directly to 'main' + - dev + + # Runs tests when code is merged or pushed directly to 'main'. + # + # Not 'dev': a push there triggers Dev Build to Docker Hub, which runs both suites + # itself before publishing. Adding it here would run the same tests twice on every + # dev commit for no extra signal. push: branches: - main From 062fa241c577309b2d61d3ca2abdcc580cd7f062 Mon Sep 17 00:00:00 2001 From: Developer Date: Mon, 3 Aug 2026 18:20:51 -0500 Subject: [PATCH 15/16] fix(update): the nav-panel button had none of the hardening MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Asked whether anything was missing, and this was: there are two places to start an update, and only one of them was fixed. Sidebar.applyUpdate ignored the response from /api/update entirely, so a 409 naming the problem was discarded and it still said "waiting for server". It waited on the version changing, which never happens on a build without APP_VERSION. And it polled every three seconds with no deadline. That is the originally reported bug, intact, in the path UPGRADE.md tells people to click — while the modal beside it had been carefully fixed. Both now call one shared client. Having two implementations is the only reason one could be hardened and the other left alone, so the fix is to have one rather than to fix the second copy. Also: the "read more" link on both surfaces pointed at README.md#updating, an anchor that does not exist anywhere in the README — so the link offered to someone whose update just failed dropped them at the top of a 570-line file. Both now point at UPGRADE.md, which is the actual guide. 12 tests for the shared client, covering what each caller relied on and what neither of them checked: an index.html fallback answering 200, a refusal with no body, and nothing being POSTed to a server that cannot act on it. --- CHANGELOG.md | 2 + frontend/src/components/Sidebar.tsx | 58 ++++--- frontend/src/components/UpdateModal.tsx | 134 +++------------- .../components/__tests__/UpdateModal.test.tsx | 13 +- .../src/utils/__tests__/updateClient.test.ts | 115 ++++++++++++++ frontend/src/utils/updateClient.ts | 146 ++++++++++++++++++ 6 files changed, 321 insertions(+), 147 deletions(-) create mode 100644 frontend/src/utils/__tests__/updateClient.test.ts create mode 100644 frontend/src/utils/updateClient.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index a278b91..d0bbc71 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,8 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). `POST /api/update` now checks the mount, the Docker socket and the compose project labels *before* answering, and returns `409` naming what is missing and what to do about it. Both steps append to `backend/data/update.log`, which lives on the data volume and so survives the container being replaced. `GET /api/update/status` reports phase and error, and the modal shows them, reassures at 45 seconds that a pull legitimately takes minutes, and gives up after six with the host command to fall back to. - **A container too old to update itself now says so immediately.** Such a container answers `POST /api/update` with "Update started" and then does nothing, so the client used to wait out the full deadline to learn what could be known at once. It is asked for `GET /api/update/status` first — a route that only exists in the self-checking build — and if that is missing the modal says the container predates it and shows the command to run on the host. The response shape is checked rather than just the status code, since a setup serving `index.html` for unknown paths answers `200` with a page. Nothing is POSTed to a server that cannot act on it. +- **The nav-panel update button had none of the above.** There were two implementations of the update flow — the modal and the panel — and only the modal's was hardened. The panel ignored the server's refusal entirely, waited on the version rather than the restart, and polled every three seconds with no deadline, so the original symptom survived in the path the upgrade guide tells people to use. Both now drive one shared client, which is the only reason a second copy could go unfixed. +- **The "read more" link on the update panel pointed at a heading that does not exist.** `README.md#updating` has no such anchor, so someone whose update just failed landed at the top of a 570-line README. Both links now go to `UPGRADE.md`, which is the actual guide. - **A successful update could hang too.** The client waited for the reported version to change, but a build without `APP_VERSION` reports `dev` before and after. `/api/version` now carries a boot id and the client waits for the restart itself. - **The update check offered downgrades.** `hasUpdate` was `latest !== current`, so a published tag trailing the running one counted as an update — a 1.8.0 instance was offered 1.7.4. It is a numeric version comparison now, and anything unparseable (`dev`, `latest`) is never offered. diff --git a/frontend/src/components/Sidebar.tsx b/frontend/src/components/Sidebar.tsx index 3655ab9..b6c446b 100644 --- a/frontend/src/components/Sidebar.tsx +++ b/frontend/src/components/Sidebar.tsx @@ -9,6 +9,7 @@ import { CurrencyIcon } from './BankWindows'; import { THEMES } from '../theme/themes'; import type { ThemeName } from '../theme/themes'; import { getTemplate } from '../sheets'; +import { startUpdate, waitForRestart, currentBootId } from '../utils/updateClient'; // Token defense config for the active game system; default is D&D-style AC const getTokenDefense = (gameSystem?: string) => @@ -57,38 +58,33 @@ function CheckUpdateButton({ token }: { token: string }) { const applyUpdate = async () => { setStatus('updating'); - try { - const checkRes = await fetch('/api/check-update', { - method: 'POST', - headers: { Authorization: `Bearer ${token}` }, - }); - const { current: originalCurrent } = await checkRes.json(); - - await fetch('/api/update', { - method: 'POST', - headers: { Authorization: `Bearer ${token}` }, - }); - setVersionMessage('Update in progress — waiting for server...'); - - // Poll /api/version until the server comes back on a different version - const poll = async () => { - try { - const res = await fetch('/api/version'); - if (!res.ok) throw new Error(); - const data = await res.json(); - if (data.version !== originalCurrent) { - window.location.href = `/?v=${Date.now()}`; - return; - } - } catch { /* server still restarting */ } - setTimeout(poll, 3000); - }; - setTimeout(poll, 10000); - } catch { + // Shared with the update modal. This path had its own copy and only the modal's was + // hardened, so the nav button — the one the upgrade guide tells people to click — + // still sat on "waiting for server" indefinitely when a stack could not update. + const { current: originalCurrent } = await fetch('/api/check-update', { + method: 'POST', + headers: { Authorization: `Bearer ${token}` }, + }).then(r => r.json()).catch(() => ({ current: '' })); + + const bootId = await currentBootId(); + const started = await startUpdate(token); + if (!started.ok) { setStatus('error'); - setVersionMessage('Update failed — try manually'); - setTimeout(() => setStatus('idle'), 5000); + setVersionMessage(started.command ? `${started.error} ${started.command}` : (started.error ?? 'Update failed')); + return; } + + setVersionMessage('Update in progress — waiting for server...'); + await waitForRestart({ + bootId, + currentVersion: originalCurrent, + onRestart: () => { window.location.href = `/?v=${Date.now()}`; }, + onStillWorking: () => setVersionMessage('Still working — pulling images, this can take a few minutes...'), + onFailed: (error, cmd) => { + setStatus('error'); + setVersionMessage(cmd ? `${error} ${cmd}` : error); + }, + }); }; const btnStyle = { background: 'none', border: 'none', cursor: 'pointer', color: 'var(--green)', fontSize: '0.6rem', opacity: 0.7, letterSpacing: '1px', marginTop: '4px', padding: 0 }; @@ -116,7 +112,7 @@ function CheckUpdateButton({ token }: { token: string }) { - README ↗ + UPGRADE GUIDE ↗
); } diff --git a/frontend/src/components/UpdateModal.tsx b/frontend/src/components/UpdateModal.tsx index a0c945b..7b40a86 100644 --- a/frontend/src/components/UpdateModal.tsx +++ b/frontend/src/components/UpdateModal.tsx @@ -1,4 +1,5 @@ import React, { useRef, useState } from 'react'; +import { startUpdate, waitForRestart, currentBootId } from '../utils/updateClient'; interface Props { current: string; @@ -41,128 +42,35 @@ export function UpdateModal({ current, latest, message, token, isDocker, onDismi }; }, []); - /** - * How long to wait for the server to come back before calling it a failure. - * - * A pull and a container recreate is minutes, not seconds, on a slow connection. But - * it is bounded: the previous version polled every three seconds forever, so a stack - * that could not update looked identical to one still working, and sat on - * "WAITING FOR SERVER" indefinitely. - */ - const DEADLINE_MS = 6 * 60 * 1000; - - /** Long enough that a normal pull has not finished, short enough to reassure. */ - const REASSURE_MS = 45 * 1000; - - /** What to run on the host when the container cannot update itself. */ - const MANUAL_COMMAND = 'docker compose pull && docker compose up -d'; - - /** - * Does the server behind this page have the self-checking updater? - * - * A container from before it has no `/api/update/status`, and asking is the one - * reliable way to find out — its `/api/update` cheerfully answers "Update started" - * and then does nothing, which is the whole failure being guarded against here. - * - * The shape is checked, not just the status code: a setup that serves index.html for - * unknown paths would otherwise answer 200 with a page and look modern. - */ - const hasModernUpdater = async () => { - try { - const res = await fetch('/api/update/status'); - if (!res.ok) return false; - const data = await res.json(); - return typeof data?.phase === 'string'; - } catch { - return false; - } - }; - const handleUpdate = async () => { setPhase('updating'); setStatusMsg('CHECKING SERVER...'); setDetail(''); + setCommand(''); - if (!(await hasModernUpdater())) { - // Told immediately rather than after a six-minute wait for a restart that this - // container was never going to perform. + const bootId = await currentBootId(); + const started = await startUpdate(token); + if (!started.ok) { setPhase('failed'); - setStatusMsg('THIS CONTAINER CANNOT UPDATE ITSELF'); - setDetail('It was built before the self-updating backend, so the in-app update would ' - + 'report success and then do nothing. Run this on the host, in the folder holding ' - + 'docker-compose.yml — after that, in-app updates work.'); - setCommand(MANUAL_COMMAND); + setStatusMsg(started.command ? 'THIS CONTAINER CANNOT UPDATE ITSELF' : 'UPDATE CANNOT RUN'); + setDetail(started.error ?? ''); + setCommand(started.command ?? ''); return; } setStatusMsg('UPDATE IN PROGRESS — WAITING FOR SERVER...'); - - let bootId = ''; - try { - const before = await (await fetch('/api/version')).json(); - bootId = before.bootId ?? ''; - } catch { /* carry on; the restart check falls back to the version */ } - - try { - const res = await fetch('/api/update', { method: 'POST', headers: { Authorization: `Bearer ${token}` } }); - if (!res.ok) { - // Preflight refused it and said why — much the commonest case being a container - // started before the compose file mounted itself. - const body = await res.json().catch(() => ({})); + await waitForRestart({ + bootId, + currentVersion: current, + onRestart: () => { window.location.href = `/?v=${Date.now()}`; }, + onStillWorking: () => setStatusMsg('STILL WORKING — PULLING IMAGES, THIS CAN TAKE A FEW MINUTES...'), + onFailed: (error, cmd) => { setPhase('failed'); - setStatusMsg('UPDATE CANNOT RUN'); - setDetail(body.error || `Server returned ${res.status}.`); - return; - } - } catch (e) { - setPhase('failed'); - setStatusMsg('UPDATE FAILED TO START'); - setDetail(e instanceof Error ? e.message : 'The server could not be reached.'); - return; - } - - const started = Date.now(); - const deadline = started + DEADLINE_MS; - const poll = async () => { - if (Date.now() - started > REASSURE_MS) { - setStatusMsg('STILL WORKING — PULLING IMAGES, THIS CAN TAKE A FEW MINUTES...'); - } - // The server reports its own failures now, so ask before assuming it is just slow. - try { - const st = await (await fetch('/api/update/status')).json(); - if (st.phase === 'failed') { - setPhase('failed'); - setStatusMsg('UPDATE FAILED'); - setDetail(st.error || 'No reason given.'); - return; - } - } catch { /* the server is restarting, which is the point */ } - - try { - const res = await fetch('/api/version'); - if (res.ok) { - const data = await res.json(); - // A restart is what matters. Waiting on the version alone hangs forever on a - // build without APP_VERSION, which reports 'dev' before and after. - const restarted = bootId ? data.bootId && data.bootId !== bootId : data.version !== current; - if (restarted) { - window.location.href = `/?v=${Date.now()}`; - return; - } - } - } catch { /* server restarting */ } - - if (Date.now() > deadline) { - setPhase('failed'); - setStatusMsg('UPDATE TIMED OUT'); - setDetail('The server did not come back within six minutes. Check backend/data/update.log ' - + 'for what happened, then recreate the stack from the host:'); - setCommand(MANUAL_COMMAND); - return; - } - setTimeout(poll, 3000); - }; - setTimeout(poll, 10000); + setStatusMsg('UPDATE FAILED'); + setDetail(error); + setCommand(cmd ?? ''); + }, + }); }; const panelStyle: React.CSSProperties = { @@ -219,12 +127,12 @@ export function UpdateModal({ current, latest, message, token, isDocker, onDismi
diff --git a/frontend/src/components/__tests__/UpdateModal.test.tsx b/frontend/src/components/__tests__/UpdateModal.test.tsx index 4b44e37..27a84aa 100644 --- a/frontend/src/components/__tests__/UpdateModal.test.tsx +++ b/frontend/src/components/__tests__/UpdateModal.test.tsx @@ -39,9 +39,13 @@ describe('UpdateModal rendering', () => { expect(screen.getByText('1.2.2')).toBeInTheDocument(); }); - it('renders README link', () => { + it('links to the upgrade guide', () => { + // Was a README#updating anchor that does not exist — the link shown to someone + // whose update just failed landed at the top of a 570-line README. render(); - expect(screen.getByText('README ↗')).toBeInTheDocument(); + const link = screen.getByText('UPGRADE GUIDE ↗'); + expect(link).toBeInTheDocument(); + expect(link.getAttribute('href')).toContain('UPGRADE.md'); }); }); @@ -158,8 +162,11 @@ describe('UpdateModal — Update Now', () => { })); render(); await userEvent.click(screen.getByText('UPDATE NOW')); + // "cannot run" and "failed to start" were two messages for one situation; the + // shared client reports a single one. What matters is that the reason reaches the + // screen rather than being swallowed. await waitFor(() => { - expect(screen.getByText(/UPDATE FAILED TO START/)).toBeInTheDocument(); + expect(screen.getByText(/UPDATE CANNOT RUN/)).toBeInTheDocument(); }); expect(screen.getByText(/network error/)).toBeInTheDocument(); }); diff --git a/frontend/src/utils/__tests__/updateClient.test.ts b/frontend/src/utils/__tests__/updateClient.test.ts new file mode 100644 index 0000000..63e7a48 --- /dev/null +++ b/frontend/src/utils/__tests__/updateClient.test.ts @@ -0,0 +1,115 @@ +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { hasModernUpdater, startUpdate, currentBootId, MANUAL_COMMAND } from '../updateClient'; + +/** + * The shared update client. + * + * It exists because there were two implementations — the update modal and the nav panel + * — and only the modal's was hardened. The panel, which is the button the upgrade guide + * points people at, still had no deadline and still ignored the server's refusal, so the + * original reported bug survived in the path most people use. + */ + +const stubFetch = (impl: (url: string, opts?: any) => any) => vi.stubGlobal('fetch', vi.fn(impl)); + +afterEach(() => { + vi.unstubAllGlobals(); + vi.restoreAllMocks(); +}); + +describe('hasModernUpdater', () => { + it('accepts a server that reports an update phase', () => { + stubFetch(async () => ({ ok: true, json: async () => ({ phase: 'idle' }) })); + return expect(hasModernUpdater()).resolves.toBe(true); + }); + + it('rejects a server with no status route', () => { + stubFetch(async () => ({ ok: false, status: 404, json: async () => ({}) })); + return expect(hasModernUpdater()).resolves.toBe(false); + }); + + it('rejects an index.html fallback answering 200 with a page', () => { + // A status-code check alone would read that as "this server has the new updater". + stubFetch(async () => ({ ok: true, json: async () => { throw new Error('not json'); } })); + return expect(hasModernUpdater()).resolves.toBe(false); + }); + + it('rejects a server that cannot be reached', () => { + stubFetch(async () => { throw new Error('offline'); }); + return expect(hasModernUpdater()).resolves.toBe(false); + }); +}); + +describe('startUpdate', () => { + it('refuses to post to a container that cannot update itself', async () => { + // Such a container answers "Update started" and does nothing, so asking first is the + // difference between an immediate answer and a six-minute wait. + const fetchMock = vi.fn(async (url: string) => { + if (String(url).includes('/api/update/status')) return { ok: false, status: 404, json: async () => ({}) }; + return { ok: true, json: async () => ({}) }; + }); + vi.stubGlobal('fetch', fetchMock); + + const res = await startUpdate('t'); + expect(res.ok).toBe(false); + expect(res.command).toBe(MANUAL_COMMAND); + + const posted = fetchMock.mock.calls.some(([u, o]: any[]) => String(u).endsWith('/api/update') && o?.method === 'POST'); + expect(posted).toBe(false); + }); + + it('passes the server refusal through verbatim', async () => { + stubFetch(async (url: string) => { + if (String(url).includes('/api/update/status')) return { ok: true, json: async () => ({ phase: 'idle' }) }; + return { ok: false, status: 409, json: async () => ({ error: '/tmp/docker-compose.yml is not mounted' }) }; + }); + const res = await startUpdate('t'); + expect(res.ok).toBe(false); + expect(res.error).toContain('not mounted'); + }); + + it('reports a refusal with no body rather than claiming success', async () => { + stubFetch(async (url: string) => { + if (String(url).includes('/api/update/status')) return { ok: true, json: async () => ({ phase: 'idle' }) }; + return { ok: false, status: 500, json: async () => { throw new Error('no body'); } }; + }); + const res = await startUpdate('t'); + expect(res.ok).toBe(false); + expect(res.error).toContain('500'); + }); + + it('reports a network failure rather than throwing at the caller', async () => { + stubFetch(async (url: string) => { + if (String(url).includes('/api/update/status')) return { ok: true, json: async () => ({ phase: 'idle' }) }; + throw new Error('network error'); + }); + const res = await startUpdate('t'); + expect(res.ok).toBe(false); + expect(res.error).toContain('network error'); + }); + + it('succeeds when the server accepts it', async () => { + stubFetch(async (url: string) => { + if (String(url).includes('/api/update/status')) return { ok: true, json: async () => ({ phase: 'idle' }) }; + return { ok: true, json: async () => ({ message: 'Update started' }) }; + }); + expect((await startUpdate('t')).ok).toBe(true); + }); +}); + +describe('currentBootId', () => { + it('reads the boot id', async () => { + stubFetch(async () => ({ ok: true, json: async () => ({ bootId: 'boot-1' }) })); + expect(await currentBootId()).toBe('boot-1'); + }); + + it('returns empty for a server too old to have one, so the caller can fall back', async () => { + stubFetch(async () => ({ ok: true, json: async () => ({ version: '1.7.0' }) })); + expect(await currentBootId()).toBe(''); + }); + + it('returns empty rather than throwing when the server is unreachable', async () => { + stubFetch(async () => { throw new Error('offline'); }); + expect(await currentBootId()).toBe(''); + }); +}); diff --git a/frontend/src/utils/updateClient.ts b/frontend/src/utils/updateClient.ts new file mode 100644 index 0000000..c2ce84c --- /dev/null +++ b/frontend/src/utils/updateClient.ts @@ -0,0 +1,146 @@ +/** + * Driving an in-app update. + * + * There are two places to start one — the update modal and the nav panel — and they had + * separate implementations. Only one of them got hardened, so the panel button, which is + * the one the upgrade guide tells people to click, still reported "waiting for server" + * for ever on a stack that could not update. Hence one implementation. + */ + +/** What to run on the host when a container cannot update itself. */ +export const MANUAL_COMMAND = 'docker compose pull && docker compose up -d'; + +/** Long enough for a pull and a recreate on a slow line; short enough to end. */ +export const DEADLINE_MS = 6 * 60 * 1000; + +/** Long enough that a normal pull has not finished, short enough to reassure. */ +export const REASSURE_MS = 45 * 1000; + +export interface UpdateOutcome { + ok: boolean; + /** Set when it failed; already phrased for a person to read. */ + error?: string; + /** Set when the operator needs to run something themselves. */ + command?: string; +} + +/** + * Does the server behind this page have the self-checking updater? + * + * A container from before it has no `/api/update/status`, and asking is the one reliable + * way to find out — its `/api/update` cheerfully answers "Update started" and then does + * nothing, which is the failure being guarded against. + * + * The shape is checked, not just the status code: a setup that serves index.html for + * unknown paths would otherwise answer 200 with a page and look modern. + */ +export async function hasModernUpdater(): Promise { + try { + const res = await fetch('/api/update/status'); + if (!res.ok) return false; + const data = await res.json(); + return typeof data?.phase === 'string'; + } catch { + return false; + } +} + +/** The running server's boot id, which changes when it restarts. */ +export async function currentBootId(): Promise { + try { + const data = await (await fetch('/api/version')).json(); + return data.bootId ?? ''; + } catch { + return ''; + } +} + +/** + * Ask the server to update, refusing to try where it cannot work. + * + * Returns the reason rather than throwing, because every caller wants to show it. + */ +export async function startUpdate(token: string): Promise { + if (!(await hasModernUpdater())) { + return { + ok: false, + error: 'This container was built before the self-updating backend, so an in-app update ' + + 'would report success and then do nothing. Run this on the host, in the folder ' + + 'holding docker-compose.yml — after that, in-app updates work.', + command: MANUAL_COMMAND, + }; + } + + try { + const res = await fetch('/api/update', { + method: 'POST', + headers: { Authorization: `Bearer ${token}` }, + }); + if (res.ok) return { ok: true }; + // Preflight refused and said why — most often a container started before the + // compose file mounted itself. + const body = await res.json().catch(() => ({})); + return { ok: false, error: body.error || `Server returned ${res.status}.` }; + } catch (e) { + return { ok: false, error: e instanceof Error ? e.message : 'The server could not be reached.' }; + } +} + +/** + * Wait for the server to come back, or give up and say why. + * + * Waits on the boot id rather than the version: a build without `APP_VERSION` reports + * `dev` before and after, so waiting on the version hangs even when the update worked. + * Falls back to the version only when the server is too old to have a boot id. + * + * Bounded, unlike the version this replaces, which polled every three seconds for ever — + * so a stack that could not update was indistinguishable from one still working. + */ +export async function waitForRestart(opts: { + bootId: string; + currentVersion: string; + onRestart: () => void; + onFailed: (error: string, command?: string) => void; + onStillWorking?: () => void; +}): Promise { + const started = Date.now(); + const deadline = started + DEADLINE_MS; + let reassured = false; + + const poll = async (): Promise => { + if (!reassured && Date.now() - started > REASSURE_MS) { + reassured = true; + opts.onStillWorking?.(); + } + + // The server records its own failures now, so ask before assuming it is just slow. + try { + const st = await (await fetch('/api/update/status')).json(); + if (st.phase === 'failed') { + return opts.onFailed(st.error || 'No reason given.'); + } + } catch { /* restarting, which is the point */ } + + try { + const res = await fetch('/api/version'); + if (res.ok) { + const data = await res.json(); + const restarted = opts.bootId + ? data.bootId && data.bootId !== opts.bootId + : data.version !== opts.currentVersion; + if (restarted) return opts.onRestart(); + } + } catch { /* restarting */ } + + if (Date.now() > deadline) { + return opts.onFailed( + 'The server did not come back within six minutes. Check backend/data/update.log ' + + 'for what happened, then recreate the stack from the host:', + MANUAL_COMMAND + ); + } + setTimeout(poll, 3000); + }; + + setTimeout(poll, 10000); +} From d82c50df84b9750a4fe10fbffd7f3b5d14695a7e Mon Sep 17 00:00:00 2001 From: Developer Date: Mon, 3 Aug 2026 18:21:21 -0500 Subject: [PATCH 16/16] docs: add updateClient to the project structure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Missed in the previous commit — the anchor I matched on did not exist, and the script reported it rather than failing, which I read past. --- README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/README.md b/README.md index e1a3181..0711a99 100644 --- a/README.md +++ b/README.md @@ -546,6 +546,7 @@ CITY_NET/ │ │ │ └── shadowrun_6e.ts # Shadowrun 6E — attributes, d6 pool skills, Edge pips (SPEND button, admin replenish), weapons (DV/AR), Stun track, gated AWAKENED/EMERGED tabs; dynamic spell list (DRAIN/CAST) and adept power list (PP cost auto-summed) │ │ ├── streamerMode.ts # IS_SPECTATOR constant — detects ?streamer=true URL param │ │ └── utils/ +│ │ ├── updateClient.ts # One implementation of the in-app update flow, shared by the update modal and the nav panel — stale-container probe, server refusal passed through verbatim, restart detected by boot id, bounded wait. Two copies is how one of them stayed unhardened │ │ ├── locationHelpers.ts # Location geometry utilities; exports ZONE_TYPE_NAMES and isUserDefinedName │ │ ├── rhombusHelpers.ts # Player token position math │ │ ├── threeHelpers.tsx # Three.js scene utilities @@ -559,6 +560,7 @@ CITY_NET/ │ │ ├── roadHelpers.test.ts # consolidateRoads, chainRoadPolylines, buildRoadRibbonGeometry │ │ ├── mapExportBounds.test.ts # Bounds coverage; GPU clamping on both axes, aspect preserved when scaling down │ │ ├── mapExportWatermark.test.ts # Watermark anchor and stacking, scaling floor, filename slugging, download link cleanup +│ │ ├── updateClient.test.ts # Stale-container detection including an index.html fallback answering 200, refusals passed through, nothing POSTed to a server that cannot act │ │ └── overpassHelpers.test.ts # Elevation, geometry, and path-sampling tests │ └── public/ │ ├── signs/ # Preset neon SVG sign images (motel, bar, cyber-clinic, etc.)