CI / lint (push) Successful in 2s
CI / extension-version (push) Successful in 2s
Build images / sign-extension (push) Successful in 3s
Build images / build-agent (push) Successful in 6s
CI / frontend-build (push) Successful in 20s
extension / lint (push) Successful in 23s
CI / backend-lint-and-test (push) Failing after 31s
CI / integration (push) Successful in 2m16s
Build images / build-ml (push) Successful in 3m8s
Build images / build-web (push) Successful in 3m16s
Build images / smoke-web (push) Skipped
Build images / promote (push) Skipped
Milestone 422 step 6. Dockerfile.ml is gone; the main image carries torch,
torchvision, transformers, onnxruntime and opencv, and serves every lane.
WHY IT HAD TO MERGE: step 5 runs every lane in one process tree, so a second
image would mean the `ml` lane could never be enabled from the UI — there
would be no worker in that container to enable. The switch needs something to
switch.
THE MODEL NO LONGER DOWNLOADS AT BOOT. `entrypoint.sh`'s ml-worker role ran
download_models before celery started, so every boot of that role reached
HuggingFace for ~3.5GB — a startup dependency on a third party for a feature
the operator may never use. Rule 164 permits a runtime fetch only for
something "optional and clearly off", so the fetch is now a TASK, enqueued
the moment the lane is ENABLED.
Being a task is what makes it visible: it gets a TaskRun row, so the download
shows in Activity with a duration and a status, and a failure is something an
operator can see and retry rather than a container that quietly never became
useful. Idempotent, so re-enabling a provisioned lane costs one no-op.
Enqueued only when the lane actually came ON (`enabled is True`, not the
resolved value) so re-saving slots does not re-fetch, and only when the
consumer change landed — a task queued onto a queue nothing consumes would
sit pending with no explanation.
`fabledcurator-ml` KEEPS PUBLISHING, from the merged Dockerfile. The
operator's Swarm stack references that name and lives outside this repo;
dropping it would not break their deploy, it would freeze it silently at the
last publish — the exact failure class this milestone keeps finding. Retiring
the NAME is its own task, gated on that stack moving. Same two-phase shape
#406 used for pixiv.
THREE LIVE BREAKAGES from deleting the file, found by grepping for it rather
than assuming the build was the only consumer:
- `docker-compose.override.yml` built the ml service from it (contributor
path would have failed at `docker compose build`).
- `tests/test_artifact_paths.py` pins the ml path set.
- `scripts/artifacts.sh` ML_PATHS named it. A path set naming a deleted file
silently stops contributing to the derived revision — which the reuse check
and the version string both read. That is #3202's recorded shape.
The `--with-ml` flag is gone from the generator and the healthcheck rather
than left defaulting to true. One image carries every lane now, so a flag
that can only be passed one way is a branch pretending to be a choice.
The advisory shipped in ecbd325 is what makes this honest to an adopter: the
lane says it is optional, names the model, and gives its download and
per-slot RAM before the switch is thrown.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LVjrnpQjRgHdvq95rASoiR
200 lines
9.1 KiB
Python
200 lines
9.1 KiB
Python
"""`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.
|
|
|
|
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
|
|
|
|
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", ""),
|
|
"agent": ("agent/Dockerfile", "agent"),
|
|
}
|
|
|
|
# COPY --from=<stage> 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<args>.+)$", 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"
|
|
)
|
|
|
|
|
|
# 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",)),
|
|
]
|
|
|
|
|
|
@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)")
|
|
]
|
|
assert any(covered_by(path, i) for i in inc), (
|
|
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
|
|
), (
|
|
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",
|
|
[
|
|
# 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"),
|
|
# 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."
|
|
)
|