diff --git a/.claude/skills/pre-push-gate/SKILL.md b/.claude/skills/pre-push-gate/SKILL.md index 0d8b448e98..8459645a0e 100644 --- a/.claude/skills/pre-push-gate/SKILL.md +++ b/.claude/skills/pre-push-gate/SKILL.md @@ -31,7 +31,7 @@ prints each stage as it starts, so the running command is the other reliable answer. It runs **every check** GitHub CI runs (which additionally runs `npm install`, -and runs `coverage` as a parallel job), plus two local-only steps. So the +and runs `coverage` as a parallel job), plus one local-only step. So the direction that matters holds: **passing `local:gate` locally means every check CI applies has already passed on your machine** — the strongest predictor of a green CI there is here, though not a proof (a different OS, and the bare test @@ -189,16 +189,14 @@ own. for a measurement that needs contention; it does not get a result sooner, because the queued run finishes before an overlapped one would. -## Local-only steps +## Local-only step -Two stages have no GitHub CI counterpart, each deliberately: +One stage has no GitHub CI counterpart, deliberately: - **`smoke:web:firefox`** — the three browser-driven web smokes again under Firefox. Trialled as a CI job and removed (#2086): across a dozen runs it never disagreed with Chromium, and `playwright install --with-deps` carries a real flake surface. Kept in front of a human about to push instead. -- **`smoke:tui`** — needs a real TTY. It _is_ invoked in CI via `npm run smoke` - and self-skips there on `process.env.CI`, so it needs no guarding. A guard (`scripts/lib/workflow-gate.mjs`, run by `npm run test:scripts`) fails the suite if a workflow invokes a `local:*` script, a non-Chromium engine pass, diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index a749c9bf57..917c8674b9 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -139,8 +139,9 @@ jobs: # boots the prod web bundle in headless chromium (#1615); smoke:web:app # goes further and drives connect → open app → widget ready against a # composable MCP App server (#1859). Both reuse the chromium installed - # above. smoke:tui self-skips here — the Ink TUI needs a real TTY (raw - # mode) that headless CI lacks, so its boot/render check is local-only. + # above. smoke:tui runs for real here too (#2408): it gives the Ink TUI + # a pseudoterminal through util-linux script(1), which ubuntu-latest + # ships, and pins CI=false for the child so Ink renders interactively. run: npm run smoke - name: Run Storybook play-function tests @@ -260,7 +261,8 @@ jobs: - name: Verify the publishable tarball end to end # Builds, `npm pack`s, installs the tarball into a clean consumer, and # drives the installed bin. Needs registry access to pull the tarball's - # runtime deps — available here. `smoke:tui` inside it self-skips on CI. + # runtime deps — available here. Its TUI check is `--tui --help` only; + # the real TUI boot is `smoke:tui`, which the `build` job runs. run: npm run pack:verify - name: Publish to npm (single package, with provenance) diff --git a/AGENTS.md b/AGENTS.md index 85caa6f6ae..e351c4d012 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -512,12 +512,12 @@ free to run a full `local:gate`). Raising a budget nobody chose hides no race. ## Mandatory pre-push gate - **ALWAYS run `npm run format` before committing.** The **root** `format` auto-fixes `core/`, the root `scripts/` tooling, the root shared surface, and every client's scope in one shot. `validate` runs the non-fixing `format:check` and will fail in CI on any unformatted file, so run the auto-fixer first rather than letting `format:check` catch it. -- **`npm run local:gate` is the mandatory pre-push command.** It runs **every check** `.github/workflows/main.yml` runs, plus two local-only steps, so a green run here is the strongest predictor of a green CI this repo has: every check CI applies has already passed on your machine. It is not a proof — CI runs on a different OS and, since #2341, runs each client's suite bare where the gate runs it only instrumented — so the one residual is a test that passes *only* when slowed down, which is a race (#1596) to fix, never headroom to keep. Expect several minutes. +- **`npm run local:gate` is the mandatory pre-push command.** It runs **every check** `.github/workflows/main.yml` runs, plus one local-only step, so a green run here is the strongest predictor of a green CI this repo has: every check CI applies has already passed on your machine. It is not a proof — CI runs on a different OS and, since #2341, runs each client's suite bare where the gate runs it only instrumented — so the one residual is a test that passes *only* when slowed down, which is a race (#1596) to fix, never headroom to keep. Expect several minutes. - **The gate runs each client's test suite once, instrumented; CI runs it twice (#2341).** CI's `build` job runs the bare `test` inside `validate` and its parallel `coverage` job runs `test:coverage`, which costs CI no wall clock. Serially in one process the bare pass was ~80s of a ~370s gate, re-running exactly the files the coverage pass runs a few minutes later — so the gate calls **`local:validate`**, which is `validate` minus each client's `test` leg (every client's `validate` is `check && test`, and `local:validate` runs the `check` half). "Every check" is therefore a claim about checks, not invocations, and it holds because `@vitest/coverage-v8` collects coverage from V8's own profiler (`Profiler.takePreciseCoverage`) and rewrites no source: a test file sees identical code either way, and the instrumented run is only *slower*, which makes it the stricter of the two for the failure class this repo actually sees (a correct test cut off under load). A test that passed only *because* it ran slower would be a race — #1596's class, a defect wherever it surfaces — and CI's bare pass still runs it. **`npm run validate` is unchanged**, because CI runs it directly and it is the inner-loop check; `verify:*` guards that ask "is this reachable from `validate`" are unaffected. `local:validate` lives in the `local:` namespace so the workflow guard keeps it out of CI by construction. - **`npm run validate` is the fast inner-loop check and is NOT an acceptable substitute.** It runs `test`, not `test:coverage`, so it does **zero** coverage gating, no smokes, and no Storybook tests. Skipping the gate is how a push passes every fast local check and still fails CI. - **Concurrent gates queue; they do not overlap.** `local:gate` takes a machine-wide lease (`scripts/gate-lease.mjs`, #2339) so a gate started in a second worktree waits for the first rather than running alongside it, and queued gates start in arrival order (#2473). Overlap is not merely slow — the web smokes bind fixed ports, so two gates reaching the same smoke together go red on a diff that cannot have caused it (measured: one of two concurrent gates failed at 279s on port 6298 while a quiet gate passed in 257s). The wait names the holder and its worktree; a holder that dies releases within 30s (unless its lock directory cannot be removed, in which case the wait runs to its 45-minute cap and names the path); `INSPECTOR_SKIP_GATE_LEASE=1` bypasses it, which is for a measurement that _needs_ contention, never for getting a result sooner — the queued run finishes sooner anyway. - There is deliberately **no `npm run ci`** — that name collided with the `npm ci` built-in, which clean-installs from the lockfile and does not run this script. -- What each stage covers, and why two of them are local-only, is [`docs/quality-gate.md`](./docs/quality-gate.md); how to diagnose a failing stage is the `pre-push-gate` skill. +- What each stage covers, and why one of them is local-only, is [`docs/quality-gate.md`](./docs/quality-gate.md); how to diagnose a failing stage is the `pre-push-gate` skill. ## Waiting on long-running work diff --git a/README.md b/README.md index 06ae527f92..286f8acf89 100644 --- a/README.md +++ b/README.md @@ -94,10 +94,10 @@ Each client self-validates from its own folder; the root scripts chain them. The ```bash npm run validate # fast inner loop: format:check + lint + typecheck + build + unit tests npm run coverage # the per-file ≥90% gate (lines/statements/functions/branches) -npm run local:gate # MANDATORY before pushing — every GitHub CI check, plus two local-only ones +npm run local:gate # MANDATORY before pushing — every GitHub CI check, plus one local-only one ``` -`npm run local:gate` chains every check below, plus the smokes and the Storybook tests. [Testing and the quality gate](./docs/quality-gate.md) owns the stage list and says what each one covers and why two are local-only; [`AGENTS.md`](./AGENTS.md) holds the testing rules themselves. +`npm run local:gate` chains every check below, plus the smokes and the Storybook tests. [Testing and the quality gate](./docs/quality-gate.md) owns the stage list and says what each one covers and why one is local-only; [`AGENTS.md`](./AGENTS.md) holds the testing rules themselves. ## Contributing — `AGENTS.md`, `CLAUDE.md`, and the skills diff --git a/clients/launcher/README.md b/clients/launcher/README.md index 9b4eec1618..dade4b0d18 100644 --- a/clients/launcher/README.md +++ b/clients/launcher/README.md @@ -100,9 +100,9 @@ through the built launcher artifact (beyond the `--help` checks in The second half is the assertion (#2147). Spawned with its stdin on `/dev/null`, Ink cannot enter raw mode for `useInput`, so the TUI painted one frame and exited 1 about 40ms later — and this smoke, which settled OK on the - first frame, won that race and reported success on every machine. It is - local-only (self-skips under `CI`), which is precisely where a false green - goes unnoticed. + first frame, won that race and reported success on every machine. It runs in + GitHub CI as well as the local gate (#2408); it pins `CI=false` for the + child, since Ink suppresses interactive frames when it detects CI. Both rebuild `test-servers/build` on **every run** — once per process, whether or not it already exists (#2111). Presence is not freshness: a smoke driving a diff --git a/docs/quality-gate.md b/docs/quality-gate.md index 4590e089bb..f609814898 100644 --- a/docs/quality-gate.md +++ b/docs/quality-gate.md @@ -12,18 +12,17 @@ Each client self-validates from its own folder; the root scripts chain them. The | Tier | How it runs | What it covers | | --- | --- | --- | | **GitHub CI** (`.github/workflows/main.yml`) | Automatically, on every push | `npm install`, then `validate`, `verify:skills:cli`, `verify:build-gate`, `verify:bundle-externals`, `smoke` (which includes `smoke:web:chromium`), `test:storybook` — plus `coverage` in a parallel job ([#2159](https://github.com/modelcontextprotocol/inspector/issues/2159)) | -| **The local gate** (`npm run local:gate`) | By hand, before you push | Every check above (the install is yours to run; `local:validate` stands in for `validate`, see below), **plus** the Firefox engine pass (`smoke:web:firefox`), and `smoke:tui` for real rather than self-skipped | +| **The local gate** (`npm run local:gate`) | By hand, before you push | Every check above (the install is yours to run; `local:validate` stands in for `validate`, see below), **plus** the Firefox engine pass (`smoke:web:firefox`) | -The local gate runs **every check** CI runs, and is not a mirror. Two of its steps have no GitHub CI counterpart, each for its own reason: +The local gate runs **every check** CI runs, and is not a mirror. One of its steps has no GitHub CI counterpart: | Local-only step | Why it is local-only | | --- | --- | | `smoke:web:firefox` | Trialled as a CI job and removed ([#2086](https://github.com/modelcontextprotocol/inspector/issues/2086)): across a dozen runs it never once disagreed with Chromium, and `playwright install --with-deps` carries a real flake surface. Kept in front of a human about to push instead. `smoke:web:webkit` is worse still — it fails two of the three smokes for reasons nobody has identified — so it is in neither tier. See [Supported browsers](#supported-browsers). | -| `smoke:tui` | The Ink TUI needs a real TTY. It _is_ invoked in CI via `npm run smoke` and self-skips there on `process.env.CI`, so it needs no guarding — it handles itself. | And one CI *invocation* the gate deliberately does not repeat: CI runs each client's unit suite **twice** — bare inside `validate` in the `build` job, instrumented inside `coverage` in the parallel job — at no wall-clock cost, because the two jobs run on separate runners. Serially in one process the **bare** pass was ~81s of a ~358s quiet gate — the baseline measured on [#2338](https://github.com/modelcontextprotocol/inspector/issues/2338) — running exactly the test files the coverage pass runs again a few minutes later ([#2341](https://github.com/modelcontextprotocol/inspector/issues/2341)). So the gate's first stage is **`local:validate`**: the same guards and `validate:core`, then each client's `check` — `format:check` + `lint` + `typecheck`, plus `build` for web, tui and launcher, which name one explicitly; cli's `validate` built only through `test`'s `pretest` hook, so dropping the `test` leg drops that build too, and `coverage:cli` builds the binary once, later — instead of its `validate` (`check` + `test`). The suites still run once, under `coverage`. That subsumes the bare pass because `@vitest/coverage-v8` reads V8's own precise-coverage profiler and rewrites nothing — the code under test is byte-identical, and the instrumented run is only slower, which is the stricter direction for the failures this repo sees. `npm run validate` and both CI jobs are unchanged. With the duplicate gone a quiet gate is **~260s** (268s in #2341's own before/after, 257s where [#2339](https://github.com/modelcontextprotocol/inspector/issues/2339) measured it) — a reader who remembers six minutes is remembering the gate before #2341. -So the direction that matters holds: **passing `npm run local:gate` means every check CI applies has already passed on your machine** — the strongest predictor of a green CI this repo has, though not a proof: CI runs on a different OS, and the bare pass above is the one invocation it has that the gate does not, so a test that passes *only* when instrumentation slows it down would surface there first. That test is a race ([#1596](https://github.com/modelcontextprotocol/inspector/issues/1596)) to fix, not a reason to put the second pass back. The reverse direction does not hold at all: CI green says nothing about the Firefox pass or the real `smoke:tui`. +So the direction that matters holds: **passing `npm run local:gate` means every check CI applies has already passed on your machine** — the strongest predictor of a green CI this repo has, though not a proof: CI runs on a different OS, and the bare pass above is the one invocation it has that the gate does not, so a test that passes *only* when instrumentation slows it down would surface there first. That test is a race ([#1596](https://github.com/modelcontextprotocol/inspector/issues/1596)) to fix, not a reason to put the second pass back. The reverse direction does not hold at all: CI green says nothing about the Firefox pass. `smoke:tui` used to be the second thing CI green said nothing about — it self-skipped under `process.env.CI` — and has run for real in CI since [#2408](https://github.com/modelcontextprotocol/inspector/issues/2408): `ubuntu-latest` ships util-linux `script(1)` for the pseudoterminal, and the smoke pins `CI=false` for the child, because Ink (through `is-in-ci`) otherwise suppresses every interactive frame and the TUI paints nothing to assert on. **Every CI job carries `timeout-minutes`** ([#2333](https://github.com/modelcontextprotocol/inspector/issues/2333), PR [#2349](https://github.com/modelcontextprotocol/inspector/pull/2349)): `build` 25, `coverage` 20, `publish` 10, `publish-github-container-registry` 35. Each is roughly **twice the slowest observed run, rounded up to the next five minutes** — `build` was read from 7.6–11.8 min and `coverage` from 5.9–8.5 min across the 40 push runs before it landed, the two publish jobs from 2.3–2.8 and 14.5–15.4 min across the last three releases — and the range each was read from is stated beside the job in the workflow, so the next raise carries a new range rather than a guess. It is a **hung-job guard, not a flake remedy**: GitHub runners are not the contended machine the local gate runs on, nothing measured implicates them, and no value there is sized for load. A release-run `build` before the coverage split ([#2159](https://github.com/modelcontextprotocol/inspector/issues/2159)) reached 15.6 min with the coverage gate still inside it; that shape no longer exists, so it is not in the range. @@ -45,7 +44,7 @@ That is the readable half, and prose rots. The enforced half is `scripts/lib/wor | `npm run verify:typecheck-coverage` | The typecheck-coverage analog of the above (#1791): for each Node client (auto-discovered from disk — enrolled via its `typecheck` script's projects, or for a `tsc -b` client like `clients/web` via its `tsconfig.json` `references`) it runs those projects with `tsc --listFilesOnly`, unions them, and **fails** listing any tracked `.ts`/`.tsx`/`.mts`/`.cts` under the client that lands in no project (so a new top-level config/helper can't silently go untypechecked). It also requires, deny-by-default, the first-party TS no client owns (`test-servers/src`, the root `vitest.shared.mts`, all of `core/`, and any new top-level location) to land in some client project's tsc pass — so a `core` `*.tsx` web's projects don't reach is caught too. Also asserts the gate is wired (each client's typecheck pass — its `typecheck` script, or web's `tsc -b` — is reachable from its `validate`, and the root chain runs each client's `validate`). Runs in `validate`. | | `npm run verify:dep-lockstep` | Guards the "one version per install-crossing dependency" invariant (#1896). v2 is not a workspace, so a client's test project compiles the shared first-party TypeScript — `core/`, `test-servers/src`, and the root-owned `vitest.shared.mts`, all of which resolve their dependencies from the **root** install — alongside the client's own sources, putting the same package in one `tsc` program twice. At the same version that's harmless; skewed, TypeScript must relate two structurally-distinct copies of every type, which for a recursive-generic surface is exponential (zod `4.3.6` vs `4.4.3` exhausted the 4GB tsc heap in `clients/web`). Derives its candidate set from **what actually enters each program** (#1965) — every client tsconfig project listed with `tsc --listFilesOnly` via the shared `scripts/lib/tsc-program.mjs`, each resolved `node_modules` file mapped to its owning install, keeping the packages that reach one program from two installs (a package whose declarations arrive only through another package's `.d.ts`, as `@modelcontextprotocol/sdk`'s do, is invisible to a scan of first-party imports). Prices each copy from the lockfile entry for the exact install path the program resolved, compares only the installs that met in one program, and **fails deny-by-default** on any disagreement not in the annotated `TOLERATED_SKEW` allowlist — empty today — with an allowlisted package tolerated only *within a major version*. A **second tier** (#2226) runs alongside it, asking the weaker but broader question the `AGENTS.md` rule actually states: does a package this repo *declares* anywhere resolve to two versions across our installs at all? Its candidate set is every name in any install's `dependencies`/`devDependencies`/`optionalDependencies` — unioned across the root and all four clients, so a copy declared by only one of them still counts — that **more than one install holds a top-level copy of** (17 packages today). Nested copies are excluded: one exists because some dependency asked for a different version, so it is that dependency's range to govern, not ours. Neither tier subsumes the other — the program tier sees a copy no manifest names (`@modelcontextprotocol/sdk`, arriving through another package's `.d.ts`), while the declared tier sees a **transitive** copy no program loads (cli's `@types/node`, hoisted via `@types/express` — the case that motivated it), two **clients** disagreeing with no root copy involved (`@types/react`, web against tui), and the peer shadows `eslint`/`typescript`/`vitest` that never enter a program. Same deny-by-default and same within-a-major rule, against its own `TOLERATED_DECLARED_SKEW` — also empty. Two limits: it reads **lockfiles**, so an uncommitted hand-installed copy is invisible, and it compares only declared names, so a purely transitive package no manifest names stays the first tier's business. Runs in `validate`. | `npm run verify:test-timeouts` | Guards the wall-clock budgets the test gates run under (#2323). The class it encodes against is a budget **nobody chose**: three of the six Vitest projects ran on Vitest's own `testTimeout: 5000` and five on its `hookTimeout`/`teardownTimeout: 10000`, sized for an idle machine rather than the one this team works on — three or four concurrent agent sessions in separate worktrees, each free to run a full `local:gate`, on eight logical cores. A correct, deterministic test cut off by such a budget fails a gate its diff did not break, which is the same "channel nobody trusts" failure [Lint has no warning tier](../AGENTS.md#lint-has-no-warning-tier) describes from the other direction; #2292, #1942 and #1742 were each an instance, found one site at a time. Deliberately **not** cited: #2278 (a missing condition wait around a geometry read) and #2250 (a real race in a test's own timing) were fixed by making the test wait for the right thing, and presenting a race fix as evidence for a larger ceiling would argue against #1596. It asks **Vitest itself** to resolve each of the four configs and reads the number a test actually gets, rather than checking that a key is absent from a config block — which would pass just as happily on a config that had stopped being loaded. A **seventh project** with no row, or a config file it does not discover (it knows all twelve filenames Vitest accepts, not just the two this repo uses), is an error rather than a silent skip. It also checks that every project loads `vitest.setup.shared.mts`. ⚠️ **Everything it reads comes from a resolved project, never from source text** — the two rules no config can report are asserted at runtime instead: `retry` by `vitest.setup.shared.mts` (which reads the value Vitest resolved, so a per-test option, a `describe` option, a project setting and a `--retry` flag are one check) and Testing Library's `asyncUtilTimeout` by `clients/web/src/test/asyncUtilTimeout.test.ts` (which reads what the project's own `waitFor`s use). Both began as source scanning in #2334 and the review found a new valid spelling missed in five consecutive rounds, so the boundary is deliberate: if a rule cannot be answered by asking the tool, assert it at runtime rather than reading the source for it. Observed **red** against the pre-#2323 config before it was trusted. Runs in `validate`; its decision logic has a sibling `verify-test-timeouts.test.mjs` under `test:scripts`. -| `npm run local:gate` | **Mandatory pre-push command.** `local:validate` → `verify:skills:cli` → `coverage` → `verify:build-gate` → `verify:bundle-externals` → `smoke` → `smoke:web:firefox` → `local:storybook`. Every GitHub CI check plus two local-only ones — see [Two tiers](#two-tiers-github-ci-and-the-local-gate). Named `local:` rather than `ci` on purpose (#2146); there is no `npm run ci` alias. **Runs under a machine-wide lease** (`scripts/gate-lease.mjs`, [#2339](https://github.com/modelcontextprotocol/inspector/issues/2339)): a second `local:gate` started in another worktree waits for the first to finish instead of running alongside it, and queued gates start in the order they arrived ([#2473](https://github.com/modelcontextprotocol/inspector/issues/2473)). Measured on `aa56551b`, a quiet gate took 257s; two started together had one **fail** at 279s on a smoke port collision (`smoke:web:chromium` binds 6298, and so did the other gate's) and the survivor take 338s — so overlapping gates are red by construction, not merely slow, and back to back both finish green in ~2x257s. The wait prints the holder's pid and worktree; a holder that dies without releasing is taken over after 30s (`proper-lockfile` stale detection) — unless its lock directory cannot be removed, in which case the wait runs to its 45-minute cap and names the path; `INSPECTOR_SKIP_GATE_LEASE=1` bypasses it. The stages themselves are `local:gate:stages`, which the wrapper runs verbatim. Why it is a queue rather than a load wait or a worker cap, and what it does and does not change about capacity, is [Multi-agent testing](#multi-agent-testing-the-gate-lease). | +| `npm run local:gate` | **Mandatory pre-push command.** `local:validate` → `verify:skills:cli` → `coverage` → `verify:build-gate` → `verify:bundle-externals` → `smoke` → `smoke:web:firefox` → `local:storybook`. Every GitHub CI check plus one local-only one — see [Two tiers](#two-tiers-github-ci-and-the-local-gate). Named `local:` rather than `ci` on purpose (#2146); there is no `npm run ci` alias. **Runs under a machine-wide lease** (`scripts/gate-lease.mjs`, [#2339](https://github.com/modelcontextprotocol/inspector/issues/2339)): a second `local:gate` started in another worktree waits for the first to finish instead of running alongside it, and queued gates start in the order they arrived ([#2473](https://github.com/modelcontextprotocol/inspector/issues/2473)). Measured on `aa56551b`, a quiet gate took 257s; two started together had one **fail** at 279s on a smoke port collision (`smoke:web:chromium` binds 6298, and so did the other gate's) and the survivor take 338s — so overlapping gates are red by construction, not merely slow, and back to back both finish green in ~2x257s. The wait prints the holder's pid and worktree; a holder that dies without releasing is taken over after 30s (`proper-lockfile` stale detection) — unless its lock directory cannot be removed, in which case the wait runs to its 45-minute cap and names the path; `INSPECTOR_SKIP_GATE_LEASE=1` bypasses it. The stages themselves are `local:gate:stages`, which the wrapper runs verbatim. Why it is a queue rather than a load wait or a worker cap, and what it does and does not change about capacity, is [Multi-agent testing](#multi-agent-testing-the-gate-lease). | | `npm run local:validate` | The gate's first stage: `validate` with each client's `test` leg removed (#2341) — the same `validate:guards` and `validate:core`, then every client's `check` — `format:check` + `lint` + `typecheck`, plus `build` for web, tui and launcher, which name one explicitly (cli's only validate-time build was `test`'s `pretest` hook, so it goes with the `test` leg and `coverage:cli` builds the binary once, later) — each client's `validate` being `check && test`. The suites run once, under `coverage`, instead of bare here and instrumented there. In the `local:` namespace so the workflow guard keeps it out of CI, where the bare pass is free. | | `npm run local:storybook` | The gate's last stage: from `clients/web`, `npx playwright install chromium` and then `test:storybook` — the Storybook play functions, run headless by Vitest's browser project (CI runs `test:storybook` directly after its own Playwright install step, which is why this wrapper is in the `local:` namespace). Since [#2340](https://github.com/modelcontextprotocol/inspector/issues/2340) (PR [#2342](https://github.com/modelcontextprotocol/inspector/pull/2342)) the project's `optimizeDeps` carries `force: true` (`getStorybookOptimizeDeps` in `clients/web/server/vite-base-config.ts`), so Vite discards its dep pre-bundle cache and re-scans every story entry before the browser opens — the cold path CI takes on every PR. Without it, Vite keys that cache on the lockfile and the config, never on what the story graph imports, so after a story gains an import of an already-installed package the "valid" cache re-runs the optimizer mid-run and sends a `full-reload` the Vitest tester iframes do not act on; every story an already-loaded iframe renders from then on fails with `Failed to fetch dynamically imported module`, and the next run is green because the cache has caught up. Measured: a stale cache went 3/3 red → 3/3 green with `force` → 3/3 red reverted, load refuted as a cause (0 of 6 under load 8–12), cost ~0.8s per run against a ~25s stage. | | `npm run pack:verify` | Publish smoke — see [Publishing](./publishing.md). | diff --git a/scripts/lib/workflow-gate.mjs b/scripts/lib/workflow-gate.mjs index 9a531b11bf..570a1fbdde 100644 --- a/scripts/lib/workflow-gate.mjs +++ b/scripts/lib/workflow-gate.mjs @@ -8,8 +8,9 @@ * `smoke:launcher`, `smoke:cli`, `smoke:tui`, `smoke:web` and * `smoke:web:chromium` — all of which BELONG there. * - **`npm run local:gate`**, the pre-push gate, runs every check CI runs and - * adds the **Firefox** engine pass, and `smoke:tui` really runs there rather - * than self-skipping. WebKit is on demand and belongs to neither tier. It + * adds the **Firefox** engine pass. (`smoke:tui` used to be a second + * local-only step by self-skipping under CI; it runs in both since #2408.) + * WebKit is on demand and belongs to neither tier. It * runs each client's unit suite once (instrumented, under `coverage`) via * `local:validate`, where CI's two parallel jobs run it twice (#2341). * @@ -54,8 +55,8 @@ * out a forbidden one. * * WHAT IT MUST NOT FORBID: `npm run smoke`, `smoke:web:chromium`, `smoke:tui`. - * Those belong in CI and are there today — `smoke:tui` self-skips under - * `process.env.CI` on its own, so it needs no guarding. + * Those belong in CI and are there today — `smoke:tui` included, which runs + * for real in CI since #2408 rather than self-skipping. * * ONLY EXECUTABLE POSITIONS ARE SCANNED, found by parsing the file rather than * by matching lines: `run:` scalars (inline and block), a custom `shell:` diff --git a/scripts/lib/workflow-gate.test.mjs b/scripts/lib/workflow-gate.test.mjs index 5ddfc395ce..ae5911fc7a 100644 --- a/scripts/lib/workflow-gate.test.mjs +++ b/scripts/lib/workflow-gate.test.mjs @@ -484,7 +484,7 @@ describe("findWorkflowViolations", () => { rules: [], }, { - name: "allows smoke:tui, which self-skips under CI on its own", + name: "allows smoke:tui, which runs for real in CI (#2408)", text: workflow(" - run: npm run smoke:tui"), rules: [], }, diff --git a/scripts/smoke-tui.mjs b/scripts/smoke-tui.mjs index 23fe2f96fc..7f505ce996 100644 --- a/scripts/smoke-tui.mjs +++ b/scripts/smoke-tui.mjs @@ -74,17 +74,13 @@ function skip(message) { process.exit(0); } -// Kept out of GitHub CI by decision, not by capability. The PTY above removes -// the technical blocker this skip used to cite (a headless runner has no TTY), -// but whether this smoke joins CI is a separate call for the maintainers, and -// #2146 is deliberately keeping the other local-only smokes out. Making it -// *valid* stands on its own: it is a local-only gate, which is exactly where a -// false green is least likely to be caught by anything else. -if (process.env.CI) { - skip( - "local-only by decision (see the header); the TUI is built and unit-tested in CI", - ); -} +// Runs in GitHub CI too (#2408). It used to skip on `process.env.CI`, first +// because a headless runner has no TTY and then, once the PTY above removed +// that blocker, by decision — which left the TUI the one shipped surface with +// no end-to-end CI signal. There is deliberately no CI branch here any more: +// `ubuntu-latest` ships util-linux `script(1)`, so CI takes exactly the path a +// Linux developer's gate takes, and a runner without one skips below for the +// same stated reason a local machine would. if (!existsSync(launcher)) { fail(`launcher build not found at ${launcher} — run \`npm run build\` first`); @@ -154,8 +150,18 @@ try { // so an ambient non-loopback value can't crash the TUI before render via // the loopback callback guard — same class smoke-cli.mjs's // SMOKE_BASE_ENV neutralizes. + // + // Pin CI and CONTINUOUS_INTEGRATION to "false" (#2408). Ink reads them + // through `is-in-ci` and, when either is set to anything else, suppresses + // every interactive frame and writes only the last one on unmount — so + // under GitHub Actions' CI=true the TUI enters the alt screen, paints + // nothing, and the marker never arrives. "false" rather than deleting the + // keys because `is-in-ci` treats exactly "0"/"false" as not-CI, which + // holds whatever else the runner exports. env: { ...process.env, + CI: "false", + CONTINUOUS_INTEGRATION: "false", MCP_OAUTH_CALLBACK_URL: "", HOME: work, USERPROFILE: work,