From e1cd6291966f31dc9ae8aae032f486c264393fc2 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Thu, 24 Sep 2026 07:07:31 -0400 Subject: [PATCH] feat: the agent derives its version instead of being asked to remember it (4386) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Operator: the agent's build string could not identify the agent. VERSION was a literal in app.py an author was meant to bump, and nobody did — the September image printed the same "2026-07-17.1" as the July one, so the one surface meant to answer "did my pull work?" answered the same either way. Nothing new was needed. scripts/artifacts.sh has derived a version per artifact since milestone 313, and build-agent has been computing the agent's on every run and printing it to the log. The image just never carried it. Three values, never folded together (rule 149): FC_VERSION YYYY.MM.DD.HHMM from the COMMIT its shipped files last changed in — identical on dev and main for the same source, which is what makes "am I running production's code?" answerable. FC_CHANNEL a sibling field, never a suffix inside the name. FC_REVISION the 12-char sha; the same string as the fc.revision LABEL, so the image and the registry cannot disagree about which commit this is. The page SHOWS the version and COMPARES the revision. Those were one value before, which is how a version acquires a second job and then cannot be changed without breaking the reload banner. An unstamped local build reads `unknown` and compares `local` — absent rather than empty, one spelling of "cannot say". scripts/artifacts.sh joins the AGENT path set in the same commit, and it had to: a version has no backstop. A revision that is computed differently stops matching the published label and forces a rebuild, so it self-corrects; a version is compared against nothing, so a change to cmd_version alone would leave the agent publishing the old format with nothing to contradict it. That is #3202's finding, and the agent was rightly exempt only while it had no version of its own. tests/test_artifact_paths.py pins it. Also corrects two build.yml comments claiming agent/ had not changed since 2026-07-17. Both were already false — it changed 2026-09-23 — and one of them is the stated rationale for the force_build escape hatch. Rewritten without dates: how long an artifact has been quiet is a `git log` question, and its answer in a comment is wrong the next time anyone commits (lesson #4383). Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01LVjrnpQjRgHdvq95rASoiR --- .forgejo/workflows/build.yml | 57 ++++++++++++---- agent/Dockerfile | 17 +++++ agent/fc_agent/app.py | 35 +++++++--- agent/fc_agent/build_info.py | 79 ++++++++++++++++++++++ scripts/artifacts.sh | 31 ++++++--- tests/test_agent_build_info.py | 120 +++++++++++++++++++++++++++++++++ tests/test_artifact_paths.py | 23 ++++--- 7 files changed, 325 insertions(+), 37 deletions(-) create mode 100644 agent/fc_agent/build_info.py create mode 100644 tests/test_agent_build_info.py diff --git a/.forgejo/workflows/build.yml b/.forgejo/workflows/build.yml index 2a41064..db5e219 100644 --- a/.forgejo/workflows/build.yml +++ b/.forgejo/workflows/build.yml @@ -41,10 +41,17 @@ on: # produce a changelog, not an image. # The escape hatch for the one thing skip-if-exists makes untestable: a - # build that WOULD be skipped. `agent/` has not changed since 2026-07-17, so - # every push since has correctly declined to build it — which also means the - # agent build path has not run in six weeks and cannot be exercised on - # demand. #3190 lives on exactly that path. + # build that WOULD be skipped. An artifact whose source is quiet has its + # build path correctly declined on every push — which also means that path + # is not exercised, and cannot be exercised on demand, for as long as the + # quiet lasts. #3190 lives on exactly such a path. + # + # This used to name the agent and a date ("unchanged since 2026-07-17"). It + # was already false when read on 2026-09-24 — the agent had changed the day + # before — and a rationale resting on a stale fact reads as settled + # reasoning forever after (lesson #4383). Which artifact is quiet is a + # question for `git log`, not for a comment; the hatch exists because ANY of + # them can be. # # Editing build.yml does not force one either, and that is deliberate: the # workflow is not shipped bytes, so it is in no artifact's path set. Putting @@ -67,11 +74,18 @@ on: # The base-image refresh (milestone 326 step 4, #3154). # # Skip-if-exists is keyed on OUR source, so an artifact whose source stops - # moving stops picking up base-image updates. `agent/` last changed - # 2026-07-17; every push since has correctly declined to rebuild it, which - # also means it will serve that day's `nvidia/cuda` layers forever. Nothing - # is wrong until it has been unchanged for months, which is precisely why - # this is a calendar trigger and not a condition on the push path. + # moving stops picking up base-image updates — it will serve the base layers + # of its last build forever. The agent is the standing example because its + # base is the heaviest (`nvidia/cuda`, ~6.3 GB) and its source the quietest: + # 48 of 1111 commits touch it, about 4% (#3114). + # + # Deliberately no date here. Nothing is wrong until an artifact has been + # unchanged for months, and "how long has it been" is a `git log` question + # whose answer in a comment is wrong the next time anyone commits — which is + # exactly how the previous version of this paragraph came to claim the agent + # had not moved since 2026-07-17 while it had moved the day before. That is + # precisely why this is a CALENDAR trigger and not a condition on the push + # path: the calendar cannot be wrong about how much time has passed. # # Weekly, Sunday 06:00 UTC. Away from CI-runner's Monday security sweep so # the two are never diagnosing each other, and on the quietest day so a @@ -1352,9 +1366,11 @@ jobs: # :dev than to the :2026.8.28 tags milestone 318 deleted. (#3114.) cache-from: type=registry,ref=git.fabledsword.com/bvandeusen/fabledcurator:buildcache cache-to: type=registry,ref=git.fabledsword.com/bvandeusen/fabledcurator:buildcache,mode=max - # Only the web image carries these: it is the one with a UI and an - # HTTP surface to report them on. The ml and agent images have - # nothing to tell. + # The agent carries its own copy of these (plus FC_REVISION) since + # 2026-09-24. This used to say the agent "has nothing to tell" — true + # of the ml image, which no longer exists (#4311), and never true of + # the agent, which has a control page and a /status endpoint and was + # reporting a hand-written literal on both. build-args: | FC_CHANNEL=${{ steps.tag.outputs.channel }} FC_VERSION=${{ steps.reuse.outputs.version }} @@ -2200,6 +2216,13 @@ jobs: set -eu DERIVED=$(sh scripts/artifacts.sh revision agent) echo "revision=$DERIVED" >> "$GITHUB_OUTPUT" + # Baked into the agent image as FC_VERSION and reported by its + # control page and /status. A pure function of the revision — same + # commit, same string — so it adds no variability the reuse check + # would have to account for. Added 2026-09-24: the agent's version + # used to be a hand-written literal in app.py that nobody bumped, so + # the September image reported the same string as the July one. + echo "version=$(sh scripts/artifacts.sh version agent)" >> "$GITHUB_OUTPUT" # The build clock, pinned to the same commit (#3265). Without it # buildkit stamps the image config with the wall clock of the build, @@ -2353,6 +2376,16 @@ jobs: # decoration — an unstamped image is one that will always rebuild. labels: | fc.revision=${{ steps.reuse.outputs.revision }} + # What the agent reports about ITSELF, as opposed to the label above, + # which is what the registry reports about it. Three values kept + # apart (rule 149). FC_REVISION is the same string as the label, so + # the image and the registry cannot disagree about which commit this + # is — a build whose self-report names a different commit from the + # tag it was published under is unrollbackable in practice. + build-args: | + FC_CHANNEL=${{ steps.tag.outputs.channel }} + FC_VERSION=${{ steps.reuse.outputs.version }} + FC_REVISION=${{ steps.reuse.outputs.revision }} # LOAD-BEARING, not a preference. On the default docker driver these # were no-ops; on the docker-container driver above, # build-push-action@v5 defaults provenance to TRUE when pushing. diff --git a/agent/Dockerfile b/agent/Dockerfile index 2f79e0c..1f95b02 100644 --- a/agent/Dockerfile +++ b/agent/Dockerfile @@ -27,6 +27,23 @@ COPY fc_agent ./fc_agent # imgutils ONNX models + the transformers SigLIP weights both cache here; mount # a volume to persist them across restarts (the SigLIP download is ~3.5 GB once). ENV HF_HOME=/models + +# Declared LAST on purpose, exactly as the web Dockerfile does: an ARG/ENV +# invalidates every layer below it, and these are the only values that differ +# between builds of otherwise identical source. Any earlier and the ~6.3 GB +# CUDA + torch layers could never be shared between the dev and main builds of +# one commit — which is the cost #3114 measured at 9m26s cold. +# +# Three values, never folded together (rule 149) — the NAME a person reads, the +# CHANNEL it came from, and the REVISION that identifies the content. See +# fc_agent/build_info.py; CI derives all three from scripts/artifacts.sh. +ARG FC_CHANNEL="" +ENV FC_CHANNEL=${FC_CHANNEL} +ARG FC_VERSION="" +ENV FC_VERSION=${FC_VERSION} +ARG FC_REVISION="" +ENV FC_REVISION=${FC_REVISION} + EXPOSE 8770 # The control UI; the worker is started from it (or POST /start). diff --git a/agent/fc_agent/app.py b/agent/fc_agent/app.py index 1338d28..f9a5d8a 100644 --- a/agent/fc_agent/app.py +++ b/agent/fc_agent/app.py @@ -12,16 +12,21 @@ from fastapi import FastAPI, Request from fastapi.responses import HTMLResponse, JSONResponse from . import logbuf +from .build_info import FC_CHANNEL, FC_REVISION, FC_VERSION, build_id, display_version from .config import Config from .gpu import read_gpu from .worker import Worker log = logging.getLogger("fc_agent.app") -# Bump on every agent change. The page embeds this and /status reports it; the UI -# warns to reload when they differ — so a stale browser-cached page can't be -# mistaken for "the new image didn't deploy". (Belt-and-braces with no-store.) -VERSION = "2026-07-17.1 · idle model-unload: after ~5 min idle the GPU models release their VRAM and reload on the next job (env IDLE_UNLOAD_SECONDS, 0=off) · sleep mode sheds to one downloader" +# DERIVED at image build time, not hand-maintained — see build_info. This was a +# literal an author was asked to bump on every agent change, and the September +# image printed the same "2026-07-17.1" as the July one, so the surface meant to +# answer "did my pull work?" answered the same either way. +# +# Two values with two jobs, kept apart (rule 149): the page SHOWS the version +# and COMPARES the build id. /status reports both, plus the raw fields, so a +# reader never has to take a formatted string apart to get at one of them. logbuf.install() cfg = Config.from_env() @@ -52,7 +57,14 @@ def _maybe_autostart() -> None: @app.get("/", response_class=HTMLResponse) def index() -> str: - return _PAGE.replace("__BUILD__", VERSION) + # Two substitutions, not one: `__VERSION__` is what a person reads in the + # meta line, `__BUILD_ID__` is what the script compares against /status to + # notice the page is a cached copy from a previous build. + return ( + _PAGE + .replace("__VERSION__", display_version()) + .replace("__BUILD_ID__", build_id()) + ) @app.post("/start") @@ -117,7 +129,14 @@ def status(): s["fc_url"] = cfg.fc_url s["configured"] = bool(cfg.token) s["queue"] = worker.latest_queue() - s["build"] = VERSION + # `build` is the comparison token the page checks — see build_info. + # `version`/`channel`/`revision` ride BESIDE it rather than inside it, so a + # reader wanting the version never has to parse it back out of something + # else. Absent rather than empty when the image carries no stamp. + s["build"] = build_id() + s["version"] = FC_VERSION or None + s["channel"] = FC_CHANNEL or None + s["revision"] = FC_REVISION or None return JSONResponse(s) @@ -203,7 +222,7 @@ _PAGE = """
FabledCurator GPU agent
—
-

Server — · token — · build __BUILD__

+

Server — · token — · build __VERSION__