diff --git a/frontend/src/App.vue b/frontend/src/App.vue index 2efab18..278b2f4 100644 --- a/frontend/src/App.vue +++ b/frontend/src/App.vue @@ -7,12 +7,20 @@ import { useTheme } from "@/composables/useTheme"; import { useShortcuts } from "@/composables/useShortcuts"; import { useAuthStore } from "@/stores/auth"; import { useSettingsStore } from "@/stores/settings"; -import { apiGet, apiPut } from "@/api/client"; +import { apiPut } from "@/api/client"; +import { fetchVersion } from "@/api/version"; useTheme(); const router = useRouter(); -const appVersion = ref("dev"); +// THREE states, not two (#3127 checklist 12). `null` is "not answered yet" and +// renders nothing; a string renders; `appVersionFailed` renders its own thing. +// This used to default to the literal "dev" and swallow the error, which meant +// an instance that could not answer was indistinguishable from a local build +// that genuinely reports "dev" — a blank standing in for `unknown`, in the one +// readout whose whole job is to say what is running. +const appVersion = ref(null); +const appVersionFailed = ref(false); const authStore = useAuthStore(); const settingsStore = useSettingsStore(); const { showShortcuts, toggleShortcuts, closeShortcuts } = useShortcuts(); @@ -119,10 +127,12 @@ onMounted(async () => { startAppServices(); } try { - const data = await apiGet<{ version: string }>("/api/version"); - appVersion.value = data.version; + appVersion.value = (await fetchVersion()).version; } catch { - // silent — version display is non-critical + // Not silent any more: the footer says it could not find out, rather than + // showing a version it never received. The full readout (version, channel, + // commit, build) lives in Settings → Config. + appVersionFailed.value = true; } }); @@ -151,7 +161,10 @@ onUnmounted(() => {
- + diff --git a/frontend/src/api/client.ts b/frontend/src/api/client.ts index 286d247..ad91d7a 100644 --- a/frontend/src/api/client.ts +++ b/frontend/src/api/client.ts @@ -52,8 +52,25 @@ export function apiErrorMessage(e: unknown, fallback: string): string { return fallback; } -export async function apiGet(path: string): Promise { - const res = await fetch(path); +/** + * A GET, optionally with a deadline. + * + * `timeoutMs` is OPT-IN rather than defaulted, deliberately. Every existing + * caller was written against a `fetch` that waits as long as the browser will, + * and handing them all a deadline in one change would alter behaviour at every + * call site at once, including ones nobody has looked at. New callers should + * pass one. + * + * Why a caller should want it: a wait with no deadline cannot report that it + * failed. It can only stay pending — which is indistinguishable, to anything + * rendering it, from "still loading". A surface that has to tell those two + * apart needs the request to give up on its own. + */ +export async function apiGet(path: string, opts?: { timeoutMs?: number }): Promise { + const res = await fetch( + path, + opts?.timeoutMs ? { signal: AbortSignal.timeout(opts.timeoutMs) } : undefined, + ); return handleResponse(res, path); } diff --git a/frontend/src/api/version.ts b/frontend/src/api/version.ts new file mode 100644 index 0000000..1d239c0 --- /dev/null +++ b/frontend/src/api/version.ts @@ -0,0 +1,42 @@ +import { apiGet } from "./client"; + +/** + * What `/api/version` answers — the client's half of `build_version_payload` + * (`src/scribe/routes/api.py`), which is where the reasoning for the shape is + * written down. + * + * EVERY FIELD BUT `version` IS OPTIONAL, and an absent one means "this build + * does not know", not "empty". A local build has no ordering key and no + * channel, and the server says so by omitting the keys rather than sending + * `""` — emitting a placeholder would let it claim a position in an update + * order it is not part of. + * + * So a renderer must read ABSENCE, never falsiness. `build` is a number and + * `0` is a legitimate ordering key, so `v.build || "unknown"` would report a + * real value as unknown; `v.build ?? "unknown"` is the correct form. + */ +export interface VersionPayload { + /** The NAME — `YYYY.MM.DD.HHMM` from commit time. Answers "is this the same code?" */ + version: string; + /** The ORDERING KEY — minutes since 2020-01-01, from build time. Absent on a local build. */ + build?: number; + /** `dev` / `main` / a tag. Its own field, never folded into the name. */ + channel?: string; + /** The commit the artifact was published under, so its claim can be checked against the registry. */ + commit?: string; +} + +/** + * The readout exists to answer "what is running?" during an incident, which is + * exactly when the server may be the thing that is unwell. Without a deadline + * a failing instance leaves the request pending forever and the surface sits + * on "still loading" — a blank standing in for `unknown`, which is the failure + * mode #3127 checklist 12 names by hand. Eight seconds is long enough for a + * slow-but-alive instance and short enough that a person watching it learns + * something. + */ +const VERSION_TIMEOUT_MS = 8000; + +export function fetchVersion(): Promise { + return apiGet("/api/version", { timeoutMs: VERSION_TIMEOUT_MS }); +} diff --git a/frontend/src/views/SettingsView.vue b/frontend/src/views/SettingsView.vue index 360447a..1a7e7cc 100644 --- a/frontend/src/views/SettingsView.vue +++ b/frontend/src/views/SettingsView.vue @@ -9,6 +9,7 @@ import type { User } from "@/types/auth"; import PaginationBar from "@/components/PaginationBar.vue"; import TagInput from "@/components/TagInput.vue"; import { fmtDate, fmtLogStamp } from "@/utils/dateFormat"; +import { fetchVersion, type VersionPayload } from "@/api/version"; const store = useSettingsStore(); const authStore = useAuthStore(); @@ -187,7 +188,37 @@ const changingPassword = ref(false); const invalidatingSessions = ref(false); const exporting = ref(false); const restoring = ref(false); -const appVersion = ref('dev'); +// ── What's running (#3127 checklist 12) ───────────────────────────────── +// Three states kept apart, because collapsing any two of them is the defect +// this readout exists to remove: `null` + no error = not asked yet (the Config +// tab has not been opened); a payload = answered, with each ABSENT field shown +// as "unknown"; `versionError` = the fetch itself failed, which is its own +// thing and must never render as a blank or as a plausible-looking value. +const versionInfo = ref(null); +const versionLoading = ref(false); +const versionError = ref(""); +const commitCopied = ref(false); + +async function loadVersionPanel() { + if (versionLoading.value) return; + versionLoading.value = true; + versionError.value = ""; + try { + versionInfo.value = await fetchVersion(); + } catch (e) { + versionInfo.value = null; + versionError.value = apiErrorMessage(e, "Could not reach the instance to ask what it is running."); + } finally { + versionLoading.value = false; + } +} + +async function copyCommit() { + if (!versionInfo.value?.commit) return; + await copyToClipboard(versionInfo.value.commit); + commitCopied.value = true; + setTimeout(() => { commitCopied.value = false; }, 2000); +} const restoreFileInput = ref(null); // Migrate stored "admin" → "config"; unknown tabs fall back to "general" @@ -201,6 +232,7 @@ function _loadTabContent(tab: string) { else if (tab === "logs") loadLogsPanel(); else if (tab === "groups") loadGroupsPanel(); else if (tab === "areas") canonStore.fetchCatalog(true); + else if (tab === "config" && !versionInfo.value) loadVersionPanel(); } if (tab === "apikeys") { fetchApiKeys(); } } @@ -554,10 +586,6 @@ function toggleProfileWorkDay(day: string) { function emptyTagsFetch(): Promise { return Promise.resolve([]) } onMounted(async () => { - try { - const v = await apiGet<{ version: string }>('/api/version') - appVersion.value = v.version - } catch { /* non-critical */ } await store.fetchSettings(); newEmail.value = authStore.user?.email ?? ""; @@ -2109,6 +2137,48 @@ async function deleteUser(userId: number) {
+
+

What's running

+

+ The build serving this page. Paste the commit into a :sha image + lookup to check the registry and the app agree about what was published. +

+ +
Reading the ledger…
+
+ {{ versionError }} + +
+
+
Version
+
{{ versionInfo.version }}
+ +
Channel
+
+ {{ versionInfo.channel ?? "unknown" }} +
+ +
Commit
+
+ {{ versionInfo.commit }} + +
+
unknown
+ +
Build
+ +
+ {{ versionInfo.build ?? "unknown" }} +
+
+
Nothing asked yet.
+
+

Application URL

@@ -2768,6 +2838,45 @@ async function deleteUser(userId: number) { letter-spacing: 0.07em; color: var(--fs-text-tertiary); } +/* What's running — a definition list of instance facts. Spacing/geometry only; + colour and type come from the tokens. */ +.version-grid { + display: grid; + grid-template-columns: max-content 1fr; + gap: 0.4rem 1rem; + margin: 0; + align-items: baseline; +} +.version-grid dt { + font-size: 0.8rem; + color: var(--fs-text-secondary); +} +.version-grid dd { + margin: 0; + font-size: 0.875rem; + font-family: var(--fs-font-mono); + color: var(--fs-text-primary); +} +/* An absent field reads as absent — never as a blank, and never styled to look + like a value it does not have (#3127 checklist 12). */ +.version-grid dd.version-unknown { + font-family: inherit; + font-style: italic; + color: var(--fs-text-tertiary); +} +.version-commit { + display: flex; + align-items: center; + gap: 0.5rem; + flex-wrap: wrap; +} +.version-sha { + overflow-wrap: anywhere; +} +.version-retry { + margin-left: 0.5rem; +} + .section-desc { margin: 0 0 1rem; font-size: 0.875rem; diff --git a/plugin/README.md b/plugin/README.md index adbd6b6..f7e9a1a 100644 --- a/plugin/README.md +++ b/plugin/README.md @@ -79,9 +79,9 @@ On install you'll be asked for: ## Notes - **Do not hand-edit `version` in `.claude-plugin/plugin.json`.** It is minted - from the clock — run `make mint-plugin` (or `python3 - scripts/mint_plugin_version.py`) after changing anything under `plugin/`, and - commit the result. The installer decides whether to refresh the cache it + from the clock — run `python3 scripts/mint_plugin_version.py` (or `make + mint-plugin`, where `make` is installed) after changing anything under + `plugin/`, and commit the result. The installer decides whether to refresh the cache it executes from by comparing that string, so content that ships without a new version reaches the repo and stops there (#2209). CI fails the lane if you forget. diff --git a/tests/test_version_readout.py b/tests/test_version_readout.py new file mode 100644 index 0000000..3d4d867 --- /dev/null +++ b/tests/test_version_readout.py @@ -0,0 +1,86 @@ +"""The app must SAY what it is running, and must not lie when it cannot find out. + +There is no frontend test runner in this repo, so these are source-inspection +guards in the unit lane — the same idiom `check_plugin.py` uses on the hook +shells. They are deliberately few and deliberately about ONE property each, +because a grep-shaped test that asserts a whole file's contents fails on every +refactor and gets deleted. + +WHY THIS FILE EXISTS. #3298: with a deploy misbehaving, nothing on the instance +could say which commit was serving it, and the one endpoint whose job that is +answered with the name of a branch. The value was fixed then. This is the other +half — the value reaching a person — and #3127 checklist 12 is specific about +the way it goes wrong: *never let a blank stand in for `unknown`*. A readout +that renders a plausible value it never received is worse than one that renders +nothing, because it ends the investigation instead of starting it. +""" +from __future__ import annotations + +import re +from pathlib import Path + +FRONTEND = Path(__file__).resolve().parents[1] / "frontend" / "src" + + +def test_something_actually_reads_the_version_endpoint(): + """The endpoint is not enough; something must ask it. + + `/api/version` answered correctly for weeks with no caller — an endpoint + reachable only by someone who already knew to curl it. Rule 27: a + capability with no surface the operator can touch is not shipped. + """ + hits = [p for p in FRONTEND.rglob("*.ts") if "/api/version" in p.read_text()] + assert hits, "nothing under frontend/src fetches /api/version" + + +def test_the_footer_does_not_default_to_a_plausible_version(): + """The regression this readout was built to remove. + + `appVersion` used to start life as the literal `"dev"` and the fetch + swallowed its own failure, so an instance that could not answer rendered + exactly what a healthy local build renders. Two very different states, one + string, and no way to tell them apart from the page. + + Pinned as "the ref does not start at a version-shaped literal" rather than + as an exact initialiser, so a later refactor can change how the state is + held without failing here — what must not come back is the plausible + default. + """ + app = (FRONTEND / "App.vue").read_text() + match = re.search(r"const appVersion = ref[^;]*;", app) + assert match, "App.vue no longer declares appVersion — update this guard" + decl = match.group(0) + assert '"dev"' not in decl and "'dev'" not in decl, ( + f"appVersion defaults to a version-shaped literal: {decl}\n" + "A failed fetch would render as a real-looking version (#3127 " + "checklist 12). Start from a not-answered-yet value instead." + ) + + +def test_optional_version_fields_are_read_by_absence_not_falsiness(): + """`build` is a number and 0 is a legitimate ordering key. + + The payload omits what it does not know rather than sending `""` or `0`, so + the renderer's job is to distinguish ABSENT from present. `||` cannot: it + would report a real `build` of 0 as unknown, and it is the form a person + reaches for by habit. `??` is the correct one, which is why this pins the + operator rather than the rendered output. + """ + view = (FRONTEND / "views" / "SettingsView.vue").read_text() + for field in ("channel", "build"): + assert f'versionInfo.{field} ?? "unknown"' in view, ( + f"the {field} readout must use `?? \"unknown\"`, never `|| \"unknown\"` — " + "an absent field and a falsy one are different answers" + ) + + +def test_the_version_request_carries_a_deadline(): + """Rule 156. A wait with no deadline cannot report that it failed. + + This readout is consulted when an instance is misbehaving, which is exactly + when it may never answer. Without a deadline the surface sits on "still + loading" forever — the blank standing in for `unknown` again, arrived at + from the other direction. + """ + src = (FRONTEND / "api" / "version.ts").read_text() + assert "timeoutMs" in src, "the version fetch must pass a deadline"