feat(mcp): wave 2 — non-SF generation fixes + provar.connection.list tool by mrdailey99 · Pull Request #134 · ProvarTesting/provardx-cli

mrdailey99 · 2026-04-24T21:34:15Z

Summary

testCaseGenerate: Fixed two P1 bugs: inner steps now correctly wrapped in <clauses><clause name="substeps">, and non-SF target URIs now use query-param format (ui:pageobject:target?pageId=pageobjects.X) instead of colon format
bestPracticesEngine: Implemented the uiWithScreenTarget validator (was silently passing all test cases due to missing VALIDATOR_REGISTRY entry); fixed getUiWithScreenTarget to read the correct XML path (arguments/argument not argument)
pageObjectGenerate: Added FILE_EXISTS guard on SSO stub write, matching the existing guard on the main .java file — prevents silent overwrite and gives agents a clear retry path
qualityHubTools: Narrowed wildcard flag detection to only flag wildcards in --plan-name values; previously flagged unrelated flag values
connectionTools (new): provar.connection.list — reads connections and named environments from the .testproject XML using fast-xml-parser; returns type, URL, SSO status; never exposes credential values from .secrets

Tests

761 tests passing (5 pre-existing infrastructure-dependent smoke timeouts unrelated to this change)
New unit tests: connectionTools.test.ts (happy path, path policy, missing file, XML parsing edge cases), bestPracticesEngine.test.ts (4 uiWithScreenTarget tests), pageObjectGenerate.test.ts (FILE_EXISTS on SSO stub), qualityHubTools.test.ts (wildcard scope fix)
TypeScript: clean compile
Lint: clean

Test plan

Verify provar.testcase.generate with page_object_uri produces <clauses><clause name="substeps"> wrapping inner steps
Verify non-SF page object URI resolves to ui:pageobject:target?pageId=pageobjects.<Class> format
Verify provar.testcase.validate fires the uiWithScreenTarget rule on colon-format URIs
Verify provar.pageobject.generate with sso_class returns FILE_EXISTS when SSO stub already exists
Verify provar.connection.list returns correct connections and environments from a real .testproject

🤖 Generated with Claude Code

…n.list Req: Wave 2 engineering review — fix non-SF page object generation bugs and add connection discovery tool for AI agents Fix: correct URI format, substeps XML structure, uiWithScreenTarget validator, SSO guard, wildcard scope, connection.list tool - testCaseGenerate: query-param URI (ui:pageobject:target?pageId=pageobjects.X) and substeps clause - bestPracticesEngine: implement uiWithScreenTarget validator, fix arguments path - pageObjectGenerate: FILE_EXISTS guard on SSO stub write - qualityHubTools: narrow wildcard detection to --plan-name values only - connectionTools: new provar.connection.list reads .testproject connections and environments Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

This PR expands and hardens the MCP toolset for Provar DX (“wave 2”), addressing generation/validation issues for non-Salesforce targets, tightening Quality Hub CLI flag handling, and adding a new connection discovery tool.

Changes:

Update provar.testcase.generate to support URI-aware nesting (non-SF page objects via UiWithScreen) and add optional post-generation validation (validate_after_edit).
Fix bestPracticesEngine by registering the missing uiWithScreenTarget validator and correcting UiWithScreen target parsing.
Add provar.connection.list to parse .testproject connections/environments, plus tests/docs/smoke coverage; also add SSO stub overwrite protection in provar.pageobject.generate and narrow Quality Hub wildcard warnings.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
`src/mcp/tools/testCaseGenerate.ts`	Adds `target_uri` nesting + `validate_after_edit` behavior and refactors XML building.
`test/unit/mcp/testCaseGenerate.test.ts`	Adds coverage for non-SF `ui:` nesting and validation field behavior.
`src/mcp/tools/bestPracticesEngine.ts`	Implements and registers `uiWithScreenTarget` validator and target extraction fix.
`test/unit/mcp/bestPracticesEngine.test.ts`	Adds `uiWithScreenTarget` validator tests.
`src/mcp/tools/pageObjectGenerate.ts`	Adds `sso_class` stub generation and guards against overwriting the stub.
`test/unit/mcp/pageObjectGenerate.test.ts`	Adds tests for SSO stub generation, path policy, and FILE_EXISTS behavior.
`src/mcp/tools/qualityHubTools.ts`	Narrows wildcard warning detection scope and returns warnings via `details`.
`test/unit/mcp/qualityHubTools.test.ts`	Adds tests for wildcard warning behavior and reformats existing cases.
`src/mcp/tools/connectionTools.ts`	New tool to list connections/environments from `.testproject`.
`test/unit/mcp/connectionTools.test.ts`	New tests for connection listing, path policy, missing file handling.
`src/mcp/server.ts`	Registers the new connection tools.
`scripts/mcp-smoke.cjs`	Adds smoke coverage for `provar.connection.list` and updates expected response count.
`docs/mcp.md`	Documents `provar.connection.list` and updates existing tool docs for new options/outputs.
`TODOS.md`	Tracks known follow-up work around partial-write error handling in page object generation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+      try {
+        const resolvedPath = path.resolve(project_path);
+        assertPathAllowed(resolvedPath, config.allowedPaths);
+
+        const testProjectPath = path.join(resolvedPath, '.testproject');
+        if (!fs.existsSync(testProjectPath)) {
+          const err = makeError(
+            'CONNECTION_FILE_NOT_FOUND',
+            `No .testproject file found at: ${testProjectPath}. Run provar.project.validate first to confirm the project structure.`,
+            requestId,
+            false,
+            { suggestion: 'Run provar.project.validate with the project_path to confirm the project root, then retry.' }
+          );
+          return { isError: true, content: [{ type: 'text' as const, text: JSON.stringify(err) }] };
+        }
+
+        let content: string;
+        try {
+          content = fs.readFileSync(testProjectPath, 'utf-8');


+function parseTestProjectXml(content: string): Record<string, unknown> {
+  let parsed: Record<string, unknown>;
+  try {
+    parsed = TP_PARSER.parse(content) as Record<string, unknown>;
+  } catch {
+    return {};
+  }
+  const raw = parsed['testProject'];
+  return raw !== null && typeof raw === 'object' ? (raw as Record<string, unknown>) : {};
+}


@@ -110,24 +128,42 @@ export function registerPageObjectGenerate(server: McpServer, config: ServerConf
          fs.writeFileSync(filePath, javaSource, 'utf-8');
          written = true;
          log('info', 'provar.pageobject.generate: wrote file', { requestId, filePath });
+
+          if (ssoSource && ssoFilePath) {
+            if (fs.existsSync(ssoFilePath) && !input.overwrite) {
+              const err = makeError(
+                'FILE_EXISTS',
+                `SSO stub file already exists: ${ssoFilePath}. Set overwrite=true to replace.`,
+                requestId
+              );
+              return { isError: true, content: [{ type: 'text' as const, text: JSON.stringify(err) }] };
+            }


    'provar.qualityhub.testrun',
-    'Trigger a Quality Hub test run. Invokes `sf provar quality-hub test run`.',
+    'Trigger a Quality Hub test run. Invokes `sf provar quality-hub test run`. ' +
+      'Warning: wildcard characters (* or ?) in flag values will cause QH plan-level reporting to be skipped — use exact plan names.',
    {
      target_org: z.string().describe('SF org alias or username'),
-      flags: z.array(z.string()).optional().default([]).describe('Additional raw CLI flags (e.g. ["--plan-name", "SmokeTests"])'),
+      flags: z
+        .array(z.string())
+        .optional()
+        .default([])
+        .describe(
+          'Additional raw CLI flags (e.g. ["--plan-name", "SmokeTests"]). Avoid wildcards in plan names — they skip QH plan-level reporting.'
+        ),


+| Parameter                   | Type                                                   | Required | Description                                                                                             |
+| --------------------------- | ------------------------------------------------------ | -------- | ------------------------------------------------------------------------------------------------------- |
+| `class_name`                | string                                                 | yes      | PascalCase Java class name (e.g. `AccountDetailPage`)                                                   |
+| `package_name`              | string                                                 | yes      | Java package (e.g. `pageobjects.accounts`)                                                              |
+| `page_type`                 | `standard` \| `salesforce`                             | yes      | Generates `@Page` or `@SalesforcePage` annotation                                                       |
+| `title`                     | string                                                 | no       | Page title for the annotation                                                                           |
+| `connection_name`           | string                                                 | no       | Salesforce connection name (for `@SalesforcePage`)                                                      |
+| `salesforce_page_attribute` | string                                                 | no       | Additional Salesforce page attribute                                                                    |
+| `fields`                    | array of `{ name, type, locator_type, locator_value }` | no       | WebElement field definitions                                                                            |
+| `sso_class`                 | string                                                 | no       | PascalCase class name for an `ILoginPage` stub (non-SF SSO). Written alongside the page object on disk. |
+| `output_path`               | string                                                 | no       | Full file path to write (must be within `allowed-paths`)                                                |
+| `overwrite`                 | boolean                                                | no       | Overwrite existing file (default: `false`)                                                              |
+| `dry_run`                   | boolean                                                | no       | Return content without writing to disk                                                                  |
+| `idempotency_key`           | string                                                 | no       | Prevents duplicate generation for the same key                                                          |


+| Parameter             | Type                                    | Required | Description                                                                                                               |
+| --------------------- | --------------------------------------- | -------- | ------------------------------------------------------------------------------------------------------------------------- |
+| `test_case_name`      | string                                  | yes      | Human-readable test case name                                                                                             |
+| `test_case_id`        | string                                  | no       | Custom test case ID (auto-generated if omitted)                                                                           |
+| `steps`               | array of `{ api_id, name, arguments? }` | no       | Step definitions                                                                                                          |
+| `target_uri`          | string                                  | no       | Page object URI. `ui:pageobject:target?pageId=pageobjects.X` triggers `UiWithScreen` nesting; `sf:` or absent → flat.     |
+| `output_path`         | string                                  | no       | File path to write (must be within `allowed-paths`)                                                                       |
+| `overwrite`           | boolean                                 | no       | Overwrite existing file (default: `false`)                                                                                |
+| `dry_run`             | boolean                                 | no       | Return XML without writing to disk                                                                                        |
+| `validate_after_edit` | boolean                                 | no       | Run structural validation after generation (default: `true`). Returns `TESTCASE_INVALID` if invalid. Set `false` to skip. |
+| `idempotency_key`     | string                                  | no       | Prevents duplicate generation for the same key                                                                            |


+            const errResult = makeError(
+              'TESTCASE_INVALID',
+              `Generated test case failed structural validation (${validationFull.error_count} error(s)). See validation field.`,
+              requestId,
+              false,
+              { validation: validationSlim }
+            );
+            log('warn', 'provar.testcase.generate: TESTCASE_INVALID', { requestId });
+            return { isError: true, content: [{ type: 'text' as const, text: JSON.stringify(errResult) }] };


+  for (const call of getAllApiCalls(tc)) {
+    const apiId = call['@_apiId'] as string | undefined;
+    if (!apiId) continue;
+    if (targetApiId && !apiId.includes('UiWithScreen')) continue;


 Triggers a Quality Hub test run. Invokes `sf provar quality-hub test run`. Returns the test run ID which can be passed to `provar.qualityhub.testrun.report` to poll for results.

+> **Wildcard warning:** if any value in `flags` contains `*` or `?`, the tool adds `details.warning` explaining that QH plan-level reporting will be skipped. Execution still proceeds — the warning is non-blocking.
+
 **Input**

-| Parameter    | Type     | Required | Description                                                                   |
-| ------------ | -------- | -------- | ----------------------------------------------------------------------------- |
-| `target_org` | string   | yes      | SF CLI org alias or username                                                  |
-| `flags`      | string[] | no       | Additional raw CLI flags (e.g. `["--configuration-file", "config/run.json"]`) |
+| Parameter    | Type     | Required | Description                                                                                                                         |
+| ------------ | -------- | -------- | ----------------------------------------------------------------------------------------------------------------------------------- |
+| `target_org` | string   | yes      | SF CLI org alias or username                                                                                                        |
+| `flags`      | string[] | no       | Additional raw CLI flags (e.g. `["--plan-name", "SmokeTests"]`). Avoid wildcards in plan names — they skip QH plan-level reporting. |



Req: Address all 9 PR review comments and 2 of 3 Codex findings on feature/wave2-nonsf-generation Fix: atomic write ordering, XML parse error surfacing, path validation, validator logic, error message clarity, and doc field name corrections - pageObjectGenerate: preflight both filePath and ssoFilePath before any write (atomic) - connectionTools: throw CONNECTION_XML_PARSE_ERROR on malformed .testproject XML - connectionTools: assertPathAllowed on testProjectPath (defence in depth) - bestPracticesEngine: use rule.check.apiId value for filter instead of hardcoded substring - testCaseGenerate: clarify TESTCASE_INVALID message to reference details.validation - qualityHubTools: narrow wildcard flag description to --plan-name values specifically - docs/mcp.md: fix fields schema (locator_strategy/element_type), steps schema (attributes), QH wildcard note Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Copilot AI review requested due to automatic review settings April 24, 2026 21:34

Copilot started reviewing on behalf of mrdailey99 April 24, 2026 21:34 View session

mrdailey99 force-pushed the feature/wave2-nonsf-generation branch from f990b20 to 2d4240c Compare April 24, 2026 21:35

Copilot AI reviewed Apr 24, 2026

View reviewed changes

mrdailey99 merged commit 13f20d8 into develop Apr 24, 2026
2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(mcp): wave 2 — non-SF generation fixes + provar.connection.list tool#134

feat(mcp): wave 2 — non-SF generation fixes + provar.connection.list tool#134
mrdailey99 merged 2 commits intodevelopfrom
feature/wave2-nonsf-generation

mrdailey99 commented Apr 24, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

mrdailey99 commented Apr 24, 2026

Summary

Tests

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants