fix(release): drop version image tags, mint the rollback unit on main
test-go / test (push) Successful in 1m43s
test-web / test (push) Successful in 1m13s
test-go / integration (push) Successful in 4m12s
release / Build signed APK (releases and dev) (push) Successful in 5m11s
release / Build + push container image (push) Successful in 38s
release / Verify release artifacts (tag releases only) (push) Skipped

The image tag map was the inverse of family rules 145 and 147 on every
count: it published :vYYYY.MM.DD.HHMM that nobody pinned, published :main
that rule 147 says should not exist, and published no commit-addressable
image at all — so the rollback unit the rule names did not exist in this
repo. A bad main push had nothing to roll back to but the previous
release tag, which may be many commits back.

The whole map is now:

  dev  → :dev
  main → :latest + :<sha>
  tag  → :latest

A release refreshes the channel and mints nothing else. The tag build
rebuilds the SAME SOURCE as main's build minutes earlier, differing only
in which APK is baked in, so rule 145's immutability clause applies
directly: move the channel tag, never re-push a commit-addressable one.
:latest has to move here rather than waiting for the next main push, or
the channel would carry the previous release's APK indefinitely — a
channel that cannot refresh itself (rule 146).

Two consequences that are not optional:

The verify job asserted the :<version> image existed. With version tags
gone that would fail every release for a tag nothing mints. Re-pointed at
the :<sha> image rather than deleted — deleting it is the tempting way to
make a failing guard go green, and it earns its keep twice now: it still
catches an image push that silently did not happen, and it additionally
proves the ordering, since a tag cut on a commit whose main build never
completed has no rollback target.

The server's self-reported version was the literal string "main" or
"dev". That was survivable while :vYYYY.MM.DD.HHMM existed to identify a
build; with version tags gone it is the ONLY thing that says which build
is running, and two dev images months apart were indistinguishable. It
now carries the derived name from ci/version.sh on every lane, with the
channel as a sibling field (rule 149) rather than folded into the string.
Surfaced at /healthz and beside the version in Settings.

Guards added for each arm of the policy, and every one was falsified
against the specific regression it names before committing. That caught
two real bugs in the guards themselves: stepBody cut at the next
`- name:`, which returns an EMPTY body for the last step in a job and
made the assertions pass vacuously, and its replacement cut at any blank
line followed by indentation, which truncated a step mid-run-block. The
helper now refuses an empty body outright.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SQ31KQpYbStyK5y58UmPLH
This commit is contained in:
2026-09-10 15:10:15 -04:00
co-authored by Claude Opus 5
parent 88508b536b
commit aeb8781c4e
7 changed files with 366 additions and 68 deletions
+196 -14
View File
@@ -4,6 +4,7 @@ import (
"os"
"os/exec"
"path/filepath"
"regexp"
"strconv"
"strings"
"testing"
@@ -178,19 +179,86 @@ func TestReleaseWorkflow_UsesTheSharedDerivation(t *testing.T) {
}
}
// devArm returns the branch of "Compute image tags" that handles refs/heads/dev.
func devArm(t *testing.T, yaml string) string {
// jobBoundary matches a blank line followed by job-level (two-space)
// indentation — the end of the last step in a job.
var jobBoundary = regexp.MustCompile(`\n\n [^ \n]`)
// stepBody returns one workflow step's text, from its `- name:` line to the
// start of the next step or the next job.
//
// The naive cut — "up to the next `- name:`" — silently returns an EMPTY body
// for the last step in a job, and every assertion over it then passes
// vacuously. That is the failure rule 167 names: a check that reads as
// coverage while asserting on nothing. Cutting at a blank line followed by
// job-level indentation handles the last-step case, which is exactly where
// `Build and push` sits.
func stepBody(t *testing.T, yaml, name string) string {
t.Helper()
const marker = `elif [[ "${GITHUB_REF}" == "refs/heads/dev" ]]; then`
i := strings.Index(yaml, marker)
i := strings.Index(yaml, "- name: "+name)
if i < 0 {
t.Fatal("no refs/heads/dev arm in Compute image tags — the dev channel is not wired")
t.Fatalf("no %q step in release.yml", name)
}
rest := yaml[i+len(marker):]
if j := strings.Index(rest, "\n else"); j >= 0 {
return rest[:j]
body := yaml[i:]
end := len(body)
if j := strings.Index(body, "\n - name:"); j >= 0 {
end = j
}
return rest
// A blank line followed by EXACTLY two spaces and then content starts a
// new job or job-level comment. The "exactly" matters: a blank line inside
// a `run:` block is followed by ten-space indentation and would otherwise
// match, truncating the step mid-body — which is how this helper first
// sliced the verify step down to its first two lines.
if loc := jobBoundary.FindStringIndex(body); loc != nil && loc[0] < end {
end = loc[0]
}
body = body[:end]
if strings.TrimSpace(strings.TrimPrefix(body, "- name: "+name)) == "" {
t.Fatalf("step %q sliced to an empty body — the assertions below would pass vacuously", name)
}
return body
}
// releaseYAML reads the workflow once per test.
func releaseYAML(t *testing.T) string {
t.Helper()
body, err := os.ReadFile(filepath.Join(repoRoot(t), ".gitea", "workflows", "release.yml"))
if err != nil {
t.Fatal(err)
}
return string(body)
}
// imageTagArms splits "Compute image tags" into its three ref branches. The
// whole tag policy lives in that if/elif/else, so the arms are the unit worth
// asserting on — a tag published from the wrong arm reaches the wrong
// audience, and every such mistake still builds and still pushes a valid
// image.
func imageTagArms(t *testing.T, yaml string) (tagArm, devArm, mainArm string) {
t.Helper()
const (
tagMarker = `if [[ "${GITHUB_REF}" == refs/tags/v* ]]; then`
devMarker = `elif [[ "${GITHUB_REF}" == "refs/heads/dev" ]]; then`
mainMarker = "\n else"
endMarker = "\n fi"
)
iTag := strings.Index(yaml, tagMarker)
iDev := strings.Index(yaml, devMarker)
iMain := -1
if iDev >= 0 {
if k := strings.Index(yaml[iDev:], mainMarker); k >= 0 {
iMain = iDev + k
}
}
if iTag < 0 || iDev < 0 || iMain < 0 {
t.Fatal("Compute image tags no longer has a tag/dev/main arm — the tag policy has been restructured, so these guards are pinning nothing")
}
iEnd := iMain
if k := strings.Index(yaml[iMain:], endMarker); k >= 0 {
iEnd = iMain + k
} else {
t.Fatal("no closing fi after the main arm")
}
return yaml[iTag:iDev], yaml[iDev:iMain], yaml[iMain:iEnd]
}
// The worst regression this wiring can produce: a dev push that also moves
@@ -198,11 +266,7 @@ func devArm(t *testing.T, yaml string) string {
// next pull. Nothing else in the suite would notice — the build stays green
// and the image is valid, it is simply the wrong audience.
func TestDevChannel_PublishesDevAloneAndNeverLatest(t *testing.T) {
body, err := os.ReadFile(filepath.Join(repoRoot(t), ".gitea", "workflows", "release.yml"))
if err != nil {
t.Fatal(err)
}
arm := devArm(t, string(body))
_, arm, _ := imageTagArms(t, releaseYAML(t))
if !strings.Contains(arm, "${IMAGE}:dev") {
t.Errorf("dev arm does not publish :dev\n%s", arm)
@@ -286,3 +350,121 @@ func TestBundleStep_GrepsCannotKillTheStep(t *testing.T) {
}
}
}
// The rollback unit, and the reason it is worth a guard: it is invisible until
// the moment it is needed. Nothing pulls :<sha> during normal operation, so if
// this arm stopped minting one, every build would stay green and every image
// would be valid — and the absence would surface only during an incident, as
// "there is nothing to roll back to."
//
// Rules 145 and 147: main push → :latest + :<sha>.
func TestMainChannel_PublishesLatestAndTheRollbackUnit(t *testing.T) {
_, _, arm := imageTagArms(t, releaseYAML(t))
if !strings.Contains(arm, "${IMAGE}:latest") {
t.Errorf("main arm does not move :latest — production would stop tracking main's tip\n%s", arm)
}
if !strings.Contains(arm, "${IMAGE}:${GITHUB_SHA}") {
t.Errorf("main arm publishes no commit-addressable image; there is no rollback target for production commits\n%s", arm)
}
// Rule 147: :latest tracks main's tip, and a second name for the same
// image sends readers looking for a distinction that does not exist.
if strings.Contains(arm, "${IMAGE}:main") {
t.Errorf("main arm publishes :main — rule 147 says that tag should not exist\n%s", arm)
}
}
// A release refreshes the CHANNEL and mints nothing else (rules 145 + 146).
//
// The specific regression: re-adding :<sha> here. The tag build rebuilds the
// SAME SOURCE as main's build minutes earlier, differing only in which APK is
// baked in — so a :<sha> minted here would overwrite main's immutable rollback
// target with different contents, under the same name. That is the exact thing
// rule 145's immutability clause exists to prevent, and it is the half with
// the incidents behind it.
func TestReleaseBuild_RefreshesTheChannelAndMintsNothingElse(t *testing.T) {
arm, _, _ := imageTagArms(t, releaseYAML(t))
if !strings.Contains(arm, "${IMAGE}:latest") {
t.Errorf("tag arm does not refresh :latest — the channel would keep serving the PREVIOUS release's APK until someone pushed to main\n%s", arm)
}
if strings.Contains(arm, "GITHUB_SHA") {
t.Errorf("tag arm mints a :<sha> image; that would re-push main's immutable rollback target with different bundled contents\n%s", arm)
}
}
// No version-numbered image tags anywhere, on any arm (rule 145, and the
// operator's 2026-09-10 decision to drop them across every project).
//
// Asserted across the whole step rather than per-arm because the mistake this
// catches is re-adding one ANYWHERE, and the tag arm is only the likeliest
// spot. `${VERSION}` still legitimately appears in the step as the build's
// self-reported version, so the assertion has to name the image-tag form
// specifically rather than the variable — otherwise it would fire on correct
// code and get "fixed" by deleting the guard.
func TestNoVersionNumberedImageTags(t *testing.T) {
yaml := releaseYAML(t)
tagArm, devArm, mainArm := imageTagArms(t, yaml)
for _, tc := range []struct{ name, arm string }{
{"tag", tagArm}, {"dev", devArm}, {"main", mainArm},
} {
for _, forbidden := range []string{
"${IMAGE}:${VERSION}",
"${IMAGE}:v",
"${IMAGE}:${GITHUB_REF#refs/tags/}",
} {
if strings.Contains(tc.arm, forbidden) {
t.Errorf("%s arm publishes a version-numbered image tag (%q); git and the build's self-reported version answer \"which build is this\"\n%s",
tc.name, forbidden, tc.arm)
}
}
}
}
// The verify job asserted the :<version> image existed. With version tags
// gone that assertion would fail every release for a tag nothing mints — so
// this pins that it was re-pointed rather than deleted, since deleting it is
// the tempting way to make a failing guard go green.
func TestVerifyJob_ChecksTheRollbackImageNotAVersionTag(t *testing.T) {
yaml := releaseYAML(t)
if !strings.Contains(yaml, "- name: Rollback image must exist for the tagged commit") {
t.Fatal("the release-verification step that checks an image exists is gone; an image push that silently did not happen would now pass verification")
}
step := stepBody(t, yaml, "Rollback image must exist for the tagged commit")
if !strings.Contains(step, "${IMAGE}:${GITHUB_SHA}") {
t.Errorf("the verify step does not inspect the commit's rollback image\n%s", step)
}
if strings.Contains(step, "${IMAGE}:${TAG}") {
t.Errorf("the verify step still inspects a version-numbered image, which is no longer published — this would fail every release\n%s", step)
}
}
// The server's self-reported version is now the ONLY thing that identifies a
// build, so a lane that stamps a channel word instead of a version silently
// removes that ability. It used to stamp the literal "main"/"dev".
func TestImageBuild_StampsADerivedVersionAndAChannel(t *testing.T) {
yaml := releaseYAML(t)
step := stepBody(t, yaml, "Build and push")
for _, want := range []string{
"MINSTREL_VERSION=",
"MINSTREL_CHANNEL=",
} {
if !strings.Contains(step, want) {
t.Errorf("Build and push does not pass %s — the image cannot report which build it is\n%s", want, step)
}
}
// The version must come from the shared derivation, not from the ref.
// Reading it off GITHUB_REF is what produced "main" and "dev" as version
// strings, which is the regression this pins.
_, _, mainArm := imageTagArms(t, yaml)
if strings.Contains(mainArm, `version=main`) {
t.Errorf("the main arm stamps the literal string \"main\" as a version; two images months apart would be indistinguishable\n%s", mainArm)
}
if !strings.Contains(yaml, "ci/version.sh HEAD | sed") {
t.Error("the image job no longer derives its version from ci/version.sh — the version and the APK's version can now drift apart")
}
}
+1
View File
@@ -218,6 +218,7 @@ func (s *Server) handleHealthz(w http.ResponseWriter, _ *http.Request) {
_ = json.NewEncoder(w).Encode(map[string]string{
"status": "ok",
"version": ServerVersion,
"channel": ServerChannel,
"min_client_version": MinClientVersion,
})
}
+21 -6
View File
@@ -5,12 +5,27 @@ package server
// older clients see version_too_old at /healthz and refuse to operate.
const MinClientVersion = "0.1.0"
// ServerVersion is the deployed server image's version tag. Defaults to
// "dev" for local builds; overridden at link time via:
// ServerVersion is the build's own version name — YYYY.MM.DD.HHMM, derived
// from the commit it was built from by ci/version.sh. Defaults to "dev" for
// local builds; overridden at link time via:
//
// -ldflags="-X 'git.fabledsword.com/bvandeusen/minstrel/internal/server.ServerVersion=v2026.05.10.2'"
// -ldflags="-X 'git.fabledsword.com/bvandeusen/minstrel/internal/server.ServerVersion=2026.09.10.1449'"
//
// release.yml passes the git tag through MINSTREL_VERSION build-arg →
// Dockerfile ldflag. Surfaced at /healthz so operators can verify which
// image their container is running without exec'ing into it.
// release.yml passes it through the MINSTREL_VERSION build-arg → Dockerfile
// ldflag. Surfaced at /healthz so operators can verify which image their
// container is running without exec'ing into it.
//
// This carried the literal strings "main" and "dev" until 2026-09-10, which
// made every image on a channel report the same thing forever. It stopped
// being cosmetic when :vYYYY.MM.DD.HHMM image tags were retired (family rule
// 145): this is now the ONLY thing that says which build is running.
var ServerVersion = "dev"
// ServerChannel is which line this build came off — "stable" or "dev", or
// "local" for a plain `docker build`.
//
// A SIBLING FIELD, never a suffix inside ServerVersion (family rule 149). The
// same commit built on both lanes reports the same version and differs only
// here; folding the two together is what makes a version string stop being
// comparable.
var ServerChannel = "local"