feat(version): sidecar and /api/client/version carry name, code and channel
test-go / test (push) Successful in 1m25s
test-go / integration (push) Successful in 5m52s

The client compares names while Android installs by versionCode, and the
wire had no way to close that gap: the sidecar was one positional line and
the endpoint returned a name only. This is the plumbing that makes the
ordering key decidable by the client at all.

The sidecar is now JSON rather than a grown positional string. That shape
was chosen against a specific failure: the obvious growth path was
"<name> <code>", which a first-space split silently mangles the moment a
third field appears — the code stops parsing as an integer and the reader
falls back to name comparison WITHOUT erroring. JSON cannot mistake a new
field for an old one.

code is a POINTER on both sides, and omitempty on the wire. Absent has to
stay distinguishable from zero: a build published before ordering keys were
recorded genuinely has no code, and zero would claim it is infinitely old
rather than unknown.

A malformed sidecar now fails loudly instead of serving a blank version.
If an unreadable file produced an empty name, every client would compare
against nothing, conclude it was current, and go quiet — "I cannot read
this" and "there is nothing newer" would return the same answer, which is
the failure mode nobody reports because nobody is offered anything to
report.

The non-tag :latest path no longer RECONSTRUCTS the bundled APK's version.
android-release now publishes the sidecar as a release asset beside the
APK, and the image build downloads it. The old reconstruction duplicated a
derivation formula across two files, and could only ever recover the name —
the ordering key is build-time minutes and exists nowhere once that build
ends. Releases predating the sidecar report their name with a null code,
which is the honest answer rather than a guessed one.

image-release also drops to a shallow checkout: it needed full history and
tags only to re-derive versions from the tagged commit, and now touches git
for nothing. MINSTREL_VERSION comes from GITHUB_REF.

Two things checked rather than assumed. The Android Json sets
ignoreUnknownKeys, so the added fields cannot break already-installed apps.
It also sets coerceInputValues, which will silently turn a null code into 0
if step 4 declares the field non-nullable — recorded on task #3811, because
reading the field declaration alone would never reveal it.

Also fixes a stale comment block describing "the Flutter client", deleted
in v2026.08.18.

Step 3 of 5 — Scribe task #3810, milestone #390.

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-09 21:51:41 -04:00
co-authored by Claude Opus 5
parent e46c6bcccf
commit 9f3e0b8cd3
3 changed files with 198 additions and 48 deletions
+69 -35
View File
@@ -39,10 +39,9 @@ name: release
# :latest (not just tags), a main build with no APK would silently strip # :latest (not just tags), a main build with no APK would silently strip
# the in-app update channel off :latest until the next release. So on # the in-app update channel off :latest until the next release. So on
# non-tag builds image-release pulls the MOST RECENT release's signed APK # non-tag builds image-release pulls the MOST RECENT release's signed APK
# and reconstructs its exact versionName from the tagged commit's timestamp # AND the version sidecar published beside it — the recorded values, not
# (the same derivation android-release bakes in) for the version sidecar — # recomputed ones — so no rebuild is needed, just a rebundle. Tag builds
# no rebuild, just rebundle. Tag builds keep bundling their own # keep bundling their own freshly-built APK.
# freshly-built APK.
# #
# Android testing (lint + detekt + unit tests, debug APK upload on main) # Android testing (lint + detekt + unit tests, debug APK upload on main)
# lives in android.yml and runs independently on every push. # lives in android.yml and runs independently on every push.
@@ -227,6 +226,8 @@ jobs:
shell: bash shell: bash
env: env:
CI_TOKEN: ${{ secrets.CI_TOKEN }} CI_TOKEN: ${{ secrets.CI_TOKEN }}
VERSION_NAME: ${{ steps.ver.outputs.name }}
VERSION_CODE: ${{ steps.ver.outputs.code }}
run: | run: |
set -euxo pipefail set -euxo pipefail
TAG="${GITHUB_REF#refs/tags/}" TAG="${GITHUB_REF#refs/tags/}"
@@ -234,6 +235,20 @@ jobs:
APK_PATH="app/build/outputs/apk/release/app-release.apk" APK_PATH="app/build/outputs/apk/release/app-release.apk"
ls -lh "${APK_PATH}" ls -lh "${APK_PATH}"
# Publish the version sidecar as a release asset next to the APK.
#
# This is what lets a later :latest build stop RECONSTRUCTING the
# bundled APK's version and simply read what was recorded. The
# ordering key in particular cannot be re-derived after the fact —
# it is build-time minutes, so once this job ends the value exists
# nowhere else. Reconstruction could only ever recover the name,
# and only by duplicating a formula that then has to be kept in
# step across two files.
SIDECAR_PATH="/tmp/minstrel.apk.version"
printf '{"name":"%s","code":%s,"channel":"stable"}\n' \
"${VERSION_NAME}" "${VERSION_CODE}" > "${SIDECAR_PATH}"
cat "${SIDECAR_PATH}"
RELEASE_JSON="$(curl -fsSL \ RELEASE_JSON="$(curl -fsSL \
-H "Authorization: token ${CI_TOKEN}" \ -H "Authorization: token ${CI_TOKEN}" \
"https://git.fabledsword.com/api/v1/repos/${REPO}/releases/tags/${TAG}")" "https://git.fabledsword.com/api/v1/repos/${REPO}/releases/tags/${TAG}")"
@@ -255,6 +270,20 @@ jobs:
exit 1 exit 1
fi fi
# Same treatment for the sidecar. Named `.apk.version` so the
# downloader's `\.apk$` match cannot pick it up by mistake.
SIDECAR_HTTP=$(curl -sS -L -o /tmp/upload-sidecar.out -w '%{http_code}' \
-H "Authorization: token ${CI_TOKEN}" \
-F "attachment=@${SIDECAR_PATH}" \
"https://git.fabledsword.com/api/v1/repos/${REPO}/releases/${RELEASE_ID}/assets?name=minstrel-${TAG}.apk.version")
echo "sidecar_upload_http=${SIDECAR_HTTP}"
cat /tmp/upload-sidecar.out || true
echo
if [ "${SIDECAR_HTTP}" -lt 200 ] || [ "${SIDECAR_HTTP}" -ge 300 ]; then
echo "::error::version sidecar upload returned HTTP ${SIDECAR_HTTP}"
exit 1
fi
image-release: image-release:
name: Build + push container image name: Build + push container image
# `needs:` waits for android-release. For tag pushes android-release # `needs:` waits for android-release. For tag pushes android-release
@@ -275,12 +304,11 @@ jobs:
- name: Checkout - name: Checkout
uses: actions/checkout@v4 uses: actions/checkout@v4
with: with:
# Full history + tags so non-tag :latest builds can resolve the # Shallow is fine here. This job used to need full history + tags to
# latest release tag's commit and reconstruct the bundled APK's # re-derive the bundled APK's version from the tagged commit; it now
# exact versionName from its timestamp (see "Bundle latest release # downloads the sidecar the release recorded, and touches git for
# APK" below). # nothing. MINSTREL_VERSION comes from GITHUB_REF, not from git.
fetch-depth: 0 fetch-depth: 1
fetch-tags: true
- name: Detect buildable project - name: Detect buildable project
id: guard id: guard
@@ -346,29 +374,30 @@ jobs:
if: steps.guard.outputs.ready == 'true' && startsWith(github.ref, 'refs/tags/v') if: steps.guard.outputs.ready == 'true' && startsWith(github.ref, 'refs/tags/v')
shell: bash shell: bash
env: env:
# Pulled from android-release.outputs.version_name so the # Both pulled from android-release's outputs so the sidecar the
# sidecar string the server hands clients matches the # server hands clients matches exactly what is baked into the APK
# versionName baked into the APK they're comparing against. # they are comparing against.
APK_VERSION_NAME: ${{ needs.android-release.outputs.version_name }} APK_VERSION_NAME: ${{ needs.android-release.outputs.version_name }}
APK_VERSION_CODE: ${{ needs.android-release.outputs.version_code }}
run: | run: |
set -euxo pipefail set -euxo pipefail
# The artifact lands as `app-release.apk` (the original Gradle # The artifact lands as `app-release.apk` (the original Gradle
# output name). The Dockerfile COPYs client/* into /app/client/ # output name). The Dockerfile COPYs client/* into /app/client/
# and the server reads minstrel.apk + minstrel.apk.version. # and the server reads minstrel.apk + minstrel.apk.version.
mv client/app-release.apk client/minstrel.apk mv client/app-release.apk client/minstrel.apk
echo "${APK_VERSION_NAME}" > client/minstrel.apk.version printf '{"name":"%s","code":%s,"channel":"stable"}\n' \
"${APK_VERSION_NAME}" "${APK_VERSION_CODE}" > client/minstrel.apk.version
cat client/minstrel.apk.version
ls -lh client/ ls -lh client/
- name: Bundle latest release APK (non-tag :latest builds) - name: Bundle latest release APK (non-tag :latest builds)
# Main pushes don't build an APK, but they DO move :latest — so # Main pushes don't build an APK, but they DO move :latest — so
# without this the in-app update channel would vanish from :latest # without this the in-app update channel would vanish from :latest
# until the next tag. Pull the most-recent release's signed APK and # until the next tag. Pull the most-recent release's signed APK and
# reconstruct its exact versionName from the tagged commit's # the sidecar published beside it, so what the server reports is what
# timestamp — the same derivation android-release uses — so the # that build actually recorded rather than something re-derived here.
# version sidecar the server hands clients matches the installed
# build.
# Degrades to an empty client/ (404 update channel) — never a wrong # Degrades to an empty client/ (404 update channel) — never a wrong
# version — if no release / APK asset / tag-count can be resolved. # version — if no release or APK asset can be resolved.
if: steps.guard.outputs.ready == 'true' && !startsWith(github.ref, 'refs/tags/v') if: steps.guard.outputs.ready == 'true' && !startsWith(github.ref, 'refs/tags/v')
shell: bash shell: bash
env: env:
@@ -386,23 +415,28 @@ jobs:
if [ -z "${TAG}" ] || [ -z "${APK_URL}" ]; then if [ -z "${TAG}" ] || [ -z "${APK_URL}" ]; then
echo "::notice::latest release '${TAG:-?}' has no APK asset — image ships without bundled APK"; exit 0 echo "::notice::latest release '${TAG:-?}' has no APK asset — image ships without bundled APK"; exit 0
fi fi
# Reconstruct the bundled APK's name with the SAME derivation
# android-release uses — commit timestamp of the tagged commit. The
# two must agree exactly: this string is what the server hands
# clients to compare against what is installed, so a mismatch here
# is an update offered forever or never offered at all.
#
# This duplication is temporary. Once the tag itself becomes
# `v<version-name>`, this whole block collapses to `${TAG#v}` with
# nothing to recompute and nothing to keep in step.
COMMIT_TS="$(git log --format=%ct -1 "${TAG}" 2>/dev/null || true)"
if [ -z "${COMMIT_TS}" ]; then
echo "::notice::could not resolve commit timestamp for ${TAG} (tag not fetched?) — skipping APK bundle"; exit 0
fi
VERSION_NAME="$(date -u -d "@${COMMIT_TS}" +%Y.%m.%d.%H%M)"
curl -fsSL -H "Authorization: token ${CI_TOKEN}" -o client/minstrel.apk "${APK_URL}" curl -fsSL -H "Authorization: token ${CI_TOKEN}" -o client/minstrel.apk "${APK_URL}"
echo "${VERSION_NAME}" > client/minstrel.apk.version
echo "::notice::bundled release APK ${TAG} as version ${VERSION_NAME}" # Take the version the release RECORDED rather than recomputing it.
# This used to re-derive the name from the tagged commit, which meant
# the formula lived in two files that had to be kept in step, and it
# could only ever recover the name — the ordering key is build-time
# minutes and does not exist anywhere after that build ends.
SIDECAR_URL="$(printf '%s' "${REL_JSON}" | grep -oP '"browser_download_url":\s*"\K[^"]+' | grep -E '\.apk\.version$' | head -1)"
if [ -n "${SIDECAR_URL}" ]; then
curl -fsSL -H "Authorization: token ${CI_TOKEN}" -o client/minstrel.apk.version "${SIDECAR_URL}"
cat client/minstrel.apk.version
else
# Releases published before sidecars were attached. Their name is
# still recoverable from the tag, but their ordering key genuinely
# is not — so it is reported ABSENT rather than guessed. A wrong
# key is an install the platform refuses; an absent one just tells
# the client to fall back to comparing names, which is exactly
# what those builds already do.
echo "::notice::release ${TAG} predates the version sidecar — bundling with name only, no ordering key"
printf '{"name":"%s","code":null,"channel":"stable"}\n' "${TAG#v}" > client/minstrel.apk.version
fi
echo "::notice::bundled release APK from ${TAG}"
ls -lh client/ ls -lh client/
- name: Build and push - name: Build and push
+53 -6
View File
@@ -6,19 +6,21 @@ package api
// /app/client/ at image build time. // /app/client/ at image build time.
// //
// Both endpoints are authenticated — the bandwidth cost of the APK // Both endpoints are authenticated — the bandwidth cost of the APK
// (~30-60 MB) makes anonymous access an abuse vector. The Flutter // (~30-60 MB) makes anonymous access an abuse vector. The client only
// client's polling only fires after login (banner mounts in the post- // polls after login, so this gate is invisible to the actual update flow.
// login shell), so this gate is invisible to the actual update flow.
// //
// /api/client/apk additionally rate-limits per user to a single // /api/client/apk additionally rate-limits per user to a single
// download every 60s. Real install flows fire one download per // download every 60s. Real install flows fire one download per
// update; anything tighter is scripted/abusive. // update; anything tighter is scripted/abusive.
// //
// Returns 404 gracefully when the APK isn't present (dev environments, // Returns 404 gracefully when the APK isn't present (dev environments,
// pre-CI-wiring); the Flutter client treats 404 as "no update channel // pre-CI-wiring); the client treats 404 as "no update channel available."
// available." //
// (These paragraphs said "the Flutter client" until 2026-09-10. That client
// was deleted in v2026.08.18 — the Android app is the only one now.)
import ( import (
"encoding/json"
"errors" "errors"
"net/http" "net/http"
"os" "os"
@@ -84,8 +86,36 @@ func clientAPKAllowDownload(userID string, now time.Time) time.Duration {
return 0 return 0
} }
// clientVersionSidecar is the JSON written beside the bundled APK by
// release.yml. It carries three values that are deliberately separate:
//
// - Name is a LABEL for people, "YYYY.MM.DD.HHMM" from the commit's
// timestamp. Two channels carrying the same code report the same name.
// - Code is the ORDERING KEY, minutes since 2020-01-01 at build time, and
// is the value Android itself installs by. It answers "may this be
// installed over that?" — the name never does.
// - Channel is a SIBLING FIELD, never a suffix inside the name.
//
// JSON rather than a positional line on purpose. The obvious growth path for
// the old one-value file was "<name> <code>", which a first-space split
// silently mangles the moment a third field appears: the code stops parsing,
// and the reader falls back to name comparison WITHOUT erroring.
type clientVersionSidecar struct {
Name string `json:"name"`
// Pointer, not int64: absent must stay distinguishable from zero. An
// artifact published before codes were recorded genuinely has no code —
// zero would claim it is infinitely old rather than unknown.
Code *int64 `json:"code"`
Channel string `json:"channel"`
}
type clientVersionResponse struct { type clientVersionResponse struct {
Version string `json:"version"` Version string `json:"version"`
// omitempty on both: the client must be able to tell "this server does
// not report a code" from "this build's code is 0", because those call
// for different behaviour on the other end.
Code *int64 `json:"code,omitempty"`
Channel string `json:"channel,omitempty"`
APKURL string `json:"apk_url"` APKURL string `json:"apk_url"`
SizeBytes int64 `json:"size_bytes"` SizeBytes int64 `json:"size_bytes"`
} }
@@ -117,8 +147,25 @@ func (h *handlers) handleClientVersion(w http.ResponseWriter, _ *http.Request) {
return return
} }
var sidecar clientVersionSidecar
if err := json.Unmarshal(versionBytes, &sidecar); err != nil {
// Fail LOUDLY rather than serving a blank version. The failure mode
// this avoids is the one that never gets reported: if an unreadable
// sidecar produced an empty name, every client would compare against
// nothing, conclude it was current, and go quiet — "I cannot read
// this" and "there is nothing newer" would be the same answer.
writeErrWithLog(w, h.logger, "client_version: sidecar is not valid JSON", err)
return
}
if sidecar.Name == "" {
http.Error(w, `{"error":{"code":"bad_client_version","message":"version sidecar has no name"}}`, http.StatusInternalServerError)
return
}
writeJSON(w, http.StatusOK, clientVersionResponse{ writeJSON(w, http.StatusOK, clientVersionResponse{
Version: strings.TrimSpace(string(versionBytes)), Version: strings.TrimSpace(sidecar.Name),
Code: sidecar.Code,
Channel: strings.TrimSpace(sidecar.Channel),
APKURL: "/api/client/apk", APKURL: "/api/client/apk",
SizeBytes: stat.Size(), SizeBytes: stat.Size(),
}) })
+75 -6
View File
@@ -77,18 +77,32 @@ func TestClientVersion_404WhenAPKButNoVersion(t *testing.T) {
} }
} }
func TestClientVersion_200WithBothFiles(t *testing.T) { // writeClientAssets stages an APK plus a raw sidecar body, and returns the
// APK's size so callers can assert size_bytes without recomputing it.
func writeClientAssets(t *testing.T, sidecar string) int64 {
t.Helper()
dir := withClientAPKDir(t) dir := withClientAPKDir(t)
body := []byte("fake apk content") body := []byte("fake apk content")
if err := os.WriteFile(filepath.Join(dir, clientAPKFilename), body, 0o644); err != nil { if err := os.WriteFile(filepath.Join(dir, clientAPKFilename), body, 0o644); err != nil {
t.Fatal(err) t.Fatal(err)
} }
if err := os.WriteFile(filepath.Join(dir, clientVersionFile), []byte("v2026.05.10\n"), 0o644); err != nil { if err := os.WriteFile(filepath.Join(dir, clientVersionFile), []byte(sidecar), 0o644); err != nil {
t.Fatal(err) t.Fatal(err)
} }
return int64(len(body))
}
func getClientVersion(t *testing.T) *httptest.ResponseRecorder {
t.Helper()
h := &handlers{logger: slog.New(slog.NewTextHandler(io.Discard, nil))} h := &handlers{logger: slog.New(slog.NewTextHandler(io.Discard, nil))}
rr := httptest.NewRecorder() rr := httptest.NewRecorder()
h.handleClientVersion(rr, httptest.NewRequest(http.MethodGet, "/api/client/version", nil)) h.handleClientVersion(rr, httptest.NewRequest(http.MethodGet, "/api/client/version", nil))
return rr
}
func TestClientVersion_200WithBothFiles(t *testing.T) {
size := writeClientAssets(t, `{"name":"2026.09.10.1432","code":3523847,"channel":"stable"}`+"\n")
rr := getClientVersion(t)
if rr.Code != http.StatusOK { if rr.Code != http.StatusOK {
t.Fatalf("want 200, got %d (body: %s)", rr.Code, rr.Body.String()) t.Fatalf("want 200, got %d (body: %s)", rr.Code, rr.Body.String())
} }
@@ -96,14 +110,69 @@ func TestClientVersion_200WithBothFiles(t *testing.T) {
if err := json.Unmarshal(rr.Body.Bytes(), &resp); err != nil { if err := json.Unmarshal(rr.Body.Bytes(), &resp); err != nil {
t.Fatal(err) t.Fatal(err)
} }
if resp.Version != "v2026.05.10" { if resp.Version != "2026.09.10.1432" {
t.Errorf("version: want trimmed v2026.05.10, got %q", resp.Version) t.Errorf("version: want 2026.09.10.1432, got %q", resp.Version)
}
if resp.Code == nil {
t.Fatal("code: want 3523847, got absent — the client decides on this, so absent means it silently falls back to name comparison")
}
if *resp.Code != 3523847 {
t.Errorf("code: want 3523847, got %d", *resp.Code)
}
if resp.Channel != "stable" {
t.Errorf("channel: want stable, got %q", resp.Channel)
} }
if resp.APKURL != "/api/client/apk" { if resp.APKURL != "/api/client/apk" {
t.Errorf("apk_url: want /api/client/apk, got %q", resp.APKURL) t.Errorf("apk_url: want /api/client/apk, got %q", resp.APKURL)
} }
if resp.SizeBytes != int64(len(body)) { if resp.SizeBytes != size {
t.Errorf("size_bytes: want %d, got %d", len(body), resp.SizeBytes) t.Errorf("size_bytes: want %d, got %d", size, resp.SizeBytes)
}
}
// A release published before ordering keys were recorded has a name and
// genuinely no code. That must arrive as ABSENT, not as 0 — zero would claim
// the build is infinitely old and offer an update to everyone forever.
func TestClientVersion_CodeAbsentIsOmittedNotZero(t *testing.T) {
writeClientAssets(t, `{"name":"2026.09.09","code":null,"channel":"stable"}`)
rr := getClientVersion(t)
if rr.Code != http.StatusOK {
t.Fatalf("want 200, got %d (body: %s)", rr.Code, rr.Body.String())
}
var resp clientVersionResponse
if err := json.Unmarshal(rr.Body.Bytes(), &resp); err != nil {
t.Fatal(err)
}
if resp.Code != nil {
t.Errorf("code: want absent, got %d", *resp.Code)
}
// The wire must omit the key entirely, so a client can distinguish
// "this server reports no code" from "this build's code is 0".
var raw map[string]any
if err := json.Unmarshal(rr.Body.Bytes(), &raw); err != nil {
t.Fatal(err)
}
if _, present := raw["code"]; present {
t.Errorf("code key should be omitted entirely, body was %s", rr.Body.String())
}
}
// The failure this guards is the one nobody reports: if an unreadable sidecar
// produced an empty version, every client would compare against nothing,
// decide it was current, and go quiet. "I cannot read this" and "there is
// nothing newer" must not be the same answer.
func TestClientVersion_MalformedSidecarErrorsRatherThanReportingNothing(t *testing.T) {
for _, sidecar := range []string{
"2026.09.10.1432", // the OLD plain-text format
`{"name":"x",`, // truncated JSON
`{"code":123,"channel":"dev"}`, // valid JSON, no name
"",
} {
writeClientAssets(t, sidecar)
rr := getClientVersion(t)
if rr.Code == http.StatusOK {
t.Errorf("sidecar %q: want an error status, got 200 with body %s", sidecar, rr.Body.String())
}
} }
} }