From cf06c81db970c46c4830ab9d660d46aa84a5001c Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Thu, 27 Aug 2026 21:36:17 -0400 Subject: [PATCH 1/5] build: one definition per artifact of what it ships (milestone 313 step 1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit scripts/artifacts.sh generalises what packaging.sh established for the extension: four published artifacts, four path sets, four independent versions derived from the newest commit touching each set. Measured on this commit, and this is the point of the whole thing: web tag=2026.8.27 version=2026.8.27.1547 rev=a7e626a ml tag=2026.8.27 version=2026.8.27.1547 rev=a7e626a agent tag=2026.7.17 version=2026.7.17.1657 rev=57e5243 extension tag=2026.8.27 version=2026.8.27.1547 rev=a7e626a The agent is six weeks behind because agent/fc_agent has not changed since 57e5243. Today it rebuilds and re-tags on every push regardless; from step 4 it will not. Three outputs, because they answer different questions and conflating them is how this goes wrong: tag YYYY.M.D the published image tag. Day precision, per the operator: same-day work is not worth pinning, so a second build that day replaces the first. version YYYY.M.D.HHMM the ordering key. The extension needs this and cannot use `tag`: Firefox compares it to decide whether an update exists, so two same-day builds must be distinguishable or the second hits the ext- cache and ships stale bytes. That is issue #2397's failure mode exactly. revision content identity. Because `tag` is only day-precise, "does this tag already exist" cannot decide whether a build can be skipped — two different builds legitimately share a tag. Step 4 keys on this instead. Path sets read from the Dockerfiles rather than guessed. Notable calls: - web includes the extension's packaged set, because build.yml bakes the signed XPI into frontend/public/extension/ before the docker build. Miss that and :latest serves a NEW extension under an unchanged web version. - web excludes frontend/test: vite builds from src/, index.html and public/, so a spec change lands in the builder layer but never in dist. - agent is agent/Dockerfile + agent/requirements.txt + agent/fc_agent, NOT agent/. README.md, ruff.toml and docker-compose.yml sit in that directory and never reach the image. - every set includes its own Dockerfile and requirements: a base-image bump changes the artifact as surely as a source edit does. - the extension's set is read from packaging.sh, not restated. One definition, per #2397. tests/test_artifact_paths.py guards both directions of being wrong, since both are silent. Too narrow — a COPY'd file missing from the set — means the version does not move when the content does, and a pin serves stale bytes. Too wide means re-versioning for a change the artifact does not ship. The test parses each Dockerfile's COPY lines and compares them against the declaration, so adding a COPY without updating the set fails the lane. No workflow reads any of this yet. Step 2 shadows it. --- scripts/artifacts.sh | 150 +++++++++++++++++++++++++++++++++++ tests/test_artifact_paths.py | 138 ++++++++++++++++++++++++++++++++ 2 files changed, 288 insertions(+) create mode 100755 scripts/artifacts.sh create mode 100644 tests/test_artifact_paths.py diff --git a/scripts/artifacts.sh b/scripts/artifacts.sh new file mode 100755 index 0000000..948706a --- /dev/null +++ b/scripts/artifacts.sh @@ -0,0 +1,150 @@ +#!/bin/sh +# Single definition of WHAT EACH PUBLISHED ARTIFACT IS BUILT FROM, and the +# version derived from it. Milestone 313; generalises the shape +# extension/scripts/packaging.sh established for the extension alone. +# +# Four artifacts, four independent versions. An artifact whose shipped files +# did not change keeps its version and does not rebuild — that is the whole +# point, and it is why each path set must match its Dockerfile rather than +# being a plausible guess. Getting a set wrong is quiet in BOTH directions: +# +# too narrow -> a pin serves stale bytes, because the version did not move +# when the content did. This is the dangerous one. +# too wide -> the artifact re-versions and rebuilds for a change it does +# not ship. Merely wasteful. +# +# tests/test_artifact_paths.py asserts every COPY source in each Dockerfile is +# covered here, so adding a COPY without updating this file fails CI. +# +# POSIX sh only — CI's run shell is busybox on some paths. +# +# -f (no pathname expansion) is load-bearing for the whole script: the lists +# below are iterated with deliberate word-splitting, and without it the shell +# would glob `frontend/test/**` against the working tree and silently narrow +# the pattern. Callers substituting the output need their own `set -f` too; +# the two guards protect different expansions. +set -euf + +ROOT=$(git rev-parse --show-toplevel) + +# --- what each artifact ships ------------------------------------------------ +# +# Each set includes its own Dockerfile and requirements: changing a base image +# or a pin changes the artifact just as surely as changing a source file. +# +# web (Dockerfile, context `.`) — the runtime stage copies backend/, alembic/, +# alembic.ini, entrypoint.sh and requirements.txt; the frontend-builder stage +# copies frontend/ and the runtime takes its `dist` output. +# +# frontend/test is excluded: `npm run build` is vite, which builds from src/, +# index.html and public/ and never reads test/. It lands in the builder layer +# but not in `dist`, so it cannot reach the shipped image. +# +# The web image ALSO bundles the signed XPI (build.yml downloads it into +# frontend/public/extension/ before the docker build), so an extension change +# changes the web image. The extension's packaged set is appended in cmd_paths +# rather than restated — one definition, per #2397. +WEB_PATHS='Dockerfile requirements.txt backend alembic alembic.ini entrypoint.sh frontend :(exclude)frontend/test :(exclude)frontend/test/**' + +# ml (Dockerfile.ml, context `.`) — no frontend, no extension. Note it copies +# BOTH requirements-ml.txt and requirements.txt. +ML_PATHS='Dockerfile.ml requirements-ml.txt requirements.txt backend alembic alembic.ini entrypoint.sh' + +# agent (agent/Dockerfile, context `agent`) — copies requirements.txt and +# fc_agent only. agent/README.md, agent/docker-compose.yml and agent/ruff.toml +# live in the directory but never reach the image, so they must not re-version +# it: this is deliberately NOT `agent/`. +AGENT_PATHS='agent/Dockerfile agent/requirements.txt agent/fc_agent' + +usage() { + echo "usage: artifacts.sh {paths|revision|version|tag} {web|ml|agent|extension}" >&2 + exit 2 +} + +# The extension's packaged set, read from its own definition rather than +# copied. packaging.sh emits `:(exclude)extension/...` entries, so the bare +# `extension` include has to come with them. +ext_paths() { + echo "extension $(sh "$ROOT/extension/scripts/packaging.sh" pathspec)" +} + +cmd_paths() { + case "$1" in + web) echo "$WEB_PATHS $(ext_paths)" ;; + ml) echo "$ML_PATHS" ;; + agent) echo "$AGENT_PATHS" ;; + extension) ext_paths ;; + *) usage ;; + esac +} + +# " " of the newest commit touching this artifact's shipped set. +# Unquoted on purpose: the pathspec must word-split into separate args. +# Globbing is already off script-wide. +newest() { + # shellcheck disable=SC2046 + set -- "$(cd "$ROOT" && git log --format='%ct %H' HEAD -- $(cmd_paths "$1") \ + | sort -n | tail -1)" + if [ -z "$1" ]; then + echo "artifacts.sh: no commit touches this artifact's shipped files" >&2 + exit 1 + fi + echo "$1" +} + +# Formatted through git rather than date(1): busybox date does not reliably +# accept `-d @`, and git's own --date=format-local is available wherever +# git is. TZ=UTC so the value does not depend on the runner's timezone. +fmt() { + (cd "$ROOT" && TZ=UTC git show -s --format=%cd --date="format-local:$2" "$1") +} + +# Leading zeros stripped so every segment is a plain integer — some version +# validators reject `08`, and a leading zero buys nothing. `0000` (midnight) +# must survive as `0`, not as the empty string. +strip0() { + printf '%s' "$1" | sed -e 's/^0*//' -e 's/^$/0/' +} + +# The IDENTITY of an artifact's content: the commit its shipped files last +# changed in. This — not the tag — is what decides whether a build can be +# skipped, because the published tag is only day-precise and two different +# builds can share it. +cmd_revision() { + echo "$(newest "$1")" | cut -d' ' -f2 | cut -c1-12 +} + +# The ORDERING KEY: full precision, YYYY.M.D.HHMM. Used by the extension, +# where the value is what Firefox compares to decide whether an update exists +# — two same-day builds MUST be distinguishable or the second never reaches +# anyone. +cmd_version() { + sha=$(echo "$(newest "$1")" | cut -d' ' -f2) + printf '%s.%s.%s.%s\n' \ + "$(fmt "$sha" %Y)" \ + "$(strip0 "$(fmt "$sha" %m)")" \ + "$(strip0 "$(fmt "$sha" %d)")" \ + "$(strip0 "$(fmt "$sha" %H%M)")" +} + +# The PUBLISHED IMAGE TAG: day precision, YYYY.M.D. Deliberately coarser than +# the ordering key, per the operator 2026-08-28 — same-day work is not +# something worth pinning, so a second build the same day replaces the first +# rather than accumulating a tag nobody would roll back to. Safe only because +# skip decisions key on cmd_revision, never on this. +cmd_tag() { + sha=$(echo "$(newest "$1")" | cut -d' ' -f2) + printf '%s.%s.%s\n' \ + "$(fmt "$sha" %Y)" \ + "$(strip0 "$(fmt "$sha" %m)")" \ + "$(strip0 "$(fmt "$sha" %d)")" +} + +[ $# -ge 2 ] || usage +case "$1" in + paths) cmd_paths "$2" ;; + revision) cmd_revision "$2" ;; + version) cmd_version "$2" ;; + tag) cmd_tag "$2" ;; + *) usage ;; +esac diff --git a/tests/test_artifact_paths.py b/tests/test_artifact_paths.py new file mode 100644 index 0000000..c226f5b --- /dev/null +++ b/tests/test_artifact_paths.py @@ -0,0 +1,138 @@ +"""`scripts/artifacts.sh` path sets must match what the Dockerfiles copy. + +Each published artifact's version derives from the newest commit touching its +own shipped file set (milestone 313). The whole scheme rests on those sets +being right, and both ways of being wrong are silent: + +* **too narrow** — a file ships but is not in the set, so the version does not + move when the content does, and a pin serves stale bytes. This is the + dangerous direction and the one this module exists for. +* **too wide** — a file is in the set but never reaches the image, so the + artifact re-versions and rebuilds for a change it does not ship. + +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. +""" +from __future__ import annotations + +import re +import subprocess +from pathlib import Path + +import pytest + +ROOT = Path(__file__).resolve().parent.parent + +# artifact -> (dockerfile, build context relative to the repo root) +ARTIFACTS = { + "web": ("Dockerfile", ""), + "ml": ("Dockerfile.ml", ""), + "agent": ("agent/Dockerfile", "agent"), +} + +# COPY --from= copies from an earlier build stage, not from the build +# context, so its source is not a repo path and cannot be in a path set. +_COPY = re.compile(r"^\s*COPY\s+(?!--from=)(?P.+)$", re.MULTILINE) + + +def declared_paths(artifact: str) -> list[str]: + out = subprocess.run( + ["sh", str(ROOT / "scripts" / "artifacts.sh"), "paths", artifact], + capture_output=True, text=True, check=True, cwd=ROOT, + ).stdout + return out.split() + + +def includes(artifact: str) -> list[str]: + """The set minus its `:(exclude)…` entries.""" + return [p for p in declared_paths(artifact) if not p.startswith(":(exclude)")] + + +def copy_sources(dockerfile: str, context: str) -> list[str]: + """Repo-relative sources of every context COPY in a Dockerfile.""" + text = (ROOT / dockerfile).read_text() + sources: list[str] = [] + for m in _COPY.finditer(text): + args = m.group("args").split() + # Last arg is the destination; everything before it is a source. + for src in args[:-1]: + # `frontend/package-lock.json*` — the glob is an optional-file + # idiom; the directory it sits in is what matters for coverage. + src = src.rstrip("*") + sources.append(f"{context}/{src}" if context else src) + return sources + + +def covered_by(path: str, include: str) -> bool: + """`path` ships if an include names it or one of its ancestors.""" + path = path.rstrip("/").lstrip("./") + include = include.rstrip("/") + return path == include or path.startswith(include + "/") + + +@pytest.mark.parametrize("artifact", sorted(ARTIFACTS)) +def test_every_copied_path_is_in_the_artifacts_path_set(artifact): + """The too-narrow direction — the one that serves stale bytes on a pin.""" + dockerfile, context = ARTIFACTS[artifact] + inc = includes(artifact) + for src in copy_sources(dockerfile, context): + assert any(covered_by(src, i) for i in inc), ( + f"{dockerfile} copies {src!r} into the {artifact} image, but no " + f"include in scripts/artifacts.sh covers it. The {artifact} " + f"version will not move when that file changes, so a pinned build " + f"will serve stale bytes. Add it to the path set.\n" + f" declared includes: {inc}" + ) + + +@pytest.mark.parametrize("artifact", sorted(ARTIFACTS)) +def test_the_dockerfile_itself_is_in_the_path_set(artifact): + """Changing a base image or a RUN changes the artifact as surely as + changing a source file, so each set must include its own Dockerfile.""" + dockerfile, _ = ARTIFACTS[artifact] + assert any(covered_by(dockerfile, i) for i in includes(artifact)), ( + f"{dockerfile} is not in the {artifact} path set — a base-image bump " + f"would not move the version." + ) + + +def test_the_web_image_versions_on_an_extension_change(): + """The web image bundles the signed XPI, so the extension's packaged files + are part of what it ships. Miss this and `:latest` serves a NEW extension + under an unchanged web version — a pin that quietly disagrees with itself. + """ + inc = includes("web") + assert any(covered_by("extension/background/background.js", i) for i in inc), ( + "the web path set does not cover the extension's packaged files, but " + "build.yml downloads the signed XPI into frontend/public/extension/ " + "before the docker build" + ) + + +@pytest.mark.parametrize( + "artifact, path", + [ + # 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. + ("agent", "agent/README.md"), + ("agent", "agent/ruff.toml"), + ("agent", "agent/docker-compose.yml"), + # vite builds from src/, index.html and public/; it never reads test/, + # so a frontend test change cannot reach `dist`. + ("web", "frontend/test/gallery.spec.js"), + ], +) +def test_files_that_never_reach_an_image_do_not_version_it(artifact, path): + paths = declared_paths(artifact) + excluded = [p[len(":(exclude)"):] for p in paths if p.startswith(":(exclude)")] + inc = [p for p in paths if not p.startswith(":(exclude)")] + + included = any(covered_by(path, i) for i in inc) + exempted = any(covered_by(path, e.rstrip("*").rstrip("/")) for e in excluded) + assert not included or exempted, ( + f"{path} is in the {artifact} path set but is not copied into the " + f"image — it would re-version and rebuild {artifact} for a change it " + f"does not ship." + ) From 0c43fa3eb2b7c0d7471d44c501bd20d8cc69f892 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Thu, 27 Aug 2026 21:41:14 -0400 Subject: [PATCH 2/5] ci: shadow the per-artifact derived versions (milestone 313 step 2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every build job now logs the tag, version and revision its artifact would get. Nothing reads them; no `set -e`, and each derivation falls back to UNAVAILABLE, so a broken script cannot fail a build. Same discipline as milestone 271 step 2, which is what made that cutover safe to do in one commit. Also fixes a landmine the plan named but had not checked: build-ml and build-agent were checking out at depth 1. Both now use fetch-depth: 0. That mattered more than it looks. A depth-1 clone sees one commit, so `git log HEAD -- ` either returns that commit's timestamp — plausible, and wrong — or returns nothing. For build-ml on this push it would have returned today's date, because HEAD touches backend/, and nothing downstream would have questioned it. For build-agent it would have returned nothing at all, since no single commit here touches agent/, and artifacts.sh exits non-zero rather than guessing. One direction is silent and one is loud; only the loud one was ever going to get noticed. What to read from the shadow lines over the next few pushes, in order of how badly each would bite: * a push touching the extension must move BOTH the extension and web, because build-web bakes the XPI in. If web does not move, its path set is too narrow and a pinned web image will serve an extension it does not name. * a push touching only agent/ must leave web and ml still. If they move, their sets are too wide and they will rebuild for changes they do not ship. * a docs-only push must move nothing. * dev and main must derive the same values for the same source. Step 3 only lets these values name a tag once those hold. --- .forgejo/workflows/build.yml | 98 ++++++++++++++++++++++++++++++++++++ 1 file changed, 98 insertions(+) diff --git a/.forgejo/workflows/build.yml b/.forgejo/workflows/build.yml index 43ab953..a641f19 100644 --- a/.forgejo/workflows/build.yml +++ b/.forgejo/workflows/build.yml @@ -103,6 +103,27 @@ jobs: # cache hit and holds AMO to one call per extension CHANGE. Only moving # backwards is a failure, so this runs on every path — cache hit # included — rather than only before a sign. + # --- shadow mode (milestone 313, step 2) ----------------------------- + # Informational ONLY. Nothing reads this and it must never fail the + # build — no `set -e`, and every derivation falls back to UNAVAILABLE. + # + # What to watch across pushes, because this is what step 3 will trust: + # * a push touching only agent/ moves the agent and leaves web and ml + # STILL. If web moves, its path set is too wide. + # * a push touching only docs moves nothing. + # * a push touching the extension moves the extension AND web, since + # web bakes in the XPI. If web does not move, its set is too narrow + # — the direction that serves stale bytes on a pin. + # * dev and main derive the same values for the same source. + - name: Shadow — derived artifact version (informational) + run: | + set -u + A=extension + T=$(sh scripts/artifacts.sh tag "$A" 2>&1 || echo UNAVAILABLE) + V=$(sh scripts/artifacts.sh version "$A" 2>&1 || echo UNAVAILABLE) + R=$(sh scripts/artifacts.sh revision "$A" 2>&1 || echo UNAVAILABLE) + echo "shadow: artifact=$A tag=$T version=$V revision=$R sha=$GITHUB_SHA" + - name: Guard — the derived version must never go backwards env: TOKEN: ${{ secrets.RELEASE_TOKEN }} @@ -320,6 +341,27 @@ jobs: # that exists perfectly well under its real name. fetch-depth: 0 + # --- shadow mode (milestone 313, step 2) ----------------------------- + # Informational ONLY. Nothing reads this and it must never fail the + # build — no `set -e`, and every derivation falls back to UNAVAILABLE. + # + # What to watch across pushes, because this is what step 3 will trust: + # * a push touching only agent/ moves the agent and leaves web and ml + # STILL. If web moves, its path set is too wide. + # * a push touching only docs moves nothing. + # * a push touching the extension moves the extension AND web, since + # web bakes in the XPI. If web does not move, its set is too narrow + # — the direction that serves stale bytes on a pin. + # * dev and main derive the same values for the same source. + - name: Shadow — derived artifact version (informational) + run: | + set -u + A=web + T=$(sh scripts/artifacts.sh tag "$A" 2>&1 || echo UNAVAILABLE) + V=$(sh scripts/artifacts.sh version "$A" 2>&1 || echo UNAVAILABLE) + R=$(sh scripts/artifacts.sh revision "$A" 2>&1 || echo UNAVAILABLE) + echo "shadow: artifact=$A tag=$T version=$V revision=$R sha=$GITHUB_SHA" + - name: Download signed XPI from Forgejo release asset # Fires on every trigger shape. dev and main each bundle the XPI their # own sign-extension just published — that is the whole point of the @@ -486,6 +528,34 @@ jobs: image: git.fabledsword.com/bvandeusen/ci-python:3.14 steps: - uses: actions/checkout@v4 + with: + # Full history: this job derives its artifact's version from the + # commit its shipped files last changed in (milestone 313). A + # depth-1 clone cannot see that commit — it either derives a wrong, + # too-low value or finds nothing at all, and neither is a failure + # the build would otherwise notice. + fetch-depth: 0 + + # --- shadow mode (milestone 313, step 2) ----------------------------- + # Informational ONLY. Nothing reads this and it must never fail the + # build — no `set -e`, and every derivation falls back to UNAVAILABLE. + # + # What to watch across pushes, because this is what step 3 will trust: + # * a push touching only agent/ moves the agent and leaves web and ml + # STILL. If web moves, its path set is too wide. + # * a push touching only docs moves nothing. + # * a push touching the extension moves the extension AND web, since + # web bakes in the XPI. If web does not move, its set is too narrow + # — the direction that serves stale bytes on a pin. + # * dev and main derive the same values for the same source. + - name: Shadow — derived artifact version (informational) + run: | + set -u + A=ml + T=$(sh scripts/artifacts.sh tag "$A" 2>&1 || echo UNAVAILABLE) + V=$(sh scripts/artifacts.sh version "$A" 2>&1 || echo UNAVAILABLE) + R=$(sh scripts/artifacts.sh revision "$A" 2>&1 || echo UNAVAILABLE) + echo "shadow: artifact=$A tag=$T version=$V revision=$R sha=$GITHUB_SHA" - name: Determine tag id: tag @@ -533,6 +603,34 @@ jobs: image: git.fabledsword.com/bvandeusen/ci-python:3.14 steps: - uses: actions/checkout@v4 + with: + # Full history: this job derives its artifact's version from the + # commit its shipped files last changed in (milestone 313). A + # depth-1 clone cannot see that commit — it either derives a wrong, + # too-low value or finds nothing at all, and neither is a failure + # the build would otherwise notice. + fetch-depth: 0 + + # --- shadow mode (milestone 313, step 2) ----------------------------- + # Informational ONLY. Nothing reads this and it must never fail the + # build — no `set -e`, and every derivation falls back to UNAVAILABLE. + # + # What to watch across pushes, because this is what step 3 will trust: + # * a push touching only agent/ moves the agent and leaves web and ml + # STILL. If web moves, its path set is too wide. + # * a push touching only docs moves nothing. + # * a push touching the extension moves the extension AND web, since + # web bakes in the XPI. If web does not move, its set is too narrow + # — the direction that serves stale bytes on a pin. + # * dev and main derive the same values for the same source. + - name: Shadow — derived artifact version (informational) + run: | + set -u + A=agent + T=$(sh scripts/artifacts.sh tag "$A" 2>&1 || echo UNAVAILABLE) + V=$(sh scripts/artifacts.sh version "$A" 2>&1 || echo UNAVAILABLE) + R=$(sh scripts/artifacts.sh revision "$A" 2>&1 || echo UNAVAILABLE) + echo "shadow: artifact=$A tag=$T version=$V revision=$R sha=$GITHUB_SHA" - name: Determine tag id: tag From 7a20c554411452d01319552108fa5a20a4904c63 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Thu, 27 Aug 2026 21:56:52 -0400 Subject: [PATCH 3/5] ci: publish a per-artifact date tag on main builds (milestone 313 step 3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Each main build now also publishes :YYYY.M.D — the date of the commit that artifact's shipped files last changed in. Purely additive; :main, :latest, :c- and the dev path are untouched, so reverting this commit reverts the behaviour. main push -> :main, :latest, :c-, :2026.8.27 dev push -> :dev Per artifact, so an image whose files did not change keeps the tag it already had. On this commit the agent reads 2026.7.17 while web and ml read 2026.8.27 — six weeks apart, from one push. Step 4 turns that into not rebuilding it. Day precision, and a second main build the same day replaces the first. Operator's call, and the reasoning is theirs: same-day work is not something worth pinning. A rollback goes to a day, not to the fourth merge of a Tuesday afternoon. It also makes retention mean "the last N days" rather than "the last N pushes". CALVER is computed inside the main branch rather than at the top of the step, and hard-fails when empty. There is no `set -e` here, so an unconditional assignment that failed would have left it empty and published the tag `fabledcurator:` — an invalid name, from a step that still reported success. It is also simply unused on the dev and tag paths. Fixed a stale comment while in this block rather than leaving it for step 7: it claimed release tags carry "no `.N` per family release-posture rule". Rule 148 was amended on 2026-08-24 to REQUIRE the suffix, after the ban caused a same-day tag to be retargeted and a release deleted to make room (note 2813). FC's own tags already carry suffixes; only the comment was asserting the superseded rule. Verified before pushing: the derivation holds across 200 commits of real history — a derived revision always touches its own path set, the version never decreases along any parent->child edge, and web tracks all three extension-only commits in the log. That last one is the direction that would serve stale bytes on a pin. --- .forgejo/workflows/build.yml | 68 +++++++++++++++++++++++++++++++++--- 1 file changed, 64 insertions(+), 4 deletions(-) diff --git a/.forgejo/workflows/build.yml b/.forgejo/workflows/build.yml index a641f19..cd61c3e 100644 --- a/.forgejo/workflows/build.yml +++ b/.forgejo/workflows/build.yml @@ -446,7 +446,10 @@ jobs: run: | # Three trigger shapes: # refs/tags/v… → tag-push: opt-in milestone label (vYY.MM.DD, - # no `.N` per family release-posture rule). + # plus `.N` when the day already carries a tag — + # family rule 148, amended 2026-08-24 after a + # same-day tag was retargeted and a release + # deleted to make room, note 2813). # Publish ONLY the immutable version tag; # don't touch :latest (the main-push build # for the merge commit already did that). @@ -468,6 +471,15 @@ jobs: # everywhere). Operator-flagged 2026-06-01 after first :c- # main-push build failed at this step. SHORT_SHA=$(printf '%s' "$GITHUB_SHA" | cut -c1-7) + # The pinnable tag (milestone 313 step 3): YYYY.M.D of the commit + # THIS artifact's shipped files last changed in. Day precision is + # deliberate — same-day work is not something worth pinning, so a + # second main build the same day replaces the first rather than + # accumulating a tag nobody would roll back to. + # + # Derived per artifact, so an image whose files did not change keeps + # the tag it already had: the agent reads 2026.7.17 today while web + # reads 2026.8.27. Step 4 uses that to stop rebuilding it at all. # `channel` is baked into the image as FC_CHANNEL and reported by # /api/extension/manifest (milestone 271 step 7). A tag-push counts as # `main`: a vYY.MM.DD tag is cut from main, so that image is a @@ -477,7 +489,17 @@ jobs: echo "tags=git.fabledsword.com/bvandeusen/fabledcurator:${TAG_NAME}" >> "$GITHUB_OUTPUT" echo "channel=main" >> "$GITHUB_OUTPUT" elif [ "${GITHUB_REF##*/}" = "main" ]; then - echo "tags=git.fabledsword.com/bvandeusen/fabledcurator:main,git.fabledsword.com/bvandeusen/fabledcurator:latest,git.fabledsword.com/bvandeusen/fabledcurator:c-${SHORT_SHA}" >> "$GITHUB_OUTPUT" + CALVER=$(sh scripts/artifacts.sh tag web) + # Guarded, and computed only on this path. There is no `set -e` in + # this step, so a failed derivation would otherwise leave CALVER + # empty and publish the tag `fabledcurator:` — an invalid + # name, from a green step. An empty pin must never reach the + # registry. + if [ -z "$CALVER" ]; then + echo "ERROR: could not derive a web version tag" >&2 + exit 1 + fi + echo "tags=git.fabledsword.com/bvandeusen/fabledcurator:main,git.fabledsword.com/bvandeusen/fabledcurator:latest,git.fabledsword.com/bvandeusen/fabledcurator:c-${SHORT_SHA},git.fabledsword.com/bvandeusen/fabledcurator:${CALVER}" >> "$GITHUB_OUTPUT" echo "channel=main" >> "$GITHUB_OUTPUT" else echo "tags=git.fabledsword.com/bvandeusen/fabledcurator:dev" >> "$GITHUB_OUTPUT" @@ -569,11 +591,30 @@ jobs: # everywhere). Operator-flagged 2026-06-01 after first :c- # main-push build failed at this step. SHORT_SHA=$(printf '%s' "$GITHUB_SHA" | cut -c1-7) + # The pinnable tag (milestone 313 step 3): YYYY.M.D of the commit + # THIS artifact's shipped files last changed in. Day precision is + # deliberate — same-day work is not something worth pinning, so a + # second main build the same day replaces the first rather than + # accumulating a tag nobody would roll back to. + # + # Derived per artifact, so an image whose files did not change keeps + # the tag it already had: the agent reads 2026.7.17 today while web + # reads 2026.8.27. Step 4 uses that to stop rebuilding it at all. if [ "${GITHUB_REF#refs/tags/}" != "${GITHUB_REF}" ]; then TAG_NAME="${GITHUB_REF#refs/tags/}" echo "tags=git.fabledsword.com/bvandeusen/fabledcurator-ml:${TAG_NAME}" >> "$GITHUB_OUTPUT" elif [ "${GITHUB_REF##*/}" = "main" ]; then - echo "tags=git.fabledsword.com/bvandeusen/fabledcurator-ml:main,git.fabledsword.com/bvandeusen/fabledcurator-ml:latest,git.fabledsword.com/bvandeusen/fabledcurator-ml:c-${SHORT_SHA}" >> "$GITHUB_OUTPUT" + CALVER=$(sh scripts/artifacts.sh tag ml) + # Guarded, and computed only on this path. There is no `set -e` in + # this step, so a failed derivation would otherwise leave CALVER + # empty and publish the tag `fabledcurator-ml:` — an invalid + # name, from a green step. An empty pin must never reach the + # registry. + if [ -z "$CALVER" ]; then + echo "ERROR: could not derive a ml version tag" >&2 + exit 1 + fi + echo "tags=git.fabledsword.com/bvandeusen/fabledcurator-ml:main,git.fabledsword.com/bvandeusen/fabledcurator-ml:latest,git.fabledsword.com/bvandeusen/fabledcurator-ml:c-${SHORT_SHA},git.fabledsword.com/bvandeusen/fabledcurator-ml:${CALVER}" >> "$GITHUB_OUTPUT" else echo "tags=git.fabledsword.com/bvandeusen/fabledcurator-ml:dev" >> "$GITHUB_OUTPUT" fi @@ -636,11 +677,30 @@ jobs: id: tag run: | SHORT_SHA=$(printf '%s' "$GITHUB_SHA" | cut -c1-7) + # The pinnable tag (milestone 313 step 3): YYYY.M.D of the commit + # THIS artifact's shipped files last changed in. Day precision is + # deliberate — same-day work is not something worth pinning, so a + # second main build the same day replaces the first rather than + # accumulating a tag nobody would roll back to. + # + # Derived per artifact, so an image whose files did not change keeps + # the tag it already had: the agent reads 2026.7.17 today while web + # reads 2026.8.27. Step 4 uses that to stop rebuilding it at all. if [ "${GITHUB_REF#refs/tags/}" != "${GITHUB_REF}" ]; then TAG_NAME="${GITHUB_REF#refs/tags/}" echo "tags=git.fabledsword.com/bvandeusen/fabledcurator-agent:${TAG_NAME}" >> "$GITHUB_OUTPUT" elif [ "${GITHUB_REF##*/}" = "main" ]; then - echo "tags=git.fabledsword.com/bvandeusen/fabledcurator-agent:main,git.fabledsword.com/bvandeusen/fabledcurator-agent:latest,git.fabledsword.com/bvandeusen/fabledcurator-agent:c-${SHORT_SHA}" >> "$GITHUB_OUTPUT" + CALVER=$(sh scripts/artifacts.sh tag agent) + # Guarded, and computed only on this path. There is no `set -e` in + # this step, so a failed derivation would otherwise leave CALVER + # empty and publish the tag `fabledcurator-agent:` — an invalid + # name, from a green step. An empty pin must never reach the + # registry. + if [ -z "$CALVER" ]; then + echo "ERROR: could not derive a agent version tag" >&2 + exit 1 + fi + echo "tags=git.fabledsword.com/bvandeusen/fabledcurator-agent:main,git.fabledsword.com/bvandeusen/fabledcurator-agent:latest,git.fabledsword.com/bvandeusen/fabledcurator-agent:c-${SHORT_SHA},git.fabledsword.com/bvandeusen/fabledcurator-agent:${CALVER}" >> "$GITHUB_OUTPUT" else echo "tags=git.fabledsword.com/bvandeusen/fabledcurator-agent:dev" >> "$GITHUB_OUTPUT" fi From 609bc82acc1fe38f5c31283259eda980cd4724db Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Fri, 28 Aug 2026 08:24:58 -0400 Subject: [PATCH 4/5] ci: reuse the published image instead of rebuilding it (milestone 313 step 4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before building, each job asks the registry whether this artifact's content is already published. On a hit it skips the build entirely and repoints the channel and date tags at the existing manifest with `imagetools create` — registry-side, no layer transfer, seconds. This is the step that stops a push touching only `agent/` from rebuilding web and ml, and stops a merge to main rebuilding what dev already built. The question is asked with a new `artifacts.sh identity`, not with the date tag: the date tag is day-precise and last-one-wins, so two different builds share it and it cannot answer "is this content published?". The commit sha would move on every push and never hit, which is the redundant rebuild being removed. The revision does both jobs — content-unique, and stable across pushes that did not touch the artifact. Identity is channel-qualified for web and only for web, because web is the only image that takes a build-arg: FC_CHANNEL is baked in and reported by /api/extension/manifest, so its dev and main builds of one revision are genuinely different images. ml and agent take none, which is what lets a merge reuse dev's build rather than rebuilding the agent's CUDA image to produce bytes that already exist. tests/test_artifact_identity.py reads the Dockerfiles and fails if that list drifts from the ARG declarations, in either direction — collapsing the channels ships an instance that reports the wrong one, and splitting them needlessly rebuilds every merge. Failure direction is deliberate: an inspect that errors for any reason reads as a miss and the build runs. Only a real 200 skips one. A tag-push never claims the identity. It rebuilds a revision main already published, and image configs are not bit-reproducible, so re-pushing r- would point an immutable tag at fresh bytes — rule 145's exact prohibition. It publishes only its own :v... label and otherwise reuses. Base-image freshness, decided rather than left implicit: an artifact whose source stops moving stops picking up base updates under its pinned tag. That is what a pin means, and rule 145 already says the refresh belongs on the moving tag instead. Filed as #3154 rather than folded in here, because the naive version regresses :latest on the next unrelated push. ci.yml's backend lane gains fetch-depth: 0 — the new tests derive real revisions, and on a depth-1 clone that derivation returns the tip sha or fails, so the lane would go green while asserting nothing. The three build jobs' shadow steps are renamed and re-commented: those values stopped being informational at step 3, and a step captioned "nothing reads this" beside steps that do is worse than no caption. --- .forgejo/workflows/build.yml | 521 +++++++++++++++++++++++++------- .forgejo/workflows/ci.yml | 7 + scripts/artifacts.sh | 56 ++++ tests/test_artifact_identity.py | 140 +++++++++ 4 files changed, 609 insertions(+), 115 deletions(-) create mode 100644 tests/test_artifact_identity.py diff --git a/.forgejo/workflows/build.yml b/.forgejo/workflows/build.yml index cd61c3e..c8a84f6 100644 --- a/.forgejo/workflows/build.yml +++ b/.forgejo/workflows/build.yml @@ -122,7 +122,7 @@ jobs: T=$(sh scripts/artifacts.sh tag "$A" 2>&1 || echo UNAVAILABLE) V=$(sh scripts/artifacts.sh version "$A" 2>&1 || echo UNAVAILABLE) R=$(sh scripts/artifacts.sh revision "$A" 2>&1 || echo UNAVAILABLE) - echo "shadow: artifact=$A tag=$T version=$V revision=$R sha=$GITHUB_SHA" + echo "derived: artifact=$A tag=$T version=$V revision=$R sha=$GITHUB_SHA" - name: Guard — the derived version must never go backwards env: @@ -341,11 +341,19 @@ jobs: # that exists perfectly well under its real name. fetch-depth: 0 - # --- shadow mode (milestone 313, step 2) ----------------------------- - # Informational ONLY. Nothing reads this and it must never fail the - # build — no `set -e`, and every derivation falls back to UNAVAILABLE. + # --- derived values, one line (milestone 313) ------------------------ + # These stopped being shadow output at step 3: `tag` is published on + # main and `revision` decides whether the build below runs at all. This + # step prints all three anyway, because the load-bearing steps each + # print only the one they use, and on dev the date tag is computed + # nowhere else. When a build is skipped or a pin looks wrong, this is + # the line that says what the commit derived. # - # What to watch across pushes, because this is what step 3 will trust: + # Still diagnostic, so it still must not fail the build — no `set -e`, + # and every derivation falls back to UNAVAILABLE. A broken echo must + # never be the reason an image does not ship. + # + # What it should say: # * a push touching only agent/ moves the agent and leaves web and ml # STILL. If web moves, its path set is too wide. # * a push touching only docs moves nothing. @@ -353,14 +361,162 @@ jobs: # web bakes in the XPI. If web does not move, its set is too narrow # — the direction that serves stale bytes on a pin. # * dev and main derive the same values for the same source. - - name: Shadow — derived artifact version (informational) + - name: Report the derived artifact version run: | set -u A=web T=$(sh scripts/artifacts.sh tag "$A" 2>&1 || echo UNAVAILABLE) V=$(sh scripts/artifacts.sh version "$A" 2>&1 || echo UNAVAILABLE) R=$(sh scripts/artifacts.sh revision "$A" 2>&1 || echo UNAVAILABLE) - echo "shadow: artifact=$A tag=$T version=$V revision=$R sha=$GITHUB_SHA" + echo "derived: artifact=$A tag=$T version=$V revision=$R sha=$GITHUB_SHA" + + - name: Determine tag + id: tag + run: | + # Three trigger shapes: + # refs/tags/v… → tag-push: opt-in milestone label (vYY.MM.DD, + # plus `.N` when the day already carries a tag — + # family rule 148, amended 2026-08-24 after a + # same-day tag was retargeted and a release + # deleted to make room, note 2813). + # Publish ONLY the immutable version tag; + # don't touch :latest (the main-push build + # for the merge commit already did that). + # refs/heads/main → push to main: publish :main + :latest + # (floating) AND :c- (immutable + # per-commit rollback substrate, per family + # release-posture rule "Tags are milestones, + # not gates — commit-SHA images are the + # rollback unit"). Rollback to any commit + # becomes `docker pull …:c-` without a + # release ceremony. + # refs/heads/dev → push to dev: publish :dev, the rolling test + # channel (family rule 146). Rolling means it may + # carry newer contents than the :c- of the + # same commit; it never writes :c- itself, + # because that is the rollback unit (rule 145). + # POSIX-safe substring (the runner shell is dash/BusyBox sh, not + # bash — `${var:0:7}` errors with "Bad substitution"; cut works + # everywhere). Operator-flagged 2026-06-01 after first :c- + # main-push build failed at this step. + SHORT_SHA=$(printf '%s' "$GITHUB_SHA" | cut -c1-7) + # The pinnable tag (milestone 313 step 3): YYYY.M.D of the commit + # THIS artifact's shipped files last changed in. Day precision is + # deliberate — same-day work is not something worth pinning, so a + # second main build the same day replaces the first rather than + # accumulating a tag nobody would roll back to. + # + # Derived per artifact, so an image whose files did not change keeps + # the tag it already had: the agent reads 2026.7.17 today while web + # reads 2026.8.27 — and the reuse step below turns that into a + # skipped build rather than a rebuild of bytes that already exist. + # `channel` is baked into the image as FC_CHANNEL and reported by + # /api/extension/manifest (milestone 271 step 7). A tag-push counts as + # `main`: a vYY.MM.DD tag is cut from main, so that image is a + # main-channel artifact wearing an immutable name. + if [ "${GITHUB_REF#refs/tags/}" != "${GITHUB_REF}" ]; then + TAG_NAME="${GITHUB_REF#refs/tags/}" + echo "tags=git.fabledsword.com/bvandeusen/fabledcurator:${TAG_NAME}" >> "$GITHUB_OUTPUT" + echo "channel=main" >> "$GITHUB_OUTPUT" + elif [ "${GITHUB_REF##*/}" = "main" ]; then + CALVER=$(sh scripts/artifacts.sh tag web) + # Guarded, and computed only on this path. There is no `set -e` in + # this step, so a failed derivation would otherwise leave CALVER + # empty and publish the tag `fabledcurator:` — an invalid + # name, from a green step. An empty pin must never reach the + # registry. + if [ -z "$CALVER" ]; then + echo "ERROR: could not derive a web version tag" >&2 + exit 1 + fi + echo "tags=git.fabledsword.com/bvandeusen/fabledcurator:main,git.fabledsword.com/bvandeusen/fabledcurator:latest,git.fabledsword.com/bvandeusen/fabledcurator:c-${SHORT_SHA},git.fabledsword.com/bvandeusen/fabledcurator:${CALVER}" >> "$GITHUB_OUTPUT" + echo "channel=main" >> "$GITHUB_OUTPUT" + else + echo "tags=git.fabledsword.com/bvandeusen/fabledcurator:dev" >> "$GITHUB_OUTPUT" + echo "channel=dev" >> "$GITHUB_OUTPUT" + fi + + # A shell step, not docker/login-action@v3, because the action's shared + # cache races itself (#3118). act_runner caches a remote action under one + # /root/.cache/act/ per runner, and build-web, build-ml and + # build-agent all start in the same second and all want this same action. + # One job re-clones the directory — which empties and repopulates it — + # while another is walking it to copy into its container, and the walker + # lstat()s a file that has just vanished. It failed twice on 2026-08-27, + # naming a DIFFERENT missing file each time (`eslint.config.mjs`, then + # `jest.config.ts`), which is what rules out a corrupt cache and points at + # a race. The loser dies with MODULE_NOT_FOUND on dist/index.js before the + # action runs at all, so the secret is never even reached. + # + # Nothing is lost by dropping it: logging in is one command, the docker + # CLI is already in the CI image (ci-requirements.md), and the same + # reasoning as family rule 5 applies — a marketplace action buys nothing + # when the tool is baked into the image the workflow already selected. + # + # Password on stdin, never as an argument: an argument lands in the + # process table and draws docker's own deprecation warning. + - name: Login to Forgejo registry + env: + TOKEN: ${{ secrets.RELEASE_TOKEN }} + ACTOR: ${{ github.actor }} + run: echo "$TOKEN" | docker login git.fabledsword.com -u "$ACTOR" --password-stdin + + # --- reuse-if-published (milestone 313, step 4) ---------------------- + # The identity tag names this artifact's CONTENT — r-, the + # commit its shipped files last changed in, plus the channel for images + # that bake one in. If the registry already carries it, the bytes this + # job would produce are already published and the build is pure waste: + # the channel and date tags get repointed at the existing manifest + # instead, registry-side, in seconds. + # + # This is what stops a push that touched only `agent/` from rebuilding + # web and ml, and a merge to main from rebuilding what dev already built. + # + # The failure direction is deliberate. An inspect that errors for ANY + # reason — network, auth, a registry hiccup — reads as a miss and the + # build runs. Only a genuine 200 skips one, so there is no path here + # that skips a build that was actually needed; the worst case is paying + # for a build we could have avoided. + # + # BASE-IMAGE FRESHNESS, decided rather than left implicit: an artifact + # whose source stops moving stops picking up base-image updates under + # its pinned tag. That is what a pin MEANS — a date tag has to keep + # serving the bytes it served (fabledcurator:2026.7.17 still + # resolves to July's image), or it is not a pin — and family rule + # 145 already says where the refresh goes instead: a rebuild with + # different contents publishes only the MOVING tag, never the immutable + # one. A scheduled channel-only refresh is tracked separately (#3154); + # it does not belong in the push path. + - name: Is this content already published? + id: reuse + env: + IMAGE: git.fabledsword.com/bvandeusen/fabledcurator + CHANNEL: ${{ steps.tag.outputs.channel }} + TAGS: ${{ steps.tag.outputs.tags }} + IS_TAG_PUSH: ${{ startsWith(github.ref, 'refs/tags/') }} + run: | + set -eu + ID=$(sh scripts/artifacts.sh identity web "$CHANNEL") + echo "identity=$ID" >> "$GITHUB_OUTPUT" + + # A tag-push builds a revision that main already published, so it + # must NOT claim the identity: image configs are not bit-reproducible + # (embedded timestamps), so re-pushing r- would point an + # immutable tag at fresh bytes — rule 145's exact prohibition. It + # publishes only its own :v… label and otherwise reuses. + if [ "$IS_TAG_PUSH" = "true" ]; then + echo "build_tags=$TAGS" >> "$GITHUB_OUTPUT" + else + echo "build_tags=$TAGS,$IMAGE:$ID" >> "$GITHUB_OUTPUT" + fi + + if docker buildx imagetools inspect "$IMAGE:$ID" >/dev/null 2>&1; then + echo "hit=true" >> "$GITHUB_OUTPUT" + echo "reuse: $IMAGE:$ID is already published — skipping the build" + else + echo "hit=false" >> "$GITHUB_OUTPUT" + echo "reuse: $IMAGE:$ID is not published — building" + fi - name: Download signed XPI from Forgejo release asset # Fires on every trigger shape. dev and main each bundle the XPI their @@ -381,7 +537,10 @@ jobs: # for up to 10min total) before giving up. Main-push's signing # eventually wins and tag-push picks the release up on a later # iteration. - if: github.ref == 'refs/heads/main' || github.ref == 'refs/heads/dev' || startsWith(github.ref, 'refs/tags/') + # Gated on the reuse miss as well: if the image is already published it + # already contains its XPI, so this would download (and on a tag-push, + # poll up to 10 minutes for) a file nothing then reads. + if: steps.reuse.outputs.hit != 'true' && (github.ref == 'refs/heads/main' || github.ref == 'refs/heads/dev' || startsWith(github.ref, 'refs/tags/')) env: TOKEN: ${{ secrets.RELEASE_TOKEN }} run: | @@ -441,109 +600,47 @@ jobs: cp "$DEST" "frontend/public/extension/fabledcurator-latest.xpi" ls -la frontend/public/extension/ - - name: Determine tag - id: tag - run: | - # Three trigger shapes: - # refs/tags/v… → tag-push: opt-in milestone label (vYY.MM.DD, - # plus `.N` when the day already carries a tag — - # family rule 148, amended 2026-08-24 after a - # same-day tag was retargeted and a release - # deleted to make room, note 2813). - # Publish ONLY the immutable version tag; - # don't touch :latest (the main-push build - # for the merge commit already did that). - # refs/heads/main → push to main: publish :main + :latest - # (floating) AND :c- (immutable - # per-commit rollback substrate, per family - # release-posture rule "Tags are milestones, - # not gates — commit-SHA images are the - # rollback unit"). Rollback to any commit - # becomes `docker pull …:c-` without a - # release ceremony. - # refs/heads/dev → push to dev: publish :dev, the rolling test - # channel (family rule 146). Rolling means it may - # carry newer contents than the :c- of the - # same commit; it never writes :c- itself, - # because that is the rollback unit (rule 145). - # POSIX-safe substring (the runner shell is dash/BusyBox sh, not - # bash — `${var:0:7}` errors with "Bad substitution"; cut works - # everywhere). Operator-flagged 2026-06-01 after first :c- - # main-push build failed at this step. - SHORT_SHA=$(printf '%s' "$GITHUB_SHA" | cut -c1-7) - # The pinnable tag (milestone 313 step 3): YYYY.M.D of the commit - # THIS artifact's shipped files last changed in. Day precision is - # deliberate — same-day work is not something worth pinning, so a - # second main build the same day replaces the first rather than - # accumulating a tag nobody would roll back to. - # - # Derived per artifact, so an image whose files did not change keeps - # the tag it already had: the agent reads 2026.7.17 today while web - # reads 2026.8.27. Step 4 uses that to stop rebuilding it at all. - # `channel` is baked into the image as FC_CHANNEL and reported by - # /api/extension/manifest (milestone 271 step 7). A tag-push counts as - # `main`: a vYY.MM.DD tag is cut from main, so that image is a - # main-channel artifact wearing an immutable name. - if [ "${GITHUB_REF#refs/tags/}" != "${GITHUB_REF}" ]; then - TAG_NAME="${GITHUB_REF#refs/tags/}" - echo "tags=git.fabledsword.com/bvandeusen/fabledcurator:${TAG_NAME}" >> "$GITHUB_OUTPUT" - echo "channel=main" >> "$GITHUB_OUTPUT" - elif [ "${GITHUB_REF##*/}" = "main" ]; then - CALVER=$(sh scripts/artifacts.sh tag web) - # Guarded, and computed only on this path. There is no `set -e` in - # this step, so a failed derivation would otherwise leave CALVER - # empty and publish the tag `fabledcurator:` — an invalid - # name, from a green step. An empty pin must never reach the - # registry. - if [ -z "$CALVER" ]; then - echo "ERROR: could not derive a web version tag" >&2 - exit 1 - fi - echo "tags=git.fabledsword.com/bvandeusen/fabledcurator:main,git.fabledsword.com/bvandeusen/fabledcurator:latest,git.fabledsword.com/bvandeusen/fabledcurator:c-${SHORT_SHA},git.fabledsword.com/bvandeusen/fabledcurator:${CALVER}" >> "$GITHUB_OUTPUT" - echo "channel=main" >> "$GITHUB_OUTPUT" - else - echo "tags=git.fabledsword.com/bvandeusen/fabledcurator:dev" >> "$GITHUB_OUTPUT" - echo "channel=dev" >> "$GITHUB_OUTPUT" - fi - - # A shell step, not docker/login-action@v3, because the action's shared - # cache races itself (#3118). act_runner caches a remote action under one - # /root/.cache/act/ per runner, and build-web, build-ml and - # build-agent all start in the same second and all want this same action. - # One job re-clones the directory — which empties and repopulates it — - # while another is walking it to copy into its container, and the walker - # lstat()s a file that has just vanished. It failed twice on 2026-08-27, - # naming a DIFFERENT missing file each time (`eslint.config.mjs`, then - # `jest.config.ts`), which is what rules out a corrupt cache and points at - # a race. The loser dies with MODULE_NOT_FOUND on dist/index.js before the - # action runs at all, so the secret is never even reached. - # - # Nothing is lost by dropping it: logging in is one command, the docker - # CLI is already in the CI image (ci-requirements.md), and the same - # reasoning as family rule 5 applies — a marketplace action buys nothing - # when the tool is baked into the image the workflow already selected. - # - # Password on stdin, never as an argument: an argument lands in the - # process table and draws docker's own deprecation warning. - - name: Login to Forgejo registry - env: - TOKEN: ${{ secrets.RELEASE_TOKEN }} - ACTOR: ${{ github.actor }} - run: echo "$TOKEN" | docker login git.fabledsword.com -u "$ACTOR" --password-stdin - - name: Build and push web image + if: steps.reuse.outputs.hit != 'true' uses: docker/build-push-action@v5 with: context: . file: Dockerfile push: true - tags: ${{ steps.tag.outputs.tags }} + tags: ${{ steps.reuse.outputs.build_tags }} # Only the web image carries a channel: it is the one that serves # /api/extension/manifest. The ml and agent images have nothing to # report it to. build-args: | FC_CHANNEL=${{ steps.tag.outputs.channel }} + # Registry-side manifest copy: no layer transfer, no local daemon, no + # rebuild. Each -t becomes another reference to the SAME manifest the + # identity tag holds, so :latest and the date pin are byte-identical to + # what was 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' + env: + IMAGE: git.fabledsword.com/bvandeusen/fabledcurator + IDENTITY: ${{ steps.reuse.outputs.identity }} + TAGS: ${{ steps.tag.outputs.tags }} + run: | + set -euf + # steps.tag emits ONE comma-separated list, because that is the shape + # docker/build-push-action takes; imagetools wants a -t per ref. + ARGS="" + IFS=, + for t in $TAGS; do ARGS="$ARGS -t $t"; done + unset IFS + # shellcheck disable=SC2086 + docker buildx imagetools create $ARGS "$IMAGE:$IDENTITY" + echo "repointed to $IMAGE:$IDENTITY: $TAGS" + build-ml: runs-on: python-ci container: @@ -558,11 +655,19 @@ jobs: # the build would otherwise notice. fetch-depth: 0 - # --- shadow mode (milestone 313, step 2) ----------------------------- - # Informational ONLY. Nothing reads this and it must never fail the - # build — no `set -e`, and every derivation falls back to UNAVAILABLE. + # --- derived values, one line (milestone 313) ------------------------ + # These stopped being shadow output at step 3: `tag` is published on + # main and `revision` decides whether the build below runs at all. This + # step prints all three anyway, because the load-bearing steps each + # print only the one they use, and on dev the date tag is computed + # nowhere else. When a build is skipped or a pin looks wrong, this is + # the line that says what the commit derived. # - # What to watch across pushes, because this is what step 3 will trust: + # Still diagnostic, so it still must not fail the build — no `set -e`, + # and every derivation falls back to UNAVAILABLE. A broken echo must + # never be the reason an image does not ship. + # + # What it should say: # * a push touching only agent/ moves the agent and leaves web and ml # STILL. If web moves, its path set is too wide. # * a push touching only docs moves nothing. @@ -570,14 +675,14 @@ jobs: # web bakes in the XPI. If web does not move, its set is too narrow # — the direction that serves stale bytes on a pin. # * dev and main derive the same values for the same source. - - name: Shadow — derived artifact version (informational) + - name: Report the derived artifact version run: | set -u A=ml T=$(sh scripts/artifacts.sh tag "$A" 2>&1 || echo UNAVAILABLE) V=$(sh scripts/artifacts.sh version "$A" 2>&1 || echo UNAVAILABLE) R=$(sh scripts/artifacts.sh revision "$A" 2>&1 || echo UNAVAILABLE) - echo "shadow: artifact=$A tag=$T version=$V revision=$R sha=$GITHUB_SHA" + echo "derived: artifact=$A tag=$T version=$V revision=$R sha=$GITHUB_SHA" - name: Determine tag id: tag @@ -599,10 +704,12 @@ jobs: # # Derived per artifact, so an image whose files did not change keeps # the tag it already had: the agent reads 2026.7.17 today while web - # reads 2026.8.27. Step 4 uses that to stop rebuilding it at all. + # reads 2026.8.27 — and the reuse step below turns that into a + # skipped build rather than a rebuild of bytes that already exist. if [ "${GITHUB_REF#refs/tags/}" != "${GITHUB_REF}" ]; then TAG_NAME="${GITHUB_REF#refs/tags/}" echo "tags=git.fabledsword.com/bvandeusen/fabledcurator-ml:${TAG_NAME}" >> "$GITHUB_OUTPUT" + echo "channel=main" >> "$GITHUB_OUTPUT" elif [ "${GITHUB_REF##*/}" = "main" ]; then CALVER=$(sh scripts/artifacts.sh tag ml) # Guarded, and computed only on this path. There is no `set -e` in @@ -615,8 +722,10 @@ jobs: exit 1 fi echo "tags=git.fabledsword.com/bvandeusen/fabledcurator-ml:main,git.fabledsword.com/bvandeusen/fabledcurator-ml:latest,git.fabledsword.com/bvandeusen/fabledcurator-ml:c-${SHORT_SHA},git.fabledsword.com/bvandeusen/fabledcurator-ml:${CALVER}" >> "$GITHUB_OUTPUT" + echo "channel=main" >> "$GITHUB_OUTPUT" else echo "tags=git.fabledsword.com/bvandeusen/fabledcurator-ml:dev" >> "$GITHUB_OUTPUT" + echo "channel=dev" >> "$GITHUB_OUTPUT" fi # Shell step rather than docker/login-action — see build-web's note on @@ -627,13 +736,98 @@ jobs: ACTOR: ${{ github.actor }} run: echo "$TOKEN" | docker login git.fabledsword.com -u "$ACTOR" --password-stdin + # --- reuse-if-published (milestone 313, step 4) ---------------------- + # The identity tag names this artifact's CONTENT — r-, the + # commit its shipped files last changed in, plus the channel for images + # that bake one in. If the registry already carries it, the bytes this + # job would produce are already published and the build is pure waste: + # the channel and date tags get repointed at the existing manifest + # instead, registry-side, in seconds. + # + # This is what stops a push that touched only `agent/` from rebuilding + # web and ml, and a merge to main from rebuilding what dev already built. + # + # The failure direction is deliberate. An inspect that errors for ANY + # reason — network, auth, a registry hiccup — reads as a miss and the + # build runs. Only a genuine 200 skips one, so there is no path here + # that skips a build that was actually needed; the worst case is paying + # for a build we could have avoided. + # + # BASE-IMAGE FRESHNESS, decided rather than left implicit: an artifact + # whose source stops moving stops picking up base-image updates under + # its pinned tag. That is what a pin MEANS — a date tag has to keep + # serving the bytes it served (fabledcurator-ml:2026.7.17 still + # resolves to July's image), or it is not a pin — and family rule + # 145 already says where the refresh goes instead: a rebuild with + # different contents publishes only the MOVING tag, never the immutable + # one. A scheduled channel-only refresh is tracked separately (#3154); + # it does not belong in the push path. + - name: Is this content already published? + id: reuse + env: + IMAGE: git.fabledsword.com/bvandeusen/fabledcurator-ml + CHANNEL: ${{ steps.tag.outputs.channel }} + TAGS: ${{ steps.tag.outputs.tags }} + IS_TAG_PUSH: ${{ startsWith(github.ref, 'refs/tags/') }} + run: | + set -eu + ID=$(sh scripts/artifacts.sh identity ml "$CHANNEL") + echo "identity=$ID" >> "$GITHUB_OUTPUT" + + # A tag-push builds a revision that main already published, so it + # must NOT claim the identity: image configs are not bit-reproducible + # (embedded timestamps), so re-pushing r- would point an + # immutable tag at fresh bytes — rule 145's exact prohibition. It + # publishes only its own :v… label and otherwise reuses. + if [ "$IS_TAG_PUSH" = "true" ]; then + echo "build_tags=$TAGS" >> "$GITHUB_OUTPUT" + else + echo "build_tags=$TAGS,$IMAGE:$ID" >> "$GITHUB_OUTPUT" + fi + + if docker buildx imagetools inspect "$IMAGE:$ID" >/dev/null 2>&1; then + echo "hit=true" >> "$GITHUB_OUTPUT" + echo "reuse: $IMAGE:$ID is already published — skipping the build" + else + echo "hit=false" >> "$GITHUB_OUTPUT" + echo "reuse: $IMAGE:$ID is not published — building" + fi + - name: Build and push ml image + if: steps.reuse.outputs.hit != 'true' uses: docker/build-push-action@v5 with: context: . file: Dockerfile.ml push: true - tags: ${{ steps.tag.outputs.tags }} + tags: ${{ steps.reuse.outputs.build_tags }} + + # Registry-side manifest copy: no layer transfer, no local daemon, no + # rebuild. Each -t becomes another reference to the SAME manifest the + # identity tag holds, so :latest and the date pin are byte-identical to + # what was 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' + env: + IMAGE: git.fabledsword.com/bvandeusen/fabledcurator-ml + IDENTITY: ${{ steps.reuse.outputs.identity }} + TAGS: ${{ steps.tag.outputs.tags }} + run: | + set -euf + # steps.tag emits ONE comma-separated list, because that is the shape + # docker/build-push-action takes; imagetools wants a -t per ref. + ARGS="" + IFS=, + for t in $TAGS; do ARGS="$ARGS -t $t"; done + unset IFS + # shellcheck disable=SC2086 + docker buildx imagetools create $ARGS "$IMAGE:$IDENTITY" + echo "repointed to $IMAGE:$IDENTITY: $TAGS" # The desktop GPU agent (#114) — published so the operator pulls + runs it on # the GPU machine instead of building locally. Independent of web/ml (its own @@ -652,11 +846,19 @@ jobs: # the build would otherwise notice. fetch-depth: 0 - # --- shadow mode (milestone 313, step 2) ----------------------------- - # Informational ONLY. Nothing reads this and it must never fail the - # build — no `set -e`, and every derivation falls back to UNAVAILABLE. + # --- derived values, one line (milestone 313) ------------------------ + # These stopped being shadow output at step 3: `tag` is published on + # main and `revision` decides whether the build below runs at all. This + # step prints all three anyway, because the load-bearing steps each + # print only the one they use, and on dev the date tag is computed + # nowhere else. When a build is skipped or a pin looks wrong, this is + # the line that says what the commit derived. # - # What to watch across pushes, because this is what step 3 will trust: + # Still diagnostic, so it still must not fail the build — no `set -e`, + # and every derivation falls back to UNAVAILABLE. A broken echo must + # never be the reason an image does not ship. + # + # What it should say: # * a push touching only agent/ moves the agent and leaves web and ml # STILL. If web moves, its path set is too wide. # * a push touching only docs moves nothing. @@ -664,14 +866,14 @@ jobs: # web bakes in the XPI. If web does not move, its set is too narrow # — the direction that serves stale bytes on a pin. # * dev and main derive the same values for the same source. - - name: Shadow — derived artifact version (informational) + - name: Report the derived artifact version run: | set -u A=agent T=$(sh scripts/artifacts.sh tag "$A" 2>&1 || echo UNAVAILABLE) V=$(sh scripts/artifacts.sh version "$A" 2>&1 || echo UNAVAILABLE) R=$(sh scripts/artifacts.sh revision "$A" 2>&1 || echo UNAVAILABLE) - echo "shadow: artifact=$A tag=$T version=$V revision=$R sha=$GITHUB_SHA" + echo "derived: artifact=$A tag=$T version=$V revision=$R sha=$GITHUB_SHA" - name: Determine tag id: tag @@ -685,10 +887,12 @@ jobs: # # Derived per artifact, so an image whose files did not change keeps # the tag it already had: the agent reads 2026.7.17 today while web - # reads 2026.8.27. Step 4 uses that to stop rebuilding it at all. + # reads 2026.8.27 — and the reuse step below turns that into a + # skipped build rather than a rebuild of bytes that already exist. if [ "${GITHUB_REF#refs/tags/}" != "${GITHUB_REF}" ]; then TAG_NAME="${GITHUB_REF#refs/tags/}" echo "tags=git.fabledsword.com/bvandeusen/fabledcurator-agent:${TAG_NAME}" >> "$GITHUB_OUTPUT" + echo "channel=main" >> "$GITHUB_OUTPUT" elif [ "${GITHUB_REF##*/}" = "main" ]; then CALVER=$(sh scripts/artifacts.sh tag agent) # Guarded, and computed only on this path. There is no `set -e` in @@ -701,8 +905,10 @@ jobs: exit 1 fi echo "tags=git.fabledsword.com/bvandeusen/fabledcurator-agent:main,git.fabledsword.com/bvandeusen/fabledcurator-agent:latest,git.fabledsword.com/bvandeusen/fabledcurator-agent:c-${SHORT_SHA},git.fabledsword.com/bvandeusen/fabledcurator-agent:${CALVER}" >> "$GITHUB_OUTPUT" + echo "channel=main" >> "$GITHUB_OUTPUT" else echo "tags=git.fabledsword.com/bvandeusen/fabledcurator-agent:dev" >> "$GITHUB_OUTPUT" + echo "channel=dev" >> "$GITHUB_OUTPUT" fi # Shell step rather than docker/login-action — see build-web's note on @@ -713,10 +919,95 @@ jobs: ACTOR: ${{ github.actor }} run: echo "$TOKEN" | docker login git.fabledsword.com -u "$ACTOR" --password-stdin + # --- reuse-if-published (milestone 313, step 4) ---------------------- + # The identity tag names this artifact's CONTENT — r-, the + # commit its shipped files last changed in, plus the channel for images + # that bake one in. If the registry already carries it, the bytes this + # job would produce are already published and the build is pure waste: + # the channel and date tags get repointed at the existing manifest + # instead, registry-side, in seconds. + # + # This is what stops a push that touched only `agent/` from rebuilding + # web and ml, and a merge to main from rebuilding what dev already built. + # + # The failure direction is deliberate. An inspect that errors for ANY + # reason — network, auth, a registry hiccup — reads as a miss and the + # build runs. Only a genuine 200 skips one, so there is no path here + # that skips a build that was actually needed; the worst case is paying + # for a build we could have avoided. + # + # BASE-IMAGE FRESHNESS, decided rather than left implicit: an artifact + # whose source stops moving stops picking up base-image updates under + # its pinned tag. That is what a pin MEANS — a date tag has to keep + # serving the bytes it served (fabledcurator-agent:2026.7.17 still + # resolves to July's image), or it is not a pin — and family rule + # 145 already says where the refresh goes instead: a rebuild with + # different contents publishes only the MOVING tag, never the immutable + # one. A scheduled channel-only refresh is tracked separately (#3154); + # it does not belong in the push path. + - name: Is this content already published? + id: reuse + env: + IMAGE: git.fabledsword.com/bvandeusen/fabledcurator-agent + CHANNEL: ${{ steps.tag.outputs.channel }} + TAGS: ${{ steps.tag.outputs.tags }} + IS_TAG_PUSH: ${{ startsWith(github.ref, 'refs/tags/') }} + run: | + set -eu + ID=$(sh scripts/artifacts.sh identity agent "$CHANNEL") + echo "identity=$ID" >> "$GITHUB_OUTPUT" + + # A tag-push builds a revision that main already published, so it + # must NOT claim the identity: image configs are not bit-reproducible + # (embedded timestamps), so re-pushing r- would point an + # immutable tag at fresh bytes — rule 145's exact prohibition. It + # publishes only its own :v… label and otherwise reuses. + if [ "$IS_TAG_PUSH" = "true" ]; then + echo "build_tags=$TAGS" >> "$GITHUB_OUTPUT" + else + echo "build_tags=$TAGS,$IMAGE:$ID" >> "$GITHUB_OUTPUT" + fi + + if docker buildx imagetools inspect "$IMAGE:$ID" >/dev/null 2>&1; then + echo "hit=true" >> "$GITHUB_OUTPUT" + echo "reuse: $IMAGE:$ID is already published — skipping the build" + else + echo "hit=false" >> "$GITHUB_OUTPUT" + echo "reuse: $IMAGE:$ID is not published — building" + fi + - name: Build and push agent image + if: steps.reuse.outputs.hit != 'true' uses: docker/build-push-action@v5 with: context: agent file: agent/Dockerfile push: true - tags: ${{ steps.tag.outputs.tags }} + tags: ${{ steps.reuse.outputs.build_tags }} + + # Registry-side manifest copy: no layer transfer, no local daemon, no + # rebuild. Each -t becomes another reference to the SAME manifest the + # identity tag holds, so :latest and the date pin are byte-identical to + # what was 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' + env: + IMAGE: git.fabledsword.com/bvandeusen/fabledcurator-agent + IDENTITY: ${{ steps.reuse.outputs.identity }} + TAGS: ${{ steps.tag.outputs.tags }} + run: | + set -euf + # steps.tag emits ONE comma-separated list, because that is the shape + # docker/build-push-action takes; imagetools wants a -t per ref. + ARGS="" + IFS=, + for t in $TAGS; do ARGS="$ARGS -t $t"; done + unset IFS + # shellcheck disable=SC2086 + docker buildx imagetools create $ARGS "$IMAGE:$IDENTITY" + echo "repointed to $IMAGE:$IDENTITY: $TAGS" diff --git a/.forgejo/workflows/ci.yml b/.forgejo/workflows/ci.yml index 17a6c9f..e7f71c8 100644 --- a/.forgejo/workflows/ci.yml +++ b/.forgejo/workflows/ci.yml @@ -115,6 +115,13 @@ jobs: SECRET_KEY: ci_unit_test_placeholder steps: - uses: actions/checkout@v4 + with: + # Full history for tests/test_artifact_identity.py, which derives + # each artifact's revision to check the identity scheme. On a + # depth-1 clone that derivation either fails or returns the tip sha + # — so the lane would go green while asserting nothing, which is + # the one outcome worse than a red one. + fetch-depth: 0 # Cache step removed 2026-05-26: act_runner's cache backend has been # broken on this homelab runner since 2026-05-15 (first as request- diff --git a/scripts/artifacts.sh b/scripts/artifacts.sh index 948706a..a1ea0d5 100755 --- a/scripts/artifacts.sh +++ b/scripts/artifacts.sh @@ -56,8 +56,25 @@ ML_PATHS='Dockerfile.ml requirements-ml.txt requirements.txt backend alembic ale # it: this is deliberately NOT `agent/`. AGENT_PATHS='agent/Dockerfile agent/requirements.txt agent/fc_agent' +# Which artifacts bake the BUILD CHANNEL into the image, and therefore cannot +# share a content identity across channels. The web image takes FC_CHANNEL as +# a build-arg and reports it from /api/extension/manifest (milestone 271 step +# 7), so `main` and `dev` builds of one revision are genuinely different +# images — reusing the dev one on main would ship an instance that names +# itself `dev` forever. +# +# ml and agent take no build-args at all: one revision, one image, and a merge +# to main can reuse exactly what dev already built. That is not a detail, it is +# most of what step 4 saves — merges would otherwise rebuild the agent's CUDA +# image to produce bytes that already exist. +# +# Extend this list if a second artifact ever gains a build-arg; +# tests/test_artifact_identity.py reads the Dockerfiles and fails if it drifts. +CHANNELLED='web' + usage() { echo "usage: artifacts.sh {paths|revision|version|tag} {web|ml|agent|extension}" >&2 + echo " artifacts.sh identity {web|ml|agent} [channel]" >&2 exit 2 } @@ -140,11 +157,50 @@ cmd_tag() { "$(strip0 "$(fmt "$sha" %d)")" } +# The CONTENT IDENTITY of a published image: an immutable tag naming exactly +# what a build of this commit would produce. build.yml asks the registry for it +# and, on a hit, skips the build entirely and repoints the channel and date +# tags at the manifest that is already there (milestone 313 step 4). +# +# It is deliberately NOT either of the other two values: +# * the date tag is day-precise and last-one-wins, so two different builds +# share it — it cannot answer "is this content published?". +# * the commit sha moves on every push, so it would never hit, which is the +# redundant rebuild this exists to remove. +# +# The revision does both jobs: it is content-unique AND stable across pushes +# that did not touch the artifact. +cmd_identity() { + _art=$1 + _chan=${2:-} + case "$_art" in + web|ml|agent) ;; + extension) + echo "artifacts.sh: the extension is cached as an ext- Forgejo release, not an image tag — use \`version\`" >&2 + exit 2 ;; + *) usage ;; + esac + for _c in $CHANNELLED; do + if [ "$_art" = "$_c" ]; then + # Refused rather than defaulted: an unqualified identity for a + # channelled artifact would let a dev image be reused as the main one. + if [ -z "$_chan" ]; then + echo "artifacts.sh: $_art bakes the channel into the image — identity needs one" >&2 + exit 2 + fi + printf 'r-%s-%s\n' "$(cmd_revision "$_art")" "$_chan" + return + fi + done + printf 'r-%s\n' "$(cmd_revision "$_art")" +} + [ $# -ge 2 ] || usage case "$1" in paths) cmd_paths "$2" ;; revision) cmd_revision "$2" ;; version) cmd_version "$2" ;; tag) cmd_tag "$2" ;; + identity) cmd_identity "$2" "${3:-}" ;; *) usage ;; esac diff --git a/tests/test_artifact_identity.py b/tests/test_artifact_identity.py new file mode 100644 index 0000000..b65ff17 --- /dev/null +++ b/tests/test_artifact_identity.py @@ -0,0 +1,140 @@ +"""`artifacts.sh identity` is what decides whether a build gets skipped. + +Milestone 313 step 4: build.yml asks the registry for `:` and, +on a hit, publishes NO new bytes — it repoints the channel and date tags at the +manifest already there. So the identity has to be a true name for the content. +Both ways of getting it wrong are silent at build time and only surface in +production: + +* **too coarse** — two genuinely different images share an identity, so the + second one never gets built and its tags point at the first one's bytes. The + live case is FC_CHANNEL: a `dev` and a `main` build of one revision differ, + and collapsing them ships an instance that reports the wrong channel forever. +* **too fine** — the identity moves when the content did not, nothing ever + hits, and step 4 buys nothing. A commit sha would do exactly this. + +The Dockerfiles are read here rather than trusted, because the coarse direction +appears the moment someone adds a build-arg without touching `CHANNELLED`. +""" +from __future__ import annotations + +import re +import subprocess +from pathlib import Path + +import pytest + +ROOT = Path(__file__).resolve().parent.parent + +# Only image artifacts have an identity — the extension is cached as an +# ext- Forgejo release, not a registry tag. +IMAGE_ARTIFACTS = { + "web": "Dockerfile", + "ml": "Dockerfile.ml", + "agent": "agent/Dockerfile", +} + +CHANNELS = ("main", "dev") + +# docker's own tag grammar: [A-Za-z0-9_][A-Za-z0-9._-]{0,127} +_TAG = re.compile(r"^[A-Za-z0-9_][A-Za-z0-9._-]{0,127}$") + +# `ARG FC_CHANNEL` in a Dockerfile means build.yml passes a per-channel value +# in, so the channel is part of what the image IS. +_ARG_CHANNEL = re.compile(r"^\s*ARG\s+FC_CHANNEL\b", re.MULTILINE) + + +def identity(artifact: str, channel: str | None = None) -> subprocess.CompletedProcess: + cmd = ["sh", str(ROOT / "scripts" / "artifacts.sh"), "identity", artifact] + if channel is not None: + cmd.append(channel) + return subprocess.run(cmd, capture_output=True, text=True, cwd=ROOT) + + +def ok(artifact: str, channel: str | None = None) -> str: + proc = identity(artifact, channel) + assert proc.returncode == 0, f"identity {artifact} {channel}: {proc.stderr}" + return proc.stdout.strip() + + +def bakes_the_channel(artifact: str) -> bool: + return bool(_ARG_CHANNEL.search((ROOT / IMAGE_ARTIFACTS[artifact]).read_text())) + + +@pytest.mark.parametrize("artifact", sorted(IMAGE_ARTIFACTS)) +def test_channel_dependence_matches_the_dockerfile(artifact): + """The coarse direction, caught at its source. + + Whether the channel belongs in the identity is not a preference — it is + dictated by whether the Dockerfile takes it as a build-arg. Adding an + `ARG FC_CHANNEL` to another image without adding it to `CHANNELLED` would + make its dev and main builds collide, and nothing else would notice. + """ + per_channel = {c: ok(artifact, c) for c in CHANNELS} + differs = len(set(per_channel.values())) > 1 + + if bakes_the_channel(artifact): + assert differs, ( + f"{IMAGE_ARTIFACTS[artifact]} declares ARG FC_CHANNEL, so a dev " + f"build and a main build of one revision are different images — " + f"but both derive the identity {per_channel['main']!r}. The main " + f"build would reuse the dev image and report the wrong channel. " + f"Add {artifact!r} to CHANNELLED in scripts/artifacts.sh." + ) + else: + assert not differs, ( + f"{IMAGE_ARTIFACTS[artifact]} takes no channel build-arg, so one " + f"revision is one image and a merge to main should reuse what dev " + f"already built — but the identity differs per channel " + f"({per_channel}), so every merge rebuilds it for nothing. Remove " + f"{artifact!r} from CHANNELLED in scripts/artifacts.sh." + ) + + +@pytest.mark.parametrize("artifact", sorted(IMAGE_ARTIFACTS)) +def test_identity_tracks_the_artifacts_own_revision(artifact): + """The fine direction: the identity must be the revision, not the push. + + `revision` is the commit this artifact's shipped files last changed in, so + it holds still across pushes that did not touch it. Anything derived from + HEAD instead would move every push and never hit the registry. + """ + rev = subprocess.run( + ["sh", str(ROOT / "scripts" / "artifacts.sh"), "revision", artifact], + capture_output=True, text=True, check=True, cwd=ROOT, + ).stdout.strip() + value = ok(artifact, "main") + assert rev and rev in value, ( + f"identity {value!r} does not contain the {artifact} revision {rev!r}" + ) + + +@pytest.mark.parametrize("artifact", sorted(IMAGE_ARTIFACTS)) +def test_identity_is_a_legal_docker_tag(artifact): + """It is pushed as a tag, so an illegal one fails at the registry — after + the build has already run.""" + for channel in CHANNELS: + value = ok(artifact, channel) + assert _TAG.match(value), f"{value!r} is not a valid docker tag" + + +def test_a_channelled_artifact_refuses_an_unqualified_identity(): + """Refusing beats defaulting. If `identity web` quietly returned the + unqualified `r-`, a workflow that forgot to pass the channel would + publish one image under a name both channels then reuse — the exact + collision the CHANNELLED list exists to prevent, reintroduced by an + omission rather than by an edit.""" + proc = identity("web") + assert proc.returncode != 0, ( + "identity web returned a value with no channel: " + f"{proc.stdout.strip()!r}" + ) + + +def test_the_extension_has_no_image_identity(): + """It is cached as an ext- release asset, and its cache key is the + version. Answering with a plausible image tag would invite a second, + divergent cache.""" + proc = identity("extension", "main") + assert proc.returncode != 0 + assert "ext-" in proc.stderr From fb2c4d5b80be1daa98d6f7d4f7da5b99b9faf4ba Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Fri, 28 Aug 2026 08:49:06 -0400 Subject: [PATCH 5/5] fix(extension): packaging.sh decides the version, so it must move the version MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `packaging.sh pathspec` excluded `extension/scripts/**` — the same list web-ext ignores. But the two lists answer different questions, and this is the one place they disagree. packaging.sh is not packaged into the XPI. It does decide the version string, and build.yml stamps that string into the manifest.json that IS packaged. Changing how the version is computed therefore changes the shipped bytes, and the derivation has to see it. Harmless while every push rebuilt the web image. Step 4 made the rebuild conditional on the derived revision moving, which turns it into a silent failure: a packaging.sh change yields a NEW version, so sign-extension misses its ext- cache and signs — while build-web sees an unmoved revision, reuses the published image and ships the OLD XPI. One orphaned AMO signature, and an instance serving code the registry calls current. Found while checking the ground under step 5, which changes the version format and is exactly the commit that would have hit it. Split the list rather than widening the shared one: NOT_VERSION_RELEVANT drives the pathspec, NOT_PACKAGED_TRACKED still drives web-ext's ignore list, and scripts/ stays out of the XPI. The two directions are not symmetric, which is why the version list is the narrower one — too wide costs a re-sign and a rebuild for a change that ships nothing new, too narrow serves stale bytes and says nothing. No version churn: the last packaging.sh commit predates the current extension revision, so the derived version is unchanged at 1.0.3500147 and web's revision stays a7e626a67a79. Both suites now pin the disagreement from their own side, because the tempting fix for either half is to make the lists one again — version.spec.js asserts the pathspec does NOT exclude scripts while the ignore list still does, and test_artifact_paths.py asserts packaging.sh is inside the extension and web path sets. --- extension/scripts/packaging.sh | 35 ++++++++++++++++++++++++++++++---- extension/test/version.spec.js | 32 ++++++++++++++++++++++++++++++- tests/test_artifact_paths.py | 30 +++++++++++++++++++++++++++++ 3 files changed, 92 insertions(+), 5 deletions(-) diff --git a/extension/scripts/packaging.sh b/extension/scripts/packaging.sh index a8aa969..399aa43 100755 --- a/extension/scripts/packaging.sh +++ b/extension/scripts/packaging.sh @@ -6,7 +6,9 @@ # keeping three copies of one fact in sync by hand is how issue #2397 happened: # # 1. web-ext's --ignore-files (extension/package.json's four scripts) -# 2. the :(exclude) pathspec (ci.yml's extension-version guard) +# 2. the :(exclude) pathspec (what moves the version — a WIDER +# set than the ignore list; see +# NOT_VERSION_RELEVANT) # 3. the git-log pathspec (the derived version, below) # # They now all read from here. POSIX sh only — CI's run shell is busybox. @@ -35,6 +37,28 @@ set -euf NOT_PACKAGED_TRACKED='package.json package-lock.json README.md .gitignore vitest.config.js scripts scripts/** test test/**' NOT_PACKAGED_BUILD='web-ext-artifacts node_modules' +# Paths under extension/ that cannot change the SHIPPED BYTES, and so must not +# move the derived version. +# +# Deliberately NOT the same list as NOT_PACKAGED_TRACKED, and the whole +# difference is `scripts/`. packaging.sh is not packaged into the XPI — but it +# DECIDES the version string, and build.yml stamps that string into the +# manifest.json that is packaged. A change to how the version is computed is +# therefore a change to the shipped bytes. +# +# Excluding it was harmless only while every push rebuilt the web image. +# Milestone 313 step 4 made the rebuild conditional on the derived revision +# moving, which turned it into a silent failure: a packaging.sh change gives a +# NEW version, so sign-extension misses its ext- cache and signs — +# while build-web sees an unmoved revision, reuses the published image, and +# ships the OLD XPI. An orphaned AMO signature, and an instance quietly serving +# code the registry says is current. +# +# The two directions are not symmetric, which is why this list is the narrower +# one. Too wide costs a re-sign and a rebuild for a change that ships nothing +# new. Too narrow serves stale bytes and says nothing. +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 exit 2 @@ -49,11 +73,14 @@ cmd_ignore() { echo "$NOT_PACKAGED_TRACKED $NOT_PACKAGED_BUILD" } -# git pathspec excluding the non-packaged tracked files, e.g. -# :(exclude)extension/package.json :(exclude)extension/test/** +# git pathspec excluding the tracked files that cannot change the shipped +# bytes, e.g. :(exclude)extension/package.json :(exclude)extension/test/** +# +# This answers "what moves the version?", NOT "what goes in the XPI?" — see +# NOT_VERSION_RELEVANT for why those differ. cmd_ignore answers the other one. # Same `set -f` requirement as above. cmd_pathspec() { - for entry in $NOT_PACKAGED_TRACKED; do + for entry in $NOT_VERSION_RELEVANT; do printf ':(exclude)extension/%s ' "$entry" done echo diff --git a/extension/test/version.spec.js b/extension/test/version.spec.js index 4e69a69..3b75088 100644 --- a/extension/test/version.spec.js +++ b/extension/test/version.spec.js @@ -44,9 +44,39 @@ describe('packaging.sh — the single definition of what ships', () => { // covering anything added later. const pathspec = packaging('pathspec') expect(pathspec).toContain(':(exclude)extension/test/**') - expect(pathspec).toContain(':(exclude)extension/scripts/**') expect(pathspec.some((e) => e.includes('.spec.js'))).toBe(false) expect(pathspec.some((e) => e.includes('helpers'))).toBe(false) + + const ignore = packaging('ignore') + expect(ignore).toContain('test/**') + expect(ignore).toContain('scripts/**') + expect(ignore.some((e) => e.includes('.spec.js'))).toBe(false) + }) + + it('lets packaging.sh move the version, though it never ships in the XPI', () => { + // The two lists answer different questions and this is the one place they + // disagree. scripts/ is ignored by web-ext — it is repo tooling, not addon + // code — but packaging.sh DECIDES the version string, and build.yml stamps + // that string into the manifest.json that does ship. So changing how the + // version is computed changes the shipped bytes. + // + // Excluding it from the pathspec was invisible while every push rebuilt the + // web image. Milestone 313 step 4 made that rebuild conditional on the + // derived revision moving, and the omission turned into a silent failure: + // a new version means sign-extension misses its ext- cache and + // signs, while build-web sees an unmoved revision, reuses the published + // image and ships the OLD XPI. An orphaned signature, and an instance + // serving code the registry calls current. + const pathspec = packaging('pathspec') + expect( + pathspec.some((e) => e.startsWith(':(exclude)extension/scripts')), + 'the pathspec excludes scripts/, so a change to how the version is ' + + 'derived would not move the version it derives', + ).toBe(false) + + // ...and it is still kept out of the package itself. Both must hold: the + // tempting "fix" for either half is to make the two lists one again. + expect(packaging('ignore')).toContain('scripts') }) it('keeps its own scripts and specs out of the XPI', () => { diff --git a/tests/test_artifact_paths.py b/tests/test_artifact_paths.py index c226f5b..8063517 100644 --- a/tests/test_artifact_paths.py +++ b/tests/test_artifact_paths.py @@ -110,6 +110,36 @@ 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. + + Left out, milestone 313 step 4 turns it silent: the new version misses the + ext- 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"): + 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" + ) + assert not any( + covered_by(path, e.rstrip("*").rstrip("/")) for e in excluded + ), ( + f"{path} is excluded from the {artifact} path set, so a change to " + f"how the version is derived would not move the version — and " + f"step 4 would reuse the image that carries the old one" + ) + + @pytest.mark.parametrize( "artifact, path", [