refactor: retire the fabledcurator-ml image name entirely (4311)
CI / lint (push) Successful in 2s
CI / extension-version (push) Successful in 2s
Build images / sign-extension (push) Successful in 4s
Build images / build-agent (push) Successful in 7s
CI / frontend-build (push) Successful in 18s
extension / lint (push) Successful in 19s
CI / backend-lint-and-test (push) Failing after 32s
Build images / build-web (push) Successful in 1m37s
CI / integration (push) Successful in 2m11s
Build images / smoke-web (push) Successful in 42s
Build images / promote (push) Skipped

Operator, 2026-09-23: *"we don't need to keep this as I'll be collapsing my
stack to the same consolidated version that we're building here."*

That was the only thing holding the name up. `fabledcurator-ml` has been the
same bytes as `fabledcurator` since milestone 422 step 6, and 8152684 had
already stopped rebuilding it — this removes the name.

Gone: the whole `build-ml` job (137 more lines), its entry in promote's
`needs` and in promote's tag loop, `ML_PATHS` and the `ml)` case in
artifacts.sh, the `ml` artifact in test_artifact_paths, the image in
release_notes.py and its test, and the README row. `docker-compose.yml`'s
ml-worker service now runs `fabledcurator:latest` with `command:
["ml-worker"]` — the service still exists for anyone who wants lane
separation, it just no longer needs a second image name.

Prose that said "all three images" is now "both", except where it means
three VALUES (revision/version/epoch) or records what happened on 2026-08-30,
when there genuinely were three.

Published tags are left alone. Nothing new goes to that name; what is already
in the registry stays pullable, so a stack that has not moved yet keeps
running rather than losing its image mid-flight.

## A latent trap this made reachable, found by running the script

`artifacts.sh revision ml` did not fail. It printed the usage line to stderr
and answered with **the newest commit in the whole repository** — a
real-looking 12-char sha on stdout, exit 0.

`newest()` inlined the path set as `git log ... -- $(cmd_paths "$1")`, and
`usage` exits from the command SUBSHELL, so the substitution came back empty
and `git log HEAD --` had no pathspec left to filter by. The reuse check
would have compared that answer against a published label, missed, and
rebuilt on every push forever with nothing going red.

It could not be fixed inside `newest`, which was my first attempt: every
caller wraps it in a substitution too, so its `exit` also died in a subshell —
stdout went empty but the status stayed 0. The guard has to run in the main
shell, so it validates the artifact name at dispatch. `newest` keeps a
defensive `|| exit 2` for a future caller that reaches it another way, and
the comment says which of the two is the real check.

Latent while every name callers passed was valid. Removing `ml` from the set
is what made a name that used to work start taking the silent path, so the
test that pins it lands here.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LVjrnpQjRgHdvq95rASoiR
This commit is contained in:
2026-09-23 08:30:47 -04:00
co-authored by Claude Opus 5
parent f0595e43e6
commit ac70f2aadc
7 changed files with 100 additions and 192 deletions
+40 -23
View File
@@ -8,7 +8,7 @@
# never reaches the image — see DERIVER below, and #3156 for the same finding
# about packaging.sh.
#
# Four artifacts, four independent versions. An artifact whose shipped files
# Three artifacts, three 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:
@@ -51,21 +51,6 @@ ROOT=$(git rev-parse --show-toplevel)
# rather than restated — one definition, per #2397.
WEB_PATHS='Dockerfile requirements.txt requirements-ml.txt backend alembic alembic.ini entrypoint.sh frontend :(exclude)frontend/test :(exclude)frontend/test/**'
# ml — THE SAME IMAGE as web since milestone 422 step 6, built from the same
# Dockerfile with the same context and published under a second name only
# because the operator's Swarm stack still references it.
#
# So it is the same path set, by assignment rather than by a copy that would
# drift. This was two lists describing one image for about an hour, and the
# artifact-paths guard caught it immediately: the merged Dockerfile carries
# the frontend-builder stage, so the ml image copies `frontend/package.json`,
# and the old ml list did not cover it — its version would not have moved when
# the frontend changed, and a pinned build would serve stale bytes (#3202).
#
# `requirements-ml.txt` is in WEB_PATHS for the mirror-image reason: the web
# image now installs the ML requirements, so changing them changes it.
ML_PATHS="$WEB_PATHS"
# 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
@@ -78,7 +63,7 @@ AGENT_PATHS='agent/Dockerfile agent/requirements.txt agent/fc_agent'
#
# Why web and nothing else. Every artifact stamps `fc.revision`, but only web
# also stamps a version (build.yml line ~488 feeds `version web` to the
# FC_VERSION build arg; ml and agent ask for `revision` alone, and the
# FC_VERSION build arg; the agent asks for `revision` alone, and the
# extension takes its version from packaging.sh). For a revision-only artifact
# this file needs no entry: any change to how the revision is COMPUTED changes
# the derived value, which then disagrees with the label on the published image
@@ -101,7 +86,7 @@ DERIVER='scripts/artifacts.sh'
usage() {
echo "usage: artifacts.sh {paths|revision|version|epoch} {web|ml|agent|extension}" >&2
echo "usage: artifacts.sh {paths|revision|version|epoch} {web|agent|extension}" >&2
exit 2
}
@@ -115,9 +100,6 @@ ext_paths() {
cmd_paths() {
case "$1" in
web) echo "$WEB_PATHS $DERIVER $(ext_paths)" ;;
# Identical to web, deliberately: it IS web's image. That includes the
# deriver and the bundled extension — the XPI is in these bytes too.
ml) echo "$ML_PATHS $DERIVER $(ext_paths)" ;;
agent) echo "$AGENT_PATHS" ;;
extension) ext_paths ;;
*) usage ;;
@@ -128,8 +110,21 @@ cmd_paths() {
# 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") \
# Resolve the path set into a variable rather than inlining it as
# `git log ... -- $(cmd_paths "$1")`. An empty substitution there leaves
# `git log HEAD --` with NO pathspec, which answers with the newest commit
# in the whole repository instead of failing — see the dispatch guard at the
# foot of this file for what that cost and why the real check lives there.
#
# This `|| exit 2` cannot be that check (it exits a subshell, since every
# caller wraps `newest` in one) — it is belt and braces for a future caller
# that reaches `newest` without passing the guard.
paths=$(cmd_paths "$1") || exit 2
# Unquoted on purpose: the pathspec must word-split into separate args.
# Globbing is off script-wide (`set -f`).
# shellcheck disable=SC2086
set -- "$(cd "$ROOT" && git log --format='%ct %H' HEAD -- $paths \
| sort -n | tail -1)"
if [ -z "$1" ]; then
echo "artifacts.sh: no commit touches this artifact's shipped files" >&2
@@ -226,6 +221,28 @@ cmd_version() {
}
[ $# -ge 2 ] || usage
# Validate the ARTIFACT here, in the main shell, before anything dispatches.
#
# It cannot be done deeper down. `cmd_revision`, `cmd_version` and `cmd_epoch`
# all call `newest` inside a command substitution, and `newest` resolves the
# path set inside another one — so an `exit` from either kills only that
# subshell. The script carried on, printed nothing, and exited 0.
#
# Before this, `revision ml` was worse than nothing: the empty substitution
# left `git log HEAD --` with no pathspec, so it answered with the newest
# commit in the WHOLE REPOSITORY — a real-looking 12-char sha on stdout, exit
# 0, and the usage line on stderr where no caller reads it. The reuse check
# would have compared that against a published label, missed, and rebuilt
# every push forever without a red run anywhere.
#
# Latent while every name callers passed was valid. #4311 removed `ml` from
# the set, which made a name that used to work start taking that path.
case "$2" in
web|agent|extension) ;;
*) usage ;;
esac
case "$1" in
paths) cmd_paths "$2" ;;
revision) cmd_revision "$2" ;;
-1
View File
@@ -83,7 +83,6 @@ API = "https://git.fabledsword.com/api/v1/repos/bvandeusen/FabledCurator"
IMAGES = (
"git.fabledsword.com/bvandeusen/fabledcurator",
"git.fabledsword.com/bvandeusen/fabledcurator-ml",
"git.fabledsword.com/bvandeusen/fabledcurator-agent",
)