From e95ef0c36966f246d2c162b4723bb748f1ef6f7a Mon Sep 17 00:00:00 2001 From: Raza Rauf Date: Thu, 29 Jan 2026 20:01:56 +0500 Subject: [PATCH 1/4] refactor: decompose agent-detector into focused modules - Extract agent-definitions.ts (221 lines) with AGENT_DEFINITIONS and types - Extract path-prober.ts (489 lines) with platform-specific binary detection - Reduce agent-detector.ts from 865 to 283 lines (67% reduction) - Add helper functions: getAgentDefinition, getAgentIds, getVisibleAgentDefinitions - Maintain API compatibility via re-exports - Add 49 new tests (26 for definitions, 23 for path-prober) --- src/__tests__/main/agent-definitions.test.ts | 253 +++++++ src/__tests__/main/agent-detector.test.ts | 2 +- src/__tests__/main/path-prober.test.ts | 452 ++++++++++++ src/main/agent-definitions.ts | 221 ++++++ src/main/agent-detector.ts | 683 ++----------------- src/main/path-prober.ts | 489 +++++++++++++ 6 files changed, 1465 insertions(+), 635 deletions(-) create mode 100644 src/__tests__/main/agent-definitions.test.ts create mode 100644 src/__tests__/main/path-prober.test.ts create mode 100644 src/main/agent-definitions.ts create mode 100644 src/main/path-prober.ts diff --git a/src/__tests__/main/agent-definitions.test.ts b/src/__tests__/main/agent-definitions.test.ts new file mode 100644 index 000000000..a864bff23 --- /dev/null +++ b/src/__tests__/main/agent-definitions.test.ts @@ -0,0 +1,253 @@ +/** + * Tests for agent-definitions.ts + * + * Tests the agent definition data structures and helper functions. + */ + +import { describe, it, expect } from 'vitest'; +import { + AGENT_DEFINITIONS, + getAgentDefinition, + getAgentIds, + getVisibleAgentDefinitions, + type AgentDefinition, + type AgentConfigOption, +} from '../../main/agent-definitions'; + +describe('agent-definitions', () => { + describe('AGENT_DEFINITIONS', () => { + it('should contain all expected agents', () => { + const agentIds = AGENT_DEFINITIONS.map((def) => def.id); + + expect(agentIds).toContain('terminal'); + expect(agentIds).toContain('claude-code'); + expect(agentIds).toContain('codex'); + expect(agentIds).toContain('opencode'); + expect(agentIds).toContain('gemini-cli'); + expect(agentIds).toContain('qwen3-coder'); + expect(agentIds).toContain('aider'); + }); + + it('should have required properties on all definitions', () => { + for (const def of AGENT_DEFINITIONS) { + expect(def.id).toBeDefined(); + expect(def.name).toBeDefined(); + expect(def.binaryName).toBeDefined(); + expect(def.command).toBeDefined(); + expect(def.args).toBeDefined(); + expect(Array.isArray(def.args)).toBe(true); + } + }); + + it('should have terminal as a hidden agent', () => { + const terminal = AGENT_DEFINITIONS.find((def) => def.id === 'terminal'); + expect(terminal?.hidden).toBe(true); + }); + + it('should have claude-code with correct base args', () => { + const claudeCode = AGENT_DEFINITIONS.find((def) => def.id === 'claude-code'); + expect(claudeCode).toBeDefined(); + expect(claudeCode?.args).toContain('--print'); + expect(claudeCode?.args).toContain('--verbose'); + expect(claudeCode?.args).toContain('--output-format'); + expect(claudeCode?.args).toContain('stream-json'); + expect(claudeCode?.args).toContain('--dangerously-skip-permissions'); + }); + + it('should have codex with batch mode configuration', () => { + const codex = AGENT_DEFINITIONS.find((def) => def.id === 'codex'); + expect(codex).toBeDefined(); + expect(codex?.batchModePrefix).toEqual(['exec']); + expect(codex?.batchModeArgs).toContain('--dangerously-bypass-approvals-and-sandbox'); + expect(codex?.jsonOutputArgs).toEqual(['--json']); + }); + + it('should have opencode with batch mode configuration', () => { + const opencode = AGENT_DEFINITIONS.find((def) => def.id === 'opencode'); + expect(opencode).toBeDefined(); + expect(opencode?.batchModePrefix).toEqual(['run']); + expect(opencode?.jsonOutputArgs).toEqual(['--format', 'json']); + expect(opencode?.noPromptSeparator).toBe(true); + }); + + it('should have opencode with default env vars for YOLO mode', () => { + const opencode = AGENT_DEFINITIONS.find((def) => def.id === 'opencode'); + expect(opencode?.defaultEnvVars).toBeDefined(); + expect(opencode?.defaultEnvVars?.OPENCODE_CONFIG_CONTENT).toContain('external_directory'); + }); + }); + + describe('getAgentDefinition', () => { + it('should return definition for valid agent ID', () => { + const claudeCode = getAgentDefinition('claude-code'); + expect(claudeCode).toBeDefined(); + expect(claudeCode?.id).toBe('claude-code'); + expect(claudeCode?.name).toBe('Claude Code'); + }); + + it('should return undefined for invalid agent ID', () => { + const invalid = getAgentDefinition('non-existent-agent'); + expect(invalid).toBeUndefined(); + }); + + it('should return definition for all known agents', () => { + const knownAgents = ['terminal', 'claude-code', 'codex', 'opencode', 'gemini-cli', 'aider']; + for (const agentId of knownAgents) { + const def = getAgentDefinition(agentId); + expect(def).toBeDefined(); + expect(def?.id).toBe(agentId); + } + }); + }); + + describe('getAgentIds', () => { + it('should return array of all agent IDs', () => { + const ids = getAgentIds(); + expect(Array.isArray(ids)).toBe(true); + expect(ids.length).toBeGreaterThan(0); + expect(ids).toContain('claude-code'); + expect(ids).toContain('terminal'); + }); + + it('should match AGENT_DEFINITIONS length', () => { + const ids = getAgentIds(); + expect(ids.length).toBe(AGENT_DEFINITIONS.length); + }); + }); + + describe('getVisibleAgentDefinitions', () => { + it('should not include hidden agents', () => { + const visible = getVisibleAgentDefinitions(); + const visibleIds = visible.map((def) => def.id); + + // Terminal should be hidden + expect(visibleIds).not.toContain('terminal'); + }); + + it('should include visible agents', () => { + const visible = getVisibleAgentDefinitions(); + const visibleIds = visible.map((def) => def.id); + + expect(visibleIds).toContain('claude-code'); + expect(visibleIds).toContain('codex'); + expect(visibleIds).toContain('opencode'); + }); + + it('should return fewer items than AGENT_DEFINITIONS', () => { + const visible = getVisibleAgentDefinitions(); + expect(visible.length).toBeLessThan(AGENT_DEFINITIONS.length); + }); + }); + + describe('Agent argument builders', () => { + it('should have resumeArgs function for claude-code', () => { + const claudeCode = getAgentDefinition('claude-code'); + expect(claudeCode?.resumeArgs).toBeDefined(); + expect(typeof claudeCode?.resumeArgs).toBe('function'); + + const args = claudeCode?.resumeArgs?.('test-session-123'); + expect(args).toEqual(['--resume', 'test-session-123']); + }); + + it('should have resumeArgs function for codex', () => { + const codex = getAgentDefinition('codex'); + expect(codex?.resumeArgs).toBeDefined(); + + const args = codex?.resumeArgs?.('thread-456'); + expect(args).toEqual(['resume', 'thread-456']); + }); + + it('should have resumeArgs function for opencode', () => { + const opencode = getAgentDefinition('opencode'); + expect(opencode?.resumeArgs).toBeDefined(); + + const args = opencode?.resumeArgs?.('session-789'); + expect(args).toEqual(['--session', 'session-789']); + }); + + it('should have modelArgs function for opencode', () => { + const opencode = getAgentDefinition('opencode'); + expect(opencode?.modelArgs).toBeDefined(); + + const args = opencode?.modelArgs?.('ollama/qwen3:8b'); + expect(args).toEqual(['--model', 'ollama/qwen3:8b']); + }); + + it('should have workingDirArgs function for codex', () => { + const codex = getAgentDefinition('codex'); + expect(codex?.workingDirArgs).toBeDefined(); + + const args = codex?.workingDirArgs?.('/path/to/project'); + expect(args).toEqual(['-C', '/path/to/project']); + }); + + it('should have imageArgs function for codex', () => { + const codex = getAgentDefinition('codex'); + expect(codex?.imageArgs).toBeDefined(); + + const args = codex?.imageArgs?.('/path/to/image.png'); + expect(args).toEqual(['-i', '/path/to/image.png']); + }); + + it('should have imageArgs function for opencode', () => { + const opencode = getAgentDefinition('opencode'); + expect(opencode?.imageArgs).toBeDefined(); + + const args = opencode?.imageArgs?.('/path/to/image.png'); + expect(args).toEqual(['-f', '/path/to/image.png']); + }); + }); + + describe('Agent config options', () => { + it('should have configOptions for codex', () => { + const codex = getAgentDefinition('codex'); + expect(codex?.configOptions).toBeDefined(); + expect(Array.isArray(codex?.configOptions)).toBe(true); + + const contextWindowOption = codex?.configOptions?.find((opt) => opt.key === 'contextWindow'); + expect(contextWindowOption).toBeDefined(); + expect(contextWindowOption?.type).toBe('number'); + expect(contextWindowOption?.default).toBe(400000); + }); + + it('should have configOptions for opencode', () => { + const opencode = getAgentDefinition('opencode'); + expect(opencode?.configOptions).toBeDefined(); + + const modelOption = opencode?.configOptions?.find((opt) => opt.key === 'model'); + expect(modelOption).toBeDefined(); + expect(modelOption?.type).toBe('text'); + expect(modelOption?.default).toBe(''); + + // Test argBuilder + expect(modelOption?.argBuilder).toBeDefined(); + expect(modelOption?.argBuilder?.('ollama/qwen3:8b')).toEqual(['--model', 'ollama/qwen3:8b']); + expect(modelOption?.argBuilder?.('')).toEqual([]); + expect(modelOption?.argBuilder?.(' ')).toEqual([]); + }); + }); + + describe('Type definitions', () => { + it('should export AgentDefinition type', () => { + const def: AgentDefinition = { + id: 'test', + name: 'Test Agent', + binaryName: 'test', + command: 'test', + args: [], + }; + expect(def.id).toBe('test'); + }); + + it('should export AgentConfigOption type', () => { + const option: AgentConfigOption = { + key: 'testKey', + type: 'text', + label: 'Test Label', + description: 'Test description', + default: 'default value', + }; + expect(option.key).toBe('testKey'); + }); + }); +}); diff --git a/src/__tests__/main/agent-detector.test.ts b/src/__tests__/main/agent-detector.test.ts index 8ebc0ae87..6965ee78d 100644 --- a/src/__tests__/main/agent-detector.test.ts +++ b/src/__tests__/main/agent-detector.test.ts @@ -499,7 +499,7 @@ describe('agent-detector', () => { expect(logger.warn).toHaveBeenCalledWith( expect.stringContaining('not executable'), - 'AgentDetector' + 'PathProber' ); } finally { Object.defineProperty(process, 'platform', { value: originalPlatform, configurable: true }); diff --git a/src/__tests__/main/path-prober.test.ts b/src/__tests__/main/path-prober.test.ts new file mode 100644 index 000000000..f05a7f160 --- /dev/null +++ b/src/__tests__/main/path-prober.test.ts @@ -0,0 +1,452 @@ +/** + * Tests for path-prober.ts + * + * Tests the platform-specific binary detection logic. + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import * as fs from 'fs'; + +// Mock dependencies before importing the module +vi.mock('../../main/utils/execFile', () => ({ + execFileNoThrow: vi.fn(), +})); + +vi.mock('../../main/utils/logger', () => ({ + logger: { + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + debug: vi.fn(), + }, +})); + +vi.mock('../../shared/pathUtils', () => ({ + expandTilde: vi.fn((p: string) => p.replace(/^~/, '/Users/testuser')), + detectNodeVersionManagerBinPaths: vi.fn(() => []), +})); + +// Import after mocking +import { + getExpandedEnv, + checkCustomPath, + checkBinaryExists, + probeWindowsPaths, + probeUnixPaths, + type BinaryDetectionResult, +} from '../../main/path-prober'; +import { execFileNoThrow } from '../../main/utils/execFile'; +import { logger } from '../../main/utils/logger'; + +describe('path-prober', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + describe('getExpandedEnv', () => { + it('should return environment with PATH', () => { + const env = getExpandedEnv(); + expect(env.PATH).toBeDefined(); + expect(typeof env.PATH).toBe('string'); + }); + + it('should include common Unix paths on non-Windows', () => { + const originalPlatform = process.platform; + Object.defineProperty(process, 'platform', { value: 'darwin', configurable: true }); + + try { + const env = getExpandedEnv(); + expect(env.PATH).toContain('/opt/homebrew/bin'); + expect(env.PATH).toContain('/usr/local/bin'); + } finally { + Object.defineProperty(process, 'platform', { value: originalPlatform, configurable: true }); + } + }); + + it('should preserve existing PATH entries', () => { + const originalPath = process.env.PATH; + const testPath = '/test/custom/path'; + process.env.PATH = testPath; + + try { + const env = getExpandedEnv(); + expect(env.PATH).toContain(testPath); + } finally { + process.env.PATH = originalPath; + } + }); + }); + + describe('checkCustomPath', () => { + let statMock: ReturnType; + let accessMock: ReturnType; + + beforeEach(() => { + statMock = vi.spyOn(fs.promises, 'stat'); + accessMock = vi.spyOn(fs.promises, 'access'); + }); + + afterEach(() => { + statMock.mockRestore(); + accessMock.mockRestore(); + }); + + it('should return exists: true for valid executable path on Unix', async () => { + const originalPlatform = process.platform; + Object.defineProperty(process, 'platform', { value: 'darwin', configurable: true }); + + try { + statMock.mockResolvedValue({ isFile: () => true } as fs.Stats); + accessMock.mockResolvedValue(undefined); + + const result = await checkCustomPath('/usr/local/bin/claude'); + expect(result.exists).toBe(true); + expect(result.path).toBe('/usr/local/bin/claude'); + } finally { + Object.defineProperty(process, 'platform', { value: originalPlatform, configurable: true }); + } + }); + + it('should return exists: false for non-executable file on Unix', async () => { + const originalPlatform = process.platform; + Object.defineProperty(process, 'platform', { value: 'darwin', configurable: true }); + + try { + statMock.mockResolvedValue({ isFile: () => true } as fs.Stats); + accessMock.mockRejectedValue(new Error('EACCES')); + + const result = await checkCustomPath('/path/to/non-executable'); + expect(result.exists).toBe(false); + expect(logger.warn).toHaveBeenCalledWith( + expect.stringContaining('not executable'), + 'PathProber' + ); + } finally { + Object.defineProperty(process, 'platform', { value: originalPlatform, configurable: true }); + } + }); + + it('should return exists: false for non-existent path', async () => { + statMock.mockRejectedValue(new Error('ENOENT')); + + const result = await checkCustomPath('/non/existent/path'); + expect(result.exists).toBe(false); + }); + + it('should expand tilde in path', async () => { + const originalPlatform = process.platform; + Object.defineProperty(process, 'platform', { value: 'darwin', configurable: true }); + + try { + statMock.mockResolvedValue({ isFile: () => true } as fs.Stats); + accessMock.mockResolvedValue(undefined); + + const result = await checkCustomPath('~/.local/bin/claude'); + expect(result.exists).toBe(true); + expect(result.path).toBe('/Users/testuser/.local/bin/claude'); + } finally { + Object.defineProperty(process, 'platform', { value: originalPlatform, configurable: true }); + } + }); + + it('should try .exe extension on Windows', async () => { + const originalPlatform = process.platform; + Object.defineProperty(process, 'platform', { value: 'win32', configurable: true }); + + try { + // First call (exact path) returns false, second call (.exe) returns true + statMock + .mockRejectedValueOnce(new Error('ENOENT')) + .mockResolvedValueOnce({ isFile: () => true } as fs.Stats); + + const result = await checkCustomPath('C:\\custom\\claude'); + expect(result.exists).toBe(true); + expect(result.path).toBe('C:\\custom\\claude.exe'); + } finally { + Object.defineProperty(process, 'platform', { value: originalPlatform, configurable: true }); + } + }); + + it('should try .cmd extension on Windows if .exe not found', async () => { + const originalPlatform = process.platform; + Object.defineProperty(process, 'platform', { value: 'win32', configurable: true }); + + try { + // First call (exact), second (.exe) return false, third (.cmd) returns true + statMock + .mockRejectedValueOnce(new Error('ENOENT')) + .mockRejectedValueOnce(new Error('ENOENT')) + .mockResolvedValueOnce({ isFile: () => true } as fs.Stats); + + const result = await checkCustomPath('C:\\custom\\claude'); + expect(result.exists).toBe(true); + expect(result.path).toBe('C:\\custom\\claude.cmd'); + } finally { + Object.defineProperty(process, 'platform', { value: originalPlatform, configurable: true }); + } + }); + + it('should skip executable check on Windows', async () => { + const originalPlatform = process.platform; + Object.defineProperty(process, 'platform', { value: 'win32', configurable: true }); + + try { + statMock.mockResolvedValue({ isFile: () => true } as fs.Stats); + // Don't mock access - it shouldn't be called for X_OK on Windows + + const result = await checkCustomPath('C:\\custom\\claude.exe'); + expect(result.exists).toBe(true); + // access should not be called with X_OK on Windows + } finally { + Object.defineProperty(process, 'platform', { value: originalPlatform, configurable: true }); + } + }); + }); + + describe('probeWindowsPaths', () => { + let accessMock: ReturnType; + + beforeEach(() => { + accessMock = vi.spyOn(fs.promises, 'access'); + }); + + afterEach(() => { + accessMock.mockRestore(); + }); + + it('should return null for unknown binary', async () => { + accessMock.mockRejectedValue(new Error('ENOENT')); + + const result = await probeWindowsPaths('unknown-binary'); + expect(result).toBeNull(); + }); + + it('should probe known paths for claude binary', async () => { + // All paths fail - binary not found + accessMock.mockRejectedValue(new Error('ENOENT')); + + const result = await probeWindowsPaths('claude'); + // Should return null since all probes fail + expect(result).toBeNull(); + // Should have tried multiple paths + expect(accessMock).toHaveBeenCalled(); + }); + }); + + describe('probeUnixPaths', () => { + let accessMock: ReturnType; + + beforeEach(() => { + accessMock = vi.spyOn(fs.promises, 'access'); + }); + + afterEach(() => { + accessMock.mockRestore(); + }); + + it('should return null for unknown binary', async () => { + accessMock.mockRejectedValue(new Error('ENOENT')); + + const result = await probeUnixPaths('unknown-binary'); + expect(result).toBeNull(); + }); + + it('should probe known paths for claude binary', async () => { + // All paths fail - binary not found + accessMock.mockRejectedValue(new Error('ENOENT')); + + const result = await probeUnixPaths('claude'); + // Should return null since all probes fail + expect(result).toBeNull(); + // Should have tried multiple paths + expect(accessMock).toHaveBeenCalled(); + }); + + it('should check both existence and executability', async () => { + const originalPlatform = process.platform; + Object.defineProperty(process, 'platform', { value: 'darwin', configurable: true }); + + try { + accessMock.mockRejectedValue(new Error('ENOENT')); + + const result = await probeUnixPaths('claude'); + expect(result).toBeNull(); + + // Verify access was called with F_OK | X_OK + expect(accessMock).toHaveBeenCalled(); + } finally { + Object.defineProperty(process, 'platform', { value: originalPlatform, configurable: true }); + } + }); + }); + + describe('checkBinaryExists', () => { + let accessMock: ReturnType; + const execMock = vi.mocked(execFileNoThrow); + + beforeEach(() => { + accessMock = vi.spyOn(fs.promises, 'access'); + }); + + afterEach(() => { + accessMock.mockRestore(); + }); + + it('should try direct probe first on Unix', async () => { + const originalPlatform = process.platform; + Object.defineProperty(process, 'platform', { value: 'darwin', configurable: true }); + + try { + // Direct probe finds the binary (first path in the list exists) + accessMock.mockResolvedValueOnce(undefined); + + const result = await checkBinaryExists('claude'); + expect(result.exists).toBe(true); + expect(result.path).toContain('claude'); + // which should not be called if direct probe succeeds + } finally { + Object.defineProperty(process, 'platform', { value: originalPlatform, configurable: true }); + } + }); + + it('should fall back to which on Unix if probe fails', async () => { + const originalPlatform = process.platform; + Object.defineProperty(process, 'platform', { value: 'darwin', configurable: true }); + + try { + // Direct probe fails + accessMock.mockRejectedValue(new Error('ENOENT')); + + // which succeeds + execMock.mockResolvedValue({ + exitCode: 0, + stdout: '/usr/local/bin/test-binary\n', + stderr: '', + }); + + const result = await checkBinaryExists('test-binary'); + expect(result.exists).toBe(true); + expect(result.path).toBe('/usr/local/bin/test-binary'); + expect(execMock).toHaveBeenCalledWith( + 'which', + ['test-binary'], + undefined, + expect.any(Object) + ); + } finally { + Object.defineProperty(process, 'platform', { value: originalPlatform, configurable: true }); + } + }); + + it('should use where on Windows', async () => { + const originalPlatform = process.platform; + Object.defineProperty(process, 'platform', { value: 'win32', configurable: true }); + + try { + // Direct probe fails + accessMock.mockRejectedValue(new Error('ENOENT')); + + // where succeeds + execMock.mockResolvedValue({ + exitCode: 0, + stdout: 'C:\\Users\\Test\\AppData\\Roaming\\npm\\test.cmd\r\n', + stderr: '', + }); + + const result = await checkBinaryExists('test'); + expect(result.exists).toBe(true); + expect(execMock).toHaveBeenCalledWith('where', ['test'], undefined, expect.any(Object)); + } finally { + Object.defineProperty(process, 'platform', { value: originalPlatform, configurable: true }); + } + }); + + it('should return exists: false if binary not found', async () => { + const originalPlatform = process.platform; + Object.defineProperty(process, 'platform', { value: 'darwin', configurable: true }); + + try { + // Direct probe fails + accessMock.mockRejectedValue(new Error('ENOENT')); + + // which fails + execMock.mockResolvedValue({ + exitCode: 1, + stdout: '', + stderr: 'not found', + }); + + const result = await checkBinaryExists('non-existent'); + expect(result.exists).toBe(false); + expect(result.path).toBeUndefined(); + } finally { + Object.defineProperty(process, 'platform', { value: originalPlatform, configurable: true }); + } + }); + + it('should prefer .exe over .cmd on Windows', async () => { + const originalPlatform = process.platform; + Object.defineProperty(process, 'platform', { value: 'win32', configurable: true }); + + try { + // Direct probe fails + accessMock.mockRejectedValue(new Error('ENOENT')); + + // where returns both .exe and .cmd + execMock.mockResolvedValue({ + exitCode: 0, + stdout: 'C:\\path\\to\\binary.cmd\r\nC:\\path\\to\\binary.exe\r\n', + stderr: '', + }); + + const result = await checkBinaryExists('binary'); + expect(result.exists).toBe(true); + expect(result.path).toBe('C:\\path\\to\\binary.exe'); + } finally { + Object.defineProperty(process, 'platform', { value: originalPlatform, configurable: true }); + } + }); + + it('should handle Windows CRLF line endings', async () => { + const originalPlatform = process.platform; + Object.defineProperty(process, 'platform', { value: 'win32', configurable: true }); + + try { + accessMock.mockRejectedValue(new Error('ENOENT')); + + execMock.mockResolvedValue({ + exitCode: 0, + stdout: 'C:\\path\\to\\binary.exe\r\n', + stderr: '', + }); + + const result = await checkBinaryExists('binary'); + expect(result.exists).toBe(true); + expect(result.path).toBe('C:\\path\\to\\binary.exe'); + // Path should not contain \r + expect(result.path).not.toContain('\r'); + } finally { + Object.defineProperty(process, 'platform', { value: originalPlatform, configurable: true }); + } + }); + }); + + describe('BinaryDetectionResult type', () => { + it('should allow exists: true with path', () => { + const result: BinaryDetectionResult = { + exists: true, + path: '/usr/local/bin/claude', + }; + expect(result.exists).toBe(true); + expect(result.path).toBeDefined(); + }); + + it('should allow exists: false without path', () => { + const result: BinaryDetectionResult = { + exists: false, + }; + expect(result.exists).toBe(false); + expect(result.path).toBeUndefined(); + }); + }); +}); diff --git a/src/main/agent-definitions.ts b/src/main/agent-definitions.ts new file mode 100644 index 000000000..c41650e25 --- /dev/null +++ b/src/main/agent-definitions.ts @@ -0,0 +1,221 @@ +/** + * Agent Definitions + * + * Contains the configuration definitions for all supported AI agents. + * This includes CLI arguments, configuration options, and default settings. + * + * Separated from agent-detector.ts for better maintainability. + */ + +import type { AgentCapabilities } from './agent-capabilities'; + +// ============ Configuration Types ============ + +/** + * Configuration option types for agent-specific settings + */ +export interface AgentConfigOption { + key: string; // Storage key + type: 'checkbox' | 'text' | 'number' | 'select'; + label: string; // UI label + description: string; // Help text + default: any; // Default value + options?: string[]; // For select type + argBuilder?: (value: any) => string[]; // Converts config value to CLI args +} + +/** + * Full agent configuration including runtime detection state + */ +export interface AgentConfig { + id: string; + name: string; + binaryName: string; + command: string; + args: string[]; // Base args always included (excludes batch mode prefix) + available: boolean; + path?: string; + customPath?: string; // User-specified custom path (shown in UI even if not available) + requiresPty?: boolean; // Whether this agent needs a pseudo-terminal + configOptions?: AgentConfigOption[]; // Agent-specific configuration + hidden?: boolean; // If true, agent is hidden from UI (internal use only) + capabilities: AgentCapabilities; // Agent feature capabilities + + // Argument builders for dynamic CLI construction + // These are optional - agents that don't have them use hardcoded behavior + batchModePrefix?: string[]; // Args added before base args for batch mode (e.g., ['run'] for OpenCode) + batchModeArgs?: string[]; // Args only applied in batch mode (e.g., ['--skip-git-repo-check'] for Codex exec) + jsonOutputArgs?: string[]; // Args for JSON output format (e.g., ['--format', 'json']) + resumeArgs?: (sessionId: string) => string[]; // Function to build resume args + readOnlyArgs?: string[]; // Args for read-only/plan mode (e.g., ['--agent', 'plan']) + modelArgs?: (modelId: string) => string[]; // Function to build model selection args (e.g., ['--model', modelId]) + yoloModeArgs?: string[]; // Args for YOLO/full-access mode (e.g., ['--dangerously-bypass-approvals-and-sandbox']) + workingDirArgs?: (dir: string) => string[]; // Function to build working directory args (e.g., ['-C', dir]) + imageArgs?: (imagePath: string) => string[]; // Function to build image attachment args (e.g., ['-i', imagePath] for Codex) + promptArgs?: (prompt: string) => string[]; // Function to build prompt args (e.g., ['-p', prompt] for OpenCode) + noPromptSeparator?: boolean; // If true, don't add '--' before the prompt in batch mode (OpenCode doesn't support it) + defaultEnvVars?: Record; // Default environment variables for this agent (merged with user customEnvVars) +} + +/** + * Agent definition without runtime detection state (used for static definitions) + */ +export type AgentDefinition = Omit; + +// ============ Agent Definitions ============ + +/** + * Static definitions for all supported agents. + * These are the base configurations before runtime detection adds availability info. + */ +export const AGENT_DEFINITIONS: AgentDefinition[] = [ + { + id: 'terminal', + name: 'Terminal', + // Use platform-appropriate default shell + binaryName: process.platform === 'win32' ? 'powershell.exe' : 'bash', + command: process.platform === 'win32' ? 'powershell.exe' : 'bash', + args: [], + requiresPty: true, + hidden: true, // Internal agent, not shown in UI + }, + { + id: 'claude-code', + name: 'Claude Code', + binaryName: 'claude', + command: 'claude', + // YOLO mode (--dangerously-skip-permissions) is always enabled - Maestro requires it + args: [ + '--print', + '--verbose', + '--output-format', + 'stream-json', + '--dangerously-skip-permissions', + ], + resumeArgs: (sessionId: string) => ['--resume', sessionId], // Resume with session ID + readOnlyArgs: ['--permission-mode', 'plan'], // Read-only/plan mode + }, + { + id: 'codex', + name: 'Codex', + binaryName: 'codex', + command: 'codex', + // Base args for interactive mode (no flags that are exec-only) + args: [], + // Codex CLI argument builders + // Batch mode: codex exec --json --dangerously-bypass-approvals-and-sandbox --skip-git-repo-check [--sandbox read-only] [-C dir] [resume ] -- "prompt" + // Sandbox modes: + // - Default (YOLO): --dangerously-bypass-approvals-and-sandbox (full system access, required by Maestro) + // - Read-only: --sandbox read-only (can only read files, overrides YOLO) + batchModePrefix: ['exec'], // Codex uses 'exec' subcommand for batch mode + batchModeArgs: ['--dangerously-bypass-approvals-and-sandbox', '--skip-git-repo-check'], // Args only valid on 'exec' subcommand + jsonOutputArgs: ['--json'], // JSON output format (must come before resume subcommand) + resumeArgs: (sessionId: string) => ['resume', sessionId], // Resume with session/thread ID + readOnlyArgs: ['--sandbox', 'read-only'], // Read-only/plan mode + yoloModeArgs: ['--dangerously-bypass-approvals-and-sandbox'], // Full access mode + workingDirArgs: (dir: string) => ['-C', dir], // Set working directory + imageArgs: (imagePath: string) => ['-i', imagePath], // Image attachment: codex exec -i /path/to/image.png + // Agent-specific configuration options shown in UI + configOptions: [ + { + key: 'contextWindow', + type: 'number', + label: 'Context Window Size', + description: + 'Maximum context window size in tokens. Required for context usage display. Common values: 400000 (GPT-5.2), 128000 (GPT-4o).', + default: 400000, // Default for GPT-5.2 models + }, + ], + }, + { + id: 'gemini-cli', + name: 'Gemini CLI', + binaryName: 'gemini', + command: 'gemini', + args: [], + }, + { + id: 'qwen3-coder', + name: 'Qwen3 Coder', + binaryName: 'qwen3-coder', + command: 'qwen3-coder', + args: [], + }, + { + id: 'opencode', + name: 'OpenCode', + binaryName: 'opencode', + command: 'opencode', + args: [], // Base args (none for OpenCode - batch mode uses 'run' subcommand) + // OpenCode CLI argument builders + // Batch mode: opencode run --format json [--model provider/model] [--session ] [--agent plan] "prompt" + // YOLO mode (auto-approve all permissions) is enabled via OPENCODE_CONFIG_CONTENT env var. + // This prevents OpenCode from prompting for permission on external_directory access, which would hang in batch mode. + batchModePrefix: ['run'], // OpenCode uses 'run' subcommand for batch mode + jsonOutputArgs: ['--format', 'json'], // JSON output format + resumeArgs: (sessionId: string) => ['--session', sessionId], // Resume with session ID + readOnlyArgs: ['--agent', 'plan'], // Read-only/plan mode + modelArgs: (modelId: string) => ['--model', modelId], // Model selection (e.g., 'ollama/qwen3:8b') + imageArgs: (imagePath: string) => ['-f', imagePath], // Image/file attachment: opencode run -f /path/to/image.png -- "prompt" + noPromptSeparator: true, // OpenCode doesn't need '--' before prompt - yargs handles positional args + // Default env vars: enable YOLO mode (allow all permissions including external_directory) + // Users can override by setting customEnvVars in agent config + defaultEnvVars: { + OPENCODE_CONFIG_CONTENT: '{"permission":{"*":"allow","external_directory":"allow"}}', + }, + // Agent-specific configuration options shown in UI + configOptions: [ + { + key: 'model', + type: 'text', + label: 'Model', + description: + 'Model to use (e.g., "ollama/qwen3:8b", "anthropic/claude-sonnet-4-20250514"). Leave empty for default.', + default: '', // Empty string means use OpenCode's default model + argBuilder: (value: string) => { + // Only add --model arg if a model is specified + if (value && value.trim()) { + return ['--model', value.trim()]; + } + return []; + }, + }, + { + key: 'contextWindow', + type: 'number', + label: 'Context Window Size', + description: + 'Maximum context window size in tokens. Required for context usage display. Varies by model (e.g., 400000 for Claude/GPT-5.2, 128000 for GPT-4o).', + default: 128000, // Default for common models (GPT-4, etc.) + }, + ], + }, + { + id: 'aider', + name: 'Aider', + binaryName: 'aider', + command: 'aider', + args: [], // Base args (placeholder - to be configured when implemented) + }, +]; + +/** + * Get an agent definition by ID (without runtime detection state) + */ +export function getAgentDefinition(agentId: string): AgentDefinition | undefined { + return AGENT_DEFINITIONS.find((def) => def.id === agentId); +} + +/** + * Get all agent IDs + */ +export function getAgentIds(): string[] { + return AGENT_DEFINITIONS.map((def) => def.id); +} + +/** + * Get all visible (non-hidden) agent definitions + */ +export function getVisibleAgentDefinitions(): AgentDefinition[] { + return AGENT_DEFINITIONS.filter((def) => !def.hidden); +} diff --git a/src/main/agent-detector.ts b/src/main/agent-detector.ts index 890f99fb2..4419bece3 100644 --- a/src/main/agent-detector.ts +++ b/src/main/agent-detector.ts @@ -1,185 +1,39 @@ +/** + * Agent Detector - Detects available AI agents on the system + * + * This module provides the main AgentDetector class that: + * - Detects which agents are available on the system + * - Manages custom paths for agents + * - Caches detection results for performance + * - Discovers available models for agents that support model selection + * + * Architecture: + * - Agent definitions are in agent-definitions.ts + * - Path probing logic is in path-prober.ts + * - Agent capabilities are in agent-capabilities.ts + */ + +import * as path from 'path'; import { execFileNoThrow } from './utils/execFile'; import { logger } from './utils/logger'; -import * as os from 'os'; -import * as fs from 'fs'; -import * as path from 'path'; -import { AgentCapabilities, getAgentCapabilities } from './agent-capabilities'; -import { expandTilde, detectNodeVersionManagerBinPaths } from '../shared/pathUtils'; +import { getAgentCapabilities } from './agent-capabilities'; +import { checkBinaryExists, checkCustomPath, getExpandedEnv } from './path-prober'; +import { AGENT_DEFINITIONS, type AgentConfig } from './agent-definitions'; + +// ============ Re-exports for API Compatibility ============ +// These exports maintain backwards compatibility with existing code -// Re-export AgentCapabilities for convenience export { AgentCapabilities } from './agent-capabilities'; +export { + AGENT_DEFINITIONS, + type AgentConfig, + type AgentConfigOption, + type AgentDefinition, +} from './agent-definitions'; -// Configuration option types for agent-specific settings -export interface AgentConfigOption { - key: string; // Storage key - type: 'checkbox' | 'text' | 'number' | 'select'; - label: string; // UI label - description: string; // Help text - default: any; // Default value - options?: string[]; // For select type - argBuilder?: (value: any) => string[]; // Converts config value to CLI args -} +const LOG_CONTEXT = 'AgentDetector'; -export interface AgentConfig { - id: string; - name: string; - binaryName: string; - command: string; - args: string[]; // Base args always included (excludes batch mode prefix) - available: boolean; - path?: string; - customPath?: string; // User-specified custom path (shown in UI even if not available) - requiresPty?: boolean; // Whether this agent needs a pseudo-terminal - configOptions?: AgentConfigOption[]; // Agent-specific configuration - hidden?: boolean; // If true, agent is hidden from UI (internal use only) - capabilities: AgentCapabilities; // Agent feature capabilities - - // Argument builders for dynamic CLI construction - // These are optional - agents that don't have them use hardcoded behavior - batchModePrefix?: string[]; // Args added before base args for batch mode (e.g., ['run'] for OpenCode) - batchModeArgs?: string[]; // Args only applied in batch mode (e.g., ['--skip-git-repo-check'] for Codex exec) - jsonOutputArgs?: string[]; // Args for JSON output format (e.g., ['--format', 'json']) - resumeArgs?: (sessionId: string) => string[]; // Function to build resume args - readOnlyArgs?: string[]; // Args for read-only/plan mode (e.g., ['--agent', 'plan']) - modelArgs?: (modelId: string) => string[]; // Function to build model selection args (e.g., ['--model', modelId]) - yoloModeArgs?: string[]; // Args for YOLO/full-access mode (e.g., ['--dangerously-bypass-approvals-and-sandbox']) - workingDirArgs?: (dir: string) => string[]; // Function to build working directory args (e.g., ['-C', dir]) - imageArgs?: (imagePath: string) => string[]; // Function to build image attachment args (e.g., ['-i', imagePath] for Codex) - promptArgs?: (prompt: string) => string[]; // Function to build prompt args (e.g., ['-p', prompt] for OpenCode) - noPromptSeparator?: boolean; // If true, don't add '--' before the prompt in batch mode (OpenCode doesn't support it) - defaultEnvVars?: Record; // Default environment variables for this agent (merged with user customEnvVars) -} - -export const AGENT_DEFINITIONS: Omit[] = [ - { - id: 'terminal', - name: 'Terminal', - // Use platform-appropriate default shell - binaryName: process.platform === 'win32' ? 'powershell.exe' : 'bash', - command: process.platform === 'win32' ? 'powershell.exe' : 'bash', - args: [], - requiresPty: true, - hidden: true, // Internal agent, not shown in UI - }, - { - id: 'claude-code', - name: 'Claude Code', - binaryName: 'claude', - command: 'claude', - // YOLO mode (--dangerously-skip-permissions) is always enabled - Maestro requires it - args: [ - '--print', - '--verbose', - '--output-format', - 'stream-json', - '--dangerously-skip-permissions', - ], - resumeArgs: (sessionId: string) => ['--resume', sessionId], // Resume with session ID - readOnlyArgs: ['--permission-mode', 'plan'], // Read-only/plan mode - }, - { - id: 'codex', - name: 'Codex', - binaryName: 'codex', - command: 'codex', - // Base args for interactive mode (no flags that are exec-only) - args: [], - // Codex CLI argument builders - // Batch mode: codex exec --json --dangerously-bypass-approvals-and-sandbox --skip-git-repo-check [--sandbox read-only] [-C dir] [resume ] -- "prompt" - // Sandbox modes: - // - Default (YOLO): --dangerously-bypass-approvals-and-sandbox (full system access, required by Maestro) - // - Read-only: --sandbox read-only (can only read files, overrides YOLO) - batchModePrefix: ['exec'], // Codex uses 'exec' subcommand for batch mode - batchModeArgs: ['--dangerously-bypass-approvals-and-sandbox', '--skip-git-repo-check'], // Args only valid on 'exec' subcommand - jsonOutputArgs: ['--json'], // JSON output format (must come before resume subcommand) - resumeArgs: (sessionId: string) => ['resume', sessionId], // Resume with session/thread ID - readOnlyArgs: ['--sandbox', 'read-only'], // Read-only/plan mode - yoloModeArgs: ['--dangerously-bypass-approvals-and-sandbox'], // Full access mode - workingDirArgs: (dir: string) => ['-C', dir], // Set working directory - imageArgs: (imagePath: string) => ['-i', imagePath], // Image attachment: codex exec -i /path/to/image.png - // Agent-specific configuration options shown in UI - configOptions: [ - { - key: 'contextWindow', - type: 'number', - label: 'Context Window Size', - description: - 'Maximum context window size in tokens. Required for context usage display. Common values: 400000 (GPT-5.2), 128000 (GPT-4o).', - default: 400000, // Default for GPT-5.2 models - }, - ], - }, - { - id: 'gemini-cli', - name: 'Gemini CLI', - binaryName: 'gemini', - command: 'gemini', - args: [], - }, - { - id: 'qwen3-coder', - name: 'Qwen3 Coder', - binaryName: 'qwen3-coder', - command: 'qwen3-coder', - args: [], - }, - { - id: 'opencode', - name: 'OpenCode', - binaryName: 'opencode', - command: 'opencode', - args: [], // Base args (none for OpenCode - batch mode uses 'run' subcommand) - // OpenCode CLI argument builders - // Batch mode: opencode run --format json [--model provider/model] [--session ] [--agent plan] "prompt" - // YOLO mode (auto-approve all permissions) is enabled via OPENCODE_CONFIG_CONTENT env var. - // This prevents OpenCode from prompting for permission on external_directory access, which would hang in batch mode. - batchModePrefix: ['run'], // OpenCode uses 'run' subcommand for batch mode - jsonOutputArgs: ['--format', 'json'], // JSON output format - resumeArgs: (sessionId: string) => ['--session', sessionId], // Resume with session ID - readOnlyArgs: ['--agent', 'plan'], // Read-only/plan mode - modelArgs: (modelId: string) => ['--model', modelId], // Model selection (e.g., 'ollama/qwen3:8b') - imageArgs: (imagePath: string) => ['-f', imagePath], // Image/file attachment: opencode run -f /path/to/image.png -- "prompt" - noPromptSeparator: true, // OpenCode doesn't need '--' before prompt - yargs handles positional args - // Default env vars: enable YOLO mode (allow all permissions including external_directory) - // Users can override by setting customEnvVars in agent config - defaultEnvVars: { - OPENCODE_CONFIG_CONTENT: '{"permission":{"*":"allow","external_directory":"allow"}}', - }, - // Agent-specific configuration options shown in UI - configOptions: [ - { - key: 'model', - type: 'text', - label: 'Model', - description: - 'Model to use (e.g., "ollama/qwen3:8b", "anthropic/claude-sonnet-4-20250514"). Leave empty for default.', - default: '', // Empty string means use OpenCode's default model - argBuilder: (value: string) => { - // Only add --model arg if a model is specified - if (value && value.trim()) { - return ['--model', value.trim()]; - } - return []; - }, - }, - { - key: 'contextWindow', - type: 'number', - label: 'Context Window Size', - description: - 'Maximum context window size in tokens. Required for context usage display. Varies by model (e.g., 400000 for Claude/GPT-5.2, 128000 for GPT-4o).', - default: 128000, // Default for common models (GPT-4, etc.) - }, - ], - }, - { - id: 'aider', - name: 'Aider', - binaryName: 'aider', - command: 'aider', - args: [], // Base args (placeholder - to be configured when implemented) - }, -]; +// ============ Agent Detector Class ============ export class AgentDetector { private cachedAgents: AgentConfig[] | null = null; @@ -234,9 +88,9 @@ export class AgentDetector { */ private async doDetectAgents(): Promise { const agents: AgentConfig[] = []; - const expandedEnv = this.getExpandedEnv(); + const expandedEnv = getExpandedEnv(); - logger.info(`Agent detection starting. PATH: ${expandedEnv.PATH}`, 'AgentDetector'); + logger.info(`Agent detection starting. PATH: ${expandedEnv.PATH}`, LOG_CONTEXT); for (const agentDef of AGENT_DEFINITIONS) { const customPath = this.customPaths[agentDef.id]; @@ -244,37 +98,34 @@ export class AgentDetector { // If user has specified a custom path, check that first if (customPath) { - detection = await this.checkCustomPath(customPath); + detection = await checkCustomPath(customPath); if (detection.exists) { logger.info( `Agent "${agentDef.name}" found at custom path: ${detection.path}`, - 'AgentDetector' + LOG_CONTEXT ); } else { - logger.warn( - `Agent "${agentDef.name}" custom path not valid: ${customPath}`, - 'AgentDetector' - ); + logger.warn(`Agent "${agentDef.name}" custom path not valid: ${customPath}`, LOG_CONTEXT); // Fall back to PATH detection - detection = await this.checkBinaryExists(agentDef.binaryName); + detection = await checkBinaryExists(agentDef.binaryName); if (detection.exists) { logger.info( `Agent "${agentDef.name}" found in PATH at: ${detection.path}`, - 'AgentDetector' + LOG_CONTEXT ); } } } else { - detection = await this.checkBinaryExists(agentDef.binaryName); + detection = await checkBinaryExists(agentDef.binaryName); if (detection.exists) { - logger.info(`Agent "${agentDef.name}" found at: ${detection.path}`, 'AgentDetector'); + logger.info(`Agent "${agentDef.name}" found at: ${detection.path}`, LOG_CONTEXT); } else if (agentDef.binaryName !== 'bash') { // Don't log bash as missing since it's always present, log others as warnings logger.warn( `Agent "${agentDef.name}" (binary: ${agentDef.binaryName}) not found. ` + `Searched in PATH: ${expandedEnv.PATH}`, - 'AgentDetector' + LOG_CONTEXT ); } } @@ -293,7 +144,7 @@ export class AgentDetector { // On Windows, log detailed path info to help debug shell execution issues if (isWindows) { - logger.info(`Agent detection complete (Windows)`, 'AgentDetector', { + logger.info(`Agent detection complete (Windows)`, LOG_CONTEXT, { platform: process.platform, agents: availableAgents.map((a) => ({ id: a.id, @@ -311,7 +162,7 @@ export class AgentDetector { } else { logger.info( `Agent detection complete. Available: ${availableAgents.map((a) => a.name).join(', ') || 'none'}`, - 'AgentDetector' + LOG_CONTEXT ); } @@ -319,442 +170,6 @@ export class AgentDetector { return agents; } - /** - * Check if a custom path points to a valid executable - * On Windows, also tries .cmd and .exe extensions if the path doesn't exist as-is - */ - private async checkCustomPath(customPath: string): Promise<{ exists: boolean; path?: string }> { - const isWindows = process.platform === 'win32'; - - // Expand tilde to home directory (Node.js fs doesn't understand ~) - const expandedPath = expandTilde(customPath); - - // Helper to check if a specific path exists and is a file - const checkPath = async (pathToCheck: string): Promise => { - try { - const stats = await fs.promises.stat(pathToCheck); - return stats.isFile(); - } catch { - return false; - } - }; - - try { - // First, try the exact path provided (with tilde expanded) - if (await checkPath(expandedPath)) { - // Check if file is executable (on Unix systems) - if (!isWindows) { - try { - await fs.promises.access(expandedPath, fs.constants.X_OK); - } catch { - logger.warn(`Custom path exists but is not executable: ${customPath}`, 'AgentDetector'); - return { exists: false }; - } - } - // Return the expanded path so it can be used directly - return { exists: true, path: expandedPath }; - } - - // On Windows, if the exact path doesn't exist, try with .cmd and .exe extensions - if (isWindows) { - const lowerPath = expandedPath.toLowerCase(); - // Only try extensions if the path doesn't already have one - if (!lowerPath.endsWith('.cmd') && !lowerPath.endsWith('.exe')) { - // Try .exe first (preferred), then .cmd - const exePath = expandedPath + '.exe'; - if (await checkPath(exePath)) { - logger.debug(`Custom path resolved with .exe extension`, 'AgentDetector', { - original: customPath, - resolved: exePath, - }); - return { exists: true, path: exePath }; - } - - const cmdPath = expandedPath + '.cmd'; - if (await checkPath(cmdPath)) { - logger.debug(`Custom path resolved with .cmd extension`, 'AgentDetector', { - original: customPath, - resolved: cmdPath, - }); - return { exists: true, path: cmdPath }; - } - } - } - - return { exists: false }; - } catch { - return { exists: false }; - } - } - - /** - * Build an expanded PATH that includes common binary installation locations. - * This is necessary because packaged Electron apps don't inherit shell environment. - */ - private getExpandedEnv(): NodeJS.ProcessEnv { - const home = os.homedir(); - const env = { ...process.env }; - const isWindows = process.platform === 'win32'; - - // Platform-specific paths - let additionalPaths: string[]; - - if (isWindows) { - // Windows-specific paths - const appData = process.env.APPDATA || path.join(home, 'AppData', 'Roaming'); - const localAppData = process.env.LOCALAPPDATA || path.join(home, 'AppData', 'Local'); - const programFiles = process.env.ProgramFiles || 'C:\\Program Files'; - const programFilesX86 = process.env['ProgramFiles(x86)'] || 'C:\\Program Files (x86)'; - - additionalPaths = [ - // Claude Code PowerShell installer (irm https://claude.ai/install.ps1 | iex) - // This is the primary installation method - installs claude.exe to ~/.local/bin - path.join(home, '.local', 'bin'), - // Claude Code winget install (winget install --id Anthropic.ClaudeCode) - path.join(localAppData, 'Microsoft', 'WinGet', 'Links'), - path.join(programFiles, 'WinGet', 'Links'), - path.join(localAppData, 'Microsoft', 'WinGet', 'Packages'), - path.join(programFiles, 'WinGet', 'Packages'), - // npm global installs (Claude Code, Codex CLI, Gemini CLI) - path.join(appData, 'npm'), - path.join(localAppData, 'npm'), - // Claude Code CLI install location (npm global) - path.join(appData, 'npm', 'node_modules', '@anthropic-ai', 'claude-code', 'cli'), - // Codex CLI install location (npm global) - path.join(appData, 'npm', 'node_modules', '@openai', 'codex', 'bin'), - // User local programs - path.join(localAppData, 'Programs'), - path.join(localAppData, 'Microsoft', 'WindowsApps'), - // Python/pip user installs (for Aider) - path.join(appData, 'Python', 'Scripts'), - path.join(localAppData, 'Programs', 'Python', 'Python312', 'Scripts'), - path.join(localAppData, 'Programs', 'Python', 'Python311', 'Scripts'), - path.join(localAppData, 'Programs', 'Python', 'Python310', 'Scripts'), - // Git for Windows (provides bash, common tools) - path.join(programFiles, 'Git', 'cmd'), - path.join(programFiles, 'Git', 'bin'), - path.join(programFiles, 'Git', 'usr', 'bin'), - path.join(programFilesX86, 'Git', 'cmd'), - path.join(programFilesX86, 'Git', 'bin'), - // Node.js - path.join(programFiles, 'nodejs'), - path.join(localAppData, 'Programs', 'node'), - // Scoop package manager (OpenCode, other tools) - path.join(home, 'scoop', 'shims'), - path.join(home, 'scoop', 'apps', 'opencode', 'current'), - // Chocolatey (OpenCode, other tools) - path.join(process.env.ChocolateyInstall || 'C:\\ProgramData\\chocolatey', 'bin'), - // Go binaries (some tools installed via 'go install') - path.join(home, 'go', 'bin'), - // Windows system paths - path.join(process.env.SystemRoot || 'C:\\Windows', 'System32'), - path.join(process.env.SystemRoot || 'C:\\Windows'), - ]; - } else { - // Unix-like paths (macOS/Linux) - additionalPaths = [ - '/opt/homebrew/bin', // Homebrew on Apple Silicon - '/opt/homebrew/sbin', - '/usr/local/bin', // Homebrew on Intel, common install location - '/usr/local/sbin', - `${home}/.local/bin`, // User local installs (pip, etc.) - `${home}/.npm-global/bin`, // npm global with custom prefix - `${home}/bin`, // User bin directory - `${home}/.claude/local`, // Claude local install location - `${home}/.opencode/bin`, // OpenCode installer default location - '/usr/bin', - '/bin', - '/usr/sbin', - '/sbin', - ]; - } - - const currentPath = env.PATH || ''; - // Use platform-appropriate path delimiter - const pathParts = currentPath.split(path.delimiter); - - // Add paths that aren't already present - for (const p of additionalPaths) { - if (!pathParts.includes(p)) { - pathParts.unshift(p); - } - } - - env.PATH = pathParts.join(path.delimiter); - return env; - } - - /** - * On Windows, directly probe known installation paths for a binary. - * This is more reliable than `where` command which may fail in packaged Electron apps. - * Returns the first existing path found, preferring .exe over .cmd. - */ - private async probeWindowsPaths(binaryName: string): Promise { - const home = os.homedir(); - const appData = process.env.APPDATA || path.join(home, 'AppData', 'Roaming'); - const localAppData = process.env.LOCALAPPDATA || path.join(home, 'AppData', 'Local'); - const programFiles = process.env.ProgramFiles || 'C:\\Program Files'; - - // Define known installation paths for each binary, in priority order - // Prefer .exe (standalone installers) over .cmd (npm wrappers) - const knownPaths: Record = { - claude: [ - // PowerShell installer (primary method) - installs claude.exe - path.join(home, '.local', 'bin', 'claude.exe'), - // Winget installation - path.join(localAppData, 'Microsoft', 'WinGet', 'Links', 'claude.exe'), - path.join(programFiles, 'WinGet', 'Links', 'claude.exe'), - // npm global installation - creates .cmd wrapper - path.join(appData, 'npm', 'claude.cmd'), - path.join(localAppData, 'npm', 'claude.cmd'), - // WindowsApps (Microsoft Store style) - path.join(localAppData, 'Microsoft', 'WindowsApps', 'claude.exe'), - ], - codex: [ - // npm global installation (primary method for Codex) - path.join(appData, 'npm', 'codex.cmd'), - path.join(localAppData, 'npm', 'codex.cmd'), - // Possible standalone in future - path.join(home, '.local', 'bin', 'codex.exe'), - ], - opencode: [ - // Scoop installation (recommended for OpenCode) - path.join(home, 'scoop', 'shims', 'opencode.exe'), - path.join(home, 'scoop', 'apps', 'opencode', 'current', 'opencode.exe'), - // Chocolatey installation - path.join( - process.env.ChocolateyInstall || 'C:\\ProgramData\\chocolatey', - 'bin', - 'opencode.exe' - ), - // Go install - path.join(home, 'go', 'bin', 'opencode.exe'), - // npm (has known issues on Windows, but check anyway) - path.join(appData, 'npm', 'opencode.cmd'), - ], - gemini: [ - // npm global installation - path.join(appData, 'npm', 'gemini.cmd'), - path.join(localAppData, 'npm', 'gemini.cmd'), - ], - aider: [ - // pip installation - path.join(appData, 'Python', 'Scripts', 'aider.exe'), - path.join(localAppData, 'Programs', 'Python', 'Python312', 'Scripts', 'aider.exe'), - path.join(localAppData, 'Programs', 'Python', 'Python311', 'Scripts', 'aider.exe'), - path.join(localAppData, 'Programs', 'Python', 'Python310', 'Scripts', 'aider.exe'), - ], - }; - - const pathsToCheck = knownPaths[binaryName] || []; - - for (const probePath of pathsToCheck) { - try { - await fs.promises.access(probePath, fs.constants.F_OK); - logger.debug(`Direct probe found ${binaryName}`, 'AgentDetector', { path: probePath }); - return probePath; - } catch { - // Path doesn't exist, continue to next - } - } - - return null; - } - - /** - * On macOS/Linux, directly probe known installation paths for a binary. - * This is necessary because packaged Electron apps don't inherit shell aliases, - * and 'which' may fail to find binaries in non-standard locations. - * Returns the first existing executable path found. - */ - private async probeUnixPaths(binaryName: string): Promise { - const home = os.homedir(); - - // Get dynamic paths from Node version managers (nvm, fnm, volta, etc.) - const versionManagerPaths = detectNodeVersionManagerBinPaths(); - - // Define known installation paths for each binary, in priority order - const knownPaths: Record = { - claude: [ - // Claude Code default installation location (irm https://claude.ai/install.ps1 equivalent on macOS) - path.join(home, '.claude', 'local', 'claude'), - // User local bin (pip, manual installs) - path.join(home, '.local', 'bin', 'claude'), - // Homebrew on Apple Silicon - '/opt/homebrew/bin/claude', - // Homebrew on Intel Mac - '/usr/local/bin/claude', - // npm global with custom prefix - path.join(home, '.npm-global', 'bin', 'claude'), - // User bin directory - path.join(home, 'bin', 'claude'), - // Add paths from Node version managers (nvm, fnm, volta, etc.) - ...versionManagerPaths.map((p) => path.join(p, 'claude')), - ], - codex: [ - // User local bin - path.join(home, '.local', 'bin', 'codex'), - // Homebrew paths - '/opt/homebrew/bin/codex', - '/usr/local/bin/codex', - // npm global - path.join(home, '.npm-global', 'bin', 'codex'), - // Add paths from Node version managers (nvm, fnm, volta, etc.) - ...versionManagerPaths.map((p) => path.join(p, 'codex')), - ], - opencode: [ - // OpenCode installer default location - path.join(home, '.opencode', 'bin', 'opencode'), - // Go install location - path.join(home, 'go', 'bin', 'opencode'), - // User local bin - path.join(home, '.local', 'bin', 'opencode'), - // Homebrew paths - '/opt/homebrew/bin/opencode', - '/usr/local/bin/opencode', - // Add paths from Node version managers (nvm, fnm, volta, etc.) - ...versionManagerPaths.map((p) => path.join(p, 'opencode')), - ], - gemini: [ - // npm global paths - path.join(home, '.npm-global', 'bin', 'gemini'), - '/opt/homebrew/bin/gemini', - '/usr/local/bin/gemini', - // Add paths from Node version managers (nvm, fnm, volta, etc.) - ...versionManagerPaths.map((p) => path.join(p, 'gemini')), - ], - aider: [ - // pip installation - path.join(home, '.local', 'bin', 'aider'), - // Homebrew paths - '/opt/homebrew/bin/aider', - '/usr/local/bin/aider', - // Add paths from Node version managers (in case installed via npm) - ...versionManagerPaths.map((p) => path.join(p, 'aider')), - ], - }; - - const pathsToCheck = knownPaths[binaryName] || []; - - for (const probePath of pathsToCheck) { - try { - // Check both existence and executability - await fs.promises.access(probePath, fs.constants.F_OK | fs.constants.X_OK); - logger.debug(`Direct probe found ${binaryName}`, 'AgentDetector', { path: probePath }); - return probePath; - } catch { - // Path doesn't exist or isn't executable, continue to next - } - } - - return null; - } - - /** - * Check if a binary exists in PATH - * On Windows, this also handles .cmd and .exe extensions properly - */ - private async checkBinaryExists(binaryName: string): Promise<{ exists: boolean; path?: string }> { - const isWindows = process.platform === 'win32'; - - // First try direct file probing of known installation paths - // This is more reliable than which/where in packaged Electron apps - if (isWindows) { - const probedPath = await this.probeWindowsPaths(binaryName); - if (probedPath) { - return { exists: true, path: probedPath }; - } - logger.debug(`Direct probe failed for ${binaryName}, falling back to where`, 'AgentDetector'); - } else { - // macOS/Linux: probe known paths first - const probedPath = await this.probeUnixPaths(binaryName); - if (probedPath) { - return { exists: true, path: probedPath }; - } - logger.debug(`Direct probe failed for ${binaryName}, falling back to which`, 'AgentDetector'); - } - - try { - // Use 'which' on Unix-like systems, 'where' on Windows - const command = isWindows ? 'where' : 'which'; - - // Use expanded PATH to find binaries in common installation locations - // This is critical for packaged Electron apps which don't inherit shell env - const env = this.getExpandedEnv(); - const result = await execFileNoThrow(command, [binaryName], undefined, env); - - if (result.exitCode === 0 && result.stdout.trim()) { - // Get all matches (Windows 'where' can return multiple) - // Handle both Unix (\n) and Windows (\r\n) line endings - const matches = result.stdout - .trim() - .split(/\r?\n/) - .map((p) => p.trim()) - .filter((p) => p); - - if (process.platform === 'win32' && matches.length > 0) { - // On Windows, prefer .exe over .cmd over extensionless - // This helps with proper execution handling - const exeMatch = matches.find((p) => p.toLowerCase().endsWith('.exe')); - const cmdMatch = matches.find((p) => p.toLowerCase().endsWith('.cmd')); - - // Return the best match: .exe > .cmd > first result - let bestMatch = exeMatch || cmdMatch || matches[0]; - - // If the first match doesn't have an extension, check if .cmd or .exe version exists - // This handles cases where 'where' returns a path without extension - if ( - !bestMatch.toLowerCase().endsWith('.exe') && - !bestMatch.toLowerCase().endsWith('.cmd') - ) { - const cmdPath = bestMatch + '.cmd'; - const exePath = bestMatch + '.exe'; - - // Check if the .exe or .cmd version exists - try { - await fs.promises.access(exePath, fs.constants.F_OK); - bestMatch = exePath; - logger.debug(`Found .exe version of ${binaryName}`, 'AgentDetector', { - path: exePath, - }); - } catch { - try { - await fs.promises.access(cmdPath, fs.constants.F_OK); - bestMatch = cmdPath; - logger.debug(`Found .cmd version of ${binaryName}`, 'AgentDetector', { - path: cmdPath, - }); - } catch { - // Neither .exe nor .cmd exists, use the original path - } - } - } - - logger.debug(`Windows binary detection for ${binaryName}`, 'AgentDetector', { - allMatches: matches, - selectedMatch: bestMatch, - isCmd: bestMatch.toLowerCase().endsWith('.cmd'), - isExe: bestMatch.toLowerCase().endsWith('.exe'), - }); - - return { - exists: true, - path: bestMatch, - }; - } - - return { - exists: true, - path: matches[0], // First match for Unix - }; - } - - return { exists: false }; - } catch { - return { exists: false }; - } - } - /** * Get a specific agent by ID */ @@ -793,13 +208,13 @@ export class AgentDetector { const agent = await this.getAgent(agentId); if (!agent || !agent.available) { - logger.warn(`Cannot discover models: agent ${agentId} not available`, 'AgentDetector'); + logger.warn(`Cannot discover models: agent ${agentId} not available`, LOG_CONTEXT); return []; } // Check if agent supports model selection if (!agent.capabilities.supportsModelSelection) { - logger.debug(`Agent ${agentId} does not support model selection`, 'AgentDetector'); + logger.debug(`Agent ${agentId} does not support model selection`, LOG_CONTEXT); return []; } @@ -807,7 +222,7 @@ export class AgentDetector { if (!forceRefresh) { const cached = this.modelCache.get(agentId); if (cached && Date.now() - cached.timestamp < this.MODEL_CACHE_TTL_MS) { - logger.debug(`Returning cached models for ${agentId}`, 'AgentDetector'); + logger.debug(`Returning cached models for ${agentId}`, LOG_CONTEXT); return cached.models; } } @@ -826,7 +241,7 @@ export class AgentDetector { * Each agent may have a different way to list available models. */ private async runModelDiscovery(agentId: string, agent: AgentConfig): Promise { - const env = this.getExpandedEnv(); + const env = getExpandedEnv(); const command = agent.path || agent.command; // Agent-specific model discovery commands @@ -838,7 +253,7 @@ export class AgentDetector { if (result.exitCode !== 0) { logger.warn( `Model discovery failed for ${agentId}: exit code ${result.exitCode}`, - 'AgentDetector', + LOG_CONTEXT, { stderr: result.stderr } ); return []; @@ -850,7 +265,7 @@ export class AgentDetector { .map((line) => line.trim()) .filter((line) => line.length > 0); - logger.info(`Discovered ${models.length} models for ${agentId}`, 'AgentDetector', { + logger.info(`Discovered ${models.length} models for ${agentId}`, LOG_CONTEXT, { models, }); return models; @@ -858,7 +273,7 @@ export class AgentDetector { default: // For agents without model discovery implemented, return empty array - logger.debug(`No model discovery implemented for ${agentId}`, 'AgentDetector'); + logger.debug(`No model discovery implemented for ${agentId}`, LOG_CONTEXT); return []; } } diff --git a/src/main/path-prober.ts b/src/main/path-prober.ts new file mode 100644 index 000000000..a18aef3be --- /dev/null +++ b/src/main/path-prober.ts @@ -0,0 +1,489 @@ +/** + * Path Prober - Platform-specific binary detection + * + * Handles detection of agent binaries on Windows and Unix-like systems. + * Packaged Electron apps don't inherit shell environment, so we need to + * probe known installation paths directly. + * + * Separated from agent-detector.ts for better maintainability and testability. + */ + +import * as os from 'os'; +import * as fs from 'fs'; +import * as path from 'path'; +import { execFileNoThrow } from './utils/execFile'; +import { logger } from './utils/logger'; +import { expandTilde, detectNodeVersionManagerBinPaths } from '../shared/pathUtils'; + +const LOG_CONTEXT = 'PathProber'; + +// ============ Types ============ + +export interface BinaryDetectionResult { + exists: boolean; + path?: string; +} + +// ============ Environment Expansion ============ + +/** + * Build an expanded PATH that includes common binary installation locations. + * This is necessary because packaged Electron apps don't inherit shell environment. + */ +export function getExpandedEnv(): NodeJS.ProcessEnv { + const home = os.homedir(); + const env = { ...process.env }; + const isWindows = process.platform === 'win32'; + + // Platform-specific paths + let additionalPaths: string[]; + + if (isWindows) { + // Windows-specific paths + const appData = process.env.APPDATA || path.join(home, 'AppData', 'Roaming'); + const localAppData = process.env.LOCALAPPDATA || path.join(home, 'AppData', 'Local'); + const programFiles = process.env.ProgramFiles || 'C:\\Program Files'; + const programFilesX86 = process.env['ProgramFiles(x86)'] || 'C:\\Program Files (x86)'; + + additionalPaths = [ + // Claude Code PowerShell installer (irm https://claude.ai/install.ps1 | iex) + // This is the primary installation method - installs claude.exe to ~/.local/bin + path.join(home, '.local', 'bin'), + // Claude Code winget install (winget install --id Anthropic.ClaudeCode) + path.join(localAppData, 'Microsoft', 'WinGet', 'Links'), + path.join(programFiles, 'WinGet', 'Links'), + path.join(localAppData, 'Microsoft', 'WinGet', 'Packages'), + path.join(programFiles, 'WinGet', 'Packages'), + // npm global installs (Claude Code, Codex CLI, Gemini CLI) + path.join(appData, 'npm'), + path.join(localAppData, 'npm'), + // Claude Code CLI install location (npm global) + path.join(appData, 'npm', 'node_modules', '@anthropic-ai', 'claude-code', 'cli'), + // Codex CLI install location (npm global) + path.join(appData, 'npm', 'node_modules', '@openai', 'codex', 'bin'), + // User local programs + path.join(localAppData, 'Programs'), + path.join(localAppData, 'Microsoft', 'WindowsApps'), + // Python/pip user installs (for Aider) + path.join(appData, 'Python', 'Scripts'), + path.join(localAppData, 'Programs', 'Python', 'Python312', 'Scripts'), + path.join(localAppData, 'Programs', 'Python', 'Python311', 'Scripts'), + path.join(localAppData, 'Programs', 'Python', 'Python310', 'Scripts'), + // Git for Windows (provides bash, common tools) + path.join(programFiles, 'Git', 'cmd'), + path.join(programFiles, 'Git', 'bin'), + path.join(programFiles, 'Git', 'usr', 'bin'), + path.join(programFilesX86, 'Git', 'cmd'), + path.join(programFilesX86, 'Git', 'bin'), + // Node.js + path.join(programFiles, 'nodejs'), + path.join(localAppData, 'Programs', 'node'), + // Scoop package manager (OpenCode, other tools) + path.join(home, 'scoop', 'shims'), + path.join(home, 'scoop', 'apps', 'opencode', 'current'), + // Chocolatey (OpenCode, other tools) + path.join(process.env.ChocolateyInstall || 'C:\\ProgramData\\chocolatey', 'bin'), + // Go binaries (some tools installed via 'go install') + path.join(home, 'go', 'bin'), + // Windows system paths + path.join(process.env.SystemRoot || 'C:\\Windows', 'System32'), + path.join(process.env.SystemRoot || 'C:\\Windows'), + ]; + } else { + // Unix-like paths (macOS/Linux) + additionalPaths = [ + '/opt/homebrew/bin', // Homebrew on Apple Silicon + '/opt/homebrew/sbin', + '/usr/local/bin', // Homebrew on Intel, common install location + '/usr/local/sbin', + `${home}/.local/bin`, // User local installs (pip, etc.) + `${home}/.npm-global/bin`, // npm global with custom prefix + `${home}/bin`, // User bin directory + `${home}/.claude/local`, // Claude local install location + `${home}/.opencode/bin`, // OpenCode installer default location + '/usr/bin', + '/bin', + '/usr/sbin', + '/sbin', + ]; + } + + const currentPath = env.PATH || ''; + // Use platform-appropriate path delimiter + const pathParts = currentPath.split(path.delimiter); + + // Add paths that aren't already present + for (const p of additionalPaths) { + if (!pathParts.includes(p)) { + pathParts.unshift(p); + } + } + + env.PATH = pathParts.join(path.delimiter); + return env; +} + +// ============ Custom Path Validation ============ + +/** + * Check if a custom path points to a valid executable + * On Windows, also tries .cmd and .exe extensions if the path doesn't exist as-is + */ +export async function checkCustomPath(customPath: string): Promise { + const isWindows = process.platform === 'win32'; + + // Expand tilde to home directory (Node.js fs doesn't understand ~) + const expandedPath = expandTilde(customPath); + + // Helper to check if a specific path exists and is a file + const checkPath = async (pathToCheck: string): Promise => { + try { + const stats = await fs.promises.stat(pathToCheck); + return stats.isFile(); + } catch { + return false; + } + }; + + try { + // First, try the exact path provided (with tilde expanded) + if (await checkPath(expandedPath)) { + // Check if file is executable (on Unix systems) + if (!isWindows) { + try { + await fs.promises.access(expandedPath, fs.constants.X_OK); + } catch { + logger.warn(`Custom path exists but is not executable: ${customPath}`, LOG_CONTEXT); + return { exists: false }; + } + } + // Return the expanded path so it can be used directly + return { exists: true, path: expandedPath }; + } + + // On Windows, if the exact path doesn't exist, try with .cmd and .exe extensions + if (isWindows) { + const lowerPath = expandedPath.toLowerCase(); + // Only try extensions if the path doesn't already have one + if (!lowerPath.endsWith('.cmd') && !lowerPath.endsWith('.exe')) { + // Try .exe first (preferred), then .cmd + const exePath = expandedPath + '.exe'; + if (await checkPath(exePath)) { + logger.debug(`Custom path resolved with .exe extension`, LOG_CONTEXT, { + original: customPath, + resolved: exePath, + }); + return { exists: true, path: exePath }; + } + + const cmdPath = expandedPath + '.cmd'; + if (await checkPath(cmdPath)) { + logger.debug(`Custom path resolved with .cmd extension`, LOG_CONTEXT, { + original: customPath, + resolved: cmdPath, + }); + return { exists: true, path: cmdPath }; + } + } + } + + return { exists: false }; + } catch { + return { exists: false }; + } +} + +// ============ Windows Path Probing ============ + +/** + * Known installation paths for binaries on Windows + */ +function getWindowsKnownPaths(binaryName: string): string[] { + const home = os.homedir(); + const appData = process.env.APPDATA || path.join(home, 'AppData', 'Roaming'); + const localAppData = process.env.LOCALAPPDATA || path.join(home, 'AppData', 'Local'); + const programFiles = process.env.ProgramFiles || 'C:\\Program Files'; + + // Define known installation paths for each binary, in priority order + // Prefer .exe (standalone installers) over .cmd (npm wrappers) + const knownPaths: Record = { + claude: [ + // PowerShell installer (primary method) - installs claude.exe + path.join(home, '.local', 'bin', 'claude.exe'), + // Winget installation + path.join(localAppData, 'Microsoft', 'WinGet', 'Links', 'claude.exe'), + path.join(programFiles, 'WinGet', 'Links', 'claude.exe'), + // npm global installation - creates .cmd wrapper + path.join(appData, 'npm', 'claude.cmd'), + path.join(localAppData, 'npm', 'claude.cmd'), + // WindowsApps (Microsoft Store style) + path.join(localAppData, 'Microsoft', 'WindowsApps', 'claude.exe'), + ], + codex: [ + // npm global installation (primary method for Codex) + path.join(appData, 'npm', 'codex.cmd'), + path.join(localAppData, 'npm', 'codex.cmd'), + // Possible standalone in future + path.join(home, '.local', 'bin', 'codex.exe'), + ], + opencode: [ + // Scoop installation (recommended for OpenCode) + path.join(home, 'scoop', 'shims', 'opencode.exe'), + path.join(home, 'scoop', 'apps', 'opencode', 'current', 'opencode.exe'), + // Chocolatey installation + path.join( + process.env.ChocolateyInstall || 'C:\\ProgramData\\chocolatey', + 'bin', + 'opencode.exe' + ), + // Go install + path.join(home, 'go', 'bin', 'opencode.exe'), + // npm (has known issues on Windows, but check anyway) + path.join(appData, 'npm', 'opencode.cmd'), + ], + gemini: [ + // npm global installation + path.join(appData, 'npm', 'gemini.cmd'), + path.join(localAppData, 'npm', 'gemini.cmd'), + ], + aider: [ + // pip installation + path.join(appData, 'Python', 'Scripts', 'aider.exe'), + path.join(localAppData, 'Programs', 'Python', 'Python312', 'Scripts', 'aider.exe'), + path.join(localAppData, 'Programs', 'Python', 'Python311', 'Scripts', 'aider.exe'), + path.join(localAppData, 'Programs', 'Python', 'Python310', 'Scripts', 'aider.exe'), + ], + }; + + return knownPaths[binaryName] || []; +} + +/** + * On Windows, directly probe known installation paths for a binary. + * This is more reliable than `where` command which may fail in packaged Electron apps. + * Returns the first existing path found, preferring .exe over .cmd. + */ +export async function probeWindowsPaths(binaryName: string): Promise { + const pathsToCheck = getWindowsKnownPaths(binaryName); + + for (const probePath of pathsToCheck) { + try { + await fs.promises.access(probePath, fs.constants.F_OK); + logger.debug(`Direct probe found ${binaryName}`, LOG_CONTEXT, { path: probePath }); + return probePath; + } catch { + // Path doesn't exist, continue to next + } + } + + return null; +} + +// ============ Unix Path Probing ============ + +/** + * Known installation paths for binaries on Unix-like systems + */ +function getUnixKnownPaths(binaryName: string): string[] { + const home = os.homedir(); + + // Get dynamic paths from Node version managers (nvm, fnm, volta, etc.) + const versionManagerPaths = detectNodeVersionManagerBinPaths(); + + // Define known installation paths for each binary, in priority order + const knownPaths: Record = { + claude: [ + // Claude Code default installation location (irm https://claude.ai/install.ps1 equivalent on macOS) + path.join(home, '.claude', 'local', 'claude'), + // User local bin (pip, manual installs) + path.join(home, '.local', 'bin', 'claude'), + // Homebrew on Apple Silicon + '/opt/homebrew/bin/claude', + // Homebrew on Intel Mac + '/usr/local/bin/claude', + // npm global with custom prefix + path.join(home, '.npm-global', 'bin', 'claude'), + // User bin directory + path.join(home, 'bin', 'claude'), + // Add paths from Node version managers (nvm, fnm, volta, etc.) + ...versionManagerPaths.map((p) => path.join(p, 'claude')), + ], + codex: [ + // User local bin + path.join(home, '.local', 'bin', 'codex'), + // Homebrew paths + '/opt/homebrew/bin/codex', + '/usr/local/bin/codex', + // npm global + path.join(home, '.npm-global', 'bin', 'codex'), + // Add paths from Node version managers (nvm, fnm, volta, etc.) + ...versionManagerPaths.map((p) => path.join(p, 'codex')), + ], + opencode: [ + // OpenCode installer default location + path.join(home, '.opencode', 'bin', 'opencode'), + // Go install location + path.join(home, 'go', 'bin', 'opencode'), + // User local bin + path.join(home, '.local', 'bin', 'opencode'), + // Homebrew paths + '/opt/homebrew/bin/opencode', + '/usr/local/bin/opencode', + // Add paths from Node version managers (nvm, fnm, volta, etc.) + ...versionManagerPaths.map((p) => path.join(p, 'opencode')), + ], + gemini: [ + // npm global paths + path.join(home, '.npm-global', 'bin', 'gemini'), + '/opt/homebrew/bin/gemini', + '/usr/local/bin/gemini', + // Add paths from Node version managers (nvm, fnm, volta, etc.) + ...versionManagerPaths.map((p) => path.join(p, 'gemini')), + ], + aider: [ + // pip installation + path.join(home, '.local', 'bin', 'aider'), + // Homebrew paths + '/opt/homebrew/bin/aider', + '/usr/local/bin/aider', + // Add paths from Node version managers (in case installed via npm) + ...versionManagerPaths.map((p) => path.join(p, 'aider')), + ], + }; + + return knownPaths[binaryName] || []; +} + +/** + * On macOS/Linux, directly probe known installation paths for a binary. + * This is necessary because packaged Electron apps don't inherit shell aliases, + * and 'which' may fail to find binaries in non-standard locations. + * Returns the first existing executable path found. + */ +export async function probeUnixPaths(binaryName: string): Promise { + const pathsToCheck = getUnixKnownPaths(binaryName); + + for (const probePath of pathsToCheck) { + try { + // Check both existence and executability + await fs.promises.access(probePath, fs.constants.F_OK | fs.constants.X_OK); + logger.debug(`Direct probe found ${binaryName}`, LOG_CONTEXT, { path: probePath }); + return probePath; + } catch { + // Path doesn't exist or isn't executable, continue to next + } + } + + return null; +} + +// ============ Binary Detection ============ + +/** + * Check if a binary exists in PATH or known installation locations. + * On Windows, this also handles .cmd and .exe extensions properly. + * + * Detection order: + * 1. Direct probe of known installation paths (most reliable) + * 2. Fall back to which/where command with expanded PATH + */ +export async function checkBinaryExists(binaryName: string): Promise { + const isWindows = process.platform === 'win32'; + + // First try direct file probing of known installation paths + // This is more reliable than which/where in packaged Electron apps + if (isWindows) { + const probedPath = await probeWindowsPaths(binaryName); + if (probedPath) { + return { exists: true, path: probedPath }; + } + logger.debug(`Direct probe failed for ${binaryName}, falling back to where`, LOG_CONTEXT); + } else { + // macOS/Linux: probe known paths first + const probedPath = await probeUnixPaths(binaryName); + if (probedPath) { + return { exists: true, path: probedPath }; + } + logger.debug(`Direct probe failed for ${binaryName}, falling back to which`, LOG_CONTEXT); + } + + try { + // Use 'which' on Unix-like systems, 'where' on Windows + const command = isWindows ? 'where' : 'which'; + + // Use expanded PATH to find binaries in common installation locations + // This is critical for packaged Electron apps which don't inherit shell env + const env = getExpandedEnv(); + const result = await execFileNoThrow(command, [binaryName], undefined, env); + + if (result.exitCode === 0 && result.stdout.trim()) { + // Get all matches (Windows 'where' can return multiple) + // Handle both Unix (\n) and Windows (\r\n) line endings + const matches = result.stdout + .trim() + .split(/\r?\n/) + .map((p) => p.trim()) + .filter((p) => p); + + if (process.platform === 'win32' && matches.length > 0) { + // On Windows, prefer .exe over .cmd over extensionless + // This helps with proper execution handling + const exeMatch = matches.find((p) => p.toLowerCase().endsWith('.exe')); + const cmdMatch = matches.find((p) => p.toLowerCase().endsWith('.cmd')); + + // Return the best match: .exe > .cmd > first result + let bestMatch = exeMatch || cmdMatch || matches[0]; + + // If the first match doesn't have an extension, check if .cmd or .exe version exists + // This handles cases where 'where' returns a path without extension + if ( + !bestMatch.toLowerCase().endsWith('.exe') && + !bestMatch.toLowerCase().endsWith('.cmd') + ) { + const cmdPath = bestMatch + '.cmd'; + const exePath = bestMatch + '.exe'; + + // Check if the .exe or .cmd version exists + try { + await fs.promises.access(exePath, fs.constants.F_OK); + bestMatch = exePath; + logger.debug(`Found .exe version of ${binaryName}`, LOG_CONTEXT, { + path: exePath, + }); + } catch { + try { + await fs.promises.access(cmdPath, fs.constants.F_OK); + bestMatch = cmdPath; + logger.debug(`Found .cmd version of ${binaryName}`, LOG_CONTEXT, { + path: cmdPath, + }); + } catch { + // Neither .exe nor .cmd exists, use the original path + } + } + } + + logger.debug(`Windows binary detection for ${binaryName}`, LOG_CONTEXT, { + allMatches: matches, + selectedMatch: bestMatch, + isCmd: bestMatch.toLowerCase().endsWith('.cmd'), + isExe: bestMatch.toLowerCase().endsWith('.exe'), + }); + + return { + exists: true, + path: bestMatch, + }; + } + + return { + exists: true, + path: matches[0], // First match for Unix + }; + } + + return { exists: false }; + } catch { + return { exists: false }; + } +} From 454cdefd44832a12fbb684c9fd58e7b39445350b Mon Sep 17 00:00:00 2001 From: Raza Rauf Date: Thu, 29 Jan 2026 22:20:58 +0500 Subject: [PATCH 2/4] refactor: consolidate agents module and reorganize test structure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Create src/main/agents/ directory with barrel exports: - Move agent-detector.ts → agents/detector.ts - Move agent-definitions.ts → agents/definitions.ts - Move agent-capabilities.ts → agents/capabilities.ts - Move agent-session-storage.ts → agents/session-storage.ts - Move path-prober.ts → agents/path-prober.ts - Add index.ts with centralized re-exports Reorganize tests to mirror source structure: - Move agent tests to __tests__/main/agents/ - Move process-listeners inline tests to __tests__/main/process-listeners/ - Move debug-package inline tests to __tests__/main/debug-package/ Add new tests for coverage gaps: - storage/claude-session-storage.test.ts (32 tests) - web-server/managers/CallbackRegistry.test.ts (29 tests) - web-server/managers/LiveSessionManager.test.ts (39 tests) Update all imports across codebase to use new agents/ module path. Test count: 16,201 → 16,301 (+100 tests) --- .../group-chat-integration.test.ts | 2 +- .../group-chat.integration.test.ts | 2 +- .../integration/provider-integration.test.ts | 2 +- .../capabilities.test.ts} | 2 +- .../definitions.test.ts} | 2 +- .../detector.test.ts} | 10 +- .../main/{ => agents}/path-prober.test.ts | 12 +- .../session-storage.test.ts} | 54 +- .../main/debug-package}/packager.test.ts | 2 +- .../main/debug-package}/sanitization.test.ts | 128 ++--- .../main/group-chat/group-chat-router.test.ts | 2 +- .../main/ipc/handlers/agentSessions.test.ts | 6 +- .../main/ipc/handlers/agents.test.ts | 6 +- src/__tests__/main/ipc/handlers/debug.test.ts | 2 +- .../process-listeners}/data-listener.test.ts | 8 +- .../process-listeners}/error-listener.test.ts | 8 +- .../process-listeners}/exit-listener.test.ts | 6 +- .../forwarding-listeners.test.ts | 6 +- .../session-id-listener.test.ts | 4 +- .../process-listeners}/stats-listener.test.ts | 12 +- .../process-listeners}/usage-listener.test.ts | 6 +- .../storage/claude-session-storage.test.ts | 416 +++++++++++++++ .../managers/CallbackRegistry.test.ts | 327 ++++++++++++ .../managers/LiveSessionManager.test.ts | 476 ++++++++++++++++++ .../capabilities.ts} | 0 .../definitions.ts} | 4 +- .../{agent-detector.ts => agents/detector.ts} | 24 +- src/main/agents/index.ts | 68 +++ src/main/{ => agents}/path-prober.ts | 8 +- .../session-storage.ts} | 4 +- src/main/debug-package/collectors/agents.ts | 2 +- src/main/debug-package/index.ts | 2 +- src/main/group-chat/group-chat-agent.ts | 2 +- src/main/group-chat/group-chat-router.ts | 2 +- src/main/index.ts | 2 +- src/main/ipc/handlers/agentSessions.ts | 8 +- src/main/ipc/handlers/agents.ts | 3 +- src/main/ipc/handlers/context.ts | 4 +- src/main/ipc/handlers/debug.ts | 2 +- src/main/ipc/handlers/groupChat.ts | 2 +- src/main/ipc/handlers/index.ts | 2 +- src/main/ipc/handlers/process.ts | 2 +- src/main/process-listeners/types.ts | 2 +- .../spawners/ChildProcessSpawner.ts | 2 +- src/main/storage/claude-session-storage.ts | 2 +- src/main/storage/codex-session-storage.ts | 2 +- src/main/storage/index.ts | 2 +- src/main/storage/opencode-session-storage.ts | 2 +- src/main/utils/agent-args.ts | 2 +- src/main/utils/context-groomer.ts | 2 +- 50 files changed, 1464 insertions(+), 194 deletions(-) rename src/__tests__/main/{agent-capabilities.test.ts => agents/capabilities.test.ts} (99%) rename src/__tests__/main/{agent-definitions.test.ts => agents/definitions.test.ts} (99%) rename src/__tests__/main/{agent-detector.test.ts => agents/detector.test.ts} (99%) rename src/__tests__/main/{ => agents}/path-prober.test.ts (97%) rename src/__tests__/main/{agent-session-storage.test.ts => agents/session-storage.test.ts} (85%) rename src/{main/debug-package/__tests__ => __tests__/main/debug-package}/packager.test.ts (99%) rename src/{main/debug-package/__tests__ => __tests__/main/debug-package}/sanitization.test.ts (81%) rename src/{main/process-listeners/__tests__ => __tests__/main/process-listeners}/data-listener.test.ts (96%) rename src/{main/process-listeners/__tests__ => __tests__/main/process-listeners}/error-listener.test.ts (91%) rename src/{main/process-listeners/__tests__ => __tests__/main/process-listeners}/exit-listener.test.ts (98%) rename src/{main/process-listeners/__tests__ => __tests__/main/process-listeners}/forwarding-listeners.test.ts (93%) rename src/{main/process-listeners/__tests__ => __tests__/main/process-listeners}/session-id-listener.test.ts (98%) rename src/{main/process-listeners/__tests__ => __tests__/main/process-listeners}/stats-listener.test.ts (93%) rename src/{main/process-listeners/__tests__ => __tests__/main/process-listeners}/usage-listener.test.ts (98%) create mode 100644 src/__tests__/main/storage/claude-session-storage.test.ts create mode 100644 src/__tests__/main/web-server/managers/CallbackRegistry.test.ts create mode 100644 src/__tests__/main/web-server/managers/LiveSessionManager.test.ts rename src/main/{agent-capabilities.ts => agents/capabilities.ts} (100%) rename src/main/{agent-definitions.ts => agents/definitions.ts} (98%) rename src/main/{agent-detector.ts => agents/detector.ts} (91%) create mode 100644 src/main/agents/index.ts rename src/main/{ => agents}/path-prober.ts (98%) rename src/main/{agent-session-storage.ts => agents/session-storage.ts} (98%) diff --git a/src/__tests__/integration/group-chat-integration.test.ts b/src/__tests__/integration/group-chat-integration.test.ts index b3dbb0432..236274493 100644 --- a/src/__tests__/integration/group-chat-integration.test.ts +++ b/src/__tests__/integration/group-chat-integration.test.ts @@ -26,7 +26,7 @@ import { describe, it, expect, beforeAll } from 'vitest'; import { spawn } from 'child_process'; import { promisify } from 'util'; import { exec } from 'child_process'; -import { getAgentCapabilities } from '../../main/agent-capabilities'; +import { getAgentCapabilities } from '../../main/agents'; const execAsync = promisify(exec); diff --git a/src/__tests__/integration/group-chat.integration.test.ts b/src/__tests__/integration/group-chat.integration.test.ts index b78c52975..14b402d0a 100644 --- a/src/__tests__/integration/group-chat.integration.test.ts +++ b/src/__tests__/integration/group-chat.integration.test.ts @@ -22,7 +22,7 @@ import { } from '../../main/group-chat/group-chat-moderator'; import { addParticipant } from '../../main/group-chat/group-chat-agent'; import { routeUserMessage } from '../../main/group-chat/group-chat-router'; -import { AgentDetector } from '../../main/agent-detector'; +import { AgentDetector } from '../../main/agents'; import { selectTestAgents, waitForAgentResponse, diff --git a/src/__tests__/integration/provider-integration.test.ts b/src/__tests__/integration/provider-integration.test.ts index 9ad30d0c3..76b0d7d7c 100644 --- a/src/__tests__/integration/provider-integration.test.ts +++ b/src/__tests__/integration/provider-integration.test.ts @@ -29,7 +29,7 @@ import { exec } from 'child_process'; import * as fs from 'fs'; import * as path from 'path'; import * as os from 'os'; -import { getAgentCapabilities } from '../../main/agent-capabilities'; +import { getAgentCapabilities } from '../../main/agents'; import { buildSshCommand, buildRemoteCommand } from '../../main/utils/ssh-command-builder'; import type { SshRemoteConfig } from '../../shared/types'; diff --git a/src/__tests__/main/agent-capabilities.test.ts b/src/__tests__/main/agents/capabilities.test.ts similarity index 99% rename from src/__tests__/main/agent-capabilities.test.ts rename to src/__tests__/main/agents/capabilities.test.ts index 50ab3648f..c199588a2 100644 --- a/src/__tests__/main/agent-capabilities.test.ts +++ b/src/__tests__/main/agents/capabilities.test.ts @@ -5,7 +5,7 @@ import { AGENT_CAPABILITIES, getAgentCapabilities, hasCapability, -} from '../../main/agent-capabilities'; +} from '../../../main/agents'; describe('agent-capabilities', () => { describe('AgentCapabilities interface', () => { diff --git a/src/__tests__/main/agent-definitions.test.ts b/src/__tests__/main/agents/definitions.test.ts similarity index 99% rename from src/__tests__/main/agent-definitions.test.ts rename to src/__tests__/main/agents/definitions.test.ts index a864bff23..04f4e1a44 100644 --- a/src/__tests__/main/agent-definitions.test.ts +++ b/src/__tests__/main/agents/definitions.test.ts @@ -12,7 +12,7 @@ import { getVisibleAgentDefinitions, type AgentDefinition, type AgentConfigOption, -} from '../../main/agent-definitions'; +} from '../../../main/agents'; describe('agent-definitions', () => { describe('AGENT_DEFINITIONS', () => { diff --git a/src/__tests__/main/agent-detector.test.ts b/src/__tests__/main/agents/detector.test.ts similarity index 99% rename from src/__tests__/main/agent-detector.test.ts rename to src/__tests__/main/agents/detector.test.ts index 6965ee78d..d2d189102 100644 --- a/src/__tests__/main/agent-detector.test.ts +++ b/src/__tests__/main/agents/detector.test.ts @@ -4,14 +4,14 @@ import { AgentConfig, AgentConfigOption, AgentCapabilities, -} from '../../main/agent-detector'; +} from '../../../main/agents'; // Mock dependencies -vi.mock('../../main/utils/execFile', () => ({ +vi.mock('../../../main/utils/execFile', () => ({ execFileNoThrow: vi.fn(), })); -vi.mock('../../main/utils/logger', () => ({ +vi.mock('../../../main/utils/logger', () => ({ logger: { info: vi.fn(), warn: vi.fn(), @@ -21,8 +21,8 @@ vi.mock('../../main/utils/logger', () => ({ })); // Get mocked modules -import { execFileNoThrow } from '../../main/utils/execFile'; -import { logger } from '../../main/utils/logger'; +import { execFileNoThrow } from '../../../main/utils/execFile'; +import { logger } from '../../../main/utils/logger'; import * as fs from 'fs'; import * as os from 'os'; diff --git a/src/__tests__/main/path-prober.test.ts b/src/__tests__/main/agents/path-prober.test.ts similarity index 97% rename from src/__tests__/main/path-prober.test.ts rename to src/__tests__/main/agents/path-prober.test.ts index f05a7f160..816cc7a6f 100644 --- a/src/__tests__/main/path-prober.test.ts +++ b/src/__tests__/main/agents/path-prober.test.ts @@ -8,11 +8,11 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import * as fs from 'fs'; // Mock dependencies before importing the module -vi.mock('../../main/utils/execFile', () => ({ +vi.mock('../../../main/utils/execFile', () => ({ execFileNoThrow: vi.fn(), })); -vi.mock('../../main/utils/logger', () => ({ +vi.mock('../../../main/utils/logger', () => ({ logger: { info: vi.fn(), warn: vi.fn(), @@ -21,7 +21,7 @@ vi.mock('../../main/utils/logger', () => ({ }, })); -vi.mock('../../shared/pathUtils', () => ({ +vi.mock('../../../shared/pathUtils', () => ({ expandTilde: vi.fn((p: string) => p.replace(/^~/, '/Users/testuser')), detectNodeVersionManagerBinPaths: vi.fn(() => []), })); @@ -34,9 +34,9 @@ import { probeWindowsPaths, probeUnixPaths, type BinaryDetectionResult, -} from '../../main/path-prober'; -import { execFileNoThrow } from '../../main/utils/execFile'; -import { logger } from '../../main/utils/logger'; +} from '../../../main/agents'; +import { execFileNoThrow } from '../../../main/utils/execFile'; +import { logger } from '../../../main/utils/logger'; describe('path-prober', () => { beforeEach(() => { diff --git a/src/__tests__/main/agent-session-storage.test.ts b/src/__tests__/main/agents/session-storage.test.ts similarity index 85% rename from src/__tests__/main/agent-session-storage.test.ts rename to src/__tests__/main/agents/session-storage.test.ts index e8bf4de40..40efb1db6 100644 --- a/src/__tests__/main/agent-session-storage.test.ts +++ b/src/__tests__/main/agents/session-storage.test.ts @@ -1,4 +1,6 @@ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import type Store from 'electron-store'; +import type { ClaudeSessionOriginsData } from '../../../main/storage/claude-session-storage'; import { AgentSessionStorage, AgentSessionInfo, @@ -11,8 +13,8 @@ import { hasSessionStorage, getAllSessionStorages, clearStorageRegistry, -} from '../../main/agent-session-storage'; -import type { ToolType } from '../../shared/types'; +} from '../../../main/agents'; +import type { ToolType } from '../../../shared/types'; // Mock storage implementation for testing class MockSessionStorage implements AgentSessionStorage { @@ -198,12 +200,12 @@ describe('ClaudeSessionStorage', () => { // For now, we test that the class can be imported it('should be importable', async () => { // Dynamic import to test module loading - const { ClaudeSessionStorage } = await import('../../main/storage/claude-session-storage'); + const { ClaudeSessionStorage } = await import('../../../main/storage/claude-session-storage'); expect(ClaudeSessionStorage).toBeDefined(); }); it('should have claude-code as agentId', async () => { - const { ClaudeSessionStorage } = await import('../../main/storage/claude-session-storage'); + const { ClaudeSessionStorage } = await import('../../../main/storage/claude-session-storage'); // Create instance without store (it will create its own) // Note: In a real test, we'd mock electron-store @@ -214,18 +216,21 @@ describe('ClaudeSessionStorage', () => { describe('OpenCodeSessionStorage', () => { it('should be importable', async () => { - const { OpenCodeSessionStorage } = await import('../../main/storage/opencode-session-storage'); + const { OpenCodeSessionStorage } = + await import('../../../main/storage/opencode-session-storage'); expect(OpenCodeSessionStorage).toBeDefined(); }); it('should have opencode as agentId', async () => { - const { OpenCodeSessionStorage } = await import('../../main/storage/opencode-session-storage'); + const { OpenCodeSessionStorage } = + await import('../../../main/storage/opencode-session-storage'); const storage = new OpenCodeSessionStorage(); expect(storage.agentId).toBe('opencode'); }); it('should return empty results for non-existent projects', async () => { - const { OpenCodeSessionStorage } = await import('../../main/storage/opencode-session-storage'); + const { OpenCodeSessionStorage } = + await import('../../../main/storage/opencode-session-storage'); const storage = new OpenCodeSessionStorage(); // Non-existent project should return empty results @@ -245,7 +250,8 @@ describe('OpenCodeSessionStorage', () => { }); it('should return message directory path for getSessionPath', async () => { - const { OpenCodeSessionStorage } = await import('../../main/storage/opencode-session-storage'); + const { OpenCodeSessionStorage } = + await import('../../../main/storage/opencode-session-storage'); const storage = new OpenCodeSessionStorage(); // getSessionPath returns the message directory for the session @@ -257,7 +263,8 @@ describe('OpenCodeSessionStorage', () => { }); it('should fail gracefully when deleting from non-existent session', async () => { - const { OpenCodeSessionStorage } = await import('../../main/storage/opencode-session-storage'); + const { OpenCodeSessionStorage } = + await import('../../../main/storage/opencode-session-storage'); const storage = new OpenCodeSessionStorage(); const deleteResult = await storage.deleteMessagePair( @@ -272,18 +279,18 @@ describe('OpenCodeSessionStorage', () => { describe('CodexSessionStorage', () => { it('should be importable', async () => { - const { CodexSessionStorage } = await import('../../main/storage/codex-session-storage'); + const { CodexSessionStorage } = await import('../../../main/storage/codex-session-storage'); expect(CodexSessionStorage).toBeDefined(); }); it('should have codex as agentId', async () => { - const { CodexSessionStorage } = await import('../../main/storage/codex-session-storage'); + const { CodexSessionStorage } = await import('../../../main/storage/codex-session-storage'); const storage = new CodexSessionStorage(); expect(storage.agentId).toBe('codex'); }); it('should return empty results for non-existent sessions directory', async () => { - const { CodexSessionStorage } = await import('../../main/storage/codex-session-storage'); + const { CodexSessionStorage } = await import('../../../main/storage/codex-session-storage'); const storage = new CodexSessionStorage(); // Non-existent project should return empty results (since ~/.codex/sessions/ likely doesn't exist in test) @@ -306,7 +313,7 @@ describe('CodexSessionStorage', () => { }); it('should return null for getSessionPath (async operation required)', async () => { - const { CodexSessionStorage } = await import('../../main/storage/codex-session-storage'); + const { CodexSessionStorage } = await import('../../../main/storage/codex-session-storage'); const storage = new CodexSessionStorage(); // getSessionPath is synchronous and always returns null for Codex @@ -316,7 +323,7 @@ describe('CodexSessionStorage', () => { }); it('should fail gracefully when deleting from non-existent session', async () => { - const { CodexSessionStorage } = await import('../../main/storage/codex-session-storage'); + const { CodexSessionStorage } = await import('../../../main/storage/codex-session-storage'); const storage = new CodexSessionStorage(); const deleteResult = await storage.deleteMessagePair( @@ -329,7 +336,7 @@ describe('CodexSessionStorage', () => { }); it('should handle empty search query', async () => { - const { CodexSessionStorage } = await import('../../main/storage/codex-session-storage'); + const { CodexSessionStorage } = await import('../../../main/storage/codex-session-storage'); const storage = new CodexSessionStorage(); const search = await storage.searchSessions('/test/project', '', 'all'); @@ -342,12 +349,12 @@ describe('CodexSessionStorage', () => { describe('Storage Module Initialization', () => { it('should export initializeSessionStorages function', async () => { - const { initializeSessionStorages } = await import('../../main/storage/index'); + const { initializeSessionStorages } = await import('../../../main/storage/index'); expect(typeof initializeSessionStorages).toBe('function'); }); it('should export CodexSessionStorage', async () => { - const { CodexSessionStorage } = await import('../../main/storage/index'); + const { CodexSessionStorage } = await import('../../../main/storage/index'); expect(CodexSessionStorage).toBeDefined(); }); @@ -355,7 +362,7 @@ describe('Storage Module Initialization', () => { // This tests that ClaudeSessionStorage can receive an external store // This prevents the dual-store bug where IPC handlers and storage class // use different electron-store instances - const { ClaudeSessionStorage } = await import('../../main/storage/claude-session-storage'); + const { ClaudeSessionStorage } = await import('../../../main/storage/claude-session-storage'); // Create a mock store const mockStore = { @@ -366,14 +373,14 @@ describe('Storage Module Initialization', () => { // Should be able to create with external store (no throw) const storage = new ClaudeSessionStorage( - mockStore as unknown as import('electron-store').default + mockStore as unknown as Store ); expect(storage.agentId).toBe('claude-code'); }); it('should export InitializeSessionStoragesOptions interface', async () => { // This tests that the options interface is exported for type-safe initialization - const storageModule = await import('../../main/storage/index'); + const storageModule = await import('../../../main/storage/index'); // The function should accept options object expect(typeof storageModule.initializeSessionStorages).toBe('function'); // Function should accept undefined options (backward compatible) @@ -383,9 +390,8 @@ describe('Storage Module Initialization', () => { it('should accept claudeSessionOriginsStore in options', async () => { // This tests the fix for the dual-store bug // When a shared store is passed, it should be used instead of creating a new one - const { initializeSessionStorages } = await import('../../main/storage/index'); - const { getSessionStorage, clearStorageRegistry } = - await import('../../main/agent-session-storage'); + const { initializeSessionStorages } = await import('../../../main/storage/index'); + const { getSessionStorage, clearStorageRegistry } = await import('../../../main/agents'); // Clear registry first clearStorageRegistry(); @@ -402,7 +408,7 @@ describe('Storage Module Initialization', () => { // Initialize with the shared store // This mimics what main/index.ts does initializeSessionStorages({ - claudeSessionOriginsStore: mockStore as unknown as import('electron-store').default, + claudeSessionOriginsStore: mockStore as unknown as Store, }); // Verify ClaudeSessionStorage was registered diff --git a/src/main/debug-package/__tests__/packager.test.ts b/src/__tests__/main/debug-package/packager.test.ts similarity index 99% rename from src/main/debug-package/__tests__/packager.test.ts rename to src/__tests__/main/debug-package/packager.test.ts index 5f6d9b169..ced70dfa5 100644 --- a/src/main/debug-package/__tests__/packager.test.ts +++ b/src/__tests__/main/debug-package/packager.test.ts @@ -13,7 +13,7 @@ import { describe, it, expect, beforeEach, afterEach } from 'vitest'; import * as path from 'path'; -import { createZipPackage, PackageContents } from '../packager'; +import { createZipPackage, PackageContents } from '../../../main/debug-package/packager'; import AdmZip from 'adm-zip'; // Use the native node:fs module to avoid any vitest mocks diff --git a/src/main/debug-package/__tests__/sanitization.test.ts b/src/__tests__/main/debug-package/sanitization.test.ts similarity index 81% rename from src/main/debug-package/__tests__/sanitization.test.ts rename to src/__tests__/main/debug-package/sanitization.test.ts index 5e8993ff6..66dc38bdc 100644 --- a/src/main/debug-package/__tests__/sanitization.test.ts +++ b/src/__tests__/main/debug-package/sanitization.test.ts @@ -51,7 +51,7 @@ describe('Debug Package Sanitization', () => { describe('sanitizePath', () => { describe('home directory replacement', () => { it('should replace home directory with ~', async () => { - const { sanitizePath } = await import('../collectors/settings'); + const { sanitizePath } = await import('../../../main/debug-package/collectors/settings'); const homeDir = os.homedir(); const testPath = `${homeDir}/Projects/MyApp`; @@ -62,7 +62,7 @@ describe('Debug Package Sanitization', () => { }); it('should replace home directory at any position in path', async () => { - const { sanitizePath } = await import('../collectors/settings'); + const { sanitizePath } = await import('../../../main/debug-package/collectors/settings'); const homeDir = os.homedir(); const testPath = `${homeDir}/deeply/nested/folder/file.txt`; @@ -72,7 +72,7 @@ describe('Debug Package Sanitization', () => { }); it('should handle home directory with trailing slash', async () => { - const { sanitizePath } = await import('../collectors/settings'); + const { sanitizePath } = await import('../../../main/debug-package/collectors/settings'); const homeDir = os.homedir(); const testPath = `${homeDir}/`; @@ -82,7 +82,7 @@ describe('Debug Package Sanitization', () => { }); it('should handle path that is exactly the home directory', async () => { - const { sanitizePath } = await import('../collectors/settings'); + const { sanitizePath } = await import('../../../main/debug-package/collectors/settings'); const homeDir = os.homedir(); const result = sanitizePath(homeDir); @@ -91,7 +91,7 @@ describe('Debug Package Sanitization', () => { }); it('should not modify paths that do not contain home directory', async () => { - const { sanitizePath } = await import('../collectors/settings'); + const { sanitizePath } = await import('../../../main/debug-package/collectors/settings'); const testPath = '/usr/local/bin/app'; const result = sanitizePath(testPath); @@ -100,7 +100,7 @@ describe('Debug Package Sanitization', () => { }); it('should handle empty string', async () => { - const { sanitizePath } = await import('../collectors/settings'); + const { sanitizePath } = await import('../../../main/debug-package/collectors/settings'); const result = sanitizePath(''); @@ -110,7 +110,7 @@ describe('Debug Package Sanitization', () => { describe('Windows path handling', () => { it('should normalize backslashes to forward slashes', async () => { - const { sanitizePath } = await import('../collectors/settings'); + const { sanitizePath } = await import('../../../main/debug-package/collectors/settings'); const testPath = 'C:\\Users\\testuser\\Documents\\Project'; const result = sanitizePath(testPath); @@ -120,7 +120,8 @@ describe('Debug Package Sanitization', () => { }); it('should handle Windows-style home directory', async () => { - const { sanitizePath: _sanitizePath } = await import('../collectors/settings'); + const { sanitizePath: _sanitizePath } = + await import('../../../main/debug-package/collectors/settings'); // Mock homedir to return Windows-style path const originalHomedir = os.homedir(); @@ -128,7 +129,8 @@ describe('Debug Package Sanitization', () => { // Re-import to get fresh module with mocked homedir vi.resetModules(); - const { sanitizePath: freshSanitizePath } = await import('../collectors/settings'); + const { sanitizePath: freshSanitizePath } = + await import('../../../main/debug-package/collectors/settings'); const testPath = 'C:\\Users\\testuser\\Documents\\Project'; const result = freshSanitizePath(testPath); @@ -139,7 +141,7 @@ describe('Debug Package Sanitization', () => { }); it('should handle mixed slash styles', async () => { - const { sanitizePath } = await import('../collectors/settings'); + const { sanitizePath } = await import('../../../main/debug-package/collectors/settings'); const testPath = '/path/to\\mixed\\slashes/file.txt'; const result = sanitizePath(testPath); @@ -152,7 +154,7 @@ describe('Debug Package Sanitization', () => { describe('edge cases and type handling', () => { it('should return null when given null', async () => { - const { sanitizePath } = await import('../collectors/settings'); + const { sanitizePath } = await import('../../../main/debug-package/collectors/settings'); // @ts-expect-error - Testing runtime behavior with wrong type const result = sanitizePath(null); @@ -161,7 +163,7 @@ describe('Debug Package Sanitization', () => { }); it('should return undefined when given undefined', async () => { - const { sanitizePath } = await import('../collectors/settings'); + const { sanitizePath } = await import('../../../main/debug-package/collectors/settings'); // @ts-expect-error - Testing runtime behavior with wrong type const result = sanitizePath(undefined); @@ -170,7 +172,7 @@ describe('Debug Package Sanitization', () => { }); it('should return numbers unchanged', async () => { - const { sanitizePath } = await import('../collectors/settings'); + const { sanitizePath } = await import('../../../main/debug-package/collectors/settings'); // @ts-expect-error - Testing runtime behavior with wrong type const result = sanitizePath(12345); @@ -179,7 +181,7 @@ describe('Debug Package Sanitization', () => { }); it('should return objects unchanged', async () => { - const { sanitizePath } = await import('../collectors/settings'); + const { sanitizePath } = await import('../../../main/debug-package/collectors/settings'); const obj = { path: '/some/path' }; // @ts-expect-error - Testing runtime behavior with wrong type @@ -189,7 +191,7 @@ describe('Debug Package Sanitization', () => { }); it('should handle paths with spaces', async () => { - const { sanitizePath } = await import('../collectors/settings'); + const { sanitizePath } = await import('../../../main/debug-package/collectors/settings'); const homeDir = os.homedir(); const testPath = `${homeDir}/My Documents/Project Files/app.tsx`; @@ -199,7 +201,7 @@ describe('Debug Package Sanitization', () => { }); it('should handle paths with special characters', async () => { - const { sanitizePath } = await import('../collectors/settings'); + const { sanitizePath } = await import('../../../main/debug-package/collectors/settings'); const homeDir = os.homedir(); const testPath = `${homeDir}/Projects/@company/app-v2.0#beta`; @@ -209,7 +211,7 @@ describe('Debug Package Sanitization', () => { }); it('should handle very long paths', async () => { - const { sanitizePath } = await import('../collectors/settings'); + const { sanitizePath } = await import('../../../main/debug-package/collectors/settings'); const homeDir = os.homedir(); const longPath = `${homeDir}/` + 'a/'.repeat(100) + 'file.txt'; @@ -228,7 +230,7 @@ describe('Debug Package Sanitization', () => { describe('API key redaction', () => { describe('sensitive key detection', () => { it('should redact apiKey', async () => { - const { collectSettings } = await import('../collectors/settings'); + const { collectSettings } = await import('../../../main/debug-package/collectors/settings'); const mockStore = { get: vi.fn(), set: vi.fn(), @@ -242,7 +244,7 @@ describe('Debug Package Sanitization', () => { }); it('should redact api_key (snake_case)', async () => { - const { collectSettings } = await import('../collectors/settings'); + const { collectSettings } = await import('../../../main/debug-package/collectors/settings'); const mockStore = { get: vi.fn(), set: vi.fn(), @@ -255,7 +257,7 @@ describe('Debug Package Sanitization', () => { }); it('should redact authToken', async () => { - const { collectSettings } = await import('../collectors/settings'); + const { collectSettings } = await import('../../../main/debug-package/collectors/settings'); const mockStore = { get: vi.fn(), set: vi.fn(), @@ -268,7 +270,7 @@ describe('Debug Package Sanitization', () => { }); it('should redact auth_token (snake_case)', async () => { - const { collectSettings } = await import('../collectors/settings'); + const { collectSettings } = await import('../../../main/debug-package/collectors/settings'); const mockStore = { get: vi.fn(), set: vi.fn(), @@ -281,7 +283,7 @@ describe('Debug Package Sanitization', () => { }); it('should redact clientToken', async () => { - const { collectSettings } = await import('../collectors/settings'); + const { collectSettings } = await import('../../../main/debug-package/collectors/settings'); const mockStore = { get: vi.fn(), set: vi.fn(), @@ -294,7 +296,7 @@ describe('Debug Package Sanitization', () => { }); it('should redact client_token (snake_case)', async () => { - const { collectSettings } = await import('../collectors/settings'); + const { collectSettings } = await import('../../../main/debug-package/collectors/settings'); const mockStore = { get: vi.fn(), set: vi.fn(), @@ -307,7 +309,7 @@ describe('Debug Package Sanitization', () => { }); it('should redact password', async () => { - const { collectSettings } = await import('../collectors/settings'); + const { collectSettings } = await import('../../../main/debug-package/collectors/settings'); const mockStore = { get: vi.fn(), set: vi.fn(), @@ -320,7 +322,7 @@ describe('Debug Package Sanitization', () => { }); it('should redact secret', async () => { - const { collectSettings } = await import('../collectors/settings'); + const { collectSettings } = await import('../../../main/debug-package/collectors/settings'); const mockStore = { get: vi.fn(), set: vi.fn(), @@ -333,7 +335,7 @@ describe('Debug Package Sanitization', () => { }); it('should redact credential', async () => { - const { collectSettings } = await import('../collectors/settings'); + const { collectSettings } = await import('../../../main/debug-package/collectors/settings'); const mockStore = { get: vi.fn(), set: vi.fn(), @@ -346,7 +348,7 @@ describe('Debug Package Sanitization', () => { }); it('should redact accessToken', async () => { - const { collectSettings } = await import('../collectors/settings'); + const { collectSettings } = await import('../../../main/debug-package/collectors/settings'); const mockStore = { get: vi.fn(), set: vi.fn(), @@ -359,7 +361,7 @@ describe('Debug Package Sanitization', () => { }); it('should redact access_token (snake_case)', async () => { - const { collectSettings } = await import('../collectors/settings'); + const { collectSettings } = await import('../../../main/debug-package/collectors/settings'); const mockStore = { get: vi.fn(), set: vi.fn(), @@ -372,7 +374,7 @@ describe('Debug Package Sanitization', () => { }); it('should redact refreshToken', async () => { - const { collectSettings } = await import('../collectors/settings'); + const { collectSettings } = await import('../../../main/debug-package/collectors/settings'); const mockStore = { get: vi.fn(), set: vi.fn(), @@ -385,7 +387,7 @@ describe('Debug Package Sanitization', () => { }); it('should redact refresh_token (snake_case)', async () => { - const { collectSettings } = await import('../collectors/settings'); + const { collectSettings } = await import('../../../main/debug-package/collectors/settings'); const mockStore = { get: vi.fn(), set: vi.fn(), @@ -398,7 +400,7 @@ describe('Debug Package Sanitization', () => { }); it('should redact privateKey', async () => { - const { collectSettings } = await import('../collectors/settings'); + const { collectSettings } = await import('../../../main/debug-package/collectors/settings'); const mockStore = { get: vi.fn(), set: vi.fn(), @@ -411,7 +413,7 @@ describe('Debug Package Sanitization', () => { }); it('should redact private_key (snake_case)', async () => { - const { collectSettings } = await import('../collectors/settings'); + const { collectSettings } = await import('../../../main/debug-package/collectors/settings'); const mockStore = { get: vi.fn(), set: vi.fn(), @@ -426,7 +428,7 @@ describe('Debug Package Sanitization', () => { describe('case insensitivity', () => { it('should redact APIKEY (uppercase)', async () => { - const { collectSettings } = await import('../collectors/settings'); + const { collectSettings } = await import('../../../main/debug-package/collectors/settings'); const mockStore = { get: vi.fn(), set: vi.fn(), @@ -439,7 +441,7 @@ describe('Debug Package Sanitization', () => { }); it('should redact ApiKey (mixed case)', async () => { - const { collectSettings } = await import('../collectors/settings'); + const { collectSettings } = await import('../../../main/debug-package/collectors/settings'); const mockStore = { get: vi.fn(), set: vi.fn(), @@ -452,7 +454,7 @@ describe('Debug Package Sanitization', () => { }); it('should redact API_KEY (uppercase snake_case)', async () => { - const { collectSettings } = await import('../collectors/settings'); + const { collectSettings } = await import('../../../main/debug-package/collectors/settings'); const mockStore = { get: vi.fn(), set: vi.fn(), @@ -465,7 +467,7 @@ describe('Debug Package Sanitization', () => { }); it('should redact PASSWORD (uppercase)', async () => { - const { collectSettings } = await import('../collectors/settings'); + const { collectSettings } = await import('../../../main/debug-package/collectors/settings'); const mockStore = { get: vi.fn(), set: vi.fn(), @@ -478,7 +480,7 @@ describe('Debug Package Sanitization', () => { }); it('should redact Secret (capitalized)', async () => { - const { collectSettings } = await import('../collectors/settings'); + const { collectSettings } = await import('../../../main/debug-package/collectors/settings'); const mockStore = { get: vi.fn(), set: vi.fn(), @@ -493,7 +495,7 @@ describe('Debug Package Sanitization', () => { describe('key name patterns containing sensitive words', () => { it('should redact myApiKeyValue (key within name)', async () => { - const { collectSettings } = await import('../collectors/settings'); + const { collectSettings } = await import('../../../main/debug-package/collectors/settings'); const mockStore = { get: vi.fn(), set: vi.fn(), @@ -506,7 +508,7 @@ describe('Debug Package Sanitization', () => { }); it('should redact userPassword (password in name)', async () => { - const { collectSettings } = await import('../collectors/settings'); + const { collectSettings } = await import('../../../main/debug-package/collectors/settings'); const mockStore = { get: vi.fn(), set: vi.fn(), @@ -519,7 +521,7 @@ describe('Debug Package Sanitization', () => { }); it('should redact adminSecret (secret in name)', async () => { - const { collectSettings } = await import('../collectors/settings'); + const { collectSettings } = await import('../../../main/debug-package/collectors/settings'); const mockStore = { get: vi.fn(), set: vi.fn(), @@ -532,7 +534,7 @@ describe('Debug Package Sanitization', () => { }); it('should redact bearerAccessToken (accesstoken in name)', async () => { - const { collectSettings } = await import('../collectors/settings'); + const { collectSettings } = await import('../../../main/debug-package/collectors/settings'); const mockStore = { get: vi.fn(), set: vi.fn(), @@ -545,7 +547,7 @@ describe('Debug Package Sanitization', () => { }); it('should redact dbCredential (credential in name)', async () => { - const { collectSettings } = await import('../collectors/settings'); + const { collectSettings } = await import('../../../main/debug-package/collectors/settings'); const mockStore = { get: vi.fn(), set: vi.fn(), @@ -560,7 +562,7 @@ describe('Debug Package Sanitization', () => { describe('nested object handling', () => { it('should redact sensitive keys in nested objects', async () => { - const { collectSettings } = await import('../collectors/settings'); + const { collectSettings } = await import('../../../main/debug-package/collectors/settings'); const mockStore = { get: vi.fn(), set: vi.fn(), @@ -577,7 +579,7 @@ describe('Debug Package Sanitization', () => { }); it('should redact deeply nested sensitive keys', async () => { - const { collectSettings } = await import('../collectors/settings'); + const { collectSettings } = await import('../../../main/debug-package/collectors/settings'); const mockStore = { get: vi.fn(), set: vi.fn(), @@ -602,7 +604,7 @@ describe('Debug Package Sanitization', () => { }); it('should track sanitized fields with full path', async () => { - const { collectSettings } = await import('../collectors/settings'); + const { collectSettings } = await import('../../../main/debug-package/collectors/settings'); const mockStore = { get: vi.fn(), set: vi.fn(), @@ -621,7 +623,7 @@ describe('Debug Package Sanitization', () => { }); it('should redact multiple sensitive keys at different levels', async () => { - const { collectSettings } = await import('../collectors/settings'); + const { collectSettings } = await import('../../../main/debug-package/collectors/settings'); const mockStore = { get: vi.fn(), set: vi.fn(), @@ -646,7 +648,7 @@ describe('Debug Package Sanitization', () => { describe('array handling', () => { it('should process arrays containing objects with sensitive keys', async () => { - const { collectSettings } = await import('../collectors/settings'); + const { collectSettings } = await import('../../../main/debug-package/collectors/settings'); const mockStore = { get: vi.fn(), set: vi.fn(), @@ -667,7 +669,7 @@ describe('Debug Package Sanitization', () => { }); it('should handle empty arrays', async () => { - const { collectSettings } = await import('../collectors/settings'); + const { collectSettings } = await import('../../../main/debug-package/collectors/settings'); const mockStore = { get: vi.fn(), set: vi.fn(), @@ -680,7 +682,7 @@ describe('Debug Package Sanitization', () => { }); it('should handle arrays of primitives', async () => { - const { collectSettings } = await import('../collectors/settings'); + const { collectSettings } = await import('../../../main/debug-package/collectors/settings'); const mockStore = { get: vi.fn(), set: vi.fn(), @@ -695,7 +697,7 @@ describe('Debug Package Sanitization', () => { describe('preservation of non-sensitive data', () => { it('should preserve boolean values', async () => { - const { collectSettings } = await import('../collectors/settings'); + const { collectSettings } = await import('../../../main/debug-package/collectors/settings'); const mockStore = { get: vi.fn(), set: vi.fn(), @@ -709,7 +711,7 @@ describe('Debug Package Sanitization', () => { }); it('should preserve number values', async () => { - const { collectSettings } = await import('../collectors/settings'); + const { collectSettings } = await import('../../../main/debug-package/collectors/settings'); const mockStore = { get: vi.fn(), set: vi.fn(), @@ -724,7 +726,7 @@ describe('Debug Package Sanitization', () => { }); it('should preserve string values without sensitive keywords', async () => { - const { collectSettings } = await import('../collectors/settings'); + const { collectSettings } = await import('../../../main/debug-package/collectors/settings'); const mockStore = { get: vi.fn(), set: vi.fn(), @@ -739,7 +741,7 @@ describe('Debug Package Sanitization', () => { }); it('should preserve null values', async () => { - const { collectSettings } = await import('../collectors/settings'); + const { collectSettings } = await import('../../../main/debug-package/collectors/settings'); const mockStore = { get: vi.fn(), set: vi.fn(), @@ -760,7 +762,7 @@ describe('Debug Package Sanitization', () => { describe('environment variable filtering', () => { describe('custom env vars masking', () => { it('should not expose custom env var values in agents collector', async () => { - const { collectAgents } = await import('../collectors/agents'); + const { collectAgents } = await import('../../../main/debug-package/collectors/agents'); const mockAgentDetector = { detectAgents: vi.fn().mockResolvedValue([ @@ -786,7 +788,7 @@ describe('Debug Package Sanitization', () => { }); it('should indicate env vars are set without showing values', async () => { - const { collectAgents } = await import('../collectors/agents'); + const { collectAgents } = await import('../../../main/debug-package/collectors/agents'); const mockAgentDetector = { detectAgents: vi.fn().mockResolvedValue([ @@ -812,7 +814,7 @@ describe('Debug Package Sanitization', () => { describe('custom args masking', () => { it('should not expose custom args values containing secrets', async () => { - const { collectAgents } = await import('../collectors/agents'); + const { collectAgents } = await import('../../../main/debug-package/collectors/agents'); const mockAgentDetector = { detectAgents: vi.fn().mockResolvedValue([ @@ -836,7 +838,7 @@ describe('Debug Package Sanitization', () => { describe('path-based environment variables', () => { it('should sanitize custom path settings', async () => { - const { collectSettings } = await import('../collectors/settings'); + const { collectSettings } = await import('../../../main/debug-package/collectors/settings'); const homeDir = os.homedir(); const mockStore = { @@ -855,7 +857,7 @@ describe('Debug Package Sanitization', () => { }); it('should sanitize folderPath settings', async () => { - const { collectSettings } = await import('../collectors/settings'); + const { collectSettings } = await import('../../../main/debug-package/collectors/settings'); const homeDir = os.homedir(); const mockStore = { @@ -879,7 +881,7 @@ describe('Debug Package Sanitization', () => { describe('comprehensive sanitization', () => { it('should sanitize complex settings object with mixed sensitive data', async () => { - const { collectSettings } = await import('../collectors/settings'); + const { collectSettings } = await import('../../../main/debug-package/collectors/settings'); const homeDir = os.homedir(); const mockStore = { @@ -931,7 +933,7 @@ describe('Debug Package Sanitization', () => { }); it('should track all sanitized fields', async () => { - const { collectSettings } = await import('../collectors/settings'); + const { collectSettings } = await import('../../../main/debug-package/collectors/settings'); const homeDir = os.homedir(); const mockStore = { @@ -952,7 +954,7 @@ describe('Debug Package Sanitization', () => { }); it('should produce output that contains no home directory paths for recognized path keys', async () => { - const { collectSettings } = await import('../collectors/settings'); + const { collectSettings } = await import('../../../main/debug-package/collectors/settings'); const homeDir = os.homedir(); const mockStore = { @@ -980,7 +982,7 @@ describe('Debug Package Sanitization', () => { }); it('should not sanitize paths in array values (by design)', async () => { - const { collectSettings } = await import('../collectors/settings'); + const { collectSettings } = await import('../../../main/debug-package/collectors/settings'); const homeDir = os.homedir(); // Note: Arrays of string paths are NOT sanitized by design @@ -1002,7 +1004,7 @@ describe('Debug Package Sanitization', () => { }); it('should produce output that contains no API keys or secrets', async () => { - const { collectSettings } = await import('../collectors/settings'); + const { collectSettings } = await import('../../../main/debug-package/collectors/settings'); const secrets = [ 'sk-1234567890abcdef', diff --git a/src/__tests__/main/group-chat/group-chat-router.test.ts b/src/__tests__/main/group-chat/group-chat-router.test.ts index aadad061d..3c7d3b1bc 100644 --- a/src/__tests__/main/group-chat/group-chat-router.test.ts +++ b/src/__tests__/main/group-chat/group-chat-router.test.ts @@ -63,7 +63,7 @@ import { GroupChatParticipant, } from '../../../main/group-chat/group-chat-storage'; import { readLog } from '../../../main/group-chat/group-chat-log'; -import { AgentDetector } from '../../../main/agent-detector'; +import { AgentDetector } from '../../../main/agents'; describe('group-chat-router', () => { let mockProcessManager: IProcessManager; diff --git a/src/__tests__/main/ipc/handlers/agentSessions.test.ts b/src/__tests__/main/ipc/handlers/agentSessions.test.ts index 84b84b229..baccd997e 100644 --- a/src/__tests__/main/ipc/handlers/agentSessions.test.ts +++ b/src/__tests__/main/ipc/handlers/agentSessions.test.ts @@ -8,7 +8,7 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { ipcMain } from 'electron'; import { registerAgentSessionsHandlers } from '../../../../main/ipc/handlers/agentSessions'; -import * as agentSessionStorage from '../../../../main/agent-session-storage'; +import * as agentSessionStorage from '../../../../main/agents'; // Mock electron's ipcMain vi.mock('electron', () => ({ @@ -18,8 +18,8 @@ vi.mock('electron', () => ({ }, })); -// Mock the agent-session-storage module -vi.mock('../../../../main/agent-session-storage', () => ({ +// Mock the agents module (session storage exports) +vi.mock('../../../../main/agents', () => ({ getSessionStorage: vi.fn(), hasSessionStorage: vi.fn(), getAllSessionStorages: vi.fn(), diff --git a/src/__tests__/main/ipc/handlers/agents.test.ts b/src/__tests__/main/ipc/handlers/agents.test.ts index e463164ce..08081670b 100644 --- a/src/__tests__/main/ipc/handlers/agents.test.ts +++ b/src/__tests__/main/ipc/handlers/agents.test.ts @@ -10,7 +10,7 @@ import { registerAgentsHandlers, AgentsHandlerDependencies, } from '../../../../main/ipc/handlers/agents'; -import * as agentCapabilities from '../../../../main/agent-capabilities'; +import * as agentCapabilities from '../../../../main/agents'; // Mock electron's ipcMain vi.mock('electron', () => ({ @@ -20,8 +20,8 @@ vi.mock('electron', () => ({ }, })); -// Mock agent-capabilities module -vi.mock('../../../../main/agent-capabilities', () => ({ +// Mock agents module (capabilities exports) +vi.mock('../../../../main/agents', () => ({ getAgentCapabilities: vi.fn(), DEFAULT_CAPABILITIES: { supportsResume: false, diff --git a/src/__tests__/main/ipc/handlers/debug.test.ts b/src/__tests__/main/ipc/handlers/debug.test.ts index a882eb59c..fa1d1a741 100644 --- a/src/__tests__/main/ipc/handlers/debug.test.ts +++ b/src/__tests__/main/ipc/handlers/debug.test.ts @@ -13,7 +13,7 @@ import { DebugHandlerDependencies, } from '../../../../main/ipc/handlers/debug'; import * as debugPackage from '../../../../main/debug-package'; -import { AgentDetector } from '../../../../main/agent-detector'; +import { AgentDetector } from '../../../../main/agents'; import { ProcessManager } from '../../../../main/process-manager'; import { WebServer } from '../../../../main/web-server'; diff --git a/src/main/process-listeners/__tests__/data-listener.test.ts b/src/__tests__/main/process-listeners/data-listener.test.ts similarity index 96% rename from src/main/process-listeners/__tests__/data-listener.test.ts rename to src/__tests__/main/process-listeners/data-listener.test.ts index d3950d89a..71e0f8610 100644 --- a/src/main/process-listeners/__tests__/data-listener.test.ts +++ b/src/__tests__/main/process-listeners/data-listener.test.ts @@ -4,10 +4,10 @@ */ import { describe, it, expect, vi, beforeEach } from 'vitest'; -import { setupDataListener } from '../data-listener'; -import type { ProcessManager } from '../../process-manager'; -import type { SafeSendFn } from '../../utils/safe-send'; -import type { ProcessListenerDependencies } from '../types'; +import { setupDataListener } from '../../../main/process-listeners/data-listener'; +import type { ProcessManager } from '../../../main/process-manager'; +import type { SafeSendFn } from '../../../main/utils/safe-send'; +import type { ProcessListenerDependencies } from '../../../main/process-listeners/types'; describe('Data Listener', () => { let mockProcessManager: ProcessManager; diff --git a/src/main/process-listeners/__tests__/error-listener.test.ts b/src/__tests__/main/process-listeners/error-listener.test.ts similarity index 91% rename from src/main/process-listeners/__tests__/error-listener.test.ts rename to src/__tests__/main/process-listeners/error-listener.test.ts index 51c8754c0..80d89512d 100644 --- a/src/main/process-listeners/__tests__/error-listener.test.ts +++ b/src/__tests__/main/process-listeners/error-listener.test.ts @@ -4,11 +4,11 @@ */ import { describe, it, expect, vi, beforeEach } from 'vitest'; -import { setupErrorListener } from '../error-listener'; -import type { ProcessManager } from '../../process-manager'; -import type { SafeSendFn } from '../../utils/safe-send'; +import { setupErrorListener } from '../../../main/process-listeners/error-listener'; +import type { ProcessManager } from '../../../main/process-manager'; +import type { SafeSendFn } from '../../../main/utils/safe-send'; import type { AgentError } from '../../../shared/types'; -import type { ProcessListenerDependencies } from '../types'; +import type { ProcessListenerDependencies } from '../../../main/process-listeners/types'; describe('Error Listener', () => { let mockProcessManager: ProcessManager; diff --git a/src/main/process-listeners/__tests__/exit-listener.test.ts b/src/__tests__/main/process-listeners/exit-listener.test.ts similarity index 98% rename from src/main/process-listeners/__tests__/exit-listener.test.ts rename to src/__tests__/main/process-listeners/exit-listener.test.ts index 793e12333..1f96f1937 100644 --- a/src/main/process-listeners/__tests__/exit-listener.test.ts +++ b/src/__tests__/main/process-listeners/exit-listener.test.ts @@ -4,9 +4,9 @@ */ import { describe, it, expect, vi, beforeEach } from 'vitest'; -import { setupExitListener } from '../exit-listener'; -import type { ProcessManager } from '../../process-manager'; -import type { ProcessListenerDependencies } from '../types'; +import { setupExitListener } from '../../../main/process-listeners/exit-listener'; +import type { ProcessManager } from '../../../main/process-manager'; +import type { ProcessListenerDependencies } from '../../../main/process-listeners/types'; describe('Exit Listener', () => { let mockProcessManager: ProcessManager; diff --git a/src/main/process-listeners/__tests__/forwarding-listeners.test.ts b/src/__tests__/main/process-listeners/forwarding-listeners.test.ts similarity index 93% rename from src/main/process-listeners/__tests__/forwarding-listeners.test.ts rename to src/__tests__/main/process-listeners/forwarding-listeners.test.ts index 5fcbcf532..2975dac1f 100644 --- a/src/main/process-listeners/__tests__/forwarding-listeners.test.ts +++ b/src/__tests__/main/process-listeners/forwarding-listeners.test.ts @@ -4,9 +4,9 @@ */ import { describe, it, expect, vi, beforeEach } from 'vitest'; -import { setupForwardingListeners } from '../forwarding-listeners'; -import type { ProcessManager } from '../../process-manager'; -import type { SafeSendFn } from '../../utils/safe-send'; +import { setupForwardingListeners } from '../../../main/process-listeners/forwarding-listeners'; +import type { ProcessManager } from '../../../main/process-manager'; +import type { SafeSendFn } from '../../../main/utils/safe-send'; describe('Forwarding Listeners', () => { let mockProcessManager: ProcessManager; diff --git a/src/main/process-listeners/__tests__/session-id-listener.test.ts b/src/__tests__/main/process-listeners/session-id-listener.test.ts similarity index 98% rename from src/main/process-listeners/__tests__/session-id-listener.test.ts rename to src/__tests__/main/process-listeners/session-id-listener.test.ts index c0f6773c2..c166ac9de 100644 --- a/src/main/process-listeners/__tests__/session-id-listener.test.ts +++ b/src/__tests__/main/process-listeners/session-id-listener.test.ts @@ -4,8 +4,8 @@ */ import { describe, it, expect, vi, beforeEach } from 'vitest'; -import { setupSessionIdListener } from '../session-id-listener'; -import type { ProcessManager } from '../../process-manager'; +import { setupSessionIdListener } from '../../../main/process-listeners/session-id-listener'; +import type { ProcessManager } from '../../../main/process-manager'; describe('Session ID Listener', () => { let mockProcessManager: ProcessManager; diff --git a/src/main/process-listeners/__tests__/stats-listener.test.ts b/src/__tests__/main/process-listeners/stats-listener.test.ts similarity index 93% rename from src/main/process-listeners/__tests__/stats-listener.test.ts rename to src/__tests__/main/process-listeners/stats-listener.test.ts index ff2a0a301..c43ce9eb9 100644 --- a/src/main/process-listeners/__tests__/stats-listener.test.ts +++ b/src/__tests__/main/process-listeners/stats-listener.test.ts @@ -4,12 +4,12 @@ */ import { describe, it, expect, vi, beforeEach } from 'vitest'; -import { setupStatsListener } from '../stats-listener'; -import type { ProcessManager } from '../../process-manager'; -import type { SafeSendFn } from '../../utils/safe-send'; -import type { QueryCompleteData } from '../../process-manager/types'; -import type { StatsDB } from '../../stats-db'; -import type { ProcessListenerDependencies } from '../types'; +import { setupStatsListener } from '../../../main/process-listeners/stats-listener'; +import type { ProcessManager } from '../../../main/process-manager'; +import type { SafeSendFn } from '../../../main/utils/safe-send'; +import type { QueryCompleteData } from '../../../main/process-manager/types'; +import type { StatsDB } from '../../../main/stats-db'; +import type { ProcessListenerDependencies } from '../../../main/process-listeners/types'; describe('Stats Listener', () => { let mockProcessManager: ProcessManager; diff --git a/src/main/process-listeners/__tests__/usage-listener.test.ts b/src/__tests__/main/process-listeners/usage-listener.test.ts similarity index 98% rename from src/main/process-listeners/__tests__/usage-listener.test.ts rename to src/__tests__/main/process-listeners/usage-listener.test.ts index 16c52d61c..6a2df6a68 100644 --- a/src/main/process-listeners/__tests__/usage-listener.test.ts +++ b/src/__tests__/main/process-listeners/usage-listener.test.ts @@ -4,9 +4,9 @@ */ import { describe, it, expect, vi, beforeEach } from 'vitest'; -import { setupUsageListener } from '../usage-listener'; -import type { ProcessManager } from '../../process-manager'; -import type { UsageStats } from '../types'; +import { setupUsageListener } from '../../../main/process-listeners/usage-listener'; +import type { ProcessManager } from '../../../main/process-manager'; +import type { UsageStats } from '../../../main/process-listeners/types'; describe('Usage Listener', () => { let mockProcessManager: ProcessManager; diff --git a/src/__tests__/main/storage/claude-session-storage.test.ts b/src/__tests__/main/storage/claude-session-storage.test.ts new file mode 100644 index 000000000..b34496e88 --- /dev/null +++ b/src/__tests__/main/storage/claude-session-storage.test.ts @@ -0,0 +1,416 @@ +/** + * Tests for ClaudeSessionStorage + * + * Verifies: + * - Session origin registration and retrieval + * - Session naming and starring + * - Context usage tracking + * - Origin info attachment to sessions + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { ClaudeSessionStorage } from '../../../main/storage/claude-session-storage'; +import type { SshRemoteConfig } from '../../../shared/types'; +import type Store from 'electron-store'; +import type { ClaudeSessionOriginsData } from '../../../main/storage/claude-session-storage'; + +// Mock electron-store +const mockStoreData: Record = {}; +vi.mock('electron-store', () => { + return { + default: vi.fn().mockImplementation(() => ({ + get: vi.fn((key: string, defaultValue?: unknown) => { + return mockStoreData[key] ?? defaultValue; + }), + set: vi.fn((key: string, value: unknown) => { + mockStoreData[key] = value; + }), + store: mockStoreData, + })), + }; +}); + +// Mock logger +vi.mock('../../../main/utils/logger', () => ({ + logger: { + info: vi.fn(), + debug: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + }, +})); + +// Mock fs/promises +vi.mock('fs/promises', () => ({ + default: { + access: vi.fn(), + readdir: vi.fn(), + stat: vi.fn(), + readFile: vi.fn(), + writeFile: vi.fn(), + }, +})); + +// Mock remote-fs utilities +vi.mock('../../../main/utils/remote-fs', () => ({ + readDirRemote: vi.fn(), + readFileRemote: vi.fn(), + statRemote: vi.fn(), +})); + +// Mock statsCache +vi.mock('../../../main/utils/statsCache', () => ({ + encodeClaudeProjectPath: vi.fn((projectPath: string) => { + // Simple encoding for tests - replace / with - + return projectPath.replace(/\//g, '-').replace(/^-/, ''); + }), +})); + +// Mock pricing +vi.mock('../../../main/utils/pricing', () => ({ + calculateClaudeCost: vi.fn(() => 0.05), +})); + +describe('ClaudeSessionStorage', () => { + let storage: ClaudeSessionStorage; + let mockStore: { + get: ReturnType; + set: ReturnType; + store: Record; + }; + + beforeEach(() => { + vi.clearAllMocks(); + + // Reset mock store data + Object.keys(mockStoreData).forEach((key) => delete mockStoreData[key]); + mockStoreData['origins'] = {}; + + mockStore = { + get: vi.fn((key: string, defaultValue?: unknown) => { + return mockStoreData[key] ?? defaultValue; + }), + set: vi.fn((key: string, value: unknown) => { + mockStoreData[key] = value; + }), + store: mockStoreData, + }; + + // Create storage with mock store + storage = new ClaudeSessionStorage(mockStore as unknown as Store); + }); + + describe('Origin Management', () => { + describe('registerSessionOrigin', () => { + it('should register a user session origin', () => { + storage.registerSessionOrigin('/project/path', 'session-123', 'user'); + + const origins = storage.getSessionOrigins('/project/path'); + expect(origins['session-123']).toEqual({ origin: 'user' }); + }); + + it('should register an auto session origin', () => { + storage.registerSessionOrigin('/project/path', 'session-456', 'auto'); + + const origins = storage.getSessionOrigins('/project/path'); + expect(origins['session-456']).toEqual({ origin: 'auto' }); + }); + + it('should register origin with session name', () => { + storage.registerSessionOrigin('/project/path', 'session-789', 'user', 'My Session'); + + const origins = storage.getSessionOrigins('/project/path'); + expect(origins['session-789']).toEqual({ + origin: 'user', + sessionName: 'My Session', + }); + }); + + it('should handle multiple sessions for same project', () => { + storage.registerSessionOrigin('/project/path', 'session-1', 'user'); + storage.registerSessionOrigin('/project/path', 'session-2', 'auto'); + storage.registerSessionOrigin('/project/path', 'session-3', 'user', 'Named'); + + const origins = storage.getSessionOrigins('/project/path'); + expect(Object.keys(origins)).toHaveLength(3); + }); + + it('should handle multiple projects', () => { + storage.registerSessionOrigin('/project/a', 'session-a', 'user'); + storage.registerSessionOrigin('/project/b', 'session-b', 'auto'); + + expect(storage.getSessionOrigins('/project/a')['session-a']).toBeDefined(); + expect(storage.getSessionOrigins('/project/b')['session-b']).toBeDefined(); + expect(storage.getSessionOrigins('/project/a')['session-b']).toBeUndefined(); + }); + + it('should persist to store', () => { + storage.registerSessionOrigin('/project/path', 'session-123', 'user'); + + expect(mockStore.set).toHaveBeenCalledWith( + 'origins', + expect.objectContaining({ + '/project/path': expect.objectContaining({ + 'session-123': 'user', + }), + }) + ); + }); + }); + + describe('updateSessionName', () => { + it('should update name for existing session with string origin', () => { + storage.registerSessionOrigin('/project/path', 'session-123', 'user'); + storage.updateSessionName('/project/path', 'session-123', 'New Name'); + + const origins = storage.getSessionOrigins('/project/path'); + expect(origins['session-123']).toEqual({ + origin: 'user', + sessionName: 'New Name', + }); + }); + + it('should update name for existing session with object origin', () => { + storage.registerSessionOrigin('/project/path', 'session-123', 'user', 'Old Name'); + storage.updateSessionName('/project/path', 'session-123', 'New Name'); + + const origins = storage.getSessionOrigins('/project/path'); + expect(origins['session-123'].sessionName).toBe('New Name'); + }); + + it('should create origin entry if session not registered', () => { + storage.updateSessionName('/project/path', 'new-session', 'Session Name'); + + const origins = storage.getSessionOrigins('/project/path'); + expect(origins['new-session']).toEqual({ + origin: 'user', + sessionName: 'Session Name', + }); + }); + + it('should preserve existing starred status', () => { + storage.registerSessionOrigin('/project/path', 'session-123', 'user'); + storage.updateSessionStarred('/project/path', 'session-123', true); + storage.updateSessionName('/project/path', 'session-123', 'Named'); + + const origins = storage.getSessionOrigins('/project/path'); + expect(origins['session-123'].starred).toBe(true); + expect(origins['session-123'].sessionName).toBe('Named'); + }); + }); + + describe('updateSessionStarred', () => { + it('should star a session', () => { + storage.registerSessionOrigin('/project/path', 'session-123', 'user'); + storage.updateSessionStarred('/project/path', 'session-123', true); + + const origins = storage.getSessionOrigins('/project/path'); + expect(origins['session-123'].starred).toBe(true); + }); + + it('should unstar a session', () => { + storage.registerSessionOrigin('/project/path', 'session-123', 'user'); + storage.updateSessionStarred('/project/path', 'session-123', true); + storage.updateSessionStarred('/project/path', 'session-123', false); + + const origins = storage.getSessionOrigins('/project/path'); + expect(origins['session-123'].starred).toBe(false); + }); + + it('should create origin entry if session not registered', () => { + storage.updateSessionStarred('/project/path', 'new-session', true); + + const origins = storage.getSessionOrigins('/project/path'); + expect(origins['new-session']).toEqual({ + origin: 'user', + starred: true, + }); + }); + + it('should preserve existing session name', () => { + storage.registerSessionOrigin('/project/path', 'session-123', 'user', 'My Session'); + storage.updateSessionStarred('/project/path', 'session-123', true); + + const origins = storage.getSessionOrigins('/project/path'); + expect(origins['session-123'].sessionName).toBe('My Session'); + expect(origins['session-123'].starred).toBe(true); + }); + }); + + describe('updateSessionContextUsage', () => { + it('should store context usage percentage', () => { + storage.registerSessionOrigin('/project/path', 'session-123', 'user'); + storage.updateSessionContextUsage('/project/path', 'session-123', 75); + + const origins = storage.getSessionOrigins('/project/path'); + expect(origins['session-123'].contextUsage).toBe(75); + }); + + it('should create origin entry if session not registered', () => { + storage.updateSessionContextUsage('/project/path', 'new-session', 50); + + const origins = storage.getSessionOrigins('/project/path'); + expect(origins['new-session']).toEqual({ + origin: 'user', + contextUsage: 50, + }); + }); + + it('should preserve existing origin data', () => { + storage.registerSessionOrigin('/project/path', 'session-123', 'auto', 'Named'); + storage.updateSessionStarred('/project/path', 'session-123', true); + storage.updateSessionContextUsage('/project/path', 'session-123', 80); + + const origins = storage.getSessionOrigins('/project/path'); + expect(origins['session-123']).toEqual({ + origin: 'auto', + sessionName: 'Named', + starred: true, + contextUsage: 80, + }); + }); + + it('should update context usage on subsequent calls', () => { + storage.registerSessionOrigin('/project/path', 'session-123', 'user'); + storage.updateSessionContextUsage('/project/path', 'session-123', 25); + storage.updateSessionContextUsage('/project/path', 'session-123', 50); + storage.updateSessionContextUsage('/project/path', 'session-123', 75); + + const origins = storage.getSessionOrigins('/project/path'); + expect(origins['session-123'].contextUsage).toBe(75); + }); + }); + + describe('getSessionOrigins', () => { + it('should return empty object for project with no sessions', () => { + const origins = storage.getSessionOrigins('/nonexistent/project'); + expect(origins).toEqual({}); + }); + + it('should normalize string origins to SessionOriginInfo format', () => { + // Simulate legacy string-only origin stored directly + mockStoreData['origins'] = { + '/project/path': { + 'session-123': 'user', + }, + }; + + const origins = storage.getSessionOrigins('/project/path'); + expect(origins['session-123']).toEqual({ origin: 'user' }); + }); + + it('should return full SessionOriginInfo for object origins', () => { + storage.registerSessionOrigin('/project/path', 'session-123', 'user', 'Named'); + storage.updateSessionStarred('/project/path', 'session-123', true); + storage.updateSessionContextUsage('/project/path', 'session-123', 60); + + const origins = storage.getSessionOrigins('/project/path'); + expect(origins['session-123']).toEqual({ + origin: 'user', + sessionName: 'Named', + starred: true, + contextUsage: 60, + }); + }); + }); + }); + + describe('Session Path', () => { + describe('getSessionPath', () => { + it('should return correct local path', () => { + const sessionPath = storage.getSessionPath('/project/path', 'session-123'); + + expect(sessionPath).toBeDefined(); + expect(sessionPath).toContain('session-123.jsonl'); + expect(sessionPath).toContain('.claude'); + expect(sessionPath).toContain('projects'); + }); + + it('should return remote path when sshConfig provided', () => { + const sshConfig: SshRemoteConfig = { + id: 'test-remote', + name: 'Test Remote', + host: 'remote.example.com', + port: 22, + username: 'testuser', + privateKeyPath: '~/.ssh/id_rsa', + enabled: true, + useSshConfig: false, + }; + const sessionPath = storage.getSessionPath('/project/path', 'session-123', sshConfig); + + expect(sessionPath).toBeDefined(); + expect(sessionPath).toContain('session-123.jsonl'); + expect(sessionPath).toContain('~/.claude/projects'); + }); + }); + }); + + describe('Agent ID', () => { + it('should have correct agent ID', () => { + expect(storage.agentId).toBe('claude-code'); + }); + }); + + describe('Edge Cases', () => { + it('should handle special characters in project path', () => { + storage.registerSessionOrigin('/path/with spaces/and-dashes', 'session-1', 'user'); + + const origins = storage.getSessionOrigins('/path/with spaces/and-dashes'); + expect(origins['session-1']).toBeDefined(); + }); + + it('should handle special characters in session ID', () => { + storage.registerSessionOrigin('/project', 'session-with-dashes-123', 'user'); + storage.registerSessionOrigin('/project', 'session_with_underscores', 'auto'); + + const origins = storage.getSessionOrigins('/project'); + expect(origins['session-with-dashes-123']).toBeDefined(); + expect(origins['session_with_underscores']).toBeDefined(); + }); + + it('should handle empty session name', () => { + storage.registerSessionOrigin('/project', 'session-123', 'user', ''); + + const origins = storage.getSessionOrigins('/project'); + // Empty string is falsy, so sessionName is not stored when empty + expect(origins['session-123']).toEqual({ origin: 'user' }); + }); + + it('should handle zero context usage', () => { + storage.updateSessionContextUsage('/project', 'session-123', 0); + + const origins = storage.getSessionOrigins('/project'); + expect(origins['session-123'].contextUsage).toBe(0); + }); + + it('should handle 100% context usage', () => { + storage.updateSessionContextUsage('/project', 'session-123', 100); + + const origins = storage.getSessionOrigins('/project'); + expect(origins['session-123'].contextUsage).toBe(100); + }); + }); + + describe('Storage Persistence', () => { + it('should call store.set on every origin update', () => { + storage.registerSessionOrigin('/project', 'session-1', 'user'); + expect(mockStore.set).toHaveBeenCalledTimes(1); + + storage.updateSessionName('/project', 'session-1', 'Name'); + expect(mockStore.set).toHaveBeenCalledTimes(2); + + storage.updateSessionStarred('/project', 'session-1', true); + expect(mockStore.set).toHaveBeenCalledTimes(3); + + storage.updateSessionContextUsage('/project', 'session-1', 50); + expect(mockStore.set).toHaveBeenCalledTimes(4); + }); + + it('should always call store.set with origins key', () => { + storage.registerSessionOrigin('/project', 'session-1', 'user'); + + expect(mockStore.set).toHaveBeenCalledWith('origins', expect.any(Object)); + }); + }); +}); diff --git a/src/__tests__/main/web-server/managers/CallbackRegistry.test.ts b/src/__tests__/main/web-server/managers/CallbackRegistry.test.ts new file mode 100644 index 000000000..19d9bf9a0 --- /dev/null +++ b/src/__tests__/main/web-server/managers/CallbackRegistry.test.ts @@ -0,0 +1,327 @@ +/** + * Tests for CallbackRegistry + * + * Verifies: + * - Callback registration and retrieval + * - Default return values when callbacks not set + * - Proper delegation to registered callbacks + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { CallbackRegistry } from '../../../../main/web-server/managers/CallbackRegistry'; + +// Mock the logger +vi.mock('../../../../main/utils/logger', () => ({ + logger: { + info: vi.fn(), + debug: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + }, +})); + +describe('CallbackRegistry', () => { + let registry: CallbackRegistry; + + beforeEach(() => { + vi.clearAllMocks(); + registry = new CallbackRegistry(); + }); + + describe('Default Return Values', () => { + it('should return empty array for getSessions when no callback set', () => { + const result = registry.getSessions(); + expect(result).toEqual([]); + }); + + it('should return null for getSessionDetail when no callback set', () => { + const result = registry.getSessionDetail('session-123'); + expect(result).toBeNull(); + }); + + it('should return null for getTheme when no callback set', () => { + const result = registry.getTheme(); + expect(result).toBeNull(); + }); + + it('should return empty array for getCustomCommands when no callback set', () => { + const result = registry.getCustomCommands(); + expect(result).toEqual([]); + }); + + it('should return false for writeToSession when no callback set', () => { + const result = registry.writeToSession('session-123', 'data'); + expect(result).toBe(false); + }); + + it('should return false for executeCommand when no callback set', async () => { + const result = await registry.executeCommand('session-123', 'ls -la'); + expect(result).toBe(false); + }); + + it('should return false for interruptSession when no callback set', async () => { + const result = await registry.interruptSession('session-123'); + expect(result).toBe(false); + }); + + it('should return false for switchMode when no callback set', async () => { + const result = await registry.switchMode('session-123', 'ai'); + expect(result).toBe(false); + }); + + it('should return false for selectSession when no callback set', async () => { + const result = await registry.selectSession('session-123'); + expect(result).toBe(false); + }); + + it('should return false for selectTab when no callback set', async () => { + const result = await registry.selectTab('session-123', 'tab-1'); + expect(result).toBe(false); + }); + + it('should return null for newTab when no callback set', async () => { + const result = await registry.newTab('session-123'); + expect(result).toBeNull(); + }); + + it('should return false for closeTab when no callback set', async () => { + const result = await registry.closeTab('session-123', 'tab-1'); + expect(result).toBe(false); + }); + + it('should return false for renameTab when no callback set', async () => { + const result = await registry.renameTab('session-123', 'tab-1', 'New Name'); + expect(result).toBe(false); + }); + + it('should return empty array for getHistory when no callback set', () => { + const result = registry.getHistory(); + expect(result).toEqual([]); + }); + }); + + describe('Callback Registration and Execution', () => { + it('should call registered getSessions callback', () => { + const mockCallback = vi.fn().mockReturnValue([ + { id: 'session-1', name: 'Session 1' }, + { id: 'session-2', name: 'Session 2' }, + ]); + registry.setGetSessionsCallback(mockCallback); + + const result = registry.getSessions(); + + expect(mockCallback).toHaveBeenCalled(); + expect(result).toHaveLength(2); + expect(result[0].id).toBe('session-1'); + }); + + it('should call registered getSessionDetail callback with arguments', () => { + const mockCallback = vi.fn().mockReturnValue({ + id: 'session-123', + tabs: [{ id: 'tab-1' }], + }); + registry.setGetSessionDetailCallback(mockCallback); + + const result = registry.getSessionDetail('session-123', 'tab-1'); + + expect(mockCallback).toHaveBeenCalledWith('session-123', 'tab-1'); + expect(result?.id).toBe('session-123'); + }); + + it('should call registered getTheme callback', () => { + const mockCallback = vi.fn().mockReturnValue({ name: 'dark', colors: {} }); + registry.setGetThemeCallback(mockCallback); + + const result = registry.getTheme(); + + expect(mockCallback).toHaveBeenCalled(); + expect(result?.name).toBe('dark'); + }); + + it('should call registered getCustomCommands callback', () => { + const mockCallback = vi.fn().mockReturnValue([{ name: 'cmd1', command: 'echo 1' }]); + registry.setGetCustomCommandsCallback(mockCallback); + + const result = registry.getCustomCommands(); + + expect(mockCallback).toHaveBeenCalled(); + expect(result).toHaveLength(1); + }); + + it('should call registered writeToSession callback', () => { + const mockCallback = vi.fn().mockReturnValue(true); + registry.setWriteToSessionCallback(mockCallback); + + const result = registry.writeToSession('session-123', 'test data'); + + expect(mockCallback).toHaveBeenCalledWith('session-123', 'test data'); + expect(result).toBe(true); + }); + + it('should call registered executeCommand callback', async () => { + const mockCallback = vi.fn().mockResolvedValue(true); + registry.setExecuteCommandCallback(mockCallback); + + const result = await registry.executeCommand('session-123', 'ls -la', 'ai'); + + expect(mockCallback).toHaveBeenCalledWith('session-123', 'ls -la', 'ai'); + expect(result).toBe(true); + }); + + it('should call registered interruptSession callback', async () => { + const mockCallback = vi.fn().mockResolvedValue(true); + registry.setInterruptSessionCallback(mockCallback); + + const result = await registry.interruptSession('session-123'); + + expect(mockCallback).toHaveBeenCalledWith('session-123'); + expect(result).toBe(true); + }); + + it('should call registered switchMode callback', async () => { + const mockCallback = vi.fn().mockResolvedValue(true); + registry.setSwitchModeCallback(mockCallback); + + const result = await registry.switchMode('session-123', 'terminal'); + + expect(mockCallback).toHaveBeenCalledWith('session-123', 'terminal'); + expect(result).toBe(true); + }); + + it('should call registered selectSession callback', async () => { + const mockCallback = vi.fn().mockResolvedValue(true); + registry.setSelectSessionCallback(mockCallback); + + const result = await registry.selectSession('session-123', 'tab-1'); + + expect(mockCallback).toHaveBeenCalledWith('session-123', 'tab-1'); + expect(result).toBe(true); + }); + + it('should call registered selectTab callback', async () => { + const mockCallback = vi.fn().mockResolvedValue(true); + registry.setSelectTabCallback(mockCallback); + + const result = await registry.selectTab('session-123', 'tab-1'); + + expect(mockCallback).toHaveBeenCalledWith('session-123', 'tab-1'); + expect(result).toBe(true); + }); + + it('should call registered newTab callback', async () => { + const mockCallback = vi.fn().mockResolvedValue({ tabId: 'new-tab-123' }); + registry.setNewTabCallback(mockCallback); + + const result = await registry.newTab('session-123'); + + expect(mockCallback).toHaveBeenCalledWith('session-123'); + expect(result?.tabId).toBe('new-tab-123'); + }); + + it('should call registered closeTab callback', async () => { + const mockCallback = vi.fn().mockResolvedValue(true); + registry.setCloseTabCallback(mockCallback); + + const result = await registry.closeTab('session-123', 'tab-1'); + + expect(mockCallback).toHaveBeenCalledWith('session-123', 'tab-1'); + expect(result).toBe(true); + }); + + it('should call registered renameTab callback', async () => { + const mockCallback = vi.fn().mockResolvedValue(true); + registry.setRenameTabCallback(mockCallback); + + const result = await registry.renameTab('session-123', 'tab-1', 'New Name'); + + expect(mockCallback).toHaveBeenCalledWith('session-123', 'tab-1', 'New Name'); + expect(result).toBe(true); + }); + + it('should call registered getHistory callback with optional parameters', () => { + const mockCallback = vi.fn().mockReturnValue([{ command: 'ls', timestamp: 123 }]); + registry.setGetHistoryCallback(mockCallback); + + const result = registry.getHistory('/project', 'session-123'); + + expect(mockCallback).toHaveBeenCalledWith('/project', 'session-123'); + expect(result).toHaveLength(1); + }); + }); + + describe('hasCallback', () => { + it('should return false for unset callbacks', () => { + expect(registry.hasCallback('getSessions')).toBe(false); + expect(registry.hasCallback('getTheme')).toBe(false); + expect(registry.hasCallback('executeCommand')).toBe(false); + }); + + it('should return true for set callbacks', () => { + registry.setGetSessionsCallback(vi.fn()); + registry.setGetThemeCallback(vi.fn()); + registry.setExecuteCommandCallback(vi.fn()); + + expect(registry.hasCallback('getSessions')).toBe(true); + expect(registry.hasCallback('getTheme')).toBe(true); + expect(registry.hasCallback('executeCommand')).toBe(true); + }); + + it('should check all callback types', () => { + // Initially all should be false + const callbackTypes = [ + 'getSessions', + 'getSessionDetail', + 'getTheme', + 'getCustomCommands', + 'writeToSession', + 'executeCommand', + 'interruptSession', + 'switchMode', + 'selectSession', + 'selectTab', + 'newTab', + 'closeTab', + 'renameTab', + 'getHistory', + ] as const; + + for (const type of callbackTypes) { + expect(registry.hasCallback(type)).toBe(false); + } + }); + }); + + describe('Callback Replacement', () => { + it('should replace existing callback with new one', () => { + const firstCallback = vi.fn().mockReturnValue([{ id: '1' }]); + const secondCallback = vi.fn().mockReturnValue([{ id: '2' }]); + + registry.setGetSessionsCallback(firstCallback); + expect(registry.getSessions()[0].id).toBe('1'); + + registry.setGetSessionsCallback(secondCallback); + expect(registry.getSessions()[0].id).toBe('2'); + + expect(firstCallback).toHaveBeenCalledTimes(1); + expect(secondCallback).toHaveBeenCalledTimes(1); + }); + }); + + describe('Async Callback Handling', () => { + it('should handle async executeCommand callback returning false', async () => { + const mockCallback = vi.fn().mockResolvedValue(false); + registry.setExecuteCommandCallback(mockCallback); + + const result = await registry.executeCommand('session-123', 'command'); + + expect(result).toBe(false); + }); + + it('should handle async switchMode callback rejection gracefully', async () => { + const mockCallback = vi.fn().mockRejectedValue(new Error('Switch failed')); + registry.setSwitchModeCallback(mockCallback); + + await expect(registry.switchMode('session-123', 'ai')).rejects.toThrow('Switch failed'); + }); + }); +}); diff --git a/src/__tests__/main/web-server/managers/LiveSessionManager.test.ts b/src/__tests__/main/web-server/managers/LiveSessionManager.test.ts new file mode 100644 index 000000000..7aef1affa --- /dev/null +++ b/src/__tests__/main/web-server/managers/LiveSessionManager.test.ts @@ -0,0 +1,476 @@ +/** + * Tests for LiveSessionManager + * + * Verifies: + * - Live session tracking (setLive, setOffline, isLive) + * - AutoRun state management + * - Broadcast callback integration + * - Memory leak prevention (cleanup on offline) + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { + LiveSessionManager, + LiveSessionBroadcastCallbacks, +} from '../../../../main/web-server/managers/LiveSessionManager'; + +// Mock the logger +vi.mock('../../../../main/utils/logger', () => ({ + logger: { + info: vi.fn(), + debug: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + }, +})); + +describe('LiveSessionManager', () => { + let manager: LiveSessionManager; + let mockBroadcastCallbacks: LiveSessionBroadcastCallbacks; + + beforeEach(() => { + vi.clearAllMocks(); + manager = new LiveSessionManager(); + mockBroadcastCallbacks = { + broadcastSessionLive: vi.fn(), + broadcastSessionOffline: vi.fn(), + broadcastAutoRunState: vi.fn(), + }; + }); + + describe('Live Session Tracking', () => { + describe('setSessionLive', () => { + it('should mark a session as live', () => { + manager.setSessionLive('session-123'); + + expect(manager.isSessionLive('session-123')).toBe(true); + }); + + it('should store agent session ID when provided', () => { + manager.setSessionLive('session-123', 'agent-session-abc'); + + const info = manager.getLiveSessionInfo('session-123'); + expect(info?.agentSessionId).toBe('agent-session-abc'); + }); + + it('should record enabledAt timestamp', () => { + const before = Date.now(); + manager.setSessionLive('session-123'); + const after = Date.now(); + + const info = manager.getLiveSessionInfo('session-123'); + expect(info?.enabledAt).toBeGreaterThanOrEqual(before); + expect(info?.enabledAt).toBeLessThanOrEqual(after); + }); + + it('should broadcast session live when callbacks set', () => { + manager.setBroadcastCallbacks(mockBroadcastCallbacks); + manager.setSessionLive('session-123', 'agent-session-abc'); + + expect(mockBroadcastCallbacks.broadcastSessionLive).toHaveBeenCalledWith( + 'session-123', + 'agent-session-abc' + ); + }); + + it('should not broadcast when callbacks not set', () => { + // No error should occur when broadcasting without callbacks + manager.setSessionLive('session-123'); + expect(manager.isSessionLive('session-123')).toBe(true); + }); + + it('should update existing session info when called again', () => { + manager.setSessionLive('session-123', 'agent-1'); + const firstInfo = manager.getLiveSessionInfo('session-123'); + + manager.setSessionLive('session-123', 'agent-2'); + const secondInfo = manager.getLiveSessionInfo('session-123'); + + expect(secondInfo?.agentSessionId).toBe('agent-2'); + expect(secondInfo?.enabledAt).toBeGreaterThanOrEqual(firstInfo!.enabledAt); + }); + }); + + describe('setSessionOffline', () => { + it('should mark a session as offline', () => { + manager.setSessionLive('session-123'); + expect(manager.isSessionLive('session-123')).toBe(true); + + manager.setSessionOffline('session-123'); + expect(manager.isSessionLive('session-123')).toBe(false); + }); + + it('should broadcast session offline when callbacks set', () => { + manager.setBroadcastCallbacks(mockBroadcastCallbacks); + manager.setSessionLive('session-123'); + manager.setSessionOffline('session-123'); + + expect(mockBroadcastCallbacks.broadcastSessionOffline).toHaveBeenCalledWith('session-123'); + }); + + it('should not broadcast if session was not live', () => { + manager.setBroadcastCallbacks(mockBroadcastCallbacks); + manager.setSessionOffline('never-existed'); + + expect(mockBroadcastCallbacks.broadcastSessionOffline).not.toHaveBeenCalled(); + }); + + it('should clean up associated AutoRun state (memory leak prevention)', () => { + manager.setSessionLive('session-123'); + manager.setAutoRunState('session-123', { + isRunning: true, + totalTasks: 10, + completedTasks: 5, + currentTask: 'Task 5', + }); + + expect(manager.getAutoRunState('session-123')).toBeDefined(); + + manager.setSessionOffline('session-123'); + + expect(manager.getAutoRunState('session-123')).toBeUndefined(); + }); + }); + + describe('isSessionLive', () => { + it('should return false for non-existent session', () => { + expect(manager.isSessionLive('non-existent')).toBe(false); + }); + + it('should return true for live session', () => { + manager.setSessionLive('session-123'); + expect(manager.isSessionLive('session-123')).toBe(true); + }); + + it('should return false after session goes offline', () => { + manager.setSessionLive('session-123'); + manager.setSessionOffline('session-123'); + expect(manager.isSessionLive('session-123')).toBe(false); + }); + }); + + describe('getLiveSessionInfo', () => { + it('should return undefined for non-existent session', () => { + expect(manager.getLiveSessionInfo('non-existent')).toBeUndefined(); + }); + + it('should return complete session info', () => { + manager.setSessionLive('session-123', 'agent-session-abc'); + + const info = manager.getLiveSessionInfo('session-123'); + + expect(info).toEqual({ + sessionId: 'session-123', + agentSessionId: 'agent-session-abc', + enabledAt: expect.any(Number), + }); + }); + }); + + describe('getLiveSessions', () => { + it('should return empty array when no sessions', () => { + expect(manager.getLiveSessions()).toEqual([]); + }); + + it('should return all live sessions', () => { + manager.setSessionLive('session-1'); + manager.setSessionLive('session-2'); + manager.setSessionLive('session-3'); + + const sessions = manager.getLiveSessions(); + + expect(sessions).toHaveLength(3); + expect(sessions.map((s) => s.sessionId)).toContain('session-1'); + expect(sessions.map((s) => s.sessionId)).toContain('session-2'); + expect(sessions.map((s) => s.sessionId)).toContain('session-3'); + }); + + it('should not include offline sessions', () => { + manager.setSessionLive('session-1'); + manager.setSessionLive('session-2'); + manager.setSessionOffline('session-1'); + + const sessions = manager.getLiveSessions(); + + expect(sessions).toHaveLength(1); + expect(sessions[0].sessionId).toBe('session-2'); + }); + }); + + describe('getLiveSessionIds', () => { + it('should return iterable of session IDs', () => { + manager.setSessionLive('session-1'); + manager.setSessionLive('session-2'); + + const ids = Array.from(manager.getLiveSessionIds()); + + expect(ids).toHaveLength(2); + expect(ids).toContain('session-1'); + expect(ids).toContain('session-2'); + }); + }); + + describe('getLiveSessionCount', () => { + it('should return 0 when no sessions', () => { + expect(manager.getLiveSessionCount()).toBe(0); + }); + + it('should return correct count', () => { + manager.setSessionLive('session-1'); + manager.setSessionLive('session-2'); + manager.setSessionLive('session-3'); + + expect(manager.getLiveSessionCount()).toBe(3); + + manager.setSessionOffline('session-2'); + + expect(manager.getLiveSessionCount()).toBe(2); + }); + }); + }); + + describe('AutoRun State Management', () => { + describe('setAutoRunState', () => { + it('should store running AutoRun state', () => { + const state = { + isRunning: true, + totalTasks: 10, + completedTasks: 3, + currentTask: 'Task 3', + }; + + manager.setAutoRunState('session-123', state); + + expect(manager.getAutoRunState('session-123')).toEqual(state); + }); + + it('should remove state when isRunning is false', () => { + manager.setAutoRunState('session-123', { + isRunning: true, + totalTasks: 10, + completedTasks: 3, + currentTask: 'Task 3', + }); + + manager.setAutoRunState('session-123', { + isRunning: false, + totalTasks: 10, + completedTasks: 10, + currentTask: 'Complete', + }); + + expect(manager.getAutoRunState('session-123')).toBeUndefined(); + }); + + it('should remove state when null is passed', () => { + manager.setAutoRunState('session-123', { + isRunning: true, + totalTasks: 10, + completedTasks: 3, + currentTask: 'Task 3', + }); + + manager.setAutoRunState('session-123', null); + + expect(manager.getAutoRunState('session-123')).toBeUndefined(); + }); + + it('should broadcast AutoRun state when callbacks set', () => { + manager.setBroadcastCallbacks(mockBroadcastCallbacks); + const state = { + isRunning: true, + totalTasks: 10, + completedTasks: 3, + currentTask: 'Task 3', + }; + + manager.setAutoRunState('session-123', state); + + expect(mockBroadcastCallbacks.broadcastAutoRunState).toHaveBeenCalledWith( + 'session-123', + state + ); + }); + + it('should broadcast null state when clearing', () => { + manager.setBroadcastCallbacks(mockBroadcastCallbacks); + manager.setAutoRunState('session-123', { + isRunning: true, + totalTasks: 10, + completedTasks: 3, + currentTask: 'Task 3', + }); + + manager.setAutoRunState('session-123', null); + + expect(mockBroadcastCallbacks.broadcastAutoRunState).toHaveBeenLastCalledWith( + 'session-123', + null + ); + }); + }); + + describe('getAutoRunState', () => { + it('should return undefined for non-existent state', () => { + expect(manager.getAutoRunState('non-existent')).toBeUndefined(); + }); + + it('should return stored state', () => { + const state = { + isRunning: true, + totalTasks: 5, + completedTasks: 2, + currentTask: 'Task 2', + }; + manager.setAutoRunState('session-123', state); + + expect(manager.getAutoRunState('session-123')).toEqual(state); + }); + }); + + describe('getAutoRunStates', () => { + it('should return empty map when no states', () => { + const states = manager.getAutoRunStates(); + expect(states.size).toBe(0); + }); + + it('should return all stored states', () => { + manager.setAutoRunState('session-1', { + isRunning: true, + totalTasks: 5, + completedTasks: 1, + currentTask: 'Task 1', + }); + manager.setAutoRunState('session-2', { + isRunning: true, + totalTasks: 10, + completedTasks: 5, + currentTask: 'Task 5', + }); + + const states = manager.getAutoRunStates(); + + expect(states.size).toBe(2); + expect(states.get('session-1')?.totalTasks).toBe(5); + expect(states.get('session-2')?.totalTasks).toBe(10); + }); + }); + }); + + describe('clearAll', () => { + it('should mark all live sessions as offline', () => { + manager.setBroadcastCallbacks(mockBroadcastCallbacks); + manager.setSessionLive('session-1'); + manager.setSessionLive('session-2'); + manager.setSessionLive('session-3'); + + manager.clearAll(); + + expect(manager.getLiveSessionCount()).toBe(0); + expect(mockBroadcastCallbacks.broadcastSessionOffline).toHaveBeenCalledTimes(3); + }); + + it('should clear all AutoRun states', () => { + manager.setSessionLive('session-1'); + manager.setAutoRunState('session-1', { + isRunning: true, + totalTasks: 5, + completedTasks: 1, + currentTask: 'Task 1', + }); + manager.setSessionLive('session-2'); + manager.setAutoRunState('session-2', { + isRunning: true, + totalTasks: 10, + completedTasks: 5, + currentTask: 'Task 5', + }); + + manager.clearAll(); + + expect(manager.getAutoRunStates().size).toBe(0); + }); + + it('should handle being called when already empty', () => { + // Should not throw + manager.clearAll(); + expect(manager.getLiveSessionCount()).toBe(0); + }); + }); + + describe('Integration Scenarios', () => { + it('should handle full session lifecycle', () => { + manager.setBroadcastCallbacks(mockBroadcastCallbacks); + + // Session comes online + manager.setSessionLive('session-123', 'agent-abc'); + expect(manager.isSessionLive('session-123')).toBe(true); + expect(mockBroadcastCallbacks.broadcastSessionLive).toHaveBeenCalled(); + + // AutoRun starts + manager.setAutoRunState('session-123', { + isRunning: true, + totalTasks: 5, + completedTasks: 0, + currentTask: 'Task 1', + }); + expect(mockBroadcastCallbacks.broadcastAutoRunState).toHaveBeenCalled(); + + // AutoRun progresses + manager.setAutoRunState('session-123', { + isRunning: true, + totalTasks: 5, + completedTasks: 3, + currentTask: 'Task 4', + }); + + // AutoRun completes + manager.setAutoRunState('session-123', { + isRunning: false, + totalTasks: 5, + completedTasks: 5, + currentTask: 'Complete', + }); + expect(manager.getAutoRunState('session-123')).toBeUndefined(); + + // Session goes offline + manager.setSessionOffline('session-123'); + expect(manager.isSessionLive('session-123')).toBe(false); + expect(mockBroadcastCallbacks.broadcastSessionOffline).toHaveBeenCalled(); + }); + + it('should handle multiple concurrent sessions', () => { + manager.setSessionLive('session-1', 'agent-1'); + manager.setSessionLive('session-2', 'agent-2'); + manager.setSessionLive('session-3', 'agent-3'); + + manager.setAutoRunState('session-1', { + isRunning: true, + totalTasks: 3, + completedTasks: 1, + currentTask: 'Task 1', + }); + manager.setAutoRunState('session-3', { + isRunning: true, + totalTasks: 5, + completedTasks: 2, + currentTask: 'Task 2', + }); + + expect(manager.getLiveSessionCount()).toBe(3); + expect(manager.getAutoRunStates().size).toBe(2); + + // Session 2 goes offline (no AutoRun state to clean) + manager.setSessionOffline('session-2'); + expect(manager.getLiveSessionCount()).toBe(2); + expect(manager.getAutoRunStates().size).toBe(2); + + // Session 1 goes offline (has AutoRun state) + manager.setSessionOffline('session-1'); + expect(manager.getLiveSessionCount()).toBe(1); + expect(manager.getAutoRunStates().size).toBe(1); + expect(manager.getAutoRunState('session-1')).toBeUndefined(); + expect(manager.getAutoRunState('session-3')).toBeDefined(); + }); + }); +}); diff --git a/src/main/agent-capabilities.ts b/src/main/agents/capabilities.ts similarity index 100% rename from src/main/agent-capabilities.ts rename to src/main/agents/capabilities.ts diff --git a/src/main/agent-definitions.ts b/src/main/agents/definitions.ts similarity index 98% rename from src/main/agent-definitions.ts rename to src/main/agents/definitions.ts index c41650e25..510935d8b 100644 --- a/src/main/agent-definitions.ts +++ b/src/main/agents/definitions.ts @@ -3,11 +3,9 @@ * * Contains the configuration definitions for all supported AI agents. * This includes CLI arguments, configuration options, and default settings. - * - * Separated from agent-detector.ts for better maintainability. */ -import type { AgentCapabilities } from './agent-capabilities'; +import type { AgentCapabilities } from './capabilities'; // ============ Configuration Types ============ diff --git a/src/main/agent-detector.ts b/src/main/agents/detector.ts similarity index 91% rename from src/main/agent-detector.ts rename to src/main/agents/detector.ts index 4419bece3..d588b2cbc 100644 --- a/src/main/agent-detector.ts +++ b/src/main/agents/detector.ts @@ -6,30 +6,14 @@ * - Manages custom paths for agents * - Caches detection results for performance * - Discovers available models for agents that support model selection - * - * Architecture: - * - Agent definitions are in agent-definitions.ts - * - Path probing logic is in path-prober.ts - * - Agent capabilities are in agent-capabilities.ts */ import * as path from 'path'; -import { execFileNoThrow } from './utils/execFile'; -import { logger } from './utils/logger'; -import { getAgentCapabilities } from './agent-capabilities'; +import { execFileNoThrow } from '../utils/execFile'; +import { logger } from '../utils/logger'; +import { getAgentCapabilities } from './capabilities'; import { checkBinaryExists, checkCustomPath, getExpandedEnv } from './path-prober'; -import { AGENT_DEFINITIONS, type AgentConfig } from './agent-definitions'; - -// ============ Re-exports for API Compatibility ============ -// These exports maintain backwards compatibility with existing code - -export { AgentCapabilities } from './agent-capabilities'; -export { - AGENT_DEFINITIONS, - type AgentConfig, - type AgentConfigOption, - type AgentDefinition, -} from './agent-definitions'; +import { AGENT_DEFINITIONS, type AgentConfig } from './definitions'; const LOG_CONTEXT = 'AgentDetector'; diff --git a/src/main/agents/index.ts b/src/main/agents/index.ts new file mode 100644 index 000000000..4748060de --- /dev/null +++ b/src/main/agents/index.ts @@ -0,0 +1,68 @@ +/** + * Agents Module + * + * This module consolidates all agent-related functionality: + * - Agent detection and configuration + * - Agent definitions and types + * - Agent capabilities + * - Session storage interface + * - Binary path probing + * + * Usage: + * ```typescript + * import { AgentDetector, AGENT_DEFINITIONS, getAgentCapabilities } from './agents'; + * ``` + */ + +// ============ Capabilities ============ +export { + type AgentCapabilities, + DEFAULT_CAPABILITIES, + AGENT_CAPABILITIES, + getAgentCapabilities, + hasCapability, +} from './capabilities'; + +// ============ Definitions ============ +export { + type AgentConfigOption, + type AgentConfig, + type AgentDefinition, + AGENT_DEFINITIONS, + getAgentDefinition, + getAgentIds, + getVisibleAgentDefinitions, +} from './definitions'; + +// ============ Detector ============ +export { AgentDetector } from './detector'; + +// ============ Path Prober ============ +export { + type BinaryDetectionResult, + getExpandedEnv, + checkCustomPath, + probeWindowsPaths, + probeUnixPaths, + checkBinaryExists, +} from './path-prober'; + +// ============ Session Storage ============ +export { + type AgentSessionOrigin, + type SessionMessage, + type AgentSessionInfo, + type PaginatedSessionsResult, + type SessionMessagesResult, + type SessionSearchResult, + type SessionSearchMode, + type SessionListOptions, + type SessionReadOptions, + type SessionOriginInfo, + type AgentSessionStorage, + registerSessionStorage, + getSessionStorage, + hasSessionStorage, + getAllSessionStorages, + clearStorageRegistry, +} from './session-storage'; diff --git a/src/main/path-prober.ts b/src/main/agents/path-prober.ts similarity index 98% rename from src/main/path-prober.ts rename to src/main/agents/path-prober.ts index a18aef3be..fc935038d 100644 --- a/src/main/path-prober.ts +++ b/src/main/agents/path-prober.ts @@ -4,16 +4,14 @@ * Handles detection of agent binaries on Windows and Unix-like systems. * Packaged Electron apps don't inherit shell environment, so we need to * probe known installation paths directly. - * - * Separated from agent-detector.ts for better maintainability and testability. */ import * as os from 'os'; import * as fs from 'fs'; import * as path from 'path'; -import { execFileNoThrow } from './utils/execFile'; -import { logger } from './utils/logger'; -import { expandTilde, detectNodeVersionManagerBinPaths } from '../shared/pathUtils'; +import { execFileNoThrow } from '../utils/execFile'; +import { logger } from '../utils/logger'; +import { expandTilde, detectNodeVersionManagerBinPaths } from '../../shared/pathUtils'; const LOG_CONTEXT = 'PathProber'; diff --git a/src/main/agent-session-storage.ts b/src/main/agents/session-storage.ts similarity index 98% rename from src/main/agent-session-storage.ts rename to src/main/agents/session-storage.ts index ca2b979fc..8f34bb302 100644 --- a/src/main/agent-session-storage.ts +++ b/src/main/agents/session-storage.ts @@ -14,8 +14,8 @@ * ``` */ -import type { ToolType, SshRemoteConfig } from '../shared/types'; -import { logger } from './utils/logger'; +import type { ToolType, SshRemoteConfig } from '../../shared/types'; +import { logger } from '../utils/logger'; const LOG_CONTEXT = '[AgentSessionStorage]'; diff --git a/src/main/debug-package/collectors/agents.ts b/src/main/debug-package/collectors/agents.ts index 22e21b42c..7ba24c728 100644 --- a/src/main/debug-package/collectors/agents.ts +++ b/src/main/debug-package/collectors/agents.ts @@ -6,7 +6,7 @@ * - Custom args/env vars show only whether they're set, not values */ -import { AgentDetector, AgentCapabilities } from '../../agent-detector'; +import { AgentDetector, type AgentCapabilities } from '../../agents'; import { sanitizePath } from './settings'; export interface AgentInfo { diff --git a/src/main/debug-package/index.ts b/src/main/debug-package/index.ts index 5e49dc443..37a167005 100644 --- a/src/main/debug-package/index.ts +++ b/src/main/debug-package/index.ts @@ -29,7 +29,7 @@ import { } from './collectors/windows-diagnostics'; import { createZipPackage, PackageContents } from './packager'; import { logger } from '../utils/logger'; -import { AgentDetector } from '../agent-detector'; +import { AgentDetector } from '../agents'; import { ProcessManager } from '../process-manager'; import { WebServer } from '../web-server'; import Store from 'electron-store'; diff --git a/src/main/group-chat/group-chat-agent.ts b/src/main/group-chat/group-chat-agent.ts index 6ca8f37db..7669c66f4 100644 --- a/src/main/group-chat/group-chat-agent.ts +++ b/src/main/group-chat/group-chat-agent.ts @@ -18,7 +18,7 @@ import { } from './group-chat-storage'; import { appendToLog } from './group-chat-log'; import { IProcessManager, isModeratorActive } from './group-chat-moderator'; -import type { AgentDetector } from '../agent-detector'; +import type { AgentDetector } from '../agents'; import { buildAgentArgs, applyAgentConfigOverrides, diff --git a/src/main/group-chat/group-chat-router.ts b/src/main/group-chat/group-chat-router.ts index a56bd8c59..c521aa169 100644 --- a/src/main/group-chat/group-chat-router.ts +++ b/src/main/group-chat/group-chat-router.ts @@ -26,7 +26,7 @@ import { getModeratorSynthesisPrompt, } from './group-chat-moderator'; import { addParticipant } from './group-chat-agent'; -import { AgentDetector } from '../agent-detector'; +import { AgentDetector } from '../agents'; import { powerManager } from '../power-manager'; import { buildAgentArgs, diff --git a/src/main/index.ts b/src/main/index.ts index 2e9bd077f..51f4a3b9b 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -5,7 +5,7 @@ import crypto from 'crypto'; // which causes "Cannot read properties of undefined (reading 'getAppPath')" errors import { ProcessManager } from './process-manager'; import { WebServer } from './web-server'; -import { AgentDetector } from './agent-detector'; +import { AgentDetector } from './agents'; import { logger } from './utils/logger'; import { tunnelManager } from './tunnel-manager'; import { powerManager } from './power-manager'; diff --git a/src/main/ipc/handlers/agentSessions.ts b/src/main/ipc/handlers/agentSessions.ts index 032200c4e..9a205a67d 100644 --- a/src/main/ipc/handlers/agentSessions.ts +++ b/src/main/ipc/handlers/agentSessions.ts @@ -21,11 +21,7 @@ import os from 'os'; import fs from 'fs/promises'; import { logger } from '../../utils/logger'; import { withIpcErrorLogging } from '../../utils/ipcHandler'; -import { - getSessionStorage, - hasSessionStorage, - getAllSessionStorages, -} from '../../agent-session-storage'; +import { getSessionStorage, hasSessionStorage, getAllSessionStorages } from '../../agents'; import { calculateClaudeCost } from '../../utils/pricing'; import { loadGlobalStatsCache, @@ -42,7 +38,7 @@ import type { SessionSearchMode, SessionListOptions, SessionReadOptions, -} from '../../agent-session-storage'; +} from '../../agents'; import type { GlobalAgentStats, ProviderStats, SshRemoteConfig } from '../../../shared/types'; import type { MaestroSettings } from './persistence'; diff --git a/src/main/ipc/handlers/agents.ts b/src/main/ipc/handlers/agents.ts index 90270abb2..e68500ea5 100644 --- a/src/main/ipc/handlers/agents.ts +++ b/src/main/ipc/handlers/agents.ts @@ -1,7 +1,6 @@ import { ipcMain } from 'electron'; import Store from 'electron-store'; -import { AgentDetector, AGENT_DEFINITIONS } from '../../agent-detector'; -import { getAgentCapabilities } from '../../agent-capabilities'; +import { AgentDetector, AGENT_DEFINITIONS, getAgentCapabilities } from '../../agents'; import { execFileNoThrow } from '../../utils/execFile'; import { logger } from '../../utils/logger'; import { diff --git a/src/main/ipc/handlers/context.ts b/src/main/ipc/handlers/context.ts index d2d359882..d271fbb19 100644 --- a/src/main/ipc/handlers/context.ts +++ b/src/main/ipc/handlers/context.ts @@ -20,10 +20,10 @@ import { requireDependency, CreateHandlerOptions, } from '../../utils/ipcHandler'; -import { getSessionStorage, type SessionMessagesResult } from '../../agent-session-storage'; +import { getSessionStorage, type SessionMessagesResult } from '../../agents'; import { groomContext, cancelAllGroomingSessions } from '../../utils/context-groomer'; import type { ProcessManager } from '../../process-manager'; -import type { AgentDetector } from '../../agent-detector'; +import type { AgentDetector } from '../../agents'; const LOG_CONTEXT = '[ContextMerge]'; diff --git a/src/main/ipc/handlers/debug.ts b/src/main/ipc/handlers/debug.ts index e6ef1a0c9..f546a5ced 100644 --- a/src/main/ipc/handlers/debug.ts +++ b/src/main/ipc/handlers/debug.ts @@ -16,7 +16,7 @@ import { DebugPackageOptions, DebugPackageDependencies, } from '../../debug-package'; -import { AgentDetector } from '../../agent-detector'; +import { AgentDetector } from '../../agents'; import { ProcessManager } from '../../process-manager'; import { WebServer } from '../../web-server'; diff --git a/src/main/ipc/handlers/groupChat.ts b/src/main/ipc/handlers/groupChat.ts index 881e77c38..d2f90bf21 100644 --- a/src/main/ipc/handlers/groupChat.ts +++ b/src/main/ipc/handlers/groupChat.ts @@ -62,7 +62,7 @@ import { import { routeUserMessage } from '../../group-chat/group-chat-router'; // Agent detector import -import { AgentDetector } from '../../agent-detector'; +import { AgentDetector } from '../../agents'; import { groomContext } from '../../utils/context-groomer'; import { v4 as uuidv4 } from 'uuid'; diff --git a/src/main/ipc/handlers/index.ts b/src/main/ipc/handlers/index.ts index 9a7025f07..65ed7fcc3 100644 --- a/src/main/ipc/handlers/index.ts +++ b/src/main/ipc/handlers/index.ts @@ -49,7 +49,7 @@ import { registerWebHandlers, WebHandlerDependencies } from './web'; import { registerLeaderboardHandlers, LeaderboardHandlerDependencies } from './leaderboard'; import { registerNotificationsHandlers } from './notifications'; import { registerAgentErrorHandlers } from './agent-error'; -import { AgentDetector } from '../../agent-detector'; +import { AgentDetector } from '../../agents'; import { ProcessManager } from '../../process-manager'; import { WebServer } from '../../web-server'; import { tunnelManager as tunnelManagerInstance } from '../../tunnel-manager'; diff --git a/src/main/ipc/handlers/process.ts b/src/main/ipc/handlers/process.ts index 77aeb4be1..54c650244 100644 --- a/src/main/ipc/handlers/process.ts +++ b/src/main/ipc/handlers/process.ts @@ -2,7 +2,7 @@ import { ipcMain, BrowserWindow } from 'electron'; import Store from 'electron-store'; import * as os from 'os'; import { ProcessManager } from '../../process-manager'; -import { AgentDetector } from '../../agent-detector'; +import { AgentDetector } from '../../agents'; import { logger } from '../../utils/logger'; import { buildAgentArgs, diff --git a/src/main/process-listeners/types.ts b/src/main/process-listeners/types.ts index b80f267dd..d91b8c481 100644 --- a/src/main/process-listeners/types.ts +++ b/src/main/process-listeners/types.ts @@ -5,7 +5,7 @@ import type { ProcessManager } from '../process-manager'; import type { WebServer } from '../web-server'; -import type { AgentDetector } from '../agent-detector'; +import type { AgentDetector } from '../agents'; import type { SafeSendFn } from '../utils/safe-send'; import type { StatsDB } from '../stats-db'; import type { GroupChat, GroupChatParticipant } from '../group-chat/group-chat-storage'; diff --git a/src/main/process-manager/spawners/ChildProcessSpawner.ts b/src/main/process-manager/spawners/ChildProcessSpawner.ts index 1dd69bfb7..0ab5d8fff 100644 --- a/src/main/process-manager/spawners/ChildProcessSpawner.ts +++ b/src/main/process-manager/spawners/ChildProcessSpawner.ts @@ -5,7 +5,7 @@ import { EventEmitter } from 'events'; import * as path from 'path'; import { logger } from '../../utils/logger'; import { getOutputParser } from '../../parsers'; -import { getAgentCapabilities } from '../../agent-capabilities'; +import { getAgentCapabilities } from '../../agents'; import type { ProcessConfig, ManagedProcess, SpawnResult } from '../types'; import type { DataBufferManager } from '../handlers/DataBufferManager'; import { StdoutHandler } from '../handlers/StdoutHandler'; diff --git a/src/main/storage/claude-session-storage.ts b/src/main/storage/claude-session-storage.ts index f2383af75..fa53a1abc 100644 --- a/src/main/storage/claude-session-storage.ts +++ b/src/main/storage/claude-session-storage.ts @@ -32,7 +32,7 @@ import type { AgentSessionOrigin, SessionOriginInfo, SessionMessage, -} from '../agent-session-storage'; +} from '../agents'; import type { ToolType, SshRemoteConfig } from '../../shared/types'; const LOG_CONTEXT = '[ClaudeSessionStorage]'; diff --git a/src/main/storage/codex-session-storage.ts b/src/main/storage/codex-session-storage.ts index 22b45df27..612915318 100644 --- a/src/main/storage/codex-session-storage.ts +++ b/src/main/storage/codex-session-storage.ts @@ -35,7 +35,7 @@ import type { SessionListOptions, SessionReadOptions, SessionMessage, -} from '../agent-session-storage'; +} from '../agents'; import type { ToolType } from '../../shared/types'; const LOG_CONTEXT = '[CodexSessionStorage]'; diff --git a/src/main/storage/index.ts b/src/main/storage/index.ts index ae555c8f2..874ba23c5 100644 --- a/src/main/storage/index.ts +++ b/src/main/storage/index.ts @@ -10,7 +10,7 @@ export { OpenCodeSessionStorage } from './opencode-session-storage'; export { CodexSessionStorage } from './codex-session-storage'; import Store from 'electron-store'; -import { registerSessionStorage } from '../agent-session-storage'; +import { registerSessionStorage } from '../agents'; import { ClaudeSessionStorage, ClaudeSessionOriginsData } from './claude-session-storage'; import { OpenCodeSessionStorage } from './opencode-session-storage'; import { CodexSessionStorage } from './codex-session-storage'; diff --git a/src/main/storage/opencode-session-storage.ts b/src/main/storage/opencode-session-storage.ts index 270ee36c8..55c04102c 100644 --- a/src/main/storage/opencode-session-storage.ts +++ b/src/main/storage/opencode-session-storage.ts @@ -33,7 +33,7 @@ import type { SessionListOptions, SessionReadOptions, SessionMessage, -} from '../agent-session-storage'; +} from '../agents'; import type { ToolType } from '../../shared/types'; const LOG_CONTEXT = '[OpenCodeSessionStorage]'; diff --git a/src/main/utils/agent-args.ts b/src/main/utils/agent-args.ts index b66becacd..f9ea0587b 100644 --- a/src/main/utils/agent-args.ts +++ b/src/main/utils/agent-args.ts @@ -1,4 +1,4 @@ -import type { AgentConfig } from '../agent-detector'; +import type { AgentConfig } from '../agents'; type BuildAgentArgsOptions = { baseArgs: string[]; diff --git a/src/main/utils/context-groomer.ts b/src/main/utils/context-groomer.ts index 8687662b8..225968327 100644 --- a/src/main/utils/context-groomer.ts +++ b/src/main/utils/context-groomer.ts @@ -15,7 +15,7 @@ import { v4 as uuidv4 } from 'uuid'; import { logger } from './logger'; import { buildAgentArgs } from './agent-args'; -import type { AgentDetector } from '../agent-detector'; +import type { AgentDetector } from '../agents'; const LOG_CONTEXT = '[ContextGroomer]'; From 7a934376f94b72ad2293aa5e344d83e98facbbdd Mon Sep 17 00:00:00 2001 From: Raza Rauf Date: Thu, 29 Jan 2026 22:54:41 +0500 Subject: [PATCH 3/4] fix: address code review feedback with type safety and performance improvements Type Safety (Medium Priority): - Replace `any` types in AgentConfigOption with discriminated union - Four specific types: CheckboxConfigOption, TextConfigOption, NumberConfigOption, SelectConfigOption - Update agent-args.ts to handle new types with proper assertions Error Handling (Medium Priority): - Add try/catch wrapper around runModelDiscovery to gracefully handle exceptions and return empty array on failure Performance (Medium Priority): - Implement parallel path probing using Promise.allSettled - Previously: sequential checks (6-10 paths, one after another) - Now: all paths checked concurrently, first success returned - Maintains priority order while reducing detection time - Significant improvement on slow file systems/network drives Configuration (Low Priority): - Make model cache TTL configurable via constructor parameter - Default remains 5 minutes, but can be customized for testing Logging (Low Priority): - Add debug logging for silent error swallowing in checkCustomPath Documentation (Low Priority): - Enhanced module-level JSDoc for detector.ts and path-prober.ts - Document detection strategy and caching behavior Testing: - Add cache TTL expiration test using fake timers - Add constructor TTL configuration test --- src/__tests__/main/agents/detector.test.ts | 75 ++++++++++++++++++ src/main/agents/definitions.ts | 55 +++++++++++-- src/main/agents/detector.ts | 92 ++++++++++++++-------- src/main/agents/path-prober.ts | 65 +++++++++++---- src/main/utils/agent-args.ts | 11 ++- 5 files changed, 240 insertions(+), 58 deletions(-) diff --git a/src/__tests__/main/agents/detector.test.ts b/src/__tests__/main/agents/detector.test.ts index d2d189102..404d48f16 100644 --- a/src/__tests__/main/agents/detector.test.ts +++ b/src/__tests__/main/agents/detector.test.ts @@ -1215,6 +1215,81 @@ describe('agent-detector', () => { }); }); + describe('model cache TTL', () => { + it('should invalidate model cache after TTL expires', async () => { + vi.useFakeTimers(); + + // Setup: opencode is available + mockExecFileNoThrow.mockImplementation(async (cmd, args) => { + const binaryName = args[0]; + if (binaryName === 'opencode') { + return { stdout: '/usr/bin/opencode\n', stderr: '', exitCode: 0 }; + } + if (cmd === '/usr/bin/opencode' && args[0] === 'models') { + return { + stdout: 'initial-model\n', + stderr: '', + exitCode: 0, + }; + } + return { stdout: '', stderr: 'not found', exitCode: 1 }; + }); + + // Create detector with short TTL for testing (100ms) + const shortTtlDetector = new AgentDetector(100); + await shortTtlDetector.detectAgents(); + + // First call - should fetch + const models1 = await shortTtlDetector.discoverModels('opencode'); + expect(models1).toEqual(['initial-model']); + + // Clear mocks to track new calls + mockExecFileNoThrow.mockClear(); + + // Second call immediately - should use cache + const models2 = await shortTtlDetector.discoverModels('opencode'); + expect(models2).toEqual(['initial-model']); + expect(mockExecFileNoThrow).not.toHaveBeenCalledWith( + '/usr/bin/opencode', + ['models'], + undefined, + expect.any(Object) + ); + + // Advance time past TTL + vi.advanceTimersByTime(150); + + // Setup new response for after cache expires + mockExecFileNoThrow.mockImplementation(async (cmd, args) => { + if (cmd === '/usr/bin/opencode' && args[0] === 'models') { + return { + stdout: 'new-model-after-ttl\n', + stderr: '', + exitCode: 0, + }; + } + return { stdout: '', stderr: '', exitCode: 1 }; + }); + + // Third call after TTL - should re-fetch + const models3 = await shortTtlDetector.discoverModels('opencode'); + expect(models3).toEqual(['new-model-after-ttl']); + expect(mockExecFileNoThrow).toHaveBeenCalledWith( + '/usr/bin/opencode', + ['models'], + undefined, + expect.any(Object) + ); + + vi.useRealTimers(); + }); + + it('should accept custom cache TTL in constructor', () => { + const customTtlDetector = new AgentDetector(60000); // 1 minute + expect(customTtlDetector).toBeDefined(); + }); + }); + describe('clearModelCache', () => { beforeEach(async () => { mockExecFileNoThrow.mockImplementation(async (cmd, args) => { diff --git a/src/main/agents/definitions.ts b/src/main/agents/definitions.ts index 510935d8b..7931e79cc 100644 --- a/src/main/agents/definitions.ts +++ b/src/main/agents/definitions.ts @@ -10,18 +10,61 @@ import type { AgentCapabilities } from './capabilities'; // ============ Configuration Types ============ /** - * Configuration option types for agent-specific settings + * Base configuration option fields shared by all types */ -export interface AgentConfigOption { +interface BaseConfigOption { key: string; // Storage key - type: 'checkbox' | 'text' | 'number' | 'select'; label: string; // UI label description: string; // Help text - default: any; // Default value - options?: string[]; // For select type - argBuilder?: (value: any) => string[]; // Converts config value to CLI args } +/** + * Checkbox configuration option (boolean value) + */ +interface CheckboxConfigOption extends BaseConfigOption { + type: 'checkbox'; + default: boolean; + argBuilder?: (value: boolean) => string[]; +} + +/** + * Text configuration option (string value) + */ +interface TextConfigOption extends BaseConfigOption { + type: 'text'; + default: string; + argBuilder?: (value: string) => string[]; +} + +/** + * Number configuration option (numeric value) + */ +interface NumberConfigOption extends BaseConfigOption { + type: 'number'; + default: number; + argBuilder?: (value: number) => string[]; +} + +/** + * Select configuration option (string value from predefined options) + */ +interface SelectConfigOption extends BaseConfigOption { + type: 'select'; + default: string; + options: string[]; + argBuilder?: (value: string) => string[]; +} + +/** + * Configuration option types for agent-specific settings. + * Uses discriminated union for full type safety. + */ +export type AgentConfigOption = + | CheckboxConfigOption + | TextConfigOption + | NumberConfigOption + | SelectConfigOption; + /** * Full agent configuration including runtime detection state */ diff --git a/src/main/agents/detector.ts b/src/main/agents/detector.ts index d588b2cbc..98c094783 100644 --- a/src/main/agents/detector.ts +++ b/src/main/agents/detector.ts @@ -1,11 +1,16 @@ /** - * Agent Detector - Detects available AI agents on the system + * Agent Detection and Configuration Manager * - * This module provides the main AgentDetector class that: - * - Detects which agents are available on the system - * - Manages custom paths for agents + * Responsibilities: + * - Detects installed agents via file system probing and PATH resolution + * - Manages agent configuration and capability metadata * - Caches detection results for performance * - Discovers available models for agents that support model selection + * + * Model Discovery: + * - Model lists are cached for 5 minutes (configurable) to balance freshness and performance + * - Each agent implements its own model discovery command + * - Cache can be manually cleared or bypassed with forceRefresh flag */ import * as path from 'path'; @@ -19,14 +24,25 @@ const LOG_CONTEXT = 'AgentDetector'; // ============ Agent Detector Class ============ +/** Default cache TTL: 5 minutes (model lists don't change frequently) */ +const DEFAULT_MODEL_CACHE_TTL_MS = 5 * 60 * 1000; + export class AgentDetector { private cachedAgents: AgentConfig[] | null = null; private detectionInProgress: Promise | null = null; private customPaths: Record = {}; // Cache for model discovery results: agentId -> { models, timestamp } private modelCache: Map = new Map(); - // Cache TTL: 5 minutes (model lists don't change frequently) - private readonly MODEL_CACHE_TTL_MS = 5 * 60 * 1000; + // Configurable cache TTL (useful for testing or different environments) + private readonly modelCacheTtlMs: number; + + /** + * Create an AgentDetector instance + * @param modelCacheTtlMs - Model cache TTL in milliseconds (default: 5 minutes) + */ + constructor(modelCacheTtlMs: number = DEFAULT_MODEL_CACHE_TTL_MS) { + this.modelCacheTtlMs = modelCacheTtlMs; + } /** * Set custom paths for agents (from user configuration) @@ -205,7 +221,7 @@ export class AgentDetector { // Check cache unless force refresh if (!forceRefresh) { const cached = this.modelCache.get(agentId); - if (cached && Date.now() - cached.timestamp < this.MODEL_CACHE_TTL_MS) { + if (cached && Date.now() - cached.timestamp < this.modelCacheTtlMs) { logger.debug(`Returning cached models for ${agentId}`, LOG_CONTEXT); return cached.models; } @@ -223,42 +239,50 @@ export class AgentDetector { /** * Run the agent-specific model discovery command. * Each agent may have a different way to list available models. + * + * This method catches all exceptions to ensure graceful degradation + * when model discovery fails for any reason. */ private async runModelDiscovery(agentId: string, agent: AgentConfig): Promise { const env = getExpandedEnv(); const command = agent.path || agent.command; - // Agent-specific model discovery commands - switch (agentId) { - case 'opencode': { - // OpenCode: `opencode models` returns one model per line - const result = await execFileNoThrow(command, ['models'], undefined, env); + try { + // Agent-specific model discovery commands + switch (agentId) { + case 'opencode': { + // OpenCode: `opencode models` returns one model per line + const result = await execFileNoThrow(command, ['models'], undefined, env); + + if (result.exitCode !== 0) { + logger.warn( + `Model discovery failed for ${agentId}: exit code ${result.exitCode}`, + LOG_CONTEXT, + { stderr: result.stderr } + ); + return []; + } + + // Parse output: one model per line (e.g., "opencode/gpt-5-nano", "ollama/gpt-oss:latest") + const models = result.stdout + .split('\n') + .map((line) => line.trim()) + .filter((line) => line.length > 0); - if (result.exitCode !== 0) { - logger.warn( - `Model discovery failed for ${agentId}: exit code ${result.exitCode}`, - LOG_CONTEXT, - { stderr: result.stderr } - ); - return []; + logger.info(`Discovered ${models.length} models for ${agentId}`, LOG_CONTEXT, { + models, + }); + return models; } - // Parse output: one model per line (e.g., "opencode/gpt-5-nano", "ollama/gpt-oss:latest") - const models = result.stdout - .split('\n') - .map((line) => line.trim()) - .filter((line) => line.length > 0); - - logger.info(`Discovered ${models.length} models for ${agentId}`, LOG_CONTEXT, { - models, - }); - return models; + default: + // For agents without model discovery implemented, return empty array + logger.debug(`No model discovery implemented for ${agentId}`, LOG_CONTEXT); + return []; } - - default: - // For agents without model discovery implemented, return empty array - logger.debug(`No model discovery implemented for ${agentId}`, LOG_CONTEXT); - return []; + } catch (error) { + logger.error(`Model discovery threw exception for ${agentId}`, LOG_CONTEXT, { error }); + return []; } } } diff --git a/src/main/agents/path-prober.ts b/src/main/agents/path-prober.ts index fc935038d..48e6636fe 100644 --- a/src/main/agents/path-prober.ts +++ b/src/main/agents/path-prober.ts @@ -1,9 +1,17 @@ /** - * Path Prober - Platform-specific binary detection + * Binary Path Detection Utilities * - * Handles detection of agent binaries on Windows and Unix-like systems. * Packaged Electron apps don't inherit shell environment, so we need to * probe known installation paths directly. + * + * Detection Strategy: + * 1. Direct file system probing of known installation paths (fastest, most reliable) + * 2. Fall back to which/where command with expanded PATH + * + * This two-tier approach ensures we find binaries even when: + * - PATH is not inherited correctly + * - Binaries are in non-standard locations + * - Shell initialization files (.bashrc, .zshrc) aren't sourced */ import * as os from 'os'; @@ -186,7 +194,8 @@ export async function checkCustomPath(customPath: string): Promise { const pathsToCheck = getWindowsKnownPaths(binaryName); - for (const probePath of pathsToCheck) { - try { + if (pathsToCheck.length === 0) { + return null; + } + + // Check all paths in parallel for performance + const results = await Promise.allSettled( + pathsToCheck.map(async (probePath) => { await fs.promises.access(probePath, fs.constants.F_OK); - logger.debug(`Direct probe found ${binaryName}`, LOG_CONTEXT, { path: probePath }); return probePath; - } catch { - // Path doesn't exist, continue to next + }) + ); + + // Return the first successful result (maintains priority order from pathsToCheck) + for (let i = 0; i < results.length; i++) { + const result = results[i]; + if (result.status === 'fulfilled') { + logger.debug(`Direct probe found ${binaryName}`, LOG_CONTEXT, { path: result.value }); + return result.value; } } @@ -356,19 +378,32 @@ function getUnixKnownPaths(binaryName: string): string[] { * On macOS/Linux, directly probe known installation paths for a binary. * This is necessary because packaged Electron apps don't inherit shell aliases, * and 'which' may fail to find binaries in non-standard locations. - * Returns the first existing executable path found. + * Returns the first existing executable path found (in priority order). + * + * Uses parallel probing for performance on slow file systems. */ export async function probeUnixPaths(binaryName: string): Promise { const pathsToCheck = getUnixKnownPaths(binaryName); - for (const probePath of pathsToCheck) { - try { + if (pathsToCheck.length === 0) { + return null; + } + + // Check all paths in parallel for performance + const results = await Promise.allSettled( + pathsToCheck.map(async (probePath) => { // Check both existence and executability await fs.promises.access(probePath, fs.constants.F_OK | fs.constants.X_OK); - logger.debug(`Direct probe found ${binaryName}`, LOG_CONTEXT, { path: probePath }); return probePath; - } catch { - // Path doesn't exist or isn't executable, continue to next + }) + ); + + // Return the first successful result (maintains priority order from pathsToCheck) + for (let i = 0; i < results.length; i++) { + const result = results[i]; + if (result.status === 'fulfilled') { + logger.debug(`Direct probe found ${binaryName}`, LOG_CONTEXT, { path: result.value }); + return result.value; } } diff --git a/src/main/utils/agent-args.ts b/src/main/utils/agent-args.ts index f9ea0587b..d71c815e2 100644 --- a/src/main/utils/agent-args.ts +++ b/src/main/utils/agent-args.ts @@ -118,7 +118,10 @@ export function applyAgentConfigOverrides( : option.default; } - finalArgs = [...finalArgs, ...option.argBuilder(value)]; + // Type assertion needed because AgentConfigOption is a discriminated union + // and we're handling all types generically here + const argBuilderFn = option.argBuilder as (value: unknown) => string[]; + finalArgs = [...finalArgs, ...argBuilderFn(value)]; } } @@ -179,9 +182,11 @@ export function getContextWindowValue( } // Fall back to agent-level config const contextWindowOption = agent?.configOptions?.find( - (option) => option.key === 'contextWindow' + (option) => option.key === 'contextWindow' && option.type === 'number' ); - const contextWindowDefault = contextWindowOption?.default ?? 0; + // Extract default value, ensuring it's a number (contextWindow should always be a number config) + const defaultValue = contextWindowOption?.default; + const contextWindowDefault = typeof defaultValue === 'number' ? defaultValue : 0; return typeof agentConfigValues.contextWindow === 'number' ? agentConfigValues.contextWindow : contextWindowDefault; From a64b20183480adec7cac8590a3a5c641d20bebe4 Mon Sep 17 00:00:00 2001 From: Raza Rauf Date: Fri, 30 Jan 2026 02:10:47 +0500 Subject: [PATCH 4/4] refactor: extract common path builders in path-prober to reduce duplication Addresses code review suggestion to reduce repeated path patterns across binary definitions by extracting shared builders (npmGlobal, localBin, homebrew, wingetLinks, etc.) as closures within each platform function. --- src/main/agents/path-prober.ts | 104 ++++++++++++++++++--------------- 1 file changed, 58 insertions(+), 46 deletions(-) diff --git a/src/main/agents/path-prober.ts b/src/main/agents/path-prober.ts index 48e6636fe..07c3278da 100644 --- a/src/main/agents/path-prober.ts +++ b/src/main/agents/path-prober.ts @@ -211,27 +211,42 @@ function getWindowsKnownPaths(binaryName: string): string[] { const localAppData = process.env.LOCALAPPDATA || path.join(home, 'AppData', 'Local'); const programFiles = process.env.ProgramFiles || 'C:\\Program Files'; + // Common path builders to reduce duplication across binary definitions + const npmGlobal = (bin: string) => [ + path.join(appData, 'npm', `${bin}.cmd`), + path.join(localAppData, 'npm', `${bin}.cmd`), + ]; + const localBin = (bin: string) => [path.join(home, '.local', 'bin', `${bin}.exe`)]; + const wingetLinks = (bin: string) => [ + path.join(localAppData, 'Microsoft', 'WinGet', 'Links', `${bin}.exe`), + path.join(programFiles, 'WinGet', 'Links', `${bin}.exe`), + ]; + const goBin = (bin: string) => [path.join(home, 'go', 'bin', `${bin}.exe`)]; + const pythonScripts = (bin: string) => [ + path.join(appData, 'Python', 'Scripts', `${bin}.exe`), + path.join(localAppData, 'Programs', 'Python', 'Python312', 'Scripts', `${bin}.exe`), + path.join(localAppData, 'Programs', 'Python', 'Python311', 'Scripts', `${bin}.exe`), + path.join(localAppData, 'Programs', 'Python', 'Python310', 'Scripts', `${bin}.exe`), + ]; + // Define known installation paths for each binary, in priority order // Prefer .exe (standalone installers) over .cmd (npm wrappers) const knownPaths: Record = { claude: [ // PowerShell installer (primary method) - installs claude.exe - path.join(home, '.local', 'bin', 'claude.exe'), + ...localBin('claude'), // Winget installation - path.join(localAppData, 'Microsoft', 'WinGet', 'Links', 'claude.exe'), - path.join(programFiles, 'WinGet', 'Links', 'claude.exe'), + ...wingetLinks('claude'), // npm global installation - creates .cmd wrapper - path.join(appData, 'npm', 'claude.cmd'), - path.join(localAppData, 'npm', 'claude.cmd'), + ...npmGlobal('claude'), // WindowsApps (Microsoft Store style) path.join(localAppData, 'Microsoft', 'WindowsApps', 'claude.exe'), ], codex: [ // npm global installation (primary method for Codex) - path.join(appData, 'npm', 'codex.cmd'), - path.join(localAppData, 'npm', 'codex.cmd'), + ...npmGlobal('codex'), // Possible standalone in future - path.join(home, '.local', 'bin', 'codex.exe'), + ...localBin('codex'), ], opencode: [ // Scoop installation (recommended for OpenCode) @@ -244,21 +259,17 @@ function getWindowsKnownPaths(binaryName: string): string[] { 'opencode.exe' ), // Go install - path.join(home, 'go', 'bin', 'opencode.exe'), + ...goBin('opencode'), // npm (has known issues on Windows, but check anyway) - path.join(appData, 'npm', 'opencode.cmd'), + ...npmGlobal('opencode'), ], gemini: [ // npm global installation - path.join(appData, 'npm', 'gemini.cmd'), - path.join(localAppData, 'npm', 'gemini.cmd'), + ...npmGlobal('gemini'), ], aider: [ // pip installation - path.join(appData, 'Python', 'Scripts', 'aider.exe'), - path.join(localAppData, 'Programs', 'Python', 'Python312', 'Scripts', 'aider.exe'), - path.join(localAppData, 'Programs', 'Python', 'Python311', 'Scripts', 'aider.exe'), - path.join(localAppData, 'Programs', 'Python', 'Python310', 'Scripts', 'aider.exe'), + ...pythonScripts('aider'), ], }; @@ -310,34 +321,37 @@ function getUnixKnownPaths(binaryName: string): string[] { // Get dynamic paths from Node version managers (nvm, fnm, volta, etc.) const versionManagerPaths = detectNodeVersionManagerBinPaths(); + // Common path builders to reduce duplication across binary definitions + const homebrew = (bin: string) => [`/opt/homebrew/bin/${bin}`, `/usr/local/bin/${bin}`]; + const localBin = (bin: string) => [path.join(home, '.local', 'bin', bin)]; + const npmGlobal = (bin: string) => [path.join(home, '.npm-global', 'bin', bin)]; + const nodeVersionManagers = (bin: string) => versionManagerPaths.map((p) => path.join(p, bin)); + // Define known installation paths for each binary, in priority order const knownPaths: Record = { claude: [ - // Claude Code default installation location (irm https://claude.ai/install.ps1 equivalent on macOS) + // Claude Code default installation location path.join(home, '.claude', 'local', 'claude'), // User local bin (pip, manual installs) - path.join(home, '.local', 'bin', 'claude'), - // Homebrew on Apple Silicon - '/opt/homebrew/bin/claude', - // Homebrew on Intel Mac - '/usr/local/bin/claude', + ...localBin('claude'), + // Homebrew (Apple Silicon + Intel) + ...homebrew('claude'), // npm global with custom prefix - path.join(home, '.npm-global', 'bin', 'claude'), + ...npmGlobal('claude'), // User bin directory path.join(home, 'bin', 'claude'), - // Add paths from Node version managers (nvm, fnm, volta, etc.) - ...versionManagerPaths.map((p) => path.join(p, 'claude')), + // Node version managers (nvm, fnm, volta, etc.) + ...nodeVersionManagers('claude'), ], codex: [ // User local bin - path.join(home, '.local', 'bin', 'codex'), + ...localBin('codex'), // Homebrew paths - '/opt/homebrew/bin/codex', - '/usr/local/bin/codex', + ...homebrew('codex'), // npm global - path.join(home, '.npm-global', 'bin', 'codex'), - // Add paths from Node version managers (nvm, fnm, volta, etc.) - ...versionManagerPaths.map((p) => path.join(p, 'codex')), + ...npmGlobal('codex'), + // Node version managers (nvm, fnm, volta, etc.) + ...nodeVersionManagers('codex'), ], opencode: [ // OpenCode installer default location @@ -345,29 +359,27 @@ function getUnixKnownPaths(binaryName: string): string[] { // Go install location path.join(home, 'go', 'bin', 'opencode'), // User local bin - path.join(home, '.local', 'bin', 'opencode'), + ...localBin('opencode'), // Homebrew paths - '/opt/homebrew/bin/opencode', - '/usr/local/bin/opencode', - // Add paths from Node version managers (nvm, fnm, volta, etc.) - ...versionManagerPaths.map((p) => path.join(p, 'opencode')), + ...homebrew('opencode'), + // Node version managers (nvm, fnm, volta, etc.) + ...nodeVersionManagers('opencode'), ], gemini: [ // npm global paths - path.join(home, '.npm-global', 'bin', 'gemini'), - '/opt/homebrew/bin/gemini', - '/usr/local/bin/gemini', - // Add paths from Node version managers (nvm, fnm, volta, etc.) - ...versionManagerPaths.map((p) => path.join(p, 'gemini')), + ...npmGlobal('gemini'), + // Homebrew paths + ...homebrew('gemini'), + // Node version managers (nvm, fnm, volta, etc.) + ...nodeVersionManagers('gemini'), ], aider: [ // pip installation - path.join(home, '.local', 'bin', 'aider'), + ...localBin('aider'), // Homebrew paths - '/opt/homebrew/bin/aider', - '/usr/local/bin/aider', - // Add paths from Node version managers (in case installed via npm) - ...versionManagerPaths.map((p) => path.join(p, 'aider')), + ...homebrew('aider'), + // Node version managers (in case installed via npm) + ...nodeVersionManagers('aider'), ], };