From 370425fe60793ff401f02b6e23d23a6012fb67d0 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 12:52:41 +0000 Subject: [PATCH] chore(ci): nightly rerun-safety gate, job timeouts, compiled-tests-in-dist guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three CI changes, all lessons #4065 taught the hard way. CI configuration only. 1. Nightly rerun-safety gate. Every job in this repo runs on a fresh clone, which makes CI structurally incapable of seeing a suite that pollutes its own working tree and therefore passes exactly once — CI always runs pass #1, so it is always green. #4065 sat in the repo through every CI run it ever had and surfaced only because somebody ran the full suite twice in one checkout while doing unrelated work, where it looked like THEIR change had broken something. The new job runs the full suite twice in one tree with `--force` (turbo would otherwise replay the cache and report green without executing anything) and fails if pass 2 disagrees with pass 1. It also prints any `.objectstack/` directories left between passes, so a failure names a file instead of reading as flakiness. 2. `timeout-minutes` on all eight ci.yml jobs. There were none, so every job inherited GitHub's 6-hour default. On PR #4100 the Test Core job hung with no output for 80 minutes and would have held a runner for six hours — and the whole time the PR read as "still running" rather than broken, which is the worst failure mode a gate can have. Ceilings are ~3-4x the healthy observed duration, so a genuinely slow run still passes. 3. A build-output guard against compiled test files. A package built with plain `tsc` that does not exclude tests emits `dist/**/*.test.js`. `files: ["dist"]` then publishes them to npm — and, worse, a package with no vitest config COLLECTS those compiled copies alongside its sources, so every `src/**/*.test.ts` also runs as a stale `dist/**/*.test.js` frozen at the last build. @objectstack/cli shipped exactly that (81 test files / 849 tests where its sources hold 58 / 581) until #4065 excluded them. That silently defeats edits: a fix to a source test appears not to work because the run is still executing the pre-fix duplicate. Everything else builds with tsup, which emits only declared entry points — so this gate exists to stop the NEXT tsc-built package repeating it, not to re-check the one already fixed. Verified: both workflow files parse as YAML and every extracted `run:` block is valid bash; the dist guard reports clean on a correctly-built tree and catches a planted `dist/**/*.test.js`. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01L5q3BZMpdiAueHf2emhDY9 --- .changeset/ci-rerun-safety-and-timeouts.md | 38 ++++++++ .github/workflows/ci.yml | 40 ++++++++ .github/workflows/rerun-safety-nightly.yml | 108 +++++++++++++++++++++ 3 files changed, 186 insertions(+) create mode 100644 .changeset/ci-rerun-safety-and-timeouts.md create mode 100644 .github/workflows/rerun-safety-nightly.yml diff --git a/.changeset/ci-rerun-safety-and-timeouts.md b/.changeset/ci-rerun-safety-and-timeouts.md new file mode 100644 index 0000000000..a755109ab3 --- /dev/null +++ b/.changeset/ci-rerun-safety-and-timeouts.md @@ -0,0 +1,38 @@ +--- +--- + +chore(ci): a nightly rerun-safety gate, job timeouts, and a compiled-tests-in-dist guard + +Three CI changes, all of them lessons #4065 taught the hard way. No package +changes — CI configuration only. + +**1. Nightly rerun-safety gate (`rerun-safety-nightly.yml`).** Every job in this +repo runs on a fresh clone, which makes CI structurally incapable of seeing a +suite that pollutes its own working tree and therefore passes exactly once. CI +always runs pass #1, so it is always green. #4065 sat in the repo through every +CI run it ever had and surfaced only because somebody ran the full suite twice in +one checkout while doing unrelated work — where it looked like *their* change had +broken something. The new job runs the full suite twice in one tree with +`--force` (turbo would otherwise replay the cache and report green without +executing anything) and fails if the second pass disagrees with the first. It +also prints any `.objectstack/` directories left behind between passes, so a +failure names a file instead of reading as flakiness. + +**2. `timeout-minutes` on all eight `ci.yml` jobs.** There were none, so every +job inherited GitHub's 6-hour default. On PR #4100 the Test Core job hung with no +output for 80 minutes and would have held a runner for six hours — and the whole +time the PR read as "still running" rather than broken, which is the worst +failure mode a gate can have. Ceilings are ~3-4× the healthy observed duration, +so a genuinely slow run still passes. + +**3. A build-output guard against compiled test files.** A package built with +plain `tsc` that does not exclude tests emits `dist/**/*.test.js`. `files: +["dist"]` then publishes them to npm — and, worse, a package with no vitest +config *collects* those compiled copies alongside its sources, so every +`src/**/*.test.ts` also runs as a stale `dist/**/*.test.js` frozen at the last +build. `@objectstack/cli` shipped exactly that (81 test files / 849 tests where +its sources hold 58 / 581) until #4065 excluded them. That silently defeats +edits: a fix to a source test appears not to work because the run is still +executing the pre-fix duplicate. Everything else here builds with tsup, which +emits only declared entry points — so this gate exists to stop the *next* +tsc-built package repeating it, not to re-check the one already fixed. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d688e7592d..c0e8ca309e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -18,6 +18,7 @@ concurrency: jobs: filter: runs-on: ubuntu-latest + timeout-minutes: 10 permissions: contents: read pull-requests: read @@ -83,6 +84,7 @@ jobs: needs: filter if: needs.filter.outputs.core == 'true' runs-on: ubuntu-latest + timeout-minutes: 45 permissions: contents: read @@ -217,6 +219,7 @@ jobs: needs: filter if: needs.filter.outputs.core == 'true' runs-on: ubuntu-latest + timeout-minutes: 30 permissions: contents: read @@ -327,6 +330,7 @@ jobs: needs: filter if: needs.filter.outputs.core == 'true' runs-on: ubuntu-latest + timeout-minutes: 45 permissions: contents: read strategy: @@ -443,6 +447,7 @@ jobs: needs: dogfood if: always() runs-on: ubuntu-latest + timeout-minutes: 10 permissions: contents: read steps: @@ -473,6 +478,7 @@ jobs: needs: filter if: needs.filter.outputs.core == 'true' runs-on: ubuntu-latest + timeout-minutes: 30 permissions: contents: read @@ -524,6 +530,38 @@ jobs: - name: Build packages (excluding docs) run: pnpm build + # A package that builds with plain `tsc` and does not exclude tests emits + # `dist/**/*.test.js`. Two costs, and the second is the dangerous one: + # + # 1. `files: ["dist"]` publishes the tests to npm. + # 2. A package with no vitest config COLLECTS those compiled copies + # alongside the sources, so every `src/**/*.test.ts` also runs as a + # stale `dist/**/*.test.js` built at the last `pnpm build`. That + # silently defeats edits — a fix to a source test appears not to work + # because the run is still executing the pre-fix duplicate — and it + # lets a source test be edited to pass while its stale twin keeps + # asserting the old behaviour, with neither obviously wrong. + # + # `@objectstack/cli` shipped exactly that (81 test files / 849 tests where + # its sources hold 58 / 581) until #4065 excluded them. The rest of the + # repo builds with tsup, which emits only declared entry points — so this + # gate exists to stop the NEXT tsc-built package repeating it, not to + # re-check the one that was fixed. + - name: No compiled test files in any dist + run: | + set -o pipefail + found="$(find packages -type d -name node_modules -prune -o \ + -type f \( -name '*.test.js' -o -name '*.test.cjs' -o -name '*.test.mjs' \) \ + -path '*/dist/*' -print | sort)" + if [ -n "$found" ]; then + echo "::error::Compiled test files found in build output. A tsc-built package is" + echo "::error::missing a test exclude in its tsconfig.build.json, so these ship to" + echo "::error::npm AND run as stale duplicates of their own sources." + echo "$found" + exit 1 + fi + echo "OK — no compiled test files in any package dist." + - name: Verify build outputs run: | if [ ! -d "packages/spec/dist" ]; then @@ -587,6 +625,7 @@ jobs: needs: filter if: needs.filter.outputs.docs == 'true' runs-on: ubuntu-latest + timeout-minutes: 30 permissions: contents: read @@ -645,6 +684,7 @@ jobs: needs: filter if: needs.filter.outputs.generated == 'true' runs-on: ubuntu-latest + timeout-minutes: 20 permissions: contents: read diff --git a/.github/workflows/rerun-safety-nightly.yml b/.github/workflows/rerun-safety-nightly.yml new file mode 100644 index 0000000000..2c8a366676 --- /dev/null +++ b/.github/workflows/rerun-safety-nightly.yml @@ -0,0 +1,108 @@ +# Rerun safety, nightly. +# +# ## The blind spot this closes +# +# Every other job in this repo runs on a FRESH CLONE. That makes CI structurally +# incapable of seeing a whole class of defect: a suite that pollutes its own +# working tree and therefore passes exactly once. CI always runs pass #1, so it +# is always green; only a human or an agent running the suite twice in one +# checkout ever sees the failure. +# +# #4065 was exactly that. `InMemoryDriver` defaulted to `persistence: 'auto'`, +# which on Node means a file adapter — so a suite seeding two rows with fixed +# ids wrote them to `.objectstack/data/memory-driver.json`, and the next run +# loaded them back and asserted four. Run N read 2N rows. It sat in the repo +# green through every CI run it ever had, and surfaced only because someone ran +# the full suite a second time in the same tree while doing unrelated work — at +# which point it looked like their own change had broken something. +# +# This job runs the full suite TWICE in one checkout. The first pass is allowed +# to fail loudly like any other; what this gate is really asserting is that the +# SECOND pass agrees with the first. +# +# ## Why `--force` +# +# Turbo would replay cached results on the second pass and report green without +# executing anything, which is precisely the wrong answer here. `--force` +# bypasses the cache so the second pass genuinely re-runs. +name: Rerun Safety + +on: + workflow_dispatch: + schedule: + - cron: '0 4 * * *' # 04:00 UTC nightly — an hour before Spec Coverage + +permissions: + contents: read + +jobs: + rerun-safety: + name: Full suite twice in one working tree + runs-on: ubuntu-latest + # Two full passes of everything, dogfood included. Generous, but bounded — + # an unbounded job is what let a hung Test Core sit for GitHub's 6h default + # while the PR read as "still running" rather than broken. + timeout-minutes: 120 + steps: + - name: Checkout repository + uses: actions/checkout@v7 + + - name: Setup Node.js + uses: actions/setup-node@v7 + with: + node-version: '22' + + - name: Enable Corepack + run: corepack enable + + - name: Get pnpm store directory + shell: bash + run: echo "STORE_PATH=$(pnpm store path --silent)" >> $GITHUB_ENV + + # Restore-only: scheduled runs read main's store cache; the per-push + # workflows own saving it. + - name: Restore pnpm cache + uses: actions/cache/restore@v6 + with: + path: ${{ env.STORE_PATH }} + key: ${{ runner.os }}-pnpm-store-v3-${{ hashFiles('**/pnpm-lock.yaml') }} + restore-keys: | + ${{ runner.os }}-pnpm-store-v3- + + - name: Install dependencies + run: pnpm install --frozen-lockfile + + - name: Build packages + run: pnpm build + + # Deliberately NOT `continue-on-error`. If pass 1 is red the suite is + # simply broken on main and that is worth failing on; this gate adds pass + # 2, it does not replace the ordinary signal. + - name: Test suite — pass 1 + run: pnpm turbo run test --concurrency=4 --force + + # Informational only. A stray `.objectstack/` is how the #4065 class shows + # up on disk, so printing what pass 1 left behind turns a pass-2 failure + # from "something is flaky" into a named file to go look at. Some entries + # here are legitimate (an example app's own project directory), which is + # why this reports rather than fails — pass 2 is the assertion. + - name: Report on-disk state left by pass 1 + if: always() + run: | + echo "Directories named .objectstack under the checkout:" + find . -path ./node_modules -prune -o -type d -name '.objectstack' -print || true + echo + echo "Files inside them:" + find . -path ./node_modules -prune -o -type d -name '.objectstack' -exec find {} -type f \; || true + + - name: Test suite — pass 2 (same working tree) + run: | + set -o pipefail + if ! pnpm turbo run test --concurrency=4 --force; then + echo "::error::The suite passes once and fails on a second run in the same" + echo "::error::working tree. Something under test writes state into the tree and" + echo "::error::reads it back on the next run. Ordinary CI cannot see this — every" + echo "::error::other job is a fresh clone, so every other job is always run #1." + echo "::error::See the on-disk state reported above, and #4065 for the pattern." + exit 1 + fi