From 41f2bec3afe7135f15774384819bd9d0e75a1f61 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Sat, 29 Aug 2026 00:38:26 -0400 Subject: [PATCH 1/3] fix(ci): the build pushes one tag; the rest are written registry-side (#3190) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit buildx on this runner pushes the first tag to the registry and then re-pushes the remaining ones through the DOCKER driver, reading them out of a local image store that a registry-direct build never populated: #27 pushing …/fabledcurator:latest DONE 15.8s #28 pushing …/fabledcurator:c-0e15c44 with docker #28 ERROR: tag does not exist: …:c-0e15c44 It is intermittent — build-ml made the identical two-tag push seconds later in the same run and succeeded — and the consequence is worse than the red job suggests. `: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 else would ever have noticed: a missing :c- has no consumer that fails, so it surfaces at the moment somebody needs to roll back, which is the worst time to learn a rollback target was never written. So the build now pushes exactly one ref — the channel's — and the existing repoint step, which already excluded the source tag and already ran on every reuse, now runs on the build path too and owns every other tag. `imagetools create` is a registry-side manifest copy: no local daemon, nothing that can be absent. This adds no new code path; it puts the build case onto the one that was already proven. Chosen over the alternative of asserting each tag resolves after the build, which would have made the failure loud without making it rarer. The cost, accepted: `imagetools create` wraps its source in an index, so :c- is an index rather than a plain image 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. `build_tags` goes with it — the tag list now has exactly one consumer. --- .forgejo/workflows/build.yml | 222 ++++++++++++++++++++++++----------- ci-requirements.md | 12 ++ 2 files changed, 165 insertions(+), 69 deletions(-) diff --git a/.forgejo/workflows/build.yml b/.forgejo/workflows/build.yml index 72660ca..4ddc819 100644 --- a/.forgejo/workflows/build.yml +++ b/.forgejo/workflows/build.yml @@ -476,7 +476,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 +484,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 +606,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 +625,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 +677,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 +695,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 +809,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 +859,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 +924,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 +942,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 +1054,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 +1104,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 +1169,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 +1187,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/ci-requirements.md b/ci-requirements.md index f57e46d..011d0a8 100644 --- a/ci-requirements.md +++ b/ci-requirements.md @@ -108,6 +108,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 From 2e01242381561bc1ff4e5f9716f9ef69c7f2f973 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Sat, 29 Aug 2026 13:43:30 -0400 Subject: [PATCH 2/3] feat(extension): derive the version as unpadded CalVer (milestone 318 step 8) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `1.0.` -> `YYYY.M.D.HHMM` UTC, from the commit time of the newest change to a packaged extension file. Same clock and same commit as before; readable instead of opaque, and the same value the rest of the family derives. The hold on this step was two questions about AMO, and Mozilla's own docs answer both: ^(0|[1-9][0-9]{0,8})([.](0|[1-9][0-9]{0,8})){0,3}$ 1. four all-numeric segments -> ACCEPTED ({0,3} more after the first). 2. leading zeros -> REJECTED. A segment is the single digit `0` or starts 1-9, so `08` and `0201` are refused. MDN says it in prose too: "Non-zero numbers must not include a leading zero." So the documented fallback applies, extension only: the same numbers rendered without the family's zero-padding. `2026.08.29.0201` and `2026.8.29.201` are one value in two renderings — rule 148 defines comparison as numeric per segment, under which they are equal — so nothing already published is reordered, and left-padding each segment recovers the family string exactly. HHMM stays one segment because AMO allows at most four. The transition is safe in the other direction too: 2026 > 1, so every CalVer outranks every published 1.0.x. build.yml's downgrade guard confirms it. Also in scope: * MAJOR.MINOR is gone. `cmd_major_minor`, `cmd_patch` and VERSION_EPOCH go with it, the committed version in manifest.json / package.json is now wholly inert, and ci.yml's MAJOR.MINOR-agreement check is retired rather than left running beside a fact that stopped existing (rule 22). * ci.yml's `extension-version` lane now asserts Mozilla's regex verbatim instead of a loose `^[0-9]+(\.[0-9]+)*$` — which would have passed the padded shape. It also asserts YYYY.M.D.HHMM, because AMO would accept a regression to `1.0.` while that orders below everything signed since. Checking here is the point: AMO 409s on re-signing, so a version it rejects is burned and cannot be reused. * `artifacts.sh version extension` delegates to packaging.sh, so the two cannot answer differently. The direction matches the existing one — artifacts.sh already asks packaging.sh for the extension's path set. #3156 is what makes this commit safe to make: packaging.sh is in web's path set, so the web revision moves with the extension version and build-web rebuilds instead of republishing an image bundling the previous XPI. Scribe #3138. Co-Authored-By: Claude Opus 5 --- .forgejo/workflows/build.yml | 25 ++++---- .forgejo/workflows/ci.yml | 56 +++++++++++------- ci-requirements.md | 25 ++++++-- extension/README.md | 33 ++++++++--- extension/scripts/packaging.sh | 102 ++++++++++++++++++++------------- extension/test/version.spec.js | 49 +++++++++------- scripts/artifacts.sh | 15 +++++ 7 files changed, 196 insertions(+), 109 deletions(-) diff --git a/.forgejo/workflows/build.yml b/.forgejo/workflows/build.yml index 4ddc819..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: | 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 011d0a8..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 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 From 1a941e900b0d71c553e93ae3552eae8a36b243f2 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Sat, 29 Aug 2026 13:46:18 -0400 Subject: [PATCH 3/3] test: encode the extension's AMO rendering exception (milestone 318 step 8) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Missed in 2e01242. This module pinned zero-padded `YYYY.MM.DD.HHMM` for all four artifacts, which is the family shape and was right until the extension acquired a documented reason not to use it. Both assertions failed exactly as written, on the value they were written to catch. Rather than exempt the extension, the exception is pinned to the constraint that justifies it: * `test_version_is_zero_padded_calver` now covers the three padded artifacts. * A new sibling covers the unpadded one against **AMO's own grammar** — `2026.08.29.0201` fails it, so a regression to padding fires immediately. Matching only `YYYY.M.D.HHMM` would not: on a date with no leading zeros the two renderings are the same string, so a padding regression would sit unseen until the first single-digit month, and surface as a burned AMO version rather than a red lane. * `test_version_and_revision_describe_the_same_commit` compares NUMBERS, per rule 148's own definition of comparison — so one assertion covers both renderings and says the real thing: whatever the padding, the extension must denote exactly the value its own commit stamps. Exact-string equality is still asserted for everything not in AMO_UNPADDED, so the exception cannot quietly spread. Scribe #3138. Co-Authored-By: Claude Opus 5 --- tests/test_artifact_identity.py | 74 ++++++++++++++++++++++++++++++++- 1 file changed, 72 insertions(+), 2 deletions(-) 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))