Logging updates (including reported SonicBoom error)

[BobDickinson]

Summary

Made logging consistent between all clients (using MCP_LOG_FILE), updated client readmes. Made logging initialization async so we have logging ready before app startup processing or early exit (this addresses the SonicBoom errors seen on early app termination). Reorganized shared logging code (which is node code) into core/logging/node. Fixed CLI error reporting through launcher (just error message now, no stack trace). Made logger required throughout using silentLogger (null logger) as default.

Testing

• Tested in UI mode
• Tested in CLI mode
• Manual testing performed

Checklist

• Code follows the style guidelines (ran npm run prettier-fix)
• Self-review completed
• Code is commented where necessary
• Documentation updated (README, comments, etc.)

[BobDickinson]

@claude review

[claude]

Claude finished @BobDickinson's task in 1m 35s —— View job

---

Code Review

• Gather context and understand the PR
• Read changed files
• Analyze code for issues
• Post review feedback

---

Overall this is a well-structured improvement. The core fix (awaiting the SonicBoom ready event before using the logger) is correct and addresses the reported issue cleanly. The refactoring to centralize file logger creation and make loggers required (non-optional) is a good step forward. A few things worth flagging:

Issues

1. Browser components importing from logging/node path

AuthDebugger.tsx and OAuthCallback.tsx are React components that run in the browser. They now import silentLogger from @modelcontextprotocol/inspector-core/logging/node:

// clients/web/src/components/AuthDebugger.tsx
import { silentLogger } from "@modelcontextprotocol/inspector-core/logging/node";

The logging/node barrel re-exports both silentLogger (browser-safe) and createFileLogger (Node.js-specific — uses pino.destination() → SonicBoom → fs). Vite's tree-shaking should prevent fileLogger.ts from being bundled, but semantically importing Node.js-specific paths in browser code is fragile: any future addition to logging/node/index.ts that has side effects or isn't tree-shakeable could silently break the browser bundle.

The old core/logging/index.ts provided a clean browser-safe re-export point for silentLogger. Deleting it and pointing browser components at /node loses that separation. A better approach would be restoring a /logging or /logging/browser export that only contains browser-safe utilities, and using that from the React components.

Fix this →

2. Missing logger name in web.ts

cli.ts passes name: "mcp-inspector-cli" to createFileLogger, but web.ts (inside runWeb()) does not:

// clients/web/src/web.ts:260
const logger = process.env.MCP_LOG_FILE
  ? await createFileLogger({
      dest: process.env.MCP_LOG_FILE,
      append: true,
      mkdir: true,
      level: "info",
      // missing: name: "mcp-inspector-web"
    })
  : silentLogger;

Log lines from the web runner won't have a name field, making it harder to distinguish sources when multiple clients log to the same file.

Fix this →

Minor Observations

honoMiddlewarePlugin accepting Promise<WebServerConfig> — The WebServerConfig | Promise<WebServerConfig> union type is a pragmatic workaround for the async config issue, and it works. An alternative would be an async factory function createHonoPlugin(config: Promise<WebServerConfig>): Promise<Plugin>, but the current approach avoids changing callers so it's reasonable.

tuiLogger mutable export — export let tuiLogger: Logger = silentLogger with reassignment inside initTuiLogger() relies on ES module live bindings for correctness (consumers re-reading tuiLogger after init). This works, but any module that destructures the import early (e.g. const { tuiLogger } = await import(...)) would miss the update. Worth a note in the file that it must be referenced by name, not cached.

createFileLogger error listener cleanup — Both once("ready", resolve) and once("error", reject) are properly using once so they auto-clean up after firing. No leak.

What Works Well

• The ready-event await in createFileLogger is the correct fix for the SonicBoom flush race on early exit.
• Making logger: Logger required (non-optional) with silentLogger as the default is cleaner than checking logger !== undefined everywhere.
• Unifying all three clients to use MCP_LOG_FILE is a good consistency improvement.
• The async logger initialization pattern (init before any work that could process.exit()) is exactly right.

---

[BobDickinson]

Claude review feedback implemented.