diff --git a/.forgejo/workflows/build.yml b/.forgejo/workflows/build.yml index ca758ff..5551d68 100644 --- a/.forgejo/workflows/build.yml +++ b/.forgejo/workflows/build.yml @@ -426,13 +426,20 @@ jobs: # everywhere). Operator-flagged 2026-06-01 after first :c- # main-push build failed at this step. SHORT_SHA=$(printf '%s' "$GITHUB_SHA" | cut -c1-7) + # `channel` is baked into the image as FC_CHANNEL and reported by + # /api/extension/manifest (milestone 271 step 7). A tag-push counts as + # `main`: a vYY.MM.DD tag is cut from main, so that image is a + # main-channel artifact wearing an immutable name. if [ "${GITHUB_REF#refs/tags/}" != "${GITHUB_REF}" ]; then TAG_NAME="${GITHUB_REF#refs/tags/}" echo "tags=git.fabledsword.com/bvandeusen/fabledcurator:${TAG_NAME}" >> "$GITHUB_OUTPUT" + echo "channel=main" >> "$GITHUB_OUTPUT" elif [ "${GITHUB_REF##*/}" = "main" ]; then echo "tags=git.fabledsword.com/bvandeusen/fabledcurator:main,git.fabledsword.com/bvandeusen/fabledcurator:latest,git.fabledsword.com/bvandeusen/fabledcurator:c-${SHORT_SHA}" >> "$GITHUB_OUTPUT" + echo "channel=main" >> "$GITHUB_OUTPUT" else echo "tags=git.fabledsword.com/bvandeusen/fabledcurator:dev" >> "$GITHUB_OUTPUT" + echo "channel=dev" >> "$GITHUB_OUTPUT" fi - name: Login to Forgejo registry @@ -449,6 +456,11 @@ jobs: file: Dockerfile push: true tags: ${{ steps.tag.outputs.tags }} + # 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. + build-args: | + FC_CHANNEL=${{ steps.tag.outputs.channel }} build-ml: runs-on: python-ci diff --git a/Dockerfile b/Dockerfile index 6c9da51..050cc04 100644 --- a/Dockerfile +++ b/Dockerfile @@ -47,6 +47,23 @@ RUN chmod +x entrypoint.sh COPY --from=frontend-builder /build/dist ./frontend/dist +# Which channel this image belongs to — `dev` or `main` (milestone 271 step 7). +# build.yml passes it; /api/extension/manifest reports it beside the version so +# an operator can tell which channel an install came from without the channel +# ever touching the version string. +# +# Empty by default, deliberately: a locally-built image then reports NO channel +# rather than claiming to be one, and the manifest omits the field entirely — +# indistinguishable from an image built before the field existed, which is +# 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. +ARG FC_CHANNEL="" +ENV FC_CHANNEL=${FC_CHANNEL} + EXPOSE 8080 ENTRYPOINT ["./entrypoint.sh"] diff --git a/backend/app/api/extension.py b/backend/app/api/extension.py index 082bc1e..868bab2 100644 --- a/backend/app/api/extension.py +++ b/backend/app/api/extension.py @@ -7,6 +7,7 @@ from __future__ import annotations import asyncio import hashlib import hmac +import os import re from pathlib import Path @@ -31,6 +32,12 @@ 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() + async def _ext_key_required(session) -> bool: """Unlike /api/credentials (which accepts the browser path with no @@ -133,13 +140,30 @@ def _read_manifest_sync() -> dict | None: return None versioned.sort(key=lambda p: p.stat().st_mtime) latest = versioned[-1] - return { + info = { "installed": True, "version": _extract_version(latest.name), "xpi_url": f"/extension/{latest.name}", "latest_url": "/extension/fabledcurator-latest.xpi", "sha256": _sha256(latest), } + # The channel goes BESIDE the version, never inside it. A `-dev` suffix is + # what silently disabled the dev channel in the sibling project this design + # comes from: the comparator returned nothing for a non-integer segment, so + # every dev version compared equal and "no update available" became + # indistinguishable from "I cannot read this version". + # + # Omitted rather than defaulted when unset. Absence already has a meaning + # every reader must handle — an image built before this field existed says + # exactly the same thing by not having the key — so a blank channel reuses + # that path instead of inventing a second "unknown" spelling. + # + # Reported verbatim, not validated against {"dev", "main"}: if an image + # declares something else, showing what it actually claims is more useful + # to whoever is debugging it than dropping the value on the floor. + if FC_CHANNEL: + info["channel"] = FC_CHANNEL + return info @extension_bp.route("/manifest", methods=["GET"]) diff --git a/ci-requirements.md b/ci-requirements.md index 4d0f3c4..0be64a8 100644 --- a/ci-requirements.md +++ b/ci-requirements.md @@ -73,6 +73,15 @@ per `docs/process.md`'s "add deps to the image when used by >1 project". `extension-version`. A depth-1 clone sees one commit and derives a wrong, too-low value **rather than failing**, so the full-history checkout is load-bearing rather than incidental. +- **`FC_CHANNEL` is a build arg, not a runtime setting.** `build.yml` passes + `dev` / `main` to the web image only (the ml and agent images have nothing to + report it to), and `/api/extension/manifest` reports it beside the version so + an install can be traced to a channel. It is declared LAST in the Dockerfile + on purpose: an ARG invalidates every layer below it, and this is the one value + that differs between the dev and main builds of identical source, so placing + it earlier would stop the two channels ever sharing a cached `pip install`. + Empty by default — a local build then reports no channel at all rather than + claiming one. - Callers MUST `set -f` before substituting the script's output. Without it the shell expands `test/**` against the working tree and silently narrows the pattern to whatever files exist at that moment — a failure that looks like diff --git a/extension/README.md b/extension/README.md index 22db07c..6dc6db1 100644 --- a/extension/README.md +++ b/extension/README.md @@ -66,6 +66,25 @@ Commit time gives both branches the same number for the same source — which is exactly what lets one AMO signature serve both channels (family rule 149, FC issue #3092). +## Channels + +`dev` and `main` each build and sign their own extension, and an install is +tied to whichever FC instance it points at — Firefox's static `update_url` +cannot apply here, since every FC install is a different host, so the extension +asks its configured backend. **The channel therefore IS the instance.** +Switching channel means repointing the FC URL in options and reinstalling from +that host; there is no separate channel setting, and adding one would +contradict each server build shipping its own extension. + +The channel is reported *beside* the version, never inside it: +`/api/extension/manifest` answers `{"version": "...", "channel": "dev"}`. It is +optional — an instance that declares none simply omits the key, and the popup, +the toolbar tooltip and the Settings card all read exactly as they did before +the field existed. Do not be tempted to make it a `-dev` version suffix: the +comparator parses each dotted segment with `parseInt`, so a suffixed segment +reads as 0 and every dev build compares equal to every other, collapsing "no +update available" and "I cannot read this version" into one answer. + ## Release Nothing to do by hand. Push to `dev`: `build.yml` signs the extension if this diff --git a/extension/background/background.js b/extension/background/background.js index 9fc8f59..edbc2a0 100644 --- a/extension/background/background.js +++ b/extension/background/background.js @@ -37,7 +37,16 @@ ensureInitialized().catch(e => console.error('init failed:', e)); // configured backend for the latest published version and nudge the operator to // reinstall the freshly-signed XPI — surfaced as a popup banner (on demand) and // a toolbar badge (daily). /api/extension/manifest is public and returns -// {version, latest_url, sha256}; the XPI is served from the web root (not /api). +// {version, latest_url, sha256} plus an OPTIONAL {channel} naming which channel +// that instance serves ("dev"/"main", #3113); the XPI is served from the web +// root (not /api). +// +// The channel IS the instance: Firefox's static update_url cannot apply here +// because every FC install is a different host, so the extension asks its +// configured backend — which means switching channel is repointing apiUrl in +// options and reinstalling from that host. There is no separate channel +// setting to build, and building one would contradict each server build +// shipping its own extension. function versionIsNewer(candidate, current) { // Dotted numeric compare so 1.0.10 > 1.0.9 (a plain string compare wouldn't). @@ -60,12 +69,22 @@ async function checkForUpdateInfo() { } const currentVersion = browser.runtime.getManifest().version; const latestVersion = info && info.version ? info.version : null; + // Which channel the configured instance serves — reported ALONGSIDE the + // version, never folded into it. A `-dev` suffix would have to survive + // versionIsNewer's parseInt above, and it wouldn't: the segment would read + // as 0 and every dev build would compare equal to every other. + // + // null is a normal answer, not a failure — an instance built before the + // field existed, or one built locally with no channel declared. Nothing + // below branches on it except the label. + const channel = info && info.channel ? info.channel : null; // latest_url is served from the web root, not the JSON API. const base = api.webRoot(); return { updateAvailable: !!latestVersion && versionIsNewer(latestVersion, currentVersion), currentVersion, latestVersion, + channel, xpiUrl: info && info.latest_url ? `${base}${info.latest_url}` : null, }; } @@ -77,7 +96,11 @@ async function refreshUpdateBadge() { await browser.action.setBadgeText({ text: r.updateAvailable ? '↑' : '' }); if (r.updateAvailable) { await browser.action.setBadgeBackgroundColor({ color: '#F4BA7A' }); - await browser.action.setTitle({ title: `FabledCurator — update available (v${r.latestVersion})` }); + // Channel first, version second, and the channel dropped entirely when + // the instance doesn't report one — so the tooltip reads exactly as it + // did before the field existed rather than saying "(unknown ...)". + const label = r.channel ? `${r.channel} v${r.latestVersion}` : `v${r.latestVersion}`; + await browser.action.setTitle({ title: `FabledCurator — update available (${label})` }); } else { await browser.action.setTitle({ title: 'FabledCurator' }); } diff --git a/extension/popup/popup.js b/extension/popup/popup.js index 4ffcd8e..771bb6a 100644 --- a/extension/popup/popup.js +++ b/extension/popup/popup.js @@ -81,8 +81,12 @@ async function checkForUpdate() { } function showUpdateBanner(r) { + // The channel names itself beside the version, never inside it (#3113). + // Absent when the instance doesn't report one, and the banner then reads + // exactly as it did before the field existed. + const channel = r.channel ? ` (${r.channel})` : ''; document.getElementById('update-text').textContent = - `Update available — v${r.latestVersion} (installed v${r.currentVersion})`; + `Update available${channel} — v${r.latestVersion} (installed v${r.currentVersion})`; // Opening the signed XPI triggers Firefox's native install prompt. document.getElementById('update-btn').addEventListener('click', () => { browser.tabs.create({ url: r.xpiUrl }); diff --git a/frontend/src/components/settings/BrowserExtensionCard.vue b/frontend/src/components/settings/BrowserExtensionCard.vue index 2dfbd38..4226eba 100644 --- a/frontend/src/components/settings/BrowserExtensionCard.vue +++ b/frontend/src/components/settings/BrowserExtensionCard.vue @@ -4,6 +4,16 @@ · Firefox · v{{ manifest.version }} + + {{ manifest.channel }} diff --git a/frontend/test/components/browserExtensionCard.spec.js b/frontend/test/components/browserExtensionCard.spec.js new file mode 100644 index 0000000..b406bb5 --- /dev/null +++ b/frontend/test/components/browserExtensionCard.spec.js @@ -0,0 +1,72 @@ +// @vitest-environment happy-dom +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest' +import { nextTick } from 'vue' + +import BrowserExtensionCard from '../../src/components/settings/BrowserExtensionCard.vue' +import { freshPinia, mountComponent } from '../support/mountComponent.js' + +// useApi is a thin fetch wrapper, so the seam is fetch itself (same shape as +// showcase.spec.js) rather than a module mock. +function stubApi(manifest) { + globalThis.fetch = vi.fn(async (url) => { + const payload = String(url).includes('/api/extension/manifest') + ? manifest + : { key: 'test-key' } + return { + ok: true, status: 200, statusText: '200', + text: async () => JSON.stringify(payload), + } + }) +} + +async function mountCard(manifest) { + stubApi(manifest) + const w = mountComponent(BrowserExtensionCard, { pinia: freshPinia() }) + // onMounted fires two fetches (manifest + key) and each resolves through a + // chain of microtasks. Yielding to a macrotask drains the whole queue, which + // a fixed number of nextTicks would only do by luck. + await new Promise((resolve) => setTimeout(resolve, 0)) + await nextTick() + return w +} + +const INSTALLED = { + installed: true, + version: '1.0.3499884', + xpi_url: '/extension/fabledcurator-1.0.3499884.xpi', + latest_url: '/extension/fabledcurator-latest.xpi', + sha256: 'abc', +} + +describe('BrowserExtensionCard — channel', () => { + beforeEach(() => { vi.restoreAllMocks() }) + afterEach(() => { delete globalThis.fetch }) + + it('names the channel the instance reports', async () => { + // The point of the whole channel scheme: an operator can tell a dev + // instance from a main one without installing anything. + const w = await mountCard({ ...INSTALLED, channel: 'dev' }) + expect(w.text()).toContain('dev') + }) + + it('shows the version and the channel as SEPARATE text, never merged', async () => { + // Regression guard with teeth: the tempting shortcut is a `-dev` version + // suffix, and that is precisely what breaks the extension's comparator — + // it parses each dotted segment with parseInt, so a suffixed segment reads + // as 0 and every dev build compares equal to every other. If someone ever + // "simplifies" by folding the channel into the version, the version text + // stops being the bare derived number and this fails. + const w = await mountCard({ ...INSTALLED, channel: 'dev' }) + expect(w.text()).toContain('v1.0.3499884') + expect(w.text()).not.toContain('1.0.3499884-dev') + }) + + it('renders no channel when the instance declares none', async () => { + // A locally-built image, or one predating the field. The card must read + // exactly as it did before the channel existed rather than inventing an + // "unknown" badge — absence is a normal answer here, not a fault. + const w = await mountCard(INSTALLED) + expect(w.text()).toContain('v1.0.3499884') + expect(w.findAll('v-chip')).toHaveLength(0) + }) +}) diff --git a/tests/test_api_extension.py b/tests/test_api_extension.py index b2885de..65a8a77 100644 --- a/tests/test_api_extension.py +++ b/tests/test_api_extension.py @@ -383,6 +383,53 @@ async def test_extension_manifest_returns_metadata_when_xpi_present(client, monk assert body["sha256"] == hashlib.sha256(b"fake-xpi-content").hexdigest() +@pytest.mark.asyncio +async def test_extension_manifest_reports_the_channel_the_image_declares( + client, monkeypatch, tmp_path +): + """The channel travels BESIDE the version, never inside it. + + Folding it in as a `1.0.3499884-dev` suffix is the failure this design + exists to avoid: the extension's comparator parses each dotted segment as + an integer, so a suffixed segment collapses to 0 and every dev build + compares equal to every other — "no update available" and "I cannot read + this version" stop being distinguishable. Asserting the two are separate + keys is what keeps a future edit from merging them. + """ + (tmp_path / "fabledcurator-1.2.3.xpi").write_bytes(b"x") + monkeypatch.setattr(extension_module, "XPI_DIR", tmp_path) + monkeypatch.setattr(extension_module, "FC_CHANNEL", "dev") + resp = await client.get("/api/extension/manifest") + assert resp.status_code == 200 + body = await resp.get_json() + assert body["channel"] == "dev" + assert body["version"] == "1.2.3" + + +@pytest.mark.asyncio +async def test_extension_manifest_omits_the_channel_when_the_image_declares_none( + client, monkeypatch, tmp_path +): + """A local build, or any image from before the field existed. + + The key must be ABSENT rather than present-and-empty: absence is the state + every consumer already handles (an older image conveys it by not having the + key at all), so a blank channel reuses that path instead of introducing a + second spelling of "unknown" for each reader to special-case. + """ + (tmp_path / "fabledcurator-1.2.3.xpi").write_bytes(b"x") + monkeypatch.setattr(extension_module, "XPI_DIR", tmp_path) + monkeypatch.setattr(extension_module, "FC_CHANNEL", "") + resp = await client.get("/api/extension/manifest") + assert resp.status_code == 200 + body = await resp.get_json() + assert "channel" not in body + # Everything else still answers — an image with no channel is not a + # degraded one, it just cannot say which channel it came from. + assert body["installed"] is True + assert body["latest_url"] == "/extension/fabledcurator-latest.xpi" + + # --- /extension/ -----------------------------------------