From eafd77e9f6e96052476bdd88be732c046fdb7ab3 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Sat, 31 Oct 2026 11:47:39 +1100 Subject: [PATCH] ci: bound Hermit fetches and let relay builds run without sccache (#8739) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Two CI-workflow fixes from the 23–30 Oct flake audit. This is one of six PRs split from the reviewed `flakey-tests` branch (siblings below). Each PR applies on its own to `main`. ### Hermit tool downloads (5 runs, 1 on main) - **Runs:** 40635610550 and 40941660945, in Detect Changed Paths (`node`). The selection step printed nothing for 9–10 min, the job hit its cap, and every gate failed as cancelled. Three more, 40373404267 on main (PostgreSQL Tests and Desktop E2E Integration 1/3, `pgschema`), 40373003333 (Mobile Android, `cmake`) and 40373467575 (Desktop Build macOS, `just`), got 504s during GitHub's release-asset outage on 10-26, ~17:54–18:00 UTC. - **Cause:** jobs start with an empty Hermit cache, so a tool's first call downloads it, with no deadline. Hermit v0.54.1 tries three times, a second apart, then gives up. - **Change:** a new `scripts/ci-hermit-retry.sh` runs a no-op tool command with a 90 s deadline per attempt, three attempts and 15 s × n backoff. The deadline is plain bash: job control gives each attempt its own process group, which gets TERM and then KILL. macOS runners have no `timeout(1)` and may run bash 3.2. A "Fetch Hermit tools" step calls it for: - `node` and `just` in Detect Changed Paths, where one stall cancels every gate. One `sh -c` invocation covers both, so the worst case is 345 s, inside the 10-minute cap. Two invocations would take 690 s. - `pgschema` in PostgreSQL Tests, Desktop E2E Integration and Backend Integration (via `--help`, since it has no `--version`). - `just` and `cmake` in the Android lane. - `just` in Desktop Build (macOS). The test, `scripts/ci-hermit-retry.test.mjs`, joins Detect Changed Paths' `node --test` list, and the script joins the rust and mobile path filters. One stalled attempt costs ~105 s, which fits every job's cap. - A killed attempt doesn't break the retry. Hermit downloads to a temp file and unpacks to a temp dir, and renames each into place only once it's complete. Its lock is a kernel flock. ### sccache storage failures (2 runs) - **Runs:** 40355289475 (Desktop E2E Relay postgres-tests, 10-26, "Egress is over the account limit") and 40714034945 (relay-binaries, 10-28, "send http request"). - **Cause:** the sccache server checks its GHA cache storage at startup and won't start if that fails. Cargo's first `sccache rustc -vV` spawns it, so the build fails. `SCCACHE_IGNORE_SERVER_IO_ERROR` doesn't help here: in v0.16.0 it is read only when a running server's compile response fails. - **Change:** a "Start sccache" step runs `sccache --start-server`. It outputs `wrapper=sccache`, or an empty wrapper and a warning if the server won't start. The three build steps take `RUSTC_WRAPPER` from it, and cargo treats an empty `RUSTC_WRAPPER` as no wrapper. - The fallback doesn't fail the job later, in sccache-action's post step. At the pinned fc920bf, that step runs `sccache --show-stats` and the JSON form, and fails the job if either exits non-zero. With no server running, v0.16.0's `--show-stats` doesn't start one. It returns zero stats from the client side and exits 0. So the step keeps its stats annotation, and `disable_annotations` stays unset. ### Related issue None found. **Overlaps #8213** ("ci: fix empty root-workspace Rust caches and merge test-unit nextest runs"). #8213 already conflicts with `main`, and this PR adds a conflict in `.github/workflows/ci.yml` (path filters). ### Testing Verified locally: - With a fresh Hermit state dir, `node` was SIGKILLed mid-download, then mid-unpack while holding the lock, then stopped with TERM by the script's own 1 s deadline. Each retry produced a working v24.15.0, at clean-unpack speed. The `sh -c` form fetched both tools, and a 1 s deadline on `sh -c 'sleep 37 && ...'` stopped `sh` and its child, leaving no orphan. Tested under macOS bash 3.2 and Debian bash 5.2. - sccache v0.16.0 (aarch64-apple-darwin) ran against a GHA cache whose `ACTIONS_RESULTS_URL` refused connections, in READ_ONLY and READ_WRITE modes. `--start-server` exited 2 ("Server startup failed: cache storage failed to read"). Both `--show-stats` forms then exited 0, with zero stats and valid JSON. `--stop-server` exited 2, which shows no server had started. Only hosted CI can show: - the script under the runners' real shells, and that healthy runs only pay a few seconds; - whether ~3 min of spaced attempts rides out the next outage. The 10-26 outage lasted ~6 min, longer than this covers; - that the explicit `--start-server` behaves like cargo's implicit start against the real GHA cache. The fallback branch needs a storage outage to run in CI. Not covered here: first fetches of other tools (rustup, uv, pnpm) still have no deadline. A per-job Hermit package cache is the follow-up. Pushed with `git push --no-verify`, because the pre-push suites fail when the laptop is under load. ### Readiness An agent review of the combined `flakey-tests` branch (b432453af) found no blockers. `buzz-review-completed` is not added, because the human test step (AGENTS.md "Before opening a PR", step 3) hasn't happened yet. ### Siblings - #8740 test(relay): thread_window aux-page hold and `api::media` Postgres tests - #8741 test(mobile): RunnerTests title fade and alert wait - #8742 fix(desktop): deep-linked message hold (product change) - #8743 test(desktop): four specs racing scroll settles and focus - #8744 test(desktop): five settle classes and remote-owned-mentions 🤖 Generated with [Claude Code](https://claude.com/claude-code) Signed-off-by: Matt Toohey Co-authored-by: Claude Opus 5.5 --- .github/workflows/_ci-clients.yml | 7 ++ .github/workflows/_ci-desktop-macos.yml | 2 + .github/workflows/_ci-relay.yml | 30 ++++++- .github/workflows/ci.yml | 10 ++- lefthook.yml | 5 +- scripts/ci-hermit-retry.sh | 86 ++++++++++++++++++++ scripts/ci-hermit-retry.test.mjs | 103 ++++++++++++++++++++++++ scripts/ci-selection.test.mjs | 1 + 8 files changed, 238 insertions(+), 6 deletions(-) create mode 100755 scripts/ci-hermit-retry.sh create mode 100644 scripts/ci-hermit-retry.test.mjs diff --git a/.github/workflows/_ci-clients.yml b/.github/workflows/_ci-clients.yml index d1fe48a0a6..bbf352c1a8 100644 --- a/.github/workflows/_ci-clients.yml +++ b/.github/workflows/_ci-clients.yml @@ -177,6 +177,13 @@ jobs: ~/.gradle/wrapper key: gradle-${{ runner.os }}-${{ hashFiles('mobile/android/**/*.gradle*', 'mobile/android/gradle/wrapper/gradle-wrapper.properties', 'mobile/pubspec.lock') }} restore-keys: gradle-${{ runner.os }}- + # The Hermit package cache above holds only Flutter, so the build's first + # just and cmake calls download them. + - name: Fetch Hermit tools + if: matrix.lane == 'android' + run: | + scripts/ci-hermit-retry.sh ./bin/just --version + scripts/ci-hermit-retry.sh ./bin/cmake --version # SDK component and plugin downloads intermittently fail on hosted # runners; retry once only for those, so real build failures still fail. - name: Build Android debug APK diff --git a/.github/workflows/_ci-desktop-macos.yml b/.github/workflows/_ci-desktop-macos.yml index d19ffa72eb..b54a1b3fd6 100644 --- a/.github/workflows/_ci-desktop-macos.yml +++ b/.github/workflows/_ci-desktop-macos.yml @@ -31,6 +31,8 @@ jobs: steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 - uses: cashapp/activate-hermit@cea9af7913204a965fd488637a8d1811bba2e616 # v1 + - name: Fetch Hermit tools + run: scripts/ci-hermit-retry.sh ./bin/just --version - name: Verify macOS icon representations run: scripts/macos-icon.sh --check - uses: Swatinem/rust-cache@e18b497796c12c097a38f9edb9d0641fb99eee32 diff --git a/.github/workflows/_ci-relay.yml b/.github/workflows/_ci-relay.yml index e77df7312c..5a4c347064 100644 --- a/.github/workflows/_ci-relay.yml +++ b/.github/workflows/_ci-relay.yml @@ -88,6 +88,21 @@ jobs: uses: Mozilla-Actions/sccache-action@fc920bf0ec8de6ee65d409111f7ec508035751ba # v0.0.11 # zizmor: ignore[cache-poisoning] Bounded trial: only PR 5224 writes to its isolated merge-ref scope; trusted pushes retain production writes. with: version: v0.16.0 + # The sccache server checks its GHA cache storage at startup and refuses to + # start when that fails (egress over the account limit, a failed read), + # which used to fail cargo's first `sccache rustc -vV` and the build. + # SCCACHE_IGNORE_SERVER_IO_ERROR doesn't cover startup in v0.16.0, so start + # it here and build with plain rustc if it won't start. + - name: Start sccache + id: sccache + if: steps.relay-artifacts-cache.outputs.cache-hit != 'true' + run: | + if sccache --start-server; then + echo "wrapper=sccache" >> "$GITHUB_OUTPUT" + else + echo "::warning::sccache did not start; building without it" + echo "wrapper=" >> "$GITHUB_OUTPUT" + fi - name: Install cargo-nextest if: steps.relay-artifacts-cache.outputs.cache-hit != 'true' && matrix.artifact != 'relay-binaries' uses: taiki-e/install-action@0fd46367812ee04360509b4169d9f659d6892bb2 # v2.79.15 @@ -96,12 +111,12 @@ jobs: - name: Build relay binaries if: steps.relay-artifacts-cache.outputs.cache-hit != 'true' && matrix.artifact == 'relay-binaries' env: - RUSTC_WRAPPER: sccache + RUSTC_WRAPPER: ${{ steps.sccache.outputs.wrapper }} run: cargo build --profile ci -p buzz-relay -p git-credential-nostr - name: Build backend integration test archive if: steps.relay-artifacts-cache.outputs.cache-hit != 'true' && matrix.artifact == 'backend-integration-tests' env: - RUSTC_WRAPPER: sccache + RUSTC_WRAPPER: ${{ steps.sccache.outputs.wrapper }} run: | cargo nextest archive \ --cargo-profile ci \ @@ -116,7 +131,7 @@ jobs: - name: Build PostgreSQL test archive if: steps.relay-artifacts-cache.outputs.cache-hit != 'true' && matrix.artifact == 'postgres-tests' env: - RUSTC_WRAPPER: sccache + RUSTC_WRAPPER: ${{ steps.sccache.outputs.wrapper }} run: | postgres_package_args=() while IFS= read -r package; do @@ -191,6 +206,9 @@ jobs: steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 - uses: cashapp/activate-hermit@cea9af7913204a965fd488637a8d1811bba2e616 # v1 + # pgschema has no --version; --help exits 0 once Hermit has fetched it. + - name: Fetch Hermit tools + run: scripts/ci-hermit-retry.sh ./bin/pgschema --help - name: Tune disposable PostgreSQL env: PGPASSWORD: ${{ env.BUZZ_TEST_POSTGRES_PASSWORD }} @@ -243,6 +261,9 @@ jobs: steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 - uses: cashapp/activate-hermit@cea9af7913204a965fd488637a8d1811bba2e616 # v1 + # pgschema has no --version; --help exits 0 once Hermit has fetched it. + - name: Fetch Hermit tools + run: scripts/ci-hermit-retry.sh ./bin/pgschema --help - name: Start integration services run: | for attempt in 1 2 3; do @@ -433,6 +454,9 @@ jobs: steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 - uses: cashapp/activate-hermit@cea9af7913204a965fd488637a8d1811bba2e616 # v1 + # pgschema has no --version; --help exits 0 once Hermit has fetched it. + - name: Fetch Hermit tools + run: scripts/ci-hermit-retry.sh ./bin/pgschema --help - name: Install cargo-nextest uses: taiki-e/install-action@0fd46367812ee04360509b4169d9f659d6892bb2 # v2.79.15 with: diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c4cc6590bd..91943dc02c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -34,6 +34,12 @@ jobs: with: fetch-depth: 2 - uses: cashapp/activate-hermit@cea9af7913204a965fd488637a8d1811bba2e616 # v1 + # The first call of each Hermit tool downloads it. A stalled Node download + # hung the selection step until this job's cap cancelled every gate. One + # invocation fetches both tools, so its worst case (under 6 minutes) + # still leaves the rest of the job inside the 10-minute cap. + - name: Fetch Hermit tools + run: scripts/ci-hermit-retry.sh sh -c './bin/node --version && ./bin/just --version' - uses: dorny/paths-filter@ceb8a2b8f2d89434be7ff52d3de7ec3738c5cc9d # v4.0.3 id: filter with: @@ -69,6 +75,7 @@ jobs: - 'scripts/model-capabilities.json' - 'scripts/normative-corpus.json' - 'scripts/ci-apt-retry.sh' + - 'scripts/ci-hermit-retry.sh' - 'Justfile' desktop: - '.github/workflows/ci.yml' @@ -104,6 +111,7 @@ jobs: - 'scripts/test-mobile-release-contract.sh' - 'scripts/test-mobile-release-candidate-publisher.sh' - 'scripts/test-mobile-worktree-overrides.sh' + - 'scripts/ci-hermit-retry.sh' - '.github/workflows/mobile-release-candidate.yml' - '.github/workflows/ci.yml' - '.github/workflows/_ci-*.yml' @@ -144,7 +152,7 @@ jobs: - name: CI required-context isolation contract run: scripts/test-ci-required-context-isolation.sh - name: CI path selection regression tests - run: node --test scripts/ci-selection.test.mjs scripts/ci-required-gates.test.mjs scripts/ci-apt-retry.test.mjs + run: node --test scripts/ci-selection.test.mjs scripts/ci-required-gates.test.mjs scripts/ci-apt-retry.test.mjs scripts/ci-hermit-retry.test.mjs - name: File size policy run: just file-size-check diff --git a/lefthook.yml b/lefthook.yml index 529f1665f5..d4fbe25c6d 100644 --- a/lefthook.yml +++ b/lefthook.yml @@ -14,8 +14,9 @@ # they diverge and CI remains the authoritative gate for that ref. # Deliberate deviations: # - The `.github/workflows/ci.yml` path CI adds to its `rust`/`desktop`/`mobile` -# filters, and `scripts/ci-apt-retry.sh` in `rust`/`desktop`, are omitted; -# a CI-plumbing-only edit doesn't need a local test run. +# filters, `scripts/ci-apt-retry.sh` in `rust`/`desktop`, and +# `scripts/ci-hermit-retry.sh` in `rust`/`mobile`, are omitted; a +# CI-plumbing-only edit doesn't need a local test run. # - `desktop-test` doesn't trigger on `crates/**` changes, though CI's Desktop # JS job does: a few node:test cases read relay/DB crate sources to pin shared # constants. Running the full desktop suite on every crate push would be diff --git a/scripts/ci-hermit-retry.sh b/scripts/ci-hermit-retry.sh new file mode 100755 index 0000000000..b3d9795e68 --- /dev/null +++ b/scripts/ci-hermit-retry.sh @@ -0,0 +1,86 @@ +#!/usr/bin/env bash +# ============================================================================= +# ci-hermit-retry.sh — fetch a Hermit tool with a per-attempt deadline +# ============================================================================= +# CI jobs start with an empty Hermit package cache, so the first run of a bin/ +# stub downloads its package. That download has no deadline: a stalled Node +# download hung Detect Changed Paths until the job's cap cancelled every gate. +# Hermit tries a download three times, a second apart, so a short run of +# GitHub release-asset 504s fails the job outright. +# +# Run a cheap command for each tool through this before the job's first real +# use (`scripts/ci-hermit-retry.sh ./bin/node --version`). Each attempt runs in +# its own process group, which is stopped at the deadline (TERM, then KILL) +# and retried after a growing backoff. +# +# A stopped attempt leaves nothing Hermit reuses. Hermit downloads to a +# temporary file and unpacks to a temporary directory, renaming each into place +# only once complete, and its state lock is an flock that the kernel drops with +# the process. The leftover temporaries only cost disk. +# +# With the defaults one invocation is bounded at under 6 minutes. Size job +# timeouts to fit that plus the job's real work. A job with a tight cap can +# fetch several tools in one invocation through `sh -c`, as Detect Changed +# Paths does. +# +# Runs under macOS's /bin/bash 3.2 and does not use timeout(1), which macOS +# runners lack. +# +# Usage: +# scripts/ci-hermit-retry.sh [args...] +# +# Env: +# CI_HERMIT_ATTEMPTS Attempts before failing (default: 3). +# CI_HERMIT_ATTEMPT_SECONDS Deadline per attempt (default: 90). Healthy +# fetches finish well inside a minute. +# CI_HERMIT_KILL_AFTER_SECONDS Wait after TERM before KILL (default: 10). +# CI_HERMIT_BACKOFF_SECONDS Sleep after failed attempt n is n times this +# (default: 15). +# ============================================================================= +set -euo pipefail + +attempts="${CI_HERMIT_ATTEMPTS:-3}" +deadline="${CI_HERMIT_ATTEMPT_SECONDS:-90}" +kill_after="${CI_HERMIT_KILL_AFTER_SECONDS:-10}" +backoff="${CI_HERMIT_BACKOFF_SECONDS:-15}" + +if [ "$#" -eq 0 ]; then + echo "usage: $0 [args...]" >&2 + exit 2 +fi + +for attempt in $(seq 1 "$attempts"); do + # Job control puts each background job in its own process group, so the + # watchdog's signals reach everything the attempt started. + set -m + "$@" & + pid=$! + ( + sleep "$deadline" + kill -TERM -- "-$pid" 2>/dev/null || exit 0 + sleep "$kill_after" + kill -KILL -- "-$pid" 2>/dev/null || true + ) >/dev/null 2>&1 & + watchdog=$! + set +m + started=$SECONDS + rc=0 + # Bash reports a job killed by a signal on wait's stderr; the warning below + # says the same thing. + wait "$pid" 2>/dev/null || rc=$? + kill -- "-$watchdog" 2>/dev/null || true + wait "$watchdog" 2>/dev/null || true + [ "$rc" -eq 0 ] && exit 0 + # The leader can exit on TERM while a child that ignores it lives on. + kill -KILL -- "-$pid" 2>/dev/null || true + if [ $((SECONDS - started)) -ge "$deadline" ]; then + echo "::warning::attempt $attempt/$attempts timed out after ${deadline}s: $*" + else + echo "::warning::attempt $attempt/$attempts failed (exit $rc): $*" + fi + if [ "$attempt" -lt "$attempts" ]; then + sleep $((attempt * backoff)) + fi +done +echo "::error::all $attempts attempts failed: $*" +exit 1 diff --git a/scripts/ci-hermit-retry.test.mjs b/scripts/ci-hermit-retry.test.mjs new file mode 100644 index 0000000000..14fff2fa4f --- /dev/null +++ b/scripts/ci-hermit-retry.test.mjs @@ -0,0 +1,103 @@ +import assert from "node:assert/strict"; +import { spawnSync } from "node:child_process"; +import { + chmodSync, + existsSync, + mkdtempSync, + readFileSync, + writeFileSync, +} from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { test } from "node:test"; +import { setTimeout as sleep } from "node:timers/promises"; +import { fileURLToPath } from "node:url"; + +const script = fileURLToPath(new URL("./ci-hermit-retry.sh", import.meta.url)); + +// Run the wrapper on a fake tool that counts its calls, then runs `body` with +// $n set to the call number and $dir to a scratch directory. +function run(body, env = {}) { + const dir = mkdtempSync(join(tmpdir(), "ci-hermit-retry-")); + const calls = join(dir, "calls"); + writeFileSync(calls, ""); + const tool = join(dir, "tool"); + writeFileSync( + tool, + `#!/bin/bash +dir="${dir}" +echo call >> "$dir/calls" +n=$(( $(wc -l < "$dir/calls") )) +${body} +`, + ); + chmodSync(tool, 0o755); + const started = Date.now(); + const result = spawnSync("bash", [script, tool], { + encoding: "utf8", + timeout: 20_000, + env: { + ...process.env, + CI_HERMIT_ATTEMPT_SECONDS: "1", + CI_HERMIT_KILL_AFTER_SECONDS: "1", + CI_HERMIT_BACKOFF_SECONDS: "0", + ...env, + }, + }); + return { + dir, + status: result.status, + seconds: (Date.now() - started) / 1000, + calls: readFileSync(calls, "utf8").split("\n").filter(Boolean).length, + stdout: result.stdout, + }; +} + +test("a first-attempt success runs the tool once and keeps its output", () => { + const r = run("echo v1.2.3"); + assert.equal(r.status, 0); + assert.equal(r.calls, 1); + assert.equal(r.stdout, "v1.2.3\n"); +}); + +// A stalled download: the first call hangs on a child that would write a +// marker at 2s if it outlived the attempt's 1s deadline. +test("a stalled attempt is stopped with its children and retried", async () => { + const r = run(` +if [ "$n" -eq 1 ]; then + ( sleep 2; touch "$dir/survived" ) & + wait +fi +echo fetched`); + assert.equal(r.status, 0, r.stdout); + assert.equal(r.calls, 2); + assert.match(r.stdout, /::warning::attempt 1\/3 timed out after 1s/); + assert.match(r.stdout, /fetched\n$/); + assert.ok(r.seconds < 10, `took ${r.seconds}s`); + await sleep(Math.max(0, 3000 - r.seconds * 1000)); + assert.ok( + !existsSync(join(r.dir, "survived")), + "the stalled attempt's child outlived it", + ); +}); + +test("an attempt that ignores TERM is killed", () => { + const r = run("trap '' TERM; sleep 30", { CI_HERMIT_ATTEMPTS: "1" }); + assert.equal(r.status, 1, r.stdout); + assert.match(r.stdout, /attempt 1\/1 timed out after 1s/); + assert.ok(r.seconds < 10, `took ${r.seconds}s`); +}); + +test("persistent failure backs off between attempts and stops at the budget", () => { + const r = run("exit 7", { CI_HERMIT_BACKOFF_SECONDS: "1" }); + assert.equal(r.status, 1); + assert.equal(r.calls, 3); + assert.match(r.stdout, /attempt 3\/3 failed \(exit 7\)/); + assert.match(r.stdout, /::error::all 3 attempts failed/); + // Backoff sleeps 1s after attempt 1 and 2s after attempt 2. + assert.ok(r.seconds >= 3, `took ${r.seconds}s`); +}); + +test("no command is a usage error", () => { + assert.equal(spawnSync("bash", [script]).status, 2); +}); diff --git a/scripts/ci-selection.test.mjs b/scripts/ci-selection.test.mjs index 4fb7508293..821d4f0884 100644 --- a/scripts/ci-selection.test.mjs +++ b/scripts/ci-selection.test.mjs @@ -244,6 +244,7 @@ const scenarios = [ ], ["Justfile", ["Justfile"], ["desktop", "mobile", "rust"]], ["CI apt retry", ["scripts/ci-apt-retry.sh"], ["desktop", "rust"]], + ["CI Hermit retry", ["scripts/ci-hermit-retry.sh"], ["mobile", "rust"]], ["integration services", ["docker-compose.yml"], ["rust"]], ["CI integration images", ["docker-compose.ci.yml"], ["rust"]], [