From 98d5d83e955d288f8da8049f5f9ade68c5bea489 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Fri, 5 Jun 2026 08:55:43 -0700 Subject: [PATCH 01/12] feat(test): add curated engine-core gate suite and test:gate command --- package.json | 3 +++ packages/cli/package.json | 1 + packages/engine/package.json | 1 + packages/engine/vitest.config.ts | 40 ++++++++++++++++++++++++++++++++ 4 files changed, 45 insertions(+) diff --git a/package.json b/package.json index 34a91cf147..def9829cae 100644 --- a/package.json +++ b/package.json @@ -16,6 +16,9 @@ "scripts": { "pretest": "node scripts/check-no-nohup.mjs && node scripts/check-no-kill-4040.mjs", "pretest:full": "node scripts/check-no-nohup.mjs && node scripts/check-no-kill-4040.mjs", + "pretest:gate": "node scripts/check-no-nohup.mjs && node scripts/check-no-kill-4040.mjs", + "test:gate": "pnpm --filter @fusion/engine test:core && pnpm --filter @runfusion/fusion test:ci-shape", + "smoke:boot": "node scripts/boot-smoke.mjs", "local": "node scripts/start-local.mjs", "dev": "node scripts/dev-with-memory.mjs", "dev:ui": "pnpm --filter @fusion/dashboard dev", diff --git a/packages/cli/package.json b/packages/cli/package.json index c314eff427..c0ad5d8b41 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -50,6 +50,7 @@ "build:exe:all": "bun run build.ts --all", "typecheck": "tsc --noEmit", "test": "vitest run --silent=passed-only --reporter=dot", + "test:ci-shape": "vitest run src/__tests__/ci-workflow.test.ts --silent=passed-only --reporter=dot", "test:slow-cli": "cross-env FUSION_TEST_SLOW_CLI=1 vitest run src/commands/__tests__/agent-export.test.ts --silent=passed-only --reporter=dot", "test:extension-integration": "cross-env FUSION_TEST_EXTENSION_INTEGRATION=1 vitest run src/__tests__/extension-integration.test.ts --silent=passed-only --reporter=dot", "test:build-exe": "cross-env FUSION_TEST_BUILD_EXE=1 vitest run --config vitest.build-exe.config.ts --silent=passed-only --reporter=dot", diff --git a/packages/engine/package.json b/packages/engine/package.json index 7282a2a2e0..6da6f356da 100644 --- a/packages/engine/package.json +++ b/packages/engine/package.json @@ -31,6 +31,7 @@ "build": "tsc", "typecheck": "tsc --noEmit", "test": "vitest run --silent=passed-only --reporter=dot --project=engine-default --project=engine-reliability", + "test:core": "vitest run --silent=passed-only --reporter=dot --project=engine-core", "test:slow": "vitest run --silent=passed-only --reporter=dot --project=engine-slow", "test:all": "vitest run --silent=passed-only --reporter=dot", "test:executor": "vitest run src/__tests__/executor-*.test.ts", diff --git a/packages/engine/vitest.config.ts b/packages/engine/vitest.config.ts index 7ccec53dcc..2c3511a630 100644 --- a/packages/engine/vitest.config.ts +++ b/packages/engine/vitest.config.ts @@ -48,6 +48,46 @@ export default defineConfig({ // does not inherit full-suite include and rerun everything single-threaded // (FN-5537: this caused long runs and external SIGTERM 143 kills). projects: [ + { + extends: true, + test: { + name: "engine-core", + // The curated merge-gate suite (see docs/testing.md "Merge gate"). + // Membership is an explicit allow-list, NOT a glob: tests earn their + // way in with evidence of value, and a flaky gate test is evicted by + // deleting its line here (no need for the flaky test to pass). + // Selection criteria: deterministic (no real git subprocesses, no + // real timers/network), fast (<~3s/file per scripts/test-timings.json), + // covering regression-prone core invariants: merge lifecycle and + // scope, files-changed/fork-point attribution, executor core paths, + // triage, scheduling, self-healing. + // Budget: the whole project must stay under ~60s wall-clock so the + // CI gate job's test run lands under ~1 minute. + include: [ + "src/__tests__/merger-merge-lifecycle.test.ts", + "src/__tests__/merger-post-merge.test.ts", + "src/__tests__/merger-conflict-resolution.test.ts", + "src/__tests__/merger-diff-scope.test.ts", + "src/__tests__/merger-file-scope-invariant.test.ts", + "src/__tests__/merger-landed-files-capture.test.ts", + "src/__tests__/branch-attribution.test.ts", + "src/__tests__/executor-core.test.ts", + "src/__tests__/executor-recovery.test.ts", + "src/__tests__/executor-base-commit-capture.test.ts", + "src/__tests__/executor-capture-modified-files-attribution.test.ts", + "src/__tests__/triage.test.ts", + "src/__tests__/triage-preflight.test.ts", + "src/__tests__/scheduler.test.ts", + "src/__tests__/scheduler-node-routing.test.ts", + "src/__tests__/scheduler-overlap-requeue.test.ts", + "src/__tests__/mission-scheduler.test.ts", + "src/__tests__/self-healing.test.ts", + "src/__tests__/heartbeat-monitor.test.ts", + "src/__tests__/workflow-node-handlers.test.ts", + ], + exclude: ["node_modules/**", "dist/**"], + }, + }, { extends: true, test: { From 9d00f533ea46dfb0327919d5eb25fde4c0378cfc Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Fri, 5 Jun 2026 08:57:24 -0700 Subject: [PATCH 02/12] feat(ci): add boot smoke check (CLI --help + real serve /api/health) --- scripts/boot-smoke.mjs | 161 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 161 insertions(+) create mode 100644 scripts/boot-smoke.mjs diff --git a/scripts/boot-smoke.mjs b/scripts/boot-smoke.mjs new file mode 100644 index 0000000000..0ffd16d583 --- /dev/null +++ b/scripts/boot-smoke.mjs @@ -0,0 +1,161 @@ +#!/usr/bin/env node +/** + * Boot smoke check — the merge gate's "the app starts and serves" proof. + * + * Verifies, against the *built* workspace (run `pnpm build` first): + * 1. The CLI answers `--help` with exit 0. + * 2. `fn serve` boots a real HTTP server on an ephemeral port and + * GET /api/health returns 200 within the timeout. + * 3. The server shuts down cleanly on SIGTERM. + * + * Safety properties (see scripts/check-no-kill-4040.mjs and AGENTS.md): + * - Never binds or touches port 4040 / FUSION_RESERVED_PORTS — an ephemeral + * port is requested from the OS (listen on 0) and double-checked against + * the reserved list. + * - Never kills anything except the child process it spawned itself. + * - Runs with an isolated $HOME (mkdtemp) so it cannot read or corrupt a + * developer's real fusion.db or auth state. + * + * Exit code is the verdict: 0 = boots and serves, non-zero = broken, with + * captured child stderr on stdout for CI logs. + */ + +import { spawn, spawnSync } from "node:child_process"; +import { mkdtempSync, rmSync } from "node:fs"; +import { createServer } from "node:net"; +import { tmpdir } from "node:os"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; + +const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), ".."); +const cliBin = path.join(repoRoot, "packages/cli/bin.mjs"); + +const HEALTH_TIMEOUT_MS = 60_000; +const SHUTDOWN_TIMEOUT_MS = 15_000; + +function parsePortList(raw) { + return String(raw ?? "") + .split(",") + .map((p) => Number.parseInt(p.trim(), 10)) + .filter((p) => Number.isInteger(p) && p > 0); +} + +const RESERVED_PORTS = new Set([4040, ...parsePortList(process.env.FUSION_RESERVED_PORTS)]); + +/** Ask the OS for a free ephemeral port, retrying if it lands on a reserved one. */ +async function getEphemeralPort() { + for (let attempt = 0; attempt < 10; attempt++) { + const port = await new Promise((resolve, reject) => { + const srv = createServer(); + srv.once("error", reject); + srv.listen(0, "127.0.0.1", () => { + const { port } = srv.address(); + srv.close(() => resolve(port)); + }); + }); + if (!RESERVED_PORTS.has(port)) return port; + } + throw new Error("could not obtain a non-reserved ephemeral port"); +} + +function fail(message, stderr = "") { + console.error(`boot-smoke: FAIL — ${message}`); + if (stderr.trim()) { + console.error("--- child stderr (tail) ---"); + console.error(stderr.split("\n").slice(-40).join("\n")); + } + process.exit(1); +} + +async function pollHealth(port, deadline) { + const url = `http://127.0.0.1:${port}/api/health`; + let lastError = "no response"; + while (Date.now() < deadline) { + try { + const res = await fetch(url, { signal: AbortSignal.timeout(2_000) }); + if (res.status === 200) return; + lastError = `HTTP ${res.status}`; + } catch (err) { + lastError = err?.cause?.code ?? err?.name ?? String(err); + } + await new Promise((r) => setTimeout(r, 500)); + } + throw new Error(`health check never returned 200 (last: ${lastError})`); +} + +async function main() { + // 1. CLI answers --help. + const help = spawnSync(process.execPath, [cliBin, "--help"], { + encoding: "utf8", + timeout: 30_000, + }); + if (help.status !== 0) { + fail(`\`fn --help\` exited ${help.status ?? `signal ${help.signal}`}`, help.stderr ?? ""); + } + if (!/serve/i.test(help.stdout ?? "")) { + fail("`fn --help` output does not mention the serve command", help.stderr ?? ""); + } + console.log("boot-smoke: `fn --help` OK"); + + // 2. Real server boot on an ephemeral port with an isolated HOME. + const port = await getEphemeralPort(); + const isolatedHome = mkdtempSync(path.join(tmpdir(), "fusion-boot-smoke-")); + let stderrBuf = ""; + + const child = spawn( + process.execPath, + [cliBin, "serve", "--port", String(port), "--host", "127.0.0.1"], + { + cwd: repoRoot, + env: { + ...process.env, + HOME: isolatedHome, + FUSION_SKIP_ONBOARDING: "1", + // Make sure nothing inherits a PORT that fights the explicit flag. + PORT: undefined, + }, + stdio: ["ignore", "pipe", "pipe"], + }, + ); + child.stderr.on("data", (d) => (stderrBuf += d)); + child.stdout.on("data", (d) => (stderrBuf += d)); + + const cleanup = () => { + if (child.exitCode === null && !child.killed) child.kill("SIGTERM"); + rmSync(isolatedHome, { recursive: true, force: true }); + }; + process.on("exit", cleanup); + + const exitedEarly = new Promise((resolve) => { + child.once("exit", (code, signal) => resolve({ code, signal })); + }); + + try { + await Promise.race([ + pollHealth(port, Date.now() + HEALTH_TIMEOUT_MS), + exitedEarly.then(({ code, signal }) => { + throw new Error(`server exited before becoming healthy (${code ?? `signal ${signal}`})`); + }), + ]); + } catch (err) { + fail(err.message, stderrBuf); + } + console.log(`boot-smoke: GET /api/health 200 on :${port}`); + + // 3. Clean shutdown of OUR child only. + child.kill("SIGTERM"); + const { code, signal } = await Promise.race([ + exitedEarly, + new Promise((resolve) => + setTimeout(() => resolve({ code: null, signal: "timeout" }), SHUTDOWN_TIMEOUT_MS), + ), + ]); + if (signal === "timeout") { + child.kill("SIGKILL"); + fail("server did not shut down within 15s of SIGTERM", stderrBuf); + } + console.log(`boot-smoke: clean shutdown (${code ?? signal})`); + console.log("boot-smoke: PASS"); +} + +main().catch((err) => fail(err.message ?? String(err))); From fd7fed22f465be452d1c0a16f0e65b47de3b38d2 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Fri, 5 Jun 2026 09:02:36 -0700 Subject: [PATCH 03/12] refactor(ci): thin PR gate to lint/typecheck/build/gate; demote full suite to non-blocking push-to-main workflow - pr-checks.yml: gate job = boot smoke + pnpm test:gate; shards/slow/inventory removed - full-suite.yml (new): 4-way shards, engine slow tier, inventory guard on push to main - ci.yml deleted (trigger-disabled since FN-1541; was dead config) - ci-workflow.test.ts rewritten to pin the new gate shape; release/signing blocks preserved - docs/contributing.md: pnpm test:gate is the merge gate; verify:workspace repositioned as deep opt-in BRANCH-PROTECTION CUTOVER: set required checks to exactly [Lint, Typecheck, Build, Gate] after merging; open PRs must rebase. --- .github/workflows/ci.yml | 69 ----- .github/workflows/full-suite.yml | 167 ++++++++++++ .github/workflows/pr-checks.yml | 150 +++-------- docs/contributing.md | 26 +- .../cli/src/__tests__/ci-workflow.test.ts | 250 +++++++++--------- .../cli/src/__tests__/package-config.test.ts | 12 +- 6 files changed, 355 insertions(+), 319 deletions(-) delete mode 100644 .github/workflows/ci.yml create mode 100644 .github/workflows/full-suite.yml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml deleted file mode 100644 index 7334449c25..0000000000 --- a/.github/workflows/ci.yml +++ /dev/null @@ -1,69 +0,0 @@ -name: CI - -# CI auto-trigger disabled per FN-1541 — workflow preserved for manual use via workflow_dispatch -on: - workflow_dispatch: - -# FN-4863: Opt JavaScript actions into Node 24 ahead of GitHub's forced cutover on 2026-06-02. -env: - FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: "true" - -jobs: - lint: - name: Lint - runs-on: ubuntu-latest - - steps: - - name: Checkout - uses: actions/checkout@v4 - - - name: Setup Node and install dependencies - uses: ./.github/actions/setup-node-pnpm - - - name: Lint - run: pnpm lint - - test-shards: - name: Test shard ${{ matrix.shard }}/3 - runs-on: ubuntu-latest - strategy: - fail-fast: false - matrix: - shard: [1, 2, 3] - - steps: - - name: Checkout - uses: actions/checkout@v4 - - - name: Setup Node and install dependencies - uses: ./.github/actions/setup-node-pnpm - - - name: Test (deterministic shard) - run: pnpm test:ci:shard --shard ${{ matrix.shard }} --total 3 - - build: - name: Build - runs-on: ubuntu-latest - needs: [lint, test-shards] - - steps: - - name: Checkout - uses: actions/checkout@v4 - - - name: Setup Node and install dependencies - uses: ./.github/actions/setup-node-pnpm - - - name: Install Bun - uses: oven-sh/setup-bun@v2 - - - name: Build workspace - run: pnpm build - - - name: Run CLI slow lane (opt-in suites) - run: pnpm test:slow-cli - - - name: Build standalone binary - run: pnpm --filter @runfusion/fusion build:exe - - - name: Verify binary exists - run: test -f packages/cli/dist/fn diff --git a/.github/workflows/full-suite.yml b/.github/workflows/full-suite.yml new file mode 100644 index 0000000000..a207d4e0bc --- /dev/null +++ b/.github/workflows/full-suite.yml @@ -0,0 +1,167 @@ +name: Full Suite (non-blocking) + +# The demoted test tier (docs/plans/2026-06-04-001-refactor-fast-trusted-test-gate-plan.md). +# Runs the full sharded suite, the engine slow tier, and the dashboard +# inventory guard on every push to main — post-merge signal only. These jobs +# are NON-BLOCKING by design: they never run on PRs and must never be added +# to branch-protection required checks. A red run here is information, not a +# merge stopper; see docs/testing.md for the quarantine ratchet that keeps +# this tier honest. + +on: + push: + branches: [main] + +concurrency: + group: full-suite-${{ github.ref }} + cancel-in-progress: true + +# FN-4863: Opt JavaScript actions into Node 24 ahead of GitHub's forced cutover on 2026-06-02. +env: + FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: "true" + +jobs: + test-shards: + name: Test shard ${{ matrix.shard }}/4 + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + shard: [1, 2, 3, 4] + + steps: + - name: Checkout + uses: actions/checkout@v4 + with: + # Engine tests run real git operations (merge-base against main, + # case-variant ref checks) that require full history. Shallow + # clones silently break tests like worktree-acquisition's resume + # misbinding path. + fetch-depth: 0 + + - name: Setup Node.js and pnpm + uses: ./.github/actions/setup-node-pnpm + + # Dist-artifact cache (L1): ensureTestArtifacts otherwise rebuilds dist/ + # for 8 packages (~71s) on every shard because CI starts with no dist. + # Key on a stable, pre-build, git-based hash of ALL build packages' source + # inputs. Exact-match only — NO restore-keys: a partial/stale dist hit is + # the exact failure mode this repo has been bitten by (FN-4232/FN-4605), + # and ensureTestArtifacts still validates/rebuilds anything missing-or-stale + # after restore, so a miss is safe but a wrong-content hit would not be. + # NEVER add node_modules here (breaks Windows pnpm junctions elsewhere). + - name: Compute dist source hash + id: dist-hash + run: echo "hash=$(node scripts/ensure-test-artifacts.mjs --print-source-hash)" >> "$GITHUB_OUTPUT" + + - name: Cache built dist artifacts + id: dist-cache + uses: actions/cache@v4 + with: + path: | + packages/core/dist + packages/dashboard/dist + packages/engine/dist + packages/plugin-sdk/dist + plugins/fusion-plugin-dependency-graph/dist + plugins/fusion-plugin-hermes-runtime/dist + plugins/fusion-plugin-openclaw-runtime/dist + plugins/fusion-plugin-paperclip-runtime/dist + key: dist-${{ runner.os }}-${{ steps.dist-hash.outputs.hash }} + + # On a cache HIT, restored dist files carry their save-time mtimes while + # checkout rewrites src mtimes to "now" (src newer than dist), which would + # make ensureTestArtifacts' mtime fallback rebuild everything and defeat + # the cache. Seed the per-package content-hash cache so its content-hash + # short-circuit fires instead. ensureTestArtifacts still runs (inside + # test:ci:shard) and rebuilds anything genuinely missing/changed. + - name: Seed artifact hash-cache on cache hit + if: steps.dist-cache.outputs.cache-hit == 'true' + run: node scripts/ensure-test-artifacts.mjs --seed-artifact-cache + + - name: Test (deterministic shard) + run: pnpm test:ci:shard --shard ${{ matrix.shard }} --total 4 + + # Each shard emits per-file vitest JSON timing reporter output under + # .timings/. Upload as an artifact so the timing snapshot can be + # refreshed locally/from the default branch via + # `node scripts/ci-test-shard.mjs --write-timings`. We do NOT commit the + # snapshot automatically — refresh is manual/scheduled only. + - name: Upload per-shard test timings + if: always() + uses: actions/upload-artifact@v4 + with: + name: test-timings-shard-${{ matrix.shard }} + # Relative outputFile paths mean each package writes its own + # /.timings/ file — glob the whole tree, not just the root. + path: | + .timings/timings-*.json + packages/*/.timings/timings-*.json + plugins/*/.timings/timings-*.json + plugins/examples/*/.timings/timings-*.json + if-no-files-found: ignore + retention-days: 14 + + # The dashboard quality gate used to enumerate its test files by hand, so any + # unenumerated app/ or src/ test file ran in NO project. This guard fails when + # a dashboard test file is neither executed by a quality project (curated + + # backfill lanes) nor on the reviewed skip-list. Cheap: it only runs + # `vitest list`, not the tests. + test-inventory-guard: + name: Dashboard curated-gate guard + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Setup Node.js and pnpm + uses: ./.github/actions/setup-node-pnpm + + # Same dist-artifact cache as test-shards (L1): the curated-gate guard runs + # `vitest list`, whose config resolution can touch built dist, so it also + # pays the cold-dist rebuild. Exact-match key on the pre-build source hash; + # NO restore-keys (stale dist is the failure mode), NO node_modules. + - name: Compute dist source hash + id: dist-hash + run: echo "hash=$(node scripts/ensure-test-artifacts.mjs --print-source-hash)" >> "$GITHUB_OUTPUT" + + - name: Cache built dist artifacts + id: dist-cache + uses: actions/cache@v4 + with: + path: | + packages/core/dist + packages/dashboard/dist + packages/engine/dist + packages/plugin-sdk/dist + plugins/fusion-plugin-dependency-graph/dist + plugins/fusion-plugin-hermes-runtime/dist + plugins/fusion-plugin-openclaw-runtime/dist + plugins/fusion-plugin-paperclip-runtime/dist + key: dist-${{ runner.os }}-${{ steps.dist-hash.outputs.hash }} + + - name: Seed artifact hash-cache on cache hit + if: steps.dist-cache.outputs.cache-hit == 'true' + run: node scripts/ensure-test-artifacts.mjs --seed-artifact-cache + + - name: Assert every dashboard test file is gated or skip-listed + run: node scripts/check-test-inventory.mjs --dashboard-curated + + # The engine-slow tier (src/**/*.slow.test.ts) runs here with a non-empty + # execution assertion, so a glob/config drift that silently empties the tier + # fails this workflow instead of passing vacuously. Engine slow tests do real + # git operations, so a full clone (fetch-depth: 0) is required. + test-slow: + name: Engine slow tier + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Setup Node.js and pnpm + uses: ./.github/actions/setup-node-pnpm + + - name: Run engine-slow with non-empty-execution assertion + run: node scripts/assert-engine-slow-nonempty.mjs diff --git a/.github/workflows/pr-checks.yml b/.github/workflows/pr-checks.yml index 4468206a2a..fb8321ca39 100644 --- a/.github/workflows/pr-checks.yml +++ b/.github/workflows/pr-checks.yml @@ -1,12 +1,21 @@ name: PR Checks +# The thin trusted merge gate (docs/plans/2026-06-04-001-refactor-fast-trusted-test-gate-plan.md). +# Blocking checks are exactly: Lint, Typecheck, Build, Gate. +# +# BRANCH-PROTECTION CUTOVER: required status checks are matched by job name. +# When this file changes job names, update the repo's branch-protection +# required checks to exactly [Lint, Typecheck, Build, Gate] — a stale required +# name (e.g. "Test shard 1/4") that no longer reports will block every PR +# with "Expected — waiting for status". Open PRs must rebase onto main after +# the cutover so they run this workflow shape. +# +# Everything that used to run here as shards / slow tier / inventory guard is +# non-blocking and lives in full-suite.yml (push to main). + on: pull_request: branches: [main] - # Also run on every push to main so post-merge regressions surface - # immediately instead of being discovered on the next PR. - push: - branches: [main] concurrency: group: pr-checks-${{ github.ref }} @@ -62,94 +71,13 @@ jobs: - name: Build run: pnpm build - test-shards: - name: Test shard ${{ matrix.shard }}/4 - runs-on: ubuntu-latest - strategy: - fail-fast: false - matrix: - shard: [1, 2, 3, 4] - - steps: - - name: Checkout - uses: actions/checkout@v4 - with: - # Engine tests run real git operations (merge-base against main, - # case-variant ref checks) that require full history. Shallow - # clones silently break tests like worktree-acquisition's resume - # misbinding path. - fetch-depth: 0 - - - name: Setup Node.js and pnpm - uses: ./.github/actions/setup-node-pnpm - - # Dist-artifact cache (L1): ensureTestArtifacts otherwise rebuilds dist/ - # for 8 packages (~71s) on every shard because CI starts with no dist. - # Key on a stable, pre-build, git-based hash of ALL build packages' source - # inputs. Exact-match only — NO restore-keys: a partial/stale dist hit is - # the exact failure mode this repo has been bitten by (FN-4232/FN-4605), - # and ensureTestArtifacts still validates/rebuilds anything missing-or-stale - # after restore, so a miss is safe but a wrong-content hit would not be. - # NEVER add node_modules here (breaks Windows pnpm junctions elsewhere). - - name: Compute dist source hash - id: dist-hash - run: echo "hash=$(node scripts/ensure-test-artifacts.mjs --print-source-hash)" >> "$GITHUB_OUTPUT" - - - name: Cache built dist artifacts - id: dist-cache - uses: actions/cache@v4 - with: - path: | - packages/core/dist - packages/dashboard/dist - packages/engine/dist - packages/plugin-sdk/dist - plugins/fusion-plugin-dependency-graph/dist - plugins/fusion-plugin-hermes-runtime/dist - plugins/fusion-plugin-openclaw-runtime/dist - plugins/fusion-plugin-paperclip-runtime/dist - key: dist-${{ runner.os }}-${{ steps.dist-hash.outputs.hash }} - - # On a cache HIT, restored dist files carry their save-time mtimes while - # checkout rewrites src mtimes to "now" (src newer than dist), which would - # make ensureTestArtifacts' mtime fallback rebuild everything and defeat - # the cache. Seed the per-package content-hash cache so its content-hash - # short-circuit fires instead. ensureTestArtifacts still runs (inside - # test:ci:shard) and rebuilds anything genuinely missing/changed. - - name: Seed artifact hash-cache on cache hit - if: steps.dist-cache.outputs.cache-hit == 'true' - run: node scripts/ensure-test-artifacts.mjs --seed-artifact-cache - - - name: Test (deterministic shard) - run: pnpm test:ci:shard --shard ${{ matrix.shard }} --total 4 - - # U1 (R4): each shard emits per-file vitest JSON timing reporter output - # under .timings/. Upload as an artifact so the timing snapshot can be - # refreshed locally/from the default branch via - # `node scripts/ci-test-shard.mjs --write-timings`. We do NOT commit the - # snapshot from PR branches — refresh is manual/scheduled only. - - name: Upload per-shard test timings - if: always() - uses: actions/upload-artifact@v4 - with: - name: test-timings-shard-${{ matrix.shard }} - # Relative outputFile paths mean each package writes its own - # /.timings/ file — glob the whole tree, not just the root. - path: | - .timings/timings-*.json - packages/*/.timings/timings-*.json - plugins/*/.timings/timings-*.json - plugins/examples/*/.timings/timings-*.json - if-no-files-found: ignore - retention-days: 14 - - # Plan U2 / R7: the dashboard quality gate used to enumerate its test files by - # hand, so any unenumerated app/ or src/ test file ran in NO project. This - # guard fails when a dashboard test file is neither executed by a quality - # project (curated + backfill lanes) nor on the reviewed skip-list. Cheap: - # it only runs `vitest list`, not the tests. - test-inventory-guard: - name: Dashboard curated-gate guard + # The only merge-blocking TEST signal (R3). Runs the boot smoke (the app + # starts and serves) plus the curated engine-core suite and the CI-shape + # test — see `test:gate` in the root package.json. Gate membership is the + # explicit allow-list in packages/engine/vitest.config.ts (engine-core + # project); a flaky gate test is evicted by removing it from that list. + gate: + name: Gate runs-on: ubuntu-latest steps: - name: Checkout @@ -158,10 +86,12 @@ jobs: - name: Setup Node.js and pnpm uses: ./.github/actions/setup-node-pnpm - # Same dist-artifact cache as test-shards (L1): the curated-gate guard runs - # `vitest list`, whose config resolution can touch built dist, so it also - # pays the cold-dist rebuild. Exact-match key on the pre-build source hash; - # NO restore-keys (stale dist is the failure mode), NO node_modules. + - name: Install Bun + uses: oven-sh/setup-bun@v2 + + # Dist-artifact cache (same contract as full-suite.yml): exact-match + # key only, NO restore-keys (stale dist is the known failure mode, + # FN-4232/FN-4605), NEVER node_modules (breaks Windows pnpm junctions). - name: Compute dist source hash id: dist-hash run: echo "hash=$(node scripts/ensure-test-artifacts.mjs --print-source-hash)" >> "$GITHUB_OUTPUT" @@ -185,25 +115,13 @@ jobs: if: steps.dist-cache.outputs.cache-hit == 'true' run: node scripts/ensure-test-artifacts.mjs --seed-artifact-cache - - name: Assert every dashboard test file is gated or skip-listed - run: node scripts/check-test-inventory.mjs --dashboard-curated + # Boot smoke needs the full built workspace (CLI dist is not in the + # cache list above); cached packages make this incremental-fast. + - name: Build + run: pnpm build - # Plan U2 / R8: the engine-slow tier (src/**/*.slow.test.ts) previously ran in - # NO automated gate — only via the local `test:full`. This job runs it and - # asserts a non-empty execution, so a glob/config drift that silently empties - # the tier fails CI instead of passing vacuously. Engine slow tests do real - # git operations, so a full clone (fetch-depth: 0) is required. - test-slow: - name: Engine slow tier - runs-on: ubuntu-latest - steps: - - name: Checkout - uses: actions/checkout@v4 - with: - fetch-depth: 0 + - name: Boot smoke (app starts and serves) + run: node scripts/boot-smoke.mjs - - name: Setup Node.js and pnpm - uses: ./.github/actions/setup-node-pnpm - - - name: Run engine-slow with non-empty-execution assertion - run: node scripts/assert-engine-slow-nonempty.mjs + - name: Gate tests (curated engine-core + CI-shape) + run: pnpm test:gate diff --git a/docs/contributing.md b/docs/contributing.md index 78c21adf59..f97cec31a2 100644 --- a/docs/contributing.md +++ b/docs/contributing.md @@ -63,11 +63,13 @@ FUSION_DEV_PREBUILD=full pnpm dev dashboard # production-like full workspace pr pnpm dev:ui # dashboard dev server only pnpm dev:hmr # dashboard API + Vite HMR UI, with no startup prebuild pnpm lint # lint all packages -pnpm test # changed-only workspace tests (falls back to full suite in safety contexts) -pnpm test:full # full workspace quality gate (clean-worktree compatible) +pnpm test # merge-gate suite + changed-only affected tests (bounded; never full-suite) +pnpm test:gate # the merge gate: curated engine-core suite + CI-shape test +pnpm smoke:boot # boot smoke: CLI --help + real serve /api/health +pnpm test:full # full workspace suite (explicit opt-in; clean-worktree compatible) pnpm build # workspace builds (excludes desktop/mobile) pnpm build:all # full workspace build (includes desktop/mobile) -pnpm verify:workspace # canonical lint -> test -> build verification gate +pnpm verify:workspace # deep opt-in verification: lint -> test:full -> build pnpm typecheck # workspace typechecks ``` @@ -80,20 +82,21 @@ Fusion codifies workspace verification as a deterministic contract: - Root test entrypoints (`pnpm test` via `scripts/test-changed.mjs` and `pnpm test:ci:shard` via `scripts/ci-test-shard.mjs`) call `scripts/ensure-test-artifacts.mjs`, which deterministically builds only missing/stale required workspace dist artifacts (`@fusion/core`, `@fusion/dashboard`, `@fusion/engine`, `@fusion/plugin-sdk`, and `@fusion-plugin-examples/{dependency-graph,hermes-runtime,openclaw-runtime,paperclip-runtime}`). - Package-scoped hooks now mirror this bootstrap for fresh worktrees where needed: `@fusion/dashboard` and `@fusion-plugin-examples/dependency-graph` run `pretest: node ../../scripts/ensure-test-artifacts.mjs`. - This includes clean states where those required dist directories are absent. -- `pnpm verify:workspace` is the canonical pre-merge gate and runs in strict order: +- `pnpm test:gate` is the merge gate: the curated `engine-core` suite plus the CI-shape test. CI blocks PRs on exactly Lint, Typecheck, Build, and Gate (boot smoke + `pnpm test:gate`) — see `.github/workflows/pr-checks.yml` and docs/testing.md. +- `pnpm verify:workspace` is the deep opt-in verification (not the merge gate) and runs in strict order: 1. `pnpm lint` 2. `pnpm test:full` 3. `pnpm build` -GitHub Actions now runs deterministic test sharding via `pnpm test:ci:shard --shard --total ` in both PR checks and manual CI, while keeping local semantics unchanged: +GitHub Actions runs deterministic test sharding via `pnpm test:ci:shard --shard --total ` in the non-blocking `full-suite.yml` workflow (push to main only — never a PR gate), while keeping local semantics unchanged: -- `pnpm test` remains changed-only local iteration. -- `pnpm test:full` remains the canonical workspace quality gate; dashboard exhaustive coverage is explicit via `pnpm --filter @fusion/dashboard test:deep`. -- `pnpm verify:workspace` remains the canonical local lint -> test -> build gate. +- `pnpm test` remains gate + changed-only local iteration. +- `pnpm test:full` remains the explicit full workspace suite; dashboard exhaustive coverage is explicit via `pnpm --filter @fusion/dashboard test:deep`. +- `pnpm verify:workspace` remains the deep opt-in lint -> test -> build verification. `test:ci:shard` is a CI-focused entrypoint (`scripts/ci-test-shard.mjs`) that deterministically balances workspace packages with `test` scripts by counting package-local `**/__tests__/**/*.test.{ts,tsx,mjs}` files, auto-splitting oversized packages into virtual shard entries (`{ name, shardIndex, shardCount }`), then assigning entries in descending weight order with best-fit placement for unsplit entries (closest under-budget fit, otherwise minimum overshoot) while keeping slices of the same package on different shards when possible. Whole entries run as grouped `pnpm --filter test` calls, and virtual entries run one-by-one via `pnpm --filter test -- --shard /`. This keeps coverage reproducible while improving shard balance. -`pnpm test` now uses a changed-only entrypoint (`scripts/test-changed.mjs`) for faster local iteration. It resolves the comparison base from `.changeset/config.json` (`baseBranch`) and runs only affected workspaces from `pnpm-workspace.yaml` (both `packages/*` and `plugins/**`) using safe package-first filtering (`pnpm --filter test`). It automatically falls back to the full suite when the run is forced (CI / `--full`), the git comparison base or diff cannot be resolved, no changes are detected, shared/root test infrastructure changes, or changed workspace paths cannot be resolved to a workspace package (fail-safe coverage behavior). +`pnpm test` now uses a changed-only entrypoint (`scripts/test-changed.mjs`) for faster local iteration. It resolves the comparison base from `.changeset/config.json` (`baseBranch`) and runs only affected workspaces from `pnpm-workspace.yaml` (both `packages/*` and `plugins/**`) using safe package-first filtering (`pnpm --filter test`). It runs the merge-gate suite first, then the affected set. The full suite runs only on explicit opt-in (`--full` / `pnpm test:full`); shared-infrastructure changes and unresolvable diffs widen the affected set but never escalate to an implicit full-suite run (the old escalation was the local OOM path). Root test entrypoints (`pnpm test`, `pnpm test:full`, and `pnpm test:ci:shard`) now use a shared CPU-aware default worker budget instead of fixed low values. By default, Fusion sets `FUSION_TEST_TOTAL_WORKERS` to `max(4, min(12, cpuCount - 1))` and `FUSION_TEST_CONCURRENCY` to `2` (clamped to the total budget), while still honoring explicit overrides from `VITEST_MAX_WORKERS`, `FUSION_TEST_TOTAL_WORKERS`, and `FUSION_TEST_CONCURRENCY`. @@ -113,7 +116,8 @@ If you add or change test entrypoints, keep this isolation guard path intact and Before submitting changes, verify: -- [ ] `pnpm verify:workspace` — canonical lint → test → build gate +- [ ] `pnpm test:gate` — the merge gate (curated engine-core suite + CI-shape test) +- [ ] `pnpm verify:workspace` — deep opt-in lint → test:full → build verification - [ ] `pnpm typecheck` — type checking passes ## Realtime/SSE change note @@ -160,7 +164,7 @@ pnpm build:exe:all # build multi-target executables Default workspace verification stays lean and deterministic: - `pnpm test` runs the standard suite and does **not** require Bun cross-build integration tests. -- `pnpm verify:workspace` remains the canonical `lint -> test -> build` gate. +- `pnpm verify:workspace` remains the deep opt-in `lint -> test -> build` verification. Slow/pre-release CLI coverage is explicit and opt-in: diff --git a/packages/cli/src/__tests__/ci-workflow.test.ts b/packages/cli/src/__tests__/ci-workflow.test.ts index 236b6fa9e5..3fa71994b9 100644 --- a/packages/cli/src/__tests__/ci-workflow.test.ts +++ b/packages/cli/src/__tests__/ci-workflow.test.ts @@ -27,12 +27,10 @@ function findCompositeSetupStep(steps: any[]) { return steps.find((step) => step.uses === "./.github/actions/setup-node-pnpm"); } -describe("CI workflow (.github/workflows/ci.yml)", () => { +describe("Merge gate (.github/workflows/pr-checks.yml)", () => { let workflow: any; let content: string; let compositeAction: any; - let buildSteps: any[]; - let testShardJob: any; let contributingContent: string; let readmeContent: string; let cliPackageJsonContent: string; @@ -41,12 +39,10 @@ describe("CI workflow (.github/workflows/ci.yml)", () => { let buildExeSuiteContent: string; beforeAll(() => { - const result = loadWorkflow("ci.yml"); + const result = loadWorkflow("pr-checks.yml"); workflow = result.parsed; content = result.content; compositeAction = loadYamlFile(".github", "actions", "setup-node-pnpm", "action.yml").parsed; - buildSteps = workflow.jobs?.build?.steps ?? []; - testShardJob = workflow.jobs?.["test-shards"]; contributingContent = readFileSync(join(workspaceRoot, "docs", "contributing.md"), "utf-8"); readmeContent = readFileSync(join(workspaceRoot, "README.md"), "utf-8"); cliPackageJsonContent = readFileSync(join(workspaceRoot, "packages", "cli", "package.json"), "utf-8"); @@ -64,134 +60,56 @@ describe("CI workflow (.github/workflows/ci.yml)", () => { ); }); - const findBuildStepByRun = (runSnippet: string) => - buildSteps.find((step) => typeof step.run === "string" && step.run.includes(runSnippet)); - it("is valid YAML", () => { expect(workflow).toBeDefined(); expect(typeof workflow).toBe("object"); }); - it("uses workflow_dispatch trigger (auto CI disabled)", () => { - expect(workflow.on).toHaveProperty("workflow_dispatch"); + it("runs on pull requests targeting main and ONLY there", () => { + expect(workflow.on?.pull_request?.branches).toContain("main"); + // Post-merge signal lives in full-suite.yml; the gate workflow must not + // double-run on push (that conflates blocking and non-blocking surfaces). + expect(workflow.on?.push).toBeUndefined(); }); - it("does not auto-trigger on push/pull_request", () => { - expect(workflow.on.push).toBeUndefined(); - expect(workflow.on.pull_request).toBeUndefined(); + it("blocks PRs on exactly lint, typecheck, build, and gate", () => { + expect(Object.keys(workflow.jobs ?? {}).sort()).toEqual(["build", "gate", "lint", "typecheck"]); }); - it("pins dependency bootstrap to frozen lockfile", () => { - const jobs = [workflow.jobs?.lint, workflow.jobs?.["test-shards"], workflow.jobs?.build]; - for (const job of jobs) { - expect(findCompositeSetupStep(job?.steps ?? [])).toBeDefined(); + it("contains no shard matrix or full-suite invocation (demoted to full-suite.yml)", () => { + expect(workflow.jobs?.["test-shards"]).toBeUndefined(); + expect(workflow.jobs?.["test-slow"]).toBeUndefined(); + expect(workflow.jobs?.["test-inventory-guard"]).toBeUndefined(); + expect(content).not.toContain("test:ci:shard"); + expect(content).not.toContain("run: pnpm test\n"); + expect(content).not.toContain("pnpm verify:workspace"); + }); + + it("gate job runs boot smoke and the dedicated test:gate command", () => { + const gateSteps = workflow.jobs?.gate?.steps ?? []; + expect( + gateSteps.some( + (step: any) => typeof step.run === "string" && step.run.includes("node scripts/boot-smoke.mjs"), + ), + ).toBe(true); + // The gate must use the dedicated command — `pnpm test` routes through + // scripts/test-changed.mjs whose selection semantics are for local runs. + expect( + gateSteps.some( + (step: any) => typeof step.run === "string" && step.run.includes("pnpm test:gate"), + ), + ).toBe(true); + }); + + it("pins dependency bootstrap to frozen lockfile in every job", () => { + for (const jobName of ["lint", "typecheck", "build", "gate"]) { + expect(findCompositeSetupStep(workflow.jobs?.[jobName]?.steps ?? [])).toBeDefined(); } expect(content).not.toContain("run: pnpm install\n"); expect(content).not.toContain("--no-frozen-lockfile"); expect(compositeAction.inputs?.["install-args"]?.default).toBe("--frozen-lockfile"); }); - it("uses deterministic test sharding and keeps lint/build as explicit jobs", () => { - expect(workflow.jobs?.lint).toBeDefined(); - expect(testShardJob).toBeDefined(); - expect(workflow.jobs?.build).toBeDefined(); - - expect(testShardJob.strategy?.matrix?.shard).toEqual([1, 2, 3]); - expect(content).toContain("pnpm test:ci:shard --shard ${{ matrix.shard }} --total 3"); - expect(content).not.toContain("pnpm verify:workspace"); - }); - - it("runs build job after lint and sharded tests, then executes slow lane and binary packaging", () => { - expect(workflow.jobs?.build?.needs).toEqual(["lint", "test-shards"]); - expect(findBuildStepByRun("pnpm build")).toBeDefined(); - expect(findBuildStepByRun("pnpm test:slow-cli")).toBeDefined(); - expect(findBuildStepByRun("build:exe")).toBeDefined(); - }); - - it("keeps contributing docs aligned with verification and slow-lane contracts", () => { - expect(contributingContent).toContain("pnpm test:full` must be runnable in a clean worktree without requiring a prior `pnpm build`."); - expect(contributingContent).toContain("`pnpm verify:workspace` is the canonical pre-merge gate"); - expect(contributingContent).toContain("1. `pnpm lint`"); - expect(contributingContent).toContain("2. `pnpm test:full`"); - expect(contributingContent).toContain("3. `pnpm build`"); - expect(contributingContent).toContain("`pnpm test` now uses a changed-only entrypoint"); - - expect(contributingContent).toContain("pnpm test:slow-cli"); - expect(contributingContent).toContain("test:pre-release"); - expect(contributingContent).toContain("test:extension-integration"); - }); - - it("keeps docs aligned with default and explicit build commands", () => { - expect(readmeContent).toContain("pnpm build # Build default workspace packages (excludes desktop/mobile)"); - expect(readmeContent).toContain("pnpm build:all # Build all packages (including desktop/mobile)"); - - expect(contributingContent).toContain("pnpm build # default build (excludes desktop/mobile)"); - expect(contributingContent).toContain("pnpm build:all # full recursive build including desktop/mobile"); - }); - - it("includes binary build step", () => { - expect(content).toContain("build:exe"); - }); - - it("keeps explicit gating for audited CLI integration suites", () => { - expect(cliPackageJsonContent).toContain('"test:slow-cli"'); - expect(cliPackageJsonContent).toContain("FUSION_TEST_SLOW_CLI=1"); - expect(cliPackageJsonContent).toContain('"test:extension-integration"'); - expect(cliPackageJsonContent).toContain("FUSION_TEST_EXTENSION_INTEGRATION=1"); - expect(cliPackageJsonContent).toContain("extension-integration.test.ts"); - expect(cliPackageJsonContent).toContain('"test:build-exe"'); - expect(cliPackageJsonContent).toContain("FUSION_TEST_BUILD_EXE=1"); - - expect(extensionSuiteContent).toContain("describe.skipIf(!SHOULD_RUN_EXTENSION_INTEGRATION)"); - expect(extensionSuiteContent).toContain("FUSION_TEST_EXTENSION_INTEGRATION"); - expect(extensionSuiteContent).toContain("dist/extension.js"); - - expect(agentExportSuiteContent).toContain("describe.skipIf(!SHOULD_RUN_SLOW_CLI)"); - expect(agentExportSuiteContent).toContain("FUSION_TEST_SLOW_CLI"); - - expect(buildExeSuiteContent).toContain('process.env.FUSION_TEST_BUILD_EXE === "1"'); - expect(buildExeSuiteContent).toContain('process.env.FUSION_TEST_BUILD_EXE === "true"'); - expect(buildExeSuiteContent).not.toContain("Boolean(process.env.FUSION_TEST_BUILD_EXE)"); - }); - - it("includes Bun setup", () => { - expect(content).toContain("oven-sh/setup-bun"); - }); - - it("verifies binary exists after build", () => { - expect(content).toContain("test -f packages/cli/dist/fn"); - }); -}); - -describe("PR checks workflow (.github/workflows/pr-checks.yml)", () => { - let workflow: any; - let content: string; - - beforeAll(() => { - const result = loadWorkflow("pr-checks.yml"); - workflow = result.parsed; - content = result.content; - }); - - it("is valid YAML", () => { - expect(workflow).toBeDefined(); - expect(typeof workflow).toBe("object"); - }); - - it("runs on pull requests targeting main", () => { - expect(workflow.on?.pull_request?.branches).toContain("main"); - }); - - it("uses the same deterministic test sharding command as manual CI", () => { - expect(workflow.jobs?.lint).toBeDefined(); - expect(workflow.jobs?.typecheck).toBeDefined(); - expect(workflow.jobs?.build).toBeDefined(); - expect(workflow.jobs?.["test-shards"]).toBeDefined(); - expect(workflow.jobs?.["test-shards"]?.strategy?.matrix?.shard).toEqual([1, 2, 3, 4]); - expect(content).toContain("pnpm test:ci:shard --shard ${{ matrix.shard }} --total 4"); - expect(content).not.toContain("run: pnpm test\n"); - }); - it("keeps lint as install + lint only, without Bun/setup build coupling", () => { const lintSteps = workflow.jobs?.lint?.steps ?? []; expect( @@ -229,7 +147,99 @@ describe("PR checks workflow (.github/workflows/pr-checks.yml)", () => { ).toBe(true); }); - it("does not spend PR action minutes on a pre-test workspace build", () => { + it("keeps contributing docs aligned with the gate contract", () => { + expect(contributingContent).toContain("pnpm test:full` must be runnable in a clean worktree without requiring a prior `pnpm build`."); + expect(contributingContent).toContain("`pnpm test:gate` is the merge gate"); + expect(contributingContent).toContain("`pnpm verify:workspace` is the deep opt-in verification (not the merge gate)"); + expect(contributingContent).toContain("1. `pnpm lint`"); + expect(contributingContent).toContain("2. `pnpm test:full`"); + expect(contributingContent).toContain("3. `pnpm build`"); + expect(contributingContent).toContain("`pnpm test` now uses a changed-only entrypoint"); + + expect(contributingContent).toContain("pnpm test:slow-cli"); + expect(contributingContent).toContain("test:pre-release"); + expect(contributingContent).toContain("test:extension-integration"); + }); + + it("keeps docs aligned with default and explicit build commands", () => { + expect(readmeContent).toContain("pnpm build # Build default workspace packages (excludes desktop/mobile)"); + expect(readmeContent).toContain("pnpm build:all # Build all packages (including desktop/mobile)"); + + expect(contributingContent).toContain("pnpm build # default build (excludes desktop/mobile)"); + expect(contributingContent).toContain("pnpm build:all # full recursive build including desktop/mobile"); + }); + + it("keeps explicit gating for audited CLI integration suites", () => { + expect(cliPackageJsonContent).toContain('"test:slow-cli"'); + expect(cliPackageJsonContent).toContain("FUSION_TEST_SLOW_CLI=1"); + expect(cliPackageJsonContent).toContain('"test:extension-integration"'); + expect(cliPackageJsonContent).toContain("FUSION_TEST_EXTENSION_INTEGRATION=1"); + expect(cliPackageJsonContent).toContain("extension-integration.test.ts"); + expect(cliPackageJsonContent).toContain('"test:build-exe"'); + expect(cliPackageJsonContent).toContain("FUSION_TEST_BUILD_EXE=1"); + + expect(extensionSuiteContent).toContain("describe.skipIf(!SHOULD_RUN_EXTENSION_INTEGRATION)"); + expect(extensionSuiteContent).toContain("FUSION_TEST_EXTENSION_INTEGRATION"); + expect(extensionSuiteContent).toContain("dist/extension.js"); + + expect(agentExportSuiteContent).toContain("describe.skipIf(!SHOULD_RUN_SLOW_CLI)"); + expect(agentExportSuiteContent).toContain("FUSION_TEST_SLOW_CLI"); + + expect(buildExeSuiteContent).toContain('process.env.FUSION_TEST_BUILD_EXE === "1"'); + expect(buildExeSuiteContent).toContain('process.env.FUSION_TEST_BUILD_EXE === "true"'); + expect(buildExeSuiteContent).not.toContain("Boolean(process.env.FUSION_TEST_BUILD_EXE)"); + }); + + it("the deleted manual CI workflow stays deleted", () => { + // ci.yml was the trigger-disabled (FN-1541) 3-shard manual workflow; the + // merge-gate redesign removed it. Reintroducing it would resurrect a + // second, drift-prone definition of the test pipeline. + expect(() => loadWorkflow("ci.yml")).toThrow(); + }); +}); + +describe("Full suite workflow (.github/workflows/full-suite.yml)", () => { + let workflow: any; + let content: string; + + beforeAll(() => { + const result = loadWorkflow("full-suite.yml"); + workflow = result.parsed; + content = result.content; + }); + + it("is valid YAML", () => { + expect(workflow).toBeDefined(); + expect(typeof workflow).toBe("object"); + }); + + it("runs ONLY on push to main — never as a PR gate", () => { + expect(workflow.on?.push?.branches).toEqual(["main"]); + expect(workflow.on?.pull_request).toBeUndefined(); + }); + + it("carries the demoted tier: 4-way shards, engine slow, inventory guard", () => { + expect(workflow.jobs?.["test-shards"]?.strategy?.matrix?.shard).toEqual([1, 2, 3, 4]); + expect(content).toContain("pnpm test:ci:shard --shard ${{ matrix.shard }} --total 4"); + expect(workflow.jobs?.["test-slow"]).toBeDefined(); + expect(workflow.jobs?.["test-inventory-guard"]).toBeDefined(); + }); + + it("keeps full clones where real-git tests need history", () => { + const shardSteps = workflow.jobs?.["test-shards"]?.steps ?? []; + const slowSteps = workflow.jobs?.["test-slow"]?.steps ?? []; + for (const steps of [shardSteps, slowSteps]) { + expect( + steps.some((step: any) => step.uses?.includes("actions/checkout") && step.with?.["fetch-depth"] === 0), + ).toBe(true); + } + }); + + it("still uploads per-shard timing artifacts for snapshot refresh", () => { + expect(content).toContain("test-timings-shard-${{ matrix.shard }}"); + }); + + it("does not spend action minutes on a pre-test workspace build", () => { const testSteps = workflow.jobs?.["test-shards"]?.steps ?? []; expect( testSteps.some( diff --git a/packages/cli/src/__tests__/package-config.test.ts b/packages/cli/src/__tests__/package-config.test.ts index 9c6ed4b801..3d34449432 100644 --- a/packages/cli/src/__tests__/package-config.test.ts +++ b/packages/cli/src/__tests__/package-config.test.ts @@ -299,10 +299,16 @@ describe("Workspace bootstrap script contract", () => { }); describe("Workflow YAML validity", () => { - it("ci.yml is valid YAML", () => { - const parsed = loadWorkflowYaml("ci.yml"); + it("pr-checks.yml is valid YAML", () => { + const parsed = loadWorkflowYaml("pr-checks.yml"); expect(parsed).toBeDefined(); - expect(parsed.name).toBe("CI"); + expect(parsed.name).toBe("PR Checks"); + }); + + it("full-suite.yml is valid YAML", () => { + const parsed = loadWorkflowYaml("full-suite.yml"); + expect(parsed).toBeDefined(); + expect(parsed.name).toBe("Full Suite (non-blocking)"); }); it("version.yml is valid YAML", () => { From bbeb86e8f9e1b701f83f6647c6f14d260c9b1f8c Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Fri, 5 Jun 2026 09:03:22 -0700 Subject: [PATCH 04/12] feat(test): add flaky-test quarantine ledger with --diff ratchet exemption note --- scripts/check-test-inventory.mjs | 8 ++++++++ scripts/lib/test-quarantine.json | 4 ++++ 2 files changed, 12 insertions(+) create mode 100644 scripts/lib/test-quarantine.json diff --git a/scripts/check-test-inventory.mjs b/scripts/check-test-inventory.mjs index 193aff28d1..93992b9be0 100644 --- a/scripts/check-test-inventory.mjs +++ b/scripts/check-test-inventory.mjs @@ -16,6 +16,14 @@ * remove (old path) + add (new path); the diff lists the removed ids so * the rename is reviewable. New ids in never fail the diff. * + * DELIBERATELY UNWIRED IN CI: the quarantine deletion ratchet + * (scripts/lib/test-quarantine.json, docs/testing.md) deletes expired + * quarantined tests by design, and a snapshot-based --diff guard would + * fail on exactly those deletions. If --diff is ever wired to a + * committed snapshot, it must exempt ledger-driven deletions (diff + * against "snapshot minus quarantined entries"), or the two mechanisms + * deadlock. + * * --dashboard-curated * Assert that every `*.test.{ts,tsx}` file under packages/dashboard/app * and packages/dashboard/src is included by at least one *executed* diff --git a/scripts/lib/test-quarantine.json b/scripts/lib/test-quarantine.json new file mode 100644 index 0000000000..39eac9c428 --- /dev/null +++ b/scripts/lib/test-quarantine.json @@ -0,0 +1,4 @@ +{ + "$comment": "Flaky-test quarantine ledger (deletion ratchet — see AGENTS.md 'Flaky tests: quarantine on sight' and docs/testing.md 'Quarantine ledger and the deletion ratchet'). A test observed failing without a corresponding real bug is quarantined ON SIGHT: add an entry here AND a matching one-line `exclude` entry in that package's vitest config, in the same commit. Every entry needs a non-empty `reason` (link the failing run) and a `quarantinedAt` date — the entry expires 14 days later, at which point the test file is DELETED unless someone rescues it with evidence it catches real regressions plus a root-cause fix (never appeasement). There is deliberately no loader module and no automation around this file: it is a dated record, the vitest config exclude is the mechanism, and the sweep is policy executed by whoever touches the suite.", + "entries": [] +} From 5a4bb9eeeb4cab4cec545dad706d32d6c2bd54c0 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Fri, 5 Jun 2026 09:08:20 -0700 Subject: [PATCH 05/12] feat(test): pnpm test runs gate + affected set; remove implicit full-suite escalation (the local OOM path) - decideExecutionPlan: implicit wide-blast reasons (missing base, diff failed, no changes, shared infra, unmapped package) route to new gate mode instead of full - CI===true force-full branch removed (CI no longer calls test-changed.mjs) - changed mode runs pnpm test:gate before the affected set - full suite reachable only via explicit --full / FUSION_TEST_FULL=1 - characterization tests updated + new only-explicit-full invariant test --- .../src/__tests__/root-test-command.test.ts | 14 ++-- scripts/__tests__/test-changed.test.mjs | 56 +++++++++----- scripts/boot-smoke.mjs | 3 + scripts/test-changed.mjs | 76 ++++++++++++++----- 4 files changed, 101 insertions(+), 48 deletions(-) diff --git a/packages/cli/src/__tests__/root-test-command.test.ts b/packages/cli/src/__tests__/root-test-command.test.ts index d91c649a48..4a8972b0a7 100644 --- a/packages/cli/src/__tests__/root-test-command.test.ts +++ b/packages/cli/src/__tests__/root-test-command.test.ts @@ -24,7 +24,7 @@ describe("root test command changed-only planning", () => { expect(plan).toEqual({ mode: "changed", packages: ["@fusion/core", "@fusion/engine"] }); }); - it("falls back to full suite when shared test infra changes", () => { + it("routes to gate mode when shared test infra changes (no implicit full suite)", () => { const plan = decideExecutionPlan({ forceFullSuite: false, comparisonBase: "abc123", @@ -32,10 +32,10 @@ describe("root test command changed-only planning", () => { packageNameByDir: new Map([["packages/core", "@fusion/core"]]), }); - expect(plan).toEqual({ mode: "full", reason: "shared-infra-changed" }); + expect(plan).toEqual({ mode: "gate", reason: "shared-infra-changed" }); }); - it("falls back to full suite when comparison base cannot be resolved", () => { + it("routes to gate mode when comparison base cannot be resolved", () => { const plan = decideExecutionPlan({ forceFullSuite: false, comparisonBase: null, @@ -43,16 +43,16 @@ describe("root test command changed-only planning", () => { packageNameByDir: new Map(), }); - expect(plan).toEqual({ mode: "full", reason: "missing-comparison-base" }); + expect(plan).toEqual({ mode: "gate", reason: "missing-comparison-base" }); }); - it("treats unknown package directories as full-suite fallback", () => { + it("treats unknown package directories as gate-mode fallback (resolver returns null)", () => { const resolved = resolveAffectedPackages(["packages/unknown/src/index.ts"], new Map()); expect(resolved).toBeNull(); }); - it("marks root workflow/config changes as full-suite triggers", () => { - expect(shouldForceFullSuite([".github/workflows/ci.yml"])).toBe(true); + it("marks root workflow/config changes as shared-infra (gate-mode) triggers", () => { + expect(shouldForceFullSuite([".github/workflows/pr-checks.yml"])).toBe(true); expect(shouldForceFullSuite(["package.json"])).toBe(true); expect(shouldForceFullSuite(["packages/core/src/store.ts"])).toBe(false); }); diff --git a/scripts/__tests__/test-changed.test.mjs b/scripts/__tests__/test-changed.test.mjs index 0ca4160a8e..0f4fa23b6b 100644 --- a/scripts/__tests__/test-changed.test.mjs +++ b/scripts/__tests__/test-changed.test.mjs @@ -248,47 +248,47 @@ test("decideExecutionPlan: forced full suite", () => { assert.equal(plan.reason, "forced"); }); -test("decideExecutionPlan: missing comparison base → full", () => { +test("decideExecutionPlan: missing comparison base → gate", () => { const plan = decideExecutionPlan({ forceFullSuite: false, comparisonBase: null, changedFiles: null, packageNameByDir: basePackageMap, }); - assert.equal(plan.mode, "full"); + assert.equal(plan.mode, "gate"); assert.equal(plan.reason, "missing-comparison-base"); }); -test("decideExecutionPlan: diff failed → full", () => { +test("decideExecutionPlan: diff failed → gate", () => { const plan = decideExecutionPlan({ forceFullSuite: false, comparisonBase: "abc123", changedFiles: null, packageNameByDir: basePackageMap, }); - assert.equal(plan.mode, "full"); + assert.equal(plan.mode, "gate"); assert.equal(plan.reason, "diff-failed"); }); -test("decideExecutionPlan: no changes → full", () => { +test("decideExecutionPlan: no changes → gate", () => { const plan = decideExecutionPlan({ forceFullSuite: false, comparisonBase: "abc123", changedFiles: [], packageNameByDir: basePackageMap, }); - assert.equal(plan.mode, "full"); + assert.equal(plan.mode, "gate"); assert.equal(plan.reason, "no-changes"); }); -test("decideExecutionPlan: shared infra changed → full", () => { +test("decideExecutionPlan: shared infra changed → gate", () => { const plan = decideExecutionPlan({ forceFullSuite: false, comparisonBase: "abc123", changedFiles: ["pnpm-lock.yaml"], packageNameByDir: basePackageMap, }); - assert.equal(plan.mode, "full"); + assert.equal(plan.mode, "gate"); assert.equal(plan.reason, "shared-infra-changed"); }); @@ -344,14 +344,14 @@ test("decideExecutionPlan: expands changed packages with reverse dependents", () assert.deepEqual(plan.packages, ["@fusion/core", "@fusion/engine", "@fusion/dashboard"]); }); -test("decideExecutionPlan: no affected package resolved → full", () => { +test("decideExecutionPlan: no affected package resolved → gate", () => { const plan = decideExecutionPlan({ forceFullSuite: false, comparisonBase: "abc123", changedFiles: ["packages/nonexistent/src/foo.ts"], packageNameByDir: basePackageMap, }); - assert.equal(plan.mode, "full"); + assert.equal(plan.mode, "gate"); assert.equal(plan.reason, "no-affected-package"); }); @@ -370,7 +370,7 @@ test("decideExecutionPlan: plugin-only workspace changes stay in changed mode", assert.deepEqual(plan.packages, ["@fusion-plugin-examples/openclaw-runtime"]); }); -test("decideExecutionPlan: plugin changes without mapping fail safe to full", () => { +test("decideExecutionPlan: plugin changes without mapping fail safe to gate", () => { const plan = decideExecutionPlan({ forceFullSuite: false, comparisonBase: "abc123", @@ -378,7 +378,7 @@ test("decideExecutionPlan: plugin changes without mapping fail safe to full", () packageNameByDir: basePackageMap, }); - assert.equal(plan.mode, "full"); + assert.equal(plan.mode, "gate"); assert.equal(plan.reason, "no-affected-package"); }); @@ -857,25 +857,41 @@ test("emitModeDecision: changed plan reports changed-packages reason + package c assert.deepEqual(lines, [line]); }); -test("emitModeDecision: full plan surfaces the decideExecutionPlan reason, packages=0", () => { +test("emitModeDecision: gate plan surfaces the decideExecutionPlan reason, packages=0", () => { assert.equal( - emitModeDecision({ mode: "full", reason: "missing-comparison-base" }, () => {}), - "[test-changed] mode=full reason=missing-comparison-base packages=0", + emitModeDecision({ mode: "gate", reason: "missing-comparison-base" }, () => {}), + "[test-changed] mode=gate reason=missing-comparison-base packages=0", ); assert.equal( - emitModeDecision({ mode: "full", reason: "shared-infra-changed" }, () => {}), - "[test-changed] mode=full reason=shared-infra-changed packages=0", + emitModeDecision({ mode: "gate", reason: "shared-infra-changed" }, () => {}), + "[test-changed] mode=gate reason=shared-infra-changed packages=0", ); }); -test("emitModeDecision: distinct full reasons round-trip from decideExecutionPlan", () => { - const full = decideExecutionPlan({ forceFullSuite: false, comparisonBase: null }); - assert.equal(emitModeDecision(full, () => {}), "[test-changed] mode=full reason=missing-comparison-base packages=0"); +test("emitModeDecision: gate and forced-full reasons round-trip from decideExecutionPlan", () => { + const gate = decideExecutionPlan({ forceFullSuite: false, comparisonBase: null }); + assert.equal(emitModeDecision(gate, () => {}), "[test-changed] mode=gate reason=missing-comparison-base packages=0"); const forced = decideExecutionPlan({ forceFullSuite: true }); assert.equal(emitModeDecision(forced, () => {}), "[test-changed] mode=full reason=forced packages=0"); }); +// The implicit full-suite escalation was the local OOM path (FN: merge-gate +// redesign). The full suite must be reachable ONLY via explicit opt-in. +test("decideExecutionPlan: full mode is reachable only via forceFullSuite", () => { + const implicitInputs = [ + { forceFullSuite: false, comparisonBase: null }, + { forceFullSuite: false, comparisonBase: "origin/main", changedFiles: null }, + { forceFullSuite: false, comparisonBase: "origin/main", changedFiles: [] }, + { forceFullSuite: false, comparisonBase: "origin/main", changedFiles: [".github/workflows/pr-checks.yml"] }, + { forceFullSuite: false, comparisonBase: "origin/main", changedFiles: ["unmapped/path.ts"], packageNameByDir: new Map() }, + ]; + for (const input of implicitInputs) { + const plan = decideExecutionPlan(input); + assert.equal(plan.mode, "gate", `expected gate mode for ${JSON.stringify(input)}`); + } +}); + // --------------------------------------------------------------------------- // U3: cache-fresh fast path — when every changed package is cache-fresh, // applyCacheToPlan yields zero active packages, which is the signal that lets diff --git a/scripts/boot-smoke.mjs b/scripts/boot-smoke.mjs index 0ffd16d583..05505b51af 100644 --- a/scripts/boot-smoke.mjs +++ b/scripts/boot-smoke.mjs @@ -9,6 +9,9 @@ * 3. The server shuts down cleanly on SIGTERM. * * Safety properties (see scripts/check-no-kill-4040.mjs and AGENTS.md): + * (port-4040-allowlist: this file only ever AVOIDS the reserved ports — it + * requests an ephemeral port and rejects reserved ones; it never binds, + * probes, or kills them.) * - Never binds or touches port 4040 / FUSION_RESERVED_PORTS — an ephemeral * port is requested from the OS (listen on 0) and double-checked against * the reserved list. diff --git a/scripts/test-changed.mjs b/scripts/test-changed.mjs index 192d192961..461de987bb 100644 --- a/scripts/test-changed.mjs +++ b/scripts/test-changed.mjs @@ -430,8 +430,16 @@ function isTestIrrelevantRootPath(file) { export function shouldForceFullSuite(changedFiles) { // NOTE: overlaps SHARED_HASH_INPUT_PATHS by intent (different axis: this list - // forces full-suite mode; that one busts every package's cache hash). When - // adding a new shared root config input, consider both lists. + // signals shared-infra changes; that one busts every package's cache hash). + // When adding a new shared root config input, consider both lists. + // + // HISTORY: this signal used to escalate `pnpm test` to an implicit full + // recursive run — which was the local OOM path (two concurrent heavy + // packages, 6GB dashboard heaps). Since the merge-gate redesign + // (docs/plans/2026-06-04-001-refactor-fast-trusted-test-gate-plan.md) it + // routes to GATE mode instead: run the merge-gate suite and point at + // `pnpm test:full` for the explicit full sweep. The full suite only ever + // runs on explicit opt-in (--full / FUSION_TEST_FULL=1). const fullSuitePaths = [ "package.json", "pnpm-lock.yaml", @@ -1012,13 +1020,16 @@ export function decideExecutionPlan({ reverseDependencyMap, }) { if (forceFullSuite) return { mode: "full", reason: "forced" }; - if (!comparisonBase) return { mode: "full", reason: "missing-comparison-base" }; - if (!changedFiles) return { mode: "full", reason: "diff-failed" }; - if (changedFiles.length === 0) return { mode: "full", reason: "no-changes" }; - if (shouldForceFullSuite(changedFiles)) return { mode: "full", reason: "shared-infra-changed" }; + // Every implicit wide-blast condition below routes to GATE mode (merge-gate + // suite only), never to an implicit full-suite run — the old escalation was + // the local OOM path. `pnpm test:full` is the explicit opt-in full sweep. + if (!comparisonBase) return { mode: "gate", reason: "missing-comparison-base" }; + if (!changedFiles) return { mode: "gate", reason: "diff-failed" }; + if (changedFiles.length === 0) return { mode: "gate", reason: "no-changes" }; + if (shouldForceFullSuite(changedFiles)) return { mode: "gate", reason: "shared-infra-changed" }; const affectedPackages = resolveAffectedPackages(changedFiles, packageNameByDir); - if (!affectedPackages || affectedPackages.length === 0) return { mode: "full", reason: "no-affected-package" }; + if (!affectedPackages || affectedPackages.length === 0) return { mode: "gate", reason: "no-affected-package" }; return { mode: "changed", @@ -1060,8 +1071,11 @@ export function normalizeForwardedArgs(argv) { } export function main(argv = process.argv.slice(2)) { + // The full suite is explicit opt-in ONLY (--full / FUSION_TEST_FULL=1). + // CI no longer routes through this script (the gate job runs `pnpm + // test:gate`; the demoted tier runs `test:ci:shard` in full-suite.yml), so + // the old `CI === "true"` force-full branch is gone. const forceFullSuite = - process.env.CI === "true" || process.env.FUSION_TEST_FULL === "1" || argv.includes("--full"); @@ -1136,7 +1150,10 @@ export function main(argv = process.argv.slice(2)) { })); } - const hasWork = plan.mode === "full" || activePackages.length > 0; + // Gate mode always has work: the merge-gate suite is not covered by the + // per-package cache (it spans engine + cli with its own selection), so it + // must never short-circuit through the cache-fresh fast path. + const hasWork = plan.mode === "full" || plan.mode === "gate" || activePackages.length > 0; // Cache-fresh fast path: nothing to run. Emit a fast-path mode line, run only // the (now cheap) isolation guard, and skip skill-sync, artifact-ensure, @@ -1177,18 +1194,7 @@ export function main(argv = process.argv.slice(2)) { try { if (plan.mode === "full") { - if (plan.reason === "missing-comparison-base") { - console.log(`[test-changed] could not resolve merge-base with ${baseBranch}; running full suite.`); - } else if (plan.reason === "diff-failed") { - console.log("[test-changed] failed to read git diff; running full suite."); - } else if (plan.reason === "no-changes") { - console.log("[test-changed] no changes detected against base; running full suite."); - } else if (plan.reason === "shared-infra-changed") { - console.log("[test-changed] shared/root test infrastructure changed; running full suite."); - } else if (plan.reason === "no-affected-package") { - console.log("[test-changed] no affected workspace package resolved; running full suite."); - } - + // Explicit opt-in only ("forced": --full / FUSION_TEST_FULL=1). runMaybeIsolated("pnpm", [`-r`, `--workspace-concurrency=${workspaceConcurrency}`, "test", ...forwardedArgs], { env: isolatedHomeEnv, onBeforeAfterCheck: cleanupIsolatedHome, @@ -1196,6 +1202,34 @@ export function main(argv = process.argv.slice(2)) { return; } + if (plan.mode === "gate") { + if (plan.reason === "missing-comparison-base") { + console.log(`[test-changed] could not resolve merge-base with ${baseBranch}; running merge-gate suite.`); + } else if (plan.reason === "diff-failed") { + console.log("[test-changed] failed to read git diff; running merge-gate suite."); + } else if (plan.reason === "no-changes") { + console.log("[test-changed] no changes detected against base; running merge-gate suite."); + } else if (plan.reason === "shared-infra-changed") { + console.log("[test-changed] shared/root test infrastructure changed; running merge-gate suite."); + } else if (plan.reason === "no-affected-package") { + console.log("[test-changed] no affected workspace package resolved; running merge-gate suite."); + } + console.log("[test-changed] need the full sweep instead? run `pnpm test:full` (explicit opt-in)."); + + runMaybeIsolated("pnpm", ["test:gate"], { + env: isolatedHomeEnv, + onBeforeAfterCheck: cleanupIsolatedHome, + }); + return; + } + + // Changed mode: merge-gate suite first, then the affected set. The gate is + // cheap (~10s) and keeps `pnpm test` green ⇒ mergeable-signal honest; the + // affected expansion preserves changed-code coverage. Overlap (engine in the + // affected set re-runs the engine-core files) is accepted by design. + console.log("[test-changed] running merge-gate suite (pnpm test:gate) before affected packages."); + run("pnpm", ["test:gate"], { env: isolatedHomeEnv }); + const filterArgs = activePackages.flatMap((pkg) => ["--filter", pkg]); console.log(`[test-changed] running tests for changed packages: ${activePackages.join(", ")}`); if (cachedPackages.length > 0) { From e2ddcdc7c1e12c81fac1981cdf1afeea3359d115 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Fri, 5 Jun 2026 09:10:19 -0700 Subject: [PATCH 06/12] docs(test): codify merge gate + flaky-test deletion ratchet policy - AGENTS.md: gate/non-blocking split replaces 'tests are required' framing; new standing rule: flaky tests quarantined on sight, deleted in 2 weeks unless rescued with root-cause fix; agent appeasement ban; gate eviction rule; product-race escalation note - docs/testing.md: merge-gate section with honest blind-spot statement, quarantine ledger mechanics, updated workflow references --- AGENTS.md | 19 +++++++++++++++---- docs/testing.md | 46 ++++++++++++++++++++++++++++++++++++---------- 2 files changed, 51 insertions(+), 14 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index e876ae834d..52ba6b3e87 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -66,16 +66,27 @@ Rules: ### Testing commands -Tests are required. Typechecks/manual checks are not substitutes. +The merge gate is thin and trusted: CI blocks PRs on exactly Lint, Typecheck, Build, and Gate (boot smoke + `pnpm test:gate`). Everything else runs non-blocking in `full-suite.yml` on push to main. A red gate means a real problem; a red non-blocking run is information, not a merge stopper. Typechecks/manual checks are not substitutes for the gate. ```bash -pnpm test -pnpm test:full +pnpm test # gate suite + changed-only affected tests (bounded; never full-suite) +pnpm test:gate # the merge gate: curated engine-core suite + CI-shape test +pnpm smoke:boot # boot smoke: CLI --help + real serve /api/health +pnpm test:full # full workspace suite — explicit opt-in only pnpm lint pnpm build -pnpm verify:workspace +pnpm verify:workspace # deep opt-in verification (lint -> test:full -> build); NOT the merge gate ``` +### Standing Rule: Flaky Tests Are Quarantined on Sight (Deletion Ratchet) + +- A test observed failing without a corresponding real bug in the change is QUARANTINED ON SIGHT: add an entry to `scripts/lib/test-quarantine.json` (`file`, `reason` with a link to the failing run, `quarantinedAt`) AND a matching one-line `exclude` in that package's vitest config, in the same commit. +- **Agents must never appease a flaky test.** No widened timeouts, no added retries, no loosened or deleted assertions to make a flake pass. Quarantine it instead. Appeasement drains the test's signal and is how the suite rotted last time. +- A quarantined test is DELETED after 14 days (`quarantinedAt` + 2 weeks) unless rescued. Rescue requires evidence the test catches real regressions plus a root-cause fix — not stabilization passes. +- A flake INSIDE the merge gate is evicted, not skipped: remove its line from the `engine-core` allow-list in `packages/engine/vitest.config.ts` (the eviction PR does not need the flaky test to pass). +- A second quarantine in the same subsystem is a product-race smell — look at the product code before the deletion clock runs out (see `docs/solutions/ui-bugs/skill-autocomplete-highlight-reset-on-swr-revalidation.md`: a flake "stabilized" three times was a real race). +- Gate admission requires evidence of value; tests never graduate into the gate by default. Mechanics: `docs/testing.md` → "Quarantine ledger and the deletion ratchet". + ### Standing Rule: Do Not Add Slow Tests (FN-5048) - Prefer narrow seams, in-memory fakes, shared harnesses, and targeted assertions. diff --git a/docs/testing.md b/docs/testing.md index 3817e9a446..4c01d04ef9 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -4,18 +4,26 @@ This guide consolidates the detailed testing guidance moved from `AGENTS.md`. -## Required workspace gates +## The merge gate -Tests are required. Typechecks and manual verification are not substitutes for assertions. +CI blocks PRs on exactly four checks (`.github/workflows/pr-checks.yml`): **Lint, Typecheck, Build, Gate**. The Gate job runs the boot smoke (`scripts/boot-smoke.mjs`: CLI `--help` + a real `fn serve` answering `GET /api/health`) and `pnpm test:gate` (the curated `engine-core` vitest project + the CI-shape test). Everything else — the 4-way shards, the engine slow tier, the dashboard inventory guard — runs NON-BLOCKING in `.github/workflows/full-suite.yml` on push to main. + +Gate membership is the explicit allow-list in `packages/engine/vitest.config.ts` (`engine-core` project). Admission requires evidence of value (the test catches real regressions); tests never graduate in by default. A flaky gate test is evicted by deleting its allow-list line — the eviction PR does not need the flaky test to pass. The whole `engine-core` project must stay under ~60s wall-clock. + +**The gate's blind spot, stated honestly:** typecheck + build + boot smoke + curated suite does not run the union suite a merge creates. Logic regressions outside the curated set land non-blocking by design — that is the accepted trade: the old broad gate caught no recalled real bugs while consuming ~70% of shipping time in flake triage. + +## Required workspace gates Use the narrowest command that exercises the behavior you changed, then broaden before reporting completion. ```bash -pnpm test # changed-only workspace tests; falls back to full gate in safety contexts -pnpm test:full # full workspace quality gate +pnpm test # gate suite + changed-only affected tests (bounded; never full-suite) +pnpm test:gate # the merge gate: curated engine-core suite + CI-shape test +pnpm smoke:boot # boot smoke: CLI --help + real serve /api/health +pnpm test:full # full workspace suite — explicit opt-in only pnpm lint # lint all packages pnpm build # build workspace packages (excludes desktop/mobile) -pnpm verify:workspace # canonical pre-merge gate: lint -> test:full -> build +pnpm verify:workspace # deep opt-in verification: lint -> test:full -> build (NOT the merge gate) ``` `pnpm test:full` runs each package's default test script with capped worker fanout (`FUSION_TEST_TOTAL_WORKERS=4 FUSION_TEST_CONCURRENCY=2 pnpm -r --workspace-concurrency=2 test`). Do not casually raise worker counts; dashboard/jsdom and integration-heavy packages destabilize when oversubscribed. Use `VITEST_MAX_WORKERS=` only for targeted package-level investigation. @@ -50,7 +58,7 @@ list only when you want it in a specific fast lane rather than the backfill catc The dashboard quality gate is a chain of curated lanes plus two backfill lanes. Together they must execute **every** `*.test.{ts,tsx}` under `packages/dashboard/app` and `packages/dashboard/src`, or the file must be on the reviewed skip-list. This is -enforced by a guard (CI job `Dashboard curated-gate guard` in `pr-checks.yml`): +enforced by a guard (CI job `Dashboard curated-gate guard` in `full-suite.yml`, non-blocking): ```bash node scripts/check-test-inventory.mjs --dashboard-curated @@ -88,19 +96,37 @@ The capture spec (which packages/projects to enumerate) lives in a renamed file shows up as a remove (old path) + add (new path), so the rename is reviewable. New test ids never fail the diff. -## Engine slow tier (CI gate) +## Engine slow tier (non-blocking CI) The `engine-slow` vitest project (`packages/engine/src/**/*.slow.test.ts`) holds the long real-git suites. It runs locally via `pnpm --filter @fusion/engine test:slow` and -in CI via the `Engine slow tier` job in `pr-checks.yml`, which uses +in CI via the `Engine slow tier` job in `full-suite.yml` (non-blocking, push to main), which uses `scripts/assert-engine-slow-nonempty.mjs` to **fail if zero tests executed** (so a glob -or config drift that silently empties the tier breaks CI instead of passing vacuously). +or config drift that silently empties the tier breaks the run instead of passing vacuously). The CI job uses `fetch-depth: 0` because these tests run real git operations. +## Quarantine ledger and the deletion ratchet + +Flaky tests are quarantined ON SIGHT and deleted on a 2-week clock. This is written policy with minimal mechanics — deliberately no loader module, no automation (see the AGENTS.md standing rule "Flaky Tests Are Quarantined on Sight"). + +**To quarantine a test** (a test that failed without a corresponding real bug in the change), in one commit: + +1. Add an entry to `scripts/lib/test-quarantine.json`: + `{ "file": "", "reason": "", "quarantinedAt": "YYYY-MM-DD" }` +2. Add a matching one-line `exclude` entry to that package's vitest config. + +**The clock:** an entry expires 14 days after `quarantinedAt`. Whoever touches the suite and finds an expired entry deletes the test file, its ledger entry, and its config exclude (git history is the archive). `scripts/check-test-inventory.mjs --diff` stays deliberately unwired in CI because it would fail on exactly these deletions. + +**Rescue** (before the clock runs out) requires both: evidence the test catches real regressions, and a root-cause fix for the flake. Stabilization passes — widened timeouts, retries, loosened assertions — are appeasement, not rescue, and are banned (for agents especially). + +**Gate eviction:** a flake inside the merge gate cannot block all merges while red — it is evicted by removing its line from the `engine-core` allow-list (no quarantine entry needed unless it should also leave the non-blocking tier). + +**Product-race escalation:** a second quarantine in the same subsystem is a smell that the flake is a real product race, not test noise — look at the product code before deleting (a dashboard flake was "stabilized" three times before being found to be a real race; see `docs/solutions/ui-bugs/skill-autocomplete-highlight-reset-on-swr-revalidation.md`). + ## CI shard balancing (duration-weighted) `scripts/ci-test-shard.mjs` packs the 4 CI shards (`pnpm test:ci:shard --shard N --total 4`, -called from `pr-checks.yml`) by **measured duration**, not test-file count, using the +called from `full-suite.yml`, non-blocking) by **measured duration**, not test-file count, using the committed `scripts/test-timings.json` snapshot (U1/R4). A package's weight is the sum of its files' recorded durations; files (or whole packages) absent from the snapshot fall back to the snapshot's **median per-file duration** so untimed packages weigh From 93b6a33c07927ca3fc5cbd80fed5cf184c71703e Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Fri, 5 Jun 2026 09:12:43 -0700 Subject: [PATCH 07/12] docs: add fast-trusted-test-gate brainstorm + plan; CONCEPTS entries for Merge Gate and Deletion Ratchet; lint fix in boot-smoke --- CONCEPTS.md | 8 + ...-04-fast-trusted-test-gate-requirements.md | 112 ++++++++++ ...01-refactor-fast-trusted-test-gate-plan.md | 211 ++++++++++++++++++ scripts/boot-smoke.mjs | 6 +- 4 files changed, 336 insertions(+), 1 deletion(-) create mode 100644 docs/brainstorms/2026-06-04-fast-trusted-test-gate-requirements.md create mode 100644 docs/plans/2026-06-04-001-refactor-fast-trusted-test-gate-plan.md diff --git a/CONCEPTS.md b/CONCEPTS.md index 8f4df9fce6..05a6427e91 100644 --- a/CONCEPTS.md +++ b/CONCEPTS.md @@ -134,6 +134,14 @@ The user's mid-stage feedback channel: free-text guidance attached to an answer, ### Rehydration Re-establishing a live agent handle for a paused CE Session by replaying its recorded conversation against the model. Replay is side-effect-suppressed: it reconstructs the agent's context without re-emitting events, re-streaming Live activity, or re-writing artifacts. +## Testing + +### Merge Gate +The minimal set of merge-blocking PR checks: typecheck, build, boot smoke, and a small curated core-engine suite. The gate is the only test signal that can block a PR; admission requires evidence of value, and tests never graduate into it by default. All other tests run non-blocking. + +### Deletion Ratchet +The standing policy for flaky tests: a test observed failing without a corresponding real bug is quarantined on sight (removed from all runs, not retried, not patched), then deleted after 2 weeks unless rescued with evidence it catches real regressions. Rescue means fixing the flake at the root; appeasement (widened timeouts, added retries, loosened assertions) is prohibited, for agents especially. + ## Flagged ambiguities - "Merging" a shared-branch-group Task had been used for both member integration and group promotion — these are distinct steps with independent gating and must not be conflated. diff --git a/docs/brainstorms/2026-06-04-fast-trusted-test-gate-requirements.md b/docs/brainstorms/2026-06-04-fast-trusted-test-gate-requirements.md new file mode 100644 index 0000000000..665d6379b5 --- /dev/null +++ b/docs/brainstorms/2026-06-04-fast-trusted-test-gate-requirements.md @@ -0,0 +1,112 @@ +--- +date: 2026-06-04 +topic: fast-trusted-test-gate +--- + +# Fast, Trusted Test Gate + +## Summary + +Shrink the PR merge gate to a minimal trusted set — typecheck, build, boot smoke, and a small curated core-engine suite — demote everything else to non-blocking, and install a deletion ratchet: flaky tests are quarantined on sight and deleted after 2 weeks unless rescued with evidence. De-duplicate the overlapping shard runs across CI workflows. + +--- + +## Problem Frame + +PRs currently trigger multiple test shard jobs at 4–10 minutes each, and the suite appears to run in both `.github/workflows/ci.yml` (3 shards) and `.github/workflows/pr-checks.yml` (4 shards). Failures are mostly flaky/infra — timeouts, port conflicts, OOM kills — not real bugs. Local developers hit OOM running tests. + +The cost is severe: roughly 70% of shipping time goes into a loop of asking an agent to fix the failure and re-running. The agent-fix loop has a known degradation mode — agents appease flaky tests (widen timeouts, add retries, loosen assertions) rather than fix root causes, draining whatever signal the tests had. + +Against that cost, the suite's recalled value is zero: no PR test failure in recent memory caught a bug that would have actually broken users. ~3,880 test files are currently pure cost. Substantial speed infrastructure already exists on this branch (affected-only runner with content-hash caching in `scripts/test-changed.mjs`, CI sharding, a `.slow` split, isolation checks) — speed tuning alone has not fixed the problem because the core issue is trust, not throughput. + +--- + +## Key Decisions + +- **Thin trusted gate over broad coverage.** The merge gate guarantees only what the maintainer needs to merge without anxiety: typecheck, build, boot smoke, core-engine correctness. The evidence (zero recalled real catches across ~3,880 files) says breadth was not buying protection. +- **Delete over quarantine-forever.** Quarantine is a 2-week waiting room, not a retirement home. Without a deletion deadline, the demoted suite rots into a permanently red zombie that still consumes attention. +- **Policy, not machinery.** The ratchet is a written rule (in `AGENTS.md` and contributor docs), not new test infrastructure. This repo's history shows a pattern of answering test pain with more test machinery (sharding, isolation checks, lock runners, kill guards); this change deliberately breaks that pattern. +- **Agents are banned from "fixing" flaky tests.** When an agent encounters a flaky test, the correct action is quarantine, never appeasement. This stops the suite-weakening loop. +- **Reuse existing infrastructure.** `scripts/test-changed.mjs`, the `.slow` split, and shard timing artifacts stay. The change is what blocks merges, not how tests run. + +--- + +## Requirements + +**Merge gate** + +- R1. The PR merge gate consists of: typecheck, build, a boot smoke check (the app starts and serves), and a curated core-engine test suite. +- R2. The curated core-engine suite runs as a single CI job targeting under ~3 minutes, with zero known-flaky tests admitted. +- R3. The gate is the only merge-blocking test signal; no other test job can block a PR. +- R4. If no suitable boot smoke check exists, a single small one is created (see Dependencies). + +**CI de-duplication** + +- R5. The full suite runs at most once per PR event; the overlapping shard runs between `.github/workflows/ci.yml` and `.github/workflows/pr-checks.yml` are consolidated. +- R6. Tests outside the gate run as a non-blocking job (post-merge or scheduled); their failures never block a PR. + +**Deletion ratchet (policy)** + +- R7. Any test observed to fail without a corresponding real bug (flake) is quarantined on sight — removed from all blocking and non-blocking runs, not retried, not patched. +- R8. A quarantined test is deleted after 2 weeks unless someone rescues it with evidence that it catches real regressions; rescue requires fixing the flake at the root, not appeasing it. +- R9. The quarantine/delete rule and the agent prohibition on flaky-test appeasement (no widened timeouts, added retries, or loosened assertions to make a flake pass) are written into `AGENTS.md`. +- R10. Admission to the blocking gate requires evidence of value; tests do not graduate into the gate by default. + +**Local development** + +- R11. The default local test command (`pnpm test`) runs the gate suite, sized so it cannot OOM a typical dev machine. +- R12. Running anything larger locally is opt-in via explicitly named commands. + +--- + +## Acceptance Examples + +- AE1. **Covers R7, R8.** + - **Given:** a test fails on a PR, and the failure does not correspond to a bug in the change. + - **When:** a maintainer or agent triages it. + - **Then:** the test is quarantined (skipped everywhere) in that same PR or a follow-up, with a dated marker; 2 weeks later it is deleted unless rescued with evidence. +- AE2. **Covers R3, R6.** + - **Given:** a non-gate test fails in the non-blocking run. + - **When:** a PR is open. + - **Then:** the PR's mergeability is unaffected; the failure surfaces as information only. +- AE3. **Covers R9.** + - **Given:** an agent is asked to deal with a red flaky test. + - **When:** it consults `AGENTS.md`. + - **Then:** it quarantines the test rather than widening timeouts, adding retries, or weakening assertions. + +--- + +## Success Criteria + +- PR wall-clock for required checks drops from tens of minutes to under ~5 minutes. +- Maintainer time spent on the fix-and-rerun loop drops from ~70% to near zero; a red gate reliably indicates a real problem. +- No local OOMs from the default test command. +- Suite size shrinks over time via the ratchet rather than growing unboundedly. + +--- + +## Scope Boundaries + +- No new test machinery: no auto-quarantine infrastructure, no flake-scoring system, no test-value telemetry (the Approach C ratchet automation was considered and rejected as more of the machinery pattern). +- No root-cause fixing of existing flaky tests as part of this work — flakes exit via quarantine and deletion. +- Healthy non-gate tests survive indefinitely in the non-blocking run; this work does not mass-delete tests that aren't flaky. +- Coverage targets and coverage tooling are untouched. + +--- + +## Dependencies / Assumptions + +- **Unverified:** whether a usable boot smoke test already exists. If not, R4 creates one — kept deliberately small. +- **Unverified:** the exact trigger overlap between `ci.yml` and `pr-checks.yml` shard jobs on PR events; confirmed shard matrices exist in both, but trigger conditions need checking during planning. +- The existing `.slow` split and `scripts/test-changed.mjs` remain the mechanism for selecting and running tests; this work changes gating, not the runner. +- Branch protection / required-checks settings are adjustable to match the new gate. + +--- + +## Outstanding Questions + +**Deferred to planning** + +- Which engine tests make the curated gate suite (selection criteria: deterministic, fast, covering core orchestration logic the maintainer actually relies on). +- Quarantine mechanics: skip annotation vs. exclusion list vs. moving files — whichever is cheapest with the existing runner. +- Where the non-blocking run lives (post-merge on main vs. scheduled) and how its results surface without demanding attention. diff --git a/docs/plans/2026-06-04-001-refactor-fast-trusted-test-gate-plan.md b/docs/plans/2026-06-04-001-refactor-fast-trusted-test-gate-plan.md new file mode 100644 index 0000000000..ddc10f5212 --- /dev/null +++ b/docs/plans/2026-06-04-001-refactor-fast-trusted-test-gate-plan.md @@ -0,0 +1,211 @@ +--- + +title: "refactor: Shrink PR merge gate to a fast trusted set with a deletion ratchet" +type: refactor +status: active +date: 2026-06-04 +origin: docs/brainstorms/2026-06-04-fast-trusted-test-gate-requirements.md + +--- + +# refactor: Shrink PR merge gate to a fast trusted set with a deletion ratchet + +## Summary + +Replace the current 9-runner PR gate (lint, typecheck, build, 4 test shards, dashboard curated-gate guard, engine slow tier) with a thin trusted gate — 4 blocking checks: lint, typecheck, build, and a gate job combining boot smoke + the curated `engine-core` suite, with the gate job's test run under ~1 minute. (Lint is preserved from the existing gate as status quo; origin R1 does not name it.) Everything else moves to a non-blocking workflow on push to main. A quarantine ledger plus 2-week deletion ratchet (written policy, minimal mechanics) keeps flaky tests from re-accumulating, and local `pnpm test` is re-defaulted so developers cannot OOM. + +--- + +## Problem Frame + +PR failures are mostly flaky/infra and consume ~70% of maintainer shipping time in an agent-fix-and-rerun loop; no recalled PR test failure caught a real user-facing bug (see origin: docs/brainstorms/2026-06-04-fast-trusted-test-gate-requirements.md). Research corrected one origin assumption: there is **no double shard run** — `.github/workflows/ci.yml` is trigger-disabled (`workflow_dispatch` only, per FN-1541). The live gate is `.github/workflows/pr-checks.yml` alone. The local OOM path is also pinned: `shouldForceFullSuite` in `scripts/test-changed.mjs` (~line 431) escalates almost any shared-file change to a full-suite run at `--workspace-concurrency=2`, with dashboard lanes requesting 6GB heaps. + +--- + +## Key Technical Decisions + +- **The gate gets a dedicated command (`pnpm test:gate`); CI never invokes `pnpm test`.** `scripts/test-changed.mjs` hard-forces the full suite when `CI === "true"` (~line 1064), so any gate job calling `pnpm test` silently expands to everything. A dedicated command is the only way the ~1 min target is structurally guaranteed. +- **Gate membership is an explicit allow-list, not a glob.** A new `engine-core` vitest project in `packages/engine/vitest.config.ts` with an enumerated include list (precedent: the existing `engine-default`/`engine-reliability`/`engine-slow` project split). Recorded membership makes R10 (tests earn their way in) enforceable, and lets a flaky gate test be evicted by editing the list — no need for the flaky test itself to pass (resolves the chicken-and-egg eviction problem). +- **The non-blocking tier is a separate workflow file, not jobs inside `pr-checks.yml`.** A red full-suite run must not paint the gate's workflow red, or "red means real" dies on day one. New `full-suite.yml` triggered on push to main carries the 4 shards, engine slow tier, and inventory guard. +- **`ci.yml` is deleted and `packages/cli/src/__tests__/ci-workflow.test.ts` is rewritten in the same unit.** That test hard-asserts the current CI shape (ci.yml exists, 3-shard matrix, pr-checks 4-shard matrix, `docs/contributing.md` gate wording). Deleting the workflow without rewriting the test makes the change self-inconsistent. The rewritten test guards the *new* gate shape and is admitted to the gate suite — a test that guards the gate's own shape earns blocking status. +- **Quarantine ledger is a dated JSON file modeled on `scripts/lib/dashboard-curated-skiplist.json`, with `check-test-inventory.mjs --diff` left unwired.** The dashboard skiplist (entries with mandatory `reason`, shared between a guard script and vitest excludes) is the proven template; the ledger adds `quarantinedAt` so the 2-week clock is computable. `--diff` would fail CI on any test deletion — the exact opposite of the ratchet — so it stays unwired, documented as a deliberate exemption. +- **Quarantine stays on-sight (origin decision), with a product-race escalation note in the policy.** Institutional learning (`docs/solutions/ui-bugs/skill-autocomplete-highlight-reset-on-swr-revalidation.md`) documents a flake "stabilized" three times that was a real product race. The policy keeps on-sight quarantine but states: a second quarantine in the same subsystem is a product-race smell worth a look before the deletion clock runs out. No triage gate, no machinery. +- **Local `pnpm test` = gate suite + affected-package tests, with the force-full fallback removed.** Keeps the value of changed-code coverage (the affected-package expansion in `test-changed.mjs` already works) while deleting the OOM path: `shouldForceFullSuite` no longer escalates to a full recursive run locally; shared-infra changes run the gate suite plus a bounded affected set instead. `pnpm test:full` remains the explicit opt-in full suite. + +--- + +## High-Level Technical Design + +CI topology before → after: + +```mermaid +flowchart TB + subgraph before [Before: pr-checks.yml on PR] + L1[lint] ~~~ T1[typecheck] ~~~ B1[build] + S1[test shard 1/4] ~~~ S2[shard 2/4] ~~~ S3[shard 3/4] ~~~ S4[shard 4/4] + G1[dashboard curated guard] ~~~ E1[engine slow tier] + end + subgraph after_gate [After: pr-checks.yml on PR — blocking] + L2[lint] ~~~ T2[typecheck] ~~~ B2[build] + GATE[gate: boot smoke + curated engine-core suite + CI-shape test] + end + subgraph after_full [After: full-suite.yml on push to main — non-blocking] + S5[test shards 1..4] ~~~ E2[engine slow tier] ~~~ G2[inventory guard] + end + before -->|this plan| after_gate + before -->|this plan| after_full +``` + +Test lifecycle state machine (where each state is recorded): + +```mermaid +stateDiagram-v2 + [*] --> NonBlocking: new test (default) + NonBlocking --> Gate: evidence of value, added to engine-core allow-list + Gate --> NonBlocking: evicted (flaked in gate, removed from allow-list) + NonBlocking --> Quarantined: flaked, ledger entry with reason + quarantinedAt + Gate --> Quarantined: flake confirmed not gate-worthy + Quarantined --> NonBlocking: rescued with evidence + root-cause fix + Quarantined --> Deleted: 2 weeks elapsed, no rescue + Deleted --> [*] +``` + +State recording: **Gate** = presence in the `engine-core` include list (or gate job steps); **Quarantined** = entry in `scripts/lib/test-quarantine.json`; **NonBlocking** = default for everything else; **Deleted** = git history. + +--- + +## Requirements + +Carried from origin (R-IDs below are the origin's; see origin doc for full text). + +**Merge gate** + +- R1–R3. Gate = typecheck, build, boot smoke, curated `engine-core` suite; gate job's test run ~1 min; only merge-blocking test signal. (Lint stays as a separate pre-existing blocking check — status-quo preservation, not new scope from R1.) +- R4. Boot smoke is greenfield — verified that nothing real exists (`packages/cli/src/commands/__tests__/serve.test.ts` mocks `listen`; the only artifact smoke lives in the disabled `ci.yml`). + +**CI de-duplication (revised by research)** + +- R5 (revised). No double run exists; the work is deleting dead `ci.yml` and consolidating the gate in `pr-checks.yml`. +- R6. Non-gate tests run non-blocking in `full-suite.yml` on push to main; failures never block a PR. + +**Deletion ratchet** + +- R7–R10. Quarantine on sight; delete after 2 weeks unless rescued; policy in `AGENTS.md` including the agent appeasement ban; gate admission requires evidence. + +**Local development** + +- R11–R12. `pnpm test` cannot OOM (gate + bounded affected set, no force-full); bigger runs are explicit opt-ins. + +--- + +## Implementation Units + +### U1. Curated `engine-core` gate suite and `test:gate` command + +- **Goal:** A single fast command that runs the gate's test content, locally and in CI. +- **Requirements:** R1, R2, R10. +- **Dependencies:** none. +- **Files:** `packages/engine/vitest.config.ts`, `packages/engine/package.json`, `package.json` (root), `scripts/test-timings.json` (read-only input for selection). +- **Approach:** Add an `engine-core` vitest project with an explicit include list. Selection criteria: deterministic (no module-level shared-state bleed, no real timers/network), fast (use `scripts/test-timings.json` per-file data; total budget ~60s single-threaded, leaving headroom under 1 min with boot smoke), and covering invariants the KB marks regression-prone: merge-queue trigger-gate eligibility, shared-branch-group landing/promotion idempotency, fork-point/files-changed attribution, executor/scheduler core paths. Exclude `reliability-interactions/**` and `*.slow.test.ts` outright (heaviest, single-threaded). Root script `test:gate` runs the `engine-core` project plus the rewritten CI-shape test from U3. Exact file list is execution-time discovery against timing data — the criteria above are binding, the list is not pre-enumerable here. +- **Patterns to follow:** the existing three-project split in `packages/engine/vitest.config.ts` (lines ~50–95). +- **Test scenarios:** Test expectation: none — config/selection unit; its verification is the gate run itself (below). +- **Verification:** `pnpm test:gate` passes locally in under ~1 min cold; running it 5× consecutively produces 5 green runs (flake screen); it executes only the allow-listed files (verify via vitest `list`). + +### U2. Boot smoke check + +- **Goal:** A real "the app starts and serves" check — greenfield (R4). +- **Requirements:** R1, R4. +- **Dependencies:** none. +- **Files:** `scripts/boot-smoke.mjs` (new), `package.json` (root, `test:gate` integration or separate `smoke:boot` script). +- **Approach:** Script builds nothing itself (gate job already runs after `pnpm build` artifacts exist or reuses the build job's dist cache — mirror the dist-artifact cache steps in `pr-checks.yml` lines ~86–121). It starts the dashboard server on an ephemeral port (respect `FUSION_RESERVED_PORTS`; never touch port 4040 — see the kill-guard conventions in `scripts/check-no-kill-4040.mjs`), polls an HTTP endpoint until 200 or a hard timeout (~60s), asserts the CLI binary answers `--help`, then shuts down cleanly. Exit code is the verdict. +- **Patterns to follow:** the packaged `--help` smoke in `scripts/release.mjs`; server-start handling in `packages/cli/src/commands/serve.ts` (the real one, not the mocked test). +- **Test scenarios:** + - Happy path: server starts → 200 within timeout → clean shutdown → exit 0. + - Error path: server fails to bind / crashes → nonzero exit with captured stderr. + - Error path: port already in use → picks another ephemeral port rather than failing or killing anything. +- **Verification:** `node scripts/boot-smoke.mjs` exits 0 on a built workspace and nonzero when the dashboard entry point is deliberately broken. + +### U3. Delete `ci.yml`, rewrite the CI-shape test, update CONTRIBUTING wording + +- **Goal:** Remove the dead workflow without leaving the repo self-inconsistent. +- **Requirements:** R5 (revised). +- **Dependencies:** U4 (the new pr-checks shape must be settled so the test asserts it; land in the same PR). +- **Files:** `.github/workflows/ci.yml` (delete), `packages/cli/src/__tests__/ci-workflow.test.ts` (rewrite), `docs/contributing.md`, `README.md` (command block the test pins, if affected). +- **Approach:** The test's first describe block loads `ci.yml` in `beforeAll` — deleting the workflow without removing that block crashes the entire suite at setup, not just one assertion. Delete the whole CI-workflow describe block; rewrite the PR-checks describe block (drop the 4-shard-matrix and no-pre-test-build assertions, add the new invariants); **preserve** the unrelated version.yml/release.yml/test-release.yml/code-signing describe blocks sharing the file. New invariants: `pr-checks.yml` contains the gate jobs (lint, typecheck, build, gate) and no shard matrices; `full-suite.yml` exists, triggers only on push to main, and contains the demoted jobs; `docs/contributing.md` names `pnpm test:gate` as the merge gate (the old "canonical pre-merge gate" string lives at `docs/contributing.md:83`). This test joins the gate suite (via U1's `test:gate`). +- **Patterns to follow:** the existing assertion style in `ci-workflow.test.ts` (YAML load + structural expectations). +- **Test scenarios:** (this unit IS a test) + - Asserts gate job set and absence of shard matrices in `pr-checks.yml`. + - Asserts `full-suite.yml` trigger is push-to-main only (no `pull_request`). + - Asserts `docs/contributing.md` gate wording matches the new commands. +- **Verification:** rewritten test passes against the new workflows and fails if a shard matrix is reintroduced into `pr-checks.yml`. + +### U4. Rework `pr-checks.yml` and create `full-suite.yml` + +- **Goal:** The blocking gate becomes lint + typecheck + build + gate; demoted jobs move to a separate non-blocking workflow. +- **Requirements:** R1, R2, R3, R6. +- **Dependencies:** U1, U2. +- **Files:** `.github/workflows/pr-checks.yml`, `.github/workflows/full-suite.yml` (new). +- **Approach:** `pr-checks.yml` keeps `lint`, `typecheck`, `build`, and gains a `gate` job (boot smoke + `pnpm test:gate`) reusing the dist-artifact cache; it loses `test-shards`, `test-slow`, and `test-inventory-guard`, and its `push: main` trigger (post-merge signal moves to full-suite.yml). `full-suite.yml` runs on `push: branches: [main]` with the 4-shard matrix, engine slow tier, and inventory guard moved verbatim, keeping the per-shard timing artifact upload. Keep the `pretest` guards (`check-no-nohup`, `check-no-kill-4040`) on any path that runs tests. Cutover (manual admin step, do immediately after merge): update branch protection required checks to exactly `Lint`, `Typecheck`, `Build`, `Gate` — stale names like `Test shard 1/4` left required will block every PR forever ("Expected — waiting for status"). Open PRs must rebase onto post-change main before merging. +- **Test scenarios:** covered by U3's rewritten CI-shape test (Covers AE2: a red `full-suite.yml` run does not affect PR mergeability — verify once live by observing a PR merge during a red main run). +- **Verification:** a test PR shows only the 4 gate checks, total wall-clock under ~5 min; a deliberate failure in a demoted test does not block that PR. + +### U5. Quarantine ledger + +- **Goal:** A single recorded place for quarantined tests, feeding both vitest excludes and the 2-week clock. +- **Requirements:** R7, R8. +- **Dependencies:** none. +- **Files:** `scripts/lib/test-quarantine.json` (new), vitest configs of packages that gain quarantined entries (exclude entries maintained by hand), `scripts/check-test-inventory.mjs` (doc comment only — `--diff` exemption note). +- **Approach:** Schema per entry: `{ "file": "", "reason": "", "quarantinedAt": "YYYY-MM-DD" }` — modeled on `scripts/lib/dashboard-curated-skiplist.json` but with the date the ratchet needs. **No loader module, no CLI flag** (a shared module wired into vitest configs would itself be new test machinery — the failure mode the origin rejected). Quarantining a test = add the ledger entry AND add a matching one-line `exclude` entry to that package's vitest config, by hand, in the same commit; the ledger is the dated record, the config exclude is the mechanism. The 2-week sweep is performed by whoever (human or agent) touches the suite, per policy in U7 — an entry is expired when `quarantinedAt` is older than 14 days. Document that `check-test-inventory.mjs --diff` stays unwired because it would fail on ratchet deletions. +- **Patterns to follow:** `scripts/lib/dashboard-curated-skiplist.json` (data file with mandatory `reason`, mirrored by config excludes). +- **Test scenarios:** Test expectation: none — a data file plus hand-maintained config excludes; no executable surface to test. (Covers AE1: a quarantined file listed in the ledger with its config exclude no longer appears in the package's vitest run — verify via vitest `list`.) +- **Verification:** adding a real test file to the ledger plus its config exclude removes it from `pnpm test:gate` and shard discovery without editing the test file itself. + +### U6. Local `pnpm test` re-default + +- **Goal:** Developers cannot OOM from the default command (R11), and changed-code coverage is preserved. +- **Requirements:** R11, R12. +- **Dependencies:** U1. +- **Files:** `scripts/test-changed.mjs`, `package.json` (root). +- **Approach:** `pnpm test` becomes: run `test:gate`, then affected-package tests via the existing changed-file → package → reverse-dependents expansion. Remove the local full-suite escalation: `shouldForceFullSuite` (~line 431) no longer triggers a recursive full run — shared-infra changes now run gate + a bounded affected set, with a printed note naming `pnpm test:full` for the full sweep. The `CI === "true"` force-full branch (~line 1064) is removed (CI no longer calls this script). `test:full`, `test:serial`, `test:fast`, `verify:workspace` keep their current full-suite semantics as explicit opt-ins; docs reposition `verify:workspace` as the deep pre-release check, not the pre-merge gate. +- **Execution note:** characterization-first — `scripts/__tests__/` has existing coverage of test-changed behavior; capture the current selection behavior you're keeping before removing the escalation paths. +- **Test scenarios:** (extend `scripts/__tests__/`) + - Changed file in one package → that package + reverse-dependents selected (unchanged behavior). + - Changed shared-infra file (e.g. `.github/workflows/x.yml`) → no full-suite escalation; gate + affected set only, hint printed. + - `--full` flag still runs the full suite (opt-in preserved). +- **Verification:** `pnpm test` after touching a workflow file completes without spawning the recursive full run; memory stays bounded (no 6GB dashboard lanes invoked). + +### U7. Policy docs: ratchet, appeasement ban, gate semantics + +- **Goal:** The policy is written where humans and agents actually look (R9). +- **Requirements:** R7, R8, R9, R10. +- **Dependencies:** U1–U6 (documents the shipped reality). +- **Files:** `AGENTS.md`, `docs/testing.md`. +- **Approach:** `AGENTS.md`: rewrite line ~69 ("Tests are required. Typechecks/manual checks are not substitutes.") to describe the gate-vs-non-blocking split; add the ratchet as a standing rule adjacent to FN-5048 (~lines 79–85): quarantine on sight via ledger entry; delete after 2 weeks unless rescued with evidence and a root-cause fix; **agents must never appease a flaky test** (no widened timeouts, added retries, loosened assertions — quarantine instead); a flake *inside the gate* is evicted from the allow-list, not skipped; a second quarantine in the same subsystem is a product-race smell — look before the clock runs out (see `docs/solutions/ui-bugs/skill-autocomplete-highlight-reset-on-swr-revalidation.md`). `docs/testing.md`: new section with ledger schema, rescue procedure, gate admission criteria (evidence of value), and the known blind spot stated honestly: the gate does not run the union suite a merge creates — logic regressions outside the curated set land non-blocking by design. Mind the AGENTS.md add/add pointer-line merge convention (`docs/solutions/best-practices/merge-conflict-extraction-vs-semantics-and-parallel-bootstrap.md`). +- **Test scenarios:** Test expectation: none — documentation unit. (AE3 — agent consults AGENTS.md and quarantines instead of appeasing — is enforced by the policy text this unit writes; U3's test pins the CONTRIBUTING wording.) +- **Verification:** AGENTS.md and docs/testing.md describe the shipped gate accurately; no remaining references to the 4-shard PR gate or `verify:workspace` as "the canonical pre-merge gate". + +--- + +## Scope Boundaries + +- No new test machinery: no auto-quarantine, no flake-scoring, no test-value telemetry, no quarantine loader module (see origin). The ledger JSON + hand-maintained vitest config excludes are the entire mechanical surface. +- No root-cause fixing of existing flaky tests; flakes exit via quarantine and deletion. +- Healthy non-gate tests survive indefinitely in `full-suite.yml`; no mass deletion. +- Release pipelines untouched: `version.yml`/`release.yml` already run zero tests (verified), so nothing weakens; the consequence — regressions can reach main and ship behind build+typecheck+smoke — is the accepted thesis of this change. +- Coverage tooling untouched. + +### Deferred to Follow-Up Work + +- Capturing the vitest auto-kill incident, port-4040 kill guards, and `.slow` split conventions into `docs/solutions/` (learnings researcher flagged these exist only in auto-memory/AGENTS.md). +- A scheduled `--write-timings` refresh if shard balance in `full-suite.yml` degrades once PR-driven timing uploads stop (accept staleness initially). +- Extending the `.slow`-style project split to non-engine packages if their non-blocking lanes ever need tiering. + +--- + +## Risks & Dependencies + +- **Branch-protection cutover is the sharpest edge.** Required checks are matched by job name; leaving a removed name required blocks all PRs indefinitely. Mitigation: U4 names the exact new check set; do the admin update immediately after merge; audit open PRs and require rebase. +- **Gate blind spot (deliberate).** Typecheck + build + smoke + curated suite does not test the union a merge creates; documented honestly in U7. +- **Curated suite quality risk.** If the allow-list admits a latent flake, the gate loses trust fast. Mitigation: 5×-consecutive-green screen in U1 verification; eviction rule in U7. +- **Timing snapshot staleness** once shards leave PRs (deferred above) — affects only non-blocking shard balance. +- **Branch protection settings are server-side** — unverifiable from the repo; the actual required-check list at cutover time must be read from GitHub settings, not assumed. \ No newline at end of file diff --git a/scripts/boot-smoke.mjs b/scripts/boot-smoke.mjs index 05505b51af..2143641776 100644 --- a/scripts/boot-smoke.mjs +++ b/scripts/boot-smoke.mjs @@ -74,12 +74,16 @@ async function pollHealth(port, deadline) { const url = `http://127.0.0.1:${port}/api/health`; let lastError = "no response"; while (Date.now() < deadline) { + const controller = new AbortController(); + const abortTimer = setTimeout(() => controller.abort(), 2_000); try { - const res = await fetch(url, { signal: AbortSignal.timeout(2_000) }); + const res = await fetch(url, { signal: controller.signal }); if (res.status === 200) return; lastError = `HTTP ${res.status}`; } catch (err) { lastError = err?.cause?.code ?? err?.name ?? String(err); + } finally { + clearTimeout(abortTimer); } await new Promise((r) => setTimeout(r, 500)); } From 9a2e721ffafec76e79d9ca585957c842e7f5cfcd Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Fri, 5 Jun 2026 09:13:02 -0700 Subject: [PATCH 08/12] chore(test): fold gate guard scripts into test:gate (pnpm pre-scripts not relied on in CI) --- package.json | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/package.json b/package.json index def9829cae..24269b677d 100644 --- a/package.json +++ b/package.json @@ -16,8 +16,7 @@ "scripts": { "pretest": "node scripts/check-no-nohup.mjs && node scripts/check-no-kill-4040.mjs", "pretest:full": "node scripts/check-no-nohup.mjs && node scripts/check-no-kill-4040.mjs", - "pretest:gate": "node scripts/check-no-nohup.mjs && node scripts/check-no-kill-4040.mjs", - "test:gate": "pnpm --filter @fusion/engine test:core && pnpm --filter @runfusion/fusion test:ci-shape", + "test:gate": "node scripts/check-no-nohup.mjs && node scripts/check-no-kill-4040.mjs && pnpm --filter @fusion/engine test:core && pnpm --filter @runfusion/fusion test:ci-shape", "smoke:boot": "node scripts/boot-smoke.mjs", "local": "node scripts/start-local.mjs", "dev": "node scripts/dev-with-memory.mjs", From 3387ac80968ca40395fc97c9e16a1701f713ba72 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Fri, 5 Jun 2026 09:23:15 -0700 Subject: [PATCH 09/12] fix(review): apply autofix feedback --- .github/workflows/full-suite.yml | 8 +++++-- .github/workflows/pr-checks.yml | 4 ++++ docs/testing.md | 2 ++ scripts/__tests__/test-changed.test.mjs | 2 +- scripts/boot-smoke.mjs | 30 +++++++++++++++++++++++-- scripts/test-changed.mjs | 6 +++++ 6 files changed, 47 insertions(+), 5 deletions(-) diff --git a/.github/workflows/full-suite.yml b/.github/workflows/full-suite.yml index a207d4e0bc..286674c0d9 100644 --- a/.github/workflows/full-suite.yml +++ b/.github/workflows/full-suite.yml @@ -12,9 +12,13 @@ on: push: branches: [main] +# Key the concurrency group by SHA, not ref: on push to main the ref is +# always refs/heads/main, so a ref-keyed group with cancel-in-progress would +# let consecutive merges cancel each other's runs — silently skipping the +# only coverage for everything the gate dropped. Per-SHA groups never collide. concurrency: - group: full-suite-${{ github.ref }} - cancel-in-progress: true + group: full-suite-${{ github.sha }} + cancel-in-progress: false # FN-4863: Opt JavaScript actions into Node 24 ahead of GitHub's forced cutover on 2026-06-02. env: diff --git a/.github/workflows/pr-checks.yml b/.github/workflows/pr-checks.yml index fb8321ca39..1f4952c5fa 100644 --- a/.github/workflows/pr-checks.yml +++ b/.github/workflows/pr-checks.yml @@ -79,6 +79,10 @@ jobs: gate: name: Gate runs-on: ubuntu-latest + # The gate's value is speed; without a job timeout a hung build or + # deadlocked vitest worker blocks every PR for GitHub's default 6 hours. + # Expected runtime is ~3-5 min. + timeout-minutes: 15 steps: - name: Checkout uses: actions/checkout@v4 diff --git a/docs/testing.md b/docs/testing.md index 4c01d04ef9..e64608225f 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -121,6 +121,8 @@ Flaky tests are quarantined ON SIGHT and deleted on a 2-week clock. This is writ **Gate eviction:** a flake inside the merge gate cannot block all merges while red — it is evicted by removing its line from the `engine-core` allow-list (no quarantine entry needed unless it should also leave the non-blocking tier). +**Gate admission:** the mirror operation — add the test's path to the `engine-core` `include` array in `packages/engine/vitest.config.ts`, citing the evidence of value (a real regression it caught) in the PR. Keep the project under its ~60s wall-clock budget. + **Product-race escalation:** a second quarantine in the same subsystem is a smell that the flake is a real product race, not test noise — look at the product code before deleting (a dashboard flake was "stabilized" three times before being found to be a real race; see `docs/solutions/ui-bugs/skill-autocomplete-highlight-reset-on-swr-revalidation.md`). ## CI shard balancing (duration-weighted) diff --git a/scripts/__tests__/test-changed.test.mjs b/scripts/__tests__/test-changed.test.mjs index 0f4fa23b6b..fd86cbdeeb 100644 --- a/scripts/__tests__/test-changed.test.mjs +++ b/scripts/__tests__/test-changed.test.mjs @@ -124,7 +124,7 @@ test("shouldForceFullSuite: returns true when scripts/check-test-isolation.mjs c }); test("shouldForceFullSuite: returns true when a GitHub workflow changed", () => { - assert.equal(shouldForceFullSuite([".github/workflows/ci.yml"]), true); + assert.equal(shouldForceFullSuite([".github/workflows/pr-checks.yml"]), true); }); test("shouldForceFullSuite: returns false for .changeset/*.md summary files", () => { diff --git a/scripts/boot-smoke.mjs b/scripts/boot-smoke.mjs index 2143641776..79940c9229 100644 --- a/scripts/boot-smoke.mjs +++ b/scripts/boot-smoke.mjs @@ -12,6 +12,11 @@ * (port-4040-allowlist: this file only ever AVOIDS the reserved ports — it * requests an ephemeral port and rejects reserved ones; it never binds, * probes, or kills them.) + * (process-supervisor-allowlist: raw spawn is intentional here — this is a + * standalone repo script outside the package graph, the child is attached + * (not detached), and lifecycle is bounded by the timeouts + signal handlers + * below; importing superviseSpawn from @fusion/core would invert the + * dependency direction for a build-time smoke check.) * - Never binds or touches port 4040 / FUSION_RESERVED_PORTS — an ephemeral * port is requested from the OS (listen on 0) and double-checked against * the reserved list. @@ -128,10 +133,26 @@ async function main() { child.stdout.on("data", (d) => (stderrBuf += d)); const cleanup = () => { - if (child.exitCode === null && !child.killed) child.kill("SIGTERM"); + // 'exit' handlers cannot await: escalate straight to SIGKILL so a child + // that ignores SIGTERM is never orphaned holding the port/tmpdir. The + // graceful SIGTERM path below runs before this on the success path. + try { + if (child.exitCode === null && !child.killed) child.kill("SIGKILL"); + } catch { + // ESRCH: child already reaped between the check and the kill — fine. + } rmSync(isolatedHome, { recursive: true, force: true }); }; process.on("exit", cleanup); + // Node does NOT fire 'exit' on signals by default. A cancelled CI job + // (timeout, manual cancel, runner eviction) sends SIGTERM — without these + // handlers the serve child would be orphaned. + for (const sig of ["SIGTERM", "SIGINT"]) { + process.on(sig, () => { + cleanup(); + process.exit(sig === "SIGINT" ? 130 : 143); + }); + } const exitedEarly = new Promise((resolve) => { child.once("exit", (code, signal) => resolve({ code, signal })); @@ -150,7 +171,12 @@ async function main() { console.log(`boot-smoke: GET /api/health 200 on :${port}`); // 3. Clean shutdown of OUR child only. - child.kill("SIGTERM"); + try { + child.kill("SIGTERM"); + } catch { + // ESRCH: server exited between the health check and the kill — the + // exitedEarly promise below already carries its exit code. + } const { code, signal } = await Promise.race([ exitedEarly, new Promise((resolve) => diff --git a/scripts/test-changed.mjs b/scripts/test-changed.mjs index 461de987bb..78567fdf72 100644 --- a/scripts/test-changed.mjs +++ b/scripts/test-changed.mjs @@ -1158,6 +1158,12 @@ export function main(argv = process.argv.slice(2)) { // Cache-fresh fast path: nothing to run. Emit a fast-path mode line, run only // the (now cheap) isolation guard, and skip skill-sync, artifact-ensure, // HOME creation, and prune. + // + // NOTE: this path is reachable only in CHANGED mode (gate mode sets hasWork + // above), and it intentionally skips the merge-gate suite too: an all-cache- + // fresh changed run means the engine/cli content feeding the gate suite is + // byte-identical to a previously green run. Any shared-infra change that + // could invalidate that reasoning routes to gate mode instead of here. if (!hasWork) { console.log("[test-changed] fast-path=cache-fresh (no packages to run)."); console.log( From 20153a3ef3875533a48e63af5dd3c893dc452f4c Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Fri, 5 Jun 2026 09:41:32 -0700 Subject: [PATCH 10/12] chore(plan): mark fast-trusted-test-gate plan completed --- .../2026-06-04-001-refactor-fast-trusted-test-gate-plan.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/plans/2026-06-04-001-refactor-fast-trusted-test-gate-plan.md b/docs/plans/2026-06-04-001-refactor-fast-trusted-test-gate-plan.md index ddc10f5212..f89647b905 100644 --- a/docs/plans/2026-06-04-001-refactor-fast-trusted-test-gate-plan.md +++ b/docs/plans/2026-06-04-001-refactor-fast-trusted-test-gate-plan.md @@ -2,7 +2,7 @@ title: "refactor: Shrink PR merge gate to a fast trusted set with a deletion ratchet" type: refactor -status: active +status: completed date: 2026-06-04 origin: docs/brainstorms/2026-06-04-fast-trusted-test-gate-requirements.md From 5aa6cad16842dc704803b8e97d6b3c340898a83d Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Fri, 5 Jun 2026 13:58:34 -0700 Subject: [PATCH 11/12] Address PR review feedback (#1453) - boot-smoke: shutdown verdict now requires SIGTERM actually delivered and a clean exit (code 0 or SIGTERM); EADDRINUSE port race retries with a fresh port (3 attempts) - test-changed: rename shouldForceFullSuite -> isSharedInfraChange (it routes to gate mode, not full); run the changed-mode gate under the isolation guard - workflows: least-privilege permissions (contents: read) on pr-checks and full-suite --- .github/workflows/full-suite.yml | 5 ++ .github/workflows/pr-checks.yml | 4 ++ .../src/__tests__/root-test-command.test.ts | 8 +-- scripts/__tests__/test-changed.test.mjs | 58 +++++++-------- scripts/boot-smoke.mjs | 72 ++++++++++++++----- scripts/test-changed.mjs | 14 ++-- 6 files changed, 106 insertions(+), 55 deletions(-) diff --git a/.github/workflows/full-suite.yml b/.github/workflows/full-suite.yml index 286674c0d9..1859ef490e 100644 --- a/.github/workflows/full-suite.yml +++ b/.github/workflows/full-suite.yml @@ -20,6 +20,11 @@ concurrency: group: full-suite-${{ github.sha }} cancel-in-progress: false +# Least-privilege token: jobs only read the repo (checkout + cache) and upload +# workflow artifacts (timings), which needs no extra permission scope. +permissions: + contents: read + # FN-4863: Opt JavaScript actions into Node 24 ahead of GitHub's forced cutover on 2026-06-02. env: FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: "true" diff --git a/.github/workflows/pr-checks.yml b/.github/workflows/pr-checks.yml index 1f4952c5fa..b8b75c6efd 100644 --- a/.github/workflows/pr-checks.yml +++ b/.github/workflows/pr-checks.yml @@ -21,6 +21,10 @@ concurrency: group: pr-checks-${{ github.ref }} cancel-in-progress: true +# Least-privilege token: every job here only reads the repo (checkout + cache). +permissions: + contents: read + # FN-4863: Opt JavaScript actions into Node 24 ahead of GitHub's forced cutover on 2026-06-02. env: FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: "true" diff --git a/packages/cli/src/__tests__/root-test-command.test.ts b/packages/cli/src/__tests__/root-test-command.test.ts index 4a8972b0a7..80b594b1a5 100644 --- a/packages/cli/src/__tests__/root-test-command.test.ts +++ b/packages/cli/src/__tests__/root-test-command.test.ts @@ -3,7 +3,7 @@ import { decideExecutionPlan, normalizeForwardedArgs, resolveAffectedPackages, - shouldForceFullSuite, + isSharedInfraChange, } from "../../../../scripts/test-changed.mjs"; import { computeSplitPlan, parseShardArgs, planShardAssignments, selectShardPackages } from "../../../../scripts/ci-test-shard.mjs"; @@ -52,9 +52,9 @@ describe("root test command changed-only planning", () => { }); it("marks root workflow/config changes as shared-infra (gate-mode) triggers", () => { - expect(shouldForceFullSuite([".github/workflows/pr-checks.yml"])).toBe(true); - expect(shouldForceFullSuite(["package.json"])).toBe(true); - expect(shouldForceFullSuite(["packages/core/src/store.ts"])).toBe(false); + expect(isSharedInfraChange([".github/workflows/pr-checks.yml"])).toBe(true); + expect(isSharedInfraChange(["package.json"])).toBe(true); + expect(isSharedInfraChange(["packages/core/src/store.ts"])).toBe(false); }); it("strips forwarded silent flags so package vitest scripts do not receive duplicates", () => { diff --git a/scripts/__tests__/test-changed.test.mjs b/scripts/__tests__/test-changed.test.mjs index fd86cbdeeb..1aa9ff284f 100644 --- a/scripts/__tests__/test-changed.test.mjs +++ b/scripts/__tests__/test-changed.test.mjs @@ -10,7 +10,7 @@ import assert from "node:assert/strict"; import { buildPackageDirByName, buildReverseDependencyMap, - shouldForceFullSuite, + isSharedInfraChange, resolveAffectedPackages, decideExecutionPlan, computePackageHash, @@ -101,72 +101,72 @@ function hashWithFakeGit(pkgDir, blobSha) { } // --------------------------------------------------------------------------- -// shouldForceFullSuite +// isSharedInfraChange // --------------------------------------------------------------------------- -test("shouldForceFullSuite: returns false for pure package changes", () => { +test("isSharedInfraChange: returns false for pure package changes", () => { assert.equal( - shouldForceFullSuite(["packages/engine/src/foo.ts", "packages/core/src/bar.ts"]), + isSharedInfraChange(["packages/engine/src/foo.ts", "packages/core/src/bar.ts"]), false, ); }); -test("shouldForceFullSuite: returns true when pnpm-lock.yaml changed", () => { - assert.equal(shouldForceFullSuite(["pnpm-lock.yaml"]), true); +test("isSharedInfraChange: returns true when pnpm-lock.yaml changed", () => { + assert.equal(isSharedInfraChange(["pnpm-lock.yaml"]), true); }); -test("shouldForceFullSuite: returns true when scripts/test-changed.mjs changed", () => { - assert.equal(shouldForceFullSuite(["scripts/test-changed.mjs"]), true); +test("isSharedInfraChange: returns true when scripts/test-changed.mjs changed", () => { + assert.equal(isSharedInfraChange(["scripts/test-changed.mjs"]), true); }); -test("shouldForceFullSuite: returns true when scripts/check-test-isolation.mjs changed", () => { - assert.equal(shouldForceFullSuite(["scripts/check-test-isolation.mjs"]), true); +test("isSharedInfraChange: returns true when scripts/check-test-isolation.mjs changed", () => { + assert.equal(isSharedInfraChange(["scripts/check-test-isolation.mjs"]), true); }); -test("shouldForceFullSuite: returns true when a GitHub workflow changed", () => { - assert.equal(shouldForceFullSuite([".github/workflows/pr-checks.yml"]), true); +test("isSharedInfraChange: returns true when a GitHub workflow changed", () => { + assert.equal(isSharedInfraChange([".github/workflows/pr-checks.yml"]), true); }); -test("shouldForceFullSuite: returns false for .changeset/*.md summary files", () => { - assert.equal(shouldForceFullSuite([".changeset/fn-5157-test-changed-allowlist.md"]), false); +test("isSharedInfraChange: returns false for .changeset/*.md summary files", () => { + assert.equal(isSharedInfraChange([".changeset/fn-5157-test-changed-allowlist.md"]), false); }); -test("shouldForceFullSuite: still returns true for .changeset/config.json", () => { - assert.equal(shouldForceFullSuite([".changeset/config.json"]), true); +test("isSharedInfraChange: still returns true for .changeset/config.json", () => { + assert.equal(isSharedInfraChange([".changeset/config.json"]), true); }); -test("shouldForceFullSuite: returns false for allowlisted root markdown files", () => { +test("isSharedInfraChange: returns false for allowlisted root markdown files", () => { for (const file of ["AGENTS.md", "README.md", "CHANGELOG.md", "CONTRIBUTING.md", "SECURITY.md"]) { - assert.equal(shouldForceFullSuite([file]), false, `${file} should stay on changed-only mode`); + assert.equal(isSharedInfraChange([file]), false, `${file} should stay on changed-only mode`); } }); -test("shouldForceFullSuite: returns false for .fusion artifacts", () => { - assert.equal(shouldForceFullSuite([".fusion/memory/MEMORY.md"]), false); - assert.equal(shouldForceFullSuite([".fusion/tasks/FN-5157/PROMPT.md"]), false); +test("isSharedInfraChange: returns false for .fusion artifacts", () => { + assert.equal(isSharedInfraChange([".fusion/memory/MEMORY.md"]), false); + assert.equal(isSharedInfraChange([".fusion/tasks/FN-5157/PROMPT.md"]), false); }); -test("shouldForceFullSuite: still returns true for root config edges", () => { +test("isSharedInfraChange: still returns true for root config edges", () => { for (const file of ["tsconfig.json", ".npmrc", "Dockerfile"]) { - assert.equal(shouldForceFullSuite([file]), true, `${file} should still force the full suite`); + assert.equal(isSharedInfraChange([file]), true, `${file} should still force the full suite`); } }); -test("shouldForceFullSuite: mixed diff with only allowlisted root paths returns false", () => { +test("isSharedInfraChange: mixed diff with only allowlisted root paths returns false", () => { assert.equal( - shouldForceFullSuite(["AGENTS.md", ".changeset/foo.md", ".fusion/tasks/FN-5154/task.json"]), + isSharedInfraChange(["AGENTS.md", ".changeset/foo.md", ".fusion/tasks/FN-5154/task.json"]), false, ); }); -test("shouldForceFullSuite: mixed diff with allowlisted root path plus explicit trigger returns true", () => { - assert.equal(shouldForceFullSuite(["AGENTS.md", "pnpm-lock.yaml"]), true); +test("isSharedInfraChange: mixed diff with allowlisted root path plus explicit trigger returns true", () => { + assert.equal(isSharedInfraChange(["AGENTS.md", "pnpm-lock.yaml"]), true); }); -test("shouldForceFullSuite: FN-5157 reproduction keeps FN-5154 diff in changed-only mode", () => { +test("isSharedInfraChange: FN-5157 reproduction keeps FN-5154 diff in changed-only mode", () => { // FN-5157: AGENTS.md + changeset summaries previously tripped the root catch-all and forced a full suite for the FN-5154 diff. assert.equal( - shouldForceFullSuite([ + isSharedInfraChange([ "AGENTS.md", ".changeset/FN-5136-quick-entry-submit-lock.md", ".changeset/FN-5141-soft-delete-terminology.md", diff --git a/scripts/boot-smoke.mjs b/scripts/boot-smoke.mjs index 79940c9229..b854f0dfa9 100644 --- a/scripts/boot-smoke.mjs +++ b/scripts/boot-smoke.mjs @@ -40,6 +40,9 @@ const cliBin = path.join(repoRoot, "packages/cli/bin.mjs"); const HEALTH_TIMEOUT_MS = 60_000; const SHUTDOWN_TIMEOUT_MS = 15_000; +// Ephemeral-port TOCTOU: retry the whole boot with a fresh port when the +// child loses the bind race (EADDRINUSE). +const BOOT_ATTEMPTS = 3; function parsePortList(raw) { return String(raw ?? "") @@ -110,6 +113,36 @@ async function main() { console.log("boot-smoke: `fn --help` OK"); // 2. Real server boot on an ephemeral port with an isolated HOME. + // The ephemeral-port probe is inherently TOCTOU (probe closes before the + // server binds), so an EADDRINUSE loss on a busy machine retries with a + // fresh port instead of failing the gate. + let cleanup = () => {}; + process.on("exit", () => cleanup()); + // Node does NOT fire 'exit' on signals by default. A cancelled CI job + // (timeout, manual cancel, runner eviction) sends SIGTERM — without these + // handlers the serve child would be orphaned. + for (const sig of ["SIGTERM", "SIGINT"]) { + process.on(sig, () => { + cleanup(); + process.exit(sig === "SIGINT" ? 130 : 143); + }); + } + + for (let attempt = 1; attempt <= BOOT_ATTEMPTS; attempt++) { + const result = await bootAndVerify(attempt, (fn) => (cleanup = fn)); + if (result === "retry-port") continue; + console.log("boot-smoke: PASS"); + return; + } + fail(`could not bind a server port after ${BOOT_ATTEMPTS} attempts (EADDRINUSE each time)`); +} + +/** + * One boot attempt: spawn, poll health, verify SIGTERM shutdown. + * Returns "retry-port" when the child lost the ephemeral-port race + * (EADDRINUSE); calls fail() (which exits) on any real failure. + */ +async function bootAndVerify(attempt, registerCleanup) { const port = await getEphemeralPort(); const isolatedHome = mkdtempSync(path.join(tmpdir(), "fusion-boot-smoke-")); let stderrBuf = ""; @@ -132,7 +165,7 @@ async function main() { child.stderr.on("data", (d) => (stderrBuf += d)); child.stdout.on("data", (d) => (stderrBuf += d)); - const cleanup = () => { + registerCleanup(() => { // 'exit' handlers cannot await: escalate straight to SIGKILL so a child // that ignores SIGTERM is never orphaned holding the port/tmpdir. The // graceful SIGTERM path below runs before this on the success path. @@ -142,17 +175,7 @@ async function main() { // ESRCH: child already reaped between the check and the kill — fine. } rmSync(isolatedHome, { recursive: true, force: true }); - }; - process.on("exit", cleanup); - // Node does NOT fire 'exit' on signals by default. A cancelled CI job - // (timeout, manual cancel, runner eviction) sends SIGTERM — without these - // handlers the serve child would be orphaned. - for (const sig of ["SIGTERM", "SIGINT"]) { - process.on(sig, () => { - cleanup(); - process.exit(sig === "SIGINT" ? 130 : 143); - }); - } + }); const exitedEarly = new Promise((resolve) => { child.once("exit", (code, signal) => resolve({ code, signal })); @@ -166,16 +189,25 @@ async function main() { }), ]); } catch (err) { + if (/EADDRINUSE/.test(stderrBuf) && attempt < BOOT_ATTEMPTS) { + console.log(`boot-smoke: port :${port} lost to another process (EADDRINUSE), retrying with a fresh port (attempt ${attempt}/${BOOT_ATTEMPTS})`); + await exitedEarly; // child is already dead or dying; wait so cleanup is race-free + rmSync(isolatedHome, { recursive: true, force: true }); + return "retry-port"; + } fail(err.message, stderrBuf); } console.log(`boot-smoke: GET /api/health 200 on :${port}`); - // 3. Clean shutdown of OUR child only. + // 3. Clean shutdown of OUR child only. The verdict requires BOTH that + // SIGTERM was actually delivered (a server that died between the health + // check and here is a failure, not a pass) AND that the exit was clean + // (SIGTERM or exit code 0) — a crash after serving is a broken boot path. + let sigtermSent = false; try { - child.kill("SIGTERM"); + sigtermSent = child.kill("SIGTERM"); } catch { - // ESRCH: server exited between the health check and the kill — the - // exitedEarly promise below already carries its exit code. + // ESRCH: server already exited — sigtermSent stays false and fails below. } const { code, signal } = await Promise.race([ exitedEarly, @@ -183,12 +215,18 @@ async function main() { setTimeout(() => resolve({ code: null, signal: "timeout" }), SHUTDOWN_TIMEOUT_MS), ), ]); + if (!sigtermSent) { + fail(`server exited on its own after the health check (${code ?? `signal ${signal}`}) — SIGTERM shutdown could not be verified`, stderrBuf); + } if (signal === "timeout") { child.kill("SIGKILL"); fail("server did not shut down within 15s of SIGTERM", stderrBuf); } + if (signal !== "SIGTERM" && code !== 0) { + fail(`server exited uncleanly on SIGTERM (${code ?? `signal ${signal}`})`, stderrBuf); + } console.log(`boot-smoke: clean shutdown (${code ?? signal})`); - console.log("boot-smoke: PASS"); + return "ok"; } main().catch((err) => fail(err.message ?? String(err))); diff --git a/scripts/test-changed.mjs b/scripts/test-changed.mjs index 78567fdf72..59a07aa1ed 100644 --- a/scripts/test-changed.mjs +++ b/scripts/test-changed.mjs @@ -113,7 +113,7 @@ const HASH_VERSION_PREFIX = "v2"; * alone would miss those, so we fold the tree in globally — the simplest * provably-correct choice (mirrors the tsconfig.base.json treatment). * - * NOTE: this list intentionally overlaps `shouldForceFullSuite`'s + * NOTE: this list intentionally overlaps `isSharedInfraChange`'s * `fullSuitePaths` (which decides full-suite mode, a different axis than cache * busting). When adding a new shared root config input, consider both lists. */ @@ -428,12 +428,13 @@ function isTestIrrelevantRootPath(file) { return ["README", "CHANGELOG.md", "LICENSE", "LICENSE.md"].includes(file); } -export function shouldForceFullSuite(changedFiles) { +export function isSharedInfraChange(changedFiles) { // NOTE: overlaps SHARED_HASH_INPUT_PATHS by intent (different axis: this list // signals shared-infra changes; that one busts every package's cache hash). // When adding a new shared root config input, consider both lists. // - // HISTORY: this signal used to escalate `pnpm test` to an implicit full + // HISTORY: previously named `shouldForceFullSuite` — this signal used to + // escalate `pnpm test` to an implicit full // recursive run — which was the local OOM path (two concurrent heavy // packages, 6GB dashboard heaps). Since the merge-gate redesign // (docs/plans/2026-06-04-001-refactor-fast-trusted-test-gate-plan.md) it @@ -1026,7 +1027,7 @@ export function decideExecutionPlan({ if (!comparisonBase) return { mode: "gate", reason: "missing-comparison-base" }; if (!changedFiles) return { mode: "gate", reason: "diff-failed" }; if (changedFiles.length === 0) return { mode: "gate", reason: "no-changes" }; - if (shouldForceFullSuite(changedFiles)) return { mode: "gate", reason: "shared-infra-changed" }; + if (isSharedInfraChange(changedFiles)) return { mode: "gate", reason: "shared-infra-changed" }; const affectedPackages = resolveAffectedPackages(changedFiles, packageNameByDir); if (!affectedPackages || affectedPackages.length === 0) return { mode: "gate", reason: "no-affected-package" }; @@ -1234,7 +1235,10 @@ export function main(argv = process.argv.slice(2)) { // affected expansion preserves changed-code coverage. Overlap (engine in the // affected set re-runs the engine-core files) is accepted by design. console.log("[test-changed] running merge-gate suite (pnpm test:gate) before affected packages."); - run("pnpm", ["test:gate"], { env: isolatedHomeEnv }); + // Run the gate under the same isolation guard as the affected set — a gate + // suite leak must trip the checker, not silently become the "before" state + // of the later run. + runMaybeIsolated("pnpm", ["test:gate"], { env: isolatedHomeEnv }); const filterArgs = activePackages.flatMap((pkg) => ["--filter", pkg]); console.log(`[test-changed] running tests for changed packages: ${activePackages.join(", ")}`); From baac9a12f035712d83860217c9b77d923046a7a6 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Fri, 5 Jun 2026 14:29:07 -0700 Subject: [PATCH 12/12] docs(solutions): capture thin-trusted-merge-gate pattern; refine Testing vocabulary in CONCEPTS.md --- CONCEPTS.md | 11 ++- .../thin-trusted-merge-gate.md | 99 +++++++++++++++++++ 2 files changed, 108 insertions(+), 2 deletions(-) create mode 100644 docs/solutions/architecture-patterns/thin-trusted-merge-gate.md diff --git a/CONCEPTS.md b/CONCEPTS.md index 1761ed67d8..6d47e1beca 100644 --- a/CONCEPTS.md +++ b/CONCEPTS.md @@ -186,10 +186,17 @@ A workflow-declared, typed task field (`string | text | number | boolean | enum ## Testing ### Merge Gate -The minimal set of merge-blocking PR checks: typecheck, build, boot smoke, and a small curated core-engine suite. The gate is the only test signal that can block a PR; admission requires evidence of value, and tests never graduate into it by default. All other tests run non-blocking. +The minimal set of merge-blocking PR checks: lint, typecheck, build, a Boot Smoke, and a small curated engine test suite. The gate is the only test signal that can block a PR; all other tests run non-blocking after merge. + +Gate membership is an explicit allow-list, never a glob: a test earns its slot with evidence of value and never graduates in by default. A flake inside the gate is *evicted* — its allow-list entry is removed — which deliberately requires no green run from the flaky test itself, so the gate can always be repaired while red. + +### Boot Smoke +The gate's "app starts and serves" proof: the CLI answers its help command and a real server boots on a throwaway port, answers its health endpoint, and shuts down cleanly on signal. A pass requires both that the shutdown signal was actually delivered and that the exit was clean — a crash after serving is a failed boot path, not a pass. ### Deletion Ratchet -The standing policy for flaky tests: a test observed failing without a corresponding real bug is quarantined on sight (removed from all runs, not retried, not patched), then deleted after 2 weeks unless rescued with evidence it catches real regressions. Rescue means fixing the flake at the root; appeasement (widened timeouts, added retries, loosened assertions) is prohibited, for agents especially. +The standing policy for flaky tests: a test observed failing without a corresponding real bug is quarantined on sight — a dated ledger entry plus exclusion from all runs, not retried, not patched — then deleted 2 weeks later unless rescued with evidence it catches real regressions plus a root-cause fix. Appeasement (widened timeouts, added retries, loosened assertions) is prohibited, for agents especially. + +A second quarantine in the same subsystem is a product-race smell: the flake may be a real bug, so the product code gets a look before the deletion clock runs out. Gate flakes exit by Merge Gate eviction rather than quarantine, unless they should also leave the non-blocking tier. ## Flagged ambiguities diff --git a/docs/solutions/architecture-patterns/thin-trusted-merge-gate.md b/docs/solutions/architecture-patterns/thin-trusted-merge-gate.md new file mode 100644 index 0000000000..967384ce78 --- /dev/null +++ b/docs/solutions/architecture-patterns/thin-trusted-merge-gate.md @@ -0,0 +1,99 @@ +--- +title: "Thin trusted merge gate with a flaky-test deletion ratchet" +date: 2026-06-05 +category: architecture-patterns +module: ci-test-gate +problem_type: architecture_pattern +component: testing_framework +severity: high +applies_when: + - "PR test gate failures are flake-dominated — most red runs do not correspond to a real regression in the PR" + - "No one can recall the last time a gate failure caught a real user-facing bug" + - "Agents or engineers are appeasing flaky tests (widened timeouts, added retries, loosened assertions)" + - "Fix-and-rerun cycles dominate PR shipping time over actual coding" + - "Local default test command escalates to a full-suite run that OOMs dev machines" +tags: [ci, merge-gate, flaky-tests, test-quarantine, deletion-ratchet, vitest, monorepo, developer-experience] +--- + +# Thin trusted merge gate with a flaky-test deletion ratchet + +## Context + +The PR test gate had undergone trust collapse: 9 required CI checks (4 duration-balanced test shards at 4–10 min each, an engine slow tier, a dashboard inventory guard) failed mostly for flaky/infra reasons. ~70% of maintainer shipping time went to an agent-fix-and-rerun loop, and no recalled PR test failure had ever caught a real user-facing bug — the gate was pure cost. Worse, agents "stabilized" flakes by widening timeouts and adding retries, draining each test's remaining signal (the suite rotted from the inside). A separate incident compounded the distrust: the fn TUI was SIGKILLing all vitest processes on a broken memory metric, producing silent exit-137 deaths misread as flakiness/OOM (auto memory [claude]). Locally, `pnpm test` escalated to an implicit full recursive run on any shared-infra change — the dev-machine OOM path. + +Shipped in Runfusion/Fusion#1453 (origin: `docs/brainstorms/2026-06-04-fast-trusted-test-gate-requirements.md`, plan: `docs/plans/2026-06-04-001-refactor-fast-trusted-test-gate-plan.md`). + +## Guidance + +The pattern: **thin trusted gate on PRs + demoted non-blocking tier on main + deletion ratchet as written policy**. + +**1. Block PRs on a minimal trusted set, and pin that set with a test.** PRs block on exactly Lint, Typecheck, Build, Gate (`.github/workflows/pr-checks.yml`). The CI-shape test (`packages/cli/src/__tests__/ci-workflow.test.ts`) — itself part of the gate — makes drift fail loudly: + +```typescript +it("blocks PRs on exactly lint, typecheck, build, and gate", () => { + expect(Object.keys(workflow.jobs ?? {}).sort()).toEqual(["build", "gate", "lint", "typecheck"]); +}); +``` + +**2. The Gate job = boot smoke + curated allow-list suite.** The boot smoke (`scripts/boot-smoke.mjs`) proves the app actually starts: CLI answers `--help`, a real `fn serve` returns `/api/health` 200 on an ephemeral port (isolated `$HOME`), and shutdown is SIGTERM-verified. The test half is `pnpm test:gate`: a curated `engine-core` vitest project (`packages/engine/vitest.config.ts`) whose membership is an **explicit file-by-file allow-list, not a glob** — 20 files, ~4s wall, ~1,025 tests, selected from committed per-file timing data (`scripts/test-timings.json`) with a determinism criterion (no real-git contention suites, no `*.slow.test.ts`, no shared-state bleed). + +**3. Demote everything else to a separate non-blocking workflow.** `full-suite.yml` runs the shards/slow tier/inventory guard on push to main only — a red run there is information, never a merge stopper. A separate workflow file (not `continue-on-error` jobs) keeps a red full-suite run from painting the gate's workflow red. Key the concurrency group by SHA: + +```yaml +concurrency: + group: full-suite-${{ github.sha }} # ref-keyed + cancel-in-progress would let + cancel-in-progress: false # consecutive merges cancel each other's coverage +``` + +**4. The deletion ratchet is policy with minimal mechanics.** A test that fails without a corresponding real bug is quarantined on sight: a dated entry in `scripts/lib/test-quarantine.json` (`file`, `reason` + failing-run link, `quarantinedAt`) plus a hand-maintained `exclude` line in that package's vitest config, same commit. The entry expires in 14 days — then the test is deleted unless rescued with evidence it catches real regressions plus a root-cause fix. There is deliberately no loader module and no automation; `check-test-inventory.mjs --diff` stays unwired because a snapshot diff guard would fail on exactly the deletions the ratchet performs. Appeasement (timeouts/retries/loosened assertions) is banned outright in `AGENTS.md` — for agents especially. + +**5. Remove implicit full-suite escalation from the local default.** `scripts/test-changed.mjs` routes every ambiguous condition to gate mode instead of an implicit full run: + +```javascript +if (!comparisonBase) return { mode: "gate", reason: "missing-comparison-base" }; +if (!changedFiles) return { mode: "gate", reason: "diff-failed" }; +if (changedFiles.length === 0) return { mode: "gate", reason: "no-changes" }; +if (isSharedInfraChange(changedFiles)) return { mode: "gate", reason: "shared-infra-changed" }; +``` + +The full suite runs only on explicit opt-in (`--full` / `pnpm test:full`). In changed mode the gate suite runs first, under the isolation guard. + +## Why This Matters + +- **Red means real.** A gate with a high false-positive rate teaches everyone that red is noise. A gate small enough to curate and cheap enough to evict from restores the only property a merge gate needs. +- **The allow-list solves the eviction chicken-and-egg.** Under a glob, removing a flaky gate test needs a PR that passes the flaky gate. Under an allow-list, eviction is deleting one line — the eviction PR never waits on the flaky test. +- **Policy beats machinery.** This repo's history answered test pain with more test machinery (sharding, isolation checks, lock runners, kill guards). The ratchet is a written rule plus a dated JSON record; automation would let entries accumulate silently. +- **Appeasement is how suites rot.** The ratchet leaves exactly two exits for a flake: deletion or a root-cause rescue. There is no "stabilize it" option — the prior suite was the proof of where that leads. +- **The blind spot is documented, not hidden.** The gate does not run the union suite a merge creates; logic regressions outside the curated set land non-blocking by design (`docs/testing.md`). Stating this honestly beats the illusion of coverage that 9 untrusted jobs provided. + +## When to Apply + +- Trust collapse indicators (see `applies_when`): flake-dominated reds, zero recalled catches, appeasement loops, fix-and-rerun dominating shipping time, reflexive dismissal of red CI. +- When eviction must be cheap: an explicit allow-list is the right gate shape even for small suites — one flaky gate test poisons the gate for everyone. +- **Counter-case:** a repeatedly "stabilized" flake can be a real product race — see `../ui-bugs/skill-autocomplete-highlight-reset-on-swr-revalidation.md` (a flake "fixed" three times that was a genuine race). The complete triage tree: root cause known → fix the invariant; root cause unknown and no real bug → quarantine → deletion clock; second quarantine in the same subsystem → product-race smell, look before deleting. + +## Examples + +Before/after CI topology: + +``` +Before: PR → lint/typecheck/build + 4 test shards (4–10 min, flaky) + slow tier + inventory guard +After: PR → Lint, Typecheck, Build, Gate (boot smoke + ~4s curated suite) [blocking] + main push → full-suite.yml: shards + slow tier + guard [non-blocking] +``` + +Operational gotchas discovered while shipping this: + +- **A PR in `CONFLICTING` mergeable state runs zero GitHub Actions.** No `pull_request` workflows fire because GitHub cannot build the merge ref — it looks exactly like "CI never started" (no pending checks at all). Fix: merge the base branch; the resulting push triggers normally. +- **Deleting a workflow file crashes tests that `readFileSync` it.** The asserting test fails at `beforeAll`, taking its whole file with it. Rewrite the test in the same PR and convert the absence into an invariant: `expect(() => loadWorkflow("ci.yml")).toThrow()`. +- **Node does not fire `'exit'` on SIGTERM/SIGINT.** A smoke script registering only `process.on("exit", cleanup)` orphans its server child when the CI job is cancelled. Register explicit signal handlers that call cleanup and re-exit with 143/130. +- **A new "always run" mode must be excluded from cache fast paths.** Gate mode initially fell into `test-changed.mjs`'s cache-fresh short-circuit and silently no-opped; the fix treats gate mode as always having work. +- **Smoke-test shutdown verdicts need both halves:** SIGTERM actually delivered AND a clean exit (`code 0`/`SIGTERM`) — otherwise a server that crashes after the health check still prints PASS. + +## Related + +- `docs/testing.md` ("The merge gate", "Quarantine ledger and the deletion ratchet") and the `AGENTS.md` standing rule — the operative policy text +- `../ui-bugs/skill-autocomplete-highlight-reset-on-swr-revalidation.md` — the complementary triage path: when a flake is a real product race, fix the invariant instead of quarantining +- `../architecture-patterns/i18n-foundation-vite-ink-monorepo-code-split-catalogs.md` — references "CI shard 1/2" as PR-blocking surfaces; post-#1453 those run non-blocking in `full-suite.yml` +- Runfusion/Fusion#1453 (the change), follow-up gaps Runfusion/Fusion#1447–#1452 (untested gate-mode invariants, dist-cache composite action, workflow-loader dedup) +- Open flaky-test issues Runfusion/Fusion#1430, Runfusion/Fusion#1355 — first candidates for the quarantine ledger under the new policy