Skip to content

Commit a9801af

Browse files
authored
chore(ci): nightly rerun-safety gate, job timeouts, compiled-tests-in-dist guard (#4156)
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. 2. `timeout-minutes` on all eight ci.yml jobs. There were none, so every job inherited GitHub's 6-hour default — and a job stuck that way reads as "still running" rather than broken, the worst failure mode a gate can have. 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"]` publishes them, and a package with no vitest config COLLECTS them alongside its sources, so every `src/**/*.test.ts` also runs as a stale duplicate frozen at the last build. That silently defeats edits. @objectstack/cli shipped exactly that (81 test files / 849 tests where its sources hold 58 / 581) until #4065. Everything else builds with tsup, so this gate exists to stop the NEXT tsc-built package repeating it. Follow-up to #4100. Related: #4154.
1 parent 43fc039 commit a9801af

3 files changed

Lines changed: 186 additions & 0 deletions

File tree

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
---
2+
---
3+
4+
chore(ci): a nightly rerun-safety gate, job timeouts, and a compiled-tests-in-dist guard
5+
6+
Three CI changes, all of them lessons #4065 taught the hard way. No package
7+
changes — CI configuration only.
8+
9+
**1. Nightly rerun-safety gate (`rerun-safety-nightly.yml`).** Every job in this
10+
repo runs on a fresh clone, which makes CI structurally incapable of seeing a
11+
suite that pollutes its own working tree and therefore passes exactly once. CI
12+
always runs pass #1, so it is always green. #4065 sat in the repo through every
13+
CI run it ever had and surfaced only because somebody ran the full suite twice in
14+
one checkout while doing unrelated work — where it looked like *their* change had
15+
broken something. The new job runs the full suite twice in one tree with
16+
`--force` (turbo would otherwise replay the cache and report green without
17+
executing anything) and fails if the second pass disagrees with the first. It
18+
also prints any `.objectstack/` directories left behind between passes, so a
19+
failure names a file instead of reading as flakiness.
20+
21+
**2. `timeout-minutes` on all eight `ci.yml` jobs.** There were none, so every
22+
job inherited GitHub's 6-hour default. On PR #4100 the Test Core job hung with no
23+
output for 80 minutes and would have held a runner for six hours — and the whole
24+
time the PR read as "still running" rather than broken, which is the worst
25+
failure mode a gate can have. Ceilings are ~3-4× the healthy observed duration,
26+
so a genuinely slow run still passes.
27+
28+
**3. A build-output guard against compiled test files.** A package built with
29+
plain `tsc` that does not exclude tests emits `dist/**/*.test.js`. `files:
30+
["dist"]` then publishes them to npm — and, worse, a package with no vitest
31+
config *collects* those compiled copies alongside its sources, so every
32+
`src/**/*.test.ts` also runs as a stale `dist/**/*.test.js` frozen at the last
33+
build. `@objectstack/cli` shipped exactly that (81 test files / 849 tests where
34+
its sources hold 58 / 581) until #4065 excluded them. That silently defeats
35+
edits: a fix to a source test appears not to work because the run is still
36+
executing the pre-fix duplicate. Everything else here builds with tsup, which
37+
emits only declared entry points — so this gate exists to stop the *next*
38+
tsc-built package repeating it, not to re-check the one already fixed.

.github/workflows/ci.yml

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ concurrency:
1818
jobs:
1919
filter:
2020
runs-on: ubuntu-latest
21+
timeout-minutes: 10
2122
permissions:
2223
contents: read
2324
pull-requests: read
@@ -83,6 +84,7 @@ jobs:
8384
needs: filter
8485
if: needs.filter.outputs.core == 'true'
8586
runs-on: ubuntu-latest
87+
timeout-minutes: 45
8688
permissions:
8789
contents: read
8890

@@ -217,6 +219,7 @@ jobs:
217219
needs: filter
218220
if: needs.filter.outputs.core == 'true'
219221
runs-on: ubuntu-latest
222+
timeout-minutes: 30
220223
permissions:
221224
contents: read
222225

@@ -327,6 +330,7 @@ jobs:
327330
needs: filter
328331
if: needs.filter.outputs.core == 'true'
329332
runs-on: ubuntu-latest
333+
timeout-minutes: 45
330334
permissions:
331335
contents: read
332336
strategy:
@@ -443,6 +447,7 @@ jobs:
443447
needs: dogfood
444448
if: always()
445449
runs-on: ubuntu-latest
450+
timeout-minutes: 10
446451
permissions:
447452
contents: read
448453
steps:
@@ -473,6 +478,7 @@ jobs:
473478
needs: filter
474479
if: needs.filter.outputs.core == 'true'
475480
runs-on: ubuntu-latest
481+
timeout-minutes: 30
476482
permissions:
477483
contents: read
478484

@@ -524,6 +530,38 @@ jobs:
524530
- name: Build packages (excluding docs)
525531
run: pnpm build
526532

533+
# A package that builds with plain `tsc` and does not exclude tests emits
534+
# `dist/**/*.test.js`. Two costs, and the second is the dangerous one:
535+
#
536+
# 1. `files: ["dist"]` publishes the tests to npm.
537+
# 2. A package with no vitest config COLLECTS those compiled copies
538+
# alongside the sources, so every `src/**/*.test.ts` also runs as a
539+
# stale `dist/**/*.test.js` built at the last `pnpm build`. That
540+
# silently defeats edits — a fix to a source test appears not to work
541+
# because the run is still executing the pre-fix duplicate — and it
542+
# lets a source test be edited to pass while its stale twin keeps
543+
# asserting the old behaviour, with neither obviously wrong.
544+
#
545+
# `@objectstack/cli` shipped exactly that (81 test files / 849 tests where
546+
# its sources hold 58 / 581) until #4065 excluded them. The rest of the
547+
# repo builds with tsup, which emits only declared entry points — so this
548+
# gate exists to stop the NEXT tsc-built package repeating it, not to
549+
# re-check the one that was fixed.
550+
- name: No compiled test files in any dist
551+
run: |
552+
set -o pipefail
553+
found="$(find packages -type d -name node_modules -prune -o \
554+
-type f \( -name '*.test.js' -o -name '*.test.cjs' -o -name '*.test.mjs' \) \
555+
-path '*/dist/*' -print | sort)"
556+
if [ -n "$found" ]; then
557+
echo "::error::Compiled test files found in build output. A tsc-built package is"
558+
echo "::error::missing a test exclude in its tsconfig.build.json, so these ship to"
559+
echo "::error::npm AND run as stale duplicates of their own sources."
560+
echo "$found"
561+
exit 1
562+
fi
563+
echo "OK — no compiled test files in any package dist."
564+
527565
- name: Verify build outputs
528566
run: |
529567
if [ ! -d "packages/spec/dist" ]; then
@@ -587,6 +625,7 @@ jobs:
587625
needs: filter
588626
if: needs.filter.outputs.docs == 'true'
589627
runs-on: ubuntu-latest
628+
timeout-minutes: 30
590629
permissions:
591630
contents: read
592631

@@ -645,6 +684,7 @@ jobs:
645684
needs: filter
646685
if: needs.filter.outputs.generated == 'true'
647686
runs-on: ubuntu-latest
687+
timeout-minutes: 20
648688
permissions:
649689
contents: read
650690

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
# Rerun safety, nightly.
2+
#
3+
# ## The blind spot this closes
4+
#
5+
# Every other job in this repo runs on a FRESH CLONE. That makes CI structurally
6+
# incapable of seeing a whole class of defect: a suite that pollutes its own
7+
# working tree and therefore passes exactly once. CI always runs pass #1, so it
8+
# is always green; only a human or an agent running the suite twice in one
9+
# checkout ever sees the failure.
10+
#
11+
# #4065 was exactly that. `InMemoryDriver` defaulted to `persistence: 'auto'`,
12+
# which on Node means a file adapter — so a suite seeding two rows with fixed
13+
# ids wrote them to `.objectstack/data/memory-driver.json`, and the next run
14+
# loaded them back and asserted four. Run N read 2N rows. It sat in the repo
15+
# green through every CI run it ever had, and surfaced only because someone ran
16+
# the full suite a second time in the same tree while doing unrelated work — at
17+
# which point it looked like their own change had broken something.
18+
#
19+
# This job runs the full suite TWICE in one checkout. The first pass is allowed
20+
# to fail loudly like any other; what this gate is really asserting is that the
21+
# SECOND pass agrees with the first.
22+
#
23+
# ## Why `--force`
24+
#
25+
# Turbo would replay cached results on the second pass and report green without
26+
# executing anything, which is precisely the wrong answer here. `--force`
27+
# bypasses the cache so the second pass genuinely re-runs.
28+
name: Rerun Safety
29+
30+
on:
31+
workflow_dispatch:
32+
schedule:
33+
- cron: '0 4 * * *' # 04:00 UTC nightly — an hour before Spec Coverage
34+
35+
permissions:
36+
contents: read
37+
38+
jobs:
39+
rerun-safety:
40+
name: Full suite twice in one working tree
41+
runs-on: ubuntu-latest
42+
# Two full passes of everything, dogfood included. Generous, but bounded —
43+
# an unbounded job is what let a hung Test Core sit for GitHub's 6h default
44+
# while the PR read as "still running" rather than broken.
45+
timeout-minutes: 120
46+
steps:
47+
- name: Checkout repository
48+
uses: actions/checkout@v7
49+
50+
- name: Setup Node.js
51+
uses: actions/setup-node@v7
52+
with:
53+
node-version: '22'
54+
55+
- name: Enable Corepack
56+
run: corepack enable
57+
58+
- name: Get pnpm store directory
59+
shell: bash
60+
run: echo "STORE_PATH=$(pnpm store path --silent)" >> $GITHUB_ENV
61+
62+
# Restore-only: scheduled runs read main's store cache; the per-push
63+
# workflows own saving it.
64+
- name: Restore pnpm cache
65+
uses: actions/cache/restore@v6
66+
with:
67+
path: ${{ env.STORE_PATH }}
68+
key: ${{ runner.os }}-pnpm-store-v3-${{ hashFiles('**/pnpm-lock.yaml') }}
69+
restore-keys: |
70+
${{ runner.os }}-pnpm-store-v3-
71+
72+
- name: Install dependencies
73+
run: pnpm install --frozen-lockfile
74+
75+
- name: Build packages
76+
run: pnpm build
77+
78+
# Deliberately NOT `continue-on-error`. If pass 1 is red the suite is
79+
# simply broken on main and that is worth failing on; this gate adds pass
80+
# 2, it does not replace the ordinary signal.
81+
- name: Test suite — pass 1
82+
run: pnpm turbo run test --concurrency=4 --force
83+
84+
# Informational only. A stray `.objectstack/` is how the #4065 class shows
85+
# up on disk, so printing what pass 1 left behind turns a pass-2 failure
86+
# from "something is flaky" into a named file to go look at. Some entries
87+
# here are legitimate (an example app's own project directory), which is
88+
# why this reports rather than fails — pass 2 is the assertion.
89+
- name: Report on-disk state left by pass 1
90+
if: always()
91+
run: |
92+
echo "Directories named .objectstack under the checkout:"
93+
find . -path ./node_modules -prune -o -type d -name '.objectstack' -print || true
94+
echo
95+
echo "Files inside them:"
96+
find . -path ./node_modules -prune -o -type d -name '.objectstack' -exec find {} -type f \; || true
97+
98+
- name: Test suite — pass 2 (same working tree)
99+
run: |
100+
set -o pipefail
101+
if ! pnpm turbo run test --concurrency=4 --force; then
102+
echo "::error::The suite passes once and fails on a second run in the same"
103+
echo "::error::working tree. Something under test writes state into the tree and"
104+
echo "::error::reads it back on the next run. Ordinary CI cannot see this — every"
105+
echo "::error::other job is a fresh clone, so every other job is always run #1."
106+
echo "::error::See the on-disk state reported above, and #4065 for the pattern."
107+
exit 1
108+
fi

0 commit comments

Comments
 (0)