From edc83c49a82b1495dc4a089e97e8cd5e5c6fbd83 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Thu, 9 Jul 2026 10:45:52 -0400 Subject: [PATCH 1/2] fix: make `npm run ci` a true superset of GitHub CI (coverage gate) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) Claude-Session: https://claude.ai/code/session_01BrihGWcrM9JGRyu41nzZYw --- AGENTS.md | 8 ++++---- package.json | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 3e9aea493..9a1a99f49 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -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`. diff --git a/package.json b/package.json index 3adbbc62f..44e757046 100644 --- a/package.json +++ b/package.json @@ -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", From 812bdb0cc0781def503b9d425e79912cb5e91b89 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Thu, 9 Jul 2026 12:35:02 -0400 Subject: [PATCH 2/2] chore: remove orphaned ci:integration script The root ci:integration alias became dead after ci switched to run coverage (which runs web's unit+integration projects under v8). Its only occurrence was its own definition. clients/web test:integration remains available for direct manual runs. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01BrihGWcrM9JGRyu41nzZYw --- package.json | 1 - 1 file changed, 1 deletion(-) diff --git a/package.json b/package.json index 00f6f7fa9..3d0696497 100644 --- a/package.json +++ b/package.json @@ -38,7 +38,6 @@ "build:web": "cd clients/web && npm run build", "build:launcher": "cd clients/launcher && npm run build", "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", "validate:cli": "cd clients/cli && npm run validate",