diff --git a/.gitea/workflows/release.yml b/.gitea/workflows/release.yml index c0bcdfaa..8389ab32 100644 --- a/.gitea/workflows/release.yml +++ b/.gitea/workflows/release.yml @@ -110,45 +110,12 @@ jobs: working-directory: ${{ github.workspace }} run: | set -euo pipefail - - # Two different clocks, deliberately. They answer different - # questions, and using one for both breaks whichever it fits worse. - # - # The NAME answers "is this the same code?" — so it derives from - # COMMIT time and reads identically on every lane that builds this - # source. A dev build and a main build of one commit must report the - # same string; build time cannot do that, it prints two numbers for - # one thing. - COMMIT_TS=$(git log --format=%ct -1 HEAD) - VERSION_NAME=$(date -u -d "@${COMMIT_TS}" +%Y.%m.%d.%H%M) - - # The ORDERING KEY answers "may this be installed over that?" — so it - # must be monotonic BY CONSTRUCTION. Minutes since 2020-01-01: ~3.5M - # today, ~525k/year, against a 2^31 ceiling. - # - # This replaced `git rev-list --count HEAD`, which was NOT monotonic - # and was commented as if it were. A commit count runs ahead on `dev`, - # so a dev build outranked the `main` release that superseded it and - # Android refused the install as a downgrade — a channel you could - # enter and not leave without uninstalling. - # - # Commit time would be wrong here too, for the mirror-image reason: - # rebuild an older commit and it goes DOWN, which on a phone is a - # refused install rather than a merely confusing label. - VERSION_CODE=$(( ( $(date -u +%s) - 1577836800 ) / 60 )) - - # Assert the emitted shape at the source. A malformed name still - # builds, signs and publishes perfectly happily, and only surfaces as - # an update nobody is ever offered — which nobody reports, because - # "no update available" and "I cannot read this" look identical. - if [[ ! "${VERSION_NAME}" =~ ^[0-9]{4}\.[0-9]{2}\.[0-9]{2}\.[0-9]{4}$ ]]; then - echo "::error::version name '${VERSION_NAME}' is not YYYY.MM.DD.HHMM" - exit 1 - fi - - echo "name=${VERSION_NAME}" >> "$GITHUB_OUTPUT" - echo "code=${VERSION_CODE}" >> "$GITHUB_OUTPUT" - echo "::notice::APK version: ${VERSION_NAME} (code=${VERSION_CODE})" + # The derivation lives in ci/version.sh, not here, so it can be + # executed by a test on every push. Anything inline in this file is + # unverifiable until a release is already running. + out="$(ci/version.sh HEAD)" + printf '%s\n' "${out}" >> "$GITHUB_OUTPUT" + echo "::notice::APK $(printf '%s' "${out}" | tr '\n' ' ')" # Checked BEFORE the expensive work, not after it. "Attach APK to gitea # Release" below resolves the release by tag and fails if it is absent — diff --git a/.gitea/workflows/test-go.yml b/.gitea/workflows/test-go.yml index b3eb990f..49cb3684 100644 --- a/.gitea/workflows/test-go.yml +++ b/.gitea/workflows/test-go.yml @@ -32,6 +32,12 @@ on: - 'cmd/**' - '.golangci.yml' - '.gitea/workflows/test-go.yml' + # The release lane's own trigger is `main` + tags, so nothing it + # contains is exercised until a release is already running. These two + # entries are what let internal/server/release_version_test.go guard + # the version derivation on ordinary dev pushes instead. + - 'ci/**' + - '.gitea/workflows/release.yml' # pull_request trigger intentionally omitted — see test-web.yml for # the rationale (single-author repo, push covers PR-merge equivalent). diff --git a/ci/version.sh b/ci/version.sh new file mode 100755 index 00000000..36583f45 --- /dev/null +++ b/ci/version.sh @@ -0,0 +1,71 @@ +#!/usr/bin/env bash +# +# Derives the three values a build is stamped with, and the tag that names it. +# +# name=YYYY.MM.DD.HHMM label for people, from the COMMIT's timestamp +# code= ordering key, minutes since 2020-01-01 at BUILD time +# tag=v what a release of this commit must be called +# +# Usage: ci/version.sh [] (default HEAD) +# +# This exists as a script rather than inline workflow YAML for one reason: +# release.yml only runs on `main` and on tags, so anything living inside it is +# unverifiable until a release is already happening — which is the worst +# possible moment to discover the version is wrong, because the failure mode +# is silent (an update nobody is offered looks exactly like being current). +# As a script it can be executed by a test on every push instead. +# +# The two clocks are deliberate and are NOT interchangeable: +# +# The NAME answers "is this the same code?" — so it must read identically on +# every lane that builds this commit. Commit time does that; build time +# prints two different strings for one thing. +# +# The CODE answers "may this be installed over that?" — so it must be +# monotonic BY CONSTRUCTION. Build time is; commit time is not (rebuild an +# older commit and it goes down, which on a phone is a refused install), and +# a commit COUNT is worse still, because it runs ahead on `dev` and inverts +# against `main`. +set -euo pipefail + +readonly EPOCH_2020=1577836800 # 2020-01-01T00:00:00Z +readonly REF="${1:-HEAD}" + +# Both clocks are overridable so a test can pin them. Nothing but tests should +# set these — the defaults are the real derivation. +commit_epoch="${MINSTREL_COMMIT_EPOCH:-}" +if [ -z "${commit_epoch}" ]; then + commit_epoch="$(git log --format=%ct -1 "${REF}")" +fi +now_epoch="${MINSTREL_NOW_EPOCH:-$(date -u +%s)}" + +if ! name="$(date -u -d "@${commit_epoch}" +%Y.%m.%d.%H%M 2>/dev/null)"; then + echo "version.sh: could not read a commit timestamp from '${commit_epoch}'" >&2 + exit 1 +fi + +if ! [ "${now_epoch}" -eq "${now_epoch}" ] 2>/dev/null; then + echo "version.sh: build timestamp '${now_epoch}' is not a number" >&2 + exit 1 +fi +code=$(( (now_epoch - EPOCH_2020) / 60 )) + +# Assert the shape here, at the source. A malformed name builds, signs and +# publishes perfectly happily; it only surfaces later as an update channel +# that has quietly stopped offering anything. +if [[ ! "${name}" =~ ^[0-9]{4}\.[0-9]{2}\.[0-9]{2}\.[0-9]{4}$ ]]; then + echo "version.sh: name '${name}' is not YYYY.MM.DD.HHMM" >&2 + exit 1 +fi + +# A non-positive key means the build clock is set before 2020, and every +# comparison downstream would be nonsense. +if [ "${code}" -le 0 ]; then + echo "version.sh: ordering key '${code}' is not positive — build clock wrong?" >&2 + exit 1 +fi + +# KEY=VALUE, which is also exactly $GITHUB_OUTPUT's format. +echo "name=${name}" +echo "code=${code}" +echo "tag=v${name}" diff --git a/internal/server/release_version_test.go b/internal/server/release_version_test.go new file mode 100644 index 00000000..a63dcbcd --- /dev/null +++ b/internal/server/release_version_test.go @@ -0,0 +1,179 @@ +package server + +import ( + "os" + "os/exec" + "path/filepath" + "strconv" + "strings" + "testing" +) + +// Guards the version derivation that stamps every build. +// +// These assertions used to be impossible to run. The derivation lived inline +// in release.yml, which triggers only on `main` and on tags — so a mistake in +// it could not surface until a release was already under way, and its failure +// mode is silence: a version nobody can compare looks exactly like being up to +// date, and nobody reports an update they were never offered. +// +// Moving it to ci/version.sh made it executable, so this runs on every push +// that touches the release machinery. That is the whole point of the file; the +// specific assertions below matter less than the fact that they run at all. +// +// The tests EXECUTE the script rather than asserting on its text, so they +// break when the behaviour changes rather than when the wording does. + +// highestOldSchemeCode is the largest versionCode ever shipped under the +// retired commit-count scheme (v2026.09.09 shipped 1895). Every code the new +// scheme emits must clear it, or Android would refuse the upgrade as a +// downgrade and the update channel would be a one-way door. +const highestOldSchemeCode = 1895 + +func repoRoot(t *testing.T) string { + t.Helper() + dir, err := os.Getwd() + if err != nil { + t.Fatal(err) + } + for { + if _, err := os.Stat(filepath.Join(dir, "go.mod")); err == nil { + return dir + } + parent := filepath.Dir(dir) + if parent == dir { + t.Fatalf("no go.mod above %s", dir) + } + dir = parent + } +} + +// runVersion executes ci/version.sh with both clocks pinned, so the result is +// deterministic. Returns the parsed KEY=VALUE output. +func runVersion(t *testing.T, commitEpoch, nowEpoch string) map[string]string { + t.Helper() + out, err := versionScript(t, commitEpoch, nowEpoch) + if err != nil { + t.Fatalf("ci/version.sh failed: %v\n%s", err, out) + } + parsed := map[string]string{} + for _, line := range strings.Split(strings.TrimSpace(out), "\n") { + if k, v, ok := strings.Cut(line, "="); ok { + parsed[k] = v + } + } + return parsed +} + +func versionScript(t *testing.T, commitEpoch, nowEpoch string) (string, error) { + t.Helper() + root := repoRoot(t) + cmd := exec.Command(filepath.Join(root, "ci", "version.sh")) + cmd.Dir = root + cmd.Env = append(os.Environ(), + "MINSTREL_COMMIT_EPOCH="+commitEpoch, + "MINSTREL_NOW_EPOCH="+nowEpoch, + ) + out, err := cmd.CombinedOutput() + return string(out), err +} + +func TestVersionName_IsCommitTimeToTheMinute(t *testing.T) { + // 2025-09-09T18:48:56Z + got := runVersion(t, "1757443736", "1789000920") + if want := "2025.09.09.1848"; got["name"] != want { + t.Errorf("name = %q, want %q", got["name"], want) + } +} + +// HHMM is the segment most likely to be silently mangled, and it only bites +// for about a tenth of the day — a build just after midnight must emit "0042", +// never "42". A stripped leading zero shifts the segment by two orders of +// magnitude and reverses comparisons against every other build that day. +func TestVersionName_PadsTheMinuteSegment(t *testing.T) { + // 2026-09-10T00:42:00Z + got := runVersion(t, "1789000920", "1789000920") + if want := "2026.09.10.0042"; got["name"] != want { + t.Errorf("name = %q, want %q — leading zero lost?", got["name"], want) + } +} + +func TestVersionName_DerivesFromCommitNotBuildClock(t *testing.T) { + // Same commit, two different build clocks: the NAME must not move, or a + // dev build and a main build of one commit would report different strings + // and the channel field would stop being the only thing separating them. + a := runVersion(t, "1757443736", "1789000920") + b := runVersion(t, "1757443736", "1789500000") + if a["name"] != b["name"] { + t.Errorf("name moved with the build clock: %q vs %q", a["name"], b["name"]) + } + if a["code"] == b["code"] { + t.Errorf("code did NOT move with the build clock (%q) — it is not build-derived", a["code"]) + } +} + +func TestVersionCode_IsMinutesSince2020AndClearsTheOldScheme(t *testing.T) { + got := runVersion(t, "1789000920", "1789000920") + code, err := strconv.Atoi(got["code"]) + if err != nil { + t.Fatalf("code %q is not an integer: %v", got["code"], err) + } + if want := (1789000920 - 1577836800) / 60; code != want { + t.Errorf("code = %d, want %d", code, want) + } + if code <= highestOldSchemeCode { + t.Errorf("code %d does not clear the retired commit-count scheme (%d) — "+ + "Android would refuse the upgrade as a downgrade", code, highestOldSchemeCode) + } + if int64(code) > 2147483647 { + t.Errorf("code %d overflows versionCode's int32 ceiling", code) + } +} + +// The tag is not chosen, it is the name with a `v`. Anything else reintroduces +// the mismatch between what a tag claims and what the artifact reports. +func TestTag_IsTheNameWithAPrefix(t *testing.T) { + got := runVersion(t, "1757443736", "1789000920") + if want := "v" + got["name"]; got["tag"] != want { + t.Errorf("tag = %q, want %q", got["tag"], want) + } +} + +// A guard that cannot fail is worse than no guard. These prove the script +// rejects the shapes it claims to reject, rather than emitting something +// plausible and letting it ship. +func TestVersionScript_RejectsUnusableClocks(t *testing.T) { + for _, tc := range []struct{ name, commit, now string }{ + {"unreadable commit timestamp", "notanumber", "1789000920"}, + {"non-numeric build clock", "1789000920", "abc"}, + {"build clock before 2020", "1789000920", "1000000000"}, + } { + t.Run(tc.name, func(t *testing.T) { + out, err := versionScript(t, tc.commit, tc.now) + if err == nil { + t.Errorf("script succeeded on %s, output: %s", tc.name, out) + } + }) + } +} + +// Pins the wiring, not the formula: if release.yml stops calling the script, +// every assertion above keeps passing while the thing that actually ships goes +// unguarded again. That silent decoupling is the specific regression here. +func TestReleaseWorkflow_UsesTheSharedDerivation(t *testing.T) { + body, err := os.ReadFile(filepath.Join(repoRoot(t), ".gitea", "workflows", "release.yml")) + if err != nil { + t.Fatal(err) + } + yaml := string(body) + + if !strings.Contains(yaml, "ci/version.sh") { + t.Error("release.yml no longer calls ci/version.sh — the derivation has drifted out of test coverage") + } + // The retired scheme, which must not come back. A presence check is safe + // against prose; the historical note in that file names the old formula + // only in comments, so match the executable form. + if strings.Contains(yaml, "$(git rev-list --count") { + t.Error("release.yml derives a commit count again — that is not monotonic across branches") + } +}