guard: an empty channel killed the lane instead of passing it

The first merge to `main` took the Android lane down (run 4857): the decide
job exited 1 in 0.16 seconds with no output at all, and the image build
skipped behind it because a failing lane must not publish.

`stable` had never published an APK, which the guard treats as a pass — there
is nothing to go backwards from, and `[ -z "$published" ]` says so in a branch
of its own. That branch was unreachable. `published="$(published_for ...)"`
under `set -e` dies on the substitution before it, and everything the pipeline
would have printed goes into the capture rather than the log.

What decided which lookups had the bug is the last command in the pipeline.
`sed` on empty input exits 0; `grep` exits 1. Three of the four end in `sed`.
Android's version_code ends in `grep -oE '[0-9]+$'`, so it was the only one —
and only on a channel with nothing on it, which is why a week of dev pushes
never saw it.

The tests now reach the half of the guard that talks to a feed, with `curl`
shadowed on PATH so they stay hermetic: an empty channel passes and builds, a
lower published version passes, a higher one fails the lane, and an equal
Android code is refused because Android will not install it.

packaging/guard-forward.sh

@@ -79,19 +79,35 @@ fetch() {
 # What the channel is serving, per artifact. ONE definition of where to look, shared
 # with `should-build.sh` — the skip decision and the guard must agree about what is
 # published, and two readers of one fact is how this repo keeps producing #2181-2183.
+#
+# EVERY LOOKUP HERE MUST SUCCEED EVEN WHEN IT FINDS NOTHING. That is what the `|| true`
+# on each pipeline is for, and it is load-bearing rather than defensive noise.
+#
+# An empty channel is a REAL state this guard is written to pass — `[ -z "$published" ]`
+# further down says so in as many words. But the value is captured as
+# `published="$(published_for ...)"`, and under `set -e` a command substitution that
+# exits non-zero kills the script before that branch is ever reached. Silently, too:
+# everything the pipeline would have said went into the capture rather than the log.
+#
+# WHICH COMMAND THE PIPELINE HAPPENS TO END ON decides whether that fires, which is the
+# part worth remembering. `sed` on empty input exits 0; `grep` exits 1. Three of these
+# four lookups end in `sed` and were fine. The one that ends in `grep -oE '[0-9]+$'` —
+# Android's version_code — was not, and it failed the whole Android lane on the first
+# merge to `main` (run 4857): exit 1, no output, 0.16 seconds, on the one channel that
+# had no APK published yet. Its three neighbours hid it until then.
 published_for() {
   case "$1" in
     desktop)
       # What the UPDATER reads. The manifest is the thing that decides whether a
       # client is offered a build, so it is the authority on what is published.
-      fetch "$SERVER/$REPO/releases/download/$2/latest.json" \
+      { fetch "$SERVER/$REPO/releases/download/$2/latest.json" \
         | grep -oE '"version"[[:space:]]*:[[:space:]]*"[^"]+"' | head -1 \
-        | sed -E 's/.*"([^"]+)"$/\1/'
+        | sed -E 's/.*"([^"]+)"$/\1/'; } || true
       ;;
     android)
-      fetch "$SERVER/$REPO/releases/download/$2/thoughtsync-android.json" \
+      { fetch "$SERVER/$REPO/releases/download/$2/thoughtsync-android.json" \
         | grep -oE '"version_code"[[:space:]]*:[[:space:]]*[0-9]+' | head -1 \
-        | grep -oE '[0-9]+$'
+        | grep -oE '[0-9]+$'; } || true
       ;;
   esac
 }
@@ -104,9 +120,9 @@ published_name() {
   case "$1" in
     desktop) published_for desktop "$2" ;;
     android)
-      fetch "$SERVER/$REPO/releases/download/$2/thoughtsync-android.json" \
+      { fetch "$SERVER/$REPO/releases/download/$2/thoughtsync-android.json" \
         | grep -oE '"version_name"[[:space:]]*:[[:space:]]*"[^"]+"' | head -1 \
-        | sed -E 's/.*"([^"]+)"$/\1/'
+        | sed -E 's/.*"([^"]+)"$/\1/'; } || true
       ;;
   esac
 }

tests/test_versioning.py

@@ -24,6 +24,7 @@ import pytest
 
 SCRIPT = Path(__file__).resolve().parent.parent / "packaging" / "version.sh"
 GUARD = Path(__file__).resolve().parent.parent / "packaging" / "guard-forward.sh"
+SHOULD_BUILD = Path(__file__).resolve().parent.parent / "packaging" / "should-build.sh"
 
 # 2020-01-01T00:00:00Z, the counter epoch. Duplicated from the script deliberately:
 # a test that imported the value could not catch the value being changed, and moving
@@ -304,3 +305,113 @@ def test_the_guard_rejects_bad_invocations(args: list[str]) -> None:
     that answers confidently for input it does not understand is worse than none."""
     r = subprocess.run(["sh", str(GUARD), *args], capture_output=True, text=True)
     assert r.returncode != 0
+
+
+# --- the backwards guard against a channel ------------------------------------
+#
+# The half of the guard that talks to a release feed, which the `compare` tests above
+# deliberately do not reach. Hermetic anyway: `curl` is shadowed on PATH by a stub, so
+# these assert what the guard does with an answer rather than what the forge returns.
+#
+# WHY THIS SECTION EXISTS AT ALL. The empty-channel case — a channel that has never
+# published this artifact — is one the guard is written to PASS, and it says so in a
+# branch of its own. It did not: `published="$(published_for ...)"` under `set -e`
+# died on the substitution before that branch could run, because Android's lookup
+# ended in a `grep` that exits 1 on no match while its three siblings ended in a `sed`
+# that exits 0. Silent, because everything the pipeline printed went into the capture.
+# It took the Android lane down on the first merge to `main` (run 4857) and nothing
+# here would have caught it.
+
+
+@pytest.fixture
+def curl_stub(tmp_path: Path):
+    """A `curl` on PATH that serves whatever the test says, or 404s.
+
+    Returns a callable: `serve(None)` for a channel with nothing published (the stub
+    exits 22, as real curl does under `-f` on an HTTP error), `serve(body)` to hand
+    back a feed.
+    """
+    bindir = tmp_path / "stubbin"
+    bindir.mkdir()
+    body = bindir / "body"
+    stub = bindir / "curl"
+    stub.write_text(
+        "#!/bin/sh\n"
+        f'[ -f "{body}" ] || exit 22\n'
+        f'cat "{body}"\n'
+    )
+    stub.chmod(0o755)
+
+    def serve(content: str | None) -> dict[str, str]:
+        if content is None:
+            body.unlink(missing_ok=True)
+        else:
+            body.write_text(content)
+        return {**os.environ, "PATH": f"{bindir}{os.pathsep}{os.environ['PATH']}"}
+
+    return serve
+
+
+def guard(repo: Path, env: dict[str, str], *args: str) -> subprocess.CompletedProcess:
+    return subprocess.run(["sh", str(GUARD), *args],
+                          cwd=repo, env=env, capture_output=True, text=True)
+
+
+@pytest.mark.parametrize("artifact", ["desktop", "android"])
+def test_an_empty_channel_passes_the_guard(repo: Path, curl_stub, artifact: str) -> None:
+    """Nothing published yet is not a backwards move — it is the first publish.
+
+    Failing here would block the very first build to reach a channel, which is exactly
+    what happened to Android on `stable`.
+    """
+    r = guard(repo, curl_stub(None), artifact, "stable")
+    assert r.returncode == 0, f"exit={r.returncode} stdout={r.stdout!r} stderr={r.stderr!r}"
+    assert "nothing to compare" in r.stdout
+
+
+@pytest.mark.parametrize("artifact", ["desktop", "android"])
+def test_an_empty_channel_reports_no_published_version(
+    repo: Path, curl_stub, artifact: str
+) -> None:
+    """`published` answers empty rather than failing — `should-build.sh` captures it
+    the same way the guard does, so a non-zero exit strands that caller too."""
+    r = guard(repo, curl_stub(None), "published", artifact, "stable")
+    assert r.returncode == 0, f"exit={r.returncode} stderr={r.stderr!r}"
+    assert r.stdout.strip() == ""
+
+
+def test_an_empty_channel_builds(repo: Path, curl_stub) -> None:
+    r = subprocess.run(["sh", str(SHOULD_BUILD), "android", "stable"],
+                       cwd=repo, env=curl_stub(None), capture_output=True, text=True)
+    assert r.returncode == 0, f"exit={r.returncode} stderr={r.stderr!r}"
+    assert r.stdout.strip() == "true"
+
+
+def test_a_lower_published_version_passes(repo: Path, curl_stub) -> None:
+    """The transition this milestone performs: `stable` served the bare 0.2.0 from
+    Cargo.toml, and the new key has to clear it."""
+    env = curl_stub('{"version": "0.2.0", "platforms": {}}')
+    r = guard(repo, env, "desktop", "stable")
+    assert r.returncode == 0, f"stderr={r.stderr!r}"
+    assert "may be published" in r.stdout
+
+
+def test_a_higher_published_version_fails_the_lane(repo: Path, curl_stub) -> None:
+    """The unrecoverable direction. A version below what is published leaves every
+    installed client reporting 'up to date' forever, so this fails rather than warns."""
+    env = curl_stub('{"version": "9.9.9", "platforms": {}}')
+    r = guard(repo, env, "desktop", "stable")
+    assert r.returncode != 0
+    assert "GUARD FAILED" in r.stderr
+
+
+def test_an_equal_android_code_fails_because_android_will_not_install_it(
+    repo: Path, curl_stub
+) -> None:
+    """Android hard-fails a non-rising versionCode, so equality is refused there —
+    unlike the desktop, where an unchanged source deriving its own value is ordinary."""
+    code = version(repo, "key", "android")
+    env = curl_stub(f'{{"version_code": {code}, "version_name": "x"}}')
+    r = guard(repo, env, "android", "stable")
+    assert r.returncode != 0
+    assert "EQUALS" in r.stderr