* refactor(launcher): self-contained validate; split smokes into top-level step (#1504) Previously `validate:launcher` built cli/tui/web and ran the cross-client smokes, so the launcher's "validation" depended on every other client being built — and the launcher had no lint/format/coverage of its own. - clients/launcher: add a web-style `validate` (format:check && lint && build && test:coverage) with a Node-only eslint flat config, prettier, and a per-file coverage gate (parse-launcher-argv.ts covered; src/index.ts excluded as binary bootstrap). 100% on the gated file. - Root `validate:launcher` now just delegates: `cd clients/launcher && npm run validate` — no dependency on the other clients. - New top-level `smoke` step (smoke:launcher + smoke:cli + smoke:tui + smoke:web) with NO build commands; `validate` runs it last. Each client's own validation builds its bundle first, so nothing builds twice (cli/tui via test:coverage, launcher via its validate; smoke:web builds dist on demand). - clients/tui test:coverage now builds the TUI bundle (mirroring cli) so the build-free smoke has it available. - CI: replace the standalone cli/tui/launcher build step with a "Validate launcher" step. Docs updated (AGENTS.md, launcher README). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(cli,tui,root): every client self-validates; smoke separate from validate Extends the launcher self-validate work to the CLI and TUI, and makes the top-level `validate` nothing but the per-client validations — smokes are now a separate top-level target. - clients/cli, clients/tui: add a web-style `validate` with a Node flat eslint config + prettier. CLI lint/format pass as-is. TUI registers eslint-plugin-react-hooks for the classic rules only (rules-of-hooks + exhaustive-deps, which the source already references); the stricter react-hooks@7 rules are not enforced on the interim component surface (#1501). - Avoid double builds: - CLI's e2e spawns the built binary, so its `test:coverage` builds first; `validate` folds that in (`format:check && lint && test:coverage`, no separate `build`). - TUI tests don't spawn a bundle, so `validate` keeps an explicit `build` (for the smoke) and `test:coverage` is pure. - Root: `validate` = validate:web && validate:cli && validate:tui && validate:launcher (each delegates to the client's own validate). `smoke` is no longer part of `validate` — it's a separate top-level target run after a build/validate, with no build commands of its own. - CI: per-client "Validate X" steps + a single "Run cross-client smokes" (npm run smoke). Docs updated (AGENTS.md, cli/tui READMEs). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore(root): drop redundant test aggregator scripts Now that every client self-validates and the top-level `validate` chains the per-client validations, the root `test` / `test:web` / `test:cli` / `test:tui` / `test:coverage` aggregators are redundant — run `npm run validate` (all) or `cd clients/<name> && npm run validate` (one). `test:storybook` stays (it's the one check not part of validate). CI was already using each client's own scripts, so it's unaffected. Docs updated. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(web): silence happy-dom infra noise in unit runs Passing unit runs printed dozens of alarming-but-expected lines that made green runs look broken — all from happy-dom performing real I/O with no server behind it: - `<iframe src="/sandbox.html">` (MCP Apps sandbox) made happy-dom fetch and later unload that URL, emitting ~40 `DOMException [NetworkError/ AbortError]` + `AsyncTaskManager destroyed` lines. Disable child-frame navigation in the unit project (the tests only assert on the iframe element/attributes, not its document), removing the noise with no coverage loss. - App-level renders read `GET /api/config` via useSandboxUrl / useServerListWritable, 404-ing ~76x. Add a benign default `fetch` (empty 200) in the unit setup; tests that exercise fetch still override it. Negative tests that assert on errors already suppress their own console output (e.g. jsonUtils/schemaUtils); CLI, TUI, and the web integration suite were already clean. Unit suite is now noise-free; 2424 web tests pass and the coverage gate is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(scripts): make validate fast (test), split coverage into its own target Each client's `validate` now runs `test` instead of `test:coverage`, so the everyday pre-push check skips v8 instrumentation (and, for web, the slower integration project). The per-file coverage gate moves to a dedicated top-level `coverage` script: coverage = coverage:web && coverage:cli && coverage:tui && coverage:launcher coverage:<name> = cd clients/<name> && npm run test:coverage `validate` still builds each client's bundle (cli via its test `pretest`, the others via an explicit `build`), so `npm run validate && npm run smoke` works unchanged. CI now runs `validate` (fast checks + builds), then `coverage` (the gate), then `smoke` — collapsing the old per-client steps. Docs updated. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * ci: drop the coverage gate from CI; keep web integration as its own step CI no longer runs `npm run coverage` — the per-file coverage gate is now a local/manual check (`npm run coverage` before pushing). To avoid silently dropping the web integration suite (it only ran via test:coverage, and `validate` runs the unit project only), CI gains a standalone `clients/web` `test:integration` step that exercises the real stdio/HTTP/OAuth/storage paths without coverage instrumentation. CI is now: validate (fast) → web integration → smoke → storybook. Docs updated. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(smoke): give smoke:launcher its own script like the others smoke:launcher was the odd one out — an inline chain of three `node clients/launcher/build/index.js ... --help` commands while smoke:cli / smoke:tui / smoke:web each run a `scripts/smoke-*.mjs`. Extract it to `scripts/smoke-launcher.mjs`, which runs the same `--help` / `--cli --help` / `--tui --help` dispatch checks but now also asserts each prints that mode's usage banner (proving the launcher resolved the right client build) and verifies the required builds exist up front with a clear error. Prints a `smoke:launcher OK — ...` summary in the same style as the rest. Docs updated. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore: reorder package.json scripts for readability Move frequently-run scripts toward the top and rarely-run-by-hand ones (pretest hooks, lint/format internals, etc.) toward the end, consistently across the root and client manifests. Pure reordering — no script names or values changed, and no other manifest content is touched. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs: drop references to removed root inspector / test:storybook scripts The reordering dropped the root `inspector` and `test:storybook` aggregators (intentional — `inspector` is redundant with `web`, and storybook is run from clients/web in CI). Update AGENTS.md so it no longer points at `npm run inspector`, and run `test:storybook` from `clients/web/`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore: address PR #1505 review (format glob gap, fetch-baseline comment) - TUI format/format:check now also cover the client-root source files (index.ts, tui.tsx, dev.ts) that live outside src/ — `eslint .` already lints them, so they were linted but not format-checked. (cli/launcher have no source outside src/; their root .ts are single-quoted configs intentionally left out of prettier, matching web.) - Reword the setup.ts default-fetch comment: it overstated auto-restore (the unit project sets no restoreMocks/unstubGlobals), and now also flags that the permissive 200 default means a test asserting on a fetch FAILURE must install its own rejecting stub. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore: remove redundant root install:clients script `install:clients` ran the same `node scripts/install-clients.mjs` as `postinstall`, which fires automatically on `npm install` — so the manual alias was redundant. Drop it and the doc references (AGENTS.md, spec). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(ci): skip smoke:tui in CI (Ink needs a real TTY) This PR made CI run the full `npm run smoke`, which surfaced that smoke:tui fails in headless CI: the Ink TUI throws "Raw mode is not supported" on mount (no TTY) and exits before rendering its first frame. The old CI never ran smoke:tui, so this was latent. smoke-tui.mjs now self-skips when `process.env.CI` is set — the boot/ render check is inherently local-only (a TUI needs a terminal). The TUI is still built and unit-tested in CI; `npm run smoke` stays the single command everywhere (full locally, tui-skipped in CI). Verified `CI=true npm run smoke` is green (launcher/cli OK, tui SKIPPED, web OK). Docs + CI comment updated. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
388ea5a7bd
commit
5efe9ddc7b
1 changed files with 5290 additions and 250 deletions
19
AGENTS.md
19
AGENTS.md
|
|
@ -54,10 +54,10 @@ inspector/
|
|||
v2 is **not** an npm workspace — each client under `clients/*` keeps its own `package.json` and `node_modules` (see the rationale in [specification/v2_cli_tui_launcher.md](specification/v2_cli_tui_launcher.md)). A single `npm install` at the repo root is still all you need: the root `postinstall` (`scripts/install-clients.mjs`) cascades `npm install` into `clients/web`, `clients/cli`, `clients/tui`, and `clients/launcher`.
|
||||
|
||||
- **Fresh clone / first-time setup:** run `npm install` at the repo root.
|
||||
- **After a pull that changes a client's dependencies:** re-run `npm install` at the root (or `npm run install:clients`) to re-sync every client.
|
||||
- **After a pull that changes a client's dependencies:** re-run `npm install` at the root to re-sync every client (the `postinstall` cascade handles it).
|
||||
- The cascade is dev-only: it exits early when the package is installed under `node_modules`, and the published tarball ships only each client's `build/`, so end users are unaffected. Set `INSPECTOR_SKIP_CLIENT_INSTALL=1` to skip it.
|
||||
|
||||
After installing, `npm run build` builds all clients. The launcher scripts (`npm run inspector` / `web` / `web:dev`) run the built launcher, so build first; for day-to-day web iteration use `cd clients/web && npm run dev`.
|
||||
After installing, `npm run build` builds all clients. The launcher scripts (`npm run web` / `web:dev`) run the built launcher, so build first; for day-to-day web iteration use `cd clients/web && npm run dev`.
|
||||
|
||||
## Repository & Project Board
|
||||
|
||||
|
|
@ -120,8 +120,9 @@ All work should be driven by items on the project board.
|
|||
- Run unit tests with `npm run test` (or `npm run test:watch` during development) from `clients/web/`
|
||||
- Run CLI tests with `npm run test` from `clients/cli/` (builds test-servers + CLI bin first via `pretest`)
|
||||
- Run TUI tests with `npm run test` from `clients/tui/`
|
||||
- From repo root, `npm run test` runs web unit tests, then CLI and TUI tests
|
||||
- Run `npm run test:coverage` to verify the per-file gate: lines ≥ 90, statements ≥ 85, functions ≥ 80, branches ≥ 50 (CI enforces this gate). Branches is intentionally relaxed because Mantine portal/media-query branches are not exercisable under happy-dom; new business-logic branches should still be covered. From the repo root, `npm run test:coverage` runs the gate for web **and** CLI **and** TUI; each client also exposes its own `test:coverage`.
|
||||
- The repo root has no aggregate `test` script — each client self-validates, so run `npm run validate` from the root (all clients, fast) or `cd clients/<name> && npm run validate` (one client). Each client still exposes its own `test` / `test:coverage` for quick iteration.
|
||||
- **`validate` is fast: it runs `test`, not `test:coverage`.** The coverage gate (slower — adds v8 instrumentation, and for web the integration project) is a **separate** top-level `npm run coverage` (and per-client `coverage:web` / `coverage:cli` / `coverage:tui` / `coverage:launcher`, each delegating to that client's `test:coverage`). Run `npm run coverage` when you want the gate. **CI does NOT run `coverage`** — the gate is local-only; CI runs `validate` (fast) plus the web integration suite (`clients/web` `test:integration`, no coverage) so the integration paths are still exercised.
|
||||
- Each client's `test:coverage` enforces a per-file gate: lines ≥ 90, statements ≥ 85, functions ≥ 80, branches ≥ 50 (CI enforces this gate). Branches is intentionally relaxed because Mantine portal/media-query branches are not exercisable under happy-dom; new business-logic branches should still be covered.
|
||||
- The **same per-file gate** is enforced for the CLI and TUI (#1484), not just web:
|
||||
- **CLI** (`clients/cli`): tests run **in-process** by importing `runCli()` (see `__tests__/helpers/cli-runner.ts`) so `clients/cli/src` is measured under v8 instrumentation. A thin out-of-process layer (`__tests__/e2e.test.ts` + `scripts/smoke-cli.mjs`) still spawns the built binary for the shebang/`process.exit` paths; `src/index.ts` (binary bootstrap) is the only coverage exclusion. `commander` uses `.exitOverride()` so a parse error throws instead of tearing down the test worker.
|
||||
- **TUI** (`clients/tui`): the gate covers the **non-React logic** only — `logger.ts`, `components/tabsConfig.ts`, and `utils/*` (server resolution lives in `core/` and is measured by the web suite). The Ink components, `App.tsx`, and `hooks/` are an **interim exclusion** in `clients/tui/vitest.config.ts` pending the renderer-based follow-up (#1501). When adding new **non-React** logic under `clients/tui/src`, it falls under the gate automatically — add tests for it.
|
||||
|
|
@ -137,10 +138,14 @@ All work should be driven by items on the project board.
|
|||
|
||||
### Lint-fixed, Formatted code
|
||||
- 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.
|
||||
- ALWAYS do `npm run validate` before pushing any changes — from the repo root it mirrors what CI enforces across every client: web `format:check` + `lint` + `build` + `test:coverage` (`validate:web`), then the CLI and TUI suites **with their coverage gates** (`test:cli` / `test:tui`, which now run each client's `test:coverage`), then a build of CLI + TUI + launcher, the launcher `--help` smoke (`smoke:launcher`), the prod CLI and TUI end-to-end smokes (`smoke:cli` / `smoke:tui`), and the prod web end-to-end smoke (`smoke:web`) — together `validate:launcher`. The sub-scripts (`validate:web`, `validate:launcher`, `smoke:launcher`, `smoke:cli`, `smoke:tui`, `smoke:web`) can be run individually when iterating on one client. Storybook is the only CI step left out (see below).
|
||||
- ALWAYS do `npm run validate` before pushing any changes — from the repo root it 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`.
|
||||
- Before pushing, also run **`npm run coverage`** — `validate` is fast and does NOT enforce the per-file gate (or, for web, run the integration project); `coverage` does both. CI does **not** run `coverage` (the gate is local-only); it runs `validate` plus a standalone web `test:integration` step.
|
||||
- **`smoke` is a separate top-level target, NOT part of `validate`.** Run it (or the individual `smoke:*`) after a build/validate: `npm run validate && npm run smoke`. 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). CI runs `validate`, then the web `test:integration` step, then `smoke`. Storybook is the only CI step left out (see below).
|
||||
- `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`.
|
||||
- `smoke:cli` (`scripts/smoke-cli.mjs`) drives `mcp-inspector --cli` through the built launcher against the bundled stdio test server via a temp `--catalog`: it asserts `tools/list` returns the server's tools (real connect over stdio), the default writable catalog is seeded empty on first run, a missing read-only `--config` errors without seeding, and `--catalog` + `--config` is rejected. `smoke:tui` (`scripts/smoke-tui.mjs`) launches `mcp-inspector --tui --catalog <temp>` and asserts the Ink app renders its first frame (the "MCP Servers" panel) within a timeout, then SIGTERMs it — a shallow boot/render check, not full interaction. Both build `test-servers/build` on demand if it's missing.
|
||||
- Also run `npm run test:storybook` before pushing — it executes every story's `play` function in headless Chromium via `@vitest/browser-playwright` (~10s). CI runs this as a separate step after the unit/lint/build checks; failures block merge. It is kept out of `validate` because it needs the Playwright browser binary and is much slower than the unit suite.
|
||||
- `smoke:cli` (`scripts/smoke-cli.mjs`) drives `mcp-inspector --cli` through the built launcher against the bundled stdio test server via a temp `--catalog`: it asserts `tools/list` returns the server's tools (real connect over stdio), the default writable catalog is seeded empty on first run, a missing read-only `--config` errors without seeding, and `--catalog` + `--config` is rejected. `smoke:tui` (`scripts/smoke-tui.mjs`) launches `mcp-inspector --tui --catalog <temp>` and asserts the Ink app renders its first frame (the "MCP Servers" panel) within a timeout, then SIGTERMs it — a shallow boot/render check, not full interaction. **`smoke:tui` is local-only: it self-skips when `process.env.CI` is set**, because the Ink TUI needs a real TTY (raw mode) that headless CI lacks — so run it (via `npm run smoke`) on your own machine before pushing. Both build `test-servers/build` on demand if it's missing.
|
||||
- Also run `npm run test:storybook` from `clients/web/` before pushing — it executes every story's `play` function in headless Chromium via `@vitest/browser-playwright` (~10s). CI runs this as a separate step (from `clients/web`) after `validate`; failures block merge. It is kept out of `validate` because it needs the Playwright browser binary and is much slower than the unit suite. (There is no root-level `test:storybook` aggregate — run it in the web client.)
|
||||
|
||||
### Typescript instructions
|
||||
- Use TypeScript for all new code
|
||||
|
|
|
|||
Loading…
Reference in a new issue