Skip to content

fix: Fix validate command: unify CLI/TUI logic, add missing schema checks, improve TUI screen (#12) #13

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
aidandaly24 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: Fix validate command: unify CLI/TUI logic, add missing schema checks, improve TUI screen (#12) #13

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/cli/commands/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,4 @@ export { registerRun } from './run';
export { registerStatus } from './status';
export { registerTraces } from './traces';
export { registerUpdate } from './update';
export { registerValidate } from './validate';
171 changes: 168 additions & 3 deletions src/cli/commands/validate/__tests__/action.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,16 @@ const {
mockReadProjectSpec,
mockReadAWSDeploymentTargets,
mockReadDeployedState,
mockReadMcpSpec,
mockReadMcpDefs,
mockConfigExists,
mockFindConfigRoot,
} = vi.hoisted(() => ({
mockReadProjectSpec: vi.fn(),
mockReadAWSDeploymentTargets: vi.fn(),
mockReadDeployedState: vi.fn(),
mockReadMcpSpec: vi.fn(),
mockReadMcpDefs: vi.fn(),
mockConfigExists: vi.fn(),
mockFindConfigRoot: vi.fn(),
}));
Expand Down Expand Up @@ -54,6 +58,8 @@ vi.mock('../../../../lib/index.js', () => {
readProjectSpec = mockReadProjectSpec;
readAWSDeploymentTargets = mockReadAWSDeploymentTargets;
readDeployedState = mockReadDeployedState;
readMcpSpec = mockReadMcpSpec;
readMcpDefs = mockReadMcpDefs;
configExists = mockConfigExists;
},
ConfigValidationError,
Expand All @@ -75,6 +81,7 @@ describe('handleValidate', () => {

expect(result.success).toBe(false);
expect(result.error).toContain('No agentcore project found');
expect(result.results).toEqual([]);
});

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

expect(result.success).toBe(true);
expect(result.results).toHaveLength(5);
expect(result.results[0]).toEqual({ file: 'agentcore.json', success: true });
expect(result.results[1]).toEqual({ file: 'aws-targets.json', success: true });
// Optional files skipped
expect(result.results[2]).toEqual({ file: 'mcp.json', success: true, skipped: true });
expect(result.results[3]).toEqual({ file: 'mcp-defs.json', success: true, skipped: true });
expect(result.results[4]).toEqual({ file: '.cli/state.json', success: true, skipped: true });
});

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

const result = await handleValidate({});

expect(result.success).toBe(false);
expect(result.error).toContain('invalid project');
// Should still report results for all files
expect(result.results).toHaveLength(5);
expect(result.results[0]?.success).toBe(false);
});

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

const result = await handleValidate({});

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

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

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

const result = await handleValidate({});

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

const result = await handleValidate({});

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

const result = await handleValidate({});

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

const result = await handleValidate({});

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

const result = await handleValidate({});

expect(result.success).toBe(false);
expect(result.error).toBe('string error');
});

it('validates mcp.json when it exists', async () => {
mockFindConfigRoot.mockReturnValue('/project/agentcore');
mockReadProjectSpec.mockResolvedValue({ name: 'Test', agents: [] });
mockReadAWSDeploymentTargets.mockResolvedValue([]);
mockConfigExists.mockImplementation((type: string) => type === 'mcp');
mockReadMcpSpec.mockResolvedValue({ mcpServers: {} });

const result = await handleValidate({});

expect(result.success).toBe(true);
expect(mockReadMcpSpec).toHaveBeenCalled();
const mcpResult = result.results.find(r => r.file === 'mcp.json');
expect(mcpResult).toEqual({ file: 'mcp.json', success: true });
});

it('returns error when mcp.json is invalid', async () => {
mockFindConfigRoot.mockReturnValue('/project/agentcore');
mockReadProjectSpec.mockResolvedValue({ name: 'Test', agents: [] });
mockReadAWSDeploymentTargets.mockResolvedValue([]);
mockConfigExists.mockImplementation((type: string) => type === 'mcp');
mockReadMcpSpec.mockRejectedValue(new Error('invalid mcp config'));

const result = await handleValidate({});

expect(result.success).toBe(false);
expect(result.error).toContain('invalid mcp config');
const mcpResult = result.results.find(r => r.file === 'mcp.json');
expect(mcpResult?.success).toBe(false);
expect(mcpResult?.error).toContain('invalid mcp config');
});

it('skips mcp.json when not present', async () => {
mockFindConfigRoot.mockReturnValue('/project/agentcore');
mockReadProjectSpec.mockResolvedValue({ name: 'Test', agents: [] });
mockReadAWSDeploymentTargets.mockResolvedValue([]);
mockConfigExists.mockReturnValue(false);

const result = await handleValidate({});

expect(result.success).toBe(true);
expect(mockReadMcpSpec).not.toHaveBeenCalled();
const mcpResult = result.results.find(r => r.file === 'mcp.json');
expect(mcpResult).toEqual({ file: 'mcp.json', success: true, skipped: true });
});

it('validates mcp-defs.json when it exists', async () => {
mockFindConfigRoot.mockReturnValue('/project/agentcore');
mockReadProjectSpec.mockResolvedValue({ name: 'Test', agents: [] });
mockReadAWSDeploymentTargets.mockResolvedValue([]);
mockConfigExists.mockImplementation((type: string) => type === 'mcpDefs');
mockReadMcpDefs.mockResolvedValue({ tools: [] });

const result = await handleValidate({});

expect(result.success).toBe(true);
expect(mockReadMcpDefs).toHaveBeenCalled();
const mcpDefsResult = result.results.find(r => r.file === 'mcp-defs.json');
expect(mcpDefsResult).toEqual({ file: 'mcp-defs.json', success: true });
});

it('returns error when mcp-defs.json is invalid', async () => {
mockFindConfigRoot.mockReturnValue('/project/agentcore');
mockReadProjectSpec.mockResolvedValue({ name: 'Test', agents: [] });
mockReadAWSDeploymentTargets.mockResolvedValue([]);
mockConfigExists.mockImplementation((type: string) => type === 'mcpDefs');
mockReadMcpDefs.mockRejectedValue(new Error('invalid mcp definitions'));

const result = await handleValidate({});

expect(result.success).toBe(false);
expect(result.error).toContain('invalid mcp definitions');
const mcpDefsResult = result.results.find(r => r.file === 'mcp-defs.json');
expect(mcpDefsResult?.success).toBe(false);
expect(mcpDefsResult?.error).toContain('invalid mcp definitions');
});

it('skips mcp-defs.json when not present', async () => {
mockFindConfigRoot.mockReturnValue('/project/agentcore');
mockReadProjectSpec.mockResolvedValue({ name: 'Test', agents: [] });
mockReadAWSDeploymentTargets.mockResolvedValue([]);
mockConfigExists.mockReturnValue(false);

const result = await handleValidate({});

expect(result.success).toBe(true);
expect(mockReadMcpDefs).not.toHaveBeenCalled();
const mcpDefsResult = result.results.find(r => r.file === 'mcp-defs.json');
expect(mcpDefsResult).toEqual({ file: 'mcp-defs.json', success: true, skipped: true });
});

it('reports all errors instead of stopping on first failure', async () => {
mockFindConfigRoot.mockReturnValue('/project/agentcore');
mockReadProjectSpec.mockRejectedValue(new Error('bad project'));
mockReadAWSDeploymentTargets.mockRejectedValue(new Error('bad targets'));
mockConfigExists.mockReturnValue(true);
mockReadMcpSpec.mockRejectedValue(new Error('bad mcp'));
mockReadMcpDefs.mockRejectedValue(new Error('bad mcp defs'));
mockReadDeployedState.mockRejectedValue(new Error('bad state'));

const result = await handleValidate({});

expect(result.success).toBe(false);
expect(result.results).toHaveLength(5);
// All files should have been validated (not stopped on first)
expect(result.results.every(r => !r.success)).toBe(true);
expect(result.error).toContain('bad project');
expect(result.error).toContain('bad targets');
expect(result.error).toContain('bad mcp');
expect(result.error).toContain('bad mcp defs');
expect(result.error).toContain('bad state');
});

it('validates all 5 files when all exist and are valid', async () => {
mockFindConfigRoot.mockReturnValue('/project/agentcore');
mockReadProjectSpec.mockResolvedValue({ name: 'Test', agents: [] });
mockReadAWSDeploymentTargets.mockResolvedValue([]);
mockConfigExists.mockReturnValue(true);
mockReadMcpSpec.mockResolvedValue({ mcpServers: {} });
mockReadMcpDefs.mockResolvedValue({ tools: [] });
mockReadDeployedState.mockResolvedValue({ targets: {} });

const result = await handleValidate({});

expect(result.success).toBe(true);
expect(result.results).toHaveLength(5);
expect(result.results.every(r => r.success)).toBe(true);
expect(mockReadProjectSpec).toHaveBeenCalled();
expect(mockReadAWSDeploymentTargets).toHaveBeenCalled();
expect(mockReadMcpSpec).toHaveBeenCalled();
expect(mockReadMcpDefs).toHaveBeenCalled();
expect(mockReadDeployedState).toHaveBeenCalled();
});
});
72 changes: 53 additions & 19 deletions src/cli/commands/validate/action.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,35 @@ export interface ValidateOptions {
directory?: string;
}

export interface ValidateFileResult {
file: string;
success: boolean;
skipped?: boolean;
error?: string;
}

export interface ValidateResult {
success: boolean;
error?: string;
results: ValidateFileResult[];
}

/**
* Schema files validated by the validate command.
* Required files must exist; optional files are skipped when absent.
*/
const SCHEMA_FILES = [
{ key: 'project', label: 'agentcore.json', required: true },
{ key: 'targets', label: 'aws-targets.json', required: true },
{ key: 'mcp', label: 'mcp.json', required: false },
{ key: 'mcpDefs', label: 'mcp-defs.json', required: false },
{ key: 'state', label: '.cli/state.json', required: false },
] as const;

/**
* Validates all AgentCore schema files in the project.
* Returns a binary success/fail result with an error message if validation fails.
* Returns per-file results so both CLI and TUI can report granular status.
* All files are validated even if earlier ones fail.
*/
export async function handleValidate(options: ValidateOptions): Promise<ValidateResult> {
const baseDir = options.directory ?? process.cwd();
Expand All @@ -30,35 +51,48 @@ export async function handleValidate(options: ValidateOptions): Promise<Validate
return {
success: false,
error: new NoProjectError().message,
results: [],
};
}

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

// Validate project spec (agentcore.json)
try {
await configIO.readProjectSpec();
} catch (err) {
return { success: false, error: formatError(err, 'agentcore.json') };
}

// Validate AWS targets (aws-targets.json)
try {
await configIO.readAWSDeploymentTargets();
} catch (err) {
return { success: false, error: formatError(err, 'aws-targets.json') };
}
for (const file of SCHEMA_FILES) {
// For optional files, skip if not present
if (!file.required) {
if (!configIO.configExists(file.key)) {
results.push({ file: file.label, success: true, skipped: true });
continue;
}
}

// Validate deployed state if it exists (.cli/state.json)
if (configIO.configExists('state')) {
try {
await configIO.readDeployedState();
if (file.key === 'project') {
await configIO.readProjectSpec();
} else if (file.key === 'targets') {
await configIO.readAWSDeploymentTargets();
} else if (file.key === 'mcp') {
await configIO.readMcpSpec();
} else if (file.key === 'mcpDefs') {
await configIO.readMcpDefs();
} else if (file.key === 'state') {
await configIO.readDeployedState();
}
results.push({ file: file.label, success: true });
} catch (err) {
return { success: false, error: formatError(err, '.cli/state.json') };
results.push({ file: file.label, success: false, error: formatError(err, file.label) });
}
}

return { success: true };
const errors = results.filter(r => !r.success);
const hasErrors = errors.length > 0;

return {
success: !hasErrors,
error: hasErrors ? errors.map(r => r.error).join('\n') : undefined,
results,
};
}

function formatError(err: unknown, fileName: string): string {
Expand Down
Loading
Loading