Skip to content

Commit ebbb01d

Browse files
committed
refactor(@angular/cli): add type safety fallbacks and deduplicate search roots for MCP projects tool
Added runtime type checks for `projectType` and `prefix` to prevent unhandled schema exceptions. Implemented `deduplicateSearchRoots` to filter out child directories from overlapping search trees, optimizing filesystem scans. Aggregated validation errors and reported them in the tool output for better visibility.
1 parent f1ed025 commit ebbb01d

File tree

2 files changed

+181
-18
lines changed

2 files changed

+181
-18
lines changed

packages/angular/cli/src/commands/mcp/tools/projects.ts

Lines changed: 136 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,18 @@ const listProjectsOutputSchema = {
112112
)
113113
.default([])
114114
.describe('A list of workspaces for which the framework version could not be determined.'),
115+
validationErrors: z
116+
.array(
117+
z.object({
118+
filePath: z.string().describe('The path to the workspace `angular.json` file.'),
119+
projectName: z.string().describe('The name of the project with invalid schema.'),
120+
message: z.string().describe('The reason why validation failed or fell back.'),
121+
}),
122+
)
123+
.default([])
124+
.describe(
125+
'A list of projects within workspaces that had invalid or malformed schema elements.',
126+
),
115127
};
116128

117129
export const LIST_PROJECTS_TOOL = declareTool({
@@ -331,6 +343,7 @@ async function findAngularCoreVersion(
331343
type WorkspaceData = z.infer<typeof listProjectsOutputSchema.workspaces>[number];
332344
type ParsingError = z.infer<typeof listProjectsOutputSchema.parsingErrors>[number];
333345
type VersioningError = z.infer<typeof listProjectsOutputSchema.versioningErrors>[number];
346+
type ValidationError = z.infer<typeof listProjectsOutputSchema.validationErrors>[number];
334347

335348
/**
336349
* Determines the unit test framework for a project based on its 'test' target configuration.
@@ -445,6 +458,41 @@ async function getProjectStyleLanguage(
445458
return 'css';
446459
}
447460

461+
/**
462+
* Validates a property using a type guard and pushes a validation error if it fails.
463+
* @param raw The raw value to validate.
464+
* @param isValid A type guard function to validate the value.
465+
* @param errorContext Context for the error message (filePath, projectName, propertyName, expectedDesc).
466+
* @param validationErrors The array to push errors to.
467+
* @returns The validated value or undefined if invalid.
468+
*/
469+
function validateProperty<T>(
470+
raw: unknown,
471+
isValid: (val: unknown) => val is T,
472+
errorContext: {
473+
configFile: string;
474+
projectName: string;
475+
propertyName: string;
476+
expectedDesc: string;
477+
},
478+
validationErrors: ValidationError[],
479+
): T | undefined {
480+
if (raw === undefined) {
481+
return undefined;
482+
}
483+
if (isValid(raw)) {
484+
return raw;
485+
}
486+
487+
validationErrors.push({
488+
filePath: errorContext.configFile,
489+
projectName: errorContext.projectName,
490+
message: `Invalid \`${errorContext.propertyName}\` '${raw}'. Expected ${errorContext.expectedDesc}. Falling back to undefined.`,
491+
});
492+
493+
return undefined;
494+
}
495+
448496
/**
449497
* Loads, parses, and transforms a single angular.json file into the tool's output format.
450498
* It checks a set of seen paths to avoid processing the same workspace multiple times.
@@ -455,36 +503,67 @@ async function getProjectStyleLanguage(
455503
async function loadAndParseWorkspace(
456504
configFile: string,
457505
seenPaths: Set<string>,
458-
): Promise<{ workspace: WorkspaceData | null; error: ParsingError | null }> {
506+
): Promise<{
507+
workspace: WorkspaceData | null;
508+
error: ParsingError | null;
509+
validationErrors: ValidationError[];
510+
}> {
459511
try {
460512
const resolvedPath = resolve(configFile);
461513
if (seenPaths.has(resolvedPath)) {
462-
return { workspace: null, error: null }; // Already processed, skip.
514+
return { workspace: null, error: null, validationErrors: [] }; // Already processed, skip.
463515
}
464516
seenPaths.add(resolvedPath);
465517

466518
const ws = await AngularWorkspace.load(configFile);
467-
const projects = [];
519+
const projects: WorkspaceData['projects'] = [];
520+
const validationErrors: ValidationError[] = [];
468521
const workspaceRoot = dirname(configFile);
469522
for (const [name, project] of ws.projects.entries()) {
470523
const sourceRoot = posix.join(project.root, project.sourceRoot ?? 'src');
471524
const fullSourceRoot = join(workspaceRoot, sourceRoot);
472525
const unitTestFramework = getUnitTestFramework(project.targets.get('test'));
473526
const styleLanguage = await getProjectStyleLanguage(project, ws, fullSourceRoot);
474527

528+
const type = validateProperty(
529+
project.extensions['projectType'],
530+
(val): val is 'application' | 'library' => val === 'application' || val === 'library',
531+
{
532+
configFile,
533+
projectName: name,
534+
propertyName: 'projectType',
535+
expectedDesc: "'application' or 'library'",
536+
},
537+
validationErrors,
538+
);
539+
540+
const selectorPrefix = validateProperty(
541+
project.extensions['prefix'],
542+
(val): val is string => typeof val === 'string',
543+
{ configFile, projectName: name, propertyName: 'prefix', expectedDesc: 'a string' },
544+
validationErrors,
545+
);
546+
547+
const builder = validateProperty(
548+
project.targets.get('build')?.builder,
549+
(val): val is string => typeof val === 'string',
550+
{ configFile, projectName: name, propertyName: 'builder', expectedDesc: 'a string' },
551+
validationErrors,
552+
);
553+
475554
projects.push({
476555
name,
477-
type: project.extensions['projectType'] as 'application' | 'library' | undefined,
478-
builder: project.targets.get('build')?.builder,
556+
type,
557+
builder,
479558
root: project.root,
480559
sourceRoot,
481-
selectorPrefix: project.extensions['prefix'] as string,
560+
selectorPrefix,
482561
unitTestFramework,
483562
styleLanguage,
484563
});
485564
}
486565

487-
return { workspace: { path: configFile, projects }, error: null };
566+
return { workspace: { path: configFile, projects }, error: null, validationErrors };
488567
} catch (error) {
489568
let message;
490569
if (error instanceof Error) {
@@ -493,7 +572,7 @@ async function loadAndParseWorkspace(
493572
message = 'An unknown error occurred while parsing the file.';
494573
}
495574

496-
return { workspace: null, error: { filePath: configFile, message } };
575+
return { workspace: null, error: { filePath: configFile, message }, validationErrors: [] };
497576
}
498577
}
499578

@@ -514,14 +593,15 @@ async function processConfigFile(
514593
workspace?: WorkspaceData;
515594
parsingError?: ParsingError;
516595
versioningError?: VersioningError;
596+
validationErrors?: ValidationError[];
517597
}> {
518-
const { workspace, error } = await loadAndParseWorkspace(configFile, seenPaths);
598+
const { workspace, error, validationErrors } = await loadAndParseWorkspace(configFile, seenPaths);
519599
if (error) {
520600
return { parsingError: error };
521601
}
522602

523603
if (!workspace) {
524-
return {}; // Skipped as it was already seen.
604+
return { validationErrors }; // If already seen, we still group validation errors if any (unlikely to be any if seen).
525605
}
526606

527607
try {
@@ -532,10 +612,11 @@ async function processConfigFile(
532612
searchRoot,
533613
);
534614

535-
return { workspace };
615+
return { workspace, validationErrors };
536616
} catch (e) {
537617
return {
538618
workspace,
619+
validationErrors,
539620
versioningError: {
540621
filePath: workspace.path,
541622
message: e instanceof Error ? e.message : 'An unknown error occurred.',
@@ -544,11 +625,37 @@ async function processConfigFile(
544625
}
545626
}
546627

628+
/**
629+
* Deduplicates overlapping search roots (e.g., if one is a child of another).
630+
* Sorting by length ensures parent directories are processed before children.
631+
* @param roots A list of normalized absolute paths used as search roots.
632+
* @returns A deduplicated list of search roots.
633+
*/
634+
function deduplicateSearchRoots(roots: string[]): string[] {
635+
const sortedRoots = [...roots].sort((a, b) => a.length - b.length);
636+
const deduplicated: string[] = [];
637+
638+
for (const root of sortedRoots) {
639+
const isSubdirectory = deduplicated.some((existing) => {
640+
const rel = relative(existing, root);
641+
642+
return rel === '' || (!rel.startsWith('..') && !isAbsolute(rel));
643+
});
644+
645+
if (!isSubdirectory) {
646+
deduplicated.push(root);
647+
}
648+
}
649+
650+
return deduplicated;
651+
}
652+
547653
async function createListProjectsHandler({ server }: McpToolContext) {
548654
return async () => {
549655
const workspaces: WorkspaceData[] = [];
550656
const parsingErrors: ParsingError[] = [];
551657
const versioningErrors: z.infer<typeof listProjectsOutputSchema.versioningErrors> = [];
658+
const validationErrors: ValidationError[] = [];
552659
const seenPaths = new Set<string>();
553660
const versionCache = new Map<string, string | undefined>();
554661

@@ -562,6 +669,8 @@ async function createListProjectsHandler({ server }: McpToolContext) {
562669
searchRoots = [process.cwd()];
563670
}
564671

672+
searchRoots = deduplicateSearchRoots(searchRoots);
673+
565674
// Pre-resolve allowed roots to handle their own symlinks or normalizations.
566675
// We ignore failures here; if a root is broken, we simply won't match against it.
567676
const realAllowedRoots = searchRoots
@@ -576,12 +685,12 @@ async function createListProjectsHandler({ server }: McpToolContext) {
576685

577686
for (const root of searchRoots) {
578687
for await (const configFile of findAngularJsonFiles(root, realAllowedRoots)) {
579-
const { workspace, parsingError, versioningError } = await processConfigFile(
580-
configFile,
581-
root,
582-
seenPaths,
583-
versionCache,
584-
);
688+
const {
689+
workspace,
690+
parsingError,
691+
versioningError,
692+
validationErrors: currentValidationErrors,
693+
} = await processConfigFile(configFile, root, seenPaths, versionCache);
585694

586695
if (workspace) {
587696
workspaces.push(workspace);
@@ -592,6 +701,9 @@ async function createListProjectsHandler({ server }: McpToolContext) {
592701
if (versioningError) {
593702
versioningErrors.push(versioningError);
594703
}
704+
if (currentValidationErrors) {
705+
validationErrors.push(...currentValidationErrors);
706+
}
595707
}
596708
}
597709

@@ -619,10 +731,16 @@ async function createListProjectsHandler({ server }: McpToolContext) {
619731
text += `\n\nWarning: The framework version for the following ${versioningErrors.length} workspace(s) could not be determined:\n`;
620732
text += versioningErrors.map((e) => `- ${e.filePath}: ${e.message}`).join('\n');
621733
}
734+
if (validationErrors.length > 0) {
735+
text += `\n\nWarning: The following ${validationErrors.length} project validation issue(s) were found (defaults used):\n`;
736+
text += validationErrors
737+
.map((e) => `- ${e.filePath} [Project: ${e.projectName}]: ${e.message}`)
738+
.join('\n');
739+
}
622740

623741
return {
624742
content: [{ type: 'text' as const, text }],
625-
structuredContent: { workspaces, parsingErrors, versioningErrors },
743+
structuredContent: { workspaces, parsingErrors, versioningErrors, validationErrors },
626744
};
627745
};
628746
}
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
import assert from 'node:assert/strict';
2+
import { exec, ProcessOutput, silentNpm } from '../../utils/process';
3+
import { updateJsonFile } from '../../utils/project';
4+
5+
const MCP_INSPECTOR_PACKAGE_NAME = '@modelcontextprotocol/inspector-cli';
6+
const MCP_INSPECTOR_PACKAGE_VERSION = '0.16.2';
7+
const MCP_INSPECTOR_COMMAND_NAME = 'mcp-inspector-cli';
8+
9+
async function runInspector(...args: string[]): Promise<ProcessOutput> {
10+
return exec(MCP_INSPECTOR_COMMAND_NAME, '--cli', 'npx', '--no', '@angular/cli', 'mcp', ...args);
11+
}
12+
13+
export default async function () {
14+
await silentNpm(
15+
'install',
16+
'--ignore-scripts',
17+
'-g',
18+
`${MCP_INSPECTOR_PACKAGE_NAME}@${MCP_INSPECTOR_PACKAGE_VERSION}`,
19+
);
20+
21+
try {
22+
// 1. Add a project with malformed attributes to angular.json
23+
await updateJsonFile('angular.json', (workspaceJson) => {
24+
workspaceJson.projects ??= {};
25+
workspaceJson.projects['invalid-lib'] = {
26+
root: 'projects/invalid-lib',
27+
sourceRoot: 'projects/invalid-lib/src',
28+
prefix: 12345 as any, // Invalid!
29+
};
30+
});
31+
32+
// 2. Call list_projects
33+
const { stdout } = await runInspector('--method', 'tools/call', '--tool-name', 'list_projects');
34+
35+
// 3. Verify that the warning section exists and lists the fallbacks
36+
assert.match(stdout, /Warning: The following \d+ project validation issue\(s\) were found/);
37+
assert.match(stdout, /Invalid `prefix`/);
38+
} finally {
39+
// 4. Cleanup angular.json
40+
await updateJsonFile('angular.json', (workspaceJson) => {
41+
delete workspaceJson.projects['invalid-lib'];
42+
});
43+
await silentNpm('uninstall', '-g', MCP_INSPECTOR_PACKAGE_NAME);
44+
}
45+
}

0 commit comments

Comments
 (0)