Commit Graph
4 Commits
Author SHA1 Message Date
bvandeusen fe48e77821 ci(extension): retire the manual-bump guard, true up the docs (step 5)
CI / lint (push) Successful in 3s
Build images / sign-extension (push) Successful in 4s
CI / extension-version (push) Successful in 3s
Build images / build-agent (push) Successful in 8s
CI / frontend-build (push) Successful in 22s
extension / lint (push) Successful in 25s
CI / backend-lint-and-test (push) Successful in 29s
Build images / build-web (push) Successful in 1m57s
Build images / build-ml (push) Successful in 2m38s
CI / integration (push) Successful in 3m50s
The guard asked whether a packaged extension file changed without the
version moving. Since step 4 nobody moves the version by hand, so it was
checking a fact that had stopped existing — and it was not merely dead
weight: it would have failed the lane on every real extension change,
demanding a bump that decides nothing. Removed rather than left running
beside the new mechanism (rule 22).

What replaces it is thinner and true. The extension-version lane now
asserts the derivation resolves on this commit, that the derived value is
the plain dotted-numeric shape AMO accepts, and that MAJOR.MINOR agrees
between manifest.json and package.json. MAJOR.MINOR is the one part still
hand-set, and packaging.sh reads it from manifest.json ALONE, so a
divergence ships a version package.json disagrees with. The lane keeps
fetch-depth: 0 — checking that the derivation survives a real checkout is
half its remaining value.

Deliberately not checked there: that the derived value beats what is
already signed. That guard belongs in build.yml, where it compares against
the real ext-* releases. Comparing against origin/main in a lane would be
wrong, because dev legitimately derives a LOWER value whenever main is
ahead on the extension, and a lane that fails for being behind is a lane
people learn to ignore.

packaging.sh is down to two consumers from three. version.spec.js's
"ci.yml derives its pathspec" test would have gone red on that, so it is
rewritten to assert the property rather than the consumer: no workflow
inlines an :(exclude)extension/ literal, across all three. That keeps the
#2397 anti-regression value while surviving consumers coming and going.
A second test pins build.yml to packaging.sh version and fails if it goes
back to grepping the committed value — which is not a style regression but
the #3092 bug itself. build.yml joins extension.yml's trigger paths, since
the suite now asserts against it.

The lockstep test narrows from the whole version string to MAJOR.MINOR.
The committed patch numbers are inert now; asserting on them would fail
for a difference that changes nothing.

Docs. extension/README.md's Release section described extension.yml
signing on main and committing the XPI into frontend/public/ — untrue
since 2026-05-25, and it told the reader to hand-bump both files, which is
now exactly the wrong instruction. Rewritten, with a Versioning section
that says plainly that editing the patch number does nothing and why the
key is commit time rather than a count. ci-requirements.md drops the third
packaging.sh consumer and names every job that needs full history. Root
README no longer claims the extension is signed on main only.
2026-08-27 11:29:44 -04:00
Claude 11dd324f89 fix(extension): exclude the test/ and scripts/ directory entries from the XPI
CI / lint (push) Successful in 2s
CI / extension-version (push) Successful in 3s
extension / lint (push) Successful in 19s
CI / frontend-build (push) Successful in 20s
CI / backend-lint-and-test (push) Successful in 38s
CI / integration (push) Successful in 3m57s
extension / lint (pull_request) Successful in 34s
The XPI-content check added in 1c6452e did its job on its first run: the
archive carried empty `test/` and `scripts/` entries.

`test/**` matches the files inside a directory but not the directory entry
itself, and web-ext writes an entry for the directory separately -- so the
contents were correctly excluded while the empty directories shipped anyway.
Nothing harmful reached users (no dev code, just two empty entries), but our
single declaration claimed these do not ship and something was shipping.

Both forms are now listed per directory. The bare name alone would not do:
minimatch's `test` does not match `test/url.spec.js`, so dropping the glob
would ship the contents instead.

Verified locally: the derived version is unchanged at 1.0.19, confirming the
added entries are redundant for the git pathspec and only affect web-ext.

Refs #2400

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-03 16:20:54 -04:00
Claude 597b91d29b refactor(extension): one definition of what ships in the XPI
CI / lint (push) Successful in 3s
CI / extension-version (push) Successful in 4s
extension / lint (push) Successful in 20s
CI / frontend-build (push) Successful in 23s
CI / backend-lint-and-test (push) Successful in 44s
CI / integration (push) Successful in 3m59s
Milestone #271 step 1. Groundwork for deriving the extension version from git;
no behavior change yet -- nothing consumes `version` so far.

"Which files end up in the XPI" was stated in two places and about to become
three. Three hand-kept copies of one fact is what allowed #2397, where the
publish path could republish a stale XPI because its cache key had no link to
the content it stood for.

New extension/scripts/packaging.sh holds the single declaration and exposes:
  ignore       web-ext --ignore-files values
  pathspec     :(exclude)extension/... for git
  version      <MAJOR.MINOR from manifest>.<commit count over packaged files>
  major-minor  /  patch

Consumers now delegate instead of restating it:
- extension/package.json -- all four web-ext scripts
- .forgejo/workflows/ci.yml -- the extension-version guard's exclusions
- (step 4) the rev-list that derives the version

scripts/** joins the non-packaged set; the script must not ship to users.

Two shell hazards, both load-bearing:

The script runs `set -euf`. Its lists are iterated with deliberate word
splitting, and without -f the shell ALSO globs them -- invoking `pathspec`
from a directory where test/ exists (exactly how ci.yml calls it) would expand
`test/**` into the individual spec files and silently stop covering anything
added later. A caller's own `set -f` cannot prevent this: the script is a
separate sh process and does not inherit it.

Callers additionally need their own `set -f` for the substituted RESULT, which
is a different expansion. version.spec.js asserts every --ignore-files caller
sets it, that the pathspec comes through with `test/**` literal and no
.spec.js paths, and that neither consumer has reinstated a hardcoded list --
the easy future regression is "simplifying" by inlining one again.

Verified: all five subcommands plus the usage/exit-2 path. Derived version on
main (8300029) is 1.0.19, matching dev. Last published is 1.0.10, so the
eventual cutover moves strictly upward and needs no offset -- Firefox refuses
downgrades. (An earlier note recorded 18; that was measured against a stale
origin/main from before the PR #234 merge.)

Refs #2398

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-03 15:33:11 -04:00
Claude f9111c06a7 test(extension): unit suite for lib/ + version-consistency specs
CI / lint (push) Successful in 3s
CI / extension-version (push) Successful in 4s
CI / frontend-build (push) Successful in 21s
extension / lint (push) Successful in 19s
CI / backend-lint-and-test (push) Successful in 43s
CI / integration (push) Successful in 3m53s
extension / lint (pull_request) Successful in 19s
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>
2026-08-02 23:47:18 -04:00