From 902e16cf92ec52122b021b59902dada4abb6bbc2 Mon Sep 17 00:00:00 2001 From: Paul Hitt Date: Sun, 6 Sep 2026 15:16:26 -0400 Subject: [PATCH 1/2] CI never ran the obfs4 suite, and replayed the others from a local cache MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two holes, found by reading the trace of the pipeline that had just merged android!78 rather than trusting its green. `:obfs4:test` was not in the task list. The module shipped in !77 with ~100 tests — the Elligator2 port, the framing, the ntor handshake, the socket factory — and not one of them has ever run in CI. A break anywhere in the transport would have merged green. The bridge-line parser added in !78 was in the same position. The rest ran, but not in this pipeline. The runner is tagged `mac-local`, so it is the developer's own machine and shares Gradle's build cache with local runs. Every test task came back FROM-CACHE — the whole job finished in 5 seconds — which means CI replayed results it never computed. The outcomes were real, they were just mine from ten minutes earlier. That is not independent verification, and nothing in the log distinguishes it: a cached pass and a real pass both print "BUILD SUCCESSFUL". `--rerun-tasks` now forces execution. And the job asserted nothing. It ended on gradle's exit status, which is the same shape of mistake the iOS gate was carrying when it ended in `grep ... || exit 1` and went green on a red suite. `:app` makes it worse by printing no per-test names at all, so a filter matching nothing looks exactly like 145 passing tests. So the gate reads the results XML: per-module floors at roughly 90% of today's counts, zero failures, and a total. Old results are deleted first — the runner reuses its build directory, and XML from a previous pipeline would otherwise be counted by the very check meant to notice a suite had stopped running. Hiding one module's results reddens it; that is how the floors were checked. 1162 tests across the four modules, 0 failures, 44 tasks executed and none from cache. ⚠️ The interop tests are opted out here and only here — they need a real lyrebird from the flamenet-elligator2 checkout, which is not in the build directory. They fail rather than skip when unconfigured, deliberately, so the opt-out is written down where it can be seen. Run `scripts/obfs4-interop.sh` before merging obfs4 changes: every other test in that module compares against recorded vectors or against itself, and two identically-wrong ends agree perfectly. Co-Authored-By: Claude Opus 5 --- .gitlab-ci.yml | 70 ++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 68 insertions(+), 2 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 7bd7309..f055931 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -1,4 +1,4 @@ -# T4: the Mac is in the merge path. Engine, client, and app unit suites on +# T4: the Mac is in the merge path. Engine, client, obfs4 and app unit suites on # the project's own runner (tag mac-local). Instrumented tests stay manual: # they need an emulator and a local relay. stages: [test] @@ -11,8 +11,74 @@ unit-tests: - if: '$CI_COMMIT_BRANCH == "main"' variables: GIT_DEPTH: "1" + # 🔴 The interop tests FAIL rather than skip when unconfigured, on purpose — + # a silent skip is how a transport suite goes green while proving nothing. + # They need a real lyrebird from the flamenet-elligator2 checkout, which is + # not in this build directory, so they are opted out HERE and nowhere else. + # Run `scripts/obfs4-interop.sh` locally before merging any obfs4 change: + # every other test in that module compares against recorded vectors or + # against itself, and two identically-wrong ends agree perfectly. + OBFS4_INTEROP_OPTOUT: "1" script: - export JAVA_HOME=/opt/homebrew/opt/openjdk@21 - export ANDROID_HOME=$HOME/Library/Android/sdk - - ./gradlew --no-daemon :engine:test :client:test :app:testDebugUnitTest + # 🔴 **Clear old results before running, or the gate counts ghosts.** The + # runner reuses its build directory, so XML from a previous pipeline + # survives — and a suite that stopped running entirely would still be + # counted, by the very check meant to notice that it had stopped. + - rm -rf ./*/build/test-results + # 🔴 **`:obfs4:test` was missing from this list.** The module shipped in !77 + # with ~100 tests — the Elligator2 port, the framing, the handshake, the + # socket factory — and not one of them ran in CI. A break in the transport + # would have merged green. + # + # 🔴 **`--rerun-tasks` because this runner is the developer's own Mac.** + # Gradle's build cache is shared with local runs, so a task whose inputs a + # local run already covered comes back FROM-CACHE and CI replays a result it + # never computed. That is not independent verification, and it is invisible: + # a cached pass and a real pass print the same "BUILD SUCCESSFUL". + - ./gradlew --no-daemon --rerun-tasks :engine:test :client:test :obfs4:test :app:testDebugUnitTest + - | + # 🔴 **Assert on the RESULTS, never on Gradle's exit status.** `:app` prints + # no per-test names at all, so a filter matching nothing and a full green + # run are indistinguishable in the log — and a skipped-as-up-to-date task + # is likewise just "BUILD SUCCESSFUL". The XML is the only thing that says + # what actually executed. + python3 - <<'PY' + import glob, re, sys + + # ⚠️ Floors, not exact counts. Set around 90% of the suite as it stands + # (539 / 403 / 75 / 145 on 2026-09-06) so a module that loses most of its + # tests to a bad filter is caught, without a gate that has to be edited + # every time somebody adds one. + MINIMUMS = {"engine": 480, "client": 360, "obfs4": 68, "app": 130} + totals = {} + for module in MINIMUMS: + tests = failures = skipped = 0 + for path in glob.glob(f"{module}/build/test-results/*/TEST-*.xml"): + text = open(path).read() + m = re.search(r'tests="(\d+)" skipped="(\d+)" failures="(\d+)" errors="(\d+)"', text) + if not m: + continue + tests += int(m.group(1)); skipped += int(m.group(2)) + failures += int(m.group(3)) + int(m.group(4)) + totals[module] = (tests, failures, skipped) + + bad = False + for module, (tests, failures, skipped) in totals.items(): + print(f"GATE: {module}: {tests} tests, {failures} failures, {skipped} skipped") + if tests < MINIMUMS[module]: + print(f"GATE: {module} reported {tests} tests — the suite did not run") + bad = True + if failures: + bad = True + # ⚠️ A floor across the whole build as well as per module: a module can be + # present and still have lost most of its suite to a bad filter. + grand = sum(t for t, _, _ in totals.values()) + print(f"GATE: {grand} tests in total") + if grand < 1050: + print(f"GATE: only {grand} tests in the whole build; something is not running") + bad = True + sys.exit(1 if bad else 0) + PY timeout: 30m From 934704d7843bbdd90dc2794145172db8334ca145 Mon Sep 17 00:00:00 2001 From: Paul Hitt Date: Sun, 6 Sep 2026 15:20:24 -0400 Subject: [PATCH 2/2] Fetch the protocol vectors in CI, which never had them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Turning off the build cache exposed twenty failures that had been there all along: AccountKdfTest, PQVectorTest and PublishedVectorsTest look for a sibling `flamenet-e2e` checkout. That exists on a developer's machine and has never existed in a build directory, so every one of them has been failing in CI since it was written — and CI reported green, because the cache was replaying passes computed here. These are the cross-implementation conformance tests. They are what says this engine derives the same keys as the one iPhone runs; twenty of them silently not running is the worst possible thing to find in the suite that guarantees the two platforms can talk. They fail rather than skip when the file is missing — deliberately, exactly so this could not hide. It still hid, one layer up, in a cache. `flamenet-e2e` is public, so the job shallow-clones it and points the three environment variables at the result. The paths are checked before the suite runs: a variable pointing at nothing would put the tests back where they were, and the failure would read as a protocol bug rather than a missing file. Cloned at its tip rather than pinned. The vectors are normative and shared with iOS, so if the published protocol changes this suite SHOULD go red without a commit here — that is a real divergence, not a stale pin. Verified the variables are actually honoured, rather than assuming: with one pointed at an empty JSON file the test reddens even though the sibling checkout is present. 539 engine tests pass against the cloned vectors. Co-Authored-By: Claude Opus 5 --- .gitlab-ci.yml | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index f055931..b03b548 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -22,6 +22,31 @@ unit-tests: script: - export JAVA_HOME=/opt/homebrew/opt/openjdk@21 - export ANDROID_HOME=$HOME/Library/Android/sdk + # 🔴 **The protocol vectors are not in this repo, and CI never had them.** + # `AccountKdfTest`, `PQVectorTest` and `PublishedVectorsTest` look for a + # sibling `flamenet-e2e` checkout. That exists on a developer's machine and + # has never existed in a build directory, so all twenty of them have been + # failing here since they were written — invisibly, because the build cache + # replayed a local pass. They fail rather than skip when the file is + # missing, exactly so this could not hide; it took `--rerun-tasks` to let + # them say so. + # + # ⚠️ Cloned at its tip rather than pinned. These vectors are normative and + # shared with iOS: if the published protocol changes, this suite SHOULD go + # red without a commit here, because that is a real divergence and not a + # stale pin. `flamenet-e2e` is public, so no credentials are needed. + - rm -rf "$CI_PROJECT_DIR/.ci-vectors" + - git clone --depth 1 https://gitlab.com/paulhitt/flamenet-e2e.git "$CI_PROJECT_DIR/.ci-vectors" + - export FLAMENET_VECTORS="$CI_PROJECT_DIR/.ci-vectors/test/vectors/protocol-v1.json" + - export FLAMENET_PQ_VECTORS="$CI_PROJECT_DIR/.ci-vectors/test/vectors/pqx3dh-v2.json" + - export FLAMENET_ACCOUNT_VECTORS="$CI_PROJECT_DIR/.ci-vectors/test/vectors/account-kdf-v1.json" + # ⚠️ Checked before the suite runs. A path that silently does not exist puts + # the tests straight back where they were, and the failure would read as a + # protocol bug rather than a missing file. + - | + for f in "$FLAMENET_VECTORS" "$FLAMENET_PQ_VECTORS" "$FLAMENET_ACCOUNT_VECTORS"; do + [ -f "$f" ] || { echo "GATE: vector file missing: $f"; exit 1; } + done # 🔴 **Clear old results before running, or the gate counts ghosts.** The # runner reuses its build directory, so XML from a previous pipeline # survives — and a suite that stopped running entirely would still be