feat(client): add JSON-RPC log export functionality#1462
feat(client): add JSON-RPC log export functionality#1462Jony-Jas wants to merge 5 commits intoMCPJam:mainfrom
Conversation
- Adds a download button to the logger toolbar allowing users to export current logs as a JSON file. Resolves MCPJam#1140
✅ 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. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds an export control and header actions to the logger view. Introduces a `LogEntrySnapshot` type and an `extractLogs` helper to transform filtered items into a simplified shape. Refactors `copyLogs` to use `extractLogs`. Adds `exportLogs` which serializes logs to JSON, creates a Blob and object URL, programmatically triggers download as `mcp-logs-.json`, revokes the URL, and shows a success toast. UI now exposes Export, Copy, Clear, and optional Close controls. No public API signatures changed. Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@mcpjam-inspector/client/src/components/logger-view.tsx`:
- Around line 194-209: exportLogs does no error handling and can fail
(JSON.stringify, DOM ops, createObjectURL); wrap the body of exportLogs in a
try/catch and show toast.error with the caught error (similar to copyLogs) so
users are informed, and move cleanup (document.body.removeChild(a) and
URL.revokeObjectURL(url)) into a finally block to ensure no DOM or object URL
leaks even when an error is thrown; reference the extractLogs call, the
Blob/URL.createObjectURL usage, the created anchor element (a), and
toast.error/toast.success to locate where to add try/catch/finally.
🧹 Nitpick comments (1)
mcpjam-inspector/client/src/components/logger-view.tsx (1)
71-71:LogEntrySnapshotpromises fields thatextractLogsnever provides.
Omit<RenderableRpcItem, 'id'>retainsprotocolandwidgetId, yet the mapping inextractLogssilently drops them. TypeScript won't protest (they're optional), but the type is a quiet lie. Either include those fields in the mapping or define a precise type that reflects reality.Proposed fix — tighten the type
-type LogEntrySnapshot = Omit<RenderableRpcItem, 'id'>; +type LogEntrySnapshot = Pick<RenderableRpcItem, 'timestamp' | 'source' | 'serverId' | 'direction' | 'method' | 'payload'>;Alternatively, include the optional fields in the mapping:
const extractLogs = (): Array<LogEntrySnapshot> => { return filteredItems.map((item) => ({ timestamp: item.timestamp, source: item.source, serverId: item.serverId, direction: item.direction, method: item.method, payload: item.payload, + protocol: item.protocol, + widgetId: item.widgetId, })); };Also applies to: 183-192
Signed-off-by: Jony Jas J <74784363+Jony-Jas@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@mcpjam-inspector/client/src/components/logger-view.tsx`:
- Around line 300-346: The header block in the LoggerView component now renders
Export/Copy/Clear/Close buttons (using exportLogs, copyLogs, clearMessages and
onClose/isCollapsable) but the older toolbar JSX later in the same component
also renders Copy, Clear and Close again, causing duplicate actions; remove the
redundant button set (the older toolbar rendering that references copyLogs,
clearMessages and onClose) or gate it behind a single source-of-truth flag so
only the new header block shows these actions, leaving exportLogs in the header
as the canonical export action.
- Line 232: The file download filename uses toISOString() which includes colons
illegal on Windows; update the assignment where a.download is set (in
logger-view.tsx) to sanitize the timestamp by replacing ":" with "-" (e.g., call
.toISOString().replace(/:/g, "-")) so the generated download name like in the
a.download = `mcp-logs-${...}.json` line produces a Windows-safe filename.
🧹 Nitpick comments (1)
mcpjam-inspector/client/src/components/logger-view.tsx (1)
69-69: Type promises more thanextractLogsdelivers.
Omit<RenderableRpcItem, 'id'>retainsprotocolandwidgetIdas optional fields, yetextractLogsnever includes them. Consider aPickof the six fields actually emitted, so the export contract is self-documenting.Suggested type
-type LogEntrySnapshot = Omit<RenderableRpcItem, 'id'>; +type LogEntrySnapshot = Pick< + RenderableRpcItem, + 'timestamp' | 'source' | 'serverId' | 'direction' | 'method' | 'payload' +>;
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@mcpjam-inspector/client/src/components/logger-view.tsx`:
- Around line 212-221: The extractLogs function is dropping optional MCP Apps
fields `protocol` and `widgetId` when mapping filteredItems into
LogEntrySnapshot; update the mapping inside extractLogs to copy `protocol` and
`widgetId` from each item (same place where timestamp, source, serverId,
direction, method, payload are copied) so the resulting LogEntrySnapshot (which
is based on Omit<RenderableRpcItem, 'id'>) preserves those optional fields for
MCP Apps traffic; locate extractLogs and ensure it includes item.protocol and
item.widgetId in the returned object.
- Added error handling - removed duplicate button caused due to merge conflict - added protol and widgets to extractLogs as optional
|
Hey @Jony-Jas! Thanks for putting this up. Looks good to me but we did do a logger redesign recently, so could you update this branch to main and see if the download icon still looks good? |
|
@Jony-Jas @khandrew1 Any updates here? Is this PR good to go? |
I need to take a look!! looks like i have updated with Older UI. |
|
@matteo8p Now looks good to go!!
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
mcpjam-inspector/client/src/components/logger-view.tsx (1)
211-222:extractLogsforward-referencesfilteredItems, declared 60 lines later.The function only executes as an event handler (post-render), so the TDZ is never triggered in practice. But the definition sits well above its sole dependency, which creates a reading-order surprise and becomes a trap if
extractLogsis ever called during render (e.g., inside auseMemo). Moving the function belowfilteredItems, or better, accepting it as a parameter, removes the coupling entirely.♻️ Suggested refactor — parameterise the dependency
- const extractLogs = (): Array<LogEntrySnapshot> => { - return filteredItems.map((item) => ({ + const extractLogs = (items: RenderableRpcItem[]): Array<LogEntrySnapshot> => { + return items.map((item) => ({ timestamp: item.timestamp, source: item.source, serverId: item.serverId, direction: item.direction, method: item.method, payload: item.payload, ...(item.protocol && { protocol: item.protocol }), ...(item.widgetId && { widgetId: item.widgetId }), })); };Then update call sites:
- const logs = extractLogs(); + const logs = extractLogs(filteredItems);Additionally, the
&&falsy guards on lines 219–220 will silently omitprotocol/widgetIdif they are ever an empty string.!== undefinedis more precise:♻️ Safer guard
- ...(item.protocol && { protocol: item.protocol }), - ...(item.widgetId && { widgetId: item.widgetId }), + ...(item.protocol !== undefined && { protocol: item.protocol }), + ...(item.widgetId !== undefined && { widgetId: item.widgetId }),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mcpjam-inspector/client/src/components/logger-view.tsx` around lines 211 - 222, extractLogs currently forward-references filteredItems (declared later) and uses falsy guards that drop empty-string values; fix by either moving the extractLogs declaration to after filteredItems or refactor extractLogs to accept filteredItems as a parameter (e.g., extractLogs(items): Array<LogEntrySnapshot>) and update all call sites (so the function no longer relies on outer scope), and change the conditional properties from ...(item.protocol && { protocol: item.protocol }) / ...(item.widgetId && { widgetId: item.widgetId }) to use explicit checks for undefined (e.g., ...(item.protocol !== undefined && { protocol: item.protocol })) to preserve empty strings.
🤖 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/components/logger-view.tsx`:
- Around line 447-456: The Export Button (the Button rendering the Download icon
and calling exportLogs) is missing the flex-shrink-0 utility class causing
inconsistent shrinking behavior with sibling buttons; update that Button's
className (the Button with onClick={exportLogs} and title="Export logs") to
include "flex-shrink-0" alongside "h-7 w-7" so it matches Copy/Clear/Close
buttons and will not compress in narrow toolbars.
- Line 586: Remove the unused default export by deleting the "export default
LoggerView;" statement so the component is only exported as the existing named
export (LoggerView); verify there are no references to a default import
elsewhere and keep the named export declaration (LoggerView) intact.
---
Nitpick comments:
In `@mcpjam-inspector/client/src/components/logger-view.tsx`:
- Around line 211-222: extractLogs currently forward-references filteredItems
(declared later) and uses falsy guards that drop empty-string values; fix by
either moving the extractLogs declaration to after filteredItems or refactor
extractLogs to accept filteredItems as a parameter (e.g., extractLogs(items):
Array<LogEntrySnapshot>) and update all call sites (so the function no longer
relies on outer scope), and change the conditional properties from
...(item.protocol && { protocol: item.protocol }) / ...(item.widgetId && {
widgetId: item.widgetId }) to use explicit checks for undefined (e.g.,
...(item.protocol !== undefined && { protocol: item.protocol })) to preserve
empty strings.
| <Button | ||
| variant="ghost" | ||
| size="icon" | ||
| onClick={exportLogs} | ||
| disabled={filteredItemCount === 0} | ||
| className="h-7 w-7" | ||
| title="Export logs" | ||
| > | ||
| <Download className="h-3.5 w-3.5" /> | ||
| </Button> |
There was a problem hiding this comment.
Export button is missing flex-shrink-0 — inconsistent with its siblings.
All adjacent buttons (Copy, Clear, Close) carry flex-shrink-0; the Export button does not, so it could compress under a narrow toolbar while they stay fixed.
🛠️ One-liner fix
<Button
variant="ghost"
size="icon"
onClick={exportLogs}
disabled={filteredItemCount === 0}
- className="h-7 w-7"
+ className="h-7 w-7 flex-shrink-0"
title="Export logs"
>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mcpjam-inspector/client/src/components/logger-view.tsx` around lines 447 -
456, The Export Button (the Button rendering the Download icon and calling
exportLogs) is missing the flex-shrink-0 utility class causing inconsistent
shrinking behavior with sibling buttons; update that Button's className (the
Button with onClick={exportLogs} and title="Export logs") to include
"flex-shrink-0" alongside "h-7 w-7" so it matches Copy/Clear/Close buttons and
will not compress in narrow toolbars.
| } | ||
|
|
||
| export default LoggerView; | ||
| export default LoggerView; No newline at end of file |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for all imports of LoggerView to determine usage pattern
rg "import\s+(\{.*\s+)?LoggerView" --type ts --type tsx -B 1 -A 1Repository: MCPJam/inspector
Length of output: 87
🏁 Script executed:
# Also search for default imports without destructuring
rg "import\s+LoggerView\s+from" --type ts --type tsx -B 1 -A 1Repository: MCPJam/inspector
Length of output: 87
🏁 Script executed:
# Search for LoggerView imports using glob patterns
rg "import.*LoggerView" -g "*.ts" -g "*.tsx" -B 1 -A 1Repository: MCPJam/inspector
Length of output: 2116
🏁 Script executed:
# Specifically search for default imports (import LoggerView from)
rg "import\s+LoggerView\s+from" -g "*.ts" -g "*.tsx" -B 1 -A 1Repository: MCPJam/inspector
Length of output: 42
🏁 Script executed:
# List the logger-view file to understand its exports
cat -n mcpjam-inspector/client/src/components/logger-view.tsx | grep -E "export|LoggerView" | head -20Repository: MCPJam/inspector
Length of output: 368
Remove the unused default export.
LoggerView is exported as a named export (line 128), and all seven import sites across the codebase use the named import pattern (import { LoggerView } from "..."). The default export at line 586 is never used and should be removed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mcpjam-inspector/client/src/components/logger-view.tsx` at line 586, Remove
the unused default export by deleting the "export default LoggerView;" statement
so the component is only exported as the existing named export (LoggerView);
verify there are no references to a default import elsewhere and keep the named
export declaration (LoggerView) intact.

Resolves #1140