Commit graph

2,046 commits

Author SHA1 Message Date
Cliff Hall
8bb389c8e3
test: switch integration project to folder-based manifest (#1314) (#1318)
* test: switch integration project to folder-based manifest (#1314)

Replace the `integrationTests` enumeration in vite.config.ts with a
folder glob (`src/test/integration/**/*.test.ts`). Files under that
folder run in the integration vitest project (node env, 30s timeouts);
everything else runs in the unit project (happy-dom).

This removes the silent misclassification trap from #1307: a new
integration-style test no longer falls into the unit project (and
happy-dom) just because someone forgot to add it to the array.

The 13 ported v1.5 tests + their helpers move under src/test/integration/
in a strict mirror of core/ source layout (mcp/, auth/, auth/node/,
storage/). Renames use git mv so history is preserved.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* test: address PR review on #1318 (glob extensions + AGENTS doc)

- vite.config.ts: widen integrationGlob from `*.test.ts` to
  `*.test.{ts,tsx}` so the integration folder fully owns its contents.
  Without this, a `.test.tsx` placed under src/test/integration/ would
  match the unit project's `{ts,tsx}` include, fail the unit `.ts`-only
  exclude, and silently land under happy-dom — the exact misclassification
  trap this PR set out to close.
- AGENTS.md: update the line describing where integration tests live;
  it still pointed at the old `src/test/core/` location and the removed
  `integrationTests` enumeration.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-18 11:54:49 -04:00
Cliff Hall
02182e3b6c
test(core): backfill coverage on inspectorClient + remote server (#1317)
* test(core): backfill coverage on inspectorClient + remote server

Resolves #1310. Drops the two TODO(#1310) exclusions from
clients/web/vite.config.ts (coverage.exclude); both files now pass the
per-file gate (lines >=90, statements >=85, functions >=80, branches >=50).

core/mcp/remote/node/server.ts (88.15 / 78.37 -> 97.29 / 100):
- Extract createTokenAuthProvider to its own file
  (core/mcp/remote/node/tokenAuthProvider.ts) so the 7 no-op
  OAuthClientProvider stubs can be unit-tested directly.
- Add focused integration tests covering the remaining gaps:
  * createTransportNode failure path on POST /api/mcp/connect
  * /api/mcp/send when transport is dead
  * /api/log non-JSON body (catch fallback)
  * forwardLogEvent unknown-level / no-messages / string-first-message
    fallbacks (lines 232, 240-242, 257-265 in the original)
  * /api/fetch missing-url + upstream-throw paths
  * /api/storage POST invalid storeId + invalid JSON body
  * /api/storage GET when the on-disk store is unparseable

core/mcp/inspectorClient.ts (73.48 / 72.09 -> 91.16 / 89.92):
Added a single "Coverage backfill (#1310)" describe block to the existing
integration test, plus a few unit-style cases:
- OAuth getters / setOAuthConfig / authenticate when oauthManager is unset.
- Simple session, roots, subscription, and instructions accessors.
- subscribe / unsubscribe error paths against a server without the
  subscribe capability; happy path against one with it.
- getPrompt + readResourceFromTemplate happy path + expansion-failure.
- setLoggingLevel "Server does not support logging" guard.
- getAppRendererClient (null before connect, cached proxy after).
- OAuth + stdio: authenticate() throws via getServerUrl()'s stdio check.
- listChanged notifications: tools/resources/prompts handlers via the
  fixture add_tool / add_resource / add_prompt tools.
- Misc tiny branches: re-connect no-op, elicit form mode, getTaskCapabilities
  on a tasks-less server, receiver-task TTL cleanup + cancel via the
  private surface (the focused-unit pass the issue describes).
- defensive `if (!this.client) throw` guards: one nullify-and-call test
  exercises 16 of them via a single instance, plus the connect() entry
  guard.

The receiver-task internals test uses (instance as unknown as
{ private fields... }) to drive createReceiverTask / cancelReceiverTask
/ TTL cleanup directly; per the issue, refactoring private helpers to
be testable from a unit test was authorized.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* test(core): address #1317 PR review feedback

- Rename the misnamed "listChanged notifications" describe in
  inspectorClient.test.ts to "capability detection after connect", since
  the test asserts only on getCapabilities() round-tripping and never
  triggers or observes a notification dispatch. Real dispatch coverage
  lives in the adjacent "list_changed + resourceUpdated notifications"
  block. Kept the resources/prompts fixtures with a note explaining why
  they are required for the server to advertise the matching capabilities.
- Expand the comment on the 250ms wait in remote-transport.test.ts'
  /api/mcp/send dead-transport test so it is clear the >= 500 assertion
  is intentionally permissive across both the dead-transport short-
  circuit and the downstream send-failure branches.
- Add a "keep this in sync" pointer on the nullify() helper so a future
  cleanup that removes any of the defensive `if (!this.client) throw`
  guards in inspectorClient.ts is reminded to drop the matching call here
  rather than silently under-covering.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-17 20:43:55 -04:00
Cliff Hall
82269e78cb
refactor(core): apply v1.5-port cleanups deferred from #1306 (#1316)
* refactor(core): apply v1.5-port cleanups deferred from #1306

Addresses issue #1309 — five small, mechanical cleanups deferred from the
v1.5 runtime port:

1. Collapse BrowserOAuthStorage / NodeOAuthStorage / RemoteOAuthStorage
   onto a shared OAuthStorageBase. Three ~130-line duplicates become
   thin adapter wrappers; behavior + public API unchanged.
2. Re-enable erasableSyntaxOnly in tsconfig.app.json by rewriting the
   seven (plus two the issue missed in sampling/elicitation create-
   message) parameter-property constructors as explicit field + assign.
3. Route the lone console.error in inspectorClient.ts through this.logger.
4. Replace `${Date.now()}-${Math.random()}` with crypto.randomUUID() at
   the five local-only ID sites (message tracking + sampling/elicit).
5. Pin the remote-server hono app generic at new Hono<Env>() and drop
   the streamSSE cast. Root cause of the cast was an unresolved hono
   subpath: the tsconfig "hono/*" wildcard didn't honor the exports
   map, so hono/streaming leaked to the root-hoisted hono@4.12.12 while
   hono itself resolved to clients/web/hono@4.12.18. Replaced the
   wildcard with an explicit hono/streaming entry in both tsconfig.app
   and tsconfig.test, removing the package-identity mismatch.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* refactor(core): address #1316 PR review feedback

- Drop the stranded no-op getServerState() read at the top of
  clearClientInformation in OAuthStorageBase (carried over verbatim
  from the three originals).
- Drop the crypto.randomUUID() guard at the task-ID site so all
  six in-scope sites are consistent; the fallback branch was dead
  code on every supported runtime.
- Remove the residual `c as Context` cast at the streamSSE call site
  in core/mcp/remote/node/server.ts — once both hono imports
  resolve to the same package the route-typed Context flows through
  to streamSSE with no cast at all.
- Move the new `options` / `config` field declarations in
  RemoteClientTransport up with the rest of the field block, matching
  the layout used by the other five rewritten classes in this PR.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-17 19:15:48 -04:00
Cliff Hall
88b55b093b
test: run v1.5-ported integration tests under node-env vitest project (#1307) (#1312)
* feat(test): run v1.5-ported integration tests under node-env vitest (#1307)

Adds an `integration` vitest project alongside `unit` and `storybook` so
the 13 v1.5-ported tests that need real HTTP/stdio servers (or fail under
happy-dom's SDK mock resolution) run again under node env with 30s
timeouts. Restores test-servers/ as a buildable package so the stdio
subprocess spawns a real .js file. Drops the v1.5-source `coverage.exclude`
globs and fills coverage gaps across the v1.5-ported core/ modules; two
large files (inspectorClient.ts, remote/node/server.ts) keep narrow
per-file excludes tracked in #1310.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* test(core): address PR #1312 review — TS6-safe tsconfig, real transport_error path

- Drop `baseUrl` / `paths` / `typeRoots` / `declaration*` from
  test-servers/tsconfig.json. They were only needed for type
  resolution, and we already pass `--noCheck` from `test-servers:build`.
  Under TS 6+ the deprecated `baseUrl` exited with code 2, which
  silently skipped vitest in the `&&` chains of test:integration /
  test:coverage / validate.
- Rework the "send() throws after closed" test to drive `closed` via
  an actual transport_error SSE event instead of mutating private
  state through a cast — matches the real path the branch guards.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-16 15:15:17 -04:00
Cliff Hall
0ead1deea6
feat(core): port v1.5 InspectorClient + transports + auth subsystem (#1302) (#1306)
* wip(core): bulk-copy v1.5 InspectorClient runtime files (#1302)

Verbatim ports from origin/v1.5/main into v2/main core/:

- logging/ (2 files)
- storage/ + storage/adapters/ (4 files)
- auth/ + auth/browser/ + auth/node/ + auth/remote/ (16 files)
- mcp/config.ts + mcp/node/ (4 files, stdio transport)
- mcp/remote/ + mcp/remote/node/ (12 files, HTTP/SSE transport)
- mcp/inspectorClient.ts (~2169 LOC), messageTrackingTransport,
  fetchTracking, oauthManager, index, state/index

Total: ~7,800 LOC of source files, copied unmodified via
`git show origin/v1.5/main:<path> > <dest>`.

Also:
- Add deps: pino, hono, @hono/node-server, atomically, zustand, @types/node
- tsconfig.app.json: extend paths to resolve new node-only deps from
  core/, add "node" to types, disable erasableSyntaxOnly (v1.5 code
  uses parameter properties — TODO to re-enable in follow-up)

Compiles down to 47 type errors, all in three categories:
- 24 TS2305: v2 dropped v1.5 type exports (InspectorClientOptions,
  CreateTransport, InspectorClientEnvironment, AppRendererClient,
  SamplingCreateMessage class, ElicitationCreateMessage class,
  jsonUtils helpers, OAuth events). Per user direction, will re-add
  these to v2's diverged files (types.ts, samplingCreateMessage.ts,
  etc.) so the ported code stays verbatim.
- 15 TS7006: implicit-any params in v1.5 code (v1.5 tsconfig was
  looser than v2's noImplicitAny). Need explicit annotations.
- 8 other TS errors: ../package.json import, Transport|null narrowing,
  hono Context generic mismatch, one unused parameter.

Tests not yet ported (19 files, ~11,170 LOC).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* wip(core): integrate ported v1.5 InspectorClient + port tests (#1302)

Source integration (typecheck + build now pass):
- Re-add v1.5 types/classes dropped during v2 evolution:
  - types.ts: CreateTransport*, AppRendererClient, InspectorClient{Environment,Options}
  - samplingCreateMessage.ts / elicitationCreateMessage.ts: classes + keep v2's
    InspectorPending{Sampling,Elicitation} interface
  - inspectorClientEventTarget.ts: oauth* event keys
  - json/jsonUtils.ts: convertParameterValue / convertToolParameters /
    convertPromptArguments + ParameterSchema
- inspectorClient.ts: replace `../package.json` import (v2 has no
  core/package.json) with a hardcoded identity constant
- remote/node/server.ts: cast hono Context for streamSSE (hono's
  generic typing tightened since v1.5)
- oauth-callback-server.ts: drop unused hostname field
- tsconfig: enable @types/node, add paths for new bare-module deps
  (pino, zustand, hono, atomically) in app + test configs
- vite.config.ts: mirror those module aliases for the unit project
  (which runs from repoRoot)

Tests ported (19 files, ~11K LOC) under clients/web/src/test/core/:
- All core/__tests__/auth/* (7 files)
- inspectorClient.test.ts + 4 OAuth variants
- mcp/oauthManager.test.ts, jsonUtils.test.ts
- transport, remote-transport, remote-server-config, storage-adapters
- helpers/oauth-client-fixtures.ts
Bulk-rewrote `from "../mcp/…"` → `from "@inspector/core/mcp/…"` etc.

Current state:
- typecheck: 0 errors
- build: passes (vite v8, 788 modules)
- unit tests: 1014 / 1034 pass (937 pre-existing + 77 new core tests)
- 20 failures in 13 files; failure modes:
  - 13 file-load failures needing the v1.5 `test-servers/` package
    (@modelcontextprotocol/inspector-test-server) which isn't in v2 yet
    or real fs/network not available in happy-dom
  - 7 assertion failures in discovery.test.ts + state-machine.test.ts
    (SDK 1.29 vs 1.25 API drift — mocks don't match new shape)

Coverage gate not yet checked against new files.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* wip(core): port test-servers, more types, partial test integration (#1302)

- Port v1.5 test-servers/ package (12 files, ~5K LOC) under
  repo-root/test-servers/, wire @modelcontextprotocol/inspector-test-server
  alias in both vite.config.ts (regex form) and tsconfig.test.json.
- Add deps: express ^5, yaml ^2, @types/express ^5.
- Re-export InspectorClientOptions/Environment/Transport* from inspectorClient.ts
  so v1.5 tests that import them from that module path keep resolving.
- Switch InspectorClientEventMap to use SamplingCreateMessage /
  ElicitationCreateMessage classes (matches v1.5 event payload contracts)
  while keeping the InspectorPending{Sampling,Elicitation} interfaces in
  the *.ts modules for v2 consumers that still use them.
- Fix verbatim-module-syntax issue in test-servers and drop unused stdio
  config field.
- Adjust inspectorClient.test.ts task types to TaskWithOptionalCreatedAt
  (SDK 1.29 tightened Task shape vs v1.5's 1.25).
- Drop spurious maxMessages/maxFetchRequests/maxStderrLogEvents from
  remote-transport.test.ts (never on InspectorClientOptions).

State:
- typecheck: 0 errors
- build: passes
- unit tests: 1099 / 1270 pass
- 171 failures in 10 test files, almost all integration tests that need
  real HTTP/fs/network (happy-dom can't host them). Next: add a node-env
  vitest project to run these properly.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(core): port v1.5 InspectorClient + transports + auth subsystem (#1302)

Bulk-port of the v1.5 core runtime (~7,800 LOC source + ~16K LOC tests +
~5K LOC test-servers/) into v2. Files copied verbatim from origin/v1.5/main
via `git show >`; only minimal adapters added on the v2 side.

Subsystems landed
-----------------
- core/logging/, core/storage/, core/auth/ (browser + node + remote)
- core/mcp/ runtime: inspectorClient.ts (2169 LOC), oauthManager,
  messageTrackingTransport, fetchTracking, config, index
- core/mcp/node/ (stdio transport)
- core/mcp/remote/ + remote/node/ (HTTP/SSE transport + Hono server)
- test-servers/ package (composable MCP test servers, fixtures, harness)

v2 adapter layer
----------------
- types.ts: append CreateTransport*, AppRendererClient,
  InspectorClient{Environment,Options} (v2 had pruned these — v1.5 needs them)
- samplingCreateMessage / elicitationCreateMessage: keep v1.5 classes AND
  v2's Inspector Pending* interfaces in the same files
- inspectorClientEventTarget: add oauth* event keys; switch pending
  sampling/elicitation payloads to the classes (matches v1.5 contracts)
- json/jsonUtils: append convertParameterValue/convertToolParameters/
  convertPromptArguments + ParameterSchema
- inspectorClient.ts: re-export Options/Environment/Transport* so tests
  that import these from `inspectorClient.js` keep resolving; replace
  `../package.json` import with a hardcoded identity (v2 has no core/package.json)
- remote/node/server.ts: cast hono Context for streamSSE (hono generic
  tightened since v1.5)
- oauth-callback-server: drop unused hostname field
- tsconfig.app.json / tsconfig.test.json: add @types/node + paths for
  pino/zustand/hono/@hono/node-server/atomically/express/yaml, disable
  erasableSyntaxOnly (parameter-property syntax from v1.5)
- vite.config.ts: regex-form module aliases (so the package's exports
  field handles subpaths like `hono/streaming`, `zustand/middleware`),
  alias for @modelcontextprotocol/inspector-test-server → test-servers/src

Tests
-----
- 19 v1.5 test files copied under clients/web/src/test/core/,
  imports rewired to @inspector/core/* and @modelcontextprotocol/inspector-test-server
- 1004 unit tests pass; 298 storybook tests pass; coverage gate passes

Known follow-ups (out-of-scope here, tracked in TODOs)
------------------------------------------------------
- 13 integration test files are excluded from the unit project (spawn real
  HTTP/stdio servers; run real OAuth flows; mock SDK auth → happy-dom CORS).
  Needs a node-env vitest project. Files listed under `test.projects[0].test.exclude`.
- Source files exercised only by those integration tests are excluded from
  the coverage gate pending the node-env project. Listed under
  `test.coverage.exclude` with comment.
- Manual smoke tests against real stdio / HTTP / OAuth server per the AC
  on #1302 still need verification locally.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs/fix: address #1306 review (AGENTS tree, #1307 links, @types/node, .substr)

- AGENTS.md: expand the project-structure tree to include all v1.5-ported
  subsystems (core/auth/{browser,node,remote}, core/storage, core/logging,
  core/mcp/{node,remote/{,node}}, core/mcp/state) plus the new top-level
  test-servers/ directory (with a note about its @modelcontextprotocol/
  inspector-test-server alias).
- vite.config.ts: reference #1307 from both the test exclude block and the
  coverage exclude block so the next person editing either sees the
  marching order for shrinking the lists.
- package.json: restore @types/node to ^24 (an earlier `npm install
  --save-dev @types/node@^22` accidentally pinned it lower than the repo
  was using; nothing here required ^22).
- core/mcp/fetchTracking.ts: replace deprecated .substr(2, 9) with
  .slice(2, 11) to match the convention in core/mcp/inspectorClient.ts.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-15 15:36:17 -04:00
Cliff Hall
e154378178
fix(coverage): include core/ files in v8 per-file gate (#1301) (#1305)
* fix(coverage): include core/ files in v8 per-file gate (#1301)

Two issues prevented coverage.include's '../../core/...' globs from
reaching core/ files:

1. allowExternal defaults to false — v8 drops any file whose absolute
   path doesn't start with the project root (clients/web/) before the
   include filter runs. Set allowExternal: true, and switch the globs
   to absolute paths so tinyglobby + picomatch handle them correctly.

2. vitest's coverage transformer only processes files inside a project
   root; for everything else it falls back to raw-TS parsing in
   rolldown, which can't handle TS-only syntax (e.g. import type) —
   silently dropping untested core/ files and bypassing the gate.
   Root the unit project at the repo root so the transformer reaches
   core/. The repo root has no node_modules, so add an explicit react
   alias and shift include/setupFiles paths accordingly.

Also add the four pure-type core files (samplingCreateMessage,
sessionStorage, inspectorClientProtocol, inspectorClientEventTarget)
to the exclude list with a comment — they have no executable code.

Verified by introducing an untested probe file, confirming exit=1 with
threshold errors, then removing it.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* refactor(coverage): address review feedback on #1305

- Drop allowExternal: true — with the unit project rooted at repoRoot,
  core/ files are no longer "external" to any project that collects
  coverage (the storybook project doesn't), so the flag is redundant.
- Extract sharedAliases / sharedDedupe constants and reference them
  from both the top-level resolve and the unit project resolve, so
  the two stay in sync if a new core/* alias is added.
- Add TODO(#1243) on the inspectorClientEventTarget.ts exclusion so
  it's revisited once the class gains real behavior in the
  InspectorClient port.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-14 20:17:10 -04:00
Cliff Hall
e3e70470b5
feat(core): port v1.5 hook layer (#1243) (#1300)
* feat(core): port v1.5 hook layer (useInspectorClient + useManaged*/usePaged* + log hooks)

Wholesale port of v1.5's React hook layer and the EventTarget-based state machinery the hooks subscribe to (#1243). Hooks decouple from the concrete InspectorClient via a new `InspectorClientProtocol` interface so this layer can land before the runtime class is ported.

Added under `core/`:
- `core/react/` — useInspectorClient + 5 useManaged* + 5 usePaged* + 3 log hooks
- `core/mcp/state/` — 5 managed*State + 5 paged*State + messageLogState + stderrLogState + fetchRequestLogState
- `core/mcp/inspectorClientProtocol.ts`, `inspectorClientEventTarget.ts`, `typedEventTarget.ts`, `samplingCreateMessage.ts`, `sessionStorage.ts`
- v2-only wrapper types in `core/mcp/types.ts`: InspectorUrlElicitRequest, InspectorPendingRequest, InspectorRequestHistoryItem

Tests live under `clients/web/src/test/core/` (28 new test files, 264 new tests). Test fake: `core/mcp/__tests__/fakeInspectorClient.ts`.

Build wiring:
- `vite.config.ts`: dedupe react/react-dom so core/ files resolve them via clients/web/node_modules.
- `tsconfig.app.json` / `tsconfig.test.json`: paths map react/react-dom/vitest to clients/web/node_modules so `tsc -b` resolves bare imports from core/ source files (which sit above the package's node_modules). Exclude core/__tests__ from app config; include in test config.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* review: address PR #1300 review findings

Bugs:
- fetchRequestLogState: add .catch() on sessionStorage.loadSession so a corrupt
  or unreadable session doesn't surface as an unhandled rejection (matches the
  saveSession swallow pattern).
- messageLogState: sync pendingRequestEntries when clearMessages() evicts a
  pending request entry, so a later response with the same id falls through
  to the append branch instead of mutating an unreachable map entry. Add tests
  for both predicate and no-predicate paths.

Cleanups:
- Extract mergeTaskIntoList helper to core/mcp/state/mergeTaskIntoList.ts so
  managed/paged RequestorTasks state classes share one definition.
- Add a one-line JSDoc note on each PagedTools/Prompts/Resources/Templates
  state explaining why they deliberately do not subscribe to *ListChanged
  (cursor invalidation mid-pagination).
- Document on useInspectorClient that appRendererClient is read lazily on
  every render and is not subscribed (changes once at connect time).

Carry-overs from v1.5 deliberately left for the follow-up InspectorClient
runtime port:
- Refresh/status-change race in Managed*State.refresh() (status check is only
  at entry; a disconnect during await can re-clear before refresh resumes).
- nextCursor exposure asymmetry across usePaged* (only usePagedRequestorTasks
  surfaces it via state field; others return it from loadPage() only).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-13 17:43:07 -04:00
Cliff Hall
a4215e57b9
ci(storybook): wire test:storybook into CI (#1251) (#1297)
* ci(storybook): wire test:storybook into CI (#1251)

- Add test:storybook:watch to clients/web for interactive iteration; the
  one-shot script already existed.
- Surface test:storybook at the repo root, delegating to the web client.
- Add a separate CI step that installs the Playwright Chromium binary
  and runs the storybook play-function suite after validate. Kept out of
  validate because the suite needs a real browser and is ~10s slower
  than the unit tests.
- Document the new pre-push step in AGENTS.md.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* ci(storybook): cache Playwright browsers + tighten AGENTS wording

Cache the Chromium binary keyed on clients/web/package-lock.json so the
~150MB download is a no-op on CI cache hits. Also clarify that CI runs
test:storybook after the unit/lint/build checks (not literal `validate`,
which is a local-only script).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-10 15:58:15 -04:00
Cliff Hall
fc41fdc10a
feat(apps): render tool.icons on ToolListItem and ToolDetailPanel (#1264) (#1293)
* feat(apps): render tool.icons on ToolListItem (#1264)

Add icon rendering to ToolListItem so the Tools sidebar matches the
Apps sidebar treatment. Falls back gracefully when tool.icons is absent.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(stories): use #228be6 in ToolListItem WithIcon data URL

The source SVG had %23228be6 pre-encoded, but encodeURIComponent then
re-escaped the % to %25, leaving fill="%23228be6" in the decoded SVG
and making the icon render black instead of Mantine blue.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(apps): render tool.icons on ToolDetailPanel (#1264)

Show the tool icon next to the title in the detail panel, matching the
header icon size used in AppsScreen (24x24, fit: contain). Adds a
WithIcon story and tests for icon-present/icon-absent states.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-09 15:45:53 -04:00
Cliff Hall
5f296589ea
feat(apps): AppsScreen and InspectorView wiring (#1263) (#1286)
* feat(apps): AppsScreen and InspectorView wiring (#1263)

Adds the Apps tab as a top-level screen composed of `AppListItem`,
`AppDetailPanel`, and `AppRenderer`. The screen is presentational —
selection, form values, running, and maximize state are local; data and
side effects come in via props and callbacks.

InspectorView filters `tools` through `isAppTool` (wrapped in a try/catch
so a malformed `_meta.ui.resourceUri` skips one tool instead of dropping
every following App), exposes the Apps tab in `ALL_TABS` after Tools, and
wires Phase-3 stub callbacks plus a stub bridge factory.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* refactor(apps): extract AppControls and rename Refresh button

Pulls the sidebar list, search input, "List updated" indicator, and
Refresh button out of `AppsScreen` into a dedicated `AppControls`
group, mirroring `ToolControls` and `TaskControls`. The Refresh button
moves from a full-width row beneath the list to the top-right of the
title row (single label "Refresh", matching `TaskControls`).

`AppsScreen` keeps the right-pane orchestration (selection, form,
running, maximize, header actions) and now delegates the entire
sidebar to `AppControls`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* chore(stories): seed InspectorView demo tools with MCP Apps

Adds three apps (Cohort Data, Weather Widget, Ops Dashboard) carrying
real `_meta.ui.resourceUri` to the demo tools list so the Apps tab has
content in the Default story alongside the existing tools.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(apps): drop AppDetailPanel title row to avoid duplicate name

The parent AppsScreen header already shows the app name, so the panel's
own title/icon row rendered the name twice. Remove the title row (and
its now-unused imports, styled subcomponents, and the four tests that
asserted on title/icon rendering).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* refactor(resources): drop annotation badges from ResourceListItem

The list row now renders just the resource title/name. Audience and
priority badges remain visible in the preview/template panels, so the
data is still surfaced — just not duplicated in the picker. Drops the
now-unused AnnotationBadge import, the badge-only stories, and the
badge-rendering test.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* chore(apps): address review — share hasInputFields, show header icon

- Extracts the duplicated `Object.keys(inputSchema.properties ?? {}).length > 0`
  check from `AppsScreen` (and the equivalent in `AppDetailPanel`'s
  `hasFields`) into a shared `hasInputFields` helper in `toolUtils`,
  with tests.
- Renders the selected tool's first icon next to the right-pane header
  title in `AppsScreen`, restoring the icon visibility that was lost
  when `AppDetailPanel`'s title row was removed.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* chore(apps): finish hasInputFields extraction; document isOpening

- AppDetailPanel now actually calls the shared `hasInputFields` helper
  instead of inlining the same `Object.keys(...).length > 0` check.
- AppsScreen comments why `isOpening={false}` is hardcoded: the screen
  flips to AppRenderer synchronously, so the loading state is unreachable
  here — the prop remains for standalone use and Phase 3 wiring.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-07 14:18:27 -04:00
Cliff Hall
323f42d11c
feat(apps): AppRenderer with imperative ref (#1262) (#1282)
* feat(apps): add AppRenderer element with imperative ref (#1262)

Introduces a thin host-side shell for embedded MCP Apps. The renderer
owns the iframe element and the AppBridge lifecycle; the wiring layer
constructs the bridge from the active MCP Client via bridgeFactory and
drives input/result/cancellation through the imperative ref handle.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* review: address PR feedback on AppRenderer

- Document BridgeFactory stability requirement (unstable identity thrashes
  the iframe on every render).
- Null bridgeRef synchronously inside teardown() before awaiting disposal,
  so a concurrent unmount cleanup cannot dispose the same bridge twice.
- Comment the iframe trust model: the outer frame loads the inspector's
  trusted same-origin sandbox-proxy, which sandboxes the actual app
  content. Adding `sandbox` to this frame would block the postMessage
  bridge.
- Rename `Error` story export to `BridgeError` to avoid shadowing the
  global Error constructor; drop the `globalThis.Error` workaround.
- Comment flushAsync to explain the two-tick microtask budget.
- Add unmount-while-tearing-down test to lock in the no-double-dispose
  guarantee.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-05 09:56:48 -04:00
Cliff Hall
1ffcdc38ca
feat(apps): add AppListItem and AppDetailPanel groups (#1261) (#1273)
* feat(apps): add AppListItem and AppDetailPanel groups (#1261)

Two presentational groups for the upcoming AppsScreen.

AppListItem (clients/web/src/components/groups/AppListItem/):
- Props { tool, selected, onClick }
- Renders tool.icons[0] (sized sm) when present, label (title ?? name),
  two-line clamped description, and a trailing chevron affordance.
- Reuses ToolListItem's selected-state highlight (UnstyledButton bg via
  --mantine-primary-color-light).

AppDetailPanel (clients/web/src/components/groups/AppDetailPanel/):
- Props { tool, formValues, isOpening, onFormChange, onOpenApp }
- Layout: optional icon + title row, optional description, divider,
  SchemaForm bound to tool.inputSchema, full-width "Open App" button.
- "Open App" is disabled while isOpening or while any required input
  field is empty (validation derived from tool.inputSchema.required).
- No annotations, progress, or cancel — execution is delegated to the
  embedded app, not run as an MCP tool call from the panel.

Convention notes:
- Issue text references @tabler IconChevronRight / IconPlayerPlay; the
  v2 web client already uses react-icons (md/ri namespaces), so this PR
  uses MdChevronRight and MdPlayArrow for consistency rather than
  introducing a second icon library.

Stories: AppListItem (Default, Selected, WithIcon, NoDescription, LongName);
AppDetailPanel (NoFields, SimpleStringParam, MultipleParams, WithIcon,
Opening, ComplexSchema).

Tests: 22 cases covering rendering branches, callback wiring, and the
disabled-button derivation. Per-file coverage 100% across lines,
statements, functions, and branches for both new components.

Closes #1261

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(stories): inline SVG for WithIcon stories so they render reliably

The Wikipedia-hosted PNG used in the AppListItem and AppDetailPanel
WithIcon stories returned a broken image in Storybook (rate-limited /
hot-link blocked). Replace it with an inline SVG sun icon as a data
URI so the stories render offline, in Chromatic, and on any network.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* test(apps): address PR #1273 review feedback

- Extract resolveDisplayLabel(name, title?) to src/utils/toolUtils.ts and
  migrate all four call sites (AppListItem, AppDetailPanel, ToolListItem,
  ToolDetailPanel) to consume it. Avoids the fourth copy when the next
  list/detail screen arrives. Includes a small unit suite.

- AppDetailPanel: only render the <Divider/> when the schema has at least
  one property — previously a no-input app showed an orphaned separator
  between the description and the Open App button.

- AppDetailPanel: align PanelTitle truncate to `true` (matches the
  ItemLabel form used in AppListItem / ToolListItem); behaviorally
  identical, just less surprising for future readers.

- AppDetailPanel: add an inline comment explaining that SchemaForm is
  disabled only while isOpening, not while validation fails — users
  should be able to keep filling required fields. The validation gate
  lives on the Open App button.

- Extract the inline SUN_ICON_SVG data URI used by the WithIcon stories
  to src/test/fixtures/storyIcons.ts so the next story that needs it
  doesn't grow a third copy.

- Tests: add the AppDetailPanel divider-conditional cases (rendered
  with fields, omitted without). Per-file coverage stays at 100% for
  AppListItem, AppDetailPanel, ToolListItem, ToolDetailPanel, and the
  new toolUtils. Suite: 70 files / 618 tests passing.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* test(toolUtils): rename empty-string test for readability

Drops the doubled "is ... is preserved" phrase in the test name.
Behavior unchanged.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-04 10:28:58 -04:00
Cliff Hall
8fa1a0c074
feat(apps): add isAppTool helper in core (#1260) (#1272)
* feat(apps): add isAppTool helper in core (#1260)

Adds @modelcontextprotocol/ext-apps as a dependency (root + clients/web)
and a thin core/mcp/apps.ts wrapper exposing:

- getAppResourceUri(tool): re-export of getToolUiResourceUri so all
  Inspector clients consume the same nested/flat _meta resolution.
- isAppTool(tool): boolean predicate built on top of getAppResourceUri.

Both surface the underlying ext-apps throw on a malformed _meta.ui.resourceUri
(string that does not start with "ui://") rather than swallowing it — server
bugs should be visible.

Tests cover nested format, deprecated flat format, nested-wins precedence,
missing _meta, and the invalid-URI throw.

Refs #1260

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* test(apps): address PR #1272 review feedback

- core/mcp/apps.ts: convert getAppResourceUri from a typed const re-export to
  a function declaration so its export style matches isAppTool. Behavior is
  unchanged.
- core/mcp/apps.ts: document the filter-iteration implication on isAppTool —
  callers using tools.filter(isAppTool) will halt on a server-malformed URI.
- apps.test.ts: add the symmetric isAppTool false-case for `_meta.ui` present
  without `resourceUri`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-01 17:03:11 -04:00
Cliff Hall
75f74d64c1
test(coverage): bootstrap vitest unit infra and add tests at 90%+ per file (#1245) (#1269)
* test(coverage): bootstrap vitest unit infra and add tests at 90%+ per file (#1245)

Establishes the unit-test runtime AGENTS.md mandates: a vitest "unit"
project alongside the existing storybook project, RTL + jest-dom +
happy-dom, a Mantine-aware render helper, and a 90%-line per-file
coverage gate enforced by CI. Adds 66 test files covering all 62
components under elements/groups/screens/views, plus core/mcp and
clients/web/src/utils. Total: 577 tests, 98.7% line coverage globally.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* test(coverage): address PR #1269 review feedback

- AGENTS.md: correct coverage gate description (lines ≥ 90, statements ≥ 85,
  functions ≥ 80, branches ≥ 50) with rationale for the relaxed branch
  threshold
- InspectorView.test.tsx: document what the Math.random() mock values
  (0.1 / 0.99) control vs. STUB_SUCCESS_RATE in InspectorView.tsx
- vite.config.ts / tsconfig.test.json: drop unused `globals: true` and the
  `vitest/globals` type now that every test file imports describe/it/expect/vi
  explicitly; manual `afterEach(cleanup)` in setup.ts remains required because
  testing-library auto-cleanup relies on globalThis.afterEach

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-01 15:15:32 -04:00
Cliff Hall
64a42e645b
fix(stories): scope ResourceSelected click to URIs region (#1248) (#1255)
The ResourceSelected story passed both sampleResources and
sampleSubscriptions, each of which renders 'config.json' as text. The
play helper's canvas.findByText('config.json') matched two nodes and
rejected.

Extends clickByText to accept an optional regionName regex; ResourceSelected
now scopes the click to within the 'URIs' Accordion panel by aria-region
name, so the subscriptions panel's matching label is filtered out without
changing what the story demonstrates.

Full storybook suite now green (273/273), unblocking #1251.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-29 17:24:52 -04:00
Cliff Hall
bcb5b643ff
docs(apps): specify the v2 Apps tab (#1265)
Adds the missing Apps tab to the v2 UX specification. Apps are the subset
of tools whose `_meta.ui.resourceUri` advertises a UI resource (per
`@modelcontextprotocol/ext-apps`). The screen mirrors the legacy v1 layout:
a sidebar of apps and a right pane that toggles between an input form
(before launch) and the embedded MCP App via `AppBridge`.

- v2_ux.md: add `[Apps]` to the navigation tab list and a new "Apps Screen"
  subsection with both states (input form / running app).
- v2_ux_features.md: full Apps Screen feature spec — ASCII diagrams,
  detection rule, presentational contract for `AppsScreen` / `AppRenderer`
  (imperative ref shape so the screen stays "dumb" and the wiring layer
  drives the bridge).
- v2_screenshots.md: link the two reference PNGs from the legacy v1 client.
- v2_ux_storybook.md: add `AppListItem` (2.3a), `AppDetailPanel` (2.5a),
  `AppRenderer` (2.5b), and `AppsScreen` (3.2a); update Build Order.

Implementation work tracked on the v2 board: #1260–#1264.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-29 17:22:59 -04:00
Cliff Hall
5a2d24bac7
docs(agents): require version label on new issues and PRs (#1257)
* docs(agents): require version label on new issues and PRs (#1256)

Recent v2 PRs (#1247, #1249, #1250, #1252, #1255) all landed without a
v2 label, making version-filtered queries unreliable. Codify the
expectation directly in the issue-driven workflow: each branch / board
gets a fixed label, and the label is set at create time via the
gh create flag rather than backfilled.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Also fix stale "PR against main" line

The PR-target line still said "Open a PR against main", which is
correct only for v1 work. Mirror the version-label mapping above so
the PR base matches the board.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-28 15:59:26 -04:00
Cliff Hall
39e1c67401
feat(experimental): derive client toggles from ClientCapabilities (#1252)
* feat(experimental): derive client toggles from ClientCapabilities (#1240)

Closes the Phase 5 audit partial: ExperimentalFeaturesPanel now accepts
clientExperimental: ClientCapabilities['experimental'] (the SDK record
shape) instead of a parallel clientToggles array of UI metadata.

Adopts Option B from the design question: toggle metadata (label,
description) lives in a static CLIENT_EXPERIMENTAL_TOGGLE_METADATA map
inside the component file. Iteration is over Object.keys of the live
record, so presence-in-record means enabled. Unknown keys (in record but
not in the metadata map) render with the raw key as label and no
description, per AC.

Renders one Checkbox per active toggle plus an indented HintText
description when metadata is known. Callback signature unchanged.

Stories cover empty, all-known, and mixed-known-and-unknown cases.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Render full toggle universe, derive checked from record

Address in-review feedback: with the strict-AC iteration over
Object.keys(clientExperimental), unchecking a toggle made its row
disappear (no schema slot for "disabled but available"), so the user
couldn't re-enable a toggle they just turned off.

Refines Option B: render the union of metadata-map keys and record keys.
Each row's checked state is derived from name-in-record. Result: known
toggles always render, regardless of record state; toggling is fully
reversible from the UI; unknown record keys still surface with a raw
label.

Story rename: NoClientToggles → NoneEnabled, AllKnownClientToggles →
AllKnownEnabled, MixedKnownAndUnknownToggles → MixedKnownAndUnknown —
the new names describe state, not record shape.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-28 12:33:08 -04:00
Cliff Hall
c04f96ffb8
feat(sampling): replace InlineSamplingRequest responseText with draftResult (#1239) (#1250)
Closes the Phase 5 audit partial: the inline card's prop interface now
matches SamplingRequestPanel's, accepting an optional CreateMessageResult
draft instead of a flat scalar. Lets future hook-layer wiring share a
single draft store between the inline card and the full panel.

Rendering:
- No draft → existing layout (request preview, model hints, action
  buttons), with the readonly placeholder textarea removed.
- Text draft → 2-line preview of draftResult.content.text (lineClamp 2).
- Image / audio draft → "[Image content]" / "[Audio content]" label,
  matching the ContentViewer fallback convention.

Updates the three consumer stories that threaded responseText="" through
(PendingClientRequests, ToolsScreen) — no callback signatures changed.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-27 18:29:58 -04:00
Cliff Hall
f1d6fbd704
feat(resources): render annotations on ResourceListItem (#1249)
* feat(resources): render annotations on ResourceListItem (#1238)

Half-finished Phase 5 spec target: ResourceListItem already used
title ?? name but ignored resource.annotations entirely. Now renders
audience and priority via the existing AnnotationBadge element, matching
how ResourcePreviewPanel surfaces the same fields.

Layout: a wrapping Group keeps the title + badges on a single line when
the row is wide, and lets badges drop below when narrow. When neither
annotation is present, the row renders the bare Text (no Group, no
leaked spacing).

Adds stories for audience-only, priority-only, both, and a
ResourceTemplate with annotations.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Address PR #1249 review feedback

- Inline the audience/priority narrowing checks instead of routing
  through aliased booleans, matching ResourcePreviewPanel's pattern
  and avoiding reliance on TS aliased-condition narrowing for the
  inner badges.
- Use a distinct resource name (settings.json) for WithAudience so
  the Storybook canvas reads more clearly across the four annotation
  stories.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-27 17:23:32 -04:00
Cliff Hall
9f452807f6
fix(history): clear stale methodFilter on clear-all (#1237) (#1247)
When the user clicked Clear in the History screen with a method filter
applied, the filter persisted across the entry reset. Subsequent entries
of a different method then displayed as zero results because the now-
orphaned filter still applied.

Wraps onClearAll at the screen level to also reset methodFilter to
undefined (Option C from the issue) — narrow, no effects, no render-time
divergence between the displayed and applied filter.

Adds a play-driven story exercising the interaction.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-27 15:43:54 -04:00
cliffhall
3afb9803fb Update AGENTS.md with issue driven workstyle and more context about the three main branches and the associated projects
Update package.json with a validate script
2026-04-27 12:14:06 -04:00
Cliff Hall
63e112113e
Merge pull request #1236 from modelcontextprotocol/v2/phase-5-spec-audit
Phase 5 audit: annotate spec, close HistoryControls partial
2026-04-27 11:59:04 -04:00
cliffhall
6b3cecd736 Address PR #1236 review feedback
- Rename RequestMethod → MessageMethod. The type covers notifications and
  results, not just requests; the new name matches the JSDoc.
- Normalize imports to the @inspector/core/mcp/types.js path alias in
  HistoryListPanel, historyUtils, and HistoryScreen (HistoryControls was
  already using the alias).
- Document the cast invariant in extractMethod: SDK types message.method
  as string, but every MessageEntry in this app originates from SDK
  schemas, so the narrowing is safe.
- Type SAMPLE_METHODS as MessageMethod[] in HistoryControls stories so
  the literal-type narrowing isn't lost across the spread.

Skipped two reviewer suggestions: the Mantine Select inline cast (reviewer
self-resolved) and the stale-methodFilter-on-clear UX edge case (deferred
as a separate follow-up — not a Phase 5 audit gap).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-27 10:24:30 -04:00
cliffhall
dde43ae47c Phase 5 audit: annotate spec, close HistoryControls partial
Audited every component under clients/web/src/components/ against its
v2_ux_interfaces.md entry. Added an "Audit results" section near the top
of the spec with the verified count (57/62 satisfied, 5 partial) and the
specific gaps still open.

While auditing, closed the HistoryControls gap directly:

- Added RequestMethod union type to core/mcp/types.ts, derived from the
  SDK's Client/Server Request and Notification schemas plus a synthetic
  "response" entry for result/error MessageEntries.
- Typed methodFilter and onMethodFilterChange in HistoryControls,
  HistoryListPanel, historyUtils.extractMethod with RequestMethod.
- Lifted the hardcoded METHOD_OPTIONS list out of HistoryControls into
  a required availableMethods prop. HistoryScreen now derives it from
  the live entries via useMemo + extractMethod.
- Updated HistoryControls.stories.tsx to pass availableMethods.

The remaining 5 partials (ElicitationUrlPanel, InlineElicitationRequest,
InlineSamplingRequest, ExperimentalFeaturesPanel, ResourceListItem) are
either blocked on the v2 core hook layer effort or call out follow-up
work that has its own design questions.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-27 09:53:43 -04:00
Cliff Hall
7558d72a79
Merge pull request #1234 from modelcontextprotocol/v2/phase-5-cleanup
Phase 5: Cleanup pass — lift ServerEntry into core/mcp/types.ts
2026-04-26 22:06:31 -04:00
cliffhall
5a60da2a12 Drop multi-line JSDoc on ServerEntry
Per AGENTS.md, comment blocks should be one short line. The interface
name and per-field invariant comments already document the shape; the
extra block was redundant.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-26 19:55:09 -04:00
cliffhall
c5d02456e0 Phase 5: Lift ServerEntry into core/mcp/types.ts
ServerEntry was previously defined as `Pick<ServerCardProps, ...>` in
ServerListScreen, inverting the natural dependency direction (data shape
derived from a UI prop type). Phase 5's cleanup pass calls this out
specifically.

Move the canonical ServerEntry into core/mcp/types.ts and have
ServerCardProps extend it. ServerListScreen, InspectorView, and their
stories now import ServerEntry from core directly.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-26 19:17:57 -04:00
Cliff Hall
56abced72b
Merge pull request #1229 from modelcontextprotocol/v2/phase-4-screen-selection-state
Phase 4: Move screen selection state into the screens
2026-04-26 17:06:20 -04:00
cliffhall
17d8532396 Address PR #1229 code review feedback
- ToolsScreen.fixtures.ts: replace empty `{}` schemas in query_database
  params and post_request body with explicit `type` for consistency
- ToolControls WithSearch story: add play function that types "git" so
  the story actually demonstrates filtering
- ResourcesScreen stories: comment expandUserProfileTemplate to document
  the userId/readUserProfileState.uri coupling

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-26 17:01:48 -04:00
cliffhall
d5559b4ea6 Make ResourcesScreen template/resource selection mutually exclusive
Selecting a resource now clears the active template selection (and vice
versa), so the right-hand panel reflects what the user just clicked.
The template's Read Resource button still only sets the resource URI,
preserving the template-with-preview layout for the read flow.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-26 11:38:11 -04:00
cliffhall
bb8241c2aa Share long tool fixture across ToolControls and InspectorView stories
Extract a 20-tool fixture (filesystem, database, git, http, misc) into
ToolsScreen.fixtures.ts — large enough that the ToolControls list scrolls
and the Tools tab in InspectorView shows realistic surface area.

Wire it into the Default stories of both ToolControls and InspectorView,
mirroring the LoggingScreen.fixtures.ts pattern.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-26 11:25:54 -04:00
cliffhall
4c1d2f2c34 Phase 4: Move screen selection state into the screens themselves
ToolsScreen, PromptsScreen, and ResourcesScreen each tracked the user's
current selection via `selectedXxxName` / `onSelectXxx` props, hoisting
screen-local UI state into whatever rendered them. InspectorView never
threaded those callbacks anywhere, so clicks in the controls panel did
not display the matching detail panel — the detail panel only rendered
if a parent fed selection back in.

Move selection into `useState` inside each screen and drop the
`selectedXxxName` / `onSelectXxx` props. MCP-derived data (`callState`,
`getPromptState`, `readState`) keeps flowing in from `InspectorView` as
props. ResourcesScreen wraps `onReadResource` so a template-expanded URI
becomes the selected resource for preview rendering.

Update each screen's stories to drop the deleted props. Stories that
need a pre-selected item use a `play` function to click it — no
component coupling to Storybook.

Add Phase 4.3 to the interfaces plan describing the rule.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-26 09:27:22 -04:00
Cliff Hall
d9b2009804
Merge pull request #1221 from modelcontextprotocol/v2/phase-2-server-settings-viewshell
Phase 2: Refactor Server/Settings/ViewShell cluster to MCP SDK types
2026-04-25 19:56:35 -04:00
cliffhall
1ed81c5f0e Address PR #1221 review round 4 feedback
- InspectorView: cancel any in-flight stub handshake timer when a new
  connect request arrives or the user disconnects, via a `useRef`-backed
  timer handle. Prevents a stale handshake resolver from overwriting the
  next attempt's connection state.
- InspectorView: document that screens unmount on tab switch (matches
  prior `{activeTab === X && <Screen />}` behavior; local screen state
  resets on every transition). If Phase 3 needs persistence, add
  `keepMounted` to the Transition.
- InspectorView: document the `mih: "100%"` height assumption on
  ScreenStageContainer — depends on `AppShell.Main` providing a definite
  parent height for the stack itself to fill (absolute children render
  regardless, but nested ScrollArea screens may need this).
- ViewHeader: comment the `Select w={140}` magic number — sized to fit
  the longest current tab label ("Resources").

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-25 19:50:58 -04:00
cliffhall
1785a5281c Share LoggingScreen fixture with InspectorView story; tune transition timing
- Extract `mixedEntries` (50 mixed-level log entries) into
  `LoggingScreen.fixtures.ts` so both `LoggingScreen.stories` and
  `InspectorView.stories` can consume it. The InspectorView Default story
  now shows a populated, scrollable Logs tab.
- InspectorView screen transitions: separate enter (350ms) and exit (250ms)
  durations via Mantine Transition's `exitDuration` prop, so screens leave
  faster than they enter.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-25 19:46:18 -04:00
cliffhall
d3c2645e38 Animate InspectorView screen transitions and refactor App.tsx
- InspectorView: wrap each screen in a Mantine `Transition` (`fade-up`,
  350ms). Outgoing screens fade out and slide down 30px; incoming screens
  fade in and slide up to meet the ViewHeader. A `pos: "relative"` Stack
  container holds the screens so the absolute-positioned `Transition`
  wrappers stack in the same area without pushing layout.
- InspectorView: reorder ALL_TABS so Logs sits beside History at the end:
  Servers · Tools · Prompts · Resources · Tasks · Logs · History.
- App.tsx: refactor to AGENTS.md guidelines — extract `PageContainer`,
  `PageStack`, `ThemeToggle`, `SubtitleText` as `.withProps()`
  subcomponents; replace inline unicode glyphs with `MdLightMode` /
  `MdDarkMode` icons (matches the ViewHeader pattern); convert the click
  handler to a `function` declaration.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-25 19:33:57 -04:00
cliffhall
473bd95826 Address PR #1221 review round 3 feedback
- InspectorView: simulate the connect flow through "connecting" with a
  randomized 50-500ms handshake delay and an 85% success rate. On success,
  measure observed latency from connect start; on failure, set a plausible
  error message. Connection error message flows through to the active
  ServerCard's connection state. Real wiring (Phase 3) replaces this with
  `useConnection`.
- InspectorView: document that the view owns connection state (input
  `serversInput` connection fields are intentionally ignored), and mark
  `noop` callbacks as Phase-3 stubs.
- ServerStatusIndicator: add `showLabel?: boolean` escape hatch so consumers
  in constrained containers can override the default viewport-based
  visibility.
- ServerListScreen.stories: add `WithActiveServer` story exercising the
  dimming + `inert` behaviour in isolation.
- ViewHeader.stories: rename "Logging" → "Logs" to match `ALL_TABS` in
  InspectorView.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-25 19:05:18 -04:00
cliffhall
0babbfa6a9 Remove ConnectedView and UnconnectedView, superseded by InspectorView
The two prior shell views took `children` and routed connection state by
having two distinct mounts. `InspectorView` collapses both into a single
state-machine view that switches `ViewHeader` modes internally, so the
old shells have no remaining call sites.

Spec section 4 (Views) rewritten to describe `InspectorView` instead.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-25 18:38:45 -04:00
cliffhall
4e99e6393f Make ViewHeader and ServerStatusIndicator responsive
Header now adapts cleanly across widths instead of wrapping or clipping:

- LeftSection/RightSection are symmetric (`flex: 1, miw: 0, wrap: nowrap`),
  so the natural-width CenterSection sits visually centered between them.
  Server name truncates with ellipsis when the left slice is too narrow.
- Below `md` (992px) the SegmentedControl swaps for a Select dropdown so
  the tab labels never have to shrink or wrap.
- Below `sm` (768px) the "Disconnect" button collapses to an MdLinkOff
  ActionIcon (with `aria-label` + `title` for the tooltip).
- Below `lg` (1200px) ServerStatusIndicator hides its text label and shows
  only the colored dot (full label still available as a tooltip on hover),
  freeing space so the Disconnect button stops getting truncated at typical
  desktop widths.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-25 18:34:07 -04:00
cliffhall
9553fb9c8a Add InspectorView state machine driving screens from props
InspectorView replaces the prior pair (ConnectedView/UnconnectedView wrapping
children) with a single state-machine view. Holds activeServer, connectionStatus,
initializeResult, latencyMs, activeTab, and availableTabs internally; renders
the appropriate screen based on activeTab; switches the header between connected
and unconnected modes.

Screen data (servers, tools, prompts, resources, resourceTemplates, subscriptions,
logs, tasks, progressByTaskId, history) flows in via props so stories can populate
the entire view with fixtures. Demo data lives in stories, not in the view.

Stories include one server per transport (stdio, sse, streamable-http) so the
TransportBadge / descriptor variants are all visible.

Reorders ViewHeader.stories tabs to match the new ALL_TABS sequence.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-25 18:33:52 -04:00
cliffhall
1149288ea6 Refactor ServerCard stack to use id-based callbacks
Distinguishes server `id` (the unique map key in MCPConfig.mcpServers)
from `name` (display label) so containers can dispatch by id without
relying on display strings. ServerCard, ConnectionToggle, ServerListScreen,
and supporting stories all updated.

- ServerCard: add `id` prop; all action callbacks now `(id: string) => void`
- ConnectionToggle: collapse `onConnect`/`onDisconnect` into single `onToggle`
- ServerListScreen: lift `activeServer` to a prop; combine `onSetActive` +
  `onToggleConnection` into a single `onToggleConnection(id)` callback
- Stories: use UUIDs for server ids so callbacks visibly carry ids, not names

Removes the "Direct"/"Via Proxy" connection mode concept, which is no
longer planned. ServerSettingsForm drops the section, ServerCard drops
the badge, InspectorServerSettings drops the field.

Removes the "Test Client Features" dropdown from ServerCard along with
its four handlers; that surface will live elsewhere.

TransportBadge now collapses `sse` and `streamable-http` into a single
"HTTP" label; ServerCard adds a grey transport descriptor next to the
badge ("Standard I/O", "SSE (Server Sent Events) [deprecated]",
"Streamable HTTP"). The version badge moves from the header row to sit
beside the transport badge.

ServerCard header no longer wraps; long server names truncate with
ellipsis.

Specification docs (`v2_scope.md`, `v2_screenshots.md`,
`v2_ux_interfaces.md`, `v2_ux_storybook.md`) updated to match.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-25 18:33:38 -04:00
cliffhall
29ce55290f Fix ServerSettingsModal stories to use useArgs for action logging
Use args + useArgs pattern so onSettingsChange and onClose fire
through Storybook's args system. Every form interaction now logs
to the Actions panel with the full updated settings object.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-21 20:19:39 -04:00
cliffhall
d27018b4f2 Change onOAuthChange to pass all OAuth fields together
Replace onOAuthChange(field, value) with onOAuthChange(oauth) where
oauth is { clientId, clientSecret, scopes }. Each keystroke now
reports the full OAuth state, not just the single field that changed.
This lets the consumer see the complete picture on every change.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-21 20:15:12 -04:00
cliffhall
e04befa4dd Fix ServerSettingsForm stories to use useArgs for action logging
Use Storybook's useArgs hook from storybook/preview-api to update
args from within the render function. This wires fn() callbacks to
the Actions panel AND updates the UI state through Storybook's args
system. All form interactions (add/remove/edit headers/metadata,
accordion open/close, connection mode, timeouts, OAuth) now both
update the UI and log to the Actions panel.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-21 19:37:27 -04:00
cliffhall
a3be456f56 Remove unused fn() from ServerSettingsForm stories
The Storybook config has no addon-actions/addon-essentials, so fn()
calls never produce visible output. The stories are fully interactive
via useState — add/remove/edit headers, metadata, timeouts, OAuth,
and connection mode all work visually in the UI.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-21 18:13:40 -04:00
cliffhall
66dcd82a42 Log actions alongside state updates in ServerSettingsForm stories
Each handler now both updates local state (so the UI reacts) and
calls a Storybook fn() spy (so the Actions panel logs every
callback with its arguments).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-21 18:04:16 -04:00
cliffhall
39ed8b0fec Make ServerSettingsForm stories fully interactive
Replace fn() stubs with useState handlers so add/remove/edit for
headers, metadata, connection mode, timeouts, and OAuth all work
interactively in Storybook. The InteractiveForm wrapper manages
settings state the same way ServerSettingsModal does.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-21 18:00:05 -04:00
cliffhall
e54c186603 Fix ServerSettingsModal stories rendering as blank screen
Wrap modal stories in AppShell to provide a page background behind
the modal overlay. Add fullscreen layout parameter.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-21 17:50:30 -04:00
cliffhall
a6fc900271 Add ServerSettingsModal group, revert form to controlled component
Revert ServerSettingsForm to a controlled component with
expandedSections/onExpandedSectionsChange props. The form is a
dumb component — accordion state belongs to its container.

Create ServerSettingsModal group that manages:
- Accordion expand/collapse state with ListToggle expand-all button
- Add/remove/edit header pairs
- Add/remove/edit metadata pairs
- Connection mode, timeout, and OAuth changes
- All changes flow through a single onSettingsChange callback

Update ServerSettingsForm stories with useState render functions
so accordion sections are interactive.

Update UnconnectedView WithSettingsModal story to use the new
ServerSettingsModal component.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-04-21 17:13:57 -04:00