feat(validate): Wave 3 — XML generator correctness + 4 new local validator rules

docs/mcp.md

@@ -637,6 +637,18 @@ Generates an XML test case skeleton with UUID v4 guids and sequential `testItemI
 - Omit or use a `sf:` URI → flat Salesforce step structure (existing behaviour)
 - `ui:pageobject:target?pageId=pageobjects.Page` → wraps all steps in a `UiWithScreen` element (testItemId=1); substeps clause at testItemId=2; inner steps start at testItemId=3
 
+**Argument XML conventions** (automatically applied by the generator):
+
+| Argument key / value pattern         | Emitted XML class             | API context                         |
+| ------------------------------------ | ----------------------------- | ----------------------------------- |
+| `target` key                         | `class="uiTarget"`            | UiWithScreen, UiWithRow             |
+| `locator` key                        | `class="uiLocator"`           | UiDoAction, UiAssert                |
+| Value matches `{VarName}` or `{A.B}` | `class="variable"` + `<path>` | Any step                            |
+| SetValues attributes                 | `class="valueList"/<namedValues>` | SetValues only                  |
+| All other values                     | `class="value" valueClass="string"` | Any step                      |
+
+AssertValues uses **flat** argument structure (`expectedValue`, `actualValue`, `comparisonType`) — not the `valueList`/namedValues format.
+
 **Input**
 
 | Parameter             | Type                                     | Required | Description                                                                                                               |
@@ -700,6 +712,10 @@ Validates an XML test case for schema correctness (validity score) and best prac
 
 - **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.
 - **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.
+- **SETVALUES-STRUCTURE-001** (ERROR) — A `SetValues` step's `values` argument uses `class="value"` (plain string) instead of `class="valueList"` with `<namedValues>` children. This causes an immediate `ClassCastException` at runtime.
+- **VAR-REF-001** — An argument value looks like a variable reference (`{VarName}` or `{Obj.Field}`) but is stored as `class="value" valueClass="string"`. Provar will treat it as a literal string, not resolve the variable. Replace with `class="variable"` and `<path>` elements.
 
 ---
 

package.json

@@ -1,7 +1,7 @@
 {
   "name": "@provartesting/provardx-cli",
   "description": "A plugin for the Salesforce CLI to orchestrate testing activities and report quality metrics to Provar Quality Hub",
-  "version": "1.5.0-beta.11",
+  "version": "1.5.0-beta.12",
   "mcpName": "io.github.ProvarTesting/provar",
   "license": "BSD-3-Clause",
   "plugins": [

scripts/mcp-smoke.cjs

@@ -248,32 +248,39 @@ async function runTests() {
   // ── 30. provar.testrun.rca ───────────────────────────────────────────────
   await callTool('provar.testrun.rca', { project_path: TMP });
 
-  // ── 31. provar.testplan.add-instance ─────────────────────────────────────
+  // ── 31. provar.testplan.create ────────────────────────────────────────────
+  // TMP is not a Provar project → NOT_A_PROJECT result
+  await callTool('provar.testplan.create', {
+    project_path: TMP,
+    plan_name: 'SmokePlan',
+  });
+
+  // ── 32. provar.testplan.add-instance ─────────────────────────────────────
   // TMP is not a Provar project → NOT_A_PROJECT result
   await callTool('provar.testplan.add-instance', {
     project_path: TMP,
     test_case_path: 'tests/Smoke/SmokeTest.testcase',
     plan_name: 'SmokePlan',
   });
 
-  // ── 32. provar.testplan.create-suite ─────────────────────────────────────
+  // ── 33. provar.testplan.create-suite ─────────────────────────────────────
   await callTool('provar.testplan.create-suite', {
     project_path: TMP,
     plan_name: 'SmokePlan',
     suite_name: 'SmokeSuite',
   });
 
-  // ── 33. provar.testplan.remove-instance ──────────────────────────────────
+  // ── 34. provar.testplan.remove-instance ──────────────────────────────────
   await callTool('provar.testplan.remove-instance', {
     project_path: TMP,
     instance_path: 'plans/SmokePlan/SmokeSuite/smoke.testinstance',
   });
 
-  // ── 34. provar.nitrox.discover ────────────────────────────────────────────
+  // ── 35. provar.nitrox.discover ────────────────────────────────────────────
   // TMP has no .testproject → empty projects list, no crash
   await callTool('provar.nitrox.discover', { search_roots: [TMP] });
 
-  // ── 35. provar.nitrox.validate ────────────────────────────────────────────
+  // ── 36. provar.nitrox.validate ────────────────────────────────────────────
   // Minimal valid root component → score 100
   await callTool('provar.nitrox.validate', {
     content: JSON.stringify({
@@ -376,8 +383,8 @@ async function runTests() {
 // ----------------------------------------------------------------------------
 server.on('close', () => {
   clearTimeout(overallTimer);
-  // initialize + tools/list + 39 tools + prompts/list + 8 prompts/get (setup excluded from default count)
-  const TOTAL_EXPECTED = 50 + (INCLUDE_SETUP ? 1 : 0);
+  // initialize + tools/list + 40 tools + prompts/list + 8 prompts/get (setup excluded from default count)
+  const TOTAL_EXPECTED = 51 + (INCLUDE_SETUP ? 1 : 0);
   let passed = 0;
   let failed = 0;
 

server.json

@@ -14,12 +14,12 @@
     "url": "https://github.com/ProvarTesting/provardx-cli",
     "source": "github"
   },
-  "version": "1.5.0-beta.11",
+  "version": "1.5.0-beta.12",
   "packages": [
     {
       "registryType": "npm",
       "identifier": "@provartesting/provardx-cli",
-      "version": "1.5.0-beta.11",
+      "version": "1.5.0-beta.12",
       "transport": {
         "type": "stdio"
       },

src/mcp/tools/testCaseGenerate.ts

@@ -69,6 +69,17 @@ function buildStepWarnings(steps: Array<{ api_id: string }>): string[] {
     );
   }
 
+  // D7: Cleanup steps placed after a potential failure point are skipped when stopOnError=false.
+  if (resolvedIds.includes(SHORTHAND_TO_FQID['ApexDeleteObject'] ?? '')) {
+    warnings.push(
+      'ApexDeleteObject detected (likely cleanup): with stopOnError=false Provar skips all steps after ' +
+        'the first failure, so cleanup steps placed at the end of the test will NOT run when an earlier ' +
+        'step fails — leaving orphaned records in the org. ' +
+        'Wrap cleanup in a Provar TearDown callable, or place create/delete inside the same UiWithScreen ' +
+        'clause so both run as a unit regardless of failure.'
+    );
+  }
+
   return warnings;
 }
 
@@ -89,10 +100,19 @@ const StepSchema = z.object({
     .record(z.string())
     .default({})
     .describe(
-      'Step argument values as key/value pairs. Written as <arguments><argument id="key"><value .../></argument></arguments> ' +
-        'inside the <apiCall> element — the format Provar runtime requires. ' +
+      'Step argument values as key/value pairs. Written as <arguments><argument id="key"><value .../></argument></arguments>. ' +
         'Do NOT rely on XML attributes on <apiCall>; the runtime silently ignores them. ' +
-        'Example: { "connectionName": "MyOrg", "objectApiName": "Opportunity" }'
+        'Special value conventions (applied automatically by the generator): ' +
+        '(1) Variable references: wrap the name in braces, e.g. "{MyVar}" → emitted as class="variable" <path element="MyVar"/>. ' +
+        '    Dotted paths are also supported: "{Obj.Field}" → two <path> elements. ' +
+        '(2) SetValues: pass each variable name and its value as a flat key/value pair; ' +
+        '    the generator wraps them in <value class="valueList"><namedValues>...</namedValues></value> automatically. ' +
+        '    Example: { "testCaseName": "TC_New", "testType": "Acceptance testing" } ' +
+        '(3) AssertValues: pass assertion arguments as flat key/value pairs; emitted as flat <argument> elements, NOT wrapped in valueList/namedValues. ' +
+        '(4) target argument (UiWithScreen / UiWithRow): pass the sf:ui:target or ui:pageobject:target URI; ' +
+        '    emitted as class="uiTarget" uri="...". ' +
+        '(5) locator argument (UiDoAction / UiAssert): pass the locator URI; emitted as class="uiLocator" uri="...". ' +
+        'All other string values use class="value" valueClass="string".'
     ),
 });
 
@@ -111,6 +131,14 @@ const TOOL_DESCRIPTION = [
   'ApexReadObject requires field names in attributes; omitting them produces MALFORMED_QUERY. Prefer ApexSoqlQuery.',
   'AssertValues on SOQL results: index paths like "ResultList[0].Field" are not supported.',
   'Use ForEach to iterate the result list, or SetValues to extract a field into a variable first.',
+  'SetValues: pass named variable values as flat key/value pairs in attributes; ' +
+    'the generator wraps them in <value class="valueList"><namedValues>...</namedValues></value> automatically.',
+  'AssertValues: pass assertion values as flat key/value argument pairs; emitted as flat arguments, NOT wrapped in namedValues. ' +
+    'If AssertValues uses namedValues-shaped content, validation reports warning ASSERT-001.',
+  'Variable references: pass values as "{VarName}" (braces); emitted as class="variable" <path element="VarName"/>.',
+  'target argument (UiWithScreen/UiWithRow): pass the URI value; emitted as class="uiTarget" uri="...".',
+  'locator argument (UiDoAction/UiAssert): pass the URI value; emitted as class="uiLocator" uri="...".',
+  'Cleanup warning: ApexDeleteObject steps near end of test will be skipped if an earlier step fails (stopOnError=false). Use a TearDown callable.',
   'Validation: when validate_after_edit=true (default) the response includes a validation field and returns TESTCASE_INVALID if the generated XML fails structural checks.',
   'Grounding: call provar.qualityhub.examples.retrieve before generating to get corpus examples for the scenario — correct XML structure for the step types you need.',
 ].join(' ');
@@ -239,28 +267,85 @@ export function registerTestCaseGenerate(server: McpServer, config: ServerConfig
 
 // ── XML builder ───────────────────────────────────────────────────────────────
 
-function buildArgumentsXml(attributes: Record<string, string>, baseIndent = '      '): string {
+// Build the <value> element for a single argument (D2/D4 aware).
+// inNamedValues: when true (inside SetValues namedValues), skip uiTarget/uiLocator dispatch.
+// apiId: resolved API ID used to restrict key-name dispatch to the correct UI APIs.
+function buildArgumentValue(key: string, val: string, indent: string, inNamedValues = false, apiId = ''): string {
+  // D4: {VarName} or {Obj.Field} → class="variable" with <path> elements.
+  const varMatch = /^\{([\w.]+)\}$/.exec(val);
+  if (varMatch) {
+    const pathElements = varMatch[1]
+      .split('.')
+      .map((p) => `${indent}  <path element="${escapeXmlAttr(p)}"/>`)
+      .join('\n');
+    return `${indent}<value class="variable">\n${pathElements}\n${indent}</value>`;
+  }
+  if (!inNamedValues) {
+    // D2: 'target' argument → class="uiTarget" (only for UiWithScreen / UiWithRow).
+    if (key === 'target' && (apiId.includes('UiWithScreen') || apiId.includes('UiWithRow'))) {
+      return `${indent}<value class="uiTarget" uri="${escapeXmlAttr(val)}"/>`;
+    }
+    // D2: 'locator' argument → class="uiLocator" (only for UiDoAction / UiAssert).
+    if (key === 'locator' && (apiId.includes('UiDoAction') || apiId.includes('UiAssert'))) {
+      return `${indent}<value class="uiLocator" uri="${escapeXmlAttr(val)}"/>`;
+    }
+  }
+  return `${indent}<value class="value" valueClass="string">${escapeXmlContent(val)}</value>`;
+}
+
+function buildArgumentsXml(attributes: Record<string, string>, baseIndent = '      ', apiId = ''): string {
   const entries = Object.entries(attributes);
   if (entries.length === 0) return '';
   const argLines = entries
-    .map(
-      ([k, v]) =>
+    .map(([k, v]) => {
+      const valueXml = buildArgumentValue(k, v, `${baseIndent}  `, false, apiId);
+      return (
         `${baseIndent}<argument id="${escapeXmlAttr(k)}">\n` +
-        `${baseIndent}  <value class="value" valueClass="string">${escapeXmlContent(v)}</value>\n` +
+        valueXml + '\n' +
         `${baseIndent}</argument>`
-    )
+      );
+    })
     .join('\n');
   return `\n${baseIndent}<arguments>\n${argLines}\n${baseIndent}</arguments>\n${baseIndent.slice(0, -2)}`;
 }
 
+// D3: SetValues — all attributes become <namedValues> under a single 'values' argument.
+function buildSetValuesXml(attributes: Record<string, string>, baseIndent: string): string {
+  const entries = Object.entries(attributes);
+  if (entries.length === 0) return '';
+  const i = (n: number): string => baseIndent + '  '.repeat(n);
+  const namedValueLines = entries
+    .map(([name, val]) => {
+      const valueXml = buildArgumentValue(name, val, `${i(3)}  `, true);
+      return `${i(3)}<namedValue name="${escapeXmlAttr(name)}">\n${valueXml}\n${i(3)}</namedValue>`;
+    })
+    .join('\n');
+  return (
+    `\n${i(0)}<arguments>\n` +
+    `${i(0)}<argument id="values">\n` +
+    `${i(1)}<value class="valueList" mutable="Mutable">\n` +
+    `${i(2)}<namedValues>\n` +
+    namedValueLines + '\n' +
+    `${i(2)}</namedValues>\n` +
+    `${i(1)}</value>\n` +
+    `${i(0)}</argument>\n` +
+    `${i(0)}</arguments>\n` +
+    `${baseIndent.slice(0, -2)}`
+  );
+}
+
 function buildFlatStepXml(
   step: { api_id: string; name: string; attributes: Record<string, string> },
   testItemId: number,
   indent: string
 ): string {
   const guid = randomUUID();
   const resolvedApiId = resolveApiId(step.api_id);
-  const argumentsXml = buildArgumentsXml(step.attributes, indent + '  ');
+  const baseIndent = indent + '  ';
+  // Use SetValues structure for any SetValues API (string-match mirrors the validator).
+  const argumentsXml = resolvedApiId.includes('SetValues')
+    ? buildSetValuesXml(step.attributes, baseIndent)
+    : buildArgumentsXml(step.attributes, baseIndent, resolvedApiId);
   if (argumentsXml) {
     return (
       `${indent}<apiCall guid="${guid}" apiId="${escapeXmlAttr(resolvedApiId)}"` +
@@ -321,7 +406,7 @@ function buildUiWithScreenXml(
     '      </clauses>\n    ';
   return (
     `    <apiCall guid="${wrapperGuid}" apiId="${wrapperApiId}"` +
-    ` name="With page" testItemId="1">${buildArgumentsXml({ target: targetUri }).trimEnd()}${clausesXml}</apiCall>`
+    ` name="With page" testItemId="1">${buildArgumentsXml({ target: targetUri }, '      ', wrapperApiId).trimEnd()}${clausesXml}</apiCall>`
   );
 }