Skip to content

Commit e3e2d52

Browse files
committed
refactor(subagents): add Zod validation to message passing (Priority 2)
Following TypeScript Standards Rule #2: No Type Assertions Without Validation Changes: - Created message schemas (packages/subagents/src/schemas/messages.ts) * PlanningRequestSchema for planner agent * ExplorationRequestSchema (discriminated union) for explorer agent * GitHubContextRequestSchema for GitHub agent * 22 comprehensive tests with 100% coverage - Replaced 3 type assertions with validated functions: * planner/index.ts: validatePlanningRequest() * explorer/index.ts: validateExplorationRequest() * github/agent.ts: validateGitHubContextRequest() - Updated 3 tests to expect 'error' messages for invalid requests * planner/__tests__/index.test.ts * explorer/__tests__/index.test.ts * coordinator/__tests__/coordinator.integration.test.ts Impact: - Zero type assertions in inter-agent communication (MEDIUM RISK boundary) - Better error messages: 'Invalid planning request: issueNumber: ...' - Bulletproof against malformed coordinator messages - All 1739 tests passing ✅ Scope: Inter-agent message validation
1 parent e26c035 commit e3e2d52

File tree

8 files changed

+406
-8
lines changed

8 files changed

+406
-8
lines changed

packages/subagents/src/coordinator/__tests__/coordinator.integration.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -186,11 +186,11 @@ describe('Coordinator → Explorer Integration', () => {
186186
});
187187

188188
expect(response).toBeDefined();
189-
expect(response?.type).toBe('response');
189+
expect(response?.type).toBe('error');
190190

191191
const result = response?.payload as { error?: string };
192192
expect(result.error).toBeDefined();
193-
expect(result.error).toContain('Unknown action');
193+
expect(result.error).toContain('Invalid exploration request');
194194
});
195195

196196
it('should handle non-existent agent gracefully', async () => {

packages/subagents/src/explorer/__tests__/index.test.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -579,10 +579,11 @@ describe('ExplorerAgent', () => {
579579
const response = await explorer.handleMessage(message);
580580

581581
expect(response).toBeDefined();
582-
expect(response?.type).toBe('response');
582+
expect(response?.type).toBe('error');
583583

584584
const result = response?.payload as { error?: string };
585585
expect(result.error).toBeDefined();
586+
expect(result.error).toContain('Invalid exploration request');
586587
});
587588

588589
it('should return error response on failure', async () => {

packages/subagents/src/explorer/index.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
* Explores and analyzes code patterns using semantic search
44
*/
55

6+
import { validateExplorationRequest } from '../schemas/messages.js';
67
import type { Agent, AgentContext, Message } from '../types';
78
import type {
89
CodeInsights,
@@ -53,7 +54,7 @@ export class ExplorerAgent implements Agent {
5354
}
5455

5556
try {
56-
const request = message.payload as unknown as ExplorationRequest;
57+
const request = validateExplorationRequest(message.payload);
5758
logger.debug('Processing exploration request', { action: request.action });
5859

5960
let result: ExplorationResult | ExplorationError;

packages/subagents/src/github/agent.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
* Provides rich context from GitHub issues, PRs, and discussions
44
*/
55

6+
import { validateGitHubContextRequest } from '../schemas/messages.js';
67
import type { Agent, AgentContext, Message } from '../types';
78
import { GitHubIndexer } from './indexer';
89
import type {
@@ -67,7 +68,7 @@ export class GitHubAgent implements Agent {
6768
}
6869

6970
try {
70-
const request = message.payload as unknown as GitHubContextRequest;
71+
const request = validateGitHubContextRequest(message.payload);
7172
logger.debug('Processing GitHub request', { action: request.action });
7273

7374
let result: GitHubContextResult | GitHubContextError;

packages/subagents/src/planner/__tests__/index.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -162,8 +162,8 @@ describe('PlannerAgent', () => {
162162
const response = await planner.handleMessage(message);
163163

164164
expect(response).toBeTruthy();
165-
expect(response?.type).toBe('response');
166-
expect((response?.payload as { error?: string }).error).toContain('Unknown action');
165+
expect(response?.type).toBe('error');
166+
expect((response?.payload as { error?: string }).error).toContain('Invalid planning request');
167167
});
168168

169169
it('should generate correct response message structure', async () => {

packages/subagents/src/planner/index.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
* Analyzes GitHub issues and creates actionable development plans
44
*/
55

6+
import { validatePlanningRequest } from '../schemas/messages.js';
67
import type { Agent, AgentContext, Message } from '../types';
78
import type { Plan, PlanningError, PlanningRequest, PlanningResult } from './types';
89

@@ -33,7 +34,7 @@ export class PlannerAgent implements Agent {
3334
}
3435

3536
try {
36-
const request = message.payload as unknown as PlanningRequest;
37+
const request = validatePlanningRequest(message.payload);
3738
logger.debug('Processing planning request', { action: request.action });
3839

3940
let result: PlanningResult | PlanningError;
Lines changed: 281 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,281 @@
1+
/**
2+
* Subagent Message Schema Tests
3+
* Validates inter-agent message payloads
4+
*/
5+
6+
import { describe, expect, it } from 'vitest';
7+
import {
8+
ExplorationRequestSchema,
9+
GitHubContextRequestSchema,
10+
PlanningRequestSchema,
11+
validateExplorationRequest,
12+
validateGitHubContextRequest,
13+
validatePlanningRequest,
14+
} from '../messages.js';
15+
16+
describe('PlanningRequestSchema', () => {
17+
it('should validate valid planning request', () => {
18+
const input = {
19+
action: 'plan',
20+
issueNumber: 42,
21+
useExplorer: true,
22+
detailLevel: 'detailed',
23+
strategy: 'sequential',
24+
};
25+
26+
const result = PlanningRequestSchema.safeParse(input);
27+
expect(result.success).toBe(true);
28+
if (result.success) {
29+
expect(result.data.action).toBe('plan');
30+
expect(result.data.issueNumber).toBe(42);
31+
expect(result.data.useExplorer).toBe(true);
32+
}
33+
});
34+
35+
it('should allow optional fields', () => {
36+
const input = {
37+
action: 'plan',
38+
issueNumber: 1,
39+
};
40+
41+
const result = PlanningRequestSchema.safeParse(input);
42+
expect(result.success).toBe(true);
43+
});
44+
45+
it('should reject negative issue number', () => {
46+
const input = {
47+
action: 'plan',
48+
issueNumber: -1,
49+
};
50+
51+
const result = PlanningRequestSchema.safeParse(input);
52+
expect(result.success).toBe(false);
53+
});
54+
55+
it('should reject invalid action', () => {
56+
const input = {
57+
action: 'invalid',
58+
issueNumber: 1,
59+
};
60+
61+
const result = PlanningRequestSchema.safeParse(input);
62+
expect(result.success).toBe(false);
63+
});
64+
65+
it('should reject invalid detail level', () => {
66+
const input = {
67+
action: 'plan',
68+
issueNumber: 1,
69+
detailLevel: 'invalid',
70+
};
71+
72+
const result = PlanningRequestSchema.safeParse(input);
73+
expect(result.success).toBe(false);
74+
});
75+
});
76+
77+
describe('ExplorationRequestSchema', () => {
78+
it('should validate pattern exploration', () => {
79+
const input = {
80+
action: 'pattern' as const,
81+
query: 'authentication',
82+
threshold: 0.8,
83+
limit: 10,
84+
};
85+
86+
const result = ExplorationRequestSchema.safeParse(input);
87+
expect(result.success).toBe(true);
88+
if (result.success && result.data.action === 'pattern') {
89+
expect(result.data.action).toBe('pattern');
90+
expect(result.data.query).toBe('authentication');
91+
}
92+
});
93+
94+
it('should validate similar code search', () => {
95+
const input = {
96+
action: 'similar' as const,
97+
filePath: 'src/auth/token.ts',
98+
};
99+
100+
const result = ExplorationRequestSchema.safeParse(input);
101+
expect(result.success).toBe(true);
102+
});
103+
104+
it('should validate relationships query', () => {
105+
const input = {
106+
action: 'relationships' as const,
107+
component: 'MyClass',
108+
type: 'all' as const,
109+
};
110+
111+
const result = ExplorationRequestSchema.safeParse(input);
112+
expect(result.success).toBe(true);
113+
if (result.success && result.data.action === 'relationships') {
114+
expect(result.data.type).toBe('all');
115+
}
116+
});
117+
118+
it('should reject empty query', () => {
119+
const input = {
120+
action: 'pattern' as const,
121+
query: '',
122+
};
123+
124+
const result = ExplorationRequestSchema.safeParse(input);
125+
expect(result.success).toBe(false);
126+
});
127+
128+
it('should reject invalid action', () => {
129+
const input = {
130+
action: 'invalid',
131+
query: 'test',
132+
};
133+
134+
const result = ExplorationRequestSchema.safeParse(input);
135+
expect(result.success).toBe(false);
136+
});
137+
138+
it('should reject threshold out of range', () => {
139+
const input = {
140+
action: 'pattern' as const,
141+
query: 'test',
142+
threshold: 1.5,
143+
};
144+
145+
const result = ExplorationRequestSchema.safeParse(input);
146+
expect(result.success).toBe(false);
147+
});
148+
});
149+
150+
describe('GitHubContextRequestSchema', () => {
151+
it('should validate context request for issue', () => {
152+
const input = {
153+
action: 'context' as const,
154+
issueNumber: 42,
155+
};
156+
157+
const result = GitHubContextRequestSchema.safeParse(input);
158+
expect(result.success).toBe(true);
159+
if (result.success) {
160+
expect(result.data.action).toBe('context');
161+
expect(result.data.issueNumber).toBe(42);
162+
}
163+
});
164+
165+
it('should validate search with options', () => {
166+
const input = {
167+
action: 'search' as const,
168+
query: 'authentication bug',
169+
searchOptions: {
170+
type: 'issue',
171+
state: 'open',
172+
labels: ['bug', 'high-priority'],
173+
},
174+
};
175+
176+
const result = GitHubContextRequestSchema.safeParse(input);
177+
expect(result.success).toBe(true);
178+
if (result.success) {
179+
expect(result.data.query).toBe('authentication bug');
180+
}
181+
});
182+
183+
it('should validate context request', () => {
184+
const input = {
185+
action: 'context' as const,
186+
issueNumber: 42,
187+
includeCodeContext: true,
188+
};
189+
190+
const result = GitHubContextRequestSchema.safeParse(input);
191+
expect(result.success).toBe(true);
192+
});
193+
194+
it('should allow requests without optional fields', () => {
195+
const input = {
196+
action: 'index' as const,
197+
};
198+
199+
const result = GitHubContextRequestSchema.safeParse(input);
200+
expect(result.success).toBe(true);
201+
});
202+
203+
it('should reject invalid action', () => {
204+
const input = {
205+
action: 'invalid-action',
206+
};
207+
208+
const result = GitHubContextRequestSchema.safeParse(input);
209+
expect(result.success).toBe(false);
210+
});
211+
});
212+
213+
describe('Validation Functions', () => {
214+
describe('validatePlanningRequest', () => {
215+
it('should return validated data for valid input', () => {
216+
const input = {
217+
action: 'plan',
218+
issueNumber: 42,
219+
};
220+
221+
const result = validatePlanningRequest(input);
222+
expect(result.action).toBe('plan');
223+
expect(result.issueNumber).toBe(42);
224+
});
225+
226+
it('should throw descriptive error for invalid input', () => {
227+
const input = {
228+
action: 'plan',
229+
issueNumber: 'not-a-number',
230+
};
231+
232+
expect(() => validatePlanningRequest(input)).toThrow('Invalid planning request');
233+
expect(() => validatePlanningRequest(input)).toThrow('issueNumber');
234+
});
235+
});
236+
237+
describe('validateExplorationRequest', () => {
238+
it('should return validated data for valid input', () => {
239+
const input = {
240+
action: 'pattern' as const,
241+
query: 'auth',
242+
};
243+
244+
const result = validateExplorationRequest(input);
245+
expect(result.action).toBe('pattern');
246+
if (result.action === 'pattern') {
247+
expect(result.query).toBe('auth');
248+
}
249+
});
250+
251+
it('should throw descriptive error for invalid input', () => {
252+
const input = {
253+
action: 'pattern' as const,
254+
query: '',
255+
};
256+
257+
expect(() => validateExplorationRequest(input)).toThrow('Invalid exploration request');
258+
});
259+
});
260+
261+
describe('validateGitHubContextRequest', () => {
262+
it('should return validated data for valid input', () => {
263+
const input = {
264+
action: 'context' as const,
265+
issueNumber: 42,
266+
};
267+
268+
const result = validateGitHubContextRequest(input);
269+
expect(result.action).toBe('context');
270+
expect(result.issueNumber).toBe(42);
271+
});
272+
273+
it('should throw descriptive error for invalid input', () => {
274+
const input = {
275+
action: 'invalid',
276+
};
277+
278+
expect(() => validateGitHubContextRequest(input)).toThrow('Invalid GitHub context request');
279+
});
280+
});
281+
});

0 commit comments

Comments
 (0)