Skip to content

Commit 62839f9

Browse files
BobDickinsonBobDickinson
committed
First cut at InspectorClient sub-manager refactor design.
1 parent 931c079 commit 62839f9

1 file changed

Lines changed: 193 additions & 0 deletions

File tree

‎docs/inspector-client-sub-managers.md‎

Lines changed: 193 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,193 @@
1+
# InspectorClient: Sub-Manager Extraction Strategy
2+
3+
This document explains why extracting focused sub-managers that `InspectorClient` delegates to is the right approach for managing the class’s size and complexity, and summarizes the recommended extractions and what would remain in the main class.
4+
5+
---
6+
7+
## Why Sub-Managers (Delegation), Not Multiple Client Classes
8+
9+
### Single entry point and lifecycle
10+
11+
Consumers (web app, CLI, TUI) expect **one** client object: one `connect()` / `disconnect()`, one place to pass environment (transport, fetch, logger, OAuth). Splitting into multiple public classes (e.g. `InspectorClient`, `InspectorOAuthClient`, `InspectorTasksClient`) would force callers to construct and wire several objects and to think about which class owns the connection. The current “one client, many methods” API is coherent and should stay.
12+
13+
### Shared state and the SDK Client
14+
15+
The MCP SDK `Client` is the single handle to the protocol. Lists (tools, resources, prompts, etc.), OAuth state, and task caches all ultimately depend on that connection and on the same event stream. If we split `InspectorClient` into multiple _public_ classes, we would have to either:
16+
17+
- Give each class a reference to the same SDK `Client` and duplicate coordination (who connects? who owns lists?), or
18+
- Introduce a “coordinator” that holds the client and the sub-clients, which is just the current `InspectorClient` under another name.
19+
20+
So the natural design is **one facade object** that owns the SDK client and the connection, and **internal** helpers that it delegates to. Those helpers receive only what they need (callbacks, adapters, config) and do not become separate public APIs.
21+
22+
### Clear boundaries without fragmenting the API
23+
24+
Sub-managers are **internal** implementation details. Each manager has a narrow responsibility (receiver tasks, OAuth, requestor tasks, tracking, etc.) and is given explicit dependencies (e.g. “call this to send a notification,” “call this to get request options”). The public API stays on `InspectorClient`: the same methods, the same types, the same single object. Callers do not see or depend on the managers.
25+
26+
### Testability and change isolation
27+
28+
Managers can be unit-tested in isolation with mocks for the adapter/callback. Changes to OAuth flow or task handling are confined to one module instead of a 3,400-line file. The main class becomes mostly wiring and delegation, which is easier to review and reason about.
29+
30+
### Summary
31+
32+
- **Do not** split `InspectorClient` into multiple public client classes or interfaces that consumers must assemble.
33+
- **Do** extract internal sub-managers that own specific state and behavior; `InspectorClient` keeps the single public API and delegates to them.
34+
35+
---
36+
37+
## Recommended Sub-Managers (Priority Order)
38+
39+
### 1. ReceiverTaskManager
40+
41+
**Responsibility:** Server-initiated tasks: create task record, TTL cleanup, hold payload promise, upsert/cancel, and notify the server of status.
42+
43+
**State:** `receiverTaskRecords` map, TTL config.
44+
45+
**Methods:** `createReceiverTask`, `emitReceiverTaskStatus`, `upsertReceiverTask`, `getReceiverTask`, `listReceiverTasks`, `getReceiverTaskPayload`, `cancelReceiverTask` (and `isTerminalTaskStatus` if kept as shared util).
46+
47+
**Interface:** Constructor takes `{ ttlMs, onEmitStatus: (task) => void, logger? }`. API: `create(opts)`, `get(taskId)`, `list()`, `getPayload(taskId)`, `upsert(task)`, `cancel(taskId)`.
48+
49+
**Usage:** Created in constructor (or first `connect()`). Message/elicit handlers in `connect()` call `receiverTaskManager.create(...)` and use the record; `tasks/get`, `tasks/cancel`, and initialize `tasks/list` handlers delegate to the manager. Manager calls `onEmitStatus` to send status notifications (InspectorClient implements that with `this.client.notification(...)`).
50+
51+
---
52+
53+
### 2. OAuthManager (OAuthFlowManager)
54+
55+
**Responsibility:** OAuth config and all OAuth flow orchestration (normal and guided), using existing `OAuthStateMachine` and `BaseOAuthClientProvider`.
56+
57+
**State:** `oauthConfig`, `oauthStateMachine`, `oauthState`.
58+
59+
**Methods:** `setOAuthConfig`, `authenticate`, `beginGuidedAuth`, `runGuidedAuth`, `setGuidedAuthorizationCode`, `completeOAuthFlow`, `getOAuthTokens`, `clearOAuthTokens`, `isOAuthAuthorized`, `getOAuthState`, `getOAuthStep`, `proceedOAuthStep`, and the private `createOAuthProvider` / `getServerUrl` (or a callback).
60+
61+
**Interface:** Constructor takes `getServerUrl`, `fetch`, `logger`, `getEventTarget`, and OAuth env (storage, navigation, redirectUrlProvider). Same public method signatures as today.
62+
63+
**Usage:** InspectorClient holds `oauthManager` when `options.oauth` is present. All public OAuth methods delegate to the manager. `getServerUrl` is implemented on InspectorClient and passed in.
64+
65+
---
66+
67+
### 3. RequestorTaskManager
68+
69+
**Responsibility:** Cache of client-initiated tasks and delegation to the SDK for get/result/cancel/list.
70+
71+
**State:** `trackedRequestorTasks` map.
72+
73+
**Methods:** `getRequestorTask`, `getRequestorTaskResult`, `cancelRequestorTask`, `listRequestorTasks`, and the internal `upsertTrackedRequestorTask` (and the notification handler that calls it).
74+
75+
**Interface:** Constructor takes a `RequestorTaskAdapter` (getTask, getTaskResult, cancelTask, listTasks) and dispatch callbacks for `taskStatusChange`, `taskCancelled`, `tasksChange`. API: same public methods plus `upsert(task)` for the notification handler.
76+
77+
**Usage:** InspectorClient implements the adapter (delegating to `this.client.experimental.tasks.*` and `getRequestOptions`). Task notification handler calls `requestorTaskManager.upsert(task)`. Public requestor-task methods delegate to the manager.
78+
79+
---
80+
81+
### 4. TrackingManager
82+
83+
**Responsibility:** Hold and trim the three observable lists (messages, stderrLogs, fetchRequests), add entries, and dispatch the corresponding events.
84+
85+
**State:** `messages`, `stderrLogs`, `fetchRequests`, and their max limits.
86+
87+
**Methods:** `addMessage`, `updateMessageResponse`, `addStderrLog`, `addFetchRequest`, `getMessages`, `getStderrLogs`, `getFetchRequests`.
88+
89+
**Interface:** Constructor takes `{ maxMessages, maxStderrLogEvents, maxFetchRequests, dispatch, logger? }`. API: add/update/get for each list.
90+
91+
**Usage:** Message-tracking, stderr, and fetch callbacks call into the manager. Getters delegate to the manager.
92+
93+
---
94+
95+
### 5. PendingSamplesElicitationManager
96+
97+
**Responsibility:** Hold pending sampling and elicitation requests and notify when they change.
98+
99+
**State:** `pendingSamples`, `pendingElicitations`.
100+
101+
**Methods:** `getPendingSamples`, `addPendingSample`, `removePendingSample`, `getPendingElicitations`, `addPendingElicitation`, `removePendingElicitation`.
102+
103+
**Interface:** Constructor takes a dispatch callback. API: get/add/remove for each list.
104+
105+
**Usage:** InspectorClient holds the manager and delegates all six methods.
106+
107+
---
108+
109+
### 6. ListSyncManager (ListStateManager)
110+
111+
**Responsibility:** Own cached lists (tools, resources, resourceTemplates, prompts, roots) and the “list” / “listAll” behavior: call SDK, update cache, dispatch list-changed events.
112+
113+
**State:** `tools`, `resources`, `resourceTemplates`, `prompts`, `roots`, `listChangedNotifications`.
114+
115+
**Methods:** All list getters/clears and all `listTools`, `listAllTools`, `listResources`, `listAllResources`, `listResourceTemplates`, `listAllResourceTemplates`, `listPrompts`, `listAllPrompts`, `getRoots`, `setRoots`, and any list-only helpers (e.g. `getToolByName` if it only touches these lists).
116+
117+
**Interface:** Constructor takes an adapter (SDK client + `getRequestOptions`) and dispatch. Manager implements all list/listAll/get/clear/setRoots and dispatches the appropriate events.
118+
119+
**Usage:** InspectorClient implements the adapter and delegates all list/roots methods and getters to the manager.
120+
121+
---
122+
123+
### 7. SessionManager
124+
125+
**Responsibility:** Persist and restore `InspectorClientSessionState` (e.g. fetch requests + timestamps) for a given session id.
126+
127+
**State:** None beyond what is passed in; session shape is defined by the client.
128+
129+
**Methods:** The logic inside `saveSession` and `restoreSession` (build/accept state, call storage, error handling and logging).
130+
131+
**Interface:** Constructor takes `InspectorClientStorage` and optional `logger`. API: `saveSession(sessionId, state)`, `loadSession(sessionId)`.
132+
133+
**Usage:** InspectorClient has `getSessionState()` (e.g. from TrackingManager) and `applySessionState(state)`. `saveSession()` / `restoreSession()` call the manager and then get/apply state.
134+
135+
---
136+
137+
## What Remains in InspectorClient
138+
139+
After extraction, InspectorClient would retain:
140+
141+
**Lifecycle and transport**
142+
143+
- Constructor: validate options, create sub-managers (injecting adapters/callbacks), initialize content cache, set config flags.
144+
- `connect()`: create transport (and optionally wrap with `MessageTrackingTransport`), attach listeners, register protocol request/notification handlers (which call into ReceiverTaskManager, etc.), run initialize, set up list-changed and other handlers.
145+
- `disconnect()`: close transport, clear transport/client references, clear or reset managers as needed.
146+
147+
**SDK client and environment**
148+
149+
- Owning the MCP SDK `Client` instance and the transport (`Transport | MessageTrackingTransport`, `baseTransport`).
150+
- `getRequestOptions()`, `buildEffectiveAuthFetch()`, and any other small helpers used by multiple managers or by `connect()`.
151+
152+
**Thin public API (delegation)**
153+
154+
- All existing public methods remain on InspectorClient; most become one-liners that delegate to the appropriate manager (OAuth, requestor tasks, list sync, tracking, pending, session) or to the SDK client (ping, callTool, readResource, getPrompt, getCompletions, subscribe/unsubscribe, setLoggingLevel, etc.).
155+
- Getters that today return instance fields instead return the corresponding manager’s getter (e.g. `getTools()` → `listSyncManager.getTools()`).
156+
157+
**Protocol and app-renderer wiring**
158+
159+
- Registration of request/notification handlers in `connect()` that coordinate with ReceiverTaskManager (createMessage/elicit with task, tasks/get, tasks/cancel, tasks/list).
160+
- `getAppRendererClient()` (proxy over the SDK client for the Apps tab).
161+
- Any remaining “glue” that routes SDK or transport events into the right manager (e.g. list_changed notifications into ListSyncManager, task notifications into RequestorTaskManager).
162+
163+
**Content cache**
164+
165+
- The content cache (`ContentCache` / `ReadOnlyContentCache`) can stay on InspectorClient and be populated by `readResource`, `getPrompt`, etc.; those methods may call into ListSyncManager only when they need current list state, or the cache may remain independent.
166+
167+
**Roots and subscriptions**
168+
169+
- If not moved into ListSyncManager, `getRoots` / `setRoots` and resource subscribe/unsubscribe stay as small wrappers around the SDK client; otherwise they live inside ListSyncManager and are delegated.
170+
171+
**Session id and restore**
172+
173+
- `getSessionId()`, `setSessionId()`, and the decision of when to call `restoreSession()` (e.g. after connect) remain on InspectorClient; the actual persist/load is in SessionManager.
174+
175+
In other words: **InspectorClient remains the single public facade**. It owns the connection, the SDK client, and the wiring; it creates and holds the sub-managers and delegates almost all domain behavior to them.
176+
177+
---
178+
179+
## Rough Code Impact
180+
181+
- **Current:** ~3,400 lines in a single class (constructor, ~80+ methods, ~50+ instance fields).
182+
- **Moved out (rough):**
183+
- ReceiverTaskManager: ~100–120 lines.
184+
- OAuthManager: ~350–400 lines (all OAuth methods and state).
185+
- RequestorTaskManager: ~100–120 lines.
186+
- TrackingManager: ~90–100 lines.
187+
- PendingSamplesElicitationManager: ~40–50 lines.
188+
- ListSyncManager: ~700–900 lines (all list/listAll/get/clear/setRoots and related state).
189+
- SessionManager: ~60–80 lines (save/restore logic; get/apply state may add a bit on the client side).
190+
- **Total moved:** on the order of **1,450–1,770 lines** into dedicated modules.
191+
- **Remaining in InspectorClient:** on the order of **1,650–1,950 lines**—constructor and setup, `connect()` / `disconnect()` and handler registration, `getRequestOptions()` and similar helpers, thin public methods that delegate to managers or the SDK client, and app-renderer proxy. The file would still be substantial but no longer a single 3,400-line monolith, with clear separation between “wiring and lifecycle” and “domain behavior in managers.”
192+
193+
These ranges are approximate; actual line counts will depend on formatting, how much duplication is removed, and how narrowly the manager boundaries are drawn.

0 commit comments

Comments
 (0)