Merge pull request #2054 from modelcontextprotocol/v2/chore/milestone-merge-v2.3.0
Some checks failed
Run install, format, lint, build, and test on every push / build (push) Has been cancelled
Run install, format, lint, build, and test on every push / publish (push) Has been cancelled
Run install, format, lint, build, and test on every push / publish-github-container-registry (push) Has been cancelled
Some checks failed
Run install, format, lint, build, and test on every push / build (push) Has been cancelled
Run install, format, lint, build, and test on every push / publish (push) Has been cancelled
Run install, format, lint, build, and test on every push / publish-github-container-registry (push) Has been cancelled
chore: merge v2/main for the v2.3.0 milestone release
This commit is contained in:
commit
86d5f58ec8
216 changed files with 18187 additions and 2078 deletions
1
.gitattributes
vendored
1
.gitattributes
vendored
|
|
@ -1 +1,2 @@
|
|||
* text=auto eol=lf
|
||||
package-lock.json linguist-generated=true
|
||||
|
|
|
|||
4
.github/copilot-instructions.md
vendored
4
.github/copilot-instructions.md
vendored
|
|
@ -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)
|
||||
|
||||
|
|
@ -100,8 +101,9 @@ Both exist and do different jobs. Theme files (`src/theme/<Component>.ts`) custo
|
|||
## Gates and PR hygiene
|
||||
|
||||
- `npm run format` before committing; **`npm run ci` before pushing** (`validate` → `coverage` → `verify:build-gate` → `smoke` → Storybook). `npm run validate` is the fast inner-loop check and is **not** a substitute — it runs `test`, not `test:coverage`, so it does zero coverage gating.
|
||||
- **A dependency bump must land in every install that declares it.** v2 is not a workspace — the root and each `clients/*` have their own `node_modules`, and a client's test project compiles `core/` and `test-servers/src` (which resolve from the **root**) alongside the client's own sources. Bumping a shared dependency in one manifest only puts two versions of it in one `tsc` program; for a recursive-generic surface like zod that exhausts the tsc heap (#1896). `verify:dep-lockstep` fails the build on this, so a PR bumping a package that the shared sources import should update the root **and every client that already lists it** — not every client unconditionally, since a package absent from an install can't skew and adding it there would be a spurious dependency.
|
||||
- **A dependency bump must land in every install that declares it.** v2 is not a workspace — the root and each `clients/*` have their own `node_modules`, and a client's test project compiles `core/` and `test-servers/src` (which resolve from the **root**) alongside the client's own sources. Bumping a shared dependency in one manifest only puts two versions of it in one `tsc` program; for a recursive-generic surface like zod that exhausts the tsc heap (#1896). `verify:dep-lockstep` fails the build on this — it derives its candidates from what each `tsc` program actually resolves (`tsc --listFilesOnly`, keeping packages that reach one program from two installs), so a package reached only through another package's `.d.ts` counts too (#1965) — so a PR bumping a package the shared sources pull in should update the root **and every client that already lists it** — not every client unconditionally, since a package absent from an install can't skew and adding it there would be a spurious dependency.
|
||||
- **A test or smoke must not touch real user state.** The web smokes run against a throwaway catalog via the shared `scripts/lib/prod-web-server.mjs` helper, never the developer's `~/.mcp-inspector/mcp.json` (#1977); the cli/tui smokes drive a temp `--catalog`. A new smoke spawning its own server, or teardown that removes a work dir without first awaiting `stopChild` (the #1801 race — `child-cleanup.mjs` exports both halves and both are required), should be flagged.
|
||||
- **Build output is never a gate target.** Lint, format, and typecheck read first-party source only; everything a build writes (`clients/*/build`, `clients/web/dist`, `storybook-static`, `coverage`, `test-servers/build`, `core/**/{build,dist}`, `*.tsbuildinfo`) stays out via each scope's `globalIgnores`, `format` globs, and tsconfig `include`. Gating generated code reports defects in vendored third-party source that nobody can fix, and a rule promotion turns that warning into a `validate` failure (#2043). Flag a PR that adds a build location without ignoring it in the same change, that widens an ignore to silence a finding in first-party code, or that adds a build directory to a tsconfig `include` to make a generated `.d.ts` resolve. Note the coverage guards don't catch this — they assert source is *covered*, not that output is *excluded*.
|
||||
- **Every PR references an issue**, first body line `Closes #<ISSUE_NUMBER>`.
|
||||
- **Every PR carries exactly one version label**, `v1` or `v2`, matching its base branch.
|
||||
- **Commits carry a `Signed-off-by:` trailer.** The DCO check is a hard merge gate and fails on a single unsigned commit; it matches the trailer against the author *or* committer, and skips only merge and bot commits. Use `git commit -s` — note `format.signOff` does *not* sign `git commit` (only `format-patch`). Repairing pushed commits means `git rebase HEAD~<n> --signoff` + `git push --force-with-lease`; remediation commits are not enabled on this repo.
|
||||
|
|
|
|||
16
.github/workflows/main.yml
vendored
16
.github/workflows/main.yml
vendored
|
|
@ -23,10 +23,10 @@ jobs:
|
|||
|
||||
steps:
|
||||
- name: Checkout code
|
||||
uses: actions/checkout@v6
|
||||
uses: actions/checkout@v7
|
||||
|
||||
- name: Setup Node.js
|
||||
uses: actions/setup-node@v6
|
||||
uses: actions/setup-node@v7
|
||||
with:
|
||||
node-version: '22.x'
|
||||
cache: 'npm'
|
||||
|
|
@ -79,7 +79,7 @@ jobs:
|
|||
# smoke reuse the cache instead of downloading its own copy. The Storybook
|
||||
# step below reuses the same install.
|
||||
- name: Cache Playwright browsers
|
||||
uses: actions/cache@v4
|
||||
uses: actions/cache@v6
|
||||
with:
|
||||
path: ~/.cache/ms-playwright
|
||||
key: playwright-${{ runner.os }}-${{ hashFiles('clients/web/package-lock.json') }}
|
||||
|
|
@ -129,10 +129,10 @@ jobs:
|
|||
id-token: write
|
||||
steps:
|
||||
- name: Checkout code
|
||||
uses: actions/checkout@v6
|
||||
uses: actions/checkout@v7
|
||||
|
||||
- name: Setup Node.js
|
||||
uses: actions/setup-node@v6
|
||||
uses: actions/setup-node@v7
|
||||
with:
|
||||
node-version: '22.x'
|
||||
cache: 'npm'
|
||||
|
|
@ -217,7 +217,7 @@ jobs:
|
|||
id-token: write
|
||||
steps:
|
||||
- name: Checkout code
|
||||
uses: actions/checkout@v6
|
||||
uses: actions/checkout@v7
|
||||
|
||||
- name: Log in to the Container registry
|
||||
uses: docker/login-action@v4
|
||||
|
|
@ -241,14 +241,14 @@ jobs:
|
|||
latest=true
|
||||
|
||||
- name: Set up QEMU
|
||||
uses: docker/setup-qemu-action@v3
|
||||
uses: docker/setup-qemu-action@v4
|
||||
|
||||
- name: Set up Docker Buildx
|
||||
uses: docker/setup-buildx-action@v4
|
||||
|
||||
- name: Build and push Docker image
|
||||
id: push
|
||||
uses: docker/build-push-action@v6
|
||||
uses: docker/build-push-action@v7
|
||||
with:
|
||||
context: .
|
||||
push: true
|
||||
|
|
|
|||
107
AGENTS.md
107
AGENTS.md
File diff suppressed because one or more lines are too long
|
|
@ -31,8 +31,12 @@ RUN npm install -g /tmp/inspector.tgz && rm /tmp/inspector.tgz
|
|||
ENV HOST=0.0.0.0 \
|
||||
DANGEROUSLY_BIND_ALL_INTERFACES=true \
|
||||
CLIENT_PORT=6274 \
|
||||
MCP_SANDBOX_PORT=6275 \
|
||||
MCP_AUTO_OPEN_ENABLED=false
|
||||
EXPOSE 6274
|
||||
# 6275 is the MCP Apps sandbox, a second listener the browser reaches directly.
|
||||
# It is only needed for the Apps tab, so it is EXPOSEd but publishing it is
|
||||
# optional — see the Docker section of the root README.
|
||||
EXPOSE 6274 6275
|
||||
|
||||
# Run as the non-root `node` user the base image ships. The inspector resolves
|
||||
# its runtime-state dir (default catalog, OAuth token storage) from `HOME`
|
||||
|
|
|
|||
116
README.md
116
README.md
|
|
@ -30,11 +30,13 @@ 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
|
||||
│ ├── json/ # JSON + parameter/argument conversion utilities, and the nullable-union
|
||||
│ │ # schema collapse shared by the web and TUI form builders
|
||||
│ ├── logging/ # Silent pino logger singleton
|
||||
│ ├── mcp/ # InspectorClient runtime, state stores, transports, config import
|
||||
│ ├── mcp/ # InspectorClient runtime, state stores, transports, config import,
|
||||
│ │ # and the RFC 6570 URI-template helpers the web form and TUI expand through
|
||||
│ ├── node/ # Node-only shared helpers: version reader, hostUrl (host normalize/canonicalize + all-interfaces/loopback detection)
|
||||
│ ├── react/ # React hooks over the state stores
|
||||
│ └── storage/ # File I/O helpers for the OAuth persist backends
|
||||
|
|
@ -69,7 +71,7 @@ npm install # root install; postinstall cascades into every client
|
|||
|
||||
The cascade (`scripts/install-clients.mjs`) is dev-only — it exits early when the package is installed as a dependency, and the published tarball ships only each client's `build/`, so end users are unaffected. Set `INSPECTOR_SKIP_CLIENT_INSTALL=1` to skip it.
|
||||
|
||||
**Where a dependency is declared.** The MCP SDK packages (`@modelcontextprotocol/client`, `core`, `server`, `server-legacy`, `ext-apps`) live in the **root** `package.json` only — never in a client's. Node resolution walks up, so the root install is on every client's chain, and the root manifest is already what the published tarball resolves against. Declaring them per client installs a second copy that can drift from the root's, which is how two versions of `ext-apps` (and of the transitive v1 `@modelcontextprotocol/sdk`) ended up in the tree before [#1970](https://github.com/modelcontextprotocol/inspector/issues/1970) — and a second copy of `client`/`core` is the failure `vitest.shared.mts` carries a `dedupe` workaround for. The same root-only placement holds for anything reached solely through root-owned code with no manifest of its own (`test-servers/src`, `core/`), and `vitest.shared.mts` aliases those to the repo root — `express` and `yaml`, both reached through `test-servers/src`, are the two today. **Whether such a package is a `dependency` or a `devDependency` follows from who consumes it at runtime, not from where it is declared:** anything `core/` imports at runtime must be a root **`dependency`**, because the client builds externalize npm packages and a published install resolves them from the root manifest, where devDependencies are absent. `express` is test-only and is a devDependency; `yaml` currently sits in `dependencies`.
|
||||
**Where a dependency is declared.** The MCP SDK packages (`@modelcontextprotocol/client`, `core`, `server`, `server-legacy`, `ext-apps`) live in the **root** `package.json` only — never in a client's. Node resolution walks up, so the root install is on every client's chain, and the root manifest is already what the published tarball resolves against. Declaring them per client installs a second copy that can drift from the root's, which is how two versions of `ext-apps` (and of the transitive v1 `@modelcontextprotocol/sdk`) ended up in the tree before [#1970](https://github.com/modelcontextprotocol/inspector/issues/1970) — and a second copy of `client`/`core` is the failure `vitest.shared.mts` carries a `dedupe` workaround for. The same root-only placement holds for anything reached solely through root-owned code with no manifest of its own (`test-servers/src`, `core/`), and `vitest.shared.mts` aliases those to the repo root — `express` and `yaml`, both reached through `test-servers/src`, are the two today. **Whether such a package is a `dependency` or a `devDependency` follows from who consumes it at runtime, not from where it is declared:** anything `core/` imports at runtime must be a root **`dependency`**, because the client builds externalize npm packages and a published install resolves them from the root manifest, where devDependencies are absent. `express` is test-only and is a devDependency; `yaml` currently sits in `dependencies`. **`vite` and `@vitejs/plugin-react` are root `dependencies` for the same reason, not by mistake** — they look like build tooling, but `clients/web/server/start-vite-dev-server.ts` imports them at runtime for `mcp-inspector --web --dev`, and `clients/web/tsup.runner.config.ts` lists both as `external`, so a published install resolves them from the root manifest. Moving them to `devDependencies` would break `--web --dev` for consumers (and the on-demand `vite build` in `ensure-web-build.ts`) while passing every local check. It does mean they show up under `npm audit --omit=dev`, which is a feature: they really are in the production tree.
|
||||
|
||||
## Running during development
|
||||
|
||||
|
|
@ -139,12 +141,14 @@ Each config below is a ready-made server for exercising one feature by hand. Loa
|
|||
| ----------------------------------------- | -------------------------------------------------- | ---------------------------------------------------------------------- |
|
||||
| `mcp-app-http.json` **(legacy era)** | An MCP App (UI resource + app tool) in the Apps tab | [#1859](https://github.com/modelcontextprotocol/inspector/issues/1859) |
|
||||
| `modern-mrtr-http.json` | A single MRTR round-trip | — |
|
||||
| `mrtr-showcase-http.json` | Every MRTR preset in one server | — |
|
||||
| `mrtr-showcase-http.json` | Every MRTR preset in one server | [#1860](https://github.com/modelcontextprotocol/inspector/issues/1860) |
|
||||
| `modern-network-http.json` | Network tab: `Mcp-*` headers + error taxonomy | [#1628](https://github.com/modelcontextprotocol/inspector/issues/1628) |
|
||||
| `xmcpheader-modern-http.json` | Tools tab: `x-mcp-header` mirroring and exclusions | [#1632](https://github.com/modelcontextprotocol/inspector/issues/1632) |
|
||||
| `pagination-http.json` | Page-by-page list fetching | [#1721](https://github.com/modelcontextprotocol/inspector/issues/1721) |
|
||||
| `structured-output-http.json` | Tools tab: a result's `structuredContent` section | [#1908](https://github.com/modelcontextprotocol/inspector/issues/1908) |
|
||||
| `duplicate-tool-names-http.json` | A `tools/list` that repeats a tool name | [#1957](https://github.com/modelcontextprotocol/inspector/issues/1957) |
|
||||
| `nullable-fields-http.json` | Tools tab: nullable (`anyOf` + `null`) arguments | [#1928](https://github.com/modelcontextprotocol/inspector/issues/1928) |
|
||||
| `rfc6570-templates-http.json` | Resources tab: RFC 6570 resource-template expansion | [#1919](https://github.com/modelcontextprotocol/inspector/issues/1919) |
|
||||
| `advertised-extensions-http.json` | Tool registration gated on advertised extensions | [#1739](https://github.com/modelcontextprotocol/inspector/issues/1739) |
|
||||
| `logging-{legacy,modern}-http.json` | Logging, both eras | [#1629](https://github.com/modelcontextprotocol/inspector/issues/1629) |
|
||||
| `subscriptions-{legacy,modern}-http.json` | Resource subscriptions, both eras | [#1630](https://github.com/modelcontextprotocol/inspector/issues/1630) |
|
||||
|
|
@ -173,8 +177,21 @@ The Inspector drives MRTR manually (`inputRequired: { autoFulfill: false }`), so
|
|||
| `mrtr_sample` | Embedded sampling → the Sampling panel |
|
||||
| `mrtr_roots` | Embedded `roots/list`, auto-answered silently from configured roots (no modal) |
|
||||
| `mrtr_edge` | An `inputRequests`-only round, then a `requestState`-only round |
|
||||
| `mrtr_empty` | Completes with an empty result — no `content`, no `structuredContent` |
|
||||
| `mrtr_loop` | Never completes → trips the `MRTR_MAX_ROUNDS` bound |
|
||||
|
||||
Run `mrtr_empty` and answer its single elicitation: the Protocol tab groups the
|
||||
exchange as an MRTR conversation ending **COMPLETE**, and the Results panel says
|
||||
**"Empty result — The tool call completed successfully and returned no content."**
|
||||
On the broken build that same result rendered as **"No results yet"**, the panel's
|
||||
pre-run placeholder ([#1860](https://github.com/modelcontextprotocol/inspector/issues/1860)) —
|
||||
so a call the user had just watched succeed read as a call that never ran. An
|
||||
empty `content` array with no `structuredContent` is a legal `CallToolResult`,
|
||||
and the panel only ever mounts once a result exists, so the placeholder wording
|
||||
could not be true there. (The neighbouring half of the same gap — a result whose
|
||||
payload lives only in `structuredContent` — was closed by
|
||||
[#1908](https://github.com/modelcontextprotocol/inspector/issues/1908).)
|
||||
|
||||
> The legacy `collect_elicitation` preset calls `server.elicitInput`, which errors on the 2026-07-28 leg — server→client requests aren't allowed there. MRTR is the modern replacement.
|
||||
|
||||
#### Network tab — standardized headers and error taxonomy
|
||||
|
|
@ -227,6 +244,46 @@ Connect (default legacy era), open the Tools tab, and type `get` into **Search t
|
|||
|
||||
The duplicated copies are appended rather than placed beside their twin on purpose. React matches a leading run of same-key children first, so a head-adjacent duplicate happens to line up and the defect hides; separating the pair is what makes it observable — and it is also the realistic shape, two tool sources concatenated.
|
||||
|
||||
#### Nullable arguments
|
||||
|
||||
`nullable-fields-http.json` serves `record_shipment`, whose four arguments are each declared with Zod's `.nullish()` — "optional **and** explicitly nullable". That compiles to `anyOf: [<branch>, { "type": "null" }]`, so the real type (and, for the enum, its `enum` list) sits on a branch rather than at the top level. `get_temp` sits alongside it with a plain, non-nullable `units` enum for comparison. Plain streamable-HTTP — connect with the **default (legacy)** protocol era.
|
||||
|
||||
Open the Tools tab and select `record_shipment`: `direction` must render as a **Select** (`envio` / `recebimento`) with a clear button that sets it back to `null`, `reference` as a text input, `quantity` as a number input, and `express` as a checkbox. On the broken build every one of them fell through to the raw-JSON textarea, which re-escaped its own contents on each keystroke until the value was unusable ([#1928](https://github.com/modelcontextprotocol/inspector/issues/1928)). The tool echoes the arguments it received, so the result panel shows exactly what was sent.
|
||||
|
||||
The **TUI** had the same gap and is worth checking against the same server (`--tui`, then test `record_shipment`): `direction` is a select, `quantity` an integer field, `express` a boolean. Both clients now share one collapse step — `normalizeNullableUnion` in [`core/json/nullableUnion.ts`](./core/json/nullableUnion.ts) — precisely so they cannot drift on which schemas they can render.
|
||||
|
||||
#### RFC 6570 resource templates
|
||||
|
||||
`rfc6570-templates-http.json` serves two resource templates straight out of [#1919](https://github.com/modelcontextprotocol/inspector/issues/1919) — `events_by_topic` (`foobar://events/{topic}`) and `events_by_query` (`foobar://events{?topic}`) — each echoing the URI it was matched against, plus a plain `foobar://events` resource (see below). Plain streamable-HTTP; connect with the **default (legacy)** protocol era.
|
||||
|
||||
Open the Resources tab and pick **events_by_topic**, then enter `foo/bar`. The request must go out as `foobar://events/foo%2Fbar`, and the result echoes back the URI the server matched. On the broken build the value was spliced in raw, so the slash created a second path segment and the SDK's matcher answered `-32602 Resource not found: foobar://events/foo/bar` — the exact failure in the issue. The same holds for `?`, `#`, `%`, spaces, and non-ASCII text.
|
||||
|
||||
**events_by_query** is the half that was invisible: the old `/\{(\w+)\}/g` scan could not see an expression carrying an operator, so no `topic` input was rendered at all. It now appears, marked **Optional** — RFC 6570 drops the whole expression when the variable is undefined, so reading with the field blank requests `foobar://events`, and filling it in requests `foobar://events?topic=foo%2Fbar`. The URI preview beside the title shows the partially-expanded form as you type, leaving unfilled expressions standing as written.
|
||||
|
||||
> The plain `foobar://events` resource is registered deliberately, not as filler. The SDK's `UriTemplate.match()` compiles `{?topic}` to a **required** `\?topic=([^&]+)`, so a template alone cannot serve the blank read — `match("foobar://events")` returns `null`. A real server exposes the unfiltered collection as its own resource; the showcase does the same so that step actually resolves.
|
||||
|
||||
The web client and the TUI expand through one shared helper, [`core/mcp/uriTemplate.ts`](./core/mcp/uriTemplate.ts) — the web Resources form directly, the TUI via `InspectorClient.readResourceFromTemplate` — and both derive their **form fields** from its parser too, which is the half that makes the sharing real: a form submits values under the names it rendered, so a parser that mangles a name silently drops the value at expansion time. (The CLI is not a consumer: it has no template form, and its `resources/read` passes the already-expanded `--uri` straight through.)
|
||||
|
||||
The SDK's `UriTemplate` is still used, but only to *validate* a template (constructing it is what rejects an unclosed expression). Its expander is not, because it is incomplete in five ways — each measured against the pinned SDK, not inferred:
|
||||
|
||||
| Shape | SDK behavior |
|
||||
| --- | --- |
|
||||
| `{a,b}` | raw-joins the values — no encoding, operator prefix dropped |
|
||||
| `{;id}` | `;` is missing from its operator list, so the variable parses as `;id` |
|
||||
| `{id:3}` | the prefix modifier is folded into the name, giving `id:3` |
|
||||
| `{+v}` / `{#v}` | `encodeURI` mangles reserved `[`/`]` (`[::1]` → `%5B::1%5D`) and double-encodes pct-triplets (`%2F` → `%252F`) |
|
||||
| `{v}` | `encodeURIComponent` leaves the sub-delims `!'()*` bare, which RFC 6570 requires encoded |
|
||||
|
||||
The `;` and `:3` rows are the ones a user sees directly: on the SDK's parse the form renders fields literally labelled `;id` and `id:3`. The `+`/`#` row is silent corruption rather than over-escaping — an IPv6 literal or an already-encoded path arrives at the server altered.
|
||||
|
||||
A template that cannot be expanded at all — an out-of-grammar modifier (`{id:abc}`), or an expression declaring no variable (`{}`, `{a,}`, `{?}`) — **withholds the read** rather than sending something. Pick **events_malformed** (`foobar://events/{topic:abc}`) to see it: Read Resource is disabled, the reason is printed under the form, and the preview shows the template as the server declared it. The alternative is worse than it looks: `x://{}` would otherwise expand to `x://` with no inputs rendered, so the form's "everything required is filled" check passes vacuously and it reads a URI that is not the template the server published.
|
||||
|
||||
Literals are pct-encoded on expansion too (RFC 6570 §3.1): `café/{var}` sends `caf%C3%A9/value`, not raw UTF-8 in the path — something the SDK's expander does not do either. And the *names* a template may use are RFC 6570's `varchar` plus a labelled tolerance for `-` and `~`: the conformance suite rejects `{default-graph-uri}`, but real servers publish such names and the SDK's matcher round-trips them, so the Inspector expands them and marks the variable `conforming: false` rather than refusing a resource that demonstrably works.
|
||||
|
||||
An **undefined** variable is what omits its expression — a variable defined as the empty string expands (`x{?q}` gives `x?q=`, `x{;q}` gives `x;q`, per RFC 6570 §3.2.7). The expander honors that distinction, so a caller such as `readResourceFromTemplate` can request either URI. Collapsing the two is a *form* concern, not a template one: both clients seed every declared variable with `""` and a text input cannot express "defined but empty", so each form drops its blanks (`definedValues`) on the way in.
|
||||
|
||||
Requiredness is a property of the **expression**, not the variable: RFC 6570 drops undefined names from a multi-name expression, so `{a,b}` with only `a` filled is expandable and a form must not block it. `requiredGroups` returns one entry per non-omittable expression and `hasRequiredValues` asks that each be satisfied by any one of its names — which no per-variable flag can express once a name recurs across expressions (`{a,b}{a,c}` is satisfied by filling `b` and `c`).
|
||||
|
||||
#### Advertised extensions
|
||||
|
||||
`advertised-extensions-http.json` serves `echo` (always) and a `get_weather` tool **gated on the `io.modelcontextprotocol/tasks` extension** (`extensionGatedTools`): the tool is registered but starts disabled, and the server enables it on `notifications/initialized` only when the client declared that extension in its `capabilities.extensions`.
|
||||
|
|
@ -284,19 +341,21 @@ Each client self-validates from its own folder; the root scripts chain them. The
|
|||
|
||||
| Script | What it does |
|
||||
| ----------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
|
||||
| `npm run validate` | Runs the three durable guards first — `verify:format-coverage` (every tracked source file is format-gated), `verify:typecheck-coverage` (every one lands in a tsconfig project), `verify:dep-lockstep` (no dependency the shared sources directly import skews across installs) — then `test:scripts` (the guards' own parser unit tests), then `validate:core` (the shared `core/` `format:check` + `lint` gate), then per client: `format:check` + `lint` + **`typecheck`** (cli/tui/launcher; web typechecks via `tsc -b` inside its `build`) + `build` + fast unit tests. The quick inner-loop check. |
|
||||
| `npm run validate` | Runs the three durable guards first — `verify:format-coverage` (every tracked source file is format-gated), `verify:typecheck-coverage` (every one lands in a tsconfig project), `verify:dep-lockstep` (no dependency reaching one `tsc` program from two installs skews across them) — then `test:scripts` (the guards' own parser unit tests), then `validate:core` (the shared `core/` `format:check` + `lint` gate), then per client: `format:check` + `lint` + **`typecheck`** (cli/tui/launcher; web typechecks via `tsc -b` inside its `build`) + `build` + fast unit tests. The quick inner-loop check. |
|
||||
| `npm run coverage` | The **per-file ≥90% gate** (lines/statements/functions/branches) under v8 instrumentation, per client. CI-enforced. For web this also runs the integration project and covers the shared `core/` runtime (including `core/json` and `core/client`). |
|
||||
| `npm run smoke` | End-to-end smokes through the built launcher (`--help` dispatch + prod cli/tui/web), plus two headless-Chromium smokes: a boot smoke that runs the prod web bundle and asserts a clean first render (no uncaught error — sync exception or unhandled rejection, how a Node built-in reaching the browser bundle manifests), and an **MCP Apps** smoke (`smoke:web:app`) that drives connect → open app → `data-app-status="ready"` against a composable App server, covering the sandbox proxy and UI-protocol bridge. |
|
||||
| `npm run verify:build-gate` | Runs a real `vite build` with a Node built-in forced into the browser graph and asserts the build **fails** via the #1769 gate (which turns Vite's browser-externalization warning into a hard error). Guards against the warning phrasing drifting in a Vite bump and silently disabling the gate. Part of `npm run ci`. |
|
||||
| `npm run verify:format-coverage` | Parses the `format:check` globs out of every `package.json` (only those reachable from `validate`), enumerates all tracked source files, and **fails** listing any not covered by a glob — the durable guard for the "every first-party source file is format-gated" invariant (#1792). Runs first in `validate`. |
|
||||
| `npm run test:scripts` | Table-driven unit tests (`node --test`) for the guard's own pure parsers (`scripts/lib/npm-scripts.mjs` + the exported helpers of `verify-typecheck-coverage.mjs` and `verify-dep-lockstep.mjs`), one case per rule they encode. Runs in `validate` — and `verify:typecheck-coverage` guards *this* gate in turn (reachable from `validate`, non-empty test set, every test file matched by the `test:scripts` glob), since `node --test` silently skips a file its glob misses and still exits 0. |
|
||||
| `npm run test:scripts` | Table-driven unit tests (`node --test`) for the guard's own pure parsers (`scripts/lib/npm-scripts.mjs`, `scripts/lib/tsc-program.mjs` + the exported helpers of `verify-typecheck-coverage.mjs` and `verify-dep-lockstep.mjs`), one case per rule they encode, plus `scripts/lib/resolve-node-bin.test.mjs` — the cross-platform bin resolver (#1939), pinned against the real `bin`/`exports` shapes of the packages the scripts actually spawn. Runs in `validate` — and `verify:typecheck-coverage` guards *this* gate in turn (reachable from `validate`, non-empty test set, every test file matched by the `test:scripts` glob), since `node --test` silently skips a file its glob misses and still exits 0. |
|
||||
| `npm run verify:typecheck-coverage` | The typecheck-coverage analog of the above (#1791): for each Node client (auto-discovered from disk — enrolled via its `typecheck` script's projects, or for a `tsc -b` client like `clients/web` via its `tsconfig.json` `references`) it runs those projects with `tsc --listFilesOnly`, unions them, and **fails** listing any tracked `.ts`/`.tsx`/`.mts`/`.cts` under the client that lands in no project (so a new top-level config/helper can't silently go untypechecked). It also requires, deny-by-default, the first-party TS no client owns (`test-servers/src`, the root `vitest.shared.mts`, all of `core/`, and any new top-level location) to land in some client project's tsc pass — so a `core` `*.tsx` web's projects don't reach is caught too. Also asserts the gate is wired (each client's typecheck pass — its `typecheck` script, or web's `tsc -b` — is reachable from its `validate`, and the root chain runs each client's `validate`). Runs in `validate`. |
|
||||
| `npm run verify:dep-lockstep` | Guards the "one version per install-crossing dependency" invariant (#1896). v2 is not a workspace, so a client's test project compiles the shared first-party TypeScript — `core/`, `test-servers/src`, and the root-owned `vitest.shared.mts`, all of which resolve their dependencies from the **root** install — alongside the client's own sources, putting the same package in one `tsc` program twice. At the same version that's harmless; skewed, TypeScript must relate two structurally-distinct copies of every type, which for a recursive-generic surface is exponential (zod `4.3.6` vs `4.4.3` exhausted the 4GB tsc heap in `clients/web`). Derives its candidate set from what those shared sources import, compares the committed lockfiles' top-level entries, and **fails deny-by-default** on any skew not in the annotated `TOLERATED_SKEW` allowlist — and an allowlisted package is tolerated only *within a major version*. Runs in `validate`. |
|
||||
| `npm run verify:dep-lockstep` | Guards the "one version per install-crossing dependency" invariant (#1896). v2 is not a workspace, so a client's test project compiles the shared first-party TypeScript — `core/`, `test-servers/src`, and the root-owned `vitest.shared.mts`, all of which resolve their dependencies from the **root** install — alongside the client's own sources, putting the same package in one `tsc` program twice. At the same version that's harmless; skewed, TypeScript must relate two structurally-distinct copies of every type, which for a recursive-generic surface is exponential (zod `4.3.6` vs `4.4.3` exhausted the 4GB tsc heap in `clients/web`). Derives its candidate set from **what actually enters each program** (#1965) — every client tsconfig project listed with `tsc --listFilesOnly` via the shared `scripts/lib/tsc-program.mjs`, each resolved `node_modules` file mapped to its owning install, keeping the packages that reach one program from two installs (a package whose declarations arrive only through another package's `.d.ts`, as `@modelcontextprotocol/sdk`'s do, is invisible to a scan of first-party imports). Prices each copy from the lockfile entry for the exact install path the program resolved, compares only the installs that met in one program, and **fails deny-by-default** on any disagreement not in the annotated `TOLERATED_SKEW` allowlist — empty today — with an allowlisted package tolerated only *within a major version*. Runs in `validate`.
|
||||
| `npm run ci` | **Mandatory pre-push command.** `validate` → `coverage` → `verify:build-gate` → `smoke` → Storybook. A true superset of GitHub CI. |
|
||||
| `npm run pack:verify` | Publish smoke — see [Publishing](#publishing). |
|
||||
|
||||
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
|
||||
|
|
@ -326,16 +385,42 @@ Publishing is automated by two release-gated jobs in [`.github/workflows/main.ym
|
|||
|
||||
A v2 release is cut from **`main`**, after the milestone's work has been merged there from `v2/main` — not from `v2/main` itself. (The v1 line releases independently from `v1/main` to the `v1-latest` tag and never touches `main`; see [Repo status](#mcp-inspector).)
|
||||
|
||||
Because there is **one version number** (only the root `package.json` has one — the clients carry none, so there is nothing to keep in sync and no `check-version` step), the release flow is just:
|
||||
Because there is **one version number** (only the root `package.json` has one — the clients carry none, so there is nothing to keep in sync and no `check-version` step), the release flow is three steps.
|
||||
|
||||
**1. Bump on `v2/main`, before the milestone merge.** The bump is part of the milestone's work, so it belongs on the develop branch and flows into `main` with everything else:
|
||||
|
||||
Substitute the real issue number and release below — the commands are written to be copy-pasteable as-is (a `2.2.0` → `2.3.0` minor bump):
|
||||
|
||||
```bash
|
||||
npm version <major|minor|patch> # bumps the root package.json + tags
|
||||
git push --follow-tags
|
||||
git checkout -b v2/chore/2010-bump-2-3-0 v2/main
|
||||
npm version minor --no-git-tag-version # or major / patch; bump only, no tag
|
||||
# PR → v2/main
|
||||
```
|
||||
|
||||
⚠️ **`--no-git-tag-version` is load-bearing.** A bare `npm version` also tags, and the tag would land on a `v2/main` commit — but the release must be cut from `main`, so the tag has to point at the merge commit there (step 3). Tagging here creates a tag on a commit that is never released.
|
||||
|
||||
**2. Merge `v2/main` → `main`** through the usual milestone-merge branch. It now carries the bump, so the release lands on `main` with the version already correct.
|
||||
|
||||
Between steps 1 and 2 the two branches **do** differ, and that is expected, not drift: `v2/main` reads the version being built while `main` still reads the one currently released. What this ordering removes is *post-release* drift — once the milestone merge lands they agree again, and `v2/main` is never left **behind** `main`. If you see `v2/main` ahead of `main`, a release is in flight; if you see it behind, something went wrong.
|
||||
|
||||
**3. Tag the `main` commit and draft the Release:**
|
||||
|
||||
```bash
|
||||
git fetch origin main
|
||||
git tag 2.3.0 origin/main && git push origin 2.3.0
|
||||
# then draft & publish a GitHub Release for that tag → triggers `publish`
|
||||
```
|
||||
|
||||
⚠️ **Tag `origin/main`, not your local `HEAD`.** `git checkout main && git pull` resolves through whatever merge-or-rebase strategy you have configured, so a divergent local `main` can quietly produce or replay local commits. Tagging `HEAD` there tags a commit that is not on `origin/main`, and `git push origin <tag>` pushes only the tag — leaving a release whose commit was never published. Naming `origin/main` explicitly makes the tagged commit exactly what the remote branch points at, regardless of local state.
|
||||
|
||||
⚠️ **No `v` prefix.** This repo's release tags are bare `x.y.z` — `2.2.0`, `2.1.0`, `2.0.0` — so tag `2.3.0`, not `v2.3.0`. Note npm's own `tag-version-prefix` defaults to `v` and the repo sets no `.npmrc`, so a bare `npm version` would have produced a `v`-prefixed tag that does not match the convention. Tagging by hand (step 3) is what keeps it right. The workflow's assert step strips a leading `v` before comparing, so a `v`-prefixed tag would still publish — it would just be inconsistent with every previous release.
|
||||
|
||||
The release's target commit selects which workflow runs, so this only publishes when a release is cut from a commit carrying this (v2) workflow.
|
||||
|
||||
**Why the bump goes on `v2/main` first ([#2010](https://github.com/modelcontextprotocol/inspector/issues/2010)).** It used to happen on the milestone-merge branch, which is cut from `main` — so the bump existed only *downstream* of `v2/main` and nothing carried it back. `v2/main` sat at `2.0.0` through both the 2.1.0 and 2.2.0 releases. That is not cosmetic: a branch cut from a milestone-merge branch silently carries the bump into an unrelated PR (this happened on [#2009](https://github.com/modelcontextprotocol/inspector/issues/2009), where a container bugfix arrived with a `2.0.0 → 2.2.0` diff), and anything reading the version in development — `readInspectorVersion()`, `--version`, `GET /api/config` — reported a version two releases old.
|
||||
|
||||
Do **not** "fix" a future drift by merging `main` back into `v2/main`. `main` carries the entire pre-v2 v1 history (retained through `ec5d8e13 chore: replace main's tree with v2` — ~230 commits `v2/main` does not have), so a back-merge grafts all of it into the develop branch's log permanently in order to deliver a two-file change. Bumping first means there is nothing to back-merge.
|
||||
|
||||
### Docker
|
||||
|
||||
A container image is published to GHCR (`ghcr.io/modelcontextprotocol/inspector`, `linux/amd64` + `linux/arm64`) by the release workflow. The [`Dockerfile`](Dockerfile) is a two-stage build: the first stage installs and `npm pack`s the publishable tarball; the second stage `npm install -g`s that tarball, so the image ships the exact same artifact as npm, with a clean `mcp-inspector` bin.
|
||||
|
|
@ -349,6 +434,15 @@ docker build -t mcp-inspector .
|
|||
docker run --rm -p 127.0.0.1:6274:6274 mcp-inspector
|
||||
```
|
||||
|
||||
**Using the Apps tab? Publish `6275` too.** The MCP Apps sandbox is a second listener the browser reaches directly, on `MCP_SANDBOX_PORT` (default `6275`). Nothing else needs it, so the single-port commands above are fine for ordinary inspection — but the Apps tab renders a blank widget without it:
|
||||
|
||||
```bash
|
||||
docker run --rm -p 127.0.0.1:6274:6274 -p 127.0.0.1:6275:6275 \
|
||||
ghcr.io/modelcontextprotocol/inspector
|
||||
```
|
||||
|
||||
Publish it on the **same port number** inside and out. The sandbox URL is handed to the browser via `/api/config` as `http://localhost:<container port>/sandbox`, so remapping it (`-p 9000:6275`) advertises a port the browser can't reach; use `-e MCP_SANDBOX_PORT=9000 -p 127.0.0.1:9000:9000` instead.
|
||||
|
||||
**Keep the `127.0.0.1:` prefix on the published port.** A bare `-p 6274:6274` publishes on **every host interface**, putting the Inspector on your local network. The container's `HOST=0.0.0.0` is a separate concern — it governs the _container's_ interfaces, not the host's — so the `DANGEROUSLY_BIND_ALL_INTERFACES` opt-in that guards a wildcard bind outside a container does not cover this. It matters more here than for an ordinary web app: the backend spawns processes on request, `GET /` embeds the API token into the served HTML, and a request arriving with **no** `Origin` header skips the origin allow-list entirely — so for any non-browser client the API token is the only guard. Publishing wider needs a real access-control boundary in front of the Inspector — a reverse proxy that authenticates, an SSH tunnel, a private network. Setting your own `MCP_INSPECTOR_API_TOKEN` does **not** substitute: `GET /` discloses whatever token is in use, so a custom one is harvested exactly as easily as a generated one.
|
||||
|
||||
**Keeping the servers you add.** The Inspector saves your server list to `$HOME/.mcp-inspector/mcp.json`, which in the image is `/home/node/.mcp-inspector/mcp.json` — inside the container's writable layer, so `--rm` discards it and every run starts with an empty list. Mount a volume there to keep it:
|
||||
|
|
|
|||
|
|
@ -258,18 +258,29 @@ describe("runMethod", () => {
|
|||
);
|
||||
expect(stdout.trim().split("\n")).toHaveLength(2);
|
||||
|
||||
// Invoke only the SIGINT handler consumeMethodOutcome registers, rather
|
||||
// than broadcasting `process.emit("SIGINT")` process-wide: `atomically`
|
||||
// (loaded via core's store-io) pulls in `when-exit`, whose module-level
|
||||
// `process.once("SIGINT")` handler re-raises the signal with
|
||||
// `process.kill(process.pid, "SIGINT")` — on Windows an unconditional
|
||||
// TerminateProcess that kills the vitest worker fork mid-run (#1941).
|
||||
const listenersBefore = new Set(process.listeners("SIGINT"));
|
||||
const streamPromise = consumeMethodOutcome(
|
||||
{
|
||||
kind: "stream",
|
||||
label: "t",
|
||||
start: (write) => {
|
||||
write({ hi: true });
|
||||
queueMicrotask(() => process.emit("SIGINT"));
|
||||
return () => {};
|
||||
},
|
||||
},
|
||||
{},
|
||||
);
|
||||
const added = process
|
||||
.listeners("SIGINT")
|
||||
.filter((listener) => !listenersBefore.has(listener));
|
||||
expect(added).toHaveLength(1);
|
||||
for (const listener of added) listener("SIGINT");
|
||||
await streamPromise;
|
||||
expect(stdout).toContain('"hi":true');
|
||||
} finally {
|
||||
|
|
|
|||
|
|
@ -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",
|
||||
},
|
||||
},
|
||||
]);
|
||||
|
|
|
|||
6
clients/cli/package-lock.json
generated
6
clients/cli/package-lock.json
generated
|
|
@ -3558,9 +3558,9 @@
|
|||
}
|
||||
},
|
||||
"node_modules/nanoid": {
|
||||
"version": "3.3.16",
|
||||
"resolved": "https://registry.npmjs.org/nanoid/-/nanoid-3.3.16.tgz",
|
||||
"integrity": "sha512-bzlKTyNJ7+LdGIIwy8ijFpIqEQIvafahV7eYykJ8Cvh42EdJeODoJ6gUJXpQJvej1BddH8OqTXZNE/KfbWAu8Q==",
|
||||
"version": "3.3.18",
|
||||
"resolved": "https://registry.npmjs.org/nanoid/-/nanoid-3.3.18.tgz",
|
||||
"integrity": "sha512-DTg4MJbGMWkfi6VZFdNt2/caMbQy4Ou+Op/hJQvGEWcnVfoA1QA+xzRKAzw9jD6+GVOOeYr/mIcuDSdug6F6+w==",
|
||||
"dev": true,
|
||||
"funding": [
|
||||
{
|
||||
|
|
|
|||
|
|
@ -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",
|
||||
},
|
||||
},
|
||||
]);
|
||||
|
|
|
|||
6
clients/launcher/package-lock.json
generated
6
clients/launcher/package-lock.json
generated
|
|
@ -1959,9 +1959,9 @@
|
|||
"license": "MIT"
|
||||
},
|
||||
"node_modules/nanoid": {
|
||||
"version": "3.3.16",
|
||||
"resolved": "https://registry.npmjs.org/nanoid/-/nanoid-3.3.16.tgz",
|
||||
"integrity": "sha512-bzlKTyNJ7+LdGIIwy8ijFpIqEQIvafahV7eYykJ8Cvh42EdJeODoJ6gUJXpQJvej1BddH8OqTXZNE/KfbWAu8Q==",
|
||||
"version": "3.3.18",
|
||||
"resolved": "https://registry.npmjs.org/nanoid/-/nanoid-3.3.18.tgz",
|
||||
"integrity": "sha512-DTg4MJbGMWkfi6VZFdNt2/caMbQy4Ou+Op/hJQvGEWcnVfoA1QA+xzRKAzw9jD6+GVOOeYr/mIcuDSdug6F6+w==",
|
||||
"dev": true,
|
||||
"funding": [
|
||||
{
|
||||
|
|
|
|||
|
|
@ -507,6 +507,9 @@ function renderApp(servers: Record<string, TuiServer>) {
|
|||
async function mount(servers: Record<string, TuiServer>) {
|
||||
const r = renderApp(servers);
|
||||
await tick();
|
||||
// The mount commit's effect flush can still be queued behind this tick, and
|
||||
// this write is the one that absorbs the dropped first keypress.
|
||||
await settleInputHandlers();
|
||||
r.stdin.write("x");
|
||||
await tick();
|
||||
return r;
|
||||
|
|
@ -532,6 +535,11 @@ const ENTER = "\r";
|
|||
*/
|
||||
async function press(r: RenderResult, keys: string[]) {
|
||||
for (const k of keys) {
|
||||
// Same hazard `waitUntil` guards against, at the other end: a caller can
|
||||
// reach here on a turn that still has React's passive-effect flush queued
|
||||
// (e.g. straight after a plain `tick`, or after an earlier key committed a
|
||||
// render), so settle before every write rather than only after a poll.
|
||||
await settleInputHandlers();
|
||||
r.stdin.write(k);
|
||||
await tick();
|
||||
await tick();
|
||||
|
|
@ -552,9 +560,29 @@ async function press(r: RenderResult, keys: string[]) {
|
|||
*/
|
||||
const POLL_TRIES = 100;
|
||||
|
||||
/**
|
||||
* One check-phase turn, queued BEHIND React's already-scheduled passive-effect
|
||||
* flush. A frame observed by a poll predicate is written during React's
|
||||
* COMMIT, but ink re-arms its useInput listeners in the passive-effect flush
|
||||
* React schedules (via setImmediate in Node) during that same commit. Node's
|
||||
* event loop runs the timers phase before the check phase, so a 25ms poll
|
||||
* tick can observe the new frame and let the test write the next keypress
|
||||
* BEFORE that flush has run — the key is then dispatched to the previous
|
||||
* commit's stale useInput closures (where e.g. pendingStepUp is still null)
|
||||
* and silently swallowed (#1942). Yielding one setImmediate turn after the
|
||||
* predicate passes sequences the next stdin write after the flush (FIFO
|
||||
* within the check queue), so "frame visible" once again implies "input
|
||||
* handlers armed".
|
||||
*/
|
||||
const settleInputHandlers = () =>
|
||||
new Promise((resolve) => setImmediate(resolve));
|
||||
|
||||
async function waitUntil(predicate: () => boolean, tries = POLL_TRIES) {
|
||||
for (let i = 0; i < tries; i++) {
|
||||
if (predicate()) return;
|
||||
if (predicate()) {
|
||||
await settleInputHandlers();
|
||||
return;
|
||||
}
|
||||
await tick();
|
||||
}
|
||||
}
|
||||
|
|
@ -626,6 +654,23 @@ afterEach(() => {
|
|||
while (mounted.length) mounted.pop()?.unmount();
|
||||
});
|
||||
|
||||
// Pins the synchronization contract the OAuth step-up assertions depend on
|
||||
// (#1942). The `setImmediate` sentinel below stands in for React's pending
|
||||
// passive-effect flush — the turn where ink re-arms `useInput`. If `waitUntil`
|
||||
// ever returns without yielding a check-phase turn, the sentinel has not run
|
||||
// and this fails, instead of the regression resurfacing as a differently-named
|
||||
// flaky OAuth test under coverage instrumentation.
|
||||
describe("test helpers", () => {
|
||||
it("waitUntil settles input handlers before resolving", async () => {
|
||||
let flushed = false;
|
||||
setImmediate(() => {
|
||||
flushed = true;
|
||||
});
|
||||
await waitUntil(() => true);
|
||||
expect(flushed).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe("App (foundation)", () => {
|
||||
it("renders the server list with the MCP Servers header", async () => {
|
||||
const r = renderApp(stdioServer());
|
||||
|
|
@ -661,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<void>((_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<void>((_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)
|
||||
|
|
|
|||
|
|
@ -77,6 +77,73 @@ const renderAndSubmit = async (
|
|||
};
|
||||
|
||||
describe("ResourceTestModal", () => {
|
||||
// RFC 6570 keeps a required expression satisfied by any ONE of its names, so
|
||||
// `{a,b}` cannot be expressed with ink-form's per-field `required` flag and
|
||||
// its members are left optional there. Without the modal's own group check
|
||||
// the TUI would submit blank -- dropping the expression and reading a
|
||||
// different resource -- while the web panel blocks the same request (#1919).
|
||||
describe("required-group validation", () => {
|
||||
it("refuses to submit when no name in a required group has a value", async () => {
|
||||
const read = vi.fn();
|
||||
const { onClose, unmount } = await renderAndSubmit(
|
||||
fakeClient(read),
|
||||
makeTemplate({ uriTemplate: "x://{a,b}" }),
|
||||
{ a: "", b: "" },
|
||||
);
|
||||
expect(read).not.toHaveBeenCalled();
|
||||
expect(onClose).not.toHaveBeenCalled();
|
||||
unmount();
|
||||
});
|
||||
|
||||
it("blocks a required variable whose name collides with Object.prototype", async () => {
|
||||
// `constructor` is a valid RFC 6570 varname, and the modal's own filter
|
||||
// used to read `Object` (length 1) as a filled value. The *message* built
|
||||
// from that filter came out naming no field; since frames render empty
|
||||
// here (see the header note), that half is asserted on
|
||||
// `unmetRequiredGroups` in the core suite. This asserts the gate.
|
||||
const read = vi.fn();
|
||||
const { onClose, unmount } = await renderAndSubmit(
|
||||
fakeClient(read),
|
||||
makeTemplate({ uriTemplate: "x://{constructor}" }),
|
||||
{ constructor: "" },
|
||||
);
|
||||
expect(read).not.toHaveBeenCalled();
|
||||
expect(onClose).not.toHaveBeenCalled();
|
||||
unmount();
|
||||
});
|
||||
|
||||
it("submits once any one name in the group has a value", async () => {
|
||||
const read = vi.fn().mockResolvedValue({
|
||||
result: { contents: [] },
|
||||
expandedUri: "x://only-a",
|
||||
});
|
||||
const { unmount } = await renderAndSubmit(
|
||||
fakeClient(read),
|
||||
makeTemplate({ uriTemplate: "x://{a,b}" }),
|
||||
{ a: "only-a", b: "" },
|
||||
);
|
||||
// The untouched field is dropped before the read: `""` is a *defined*
|
||||
// RFC 6570 value that would expand to a valueless pair, and ink-form
|
||||
// cannot distinguish "never touched" from "deliberately empty".
|
||||
expect(read).toHaveBeenCalledWith("x://{a,b}", { a: "only-a" });
|
||||
unmount();
|
||||
});
|
||||
|
||||
it("does not block a template whose only expression is omittable", async () => {
|
||||
const read = vi.fn().mockResolvedValue({
|
||||
result: { contents: [] },
|
||||
expandedUri: "x://events",
|
||||
});
|
||||
const { unmount } = await renderAndSubmit(
|
||||
fakeClient(read),
|
||||
makeTemplate({ uriTemplate: "x://events{?topic}" }),
|
||||
{ topic: "" },
|
||||
);
|
||||
expect(read).toHaveBeenCalled();
|
||||
unmount();
|
||||
});
|
||||
});
|
||||
|
||||
it("renders the form initially without invoking the client", async () => {
|
||||
const read = vi.fn();
|
||||
const api = render(
|
||||
|
|
|
|||
|
|
@ -213,4 +213,173 @@ describe("schemaToForm", () => {
|
|||
);
|
||||
expect(form.sections[0]!.fields[0]).toMatchObject({ initialValue: "hi" });
|
||||
});
|
||||
|
||||
// #2015 (the TUI twin of #1928): an argument declared with Zod's `.nullish()`
|
||||
// has no top-level `type`/`enum` — they sit on the surviving `anyOf` branch —
|
||||
// so before normalization every one of these degraded to a plain text field.
|
||||
describe("nullable unions", () => {
|
||||
it("renders a select for an anyOf string-enum|null argument", () => {
|
||||
const form = schemaToForm(
|
||||
{
|
||||
properties: {
|
||||
direction: {
|
||||
anyOf: [
|
||||
{ type: "string", enum: ["envio", "recebimento"] },
|
||||
{ type: "null" },
|
||||
],
|
||||
},
|
||||
},
|
||||
},
|
||||
"nullableEnum",
|
||||
);
|
||||
expect(form.sections[0]!.fields[0]).toMatchObject({
|
||||
name: "direction",
|
||||
type: "select",
|
||||
options: [
|
||||
{ label: "envio", value: "envio" },
|
||||
{ label: "recebimento", value: "recebimento" },
|
||||
],
|
||||
});
|
||||
});
|
||||
|
||||
it("maps the remaining nullable scalar branches to their typed fields", () => {
|
||||
const form = schemaToForm(
|
||||
{
|
||||
properties: {
|
||||
reference: { anyOf: [{ type: "string" }, { type: "null" }] },
|
||||
quantity: {
|
||||
anyOf: [
|
||||
{ type: "integer", minimum: 1, maximum: 9 },
|
||||
{ type: "null" },
|
||||
],
|
||||
},
|
||||
ratio: { anyOf: [{ type: "number" }, { type: "null" }] },
|
||||
express: { anyOf: [{ type: "boolean" }, { type: "null" }] },
|
||||
},
|
||||
},
|
||||
"nullableScalars",
|
||||
);
|
||||
const fields = form.sections[0]!.fields;
|
||||
expect(fields.map((f) => ({ name: f.name, type: f.type }))).toEqual([
|
||||
{ name: "reference", type: "string" },
|
||||
{ name: "quantity", type: "integer" },
|
||||
{ name: "ratio", type: "float" },
|
||||
{ name: "express", type: "boolean" },
|
||||
]);
|
||||
// The branch's own constraints are hoisted along with its type.
|
||||
expect(fields[1]).toMatchObject({ min: 1, max: 9 });
|
||||
});
|
||||
|
||||
it("renders a select for a nullable array-of-enum argument", () => {
|
||||
const form = schemaToForm(
|
||||
{
|
||||
properties: {
|
||||
tags: {
|
||||
anyOf: [
|
||||
{ type: "array", items: { enum: ["a", "b"] } },
|
||||
{ type: "null" },
|
||||
],
|
||||
},
|
||||
},
|
||||
},
|
||||
"nullableArrayEnum",
|
||||
);
|
||||
expect(form.sections[0]!.fields[0]).toMatchObject({
|
||||
type: "select",
|
||||
options: [
|
||||
{ label: "a", value: "a" },
|
||||
{ label: "b", value: "b" },
|
||||
],
|
||||
});
|
||||
});
|
||||
|
||||
// With the null left in the list, the all-strings check would reject the
|
||||
// enum and this would degrade to a plain text field.
|
||||
it("renders a select for a type: [string, null] enum, null stripped", () => {
|
||||
const form = schemaToForm(
|
||||
{
|
||||
properties: {
|
||||
direction: {
|
||||
type: ["string", "null"],
|
||||
enum: ["envio", "recebimento", null],
|
||||
},
|
||||
},
|
||||
},
|
||||
"typeArrayNullableEnum",
|
||||
);
|
||||
expect(form.sections[0]!.fields[0]).toMatchObject({
|
||||
type: "select",
|
||||
options: [
|
||||
{ label: "envio", value: "envio" },
|
||||
{ label: "recebimento", value: "recebimento" },
|
||||
],
|
||||
});
|
||||
});
|
||||
|
||||
it("collapses the type: [T, null] encoding too", () => {
|
||||
const form = schemaToForm(
|
||||
{
|
||||
properties: {
|
||||
note: { type: ["string", "null"] },
|
||||
flag: { type: ["boolean", "null"] },
|
||||
},
|
||||
},
|
||||
"typeArrayNull",
|
||||
);
|
||||
expect(
|
||||
form.sections[0]!.fields.map((f) => ({ name: f.name, type: f.type })),
|
||||
).toEqual([
|
||||
{ name: "note", type: "string" },
|
||||
{ name: "flag", type: "boolean" },
|
||||
]);
|
||||
});
|
||||
|
||||
// ink-form hands a select's stringified value straight back, so a numeric
|
||||
// enum routed to a select would submit "1" for 1. The typed field loses the
|
||||
// enum constraint but keeps the value's type — the safer loss.
|
||||
it("routes a nullable numeric enum to its typed field, not a select", () => {
|
||||
const form = schemaToForm(
|
||||
{
|
||||
properties: {
|
||||
level: {
|
||||
anyOf: [{ type: "integer", enum: [1, 2] }, { type: "null" }],
|
||||
},
|
||||
ratio: {
|
||||
anyOf: [{ type: "number", enum: [0.5, 1.5] }, { type: "null" }],
|
||||
},
|
||||
},
|
||||
},
|
||||
"numericEnum",
|
||||
);
|
||||
expect(
|
||||
form.sections[0]!.fields.map((f) => ({ name: f.name, type: f.type })),
|
||||
).toEqual([
|
||||
{ name: "level", type: "integer" },
|
||||
{ name: "ratio", type: "float" },
|
||||
]);
|
||||
});
|
||||
|
||||
it("routes a plain numeric enum to its typed field too", () => {
|
||||
const form = schemaToForm(
|
||||
{ properties: { level: { type: "integer", enum: [1, 2] } } },
|
||||
"plainNumericEnum",
|
||||
);
|
||||
expect(form.sections[0]!.fields[0]).toMatchObject({ type: "integer" });
|
||||
});
|
||||
|
||||
it("leaves a union of two real types as a plain string field", () => {
|
||||
const form = schemaToForm(
|
||||
{
|
||||
properties: {
|
||||
mixed: { anyOf: [{ type: "string" }, { type: "number" }] },
|
||||
},
|
||||
},
|
||||
"realUnion",
|
||||
);
|
||||
expect(form.sections[0]!.fields[0]).toMatchObject({
|
||||
name: "mixed",
|
||||
type: "string",
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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<void>`; await it so the
|
||||
// assertions below can't run before an async setup has registered.
|
||||
await inkFormLabelPatch.setup(
|
||||
build as unknown as Parameters<typeof inkFormLabelPatch.setup>[0],
|
||||
);
|
||||
|
||||
|
|
|
|||
|
|
@ -11,7 +11,38 @@ describe("uriTemplateToForm", () => {
|
|||
expect(form.title).toBe("Read Resource: file");
|
||||
const fields = form.sections[0]!.fields;
|
||||
expect(fields.map((f) => f.name)).toEqual(["path", "name"]);
|
||||
expect(fields[0]).toMatchObject({ type: "string", required: false });
|
||||
// Simple `{path}` sits mid-URI, so omitting it would leave an empty path
|
||||
// segment rather than a shorter URI -- it is mandatory, matching the web
|
||||
// panel. Both clients read this from core's `requiredGroups`.
|
||||
expect(fields[0]).toMatchObject({ type: "string", required: true });
|
||||
});
|
||||
|
||||
it("names fields as the expander looks them up, not as the SDK parses them", () => {
|
||||
// The SDK reports these as ";id" and "id:3"; submitting under those keys
|
||||
// would make the expander find nothing and drop the expression (#1919).
|
||||
expect(
|
||||
uriTemplateToForm("x://a{;id}", "matrix").sections[0]!.fields.map(
|
||||
(f) => f.name,
|
||||
),
|
||||
).toEqual(["id"]);
|
||||
expect(
|
||||
uriTemplateToForm("x://a/{id:3}", "prefix").sections[0]!.fields.map(
|
||||
(f) => f.name,
|
||||
),
|
||||
).toEqual(["id"]);
|
||||
});
|
||||
|
||||
it("leaves a shared required group optional rather than demanding every name", () => {
|
||||
// `{a,b}` is satisfied by either name, and ink-form cannot say "any one
|
||||
// of"; marking both required would refuse input the expander accepts.
|
||||
const fields = uriTemplateToForm("x://{a,b}", "pair").sections[0]!.fields;
|
||||
expect(fields.map((f) => f.name)).toEqual(["a", "b"]);
|
||||
expect(fields.every((f) => f.required === false)).toBe(true);
|
||||
});
|
||||
|
||||
it("marks an omittable variable optional", () => {
|
||||
const fields = uriTemplateToForm("x://a{?topic}", "q").sections[0]!.fields;
|
||||
expect(fields[0]).toMatchObject({ name: "topic", required: false });
|
||||
});
|
||||
|
||||
it("returns an empty Template Variables section for a static URI", () => {
|
||||
|
|
@ -33,3 +64,15 @@ describe("uriTemplateToForm", () => {
|
|||
expect(form.sections[0]!.fields).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("a name repeated inside one expression", () => {
|
||||
it("is required, not treated as a shared group", () => {
|
||||
// `{a,a}` is one requirement named twice. Before core deduplicated the
|
||||
// group, `requiredGroups` returned ["a","a"], so this form's
|
||||
// `length === 1` test left the field optional while ResourceTestModal's
|
||||
// submit guard still refused a blank -- an un-submittable form.
|
||||
const [field] = uriTemplateToForm("x://{a,a}", "T").sections[0].fields;
|
||||
expect(field.name).toBe("a");
|
||||
expect(field.required).toBe(true);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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",
|
||||
},
|
||||
},
|
||||
]);
|
||||
|
|
|
|||
6
clients/tui/package-lock.json
generated
6
clients/tui/package-lock.json
generated
|
|
@ -4295,9 +4295,9 @@
|
|||
}
|
||||
},
|
||||
"node_modules/nanoid": {
|
||||
"version": "3.3.16",
|
||||
"resolved": "https://registry.npmjs.org/nanoid/-/nanoid-3.3.16.tgz",
|
||||
"integrity": "sha512-bzlKTyNJ7+LdGIIwy8ijFpIqEQIvafahV7eYykJ8Cvh42EdJeODoJ6gUJXpQJvej1BddH8OqTXZNE/KfbWAu8Q==",
|
||||
"version": "3.3.18",
|
||||
"resolved": "https://registry.npmjs.org/nanoid/-/nanoid-3.3.18.tgz",
|
||||
"integrity": "sha512-DTg4MJbGMWkfi6VZFdNt2/caMbQy4Ou+Op/hJQvGEWcnVfoA1QA+xzRKAzw9jD6+GVOOeYr/mIcuDSdug6F6+w==",
|
||||
"dev": true,
|
||||
"funding": [
|
||||
{
|
||||
|
|
|
|||
|
|
@ -170,6 +170,17 @@ function App({
|
|||
pendingStepUpRef.current = pendingStepUp;
|
||||
}, [pendingStepUp]);
|
||||
const [connectError, setConnectError] = useState<string | null>(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<string | null>(null);
|
||||
const oauthInProgressRef = useRef(false);
|
||||
const callbackServerRef = useRef<OAuthCallbackServer | null>(null);
|
||||
const selectedServerRef = useRef<string | null>(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({
|
|||
<Text color="red">OAuth: {oauthMessage}</Text>
|
||||
</Box>
|
||||
)}
|
||||
{disconnectError && (
|
||||
<Box marginTop={1}>
|
||||
<Text color="red">Disconnect failed: {disconnectError}</Text>
|
||||
</Box>
|
||||
)}
|
||||
</Box>
|
||||
</Box>
|
||||
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
Some files were not shown because too many files have changed in this diff Show more
Loading…
Reference in a new issue