Shared code architecture

[BobDickinson]

This draft PR introduces a shared code architecture that enables code reuse across the MCP Inspector's CLI, TUI, and web client interfaces. This addresses the feature drift and maintenance burden caused by the CLI and web client maintaining separate implementations, with immediate code sharing across the CLI and TUI, and planned support for the web client (v2).

What's Included

1. TUI Integration

• Integrated the TUI from mcp-inspect as a first-class workspace
• TUI is accessible via mcp-inspector --tui flag
• Currently experimental; feature gaps include OAuth, completions, elicitation, and sampling

2. Shared Package (@modelcontextprotocol/inspector-shared)

• Created internal workspace package for shared MCP client code
• Includes InspectorClient - a comprehensive wrapper around the MCP SDK Client that manages:
  • Client and transport lifecycle
  • Message tracking and stderr logging
  • Server data management (tools, resources, prompts)
  • Event-driven updates via EventTarget (cross-platform compatible)
  • High-level MCP method wrappers with automatic parameter conversion

3. CLI Migration

• Migrated CLI to use InspectorClient from shared package
• Consolidated CLI helper functions into InspectorClient methods
• Removed duplicate code (CLI transport implementation, cli/src/client/ directory, etc)
• All CLI tests passing

4. Shared React Code

• useInspectorClient hook for reactive state management
• Works in both TUI and v2 web client (future)
• Subscribes to EventTarget events and provides reactive React state

Architecture

The shared package includes:

• shared/mcp/ - MCP client/server interaction (InspectorClient, transport creation, config loading)
• shared/react/ - Shared React code (hooks, components, etc.)
• shared/json/ - JSON utilities for parameter conversion
• shared/test/ - Test fixtures and harness servers

Both CLI and TUI now use the same InspectorClient implementation, ensuring consistent behavior and eliminating code duplication (both apps are now thin UX-only implementations).

Next Steps

Phase 4: TUI Feature Gap Implementation (Planned)

• Address TUI feature gaps (OAuth, completions, elicitation, sampling) by extending InspectorClient
• This dual-purpose approach: brings TUI to feature parity while preparing InspectorClient for v2 web client integration
• When complete, InspectorClient will be very close to ready for full v2 web client support

Phase 5: v2 Web Client Integration (Planned)

• Integrate InspectorClient with the v2 web client once Phase 4 is complete and v2 web client is ready
• Update InspectorClient and other shared code as required to support v2 while maintaining CLI and TUI compatability

Documentation

See docs/shared-code-architecture.md (included in this PR) for:

• Complete architecture overview
• InspectorClient capabilities and API
• Integration history and current usage patterns
• Detailed web client integration plan

Testing

• All CLI tests passing
• TUI tested with test harness servers
• TypeScript Project References ensure correct build ordering
• CI workflow updated to build shared package first

---

This is a draft PR for review. Feedback welcome on the architecture and integration approach.

[BobDickinson]

Another element that I probably should have implemented in this PR is that we can now build a comprehensive test suite around the shared InspectorClient that will provide very good coverage of the functional elements of the Inspector, so that all we really need to test in the TUI or web client is the UX part.

[BobDickinson]

I merged main and resolved conflicts. All projects build and all tests pass, CI tests pass. Should be ready to review/test.

[cliffhall]

@claude /review

[claude]

Claude Code is working…

I'll analyze this and get back to you.

View job run

[BobDickinson]

Per the suggestion from @olaservo - I think the idea of moving this code to a v1.5 branch makes sense. It will make it easier for people to review, test, and collaborate on the code.

If we do that, I'd want to make a couple of changes on my branch first.

• Rename shared to core - I named it shared instead of core because I thought there might be some situation where it needed to coexist with the v2 core, but I think core is the proper dir/package name and this is probably the time to make that change (whether or not this evolves into the v2 core).
• Remove client and server projects and tooling - I left these in place also with the idea that web would coexist with them if merged to main, but if this v1.5 test/proto branch is meant to show a possible future where web based on InspectorClient is the Inspector UX, then I'd prefer to clean up the branch now (and also remove the --web launcher option and just make the cli launch web by defailt like it uses to launch client/server).

It may or may not also make sense to resolve any major issues from the pending Claude review before we do this.

Anyway, if the branch plan is agreed, let me know and I'll make the above changes to my local branch and let you know when it's ready to be merged to this repo.

After the branch is created and populated, we can close this PR.

cc: @cliffhall

[BobDickinson]

@cliffhall - The Claude review failed. I'm not sure why it fails on this PR and not others. There are some open issues that may be relevant: anthropics/claude-code-action#223, which may be a duplicate of anthropics/claude-code-action#46.

[cliffhall]

Per the suggestion from @olaservo - I think the idea of moving this code to a v1.5 branch makes sense. It will make it easier for people to review, test, and collaborate on the code.

Agreed.

If we do that, I'd want to make a couple of changes on my branch first.

The proposed changes sound good.

It may or may not also make sense to resolve any major issues from the pending Claude review before we do this.

I think Claude may just be having difficulty with all the commits that went into it. Could be a single squash commit would make it easier.

Anyway, if the branch plan is agreed, let me know and I'll make the above changes to my local branch and let you know when it's ready to be merged to this repo.

I think that's good. We can treat 1.5 not as a deviation from what we have now but a whole new entity that's a stepping stone toward 2.0. We can review it and test the heck out of it against all the things and get confidence in it that way.

I added a v1.5/main branch you can create a new PR against with the above suggested changes (i.e., removing client and server, etc)

[cliffhall]

@claude /review

[claude]

Claude finished @cliffhall's task in 4m 54s —— View job

---

PR Review: Shared Code Architecture

• Gather context and understand the PR
• Review core architecture (core/ package)
• Review CLI migration
• Review TUI integration
• Review test coverage
• Review CI/build changes
• Post comprehensive review

---

Overview

This is a substantial, well-thought-out architectural PR that introduces a shared core package (@modelcontextprotocol/inspector-core) enabling code reuse across CLI, TUI, and the web client. The central piece is InspectorClient — a comprehensive wrapper around the MCP SDK Client that manages transport lifecycle, message tracking, OAuth, server state, and event-driven updates.

The PR is in good shape. The architecture is sound, the test suite is extensive (6000+ lines covering InspectorClient), and all packages build cleanly. The key concerns below are mostly refinements rather than blockers.

---

Strengths

• Clean environment injection pattern — InspectorClientEnvironment separates Node/browser concerns well. InspectorClient is truly portable.
• Comprehensive tests — core/__tests__/inspectorClient.test.ts at 6016 lines is excellent coverage: connection lifecycle, all MCP methods, pagination, sampling, elicitation, tasks, subscriptions, OAuth. This gives a solid regression foundation.
• Type-safe EventTarget — InspectorClientEventTarget with InspectorClientEventMap and TypedEvent<K> gives compile-time safety for all event subscriptions. This is a significant improvement over stringly-typed events.
• Web client already partially integrated — web/ already imports useInspectorClient, InspectorClient types, and auth types from core. This PR has done the heavy lifting for eventual full v2 integration.
• Good separation of concerns — ContentCache, MessageTrackingTransport, SamplingCreateMessage, ElicitationCreateMessage are all cleanly factored out of the main class.
• Build ordering enforced — TypeScript project references + CI workflow order (core → test → cli/web/tui) prevents build-time surprises.

---

Issues and Suggestions

1. Hardcoded client version in InspectorClient constructor

core/mcp/inspectorClient.ts:527

name: "@modelcontextprotocol/inspector",
version: "0.18.0",  // ← stale, current version is 0.20.0

The default clientIdentity.version is hardcoded to "0.18.0" while the package is 0.20.0. This should dynamically read from package.json or be updated as part of the version bump script. Fix this →

---

2. any types on transport fields and repeated params: any

core/mcp/inspectorClient.ts:348-349

private transport: any = null;
private baseTransport: any = null;

These could be typed as Transport | MessageTrackingTransport | null (importing Transport from @modelcontextprotocol/sdk/shared/transport.js). The type is already used in messageTrackingTransport.ts and types.ts.

Additionally, methods like listPrompts, readResource, listTools, etc. all use const params: any = ... when building request parameters. The root cause is that the MCP SDK client methods don't expose a generic _meta overlay type. A shared internal helper type for { _meta?: Record<string, string>; cursor?: string } would reduce the any proliferation without requiring SDK changes.

---

3. useInspectorClient uses any[] for lists

core/react/useInspectorClient.ts:24-27, 54-62

tools: any[];
resources: any[];
resourceTemplates: any[];
prompts: any[];

InspectorClient.getTools() returns Tool[], getResources() returns Resource[], etc. — these are all properly typed. The hook's interface and useState calls should use the SDK types directly. This is a minor but visible type-safety gap. Fix this →

---

4. InspectorClient is a very large class (3427 lines)

This is the most significant structural concern. The class handles: connection management, message tracking, stderr, fetch tracking, tools, resources, resource templates, prompts, completions, sampling, elicitation, roots, resource subscriptions, task management (requestor + receiver), OAuth (normal + guided), session storage, content caching, and logging.

While the current implementation works well and has good test coverage, consider whether some of these concerns could be decomposed into focused sub-managers (e.g. TaskManager, OAuthManager) that InspectorClient delegates to. This would make the class easier to navigate and test in isolation. This is a recommendation for future refinement rather than a blocker for this PR.

---

5. CI workflow doesn't trigger on core/** changes

.github/workflows/cli_tests.yml triggers on cli/** paths and builds core as a dependency, but there's no dedicated core test workflow triggered by core/** changes. A PR touching only core/ wouldn't trigger the CLI tests workflow (since cli/** didn't change), potentially missing regressions.

Suggest adding a core_tests.yml workflow (or adding core/** to the path triggers of cli_tests.yml) to ensure core tests run on every core change. Fix this →

---

6. OAuth state-machine.ts listed as RENAMED but appears to be heavily modified

The core/auth/state-machine.ts is listed as +92/-39 (RENAMED). It would help to clarify in documentation or comments what the source file was, since it's now the canonical OAuth state machine implementation.

---

7. pino as a peer dependency for core may be unexpected

The core package declares pino as a peer dependency. This is reasonable for Node environments but browser consumers (the web client) will need to satisfy this peer dependency or explicitly ignore it. Verify the browser bundle path doesn't accidentally pull in pino's Node-specific code.

---

8. Minor: @deprecated on InspectorClientLogger type

core/logging/logger.ts

/** @deprecated Use pino.Logger directly; kept for backward compatibility. */
export type InspectorClientLogger = pino.Logger;

Since this is a new internal package (not yet published as a public API), there are no external consumers to maintain backward compatibility with. If this type isn't used anywhere, it can be removed. If it is used, the deprecation rationale should be noted for future cleanup.

---

Test Coverage

The test suite is impressive:

• core/__tests__/inspectorClient.test.ts — 6016 lines covering all major features
• Full OAuth E2E tests (inspectorClient-oauth-e2e.test.ts, inspectorClient-oauth.test.ts, etc.)
• useInspectorClient hook tests
• Content cache, transport, remote transport, storage adapter tests

One gap: there are no tests specifically validating the useInspectorClient hook's serialization-based deduplication (previousMessagesRef, previousStderrLogsRef) to prevent infinite re-render loops. These ref-based guards are non-trivial and worth a focused test.

---

Summary

This is a well-executed architectural refactor. The InspectorClient + InspectorClientEventTarget + environment injection pattern is sound and the existing web client integration confirms the architecture's viability. The main actionable items are:

1. Fix stale hardcoded version (0.18.0 → should match 0.20.0)
2. Type the transport fields (remove any on transport/baseTransport)
3. Type the hook's list arrays (remove any[] in useInspectorClient)
4. Add a core_tests.yml CI workflow for core/** path triggers

The PR is ready to merge to v1.5/main once the above are addressed (particularly the hardcoded version).

[BobDickinson]

I addressed the following concerns from the Claude review:

• 1. Hardcoded client version in InspectorClient constructor
• 2. any types on transport fields and repeated params: any
• 3. useInspectorClient uses any[] for lists
• 5. CI workflow doesn't trigger on core/** changes
• 8. Minor: @deprecated on InspectorClientLogger type

I agree that InspectorClient is very large (the test module is even larger). We should look at decomposing it if there are clear ways to do it that provide advantages around separation of concerns, testability, etc (my suspicion is that will be the case). I'm not in favor of breaking it up just because it's large. In the world of AI, the agents and models often perform better with larger, less decomposed implementations. Anyway, I suspect there is a decomposition that probably makes sense, so I'll start investigating some strategies and rationale for this.

[BobDickinson]

I worked up a design for implementing sub-managers (as recommended) and added the design doc to this PR as docs/inspector-client-sub-managers.md. After this PR is merged, I'll make a new PR with an implementation of that and we can use that as a place to have the discussion about how to make InspectorClient more manageable.