feat(@angular/cli): make get_best_practices tool version-aware

clydin · clydin · commit 68e711307eae · 2025-10-21T06:53:46.000-04:00
Introduces a version-aware mechanism for the `get_best_practices` MCP tool to solve the version-mismatch problem where the AI could provide guidance for a different version of Angular than what the user's project has installed.

The tool now accepts a `workspacePath` to locate the project's installed `@angular/core` package. It reads a new `angular` metadata property from the framework's `package.json` to find the path to a version-specific `best-practices.md` file co-located within the framework package itself.

This change ensures the AI assistant's guidance is perfectly aligned with the project's actual framework version, dramatically increasing its accuracy and reliability.

If a version-specific guide cannot be resolved, the tool gracefully falls back to the generic guide bundled with the CLI to ensure backward compatibility. The core MCP server instructions are also updated to guide the AI on the new workflow.
diff --git a/packages/angular/cli/src/commands/mcp/mcp-server.ts b/packages/angular/cli/src/commands/mcp/mcp-server.ts
@@ -68,11 +68,12 @@ equivalent actions.
 
 <Core Workflows & Tool Guide>
 * **1. Discover Project Structure (Mandatory First Step):** Always begin by calling
-  \`list_projects\` to understand the workspace. The outputs from this tool are often
-  required inputs for other tools.
+  \`list_projects\` to understand the workspace. The \`path\` property for a workspace
+  is a required input for other tools.
 
-* **2. Write & Modify Code:** Before writing or changing code, you MUST consult the
-  \`get_best_practices\` tool to learn the current, non-negotiable coding standards.
+* **2. Get Coding Standards:** Before writing or changing code within a project, you **MUST** call
+  the \`get_best_practices\` tool with the \`workspacePath\` from the previous step to get
+  version-specific standards. For general knowledge, you can call the tool without this path.
 
 * **3. Answer User Questions:**
     - For conceptual questions ("what is..."), use \`search_documentation\`.
diff --git a/packages/angular/cli/src/commands/mcp/tools/best-practices.ts b/packages/angular/cli/src/commands/mcp/tools/best-practices.ts
@@ -6,9 +6,35 @@
  * found in the LICENSE file at https://angular.dev/license
  */
 
-import { readFile } from 'node:fs/promises';
+/**
+ * @fileoverview
+ * This file defines the `get_best_practices` MCP tool. The tool is designed to be version-aware,
+ * dynamically resolving the best practices guide from the user's installed version of
+ * `@angular/core`. It achieves this by reading a custom `angular` metadata block in the
+ * framework's `package.json`. If this resolution fails, it gracefully falls back to a generic
+ * guide bundled with the Angular CLI.
+ */
+
+import { readFile, stat } from 'node:fs/promises';
+import { createRequire } from 'node:module';
 import path from 'node:path';
-import { declareTool } from './tool-registry';
+import { z } from 'zod';
+import { VERSION } from '../../../utilities/version';
+import { McpToolContext, declareTool } from './tool-registry';
+
+const bestPracticesInputSchema = z.object({
+  workspacePath: z
+    .string()
+    .optional()
+    .describe(
+      'The absolute path to the `angular.json` file for the workspace. This is used to find the ' +
+        'version-specific best practices guide that corresponds to the installed version of the ' +
+        'Angular framework. You **MUST** get this path from the `list_projects` tool. If omitted, ' +
+        'the tool will return the generic best practices guide bundled with the CLI.',
+    ),
+});
+
+type BestPracticesInput = z.infer<typeof bestPracticesInputSchema>;
 
 export const BEST_PRACTICES_TOOL = declareTool({
   name: 'get_best_practices',
@@ -24,33 +50,173 @@ that **MUST** be followed for any task involving the creation, analysis, or modi
 * To verify that existing code aligns with current Angular conventions before making changes.
 </Use Cases>
 <Operational Notes>
+* **Project-Specific Use (Recommended):** For tasks inside a user's project, you **MUST** provide the
+  \`workspacePath\` argument to get the guide that matches the project's Angular version. Get this
+  path from \`list_projects\`.
+* **General Use:** If no project context is available (e.g., for general questions or learning),
+  you can call the tool without the \`workspacePath\` argument. It will return the latest
+  generic best practices guide.
 * The content of this guide is non-negotiable and reflects the official, up-to-date standards for Angular development.
 * You **MUST** internalize and apply the principles from this guide in all subsequent Angular-related tasks.
 * Failure to adhere to these best practices will result in suboptimal and outdated code.
 </Operational Notes>`,
+  inputSchema: bestPracticesInputSchema.shape,
   isReadOnly: true,
   isLocalOnly: true,
-  factory: () => {
-    let bestPracticesText: string;
+  factory: createBestPracticesHandler,
+});
+
+/**
+ * Retrieves the content of the generic best practices guide that is bundled with the CLI.
+ * This serves as a fallback when a version-specific guide cannot be found.
+ * @returns A promise that resolves to the string content of the bundled markdown file.
+ */
+async function getBundledBestPractices(): Promise<string> {
+  return readFile(path.join(__dirname, '..', 'resources', 'best-practices.md'), 'utf-8');
+}
+
+/**
+ * Attempts to find and read a version-specific best practices guide from the user's installed
+ * version of `@angular/core`. It looks for a custom `angular` metadata property in the
+ * framework's `package.json` to locate the guide.
+ *
+ * @example A sample `package.json` `angular` field:
+ * ```json
+ * {
+ *   "angular": {
+ *     "bestPractices": {
+ *       "format": "markdown",
+ *       "path": "./resources/best-practices.md"
+ *     }
+ *   }
+ * }
+ * ```
+ *
+ * @param workspacePath The absolute path to the user's `angular.json` file.
+ * @param logger The MCP tool context logger for reporting warnings.
+ * @returns A promise that resolves to an object containing the guide's content and source,
+ *     or `undefined` if the guide could not be resolved.
+ */
+async function getVersionSpecificBestPractices(
+  workspacePath: string,
+  logger: McpToolContext['logger'],
+): Promise<{ content: string; source: string } | undefined> {
+  // 1. Resolve the path to package.json
+  let pkgJsonPath: string;
+  try {
+    const workspaceRequire = createRequire(workspacePath);
+    pkgJsonPath = workspaceRequire.resolve('@angular/core/package.json');
+  } catch (e) {
+    logger.warn(
+      `Could not resolve '@angular/core/package.json' from '${workspacePath}'. ` +
+        'Is Angular installed in this project? Falling back to the bundled guide.',
+    );
+
+    return undefined;
+  }
 
-    return async () => {
-      bestPracticesText ??= await readFile(
-        path.join(__dirname, '..', 'resources', 'best-practices.md'),
-        'utf-8',
+  // 2. Read and parse package.json, then find and read the guide.
+  try {
+    const pkgJsonContent = await readFile(pkgJsonPath, 'utf-8');
+    const pkgJson = JSON.parse(pkgJsonContent);
+    const bestPracticesInfo = pkgJson['angular']?.bestPractices;
+
+    if (
+      bestPracticesInfo &&
+      bestPracticesInfo.format === 'markdown' &&
+      typeof bestPracticesInfo.path === 'string'
+    ) {
+      const packageDirectory = path.dirname(pkgJsonPath);
+      const guidePath = path.resolve(packageDirectory, bestPracticesInfo.path);
+
+      // Ensure the resolved guide path is within the package boundary.
+      // Uses path.relative to create a cross-platform, case-insensitive check.
+      // If the relative path starts with '..' or is absolute, it is a traversal attempt.
+      const relativePath = path.relative(packageDirectory, guidePath);
+      if (relativePath.startsWith('..') || path.isAbsolute(relativePath)) {
+        logger.warn(
+          `Detected a potential path traversal attempt in '${pkgJsonPath}'. ` +
+            `The path '${bestPracticesInfo.path}' escapes the package boundary. ` +
+            'Falling back to the bundled guide.',
+        );
+
+        return undefined;
+      }
+
+      // Check the file size to prevent reading a very large file.
+      const stats = await stat(guidePath);
+      if (stats.size > 1024 * 1024) {
+        // 1MB
+        logger.warn(
+          `The best practices guide at '${guidePath}' is larger than 1MB (${stats.size} bytes). ` +
+            'This is unexpected and the file will not be read. Falling back to the bundled guide.',
+        );
+
+        return undefined;
+      }
+
+      const content = await readFile(guidePath, 'utf-8');
+      const source = `framework version ${pkgJson.version}`;
+
+      return { content, source };
+    } else {
+      logger.warn(
+        `Did not find valid 'angular.bestPractices' metadata in '${pkgJsonPath}'. ` +
+          'Falling back to the bundled guide.',
       );
+    }
+  } catch (e) {
+    logger.warn(
+      `Failed to read or parse version-specific best practices referenced in '${pkgJsonPath}': ${
+        e instanceof Error ? e.message : e
+      }. Falling back to the bundled guide.`,
+    );
+  }
+
+  return undefined;
+}
 
-      return {
-        content: [
-          {
-            type: 'text',
-            text: bestPracticesText,
-            annotations: {
-              audience: ['assistant'],
-              priority: 0.9,
-            },
+/**
+ * Creates the handler function for the `get_best_practices` tool.
+ * The handler orchestrates the process of first attempting to get a version-specific guide
+ * and then falling back to the bundled guide if necessary.
+ * @param context The MCP tool context, containing the logger.
+ * @returns An async function that serves as the tool's executor.
+ */
+function createBestPracticesHandler({ logger }: McpToolContext) {
+  let bundledBestPractices: Promise<string>;
+
+  return async (input: BestPracticesInput) => {
+    let content: string | undefined;
+    let source: string | undefined;
+
+    // First, try to get the version-specific guide.
+    if (input.workspacePath) {
+      const versionSpecific = await getVersionSpecificBestPractices(input.workspacePath, logger);
+      if (versionSpecific) {
+        content = versionSpecific.content;
+        source = versionSpecific.source;
+      }
+    }
+
+    // If the version-specific guide was not found for any reason, fall back to the bundled version.
+    if (content === undefined) {
+      content = await (bundledBestPractices ??= getBundledBestPractices());
+      source = `bundled (CLI v${VERSION.full})`;
+    }
+
+    return {
+      content: [
+        {
+          type: 'text' as const,
+          text: content,
+          annotations: {
+            audience: ['assistant'],
+            priority: 0.9,
+            source,
           },
-        ],
-      };
+        },
+      ],
     };
-  },
-});
+  };
+}
