fix(ci): /api/version reported the channel where the build belongs (rule 149)
CI & Build / Python lint (push) Successful in 5s
CI & Build / TypeScript typecheck (push) Successful in 11s
CI & Build / Plugin hooks (push) Successful in 11s
CI & Build / integration (push) Successful in 31s
CI & Build / Python tests (push) Successful in 1m3s
CI & Build / Build & push image (push) Successful in 28s
CI & Build / Python lint (push) Successful in 5s
CI & Build / TypeScript typecheck (push) Successful in 11s
CI & Build / Plugin hooks (push) Successful in 11s
CI & Build / integration (push) Successful in 31s
CI & Build / Python tests (push) Successful in 1m3s
CI & Build / Build & push image (push) Successful in 28s
CI set BUILD_VERSION to the CHANNEL — literally "dev", "main", or the tag —
so a running instance answered "which build are you?" with the name of a
branch: {"version":"main"}. 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.
Rule 149's three values, now three fields:
version the NAME, YYYY.MM.DD.HHMM from COMMIT time — "is this the same
code?", so two lanes carrying one commit report one string
build the ORDERING KEY, minutes since 2020-01-01 from BUILD time —
"may this be installed over that?", and the only value anything
may compare
channel its own field. Never a suffix, never a segment of the name
Plus `commit`, so the artifact's claim about itself can be checked against
the :<sha> it was published under (rule 145) — which is exactly the question
that could not be answered tonight.
THE TWO CLOCKS ARE DELIBERATE and look like an inconsistency. The name comes
from the commit so two lanes building one source agree; the key comes from
the build so it cannot go backwards when an older commit is rebuilt. A test
pins both derivations against being "tidied" into one.
ABSENT RATHER THAN EMPTY when unknown. A local build 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 malformed key is dropped rather than passed
through — a reader that cannot order is correct, one that orders on garbage
is not. The key is an int, because a string ordering key is how a comparison
silently becomes lexicographic ("9" > "10").
The payload builder is extracted from the route so it can be tested as a
dict rather than through app startup and a request context.
Tests pin the SHAPE the lanes emit, not the values, including the midnight
leading-zero case rule 149 names specifically — and assert CI never stamps a
branch name as the version again.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -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 :<sha>
|
||||
# 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 (:<sha>) 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
|
||||
|
||||
+21
-2
@@ -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 :<sha>
|
||||
# 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"]
|
||||
|
||||
@@ -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 `:<sha>` 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())
|
||||
|
||||
@@ -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)
|
||||
Reference in New Issue
Block a user