diff --git a/AGENTS.md b/AGENTS.md index 1ba109fd0..733691ff0 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -214,12 +214,12 @@ gh project item-edit --project-id PVT_kwDOCt2Azc4BJVxt --id "$ITEM_ID" --field-i - after making the changes, respond to each review comment with what was done (or why it was ignored) ### Mandatory pre-push gate -- ALWAYS do `npm run format` before committing — the **root** `format` auto-fixes `core/` (`format:core`), the root `scripts/**/*.{mjs,js,cjs,ts}` (`format:scripts`), and every client's scope in one shot. `validate` runs `format:check` (the non-fixing variant, including `format:check:core` and `format:check:scripts`) 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 format` before committing — the **root** `format` auto-fixes `core/` (`format:core`), the root `scripts/` tooling (`format:scripts`), the root "shared" surface (`format:shared` — `test-servers/src/**`, `vitest.shared.mts`, the root `eslint.config.js`), and every client's scope in one shot. Every **client** format glob uses the uniform extension set `*.{ts,tsx,mts,cts,js,jsx,mjs,cjs}` (#1792) so a new-extension file can't slip the gate; `core/` stays `{ts,tsx}` and the shared surface `{ts,tsx,mts,cts}` (their surfaces can't hold the other extensions), and `npm run verify:format-coverage` (the first step of `validate`, #1792) is the backstop — it fails if any tracked source file is left uncovered by a `format:check` glob regardless of which glob was expected to catch it. `validate` runs `format:check` (the non-fixing variant, including `format:check:core`, `format:check:scripts`, and `format:check:shared`) 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` is the mandatory pre-push command** — it mirrors `.github/workflows/main.yml` (minus `npm install`): `validate` → `coverage` → `verify:build-gate` (the #1769 browser-externalized-builtin build gate) → `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` runs the **`core/` gate first** (`validate:core`) and then chains the four per-client validations (`validate:web` → `validate:cli` → `validate:tui` → `validate:launcher`); each client delegates to its own `npm run validate` 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). - - **`validate:core` is the root-owned format + lint gate (#1689, widened in #1778).** Each client's `prettier`/`eslint` is scoped to its own dir, so nothing reached `core/` or the root `scripts/` before — `validate:core` closes that: it runs `format:check:core` (`prettier --check "core/**/*.{ts,tsx}"`) + `format:check:scripts` (`prettier --check "scripts/**/*.{mjs,js,cjs,ts}"`, the root build/verify tooling — #1778; a brace glob so a future non-`.mjs` script is auto-included) + `lint:core` (`eslint "core/**/*.{ts,tsx}"` via the **root** `eslint.config.js`). Use `npm run format:core` / `npm run format:scripts` to auto-fix (both folded into the root `format`). The `scripts/` gate is prettier-only — the root has no eslint config for `.mjs`. The root carries prettier/eslint as devDependencies for this; `core/` is isomorphic (browser + Node globals, no JSX today — the `{ts,tsx}` glob future-proofs against a `core/**/*.tsx`). The root `eslint.config.js` honors an `_`-prefix as the intentionally-unused marker (`argsIgnorePattern`/`varsIgnorePattern`/`caughtErrorsIgnorePattern: '^_'`). +- ALWAYS do `npm run format` before committing, then **`npm run ci`** before pushing. From the repo root, `validate` runs **`verify:format-coverage` first** (the #1792 guard — asserts every tracked source file is covered by a `format:check` glob), then the **`core/` gate** (`validate:core`), then chains the four per-client validations (`validate:web` → `validate:cli` → `validate:tui` → `validate:launcher`); each client delegates to its own `npm run validate` 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). + - **`validate:core` is the root-owned format + lint gate (#1689, widened in #1778 and #1767).** Each client's `prettier`/`eslint` is scoped to its own dir, so nothing reached `core/`, the root `scripts/`, or the root "shared" surface before — `validate:core` closes that: it runs `format:check:core` (`prettier --check "core/**/*.{ts,tsx}"`) + `format:check:scripts` (`prettier --check "scripts/**/*.{ts,tsx,mts,cts,js,jsx,mjs,cjs}"`, the root build/verify tooling — #1778) + `format:check:shared` + `lint:core` (`eslint "core/**/*.{ts,tsx}"` via the **root** `eslint.config.js`) + `lint:shared`. Use `npm run format:core` / `npm run format:scripts` / `npm run format:shared` to auto-fix (all folded into the root `format`). The **shared surface** (#1767) is `test-servers/src/**/*.{ts,tsx,mts,cts}`, the root `vitest.shared.mts`, and the root `eslint.config.js` — first-party code no client's `eslint .` / `prettier` reaches; it is both prettier-gated (`format:check:shared`) and eslint-gated (`lint:shared`, via a second `files` block in the root `eslint.config.js` scoped to Node globals). The `scripts/` gate is prettier-only — the root has no eslint config for `.mjs`. The root carries prettier/eslint as devDependencies for this; `core/` is isomorphic (browser + Node globals, no JSX today — the `{ts,tsx}` glob future-proofs against a `core/**/*.tsx`). The root `eslint.config.js` honors an `_`-prefix as the intentionally-unused marker (`argsIgnorePattern`/`varsIgnorePattern`/`caughtErrorsIgnorePattern: '^_'`). **prettier is pinned to an exact version** (not a caret) in all five `package.json`s (#1790) so the gate's verdict can't shift with an in-range patch bump. - **cli and tui now typecheck their `src` (#1689).** Their `build`/`test` run through esbuild (no type check), so each has a `typecheck` script (`tsc --noEmit -p tsconfig.json`) folded into `validate`. Their `tsconfig.json` matches `clients/web/tsconfig.app.json`'s module/lib *resolution* options — DOM lib, `moduleResolution: bundler`, and **no** `noUncheckedIndexedAccess` (web's app config does not extend `tsconfig.base`, so re-enabling it would surface `core/` issues web never gates) — so the imported `core/` sources are validated the same way web validates them. It does **not** mirror web's extra strictness flags (`noUnusedLocals`, `verbatimModuleSyntax`, ES2023 target, …), so cli/tui's own `src` is checked slightly more loosely than web's. `core/` itself still typechecks through web's `tsc -b`. - - 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 && typecheck && 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 && (typecheck &&) build && test`. (#1778) `clients/web`'s `format`/`format:check` covers `src`, `server`, and its top-level configs (`*.{ts,js}` — `vite.config.ts`, `tsup.runner.config.ts`, `eslint.config.js`, …), not just `src`, so the Node backend and Vite/build config are prettier-gated too. + - 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 && typecheck && 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 && (typecheck &&) build && test`. (#1778, #1789, #1792) `clients/web`'s `format`/`format:check` covers `src`, `server`, `.storybook`, and its top-level configs (the uniform `*.{ts,tsx,mts,cts,js,jsx,mjs,cjs}` glob — `vite.config.ts`, `tsup.runner.config.ts`, `eslint.config.js`, …), not just `src`, so the Node backend, Storybook config, and Vite/build config are prettier-gated too; `clients/launcher`'s covers `src`, `__tests__`, `scripts`, and its top-level configs (the `*.` top-level glob is non-recursive, so each nested dir — `.storybook`, `scripts` — is named explicitly). The `verify:format-coverage` guard (#1792) enforces that this coverage stays complete. - **`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` / `smoke:web:browser`, 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 `coverage` gate (which also covers the web integration project), then `verify:build-gate` (the #1769 build gate — see below), then `smoke` (with Playwright chromium installed just before it, since `smoke:web:browser` needs it). GitHub CI runs this same chain as separate workflow steps, with the Storybook play-function tests last (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. @@ -251,11 +251,11 @@ The web client keeps two grab-bag directories under `clients/web/src`, split by - _Importing from `@inspector/core`._ Two forms are fine: a **type-only** import is not a subsystem dependency (`pendingReauth` is pure type declarations), and **re-exporting pure functions or constants** from core is not subsystem ownership either (`oauthUx`/`oauthFlow` re-export core copy/predicates). What makes a module `lib` is wrapping core's *stateful runtime*, not merely importing from it. - **`src/lib/`** — infrastructure / integration / stateful adapters. Modules that instantiate or compose subsystems, wrap the `@inspector/core` **runtime** (not just its types), touch the DOM / `window` / `sessionStorage`, or otherwise produce side effects. (Anchors: `environmentFactory` composes `InspectorClientEnvironment`; `remoteOAuthStorage` is an adapter class over `core/auth`; `oauthResume` reads/writes `sessionStorage`; `browserTabVisibility` registers `visibilitychange` listeners; `clearServerOAuthState` drives the live `InspectorClient` / `OAuthStorage`; `downloadFile` triggers browser downloads.) -The top-level **`src/types/`** is a sibling of both and is not the place for new domain types — it's essentially the home for ambient `.d.ts` module stubs (e.g. the `react-syntax-highlighter` shims wired through `tsconfig.app.json` `paths`). One plain-`.ts` domain type still lingers there, the dead `navigation.ts` `InspectorTab`, which [#1785](https://github.com/modelcontextprotocol/inspector/issues/1785) will remove; until then a pure domain type belongs in `utils/`, not `src/types/`. +The top-level **`src/types/`** is a sibling of both and is not the place for new domain types — it's now purely the home for ambient `.d.ts` module stubs (e.g. the `react-syntax-highlighter` shims wired through `tsconfig.app.json` `paths`). The last plain-`.ts` domain type there, the dead `navigation.ts` `InspectorTab`, was removed in #1785, so a pure domain type belongs in `utils/`, not `src/types/`. Cross-directory imports point **one way, `lib → utils`** (infra depends on pure helpers, never the reverse). Keep it that way: if a `utils/` module needs a type currently exported from a `lib/` module, declare the type in `utils/` and re-export it from `lib/` (as `pendingReauth` owns `OAuthResumeAuthKind` and `oauthResume` re-exports it), rather than importing "up" from `utils` into `lib`. -Nothing **enforces** the boundary: no path alias keys off it, and the coverage `include` in `clients/web/vite.config.ts` lists **both** `src/lib/**` and `src/utils/**`, so a move between them is coverage-neutral (this is why the refactor was gate-safe). It's a human-legible signal at import time, valuable in a codebase this test-heavy (the ≥90% per-file gate). Note that `include` is a **whitelist** — it names `components`/`lib`/`utils`/`server` (plus the `core/*` runtime), so a module placed **outside** those directories (`hooks/`, `theme/`, `types/`, `App.tsx`, or a brand-new grab-bag) falls out of the ≥90 gate entirely, silently. When adding a module, place it by the rule and keep it inside a gated directory; when it genuinely mixes both (e.g. `downloadFile` bundles DOM-side-effect helpers with a couple of pure ones), keep it whole on its dominant side (`lib`) rather than splitting hairs. +Nothing **enforces** the boundary: no path alias keys off it, and the coverage `include` in `clients/web/vite.config.ts` lists **both** `src/lib/**` and `src/utils/**`, so a move between them is coverage-neutral (this is why the refactor was gate-safe). It's a human-legible signal at import time, valuable in a codebase this test-heavy (the ≥90% per-file gate). Note that `include` is a **whitelist** — it names `components`/`hooks`/`theme`/`lib`/`utils`/`server` (plus the `core/*` runtime; `hooks` and `theme` were added in #1787), so a module placed **outside** those directories (`types/`, `App.tsx`, or a brand-new grab-bag) falls out of the ≥90 gate entirely, silently. The **deliberate, documented** top-level-file exceptions are `src/App.tsx` — a ~4.5k-line composition root at ~42% branch coverage (gating it is a dedicated testing/decomposition effort, not a whitelist tweak) — and the `src/main.tsx` / `src/index.ts` bootstraps (browser `createRoot` render and the bin `runWeb` re-export, the analog of `clients/cli`'s excluded `src/index.ts`). All three are called out in a comment on the `include` array itself rather than left silent. When adding a module, place it by the rule and keep it inside a gated directory; when it genuinely mixes both (e.g. `downloadFile` bundles DOM-side-effect helpers with a couple of pure ones), keep it whole on its dominant side (`lib`) rather than splitting hairs. ## React instructions - UI Components diff --git a/README.md b/README.md index 077406f9b..a8b358ce7 100644 --- a/README.md +++ b/README.md @@ -35,7 +35,7 @@ inspector/ │ ├── react/ # React hooks over the state stores │ └── storage/ # File I/O helpers for the OAuth persist backends ├── test-servers/ # Composable MCP test servers + fixtures used by integration tests -├── scripts/ # Root build/verify tooling (install cascade, smokes, verify-build-gate, pack:verify) +├── scripts/ # Root build/verify tooling (install cascade, smokes, verify-build-gate, verify-format-coverage, pack:verify) ├── specification/ # Design/build specifications ├── AGENTS.md # Contribution rules for agents AND humans (see below) └── README.md # You are here @@ -140,14 +140,15 @@ Each client self-validates from its own folder; the root scripts chain them. The | Script | What it does | | --------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `npm run validate` | Runs `validate:core` (the shared `core/` `format:check` + `lint` gate) first, then per client: `format:check` + `lint` + **`typecheck`** (cli/tui only) + `build` + fast unit tests. The quick inner-loop check. | +| `npm run validate` | Runs `verify:format-coverage` (asserts every tracked source file is format-gated) first, then `validate:core` (the shared `core/` `format:check` + `lint` gate), then per client: `format:check` + `lint` + **`typecheck`** (cli/tui only) + `build` + fast unit tests. The quick inner-loop check. | | `npm run coverage` | The **per-file ≥90% gate** (lines/statements/functions/branches) under v8 instrumentation, per client. CI-enforced. For web this also runs the integration project and covers the shared `core/` runtime (including `core/json` and `core/client`). | | `npm run smoke` | End-to-end smokes through the built launcher (`--help` dispatch + prod cli/tui/web), plus a headless-Chromium boot smoke that runs the prod web bundle and asserts a clean first render (no uncaught error — sync exception or unhandled rejection, how a Node built-in reaching the browser bundle manifests). | | `npm run verify:build-gate` | Runs a real `vite build` with a Node built-in forced into the browser graph and asserts the build **fails** via the #1769 gate (which turns Vite's browser-externalization warning into a hard error). Guards against the warning phrasing drifting in a Vite bump and silently disabling the gate. Part of `npm run ci`. | +| `npm run verify:format-coverage` | Parses the `format:check` globs out of every `package.json` (only those reachable from `validate`), enumerates all tracked source files, and **fails** listing any not covered by a glob — the durable guard for the "every first-party source file is format-gated" invariant (#1792). Runs first in `validate`. | | `npm run ci` | **Mandatory pre-push command.** `validate` → `coverage` → `verify:build-gate` → `smoke` → Storybook. A true superset of GitHub CI. | | `npm run pack:verify` | Publish smoke — see [Publishing](#publishing). | -Per-client scripts exist too (`validate:web`, `coverage:cli`, `smoke:tui`, …), plus root `validate:core` / `format:core` for the shared `core/` package and `format:scripts` for the root `scripts/` tooling. Run `npm run format` before committing — the root `format` fixes `core/`, the root `scripts/`, and every client; `validate` runs the non-fixing `format:check` and fails CI on any unformatted file. +Per-client scripts exist too (`validate:web`, `coverage:cli`, `smoke:tui`, …), plus root `validate:core` / `format:core` for the shared `core/` package, `format:scripts` for the root `scripts/` tooling, and `format:shared` / `lint:shared` for the root "shared" surface (`test-servers/src/**`, `vitest.shared.mts`, the root `eslint.config.js`). Run `npm run format` before committing — the root `format` fixes `core/`, the root `scripts/`, the shared surface, and every client; `validate` runs the non-fixing `format:check` and fails CI on any unformatted file. For the full testing rules — the ≥90% per-file gate, where test files live, the unit vs. integration vs. storybook projects, and the `v8 ignore` policy — see [`AGENTS.md`](./AGENTS.md). diff --git a/clients/cli/eslint.config.js b/clients/cli/eslint.config.js index 1157168e4..9407f9975 100644 --- a/clients/cli/eslint.config.js +++ b/clients/cli/eslint.config.js @@ -1,18 +1,18 @@ -import js from '@eslint/js'; -import globals from 'globals'; -import tseslint from 'typescript-eslint'; -import { defineConfig, globalIgnores } from 'eslint/config'; +import js from "@eslint/js"; +import globals from "globals"; +import tseslint from "typescript-eslint"; +import { defineConfig, globalIgnores } from "eslint/config"; // The CLI is plain Node TypeScript (no React/browser), so this mirrors the web // client's flat config minus the React/Storybook plugins. export default defineConfig([ - globalIgnores(['build', 'coverage']), + globalIgnores(["build", "coverage"]), { - files: ['**/*.ts'], + files: ["**/*.ts"], extends: [js.configs.recommended, tseslint.configs.recommended], languageOptions: { ecmaVersion: 2022, - sourceType: 'module', + sourceType: "module", globals: globals.node, }, }, diff --git a/clients/cli/package-lock.json b/clients/cli/package-lock.json index 2760136d3..0198d9f4d 100644 --- a/clients/cli/package-lock.json +++ b/clients/cli/package-lock.json @@ -29,7 +29,7 @@ "@vitest/coverage-v8": "^4.1.0", "eslint": "^9.39.4", "globals": "^17.4.0", - "prettier": "^3.8.1", + "prettier": "3.8.4", "tsup": "^8.5.0", "typescript": "~5.9.3", "typescript-eslint": "^8.56.1", diff --git a/clients/cli/package.json b/clients/cli/package.json index 9ef0e9bd8..b16990e31 100644 --- a/clients/cli/package.json +++ b/clients/cli/package.json @@ -28,8 +28,8 @@ "test:cli-metadata": "vitest run metadata.test.ts", "pretest": "npm run test-servers:build && npm run build", "lint": "eslint .", - "format": "prettier --write src __tests__", - "format:check": "prettier --check src __tests__" + "format": "prettier --write src __tests__ \"*.{ts,tsx,mts,cts,js,jsx,mjs,cjs}\"", + "format:check": "prettier --check src __tests__ \"*.{ts,tsx,mts,cts,js,jsx,mjs,cjs}\"" }, "dependencies": { "@modelcontextprotocol/client": "2.0.0-beta.5", @@ -50,7 +50,7 @@ "@vitest/coverage-v8": "^4.1.0", "eslint": "^9.39.4", "globals": "^17.4.0", - "prettier": "^3.8.1", + "prettier": "3.8.4", "tsup": "^8.5.0", "typescript": "~5.9.3", "typescript-eslint": "^8.56.1", diff --git a/clients/cli/tsup.config.ts b/clients/cli/tsup.config.ts index d65d37ec3..bacd84730 100644 --- a/clients/cli/tsup.config.ts +++ b/clients/cli/tsup.config.ts @@ -1,32 +1,32 @@ -import { defineConfig } from 'tsup'; -import path from 'node:path'; -import { fileURLToPath } from 'node:url'; +import { defineConfig } from "tsup"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; const dirname = path.dirname(fileURLToPath(import.meta.url)); -const repoRoot = path.resolve(dirname, '../..'); +const repoRoot = path.resolve(dirname, "../.."); export default defineConfig({ - entry: ['src/index.ts'], - format: ['esm'], - outDir: 'build', + entry: ["src/index.ts"], + format: ["esm"], + outDir: "build", clean: true, // No source maps in the published bundle — they roughly double the on-disk // size and aren't needed at runtime (debug via `npm run dev` on the source). sourcemap: false, - target: 'node22', - platform: 'node', + target: "node22", + platform: "node", // Bundle core source; leave npm deps external. noExternal: [/^@inspector\/core/], external: [ - '@napi-rs/keyring', - '@modelcontextprotocol/client', - '@modelcontextprotocol/core', - 'commander', - 'pino', + "@napi-rs/keyring", + "@modelcontextprotocol/client", + "@modelcontextprotocol/core", + "commander", + "pino", ], esbuildOptions(options) { options.alias = { - '@inspector/core': path.join(repoRoot, 'core'), + "@inspector/core": path.join(repoRoot, "core"), }; }, }); diff --git a/clients/cli/vitest.config.ts b/clients/cli/vitest.config.ts index 4480e1bd3..d6afd7333 100644 --- a/clients/cli/vitest.config.ts +++ b/clients/cli/vitest.config.ts @@ -1,7 +1,7 @@ -import path from 'node:path'; -import { fileURLToPath } from 'node:url'; -import { defineConfig } from 'vitest/config'; -import { vitestSharedPaths } from '../../vitest.shared.mts'; +import path from "node:path"; +import { fileURLToPath } from "node:url"; +import { defineConfig } from "vitest/config"; +import { vitestSharedPaths } from "../../vitest.shared.mts"; const dirname = path.dirname(fileURLToPath(import.meta.url)); const { projectResolve } = vitestSharedPaths(dirname); @@ -10,8 +10,8 @@ export default defineConfig({ resolve: projectResolve, test: { globals: false, - environment: 'node', - include: ['__tests__/**/*.test.ts'], + environment: "node", + include: ["__tests__/**/*.test.ts"], testTimeout: 15000, // The in-process runner (__tests__/helpers/cli-runner.ts) patches // process.std{out,err}.write to capture CLI output. Test files run in @@ -19,18 +19,18 @@ export default defineConfig({ // those global patches never overlap. `forks` is vitest's default, but pin // it explicitly so the capture isolation can't regress to a shared-thread // pool. See #1484. - pool: 'forks', + pool: "forks", coverage: { - provider: 'v8', - reporter: ['text', 'html', 'json-summary'], - include: ['src/**/*.ts'], + provider: "v8", + reporter: ["text", "html", "json-summary"], + include: ["src/**/*.ts"], exclude: [ // Binary bootstrap: shebang + `isMain` guard + `runCli()`/`process.exit` // wiring that only runs when launched as the real binary. Exercised by // the out-of-process layer (__tests__/e2e.test.ts + scripts/smoke-cli.mjs), // which spawns build/index.js and so can't be measured under in-process // coverage. Mirrors web's `**/index.{ts,tsx}` exclusion. - 'src/index.ts', + "src/index.ts", ], thresholds: { perFile: true, diff --git a/clients/launcher/eslint.config.js b/clients/launcher/eslint.config.js index 17a394f19..535689a1f 100644 --- a/clients/launcher/eslint.config.js +++ b/clients/launcher/eslint.config.js @@ -1,18 +1,18 @@ -import js from '@eslint/js'; -import globals from 'globals'; -import tseslint from 'typescript-eslint'; -import { defineConfig, globalIgnores } from 'eslint/config'; +import js from "@eslint/js"; +import globals from "globals"; +import tseslint from "typescript-eslint"; +import { defineConfig, globalIgnores } from "eslint/config"; // The launcher is plain Node TypeScript (no React/browser), so this mirrors // the web client's flat config minus the React/Storybook plugins. export default defineConfig([ - globalIgnores(['build', 'coverage']), + globalIgnores(["build", "coverage"]), { - files: ['**/*.{ts,mts}'], + files: ["**/*.{ts,mts}"], extends: [js.configs.recommended, tseslint.configs.recommended], languageOptions: { ecmaVersion: 2022, - sourceType: 'module', + sourceType: "module", globals: globals.node, }, }, diff --git a/clients/launcher/package-lock.json b/clients/launcher/package-lock.json index 55f0543cf..4c112ddc4 100644 --- a/clients/launcher/package-lock.json +++ b/clients/launcher/package-lock.json @@ -19,7 +19,7 @@ "@vitest/coverage-v8": "^4.1.0", "eslint": "^9.39.4", "globals": "^17.4.0", - "prettier": "^3.8.1", + "prettier": "3.8.4", "typescript": "~5.9.3", "typescript-eslint": "^8.56.1", "vitest": "^4.1.0" diff --git a/clients/launcher/package.json b/clients/launcher/package.json index 6a5f5ff76..f0602565a 100644 --- a/clients/launcher/package.json +++ b/clients/launcher/package.json @@ -15,8 +15,8 @@ "build": "tsc", "postbuild": "node scripts/make-executable.js", "lint": "eslint .", - "format": "prettier --write src __tests__", - "format:check": "prettier --check src __tests__", + "format": "prettier --write src __tests__ scripts \"*.{ts,tsx,mts,cts,js,jsx,mjs,cjs}\"", + "format:check": "prettier --check src __tests__ scripts \"*.{ts,tsx,mts,cts,js,jsx,mjs,cjs}\"", "test": "vitest run", "test:coverage": "vitest run --coverage", "validate": "npm run format:check && npm run lint && npm run build && npm run test" @@ -30,7 +30,7 @@ "@vitest/coverage-v8": "^4.1.0", "eslint": "^9.39.4", "globals": "^17.4.0", - "prettier": "^3.8.1", + "prettier": "3.8.4", "typescript": "~5.9.3", "typescript-eslint": "^8.56.1", "vitest": "^4.1.0" diff --git a/clients/launcher/vitest.config.ts b/clients/launcher/vitest.config.ts index b64920005..eef71846a 100644 --- a/clients/launcher/vitest.config.ts +++ b/clients/launcher/vitest.config.ts @@ -1,14 +1,14 @@ -import { defineConfig } from 'vitest/config'; +import { defineConfig } from "vitest/config"; export default defineConfig({ test: { globals: false, - environment: 'node', - include: ['__tests__/**/*.test.ts'], + environment: "node", + include: ["__tests__/**/*.test.ts"], coverage: { - provider: 'v8', - reporter: ['text', 'html', 'json-summary'], - include: ['src/**/*.ts'], + provider: "v8", + reporter: ["text", "html", "json-summary"], + include: ["src/**/*.ts"], exclude: [ // Binary bootstrap: shebang + commander wiring that dynamically imports // each client's build/index.js and dispatches via process.exit. It only @@ -16,7 +16,7 @@ export default defineConfig({ // launcher smokes (smoke:launcher / smoke:cli / smoke:tui / smoke:web). // Mirrors the cli/web `index.ts` exclusion. The pure arg-parsing logic // lives in parse-launcher-argv.ts, which is unit-tested and gated below. - 'src/index.ts', + "src/index.ts", ], thresholds: { perFile: true, diff --git a/clients/tui/eslint.config.js b/clients/tui/eslint.config.js index 64bf01aaa..b4f8a251a 100644 --- a/clients/tui/eslint.config.js +++ b/clients/tui/eslint.config.js @@ -1,8 +1,8 @@ -import js from '@eslint/js'; -import globals from 'globals'; -import reactHooks from 'eslint-plugin-react-hooks'; -import tseslint from 'typescript-eslint'; -import { defineConfig, globalIgnores } from 'eslint/config'; +import js from "@eslint/js"; +import globals from "globals"; +import reactHooks from "eslint-plugin-react-hooks"; +import tseslint from "typescript-eslint"; +import { defineConfig, globalIgnores } from "eslint/config"; // The TUI is Node + React (Ink) TypeScript. This mirrors the web client's flat // config minus the browser-only plugins (react-refresh / Storybook). @@ -15,19 +15,19 @@ import { defineConfig, globalIgnores } from 'eslint/config'; // component/hook surface that is itself the interim-excluded one tracked in // #1501. Enforcing them would require refactoring code out of scope here. export default defineConfig([ - globalIgnores(['build', 'coverage']), + globalIgnores(["build", "coverage"]), { - files: ['**/*.{ts,tsx}'], - plugins: { 'react-hooks': reactHooks }, + files: ["**/*.{ts,tsx}"], + plugins: { "react-hooks": reactHooks }, extends: [js.configs.recommended, tseslint.configs.recommended], languageOptions: { ecmaVersion: 2022, - sourceType: 'module', + sourceType: "module", globals: globals.node, }, rules: { - 'react-hooks/rules-of-hooks': 'error', - 'react-hooks/exhaustive-deps': 'warn', + "react-hooks/rules-of-hooks": "error", + "react-hooks/exhaustive-deps": "warn", }, }, ]); diff --git a/clients/tui/package-lock.json b/clients/tui/package-lock.json index 1f24c6f61..e805f79cf 100644 --- a/clients/tui/package-lock.json +++ b/clients/tui/package-lock.json @@ -34,7 +34,7 @@ "eslint-plugin-react-hooks": "^7.0.1", "globals": "^17.4.0", "ink-testing-library": "^4.0.0", - "prettier": "^3.8.1", + "prettier": "3.8.4", "tsup": "^8.5.0", "tsx": "^4.21.0", "typescript": "~5.9.3", diff --git a/clients/tui/package.json b/clients/tui/package.json index 67095d5dd..005801f47 100644 --- a/clients/tui/package.json +++ b/clients/tui/package.json @@ -23,8 +23,8 @@ "test": "vitest run", "test:coverage": "vitest run --coverage", "lint": "eslint .", - "format": "prettier --write src __tests__ index.ts tui.tsx dev.ts", - "format:check": "prettier --check src __tests__ index.ts tui.tsx dev.ts" + "format": "prettier --write src __tests__ \"*.{ts,tsx,mts,cts,js,jsx,mjs,cjs}\"", + "format:check": "prettier --check src __tests__ \"*.{ts,tsx,mts,cts,js,jsx,mjs,cjs}\"" }, "dependencies": { "@modelcontextprotocol/client": "2.0.0-beta.5", @@ -53,7 +53,7 @@ "eslint-plugin-react-hooks": "^7.0.1", "globals": "^17.4.0", "ink-testing-library": "^4.0.0", - "prettier": "^3.8.1", + "prettier": "3.8.4", "tsup": "^8.5.0", "tsx": "^4.21.0", "typescript": "~5.9.3", diff --git a/clients/tui/tsup.config.ts b/clients/tui/tsup.config.ts index 7d4313076..92a637b37 100644 --- a/clients/tui/tsup.config.ts +++ b/clients/tui/tsup.config.ts @@ -1,37 +1,37 @@ -import { defineConfig } from 'tsup'; -import path from 'node:path'; -import { fileURLToPath } from 'node:url'; +import { defineConfig } from "tsup"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; const dirname = path.dirname(fileURLToPath(import.meta.url)); -const repoRoot = path.resolve(dirname, '../..'); +const repoRoot = path.resolve(dirname, "../.."); export default defineConfig({ - entry: ['index.ts'], - format: ['esm'], - outDir: 'build', + entry: ["index.ts"], + format: ["esm"], + outDir: "build", clean: true, // No source maps in the published bundle — they roughly double the on-disk // size and aren't needed at runtime (debug via `npm run dev` on the source). sourcemap: false, - target: 'node22', - platform: 'node', + target: "node22", + platform: "node", noExternal: [/^@inspector\/core/], external: [ - 'react', - 'ink', - 'ink-form', - 'ink-scroll-view', - 'open', - 'commander', - 'pino', - '@modelcontextprotocol/client', - '@modelcontextprotocol/core', - '@napi-rs/keyring', + "react", + "ink", + "ink-form", + "ink-scroll-view", + "open", + "commander", + "pino", + "@modelcontextprotocol/client", + "@modelcontextprotocol/core", + "@napi-rs/keyring", ], esbuildOptions(options) { options.alias = { - '@inspector/core': path.join(repoRoot, 'core'), + "@inspector/core": path.join(repoRoot, "core"), }; - options.jsx = 'automatic'; + options.jsx = "automatic"; }, }); diff --git a/clients/web/.storybook/main.ts b/clients/web/.storybook/main.ts index 3f89d8e61..69b7bb220 100644 --- a/clients/web/.storybook/main.ts +++ b/clients/web/.storybook/main.ts @@ -1,18 +1,15 @@ -import type { StorybookConfig } from '@storybook/react-vite'; +import type { StorybookConfig } from "@storybook/react-vite"; const config: StorybookConfig = { - "stories": [ - "../src/**/*.mdx", - "../src/**/*.stories.@(js|jsx|mjs|ts|tsx)" - ], - "addons": [ + stories: ["../src/**/*.mdx", "../src/**/*.stories.@(js|jsx|mjs|ts|tsx)"], + addons: [ "@chromatic-com/storybook", "@storybook/addon-vitest", "@storybook/addon-a11y", "@storybook/addon-docs", - "@storybook/addon-onboarding" + "@storybook/addon-onboarding", ], - "framework": "@storybook/react-vite", + framework: "@storybook/react-vite", previewHead: (head) => ` ${head} @@ -20,4 +17,4 @@ const config: StorybookConfig = { `, }; -export default config; \ No newline at end of file +export default config; diff --git a/clients/web/.storybook/preview.tsx b/clients/web/.storybook/preview.tsx index a17e18d82..6b419a436 100644 --- a/clients/web/.storybook/preview.tsx +++ b/clients/web/.storybook/preview.tsx @@ -1,64 +1,74 @@ -import type { Preview } from '@storybook/react-vite' -import { MantineProvider, useMantineColorScheme, type CSSVariablesResolver } from '@mantine/core' -import { Notifications } from '@mantine/notifications' -import '@mantine/core/styles.css' -import '@mantine/notifications/styles.css' -import '../src/App.css' -import { theme } from '../src/theme/theme' -import { useEffect } from 'react' +import type { Preview } from "@storybook/react-vite"; +import { + MantineProvider, + useMantineColorScheme, + type CSSVariablesResolver, +} from "@mantine/core"; +import { Notifications } from "@mantine/notifications"; +import "@mantine/core/styles.css"; +import "@mantine/notifications/styles.css"; +import "../src/App.css"; +import { theme } from "../src/theme/theme"; +import { useEffect } from "react"; // eslint-disable-next-line react-refresh/only-export-components function ColorSchemeWrapper({ colorScheme, children, }: { - colorScheme: 'light' | 'dark' - children: React.ReactNode + colorScheme: "light" | "dark"; + children: React.ReactNode; }) { - const { setColorScheme } = useMantineColorScheme() + const { setColorScheme } = useMantineColorScheme(); useEffect(() => { - setColorScheme(colorScheme) - }, [colorScheme, setColorScheme]) + setColorScheme(colorScheme); + }, [colorScheme, setColorScheme]); - return <>{children} + return <>{children}; } const resolver: CSSVariablesResolver = () => ({ variables: {}, light: {}, dark: { - '--mantine-color-body': 'var(--mantine-color-dark-9)', + "--mantine-color-body": "var(--mantine-color-dark-9)", }, -}) +}); const preview: Preview = { globalTypes: { colorScheme: { - description: 'Mantine color scheme', + description: "Mantine color scheme", toolbar: { - title: 'Color Scheme', - icon: 'mirror', + title: "Color Scheme", + icon: "mirror", items: [ - { value: 'light', title: 'Light', icon: 'sun' }, - { value: 'dark', title: 'Dark', icon: 'moon' }, + { value: "light", title: "Light", icon: "sun" }, + { value: "dark", title: "Dark", icon: "moon" }, ], dynamicTitle: true, }, }, }, initialGlobals: { - colorScheme: 'light', + colorScheme: "light", }, decorators: [ (Story, context) => { - const isFullscreen = context.parameters?.layout === 'fullscreen' + const isFullscreen = context.parameters?.layout === "fullscreen"; return ( - - + + {isFullscreen ? ( -
+
) : ( @@ -66,7 +76,7 @@ const preview: Preview = { )} - ) + ); }, ], parameters: { @@ -80,7 +90,7 @@ const preview: Preview = { // `'error'` fails the Storybook play-function tests (part of `npm run ci`) // on any axe violation, so the zero-violation state this PR reached is // enforced going forward rather than being a point-in-time result. - test: 'error', + test: "error", // Stories render presentational components (and whole screens) in // isolation, outside the app's `AppShell` — which is what provides the // page landmarks (`
`, nav) in the real app. The `region` rule @@ -90,11 +100,11 @@ const preview: Preview = { // by the components under test. Disable it here so the a11y panel surfaces // only rules the components can actually own. config: { - rules: [{ id: 'region', enabled: false }], + rules: [{ id: "region", enabled: false }], }, }, - layout: 'centered', + layout: "centered", }, -} +}; -export default preview +export default preview; diff --git a/clients/web/.storybook/vitest.setup.ts b/clients/web/.storybook/vitest.setup.ts index 44922d55e..fd7ac45e5 100644 --- a/clients/web/.storybook/vitest.setup.ts +++ b/clients/web/.storybook/vitest.setup.ts @@ -1,7 +1,7 @@ import * as a11yAddonAnnotations from "@storybook/addon-a11y/preview"; -import { setProjectAnnotations } from '@storybook/react-vite'; -import * as projectAnnotations from './preview'; +import { setProjectAnnotations } from "@storybook/react-vite"; +import * as projectAnnotations from "./preview"; // This is an important step to apply the right configuration when testing your stories. // More info at: https://storybook.js.org/docs/api/portable-stories/portable-stories-vitest#setprojectannotations -setProjectAnnotations([a11yAddonAnnotations, projectAnnotations]); \ No newline at end of file +setProjectAnnotations([a11yAddonAnnotations, projectAnnotations]); diff --git a/clients/web/package-lock.json b/clients/web/package-lock.json index 027df5286..18e0d71cd 100644 --- a/clients/web/package-lock.json +++ b/clients/web/package-lock.json @@ -69,7 +69,7 @@ "globals": "^17.4.0", "happy-dom": "^20.9.0", "playwright": "^1.58.2", - "prettier": "^3.8.4", + "prettier": "3.8.4", "storybook": "^10.2.19", "tsup": "^8.5.1", "typescript": "~5.9.3", diff --git a/clients/web/package.json b/clients/web/package.json index 3628f6d51..9be68f0a5 100644 --- a/clients/web/package.json +++ b/clients/web/package.json @@ -26,8 +26,8 @@ "test:integration:watch": "npm run test-servers:build && vitest --project=integration", "test-servers:build": "tsc -p ../../test-servers --noCheck", "lint": "eslint .", - "format": "prettier --write src server \"*.{ts,js}\"", - "format:check": "prettier --check src server \"*.{ts,js}\"", + "format": "prettier --write src server .storybook \"*.{ts,tsx,mts,cts,js,jsx,mjs,cjs}\"", + "format:check": "prettier --check src server .storybook \"*.{ts,tsx,mts,cts,js,jsx,mjs,cjs}\"", "preview": "vite preview", "storybook": "storybook dev -p 6006" }, @@ -91,7 +91,7 @@ "globals": "^17.4.0", "happy-dom": "^20.9.0", "playwright": "^1.58.2", - "prettier": "^3.8.1", + "prettier": "3.8.4", "storybook": "^10.2.19", "tsup": "^8.5.1", "typescript": "~5.9.3", diff --git a/clients/web/src/hooks/useImportClientConfig.test.tsx b/clients/web/src/hooks/useImportClientConfig.test.tsx new file mode 100644 index 000000000..9900f283e --- /dev/null +++ b/clients/web/src/hooks/useImportClientConfig.test.tsx @@ -0,0 +1,391 @@ +import { describe, it, expect, vi } from "vitest"; +import { renderHook, act } from "@testing-library/react"; +import { + useImportClientConfig, + type UseImportClientConfigOptions, +} from "./useImportClientConfig"; +import type { ImportSourceResult } from "@inspector/core/mcp/import/index.js"; +import type { MCPConfig } from "@inspector/core/mcp/types.js"; + +function config(...ids: string[]): MCPConfig { + const mcpServers: MCPConfig["mcpServers"] = {}; + for (const id of ids) mcpServers[id] = { command: "node", args: [id] }; + return { mcpServers }; +} + +function foundSource(cfg: MCPConfig): ImportSourceResult { + return { type: "claude", found: true, config: cfg, searched: ["/path"] }; +} + +function setup(over: Partial = {}) { + const onFetchSource = + over.onFetchSource ?? vi.fn(async () => foundSource(config("alpha"))); + const onAddServer = over.onAddServer ?? vi.fn(async () => {}); + const onUpdateServer = over.onUpdateServer ?? vi.fn(async () => {}); + const opts: UseImportClientConfigOptions = { + opened: true, + existingIds: [], + onFetchSource, + onAddServer, + onUpdateServer, + ...over, + }; + const view = renderHook( + (p: UseImportClientConfigOptions) => useImportClientConfig(p), + { initialProps: opts }, + ); + return { ...view, onFetchSource, onAddServer, onUpdateServer }; +} + +describe("useImportClientConfig", () => { + it("starts in the select phase with nothing to import", () => { + const { result } = setup(); + expect(result.current.phase).toBe("select"); + expect(result.current.plan).toBeNull(); + expect(result.current.canImport).toBe(false); + expect(result.current.importCount).toBe(0); + }); + + it("setSelectedType records the choice", () => { + const { result } = setup(); + act(() => result.current.setSelectedType("claude")); + expect(result.current.selectedType).toBe("claude"); + }); + + it("pickSource loads a config and moves to review", async () => { + const { result } = setup({ + onFetchSource: vi.fn(async () => foundSource(config("alpha", "beta"))), + }); + await act(async () => { + await result.current.pickSource("claude"); + }); + expect(result.current.phase).toBe("review"); + expect(result.current.plan?.additions).toHaveLength(2); + expect(result.current.importCount).toBe(2); + expect(result.current.canImport).toBe(true); + }); + + it("pickSource surfaces a fetch-reported error", async () => { + const { result } = setup({ + onFetchSource: vi.fn(async () => ({ + type: "claude", + found: false, + searched: [], + error: "backend blew up", + })), + }); + await act(async () => { + await result.current.pickSource("claude"); + }); + expect(result.current.phase).toBe("select"); + expect(result.current.error).toBe("backend blew up"); + }); + + it("pickSource shows a not-found notice", async () => { + const { result } = setup({ + onFetchSource: vi.fn(async () => ({ + type: "claude", + found: false, + searched: ["/a", "/b"], + })), + }); + await act(async () => { + await result.current.pickSource("claude"); + }); + expect(result.current.phase).toBe("select"); + expect(result.current.notice).toContain("No config found"); + }); + + it("pickSource handles an empty config as no servers", async () => { + const { result } = setup({ + onFetchSource: vi.fn(async () => foundSource(config())), + }); + await act(async () => { + await result.current.pickSource("claude"); + }); + expect(result.current.phase).toBe("select"); + expect(result.current.error).toContain("No servers found"); + }); + + it("pickSource surfaces a thrown fetch error", async () => { + const { result } = setup({ + onFetchSource: vi.fn(async () => { + throw new Error("network down"); + }), + }); + await act(async () => { + await result.current.pickSource("claude"); + }); + expect(result.current.phase).toBe("select"); + expect(result.current.error).toBe("network down"); + }); + + it("pickFile parses an uploaded config", async () => { + const { result } = setup(); + const raw = JSON.stringify({ mcpServers: { gamma: { command: "go" } } }); + const file = new File([raw], "claude.json"); + await act(async () => { + await result.current.pickFile(file); + }); + expect(result.current.phase).toBe("review"); + expect(result.current.plan?.additions[0].id).toBe("gamma"); + }); + + it("pickFile ignores a null file", async () => { + const { result } = setup(); + await act(async () => { + await result.current.pickFile(null); + }); + expect(result.current.phase).toBe("select"); + }); + + it("pickFile surfaces a parse error", async () => { + const { result } = setup(); + const file = new File(["{ not json"], "claude.json"); + await act(async () => { + await result.current.pickFile(file); + }); + expect(result.current.phase).toBe("select"); + expect(result.current.error).toBeTruthy(); + }); + + it("runImport adds new servers and reports outcomes", async () => { + const onAddServer = vi.fn(async () => {}); + const { result } = setup({ + onFetchSource: vi.fn(async () => foundSource(config("alpha", "beta"))), + onAddServer, + }); + await act(async () => { + await result.current.pickSource("claude"); + }); + await act(async () => { + await result.current.runImport(); + }); + expect(result.current.phase).toBe("summary"); + expect(onAddServer).toHaveBeenCalledTimes(2); + expect(result.current.outcomes.every((o) => o.status === "added")).toBe( + true, + ); + }); + + it("runImport skips additions opted out", async () => { + const onAddServer = vi.fn(async () => {}); + const { result } = setup({ + onFetchSource: vi.fn(async () => foundSource(config("alpha", "beta"))), + onAddServer, + }); + await act(async () => { + await result.current.pickSource("claude"); + }); + act(() => result.current.setAdditionAction("beta", "skip")); + expect(result.current.importCount).toBe(1); + await act(async () => { + await result.current.runImport(); + }); + expect(onAddServer).toHaveBeenCalledTimes(1); + expect(result.current.outcomes.find((o) => o.id === "beta")?.status).toBe( + "skipped", + ); + }); + + it("runImport reports an addition failure", async () => { + const onAddServer = vi.fn(async () => { + throw new Error("add failed"); + }); + const { result } = setup({ + onFetchSource: vi.fn(async () => foundSource(config("alpha"))), + onAddServer, + }); + await act(async () => { + await result.current.pickSource("claude"); + }); + await act(async () => { + await result.current.runImport(); + }); + const outcome = result.current.outcomes[0]; + expect(outcome.status).toBe("failed"); + expect(outcome.detail).toBe("add failed"); + }); + + it("resolves conflicts by overwrite, skip, and rename", async () => { + const onAddServer = vi.fn(async () => {}); + const onUpdateServer = vi.fn(async () => {}); + const { result } = setup({ + existingIds: ["alpha", "beta", "gamma"], + onFetchSource: vi.fn(async () => + foundSource(config("alpha", "beta", "gamma")), + ), + onAddServer, + onUpdateServer, + }); + await act(async () => { + await result.current.pickSource("claude"); + }); + expect(result.current.plan?.conflicts).toHaveLength(3); + // Default is skip for all three. + act(() => result.current.setResolution("alpha", "overwrite")); + act(() => result.current.setResolution("gamma", "rename")); + act(() => result.current.setRenameTo("gamma", "gamma-new")); + await act(async () => { + await result.current.runImport(); + }); + expect(onUpdateServer).toHaveBeenCalledWith( + "alpha", + "alpha", + expect.any(Object), + ); + expect(onAddServer).toHaveBeenCalledWith("gamma-new", expect.any(Object)); + const statuses = Object.fromEntries( + result.current.outcomes.map((o) => [o.id, o.status]), + ); + expect(statuses["alpha"]).toBe("overwritten"); + expect(statuses["beta"]).toBe("skipped"); + expect(statuses["gamma-new"]).toBe("renamed"); + }); + + it("flags invalid and colliding rename targets", async () => { + const { result } = setup({ + existingIds: ["alpha", "beta"], + onFetchSource: vi.fn(async () => foundSource(config("alpha", "beta"))), + }); + await act(async () => { + await result.current.pickSource("claude"); + }); + // Empty rename target. + act(() => result.current.setResolution("alpha", "rename")); + act(() => result.current.setRenameTo("alpha", " ")); + expect(result.current.renameErrors["alpha"]).toContain("required"); + // Syntactically invalid. + act(() => result.current.setRenameTo("alpha", "bad id!")); + expect(result.current.renameErrors["alpha"]).toContain("letters"); + // Collides with the other existing id. + act(() => result.current.setRenameTo("alpha", "beta")); + expect(result.current.renameErrors["alpha"]).toContain("already in use"); + expect(result.current.canImport).toBe(false); + }); + + it("flags a rename target that collides with another rename", async () => { + const { result } = setup({ + existingIds: ["alpha", "beta"], + onFetchSource: vi.fn(async () => foundSource(config("alpha", "beta"))), + }); + await act(async () => { + await result.current.pickSource("claude"); + }); + act(() => result.current.setResolution("alpha", "rename")); + act(() => result.current.setResolution("beta", "rename")); + act(() => result.current.setRenameTo("alpha", "shared")); + act(() => result.current.setRenameTo("beta", "shared")); + // The collision check is symmetric — each rename sees the other's target + // already claimed, so both must be flagged (asserting only one would pass + // even if the check regressed to one-directional). + expect(result.current.renameErrors["alpha"]).toContain("already in use"); + expect(result.current.renameErrors["beta"]).toContain("already in use"); + expect(result.current.canImport).toBe(false); + }); + + it("renames to the original id when the rename target is blank", async () => { + const onAddServer = vi.fn(async () => {}); + const { result } = setup({ + existingIds: ["alpha"], + onFetchSource: vi.fn(async () => foundSource(config("alpha"))), + onAddServer, + }); + await act(async () => { + await result.current.pickSource("claude"); + }); + act(() => result.current.setResolution("alpha", "rename")); + act(() => result.current.setRenameTo("alpha", " ")); + // A blank rename also sets renameErrors, so canImport is false and the real + // UI blocks submit here — this exercises the defensive fallback at + // useImportClientConfig.ts (`res.renameTo.trim() || conflict.id`) + // programmatically, which the UI can't reach. + await act(async () => { + await result.current.runImport(); + }); + // A blank rename target falls back to the original id. + expect(onAddServer).toHaveBeenCalledWith("alpha", expect.any(Object)); + expect(result.current.outcomes[0].status).toBe("renamed"); + }); + + it("back returns to the select phase", async () => { + const { result } = setup(); + await act(async () => { + await result.current.pickSource("claude"); + }); + expect(result.current.phase).toBe("review"); + act(() => result.current.back()); + expect(result.current.phase).toBe("select"); + }); + + it("resets when the modal is re-opened", async () => { + const { result, rerender } = setup({ opened: false }); + const props = (opened: boolean): UseImportClientConfigOptions => ({ + opened, + existingIds: [], + onFetchSource: vi.fn(async () => foundSource(config("alpha"))), + onAddServer: vi.fn(async () => {}), + onUpdateServer: vi.fn(async () => {}), + }); + rerender(props(true)); + await act(async () => { + await result.current.pickSource("claude"); + }); + expect(result.current.phase).toBe("review"); + rerender(props(false)); + rerender(props(true)); + expect(result.current.phase).toBe("select"); + expect(result.current.plan).toBeNull(); + }); + + it("stringifies a non-Error thrown by pickSource", async () => { + const { result } = setup({ + onFetchSource: vi.fn(async () => { + throw "plain fetch failure"; + }), + }); + await act(async () => { + await result.current.pickSource("claude"); + }); + expect(result.current.error).toBe("plain fetch failure"); + }); + + it("stringifies a non-Error thrown by pickFile", async () => { + const { result } = setup(); + const file = new File(["x"], "claude.json"); + vi.spyOn(file, "text").mockRejectedValue("plain read failure"); + await act(async () => { + await result.current.pickFile(file); + }); + expect(result.current.error).toBe("plain read failure"); + }); + + it("stringifies a non-Error thrown while applying an import", async () => { + const onAddServer = vi.fn(async () => { + throw "plain add failure"; + }); + const { result } = setup({ + onFetchSource: vi.fn(async () => foundSource(config("alpha"))), + onAddServer, + }); + await act(async () => { + await result.current.pickSource("claude"); + }); + await act(async () => { + await result.current.runImport(); + }); + expect(result.current.outcomes[0]).toMatchObject({ + status: "failed", + detail: "plain add failure", + }); + }); + + it("runImport with no plan is a no-op", async () => { + const { result } = setup(); + await act(async () => { + await result.current.runImport(); + }); + expect(result.current.phase).toBe("select"); + expect(result.current.outcomes).toEqual([]); + }); +}); diff --git a/clients/web/src/hooks/useServerJsonImport.test.tsx b/clients/web/src/hooks/useServerJsonImport.test.tsx new file mode 100644 index 000000000..a310a2d84 --- /dev/null +++ b/clients/web/src/hooks/useServerJsonImport.test.tsx @@ -0,0 +1,257 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { renderHook, act } from "@testing-library/react"; +import { + useServerJsonImport, + VALIDATE_DEBOUNCE_MS, + COLLAPSE_DELAY_MS, + HIGHLIGHT_DURATION_MS, + type UseServerJsonImportOptions, +} from "./useServerJsonImport"; + +// A valid registry server.json with two runnable options; option 0 declares a +// required env var (with no default) and an optional one (with a default). +const VALID_SERVER_JSON = JSON.stringify({ + name: "io.github.acme/weather", + packages: [ + { + registryType: "npm", + identifier: "weather-mcp", + environmentVariables: [ + { name: "API_KEY", isRequired: true, description: "The key" }, + { name: "REGION", isRequired: false, default: "us" }, + ], + }, + { registryType: "pypi", identifier: "weather-py" }, + ], +}); + +function setup(over: Partial = {}) { + const onAddServer = over.onAddServer ?? vi.fn(async () => {}); + const opts: UseServerJsonImportOptions = { + opened: true, + existingIds: [], + onAddServer, + ...over, + }; + const view = renderHook( + (p: UseServerJsonImportOptions) => useServerJsonImport(p), + { initialProps: opts }, + ); + return { ...view, onAddServer }; +} + +/** Set the textarea content and flush the validation debounce. */ +function typeAndFlush( + result: { current: ReturnType }, + content: string, +) { + act(() => result.current.setRawText(content)); + act(() => { + vi.advanceTimersByTime(VALIDATE_DEBOUNCE_MS); + }); +} + +describe("useServerJsonImport", () => { + beforeEach(() => { + vi.useFakeTimers(); + }); + afterEach(() => { + vi.runOnlyPendingTimers(); + vi.useRealTimers(); + }); + + it("starts empty with an info prompt and the Add button disabled", () => { + const { result } = setup(); + expect(result.current.draft.rawText).toBe(""); + expect(result.current.canAdd).toBe(false); + expect(result.current.validation).toEqual([ + { type: "info", message: "Paste server.json content to validate." }, + ]); + expect(result.current.packages).toBeUndefined(); + expect(result.current.envVars).toEqual([]); + }); + + it("validates pasted content and enables Add", () => { + const { result } = setup(); + typeAndFlush(result, VALID_SERVER_JSON); + expect(result.current.canAdd).toBe(true); + expect(result.current.defaultServerName).toBe("weather"); + expect(result.current.packages).toHaveLength(2); + expect(result.current.validation[0]).toMatchObject({ type: "success" }); + expect(result.current.validation[1]).toMatchObject({ type: "info" }); + // Env vars of the first option, with the optional one's default prefilled. + expect(result.current.envVars).toEqual([ + { name: "API_KEY", description: "The key", required: true, value: "" }, + { name: "REGION", description: undefined, required: false, value: "us" }, + ]); + }); + + it("surfaces a parse error for invalid JSON", () => { + const { result } = setup(); + typeAndFlush(result, "{ not json"); + expect(result.current.canAdd).toBe(false); + expect(result.current.validation[0].type).toBe("error"); + }); + + it("flags an invalid overridden id", () => { + const { result } = setup(); + typeAndFlush(result, VALID_SERVER_JSON); + act(() => result.current.setServerName("bad id!")); + expect(result.current.canAdd).toBe(false); + expect( + result.current.validation.some( + (v) => v.type === "error" && v.message.includes("Server id"), + ), + ).toBe(true); + }); + + it("warns on a duplicate id", () => { + const { result } = setup({ existingIds: ["weather"] }); + typeAndFlush(result, VALID_SERVER_JSON); + expect(result.current.canAdd).toBe(false); + expect( + result.current.validation.some( + (v) => v.type === "warning" && v.message.includes("already exists"), + ), + ).toBe(true); + }); + + it("selectPackage switches the active option and its env vars", () => { + const { result } = setup(); + typeAndFlush(result, VALID_SERVER_JSON); + act(() => result.current.selectPackage(1)); + expect(result.current.draft.selectedPackageIndex).toBe(1); + // The pypi option declares no env vars. + expect(result.current.envVars).toEqual([]); + }); + + it("setEnvVar overrides a declared env value", () => { + const { result } = setup(); + typeAndFlush(result, VALID_SERVER_JSON); + act(() => result.current.setEnvVar("API_KEY", "secret")); + expect( + result.current.envVars.find((v) => v.name === "API_KEY")?.value, + ).toBe("secret"); + }); + + it("re-opens the File Contents disclosure when cleared", () => { + const { result } = setup(); + typeAndFlush(result, VALID_SERVER_JSON); + act(() => result.current.setFileContentsOpen(false)); + expect(result.current.fileContentsOpen).toBe(false); + typeAndFlush(result, ""); + expect(result.current.fileContentsOpen).toBe(true); + }); + + it("flashes then auto-collapses the disclosure after content loads", () => { + const { result } = setup(); + act(() => result.current.setRawText(VALID_SERVER_JSON)); + expect(result.current.fileContentsOpen).toBe(true); + act(() => { + vi.advanceTimersByTime(COLLAPSE_DELAY_MS); + }); + expect(result.current.fileContentsHighlight).toBe(true); + act(() => { + vi.advanceTimersByTime(HIGHLIGHT_DURATION_MS); + }); + expect(result.current.fileContentsHighlight).toBe(false); + expect(result.current.fileContentsOpen).toBe(false); + }); + + it("resets its state when the modal is re-opened", () => { + const { result, rerender } = setup({ opened: false }); + // Open, dirty the draft, then close and re-open — the draft must clear. + rerender({ opened: true, existingIds: [], onAddServer: vi.fn() }); + typeAndFlush(result, VALID_SERVER_JSON); + expect(result.current.draft.rawText).not.toBe(""); + rerender({ opened: false, existingIds: [], onAddServer: vi.fn() }); + rerender({ opened: true, existingIds: [], onAddServer: vi.fn() }); + expect(result.current.draft.rawText).toBe(""); + }); + + it("pickFile reads a file into the draft", async () => { + const { result } = setup(); + const file = new File([VALID_SERVER_JSON], "server.json"); + await act(async () => { + await result.current.pickFile(file); + }); + expect(result.current.draft.rawText).toBe(VALID_SERVER_JSON); + }); + + it("pickFile ignores a null file", async () => { + const { result } = setup(); + await act(async () => { + await result.current.pickFile(null); + }); + expect(result.current.draft.rawText).toBe(""); + }); + + it("pickFile surfaces a read error", async () => { + const { result } = setup(); + const file = new File(["x"], "server.json"); + vi.spyOn(file, "text").mockRejectedValue(new Error("read boom")); + await act(async () => { + await result.current.pickFile(file); + }); + expect( + result.current.validation.some((v) => v.message.includes("read boom")), + ).toBe(true); + }); + + it("submit persists the selected server and resolves true", async () => { + const onAddServer = vi.fn(async () => {}); + const { result } = setup({ onAddServer }); + typeAndFlush(result, VALID_SERVER_JSON); + let ok = false; + await act(async () => { + ok = await result.current.submit(); + }); + expect(ok).toBe(true); + expect(onAddServer).toHaveBeenCalledWith("weather", expect.any(Object)); + }); + + it("submit refuses invalid content", async () => { + const onAddServer = vi.fn(async () => {}); + const { result } = setup({ onAddServer }); + typeAndFlush(result, "{ not json"); + let ok = true; + await act(async () => { + ok = await result.current.submit(); + }); + expect(ok).toBe(false); + expect(onAddServer).not.toHaveBeenCalled(); + expect( + result.current.validation.some((v) => + v.message.includes("Fix the validation errors"), + ), + ).toBe(true); + }); + + it("submit refuses a duplicate id", async () => { + const onAddServer = vi.fn(async () => {}); + const { result } = setup({ existingIds: ["weather"], onAddServer }); + typeAndFlush(result, VALID_SERVER_JSON); + let ok = true; + await act(async () => { + ok = await result.current.submit(); + }); + expect(ok).toBe(false); + expect(onAddServer).not.toHaveBeenCalled(); + }); + + it("submit surfaces an onAddServer failure", async () => { + const onAddServer = vi.fn(async () => { + throw new Error("save boom"); + }); + const { result } = setup({ onAddServer }); + typeAndFlush(result, VALID_SERVER_JSON); + let ok = true; + await act(async () => { + ok = await result.current.submit(); + }); + expect(ok).toBe(false); + expect( + result.current.validation.some((v) => v.message.includes("save boom")), + ).toBe(true); + }); +}); diff --git a/clients/web/src/hooks/useServerJsonImport.ts b/clients/web/src/hooks/useServerJsonImport.ts index 6736ec6cf..5692140e3 100644 --- a/clients/web/src/hooks/useServerJsonImport.ts +++ b/clients/web/src/hooks/useServerJsonImport.ts @@ -16,19 +16,19 @@ import type { } from "../components/groups/ImportServerJsonPanel/ImportServerJsonPanel"; /** Debounce (ms) before a textarea edit re-triggers parse/validation. */ -const VALIDATE_DEBOUNCE_MS = 300; +export const VALIDATE_DEBOUNCE_MS = 300; /** * Delay (ms) after content is loaded/pasted before the File Contents disclosure * auto-collapses — long enough to read as "the content was accepted". */ -const COLLAPSE_DELAY_MS = 1000; +export const COLLAPSE_DELAY_MS = 1000; /** * How long (ms) to flash the disclosure with its hover background just before it * collapses, so the collapse reads as intentional rather than abrupt. */ -const HIGHLIGHT_DURATION_MS = 250; +export const HIGHLIGHT_DURATION_MS = 250; const EMPTY_DRAFT: InspectorServerJsonDraft = { rawText: "", diff --git a/clients/web/src/theme/Paper.test.tsx b/clients/web/src/theme/Paper.test.tsx new file mode 100644 index 000000000..e1ccd19d7 --- /dev/null +++ b/clients/web/src/theme/Paper.test.tsx @@ -0,0 +1,34 @@ +import { describe, it, expect } from "vitest"; +import { Paper } from "@mantine/core"; +import { renderWithMantine } from "../test/renderWithMantine"; + +// The theme's ThemePaper.extend is applied transitively when components render a +// Paper, but no single component exercises every variant — the `code`, +// `contained`, and `panel` branches of its classNames/styles callbacks were +// uncovered. Render each variant (plus the default) under the project theme so +// both callbacks run for every branch. See #1787 (theme/** brought under the +// coverage gate). + +const VARIANTS = ["code", "contained", "panel"] as const; + +describe("ThemePaper variants", () => { + it("renders the default variant (no variant-specific styles)", () => { + const { getByText } = renderWithMantine(default); + expect(getByText("default")).toBeTruthy(); + }); + + for (const variant of VARIANTS) { + it(`renders the "${variant}" variant`, () => { + const { getByText } = renderWithMantine( + {variant}, + ); + expect(getByText(variant)).toBeTruthy(); + }); + } + + it('applies the paper-code class only to the "code" variant', () => { + const { getByText } = renderWithMantine(c); + // classNames returns { root: "paper-code" } for the code variant. + expect(getByText("c").className).toContain("paper-code"); + }); +}); diff --git a/clients/web/src/types/navigation.ts b/clients/web/src/types/navigation.ts deleted file mode 100644 index e60781015..000000000 --- a/clients/web/src/types/navigation.ts +++ /dev/null @@ -1,8 +0,0 @@ -export type InspectorTab = - | "tools" - | "apps" - | "prompts" - | "resources" - | "logs" - | "tasks" - | "protocol"; diff --git a/clients/web/vite.config.ts b/clients/web/vite.config.ts index 21cc39af6..3df74bd13 100644 --- a/clients/web/vite.config.ts +++ b/clients/web/vite.config.ts @@ -167,8 +167,20 @@ export default defineConfig(({ command }) => { coverage: { provider: "v8", reporter: ["text", "html", "json-summary"], + // Whitelist of gated directories. Deliberate top-level-file omissions + // (every src *directory* below is gated): + // • `src/App.tsx` — a ~4.5k-line composition root at ~42% branch + // coverage; gating it is a dedicated testing/decomposition effort, + // not a whitelist tweak. + // • `src/main.tsx` / `src/index.ts` — the browser and bin bootstraps + // (createRoot render / `runWeb` re-export), the analog of + // clients/cli's excluded `src/index.ts`. + // These omissions are intentional and documented here (and in AGENTS.md) + // rather than silent. Add new gated dirs here as they appear. include: [ "src/components/**/*.{ts,tsx}", + "src/hooks/**/*.{ts,tsx}", + "src/theme/**/*.{ts,tsx}", "src/lib/**/*.{ts,tsx}", "src/utils/**/*.{ts,tsx}", "clients/web/server/**/*.{ts,tsx}", diff --git a/eslint.config.js b/eslint.config.js index 4db86dbe0..dfa945d1b 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -3,13 +3,33 @@ import globals from "globals"; import tseslint from "typescript-eslint"; import { defineConfig, globalIgnores } from "eslint/config"; -// Root-level lint gate for the shared `core/` package. Each client's own -// `eslint .` is scoped to its own directory, so nothing reached `core/` before -// (#1689) — this config closes that gap. `core/` is isomorphic TypeScript -// (browser-side OAuth + Node backends + shared runtime), so both browser and -// Node globals apply; there is no JSX in `core/`, so no React plugin is needed. +// Root-level lint gate for first-party code that no client's own `eslint .` +// (each scoped to its own directory) reaches: the shared `core/` package +// (#1689) and the root-owned "shared" surface — `test-servers/src/**`, the +// root `vitest.shared.mts`, and this config file itself (#1767). `core/` is +// isomorphic TypeScript (browser-side OAuth + Node backends + shared runtime); +// the shared surface is Node-only tooling/tests. Neither has JSX, so no React +// plugin is needed. +const sharedRules = { + // An `_`-prefix is the explicit "intentionally unused" marker — + // interface-conformance params in fakes, destructuring-rest omissions, + // and reserved-for-later args. Honor it rather than deleting signal. + "@typescript-eslint/no-unused-vars": [ + "error", + { + argsIgnorePattern: "^_", + varsIgnorePattern: "^_", + caughtErrorsIgnorePattern: "^_", + }, + ], +}; + export default defineConfig([ - globalIgnores(["core/**/build/**", "core/**/dist/**"]), + globalIgnores([ + "core/**/build/**", + "core/**/dist/**", + "test-servers/build/**", + ]), { files: ["core/**/*.{ts,tsx}"], extends: [js.configs.recommended, tseslint.configs.recommended], @@ -21,18 +41,22 @@ export default defineConfig([ ...globals.browser, }, }, - rules: { - // An `_`-prefix is `core/`'s explicit "intentionally unused" marker — - // interface-conformance params in fakes, destructuring-rest omissions, - // and reserved-for-later args. Honor it rather than deleting signal. - "@typescript-eslint/no-unused-vars": [ - "error", - { - argsIgnorePattern: "^_", - varsIgnorePattern: "^_", - caughtErrorsIgnorePattern: "^_", - }, - ], + rules: sharedRules, + }, + { + files: [ + "test-servers/src/**/*.{ts,tsx,mts,cts}", + "vitest.shared.mts", + "eslint.config.js", + ], + extends: [js.configs.recommended, tseslint.configs.recommended], + languageOptions: { + ecmaVersion: 2022, + sourceType: "module", + globals: { + ...globals.node, + }, }, + rules: sharedRules, }, ]); diff --git a/package-lock.json b/package-lock.json index 22e5af663..903dd105e 100644 --- a/package-lock.json +++ b/package-lock.json @@ -41,7 +41,7 @@ "@eslint/js": "^9.39.5", "eslint": "^9.39.5", "globals": "^17.7.0", - "prettier": "^3.8.4", + "prettier": "3.8.4", "typescript": "~5.9.3", "typescript-eslint": "^8.65.0" }, diff --git a/package.json b/package.json index afd8037d3..dcd710d5f 100644 --- a/package.json +++ b/package.json @@ -40,14 +40,18 @@ "ci": "npm run validate && npm run coverage && npm run verify:build-gate && npm run smoke && npm run ci:storybook", "ci:storybook": "cd clients/web && npx playwright install chromium && npm run test:storybook", "verify:build-gate": "node scripts/verify-build-gate.mjs", - "validate": "npm run validate:core && npm run validate:web && npm run validate:cli && npm run validate:tui && npm run validate:launcher", - "validate:core": "npm run format:check:core && npm run format:check:scripts && npm run lint:core", + "validate": "npm run verify:format-coverage && npm run validate:core && npm run validate:web && npm run validate:cli && npm run validate:tui && npm run validate:launcher", + "verify:format-coverage": "node scripts/verify-format-coverage.mjs", + "validate:core": "npm run format:check:core && npm run format:check:scripts && npm run format:check:shared && npm run lint:core && npm run lint:shared", "lint:core": "eslint \"core/**/*.{ts,tsx}\"", + "lint:shared": "eslint \"test-servers/src/**/*.{ts,tsx,mts,cts}\" vitest.shared.mts eslint.config.js", "format:core": "prettier --write \"core/**/*.{ts,tsx}\"", "format:check:core": "prettier --check \"core/**/*.{ts,tsx}\"", - "format:scripts": "prettier --write \"scripts/**/*.{mjs,js,cjs,ts}\"", - "format:check:scripts": "prettier --check \"scripts/**/*.{mjs,js,cjs,ts}\"", - "format": "npm run format:core && npm run format:scripts && cd clients/web && npm run format && cd ../cli && npm run format && cd ../tui && npm run format && cd ../launcher && npm run format", + "format:scripts": "prettier --write \"scripts/**/*.{ts,tsx,mts,cts,js,jsx,mjs,cjs}\"", + "format:check:scripts": "prettier --check \"scripts/**/*.{ts,tsx,mts,cts,js,jsx,mjs,cjs}\"", + "format:shared": "prettier --write \"test-servers/src/**/*.{ts,tsx,mts,cts}\" vitest.shared.mts eslint.config.js", + "format:check:shared": "prettier --check \"test-servers/src/**/*.{ts,tsx,mts,cts}\" vitest.shared.mts eslint.config.js", + "format": "npm run format:core && npm run format:scripts && npm run format:shared && cd clients/web && npm run format && cd ../cli && npm run format && cd ../tui && npm run format && cd ../launcher && npm run format", "validate:cli": "cd clients/cli && npm run validate", "validate:tui": "cd clients/tui && npm run validate", "validate:web": "cd clients/web && npm run validate", @@ -102,7 +106,7 @@ "@eslint/js": "^9.39.5", "eslint": "^9.39.5", "globals": "^17.7.0", - "prettier": "^3.8.1", + "prettier": "3.8.4", "typescript": "~5.9.3", "typescript-eslint": "^8.65.0" } diff --git a/scripts/verify-format-coverage.mjs b/scripts/verify-format-coverage.mjs new file mode 100644 index 000000000..13fca8299 --- /dev/null +++ b/scripts/verify-format-coverage.mjs @@ -0,0 +1,237 @@ +#!/usr/bin/env node +// Durable guard for the "every first-party source file is format-gated" invariant +// that #1789 established by a one-shot manual audit (PR #1792). A prettier +// `format`/`format:check` glob that stops covering a file fails silently — the +// file is simply skipped, not reported — so a nested dir (`.storybook/`, a +// client `scripts/`) or a new extension can re-open the gap unnoticed. This is +// the format-coverage analog of `verify:build-gate`: it enumerates every tracked +// source file and asserts each is matched by at least one `prettier --check` +// glob declared across the repo's `package.json`s. Exits non-zero, listing the +// offenders, on any miss. +// +// Source of truth is the `format:check*` scripts themselves — this parser reads +// the globs out of them, so widening/narrowing a glob is reflected here with no +// second list to keep in sync. + +import { readFileSync } from "node:fs"; +import { execFileSync } from "node:child_process"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; + +const repoRoot = path.resolve( + path.dirname(fileURLToPath(import.meta.url)), + "..", +); + +// File extensions prettier formats as first-party source here. Kept in sync with +// the union of the format globs; a file with one of these extensions that no +// glob matches is the failure this guard exists to catch. +const SOURCE_EXTENSIONS = [ + "ts", + "tsx", + "mts", + "cts", + "js", + "jsx", + "mjs", + "cjs", +]; + +// The `package.json`s whose `format:check*` scripts define the gate. The root +// carries the split `format:check:core|scripts|shared`; each client carries a +// single `format:check`. Paths are relative to the repo root; each glob resolves +// relative to its manifest's directory (prettier's cwd for that script). +const MANIFESTS = [ + ".", + "clients/web", + "clients/cli", + "clients/tui", + "clients/launcher", +]; + +/** Split a shell-ish command string into args, honoring double quotes. */ +function tokenize(command) { + const tokens = []; + const re = /"([^"]*)"|(\S+)/g; + let m; + while ((m = re.exec(command)) !== null) { + tokens.push(m[1] !== undefined ? m[1] : m[2]); + } + return tokens; +} + +/** + * Names of scripts transitively reachable from `entry` by following `npm run + * ` references within a manifest. Used so we only trust a `format:check` + * glob that CI actually runs — a `prettier --check` script that nothing invokes + * from `validate` doesn't gate anything, and counting its globs would let the + * gate be silently unwired (the file still "matches a glob" that never runs). + */ +function reachableScripts(scripts, entry = "validate") { + const reached = new Set(); + const queue = [entry]; + const runRef = /npm run ([\w:-]+)/g; + while (queue.length > 0) { + const name = queue.shift(); + if (reached.has(name)) continue; + reached.add(name); + const cmd = scripts?.[name]; + if (typeof cmd !== "string") continue; + for (const m of cmd.matchAll(runRef)) queue.push(m[1]); + } + return reached; +} + +/** + * Extract the path/glob args from every `prettier --check …` in a manifest's + * scripts that is reachable from `validate`. Restricting to reachable scripts is + * what makes the guard assert "this file is checked by CI", not merely "some + * glob covers it". + */ +function prettierCheckArgs(scripts) { + const reachable = reachableScripts(scripts); + const args = []; + for (const [name, value] of Object.entries(scripts ?? {})) { + if (!reachable.has(name)) continue; + if (typeof value !== "string" || !value.includes("prettier --check")) + continue; + // A manifest may chain `prettier --check …` inside a larger script; take the + // segment starting at each occurrence up to the next `&&`/`||`/`;`. + for (const segment of value.split(/&&|\|\||;/)) { + const trimmed = segment.trim(); + if (!trimmed.startsWith("prettier --check")) continue; + const tokens = tokenize(trimmed).slice(2); // drop `prettier` `--check` + for (const t of tokens) { + if (t.startsWith("-")) continue; // a flag, not a path + args.push(t); + } + } + } + return args; +} + +const GLOB_CHARS = /[*?{}[\]]/; + +/** Convert a prettier glob to an anchored RegExp over repo-relative POSIX paths. */ +function globToRegExp(glob) { + let re = ""; + for (let i = 0; i < glob.length; i++) { + const c = glob[i]; + if (c === "*") { + if (glob[i + 1] === "*") { + // `**` (optionally `**/`) crosses path separators. + if (glob[i + 2] === "/") { + re += "(?:.*/)?"; + i += 2; + } else { + re += ".*"; + i += 1; + } + } else { + re += "[^/]*"; // `*` stays within a segment + } + } else if (c === "?") { + re += "[^/]"; + } else if (c === "{") { + re += "(?:"; + } else if (c === "}") { + re += ")"; + } else if (c === ",") { + re += "|"; + } else if (".+^$()|\\/".includes(c)) { + re += "\\" + c; + } else { + re += c; + } + } + return new RegExp("^" + re + "$"); +} + +/** + * Assert the root `validate` chain actually invokes each non-root manifest's + * `validate` (via `cd && npm run validate`). Without this, a client's + * globs would still be harvested from its own `validate` and count as coverage + * even if the root chain stopped running that client — the same "gate silently + * stops gating" failure as the reachable-script check, one level up. Returns the + * list of manifest dirs the root chain does NOT reach. + */ +function clientsUnreachedFromRoot() { + const rootPkg = JSON.parse( + readFileSync(path.join(repoRoot, "package.json"), "utf8"), + ); + const reachedNames = reachableScripts(rootPkg.scripts); + const reachedCommands = [...reachedNames] + .map((n) => rootPkg.scripts?.[n]) + .filter((c) => typeof c === "string"); + return MANIFESTS.filter((dir) => dir !== ".").filter( + (dir) => + !reachedCommands.some( + (c) => c.includes(`cd ${dir}`) && /npm run validate/.test(c), + ), + ); +} + +/** Build the set of coverage predicates from all manifests' format globs. */ +function buildMatchers() { + const matchers = []; + for (const manifestDir of MANIFESTS) { + const manifestPath = path.join(repoRoot, manifestDir, "package.json"); + const pkg = JSON.parse(readFileSync(manifestPath, "utf8")); + for (const arg of prettierCheckArgs(pkg.scripts)) { + const rel = manifestDir === "." ? arg : path.posix.join(manifestDir, arg); + if (GLOB_CHARS.test(arg)) { + const re = globToRegExp(rel); + matchers.push((f) => re.test(f)); + } else { + // A bare path: a directory prettier recurses into, or an exact file. + matchers.push((f) => f === rel || f.startsWith(rel + "/")); + } + } + } + return matchers; +} + +function trackedSourceFiles() { + const out = execFileSync( + "git", + ["ls-files", ...SOURCE_EXTENSIONS.map((e) => `*.${e}`)], + { cwd: repoRoot, encoding: "utf8" }, + ); + return out.split("\n").filter(Boolean); +} + +const unreachedClients = clientsUnreachedFromRoot(); +if (unreachedClients.length > 0) { + console.error( + `verify:format-coverage — the root \`validate\` chain does not invoke ${unreachedClients.length} client validation(s):\n`, + ); + for (const dir of unreachedClients) + console.error(` ${dir} (expected \`cd ${dir} && npm run validate\`)`); + console.error( + "\nA client whose `validate` the root chain never runs is not format-gated by CI,", + ); + console.error( + "even though its globs exist. Restore the `validate:` link in the root `validate`.", + ); + process.exit(1); +} + +const matchers = buildMatchers(); +const files = trackedSourceFiles(); +const ungated = files.filter((f) => !matchers.some((m) => m(f))); + +if (ungated.length > 0) { + console.error( + `verify:format-coverage — ${ungated.length} tracked source file(s) are not covered by any prettier format glob:\n`, + ); + for (const f of ungated) console.error(" " + f); + console.error( + "\nAdd the file's directory (or a matching glob) to the relevant `format`/`format:check` script,", + ); + console.error("or widen the extension set. See AGENTS.md."); + process.exit(1); +} + +console.log( + `verify:format-coverage — OK: all ${files.length} tracked source files are format-gated.`, +); diff --git a/specification/v2_ux_interfaces_plan.md b/specification/v2_ux_interfaces_plan.md index 9041de277..c9dd7431f 100644 --- a/specification/v2_ux_interfaces_plan.md +++ b/specification/v2_ux_interfaces_plan.md @@ -82,7 +82,7 @@ and should be replaced verbatim during the refactor: | `InspectorServerSettings` | Closest v1.5 analog is `InspectorClientOptions` (timeouts, OAuth, sampling/elicit/roots flags). Settings form should accept the relevant subset. | `core/mcp/types.ts` | | `InspectorRequestHistoryItem` | Subset of `MessageEntry` filtered to outbound requests. No new type needed. | `core/mcp/types.ts` | | `InspectorUrlElicitRequest` | URL-mode elicitation IS supported by v1.5 via `InspectorClientOptions.elicit: { url: true }`; the request/response shape lives in `core/mcp/elicitationCreateMessage.ts`. v2 should mirror that file's types rather than invent. | `core/mcp/elicitationCreateMessage.ts` | -| `InspectorTab` | No v1.5 analog — v2-only enum. Place under `clients/web/src/types/` or alongside `ViewHeader`. | new (v2-only) | +| `InspectorTab` | No v1.5 analog — v2-only enum. Superseded by `InspectorTabId` in `clients/web/src/utils/inspectorTabs.ts` (#1785). | new (v2-only) | ## Non-goals @@ -181,7 +181,7 @@ have no v1.5 equivalent): **Tab enum placement**: `InspectorTab` is a UI-routing concept, not an MCP or transport concept. Place it under `clients/web/src/types/navigation.ts` rather than in `core/mcp/types.ts`. Everything else lives in `core/mcp/types.ts`. -_(Superseded by [#1785](https://github.com/modelcontextprotocol/inspector/issues/1785): the live tab-id type is `InspectorTabId` in `clients/web/src/utils/inspectorTabs.ts`, and the `navigation.ts` `InspectorTab` here is dead and slated for removal — a pure UI domain type belongs in `utils/`, per the `src/lib` vs `src/utils` rule in AGENTS.md.)_ +_(Superseded by [#1785](https://github.com/modelcontextprotocol/inspector/issues/1785): the live tab-id type is `InspectorTabId` in `clients/web/src/utils/inspectorTabs.ts`, and the dead `navigation.ts` `InspectorTab` was removed — a pure UI domain type belongs in `utils/`, not `src/types/`, per the `src/lib` vs `src/utils` rule in AGENTS.md.)_ **Custom headers**: `clients/web/src/utils/customHeaders.ts` is a verbatim copy of the v1.5 module. It owns `CustomHeader` / `CustomHeaders` shape used by diff --git a/test-servers/src/modern-tasks.ts b/test-servers/src/modern-tasks.ts index bc7a6d773..15531921e 100644 --- a/test-servers/src/modern-tasks.ts +++ b/test-servers/src/modern-tasks.ts @@ -41,7 +41,11 @@ const DEFAULT_POLL_INTERVAL_MS = 500; const SIMPLE_WORKING_POLLS = 2; type ModernTaskStatus = - "working" | "input_required" | "completed" | "failed" | "cancelled"; + | "working" + | "input_required" + | "completed" + | "failed" + | "cancelled"; interface ModernTaskEntry { taskId: string; diff --git a/test-servers/src/test-server-fixtures.ts b/test-servers/src/test-server-fixtures.ts index fff1d5652..a87311efa 100644 --- a/test-servers/src/test-server-fixtures.ts +++ b/test-servers/src/test-server-fixtures.ts @@ -1575,7 +1575,8 @@ export function createAddToolTool(): ToolDefinition { { description: params.description as string, inputSchema: params.inputSchema as - Record | undefined, + | Record + | undefined, }, async () => { return { @@ -1679,7 +1680,8 @@ export function createAddPromptTool(): ToolDefinition { { description: params.description as string | undefined, argsSchema: params.argsSchema as - Record | undefined, + | Record + | undefined, }, async () => { return { @@ -1823,7 +1825,9 @@ export function createSendProgressTool( // Extract progressToken from metadata const progressToken = extra?._meta?.progressToken as - string | number | undefined; + | string + | number + | undefined; // Send progress notifications let sent = 0; @@ -2264,9 +2268,12 @@ export function createTaskTool( handler: { createTask: async (args, extra) => { const message = (args as Record)?.message as - string | undefined; + | string + | undefined; const progressToken = extra._meta?.progressToken as - string | number | undefined; + | string + | number + | undefined; const task = await extra.taskStore.createTask({}); runTaskExecution({ task, diff --git a/test-servers/src/test-server-http.ts b/test-servers/src/test-server-http.ts index 692c8b4fb..bbbb47cef 100644 --- a/test-servers/src/test-server-http.ts +++ b/test-servers/src/test-server-http.ts @@ -210,7 +210,8 @@ export class TestServerHttp { private recordedRequests: RecordedRequest[] = []; private httpServer?: HttpServer; private transport?: - WebStandardStreamableHTTPServerTransport | SSEServerTransport; + | WebStandardStreamableHTTPServerTransport + | SSEServerTransport; private baseUrl?: string; private currentRequestHeaders?: Record; private currentLogLevel: string | null = null; diff --git a/vitest.shared.mts b/vitest.shared.mts index f0c25d724..060e099f2 100644 --- a/vitest.shared.mts +++ b/vitest.shared.mts @@ -4,82 +4,112 @@ * that client's node_modules. */ -import path from 'node:path'; -import { fileURLToPath } from 'node:url'; +import path from "node:path"; +import { fileURLToPath } from "node:url"; export function vitestSharedPaths(clientDir: string) { const dirname = path.resolve(clientDir); - const repoRoot = path.resolve(dirname, '../..'); + const repoRoot = path.resolve(dirname, "../.."); const sharedAliases = { - '@inspector/core': path.resolve(repoRoot, 'core'), - '@modelcontextprotocol/inspector-test-server': path.resolve( + "@inspector/core": path.resolve(repoRoot, "core"), + "@modelcontextprotocol/inspector-test-server": path.resolve( repoRoot, - 'test-servers/build/index.js', + "test-servers/build/index.js", ), }; const sharedDedupe = [ - 'react', - 'react-dom', - '@modelcontextprotocol/client', - '@modelcontextprotocol/core', + "react", + "react-dom", + "@modelcontextprotocol/client", + "@modelcontextprotocol/core", ]; const nodeModulesAliases = [ - { find: /^react$/, replacement: path.resolve(dirname, 'node_modules/react') }, + { + find: /^react$/, + replacement: path.resolve(dirname, "node_modules/react"), + }, { find: /^react\/jsx-runtime$/, - replacement: path.resolve(dirname, 'node_modules/react/jsx-runtime.js'), + replacement: path.resolve(dirname, "node_modules/react/jsx-runtime.js"), }, { find: /^react\/jsx-dev-runtime$/, - replacement: path.resolve(dirname, 'node_modules/react/jsx-dev-runtime.js'), + replacement: path.resolve( + dirname, + "node_modules/react/jsx-dev-runtime.js", + ), }, { find: /^react-dom$/, - replacement: path.resolve(dirname, 'node_modules/react-dom'), + replacement: path.resolve(dirname, "node_modules/react-dom"), }, { find: /^react-dom\/client$/, - replacement: path.resolve(dirname, 'node_modules/react-dom/client.js'), + replacement: path.resolve(dirname, "node_modules/react-dom/client.js"), }, - { find: /^pino$/, replacement: path.resolve(dirname, 'node_modules/pino') }, + { find: /^pino$/, replacement: path.resolve(dirname, "node_modules/pino") }, { find: /^pino\/browser\.js$/, - replacement: path.resolve(dirname, 'node_modules/pino/browser.js'), + replacement: path.resolve(dirname, "node_modules/pino/browser.js"), }, { find: /^hono$/, - replacement: path.resolve(dirname, 'node_modules/hono/dist/index.js'), + replacement: path.resolve(dirname, "node_modules/hono/dist/index.js"), }, { find: /^hono\/streaming$/, - replacement: path.resolve(dirname, 'node_modules/hono/dist/helper/streaming/index.js'), + replacement: path.resolve( + dirname, + "node_modules/hono/dist/helper/streaming/index.js", + ), }, { find: /^@hono\/node-server$/, - replacement: path.resolve(dirname, 'node_modules/@hono/node-server'), + replacement: path.resolve(dirname, "node_modules/@hono/node-server"), + }, + { + find: /^atomically$/, + replacement: path.resolve(dirname, "node_modules/atomically"), + }, + { + find: /^chokidar$/, + replacement: path.resolve(dirname, "node_modules/chokidar"), }, - { find: /^atomically$/, replacement: path.resolve(dirname, 'node_modules/atomically') }, - { find: /^chokidar$/, replacement: path.resolve(dirname, 'node_modules/chokidar') }, { find: /^@napi-rs\/keyring$/, - replacement: path.resolve(dirname, 'node_modules/@napi-rs/keyring'), + replacement: path.resolve(dirname, "node_modules/@napi-rs/keyring"), + }, + { + find: /^express$/, + replacement: path.resolve(dirname, "node_modules/express"), + }, + { + find: /^yaml$/, + replacement: path.resolve(repoRoot, "node_modules/yaml"), }, - { find: /^express$/, replacement: path.resolve(dirname, 'node_modules/express') }, - { find: /^yaml$/, replacement: path.resolve(repoRoot, 'node_modules/yaml') }, ]; const projectResolve = { alias: [ - ...Object.entries(sharedAliases).map(([find, replacement]) => ({ find, replacement })), + ...Object.entries(sharedAliases).map(([find, replacement]) => ({ + find, + replacement, + })), ...nodeModulesAliases, ], dedupe: sharedDedupe, }; - return { repoRoot, sharedAliases, sharedDedupe, nodeModulesAliases, projectResolve }; + return { + repoRoot, + sharedAliases, + sharedDedupe, + nodeModulesAliases, + projectResolve, + }; } /** Convenience for importers that only have import.meta.url. */