Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 11 additions & 6 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,13 @@ inspector/
│ ├── tui/ # TUI client (Ink + React, tsup bundle)
│ ├── launcher/ # Shared launcher (relative imports into sibling build/ outputs)
├── core/ # Shared core code (no package.json — consumed via the `@inspector/core` vite alias)
│ ├── auth/ # OAuth: state machine, providers, discovery, storage
│ ├── auth/ # OAuth: state machine, providers, discovery, storage;
│ │ # mid-session recovery (challenge.ts WWW-Authenticate
│ │ # parsing, scopes.ts SEP-2350 scope union, oauthUx.ts
│ │ # shared copy, mcpAuth.ts force-reauthorization)
│ │ ├── browser/ # Browser-side OAuth (sessionStorage, BrowserNavigation)
│ │ ├── node/ # Node-side OAuth (NodeOAuthStorage, OAuthCallbackServer)
│ │ ├── node/ # Node-side OAuth (NodeOAuthStorage, OAuthCallbackServer,
│ │ │ # runner-interactive-oauth loopback callback flow)
│ │ └── remote/ # Remote OAuth storage (delegates to the remote server)
│ ├── json/ # JSON utilities and parameter/argument conversion
│ ├── logging/ # Silent pino logger singleton
Expand Down Expand Up @@ -189,14 +193,15 @@ gh project item-edit --project-id PVT_kwDOCt2Azc4BJVxt --id "$ITEM_ID" --field-i

### Lint-fixed, Formatted code
- ALWAYS do `npm run format` before committing — it auto-fixes any Prettier issues. `validate` runs `format:check` (the non-fixing variant) and will fail in CI on any unformatted file, so always run the auto-fixer first rather than letting `format:check` catch it.
- ALWAYS do `npm run validate` before pushing any changes — from the repo root it chains the four per-client validations (`validate:web` → `validate:cli` → `validate:tui` → `validate:launcher`); each delegates to that client's own `npm run validate` = `format:check` + `lint` + `build` + `test` in its own folder (no coverage — fast). Every client is self-validating and the top level just chains them, building each client's bundle along the way (no cross-client build dependencies).
- **`npm run ci` before pushing** — runs the same steps as `.github/workflows/main.yml` (minus `npm install`): `validate` → web `test:integration` → `smoke` → Storybook play-function tests (installs Playwright chromium if needed). Use this when you want CI parity; expect several minutes. **`npm run validate`** remains the fast loop during development (unit tests only — no web integration, smoke, or Storybook).
- ALWAYS do `npm run format` before committing, then **`npm run ci`** (or at minimum `npm run validate`) before pushing. From the repo root, `validate` chains the four per-client validations (`validate:web` → `validate:cli` → `validate:tui` → `validate:launcher`); each delegates to that client's own `npm run validate` = `format:check` + `lint` + `build` + `test` in its own folder (no coverage — fast). Every client is self-validating and the top level just chains them, building each client's bundle along the way (no cross-client build dependencies).
- The one CLI nuance: `clients/cli`'s out-of-process `e2e.test.ts` spawns the built binary, so its `test` **builds first** via `pretest` (`test-servers:build && build`). To avoid building it twice, `clients/cli`'s `validate` folds that in — it is `format:check && lint && test` with **no** separate `build` step (the other clients, whose tests don't spawn their bundle, keep an explicit `build`). `validate:web`/`validate:tui`/`validate:launcher` are the uniform `format:check && lint && build && test`.
- Before pushing, also run **`npm run coverage`** — `validate` is fast and does NOT enforce the per-file gate (or, for web, run the integration project); `coverage` does both. CI does **not** run `coverage` (the gate is local-only); it runs `validate` plus a standalone web `test:integration` step.
- **`smoke` is a separate top-level target, NOT part of `validate`.** Run it (or the individual `smoke:*`) after a build/validate: `npm run validate && npm run smoke`. It runs `smoke:launcher` (`--help` dispatch) plus the prod `smoke:cli` / `smoke:tui` / `smoke:web`, and contains **no build commands** — it assumes the cli/tui/launcher bundles already exist (a full `validate` builds them; `smoke:web` builds `clients/web/dist` on demand). CI runs `validate`, then the web `test:integration` step, then `smoke`. Storybook is the only CI step left out (see below).
- Optionally also run **`npm run coverage`** when you want the local-only per-file ≥90 gate CI does **not** run `coverage`; it relies on `validate` + web `test:integration` instead.
- **`smoke` is NOT part of `validate`** it is included in `npm run ci`. It runs `smoke:launcher` (`--help` dispatch) plus the prod `smoke:cli` / `smoke:tui` / `smoke:web`, and contains **no build commands** — it assumes the cli/tui/launcher bundles already exist (a full `validate` builds them; `smoke:web` builds `clients/web/dist` on demand).
- `smoke:launcher` (`scripts/smoke-launcher.mjs`) runs the built launcher with `--help`, `--cli --help`, and `--tui --help`, asserting each exits 0 and prints that mode's usage banner (which also proves the launcher resolved and loaded the right client build). It's the cheap dispatch check before the heavier prod smokes below.
- `smoke:web` (`scripts/smoke-web.mjs`) starts `mcp-inspector --web` (prod, no `--dev`) against the built `clients/web/dist` and asserts `GET /` serves the SPA (HTTP 200) with the injected `__INSPECTOR_API_TOKEN__`. Prod `--web` serves from `clients/web/dist`, which ships in the published package but is absent in a fresh checkout — the runner builds it on demand (`build:client` = `vite build`) on first launch, or exits with an actionable error if that build can't run (see `clients/web/server/ensure-web-build.ts` and the launcher README). `--dev` runs Vite directly and never needs `dist`.
- `smoke:cli` (`scripts/smoke-cli.mjs`) drives `mcp-inspector --cli` through the built launcher against the bundled stdio test server via a temp `--catalog`: it asserts `tools/list` returns the server's tools (real connect over stdio), the default writable catalog is seeded empty on first run, a missing read-only `--config` errors without seeding, and `--catalog` + `--config` is rejected. `smoke:tui` (`scripts/smoke-tui.mjs`) launches `mcp-inspector --tui --catalog <temp>` and asserts the Ink app renders its first frame (the "MCP Servers" panel) within a timeout, then SIGTERMs it — a shallow boot/render check, not full interaction. **`smoke:tui` is local-only: it self-skips when `process.env.CI` is set**, because the Ink TUI needs a real TTY (raw mode) that headless CI lacks — so run it (via `npm run smoke`) on your own machine before pushing. Both build `test-servers/build` on demand if it's missing.
- Also run `npm run test:storybook` from `clients/web/` before pushing — it executes every story's `play` function in headless Chromium via `@vitest/browser-playwright` (~10s). CI runs this as a separate step (from `clients/web`) after `validate`; failures block merge. It is kept out of `validate` because it needs the Playwright browser binary and is much slower than the unit suite. (There is no root-level `test:storybook` aggregate — run it in the web client.)
- Storybook play-function tests (`clients/web` `test:storybook`) run in headless Chromium via `@vitest/browser-playwright` (~10s). They are part of `npm run ci` (which installs Playwright chromium first); kept out of `validate` because they need the browser binary and are slower than the unit suite.

### Typescript instructions
- Use TypeScript for all new code
Expand Down
16 changes: 11 additions & 5 deletions clients/cli/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -102,9 +102,15 @@ Options that specify the MCP server (catalog/config file, ad-hoc command/URL, en

### CLI-specific (OAuth for HTTP servers)

The CLI **reuses** OAuth tokens from `~/.mcp-inspector/storage/oauth.json` (same file as the TUI). Complete first-time authorization in the **web** or **TUI** client, then run one-shot CLI commands against HTTP/SSE servers without signing in again.
The CLI runs the same loopback callback server as the TUI (`http://127.0.0.1:6276/oauth/callback` by default). On connect **401** or mid-session interactive auth (re-login / step-up), it:

The CLI does **not** start a local callback server or retry connect on 401. If tokens are missing or expired, connect fails; `ConsoleNavigation` may print an authorize URL to stdout, but the CLI cannot finish the redirect flow. Use the TUI for interactive runner OAuth until Phase 4 adds a CLI callback server.
1. Starts the callback listener on `--callback-url` (or `MCP_OAUTH_CALLBACK_URL`)
2. Prints the authorization URL to the console (`ConsoleNavigation`)
3. Waits for the browser redirect, exchanges the code, and retries connect or the failed RPC

**Step-up (standard OAuth):** when an RPC needs extra scopes, the CLI prompts on stderr: `Proceed with step-up authorization? [y/N]`. **y** continues; **N** exits with an error. EMA step-up re-mints silently (no prompt).

**Shared OAuth storage:** the CLI **reuses** tokens from `~/.mcp-inspector/storage/oauth.json` when they already exist (same file as other Inspector clients). That is passive file sharing, not launching another app.

**Shared with TUI** (config only, not interactive login):

Expand All @@ -119,9 +125,9 @@ The CLI does **not** start a local callback server or retry connect on 401. If t
| ------- | ---------------- |
| **Web** | `http://localhost:6274/oauth/callback` |
| **TUI** | `http://127.0.0.1:6276/oauth/callback` (interactive — callback server) |
| **CLI** | `http://127.0.0.1:6276/oauth/callback` (redirect URI in OAuth metadata only; no listener) |
| **CLI** | `http://127.0.0.1:6276/oauth/callback` (interactive — same callback server as TUI) |

Register `http://127.0.0.1:6276/oauth/callback` on static or enterprise IdPs that require pre-registered redirect URIs before using the **TUI** (or when your OAuth app expects that URI). Override with `--callback-url` or `MCP_OAUTH_CALLBACK_URL`. The CLI passes this value as `redirect_uri` when an OAuth flow runs, but does not listen on the port.
Register `http://127.0.0.1:6276/oauth/callback` on static or enterprise IdPs that require pre-registered redirect URIs before using the **TUI** or **CLI**. Override with `--callback-url` or `MCP_OAUTH_CALLBACK_URL`. Only one process should bind the default port at a time.

#### Flags

Expand All @@ -141,7 +147,7 @@ npx @modelcontextprotocol/inspector --cli --catalog mcp.json --server my-http-se
--method tools/list
```

See [EMA / enterprise-managed auth](../../specification/v2_auth_ema.md) and [OAuth smoke testing](../../specification/v2_auth_smoke_testing.md) for configuration details and staging servers.
See [EMA / enterprise-managed auth](../../specification/v2_auth_ema.md) and [OAuth smoke testing](../../specification/v2_auth_smoke_testing.md) (§3 Stytch/CIMD; [§5 mid-session manual validation](v2_auth_smoke_testing.md#5-mid-session-auth--step-up--manual-validation) — CLI **C1–C2**).

## Why use the CLI?

Expand Down
6 changes: 5 additions & 1 deletion clients/cli/__tests__/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ npm run test:cli # cli.test.ts
npm run test:cli-tools # tools.test.ts
npm run test:cli-headers # headers.test.ts
npm run test:cli-metadata # metadata.test.ts
npx vitest run oauth-interactive.test.ts cliOAuth.test.ts # OAuth interactive smoke parity
```

## How the CLI is exercised
Expand All @@ -42,7 +43,10 @@ root provides a further end-to-end check of the binary.)
- `metadata.test.ts` - Metadata functionality: General metadata, tool-specific metadata, parsing, merging, validation
- `methods.test.ts` - Method/option-validation paths not covered elsewhere (resource templates, missing/invalid options, the `--` target separator)
- `error-handler.test.ts` - The binary's `handleError` error sink, exercised in-process with `process.exit` stubbed
- `e2e.test.ts` - Out-of-process spawn of the built binary (exit codes + boot)
- `oauth-runner.test.ts` - OAuth flag wiring (`--client-config`, `--callback-url`, CIMD overrides)
- `cliOAuth.test.ts` - Unit tests for `cliOAuth.ts` (step-up confirm, helper wiring, retry)
- `oauth-interactive.test.ts` - **Integration** smoke parity for CLI interactive OAuth: connect-time callback server + step-up **y/N** against composable `TestServerHttp` (auto-completes authorize URL programmatically; not a subprocess binary e2e)
- `e2e.test.ts` - Out-of-process spawn of the built binary (exit codes + boot; no OAuth)

## Helpers

Expand Down
230 changes: 230 additions & 0 deletions clients/cli/__tests__/cliOAuth.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,230 @@
import { describe, it, expect, vi, afterEach } from "vitest";
import { AuthRecoveryRequiredError } from "@inspector/core/auth/challenge.js";
import { MutableRedirectUrlProvider } from "@inspector/core/auth/index.js";
import * as runnerInteractive from "@inspector/core/auth/node/runner-interactive-oauth.js";
import {
handleCliAuthRecoveryRequired,
isStandardOAuthStepUp,
runCliInteractiveOAuth,
withCliAuthRecoveryRetry,
} from "../src/cliOAuth.js";

describe("cliOAuth", () => {
afterEach(() => {
vi.restoreAllMocks();
});

describe("isStandardOAuthStepUp", () => {
it("returns true for insufficient_scope on non-EMA servers", () => {
expect(
isStandardOAuthStepUp(
{ reason: "insufficient_scope", requiredScopes: ["weather:read"] },
{},
),
).toBe(true);
});

it("returns false for EMA servers", () => {
expect(
isStandardOAuthStepUp(
{ reason: "insufficient_scope", requiredScopes: ["weather:read"] },
{ enterpriseManaged: true },
),
).toBe(false);
});
});

it("runCliInteractiveOAuth writes success to stderr", async () => {
vi.spyOn(runnerInteractive, "runRunnerInteractiveOAuth").mockResolvedValue({
kind: "success",
});
const client = {
authenticate: vi.fn(),
beginInteractiveAuthorization: vi.fn(),
completeOAuthFlow: vi.fn(),
checkAuthChallengeSatisfied: vi.fn(),
};
const redirectUrlProvider = new MutableRedirectUrlProvider();
const stderrSpy = vi
.spyOn(process.stderr, "write")
.mockImplementation(() => true);

await runCliInteractiveOAuth(client, redirectUrlProvider, {
hostname: "127.0.0.1",
port: 6276,
pathname: "/oauth/callback",
});

expect(stderrSpy).toHaveBeenCalledWith("Authorization complete.\n");
});

it("runCliInteractiveOAuth throws when scopes remain insufficient", async () => {
const challenge = {
reason: "insufficient_scope" as const,
requiredScopes: ["weather:read"],
};
vi.spyOn(runnerInteractive, "runRunnerInteractiveOAuth").mockResolvedValue({
kind: "insufficient_scope",
challenge,
});
const client = {
authenticate: vi.fn(),
beginInteractiveAuthorization: vi.fn(),
completeOAuthFlow: vi.fn(),
checkAuthChallengeSatisfied: vi.fn(),
};

await expect(
runCliInteractiveOAuth(
client,
new MutableRedirectUrlProvider(),
{ hostname: "127.0.0.1", port: 6276, pathname: "/oauth/callback" },
{ authChallenge: challenge },
),
).rejects.toThrow(/required scopes were not granted/);
});

it("handleCliAuthRecoveryRequired declines standard step-up when user says no", async () => {
const runSpy = vi
.spyOn(runnerInteractive, "runRunnerInteractiveOAuth")
.mockResolvedValue({ kind: "success" });
const client = {
authenticate: vi.fn(),
beginInteractiveAuthorization: vi.fn(),
completeOAuthFlow: vi.fn(),
checkAuthChallengeSatisfied: vi.fn(),
};
const error = new AuthRecoveryRequiredError(
new URL("https://as.example/authorize"),
{ reason: "insufficient_scope", requiredScopes: ["weather:read"] },
);

await expect(
handleCliAuthRecoveryRequired(
client,
error,
new MutableRedirectUrlProvider(),
{ hostname: "127.0.0.1", port: 6276, pathname: "/oauth/callback" },
{},
async () => false,
),
).rejects.toThrow("Step-up authorization declined.");

expect(runSpy).not.toHaveBeenCalled();
});

it("handleCliAuthRecoveryRequired runs OAuth after step-up confirm", async () => {
const runSpy = vi
.spyOn(runnerInteractive, "runRunnerInteractiveOAuth")
.mockResolvedValue({ kind: "success" });
const client = {
authenticate: vi.fn(),
beginInteractiveAuthorization: vi.fn(),
completeOAuthFlow: vi.fn(),
checkAuthChallengeSatisfied: vi.fn(),
};
const authorizationUrl = new URL("https://as.example/authorize");
const challenge = {
reason: "insufficient_scope" as const,
requiredScopes: ["weather:read"],
};
const error = new AuthRecoveryRequiredError(authorizationUrl, challenge);

await handleCliAuthRecoveryRequired(
client,
error,
new MutableRedirectUrlProvider(),
{ hostname: "127.0.0.1", port: 6276, pathname: "/oauth/callback" },
{},
async () => true,
);

expect(runSpy).toHaveBeenCalledWith(
expect.objectContaining({
authorizationUrl,
authChallenge: challenge,
}),
);
});

it("handleCliAuthRecoveryRequired skips step-up when storage already satisfies", async () => {
const runSpy = vi.spyOn(runnerInteractive, "runRunnerInteractiveOAuth");
const client = {
authenticate: vi.fn(),
beginInteractiveAuthorization: vi.fn(),
completeOAuthFlow: vi.fn(),
checkAuthChallengeSatisfied: vi.fn().mockResolvedValue(true),
};
const error = new AuthRecoveryRequiredError(
new URL("https://as.example/authorize"),
{
reason: "insufficient_scope",
requiredScopes: ["weather:read"],
},
);

await handleCliAuthRecoveryRequired(
client,
error,
new MutableRedirectUrlProvider(),
{ hostname: "127.0.0.1", port: 6276, pathname: "/oauth/callback" },
);

expect(runSpy).not.toHaveBeenCalled();
});

it("handleCliAuthRecoveryRequired skips OAuth when storage already satisfies reauth", async () => {
const runSpy = vi.spyOn(runnerInteractive, "runRunnerInteractiveOAuth");
const client = {
authenticate: vi.fn(),
beginInteractiveAuthorization: vi.fn(),
completeOAuthFlow: vi.fn(),
checkAuthChallengeSatisfied: vi.fn().mockResolvedValue(true),
};
const error = new AuthRecoveryRequiredError(
new URL("https://as.example/authorize"),
{ reason: "token_expired" },
);

await handleCliAuthRecoveryRequired(
client,
error,
new MutableRedirectUrlProvider(),
{ hostname: "127.0.0.1", port: 6276, pathname: "/oauth/callback" },
);

expect(runSpy).not.toHaveBeenCalled();
});

it("withCliAuthRecoveryRetry reruns the operation after interactive recovery", async () => {
vi.spyOn(runnerInteractive, "runRunnerInteractiveOAuth").mockResolvedValue({
kind: "success",
});
const client = {
authenticate: vi.fn(),
beginInteractiveAuthorization: vi.fn(),
completeOAuthFlow: vi.fn(),
checkAuthChallengeSatisfied: vi.fn(),
};
const fn = vi
.fn()
.mockRejectedValueOnce(
new AuthRecoveryRequiredError(new URL("https://as.example/authorize"), {
reason: "unauthorized",
}),
)
.mockResolvedValueOnce("ok");

const result = await withCliAuthRecoveryRetry(
client,
new MutableRedirectUrlProvider(),
{ hostname: "127.0.0.1", port: 6276, pathname: "/oauth/callback" },
{ enterpriseManaged: true },
fn,
async () => true,
);

expect(result).toBe("ok");
expect(fn).toHaveBeenCalledTimes(2);
});
});
Loading
Loading