Add interactive learning sandbox with live MCP server exploration#1672
Open
chelojimenez wants to merge 3 commits intomainfrom
Open
Add interactive learning sandbox with live MCP server exploration#1672chelojimenez wants to merge 3 commits intomainfrom
chelojimenez wants to merge 3 commits intomainfrom
Conversation
Introduce a "Try MCP" group in the learning syllabus with three interactive modules (learning-tools, learning-resources, learning-prompts) that connect to a live learning server. Key architectural decisions: - LearningStateProvider creates an isolated AppState + runtime API using its own useReducer(appReducer), keeping the learning server completely separate from workspace state. - AppStateProvider extended with optional runtimeApi prop and useSharedAppRuntime() hook for subtree-scoped server operations. - useLearningServer hook auto-connects on mount, reconnects on URL change, and disconnects on unmount with stale-op token tracking. - Hosted mode: runtime-config registry in web/context.ts lets buildHostedServerRequest resolve learning servers without Convex. - Runtime MCP API helpers (testRuntimeServerConnection, reconnectRuntimeServer) added to mcp-api.ts. https://claude.ai/code/session_01GNY4EtfFw35rBxUoqzm9cA
dosubot
bot
added
the
size:XL
Contributor
Author
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
|
🚅 Environment inspector-pr-1672 in triumphant-alignment has no services deployed. 1 service not affected by this PR
|
Contributor
WalkthroughThis pull request introduces a learning sandbox system for interactive exploration of MCP tools, resources, and prompts. It adds conditional routing in Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
mcpjam-inspector/client/src/state/mcp-api.ts (1)
173-216: Deduplicate the runtime wrappers.These two exports currently mirror
testConnection()andreconnectServer()line-for-line, so the next timeout or hosted-mode fix has to land in two places.♻️ Suggested simplification
export async function testRuntimeServerConnection( serverConfig: MCPServerConfig, serverId: string, ) { - if (HOSTED_MODE) { - // Runtime servers register their config in the hosted runtime registry - // so buildHostedServerRequest can resolve them without a Convex server ID. - return safeValidateHostedServer(serverId, serverConfig); - } - - const res = await authFetchWithTimeout( - "/api/mcp/connect", - { - method: "POST", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ serverConfig, serverId }), - }, - 20000, - ); - return res.json(); + return testConnection(serverConfig, serverId); } @@ export async function reconnectRuntimeServer( serverId: string, serverConfig: MCPServerConfig, ) { - if (HOSTED_MODE) { - return safeValidateHostedServer(serverId, serverConfig); - } - - const res = await authFetchWithTimeout( - "/api/mcp/servers/reconnect", - { - method: "POST", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ serverId, serverConfig }), - }, - 20000, - ); - return res.json(); + return reconnectServer(serverId, serverConfig); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mcpjam-inspector/client/src/state/mcp-api.ts` around lines 173 - 216, The two functions testRuntimeServerConnection and reconnectRuntimeServer duplicate logic from the existing server wrappers (e.g., testConnection and reconnectServer) — refactor to remove duplication by delegating to a single shared helper or the existing functions: extract the common authFetchWithTimeout call into a helper (e.g., callRuntimeServerEndpoint or reuse testConnection/reconnectServer) that accepts the endpoint ("/api/mcp/connect" or "/api/mcp/servers/reconnect"), payload ({serverId, serverConfig}) and timeout, and have both testRuntimeServerConnection and reconnectRuntimeServer simply call that helper (preserving the HOSTED_MODE branch that returns safeValidateHostedServer).mcpjam-inspector/client/src/components/LearningTab.tsx (1)
344-395: Consolidate module metadata to one source of truth.
INTERACTIVE_MODULE_IDS,labels, and explorer conditionals repeat the same IDs. A single registry object would reduce drift risk when modules evolve.♻️ Proposed refactor
-const INTERACTIVE_MODULE_IDS = new Set([ - "learning-tools", - "learning-resources", - "learning-prompts", -]); +const INTERACTIVE_MODULES = { + "learning-tools": { + title: "Explore Tools", + Explorer: LearningToolsExplorer, + }, + "learning-resources": { + title: "Explore Resources", + Explorer: LearningResourcesExplorer, + }, + "learning-prompts": { + title: "Explore Prompts", + Explorer: LearningPromptsExplorer, + }, +} as const; + +const INTERACTIVE_MODULE_IDS = new Set(Object.keys(INTERACTIVE_MODULES)); function InteractiveSandboxShell({ moduleId, @@ - const labels: Record<string, string> = { - "learning-tools": "Explore Tools", - "learning-resources": "Explore Resources", - "learning-prompts": "Explore Prompts", - }; + const moduleMeta = + INTERACTIVE_MODULES[moduleId as keyof typeof INTERACTIVE_MODULES]; + const ExplorerComponent = moduleMeta?.Explorer; @@ - {labels[moduleId] ?? "Try MCP"} + {moduleMeta?.title ?? "Try MCP"} @@ - {moduleId === "learning-tools" && <LearningToolsExplorer />} - {moduleId === "learning-resources" && <LearningResourcesExplorer />} - {moduleId === "learning-prompts" && <LearningPromptsExplorer />} + {ExplorerComponent ? <ExplorerComponent /> : null}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mcpjam-inspector/client/src/components/LearningTab.tsx` around lines 344 - 395, The module IDs and labels are duplicated across INTERACTIVE_MODULE_IDS, the labels map and the conditional JSX in InteractiveSandboxShell, causing drift; consolidate into a single registry constant (e.g., INTERACTIVE_MODULE_REGISTRY) that maps id -> { label, component } and replace INTERACTIVE_MODULE_IDS, the labels object, and the three conditional renderings by deriving the label and the explorer component from that registry inside InteractiveSandboxShell (look up moduleId to render the correct explorer component and label, and fall back to defaults when missing).mcpjam-inspector/client/src/components/learning-sandbox/LearningConnectionGate.tsx (1)
22-43: Add live-region semantics for loading/error feedback.Consider adding
role="status"+aria-live="polite"for loading states androle="alert"for failure state so screen-reader users get immediate connection feedback.Also applies to: 49-53
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mcpjam-inspector/client/src/components/learning-sandbox/LearningConnectionGate.tsx` around lines 22 - 43, The loading and error JSX in LearningConnectionGate lack live-region semantics; update the container elements rendered when connectionStatus === "connecting" to include role="status" and aria-live="polite" so screen readers announce the loading message, and update the container rendered when connectionStatus === "failed" to include role="alert" (or role="status" with aria-live="assertive") so failures are announced immediately; apply the same changes to the other equivalent JSX block used for the second connection status return. Ensure you modify the outermost divs in the relevant conditional return blocks in the LearningConnectionGate component.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@mcpjam-inspector/client/src/hooks/use-learning-server.ts`:
- Around line 21-25: Wrap URL parsing in buildLearningServerConfig(url: string)
with a try-catch so it returns null on invalid/malformed URLs instead of
throwing; change its signature/return to MCPServerConfig | null. Update both
call sites that pass VITE_LEARNING_SERVER_URL into buildLearningServerConfig
(the two effects that currently call it) to check for a null return and skip
attempting to create a LearningConnection/connection when config is null (i.e.,
do not call connect or instantiate LearningConnectionGate logic if
buildLearningServerConfig returns null). Ensure any downstream variables/types
accept a nullable config and short-circuit the effect when config === null.
In `@mcpjam-inspector/client/src/lib/apis/web/context.ts`:
- Around line 45-63: runtimeServerConfigs should be a prototype-free object to
avoid collisions with keys like "toString" or "__proto__"; change its
initialization to a prototype-less map (e.g., Object.create(null)) and keep
registerRuntimeServerConfig and unregisterRuntimeServerConfig using the same
keys so buildHostedServerRequest will correctly detect only explicitly
registered servers; ensure any key-existence checks against runtimeServerConfigs
rely on direct property access (not inherited) so registry behavior is safe for
reserved names.
In `@mcpjam-inspector/client/src/state/LearningStateProvider.tsx`:
- Around line 66-67: The global opTokenRef (opTokenRef) currently stores a
single numeric token which is incremented on any connect/disconnect and is
checked by async callbacks, causing unrelated servers' in-flight ops to be
invalidated; change opTokenRef to track tokens per server (e.g., a Map or object
keyed by serverName) and update/increment only the token for the specific server
when that server reconnects/unmounts, and when starting async operations capture
and compare the token for that serverName rather than the global token so only
ops for that server are ignored when stale.
---
Nitpick comments:
In
`@mcpjam-inspector/client/src/components/learning-sandbox/LearningConnectionGate.tsx`:
- Around line 22-43: The loading and error JSX in LearningConnectionGate lack
live-region semantics; update the container elements rendered when
connectionStatus === "connecting" to include role="status" and
aria-live="polite" so screen readers announce the loading message, and update
the container rendered when connectionStatus === "failed" to include
role="alert" (or role="status" with aria-live="assertive") so failures are
announced immediately; apply the same changes to the other equivalent JSX block
used for the second connection status return. Ensure you modify the outermost
divs in the relevant conditional return blocks in the LearningConnectionGate
component.
In `@mcpjam-inspector/client/src/components/LearningTab.tsx`:
- Around line 344-395: The module IDs and labels are duplicated across
INTERACTIVE_MODULE_IDS, the labels map and the conditional JSX in
InteractiveSandboxShell, causing drift; consolidate into a single registry
constant (e.g., INTERACTIVE_MODULE_REGISTRY) that maps id -> { label, component
} and replace INTERACTIVE_MODULE_IDS, the labels object, and the three
conditional renderings by deriving the label and the explorer component from
that registry inside InteractiveSandboxShell (look up moduleId to render the
correct explorer component and label, and fall back to defaults when missing).
In `@mcpjam-inspector/client/src/state/mcp-api.ts`:
- Around line 173-216: The two functions testRuntimeServerConnection and
reconnectRuntimeServer duplicate logic from the existing server wrappers (e.g.,
testConnection and reconnectServer) — refactor to remove duplication by
delegating to a single shared helper or the existing functions: extract the
common authFetchWithTimeout call into a helper (e.g., callRuntimeServerEndpoint
or reuse testConnection/reconnectServer) that accepts the endpoint
("/api/mcp/connect" or "/api/mcp/servers/reconnect"), payload ({serverId,
serverConfig}) and timeout, and have both testRuntimeServerConnection and
reconnectRuntimeServer simply call that helper (preserving the HOSTED_MODE
branch that returns safeValidateHostedServer).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5144a421-4e76-4b15-8ee9-972818fdee9d
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (11)
mcpjam-inspector/client/src/components/LearningTab.tsxmcpjam-inspector/client/src/components/learning-sandbox/LearningConnectionGate.tsxmcpjam-inspector/client/src/components/learning-sandbox/LearningPromptsExplorer.tsxmcpjam-inspector/client/src/components/learning-sandbox/LearningResourcesExplorer.tsxmcpjam-inspector/client/src/components/learning-sandbox/LearningToolsExplorer.tsxmcpjam-inspector/client/src/components/lifecycle/learning-concepts.tsmcpjam-inspector/client/src/hooks/use-learning-server.tsmcpjam-inspector/client/src/lib/apis/web/context.tsmcpjam-inspector/client/src/state/LearningStateProvider.tsxmcpjam-inspector/client/src/state/app-state-context.tsxmcpjam-inspector/client/src/state/mcp-api.ts
Comment on lines
+21
to
+25
| function buildLearningServerConfig(url: string): MCPServerConfig { | ||
| return { | ||
| url: new URL(url), | ||
| transportType: "streamable-http", | ||
| } as MCPServerConfig; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
node <<'NODE'
for (const value of [
"https://learning-server.mcpjam.com/mcp",
"/mcp",
"not a url",
]) {
try {
console.log(value, "=>", new URL(value).toString());
} catch (error) {
console.log(value, "=> throws", error.name);
}
}
NODERepository: MCPJam/inspector
Length of output: 195
🏁 Script executed:
cat -n mcpjam-inspector/client/src/hooks/use-learning-server.tsRepository: MCPJam/inspector
Length of output: 2898
Guard buildLearningServerConfig() against malformed URLs.
The new URL() constructor throws synchronously for invalid URLs. If VITE_LEARNING_SERVER_URL contains a malformed value, the effect crashes before LearningConnectionGate can render.
Wrap URL construction in a try-catch and return null on failure, then skip the server connection when config is null. This applies at three locations: the function itself (lines 21–25) and both call sites within the effects (lines 49–50 and 65–66).
The initial repro confirms valid absolute URLs succeed while relative paths and invalid strings throw TypeError.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mcpjam-inspector/client/src/hooks/use-learning-server.ts` around lines 21 -
25, Wrap URL parsing in buildLearningServerConfig(url: string) with a try-catch
so it returns null on invalid/malformed URLs instead of throwing; change its
signature/return to MCPServerConfig | null. Update both call sites that pass
VITE_LEARNING_SERVER_URL into buildLearningServerConfig (the two effects that
currently call it) to check for a null return and skip attempting to create a
LearningConnection/connection when config is null (i.e., do not call connect or
instantiate LearningConnectionGate logic if buildLearningServerConfig returns
null). Ensure any downstream variables/types accept a nullable config and
short-circuit the effect when config === null.
Comment on lines
+45
to
+63
| const runtimeServerConfigs: Record<string, unknown> = {}; | ||
|
|
||
| /** | ||
| * Register a runtime server config so hosted request builders can resolve it | ||
| * without a Convex server ID. | ||
| */ | ||
| export function registerRuntimeServerConfig( | ||
| serverName: string, | ||
| config: unknown, | ||
| ): void { | ||
| runtimeServerConfigs[serverName] = config; | ||
| } | ||
|
|
||
| /** | ||
| * Remove a previously registered runtime server config. | ||
| */ | ||
| export function unregisterRuntimeServerConfig(serverName: string): void { | ||
| delete runtimeServerConfigs[serverName]; | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
node <<'NODE'
const registry = {};
console.log("truthy before registration:", Boolean(registry["toString"]));
registry["__proto__"] = { injected: true };
console.log("prototype changed:", Object.getPrototypeOf(registry).injected === true);
NODERepository: MCPJam/inspector
Length of output: 116
🏁 Script executed:
cat -n mcpjam-inspector/client/src/lib/apis/web/context.ts | head -80Repository: MCPJam/inspector
Length of output: 3263
🏁 Script executed:
rg -n "buildHostedServerRequest|runtimeServerConfigs\[" mcpjam-inspector/client/src/lib/apis/web/context.tsRepository: MCPJam/inspector
Length of output: 343
🏁 Script executed:
sed -n '268,290p' mcpjam-inspector/client/src/lib/apis/web/context.tsRepository: MCPJam/inspector
Length of output: 833
Use a prototype-free registry for runtime configs.
runtimeServerConfigs is a plain object, so names like "toString" or "constructor" read as truthy even when never registered, and "__proto__" mutates the registry's prototype. Since buildHostedServerRequest() checks this registry first (line 273), a legitimate hosted server with one of those names gets misrouted into the runtime path.
Suggested fix
-const runtimeServerConfigs: Record<string, unknown> = {};
+const runtimeServerConfigs = new Map<string, unknown>();
@@
export function registerRuntimeServerConfig(
serverName: string,
config: unknown,
): void {
- runtimeServerConfigs[serverName] = config;
+ runtimeServerConfigs.set(serverName, config);
}
@@
export function unregisterRuntimeServerConfig(serverName: string): void {
- delete runtimeServerConfigs[serverName];
+ runtimeServerConfigs.delete(serverName);
}
@@
- const runtimeConfig = runtimeServerConfigs[serverNameOrId];
- if (runtimeConfig) {
+ const runtimeConfig = runtimeServerConfigs.get(serverNameOrId);
+ if (runtimeServerConfigs.has(serverNameOrId)) {
return buildGuestServerRequest(
runtimeConfig,
undefined,
hostedApiContext.clientCapabilities,
);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mcpjam-inspector/client/src/lib/apis/web/context.ts` around lines 45 - 63,
runtimeServerConfigs should be a prototype-free object to avoid collisions with
keys like "toString" or "__proto__"; change its initialization to a
prototype-less map (e.g., Object.create(null)) and keep
registerRuntimeServerConfig and unregisterRuntimeServerConfig using the same
keys so buildHostedServerRequest will correctly detect only explicitly
registered servers; ensure any key-existence checks against runtimeServerConfigs
rely on direct property access (not inherited) so registry behavior is safe for
reserved names.
Comment on lines
+66
to
+67
| // Per-server op tokens so stale async results are ignored after reconnect/unmount. | ||
| const opTokenRef = useRef(0); |
Contributor
There was a problem hiding this comment.
Global stale-op token can drop valid results from other servers.
On Line 67 and Line 150, a single token is shared across all server names. Any new connect/disconnect invalidates unrelated in-flight operations, which can silently discard legitimate success/failure updates.
🐛 Proposed fix (per-server token tracking)
- // Per-server op tokens so stale async results are ignored after reconnect/unmount.
- const opTokenRef = useRef(0);
+ // Per-server op tokens so stale async results are ignored per server name.
+ const opTokenRef = useRef<Record<string, number>>({});
+
+ const nextToken = useCallback((name: string) => {
+ const next = (opTokenRef.current[name] ?? 0) + 1;
+ opTokenRef.current[name] = next;
+ return next;
+ }, []);
const connectRuntimeServer: AppRuntimeContextValue["connectRuntimeServer"] =
useCallback(async ({ name, config, silent }) => {
- const token = ++opTokenRef.current;
+ const token = nextToken(name);
@@
- if (opTokenRef.current !== token) return;
+ if (opTokenRef.current[name] !== token) return;
@@
- if (opTokenRef.current === token && infoRes.initInfo) {
+ if (opTokenRef.current[name] === token && infoRes.initInfo) {
@@
- if (opTokenRef.current !== token) return;
+ if (opTokenRef.current[name] !== token) return;
@@
- }, []);
+ }, [nextToken]);
const disconnectRuntimeServer: AppRuntimeContextValue["disconnectRuntimeServer"] =
useCallback(async (name: string) => {
- ++opTokenRef.current; // Invalidate any in-flight ops for this server.
+ opTokenRef.current[name] = (opTokenRef.current[name] ?? 0) + 1; // Invalidate only this server.Also applies to: 71-71, 89-89, 123-123, 135-135, 148-151
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mcpjam-inspector/client/src/state/LearningStateProvider.tsx` around lines 66
- 67, The global opTokenRef (opTokenRef) currently stores a single numeric token
which is incremented on any connect/disconnect and is checked by async
callbacks, causing unrelated servers' in-flight ops to be invalidated; change
opTokenRef to track tokens per server (e.g., a Map or object keyed by
serverName) and update/increment only the token for the specific server when
that server reconnects/unmounts, and when starting async operations capture and
compare the token for that serverName rather than the global token so only ops
for that server are ignored when stale.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR introduces an interactive learning sandbox that allows users to connect to a live MCP server and explore tools, resources, and prompts in real time. The sandbox is isolated from the main app state and provides a guided learning experience within the Learning tab.
Key Changes
LearningStateProvider (
LearningStateProvider.tsx): New provider component that creates an isolatedAppStatewith its own reducer instance. Manages connection/disconnection of runtime servers and exposes aAppRuntimeContextValueAPI for child components to interact with the learning server without affecting the main app state.useLearningServer hook (
use-learning-server.ts): Custom hook that auto-connects to a configurable learning server on mount, handles reconnection when the server URL changes, and cleans up on unmount. Reads the server URL from environment variables with a fallback default.Learning explorers (
LearningToolsExplorer.tsx,LearningResourcesExplorer.tsx,LearningPromptsExplorer.tsx): Three new explorer components that wrap existing tab components (ToolsTab,ResourcesTab,PromptsTab) with connection state management and loading/error gates.LearningConnectionGate (
LearningConnectionGate.tsx): Gating component that displays loading, error, or disconnected states while the learning server is connecting, and renders children only when fully connected.InteractiveSandboxShell (in
LearningTab.tsx): New shell component that wraps explorers inLearningStateProviderand provides back/complete navigation. Routes interactive modules through this provider.Runtime API context (
app-state-context.tsx): ExtendedAppStateContextwith newAppRuntimeContextValueinterface andAppRuntimeContext. AddeduseSharedAppRuntime()hook to access the runtime API from child components.Runtime server connection APIs (
mcp-api.ts): AddedtestRuntimeServerConnection()andreconnectRuntimeServer()functions to connect servers using explicit configs without workspace persistence. Supports both hosted and local modes.Runtime config registry (
context.ts): Module-scoped registry (runtimeServerConfigs) to store runtime server configs so hosted request builders can resolve them without a Convex server ID. AddedregisterRuntimeServerConfig()andunregisterRuntimeServerConfig()functions.Learning concepts (
learning-concepts.ts): Added new "Try MCP" learning group with three interactive modules: Explore Tools, Explore Resources, and Explore Prompts.Notable Implementation Details
opTokenRefto track operation tokens and ignore stale async results after reconnect/unmount, preventing race conditions.silent: trueto suppress console warnings during normal operation.https://claude.ai/code/session_01GNY4EtfFw35rBxUoqzm9cA
Note
Medium Risk
Adds new runtime server connection/state plumbing and a hosted-mode request path for non-persisted servers; mistakes could break server connectivity or hosted request resolution.
Overview
Adds a new interactive learning sandbox in
LearningTabwith three “Try MCP” modules that connect to a live learning server and let users explore Tools/Resources/Prompts with back/mark-complete controls.Introduces an isolated
LearningStateProvider+useLearningServerhook to auto-connect/disconnect a fixed learning server, plus aLearningConnectionGateto render loading/error states until connected.Extends state infrastructure with an optional
AppRuntimeContext(runtime connect/disconnect/get entry API), adds runtime connect/reconnect helpers inmcp-api.ts, and updates hosted request building via a module-scoped runtime-config registry so hosted calls can resolve runtime servers without a persisted workspace/server ID.Written by Cursor Bugbot for commit 204542f. This will update automatically on new commits. Configure here.