diff --git a/README.md b/README.md index 43469cb..a45a9a9 100644 --- a/README.md +++ b/README.md @@ -100,7 +100,7 @@ sf provar auth login claude mcp add provar -s user -- sf provar mcp start --allowed-paths /path/to/your/provar/project ``` -πŸ“– **[docs/mcp.md](https://github.com/ProvarTesting/provardx-cli/blob/main/docs/mcp.md) β€” full setup, all 42 tools, 6 resources, 11 MCP prompts, troubleshooting.** +πŸ“– **[docs/mcp.md](https://github.com/ProvarTesting/provardx-cli/blob/main/docs/mcp.md) β€” full setup, all 43 tools, 6 resources, 11 MCP prompts, troubleshooting.** --- @@ -251,7 +251,7 @@ DESCRIPTION Note: --json is not available on this command β€” stdout is reserved for MCP traffic. TOOLS EXPOSED - 42 tools across: project inspection & org describe, Page Object and test-case + 43 tools across: project inspection & org describe, Page Object and test-case authoring/validation, test-suite/plan validation, properties files, Quality Hub (test runs, defects, corpus examples), Provar Automation, ANT build, and NitroX components. See docs/mcp.md for the full catalogue with schemas and examples. diff --git a/docs/PROVAR_TEST_STEP_REFERENCE.md b/docs/PROVAR_TEST_STEP_REFERENCE.md index db26579..d69d94c 100644 --- a/docs/PROVAR_TEST_STEP_REFERENCE.md +++ b/docs/PROVAR_TEST_STEP_REFERENCE.md @@ -361,11 +361,20 @@ Valid `action` values: `ObjectHome` | `New` | `View` | `Edit` | `Delete` | `Clon Performs a single UI interaction on a field or button. The `interaction` URI determines the action type. -**Interaction types:** +**Interaction types (a common subset β€” the vocabulary is open, NOT limited to these):** -- `ui:interaction?name=action` β€” click a button or link +- `ui:interaction?name=action` β€” activate a button or link - `ui:interaction?name=set` β€” fill/type into a field or select a picklist value +- `ui:interaction?name=click` β€” click a control; distinct from `action`, and both are valid - `ui:interaction?name=file` β€” upload a file (uses `fileLocation` instead of `value`) +- `ui:interaction?name=check` / `name=uncheck` β€” tick or untick a checkbox +- `ui:interaction?name=doubleClick`, `name=hover`, `name=invoke`, `name=clear`, `name=toggle`, `name=sfLookup`, `name=sfSelect` β€” long-tail interactions the IDE recorder emits + +> A corpus survey of real Provar projects found **39 distinct interaction names** in +> well-formed `uiInteraction` nodes. Treat the list above as examples, not an +> allow-list. Validation rejects only names borrowed from other frameworks +> (`type`, `fill`, `enter`, `input`, `tap`, `press`): use `set` to type into a field, +> and `action` or `click` to activate a control. **Locator URI format:** `ui:locator?name=FIELD_OR_BUTTON_NAME&binding=ENCODED_BINDING` diff --git a/docs/VALIDATION_RULE_REGISTRY.md b/docs/VALIDATION_RULE_REGISTRY.md index 2269776..3e7a5a8 100644 --- a/docs/VALIDATION_RULE_REGISTRY.md +++ b/docs/VALIDATION_RULE_REGISTRY.md @@ -11,35 +11,38 @@ Provar test-case validation runs in two layers. This registry is the single cano **The validity bridge (PDX-509):** a `critical` best-practice violation is surfaced into `issues[]` as an `ERROR` and therefore gates `is_valid` β€” EXCEPT where a Layer-1 check already owns the concept (then it is suppressed to avoid double-reporting). `major`/`minor`/`info` affect `quality_score` (and the `needs_improvement` status) only. The `status` field is tri-state: `invalid` (a critical) / `needs_improvement` (loads but `quality_score < quality_threshold`) / `valid`. -**Counts:** Layer 1 β€” 23 rules (18 gating). Layer 2 β€” 179 rules (critical 64 / major 68 / minor 29 / info 18; 58 bridged to `is_valid`). +**Counts:** Layer 1 β€” 26 rules (19 gating). Layer 2 β€” 182 rules (critical 64 / major 69 / minor 30 / info 19; 58 bridged to `is_valid`). ## Layer 1 β€” Structural validity rules -| Rule ID | Severity | Gates is_valid? | Applies to | Checks | -| ------------------------- | -------- | --------------- | ---------- | ---------------------------------------------------------------------------------------------- | -| `TC_001` | ERROR | Yes | document | XML declaration present ( first line). | -| `TC_002` | ERROR | Yes | document | XML is well-formed (parses without error). | -| `TC_003` | ERROR | Yes | document | Root element is . | -| `TC_010` | ERROR | Yes | testCase | testCase id, when present, is a non-negative integer (id is optional; guid is the identifier). | -| `TC_011` | ERROR | Yes | testCase | testCase has a guid attribute. | -| `TC_012` | ERROR | Yes | testCase | testCase guid is a valid UUID v4. | -| `TC_020` | ERROR | Yes | testCase | testCase has a element. | -| `TC_030` | ERROR | Yes | apiCall | Each apiCall has a guid attribute. | -| `TC_031` | ERROR | Yes | apiCall | Each apiCall guid is a valid UUID v4. | -| `TC_032` | ERROR | Yes | apiCall | Each apiCall has an apiId attribute. | -| `TC_033` | WARNING | No | apiCall | Each apiCall has a descriptive name attribute. | -| `TC_034` | ERROR | Yes | apiCall | Each apiCall has a testItemId attribute. | -| `TC_035` | ERROR | Yes | apiCall | apiCall testItemId is a whole number. | -| `DATA-001` | WARNING | No | testCase | only iterates under a test plan; flags direct testCase-mode execution. | -| `VAR-REF-001` | WARNING | No | argument | A whole-token {Var} stored as valueClass="string" (use class="variable"). | -| `VAR-REF-002` | WARNING | No | argument | {Var} tokens embedded in a plain string (use class="compound"). | -| `UI-TARGET-001` | ERROR | Yes | apiCall | UiWithScreen/UiWithRow target uses class="uiTarget". | -| `UI-LOCATOR-001` | ERROR | Yes | apiCall | UI action locator uses class="uiLocator". | -| `UI-INTERACTION-001` | ERROR | Yes | apiCall | UiDoAction interaction uses class="uiInteraction". | -| `UI-ASSERT-STRUCTURE-001` | ERROR | Yes | apiCall | UiAssert uses nested field/column/page assertion containers, not a flat argument. | -| `SETVALUES-STRUCTURE-001` | ERROR | Yes | apiCall | SetValues values argument uses class="valueList" with . | -| `ASSERT-001` | WARNING | No | apiCall | AssertValues namedValues format flagged for variable/Apex comparisons. | -| `COMPARISON-TYPE-001` | ERROR | Yes | apiCall | comparisonType is within the step-scoped enum subset (load-blocking otherwise). | +| Rule ID | Severity | Gates is_valid? | Applies to | Checks | +| ----------------------------- | -------- | --------------- | ---------- | ------------------------------------------------------------------------------------------------------------------------------------ | +| `TC_001` | ERROR | Yes | document | XML declaration present ( first line). | +| `TC_002` | ERROR | Yes | document | XML is well-formed (parses without error). | +| `TC_003` | ERROR | Yes | document | Root element is . | +| `TC_010` | ERROR | Yes | testCase | testCase id, when present, is a non-negative integer (id is optional; guid is the identifier). | +| `TC_011` | ERROR | Yes | testCase | testCase has a guid attribute. | +| `TC_012` | ERROR | Yes | testCase | testCase guid is a valid UUID v4. | +| `TC_020` | ERROR | Yes | testCase | testCase has a element. | +| `TC_030` | ERROR | Yes | apiCall | Each apiCall has a guid attribute. | +| `TC_031` | ERROR | Yes | apiCall | Each apiCall guid is a valid UUID v4. | +| `TC_032` | ERROR | Yes | apiCall | Each apiCall has an apiId attribute. | +| `TC_033` | WARNING | No | apiCall | Each apiCall has a descriptive name attribute. | +| `TC_034` | ERROR | Yes | apiCall | Each apiCall has a testItemId attribute. | +| `TC_035` | ERROR | Yes | apiCall | apiCall testItemId is a whole number. | +| `DATA-001` | WARNING | No | testCase | only iterates under a test plan; flags direct testCase-mode execution. | +| `VAR-REF-001` | WARNING | No | argument | A whole-token {Var} stored as valueClass="string" (use class="variable"). | +| `VAR-REF-002` | WARNING | No | argument | {Var} tokens embedded in a plain string (use class="compound"). | +| `UI-TARGET-001` | ERROR | Yes | apiCall | UiWithScreen/UiWithRow target uses class="uiTarget". | +| `UI-LOCATOR-001` | ERROR | Yes | apiCall | UI action locator uses class="uiLocator". | +| `UI-INTERACTION-001` | ERROR | Yes | apiCall | UiDoAction interaction uses class="uiInteraction". | +| `UI-INTERACTION-002` | ERROR | Yes | apiCall | UiDoAction interaction name is a real Provar interaction, not one borrowed from another framework (type/fill/enter/input/tap/press). | +| `UI-ASSERT-STRUCTURE-001` | ERROR | Yes | apiCall | UiAssert field assertions use a nested , not a flat argument or a block. | +| `SF-CONNECT-TYPE-001` | WARNING | No | testCase | A Salesforce UI test connects via ApexConnect (API + UI), not a UiConnect with no ApexConnect. | +| `CONNECT-REF-CONSISTENCY-001` | WARNING | No | apiCall | Every connection reference (uiConnectionName/apexConnectionName/…) matches a connect step's resultName. | +| `SETVALUES-STRUCTURE-001` | ERROR | Yes | apiCall | SetValues values argument uses class="valueList" with . | +| `ASSERT-001` | WARNING | No | apiCall | AssertValues namedValues format flagged for variable/Apex comparisons. | +| `COMPARISON-TYPE-001` | ERROR | Yes | apiCall | comparisonType is within the step-scoped enum subset (load-blocking otherwise). | ## Layer 2 β€” Best-practice rules @@ -108,7 +111,10 @@ Provar test-case validation runs in two layers. This registry is the single cano | `SETVALUES-FUNC-STR-001` | StructureAndGrouping | major | 5 | No | SetValues must not use string interpolation for function calls. | | `SETVALUES-INVALID-ELEMENT-001` | StructureAndGrouping | critical | 10 | Yes | SetValues must not contain invalid child elements. | | `SETVALUES-ZERO-IDX-001` | StructureAndGrouping | major | 5 | No | SetValues string expression must not use [0] index. | +| `STEP-IDE-PARITY-001` | StructureAndGrouping | info | 1 | No | Steps should round-trip as IDE-authored XML. | | `STEP-NAMES-001` | StructureAndGrouping | minor | 2 | No | Custom step names for UI asserts and sets. | +| `STEP-RECOMMENDED-ARGS-001` | StructureAndGrouping | minor | 2 | No | Steps should declare conventionally-expected arguments. | +| `STEP-REQUIRED-ARGS-001` | StructureAndGrouping | major | 5 | No | Steps must declare all schema-required arguments. | | `STRUCT-GROUP-001` | StructureAndGrouping | minor | 2 | No | All steps are inside Group steps or BDD structure. | | `STRUCT-SUMMARY-001` | StructureAndGrouping | info | 1 | No | Test case has top-level summary. | | `UI-ASSERT-STRUCT-001` | StructureAndGrouping | critical | 8 | Yes | UiAssert steps must include all required arguments. | diff --git a/docs/mcp-pilot-guide.md b/docs/mcp-pilot-guide.md index 06f5e6b..1f588dd 100644 --- a/docs/mcp-pilot-guide.md +++ b/docs/mcp-pilot-guide.md @@ -406,16 +406,52 @@ NitroX is Provar's Hybrid Model for locators β€” it maps Salesforce component-ba **Without an API key configured:** - `isError` must be `false` (NOT `true`) β€” the generation workflow must continue -- `examples` must be `[]` +- `source` is present on every path: `corpus`, `bundled`, or `none` +- For a **Salesforce UI** query, `source` is `bundled` and `examples` carries a + known-correct offline example (`quality_tier: "bundled"`), so a brand-new project + still has a validated structural pattern to follow +- For an **unrelated** query (REST, database, NitroX), `source` is `none` and + `examples` is `[]` β€” the bundled set is deliberately withheld off-topic, because a + mismatched example steers generation worse than no example at all - `warning` must mention `sf provar auth login` +**On rate limit or network failure:** `source` is `none` and `examples` is `[]` β€” +bundled examples are withheld so a transient outage is never disguised as "no matches". + **What to look for:** -- The AI acknowledges the missing key and offers to continue without grounding +- The AI acknowledges the missing key and either uses the bundled example or falls + back to `provar_step_schema` for the step types it needs - No error is thrown that would abort the session --- +### Scenario 10a: Step Schema Lookup Without Corpus Access + +**Goal:** Confirm an agent with no API key can still author correct steps, and that the +fallback named in error messages actually resolves to a callable tool. + +> "What arguments does a UiWithScreen step require? Then list every UI step type." + +**What to look for:** + +- The AI calls `provar_step_schema` with `api_id: "UiWithScreen"`, then `category: "UI"` +- Required arguments come back as `uiConnectionName` and `target` β€” **not** a phantom + `screenName` +- The response separates three tiers: `required_arguments` (Provar fails without + these), `recommended_arguments` (present on 80-99% of real steps), and + `ide_emitted_arguments` (what the Provar IDE writes even when empty) +- The tool is reachable regardless of `PROVAR_MCP_TOOLS` β€” it is registered in the + `validation`, `qualityhub` and `authoring` groups, because all three name it as a + recovery path + +**Also verify the recovery loop end to end:** validate a test case with a structural +error, confirm the error text names `provar_step_schema`, then confirm that tool is +callable in the same session. Guidance naming an unavailable tool is a dead end β€” the +exact failure this scenario guards against. + +--- + ### Scenario 11: Migration Prompts β€” Convert a Test from Another Framework **Goal:** Use the MCP migration prompts to convert a test written in CRT, Selenium, or Playwright into a Provar XML test case in one step. diff --git a/docs/mcp.md b/docs/mcp.md index b350704..dda89f7 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -61,6 +61,8 @@ The Provar DX CLI ships with a built-in **Model Context Protocol (MCP) server** - [provar_nitrox_patch](#provar_nitrox_patch) - [Quality Hub API tools](#quality-hub-api-tools) - [provar_qualityhub_examples_retrieve](#provar_qualityhub_examples_retrieve) + - [Step schema reference](#step-schema-reference) + - [provar_step_schema](#provar_step_schema) - [Org metadata via Salesforce Hosted MCP](#org-metadata-via-salesforce-hosted-mcp) - [MCP Prompts](#mcp-prompts) - [Migration prompts](#migration-prompts) @@ -565,7 +567,7 @@ Paste the [standard config](#the-standard-config-recommended) into either file u } ``` -> **Tool limit:** Agentforce Vibes loads approximately 20 tools per MCP server at runtime. The Provar MCP server exposes 42 tools β€” you may need to restart or re-enable the server between tasks if the active tool list gets out of date. Salesforce is tracking this limit; consult the [Agentforce Vibes MCP documentation](https://developer.salesforce.com/docs/platform/einstein-for-devs/guide/devagent-mcp.html) for the latest guidance. +> **Tool limit:** Agentforce Vibes loads approximately 20 tools per MCP server at runtime. The Provar MCP server exposes 43 tools β€” you may need to restart or re-enable the server between tasks if the active tool list gets out of date. Salesforce is tracking this limit; consult the [Agentforce Vibes MCP documentation](https://developer.salesforce.com/docs/platform/einstein-for-devs/guide/devagent-mcp.html) for the latest guidance. @@ -666,16 +668,16 @@ PROVAR_MCP_TOOLS=nitrox,authoring Restricts which tool groups are registered when the server starts. Only the groups listed (comma-separated, case-insensitive) are made available. `provardx_ping` is always registered regardless of this setting. -| Group name | Tools registered | -| ------------ | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `nitrox` | `provar_nitrox_discover`, `provar_nitrox_generate`, `provar_nitrox_patch`, `provar_nitrox_read`, `provar_nitrox_validate` | -| `automation` | `provar_automation_setup`, `provar_automation_config_load`, `provar_automation_metadata_download`, `provar_automation_compile`, `provar_automation_testrun` | -| `qualityhub` | `provar_qualityhub_connect`, `provar_qualityhub_display`, `provar_qualityhub_testrun`, `provar_qualityhub_testrun_abort`, `provar_qualityhub_testrun_report`, `provar_qualityhub_examples_retrieve`, `provar_qualityhub_testcase_retrieve`, `provar_qualityhub_defect_create` | -| `validation` | `provar_project_validate`, `provar_ant_generate`, `provar_ant_validate`, `provar_properties_*`, `provar_testcase_validate`, `provar_testsuite_validate`, `provar_testplan_validate`, `provar_pageobject_validate` | -| `authoring` | `provar_testcase_generate`, `provar_pageobject_generate`, `provar_testcase_step_edit`, `provar_testplan_*` | -| `inspect` | `provar_project_inspect` | -| `connection` | `provar_connection_list` | -| `rca` | `provar_testrun_rca`, `provar_testrun_report_locate` | +| Group name | Tools registered | +| ------------ | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `nitrox` | `provar_nitrox_discover`, `provar_nitrox_generate`, `provar_nitrox_patch`, `provar_nitrox_read`, `provar_nitrox_validate` | +| `automation` | `provar_automation_setup`, `provar_automation_config_load`, `provar_automation_metadata_download`, `provar_automation_compile`, `provar_automation_testrun` | +| `qualityhub` | `provar_qualityhub_connect`, `provar_qualityhub_display`, `provar_qualityhub_testrun`, `provar_qualityhub_testrun_abort`, `provar_qualityhub_testrun_report`, `provar_qualityhub_examples_retrieve`, `provar_qualityhub_testcase_retrieve`, `provar_qualityhub_defect_create`, `provar_step_schema` | +| `validation` | `provar_project_validate`, `provar_ant_generate`, `provar_ant_validate`, `provar_properties_*`, `provar_testcase_validate`, `provar_testsuite_validate`, `provar_testplan_validate`, `provar_pageobject_validate`, `provar_step_schema` | +| `authoring` | `provar_testcase_generate`, `provar_pageobject_generate`, `provar_testcase_step_edit`, `provar_testplan_*`, `provar_step_schema` | +| `inspect` | `provar_project_inspect` | +| `connection` | `provar_connection_list` | +| `rca` | `provar_testrun_rca`, `provar_testrun_report_locate` | **Example β€” NitroX-only session:** @@ -1058,7 +1060,7 @@ Validates an XML test case for schema correctness (validity score) and best prac - **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 locator-bearing UI step (`UiDoAction`, `UiAssert`, `UiRead`, `UiFill`) has a `locator` argument that uses the wrong XML class. Must be `class="uiLocator"` or Provar cannot resolve the element. - **UI-INTERACTION-001** (ERROR) β€” A UI action step (e.g. `UiDoAction`) has an `interaction` argument that uses the wrong XML class (e.g. `class="value"`). Must be `class="uiInteraction"` (``). A plain string runs green from the CLI but renders the Action field blank in the Provar IDE step editor. The `interaction` attribute is converted automatically by `provar_testcase_generate`. -- **UI-ASSERT-STRUCTURE-001** (ERROR) β€” A `UiAssert` step carries a flat top-level field-assertion argument (`fieldLocator`, `attributeName`, `comparisonType`, or `expectedValue`) instead of the nested `fieldAssertions` β†’ `uiFieldAssertion` structure the Provar IDE Result Assertions tab binds from. The flat shape runs green from the CLI but renders the Result Assertions tab blank in the IDE. The correct form nests a `` containing a **bare** `` element (NOT `class="uiLocator"`) and ``, plus empty `columnAssertions`/`pageAssertions`. `provar_testcase_generate` builds this structure automatically when you pass `fieldLocator`/`attributeName`/`comparisonType`/`expectedValue` as flat attributes. +- **UI-ASSERT-STRUCTURE-001** (ERROR) β€” A `UiAssert` step expresses its field assertion in a wrong shape instead of the nested `fieldAssertions` β†’ `uiFieldAssertion` structure the Provar IDE Result Assertions tab binds from. Two wrong shapes are caught: (a) flat top-level field-assertion arguments (`fieldLocator`, `attributeName`, `comparisonType`, or `expectedValue`); and (b) a `fieldAssertions` argument filled with a ``/`` block (the shape an LLM reaches for by analogy with `UiFill`/`SetValues`). Both run green from the CLI but render the Result Assertions tab blank in the IDE. This rule now runs on **nested** UI steps too (UI actions/asserts live inside a `UiWithScreen` ``), not just top-level steps. The correct form nests a `` containing a **bare** `` element (NOT `class="uiLocator"`) and ``, plus empty `columnAssertions`/`pageAssertions`. `provar_testcase_generate` builds this structure automatically when you pass `fieldLocator`/`attributeName`/`comparisonType`/`expectedValue` as flat attributes. - **SETVALUES-STRUCTURE-001** (ERROR) β€” A `SetValues` step's `values` argument uses `class="value"` (plain string) instead of `class="valueList"` with `` children. This causes an immediate `ClassCastException` at runtime. - **COMPARISON-TYPE-001** (ERROR) β€” A `comparisonType` value is used outside the subset its step type allows. `comparisonType` is a single Provar enum but each step type accepts only a subset: **AssertValues** accepts the 16-value set (`EqualTo, NotEqualTo, GreaterThan, GreaterThanOrEqualTo, LessThan, LessThanOrEqualTo, IsPresent, IsEmpty, Matches, NotMatches, Contains, NotContains, StartsWith, NotStartsWith, EndsWith, NotEndsWith`); a **UI Assert** (`uiAttributeAssertion`) accepts only the 6-value set (`EqualTo, Contains, StartsWith, EndsWith, Matches, None`). A value outside the step's subset (e.g. `NotEqualTo` on a UI Assert) is load-blocking β€” the whole test case fails to load at runtime with `IllegalArgumentException: No enum constant com.provar.core.model.base.java.ComparisonType.`. This local check runs even offline / in `local_fallback`, so the load-blocker is caught without the Quality Hub back-end. Only literal `comparisonType` values are checked; variable / compound references are skipped. See [`provar://docs/step-reference`](#resources) for the full step-scoped tables. - **UI-NEST-STRUCT-001** (severity `major`, weight 7, category `XMLSchema`) β€” A UI action step (`UiDoAction`, `UiAssert`, `UiRead`, `UiFill`, `UiNavigate`, `UiWithRow`, or `UiHandleAlert`) is emitted outside a screen ancestor. To pass, every UI action must descend from a `UiWithScreen` or `UiWithRow` `apiCall` through a `` path. Control-flow wrappers (`If`/`ForEach`/`DoWhile`/`WaitFor`/`Switch`) between the screen ancestor and the UI action are allowed; steps inside `` are exempt (disabled / settings blocks). One violation is emitted per offending step, so `(rule_id, test_item_id)` de-duplicates cleanly against the Quality Hub API. Provar IDE cannot bind flat-emitted UI actions to a screen context and they will not render in the editor. Wrap each offending step in the canonical chain: @@ -1108,10 +1110,19 @@ Validates an XML test case for schema correctness (validity score) and best prac - **`SETVALUES-VALUE-001`** (`namedValueValue`, critical) β€” a `` inside a `SetValues` step is missing its child `` element. - **`UI-ASSERT-STRUCT-002`** (`uiAssertHallucinatedGeneratedParameters`, critical) β€” a `UiAssert` step contains a `` element, which is never valid on `UiAssert` and blocks validation. - **`UI-ASSERT-STRUCT-001`** (`uiAssertMissingArguments`, critical) β€” a `UiAssert` step is missing one or more required arguments (`fieldAssertions`, `columnAssertions`, `pageAssertions`, `resultScope`, `captureAfter`, `beforeWait`, `autoRetry` β€” present even if empty). - - **`UI-BINDING-ORDER-001`** (`bindingParameterOrder`, critical) β€” a `uiLocator` binding URI lists `action=`/`field=` before `object=`; the corpus-majority convention is `object=` first. - - **`UI-CONN-LITERAL-001`** (`uiConnectionNameLiteral`, critical) β€” a UI step's `uiConnectionName` uses a `class="variable"` value; it must be a literal connection name. - - **`FUNCCALL-VALID-001`** (`validFuncCallId`, major) β€” a `` uses an `id` that is not one of Provar's built-in functions (e.g. the hallucinated `Concatenate`/`Substring`); use the documented set (`Count`, `DateAdd`, `StringReplace`, …) or `class="compound"` for concatenation. - - **`RENDER-ROOT-001`** (`rootAttributes`, minor) β€” the root `` element carries an attribute outside the allowed set (`guid`, `id`, `name`, `visibility`, `registryId`, `failureBehaviour`). + - **`STEP-REQUIRED-ARGS-001`** (`schemaRequiredArguments`, major) β€” generalises the UiAssert-only check above to **every** step type: any step missing an argument that `provar_test_step_schema.json` lists in its `required_arguments` tier is flagged. This closes the gap where a test case could score 100 while omitting load-blocking arguments on `UiConnect`, `UiWithScreen`, `UiDoAction`, DB/Apex/REST steps, etc. The set is read from the schema at runtime β€” the same data `provar_step_schema` serves β€” so correcting the schema automatically corrects this rule; nothing is hardcoded. It is **major** (docks `quality_score`, does not gate `is_valid`) because the schema is a bundled reference rather than the authoritative Quality Hub ruleset. Emits **one aggregated violation carrying a `count`**, not one per step, so the logarithmic damping in the score formula applies. `(apiId, argument)` pairs already enforced by a dedicated `mustContainArgument` rule are suppressed so a single defect is never scored twice. `UiAssert`, the NitroX MS connect variants, and the experimental `ProvarLabs` category are excluded. + - **`STEP-RECOMMENDED-ARGS-001`** (`schemaRecommendedArguments`, minor, weight 2) β€” arguments in the schema's `recommended_arguments` tier: present on **80-99%** of real Provar steps of the same type, so strongly conventional but not load-blocking. Aggregated with a `count`. Surfaces the gap without materially moving `quality_score`. + - **`STEP-IDE-PARITY-001`** (`stepIdeParity`, info, weight 1) β€” arguments in the `ide_emitted_arguments` tier: the Provar IDE writes a step type's full argument set **even when the values are empty** (`ApexConnect` carries 18 on 100% of 1,524 real instances; `UiDoAction` 8 on 16,485). XML omitting them runs correctly but does not round-trip as IDE-authored, which is the most common reason users post-process generated test cases. Deliberately **advisory**: a heavily-affected file loses under 2 points, so IDE-parity drift alone can never push a valid test case below the quality threshold. + +> **The three argument tiers.** `required_arguments` means _the step needs this to function at runtime_ β€” scored `major`, not `critical`, because the file still loads. Genuinely load-blocking omissions have their own `critical` rules (`CONTROL-IF-001`, `SQL-QUERY-001`, `DB-CONNECT-001`, …) which the dedupe suppresses, so this tier sits deliberately beneath them. `ide_emitted_arguments` means _the IDE always writes this_. Conflating the two is what made an earlier revision of this rule set flag 704 of 2,701 real test cases as below-threshold β€” an argument present on 100% of real files usually means "the IDE emits it", not "the step breaks without it". The tiers derive from corpus frequency plus the reference doc, not from execution testing, which is why the required tier does not gate `is_valid`. Reconciled against a 1,457-file corpus; see `provar_step_schema` for any step type's three lists. + +- **`UI-BINDING-ORDER-001`** (`bindingParameterOrder`, critical) β€” a `uiLocator` binding URI lists `action=`/`field=` before `object=`; the corpus-majority convention is `object=` first. +- **`UI-CONN-LITERAL-001`** (`uiConnectionNameLiteral`, critical) β€” a UI step's `uiConnectionName` uses a `class="variable"` value; it must be a literal connection name. +- **`UI-INTERACTION-002`** (local, ERROR) β€” a `UiDoAction` `interaction` uses a name that is not a real Provar interaction. Provar ships a large interaction vocabulary β€” the corpus alone carries **39 distinct names** (`action`, `set`, `click`, `check`, `invoke`, `doubleClick`, `sfLookup`, `hover`, `file`, `clear`, `toggle`, `uncheck`, …) β€” so this rule deliberately **denylists** only names with zero corpus evidence that are known analogies from other frameworks (`type`, `fill`, `enter`, `input`, `tap`, `press`), naming the correct replacement (e.g. `type` β†’ `set`). It does **not** flag `click`, which is a genuine IDE-emitted Provar interaction with 1,240 corpus occurrences. Complements `UI-INTERACTION-001`, which checks the value **class**. +- **`SF-CONNECT-TYPE-001`** (local, WARNING) β€” the test drives the Salesforce UI (`sf:ui:` targets) but connects with `UiConnect` and **no** `ApexConnect`. `UiConnect` opens a browser only and has no Salesforce credential of its own; a Salesforce UI test should connect via `ApexConnect` (which opens the API connection and, with `quickUiLogin=true`/`uiApplicationName=LightningSales`, the Lightning UI). Add `UiConnect` only for a second, browser-only session that reuses the `ApexConnect` result. WARNING, not ERROR, because the connection could in rare setups be supplied by a parent callable test. +- **`CONNECT-REF-CONSISTENCY-001`** (local, WARNING) β€” a step references a connection (`uiConnectionName` / `apexConnectionName` / `dbConnectionName` / `webConnectionName`) that resolves to nothing. Provar resolves such a reference against **either** a connect step's `resultName` (which defaults to `connectionName` when omitted) **or** a connection declared in the project's `.testproject`; the rule checks both, reading project connections when `file_path` is supplied. It stays silent when: the test has no connect step at all (connections may be inherited from a parent callable test); any connect step's `connectionName`/`resultName` is non-literal β€” a `class="variable"` value, a `` with no class, or an empty argument resolved from project config β€” since the step's result cannot then be known statically; or no project context is available. Each suppression exists to avoid reporting a valid reference as dangling. +- **`FUNCCALL-VALID-001`** (`validFuncCallId`, major) β€” a `` uses an `id` that is not one of Provar's built-in functions (e.g. the hallucinated `Concatenate`/`Substring`); use the documented set (`Count`, `DateAdd`, `StringReplace`, …) or `class="compound"` for concatenation. +- **`RENDER-ROOT-001`** (`rootAttributes`, minor) β€” the root `` element carries an attribute outside the allowed set (`guid`, `id`, `name`, `visibility`, `registryId`, `failureBehaviour`). **Error codes** @@ -2378,7 +2389,7 @@ These tools call the Quality Hub HTTP API directly (no `sf` subprocess). They re Retrieve N similar Provar test case examples from the Quality Hub corpus (1000+ tests indexed in Bedrock). Use this **before** `provar_testcase_generate` to provide few-shot grounding examples. -If retrieval fails for any reason (no key, invalid key, rate limit, network error), the tool returns `{ examples: [], count: 0, warning: "..." }` with `isError: false` so the generation workflow can continue without grounding. It **never** hard-errors on API failure. +When **no API key** is configured (or the key is **invalid**), the tool returns a small set of offline, known-correct **bundled** examples (`source: "bundled"`, `quality_tier: "bundled"`) instead of an empty list, so first-test-case generation in a brand-new project still has a validated structural pattern to follow. On **transient** failures (rate limit, network error) it returns `{ examples: [], count: 0, warning: "..." }` so a retry is the obvious next move. In all cases `isError: false` β€” it **never** hard-errors on API failure. Bundled examples are validated in CI (`quality_score === 100`, zero violations) against the same rules `provar_testcase_validate` applies. | Input | Type | Required | Default | Description | | --------------------- | ------- | -------- | ------- | ------------------------------------------------------------------------------ | @@ -2397,20 +2408,46 @@ If retrieval fails for any reason (no key, invalid key, rate limit, network erro Each element in `examples`: -| Field | Description | -| ------------------- | ----------------------------------------------------------------- | -| `id` | Corpus path (e.g. `tier4/SalesCloud/create.xml`) | -| `name` | Test case name | -| `xml` | Full Provar XML test case content | -| `similarity_score` | Similarity score in [0, 1] | -| `salesforce_object` | Primary Salesforce object the test exercises | -| `quality_tier` | Corpus tier (`tier4`, `tier3`, `tier2`, `tier1`) | -| `full_content` | `true` when the full XML was returned (not truncated server-side) | +| Field | Description | +| ------------------- | ----------------------------------------------------------------------------------- | +| `id` | Corpus path (e.g. `tier4/SalesCloud/create.xml`) | +| `name` | Test case name | +| `xml` | Full Provar XML test case content | +| `similarity_score` | Similarity score in [0, 1] | +| `salesforce_object` | Primary Salesforce object the test exercises | +| `quality_tier` | Corpus tier (`tier4`, `tier3`, `tier2`, `tier1`), or `bundled` for offline examples | +| `full_content` | `true` when the full XML was returned (not truncated server-side) | +| `source` | `bundled` on offline examples (absent for corpus matches) | **Error codes:** `INVALID_QUERY` (empty query β€” only error that sets `isError: true`) --- +## Step schema reference + +### `provar_step_schema` + +Returns the argument schema for Provar test-case step types, read from the bundled step reference (`provar_test_step_schema.json`). This is the **tool** form of the `provar://schema/test-step` MCP resource β€” agents that cannot read MCP resources can still reach the schema to ground test-case generation and hand-editing in exact argument names. Call it before `provar_testcase_generate` / `provar_testcase_step_edit`, or when `provar_testcase_validate` reports a structural error, to get a step type's required/optional arguments. Read-only and offline. + +| Input | Type | Required | Default | Description | +| ---------- | ------ | -------- | ------- | ------------------------------------------------------------------------------------------------------------------------ | +| `api_id` | string | no | β€” | Full apiId (`com.provar.plugins.forcedotcom.core.ui.UiConnect`) or short name (`UiConnect`). Returns that step's schema. | +| `category` | string | no | β€” | Category to list (`UI`, `Salesforce`, `Control`, `Data`, `Utility`, `ProvarAI`, `ProvarLabs`). | + +Precedence: `api_id` β†’ single step; else `category` β†’ steps in that category; else the full index. + +| Output field | Description | +| ------------ | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `step` | (with `api_id`) `{ apiId, description, category, required_arguments[], optional_arguments[], validation_rules[], best_practices[] }`. Each argument is `{ id, type, default?, description? }`. | +| `steps` | (with `category` or index) array of `{ name, apiId, category, required_arguments[] }` | +| `count` | Number of step types returned (category/index modes) | + +**Example** β€” `{ "api_id": "UiConnect" }` returns `step.required_arguments = [{ "id": "connectionName", "type": "string" }]`. + +**Error codes:** `STEP_TYPE_NOT_FOUND` (no step matches `api_id`; includes a `suggestion` with near matches), `STEP_SCHEMA_NOT_FOUND` (bundled schema unreadable β€” reinstall the plugin). + +--- + ### Org metadata via Salesforce Hosted MCP Provar MCP does not include a built-in org introspection tool. Instead, connect the **Salesforce Hosted MCP Server** (`platform/sobject-reads`) alongside Provar MCP and call `getObjectSchema` to retrieve sObject field metadata. Pass the result as additional context in your `provar_qualityhub_examples_retrieve` query. diff --git a/package.json b/package.json index 934ce8f..c2ce935 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.6.4", + "version": "1.6.5", "mcpName": "io.github.ProvarTesting/provar", "license": "BSD-3-Clause", "plugins": [ diff --git a/scripts/mcp-smoke.cjs b/scripts/mcp-smoke.cjs index 467df36..f8a0e9a 100644 --- a/scripts/mcp-smoke.cjs +++ b/scripts/mcp-smoke.cjs @@ -191,6 +191,14 @@ async function runTests() { output_path: path.join(TMP, 'steps-required-smoke-rejected.testcase'), }); + // ── 6c. provar_step_schema (by api_id) ──────────────────────────────────── + // Registered in authoring, validation AND qualityhub: all three name it as the + // recovery path when generation or validation hits a step it cannot shape. Gate on + // any of them, or a PROVAR_MCP_TOOLS=validation smoke run would skip the very case + // this covers β€” guidance pointing at a tool the session does not expose. + if (inGroup('authoring') || inGroup('validation') || inGroup('qualityhub')) + await callTool('provar_step_schema', { api_id: 'UiConnect' }); + // ── 7. provar_testcase_validate ─────────────────────────────────────────── if (inGroup('validation')) await callTool('provar_testcase_validate', { content: '' }); diff --git a/server.json b/server.json index b269900..567053b 100644 --- a/server.json +++ b/server.json @@ -14,12 +14,12 @@ "url": "https://github.com/ProvarTesting/provardx-cli", "source": "github" }, - "version": "1.6.4", + "version": "1.6.5", "packages": [ { "registryType": "npm", "identifier": "@provartesting/provardx-cli", - "version": "1.6.4", + "version": "1.6.5", "transport": { "type": "stdio" }, diff --git a/src/mcp/examples/bundledExamples.ts b/src/mcp/examples/bundledExamples.ts new file mode 100644 index 0000000..1b06709 --- /dev/null +++ b/src/mcp/examples/bundledExamples.ts @@ -0,0 +1,241 @@ +/* + * 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 */ + +/** + * Offline, known-correct example test cases bundled with the MCP server. + * + * `provar_qualityhub_examples_retrieve` returns these (clearly labelled `bundled`) + * when no Provar API key is configured, so first-test-case generation in a brand-new + * project has *some* grounded XML to pattern-match against instead of falling back + * entirely to model recollection. + * + * Every example here is validated by test/unit/mcp/bundledExamples.test.ts, which runs + * it through the same best-practices + structural validators the server exposes and + * asserts quality_score === 100 with zero violations. If you add or edit an example, + * that test guards it against drifting out of correctness β€” it is what caught the + * original example using UiConnect / interaction=click / flat field assertions. + */ + +export interface BundledExample { + id: string; + name: string; + /** Short scenario key used for lightweight query matching. */ + scenario: string; + salesforce_object: string; + keywords: string[]; + xml: string; +} + +const UI_CREATE_VALIDATE_ACCOUNT = ` + + Creates an Account through the Salesforce UI (mandatory Name field), saves it, and verifies the Name on the record detail page. + + + + MyConnection + MyConnection + Test + LightningSales + true + + + + + + + + + + + + + + + + + + + MyConnection + + Always + On SF Account Home screen + false + false + + + + + + + + + + + + Click the New button + false + false + + + + + + + + + + + + MyConnection + + Dont + NewAccountId + false + false + + + + + + + + + + + + + Set the Account Name field + false + false + Provar Automation Account + + + + + + + + + + Click the Save button + false + false + + + + + + + + + + + + MyConnection + + Dont + On SF Account View screen + false + false + + + + + + + + + + + + + + + + + Provar Automation Account + + + + + + + + AccountNameAssertion + Test + false + + + + + + + + + + +`; + +export const BUNDLED_EXAMPLES: BundledExample[] = [ + { + id: 'bundled-ui-create-validate-account', + name: 'Create and verify an Account via the Salesforce UI', + scenario: 'ui-create-validate', + salesforce_object: 'Account', + keywords: ['ui', 'create', 'new', 'account', 'validate', 'verify', 'assert', 'save', 'record', 'detail'], + xml: UI_CREATE_VALIDATE_ACCOUNT, + }, +]; + +export interface BundledExampleResult { + id: string; + name: string; + xml: string; + similarity_score: number; + salesforce_object: string; + quality_tier: string; + full_content: boolean; + source: 'bundled'; +} + +/** + * Pick up to `n` bundled examples for a query, ranked by keyword overlap. + * + * Relevance-gated: an example with ZERO keyword overlap is not returned. The bundle + * is small and Salesforce-UI shaped, so answering a REST, database or NitroX query + * with an unrelated Account UI example is worse than answering honestly with nothing + * β€” the caller pattern-matches whatever it is handed. An empty result routes the + * caller to provar_step_schema instead, which does cover those step types. + * + * `similarity_score` is a keyword-overlap proxy, NOT the corpus's cosine similarity. + * It is reported on the same field for shape compatibility but deliberately capped + * below 1 so it can never outrank a real corpus match if the two are ever merged. + */ +export function selectBundledExamples(query: string, n?: number): BundledExampleResult[] { + const q = query.toLowerCase(); + // Tolerate a missing/invalid n (callers that bypass the tool's zod default): return all. + const limit = typeof n === 'number' && Number.isFinite(n) && n > 0 ? Math.floor(n) : BUNDLED_EXAMPLES.length; + const scored = BUNDLED_EXAMPLES.map((ex) => ({ + ex, + hits: ex.keywords.filter((k) => q.includes(k)).length, + })) + .filter(({ hits }) => hits > 0) + .sort((a, b) => b.hits - a.hits); + + return scored.slice(0, Math.max(0, limit)).map(({ ex, hits }) => ({ + id: ex.id, + name: ex.name, + xml: ex.xml, + similarity_score: Math.min(0.95, 0.5 + hits * 0.05), + salesforce_object: ex.salesforce_object, + quality_tier: 'bundled', + full_content: true, + source: 'bundled', + })); +} diff --git a/src/mcp/prompts/loopPrompts.ts b/src/mcp/prompts/loopPrompts.ts index 9f3a9c0..cac2783 100644 --- a/src/mcp/prompts/loopPrompts.ts +++ b/src/mcp/prompts/loopPrompts.ts @@ -66,7 +66,7 @@ Follow these steps in order: (e.g. "close opportunity" or "create lead"). Use the returned XML examples as the sole reference for Provar step structure and argument patterns. Do not invent XML structure from prior knowledge. If the response has \`"count": 0\` with a \`"warning"\` field (API unavailable or not configured), - fall back: read the \`provar://docs/step-reference\` MCP resource for step types and attribute + fall back: call the \`provar_step_schema\` tool (api_id or category) for step types and attribute formats, then continue. 3. **Map acceptance criteria to steps** β€” for each acceptance criterion, identify the corresponding Provar @@ -165,7 +165,7 @@ Follow these steps in order: failing step's scenario (e.g. "close opportunity UiDoAction" or "assert field value UiAssert"). Use the returned examples to verify the correct structure for the failing step type. If the response has \`"count": 0\` with a \`"warning"\` field, fall back: read the - \`provar://docs/step-reference\` MCP resource for the correct attribute schema for the failing + \`provar_step_schema\` tool (api_id) for the correct attribute schema for the failing step type, then continue. 4. **Diagnose the root cause** β€” compare the failing step's XML against the corpus examples. Common issues: @@ -236,7 +236,7 @@ Follow these steps in order: 3. **Get corpus examples** β€” call \`provar_qualityhub_examples_retrieve\` with keywords describing the test scenario (e.g. "create opportunity", "close opportunity"). Use the returned examples as a quality baseline. If the response has \`"count": 0\` with a \`"warning"\` field, fall back: read - the \`provar://docs/step-reference\` MCP resource for step type schemas, then continue the review + the \`provar_step_schema\` tool (api_id or category) for step type schemas, then continue the review using that as the quality baseline. 4. **Review for quality gaps** β€” check for each of the following, noting pass or fail: @@ -343,11 +343,11 @@ ${ 3. **Get corpus examples** β€” call \`provar_qualityhub_examples_retrieve\` with "${objectName.toLowerCase()}" as the query to understand what test patterns exist in the corpus for this object. If the response has \`"count": 0\` with a \`"warning"\` field, fall back: read the - \`provar://docs/step-reference\` MCP resource for step type schemas, then continue.` + \`provar_step_schema\` tool (api_id or category) for step type schemas, then continue.` : `2. **Get corpus examples** β€” call \`provar_qualityhub_examples_retrieve\` with "${objectName.toLowerCase()}" as the query to understand what test patterns exist in the corpus for this object. If the response has \`"count": 0\` with a \`"warning"\` field, fall back: read the - \`provar://docs/step-reference\` MCP resource for step type schemas, then continue.` + \`provar_step_schema\` tool (api_id or category) for step type schemas, then continue.` } ${targetOrg ? '4' : '3'}. **Build the coverage matrix** β€” define the standard test scenarios for "${objectName}": @@ -438,7 +438,7 @@ This is a **database test**, NOT a Salesforce UI or Apex test. Do not use UiConn Follow these steps in order: 1. **Get corpus examples** β€” call \`provar_qualityhub_examples_retrieve\` with a query that includes "database DbConnect SqlQuery" plus keywords from the story (e.g. "database SQL Server verify record count"). Use the returned XML examples as the reference for correct step structure. - If the response has \`"count": 0\` with a \`"warning"\` field (API unavailable or not configured), fall back: read the \`provar://docs/step-reference\` MCP resource β€” specifically the Database Steps section β€” for the correct attribute schema, then continue. + If the response has \`"count": 0\` with a \`"warning"\` field (API unavailable or not configured), fall back: call the \`provar_step_schema\` tool with category="Data" for the correct attribute schema, then continue. 2. **Generate the test case** β€” produce valid Provar XML. Apply these database-specific rules: diff --git a/src/mcp/prompts/migrationPrompts.ts b/src/mcp/prompts/migrationPrompts.ts index 37c7f53..f42f513 100644 --- a/src/mcp/prompts/migrationPrompts.ts +++ b/src/mcp/prompts/migrationPrompts.ts @@ -69,7 +69,7 @@ Follow these steps in order: describe the source test's main scenario (e.g. "create opportunity", "close case", "convert lead"). Use the returned examples as few-shot grounding for the Provar XML format and step patterns. If the response has \`"count": 0\` with a \`"warning"\` field (API unavailable or not configured), - fall back: read the \`provar://docs/step-reference\` MCP resource for step types and attribute + fall back: call the \`provar_step_schema\` tool (api_id or category) for step types and attribute formats, then continue with generation based on that reference. 2. **Generate the test case** β€” produce a valid Provar XML test case that faithfully captures diff --git a/src/mcp/rules/provar_best_practices_rules.json b/src/mcp/rules/provar_best_practices_rules.json index 80248df..a3e41a4 100644 --- a/src/mcp/rules/provar_best_practices_rules.json +++ b/src/mcp/rules/provar_best_practices_rules.json @@ -2624,6 +2624,54 @@ "notes": "Based on corpus analysis of 1451 test files with 329,424 elements. Common AI hallucination: class='null' (never valid). Empty arguments should have no child.", "source": "Corpus analysis: All valid value class patterns from InternalProjects" }, + { + "id": "STEP-REQUIRED-ARGS-001", + "category": "StructureAndGrouping", + "name": "Steps must declare all schema-required arguments", + "description": "Every step type has a set of arguments a step needs to do its job at RUNTIME. Severity is `major`, matching the taxonomy: `critical` means the file will not load (gates is_valid), `major` means it loads but the step misbehaves when executed. Genuinely load-blocking omissions already have dedicated `critical` rules (CONTROL-IF-001, SQL-QUERY-001, DB-CONNECT-001, …), which the dedupe below suppresses, so this rule sits deliberately beneath them. The tier is derived from corpus frequency plus the reference doc, NOT from execution testing, so it does not claim load-blocking authority. This rule derives that set for each apiId from the required_arguments tier of provar_test_step_schema.json and flags any step missing one. It generalises the UiAssert-only UI-ASSERT-STRUCT-001 check to all step types, so a test case can no longer score 100 while omitting load-blocking arguments on non-UiAssert steps. Emits ONE aggregated violation carrying a count, not one per step, so the logarithmic damping in calculateBPScore applies. (apiId, argument) pairs already enforced by a dedicated mustContainArgument rule are suppressed so a single defect is never scored twice. UiAssert, the NitroX MS connect variants, and the experimental ProvarLabs category are excluded.", + "appliesTo": ["Step"], + "severity": "major", + "weight": 5, + "recommendation": "Add the missing argument(s) to the step. Arguments that carry no value should still be declared as empty self-closing tags (e.g. ). Call the provar_step_schema tool (api_id or category) for each step type's required arguments.", + "check": { + "type": "schemaRequiredArguments", + "target": "step" + }, + "notes": "Schema-driven: the required-argument list is read from provar_test_step_schema.json at runtime, so correcting the schema automatically corrects this rule. No per-argument list is hardcoded here. The required tier is reconciled against 1,457 real .testcase files - an argument is listed only where the step needs it to function, never merely because the IDE always emits it (that is the ide_emitted_arguments tier, checked by STEP-IDE-PARITY-001). Evidence is corpus frequency and the reference doc; promoting this tier to `critical` would gate is_valid on frequency-derived evidence, which is what caused 704 of 2,701 real files to fall below threshold in an earlier revision.", + "source": "Derived from provar_test_step_schema.json (reconciled against a 1,457-file corpus and docs/PROVAR_TEST_STEP_REFERENCE.md)" + }, + { + "id": "STEP-RECOMMENDED-ARGS-001", + "category": "StructureAndGrouping", + "name": "Steps should declare conventionally-expected arguments", + "description": "Arguments that appear on 80-99% of real Provar steps of the same type: strongly conventional but not load-blocking, so a step omitting one still runs. Derived from the recommended_arguments tier of provar_test_step_schema.json. Emits one aggregated violation with a count. Runs at minor severity so it surfaces the gap without materially moving quality_score.", + "appliesTo": ["Step"], + "severity": "minor", + "weight": 2, + "recommendation": "Consider adding the listed argument(s). They are present on the large majority of real steps of this type, so their absence usually means the step was hand-written rather than authored in the Provar IDE. Call provar_step_schema for the full argument set.", + "check": { + "type": "schemaRecommendedArguments", + "target": "step" + }, + "notes": "The 80-99% presence band from the corpus survey. Below 80% an argument is treated as genuinely optional and is not checked at all.", + "source": "Corpus analysis: 1,457 .testcase files / 43,797 apiCall steps" + }, + { + "id": "STEP-IDE-PARITY-001", + "category": "StructureAndGrouping", + "name": "Steps should round-trip as IDE-authored XML", + "description": "The Provar IDE emits a step type's full argument set even when the values are empty - ApexConnect carries 18 arguments on 100% of 1,524 real instances, UiDoAction 8 on 16,485. Generated XML that omits them runs correctly but does not look IDE-authored, which is the most common reason users post-process generated test cases. Derived from the ide_emitted_arguments tier. Runs at info severity with weight 1, so a heavily-affected file loses under 2 points: the gap is always reported, but IDE-parity drift alone can never push a valid test case below the quality threshold.", + "appliesTo": ["Step"], + "severity": "info", + "weight": 1, + "recommendation": "Declare the listed arguments as empty self-closing tags (e.g. ) so the step matches what the Provar IDE writes. This is a fidelity improvement, not a correctness fix - the step runs either way.", + "check": { + "type": "stepIdeParity", + "target": "step" + }, + "notes": "Deliberately advisory. Scoring IDE parity at major severity is what caused 704 of 2,701 real test cases to drop below the quality threshold; at info/weight 1 the same information is surfaced without the over-gating. Not deduplicated against mustContainArgument rules, because a dedicated rule firing at critical severity and this one firing at info convey different things (broken vs. merely non-IDE-shaped).", + "source": "Corpus analysis: 1,457 .testcase files / 43,797 apiCall steps" + }, { "id": "UI-ASSERT-STRUCT-001", "category": "StructureAndGrouping", diff --git a/src/mcp/rules/provar_layer1_rules.json b/src/mcp/rules/provar_layer1_rules.json index 289eb69..cbd44b3 100644 --- a/src/mcp/rules/provar_layer1_rules.json +++ b/src/mcp/rules/provar_layer1_rules.json @@ -122,11 +122,29 @@ "applies_to": "apiCall", "description": "UiDoAction interaction uses class=\"uiInteraction\"." }, + { + "id": "UI-INTERACTION-002", + "severity": "ERROR", + "applies_to": "apiCall", + "description": "UiDoAction interaction name is a real Provar interaction, not one borrowed from another framework (type/fill/enter/input/tap/press)." + }, { "id": "UI-ASSERT-STRUCTURE-001", "severity": "ERROR", "applies_to": "apiCall", - "description": "UiAssert uses nested field/column/page assertion containers, not a flat argument." + "description": "UiAssert field assertions use a nested , not a flat argument or a block." + }, + { + "id": "SF-CONNECT-TYPE-001", + "severity": "WARNING", + "applies_to": "testCase", + "description": "A Salesforce UI test connects via ApexConnect (API + UI), not a UiConnect with no ApexConnect." + }, + { + "id": "CONNECT-REF-CONSISTENCY-001", + "severity": "WARNING", + "applies_to": "apiCall", + "description": "Every connection reference (uiConnectionName/apexConnectionName/…) matches a connect step's resultName." }, { "id": "SETVALUES-STRUCTURE-001", diff --git a/src/mcp/rules/provar_test_step_schema.json b/src/mcp/rules/provar_test_step_schema.json index 27761cb..2e9b37a 100644 --- a/src/mcp/rules/provar_test_step_schema.json +++ b/src/mcp/rules/provar_test_step_schema.json @@ -119,13 +119,21 @@ "id": "failureMessage", "type": "string", "description": "Custom failure message" - }, - { - "id": "continueOnFailure", - "type": "boolean", - "default": "false" } ], + "recommended_arguments": [], + "ide_emitted_arguments": [ + "expectedValue", + "comparisonType", + "actualValue", + "caseSensitive", + "numeric", + "retainDecimals", + "nullGreater", + "matchMultiLine", + "matchDotAll", + "failureMessage" + ], "validation_rules": [ "expectedValue and actualValue types should be compatible", "comparisonType must be valid operator (EqualTo, NotEqualTo, Contains, NotContain, StartsWith, EndsWith, GreaterThan, LessThan, GreaterOrEqual, LessOrEqual, Matches, NotMatches)", @@ -144,6 +152,8 @@ "category": "Control", "required_arguments": [], "optional_arguments": [], + "recommended_arguments": [], + "ide_emitted_arguments": [], "validation_rules": ["Must be inside a ForEach or While loop"], "best_practices": [ "Use Break sparingly - consider loop conditions instead", @@ -163,6 +173,8 @@ } ], "optional_arguments": [], + "recommended_arguments": [], + "ide_emitted_arguments": ["message"], "validation_rules": ["message should be descriptive"], "best_practices": ["Use for critical validation failures", "Provide clear, actionable failure messages"] }, @@ -173,6 +185,8 @@ "category": "Control", "required_arguments": [], "optional_arguments": [], + "recommended_arguments": [], + "ide_emitted_arguments": ["description"], "substeps": true, "validation_rules": ["Should be at end of test", "Typically contains cleanup operations"], "best_practices": [ @@ -188,17 +202,17 @@ "category": "Control", "required_arguments": [ { - "id": "valuePath", - "type": "variable", - "description": "Collection to iterate over" + "id": "list", + "type": "string" }, { - "id": "iterationPathName", - "type": "string", - "description": "Variable name for current item" + "id": "valueName", + "type": "string" } ], "optional_arguments": [], + "recommended_arguments": [], + "ide_emitted_arguments": ["list", "fromItem", "valueName", "continueOnFailure"], "substeps": true, "validation_rules": [ "valuePath must reference a valid list/collection", @@ -216,13 +230,7 @@ "apiId": "com.provar.plugins.bundled.apis.control.StepGroup", "description": "Organize related steps into logical groups", "category": "Control", - "required_arguments": [ - { - "id": "name", - "type": "string", - "description": "Group name" - } - ], + "required_arguments": [], "optional_arguments": [ { "id": "description", @@ -230,6 +238,8 @@ "description": "Detailed group description" } ], + "recommended_arguments": [], + "ide_emitted_arguments": ["description"], "substeps": true, "validation_rules": ["name should be descriptive", "Should contain multiple related steps"], "best_practices": ["Use for tests with 10+ steps", "Group by logical functionality", "Add clear descriptions"] @@ -247,6 +257,8 @@ } ], "optional_arguments": [], + "recommended_arguments": [], + "ide_emitted_arguments": ["condition"], "substeps": true, "clauses": ["then", "else"], "validation_rules": [ @@ -267,6 +279,8 @@ "category": "Control", "required_arguments": [], "optional_arguments": [], + "recommended_arguments": [], + "ide_emitted_arguments": ["values"], "child_elements": { "namedValues": { "description": "Container for namedValue elements", @@ -301,14 +315,9 @@ "description": "Sleep duration" } ], - "optional_arguments": [ - { - "id": "unit", - "type": "string", - "default": "seconds", - "description": "Time unit (seconds)" - } - ], + "optional_arguments": [], + "recommended_arguments": [], + "ide_emitted_arguments": ["sleepSecs"], "validation_rules": ["duration must be positive number", "Avoid long sleep durations"], "best_practices": [ "Prefer WaitFor over fixed Sleep", @@ -329,6 +338,8 @@ } ], "optional_arguments": [], + "recommended_arguments": [], + "ide_emitted_arguments": ["value"], "substeps": false, "clauses": ["case"], "structure": "Switch contains clause name='case' with steps containing multiple SwitchCase apiCalls", @@ -373,6 +384,8 @@ "description": "Internal reference to parent Switch value" } ], + "recommended_arguments": [], + "ide_emitted_arguments": ["value", "caseSensitive", "alreadyMatched", "switchValue"], "substeps": true, "clauses": ["steps"], "validation_rules": [ @@ -394,23 +407,14 @@ "category": "Control", "required_arguments": [], "optional_arguments": [ - { - "id": "duration", - "type": "decimal", - "description": "Maximum wait duration" - }, { "id": "condition", "type": "boolean", "description": "Condition to wait for" - }, - { - "id": "pollInterval", - "type": "decimal", - "default": "1", - "description": "How often to check condition (seconds)" } ], + "recommended_arguments": [], + "ide_emitted_arguments": ["condition", "testAtStart", "maxIterations", "sleepSecs", "continueOnFailure"], "validation_rules": ["Must specify duration OR condition", "duration should be reasonable (< 60 seconds)"], "best_practices": [ "Prefer condition-based over fixed duration", @@ -430,13 +434,9 @@ "description": "Loop condition" } ], - "optional_arguments": [ - { - "id": "maxIterations", - "type": "decimal", - "description": "Maximum loop iterations to prevent infinite loops" - } - ], + "optional_arguments": [], + "recommended_arguments": [], + "ide_emitted_arguments": ["condition", "counterName", "counterStart", "counterEnd", "continueOnFailure"], "substeps": true, "validation_rules": [ "condition must be boolean", @@ -476,17 +476,37 @@ "default": "true" }, { - "id": "commitBehavior", - "type": "string", - "default": "AfterTest" + "id": "commitBehaviour", + "type": "string" }, { "id": "ifAlreadyOpen", "type": "string", "default": "Fail" + }, + { + "id": "resultName", + "type": "string" + }, + { + "id": "resultScope", + "type": "string" } ], - "validation_rules": ["Should be first DB operation", "connectionName must be unique"], + "recommended_arguments": [], + "ide_emitted_arguments": [ + "connectionName", + "autoCommit", + "ifAlreadyOpen", + "commitBehaviour", + "resultName", + "resultScope" + ], + "validation_rules": [ + "Should be first DB operation", + "connectionName must be unique", + "The argument is spelled commitBehaviour (British spelling) β€” commitBehavior is not recognised." + ], "best_practices": ["Use connection pooling when possible", "Set appropriate commit behavior"] }, @@ -510,10 +530,13 @@ "type": "string" } ], + "recommended_arguments": [], + "ide_emitted_arguments": ["dbConnectionName", "tableName"], "validation_rules": [ "Must have DbConnect first", "tableName must be valid", - "Should have whereClause to avoid deleting all records" + "Should have whereClause to avoid deleting all records", + "Row selection is emitted as DYNAMICALLY-NAMED arguments prefixed with where__, one per predicate column β€” β€” NOT as a single whereClause argument." ], "best_practices": ["Always use WHERE clause", "Test with SELECT first"] }, @@ -539,10 +562,13 @@ "description": "Column name/value pairs" } ], + "recommended_arguments": [], + "ide_emitted_arguments": ["dbConnectionName", "tableName", "resultName", "resultScope"], "validation_rules": [ "Must have DbConnect first", "tableName must be valid", - "columnValues should match table schema" + "columnValues should match table schema", + "Column values are emitted as DYNAMICALLY-NAMED arguments, one per column β€” , β€” NOT as a single container argument." ], "best_practices": ["Validate data before insert", "Handle insert failures gracefully"] }, @@ -571,7 +597,13 @@ "type": "string" } ], - "validation_rules": ["Must have DbConnect first", "Should store results in variable"], + "recommended_arguments": [], + "ide_emitted_arguments": ["dbConnectionName", "tableName", "resultName", "resultScope"], + "validation_rules": [ + "Must have DbConnect first", + "Should store results in variable", + "Row selection is emitted as DYNAMICALLY-NAMED arguments prefixed with where__, one per predicate column β€” β€” NOT as a single whereClause argument." + ], "best_practices": ["Use WHERE clause to limit results", "Store in meaningful variable name"] }, @@ -599,7 +631,13 @@ "type": "string" } ], - "validation_rules": ["Must have DbConnect first", "Should have WHERE clause"], + "recommended_arguments": [], + "ide_emitted_arguments": ["dbConnectionName", "tableName"], + "validation_rules": [ + "Must have DbConnect first", + "Should have WHERE clause", + "Column values are emitted as DYNAMICALLY-NAMED arguments, one per column β€” , β€” NOT as a single container argument. Row selection is emitted as DYNAMICALLY-NAMED arguments prefixed with where__, one per predicate column β€” β€” NOT as a single whereClause argument." + ], "best_practices": ["Always use WHERE clause", "Verify update with SELECT"] }, @@ -624,6 +662,8 @@ "type": "string" } ], + "recommended_arguments": [], + "ide_emitted_arguments": ["dbConnectionName", "query", "resultName", "resultScope"], "validation_rules": ["Must have DbConnect first", "query must be valid SQL"], "best_practices": ["Use parameterized queries", "Store results for verification"] }, @@ -648,6 +688,8 @@ "type": "string" } ], + "recommended_arguments": [], + "ide_emitted_arguments": ["connectionName", "resultName", "resultScope"], "validation_rules": ["Required before RestRequest", "Required for Agentforce connections"], "best_practices": ["Reuse connections when possible", "Configure authentication properly"] }, @@ -658,33 +700,30 @@ "category": "Data", "required_arguments": [ { - "id": "webConnectionName", + "id": "connectionName", "type": "string" }, { - "id": "endpoint", + "id": "restResourceUrl", "type": "string" - }, - { - "id": "method", - "type": "string", - "description": "HTTP method (GET, POST, PUT, DELETE, PATCH)" } ], "optional_arguments": [ - { - "id": "body", - "type": "string" - }, - { - "id": "headers", - "type": "object" - }, { "id": "resultName", "type": "string" } ], + "recommended_arguments": [], + "ide_emitted_arguments": [ + "connectionName", + "targetValue", + "resultName", + "resultScope", + "statusResultName", + "restResourceUrl", + "restRequestHeader" + ], "validation_rules": [ "Must have WebConnect first", "method must be valid HTTP method", @@ -723,6 +762,19 @@ "type": "string" } ], + "recommended_arguments": [], + "ide_emitted_arguments": [ + "connectionName", + "wsdlLocation", + "service", + "port", + "operation", + "tnsPrefix", + "requestBody", + "resultName", + "resultScope", + "statusResultName" + ], "validation_rules": [ "Must have WebConnect first", "operation must be valid SOAP operation", @@ -737,11 +789,11 @@ "category": "Data", "required_arguments": [ { - "id": "connectionName", + "id": "destinationConnectionName", "type": "string" }, { - "id": "message", + "id": "body", "type": "string" } ], @@ -755,6 +807,8 @@ "type": "object" } ], + "recommended_arguments": [], + "ide_emitted_arguments": ["destinationConnectionName", "connectionId", "body", "payloadFormat"], "validation_rules": ["connectionName must be messaging connection", "message should be valid format"], "best_practices": ["Use for async testing", "Validate message format"] }, @@ -765,7 +819,7 @@ "category": "Data", "required_arguments": [ { - "id": "connectionName", + "id": "sourceConnectionName", "type": "string" } ], @@ -786,6 +840,16 @@ "type": "string" } ], + "recommended_arguments": [], + "ide_emitted_arguments": [ + "sourceConnectionName", + "connectionId", + "timeoutSecs", + "failOnTimeout", + "resultName", + "resultScope", + "resultFormat" + ], "validation_rules": ["timeout should be reasonable", "Store result for verification"], "best_practices": ["Set appropriate timeout", "Use selector to filter messages"] }, @@ -796,15 +860,11 @@ "category": "Data", "required_arguments": [ { - "id": "connectionName", - "type": "string" - }, - { - "id": "message", + "id": "destinationConnectionName", "type": "string" }, { - "id": "recipient", + "id": "body", "type": "string" } ], @@ -814,6 +874,8 @@ "type": "object" } ], + "recommended_arguments": ["to", "cc", "bcc", "subject", "signature"], + "ide_emitted_arguments": ["destinationConnectionName", "body", "payloadFormat", "attachments"], "validation_rules": ["recipient must be valid", "message format should be correct"], "best_practices": ["Validate recipient exists", "Handle send failures"] }, @@ -824,11 +886,7 @@ "category": "Data", "required_arguments": [ { - "id": "connectionName", - "type": "string" - }, - { - "id": "topic", + "id": "sourceConnectionName", "type": "string" } ], @@ -838,6 +896,8 @@ "type": "string" } ], + "recommended_arguments": ["folder", "from", "subject", "parseAttachment"], + "ide_emitted_arguments": ["sourceConnectionName", "resultName", "resultScope"], "validation_rules": ["topic must exist", "Subscribe before ReceiveMessage"], "best_practices": ["Unsubscribe in cleanup", "Handle subscription errors"] } @@ -858,6 +918,8 @@ } ], "optional_arguments": [], + "recommended_arguments": [], + "ide_emitted_arguments": ["description"], "substeps": true, "validation_rules": [ "Should be first BDD step in scenario", @@ -883,6 +945,8 @@ } ], "optional_arguments": [], + "recommended_arguments": [], + "ide_emitted_arguments": ["description"], "substeps": true, "validation_rules": ["Should follow Given step", "Description should describe action", "Must contain substeps"], "best_practices": [ @@ -904,6 +968,8 @@ } ], "optional_arguments": [], + "recommended_arguments": [], + "ide_emitted_arguments": ["description"], "substeps": true, "validation_rules": [ "Should follow When step", @@ -929,6 +995,8 @@ } ], "optional_arguments": [], + "recommended_arguments": [], + "ide_emitted_arguments": ["description"], "substeps": true, "validation_rules": [ "Should follow Given, When, or Then", @@ -954,6 +1022,8 @@ } ], "optional_arguments": [], + "recommended_arguments": [], + "ide_emitted_arguments": ["description"], "substeps": true, "validation_rules": [ "Should follow Given, When, or Then", @@ -973,12 +1043,13 @@ "category": "Design", "required_arguments": [ { - "id": "description", - "type": "string", - "description": "Actual result description" + "id": "result", + "type": "string" } ], "optional_arguments": [], + "recommended_arguments": [], + "ide_emitted_arguments": ["result"], "substeps": true, "validation_rules": ["description should be descriptive"], "best_practices": [ @@ -1010,6 +1081,8 @@ } ], "optional_arguments": [], + "recommended_arguments": [], + "ide_emitted_arguments": ["name", "description", "expectedResult"], "validation_rules": [ "Should be replaced with actual implementation", "description should describe intended action" @@ -1026,7 +1099,7 @@ "description": "AI-powered testing capabilities including Agentforce and test data generation", "AIAgentSession": { - "apiId": "com.provar.plugins.forcedotcom.core.testapis.ai.AIAgentSession", + "apiId": "com.provar.core.ai.api.AIAgentSession", "description": "Create Agentforce AI agent session", "category": "ProvarAI", "required_arguments": [ @@ -1054,6 +1127,8 @@ "description": "End session after creation" } ], + "recommended_arguments": [], + "ide_emitted_arguments": ["agentConnectionName", "agentId", "governmentCloud", "resultName", "resultScope"], "validation_rules": [ "Must have WebConnect step first", "agentId must be valid GUID format", @@ -1067,7 +1142,7 @@ }, "AIAgentConversation": { - "apiId": "com.provar.plugins.forcedotcom.core.testapis.ai.AIAgentConversation", + "apiId": "com.provar.core.ai.api.AIAgentConversation", "description": "Send message to AI agent and receive response", "category": "ProvarAI", "required_arguments": [ @@ -1098,6 +1173,16 @@ "default": "false" } ], + "recommended_arguments": [], + "ide_emitted_arguments": [ + "agentConnectionName", + "sessionID", + "message", + "resultName", + "resultScope", + "endSession", + "governmentCloud" + ], "validation_rules": [ "Must have AIAgentSession first", "sessionID must reference valid session", @@ -1111,7 +1196,7 @@ }, "GenerateUtterance": { - "apiId": "com.provar.plugins.forcedotcom.core.testapis.ai.GenerateUtterance", + "apiId": "com.provar.core.ai.api.GenerateUtterance", "description": "Generate AI-powered test utterances for intent testing", "category": "ProvarAI", "required_arguments": [ @@ -1139,6 +1224,8 @@ "default": "false" } ], + "recommended_arguments": [], + "ide_emitted_arguments": ["intent", "count", "resultName", "resultScope", "generateCsvFile"], "validation_rules": ["intent should be descriptive", "count should be reasonable (1-100)"], "best_practices": [ "Use for comprehensive intent testing", @@ -1148,7 +1235,7 @@ }, "IntentValidator": { - "apiId": "com.provar.plugins.forcedotcom.core.testapis.ai.IntentValidator", + "apiId": "com.provar.core.ai.api.IntentValidator", "description": "Validate that utterance matches expected intent", "category": "ProvarAI", "required_arguments": [ @@ -1175,6 +1262,8 @@ "default": "false" } ], + "recommended_arguments": [], + "ide_emitted_arguments": ["intent", "utterance", "resultName", "resultScope", "continueOnFailure"], "validation_rules": ["intent must be valid", "utterance should be test input"], "best_practices": [ "Use with GenerateUtterance", @@ -1184,30 +1273,39 @@ }, "GenerateTestData": { - "apiId": "com.provar.plugins.forcedotcom.core.testapis.generate.GenerateTestData", + "apiId": "com.provar.core.ai.api.GenerateTestData", "description": "AI-powered test data generation", "category": "ProvarAI", "required_arguments": [ { - "id": "dataType", - "type": "string", - "description": "Type of data to generate (email, name, address, etc.)" + "id": "apexConnectionName", + "type": "string" }, { - "id": "resultName", - "type": "string", - "description": "Variable to store generated data" + "id": "objectType", + "type": "string" } ], "optional_arguments": [ { - "id": "count", - "type": "decimal", - "default": "1", - "description": "Number of data items to generate" + "id": "totalRows", + "type": "string" + }, + { + "id": "resultIdName", + "type": "string" + }, + { + "id": "resultScope", + "type": "string" } ], - "validation_rules": ["dataType must be supported type", "count should be reasonable"], + "recommended_arguments": [], + "ide_emitted_arguments": ["apexConnectionName", "objectType", "totalRows", "resultIdName", "resultScope"], + "validation_rules": [ + "Row count is totalRows, not count (count belongs to GenerateUtterance).", + "The generated record id is stored in resultIdName; there is no dataType argument on this step." + ], "best_practices": [ "Use for realistic test data", "Validate generated data format", @@ -1216,7 +1314,7 @@ }, "ImageValidator": { - "apiId": "com.provar.plugins.forcedotcom.core.testapis.ai.ImageValidator", + "apiId": "com.provar.core.ai.api.ImageValidator", "description": "AI-powered image validation", "category": "ProvarAI", "required_arguments": [ @@ -1226,9 +1324,8 @@ "description": "Path to image file" }, { - "id": "validationCriteria", - "type": "string", - "description": "What to validate in image" + "id": "imageDescription", + "type": "string" }, { "id": "resultName", @@ -1241,8 +1338,15 @@ "type": "decimal", "default": "0.8", "description": "Minimum confidence score (0.0-1.0)" + }, + { + "id": "validationCriteria", + "type": "string", + "description": "What to validate in image" } ], + "recommended_arguments": [], + "ide_emitted_arguments": ["imagePath", "imageDescription", "resultName", "resultScope", "continueOnFailure"], "validation_rules": [ "imagePath must be valid file path", "validationCriteria should be specific", @@ -1263,18 +1367,7 @@ "apiId": "com.provar.plugins.forcedotcom.core.testapis.GenerateTestCase", "description": "AI-powered test case generation from scenario", "category": "ProvarLabs", - "required_arguments": [ - { - "id": "scenario", - "type": "string", - "description": "Test scenario description" - }, - { - "id": "resultName", - "type": "string", - "description": "Variable to store generated test case" - } - ], + "required_arguments": [], "optional_arguments": [ { "id": "template", @@ -1287,6 +1380,8 @@ "description": "Test framework (BDD, standard, etc.)" } ], + "recommended_arguments": [], + "ide_emitted_arguments": [], "validation_rules": ["scenario should be clear and detailed", "Review generated test before use"], "best_practices": [ "Use for rapid test creation", @@ -1299,13 +1394,7 @@ "apiId": "com.provar.plugins.bundled.apis.provarlabs.PageObjectCleaner", "description": "Clean up and optimize page object models", "category": "ProvarLabs", - "required_arguments": [ - { - "id": "pageObjectPath", - "type": "string", - "description": "Path to page object file" - } - ], + "required_arguments": [], "optional_arguments": [ { "id": "cleanupStrategy", @@ -1318,6 +1407,8 @@ "type": "string" } ], + "recommended_arguments": [], + "ide_emitted_arguments": [], "validation_rules": ["pageObjectPath must be valid", "Backup page objects before cleaning"], "best_practices": [ "Run periodically to maintain page objects", @@ -1367,6 +1458,28 @@ "default": "Test" } ], + "recommended_arguments": [], + "ide_emitted_arguments": [ + "connectionName", + "resultName", + "resultScope", + "uiApplicationName", + "quickUiLogin", + "closeAllPrimaryTabs", + "reuseConnectionName", + "alreadyOpenBehaviour", + "privateBrowsingMode", + "enableObjectIdLogging", + "autoCleanup", + "cleanupConnectionName", + "logFileLocation", + "username", + "password", + "securityToken", + "environment", + "lightningMode", + "webBrowser" + ], "validation_rules": [ "Should be first step when using Apex API", "connectionName must be unique", @@ -1399,17 +1512,20 @@ "description": "Salesforce object type (Account, Contact, etc.)" } ], - "optional_arguments": [ - { - "id": "fieldValues", - "type": "object", - "description": "Field values for the object" - } + "optional_arguments": [], + "recommended_arguments": [], + "ide_emitted_arguments": [ + "objectType", + "resultIdName", + "apexConnectionName", + "assignmentRuleId", + "resultScope" ], "validation_rules": [ "Must have ApexConnect first", "objectType must be valid SF object", - "Required fields must be provided" + "Required fields must be provided", + "Field values are emitted as DYNAMICALLY-NAMED arguments, one per field β€” , β€” NOT as a single container argument. There is no fieldValues/columnValues argument; inventing one produces a step Provar ignores." ], "best_practices": ["Store result ID for later use", "Provide all required fields", "Use for test data setup"] }, @@ -1437,13 +1553,9 @@ "description": "ID of object to read" } ], - "optional_arguments": [ - { - "id": "fields", - "type": "array", - "description": "Fields to retrieve" - } - ], + "optional_arguments": [], + "recommended_arguments": [], + "ide_emitted_arguments": ["objectType", "resultName", "apexConnectionName", "objectId", "resultScope"], "validation_rules": ["Must have ApexConnect first", "objectId must be valid 15 or 18 character SF ID"], "best_practices": ["Specify fields to retrieve", "Use for verification steps"] }, @@ -1466,13 +1578,21 @@ "type": "variable" } ], - "optional_arguments": [ - { - "id": "fieldValues", - "type": "object" - } + "optional_arguments": [], + "recommended_arguments": [], + "ide_emitted_arguments": [ + "objectType", + "resultName", + "apexConnectionName", + "objectId", + "assignmentRuleId", + "resultScope" + ], + "validation_rules": [ + "Must have ApexConnect first", + "objectId must be valid", + "Field values are emitted as DYNAMICALLY-NAMED arguments, one per field β€” , β€” NOT as a single container argument. There is no fieldValues/columnValues argument; inventing one produces a step Provar ignores." ], - "validation_rules": ["Must have ApexConnect first", "objectId must be valid"], "best_practices": ["Only update fields that changed", "Verify update with ApexReadObject"] }, @@ -1491,6 +1611,8 @@ } ], "optional_arguments": [], + "recommended_arguments": [], + "ide_emitted_arguments": ["objectId", "apexConnectionName"], "validation_rules": ["Must have ApexConnect first", "Use only if autoCleanup=false"], "best_practices": [ "Prefer autoCleanup over manual delete", @@ -1526,6 +1648,8 @@ "default": "Test" } ], + "recommended_arguments": [], + "ide_emitted_arguments": ["soqlQuery", "apexConnectionName", "resultListName", "resultScope"], "validation_rules": [ "Must have ApexConnect first", "SOQL must have SELECT and FROM clauses", @@ -1562,6 +1686,8 @@ "description": "Variable to store execution results" } ], + "recommended_arguments": [], + "ide_emitted_arguments": ["apexConnectionName", "apexBlock", "results"], "validation_rules": ["Must have ApexConnect first", "apexBlock must be valid Apex syntax"], "best_practices": [ "Use for complex operations not available via API", @@ -1589,16 +1715,18 @@ "description": "Bulk operation (insert, update, delete, etc.)" } ], - "optional_arguments": [ - { - "id": "records", - "type": "array", - "description": "Records to process" - }, - { - "id": "resultName", - "type": "string" - } + "optional_arguments": [], + "recommended_arguments": [], + "ide_emitted_arguments": [ + "apexConnectionName", + "objectType", + "operation", + "externalIdFieldName", + "assignmentRuleId", + "data", + "contentType", + "resultIdName", + "resultScope" ], "validation_rules": [ "Must have ApexConnect first", @@ -1632,6 +1760,8 @@ "type": "string" } ], + "recommended_arguments": [], + "ide_emitted_arguments": ["apexConnectionName", "workItemId", "comments"], "validation_rules": ["Must have ApexConnect first", "workItemId must be valid"], "best_practices": ["Add approval comments", "Verify approval status after"] }, @@ -1660,6 +1790,15 @@ "type": "string" } ], + "recommended_arguments": [], + "ide_emitted_arguments": [ + "apexConnectionName", + "objectId", + "comments", + "approver", + "resultWorkItemIdsName", + "resultScope" + ], "validation_rules": ["Must have ApexConnect first", "Object must have approval process"], "best_practices": ["Verify record meets approval criteria", "Track approval process status"] }, @@ -1705,6 +1844,23 @@ "type": "string" } ], + "recommended_arguments": [], + "ide_emitted_arguments": [ + "apexConnectionName", + "leadId", + "accountId", + "contactId", + "ownerId", + "convertedStatus", + "createOpportunity", + "opportunityName", + "overwriteLeadSource", + "sendNotificationEmail", + "resultAccountIdName", + "resultContactIdName", + "resultOpportunityIdName", + "resultScope" + ], "validation_rules": ["Must have ApexConnect first", "convertedStatus must be valid lead status"], "best_practices": ["Verify lead meets conversion criteria", "Store converted IDs for verification"] }, @@ -1740,6 +1896,15 @@ "description": "File path to save layout" } ], + "recommended_arguments": [], + "ide_emitted_arguments": [ + "apexConnectionName", + "objectName", + "extractRecordPageLayout", + "extractDynamicFormLayout", + "includeFieldPermissions", + "dataUrl" + ], "validation_rules": ["Must have ApexConnect first", "objectName must be valid"], "best_practices": ["Use for layout validation tests", "Save layouts for regression testing"] }, @@ -1775,6 +1940,19 @@ "default": "false" } ], + "recommended_arguments": [], + "ide_emitted_arguments": [ + "apexConnectionName", + "objectName", + "extractRecordPageLayout", + "extractDynamicFormLayout", + "dataUrl", + "includeFieldPermissions", + "reportUnmatchedExpectedRecordTypes", + "reportUnmatchedActualRecordTypes", + "reportUnmatchedExpectedFields", + "reportUnmatchedActualFields" + ], "validation_rules": [ "Must have ApexConnect first", "Must have ExtractSalesforceLayout first", @@ -1805,6 +1983,8 @@ "description": "Maximum records to log" } ], + "recommended_arguments": [], + "ide_emitted_arguments": ["apexConnectionName", "selector", "maxMatches"], "validation_rules": [ "Must have ApexConnect first with enableObjectIdLogging=true", "selector must be valid SOQL", @@ -1836,20 +2016,10 @@ "id": "connectionId", "type": "string" }, - { - "id": "uiApplicationName", - "type": "string", - "default": "LightningSales" - }, { "id": "reuseConnectionName", "type": "string" }, - { - "id": "closeAllOtherTabs", - "type": "boolean", - "default": "true" - }, { "id": "privateBrowsingMode", "type": "boolean", @@ -1861,7 +2031,21 @@ "default": "Chrome_Headless" } ], - "validation_rules": ["Should be first UI step", "connectionName must be unique"], + "recommended_arguments": [], + "ide_emitted_arguments": [ + "connectionName", + "reuseConnectionName", + "privateBrowsingMode", + "resultName", + "resultScope", + "webBrowser" + ], + "validation_rules": [ + "Browser-only: UiConnect opens a UI session but has NO Salesforce credential of its own β€” it reuses an existing ApexConnect result. For a Salesforce UI test use ApexConnect (with quickUiLogin=true, uiApplicationName=LightningSales), NOT UiConnect (rule SF-CONNECT-TYPE-001).", + "Add UiConnect only for a SECOND, browser-only session alongside an ApexConnect.", + "connectionName must be unique", + "Downstream steps reference this connection via uiConnectionName = this step's resultName (defaults to connectionName)." + ], "best_practices": [ "Use headless mode for CI/CD", "Reuse connections when possible", @@ -1897,10 +2081,23 @@ } ], "optional_arguments": [ - { "id": "reuseConnectionName", "type": "string" }, - { "id": "privateBrowsingMode", "type": "boolean", "default": "false" }, - { "id": "webBrowser", "type": "string", "default": "Chrome_Headless" } + { + "id": "reuseConnectionName", + "type": "string" + }, + { + "id": "privateBrowsingMode", + "type": "boolean", + "default": "false" + }, + { + "id": "webBrowser", + "type": "string", + "default": "Chrome_Headless" + } ], + "recommended_arguments": [], + "ide_emitted_arguments": [], "validation_rules": [ "Use INSTEAD of UiConnect/ApexConnect for Microsoft Dynamics 365 targets", "resultName is required so downstream UI steps can set uiConnectionName", @@ -1936,10 +2133,23 @@ } ], "optional_arguments": [ - { "id": "reuseConnectionName", "type": "string" }, - { "id": "privateBrowsingMode", "type": "boolean", "default": "false" }, - { "id": "webBrowser", "type": "string", "default": "Chrome_Headless" } + { + "id": "reuseConnectionName", + "type": "string" + }, + { + "id": "privateBrowsingMode", + "type": "boolean", + "default": "false" + }, + { + "id": "webBrowser", + "type": "string", + "default": "Chrome_Headless" + } ], + "recommended_arguments": [], + "ide_emitted_arguments": [], "validation_rules": [ "Use INSTEAD of UiConnect/ApexConnect for Microsoft Dataverse targets", "resultName is required so downstream UI steps can set uiConnectionName", @@ -1979,10 +2189,23 @@ } ], "optional_arguments": [ - { "id": "reuseConnectionName", "type": "string" }, - { "id": "privateBrowsingMode", "type": "boolean", "default": "false" }, - { "id": "webBrowser", "type": "string", "default": "Chrome_Headless" } + { + "id": "reuseConnectionName", + "type": "string" + }, + { + "id": "privateBrowsingMode", + "type": "boolean", + "default": "false" + }, + { + "id": "webBrowser", + "type": "string", + "default": "Chrome_Headless" + } ], + "recommended_arguments": [], + "ide_emitted_arguments": [], "validation_rules": [ "Use INSTEAD of UiConnect/ApexConnect for Microsoft Power Apps targets", "resultName is required so downstream UI steps can set uiConnectionName", @@ -2028,10 +2251,23 @@ } ], "optional_arguments": [ - { "id": "reuseConnectionName", "type": "string" }, - { "id": "privateBrowsingMode", "type": "boolean", "default": "false" }, - { "id": "webBrowser", "type": "string", "default": "Chrome_Headless" } + { + "id": "reuseConnectionName", + "type": "string" + }, + { + "id": "privateBrowsingMode", + "type": "boolean", + "default": "false" + }, + { + "id": "webBrowser", + "type": "string", + "default": "Chrome_Headless" + } ], + "recommended_arguments": [], + "ide_emitted_arguments": [], "validation_rules": [ "Use INSTEAD of UiConnect/ApexConnect for Microsoft Power Pages targets", "resultName is required so downstream UI steps can set uiConnectionName", @@ -2049,11 +2285,6 @@ "description": "Navigate to and interact with Salesforce screen", "category": "UI", "required_arguments": [ - { - "id": "screenName", - "type": "string", - "description": "Screen name (e.g., 'Account Home', 'Account New')" - }, { "id": "uiConnectionName", "type": "string" @@ -2101,6 +2332,18 @@ "description": "Record ID to edit/view (required for Edit/View screens when navigate=Always)" } ], + "recommended_arguments": [], + "ide_emitted_arguments": [ + "uiConnectionName", + "target", + "targetDescription", + "navigate", + "windowSelection", + "windowSize", + "closeWindow", + "captureBefore", + "captureAfter" + ], "substeps": true, "naming_convention": "On {ObjectName} {ScreenType} screen", "validation_rules": [ @@ -2150,10 +2393,22 @@ "default": "false" } ], + "recommended_arguments": [], + "ide_emitted_arguments": [ + "locator", + "interaction", + "interactionDescription", + "beforeWait", + "afterWait", + "autoRetry", + "captureBefore", + "captureAfter" + ], "parent_required": "UiWithScreen", "validation_rules": [ "Must be inside UiWithScreen substeps", - "value required for set/select interactions", + "interaction name must be a real Provar interaction. The most common are action (activate a button or link), set (fill a field or pick a picklist value), click, check/uncheck, and file (upload); Provar ships many more (doubleClick, hover, invoke, clear, toggle, sfLookup, sfSelect, …), so treat this as a common subset, not an exhaustive list. Do NOT invent names borrowed from other frameworks β€” type, fill, enter, input, tap and press are not Provar interactions and render the IDE Action widget blank (rule UI-INTERACTION-002); use set for typing into a field and action or click to activate a control.", + "value required for set interactions (a literal or a class=variable reference); omit value for action interactions", "Picklist: use ui:interaction?name=set", "Lead Conversion buttons: MUST use name=submitConvert and action=submitConvert (NOT Convert, ConvertButton, or action=convert)", "Dialog fields (e.g., OpportunityName): MUST bind to the TARGET object (object=Opportunity with field=name, NOT object=Lead with field=OpportunityName)" @@ -2192,29 +2447,63 @@ "category": "UI", "required_arguments": [ { - "id": "locator", - "type": "uiLocator" + "id": "fieldAssertions", + "type": "valueList", + "description": "Field-level assertions as namedValues (empty valueList when unused)" }, { - "id": "assertionType", - "type": "string", - "description": "Type of assertion (EqualTo, Contains, etc.)" - } - ], - "optional_arguments": [ + "id": "columnAssertions", + "type": "valueList", + "description": "Column-level assertions (empty valueList when unused)" + }, { - "id": "expectedValue", - "type": "value" + "id": "pageAssertions", + "type": "valueList", + "description": "Page-level assertions (empty valueList when unused)" }, { - "id": "continueOnFailure", + "id": "resultScope", + "type": "string", + "default": "Test" + }, + { + "id": "captureAfter", "type": "boolean", "default": "false" + }, + { + "id": "beforeWait", + "type": "string", + "description": "Include even if empty ()" + }, + { + "id": "autoRetry", + "type": "string", + "description": "Include even if empty ()" } ], + "optional_arguments": [ + { + "id": "resultName", + "type": "string" + } + ], + "recommended_arguments": [], + "ide_emitted_arguments": [ + "fieldAssertions", + "columnAssertions", + "pageAssertions", + "resultName", + "resultScope", + "captureAfter", + "beforeWait", + "autoRetry" + ], "parent_required": "UiWithScreen", "validation_rules": [ "Must be inside UiWithScreen substeps", + "fieldAssertions MUST nest a containing a BARE (NOT class=\"uiLocator\") and an with a holding the expected . Do NOT use a / block (that is the UiFill/SetValues shape) or flat fieldLocator/attributeName arguments β€” either renders the IDE Result Assertions tab blank (rule UI-ASSERT-STRUCTURE-001).", + "comparisonType (UI Assert) must be one of: EqualTo, Contains, StartsWith, EndsWith, Matches, None.", "Currency fields: format as $1,000.00", "Phone fields: format as (123) 456-7890", "Date/time: use Contains or EqualTo based on format" @@ -2230,19 +2519,10 @@ "apiId": "com.provar.plugins.forcedotcom.core.ui.UiNavigate", "description": "Navigate to URL", "category": "UI", - "required_arguments": [ - { - "id": "url", - "type": "string", - "description": "URL to navigate to" - } - ], - "optional_arguments": [ - { - "id": "uiConnectionName", - "type": "string" - } - ], + "required_arguments": [], + "optional_arguments": [], + "recommended_arguments": [], + "ide_emitted_arguments": ["navigate", "captureBefore", "captureAfter"], "validation_rules": ["url must be valid", "Use for non-Salesforce sites or direct URLs"], "best_practices": [ "Prefer UiWithScreen for Salesforce navigation", @@ -2255,22 +2535,31 @@ "apiId": "com.provar.plugins.forcedotcom.core.ui.UiFill", "description": "Auto-fill form fields", "category": "UI", - "required_arguments": [ - { - "id": "formLocator", - "type": "uiLocator", - "description": "Form container locator" - } - ], + "required_arguments": [], + "required_one_of": [["values", "locator"]], "optional_arguments": [ { - "id": "fieldValues", - "type": "object", - "description": "Field name/value pairs" + "id": "locator", + "type": "string" + }, + { + "id": "values", + "type": "string" + }, + { + "id": "ignoreFields", + "type": "string" } ], + "recommended_arguments": [], + "ide_emitted_arguments": ["values", "ignoreFields"], "parent_required": "UiWithScreen", - "validation_rules": ["Must be inside UiWithScreen substeps", "Field names must match form fields"], + "validation_rules": [ + "Must be inside UiWithScreen substeps", + "Field names must match form fields", + "Supply EITHER values (a namedValues valueList of field name/value pairs) OR locator (a single field to fill). A UiFill with neither does nothing at runtime.", + "There is no fieldValues or value argument on UiFill β€” those names appear in no real Provar test case." + ], "best_practices": ["Use for multi-field forms", "Verify all fields filled", "Handle required fields first"] }, @@ -2280,17 +2569,28 @@ "category": "UI", "required_arguments": [ { - "id": "tableLocator", + "id": "locator", "type": "uiLocator", "description": "Table/list locator" }, { - "id": "rowSelector", - "type": "string", - "description": "Row selection criteria" + "id": "uiRowLocator", + "type": "uiLocator", + "description": "Row selection locator" } ], "optional_arguments": [], + "recommended_arguments": [], + "ide_emitted_arguments": [ + "uiConnectionName", + "locator", + "uiRowLocator", + "locatorDescription", + "failIfNotFound", + "debugRowLocator", + "resultName", + "resultScope" + ], "substeps": true, "parent_required": "UiWithScreen", "validation_rules": [ @@ -2311,18 +2611,14 @@ "category": "UI", "required_arguments": [ { - "id": "action", - "type": "string", - "description": "Action to take (Accept, Dismiss, GetText)" - } - ], - "optional_arguments": [ - { - "id": "resultName", - "type": "string", - "description": "Variable to store alert text" + "id": "alerts", + "type": "valueList", + "description": "Alert handling entries (action per alert: Accept, Dismiss, GetText)" } ], + "optional_arguments": [], + "recommended_arguments": [], + "ide_emitted_arguments": ["alerts", "captureBefore", "captureAfter"], "validation_rules": ["action must be valid (Accept, Dismiss, GetText)", "Store text when using GetText"], "best_practices": ["Handle alerts promptly", "Validate alert text", "Use in Try/Catch for optional alerts"] } @@ -2337,9 +2633,8 @@ "category": "Utility", "required_arguments": [ { - "id": "dataUrl", - "type": "string", - "description": "Path to Excel/CSV file" + "id": "urlString", + "type": "string" }, { "id": "resultName", @@ -2365,6 +2660,15 @@ "description": "Where column names are located" } ], + "recommended_arguments": [], + "ide_emitted_arguments": [ + "urlString", + "contentFormat", + "contentEncoding", + "resultName", + "resultScope", + "storeAs" + ], "validation_rules": [ "dataUrl must be valid file path", "File must exist and be readable", @@ -2403,6 +2707,8 @@ "type": "string" } ], + "recommended_arguments": [], + "ide_emitted_arguments": ["data", "dataUrl", "targetDescription"], "validation_rules": ["data must be valid variable", "dataUrl must be writable path"], "best_practices": ["Use for test result recording", "Ensure output directory exists", "Handle write failures"] }, @@ -2418,9 +2724,8 @@ "description": "Input string" }, { - "id": "searchString", - "type": "string", - "description": "Text to find" + "id": "find", + "type": "string" }, { "id": "replacement", @@ -2432,19 +2737,9 @@ "type": "string" } ], - "optional_arguments": [ - { - "id": "caseSensitive", - "type": "boolean", - "default": "true" - }, - { - "id": "replaceAll", - "type": "boolean", - "default": "true", - "description": "Replace all occurrences vs first only" - } - ], + "optional_arguments": [], + "recommended_arguments": [], + "ide_emitted_arguments": ["input", "find", "replacement", "caseInsensitive", "resultName", "resultScope"], "validation_rules": ["searchString should not be empty", "Store result in variable"], "best_practices": ["Use for string formatting", "Consider regex for complex patterns", "Test with edge cases"] }, @@ -2460,9 +2755,8 @@ "description": "String to split" }, { - "id": "delimiter", - "type": "string", - "description": "Delimiter character/string" + "id": "separator", + "type": "string" }, { "id": "resultName", @@ -2470,13 +2764,9 @@ "description": "Variable to store array" } ], - "optional_arguments": [ - { - "id": "maxSplits", - "type": "decimal", - "description": "Maximum number of splits" - } - ], + "optional_arguments": [], + "recommended_arguments": [], + "ide_emitted_arguments": ["input", "separator", "resultName", "resultScope"], "validation_rules": ["delimiter should not be empty", "Store result as list"], "best_practices": ["Use for parsing delimited data", "Iterate results with ForEach", "Handle empty splits"] }, @@ -2492,28 +2782,17 @@ "description": "Input string to match against" }, { - "id": "pattern", - "type": "string", - "description": "Regex pattern or match string" + "id": "regex", + "type": "string" }, { "id": "resultName", "type": "string" } ], - "optional_arguments": [ - { - "id": "isRegex", - "type": "boolean", - "default": "false", - "description": "Treat pattern as regex" - }, - { - "id": "caseSensitive", - "type": "boolean", - "default": "true" - } - ], + "optional_arguments": [], + "recommended_arguments": [], + "ide_emitted_arguments": ["input", "regex", "caseInsensitive", "resultName", "resultScope"], "validation_rules": ["pattern must be valid regex if isRegex=true", "Store match results"], "best_practices": [ "Use for validation and extraction", @@ -2536,11 +2815,6 @@ "id": "list2", "type": "variable", "description": "Second list" - }, - { - "id": "resultName", - "type": "string", - "description": "Variable to store comparison results" } ], "optional_arguments": [ @@ -2551,6 +2825,18 @@ "description": "Comparison type (Equal, Contains, etc.)" } ], + "recommended_arguments": [], + "ide_emitted_arguments": [ + "list1", + "list2", + "matchColumns", + "compareColumns", + "ignoreColumns", + "differenceHandling", + "unmatched1Handling", + "unmatched2Handling", + "caseInsensitive" + ], "validation_rules": ["Both lists must be valid", "Store comparison result"], "best_practices": [ "Use for data validation", @@ -2678,8 +2964,8 @@ "category": "UI", "steps": [ { - "step": "UiConnect", - "purpose": "Establish UI connection" + "step": "ApexConnect (quickUiLogin=true, uiApplicationName=LightningSales)", + "purpose": "Establish the Salesforce connection (API + Lightning UI). Use ApexConnect for Salesforce UI tests β€” NOT UiConnect, which is browser-only and needs an existing ApexConnect result as its credential. Downstream UI steps set uiConnectionName to this step's resultName (which defaults to connectionName)." }, { "step": "UiWithScreen (navigate=Always)", diff --git a/src/mcp/server.ts b/src/mcp/server.ts index 309d3e0..546b996 100644 --- a/src/mcp/server.ts +++ b/src/mcp/server.ts @@ -35,6 +35,7 @@ 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 { registerAllStepSchemaTools } from './tools/stepSchemaTools.js'; import { registerAllPrompts } from './prompts/index.js'; import { createDepthGuardState, @@ -49,7 +50,18 @@ import { desc } from './tools/descHelper.js'; const TOOL_GROUPS: Record void>> = { nitrox: [registerAllNitroXTools], automation: [registerAllAutomationTools], - qualityhub: [registerAllQualityHubTools, registerAllQualityHubApiTools, registerAllDefectTools], + // provar_step_schema is read-only, offline, and named as the recovery path by + // guidance in three different groups: provar_testcase_validate's structural-error + // text (validation), examples_retrieve's no-key fallback (qualityhub), and the + // generator (authoring). Registering it in each means that guidance resolves to a + // tool the session actually exposes, instead of naming one it does not β€” the same + // dead-end this change set out to remove, just relocated. + qualityhub: [ + registerAllQualityHubTools, + registerAllQualityHubApiTools, + registerAllDefectTools, + registerAllStepSchemaTools, + ], validation: [ registerProjectValidateFromPath, registerAllAntTools, @@ -58,12 +70,14 @@ const TOOL_GROUPS: Record void>(); for (const [group, registrars] of Object.entries(TOOL_GROUPS)) { if (activeGroups === null || activeGroups.has(group)) { for (const register of registrars) { + if (alreadyRegistered.has(register)) continue; + alreadyRegistered.add(register); register(server, config); } } diff --git a/src/mcp/tools/bestPracticesEngine.ts b/src/mcp/tools/bestPracticesEngine.ts index 54b15c2..67d2cbe 100644 --- a/src/mcp/tools/bestPracticesEngine.ts +++ b/src/mcp/tools/bestPracticesEngine.ts @@ -70,6 +70,26 @@ export interface BPViolation { * empty means "no stable id" β€” such violations are never merged together. */ test_item_id?: string; + /** + * Volatile, human-facing detail for aggregated violations: how many steps are + * affected, the per-argument breakdown, and a sample of offending steps. + * + * Deliberately kept OUT of `message`, because validationDiff keys baseline + * comparisons on the rendered message β€” putting counts or step names there makes + * every partial fix look like one finding resolved and a different one added. + */ + details?: Record; + /** + * Stable identity for baseline diffing, used INSTEAD of the rendered message when + * present (see validationDiff.violationKey). + * + * Aggregate rules emit one violation per file whose message necessarily changes as + * steps are fixed. Without a separate identity the diff treats each revision as a + * different finding and reports a resolution that has not happened. Set it to a + * value that is constant for "this rule, this file" β€” the message then stays free to + * carry the specifics a caller needs to act on. + */ + diff_identity?: string; } export interface BPEngineResult { @@ -151,12 +171,24 @@ const VALID_API_IDS = new Set([ 'com.provar.plugins.bundled.apis.list.ListCompare', // Provar Labs 'com.provar.plugins.bundled.apis.provarlabs.PageObjectCleaner', - // Forcedotcom β€” AI / agent - 'com.provar.plugins.forcedotcom.core.testapis.ai.AIAgentSession', - 'com.provar.plugins.forcedotcom.core.testapis.ai.AIAgentConversation', - 'com.provar.plugins.forcedotcom.core.testapis.ai.GenerateUtterance', - 'com.provar.plugins.forcedotcom.core.testapis.ai.IntentValidator', - 'com.provar.plugins.forcedotcom.core.testapis.ai.ImageValidator', + // Provar AI β€” current namespace. Verified against a freshly IDE-minted test case + // (2026): the IDE writes `com.provar.core.ai.api.*`. The previously-listed + // `...forcedotcom.core.testapis.ai.*` form appears in no real file anywhere and was + // never valid β€” listing it here made API-UNKNOWN-001 (critical, weight 10) reject + // genuine IDE-authored XML as "does not exist in Provar", gating is_valid=false. + 'com.provar.core.ai.api.AIAgentSession', + 'com.provar.core.ai.api.AIAgentConversation', + 'com.provar.core.ai.api.GenerateUtterance', + 'com.provar.core.ai.api.IntentValidator', + 'com.provar.core.ai.api.ImageValidator', + 'com.provar.core.ai.api.GenerateTestData', + // Provar AI β€” legacy namespace, kept for backward compatibility: pre-rollout files + // in the corpus still carry these (AIAgentConversation x8, IntentValidator x8, + // GenerateTestData x18, AIAgentSession x3, GenerateUtterance x3). + 'com.provar.plugins.forcedotcom.core.testapis.generate.AIAgentSession', + 'com.provar.plugins.forcedotcom.core.testapis.generate.AIAgentConversation', + 'com.provar.plugins.forcedotcom.core.testapis.generate.GenerateUtterance', + 'com.provar.plugins.forcedotcom.core.testapis.generate.IntentValidator', 'com.provar.plugins.forcedotcom.core.testapis.generate.GenerateTestData', 'com.provar.plugins.forcedotcom.core.testapis.GenerateTestCase', // Forcedotcom β€” Apex / API @@ -1353,22 +1385,102 @@ function argumentHasMeaningfulValue(arg: XmlNode): boolean { continue; } if (typeof value !== 'object') continue; - const v = value; - const vClass = (v['@_class'] as string | undefined) ?? ''; - // nodeText coerces a numeric #text to string first β€” a bare `.trim()` throws on it. - const text = nodeText(v); + if (valueNodeIsMeaningful(value)) return true; + } + return false; +} - if (vClass === 'variable') { - if (v['path'] != null || text.length > 0) return true; - continue; // bare β€” not meaningful +/** + * Class-aware content check for a single `` node. Split out of + * {@link argumentHasMeaningfulValue} to keep that function within the complexity + * budget as the class table grew. + */ +function valueNodeIsMeaningful(v: XmlNode): boolean { + const vClass = (v['@_class'] as string | undefined) ?? ''; + // nodeText coerces a numeric #text to string first β€” a bare `.trim()` throws on it. + const text = nodeText(v); + + if (vClass === 'variable') { + // Quality Hub parity: the presence of a makes the reference meaningful. + return v['path'] != null || text.length > 0; + } + if (vClass === 'funcCall' || MEANINGFUL_VALUE_OPERATOR_CLASSES.has(vClass)) return true; + if (vClass === 'compound') { + const parts = v['parts']; + return Boolean(parts && typeof parts === 'object' && Object.keys(parts).some((k) => !k.startsWith('@_'))); + } + return text.length > 0; +} + +/** + * Schema-tier content check for `required_one_of`. + * + * Layered ON TOP of {@link argumentHasMeaningfulValue} rather than folded into it. + * That helper mirrors the Quality Hub `MustContainArgumentValidator` exactly, and the + * `mustContainArgument` rules share it β€” so widening it changes Layer-2 scoring and + * breaks the parity that makes local and API results agree. An earlier revision did + * exactly that: a `uiLocator` with text but no `uri` flipped to empty, and a uri-only + * one flipped to meaningful, with no backend evidence for either. The corpus could not + * detect the divergence because no real file carries those shapes in a rule-targeted + * argument, which is precisely why it must not be assumed safe. + * + * The additions here are OURS, used only by the schema tier this branch introduces: + * URI-bearing classes, checked per class so a plain `value` cannot claim a `uri`; and + * `valueList` / `namedValues` containers, meaningful only when an entry inside is. + */ +function schemaArgumentHasValue(arg: XmlNode): boolean { + // Class-specific rules run FIRST. Delegating to the parity helper up front was a + // bypass: that helper accepts non-empty text for any class it does not recognise, so + // `junk` satisfied the group without the uri the + // class actually requires, and the stricter check below never ran. + for (const value of toArr(arg['value'] as XmlNode | string | Array)) { + if (value == null || typeof value !== 'object') continue; + const vClass = (value['@_class'] as string | undefined) ?? ''; + if (URI_PAYLOAD_VALUE_CLASSES.has(vClass)) { + const uri = value['@_uri']; + if (typeof uri === 'string' && uri.trim().length > 0) return true; + continue; // text does not substitute for the uri this class carries its payload in } - if (vClass === 'funcCall' || MEANINGFUL_VALUE_OPERATOR_CLASSES.has(vClass)) return true; - if (vClass === 'compound') { - const parts = v['parts']; - if (parts && typeof parts === 'object' && Object.keys(parts).some((k) => !k.startsWith('@_'))) return true; + if (vClass === 'valueList' || value['namedValues'] != null) { + if (containerHasMeaningfulEntry(value)) return true; + continue; // stray text inside a container is not a populated entry + } + // Any other class: defer to the unchanged Quality Hub parity contract. + if (argumentHasMeaningfulValue({ value } as XmlNode)) return true; + } + return false; +} + +/** Value classes whose payload lives in the `uri` attribute rather than in text. */ +const URI_PAYLOAD_VALUE_CLASSES: ReadonlySet = new Set(['uiLocator', 'uiTarget', 'uiInteraction', 'uiWait']); + +/** + * True when a `valueList` / `namedValues` container holds at least one entry that + * itself carries a value. A `` names a destination without + * assigning anything, so a container of those is as inert as an empty one. + */ +function containerHasMeaningfulEntry(container: XmlNode, depth = 0): boolean { + if (depth > 6) return false; + for (const [key, child] of Object.entries(container)) { + if (key.startsWith('@_') || child == null) continue; + if (key === '#text') { + // The container's OWN text is not an entry β€” a valueList holds namedValues, so + // `stray text` carries nothing usable. Text on a + // nested entry (a namedValue's assigned value) does count, hence the depth check. + if (depth > 0 && String(child).trim().length > 0) return true; continue; } - if (text.length > 0) return true; + for (const entry of toArr(child as XmlNode | string | Array)) { + if (entry == null) continue; + if (typeof entry === 'string') { + if (entry.trim().length > 0) return true; + continue; + } + if (typeof entry !== 'object') continue; + // A nested is judged by the same class-aware rules. + if (entry['value'] != null && argumentHasMeaningfulValue(entry)) return true; + if (containerHasMeaningfulEntry(entry, depth + 1)) return true; + } } return false; } @@ -2304,6 +2416,258 @@ function validateUiAssertMissingArguments(tc: XmlNode, rule: BPRule): BPViolatio ); } +// STEP-REQUIRED-ARGS-001 β€” schema-driven required-argument check for ALL step types. +// +// The dedicated hardcoded UiAssert rule (UI-ASSERT-STRUCT-001, above) only ever fired +// for UiAssert, so a test case could omit required arguments on UiConnect, UiWithScreen, +// UiDoAction, etc. and still score 100/100. This rule derives the required-argument set +// for every apiId from provar_test_step_schema.json and flags any step missing one β€” the +// same data source read by the provar://schema/test-step resource, so the validator and +// the schema reference can never drift apart. It runs at `major` severity (docks +// quality_score, does not gate is_valid), because the schema is a doc-derived reference +// rather than the authoritative Quality Hub ruleset. +// +// Step types with a dedicated required-argument rule are excluded here to avoid +// double-reporting: UiAssert (UI-ASSERT-STRUCT-001) and the NitroX MS connect variants +// (UI-NITROX-VARIANT-ARG-001). +const REQUIRED_ARGS_EXCLUDED_API_IDS: ReadonlySet = new Set([UI_ASSERT_API_ID]); + +// Categories whose argument contracts are still moving. Descriptive schema entries are +// retained (provar_step_schema can still answer questions about them) but nothing in +// them is scored, so experimental steps do not generate churn. +const UNSCORED_SCHEMA_CATEGORIES: ReadonlySet = new Set(['ProvarLabs']); + +interface SchemaArgTiers { + category: string; + required: readonly string[]; + recommended: readonly string[]; + ideEmitted: readonly string[]; + /** + * Groups where at least ONE member must be present. Used where no single argument + * is individually load-blocking but the step is inert without any of them β€” e.g. + * UiFill needs either `values` (a namedValues list) or `locator` (a single field). + */ + requiredOneOf: ReadonlyArray; +} + +/** Build apiId β†’ argument tiers from the bundled step schema. Cached after first read. */ +let schemaArgTiers: Map | null = null; +function getSchemaArgTiers(): Map { + if (schemaArgTiers) return schemaArgTiers; + const map = new Map(); + const ids = (v: unknown): string[] => + Array.isArray(v) + ? v + .map((r) => (typeof r === 'string' ? r : r && typeof r === 'object' ? (r as { id?: unknown }).id : undefined)) + .filter((id): id is string => typeof id === 'string') + : []; + try { + const raw = readFileSync(join(dirPath, '..', 'rules', 'provar_test_step_schema.json'), 'utf-8'); + const parsed = JSON.parse(raw) as { apiCalls?: Record }; + for (const [categoryName, category] of Object.entries(parsed.apiCalls ?? {})) { + if (!category || typeof category !== 'object') continue; + for (const entry of Object.values(category as Record)) { + if (!entry || typeof entry !== 'object') continue; + const e = entry as { + apiId?: unknown; + category?: unknown; + required_arguments?: unknown; + recommended_arguments?: unknown; + ide_emitted_arguments?: unknown; + required_one_of?: unknown; + }; + if (typeof e.apiId !== 'string') continue; + const oneOf = Array.isArray(e.required_one_of) + ? e.required_one_of + .filter(Array.isArray) + .map((g) => (g as unknown[]).filter((x): x is string => typeof x === 'string')) + : []; + map.set(e.apiId, { + category: typeof e.category === 'string' ? e.category : categoryName, + required: ids(e.required_arguments), + recommended: ids(e.recommended_arguments), + ideEmitted: ids(e.ide_emitted_arguments), + requiredOneOf: oneOf.filter((g) => g.length > 0), + }); + } + } + } catch { + // Schema missing/corrupt β†’ empty map β†’ rules silently pass (graceful degradation). + } + schemaArgTiers = map; + return schemaArgTiers; +} + +/* + * (apiId, argumentId) pairs already enforced by a dedicated `mustContainArgument` rule. + * Read from the rule catalogue at runtime rather than hardcoded, so adding a new + * mustContainArgument rule automatically suppresses the schema-driven duplicate. + * + * Without this, one defect is scored twice β€” a WebConnect missing `connectionName` + * fires both REST-CONN-001 (critical, weight 8) and the schema rule (major, weight 5). + */ +let coveredArgPairs: Set | null = null; +function getCoveredArgPairs(): Set { + if (coveredArgPairs) return coveredArgPairs; + const set = new Set(); + try { + const raw = readFileSync(join(dirPath, '..', 'rules', 'provar_best_practices_rules.json'), 'utf-8'); + const parsed = JSON.parse(raw) as { rules?: Array<{ check?: Record }> }; + for (const rule of parsed.rules ?? []) { + const check = rule.check; + if (!check || check['type'] !== 'mustContainArgument') continue; + const apiId = check['apiId']; + // The catalogue uses `argument`; `argumentId` is tolerated for forward-compat. + const arg = typeof check['argument'] === 'string' ? check['argument'] : check['argumentId']; + if (typeof apiId === 'string' && typeof arg === 'string') set.add(`${apiId}::${arg}`); + } + } catch { + // Catalogue unreadable β†’ no suppression; duplicates are preferable to silence. + } + coveredArgPairs = set; + return coveredArgPairs; +} + +/** Steps whose declared tier args are absent, plus the total offending-step count. */ +function collectMissingTierArgs( + tc: XmlNode, + pick: (t: SchemaArgTiers) => readonly string[], + dedupeAgainstDedicatedRules: boolean, + checkOneOf = false +): { steps: string[]; missingByArg: Map; count: number } { + const tiers = getSchemaArgTiers(); + const covered = dedupeAgainstDedicatedRules ? getCoveredArgPairs() : null; + const steps: string[] = []; + const missingByArg = new Map(); + let count = 0; + for (const call of getAllApiCalls(tc)) { + const apiId = call['@_apiId'] as string | undefined; + if (!apiId || REQUIRED_ARGS_EXCLUDED_API_IDS.has(apiId) || apiId.includes('NitroXConnect')) continue; + const tier = tiers.get(apiId); + if (!tier || UNSCORED_SCHEMA_CATEGORIES.has(tier.category)) continue; + const wanted = pick(tier); + const oneOfGroups = checkOneOf ? tier.requiredOneOf : []; + if (wanted.length === 0 && oneOfGroups.length === 0) continue; + const present = new Set(); + const populated = new Set(); + for (const a of getCallArguments(call)) { + const id = a['@_id'] as string | undefined; + if (!id) continue; + present.add(id); + if (schemaArgumentHasValue(a)) populated.add(id); + } + const missing = wanted.filter((r) => !present.has(r) && !(covered && covered.has(`${apiId}::${r}`))); + // A one-of group is satisfied only when a member is present AND carries a value. + // Presence alone is not enough: the group exists because the step is inert + // without one of them, and `` is exactly as inert as + // omitting it. Required-tier args stay presence-based β€” Provar's convention is to + // declare them empty (see ide_emitted_arguments), so absence is the real signal. + for (const group of oneOfGroups) { + if (!group.some((g) => populated.has(g))) missing.push(`one of [${group.join(' | ')}] with a value`); + } + if (!missing.length) continue; + count++; + const ctx = stepContext(call); + if (steps.length < 10) steps.push(`'${ctx.title}' (testItemId=${ctx.tid}): ${missing.join(', ')}`); + for (const m of missing) missingByArg.set(m, (missingByArg.get(m) ?? 0) + 1); + } + return { steps, missingByArg, count }; +} + +/* + * One aggregated violation per rule, carrying `count`, rather than one violation per + * offending step. calculateBPScore damps a single violation logarithmically + * (1 + log2(count)) but sums per-violation penalties linearly, so per-step emission + * bypassed the damping entirely: a file with 71 offending steps scored + * 71 x 5 x 0.75 = 266 points of deduction (score 0) where the aggregated form costs + * 5 x 0.75 x (1 + log2 71) = 26.8. This also matches the Quality Hub Lambda, which + * emits one violation per rule with a count. + */ +/** + * Identity scope for an aggregate violation: the test case it came from. Falls back to + * the name, then to a stable marker, so a file lacking a guid still gets a key that is + * at least consistent within its own run. + */ +function testCaseScope(tc: XmlNode): string { + const root = (tc['testCase'] as XmlNode | undefined) ?? tc; + const guid = root['@_guid']; + if (typeof guid === 'string' && guid.length > 0) return guid; + const name = root['@_name']; + if (typeof name === 'string' && name.length > 0) return name; + return 'unscoped'; +} + +function aggregateTierViolation( + rule: BPRule, + found: { steps: string[]; missingByArg: Map; count: number }, + lead: string, + scope: string +): BPViolation[] { + if (found.count === 0) return []; + // Identity and presentation are separated. `diff_identity` is what validationDiff + // keys on: constant for this rule over this file, so partial remediation reads as + // "still unresolved, fewer steps" instead of "resolved + added". The MESSAGE stays + // specific, because that is what a user or agent reads to decide the next fix β€” + // making it generic kept the diff honest but left partial-fix responses + // unactionable. + const missingArgs = [...found.missingByArg.keys()].sort(); + const more = found.count > found.steps.length ? ` …and ${found.count - found.steps.length} more step(s)` : ''; + const v = makeViolation( + rule, + `${lead} ${found.count} step(s) affected. Missing argument(s): ${missingArgs.join(', ')}. Steps: ${found.steps.join( + '; ' + )}${more}`, + found.count + ); + // Scoped to the test case, NOT a bare literal. Suite validation flattens every test + // case's violations into one array, so a constant identity made same-rule aggregates + // from different files collide on one diff key β€” computeDiff keeps a single sample + // per key, so an unchanged file could mask a partially-fixed one and updated[] would + // come back empty with work still outstanding. + v.diff_identity = `aggregate:${scope}`; + v.details = { + affected_steps: found.count, + missing_arguments: missingArgs, + missing_by_argument: Object.fromEntries([...found.missingByArg.entries()].sort((a, b) => b[1] - a[1])), + steps: `${found.steps.join('; ')}${more}`, + }; + return [v]; +} + +/** STEP-REQUIRED-ARGS-001 β€” schema-declared load-blocking arguments (major). */ +function validateSchemaRequiredArguments(tc: XmlNode, rule: BPRule): BPViolation[] { + const found = collectMissingTierArgs(tc, (t) => t.required, true, true); + return aggregateTierViolation( + rule, + found, + 'One or more steps are missing an argument the Provar step schema lists as required.', + testCaseScope(tc) + ); +} + +/** STEP-RECOMMENDED-ARGS-001 β€” strongly conventional but not load-blocking (minor). */ +function validateSchemaRecommendedArguments(tc: XmlNode, rule: BPRule): BPViolation[] { + const found = collectMissingTierArgs(tc, (t) => t.recommended, true); + return aggregateTierViolation( + rule, + found, + 'One or more steps omit an argument that appears on 80-99% of real Provar steps of the same type.', + testCaseScope(tc) + ); +} + +/** STEP-IDE-PARITY-001 β€” arguments the Provar IDE always emits, even when empty (info). */ +function validateStepIdeParity(tc: XmlNode, rule: BPRule): BPViolation[] { + const found = collectMissingTierArgs(tc, (t) => t.ideEmitted, false); + return aggregateTierViolation( + rule, + found, + 'One or more steps omit arguments the Provar IDE emits for that step type even when empty, ' + + 'so the XML does not round-trip as IDE-authored.', + testCaseScope(tc) + ); +} + // UI-BINDING-ORDER-001 β€” binding URIs must list object= before action=/field= (percent-encoded). const BINDING_WRONG_ACTION_FIRST = /object%3Faction%3D[^%]+%26object%3D/; const BINDING_WRONG_FIELD_FIRST = /object%3Ffield%3D[^%]+%26object%3D/; @@ -2428,6 +2792,9 @@ const VALIDATOR_REGISTRY: Record = { type MultiValidatorFn = (tc: XmlNode, rule: BPRule) => BPViolation[]; const MULTI_VALIDATOR_REGISTRY: Record = { + schemaRequiredArguments: validateSchemaRequiredArguments, + schemaRecommendedArguments: validateSchemaRecommendedArguments, + stepIdeParity: validateStepIdeParity, uiActionNestingStructure: validateUiActionNestingStructure, uiAssertScreenContext: validateUiAssertScreenContext, nitroxConnectInvalidArgs: validateNitroxConnectInvalidArgs, diff --git a/src/mcp/tools/connectionTools.ts b/src/mcp/tools/connectionTools.ts index 398b240..7a12842 100644 --- a/src/mcp/tools/connectionTools.ts +++ b/src/mcp/tools/connectionTools.ts @@ -134,6 +134,33 @@ function buildConnectionMap(tp: Record): Map | undefined { + const names = new Set(); + try { + for (const info of buildConnectionMap(parseTestProjectXml(testProjectXml)).values()) { + if (info.name) names.add(info.name); + } + } catch { + // Unparseable β†’ UNDEFINED, not an empty set. The two are not the same: an empty set + // is authoritative ("this project declares no connections", so a reference to one is + // genuinely dangling), whereas a parse failure means we simply do not know. Returning + // an empty set there made the validator emit dangling-reference warnings off a + // malformed project file, contradicting its own documented conservative policy. + return undefined; + } + return names; +} + function parseConnectionList(content: string): ConnectionEntry[] { const tp = parseTestProjectXml(content); const map = buildConnectionMap(tp); diff --git a/src/mcp/tools/projectValidateFromPath.ts b/src/mcp/tools/projectValidateFromPath.ts index 7662ed3..a7dd926 100644 --- a/src/mcp/tools/projectValidateFromPath.ts +++ b/src/mcp/tools/projectValidateFromPath.ts @@ -274,7 +274,7 @@ export function registerProjectValidateFromPath(server: McpServer, config: Serve .string() .optional() .describe( - 'run_id from a previous call. When provided, returns only project-level violations that are new or resolved since that run: { added, resolved, unchanged_count, run_id }. If not found, returns error BASELINE_NOT_FOUND.' + 'run_id from a previous call. When provided, returns only project-level violations that are new or resolved since that run: { added, resolved, updated, unchanged_count, run_id }. `updated` carries findings that survived but whose message or details changed, so a partially-fixed aggregate still tells you what remains without echoing every unchanged finding. If not found, returns error BASELINE_NOT_FOUND.' ), }, }, diff --git a/src/mcp/tools/qualityHubApiTools.ts b/src/mcp/tools/qualityHubApiTools.ts index 1fc3d8e..df545a2 100644 --- a/src/mcp/tools/qualityHubApiTools.ts +++ b/src/mcp/tools/qualityHubApiTools.ts @@ -18,21 +18,32 @@ import { QualityHubRateLimitError, REQUEST_ACCESS_URL, } from '../../services/qualityHub/client.js'; +import { selectBundledExamples } from '../examples/bundledExamples.js'; import { desc } from './descHelper.js'; const CORPUS_FALLBACK_HINT = - 'Fallback: read the provar://docs/step-reference MCP resource for step types and attribute formats, then continue.'; + 'Fallback: call the provar_step_schema tool (api_id or category) for step types and their required/optional arguments, then continue.'; + +const BUNDLED_NOTE = + 'These are offline, bundled reference examples (quality_tier="bundled", source="bundled"), NOT corpus-retrieved ' + + 'matches β€” use them as a structural pattern, adapting object/field/connection names to your project.'; const CORPUS_ONBOARDING_WARNING = - 'Corpus retrieval skipped β€” no Provar API key configured. Continuing without example grounding.\n' + - 'To enable corpus retrieval: run sf provar auth login\n' + + 'Corpus retrieval skipped β€” no Provar API key configured. Returning bundled offline examples instead.\n' + + 'To enable full corpus retrieval: run sf provar auth login\n' + `No account? Request access at: ${REQUEST_ACCESS_URL}\n` + - CORPUS_FALLBACK_HINT; + BUNDLED_NOTE; + +const CORPUS_NO_BUNDLED_MATCH_WARNING = + 'Corpus retrieval skipped β€” no Provar API key configured, and no bundled offline example matches this query.\n' + + 'The bundled set covers Salesforce UI scenarios only; it is deliberately not returned for unrelated step types.\n' + + `${CORPUS_FALLBACK_HINT}\n` + + `To enable full corpus retrieval: run sf provar auth login. No account? Request access at: ${REQUEST_ACCESS_URL}`; const CORPUS_AUTH_WARNING = - 'Corpus retrieval skipped β€” API key is invalid or expired. Continuing without example grounding.\n' + + 'Corpus retrieval skipped β€” API key is invalid or expired. Returning bundled offline examples instead.\n' + `Run sf provar auth login to get a new key, or request access at: ${REQUEST_ACCESS_URL}\n` + - CORPUS_FALLBACK_HINT; + BUNDLED_NOTE; const CORPUS_RATE_LIMIT_WARNING = 'Corpus retrieval skipped β€” rate limit reached. Continuing without example grounding. Try again shortly.\n' + @@ -56,14 +67,16 @@ export function registerCorpusExamplesRetrieve(server: McpServer): void { 'Use this BEFORE writing any Provar .testcase XML β€” whether via provar_testcase_generate, Write, or Edit.', 'Pass a user story, requirement, source test file content, or step type keywords as the query.', 'Returns up to N example Provar XML test cases ordered by similarity score.', - 'If retrieval fails (no auth, network error, rate limit), returns empty examples with a warning β€” the', - 'generation workflow can still continue without grounding. Never hard-errors on API failure.', + 'If no API key is configured (or the key is invalid), returns a small set of offline, known-correct', + 'BUNDLED examples (quality_tier="bundled", source="bundled") so first-test-case generation still has', + 'a structural pattern to follow. On transient failures (rate limit, network) returns empty with a', + 'warning. Never hard-errors on API failure β€” the generation workflow can always continue.', '', 'For org-specific field metadata: first call getObjectSchema from the Salesforce Hosted MCP', '(platform/sobject-reads β€” https://api.salesforce.com/platform/mcp/v1/platform/sobject-reads),', 'then include key field names in your query (e.g. "Opportunity: CloseDate, Amount, StageName").', '', - 'Requires a Provar API key (sf provar auth login). Without a key, returns empty examples with onboarding instructions.', + 'A Provar API key (sf provar auth login) unlocks full corpus retrieval; without one you still get bundled offline examples plus onboarding instructions.', ].join('\n'), 'Retrieve similar Provar test case examples from the Quality Hub corpus.' ), @@ -126,13 +139,17 @@ export function registerCorpusExamplesRetrieve(server: McpServer): void { const apiKey = credentialsService.resolveApiKey(); if (!apiKey) { - log('warn', 'provar_qualityhub_examples_retrieve: no api key', { requestId }); + log('warn', 'provar_qualityhub_examples_retrieve: no api key β€” returning bundled examples', { requestId }); + const examples = selectBundledExamples(query, n); const result = { requestId, - examples: [], - count: 0, + examples, + count: examples.length, query_truncated: false, - warning: CORPUS_ONBOARDING_WARNING, + // Relevance-gated: the bundle only covers Salesforce UI scenarios, so an + // off-topic query correctly yields nothing rather than a misleading match. + source: examples.length > 0 ? ('bundled' as const) : ('none' as const), + warning: examples.length > 0 ? CORPUS_ONBOARDING_WARNING : CORPUS_NO_BUNDLED_MATCH_WARNING, }; return { content: [{ type: 'text' as const, text: JSON.stringify(result) }], structuredContent: result }; } @@ -157,14 +174,30 @@ export function registerCorpusExamplesRetrieve(server: McpServer): void { query_truncated: response.query_truncated, }); - const result = { requestId, ...response }; + // `source` is set on every path, not just the bundled ones, so a caller can + // branch on it unconditionally instead of inferring "corpus" from its absence. + const result = { requestId, ...response, source: 'corpus' as const }; return { content: [{ type: 'text' as const, text: JSON.stringify(result) }], structuredContent: result }; } catch (err: unknown) { - let warning: string; + // On a bad/expired key, bundled examples are still useful grounding. On transient + // failures (rate limit, network) return empty β€” retrying is the right move, and + // bundled examples would mask a temporary outage as "no matches". if (err instanceof QualityHubAuthError) { - warning = CORPUS_AUTH_WARNING; - log('warn', 'provar_qualityhub_examples_retrieve: auth error', { requestId }); - } else if (err instanceof QualityHubRateLimitError) { + log('warn', 'provar_qualityhub_examples_retrieve: auth error β€” returning bundled examples', { requestId }); + const examples = selectBundledExamples(query, n); + const result = { + requestId, + examples, + count: examples.length, + query_truncated: false, + source: examples.length > 0 ? ('bundled' as const) : ('none' as const), + warning: examples.length > 0 ? CORPUS_AUTH_WARNING : CORPUS_NO_BUNDLED_MATCH_WARNING, + }; + return { content: [{ type: 'text' as const, text: JSON.stringify(result) }], structuredContent: result }; + } + + let warning: string; + if (err instanceof QualityHubRateLimitError) { warning = CORPUS_RATE_LIMIT_WARNING; log('warn', 'provar_qualityhub_examples_retrieve: rate limited', { requestId }); } else { @@ -174,7 +207,8 @@ export function registerCorpusExamplesRetrieve(server: McpServer): void { } // Degrade gracefully β€” never isError:true. The LLM continues without grounding. - const result = { requestId, examples: [], count: 0, query_truncated: false, warning }; + // `source: 'none'` keeps the response shape uniform across all four paths. + const result = { requestId, examples: [], count: 0, query_truncated: false, source: 'none' as const, warning }; return { content: [{ type: 'text' as const, text: JSON.stringify(result) }], structuredContent: result }; } } diff --git a/src/mcp/tools/stepSchemaTools.ts b/src/mcp/tools/stepSchemaTools.ts new file mode 100644 index 0000000..8b5cacb --- /dev/null +++ b/src/mcp/tools/stepSchemaTools.ts @@ -0,0 +1,183 @@ +/* + * 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 { readFileSync } from 'node:fs'; +import { fileURLToPath } from 'node:url'; +import { dirname, join } from 'node:path'; +import { z } from 'zod'; +import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; +import { makeError, makeRequestId } from '../schemas/common.js'; +import { log } from '../logging/logger.js'; +import { desc } from './descHelper.js'; + +// ── Schema shape (only the fields this tool surfaces) ───────────────────────── + +interface SchemaArgument { + id: string; + type?: string; + default?: string; + description?: string; +} + +interface StepEntry { + apiId: string; + description?: string; + category?: string; + required_arguments?: SchemaArgument[]; + optional_arguments?: SchemaArgument[]; + validation_rules?: string[]; + best_practices?: string[]; +} + +interface StepIndexEntry { + name: string; + apiId: string; + category: string; + required_arguments: string[]; +} + +const dirPath = dirname(fileURLToPath(import.meta.url)); + +// ── Schema loading (lazy, module-level singleton) ───────────────────────────── + +interface LoadedSchema { + /** short name (lower) β†’ entry */ byName: Map; + /** apiId (lower) β†’ entry */ byApiId: Map; + index: StepIndexEntry[]; +} + +let loaded: LoadedSchema | null = null; + +function loadSchema(): LoadedSchema | null { + if (loaded) return loaded; + let parsed: { apiCalls?: Record }; + try { + const raw = readFileSync(join(dirPath, '..', 'rules', 'provar_test_step_schema.json'), 'utf-8'); + parsed = JSON.parse(raw) as { apiCalls?: Record }; + } catch { + return null; + } + const byName = new Map(); + const byApiId = new Map(); + const index: StepIndexEntry[] = []; + for (const [categoryName, category] of Object.entries(parsed.apiCalls ?? {})) { + if (!category || typeof category !== 'object') continue; + for (const [stepName, entry] of Object.entries(category as Record)) { + if (!entry || typeof entry !== 'object') continue; + const e = entry as StepEntry; + if (typeof e.apiId !== 'string') continue; + const reqIds = (e.required_arguments ?? []).map((a) => a.id).filter((id): id is string => typeof id === 'string'); + byName.set(stepName.toLowerCase(), e); + byApiId.set(e.apiId.toLowerCase(), e); + index.push({ name: stepName, apiId: e.apiId, category: e.category ?? categoryName, required_arguments: reqIds }); + } + } + loaded = { byName, byApiId, index }; + return loaded; +} + +/** Resolve an api_id argument (full apiId or short step name, e.g. "UiConnect") to an entry. */ +function resolveStep(schema: LoadedSchema, apiId: string): StepEntry | null { + const key = apiId.trim().toLowerCase(); + return schema.byApiId.get(key) ?? schema.byName.get(key) ?? schema.byName.get(key.split('.').pop() ?? key) ?? null; +} + +// ── Tool: provar_step_schema ────────────────────────────────────────────────── + +function registerStepSchema(server: McpServer): void { + server.registerTool( + 'provar_step_schema', + { + title: 'Provar Step Schema', + description: desc( + [ + 'Return the argument schema for Provar test-case step types from the bundled step reference.', + 'Grounds test-case generation and hand-editing in exact argument names β€” pass api_id (full apiId like', + '"com.provar.plugins.forcedotcom.core.ui.UiConnect" or the short name "UiConnect") to get that step type\'s', + 'required_arguments, optional_arguments (each with id/type/default), validation_rules, and best_practices.', + 'Pass category (e.g. "UI", "Salesforce", "Control") to list every step type in that category.', + 'Omit both to get an index of all step types (name, apiId, category, required-argument ids).', + 'Read-only and offline β€” this is the tool form of the provar://schema/test-step resource, so agents that', + 'cannot read MCP resources can still reach the schema. Use it before provar_testcase_generate / _step_edit.', + ].join(' '), + 'Return required/optional arguments for a Provar step type (by api_id), a category, or the full index.' + ), + inputSchema: { + api_id: z + .string() + .optional() + .describe( + desc( + 'Full apiId or short step name (e.g. "UiConnect"). Returns that step type\'s full argument schema.', + 'string, optional; full apiId or short step name' + ) + ), + category: z + .string() + .optional() + .describe( + desc( + 'Category name (UI, Salesforce, Control, Data, Utility, ProvarAI, ProvarLabs). Lists steps in it.', + 'string, optional; category to list' + ) + ), + }, + }, + ({ api_id, category }) => { + const requestId = makeRequestId(); + log('info', 'provar_step_schema', { requestId, api_id, category }); + + const schema = loadSchema(); + if (!schema) { + const err = makeError( + 'STEP_SCHEMA_NOT_FOUND', + 'provar_test_step_schema.json could not be read. Reinstall or upgrade the plugin and try again.', + requestId, + false + ); + return { isError: true, content: [{ type: 'text' as const, text: JSON.stringify(err) }] }; + } + + if (api_id) { + const entry = resolveStep(schema, api_id); + if (!entry) { + const near = schema.index + .filter((s) => s.name.toLowerCase().includes(api_id.trim().toLowerCase())) + .slice(0, 8) + .map((s) => s.name); + const err = makeError( + 'STEP_TYPE_NOT_FOUND', + `No step type matches api_id "${api_id}".`, + requestId, + false, + near.length + ? { suggestion: `Did you mean one of: ${near.join(', ')}? Or omit api_id to list all step types.` } + : { suggestion: 'Omit api_id to get the full index of step types.' } + ); + return { isError: true, content: [{ type: 'text' as const, text: JSON.stringify(err) }] }; + } + const result = { requestId, step: entry }; + return { content: [{ type: 'text' as const, text: JSON.stringify(result) }], structuredContent: result }; + } + + if (category) { + const cat = category.trim().toLowerCase(); + const steps = schema.index.filter((s) => s.category.toLowerCase() === cat); + const result = { requestId, category, count: steps.length, steps }; + return { content: [{ type: 'text' as const, text: JSON.stringify(result) }], structuredContent: result }; + } + + const result = { requestId, count: schema.index.length, steps: schema.index }; + return { content: [{ type: 'text' as const, text: JSON.stringify(result) }], structuredContent: result }; + } + ); +} + +export function registerAllStepSchemaTools(server: McpServer): void { + registerStepSchema(server); +} diff --git a/src/mcp/tools/testCaseGenerate.ts b/src/mcp/tools/testCaseGenerate.ts index 4a7b380..74be923 100644 --- a/src/mcp/tools/testCaseGenerate.ts +++ b/src/mcp/tools/testCaseGenerate.ts @@ -241,7 +241,7 @@ const TOOL_DESCRIPTION = [ 'Cleanup warning: ApexDeleteObject steps near end of test will be skipped if an earlier step fails (stopOnError=false). Use a TearDown callable.', 'Validation: when validate_after_edit=true (default) the response includes a validation field and returns TESTCASE_INVALID if the generated XML fails structural checks.', 'Grounding: call provar_qualityhub_examples_retrieve before generating to get corpus examples for the scenario β€” correct XML structure for the step types you need.', - 'If the response has count: 0 with a warning field (API unavailable or not configured), fall back: read the provar://docs/step-reference MCP resource for step types and attribute formats, then continue.', + 'If the response has count: 0 with a warning field (API unavailable or not configured), fall back: call the provar_step_schema tool (api_id or category) for step types and their required/optional arguments, then continue.', ].join(' '); export function registerTestCaseGenerate(server: McpServer, config: ServerConfig): void { diff --git a/src/mcp/tools/testCaseStepTools.ts b/src/mcp/tools/testCaseStepTools.ts index bafb4e7..a850a80 100644 --- a/src/mcp/tools/testCaseStepTools.ts +++ b/src/mcp/tools/testCaseStepTools.ts @@ -113,7 +113,7 @@ export function registerTestCaseStepEdit(server: McpServer, config: ServerConfig 'Returns STEP_NOT_FOUND (with all_test_item_ids list) when the target step is absent.', 'Returns INVALID_STEP_XML when step_xml cannot be parsed or contains β‰ 1 elements.', 'Returns INVALID_XML_AFTER_EDIT (backup restored) when the mutated file fails validation.', - 'Grounding for step_xml: call provar_qualityhub_examples_retrieve for corpus examples of the step type you need; if the response has count: 0 with a warning field, fall back: read the provar://docs/step-reference MCP resource.', + "Grounding for step_xml: call provar_qualityhub_examples_retrieve for corpus examples of the step type you need; if the response has count: 0 with a warning field, fall back: call the provar_step_schema tool (api_id or category) for the step type's required/optional arguments.", ].join(' '), 'AMENDMENT-ONLY: add or remove a single apiCall step in an existing Provar test case (not for constructing new ones).' ), diff --git a/src/mcp/tools/testCaseValidate.ts b/src/mcp/tools/testCaseValidate.ts index c99307c..ef14c34 100644 --- a/src/mcp/tools/testCaseValidate.ts +++ b/src/mcp/tools/testCaseValidate.ts @@ -47,6 +47,7 @@ import { UI_ASSERT_COMPARISON_TYPE_SET, } from '../rules/comparisonTypeSets.js'; import { runBestPractices } from './bestPracticesEngine.js'; +import { parseProjectConnectionNames } from './connectionTools.js'; import { desc } from './descHelper.js'; import { UI_ACTION_API_IDS, UI_SCREEN_CONTAINER_API_IDS, UI_LOCATOR_BEARING_API_IDS } from './uiActionApiIds.js'; @@ -104,16 +105,72 @@ function deriveQualityVerdict(isValid: boolean, qualityScore: number, qualityThr return { status, quality_threshold, meets_quality_threshold }; } +/** + * Connection names declared in the project that owns `testCaseFilePath`. + * + * Walks up from the test case looking for a `.testproject` marker, the same way + * the plan-mode resolver locates `provardx-properties.json`. Returns undefined + * when no project is found or it cannot be read β€” CONNECT-REF-CONSISTENCY-001 + * then stays conservative instead of reporting valid project-level references as + * dangling connections. + * + * EVERY candidate is checked against the path policy before any filesystem call. + * The walk ascends, so without this it would read `.testproject` from directories + * above `--allowed-paths` β€” and because a suppressed reference is observable in the + * output, that would also let a caller probe connection names outside the boundary. + * Leaving the allowed roots ends the walk rather than skipping the candidate: every + * further ancestor is outside too. + */ +function resolveProjectConnectionNames( + testCaseFilePath: string, + allowedPaths: string[] +): ReadonlySet | undefined { + try { + let dir = path.dirname(path.resolve(testCaseFilePath)); + for (let depth = 0; depth < 12; depth++) { + const candidate = path.join(dir, '.testproject'); + try { + assertPathAllowed(candidate, allowedPaths); + } catch { + return undefined; // ascended out of the allowed roots β€” stop, do not read + } + if (fs.existsSync(candidate)) { + return parseProjectConnectionNames(fs.readFileSync(candidate, 'utf-8')); + } + const parent = path.dirname(dir); + if (parent === dir) break; + dir = parent; + } + } catch { + // Unreadable project β†’ undefined β†’ rule stays conservative. + } + return undefined; +} + +/** + * Everything the validator can learn from the test case's project on disk: whether it + * runs under a plan (DATA-001) and which connections the project declares + * (CONNECT-REF-CONSISTENCY-001). Returns empty options for content-only validation, + * where there is no project to inspect. + */ +function resolveProjectContext(filePath: string | undefined, allowedPaths: string[]): ValidateTestCaseOptions { + if (!filePath) return { planMode: 'unknown' }; + return { + planMode: resolveTestCasePlanMode({ testCaseFilePath: filePath, allowedPaths }).mode, + projectConnectionNames: resolveProjectConnectionNames(filePath, allowedPaths), + }; +} + /** Resolve validation result from QualityHub API or fall back to local. */ async function resolveBaseResult( source: string, apiKey: string | null, requestId: string, - planMode: TestCasePlanMode = 'unknown' + opts: ValidateTestCaseOptions = {} ): Promise { if (!apiKey) { return { - ...validateTestCase(source, undefined, { planMode }), + ...validateTestCase(source, undefined, opts), validation_source: 'local', validation_warning: ONBOARDING_MESSAGE, }; @@ -121,7 +178,7 @@ async function resolveBaseResult( const baseUrl = getQualityHubBaseUrl(); try { const apiResult = await qualityHubClient.validateTestCaseViaApi(source, apiKey, baseUrl); - const localMeta = validateTestCase(source, undefined, { planMode }); + const localMeta = validateTestCase(source, undefined, opts); log('info', 'provar_testcase_validate: quality_hub', { requestId }); return { ...apiResult, @@ -146,7 +203,7 @@ async function resolveBaseResult( log('warn', 'provar_testcase_validate: api unreachable, falling back', { requestId }); } return { - ...validateTestCase(source, undefined, { planMode }), + ...validateTestCase(source, undefined, opts), validation_source: 'local_fallback', validation_warning: warning, }; @@ -165,7 +222,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. Returns a tri-state status: 'invalid' (a critical defect β€” the test will not load in Provar, is_valid=false), 'needs_improvement' (loads but quality_score is below quality_threshold), or 'valid' (loads and clears the bar); meets_quality_threshold and the effective quality_threshold are also returned. Note: a critical best-practice violation (e.g. an unknown apiId) now gates is_valid the same way a structural error does β€” it surfaces in issues[] as an ERROR. major/minor/info best-practice violations affect quality_score (and the status verdict) only. 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 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. Returns a tri-state status: 'invalid' (a critical defect β€” the test will not load in Provar, is_valid=false), 'needs_improvement' (loads but quality_score is below quality_threshold), or 'valid' (loads and clears the bar); meets_quality_threshold and the effective quality_threshold are also returned. Note: a critical best-practice violation (e.g. an unknown apiId) now gates is_valid the same way a structural error does β€” it surfaces in issues[] as an ERROR. major/minor/info best-practice violations affect quality_score (and the status verdict) only. 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, call the provar_step_schema tool (api_id or category) for correct step attribute schemas.", 'Validate a Provar XML test case: structure, UUIDs, steps, quality scoring; run_id for baseline diff.' ), inputSchema: { @@ -204,7 +261,7 @@ export function registerTestCaseValidate(server: McpServer, config: ServerConfig .optional() .describe( desc( - 'run_id from a previous call. When provided, returns only issues that are new or resolved since that run: { added, resolved, unchanged_count, run_id }. If not found, returns error BASELINE_NOT_FOUND.', + 'run_id from a previous call. When provided, returns only issues that are new or resolved since that run: { added, resolved, updated, unchanged_count, run_id }. `updated` carries findings that survived but whose message or details changed, so a partially-fixed aggregate still tells you what remains without echoing every unchanged finding. If not found, returns error BASELINE_NOT_FOUND.', 'string, optional; prev run_id for diff response' ) ), @@ -234,10 +291,8 @@ export function registerTestCaseValidate(server: McpServer, config: ServerConfig } const apiKey = resolveApiKey(); - const planMode: TestCasePlanMode = file_path - ? resolveTestCasePlanMode({ testCaseFilePath: file_path, allowedPaths: config.allowedPaths }).mode - : 'unknown'; - const baseResult = await resolveBaseResult(source, apiKey, requestId, planMode); + const projectContext = resolveProjectContext(file_path, config.allowedPaths); + const baseResult = await resolveBaseResult(source, apiKey, requestId, projectContext); const storageDir = tcStorageDir(); const context = tcRunContext(file_path, source); @@ -474,6 +529,14 @@ function maybeEmitDataTableWarning( */ export interface ValidateTestCaseOptions { planMode?: TestCasePlanMode; + /** + * Connection names declared in the project's `.testproject`, resolved by the MCP + * handler when `file_path` is supplied. CONNECT-REF-CONSISTENCY-001 treats these + * as valid reference targets alongside connect-step resultNames β€” Provar resolves + * against both. Omitted (content-only validation) means "no project context", and + * the rule stays conservative rather than guessing. + */ + projectConnectionNames?: ReadonlySet; } /** Pure function β€” exported for unit testing */ @@ -566,7 +629,15 @@ export function validateTestCase( ? ((Array.isArray(rawApiCalls) ? rawApiCalls : [rawApiCalls]) as Array>) : []; - for (const call of apiCalls) { + // Validate EVERY apiCall in the tree, not just top-level ones. UI action and + // assertion steps (UiDoAction, UiFill, UiAssert, …) live nested inside a + // UiWithScreen ``, so a top-level-only walk silently + // skipped every real UI step β€” the per-call structural rules (UI-INTERACTION-*, + // UI-ASSERT-STRUCTURE-001, UI-LOCATOR-001) never fired on them. Collect + // recursively so those rules apply wherever the step actually sits. + const allApiCalls: Array> = []; + collectNodesByKey(tc, 'apiCall', allApiCalls); + for (const call of allApiCalls) { validateApiCall(call, issues); } @@ -576,6 +647,11 @@ export function validateTestCase( // covered too, not just top-level apiCalls. validateComparisonTypes(tc, issues); + // SF-CONNECT-TYPE-001 / CONNECT-REF-CONSISTENCY-001: connection-topology checks + // that need the whole test (all connect steps + all downstream references). + validateSalesforceConnectType(tc, issues); + validateConnectionReferenceConsistency(tc, issues, options.projectConnectionNames); + // VAR-REF-001 / VAR-REF-002: detect {VarName} tokens inside valueClass="string" elements. // Provar does not interpolate {…} tokens in plain string values at runtime β€” they must use // class="variable" (pure reference) or class="compound" (embedded in surrounding text). @@ -739,9 +815,64 @@ function checkUiInteraction(call: Record, stepName: string, iss 'Emit the interaction as: . ' + 'In provar_testcase_generate the "interaction" attribute is converted automatically.', }); + return; + } + // UI-INTERACTION-002: the interaction NAME must be a real Provar interaction. + // Provar ships a large interaction vocabulary β€” the corpus alone carries 39 + // distinct names (action, set, click, check, invoke, doubleClick, sfLookup, + // hover, file, clear, toggle, uncheck, …) β€” so this deliberately denylists only + // names with zero corpus evidence that are known LLM analogies from other + // frameworks. Allow-listing would false-fire on the long tail of valid-but-rare + // interactions; see INVALID_INTERACTION_NAMES for the evidence behind each entry. + const uri = (interactionNode['@_uri'] as string | undefined) ?? ''; + const nameMatch = /[?&]name=([^&"]+)/.exec(uri); + const interactionName = nameMatch ? nameMatch[1] : ''; + const correction = INVALID_INTERACTION_NAMES[interactionName.toLowerCase()]; + if (correction) { + issues.push({ + rule_id: 'UI-INTERACTION-002', + severity: 'ERROR', + message: `"${stepName}" uses interaction name="${interactionName}", which is not a valid Provar UI interaction β€” use name="${correction}" instead.`, + applies_to: 'apiCall', + suggestion: `Emit . ${INTERACTION_VOCABULARY_HINT}`, + }); } } +/** + * The single canonical description of Provar's interaction vocabulary, shared by every + * surface that mentions it so schema, registry and emitted suggestions cannot drift. + * + * Deliberately NON-exhaustive: an earlier revision listed "action, set, file" as if + * complete, which reads as declaring the other 36 real interactions invalid β€” `click` + * among them, contradicting the validator that now accepts it. + */ +export const INTERACTION_VOCABULARY_HINT = + 'Provar interaction names are an open vocabulary β€” action, set, click, check/uncheck, ' + + 'doubleClick, hover, invoke, clear, file and more are all real. Only names borrowed from ' + + 'other frameworks (type, fill, enter, input, tap, press) are rejected: use set to type ' + + 'into a field, and action or click to activate a control.'; + +// Known-wrong interaction names β†’ the correct Provar interaction they map to. +// +// Corpus evidence (2,701 real .testcase files): 39 distinct interaction names appear +// in well-formed nodes. The three most common are +// action (15,983), set (11,787) and click (1,240). `click` is a REAL Provar +// interaction emitted by the IDE recorder β€” denylisting it invalidated 164 real +// customer test cases β€” so it must never appear here. Likewise `select`: the corpus +// carries sfSelect/sfLookup/sfIleActivate, so a bare `select` is too close to a real +// family to reject at ERROR tier. The names kept below have zero corpus occurrences +// in either direction and are the shapes an LLM reaches for by analogy with +// Selenium/Playwright. +const INVALID_INTERACTION_NAMES: Record = { + tap: 'action', + press: 'action', + type: 'set', + fill: 'set', + enter: 'set', + input: 'set', +}; + // Flat-form argument ids that the broken/CLI-only UiAssert shape carries at the // top level. The IDE-renderable contract nests these inside a uiFieldAssertion. const UI_ASSERT_FLAT_ARG_IDS = ['fieldLocator', 'attributeName', 'comparisonType', 'expectedValue']; @@ -755,9 +886,32 @@ const UI_ASSERT_FLAT_ARG_IDS = ['fieldLocator', 'attributeName', 'comparisonType // (Named "-STRUCTURE-001" to match the local SETVALUES-STRUCTURE-001 convention // and avoid colliding with the best-practice JSON rule UI-ASSERT-STRUCT-001.) function checkUiAssertStructure(call: Record, stepName: string, issues: ValidationIssue[]): void { - const flatArg = getArgList(call).find((a) => - UI_ASSERT_FLAT_ARG_IDS.includes((a['@_id'] as string | undefined) ?? '') - ); + const args = getArgList(call); + + // Shape B: fieldAssertions present but filled with / + // instead of the required . This is the shape an LLM reaches + // for by analogy with UiFill/SetValues; it renders the IDE Result Assertions + // tab blank exactly like the flat form. Detected here because the flat-arg + // check below only covers top-level fieldLocator-style arguments. + const fieldAssertionsArg = args.find((a) => (a['@_id'] as string | undefined) === 'fieldAssertions'); + const faValue = fieldAssertionsArg?.['value'] as Record | undefined; + if (faValue && typeof faValue === 'object' && faValue['namedValues'] != null && faValue['uiFieldAssertion'] == null) { + issues.push({ + rule_id: 'UI-ASSERT-STRUCTURE-001', + severity: 'ERROR', + message: `UiAssert step "${stepName}" fieldAssertions uses β€” field assertions must be nested inside a (with a bare ), or the Provar IDE Result Assertions tab renders blank.`, + applies_to: 'apiCall', + suggestion: + 'Replace the namedValues block with: ' + + '' + + '' + + '. In provar_testcase_generate pass fieldLocator/attributeName/' + + 'comparisonType/expectedValue as flat attributes β€” the generator builds this structure automatically.', + }); + return; + } + + const flatArg = args.find((a) => UI_ASSERT_FLAT_ARG_IDS.includes((a['@_id'] as string | undefined) ?? '')); if (!flatArg) return; const offendingId = (flatArg['@_id'] as string | undefined) ?? ''; issues.push({ @@ -917,6 +1071,191 @@ function literalValueText(valueNode: Record | undefined): strin * ...ComparisonType.`), so this is emitted at ERROR tier β€” not as a * best-practices quality warning β€” to mirror the runtime failure offline. */ +/** Read an argument's literal string value; returns undefined for missing/variable/non-literal values. */ +function literalArgValue(call: Record, id: string): string | undefined { + const arg = getArgList(call).find((a) => (a['@_id'] as string | undefined) === id); + const v = arg?.['value'] as Record | undefined; + if (!v || typeof v !== 'object' || v['@_class'] !== 'value') return undefined; + const text = v['#text']; + if (typeof text === 'string') return text; + if (typeof text === 'number' || typeof text === 'boolean') return String(text); + return undefined; +} + +const CONNECT_REFERENCE_ARG_IDS = ['uiConnectionName', 'apexConnectionName', 'dbConnectionName', 'webConnectionName']; + +/** Which connection family each downstream reference argument draws from. */ +const REFERENCE_ARG_FAMILY: Record = { + uiConnectionName: 'ui', + apexConnectionName: 'apex', + dbConnectionName: 'db', + webConnectionName: 'web', +}; + +/** + * Which connect-step families can satisfy each reference. + * + * Not simple equality: `ApexConnect` with `quickUiLogin` opens the Lightning UI as well + * as the API connection, so a `uiConnectionName` legitimately resolves to an + * ApexConnect result β€” that is the canonical Salesforce pattern SF-CONNECT-TYPE-001 + * actively steers people toward. Requiring an exact family match made these two rules + * contradict each other, flagging the very shape the other one recommends. + * + * `other` (an unclassified connect apiId) satisfies everything: an unknown connect type + * is a reason to stay quiet, not to accuse. + */ +const REFERENCE_ARG_ACCEPTS: Record> = { + uiConnectionName: new Set(['ui', 'apex', 'other']), + apexConnectionName: new Set(['apex', 'other']), + dbConnectionName: new Set(['db', 'other']), + webConnectionName: new Set(['web', 'other']), +}; + +/** + * The connection family a connect step produces, derived from its apiId. Used to scope + * "this step's result is unresolvable" to the references it could actually satisfy, + * instead of silencing the rule for the whole file. + */ +function connectFamily(apiId: string): string { + if (/\.db\.|DbConnect$/.test(apiId)) return 'db'; + if (/restservice|WebConnect$/.test(apiId)) return 'web'; + if (apiId.endsWith('ApexConnect')) return 'apex'; + if (/UiConnect$|NitroXConnect/.test(apiId)) return 'ui'; + return 'other'; +} + +/** True when this apiId opens a connection (its result is what later steps reference). */ +function isConnectStep(apiId: string): boolean { + return /Connect(?::|$)/.test(apiId); +} + +// SF-CONNECT-TYPE-001: a Salesforce UI test authenticates through ApexConnect +// (which opens an API connection AND, via quickUiLogin, the Lightning UI). +// UiConnect opens a browser only and needs an existing ApexConnect result as its +// Salesforce credential β€” so a test that drives Salesforce UI (sf:ui:target) but +// has UiConnect and NO ApexConnect has no Salesforce credential. WARNING, not +// ERROR: UiConnect is syntactically valid and the connection could, in rare +// setups, be supplied by a parent callable test. +function validateSalesforceConnectType(tc: Record, issues: ValidationIssue[]): void { + const calls: Array> = []; + collectNodesByKey(tc, 'apiCall', calls); + const apiIds = calls.map((c) => (c['@_apiId'] as string | undefined) ?? ''); + const hasUiConnect = apiIds.some((a) => a.endsWith('.UiConnect')); + const hasApexConnect = apiIds.some((a) => a.endsWith('.ApexConnect')); + if (!hasUiConnect || hasApexConnect) return; + const hasSalesforceUi = calls.some((c) => + getArgList(c).some((a) => { + const v = a['value'] as Record | undefined; + const uri = (v?.['@_uri'] as string | undefined) ?? ''; + return uri.startsWith('sf:ui:'); + }) + ); + if (!hasSalesforceUi) return; + issues.push({ + rule_id: 'SF-CONNECT-TYPE-001', + severity: 'WARNING', + message: + 'This test drives the Salesforce UI (sf:ui: targets) but connects with UiConnect and no ApexConnect. ' + + 'UiConnect opens a browser only and has no Salesforce credential of its own.', + applies_to: 'testCase', + suggestion: + 'Use ApexConnect as the Salesforce connection step (it opens the API connection and, with quickUiLogin=true ' + + 'and uiApplicationName=LightningSales, the Lightning UI too). Add UiConnect only when you need a second, ' + + 'browser-only session that reuses the ApexConnect result.', + }); +} + +// CONNECT-REF-CONSISTENCY-001: every literal connection reference +// (uiConnectionName / apexConnectionName / …) must match the resultName produced +// by a connect step in this test (a connect step's result defaults to its +// connectionName when resultName is omitted). A reference to a name no connect +// step produced is a dangling connection. WARNING, since the connection could be +// established by a parent callable test. +/** + * Survey a test's connect steps: which connection names each family concretely + * produces, and which families produced a result we cannot resolve statically. + * + * Both are held PER FAMILY. A single global name set let a DbConnect result satisfy a + * `uiConnectionName`; a single global opaque flag let one dynamic DbConnect silence the + * rule for the whole file. + */ +function scanConnectSteps(calls: Array>): { + availableByFamily: Map>; + opaqueFamilies: Set; + connectStepCount: number; +} { + const availableByFamily = new Map>(); + const opaqueFamilies = new Set(); + let connectStepCount = 0; + + const addResult = (family: string, name: string): void => { + const set = availableByFamily.get(family) ?? new Set(); + set.add(name); + availableByFamily.set(family, set); + }; + + for (const c of calls) { + const apiId = (c['@_apiId'] as string | undefined) ?? ''; + if (!isConnectStep(apiId)) continue; + connectStepCount++; + const family = connectFamily(apiId); + const resultName = literalArgValue(c, 'resultName'); + const connectionName = literalArgValue(c, 'connectionName'); + if (resultName) addResult(family, resultName); + else if (connectionName) addResult(family, connectionName); + // Neither name is a literal: a variable connectionName, a with no class, or + // an empty argument resolved from project config. We cannot know what this step + // produces, so references of a compatible family become unverifiable. + else opaqueFamilies.add(family); + } + return { availableByFamily, opaqueFamilies, connectStepCount }; +} + +function validateConnectionReferenceConsistency( + tc: Record, + issues: ValidationIssue[], + projectConnectionNames?: ReadonlySet +): void { + const calls: Array> = []; + collectNodesByKey(tc, 'apiCall', calls); + + const { availableByFamily, opaqueFamilies, connectStepCount } = scanConnectSteps(calls); + // Connections may be inherited from a parent callable test. + if (connectStepCount === 0) return; + + for (const c of calls) { + const apiId = (c['@_apiId'] as string | undefined) ?? ''; + if (isConnectStep(apiId)) continue; + for (const refId of CONNECT_REFERENCE_ARG_IDS) { + const ref = literalArgValue(c, refId); + if (!ref) continue; + const refFamily = REFERENCE_ARG_FAMILY[refId] ?? ''; + const accepts = REFERENCE_ARG_ACCEPTS[refId] ?? new Set([refFamily]); + // Resolve only against connect steps of a COMPATIBLE family. + if ([...accepts].some((f) => availableByFamily.get(f)?.has(ref))) continue; + // An opaque connect step of a compatible family could be what produces `ref`. + if ([...accepts].some((f) => opaqueFamilies.has(f))) continue; + // Provar resolves connection references against the PROJECT's connection list + // too, not only against connect-step resultNames. A name declared in + // .testproject is valid even though no connect step in this file produces it. + if (projectConnectionNames?.has(ref)) continue; + const stepName = (c['@_name'] as string | undefined) ?? '(unnamed)'; + issues.push({ + rule_id: 'CONNECT-REF-CONSISTENCY-001', + severity: 'WARNING', + message: `Step "${stepName}" references ${refId}="${ref}", but no ${refFamily} connect step in this test produces that connection result${ + projectConnectionNames ? ' and the project declares no connection of that name' : '' + } (available ${refFamily} results: ${ + [...(availableByFamily.get(refFamily) ?? [])].map((n) => `"${n}"`).join(', ') || 'none' + }).`, + applies_to: 'apiCall', + suggestion: `Set the connect step's resultName to "${ref}", or change ${refId} to match a connect step's resultName or a connection declared in the project's .testproject. Downstream steps reference the connect step's resultName β€” which defaults to the connectionName when resultName is omitted.`, + }); + break; // one violation per step is enough + } + } +} + function validateComparisonTypes(tc: Record, issues: ValidationIssue[]): void { // AssertValues steps β†’ the 16-value AssertValues subset. const apiCalls: Array> = []; diff --git a/src/mcp/tools/testSuiteValidate.ts b/src/mcp/tools/testSuiteValidate.ts index c5be195..6708489 100644 --- a/src/mcp/tools/testSuiteValidate.ts +++ b/src/mcp/tools/testSuiteValidate.ts @@ -135,7 +135,7 @@ export function registerTestSuiteValidate(server: McpServer): void { .optional() .describe( desc( - 'run_id from a previous call. When provided, returns only violations that are new or resolved since that run: { added, resolved, unchanged_count, run_id }. If not found, returns error BASELINE_NOT_FOUND.', + 'run_id from a previous call. When provided, returns only violations that are new or resolved since that run: { added, resolved, updated, unchanged_count, run_id }. `updated` carries findings that survived but whose message or details changed, so a partially-fixed aggregate still tells you what remains without echoing every unchanged finding. If not found, returns error BASELINE_NOT_FOUND.', 'string, optional; prev run_id for diff response' ) ), diff --git a/src/mcp/utils/validationDiff.ts b/src/mcp/utils/validationDiff.ts index 84106ea..8e9ee55 100644 --- a/src/mcp/utils/validationDiff.ts +++ b/src/mcp/utils/validationDiff.ts @@ -24,6 +24,19 @@ export interface DiffResult { added: DiffableViolation[]; resolved: DiffableViolation[]; unchanged_count: number; + /** + * Findings that survived the diff but whose CONTENT changed β€” same stable identity, + * different message, count or details. + * + * `unchanged_count` alone is not actionable for aggregate rules: one violation per + * file with a stable identity stays "unchanged" while its remaining work shrinks, so + * a bare count says something is left but not what. Returning every surviving + * finding instead would make a large unchanged suite echo back its entire violation + * set β€” testsuite validation collects issues from every test case with no size + * bound. Only findings that actually moved are emitted, so this stays bounded by how + * much changed rather than by suite size. + */ + updated: DiffableViolation[]; run_id: string; } @@ -53,16 +66,42 @@ function shortHash(input: string): string { return createHash('sha1').update(input).digest('hex').slice(0, 8); } -/** Build a unique key for a violation so additions/resolutions can be detected. */ +/** + * Build a unique key for a violation so additions/resolutions can be detected. + * + * Prefers an explicit `diff_identity` when a rule supplies one. Aggregate rules emit + * one violation per file carrying a count, and those need an identity that survives + * partial remediation: keying on the rendered message made "13 steps missing X" and + * "12 steps missing X" two different findings, so the baseline diff reported a + * resolution that had not happened. Supplying `diff_identity` lets such a rule keep a + * specific, actionable message while remaining ONE stable finding across fixes. + * + * Falls back to the message for every rule that does not set it, preserving existing + * behaviour for per-step rules where the message is the natural identity. + */ function violationKey(v: DiffableViolation): string { const rule_id = String(v['rule_id'] ?? ''); const applies_to = Array.isArray(v['applies_to']) ? (v['applies_to'] as string[]).join(',') : String(v['applies_to'] ?? ''); + const explicit = v['diff_identity']; + if (typeof explicit === 'string' && explicit.length > 0) { + return `${rule_id}||${applies_to}||${explicit}`; + } const message = String(v['message'] ?? ''); return `${rule_id}||${applies_to}||${message}`; } +/** + * True when two occurrences sharing a stable identity differ in what they report. + * Compares the fields a caller acts on; ignores ordering-insensitive metadata. + */ +function violationContentChanged(before: DiffableViolation, after: DiffableViolation): boolean { + if (String(before['message'] ?? '') !== String(after['message'] ?? '')) return true; + if (String(before['count'] ?? '') !== String(after['count'] ?? '')) return true; + return JSON.stringify(before['details'] ?? null) !== JSON.stringify(after['details'] ?? null); +} + function loadIndex(storageDir: string): RunsIndex { const indexPath = path.join(storageDir, INDEX_FILE); try { @@ -224,13 +263,25 @@ export function computeDiff(baseline: DiffableViolation[], current: DiffableViol } const added: DiffableViolation[] = []; + const updated: DiffableViolation[] = []; const resolved: DiffableViolation[] = []; let unchanged_count = 0; // Tally additions: occurrences in current that exceed baseline count for (const [key, { count: curr, sample }] of currentCounts) { - const base = baselineCounts.get(key)?.count ?? 0; + const baseEntry = baselineCounts.get(key); + const base = baseEntry?.count ?? 0; unchanged_count += Math.min(base, curr); + // A finding that survived but whose CONTENT moved. Only these are emitted β€” not + // every surviving finding. A stable identity keeps a partially-fixed aggregate in + // unchanged_count, and a bare count cannot tell the caller what still needs work; + // but returning every unchanged finding made a large unchanged suite echo back its + // entire violation set, which is what the count exists to avoid. Emitting only the + // ones that actually changed solves the stale-message problem and stays bounded by + // the number of findings that moved, not the size of the suite. + if (baseEntry && Math.min(base, curr) > 0 && violationContentChanged(baseEntry.sample, sample)) { + updated.push(sample); + } const addedCount = curr - base; for (let i = 0; i < addedCount; i++) added.push(sample); } @@ -242,5 +293,5 @@ export function computeDiff(baseline: DiffableViolation[], current: DiffableViol for (let i = 0; i < resolvedCount; i++) resolved.push(sample); } - return { added, resolved, unchanged_count }; + return { added, resolved, unchanged_count, updated }; } diff --git a/test/fixtures/account-ui-gaps-bad.testcase b/test/fixtures/account-ui-gaps-bad.testcase new file mode 100644 index 0000000..0c9cd5e --- /dev/null +++ b/test/fixtures/account-ui-gaps-bad.testcase @@ -0,0 +1,204 @@ + + + Creates an Account through the Salesforce Lightning UI with the mandatory Name field, saves it, and validates the created record. This fixture reproduces the four generation gaps (wrong connect type, dangling connection reference, invalid interaction name, flat field assertions). + + + + + AdminOauth + + + UiConnection + + + Test + + + + fc530f8d-1a88-4ac9-91ce-4686f43e42da + + + + + + + + + AdminOauth + + + + + + Always + + + On SF Account Home screen + + + Default + + + Default + + + false + + + false + + + + + + + + + + + + + + + + Click the New button + + + false + + + false + + + + + + + + + + + + + + AdminOauth + + + + + + Dont + + + NewAccountId + + + On SF Account New screen + + + false + + + false + + + + + + + + + + + + + + + Provar MCP Claude Account + + + + + + + + + + + + + + + + Click Save + + + false + + + false + + + + + + + + + + + AdminOauth + + + + + + Dont + + + On SF Account View screen + + + false + + + false + + + + + + + + + + + + Provar MCP Claude Account + + + + + + + + + + + + Test + + + false + + + + + + + + + + + + diff --git a/test/fixtures/account-ui-gaps-good.testcase b/test/fixtures/account-ui-gaps-good.testcase new file mode 100644 index 0000000..36bca33 --- /dev/null +++ b/test/fixtures/account-ui-gaps-good.testcase @@ -0,0 +1,242 @@ + + + Creates an Account through the Salesforce Lightning UI, saves it, and validates the created record. Corrected shape: ApexConnect, consistent connection references, valid interaction names, nested field assertions. + + + + + AdminOauth + + + fc530f8d-1a88-4ac9-91ce-4686f43e42da + + + AdminOauth + + + Test + + + LightningSales + + + true + + + true + + + Fail + + + + + + + + + + + + + AccountName + + + TestAccount + + + Test + + + + + + + + + + AdminOauth + + + + + + Always + + + On SF Account Home screen + + + false + + + false + + + + + + + + + + + + + + + Click the New button + + + false + + + false + + + + + + + + + + + AdminOauth + + + + + + Dont + + + NewAccountId + + + false + + + false + + + + + + + + + + + + + + + Set the Account Name field + + + false + + + false + + + + + + + + + + + + + + + + + + Click the Save button + + + false + + + false + + + + + + + + + + + AdminOauth + + + + + + Dont + + + On SF Account View screen + + + false + + + false + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + AccountNameAssertion + + + Test + + + false + + + + + + + + + + + diff --git a/test/unit/mcp/bestPracticesEngine.test.ts b/test/unit/mcp/bestPracticesEngine.test.ts index 67df39a..3ac2a36 100644 --- a/test/unit/mcp/bestPracticesEngine.test.ts +++ b/test/unit/mcp/bestPracticesEngine.test.ts @@ -177,16 +177,14 @@ describe('runBestPractices', () => { it('passes for a valid SF target URI (sf:ui:target?object=Account&action=view)', () => { const result = runBestPractices(buildUwsXml('sf:ui:target?object=Account&action=view')); - const uwsViolation = result.violations.find( - (v) => v.rule_id.includes('UI-SCREEN') || v.message.includes('UiWithScreen') - ); + const uwsViolation = result.violations.find((v) => v.rule_id.includes('UI-SCREEN-TARGET')); assert.ok(!uwsViolation, `Expected no uiWithScreenTarget violation, got: ${uwsViolation?.message}`); }); it('passes for a valid page object target URI (ui:pageobject:target?pageId=pageobjects.LoginPage)', () => { const result = runBestPractices(buildUwsXml('ui:pageobject:target?pageId=pageobjects.LoginPage')); const uwsViolation = result.violations.find( - (v) => v.message.includes('UiWithScreen') || v.message.includes('pageId') + (v) => v.rule_id.includes('UI-SCREEN-TARGET') || v.message.includes('pageId') ); assert.ok(!uwsViolation, `Expected no uiWithScreenTarget violation, got: ${uwsViolation?.message}`); }); @@ -509,7 +507,11 @@ describe('runBestPractices', () => { vs[0].message.includes('testItemId=14'), `Expected the violation to name the offending UiAssert (testItemId=14): ${vs[0].message}` ); - assert.equal(result.quality_score, 96.25, `Expected quality_score 96.25, got ${result.quality_score}`); + // Assert THIS rule's contribution, not the whole-file score: the global score + // also carries unrelated advisory rules (e.g. STEP-IDE-PARITY-001 on a + // hand-written fixture), so pinning it here would make the test fail whenever + // any other rule changes β€” while saying nothing about de-duplication. + assert.equal(calculateBPScore(vs), 96.25, `Expected a single majorΓ—5 deduction; got ${calculateBPScore(vs)}`); }); // ─ NEGATIVE: good fixture (assert under action=View) does NOT fire ─ @@ -2027,3 +2029,400 @@ describe('reference doc ⇄ validator apiId parity (PDX-512)', () => { ); }); }); + +// ── STEP-REQUIRED-ARGS-001 (schema-driven required arguments, all step types) ── + +describe('STEP-REQUIRED-ARGS-001 schema-driven required arguments', () => { + const TC = '550e8400-e29b-41d4-a716-4466554409a0'; + const G = (n: number): string => `550e8400-e29b-41d4-a716-4466554409${String(n).padStart(2, '0')}`; + + const stepArgsViolations = (xml: string): BPViolation[] => + runBestPractices(xml).violations.filter((v) => v.rule_id === 'STEP-REQUIRED-ARGS-001'); + + // STEP-RECOMMENDED-ARGS-001 β€” the 80-99% presence band, scored minor/weight 2. + it('STEP-RECOMMENDED-ARGS-001 fires on a conventionally-expected argument and stays minor', () => { + // SendMessage carries to/cc/bcc/subject/signature on 85.7% of real instances: + // conventional, not load-blocking. required (destinationConnectionName, body) present. + const xml = ` + + + + + Mail + hello + + + +`; + const vs = runBestPractices(xml).violations.filter((v) => v.rule_id === 'STEP-RECOMMENDED-ARGS-001'); + assert.equal(vs.length, 1, 'one aggregated recommended-args violation'); + assert.equal(vs[0].severity, 'minor'); + assert.equal(vs[0].weight, 2); + // Argument names live in `details`, not the message: the message is the + // baseline-diff identity and must stay invariant across partial fixes. + assert.ok( + (vs[0].details as { missing_arguments: string[] }).missing_arguments.includes('to'), + `should name the missing conventional arg(s): ${JSON.stringify(vs[0].details)}` + ); + // required args are satisfied, so the major rule must stay silent + assert.equal(stepArgsViolations(xml).length, 0, 'required-args rule must not fire when required args are present'); + }); + + // The aggregated message is the baseline-diff identity (validationDiff keys on + // rule_id||applies_to||message). Counts and step names must stay OUT of it, or + // fixing one of many offending steps reads as "old finding resolved, new finding + // added" instead of "same finding, fewer steps". + it('keeps the aggregated message stable when only the number of offending steps changes', () => { + const step = (tid: number): string => + ` + + `; + const wrap = (steps: string): string => ` +${steps}`; + + const three = stepArgsViolations(wrap(step(1) + step(2) + step(3))); + const two = stepArgsViolations(wrap(step(1) + step(2))); + assert.equal(three.length, 1); + assert.equal(two.length, 1); + assert.equal(three[0].diff_identity, two[0].diff_identity, 'diff identity must be stable across step counts'); + assert.ok(three[0].message !== two[0].message, 'message stays specific and may differ'); + assert.equal(three[0].count, 3); + assert.equal(two[0].count, 2); + // The volatile detail still reaches the caller, just not through the diff key. + assert.equal((three[0].details as { affected_steps: number }).affected_steps, 3); + assert.equal((two[0].details as { affected_steps: number }).affected_steps, 2); + }); + + // The partial-fix case one level up: fixing every instance of ONE missing argument + // while another remains shrinks the missing-name SET. An earlier fix keyed the + // message on that set, so this still reported a resolution that had not happened. + it('keeps the message stable when the SET of missing arguments shrinks', () => { + const wrap = (args: string): string => ` + + + ${args} + +`; + + const both = stepArgsViolations(wrap('')); + const one = stepArgsViolations( + wrap( + '' + ) + ); + assert.equal(both.length, 1); + assert.equal(one.length, 1); + assert.equal(both[0].diff_identity, one[0].diff_identity, 'diff identity must survive a shrinking missing-arg set'); + assert.ok(both[0].message.includes('target'), 'message stays actionable'); + assert.ok(!one[0].message.includes('target'), 'message reflects what is actually still missing'); + assert.deepEqual((both[0].details as { missing_arguments: string[] }).missing_arguments, [ + 'target', + 'uiConnectionName', + ]); + assert.deepEqual((one[0].details as { missing_arguments: string[] }).missing_arguments, ['uiConnectionName']); + }); + + // The schema tier must apply its OWN class rules before falling back to the parity + // helper. Delegating first was a bypass: the parity contract accepts non-empty text + // for any unrecognised class, so text inside a uiLocator or a valueList satisfied the + // group without the uri or populated entry those classes actually require. + it('required_one_of does not accept stray text in place of a class-specific payload', () => { + const uiFill = (inner: string): string => ` + + + ${inner} + +`; + const fires = (inner: string): boolean => stepArgsViolations(uiFill(inner)).length > 0; + + assert.ok(fires('junk'), 'text is not a uri'); + assert.ok( + fires('stray text'), + 'text is not a populated container entry' + ); + // The legitimate forms still satisfy it. + assert.ok(!fires('')); + assert.ok( + !fires( + 'v' + ) + ); + }); + + // diff_identity must be scoped to the test case. Suite validation flattens every + // test case's violations into one array, so a constant identity made same-rule + // aggregates from different files collide on one diff key β€” computeDiff keeps a + // single sample per key, so an unchanged file could mask a partially-fixed one. + it('aggregate diff_identity is scoped per test case, not a shared constant', () => { + const mk = (guid: string): string => ` + + + + +`; + const a = stepArgsViolations(mk(TC))[0]; + const b = stepArgsViolations(mk('550e8400-e29b-41d4-a716-4466554409ff'))[0]; + assert.ok(a?.diff_identity && b?.diff_identity, 'both aggregates carry an identity'); + assert.notEqual(a.diff_identity, b.diff_identity, 'different test cases must not share a diff key'); + }); + + // PARITY GUARD. argumentHasMeaningfulValue mirrors the Quality Hub + // MustContainArgumentValidator, and every mustContainArgument rule shares it, so + // widening it silently desynchronises local and API scores. An earlier revision + // folded URI and container semantics into it: a uiLocator with text but no uri + // flipped to empty, a uri-only one flipped to meaningful. The corpus could not + // detect that β€” no real file carries those shapes in a rule-targeted argument β€” + // which is exactly why it needs a guard rather than a measurement. + it('the backend-parity helper is NOT widened by the schema tier', () => { + const assertStep = (valueXml: string): string => ` + + + + x + EqualTo + ${valueXml} + + +`; + const expectedFires = (valueXml: string): boolean => + runBestPractices(assertStep(valueXml)).violations.some((v) => v.rule_id === 'ASSERT-EXPECTED-001'); + + // Backend contract: a non-empty text value is meaningful whatever its class. + assert.ok(!expectedFires('some text'), 'text is meaningful under parity rules'); + // Backend contract: a uri-only value has no text, so the parity helper treats it + // as empty. The schema tier may disagree; ASSERT-EXPECTED-001 must not. + assert.ok( + expectedFires(''), + 'the parity helper must not start accepting uri-only values for mustContainArgument rules' + ); + }); + + // Recursion guard: an empty container is not a value. Without recursion, + // `` counted as populated and UiFill's one-of group passed on a step + // that does nothing at runtime. + it('required_one_of rejects empty nested containers', () => { + const uiFill = (inner: string): string => ` + + + ${inner} + +`; + const fires = (inner: string): boolean => stepArgsViolations(uiFill(inner)).length > 0; + + // Metadata-only shapes: `valueClass` is a type and a namedValue's `name` is the + // destination field β€” neither assigns anything. A deny-list excluding only + // class/mutable let both count as values, so an inert UiFill still passed. + assert.ok( + fires(''), + 'valueClass alone is a type, not a value' + ); + assert.ok( + fires( + '' + ), + 'a namedValue naming a destination but assigning nothing' + ); + assert.ok( + fires(''), + 'empty namedValues' + ); + assert.ok( + fires( + '' + ), + 'empty namedValue' + ); + assert.ok(fires(''), 'uiLocator with no uri'); + // NOT asserted as inert: argumentHasMeaningfulValue mirrors the Quality Hub + // MustContainArgumentValidator exactly, and there the presence of a element + // makes a variable reference meaningful regardless of its contents. Diverging here + // would break the Layer-2 parity that makes local and API scores agree, so an empty + // is deliberately accepted β€” a different rule's concern, not this one's. + assert.ok( + !fires(''), + 'a element is a variable reference (Quality Hub parity), so the one-of group is satisfied' + ); + + assert.ok( + !fires( + 'x' + ), + 'a populated namedValue is a real value' + ); + assert.ok( + !fires(''), + 'a variable reference with a path element is a real value' + ); + }); + + // required_one_of exists because a UiFill with neither `values` nor `locator` is + // inert. An argument declared but left EMPTY is exactly as inert as an absent one. + it('required_one_of is not satisfied by an empty argument', () => { + const uiFill = (inner: string): string => ` + + + ${inner} + +`; + + const empty = stepArgsViolations(uiFill('')); + assert.equal(empty.length, 1, 'an empty `values` must not satisfy the one-of group'); + assert.ok( + (empty[0].details as { missing_arguments: string[] }).missing_arguments.some((m) => m.startsWith('one of')), + JSON.stringify(empty[0].details) + ); + + // A namedValue that only NAMES a destination assigns nothing β€” this assertion + // originally expected it to pass, encoding the very bypass the recursive check + // now closes. It must carry an actual value. + const populated = stepArgsViolations( + uiFill( + 'Acme Ltd' + ) + ); + assert.equal(populated.length, 0, 'a namedValue carrying a value satisfies the group'); + + const viaLocator = stepArgsViolations( + uiFill('') + ); + assert.equal(viaLocator.length, 0, 'a populated `locator` also satisfies the group'); + }); + + // API-UNKNOWN-001 regression guard: the Provar AI namespace is real. A freshly + // IDE-minted test case writes `com.provar.core.ai.api.*`; listing only the + // never-real `...forcedotcom.core.testapis.ai.*` form made this critical/weight-10 + // rule reject genuine IDE-authored XML and gate is_valid=false. + it('API-UNKNOWN-001 does not fire on real Provar AI apiIds (current or legacy namespace)', () => { + for (const apiId of [ + 'com.provar.core.ai.api.AIAgentSession', + 'com.provar.core.ai.api.AIAgentConversation', + 'com.provar.core.ai.api.ImageValidator', + 'com.provar.core.ai.api.GenerateTestData', + 'com.provar.plugins.forcedotcom.core.testapis.generate.IntentValidator', + ]) { + const xml = ` + + + + +`; + const unknown = runBestPractices(xml).violations.filter((v) => v.rule_id === 'API-UNKNOWN-001'); + assert.equal(unknown.length, 0, `API-UNKNOWN-001 must not fire on the real apiId ${apiId}`); + } + }); + + it('fires when UiConnect uses `connection` instead of the required `connectionName` (the Appendix A bug)', () => { + const xml = ` + + + + AdminOauth + + +`; + const vs = stepArgsViolations(xml); + assert.equal(vs.length, 1, 'UiConnect missing connectionName should raise exactly one violation'); + assert.ok( + (vs[0].details as { missing_arguments: string[] }).missing_arguments.includes('connectionName'), + `should name the missing arg: ${JSON.stringify(vs[0].details)}` + ); + assert.equal(vs[0].severity, 'major', 'rule must be major (does not gate is_valid)'); + }); + + // Aggregated, NOT one violation per step. calculateBPScore damps a single violation + // logarithmically (1 + log2 count) but sums per-violation penalties linearly, so + // per-step emission bypassed the damping: a 71-step file deducted 266 points + // (score 0) where the aggregated form costs 26.8. See STEP-REQUIRED-ARGS-001 notes. + it('aggregates into ONE violation carrying a count, covering every offending step', () => { + const xml = ` + + + + + + + + + + + +`; + const vs = stepArgsViolations(xml); + assert.equal(vs.length, 1, 'one aggregated violation, not one per step'); + assert.equal(vs[0].count, 2, 'count reflects both offending steps'); + const missing = (vs[0].details as { missing_arguments: string[] }).missing_arguments; + assert.ok(missing.includes('uiConnectionName'), 'UiWithScreen missing uiConnectionName'); + assert.ok(missing.includes('interaction'), 'UiDoAction missing interaction'); + }); + + it('scores one aggregated violation far below the per-step equivalent', () => { + // Regression guard for the 10x over-penalty: 2 offending steps must cost + // 5 * 0.75 * (1 + log2 2) = 7.5 points, not 2 * 5 * 0.75 = 7.5 ... at 2 steps the + // two forms coincide; the divergence is what matters at scale, so assert the + // damping is actually applied by checking a high count. + const many = calculateBPScore([ + { rule_id: 'STEP-REQUIRED-ARGS-001', severity: 'major', weight: 5, count: 71 } as never, + ]); + assert.ok(many > 70, `aggregated 71-step penalty should leave score > 70, got ${many}`); + }); + + it('does NOT fire when every required argument is present (no false positive on a correct file)', () => { + const xml = ` + + + + AdminOauth + + + + UiConnection + + + + + + + + + +`; + assert.deepEqual(stepArgsViolations(xml), [], 'complete steps must not be flagged'); + }); + + it('does NOT resurrect the removed phantom args (screenName on UiWithScreen, formLocator on UiFill)', () => { + // A UiWithScreen with target+uiConnectionName but NO screenName, and a UiFill with locator (not formLocator), + // are correct Provar steps β€” the pre-fix schema would have flagged both. Regression guard. + const xml = ` + + + + + UiConnection + + + + + + + + + +`; + const vs = stepArgsViolations(xml); + assert.equal(vs.length, 0, `must not flag screenName/formLocator: ${vs.map((v) => v.message).join(' | ')}`); + }); + + it('excludes UiAssert (covered by the dedicated UI-ASSERT-STRUCT-001 rule)', () => { + const xml = ` + + + + + + +`; + assert.deepEqual(stepArgsViolations(xml), [], 'UiAssert must not be double-reported by STEP-REQUIRED-ARGS-001'); + }); +}); diff --git a/test/unit/mcp/bundledExamples.test.ts b/test/unit/mcp/bundledExamples.test.ts new file mode 100644 index 0000000..89b1358 --- /dev/null +++ b/test/unit/mcp/bundledExamples.test.ts @@ -0,0 +1,65 @@ +/* + * 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 { describe, it } from 'mocha'; +import { BUNDLED_EXAMPLES, selectBundledExamples } from '../../../src/mcp/examples/bundledExamples.js'; +import { runBestPractices } from '../../../src/mcp/tools/bestPracticesEngine.js'; +import { validateTestCase } from '../../../src/mcp/tools/testCaseValidate.js'; + +// Every bundled example MUST pass the same validators the server exposes β€” otherwise +// we would be shipping the exact "looks right but is wrong" XML the fix set out to prevent. + +describe('bundled examples', () => { + it('ships at least one example', () => { + assert.ok(BUNDLED_EXAMPLES.length >= 1); + }); + + for (const ex of BUNDLED_EXAMPLES) { + describe(`example: ${ex.id}`, () => { + it('is structurally valid (validity_score 100, is_valid)', () => { + const meta = validateTestCase(ex.xml, undefined, {}); + assert.equal(meta.is_valid, true, `structural errors: ${JSON.stringify(meta.issues ?? [])}`); + assert.equal(meta.validity_score, 100); + }); + + it('scores 100 with zero best-practice violations', () => { + const bp = runBestPractices(ex.xml); + assert.equal( + bp.violations.length, + 0, + `violations: ${bp.violations.map((v) => `${v.rule_id}: ${v.message}`).join(' | ')}` + ); + assert.equal(bp.quality_score, 100); + }); + }); + } + + it('selectBundledExamples returns up to n, labelled bundled, with full XML', () => { + const picked = selectBundledExamples('create and validate an account via ui', 5); + assert.ok(picked.length >= 1 && picked.length <= 5); + assert.ok(picked.every((p) => p.source === 'bundled' && p.quality_tier === 'bundled')); + assert.ok(picked[0].xml.includes(' { + assert.deepEqual(selectBundledExamples('zzz nothing matches here', 1), []); + assert.deepEqual(selectBundledExamples('rest api soap request database insert', 5), []); + }); + + it('ranks a keyword-matching example and keeps similarity_score below a real corpus match', () => { + const onTopic = selectBundledExamples('ui account create validate', 1)[0]; + assert.ok(onTopic, 'an on-topic query must still return the bundled example'); + assert.ok(onTopic.similarity_score > 0, 'on-topic match scores above zero'); + assert.ok(onTopic.similarity_score < 1, 'keyword proxy must never claim a perfect corpus-grade match'); + }); +}); diff --git a/test/unit/mcp/loopPrompts.test.ts b/test/unit/mcp/loopPrompts.test.ts index a183c54..d52a230 100644 --- a/test/unit/mcp/loopPrompts.test.ts +++ b/test/unit/mcp/loopPrompts.test.ts @@ -347,9 +347,16 @@ describe('loopPrompts β€” provar.loop.db', () => { assert.ok(text.includes('ApexConnect'), 'should name ApexConnect in the forbidden list'); }); - it('includes step-reference fallback instruction', () => { + // The fallback must name a TOOL, not an MCP resource: a tools-only client cannot + // read resources, so `provar://docs/step-reference` was a dead end for exactly the + // clients most likely to need the fallback. + it('falls back to the provar_step_schema tool, not the unreadable resource', () => { const result = server.call('provar.loop.db', { story: 'any db test' }); const text = getMessageText(result); - assert.ok(text.includes('provar://docs/step-reference'), 'should include step-reference fallback'); + assert.ok(text.includes('provar_step_schema'), 'should route the fallback to the step-schema tool'); + assert.ok( + !text.includes('provar://docs/step-reference'), + 'must not send a tools-only client to an MCP resource it cannot read' + ); }); }); diff --git a/test/unit/mcp/qualityHubApiTools.test.ts b/test/unit/mcp/qualityHubApiTools.test.ts index 0f453c0..575b9ba 100644 --- a/test/unit/mcp/qualityHubApiTools.test.ts +++ b/test/unit/mcp/qualityHubApiTools.test.ts @@ -121,30 +121,41 @@ describe('qualityHubApiTools', () => { assert.equal(opts.prefer_high_quality, false); }); - it('returns empty examples with no isError when API key is not configured', async () => { + it('returns BUNDLED examples (not empty) with no isError when API key is not configured', async () => { resolveKeyStub.returns(null); - const result = await server.call('provar_qualityhub_examples_retrieve', { query: 'Create an Opportunity' }); + const result = await server.call('provar_qualityhub_examples_retrieve', { + query: 'Create and validate an Account via UI', + }); // CRITICAL: must NOT be isError:true β€” the LLM workflow must continue assert.equal(isError(result), false, 'Must not set isError:true when key missing'); const body = parseBody(result); - assert.deepEqual(body.examples, []); - assert.equal(body.count, 0); + const examples = body.examples as Array<{ source?: string; quality_tier?: string; xml?: string }>; + assert.ok(examples.length > 0, 'Should return bundled examples instead of empty'); + assert.equal(body.count, examples.length); + assert.equal(body.source, 'bundled'); + assert.ok( + examples.every((e) => e.source === 'bundled' && e.quality_tier === 'bundled'), + 'Every example must be labelled bundled' + ); + assert.ok(examples[0].xml && examples[0].xml.includes(' 0, 'Should include warning message'); assert.ok(warning.includes('sf provar auth login'), 'Warning should mention auth login'); + assert.ok(warning.toLowerCase().includes('bundled'), 'Warning should flag examples as bundled'); }); - it('returns empty examples with no isError on 401 auth error', async () => { + it('returns BUNDLED examples with no isError on 401 auth error', async () => { retrieveStub.rejects(new QualityHubAuthError('Key invalid')); - const result = await server.call('provar_qualityhub_examples_retrieve', { query: 'Create an Opportunity' }); + const result = await server.call('provar_qualityhub_examples_retrieve', { query: 'Create an Account via UI' }); // CRITICAL: must NOT be isError:true β€” graceful degrade assert.equal(isError(result), false, 'Must not set isError:true on auth failure'); const body = parseBody(result); - assert.deepEqual(body.examples, []); + const examples = body.examples as Array<{ source?: string }>; + assert.ok(examples.length > 0, 'Bad key should still yield bundled examples'); + assert.equal(body.source, 'bundled'); assert.ok(typeof body.warning === 'string', 'Should include warning'); }); diff --git a/test/unit/mcp/stepSchemaTools.test.ts b/test/unit/mcp/stepSchemaTools.test.ts new file mode 100644 index 0000000..5c21141 --- /dev/null +++ b/test/unit/mcp/stepSchemaTools.test.ts @@ -0,0 +1,185 @@ +/* + * 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 { readFileSync } from 'node:fs'; +import { join } from 'node:path'; +import { describe, it } from 'mocha'; +import { registerAllStepSchemaTools } from '../../../src/mcp/tools/stepSchemaTools.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); + } +} + +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 StepArg { + id: string; +} +interface StepEntry { + apiId: string; + required_arguments?: StepArg[]; + optional_arguments?: StepArg[]; +} + +function makeServer(): MockMcpServer { + const server = new MockMcpServer(); + registerAllStepSchemaTools(server as unknown as Parameters[0]); + return server; +} + +describe('provar_step_schema', () => { + it('returns the schema for a step type by short name (UiConnect)', () => { + const res = makeServer().call('provar_step_schema', { api_id: 'UiConnect' }); + assert.ok(!isError(res)); + const step = parseText(res).step as StepEntry; + assert.equal(step.apiId, 'com.provar.plugins.forcedotcom.core.ui.UiConnect'); + const req = (step.required_arguments ?? []).map((a) => a.id); + assert.deepEqual(req, ['connectionName'], 'UiConnect requires connectionName'); + }); + + it('resolves by full apiId as well as short name', () => { + const res = makeServer().call('provar_step_schema', { + api_id: 'com.provar.plugins.forcedotcom.core.ui.UiDoAction', + }); + const step = parseText(res).step as StepEntry; + const req = (step.required_arguments ?? []).map((a) => a.id); + assert.deepEqual(req, ['locator', 'interaction']); + }); + + it('reflects the corrected schema β€” no phantom screenName/formLocator', () => { + const server = makeServer(); + const uws = ( + parseText(server.call('provar_step_schema', { api_id: 'UiWithScreen' })).step as StepEntry + ).required_arguments!.map((a) => a.id); + assert.ok(!uws.includes('screenName'), `UiWithScreen must not require phantom screenName: ${uws.join(',')}`); + assert.deepEqual(uws, ['uiConnectionName', 'target']); + + // UiFill requires nothing. Only 2 real UiFill steps exist across the 1,457-file + // corpus and both carry `values`/`ignoreFields`, never `locator` β€” requiring + // `locator` dropped a genuine IDE-exported fixture (Contact_Lead_flat) to 42.75. + const fill = ( + parseText(server.call('provar_step_schema', { api_id: 'UiFill' })).step as StepEntry + ).required_arguments!.map((a) => a.id); + assert.ok(!fill.includes('formLocator'), `UiFill must not require phantom formLocator: ${fill.join(',')}`); + assert.deepEqual(fill, []); + }); + + it('lists all step types in a category', () => { + const res = makeServer().call('provar_step_schema', { category: 'UI' }); + const body = parseText(res); + assert.ok((body.count as number) > 0); + const steps = body.steps as Array<{ apiId: string }>; + assert.ok(steps.every((s) => s.apiId.length > 0)); + assert.ok(steps.some((s) => s.apiId.endsWith('.UiConnect'))); + }); + + it('returns the full index when no arguments are given', () => { + const body = parseText(makeServer().call('provar_step_schema', {})); + assert.ok((body.count as number) >= 60, 'index should cover the full step catalog'); + const steps = body.steps as Array<{ name: string; apiId: string; category: string }>; + assert.ok(steps.every((s) => s.name && s.apiId && s.category)); + }); + + it('returns STEP_TYPE_NOT_FOUND with a suggestion for an unknown api_id', () => { + const res = makeServer().call('provar_step_schema', { api_id: 'UiTotallyMadeUp' }); + assert.ok(isError(res)); + const err = parseText(res); + assert.equal(err.error_code, 'STEP_TYPE_NOT_FOUND'); + assert.ok(err.details, 'should include a suggestion'); + }); +}); + +// Drift guard: the schema is the single source of truth for generation guidance. +// These assertions lock the corrections that steer generation away from the four +// gaps, so the guidance can never silently regress (e.g. back to recommending +// UiConnect or interaction=click). +describe('step schema generation guidance (drift guard)', () => { + const schema = JSON.parse( + readFileSync(join(process.cwd(), 'src', 'mcp', 'rules', 'provar_test_step_schema.json'), 'utf-8') + ) as { + apiCalls: { UI: Record }; + common_patterns: { ui_interaction_pattern: { steps: Array<{ step: string }> } }; + }; + const rulesText = (step: string): string => (schema.apiCalls.UI[step].validation_rules ?? []).join(' | '); + + it('ui_interaction_pattern opens with ApexConnect, not UiConnect (gap 1)', () => { + const first = schema.common_patterns.ui_interaction_pattern.steps[0].step; + assert.ok(first.includes('ApexConnect'), `pattern should open with ApexConnect, got: ${first}`); + assert.ok(!first.startsWith('UiConnect'), 'pattern must not open with UiConnect'); + }); + + it('UiConnect guidance steers Salesforce UI tests to ApexConnect (gap 1)', () => { + const t = rulesText('UiConnect'); + assert.ok(t.includes('ApexConnect') && /browser-only/i.test(t), `UiConnect guidance missing steer: ${t}`); + }); + + // The guidance served by provar_step_schema MUST agree with what the validator + // accepts. `click` is a real Provar interaction (1,240 corpus occurrences, emitted + // by the IDE recorder) and UI-INTERACTION-002 no longer rejects it β€” an earlier + // revision of this guard asserted the opposite and locked the contradiction in, so + // the tool told the model to rewrite interactions the validator accepts. + it('UiDoAction guidance names real interactions and does NOT forbid click (gap 3)', () => { + const t = rulesText('UiDoAction'); + assert.ok(t.includes('action') && t.includes('set'), `interaction vocab missing: ${t}`); + assert.ok(!/NEVER use[^.]*click/i.test(t), `guidance must not forbid the real interaction "click": ${t}`); + assert.ok(/\btype\b/.test(t) && /\bfill\b/.test(t), `guidance should name the borrowed-name pitfalls: ${t}`); + }); + + // The BUNDLED REFERENCE DOC is a fourth surface an agent reads. Three successive + // fixes corrected the validator, the schema, the registry and the emitted + // suggestion, and each time this file was missed β€” it labelled a three-item list + // "Interaction types" with no qualifier, which reads as an allow-list excluding + // click. Guard it alongside the others. + it('the bundled step reference presents interactions as non-exhaustive and includes click', () => { + const doc = readFileSync(join(process.cwd(), 'docs', 'PROVAR_TEST_STEP_REFERENCE.md'), 'utf-8'); + const section = doc.slice(doc.indexOf('**Interaction types'), doc.indexOf('**Locator URI format:**')); + assert.ok(section.length > 0, 'interaction section not found'); + assert.ok(section.includes('name=click'), 'reference must list click as a real interaction'); + assert.ok( + /not limited to|non-exhaustive|subset|examples, not an/i.test(section), + `reference must not present the list as exhaustive: ${section.slice(0, 200)}` + ); + for (const bad of ['type', 'fill', 'enter', 'input', 'tap', 'press']) { + assert.ok(section.includes(bad), `reference should name the rejected borrowed name "${bad}"`); + } + }); + + // Drift guard tying the schema to the validator's denylist, so the two can never + // disagree about which interaction names are wrong. + it('schema guidance and UI-INTERACTION-002 agree on the invalid names', () => { + const t = rulesText('UiDoAction').toLowerCase(); + for (const bad of ['type', 'fill', 'enter', 'input', 'tap', 'press']) { + assert.ok(t.includes(bad), `schema should warn against "${bad}" (the validator rejects it): ${t}`); + } + assert.ok(!t.includes('never use name=click'), 'schema must not contradict the validator on click'); + }); + + it('UiAssert guidance requires nested uiFieldAssertion and warns off namedValues (gap 4)', () => { + const t = rulesText('UiAssert'); + assert.ok(t.includes('uiFieldAssertion') && t.includes('namedValues'), `assert nesting guidance missing: ${t}`); + }); +}); diff --git a/test/unit/mcp/testCaseGenerate.test.ts b/test/unit/mcp/testCaseGenerate.test.ts index 2616b99..edb56bf 100644 --- a/test/unit/mcp/testCaseGenerate.test.ts +++ b/test/unit/mcp/testCaseGenerate.test.ts @@ -86,7 +86,7 @@ afterEach(() => { // ── tool description ────────────────────────────────────────────────────────── describe('provar_testcase_generate description', () => { - it('references corpus tool and step-reference fallback', () => { + it('references corpus tool and step-schema fallback', () => { const reg = server.registrations.find((r) => r.name === 'provar_testcase_generate'); assert.ok(reg, 'tool should be registered'); assert.ok( @@ -94,8 +94,8 @@ describe('provar_testcase_generate description', () => { 'description should reference corpus tool' ); assert.ok( - reg.description.includes('provar://docs/step-reference'), - 'description should include step-reference fallback' + reg.description.includes('provar_step_schema'), + 'description should include the provar_step_schema fallback' ); }); diff --git a/test/unit/mcp/testCaseStepTools.test.ts b/test/unit/mcp/testCaseStepTools.test.ts index c353d26..fc3a1bf 100644 --- a/test/unit/mcp/testCaseStepTools.test.ts +++ b/test/unit/mcp/testCaseStepTools.test.ts @@ -94,7 +94,7 @@ afterEach(() => { // ── tool description ────────────────────────────────────────────────────────── describe('provar_testcase_step_edit description', () => { - it('references corpus tool and step-reference fallback', () => { + it('references corpus tool and step-schema fallback', () => { const reg = server.registrations.find((r) => r.name === 'provar_testcase_step_edit'); assert.ok(reg, 'tool should be registered'); assert.ok( @@ -102,8 +102,8 @@ describe('provar_testcase_step_edit description', () => { 'description should reference corpus tool' ); assert.ok( - reg.description.includes('provar://docs/step-reference'), - 'description should include step-reference fallback' + reg.description.includes('provar_step_schema'), + 'description should include the provar_step_schema fallback' ); }); diff --git a/test/unit/mcp/testCaseValidate.test.ts b/test/unit/mcp/testCaseValidate.test.ts index c3ca6d6..f62020b 100644 --- a/test/unit/mcp/testCaseValidate.test.ts +++ b/test/unit/mcp/testCaseValidate.test.ts @@ -27,6 +27,7 @@ import { const GUID_TC = '550e8400-e29b-41d4-a716-446655440000'; const GUID_S1 = '6ba7b810-9dad-4000-8000-00c04fd430c8'; const GUID_S2 = '6ba7b811-9dad-4001-9001-00c04fd430c8'; +const GUID_S3 = '6ba7b812-9dad-4002-a002-00c04fd430c8'; const VALID_TC = ` @@ -955,6 +956,292 @@ describe('validateTestCase', () => { 'UI-ASSERT-STRUCTURE-001 must only apply to UiAssert steps' ); }); + + // Recursion coverage: UI steps live nested under UiWithScreen substeps. The + // per-call structural rules must run there too, not just at the top level. + it('fires for a NESTED UiAssert with the flat namedValues shape', () => { + const r = validateTestCase( + ` + + + + + + + + X + + + + + +` + ); + assert.ok( + r.issues.some((i) => i.rule_id === 'UI-ASSERT-STRUCTURE-001'), + 'nested UiAssert must be reached by the structural rule' + ); + }); + }); + + // Phase-2 generation-gap rules, dogfooded against the account-ui fixtures. + describe('generation-gap rules (account-ui fixtures)', () => { + const bad = fs.readFileSync( + path.resolve(process.cwd(), 'test', 'fixtures', 'account-ui-gaps-bad.testcase'), + 'utf-8' + ); + const good = fs.readFileSync( + path.resolve(process.cwd(), 'test', 'fixtures', 'account-ui-gaps-good.testcase'), + 'utf-8' + ); + const GAP_RULES = [ + 'SF-CONNECT-TYPE-001', + 'CONNECT-REF-CONSISTENCY-001', + 'UI-INTERACTION-002', + 'UI-ASSERT-STRUCTURE-001', + ]; + + it('bad fixture: all four gap rules fire and is_valid is false', () => { + const r = validateTestCase(bad); + for (const rule of GAP_RULES) { + assert.ok( + r.issues.some((i) => i.rule_id === rule), + `expected ${rule} to fire on the bad fixture` + ); + } + assert.equal(r.is_valid, false, 'ERROR-tier gap rules must gate is_valid'); + }); + + it('good fixture: none of the gap rules fire and is_valid is true', () => { + const r = validateTestCase(good); + const fired = r.issues.filter((i) => GAP_RULES.includes(i.rule_id)).map((i) => i.rule_id); + assert.deepEqual( + [...new Set(fired)], + [], + `no gap rule should fire on the good fixture, got: ${fired.join(', ')}` + ); + assert.equal(r.is_valid, true); + }); + }); + + describe('UI-INTERACTION-002 (invalid interaction name)', () => { + it('errors on interaction name="type" and suggests "set"', () => { + const r = validateTestCase( + ` + + + + + + + + + +` + ); + const issue = r.issues.find((i) => i.rule_id === 'UI-INTERACTION-002'); + assert.ok(issue, 'expected UI-INTERACTION-002'); + assert.equal(issue.severity, 'ERROR'); + assert.ok(issue.message.includes('set'), 'should steer to name="set"'); + }); + + // The EMITTED suggestion is user-visible and is what an agent acts on. An earlier + // fix corrected the denylist, the schema and the docs but left this text saying + // "Valid UiDoAction interactions are: action, set, file" β€” exhaustive phrasing + // that still reads as declaring `click` invalid. The drift guard checked schema + // text only, so this escaped. + it('the emitted suggestion does not present an exhaustive list that excludes click', () => { + const r = validateTestCase( + ` + + + + + + + + + +` + ); + const issue = r.issues.find((i) => i.rule_id === 'UI-INTERACTION-002'); + assert.ok(issue?.suggestion, 'expected a suggestion'); + const s = issue.suggestion; + assert.ok(!/valid[^.]*interactions are:/i.test(s), `suggestion must not read as exhaustive: ${s}`); + assert.ok(s.includes('click'), `suggestion should name click as valid: ${s}`); + assert.ok(s.includes('set'), `suggestion should still steer to the correction: ${s}`); + }); + + // `click` is a REAL Provar interaction, not a hallucination: 1,240 well-formed + // occurrences across the 2,701-file corpus, third only to action (15,983) and + // set (11,787), emitted by the IDE recorder. Denylisting it flipped 164 real + // customer test cases to is_valid=false. Regression guard for that. + it('does NOT fire on name="click" β€” a real IDE-emitted Provar interaction', () => { + const r = validateTestCase( + ` + + + + + + + + + +` + ); + assert.ok( + !r.issues.some((i) => i.rule_id === 'UI-INTERACTION-002'), + 'UI-INTERACTION-002 must not fire on the real interaction name "click"' + ); + }); + + it('does not fire for valid interaction names (action, set)', () => { + for (const name of ['action', 'set']) { + const r = validateTestCase( + ` + + + + + + + + + +` + ); + assert.ok( + !r.issues.some((i) => i.rule_id === 'UI-INTERACTION-002'), + `UI-INTERACTION-002 must not fire for valid name="${name}"` + ); + } + }); + }); + + describe('CONNECT-REF-CONSISTENCY-001', () => { + it('does not fire when no connect step exists (connection inherited from parent)', () => { + const r = validateTestCase( + ` + + + + + Inherited + + + + +` + ); + assert.ok( + !r.issues.some((i) => i.rule_id === 'CONNECT-REF-CONSISTENCY-001'), + 'must not fire without a connect step in the test' + ); + }); + + // A connect step whose name is non-literal makes only its OWN family + // unverifiable. Suppressing the whole file let genuinely dangling references of + // unrelated families through β€” a real false negative on intentional-violation + // fixtures that carry a dynamic DbConnect alongside broken UI references. + it('an opaque DbConnect does not silence a dangling UI reference', () => { + const r = validateTestCase( + ` + + + + + + + RealUi + + + + NoSuchConnection + + + + +` + ); + assert.ok( + r.issues.some((i) => i.rule_id === 'CONNECT-REF-CONSISTENCY-001'), + 'a dangling ui reference must still be reported despite an opaque db connect' + ); + }); + + it('an opaque connect DOES silence a reference of its own family', () => { + const r = validateTestCase( + ` + + + + + + + Unknown + + +` + ); + assert.ok( + !r.issues.some((i) => i.rule_id === 'CONNECT-REF-CONSISTENCY-001'), + 'the opaque db connect could be what produces this db reference' + ); + }); + + // Connection results are family-scoped. A single global name set let a DbConnect + // result satisfy a uiConnectionName, suppressing a genuinely dangling reference + // whenever two families happened to share a name. + it('a DB connect result does not satisfy a UI connection reference of the same name', () => { + const r = validateTestCase( + ` + + + + Shared + + + + Shared + + + + +` + ); + assert.ok( + r.issues.some((i) => i.rule_id === 'CONNECT-REF-CONSISTENCY-001'), + 'a db result must not satisfy a ui reference merely by sharing a name' + ); + }); + + it('accepts a connection declared in the project rather than by a connect step', () => { + const xml = ` + + + + Local + + + + ProjectLevel + + + + +`; + assert.ok( + validateTestCase(xml).issues.some((i) => i.rule_id === 'CONNECT-REF-CONSISTENCY-001'), + 'without project context the reference looks dangling' + ); + assert.ok( + !validateTestCase(xml, undefined, { projectConnectionNames: new Set(['ProjectLevel']) }).issues.some( + (i) => i.rule_id === 'CONNECT-REF-CONSISTENCY-001' + ), + 'a project-declared connection is a valid reference target' + ); + }); }); describe('SETVALUES-STRUCTURE-001', () => { @@ -1842,13 +2129,13 @@ describe('provar_testcase_validate description', () => { } } - it('includes step-reference guidance', () => { + it('includes step-schema tool guidance', () => { const srv = new DescriptionCapturingServer(); registerTestCaseValidate(srv as unknown as McpServer, { allowedPaths: [] }); assert.ok(srv.capturedDescription, 'description should be captured'); assert.ok( - String(srv.capturedDescription).includes('provar://docs/step-reference'), - 'description should include step-reference guidance' + String(srv.capturedDescription).includes('provar_step_schema'), + 'description should point at the provar_step_schema tool for step attribute schemas' ); }); });