test(cli,tui): typecheck the __tests__ dirs (#1791)

.github/workflows/main.yml

@@ -28,8 +28,10 @@ jobs:
         # clients/launcher, so this single step sets up every client.
         run: npm install
 
-      - name: Validate (format, lint, build, fast tests)
-        # Each client self-validates: format:check + lint + build + test (no
+      - name: Validate (coverage guards, format, lint, typecheck, build, fast tests)
+        # Runs the two coverage guards first (verify:format-coverage +
+        # verify:typecheck-coverage), then validate:core, then each client's
+        # self-validation: format:check + lint + typecheck + build + test (no
         # coverage instrumentation — fast). This also builds every client bundle
         # (web dist, cli/tui/launcher) that the smokes below need. The heavier
         # per-file coverage gate runs in its own step below. Unit tests run in

README.md

@@ -146,6 +146,8 @@ Each client self-validates from its own folder; the root scripts chain them. The
 | `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 test:scripts` | Table-driven unit tests (`node --test`) for the guard's own pure parsers (`scripts/lib/npm-scripts.mjs` + the exported helpers of `verify-typecheck-coverage.mjs`), one case per rule they encode. Runs in `validate` — and `verify:typecheck-coverage` guards *this* gate in turn (reachable from `validate`, non-empty test set, every test file matched by the `test:scripts` glob), since `node --test` silently skips a file its glob misses and still exits 0. |
+| `npm run verify:typecheck-coverage` | The typecheck-coverage analog of the above (#1791): for each Node client (auto-discovered from disk — enrolled via its `typecheck` script's projects, or for a `tsc -b` client like `clients/web` via its `tsconfig.json` `references`) it runs those projects with `tsc --listFilesOnly`, unions them, and **fails** listing any tracked `.ts`/`.tsx`/`.mts`/`.cts` under the client that lands in no project (so a new top-level config/helper can't silently go untypechecked). It also requires, deny-by-default, the first-party TS no client owns (`test-servers/src`, the root `vitest.shared.mts`, all of `core/`, and any new top-level location) to land in some client project's tsc pass — so a `core` `*.tsx` web's projects don't reach is caught too. Also asserts the gate is wired (each client's typecheck pass — its `typecheck` script, or web's `tsc -b` — is reachable from its `validate`, and the root chain runs each client's `validate`). Runs in `validate`.                        |
 | `npm run 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).                                                                                                                                                                                                          |
 

clients/cli/__tests__/cli.test.ts

@@ -22,7 +22,7 @@ import {
   createEchoTool,
   createTestServerInfo,
 } from "@modelcontextprotocol/inspector-test-server";
-import type { MCPServerConfig } from "@modelcontextprotocol/inspector-core/mcp/index.js";
+import type { MCPServerConfig } from "@inspector/core/mcp/index.js";
 
 describe("CLI Tests", () => {
   describe("Basic CLI Mode", () => {
@@ -368,7 +368,7 @@ describe("CLI Tests", () => {
         // the config file into the connection's settings.
         const sawHeader = server
           .getRecordedRequests()
-          .some((r) => r.headers["x-custom-token"] === "secret-123");
+          .some((r) => r.headers?.["x-custom-token"] === "secret-123");
         expect(sawHeader).toBe(true);
       } finally {
         await server.stop();
@@ -409,7 +409,7 @@ describe("CLI Tests", () => {
         expectCliSuccess(result);
         const tokens = server
           .getRecordedRequests()
-          .map((r) => r.headers["x-custom-token"]);
+          .map((r) => r.headers?.["x-custom-token"]);
         expect(tokens).toContain("from-cli");
         expect(tokens).not.toContain("from-disk");
       } finally {

clients/cli/__tests__/cliOAuth.test.ts

@@ -13,6 +13,10 @@ import {
 } from "../src/cliOAuth.js";
 import type { MCPServerConfig } from "@inspector/core/mcp/types.js";
 import { createInterface } from "node:readline/promises";
+import {
+  makeFakeCliOAuthClient,
+  makeFakeServerSettings,
+} from "./helpers/oauth-test-fakes.js";
 
 // `confirmStepUpFromStdin` (the default step-up confirmer) reads a line from
 // stdin via node:readline/promises. Mock the module so the default path can be
@@ -103,7 +107,7 @@ describe("cliOAuth", () => {
       expect(
         isStandardOAuthStepUp(
           { reason: "insufficient_scope", requiredScopes: ["weather:read"] },
-          {},
+          makeFakeServerSettings(),
         ),
       ).toBe(true);
     });
@@ -112,7 +116,7 @@ describe("cliOAuth", () => {
       expect(
         isStandardOAuthStepUp(
           { reason: "insufficient_scope", requiredScopes: ["weather:read"] },
-          { enterpriseManaged: true },
+          makeFakeServerSettings({ enterpriseManaged: true }),
         ),
       ).toBe(false);
     });
@@ -220,7 +224,7 @@ describe("cliOAuth", () => {
         error,
         new MutableRedirectUrlProvider(),
         { hostname: "127.0.0.1", port: 6276, pathname: "/oauth/callback" },
-        {},
+        makeFakeServerSettings(),
         { confirmStepUp: async () => false, isTTY: true },
       ),
     ).rejects.toThrow("Step-up authorization declined.");
@@ -250,7 +254,7 @@ describe("cliOAuth", () => {
       error,
       new MutableRedirectUrlProvider(),
       { hostname: "127.0.0.1", port: 6276, pathname: "/oauth/callback" },
-      {},
+      makeFakeServerSettings(),
       { confirmStepUp: async () => true, isTTY: true },
     );
 
@@ -338,7 +342,7 @@ describe("cliOAuth", () => {
       OAUTH_HTTP_CONFIG,
       new MutableRedirectUrlProvider(),
       { hostname: "127.0.0.1", port: 6276, pathname: "/oauth/callback" },
-      { enterpriseManaged: true },
+      makeFakeServerSettings({ enterpriseManaged: true }),
       fn,
       { confirmStepUp: async () => true, isTTY: true },
     );
@@ -375,7 +379,7 @@ describe("cliOAuth", () => {
         standardStepUpError(),
         new MutableRedirectUrlProvider(),
         CALLBACK_URL_CONFIG,
-        {},
+        makeFakeServerSettings(),
         INTERACTIVE,
       );
 
@@ -395,7 +399,7 @@ describe("cliOAuth", () => {
         standardStepUpError(),
         new MutableRedirectUrlProvider(),
         CALLBACK_URL_CONFIG,
-        {},
+        makeFakeServerSettings(),
         INTERACTIVE,
       );
 
@@ -412,7 +416,7 @@ describe("cliOAuth", () => {
           standardStepUpError(),
           new MutableRedirectUrlProvider(),
           CALLBACK_URL_CONFIG,
-          {},
+          makeFakeServerSettings(),
           INTERACTIVE,
         ),
       ).rejects.toThrow("Step-up authorization declined.");
@@ -431,7 +435,7 @@ describe("cliOAuth", () => {
           standardStepUpError(),
           new MutableRedirectUrlProvider(),
           CALLBACK_URL_CONFIG,
-          {},
+          makeFakeServerSettings(),
           INTERACTIVE,
         ),
       ).rejects.toThrow("Step-up authorization declined.");
@@ -455,7 +459,7 @@ describe("cliOAuth", () => {
           standardStepUpError(),
           new MutableRedirectUrlProvider(),
           CALLBACK_URL_CONFIG,
-          {},
+          makeFakeServerSettings(),
           INTERACTIVE,
         ),
       ).rejects.toThrow("Step-up authorization declined.");
@@ -482,7 +486,7 @@ describe("cliOAuth", () => {
         standardStepUpError(),
         new MutableRedirectUrlProvider(),
         CALLBACK_URL_CONFIG,
-        {},
+        makeFakeServerSettings(),
         INTERACTIVE,
       );
 
@@ -503,7 +507,7 @@ describe("cliOAuth", () => {
           standardStepUpError(),
           new MutableRedirectUrlProvider(),
           CALLBACK_URL_CONFIG,
-          {},
+          makeFakeServerSettings(),
           { isTTY: false },
         );
         expect(runSpy).toHaveBeenCalled();
@@ -522,7 +526,7 @@ describe("cliOAuth", () => {
           standardStepUpError(),
           new MutableRedirectUrlProvider(),
           CALLBACK_URL_CONFIG,
-          {},
+          makeFakeServerSettings(),
           { isTTY: true, stepUpPromptTimeoutMs: 20 },
         ),
       ).rejects.toMatchObject({
@@ -556,7 +560,7 @@ describe("cliOAuth", () => {
           standardStepUpError(),
           new MutableRedirectUrlProvider(),
           CALLBACK_URL_CONFIG,
-          {},
+          makeFakeServerSettings(),
           INTERACTIVE,
         );
         let settled = false;
@@ -596,11 +600,10 @@ describe("cliOAuth", () => {
           ),
         )
         .mockResolvedValueOnce(undefined);
-      const client = {
+      const client = makeFakeCliOAuthClient({
         connect,
-        disconnect: vi.fn(),
         checkAuthChallengeSatisfied: vi.fn().mockResolvedValue(true),
-      };
+      });
 
       await connectInspectorWithOAuth(
         client,
@@ -626,11 +629,10 @@ describe("cliOAuth", () => {
           ),
         )
         .mockResolvedValueOnce(undefined);
-      const client = {
+      const client = makeFakeCliOAuthClient({
         connect,
-        disconnect: vi.fn(),
         checkAuthChallengeSatisfied: vi.fn().mockResolvedValue(false),
-      };
+      });
 
       await connectInspectorWithOAuth(
         client,
@@ -654,11 +656,11 @@ describe("cliOAuth", () => {
         .mockRejectedValueOnce(new Error("Connection failed for server (401)"))
         .mockResolvedValueOnce(undefined);
       // A rejecting disconnect exercises the `.catch(() => {})` guard.
-      const client = {
+      const client = makeFakeCliOAuthClient({
         connect,
         disconnect: vi.fn().mockRejectedValue(new Error("disconnect failed")),
         checkAuthChallengeSatisfied: vi.fn(),
-      };
+      });
 
       await connectInspectorWithOAuth(
         client,
@@ -679,11 +681,10 @@ describe("cliOAuth", () => {
       const connect = vi
         .fn()
         .mockRejectedValue(new Error("some unrelated failure"));
-      const client = {
+      const client = makeFakeCliOAuthClient({
         connect,
-        disconnect: vi.fn(),
         checkAuthChallengeSatisfied: vi.fn(),
-      };
+      });
 
       await expect(
         connectInspectorWithOAuth(
@@ -698,11 +699,10 @@ describe("cliOAuth", () => {
 
     it("rethrows when the server config is not OAuth-capable", async () => {
       const connect = vi.fn().mockRejectedValue(new Error("nope (401)"));
-      const client = {
+      const client = makeFakeCliOAuthClient({
         connect,
-        disconnect: vi.fn(),
         checkAuthChallengeSatisfied: vi.fn(),
-      };
+      });
 
       await expect(
         connectInspectorWithOAuth(
@@ -721,11 +721,10 @@ describe("cliOAuth", () => {
           reason: "token_expired",
         }),
       );
-      const client = {
+      const client = makeFakeCliOAuthClient({
         connect,
-        disconnect: vi.fn(),
         checkAuthChallengeSatisfied: vi.fn().mockResolvedValue(false),
-      };
+      });
 
       await expect(
         connectInspectorWithOAuth(
@@ -751,11 +750,10 @@ describe("cliOAuth", () => {
           ),
         )
         .mockResolvedValueOnce(undefined);
-      const client = {
+      const client = makeFakeCliOAuthClient({
         connect,
-        disconnect: vi.fn(),
         checkAuthChallengeSatisfied: vi.fn().mockResolvedValue(true),
-      };
+      });
 
       await connectInspectorWithOAuth(
         client,
@@ -775,11 +773,10 @@ describe("cliOAuth", () => {
       const connect = vi
         .fn()
         .mockRejectedValue(new Error("Connection failed for server (401)"));
-      const client = {
+      const client = makeFakeCliOAuthClient({
         connect,
-        disconnect: vi.fn(),
         checkAuthChallengeSatisfied: vi.fn(),
-      };
+      });
 
       await expect(
         connectInspectorWithOAuth(
@@ -1101,11 +1098,10 @@ describe("cliOAuth", () => {
     const connect = vi.fn().mockRejectedValue(err);
     await expect(
       connectInspectorWithOAuth(
-        {
+        makeFakeCliOAuthClient({
           connect,
-          disconnect: vi.fn(),
           checkAuthChallengeSatisfied: vi.fn().mockResolvedValue(false),
-        },
+        }),
         OAUTH_HTTP_CONFIG,
         new MutableRedirectUrlProvider(),
         CALLBACK_URL_CONFIG,

clients/cli/__tests__/helpers/fixtures.ts

@@ -3,7 +3,7 @@ import * as path from "path";
 import * as os from "os";
 import * as crypto from "crypto";
 import { getTestMcpServerCommand } from "@modelcontextprotocol/inspector-test-server";
-import type { MCPServerConfig } from "@modelcontextprotocol/inspector-core/mcp/index.js";
+import type { MCPServerConfig } from "@inspector/core/mcp/index.js";
 
 /**
  * Sentinel value for tests that don't need a real server

clients/cli/__tests__/helpers/oauth-test-fakes.ts

@@ -0,0 +1,75 @@
+import { vi } from "vitest";
+import {
+  DEFAULT_MAX_FETCH_REQUESTS,
+  DEFAULT_TASK_TTL_MS,
+} from "@inspector/core/mcp/types.js";
+import type { InspectorServerSettings } from "@inspector/core/mcp/types.js";
+import type { CliOAuthClient } from "../../src/cliOAuth.js";
+
+/**
+ * Typed mock factories for the CLI OAuth tests. They exist so a test can supply
+ * only the surface a given code path exercises while `tsc` still sees a
+ * complete `CliOAuthClient` / `InspectorServerSettings` — avoiding a spray of
+ * `as unknown as` casts (see #1791 and the AGENTS.md `as`-cast policy).
+ */
+
+/**
+ * A full {@link CliOAuthClient} whose every method is a `vi.fn()` resolving to a
+ * benign value. Pass `overrides` to swap in the method(s) a test drives (e.g. a
+ * `connect` that rejects with `AuthRecoveryRequiredError`).
+ */
+export function makeFakeCliOAuthClient(
+  overrides: Partial<CliOAuthClient> = {},
+): CliOAuthClient {
+  // The defaults are typed against the real CliOAuthClient signatures (not bare
+  // vi.fn()s) so a default with a wrong implementation/return would be rejected,
+  // the same way the tui App spies are typed. Note this does NOT extend to
+  // `overrides`: a caller passing a bare `vi.fn()` gets `Mock<(...args) => any>`,
+  // assignable to any member, so an override's implementation is unchecked — the
+  // factory guarantees the object's shape, not each override's signature. (And
+  // since `exactOptionalPropertyTypes` is off, `Partial<…>` even admits an
+  // explicit `undefined` for a required member.)
+  return {
+    connect: vi.fn<CliOAuthClient["connect"]>().mockResolvedValue(undefined),
+    disconnect: vi
+      .fn<CliOAuthClient["disconnect"]>()
+      .mockResolvedValue(undefined),
+    authenticate: vi
+      .fn<CliOAuthClient["authenticate"]>()
+      .mockResolvedValue(undefined),
+    beginInteractiveAuthorization: vi
+      .fn<CliOAuthClient["beginInteractiveAuthorization"]>()
+      .mockResolvedValue(undefined),
+    completeOAuthFlow: vi
+      .fn<CliOAuthClient["completeOAuthFlow"]>()
+      .mockResolvedValue(undefined),
+    checkAuthChallengeSatisfied: vi
+      .fn<CliOAuthClient["checkAuthChallengeSatisfied"]>()
+      .mockResolvedValue(false),
+    ...overrides,
+  };
+}
+
+/**
+ * A full {@link InspectorServerSettings} with representative defaults for its
+ * required fields (the timeouts at 0 = "SDK default", `taskTtl` /
+ * `maxFetchRequests` at their product defaults, empty lists elsewhere). Pass
+ * `overrides` for the field(s) under test (e.g. `{ enterpriseManaged: true }`);
+ * as a `Partial<…>` with `exactOptionalPropertyTypes` off it also admits an
+ * explicit `undefined` for a required field, so don't rely on it to reject one.
+ */
+export function makeFakeServerSettings(
+  overrides: Partial<InspectorServerSettings> = {},
+): InspectorServerSettings {
+  return {
+    headers: [],
+    metadata: [],
+    env: [],
+    connectionTimeout: 0,
+    requestTimeout: 0,
+    taskTtl: DEFAULT_TASK_TTL_MS,
+    maxFetchRequests: DEFAULT_MAX_FETCH_REQUESTS,
+    roots: [],
+    ...overrides,
+  };
+}