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 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 69aa43c..d0bbc71 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,38 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). --- +## [1.8.1] - 2026-08-02 + +### Added + +- **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. + + 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 + +- **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. + + `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. + +### 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..0711a99 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 — 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/ -│ │ ├── 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 /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 @@ -362,7 +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; 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 │ ├── locations.test.js # Location CRUD and classification │ ├── locations.global.test.js # Custom structure global persistence tests @@ -543,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 @@ -556,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.) @@ -564,7 +569,8 @@ CITY_NET/ ├── docs/ # Reference docs (deployment plans, feature notes) ├── Dockerfile.backend ├── Dockerfile.frontend -├── docker-compose.yml +├── .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/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. diff --git a/backend/.env.example b/backend/.env.example index 77ed05c..85aabb8 100644 --- a/backend/.env.example +++ b/backend/.env.example @@ -17,3 +17,18 @@ DUCKDNS_TOKEN=your-duckdns-token # Timezone for DuckDNS container (e.g. America/New_York) TZ=America/Chicago + +# ── Release channel (optional) ─────────────────────────────────────────────── +# Which images this deployment runs. Stable unless you change it. +# +# IMAGE_TAG=latest stable releases (default) +# IMAGE_TAG=dev development builds — unreleased, and they may break +# +# 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. +# +# 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 1230273..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'; @@ -239,3 +241,186 @@ 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); + }); +}); + +// ─── 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); + }); + }); +}); diff --git a/backend/__tests__/docker_config.test.js b/backend/__tests__/docker_config.test.js index 5a70101..a78345e 100644 --- a/backend/__tests__/docker_config.test.js +++ b/backend/__tests__/docker_config.test.js @@ -61,3 +61,85 @@ 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 pointed at stable', () => { + // Dev builds are unreleased code; nobody should arrive on one by default. + expect(env()).toMatch(/^IMAGE_TAG=latest$/m); + }); + + 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); + }); +}); + +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/); + }); +}); diff --git a/backend/__tests__/updater.test.js b/backend/__tests__/updater.test.js new file mode 100644 index 0000000..0c600a5 --- /dev/null +++ b/backend/__tests__/updater.test.js @@ -0,0 +1,298 @@ +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('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', () => { + // '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('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('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', () => { + 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; + + 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..a99677a 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) --- @@ -255,19 +238,20 @@ 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.allowsDevBuilds(); 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'; - 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 +278,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 +310,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..28cfeb8 --- /dev/null +++ b/backend/updater.js @@ -0,0 +1,300 @@ +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'); +} + +/** + * 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, + * 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 a = parseVersion(candidate); + const b = parseVersion(current); + if (!a || !b) return false; + return compareVersions(a, b) > 0; +} + +/** + * The image tag this deployment runs, and the only thing that selects a channel. + * + * 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. + * + * 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 imageTag(env = process.env) { + return String(env.IMAGE_TAG ?? '').trim() || 'latest'; +} + +/** + * 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 allowsDevBuilds(env = process.env) { + return imageTag(env) === 'dev'; +} + +/** + * 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, + parseVersion, + compareVersions, + isVersionTag, + imageTag, + allowsDevBuilds, + isNewerVersion, + buildUpdateHelperArgs, + readComposeLabels, + preflight, + runUpdate, + getState, + resetState, + logPath, +}; 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 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/App.tsx b/frontend/src/App.tsx index 4502070..aaedcf9 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -156,7 +156,12 @@ function App() { .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 }); + setUpdateInfo({ + current: updateData.current, + latest: updateData.latest, + message: updateData.message, + isDocker: versionData.isDocker ?? false, + }); }) .catch(() => {}); }, [token, isAdmin]); 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 b9eaa5d..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; @@ -11,8 +12,10 @@ 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(''); + const [command, setCommand] = useState(''); // Draggable const modalRef = useRef(null); @@ -41,25 +44,33 @@ export function UpdateModal({ current, latest, message, token, isDocker, onDismi const handleUpdate = async () => { setPhase('updating'); - setStatusMsg('UPDATE IN PROGRESS — WAITING FOR SERVER...'); - 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 data = await res.json(); - if (data.version !== current) { - 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'); + setStatusMsg('CHECKING SERVER...'); + setDetail(''); + setCommand(''); + + const bootId = await currentBootId(); + const started = await startUpdate(token); + if (!started.ok) { + setPhase('failed'); + 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...'); + 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 FAILED'); + setDetail(error); + setCommand(cmd ?? ''); + }, + }); }; const panelStyle: React.CSSProperties = { @@ -116,12 +127,12 @@ export function UpdateModal({ current, latest, message, token, isDocker, onDismi
- README ↗ + UPGRADE GUIDE ↗
@@ -160,6 +171,28 @@ export function UpdateModal({ current, latest, message, token, isDocker, onDismi {phase === 'updating' && (
{statusMsg}
)} + + {phase === 'failed' && ( + <> +
{statusMsg}
+
{detail}
+ {command && ( +
+ {command} +
+ )} +
+ + +
+ + )} ); diff --git a/frontend/src/components/__tests__/UpdateModal.test.tsx b/frontend/src/components/__tests__/UpdateModal.test.tsx index 64efa37..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'); }); }); @@ -122,7 +126,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 +140,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,11 +155,112 @@ 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')); + // "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/)).toBeInTheDocument(); + expect(screen.getByText(/UPDATE CANNOT RUN/)).toBeInTheDocument(); }); + 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' }) }; + } + 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(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()); + + await userEvent.click(screen.getByText('BACK')); + expect(screen.getByText('UPDATE NOW')).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); +} 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": {