Skip to content

Commit 46e7a10

Browse files
committed
feat(core): add service layer with 7 services for testability
Create comprehensive service layer to eliminate code duplication between CLI and MCP, improve testability, and provide consistent APIs. Services Created: - StatsService: Repository statistics retrieval - HealthService: Component health checks - MetricsService: Analytics and metrics queries - SearchService: Semantic code search (eliminates 4x duplication) - GitHubService: GitHub operations (eliminates 3x duplication) - CoordinatorService: Subagent setup (eliminates 100% duplicate code) - GitHistoryService: Git history indexing and search Key Improvements: - Dependency injection pattern throughout (no constructor mocking) - 62 comprehensive tests with 100% coverage - Zero biome-ignore comments - Consistent behavior across CLI and MCP - Single source of truth for each operation Technical Details: - All services use factory pattern for dependency injection - Dynamic imports to avoid cross-package TypeScript issues - Clean separation of concerns - Proper resource cleanup (close methods) - Configurable options with sensible defaults Files Changed: - packages/core/src/services/ (7 services + 7 test files) - packages/core/src/index.ts (export services) - packages/core/src/storage/path.ts (add metrics path) Stats: - Production code: 1,374 lines - Test code: 1,681 lines - Tests: 62 passing - Build: ✅ Clean (5.046s) - Lint: ✅ 108 files, 0 errors Closes #148
1 parent 54efb97 commit 46e7a10

18 files changed

Lines changed: 3180 additions & 4 deletions

packages/core/src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ export * from './map';
1010
export * from './metrics';
1111
export * from './observability';
1212
export * from './scanner';
13+
export * from './services';
1314
export * from './storage';
1415
export * from './utils';
1516
export * from './vector';
Lines changed: 194 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,194 @@
1+
/**
2+
* Tests for CoordinatorService
3+
*/
4+
5+
import { describe, expect, it, vi } from 'vitest';
6+
import type { RepositoryIndexer } from '../../indexer/index.js';
7+
import type { SubagentCoordinator } from '../coordinator-service.js';
8+
import { CoordinatorService } from '../coordinator-service.js';
9+
10+
describe('CoordinatorService', () => {
11+
describe('createCoordinator', () => {
12+
it('should create and configure coordinator with all agents', async () => {
13+
const mockIndexer = {} as RepositoryIndexer;
14+
15+
const mockContextManager = {
16+
setIndexer: vi.fn(),
17+
};
18+
19+
const mockCoordinator: SubagentCoordinator = {
20+
registerAgent: vi.fn().mockResolvedValue(undefined),
21+
getContextManager: vi.fn().mockReturnValue(mockContextManager),
22+
shutdown: vi.fn().mockResolvedValue(undefined),
23+
};
24+
25+
const mockExplorerAgent = { name: 'explorer' };
26+
const mockPlannerAgent = { name: 'planner' };
27+
const mockPrAgent = { name: 'pr' };
28+
29+
const factories = {
30+
createCoordinator: vi.fn().mockResolvedValue(mockCoordinator),
31+
createExplorerAgent: vi.fn().mockResolvedValue(mockExplorerAgent),
32+
createPlannerAgent: vi.fn().mockResolvedValue(mockPlannerAgent),
33+
createPrAgent: vi.fn().mockResolvedValue(mockPrAgent),
34+
};
35+
36+
const service = new CoordinatorService(
37+
{
38+
repositoryPath: '/test/repo',
39+
maxConcurrentTasks: 10,
40+
defaultMessageTimeout: 60000,
41+
logLevel: 'debug',
42+
},
43+
factories
44+
);
45+
46+
const coordinator = await service.createCoordinator(mockIndexer);
47+
48+
// Verify coordinator factory called with correct config
49+
expect(factories.createCoordinator).toHaveBeenCalledWith({
50+
maxConcurrentTasks: 10,
51+
defaultMessageTimeout: 60000,
52+
logLevel: 'debug',
53+
});
54+
55+
// Verify context manager setup
56+
expect(mockCoordinator.getContextManager).toHaveBeenCalledOnce();
57+
expect(mockContextManager.setIndexer).toHaveBeenCalledWith(mockIndexer);
58+
59+
// Verify all agents created
60+
expect(factories.createExplorerAgent).toHaveBeenCalledOnce();
61+
expect(factories.createPlannerAgent).toHaveBeenCalledOnce();
62+
expect(factories.createPrAgent).toHaveBeenCalledOnce();
63+
64+
// Verify all agents registered
65+
expect(mockCoordinator.registerAgent).toHaveBeenCalledTimes(3);
66+
expect(mockCoordinator.registerAgent).toHaveBeenCalledWith(mockExplorerAgent);
67+
expect(mockCoordinator.registerAgent).toHaveBeenCalledWith(mockPlannerAgent);
68+
expect(mockCoordinator.registerAgent).toHaveBeenCalledWith(mockPrAgent);
69+
70+
expect(coordinator).toBe(mockCoordinator);
71+
});
72+
73+
it('should use default configuration when not provided', async () => {
74+
const mockIndexer = {} as RepositoryIndexer;
75+
const mockContextManager = { setIndexer: vi.fn() };
76+
const mockCoordinator: SubagentCoordinator = {
77+
registerAgent: vi.fn().mockResolvedValue(undefined),
78+
getContextManager: vi.fn().mockReturnValue(mockContextManager),
79+
shutdown: vi.fn().mockResolvedValue(undefined),
80+
};
81+
82+
const factories = {
83+
createCoordinator: vi.fn().mockResolvedValue(mockCoordinator),
84+
createExplorerAgent: vi.fn().mockResolvedValue({}),
85+
createPlannerAgent: vi.fn().mockResolvedValue({}),
86+
createPrAgent: vi.fn().mockResolvedValue({}),
87+
};
88+
89+
const service = new CoordinatorService({ repositoryPath: '/test/repo' }, factories);
90+
91+
await service.createCoordinator(mockIndexer);
92+
93+
// Verify default configuration used
94+
expect(factories.createCoordinator).toHaveBeenCalledWith({
95+
maxConcurrentTasks: 5, // default
96+
defaultMessageTimeout: 30000, // default
97+
logLevel: 'info', // default
98+
});
99+
});
100+
101+
it('should register agents in order', async () => {
102+
const mockIndexer = {} as RepositoryIndexer;
103+
const mockContextManager = { setIndexer: vi.fn() };
104+
105+
const registrationOrder: string[] = [];
106+
const mockCoordinator: SubagentCoordinator = {
107+
registerAgent: vi.fn().mockImplementation((agent: { name: string }) => {
108+
registrationOrder.push(agent.name);
109+
return Promise.resolve();
110+
}),
111+
getContextManager: vi.fn().mockReturnValue(mockContextManager),
112+
shutdown: vi.fn().mockResolvedValue(undefined),
113+
};
114+
115+
const factories = {
116+
createCoordinator: vi.fn().mockResolvedValue(mockCoordinator),
117+
createExplorerAgent: vi.fn().mockResolvedValue({ name: 'explorer' }),
118+
createPlannerAgent: vi.fn().mockResolvedValue({ name: 'planner' }),
119+
createPrAgent: vi.fn().mockResolvedValue({ name: 'pr' }),
120+
};
121+
122+
const service = new CoordinatorService({ repositoryPath: '/test/repo' }, factories);
123+
124+
await service.createCoordinator(mockIndexer);
125+
126+
expect(registrationOrder).toEqual(['explorer', 'planner', 'pr']);
127+
});
128+
});
129+
130+
describe('updateConfig', () => {
131+
it('should update configuration', () => {
132+
const service = new CoordinatorService({
133+
repositoryPath: '/test/repo',
134+
maxConcurrentTasks: 5,
135+
defaultMessageTimeout: 30000,
136+
logLevel: 'info',
137+
});
138+
139+
service.updateConfig({
140+
maxConcurrentTasks: 10,
141+
logLevel: 'debug',
142+
});
143+
144+
const config = service.getConfig();
145+
expect(config.maxConcurrentTasks).toBe(10);
146+
expect(config.defaultMessageTimeout).toBe(30000); // unchanged
147+
expect(config.logLevel).toBe('debug');
148+
});
149+
150+
it('should partially update configuration', () => {
151+
const service = new CoordinatorService({
152+
repositoryPath: '/test/repo',
153+
});
154+
155+
service.updateConfig({ maxConcurrentTasks: 15 });
156+
157+
const config = service.getConfig();
158+
expect(config.maxConcurrentTasks).toBe(15);
159+
expect(config.defaultMessageTimeout).toBe(30000); // default
160+
expect(config.logLevel).toBe('info'); // default
161+
});
162+
});
163+
164+
describe('getConfig', () => {
165+
it('should return current configuration', () => {
166+
const service = new CoordinatorService({
167+
repositoryPath: '/test/repo',
168+
maxConcurrentTasks: 7,
169+
defaultMessageTimeout: 45000,
170+
logLevel: 'warn',
171+
});
172+
173+
const config = service.getConfig();
174+
175+
expect(config).toEqual({
176+
maxConcurrentTasks: 7,
177+
defaultMessageTimeout: 45000,
178+
logLevel: 'warn',
179+
});
180+
});
181+
182+
it('should return default configuration when not specified', () => {
183+
const service = new CoordinatorService({ repositoryPath: '/test/repo' });
184+
185+
const config = service.getConfig();
186+
187+
expect(config).toEqual({
188+
maxConcurrentTasks: 5,
189+
defaultMessageTimeout: 30000,
190+
logLevel: 'info',
191+
});
192+
});
193+
});
194+
});

0 commit comments

Comments
 (0)