Skip to content

Commit 2b3859c

Browse files
author
loopy-symphony
committed
fix: Fix validate command: unify CLI/TUI logic, add missing schema checks, improve TUI screen (#12)
1 parent 686dbee commit 2b3859c

6 files changed

Lines changed: 286 additions & 126 deletions

File tree

src/cli/commands/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,3 +13,4 @@ export { registerRun } from './run';
1313
export { registerStatus } from './status';
1414
export { registerTraces } from './traces';
1515
export { registerUpdate } from './update';
16+
export { registerValidate } from './validate';

src/cli/commands/validate/__tests__/action.test.ts

Lines changed: 168 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,16 @@ const {
55
mockReadProjectSpec,
66
mockReadAWSDeploymentTargets,
77
mockReadDeployedState,
8+
mockReadMcpSpec,
9+
mockReadMcpDefs,
810
mockConfigExists,
911
mockFindConfigRoot,
1012
} = vi.hoisted(() => ({
1113
mockReadProjectSpec: vi.fn(),
1214
mockReadAWSDeploymentTargets: vi.fn(),
1315
mockReadDeployedState: vi.fn(),
16+
mockReadMcpSpec: vi.fn(),
17+
mockReadMcpDefs: vi.fn(),
1418
mockConfigExists: vi.fn(),
1519
mockFindConfigRoot: vi.fn(),
1620
}));
@@ -54,6 +58,8 @@ vi.mock('../../../../lib/index.js', () => {
5458
readProjectSpec = mockReadProjectSpec;
5559
readAWSDeploymentTargets = mockReadAWSDeploymentTargets;
5660
readDeployedState = mockReadDeployedState;
61+
readMcpSpec = mockReadMcpSpec;
62+
readMcpDefs = mockReadMcpDefs;
5763
configExists = mockConfigExists;
5864
},
5965
ConfigValidationError,
@@ -75,6 +81,7 @@ describe('handleValidate', () => {
7581

7682
expect(result.success).toBe(false);
7783
expect(result.error).toContain('No agentcore project found');
84+
expect(result.results).toEqual([]);
7885
});
7986

8087
it('returns success when all configs are valid', async () => {
@@ -86,22 +93,35 @@ describe('handleValidate', () => {
8693
const result = await handleValidate({});
8794

8895
expect(result.success).toBe(true);
96+
expect(result.results).toHaveLength(5);
97+
expect(result.results[0]).toEqual({ file: 'agentcore.json', success: true });
98+
expect(result.results[1]).toEqual({ file: 'aws-targets.json', success: true });
99+
// Optional files skipped
100+
expect(result.results[2]).toEqual({ file: 'mcp.json', success: true, skipped: true });
101+
expect(result.results[3]).toEqual({ file: 'mcp-defs.json', success: true, skipped: true });
102+
expect(result.results[4]).toEqual({ file: '.cli/state.json', success: true, skipped: true });
89103
});
90104

91105
it('returns error when project spec fails', async () => {
92106
mockFindConfigRoot.mockReturnValue('/project/agentcore');
93107
mockReadProjectSpec.mockRejectedValue(new Error('invalid project'));
108+
mockReadAWSDeploymentTargets.mockResolvedValue([]);
109+
mockConfigExists.mockReturnValue(false);
94110

95111
const result = await handleValidate({});
96112

97113
expect(result.success).toBe(false);
98114
expect(result.error).toContain('invalid project');
115+
// Should still report results for all files
116+
expect(result.results).toHaveLength(5);
117+
expect(result.results[0]?.success).toBe(false);
99118
});
100119

101120
it('returns error when AWS targets fails', async () => {
102121
mockFindConfigRoot.mockReturnValue('/project/agentcore');
103122
mockReadProjectSpec.mockResolvedValue({ name: 'Test', agents: [] });
104123
mockReadAWSDeploymentTargets.mockRejectedValue(new Error('bad targets'));
124+
mockConfigExists.mockReturnValue(false);
105125

106126
const result = await handleValidate({});
107127

@@ -113,7 +133,7 @@ describe('handleValidate', () => {
113133
mockFindConfigRoot.mockReturnValue('/project/agentcore');
114134
mockReadProjectSpec.mockResolvedValue({ name: 'Test', agents: [] });
115135
mockReadAWSDeploymentTargets.mockResolvedValue([]);
116-
mockConfigExists.mockReturnValue(true);
136+
mockConfigExists.mockImplementation((type: string) => type === 'state');
117137
mockReadDeployedState.mockResolvedValue({ targets: {} });
118138

119139
const result = await handleValidate({});
@@ -126,7 +146,7 @@ describe('handleValidate', () => {
126146
mockFindConfigRoot.mockReturnValue('/project/agentcore');
127147
mockReadProjectSpec.mockResolvedValue({ name: 'Test', agents: [] });
128148
mockReadAWSDeploymentTargets.mockResolvedValue([]);
129-
mockConfigExists.mockReturnValue(true);
149+
mockConfigExists.mockImplementation((type: string) => type === 'state');
130150
mockReadDeployedState.mockRejectedValue(new Error('bad state'));
131151

132152
const result = await handleValidate({});
@@ -150,7 +170,11 @@ describe('handleValidate', () => {
150170
it('formats ConfigValidationError with its message', async () => {
151171
mockFindConfigRoot.mockReturnValue('/project/agentcore');
152172
const { ConfigValidationError } = await import('../../../../lib/index.js');
153-
mockReadProjectSpec.mockRejectedValue(new (ConfigValidationError as any)('field "name" is required'));
173+
const err = new Error('field "name" is required');
174+
Object.setPrototypeOf(err, ConfigValidationError.prototype);
175+
mockReadProjectSpec.mockRejectedValue(err);
176+
mockReadAWSDeploymentTargets.mockResolvedValue([]);
177+
mockConfigExists.mockReturnValue(false);
154178

155179
const result = await handleValidate({});
156180

@@ -162,6 +186,8 @@ describe('handleValidate', () => {
162186
mockFindConfigRoot.mockReturnValue('/project/agentcore');
163187
const { ConfigParseError } = await import('../../../../lib/index.js');
164188
mockReadProjectSpec.mockRejectedValue(new ConfigParseError('agentcore.json', new Error('Unexpected token')));
189+
mockReadAWSDeploymentTargets.mockResolvedValue([]);
190+
mockConfigExists.mockReturnValue(false);
165191

166192
const result = await handleValidate({});
167193

@@ -176,6 +202,8 @@ describe('handleValidate', () => {
176202
mockReadProjectSpec.mockRejectedValue(
177203
new ConfigReadError('agentcore.json', new Error('EACCES: permission denied'))
178204
);
205+
mockReadAWSDeploymentTargets.mockResolvedValue([]);
206+
mockConfigExists.mockReturnValue(false);
179207

180208
const result = await handleValidate({});
181209

@@ -188,6 +216,8 @@ describe('handleValidate', () => {
188216
mockFindConfigRoot.mockReturnValue('/project/agentcore');
189217
const { ConfigNotFoundError } = await import('../../../../lib/index.js');
190218
mockReadProjectSpec.mockRejectedValue(new ConfigNotFoundError('/path/agentcore.json', 'project'));
219+
mockReadAWSDeploymentTargets.mockResolvedValue([]);
220+
mockConfigExists.mockReturnValue(false);
191221

192222
const result = await handleValidate({});
193223

@@ -198,10 +228,145 @@ describe('handleValidate', () => {
198228
it('formats non-Error values as strings', async () => {
199229
mockFindConfigRoot.mockReturnValue('/project/agentcore');
200230
mockReadProjectSpec.mockRejectedValue('string error');
231+
mockReadAWSDeploymentTargets.mockResolvedValue([]);
232+
mockConfigExists.mockReturnValue(false);
201233

202234
const result = await handleValidate({});
203235

204236
expect(result.success).toBe(false);
205237
expect(result.error).toBe('string error');
206238
});
239+
240+
it('validates mcp.json when it exists', async () => {
241+
mockFindConfigRoot.mockReturnValue('/project/agentcore');
242+
mockReadProjectSpec.mockResolvedValue({ name: 'Test', agents: [] });
243+
mockReadAWSDeploymentTargets.mockResolvedValue([]);
244+
mockConfigExists.mockImplementation((type: string) => type === 'mcp');
245+
mockReadMcpSpec.mockResolvedValue({ mcpServers: {} });
246+
247+
const result = await handleValidate({});
248+
249+
expect(result.success).toBe(true);
250+
expect(mockReadMcpSpec).toHaveBeenCalled();
251+
const mcpResult = result.results.find(r => r.file === 'mcp.json');
252+
expect(mcpResult).toEqual({ file: 'mcp.json', success: true });
253+
});
254+
255+
it('returns error when mcp.json is invalid', async () => {
256+
mockFindConfigRoot.mockReturnValue('/project/agentcore');
257+
mockReadProjectSpec.mockResolvedValue({ name: 'Test', agents: [] });
258+
mockReadAWSDeploymentTargets.mockResolvedValue([]);
259+
mockConfigExists.mockImplementation((type: string) => type === 'mcp');
260+
mockReadMcpSpec.mockRejectedValue(new Error('invalid mcp config'));
261+
262+
const result = await handleValidate({});
263+
264+
expect(result.success).toBe(false);
265+
expect(result.error).toContain('invalid mcp config');
266+
const mcpResult = result.results.find(r => r.file === 'mcp.json');
267+
expect(mcpResult?.success).toBe(false);
268+
expect(mcpResult?.error).toContain('invalid mcp config');
269+
});
270+
271+
it('skips mcp.json when not present', async () => {
272+
mockFindConfigRoot.mockReturnValue('/project/agentcore');
273+
mockReadProjectSpec.mockResolvedValue({ name: 'Test', agents: [] });
274+
mockReadAWSDeploymentTargets.mockResolvedValue([]);
275+
mockConfigExists.mockReturnValue(false);
276+
277+
const result = await handleValidate({});
278+
279+
expect(result.success).toBe(true);
280+
expect(mockReadMcpSpec).not.toHaveBeenCalled();
281+
const mcpResult = result.results.find(r => r.file === 'mcp.json');
282+
expect(mcpResult).toEqual({ file: 'mcp.json', success: true, skipped: true });
283+
});
284+
285+
it('validates mcp-defs.json when it exists', async () => {
286+
mockFindConfigRoot.mockReturnValue('/project/agentcore');
287+
mockReadProjectSpec.mockResolvedValue({ name: 'Test', agents: [] });
288+
mockReadAWSDeploymentTargets.mockResolvedValue([]);
289+
mockConfigExists.mockImplementation((type: string) => type === 'mcpDefs');
290+
mockReadMcpDefs.mockResolvedValue({ tools: [] });
291+
292+
const result = await handleValidate({});
293+
294+
expect(result.success).toBe(true);
295+
expect(mockReadMcpDefs).toHaveBeenCalled();
296+
const mcpDefsResult = result.results.find(r => r.file === 'mcp-defs.json');
297+
expect(mcpDefsResult).toEqual({ file: 'mcp-defs.json', success: true });
298+
});
299+
300+
it('returns error when mcp-defs.json is invalid', async () => {
301+
mockFindConfigRoot.mockReturnValue('/project/agentcore');
302+
mockReadProjectSpec.mockResolvedValue({ name: 'Test', agents: [] });
303+
mockReadAWSDeploymentTargets.mockResolvedValue([]);
304+
mockConfigExists.mockImplementation((type: string) => type === 'mcpDefs');
305+
mockReadMcpDefs.mockRejectedValue(new Error('invalid mcp definitions'));
306+
307+
const result = await handleValidate({});
308+
309+
expect(result.success).toBe(false);
310+
expect(result.error).toContain('invalid mcp definitions');
311+
const mcpDefsResult = result.results.find(r => r.file === 'mcp-defs.json');
312+
expect(mcpDefsResult?.success).toBe(false);
313+
expect(mcpDefsResult?.error).toContain('invalid mcp definitions');
314+
});
315+
316+
it('skips mcp-defs.json when not present', async () => {
317+
mockFindConfigRoot.mockReturnValue('/project/agentcore');
318+
mockReadProjectSpec.mockResolvedValue({ name: 'Test', agents: [] });
319+
mockReadAWSDeploymentTargets.mockResolvedValue([]);
320+
mockConfigExists.mockReturnValue(false);
321+
322+
const result = await handleValidate({});
323+
324+
expect(result.success).toBe(true);
325+
expect(mockReadMcpDefs).not.toHaveBeenCalled();
326+
const mcpDefsResult = result.results.find(r => r.file === 'mcp-defs.json');
327+
expect(mcpDefsResult).toEqual({ file: 'mcp-defs.json', success: true, skipped: true });
328+
});
329+
330+
it('reports all errors instead of stopping on first failure', async () => {
331+
mockFindConfigRoot.mockReturnValue('/project/agentcore');
332+
mockReadProjectSpec.mockRejectedValue(new Error('bad project'));
333+
mockReadAWSDeploymentTargets.mockRejectedValue(new Error('bad targets'));
334+
mockConfigExists.mockReturnValue(true);
335+
mockReadMcpSpec.mockRejectedValue(new Error('bad mcp'));
336+
mockReadMcpDefs.mockRejectedValue(new Error('bad mcp defs'));
337+
mockReadDeployedState.mockRejectedValue(new Error('bad state'));
338+
339+
const result = await handleValidate({});
340+
341+
expect(result.success).toBe(false);
342+
expect(result.results).toHaveLength(5);
343+
// All files should have been validated (not stopped on first)
344+
expect(result.results.every(r => !r.success)).toBe(true);
345+
expect(result.error).toContain('bad project');
346+
expect(result.error).toContain('bad targets');
347+
expect(result.error).toContain('bad mcp');
348+
expect(result.error).toContain('bad mcp defs');
349+
expect(result.error).toContain('bad state');
350+
});
351+
352+
it('validates all 5 files when all exist and are valid', async () => {
353+
mockFindConfigRoot.mockReturnValue('/project/agentcore');
354+
mockReadProjectSpec.mockResolvedValue({ name: 'Test', agents: [] });
355+
mockReadAWSDeploymentTargets.mockResolvedValue([]);
356+
mockConfigExists.mockReturnValue(true);
357+
mockReadMcpSpec.mockResolvedValue({ mcpServers: {} });
358+
mockReadMcpDefs.mockResolvedValue({ tools: [] });
359+
mockReadDeployedState.mockResolvedValue({ targets: {} });
360+
361+
const result = await handleValidate({});
362+
363+
expect(result.success).toBe(true);
364+
expect(result.results).toHaveLength(5);
365+
expect(result.results.every(r => r.success)).toBe(true);
366+
expect(mockReadProjectSpec).toHaveBeenCalled();
367+
expect(mockReadAWSDeploymentTargets).toHaveBeenCalled();
368+
expect(mockReadMcpSpec).toHaveBeenCalled();
369+
expect(mockReadMcpDefs).toHaveBeenCalled();
370+
expect(mockReadDeployedState).toHaveBeenCalled();
371+
});
207372
});

src/cli/commands/validate/action.ts

Lines changed: 53 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,35 @@ export interface ValidateOptions {
1212
directory?: string;
1313
}
1414

15+
export interface ValidateFileResult {
16+
file: string;
17+
success: boolean;
18+
skipped?: boolean;
19+
error?: string;
20+
}
21+
1522
export interface ValidateResult {
1623
success: boolean;
1724
error?: string;
25+
results: ValidateFileResult[];
1826
}
1927

28+
/**
29+
* Schema files validated by the validate command.
30+
* Required files must exist; optional files are skipped when absent.
31+
*/
32+
const SCHEMA_FILES = [
33+
{ key: 'project', label: 'agentcore.json', required: true },
34+
{ key: 'targets', label: 'aws-targets.json', required: true },
35+
{ key: 'mcp', label: 'mcp.json', required: false },
36+
{ key: 'mcpDefs', label: 'mcp-defs.json', required: false },
37+
{ key: 'state', label: '.cli/state.json', required: false },
38+
] as const;
39+
2040
/**
2141
* Validates all AgentCore schema files in the project.
22-
* Returns a binary success/fail result with an error message if validation fails.
42+
* Returns per-file results so both CLI and TUI can report granular status.
43+
* All files are validated even if earlier ones fail.
2344
*/
2445
export async function handleValidate(options: ValidateOptions): Promise<ValidateResult> {
2546
const baseDir = options.directory ?? process.cwd();
@@ -30,35 +51,48 @@ export async function handleValidate(options: ValidateOptions): Promise<Validate
3051
return {
3152
success: false,
3253
error: new NoProjectError().message,
54+
results: [],
3355
};
3456
}
3557

3658
const configIO = new ConfigIO({ baseDir: configRoot });
59+
const results: ValidateFileResult[] = [];
3760

38-
// Validate project spec (agentcore.json)
39-
try {
40-
await configIO.readProjectSpec();
41-
} catch (err) {
42-
return { success: false, error: formatError(err, 'agentcore.json') };
43-
}
44-
45-
// Validate AWS targets (aws-targets.json)
46-
try {
47-
await configIO.readAWSDeploymentTargets();
48-
} catch (err) {
49-
return { success: false, error: formatError(err, 'aws-targets.json') };
50-
}
61+
for (const file of SCHEMA_FILES) {
62+
// For optional files, skip if not present
63+
if (!file.required) {
64+
if (!configIO.configExists(file.key)) {
65+
results.push({ file: file.label, success: true, skipped: true });
66+
continue;
67+
}
68+
}
5169

52-
// Validate deployed state if it exists (.cli/state.json)
53-
if (configIO.configExists('state')) {
5470
try {
55-
await configIO.readDeployedState();
71+
if (file.key === 'project') {
72+
await configIO.readProjectSpec();
73+
} else if (file.key === 'targets') {
74+
await configIO.readAWSDeploymentTargets();
75+
} else if (file.key === 'mcp') {
76+
await configIO.readMcpSpec();
77+
} else if (file.key === 'mcpDefs') {
78+
await configIO.readMcpDefs();
79+
} else if (file.key === 'state') {
80+
await configIO.readDeployedState();
81+
}
82+
results.push({ file: file.label, success: true });
5683
} catch (err) {
57-
return { success: false, error: formatError(err, '.cli/state.json') };
84+
results.push({ file: file.label, success: false, error: formatError(err, file.label) });
5885
}
5986
}
6087

61-
return { success: true };
88+
const errors = results.filter(r => !r.success);
89+
const hasErrors = errors.length > 0;
90+
91+
return {
92+
success: !hasErrors,
93+
error: hasErrors ? errors.map(r => r.error).join('\n') : undefined,
94+
results,
95+
};
6296
}
6397

6498
function formatError(err: unknown, fileName: string): string {

0 commit comments

Comments
 (0)