diff --git a/.forgejo/workflows/ci.yml b/.forgejo/workflows/ci.yml index 2acbc37..7f0e7b8 100644 --- a/.forgejo/workflows/ci.yml +++ b/.forgejo/workflows/ci.yml @@ -327,6 +327,14 @@ jobs: packages: write steps: - uses: actions/checkout@v6 + with: + # Rule 149 asks for this on any job deriving the version NAME. The + # name here comes from HEAD's commit TIME, which a depth-1 clone + # already has — but the rule states it unconditionally because the + # failure it guards is silent (a too-low value, every lane green), + # and a later change to how the name is derived would inherit the + # landmine rather than the guard. + fetch-depth: 0 - name: Generate image tags and version id: tags @@ -339,7 +347,27 @@ jobs: # the runner log on commit 2a374d9. run: | TAGS="${{ env.IMAGE }}:${{ github.sha }}" - BUILD_VERSION="dev" + + # THREE VALUES, NEVER FOLDED TOGETHER (rule 149). Until 2026-08-31 + # BUILD_VERSION was the CHANNEL — "dev" / "main" / the tag — so the + # image self-reported {"version":"main"}, a channel name where a + # build identifier belongs. That cost a debugging session: with the + # deploy misbehaving, nothing on the running instance could say + # which commit was serving it. + + # 1. ORDERING KEY — BUILD time, monotonic by construction. Minutes + # since 2020-01-01. Never a commit count (not monotonic across + # branches) and never commit time (goes DOWN when an older + # commit is rebuilt). + BUILD_KEY=$(( ( $(date -u +%s) - 1577836800 ) / 60 )) + + # 2. NAME — COMMIT time, so the same source reports the same string + # on every lane and the channel is the only thing that differs. + COMMIT_TS=$(git log --format=%ct -1 HEAD) + BUILD_NAME=$(date -u -d "@$COMMIT_TS" +%Y.%m.%d.%H%M) + + # 3. CHANNEL — its own value. Never a suffix, never a segment. + CHANNEL="dev" case "${{ github.ref }}" in refs/heads/dev) TAGS="$TAGS,${{ env.IMAGE }}:dev" @@ -348,15 +376,17 @@ jobs: # main IS the production line: publish :latest (plus the : # set above). No separate :main tag. TAGS="$TAGS,${{ env.IMAGE }}:latest" - BUILD_VERSION="main" + CHANNEL="stable" ;; refs/tags/*) TAGS="$TAGS,${{ env.IMAGE }}:latest,${{ env.IMAGE }}:${{ github.ref_name }}" - BUILD_VERSION="${{ github.ref_name }}" + CHANNEL="stable" ;; esac echo "value=$TAGS" >> $GITHUB_OUTPUT - echo "build_version=$BUILD_VERSION" >> $GITHUB_OUTPUT + echo "build_name=$BUILD_NAME" >> $GITHUB_OUTPUT + echo "build_key=$BUILD_KEY" >> $GITHUB_OUTPUT + echo "channel=$CHANNEL" >> $GITHUB_OUTPUT - name: Free disk space # Self-hosted runner housekeeping. Two-step cleanup: @@ -386,7 +416,15 @@ jobs: push: true provenance: false tags: ${{ steps.tags.outputs.value }} - build-args: BUILD_VERSION=${{ steps.tags.outputs.build_version }} + # All three, plus the commit — rule 145: the registry's identity for + # a build (:) and the artifact's identity for itself must + # agree, and they can only be checked against each other if the + # artifact says which commit it is. + build-args: | + BUILD_VERSION=${{ steps.tags.outputs.build_name }} + BUILD_KEY=${{ steps.tags.outputs.build_key }} + BUILD_CHANNEL=${{ steps.tags.outputs.channel }} + BUILD_COMMIT=${{ github.sha }} # Registry-backed layer cache. Pull from :cache to prime # BuildKit, push updated layers back to :cache so the next # build starts warm even if the runner's local cache was diff --git a/Dockerfile b/Dockerfile index ffe1e79..c94d385 100644 --- a/Dockerfile +++ b/Dockerfile @@ -41,10 +41,29 @@ COPY alembic/ alembic/ # Ensure Python finds the source tree (where static files live) before site-packages ENV PYTHONPATH=/app/src -# Version is injected at build time via --build-arg BUILD_VERSION=YY.MM.DD.N -# Falls back to "dev" for local / untagged builds +# THREE VALUES, NEVER FOLDED TOGETHER (rule 149), plus the commit. +# +# BUILD_VERSION is the NAME (YYYY.MM.DD.HHMM, from COMMIT time) — the same +# string on every lane for the same source, so it answers "is this the same +# code?" rather than "which lane built it?". +# BUILD_KEY is the ORDERING KEY (minutes since 2020-01-01, from BUILD time) — +# the only value anything may compare to decide what is newer. +# BUILD_CHANNEL is its own field. Never a suffix, never a segment of the name. +# BUILD_COMMIT lets the artifact's self-report be checked against the : +# it was published under (rule 145). +# +# Each defaults to empty rather than to a placeholder, EXCEPT the name: a +# local build genuinely has no ordering key or channel, and the endpoint says +# so by omitting them. Inventing values would make a local image claim a +# position in an update order it is not part of. ARG BUILD_VERSION=dev +ARG BUILD_KEY= +ARG BUILD_CHANNEL= +ARG BUILD_COMMIT= ENV APP_VERSION=$BUILD_VERSION +ENV APP_BUILD_KEY=$BUILD_KEY +ENV APP_CHANNEL=$BUILD_CHANNEL +ENV APP_COMMIT=$BUILD_COMMIT EXPOSE 5000 CMD ["sh", "-c", "alembic upgrade head && hypercorn 'scribe.app:create_app()' --bind 0.0.0.0:5000 --keep-alive 600"] diff --git a/src/scribe/routes/api.py b/src/scribe/routes/api.py index a15fd84..7cf9477 100644 --- a/src/scribe/routes/api.py +++ b/src/scribe/routes/api.py @@ -10,6 +10,61 @@ async def health(): return jsonify({"status": "ok"}) +def build_version_payload() -> dict: + """What build is this, separated into the values that answer different + questions (rule 149). + + UNTIL 2026-08-31 THIS RETURNED THE CHANNEL. `BUILD_VERSION` in CI was + literally "dev" / "main" / the tag, so a running instance reported + `{"version": "main"}` — a channel name sitting where a build identifier + belongs. The cost was concrete: with a deploy misbehaving, nothing on the + instance could say which commit was serving it, and the one endpoint whose + job that is answered with the name of a branch. + + The three values, and why they are three: + + - `version` — the NAME, `YYYY.MM.DD.HHMM` from COMMIT time. Answers "is + this the same code?", so two channels carrying one commit report the + same string. + - `build` — the ORDERING KEY, minutes since 2020-01-01 from BUILD time. + Answers "may this be installed over that?". The ONLY value anything may + compare; it is monotonic by construction, which neither a commit count + (branches diverge) nor a commit time (rebuilds go backwards) is. + - `channel` — its own field, never folded into the name. + + Plus `commit`, so the artifact's claim about itself can be checked against + the `:` it was published under (rule 145). + + ABSENT RATHER THAN EMPTY when unknown. A local build has no ordering key + and no channel, and saying so is honest; emitting `""` or a placeholder + would let it claim a position in an update order it is not part of. A + reader must treat a missing `build` as "cannot be ordered", not as zero. + """ + payload: dict = {"version": os.environ.get("APP_VERSION", "dev")} + + # Reported verbatim, never validated against an enum — a build claiming + # something unexpected is better shown than dropped (rule 149). + for key, env in (("channel", "APP_CHANNEL"), ("commit", "APP_COMMIT")): + value = (os.environ.get(env) or "").strip() + if value: + payload[key] = value + + raw_key = (os.environ.get("APP_BUILD_KEY") or "").strip() + if raw_key: + try: + # An INTEGER, not a string. A string ordering key is how a + # comparison silently becomes lexicographic — "9" > "10" — which + # is the same class of fault as folding the channel in: it reads + # fine and orders wrong. + payload["build"] = int(raw_key) + except ValueError: + # A malformed key is omitted rather than passed through: a reader + # that cannot order is correct, one that orders on garbage is not. + pass + + return payload + + @api.route("/version") async def version(): - return jsonify({"version": os.environ.get("APP_VERSION", "dev")}) + return jsonify(build_version_payload()) diff --git a/tests/test_version_endpoint.py b/tests/test_version_endpoint.py new file mode 100644 index 0000000..ebe01e7 --- /dev/null +++ b/tests/test_version_endpoint.py @@ -0,0 +1,182 @@ +"""`/api/version` reports three values, and never folds them together. + +WHAT THIS IS ABOUT (rule 149). Until 2026-08-31 the endpoint returned +`{"version": "main"}` — CI set `BUILD_VERSION` to the CHANNEL, so a running +instance answered the question "which build are you?" with the name of a +branch. The cost was concrete rather than theoretical: during #3244's live +acceptance a deploy was behaving as though it held older code, and the one +endpoint whose job is to settle that could not. + +The three values answer different questions and so cannot be one value: + + version the NAME, from COMMIT time — "is this the same code?" + build the ORDERING KEY, BUILD time — "may this be installed over that?" + channel its own field — "which line is this?" + +These pin the SHAPE the lanes emit, not the values — a test asserting today's +timestamp would fail tomorrow, and one asserting the format catches the thing +that actually breaks: a channel creeping back into the name, or an ordering +key that is not orderable. +""" +import os +import pathlib +import re +from datetime import datetime, timezone +from unittest.mock import patch + +import pytest + +CI = pathlib.Path(__file__).resolve().parents[1] / ".forgejo/workflows/ci.yml" + +# The NAME's shape: four dot-separated numeric fields, zero-padded, and +# nothing else. A channel token anywhere in here is the bug this file exists +# to prevent. +NAME_RE = re.compile(r"^\d{4}\.\d{2}\.\d{2}\.\d{4}$") + + +_ENV_KEYS = ("APP_VERSION", "APP_BUILD_KEY", "APP_CHANNEL", "APP_COMMIT") + + +def _version_payload(env: dict) -> dict: + """The real payload builder, under a controlled environment. + + Calls `build_version_payload` rather than the route: the payload is the + behaviour, and reaching it through an app and a request context would + make these tests depend on app startup to assert a dict. The route is a + one-line `jsonify` wrapper over this. + """ + from scribe.routes.api import build_version_payload + + with patch.dict(os.environ, env, clear=False): + # patch.dict cannot REMOVE, and "absent" is exactly what several of + # these assert — so anything the caller left out is cleared. + for key in _ENV_KEYS: + if key not in env: + os.environ.pop(key, None) + return build_version_payload() + + +def test_the_three_values_are_three_fields(): + """The headline. One field cannot answer three questions, and the failure + mode of trying is silent: the string looks plausible and orders wrong.""" + out = _version_payload({ + "APP_VERSION": "2026.08.31.0403", + "APP_BUILD_KEY": "3505443", + "APP_CHANNEL": "stable", + "APP_COMMIT": "b267037", + }) + assert out["version"] == "2026.08.31.0403" + assert out["build"] == 3505443 + assert out["channel"] == "stable" + assert out["commit"] == "b267037" + + +def test_the_channel_is_never_inside_the_name(): + """The regression itself. `{"version": "main"}` is what this catches.""" + out = _version_payload({ + "APP_VERSION": "2026.08.31.0403", "APP_CHANNEL": "stable", + }) + assert NAME_RE.match(out["version"]), ( + f"the version name is {out['version']!r} — not YYYY.MM.DD.HHMM. A " + f"channel or branch name here is the 2026-08-31 bug returning." + ) + assert "stable" not in out["version"] + + +def test_the_ordering_key_is_an_INTEGER(): + """A string ordering key is how a comparison silently becomes + lexicographic — "9" > "10" — which reads fine and orders wrong.""" + out = _version_payload({"APP_VERSION": "x", "APP_BUILD_KEY": "3505443"}) + assert isinstance(out["build"], int) + assert not isinstance(out["build"], bool) + + +def test_unknown_values_are_ABSENT_not_empty(): + """A local build genuinely has no ordering key and no channel. Emitting + `""` or a placeholder would let it claim a position in an update order it + is not part of; a reader must see "cannot be ordered", not zero.""" + out = _version_payload({"APP_VERSION": "dev"}) + assert out == {"version": "dev"} + assert "build" not in out and "channel" not in out and "commit" not in out + + +def test_an_empty_env_var_counts_as_absent(): + """Docker sets an ARG with no default to the empty string, so "unset" and + "set to nothing" both reach the handler as ''.""" + out = _version_payload({ + "APP_VERSION": "dev", "APP_CHANNEL": "", "APP_BUILD_KEY": "", + "APP_COMMIT": " ", + }) + assert out == {"version": "dev"} + + +def test_a_malformed_ordering_key_is_dropped_not_passed_through(): + """A reader that cannot order is correct; one that orders on garbage is + not. Dropping it degrades to "unorderable", which is a state the caller + already has to handle.""" + out = _version_payload({"APP_VERSION": "dev", "APP_BUILD_KEY": "main"}) + assert "build" not in out + + +def test_the_channel_is_reported_verbatim(): + """Never validated against an enum — a build claiming something + unexpected is better shown than dropped (rule 149).""" + out = _version_payload({"APP_VERSION": "dev", "APP_CHANNEL": "canary"}) + assert out["channel"] == "canary" + + +# ── The lane, as CI actually writes it ───────────────────────────────── + +def test_ci_does_not_stamp_the_channel_as_the_version(): + """The bug lived in the workflow, not the handler. A correct handler fed + `BUILD_VERSION=main` still reports a branch name.""" + text = CI.read_text() + assert "BUILD_VERSION=${{ steps.tags.outputs.build_name }}" in text, ( + "CI no longer passes the derived NAME as BUILD_VERSION. If it is " + "passing a branch or channel again, /api/version is lying." + ) + for wrong in ('BUILD_VERSION="main"', 'BUILD_VERSION="dev"'): + assert wrong not in text, ( + f"CI sets {wrong} — that is the channel in the version field, " + f"which is the 2026-08-31 regression." + ) + + +def test_ci_derives_the_name_from_COMMIT_time_and_the_key_from_BUILD_time(): + """The two clocks are deliberate and easy to "tidy" into one. + + The name must come from the commit so two lanes building one source agree; + the key must come from the build so it cannot go backwards when an older + commit is rebuilt. Collapsing them breaks whichever question loses. + """ + text = CI.read_text() + assert "git log --format=%ct -1 HEAD" in text, ( + "the version NAME is no longer derived from commit time — two lanes " + "building the same commit will now report different strings" + ) + assert "$(date -u +%s) - 1577836800" in text, ( + "the ORDERING KEY is no longer minutes-since-2020 from build time; " + "if it now comes from the commit it can go backwards on a rebuild" + ) + + +def test_ci_passes_all_three_plus_the_commit(): + text = CI.read_text() + for arg in ("BUILD_KEY=", "BUILD_CHANNEL=", "BUILD_COMMIT="): + assert arg in text, f"CI no longer passes {arg} to the image build" + + +@pytest.mark.parametrize("commit_epoch,expected", [ + # Midnight, where a naive formatter drops the leading zeros and yields + # "2026.01.05.0" — rule 149 names this case specifically. + (datetime(2026, 1, 5, 0, 0, tzinfo=timezone.utc), "2026.01.05.0000"), + (datetime(2026, 1, 5, 0, 7, tzinfo=timezone.utc), "2026.01.05.0007"), + (datetime(2026, 12, 31, 23, 59, tzinfo=timezone.utc), "2026.12.31.2359"), + (datetime(2026, 8, 31, 4, 3, tzinfo=timezone.utc), "2026.08.31.0403"), +]) +def test_the_name_format_zero_pads_every_field(commit_epoch, expected): + """`date -u +%Y.%m.%d.%H%M` is what CI runs; this pins what that must + produce, so a reformat that loses zero-padding fails here rather than in + a comparison months later.""" + assert commit_epoch.strftime("%Y.%m.%d.%H%M") == expected + assert NAME_RE.match(expected)