diff --git a/docs/mcp-pilot-guide.md b/docs/mcp-pilot-guide.md index bf7e0073..27381cfd 100644 --- a/docs/mcp-pilot-guide.md +++ b/docs/mcp-pilot-guide.md @@ -483,6 +483,37 @@ If any FAIL indicator appears, report it to the Provar team with the prompt and --- +### Scenario 13: Org-Aware Test Case Generation + +The `provar_org_describe` tool surfaces cached Salesforce describe data from the Provar IDE workspace so the agent knows which fields on each object are required and what their types are — without making a live Salesforce API call. Use it as a hint source before generating data-heavy steps. + +**Prerequisite:** the project must have been opened in Provar IDE at least once with the named connection loaded, so the `.metadata//` directory is populated. + +**Try this prompt:** + +> "Before generating a test case that creates an Account, call `provar_org_describe` against my project at `/path/to/MyProject` for connection `MyOrg` and the `Account` object only. Use the required-field list to populate the create form." + +The tool returns the discovered workspace path, a cache age, and per-object required-field metadata. Example call: + +```jsonc +{ + "project_path": "/Users/you/git/MyProject", + "connection_name": "MyOrg", + "objects": ["Account"], + "field_filter": "required" +} +``` + +**What to look for (PASS):** + +- Response includes `workspace_path` resolved to a real `workspace-*` directory. +- `objects[0].required_fields` contains at least one field with `nillable: false`. +- The follow-up `provar_testcase_generate` call uses field names from the response. + +**Cache-miss behaviour (also PASS):** if the cache directory does not exist the tool returns `details.suggestion` telling the agent how to recover — either open the project in Provar IDE to populate the cache, or pass field-type hints inline. + +--- + ## Security Model ### What the server does diff --git a/docs/mcp.md b/docs/mcp.md index 33d23421..2c262a8c 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -50,6 +50,9 @@ 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) + - [Org metadata access](#org-metadata-access) + - [provar_org_describe](#provar_org_describe) + - [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) @@ -78,6 +81,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 +535,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` | `` 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 | + +Warning-code messages emitted via `formatWarning()` follow the shape `WARNING []: ` (optionally suffixed with ` Did you mean ''?` when a typo is detected). Other free-form warnings without a structured code — such as the placeholder warnings emitted by `provar_properties_validate` — remain plain strings. See `src/mcp/utils/warningCodes.ts` for the canonical enum. + +--- + ## Available tools ### `provardx_ping` @@ -729,13 +750,19 @@ The tool's chip-level `title` — `Generate Test Case (full steps in one call)` **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"` + `` | Any step | -| SetValues attributes | `class="valueList"/` | SetValues only | -| All other values | `class="value" valueClass="string"` | Any step | +| 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"` + `` | Any step | +| SetValues attributes | `class="valueList"/` | SetValues only | +| Value `YYYY-MM-DDTHH:MM:SS` + optional `.fff` + optional `Z`/`±HH:MM` (ISO-8601, end-anchored) | `class="value" valueClass="datetime"` | Any step | +| Value `YYYY-MM-DD` (ISO-8601) | `class="value" valueClass="date"` | Any step | +| Value `true` / `false` | `class="value" valueClass="boolean"` | Any step | +| Numeric value `^-?\d+(\.\d+)?$` (`42`, `-5`, `3.14`) | `class="value" valueClass="decimal"` | Any step | +| All other values | `class="value" valueClass="string"` | Any step | + +`valueClass` is inferred automatically by `inferSalesforceValueClass(key, val, fieldTypeHint?)`. Detection order: explicit `fieldTypeHint` (wired in a follow-up tool surface — `field_type_hints` param) → ISO-8601 datetime (with optional fractional seconds and timezone, end-anchored) → ISO-8601 date → boolean → decimal → string. Per the canonical Provar reference numbers always emit as `valueClass="decimal"` (there is no separate `integer` valueClass). Provar runtime silently discards date fields emitted as `valueClass="string"`, so always pass date / datetime values in ISO-8601 form. AssertValues uses **flat** argument structure (`expectedValue`, `actualValue`, `comparisonType`) — not the `valueList`/namedValues format. @@ -825,7 +852,7 @@ Validates an XML test case for schema correctness (validity score) and best prac **Warning rules:** -- **DATA-001** — `testCase` declares a `` 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 `` 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. @@ -1086,7 +1113,7 @@ Updates one or more fields in a `provardx-properties.json` file. Only the suppli ### `provar_properties_validate` -Validates a `provardx-properties.json` file against the ProvarDX schema. Checks required fields, valid enum values, and warns about unfilled `${PLACEHOLDER}` values. Accepts either a file path or inline JSON content. +Validates a `provardx-properties.json` file against the ProvarDX schema. Checks required fields, valid enum values, and warns about unfilled `${PLACEHOLDER}` values. Also surfaces a `SCHEMA-001` warning for any unknown top-level, `metadata.*`, or `environment.*` key, with a "Did you mean ..." suggestion when a canonical key is within Levenshtein distance 2. Accepts either a file path or inline JSON content. **Input** @@ -1097,12 +1124,17 @@ Validates a `provardx-properties.json` file against the ProvarDX schema. Checks **Output** -| Field | Description | -| --------------- | ----------------------------------------------- | -| `is_valid` | `true` if no errors | -| `error_count` | Number of validation errors | -| `warning_count` | Number of warnings (e.g. unfilled placeholders) | -| `issues` | Array of `{ field, severity, message }` | +| Field | Description | +| --------------- | ----------------------------------------------------------- | +| `is_valid` | `true` if no errors (warnings alone do not flip `is_valid`) | +| `error_count` | Number of validation errors | +| `warning_count` | Number of warnings (placeholders, unknown keys, etc.) | +| `errors` | Array of `{ field, severity: 'error', message }` | +| `warnings` | Array of `{ field, severity: 'warning', message }` | + +**Warning codes (`warnings` array):** + +- `SCHEMA-001` — unknown key at top-level / `metadata.*` / `environment.*`. Example: `WARNING [SCHEMA-001]: Unknown field 'testCases' at top-level. Did you mean 'testCase'?` Unknown keys are **warnings, not errors**, so additive Provar versions do not break older MCP clients. The classic instance is the `testCases` (plural) typo for the canonical `testCase` (singular) — if you see SCHEMA-001 on `testCases`, fix the spelling before running any tests. **Error codes:** `MISSING_INPUT`, `PROPERTIES_FILE_NOT_FOUND`, `MALFORMED_JSON`, `PATH_NOT_ALLOWED` @@ -1377,7 +1409,7 @@ Triggers a Provar Automation test run using the currently loaded properties file | --------- | -------- | -------- | ------------------------------------------------------------------------ | | `flags` | string[] | no | Raw CLI flags to forward (e.g. `["--project-path", "/path/to/project"]`) | -**Output** — `{ requestId, exitCode, stdout, stderr[, output_lines_suppressed][, steps][, details.warning] }` +**Output** — `{ requestId, exitCode, stdout, stderr[, output_lines_suppressed][, steps][, details.warning][, warnings] }` The `stdout` field is filtered before returning: Java schema-validator lines (`com.networknt.schema.*`) and stale logger-lock `SEVERE` warnings are stripped. If any lines were suppressed, `output_lines_suppressed` contains the count. @@ -1386,15 +1418,32 @@ After each run, the tool scans the results directory for JUnit XML files and add ```json "steps": [ { "testItemId": "1", "title": "TC-Login-001-LoginAndVerify.testcase", "status": "pass" }, - { "testItemId": "2", "title": "TC-Login-002-ForgotPassword.testcase", "status": "fail", "errorMessage": "Execution failed: Element not found" } + { "testItemId": "2", "title": "TC-Login-002-ForgotPassword.testcase", "status": "fail", "errorMessage": "TimeoutException: page did not load", + "error_category": "TIMEOUT", "retryable": true } ] -``` -Each entry represents one test case. `status` is `"pass"`, `"fail"`, or `"skip"`. If the results directory cannot be located or contains no JUnit XML, `details.warning` explains why and `steps` is absent. +Each entry represents one test case. status is "pass", "fail", or "skip". If the results directory cannot be located or contains no JUnit XML, +details.warning explains why and steps is absent. -**Error codes:** `AUTOMATION_TESTRUN_FAILED`, `SF_NOT_FOUND` +Failed steps may include two optional classification fields: ---- +- error_category — one of INFRASTRUCTURE, ASSERTION, LOCATOR, TIMEOUT, OTHER, set when the failure text matches a known pattern. +- retryable — true when error_category is INFRASTRUCTURE or TIMEOUT (transient causes), false for ASSERTION/LOCATOR/OTHER. Absent when no +pattern matched. + +Zero-tests guard (RUN-001): when the sf command exits 0, the results directory was located, and at least one JUnit XML file parsed successfully +but contains zero executed test cases, the response includes a warnings[] array containing a RUN-001 (#warning-codes) message. This is almost +always a typo such as testCase vs testCases (or some other unknown key) in provardx-properties.json — the run silently selected nothing. The +warning is additive and never flips exitCode or sets isError; the failure surface remains driven by the underlying sf exit code. + +▎ Why RUN-001 stays silent when no JUnit data is available: if the results directory cannot be located, contains no XML files, or every XML file +▎ fails to parse, the tool genuinely has no data on which to assert "zero tests ran" — the absence of parsed results is just "we don't know +▎ what ran". In those cases the response carries details.warning (explaining why structured step data is missing) and RUN-001 is suppressed to +▎ avoid misdirecting the agent toward a typo when the real issue is a missing/unreadable results dir. + +Error codes: AUTOMATION_TESTRUN_FAILED, SF_NOT_FOUND +Warning codes: RUN-001 (zero tests executed despite success) +``` ### `provar_automation_compile` @@ -1535,22 +1584,26 @@ Use `mode="failures"` when you only need the list of failing test case names wit **`FailureReport` fields (mode=rca only):** -| Field | Description | -| --------------------- | -------------------------------------------------------- | -| `test_case` | Test case filename from JUnit `` | -| `error_class` | Extracted exception class name | -| `error_message` | First 500 chars of failure/error text | -| `root_cause_category` | One of 12 categories (see table below) | -| `root_cause_summary` | Human-readable cause description | -| `recommendation` | Suggested fix action | -| `page_object` | Extracted from `Page Object: ...` pattern, or `null` | -| `operation` | Extracted from `operation: ...` pattern, or `null` | -| `report_html` | Path to per-test HTML report if found, else `null` | -| `screenshot_dir` | Path to `Artifacts/` directory if it exists, else `null` | -| `pre_existing` | `true` if the same test failed in a prior Increment run | +| Field | Description | +| --------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------ | +| `test_case` | Test case filename from JUnit `` | +| `error_class` | Extracted exception class name | +| `error_message` | First 500 chars of failure/error text | +| `root_cause_category` | One of 17 categories (see list below) | +| `root_cause_summary` | Human-readable cause description | +| `recommendation` | Suggested fix action | +| `page_object` | Extracted from `Page Object: ...` pattern, or `null` | +| `operation` | Extracted from `operation: ...` pattern, or `null` | +| `report_html` | Path to per-test HTML report if found, else `null` | +| `screenshot_dir` | Path to `Artifacts/` directory if it exists, else `null` | +| `pre_existing` | `true` if the same test failed in a prior Increment run | +| `error_category` | Optional. One of `INFRASTRUCTURE` \| `ASSERTION` \| `LOCATOR` \| `TIMEOUT` \| `OTHER`. Absent when no known pattern matched. | +| `retryable` | Optional. `true` when `error_category` is `INFRASTRUCTURE` or `TIMEOUT` (transient causes); `false` otherwise. Absent when `error_category` is absent. | **Root cause categories:** `DRIVER_VERSION_MISMATCH`, `LOCATOR_STALE`, `TIMEOUT`, `ASSERTION_FAILED`, `CREDENTIAL_FAILURE`, `MISSING_CALLABLE`, `METADATA_CACHE`, `PAGE_OBJECT_COMPILE`, `CONNECTION_REFUSED`, `DATA_SETUP`, `LICENSE_INVALID`, `SALESFORCE_VALIDATION`, `SALESFORCE_PICKLIST`, `SALESFORCE_REFERENCE`, `SALESFORCE_ACCESS`, `SALESFORCE_TRIGGER`, `UNKNOWN` +**Error category vs. root cause category:** `root_cause_category` is fine-grained (17 buckets) and drives the human-readable `recommendation`. `error_category` is coarse-grained (5 buckets) and drives automated retry policy via `retryable`. The two are independent classifiers over the same failure text — both may be set on the same failure. + Salesforce DML error categories (`SALESFORCE_*`) represent test-data failures — they appear in `failures[].root_cause_category` but are **not** included in `infrastructure_issues`. **Error codes:** `RESULTS_NOT_CONFIGURED`, `PATH_NOT_ALLOWED`, `PATH_TRAVERSAL` @@ -1684,6 +1737,168 @@ 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 `` 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 `` 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 | +| -------------------------------------------------------------------------------------------- | ----------------- | -------- | +| `` present **and** referenced from a `.testinstance` inside `plans/` | No | — | +| `` present **and** referenced via top-level `testCase` / `testCases` array | Yes | WARNING | +| `` present **and** project context cannot be resolved (no active properties file) | Yes (structural) | WARNING | +| No `` 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 `/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. + +--- + +## Org metadata access + +Tools that surface Salesforce org metadata to authoring tools without making a live API call. These read from data that has already been written to disk by the Provar IDE — they do **not** trigger metadata downloads themselves and they do **not** require an authenticated session. + +> **Distinct from `.provarCaches`:** the runtime cache used by `provar_automation_testrun` lives at `/.provarCaches/` and is regenerated per run. The cache read by `provar_org_describe` lives in the Provar IDE **workspace** (`/.metadata//`) and is updated when a user opens the project and loads a connection in the IDE. + +### `provar_org_describe` + +Read cached Salesforce describe data for one connection from the Provar workspace `.metadata` cache. Returns the object list, required-field schema, and a cache age. Use this before calling `provar_testcase_generate` so the generator can produce steps with correctly-typed field values. + +**Prerequisite:** the project must have been opened in Provar IDE at least once with the named connection loaded. If the cache is missing, the tool returns a structured response with `details.suggestion` rather than an error. + +**Workspace discovery heuristic** — the tool walks candidate directories in this order and uses the first one that exists: + +1. `/workspace-/` — sibling workspace pattern (default for Provar IDE in this workspace layout). +2. `/Provar_Workspaces/workspace-/` — shared `Provar_Workspaces` directory. +3. `~/Provar/workspace-/` — user-home fallback. + +`` is the project's basename with whitespace collapsed to single dashes and lowercased: `"My Project"` → `"my-project"`. + +| Input | Type | Required | Default | Description | +| ----------------- | ----------------------- | -------- | ------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| `project_path` | string | yes | — | Absolute path to the Provar test project root (the directory containing `.testproject`). Must be within `--allowed-paths`. | +| `connection_name` | string | yes | — | Connection name as defined in `.testproject` (e.g. `"MyOrg"`). Must match the `.metadata` subdirectory exactly. Path separators in this value are rejected (`PATH_TRAVERSAL`). | +| `objects` | string[] | no | all | Filter — only return data for these object API names. When omitted, lists every object cached under the connection directory. | +| `field_filter` | `'required'` \| `'all'` | no | `'required'` | Which fields to return. `'required'` includes only fields with `nillable=false`; `'all'` returns every cached field. | + +| Output field | Description | +| ------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `requestId` | UUID for this invocation. Echoed in MCP server logs for cross-correlation. Consistent with the rest of the MCP tool surface. | +| `workspace_path` | Absolute resolved path to the discovered workspace, or `null` when none of the three candidate directories exists (or all candidates were outside `--allowed-paths`). | +| `cache_age_ms` | `mtime` delta in milliseconds of the connection cache directory, or `null` when the cache is missing. | +| `objects[]` | Array of `{ name, exists, required_fields, field_count, error_message? }`. `exists` is `true` (cache file present), `false` (requested but not cached), or `null` (cache miss — the whole `.metadata/` directory is absent). | +| `objects[].error_message` | Present **only** when a cache file existed but failed to parse (`exists: true, field_count: 0`). Lets the agent distinguish a corrupt / unsupported cache file from a missing one. | +| `details.suggestion` | Present **only** on cache miss. Tells the agent how to populate the cache (open Provar IDE) or how to proceed without it (inline hints). | + +**Example — happy path:** + +```jsonc +// Request +{ + "project_path": "/Users/me/git/MyProject", + "connection_name": "MyOrg", + "objects": ["Account", "Contact"], + "field_filter": "required" +} + +// Response +{ + "requestId": "01HEXX...K7P", + "workspace_path": "/Users/me/git/workspace-MyProject", + "cache_age_ms": 1839200, + "objects": [ + { + "name": "Account", + "exists": true, + "required_fields": [ + { "name": "Name", "type": "string", "default_value": null, "nillable": false } + ], + "field_count": 24 + }, + { + "name": "Contact", + "exists": true, + "required_fields": [ + { "name": "LastName", "type": "string", "default_value": null, "nillable": false } + ], + "field_count": 31 + } + ] +} +``` + +**Example — cache miss:** + +```jsonc +// Response when the .metadata/ directory does not exist +{ + "requestId": "01HEXX...K7P", + "workspace_path": "/Users/me/git/workspace-MyProject", + "cache_age_ms": null, + "objects": [{ "name": "Account", "exists": null, "required_fields": [], "field_count": 0 }], + "details": { + "suggestion": "Open this project in Provar IDE and load the 'MyOrg' connection, or pass field-type hints inline to provar_testcase_generate." + } +} +``` + +**Example — parse error on a cached file:** + +```jsonc +// Response when Account.json exists but is corrupt / unparseable +{ + "requestId": "01HEXX...K7P", + "workspace_path": "/Users/me/git/workspace-MyProject", + "cache_age_ms": 1839200, + "objects": [ + { + "name": "Account", + "exists": true, + "required_fields": [], + "field_count": 0, + "error_message": "Failed to parse cache file (Account.json): Unexpected token } in JSON at position 42" + } + ] +} +``` + +**On-disk cache schema (one file per object).** The tool reads `.json` first, then `.xml`, then `.object` as a fallback: + +```jsonc +// /.metadata//Account.json +{ + "name": "Account", + "fields": [ + { "name": "Name", "type": "string", "defaultValue": null, "nillable": false }, + { "name": "Phone", "type": "phone", "defaultValue": null, "nillable": true } + ] +} +``` + +**Error codes:** + +| Code | Meaning | +| ------------------ | ---------------------------------------------------------------------------------------------- | +| `PATH_NOT_ALLOWED` | `project_path` or the resolved workspace path is outside `--allowed-paths`. | +| `PATH_TRAVERSAL` | `project_path` contains `..` segments, or `connection_name` contains a path separator or `..`. | + --- ## NitroX — Hybrid Model page objects diff --git a/package.json b/package.json index 2a0d1730..d7040ea1 100644 --- a/package.json +++ b/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.1", + "version": "1.5.2", "mcpName": "io.github.ProvarTesting/provar", "license": "BSD-3-Clause", "plugins": [ diff --git a/scripts/mcp-smoke.cjs b/scripts/mcp-smoke.cjs index 6cea444b..ae54a7f4 100644 --- a/scripts/mcp-smoke.cjs +++ b/scripts/mcp-smoke.cjs @@ -461,6 +461,36 @@ async function runTests() { test_item_id: '1', }); + // ── 54. provar_org_describe — cache miss ───────────────────────────────── + // TMP has no workspace at all → cache-miss response with details.suggestion + if (inGroup('inspect')) + await callTool('provar_org_describe', { + project_path: TMP, + connection_name: 'SmokeOrg', + objects: ['Account'], + }); + + // ── 55. provar_org_describe — happy path ───────────────────────────────── + // Set up a sibling workspace + .metadata/ with one fake object. + if (inGroup('inspect')) { + const fs = require('fs'); + const orgProject = path.join(TMP, 'org-describe-smoke-project'); + fs.mkdirSync(orgProject, { recursive: true }); + const cxnDir = path.join(TMP, 'workspace-org-describe-smoke-project', '.metadata', 'SmokeOrg'); + fs.mkdirSync(cxnDir, { recursive: true }); + fs.writeFileSync( + path.join(cxnDir, 'Account.json'), + JSON.stringify({ + name: 'Account', + fields: [{ name: 'Name', type: 'string', defaultValue: null, nillable: false }], + }) + ); + await callTool('provar_org_describe', { + project_path: orgProject, + connection_name: 'SmokeOrg', + }); + } + server.stdin.end(); } diff --git a/server.json b/server.json index 01e3d72e..6d392a13 100644 --- a/server.json +++ b/server.json @@ -14,12 +14,12 @@ "url": "https://github.com/ProvarTesting/provardx-cli", "source": "github" }, - "version": "1.5.1", + "version": "1.5.2", "packages": [ { "registryType": "npm", "identifier": "@provartesting/provardx-cli", - "version": "1.5.1", + "version": "1.5.2", "transport": { "type": "stdio" }, diff --git a/src/mcp/server.ts b/src/mcp/server.ts index 20429dba..1f9de722 100644 --- a/src/mcp/server.ts +++ b/src/mcp/server.ts @@ -34,6 +34,7 @@ import { registerAllTestPlanTools } from './tools/testPlanTools.js'; import { registerAllNitroXTools } from './tools/nitroXTools.js'; import { registerAllTestCaseStepTools } from './tools/testCaseStepTools.js'; import { registerAllConnectionTools } from './tools/connectionTools.js'; +import { registerAllOrgDescribeTools } from './tools/orgDescribeTools.js'; import { registerAllPrompts } from './prompts/index.js'; import { createDepthGuardState, @@ -64,7 +65,7 @@ const TOOL_GROUPS: Record): JUnitStepResult const errorMessage = extractFailureText(tc['failure'] ?? tc['error']); const step: JUnitStepResult = { testItemId: String(idx), title, status }; - if (errorMessage) step.errorMessage = errorMessage; + if (errorMessage) { + step.errorMessage = errorMessage; + const error_category = classifyStepErrorCategory(errorMessage); + const retryable = isStepRetryable(error_category); + if (error_category !== undefined) step.error_category = error_category; + if (retryable !== undefined) step.retryable = retryable; + } steps.push(step); } } @@ -1071,7 +1118,7 @@ function findXmlFiles(dir: string): string[] { */ export function parseJUnitResults(resultsDir: string): JUnitParseResult { if (!fs.existsSync(resultsDir)) { - return { steps: [], warning: `Results directory not found: ${resultsDir}` }; + return { steps: [], warning: `Results directory not found: ${resultsDir}`, parsedAny: false }; } const xmlFiles = findXmlFiles(resultsDir); @@ -1079,6 +1126,7 @@ export function parseJUnitResults(resultsDir: string): JUnitParseResult { return { steps: [], warning: 'No JUnit XML files found in results directory — structured step output unavailable.', + parsedAny: false, }; } @@ -1111,12 +1159,17 @@ export function parseJUnitResults(resultsDir: string): JUnitParseResult { return { steps: [], warning: 'JUnit XML files found but could not be parsed — structured step output unavailable.', + parsedAny: false, }; } if (allSteps.length === 0) { + // We did parse at least one file; the file just had zero entries (or none we could + // recognise as steps). This is the legitimate "selector matched nothing" signal that RUN-001 + // is built to catch. return { steps: [], warning: 'JUnit XML found but no test steps could be extracted — files may not be standard JUnit format.', + parsedAny: true, }; } @@ -1124,7 +1177,7 @@ export function parseJUnitResults(resultsDir: string): JUnitParseResult { parseFailures > 0 ? `${parseFailures} JUnit XML file(s) could not be parsed — step data may be incomplete.` : undefined; - return { steps: allSteps, warning }; + return { steps: allSteps, warning, parsedAny: true }; } // ── Registration ────────────────────────────────────────────────────────────── diff --git a/src/mcp/tools/automationTools.ts b/src/mcp/tools/automationTools.ts index ccc712df..ed15db95 100644 --- a/src/mcp/tools/automationTools.ts +++ b/src/mcp/tools/automationTools.ts @@ -15,6 +15,7 @@ import { makeError, makeRequestId } from '../schemas/common.js'; import { log } from '../logging/logger.js'; import type { ServerConfig } from '../server.js'; import { assertPathAllowed, PathPolicyError } from '../security/pathPolicy.js'; +import { WARNING_CODES, formatWarning } from '../utils/warningCodes.js'; import { parseJUnitResults } from './antTools.js'; import { runSfCommand } from './sfSpawn.js'; import { desc } from './descHelper.js'; @@ -226,6 +227,38 @@ function readResultsPathFromSfConfig(config: ServerConfig): string | null { // ── Tool: provar_automation_testrun ─────────────────────────────────────────── +/** + * JUnit introspection for the testrun response. Returns enough structure that + * downstream warning emitters (RUN-001 zero-tests, future JUNIT-001 expected-vs- + * actual mismatch) can read a single object instead of re-parsing. + */ +type JUnitIntrospection = { + steps: ReturnType['steps']; + stepCount: number; + parseWarning: string | undefined; + resultsPathResolved: boolean; + /** + * True iff at least one JUnit XML file was located AND parsed without throwing. + * Gates RUN-001: a `stepCount === 0` only means "zero tests executed" when we know we + * actually have parseable data. With `parsedAny === false` the count is "we don't know", + * which must stay silent (details.warning already covers it). + */ + parsedAny: boolean; +}; + +function introspectJUnit(config: ServerConfig): JUnitIntrospection { + const resultsPath = readResultsPathFromSfConfig(config); + if (!resultsPath) { + return { steps: [], stepCount: 0, parseWarning: undefined, resultsPathResolved: false, parsedAny: false }; + } + const { steps, warning, parsedAny } = parseJUnitResults(resultsPath); + return { steps, stepCount: steps.length, parseWarning: warning, resultsPathResolved: true, parsedAny }; +} + +const ZERO_TESTS_MESSAGE = + 'Test run exited successfully but zero tests were executed. ' + + 'Check the testCase / testCases (note spelling) field in provardx-properties.json.'; + export function registerAutomationTestRun(server: McpServer, config: ServerConfig): void { server.registerTool( 'provar_automation_testrun', @@ -240,9 +273,12 @@ export function registerAutomationTestRun(server: McpServer, config: ServerConfi 'For grid/CI execution via Provar Quality Hub instead of running locally, use provar_qualityhub_testrun.', 'Output buffer: a 50 MB maxBuffer is set so ENOBUFS on verbose Provar runs is now rare.', 'If ENOBUFS still occurs (extremely verbose logging), run `sf provar automation test run --json` directly in the terminal and pipe or tail the output instead of retrying this tool.', + 'Zero-tests guard: if the sf exit code is 0, the results directory was located, and at least one JUnit XML file parsed successfully but contains zero executed tests, a RUN-001 warning is added to `warnings[]` — usually a typo such as `testCase` vs `testCases` in provardx-properties.json. When no JUnit data is available (dir missing or all XML unparseable), `details.warning` is set instead and RUN-001 stays silent.', 'Typical local AI loop: config.load → compile → testrun → inspect results.', + 'Each failed step in `steps[]` may include optional error_category (INFRASTRUCTURE|ASSERTION|LOCATOR|TIMEOUT|OTHER)', + 'and retryable (boolean) fields when the failure text matches a known pattern — use these to drive automated retry policy.', ].join(' '), - 'Run local Provar tests via sf CLI; requires config_load first.' + 'Run local Provar tests via sf CLI; requires config_load first. Surfaces RUN-001 on zero-tests-executed.' ), inputSchema: { flags: z @@ -274,11 +310,10 @@ export function registerAutomationTestRun(server: McpServer, config: ServerConfi const result = runSfCommand(['provar', 'automation', 'test', 'run', ...flags], sf_path); const { filtered, suppressed } = filterTestRunOutput(result.stdout); - // Attempt to enrich the response with structured step data from JUnit XML - const resultsPath = readResultsPathFromSfConfig(config); - const { steps, warning: junitWarning } = resultsPath - ? parseJUnitResults(resultsPath) - : { steps: [], warning: undefined }; + // Enrich the response with structured step data + warning hooks from JUnit XML. + // Single introspection call keeps the wiring extensible (e.g. future JUNIT-001 + // expected-vs-actual mismatch can read stepCount from the same struct). + const junit = introspectJUnit(config); if (result.exitCode !== 0) { const { filtered: filteredErr, suppressed: suppressedErr } = filterTestRunOutput( @@ -288,11 +323,11 @@ export function registerAutomationTestRun(server: McpServer, config: ServerConfi ...makeError('AUTOMATION_TESTRUN_FAILED', filteredErr, requestId), ...(suppressedErr > 0 ? { output_lines_suppressed: suppressedErr } : {}), }; - if (steps.length > 0) errBody['steps'] = steps; - if (!resultsPath || junitWarning) { + if (junit.steps.length > 0) errBody['steps'] = junit.steps; + if (!junit.resultsPathResolved || junit.parseWarning) { errBody['details'] = { warning: - junitWarning ?? + junit.parseWarning ?? 'Could not locate results directory — step-level output unavailable. Run provar_automation_config_load first.', }; } @@ -306,8 +341,27 @@ export function registerAutomationTestRun(server: McpServer, config: ServerConfi stderr: result.stderr, }; if (suppressed > 0) response['output_lines_suppressed'] = suppressed; - if (steps.length > 0) response['steps'] = steps; - if (junitWarning) response['details'] = { warning: junitWarning }; + if (junit.steps.length > 0) response['steps'] = junit.steps; + if (junit.parseWarning) response['details'] = { warning: junit.parseWarning }; + + // RUN-001: sf reported success but zero tests actually executed. + // Almost always a typo in the testCase / testCases field of provardx-properties.json. + // Only fires when: + // 1. The results dir was located (resultsPathResolved), AND + // 2. At least one JUnit XML file was successfully parsed (parsedAny). + // Without (2) `stepCount === 0` just means "we don't have parseable data" — not + // "zero tests ran" — and the agent would be misdirected toward a typo when the + // real issue is a missing/unparseable results dir. That case is already surfaced + // via `details.warning` from the parse layer. With parsedAny === true and zero + // extracted steps, we know the selector genuinely matched nothing. + if (junit.resultsPathResolved && junit.parsedAny && junit.stepCount === 0) { + const warningStr = formatWarning(WARNING_CODES.RUN_001, ZERO_TESTS_MESSAGE); + // Append rather than overwrite so future warning emitters (e.g. JUNIT-001 mismatch + // in PDX-491) can coexist on the same response without stepping on each other. + const existing = response['warnings'] as string[] | undefined; + response['warnings'] = existing ? existing.concat(warningStr) : [warningStr]; + } + return { content: [{ type: 'text' as const, text: JSON.stringify(response) }], structuredContent: response }; } catch (err) { return handleSpawnError(err, requestId, 'provar_automation_testrun'); diff --git a/src/mcp/tools/orgDescribeTools.ts b/src/mcp/tools/orgDescribeTools.ts new file mode 100644 index 00000000..26d52649 --- /dev/null +++ b/src/mcp/tools/orgDescribeTools.ts @@ -0,0 +1,467 @@ +/* + * Copyright (c) 2024 Provar Limited. + * All rights reserved. + * Licensed under the BSD 3-Clause license. + * For full license text, see LICENSE.md file in the repo root or https://opensource.org/licenses/BSD-3-Clause + */ + +/* eslint-disable camelcase */ +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { XMLParser } from 'fast-xml-parser'; +import { z } from 'zod'; +import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; +import type { ServerConfig } from '../server.js'; +import { assertPathAllowed, PathPolicyError } from '../security/pathPolicy.js'; +import { makeError, makeRequestId } from '../schemas/common.js'; +import { log } from '../logging/logger.js'; +import { desc } from './descHelper.js'; + +// ── Types ───────────────────────────────────────────────────────────────────── + +/** + * Field entry as returned by provar_org_describe. Mirrors the shape of a Salesforce + * describeSObjectResult.fields[] element, scoped to the four properties the + * authoring tools care about. + */ +export interface OrgDescribeField { + name: string; + type: string; + default_value: string | null; + nillable: boolean; +} + +export interface OrgDescribeObject { + name: string; + exists: boolean | null; + required_fields: OrgDescribeField[]; + field_count: number; + /** Present only when the cache file existed but failed to parse. */ + error_message?: string; +} + +export interface OrgDescribeResult { + workspace_path: string | null; + cache_age_ms: number | null; + objects: OrgDescribeObject[]; + details?: { suggestion: string }; +} + +/* + * On-disk cache schema (one file per object) consumed by this tool. + * The cache writer is Provar IDE; this tool is read-only. + * + * Layout: /.metadata//.json + * + * Each JSON file contains: { name: "Account", fields: [ { name, type, defaultValue, nillable }, ... ] } + * + * As a fallback, .xml / .object files (CustomObject metadata) are also accepted + * to ease migration from the legacy Provar IDE Eclipse cache layout. + */ +interface CachedField { + name: string; + type?: string; + defaultValue?: string | null; + nillable?: boolean; +} + +interface CachedObject { + name?: string; + fields?: CachedField[]; +} + +// ── Workspace discovery ─────────────────────────────────────────────────────── + +/** + * Normalise a project basename for use in fallback workspace dir names: + * "My Project Path " → "my-project-path". + */ +export function projectNameDashes(projectPath: string): string { + return path.basename(projectPath).trim().replace(/\s+/g, '-').toLowerCase(); +} + +/** + * Build the ordered list of candidate workspace directories. + * First existing wins. + * 1. /workspace-/ — sibling workspace pattern. + * 2. /Provar_Workspaces/workspace-/ + * 3. ~/Provar/workspace-/ — user-home fallback. + */ +export function workspaceCandidates(projectPath: string): string[] { + const resolved = path.resolve(projectPath); + const parent = path.dirname(resolved); + const basename = path.basename(resolved); + const dashes = projectNameDashes(resolved); + return [ + path.join(parent, `workspace-${basename}`), + path.join(parent, 'Provar_Workspaces', `workspace-${dashes}`), + path.join(os.homedir(), 'Provar', `workspace-${dashes}`), + ]; +} + +/** + * Returns the first candidate workspace that exists AND is within allowedPaths, or null. + * + * Path policy is enforced PER CANDIDATE before any filesystem call: a candidate that + * sits outside `--allowed-paths` is silently skipped (it is not an error — discovery + * just moves on to the next). This means we never call fs.existsSync / fs.statSync + * against directories that the operator has explicitly placed off-limits, including + * the user-home fallback (~/Provar/...) when home sits outside the policy. + * + * When allowedPaths is empty (unrestricted mode), assertPathAllowed is a no-op and + * all candidates are probed exactly as before. + */ +export function discoverWorkspace(projectPath: string, allowedPaths: string[] = []): string | null { + for (const candidate of workspaceCandidates(projectPath)) { + try { + assertPathAllowed(candidate, allowedPaths); + } catch { + // Candidate outside policy — skip without touching the filesystem. + continue; + } + try { + if (fs.existsSync(candidate) && fs.statSync(candidate).isDirectory()) { + return candidate; + } + } catch { + // Permission errors etc. — skip and try next candidate + } + } + return null; +} + +// ── Cache reading ───────────────────────────────────────────────────────────── + +const XML_PARSER = new XMLParser({ + ignoreAttributes: false, + attributeNamePrefix: '@_', + parseAttributeValue: false, + isArray: (name): boolean => name === 'fields', +}); + +/** Parse a JSON cache file into the canonical CachedObject shape. */ +function readJsonCacheFile(filePath: string): CachedObject { + const raw = fs.readFileSync(filePath, 'utf-8'); + return JSON.parse(raw) as CachedObject; +} + +/** + * Parse a legacy .object XML file (CustomObject metadata) into the canonical shape. + * Only extracts the bare minimum the tool needs: field name, type, nillable. + */ +function readXmlCacheFile(filePath: string): CachedObject { + const raw = fs.readFileSync(filePath, 'utf-8'); + const parsed = XML_PARSER.parse(raw) as Record; + const root = (parsed['CustomObject'] ?? parsed['toolingObjectInfo'] ?? {}) as Record; + const fieldsRaw = root['fields']; + if (!Array.isArray(fieldsRaw)) return { name: path.basename(filePath, path.extname(filePath)), fields: [] }; + + const fields: CachedField[] = []; + for (const f of fieldsRaw as Array>) { + const name = (f['fullName'] ?? f['name']) as string | undefined; + if (!name) continue; + // fast-xml-parser with parseTagValue=true (the default) coerces `true` + // into the boolean true; with parseTagValue=false it would stay as the string "true". + // Accept BOTH forms so we don't misclassify required fields as nillable on either path. + const requiredRaw = f['required']; + const isRequired = requiredRaw === true || requiredRaw === 'true'; + fields.push({ + name, + type: (f['type'] as string | undefined) ?? 'unknown', + defaultValue: (f['defaultValue'] as string | undefined) ?? null, + // XML defaults: required = !nillable. In the .object format, "required" is rare, + // so we default to nillable=true (optional) unless explicitly required. + nillable: !isRequired, + }); + } + return { name: path.basename(filePath, path.extname(filePath)), fields }; +} + +/** Look up the cache file for one object, trying .json then .xml. */ +function findObjectCacheFile(connectionDir: string, objectName: string): string | null { + const jsonPath = path.join(connectionDir, `${objectName}.json`); + if (fs.existsSync(jsonPath)) return jsonPath; + const xmlPath = path.join(connectionDir, `${objectName}.xml`); + if (fs.existsSync(xmlPath)) return xmlPath; + // Legacy CustomObject layout (.object extension) + const objPath = path.join(connectionDir, `${objectName}.object`); + if (fs.existsSync(objPath)) return objPath; + return null; +} + +/** List all cached object names (stripped of extension) in a connection directory. */ +function listCachedObjectNames(connectionDir: string): string[] { + let entries: string[]; + try { + entries = fs.readdirSync(connectionDir); + } catch { + return []; + } + const names = new Set(); + for (const entry of entries) { + const ext = path.extname(entry); + if (ext === '.json' || ext === '.xml' || ext === '.object') { + names.add(path.basename(entry, ext)); + } + } + return [...names].sort(); +} + +/** Read one object's cache file and convert it to an OrgDescribeObject. */ +function readObject(connectionDir: string, objectName: string, fieldFilter: 'required' | 'all'): OrgDescribeObject { + const file = findObjectCacheFile(connectionDir, objectName); + if (!file) { + return { name: objectName, exists: false, required_fields: [], field_count: 0 }; + } + let cached: CachedObject; + try { + cached = path.extname(file) === '.json' ? readJsonCacheFile(file) : readXmlCacheFile(file); + } catch (e) { + const errorMessage = (e as Error).message; + log('warn', 'org_describe: failed to parse cache file', { file, error: errorMessage }); + // The cache file is present but unreadable. Report exists=true so the caller + // can distinguish "cache corrupt / unsupported format" from "object not cached" + // (exists=false). field_count=0 since we have no parsed fields, and error_message + // carries the underlying parse failure for diagnostics. + return { + name: objectName, + exists: true, + required_fields: [], + field_count: 0, + error_message: `Failed to parse cache file (${path.basename(file)}): ${errorMessage}`, + }; + } + + const allFields = cached.fields ?? []; + const filtered = fieldFilter === 'required' ? allFields.filter((f) => f.nillable === false) : allFields; + + const fields: OrgDescribeField[] = filtered.map((f) => ({ + name: f.name, + type: f.type ?? 'unknown', + default_value: f.defaultValue ?? null, + nillable: f.nillable ?? true, + })); + + return { + name: cached.name ?? objectName, + exists: true, + required_fields: fields, + field_count: allFields.length, + }; +} + +/** Compute the mtime delta (ms) of a directory. Returns null on error. */ +function cacheAgeMs(dir: string): number | null { + try { + const stat = fs.statSync(dir); + return Math.max(0, Date.now() - stat.mtimeMs); + } catch { + return null; + } +} + +// ── Suggestion strings ──────────────────────────────────────────────────────── + +function cacheMissSuggestion(connectionName: string): string { + return ( + `Open this project in Provar IDE and load the '${connectionName}' connection, ` + + 'or pass field-type hints inline to provar_testcase_generate.' + ); +} + +// ── Core logic ──────────────────────────────────────────────────────────────── + +interface DescribeArgs { + project_path: string; + connection_name: string; + objects?: string[]; + field_filter?: 'required' | 'all'; +} + +/** + * Resolve & policy-check the workspace + connection directory. + * Returns the connection directory if it exists and is allowed, otherwise null. + */ +function resolveConnectionDir( + workspacePath: string | null, + connectionName: string, + allowedPaths: string[] +): { connectionDir: string | null; resolvedWorkspace: string | null } { + if (!workspacePath) return { connectionDir: null, resolvedWorkspace: null }; + + // Reject path-shaped connection names outright. A real connection name from a + // .testproject is an identifier (e.g. "MyOrg"); any separator or traversal + // segment is almost certainly a misuse or injection attempt. + const hasSeparator = connectionName.includes('/') || connectionName.includes('\\'); + const hasTraversal = connectionName === '..' || connectionName.split(/[/\\]+/).includes('..'); + if (hasSeparator || hasTraversal) { + throw new PathPolicyError( + 'PATH_TRAVERSAL', + `Invalid connection_name (must not contain path separators or directory-traversal segments ('..')): ${connectionName}` + ); + } + + // Path policy: workspace MUST be inside allowed paths before any fs call against it. + const resolvedWorkspace = path.resolve(workspacePath); + assertPathAllowed(resolvedWorkspace, allowedPaths); + + const connectionDir = path.resolve(resolvedWorkspace, '.metadata', connectionName); + // Belt-and-braces check after composition. + assertPathAllowed(connectionDir, allowedPaths); + + if (!fs.existsSync(connectionDir) || !fs.statSync(connectionDir).isDirectory()) { + return { connectionDir: null, resolvedWorkspace }; + } + return { connectionDir, resolvedWorkspace }; +} + +function buildCacheMissResponse( + resolvedWorkspace: string | null, + args: DescribeArgs, + requestId: string +): Record { + const objects: OrgDescribeObject[] = (args.objects ?? []).map((name) => ({ + name, + exists: null, + required_fields: [], + field_count: 0, + })); + return { + requestId, + workspace_path: resolvedWorkspace, + cache_age_ms: null, + objects, + details: { suggestion: cacheMissSuggestion(args.connection_name) }, + }; +} + +function buildHappyResponse( + resolvedWorkspace: string, + connectionDir: string, + args: DescribeArgs, + requestId: string +): Record { + const fieldFilter = args.field_filter ?? 'required'; + const requestedNames = args.objects?.length ? args.objects : listCachedObjectNames(connectionDir); + const objects = requestedNames.map((name) => readObject(connectionDir, name, fieldFilter)); + return { + requestId, + workspace_path: resolvedWorkspace, + cache_age_ms: cacheAgeMs(connectionDir), + objects, + }; +} + +// ── Tool registration ───────────────────────────────────────────────────────── + +export function registerOrgDescribe(server: McpServer, config: ServerConfig): void { + server.registerTool( + 'provar_org_describe', + { + title: 'Describe Org Objects From Workspace Cache', + description: desc( + [ + 'Read cached Salesforce describe data for one connection from the Provar workspace .metadata cache.', + 'Prerequisite: the project must have been opened in Provar IDE at least once with the named connection loaded', + '— this tool is read-only and does NOT trigger a metadata download.', + 'Workspace discovery tries in order: /workspace-, ', + '/Provar_Workspaces/workspace-, then ~/Provar/workspace-.', + 'Returns an empty result with details.suggestion when the cache is missing.', + 'Distinct from the runtime .provarCaches cache used by test execution.', + ].join(' '), + 'Read cached describe data for one connection from the Provar workspace .metadata cache.' + ), + inputSchema: { + project_path: z + .string() + .describe( + desc( + 'Absolute path to the Provar test project root (the directory containing .testproject). Must be within --allowed-paths.', + 'string, absolute path to Provar test project root' + ) + ), + connection_name: z + .string() + .describe( + desc( + 'Connection name as defined in the .testproject file (e.g. "MyOrg"). The .metadata cache subdirectory must match this exactly.', + 'string, connection name as defined in .testproject' + ) + ), + objects: z + .array(z.string()) + .optional() + .describe( + desc( + 'Optional filter — only return data for these object API names (e.g. ["Account","Contact"]). When omitted, lists all cached objects for the connection.', + 'string[], optional; restrict to these object API names' + ) + ), + field_filter: z + .enum(['required', 'all']) + .optional() + .default('required') + .describe( + desc( + 'Which fields to include. "required" (default) returns only fields with nillable=false; "all" returns every cached field.', + "'required' | 'all'; default 'required'" + ) + ), + }, + }, + (args: DescribeArgs) => { + const requestId = makeRequestId(); + log('info', 'provar_org_describe', { + requestId, + connection_name: args.connection_name, + object_count: args.objects?.length ?? null, + }); + + try { + assertPathAllowed(args.project_path, config.allowedPaths); + const workspacePath = discoverWorkspace(args.project_path, config.allowedPaths); + const { connectionDir, resolvedWorkspace } = resolveConnectionDir( + workspacePath, + args.connection_name, + config.allowedPaths + ); + + if (!connectionDir) { + const response = buildCacheMissResponse(resolvedWorkspace, args, requestId); + return { + content: [{ type: 'text' as const, text: JSON.stringify(response) }], + structuredContent: response, + }; + } + + const response = buildHappyResponse(resolvedWorkspace!, connectionDir, args, requestId); + return { + content: [{ type: 'text' as const, text: JSON.stringify(response) }], + structuredContent: response, + }; + } catch (err: unknown) { + const error = err as Error & { code?: string }; + return { + isError: true, + content: [ + { + type: 'text' as const, + text: JSON.stringify( + makeError( + error instanceof PathPolicyError ? error.code : error.code ?? 'ORG_DESCRIBE_ERROR', + error.message, + requestId + ) + ), + }, + ], + }; + } + } + ); +} + +export function registerAllOrgDescribeTools(server: McpServer, config: ServerConfig): void { + registerOrgDescribe(server, config); +} diff --git a/src/mcp/tools/propertiesTools.ts b/src/mcp/tools/propertiesTools.ts index f7b558df..06d9846f 100644 --- a/src/mcp/tools/propertiesTools.ts +++ b/src/mcp/tools/propertiesTools.ts @@ -16,6 +16,7 @@ import type { ServerConfig } from '../server.js'; import { assertPathAllowed, PathPolicyError } from '../security/pathPolicy.js'; import { makeError, makeRequestId } from '../schemas/common.js'; import { log } from '../logging/logger.js'; +import { WARNING_CODES, formatWarning } from '../utils/warningCodes.js'; import { desc } from './descHelper.js'; // ── Validation helpers ──────────────────────────────────────────────────────── @@ -30,12 +31,118 @@ const TOP_REQUIRED = ['provarHome', 'projectPath', 'resultsPath', 'metadata', 'e const METADATA_REQUIRED = ['metadataLevel', 'cachePath'] as const; const ENV_REQUIRED = ['webBrowser', 'webBrowserConfig', 'webBrowserProviderName', 'webBrowserDeviceName'] as const; +/** + * Canonical key sets for provardx-properties.json. Sourced from the documented schema in + * `docs/mcp.md` (provar_properties_set updates schema) and the `propertyFileContent` template + * from `@provartesting/provardx-plugins-utils`. Sibling PRs (PDX-488, PDX-494) may extend + * these sets — keep them exported and additive. + * + * NOTE: This set is intentionally lenient on "future Provar additions" — unknown keys emit a + * SCHEMA-001 warning (not a hard error) so older MCP clients keep working when new keys ship. + */ +export const CANONICAL_TOP_LEVEL_KEYS: readonly string[] = [ + // Required + 'provarHome', + 'projectPath', + 'resultsPath', + 'metadata', + 'environment', + // Optional — documented in docs/mcp.md properties_set schema + 'resultsPathDisposition', + 'testOutputLevel', + 'pluginOutputlevel', + 'stopOnError', + 'excludeCallable', + 'testprojectSecrets', + 'testCase', + 'testPlan', + 'connectionOverride', + // Optional — present in the standard template (propertyFileContent.js) + 'smtpPath', + 'lightningMode', + 'connectionRefreshType', + 'testplanFeatures', +]; + +export const CANONICAL_METADATA_KEYS: readonly string[] = ['metadataLevel', 'cachePath']; + +export const CANONICAL_ENVIRONMENT_KEYS: readonly string[] = [ + 'testEnvironment', + 'webBrowser', + 'webBrowserConfig', + 'webBrowserProviderName', + 'webBrowserDeviceName', +]; + const VALID_RESULTS_DISPOSITION = ['Increment', 'Replace', 'Fail']; const VALID_OUTPUT_LEVELS = ['BASIC', 'DETAILED', 'DIAGNOSTIC']; const VALID_PLUGIN_LEVELS = ['SEVERE', 'WARNING', 'INFO', 'FINE', 'FINER', 'FINEST']; const VALID_BROWSERS = ['Chrome', 'Safari', 'Edge', 'Edge_Legacy', 'Firefox', 'IE', 'Chrome_Headless']; const VALID_METADATA_LEVELS = ['Reuse', 'Reload', 'Refresh']; +/** + * Iterative Levenshtein distance — small, no-dependency implementation used solely for + * "did you mean ..." suggestions in SCHEMA-001 warnings. Returns the edit distance between + * `a` and `b`. Case-sensitive. + */ +function levenshtein(a: string, b: string): number { + if (a === b) return 0; + if (a.length === 0) return b.length; + if (b.length === 0) return a.length; + let prev: number[] = Array.from({ length: b.length + 1 }, (_, i) => i); + for (let i = 1; i <= a.length; i++) { + const curr: number[] = [i]; + for (let j = 1; j <= b.length; j++) { + const cost = a[i - 1] === b[j - 1] ? 0 : 1; + curr.push(Math.min(curr[j - 1] + 1, prev[j] + 1, prev[j - 1] + cost)); + } + prev = curr; + } + return prev[b.length]; +} + +/** + * Returns the canonical key whose Levenshtein distance from `key` is <= 2, or `undefined` + * if no key is within that threshold. Picks the closest match (smallest distance, then + * lexicographic) to keep suggestions stable across runs. + */ +function closestCanonicalKey(key: string, canonical: readonly string[]): string | undefined { + let best: { key: string; dist: number } | undefined; + for (const candidate of canonical) { + const dist = levenshtein(key, candidate); + if (dist > 2) continue; + if (!best || dist < best.dist || (dist === best.dist && candidate < best.key)) { + best = { key: candidate, dist }; + } + } + return best?.key; +} + +/** + * Emit SCHEMA-001 warnings for any keys in `actual` that are not in `canonical`. The + * `pathLabel` is the human-readable scope (`top-level`, `metadata`, `environment`). + */ +function findUnknownKeys( + actual: Record, + canonical: readonly string[], + pathLabel: string, + fieldPrefix: string +): ValidationError[] { + const results: ValidationError[] = []; + const canonicalSet = new Set(canonical); + for (const key of Object.keys(actual)) { + if (canonicalSet.has(key)) continue; + const suggestion = closestCanonicalKey(key, canonical); + const message = formatWarning(WARNING_CODES.SCHEMA_001, `Unknown field '${key}' at ${pathLabel}.`, suggestion); + results.push({ + field: fieldPrefix ? `${fieldPrefix}.${key}` : key, + message, + severity: 'warning', + }); + } + return results; +} + // eslint-disable-next-line complexity function validateProperties(props: Record): ValidationError[] { const errors: ValidationError[] = []; @@ -134,6 +241,15 @@ function validateProperties(props: Record): ValidationError[] { } } + // SCHEMA-001: unknown keys at any level — warning only so additive Provar versions don't break old clients + errors.push(...findUnknownKeys(props, CANONICAL_TOP_LEVEL_KEYS, 'top-level', '')); + if (meta && typeof meta === 'object') { + errors.push(...findUnknownKeys(meta, CANONICAL_METADATA_KEYS, 'metadata', 'metadata')); + } + if (env && typeof env === 'object') { + errors.push(...findUnknownKeys(env, CANONICAL_ENVIRONMENT_KEYS, 'environment', 'environment')); + } + return errors; } @@ -624,9 +740,12 @@ export function registerPropertiesValidate(server: McpServer, config: ServerConf [ 'Validate a provardx-properties.json file against the ProvarDX schema.', 'Checks required fields, valid enum values, and warns about unfilled placeholder values.', + 'Also emits a SCHEMA-001 warning for any unknown top-level, metadata.*, or environment.* key', + "(e.g. 'testCases' → did you mean 'testCase'?). Unknown keys are warnings, not errors,", + 'so additive keys in future Provar versions do not break older MCP clients.', 'Accepts either a file path or inline JSON content.', ].join(' '), - 'Validate a provardx-properties.json against required fields and enum values.' + 'Validate a provardx-properties.json; warns on unknown keys (SCHEMA-001) like testCases vs testCase.' ), inputSchema: { file_path: z diff --git a/src/mcp/tools/rcaTools.ts b/src/mcp/tools/rcaTools.ts index f37b6a97..e07a9808 100644 --- a/src/mcp/tools/rcaTools.ts +++ b/src/mcp/tools/rcaTools.ts @@ -32,6 +32,8 @@ interface LocateResult { resolution_source: string; } +type ErrorCategory = 'INFRASTRUCTURE' | 'ASSERTION' | 'LOCATOR' | 'TIMEOUT' | 'OTHER'; + interface FailureReport { test_case: string; error_class: string | null; @@ -44,6 +46,48 @@ interface FailureReport { report_html: string | null; screenshot_dir: string | null; pre_existing: boolean; + error_category?: ErrorCategory; + retryable?: boolean; +} + +/** + * Classify a failure message into a structured error category for retry decisions. + * + * Categories are coarse-grained and intended to drive automated retry policy + * downstream (e.g. retry INFRASTRUCTURE/TIMEOUT, never retry ASSERTION/LOCATOR). + * + * Returns `undefined` when no pattern matches — callers should leave the field unset. + */ +export function classifyErrorCategory(errorText: string): ErrorCategory | undefined { + // INFRASTRUCTURE — transient network / socket / browser-process failures. + if (/Connection reset|Failed to read client socket message|socket hang up|ECONNRESET/i.test(errorText)) { + return 'INFRASTRUCTURE'; + } + // LOCATOR — page object selector no longer matches the rendered DOM. + if (/NoSuchElementException/i.test(errorText)) return 'LOCATOR'; + // TIMEOUT — element or operation did not complete in time. + if (/TimeoutException/i.test(errorText)) return 'TIMEOUT'; + // ASSERTION — explicit assertion failure raised by the test or framework. + if (/AssertionException/i.test(errorText)) return 'ASSERTION'; + // OTHER — known exception class but not fitting the four primary buckets. + if ( + /SessionNotCreatedException|WebDriverException|ClassNotFoundException|LicenseException|InvalidPasswordException/i.test( + errorText + ) + ) { + return 'OTHER'; + } + return undefined; +} + +/** + * A failure is retryable only when the underlying cause is transient. + * INFRASTRUCTURE (network blips) and TIMEOUT (slow page) can succeed on retry; + * ASSERTION / LOCATOR / OTHER are deterministic and should not be retried. + */ +export function isRetryable(category: ErrorCategory | undefined): boolean | undefined { + if (category === undefined) return undefined; + return category === 'INFRASTRUCTURE' || category === 'TIMEOUT'; } // ── Root cause classification ───────────────────────────────────────────────── @@ -667,7 +711,9 @@ function buildFailureReports( const poMatch = /Page Object:\s*([\w.]+)/i.exec(failureText); const opMatch = /operation:\s*(\w+)/i.exec(failureText); const matchingHtml = htmlFiles.find((f) => path.basename(f) === `${tc.name}.html`); - reports.push({ + const error_category = classifyErrorCategory(failureText); + const retryable = isRetryable(error_category); + const report: FailureReport = { test_case: tc.name, error_class, error_message: failureText.slice(0, 500), @@ -679,7 +725,10 @@ function buildFailureReports( report_html: matchingHtml ?? null, screenshot_dir: screenshotDir, pre_existing: priorFailed.has(tc.name), - }); + }; + if (error_category !== undefined) report.error_category = error_category; + if (retryable !== undefined) report.retryable = retryable; + reports.push(report); } return reports; } @@ -699,6 +748,10 @@ export function registerTestRunRca(server: McpServer, config: ServerConfig): voi 'Use mode="failures" to get a lightweight array of failed test cases', '([{ testItemId, title, errorMessage }]) without the full RCA classification — useful when you', 'need failure names quickly without loading the HTML report.', + 'In mode="rca" (default), each entry in failures[] additionally includes optional error_category', + '(INFRASTRUCTURE|ASSERTION|LOCATOR|TIMEOUT|OTHER) and retryable (boolean) fields when the failure', + 'text matches a known pattern — INFRASTRUCTURE/TIMEOUT are flagged retryable, others are not.', + 'These fields are NOT included in mode="failures" output.', ].join(' '), 'Parse a Provar test run JUnit.xml and produce an RCA report with failure classification.' ), diff --git a/src/mcp/tools/testCaseGenerate.ts b/src/mcp/tools/testCaseGenerate.ts index 6e97b736..cea87f73 100644 --- a/src/mcp/tools/testCaseGenerate.ts +++ b/src/mcp/tools/testCaseGenerate.ts @@ -150,6 +150,11 @@ const TOOL_DESCRIPTION = [ 'Variable references: pass values as "{VarName}" (braces); emitted as class="variable" .', '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="...".', + 'valueClass auto-detection: argument values are typed automatically before XML emission. ' + + 'ISO-8601 date "YYYY-MM-DD" → valueClass="date"; ISO-8601 datetime "YYYY-MM-DDTHH:MM:SS" (optional fractional seconds + timezone) → "datetime"; ' + + '"true"/"false" → "boolean"; numeric string (e.g. "42", "-5", "3.14") → "decimal"; otherwise "string". ' + + 'Pass dates / booleans / numbers in those formats — Provar runtime silently discards date fields emitted as valueClass="string". ' + + 'Note: numbers always emit valueClass="decimal" per the canonical Provar reference (there is no separate "integer" valueClass).', 'Edit page objects: action=Edit targets require a compiled page object for the SF object. ' + 'If none exists in the project page-objects directory, the locator binding will fail at runtime. ' + 'For objects without a compiled Edit page object, use inline edit instead: sfIleActivate to activate the field, ' + @@ -374,6 +379,40 @@ export function registerTestCaseGenerate(server: McpServer, config: ServerConfig // ── XML builder ─────────────────────────────────────────────────────────────── +// PDX-493 (H3): infer the Salesforce `valueClass` attribute that should be emitted on a +// `` element from an argument's key + string value. +// +// Detection order: +// 1. Explicit `fieldTypeHint` (from `field_type_hints` param or `provar_org_describe` cache — wired +// in PDX-492 H2b) wins if provided. +// 2. ISO-8601 datetime → 'datetime' (e.g. "2026-05-19T10:30:00", with optional fractional seconds +// and optional timezone). The regex is end-anchored so trailing garbage (e.g. +// "2026-05-19T10:30:00not-a-zone") is rejected as plain 'string'. +// 3. ISO-8601 date → 'date' (e.g. "2026-05-19"). +// 4. Literal 'true' / 'false' → 'boolean'. +// 5. Numeric string (integer or decimal, optional leading '-') → 'decimal'. +// Per `docs/PROVAR_TEST_STEP_REFERENCE.md` (lines 1338, 1428) the canonical Provar +// valueClass for numbers is `decimal` — there is no `integer` valueClass in the +// reference grammar, so both `42` and `3.14` emit as `valueClass="decimal"`. +// 6. Else → 'string'. +// +// The `key` argument is reserved for future heuristics (e.g. SF naming conventions like *__c) +// but is intentionally not consulted today — explicit hints + value-shape regexes are the +// safer signal until org-describe is wired. +export function inferSalesforceValueClass( + // eslint-disable-next-line @typescript-eslint/no-unused-vars + key: string, + val: string, + fieldTypeHint?: 'date' | 'datetime' | 'boolean' | 'decimal' | 'string' +): 'date' | 'datetime' | 'boolean' | 'decimal' | 'string' { + if (fieldTypeHint) return fieldTypeHint; + if (/^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}(\.\d+)?(Z|[+-]\d{2}:?\d{2})?$/.test(val)) return 'datetime'; + if (/^\d{4}-\d{2}-\d{2}$/.test(val)) return 'date'; + if (val === 'true' || val === 'false') return 'boolean'; + if (/^-?\d+(\.\d+)?$/.test(val)) return 'decimal'; + return 'string'; +} + // F1/F3: build class="compound" for strings that mix literal text with {VarName} tokens. function buildCompoundValue(val: string, indent: string): string { const i = `${indent} `; @@ -423,7 +462,11 @@ function buildArgumentValue(key: string, val: string, indent: string, inNamedVal return `${indent}`; } } - return `${indent}${escapeXmlContent(val)}`; + // PDX-493 (H3): infer valueClass for date / datetime / boolean / decimal / string. The + // `fieldTypeHint` parameter on `inferSalesforceValueClass` is intentionally not threaded + // through here yet — it lands in PDX-492 (H2b) along with the `field_type_hints` tool input. + const inferred = inferSalesforceValueClass(key, val); + return `${indent}${escapeXmlContent(val)}`; } function buildArgumentsXml(attributes: Record, baseIndent = ' ', apiId = ''): string { diff --git a/src/mcp/tools/testCaseValidate.ts b/src/mcp/tools/testCaseValidate.ts index b374db21..15e0e233 100644 --- a/src/mcp/tools/testCaseValidate.ts +++ b/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 { 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), 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), 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 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 ``. + * + * 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, + 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, + ' 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 but CLI standalone execution does not bind CSV column variables — steps using 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 `` 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: 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 but CLI standalone execution does not bind CSV column variables — steps using 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 → fast-xml-parser yields '' const rawSteps = tc['steps']; diff --git a/src/mcp/utils/testCasePlanMode.ts b/src/mcp/utils/testCasePlanMode.ts new file mode 100644 index 00000000..0f98ac74 --- /dev/null +++ b/src/mcp/utils/testCasePlanMode.ts @@ -0,0 +1,238 @@ +/* + * Copyright (c) 2024 Provar Limited. + * All rights reserved. + * Licensed under the BSD 3-Clause license. + * For full license text, see LICENSE.md file in the repo root or https://opensource.org/licenses/BSD-3-Clause + */ + +/* eslint-disable camelcase */ +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { assertPathAllowed } from '../security/pathPolicy.js'; + +/** + * How the project's provardx-properties.json runs this test case. + * + * `direct` means the test case path appears in `testCase` or `testCases` and + * no `.testinstance` references it — data-driven `` rows will not + * iterate. `plan` means the test case is referenced by at least one + * `.testinstance` under `plans/`, so data-driven execution works. `unknown` + * means the properties file could not be resolved or parsed (no + * `~/.sf/config.json`, file outside allowed paths, missing project root, etc.) + * — callers should default to the safe behaviour (emit the structural + * warning) when the mode is unknown. + */ +export type TestCasePlanMode = 'direct' | 'plan' | 'unknown'; + +export interface ResolvedTestCaseMode { + mode: TestCasePlanMode; + /** The properties-file path consulted (when resolved). */ + propertiesFilePath?: string; + /** The project root resolved from the properties file (when present). */ + projectPath?: string; +} + +interface ResolveOptions { + /** Path to the test case file under validation. */ + testCaseFilePath: string; + /** Allowed-paths policy from the MCP server config. */ + allowedPaths: string[]; + /** Override of ~/.sf/config.json location for testing. */ + sfConfigPathOverride?: string; + /** Override of provardx-properties.json location for testing. */ + propertiesFilePathOverride?: string; +} + +/** + * Resolve whether a given test case file is referenced directly via + * `testCase`/`testCases` or via a `.testinstance` inside a plan. + * + * The resolution flow is intentionally best-effort and silent: any file + * read failure, JSON parse error, or path-policy violation collapses to + * `mode: 'unknown'` so the caller falls back to default behaviour rather + * than surfacing a confusing error from the validator. + */ +export function resolveTestCasePlanMode(opts: ResolveOptions): ResolvedTestCaseMode { + const propsPath = readPropertiesFilePath(opts); + if (!propsPath) return { mode: 'unknown' }; + + let propsObj: Record; + try { + propsObj = JSON.parse(fs.readFileSync(propsPath, 'utf-8')) as Record; + } catch { + return { mode: 'unknown', propertiesFilePath: propsPath }; + } + + const projectPath = typeof propsObj['projectPath'] === 'string' ? propsObj['projectPath'] : null; + if (!projectPath) { + // Without a project root we cannot resolve relative `testCase` entries or + // walk `plans/`. The mode is unknown. + return { mode: 'unknown', propertiesFilePath: propsPath }; + } + + let resolvedProjectPath: string; + try { + resolvedProjectPath = path.resolve(projectPath); + assertPathAllowed(resolvedProjectPath, opts.allowedPaths); + } catch { + return { mode: 'unknown', propertiesFilePath: propsPath }; + } + + const resolvedTestCasePath = path.resolve(opts.testCaseFilePath); + + // A `.testinstance` reference inside any plan wins — plan mode supports + // data-driven iteration. + if (isReferencedFromPlanInstance(resolvedProjectPath, resolvedTestCasePath)) { + return { mode: 'plan', propertiesFilePath: propsPath, projectPath: resolvedProjectPath }; + } + + if (isReferencedDirectly(propsObj, resolvedProjectPath, resolvedTestCasePath)) { + return { mode: 'direct', propertiesFilePath: propsPath, projectPath: resolvedProjectPath }; + } + + return { mode: 'unknown', propertiesFilePath: propsPath, projectPath: resolvedProjectPath }; +} + +/** + * Resolve the active properties file path. Prefers an explicit override + * (used by tests), then `~/.sf/config.json`'s `PROVARDX_PROPERTIES_FILE_PATH`. + * + * Both the override and the value read from `~/.sf/config.json` are funnelled + * through `assertPathAllowed` so a caller cannot trick the resolver into + * reading a properties file outside the configured `allowedPaths`. Any policy + * violation collapses to `null` to preserve the helper's best-effort contract. + */ +function readPropertiesFilePath(opts: ResolveOptions): string | null { + if (opts.propertiesFilePathOverride) { + return enforceAllowed(opts.propertiesFilePathOverride, opts.allowedPaths); + } + try { + const sfConfigPath = opts.sfConfigPathOverride ?? path.join(os.homedir(), '.sf', 'config.json'); + if (!fs.existsSync(sfConfigPath)) return null; + const sfConfig = JSON.parse(fs.readFileSync(sfConfigPath, 'utf-8')) as Record; + const propsPath = sfConfig['PROVARDX_PROPERTIES_FILE_PATH'] as string | undefined; + if (!propsPath) return null; + return enforceAllowed(propsPath, opts.allowedPaths); + } catch { + return null; + } +} + +/** + * Resolve a candidate properties-file path through `assertPathAllowed`, then + * canonicalize via `fs.realpathSync` when the file exists so a symlink + * inside an allowed directory cannot redirect the read outside it. Returns + * `null` on any policy violation, missing file, or unexpected error — the + * caller treats `null` as "mode unknown" and falls back to default behaviour. + */ +function enforceAllowed(candidate: string, allowedPaths: string[]): string | null { + try { + const resolved = path.resolve(candidate); + assertPathAllowed(resolved, allowedPaths); + if (!fs.existsSync(resolved)) return null; + try { + return fs.realpathSync(resolved); + } catch { + return resolved; + } + } catch { + return null; + } +} + +/** + * Does the properties file's top-level `testCase` or `testCases` array + * reference this test case file? Entries are interpreted relative to + * `/tests/` (per the Provar runtime convention). + */ +function isReferencedDirectly(props: Record, projectPath: string, testCaseFilePath: string): boolean { + const entries: string[] = []; + const tc = props['testCase']; + const tcs = props['testCases']; + for (const candidate of [tc, tcs]) { + if (typeof candidate === 'string') { + entries.push(candidate); + } else if (Array.isArray(candidate)) { + for (const e of candidate as unknown[]) { + if (typeof e === 'string') entries.push(e); + } + } + } + if (entries.length === 0) return false; + + const testsDir = path.join(projectPath, 'tests'); + const targetNorm = path.resolve(testCaseFilePath).toLowerCase(); + + for (const entry of entries) { + // Provar accepts both bare names ("MyTest") and relative paths + // ("Module/MyTest.testcase"). Allow either; match with and without the + // `.testcase` extension. + const variants: string[] = []; + const trimmed = entry.replace(/^[/\\]+/, ''); + variants.push(path.resolve(testsDir, trimmed)); + if (!/\.testcase$/i.test(trimmed)) { + variants.push(path.resolve(testsDir, `${trimmed}.testcase`)); + } + for (const v of variants) { + if (v.toLowerCase() === targetNorm) return true; + } + } + return false; +} + +/** + * Walk `/plans/` for `.testinstance` files referencing this + * test case via `testCasePath="..."`. Best-effort — any read error skips + * the offending file. + */ +function isReferencedFromPlanInstance(projectPath: string, testCaseFilePath: string): boolean { + const plansDir = path.join(projectPath, 'plans'); + if (!fs.existsSync(plansDir)) return false; + + const testsDir = path.join(projectPath, 'tests'); + const targetNorm = path.resolve(testCaseFilePath).toLowerCase(); + let found = false; + + const walk = (dir: string): void => { + if (found) return; + let entries: fs.Dirent[]; + try { + entries = fs.readdirSync(dir, { withFileTypes: true }); + } catch { + return; + } + for (const entry of entries) { + if (found) return; + if (entry.name.startsWith('.') && !entry.name.endsWith('.testinstance')) continue; + const full = path.join(dir, entry.name); + if (entry.isDirectory()) { + walk(full); + continue; + } + if (!entry.name.endsWith('.testinstance')) continue; + let content: string; + try { + content = fs.readFileSync(full, 'utf-8'); + } catch { + continue; + } + const matches = content.matchAll(/testCasePath=["']([^"']+)["']/g); + for (const m of matches) { + const rel = m[1].replace(/\\/g, '/'); + // testCasePath in Provar testinstances is conventionally relative to + // `/tests/`. Also tolerate paths relative to project root. + const candidates = [path.resolve(testsDir, rel), path.resolve(projectPath, rel)]; + for (const c of candidates) { + if (c.toLowerCase() === targetNorm) { + found = true; + return; + } + } + } + } + }; + + walk(plansDir); + return found; +} diff --git a/src/mcp/utils/warningCodes.ts b/src/mcp/utils/warningCodes.ts new file mode 100644 index 00000000..9da5d9a3 --- /dev/null +++ b/src/mcp/utils/warningCodes.ts @@ -0,0 +1,22 @@ +/* + * Copyright (c) 2024 Provar Limited. + * All rights reserved. + * Licensed under the BSD 3-Clause license. + * For full license text, see LICENSE.md file in the repo root or https://opensource.org/licenses/BSD-3-Clause + */ + +export const WARNING_CODES = { + PROVARHOME_001: 'PROVARHOME-001', + DATA_001: 'DATA-001', + PARALLEL_001: 'PARALLEL-001', + SCHEMA_001: 'SCHEMA-001', + RUN_001: 'RUN-001', + JUNIT_001: 'JUNIT-001', +} as const; + +export type WarningCode = (typeof WARNING_CODES)[keyof typeof WARNING_CODES]; + +export function formatWarning(code: WarningCode, message: string, suggestion?: string): string { + const base = `WARNING [${code}]: ${message}`; + return suggestion ? `${base} Did you mean '${suggestion}'?` : base; +} diff --git a/test/fixtures/properties/testcases-typo.json b/test/fixtures/properties/testcases-typo.json new file mode 100644 index 00000000..bdef42eb --- /dev/null +++ b/test/fixtures/properties/testcases-typo.json @@ -0,0 +1,21 @@ +{ + "provarHome": "/opt/provar", + "projectPath": "/projects/sample-project", + "resultsPath": "/projects/sample-project/ANT/Results", + "resultsPathDisposition": "Replace", + "testOutputLevel": "BASIC", + "pluginOutputlevel": "WARNING", + "stopOnError": false, + "metadata": { + "metadataLevel": "Reuse", + "cachePath": "/projects/sample-project/ANT/.provarCaches" + }, + "environment": { + "testEnvironment": "default", + "webBrowser": "Chrome_Headless", + "webBrowserConfig": "Full Screen", + "webBrowserProviderName": "Desktop", + "webBrowserDeviceName": "Full Screen" + }, + "testCases": ["tests/SmokeFlow.testcase"] +} diff --git a/test/unit/mcp/antTools.test.ts b/test/unit/mcp/antTools.test.ts index 74e37bc2..3f027091 100644 --- a/test/unit/mcp/antTools.test.ts +++ b/test/unit/mcp/antTools.test.ts @@ -800,12 +800,14 @@ describe('parseJUnitResults', () => { const result = parseJUnitResults(path.join(junitTmpDir, 'nonexistent')); assert.deepEqual(result.steps, []); assert.ok(result.warning?.includes('not found')); + assert.equal(result.parsedAny, false, 'parsedAny must be false when dir is missing'); }); it('returns warning when directory contains no XML files', () => { const result = parseJUnitResults(junitTmpDir); assert.deepEqual(result.steps, []); assert.ok(result.warning?.includes('No JUnit XML')); + assert.equal(result.parsedAny, false, 'parsedAny must be false when no XML files exist'); }); it('extracts steps from a bare JUnit file', () => { @@ -818,6 +820,7 @@ describe('parseJUnitResults', () => { assert.equal(result.steps[1].status, 'fail'); assert.ok(result.steps[1].errorMessage?.includes('Element not found')); assert.equal(result.warning, undefined); + assert.equal(result.parsedAny, true, 'parsedAny must be true when at least one file parsed'); }); it('extracts steps from a wrapper JUnit file', () => { @@ -836,6 +839,18 @@ describe('parseJUnitResults', () => { const result = parseJUnitResults(junitTmpDir); assert.deepEqual(result.steps, []); assert.ok((result.warning?.length ?? 0) > 0); + // parsedAny must be TRUE here: the file was readable and parsed, it just has zero + // entries. This is the legitimate RUN-001 signal — distinct from "we have + // no data at all". + assert.equal(result.parsedAny, true, 'parsedAny must be true when XML parsed but had no steps'); + }); + + it('returns parsedAny=false when all XML files fail to parse', () => { + fs.writeFileSync(path.join(junitTmpDir, 'broken.xml'), ' { @@ -847,4 +862,55 @@ describe('parseJUnitResults', () => { assert.ok(result.steps[0].errorMessage?.includes('Execution failed')); assert.ok(result.steps[0].errorMessage?.includes('stack trace here')); }); + + // ── PDX-490: error_category + retryable on step results ───────────────────── + + function writeFailureJunit(dir: string, failureBody: string): void { + const xml = `${failureBody}`; + fs.writeFileSync(path.join(dir, 'JUnit.xml'), xml); + } + + it('populates error_category=INFRASTRUCTURE and retryable=true for Connection reset', () => { + writeFailureJunit(junitTmpDir, 'Connection reset by peer while reading response'); + const result = parseJUnitResults(junitTmpDir); + assert.equal(result.steps[0].error_category, 'INFRASTRUCTURE'); + assert.equal(result.steps[0].retryable, true); + }); + + it('populates error_category=LOCATOR and retryable=false for NoSuchElementException', () => { + writeFailureJunit(junitTmpDir, 'NoSuchElementException: Unable to locate element'); + const result = parseJUnitResults(junitTmpDir); + assert.equal(result.steps[0].error_category, 'LOCATOR'); + assert.equal(result.steps[0].retryable, false); + }); + + it('populates error_category=TIMEOUT and retryable=true for TimeoutException', () => { + writeFailureJunit(junitTmpDir, 'TimeoutException: operation did not complete'); + const result = parseJUnitResults(junitTmpDir); + assert.equal(result.steps[0].error_category, 'TIMEOUT'); + assert.equal(result.steps[0].retryable, true); + }); + + it('populates error_category=ASSERTION and retryable=false for AssertionException', () => { + writeFailureJunit(junitTmpDir, 'AssertionException: expected X but was Y'); + const result = parseJUnitResults(junitTmpDir); + assert.equal(result.steps[0].error_category, 'ASSERTION'); + assert.equal(result.steps[0].retryable, false); + }); + + it('leaves error_category and retryable undefined when no pattern matches', () => { + writeFailureJunit(junitTmpDir, 'something completely unrecognised XYZ_BANANA'); + const result = parseJUnitResults(junitTmpDir); + assert.equal(result.steps[0].error_category, undefined); + assert.equal(result.steps[0].retryable, undefined); + }); + + it('does not set error_category or retryable on passing steps', () => { + const xml = ''; + fs.writeFileSync(path.join(junitTmpDir, 'JUnit.xml'), xml); + const result = parseJUnitResults(junitTmpDir); + assert.equal(result.steps[0].status, 'pass'); + assert.equal(result.steps[0].error_category, undefined); + assert.equal(result.steps[0].retryable, undefined); + }); }); diff --git a/test/unit/mcp/automationTools.test.ts b/test/unit/mcp/automationTools.test.ts index 2a9c3c30..2f63ab92 100644 --- a/test/unit/mcp/automationTools.test.ts +++ b/test/unit/mcp/automationTools.test.ts @@ -20,6 +20,7 @@ import { filterTestRunOutput, setSfResultsPathForTesting, } from '../../../src/mcp/tools/automationTools.js'; +import { WARNING_CODES, formatWarning } from '../../../src/mcp/utils/warningCodes.js'; // ── Minimal mock server ─────────────────────────────────────────────────────── @@ -198,7 +199,10 @@ describe('automationTools', () => { } }); - it('omits steps[] and adds details.warning when results dir has no XML', () => { + it('omits steps[] and adds details.warning when results dir has no XML — RUN-001 stays silent', () => { + // PDX-486 follow-up: when the results dir is resolvable but contains zero XML files, + // we have no data on which to claim "zero tests executed". parsedAny is false, so + // RUN-001 must NOT fire; the missing-data case is surfaced via details.warning only. const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'junit-empty-')); setSfResultsPathForTesting(tmpDir); @@ -210,15 +214,22 @@ describe('automationTools', () => { assert.ok(body.details, 'details should be present'); const details = body.details as Record; assert.ok(typeof details.warning === 'string', 'details.warning should be a string'); + assert.equal(body.warnings, undefined, 'RUN-001 must NOT fire when no XML data is parseable'); } finally { setSfResultsPathForTesting(undefined); fs.rmSync(tmpDir, { recursive: true, force: true }); } }); - it('omits steps[] and adds details.warning when results dir contains malformed XML', () => { + it('omits steps[] and adds details.warning when results dir contains malformed XML — RUN-001 stays silent', () => { + // PDX-486 follow-up: if every XML file in the results dir fails to parse, parsedAny + // is false. RUN-001 must stay silent — the agent should be steered toward "why is + // the JUnit output malformed?" (via details.warning), not toward a properties typo. const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'junit-bad-')); - fs.writeFileSync(path.join(tmpDir, 'results.xml'), '', 'utf-8'); + // fast-xml-parser is permissive about *some* malformed text — this input shape (truncated + // before any matchable close tag) is one that genuinely throws under the strict isArray + // config used in antTools.parseJUnitResults, exercising the parsedAny=false branch. + fs.writeFileSync(path.join(tmpDir, 'results.xml'), ' { const body = parseBody(result); assert.equal(body.steps, undefined, 'steps should be absent when XML is malformed'); assert.ok(body.details, 'details should be present with warning'); + const details = body.details as Record; + assert.ok(typeof details.warning === 'string', 'details.warning should describe the parse failure'); + assert.equal(body.warnings, undefined, 'RUN-001 must NOT fire when XML data is unparseable'); } finally { setSfResultsPathForTesting(undefined); fs.rmSync(tmpDir, { recursive: true, force: true }); } }); + + // ── RUN-001 zero-tests guard (PDX-486) ────────────────────────────────── + + describe('RUN-001 zero-tests-executed warning', () => { + const expectedWarning = formatWarning( + WARNING_CODES.RUN_001, + 'Test run exited successfully but zero tests were executed. ' + + 'Check the testCase / testCases (note spelling) field in provardx-properties.json.' + ); + + it('fires when exit is 0 and JUnit step count is 0 (empty )', () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'junit-zero-')); + // Valid JUnit XML but containing zero elements — the canonical + // signal that the test selector (e.g. a misspelled testCase field) matched nothing. + const emptySuiteXml = ` +`; + fs.writeFileSync(path.join(tmpDir, 'results.xml'), emptySuiteXml, 'utf-8'); + setSfResultsPathForTesting(tmpDir); + + try { + spawnStub.returns(makeSpawnResult('tests done', '', 0)); + const result = server.call('provar_automation_testrun', { flags: [] }); + assert.ok(!isError(result), 'tool should still report success — RUN-001 is additive'); + const body = parseBody(result); + assert.equal(body.exitCode, 0); + assert.ok(Array.isArray(body.warnings), 'warnings[] should be present'); + const warnings = body.warnings as string[]; + assert.equal(warnings.length, 1); + assert.equal(warnings[0], expectedWarning, 'warning string must match formatWarning output exactly'); + // Sanity-check the format envelope without inline strings: + assert.ok(warnings[0].startsWith(`WARNING [${WARNING_CODES.RUN_001}]:`)); + } finally { + setSfResultsPathForTesting(undefined); + fs.rmSync(tmpDir, { recursive: true, force: true }); + } + }); + + it('does NOT fire when exit is 0 and step count >= 1', () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'junit-some-')); + const oneCaseXml = ` + + +`; + fs.writeFileSync(path.join(tmpDir, 'results.xml'), oneCaseXml, 'utf-8'); + setSfResultsPathForTesting(tmpDir); + + try { + spawnStub.returns(makeSpawnResult('tests done', '', 0)); + const result = server.call('provar_automation_testrun', { flags: [] }); + const body = parseBody(result); + assert.equal(body.exitCode, 0); + const steps = body.steps as unknown[] | undefined; + assert.ok(steps && steps.length === 1, 'steps should reflect the one parsed testcase'); + assert.equal(body.warnings, undefined, 'warnings[] must be absent when at least one test ran'); + } finally { + setSfResultsPathForTesting(undefined); + fs.rmSync(tmpDir, { recursive: true, force: true }); + } + }); + + it('does NOT fire when exit is non-zero (genuine failure path)', () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'junit-fail-')); + // Even if zero steps were extracted, a non-zero exit is a real failure, not a typo. + const emptySuiteXml = ` +`; + fs.writeFileSync(path.join(tmpDir, 'results.xml'), emptySuiteXml, 'utf-8'); + setSfResultsPathForTesting(tmpDir); + + try { + spawnStub.returns(makeSpawnResult('', 'compilation error', 1)); + const result = server.call('provar_automation_testrun', { flags: [] }); + assert.ok(isError(result), 'should report error on non-zero exit'); + const body = parseBody(result); + assert.equal(body.error_code, 'AUTOMATION_TESTRUN_FAILED'); + // Failure path uses the error body shape — RUN-001 lives on the success-path response only. + assert.equal(body.warnings, undefined, 'warnings[] must not be set on the failure path'); + } finally { + setSfResultsPathForTesting(undefined); + fs.rmSync(tmpDir, { recursive: true, force: true }); + } + }); + + it('does NOT fire when results dir cannot be located (stay silent — config_load issue, not a typo)', () => { + // Explicitly disable the results path override so introspection returns "not resolved". + // RUN-001 must stay silent here so the agent isn't misdirected toward a typo when the + // real issue is "config_load was never called" (or the results dir is outside allowed paths). + // The existing failure-path details.warning channel covers the config_load case on errors; + // on the success path with no resolvable results dir we simply don't emit RUN-001 — silence + // is correct because we genuinely don't know whether tests ran or not. + setSfResultsPathForTesting(null); + + try { + spawnStub.returns(makeSpawnResult('tests done', '', 0)); + const result = server.call('provar_automation_testrun', { flags: [] }); + const body = parseBody(result); + assert.equal(body.exitCode, 0); + assert.equal(body.warnings, undefined, 'no RUN-001 when results dir is unresolved'); + } finally { + setSfResultsPathForTesting(undefined); + } + }); + }); }); // ── provar_automation_compile ───────────────────────────────────────────── diff --git a/test/unit/mcp/orgDescribeTools.test.ts b/test/unit/mcp/orgDescribeTools.test.ts new file mode 100644 index 00000000..e0b851d6 --- /dev/null +++ b/test/unit/mcp/orgDescribeTools.test.ts @@ -0,0 +1,594 @@ +/* + * Copyright (c) 2024 Provar Limited. + * All rights reserved. + * Licensed under the BSD 3-Clause license. + * For full license text, see LICENSE.md file in the repo root or https://opensource.org/licenses/BSD-3-Clause + */ + +/* eslint-disable camelcase */ +import { strict as assert } from 'node:assert'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { describe, it, beforeEach, afterEach } from 'mocha'; +import { + registerOrgDescribe, + discoverWorkspace, + projectNameDashes, + workspaceCandidates, +} from '../../../src/mcp/tools/orgDescribeTools.js'; +import type { ServerConfig } from '../../../src/mcp/server.js'; + +// ── Minimal McpServer mock ───────────────────────────────────────────────────── + +type ToolHandler = (args: Record) => unknown; + +class MockMcpServer { + private handlers = new Map(); + + public registerTool(name: string, _config: unknown, handler: ToolHandler): void { + this.handlers.set(name, handler); + } + + public call(name: string, args: Record): ReturnType { + const h = this.handlers.get(name); + if (!h) throw new Error(`Tool not registered: ${name}`); + return h(args); + } +} + +// ── Helpers ──────────────────────────────────────────────────────────────────── + +function parseText(result: unknown): Record { + const r = result as { content: Array<{ type: string; text: string }> }; + return JSON.parse(r.content[0].text) as Record; +} + +function isError(result: unknown): boolean { + return (result as { isError?: boolean }).isError === true; +} + +interface CachedField { + name: string; + type: string; + defaultValue: string | null; + nillable: boolean; +} + +/** Write a JSON cache file for one object. */ +function writeJsonCache(connectionDir: string, objectName: string, fields: CachedField[]): void { + fs.mkdirSync(connectionDir, { recursive: true }); + fs.writeFileSync( + path.join(connectionDir, `${objectName}.json`), + JSON.stringify({ name: objectName, fields }), + 'utf-8' + ); +} + +/** + * Write a legacy .object / .xml cache file (CustomObject metadata) with the given fields. + * `required` is emitted as the string "true" / "false" — but fast-xml-parser with the + * default parseTagValue=true will coerce it to a boolean before reaching the reader. + * The implementation must accept BOTH forms, which is what the legacy-format tests assert. + */ +function writeXmlCache( + connectionDir: string, + objectName: string, + fields: Array<{ name: string; type: string; required: boolean }>, + ext: '.xml' | '.object' = '.xml' +): void { + fs.mkdirSync(connectionDir, { recursive: true }); + const fieldsXml = fields + .map( + (f) => + ` \n ${f.name}\n ${f.type}\n ${String( + f.required + )}\n ` + ) + .join('\n'); + const xml = `\n\n${fieldsXml}\n\n`; + fs.writeFileSync(path.join(connectionDir, `${objectName}${ext}`), xml, 'utf-8'); +} + +// ── Test setup ───────────────────────────────────────────────────────────────── + +let tmpRoot: string; +let projectPath: string; +let server: MockMcpServer; +let config: ServerConfig; + +beforeEach(() => { + // Use realpathSync to canonicalise the path on macOS (/var → /private/var) so + // assertPathAllowed comparisons match the realpath the policy resolves to. + tmpRoot = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'org-describe-test-'))); + projectPath = path.join(tmpRoot, 'MyProject'); + fs.mkdirSync(projectPath, { recursive: true }); + + server = new MockMcpServer(); + // tmpRoot must be allowed so both the project path and any sibling workspace + // candidate (also under tmpRoot) pass the path policy check. + config = { allowedPaths: [tmpRoot] }; + registerOrgDescribe(server as never, config); +}); + +afterEach(() => { + fs.rmSync(tmpRoot, { recursive: true, force: true }); +}); + +// ── projectNameDashes / workspaceCandidates ─────────────────────────────────── + +describe('projectNameDashes', () => { + it('lowercases and replaces whitespace with single dashes', () => { + assert.equal(projectNameDashes('/x/My Project Path'), 'my-project-path'); + assert.equal(projectNameDashes('/x/ Spaced Name '), 'spaced-name'); + }); +}); + +describe('workspaceCandidates', () => { + it('returns three candidates in expected order', () => { + const cands = workspaceCandidates('/Users/alice/projects/My Project'); + assert.equal(cands.length, 3); + assert.ok( + cands[0].endsWith(`${path.sep}workspace-My Project`), + `Expected sibling workspace first, got: ${cands[0]}` + ); + assert.ok( + cands[1].includes('Provar_Workspaces') && cands[1].endsWith('workspace-my-project'), + `Expected Provar_Workspaces second, got: ${cands[1]}` + ); + assert.ok(cands[2].endsWith(`${path.sep}Provar${path.sep}workspace-my-project`)); + }); +}); + +// ── (a) Workspace discovery — sibling pattern ───────────────────────────────── + +describe('provar_org_describe — workspace discovery', () => { + it('(a) finds the sibling workspace at /workspace-', () => { + // /workspace-MyProject is the sibling pattern + const siblingWorkspace = path.join(tmpRoot, 'workspace-MyProject'); + const connectionDir = path.join(siblingWorkspace, '.metadata', 'MyOrg'); + writeJsonCache(connectionDir, 'Account', [{ name: 'Name', type: 'string', defaultValue: null, nillable: false }]); + + const result = server.call('provar_org_describe', { + project_path: projectPath, + connection_name: 'MyOrg', + }); + + assert.equal(isError(result), false); + const body = parseText(result); + assert.equal(body['workspace_path'], siblingWorkspace, 'should discover sibling workspace'); + const objects = body['objects'] as Array<{ name: string; exists: boolean | null; field_count: number }>; + assert.equal(objects.length, 1); + assert.equal(objects[0].name, 'Account'); + assert.equal(objects[0].exists, true); + assert.equal(objects[0].field_count, 1); + }); + + it('(b) falls back to user-home workspace when sibling missing (via override)', () => { + // Stand in for ~/Provar by using a HOME override. The tool uses os.homedir(), + // and we override $HOME for this test only. Set the home to a tmp dir so the + // path is inside allowed paths. + const fakeHome = path.join(tmpRoot, 'fakehome'); + fs.mkdirSync(fakeHome, { recursive: true }); + + const homeWorkspace = path.join(fakeHome, 'Provar', 'workspace-myproject'); + const connectionDir = path.join(homeWorkspace, '.metadata', 'MyOrg'); + writeJsonCache(connectionDir, 'Contact', [ + { name: 'LastName', type: 'string', defaultValue: null, nillable: false }, + ]); + + // Override HOME and USERPROFILE so os.homedir() returns fakeHome + const oldHome = process.env['HOME']; + const oldUserProfile = process.env['USERPROFILE']; + process.env['HOME'] = fakeHome; + process.env['USERPROFILE'] = fakeHome; + try { + const result = server.call('provar_org_describe', { + project_path: projectPath, + connection_name: 'MyOrg', + }); + assert.equal(isError(result), false); + const body = parseText(result); + assert.equal(body['workspace_path'], homeWorkspace, 'should discover user-home workspace'); + const objects = body['objects'] as Array<{ name: string; exists: boolean | null }>; + assert.ok( + objects.some((o) => o.name === 'Contact' && o.exists === true), + 'should list Contact from home workspace cache' + ); + } finally { + if (oldHome === undefined) delete process.env['HOME']; + else process.env['HOME'] = oldHome; + if (oldUserProfile === undefined) delete process.env['USERPROFILE']; + else process.env['USERPROFILE'] = oldUserProfile; + } + }); + + it('discoverWorkspace returns null when no candidate exists', () => { + assert.equal(discoverWorkspace(projectPath, [tmpRoot]), null); + }); + + it('discoverWorkspace skips candidates outside allowedPaths without touching the filesystem', () => { + // Create a sibling workspace that DOES exist on disk but lies outside the allowed root. + // We force this by creating a parallel tmp tree and only allowing tmpRoot. + // The sibling pattern resolves to /workspace-; the parent of + // projectPath is tmpRoot, so the sibling itself is inside the allowed set — + // which is what we want for the happy case. To exercise the policy skip we use + // a separate "outside" project path whose sibling candidate lives outside. + const outsideRoot = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'org-describe-outside-'))); + try { + const outsideProject = path.join(outsideRoot, 'OtherProject'); + fs.mkdirSync(outsideProject, { recursive: true }); + const outsideSibling = path.join(outsideRoot, 'workspace-OtherProject'); + fs.mkdirSync(outsideSibling, { recursive: true }); + + // With only tmpRoot allowed, discoverWorkspace MUST NOT return the outside sibling + // even though it exists on disk. + assert.equal(discoverWorkspace(outsideProject, [tmpRoot]), null); + } finally { + fs.rmSync(outsideRoot, { recursive: true, force: true }); + } + }); +}); + +// ── (c) Cache miss ──────────────────────────────────────────────────────────── + +describe('provar_org_describe — cache miss', () => { + it('(c) returns suggestion when workspace exists but .metadata/ absent', () => { + // Create the sibling workspace dir but NOT the connection subdir + const siblingWorkspace = path.join(tmpRoot, 'workspace-MyProject'); + fs.mkdirSync(path.join(siblingWorkspace, '.metadata'), { recursive: true }); + + const result = server.call('provar_org_describe', { + project_path: projectPath, + connection_name: 'MissingOrg', + objects: ['Account'], + }); + + assert.equal(isError(result), false); + const body = parseText(result); + assert.equal(body['workspace_path'], siblingWorkspace); + assert.equal(body['cache_age_ms'], null); + + const details = body['details'] as { suggestion: string }; + assert.ok(details, 'details should be present on cache miss'); + assert.ok( + details.suggestion.includes('Provar IDE') && details.suggestion.includes('MissingOrg'), + `suggestion should mention IDE and connection name; got: ${details.suggestion}` + ); + + const objects = body['objects'] as Array<{ name: string; exists: boolean | null; required_fields: unknown[] }>; + assert.equal(objects.length, 1); + assert.equal(objects[0].name, 'Account'); + assert.equal(objects[0].exists, null, 'exists must be null when cache missing entirely'); + }); + + it('returns suggestion when no workspace at all is discoverable', () => { + // No HOME override + no sibling workspace ⇒ workspace_path null. But os.homedir() + // will still produce a path; set HOME to a non-existent dir so the candidate doesn't exist. + const fakeHome = path.join(tmpRoot, 'nope'); + const oldHome = process.env['HOME']; + const oldUserProfile = process.env['USERPROFILE']; + process.env['HOME'] = fakeHome; + process.env['USERPROFILE'] = fakeHome; + try { + const result = server.call('provar_org_describe', { + project_path: projectPath, + connection_name: 'AnyOrg', + objects: ['Account', 'Contact'], + }); + assert.equal(isError(result), false); + const body = parseText(result); + assert.equal(body['workspace_path'], null); + assert.ok(body['details'], 'suggestion should be present'); + } finally { + if (oldHome === undefined) delete process.env['HOME']; + else process.env['HOME'] = oldHome; + if (oldUserProfile === undefined) delete process.env['USERPROFILE']; + else process.env['USERPROFILE'] = oldUserProfile; + } + }); +}); + +// ── (d) Path policy ─────────────────────────────────────────────────────────── + +describe('provar_org_describe — path policy', () => { + it('(d) rejects project_path outside allowed paths with PATH_NOT_ALLOWED', () => { + const strictServer = new MockMcpServer(); + registerOrgDescribe(strictServer as never, { allowedPaths: [tmpRoot] }); + + const result = strictServer.call('provar_org_describe', { + project_path: path.join(os.tmpdir(), 'definitely-outside'), + connection_name: 'MyOrg', + }); + + assert.equal(isError(result), true); + const code = parseText(result)['error_code'] as string; + assert.ok(code === 'PATH_NOT_ALLOWED' || code === 'PATH_TRAVERSAL', `Unexpected error_code: ${code}`); + }); + + it('rejects connection_name that would escape workspace dir via traversal', () => { + const siblingWorkspace = path.join(tmpRoot, 'workspace-MyProject'); + fs.mkdirSync(path.join(siblingWorkspace, '.metadata'), { recursive: true }); + + const result = server.call('provar_org_describe', { + project_path: projectPath, + connection_name: '../../escape', + }); + + assert.equal(isError(result), true); + const code = parseText(result)['error_code'] as string; + assert.ok(code === 'PATH_TRAVERSAL' || code === 'PATH_NOT_ALLOWED', `Unexpected error_code: ${code}`); + }); +}); + +// ── (e) Happy path ──────────────────────────────────────────────────────────── + +describe('provar_org_describe — happy path', () => { + it('(e) returns the expected shape for two cached objects', () => { + const siblingWorkspace = path.join(tmpRoot, 'workspace-MyProject'); + const connectionDir = path.join(siblingWorkspace, '.metadata', 'MyOrg'); + writeJsonCache(connectionDir, 'Account', [ + { name: 'Name', type: 'string', defaultValue: null, nillable: false }, + { name: 'AccountNumber', type: 'string', defaultValue: null, nillable: true }, + ]); + writeJsonCache(connectionDir, 'Contact', [ + { name: 'LastName', type: 'string', defaultValue: null, nillable: false }, + { name: 'Email', type: 'email', defaultValue: null, nillable: true }, + ]); + + const result = server.call('provar_org_describe', { + project_path: projectPath, + connection_name: 'MyOrg', + }); + + assert.equal(isError(result), false); + const body = parseText(result); + assert.equal(body['workspace_path'], siblingWorkspace); + assert.ok(typeof body['cache_age_ms'] === 'number' && body['cache_age_ms'] >= 0); + + const objects = body['objects'] as Array<{ + name: string; + exists: boolean | null; + required_fields: Array<{ name: string; nillable: boolean }>; + field_count: number; + }>; + assert.equal(objects.length, 2); + + const account = objects.find((o) => o.name === 'Account'); + assert.ok(account); + assert.equal(account.exists, true); + assert.equal(account.field_count, 2, 'field_count reports total cached fields, not filtered'); + // default field_filter is "required" → only nillable=false fields included + assert.equal(account.required_fields.length, 1); + assert.equal(account.required_fields[0].name, 'Name'); + }); +}); + +// ── (f) field_filter ────────────────────────────────────────────────────────── + +describe('provar_org_describe — field_filter', () => { + it('(f) field_filter=required excludes nillable fields', () => { + const siblingWorkspace = path.join(tmpRoot, 'workspace-MyProject'); + const connectionDir = path.join(siblingWorkspace, '.metadata', 'MyOrg'); + writeJsonCache(connectionDir, 'Account', [ + { name: 'Name', type: 'string', defaultValue: null, nillable: false }, + { name: 'Phone', type: 'phone', defaultValue: null, nillable: true }, + ]); + + const result = server.call('provar_org_describe', { + project_path: projectPath, + connection_name: 'MyOrg', + field_filter: 'required', + }); + + const body = parseText(result); + const objects = body['objects'] as Array<{ required_fields: Array<{ name: string }> }>; + const fields = objects[0].required_fields.map((f) => f.name); + assert.deepEqual(fields, ['Name'], 'only nillable=false fields should appear'); + }); + + it('(f) field_filter=all includes nillable fields', () => { + const siblingWorkspace = path.join(tmpRoot, 'workspace-MyProject'); + const connectionDir = path.join(siblingWorkspace, '.metadata', 'MyOrg'); + writeJsonCache(connectionDir, 'Account', [ + { name: 'Name', type: 'string', defaultValue: null, nillable: false }, + { name: 'Phone', type: 'phone', defaultValue: null, nillable: true }, + ]); + + const result = server.call('provar_org_describe', { + project_path: projectPath, + connection_name: 'MyOrg', + field_filter: 'all', + }); + + const body = parseText(result); + const objects = body['objects'] as Array<{ required_fields: Array<{ name: string }> }>; + const names = objects[0].required_fields.map((f) => f.name).sort(); + assert.deepEqual(names, ['Name', 'Phone']); + }); +}); + +// ── (g) objects filter ──────────────────────────────────────────────────────── + +describe('provar_org_describe — objects filter', () => { + it('(g) restricts response to requested object names only', () => { + const siblingWorkspace = path.join(tmpRoot, 'workspace-MyProject'); + const connectionDir = path.join(siblingWorkspace, '.metadata', 'MyOrg'); + writeJsonCache(connectionDir, 'Account', [{ name: 'Name', type: 'string', defaultValue: null, nillable: false }]); + writeJsonCache(connectionDir, 'Contact', [ + { name: 'LastName', type: 'string', defaultValue: null, nillable: false }, + ]); + writeJsonCache(connectionDir, 'Lead', [{ name: 'Company', type: 'string', defaultValue: null, nillable: false }]); + + const result = server.call('provar_org_describe', { + project_path: projectPath, + connection_name: 'MyOrg', + objects: ['Account', 'Lead'], + }); + + const body = parseText(result); + const objects = body['objects'] as Array<{ name: string }>; + const names = objects.map((o) => o.name).sort(); + assert.deepEqual(names, ['Account', 'Lead'], 'Contact should be excluded'); + }); + + it('reports exists=false for a requested object not present in cache', () => { + const siblingWorkspace = path.join(tmpRoot, 'workspace-MyProject'); + const connectionDir = path.join(siblingWorkspace, '.metadata', 'MyOrg'); + writeJsonCache(connectionDir, 'Account', [{ name: 'Name', type: 'string', defaultValue: null, nillable: false }]); + + const result = server.call('provar_org_describe', { + project_path: projectPath, + connection_name: 'MyOrg', + objects: ['Account', 'Ghost'], + }); + + const body = parseText(result); + const objects = body['objects'] as Array<{ name: string; exists: boolean | null }>; + const ghost = objects.find((o) => o.name === 'Ghost'); + assert.ok(ghost); + assert.equal(ghost.exists, false, 'object not in cache → exists=false'); + }); +}); + +// ── (h) Legacy cache formats — .xml / .object ──────────────────────────────── + +describe('provar_org_describe — legacy cache formats', () => { + it('(h.1) parses .xml CustomObject metadata and classifies required vs nillable correctly', () => { + // Regression guard for the required-flag bug: fast-xml-parser's default + // parseTagValue=true coerces "true" to the boolean true. + // The reader must treat boolean and string forms identically. + const siblingWorkspace = path.join(tmpRoot, 'workspace-MyProject'); + const connectionDir = path.join(siblingWorkspace, '.metadata', 'MyOrg'); + writeXmlCache( + connectionDir, + 'Account', + [ + { name: 'Name', type: 'string', required: true }, + { name: 'Phone', type: 'phone', required: false }, + ], + '.xml' + ); + + const result = server.call('provar_org_describe', { + project_path: projectPath, + connection_name: 'MyOrg', + objects: ['Account'], + field_filter: 'all', + }); + + assert.equal(isError(result), false); + const body = parseText(result); + const objects = body['objects'] as Array<{ + name: string; + exists: boolean | null; + required_fields: Array<{ name: string; nillable: boolean }>; + field_count: number; + }>; + assert.equal(objects.length, 1); + assert.equal(objects[0].exists, true); + assert.equal(objects[0].field_count, 2); + const byName = new Map(objects[0].required_fields.map((f) => [f.name, f.nillable])); + assert.equal( + byName.get('Name'), + false, + 'required field should have nillable=false (NOT misclassified as nillable)' + ); + assert.equal(byName.get('Phone'), true, 'non-required field should have nillable=true'); + }); + + it('(h.2) parses .object CustomObject metadata (legacy Eclipse layout)', () => { + const siblingWorkspace = path.join(tmpRoot, 'workspace-MyProject'); + const connectionDir = path.join(siblingWorkspace, '.metadata', 'MyOrg'); + writeXmlCache( + connectionDir, + 'Contact', + [ + { name: 'LastName', type: 'string', required: true }, + { name: 'Email', type: 'email', required: false }, + ], + '.object' + ); + + const result = server.call('provar_org_describe', { + project_path: projectPath, + connection_name: 'MyOrg', + objects: ['Contact'], + field_filter: 'required', + }); + + assert.equal(isError(result), false); + const body = parseText(result); + const objects = body['objects'] as Array<{ + name: string; + exists: boolean | null; + required_fields: Array<{ name: string }>; + field_count: number; + }>; + assert.equal(objects[0].exists, true); + assert.equal(objects[0].field_count, 2, 'field_count counts all parsed fields, regardless of filter'); + // field_filter='required' → only nillable=false survives + const names = objects[0].required_fields.map((f) => f.name); + assert.deepEqual(names, ['LastName'], 'only the required field should pass the filter'); + }); +}); + +// ── (i) Parse-error reporting ───────────────────────────────────────────────── + +describe('provar_org_describe — parse errors', () => { + it('(i) returns exists=true with error_message when a cache file is corrupt', () => { + // A cache file that EXISTS but does not parse must NOT be reported as exists=false + // (that would conflate "not cached" with "corrupt") — the contract is exists=true, + // field_count=0, error_message set. + const siblingWorkspace = path.join(tmpRoot, 'workspace-MyProject'); + const connectionDir = path.join(siblingWorkspace, '.metadata', 'MyOrg'); + fs.mkdirSync(connectionDir, { recursive: true }); + fs.writeFileSync(path.join(connectionDir, 'Account.json'), '{ not valid json', 'utf-8'); + + const result = server.call('provar_org_describe', { + project_path: projectPath, + connection_name: 'MyOrg', + objects: ['Account'], + }); + + assert.equal(isError(result), false); + const body = parseText(result); + const objects = body['objects'] as Array<{ + name: string; + exists: boolean | null; + field_count: number; + error_message?: string; + }>; + assert.equal(objects.length, 1); + assert.equal(objects[0].exists, true, 'corrupt cache file is "present" — not missing'); + assert.equal(objects[0].field_count, 0); + const errMsg = objects[0].error_message; + assert.ok(errMsg, 'error_message should describe the parse failure'); + assert.ok(errMsg.includes('Account.json'), `error_message should reference the file name; got: ${errMsg}`); + }); +}); + +// ── (j) Connection-name traversal — bare `..` ───────────────────────────────── + +describe('provar_org_describe — connection_name validation', () => { + it('(j) rejects bare ".." connection_name with PATH_TRAVERSAL and a clear message', () => { + // Regression guard for the broadened error message: the validator rejects `..` + // even when no separator is present. The message must mention BOTH conditions. + const siblingWorkspace = path.join(tmpRoot, 'workspace-MyProject'); + fs.mkdirSync(path.join(siblingWorkspace, '.metadata'), { recursive: true }); + + const result = server.call('provar_org_describe', { + project_path: projectPath, + connection_name: '..', + }); + + assert.equal(isError(result), true); + const body = parseText(result); + assert.equal(body['error_code'], 'PATH_TRAVERSAL'); + const msg = body['message'] as string; + assert.ok( + /path separators/i.test(msg) && msg.includes('..'), + `error message should mention both path separators and '..'; got: ${msg}` + ); + }); +}); diff --git a/test/unit/mcp/propertiesTools.test.ts b/test/unit/mcp/propertiesTools.test.ts index d405b084..442e0369 100644 --- a/test/unit/mcp/propertiesTools.test.ts +++ b/test/unit/mcp/propertiesTools.test.ts @@ -535,4 +535,107 @@ describe('provar_properties_validate', () => { 'Expected error on metadata.metadataLevel' ); }); + + // ── SCHEMA-001: unknown-key detection (PDX-486 VALIDATE-TYPO-A) ────────────── + + describe('SCHEMA-001 unknown-key detection', () => { + it('fires for unknown top-level key with did-you-mean suggestion within distance 2', () => { + const props = validProps(); + // Classic typo: testCases (plural) vs canonical testCase + props['testCases'] = ['tests/foo.testcase']; + + const result = server.call('provar_properties_validate', { content: JSON.stringify(props) }); + + const body = parseText(result); + const warnings = body['warnings'] as Array<{ field: string; message: string; severity: string }>; + const w = warnings.find((x) => x.field === 'testCases'); + assert.ok(w, 'Expected SCHEMA-001 warning for testCases'); + assert.ok(w.message.includes('SCHEMA-001'), 'Warning should reference SCHEMA-001'); + assert.ok(w.message.includes("Unknown field 'testCases'"), 'Warning should name the offending key'); + assert.ok(w.message.includes('top-level'), 'Warning should label the scope as top-level'); + assert.ok(w.message.includes("'testCase'"), `Expected "did you mean 'testCase'" suggestion, got: ${w.message}`); + }); + + it("fires for unknown metadata.* key (e.g. 'metadataLvel') with suggestion", () => { + const props = validProps(); + (props['metadata'] as Record)['metadataLvel'] = 'Reuse'; + + const result = server.call('provar_properties_validate', { content: JSON.stringify(props) }); + + const body = parseText(result); + const warnings = body['warnings'] as Array<{ field: string; message: string }>; + const w = warnings.find((x) => x.field === 'metadata.metadataLvel'); + assert.ok(w, 'Expected SCHEMA-001 warning for metadata.metadataLvel'); + assert.ok(w.message.includes('SCHEMA-001')); + assert.ok(w.message.includes('metadata'), 'Warning should label the scope as metadata'); + assert.ok(w.message.includes("'metadataLevel'"), `Expected suggestion, got: ${w.message}`); + }); + + it("fires for unknown environment.* key (e.g. 'testEnvironments' → testEnvironment)", () => { + const props = validProps(); + (props['environment'] as Record)['testEnvironments'] = 'QA'; + + const result = server.call('provar_properties_validate', { content: JSON.stringify(props) }); + + const body = parseText(result); + const warnings = body['warnings'] as Array<{ field: string; message: string }>; + const w = warnings.find((x) => x.field === 'environment.testEnvironments'); + assert.ok(w, 'Expected SCHEMA-001 warning for environment.testEnvironments'); + assert.ok(w.message.includes('SCHEMA-001')); + assert.ok(w.message.includes('environment')); + assert.ok(w.message.includes("'testEnvironment'"), `Expected testEnvironment suggestion, got: ${w.message}`); + }); + + it('does NOT fire SCHEMA-001 for known top-level / metadata / environment keys', () => { + const result = server.call('provar_properties_validate', { content: JSON.stringify(validProps()) }); + + const body = parseText(result); + const warnings = body['warnings'] as Array<{ message: string }>; + assert.ok( + warnings.every((w) => !w.message.includes('SCHEMA-001')), + `Expected zero SCHEMA-001 warnings on a valid file, got: ${JSON.stringify(warnings)}` + ); + }); + + it('emits SCHEMA-001 with no "Did you mean" when no canonical key is within distance 2', () => { + const props = validProps(); + props['totallyUnrelatedKey'] = 'x'; + + const result = server.call('provar_properties_validate', { content: JSON.stringify(props) }); + + const body = parseText(result); + const warnings = body['warnings'] as Array<{ field: string; message: string }>; + const w = warnings.find((x) => x.field === 'totallyUnrelatedKey'); + assert.ok(w, 'Expected SCHEMA-001 warning for totallyUnrelatedKey'); + assert.ok(w.message.includes('SCHEMA-001')); + assert.ok(!w.message.includes('Did you mean'), `Should not include suggestion, got: ${w.message}`); + }); + + it('is_valid stays true when the only issues are SCHEMA-001 warnings (no errors)', () => { + const props = validProps(); + props['testCases'] = ['x']; + + const result = server.call('provar_properties_validate', { content: JSON.stringify(props) }); + + const body = parseText(result); + assert.equal(body['is_valid'], true, 'Unknown keys are warnings only — is_valid must remain true'); + assert.equal(body['error_count'], 0); + assert.ok((body['warning_count'] as number) >= 1); + }); + + it("loads test/fixtures/properties/testcases-typo.json and reports SCHEMA-001 with 'testCase' suggestion", () => { + // Tests run from the repo root via wireit/yarn; resolve relative to cwd to avoid ESM __dirname. + const fixturePath = path.resolve(process.cwd(), 'test', 'fixtures', 'properties', 'testcases-typo.json'); + const content = fs.readFileSync(fixturePath, 'utf-8'); + + const result = server.call('provar_properties_validate', { content }); + + const body = parseText(result); + assert.equal(body['is_valid'], true, 'Fixture should pass structural validation (warnings only)'); + const warnings = body['warnings'] as Array<{ field: string; message: string }>; + const w = warnings.find((x) => x.field === 'testCases'); + assert.ok(w, 'Expected SCHEMA-001 warning for the testCases typo in the fixture'); + assert.ok(w.message.includes("Did you mean 'testCase'?"), `Expected suggestion, got: ${w.message}`); + }); + }); }); diff --git a/test/unit/mcp/rcaTools.test.ts b/test/unit/mcp/rcaTools.test.ts index 42ce4ec1..48f01ee9 100644 --- a/test/unit/mcp/rcaTools.test.ts +++ b/test/unit/mcp/rcaTools.test.ts @@ -717,3 +717,89 @@ describe('provar_testrun_rca mode=failures', () => { ); }); }); + +// ── PDX-490: error_category + retryable on failures[] ───────────────────────── + +describe('provar_testrun_rca error_category and retryable (PDX-490)', () => { + function junitWithFailure(testName: string, failureText: string): string { + return ` + + + + ${failureText} + + +`; + } + + function getFirstFailure(junit: string, name: string): Record { + const resultsDir = makeResultsDir(path.join(tmpDir, name), junit); + const body = parseText(server.call('provar_testrun_rca', { project_path: tmpDir, results_path: resultsDir })); + const failures = body['failures'] as Array>; + assert.equal(failures.length, 1, `expected exactly one failure in ${name}`); + return failures[0]; + } + + it('classifies "Connection reset" as INFRASTRUCTURE and retryable=true', () => { + const f = getFirstFailure(junitWithFailure('NetTest', 'Connection reset by peer'), 'cat-infra'); + assert.equal(f['error_category'], 'INFRASTRUCTURE'); + assert.equal(f['retryable'], true); + }); + + it('classifies "ECONNRESET" as INFRASTRUCTURE and retryable=true', () => { + const f = getFirstFailure(junitWithFailure('NetTest2', 'socket error: ECONNRESET while reading'), 'cat-econnreset'); + assert.equal(f['error_category'], 'INFRASTRUCTURE'); + assert.equal(f['retryable'], true); + }); + + it('classifies NoSuchElementException as LOCATOR and retryable=false', () => { + const f = getFirstFailure( + junitWithFailure('LocTest', 'NoSuchElementException: Unable to locate element #submit'), + 'cat-locator' + ); + assert.equal(f['error_category'], 'LOCATOR'); + assert.equal(f['retryable'], false); + }); + + it('classifies TimeoutException as TIMEOUT and retryable=true', () => { + const f = getFirstFailure( + junitWithFailure('TimeoutTest', 'TimeoutException: page did not load in 30s'), + 'cat-timeout' + ); + assert.equal(f['error_category'], 'TIMEOUT'); + assert.equal(f['retryable'], true); + }); + + it('classifies AssertionException as ASSERTION and retryable=false', () => { + const f = getFirstFailure( + junitWithFailure('AssertTest', 'AssertionException: expected "yes" but got "no"'), + 'cat-assertion' + ); + assert.equal(f['error_category'], 'ASSERTION'); + assert.equal(f['retryable'], false); + }); + + it('a plain "expected X but was Y" assertion text with no exception class is undefined (no match)', () => { + const f = getFirstFailure(junitWithFailure('PlainAssert', 'expected 5 but was 3'), 'cat-undef'); + // No known exception class → error_category should be undefined / absent and retryable absent. + assert.equal(f['error_category'], undefined); + assert.equal(f['retryable'], undefined); + }); + + it('classifies WebDriverException (matches errorClassPatterns but not primary buckets) as OTHER and retryable=false', () => { + const f = getFirstFailure(junitWithFailure('DriverTest', 'WebDriverException: chrome not reachable'), 'cat-other'); + assert.equal(f['error_category'], 'OTHER'); + assert.equal(f['retryable'], false); + }); + + it('error_class still populated alongside the new fields (additive — no breaking change)', () => { + const f = getFirstFailure(junitWithFailure('LocTest2', 'NoSuchElementException: missing button'), 'cat-additive'); + assert.equal(f['error_class'], 'NoSuchElementException'); + assert.equal(f['error_category'], 'LOCATOR'); + assert.equal(f['retryable'], false); + // Existing fields must remain intact: + assert.equal(f['root_cause_category'], 'LOCATOR_STALE'); + assert.ok(typeof f['error_message'] === 'string'); + assert.equal(f['pre_existing'], false); + }); +}); diff --git a/test/unit/mcp/testCaseGenerate.test.ts b/test/unit/mcp/testCaseGenerate.test.ts index 95c7579b..7c286e1a 100644 --- a/test/unit/mcp/testCaseGenerate.test.ts +++ b/test/unit/mcp/testCaseGenerate.test.ts @@ -11,7 +11,7 @@ import fs from 'node:fs'; import os from 'node:os'; import path from 'node:path'; import { describe, it, beforeEach, afterEach } from 'mocha'; -import { registerTestCaseGenerate } from '../../../src/mcp/tools/testCaseGenerate.js'; +import { registerTestCaseGenerate, inferSalesforceValueClass } from '../../../src/mcp/tools/testCaseGenerate.js'; import type { ServerConfig } from '../../../src/mcp/server.js'; // ── Minimal McpServer mock ───────────────────────────────────────────────────── @@ -699,6 +699,231 @@ describe('provar_testcase_generate', () => { }); }); + // PDX-493 (H3): date/datetime/boolean/decimal valueClass dispatch via inferSalesforceValueClass. + // Numbers emit `valueClass="decimal"` per canonical reference (PROVAR_TEST_STEP_REFERENCE.md + // lines 1338, 1428) — there is no `integer` valueClass in the Provar grammar. + describe('PDX-493 — inferSalesforceValueClass helper', () => { + it('returns "datetime" for ISO-8601 datetime string', () => { + assert.equal(inferSalesforceValueClass('CloseDate', '2026-05-19T10:30:00'), 'datetime'); + }); + + it('returns "datetime" for ISO-8601 datetime with fractional seconds + zone', () => { + assert.equal(inferSalesforceValueClass('CloseDate', '2026-05-19T10:30:00.123Z'), 'datetime'); + }); + + it('returns "datetime" for ISO-8601 datetime with numeric timezone offset', () => { + assert.equal(inferSalesforceValueClass('CloseDate', '2026-05-19T10:30:00+05:30'), 'datetime'); + assert.equal(inferSalesforceValueClass('CloseDate', '2026-05-19T10:30:00-0800'), 'datetime'); + }); + + it('returns "string" for datetime-looking values with trailing garbage (end-anchored)', () => { + // Guards against the un-anchored regex bug: trailing junk after seconds must not + // be silently accepted as datetime. + assert.equal(inferSalesforceValueClass('CloseDate', '2026-05-19T10:30:00not-a-zone'), 'string'); + }); + + it('returns "date" for ISO-8601 date string', () => { + assert.equal(inferSalesforceValueClass('CloseDate', '2026-05-19'), 'date'); + }); + + it('returns "boolean" for "true"', () => { + assert.equal(inferSalesforceValueClass('IsActive', 'true'), 'boolean'); + }); + + it('returns "boolean" for "false"', () => { + assert.equal(inferSalesforceValueClass('IsActive', 'false'), 'boolean'); + }); + + it('returns "decimal" for positive integer string', () => { + assert.equal(inferSalesforceValueClass('Quantity', '42'), 'decimal'); + }); + + it('returns "decimal" for negative integer string', () => { + assert.equal(inferSalesforceValueClass('Delta', '-5'), 'decimal'); + }); + + it('returns "decimal" for positive decimal string', () => { + assert.equal(inferSalesforceValueClass('Amount', '3.14'), 'decimal'); + }); + + it('returns "decimal" for negative decimal string', () => { + assert.equal(inferSalesforceValueClass('Adjustment', '-12.5'), 'decimal'); + }); + + it('returns "string" for plain text', () => { + assert.equal(inferSalesforceValueClass('Name', 'Acme Corp'), 'string'); + }); + + it('returns "decimal" not "date" for a short numeric string like "12"', () => { + // Edge case: the date regex requires the full ISO yyyy-mm-dd form, so a bare "12" + // is decimal, not date. Guards against false-positive date detection on numeric IDs. + assert.equal(inferSalesforceValueClass('Code', '12'), 'decimal'); + }); + + it('returns "string" for date-looking strings that miss the ISO format', () => { + // Confirms the regex is strict: month/day shape matters. + assert.equal(inferSalesforceValueClass('CloseDate', '2026/05/19'), 'string'); + assert.equal(inferSalesforceValueClass('CloseDate', '05-19-2026'), 'string'); + }); + + it('explicit fieldTypeHint wins over format detection', () => { + // Value looks like a string but the hint says it's a date — hint wins. + assert.equal(inferSalesforceValueClass('CloseDate', 'today', 'date'), 'date'); + // Value looks like a date but the hint says string — hint wins (e.g. an external + // ID that happens to look like a date). + assert.equal(inferSalesforceValueClass('ExternalId', '2026-05-19', 'string'), 'string'); + // Value is decimal, hint says boolean — hint wins. + assert.equal(inferSalesforceValueClass('IsActive', '1', 'boolean'), 'boolean'); + }); + }); + + describe('PDX-493 — valueClass emission in generated XML', () => { + it('emits valueClass="date" for an ISO-8601 date string', () => { + const result = server.call('provar_testcase_generate', { + test_case_name: 'DateField', + steps: [ + { + api_id: 'ApexCreateObject', + name: 'Create Opp', + attributes: { CloseDate: '2026-05-19' }, + }, + ], + dry_run: true, + overwrite: false, + }); + + const xml = parseText(result)['xml_content'] as string; + assert.ok( + xml.includes('valueClass="date">2026-05-19'), + `Expected valueClass="date" for ISO date; got: ${xml}` + ); + }); + + it('emits valueClass="datetime" for an ISO-8601 datetime string', () => { + const result = server.call('provar_testcase_generate', { + test_case_name: 'DatetimeField', + steps: [ + { + api_id: 'ApexCreateObject', + name: 'Create Event', + attributes: { StartTime: '2026-05-19T10:30:00' }, + }, + ], + dry_run: true, + overwrite: false, + }); + + const xml = parseText(result)['xml_content'] as string; + assert.ok( + xml.includes('valueClass="datetime">2026-05-19T10:30:00'), + `Expected valueClass="datetime" for ISO datetime; got: ${xml}` + ); + }); + + it('emits valueClass="boolean" for "true" / "false" literals', () => { + const result = server.call('provar_testcase_generate', { + test_case_name: 'BoolField', + steps: [ + { + api_id: 'ApexCreateObject', + name: 'Create Account', + attributes: { IsActive: 'true', IsDeleted: 'false' }, + }, + ], + dry_run: true, + overwrite: false, + }); + + const xml = parseText(result)['xml_content'] as string; + assert.ok( + xml.includes('valueClass="boolean">true'), + `Expected valueClass="boolean" for "true"; got: ${xml}` + ); + assert.ok( + xml.includes('valueClass="boolean">false'), + `Expected valueClass="boolean" for "false"; got: ${xml}` + ); + }); + + it('emits valueClass="decimal" for an integer-only string', () => { + const result = server.call('provar_testcase_generate', { + test_case_name: 'IntField', + steps: [ + { + api_id: 'ApexCreateObject', + name: 'Create Opp', + attributes: { Quantity: '42' }, + }, + ], + dry_run: true, + overwrite: false, + }); + + const xml = parseText(result)['xml_content'] as string; + assert.ok(xml.includes('valueClass="decimal">42'), `Expected valueClass="decimal" for "42"; got: ${xml}`); + }); + + it('emits valueClass="decimal" for a negative integer string', () => { + const result = server.call('provar_testcase_generate', { + test_case_name: 'NegIntField', + steps: [ + { + api_id: 'ApexCreateObject', + name: 'Create Adjustment', + attributes: { Delta: '-5' }, + }, + ], + dry_run: true, + overwrite: false, + }); + + const xml = parseText(result)['xml_content'] as string; + assert.ok(xml.includes('valueClass="decimal">-5'), `Expected valueClass="decimal" for "-5"; got: ${xml}`); + }); + + it('emits valueClass="decimal" for a positive decimal string', () => { + const result = server.call('provar_testcase_generate', { + test_case_name: 'DecimalField', + steps: [ + { + api_id: 'ApexCreateObject', + name: 'Create Opp', + attributes: { Amount: '3.14' }, + }, + ], + dry_run: true, + overwrite: false, + }); + + const xml = parseText(result)['xml_content'] as string; + assert.ok( + xml.includes('valueClass="decimal">3.14'), + `Expected valueClass="decimal" for "3.14"; got: ${xml}` + ); + }); + + it('emits valueClass="string" for plain text', () => { + const result = server.call('provar_testcase_generate', { + test_case_name: 'StringField', + steps: [ + { + api_id: 'ApexCreateObject', + name: 'Create Account', + attributes: { Name: 'Acme Corp' }, + }, + ], + dry_run: true, + overwrite: false, + }); + + const xml = parseText(result)['xml_content'] as string; + assert.ok( + xml.includes('valueClass="string">Acme Corp'), + `Expected valueClass="string" for "Acme Corp"; got: ${xml}` + ); + }); + }); + describe('target_uri — non-SF page object (ui:) nesting', () => { it('wraps steps in UiWithScreen when target_uri uses ?pageId= format', () => { const result = server.call('provar_testcase_generate', { diff --git a/test/unit/mcp/testCasePlanMode.test.ts b/test/unit/mcp/testCasePlanMode.test.ts new file mode 100644 index 00000000..1dcdc328 --- /dev/null +++ b/test/unit/mcp/testCasePlanMode.test.ts @@ -0,0 +1,231 @@ +/* + * Copyright (c) 2024 Provar Limited. + * All rights reserved. + * Licensed under the BSD 3-Clause license. + * For full license text, see LICENSE.md file in the repo root or https://opensource.org/licenses/BSD-3-Clause + */ + +/* eslint-disable camelcase */ +import { strict as assert } from 'node:assert'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { describe, it, beforeEach, afterEach } from 'mocha'; +import { resolveTestCasePlanMode } from '../../../src/mcp/utils/testCasePlanMode.js'; + +/** + * Build a minimal Provar project on disk for plan-mode resolution tests. + * + * Layout: `/tests/Module/MyTest.testcase`, + * `/plans/Plan1/Suite1/MyTest.testinstance` (only when + * `wirePlan=true`), and `/provardx-properties.json`. + * + * Returns the test-case path and the properties-file path so callers can + * point the resolver at the right inputs. + */ +function buildProject(opts: { + root: string; + references: 'direct-testCase' | 'direct-testCases' | 'none'; + wirePlan: boolean; +}): { testCasePath: string; propertiesPath: string; projectPath: string } { + const projectPath = path.join(opts.root, 'project'); + fs.mkdirSync(path.join(projectPath, 'tests', 'Module'), { recursive: true }); + const testCasePath = path.join(projectPath, 'tests', 'Module', 'MyTest.testcase'); + fs.writeFileSync(testCasePath, ''); + + if (opts.wirePlan) { + fs.mkdirSync(path.join(projectPath, 'plans', 'Plan1', 'Suite1'), { recursive: true }); + const inst = path.join(projectPath, 'plans', 'Plan1', 'Suite1', 'MyTest.testinstance'); + fs.writeFileSync(inst, ''); + } + + const props: Record = { + projectPath, + provarHome: '/tmp/provarHome', + resultsPath: 'ANT/Results', + }; + if (opts.references === 'direct-testCase') { + props.testCase = ['Module/MyTest.testcase']; + } else if (opts.references === 'direct-testCases') { + props.testCases = ['Module/MyTest.testcase']; + } + + const propertiesPath = path.join(projectPath, 'provardx-properties.json'); + fs.writeFileSync(propertiesPath, JSON.stringify(props, null, 2)); + return { testCasePath, propertiesPath, projectPath }; +} + +describe('resolveTestCasePlanMode', () => { + let tmp: string; + + beforeEach(() => { + // Realpath the tmp root immediately so every derived path uses the canonical form. + // Required on macOS where os.tmpdir() returns /var/folders/... but realpathSync + // canonicalises through the /var → /private/var symlink. The resolver under test + // now calls fs.realpathSync internally, so comparisons against constructed paths + // would otherwise diverge from the resolved result on Mac. + tmp = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'pdx489-mode-'))); + }); + + afterEach(() => { + fs.rmSync(tmp, { recursive: true, force: true }); + }); + + it('returns "direct" when properties references via top-level testCase', () => { + const { testCasePath, propertiesPath, projectPath } = buildProject({ + root: tmp, + references: 'direct-testCase', + wirePlan: false, + }); + const result = resolveTestCasePlanMode({ + testCaseFilePath: testCasePath, + allowedPaths: [tmp], + propertiesFilePathOverride: propertiesPath, + }); + assert.equal(result.mode, 'direct'); + assert.equal(result.propertiesFilePath, propertiesPath); + assert.equal(result.projectPath, projectPath); + }); + + it('returns "direct" when properties references via testCases (plural)', () => { + const { testCasePath, propertiesPath } = buildProject({ + root: tmp, + references: 'direct-testCases', + wirePlan: false, + }); + const result = resolveTestCasePlanMode({ + testCaseFilePath: testCasePath, + allowedPaths: [tmp], + propertiesFilePathOverride: propertiesPath, + }); + assert.equal(result.mode, 'direct'); + }); + + it('returns "plan" when a .testinstance references the test case (even if testCase array also lists it)', () => { + const { testCasePath, propertiesPath } = buildProject({ + root: tmp, + references: 'direct-testCase', + wirePlan: true, + }); + const result = resolveTestCasePlanMode({ + testCaseFilePath: testCasePath, + allowedPaths: [tmp], + propertiesFilePathOverride: propertiesPath, + }); + assert.equal(result.mode, 'plan', 'Plan-instance reference must win over direct testCase array'); + }); + + it('returns "plan" when only a .testinstance references it', () => { + const { testCasePath, propertiesPath } = buildProject({ + root: tmp, + references: 'none', + wirePlan: true, + }); + const result = resolveTestCasePlanMode({ + testCaseFilePath: testCasePath, + allowedPaths: [tmp], + propertiesFilePathOverride: propertiesPath, + }); + assert.equal(result.mode, 'plan'); + }); + + it('returns "unknown" when neither testCase nor a .testinstance references it', () => { + const { testCasePath, propertiesPath } = buildProject({ + root: tmp, + references: 'none', + wirePlan: false, + }); + const result = resolveTestCasePlanMode({ + testCaseFilePath: testCasePath, + allowedPaths: [tmp], + propertiesFilePathOverride: propertiesPath, + }); + assert.equal(result.mode, 'unknown'); + }); + + it('returns "unknown" when the properties file does not exist', () => { + const result = resolveTestCasePlanMode({ + testCaseFilePath: path.join(tmp, 'nope.testcase'), + allowedPaths: [tmp], + propertiesFilePathOverride: path.join(tmp, 'missing.json'), + }); + // Override is honoured but fs read fails → 'unknown' + assert.equal(result.mode, 'unknown'); + }); + + it('returns "unknown" when sf config has no PROVARDX_PROPERTIES_FILE_PATH entry', () => { + const sfDir = path.join(tmp, 'sf'); + fs.mkdirSync(sfDir); + fs.writeFileSync(path.join(sfDir, 'config.json'), JSON.stringify({})); + const result = resolveTestCasePlanMode({ + testCaseFilePath: path.join(tmp, 'nope.testcase'), + allowedPaths: [tmp], + sfConfigPathOverride: path.join(sfDir, 'config.json'), + }); + assert.equal(result.mode, 'unknown'); + }); + + it('returns "unknown" when properties file is outside allowed paths', () => { + const { testCasePath, propertiesPath } = buildProject({ + root: tmp, + references: 'direct-testCase', + wirePlan: false, + }); + // Allowed paths intentionally does NOT include the properties path. + const result = resolveTestCasePlanMode({ + testCaseFilePath: testCasePath, + allowedPaths: [path.join(tmp, 'unrelated-dir')], + propertiesFilePathOverride: propertiesPath, + }); + assert.equal(result.mode, 'unknown'); + }); + + it('honours assertPathAllowed on propertiesFilePathOverride: override inside allowed paths is consulted', () => { + // Security regression guard: a properties-file override path must pass the + // path-policy check before the resolver opens it. When the override sits + // inside the allowed-paths tree the helper should consult it normally and + // return the resolved mode (here: "direct"). + const { testCasePath, propertiesPath, projectPath } = buildProject({ + root: tmp, + references: 'direct-testCase', + wirePlan: false, + }); + const result = resolveTestCasePlanMode({ + testCaseFilePath: testCasePath, + allowedPaths: [tmp], + propertiesFilePathOverride: propertiesPath, + }); + assert.equal(result.mode, 'direct'); + assert.equal(result.projectPath, projectPath); + }); + + it('honours assertPathAllowed on propertiesFilePathOverride: override outside allowed paths is ignored without throwing', () => { + // Security regression guard for the Copilot-flagged path-policy bypass: + // the override must be funnelled through assertPathAllowed and a + // violation must collapse to 'unknown' rather than reading the file or + // throwing — the helper's contract is best-effort silent fallback. + const outsideRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'pdx489-outside-')); + try { + const { testCasePath } = buildProject({ + root: tmp, + references: 'direct-testCase', + wirePlan: false, + }); + // Write a properties file OUTSIDE the allowed-paths tree. + const outsideProps = path.join(outsideRoot, 'provardx-properties.json'); + fs.writeFileSync( + outsideProps, + JSON.stringify({ projectPath: outsideRoot, provarHome: '/tmp', resultsPath: 'r' }) + ); + const result = resolveTestCasePlanMode({ + testCaseFilePath: testCasePath, + allowedPaths: [tmp], + propertiesFilePathOverride: outsideProps, + }); + assert.equal(result.mode, 'unknown'); + assert.equal(result.propertiesFilePath, undefined, 'must not expose a path it rejected'); + } finally { + fs.rmSync(outsideRoot, { recursive: true, force: true }); + } + }); +}); diff --git a/test/unit/mcp/testCaseValidate.test.ts b/test/unit/mcp/testCaseValidate.test.ts index 9ff5bd2e..ad7a1391 100644 --- a/test/unit/mcp/testCaseValidate.test.ts +++ b/test/unit/mcp/testCaseValidate.test.ts @@ -226,16 +226,16 @@ describe('validateTestCase', () => { }); describe('DATA-001', () => { - it('warns when testCase has a child element', () => { - const r = validateTestCase( - ` + const DATA_TABLE_TC = ` mydata.csv -` - ); +`; + + it('warns (structural) when testCase has a child element and mode is unknown', () => { + const r = validateTestCase(DATA_TABLE_TC); assert.ok( r.issues.some((i) => i.rule_id === 'DATA-001'), 'Expected DATA-001' @@ -248,6 +248,42 @@ describe('validateTestCase', () => { const r = validateTestCase(VALID_TC); assert.ok(!r.issues.some((i) => i.rule_id === 'DATA-001'), 'DATA-001 should not fire for valid test case'); }); + + it('PDX-489: emits DATA-001 with formatWarning text when planMode is direct', () => { + const r = validateTestCase(DATA_TABLE_TC, undefined, { planMode: 'direct' }); + const issue = r.issues.find((i) => i.rule_id === 'DATA-001'); + assert.ok(issue, 'Expected DATA-001 to fire in direct mode'); + assert.ok( + issue.message.startsWith('WARNING [DATA-001]:'), + `Message must use formatWarning prefix, got: ${issue.message}` + ); + assert.ok( + issue.message.includes('only iterates when run through a test plan instance'), + `Message must reference the plan-instance fix: ${issue.message}` + ); + assert.ok( + issue.message.includes('provar_testplan_add-instance'), + `Message must reference provar_testplan_add-instance: ${issue.message}` + ); + }); + + it('PDX-489: suppresses DATA-001 when planMode is plan', () => { + const r = validateTestCase(DATA_TABLE_TC, undefined, { planMode: 'plan' }); + assert.ok( + !r.issues.some((i) => i.rule_id === 'DATA-001'), + 'DATA-001 must not fire when the test case is referenced from a .testinstance' + ); + }); + + it('PDX-489: keeps structural DATA-001 when planMode is unknown', () => { + const r = validateTestCase(DATA_TABLE_TC, undefined, { planMode: 'unknown' }); + const issue = r.issues.find((i) => i.rule_id === 'DATA-001'); + assert.ok(issue, 'Expected DATA-001 to fire in unknown mode'); + assert.ok( + !issue.message.startsWith('WARNING [DATA-001]:'), + 'Unknown-mode message should NOT use the formatWarning prefix (preserves prior behaviour)' + ); + }); }); describe('ASSERT-001', () => { @@ -1140,6 +1176,117 @@ describe('validateTestCaseXml', () => { }); }); +// ── PDX-489 handler-level DATA-001 integration ──────────────────────────────── + +/** + * Build a project that wires a dataTable test case directly (via testCase / + * testCases array) OR through a .testinstance — and writes ~/.sf/config.json + * pointing at the project's provardx-properties.json so the handler can + * resolve plan mode without explicit overrides. + */ +function buildDataTableProject( + tmpRoot: string, + references: 'direct-testCase' | 'direct-testCases' | 'plan' +): { testCasePath: string; allowedPaths: string[] } { + const projectPath = path.join(tmpRoot, 'project'); + fs.mkdirSync(path.join(projectPath, 'tests', 'Module'), { recursive: true }); + const testCasePath = path.join(projectPath, 'tests', 'Module', 'DataTest.testcase'); + fs.writeFileSync( + testCasePath, + ` + + mydata.csv + + + +` + ); + + const props: Record = { + projectPath, + provarHome: '/tmp/provarHome', + resultsPath: 'ANT/Results', + }; + + if (references === 'direct-testCase') { + props.testCase = ['Module/DataTest.testcase']; + } else if (references === 'direct-testCases') { + props.testCases = ['Module/DataTest.testcase']; + } else { + fs.mkdirSync(path.join(projectPath, 'plans', 'Plan1', 'Suite1'), { recursive: true }); + fs.writeFileSync( + path.join(projectPath, 'plans', 'Plan1', 'Suite1', 'DataTest.testinstance'), + '' + ); + } + + const propertiesPath = path.join(projectPath, 'provardx-properties.json'); + fs.writeFileSync(propertiesPath, JSON.stringify(props, null, 2)); + + // Wire ~/.sf/config.json so the resolver picks up the properties file. + const sfDir = path.join(tmpRoot, '.sf'); + fs.mkdirSync(sfDir, { recursive: true }); + fs.writeFileSync(path.join(sfDir, 'config.json'), JSON.stringify({ PROVARDX_PROPERTIES_FILE_PATH: propertiesPath })); + + return { testCasePath, allowedPaths: [tmpRoot] }; +} + +describe('PDX-489: DATA-001 handler integration via validateTestCaseXml', () => { + let tmpRoot: string; + let origHomedir: () => string; + + beforeEach(() => { + tmpRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'pdx489-handler-')); + origHomedir = os.homedir; + (os as unknown as { homedir: () => string }).homedir = (): string => tmpRoot; + }); + + afterEach(() => { + (os as unknown as { homedir: () => string }).homedir = origHomedir; + fs.rmSync(tmpRoot, { recursive: true, force: true }); + }); + + it('fires DATA-001 with formatWarning prefix when properties uses top-level testCase', () => { + const { testCasePath, allowedPaths } = buildDataTableProject(tmpRoot, 'direct-testCase'); + const result = validateTestCaseXml(testCasePath, { allowedPaths } as unknown as ServerConfig); + const issue = result.issues.find((i) => i.rule_id === 'DATA-001'); + assert.ok(issue, 'Expected DATA-001 in direct testCase mode'); + assert.ok(issue.message.startsWith('WARNING [DATA-001]:'), `Expected formatWarning prefix: ${issue.message}`); + assert.ok( + issue.message.includes('provar_testplan_add-instance'), + `Expected guidance to plan-instance: ${issue.message}` + ); + }); + + it('fires DATA-001 when properties uses testCases (plural) — typo-tolerant', () => { + const { testCasePath, allowedPaths } = buildDataTableProject(tmpRoot, 'direct-testCases'); + const result = validateTestCaseXml(testCasePath, { allowedPaths } as unknown as ServerConfig); + const issue = result.issues.find((i) => i.rule_id === 'DATA-001'); + assert.ok(issue, 'Expected DATA-001 when testCases array references the test'); + assert.ok(issue.message.startsWith('WARNING [DATA-001]:')); + }); + + it('does NOT fire DATA-001 when test case is referenced from a .testinstance', () => { + const { testCasePath, allowedPaths } = buildDataTableProject(tmpRoot, 'plan'); + const result = validateTestCaseXml(testCasePath, { allowedPaths } as unknown as ServerConfig); + assert.ok( + !result.issues.some((i) => i.rule_id === 'DATA-001'), + 'DATA-001 must not fire when the test case is wired via a plan instance' + ); + }); + + it('does NOT fire DATA-001 when test case has no (direct mode)', () => { + // Reuse the direct project but rewrite the test case content without a . + const { testCasePath, allowedPaths } = buildDataTableProject(tmpRoot, 'direct-testCase'); + fs.writeFileSync(testCasePath, VALID_TC); + const result = validateTestCaseXml(testCasePath, { allowedPaths } as unknown as ServerConfig); + assert.ok( + !result.issues.some((i) => i.rule_id === 'DATA-001'), + 'DATA-001 must not fire when no is present' + ); + }); +}); + // ── tool description ────────────────────────────────────────────────────────── describe('provar_testcase_validate description', () => { diff --git a/test/unit/mcp/utils/warningCodes.test.ts b/test/unit/mcp/utils/warningCodes.test.ts new file mode 100644 index 00000000..3ae7ff07 --- /dev/null +++ b/test/unit/mcp/utils/warningCodes.test.ts @@ -0,0 +1,64 @@ +/* + * Copyright (c) 2024 Provar Limited. + * All rights reserved. + * Licensed under the BSD 3-Clause license. + * For full license text, see LICENSE.md file in the repo root or https://opensource.org/licenses/BSD-3-Clause + */ + +import { strict as assert } from 'node:assert'; +import { describe, it } from 'mocha'; +import { WARNING_CODES, formatWarning } from '../../../../src/mcp/utils/warningCodes.js'; + +describe('WARNING_CODES', () => { + const expected: Record = { + PROVARHOME_001: 'PROVARHOME-001', + DATA_001: 'DATA-001', + PARALLEL_001: 'PARALLEL-001', + SCHEMA_001: 'SCHEMA-001', + RUN_001: 'RUN-001', + JUNIT_001: 'JUNIT-001', + }; + + it('maps each key to its expected wire string', () => { + for (const [key, value] of Object.entries(expected)) { + assert.equal( + (WARNING_CODES as Record)[key], + value, + `WARNING_CODES.${key} should equal '${value}'` + ); + } + }); + + // Guards against silent enum drift: a new code added without updating this test, + // or an accidentally-removed code, must fail the build. + it('contains exactly the expected key set (no additions, no omissions)', () => { + assert.deepEqual(Object.keys(WARNING_CODES).sort(), Object.keys(expected).sort()); + }); + + it('contains exactly the expected value set (no additions, no omissions)', () => { + assert.deepEqual(Object.values(WARNING_CODES).sort(), Object.values(expected).sort()); + }); +}); + +describe('formatWarning', () => { + it('returns the prefixed message exactly when no suggestion is provided', () => { + assert.equal( + formatWarning(WARNING_CODES.PROVARHOME_001, 'provarHome is missing'), + 'WARNING [PROVARHOME-001]: provarHome is missing' + ); + }); + + it('appends the "Did you mean" suffix exactly when a suggestion is provided', () => { + assert.equal( + formatWarning(WARNING_CODES.SCHEMA_001, "unknown key 'parralelMode'", 'parallelMode'), + "WARNING [SCHEMA-001]: unknown key 'parralelMode' Did you mean 'parallelMode'?" + ); + }); + + it('omits the suffix when suggestion is an empty string', () => { + assert.equal( + formatWarning(WARNING_CODES.DATA_001, 'data-table iteration not bound', ''), + 'WARNING [DATA-001]: data-table iteration not bound' + ); + }); +});