diff --git a/.forgejo/workflows/ci.yml b/.forgejo/workflows/ci.yml index 4a0af72..5f17846 100644 --- a/.forgejo/workflows/ci.yml +++ b/.forgejo/workflows/ci.yml @@ -2,6 +2,7 @@ name: CI # CI lanes per FabledRulebook/forgejo.md "CI philosophy": # - lint: ruff only, no dep install — fast-fail for the common lint bounce. +# - extension-version: guards the extension publish path (see the job). # - backend-lint-and-test: `pytest -m "not integration"`, no service containers. # - frontend-build: vitest unit + vite build. # - integration: pgvector + redis service containers; alembic + `pytest -m integration`. @@ -41,6 +42,121 @@ jobs: # catching syntax errors before the image build. run: python -m compileall -q agent/fc_agent + # Guards the extension publish path, which has no self-correcting behavior. + # + # build.yml's sign-extension job keys its AMO-signing cache purely on the + # version string in extension/package.json: if an `ext-` Forgejo + # release already carries an XPI, signing is SKIPPED and that old signed XPI + # is what build-web bakes into `:latest`. Nothing in that path inspects + # whether extension/ actually changed — so a forgotten version bump ships a + # stale extension on a fully green build, silently. (AMO can't help: it 409s + # on re-signing a version, which is exactly why the cache exists.) + # + # This job makes that case loud, on the dev push, instead of invisible at + # merge-to-main. It is pure git + text work — no deps, no services. + extension-version: + runs-on: python-ci + container: + image: git.fabledsword.com/bvandeusen/ci-python:3.14 + steps: + - uses: actions/checkout@v4 + with: + # Full history: the check diffs against the push's `before` SHA (or + # the PR base), which a depth-1 clone wouldn't contain. + fetch-depth: 0 + - name: Extension version guard + env: + BEFORE: ${{ github.event.before }} + PR_BASE: ${{ github.event.pull_request.base.sha }} + run: | + set -eu + # busybox sh on the act_runner — no bashisms (family rule). + ver() { grep -E '"version"' "$1" | head -1 | sed -E 's/.*"version"[[:space:]]*:[[:space:]]*"([^"]+)".*/\1/'; } + PKG=$(ver extension/package.json) + MAN=$(ver extension/manifest.json) + test -n "$PKG" || { echo "ERROR: no version found in extension/package.json"; exit 1; } + test -n "$MAN" || { echo "ERROR: no version found in extension/manifest.json"; exit 1; } + + # (1) Unconditional: the two version strings must agree. `web-ext sign` + # reads manifest.json (package.json sits in --ignore-files and isn't + # even inside the XPI), so AMO signs MAN and Firefox installs MAN. + # build.yml keys its cache, release tag, XPI filename — and therefore + # the version /api/extension/manifest reports to the update prompt — + # on PKG. Divergence either hard-fails at AMO or ships a mislabelled + # XPI whose update prompt lies about what's installed. + if [ "$MAN" != "$PKG" ]; then + echo "ERROR: extension version mismatch." + echo " extension/manifest.json = $MAN <- what AMO signs / Firefox installs" + echo " extension/package.json = $PKG <- what CI caches, names, and reports" + echo "Set both to the same value." + exit 1 + fi + + # (2) If the SHIPPED extension changed, the version must have moved. + # + # Compare against MAIN, not against the previous push. The publish + # decision is made at merge-to-main against whatever ext- + # already exists, so "differs from main" is the question that matters. + # Diffing against the previous dev push instead would demand a fresh + # bump on every iteration — push, tweak the extension again, and CI + # would insist on a second bump that buys nothing, inflating the + # version for no reason. On a main push there is no "main to compare + # to" yet, so fall back to that push's own before-SHA. + if [ "${GITHUB_REF##*/}" = "main" ]; then + BASE="${BEFORE:-}" + else + BASE=$(git rev-parse --verify -q origin/main 2>/dev/null || git rev-parse --verify -q main 2>/dev/null || echo "") + # PR base is the fallback when main isn't in the clone at all. + [ -n "$BASE" ] || BASE="${PR_BASE:-}" + fi + case "$BASE" in + ''|0000000000000000000000000000000000000000) + echo "No usable base ref (no main in clone / first push) — skipping the bump check." + echo "OK: extension version $PKG" + exit 0 + ;; + esac + if ! git cat-file -e "$BASE^{commit}" 2>/dev/null; then + echo "Base commit $BASE not in this clone — skipping the bump check." + echo "OK: extension version $PKG" + exit 0 + fi + # Exclusions mirror --ignore-files in extension/package.json's web-ext + # scripts: these files are not packaged into the XPI, so touching them + # (e.g. Renovate bumping the web-ext devDep, or editing a spec) + # changes nothing shipped and must not demand a version bump. + # KEEP IN SYNC with --ignore-files — a file packaged into the XPI but + # excluded here is exactly the silent-stale-ship this job exists to + # prevent. test/version.spec.js pins the two lists' shared intent. + CHANGED=$(git diff --name-only "$BASE" HEAD -- extension/ \ + ':(exclude)extension/package.json' \ + ':(exclude)extension/package-lock.json' \ + ':(exclude)extension/README.md' \ + ':(exclude)extension/.gitignore' \ + ':(exclude)extension/vitest.config.js' \ + ':(exclude)extension/test/**') + if [ -z "$CHANGED" ]; then + echo "No packaged extension files changed since $BASE — nothing to guard." + echo "OK: extension version $PKG" + exit 0 + fi + echo "Packaged extension files changed since $BASE:" + echo "$CHANGED" | sed 's/^/ /' + PKG_OLD=$(git show "$BASE:extension/package.json" 2>/dev/null | grep -E '"version"' | head -1 | sed -E 's/.*"version"[[:space:]]*:[[:space:]]*"([^"]+)".*/\1/') + if [ -z "$PKG_OLD" ]; then + echo "Could not read the base version — skipping the bump check." + echo "OK: extension version $PKG" + exit 0 + fi + if [ "$PKG_OLD" = "$PKG" ]; then + echo "ERROR: packaged extension files changed but the version is still $PKG." + echo "build.yml would find the existing ext-$PKG release, skip AMO signing," + echo "and bake the OLD signed XPI into :latest — a green build shipping stale code." + echo "Bump the version in BOTH extension/package.json and extension/manifest.json." + exit 1 + fi + echo "OK: extension version $PKG_OLD -> $PKG" + backend-lint-and-test: runs-on: python-ci container: diff --git a/.forgejo/workflows/extension.yml b/.forgejo/workflows/extension.yml index 3a50314..a67c578 100644 --- a/.forgejo/workflows/extension.yml +++ b/.forgejo/workflows/extension.yml @@ -1,5 +1,5 @@ name: extension -# Lint-only workflow. The sign-and-publish dance moved into build.yml's +# Lint + unit tests. The sign-and-publish dance moved into build.yml's # `sign-extension` job (2026-05-25) — `:latest` now always bundles the XPI # because sign-extension runs as a build-web dependency in the SAME workflow, # eliminating the prior race between build.yml and a separate extension.yml. @@ -10,10 +10,15 @@ on: paths: - 'extension/**' - '.forgejo/workflows/extension.yml' + # test/version.spec.js asserts ci.yml's extension-version guard never + # ignores a file web-ext actually packages, so a ci.yml-only edit can + # break this suite and must trigger it. + - '.forgejo/workflows/ci.yml' pull_request: branches: [main] paths: - 'extension/**' + - '.forgejo/workflows/ci.yml' workflow_dispatch: jobs: @@ -23,7 +28,13 @@ jobs: image: node:24-bookworm-slim steps: - uses: actions/checkout@v4 - - name: Install web-ext - run: cd extension && npm install --no-save --no-audit --no-fund + # Not --no-save: vitest and web-ext are both real devDependencies now, + # and the suite needs vitest resolvable from node_modules. + - name: Install dev dependencies + run: cd extension && npm install --no-audit --no-fund - name: Lint run: cd extension && npm run lint + # Pure-logic specs over lib/url.js and lib/platforms.js plus manifest / + # package version-consistency checks. No browser, no network. + - name: Unit tests + run: cd extension && npm run test:unit diff --git a/ci-requirements.md b/ci-requirements.md index c2265ca..c7a5438 100644 --- a/ci-requirements.md +++ b/ci-requirements.md @@ -11,12 +11,22 @@ git.fabledsword.com/bvandeusen/ci-python:3.14 - python 3.14 - ruff (analyzer for `backend/`, `tests/`, `alembic/`) - node (frontend job: `npm install` + vitest + vite build) -- docker CLI + buildx (`.forgejo/workflows/build.yml`: build-web, build-ml — Forgejo registry push) +- docker CLI + buildx (`.forgejo/workflows/build.yml`: build-web, build-ml — Fabled-Git registry push) + +## Secondary runtime image + +node:24-bookworm-slim — `.forgejo/workflows/extension.yml` only. + +The extension lane is the one job that does NOT run on `ci-python:3.14`: it +needs a current Node for `web-ext` and vitest and nothing Python at all. Kept +on the upstream slim image rather than adding a Node toolchain to `ci-python`, +per `docs/process.md`'s "add deps to the image when used by >1 project". ## Per-job tool installs - `pip install -r requirements.txt pytest pytest-asyncio` — in `backend-lint-and-test` and `integration` jobs - `npm install --no-audit --no-fund` — in `frontend-build` job +- `npm install --no-audit --no-fund` — in `extension.yml`'s `lint` job (web-ext + vitest) ## Notes @@ -26,12 +36,20 @@ git.fabledsword.com/bvandeusen/ci-python:3.14 "add deps to image when used by >1 project" rule: FC alone is one Python project, so the deps live in `requirements.txt` and install per-job. Reconsider when a second Fabled-family Python backend lands. -- Integration uses Forgejo Actions `services:` + socket-discovered bridge IPs +- Integration uses Fabled-Git Actions `services:` + socket-discovered bridge IPs because `act_runner` (swarm-runner v0.6+) puts services on the default bridge with no embedded DNS. The pattern is documented in the rulebook's - `forgejo.md` "CI philosophy" section and FC's `ci.yml` is the canonical + `fabled-git.md` "CI philosophy" section and FC's `ci.yml` is the canonical example. - No `package-lock.json` is tracked yet (FC's `feedback_no_local_runs` memory bans `npm install` locally). Using `npm install` rather than `npm ci` until a lockfile lands. - No `imagemagick` / `pandoc` per-job installs needed. +- `extension/`'s vitest specs load `lib/*.js` by evaluating the real file as a + classic script (`test/helpers/loadLib.js`) rather than adding `module.exports` + shims to production code — the libs ship as `background.scripts`, not ES + modules, so the specs exercise exactly the bytes packaged into the XPI. +- Extension test files are excluded from the XPI via `--ignore-files` in + `extension/package.json`, and the same paths are excluded from `ci.yml`'s + `extension-version` guard. Those two lists must agree — `test/version.spec.js` + asserts the guard never ignores a file web-ext actually packages. diff --git a/extension/lib/api.js b/extension/lib/api.js index a6b71b2..e66ea93 100644 --- a/extension/lib/api.js +++ b/extension/lib/api.js @@ -11,7 +11,10 @@ class FabledCuratorAPI { async init() { const cfg = await browser.storage.local.get(['apiUrl', 'apiKey']); - this.baseUrl = cfg.apiUrl || null; + // Normalize on READ, not just on save: configs stored before the options + // page started normalizing are missing the `/api` suffix, and this heals + // them without the operator having to reopen Settings. + this.baseUrl = normalizeApiUrl(cfg.apiUrl) || null; this.apiKey = cfg.apiKey || null; return this.isConfigured(); } @@ -50,6 +53,13 @@ class FabledCuratorAPI { } catch { message = `HTTP ${response.status}: ${response.statusText}`; } + // 404/405 from FC almost always means the request never reached the JSON + // API — it fell through to the SPA catch-all, which serves HTML on GET + // and rejects everything else. Say so, rather than making the operator + // decode "Method Not Allowed" on an endpoint that plainly allows POST. + if (response.status === 404 || response.status === 405) { + message += ` — ${url} isn't the FC API. Check the FC URL in settings.`; + } const err = new Error(message); err.status = response.status; throw err; @@ -96,11 +106,10 @@ class FabledCuratorAPI { return this.request('GET', '/extension/manifest'); } - // The web/SPA root: baseUrl with the trailing slash + `/api` suffix stripped. - // Where the Vue router (artist pages) and the served XPI live, NOT the JSON - // API. Used by OPEN_ARTIST_PAGE + the self-update check. + // The web/SPA root: where the Vue router (artist pages) and the served XPI + // live, NOT the JSON API. Used by OPEN_ARTIST_PAGE + the self-update check. webRoot() { - return (this.baseUrl || '').replace(/\/+$/, '').replace(/\/api$/, ''); + return webRootFromApiUrl(this.baseUrl); } // Connection test = the cheapest read with auth. diff --git a/extension/lib/url.js b/extension/lib/url.js new file mode 100644 index 0000000..2329adf --- /dev/null +++ b/extension/lib/url.js @@ -0,0 +1,32 @@ +/** + * Canonical FC endpoint derivation, shared by the background client and the + * options page so a URL entered either way behaves identically. + * + * FC serves two things on one origin: the JSON API under `/api`, and the Vue + * SPA from the root. `api.js` builds requests as `${baseUrl}/credentials`, so + * the stored base URL has to carry the `/api` suffix. + */ + +/** + * Accept what an operator would naturally type — the instance root + * (`http://curator.example.com`) or the API root (`.../api`) — and return the + * API root either way. + * + * Worth normalizing rather than validating: a root-form URL doesn't fail + * loudly, it lands on the SPA catch-all, which answers `GET /credentials` with + * 200 HTML and rejects `POST /credentials` with 405. The operator sees a + * working Test Connection and a broken export. + */ +function normalizeApiUrl(raw) { + const trimmed = (raw || '').trim().replace(/\/+$/, ''); + if (!trimmed) return ''; + return /\/api$/i.test(trimmed) ? trimmed : `${trimmed}/api`; +} + +/** + * The SPA root — where the Vue router (artist pages) and the served XPI live, + * NOT the JSON API. Accepts either input form, same as normalizeApiUrl. + */ +function webRootFromApiUrl(raw) { + return normalizeApiUrl(raw).replace(/\/api$/i, ''); +} diff --git a/extension/manifest.json b/extension/manifest.json index 4348ba1..e763e3f 100644 --- a/extension/manifest.json +++ b/extension/manifest.json @@ -1,7 +1,7 @@ { "manifest_version": 3, "name": "FabledCurator", - "version": "1.0.9", + "version": "1.0.10", "description": "Export cookies from supported platforms to FabledCurator and add creators as sources in one click.", "browser_specific_settings": { @@ -46,7 +46,7 @@ }, "background": { - "scripts": ["lib/platforms.js", "lib/cookies.js", "lib/api.js", "background/background.js"] + "scripts": ["lib/platforms.js", "lib/cookies.js", "lib/url.js", "lib/api.js", "background/background.js"] }, "options_ui": { diff --git a/extension/options/options.html b/extension/options/options.html index 27e5136..9af858c 100644 --- a/extension/options/options.html +++ b/extension/options/options.html @@ -21,9 +21,12 @@

FabledCurator extension

- - -
Find this on FC → Settings → Maintenance → Browser extension.
+ + +
+ Your FabledCurator address — with or without the trailing /api; both work. + Find it on FC → Settings → Maintenance → Browser extension. +
@@ -36,6 +39,7 @@ + diff --git a/extension/options/options.js b/extension/options/options.js index 4798183..e453273 100644 --- a/extension/options/options.js +++ b/extension/options/options.js @@ -8,7 +8,7 @@ document.addEventListener('DOMContentLoaded', async () => { }); async function save() { - const apiUrl = document.getElementById('api-url').value.trim().replace(/\/+$/, ''); + const apiUrl = normalizeApiUrl(document.getElementById('api-url').value); const apiKey = document.getElementById('api-key').value.trim(); if (!apiUrl || !apiKey) { showStatus('Both fields are required.', 'err'); @@ -16,11 +16,14 @@ async function save() { } await browser.storage.local.set({ apiUrl, apiKey }); await browser.storage.local.remove(['lastConnectionTest', 'lastConnectionStatus']); - showStatus('Saved.', 'ok'); + // Show what was actually stored — the operator may have typed the instance + // root and it was normalized to the API root. + document.getElementById('api-url').value = apiUrl; + showStatus(`Saved — using ${apiUrl}`, 'ok'); } async function test() { - const apiUrl = document.getElementById('api-url').value.trim().replace(/\/+$/, ''); + const apiUrl = normalizeApiUrl(document.getElementById('api-url').value); const apiKey = document.getElementById('api-key').value.trim(); if (!apiUrl || !apiKey) { showStatus('Fill both fields first.', 'err'); @@ -31,8 +34,23 @@ async function test() { method: 'GET', headers: { 'X-Extension-Key': apiKey }, }); - if (r.ok) showStatus(`Connected — HTTP ${r.status}.`, 'ok'); - else showStatus(`HTTP ${r.status}: ${r.statusText}`, 'err'); + if (!r.ok) { + showStatus(`HTTP ${r.status}: ${r.statusText}`, 'err'); + return; + } + // A 200 is NOT sufficient. If the URL resolves to the Vue SPA instead of + // the JSON API, the catch-all route returns 200 with an HTML document — + // which used to report "Connected" on a config that could not POST at all. + const contentType = r.headers.get('content-type') || ''; + if (!contentType.includes('json')) { + showStatus( + `${apiUrl} answered with ${contentType || 'no content-type'}, not JSON ` + + '— that looks like the FC web UI rather than its API.', + 'err', + ); + return; + } + showStatus(`Connected to ${apiUrl} — HTTP ${r.status}.`, 'ok'); } catch (e) { showStatus(`Cannot reach ${apiUrl}: ${e.message}`, 'err'); } diff --git a/extension/package.json b/extension/package.json index 16a9858..c441e82 100644 --- a/extension/package.json +++ b/extension/package.json @@ -1,15 +1,17 @@ { "name": "fabledcurator-extension", - "version": "1.0.9", + "version": "1.0.10", "private": true, "description": "Firefox extension for FabledCurator", "scripts": { - "lint": "web-ext lint --source-dir=. --no-config-discovery --ignore-files package.json package-lock.json web-ext-artifacts node_modules README.md .gitignore", - "start": "web-ext run --source-dir=. --no-config-discovery --ignore-files package.json package-lock.json web-ext-artifacts node_modules README.md .gitignore --firefox=firefox", - "build": "web-ext build --source-dir=. --no-config-discovery --ignore-files package.json package-lock.json web-ext-artifacts node_modules README.md .gitignore --overwrite-dest", - "sign": "web-ext sign --source-dir=. --no-config-discovery --ignore-files package.json package-lock.json web-ext-artifacts node_modules README.md .gitignore --channel=unlisted --api-key=$WEB_EXT_API_KEY --api-secret=$WEB_EXT_API_SECRET" + "lint": "web-ext lint --source-dir=. --no-config-discovery --ignore-files package.json package-lock.json web-ext-artifacts node_modules README.md .gitignore vitest.config.js \"test/**\"", + "start": "web-ext run --source-dir=. --no-config-discovery --ignore-files package.json package-lock.json web-ext-artifacts node_modules README.md .gitignore vitest.config.js \"test/**\" --firefox=firefox", + "build": "web-ext build --source-dir=. --no-config-discovery --ignore-files package.json package-lock.json web-ext-artifacts node_modules README.md .gitignore vitest.config.js \"test/**\" --overwrite-dest", + "sign": "web-ext sign --source-dir=. --no-config-discovery --ignore-files package.json package-lock.json web-ext-artifacts node_modules README.md .gitignore vitest.config.js \"test/**\" --channel=unlisted --api-key=$WEB_EXT_API_KEY --api-secret=$WEB_EXT_API_SECRET", + "test:unit": "vitest run" }, "devDependencies": { + "vitest": "^4.0.0", "web-ext": "^10.0.0" } } diff --git a/extension/test/helpers/loadLib.js b/extension/test/helpers/loadLib.js new file mode 100644 index 0000000..e5ded02 --- /dev/null +++ b/extension/test/helpers/loadLib.js @@ -0,0 +1,29 @@ +import { readFileSync } from 'node:fs' +import { fileURLToPath } from 'node:url' +import path from 'node:path' + +const LIB_DIR = path.join(path.dirname(fileURLToPath(import.meta.url)), '..', '..', 'lib') + +/** + * Load an extension lib and hand back the globals it declares. + * + * The files under lib/ are CLASSIC scripts, not ES modules: manifest.json + * lists them in `background.scripts` and options.html pulls them in with a + * plain