fix(ci): the file that decides FC_VERSION now moves it (#3202)
CI / lint (push) Successful in 3s
Build images / sign-extension (push) Successful in 4s
CI / extension-version (push) Successful in 4s
Build images / build-ml (push) Successful in 5s
Build images / build-agent (push) Successful in 6s
CI / frontend-build (push) Successful in 29s
CI / backend-lint-and-test (push) Successful in 1m5s
Build images / build-web (push) Successful in 3m15s
CI / integration (push) Successful in 4m29s

`scripts/artifacts.sh` decides both values the web image carries — the
`fc.revision` label the reuse check compares and the `FC_VERSION` baked into
the image — and was in no artifact's path set. So a change to `cmd_version`
alone left every revision untouched, the reuse check hit, the build was
skipped, and the published image went on reporting the OLD version format,
indefinitely, until some unrelated commit forced a rebuild. Nothing goes red;
the footer just shows a well-formed string of the wrong shape.

Milestone 318 step 5 is the worked instance:

  b3989d0 -> rev=fb2c4d5b80be  ver=2026.8.28.1249
  5771fd5 -> rev=fb2c4d5b80be  ver=2026.08.28.1249
  bce894b -> rev=bce894ba2499  ver=2026.08.28.2208

Same revision across the zero-pad commit, so web's build was skipped. It cost
nothing only by timing: FC_VERSION did not exist until step 6 landed one
commit later.

Web only, and that is the interesting part. Every artifact stamps a revision,
but only web also stamps a version. A revision-only artifact needs no entry
here, because changing how a revision is COMPUTED changes the derived value,
which then disagrees with the label on the published image and forces a
rebuild — the mechanism self-corrects, since it compares against a string
stamped into a real artifact. Nothing compares a version to anything. That
asymmetry is why this was invisible and is now written down in both files.

Named as a file rather than `scripts`: release_notes.py sits beside it and
only reads derived values, so it decides nothing and must not re-version web.

This is #3156 one level up — packaging.sh excluded from the version it
derives — so the guard is generalised rather than duplicated: one DERIVERS
table naming each deriver and the artifacts whose identity it decides. The
too-wide test gains a note saying where the line is, since "copied into no
image" no longer settles it on its own.
This commit is contained in:
2026-08-28 22:01:58 -04:00
parent b6b9fd8287
commit a3071a7549
2 changed files with 77 additions and 14 deletions
+44 -13
View File
@@ -13,6 +13,10 @@ being right, and both ways of being wrong are silent:
Nothing else notices either. The version still derives, CI still goes green,
and the mismatch only surfaces as "I pinned that build and got the wrong
bytes". So the Dockerfiles are read here and compared against the declaration.
The COPY list is not the whole answer, though. A file that DECIDES what an
artifact reports belongs in its set even though it is copied into nothing —
see DERIVERS below, where the same finding is recorded twice (#3156, #3202).
"""
from __future__ import annotations
@@ -110,26 +114,48 @@ def test_the_web_image_versions_on_an_extension_change():
)
def test_the_web_image_versions_on_a_version_derivation_change():
"""packaging.sh ships in no image, yet it belongs in the sets that bundle
the XPI — because it decides the version string build.yml stamps into the
packaged manifest.json. Changing the derivation changes the shipped bytes.
# A file that DECIDES an artifact's identity is part of what that artifact is
# built from, even though it is copied into no image. Both entries here are the
# same finding twice — #3156 for packaging.sh, #3202 for artifacts.sh — and
# both were latent for the same reason: the version has no backstop.
#
# The revision does. Change how a REVISION is computed and the derived value
# stops matching the label on the published image, which forces a rebuild; the
# mechanism self-corrects because it compares against a string stamped into a
# real artifact. Nothing compares a version to anything, so a version-only
# derivation change is invisible unless the deriver is in the set.
DERIVERS = [
# packaging.sh decides the version build.yml stamps into the packaged
# manifest.json, so changing it changes the shipped bytes. Left out,
# milestone 313 step 4 turns silent: the new version misses the
# ext-<version> cache and gets signed, while web's revision has not moved,
# so the reuse path republishes the old image and the fresh signature is
# orphaned. Guarded for web too, since web bundles what the extension makes.
("extension/scripts/packaging.sh", ("extension", "web")),
# artifacts.sh decides the FC_VERSION baked into the web image (#3202).
# Web only, and deliberately: ml and agent ask this script for `revision`
# alone, so they are covered by the self-correcting path above, and the
# extension takes its version from packaging.sh. Milestone 318 step 5 is
# the worked instance — b3989d0 and 5771fd5 share revision fb2c4d5b80be
# while the version moved 2026.8.28.1249 -> 2026.08.28.1249. It was
# harmless only because FC_VERSION did not exist until one commit later.
("scripts/artifacts.sh", ("web",)),
]
Left out, milestone 313 step 4 turns it silent: the new version misses the
ext-<version> cache and gets signed, while web's revision has not moved, so
the reuse path republishes the old image and the fresh signature is
orphaned. Guarded for web and the extension both, since web bundles what
the extension produces.
"""
for artifact in ("extension", "web"):
@pytest.mark.parametrize("path, artifacts", DERIVERS, ids=lambda v: str(v))
def test_a_version_deriver_is_in_the_set_of_what_it_decides(path, artifacts):
for artifact in artifacts:
inc = includes(artifact)
excluded = [
p[len(":(exclude)"):] for p in declared_paths(artifact)
if p.startswith(":(exclude)")
]
path = "extension/scripts/packaging.sh"
assert any(covered_by(path, i) for i in inc), (
f"{path} is not in the {artifact} path set"
f"{path} decides the version {artifact} reports, but is not in the "
f"{artifact} path set. A change to the derivation would leave the "
f"revision untouched, the build skipped, and the published image "
f"reporting the old version — with nothing to disagree with it."
)
assert not any(
covered_by(path, e.rstrip("*").rstrip("/")) for e in excluded
@@ -146,6 +172,11 @@ def test_the_web_image_versions_on_a_version_derivation_change():
# Deliberate exclusions — the too-wide direction. Each of these lives
# beside shipped code but never reaches an image, and including it
# would re-version the artifact for a change it does not carry.
#
# "Never reaches an image" is the test, not "is not source": DERIVERS
# above are also copied into nothing and DO belong in their sets,
# because they decide what the image reports. The line between the two
# lists is whether the file has a say in the artifact's identity.
("agent", "agent/README.md"),
("agent", "agent/ruff.toml"),
("agent", "agent/docker-compose.yml"),