PDX-469: chore(mcp) — harden MCP infra: typo guard, release token, diff scoping, test isolation

.github/workflows/DeployManual.yml

@@ -24,6 +24,13 @@ jobs:
           registry-url: 'https://registry.npmjs.org'
           scope: '@provartesting'
       - name: Install dependencies and build
+        env:
+          # Required by scripts/fetch-nitrox-packages.cjs to pull the latest
+          # NitroX schemas and component catalog from ProvarTesting/factPackages
+          # at release time. Without this, prepack falls back to the bundled
+          # snapshots and ships them as-is. The auto-provided GITHUB_TOKEN has
+          # read access to factPackages.
+          GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
         run: |
           npm install -g @salesforce/cli
           yarn

src/mcp/server.ts

@@ -81,17 +81,38 @@ export interface ServerConfig {
 export function parseActiveGroups(): Set<string> | null {
   const env = process.env['PROVAR_MCP_TOOLS'];
   if (!env?.trim()) return null;
-  const groups = new Set(
+  const requested = new Set(
     env
       .split(',')
       .map((g) => g.trim().toLowerCase())
       .filter(Boolean)
   );
-  if (groups.size === 0) {
+  if (requested.size === 0) {
     log('warn', 'PROVAR_MCP_TOOLS was set but contained no valid group names — activating all groups', { raw: env });
     return null;
   }
-  return groups;
+  const known = new Set(Object.keys(TOOL_GROUPS));
+  const matched = new Set<string>();
+  const unknown: string[] = [];
+  for (const g of requested) {
+    if (known.has(g)) matched.add(g);
+    else unknown.push(g);
+  }
+  if (unknown.length > 0) {
+    log('warn', 'PROVAR_MCP_TOOLS contains unknown group names — they will be ignored', {
+      raw: env,
+      unknown,
+      known: [...known],
+    });
+  }
+  if (matched.size === 0) {
+    log('warn', 'PROVAR_MCP_TOOLS matched no known group names — activating all groups', {
+      raw: env,
+      known: [...known],
+    });
+    return null;
+  }
+  return matched;
 }
 
 export function createProvarMcpServer(config: ServerConfig): McpServer {

src/mcp/tools/projectValidateFromPath.ts

@@ -23,6 +23,7 @@ import {
   hasAnyRun,
   loadBaselineViolations,
   computeDiff,
+  computeContextHash,
   type DiffableViolation,
 } from '../utils/validationDiff.js';
 import { desc } from './descHelper.js';
@@ -297,6 +298,7 @@ export function registerProjectValidateFromPath(server: McpServer, config: Serve
         if (results_dir) assertPathAllowed(results_dir, config.allowedPaths);
 
         const storageDir = results_dir ?? path.join(project_path, 'provardx', 'validation');
+        const contextHash = computeContextHash('project', project_path);
         const runId = generateRunId(project_path);
 
         const result = validateProjectFromPath({
@@ -318,14 +320,14 @@ export function registerProjectValidateFromPath(server: McpServer, config: Serve
         // Load baseline BEFORE saving to prevent eviction of the requested baseline.
         const baseline =
           baseline_run_id !== undefined && baseline_run_id !== ''
-            ? loadBaselineViolations(storageDir, baseline_run_id)
+            ? loadBaselineViolations(storageDir, baseline_run_id, contextHash)
             : null;
 
         const hasBaseline = hasAnyRun(storageDir);
 
         if (save_results !== false) {
           try {
-            saveRun(storageDir, runId, currentViolations);
+            saveRun(storageDir, runId, currentViolations, contextHash);
           } catch (saveErr) {
             log('warn', 'provar_project_validate: could not save run for diff', {
               requestId,

src/mcp/tools/testCaseValidate.ts

@@ -7,7 +7,6 @@
 
 /* eslint-disable camelcase */
 import fs from 'node:fs';
-import os from 'node:os';
 import path from 'node:path';
 import { createHash } from 'node:crypto';
 import { z } from 'zod';
@@ -33,6 +32,8 @@ import {
   hasAnyRun,
   loadBaselineViolations,
   computeDiff,
+  computeContextHash,
+  resolveValidationDir,
   type DiffableViolation,
 } from '../utils/validationDiff.js';
 import { runBestPractices } from './bestPracticesEngine.js';
@@ -67,7 +68,7 @@ const TC_VALIDATE_SUMMARY_FIELDS = [
 
 /** Storage dir for testcase diff runs (namespaced to avoid cross-tool baseline collisions). */
 function tcStorageDir(): string {
-  return path.join(os.homedir(), '.provardx', 'validation', 'testcase');
+  return resolveValidationDir('testcase');
 }
 
 /** Resolve validation result from QualityHub API or fall back to local. */
@@ -183,7 +184,9 @@ export function registerTestCaseValidate(server: McpServer, config: ServerConfig
         const baseResult = await resolveBaseResult(source, apiKey, requestId);
 
         const storageDir = tcStorageDir();
-        const runId = generateRunId(tcRunContext(file_path, source));
+        const context = tcRunContext(file_path, source);
+        const contextHash = computeContextHash('tc', context);
+        const runId = generateRunId(context);
         const bpViolations = (baseResult.best_practices_violations ?? []) as unknown as DiffableViolation[];
         const currentViolations: DiffableViolation[] = [
           ...(baseResult.issues as unknown as DiffableViolation[]),
@@ -193,13 +196,13 @@ export function registerTestCaseValidate(server: McpServer, config: ServerConfig
         // Load baseline BEFORE saving to prevent eviction of the requested baseline
         const baseline =
           baseline_run_id !== undefined && baseline_run_id !== ''
-            ? loadBaselineViolations(storageDir, baseline_run_id)
+            ? loadBaselineViolations(storageDir, baseline_run_id, contextHash)
             : null;
 
         const hasBaseline = hasAnyRun(storageDir);
 
         try {
-          saveRun(storageDir, runId, currentViolations);
+          saveRun(storageDir, runId, currentViolations, contextHash);
         } catch (saveErr) {
           log('warn', 'provar_testcase_validate: could not save run for diff', {
             requestId,

src/mcp/tools/testSuiteValidate.ts

@@ -6,8 +6,6 @@
  */
 
 /* eslint-disable camelcase */
-import os from 'node:os';
-import path from 'node:path';
 import { z } from 'zod';
 import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js';
 import { makeError, makeRequestId } from '../schemas/common.js';
@@ -20,6 +18,8 @@ import {
   hasAnyRun,
   loadBaselineViolations,
   computeDiff,
+  computeContextHash,
+  resolveValidationDir,
   type DiffableViolation,
 } from '../utils/validationDiff.js';
 import { validateSuite, buildHierarchySummary, type TestSuiteInput, type SuiteResult } from './hierarchyValidate.js';
@@ -78,7 +78,7 @@ const SUITE_VALIDATE_SUMMARY_FIELDS = [
 ];
 
 function suiteStorageDir(): string {
-  return path.join(os.homedir(), '.provardx', 'validation', 'testsuite');
+  return resolveValidationDir('testsuite');
 }
 
 export function registerTestSuiteValidate(server: McpServer): void {
@@ -154,19 +154,20 @@ export function registerTestSuiteValidate(server: McpServer): void {
         const summary = buildHierarchySummary(result);
 
         const storageDir = suiteStorageDir();
+        const contextHash = computeContextHash('suite', suite_name);
         const runId = generateRunId(suite_name);
         const currentViolations = collectAllViolations(result);
 
         // Load baseline BEFORE saving to prevent eviction of the requested baseline
         const baseline =
           baseline_run_id !== undefined && baseline_run_id !== ''
-            ? loadBaselineViolations(storageDir, baseline_run_id)
+            ? loadBaselineViolations(storageDir, baseline_run_id, contextHash)
             : null;
 
         const hasBaseline = hasAnyRun(storageDir);
 
         try {
-          saveRun(storageDir, runId, currentViolations);
+          saveRun(storageDir, runId, currentViolations, contextHash);
         } catch (saveErr) {
           log('warn', 'provar_testsuite_validate: could not save run for diff', {
             requestId,

src/mcp/utils/validationDiff.ts

@@ -7,11 +7,14 @@
 
 /* eslint-disable camelcase */
 import fs from 'node:fs';
+import os from 'node:os';
 import path from 'node:path';
 import { createHash } from 'node:crypto';
 
 const MAX_RUNS = 20;
 const INDEX_FILE = '.runs.json';
+const DEFAULT_ROOT_NAME = '.provardx';
+const VALIDATION_SUBDIR = 'validation';
 
 // ── Public types ──────────────────────────────────────────────────────────────
 
@@ -28,6 +31,15 @@ interface RunRecord {
   run_id: string;
   timestamp: number;
   filename: string;
+  /**
+   * Hash of `${toolTag}|${context}`. Used by loadBaselineViolations to reject
+   * a run_id whose context (file path, suite name, etc.) does not match the
+   * calling context — prevents cross-context diffs. Optional for backward
+   * compatibility with index records written before this field existed; those
+   * older records are treated as not matching any caller and are effectively
+   * invalidated within one or two new runs as the FIFO cap evicts them.
+   */
+  context_hash?: string;
 }
 
 interface RunsIndex {
@@ -67,6 +79,27 @@ function saveIndex(storageDir: string, index: RunsIndex): void {
 
 // ── Public API ────────────────────────────────────────────────────────────────
 
+/**
+ * Compute a stable 8-char context hash for a tool + context pair. Used to
+ * scope baseline run lookups so that a run_id from context A cannot be diffed
+ * against context B (different project, different suite, different file).
+ */
+export function computeContextHash(toolTag: string, context: string): string {
+  return shortHash(`${toolTag}|${context}`);
+}
+
+/**
+ * Resolve the validation storage root for a given tool subdir. Honors the
+ * PROVAR_MCP_VALIDATION_DIR env var when set; otherwise falls back to
+ * `~/.provardx/validation/<subdir>`. The env override is useful for restricted
+ * CI/dev environments where the home directory is read-only or shared.
+ */
+export function resolveValidationDir(subdir: string): string {
+  const override = process.env['PROVAR_MCP_VALIDATION_DIR']?.trim();
+  if (override) return path.join(override, subdir);
+  return path.join(os.homedir(), DEFAULT_ROOT_NAME, VALIDATION_SUBDIR, subdir);
+}
+
 /** Generate a run ID from a context string (e.g. project path or suite name). */
 export function generateRunId(context: string): string {
   const rand = Math.random().toString(36).slice(2, 6);
@@ -86,15 +119,29 @@ export function hasAnyRun(storageDir: string): boolean {
  * Save the current violations as a new run in the storage directory.
  * Caps the index at MAX_RUNS by evicting the oldest entry when full.
  * Returns the generated run_id.
+ *
+ * When `contextHash` is provided, it is recorded alongside the run so that
+ * `loadBaselineViolations` can reject a baseline_run_id whose context does
+ * not match the calling context (prevents cross-context diffs).
  */
-export function saveRun(storageDir: string, runId: string, violations: DiffableViolation[]): string {
+export function saveRun(
+  storageDir: string,
+  runId: string,
+  violations: DiffableViolation[],
+  contextHash?: string
+): string {
   fs.mkdirSync(storageDir, { recursive: true });
 
   const filename = `${runId}.json`;
   fs.writeFileSync(path.join(storageDir, filename), JSON.stringify(violations), 'utf-8');
 
   const index = loadIndex(storageDir);
-  index.runs.push({ run_id: runId, timestamp: Date.now(), filename });
+  index.runs.push({
+    run_id: runId,
+    timestamp: Date.now(),
+    filename,
+    ...(contextHash ? { context_hash: contextHash } : {}),
+  });
 
   // Evict oldest entries when over the cap
   while (index.runs.length > MAX_RUNS) {
@@ -117,12 +164,26 @@ export function saveRun(storageDir: string, runId: string, violations: DiffableV
  * Returns null if the run is not found in the index (BASELINE_NOT_FOUND).
  * The filename is looked up from the index only — the run_id itself is never
  * used to construct a file path, preventing path traversal.
+ *
+ * When `expectedContextHash` is provided, the record's `context_hash` must
+ * match. Records without a `context_hash` (written by older versions before
+ * H3) are treated as a mismatch and are effectively retired within one or
+ * two new runs as the FIFO cap evicts them. This guard prevents diffing a
+ * baseline from a different file/suite/project against the current context.
  */
-export function loadBaselineViolations(storageDir: string, baselineRunId: string): DiffableViolation[] | null {
+export function loadBaselineViolations(
+  storageDir: string,
+  baselineRunId: string,
+  expectedContextHash?: string
+): DiffableViolation[] | null {
   const index = loadIndex(storageDir);
   const record = index.runs.find((r) => r.run_id === baselineRunId);
   if (!record) return null;
 
+  if (expectedContextHash !== undefined && record.context_hash !== expectedContextHash) {
+    return null;
+  }
+
   // Use the filename from the index, not the run_id
   try {
     const content = fs.readFileSync(path.join(storageDir, record.filename), 'utf-8');

test/unit/mcp/startupTuning.test.ts

@@ -155,6 +155,25 @@ describe('parseActiveGroups() (PDX-469)', () => {
     process.env['PROVAR_MCP_TOOLS'] = ',,';
     assert.equal(parseActiveGroups(), null);
   });
+
+  // ── H1: unknown group names ───────────────────────────────────────────────
+  it('returns null when every requested group name is unknown (typo footgun)', () => {
+    // Pre-fix: a typo like 'validaton' produced Set{'validaton'} which matched
+    // no group and silently disabled all tools. Now we fall back to null (all
+    // tools) so the server is never left with an empty Provar tool surface.
+    process.env['PROVAR_MCP_TOOLS'] = 'validaton';
+    assert.equal(parseActiveGroups(), null);
+  });
+
+  it('keeps known names and ignores unknown ones in a mixed list', () => {
+    process.env['PROVAR_MCP_TOOLS'] = 'nitroX,bogusgroup,validation';
+    const groups = parseActiveGroups();
+    assert.ok(groups instanceof Set);
+    assert.equal(groups.size, 2);
+    assert.ok(groups.has('nitrox'));
+    assert.ok(groups.has('validation'));
+    assert.ok(!groups.has('bogusgroup'));
+  });
 });
 
 // ── PDX-469: tool profile registration ────────────────────────────────────────

test/unit/mcp/testSuiteValidate.test.ts

@@ -7,7 +7,10 @@
 
 /* eslint-disable camelcase */
 import { strict as assert } from 'node:assert';
-import { describe, it, beforeEach } from 'mocha';
+import fs from 'node:fs';
+import os from 'node:os';
+import path from 'node:path';
+import { describe, it, beforeEach, afterEach } from 'mocha';
 import { registerTestSuiteValidate } from '../../../src/mcp/tools/testSuiteValidate.js';
 
 // ── Minimal McpServer mock ─────────────────────────────────────────────────────
@@ -70,18 +73,33 @@ const TC_LOGOUT = { name: 'LogoutTest.testcase', xml_content: makeXml(G.tc2, G.s
 const TC_LOGIN_ALIAS = { name: 'LoginTest.testcase', xml: makeXml(G.tc1, G.s1, 'tc-001') };
 const TC_LOGOUT_ALIAS = { name: 'LogoutTest.testcase', xml: makeXml(G.tc2, G.s2, 'tc-002') };
 
-// ── Test setup ─────────────────────────────────────────────────────────────────
-
-let server: MockMcpServer;
-
-beforeEach(() => {
-  server = new MockMcpServer();
-  registerTestSuiteValidate(server as never);
-});
-
 // ── provar_testsuite_validate ─────────────────────────────────────────────────
 
 describe('provar_testsuite_validate', () => {
+  let server: MockMcpServer;
+  let origHomedir: () => string;
+  let tempHome: string;
+
+  beforeEach(() => {
+    // Redirect os.homedir() into a temp dir so suiteStorageDir() writes to
+    // an isolated location instead of polluting the real developer/CI home.
+    // NOTE: scoped INSIDE this describe so the stub does not leak into other
+    // test files. Mocha root-level beforeEach attaches to the root suite and
+    // runs before every test in every file — see auth/rotate.test.ts which
+    // relies on the real os.homedir() and would otherwise see this stub.
+    tempHome = fs.mkdtempSync(path.join(os.tmpdir(), 'pvts-home-'));
+    origHomedir = os.homedir;
+    (os as unknown as { homedir: () => string }).homedir = (): string => tempHome;
+
+    server = new MockMcpServer();
+    registerTestSuiteValidate(server as never);
+  });
+
+  afterEach(() => {
+    (os as unknown as { homedir: () => string }).homedir = origHomedir;
+    fs.rmSync(tempHome, { recursive: true, force: true });
+  });
+
   describe('happy path', () => {
     it('returns a result (not an error) for a valid non-empty suite', () => {
       const result = server.call('provar_testsuite_validate', {