feat(mcp-doctor): health-check & benchmark your MCP servers#201
feat(mcp-doctor): health-check & benchmark your MCP servers#201genesiscz wants to merge 3 commits into
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughIntroduces a new Changesmcp-doctor CLI tool
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces mcp-doctor, a CLI tool to health-check and benchmark configured Model Context Protocol (MCP) servers. It discovers configurations from multiple sources, probes servers over stdio, HTTP, or SSE, detects duplicate tools, and formats the results into a report. The review feedback highlights several robustness improvements: moving transport initialization inside a try-catch block to prevent CLI crashes, correcting timeout classification, validating URLs during discovery, defensively checking the configuration object structure, including invalid servers in the summary error count, and validating CLI timeout and slow threshold inputs.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| const client = new Client(CLIENT_INFO); | ||
| const transport = buildTransport(server); | ||
| const startedAt = Date.now(); | ||
| let finishedAt: number | null = null; | ||
| let error: string | null = null; | ||
| let tools: string[] = []; | ||
| let resourceCount = 0; | ||
| let promptCount = 0; | ||
| let serverInfo: { name: string; version: string } | null = null; | ||
|
|
||
| try { | ||
| await withTimeout(client.connect(transport), opts.timeoutMs); |
There was a problem hiding this comment.
The buildTransport(server) function is called outside of the try/catch block. If transport initialization fails (for example, if new URL(server.url) throws a TypeError due to an invalid URL configuration), the error will go uncaught. This will cause the entire Promise.all in probeAll to reject, crashing the CLI tool.
Moving buildTransport inside the try block ensures that any initialization errors are gracefully caught, logged, and reported as a probe failure for that specific server.
| const client = new Client(CLIENT_INFO); | |
| const transport = buildTransport(server); | |
| const startedAt = Date.now(); | |
| let finishedAt: number | null = null; | |
| let error: string | null = null; | |
| let tools: string[] = []; | |
| let resourceCount = 0; | |
| let promptCount = 0; | |
| let serverInfo: { name: string; version: string } | null = null; | |
| try { | |
| await withTimeout(client.connect(transport), opts.timeoutMs); | |
| const client = new Client(CLIENT_INFO); | |
| const startedAt = Date.now(); | |
| let finishedAt: number | null = null; | |
| let error: string | null = null; | |
| let tools: string[] = []; | |
| let resourceCount = 0; | |
| let promptCount = 0; | |
| let serverInfo: { name: string; version: string } | null = null; | |
| try { | |
| const transport = buildTransport(server); | |
| await withTimeout(client.connect(transport), opts.timeoutMs); |
There was a problem hiding this comment.
/gemini Fixed in 82b3ca5 (82b3ca5) - moved the buildTransport call inside the try block so a malformed remote URL throwing from new URL() now fails that single server instead of rejecting the Promise.all and crashing the whole batch. Also validating the URL at discovery time (see the discovery.ts thread) so bad URLs surface offline in list too.
There was a problem hiding this comment.
Thank you for the update and for confirming the fix in 82b3ca5. Validating the URL at discovery time is a great approach to ensure that configuration issues are caught early and reported clearly, rather than failing during the probe phase. This change significantly improves the robustness of the mcp-doctor tool.
| if (input.error) { | ||
| const latency = input.finishedAt === null ? null : input.finishedAt - input.startedAt; | ||
| return { status: "error", latencyMs: latency }; | ||
| } |
There was a problem hiding this comment.
The "timeout" status is currently unreachable for connection timeouts. When withTimeout times out, it throws an error which is caught in probeServer and assigned to error. Because input.error is non-null, classifyResult always returns "error" status instead of "timeout".
Checking if the error message indicates a timeout when finishedAt is null allows us to correctly classify and report actual timeouts.
| if (input.error) { | |
| const latency = input.finishedAt === null ? null : input.finishedAt - input.startedAt; | |
| return { status: "error", latencyMs: latency }; | |
| } | |
| if (input.error) { | |
| const isTimeout = input.error.toLowerCase().includes("timeout") || input.error.toLowerCase().includes("timed out"); | |
| if (isTimeout && input.finishedAt === null) { | |
| return { status: "timeout", latencyMs: null }; | |
| } | |
| const latency = input.finishedAt === null ? null : input.finishedAt - input.startedAt; | |
| return { status: "error", latencyMs: latency }; | |
| } |
There was a problem hiding this comment.
/gemini Fixed in 82b3ca5 (82b3ca5) - the timeout branch was dead because withTimeout rejects with an Error, probe sets the error field, and classifyResult checks error first. Now I lowercase the message and check for timeout or timed out, gated on finishedAt being null, so genuine handshake timeouts classify as timeout instead of generic error.
There was a problem hiding this comment.
The fix in commit 82b3ca5 correctly addresses the issue where connection timeouts were being masked as generic errors. By checking for timeout-related keywords in the error message when finishedAt is null, the classifyResult function now correctly categorizes these as timeout status, which is the expected behavior for handshake failures. This is a robust approach given the current error handling flow.
| if (typeof def.url === "string" && def.url.length > 0) { | ||
| const transport = def.type === "sse" ? "sse" : "http"; | ||
| return { name, transport, source, url: def.url }; | ||
| } |
There was a problem hiding this comment.
The normalizeOne function does not validate that def.url is a valid URL. If an invalid URL is configured, it is still treated as a valid remote server, which then fails during the probe phase.
Validating the URL during discovery allows us to catch malformed URLs early, marking them as invalid so they are correctly flagged in the offline tools mcp-doctor list command.
| if (typeof def.url === "string" && def.url.length > 0) { | |
| const transport = def.type === "sse" ? "sse" : "http"; | |
| return { name, transport, source, url: def.url }; | |
| } | |
| if (typeof def.url === "string" && def.url.length > 0) { | |
| try { | |
| new URL(def.url); | |
| const transport = def.type === "sse" ? "sse" : "http"; | |
| return { name, transport, source, url: def.url }; | |
| } catch { | |
| return { | |
| name, | |
| transport: "stdio", | |
| source, | |
| invalidReason: `invalid URL: "${def.url}"`, | |
| }; | |
| } | |
| } |
There was a problem hiding this comment.
/gemini Fixed in 82b3ca5 (82b3ca5) - normalizeOne now wraps the url in new URL() and returns an InvalidServer with an invalidReason when it throws, so a malformed URL is flagged invalid at discovery instead of only failing at probe time. The InvalidServer type, isInvalidServer guard, and the invalid rendering in formatConfigTable already exist, so it shows up correctly in offline list output. Root-cause companion to the probe try/catch fix.
There was a problem hiding this comment.
Thank you for the update, genesiscz. That approach sounds correct and robust. By handling the URL validation during the discovery phase and marking the server as invalid with a clear reason, you've successfully improved the user experience by providing immediate feedback rather than waiting for a probe failure. This aligns perfectly with the goal of making mcp-doctor list a fast, reliable offline tool.
| const map = config?.mcpServers; | ||
| if (!map) { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
If mcpServers in the JSON configuration is not a plain object (e.g., if it is an array, string, or other unexpected type), Object.entries(map) will behave unexpectedly or throw an error.
Adding a defensive check to ensure map is a non-array object prevents potential crashes when parsing malformed configuration files.
| const map = config?.mcpServers; | |
| if (!map) { | |
| continue; | |
| } | |
| const map = config?.mcpServers; | |
| if (!map || typeof map !== "object" || Array.isArray(map)) { | |
| continue; | |
| } |
There was a problem hiding this comment.
/gemini Fixed in 82b3ca5 (82b3ca5) - mcpServers comes from an unchecked cast over untrusted file contents, so it could be an array/string/number at runtime where the old truthy-only check would feed garbage index pairs into Object.entries. Added a guard that also requires it to be a non-array object before iterating.
There was a problem hiding this comment.
Thank you for the update and for applying the fix in 82b3ca5. The added guard ensures that mcpServers is a non-array object, which correctly prevents runtime errors when parsing potentially malformed configuration files. This looks good.
| const count = (s: Status): number => results.filter((r) => r.status === s).length; | ||
| const summary = { | ||
| total: results.length, | ||
| ok: count("ok"), | ||
| slow: count("slow"), | ||
| timeout: count("timeout"), | ||
| error: count("error"), | ||
| duplicateTools: duplicates.length, | ||
| }; |
There was a problem hiding this comment.
Invalid servers are assigned status: "invalid", but they are currently not counted in any of the summary status fields (ok, slow, timeout, error), even though they are included in total. This leads to a confusing summary line where the numbers do not add up (e.g., 1 servers (0 ok · 0 slow · 0 error)).
Counting invalid servers under error in the summary resolves this mismatch cleanly.
| const count = (s: Status): number => results.filter((r) => r.status === s).length; | |
| const summary = { | |
| total: results.length, | |
| ok: count("ok"), | |
| slow: count("slow"), | |
| timeout: count("timeout"), | |
| error: count("error"), | |
| duplicateTools: duplicates.length, | |
| }; | |
| const count = (s: Status): number => results.filter((r) => r.status === s).length; | |
| const summary = { | |
| total: results.length, | |
| ok: count("ok"), | |
| slow: count("slow"), | |
| timeout: count("timeout"), | |
| error: count("error") + count("invalid"), | |
| duplicateTools: duplicates.length, | |
| }; |
There was a problem hiding this comment.
/gemini Fixed in 82b3ca5 (82b3ca5) - invalid servers counted toward total but no status sub-count, so the header could read 1 servers (0 ok, 0 slow, 0 error). Folded invalid into the error count as suggested, and also added the existing-but-unshown timeout count to the header line so timed-out-only runs reconcile too.
There was a problem hiding this comment.
Thank you for the update and for incorporating the suggested fix for the invalid server status count. Adding the timeout count to the header is also a great improvement for reconciliation. I have reviewed the changes in 82b3ca5 and they look correct and address the issues identified.
| async function probeAll(servers: NormalizedServer[], opts: SharedOpts): Promise<ProbeResult[]> { | ||
| const timeoutMs = Number(opts.timeout); | ||
| const slowThresholdMs = Number(opts.slow); | ||
| return Promise.all(servers.map((server) => probeServer(server, { timeoutMs, slowThresholdMs }))); | ||
| } |
There was a problem hiding this comment.
The opts.timeout and opts.slow options are parsed as numbers without validation. To prevent runtime issues like NaN propagation, these CLI arguments should be validated and normalized at the entry point (e.g., in the commander action handler) before being passed to business logic functions like probeAll.
async function probeAll(servers: NormalizedServer[], opts: SharedOpts): Promise<ProbeResult[]> {
const timeoutMs = Number(opts.timeout);
const slowThresholdMs = Number(opts.slow);
if (Number.isNaN(timeoutMs) || timeoutMs <= 0) {
throw new Error(`Invalid --timeout value: "${opts.timeout}". Must be a positive number.`);
}
if (Number.isNaN(slowThresholdMs) || slowThresholdMs <= 0) {
throw new Error(`Invalid --slow value: "${opts.slow}". Must be a positive number.`);
}
return Promise.all(servers.map((server) => probeServer(server, { timeoutMs, slowThresholdMs })));
}References
- Validate and normalize CLI arguments at the entry point (e.g., in a commander action handler) before they are passed to business logic.
There was a problem hiding this comment.
/gemini Fixed in 82b3ca5 (82b3ca5) - Number(opts.timeout) and Number(opts.slow) were unvalidated, so --timeout abc yielded NaN which made setTimeout fire immediately and the slow comparison always false. Added validation in probeAll using Number.isFinite plus a positive check, throwing a clear error (Number.isFinite also rejects Infinity).
There was a problem hiding this comment.
Thank you for the update, genesiscz. Using Number.isFinite and adding a positive check is a robust way to handle these inputs and prevents the NaN propagation issues I noted. This looks like a solid fix.
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/mcp-doctor/index.ts`:
- Around line 85-88: Import isInteractive from `@app/utils/cli` at the top of the
file, then wrap the spinner initialization and usage (the spinner.start() and
spinner.stop() calls) in a conditional check for isInteractive(). The probeAll()
function call should still execute regardless of interactivity, but the spinner
visual feedback should only be shown in interactive environments to prevent
unwanted output in CI or piped environments.
- Around line 48-49: The Number() conversions of opts.timeout and opts.slow into
timeoutMs and slowThresholdMs variables do not validate that the conversions
succeeded. Add validation after each Number() call to check that the result is
not NaN, and handle invalid numeric input by either throwing an error with a
clear message indicating the option must be numeric, or falling back to a
sensible default value. This ensures only valid numbers are passed to
probeServer.
In `@src/mcp-doctor/lib/probe.ts`:
- Around line 22-28: The type assertion `as Record<string, string>` on the
merged environment object in the StdioClientTransport creation is unsafe because
process.env values can be string or undefined. Remove the type assertion and
either let TypeScript infer the proper type, or defensively filter out undefined
values from the merged environment object (using Object.entries() with a filter
condition) before passing it to ensure all values are strings and the type
matches what StdioClientTransport expects.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3d096198-d37f-4e9e-942a-5f0b44d06d07
📒 Files selected for processing (7)
src/mcp-doctor/index.tssrc/mcp-doctor/lib/discovery.tssrc/mcp-doctor/lib/duplicates.tssrc/mcp-doctor/lib/probe.tssrc/mcp-doctor/lib/report.tssrc/mcp-doctor/lib/types.tssrc/mcp-doctor/mcp-doctor.test.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (6)
src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.ts: Always import and useSafeJSONfrom@app/utils/jsoninstead of the globalJSONobject; useSafeJSON.parse()andSafeJSON.stringify()everywhere
Uselogger(from@app/logger) for diagnostics; write to day-stamped file always and to stderr only when log level permits; useout.result()as the only writer to stdout
Log enough context to triage issues from logs alone without re-running the tool; log key decision branches, external-resource access, mode/config resolution, and result counts
Prefererror: errovererror: err instanceof Error ? err.message : String(err)when the error field accepts unknown type
Files:
src/mcp-doctor/lib/duplicates.tssrc/mcp-doctor/lib/discovery.tssrc/mcp-doctor/lib/probe.tssrc/mcp-doctor/lib/types.tssrc/mcp-doctor/mcp-doctor.test.tssrc/mcp-doctor/lib/report.tssrc/mcp-doctor/index.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.{ts,tsx}: Do not useas anytype assertions; use proper type narrowing, type guards, or explicit interfaces instead
When working with union types, use discriminant checks (e.g.entity.className === "User") instead of type assertions
Never use barecatch {}blocks; at minimum log the caught error with context usinglogger.debug()orlogger.warn()
For functions with 3+ parameters or optional parameters, use an object parameter instead of positional parameters
Do not use one-lineifstatements, even for early returns; always use block form with braces
Add an empty line beforeifstatements unless the preceding line is a variable declaration used by thatif
Add an empty line after closing}unless followed byelse,catch,finally, or another}
Always checkisInteractive()from@app/utils/clibefore showing prompts; provide sensible defaults or error withsuggestCommand()in non-interactive mode
Files:
src/mcp-doctor/lib/duplicates.tssrc/mcp-doctor/lib/discovery.tssrc/mcp-doctor/lib/probe.tssrc/mcp-doctor/lib/types.tssrc/mcp-doctor/mcp-doctor.test.tssrc/mcp-doctor/lib/report.tssrc/mcp-doctor/index.ts
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Never add a file-path comment as the first line of files (e.g.,// src/path/to/file.ts)
Do not add comments that restate what the code already says; avoid obvious comments like// Build initial contextbeforebuildContext()
Files:
src/mcp-doctor/lib/duplicates.tssrc/mcp-doctor/lib/discovery.tssrc/mcp-doctor/lib/probe.tssrc/mcp-doctor/lib/types.tssrc/mcp-doctor/mcp-doctor.test.tssrc/mcp-doctor/lib/report.tssrc/mcp-doctor/index.ts
src/**/*.test.ts
📄 CodeRabbit inference engine (CLAUDE.md)
When adding database logic, use an in-memory
new Database(":memory:")for testing; group test files alongside source files
Files:
src/mcp-doctor/mcp-doctor.test.ts
src/**/index.ts
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/index.ts: Tool entry points must use a TypeScript file with shebang that shows an interactive tool selector when run without arguments, and executes the specified tool viabun runwhen given arguments
Tool entry points must end withawait runTool(program, { tool })from@app/utils/cli, which owns-v/--readme/help registration and console-level resolution
Files:
src/mcp-doctor/index.ts
src/*/index.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Tool directories must contain either
index.ts/index.tsxor standalone.ts/.tsxfiles; tool name is derived from directory name or filename without extension
Files:
src/mcp-doctor/index.ts
🧠 Learnings (22)
📚 Learning: 2026-02-24T15:32:37.494Z
Learnt from: genesiscz
Repo: genesiscz/GenesisTools PR: 54
File: src/github/lib/output.ts:109-113
Timestamp: 2026-02-24T15:32:37.494Z
Learning: In TypeScript files under src/, do not require a leading blank line before an if statement that is the first statement inside a function body (immediately after the function signature). The blank line rule should only apply to if statements that come after other statements within the function body. Apply this guideline consistently across TS files in src to reduce unnecessary vertical whitespace and keep concise function bodies.
Applied to files:
src/mcp-doctor/lib/duplicates.tssrc/mcp-doctor/lib/discovery.tssrc/mcp-doctor/lib/probe.tssrc/mcp-doctor/lib/types.tssrc/mcp-doctor/mcp-doctor.test.tssrc/mcp-doctor/lib/report.tssrc/mcp-doctor/index.ts
📚 Learning: 2026-03-09T13:13:58.786Z
Learnt from: genesiscz
Repo: genesiscz/GenesisTools PR: 81
File: src/github/commands/get.ts:209-212
Timestamp: 2026-03-09T13:13:58.786Z
Learning: In the GenesisTools repo (genesiscz/GenesisTools), do not treat CI formatter warnings as enforceable formatting rules for TypeScript files under src/. Focus reviews on logical correctness and consistency with existing code patterns. For files under src (e.g., src/github/commands/get.ts), prioritize code structure, readability, naming, correctness, and adherence to project conventions over automated formatting warnings from CI tools.
Applied to files:
src/mcp-doctor/lib/duplicates.tssrc/mcp-doctor/lib/discovery.tssrc/mcp-doctor/lib/probe.tssrc/mcp-doctor/lib/types.tssrc/mcp-doctor/mcp-doctor.test.tssrc/mcp-doctor/lib/report.tssrc/mcp-doctor/index.ts
📚 Learning: 2026-03-12T01:26:31.610Z
Learnt from: genesiscz
Repo: genesiscz/GenesisTools PR: 95
File: src/timely/utils/entry-processor.ts:0-0
Timestamp: 2026-03-12T01:26:31.610Z
Learning: In code paths where JSON is consumed, prefer strict RFC 8259 validation by using SafeJSON.parse(text, { strict: true }) instead of the lenient default. Apply this at non-config boundaries (e.g., API responses, JSONL, cache outputs, subprocess outputs). Reserve the lenient comment-json behavior only for user-authored config files that may legitimately contain comments or trailing commas. For src/timely/utils/entry-processor.ts and similar modules, replace or wrap JSON parsing with SafeJSON.parse(text, { strict: true }) unless you are explicitly handling config files that require comments.
Applied to files:
src/mcp-doctor/lib/duplicates.tssrc/mcp-doctor/lib/discovery.tssrc/mcp-doctor/lib/probe.tssrc/mcp-doctor/lib/types.tssrc/mcp-doctor/mcp-doctor.test.tssrc/mcp-doctor/lib/report.tssrc/mcp-doctor/index.ts
📚 Learning: 2026-03-12T01:58:27.831Z
Learnt from: genesiscz
Repo: genesiscz/GenesisTools PR: 103
File: src/port/index.ts:137-144
Timestamp: 2026-03-12T01:58:27.831Z
Learning: In GenesisTools, apply a no-obvious-comments rule: do not add inline comments for well-known POSIX patterns or standard idioms (e.g., a process.kill(pid, 0) probe) when surrounding code is self-documenting through descriptive function/variable names. This guidance applies to TypeScript files under src (src/**/*.ts). Only include comments if they add non-obvious rationale, edge-case behavior, or explain complex logic that cannot be inferred from code alone.
Applied to files:
src/mcp-doctor/lib/duplicates.tssrc/mcp-doctor/lib/discovery.tssrc/mcp-doctor/lib/probe.tssrc/mcp-doctor/lib/types.tssrc/mcp-doctor/mcp-doctor.test.tssrc/mcp-doctor/lib/report.tssrc/mcp-doctor/index.ts
📚 Learning: 2026-03-22T22:19:44.520Z
Learnt from: genesiscz
Repo: genesiscz/GenesisTools PR: 119
File: src/indexer/commands/graph.ts:34-34
Timestamp: 2026-03-22T22:19:44.520Z
Learning: In genesiscz/GenesisTools, when using `SafeJSON.parse` in `src/**/*.ts`, it is acceptable to omit `{ strict: true }` if (and only if) the JSON being parsed is internal cache/state written by the same codebase (e.g., data saved by one internal writer and later read from a corresponding cached file). Do not require strict mode for these internal, machine-generated cache files. Require `{ strict: true }` at external/untrusted boundaries instead (e.g., API responses, third-party JSONL, subprocess output, or any JSON whose contents may not have been produced by trusted internal code).
Applied to files:
src/mcp-doctor/lib/duplicates.tssrc/mcp-doctor/lib/discovery.tssrc/mcp-doctor/lib/probe.tssrc/mcp-doctor/lib/types.tssrc/mcp-doctor/mcp-doctor.test.tssrc/mcp-doctor/lib/report.tssrc/mcp-doctor/index.ts
📚 Learning: 2026-03-25T19:55:27.917Z
Learnt from: genesiscz
Repo: genesiscz/GenesisTools PR: 129
File: src/utils/search/stores/vector-store.ts:19-23
Timestamp: 2026-03-25T19:55:27.917Z
Learning: When reviewing this codebase’s “3+ parameters → object parameter” guideline, only suggest object-parameter refactoring when the function’s parameters are ambiguous or include optional/unclear semantics. Do not flag tightly-defined utility/helper functions where (1) all parameters are required, (2) meanings are semantically clear from parameter names, and (3) the ordering is well-ordered and obvious. For example, functions like bruteForceVectorSearch(memoryIndex, queryVector, limit) should be allowed to keep positional parameters because the intent is clear.
Applied to files:
src/mcp-doctor/lib/duplicates.tssrc/mcp-doctor/lib/discovery.tssrc/mcp-doctor/lib/probe.tssrc/mcp-doctor/lib/types.tssrc/mcp-doctor/mcp-doctor.test.tssrc/mcp-doctor/lib/report.tssrc/mcp-doctor/index.ts
📚 Learning: 2026-05-05T03:52:21.057Z
Learnt from: genesiscz
Repo: genesiscz/GenesisTools PR: 163
File: src/debugging-master/core/dashboard-server.ts:115-127
Timestamp: 2026-05-05T03:52:21.057Z
Learning: When reviewing Bun.serve fetch handlers in this repo, don’t treat `req.signal` as possibly `undefined` at runtime. Bun guarantees an `AbortSignal` on every incoming Request, so `req.signal?.addEventListener(...)` is unnecessary for runtime safety and is only a TypeScript narrowing artifact (e.g., the type might be `AbortSignal | null`). Therefore, don’t raise concerns about SSE/subscription cleanup being skipped because `req.signal` could be missing; cleanup decisions should be based on the actual handler lifecycle, not an imagined runtime absence of `req.signal`.
Applied to files:
src/mcp-doctor/lib/duplicates.tssrc/mcp-doctor/lib/discovery.tssrc/mcp-doctor/lib/probe.tssrc/mcp-doctor/lib/types.tssrc/mcp-doctor/mcp-doctor.test.tssrc/mcp-doctor/lib/report.tssrc/mcp-doctor/index.ts
📚 Learning: 2026-06-11T22:07:33.681Z
Learnt from: genesiscz
Repo: genesiscz/GenesisTools PR: 216
File: src/macos/commands/mail/show.ts:8-8
Timestamp: 2026-06-11T22:07:33.681Z
Learning: In genesiscz/GenesisTools, treat `chalk` as the project-wide standard for colored terminal output in TypeScript files under `src/`. During code reviews, do not recommend replacing `chalk` with `picocolors` (or other alternatives) since CLAUDE.md and existing modules (e.g., `mcp-manager`) consistently use `chalk`. Only suggest converting `chalk` usages when the PR/user explicitly requests a full-project migration.
Applied to files:
src/mcp-doctor/lib/duplicates.tssrc/mcp-doctor/lib/discovery.tssrc/mcp-doctor/lib/probe.tssrc/mcp-doctor/lib/types.tssrc/mcp-doctor/mcp-doctor.test.tssrc/mcp-doctor/lib/report.tssrc/mcp-doctor/index.ts
📚 Learning: 2026-06-11T22:07:40.404Z
Learnt from: genesiscz
Repo: genesiscz/GenesisTools PR: 216
File: src/mcp-manager/commands/remove.ts:6-6
Timestamp: 2026-06-11T22:07:40.404Z
Learning: In genesiscz/GenesisTools, when reviewing TypeScript CLI/terminal output code under `src/`, use `chalk` for colored terminal/CLI output. Do not suggest switching to `picocolors` (or any other color library); the repo convention (including `mcp-manager` and `CLAUDE.md`) standardizes on `chalk`.
Applied to files:
src/mcp-doctor/lib/duplicates.tssrc/mcp-doctor/lib/discovery.tssrc/mcp-doctor/lib/probe.tssrc/mcp-doctor/lib/types.tssrc/mcp-doctor/mcp-doctor.test.tssrc/mcp-doctor/lib/report.tssrc/mcp-doctor/index.ts
📚 Learning: 2026-06-11T22:07:46.747Z
Learnt from: genesiscz
Repo: genesiscz/GenesisTools PR: 216
File: src/mcp-manager/utils/providers/claude.ts:7-7
Timestamp: 2026-06-11T22:07:46.747Z
Learning: In genesiscz/GenesisTools, `chalk` is the approved, project-wide standard for colored terminal/CLI output. For TypeScript files under `src/`, code reviews should not flag existing `chalk` usage and should not recommend migrating from `chalk` to `picocolors` (or otherwise proposing a replacement) as part of PRs, especially when the PR only touches a subset of files.
Applied to files:
src/mcp-doctor/lib/duplicates.tssrc/mcp-doctor/lib/discovery.tssrc/mcp-doctor/lib/probe.tssrc/mcp-doctor/lib/types.tssrc/mcp-doctor/mcp-doctor.test.tssrc/mcp-doctor/lib/report.tssrc/mcp-doctor/index.ts
📚 Learning: 2026-02-24T15:32:44.925Z
Learnt from: genesiscz
Repo: genesiscz/GenesisTools PR: 54
File: src/github/lib/review-output.ts:18-20
Timestamp: 2026-02-24T15:32:44.925Z
Learning: In TypeScript files, do not require a blank line between the opening brace of a function and the first statement if the first statement is the if statement immediately after the signature. The blank-line rule applies to separating an if from unrelated preceding code within the same block, not to spacing after the function opening brace. Apply this rule to all TS functions across the codebase.
Applied to files:
src/mcp-doctor/lib/duplicates.tssrc/mcp-doctor/lib/discovery.tssrc/mcp-doctor/lib/probe.tssrc/mcp-doctor/lib/types.tssrc/mcp-doctor/mcp-doctor.test.tssrc/mcp-doctor/lib/report.tssrc/mcp-doctor/index.ts
📚 Learning: 2026-03-12T01:26:03.611Z
Learnt from: genesiscz
Repo: genesiscz/GenesisTools PR: 95
File: src/ask/lib/ChatSessionManager.ts:0-0
Timestamp: 2026-03-12T01:26:03.611Z
Learning: Use SafeJSON.parse(text, { strict: true }) for strict RFC 8259 validation in all non-config boundaries (API responses, JSONL, cache, subprocess output). The 3-arg form SafeJSON.parse(text, null, { strict: true }) is invalid and should not be used. Only lenient default (no options) is appropriate for user-authored config files that may contain comments/trailing commas. Apply this guideline across TypeScript files (src/**/*.ts) wherever SafeJSON.parse is used.
Applied to files:
src/mcp-doctor/lib/duplicates.tssrc/mcp-doctor/lib/discovery.tssrc/mcp-doctor/lib/probe.tssrc/mcp-doctor/lib/types.tssrc/mcp-doctor/mcp-doctor.test.tssrc/mcp-doctor/lib/report.tssrc/mcp-doctor/index.ts
📚 Learning: 2026-03-12T01:26:18.985Z
Learnt from: genesiscz
Repo: genesiscz/GenesisTools PR: 95
File: src/claude/lib/history/search.ts:0-0
Timestamp: 2026-03-12T01:26:18.985Z
Learning: When using SafeJSON.parse in TypeScript code, prefer the two-argument form SafeJSON.parse(text, { strict: true }) to enable strict RFC 8259 validation via the native JSON.parse. Do NOT use the three-argument form SafeJSON.parse(text, null, { strict: true }). Apply strict parsing at remote/third-party API boundaries, JSONL parsing points, and subprocess output. Fall back to the lenient/default form only for user-authored config files that may legitimately contain comments or trailing commas. This pattern keeps strict validation where appropriate and preserves leniency for internal/config data.
Applied to files:
src/mcp-doctor/lib/duplicates.tssrc/mcp-doctor/lib/discovery.tssrc/mcp-doctor/lib/probe.tssrc/mcp-doctor/lib/types.tssrc/mcp-doctor/mcp-doctor.test.tssrc/mcp-doctor/lib/report.tssrc/mcp-doctor/index.ts
📚 Learning: 2026-03-12T01:26:27.000Z
Learnt from: genesiscz
Repo: genesiscz/GenesisTools PR: 95
File: src/debugging-master/commands/tail.ts:0-0
Timestamp: 2026-03-12T01:26:27.000Z
Learning: In the genesiscz/GenesisTools repository, prefer using SafeJSON.parse(text, { strict: true }) (2-argument form) at all non-config JSON boundaries such as API responses, JSONL parsers, cache files, and subprocess stdout. Reserve the lenient default (SafeJSON.parse(text) with no options) only for user-authored config files that may legitimately contain comments or trailing commas.
Applied to files:
src/mcp-doctor/lib/duplicates.tssrc/mcp-doctor/lib/discovery.tssrc/mcp-doctor/lib/probe.tssrc/mcp-doctor/lib/types.tssrc/mcp-doctor/mcp-doctor.test.tssrc/mcp-doctor/lib/report.tssrc/mcp-doctor/index.ts
📚 Learning: 2026-03-12T01:26:24.859Z
Learnt from: genesiscz
Repo: genesiscz/GenesisTools PR: 95
File: src/azure-devops/commands/history-sync.ts:0-0
Timestamp: 2026-03-12T01:26:24.859Z
Learning: In GenesisTools, ensure SafeJSON.parse is called with exactly two arguments. Use SafeJSON.parse(text, { strict: true }) for strict RFC 8259 validation, or pass a reviver function as the second argument. Do not call SafeJSON.parse(text, null, { strict: true }) since the function signature does not support a three-argument form. Apply this guideline to all TypeScript files that use SafeJSON.parse (e.g., src/utils/json.ts) and other related code.
Applied to files:
src/mcp-doctor/lib/duplicates.tssrc/mcp-doctor/lib/discovery.tssrc/mcp-doctor/lib/probe.tssrc/mcp-doctor/lib/types.tssrc/mcp-doctor/mcp-doctor.test.tssrc/mcp-doctor/lib/report.tssrc/mcp-doctor/index.ts
📚 Learning: 2026-03-17T01:30:56.939Z
Learnt from: genesiscz
Repo: genesiscz/GenesisTools PR: 107
File: src/utils/macos/tts.ts:130-139
Timestamp: 2026-03-17T01:30:56.939Z
Learning: In genesiscz/GenesisTools, do not suggest converting two-argument functions with an optional second parameter (for example setMute(muted: boolean, app?: string)) to an object-parameter form. The project prefers simple positional parameters for short utility functions, even when an optional argument is present. The object-parameter guideline should only apply when a function has 3 or more parameters.
Applied to files:
src/mcp-doctor/lib/duplicates.tssrc/mcp-doctor/lib/discovery.tssrc/mcp-doctor/lib/probe.tssrc/mcp-doctor/lib/types.tssrc/mcp-doctor/mcp-doctor.test.tssrc/mcp-doctor/lib/report.tssrc/mcp-doctor/index.ts
📚 Learning: 2026-03-22T22:19:49.876Z
Learnt from: genesiscz
Repo: genesiscz/GenesisTools PR: 119
File: src/indexer/index.ts:41-56
Timestamp: 2026-03-22T22:19:49.876Z
Learning: When using Bun projects, treat `import.meta.dir` as an absolute directory path provided by Bun. If you build paths by concatenating with `import.meta.dir` (e.g., `import.meta.dir + "/file.ts"`), do not require `path.resolve()` as it would be redundant. Only apply `path.resolve()` guidance when the base path is relative (not when the base is already an absolute `import.meta.dir`).
Applied to files:
src/mcp-doctor/lib/duplicates.tssrc/mcp-doctor/lib/discovery.tssrc/mcp-doctor/lib/probe.tssrc/mcp-doctor/lib/types.tssrc/mcp-doctor/mcp-doctor.test.tssrc/mcp-doctor/lib/report.tssrc/mcp-doctor/index.ts
📚 Learning: 2026-03-12T03:48:42.474Z
Learnt from: genesiscz
Repo: genesiscz/GenesisTools PR: 104
File: src/darwinkit/index.ts:146-156
Timestamp: 2026-03-12T03:48:42.474Z
Learning: In TypeScript files that use Commander subcommands and exit after showing help, replace code after Command.help() with the pattern: call sub.outputHelp(); (returns void) followed by process.exit(0) or process.exit(1). This avoids TS7027 unreachable-code because Command.help() returns never. Apply this pattern in all src/**/*.ts files where subcommands need to display help before exiting.
Applied to files:
src/mcp-doctor/lib/duplicates.tssrc/mcp-doctor/lib/discovery.tssrc/mcp-doctor/lib/probe.tssrc/mcp-doctor/lib/types.tssrc/mcp-doctor/mcp-doctor.test.tssrc/mcp-doctor/lib/report.tssrc/mcp-doctor/index.ts
📚 Learning: 2026-05-05T11:58:33.420Z
Learnt from: genesiscz
Repo: genesiscz/GenesisTools PR: 163
File: src/indexer/lib/sources/mail-source.dateSent.probe.test.ts:0-0
Timestamp: 2026-05-05T11:58:33.420Z
Learning: This repo uses Biome 2.x. The console lint rule is `noConsole` (located at `lint/suspicious/noConsole`), not `noConsoleLog`. In this codebase, `noConsole` is disabled in `biome.json`, so adding a `// biome-ignore lint/suspicious/noConsole:<...>` suppression comment is a no-op and should be avoided (CI flags it as having no effect). When reviewing, do not suggest adding Biome suppression comments for console usage; if a `console.*` call must remain, leave it without a `biome-ignore` comment.
Applied to files:
src/mcp-doctor/lib/duplicates.tssrc/mcp-doctor/lib/discovery.tssrc/mcp-doctor/lib/probe.tssrc/mcp-doctor/lib/types.tssrc/mcp-doctor/mcp-doctor.test.tssrc/mcp-doctor/lib/report.tssrc/mcp-doctor/index.ts
📚 Learning: 2026-05-18T14:02:30.445Z
Learnt from: genesiscz
Repo: genesiscz/GenesisTools PR: 171
File: src/utils/ui/layouts/AuthLayout.tsx:34-34
Timestamp: 2026-05-18T14:02:30.445Z
Learning: When reviewing a PR, before leaving any comment on a specific file and hunk, verify that the file (and the relevant lines) actually exist in the PR’s current diff. For example, use `git diff --name-only <base>...<head>` (or the PR’s file list) to confirm the file is part of the diff, since pre-rebase/stale hunk references can lead to incorrect or outdated comments.
Applied to files:
src/mcp-doctor/lib/duplicates.tssrc/mcp-doctor/lib/discovery.tssrc/mcp-doctor/lib/probe.tssrc/mcp-doctor/lib/types.tssrc/mcp-doctor/mcp-doctor.test.tssrc/mcp-doctor/lib/report.tssrc/mcp-doctor/index.ts
📚 Learning: 2026-03-22T22:19:53.048Z
Learnt from: genesiscz
Repo: genesiscz/GenesisTools PR: 119
File: src/utils/search/stores/qdrant-vector-store.test.ts:192-206
Timestamp: 2026-03-22T22:19:53.048Z
Learning: In src/**/*.test.ts, it is acceptable to include comments that explain the semantic role or conceptual grouping of numeric/vector test data clusters (e.g., “Cluster 1: 'code' vectors”, “Query close to 'docs' cluster”). Even if variable/identifier names partially suggest intent, these comments should be treated as readable context (describing how clusters/queries relate conceptually) rather than “obvious comments,” and should not be flagged by the no-obvious-comments rule when they genuinely clarify the test data grouping and relationships.
Applied to files:
src/mcp-doctor/mcp-doctor.test.ts
📚 Learning: 2026-05-19T18:33:15.211Z
Learnt from: genesiscz
Repo: genesiscz/GenesisTools PR: 176
File: src/telegram/index.ts:25-28
Timestamp: 2026-05-19T18:33:15.211Z
Learning: When reviewing legacy CLI entrypoint files (e.g., src/**/index.ts) that call `await runTool(program, { tool: "..." })`, allow the `.catch()` handler to keep `console.error(err); process.exit(1)` without requiring a switch to `logger.error` **only** for minimal-touch migrations that were done solely to satisfy the “no-default-import” gate and that add no new feature/behavior code. If the PR introduces any new feature logic or expands the catch-handling beyond that migration, prefer `logger.error` (and follow the repo’s normal logging conventions).
Applied to files:
src/mcp-doctor/index.ts
🔇 Additional comments (18)
src/mcp-doctor/lib/types.ts (4)
1-5: LGTM!
7-32: LGTM!
34-38: LGTM!
40-72: LGTM!src/mcp-doctor/lib/discovery.ts (1)
1-109: LGTM!src/mcp-doctor/mcp-doctor.test.ts (1)
1-188: LGTM!src/mcp-doctor/lib/probe.ts (1)
70-101: LGTM!src/mcp-doctor/lib/duplicates.ts (1)
1-31: LGTM!src/mcp-doctor/lib/report.ts (1)
1-125: LGTM!src/mcp-doctor/index.ts (9)
1-8: LGTM!
10-16: LGTM!
18-25: LGTM!
27-39: LGTM!
41-45: LGTM!
53-55: LGTM!
57-73: LGTM!
99-118: LGTM!
120-120: LGTM!
| const spinner = out.spinner(); | ||
| spinner.start(`Probing ${servers.length} servers…`); | ||
| const results = await probeAll(servers, opts); | ||
| spinner.stop("Probe complete"); |
There was a problem hiding this comment.
Check isInteractive() before using spinner.
Per coding guidelines, always check isInteractive() from @app/utils/cli before showing prompts or spinners. In non-interactive environments (e.g., CI, piped output), the spinner may cause issues or produce unwanted output.
🔧 Proposed fix to guard spinner with isInteractive check
+ const interactive = isInteractive();
- const spinner = out.spinner();
- spinner.start(`Probing ${servers.length} servers…`);
+ const spinner = interactive ? out.spinner() : null;
+ spinner?.start(`Probing ${servers.length} servers…`);
const results = await probeAll(servers, opts);
- spinner.stop("Probe complete");
+ spinner?.stop("Probe complete");Don't forget to import isInteractive:
-import { runTool } from "`@app/utils/cli`";
+import { isInteractive, runTool } from "`@app/utils/cli`";🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/mcp-doctor/index.ts` around lines 85 - 88, Import isInteractive from
`@app/utils/cli` at the top of the file, then wrap the spinner initialization and
usage (the spinner.start() and spinner.stop() calls) in a conditional check for
isInteractive(). The probeAll() function call should still execute regardless of
interactivity, but the spinner visual feedback should only be shown in
interactive environments to prevent unwanted output in CI or piped environments.
Source: Coding guidelines
There was a problem hiding this comment.
@coderabbitai Won't fix - this is already handled at the root. out.spinner() in src/logger/out.ts is defined as isQuietOutput() ? createQuietSpinner() : p.spinner(), and isQuietOutput() in src/utils/cli/output-mode.ts returns true whenever process.stdout.isTTY is false (CI, pipes). So in non-interactive output the spinner is already the no-op quiet spinner that emits no clack frames. Wrapping with isInteractive() at this call site would duplicate logic the out layer centralizes, and it would gate on stdin TTY where the spinner correctly gates on stdout TTY (the actual output channel).
There was a problem hiding this comment.
Rate Limit Exceeded
@genesiscz have exceeded the limit for the number of chat messages per hour. Please wait 5 minutes and 19 seconds before sending another message.
| return new StdioClientTransport({ | ||
| command: stdio.command, | ||
| args: stdio.args, | ||
| env: { ...process.env, ...stdio.env } as Record<string, string>, | ||
| cwd: stdio.cwd, | ||
| stderr: "ignore", | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
Type assertion on merged environment could hide undefined values.
Line 25 asserts { ...process.env, ...stdio.env } as Record<string, string>, but process.env has type NodeJS.ProcessEnv where values can be string | undefined. If process.env contains undefined values, this assertion could be incorrect.
However, this is a common pattern and the MCP SDK's StdioClientTransport likely handles it gracefully. If issues arise, consider filtering undefined values explicitly.
🛡️ Optional defensive fix
return new StdioClientTransport({
command: stdio.command,
args: stdio.args,
- env: { ...process.env, ...stdio.env } as Record<string, string>,
+ env: Object.fromEntries(
+ Object.entries({ ...process.env, ...stdio.env })
+ .filter(([, v]) => v !== undefined)
+ ) as Record<string, string>,
cwd: stdio.cwd,
stderr: "ignore",
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return new StdioClientTransport({ | |
| command: stdio.command, | |
| args: stdio.args, | |
| env: { ...process.env, ...stdio.env } as Record<string, string>, | |
| cwd: stdio.cwd, | |
| stderr: "ignore", | |
| }); | |
| return new StdioClientTransport({ | |
| command: stdio.command, | |
| args: stdio.args, | |
| env: Object.fromEntries( | |
| Object.entries({ ...process.env, ...stdio.env }) | |
| .filter(([, v]) => v !== undefined) | |
| ) as Record<string, string>, | |
| cwd: stdio.cwd, | |
| stderr: "ignore", | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/mcp-doctor/lib/probe.ts` around lines 22 - 28, The type assertion `as
Record<string, string>` on the merged environment object in the
StdioClientTransport creation is unsafe because process.env values can be string
or undefined. Remove the type assertion and either let TypeScript infer the
proper type, or defensively filter out undefined values from the merged
environment object (using Object.entries() with a filter condition) before
passing it to ensure all values are strings and the type matches what
StdioClientTransport expects.
There was a problem hiding this comment.
@coderabbitai Fixed in aa69e6b - the merged env is now built into an explicit typed accumulator that copies only entries whose value is not undefined, so the result is genuinely Record of string to string and the unsound cast is gone entirely (no cast at all now). process.env values are string-or-undefined and StdioClientTransport expects string-only env, so filtering is correct.
There was a problem hiding this comment.
Rate Limit Exceeded
@genesiscz have exceeded the limit for the number of chat messages per hour. Please wait 5 minutes and 30 seconds before sending another message.
- probe: move buildTransport inside try so a bad URL fails one server, not the batch - discovery: validate def.url with new URL(); mark malformed remotes invalid - discovery: guard mcpServers against non-object/array config values - report: make timeout status reachable; surface timeout count in header - report: fold invalid servers into error count so summary totals reconcile - index: validate --timeout/--slow as positive finite numbers
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
- probe: move buildTransport inside try so a bad URL fails one server, not the batch - discovery: validate def.url with new URL(); mark malformed remotes invalid - discovery: guard mcpServers against non-object/array config values - report: make timeout status reachable; surface timeout count in header - report: fold invalid servers into error count so summary totals reconcile - index: validate --timeout/--slow as positive finite numbers
genesiscz
force-pushed
the
feat/pro-mcp-doctor
branch
from
82b3ca5 to
48c2ce1
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
- probe.ts: filter undefined values out of merged process.env before passing to StdioClientTransport instead of an unsound 'as Record<string, string>' cast that assumed no undefined env values
1 participant
What
tools mcp-doctor— health-check & benchmark your configured MCP servers.It discovers every configured Model Context Protocol server (from the top-level
mcpServersmap of~/.claude.json, plus project-level.mcp.jsonand.cursor/mcp.json), spawns/connects to each via the installed@modelcontextprotocol/sdkClient, runs theinitializehandshake, lists its tools, measures startup latency, and reports a health table that flags dead, slow, and tool-name-colliding servers — before they bite you mid-session.CLI surface
Shared flags (work before or after the subcommand token):
--json,--timeout <ms>(default 15000),--slow <ms>(default 3000),--only <names>(comma-list),--project <dir>(default cwd).Design
Thin commander entrypoint over pure, testable lib functions:
discovery.ts—readConfigSources()(thin IO) +mergeServers()(PURE normalize, source attribution, project-over-global precedence,invalidentries for malformed defs).report.ts—classifyResult()(PURE, clock-injected status: ok/slow/timeout/error) +buildReport()/formatHealthTable()/formatConfigTable()(PURE shaping).duplicates.ts—detectDuplicateTools()(PURE cross-server tool-name collision detection, deterministically sorted).probe.ts— INTEGRATION-only: builds the matching transport (stdio / streamable-http / sse),connect()→listTools()(+ resources/prompts when advertised) →close(), guarded by the sharedwithTimeout(...)wall-clock race. This is the ONLY file that imports the SDK transports or reads the clock.Latency is measured from just-before-connect to handshake-complete. Output discipline: result data → stdout only via
out.result(...)/out.println(...); status/spinners/warnings → stderr; diagnostics →logger.--jsonemits parseable JSON with no banner bytes.Testability boundary
The default unit test imports only
discovery/duplicates/report— neverprobe.ts— so no SDK transport is loaded on the test path. The test is fully hermetic: synthetic JSON blobs + injected timestamps + fake tool lists, no network, no spawn, no dependence on the machine's live MCP config.Verification (local)
bunx biome check src/mcp-doctor— exits 0 (clean).bun test src/mcp-doctor— 14 pass, 0 fail, 35 assertions on the four pure cores (mergeServers,classifyResult,detectDuplicateTools,buildReport/formatHealthTable).tsgo --noEmit— no type errors insrc/mcp-doctor/../tools mcp-doctor --help— exit 0, listslist/check(default) /tools <server>+ shared flags../tools mcp-doctor list/check --only foowith flags after the subcommand parse correctly (shared options registered per-subcommand, read viaoptsWithGlobals()).Non-goals
No auto-fixing, no nested per-project block parsing of
~/.claude.json, no real spawning on the default test path, no call-tool, no watch/daemon mode.Summary by CodeRabbit
New Features
mcp-doctorCLI tool for managing and diagnosing MCP serverslistcommand discovers and displays configured servers without probingcheckcommand verifies server health and connectivity with concurrent probingtoolscommand inspects available tools on specific serversTests