chore(core): promote the CLI surface mcpdo shares into core/cli (#2461)

.claude/skills/project-structure/SKILL.md

@@ -46,6 +46,7 @@ an MCP server, the request/response lifecycle, and a set of state stores.
 | `core/auth/` | OAuth end to end — providers, discovery, storage, endpoint overrides, scopes, revocation, mid-session recovery — split into isomorphic logic plus `browser/`, `node/` and `remote/` backends |
 | `core/auth/node/` | Node OAuth storage + loopback callback server, **and** the `SecretStore` backends (keychain / file / memory) and their selection policy |
 | `core/extension/<name>/` | Host-side adapters for MCP extensions whose protocol an upstream SDK owns. `tasks/` wraps `@modelcontextprotocol/ext-tasks` with what that package leaves to its host: the raw `rawDispatch` channel, progress routing, error identity, and task-view conversions. Imports nothing from `InspectorClient`; host state arrives through narrow interfaces |
+| `core/cli/` | Node-only surface the one-shot CLI and `mcpdo` both run on (#2461): `error-handler` (exit codes, `CliExitCodeError`, the error envelope), the method handlers and their option/format types (`handlers/`), the interactive OAuth connect flow (`cliOAuth`, `cli-oauth-navigation`) and output helpers (`style`, `utils/awaitable-log`). One-shot-only output (`emit-result`, `consume-outcome`, `servers-write`, …) stays in `clients/cli/src` |
 | `core/client/` | Install-level client config (`client.json`): browser-safe parse plus Node load/save, remote backend, secrets, runner |
 | `core/json/` | JSON + parameter/argument conversion; the schema normalizations all three form builders share (nullable unions, root composition) and the tool-schema portability lint |
 | `core/react/` | React hooks over the state stores — consumed by both the web and TUI React trees. Every subscription reads its snapshot **during render** via `useSyncExternalStore` (#1955); `useStoreSnapshot.ts` caches the fresh-value-per-read getters |
@@ -55,7 +56,9 @@ an MCP server, the request/response lifecycle, and a set of state stores.
 
 `core/` is isomorphic (browser + Node) and has **no `package.json`** — it is not
 published on its own. Its tests live in `clients/web/src/test/core/`, and its
-browser-consumed runtime is inside the web coverage gate.
+browser-consumed runtime is inside the web coverage gate. **`core/cli/` is the
+exception**: it is Node-only, its tests are `clients/cli/__tests__/`, and it is
+gated by `clients/cli`'s coverage run instead.
 
 ## `clients/web/server/` — the Node backend
 

.claude/skills/testing/SKILL.md

@@ -128,7 +128,9 @@ web-owned test living under `src/test/` instead is a bug.
 1. **Tests of the repo-root `core/` package** → `src/test/core/…`, mirroring the
    `core/` folder layout. `core/` physically lives outside `clients/web/`, is
    consumed via the `@inspector/core` alias, and has no test harness of its own.
-   This includes `core/json/*` and `core/client/*`.
+   This includes `core/json/*` and `core/client/*`. **Except `core/cli/`** —
+   the Node-only surface the one-shot CLI and mcpdo share (#2461): its tests
+   live in `clients/cli/__tests__/` and `clients/cli`'s coverage run gates it.
 2. **The `integration` project** → `src/test/integration/…`, mirroring the
    `core/` source layout (`mcp/`, `mcp/node/`, `mcp/remote/`, `auth/`,
    `auth/node/`, `storage/`). **Placement is the manifest** — any file under that
@@ -218,8 +220,9 @@ Scope notes:
   (a composition root at ~42% branch coverage — gating it is a dedicated
   decomposition effort) and the `src/main.tsx` / `src/index.ts` bootstraps.
 - **CLI** tests run **in-process** by importing `runCli()`
-  (`__tests__/helpers/cli-runner.ts`) so `src` is measured; `src/index.ts` is the
-  only exclusion. `commander` uses `.exitOverride()` so a parse error throws
+  (`__tests__/helpers/cli-runner.ts`) so `src` **and the shared `core/cli/`** are
+  measured (the latter via `allowExternal`, since it sits outside the project
+  root); `src/index.ts` is the only exclusion. `commander` uses `.exitOverride()` so a parse error throws
   instead of tearing down the test worker.
 - **TUI** covers **all of `src/**`, React surface included**. Components mount
 through `**tests**/helpers/renderTui.tsx`—`ink-testing-library`'s `render`with every frame ANSI-stripped — alongside the passthrough doubles in the same

AGENTS.md

@@ -49,6 +49,8 @@ inspector/
 │   └── launcher/     The `mcp-inspector` bin; dispatches to web/cli/tui in-process
 ├── core/             Shared code, consumed via the `@inspector/core` alias (no package.json)
 │   ├── auth/         OAuth end to end + the per-server SecretStore backends
+│   ├── cli/          Node-only surface the one-shot CLI and mcpdo share (method handlers,
+│   │                 exit codes, interactive OAuth flow, output styling); tested + gated by clients/cli
 │   ├── client/       Install-level client config (`client.json`)
 │   ├── extension/    Host-side adapters for MCP extensions whose protocol an upstream SDK owns
 │   │   └── tasks/    The Inspector's glue around `@modelcontextprotocol/ext-tasks` (raw dispatch, progress routing)
@@ -410,11 +412,11 @@ When asked to respond to a code review of a PR:
 The _procedure_ — where a given test file goes, which command runs it, how to
 diagnose a failing gate — is the `testing` skill. These are the rules.
 
-- **Ensure all code has corresponding tests.** New code must clear **≥ 90 on all four dimensions** — lines, statements, functions, and branches — per file. This gate is enforced by each client's `test:coverage` across `clients/web`, `clients/cli`, `clients/tui`, `clients/launcher`, and (experimentally) `clients/mcpdo`, and **CI enforces it**: a PR that drops any file below 90 on any dimension fails. **mcpdo** excludes only true bootstraps from the gate (`src/mcp-bin.ts`, `src/daemon/run.ts` — see `clients/mcpdo/vitest.config.ts`); its build-time `@inspector/cli` alias reaches into `clients/cli/src` for shared handlers/error-handler/OAuth helpers (temporary, not a published API — #2461 tracks promoting that surface to a shared area).
+- **Ensure all code has corresponding tests.** New code must clear **≥ 90 on all four dimensions** — lines, statements, functions, and branches — per file. This gate is enforced by each client's `test:coverage` across `clients/web`, `clients/cli`, `clients/tui`, `clients/launcher`, and (experimentally) `clients/mcpdo`, and **CI enforces it**: a PR that drops any file below 90 on any dimension fails. **mcpdo** excludes only true bootstraps from the gate (`src/mcp-bin.ts`, `src/daemon/run.ts` — see `clients/mcpdo/vitest.config.ts`). The node-only surface mcpdo shares with the one-shot CLI (`core/cli/` — handlers, error-handler, OAuth helpers; #2461) is gated by `clients/cli`'s coverage run, whose suites exercise it; see `clients/cli/vitest.config.ts`.
 - **A genuinely-unreachable branch is annotated at the source, never waved through by lowering the gate.** Use a justified `/* v8 ignore … -- <reason> */`. Acceptable reasons: happy-dom-inherent paths (Mantine portal mount points, `useMediaQuery` fallbacks, `typeof window` SSR guards); React StrictMode effect-replay blocks; and provably-dead defensive guards (a `?? fallback` for a value the types guarantee non-null, a `Select.onChange` receiving a value outside the allowed list). Reach for it only when the branch is genuinely impossible to exercise.
 - **In unit tests that expect error output, suppress it from the console.**
 - **Test placement — side-by-side by default, `src/test/` only for what can't be co-located, and the Node clients are different.**
-  - **`clients/web`**: `<Name>.test.tsx` **next to the source** — components, hooks, `lib/`, `utils/`. A web-owned test living under `src/test/` instead is a bug. `src/test/` is for the three things that cannot be co-located: tests of the repo-root **`core/`** package (`src/test/core/…`, mirroring the `core/` layout — it lives outside `clients/web/` and has no harness of its own); the **`integration`** project (`src/test/integration/…` — _placement is the manifest_, picked up by a folder glob, with no enumeration to keep in sync); and **shared test infrastructure** (`renderWithMantine.tsx`, `setup.ts`, `fixtures/`).
+  - **`clients/web`**: `<Name>.test.tsx` **next to the source** — components, hooks, `lib/`, `utils/`. A web-owned test living under `src/test/` instead is a bug. `src/test/` is for the three things that cannot be co-located: tests of the repo-root **`core/`** package (`src/test/core/…`, mirroring the `core/` layout — it lives outside `clients/web/` and has no harness of its own) — **except `core/cli/`**, the Node-only surface the CLI and mcpdo share, whose tests stay in `clients/cli/__tests__/` and whose coverage `clients/cli` gates (#2461); the **`integration`** project (`src/test/integration/…` — _placement is the manifest_, picked up by a folder glob, with no enumeration to keep in sync); and **shared test infrastructure** (`renderWithMantine.tsx`, `setup.ts`, `fixtures/`).
   - **`clients/cli`, `clients/mcpdo`, `clients/tui`, `clients/launcher`**: **all** tests in a top-level **`__tests__/`**, not beside their source. Their `tsconfig.json` excludes `**/*.test.*`, so a co-located test lands in **no** tsconfig project and fails `npm run verify:typecheck-coverage`.
   - **Root tooling**: a `scripts/*.mjs` helper with pure logic gets a sibling `*.test.mjs`. Keep that exact filename — `node --test` silently _skips_ a file its glob misses and still exits 0.
 - **Render Ink components through the TUI's own `render`** (`clients/tui/__tests__/helpers/renderTui.tsx`), never `ink-testing-library`'s directly. It is the same function with every frame ANSI-stripped, which is what keeps an assertion on styled text from depending on the ambient environment: Ink writes styling _inside_ the styled run, so `<Text underline>I</Text>nfo` reaches the frame buffer with escapes between `I` and `nfo` and `toContain("Info")` fails. It only bites where chalk emits color — a developer whose shell exports `FORCE_COLOR` — so CI is green on a suite that is broken for them (#2207). A test that genuinely needs the raw bytes reads `stdout.lastFrame()` off the returned instance.

clients/cli/__tests__/README.md

@@ -3,7 +3,8 @@
 Tests live under `__tests__/` and run via Vitest.
 
 - Most tests import `runCli()` **in-process** (see `helpers/cli-runner.ts`) so
-  `clients/cli/src` is measured under the coverage gate. Suite-wide
+  `clients/cli/src` and the shared `core/cli` surface are measured under the
+  coverage gate. Suite-wide
   `helpers/mock-open-url.ts` (vitest `setupFiles`) mocks core's `openUrl` so an armed
   interactive OAuth path cannot launch a real browser.
 - `e2e.test.ts` (and root `scripts/smoke-cli.mjs`) spawn the built binary for

clients/cli/__tests__/cli-oauth-navigation.test.ts

@@ -3,7 +3,7 @@ import {
   createCliOAuthNavigation,
   isCliAutoOpenForced,
   resolveCliAutoOpenEnabled,
-} from "../src/cli-oauth-navigation.js";
+} from "@inspector/core/cli/cli-oauth-navigation.js";
 import { openUrl } from "@inspector/core/node/openUrl.js";
 
 // Local mock (in addition to suite-wide setupFiles) so this file owns a

clients/cli/__tests__/cliOAuth.test.ts

@@ -10,7 +10,7 @@ import {
   assertInteractiveOAuthAllowed,
   withCliAuthRecoveryRetry,
   STEP_UP_PIPE_TIMEOUT_MS,
-} from "../src/cliOAuth.js";
+} from "@inspector/core/cli/cliOAuth.js";
 import type { MCPServerConfig } from "@inspector/core/mcp/types.js";
 import { createInterface } from "node:readline/promises";
 import {

clients/cli/__tests__/completion.test.ts

@@ -23,7 +23,7 @@ import {
   renderCompletion,
   type CompletionShell,
 } from "../src/completion.js";
-import { ONE_SHOT_METHODS } from "../src/handlers/method-types.js";
+import { ONE_SHOT_METHODS } from "@inspector/core/cli/handlers/method-types.js";
 
 async function script(shell: CompletionShell): Promise<string> {
   const result = await runCli(["--completion", shell]);

clients/cli/__tests__/consume-outcome-stream.test.ts

@@ -1,6 +1,6 @@
 import { describe, it, expect, afterEach, vi } from "vitest";
 import { consumeMethodOutcome } from "../src/handlers/consume-outcome.js";
-import type { MethodOutcome } from "../src/handlers/method-types.js";
+import type { MethodOutcome } from "@inspector/core/cli/handlers/method-types.js";
 
 /**
  * The long-lived stream path's stdout error handling (#2412). A reader that

clients/cli/__tests__/emit-result.test.ts

@@ -1,6 +1,6 @@
 import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
 import { emitResult, collectAppInfo } from "../src/cli.js";
-import { CliExitCodeError } from "../src/error-handler.js";
+import { CliExitCodeError } from "@inspector/core/cli/error-handler.js";
 import type { InspectorClient } from "@inspector/core/mcp/index.js";
 
 /**

clients/cli/__tests__/error-handler.test.ts

@@ -5,7 +5,7 @@ import {
   classifyError,
   formatErrorOutput,
   handleError,
-} from "../src/error-handler.js";
+} from "@inspector/core/cli/error-handler.js";
 import { UnauthorizedError } from "@modelcontextprotocol/client";
 import {
   SecretFileLockHeldError,

clients/cli/__tests__/format-output.test.ts

@@ -1,5 +1,5 @@
 import { describe, it, expect, afterEach } from "vitest";
-import { writeFormattedResult } from "../src/handlers/format-output.js";
+import { writeFormattedResult } from "@inspector/core/cli/handlers/format-output.js";
 
 describe("writeFormattedResult", () => {
   let originalWrite: typeof process.stdout.write;

clients/cli/__tests__/helpers/cli-runner.ts

@@ -1,5 +1,5 @@
 import { runCli as invokeCli } from "../../src/cli.js";
-import { formatErrorOutput } from "../../src/error-handler.js";
+import { formatErrorOutput } from "@inspector/core/cli/error-handler.js";
 
 export interface CliResult {
   exitCode: number | null;
@@ -45,7 +45,7 @@ function captureWrite(append: (text: string) => void) {
 
 /**
  * Run the CLI **in-process** by importing and invoking `runCli` directly, so
- * its source (`clients/cli/src/**`) is measured under vitest's coverage
+ * its source (`clients/cli/src/**`, plus the shared `core/cli/**`) is measured under vitest's coverage
  * instrumentation. The previous implementation spawned `build/index.js` as a
  * subprocess, which left CLI source invisible to coverage (#1484).
  *

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

@@ -4,7 +4,7 @@ import {
   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";
+import type { CliOAuthClient } from "@inspector/core/cli/cliOAuth.js";
 
 /**
  * Typed mock factories for the CLI OAuth tests. They exist so a test can supply

clients/cli/__tests__/method-types.test.ts

@@ -3,7 +3,7 @@ import {
   isOneShotMethod,
   ONE_SHOT_METHODS,
   CONNECTION_RPC_METHODS,
-} from "../src/handlers/method-types.js";
+} from "@inspector/core/cli/handlers/method-types.js";
 
 describe("CONNECTION_RPC_METHODS", () => {
   it("lists the full RPC method set supported by runMethod", () => {

clients/cli/__tests__/oauth-interactive.test.ts

@@ -28,7 +28,7 @@ import { NodeOAuthStorage } from "@inspector/core/auth/node/index.js";
 import {
   connectInspectorWithOAuth,
   withCliAuthRecoveryRetry,
-} from "../src/cliOAuth.js";
+} from "@inspector/core/cli/cliOAuth.js";
 import type { MCPServerConfig } from "@inspector/core/mcp/types.js";
 import { makeFakeServerSettings } from "./helpers/oauth-test-fakes.js";
 

clients/cli/__tests__/output-file.test.ts

@@ -11,8 +11,8 @@ import {
   renderResultForFile,
   validateOutputOptions,
   writeResultFile,
-} from "../src/handlers/output-file.js";
-import { CliExitCodeError } from "../src/error-handler.js";
+} from "@inspector/core/cli/handlers/output-file.js";
+import { CliExitCodeError } from "@inspector/core/cli/error-handler.js";
 import { emitResult } from "../src/handlers/emit-result.js";
 
 /**

clients/cli/__tests__/run-method-mocks.test.ts

@@ -1,5 +1,5 @@
 import { describe, it, expect, vi } from "vitest";
-import { runMethod } from "../src/handlers/run-method.js";
+import { runMethod } from "@inspector/core/cli/handlers/run-method.js";
 import type { InspectorClient } from "@inspector/core/mcp/index.js";
 
 function mockClient(overrides: Partial<InspectorClient> = {}): InspectorClient {

clients/cli/__tests__/run-method-skills.test.ts

@@ -1,10 +1,10 @@
 import { describe, it, expect, vi } from "vitest";
-import { runMethod } from "../src/handlers/run-method.js";
+import { runMethod } from "@inspector/core/cli/handlers/run-method.js";
 import {
   skillVerificationExitCode,
   summarizeSkillVerification,
-} from "../src/handlers/skills-verify.js";
-import { EXIT_CODES } from "../src/error-handler.js";
+} from "@inspector/core/cli/handlers/skills-verify.js";
+import { EXIT_CODES } from "@inspector/core/cli/error-handler.js";
 import type { InspectorClient } from "@inspector/core/mcp/index.js";
 import type { SkillEntry } from "@inspector/core/mcp/skillsSchemas.js";
 import { sha256Digest } from "@inspector/core/mcp/skills.js";

clients/cli/__tests__/run-method.test.ts

@@ -2,7 +2,7 @@ import { describe, it, expect, afterEach, vi } from "vitest";
 import { getTestMcpServerCommand } from "@modelcontextprotocol/inspector-test-server";
 import { InspectorClient } from "@inspector/core/mcp/index.js";
 import { createTransportNode } from "@inspector/core/mcp/node/index.js";
-import { runMethod } from "../src/handlers/run-method.js";
+import { runMethod } from "@inspector/core/cli/handlers/run-method.js";
 import { consumeMethodOutcome } from "../src/handlers/consume-outcome.js";
 
 describe("runMethod", () => {

clients/cli/__tests__/schema-lint-report.test.ts

@@ -1,6 +1,9 @@
 import { describe, it, expect, beforeEach, afterEach } from "vitest";
 import { emitResult, runCli } from "../src/cli.js";
-import { CliExitCodeError, EXIT_CODES } from "../src/error-handler.js";
+import {
+  CliExitCodeError,
+  EXIT_CODES,
+} from "@inspector/core/cli/error-handler.js";
 import {
   lintListResult,
   toolsFromResult,

clients/cli/__tests__/servers-list.test.ts

@@ -14,7 +14,7 @@ import {
   sanitizeServerSettings,
   showServerEntry,
   summarizeServerConfig,
-} from "../src/handlers/servers-list.js";
+} from "@inspector/core/cli/handlers/servers-list.js";
 import type {
   InspectorServerSettings,
   MCPServerConfig,

clients/cli/__tests__/skills-verify-cli.test.ts

@@ -6,7 +6,7 @@ import {
 import type { SkillVerifyReport } from "@inspector/core/mcp/skillsVerification.js";
 import { runCli } from "../src/cli.js";
 import { consumeMethodOutcome } from "../src/handlers/consume-outcome.js";
-import { EXIT_CODES } from "../src/error-handler.js";
+import { EXIT_CODES } from "@inspector/core/cli/error-handler.js";
 import {
   runCli as runCliCaptured,
   type CliResult,

clients/cli/__tests__/style.test.ts

@@ -4,7 +4,7 @@ import {
   resolveAnsiEnabled,
   styleFromOpts,
   PLAIN,
-} from "../src/style.js";
+} from "@inspector/core/cli/style.js";
 
 describe("resolveAnsiEnabled", () => {
   it("is off for --plain, json, NO_COLOR, and non-TTY", () => {