Skip to content

Conversation

@r2hu1
Copy link
Contributor

@r2hu1 r2hu1 commented Jan 8, 2026

@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. enhancement New feature or request labels Jan 8, 2026
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 8, 2026

Walkthrough

ServersTab was updated to add client-side JSON export: a new in-component downloadJson helper and handleExportAsJsonClick produce a timestamped, formatted JSON export using formatJsonConfig. UI/menu was restructured to conditionally show an “Export Servers” action when connected servers exist, move “Add manually” and “Import JSON” into a hover panel, and wire analytics events. FileSymlink from lucide-react was imported. A separate WorkspaceSelector change only adjusted inline span formatting.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (3)
client/src/lib/json-config-parser.ts (1)

16-51: Consider logging skipped servers.

The implementation correctly handles SSE and stdio server types per MCP standards. However, servers lacking both url and command are silently skipped, which could obscure configuration issues during export.

📝 Optional enhancement to log skipped servers
     } else if ("command" in config && config.command) {
       const serverConfig: JsonServerConfig = {
         command: config.command,
         args: config.args || [],
       };

       // Only add env if it exists and has properties
       if (config.env && Object.keys(config.env).length > 0) {
         serverConfig.env = config.env;
       }

       mcpServers[key] = serverConfig;
+    } else {
+      console.warn(`Skipping server "${key}": missing required url or command`);
     }
   }
client/src/components/ServersTab.tsx (2)

51-52: Remove unused import.

MCPServerConfig is imported but not referenced in this file.

🧹 Proposed cleanup
-import { MCPServerConfig } from "@/sdk";
 import { formatJsonConfig } from "@/lib/json-config-parser";

85-85: Remove unused state variable.

isExportingJson is set but never checked elsewhere in the component.

🧹 Proposed cleanup
   const [isImportingJson, setIsImportingJson] = useState(false);
-  const [isExportingJson, setIsExportingJson] = useState(false);
   const [isEditingServer, setIsEditingServer] = useState(false);

And remove the setter call:

   const handleExportAsJsonClick = () => {
     posthog.capture("export_servers_to_json_button_clicked", {
       location: "servers_tab",
       platform: detectPlatform(),
       environment: detectEnvironment(),
     });
-    setIsExportingJson(true);
     setIsActionMenuOpen(false);
     const formattedJson = formatJsonConfig(connectedServerConfigs);
     const fileName = `mcp-servers-config-${new Date().toISOString()}.json`;
     downloadJson(fileName, formattedJson);
   };
📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between eb74667 and 207ea85.

📒 Files selected for processing (2)
  • client/src/components/ServersTab.tsx
  • client/src/lib/json-config-parser.ts
🧰 Additional context used
📓 Path-based instructions (1)
client/**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

Browser console.* methods are acceptable for client-side debugging in client code

Files:

  • client/src/lib/json-config-parser.ts
  • client/src/components/ServersTab.tsx
🧬 Code graph analysis (1)
client/src/lib/json-config-parser.ts (1)
client/src/hooks/use-app-state.ts (1)
  • ServerWithName (32-32)
🔍 Remote MCP

Based on my search of MCP configuration standards and documentation, here are the relevant facts for reviewing this PR:

MCP Server Configuration Standard Format

The MCP JSON configuration format defines how MCP clients should configure and launch MCP servers, providing a consistent way to specify server commands, arguments, and environment variables, using a mcpServers object where each key represents a server name.

Each MCP server entry object must have either the "command" property (for stdio transport) or the "url" property (for HTTP/SSE transport). The standard structure includes:

  • Stdio-type servers: If the "command" property is present, the MCP server uses stdio transport
  • SSE-type servers: Support for "url" property for HTTP-based transport
  • Optional properties: An object containing environment variables (env) to set when launching the server
  • Command arguments: An array of command-line arguments passed to the server executable, passed in order

Key Implementation Details for Review

The PR's formatJsonConfig function aligns with the standard by:

  1. Distinguishing transport types: The code differentiates between SSE-type servers (with url property) and stdio-type servers (with command property), matching the MCP specification
  2. Default args handling: The implementation defaults args to an empty array for stdio servers, which is appropriate as args are optional per the standard
  3. Conditional env inclusion: Environment variables are only included when present and non-empty, following best practices for configuration export

This implementation correctly produces JSON that conforms to the widely-adopted MCP configuration standard used by Claude Desktop, VS Code, Cursor, Amazon Q, and other MCP-compatible clients.

[::web_search::]

🔇 Additional comments (4)
client/src/lib/json-config-parser.ts (1)

2-2: LGTM!

The import is correctly used by the new formatJsonConfig function.

client/src/components/ServersTab.tsx (3)

7-7: LGTM!

The FileSymlink icon is appropriately used for the export menu item.


159-184: Export implementation works correctly.

The download mechanism and export handler follow established patterns. The ISO timestamp in the filename is browser-compatible, and analytics tracking aligns with existing conventions.


374-381: LGTM!

The export menu item integrates seamlessly with the existing action menu pattern.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In @client/src/components/ServersTab.tsx:
- Line 179: The filename generation in ServersTab.tsx uses new
Date().toISOString() which includes colons invalid on Windows; update the
fileName assignment (symbol: fileName) to use a filesystem-safe timestamp by
replacing colons with hyphens (e.g., call toISOString().replace(/:/g, '-')) or
by formatting the date to omit colons and/or milliseconds (e.g., build
YYYY-MM-DD-HH-mm-ss) so the resulting filename like
mcp-servers-config-2026-01-08-07-44-22.json is Windows-compatible.
🧹 Nitpick comments (3)
client/src/lib/json-config-parser.ts (1)

33-33: Consider type checking before toString() conversion.

If config.url is already a string type, the .toString() call is redundant. Consider either:

  • Removing .toString() if the type is guaranteed to be string, or
  • Adding a type guard if it could be a URL object
♻️ Potential simplification
-        url: config.url.toString(),
+        url: config.url,
client/src/components/ServersTab.tsx (2)

157-169: Improve type safety of downloadJson parameter.

The data: any parameter weakens type safety. Consider using data: JsonConfig or data: Record<string, unknown> to provide better type checking.

♻️ Proposed refactor
-  const downloadJson = (filename: string, data: any) => {
+  const downloadJson = (filename: string, data: JsonConfig) => {
     const blob = new Blob([JSON.stringify(data, null, 2)], {

Note: This requires importing JsonConfig from @/lib/json-config-parser.


171-181: Consider adding success feedback for export action.

For consistency with other actions in this component (e.g., tunnel creation at line 214), consider adding a toast notification after successful export.

✨ Suggested enhancement
   const handleExportAsJsonClick = () => {
     posthog.capture("export_servers_to_json_button_clicked", {
       location: "servers_tab",
       platform: detectPlatform(),
       environment: detectEnvironment(),
     });
     setIsActionMenuOpen(false);
     const formattedJson = formatJsonConfig(connectedServerConfigs);
     const fileName = `mcp-servers-config-${new Date().toISOString()}.json`;
     downloadJson(fileName, formattedJson);
+    toast.success("Servers exported successfully");
   };
📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 207ea85 and 84572f0.

📒 Files selected for processing (2)
  • client/src/components/ServersTab.tsx
  • client/src/lib/json-config-parser.ts
🧰 Additional context used
📓 Path-based instructions (1)
client/**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

Browser console.* methods are acceptable for client-side debugging in client code

Files:

  • client/src/components/ServersTab.tsx
  • client/src/lib/json-config-parser.ts
🧬 Code graph analysis (2)
client/src/components/ServersTab.tsx (2)
client/src/lib/PosthogUtils.ts (2)
  • detectPlatform (27-54)
  • detectEnvironment (56-58)
client/src/lib/json-config-parser.ts (1)
  • formatJsonConfig (21-53)
client/src/lib/json-config-parser.ts (1)
client/src/hooks/use-app-state.ts (1)
  • ServerWithName (32-32)
🔍 Remote MCP Context7

Summary of additional relevant facts for PR #1153 (Export servers to JSON)

  • Files touched
    • client/src/components/ServersTab.tsx — adds "Export as JSON" action, downloadJson helper, handleExportAsJsonClick (formats server configs, timestamped filename, triggers download, updates analytics, manages action menu UI).
    • client/src/lib/json-config-parser.ts — adds export function formatJsonConfig(serversObj: Record<string, ServerWithName>): JsonConfig that:
      • Maps servers into mcpServers entries.
      • Classifies servers as SSE-type when url is present, stdio-type when command is present.
      • Includes args (defaults to [] for stdio), includes env only when present/non-empty.
      • Logs a warning for servers missing both url and command.
  • Conformance to MCP JSON config expectations
    • The exporter produces mcpServers entries with either command (stdio) or url (SSE) and optional env/args, matching the MCP configuration standard described in the PR context.
  • UX/behavior notes to verify in review
    • Timestamp format used in filename (ensure acceptable for OS/filesystems).
    • Warning behavior when servers lack both url and command (confirm visibility and log destination).
    • Analytics event triggered on export (ensure no sensitive data sent).
    • Action menu state updated correctly (menu closes/opens as expected).
  • Minor changes mentioned in commits: "feat: log skipped servers" and "feat: rm unused vars" — verify logging and unused variable removals are intentional.

Tools/sources used

  • Context7_resolve-library-id (attempted lookup for MCP JSON format context; quota hit after initial response)
🔇 Additional comments (3)
client/src/lib/json-config-parser.ts (2)

35-46: Well-structured stdio server formatting.

The logic correctly defaults args to an empty array and conditionally includes env only when non-empty, producing clean JSON output.


29-35: No action needed—mutual exclusivity is enforced by the type system.

MCPServerConfig is a discriminated union (StdioServerConfig | HttpServerConfig) where StdioServerConfig sets url?: never to prevent the url property, and HttpServerConfig omits the command property entirely. This type-level constraint ensures both properties can never coexist. The validation function provides an additional safety check during import, and the if-else structure correctly handles the union type without issue.

client/src/components/ServersTab.tsx (1)

371-378: Export button integration looks good.

The button follows established patterns, uses an appropriate icon, and is consistently styled with sibling actions.

@matteo8p
Copy link
Collaborator

matteo8p commented Jan 9, 2026

@r2hu1 Functionality is great, thanks for taking the time to do this.

I don't think the export button should be nested in the "Add server" button. Can we move the export servers button out and put it to the left of the add server button?

@r2hu1
Copy link
Contributor Author

r2hu1 commented Jan 9, 2026

that make sense

@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. and removed size:M This PR changes 30-99 lines, ignoring generated files. labels Jan 9, 2026
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
client/src/components/ServersTab.tsx (1)

171-184: Export logic is sound; consider closing action menu for UX consistency.

The export implementation correctly formats data, generates a filesystem-safe timestamp, and triggers download. Analytics capture is appropriate—no sensitive data leaked.

One minor consideration: handleImportJsonClick (line 154) closes the action menu, but this handler does not. Since the Export button exists outside the HoverCard, it's not strictly necessary—but closing the menu when Export is clicked would provide consistent UX if the user had hovered over "Add Server" first.

♻️ Optional: close action menu on export
  const handleExportAsJsonClick = () => {
    posthog.capture("export_servers_to_json_button_clicked", {
      location: "servers_tab",
      platform: detectPlatform(),
      environment: detectEnvironment(),
    });
    const formattedJson = formatJsonConfig(connectedServerConfigs);
    const timestamp = new Date()
      .toISOString()
      .split(".")[0]
      .replace(/[T:]/g, "-");
    const fileName = `mcp-servers-config-${timestamp}.json`;
    downloadJson(fileName, formattedJson);
+   setIsActionMenuOpen(false);
  };
📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7858c43 and 01fe92e.

📒 Files selected for processing (1)
  • client/src/components/ServersTab.tsx
🧰 Additional context used
📓 Path-based instructions (1)
client/**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

Browser console.* methods are acceptable for client-side debugging in client code

Files:

  • client/src/components/ServersTab.tsx
🧬 Code graph analysis (1)
client/src/components/ServersTab.tsx (2)
client/src/lib/PosthogUtils.ts (2)
  • detectPlatform (27-54)
  • detectEnvironment (56-58)
client/src/lib/json-config-parser.ts (1)
  • formatJsonConfig (21-53)
🔍 Remote MCP Context7

Summary of additional relevant facts to check when reviewing PR #1153 (Export servers to JSON)

  • formatJsonConfig behavior

    • Converts ServerWithName -> JsonConfig.mcpServers; classifies SSE (url present) vs stdio (command present); includes args (defaults to []) and env only when non-empty; logs a warning and skips when neither url nor command present.
  • Filename / filesystem safety

    • Verify generated timestamped filename contains only filesystem-safe characters and correct timezone/format for reproducible names.
  • Sensitive data / analytics

    • Ensure analytics event triggered on export does not include server credentials, tokens, or other sensitive fields; exported JSON should not be inadvertently sent to analytics.
  • Download implementation concerns

    • Confirm downloadJson uses a Blob and triggers browser download in a cross-browser-safe way and handles large payloads without memory issues; verify action menu closes reliably after export.
  • Logging and UX

    • Confirm the warning for skipped servers is only developer-facing (console) and not shown to users; ensure skipped servers are intentional and documented.
  • UI placement change

    • PR already separates the Export button from the Add server button per reviewer request; verify final placement matches requested UX (export button to the left of Add).

Tool note: attempted Context7 lookup but monthly quota was exceeded during resolution; used available context and prior research to produce these review points.

🔇 Additional comments (3)
client/src/components/ServersTab.tsx (3)

7-7: LGTM: Clean imports for export functionality.

The FileSymlink icon and formatJsonConfig utility are appropriately imported and serve the export feature well.

Also applies to: 51-51


157-169: LGTM: Standard download implementation with proper cleanup.

The blob-to-download pattern is correctly implemented with memory cleanup via URL.revokeObjectURL. Cross-browser compatible for modern environments.


339-390: Excellent implementation—perfectly addresses reviewer feedback.

The Export Servers button is now cleanly separated from the Add Server button and positioned to its left, exactly as requested. Conditional rendering ensures export only appears when servers exist—sensible UX. The HoverCard structure for "Add manually" and "Import JSON" maintains clear action grouping.

@r2hu1
Copy link
Contributor Author

r2hu1 commented Jan 9, 2026

Strange

Screenshot 2026-01-09 at 9 43 33 PM

@matteo8p
Copy link
Collaborator

@r2hu1 Could you fix the merge conflicts?

r2hu1 and others added 5 commits January 16, 2026 12:18
@r2hu1 r2hu1 force-pushed the export-servers-to-json branch from 01fe92e to b1ca0d2 Compare January 16, 2026 06:56
@chelojimenez
Copy link
Contributor

chelojimenez commented Jan 16, 2026

Snyk checks have passed. No issues have been found so far.

Status Scanner Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues
Licenses 0 0 0 0 0 issues
Code Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@r2hu1
Copy link
Contributor Author

r2hu1 commented Jan 16, 2026

@matteo8p done

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
mcpjam-inspector/client/src/components/ServersTab.tsx (1)

1-1: Address Prettier formatting violation.

The pipeline indicates code style issues. Run npx prettier --write on this file to resolve before merging.

🤖 Fix all issues with AI agents
In `@mcpjam-inspector/client/src/components/ServersTab.tsx`:
- Line 502: Remove the now-unused prop from the ServersTab usage in App.tsx:
locate the <ServersTab ... /> JSX where
isLoadingWorkspaces={isLoadingRemoteWorkspaces} is being passed and delete that
prop entirely so that ServersTab is not given the removed isLoadingWorkspaces
prop; ensure no other references to isLoadingWorkspaces remain in App.tsx and
run TypeScript to verify the prop types align with ServersTabProps.
🧹 Nitpick comments (1)
mcpjam-inspector/client/src/components/ServersTab.tsx (1)

339-351: UI restructure satisfies the PR feedback — minor simplification possible.

The export button now sits independently to the left of "Add Server," as requested. However, connectedServerConfigs ?? {} is defensive coding for a required prop. Consider reusing the existing connectedCount variable for consistency:

✨ Suggested simplification
-      {Object.keys(connectedServerConfigs ?? {}).length > 0 && (
+      {connectedCount > 0 && (
📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 01fe92e and b1ca0d2.

📒 Files selected for processing (1)
  • mcpjam-inspector/client/src/components/ServersTab.tsx
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2026-01-10T18:55:38.803Z
Learnt from: CR
Repo: MCPJam/inspector PR: 0
File: mcpjam-inspector/AGENTS.md:0-0
Timestamp: 2026-01-10T18:55:38.803Z
Learning: Applies to mcpjam-inspector/server/**/*.{ts,tsx,js} : Do not use `console.log`, `console.warn`, or `console.error` directly in server code

Applied to files:

  • mcpjam-inspector/client/src/components/ServersTab.tsx
📚 Learning: 2026-01-10T18:55:38.803Z
Learnt from: CR
Repo: MCPJam/inspector PR: 0
File: mcpjam-inspector/AGENTS.md:0-0
Timestamp: 2026-01-10T18:55:38.803Z
Learning: Applies to mcpjam-inspector/server/**/*.{ts,tsx} : Use the centralized logger utility from `@/utils/logger` for server code with methods like `logger.error()`, `logger.warn()`, `logger.info()`, and `logger.debug()`

Applied to files:

  • mcpjam-inspector/client/src/components/ServersTab.tsx
📚 Learning: 2026-01-10T18:55:38.803Z
Learnt from: CR
Repo: MCPJam/inspector PR: 0
File: mcpjam-inspector/AGENTS.md:0-0
Timestamp: 2026-01-10T18:55:38.803Z
Learning: Applies to mcpjam-inspector/src/**/*.{ts,tsx,js,jsx} : Browser `console.*` is acceptable for client-side debugging

Applied to files:

  • mcpjam-inspector/client/src/components/ServersTab.tsx
🧬 Code graph analysis (1)
mcpjam-inspector/client/src/components/ServersTab.tsx (1)
mcpjam-inspector/client/src/lib/json-config-parser.ts (1)
  • formatJsonConfig (21-53)
🪛 GitHub Actions: Prettier and Build Check
mcpjam-inspector/client/src/components/ServersTab.tsx

[warning] 1-1: Code style issues found in file. Run Prettier with --write to fix.

🔇 Additional comments (4)
mcpjam-inspector/client/src/components/ServersTab.tsx (4)

7-7: Imports align with new export functionality.

FileSymlink and formatJsonConfig are both utilized appropriately downstream.

Also applies to: 51-51


157-169: Well-structured download utility with proper cleanup.

The URL.revokeObjectURL call prevents memory leaks. One minor refinement: data: any could be data: unknown for stricter type safety, though this is acceptable for a local helper.


171-184: Export handler is clean and functional.

Timestamp generation produces a filesystem-safe format. The formatJsonConfig utility gracefully handles malformed server entries by logging warnings rather than throwing.


352-390: HoverCard menu structure is intuitive.

The primary "Add Server" button provides direct access while the hover panel exposes additional options. This pattern balances discoverability with efficiency.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.

: connectedCount > 0
? renderConnectedContent()
: renderEmptyContent()}
{connectedCount > 0 ? renderConnectedContent() : renderEmptyContent()}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

rg -n "isLoadingWorkspaces" --type=ts --type=tsx

Repository: MCPJam/inspector

Length of output: 87


🏁 Script executed:

# Get more context on ServersTab component definition
rg -A 15 "interface ServersTabProps" --type=ts --type=tsx

Repository: MCPJam/inspector

Length of output: 87


🏁 Script executed:

# Check for ServersTab usage/imports to find callers
rg -n "ServersTab" --type=ts --type=tsx | head -30

Repository: MCPJam/inspector

Length of output: 87


🏁 Script executed:

# Find all TypeScript/TSX files first
fd -e tsx -e ts | head -20

Repository: MCPJam/inspector

Length of output: 1171


🏁 Script executed:

# Search for isLoadingWorkspaces without type restrictions
rg "isLoadingWorkspaces"

Repository: MCPJam/inspector

Length of output: 813


🏁 Script executed:

# Look for ServersTab.tsx directly
fd -n "ServersTab.tsx"

Repository: MCPJam/inspector

Length of output: 290


🏁 Script executed:

# Find ServersTab.tsx without -n flag
fd "ServersTab.tsx"

Repository: MCPJam/inspector

Length of output: 113


🏁 Script executed:

# Check App.tsx to see how ServersTab is used
rg -A 2 -B 2 "ServersTab" mcpjam-inspector/client/src/App.tsx

Repository: MCPJam/inspector

Length of output: 516


🏁 Script executed:

# Read ServersTab.tsx to check the props interface and line 502
cat -n mcpjam-inspector/client/src/components/ServersTab.tsx | grep -A 30 "interface ServersTabProps"

Repository: MCPJam/inspector

Length of output: 1220


🏁 Script executed:

# Check around line 502 specifically
sed -n '495,510p' mcpjam-inspector/client/src/components/ServersTab.tsx

Repository: MCPJam/inspector

Length of output: 428


🏁 Script executed:

# Check the full App.tsx ServersTab usage to see if isLoadingWorkspaces is still passed
rg -A 10 "<ServersTab" mcpjam-inspector/client/src/App.tsx

Repository: MCPJam/inspector

Length of output: 479


Update App.tsx to remove the unused isLoadingWorkspaces prop.

The isLoadingWorkspaces prop has been removed from ServersTabProps, but App.tsx still passes it. Remove the isLoadingWorkspaces={isLoadingRemoteWorkspaces} line from the <ServersTab> component in App.tsx to complete this refactor.

🤖 Prompt for AI Agents
In `@mcpjam-inspector/client/src/components/ServersTab.tsx` at line 502, Remove
the now-unused prop from the ServersTab usage in App.tsx: locate the <ServersTab
... /> JSX where isLoadingWorkspaces={isLoadingRemoteWorkspaces} is being passed
and delete that prop entirely so that ServersTab is not given the removed
isLoadingWorkspaces prop; ensure no other references to isLoadingWorkspaces
remain in App.tsx and run TypeScript to verify the prop types align with
ServersTabProps.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
mcpjam-inspector/client/src/components/ServersTab.tsx (1)

512-519: Remove the duplicate render; it bypasses loading and doubles content.

Line 519 unconditionally renders again, so content appears twice and isLoadingWorkspaces is effectively ignored.

🧩 Proposed fix
       {isLoadingWorkspaces
         ? renderLoadingContent()
         : connectedCount > 0
           ? renderConnectedContent()
           : renderEmptyContent()}
-      {connectedCount > 0 ? renderConnectedContent() : renderEmptyContent()}
🤖 Fix all issues with AI agents
In `@mcpjam-inspector/client/src/components/ServersTab.tsx`:
- Around line 160-171: The downloadJson function revokes the object URL
immediately after a.click(), which can abort downloads in some browsers; change
it to defer revocation by moving URL.revokeObjectURL(url) into a short callback
(e.g., setTimeout(() => URL.revokeObjectURL(url), 0) or attach to the anchor's
onload/onfocus event) so the file download completes reliably while still
cleaning up the created blob URL; update the logic in downloadJson accordingly.
🧹 Nitpick comments (1)
mcpjam-inspector/client/src/components/ServersTab.tsx (1)

174-186: Consider warning when exports include env values.

formatJsonConfig includes env when present, so exports may carry secrets. A brief UI warning or an “omit env” option would reduce accidental leakage.

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4e6d1c1 and ddcf20e.

📒 Files selected for processing (1)
  • mcpjam-inspector/client/src/components/ServersTab.tsx
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2026-01-10T18:55:38.803Z
Learnt from: CR
Repo: MCPJam/inspector PR: 0
File: mcpjam-inspector/AGENTS.md:0-0
Timestamp: 2026-01-10T18:55:38.803Z
Learning: Applies to mcpjam-inspector/server/**/*.{ts,tsx} : Use the centralized logger utility from `@/utils/logger` for server code with methods like `logger.error()`, `logger.warn()`, `logger.info()`, and `logger.debug()`

Applied to files:

  • mcpjam-inspector/client/src/components/ServersTab.tsx
📚 Learning: 2026-01-10T18:55:38.803Z
Learnt from: CR
Repo: MCPJam/inspector PR: 0
File: mcpjam-inspector/AGENTS.md:0-0
Timestamp: 2026-01-10T18:55:38.803Z
Learning: Applies to mcpjam-inspector/server/**/*.{ts,tsx,js} : Do not use `console.log`, `console.warn`, or `console.error` directly in server code

Applied to files:

  • mcpjam-inspector/client/src/components/ServersTab.tsx
📚 Learning: 2026-01-10T18:55:38.803Z
Learnt from: CR
Repo: MCPJam/inspector PR: 0
File: mcpjam-inspector/AGENTS.md:0-0
Timestamp: 2026-01-10T18:55:38.803Z
Learning: Applies to mcpjam-inspector/src/**/*.{ts,tsx,js,jsx} : Browser `console.*` is acceptable for client-side debugging

Applied to files:

  • mcpjam-inspector/client/src/components/ServersTab.tsx
🧬 Code graph analysis (1)
mcpjam-inspector/client/src/components/ServersTab.tsx (1)
mcpjam-inspector/client/src/lib/json-config-parser.ts (1)
  • formatJsonConfig (21-53)

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.

Comment on lines +160 to +171
const downloadJson = (filename: string, data: any) => {
const blob = new Blob([JSON.stringify(data, null, 2)], {
type: "application/json;charset=utf-8",
});
const url = URL.createObjectURL(blob);
const a = document.createElement("a");
a.href = url;
a.download = filename;
document.body.appendChild(a);
a.click();
document.body.removeChild(a);
URL.revokeObjectURL(url);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Defer URL revocation to avoid flaky downloads.

Revoking immediately after a.click() can cancel the download in some browsers. A short defer makes the flow more reliable.

🧩 Proposed fix
     a.click();
     document.body.removeChild(a);
-    URL.revokeObjectURL(url);
+    // Allow the browser to start the download before revoking
+    setTimeout(() => URL.revokeObjectURL(url), 0);
🤖 Prompt for AI Agents
In `@mcpjam-inspector/client/src/components/ServersTab.tsx` around lines 160 -
171, The downloadJson function revokes the object URL immediately after
a.click(), which can abort downloads in some browsers; change it to defer
revocation by moving URL.revokeObjectURL(url) into a short callback (e.g.,
setTimeout(() => URL.revokeObjectURL(url), 0) or attach to the anchor's
onload/onfocus event) so the file download completes reliably while still
cleaning up the created blob URL; update the logic in downloadJson accordingly.

Copy link
Collaborator

@matteo8p matteo8p left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ship

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Jan 16, 2026
@matteo8p matteo8p merged commit 2773896 into MCPJam:main Jan 16, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request lgtm This PR has been approved by a maintainer size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants