fix: make npm run ci a true superset of GitHub CI (coverage gate)

The local `npm run ci` was not at parity with `.github/workflows/main.yml`.
CI runs `validate → coverage → smoke → test:storybook`, but the root `ci`
script ran `validate → ci:integration → smoke → ci:storybook`, omitting
`npm run coverage` — the per-file ≥90 (lines/statements/functions/branches)
gate that CI enforces. So a push could pass every documented local pre-push
command and still fail CI on the coverage gate (this exact gap broke PR #1601
on a HistoryEntry.tsx function-coverage regression).

Two changes:

1. Root `package.json` `ci` script now chains `npm run coverage` in place of
   `ci:integration`. Because web's `coverage:web` (`test:coverage`) already
   runs the integration project under instrumentation, `coverage` supersedes
   the old integration-only step and makes `ci` a genuine superset of GitHub
   CI. The `ci:integration` / `ci:storybook` script definitions are left in
   place for any other references.

2. AGENTS.md: rename the pre-push section to "Mandatory pre-push gate", make
   running the coverage gate a hard non-optional step (remove the "or at
   minimum `npm run validate`" loophole and the soft "optionally also run
   coverage" phrasing), and correct the parity claim so `npm run ci` is
   described as `validate → coverage → smoke → storybook`.

Closes #1619

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BrihGWcrM9JGRyu41nzZYw
This commit is contained in:
cliffhall 2026-07-09 10:45:52 -04:00
commit edc83c49a8
2 changed files with 5 additions and 5 deletions

View file

@ -191,12 +191,12 @@ gh project item-edit --project-id PVT_kwDOCt2Azc4BJVxt --id "$ITEM_ID" --field-i
- you are free to implement suggestions in a different way or to ignore if there is a good reason
- after making the changes, respond to each review comment with what was done (or why it was ignored)
### Lint-fixed, Formatted code
### Mandatory pre-push gate
- ALWAYS do `npm run format` before committing — it auto-fixes any Prettier issues. `validate` runs `format:check` (the non-fixing variant) and will fail in CI on any unformatted file, so always run the auto-fixer first rather than letting `format:check` catch it.
- **`npm run ci` before pushing** — runs the same steps as `.github/workflows/main.yml` (minus `npm install`): `validate` → web `test:integration` → `smoke` → Storybook play-function tests (installs Playwright chromium if needed). Use this when you want CI parity; expect several minutes. **`npm run validate`** remains the fast loop during development (unit tests only — no web integration, smoke, or Storybook).
- ALWAYS do `npm run format` before committing, then **`npm run ci`** (or at minimum `npm run validate`) before pushing. From the repo root, `validate` chains the four per-client validations (`validate:web` → `validate:cli` → `validate:tui` → `validate:launcher`); each delegates to that client's own `npm run validate` = `format:check` + `lint` + `build` + `test` in its own folder (no coverage — fast). Every client is self-validating and the top level just chains them, building each client's bundle along the way (no cross-client build dependencies).
- **`npm run ci` is the mandatory pre-push command** — it mirrors `.github/workflows/main.yml` (minus `npm install`): `validate` → `coverage` → `smoke` → Storybook play-function tests (installs Playwright chromium if needed). It now runs **`npm run coverage`**, the per-file ≥90 gate (lines/statements/functions/branches) that CI enforces — so `npm run ci` is a true superset of GitHub CI, and passing it locally means CI's gates will pass. Expect several minutes. **`npm run validate`** remains the fast inner-loop check during development (unit tests only — no coverage gate, no smoke, no Storybook), but it is **NOT** an acceptable substitute for `npm run ci` before pushing: `validate` runs `test`, not `test:coverage`, so it does **zero** coverage gating. Skipping the gate is how a push passes every fast local check and still fails CI (this exact gap broke PR #1601 on a function-coverage regression).
- ALWAYS do `npm run format` before committing, then **`npm run ci`** before pushing. From the repo root, `validate` chains the four per-client validations (`validate:web` → `validate:cli` → `validate:tui` → `validate:launcher`); each delegates to that client's own `npm run validate` = `format:check` + `lint` + `build` + `test` in its own folder (no coverage — fast). Every client is self-validating and the top level just chains them, building each client's bundle along the way (no cross-client build dependencies).
- The one CLI nuance: `clients/cli`'s out-of-process `e2e.test.ts` spawns the built binary, so its `test` **builds first** via `pretest` (`test-servers:build && build`). To avoid building it twice, `clients/cli`'s `validate` folds that in — it is `format:check && lint && test` with **no** separate `build` step (the other clients, whose tests don't spawn their bundle, keep an explicit `build`). `validate:web`/`validate:tui`/`validate:launcher` are the uniform `format:check && lint && build && test`.
- Optionally also run **`npm run coverage`** when you want the local-only per-file ≥90 gate — CI does **not** run `coverage`; it relies on `validate` + web `test:integration` instead.
- **`npm run coverage`** is the per-file ≥90 gate and is now part of `npm run ci` — never treat it as optional before a push. It supersedes the old standalone `test:integration` step: web's `test:coverage` runs the `unit` **and** `integration` projects under v8 instrumentation, so `coverage` both enforces the ≥90 gate and exercises the same web integration paths CI covers.
- **`smoke` is NOT part of `validate`** — it is included in `npm run ci`. It runs `smoke:launcher` (`--help` dispatch) plus the prod `smoke:cli` / `smoke:tui` / `smoke:web`, and contains **no build commands** — it assumes the cli/tui/launcher bundles already exist (a full `validate` builds them; `smoke:web` builds `clients/web/dist` on demand).
- `smoke:launcher` (`scripts/smoke-launcher.mjs`) runs the built launcher with `--help`, `--cli --help`, and `--tui --help`, asserting each exits 0 and prints that mode's usage banner (which also proves the launcher resolved and loaded the right client build). It's the cheap dispatch check before the heavier prod smokes below.
- `smoke:web` (`scripts/smoke-web.mjs`) starts `mcp-inspector --web` (prod, no `--dev`) against the built `clients/web/dist` and asserts `GET /` serves the SPA (HTTP 200) with the injected `__INSPECTOR_API_TOKEN__`. Prod `--web` serves from `clients/web/dist`, which ships in the published package but is absent in a fresh checkout — the runner builds it on demand (`build:client` = `vite build`) on first launch, or exits with an actionable error if that build can't run (see `clients/web/server/ensure-web-build.ts` and the launcher README). `--dev` runs Vite directly and never needs `dist`.

View file

@ -37,7 +37,7 @@
"build:tui": "cd clients/tui && npm run build",
"build:web": "cd clients/web && npm run build",
"build:launcher": "cd clients/launcher && npm run build",
"ci": "npm run validate && npm run ci:integration && npm run smoke && npm run ci:storybook",
"ci": "npm run validate && npm run coverage && npm run smoke && npm run ci:storybook",
"ci:integration": "cd clients/web && npm run test:integration",
"ci:storybook": "cd clients/web && npx playwright install chromium && npm run test:storybook",
"validate": "npm run validate:web && npm run validate:cli && npm run validate:tui && npm run validate:launcher",