diff --git a/src/main/index.ts b/src/main/index.ts index 2ee7c6c1..02bb817c 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -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) { diff --git a/src/main/services/extensions/install/McpInstallService.ts b/src/main/services/extensions/install/McpInstallService.ts index ec42a3aa..3c83c0fe 100644 --- a/src/main/services/extensions/install/McpInstallService.ts +++ b/src/main/services/extensions/install/McpInstallService.ts @@ -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 { 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) { diff --git a/src/main/services/extensions/install/PluginInstallService.ts b/src/main/services/extensions/install/PluginInstallService.ts index d81f24b4..f3f91d74 100644 --- a/src/main/services/extensions/install/PluginInstallService.ts +++ b/src/main/services/extensions/install/PluginInstallService.ts @@ -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 { 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) { diff --git a/src/main/services/extensions/state/McpHealthDiagnosticsService.ts b/src/main/services/extensions/state/McpHealthDiagnosticsService.ts index 2f6e0f47..1000adc9 100644 --- a/src/main/services/extensions/state/McpHealthDiagnosticsService.ts +++ b/src/main/services/extensions/state/McpHealthDiagnosticsService.ts @@ -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 { - 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'); diff --git a/src/main/services/team/ClaudeBinaryResolver.ts b/src/main/services/team/ClaudeBinaryResolver.ts index 7df3f062..644354cf 100644 --- a/src/main/services/team/ClaudeBinaryResolver.ts +++ b/src/main/services/team/ClaudeBinaryResolver.ts @@ -167,6 +167,12 @@ async function resolveFromExplicitPath(inputPath: string): Promise | null = null; @@ -177,10 +183,25 @@ export class ClaudeBinaryResolver { */ static clearCache(): void { cachedPath = undefined; + cacheVerifiedAt = 0; } static async resolve(): Promise { - 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; } diff --git a/src/main/utils/childProcess.ts b/src/main/utils/childProcess.ts index 02165f84..82ca5118 100644 --- a/src/main/utils/childProcess.ts +++ b/src/main/utils/childProcess.ts @@ -106,8 +106,7 @@ function withCliEnv { - 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 diff --git a/src/main/utils/cliEnv.ts b/src/main/utils/cliEnv.ts new file mode 100644 index 00000000..bf08528a --- /dev/null +++ b/src/main/utils/cliEnv.ts @@ -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), + }; +} diff --git a/src/renderer/components/extensions/mcp/McpServersPanel.tsx b/src/renderer/components/extensions/mcp/McpServersPanel.tsx index e17e8335..136713cc 100644 --- a/src/renderer/components/extensions/mcp/McpServersPanel.tsx +++ b/src/renderer/components/extensions/mcp/McpServersPanel.tsx @@ -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 = ({ )} - {mcpDiagnosticsError && ( -
- {mcpDiagnosticsError} -
- )} + {mcpDiagnosticsError && + (mcpDiagnosticsError.includes(CLI_NOT_FOUND_MARKER) ? ( +
+ +
+

Claude CLI not installed

+

+ MCP health checks require Claude CLI. Go to the Dashboard to install it + automatically. +

+
+
+ ) : ( +
+ {mcpDiagnosticsError} +
+ ))} {/* Empty state */} {!isLoading && displayServers.length === 0 && ( diff --git a/src/shared/constants/cli.ts b/src/shared/constants/cli.ts new file mode 100644 index 00000000..b0789a50 --- /dev/null +++ b/src/shared/constants/cli.ts @@ -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.'; diff --git a/src/shared/constants/index.ts b/src/shared/constants/index.ts index 75122750..0cfa18c1 100644 --- a/src/shared/constants/index.ts +++ b/src/shared/constants/index.ts @@ -4,6 +4,7 @@ export * from './agentBlocks'; export * from './cache'; +export * from './cli'; export * from './crossTeam'; export * from './kanban'; export * from './memberColors'; diff --git a/test/main/services/extensions/McpInstallService.test.ts b/test/main/services/extensions/McpInstallService.test.ts index 1e4f79e6..40f1dc5a 100644 --- a/test/main/services/extensions/McpInstallService.test.ts +++ b/test/main/services/extensions/McpInstallService.test.ts @@ -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 }), ); diff --git a/test/main/services/extensions/PluginInstallService.test.ts b/test/main/services/extensions/PluginInstallService.test.ts index 4936527a..6e8f625e 100644 --- a/test/main/services/extensions/PluginInstallService.test.ts +++ b/test/main/services/extensions/PluginInstallService.test.ts @@ -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');