Skip to content

Commit 6256e11

Browse files
committed
refactor(agent-manager): normalize agent types and remove display fields
1 parent 19d606b commit 6256e11

10 files changed

Lines changed: 84 additions & 185 deletions

File tree

docs/ai/design/feature-agent-manager.md

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -65,11 +65,10 @@ packages/agent-manager/
6565

6666
## Data Models
6767

68-
All types are extracted from the existing `AgentAdapter.ts` without changes:
68+
Types are adapted for a data-first package contract:
6969

70-
- **AgentType**: `'Claude Code' | 'Gemini CLI' | 'Codex' | 'Other'`
70+
- **AgentType**: `'claude' | 'gemini_cli' | 'codex' | 'other'`
7171
- **AgentStatus**: Enum (`RUNNING`, `WAITING`, `IDLE`, `UNKNOWN`)
72-
- **StatusConfig**: `{ emoji, label, color }`
7372
- **AgentInfo**: Full agent information (name, type, status, pid, projectPath, sessionId, slug, lastActive, etc.)
7473
- **ProcessInfo**: `{ pid, command, cwd, tty }`
7574
- **AgentAdapter**: Interface with `type`, `detectAgents()`, `canHandle()`
@@ -86,8 +85,8 @@ export { AgentManager } from './AgentManager';
8685
// Adapters
8786
export { ClaudeCodeAdapter } from './adapters/ClaudeCodeAdapter';
8887
export type { AgentAdapter } from './adapters/AgentAdapter';
89-
export { AgentStatus, STATUS_CONFIG } from './adapters/AgentAdapter';
90-
export type { AgentType, AgentInfo, ProcessInfo, StatusConfig } from './adapters/AgentAdapter';
88+
export { AgentStatus } from './adapters/AgentAdapter';
89+
export type { AgentType, AgentInfo, ProcessInfo } from './adapters/AgentAdapter';
9190

9291
// Terminal
9392
export { TerminalFocusManager } from './terminal/TerminalFocusManager';
@@ -109,10 +108,16 @@ manager.registerAdapter(new ClaudeCodeAdapter());
109108

110109
const agents = await manager.listAgents();
111110
agents.forEach(agent => {
112-
console.log(`${agent.name}: ${agent.statusDisplay}`);
111+
console.log(`${agent.name}: ${agent.status}`);
113112
});
114113
```
115114

115+
### Migration Notes
116+
117+
- `AgentType` values are now normalized codes (`claude`, `gemini_cli`, `codex`, `other`)
118+
- `AgentInfo` no longer includes UI/display fields (`statusDisplay`, `lastActiveDisplay`)
119+
- `STATUS_CONFIG` / `StatusConfig` were removed; consumers should map presentation in their own layer
120+
116121
## Component Breakdown
117122

118123
### 1. AgentManager (core orchestrator)
@@ -126,9 +131,9 @@ agents.forEach(agent => {
126131
### 2. AgentAdapter + Types (interface layer)
127132
- Interface contract for adapters
128133
- Type definitions and enums
129-
- Status display configuration
134+
- Normalized agent type codes for machine-friendly integrations
130135
- **Extracted from**: `packages/cli/src/lib/adapters/AgentAdapter.ts`
131-
- **Changes**: None — direct copy
136+
- **Changes**: Agent type literals normalized; display-oriented fields removed from core model
132137

133138
### 3. ClaudeCodeAdapter (concrete adapter)
134139
- Claude Code process detection via `ps aux`

docs/ai/implementation/feature-agent-manager.md

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,10 +91,18 @@ Completed on February 25, 2026 in worktree `feature-agent-manager`.
9191
- `src/__tests__/adapters/ClaudeCodeAdapter.test.ts`
9292

9393
Validation:
94-
- `npm run lint` passes (warnings only from inherited `any` usage in extracted code)
94+
- `npm run lint` passes
9595
- `npm run typecheck` passes
9696
- `npm run build` passes
97-
- `npm run test` passes (43 tests)
97+
- `npm run test` passes (38 tests)
98+
99+
Data-model refinements (February 25, 2026):
100+
- Normalized `AgentType` to code-style values: `claude`, `gemini_cli`, `codex`, `other`
101+
- Removed display-oriented contract elements from package API:
102+
- Removed `STATUS_CONFIG` and `StatusConfig`
103+
- Removed `AgentInfo.statusDisplay`
104+
- Removed `AgentInfo.lastActiveDisplay`
105+
- Updated `ClaudeCodeAdapter` to return data-only fields (`status`, `lastActive`, `summary`) without UI formatting
98106

99107
## Phase 6 Check Implementation (February 25, 2026)
100108

@@ -150,3 +158,15 @@ Validation:
150158

151159
- No remaining blocking correctness or security issues in `packages/agent-manager`.
152160
- Feature is ready for commit/PR from a code review perspective.
161+
162+
## Code Review Continuation (February 25, 2026)
163+
164+
### Findings
165+
166+
1. **No new blocking issues** after `AgentType` normalization and display-field removal.
167+
2. **Compatibility note**: this is an intentional contract change for package consumers (type literals and removed display fields/constants).
168+
169+
### Documentation Updates Applied
170+
171+
- Requirements/design docs updated to describe the data-first API boundary.
172+
- Added explicit migration notes for callers formatting status/time displays externally.

docs/ai/requirements/feature-agent-manager.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ Agent detection and management code (AgentManager, adapters, process utilities,
2828
- Improve code quality during extraction (better error handling, consistent patterns)
2929
- Include TerminalFocusManager as an optional export (terminal focus capability)
3030
- Maintain backward compatibility — CLI should still function identically after extraction
31+
- Keep package contracts data-first (machine-friendly enums/codes, no UI display formatting)
3132

3233
### Non-Goals
3334
- Modifying the CLI `agent` command behavior or UI (stays in CLI, just re-imports)
@@ -66,6 +67,7 @@ Agent detection and management code (AgentManager, adapters, process utilities,
6667
- The CLI package will NOT be modified to import from the new package in this iteration
6768
- TerminalFocusManager is macOS-specific and should be documented as such
6869
- Process detection utilities (`ps aux`, `lsof`) are Unix/macOS-specific
70+
- Any consumer-facing formatting (emoji/labels/relative-time strings) is the responsibility of callers, not this package
6971

7072
## Questions & Open Items
7173

docs/ai/testing/feature-agent-manager.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ description: Testing approach for the @ai-devkit/agent-manager package
2929
### ClaudeCodeAdapter (`src/__tests__/adapters/ClaudeCodeAdapter.test.ts`)
3030
- [x] Adapter type and canHandle()
3131
- [x] Agent detection (mocked process/session data)
32-
- [x] Helper methods: truncateSummary(), getRelativeTime(), determineStatus(), generateAgentName()
32+
- [x] Helper methods: determineStatus(), generateAgentName()
3333

3434
## Test Data
3535

@@ -48,5 +48,5 @@ description: Testing approach for the @ai-devkit/agent-manager package
4848
Executed on February 25, 2026:
4949

5050
- `npm run test` passed
51-
- Total: 44 tests passed, 2 suites passed
51+
- Total: 38 tests passed, 2 suites passed
5252
- Claude adapter unit tests are deterministic and run without relying on host process permissions

packages/agent-manager/src/__tests__/AgentManager.test.ts

Lines changed: 32 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -39,16 +39,14 @@ class MockAdapter implements AgentAdapter {
3939
function createMockAgent(overrides: Partial<AgentInfo> = {}): AgentInfo {
4040
return {
4141
name: 'test-agent',
42-
type: 'Claude Code',
42+
type: 'claude',
4343
status: AgentStatus.RUNNING,
44-
statusDisplay: '🟢 run',
4544
summary: 'Test summary',
4645
pid: 12345,
4746
projectPath: '/test/path',
4847
sessionId: 'test-session-id',
4948
slug: 'test-slug',
5049
lastActive: new Date(),
51-
lastActiveDisplay: 'just now',
5250
...overrides,
5351
};
5452
}
@@ -62,47 +60,47 @@ describe('AgentManager', () => {
6260

6361
describe('registerAdapter', () => {
6462
it('should register a new adapter', () => {
65-
const adapter = new MockAdapter('Claude Code');
63+
const adapter = new MockAdapter('claude');
6664

6765
manager.registerAdapter(adapter);
6866

69-
expect(manager.hasAdapter('Claude Code')).toBe(true);
67+
expect(manager.hasAdapter('claude')).toBe(true);
7068
expect(manager.getAdapterCount()).toBe(1);
7169
});
7270

7371
it('should throw error when registering duplicate adapter type', () => {
74-
const adapter1 = new MockAdapter('Claude Code');
75-
const adapter2 = new MockAdapter('Claude Code');
72+
const adapter1 = new MockAdapter('claude');
73+
const adapter2 = new MockAdapter('claude');
7674

7775
manager.registerAdapter(adapter1);
7876

7977
expect(() => manager.registerAdapter(adapter2)).toThrow(
80-
'Adapter for type "Claude Code" is already registered'
78+
'Adapter for type "claude" is already registered'
8179
);
8280
});
8381

8482
it('should allow registering multiple different adapter types', () => {
85-
const adapter1 = new MockAdapter('Claude Code');
86-
const adapter2 = new MockAdapter('Gemini CLI');
83+
const adapter1 = new MockAdapter('claude');
84+
const adapter2 = new MockAdapter('gemini_cli');
8785

8886
manager.registerAdapter(adapter1);
8987
manager.registerAdapter(adapter2);
9088

9189
expect(manager.getAdapterCount()).toBe(2);
92-
expect(manager.hasAdapter('Claude Code')).toBe(true);
93-
expect(manager.hasAdapter('Gemini CLI')).toBe(true);
90+
expect(manager.hasAdapter('claude')).toBe(true);
91+
expect(manager.hasAdapter('gemini_cli')).toBe(true);
9492
});
9593
});
9694

9795
describe('unregisterAdapter', () => {
9896
it('should unregister an existing adapter', () => {
99-
const adapter = new MockAdapter('Claude Code');
97+
const adapter = new MockAdapter('claude');
10098
manager.registerAdapter(adapter);
10199

102-
const removed = manager.unregisterAdapter('Claude Code');
100+
const removed = manager.unregisterAdapter('claude');
103101

104102
expect(removed).toBe(true);
105-
expect(manager.hasAdapter('Claude Code')).toBe(false);
103+
expect(manager.hasAdapter('claude')).toBe(false);
106104
expect(manager.getAdapterCount()).toBe(0);
107105
});
108106

@@ -119,8 +117,8 @@ describe('AgentManager', () => {
119117
});
120118

121119
it('should return all registered adapters', () => {
122-
const adapter1 = new MockAdapter('Claude Code');
123-
const adapter2 = new MockAdapter('Gemini CLI');
120+
const adapter1 = new MockAdapter('claude');
121+
const adapter2 = new MockAdapter('gemini_cli');
124122

125123
manager.registerAdapter(adapter1);
126124
manager.registerAdapter(adapter2);
@@ -134,12 +132,12 @@ describe('AgentManager', () => {
134132

135133
describe('hasAdapter', () => {
136134
it('should return true for registered adapter', () => {
137-
manager.registerAdapter(new MockAdapter('Claude Code'));
138-
expect(manager.hasAdapter('Claude Code')).toBe(true);
135+
manager.registerAdapter(new MockAdapter('claude'));
136+
expect(manager.hasAdapter('claude')).toBe(true);
139137
});
140138

141139
it('should return false for non-registered adapter', () => {
142-
expect(manager.hasAdapter('Claude Code')).toBe(false);
140+
expect(manager.hasAdapter('claude')).toBe(false);
143141
});
144142
});
145143

@@ -154,7 +152,7 @@ describe('AgentManager', () => {
154152
createMockAgent({ name: 'agent1' }),
155153
createMockAgent({ name: 'agent2' }),
156154
];
157-
const adapter = new MockAdapter('Claude Code', mockAgents);
155+
const adapter = new MockAdapter('claude', mockAgents);
158156

159157
manager.registerAdapter(adapter);
160158
const agents = await manager.listAgents();
@@ -165,11 +163,11 @@ describe('AgentManager', () => {
165163
});
166164

167165
it('should aggregate agents from multiple adapters', async () => {
168-
const claudeAgents = [createMockAgent({ name: 'claude-agent', type: 'Claude Code' })];
169-
const geminiAgents = [createMockAgent({ name: 'gemini-agent', type: 'Gemini CLI' })];
166+
const claudeAgents = [createMockAgent({ name: 'claude-agent', type: 'claude' })];
167+
const geminiAgents = [createMockAgent({ name: 'gemini-agent', type: 'gemini_cli' })];
170168

171-
manager.registerAdapter(new MockAdapter('Claude Code', claudeAgents));
172-
manager.registerAdapter(new MockAdapter('Gemini CLI', geminiAgents));
169+
manager.registerAdapter(new MockAdapter('claude', claudeAgents));
170+
manager.registerAdapter(new MockAdapter('gemini_cli', geminiAgents));
173171

174172
const agents = await manager.listAgents();
175173

@@ -185,7 +183,7 @@ describe('AgentManager', () => {
185183
createMockAgent({ name: 'running-agent', status: AgentStatus.RUNNING }),
186184
createMockAgent({ name: 'unknown-agent', status: AgentStatus.UNKNOWN }),
187185
];
188-
const adapter = new MockAdapter('Claude Code', mockAgents);
186+
const adapter = new MockAdapter('claude', mockAgents);
189187

190188
manager.registerAdapter(adapter);
191189
const agents = await manager.listAgents();
@@ -197,10 +195,10 @@ describe('AgentManager', () => {
197195
});
198196

199197
it('should handle adapter errors gracefully', async () => {
200-
const goodAdapter = new MockAdapter('Claude Code', [
198+
const goodAdapter = new MockAdapter('claude', [
201199
createMockAgent({ name: 'good-agent' }),
202200
]);
203-
const badAdapter = new MockAdapter('Gemini CLI', [], true); // Will fail
201+
const badAdapter = new MockAdapter('gemini_cli', [], true); // Will fail
204202

205203
manager.registerAdapter(goodAdapter);
206204
manager.registerAdapter(badAdapter);
@@ -213,8 +211,8 @@ describe('AgentManager', () => {
213211
});
214212

215213
it('should return empty array when all adapters fail', async () => {
216-
const adapter1 = new MockAdapter('Claude Code', [], true);
217-
const adapter2 = new MockAdapter('Gemini CLI', [], true);
214+
const adapter1 = new MockAdapter('claude', [], true);
215+
const adapter2 = new MockAdapter('gemini_cli', [], true);
218216

219217
manager.registerAdapter(adapter1);
220218
manager.registerAdapter(adapter2);
@@ -230,18 +228,18 @@ describe('AgentManager', () => {
230228
});
231229

232230
it('should return correct count', () => {
233-
manager.registerAdapter(new MockAdapter('Claude Code'));
231+
manager.registerAdapter(new MockAdapter('claude'));
234232
expect(manager.getAdapterCount()).toBe(1);
235233

236-
manager.registerAdapter(new MockAdapter('Gemini CLI'));
234+
manager.registerAdapter(new MockAdapter('gemini_cli'));
237235
expect(manager.getAdapterCount()).toBe(2);
238236
});
239237
});
240238

241239
describe('clear', () => {
242240
it('should remove all adapters', () => {
243-
manager.registerAdapter(new MockAdapter('Claude Code'));
244-
manager.registerAdapter(new MockAdapter('Gemini CLI'));
241+
manager.registerAdapter(new MockAdapter('claude'));
242+
manager.registerAdapter(new MockAdapter('gemini_cli'));
245243

246244
manager.clear();
247245

0 commit comments

Comments
 (0)