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