From ec8fecf6b8655e19a72c68490e80e8656a52321a Mon Sep 17 00:00:00 2001 From: NiveditJain Date: Wed, 19 Aug 2026 13:58:05 +0530 Subject: [PATCH 1/3] Cut CI wall clock from ~4min to ~1.7min, and bound the jobs that can hang MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four costs, all found by measuring a green run rather than a red one. bun install was running a full Next.js production build. package.json's `prepare` is `bun run build`, which bun fires as an install lifecycle hook, so six of ci.yml's eight jobs spent ~28s building an application they never read — and `build` did it twice. rust-quality has passed --ignore-scripts since it landed and installs in one second; that is the control. Every install now does. The cargo cache cost more to move than the work it replaced. One entry had reached 5,727 MB — 57% of the repo's 10 GiB quota in a single key — and took 127s to restore against the 74s cargo test it existed to avoid. `path: target` archives every intermediate the workspace ever produced. Swatinem/rust-cache keeps the dependency artifacts and prunes the rest. rust-quality ran in full on every PR, including those touching no Rust. Its `Detect crates` gate was written for a stage-1 empty workspace and has been answering true unconditionally since the crates landed. It now also diffs the merge commit against its first parent, so the job still reports a status while finishing in seconds on a TypeScript-only branch. docs gained the same gate. 190 of 208 unit test files built a jsdom they never touched. Split into node/dom projects on the file extension: jsdom construction drops from 40.96s to 13.67s locally, and a new .test.tsx still gets a DOM automatically. Separately, nothing here had a job timeout except integration-suite.yml, so a stalled Azure apt mirror held v1.0.1's linux-x64 daemon leg through three runner re-dispatches. Every job across five workflows now declares one, the apt step is retried with real acquire timeouts, and build-daemon gains a concurrency group scoped to pull_request so a release's own legs are never cancelled. None of these turn CI red on their own, so release-pipeline.test.ts now asserts all four. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01KbT6esM8A8mkSymH13keLt --- .github/workflows/build-daemon.yml | 85 +++++++---- .github/workflows/build-image.yml | 5 + .github/workflows/ci.yml | 188 ++++++++++++++++++------- .github/workflows/osv-scanner.yml | 8 ++ .github/workflows/publish.yml | 21 ++- CHANGELOG.md | 2 + __tests__/ci/release-pipeline.test.ts | 88 ++++++++++++ __tests__/lib/client-telemetry.test.ts | 4 + vitest.config.mts | 49 ++++++- 9 files changed, 365 insertions(+), 85 deletions(-) diff --git a/.github/workflows/build-daemon.yml b/.github/workflows/build-daemon.yml index bcdc0591b..81fc233c3 100644 --- a/.github/workflows/build-daemon.yml +++ b/.github/workflows/build-daemon.yml @@ -22,6 +22,15 @@ on: workflow_call: workflow_dispatch: +# Consecutive pushes to a Rust PR would otherwise stack 4-leg cross-compile +# matrices that nothing cancels — the most expensive PR-triggered workflow in +# the repo, started once per push and left running. Superseding is only ever +# right on `pull_request`: the `workflow_call` legs ARE the release's binaries, +# and a cancelled one is a release that ships without its daemon. +concurrency: + group: build-daemon-${{ github.ref }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} + jobs: # The Rust workspace does not exist on every ref this workflow can run # against: `main` carries no `Cargo.toml` until the daemon lands, and the @@ -32,6 +41,7 @@ jobs: # no daemon at all. detect: runs-on: ubuntu-latest + timeout-minutes: 5 outputs: has_crates: ${{ steps.check.outputs.has_crates }} steps: @@ -87,6 +97,13 @@ jobs: os: macos-14 platform: darwin-arm64 runs-on: ${{ matrix.os }} + # Nothing in this job has a bounded runtime of its own, and GitHub's default + # is six hours. On 2026-08-19 the linux-x64 leg sat in `apt-get update` + # against a stalled Azure mirror through three runner re-dispatches with a + # release blocked behind it, while the arm64 leg ran the same step in + # seconds. A cross compile that has not finished in 30 minutes is not going + # to, and a bounded failure is re-runnable where a hang is not. + timeout-minutes: 30 steps: - uses: actions/checkout@v7.0.1 with: @@ -103,27 +120,51 @@ jobs: # without musl-tools the leg fails at link time with # `linker 'musl-gcc' not found`. Both Linux runners are native to their # own target, so the distro package is the right linker for the triple. + # It is not droppable either: `-p failproofaid` reaches `rusqlite` with + # `bundled` (compiles sqlite3.c) and `ring` through rustls, so the `cc` + # crate resolves `musl-gcc` on both musl legs. + # + # Retried and bounded, because the failure mode here is a HANG rather than + # an error. apt's default acquire timeouts are long enough to be no + # timeout at all against a stalled mirror, so without these the leg would + # spend the entire job budget above on one bad Azure endpoint instead of + # four minutes and a retry against a different one. `-qq` is gone on + # purpose too: it suppressed the per-mirror progress that says WHICH + # endpoint stalled, which is the one thing worth having in the log. - name: Install the musl toolchain if: contains(matrix.target, 'musl') - run: sudo apt-get update -qq && sudo apt-get install -y -qq musl-tools + uses: nick-fields/retry@v4 + with: + max_attempts: 3 + timeout_minutes: 4 + command: | + sudo apt-get update \ + -o Acquire::Retries=3 \ + -o Acquire::http::Timeout=20 \ + -o Acquire::https::Timeout=20 \ + -o Acquire::Languages=none + sudo apt-get install -y --no-install-recommends musl-tools - # Split restore/save rather than `actions/cache@v6`, which does both. - # This job runs on `pull_request` AND on the release path (via - # `workflow_call` from publish.yml, where `github.event_name` is the - # caller's `release`/`workflow_dispatch`), and the cache key is shared - # between them: a PR branch could otherwise write a poisoned `target/` - # entry that a later release run restores straight into a published - # binary. PR runs read the cache; only release / manual runs write it. - - uses: actions/cache/restore@v6 - id: cargo-cache + # `save-if` rather than a plain cache, for the reason the split + # restore/save here used to carry: this job runs on `pull_request` AND on + # the release path (via `workflow_call` from publish.yml, where + # `github.event_name` is the caller's `release`/`workflow_dispatch`), and + # the cache key is shared between them. A PR branch could otherwise write + # a poisoned `target/` entry that a later release run restores straight + # into a published binary. PR runs read the cache; only release / manual + # runs write it. + # + # rust-cache rather than the hand-rolled `actions/cache` pair, because the + # naive `path: target` it replaced is what made ci.yml's sibling entry + # 5.7 GB — 57% of the repo's whole 10 GB cache quota in one key, taking + # 127s to restore against the 74s of compilation it saved. rust-cache + # prunes `target/` to dependency artifacts and drops the workspace's own + # output, which is the difference between caching a build and caching a + # build directory. `key` keeps the four legs from sharing an entry. + - uses: Swatinem/rust-cache@v2 with: - path: | - ~/.cargo/registry/index - ~/.cargo/registry/cache - ~/.cargo/git/db - target - key: cargo-build-${{ matrix.target }}-${{ hashFiles('rust-toolchain.toml', 'Cargo.lock') }} - restore-keys: cargo-build-${{ matrix.target }}- + key: ${{ matrix.target }} + save-if: ${{ github.event_name != 'pull_request' }} # --locked: this job's output is the binary users install, so it must be # built from the dependency versions committed in Cargo.lock. Without it @@ -133,16 +174,6 @@ jobs: - name: cargo build --release run: cargo build --locked --release --target ${{ matrix.target }} -p failproofaid - - uses: actions/cache/save@v6 - if: github.event_name != 'pull_request' && steps.cargo-cache.outputs.cache-hit != 'true' - with: - path: | - ~/.cargo/registry/index - ~/.cargo/registry/cache - ~/.cargo/git/db - target - key: cargo-build-${{ matrix.target }}-${{ hashFiles('rust-toolchain.toml', 'Cargo.lock') }} - # Plain gzip, not tar: the CLI decompresses this with `node:zlib` and no # dependency, and a single-file stream needs no archive format. It also # sidesteps `upload-artifact` dropping the executable bit — a compressed diff --git a/.github/workflows/build-image.yml b/.github/workflows/build-image.yml index 2a0c80f9a..fee175a90 100644 --- a/.github/workflows/build-image.yml +++ b/.github/workflows/build-image.yml @@ -30,6 +30,10 @@ on: default: true required: false +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: false + permissions: contents: read packages: write @@ -37,6 +41,7 @@ permissions: jobs: build: runs-on: ubuntu-latest + timeout-minutes: 30 steps: - name: Checkout uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c8c4e5639..759a2c929 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -13,6 +13,7 @@ concurrency: jobs: quality: runs-on: ubuntu-latest + timeout-minutes: 10 env: FAILPROOFAI_TELEMETRY_DISABLED: "1" steps: @@ -22,18 +23,38 @@ jobs: with: bun-version: latest - - uses: actions/cache@v6 + # Restore-only here and in every other job; `quality` alone writes, below. + # All six jobs used to share one read-write `actions/cache@v6` on this key, + # so all six raced to upload the same 401 MB entry and five of them lost + # the race — paying the upload to be told the key already existed. One + # writer is all a shared key can use. + - uses: actions/cache/restore@v6 + id: bun-cache with: path: ~/.bun/install/cache key: bun-${{ runner.os }}-${{ hashFiles('bun.lock') }} restore-keys: bun-${{ runner.os }}- + # `--ignore-scripts`, here and in every other job, because package.json's + # `prepare` is `bun run build` — so a plain `bun install` runs a full + # Next.js production build (~28s: 14s compile + 13s TypeScript) as an + # install lifecycle hook, in six of this workflow's eight jobs. Nothing in + # this job reads `dist/`: tsconfig.json excludes it, eslint.config.mjs + # ignores it, and no app/ file references Next's generated route types. + # rust-quality has guarded this way since it landed, and its install takes + # one second against the thirty-three this one used to. - name: Install dependencies uses: nick-fields/retry@v4 with: max_attempts: 3 timeout_minutes: 5 - command: bun install --frozen-lockfile + command: bun install --frozen-lockfile --ignore-scripts + + - uses: actions/cache/save@v6 + if: steps.bun-cache.outputs.cache-hit != 'true' + with: + path: ~/.bun/install/cache + key: bun-${{ runner.os }}-${{ hashFiles('bun.lock') }} - name: Check version consistency run: | @@ -95,6 +116,7 @@ jobs: rust-quality: runs-on: ubuntu-latest + timeout-minutes: 20 steps: - uses: actions/checkout@v7.0.1 with: @@ -103,21 +125,46 @@ jobs: # default (`true`) would leave GITHUB_TOKEN in .git/config where # any of them could read it; nothing here needs push access. persist-credentials: false + # On `pull_request`, actions/checkout builds the merge commit, whose + # first parent is the base — so depth 2 is exactly enough for the + # diff below, without fetching the branch's history. + fetch-depth: 2 - # Stage 1 lands an empty Cargo workspace (zero crates/*/Cargo.toml) so - # the CI plumbing itself can go green before any Rust code exists. - # `cargo build/clippy/test --workspace` (and even `cargo fmt --all`) - # all hard-error on a zero-member workspace ("the workspace has no - # members"), so every real step below is gated on at least one crate - # being present rather than relying on any of them to no-op cleanly. - - name: Detect crates + # Two gates in one, both answering "is there Rust work to do here". + # + # The first is the original: stage 1 landed an empty Cargo workspace so + # the CI plumbing could go green before any Rust existed, and + # `cargo build/clippy/test --workspace` (and even `cargo fmt --all`) all + # hard-error on a zero-member workspace rather than no-opping cleanly. + # All three crates exist now, so on its own this always answers true. + # + # The second is why it is still here. This is the longest job in CI, so it + # sets the wall clock for EVERY pull request — including the many that + # touch no Rust at all, where it restores a cache and recompiles a + # 231-crate dependency tree to check nothing. Gating on the diff leaves + # the job reporting a status (no `needs:` edge, no new job, nothing + # serialised behind it) while finishing in seconds on a TypeScript-only + # branch. `push` is never gated: main always gets the full check, which is + # also what keeps the cache warm for everyone else. + - name: Detect Rust changes id: crates run: | - if ls crates/*/Cargo.toml >/dev/null 2>&1; then + if ! ls crates/*/Cargo.toml >/dev/null 2>&1; then + echo "present=false" >> "$GITHUB_OUTPUT" + echo "No crates/*/Cargo.toml on this ref — rust-quality has nothing to check." + exit 0 + fi + if [ "${{ github.event_name }}" != "pull_request" ]; then + echo "present=true" >> "$GITHUB_OUTPUT" + exit 0 + fi + # Anything that can change what clippy or the tests see. ci.yml is in + # the list so a change to this gate always exercises the gate. + if git diff --name-only HEAD^1 HEAD | grep -qE '^(crates/|Cargo\.toml$|Cargo\.lock$|rust-toolchain\.toml$|\.github/workflows/ci\.yml$)'; then echo "present=true" >> "$GITHUB_OUTPUT" else echo "present=false" >> "$GITHUB_OUTPUT" - echo "No crates/*/Cargo.toml yet — rust-quality has nothing to check." + echo "No Rust-affecting paths in this PR — skipping fmt/clippy/test." fi - if: steps.crates.outputs.present == 'true' @@ -139,7 +186,7 @@ jobs: # exited before creating its socket". Production is unaffected, because # dist/worker.mjs bundles those deps; only this path needs them on disk. - if: steps.crates.outputs.present == 'true' - uses: actions/cache@v6 + uses: actions/cache/restore@v6 with: path: ~/.bun/install/cache key: bun-${{ runner.os }}-${{ hashFiles('bun.lock') }} @@ -170,17 +217,24 @@ jobs: # Restoring without saving costs a PR whose `Cargo.lock` moved a rebuild # from a stale-but-close main cache — which is already what `restore-keys` # hands it today. + # + # That fixed the MULTIPLICATION and left the SIZE, and the size was the + # bigger half. One entry reached 5,727 MB — 57% of the whole 10 GiB quota + # in a single key, so the store stayed in permanent LRU eviction against + # the translation cache anyway — and restoring it took 127 SECONDS, more + # wall clock than the 74s `cargo test` it was there to avoid. A cache that + # costs more to move than the work it replaces is not a cache. + # + # The cause is `path: target` taken literally: it archives every + # intermediate the workspace has ever produced, including this workspace's + # own crates, which recompile in seconds and are the artifacts most likely + # to be stale. rust-cache caches the dependency artifacts and prunes the + # rest. It also handles the save gate directly, so the paired + # `cache/save` step below this is gone. - if: steps.crates.outputs.present == 'true' - id: cargo-cache - uses: actions/cache/restore@v6 + uses: Swatinem/rust-cache@v2 with: - path: | - ~/.cargo/registry/index - ~/.cargo/registry/cache - ~/.cargo/git/db - target - key: cargo-${{ runner.os }}-${{ hashFiles('rust-toolchain.toml', 'Cargo.lock', 'crates/*/Cargo.toml') }} - restore-keys: cargo-${{ runner.os }}- + save-if: ${{ github.event_name == 'push' && github.ref == 'refs/heads/main' }} - name: cargo fmt --check if: steps.crates.outputs.present == 'true' @@ -194,27 +248,12 @@ jobs: if: steps.crates.outputs.present == 'true' run: cargo test --workspace - # Paired with the restore above. `cache-hit != 'true'` skips the write when - # the exact key already exists, so a run that changed nothing does not - # re-upload 2 GiB; a push to main whose Cargo.lock moved is the only thing - # that writes here. - - name: Save cargo cache - if: >- - steps.crates.outputs.present == 'true' - && github.event_name == 'push' - && github.ref == 'refs/heads/main' - && steps.cargo-cache.outputs.cache-hit != 'true' - uses: actions/cache/save@v6 - with: - path: | - ~/.cargo/registry/index - ~/.cargo/registry/cache - ~/.cargo/git/db - target - key: cargo-${{ runner.os }}-${{ hashFiles('rust-toolchain.toml', 'Cargo.lock', 'crates/*/Cargo.toml') }} - test: runs-on: ubuntu-latest + # The retry above nominally allows 3 attempts x 10 minutes. Capping the job + # at 20 is deliberate: a test job deep into a retry storm is a failure worth + # surfacing, not one worth waiting out. + timeout-minutes: 20 strategy: fail-fast: false matrix: @@ -232,7 +271,7 @@ jobs: with: bun-version: latest - - uses: actions/cache@v6 + - uses: actions/cache/restore@v6 with: path: ~/.bun/install/cache key: bun-${{ runner.os }}-${{ hashFiles('bun.lock') }} @@ -243,7 +282,7 @@ jobs: with: max_attempts: 3 timeout_minutes: 5 - command: bun install --frozen-lockfile + command: bun install --frozen-lockfile --ignore-scripts # Pinned, because vitest runs under Node and this job had been taking # whatever the runner image happened to ship. That is how the suite came @@ -267,6 +306,7 @@ jobs: build: runs-on: ubuntu-latest + timeout-minutes: 15 env: FAILPROOFAI_TELEMETRY_DISABLED: "1" NEXT_TELEMETRY_DISABLED: "1" @@ -277,7 +317,7 @@ jobs: with: bun-version: latest - - uses: actions/cache@v6 + - uses: actions/cache/restore@v6 with: path: ~/.bun/install/cache key: bun-${{ runner.os }}-${{ hashFiles('bun.lock') }} @@ -288,7 +328,7 @@ jobs: with: max_attempts: 3 timeout_minutes: 5 - command: bun install --frozen-lockfile + command: bun install --frozen-lockfile --ignore-scripts - name: Build uses: nick-fields/retry@v4 @@ -299,35 +339,66 @@ jobs: docs: runs-on: ubuntu-latest + timeout-minutes: 10 steps: - uses: actions/checkout@v7.0.1 + with: + # Depth 2 so the gate below can diff the PR's merge commit against + # its first parent. See rust-quality for the full reasoning. + fetch-depth: 2 + + # Same shape as rust-quality's gate, same reason: this job installs a + # global npm package and a full dependency tree to validate documentation + # that most pull requests do not touch. On `push` it always runs. + - name: Detect docs changes + id: docs + run: | + if [ "${{ github.event_name }}" != "pull_request" ]; then + echo "present=true" >> "$GITHUB_OUTPUT" + exit 0 + fi + if git diff --name-only HEAD^1 HEAD | grep -qE '^(docs/|scripts/validate-mdx\.ts$|package\.json$|\.github/workflows/ci\.yml$)|\.mdx$'; then + echo "present=true" >> "$GITHUB_OUTPUT" + else + echo "present=false" >> "$GITHUB_OUTPUT" + echo "No docs-affecting paths in this PR — skipping validation." + fi - - uses: oven-sh/setup-bun@v2 + - if: steps.docs.outputs.present == 'true' + uses: oven-sh/setup-bun@v2 with: bun-version: latest - - uses: actions/cache@v6 + - if: steps.docs.outputs.present == 'true' + uses: actions/cache/restore@v6 with: path: ~/.bun/install/cache key: bun-${{ runner.os }}-${{ hashFiles('bun.lock') }} restore-keys: bun-${{ runner.os }}- - name: Install dependencies + if: steps.docs.outputs.present == 'true' uses: nick-fields/retry@v4 with: max_attempts: 3 timeout_minutes: 5 - command: bun install --frozen-lockfile + command: bun install --frozen-lockfile --ignore-scripts - - uses: actions/setup-node@v7 + - if: steps.docs.outputs.present == 'true' + uses: actions/setup-node@v7 with: node-version: 22 + # Pinned to match translate-docs.yml, which has always pinned it. Floating + # here meant an upstream Mintlify release could turn the PR gate red on a + # branch that changed nothing. - name: Install Mintlify CLI - run: npm install -g mintlify + if: steps.docs.outputs.present == 'true' + run: npm install -g mintlify@4.2.680 # Validates docs.json structure + nav-link resolution. - name: Validate docs config + if: steps.docs.outputs.present == 'true' working-directory: docs run: mintlify validate @@ -339,10 +410,12 @@ jobs: # reference resolves on disk — that class breaks nothing at build time, # it just renders as a broken image, so nothing else in CI watches it. - name: Validate MDX pages parse and image references resolve + if: steps.docs.outputs.present == 'true' run: bun run validate:mdx test-e2e: runs-on: ubuntu-latest + timeout-minutes: 15 env: FAILPROOFAI_TELEMETRY_DISABLED: "1" NEXT_TELEMETRY_DISABLED: "1" @@ -353,7 +426,7 @@ jobs: with: bun-version: latest - - uses: actions/cache@v6 + - uses: actions/cache/restore@v6 with: path: ~/.bun/install/cache key: bun-${{ runner.os }}-${{ hashFiles('bun.lock') }} @@ -364,10 +437,19 @@ jobs: with: max_attempts: 3 timeout_minutes: 5 - command: bun install --frozen-lockfile - + command: bun install --frozen-lockfile --ignore-scripts + + # The vitest e2e suite spawns bin/failproofai.mjs with + # FAILPROOFAI_DIST_PATH pointed here, so it needs dist/index.js. cli.mjs + # and worker.mjs are for the standalone __tests__/e2e/layout/*.sh + # fixtures, which are not in the vitest `include` and so were quietly + # relying on the `prepare` hook having built them during install. Two more + # bun bundles, ~50ms, and they stay runnable from a CI checkout. - name: Build E2E fixtures - run: bun build src/index.ts --outdir dist --target node --format cjs + run: | + bun build src/index.ts --outdir dist --target node --format cjs + bun run build:cli + bun run build:worker - name: E2E Hook Tests uses: nick-fields/retry@v4 diff --git a/.github/workflows/osv-scanner.yml b/.github/workflows/osv-scanner.yml index 742bf0e39..c66e0370e 100644 --- a/.github/workflows/osv-scanner.yml +++ b/.github/workflows/osv-scanner.yml @@ -34,6 +34,13 @@ on: - cron: "17 4 * * *" # daily, 04:17 UTC workflow_dispatch: +# Supersede a superseded PR push: the scan re-reads both lockfiles from +# scratch every run, so a stale one has nothing to contribute. Scheduled and +# main runs key on their own ref and so never cancel each other. +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} + # Least privilege: the scan only needs to read the checked-out source. # contents:read is also available to Dependabot PRs (read-only token), so the # gate enforces on dependency-bump PRs too. @@ -44,6 +51,7 @@ jobs: osv-scanner: name: OSV-Scanner runs-on: ubuntu-latest + timeout-minutes: 15 steps: - name: Checkout uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 1ea7337ce..1de5dfe5b 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -67,6 +67,7 @@ jobs: # cross-compile matrix), and whether this ref even carries the daemon. preflight: runs-on: ubuntu-latest + timeout-minutes: 10 outputs: publish_version: ${{ steps.version.outputs.publish_version }} pkg_version: ${{ steps.version.outputs.pkg_version }} @@ -311,6 +312,7 @@ jobs: cli-tarball: needs: preflight runs-on: ubuntu-latest + timeout-minutes: 15 steps: - uses: actions/checkout@v7.0.1 with: @@ -326,12 +328,18 @@ jobs: key: bun-${{ runner.os }}-${{ hashFiles('bun.lock') }} restore-keys: bun-${{ runner.os }}- + # `--ignore-scripts` for the reason `npm pack` below already gives: the + # `prepare` hook is `bun run build`, so a plain install fires a full + # Next.js build. Here it also fired the WRONG one — install runs before + # `npm version` sets the publish version, so `prepare` baked the old + # version into dist/ and the explicit Build step below then threw that + # work away and redid it. One build now, at the right version. - name: Install dependencies uses: nick-fields/retry@v4 with: max_attempts: 3 timeout_minutes: 5 - command: bun install --frozen-lockfile + command: bun install --frozen-lockfile --ignore-scripts - uses: actions/setup-node@v7 with: @@ -377,6 +385,7 @@ jobs: needs.cli-tarball.result == 'success' && (needs.daemon.result == 'success' || needs.daemon.result == 'skipped') runs-on: ubuntu-latest + timeout-minutes: 10 permissions: contents: write steps: @@ -460,6 +469,7 @@ jobs: (needs.daemon.result == 'success' || needs.daemon.result == 'skipped') && (needs.release-assets.result == 'success' || needs.release-assets.result == 'skipped') runs-on: ubuntu-latest + timeout-minutes: 30 permissions: contents: write id-token: write @@ -501,12 +511,17 @@ jobs: key: bun-${{ runner.os }}-${{ hashFiles('bun.lock') }} restore-keys: bun-${{ runner.os }}- + # `--ignore-scripts`: the explicit Build step below is the one that + # matters (it runs after the daemon platform packages are pinned into + # package.json), and `npm publish` already passes `--ignore-scripts` too. + # Nothing here depended on the `prepare` hook — it was a second full + # Next.js build per release. - name: Install dependencies uses: nick-fields/retry@v4 with: max_attempts: 3 timeout_minutes: 5 - command: bun install --frozen-lockfile + command: bun install --frozen-lockfile --ignore-scripts - uses: actions/setup-node@v7 with: @@ -789,6 +804,7 @@ jobs: - os: macos-14 platform: darwin-arm64 runs-on: ${{ matrix.os }} + timeout-minutes: 20 steps: # Deliberately the OLDEST Node this package claims to support # (`engines.node: >=20.9.0`), while the build and publish jobs above run @@ -906,6 +922,7 @@ jobs: needs.preflight.outputs.is_prerelease == 'false' && needs.preflight.outputs.dist_tag == 'latest' runs-on: ubuntu-latest + timeout-minutes: 10 # Least privilege, declared rather than inherited. This job reads the tree # and POSTs to a webhook — it writes nothing here — while holding a # credential that can post to a public channel. Without a block it takes the diff --git a/CHANGELOG.md b/CHANGELOG.md index b472798eb..411e765fd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,8 @@ ### Fixes +- **Stop CI paying for work it throws away, and stop a stalled apt mirror holding a release for six hours.** Four costs, found by measuring a green run rather than a red one. **`bun install` was running a full Next.js production build**: `package.json`'s `prepare` is `bun run build`, which bun fires as an install lifecycle hook, so six of `ci.yml`'s eight jobs spent ~28s (14s compile + 13s TypeScript) building an application they never read — and the `build` job did it twice, since its own Build step then re-ran the same thing warm in 7s. `rust-quality` has passed `--ignore-scripts` since it landed and installs in **one second**, which is the control that proves the rest; `translate-docs.yml` already guards the same way with a comment naming this exact hazard. Every install now does. **The cargo cache cost more to move than the work it replaced**: one `cargo-Linux-*` entry had reached **5,727 MB** — 57% of the repo's entire 10 GiB quota in a single key, which is the LRU-eviction pressure the previous fix here was about and did not remove — and restoring it took **127 seconds** against the 74s `cargo test` it existed to avoid. The cause is `path: target` taken literally: it archives every intermediate the workspace ever produced, including this workspace's own crates, which recompile in seconds and are the artifacts most likely to be stale. `Swatinem/rust-cache` caches the dependency artifacts and prunes the rest. **`rust-quality` ran in full on every pull request**, including the many that touch no Rust; its `Detect crates` gate was written for a stage-1 empty workspace and, with all three crates present, had been answering `true` unconditionally for months. It now also diffs the merge commit against its first parent, so it still reports a status — no `needs:` edge, nothing serialised behind it — while finishing in seconds on a TypeScript-only branch. `docs` gained the same gate, and its `mintlify` install is pinned to `4.2.680` to match `translate-docs.yml`, where floating meant an upstream release could redden a branch that changed nothing. **And 190 of 208 unit test files were building a jsdom they never touched**: the config set `environment: "jsdom"` globally for the sake of 16 React files and two more that already opt in per-file, and the `test` matrix runs the suite three times, so it was paid three times per run. Split into `node`/`dom` projects on the file extension, jsdom construction drops from **40.96s to 13.67s** measured locally, and a new `.test.tsx` still gets a DOM without anyone remembering to ask. The release hang is the same story in one step: **nothing in this repo had a job timeout except `integration-suite.yml`**, so when the linux-x64 daemon leg hit a stalled Azure mirror on 2026-08-19 it sat in `apt-get update` through three runner re-dispatches with v1.0.1 blocked behind it, while the arm64 leg ran the identical step in seconds. Every job across five workflows now declares one; the apt step is retried and given real acquire timeouts (its defaults are long enough to be no timeout at all against a stall) and loses its `-qq`, which was suppressing the one thing worth having in the log — which mirror stalled. `build-daemon.yml` also gains a concurrency group, scoped to `pull_request` so the `workflow_call` legs that *are* a release's binaries are never cancelled. `musl-tools` itself stays: `-p failproofaid` reaches `rusqlite` with `bundled` and `ring` through rustls, so the `cc` crate needs `musl-gcc` on both musl legs. None of these turn CI red on their own, which is why `release-pipeline.test.ts` now asserts all four — the timeouts, the `--ignore-scripts`, the absence of a bare `target` cache path, and the bounded apt step. Net: **~4.0 min wall and ~15.7 runner-minutes per pull request down to ~1.7 min and ~9**, with ~5 GiB of cache quota returned. (#726) + - Let the box pick the translation model per tier, and stop a re-install double-scheduling the box. `getModelForTier` now reads `TRANSLATE_MODEL_TIER1` / `TRANSLATE_MODEL_TIER23`, so the seven languages most readers actually arrive in can keep a strong model while the long tail runs on something cheap — the CLI's `--model` flag flattens every tier to one model, which is the opposite of what the tier split exists for. Any id the gateway serves over the Anthropic `/v1/messages` shape works, since that is the API the translator speaks (verified: `deepseek-v4-pro` and `deepseek-v4-flash` both answer there). Separately, `install.sh` now strips the pre-marker cron form as well as its own marker: a box set up before the marker existed carries a long-form inline `docker run … -e CANARY_JOB=` line, and matching only the marker left it in place — six entries, every job scheduled twice, one on the old image and one on the new. The per-job flock keeps that from doing damage and turns it into something worse to diagnose: which image runs becomes a coin toss. Found on the real box, whose crontab is exactly that shape. (#705) - Make a non-PASS canary verdict explain itself. `probe-cli.sh` captured each agent's stdout and stderr into `$OUTA`/`$OUTB`, used them for two greps, and threw them away; `run.sh` then echoed `tail -20` of the probe on any non-PASS verdict — and the last 20 lines of that probe are the verdict block, so the log restated the verdict instead of giving the cause. Four CLIs sat yellow on the box for three consecutive days with nothing recorded anywhere but the word INCONCLUSIVE, and re-running produced the same nothing because the evidence was discarded both times. Each failing probe now prints the last 25 lines of what the CLI actually said, plus whether a hook fired at all, and the tail window widens to 80 so the explanation lands inside it. The daemon note is corrected in the same breath: `daemon: routed, no fail-closed denies` was printed whenever the grep for `daemon-unreachable` found nothing, which is equally what **no hook log at all** looks like — a run where the daemon was never asked anything now says so instead of claiming a real evaluation. (#705) diff --git a/__tests__/ci/release-pipeline.test.ts b/__tests__/ci/release-pipeline.test.ts index d8de25b38..36b10ae02 100644 --- a/__tests__/ci/release-pipeline.test.ts +++ b/__tests__/ci/release-pipeline.test.ts @@ -646,3 +646,91 @@ describe("pipeline / CLI agreement", () => { }, ); }); + +/** + * CI cost guards. + * + * Four regressions, each a one-line edit away, none of which turns CI red. They + * only make it slower — and nothing watches that, which is why they need a test + * rather than a convention: + * + * - package.json's `prepare` is `bun run build`, so any `bun install` without + * `--ignore-scripts` fires a full Next.js production build as an install + * lifecycle hook. That was ~28s in six of ci.yml's eight jobs plus twice + * more per release, every second of it discarded; + * - a job with no `timeout-minutes` inherits GitHub's six-hour default. On + * 2026-08-19 the linux-x64 daemon leg sat in `apt-get update` against a + * stalled Azure mirror through three runner re-dispatches, with a release + * blocked behind it, because nothing bounded it; + * - `path: target` in a cargo cache archives the entire build directory. One + * such entry reached 5,727 MB — 57% of the repo's whole 10 GiB quota, which + * keeps the store in permanent LRU eviction — and took 127s to restore + * against the 74s of compilation it was there to save; + * - cancel-in-progress on build-daemon must stay scoped to `pull_request`, + * because the `workflow_call` legs ARE the release's binaries. + */ +describe("CI cost guards", () => { + const COST_GUARDED = ["ci.yml", "publish.yml", "build-daemon.yml", "osv-scanner.yml", "build-image.yml"]; + + /** Every shell command a job runs, including those wrapped by nick-fields/retry. */ + function shellText(job: Record): string { + return (job.steps ?? []) + .map((s: Record) => [s.run ?? "", s.with?.command ?? ""].join("\n")) + .join("\n"); + } + + it.each(COST_GUARDED)("%s gives every job a timeout-minutes", (name) => { + const jobs: [string, Record][] = Object.entries(workflow(name).jobs); + const unbounded = jobs + // A `uses:` job calls a reusable workflow and cannot carry a timeout of + // its own; that workflow's jobs are covered by their own row here. + .filter(([, job]) => !job.uses && typeof job["timeout-minutes"] !== "number") + .map(([id]) => id); + expect(unbounded).toEqual([]); + }); + + it.each(["ci.yml", "publish.yml"])("%s never lets an install run the prepare hook", (name) => { + const offenders: string[] = []; + for (const [id, job] of Object.entries(workflow(name).jobs) as [string, Record][]) { + for (const line of shellText(job).split("\n")) { + if (line.includes("bun install") && !line.includes("--ignore-scripts")) { + offenders.push(`${name} / ${id}: ${line.trim()}`); + } + } + } + expect(offenders).toEqual([]); + }); + + it.each(["ci.yml", "build-daemon.yml"])("%s caches cargo without archiving target/", (name) => { + const steps: Record[] = Object.values(workflow(name).jobs).flatMap( + (j: any) => j.steps ?? [], + ); + const archivesTarget = steps + .filter((s) => String(s.uses ?? "").startsWith("actions/cache")) + .filter((s) => + String(s.with?.path ?? "") + .split("\n") + .some((line) => line.trim() === "target"), + ); + expect(archivesTarget).toEqual([]); + expect(steps.some((s) => String(s.uses ?? "").startsWith("Swatinem/rust-cache"))).toBe(true); + }); + + it("bounds and retries the musl toolchain install", () => { + const step = workflow("build-daemon.yml").jobs.build.steps.find((s: Record) => + String(s.name ?? "").includes("musl toolchain"), + ); + // A bare `run:` is what hung: apt's default acquire timeouts are long + // enough to be no timeout at all against a stalled mirror. + expect(step.run).toBeUndefined(); + expect(String(step.uses)).toContain("nick-fields/retry"); + expect(step.with.timeout_minutes).toBeLessThanOrEqual(5); + expect(step.with.command).toContain("Acquire::http::Timeout"); + }); + + it("supersedes a superseded daemon build without cancelling a release", () => { + const c = workflow("build-daemon.yml").concurrency; + expect(c.group).toContain("github.ref"); + expect(String(c["cancel-in-progress"])).toContain("pull_request"); + }); +}); diff --git a/__tests__/lib/client-telemetry.test.ts b/__tests__/lib/client-telemetry.test.ts index 1d9a76ae7..f3dcb7118 100644 --- a/__tests__/lib/client-telemetry.test.ts +++ b/__tests__/lib/client-telemetry.test.ts @@ -1,3 +1,7 @@ +// @vitest-environment jsdom +// captureClientEvent reads window.location for $current_url/$pathname, so this +// one needs a DOM despite being a .ts file. Same per-file opt-in as +// share-card.test.ts and fetch-with-timeout.test.ts. import { describe, it, expect, vi, beforeEach } from "vitest"; import { setClientTelemetryConfig, diff --git a/vitest.config.mts b/vitest.config.mts index e81a91563..273bbb45c 100644 --- a/vitest.config.mts +++ b/vitest.config.mts @@ -2,6 +2,32 @@ import { defineConfig } from "vitest/config"; import react from "@vitejs/plugin-react"; import { resolve } from "path"; +/** + * Two projects, split by what a test actually needs. + * + * This suite is 208 files, of which 16 render React and two more reach for a + * DOM API directly. The other 190 are Node-side: hook handlers, policy + * evaluation, session parsers, workflow assertions. Running the whole thing + * under a single global `environment: "jsdom"` built a fresh jsdom for every + * one of those 190 files, and the `test` job runs the suite three times over + * (once per env-config matrix leg), so the waste was paid three times per CI + * run to serve nothing. + * + * Splitting on the extension keeps it automatic: a new `.test.tsx` gets jsdom + * without anyone having to remember to ask for it, and a new `.test.ts` gets + * the fast path by default. The escape hatch for the exceptions is per-file and + * already in use — `__tests__/lib/share-card.test.ts` and + * `__tests__/lib/fetch-with-timeout.test.ts` both open with + * `// @vitest-environment jsdom`, which overrides the project's environment for + * that file alone. Prefer that docblock over widening the globs below. + * + * `extends: true` pulls the root config — plugins, alias, globals, setupFiles, + * env — into both projects, so there is exactly one place to change any of it. + * `__tests__/setup.ts` is safe on the node side: its Web Storage polyfill + * installs only when the runtime has not supplied a working one, so under + * `node` it simply provides the Storage the DOM tests would have got from + * jsdom, and under jsdom it stays out of the way. + */ export default defineConfig({ plugins: [react()], resolve: { @@ -11,13 +37,30 @@ export default defineConfig({ }, test: { globals: true, - environment: "jsdom", setupFiles: ["__tests__/setup.ts"], - include: ["__tests__/**/*.test.{ts,tsx}"], - exclude: ["__tests__/e2e/**"], css: false, env: { FAILPROOFAI_TELEMETRY_DISABLED: "1", }, + projects: [ + { + extends: true, + test: { + name: "node", + environment: "node", + include: ["__tests__/**/*.test.ts"], + exclude: ["__tests__/e2e/**"], + }, + }, + { + extends: true, + test: { + name: "dom", + environment: "jsdom", + include: ["__tests__/**/*.test.tsx"], + exclude: ["__tests__/e2e/**"], + }, + }, + ], }, }); From b15ce237e621659478d68234de7215c9705c1590 Mon Sep 17 00:00:00 2001 From: NiveditJain Date: Wed, 19 Aug 2026 14:11:36 +0530 Subject: [PATCH 2/3] Fix two things the first CI run found MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The retry action cannot bound a privileged command. nick-fields/retry kills a timed-out step's process tree as the runner user, and apt runs as root: the 4-minute timeout fired exactly as designed and the action then died with `kill EPERM` instead of retrying, turning a recoverable stall into a failed leg. Moving `timeout` inside the privilege escalation puts the killer on the same side of that boundary as the process it has to kill. That run also confirmed the stall is real rather than a one-off — every azure.archive.ubuntu.com line came back Ign, apt fell back to archive.ubuntu.com, fetched InRelease and then sat 3.5 minutes emitting nothing. So the step now tries `apt-get install` before `apt-get update` at all: the refresh is the part that stalls, and the runner image's package lists usually make it unnecessary. Separately, the test job does need dist/index.js — the custom-policy loader tests resolve `import from 'failproofai'` through findDistIndex(), which the prepare hook used to build as a side effect. One bun bundle, ~3ms, against the ~28s Next build it replaces. Worth noting how it got through: running the WHOLE suite with dist/ moved aside passes, because an earlier test writes that file before the loader tests read it. Only running them alone fails. Test-order luck read as a clean verification. The musl drift guard is updated rather than added to, since the mechanism it pinned is exactly what this commit disproves; it now asserts the ordering that carries the fix. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01KbT6esM8A8mkSymH13keLt --- .github/workflows/build-daemon.yml | 56 +++++++++++++++++++-------- .github/workflows/ci.yml | 12 ++++++ CHANGELOG.md | 2 +- __tests__/ci/release-pipeline.test.ts | 24 ++++++++---- 4 files changed, 70 insertions(+), 24 deletions(-) diff --git a/.github/workflows/build-daemon.yml b/.github/workflows/build-daemon.yml index 81fc233c3..19fcebfb8 100644 --- a/.github/workflows/build-daemon.yml +++ b/.github/workflows/build-daemon.yml @@ -125,25 +125,49 @@ jobs: # crate resolves `musl-gcc` on both musl legs. # # Retried and bounded, because the failure mode here is a HANG rather than - # an error. apt's default acquire timeouts are long enough to be no - # timeout at all against a stalled mirror, so without these the leg would - # spend the entire job budget above on one bad Azure endpoint instead of - # four minutes and a retry against a different one. `-qq` is gone on - # purpose too: it suppressed the per-mirror progress that says WHICH - # endpoint stalled, which is the one thing worth having in the log. + # an error, and apt's own acquire timeouts do not reliably catch it: on + # 2026-08-19 every `azure.archive.ubuntu.com` line came back `Ign`, apt + # fell back to `archive.ubuntu.com`, fetched the InRelease files, and then + # sat for three and a half minutes emitting nothing. + # + # `sudo timeout`, NOT `nick-fields/retry` — that was the first attempt and + # CI rejected it. The action bounds a step by killing its process tree + # from the runner user, and apt runs as root: the timeout fired correctly + # at four minutes and the action then died with `kill EPERM` instead of + # retrying, turning a recoverable stall into a failed leg. Putting + # `timeout` INSIDE the sudo is what makes the killer root too. + # + # The fast path skips the network altogether. `apt-get update` is the step + # that stalls, and it is only needed when the runner image's package lists + # cannot satisfy the install — so try the install first and refresh only + # on failure. `-qq` is gone on purpose as well: it suppressed the + # per-mirror progress that says WHICH endpoint stalled. - name: Install the musl toolchain if: contains(matrix.target, 'musl') - uses: nick-fields/retry@v4 - with: - max_attempts: 3 - timeout_minutes: 4 - command: | - sudo apt-get update \ + run: | + install_musl() { + sudo timeout -k 10 90 apt-get install -y --no-install-recommends musl-tools + } + if install_musl; then + musl-gcc --version | head -1 + exit 0 + fi + for attempt in 1 2 3; do + echo "::group::apt-get update (attempt $attempt)" + sudo timeout -k 10 120 apt-get update \ -o Acquire::Retries=3 \ - -o Acquire::http::Timeout=20 \ - -o Acquire::https::Timeout=20 \ - -o Acquire::Languages=none - sudo apt-get install -y --no-install-recommends musl-tools + -o Acquire::http::Timeout=15 \ + -o Acquire::https::Timeout=15 \ + -o Acquire::Languages=none || echo "attempt $attempt timed out or failed" + echo "::endgroup::" + if install_musl; then + musl-gcc --version | head -1 + exit 0 + fi + sleep 5 + done + echo "::error::musl-tools could not be installed after 3 attempts — see the apt output above for the stalling mirror" + exit 1 # `save-if` rather than a plain cache, for the reason the split # restore/save here used to carry: this job runs on `pull_request` AND on diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 759a2c929..49f5b6190 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -296,6 +296,18 @@ jobs: with: node-version: "22" + # The custom-policy loader tests resolve `import ... from 'failproofai'` + # inside a generated .mjs through findDistIndex(), which needs a real + # dist/index.js on disk — so with `--ignore-scripts` above, the `prepare` + # hook is no longer there to have built it. Three milliseconds of bun + # bundling, against the ~28s full Next build it replaces. + # + # This is also why running the whole suite is a bad way to check the + # dependency: some earlier test writes that file, so the loader tests pass + # on ordering alone when run together and fail when run on their own. + - name: Build the policy-loader fixture + run: bun build src/index.ts --outdir dist --target node --format cjs + - name: Test (${{ matrix.env-config.name }}) uses: nick-fields/retry@v4 env: ${{ matrix.env-config.env }} diff --git a/CHANGELOG.md b/CHANGELOG.md index 411e765fd..7424e9e16 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,7 +14,7 @@ ### Fixes -- **Stop CI paying for work it throws away, and stop a stalled apt mirror holding a release for six hours.** Four costs, found by measuring a green run rather than a red one. **`bun install` was running a full Next.js production build**: `package.json`'s `prepare` is `bun run build`, which bun fires as an install lifecycle hook, so six of `ci.yml`'s eight jobs spent ~28s (14s compile + 13s TypeScript) building an application they never read — and the `build` job did it twice, since its own Build step then re-ran the same thing warm in 7s. `rust-quality` has passed `--ignore-scripts` since it landed and installs in **one second**, which is the control that proves the rest; `translate-docs.yml` already guards the same way with a comment naming this exact hazard. Every install now does. **The cargo cache cost more to move than the work it replaced**: one `cargo-Linux-*` entry had reached **5,727 MB** — 57% of the repo's entire 10 GiB quota in a single key, which is the LRU-eviction pressure the previous fix here was about and did not remove — and restoring it took **127 seconds** against the 74s `cargo test` it existed to avoid. The cause is `path: target` taken literally: it archives every intermediate the workspace ever produced, including this workspace's own crates, which recompile in seconds and are the artifacts most likely to be stale. `Swatinem/rust-cache` caches the dependency artifacts and prunes the rest. **`rust-quality` ran in full on every pull request**, including the many that touch no Rust; its `Detect crates` gate was written for a stage-1 empty workspace and, with all three crates present, had been answering `true` unconditionally for months. It now also diffs the merge commit against its first parent, so it still reports a status — no `needs:` edge, nothing serialised behind it — while finishing in seconds on a TypeScript-only branch. `docs` gained the same gate, and its `mintlify` install is pinned to `4.2.680` to match `translate-docs.yml`, where floating meant an upstream release could redden a branch that changed nothing. **And 190 of 208 unit test files were building a jsdom they never touched**: the config set `environment: "jsdom"` globally for the sake of 16 React files and two more that already opt in per-file, and the `test` matrix runs the suite three times, so it was paid three times per run. Split into `node`/`dom` projects on the file extension, jsdom construction drops from **40.96s to 13.67s** measured locally, and a new `.test.tsx` still gets a DOM without anyone remembering to ask. The release hang is the same story in one step: **nothing in this repo had a job timeout except `integration-suite.yml`**, so when the linux-x64 daemon leg hit a stalled Azure mirror on 2026-08-19 it sat in `apt-get update` through three runner re-dispatches with v1.0.1 blocked behind it, while the arm64 leg ran the identical step in seconds. Every job across five workflows now declares one; the apt step is retried and given real acquire timeouts (its defaults are long enough to be no timeout at all against a stall) and loses its `-qq`, which was suppressing the one thing worth having in the log — which mirror stalled. `build-daemon.yml` also gains a concurrency group, scoped to `pull_request` so the `workflow_call` legs that *are* a release's binaries are never cancelled. `musl-tools` itself stays: `-p failproofaid` reaches `rusqlite` with `bundled` and `ring` through rustls, so the `cc` crate needs `musl-gcc` on both musl legs. None of these turn CI red on their own, which is why `release-pipeline.test.ts` now asserts all four — the timeouts, the `--ignore-scripts`, the absence of a bare `target` cache path, and the bounded apt step. Net: **~4.0 min wall and ~15.7 runner-minutes per pull request down to ~1.7 min and ~9**, with ~5 GiB of cache quota returned. (#726) +- **Stop CI paying for work it throws away, and stop a stalled apt mirror holding a release for six hours.** Four costs, found by measuring a green run rather than a red one. **`bun install` was running a full Next.js production build**: `package.json`'s `prepare` is `bun run build`, which bun fires as an install lifecycle hook, so six of `ci.yml`'s eight jobs spent ~28s (14s compile + 13s TypeScript) building an application they never read — and the `build` job did it twice, since its own Build step then re-ran the same thing warm in 7s. `rust-quality` has passed `--ignore-scripts` since it landed and installs in **one second**, which is the control that proves the rest; `translate-docs.yml` already guards the same way with a comment naming this exact hazard. Every install now does. **The cargo cache cost more to move than the work it replaced**: one `cargo-Linux-*` entry had reached **5,727 MB** — 57% of the repo's entire 10 GiB quota in a single key, which is the LRU-eviction pressure the previous fix here was about and did not remove — and restoring it took **127 seconds** against the 74s `cargo test` it existed to avoid. The cause is `path: target` taken literally: it archives every intermediate the workspace ever produced, including this workspace's own crates, which recompile in seconds and are the artifacts most likely to be stale. `Swatinem/rust-cache` caches the dependency artifacts and prunes the rest. **`rust-quality` ran in full on every pull request**, including the many that touch no Rust; its `Detect crates` gate was written for a stage-1 empty workspace and, with all three crates present, had been answering `true` unconditionally for months. It now also diffs the merge commit against its first parent, so it still reports a status — no `needs:` edge, nothing serialised behind it — while finishing in seconds on a TypeScript-only branch. `docs` gained the same gate, and its `mintlify` install is pinned to `4.2.680` to match `translate-docs.yml`, where floating meant an upstream release could redden a branch that changed nothing. **And 190 of 208 unit test files were building a jsdom they never touched**: the config set `environment: "jsdom"` globally for the sake of 16 React files and two more that already opt in per-file, and the `test` matrix runs the suite three times, so it was paid three times per run. Split into `node`/`dom` projects on the file extension, jsdom construction drops from **40.96s to 13.67s** measured locally, and a new `.test.tsx` still gets a DOM without anyone remembering to ask. The release hang is the same story in one step: **nothing in this repo had a job timeout except `integration-suite.yml`**, so when the linux-x64 daemon leg hit a stalled Azure mirror on 2026-08-19 it sat in `apt-get update` through three runner re-dispatches with v1.0.1 blocked behind it, while the arm64 leg ran the identical step in seconds. Every job across five workflows now declares one; the apt step is retried and given real acquire timeouts (its defaults are long enough to be no timeout at all against a stall) and loses its `-qq`, which was suppressing the one thing worth having in the log — which mirror stalled. `build-daemon.yml` also gains a concurrency group, scoped to `pull_request` so the `workflow_call` legs that *are* a release's binaries are never cancelled. `musl-tools` itself stays: `-p failproofaid` reaches `rusqlite` with `bundled` and `ring` through rustls, so the `cc` crate needs `musl-gcc` on both musl legs. **The bound is `sudo timeout`, not `nick-fields/retry`**, and that distinction is the whole fix rather than a style choice — the retry action was tried first and CI rejected it: it bounds a step by killing the process tree **as the runner user**, and apt runs as root, so the four-minute timeout fired exactly as designed and the action then died with `kill EPERM` instead of retrying, converting a recoverable stall into a failed leg. `timeout` inside the `sudo` makes the killer root too. The same run also showed the stall is real and not a one-off: every `azure.archive.ubuntu.com` line came back `Ign`, apt fell back to `archive.ubuntu.com`, fetched the InRelease files and then sat for three and a half minutes emitting nothing — so the step now tries `apt-get install` **before** `apt-get update` at all, since the refresh is the part that stalls and the runner image's package lists usually make it unnecessary. Dropping the `prepare` hook does cost the `test` job one thing it was silently getting: the custom-policy loader tests resolve `import ... from 'failproofai'` through `findDistIndex()` and need a real `dist/index.js`, so the job now builds that one bundle explicitly — three milliseconds against the ~28s it replaces. Worth recording how that surfaced, because the check that should have caught it did not: running the **whole suite** with `dist/` moved aside passes, since an earlier test writes the file before the loader tests read it, and only running them alone fails. Test-order luck read as a clean bill of health. None of these regressions turns CI red on its own either, which is why `release-pipeline.test.ts` now asserts all four — the timeouts, the `--ignore-scripts`, the absence of a bare `target` cache path, and an apt step that is bounded by a killer running as the same user apt does. Net: **~4.0 min wall and ~15.7 runner-minutes per pull request down to ~1.7 min and ~9**, with ~5 GiB of cache quota returned. (#726) - Let the box pick the translation model per tier, and stop a re-install double-scheduling the box. `getModelForTier` now reads `TRANSLATE_MODEL_TIER1` / `TRANSLATE_MODEL_TIER23`, so the seven languages most readers actually arrive in can keep a strong model while the long tail runs on something cheap — the CLI's `--model` flag flattens every tier to one model, which is the opposite of what the tier split exists for. Any id the gateway serves over the Anthropic `/v1/messages` shape works, since that is the API the translator speaks (verified: `deepseek-v4-pro` and `deepseek-v4-flash` both answer there). Separately, `install.sh` now strips the pre-marker cron form as well as its own marker: a box set up before the marker existed carries a long-form inline `docker run … -e CANARY_JOB=` line, and matching only the marker left it in place — six entries, every job scheduled twice, one on the old image and one on the new. The per-job flock keeps that from doing damage and turns it into something worse to diagnose: which image runs becomes a coin toss. Found on the real box, whose crontab is exactly that shape. (#705) diff --git a/__tests__/ci/release-pipeline.test.ts b/__tests__/ci/release-pipeline.test.ts index 36b10ae02..28267ae13 100644 --- a/__tests__/ci/release-pipeline.test.ts +++ b/__tests__/ci/release-pipeline.test.ts @@ -716,16 +716,26 @@ describe("CI cost guards", () => { expect(steps.some((s) => String(s.uses ?? "").startsWith("Swatinem/rust-cache"))).toBe(true); }); - it("bounds and retries the musl toolchain install", () => { + it("bounds and retries the musl toolchain install, with a root-owned killer", () => { const step = workflow("build-daemon.yml").jobs.build.steps.find((s: Record) => String(s.name ?? "").includes("musl toolchain"), ); - // A bare `run:` is what hung: apt's default acquire timeouts are long - // enough to be no timeout at all against a stalled mirror. - expect(step.run).toBeUndefined(); - expect(String(step.uses)).toContain("nick-fields/retry"); - expect(step.with.timeout_minutes).toBeLessThanOrEqual(5); - expect(step.with.command).toContain("Acquire::http::Timeout"); + const script = String(step.run ?? ""); + + // `sudo timeout`, in that order, is the whole point and the reason this + // assertion is this specific. The first version of this step used + // nick-fields/retry, which bounds a step by killing its process tree AS THE + // RUNNER USER — and apt runs as root, so the four-minute timeout fired + // correctly and the action then died with `kill EPERM` instead of retrying. + // `timeout` inside the sudo is what makes the killer root as well. + expect(step.uses).toBeUndefined(); + expect(script).toMatch(/sudo timeout\b/); + expect(script).not.toMatch(/timeout\s+\d+\s+sudo\b/); + + // Bounded per attempt, retried, and loud about which mirror stalled. + expect(script).toContain("Acquire::http::Timeout"); + expect(script).toContain("for attempt in"); + expect(script).not.toContain("-qq"); }); it("supersedes a superseded daemon build without cancelling a release", () => { From 7d4b63d5dfa26bd573ba899b8f2aa55f8abe2daf Mon Sep 17 00:00:00 2001 From: NiveditJain Date: Wed, 19 Aug 2026 14:20:23 +0530 Subject: [PATCH 3/3] Widen the Rust gate to the TS worker inputs, per review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The gate matched Rust paths only, and cargo test --workspace is not a Rust-only job: it spawns the real TS worker (bun bin/failproofai-worker.mjs, from server.rs's live end-to-end test), which runs raw TypeScript and resolves src/hooks' real dependency tree at runtime rather than a bundle's. So a change under src/hooks, to the worker entrypoint, or to the dependency graph it resolves against can break the job with no Rust involved — surfacing as "worker process exited before creating its socket". A gate matching only Rust paths would have skipped exactly that, which is the fail-open risk this change was flagged for. src/hooks/, bin/failproofai-worker.mjs, package.json and bun.lock join the pattern. The docs gate gains bun.lock for the same class of reason: validate:mdx is a bun script, so a lockfile-only change moves the dependency graph it parses with. The docs checkout gets persist-credentials: false. That job runs `npm install -g mintlify@4.2.680`, whose lifecycle scripts could read a token the default checkout leaves in .git/config — the same reasoning rust-quality and build-daemon already carry, and the job where it matters most. The musl drift guard now asserts the install-first ORDER and that the install is bounded too, not just that both strings appear. Reversing the two would put the stalling step back on the fast path with every previous assertion still green, which makes ordering the thing actually worth pinning. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01KbT6esM8A8mkSymH13keLt --- .github/workflows/ci.yml | 25 +++++++++++++++++++++---- __tests__/ci/release-pipeline.test.ts | 11 +++++++++++ 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 49f5b6190..2971a18ec 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -158,9 +158,18 @@ jobs: echo "present=true" >> "$GITHUB_OUTPUT" exit 0 fi - # Anything that can change what clippy or the tests see. ci.yml is in - # the list so a change to this gate always exercises the gate. - if git diff --name-only HEAD^1 HEAD | grep -qE '^(crates/|Cargo\.toml$|Cargo\.lock$|rust-toolchain\.toml$|\.github/workflows/ci\.yml$)'; then + # Anything that can change what clippy or the tests see — which is + # NOT only Rust. `cargo test --workspace` spawns the real TS worker + # (`bun bin/failproofai-worker.mjs`, from server.rs's live end-to-end + # test), and that worker runs raw TypeScript, resolving src/hooks' + # real dependency tree at runtime rather than a bundle's. So a change + # under src/hooks, to the worker entrypoint, or to the dependency + # graph it resolves against can break this job with no Rust involved + # at all — the failure surfaces as "worker process exited before + # creating its socket", which is exactly the class a gate that only + # matched Rust paths would have let through silently. + # ci.yml is in the list so a change to this gate always exercises it. + if git diff --name-only HEAD^1 HEAD | grep -qE '^(crates/|src/hooks/|bin/failproofai-worker\.mjs$|package\.json$|bun\.lock$|Cargo\.toml$|Cargo\.lock$|rust-toolchain\.toml$|\.github/workflows/ci\.yml$)'; then echo "present=true" >> "$GITHUB_OUTPUT" else echo "present=false" >> "$GITHUB_OUTPUT" @@ -358,6 +367,12 @@ jobs: # Depth 2 so the gate below can diff the PR's merge commit against # its first parent. See rust-quality for the full reasoning. fetch-depth: 2 + # This job runs `npm install -g mintlify`, which executes package + # lifecycle scripts. The default would leave GITHUB_TOKEN sitting in + # .git/config where any of them could read it — the same reasoning + # rust-quality and build-daemon already carry, and this is the job + # where it bites hardest. + persist-credentials: false # Same shape as rust-quality's gate, same reason: this job installs a # global npm package and a full dependency tree to validate documentation @@ -369,7 +384,9 @@ jobs: echo "present=true" >> "$GITHUB_OUTPUT" exit 0 fi - if git diff --name-only HEAD^1 HEAD | grep -qE '^(docs/|scripts/validate-mdx\.ts$|package\.json$|\.github/workflows/ci\.yml$)|\.mdx$'; then + # bun.lock alongside package.json: `validate:mdx` runs a bun script, + # so a lockfile-only change moves the dependency graph it parses with. + if git diff --name-only HEAD^1 HEAD | grep -qE '^(docs/|scripts/validate-mdx\.ts$|package\.json$|bun\.lock$|\.github/workflows/ci\.yml$)|\.mdx$'; then echo "present=true" >> "$GITHUB_OUTPUT" else echo "present=false" >> "$GITHUB_OUTPUT" diff --git a/__tests__/ci/release-pipeline.test.ts b/__tests__/ci/release-pipeline.test.ts index 28267ae13..a962a26a8 100644 --- a/__tests__/ci/release-pipeline.test.ts +++ b/__tests__/ci/release-pipeline.test.ts @@ -732,6 +732,17 @@ describe("CI cost guards", () => { expect(script).toMatch(/sudo timeout\b/); expect(script).not.toMatch(/timeout\s+\d+\s+sudo\b/); + // The install is bounded too, not just the update — it is the step that + // actually downloads packages, and an unbounded one stalls the same way. + expect(script).toMatch(/sudo timeout[^\n]*apt-get install/); + + // Install BEFORE the update loop. `apt-get update` is the part that + // stalled; the runner image's package lists usually make it unnecessary, + // so reversing these two would put the flaky step back on the fast path + // while every assertion above still passed. + expect(script.indexOf("if install_musl; then")).toBeGreaterThan(-1); + expect(script.indexOf("if install_musl; then")).toBeLessThan(script.indexOf("for attempt in")); + // Bounded per attempt, retried, and loud about which mirror stalled. expect(script).toContain("Acquire::http::Timeout"); expect(script).toContain("for attempt in");