diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 32122205..63b3edef 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -11,6 +11,7 @@ The Inspector ships as one package with three clients (**Web**, **CLI**, **TUI** - **Avoid double casts (`as unknown as T`).** They erase all type safety and usually mean the real type is being worked around. Prefer a type guard, a narrower single cast, or fixing the underlying type. If genuinely unavoidable (a documented gap in a third-party type, or bridging structurally-identical shapes TS can't relate), it **must** carry an inline comment justifying why it's safe and why nothing better exists. An unjustified `as unknown as` is not acceptable in review. - Prefer inference, type guards, and precise annotations over assertions. - An `_` prefix is the intentionally-unused marker (`argsIgnorePattern` / `varsIgnorePattern` / `caughtErrorsIgnorePattern`). +- **Never float a promise.** `@typescript-eslint/no-floating-promises` is `error` in all five ESLint scopes, so every promise must be awaited, `.catch(…)`-terminated, returned, or explicitly discarded with `void`. Prefer **holding and settling** it — a floated rejection surfaces in an unrelated file as an SDK-internal stack, which is how two un-held `callTool` calls failed the whole `npm run ci` chain (#1947). Reach for `void` only when the callee already owns its failures (it ends in a `catch`) or the caller genuinely cannot await (a sync effect body, an Ink key handler), and say which in a comment. In Storybook play functions, `expect(...)` from `storybook/test` is instrumented and returns a promise — it must be awaited. ## React and UI (web client) diff --git a/AGENTS.md b/AGENTS.md index d85e29a0..c7f5e841 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -45,7 +45,13 @@ v2/main/ │ │ # providers.redirectToAuthorization, the only │ │ # seam that sees the SDK-built authorize URL; │ │ # authorization request only, never the token -│ │ # request — #2018) +│ │ # request — #2018; +│ │ # endpointOverrides.ts per-server +│ │ # authorization/token URL overrides — a fetch +│ │ # wrapper that rewrites the discovered AS +│ │ # metadata document, the one seam SDK v2 routes +│ │ # BOTH endpoints through (neither reaches the +│ │ # OAuthClientProvider) — #1906) │ │ ├── browser/ # Browser-side OAuth (sessionStorage, BrowserNavigation) │ │ ├── node/ # Node-side OAuth (NodeOAuthStorage, OAuthCallbackServer, │ │ │ # runner-interactive-oauth loopback callback flow) @@ -746,6 +752,12 @@ The ⚠️ option-deletion hazard, the snapshot rule, and the recovery recipe ab - Use type guards and type assertions to handle potential type mismatches and ensure type safety - Take advantage of TypeScript's advanced features like generics, type aliases, and conditional types to write more expressive and reusable code - Regularly review and refactor TypeScript code to ensure it remains well-structured and adheres to evolving best practices +- **NEVER leave a promise floating.** `@typescript-eslint/no-floating-promises` is enabled at `error` in **all five** ESLint scopes — `clients/{web,cli,tui,launcher}` and the root `core/` + shared gate (#1959). Every promise must be awaited, returned, terminated with `.catch(…)`, or explicitly discarded with the `void` operator. + - **The class it catches is invisible at review time.** A floated call reads like an awaited one minus four characters, and the unhandled rejection it produces surfaces somewhere else entirely — a different test, a different file, a stack pointing at SDK internals. Two un-held `client.callTool(...)` promises made `npm run ci` unpassable in #1947: `disconnect()` rejected them with `Connection closed`, the unhandled rejection failed the whole vitest run, and the chain aborted at `coverage`, silently skipping `verify:build-gate`, `smoke`, and Storybook. Attributing it took a full investigation; the fix was two lines. + - **Prefer holding and settling the promise.** `void` is an escape hatch, not a fix — it is visible at review time (strictly better than nothing) but still discards the rejection. Reach for it only when the callee already owns its failures (it ends in a `catch` that surfaces the message) or the caller genuinely cannot await — a synchronous `useEffect` body, an Ink `useInput` key handler, a Hono `stream.onAbort` listener. Say **which** of those it is in a one-line comment; an unexplained `void` is a review finding. Where the callee does *not* own its failures, give it a `catch` rather than voiding the call (see `handleDisconnect` in `clients/tui/src/App.tsx`), or terminate with `.catch(…)` at the call site (see `open(url)` in `clients/web/server/{server,vite-hono-plugin}.ts`). + - **In Storybook play functions, `expect(...)` from `storybook/test` returns a promise.** Storybook instruments it so the interactions panel can trace each assertion, so every `expect` in a play function is awaited — as is any shared helper that wraps one (`src/test/scrollAreaStoryAssertions.ts` is `async` for this reason). + - **The rule is type-aware, so each scope's ESLint config carries a parser project.** Each client's config must name **every leaf project covering its lint surface**, since the parser needs a program that literally *contains* the file — for cli, tui, and launcher that is the two they already typecheck (`tsconfig.json` + `tsconfig.test.json`, `src` in the first and `__tests__` only in the second), and for **web it is four** (`tsconfig.app.json`, `tsconfig.node.json`, `tsconfig.storybook.json`, `tsconfig.test.json`) — web's `tsconfig.json` is a solution file with `files: []`, so naming it alone would contain nothing. Adding a leaf project to a client means adding it here too. The root config instead points at **`tsconfig.lint.json`**, which exists solely to give the parser a program covering `core/**`, `test-servers/src/**`, and `vitest.shared.mts` — none of which is rooted in a tsconfig of its own. That file emits nothing and gates nothing: type *checking* for those sources stays where it was (`core/` through `clients/web`'s `tsc -b`, `test-servers/src` through `clients/cli`'s test project). It sets `moduleResolution: bundler` deliberately — `core/` uses extensionless relative imports, which NodeNext fails to resolve, degrading every import to `any` so the rule silently stops seeing promises at all. **Its `include` must stay a superset of the root config's type-aware `files` globs** — a file the lint block matches but the project omits fails outright with "was not found in any of the provided project(s)" rather than being checked, so widening one means widening the other (that is why the `include` carries `test-servers/src/**/*.tsx`, which nothing has produced yet). Note also that a tsconfig `include` does **not** expand braces — `core/**/*.{ts,tsx}` matches nothing; list the extensions separately. + - Type-aware linting costs real time: `clients/web`'s `eslint .` went from ~8s to ~19s, and `lint` runs inside `validate`, the fast inner-loop check. That is the price of the guarantee; if it needs reducing later, narrowing the projects each scope loads is the lever, not dropping the rule. ## Web source layout: `src/lib` vs `src/utils` diff --git a/README.md b/README.md index 3642aece..3286f2d9 100644 --- a/README.md +++ b/README.md @@ -30,7 +30,7 @@ inspector/ │ ├── tui/ # TUI client (Ink + React, tsup bundle) │ └── launcher/ # Shared launcher — provides the `mcp-inspector` bin, dispatches to web/cli/tui ├── core/ # Shared code consumed via the `@inspector/core` alias (no package.json) -│ ├── auth/ # OAuth: providers, discovery, storage, mid-session recovery (browser/node/remote backends) +│ ├── auth/ # OAuth: providers, discovery, storage, endpoint overrides, mid-session recovery (browser/node/remote backends) │ ├── client/ # Install-level client config (`client.json`): browser-safe parse/validate + Node load/save, remote backend, secrets │ ├── json/ # JSON + parameter/argument conversion utilities, and the nullable-union │ │ # schema collapse shared by the web and TUI form builders @@ -339,6 +339,8 @@ Each client self-validates from its own folder; the root scripts chain them. The 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. +**Linting is type-aware.** All five ESLint scopes (`clients/{web,cli,tui,launcher}` plus the root `core/` + shared gate) enable `@typescript-eslint/no-floating-promises` at `error`, so a promise that is neither awaited, returned, `.catch(…)`-terminated, nor explicitly discarded with `void` fails `lint` — and therefore `validate` ([#1959](https://github.com/modelcontextprotocol/inspector/issues/1959)). The rule needs type information, so each scope's config names a parser project; the root scope's is **`tsconfig.lint.json`**, a lint-only project covering `core/**`, `test-servers/src/**`, and `vitest.shared.mts`, which have no tsconfig of their own. It emits nothing and changes no typecheck — but a new first-party TS location added to the root lint scope must be added to its `include`. See **TypeScript instructions** in [`AGENTS.md`](./AGENTS.md) for when `void` is acceptable. + 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). ## Publishing diff --git a/clients/cli/eslint.config.js b/clients/cli/eslint.config.js index 9407f997..699b202f 100644 --- a/clients/cli/eslint.config.js +++ b/clients/cli/eslint.config.js @@ -16,4 +16,22 @@ export default defineConfig([ globals: globals.node, }, }, + { + // Type-aware pass for `no-floating-promises` (#1959). The rule needs type + // information, which `tseslint.configs.recommended` does not provide, and + // the parser needs a project that literally contains the linted file — so + // both of this client's tsconfig projects are listed, exactly as + // `npm run typecheck` runs them (`src` is in the first, `__tests__` only + // in the second). + files: ["**/*.ts"], + languageOptions: { + parserOptions: { + project: ["./tsconfig.json", "./tsconfig.test.json"], + tsconfigRootDir: import.meta.dirname, + }, + }, + rules: { + "@typescript-eslint/no-floating-promises": "error", + }, + }, ]); diff --git a/clients/launcher/eslint.config.js b/clients/launcher/eslint.config.js index 535689a1..b13efaba 100644 --- a/clients/launcher/eslint.config.js +++ b/clients/launcher/eslint.config.js @@ -16,4 +16,19 @@ export default defineConfig([ globals: globals.node, }, }, + { + // Type-aware pass for `no-floating-promises` (#1959) — see the CLI config + // for the reasoning. Both tsconfig projects are listed because `src` and + // `__tests__` live in different ones. + files: ["**/*.{ts,mts}"], + languageOptions: { + parserOptions: { + project: ["./tsconfig.json", "./tsconfig.test.json"], + tsconfigRootDir: import.meta.dirname, + }, + }, + rules: { + "@typescript-eslint/no-floating-promises": "error", + }, + }, ]); diff --git a/clients/tui/__tests__/App.test.tsx b/clients/tui/__tests__/App.test.tsx index 1efcd408..a66fe603 100644 --- a/clients/tui/__tests__/App.test.tsx +++ b/clients/tui/__tests__/App.test.tsx @@ -706,6 +706,91 @@ describe("App (foundation)", () => { expect(h.disconnect).toHaveBeenCalled(); }); + it("surfaces a disconnect failure instead of floating the rejection", async () => { + // 'd' is a key handler, so it cannot await handleDisconnect — the handler + // has to own the failure itself or it escapes as an unhandled rejection + // and fails the whole run from somewhere else (#1959). + // The banner is deliberately independent of connection status: a rejected + // disconnect leaves the status "connected", so anything gated on + // `status === "error"` would never be seen. + h.ctrl.status = "connected"; + h.disconnect.mockRejectedValue(new Error("discfail")); + const r = await mount(oneStdio()); + r.stdin.write("d"); + await expectFrame(r, "Disconnect failed: discfail"); + }); + + it("owns a non-Error disconnect rejection too", async () => { + // The catch stringifies a non-Error rejection rather than reading + // `.message` off it; a throw here would escape the same way. + h.ctrl.status = "connected"; + h.disconnect.mockRejectedValue("plainstring"); + const r = await mount(oneStdio()); + r.stdin.write("d"); + await expectFrame(r, "Disconnect failed: plainstring"); + }); + + it("clears a disconnect failure once a retry succeeds", async () => { + // A stale banner would keep reporting a failure the user has since fixed. + h.ctrl.status = "connected"; + h.disconnect.mockRejectedValueOnce(new Error("discfail")); + const r = await mount(oneStdio()); + r.stdin.write("d"); + await expectFrame(r, "Disconnect failed: discfail"); + r.stdin.write("d"); + await waitUntil(() => !(r.lastFrame() ?? "").includes("Disconnect failed")); + expect(r.lastFrame() ?? "").not.toContain("Disconnect failed"); + }); + + it("drops a disconnect rejection that lands after the user switched servers", async () => { + // The stale attempt is the one that usually rejects, so without an + // attempt token server alpha's failure would surface in beta's header. + h.ctrl.status = "connected"; + let rejectDisconnect: (err: Error) => void = () => {}; + h.disconnect.mockImplementationOnce( + () => + new Promise((_resolve, reject) => { + rejectDisconnect = reject; + }), + ); + const r = await mount(stdioServer()); + r.stdin.write("d"); // alpha's disconnect starts, and hangs + await tick(); + await press(r, [DOWN]); // switch to beta + await expectFrame(r, "beta"); + rejectDisconnect(new Error("stale-alpha-failure")); + // Give the rejection every chance to be published before asserting it + // wasn't — a bare tick would pass even with the guard removed. + await waitUntil(() => + (r.lastFrame() ?? "").includes("stale-alpha-failure"), + ); + expect(r.lastFrame() ?? "").not.toContain("stale-alpha-failure"); + }); + + it("drops a stale disconnect rejection across an A → B → A round trip", async () => { + // The server-name check alone passes here: by the time the rejection + // lands, alpha is selected again. Only retiring the attempt token on + // every switch catches it. + h.ctrl.status = "connected"; + let rejectDisconnect: (err: Error) => void = () => {}; + h.disconnect.mockImplementationOnce( + () => + new Promise((_resolve, reject) => { + rejectDisconnect = reject; + }), + ); + const r = await mount(stdioServer()); + r.stdin.write("d"); // alpha's disconnect starts, and hangs + await tick(); + await press(r, [DOWN]); // alpha -> beta + await expectFrame(r, "b.js"); + await press(r, [UP]); // beta -> alpha again + await expectFrame(r, "s.js"); + rejectDisconnect(new Error("round-trip-failure")); + await waitUntil(() => (r.lastFrame() ?? "").includes("round-trip-failure")); + expect(r.lastFrame() ?? "").not.toContain("round-trip-failure"); + }); + it("switches tabs via accelerator keys", async () => { const r = await mount(stdioServer()); await press(r, ["t"]); // tools tab (server is auto-selected) diff --git a/clients/tui/__tests__/tsupConfig.test.ts b/clients/tui/__tests__/tsupConfig.test.ts index a9bfc35f..8c61e9e9 100644 --- a/clients/tui/__tests__/tsupConfig.test.ts +++ b/clients/tui/__tests__/tsupConfig.test.ts @@ -207,7 +207,9 @@ describe("tui tsup config", () => { // the double cast is bridging a real structural gap, not hiding a mismatch. // A hook the patch called but the stub lacks fails as undefined here rather // than passing silently. - inkFormLabelPatch.setup( + // esbuild types `setup` as `void | Promise`; await it so the + // assertions below can't run before an async setup has registered. + await inkFormLabelPatch.setup( build as unknown as Parameters[0], ); diff --git a/clients/tui/eslint.config.js b/clients/tui/eslint.config.js index b4f8a251..d37c6b7d 100644 --- a/clients/tui/eslint.config.js +++ b/clients/tui/eslint.config.js @@ -30,4 +30,19 @@ export default defineConfig([ "react-hooks/exhaustive-deps": "warn", }, }, + { + // Type-aware pass for `no-floating-promises` (#1959) — see the CLI config + // for the reasoning. Both tsconfig projects are listed because `src` and + // `__tests__` live in different ones. + files: ["**/*.{ts,tsx}"], + languageOptions: { + parserOptions: { + project: ["./tsconfig.json", "./tsconfig.test.json"], + tsconfigRootDir: import.meta.dirname, + }, + }, + rules: { + "@typescript-eslint/no-floating-promises": "error", + }, + }, ]); diff --git a/clients/tui/src/App.tsx b/clients/tui/src/App.tsx index 62c9e743..240bbd82 100644 --- a/clients/tui/src/App.tsx +++ b/clients/tui/src/App.tsx @@ -170,6 +170,17 @@ function App({ pendingStepUpRef.current = pendingStepUp; }, [pendingStepUp]); const [connectError, setConnectError] = useState(null); + // Monotonic token for in-flight disconnects — see handleDisconnect. + const disconnectAttemptRef = useRef(0); + // A failed disconnect is deliberately NOT folded into `connectError`. That + // one is only rendered by InfoTab when the status is "error", which a + // rejected disconnect leaves untouched (the status stays "connected"), so + // the message would never be seen — and it feeds + // `connectError ?? inspectorLastError`, where a stale value would go on + // masking a later, real connection error. This renders in the header + // regardless of status and is cleared on the next connect/disconnect + // attempt and on a server switch. + const [disconnectError, setDisconnectError] = useState(null); const oauthInProgressRef = useRef(false); const callbackServerRef = useRef(null); const selectedServerRef = useRef(null); @@ -434,6 +445,13 @@ function App({ useEffect(() => { setOauthStatus("idle"); setOauthMessage(null); + // The header banner is server-scoped, so a failure from the server we just + // left must not be read as this one's. Clearing is not enough: a switch + // away and back (A → B → A) would leave both the attempt token and the + // server name matching again, so the stale rejection would land on the + // re-selected A. Retiring the token on every switch is what closes that. + disconnectAttemptRef.current++; + setDisconnectError(null); const stepUp = pendingStepUpRef.current; if (stepUp && selectedServer && stepUp.serverName !== selectedServer) { setPendingStepUp(null); @@ -757,6 +775,10 @@ function App({ if (!selectedServer || !selectedInspectorClient || !selectedServerConfig) { return; } + // A connect attempt supersedes whatever the last disconnect reported — + // including one still in flight, which is what the counter bump retires. + disconnectAttemptRef.current++; + setDisconnectError(null); const finishConnect = async () => { await connectInspector(); @@ -887,8 +909,31 @@ function App({ // Disconnect handler const handleDisconnect = useCallback(async () => { if (!selectedServer) return; - await disconnectInspector(); - // InspectorClient will update status automatically, and data is preserved + // Claim this attempt before awaiting. A disconnect can still be in flight + // when the user switches servers or presses 'd' again, and it is the + // *stale* one that usually rejects — so the catch below must be able to + // tell "my failure" from "one that has since been superseded", or server + // A's error lands in server B's header. `handleConnect` bumps the same + // counter for the same reason. + const attempt = ++disconnectAttemptRef.current; + const attemptServer = selectedServer; + // Clear first, so a retry that succeeds leaves no stale message behind. + setDisconnectError(null); + try { + await disconnectInspector(); + // InspectorClient will update status automatically, and data is preserved + } catch (err) { + // Nothing above this catches: the only caller is the key handler, which + // cannot await, so without this the rejection escapes unhandled. A + // superseded one is still owned here — just not published. + if ( + disconnectAttemptRef.current !== attempt || + selectedServerRef.current !== attemptServer + ) { + return; + } + setDisconnectError(err instanceof Error ? err.message : String(err)); + } }, [selectedServer, disconnectInspector]); const handleClearOAuth = useCallback(async () => { @@ -1428,12 +1473,15 @@ function App({ input.toLowerCase() === "c" && (inspectorStatus === "disconnected" || inspectorStatus === "error") ) { - handleConnect(); + // Both handlers own their failures internally (each ends in a catch + // that surfaces the message), so there is nothing left for a key + // handler — which cannot await — to do with the promise. + void handleConnect(); } else if ( input.toLowerCase() === "d" && (inspectorStatus === "connected" || inspectorStatus === "connecting") ) { - handleDisconnect(); + void handleDisconnect(); } } }); @@ -1617,6 +1665,11 @@ function App({ OAuth: {oauthMessage} )} + {disconnectError && ( + + Disconnect failed: {disconnectError} + + )} diff --git a/clients/tui/src/components/PromptsTab.tsx b/clients/tui/src/components/PromptsTab.tsx index 4f90c1a7..46db93b5 100644 --- a/clients/tui/src/components/PromptsTab.tsx +++ b/clients/tui/src/components/PromptsTab.tsx @@ -53,8 +53,10 @@ export function PromptsTab({ if (selectedPrompt.arguments && selectedPrompt.arguments.length > 0) { onFetchPrompt(selectedPrompt); } else { - // No arguments, fetch directly - (async () => { + // No arguments, fetch directly. The IIFE catches everything it can + // throw, so there is no rejection for this key handler — which + // cannot await — to own. + void (async () => { try { const invocation = await inspectorClient.getPrompt( selectedPrompt.name, diff --git a/clients/tui/src/components/ResourcesTab.tsx b/clients/tui/src/components/ResourcesTab.tsx index 86ba91ac..4d0fa05d 100644 --- a/clients/tui/src/components/ResourcesTab.tsx +++ b/clients/tui/src/components/ResourcesTab.tsx @@ -186,7 +186,9 @@ export function ResourcesTab({ } }; - fetchContent(); + // An effect body is synchronous, and fetchContent already catches + // everything it can throw, so there is no rejection left to own. + void fetchContent(); }, [shouldFetchResource, inspectorClient, onAuthRecoveryRequired]); const listWidth = Math.floor(width * 0.4); diff --git a/clients/web/eslint.config.js b/clients/web/eslint.config.js index 86602ed3..21726da5 100644 --- a/clients/web/eslint.config.js +++ b/clients/web/eslint.config.js @@ -31,5 +31,32 @@ export default defineConfig([ "react-refresh/only-export-components": "off", }, }, + { + // Type-aware pass for `no-floating-promises` (#1959). The rule needs type + // information, which `tseslint.configs.recommended` does not provide, and + // the parser needs a project that literally contains the linted file — so + // all four leaf projects are listed (`tsconfig.json` itself is a + // solution file with `files: []`, so it contains nothing). + // + // `.d.ts` is excluded: `vitest.shims.d.ts` is pulled in through a `types` + // reference rather than an `include`, so no project owns it, and a + // declaration file cannot float a promise anyway. + files: ["**/*.{ts,tsx}"], + ignores: ["**/*.d.ts"], + languageOptions: { + parserOptions: { + project: [ + "./tsconfig.app.json", + "./tsconfig.node.json", + "./tsconfig.storybook.json", + "./tsconfig.test.json", + ], + tsconfigRootDir: import.meta.dirname, + }, + }, + rules: { + "@typescript-eslint/no-floating-promises": "error", + }, + }, ...storybook.configs["flat/recommended"], ]); diff --git a/clients/web/server/server.ts b/clients/web/server/server.ts index 9f6ff134..fe465530 100644 --- a/clients/web/server/server.ts +++ b/clients/web/server/server.ts @@ -9,7 +9,6 @@ import { readFileSync } from "node:fs"; import { join, dirname } from "node:path"; import { fileURLToPath } from "node:url"; import { randomBytes } from "node:crypto"; -import open from "open"; import { serve } from "@hono/node-server"; import { serveStatic } from "@hono/node-server/serve-static"; import { Hono } from "hono"; @@ -22,6 +21,7 @@ import type { WebServerConfig } from "./web-server-config.js"; import { webServerConfigToInitialPayload, printServerBanner, + openBrowser, } from "./web-server-config.js"; import type { WebServerHandle } from "./types.js"; @@ -125,7 +125,9 @@ export async function startHonoServer( sandboxUrl ?? undefined, ); if (config.autoOpen) { - open(url); + // Never a bare `open(url)`: this callback is synchronous, so a + // rejection would escape it. See openBrowser. + openBrowser(url); } }, ); diff --git a/clients/web/server/vite-hono-plugin.ts b/clients/web/server/vite-hono-plugin.ts index 1331930c..880913e6 100644 --- a/clients/web/server/vite-hono-plugin.ts +++ b/clients/web/server/vite-hono-plugin.ts @@ -9,7 +9,6 @@ import type { IncomingMessage, ServerResponse } from "node:http"; import type { Plugin } from "vite"; -import open from "open"; // Direct leaf import — the `core/mcp/remote/node/index.ts` barrel re-exports // from `../constants.js`, which would otherwise drag the broader // `core/mcp/remote/index.ts` barrel (including `createRemoteLogger`, with its @@ -22,6 +21,7 @@ import type { WebServerConfig } from "./web-server-config.js"; import { webServerConfigToInitialPayload, printServerBanner, + openBrowser, } from "./web-server-config.js"; export function honoMiddlewarePlugin(config: WebServerConfig): Plugin { @@ -114,7 +114,9 @@ export function honoMiddlewarePlugin(config: WebServerConfig): Plugin { ); if (config.autoOpen) { - open(url); + // Never a bare `open(url)`: `logBanner` is synchronous, so a + // rejection would escape it. See openBrowser. + openBrowser(url); } }; diff --git a/clients/web/server/web-server-config.ts b/clients/web/server/web-server-config.ts index 6e77bd28..6bb13be2 100644 --- a/clients/web/server/web-server-config.ts +++ b/clients/web/server/web-server-config.ts @@ -2,6 +2,7 @@ * Config object for the web server (dev and prod). Passed in-process; no env handoff. */ +import open from "open"; import pino from "pino"; import type { Logger } from "pino"; import type { MCPConfig, MCPServerConfig } from "../../../core/mcp/types.ts"; @@ -203,6 +204,24 @@ export function printServerBanner( return url; } +/** + * Launch the user's browser at `url`, reporting a failure rather than throwing. + * + * Both callers — the prod Hono server's `serve()` listen callback and the dev + * plugin's `logBanner` — are synchronous, so a rejection has nowhere to go and + * escapes as an unhandled rejection (#1959). On a headless or otherwise + * browser-less host that rejection is the ordinary outcome, not an edge case, + * and by the time it happens the server is already listening and + * `printServerBanner` has printed a perfectly usable URL. So warn and carry + * on. Shared between the two servers deliberately: two copies of a one-line + * `.catch` is two chances for one of them to drift back to a bare `open(url)`. + */ +export function openBrowser(url: string): void { + open(url).catch((err: unknown) => { + console.warn("Could not open a browser automatically:", err); + }); +} + export interface BuildWebServerConfigOptions { initialMcpConfig?: MCPServerConfig | null; /** Catalog/session file the backend should serve; see {@link WebServerConfig.mcpConfigPath}. */ diff --git a/clients/web/src/App.tsx b/clients/web/src/App.tsx index 72d8bd1f..5837f27f 100644 --- a/clients/web/src/App.tsx +++ b/clients/web/src/App.tsx @@ -64,6 +64,7 @@ import { ResourceSubscriptionsState } from "@inspector/core/mcp/state/resourceSu import { cleanRoots, oauthAuthorizationParamsFromSettings, + oauthEndpointOverridesFromSettings, serializeMcpConfig, } from "@inspector/core/mcp/serverList.js"; import type { ClientConfig } from "@inspector/core/client/types.js"; @@ -2315,12 +2316,16 @@ function App() { const serverAuthorizationParams = savedSettings ? oauthAuthorizationParamsFromSettings(savedSettings) : undefined; + const serverEndpointOverrides = savedSettings + ? oauthEndpointOverridesFromSettings(savedSettings) + : undefined; const oauthFromServer = savedSettings && (savedSettings.oauthClientId || savedSettings.oauthClientSecret || savedSettings.oauthScopes || serverAuthorizationParams || + serverEndpointOverrides || savedSettings.enterpriseManaged) ? { ...(savedSettings.oauthClientId && { @@ -2335,6 +2340,7 @@ function App() { ...(serverAuthorizationParams && { authorizationParams: serverAuthorizationParams, }), + ...serverEndpointOverrides, ...(savedSettings.enterpriseManaged && { enterpriseManaged: true, }), diff --git a/clients/web/src/components/elements/CategoryBadge/CategoryBadge.stories.tsx b/clients/web/src/components/elements/CategoryBadge/CategoryBadge.stories.tsx index f9118c49..efbeab28 100644 --- a/clients/web/src/components/elements/CategoryBadge/CategoryBadge.stories.tsx +++ b/clients/web/src/components/elements/CategoryBadge/CategoryBadge.stories.tsx @@ -14,7 +14,7 @@ export const Transport: Story = { args: { category: "transport" }, play: async ({ canvasElement }) => { const canvas = within(canvasElement); - expect(canvas.getByText("transport")).toBeInTheDocument(); + await expect(canvas.getByText("transport")).toBeInTheDocument(); }, }; @@ -22,6 +22,6 @@ export const Auth: Story = { args: { category: "auth" }, play: async ({ canvasElement }) => { const canvas = within(canvasElement); - expect(canvas.getByText("auth")).toBeInTheDocument(); + await expect(canvas.getByText("auth")).toBeInTheDocument(); }, }; diff --git a/clients/web/src/components/elements/EmbeddableScrollArea/EmbeddableScrollArea.stories.tsx b/clients/web/src/components/elements/EmbeddableScrollArea/EmbeddableScrollArea.stories.tsx index e85f9b65..87da3cdb 100644 --- a/clients/web/src/components/elements/EmbeddableScrollArea/EmbeddableScrollArea.stories.tsx +++ b/clients/web/src/components/elements/EmbeddableScrollArea/EmbeddableScrollArea.stories.tsx @@ -24,7 +24,7 @@ export const FullSize: Story = { ), play: async ({ canvasElement }) => { const canvas = within(canvasElement); - expect(canvas.getByText("Row 1")).toBeInTheDocument(); + await expect(canvas.getByText("Row 1")).toBeInTheDocument(); }, }; @@ -38,6 +38,6 @@ export const Embedded: Story = { ), play: async ({ canvasElement }) => { const canvas = within(canvasElement); - expect(canvas.getByText("Row 1")).toBeInTheDocument(); + await expect(canvas.getByText("Row 1")).toBeInTheDocument(); }, }; diff --git a/clients/web/src/components/elements/ExpandToggle/ExpandToggle.stories.tsx b/clients/web/src/components/elements/ExpandToggle/ExpandToggle.stories.tsx index df4054c8..90369ab1 100644 --- a/clients/web/src/components/elements/ExpandToggle/ExpandToggle.stories.tsx +++ b/clients/web/src/components/elements/ExpandToggle/ExpandToggle.stories.tsx @@ -15,7 +15,9 @@ export const Collapsed: Story = { args: { expanded: false }, play: async ({ canvasElement }) => { const canvas = within(canvasElement); - expect(canvas.getByRole("button", { name: "Expand" })).toBeInTheDocument(); + await expect( + canvas.getByRole("button", { name: "Expand" }), + ).toBeInTheDocument(); }, }; @@ -23,7 +25,7 @@ export const Expanded: Story = { args: { expanded: true }, play: async ({ canvasElement }) => { const canvas = within(canvasElement); - expect( + await expect( canvas.getByRole("button", { name: "Collapse" }), ).toBeInTheDocument(); }, diff --git a/clients/web/src/components/elements/McpErrorBadge/McpErrorBadge.stories.tsx b/clients/web/src/components/elements/McpErrorBadge/McpErrorBadge.stories.tsx index 4616b4fa..c189d654 100644 --- a/clients/web/src/components/elements/McpErrorBadge/McpErrorBadge.stories.tsx +++ b/clients/web/src/components/elements/McpErrorBadge/McpErrorBadge.stories.tsx @@ -14,7 +14,7 @@ export const HeaderMismatch: Story = { args: { code: -32020, name: "HeaderMismatch" }, play: async ({ canvasElement }) => { const canvas = within(canvasElement); - expect(canvas.getByText("-32020 HeaderMismatch")).toBeInTheDocument(); + await expect(canvas.getByText("-32020 HeaderMismatch")).toBeInTheDocument(); }, }; @@ -22,7 +22,7 @@ export const MissingCapability: Story = { args: { code: -32021, name: "MissingRequiredClientCapability" }, play: async ({ canvasElement }) => { const canvas = within(canvasElement); - expect( + await expect( canvas.getByText("-32021 MissingRequiredClientCapability"), ).toBeInTheDocument(); }, @@ -32,7 +32,7 @@ export const UnsupportedVersion: Story = { args: { code: -32022, name: "UnsupportedProtocolVersion" }, play: async ({ canvasElement }) => { const canvas = within(canvasElement); - expect( + await expect( canvas.getByText("-32022 UnsupportedProtocolVersion"), ).toBeInTheDocument(); }, @@ -47,6 +47,6 @@ export const MethodNotFound: Story = { }, play: async ({ canvasElement }) => { const canvas = within(canvasElement); - expect(canvas.getByText("-32601 MethodNotFound")).toBeInTheDocument(); + await expect(canvas.getByText("-32601 MethodNotFound")).toBeInTheDocument(); }, }; diff --git a/clients/web/src/components/elements/MessageDirectionBadge/MessageDirectionBadge.stories.tsx b/clients/web/src/components/elements/MessageDirectionBadge/MessageDirectionBadge.stories.tsx index a4aa55e0..c99b8a12 100644 --- a/clients/web/src/components/elements/MessageDirectionBadge/MessageDirectionBadge.stories.tsx +++ b/clients/web/src/components/elements/MessageDirectionBadge/MessageDirectionBadge.stories.tsx @@ -14,7 +14,7 @@ export const Outgoing: Story = { args: { direction: "outgoing" }, play: async ({ canvasElement }) => { const canvas = within(canvasElement); - expect(canvas.getByText("client → server")).toBeInTheDocument(); + await expect(canvas.getByText("client → server")).toBeInTheDocument(); }, }; @@ -22,6 +22,6 @@ export const Incoming: Story = { args: { direction: "incoming" }, play: async ({ canvasElement }) => { const canvas = within(canvasElement); - expect(canvas.getByText("server → client")).toBeInTheDocument(); + await expect(canvas.getByText("server → client")).toBeInTheDocument(); }, }; diff --git a/clients/web/src/components/elements/MethodBadge/MethodBadge.stories.tsx b/clients/web/src/components/elements/MethodBadge/MethodBadge.stories.tsx index 91e3abb5..312d249a 100644 --- a/clients/web/src/components/elements/MethodBadge/MethodBadge.stories.tsx +++ b/clients/web/src/components/elements/MethodBadge/MethodBadge.stories.tsx @@ -14,7 +14,7 @@ export const ToolsList: Story = { args: { method: "tools/list" }, play: async ({ canvasElement }) => { const canvas = within(canvasElement); - expect(canvas.getByText("tools/list")).toBeInTheDocument(); + await expect(canvas.getByText("tools/list")).toBeInTheDocument(); }, }; @@ -22,6 +22,6 @@ export const TasksList: Story = { args: { method: "tasks/list" }, play: async ({ canvasElement }) => { const canvas = within(canvasElement); - expect(canvas.getByText("tasks/list")).toBeInTheDocument(); + await expect(canvas.getByText("tasks/list")).toBeInTheDocument(); }, }; diff --git a/clients/web/src/components/elements/MonitoringToggle/MonitoringToggle.stories.tsx b/clients/web/src/components/elements/MonitoringToggle/MonitoringToggle.stories.tsx index 9541812e..109a47d8 100644 --- a/clients/web/src/components/elements/MonitoringToggle/MonitoringToggle.stories.tsx +++ b/clients/web/src/components/elements/MonitoringToggle/MonitoringToggle.stories.tsx @@ -15,7 +15,7 @@ export const Closed: Story = { args: { open: false }, play: async ({ canvasElement }) => { const canvas = within(canvasElement); - expect( + await expect( canvas.getByRole("button", { name: "Open monitoring sidebar" }), ).toBeInTheDocument(); }, @@ -25,7 +25,7 @@ export const Open: Story = { args: { open: true }, play: async ({ canvasElement }) => { const canvas = within(canvasElement); - expect( + await expect( canvas.getByRole("button", { name: "Close monitoring sidebar" }), ).toBeInTheDocument(); }, diff --git a/clients/web/src/components/elements/PinToggle/PinToggle.stories.tsx b/clients/web/src/components/elements/PinToggle/PinToggle.stories.tsx index d259eb84..a15900d4 100644 --- a/clients/web/src/components/elements/PinToggle/PinToggle.stories.tsx +++ b/clients/web/src/components/elements/PinToggle/PinToggle.stories.tsx @@ -15,7 +15,9 @@ export const Unpinned: Story = { args: { pinned: false }, play: async ({ canvasElement }) => { const canvas = within(canvasElement); - expect(canvas.getByRole("button", { name: "Pin" })).toBeInTheDocument(); + await expect( + canvas.getByRole("button", { name: "Pin" }), + ).toBeInTheDocument(); }, }; @@ -23,6 +25,8 @@ export const Pinned: Story = { args: { pinned: true }, play: async ({ canvasElement }) => { const canvas = within(canvasElement); - expect(canvas.getByRole("button", { name: "Unpin" })).toBeInTheDocument(); + await expect( + canvas.getByRole("button", { name: "Unpin" }), + ).toBeInTheDocument(); }, }; diff --git a/clients/web/src/components/elements/ReplayButton/ReplayButton.stories.tsx b/clients/web/src/components/elements/ReplayButton/ReplayButton.stories.tsx index 6391f2b4..a7bd9b17 100644 --- a/clients/web/src/components/elements/ReplayButton/ReplayButton.stories.tsx +++ b/clients/web/src/components/elements/ReplayButton/ReplayButton.stories.tsx @@ -14,6 +14,8 @@ type Story = StoryObj; export const Default: Story = { play: async ({ canvasElement }) => { const canvas = within(canvasElement); - expect(canvas.getByRole("button", { name: "Replay" })).toBeInTheDocument(); + await expect( + canvas.getByRole("button", { name: "Replay" }), + ).toBeInTheDocument(); }, }; diff --git a/clients/web/src/components/elements/ResizeHandle/ResizeHandle.stories.tsx b/clients/web/src/components/elements/ResizeHandle/ResizeHandle.stories.tsx index 4fb1c122..7b8072dc 100644 --- a/clients/web/src/components/elements/ResizeHandle/ResizeHandle.stories.tsx +++ b/clients/web/src/components/elements/ResizeHandle/ResizeHandle.stories.tsx @@ -44,7 +44,7 @@ export const Default: Story = { render: () => , play: async ({ canvasElement }) => { const canvas = within(canvasElement); - expect( + await expect( canvas.getByRole("separator", { name: "Resize demo panel" }), ).toBeInTheDocument(); }, diff --git a/clients/web/src/components/elements/ScreenStage/ScreenStage.stories.tsx b/clients/web/src/components/elements/ScreenStage/ScreenStage.stories.tsx index ab215303..c7af29bc 100644 --- a/clients/web/src/components/elements/ScreenStage/ScreenStage.stories.tsx +++ b/clients/web/src/components/elements/ScreenStage/ScreenStage.stories.tsx @@ -48,7 +48,7 @@ export const FillsHostWidth: Story = { // sharper discriminator: a row flex leaves the layer full-width while // collapsing the child to the single "x"). const layer = child.parentElement as HTMLElement; - expect(child.getBoundingClientRect().width).toBeCloseTo( + await expect(child.getBoundingClientRect().width).toBeCloseTo( layer.getBoundingClientRect().width, 0, ); @@ -77,11 +77,11 @@ export const FillVariant: Story = { // Width still stretches onto the child (relative to the layer, as above); // the `fill` height comes from `bottom: 0` making the layer span the host. const layer = child.parentElement as HTMLElement; - expect(child.getBoundingClientRect().width).toBeCloseTo( + await expect(child.getBoundingClientRect().width).toBeCloseTo( layer.getBoundingClientRect().width, 0, ); - expect(layer.getBoundingClientRect().height).toBeGreaterThan(160 - 4); + await expect(layer.getBoundingClientRect().height).toBeGreaterThan(160 - 4); }, }; @@ -97,6 +97,6 @@ export const Inactive: Story = { ), play: async ({ canvasElement }) => { const canvas = within(canvasElement); - expect(canvas.queryByTestId("stage-child")).not.toBeInTheDocument(); + await expect(canvas.queryByTestId("stage-child")).not.toBeInTheDocument(); }, }; diff --git a/clients/web/src/components/groups/ConnectionInfoContent/ConnectionInfoContent.stories.tsx b/clients/web/src/components/groups/ConnectionInfoContent/ConnectionInfoContent.stories.tsx index eaf54cee..50549076 100644 --- a/clients/web/src/components/groups/ConnectionInfoContent/ConnectionInfoContent.stories.tsx +++ b/clients/web/src/components/groups/ConnectionInfoContent/ConnectionInfoContent.stories.tsx @@ -114,8 +114,10 @@ export const ServerInfoNotReported: Story = { }, play: async ({ canvasElement }) => { const canvas = within(canvasElement); - expect(canvas.queryByText("my-catalog-name")).not.toBeInTheDocument(); - expect(canvas.getAllByText(SERVER_INFO_NOT_REPORTED_LABEL)).toHaveLength(2); + await expect(canvas.queryByText("my-catalog-name")).not.toBeInTheDocument(); + await expect( + canvas.getAllByText(SERVER_INFO_NOT_REPORTED_LABEL), + ).toHaveLength(2); }, }; @@ -195,11 +197,11 @@ export const WithOAuthIdToken: Story = { }, play: async ({ canvasElement }) => { const canvas = within(canvasElement); - expect(canvas.getByText("ID Token")).toBeInTheDocument(); + await expect(canvas.getByText("ID Token")).toBeInTheDocument(); // Only the ID token is a JWT here, so exactly one decode toggle is offered. await userEvent.click( canvas.getByRole("button", { name: "Decode JWT for ID Token" }), ); - expect(canvas.getByText(/"sub": "user-42"/)).toBeInTheDocument(); + await expect(canvas.getByText(/"sub": "user-42"/)).toBeInTheDocument(); }, }; diff --git a/clients/web/src/components/groups/LogStreamPanel/LogStreamPanel.stories.tsx b/clients/web/src/components/groups/LogStreamPanel/LogStreamPanel.stories.tsx index 111f54e6..a7545dc3 100644 --- a/clients/web/src/components/groups/LogStreamPanel/LogStreamPanel.stories.tsx +++ b/clients/web/src/components/groups/LogStreamPanel/LogStreamPanel.stories.tsx @@ -77,6 +77,6 @@ export const WithEntries: Story = { }, // List scrollbar reserves a gutter and stays hidden when idle (#1474). play: async ({ canvasElement }) => { - expectScrollbarGutterIdleHidden(canvasElement); + await expectScrollbarGutterIdleHidden(canvasElement); }, }; diff --git a/clients/web/src/components/groups/MessageDirectionFilter/MessageDirectionFilter.stories.tsx b/clients/web/src/components/groups/MessageDirectionFilter/MessageDirectionFilter.stories.tsx index 362b30bd..8296dd63 100644 --- a/clients/web/src/components/groups/MessageDirectionFilter/MessageDirectionFilter.stories.tsx +++ b/clients/web/src/components/groups/MessageDirectionFilter/MessageDirectionFilter.stories.tsx @@ -31,10 +31,10 @@ export const AllVisible: Story = { args: { visibleDirections: { client: true, server: true } }, play: async ({ canvasElement }) => { const canvas = within(canvasElement); - expect( + await expect( canvas.getByRole("button", { name: "client → server" }), ).toBeInTheDocument(); - expect( + await expect( canvas.getByRole("button", { name: "Deselect All" }), ).toBeInTheDocument(); }, diff --git a/clients/web/src/components/groups/MonitoringControls/MonitoringControls.stories.tsx b/clients/web/src/components/groups/MonitoringControls/MonitoringControls.stories.tsx index 3a3eb7ff..eca1065a 100644 --- a/clients/web/src/components/groups/MonitoringControls/MonitoringControls.stories.tsx +++ b/clients/web/src/components/groups/MonitoringControls/MonitoringControls.stories.tsx @@ -20,7 +20,7 @@ type Story = StoryObj; export const AllTabs: Story = { play: async ({ canvasElement }) => { const canvas = within(canvasElement); - expect(canvas.getByRole("radio", { name: "Logs" })).toBeChecked(); + await expect(canvas.getByRole("radio", { name: "Logs" })).toBeChecked(); }, }; @@ -28,6 +28,6 @@ export const StdioServer: Story = { args: { tabs: ["Logs", "Protocol"], value: "Protocol" }, play: async ({ canvasElement }) => { const canvas = within(canvasElement); - expect(canvas.queryByRole("radio", { name: "Network" })).toBeNull(); + await expect(canvas.queryByRole("radio", { name: "Network" })).toBeNull(); }, }; diff --git a/clients/web/src/components/groups/MonitoringScreen/MonitoringScreen.stories.tsx b/clients/web/src/components/groups/MonitoringScreen/MonitoringScreen.stories.tsx index 56c1cca2..b91d5d3b 100644 --- a/clients/web/src/components/groups/MonitoringScreen/MonitoringScreen.stories.tsx +++ b/clients/web/src/components/groups/MonitoringScreen/MonitoringScreen.stories.tsx @@ -47,7 +47,7 @@ type Story = StoryObj; export const Logs: Story = { play: async ({ canvasElement }) => { const canvas = within(canvasElement); - expect(canvas.getByText("Log stream")).toBeInTheDocument(); + await expect(canvas.getByText("Log stream")).toBeInTheDocument(); }, }; @@ -55,6 +55,6 @@ export const Protocol: Story = { args: { value: "Protocol" }, play: async ({ canvasElement }) => { const canvas = within(canvasElement); - expect(canvas.getByText("Request history")).toBeInTheDocument(); + await expect(canvas.getByText("Request history")).toBeInTheDocument(); }, }; diff --git a/clients/web/src/components/groups/NetworkStreamPanel/NetworkStreamPanel.stories.tsx b/clients/web/src/components/groups/NetworkStreamPanel/NetworkStreamPanel.stories.tsx index a4248d1e..e006e261 100644 --- a/clients/web/src/components/groups/NetworkStreamPanel/NetworkStreamPanel.stories.tsx +++ b/clients/web/src/components/groups/NetworkStreamPanel/NetworkStreamPanel.stories.tsx @@ -50,7 +50,7 @@ export const WithEntries: Story = { args: { entries: sample }, // List scrollbar reserves a gutter and stays hidden when idle (#1474). play: async ({ canvasElement }) => { - expectScrollbarGutterIdleHidden(canvasElement); + await expectScrollbarGutterIdleHidden(canvasElement); }, }; diff --git a/clients/web/src/components/groups/OutputValidationModal/OutputValidationModal.stories.tsx b/clients/web/src/components/groups/OutputValidationModal/OutputValidationModal.stories.tsx index 0942c7dd..40544352 100644 --- a/clients/web/src/components/groups/OutputValidationModal/OutputValidationModal.stories.tsx +++ b/clients/web/src/components/groups/OutputValidationModal/OutputValidationModal.stories.tsx @@ -45,14 +45,14 @@ export const Default: Story = { // Mantine renders the modal in a portal at document.body. const body = within(canvasElement.ownerDocument.body); await body.findByText("Output schema validation"); - expect(body.getByText(/open_pattern_editor/)).toBeInTheDocument(); + await expect(body.getByText(/open_pattern_editor/)).toBeInTheDocument(); const details = body.getByLabelText( "Validation details", ) as HTMLTextAreaElement; - expect(details.value).toContain( + await expect(details.value).toContain( "data/samples/0 must NOT have additional properties", ); - expect(details.readOnly).toBe(true); + await expect(details.readOnly).toBe(true); }, }; diff --git a/clients/web/src/components/groups/PendingClientRequestModal/PendingClientRequestModal.stories.tsx b/clients/web/src/components/groups/PendingClientRequestModal/PendingClientRequestModal.stories.tsx index 5130e98c..ee81a687 100644 --- a/clients/web/src/components/groups/PendingClientRequestModal/PendingClientRequestModal.stories.tsx +++ b/clients/web/src/components/groups/PendingClientRequestModal/PendingClientRequestModal.stories.tsx @@ -122,11 +122,11 @@ export const Sampling: Story = { play: async ({ canvasElement, args }) => { const body = within(canvasElement.ownerDocument.body); await body.findByText("Sampling Request"); - expect( + await expect( body.getByText("The server is requesting an LLM completion."), ).toBeInTheDocument(); await userEvent.click(body.getByRole("button", { name: "Send Response" })); - expect(args.onSamplingRespond).toHaveBeenCalled(); + await expect(args.onSamplingRespond).toHaveBeenCalled(); }, }; @@ -144,8 +144,8 @@ export const SamplingInputRequired: Story = { play: async ({ canvasElement }) => { const body = within(canvasElement.ownerDocument.body); await body.findByText("Sampling Request"); - expect(body.getByText("input_required")).toBeInTheDocument(); - expect(body.getByText(/sent back as a retry/i)).toBeInTheDocument(); + await expect(body.getByText("input_required")).toBeInTheDocument(); + await expect(body.getByText(/sent back as a retry/i)).toBeInTheDocument(); }, }; @@ -172,11 +172,11 @@ export const ElicitationForm: Story = { play: async ({ canvasElement, args }) => { const body = within(canvasElement.ownerDocument.body); await body.findByText("Elicitation Request"); - expect( + await expect( body.getByText(/Please provide your database connection details/), ).toBeInTheDocument(); await userEvent.click(body.getByRole("button", { name: "Submit" })); - expect(args.onElicitationRespond).toHaveBeenCalledWith({ + await expect(args.onElicitationRespond).toHaveBeenCalledWith({ action: "accept", content: {}, }); @@ -207,7 +207,7 @@ export const ElicitationUrl: Story = { play: async ({ canvasElement }) => { const body = within(canvasElement.ownerDocument.body); await body.findByText("Elicitation Request"); - expect( + await expect( body.getByText("https://example.com/authorize?token=abc123"), ).toBeInTheDocument(); }, diff --git a/clients/web/src/components/groups/ProtocolListPanel/ProtocolListPanel.stories.tsx b/clients/web/src/components/groups/ProtocolListPanel/ProtocolListPanel.stories.tsx index 2e1dfe8d..861f23c2 100644 --- a/clients/web/src/components/groups/ProtocolListPanel/ProtocolListPanel.stories.tsx +++ b/clients/web/src/components/groups/ProtocolListPanel/ProtocolListPanel.stories.tsx @@ -107,7 +107,7 @@ export const WithEntries: Story = { // bar never overlays the cards, and uses type="scroll" so it stays hidden // when idle rather than appearing on hover (#1474). play: async ({ canvasElement }) => { - expectScrollbarGutterIdleHidden(canvasElement); + await expectScrollbarGutterIdleHidden(canvasElement); }, }; diff --git a/clients/web/src/components/groups/ResourceControls/ResourceControls.stories.tsx b/clients/web/src/components/groups/ResourceControls/ResourceControls.stories.tsx index 958ec6aa..e7cd4f90 100644 --- a/clients/web/src/components/groups/ResourceControls/ResourceControls.stories.tsx +++ b/clients/web/src/components/groups/ResourceControls/ResourceControls.stories.tsx @@ -86,8 +86,8 @@ export const Default: Story = { // chevron points down. play: async ({ canvasElement }) => { const transforms = chevronTransforms(canvasElement); - expect(transforms).toHaveLength(3); - for (const t of transforms) expect(t).toBe(ROTATED_DOWN); + await expect(transforms).toHaveLength(3); + for (const t of transforms) await expect(t).toBe(ROTATED_DOWN); }, }; @@ -122,9 +122,9 @@ export const Collapsed: Story = { }, play: async ({ canvasElement }) => { const transforms = chevronTransforms(canvasElement); - expect(transforms).toHaveLength(3); + await expect(transforms).toHaveLength(3); // Identity matrix = the right-pointing arrow's natural 0° (not rotated). - for (const t of transforms) expect(t).toBe(NOT_ROTATED); + for (const t of transforms) await expect(t).toBe(NOT_ROTATED); }, }; @@ -140,10 +140,10 @@ export const EmptySectionCollapsed: Story = { }, play: async ({ canvasElement }) => { const canvas = within(canvasElement); - expect(canvas.getByText("Subscriptions (0)")).toBeInTheDocument(); + await expect(canvas.getByText("Subscriptions (0)")).toBeInTheDocument(); const transforms = chevronTransforms(canvasElement); const downCount = transforms.filter((t) => t === ROTATED_DOWN).length; - expect(downCount).toBe(2); // URIs + Templates down, Subscriptions right + await expect(downCount).toBe(2); // URIs + Templates down, Subscriptions right }, }; @@ -158,11 +158,11 @@ export const SubscriptionsUnsupported: Story = { }, play: async ({ canvasElement }) => { const canvas = within(canvasElement); - expect(canvas.queryByText(/Subscriptions/)).not.toBeInTheDocument(); + await expect(canvas.queryByText(/Subscriptions/)).not.toBeInTheDocument(); // Only the two remaining sections render, both open → both chevrons down. const transforms = chevronTransforms(canvasElement); - expect(transforms).toHaveLength(2); - for (const t of transforms) expect(t).toBe(ROTATED_DOWN); + await expect(transforms).toHaveLength(2); + for (const t of transforms) await expect(t).toBe(ROTATED_DOWN); }, }; diff --git a/clients/web/src/components/groups/ResourceLink/ResourceLink.stories.tsx b/clients/web/src/components/groups/ResourceLink/ResourceLink.stories.tsx index 655a5e7a..ac6b6ca2 100644 --- a/clients/web/src/components/groups/ResourceLink/ResourceLink.stories.tsx +++ b/clients/web/src/components/groups/ResourceLink/ResourceLink.stories.tsx @@ -45,10 +45,12 @@ export const Static: Story = { }, play: async ({ canvasElement }) => { const canvas = within(canvasElement); - expect(canvas.getByText(URI)).toBeInTheDocument(); + await expect(canvas.getByText(URI)).toBeInTheDocument(); // Copy button is present; there's no expand control in the static card. - expect(canvas.getByRole("button", { name: "Copy" })).toBeInTheDocument(); - expect( + await expect( + canvas.getByRole("button", { name: "Copy" }), + ).toBeInTheDocument(); + await expect( canvas.queryByRole("button", { name: /^Expand resource/ }), ).not.toBeInTheDocument(); }, diff --git a/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.test.tsx b/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.test.tsx index d238228a..09ded171 100644 --- a/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.test.tsx +++ b/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.test.tsx @@ -701,6 +701,8 @@ describe("ServerSettingsForm", () => { clientSecret: "", scopes: "", authorizationParams: [], + authorizationUrl: "", + tokenUrl: "", enterpriseManaged: false, }); }); @@ -966,6 +968,104 @@ describe("ServerSettingsForm", () => { expect(screen.getByText(/state, scope are set/)).toBeInTheDocument(); }); + // #1906 — the endpoint overrides ride the same `onOAuthChange` callback as + // the rest of the Authorization section. + it("invokes onOAuthChange when an endpoint override is typed", async () => { + const user = userEvent.setup(); + const onOAuthChange = vi.fn(); + renderWithMantine( + , + ); + await user.type( + screen.getByRole("textbox", { name: /Authorization URL override/i }), + "h", + ); + expect(onOAuthChange).toHaveBeenCalledWith( + expect.objectContaining({ authorizationUrl: "h" }), + ); + + onOAuthChange.mockClear(); + await user.type( + screen.getByRole("textbox", { name: /Token URL override/i }), + "h", + ); + expect(onOAuthChange).toHaveBeenCalledWith( + expect.objectContaining({ tokenUrl: "h" }), + ); + }); + + it("says the endpoint overrides are unused under enterprise-managed auth", () => { + const { rerender } = renderWithMantine( + , + ); + expect( + screen.getAllByText( + /Not applied while Enterprise-managed authorization is on/, + ), + ).toHaveLength(2); + + rerender( + , + ); + expect( + screen.queryByText( + /Not applied while Enterprise-managed authorization is on/, + ), + ).not.toBeInTheDocument(); + }); + + it("flags an endpoint override that is not an absolute http(s) URL", () => { + renderWithMantine( + , + ); + expect( + screen.getByText('"/authorize" is not an absolute URL.'), + ).toBeInTheDocument(); + expect(screen.queryByText(/is not an http\(s\) URL/)).toBeNull(); + }); + + it("clears an endpoint override through its clear button", async () => { + const user = userEvent.setup(); + const onOAuthChange = vi.fn(); + renderWithMantine( + , + ); + const clearButtons = screen.getAllByRole("button", { name: /clear/i }); + await user.click(clearButtons[clearButtons.length - 1]); + expect(onOAuthChange).toHaveBeenCalledWith( + expect.objectContaining({ tokenUrl: "" }), + ); + }); + it("invokes onOAuthChange with the chosen insufficient-scope policy (SEP-2350)", async () => { const user = userEvent.setup(); const onOAuthChange = vi.fn(); @@ -1007,6 +1107,8 @@ describe("ServerSettingsForm", () => { clientSecret: "", scopes: "", authorizationParams: [], + authorizationUrl: "", + tokenUrl: "", enterpriseManaged: true, }); }); @@ -1112,6 +1214,8 @@ describe("ServerSettingsForm", () => { clientSecret: "z", scopes: "", authorizationParams: [], + authorizationUrl: "", + tokenUrl: "", enterpriseManaged: false, }); }); diff --git a/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.tsx b/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.tsx index 276c9cf9..b2a786c6 100644 --- a/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.tsx +++ b/clients/web/src/components/groups/ServerSettingsForm/ServerSettingsForm.tsx @@ -32,6 +32,7 @@ import { authorizationParamKeyError, isReservedAuthorizationParam, } from "@inspector/core/auth/authorizationParams.js"; +import { oauthEndpointUrlError } from "@inspector/core/auth/endpointOverrides.js"; import { ADVERTISABLE_EXTENSIONS } from "@inspector/core/mcp/extensions.js"; import type { Root } from "@modelcontextprotocol/client"; @@ -478,11 +479,20 @@ export function ServerSettingsForm({ clientSecret: settings.oauthClientSecret ?? "", scopes: settings.oauthScopes ?? "", authorizationParams, + authorizationUrl: settings.oauthAuthorizationUrl ?? "", + tokenUrl: settings.oauthTokenUrl ?? "", enterpriseManaged: settings.enterpriseManaged ?? false, onInsufficientScope: settings.oauthOnInsufficientScope, }; } + // #1906 — the overrides are suppressed under EMA for the same reason the + // custom authorization parameters are: that leg authorizes against the + // enterprise IdP, a different authorization server. + const endpointOverrideEmaNote = settings.enterpriseManaged + ? " Not applied while Enterprise-managed authorization is on: that flow authorizes against the enterprise IdP, a different authorization server." + : ""; + const rejectedParamKeys = authorizationParams .map((p) => p.key.trim()) .filter((key) => isReservedAuthorizationParam(key)); @@ -863,6 +873,55 @@ export function ServerSettingsForm({ + Add Parameter + + onOAuthChange({ + ...currentOAuth(), + authorizationUrl: e.currentTarget.value, + }) + } + rightSection={ + settings.oauthAuthorizationUrl ? ( + + onOAuthChange({ + ...currentOAuth(), + authorizationUrl: "", + }) + } + /> + ) : null + } + /> + + onOAuthChange({ + ...currentOAuth(), + tokenUrl: e.currentTarget.value, + }) + } + rightSection={ + settings.oauthTokenUrl ? ( + + onOAuthChange({ ...currentOAuth(), tokenUrl: "" }) + } + /> + ) : null + } + />