fix: resolve CLI binary dynamically for MCP diagnostics and extension installs

Use ClaudeBinaryResolver instead of null binary path in extension services
(McpHealthDiagnosticsService, PluginInstallService, McpInstallService).
Packaged Electron on macOS has minimal PATH — bare `claude` lookup fails
with ENOENT. Now all CLI calls resolve the binary via ClaudeBinaryResolver
which checks PATH, NVM, standard install dirs and login shell env.

- Add buildEnrichedEnv() helper for child process env (PATH, HOME, USERPROFILE)
- Add stale cache re-verification with 30s TTL in ClaudeBinaryResolver
- Guard execCli() against null binaryPath with explicit error
- Replace projectPath.startsWith('/') with path.isAbsolute() for Windows
- Extract CLI_NOT_FOUND_MARKER/MESSAGE constants for consistent error detection
- Show amber info banner instead of red error when CLI not installed
This commit is contained in:
iliya 2026-03-22 13:10:11 +02:00
parent 4a889a8f3d
commit 60cf80f90a
12 changed files with 203 additions and 50 deletions

View file

@ -793,7 +793,7 @@ function initializeServices(): void {
const glamaMcpService = new GlamaMcpEnrichmentService();
const mcpAggregator = new McpCatalogAggregator(officialMcpRegistry, glamaMcpService);
const mcpStateService = new McpInstallationStateService();
const mcpHealthDiagnosticsService = new McpHealthDiagnosticsService(null);
const mcpHealthDiagnosticsService = new McpHealthDiagnosticsService();
const skillsCatalogService = new SkillsCatalogService();
const skillsMutationService = new SkillsMutationService();
skillsWatcherService = new SkillsWatcherService();
@ -804,9 +804,9 @@ function initializeServices(): void {
mcpStateService
);
// Install services — pass null for binary (uses PATH lookup via execCli fallback)
const pluginInstallService = new PluginInstallService(null, pluginCatalogService);
const mcpInstallService = new McpInstallService(null, mcpAggregator);
// Install services — resolve binary dynamically via ClaudeBinaryResolver
const pluginInstallService = new PluginInstallService(pluginCatalogService);
const mcpInstallService = new McpInstallService(mcpAggregator);
const apiKeyService = new ApiKeyService();
// warmup() and ensureInstalled() are deferred to after window creation
// (did-finish-load handler) to avoid thread pool contention at startup.
@ -1167,6 +1167,7 @@ function createWindow(): void {
}
setTimeout(() => updaterService.checkForUpdates(), 3000);
updaterService.startPeriodicCheck(60 * 60 * 1000);
// Defer non-critical startup work to avoid thread pool contention.
// The window is now visible and responsive; these run in the background.
@ -1255,6 +1256,7 @@ function createWindow(): void {
notificationManager.setMainWindow(null);
}
if (updaterService) {
updaterService.stopPeriodicCheck();
updaterService.setMainWindow(null);
}
if (cliInstallerService) {

View file

@ -7,8 +7,12 @@
* from renderer).
*/
import { ClaudeBinaryResolver } from '@main/services/team/ClaudeBinaryResolver';
import { execCli } from '@main/utils/childProcess';
import { buildEnrichedEnv } from '@main/utils/cliEnv';
import { CLI_NOT_FOUND_MESSAGE } from '@shared/constants/cli';
import { createLogger } from '@shared/utils/logger';
import path from 'path';
import type { McpCatalogAggregator } from '../catalog/McpCatalogAggregator';
import type {
@ -34,10 +38,7 @@ const HEADER_KEY_RE = /^[A-Za-z][\w-]{0,100}$/;
const TIMEOUT_MS = 30_000;
export class McpInstallService {
constructor(
private readonly claudeBinary: string | null,
private readonly aggregator: McpCatalogAggregator
) {}
constructor(private readonly aggregator: McpCatalogAggregator) {}
async install(request: McpInstallRequest): Promise<OperationResult> {
const { registryId, serverName, scope, projectPath, envValues, headers } = request;
@ -79,7 +80,7 @@ export class McpInstallService {
}
// 5. Validate projectPath (if provided, must be absolute)
if (projectPath && !projectPath.startsWith('/')) {
if (projectPath && !path.isAbsolute(projectPath)) {
return {
state: 'error',
error: 'projectPath must be an absolute path',
@ -161,9 +162,18 @@ export class McpInstallService {
// Don't log env values or header values (may contain secrets)
try {
const { stderr } = await execCli(this.claudeBinary, args, {
const claudeBinary = await ClaudeBinaryResolver.resolve();
if (!claudeBinary) {
return {
state: 'error',
error: CLI_NOT_FOUND_MESSAGE,
};
}
const { stderr } = await execCli(claudeBinary, args, {
timeout: TIMEOUT_MS,
cwd: projectPath,
env: buildEnrichedEnv(claudeBinary),
});
if (stderr) {
@ -214,7 +224,7 @@ export class McpInstallService {
}
}
if (projectPath && !projectPath.startsWith('/')) {
if (projectPath && !path.isAbsolute(projectPath)) {
return { state: 'error', error: 'projectPath must be an absolute path' };
}
@ -263,9 +273,18 @@ export class McpInstallService {
);
try {
const { stderr } = await execCli(this.claudeBinary, args, {
const claudeBinary = await ClaudeBinaryResolver.resolve();
if (!claudeBinary) {
return {
state: 'error',
error: CLI_NOT_FOUND_MESSAGE,
};
}
const { stderr } = await execCli(claudeBinary, args, {
timeout: TIMEOUT_MS,
cwd: projectPath,
env: buildEnrichedEnv(claudeBinary),
});
if (stderr) {
@ -300,7 +319,7 @@ export class McpInstallService {
};
}
if (projectPath && !projectPath.startsWith('/')) {
if (projectPath && !path.isAbsolute(projectPath)) {
return {
state: 'error',
error: 'projectPath must be an absolute path',
@ -316,9 +335,18 @@ export class McpInstallService {
logger.info(`Removing MCP server: ${name} (scope: ${scope ?? 'local'})`);
try {
await execCli(this.claudeBinary, args, {
const claudeBinary = await ClaudeBinaryResolver.resolve();
if (!claudeBinary) {
return {
state: 'error',
error: CLI_NOT_FOUND_MESSAGE,
};
}
await execCli(claudeBinary, args, {
timeout: TIMEOUT_MS,
cwd: projectPath,
env: buildEnrichedEnv(claudeBinary),
});
return { state: 'success' };
} catch (err) {

View file

@ -5,8 +5,12 @@
* from the current catalog snapshot (never trusts renderer-provided paths).
*/
import { ClaudeBinaryResolver } from '@main/services/team/ClaudeBinaryResolver';
import { execCli } from '@main/utils/childProcess';
import { buildEnrichedEnv } from '@main/utils/cliEnv';
import { CLI_NOT_FOUND_MESSAGE } from '@shared/constants/cli';
import { createLogger } from '@shared/utils/logger';
import path from 'path';
import type { PluginCatalogService } from '../catalog/PluginCatalogService';
import type { OperationResult, PluginInstallRequest } from '@shared/types/extensions';
@ -23,10 +27,7 @@ const INSTALL_TIMEOUT_MS = 120_000; // plugins may clone repos
const UNINSTALL_TIMEOUT_MS = 30_000;
export class PluginInstallService {
constructor(
private readonly claudeBinary: string | null,
private readonly catalogService: PluginCatalogService
) {}
constructor(private readonly catalogService: PluginCatalogService) {}
async install(request: PluginInstallRequest): Promise<OperationResult> {
const { pluginId, scope, projectPath } = request;
@ -40,7 +41,7 @@ export class PluginInstallService {
}
// 2. Validate projectPath
if (projectPath && !projectPath.startsWith('/')) {
if (projectPath && !path.isAbsolute(projectPath)) {
return {
state: 'error',
error: 'projectPath must be an absolute path',
@ -76,9 +77,18 @@ export class PluginInstallService {
logger.info(`Installing plugin: ${qualifiedName} (scope: ${scope ?? 'user'})`);
try {
const { stdout, stderr } = await execCli(this.claudeBinary, args, {
const claudeBinary = await ClaudeBinaryResolver.resolve();
if (!claudeBinary) {
return {
state: 'error',
error: CLI_NOT_FOUND_MESSAGE,
};
}
const { stdout, stderr } = await execCli(claudeBinary, args, {
timeout: INSTALL_TIMEOUT_MS,
cwd: projectPath,
env: buildEnrichedEnv(claudeBinary),
});
if (stderr && !stdout) {
@ -106,7 +116,7 @@ export class PluginInstallService {
};
}
if (projectPath && !projectPath.startsWith('/')) {
if (projectPath && !path.isAbsolute(projectPath)) {
return {
state: 'error',
error: 'projectPath must be an absolute path',
@ -140,9 +150,18 @@ export class PluginInstallService {
logger.info(`Uninstalling plugin: ${qualifiedName} (scope: ${scope ?? 'user'})`);
try {
await execCli(this.claudeBinary, args, {
const claudeBinary = await ClaudeBinaryResolver.resolve();
if (!claudeBinary) {
return {
state: 'error',
error: CLI_NOT_FOUND_MESSAGE,
};
}
await execCli(claudeBinary, args, {
timeout: UNINSTALL_TIMEOUT_MS,
cwd: projectPath,
env: buildEnrichedEnv(claudeBinary),
});
return { state: 'success' };
} catch (err) {

View file

@ -2,7 +2,10 @@
* Runs `claude mcp list` and parses per-server health statuses.
*/
import { ClaudeBinaryResolver } from '@main/services/team/ClaudeBinaryResolver';
import { execCli } from '@main/utils/childProcess';
import { buildEnrichedEnv } from '@main/utils/cliEnv';
import { CLI_NOT_FOUND_MESSAGE } from '@shared/constants/cli';
import { createLogger } from '@shared/utils/logger';
import type { McpServerDiagnostic, McpServerHealthStatus } from '@shared/types/extensions';
@ -12,11 +15,15 @@ const logger = createLogger('Extensions:McpHealthDiagnostics');
const TIMEOUT_MS = 30_000;
export class McpHealthDiagnosticsService {
constructor(private readonly claudeBinary: string | null) {}
async diagnose(): Promise<McpServerDiagnostic[]> {
const { stdout, stderr } = await execCli(this.claudeBinary, ['mcp', 'list'], {
const claudeBinary = await ClaudeBinaryResolver.resolve();
if (!claudeBinary) {
throw new Error(CLI_NOT_FOUND_MESSAGE);
}
const { stdout, stderr } = await execCli(claudeBinary, ['mcp', 'list'], {
timeout: TIMEOUT_MS,
env: buildEnrichedEnv(claudeBinary),
});
const output = [stdout, stderr].filter(Boolean).join('\n');

View file

@ -167,6 +167,12 @@ async function resolveFromExplicitPath(inputPath: string): Promise<string | null
let cachedPath: string | null | undefined;
/** Timestamp of last successful cache verification (ms). */
let cacheVerifiedAt = 0;
/** Re-verify cached binary at most once per 30 seconds. */
const CACHE_VERIFY_TTL_MS = 30_000;
/** Coalesce concurrent first resolves so `cachedPath` is not torn by parallel scans. */
let resolveInFlight: Promise<string | null> | null = null;
@ -177,10 +183,25 @@ export class ClaudeBinaryResolver {
*/
static clearCache(): void {
cachedPath = undefined;
cacheVerifiedAt = 0;
}
static async resolve(): Promise<string | null> {
if (cachedPath !== undefined) return cachedPath;
if (cachedPath !== undefined) {
const now = Date.now();
// Re-verify the cached binary still exists, but at most once per TTL
if (cachedPath !== null && now - cacheVerifiedAt > CACHE_VERIFY_TTL_MS) {
if (await isExecutable(cachedPath)) {
cacheVerifiedAt = now;
return cachedPath;
}
cachedPath = undefined;
cacheVerifiedAt = 0;
// Fall through to full resolution below
} else {
return cachedPath;
}
}
if (!resolveInFlight) {
resolveInFlight = ClaudeBinaryResolver.runResolve().finally(() => {
resolveInFlight = null;
@ -203,6 +224,7 @@ export class ClaudeBinaryResolver {
if (resolvedOverride) {
cachedPath = resolvedOverride;
cacheVerifiedAt = Date.now();
return cachedPath;
}
}
@ -211,6 +233,7 @@ export class ClaudeBinaryResolver {
const fromPath = await resolveFromPathEnv(baseBinaryName, enrichedPath);
if (fromPath) {
cachedPath = fromPath;
cacheVerifiedAt = Date.now();
return cachedPath;
}
@ -259,6 +282,7 @@ export class ClaudeBinaryResolver {
const found = results.find((r) => r.ok);
if (found) {
cachedPath = found.path;
cacheVerifiedAt = Date.now();
return cachedPath;
}

View file

@ -106,8 +106,7 @@ function withCliEnv<T extends { env?: NodeJS.ProcessEnv | Record<string, string
/**
* Execute a CLI binary, falling back to running the command through a
* shell on Windows if the normal path-based spawn fails. `binaryPath`
* may be `null` which causes `claude` (lookup via PATH) to be used.
* shell on Windows if the normal path-based spawn fails.
*
* The return value matches the shape of Node's `execFile` promise: an
* object with `stdout` and `stderr` strings.
@ -117,7 +116,12 @@ export async function execCli(
args: string[],
options: ExecFileOptions = {}
): Promise<{ stdout: string; stderr: string }> {
const target = binaryPath || 'claude';
if (!binaryPath) {
throw new Error(
'Claude CLI binary path is null. Resolve the binary via ClaudeBinaryResolver before calling execCli.'
);
}
const target = binaryPath;
const opts = withCliEnv(options);
// attempt the normal execFile path first

22
src/main/utils/cliEnv.ts Normal file
View file

@ -0,0 +1,22 @@
/**
* Builds an enriched environment for Claude CLI child processes.
*
* Packaged Electron apps on macOS receive a minimal PATH (often just /usr/bin:/bin).
* This helper merges the user's interactive-shell env (cached during startup) with
* common install locations so that `claude` and its subprocesses (node, npx, etc.)
* can find the tools they need.
*/
import { buildMergedCliPath } from '@main/utils/cliPathMerge';
import { getCachedShellEnv, getShellPreferredHome } from '@main/utils/shellEnv';
export function buildEnrichedEnv(binaryPath?: string | null): NodeJS.ProcessEnv {
const home = getShellPreferredHome();
return {
...process.env,
...(getCachedShellEnv() ?? {}),
HOME: home,
USERPROFILE: home,
PATH: buildMergedCliPath(binaryPath),
};
}

View file

@ -15,6 +15,7 @@ import {
} from '@renderer/components/ui/select';
import { useStore } from '@renderer/store';
import { formatRelativeTime } from '@renderer/utils/formatters';
import { CLI_NOT_FOUND_MARKER } from '@shared/constants/cli';
import { sanitizeMcpServerName } from '@shared/utils/extensionNormalizers';
import { AlertTriangle, RefreshCw, Search, Server } from 'lucide-react';
@ -311,11 +312,23 @@ export const McpServersPanel = ({
</div>
)}
{mcpDiagnosticsError && (
<div className="rounded-md border border-red-500/30 bg-red-500/5 p-4 text-sm text-red-400">
{mcpDiagnosticsError}
</div>
)}
{mcpDiagnosticsError &&
(mcpDiagnosticsError.includes(CLI_NOT_FOUND_MARKER) ? (
<div className="flex items-start gap-3 rounded-md border border-amber-500/30 bg-amber-500/5 px-4 py-3">
<AlertTriangle className="mt-0.5 size-4 shrink-0 text-amber-400" />
<div>
<p className="text-sm font-medium text-amber-300">Claude CLI not installed</p>
<p className="mt-0.5 text-xs text-text-muted">
MCP health checks require Claude CLI. Go to the Dashboard to install it
automatically.
</p>
</div>
</div>
) : (
<div className="rounded-md border border-red-500/30 bg-red-500/5 p-4 text-sm text-red-400">
{mcpDiagnosticsError}
</div>
))}
{/* Empty state */}
{!isLoading && displayServers.length === 0 && (

View file

@ -0,0 +1,15 @@
/**
* CLI error message marker.
*
* All "CLI not found" error messages across main process services MUST
* include this substring so the renderer can detect CLI-missing state
* without relying on brittle string matching against the full message.
*/
export const CLI_NOT_FOUND_MARKER = 'CLI not found';
/**
* User-facing message when CLI binary cannot be resolved.
* Contains CLI_NOT_FOUND_MARKER so the renderer can detect it.
*/
export const CLI_NOT_FOUND_MESSAGE =
'Claude CLI not found. Go to the Dashboard to install it automatically.';

View file

@ -4,6 +4,7 @@
export * from './agentBlocks';
export * from './cache';
export * from './cli';
export * from './crossTeam';
export * from './kanban';
export * from './memberColors';

View file

@ -11,6 +11,14 @@ vi.mock('@main/utils/childProcess', () => ({
execCli: vi.fn(),
}));
vi.mock('@main/services/team/ClaudeBinaryResolver', () => ({
ClaudeBinaryResolver: {
resolve: vi.fn().mockResolvedValue('/usr/local/bin/claude'),
},
}));
import { ClaudeBinaryResolver } from '@main/services/team/ClaudeBinaryResolver';
import { execCli } from '@main/utils/childProcess';
const mockExecCli = vi.mocked(execCli);
@ -67,8 +75,9 @@ describe('McpInstallService', () => {
beforeEach(() => {
vi.clearAllMocks();
vi.mocked(ClaudeBinaryResolver.resolve).mockResolvedValue('/usr/local/bin/claude');
aggregator = createMockAggregator();
service = new McpInstallService(null, aggregator);
service = new McpInstallService(aggregator);
});
afterEach(() => {
@ -91,7 +100,7 @@ describe('McpInstallService', () => {
expect(result.state).toBe('success');
expect(mockExecCli).toHaveBeenCalledWith(
null,
'/usr/local/bin/claude',
['mcp', 'add', '-s', 'user', '-e', 'UPSTASH_API_KEY=test-key-123', 'context7', '--', 'npx', '-y', '@upstash/context7-mcp@1.0.0'],
expect.objectContaining({ timeout: 30_000 }),
);
@ -135,7 +144,7 @@ describe('McpInstallService', () => {
describe('install (http)', () => {
it('builds correct CLI args for HTTP server', async () => {
aggregator = createMockAggregator(makeHttpServer());
service = new McpInstallService(null, aggregator);
service = new McpInstallService(aggregator);
mockExecCli.mockResolvedValue({ stdout: '', stderr: '' });
const result = await service.install({
@ -148,7 +157,7 @@ describe('McpInstallService', () => {
expect(result.state).toBe('success');
expect(mockExecCli).toHaveBeenCalledWith(
null,
'/usr/local/bin/claude',
['mcp', 'add', '-s', 'user', '-t', 'sse', '-H', 'Authorization: Bearer token123', 'example-http', 'https://mcp.example.com/sse'],
expect.objectContaining({ timeout: 30_000 }),
);
@ -174,7 +183,7 @@ describe('McpInstallService', () => {
it('returns error if server not found in registry', async () => {
aggregator = createMockAggregator(null);
service = new McpInstallService(null, aggregator);
service = new McpInstallService(aggregator);
const result = await service.install({
registryId: 'nonexistent',
@ -194,7 +203,7 @@ describe('McpInstallService', () => {
installSpec: null,
};
aggregator = createMockAggregator(serverNoSpec);
service = new McpInstallService(null, aggregator);
service = new McpInstallService(aggregator);
const result = await service.install({
registryId: 'test',
@ -232,7 +241,7 @@ describe('McpInstallService', () => {
it('masks header values in error messages', async () => {
aggregator = createMockAggregator(makeHttpServer());
service = new McpInstallService(null, aggregator);
service = new McpInstallService(aggregator);
mockExecCli.mockRejectedValue(
new Error('Auth failed with Bearer my-token-value'),
);
@ -260,7 +269,7 @@ describe('McpInstallService', () => {
expect(result.state).toBe('success');
expect(mockExecCli).toHaveBeenCalledWith(
null,
'/usr/local/bin/claude',
['mcp', 'remove', 'context7'],
expect.objectContaining({ timeout: 30_000 }),
);

View file

@ -10,6 +10,14 @@ vi.mock('@main/utils/childProcess', () => ({
execCli: vi.fn(),
}));
vi.mock('@main/services/team/ClaudeBinaryResolver', () => ({
ClaudeBinaryResolver: {
resolve: vi.fn().mockResolvedValue('/usr/local/bin/claude'),
},
}));
import { ClaudeBinaryResolver } from '@main/services/team/ClaudeBinaryResolver';
import { execCli } from '@main/utils/childProcess';
const mockExecCli = vi.mocked(execCli);
@ -33,8 +41,9 @@ describe('PluginInstallService', () => {
beforeEach(() => {
vi.clearAllMocks();
vi.mocked(ClaudeBinaryResolver.resolve).mockResolvedValue('/usr/local/bin/claude');
catalog = createMockCatalog();
service = new PluginInstallService(null, catalog);
service = new PluginInstallService(catalog);
});
afterEach(() => {
@ -54,7 +63,7 @@ describe('PluginInstallService', () => {
expect(result.state).toBe('success');
expect(mockExecCli).toHaveBeenCalledWith(
null,
'/usr/local/bin/claude',
['plugin', 'install', 'context7@claude-plugins-official'],
expect.objectContaining({ timeout: 120_000 }),
);
@ -70,7 +79,7 @@ describe('PluginInstallService', () => {
});
expect(mockExecCli).toHaveBeenCalledWith(
null,
'/usr/local/bin/claude',
['plugin', 'install', '-s', 'project', 'context7@claude-plugins-official'],
expect.objectContaining({ cwd: '/tmp/test-project' }),
);
@ -80,7 +89,7 @@ describe('PluginInstallService', () => {
catalog = createMockCatalog({
resolvePlugin: vi.fn().mockResolvedValue(null) as PluginCatalogService['resolvePlugin'],
});
service = new PluginInstallService(null, catalog);
service = new PluginInstallService(catalog);
const result = await service.install({ pluginId: 'nonexistent', scope: 'user' });
@ -95,7 +104,7 @@ describe('PluginInstallService', () => {
qualifiedName: '../../../etc/passwd',
}) as PluginCatalogService['resolvePlugin'],
});
service = new PluginInstallService(null, catalog);
service = new PluginInstallService(catalog);
const result = await service.install({ pluginId: 'evil', scope: 'user' });
@ -124,7 +133,7 @@ describe('PluginInstallService', () => {
expect(result.state).toBe('success');
expect(mockExecCli).toHaveBeenCalledWith(
null,
'/usr/local/bin/claude',
['plugin', 'uninstall', 'context7@claude-plugins-official'],
expect.objectContaining({ timeout: 30_000 }),
);
@ -136,7 +145,7 @@ describe('PluginInstallService', () => {
await service.uninstall('context7', 'project', '/tmp/test-project');
expect(mockExecCli).toHaveBeenCalledWith(
null,
'/usr/local/bin/claude',
['plugin', 'uninstall', '-s', 'project', 'context7@claude-plugins-official'],
expect.objectContaining({ cwd: '/tmp/test-project' }),
);
@ -146,7 +155,7 @@ describe('PluginInstallService', () => {
catalog = createMockCatalog({
resolvePlugin: vi.fn().mockResolvedValue(null) as PluginCatalogService['resolvePlugin'],
});
service = new PluginInstallService(null, catalog);
service = new PluginInstallService(catalog);
const result = await service.uninstall('nonexistent');