Code Review

I've done a thorough review of the changes across both files. Overall this is a solid PR that correctly aligns Claude CLI resolution with the existing Gemini/Codex pattern. A few observations and questions:

What looks good

1. Consistent pattern adoption — Using resolveAvailableCliCommand() with CLAUDE_CLI_PATH / appendWindowsSuffixes: true matches exactly how Gemini and Codex already work. This is the right approach.

2. resolveClaudeCommand() helper in mcp.js — Centralizing the 5 hardcoded spawn('claude', ...) calls into one helper is a clean de-duplication.

3. getSpawnOptions() helper — Simple and effective for ensuring shell: true on Windows across all spawn sites.

4. Dead import cleanup — Removing the redundant const { spawn } = await import('child_process') and const { promisify } = await import('util') / const exec = promisify(spawn) inside individual route handlers is good. These were shadowing the top-level import and exec was never even used.

Issues / Questions

1. Redundant resolution — resolveClaudeCommand() is called on every single MCP request

Each route handler (/cli/list, /cli/add, /cli/add-json, /cli/remove/:name, /cli/get/:name) calls await resolveClaudeCommand() independently. Under the hood this spawns a probe process (claude --help) every time. For a user hitting multiple MCP endpoints in sequence, this means 5+ unnecessary probe spawns.

Consider resolving once at middleware level or caching the result (even a short-lived module-level cache with a TTL would help). The auth side doesn't have this problem because checkClaudeCredentials is called infrequently, but MCP routes can be hit in rapid succession.

2. resolveClaudeCommand() falls back to 'claude' when resolution fails — is this intentional?

async function resolveClaudeCommand() {
  return await resolveAvailableCliCommand({ ... }) || 'claude';
}

If resolveAvailableCliCommand returns null (i.e., no working claude binary was found), this silently falls back to 'claude' which will then fail with ENOENT on Windows — the exact original bug. The Gemini/Codex pattern in cli-auth.js handles null explicitly by returning an error early. The auth side of this PR does this correctly (early return with checkClaudeCredentialsFile({ cliAvailable: false })), but the MCP side swallows the failure.

Suggest either: (a) returning a 503 with a "Claude CLI not installed" message when resolution fails, matching how codex.js route handlers behave, or (b) at minimum, not falling back to 'claude' so the error surfaces clearly.

3. cliCommand propagation in checkClaudeCredentials — the on('error') path has a subtle inconsistency

In the error handler:

childProcess.on('error', (error) => {
  const commandMissing = error?.code === 'ENOENT';
  checkClaudeCredentialsFile({ cliAvailable: !commandMissing, cliCommand: resolvedCliCommand }).then(resolve);
});

If the command was already resolved successfully by resolveAvailableCliCommand, getting ENOENT here would be surprising (race condition where the binary was uninstalled between probe and use?). Passing cliAvailable: false while resolvedCliCommand is a valid string could produce a confusing state for the frontend. This is a minor edge case but worth noting — the pre-resolution should make ENOENT essentially impossible in this path, so the !commandMissing logic is now dead code. Consider simplifying to always pass cliAvailable: true since we already validated the command exists.

4. Minor: whitespace-only changes inflate the diff

Several hunks in mcp.js only change trailing whitespace on blank lines (e.g., removing trailing spaces between event handlers). These are harmless but add noise — roughly 15 of the 67 added lines are whitespace-only. Not a blocker, just something to be aware of for future PRs.

Summary

The core fix is correct and follows established patterns. The main actionable item is point #2 — the || 'claude' fallback in resolveClaudeCommand() defeats the purpose of the fix for MCP routes on Windows. Points 1 and 3 are non-blocking suggestions for robustness.