From bce894ba2499f2d10628cb727dd4ebbbae28051a Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Fri, 28 Aug 2026 18:08:31 -0400 Subject: [PATCH] feat(settings): the instance reports which build it is (318 step 6) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A dim line at the foot of Settings: `FabledCurator 2026.08.28.1249 · dev`. This is no longer a convenience. Milestone 318 stopped publishing version image tags, so an instance's own report is the ONLY answer to "which build is this?" — there is no registry name left to check it against. Note #3127 §5 says it directly: a wrong answer here has no second source to contradict it. Three states, kept distinct because collapsing any two of them lies: not asked yet render nothing asked, no version render "unknown" asked, has a version render it A blank footer reads as "no version", which is a different claim from "I cannot say". And a failed health call deliberately does NOT mark the build loaded — a network blip says nothing about the image, and presenting it as "unknown" would look like a defective build. Carried on /api/health rather than a new route: it answers at the same cost (two module constants, no I/O) and TopNav already fetches it app-wide, so a separate endpoint would mean a second request for two strings. Both fields are OMITTED when unset rather than sent empty. Absence already means "cannot say" — an image predating the field says exactly that by not having the key — so a second spelling would make every reader special-case it. The pre-existing test asserting the body is EXACTLY {"status": "ok"} is what keeps a well-meaning `or ""` default from creeping in. FC_CHANNEL now has one definition. It was read from the environment in extension.py and would have been read again here; the new build_info module holds both, and extension.py binds it as a module-level name so existing tests monkeypatch it exactly as before. Separate from config.py on purpose: those are operator settings meant to be changed, these describe the artifact. Channel sits beside the version, never inside it (rule 149), asserted from both ends. A `-dev` suffix would read as a 0 segment to the extension's parseInt comparator and make every dev build compare equal — #2993 exactly. Not hidden, per the operator and §7: the JS bundle and asset hashes fingerprint the build anyway, and "I'm on 2026.08.28.1249" is the single most useful line in a bug report. --- .forgejo/workflows/build.yml | 11 +++- Dockerfile | 12 +++- backend/app/api/extension.py | 13 ++-- backend/app/api/health.py | 19 +++++- backend/app/build_info.py | 30 +++++++++ frontend/src/stores/system.js | 26 +++++++- frontend/src/views/SettingsView.vue | 15 +++++ frontend/test/systemBuild.spec.js | 96 +++++++++++++++++++++++++++++ tests/test_health.py | 60 ++++++++++++++++++ 9 files changed, 268 insertions(+), 14 deletions(-) create mode 100644 backend/app/build_info.py create mode 100644 frontend/test/systemBuild.spec.js diff --git a/.forgejo/workflows/build.yml b/.forgejo/workflows/build.yml index 47228b7..23ddd18 100644 --- a/.forgejo/workflows/build.yml +++ b/.forgejo/workflows/build.yml @@ -482,6 +482,10 @@ jobs: set -eu DERIVED=$(sh scripts/artifacts.sh revision web) echo "revision=$DERIVED" >> "$GITHUB_OUTPUT" + # Baked into the web image as FC_VERSION and reported by /api/health. + # A pure function of the revision — same commit, same string — so it + # adds no variability the reuse check would have to account for. + echo "version=$(sh scripts/artifacts.sh version web)" >> "$GITHUB_OUTPUT" echo "build_tags=$TAGS" >> "$GITHUB_OUTPUT" # The moving tag for this channel. Which tag we ask IS the channel — @@ -611,11 +615,12 @@ jobs: # decoration — an unstamped image is one that will always rebuild. labels: | fc.revision=${{ steps.reuse.outputs.revision }} - # Only the web image carries a channel: it is the one that serves - # /api/extension/manifest. The ml and agent images have nothing to - # report it to. + # Only the web image carries these: it is the one with a UI and an + # HTTP surface to report them on. The ml and agent images have + # nothing to tell. build-args: | FC_CHANNEL=${{ steps.tag.outputs.channel }} + FC_VERSION=${{ steps.reuse.outputs.version }} # Registry-side manifest copy: no layer transfer, no local daemon, no # rebuild. Each -t becomes another reference to the SAME manifest the diff --git a/Dockerfile b/Dockerfile index 050cc04..f0e184d 100644 --- a/Dockerfile +++ b/Dockerfile @@ -58,11 +58,17 @@ COPY --from=frontend-builder /build/dist ./frontend/dist # exactly the shape every reader already has to handle. # # Declared LAST on purpose. An ARG/ENV invalidates every layer below it, and -# this is the one value that differs between the dev and main builds of -# identical source — put it any earlier and the two channels could never share -# a cached pip install. +# these are the values that differ between builds of otherwise identical +# source — put them any earlier and the two channels could never share a +# cached pip install. +# +# FC_VERSION is what the instance reports about itself in the UI. Since +# milestone 318 stopped publishing version image tags, that self-report is +# the only answer to "which build is this?" — nothing else names it. ARG FC_CHANNEL="" ENV FC_CHANNEL=${FC_CHANNEL} +ARG FC_VERSION="" +ENV FC_VERSION=${FC_VERSION} EXPOSE 8080 diff --git a/backend/app/api/extension.py b/backend/app/api/extension.py index 868bab2..e79e31a 100644 --- a/backend/app/api/extension.py +++ b/backend/app/api/extension.py @@ -7,13 +7,13 @@ from __future__ import annotations import asyncio import hashlib import hmac -import os import re from pathlib import Path from quart import Blueprint, jsonify, request from sqlalchemy import select +from ..build_info import FC_CHANNEL as _FC_CHANNEL from ..extensions import get_session from ..models import AppSetting from ..services.extension_service import ( @@ -33,10 +33,13 @@ XPI_DIR = Path("/app/frontend/dist/extension") _XPI_VERSION_RE = re.compile(r"fabledcurator-(?P[\w.-]+)\.xpi$") # Which channel this image belongs to — "dev" or "main" — baked in at build -# time from the FC_CHANNEL build arg (milestone 271 step 7). Empty for a local -# build, or for any image predating the field. Tests override by monkeypatching -# this constant, same as XPI_DIR above. -FC_CHANNEL = os.environ.get("FC_CHANNEL", "").strip() +# time (milestone 271 step 7). Read from build_info rather than the environment +# a second time: /api/health reports the same value, and two independent +# `os.environ.get` calls are two things that can drift. +# +# Still bound as a module-level name here, so tests monkeypatch +# `extension.FC_CHANNEL` exactly as they did before, same as XPI_DIR above. +FC_CHANNEL = _FC_CHANNEL async def _ext_key_required(session) -> bool: diff --git a/backend/app/api/health.py b/backend/app/api/health.py index ab4918e..7da388c 100644 --- a/backend/app/api/health.py +++ b/backend/app/api/health.py @@ -1,5 +1,20 @@ -"""Health endpoint — no DB or Redis touch; just liveness.""" +"""Health endpoint — no DB or Redis touch; liveness, plus the build's identity. + +The identity rides here rather than on a route of its own because it answers +at the same cost: two module constants, no I/O, nothing that can be slow or +fail. It is also already fetched app-wide — TopNav calls `refreshHealth` on +mount — so a separate endpoint would mean a second request for two strings. + +Both fields are OMITTED when unset rather than sent empty. See build_info. +""" + +from ..build_info import FC_CHANNEL, FC_VERSION async def get_health(): - return {"status": "ok"}, 200 + body = {"status": "ok"} + if FC_VERSION: + body["version"] = FC_VERSION + if FC_CHANNEL: + body["channel"] = FC_CHANNEL + return body, 200 diff --git a/backend/app/build_info.py b/backend/app/build_info.py new file mode 100644 index 0000000..7c04bbf --- /dev/null +++ b/backend/app/build_info.py @@ -0,0 +1,30 @@ +"""What this build IS — stamped at image build time, not configurable. + +Deliberately separate from `config.py`. Those are operator settings, read from +the environment and meant to be changed. These describe the artifact itself and +are baked in by CI (the `FC_VERSION` / `FC_CHANNEL` build args); an operator +setting them by hand is not a supported thing to do, it is just how a value +gets from the build into the running process. + +**Absent rather than empty when unknown.** A locally-built image has no version, +and neither did any image predating the field — one spelling of "cannot say", +which every reader already has to handle, instead of a second one to +special-case (note #3127 §7). + +**Why this matters more than it used to.** Milestone 318 stopped publishing +version image tags, so a running instance's self-report is now the *only* +answer to "which build is this?" — there is no registry name left to check it +against. A wrong value here has nothing to contradict it. That is why the UI +renders `unknown` rather than a blank or a plausible default: an empty footer +reads as "no version", which is a different and false claim. + +The channel lives BESIDE the version and is never folded into it (rule 149). +A `-dev` suffix would be parsed by the extension's comparator as a segment +worth 0, making every dev build compare equal to every other — issue #2993's +exact failure. +""" + +import os + +FC_VERSION = os.environ.get("FC_VERSION", "").strip() +FC_CHANNEL = os.environ.get("FC_CHANNEL", "").strip() diff --git a/frontend/src/stores/system.js b/frontend/src/stores/system.js index b3f772f..eae4059 100644 --- a/frontend/src/stores/system.js +++ b/frontend/src/stores/system.js @@ -5,6 +5,18 @@ import { useApi } from '../composables/useApi.js' export const useSystemStore = defineStore('system', () => { const api = useApi() const healthy = ref(null) // null=unknown, true=ok, false=down + // What the instance says it is. Since milestone 318 stopped publishing + // version image tags, this is the only answer to "which build is this?" — + // there is no registry name left to check it against. + // + // Three states, and collapsing any two of them would lie: + // buildLoaded=false we have not asked yet -> render nothing + // buildLoaded=true, version='' the build cannot say -> render "unknown" + // buildLoaded=true, version=x this build is x + // A blank footer would read as "no version", which is a different claim. + const buildVersion = ref('') + const buildChannel = ref('') + const buildLoaded = ref(false) const stats = ref(null) const statsLoading = ref(false) @@ -12,8 +24,17 @@ export const useSystemStore = defineStore('system', () => { try { const body = await api.get('/api/health') healthy.value = body.status === 'ok' + // Absent means "cannot say" — the server omits these rather than + // sending empty strings, so `?? ''` preserves that rather than + // inventing a value for it. + buildVersion.value = body.version ?? '' + buildChannel.value = body.channel ?? '' + buildLoaded.value = true } catch { healthy.value = false + // Deliberately NOT setting buildLoaded: a failed health call tells us + // nothing about the build, and claiming "unknown" would present a + // network blip as a defective image. } } @@ -26,5 +47,8 @@ export const useSystemStore = defineStore('system', () => { } } - return { healthy, stats, statsLoading, refreshHealth, refreshStats } + return { + healthy, stats, statsLoading, refreshHealth, refreshStats, + buildVersion, buildChannel, buildLoaded, + } }) diff --git a/frontend/src/views/SettingsView.vue b/frontend/src/views/SettingsView.vue index 21fa1db..35381cb 100644 --- a/frontend/src/views/SettingsView.vue +++ b/frontend/src/views/SettingsView.vue @@ -54,6 +54,21 @@ + + +
+ FabledCurator {{ system.buildVersion || 'unknown' }} + · {{ system.buildChannel }} +
diff --git a/frontend/test/systemBuild.spec.js b/frontend/test/systemBuild.spec.js new file mode 100644 index 0000000..eef4908 --- /dev/null +++ b/frontend/test/systemBuild.spec.js @@ -0,0 +1,96 @@ +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest' +import { setActivePinia, createPinia } from 'pinia' +import { useSystemStore } from '../src/stores/system.js' + +// Which build am I running? Milestone 318 stopped publishing version image +// tags, so the instance's own report is the ONLY answer — there is no registry +// name left to check it against. That promotes this from a convenience to the +// mechanism, and it means the three states below have to stay distinct: a +// wrong answer here has nothing to contradict it. +// +// not asked yet -> render nothing +// asked, no version -> render "unknown" +// asked, has a version -> render it +// +// Collapsing the first two would show "unknown" during every page load, and +// collapsing either into a blank would read as "no version", which is a +// different and false claim. + +function stubHealth(body, { fail = false } = {}) { + globalThis.fetch = vi.fn(async () => { + if (fail) throw new Error('network down') + return { + ok: true, status: 200, statusText: '200', + text: async () => JSON.stringify(body), + } + }) +} + +describe('system store — build identity', () => { + beforeEach(() => setActivePinia(createPinia())) + afterEach(() => { vi.restoreAllMocks(); delete globalThis.fetch }) + + it('starts having asked nothing, so the footer renders nothing', () => { + const s = useSystemStore() + expect(s.buildLoaded).toBe(false) + }) + + it('reports the version and channel the instance claims', async () => { + stubHealth({ status: 'ok', version: '2026.08.28.1249', channel: 'dev' }) + const s = useSystemStore() + await s.refreshHealth() + + expect(s.buildLoaded).toBe(true) + expect(s.buildVersion).toBe('2026.08.28.1249') + expect(s.buildChannel).toBe('dev') + }) + + it('keeps the channel OUT of the version string', async () => { + // The tempting shortcut is a `-dev` suffix. The extension's comparator + // parses each dotted segment with parseInt, so a suffixed segment reads as + // 0 and every dev build compares equal to every other — #2993 exactly + // (rule 149). If anyone ever "simplifies" by folding them together, the + // version stops being the bare derived number and this fails. + stubHealth({ status: 'ok', version: '2026.08.28.1249', channel: 'dev' }) + const s = useSystemStore() + await s.refreshHealth() + + expect(s.buildVersion).toBe('2026.08.28.1249') + expect(s.buildVersion).not.toContain('dev') + }) + + it('treats an absent version as "cannot say", not as a value', async () => { + // A locally-built image, or one predating the field. The server omits the + // key rather than sending an empty string; `?? ''` must preserve that + // rather than inventing something. The view renders "unknown" from it. + stubHealth({ status: 'ok' }) + const s = useSystemStore() + await s.refreshHealth() + + expect(s.buildLoaded).toBe(true) + expect(s.buildVersion).toBe('') + expect(s.buildChannel).toBe('') + }) + + it('reports a version with no channel without inventing one', async () => { + stubHealth({ status: 'ok', version: '2026.08.28.1249' }) + const s = useSystemStore() + await s.refreshHealth() + + expect(s.buildVersion).toBe('2026.08.28.1249') + expect(s.buildChannel).toBe('') + }) + + it('does not claim "unknown" when the health call itself failed', async () => { + // A network blip says nothing about the build. Marking it loaded here + // would present a transient failure as a defective image — and since + // nothing else names the build, there would be no second source to + // correct the impression. + stubHealth(null, { fail: true }) + const s = useSystemStore() + await s.refreshHealth() + + expect(s.healthy).toBe(false) + expect(s.buildLoaded).toBe(false) + }) +}) diff --git a/tests/test_health.py b/tests/test_health.py index 2a2069d..2041918 100644 --- a/tests/test_health.py +++ b/tests/test_health.py @@ -9,3 +9,63 @@ async def test_health_returns_ok(client): assert response.status_code == 200 body = await response.get_json() assert body == {"status": "ok"} + + +# --- build identity (milestone 318 step 6) -------------------------------- +# +# With no version image tags left, /api/health is the only place an instance +# says which build it is. Both fields are OMITTED when unset rather than sent +# empty: absence already means "cannot say" — an image predating the field +# says exactly that by not having the key — and a second spelling would make +# every reader special-case it (note #3127 §7). +# +# The test above is load-bearing for that: it asserts the body is EXACTLY +# {"status": "ok"} when nothing is stamped, so a well-meaning `or ""` default +# fails it. + + +@pytest.mark.asyncio +async def test_health_reports_the_build_it_is(client, monkeypatch): + from backend.app.api import health + + monkeypatch.setattr(health, "FC_VERSION", "2026.08.28.1249") + monkeypatch.setattr(health, "FC_CHANNEL", "dev") + + body = await (await client.get("/api/health")).get_json() + assert body == { + "status": "ok", + "version": "2026.08.28.1249", + "channel": "dev", + } + + +@pytest.mark.asyncio +async def test_health_keeps_the_channel_out_of_the_version(client, monkeypatch): + """Rule 149, asserted rather than assumed. + + The tempting shortcut is a `-dev` suffix on the version. The extension's + comparator parses each dotted segment with `parseInt`, so a suffixed + segment reads as 0 and every dev build compares equal to every other — + #2993 exactly. Two separate keys cannot express that mistake. + """ + from backend.app.api import health + + monkeypatch.setattr(health, "FC_VERSION", "2026.08.28.1249") + monkeypatch.setattr(health, "FC_CHANNEL", "dev") + + body = await (await client.get("/api/health")).get_json() + assert body["version"] == "2026.08.28.1249" + assert "dev" not in body["version"] + + +@pytest.mark.asyncio +async def test_health_omits_a_channel_it_cannot_name(client, monkeypatch): + """A locally-built image has a version but no channel. It must not gain an + empty one — the key's absence is the answer.""" + from backend.app.api import health + + monkeypatch.setattr(health, "FC_VERSION", "2026.08.28.1249") + monkeypatch.setattr(health, "FC_CHANNEL", "") + + body = await (await client.get("/api/health")).get_json() + assert body == {"status": "ok", "version": "2026.08.28.1249"}