From 2d4240cbf352f58128f78776a6f3d368e2d9968e Mon Sep 17 00:00:00 2001 From: Michael Dailey Date: Fri, 24 Apr 2026 16:33:01 -0500 Subject: [PATCH 1/2] PDX-0: feat(mcp): wave 2 non-SF generation fixes and provar.connection.list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- docs/mcp.md | 123 +++++++--- scripts/mcp-smoke.cjs | 8 +- src/mcp/server.ts | 2 + src/mcp/tools/bestPracticesEngine.ts | 269 +++++++++++++++------- src/mcp/tools/connectionTools.ts | 211 +++++++++++++++++ src/mcp/tools/pageObjectGenerate.ts | 118 +++++++--- src/mcp/tools/qualityHubTools.ts | 159 +++++++++++-- src/mcp/tools/testCaseGenerate.ts | 150 +++++++++--- test/unit/mcp/bestPracticesEngine.test.ts | 77 +++++-- test/unit/mcp/connectionTools.test.ts | 215 +++++++++++++++++ test/unit/mcp/pageObjectGenerate.test.ts | 139 ++++++++++- test/unit/mcp/qualityHubTools.test.ts | 158 +++++++++---- test/unit/mcp/testCaseGenerate.test.ts | 97 ++++++++ 13 files changed, 1463 insertions(+), 263 deletions(-) create mode 100644 src/mcp/tools/connectionTools.ts create mode 100644 test/unit/mcp/connectionTools.test.ts diff --git a/docs/mcp.md b/docs/mcp.md index cfa5685..adc5858 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -18,6 +18,7 @@ The Provar DX CLI ships with a built-in **Model Context Protocol (MCP) server** - [Available tools](#available-tools) - [provardx.ping](#provardxping) - [provar.project.inspect](#provarprojectinspect) + - [provar.connection.list](#provarconnectionlist) - [provar.pageobject.generate](#provarpageobjectgenerate) - [provar.pageobject.validate](#provarpageobjectvalidate) - [provar.testcase.generate](#provartestcasegenerate) @@ -529,27 +530,71 @@ Provar test plans live in `plans/`. Each plan is a directory containing a `.plan --- +### `provar.connection.list` + +Lists all connections and named environments defined in the project's `.testproject` file. Use this **before** generating test cases or page objects to discover the exact connection names to use. + +**Prerequisite:** the project must have a `.testproject` file. Run `provar.project.validate` first if unsure of the project root. + +**Security:** only connection names, types, and URLs are returned — credential values from `.secrets` are never included in the output. + +**Input** + +| Parameter | Type | Required | Description | +| -------------- | ------ | -------- | ----------------------------------------------------------------- | +| `project_path` | string | yes | Absolute path to the Provar project root (within `allowed-paths`) | + +**Output** + +```json +{ + "connections": [ + { "name": "MyOrg", "type": "Salesforce", "url": "sfdc://...", "sso_configured": false }, + { "name": "OktaSso", "type": "SSO", "url": "sso://...", "sso_configured": true } + ], + "environments": [ + { "name": "QA", "connection": "MyOrg", "url": "https://qa.example.com" }, + { "name": "UAT", "connection": "AdminOrg" } + ], + "summary": { "connection_count": 2, "environment_count": 2 } +} +``` + +Connection `type` values: `Salesforce`, `Web`, `Quality Hub`, `Web Service`, `Database`, `Google`, `Microsoft`, `Zephyr`, `SSO`, or the raw class name for unknown types. + +**Error codes** + +| Code | Meaning | +| --------------------------- | ------------------------------------------------------------------------- | +| `CONNECTION_FILE_NOT_FOUND` | No `.testproject` at the given path. Run `provar.project.validate` first. | +| `PATH_NOT_ALLOWED` | `project_path` is outside the server's `--allowed-paths` | + +--- + ### `provar.pageobject.generate` -Generates a Java Page Object skeleton with the correct `@Page` or `@SalesforcePage` annotation and `@FindBy` field stubs. +Generates a Java Page Object skeleton with the correct `@Page` or `@SalesforcePage` annotation and `@FindBy` field stubs. Optionally generates an `ILoginPage` implementation stub for non-SF SSO connections. **Input** -| 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 | -| `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 | +| --------------------------- | ------------------------------------------------------ | -------- | ------------------------------------------------------------------------------------------------------- | +| `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 | + +**Output** — `{ java_source: string, file_path?: string, written: boolean, sso_stub_source?: string, sso_stub_file_path?: string, sso_stub_written?: boolean }` -**Output** — `{ content: string, file_path?: string, written: boolean }` +When `sso_class` is provided the response includes `sso_stub_source` (the `ILoginPage` implementation), `sso_stub_file_path` (derived from `output_path`'s directory), and `sso_stub_written`. --- @@ -586,19 +631,35 @@ Validates a Java Page Object source file against 30+ quality rules (structural c Generates an XML test case skeleton with UUID v4 guids and sequential `testItemId` values. +**URI-aware XML structure:** use `target_uri` to pick the correct XML nesting: + +- Omit or use a `sf:` URI → flat Salesforce step structure (existing behaviour) +- `ui:pageobject:target?pageId=pageobjects.Page` → wraps all steps in a `UiWithScreen` element (testItemId=1); substeps clause at testItemId=2; inner steps start at testItemId=3 + **Input** -| 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 | -| `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 | -| `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 | + +**Output** — `{ xml_content: string, file_path?: string, written: boolean, validation?: ValidationResult }` + +`validation` is present when `validate_after_edit=true` (default). If the generated XML fails validation the tool returns `TESTCASE_INVALID` with the `validation` field in `details`. -**Output** — `{ content: string, file_path?: string, written: boolean }` +**Error codes** + +| Code | Meaning | +| ------------------ | --------------------------------------------------------------------- | +| `TESTCASE_INVALID` | Generated XML failed structural validation (see `details.validation`) | +| `FILE_EXISTS` | `output_path` already exists and `overwrite=false` | --- @@ -1034,14 +1095,16 @@ Displays information about the currently connected Quality Hub org. Invokes `sf 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. | -**Output** — `{ requestId, exitCode, stdout, stderr }` +**Output** — `{ requestId, exitCode, stdout, stderr, details?: { warning: string } }` **Error codes:** `QH_TESTRUN_FAILED`, `SF_NOT_FOUND` diff --git a/scripts/mcp-smoke.cjs b/scripts/mcp-smoke.cjs index 5bdeb5d..42ee048 100644 --- a/scripts/mcp-smoke.cjs +++ b/scripts/mcp-smoke.cjs @@ -356,6 +356,10 @@ async function runTests() { arguments: { story: 'Verify Users table has at least one Active record after Salesforce flow runs' }, }); + // ── 49. provar.connection.list ──────────────────────────────────────────── + // TMP has no .testproject → CONNECTION_FILE_NOT_FOUND result (not a protocol error) + await callTool('provar.connection.list', { project_path: TMP }); + server.stdin.end(); } @@ -364,8 +368,8 @@ async function runTests() { // ---------------------------------------------------------------------------- server.on('close', () => { clearTimeout(overallTimer); - // initialize + tools/list + 37 tools + prompts/list + 8 prompts/get (setup excluded from default count) - const TOTAL_EXPECTED = 48 + (INCLUDE_SETUP ? 1 : 0); + // initialize + tools/list + 38 tools + prompts/list + 8 prompts/get (setup excluded from default count) + const TOTAL_EXPECTED = 49 + (INCLUDE_SETUP ? 1 : 0); let passed = 0; let failed = 0; diff --git a/src/mcp/server.ts b/src/mcp/server.ts index 88adf9c..097bc5b 100644 --- a/src/mcp/server.ts +++ b/src/mcp/server.ts @@ -32,6 +32,7 @@ import { registerAllAntTools } from './tools/antTools.js'; import { registerAllRcaTools } from './tools/rcaTools.js'; import { registerAllTestPlanTools } from './tools/testPlanTools.js'; import { registerAllNitroXTools } from './tools/nitroXTools.js'; +import { registerAllConnectionTools } from './tools/connectionTools.js'; import { registerAllPrompts } from './prompts/index.js'; export interface ServerConfig { @@ -80,6 +81,7 @@ export function createProvarMcpServer(config: ServerConfig): McpServer { registerAllRcaTools(server); registerAllTestPlanTools(server, config); registerAllNitroXTools(server, config); + registerAllConnectionTools(server, config); // ── Provar prompts ─────────────────────────────────────────────────────────── registerAllPrompts(server); diff --git a/src/mcp/tools/bestPracticesEngine.ts b/src/mcp/tools/bestPracticesEngine.ts index ecee7fb..0bc70ba 100644 --- a/src/mcp/tools/bestPracticesEngine.ts +++ b/src/mcp/tools/bestPracticesEngine.ts @@ -83,10 +83,7 @@ let rulesConfig: BPRulesConfig | null = null; function getRulesConfig(): BPRulesConfig { if (!rulesConfig) { try { - const raw = readFileSync( - join(dirPath, '..', 'rules', 'provar_best_practices_rules.json'), - 'utf-8' - ); + const raw = readFileSync(join(dirPath, '..', 'rules', 'provar_best_practices_rules.json'), 'utf-8'); rulesConfig = JSON.parse(raw) as BPRulesConfig; } catch { rulesConfig = { schemaVersion: '1.0', rules: [], scoring: { defaultMaxScore: 100 } }; @@ -224,9 +221,7 @@ function getAllApiCalls(node: XmlNode): XmlNode[] { function getDirectSteps(tc: XmlNode): XmlNode[] { const steps = tc['steps'] as XmlNode | undefined; if (!steps || typeof steps !== 'object') return []; - return toArr(steps['apiCall'] as XmlNode | XmlNode[]).filter( - (c) => c && typeof c === 'object' - ); + return toArr(steps['apiCall'] as XmlNode | XmlNode[]).filter((c) => c && typeof c === 'object'); } /** @@ -249,7 +244,7 @@ function getArgValue(call: XmlNode, argId: string): string | undefined { const partsNode = (valElem['parts'] as XmlNode | undefined)?.['value']; const texts = toArr(partsNode as XmlNode | XmlNode[]) .filter((p) => p && typeof p === 'object') - .map((p) => (p)['#text'] as string | undefined) + .map((p) => p['#text'] as string | undefined) .filter(Boolean) as string[]; return texts.join('') || undefined; } @@ -260,9 +255,7 @@ function getArgValue(call: XmlNode, argId: string): string | undefined { /** Return all elements from an apiCall. */ function getArguments(call: XmlNode): XmlNode[] { - return toArr(call['argument'] as XmlNode | XmlNode[]).filter( - (a) => a && typeof a === 'object' - ); + return toArr(call['argument'] as XmlNode | XmlNode[]).filter((a) => a && typeof a === 'object'); } /** Return true if the apiCall has a disabled. */ @@ -276,7 +269,10 @@ function isDisabledCall(call: XmlNode): boolean { // ── Scoring formula (exact Lambda match) ───────────────────────────────────── const SEVERITY_MULT: Record = { - critical: 1.0, major: 0.75, minor: 0.5, info: 0.25, + critical: 1.0, + major: 0.75, + minor: 0.5, + info: 0.25, }; export function calculateBPScore(violations: BPViolation[]): number { @@ -328,9 +324,7 @@ function validateUnknownApiId(tc: XmlNode, rule: BPRule): BPViolation | null { if (!unknowns.length) return null; const MAX = 5; - const msgs = unknowns.slice(0, 3).map( - (v) => `'${v.apiId}'${v.tid ? ` (testItemId=${v.tid})` : ''}` - ); + const msgs = unknowns.slice(0, 3).map((v) => `'${v.apiId}'${v.tid ? ` (testItemId=${v.tid})` : ''}`); let message = `Unknown apiId(s) detected — these APIs do not exist in Provar: ${msgs.join('; ')}`; if (unknowns.length > 3) message += ` (+${unknowns.length - 3} more)`; return makeViolation(rule, message, Math.min(unknowns.length, MAX)); @@ -339,8 +333,8 @@ function validateUnknownApiId(tc: XmlNode, rule: BPRule): BPViolation | null { /** VALID-GUID-001 — test case must have at least one valid identifier. */ function validateValidIdentifier(tc: XmlNode, rule: BPRule): BPViolation | null { const guid = tc['@_guid'] as string | undefined; - const id = tc['@_id'] as string | undefined; - const rid = tc['@_registryId'] as string | undefined; + const id = tc['@_id'] as string | undefined; + const rid = tc['@_registryId'] as string | undefined; if (!guid && !id && !rid) { return makeViolation(rule, 'Test case missing valid identifier (guid, id, or registryId)'); } @@ -386,11 +380,7 @@ function validateWholeNumberTestItemId(tc: XmlNode, rule: BPRule): BPViolation | } if (!invalid.length) return null; - return makeViolation( - rule, - `Invalid testItemId values: ${invalid.slice(0, 3).join(', ')}`, - invalid.length - ); + return makeViolation(rule, `Invalid testItemId values: ${invalid.slice(0, 3).join(', ')}`, invalid.length); } /** VAR-NAMING-001 — variable names must match the identifier pattern. */ @@ -421,7 +411,7 @@ function validateVariableNaming(tc: XmlNode, rule: BPRule): BPViolation | null { if (nvContainer && typeof nvContainer === 'object') { for (const nv of toArr(nvContainer['namedValue'] as XmlNode | XmlNode[])) { if (!nv || typeof nv !== 'object') continue; - const name = (nv)['@_name'] as string | undefined; + const name = nv['@_name'] as string | undefined; if (name && !['valuePath', 'value', 'valueScope'].includes(name)) { checkName(name, 'SetValues variable'); } @@ -440,7 +430,7 @@ function validateVariableNaming(tc: XmlNode, rule: BPRule): BPViolation | null { /** STRUCT-GROUP-001 — direct steps should be wrapped in grouping elements. */ function validateMustAllBeInGroups(tc: XmlNode, rule: BPRule): BPViolation | null { const check = rule.check; - const acceptBdd = (check['acceptBddSteps'] as boolean | undefined) ?? true; + const acceptBdd = (check['acceptBddSteps'] as boolean | undefined) ?? true; const acceptFinally = (check['acceptFinallyBlocks'] as boolean | undefined) ?? true; const GROUPING_IDS = new Set(['com.provar.plugins.bundled.apis.control.StepGroup']); @@ -451,7 +441,8 @@ function validateMustAllBeInGroups(tc: XmlNode, rule: BPRule): BPViolation | nul 'com.provar.plugins.bundled.apis.bdd.Then', 'com.provar.plugins.bundled.apis.bdd.And', 'com.provar.plugins.bundled.apis.bdd.But', - ]) GROUPING_IDS.add(id); + ]) + GROUPING_IDS.add(id); } if (acceptFinally) GROUPING_IDS.add('com.provar.plugins.bundled.apis.control.Finally'); @@ -467,8 +458,9 @@ function validateMustAllBeInGroups(tc: XmlNode, rule: BPRule): BPViolation | nul if (!GROUPING_IDS.has(apiId)) { const title = (step['@_title'] as string | undefined) ?? - (step['@_name'] as string | undefined) ?? - apiId.split('.').pop() ?? ''; + (step['@_name'] as string | undefined) ?? + apiId.split('.').pop() ?? + ''; ungrouped.push(title.substring(0, 30)); } } @@ -477,10 +469,7 @@ function validateMustAllBeInGroups(tc: XmlNode, rule: BPRule): BPViolation | nul if (directSteps.length > 0 && apiCallCount / directSteps.length >= 0.8) return null; if (ungrouped.length > 3) { - return makeViolation( - rule, - `${ungrouped.length} steps not in groups: ${ungrouped.slice(0, 3).join(', ')}...` - ); + return makeViolation(rule, `${ungrouped.length} steps not in groups: ${ungrouped.slice(0, 3).join(', ')}...`); } return null; } @@ -489,36 +478,82 @@ function validateMustAllBeInGroups(tc: XmlNode, rule: BPRule): BPViolation | nul function validateDisabledStep(tc: XmlNode, rule: BPRule): BPViolation | null { for (const call of getAllApiCalls(tc)) { if (!isDisabledCall(call)) continue; - const tid = call['@_testItemId'] as string | undefined; - const apiId = (call['@_apiId'] as string | undefined) ?? 'unknown'; - const name = - (call['@_name'] as string | undefined) ?? + const tid = call['@_testItemId'] as string | undefined; + const apiId = (call['@_apiId'] as string | undefined) ?? 'unknown'; + const name = + (call['@_name'] as string | undefined) ?? (call['@_title'] as string | undefined) ?? - apiId.split('.').pop() ?? 'unnamed'; - return makeViolation( - rule, - `Disabled step found: "${name}"${tid ? ` (testItemId=${tid})` : ''}` - ); + apiId.split('.').pop() ?? + 'unnamed'; + return makeViolation(rule, `Disabled step found: "${name}"${tid ? ` (testItemId=${tid})` : ''}`); } return null; } // Arguments that should NOT be checked for duplicate literals const LITERAL_EXCLUDE_ARGS = new Set([ - 'uiConnectionName', 'apexConnectionName', 'resultScope', 'resultName', - 'sfUiTargetResultScope', 'sfUiTargetResultName', 'objectType', 'connectionId', - 'connectionName', 'comparisonType', 'assertionType', 'httpMethod', 'controlType', - 'loopType', 'navigate', 'windowSelection', 'windowSize', 'alreadyOpenBehaviour', - 'captureBefore', 'captureAfter', 'interactionDescription', 'targetDescription', 'title', + 'uiConnectionName', + 'apexConnectionName', + 'resultScope', + 'resultName', + 'sfUiTargetResultScope', + 'sfUiTargetResultName', + 'objectType', + 'connectionId', + 'connectionName', + 'comparisonType', + 'assertionType', + 'httpMethod', + 'controlType', + 'loopType', + 'navigate', + 'windowSelection', + 'windowSize', + 'alreadyOpenBehaviour', + 'captureBefore', + 'captureAfter', + 'interactionDescription', + 'targetDescription', + 'title', ]); const METADATA_FIELDS = new Set([ - 'status', 'stage', 'recordtype', 'recordtypeid', 'type', 'subtype', 'category', - 'priority', 'source', 'origin', 'reason', 'result', 'state', 'severity', - 'casestatus', 'leadstatus', 'opportunitystage', 'accounttype', 'industry', + 'status', + 'stage', + 'recordtype', + 'recordtypeid', + 'type', + 'subtype', + 'category', + 'priority', + 'source', + 'origin', + 'reason', + 'result', + 'state', + 'severity', + 'casestatus', + 'leadstatus', + 'opportunitystage', + 'accounttype', + 'industry', ]); const DEFAULT_LITERAL_VALUES = new Set([ - '0', '1', '0.0', '1.0', '', 'N/A', 'n/a', 'NA', 'None', 'Default', 'default', - 'Test', 'Global', 'Local', 'Folder', 'GroupStep', + '0', + '1', + '0.0', + '1.0', + '', + 'N/A', + 'n/a', + 'NA', + 'None', + 'Default', + 'default', + 'Test', + 'Global', + 'Local', + 'Folder', + 'GroupStep', ]); /** DDT-VAR-001 — detect hardcoded literal values repeated ≥2 times. */ @@ -563,8 +598,8 @@ function validateDetectDuplicatesLiterals(tc: XmlNode, rule: BPRule): BPViolatio /** NC-FOLDER-001 / NC-PARAM-001 — regex check on target value. */ function validateRegex(tc: XmlNode, rule: BPRule, metadata: BPMetadata): BPViolation | null { - const check = rule.check; - const target = (check['target'] as string | undefined) ?? ''; + const check = rule.check; + const target = (check['target'] as string | undefined) ?? ''; const pattern = (check['pattern'] as string | undefined) ?? ''; if (!pattern) return null; @@ -580,7 +615,10 @@ function validateRegex(tc: XmlNode, rule: BPRule, metadata: BPMetadata): BPViola outer: for (const call of getAllApiCalls(tc)) { for (const arg of getArguments(call)) { const argId = arg['@_id'] as string | undefined; - if (argId) { value = argId; break outer; } + if (argId) { + value = argId; + break outer; + } } } } @@ -599,17 +637,33 @@ function validateRegex(tc: XmlNode, rule: BPRule, metadata: BPMetadata): BPViola // Steps that WRITE to resultName (not just read) const RESULT_WRITE_STEPS = new Set([ - 'ApexConnect', 'UiConnect', 'DbConnect', 'WebConnect', - 'ApexCreateObject', 'ApexUpdateObject', 'ApexReadObject', - 'ApexSoqlQuery', 'ApexSoqlBuilder', - 'RestRequest', 'SoapRequest', - 'SqlQuery', 'DbInsert', 'DbRead', 'DbUpdate', - 'UiWithScreen', 'UiWithRow', + 'ApexConnect', + 'UiConnect', + 'DbConnect', + 'WebConnect', + 'ApexCreateObject', + 'ApexUpdateObject', + 'ApexReadObject', + 'ApexSoqlQuery', + 'ApexSoqlBuilder', + 'RestRequest', + 'SoapRequest', + 'SqlQuery', + 'DbInsert', + 'DbRead', + 'DbUpdate', + 'UiWithScreen', + 'UiWithRow', ]); // Patterns to exclude from result-name tracking const RESULT_EXCLUDE_PATTERNS = [ - 'StepGroup', 'control.If', 'control.While', 'control.ForEach', - 'control.Try', 'control.Finally', 'control.SetValues', + 'StepGroup', + 'control.If', + 'control.While', + 'control.ForEach', + 'control.Try', + 'control.Finally', + 'control.SetValues', ]; /** APEX-RESULTNAME-001 — detect duplicate resultNames within the same scope. */ @@ -640,9 +694,7 @@ function validateUniqueResultNames(tc: XmlNode, rule: BPRule): BPViolation | nul for (const [scope, names] of byScope) { for (const [name, steps] of names) { if (steps.length > 1) { - violations.push( - `'${name}' in ${scope} scope: written ${steps.length} times (${steps.slice(0, 2).join(', ')})` - ); + violations.push(`'${name}' in ${scope} scope: written ${steps.length} times (${steps.slice(0, 2).join(', ')})`); } } } @@ -655,21 +707,87 @@ function validateUniqueResultNames(tc: XmlNode, rule: BPRule): BPViolation | nul return makeViolation(rule, msg, Math.min(violations.length, MAX)); } +// ── UiWithScreen target URI validator ───────────────────────────────────────── + +const VALID_SF_UI_PARAMS = new Set([ + 'object', + 'action', + 'lightningComponent', + 'lightningWebComponent', + 'auraComponent', + 'application', + 'lookup', + 'fieldService', + 'tab', +]); + +/** Read the "target" argument from a UiWithScreen call via the wrapper. */ +function getUiWithScreenTarget(call: XmlNode): string | undefined { + const argsNode = call['arguments'] as XmlNode | undefined; + if (!argsNode || typeof argsNode !== 'object') return undefined; + for (const arg of toArr(argsNode['argument'] as XmlNode | XmlNode[])) { + if (!arg || typeof arg !== 'object') continue; + if (arg['@_id'] !== 'target') continue; + const valElem = arg['value'] as XmlNode | undefined; + if (!valElem || typeof valElem !== 'object') return undefined; + return (valElem['#text'] as string | undefined) ?? undefined; + } + return undefined; +} + +/** UI-SCREEN-TARGET — validate UiWithScreen target URIs (SF and page object formats). */ +function validateUiWithScreenTarget(tc: XmlNode, rule: BPRule): BPViolation | null { + const targetApiId = rule.check['apiId'] as string | undefined; + const violations: string[] = []; + + for (const call of getAllApiCalls(tc)) { + const apiId = call['@_apiId'] as string | undefined; + if (!apiId) continue; + if (targetApiId && !apiId.includes('UiWithScreen')) continue; + + const target = getUiWithScreenTarget(call); + if (!target) continue; + + if (target.startsWith('sf:')) { + const qs = target.includes('?') ? target.split('?')[1] : ''; + const hasValidParam = [...new URLSearchParams(qs).keys()].some((k) => VALID_SF_UI_PARAMS.has(k)); + if (!hasValidParam) { + violations.push(`UiWithScreen target "${target}" has no recognised SF parameters`); + } + } else if (target.startsWith('ui:pageobject:target')) { + if (!target.includes('?')) { + violations.push(`UiWithScreen target "${target}" uses colon format — use ?pageId=pageobjects.`); + } else { + const pageId = new URLSearchParams(target.split('?')[1]).get('pageId'); + if (!pageId?.startsWith('pageobjects.')) { + violations.push(`UiWithScreen target "${target}" pageId must start with "pageobjects."`); + } + } + } + } + + if (!violations.length) return null; + let msg = violations.slice(0, 2).join('; '); + if (violations.length > 2) msg += ` (and ${violations.length - 2} more)`; + return makeViolation(rule, msg, Math.min(violations.length, 3)); +} + // ── Validator dispatch map ──────────────────────────────────────────────────── type ValidatorFn = (tc: XmlNode, rule: BPRule) => BPViolation | null; const VALIDATOR_REGISTRY: Record = { - unknownApiId: validateUnknownApiId, - validIdentifier: validateValidIdentifier, - mustExist: validateMustExist, - mustExistAndNotEmpty: validateMustExistAndNotEmpty, - wholeNumberTestItemId: validateWholeNumberTestItemId, - variableNaming: validateVariableNaming, - mustAllBeInGroups: validateMustAllBeInGroups, - disabledStep: validateDisabledStep, + unknownApiId: validateUnknownApiId, + validIdentifier: validateValidIdentifier, + mustExist: validateMustExist, + mustExistAndNotEmpty: validateMustExistAndNotEmpty, + wholeNumberTestItemId: validateWholeNumberTestItemId, + variableNaming: validateVariableNaming, + mustAllBeInGroups: validateMustAllBeInGroups, + disabledStep: validateDisabledStep, detectDuplicatesLiterals: validateDetectDuplicatesLiterals, - uniqueResultNames: validateUniqueResultNames, + uniqueResultNames: validateUniqueResultNames, + uiWithScreenTarget: validateUiWithScreenTarget, // 'regex' is dispatched separately (needs metadata) }; @@ -690,10 +808,7 @@ const XML_PARSER = new XMLParser({ * @param metadata Optional context (testName used for regex rules). * @returns Quality score (0–100) + list of violations. */ -export function runBestPractices( - xmlContent: string, - metadata: BPMetadata = {} -): BPEngineResult { +export function runBestPractices(xmlContent: string, metadata: BPMetadata = {}): BPEngineResult { let parsed: XmlNode; try { parsed = XML_PARSER.parse(xmlContent) as XmlNode; diff --git a/src/mcp/tools/connectionTools.ts b/src/mcp/tools/connectionTools.ts new file mode 100644 index 0000000..4e09828 --- /dev/null +++ b/src/mcp/tools/connectionTools.ts @@ -0,0 +1,211 @@ +/* + * Copyright (c) 2024 Provar Limited. + * All rights reserved. + * Licensed under the BSD 3-Clause license. + * For full license text, see LICENSE.md file in the repo root or https://opensource.org/licenses/BSD-3-Clause + */ + +/* eslint-disable camelcase */ +import fs from 'node:fs'; +import path from 'node:path'; +import { XMLParser } from 'fast-xml-parser'; +import { z } from 'zod'; +import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; +import type { ServerConfig } from '../server.js'; +import { assertPathAllowed, PathPolicyError } from '../security/pathPolicy.js'; +import { makeError, makeRequestId } from '../schemas/common.js'; +import { log } from '../logging/logger.js'; + +// ── Types ───────────────────────────────────────────────────────────────────── + +interface ConnectionEntry { + name: string; + type: string; + url?: string; + sso_configured: boolean; +} + +interface EnvironmentEntry { + name: string; + connection: string; + url?: string; +} + +// ── Class → human-readable type mapping ────────────────────────────────────── + +const CLASS_TO_TYPE: Record = { + sf: 'Salesforce', + ui: 'Web', + testmanager: 'Quality Hub', + webservice: 'Web Service', + database: 'Database', + google: 'Google', + msexc: 'Microsoft', + zephyr: 'Zephyr', + zephyrScale: 'Zephyr', + zephyrServer: 'Zephyr', + sso: 'SSO', +}; + +function classToType(className: string): string { + return CLASS_TO_TYPE[className] ?? className; +} + +// ── .testproject parsers ────────────────────────────────────────────────────── + +const TP_PARSER = new XMLParser({ + ignoreAttributes: false, + attributeNamePrefix: '@_', + parseAttributeValue: false, + isArray: (name): boolean => name === 'connectionClass' || name === 'connection' || name === 'environment', +}); + +function parseTestProjectXml(content: string): Record { + let parsed: Record; + try { + parsed = TP_PARSER.parse(content) as Record; + } catch { + return {}; + } + const raw = parsed['testProject']; + return raw !== null && typeof raw === 'object' ? (raw as Record) : {}; +} + +function parseConnectionList(content: string): ConnectionEntry[] { + const tp = parseTestProjectXml(content); + const cc = tp['connectionClasses']; + if (!cc || typeof cc !== 'object') return []; + + const classesRaw = (cc as Record)['connectionClass']; + if (!classesRaw) return []; + const classes = classesRaw as Array>; + + const connections: ConnectionEntry[] = []; + for (const cls of classes) { + const className = cls['@_name'] as string | undefined; + if (!className) continue; + const connsRaw = cls['connection']; + if (!connsRaw) continue; + for (const conn of connsRaw as Array>) { + const connName = conn['@_name'] as string | undefined; + if (!connName) continue; + const url = conn['@_url'] as string | undefined; + connections.push({ + name: connName, + type: classToType(className), + ...(url ? { url } : {}), + sso_configured: className === 'sso', + }); + } + } + return connections; +} + +function parseEnvironmentList(content: string): EnvironmentEntry[] { + const tp = parseTestProjectXml(content); + const envSection = tp['environments']; + if (!envSection || typeof envSection !== 'object') return []; + + const envsRaw = (envSection as Record)['environment']; + if (!envsRaw) return []; + + const environments: EnvironmentEntry[] = []; + for (const env of envsRaw as Array>) { + const name = env['@_name'] as string | undefined; + if (!name) continue; + const connection = env['@_connectionName'] as string | undefined; + const url = env['@_url'] as string | undefined; + environments.push({ + name, + connection: connection ?? '', + ...(url ? { url } : {}), + }); + } + return environments; +} + +// ── Tool registration ───────────────────────────────────────────────────────── + +export function registerConnectionList(server: McpServer, config: ServerConfig): void { + server.tool( + 'provar.connection.list', + [ + 'List all connections and named environments defined in the .testproject file.', + 'Use this before generating test cases or page objects to get the correct connection names.', + 'Returns connections (name, type, url, sso_configured) and environments (name, connection, url).', + 'Prerequisite: the project must have a .testproject file — run provar.project.validate first if unsure.', + 'Security: only connection names, types, and URLs are returned — credential values from .secrets are never included.', + ].join(' '), + { + project_path: z + .string() + .describe('Absolute or relative path to the Provar project root directory (must be within --allowed-paths)'), + }, + ({ project_path }) => { + const requestId = makeRequestId(); + log('info', 'provar.connection.list', { requestId, project_path }); + + 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'); + } catch (readErr) { + const err = makeError( + 'CONNECTION_FILE_READ_ERROR', + `Failed to read .testproject: ${(readErr as Error).message}`, + requestId, + false + ); + return { isError: true, content: [{ type: 'text' as const, text: JSON.stringify(err) }] }; + } + + const connections = parseConnectionList(content); + const environments = parseEnvironmentList(content); + + const result = { + requestId, + project_path: resolvedPath, + connections, + environments, + summary: { + connection_count: connections.length, + environment_count: environments.length, + }, + }; + return { + content: [{ type: 'text' as const, text: JSON.stringify(result) }], + structuredContent: result, + }; + } catch (err: unknown) { + const error = err as Error & { code?: string }; + const errResult = makeError( + error instanceof PathPolicyError ? error.code : error.code ?? 'CONNECTION_LIST_ERROR', + error.message, + requestId, + false + ); + log('error', 'provar.connection.list failed', { requestId, error: error.message }); + return { isError: true, content: [{ type: 'text' as const, text: JSON.stringify(errResult) }] }; + } + } + ); +} + +export function registerAllConnectionTools(server: McpServer, config: ServerConfig): void { + registerConnectionList(server, config); +} diff --git a/src/mcp/tools/pageObjectGenerate.ts b/src/mcp/tools/pageObjectGenerate.ts index e40f295..43e4023 100644 --- a/src/mcp/tools/pageObjectGenerate.ts +++ b/src/mcp/tools/pageObjectGenerate.ts @@ -16,13 +16,28 @@ import { makeError, makeRequestId } from '../schemas/common.js'; import { log } from '../logging/logger.js'; const VALID_LOCATOR_STRATEGIES = [ - 'xpath', 'id', 'css', 'name', 'className', 'tagName', - 'linkText', 'partialLinkText', 'visualforce', 'label', + 'xpath', + 'id', + 'css', + 'name', + 'className', + 'tagName', + 'linkText', + 'partialLinkText', + 'visualforce', + 'label', ] as const; const VALID_ELEMENT_TYPES = [ - 'TextType', 'ButtonType', 'LinkType', 'ChoiceListType', - 'RadioType', 'FileType', 'DateType', 'RichTextType', 'BooleanType', + 'TextType', + 'ButtonType', + 'LinkType', + 'ChoiceListType', + 'RadioType', + 'FileType', + 'DateType', + 'RichTextType', + 'BooleanType', ] as const; const FieldSchema = z.object({ @@ -38,7 +53,13 @@ const FieldSchema = z.object({ export function registerPageObjectGenerate(server: McpServer, config: ServerConfig): void { server.tool( 'provar.pageobject.generate', - 'Generate a Provar Java Page Object skeleton with @Page/@SalesforcePage annotation, standard imports, and @FindBy WebElement fields. Returns Java source. Writes to disk only when dry_run=false.', + [ + 'Generate a Provar Java Page Object skeleton with @Page/@SalesforcePage annotation, standard imports, and @FindBy WebElement fields.', + 'Returns Java source. Writes to disk only when dry_run=false.', + 'SSO support: set sso_class to also generate an ILoginPage implementation stub for non-SF SSO pages.', + 'Example: sso_class="LoginPageSso" generates a LoginPageSso.java that implements ILoginPage with loginAs() and logout() stubs.', + 'The ILoginPage stub is written to the same directory as output_path when dry_run=false.', + ].join(' '), { class_name: z.string().describe('PascalCase class name, e.g. AccountDetailPage'), package_name: z @@ -49,10 +70,7 @@ export function registerPageObjectGenerate(server: McpServer, config: ServerConf .enum(['standard', 'salesforce']) .default('standard') .describe('@Page (standard) or @SalesforcePage (salesforce)'), - title: z - .string() - .optional() - .describe('Page title attribute; defaults to class_name if omitted'), + title: z.string().optional().describe('Page title attribute; defaults to class_name if omitted'), connection_name: z .string() .optional() @@ -62,22 +80,18 @@ export function registerPageObjectGenerate(server: McpServer, config: ServerConf .optional() .describe('Page type attribute for @SalesforcePage'), fields: z.array(FieldSchema).default([]).describe('WebElement fields to generate'), - output_path: z + sso_class: z .string() .optional() - .describe('Suggested file path for the .java file (returned in response)'), - overwrite: z - .boolean() - .default(false) - .describe('Overwrite existing file when dry_run=false'), - dry_run: z - .boolean() - .default(true) - .describe('true = return source only (default); false = write to output_path'), - idempotency_key: z - .string() - .optional() - .describe('Caller-provided key echoed back for deduplication tracking'), + .describe( + 'PascalCase class name for an ILoginPage implementation stub (non-SF SSO pages). ' + + 'When provided, an additional Java class implementing ILoginPage is generated alongside the page object. ' + + 'Example: "LoginPageSso" → LoginPageSso.java with loginAs() and logout() method stubs.' + ), + output_path: z.string().optional().describe('Suggested file path for the .java file (returned in response)'), + overwrite: z.boolean().default(false).describe('Overwrite existing file when dry_run=false'), + dry_run: z.boolean().default(true).describe('true = return source only (default); false = write to output_path'), + idempotency_key: z.string().optional().describe('Caller-provided key echoed back for deduplication tracking'), }, (input) => { const requestId = makeRequestId(); @@ -85,17 +99,21 @@ export function registerPageObjectGenerate(server: McpServer, config: ServerConf requestId, class_name: input.class_name, dry_run: input.dry_run, + sso_class: input.sso_class, }); try { const javaSource = buildPageObjectSource(input); - const filePath: string | undefined = input.output_path - ? path.resolve(input.output_path) - : undefined; + const filePath: string | undefined = input.output_path ? path.resolve(input.output_path) : undefined; let written = false; + const ssoSource = input.sso_class ? buildSsoLoginPageSource(input.sso_class, input.package_name) : undefined; + const ssoFilePath: string | undefined = + ssoSource && filePath ? path.join(path.dirname(filePath), `${input.sso_class}.java`) : undefined; + if (filePath && !input.dry_run) { assertPathAllowed(filePath, config.allowedPaths); + if (ssoFilePath) assertPathAllowed(ssoFilePath, config.allowedPaths); if (fs.existsSync(filePath) && !input.overwrite) { const err = makeError( @@ -110,9 +128,22 @@ 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) }] }; + } + fs.writeFileSync(ssoFilePath, ssoSource, 'utf-8'); + log('info', 'provar.pageobject.generate: wrote SSO stub', { requestId, ssoFilePath }); + } } - const result = { + const result: Record = { requestId, java_source: javaSource, file_path: filePath, @@ -120,6 +151,11 @@ export function registerPageObjectGenerate(server: McpServer, config: ServerConf dry_run: input.dry_run, idempotency_key: input.idempotency_key, }; + if (ssoSource) { + result['sso_stub_source'] = ssoSource; + result['sso_stub_file_path'] = ssoFilePath; + result['sso_stub_written'] = written && !!ssoFilePath; + } return { content: [{ type: 'text' as const, text: JSON.stringify(result) }], structuredContent: result, @@ -127,7 +163,7 @@ export function registerPageObjectGenerate(server: McpServer, config: ServerConf } catch (err: unknown) { const error = err as Error & { code?: string }; const errResult = makeError( - error instanceof PathPolicyError ? error.code : (error.code ?? 'GENERATE_ERROR'), + error instanceof PathPolicyError ? error.code : error.code ?? 'GENERATE_ERROR', error.message, requestId, false @@ -139,7 +175,7 @@ export function registerPageObjectGenerate(server: McpServer, config: ServerConf ); } -// ── Source builder ──────────────────────────────────────────────────────────── +// ── Source builders ─────────────────────────────────────────────────────────── function buildPageObjectSource(input: { class_name: string; @@ -150,8 +186,7 @@ function buildPageObjectSource(input: { salesforce_page_attribute?: string; fields: Array>; }): string { - const { class_name, package_name, page_type, title, connection_name, salesforce_page_attribute, fields } = - input; + const { class_name, package_name, page_type, title, connection_name, salesforce_page_attribute, fields } = input; const pageTitle = title ?? class_name; let annotation: string; @@ -190,3 +225,24 @@ ${fieldBlocks} } `; } + +function buildSsoLoginPageSource(ssoClass: string, packageName: string): string { + return `package ${packageName}; + +import com.provar.core.testapi.annotations.sso.ILoginPage; + +public class ${ssoClass} implements ILoginPage { + + @Override + public void loginAs(String username, String password) { + // TODO: implement SSO login + } + + @Override + public void logout() { + // TODO: implement SSO logout + } + +} +`; +} diff --git a/src/mcp/tools/qualityHubTools.ts b/src/mcp/tools/qualityHubTools.ts index 223dc7d..8afc7a8 100644 --- a/src/mcp/tools/qualityHubTools.ts +++ b/src/mcp/tools/qualityHubTools.ts @@ -12,13 +12,21 @@ import { makeError, makeRequestId } from '../schemas/common.js'; import { log } from '../logging/logger.js'; import { runSfCommand } from './sfSpawn.js'; - -function handleSpawnError(err: unknown, requestId: string, toolName: string): { isError: true; content: Array<{ type: 'text'; text: string }> } { +function handleSpawnError( + err: unknown, + requestId: string, + toolName: string +): { isError: true; content: Array<{ type: 'text'; text: string }> } { const error = err as Error & { code?: string }; log('error', `${toolName} failed`, { requestId, error: error.message }); return { isError: true as const, - content: [{ type: 'text' as const, text: JSON.stringify(makeError(error.code ?? 'SF_ERROR', error.message, requestId, false)) }], + content: [ + { + type: 'text' as const, + text: JSON.stringify(makeError(error.code ?? 'SF_ERROR', error.message, requestId, false)), + }, + ], }; } @@ -30,7 +38,11 @@ export function registerQualityHubConnect(server: McpServer): void { 'Connect to a Provar Quality Hub org. Invokes `sf provar quality-hub connect` with the supplied flags.', { target_org: z.string().describe('SF org alias or username to connect as'), - flags: z.array(z.string()).optional().default([]).describe('Additional raw CLI flags to forward (e.g. ["--json"])'), + flags: z + .array(z.string()) + .optional() + .default([]) + .describe('Additional raw CLI flags to forward (e.g. ["--json"])'), }, ({ target_org, flags }) => { const requestId = makeRequestId(); @@ -41,7 +53,15 @@ export function registerQualityHubConnect(server: McpServer): void { const response = { requestId, exitCode: result.exitCode, stdout: result.stdout, stderr: result.stderr }; if (result.exitCode !== 0) { - return { isError: true as const, content: [{ type: 'text' as const, text: JSON.stringify(makeError('QH_CONNECT_FAILED', result.stderr || result.stdout, requestId)) }] }; + return { + isError: true as const, + content: [ + { + type: 'text' as const, + text: JSON.stringify(makeError('QH_CONNECT_FAILED', result.stderr || result.stdout, requestId)), + }, + ], + }; } return { content: [{ type: 'text' as const, text: JSON.stringify(response) }], structuredContent: response }; @@ -74,7 +94,15 @@ export function registerQualityHubDisplay(server: McpServer): void { const response = { requestId, exitCode: result.exitCode, stdout: result.stdout, stderr: result.stderr }; if (result.exitCode !== 0) { - return { isError: true as const, content: [{ type: 'text' as const, text: JSON.stringify(makeError('QH_DISPLAY_FAILED', result.stderr || result.stdout, requestId)) }] }; + return { + isError: true as const, + content: [ + { + type: 'text' as const, + text: JSON.stringify(makeError('QH_DISPLAY_FAILED', result.stderr || result.stdout, requestId)), + }, + ], + }; } return { content: [{ type: 'text' as const, text: JSON.stringify(response) }], structuredContent: response }; @@ -87,24 +115,65 @@ export function registerQualityHubDisplay(server: McpServer): void { // ── Tool: provar.qualityhub.testrun ────────────────────────────────────────── +function detectWildcardFlags(flags: string[]): string | undefined { + for (let i = 0; i < flags.length - 1; i++) { + if (flags[i] === '--plan-name') { + const value = flags[i + 1]; + if (value.includes('*') || value.includes('?')) { + return ( + `Wildcard testPlan scope detected ("${value}"). ` + + 'QH plan-level reporting will be skipped. ' + + 'Use exact plan names for QH plan reporting.' + ); + } + } + } + return undefined; +} + export function registerQualityHubTestRun(server: McpServer): void { server.tool( '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.' + ), }, ({ target_org, flags }) => { const requestId = makeRequestId(); log('info', 'provar.qualityhub.testrun', { requestId, target_org }); try { + const wildcardWarning = detectWildcardFlags(flags); const result = runSfCommand(['provar', 'quality-hub', 'test', 'run', '--target-org', target_org, ...flags]); - const response = { requestId, exitCode: result.exitCode, stdout: result.stdout, stderr: result.stderr }; + const response: Record = { + requestId, + exitCode: result.exitCode, + stdout: result.stdout, + stderr: result.stderr, + }; + + if (wildcardWarning) { + response['details'] = { warning: wildcardWarning }; + } if (result.exitCode !== 0) { - return { isError: true as const, content: [{ type: 'text' as const, text: JSON.stringify(makeError('QH_TESTRUN_FAILED', result.stderr || result.stdout, requestId)) }] }; + return { + isError: true as const, + content: [ + { + type: 'text' as const, + text: JSON.stringify(makeError('QH_TESTRUN_FAILED', result.stderr || result.stdout, requestId)), + }, + ], + }; } return { content: [{ type: 'text' as const, text: JSON.stringify(response) }], structuredContent: response }; @@ -132,15 +201,29 @@ export function registerQualityHubTestRunReport(server: McpServer): void { try { const result = runSfCommand([ - 'provar', 'quality-hub', 'test', 'run', 'report', - '--target-org', target_org, - '--run-id', run_id, + 'provar', + 'quality-hub', + 'test', + 'run', + 'report', + '--target-org', + target_org, + '--run-id', + run_id, ...flags, ]); const response = { requestId, exitCode: result.exitCode, stdout: result.stdout, stderr: result.stderr }; if (result.exitCode !== 0) { - return { isError: true as const, content: [{ type: 'text' as const, text: JSON.stringify(makeError('QH_REPORT_FAILED', result.stderr || result.stdout, requestId)) }] }; + return { + isError: true as const, + content: [ + { + type: 'text' as const, + text: JSON.stringify(makeError('QH_REPORT_FAILED', result.stderr || result.stdout, requestId)), + }, + ], + }; } const failureStatuses = new Set(['FAIL', 'FAILED']); @@ -187,15 +270,29 @@ export function registerQualityHubTestRunAbort(server: McpServer): void { try { const result = runSfCommand([ - 'provar', 'quality-hub', 'test', 'run', 'abort', - '--target-org', target_org, - '--run-id', run_id, + 'provar', + 'quality-hub', + 'test', + 'run', + 'abort', + '--target-org', + target_org, + '--run-id', + run_id, ...flags, ]); const response = { requestId, exitCode: result.exitCode, stdout: result.stdout, stderr: result.stderr }; if (result.exitCode !== 0) { - return { isError: true as const, content: [{ type: 'text' as const, text: JSON.stringify(makeError('QH_ABORT_FAILED', result.stderr || result.stdout, requestId)) }] }; + return { + isError: true as const, + content: [ + { + type: 'text' as const, + text: JSON.stringify(makeError('QH_ABORT_FAILED', result.stderr || result.stdout, requestId)), + }, + ], + }; } return { content: [{ type: 'text' as const, text: JSON.stringify(response) }], structuredContent: response }; @@ -214,18 +311,38 @@ export function registerQualityHubTestcaseRetrieve(server: McpServer): void { 'Retrieve Quality Hub test cases by user story or component. Invokes `sf provar quality-hub testcase retrieve`.', { target_org: z.string().describe('SF org alias or username'), - flags: z.array(z.string()).optional().default([]).describe('Additional raw CLI flags (e.g. ["--user-story", "US-123"])'), + flags: z + .array(z.string()) + .optional() + .default([]) + .describe('Additional raw CLI flags (e.g. ["--user-story", "US-123"])'), }, ({ target_org, flags }) => { const requestId = makeRequestId(); log('info', 'provar.qualityhub.testcase.retrieve', { requestId, target_org }); try { - const result = runSfCommand(['provar', 'quality-hub', 'testcase', 'retrieve', '--target-org', target_org, ...flags]); + const result = runSfCommand([ + 'provar', + 'quality-hub', + 'testcase', + 'retrieve', + '--target-org', + target_org, + ...flags, + ]); const response = { requestId, exitCode: result.exitCode, stdout: result.stdout, stderr: result.stderr }; if (result.exitCode !== 0) { - return { isError: true as const, content: [{ type: 'text' as const, text: JSON.stringify(makeError('QH_RETRIEVE_FAILED', result.stderr || result.stdout, requestId)) }] }; + return { + isError: true as const, + content: [ + { + type: 'text' as const, + text: JSON.stringify(makeError('QH_RETRIEVE_FAILED', result.stderr || result.stdout, requestId)), + }, + ], + }; } return { content: [{ type: 'text' as const, text: JSON.stringify(response) }], structuredContent: response }; diff --git a/src/mcp/tools/testCaseGenerate.ts b/src/mcp/tools/testCaseGenerate.ts index d4ec39f..e116f99 100644 --- a/src/mcp/tools/testCaseGenerate.ts +++ b/src/mcp/tools/testCaseGenerate.ts @@ -99,6 +99,9 @@ const StepSchema = z.object({ const TOOL_DESCRIPTION = [ 'Generate a Provar XML test case skeleton with proper UUID v4 guids, sequential testItemId values, and structure.', 'Returns XML content. Writes to disk only when dry_run=false.', + 'URI-aware generation: use target_uri to control the XML nesting structure.', + ' - sf:ui:target (or omit target_uri) → flat Salesforce XML structure (existing behaviour).', + ' - ui:pageobject:target?pageId=pageobjects.PageClass → wraps all steps in a UiWithScreen element targeting that non-SF page object.', 'API IDs: shorthand forms (e.g. UiConnect, ApexSoqlQuery) are automatically expanded to fully-qualified IDs required by the Provar runtime.', 'Step arguments: attributes are emitted as — the only format the Provar runtime processes.', 'Shorthand XML attributes on are silently ignored at runtime; always supply arguments via the attributes map.', @@ -108,7 +111,7 @@ const TOOL_DESCRIPTION = [ 'ApexReadObject requires field names in attributes; omitting them produces MALFORMED_QUERY. Prefer ApexSoqlQuery.', 'AssertValues on SOQL results: index paths like "ResultList[0].Field" are not supported.', 'Use ForEach to iterate the result list, or SetValues to extract a field into a variable first.', - 'Validation: the response always includes a validation field with is_valid, validity_score, quality_score, and any structural issues — check this before attempting to run the test case.', + 'Validation: when validate_after_edit=true (default) the response includes a validation field and returns TESTCASE_INVALID if the generated XML fails structural checks.', 'Grounding: call provar.qualityhub.examples.retrieve before generating to get corpus examples for the scenario — correct XML structure for the step types you need.', ].join(' '); @@ -120,9 +123,26 @@ export function registerTestCaseGenerate(server: McpServer, config: ServerConfig test_case_name: z.string().describe('Test case name (human-readable label)'), test_case_id: z.string().optional().describe('Explicit test case id; auto-generated UUID v4 if omitted'), steps: z.array(StepSchema).default([]).describe('Ordered list of test steps'), + target_uri: z + .string() + .optional() + .describe( + 'Page object URI that determines the XML nesting structure. ' + + 'Omit or use "sf:ui:target" for Salesforce targets (flat structure). ' + + 'Use "ui:pageobject:target?pageId=pageobjects.PageClass" for non-SF page objects — ' + + 'steps are wrapped in a UiWithScreen element targeting that class.' + ), output_path: z.string().optional().describe('Suggested file path for the .xml file (returned in response)'), overwrite: z.boolean().default(false).describe('Overwrite if output_path file already exists'), dry_run: z.boolean().default(true).describe('true = return XML only (default); false = write to output_path'), + validate_after_edit: z + .boolean() + .default(true) + .describe( + 'Run structural validation after generation (default: true). ' + + 'Returns TESTCASE_INVALID error if the generated XML fails validation. ' + + 'Set false to skip validation and omit the validation field from the response.' + ), idempotency_key: z.string().optional().describe('Caller-provided key echoed back for deduplication tracking'), }, (input) => { @@ -131,6 +151,7 @@ export function registerTestCaseGenerate(server: McpServer, config: ServerConfig requestId, test_case_name: input.test_case_name, dry_run: input.dry_run, + target_uri: input.target_uri, }); try { @@ -157,16 +178,8 @@ export function registerTestCaseGenerate(server: McpServer, config: ServerConfig } const warnings = buildStepWarnings(input.steps); - const validationFull = validateTestCase(xmlContent, input.test_case_name); - const validationSlim = { - is_valid: validationFull.is_valid, - validity_score: validationFull.validity_score, - quality_score: validationFull.quality_score, - error_count: validationFull.error_count, - warning_count: validationFull.warning_count, - issues: validationFull.issues, - }; - const result = { + const runValidation = input.validate_after_edit !== false; + const baseResult = { requestId, xml_content: xmlContent, file_path: filePath, @@ -174,12 +187,40 @@ export function registerTestCaseGenerate(server: McpServer, config: ServerConfig dry_run: input.dry_run, step_count: input.steps.length, idempotency_key: input.idempotency_key, - validation: validationSlim, ...(warnings.length > 0 ? { warnings } : {}), }; + + if (runValidation) { + const validationFull = validateTestCase(xmlContent, input.test_case_name); + const validationSlim = { + is_valid: validationFull.is_valid, + validity_score: validationFull.validity_score, + quality_score: validationFull.quality_score, + error_count: validationFull.error_count, + warning_count: validationFull.warning_count, + issues: validationFull.issues, + }; + if (!validationFull.is_valid) { + 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) }] }; + } + const result = { ...baseResult, validation: validationSlim }; + return { + content: [{ type: 'text' as const, text: JSON.stringify(result) }], + structuredContent: result, + }; + } + return { - content: [{ type: 'text' as const, text: JSON.stringify(result) }], - structuredContent: result, + content: [{ type: 'text' as const, text: JSON.stringify(baseResult) }], + structuredContent: baseResult, }; } catch (err: unknown) { const error = err as Error & { code?: string }; @@ -198,59 +239,92 @@ export function registerTestCaseGenerate(server: McpServer, config: ServerConfig // ── XML builder ─────────────────────────────────────────────────────────────── -function buildArgumentsXml(attributes: Record): string { +function buildArgumentsXml(attributes: Record, baseIndent = ' '): string { const entries = Object.entries(attributes); if (entries.length === 0) return ''; const argLines = entries .map( ([k, v]) => - ` \n` + - ` ${escapeXmlContent(v)}\n` + - ' ' + `${baseIndent}\n` + + `${baseIndent} ${escapeXmlContent(v)}\n` + + `${baseIndent}` ) .join('\n'); - return `\n \n${argLines}\n \n `; + return `\n${baseIndent}\n${argLines}\n${baseIndent}\n${baseIndent.slice(0, -2)}`; +} + +function buildFlatStepXml( + step: { api_id: string; name: string; attributes: Record }, + testItemId: number, + indent: string +): string { + const guid = randomUUID(); + const resolvedApiId = resolveApiId(step.api_id); + const argumentsXml = buildArgumentsXml(step.attributes, indent + ' '); + if (argumentsXml) { + return ( + `${indent}${argumentsXml}` + ); + } + return ( + `${indent}` + ); } function buildTestCaseXml(input: { test_case_name: string; test_case_id?: string; steps: Array<{ api_id: string; name: string; attributes: Record }>; + target_uri?: string; }): string { const testCaseId = input.test_case_id ?? randomUUID(); const testCaseGuid = randomUUID(); const registryId = randomUUID(); - const stepLines = input.steps - .map((step, i) => { - const guid = randomUUID(); - const testItemId = i + 1; - const resolvedApiId = resolveApiId(step.api_id); - const argumentsXml = buildArgumentsXml(step.attributes); - if (argumentsXml) { - return ( - ` ${argumentsXml}` - ); - } - return ( - ` ` - ); - }) - .join('\n'); + let stepLines: string; + const isNonSf = !!input.target_uri && input.target_uri.startsWith('ui:'); + + if (isNonSf && input.target_uri) { + stepLines = buildUiWithScreenXml(input.steps, input.target_uri); + } else { + const lines = input.steps.map((step, i) => buildFlatStepXml(step, i + 1, ' ')).join('\n'); + stepLines = lines || ' '; + } return ( '\n' + `\n` + ' \n' + - (stepLines || ' ') + + stepLines + '\n \n' + '\n' ); } +function buildUiWithScreenXml( + steps: Array<{ api_id: string; name: string; attributes: Record }>, + targetUri: string +): string { + const wrapperGuid = randomUUID(); + const wrapperApiId = resolveApiId('UiWithScreen'); + // Inner steps use testItemIds starting at 3; the substeps clause itself occupies testItemId=2 + const innerLines = steps.map((step, i) => buildFlatStepXml(step, i + 3, ' ')).join('\n'); + const stepsContent = innerLines ? `\n${innerLines}\n ` : ''; + const clausesXml = + '\n \n' + + ' \n' + + ` ${stepsContent}\n` + + ' \n' + + ' \n '; + return ( + ` ${buildArgumentsXml({ target: targetUri }).trimEnd()}${clausesXml}` + ); +} + function escapeXmlAttr(value: string): string { return value.replace(/&/g, '&').replace(/"/g, '"').replace(//g, '>'); } diff --git a/test/unit/mcp/bestPracticesEngine.test.ts b/test/unit/mcp/bestPracticesEngine.test.ts index c4fcbb3..3e7ba1b 100644 --- a/test/unit/mcp/bestPracticesEngine.test.ts +++ b/test/unit/mcp/bestPracticesEngine.test.ts @@ -8,19 +8,11 @@ /* eslint-disable camelcase */ import { strict as assert } from 'node:assert'; import { describe, it } from 'mocha'; -import { - calculateBPScore, - runBestPractices, - type BPViolation, -} from '../../../src/mcp/tools/bestPracticesEngine.js'; +import { calculateBPScore, runBestPractices, type BPViolation } from '../../../src/mcp/tools/bestPracticesEngine.js'; // ── Helper: build a minimal violation ──────────────────────────────────────── -function makeViolation( - severity: BPViolation['severity'], - weight: number, - count?: number -): BPViolation { +function makeViolation(severity: BPViolation['severity'], weight: number, count?: number): BPViolation { return { rule_id: 'TEST-001', name: 'Test Violation', @@ -37,9 +29,9 @@ function makeViolation( // ── Valid XML fixture (passes all schema-level rules) ───────────────────────── -const GUID_TC = '550e8400-e29b-41d4-a716-446655440000'; -const GUID_S1 = '550e8400-e29b-41d4-a716-446655440011'; -const GUID_S2 = '550e8400-e29b-41d4-a716-446655440012'; +const GUID_TC = '550e8400-e29b-41d4-a716-446655440000'; +const GUID_S1 = '550e8400-e29b-41d4-a716-446655440011'; +const GUID_S2 = '550e8400-e29b-41d4-a716-446655440012'; const VALID_XML = ` @@ -97,10 +89,7 @@ describe('calculateBPScore', () => { // critical weight=5: 100 - 5 = 95 // major weight=4: 95 - 3 = 92 // net: 92 - const score = calculateBPScore([ - makeViolation('critical', 5), - makeViolation('major', 4), - ]); + const score = calculateBPScore([makeViolation('critical', 5), makeViolation('major', 4)]); assert.equal(score, 92); }); @@ -122,7 +111,7 @@ describe('runBestPractices', () => { describe('valid XML test case', () => { it('returns a quality_score between 0 and 100', () => { const result = runBestPractices(VALID_XML); - assert.ok(result.quality_score >= 0, `score ${result.quality_score} should be >= 0`); + assert.ok(result.quality_score >= 0, `score ${result.quality_score} should be >= 0`); assert.ok(result.quality_score <= 100, `score ${result.quality_score} should be <= 100`); }); @@ -166,4 +155,56 @@ describe('runBestPractices', () => { assert.equal(result.quality_score, recalculated); }); }); + + describe('uiWithScreenTarget validator', () => { + const GUID_UWS = '550e8400-e29b-41d4-a716-446655440020'; + const GUID_TC2 = '550e8400-e29b-41d4-a716-446655440030'; + + function buildUwsXml(target: string): string { + return ` + + + + + + ${target} + + + + +`; + } + + 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') + ); + 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') + ); + assert.ok(!uwsViolation, `Expected no uiWithScreenTarget violation, got: ${uwsViolation?.message}`); + }); + + it('fires for page object target in colon format (ui:pageobject:target:com.example.Class)', () => { + const result = runBestPractices(buildUwsXml('ui:pageobject:target:com.example.LoginPage')); + const uwsViolation = result.violations.find( + (v) => v.message.includes('colon format') || v.message.includes('UiWithScreen') + ); + assert.ok(uwsViolation, 'Expected uiWithScreenTarget violation for colon format URI'); + }); + + it('fires for page object target with pageId missing pageobjects. prefix', () => { + const result = runBestPractices(buildUwsXml('ui:pageobject:target?pageId=LoginPage')); + const uwsViolation = result.violations.find( + (v) => v.message.includes('pageobjects.') || v.message.includes('UiWithScreen') + ); + assert.ok(uwsViolation, 'Expected uiWithScreenTarget violation for missing pageobjects. prefix'); + }); + }); }); diff --git a/test/unit/mcp/connectionTools.test.ts b/test/unit/mcp/connectionTools.test.ts new file mode 100644 index 0000000..c75e84b --- /dev/null +++ b/test/unit/mcp/connectionTools.test.ts @@ -0,0 +1,215 @@ +/* + * Copyright (c) 2024 Provar Limited. + * All rights reserved. + * Licensed under the BSD 3-Clause license. + * For full license text, see LICENSE.md file in the repo root or https://opensource.org/licenses/BSD-3-Clause + */ + +/* eslint-disable camelcase */ +import { strict as assert } from 'node:assert'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { describe, it, beforeEach, afterEach } from 'mocha'; +import { registerConnectionList } from '../../../src/mcp/tools/connectionTools.js'; +import type { ServerConfig } from '../../../src/mcp/server.js'; + +// ── Minimal McpServer mock ──────────────────────────────────────────────────── + +type ToolHandler = (args: Record) => unknown; + +class MockMcpServer { + private handlers = new Map(); + + public tool(name: string, _description: string, _schema: unknown, handler: ToolHandler): void { + this.handlers.set(name, handler); + } + + public call(name: string, args: Record): ReturnType { + const h = this.handlers.get(name); + if (!h) throw new Error(`Tool not registered: ${name}`); + return h(args); + } +} + +// ── Helpers ─────────────────────────────────────────────────────────────────── + +function parseText(result: unknown): Record { + const r = result as { content: Array<{ type: string; text: string }> }; + return JSON.parse(r.content[0].text) as Record; +} + +function isError(result: unknown): boolean { + return (result as { isError?: boolean }).isError === true; +} + +function writeTestProject(dir: string, content: string): void { + fs.writeFileSync(path.join(dir, '.testproject'), content, 'utf-8'); +} + +// ── .testproject fixture content ────────────────────────────────────────────── + +const BASIC_TEST_PROJECT = ` + + + + + + + + + + + + + + + + + + + + + + +`; + +const EMPTY_TEST_PROJECT = ` + + +`; + +// ── Test setup ──────────────────────────────────────────────────────────────── + +let tmpDir: string; +let server: MockMcpServer; +let config: ServerConfig; + +beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'conntools-test-')); + server = new MockMcpServer(); + config = { allowedPaths: [tmpDir] }; + registerConnectionList(server as never, config); +}); + +afterEach(() => { + fs.rmSync(tmpDir, { recursive: true, force: true }); +}); + +// ── provar.connection.list ──────────────────────────────────────────────────── + +describe('provar.connection.list', () => { + describe('happy path', () => { + it('returns connections array with name, type, and url', () => { + writeTestProject(tmpDir, BASIC_TEST_PROJECT); + const result = server.call('provar.connection.list', { project_path: tmpDir }); + + assert.equal(isError(result), false); + const body = parseText(result); + const connections = body['connections'] as Array>; + assert.ok(Array.isArray(connections), 'Expected connections array'); + assert.equal(connections.length, 4, 'Expected 4 connections'); + }); + + it('maps sf class to Salesforce type', () => { + writeTestProject(tmpDir, BASIC_TEST_PROJECT); + const result = server.call('provar.connection.list', { project_path: tmpDir }); + const connections = parseText(result)['connections'] as Array>; + const sfConns = connections.filter((c) => c['type'] === 'Salesforce'); + assert.equal(sfConns.length, 2, 'Expected 2 Salesforce connections'); + assert.equal(sfConns[0]['name'], 'MyOrg'); + }); + + it('maps ui class to Web type', () => { + writeTestProject(tmpDir, BASIC_TEST_PROJECT); + const result = server.call('provar.connection.list', { project_path: tmpDir }); + const connections = parseText(result)['connections'] as Array>; + const webConns = connections.filter((c) => c['type'] === 'Web'); + assert.equal(webConns.length, 1); + assert.equal(webConns[0]['name'], 'Chrome'); + }); + + it('marks sso class connections as sso_configured=true', () => { + writeTestProject(tmpDir, BASIC_TEST_PROJECT); + const result = server.call('provar.connection.list', { project_path: tmpDir }); + const connections = parseText(result)['connections'] as Array>; + const ssoConn = connections.find((c) => c['name'] === 'OktaSso'); + assert.ok(ssoConn, 'Expected OktaSso connection'); + assert.equal(ssoConn['sso_configured'], true); + }); + + it('marks non-sso connections as sso_configured=false', () => { + writeTestProject(tmpDir, BASIC_TEST_PROJECT); + const result = server.call('provar.connection.list', { project_path: tmpDir }); + const connections = parseText(result)['connections'] as Array>; + const sfConn = connections.find((c) => c['name'] === 'MyOrg'); + assert.ok(sfConn); + assert.equal(sfConn['sso_configured'], false); + }); + + it('returns environments with name, connection, and url', () => { + writeTestProject(tmpDir, BASIC_TEST_PROJECT); + const result = server.call('provar.connection.list', { project_path: tmpDir }); + const environments = parseText(result)['environments'] as Array>; + assert.ok(Array.isArray(environments)); + assert.equal(environments.length, 2); + const qa = environments.find((e) => e['name'] === 'QA'); + assert.ok(qa); + assert.equal(qa['connection'], 'MyOrg'); + assert.equal(qa['url'], 'https://qa.example.com'); + }); + + it('returns environment without url when not present', () => { + writeTestProject(tmpDir, BASIC_TEST_PROJECT); + const result = server.call('provar.connection.list', { project_path: tmpDir }); + const environments = parseText(result)['environments'] as Array>; + const uat = environments.find((e) => e['name'] === 'UAT'); + assert.ok(uat); + assert.equal(uat['url'], undefined, 'UAT has no url attribute'); + }); + + it('returns summary with correct counts', () => { + writeTestProject(tmpDir, BASIC_TEST_PROJECT); + const result = server.call('provar.connection.list', { project_path: tmpDir }); + const summary = parseText(result)['summary'] as Record; + assert.equal(summary['connection_count'], 4); + assert.equal(summary['environment_count'], 2); + }); + + it('returns empty arrays for project with no connections or environments', () => { + writeTestProject(tmpDir, EMPTY_TEST_PROJECT); + const result = server.call('provar.connection.list', { project_path: tmpDir }); + assert.equal(isError(result), false); + const body = parseText(result); + assert.deepEqual(body['connections'], []); + assert.deepEqual(body['environments'], []); + }); + }); + + describe('error cases', () => { + it('returns CONNECTION_FILE_NOT_FOUND when .testproject is missing', () => { + const result = server.call('provar.connection.list', { project_path: tmpDir }); + assert.equal(isError(result), true); + const body = parseText(result); + assert.equal(body['error_code'], 'CONNECTION_FILE_NOT_FOUND'); + }); + + it('CONNECTION_FILE_NOT_FOUND includes a suggestion', () => { + const result = server.call('provar.connection.list', { project_path: tmpDir }); + const body = parseText(result); + const details = body['details'] as Record; + assert.ok(details?.['suggestion'], 'Expected suggestion in details'); + }); + + it('returns PATH_NOT_ALLOWED when project_path is outside allowed paths', () => { + const strictServer = new MockMcpServer(); + registerConnectionList(strictServer as never, { allowedPaths: [tmpDir] }); + const result = strictServer.call('provar.connection.list', { + project_path: path.join(os.tmpdir(), 'some-other-project'), + }); + assert.equal(isError(result), true); + const code = parseText(result)['error_code'] as string; + assert.ok(code === 'PATH_NOT_ALLOWED' || code === 'PATH_TRAVERSAL', `Unexpected: ${code}`); + }); + }); +}); diff --git a/test/unit/mcp/pageObjectGenerate.test.ts b/test/unit/mcp/pageObjectGenerate.test.ts index 566163c..217abc2 100644 --- a/test/unit/mcp/pageObjectGenerate.test.ts +++ b/test/unit/mcp/pageObjectGenerate.test.ts @@ -168,8 +168,18 @@ describe('provar.pageobject.generate', () => { package_name: 'pageobjects', page_type: 'standard', fields: [ - { name: 'accountName', locator_strategy: 'xpath', locator_value: "//input[@name='accountName']", element_type: 'TextType' }, - { name: 'saveButton', locator_strategy: 'css', locator_value: "[data-testid='save']", element_type: 'ButtonType' }, + { + name: 'accountName', + locator_strategy: 'xpath', + locator_value: "//input[@name='accountName']", + element_type: 'TextType', + }, + { + name: 'saveButton', + locator_strategy: 'css', + locator_value: "[data-testid='save']", + element_type: 'ButtonType', + }, ], dry_run: true, overwrite: false, @@ -362,4 +372,129 @@ describe('provar.pageobject.generate', () => { assert.equal(parseText(result)['idempotency_key'], undefined); }); }); + + describe('sso_class — ILoginPage stub generation', () => { + it('returns sso_stub_source when sso_class is provided', () => { + const result = server.call('provar.pageobject.generate', { + class_name: 'LoginPage', + package_name: 'pageobjects', + page_type: 'standard', + fields: [], + sso_class: 'LoginPageSso', + dry_run: true, + overwrite: false, + }); + + assert.equal(isError(result), false); + const body = parseText(result); + const ssoSource = body['sso_stub_source'] as string; + assert.ok(typeof ssoSource === 'string' && ssoSource.length > 0, 'Expected sso_stub_source'); + assert.ok(ssoSource.includes('implements ILoginPage'), 'Stub must implement ILoginPage'); + assert.ok(ssoSource.includes('class LoginPageSso'), 'Stub class name must match sso_class'); + }); + + it('sso stub includes loginAs and logout method stubs', () => { + const result = server.call('provar.pageobject.generate', { + class_name: 'LoginPage', + package_name: 'pageobjects', + page_type: 'standard', + fields: [], + sso_class: 'LoginPageSso', + dry_run: true, + overwrite: false, + }); + + const ssoSource = parseText(result)['sso_stub_source'] as string; + assert.ok(ssoSource.includes('loginAs'), 'Expected loginAs method'); + assert.ok(ssoSource.includes('logout'), 'Expected logout method'); + }); + + it('uses the correct package in sso stub', () => { + const result = server.call('provar.pageobject.generate', { + class_name: 'LoginPage', + package_name: 'pageobjects.auth', + page_type: 'standard', + fields: [], + sso_class: 'AuthSso', + dry_run: true, + overwrite: false, + }); + + const ssoSource = parseText(result)['sso_stub_source'] as string; + assert.ok(ssoSource.includes('package pageobjects.auth;'), 'Expected correct package'); + }); + + it('does not include sso fields when sso_class is omitted', () => { + const result = server.call('provar.pageobject.generate', { + class_name: 'AccountPage', + package_name: 'pageobjects', + page_type: 'standard', + fields: [], + dry_run: true, + overwrite: false, + }); + + const body = parseText(result); + assert.ok(!('sso_stub_source' in body), 'sso_stub_source should be absent when no sso_class'); + }); + + it('writes both page object and SSO stub to disk when dry_run=false', () => { + const poPath = path.join(tmpDir, 'LoginPage.java'); + const result = server.call('provar.pageobject.generate', { + class_name: 'LoginPage', + package_name: 'pageobjects', + page_type: 'standard', + fields: [], + sso_class: 'LoginPageSso', + output_path: poPath, + dry_run: false, + overwrite: false, + }); + + assert.equal(isError(result), false); + assert.equal(fs.existsSync(poPath), true, 'Page object file should be written'); + const ssoPath = path.join(tmpDir, 'LoginPageSso.java'); + assert.equal(fs.existsSync(ssoPath), true, 'SSO stub file should be written'); + }); + + it('validates path policy on SSO stub path', () => { + const strictServer = new MockMcpServer(); + registerPageObjectGenerate(strictServer as never, { allowedPaths: [tmpDir] }); + + const result = strictServer.call('provar.pageobject.generate', { + class_name: 'LoginPage', + package_name: 'pageobjects', + page_type: 'standard', + fields: [], + sso_class: 'LoginPageSso', + output_path: path.join(os.tmpdir(), 'some-other-dir', 'LoginPage.java'), + dry_run: false, + overwrite: false, + }); + + assert.equal(isError(result), true); + const code = parseText(result)['error_code'] as string; + assert.ok(code === 'PATH_NOT_ALLOWED' || code === 'PATH_TRAVERSAL', `Unexpected: ${code}`); + }); + + it('returns FILE_EXISTS when SSO stub already exists and overwrite=false', () => { + const poPath = path.join(tmpDir, 'LoginPage.java'); + const ssoPath = path.join(tmpDir, 'LoginPageSso.java'); + fs.writeFileSync(ssoPath, '// existing stub', 'utf-8'); + + const result = server.call('provar.pageobject.generate', { + class_name: 'LoginPage', + package_name: 'pageobjects', + page_type: 'standard', + fields: [], + sso_class: 'LoginPageSso', + output_path: poPath, + dry_run: false, + overwrite: false, + }); + + assert.equal(isError(result), true); + assert.equal(parseText(result)['error_code'], 'FILE_EXISTS'); + }); + }); }); diff --git a/test/unit/mcp/qualityHubTools.test.ts b/test/unit/mcp/qualityHubTools.test.ts index 81cf1a0..d80434c 100644 --- a/test/unit/mcp/qualityHubTools.test.ts +++ b/test/unit/mcp/qualityHubTools.test.ts @@ -31,11 +31,23 @@ class MockMcpServer { // ── Helpers ─────────────────────────────────────────────────────────────────── -function makeSpawnResult(stdout: string, stderr: string, status: number): { stdout: string; stderr: string; status: number; error: undefined; pid: number; output: never[]; signal: null } { +function makeSpawnResult( + stdout: string, + stderr: string, + status: number +): { stdout: string; stderr: string; status: number; error: undefined; pid: number; output: never[]; signal: null } { return { stdout, stderr, status, error: undefined, pid: 1, output: [], signal: null }; } -function makeEnoentResult(): { stdout: string; stderr: string; status: null; error: Error & { code: string }; pid: undefined; output: never[]; signal: null } { +function makeEnoentResult(): { + stdout: string; + stderr: string; + status: null; + error: Error & { code: string }; + pid: undefined; + output: never[]; + signal: null; +} { const err = Object.assign(new Error('spawn sf ENOENT'), { code: 'ENOENT' }); return { stdout: '', stderr: '', status: null, error: err, pid: undefined, output: [], signal: null }; } @@ -167,6 +179,40 @@ describe('qualityHubTools', () => { const result = server.call('provar.qualityhub.testrun', { target_org: 'myorg', flags: [] }); assert.equal(parseBody(result).error_code, 'SF_NOT_FOUND'); }); + + it('adds wildcard warning when flags contain * glob pattern', () => { + spawnStub.returns(makeSpawnResult('run started', '', 0)); + const result = server.call('provar.qualityhub.testrun', { + target_org: 'myorg', + flags: ['--plan-name', 'Suite/E2E*'], + }); + assert.equal(isError(result), false, 'wildcard should not block execution'); + const body = parseBody(result); + const details = body.details as Record | undefined; + assert.ok(details?.warning, 'Expected warning in details'); + assert.ok((details.warning as string).includes('Wildcard'), 'Warning should mention wildcard'); + }); + + it('adds wildcard warning when flags contain ? pattern', () => { + spawnStub.returns(makeSpawnResult('run started', '', 0)); + const result = server.call('provar.qualityhub.testrun', { + target_org: 'myorg', + flags: ['--plan-name', 'Suite?Test'], + }); + const body = parseBody(result); + const details = body.details as Record | undefined; + assert.ok(details?.warning, 'Expected warning for ? wildcard'); + }); + + it('does not add warning for exact plan name flags', () => { + spawnStub.returns(makeSpawnResult('run started', '', 0)); + const result = server.call('provar.qualityhub.testrun', { + target_org: 'myorg', + flags: ['--plan-name', 'SmokeTests'], + }); + const body = parseBody(result); + assert.ok(!body.details, 'No details warning for exact plan name'); + }); }); // ── provar.qualityhub.testrun.report ───────────────────────────────────────── @@ -182,58 +228,66 @@ describe('qualityHubTools', () => { it('returns QH_REPORT_FAILED on non-zero exit', () => { spawnStub.returns(makeSpawnResult('', 'not found', 1)); - const result = server.call('provar.qualityhub.testrun.report', { target_org: 'myorg', run_id: 'abc-123', flags: [] }); + const result = server.call('provar.qualityhub.testrun.report', { + target_org: 'myorg', + run_id: 'abc-123', + flags: [], + }); assert.ok(isError(result)); assert.equal(parseBody(result).error_code, 'QH_REPORT_FAILED'); }); it('returns SF_NOT_FOUND on ENOENT', () => { spawnStub.returns(makeEnoentResult()); - const result = server.call('provar.qualityhub.testrun.report', { target_org: 'myorg', run_id: 'abc-123', flags: [] }); + const result = server.call('provar.qualityhub.testrun.report', { + target_org: 'myorg', + run_id: 'abc-123', + flags: [], + }); assert.equal(parseBody(result).error_code, 'SF_NOT_FOUND'); }); describe('failure detection', () => { it('sets suggestion when JSON result.status is "FAILED"', () => { - spawnStub.returns(makeSpawnResult( - JSON.stringify({ result: { status: 'FAILED' } }), - '', - 0 - )); - const result = server.call('provar.qualityhub.testrun.report', { target_org: 'myorg', run_id: 'abc-123', flags: [] }); + spawnStub.returns(makeSpawnResult(JSON.stringify({ result: { status: 'FAILED' } }), '', 0)); + const result = server.call('provar.qualityhub.testrun.report', { + target_org: 'myorg', + run_id: 'abc-123', + flags: [], + }); const body = parseBody(result); assert.ok(typeof body.suggestion === 'string' && body.suggestion.length > 0, 'Expected suggestion when FAILED'); }); it('sets suggestion when JSON result.status is "FAIL"', () => { - spawnStub.returns(makeSpawnResult( - JSON.stringify({ result: { status: 'FAIL' } }), - '', - 0 - )); - const result = server.call('provar.qualityhub.testrun.report', { target_org: 'myorg', run_id: 'abc-123', flags: [] }); + spawnStub.returns(makeSpawnResult(JSON.stringify({ result: { status: 'FAIL' } }), '', 0)); + const result = server.call('provar.qualityhub.testrun.report', { + target_org: 'myorg', + run_id: 'abc-123', + flags: [], + }); const body = parseBody(result); assert.ok(typeof body.suggestion === 'string' && body.suggestion.length > 0, 'Expected suggestion when FAIL'); }); it('does NOT set suggestion when status is "RUNNING"', () => { - spawnStub.returns(makeSpawnResult( - JSON.stringify({ result: { status: 'RUNNING' } }), - '', - 0 - )); - const result = server.call('provar.qualityhub.testrun.report', { target_org: 'myorg', run_id: 'abc-123', flags: [] }); + spawnStub.returns(makeSpawnResult(JSON.stringify({ result: { status: 'RUNNING' } }), '', 0)); + const result = server.call('provar.qualityhub.testrun.report', { + target_org: 'myorg', + run_id: 'abc-123', + flags: [], + }); const body = parseBody(result); assert.ok(!body.suggestion, 'Expected no suggestion for non-failure status'); }); it('does NOT set suggestion when status is "PASSED"', () => { - spawnStub.returns(makeSpawnResult( - JSON.stringify({ result: { status: 'PASSED' } }), - '', - 0 - )); - const result = server.call('provar.qualityhub.testrun.report', { target_org: 'myorg', run_id: 'abc-123', flags: [] }); + spawnStub.returns(makeSpawnResult(JSON.stringify({ result: { status: 'PASSED' } }), '', 0)); + const result = server.call('provar.qualityhub.testrun.report', { + target_org: 'myorg', + run_id: 'abc-123', + flags: [], + }); const body = parseBody(result); assert.ok(!body.suggestion, 'Expected no suggestion when PASSED'); }); @@ -241,26 +295,31 @@ describe('qualityHubTools', () => { it('does NOT false-positive on "failure" in plain text output (word in non-status context)', () => { // Before PR #110 the check was /fail/i which would match "failure" anywhere in output; // now it only matches the "status" field value. - spawnStub.returns(makeSpawnResult( - '{"message": "No failure detected in this output", "result": {"status": "PASSED"}}', - '', - 0 - )); - const result = server.call('provar.qualityhub.testrun.report', { target_org: 'myorg', run_id: 'abc-123', flags: [] }); + spawnStub.returns( + makeSpawnResult('{"message": "No failure detected in this output", "result": {"status": "PASSED"}}', '', 0) + ); + const result = server.call('provar.qualityhub.testrun.report', { + target_org: 'myorg', + run_id: 'abc-123', + flags: [], + }); const body = parseBody(result); assert.ok(!body.suggestion, 'Expected no suggestion — "failure" in message should not trigger detection'); }); it('falls back to regex extraction when stdout is not valid JSON', () => { // Non-JSON output with "status": "FAILED" substring - spawnStub.returns(makeSpawnResult( - '"status": "FAILED"', - '', - 0 - )); - const result = server.call('provar.qualityhub.testrun.report', { target_org: 'myorg', run_id: 'abc-123', flags: [] }); + spawnStub.returns(makeSpawnResult('"status": "FAILED"', '', 0)); + const result = server.call('provar.qualityhub.testrun.report', { + target_org: 'myorg', + run_id: 'abc-123', + flags: [], + }); const body = parseBody(result); - assert.ok(typeof body.suggestion === 'string' && body.suggestion.length > 0, 'Expected suggestion from regex fallback'); + assert.ok( + typeof body.suggestion === 'string' && body.suggestion.length > 0, + 'Expected suggestion from regex fallback' + ); }); }); }); @@ -278,14 +337,22 @@ describe('qualityHubTools', () => { it('returns QH_ABORT_FAILED on non-zero exit', () => { spawnStub.returns(makeSpawnResult('', 'abort failed', 1)); - const result = server.call('provar.qualityhub.testrun.abort', { target_org: 'myorg', run_id: 'abc-123', flags: [] }); + const result = server.call('provar.qualityhub.testrun.abort', { + target_org: 'myorg', + run_id: 'abc-123', + flags: [], + }); assert.ok(isError(result)); assert.equal(parseBody(result).error_code, 'QH_ABORT_FAILED'); }); it('returns SF_NOT_FOUND on ENOENT', () => { spawnStub.returns(makeEnoentResult()); - const result = server.call('provar.qualityhub.testrun.abort', { target_org: 'myorg', run_id: 'abc-123', flags: [] }); + const result = server.call('provar.qualityhub.testrun.abort', { + target_org: 'myorg', + run_id: 'abc-123', + flags: [], + }); assert.equal(parseBody(result).error_code, 'SF_NOT_FOUND'); }); }); @@ -295,7 +362,10 @@ describe('qualityHubTools', () => { describe('provar.qualityhub.testcase.retrieve', () => { it('calls sf with testcase retrieve args', () => { spawnStub.returns(makeSpawnResult('[]', '', 0)); - const result = server.call('provar.qualityhub.testcase.retrieve', { target_org: 'myorg', flags: ['--user-story', 'US-1'] }); + const result = server.call('provar.qualityhub.testcase.retrieve', { + target_org: 'myorg', + flags: ['--user-story', 'US-1'], + }); const body = parseBody(result); assert.equal(body.exitCode, 0); const args = spawnStub.firstCall.args[1] as string[]; diff --git a/test/unit/mcp/testCaseGenerate.test.ts b/test/unit/mcp/testCaseGenerate.test.ts index cd24444..bbdcf1b 100644 --- a/test/unit/mcp/testCaseGenerate.test.ts +++ b/test/unit/mcp/testCaseGenerate.test.ts @@ -392,4 +392,101 @@ describe('provar.testcase.generate', () => { assert.ok(!xml.includes('valueClass="String"'), 'Must not emit uppercase valueClass="String"'); }); }); + + describe('target_uri — non-SF page object (ui:) nesting', () => { + it('wraps steps in UiWithScreen when target_uri uses ?pageId= format', () => { + const result = server.call('provar.testcase.generate', { + test_case_name: 'Non-SF Login', + steps: [{ api_id: 'UiDoAction', name: 'Enter username', attributes: { field: 'username' } }], + target_uri: 'ui:pageobject:target?pageId=pageobjects.LoginPage', + dry_run: true, + overwrite: false, + validate_after_edit: true, + }); + + assert.equal(isError(result), false); + const xml = parseText(result)['xml_content'] as string; + assert.ok(xml.includes('UiWithScreen'), 'Expected UiWithScreen wrapper'); + assert.ok(xml.includes('testItemId="1"'), 'UiWithScreen should be testItemId=1'); + assert.ok(xml.includes('ui:pageobject:target?pageId=pageobjects.LoginPage'), 'Expected target URI in XML'); + assert.ok(xml.includes(' { + const result = server.call('provar.testcase.generate', { + test_case_name: 'Non-SF Multi', + steps: [ + { api_id: 'UiDoAction', name: 'Step A', attributes: {} }, + { api_id: 'UiDoAction', name: 'Step B', attributes: {} }, + ], + target_uri: 'ui:pageobject:target?pageId=pageobjects.LoginPage', + dry_run: true, + overwrite: false, + validate_after_edit: true, + }); + + const xml = parseText(result)['xml_content'] as string; + assert.ok(xml.includes('testItemId="2"'), 'substeps clause should be testItemId=2'); + assert.ok(xml.includes('testItemId="3"'), 'First inner step should be testItemId=3'); + assert.ok(xml.includes('testItemId="4"'), 'Second inner step should be testItemId=4'); + }); + + it('uses flat structure when target_uri starts with sf:', () => { + const result = server.call('provar.testcase.generate', { + test_case_name: 'SF Target', + steps: [{ api_id: 'UiConnect', name: 'Connect', attributes: {} }], + target_uri: 'sf:ui:target:Salesforce__Standard__Account', + dry_run: true, + overwrite: false, + validate_after_edit: true, + }); + + const xml = parseText(result)['xml_content'] as string; + assert.ok(!xml.includes('UiWithScreen'), 'Should not wrap in UiWithScreen for sf: target'); + assert.ok(xml.includes('testItemId="1"'), 'Step should be testItemId=1 directly'); + }); + + it('uses flat structure when target_uri is omitted', () => { + const result = server.call('provar.testcase.generate', { + test_case_name: 'No URI', + steps: [{ api_id: 'UiConnect', name: 'Connect', attributes: {} }], + dry_run: true, + overwrite: false, + validate_after_edit: true, + }); + + const xml = parseText(result)['xml_content'] as string; + assert.ok(!xml.includes('UiWithScreen'), 'No UiWithScreen without target_uri'); + }); + }); + + describe('validate_after_edit', () => { + it('includes validation field when validate_after_edit=true (default)', () => { + const result = server.call('provar.testcase.generate', { + test_case_name: 'Validated', + steps: [{ api_id: 'UiConnect', name: 'Connect', attributes: {} }], + dry_run: true, + overwrite: false, + validate_after_edit: true, + }); + + assert.equal(isError(result), false); + const body = parseText(result); + assert.ok(body['validation'], 'Expected validation field'); + }); + + it('omits validation field when validate_after_edit=false', () => { + const result = server.call('provar.testcase.generate', { + test_case_name: 'Skip Validation', + steps: [], + dry_run: true, + overwrite: false, + validate_after_edit: false, + }); + + assert.equal(isError(result), false); + const body = parseText(result); + assert.ok(!('validation' in body), 'validation field should be absent when validate_after_edit=false'); + }); + }); }); From 3f61a9a200cd96af28c555a0862758cc2406d348 Mon Sep 17 00:00:00 2001 From: Michael Dailey Date: Fri, 24 Apr 2026 16:58:48 -0500 Subject: [PATCH 2/2] PDX-0: fix(mcp): address PR #134 review comments and Codex findings 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 --- docs/mcp.md | 58 ++++++++-------- src/mcp/tools/bestPracticesEngine.ts | 2 +- src/mcp/tools/connectionTools.ts | 10 ++- src/mcp/tools/pageObjectGenerate.ts | 84 +++++++++++++++++------- src/mcp/tools/qualityHubTools.ts | 2 +- src/mcp/tools/testCaseGenerate.ts | 2 +- test/unit/mcp/connectionTools.test.ts | 7 ++ test/unit/mcp/pageObjectGenerate.test.ts | 19 ++++++ 8 files changed, 125 insertions(+), 59 deletions(-) diff --git a/docs/mcp.md b/docs/mcp.md index adc5858..0f1ec55 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -577,20 +577,20 @@ Generates a Java Page Object skeleton with the correct `@Page` or `@SalesforcePa **Input** -| 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 | +| --------------------------- | ------------------------------------------------------------------ | -------- | ------------------------------------------------------------------------------------------------------- | +| `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, element_type, locator_strategy, 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 | **Output** — `{ java_source: string, file_path?: string, written: boolean, sso_stub_source?: string, sso_stub_file_path?: string, sso_stub_written?: boolean }` @@ -638,17 +638,17 @@ Generates an XML test case skeleton with UUID v4 guids and sequential `testItemI **Input** -| 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 | +| 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, attributes? }` | 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 | **Output** — `{ xml_content: string, file_path?: string, written: boolean, validation?: ValidationResult }` @@ -1099,10 +1099,10 @@ Triggers a Quality Hub test run. Invokes `sf provar quality-hub test run`. Retur **Input** -| 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. | +| 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-name` values — they skip QH plan-level reporting. | **Output** — `{ requestId, exitCode, stdout, stderr, details?: { warning: string } }` diff --git a/src/mcp/tools/bestPracticesEngine.ts b/src/mcp/tools/bestPracticesEngine.ts index 0bc70ba..bcca9ca 100644 --- a/src/mcp/tools/bestPracticesEngine.ts +++ b/src/mcp/tools/bestPracticesEngine.ts @@ -743,7 +743,7 @@ function validateUiWithScreenTarget(tc: XmlNode, rule: BPRule): BPViolation | nu for (const call of getAllApiCalls(tc)) { const apiId = call['@_apiId'] as string | undefined; if (!apiId) continue; - if (targetApiId && !apiId.includes('UiWithScreen')) continue; + if (targetApiId && !apiId.includes(targetApiId)) continue; const target = getUiWithScreenTarget(call); if (!target) continue; diff --git a/src/mcp/tools/connectionTools.ts b/src/mcp/tools/connectionTools.ts index 4e09828..c500e75 100644 --- a/src/mcp/tools/connectionTools.ts +++ b/src/mcp/tools/connectionTools.ts @@ -60,12 +60,16 @@ const TP_PARSER = new XMLParser({ isArray: (name): boolean => name === 'connectionClass' || name === 'connection' || name === 'environment', }); +class XmlParseError extends Error { + public code = 'CONNECTION_XML_PARSE_ERROR'; +} + function parseTestProjectXml(content: string): Record { let parsed: Record; try { parsed = TP_PARSER.parse(content) as Record; - } catch { - return {}; + } catch (e) { + throw new XmlParseError(`Failed to parse .testproject XML: ${(e as Error).message}`); } const raw = parsed['testProject']; return raw !== null && typeof raw === 'object' ? (raw as Record) : {}; @@ -161,6 +165,8 @@ export function registerConnectionList(server: McpServer, config: ServerConfig): return { isError: true, content: [{ type: 'text' as const, text: JSON.stringify(err) }] }; } + assertPathAllowed(testProjectPath, config.allowedPaths); + let content: string; try { content = fs.readFileSync(testProjectPath, 'utf-8'); diff --git a/src/mcp/tools/pageObjectGenerate.ts b/src/mcp/tools/pageObjectGenerate.ts index 43e4023..c1c1366 100644 --- a/src/mcp/tools/pageObjectGenerate.ts +++ b/src/mcp/tools/pageObjectGenerate.ts @@ -50,6 +50,56 @@ const FieldSchema = z.object({ element_type: z.enum(VALID_ELEMENT_TYPES).default('TextType'), }); +type ToolResult = { isError: true; content: Array<{ type: 'text'; text: string }> } | null; + +function preflightAndWrite( + filePath: string, + javaSource: string, + ssoFilePath: string | undefined, + ssoSource: string | undefined, + overwrite: boolean, + requestId: string +): ToolResult { + if (fs.existsSync(filePath) && !overwrite) { + return { + isError: true, + content: [ + { + type: 'text' as const, + text: JSON.stringify( + makeError('FILE_EXISTS', `File already exists: ${filePath}. Set overwrite=true to replace.`, requestId) + ), + }, + ], + }; + } + if (ssoSource && ssoFilePath && fs.existsSync(ssoFilePath) && !overwrite) { + return { + isError: true, + content: [ + { + type: 'text' as const, + text: JSON.stringify( + makeError( + 'FILE_EXISTS', + `SSO stub file already exists: ${ssoFilePath}. Set overwrite=true to replace.`, + requestId + ) + ), + }, + ], + }; + } + fs.mkdirSync(path.dirname(filePath), { recursive: true }); + fs.writeFileSync(filePath, javaSource, 'utf-8'); + log('info', 'provar.pageobject.generate: wrote file', { requestId, filePath }); + if (ssoSource && ssoFilePath) { + fs.writeFileSync(ssoFilePath, ssoSource, 'utf-8'); + log('info', 'provar.pageobject.generate: wrote SSO stub', { requestId, ssoFilePath }); + } + return null; +} + export function registerPageObjectGenerate(server: McpServer, config: ServerConfig): void { server.tool( 'provar.pageobject.generate', @@ -115,32 +165,16 @@ export function registerPageObjectGenerate(server: McpServer, config: ServerConf assertPathAllowed(filePath, config.allowedPaths); if (ssoFilePath) assertPathAllowed(ssoFilePath, config.allowedPaths); - if (fs.existsSync(filePath) && !input.overwrite) { - const err = makeError( - 'FILE_EXISTS', - `File already exists: ${filePath}. Set overwrite=true to replace.`, - requestId - ); - return { isError: true, content: [{ type: 'text' as const, text: JSON.stringify(err) }] }; - } - - fs.mkdirSync(path.dirname(filePath), { recursive: true }); - fs.writeFileSync(filePath, javaSource, 'utf-8'); + const preflightErr = preflightAndWrite( + filePath, + javaSource, + ssoFilePath, + ssoSource, + input.overwrite, + requestId + ); + if (preflightErr) return preflightErr; 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) }] }; - } - fs.writeFileSync(ssoFilePath, ssoSource, 'utf-8'); - log('info', 'provar.pageobject.generate: wrote SSO stub', { requestId, ssoFilePath }); - } } const result: Record = { diff --git a/src/mcp/tools/qualityHubTools.ts b/src/mcp/tools/qualityHubTools.ts index 8afc7a8..a1b8120 100644 --- a/src/mcp/tools/qualityHubTools.ts +++ b/src/mcp/tools/qualityHubTools.ts @@ -143,7 +143,7 @@ export function registerQualityHubTestRun(server: McpServer): void { .optional() .default([]) .describe( - 'Additional raw CLI flags (e.g. ["--plan-name", "SmokeTests"]). Avoid wildcards in plan names — they skip QH plan-level reporting.' + 'Additional raw CLI flags (e.g. ["--plan-name", "SmokeTests"]). Avoid wildcards in --plan-name values — they skip QH plan-level reporting.' ), }, ({ target_org, flags }) => { diff --git a/src/mcp/tools/testCaseGenerate.ts b/src/mcp/tools/testCaseGenerate.ts index e116f99..e053cf9 100644 --- a/src/mcp/tools/testCaseGenerate.ts +++ b/src/mcp/tools/testCaseGenerate.ts @@ -203,7 +203,7 @@ export function registerTestCaseGenerate(server: McpServer, config: ServerConfig if (!validationFull.is_valid) { const errResult = makeError( 'TESTCASE_INVALID', - `Generated test case failed structural validation (${validationFull.error_count} error(s)). See validation field.`, + `Generated test case failed structural validation (${validationFull.error_count} error(s)). See details.validation.`, requestId, false, { validation: validationSlim } diff --git a/test/unit/mcp/connectionTools.test.ts b/test/unit/mcp/connectionTools.test.ts index c75e84b..4da062f 100644 --- a/test/unit/mcp/connectionTools.test.ts +++ b/test/unit/mcp/connectionTools.test.ts @@ -211,5 +211,12 @@ describe('provar.connection.list', () => { const code = parseText(result)['error_code'] as string; assert.ok(code === 'PATH_NOT_ALLOWED' || code === 'PATH_TRAVERSAL', `Unexpected: ${code}`); }); + + it('returns CONNECTION_XML_PARSE_ERROR for malformed .testproject XML', () => { + writeTestProject(tmpDir, ' { assert.equal(isError(result), true); assert.equal(parseText(result)['error_code'], 'FILE_EXISTS'); }); + + it('does not write main file when SSO stub preflight fails (atomic write)', () => { + const poPath = path.join(tmpDir, 'LoginPage.java'); + const ssoPath = path.join(tmpDir, 'LoginPageSso.java'); + fs.writeFileSync(ssoPath, '// existing stub', 'utf-8'); + + server.call('provar.pageobject.generate', { + class_name: 'LoginPage', + package_name: 'pageobjects', + page_type: 'standard', + fields: [], + sso_class: 'LoginPageSso', + output_path: poPath, + dry_run: false, + overwrite: false, + }); + + assert.equal(fs.existsSync(poPath), false, 'Main .java should not be written when SSO preflight fails'); + }); }); });