Extension: fix credential-push 405, guard the publish path, add a unit suite #234

Merged

bvandeusen merged 4 commits from dev into main 2026-08-03 08:29:09 -04:00

Conversation 0 Commits 4 Files Changed 14

+569 -26

bvandeusen commented 2026-08-03 08:28:59 -04:00

Owner

Copy Link

Copy Source

Extension credential push returned 405 (#2393)

The stored apiUrl had to already carry the /api suffix — lib/api.js builds requests as ${baseUrl}/credentials — but the options label read "FC base URL", so entering the instance root sent every request one path segment short. POST /credentials hit the Vue SPA catch-all and came back 405; GET /extension/manifest 404'd.

The status code was misleading because /credentials is also a Vue router path: the catch-all answers GET with 200 HTML and only rejects the POST.

Compounding it, Test Connection reported success on the broken config — it passed on r.ok alone, and against the SPA fallback that's a 200 with an HTML body. The one affordance meant to catch this was masking it.

Fixed by normalizing rather than validating: new lib/url.js accepts either the instance root or the API root; api.js normalizes on read so already-broken stored configs heal without reopening Settings; the connection test now asserts a JSON content-type.

Publish path could silently ship a stale XPI (#2397)

build.yml's sign-extension keys its AMO-signing cache purely on the version in extension/package.json. If an ext-<version> release already has an XPI, signing is skipped and that old XPI is baked into :latest. Nothing in that path inspects whether extension/ changed — so a forgotten bump ships stale code on a fully green build, and that's the default outcome of forgetting. AMO can't backstop it: it 409s on re-signing a version, which is why the cache exists.

New extension-version CI job fails the push when the manifest and package versions disagree, or when a packaged file changed without a version bump. Compares against main (where the publish decision is actually made) rather than the previous push, so iterating on dev doesn't demand repeated bumps.

No test harness existed for extension/

web-ext lint was the only signal. Adds vitest and 34 tests: URL normalization, platform/artist-page matching (pinning the #1485 Patreon /c/ + /cw/ regression), and version-consistency checks. Specs evaluate the real lib/*.js files as classic scripts, so they exercise exactly the bytes packaged into the XPI.

Wiring this up surfaced that web-ext would otherwise have bundled test/ and vitest.config.js into the shipped XPI; both are now in --ignore-files, and a spec asserts the CI guard never ignores a file web-ext actually packages.

---

On merge: version 1.0.10 has no ext-1.0.10 release, so sign-extension will do a real AMO round trip (1–5 min) before build-web — this build is slower than usual by design. :latest will then carry the fixed extension.

Also included: 306de50 (pre-existing dev-only docs commit, unrelated to the above).

Follow-up planned in Scribe milestone #271 (derive the extension version from git, retiring the manual bump entirely). Not in this PR.

## Extension credential push returned 405 (#2393) The stored `apiUrl` had to already carry the `/api` suffix — `lib/api.js` builds requests as `${baseUrl}/credentials` — but the options label read "FC base URL", so entering the instance root sent every request one path segment short. `POST /credentials` hit the Vue SPA catch-all and came back **405**; `GET /extension/manifest` **404**'d. The status code was misleading because `/credentials` is also a Vue router path: the catch-all answers GET with 200 HTML and only rejects the POST. Compounding it, **Test Connection reported success on the broken config** — it passed on `r.ok` alone, and against the SPA fallback that's a 200 with an HTML body. The one affordance meant to catch this was masking it. Fixed by normalizing rather than validating: new `lib/url.js` accepts either the instance root or the API root; `api.js` normalizes on *read* so already-broken stored configs heal without reopening Settings; the connection test now asserts a JSON content-type. ## Publish path could silently ship a stale XPI (#2397) `build.yml`'s `sign-extension` keys its AMO-signing cache purely on the version in `extension/package.json`. If an `ext-<version>` release already has an XPI, signing is skipped and that **old** XPI is baked into `:latest`. Nothing in that path inspects whether `extension/` changed — so a forgotten bump ships stale code on a fully green build, and that's the default outcome of forgetting. AMO can't backstop it: it 409s on re-signing a version, which is why the cache exists. New `extension-version` CI job fails the push when the manifest and package versions disagree, or when a packaged file changed without a version bump. Compares against `main` (where the publish decision is actually made) rather than the previous push, so iterating on `dev` doesn't demand repeated bumps. ## No test harness existed for `extension/` `web-ext lint` was the only signal. Adds vitest and 34 tests: URL normalization, platform/artist-page matching (pinning the #1485 Patreon `/c/` + `/cw/` regression), and version-consistency checks. Specs evaluate the real `lib/*.js` files as classic scripts, so they exercise exactly the bytes packaged into the XPI. Wiring this up surfaced that web-ext would otherwise have bundled `test/` and `vitest.config.js` **into the shipped XPI**; both are now in `--ignore-files`, and a spec asserts the CI guard never ignores a file web-ext actually packages. --- **On merge:** version `1.0.10` has no `ext-1.0.10` release, so `sign-extension` will do a real AMO round trip (1–5 min) before `build-web` — this build is slower than usual by design. `:latest` will then carry the fixed extension. Also included: `306de50` (pre-existing dev-only docs commit, unrelated to the above). Follow-up planned in Scribe milestone #271 (derive the extension version from git, retiring the manual bump entirely). Not in this PR.

bvandeusen added 4 commits 2026-08-03 08:29:00 -04:00

docs: Fabled-Git, not Forgejo, in ci-requirements ... 306de50f61

The instance has run Gitea since the migration. Also fixes a dead rulebook
pointer: the topic was renamed forgejo.md -> fabled-git.md, so the
"CI philosophy" reference pointed at a file that no longer exists.

Prose only — no workflow or path change. Scribe issue #2272.

fix(extension): normalize FC URL so credential push doesn't 405 ... 8214afee1e

The stored apiUrl was required to already carry the `/api` suffix, since
api.js builds requests as `${baseUrl}/credentials`. The options label read
"FC base URL", so entering the instance root -- the natural reading --
sent every request one path segment short: POST /credentials hit the Vue
SPA catch-all and came back 405, and GET /extension/manifest 404'd.

Worse, Test Connection reported success on it: the catch-all answers GET
/credentials with 200 HTML, so `r.ok` was true and the only affordance
meant to catch this misconfiguration actively masked it.

Normalize instead of validate (rules 92, 26):

- New lib/url.js: normalizeApiUrl / webRootFromApiUrl, one source shared
  by the background client and the options page. Accepts either the
  instance root or the API root.
- api.js normalizes on read, so configs already stored in the broken form
  heal themselves without the operator reopening Settings.
- options.js stores the canonical form, echoes back what it saved, and
  the test now asserts a JSON content-type -- killing the false green.
- 404/405 in request() now names the URL and points at the setting.
- Options label/placeholder state that both forms work.

Version 1.0.9 -> 1.0.10 in BOTH manifest.json and package.json; build.yml
resolves the release version from package.json, and a stale value there
would hit the cached ext-1.0.9 asset and republish the old XPI unsigned
against the new code.

Refs #2393

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

ci: guard the extension publish path against a missed version bump ... c37a180c3c

build.yml's sign-extension keys its AMO-signing cache purely on the version
string in extension/package.json. If an ext-<version> release already has an
XPI, signing is skipped and build-web bakes that OLD signed XPI into :latest.
Nothing in that path inspects whether extension/ actually changed, so a
forgotten bump ships a stale extension on a fully green build -- silently, and
as the default outcome of forgetting. AMO can't backstop it either: it 409s on
re-signing a version, which is precisely why the cache exists.

New extension-version job, pure git + text, no deps or services:

1. Unconditional consistency check. manifest.json and package.json versions
   must match. web-ext sign reads manifest.json (package.json is in
   --ignore-files and isn't even inside the XPI), so AMO signs the manifest
   version; build.yml keys its cache, release tag, XPI filename -- and so the
   version /api/extension/manifest reports to the update prompt -- on
   package.json. Divergence either 409s at AMO or ships an XPI whose update
   prompt lies about what's installed.

2. Changed-without-bump check. If any PACKAGED file under extension/ differs,
   the version must have moved. Exclusions mirror --ignore-files so a Renovate
   web-ext devDep bump in package.json doesn't falsely demand one.

Compared against main rather than the previous push: the publish decision is
made at merge-to-main against whatever ext-<version> exists, so "differs from
main" is the question that matters. Diffing against the previous dev push
would demand a fresh bump on every iteration, inflating the version to buy
nothing.

Bumping stays manual -- making it automatic requires rewriting the version in
CI and committing back to a protected branch, which this workflow deliberately
avoided. This only ensures a missed bump can no longer be silent.

Refs #2393

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

test(extension): unit suite for lib/ + version-consistency specs ... f9111c06a7

extension/ had no test harness at all -- web-ext lint was the only signal, so
the URL-normalization fix in 8214afe shipped with nothing exercising it.

Adds vitest (mirroring frontend/vitest.config.js) and three specs:

- url.spec.js       normalizeApiUrl / webRootFromApiUrl, including the #2393
                    regression: instance-root input must reach /api/credentials,
                    idempotence, trailing-slash and whitespace handling, and
                    that empty input never yields a bare "/api" (which
                    isConfigured() would read as configured).
- platforms.spec.js getPlatformFromUrl / isArtistPage, pinning the #1485
                    regression -- all three Patreon creator URL shapes
                    (bare, /c/, /cw/) plus inner pages, with nav pages
                    excluded -- and table-integrity checks.
- version.spec.js   manifest.json and package.json versions in lockstep,
                    AMO-safe version format, and url.js ordered before api.js
                    in background.scripts (classic scripts share one scope, so
                    a reorder is a runtime ReferenceError with no build signal).

Specs load lib/*.js by evaluating the real file as a classic script
(test/helpers/loadLib.js) instead of adding module.exports shims to production
code that would never run in the browser. The suite therefore exercises exactly
the bytes packaged into the XPI.

Two packaging consequences, both handled:

- web-ext would otherwise bundle test/ and vitest.config.js INTO the XPI;
  both are now in --ignore-files across all four web-ext scripts.
- ci.yml's extension-version guard must ignore the same paths, or editing a
  spec would demand a pointless version bump. The dangerous drift direction is
  the opposite one -- a guard exclusion for a file that DOES ship would let a
  real change pass unnoticed -- so version.spec.js asserts every :(exclude) in
  ci.yml appears in --ignore-files.

extension.yml also triggers on ci.yml now, since version.spec.js reads it.

Refs #2393, #2397

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

bvandeusen merged commit 8300029741 into main 2026-08-03 08:29:09 -04:00

bvandeusen referenced this issue from a commit 2026-08-03 15:33:13 -04:00

refactor(extension): one definition of what ships in the XPI

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

No items

No labels

Milestone

No items

No Milestone

Projects

Clear projects

No projects

Assignees

Clear assignees

renovate-bot bvandeusen

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: bvandeusen/FabledCurator#234