PDX-489: feat(mcp) — DATA-001 warning for dataTable in direct testCase-mode (Thread E)

docs/mcp.md

@@ -50,14 +50,15 @@ The Provar DX CLI ships with a built-in **Model Context Protocol (MCP) server**
   - [provar_testplan_add-instance](#provar_testplan_add-instance)
   - [provar_testplan_create-suite](#provar_testplan_create-suite)
   - [provar_testplan_remove-instance](#provar_testplan_remove-instance)
-  - [NitroX — Hybrid Model page objects](#nitrox--hybrid-model-page-objects)
-    - [provar_nitrox_discover](#provar_nitrox_discover)
-    - [provar_nitrox_read](#provar_nitrox_read)
-    - [provar_nitrox_validate](#provar_nitrox_validate)
-    - [provar_nitrox_generate](#provar_nitrox_generate)
-    - [provar_nitrox_patch](#provar_nitrox_patch)
-  - [Quality Hub API tools](#quality-hub-api-tools)
-    - [provar_qualityhub_examples_retrieve](#provar_qualityhub_examples_retrieve)
+- [Data-driven execution](#data-driven-execution)
+- [NitroX — Hybrid Model page objects](#nitrox--hybrid-model-page-objects)
+  - [provar_nitrox_discover](#provar_nitrox_discover)
+  - [provar_nitrox_read](#provar_nitrox_read)
+  - [provar_nitrox_validate](#provar_nitrox_validate)
+  - [provar_nitrox_generate](#provar_nitrox_generate)
+  - [provar_nitrox_patch](#provar_nitrox_patch)
+- [Quality Hub API tools](#quality-hub-api-tools)
+  - [provar_qualityhub_examples_retrieve](#provar_qualityhub_examples_retrieve)
   - [Org metadata via Salesforce Hosted MCP](#org-metadata-via-salesforce-hosted-mcp)
 - [MCP Prompts](#mcp-prompts)
   - [Migration prompts](#migration-prompts)
@@ -78,6 +79,7 @@ The Provar DX CLI ships with a built-in **Model Context Protocol (MCP) server**
 - [Quality scores explained](#quality-scores-explained)
 - [API compatibility — `xml` vs `xml_content`](#api-compatibility--xml-vs-xml_content)
 - [Performance Tuning](#performance-tuning)
+- [Warning codes](#warning-codes)
 
 ---
 
@@ -531,6 +533,23 @@ On **Windows**, path comparisons are performed case-insensitively to account for
 
 ---
 
+## Warning codes
+
+Cross-cutting warning codes surfaced by validation, configuration, and run tooling. These complement the per-tool `rule_id` codes (e.g. `TC_001`, `VAR-REF-001`) documented under [Available tools](#available-tools). Subsequent revisions will refine the meanings as the relevant tool surfaces stabilise.
+
+| Code             | Surfaced by                             | Meaning                                                                                                                                           |
+| ---------------- | --------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------- |
+| `PROVARHOME-001` | properties / automation tooling         | `provarHome` is missing, blank, or does not point to a Provar install                                                                             |
+| `DATA-001`       | `provar_testcase_validate`              | `<dataTable>` iteration is silently ignored when a test case runs in direct `testCase`-mode (see [Data-driven execution](#data-driven-execution)) |
+| `PARALLEL-001`   | automation / run tooling                | Parallel-mode cache mismatch between properties and active runtime config                                                                         |
+| `SCHEMA-001`     | strict properties / config validators   | Unknown or misspelled key in a JSON / properties schema (typo guard)                                                                              |
+| `RUN-001`        | `provar_automation_testrun` and friends | Test run produced no executable results — check input selection                                                                                   |
+| `JUNIT-001`      | report / RCA tooling                    | JUnit results file is missing, empty, or not parseable                                                                                            |
+
+Warnings emitted programmatically follow the shape `WARNING [<CODE>]: <message>` — and when a typo is detected, the message is suffixed with `Did you mean '<suggestion>'?`. See `src/mcp/utils/warningCodes.ts` for the canonical enum.
+
+---
+
 ## Available tools
 
 ### `provardx_ping`
@@ -825,7 +844,7 @@ Validates an XML test case for schema correctness (validity score) and best prac
 
 **Warning rules:**
 
-- **DATA-001** — `testCase` declares a `<dataTable>` element. CLI standalone execution does not bind CSV column variables; steps using variable references will resolve to null. Use `SetValues` (Test scope) steps instead, or add the test to a test plan.
+- **DATA-001** — `testCase` declares a `<dataTable>` element. When the validator is called with `file_path` and the project's `provardx-properties.json` references that test case directly via top-level `testCase` or `testCases` (rather than via a `.testinstance` inside a plan), the warning carries the `WARNING [DATA-001]:` prefix and recommends wiring the test into a plan via `provar_testplan_add-instance`. When `file_path` is not supplied (or the project context cannot be resolved), the warning falls back to a structural advisory recommending `SetValues` (Test scope) steps. The warning is suppressed entirely when a `.testinstance` references the test case, because data-driven iteration works in that mode. See also [Data-driven execution](#data-driven-execution).
 - **ASSERT-001** — An `AssertValues` step uses the `argument id="values"` (namedValues) format, which is designed for UI element attribute assertions. For Apex/SOQL result or variable comparisons this silently passes as `null=null`. Use separate `expectedValue`, `actualValue`, and `comparisonType` arguments instead.
 - **UI-TARGET-001** — A UiWithScreen or UiWithRow `target` argument uses the wrong XML class (e.g. `class="value"`). Must be `class="uiTarget"` or the screen binding is silently ignored at runtime.
 - **UI-LOCATOR-001** — A UiDoAction or UiAssert `locator` argument uses the wrong XML class. Must be `class="uiLocator"` or Provar cannot resolve the element.
@@ -1684,6 +1703,38 @@ Remove a `.testinstance` file from a plan suite. Path is validated to stay withi
 
 ---
 
+## Data-driven execution
+
+Provar's data-driven execution relies on the `<dataTable>` element inside a `.testcase` XML. The runtime only **iterates rows** when the test case is launched through a test-plan instance (a `.testinstance` file under `plans/`). When the same test case is launched directly via the top-level `testCase` or `testCases` property in `provardx-properties.json`, the runtime ignores the data table entirely — every step referencing a `<value class="variable">` resolves to `null`, and the test typically completes "successfully" against an empty row set.
+
+This produces silent-pass behaviour that is hard to spot from a log: the run exits 0, JUnit shows one test case, and the data-driven assertions never fire. The MCP server detects this configuration mismatch and surfaces a **`DATA-001`** warning so an AI agent can recover before the next run.
+
+**When does `DATA-001` fire?**
+
+| Condition (validator called with `file_path`)                                                | DATA-001 emitted? | Severity |
+| -------------------------------------------------------------------------------------------- | ----------------- | -------- |
+| `<dataTable>` present **and** referenced from a `.testinstance` inside `plans/`              | No                | —        |
+| `<dataTable>` present **and** referenced via top-level `testCase` / `testCases` array        | Yes               | WARNING  |
+| `<dataTable>` present **and** project context cannot be resolved (no active properties file) | Yes (structural)  | WARNING  |
+| No `<dataTable>` element                                                                     | No                | —        |
+
+The plan-mode resolver consults the properties file registered by [`provar_automation_config_load`](#provar_automation_config_load) (`PROVARDX_PROPERTIES_FILE_PATH` in `~/.sf/config.json`), reads `projectPath`, then:
+
+1. Walks `<projectPath>/plans/**/*.testinstance` for any `testCasePath="..."` referencing the test under validation. If found → `plan` mode → DATA-001 suppressed.
+2. Otherwise checks `testCase` / `testCases` for a direct reference. If found → `direct` mode → DATA-001 with the PDX-489 advisory.
+3. Falls back to `unknown` mode when no project context is resolvable — DATA-001 still fires (structural fallback) so authors editing a test case in isolation are still warned.
+
+**Recommended workaround**
+
+When `DATA-001` fires in direct mode, wire the test case into a plan via [`provar_testplan_add-instance`](#provar_testplan_add-instance) and run via the `testPlan` property in `provardx-properties.json` instead of `testCase` / `testCases`. The pattern is:
+
+1. Use [`provar_testplan_create-suite`](#provar_testplan_create-suite) to add a suite under an existing plan if needed.
+2. Use [`provar_testplan_add-instance`](#provar_testplan_add-instance) to create the `.testinstance` linking the test case to the suite.
+3. Update `provardx-properties.json` to reference the plan (and remove the direct `testCase` entry if it no longer applies) before invoking [`provar_automation_testrun`](#provar_automation_testrun).
+4. Re-run [`provar_testcase_validate`](#provar_testcase_validate) on the test case file — DATA-001 should no longer appear.
+
+The constraint is also referenced in the [`provar_testcase_generate`](#provar_testcase_generate) tool description so an agent constructing a new data-driven test case sees the limitation up front, and in [`provar_automation_testrun`](#provar_automation_testrun) so an agent triggering a run is reminded that direct-mode execution will not iterate.
+
 ---
 
 ## NitroX — Hybrid Model page objects

src/mcp/tools/testCaseValidate.ts

@@ -36,6 +36,8 @@ import {
   resolveValidationDir,
   type DiffableViolation,
 } from '../utils/validationDiff.js';
+import { resolveTestCasePlanMode, type TestCasePlanMode } from '../utils/testCasePlanMode.js';
+import { WARNING_CODES, formatWarning } from '../utils/warningCodes.js';
 import { runBestPractices } from './bestPracticesEngine.js';
 import { desc } from './descHelper.js';
 
@@ -75,15 +77,20 @@ function tcStorageDir(): string {
 async function resolveBaseResult(
   source: string,
   apiKey: string | null,
-  requestId: string
+  requestId: string,
+  planMode: TestCasePlanMode = 'unknown'
 ): Promise<TestCaseValidationResult> {
   if (!apiKey) {
-    return { ...validateTestCase(source), validation_source: 'local', validation_warning: ONBOARDING_MESSAGE };
+    return {
+      ...validateTestCase(source, undefined, { planMode }),
+      validation_source: 'local',
+      validation_warning: ONBOARDING_MESSAGE,
+    };
   }
   const baseUrl = getQualityHubBaseUrl();
   try {
     const apiResult = await qualityHubClient.validateTestCaseViaApi(source, apiKey, baseUrl);
-    const localMeta = validateTestCase(source);
+    const localMeta = validateTestCase(source, undefined, { planMode });
     log('info', 'provar_testcase_validate: quality_hub', { requestId });
     return {
       ...apiResult,
@@ -107,7 +114,11 @@ async function resolveBaseResult(
       warning = UNREACHABLE_WARNING;
       log('warn', 'provar_testcase_validate: api unreachable, falling back', { requestId });
     }
-    return { ...validateTestCase(source), validation_source: 'local_fallback', validation_warning: warning };
+    return {
+      ...validateTestCase(source, undefined, { planMode }),
+      validation_source: 'local_fallback',
+      validation_warning: warning,
+    };
   }
 }
 
@@ -123,7 +134,7 @@ export function registerTestCaseValidate(server: McpServer, config: ServerConfig
     {
       title: 'Validate Test Case',
       description: desc(
-        'Validate a Provar XML test case for structural correctness and quality. Checks XML declaration, root element, required attributes (guid UUID v4, testItemId integer), <steps> presence, and applies best-practice rules. When a Provar API key is configured (via sf provar auth login or PROVAR_API_KEY env var), calls the Quality Hub API for full 170-rule scoring. Falls back to local validation if no key is set or the API is unavailable. Returns validity_score (schema compliance), quality_score (best practices, 0–100), and validation_source indicating which ruleset was applied. Every response includes run_id — pass it as baseline_run_id in the next call to receive only new/resolved issues. When structural errors are returned, consult the provar://docs/step-reference MCP resource for correct step attribute schemas.',
+        "Validate a Provar XML test case for structural correctness and quality. Checks XML declaration, root element, required attributes (guid UUID v4, testItemId integer), <steps> presence, and applies best-practice rules. When a Provar API key is configured (via sf provar auth login or PROVAR_API_KEY env var), calls the Quality Hub API for full 170-rule scoring. Falls back to local validation if no key is set or the API is unavailable. Returns validity_score (schema compliance), quality_score (best practices, 0–100), and validation_source indicating which ruleset was applied. Every response includes run_id — pass it as baseline_run_id in the next call to receive only new/resolved issues. Data-driven note (DATA-001): when file_path is supplied and the project's provardx-properties.json references the test case directly via top-level `testCase` / `testCases` rather than via a `.testinstance` inside a plan, the validator emits DATA-001 warning a <dataTable> declaration will resolve all variables to null in direct testCase-mode — wire the test into a plan via provar_testplan_add-instance to enable data-driven iteration. When structural errors are returned, consult the provar://docs/step-reference MCP resource for correct step attribute schemas.",
         'Validate a Provar XML test case: structure, UUIDs, steps, quality scoring; run_id for baseline diff.'
       ),
       inputSchema: {
@@ -181,7 +192,10 @@ export function registerTestCaseValidate(server: McpServer, config: ServerConfig
         }
 
         const apiKey = resolveApiKey();
-        const baseResult = await resolveBaseResult(source, apiKey, requestId);
+        const planMode: TestCasePlanMode = file_path
+          ? resolveTestCasePlanMode({ testCaseFilePath: file_path, allowedPaths: config.allowedPaths }).mode
+          : 'unknown';
+        const baseResult = await resolveBaseResult(source, apiKey, requestId, planMode);
 
         const storageDir = tcStorageDir();
         const context = tcRunContext(file_path, source);
@@ -306,7 +320,11 @@ export function validateTestCaseXml(filePath: string, config: ServerConfig): Tes
   if (!fs.existsSync(resolved)) {
     throw Object.assign(new Error(`File not found: ${resolved}`), { code: 'TESTCASE_FILE_NOT_FOUND' });
   }
-  return validateTestCase(fs.readFileSync(resolved, 'utf-8'));
+  const planMode = resolveTestCasePlanMode({
+    testCaseFilePath: resolved,
+    allowedPaths: config.allowedPaths,
+  }).mode;
+  return validateTestCase(fs.readFileSync(resolved, 'utf-8'), undefined, { planMode });
 }
 
 /** TC_010/TC_011: validate testCase id and guid attributes. */
@@ -348,8 +366,69 @@ function checkTestCaseIdAndGuid(tcId: string | null, tcGuid: string | undefined,
   }
 }
 
+/**
+ * Emit the DATA-001 warning when the test case declares a `<dataTable>`.
+ *
+ * PDX-489: when the validator handler resolves the project's
+ * `provardx-properties.json` and finds the test case is referenced from a
+ * `.testinstance` (plan mode), suppress the warning — data-driven execution
+ * works there. When the project references the test case directly via
+ * top-level `testCase` / `testCases`, emit the PDX-489 advisory. When no
+ * project context is available (plan mode unknown — pure function called
+ * without file resolution), keep the structural advisory so the warning
+ * surface stays backward-compatible.
+ */
+function maybeEmitDataTableWarning(
+  tc: Record<string, unknown>,
+  planMode: TestCasePlanMode,
+  issues: ValidationIssue[]
+): void {
+  const hasDataTable = 'dataTable' in tc && tc['dataTable'] != null;
+  if (!hasDataTable || planMode === 'plan') return;
+  if (planMode === 'direct') {
+    issues.push({
+      rule_id: 'DATA-001',
+      severity: 'WARNING',
+      message: formatWarning(
+        WARNING_CODES.DATA_001,
+        '<dataTable> only iterates when run through a test plan instance. Direct testCase-mode execution will resolve all data-driven variables as null. Add this test to a plan via provar_testplan_add-instance.'
+      ),
+      applies_to: 'testCase',
+      suggestion:
+        'Move this test case under a test plan and reference it through a .testinstance — use provar_testplan_add-instance to wire it up.',
+    });
+    return;
+  }
+  issues.push({
+    rule_id: 'DATA-001',
+    severity: 'WARNING',
+    message:
+      'testCase declares a <dataTable> but CLI standalone execution does not bind CSV column variables — steps using <value class="variable"> references will resolve to null.',
+    applies_to: 'testCase',
+    suggestion:
+      'Use SetValues (Test scope) steps to bind data for standalone CLI execution, or add this test case to a test plan.',
+  });
+}
+
+/**
+ * Validator options threaded through from the MCP handler. `planMode` controls
+ * how DATA-001 (the data-driven `<dataTable>` warning) is surfaced.
+ *
+ * `direct` emits the PDX-489 warning recommending a plan instance. `plan`
+ * suppresses DATA-001 entirely because data-driven execution works under plan
+ * mode. `unknown` (default) emits the structural DATA-001 warning so callers
+ * without project context still receive the original advisory.
+ */
+export interface ValidateTestCaseOptions {
+  planMode?: TestCasePlanMode;
+}
+
 /** Pure function — exported for unit testing */
-export function validateTestCase(xmlContent: string, testName?: string): TestCaseValidationResult {
+export function validateTestCase(
+  xmlContent: string,
+  testName?: string,
+  options: ValidateTestCaseOptions = {}
+): TestCaseValidationResult {
   const issues: ValidationIssue[] = [];
 
   // TC_001: XML declaration
@@ -423,18 +502,7 @@ export function validateTestCase(xmlContent: string, testName?: string): TestCas
     return finalize(issues, tcId, tcName, 0, xmlContent, testName);
   }
 
-  // DATA-001: <dataTable> binding is silently ignored in standalone CLI execution
-  if ('dataTable' in tc && tc['dataTable'] != null) {
-    issues.push({
-      rule_id: 'DATA-001',
-      severity: 'WARNING',
-      message:
-        'testCase declares a <dataTable> but CLI standalone execution does not bind CSV column variables — steps using <value class="variable"> references will resolve to null.',
-      applies_to: 'testCase',
-      suggestion:
-        'Use SetValues (Test scope) steps to bind data for standalone CLI execution, or add this test case to a test plan.',
-    });
-  }
+  maybeEmitDataTableWarning(tc, options.planMode ?? 'unknown', issues);
 
   // Same self-closing guard for <steps/> → fast-xml-parser yields ''
   const rawSteps = tc['steps'];