diff --git a/.forgejo/workflows/build.yml b/.forgejo/workflows/build.yml index 72660ca..42a9a72 100644 --- a/.forgejo/workflows/build.yml +++ b/.forgejo/workflows/build.yml @@ -75,18 +75,23 @@ jobs: fetch-depth: 0 # The version is DERIVED, not read from the repo (milestone 271 step 4, - # cut over 2026-08-27). `packaging.sh version` returns MAJOR.MINOR from - # manifest.json plus a patch component that is the commit TIME of the - # newest change to a PACKAGED extension file, in minutes since - # 2020-01-01 — family rule 149, never a commit count, which orders by + # cut over 2026-08-27). `packaging.sh version` returns `YYYY.M.D.HHMM` + # UTC — the commit TIME of the newest change to a PACKAGED extension + # file, per family rule 148/149. Never a commit count, which orders by # branch rather than by recency. # - # The committed "version" in manifest.json / package.json no longer - # decides anything: the stamp step below overwrites it in the working - # tree before web-ext ever reads it. It is deliberately NOT committed - # back — the commit carrying the bump would itself be a change to the - # extension and would move the version again. The repo holds the source; - # the build derives the label. + # Unpadded, and only here: AMO's grammar rejects a leading zero, so the + # extension renders rule 148's numbers without the family's padding + # (milestone 318 step 8). Same value, one character narrower per segment; + # ci.yml's extension-version lane checks the string against Mozilla's + # published regex before this job ever calls AMO. + # + # The committed "version" in manifest.json / package.json decides NOTHING + # — not even a MAJOR.MINOR prefix, which step 8 removed. The stamp step + # below overwrites it in the working tree before web-ext ever reads it, + # and it is deliberately NOT committed back: the commit carrying the bump + # would itself be a change to the extension and would move the version + # again. The repo holds the source; the build derives the label. - name: Derive extension version id: extver run: | @@ -476,7 +481,6 @@ jobs: env: IMAGE: git.fabledsword.com/bvandeusen/fabledcurator CHANNEL: ${{ steps.tag.outputs.channel }} - TAGS: ${{ steps.tag.outputs.tags }} run: | set -eu DERIVED=$(sh scripts/artifacts.sh revision web) @@ -485,7 +489,6 @@ jobs: # A pure function of the revision — same commit, same string — so it # adds no variability the reuse check would have to account for. echo "version=$(sh scripts/artifacts.sh version web)" >> "$GITHUB_OUTPUT" - echo "build_tags=$TAGS" >> "$GITHUB_OUTPUT" # The moving tag for this channel. Which tag we ask IS the channel — # that is why the revision needs no -main/-dev qualifier any more. @@ -608,7 +611,13 @@ jobs: context: . file: Dockerfile push: true - tags: ${{ steps.reuse.outputs.build_tags }} + # ONE tag, the channel's. Every other tag is written by the step + # below, registry-side. buildx here pushes the first tag to the + # registry and then re-pushes the rest through the DOCKER driver, + # out of a local image store a registry-direct build never filled — + # #3190, which cost `main` its :c- on 2026-08-29 while :latest + # published perfectly well. + tags: ${{ steps.reuse.outputs.channel_ref }} # The reuse key. Read back off the channel tag on the next push to # decide whether that push needs to build at all, so this is not # decoration — an unstamped image is one that will always rebuild. @@ -621,17 +630,40 @@ jobs: FC_CHANNEL=${{ steps.tag.outputs.channel }} FC_VERSION=${{ steps.reuse.outputs.version }} - # Registry-side manifest copy: no layer transfer, no local daemon, no - # rebuild. Each -t becomes another reference to the SAME manifest the - # channel tag already holds, so :c- is byte-identical to what is - # published rather than a lookalike rebuild. + # Every tag but the channel's own is written HERE, registry-side, + # whether or not a build ran. Each -t becomes another reference to the + # SAME manifest the channel tag holds, so :c- is byte-identical to + # what is published rather than a lookalike rebuild. # - # Runs on EVERY reuse, which is what keeps family rule 146 true: a - # rolling channel refreshes itself, so skipping a build must never mean - # leaving :dev or :latest pointing at something older than the commit - # that was just pushed. - - name: Repoint the tags at the published image (reuse) - if: steps.reuse.outputs.hit == 'true' + # Owning the build path too is #3190's fix, not a tidy-up: + # + # #27 pushing …/fabledcurator:latest DONE 15.8s + # #28 pushing …/fabledcurator:c-0e15c44 with docker + # #28 ERROR: tag does not exist: …:c-0e15c44 + # + # Intermittent — build-ml made the identical two-tag push seconds later + # and succeeded — and worse than it looks. `:latest` had already + # published, so production was correct while the immutable rollback tag + # rule 145 requires of every main push simply did not exist. Nothing but + # the red job would ever have noticed: a missing :c- has no + # consumer that fails, so it surfaces when somebody needs to roll back. + # + # `imagetools create` is a registry-side manifest copy — no layer + # transfer, no local daemon, nothing that can be absent. The reuse case + # has always gone this way, so this puts the build case on the code that + # was already proven rather than on a second path. + # + # Running on every path also keeps family rule 146 true: a rolling + # channel refreshes itself, so skipping a build must never leave :dev or + # :latest pointing at something older than the commit just pushed. + # + # The cost, accepted knowingly: `imagetools create` wraps its source in + # an index, so :c- becomes an index and fc.revision does not + # resolve through it. Nothing reads that label off :c- — the reuse + # check only ever inspects the CHANNEL tag — and the index names the + # same manifest, so a pull is byte-identical. The reuse path already + # produced :c- this way; this only makes it uniform. + - name: Write the remaining tags from the published image env: IMAGE: git.fabledsword.com/bvandeusen/fabledcurator SOURCE: ${{ steps.reuse.outputs.channel_ref }} @@ -650,15 +682,16 @@ jobs: # ml:dev reported fc.revision= one push after run 4749 had read # a7e626a67a79 off it. Nothing failed; the savings just evaporated. # - # Excluding the source means the channel tag is only ever written by - # a real build, so it stays a plain image and stays readable. On dev - # that leaves nothing to do — :dev already points at the right - # content, which is what the hit established. On main it leaves - # :c-, which rule 145 requires of every main push whether or not - # a build ran. + # Excluding the source means the channel tag is only ever written + # by a real build, so it stays a plain image and stays readable. + # On dev that leaves nothing to do either way: the build pushed :dev + # itself, or the hit established it was already right. On main it + # leaves :c-, which rule 145 requires of every main push whether + # or not a build ran. # - # steps.tag emits ONE comma-separated list, because that is the shape - # docker/build-push-action takes; imagetools wants a -t per ref. + # steps.tag emits ONE comma-separated list; imagetools wants a -t per + # ref. (That list used to feed docker/build-push-action directly — + # which is exactly what #3190 made unsafe.) ARGS="" IFS=, for t in $TAGS; do @@ -667,8 +700,8 @@ jobs: done unset IFS if [ -z "$ARGS" ]; then - echo "repoint: $SOURCE already carries this revision and is the" - echo "repoint: only tag for this channel — nothing to write." + echo "repoint: $SOURCE is the only tag for this channel and" + echo "repoint: already holds this revision — nothing to write." exit 0 fi # shellcheck disable=SC2086 @@ -781,12 +814,10 @@ jobs: env: IMAGE: git.fabledsword.com/bvandeusen/fabledcurator-ml CHANNEL: ${{ steps.tag.outputs.channel }} - TAGS: ${{ steps.tag.outputs.tags }} run: | set -eu DERIVED=$(sh scripts/artifacts.sh revision ml) echo "revision=$DERIVED" >> "$GITHUB_OUTPUT" - echo "build_tags=$TAGS" >> "$GITHUB_OUTPUT" # The moving tag for this channel. Which tag we ask IS the channel — # that is why the revision needs no -main/-dev qualifier any more. @@ -833,24 +864,53 @@ jobs: context: . file: Dockerfile.ml push: true - tags: ${{ steps.reuse.outputs.build_tags }} + # ONE tag, the channel's. Every other tag is written by the step + # below, registry-side. buildx here pushes the first tag to the + # registry and then re-pushes the rest through the DOCKER driver, + # out of a local image store a registry-direct build never filled — + # #3190, which cost `main` its :c- on 2026-08-29 while :latest + # published perfectly well. + tags: ${{ steps.reuse.outputs.channel_ref }} # The reuse key. Read back off the channel tag on the next push to # decide whether that push needs to build at all, so this is not # decoration — an unstamped image is one that will always rebuild. labels: | fc.revision=${{ steps.reuse.outputs.revision }} - # Registry-side manifest copy: no layer transfer, no local daemon, no - # rebuild. Each -t becomes another reference to the SAME manifest the - # channel tag already holds, so :c- is byte-identical to what is - # published rather than a lookalike rebuild. + # Every tag but the channel's own is written HERE, registry-side, + # whether or not a build ran. Each -t becomes another reference to the + # SAME manifest the channel tag holds, so :c- is byte-identical to + # what is published rather than a lookalike rebuild. # - # Runs on EVERY reuse, which is what keeps family rule 146 true: a - # rolling channel refreshes itself, so skipping a build must never mean - # leaving :dev or :latest pointing at something older than the commit - # that was just pushed. - - name: Repoint the tags at the published image (reuse) - if: steps.reuse.outputs.hit == 'true' + # Owning the build path too is #3190's fix, not a tidy-up: + # + # #27 pushing …/fabledcurator:latest DONE 15.8s + # #28 pushing …/fabledcurator:c-0e15c44 with docker + # #28 ERROR: tag does not exist: …:c-0e15c44 + # + # Intermittent — build-ml made the identical two-tag push seconds later + # and succeeded — and worse than it looks. `:latest` had already + # published, so production was correct while the immutable rollback tag + # rule 145 requires of every main push simply did not exist. Nothing but + # the red job would ever have noticed: a missing :c- has no + # consumer that fails, so it surfaces when somebody needs to roll back. + # + # `imagetools create` is a registry-side manifest copy — no layer + # transfer, no local daemon, nothing that can be absent. The reuse case + # has always gone this way, so this puts the build case on the code that + # was already proven rather than on a second path. + # + # Running on every path also keeps family rule 146 true: a rolling + # channel refreshes itself, so skipping a build must never leave :dev or + # :latest pointing at something older than the commit just pushed. + # + # The cost, accepted knowingly: `imagetools create` wraps its source in + # an index, so :c- becomes an index and fc.revision does not + # resolve through it. Nothing reads that label off :c- — the reuse + # check only ever inspects the CHANNEL tag — and the index names the + # same manifest, so a pull is byte-identical. The reuse path already + # produced :c- this way; this only makes it uniform. + - name: Write the remaining tags from the published image env: IMAGE: git.fabledsword.com/bvandeusen/fabledcurator-ml SOURCE: ${{ steps.reuse.outputs.channel_ref }} @@ -869,15 +929,16 @@ jobs: # ml:dev reported fc.revision= one push after run 4749 had read # a7e626a67a79 off it. Nothing failed; the savings just evaporated. # - # Excluding the source means the channel tag is only ever written by - # a real build, so it stays a plain image and stays readable. On dev - # that leaves nothing to do — :dev already points at the right - # content, which is what the hit established. On main it leaves - # :c-, which rule 145 requires of every main push whether or not - # a build ran. + # Excluding the source means the channel tag is only ever written + # by a real build, so it stays a plain image and stays readable. + # On dev that leaves nothing to do either way: the build pushed :dev + # itself, or the hit established it was already right. On main it + # leaves :c-, which rule 145 requires of every main push whether + # or not a build ran. # - # steps.tag emits ONE comma-separated list, because that is the shape - # docker/build-push-action takes; imagetools wants a -t per ref. + # steps.tag emits ONE comma-separated list; imagetools wants a -t per + # ref. (That list used to feed docker/build-push-action directly — + # which is exactly what #3190 made unsafe.) ARGS="" IFS=, for t in $TAGS; do @@ -886,8 +947,8 @@ jobs: done unset IFS if [ -z "$ARGS" ]; then - echo "repoint: $SOURCE already carries this revision and is the" - echo "repoint: only tag for this channel — nothing to write." + echo "repoint: $SOURCE is the only tag for this channel and" + echo "repoint: already holds this revision — nothing to write." exit 0 fi # shellcheck disable=SC2086 @@ -998,12 +1059,10 @@ jobs: env: IMAGE: git.fabledsword.com/bvandeusen/fabledcurator-agent CHANNEL: ${{ steps.tag.outputs.channel }} - TAGS: ${{ steps.tag.outputs.tags }} run: | set -eu DERIVED=$(sh scripts/artifacts.sh revision agent) echo "revision=$DERIVED" >> "$GITHUB_OUTPUT" - echo "build_tags=$TAGS" >> "$GITHUB_OUTPUT" # The moving tag for this channel. Which tag we ask IS the channel — # that is why the revision needs no -main/-dev qualifier any more. @@ -1050,24 +1109,53 @@ jobs: context: agent file: agent/Dockerfile push: true - tags: ${{ steps.reuse.outputs.build_tags }} + # ONE tag, the channel's. Every other tag is written by the step + # below, registry-side. buildx here pushes the first tag to the + # registry and then re-pushes the rest through the DOCKER driver, + # out of a local image store a registry-direct build never filled — + # #3190, which cost `main` its :c- on 2026-08-29 while :latest + # published perfectly well. + tags: ${{ steps.reuse.outputs.channel_ref }} # The reuse key. Read back off the channel tag on the next push to # decide whether that push needs to build at all, so this is not # decoration — an unstamped image is one that will always rebuild. labels: | fc.revision=${{ steps.reuse.outputs.revision }} - # Registry-side manifest copy: no layer transfer, no local daemon, no - # rebuild. Each -t becomes another reference to the SAME manifest the - # channel tag already holds, so :c- is byte-identical to what is - # published rather than a lookalike rebuild. + # Every tag but the channel's own is written HERE, registry-side, + # whether or not a build ran. Each -t becomes another reference to the + # SAME manifest the channel tag holds, so :c- is byte-identical to + # what is published rather than a lookalike rebuild. # - # Runs on EVERY reuse, which is what keeps family rule 146 true: a - # rolling channel refreshes itself, so skipping a build must never mean - # leaving :dev or :latest pointing at something older than the commit - # that was just pushed. - - name: Repoint the tags at the published image (reuse) - if: steps.reuse.outputs.hit == 'true' + # Owning the build path too is #3190's fix, not a tidy-up: + # + # #27 pushing …/fabledcurator:latest DONE 15.8s + # #28 pushing …/fabledcurator:c-0e15c44 with docker + # #28 ERROR: tag does not exist: …:c-0e15c44 + # + # Intermittent — build-ml made the identical two-tag push seconds later + # and succeeded — and worse than it looks. `:latest` had already + # published, so production was correct while the immutable rollback tag + # rule 145 requires of every main push simply did not exist. Nothing but + # the red job would ever have noticed: a missing :c- has no + # consumer that fails, so it surfaces when somebody needs to roll back. + # + # `imagetools create` is a registry-side manifest copy — no layer + # transfer, no local daemon, nothing that can be absent. The reuse case + # has always gone this way, so this puts the build case on the code that + # was already proven rather than on a second path. + # + # Running on every path also keeps family rule 146 true: a rolling + # channel refreshes itself, so skipping a build must never leave :dev or + # :latest pointing at something older than the commit just pushed. + # + # The cost, accepted knowingly: `imagetools create` wraps its source in + # an index, so :c- becomes an index and fc.revision does not + # resolve through it. Nothing reads that label off :c- — the reuse + # check only ever inspects the CHANNEL tag — and the index names the + # same manifest, so a pull is byte-identical. The reuse path already + # produced :c- this way; this only makes it uniform. + - name: Write the remaining tags from the published image env: IMAGE: git.fabledsword.com/bvandeusen/fabledcurator-agent SOURCE: ${{ steps.reuse.outputs.channel_ref }} @@ -1086,15 +1174,16 @@ jobs: # ml:dev reported fc.revision= one push after run 4749 had read # a7e626a67a79 off it. Nothing failed; the savings just evaporated. # - # Excluding the source means the channel tag is only ever written by - # a real build, so it stays a plain image and stays readable. On dev - # that leaves nothing to do — :dev already points at the right - # content, which is what the hit established. On main it leaves - # :c-, which rule 145 requires of every main push whether or not - # a build ran. + # Excluding the source means the channel tag is only ever written + # by a real build, so it stays a plain image and stays readable. + # On dev that leaves nothing to do either way: the build pushed :dev + # itself, or the hit established it was already right. On main it + # leaves :c-, which rule 145 requires of every main push whether + # or not a build ran. # - # steps.tag emits ONE comma-separated list, because that is the shape - # docker/build-push-action takes; imagetools wants a -t per ref. + # steps.tag emits ONE comma-separated list; imagetools wants a -t per + # ref. (That list used to feed docker/build-push-action directly — + # which is exactly what #3190 made unsafe.) ARGS="" IFS=, for t in $TAGS; do @@ -1103,8 +1192,8 @@ jobs: done unset IFS if [ -z "$ARGS" ]; then - echo "repoint: $SOURCE already carries this revision and is the" - echo "repoint: only tag for this channel — nothing to write." + echo "repoint: $SOURCE is the only tag for this channel and" + echo "repoint: already holds this revision — nothing to write." exit 0 fi # shellcheck disable=SC2086 diff --git a/.forgejo/workflows/ci.yml b/.forgejo/workflows/ci.yml index 7620eca..af31cb5 100644 --- a/.forgejo/workflows/ci.yml +++ b/.forgejo/workflows/ci.yml @@ -2,7 +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: the derived version resolves and MAJOR.MINOR agrees. +# - extension-version: the derived version resolves and is a shape AMO takes. # - 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`. @@ -58,9 +58,12 @@ jobs: # the extension.yml suite runs on node:24-slim, which is exactly why # version.spec.js sticks to packaging.sh's git-free subcommands. # 1. the derivation actually resolves on this commit - # 2. MAJOR.MINOR agrees between the two files — 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 + # 2. the derived string is one AMO will accept, checked against Mozilla's + # own published grammar rather than a loose "digits and dots" + # + # The MAJOR.MINOR-agreement check that used to be (2) is gone with milestone + # 318 step 8: the committed version no longer seeds anything, so there is no + # hand-set part left for the two files to disagree about. # # Deliberately NOT checked here: that the derived value beats what has already # been signed. That guard belongs in build.yml, where it compares against the @@ -85,27 +88,38 @@ jobs: # busybox sh on the act_runner — no bashisms (family rule). VERSION=$(sh extension/scripts/packaging.sh version) echo "derived: $VERSION" - # The shape AMO accepts, and the shape build.yml will stamp. - if ! echo "$VERSION" | grep -qE '^[0-9]+(\.[0-9]+)*$'; then - echo "ERROR: derived version '$VERSION' is not plain dotted-numeric." - echo "AMO would reject it, and build.yml stamps it verbatim." + + # Mozilla's published grammar for AMO, transcribed verbatim from + # MDN's manifest.json/version page: + # + # ^(0|[1-9][0-9]{0,8})([.](0|[1-9][0-9]{0,8})){0,3}$ + # + # Not the looser `^[0-9]+(\.[0-9]+)*$` this lane used to carry. That + # one passes `2026.08.29.0201`, which AMO REJECTS — a segment must be + # the single digit 0 or start 1-9 — and it also passes five segments, + # where AMO allows four. Both would surface as a failed sign with the + # version already burned: AMO 409s on re-signing, so a rejected value + # cannot be reclaimed and cannot be reused. This lane is the cheap + # place to find out. (#3138, milestone 318 step 8.) + if ! echo "$VERSION" | grep -qE '^(0|[1-9][0-9]{0,8})(\.(0|[1-9][0-9]{0,8})){0,3}$'; then + echo "ERROR: derived version '$VERSION' is not a version AMO accepts." + echo "AMO's grammar: ^(0|[1-9][0-9]{0,8})([.](0|[1-9][0-9]{0,8})){0,3}$" + echo "Most likely cause: a zero-padded segment (08, 0201). The rest" + echo "of the family pads; the extension must not — see packaging.sh." exit 1 fi - mm() { grep -E '"version"' "$1" | head -1 | sed -E 's/.*"version"[[:space:]]*:[[:space:]]*"([0-9]+\.[0-9]+).*/\1/'; } - MAN=$(mm extension/manifest.json) - PKG=$(mm extension/package.json) - test -n "$MAN" || { echo "ERROR: no parseable version in extension/manifest.json"; exit 1; } - test -n "$PKG" || { echo "ERROR: no parseable version in extension/package.json"; exit 1; } - if [ "$MAN" != "$PKG" ]; then - echo "ERROR: MAJOR.MINOR disagrees between the two files." - echo " extension/manifest.json = $MAN <- packaging.sh reads MAJOR.MINOR from here" - echo " extension/package.json = $PKG" - echo "Only MAJOR.MINOR is hand-set. The patch component is derived from" - echo "commit time and overwritten at build time, so the committed patch" - echo "numbers are inert — but MAJOR.MINOR still ships. Set both the same." + + # ...and the shape this project actually derives. AMO would happily + # take `1.0.3500147` too, so the grammar check alone would not notice + # a regression to the pre-318 shape — which orders BELOW everything + # signed since, and is unrecoverable once Firefox has the higher one. + if ! echo "$VERSION" | grep -qE '^20[0-9][0-9]\.[0-9]{1,2}\.[0-9]{1,2}\.[0-9]{1,4}$'; then + echo "ERROR: derived version '$VERSION' is not YYYY.M.D.HHMM." + echo "Rule 148's CalVer is what build.yml signs; the old" + echo "1.0. shape would order below every ext-2026.* release." exit 1 fi - echo "OK: MAJOR.MINOR $MAN, derived version $VERSION" + echo "OK: derived version $VERSION" backend-lint-and-test: runs-on: python-ci diff --git a/ci-requirements.md b/ci-requirements.md index f57e46d..2744b40 100644 --- a/ci-requirements.md +++ b/ci-requirements.md @@ -71,12 +71,25 @@ per `docs/process.md`'s "add deps to the image when used by >1 project". published bytes?"*, not *"is this file copied in?"* — which is why the script keeps two lists rather than one. - **The shipped extension version is derived, not committed.** It is the commit - TIME of the newest packaged-extension change (minutes since 2020-01-01, per - family rule 149 — never a commit count, which orders by branch rather than by - recency). `build.yml`'s `sign-extension` computes it and stamps it into - `extension/manifest.json` + `package.json` in the working tree before signing; - the stamp is never committed. Treat the version in the repo as a base: only - its MAJOR.MINOR is read, and its patch component is inert. + TIME of the newest packaged-extension change, rendered `YYYY.M.D.HHMM` UTC + (family rules 148/149 — never a commit count, which orders by branch rather + than by recency). `build.yml`'s `sign-extension` computes it and stamps it + into `extension/manifest.json` + `package.json` in the working tree before + signing; the stamp is never committed. The version in the repo is **wholly + inert** — since milestone 318 step 8 there is no hand-set MAJOR.MINOR either. +- **The extension is the one artifact that does not zero-pad, and that is not a + drift** (#3138). Mozilla's grammar for AMO is + `^(0|[1-9][0-9]{0,8})([.](0|[1-9][0-9]{0,8})){0,3}$` — a segment is the single + digit `0` or starts 1-9, and there are at most four. `2026.08.29.0201` is + rejected; `2026.8.29.201` is the same value one character narrower per + segment, and rule 148 defines comparison as numeric per segment, so nothing is + reordered. `ci.yml`'s `extension-version` lane asserts the derived string + against that exact regex, plus a `YYYY.M.D.HHMM` shape check that would catch + a regression to the pre-318 `1.0.` — which AMO would accept and which + orders below everything already signed. Checking here is the whole point: AMO + 409s on re-signing, so a version it rejects is burned and cannot be reused. + `scripts/artifacts.sh version extension` **delegates** to `packaging.sh` so + the two cannot answer differently. - Every job that derives anything checks out with `fetch-depth: 0` — all four `build.yml` jobs, `ci.yml`'s `extension-version` and `backend-lint-and-test` (for `tests/test_artifact_paths.py` and `test_artifact_identity.py`), and @@ -108,6 +121,18 @@ per `docs/process.md`'s "add deps to the image when used by >1 project". source tag — `imagetools create` wraps its source in a manifest index, and config labels do not resolve through an index, so writing the channel tag from itself destroys the label the next run reads (#3183). +- **The build pushes exactly ONE tag — the channel's — and every other tag is + written registry-side afterwards** (#3190). buildx on this runner pushes the + first tag to the registry and then re-pushes the rest through the docker + driver, out of a local image store that a registry-direct build never fills; + it fails intermittently with `tag does not exist`. On `dev` that only reddens + a job, but on `main` it silently skips `:c-` while `:latest` publishes + fine — a missing rollback tag has no consumer that fails, so nothing but the + red job would notice until somebody needs to roll back. `imagetools create` + has no local store to be absent from, and it is the code the reuse path + already ran, so both paths now share one proven route. The cost: `:c-` + is an index rather than a plain image, so `fc.revision` does not resolve + through it — nothing reads it there, and the index names the same manifest. - **`FC_CHANNEL` and `FC_VERSION` are build args, not runtime settings.** `build.yml` passes them to the web image only — the ml and agent images have nothing to report them to. `/api/health` returns both, the foot of Settings diff --git a/extension/README.md b/extension/README.md index 6dc6db1..1861280 100644 --- a/extension/README.md +++ b/extension/README.md @@ -38,27 +38,42 @@ npm run build # unsigned XPI in web-ext-artifacts/ - [ ] Subscriptions list: popup → "Sources" tab → list renders - [ ] Check now: click play icon on source row → no error toast -## Versioning — don't hand-edit the patch number +## Versioning — the committed number decides nothing The shipped version is **derived**, not committed. `scripts/packaging.sh -version` returns `MAJOR.MINOR` from `manifest.json` plus a patch component -that is the commit *time* of the newest change to a packaged extension file, -in minutes since 2020-01-01. `build.yml` computes it and stamps it into both +version` returns `YYYY.M.D.HHMM` in UTC: the commit *time* of the newest change +to a packaged extension file. `build.yml` computes it and stamps it into both `manifest.json` and `package.json` at build time. The stamp is never committed — the commit carrying it would itself be a change to the extension, which would move the version again. So: -- **Editing the patch number does nothing.** It is overwritten before web-ext - ever reads it. There is no bump to make, and none to forget. -- **MAJOR.MINOR is still yours.** It carries the deliberate meaning, it is read - from `manifest.json` alone, and CI fails the `extension-version` lane if the - two files disagree on it. +- **Editing the version does nothing.** All of it is overwritten before web-ext + ever reads it. There is no bump to make, and none to forget. There is no + hand-set part left either: MAJOR.MINOR went away with milestone 318 step 8. - `npm run build` locally produces an XPI labelled with the *committed* version, since nothing stamped it. Fine for loading into a test profile; not what ships. +**Why the extension is the one artifact that does not zero-pad.** Every other +FC artifact emits rule 148's `YYYY.MM.DD.HHMM`. AMO will not take it: Mozilla's +grammar for addons.mozilla.org is + +``` +^(0|[1-9][0-9]{0,8})([.](0|[1-9][0-9]{0,8})){0,3}$ +``` + +— each segment is the single digit `0` or starts 1-9, so `08` and `0201` are +rejected, and at most four segments are allowed. The extension therefore emits +**the same numbers unpadded**: `2026.8.29.201` where the rest of the family +says `2026.08.29.0201`. Rule 148 already defines comparison as numeric per +segment, under which the two are equal, so nothing is reordered by the choice +and left-padding each segment recovers the family string exactly. `ci.yml`'s +`extension-version` lane checks the derived string against that regex on every +push — the cheap place to find out, because AMO 409s on re-signing and a +rejected version is burned for good. + Why commit time and not a commit count: a count is per-branch, so `dev` and `main` count different histories of the same code and their versions end up ordered by which branch accumulated more commits rather than by which is newer. diff --git a/extension/scripts/packaging.sh b/extension/scripts/packaging.sh index 399aa43..261e69e 100755 --- a/extension/scripts/packaging.sh +++ b/extension/scripts/packaging.sh @@ -60,7 +60,7 @@ NOT_PACKAGED_BUILD='web-ext-artifacts node_modules' NOT_VERSION_RELEVANT='package.json package-lock.json README.md .gitignore vitest.config.js test test/**' usage() { - echo "usage: packaging.sh {ignore|pathspec|version|major-minor|patch}" >&2 + echo "usage: packaging.sh {ignore|pathspec|version}" >&2 exit 2 } @@ -86,36 +86,48 @@ cmd_pathspec() { echo } -# MAJOR.MINOR stays hand-set in manifest.json — it's the part that carries -# deliberate meaning. Only the patch component is derived. -cmd_major_minor() { - root=$(git rev-parse --show-toplevel) - grep -E '"version"' "$root/extension/manifest.json" \ - | head -1 \ - | sed -E 's/.*"version"[[:space:]]*:[[:space:]]*"([0-9]+)\.([0-9]+).*/\1.\2/' +# Strip leading zeros from one segment, leaving at least one digit. +# +# This exists for AMO and nothing else. Mozilla's version grammar for +# addons.mozilla.org is documented as +# +# ^(0|[1-9][0-9]{0,8})([.](0|[1-9][0-9]{0,8})){0,3}$ +# +# — each segment is either the single digit `0` or starts 1-9, so `08` and +# `0201` are rejected outright, while `0` itself is fine. MDN states it in +# prose too: "Non-zero numbers must not include a leading zero." +# +# POSIX sh has no trim-loop, hence the while. +unpad() { + s=$1 + while [ "${#s}" -gt 1 ]; do + case "$s" in + 0*) s=${s#0} ;; + *) break ;; + esac + done + printf '%s' "$s" } -# 2020-01-01T00:00:00Z — the anchor for the derived patch component. Fixed -# forever; moving it would renumber every version downwards. -VERSION_EPOCH=1577836800 - -# Minutes since VERSION_EPOCH of the LATEST commit that touched a PACKAGED -# extension file. +# The extension's version: `YYYY.M.D.HHMM`, UTC, derived from the commit TIME +# of the newest change to a PACKAGED extension file. # -# Time-derived, per family rule 149: an artifact's ordering key must never be a -# commit count. A count is per-branch — `dev` and `main` count different -# histories of the same code — so the moment BOTH channels publish, their -# versions order by which branch accumulated more commits rather than by which -# is newer. A squash-merge makes that permanent: main gains one commit where dev -# gained five, so dev climbs away from main and a dev install can never cross -# back. That is Roundtable's 2026-08-24 incident (`versionCode` was the branch's -# commit count) in a different repo. Measured here on 2026-08-27: main=23, -# dev=24 under the old formula — one apart, which is exactly how the inversion -# stays invisible until it strands somebody. +# THE ONE DELIBERATE DEPARTURE FROM THE FAMILY SHAPE, and it is a rendering +# difference only. Rule 148 says `YYYY.MM.DD.HHMM` zero-padded, and every other +# FC artifact emits exactly that. AMO's grammar (see unpad) forbids the padding, +# and AMO is not negotiable: a rejected version is burned, since AMO 409s on +# re-signing a version it has already seen. So the extension emits THE SAME +# NUMBERS unpadded — 2026.08.29.0201 and 2026.8.29.201 are one value in two +# renderings, and rule 148 already specifies comparison as numeric per segment, +# under which they are equal. Nothing published is reordered by the choice, and +# left-padding each segment recovers the family string exactly. +# +# HHMM is one segment, not two, because AMO allows at most FOUR. Unpadded that +# reads oddly (00:14 -> `14`, midnight -> `0`) but stays strictly increasing +# within a day, which is all the ordering needs. # # Why the commit's time and not the build's: -# * MONOTONIC — max() over a set that only ever gains members. Verified -# across all 24 extension-touching commits: zero non-monotonic steps. +# * MONOTONIC — max() over a set that only ever gains members. # * STABLE while the extension is unchanged, so an unchanged extension keeps # its version, the ext- signature cache still hits, and AMO is # called once per extension CHANGE rather than once per push. Build-time @@ -126,32 +138,40 @@ VERSION_EPOCH=1577836800 # produced for byte-identical code. Same code, same version, one signing. # * REPRODUCIBLE — any checkout of a commit yields that commit's version. # +# Never a commit count (family rule 149): a count is per-branch, so `dev` and +# `main` count different histories of the same code and order by which branch +# accumulated more commits rather than by which is newer. A squash-merge makes +# that permanent. Roundtable's 2026-08-24 incident, in a different repo. +# # Requires real history: a depth-1 clone sees one commit and will derive a wrong # (too low) value. Every consumer must check out with fetch-depth: 0. -cmd_patch() { +# +# Formatted through git rather than date(1): busybox date does not reliably +# accept `-d @`, and git's --date=format-local is available wherever git +# is. TZ=UTC so the value does not depend on the runner's timezone. +cmd_version() { root=$(git rev-parse --show-toplevel) # Unquoted on purpose: the pathspec must word-split into separate args. # Globbing is already off script-wide (set -euf above). # shellcheck disable=SC2046 - ts=$(cd "$root" && git log --format=%ct HEAD -- extension/ $(cmd_pathspec) \ - | sort -n | tail -1) - if [ -z "$ts" ]; then + sha=$(cd "$root" && git log --format='%ct %H' HEAD -- extension/ $(cmd_pathspec) \ + | sort -n | tail -1 | cut -d' ' -f2) + if [ -z "$sha" ]; then echo "packaging.sh: no commit touches a packaged extension file" >&2 exit 1 fi - echo $(( (ts - VERSION_EPOCH) / 60 )) -} - -cmd_version() { - echo "$(cmd_major_minor).$(cmd_patch)" + padded=$(cd "$root" && TZ=UTC git show -s --format=%cd \ + --date='format-local:%Y.%m.%d.%H%M' "$sha") + # Rebinding the function's own positional params, which are unused here. + # shellcheck disable=SC2046 + set -- $(echo "$padded" | tr '.' ' ') + echo "$(unpad "$1").$(unpad "$2").$(unpad "$3").$(unpad "$4")" } [ $# -ge 1 ] || usage case "$1" in - ignore) cmd_ignore ;; - pathspec) cmd_pathspec ;; - version) cmd_version ;; - major-minor) cmd_major_minor ;; - patch) cmd_patch ;; - *) usage ;; + ignore) cmd_ignore ;; + pathspec) cmd_pathspec ;; + version) cmd_version ;; + *) usage ;; esac diff --git a/extension/test/version.spec.js b/extension/test/version.spec.js index ed8a307..b84a7e1 100644 --- a/extension/test/version.spec.js +++ b/extension/test/version.spec.js @@ -8,10 +8,10 @@ const EXT_DIR = path.join(path.dirname(fileURLToPath(import.meta.url)), '..') const read = (name) => JSON.parse(readFileSync(path.join(EXT_DIR, name), 'utf8')) const readText = (...seg) => readFileSync(path.join(EXT_DIR, ...seg), 'utf8') -// Only the git-free subcommands are exercised here: `version`/`patch` shell out -// to git, and the extension lane runs on node:24-bookworm-slim which may not -// ship it. Those two are covered where git is guaranteed — ci.yml and build.yml -// run on ci-python. +// Only the git-free subcommands are exercised here: `version` shells out to +// git, and the extension lane runs on node:24-bookworm-slim which may not ship +// it. That one is covered where git is guaranteed — ci.yml's extension-version +// lane and build.yml both run on ci-python. const packaging = (cmd) => execFileSync('sh', [path.join(EXT_DIR, 'scripts', 'packaging.sh'), cmd], { cwd: EXT_DIR, @@ -145,28 +145,33 @@ describe('consumers delegate rather than keeping their own copy', () => { }) describe('extension version', () => { - const majorMinor = (v) => v.split('.').slice(0, 2).join('.') + // Mozilla's published grammar for addons.mozilla.org, transcribed from MDN's + // manifest.json/version page. Each segment is the single digit 0 or starts + // 1-9 — so no leading zeros — and there are at most four of them. + const AMO = /^(0|[1-9][0-9]{0,8})(\.(0|[1-9][0-9]{0,8})){0,3}$/ - it('keeps the hand-set MAJOR.MINOR in lockstep across both files', () => { - // Narrowed from full-string equality at milestone 271 step 5. Since step 4 - // the patch component is derived from commit time and stamped into both - // files at build time, so the committed patch numbers are inert — nothing - // reads them and they are not what ships. Asserting on them would fail for - // a difference that changes nothing. + it('keeps a committed version AMO would accept, though it ships nothing', () => { + // The committed value is wholly inert since milestone 318 step 8: there is + // no hand-set MAJOR.MINOR left for packaging.sh to read, and build.yml + // stamps the derived string over both files before web-ext sees them. // - // MAJOR.MINOR is the opposite: still hand-set, still shipped, and - // packaging.sh reads it from manifest.json ALONE. Let the two diverge and - // the extension ships a version package.json disagrees with, with no other - // signal. - expect(majorMinor(read('manifest.json').version)) - .toBe(majorMinor(read('package.json').version)) + // It is still asserted, for one reason: `npm run build` locally packages + // whatever is committed, so a value AMO would reject turns a local build + // into a confusing failure with no CI signal ahead of it. ci.yml checks + // the same grammar against the DERIVED value, which is the one AMO sees. + for (const file of ['manifest.json', 'package.json']) { + expect(read(file).version, `${file} version is not AMO-shaped`).toMatch(AMO) + } }) - it('uses a plain dotted numeric version AMO will accept', () => { - // The committed value seeds MAJOR.MINOR, so it still has to parse even - // though its patch component never ships. ci.yml asserts the same shape on - // the DERIVED value, which is the one AMO actually sees. - expect(read('package.json').version).toMatch(/^\d+(\.\d+)*$/) + it('rejects the zero-padded family shape, which is why the extension unpads', () => { + // Guards the reason for the exception, not just its result. If this ever + // starts passing, someone has loosened the pattern and the next sign burns + // an AMO version to find out. (#3138.) + expect('2026.08.29.0201').not.toMatch(AMO) + expect('2026.8.29.201').toMatch(AMO) + // Five segments: AMO allows four. + expect('2026.8.29.2.1').not.toMatch(AMO) }) it('declares manifest v3', () => { diff --git a/scripts/artifacts.sh b/scripts/artifacts.sh index a7cfcdf..5f86a79 100755 --- a/scripts/artifacts.sh +++ b/scripts/artifacts.sh @@ -170,6 +170,21 @@ cmd_revision() { # exists — at which point two lanes derive different answers for one source # and the shared-signature property is lost. cmd_version() { + # The extension is the one artifact this script does not FORMAT, only route. + # AMO's version grammar forbids leading zeros, so the extension emits the + # same numbers unpadded (#3138) — a rendering exception, documented in + # packaging.sh beside the signing step that has to obey it. Delegating keeps + # one answer per artifact: `artifacts.sh version extension` and + # `packaging.sh version` cannot drift into two. + # + # The direction is deliberate. artifacts.sh already asks packaging.sh for the + # extension's PATH SET (ext_paths above), so the version has to flow the same + # way; reversing it would have packaging.sh call back into this script, which + # would call packaging.sh for the paths again. + if [ "$1" = extension ]; then + sh "$ROOT/extension/scripts/packaging.sh" version + return + fi sha=$(echo "$(newest "$1")" | cut -d' ' -f2) # One git call for the whole string rather than four and a sed. git's # format-local takes the complete format, and doing it in pieces was only diff --git a/tests/test_artifact_identity.py b/tests/test_artifact_identity.py index 60d086f..fe614bf 100644 --- a/tests/test_artifact_identity.py +++ b/tests/test_artifact_identity.py @@ -34,6 +34,15 @@ is genuinely the commit those paths last changed in. emit. Two nearly-identical formats are more dangerous than two obviously different ones, and the only thing keeping them identical is a test. +**The extension is the one exception, and it is a rendering exception only.** +AMO's version grammar forbids a leading zero, so the extension emits the same +numbers unpadded — `2026.8.29.201` where the family says `2026.08.29.0201` +(#3138, milestone 318 step 8). Rule 148 defines comparison as numeric per +dot-segment, under which the two are equal, so this is pinned in both +directions below: the extension must satisfy AMO's grammar, and every artifact +must derive the same NUMBERS its own commit stamps. An exception left as "the +extension is different" would drift into being differently different. + The identity-TAG tests this file used to hold are gone with the tag. There is no longer a `CHANNELLED` list to drift (the channel is which tag you inspect), and no `identity` subcommand to refuse an unqualified call. @@ -57,6 +66,26 @@ _REVISION = re.compile(r"^[0-9a-f]{12}$") # YYYY.MM.DD.HHMM, every segment zero-padded to its full width. _VERSION = re.compile(r"^\d{4}\.\d{2}\.\d{2}\.\d{4}$") +# The artifacts that cannot use the padded rendering. Exactly one, and the +# reason is external: `packaging.sh` derives the extension's version and AMO +# refuses to sign a padded one. +AMO_UNPADDED = frozenset({"extension"}) + +# Mozilla's published grammar for addons.mozilla.org, transcribed from MDN's +# manifest.json/version page. A segment is the single digit `0` or starts 1-9, +# and there are at most four. This is the constraint the exception exists for, +# so it is what the exception is tested against — `2026.08.29.0201` fails it. +_AMO = re.compile(r"^(0|[1-9][0-9]{0,8})(\.(0|[1-9][0-9]{0,8})){0,3}$") + +# YYYY.M.D.HHMM — four segments, none of them zero-padded. +_UNPADDED = re.compile(r"^\d{4}(\.(0|[1-9]\d*)){3}$") + + +def segments(value: str) -> tuple[int, ...]: + """A version as the numbers it denotes, which is how rule 148 says to + compare one. `2026.08.29.0201` and `2026.8.29.201` are one value here.""" + return tuple(int(part) for part in value.split(".")) + # Everything here goes through artifacts.sh rather than importing a sibling # test module. That is the interface build.yml actually calls, so the tests @@ -126,7 +155,7 @@ def test_revision_is_a_legal_label_value_and_is_stable(artifact): assert first == revision(artifact), "revision is not stable across calls" -@pytest.mark.parametrize("artifact", ARTIFACTS) +@pytest.mark.parametrize("artifact", sorted(set(ARTIFACTS) - AMO_UNPADDED)) def test_version_is_zero_padded_calver(artifact): """The family shape, pinned. @@ -148,6 +177,31 @@ def test_version_is_zero_padded_calver(artifact): ) +@pytest.mark.parametrize("artifact", sorted(AMO_UNPADDED)) +def test_the_unpadded_artifacts_derive_something_amo_will_sign(artifact): + """The other half of the family shape: the documented exception, tested + against the constraint that justifies it rather than against itself. + + A padded value passes `_UNPADDED` on any date with no leading zeros, so + that pattern alone would let a regression sit unnoticed until the first + single-digit month — at which point the failure is a burned AMO version, + not a red lane. AMO's grammar is the assertion that fires immediately. + """ + value = artifacts("version", artifact).strip() + assert _AMO.match(value), ( + f"{artifact} derives {value!r}, which AMO refuses: a segment must be " + f"the single digit `0` or start 1-9, and there are at most four. " + f"Almost certainly a zero-padded segment — the family pads and this " + f"artifact must not (#3138). AMO 409s on re-signing, so a version it " + f"rejects is burned." + ) + assert _UNPADDED.match(value), ( + f"{artifact} derives {value!r}, which is not YYYY.M.D.HHMM. AMO would " + f"also accept the pre-318 `1.0.`, and that orders below every " + f"ext-2026.* release already signed." + ) + + @pytest.mark.parametrize("artifact", ARTIFACTS) def test_version_and_revision_describe_the_same_commit(artifact): """They are derived independently and must not be able to disagree. @@ -162,5 +216,21 @@ def test_version_and_revision_describe_the_same_commit(artifact): capture_output=True, text=True, check=True, cwd=ROOT, env={"TZ": "UTC", "PATH": os.environ.get("PATH", "")}, ).stdout.strip() - assert artifacts("version", artifact).strip() == stamped + derived = artifacts("version", artifact).strip() + + # Compared as NUMBERS, which is how rule 148 defines comparison and the + # only way one assertion can cover both renderings. This is what makes the + # extension's exception cosmetic rather than semantic: it must denote + # exactly the value its own commit stamps, whatever the padding. + assert segments(derived) == segments(stamped), ( + f"{artifact} derives {derived!r}, but its newest shipped commit " + f"{sha[:12]} is {stamped!r}. The instance would name one commit while " + f"carrying another's bytes." + ) + if artifact not in AMO_UNPADDED: + assert derived == stamped, ( + f"{artifact} derives {derived!r} where the family shape is " + f"{stamped!r} — same numbers, wrong rendering. Only the artifacts " + f"in AMO_UNPADDED may differ here." + ) assert sha.startswith(revision(artifact))