diff --git a/.gitea/workflows/release.yml b/.gitea/workflows/release.yml index 4c7f7f88..bb0ac992 100644 --- a/.gitea/workflows/release.yml +++ b/.gitea/workflows/release.yml @@ -421,7 +421,10 @@ jobs: # the sidecar published beside it, so what the server reports is what # that build actually recorded rather than something re-derived here. # Degrades to an empty client/ (404 update channel) — never a wrong - # version — if no release or APK asset can be resolved. + # version — if no release or APK asset can be resolved. That + # degradation only actually works because the greps below carry + # `|| true`; under the runner's default pipefail a non-matching grep + # kills the step instead of falling through to the empty-case branch. if: steps.guard.outputs.ready == 'true' && github.ref == 'refs/heads/main' shell: bash env: @@ -434,8 +437,15 @@ jobs: if [ -z "${REL_JSON}" ]; then echo "::notice::no published release — image ships without bundled APK"; exit 0 fi - TAG="$(printf '%s' "${REL_JSON}" | grep -oP '"tag_name":\s*"\K[^"]+' | head -1)" - APK_URL="$(printf '%s' "${REL_JSON}" | grep -oP '"browser_download_url":\s*"\K[^"]+' | grep -E '\.apk$' | head -1)" + # `|| true` on every one of these, and it is load-bearing rather + # than defensive habit. The runner already invokes this shell as + # `bash -e -o pipefail`, so a pipeline whose grep matches NOTHING + # exits non-zero even though `head` succeeded — and the step dies at + # the assignment, before ever reaching the `if` written to handle the + # empty case. Every "degrades gracefully" branch below is unreachable + # without this. + TAG="$(printf '%s' "${REL_JSON}" | grep -oP '"tag_name":\s*"\K[^"]+' | head -1)" || true + APK_URL="$(printf '%s' "${REL_JSON}" | grep -oP '"browser_download_url":\s*"\K[^"]+' | grep -E '\.apk$' | head -1)" || true if [ -z "${TAG}" ] || [ -z "${APK_URL}" ]; then echo "::notice::latest release '${TAG:-?}' has no APK asset — image ships without bundled APK"; exit 0 fi @@ -446,7 +456,7 @@ jobs: # 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)" + SIDECAR_URL="$(printf '%s' "${REL_JSON}" | grep -oP '"browser_download_url":\s*"\K[^"]+' | grep -E '\.apk\.version$' | head -1)" || true if [ -n "${SIDECAR_URL}" ]; then curl -fsSL -H "Authorization: token ${CI_TOKEN}" -o client/minstrel.apk.version "${SIDECAR_URL}" cat client/minstrel.apk.version diff --git a/internal/server/release_version_test.go b/internal/server/release_version_test.go index 892103ff..4dbf2bef 100644 --- a/internal/server/release_version_test.go +++ b/internal/server/release_version_test.go @@ -240,3 +240,49 @@ func TestDevChannel_RebundlePathIsMainOnly(t *testing.T) { t.Errorf("the rebundle step is not gated to main; a dev push would overwrite its own APK\n%s", step) } } + +// The runner invokes `shell: bash` as `bash -e -o pipefail`. That makes any +// command substitution whose grep matches NOTHING fatal at the assignment — +// pipefail reports the grep's non-zero status even though `head` succeeded — +// so the step dies before reaching the `if` written to handle the empty case. +// +// This is not hypothetical. It took down the first `main` build after the +// version rework: the newest release predated sidecar assets, its +// `.apk.version` grep matched nothing, and the step aborted instead of +// falling through to the name-only branch that exists precisely for it. The +// same latent hazard sat on the other two assignments and had simply never +// fired, because their greps always matched. +// +// Pins the property that makes every "degrades gracefully" branch in that +// step reachable at all. +func TestBundleStep_GrepsCannotKillTheStep(t *testing.T) { + body, err := os.ReadFile(filepath.Join(repoRoot(t), ".gitea", "workflows", "release.yml")) + if err != nil { + t.Fatal(err) + } + yaml := string(body) + + i := strings.Index(yaml, "- name: Bundle latest release APK") + if i < 0 { + t.Fatal("no 'Bundle latest release APK' step") + } + step := yaml[i:] + if j := strings.Index(step, "\n - name:"); j >= 0 { + step = step[:j] + } + + for _, line := range strings.Split(step, "\n") { + trimmed := strings.TrimSpace(line) + if strings.HasPrefix(trimmed, "#") || !strings.Contains(trimmed, "grep") { + continue + } + // Only assignments from a command substitution can abort the step. + if !strings.Contains(trimmed, `="$(`) { + continue + } + if !strings.HasSuffix(trimmed, "|| true") { + t.Errorf("this assignment dies under pipefail when its grep matches nothing, "+ + "skipping the empty-case branch below it:\n %s", trimmed) + } + } +}