From debca7856a370639bd10e4824b8ebafb8bd2ac64 Mon Sep 17 00:00:00 2001 From: Michael Dailey Date: Fri, 1 May 2026 11:24:26 -0500 Subject: [PATCH 01/19] PDX-0: fix(mcp): address session 3 feedback D1-D4 + D7 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit D1: add provar.testplan.create tool — creates plans/{name}/ directory and root .planitem; previously agents had to create plan files manually. D2: emit class="uiTarget" uri="..." for the 'target' argument and class="uiLocator" uri="..." for 'locator' argument in XML generation; previously both were written as valueClass="string" causing a ClassCastException in the Provar runtime. D3: SetValues / AssertValues steps now emit ... for the 'values' argument; previously a pipe-delimited string was written, causing an immediate ClassCastException. D4: attribute values matching {VarName} or {Obj.Field} are emitted as class="variable" with children; previously emitted as plain string literals which the runtime rejects. D7: provar.testcase.generate now includes a cleanup warning when ApexDeleteObject steps are present, explaining that cleanup steps after a failure point are skipped when stopOnError=false. Also updates StepSchema.attributes description and TOOL_DESCRIPTION to document all four special value conventions for agent grounding. Tests: 19 new assertions across testCaseGenerate.test.ts and testPlanTools.test.ts covering happy path, dry_run, and error cases. Smoke test: +1 entry for provar.testplan.create (TOTAL_EXPECTED 50->51). Co-Authored-By: Claude Sonnet 4.6 --- scripts/mcp-smoke.cjs | 15 +- src/mcp/tools/testCaseGenerate.ts | 99 +++++++++- src/mcp/tools/testPlanTools.ts | 115 ++++++++++++ test/unit/mcp/testCaseGenerate.test.ts | 243 +++++++++++++++++++++++++ test/unit/mcp/testPlanTools.test.ts | 141 +++++++++++++- 5 files changed, 598 insertions(+), 15 deletions(-) diff --git a/scripts/mcp-smoke.cjs b/scripts/mcp-smoke.cjs index 06b709ff..68e52c53 100644 --- a/scripts/mcp-smoke.cjs +++ b/scripts/mcp-smoke.cjs @@ -248,7 +248,14 @@ async function runTests() { // ── 30. provar.testrun.rca ─────────────────────────────────────────────── await callTool('provar.testrun.rca', { project_path: TMP }); - // ── 31. provar.testplan.add-instance ───────────────────────────────────── + // ── 31. provar.testplan.create ──────────────────────────────────────────── + // TMP is not a Provar project → NOT_A_PROJECT result + await callTool('provar.testplan.create', { + project_path: TMP, + plan_name: 'SmokePlan', + }); + + // ── 33. provar.testplan.add-instance ───────────────────────────────────── // TMP is not a Provar project → NOT_A_PROJECT result await callTool('provar.testplan.add-instance', { project_path: TMP, @@ -256,14 +263,14 @@ async function runTests() { plan_name: 'SmokePlan', }); - // ── 32. provar.testplan.create-suite ───────────────────────────────────── + // ── 34. provar.testplan.create-suite ───────────────────────────────────── await callTool('provar.testplan.create-suite', { project_path: TMP, plan_name: 'SmokePlan', suite_name: 'SmokeSuite', }); - // ── 33. provar.testplan.remove-instance ────────────────────────────────── + // ── 35. provar.testplan.remove-instance ────────────────────────────────── await callTool('provar.testplan.remove-instance', { project_path: TMP, instance_path: 'plans/SmokePlan/SmokeSuite/smoke.testinstance', @@ -377,7 +384,7 @@ async function runTests() { server.on('close', () => { clearTimeout(overallTimer); // initialize + tools/list + 39 tools + prompts/list + 8 prompts/get (setup excluded from default count) - const TOTAL_EXPECTED = 50 + (INCLUDE_SETUP ? 1 : 0); + const TOTAL_EXPECTED = 51 + (INCLUDE_SETUP ? 1 : 0); let passed = 0; let failed = 0; diff --git a/src/mcp/tools/testCaseGenerate.ts b/src/mcp/tools/testCaseGenerate.ts index e053cf94..3badee23 100644 --- a/src/mcp/tools/testCaseGenerate.ts +++ b/src/mcp/tools/testCaseGenerate.ts @@ -69,6 +69,17 @@ function buildStepWarnings(steps: Array<{ api_id: string }>): string[] { ); } + // D7: Cleanup steps placed after a potential failure point are skipped when stopOnError=false. + if (resolvedIds.includes(SHORTHAND_TO_FQID['ApexDeleteObject'] ?? '')) { + warnings.push( + 'ApexDeleteObject detected (likely cleanup): with stopOnError=false Provar skips all steps after ' + + 'the first failure, so cleanup steps placed at the end of the test will NOT run when an earlier ' + + 'step fails — leaving orphaned records in the org. ' + + 'Wrap cleanup in a Provar TearDown callable, or place create/delete inside the same UiWithScreen ' + + 'clause so both run as a unit regardless of failure.' + ); + } + return warnings; } @@ -89,10 +100,18 @@ const StepSchema = z.object({ .record(z.string()) .default({}) .describe( - 'Step argument values as key/value pairs. Written as ' + - 'inside the element — the format Provar runtime requires. ' + + 'Step argument values as key/value pairs. Written as . ' + 'Do NOT rely on XML attributes on ; the runtime silently ignores them. ' + - 'Example: { "connectionName": "MyOrg", "objectApiName": "Opportunity" }' + 'Special value conventions (applied automatically by the generator): ' + + '(1) Variable references: wrap the name in braces, e.g. "{MyVar}" → emitted as class="variable" . ' + + ' Dotted paths are also supported: "{Obj.Field}" → two elements. ' + + '(2) SetValues / AssertValues: pass each variable name and its value as a flat key/value pair; ' + + ' the generator wraps them in ... automatically. ' + + ' Example for SetValues: { "testCaseName": "TC_New", "testType": "Acceptance testing" } ' + + '(3) target argument (UiWithScreen / UiWithRow): pass the sf:ui:target or ui:pageobject:target URI; ' + + ' emitted as class="uiTarget" uri="...". ' + + '(4) locator argument (UiDoAction / UiAssert): pass the locator URI; emitted as class="uiLocator" uri="...". ' + + 'All other string values use class="value" valueClass="string".' ), }); @@ -111,6 +130,12 @@ const TOOL_DESCRIPTION = [ 'ApexReadObject requires field names in attributes; omitting them produces MALFORMED_QUERY. Prefer ApexSoqlQuery.', 'AssertValues on SOQL results: index paths like "ResultList[0].Field" are not supported.', 'Use ForEach to iterate the result list, or SetValues to extract a field into a variable first.', + 'SetValues / AssertValues: pass named variable values as flat key/value pairs in attributes; ' + + 'the generator wraps them in ... automatically.', + 'Variable references: pass values as "{VarName}" (braces); emitted as class="variable" .', + 'target argument (UiWithScreen/UiWithRow): pass the URI value; emitted as class="uiTarget" uri="...".', + 'locator argument (UiDoAction/UiAssert): pass the URI value; emitted as class="uiLocator" uri="...".', + 'Cleanup warning: ApexDeleteObject steps near end of test will be skipped if an earlier step fails (stopOnError=false). Use a TearDown callable.', 'Validation: when validate_after_edit=true (default) the response includes a validation field and returns TESTCASE_INVALID if the generated XML fails structural checks.', 'Grounding: call provar.qualityhub.examples.retrieve before generating to get corpus examples for the scenario — correct XML structure for the step types you need.', ].join(' '); @@ -239,20 +264,75 @@ export function registerTestCaseGenerate(server: McpServer, config: ServerConfig // ── XML builder ─────────────────────────────────────────────────────────────── +// APIs whose 'values' argument must use class="valueList"/ (D3). +const SET_VALUES_APIS = new Set([ + SHORTHAND_TO_FQID['SetValues'], // com.provar.plugins.bundled.apis.control.SetValues + SHORTHAND_TO_FQID['AssertValues'], // com.provar.plugins.bundled.apis.AssertValues +]); + +// Build the element for a single argument (D2/D4 aware). +function buildArgumentValue(key: string, val: string, indent: string): string { + // D4: {VarName} or {Obj.Field} → class="variable" with elements. + const varMatch = /^\{([\w.]+)\}$/.exec(val); + if (varMatch) { + const pathElements = varMatch[1] + .split('.') + .map((p) => `${indent} `) + .join('\n'); + return `${indent}\n${pathElements}\n${indent}`; + } + // D2: 'target' argument (UiWithScreen / UiWithRow) → class="uiTarget". + if (key === 'target') { + return `${indent}`; + } + // D2: 'locator' argument (UiDoAction / UiAssert) → class="uiLocator". + if (key === 'locator') { + return `${indent}`; + } + return `${indent}${escapeXmlContent(val)}`; +} + function buildArgumentsXml(attributes: Record, baseIndent = ' '): string { const entries = Object.entries(attributes); if (entries.length === 0) return ''; const argLines = entries - .map( - ([k, v]) => + .map(([k, v]) => { + const valueXml = buildArgumentValue(k, v, `${baseIndent} `); + return ( `${baseIndent}\n` + - `${baseIndent} ${escapeXmlContent(v)}\n` + + valueXml + '\n' + `${baseIndent}` - ) + ); + }) .join('\n'); return `\n${baseIndent}\n${argLines}\n${baseIndent}\n${baseIndent.slice(0, -2)}`; } +// D3: SetValues / AssertValues — all attributes become under a single 'values' argument. +function buildSetValuesXml(attributes: Record, baseIndent: string): string { + const entries = Object.entries(attributes); + if (entries.length === 0) return ''; + const i = (n: number): string => baseIndent + ' '.repeat(n); + const namedValueLines = entries + .map(([name, val]) => { + const valueXml = buildArgumentValue(name, val, `${i(3)} `); + return `${i(3)}\n${valueXml}\n${i(3)}`; + }) + .join('\n'); + return ( + `\n${i(0)}\n` + + `${i(0)}\n` + + `${i(1)}\n` + + `${i(2)}\n` + + namedValueLines + '\n' + + `${i(2)}\n` + + `${i(1)}\n` + + `${i(0)}\n` + + `${baseIndent.slice(0, -2)}\n` + + `${baseIndent.slice(0, -2)}` + ); +} + function buildFlatStepXml( step: { api_id: string; name: string; attributes: Record }, testItemId: number, @@ -260,7 +340,10 @@ function buildFlatStepXml( ): string { const guid = randomUUID(); const resolvedApiId = resolveApiId(step.api_id); - const argumentsXml = buildArgumentsXml(step.attributes, indent + ' '); + const baseIndent = indent + ' '; + const argumentsXml = SET_VALUES_APIS.has(resolvedApiId) + ? buildSetValuesXml(step.attributes, baseIndent) + : buildArgumentsXml(step.attributes, baseIndent); if (argumentsXml) { return ( `${indent} { + const requestId = makeRequestId(); + log('info', 'provar.testplan.create', { requestId, project_path, plan_name }); + + try { + assertPathAllowed(project_path, config.allowedPaths); + + const projectRoot = path.resolve(project_path); + + const testProjectFiles = fs.existsSync(projectRoot) + ? fs.readdirSync(projectRoot).filter((f) => f.endsWith('.testproject')) + : []; + if (testProjectFiles.length === 0) { + return { + isError: true, + content: [ + { + type: 'text' as const, + text: JSON.stringify(makeError('NOT_A_PROJECT', `No .testproject file found in ${projectRoot}`, requestId)), + }, + ], + }; + } + + const planDir = path.join(projectRoot, 'plans', plan_name); + const planItemPath = path.join(planDir, '.planitem'); + + if (!overwrite && fs.existsSync(planItemPath)) { + return { + isError: true, + content: [ + { + type: 'text' as const, + text: JSON.stringify( + makeError( + 'PLAN_EXISTS', + `Plan already exists: ${planItemPath}. Set overwrite: true to replace the .planitem file.`, + requestId + ) + ), + }, + ], + }; + } + + const guid = randomUUID(); + const xmlContent = buildPlanItemXml(guid); + + if (!dry_run) { + fs.mkdirSync(planDir, { recursive: true }); + fs.writeFileSync(planItemPath, xmlContent, 'utf-8'); + } + + const response = { + requestId, + plan_dir: planDir, + planitem_path: planItemPath, + guid, + dry_run: dry_run ?? false, + created: !dry_run, + next_steps: dry_run + ? 'Review the plan structure, then call provar.testplan.create with dry_run=false to write to disk.' + : `Plan created at ${planDir}. Use provar.testplan.create-suite to add suites, then provar.testplan.add-instance to wire test cases into the plan.`, + }; + return { + content: [{ type: 'text' as const, text: JSON.stringify(response) }], + structuredContent: response, + }; + } catch (err: unknown) { + const error = err as Error & { code?: string }; + return { + isError: true, + content: [ + { + type: 'text' as const, + text: JSON.stringify( + makeError( + error instanceof PathPolicyError ? error.code : (error.code ?? 'CREATE_PLAN_ERROR'), + error.message, + requestId + ) + ), + }, + ], + }; + } + } + ); +} + // ── provar.testplan.add-instance ────────────────────────────────────────────── export function registerTestPlanAddInstance(server: McpServer, config: ServerConfig): void { @@ -358,6 +472,7 @@ export function registerTestPlanRemoveInstance(server: McpServer, config: Server // ── Convenience re-export ───────────────────────────────────────────────────── export function registerAllTestPlanTools(server: McpServer, config: ServerConfig): void { + registerTestPlanCreate(server, config); registerTestPlanAddInstance(server, config); registerTestPlanCreateSuite(server, config); registerTestPlanRemoveInstance(server, config); diff --git a/test/unit/mcp/testCaseGenerate.test.ts b/test/unit/mcp/testCaseGenerate.test.ts index bbdcf1b0..da31040f 100644 --- a/test/unit/mcp/testCaseGenerate.test.ts +++ b/test/unit/mcp/testCaseGenerate.test.ts @@ -460,6 +460,249 @@ describe('provar.testcase.generate', () => { }); }); + describe('D2 — uiTarget / uiLocator argument types', () => { + it('emits class="uiTarget" uri="..." for the target argument', () => { + const result = server.call('provar.testcase.generate', { + test_case_name: 'UI Target Test', + steps: [ + { + api_id: 'UiWithScreen', + name: 'With page', + attributes: { target: 'sf:ui:target?pageObject=pageobjects.Account&flexiPage=Account_flexiPage' }, + }, + ], + dry_run: true, + overwrite: false, + validate_after_edit: false, + }); + + const xml = parseText(result)['xml_content'] as string; + assert.ok(xml.includes('class="uiTarget"'), 'Expected class="uiTarget"'); + assert.ok(xml.includes('uri="sf:ui:target?'), 'Expected uri attribute with sf:ui:target value'); + assert.ok(!xml.includes('valueClass="string">sf:ui:target'), 'Must NOT emit uiTarget URI as a plain string value'); + }); + + it('emits class="uiLocator" uri="..." for the locator argument', () => { + const result = server.call('provar.testcase.generate', { + test_case_name: 'UI Locator Test', + steps: [ + { + api_id: 'UiDoAction', + name: 'Click button', + attributes: { locator: 'sf:ui:locator:button?label=Save' }, + }, + ], + dry_run: true, + overwrite: false, + validate_after_edit: false, + }); + + const xml = parseText(result)['xml_content'] as string; + assert.ok(xml.includes('class="uiLocator"'), 'Expected class="uiLocator"'); + assert.ok(xml.includes('uri="sf:ui:locator:'), 'Expected uri attribute with locator value'); + assert.ok(!xml.includes('valueClass="string">sf:ui:locator'), 'Must NOT emit locator URI as a plain string value'); + }); + + it('uiTarget also applies inside UiWithScreen wrapper when target_uri is non-SF', () => { + const result = server.call('provar.testcase.generate', { + test_case_name: 'Non-SF With Target', + steps: [], + target_uri: 'ui:pageobject:target?pageId=pageobjects.LoginPage', + dry_run: true, + overwrite: false, + validate_after_edit: false, + }); + + const xml = parseText(result)['xml_content'] as string; + assert.ok(xml.includes('class="uiTarget"'), 'Wrapper UiWithScreen target should use uiTarget class'); + assert.ok(xml.includes('uri="ui:pageobject:target?pageId=pageobjects.LoginPage"'), 'URI should appear as attribute'); + }); + }); + + describe('D3 — SetValues / AssertValues use valueList/namedValues structure', () => { + it('SetValues emits with ', () => { + const result = server.call('provar.testcase.generate', { + test_case_name: 'SetValues Test', + steps: [ + { + api_id: 'SetValues', + name: 'Set test vars', + attributes: { testCaseName: 'TC_New', testType: 'Acceptance testing' }, + }, + ], + dry_run: true, + overwrite: false, + validate_after_edit: false, + }); + + const xml = parseText(result)['xml_content'] as string; + assert.ok(xml.includes('class="valueList"'), 'Expected class="valueList"'); + assert.ok(xml.includes('mutable="Mutable"'), 'Expected mutable="Mutable"'); + assert.ok(xml.includes(''), 'Expected element'); + assert.ok(xml.includes(''), 'Expected namedValue for testCaseName'); + assert.ok(xml.includes(''), 'Expected namedValue for testType'); + assert.ok(xml.includes(''), 'Expected argument id="values"'); + assert.ok( + !xml.includes('testCaseName|TC_New'), + 'Must NOT emit pipe-delimited string for SetValues' + ); + }); + + it('AssertValues also emits valueList/namedValues structure', () => { + const result = server.call('provar.testcase.generate', { + test_case_name: 'AssertValues Test', + steps: [ + { + api_id: 'AssertValues', + name: 'Assert vars', + attributes: { opportunityName: 'My Opp' }, + }, + ], + dry_run: true, + overwrite: false, + validate_after_edit: false, + }); + + const xml = parseText(result)['xml_content'] as string; + assert.ok(xml.includes('class="valueList"'), 'Expected valueList for AssertValues'); + assert.ok(xml.includes(''), 'Expected namedValue for opportunityName'); + }); + + it('non-SetValues steps still use flat argument structure', () => { + const result = server.call('provar.testcase.generate', { + test_case_name: 'Flat Args Test', + steps: [ + { api_id: 'ApexCreateObject', name: 'Create record', attributes: { objectApiName: 'Opportunity' } }, + ], + dry_run: true, + overwrite: false, + validate_after_edit: false, + }); + + const xml = parseText(result)['xml_content'] as string; + assert.ok(xml.includes(''), 'Expected flat argument id'); + assert.ok(!xml.includes('valueList'), 'Must NOT emit valueList for non-SetValues steps'); + }); + }); + + describe('D4 — Variable references use class="variable" with elements', () => { + it('{VarName} emits class="variable" ', () => { + const result = server.call('provar.testcase.generate', { + test_case_name: 'Variable Ref Test', + steps: [ + { + api_id: 'ApexCreateObject', + name: 'Create record', + attributes: { provar__Test_Project__c: '{TestProjectId}' }, + }, + ], + dry_run: true, + overwrite: false, + validate_after_edit: false, + }); + + const xml = parseText(result)['xml_content'] as string; + assert.ok(xml.includes('class="variable"'), 'Expected class="variable"'); + assert.ok(xml.includes(''), 'Expected '); + assert.ok(!xml.includes('valueClass="string">{TestProjectId}'), 'Must NOT emit {VarName} as a string literal'); + }); + + it('{Obj.Field} dotted path emits two elements', () => { + const result = server.call('provar.testcase.generate', { + test_case_name: 'Dotted Variable Test', + steps: [ + { + api_id: 'ApexCreateObject', + name: 'Create with nested var', + attributes: { Name: '{Opportunity.Name}' }, + }, + ], + dry_run: true, + overwrite: false, + validate_after_edit: false, + }); + + const xml = parseText(result)['xml_content'] as string; + assert.ok(xml.includes(''), 'Expected first path element'); + assert.ok(xml.includes(''), 'Expected second path element'); + }); + + it('variable reference also works inside SetValues namedValues', () => { + const result = server.call('provar.testcase.generate', { + test_case_name: 'SetValues With Var', + steps: [ + { + api_id: 'SetValues', + name: 'Set with variable', + attributes: { projectId: '{TestProjectId}', label: 'Static Label' }, + }, + ], + dry_run: true, + overwrite: false, + validate_after_edit: false, + }); + + const xml = parseText(result)['xml_content'] as string; + assert.ok(xml.includes('class="variable"'), 'Expected variable reference inside namedValues'); + assert.ok(xml.includes('')); + assert.ok(xml.includes('valueClass="string">Static Label'), 'Static value should still be a plain string'); + }); + + it('plain string values without braces are not treated as variable references', () => { + const result = server.call('provar.testcase.generate', { + test_case_name: 'No Var Test', + steps: [{ api_id: 'ApexCreateObject', name: 'Create', attributes: { Name: 'Literal Name' } }], + dry_run: true, + overwrite: false, + validate_after_edit: false, + }); + + const xml = parseText(result)['xml_content'] as string; + assert.ok(xml.includes('valueClass="string">Literal Name'), 'Plain string should use valueClass="string"'); + assert.ok(!xml.includes('class="variable"'), 'No variable element expected'); + }); + }); + + describe('D7 — Cleanup warning for ApexDeleteObject', () => { + it('includes cleanup warning when ApexDeleteObject is in the step list', () => { + const result = server.call('provar.testcase.generate', { + test_case_name: 'Create and Delete', + steps: [ + { api_id: 'ApexCreateObject', name: 'Create record', attributes: { objectApiName: 'Account' } }, + { api_id: 'ApexDeleteObject', name: 'Delete record', attributes: { objectApiName: 'Account' } }, + ], + dry_run: true, + overwrite: false, + validate_after_edit: false, + }); + + assert.equal(isError(result), false); + const body = parseText(result); + const warnings = body['warnings'] as string[] | undefined; + assert.ok(Array.isArray(warnings) && warnings.length > 0, 'Expected at least one warning'); + assert.ok( + warnings.some((w) => w.includes('ApexDeleteObject') && w.includes('cleanup')), + 'Expected cleanup warning mentioning ApexDeleteObject' + ); + }); + + it('does NOT warn when no ApexDeleteObject steps are present', () => { + const result = server.call('provar.testcase.generate', { + test_case_name: 'No Cleanup', + steps: [{ api_id: 'ApexCreateObject', name: 'Create', attributes: {} }], + dry_run: true, + overwrite: false, + validate_after_edit: false, + }); + + assert.equal(isError(result), false); + const body = parseText(result); + const warnings = body['warnings'] as string[] | undefined; + const hasCleanupWarning = warnings?.some((w) => w.includes('ApexDeleteObject')); + assert.ok(!hasCleanupWarning, 'No cleanup warning expected without ApexDeleteObject'); + }); + }); + describe('validate_after_edit', () => { it('includes validation field when validate_after_edit=true (default)', () => { const result = server.call('provar.testcase.generate', { diff --git a/test/unit/mcp/testPlanTools.test.ts b/test/unit/mcp/testPlanTools.test.ts index e3d7184f..28f299f5 100644 --- a/test/unit/mcp/testPlanTools.test.ts +++ b/test/unit/mcp/testPlanTools.test.ts @@ -96,6 +96,135 @@ afterEach(() => { fs.rmSync(tmpDir, { recursive: true, force: true }); }); +// ── provar.testplan.create ──────────────────────────────────────────────────── + +describe('provar.testplan.create', () => { + describe('happy path', () => { + it('creates plan directory and .planitem, returns expected fields', () => { + makeProject(projectDir); + + const result = server.call('provar.testplan.create', { + project_path: projectDir, + plan_name: 'MyNewPlan', + overwrite: false, + dry_run: false, + }); + + assert.equal(isError(result), false); + const body = parseText(result); + assert.equal(body['created'], true); + assert.equal(body['dry_run'], false); + assert.ok(typeof body['guid'] === 'string' && body['guid'].length > 0, 'Expected guid'); + + const planDir = path.join(projectDir, 'plans', 'MyNewPlan'); + assert.ok(fs.existsSync(planDir), 'Plan directory should be created'); + assert.ok(fs.existsSync(path.join(planDir, '.planitem')), '.planitem should be written'); + + const xml = fs.readFileSync(path.join(planDir, '.planitem'), 'utf-8'); + assert.ok(xml.includes(' { + makeProject(projectDir); + + const result = server.call('provar.testplan.create', { + project_path: projectDir, + plan_name: 'GuidedPlan', + overwrite: false, + dry_run: false, + }); + + const body = parseText(result); + assert.ok(typeof body['next_steps'] === 'string' && body['next_steps'].length > 0, 'Expected next_steps'); + }); + }); + + describe('dry_run', () => { + it('returns created=false and does not write to disk', () => { + makeProject(projectDir); + + const result = server.call('provar.testplan.create', { + project_path: projectDir, + plan_name: 'DryPlan', + overwrite: false, + dry_run: true, + }); + + assert.equal(isError(result), false); + const body = parseText(result); + assert.equal(body['created'], false); + assert.equal(body['dry_run'], true); + assert.ok(typeof body['guid'] === 'string', 'Expected guid even in dry_run'); + + const planDir = path.join(projectDir, 'plans', 'DryPlan'); + assert.equal(fs.existsSync(planDir), false, 'Directory must not be created in dry_run mode'); + }); + }); + + describe('error cases', () => { + it('returns NOT_A_PROJECT when .testproject is missing', () => { + fs.mkdirSync(projectDir, { recursive: true }); + + const result = server.call('provar.testplan.create', { + project_path: projectDir, + plan_name: 'MyPlan', + overwrite: false, + dry_run: false, + }); + + assert.equal(isError(result), true); + assert.equal(errorCode(result), 'NOT_A_PROJECT'); + }); + + it('returns PLAN_EXISTS when .planitem already exists and overwrite=false', () => { + makeProject(projectDir); + makePlan(projectDir, 'ExistingPlan'); + + const result = server.call('provar.testplan.create', { + project_path: projectDir, + plan_name: 'ExistingPlan', + overwrite: false, + dry_run: false, + }); + + assert.equal(isError(result), true); + assert.equal(errorCode(result), 'PLAN_EXISTS'); + }); + + it('overwrites .planitem when overwrite=true and plan already exists', () => { + makeProject(projectDir); + makePlan(projectDir, 'ExistingPlan'); + + const result = server.call('provar.testplan.create', { + project_path: projectDir, + plan_name: 'ExistingPlan', + overwrite: true, + dry_run: false, + }); + + assert.equal(isError(result), false); + assert.equal(parseText(result)['created'], true); + }); + + it('returns PATH_NOT_ALLOWED when project_path is outside allowedPaths', () => { + const strictServer = new MockMcpServer(); + registerAllTestPlanTools(strictServer as never, { allowedPaths: [tmpDir] }); + + const result = strictServer.call('provar.testplan.create', { + project_path: path.join(os.tmpdir(), 'outside-project'), + plan_name: 'MyPlan', + overwrite: false, + dry_run: false, + }); + + assert.equal(isError(result), true); + const code = errorCode(result); + assert.ok(code === 'PATH_NOT_ALLOWED' || code === 'PATH_TRAVERSAL', `Unexpected error code: ${code}`); + }); + }); +}); + // ── provar.testplan.add-instance ─────────────────────────────────────────────── describe('provar.testplan.add-instance', () => { @@ -585,14 +714,20 @@ describe('provar.testplan.remove-instance', () => { // ── registerAllTestPlanTools ─────────────────────────────────────────────────── describe('registerAllTestPlanTools', () => { - it('registers all three tools', () => { + it('registers all four tools', () => { const freshServer = new MockMcpServer(); registerAllTestPlanTools(freshServer as never, config); - // Each tool should be callable without throwing "Tool not registered" makeProject(projectDir); - // Verify each tool is registered by checking it doesn't throw + assert.doesNotThrow(() => { + freshServer.call('provar.testplan.create', { + project_path: projectDir, + plan_name: 'P', + overwrite: false, + dry_run: true, + }); + }); assert.doesNotThrow(() => { freshServer.call('provar.testplan.add-instance', { project_path: projectDir, From d6d4da60a90840d94e95d83f6df6695d755e4e3c Mon Sep 17 00:00:00 2001 From: Michael Dailey Date: Fri, 1 May 2026 12:43:02 -0500 Subject: [PATCH 02/19] PDX-0: feat(validate): add UI-TARGET-001, UI-LOCATOR-001, SETVALUES-STRUCTURE-001, VAR-REF-001 rules Addresses Session 3 feedback D2 (uiTarget class), D3 (SetValues valueList structure), and D4 (variable reference detection). Mirrors quality-hub-agents backend rule IDs where they exist; VAR-REF-001 fills a gap present in both local and backend validation. --- src/mcp/tools/testCaseValidate.ts | 128 ++++++++-- test/unit/mcp/testCaseValidate.test.ts | 341 +++++++++++++++++++++++++ 2 files changed, 451 insertions(+), 18 deletions(-) diff --git a/src/mcp/tools/testCaseValidate.ts b/src/mcp/tools/testCaseValidate.ts index 607c9d3c..f872b0a2 100644 --- a/src/mcp/tools/testCaseValidate.ts +++ b/src/mcp/tools/testCaseValidate.ts @@ -312,9 +312,33 @@ export function validateTestCase(xmlContent: string, testName?: string): TestCas validateApiCall(call, issues); } + // VAR-REF-001 (gap in both local and quality-hub-agents backend): + // Detect {VarName} or {Obj.Field} literals stored as plain string values. + // Provar will pass these as-is to the API rather than resolving them as variable references. + const varLiteralRe = /]+valueClass="string"[^>]*>\{([\w.]+)\}<\/value>/g; + let varMatch: RegExpExecArray | null; + while ((varMatch = varLiteralRe.exec(xmlContent)) !== null) { + issues.push({ + rule_id: 'VAR-REF-001', + severity: 'WARNING', + message: `Argument value "{${varMatch[1]}}" looks like a variable reference but is stored as a plain string — Provar will not resolve it at runtime.`, + applies_to: 'argument', + suggestion: `Replace with . In provar.testcase.generate, use the {VarName} syntax in the attributes object — the generator converts it automatically.`, + }); + } + return finalize(issues, tcId, tcName, apiCalls.length, xmlContent, testName); } +/** Normalise fast-xml-parser's single-or-array representation of children. */ +function getArgList(call: Record): Array> { + const rawArgs = call['arguments'] as Record | undefined; + if (!rawArgs) return []; + const argRaw = rawArgs['argument']; + if (!argRaw) return []; + return (Array.isArray(argRaw) ? argRaw : [argRaw]) as Array>; +} + function validateApiCall(call: Record, issues: ValidationIssue[]): void { const callGuid = call['@_guid'] as string | undefined; const apiId = call['@_apiId'] as string | undefined; @@ -376,31 +400,99 @@ function validateApiCall(call: Record, issues: ValidationIssue[ }); } - // ASSERT-001: AssertValues using UI namedValues format instead of variable format - if (apiId?.includes('AssertValues')) { - const rawArgs = call['arguments'] as Record | undefined; - if (rawArgs) { - const argRaw = rawArgs['argument']; - const argList: Array> = !argRaw - ? [] - : Array.isArray(argRaw) - ? (argRaw as Array>) - : [argRaw as Record]; - const hasValuesArg = argList.some((a) => (a['@_id'] as string | undefined) === 'values'); - if (hasValuesArg) { + if (apiId) validateApiCallArgs(call, apiId, name, issues); +} + +function checkUiTarget(call: Record, apiId: string, stepName: string, issues: ValidationIssue[]): void { + const targetArg = getArgList(call).find((a) => (a['@_id'] as string | undefined) === 'target'); + if (!targetArg) return; + const valClass = (targetArg['value'] as Record | undefined)?.['@_class'] as string | undefined; + if (valClass && valClass !== 'uiTarget') { + const apiLabel = apiId.includes('UiWithRow') ? 'UiWithRow' : 'UiWithScreen'; + issues.push({ + rule_id: 'UI-TARGET-001', + severity: 'ERROR', + message: `${apiLabel} step "${stepName}" target argument uses class="${valClass}" — must be class="uiTarget".`, + applies_to: 'apiCall', + suggestion: + 'Emit the target as: or uri="ui:pageobject:target?pageId=...". ' + + 'In provar.testcase.generate the "target" attribute is converted automatically.', + }); + } +} + +function validateApiCallArgs( + call: Record, + apiId: string, + name: string | undefined, + issues: ValidationIssue[] +): void { + const stepName = name ?? '(unnamed)'; + + // UI-TARGET-001 (mirrors quality-hub-agents UI-SCREEN-TARGET-001): + // UiWithScreen / UiWithRow target argument must use class="uiTarget", not a plain string. + // A plain string causes: "Can not set IUiTargetValue field ... to java.lang.String" + if (apiId.includes('UiWithScreen') || apiId.includes('UiWithRow')) { + checkUiTarget(call, apiId, stepName, issues); + } + + // UI-LOCATOR-001 (local rule, no direct backend equivalent): + // UiDoAction / UiAssert / UiScrollToElement locator argument must use class="uiLocator". + if (apiId.includes('UiDoAction') || apiId.includes('UiAssert') || apiId.includes('UiScrollToElement')) { + const locatorArg = getArgList(call).find((a) => (a['@_id'] as string | undefined) === 'locator'); + if (locatorArg) { + const valClass = (locatorArg['value'] as Record | undefined)?.['@_class'] as string | undefined; + if (valClass && valClass !== 'uiLocator') { + issues.push({ + rule_id: 'UI-LOCATOR-001', + severity: 'ERROR', + message: `"${stepName}" locator argument uses class="${valClass}" — must be class="uiLocator".`, + applies_to: 'apiCall', + suggestion: + 'Emit the locator as: . ' + + 'In provar.testcase.generate the "locator" attribute is converted automatically.', + }); + } + } + } + + // SETVALUES-STRUCTURE-001 (mirrors quality-hub-agents SETVALUES-STRUCTURE-001): + // SetValues values argument must use class="valueList" with children. + // A plain string value causes an immediate ClassCastException at runtime. + if (apiId.includes('SetValues') && !apiId.includes('AssertValues')) { + const valuesArg = getArgList(call).find((a) => (a['@_id'] as string | undefined) === 'values'); + if (valuesArg) { + const valClass = (valuesArg['value'] as Record | undefined)?.['@_class'] as string | undefined; + if (valClass && valClass !== 'valueList') { issues.push({ - rule_id: 'ASSERT-001', - severity: 'WARNING', - message: `AssertValues step "${ - name ?? '(unnamed)' - }" uses namedValues format (argument id="values") — designed for UI element attribute assertions. For Apex/SOQL result or variable comparisons this silently passes as null=null.`, + rule_id: 'SETVALUES-STRUCTURE-001', + severity: 'ERROR', + message: `SetValues step "${stepName}" values argument uses class="${valClass}" — must use class="valueList" with children.`, applies_to: 'apiCall', suggestion: - 'Use separate expectedValue, actualValue, and comparisonType arguments for variable or Apex result comparisons.', + 'Wrap variable assignments in: ' + + 'value' + + '. In provar.testcase.generate pass each variable as a flat key/value pair ' + + 'in attributes — the generator builds the valueList structure automatically.', }); } } } + + // ASSERT-001: AssertValues using UI namedValues format instead of variable format + if (apiId.includes('AssertValues')) { + const hasValuesArg = getArgList(call).some((a) => (a['@_id'] as string | undefined) === 'values'); + if (hasValuesArg) { + issues.push({ + rule_id: 'ASSERT-001', + severity: 'WARNING', + message: `AssertValues step "${stepName}" uses namedValues format (argument id="values") — designed for UI element attribute assertions. For Apex/SOQL result or variable comparisons this silently passes as null=null.`, + applies_to: 'apiCall', + suggestion: + 'Use separate expectedValue, actualValue, and comparisonType arguments for variable or Apex result comparisons.', + }); + } + } } function finalize( diff --git a/test/unit/mcp/testCaseValidate.test.ts b/test/unit/mcp/testCaseValidate.test.ts index 628ce592..456fc09b 100644 --- a/test/unit/mcp/testCaseValidate.test.ts +++ b/test/unit/mcp/testCaseValidate.test.ts @@ -293,6 +293,347 @@ describe('validateTestCase', () => { assert.ok(!r.issues.some((i) => i.rule_id === 'ASSERT-001'), 'ASSERT-001 should not fire for SetValues'); }); }); + + describe('UI-TARGET-001', () => { + it('errors when UiWithScreen target argument uses class="value" (plain string)', () => { + const r = validateTestCase( + ` + + + + + + sf:ui:target?object=Account + + + + +` + ); + assert.ok( + r.issues.some((i) => i.rule_id === 'UI-TARGET-001'), + 'Expected UI-TARGET-001' + ); + const issue = r.issues.find((i) => i.rule_id === 'UI-TARGET-001')!; + assert.equal(issue.severity, 'ERROR'); + assert.ok(issue.message.includes('uiTarget'), `Message should mention uiTarget: ${issue.message}`); + }); + + it('does not fire when UiWithScreen target uses class="uiTarget"', () => { + const r = validateTestCase( + ` + + + + + + + + + + +` + ); + assert.ok( + !r.issues.some((i) => i.rule_id === 'UI-TARGET-001'), + 'UI-TARGET-001 should not fire for correct uiTarget class' + ); + }); + + it('does not fire when UiWithScreen has no target argument', () => { + const r = validateTestCase( + ` + + + + + 1280 + + + +` + ); + assert.ok( + !r.issues.some((i) => i.rule_id === 'UI-TARGET-001'), + 'UI-TARGET-001 should not fire when no target argument present' + ); + }); + + it('also fires for UiWithRow steps with wrong target class', () => { + const r = validateTestCase( + ` + + + + + + sf:ui:target?object=Account + + + + +` + ); + assert.ok( + r.issues.some((i) => i.rule_id === 'UI-TARGET-001'), + 'Expected UI-TARGET-001 for UiWithRow' + ); + }); + }); + + describe('UI-LOCATOR-001', () => { + it('errors when UiDoAction locator argument uses class="value" (plain string)', () => { + const r = validateTestCase( + ` + + + + + + sf:ui:locator:label?label=Save + + + + +` + ); + assert.ok( + r.issues.some((i) => i.rule_id === 'UI-LOCATOR-001'), + 'Expected UI-LOCATOR-001' + ); + const issue = r.issues.find((i) => i.rule_id === 'UI-LOCATOR-001')!; + assert.equal(issue.severity, 'ERROR'); + assert.ok(issue.message.includes('uiLocator'), `Message should mention uiLocator: ${issue.message}`); + }); + + it('does not fire when UiDoAction locator uses class="uiLocator"', () => { + const r = validateTestCase( + ` + + + + + + + + + + +` + ); + assert.ok( + !r.issues.some((i) => i.rule_id === 'UI-LOCATOR-001'), + 'UI-LOCATOR-001 should not fire for correct uiLocator class' + ); + }); + + it('also fires for UiAssert steps with wrong locator class', () => { + const r = validateTestCase( + ` + + + + + + sf:ui:locator:label?label=Name + + + + +` + ); + assert.ok( + r.issues.some((i) => i.rule_id === 'UI-LOCATOR-001'), + 'Expected UI-LOCATOR-001 for UiAssert' + ); + }); + }); + + describe('SETVALUES-STRUCTURE-001', () => { + it('errors when SetValues values argument uses class="value" (plain string)', () => { + const r = validateTestCase( + ` + + + + + + myVar=hello + + + + +` + ); + assert.ok( + r.issues.some((i) => i.rule_id === 'SETVALUES-STRUCTURE-001'), + 'Expected SETVALUES-STRUCTURE-001' + ); + const issue = r.issues.find((i) => i.rule_id === 'SETVALUES-STRUCTURE-001')!; + assert.equal(issue.severity, 'ERROR'); + assert.ok(issue.message.includes('valueList'), `Message should mention valueList: ${issue.message}`); + }); + + it('does not fire when SetValues values argument uses class="valueList"', () => { + const r = validateTestCase( + ` + + + + + + + + + hello + + + + + + + +` + ); + assert.ok( + !r.issues.some((i) => i.rule_id === 'SETVALUES-STRUCTURE-001'), + 'SETVALUES-STRUCTURE-001 should not fire for correct valueList structure' + ); + }); + + it('does not fire when SetValues has no values argument (self-closing)', () => { + const r = validateTestCase( + ` + + + + + + +` + ); + assert.ok( + !r.issues.some((i) => i.rule_id === 'SETVALUES-STRUCTURE-001'), + 'SETVALUES-STRUCTURE-001 should not fire when no values argument present' + ); + }); + + it('does not fire for AssertValues (only targets SetValues)', () => { + const r = validateTestCase( + ` + + + + + + myVar=hello + + + + +` + ); + assert.ok( + !r.issues.some((i) => i.rule_id === 'SETVALUES-STRUCTURE-001'), + 'SETVALUES-STRUCTURE-001 should not fire for AssertValues' + ); + }); + }); + + describe('VAR-REF-001', () => { + it('warns when a plain string value contains a {VarName} pattern', () => { + const r = validateTestCase( + ` + + + + + + {AccountId} + + + + +` + ); + assert.ok( + r.issues.some((i) => i.rule_id === 'VAR-REF-001'), + 'Expected VAR-REF-001' + ); + const issue = r.issues.find((i) => i.rule_id === 'VAR-REF-001')!; + assert.equal(issue.severity, 'WARNING'); + assert.ok(issue.message.includes('{AccountId}'), `Message should include variable name: ${issue.message}`); + }); + + it('warns for dotted path {Obj.Field} stored as plain string', () => { + const r = validateTestCase( + ` + + + + + + {Record.Name} + + + + +` + ); + assert.ok( + r.issues.some((i) => i.rule_id === 'VAR-REF-001'), + 'Expected VAR-REF-001 for dotted path' + ); + assert.ok( + r.issues.find((i) => i.rule_id === 'VAR-REF-001')!.message.includes('{Record.Name}'), + 'Message should include dotted variable name' + ); + }); + + it('does not fire when value uses class="variable" (correct structure)', () => { + const r = validateTestCase( + ` + + + + + + + + + + + + +` + ); + assert.ok( + !r.issues.some((i) => i.rule_id === 'VAR-REF-001'), + 'VAR-REF-001 should not fire for correct class="variable" structure' + ); + }); + + it('does not fire for plain string content without curly braces', () => { + const r = validateTestCase( + ` + + + + + + John Smith + + + + +` + ); + assert.ok( + !r.issues.some((i) => i.rule_id === 'VAR-REF-001'), + 'VAR-REF-001 should not fire for plain string without curly braces' + ); + }); + }); }); // ── Handler-level tests (registerTestCaseValidate) ──────────────────────────── From 849c6b979508d63582a38923d985989a879c588b Mon Sep 17 00:00:00 2001 From: Michael Dailey Date: Fri, 1 May 2026 14:10:57 -0500 Subject: [PATCH 03/19] =?UTF-8?q?PDX-0:=20fix:=20pre-landing=20review=20fi?= =?UTF-8?q?xes=20(D1=E2=80=93D3)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit D1: add assertPathAllowed(planDir) in testPlanTools create-plan path D2: skip uiTarget/uiLocator dispatch inside SetValues namedValues context D3: remove AssertValues from SET_VALUES_APIS; update test to expect flat args Co-Authored-By: Claude Sonnet 4.6 --- src/mcp/tools/testCaseGenerate.ts | 28 ++++++++++++++------------ src/mcp/tools/testPlanTools.ts | 1 + test/unit/mcp/testCaseGenerate.test.ts | 7 ++++--- 3 files changed, 20 insertions(+), 16 deletions(-) diff --git a/src/mcp/tools/testCaseGenerate.ts b/src/mcp/tools/testCaseGenerate.ts index 3badee23..e4fc75c2 100644 --- a/src/mcp/tools/testCaseGenerate.ts +++ b/src/mcp/tools/testCaseGenerate.ts @@ -266,12 +266,12 @@ export function registerTestCaseGenerate(server: McpServer, config: ServerConfig // APIs whose 'values' argument must use class="valueList"/ (D3). const SET_VALUES_APIS = new Set([ - SHORTHAND_TO_FQID['SetValues'], // com.provar.plugins.bundled.apis.control.SetValues - SHORTHAND_TO_FQID['AssertValues'], // com.provar.plugins.bundled.apis.AssertValues + SHORTHAND_TO_FQID['SetValues'] ?? '', // com.provar.plugins.bundled.apis.control.SetValues ]); // Build the element for a single argument (D2/D4 aware). -function buildArgumentValue(key: string, val: string, indent: string): string { +// inNamedValues: when true (inside SetValues namedValues), skip uiTarget/uiLocator dispatch. +function buildArgumentValue(key: string, val: string, indent: string, inNamedValues = false): string { // D4: {VarName} or {Obj.Field} → class="variable" with elements. const varMatch = /^\{([\w.]+)\}$/.exec(val); if (varMatch) { @@ -281,13 +281,15 @@ function buildArgumentValue(key: string, val: string, indent: string): string { .join('\n'); return `${indent}\n${pathElements}\n${indent}`; } - // D2: 'target' argument (UiWithScreen / UiWithRow) → class="uiTarget". - if (key === 'target') { - return `${indent}`; - } - // D2: 'locator' argument (UiDoAction / UiAssert) → class="uiLocator". - if (key === 'locator') { - return `${indent}`; + if (!inNamedValues) { + // D2: 'target' argument (UiWithScreen / UiWithRow) → class="uiTarget". + if (key === 'target') { + return `${indent}`; + } + // D2: 'locator' argument (UiDoAction / UiAssert) → class="uiLocator". + if (key === 'locator') { + return `${indent}`; + } } return `${indent}${escapeXmlContent(val)}`; } @@ -308,14 +310,14 @@ function buildArgumentsXml(attributes: Record, baseIndent = ' return `\n${baseIndent}\n${argLines}\n${baseIndent}\n${baseIndent.slice(0, -2)}`; } -// D3: SetValues / AssertValues — all attributes become under a single 'values' argument. +// D3: SetValues — all attributes become under a single 'values' argument. function buildSetValuesXml(attributes: Record, baseIndent: string): string { const entries = Object.entries(attributes); if (entries.length === 0) return ''; const i = (n: number): string => baseIndent + ' '.repeat(n); const namedValueLines = entries .map(([name, val]) => { - const valueXml = buildArgumentValue(name, val, `${i(3)} `); + const valueXml = buildArgumentValue(name, val, `${i(3)} `, true); return `${i(3)}\n${valueXml}\n${i(3)}`; }) .join('\n'); @@ -328,7 +330,7 @@ function buildSetValuesXml(attributes: Record, baseIndent: strin `${i(2)}\n` + `${i(1)}\n` + `${i(0)}\n` + - `${baseIndent.slice(0, -2)}\n` + + `${i(0)}\n` + `${baseIndent.slice(0, -2)}` ); } diff --git a/src/mcp/tools/testPlanTools.ts b/src/mcp/tools/testPlanTools.ts index ac16cd6d..d1a6519c 100644 --- a/src/mcp/tools/testPlanTools.ts +++ b/src/mcp/tools/testPlanTools.ts @@ -106,6 +106,7 @@ export function registerTestPlanCreate(server: McpServer, config: ServerConfig): } const planDir = path.join(projectRoot, 'plans', plan_name); + assertPathAllowed(planDir, config.allowedPaths); const planItemPath = path.join(planDir, '.planitem'); if (!overwrite && fs.existsSync(planItemPath)) { diff --git a/test/unit/mcp/testCaseGenerate.test.ts b/test/unit/mcp/testCaseGenerate.test.ts index da31040f..e3729beb 100644 --- a/test/unit/mcp/testCaseGenerate.test.ts +++ b/test/unit/mcp/testCaseGenerate.test.ts @@ -548,7 +548,7 @@ describe('provar.testcase.generate', () => { ); }); - it('AssertValues also emits valueList/namedValues structure', () => { + it('AssertValues uses flat argument structure (not valueList)', () => { const result = server.call('provar.testcase.generate', { test_case_name: 'AssertValues Test', steps: [ @@ -564,8 +564,9 @@ describe('provar.testcase.generate', () => { }); const xml = parseText(result)['xml_content'] as string; - assert.ok(xml.includes('class="valueList"'), 'Expected valueList for AssertValues'); - assert.ok(xml.includes(''), 'Expected namedValue for opportunityName'); + assert.ok(xml.includes(''), 'Expected flat argument id for AssertValues'); + assert.ok(!xml.includes('class="valueList"'), 'AssertValues must NOT emit valueList structure'); + assert.ok(!xml.includes(' { From c6ba663b0aa9a813449ec2796c2428f65ae20b7d Mon Sep 17 00:00:00 2001 From: Michael Dailey Date: Fri, 1 May 2026 14:12:07 -0500 Subject: [PATCH 04/19] PDX-0: chore: bump to 1.5.0-beta.12; update docs for Wave 3 validator rules Add docs for UI-TARGET-001, UI-LOCATOR-001, SETVALUES-STRUCTURE-001, VAR-REF-001 Document generator XML argument conventions (uiTarget/uiLocator/variable/valueList) Co-Authored-By: Claude Sonnet 4.6 --- docs/mcp.md | 16 ++++++++++++++++ package.json | 2 +- server.json | 4 ++-- 3 files changed, 19 insertions(+), 3 deletions(-) diff --git a/docs/mcp.md b/docs/mcp.md index fa3dce8a..3366c616 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -637,6 +637,18 @@ Generates an XML test case skeleton with UUID v4 guids and sequential `testItemI - Omit or use a `sf:` URI → flat Salesforce step structure (existing behaviour) - `ui:pageobject:target?pageId=pageobjects.Page` → wraps all steps in a `UiWithScreen` element (testItemId=1); substeps clause at testItemId=2; inner steps start at testItemId=3 +**Argument XML conventions** (automatically applied by the generator): + +| Argument key / value pattern | Emitted XML class | API context | +| ------------------------------------ | ----------------------------- | ----------------------------------- | +| `target` key | `class="uiTarget"` | UiWithScreen, UiWithRow | +| `locator` key | `class="uiLocator"` | UiDoAction, UiAssert | +| Value matches `{VarName}` or `{A.B}` | `class="variable"` + `` | Any step | +| SetValues attributes | `class="valueList"/` | SetValues only | +| All other values | `class="value" valueClass="string"` | Any step | + +AssertValues uses **flat** argument structure (`expectedValue`, `actualValue`, `comparisonType`) — not the `valueList`/namedValues format. + **Input** | Parameter | Type | Required | Description | @@ -700,6 +712,10 @@ Validates an XML test case for schema correctness (validity score) and best prac - **DATA-001** — `testCase` declares a `` element. CLI standalone execution does not bind CSV column variables; steps using variable references will resolve to null. Use `SetValues` (Test scope) steps instead, or add the test to a test plan. - **ASSERT-001** — An `AssertValues` step uses the `argument id="values"` (namedValues) format, which is designed for UI element attribute assertions. For Apex/SOQL result or variable comparisons this silently passes as `null=null`. Use separate `expectedValue`, `actualValue`, and `comparisonType` arguments instead. +- **UI-TARGET-001** — A UiWithScreen or UiWithRow `target` argument uses the wrong XML class (e.g. `class="value"`). Must be `class="uiTarget"` or the screen binding is silently ignored at runtime. +- **UI-LOCATOR-001** — A UiDoAction or UiAssert `locator` argument uses the wrong XML class. Must be `class="uiLocator"` or Provar cannot resolve the element. +- **SETVALUES-STRUCTURE-001** (ERROR) — A `SetValues` step's `values` argument uses `class="value"` (plain string) instead of `class="valueList"` with `` children. This causes an immediate `ClassCastException` at runtime. +- **VAR-REF-001** — An argument value looks like a variable reference (`{VarName}` or `{Obj.Field}`) but is stored as `class="value" valueClass="string"`. Provar will treat it as a literal string, not resolve the variable. Replace with `class="variable"` and `` elements. --- diff --git a/package.json b/package.json index 8c47f578..8f459891 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "@provartesting/provardx-cli", "description": "A plugin for the Salesforce CLI to orchestrate testing activities and report quality metrics to Provar Quality Hub", - "version": "1.5.0-beta.11", + "version": "1.5.0-beta.12", "mcpName": "io.github.ProvarTesting/provar", "license": "BSD-3-Clause", "plugins": [ diff --git a/server.json b/server.json index 781cc2a9..c23758f0 100644 --- a/server.json +++ b/server.json @@ -14,12 +14,12 @@ "url": "https://github.com/ProvarTesting/provardx-cli", "source": "github" }, - "version": "1.5.0-beta.11", + "version": "1.5.0-beta.12", "packages": [ { "registryType": "npm", "identifier": "@provartesting/provardx-cli", - "version": "1.5.0-beta.11", + "version": "1.5.0-beta.12", "transport": { "type": "stdio" }, From 7f7ea1cf035d5c4e87fc525c56ceb0e025f1be38 Mon Sep 17 00:00:00 2001 From: Michael Dailey Date: Fri, 1 May 2026 14:12:52 -0500 Subject: [PATCH 05/19] PDX-0: chore: fix duplicate comment numbers in mcp-smoke.cjs Co-Authored-By: Claude Sonnet 4.6 --- scripts/mcp-smoke.cjs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/scripts/mcp-smoke.cjs b/scripts/mcp-smoke.cjs index 68e52c53..39bdf46f 100644 --- a/scripts/mcp-smoke.cjs +++ b/scripts/mcp-smoke.cjs @@ -255,7 +255,7 @@ async function runTests() { plan_name: 'SmokePlan', }); - // ── 33. provar.testplan.add-instance ───────────────────────────────────── + // ── 32. provar.testplan.add-instance ───────────────────────────────────── // TMP is not a Provar project → NOT_A_PROJECT result await callTool('provar.testplan.add-instance', { project_path: TMP, @@ -263,24 +263,24 @@ async function runTests() { plan_name: 'SmokePlan', }); - // ── 34. provar.testplan.create-suite ───────────────────────────────────── + // ── 33. provar.testplan.create-suite ───────────────────────────────────── await callTool('provar.testplan.create-suite', { project_path: TMP, plan_name: 'SmokePlan', suite_name: 'SmokeSuite', }); - // ── 35. provar.testplan.remove-instance ────────────────────────────────── + // ── 34. provar.testplan.remove-instance ────────────────────────────────── await callTool('provar.testplan.remove-instance', { project_path: TMP, instance_path: 'plans/SmokePlan/SmokeSuite/smoke.testinstance', }); - // ── 34. provar.nitrox.discover ──────────────────────────────────────────── + // ── 35. provar.nitrox.discover ──────────────────────────────────────────── // TMP has no .testproject → empty projects list, no crash await callTool('provar.nitrox.discover', { search_roots: [TMP] }); - // ── 35. provar.nitrox.validate ──────────────────────────────────────────── + // ── 36. provar.nitrox.validate ──────────────────────────────────────────── // Minimal valid root component → score 100 await callTool('provar.nitrox.validate', { content: JSON.stringify({ @@ -383,7 +383,7 @@ async function runTests() { // ---------------------------------------------------------------------------- server.on('close', () => { clearTimeout(overallTimer); - // initialize + tools/list + 39 tools + prompts/list + 8 prompts/get (setup excluded from default count) + // initialize + tools/list + 40 tools + prompts/list + 8 prompts/get (setup excluded from default count) const TOTAL_EXPECTED = 51 + (INCLUDE_SETUP ? 1 : 0); let passed = 0; let failed = 0; From c9fa3fe72b3f490024123882150a741b81b0fc28 Mon Sep 17 00:00:00 2001 From: Michael Dailey Date: Fri, 1 May 2026 14:57:56 -0500 Subject: [PATCH 06/19] PDX-0: fix: address Copilot PR review comments - Separate SetValues/AssertValues in StepSchema and TOOL_DESCRIPTION - Replace SET_VALUES_APIS Set with resolvedApiId.includes('SetValues') (mirrors validator) - Make target/locator dispatch API-aware (pass resolvedApiId through buildArgumentsXml) - Fix buildUiWithScreenXml to pass wrapperApiId so target emits uiTarget class - Validate plan_name has no path separators to prevent path.join injection - Add UiScrollToElement to UI-LOCATOR-001 docs and argument convention table Co-Authored-By: Claude Sonnet 4.6 --- docs/mcp.md | 4 +-- src/mcp/tools/testCaseGenerate.ts | 45 ++++++++++++++++--------------- src/mcp/tools/testPlanTools.ts | 11 ++++++++ 3 files changed, 37 insertions(+), 23 deletions(-) diff --git a/docs/mcp.md b/docs/mcp.md index 3366c616..5056eeb3 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -642,7 +642,7 @@ Generates an XML test case skeleton with UUID v4 guids and sequential `testItemI | Argument key / value pattern | Emitted XML class | API context | | ------------------------------------ | ----------------------------- | ----------------------------------- | | `target` key | `class="uiTarget"` | UiWithScreen, UiWithRow | -| `locator` key | `class="uiLocator"` | UiDoAction, UiAssert | +| `locator` key | `class="uiLocator"` | UiDoAction, UiAssert, UiScrollToElement | | Value matches `{VarName}` or `{A.B}` | `class="variable"` + `` | Any step | | SetValues attributes | `class="valueList"/` | SetValues only | | All other values | `class="value" valueClass="string"` | Any step | @@ -713,7 +713,7 @@ Validates an XML test case for schema correctness (validity score) and best prac - **DATA-001** — `testCase` declares a `` element. CLI standalone execution does not bind CSV column variables; steps using variable references will resolve to null. Use `SetValues` (Test scope) steps instead, or add the test to a test plan. - **ASSERT-001** — An `AssertValues` step uses the `argument id="values"` (namedValues) format, which is designed for UI element attribute assertions. For Apex/SOQL result or variable comparisons this silently passes as `null=null`. Use separate `expectedValue`, `actualValue`, and `comparisonType` arguments instead. - **UI-TARGET-001** — A UiWithScreen or UiWithRow `target` argument uses the wrong XML class (e.g. `class="value"`). Must be `class="uiTarget"` or the screen binding is silently ignored at runtime. -- **UI-LOCATOR-001** — A UiDoAction or UiAssert `locator` argument uses the wrong XML class. Must be `class="uiLocator"` or Provar cannot resolve the element. +- **UI-LOCATOR-001** — A UiDoAction, UiAssert, or UiScrollToElement `locator` argument uses the wrong XML class. Must be `class="uiLocator"` or Provar cannot resolve the element. - **SETVALUES-STRUCTURE-001** (ERROR) — A `SetValues` step's `values` argument uses `class="value"` (plain string) instead of `class="valueList"` with `` children. This causes an immediate `ClassCastException` at runtime. - **VAR-REF-001** — An argument value looks like a variable reference (`{VarName}` or `{Obj.Field}`) but is stored as `class="value" valueClass="string"`. Provar will treat it as a literal string, not resolve the variable. Replace with `class="variable"` and `` elements. diff --git a/src/mcp/tools/testCaseGenerate.ts b/src/mcp/tools/testCaseGenerate.ts index e4fc75c2..dd9c256b 100644 --- a/src/mcp/tools/testCaseGenerate.ts +++ b/src/mcp/tools/testCaseGenerate.ts @@ -105,12 +105,13 @@ const StepSchema = z.object({ 'Special value conventions (applied automatically by the generator): ' + '(1) Variable references: wrap the name in braces, e.g. "{MyVar}" → emitted as class="variable" . ' + ' Dotted paths are also supported: "{Obj.Field}" → two elements. ' + - '(2) SetValues / AssertValues: pass each variable name and its value as a flat key/value pair; ' + + '(2) SetValues: pass each variable name and its value as a flat key/value pair; ' + ' the generator wraps them in ... automatically. ' + - ' Example for SetValues: { "testCaseName": "TC_New", "testType": "Acceptance testing" } ' + - '(3) target argument (UiWithScreen / UiWithRow): pass the sf:ui:target or ui:pageobject:target URI; ' + + ' Example: { "testCaseName": "TC_New", "testType": "Acceptance testing" } ' + + '(3) AssertValues: pass assertion arguments as flat key/value pairs; emitted as flat elements, NOT wrapped in valueList/namedValues. ' + + '(4) target argument (UiWithScreen / UiWithRow): pass the sf:ui:target or ui:pageobject:target URI; ' + ' emitted as class="uiTarget" uri="...". ' + - '(4) locator argument (UiDoAction / UiAssert): pass the locator URI; emitted as class="uiLocator" uri="...". ' + + '(5) locator argument (UiDoAction / UiAssert / UiScrollToElement): pass the locator URI; emitted as class="uiLocator" uri="...". ' + 'All other string values use class="value" valueClass="string".' ), }); @@ -130,11 +131,13 @@ const TOOL_DESCRIPTION = [ 'ApexReadObject requires field names in attributes; omitting them produces MALFORMED_QUERY. Prefer ApexSoqlQuery.', 'AssertValues on SOQL results: index paths like "ResultList[0].Field" are not supported.', 'Use ForEach to iterate the result list, or SetValues to extract a field into a variable first.', - 'SetValues / AssertValues: pass named variable values as flat key/value pairs in attributes; ' + + 'SetValues: pass named variable values as flat key/value pairs in attributes; ' + 'the generator wraps them in ... automatically.', + 'AssertValues: pass assertion values as flat key/value argument pairs; emitted as flat arguments, NOT wrapped in namedValues. ' + + 'If AssertValues uses namedValues-shaped content, validation reports warning ASSERT-001.', 'Variable references: pass values as "{VarName}" (braces); emitted as class="variable" .', 'target argument (UiWithScreen/UiWithRow): pass the URI value; emitted as class="uiTarget" uri="...".', - 'locator argument (UiDoAction/UiAssert): pass the URI value; emitted as class="uiLocator" uri="...".', + 'locator argument (UiDoAction/UiAssert/UiScrollToElement): pass the URI value; emitted as class="uiLocator" uri="...".', 'Cleanup warning: ApexDeleteObject steps near end of test will be skipped if an earlier step fails (stopOnError=false). Use a TearDown callable.', 'Validation: when validate_after_edit=true (default) the response includes a validation field and returns TESTCASE_INVALID if the generated XML fails structural checks.', 'Grounding: call provar.qualityhub.examples.retrieve before generating to get corpus examples for the scenario — correct XML structure for the step types you need.', @@ -264,14 +267,10 @@ export function registerTestCaseGenerate(server: McpServer, config: ServerConfig // ── XML builder ─────────────────────────────────────────────────────────────── -// APIs whose 'values' argument must use class="valueList"/ (D3). -const SET_VALUES_APIS = new Set([ - SHORTHAND_TO_FQID['SetValues'] ?? '', // com.provar.plugins.bundled.apis.control.SetValues -]); - // Build the element for a single argument (D2/D4 aware). // inNamedValues: when true (inside SetValues namedValues), skip uiTarget/uiLocator dispatch. -function buildArgumentValue(key: string, val: string, indent: string, inNamedValues = false): string { +// apiId: resolved API ID used to restrict key-name dispatch to the correct UI APIs. +function buildArgumentValue(key: string, val: string, indent: string, inNamedValues = false, apiId = ''): string { // D4: {VarName} or {Obj.Field} → class="variable" with elements. const varMatch = /^\{([\w.]+)\}$/.exec(val); if (varMatch) { @@ -282,24 +281,27 @@ function buildArgumentValue(key: string, val: string, indent: string, inNamedVal return `${indent}\n${pathElements}\n${indent}`; } if (!inNamedValues) { - // D2: 'target' argument (UiWithScreen / UiWithRow) → class="uiTarget". - if (key === 'target') { + // D2: 'target' argument → class="uiTarget" (only for UiWithScreen / UiWithRow). + if (key === 'target' && (apiId.includes('UiWithScreen') || apiId.includes('UiWithRow'))) { return `${indent}`; } - // D2: 'locator' argument (UiDoAction / UiAssert) → class="uiLocator". - if (key === 'locator') { + // D2: 'locator' argument → class="uiLocator" (only for UiDoAction / UiAssert / UiScrollToElement). + if ( + key === 'locator' && + (apiId.includes('UiDoAction') || apiId.includes('UiAssert') || apiId.includes('UiScrollToElement')) + ) { return `${indent}`; } } return `${indent}${escapeXmlContent(val)}`; } -function buildArgumentsXml(attributes: Record, baseIndent = ' '): string { +function buildArgumentsXml(attributes: Record, baseIndent = ' ', apiId = ''): string { const entries = Object.entries(attributes); if (entries.length === 0) return ''; const argLines = entries .map(([k, v]) => { - const valueXml = buildArgumentValue(k, v, `${baseIndent} `); + const valueXml = buildArgumentValue(k, v, `${baseIndent} `, false, apiId); return ( `${baseIndent}\n` + valueXml + '\n' + @@ -343,9 +345,10 @@ function buildFlatStepXml( const guid = randomUUID(); const resolvedApiId = resolveApiId(step.api_id); const baseIndent = indent + ' '; - const argumentsXml = SET_VALUES_APIS.has(resolvedApiId) + // Use SetValues structure for any SetValues API (string-match mirrors the validator). + const argumentsXml = resolvedApiId.includes('SetValues') ? buildSetValuesXml(step.attributes, baseIndent) - : buildArgumentsXml(step.attributes, baseIndent); + : buildArgumentsXml(step.attributes, baseIndent, resolvedApiId); if (argumentsXml) { return ( `${indent}\n '; return ( ` ${buildArgumentsXml({ target: targetUri }).trimEnd()}${clausesXml}` + ` name="With page" testItemId="1">${buildArgumentsXml({ target: targetUri }, ' ', wrapperApiId).trimEnd()}${clausesXml}` ); } diff --git a/src/mcp/tools/testPlanTools.ts b/src/mcp/tools/testPlanTools.ts index d1a6519c..716c4022 100644 --- a/src/mcp/tools/testPlanTools.ts +++ b/src/mcp/tools/testPlanTools.ts @@ -105,6 +105,17 @@ export function registerTestPlanCreate(server: McpServer, config: ServerConfig): }; } + if (plan_name.includes('/') || plan_name.includes('\\') || path.isAbsolute(plan_name)) { + return { + isError: true, + content: [ + { + type: 'text' as const, + text: JSON.stringify(makeError('INVALID_PLAN_NAME', `plan_name must be a simple directory name without path separators: "${plan_name}"`, requestId)), + }, + ], + }; + } const planDir = path.join(projectRoot, 'plans', plan_name); assertPathAllowed(planDir, config.allowedPaths); const planItemPath = path.join(planDir, '.planitem'); From 7d148f3c504e400a006937d790ba19b7e13d8867 Mon Sep 17 00:00:00 2001 From: Michael Dailey Date: Fri, 1 May 2026 15:32:59 -0500 Subject: [PATCH 07/19] PDX-0: fix: remove UiScrollToElement from UI-LOCATOR-001 (no corpus evidence) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit UiScrollToElement has zero usage in the internal test corpus and no documented locator argument. The inclusion was speculative — reverting to UiDoAction/UiAssert only. Co-Authored-By: Claude Sonnet 4.6 --- docs/mcp.md | 4 ++-- src/mcp/tools/testCaseGenerate.ts | 11 ++++------- src/mcp/tools/testCaseValidate.ts | 4 ++-- 3 files changed, 8 insertions(+), 11 deletions(-) diff --git a/docs/mcp.md b/docs/mcp.md index 5056eeb3..3366c616 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -642,7 +642,7 @@ Generates an XML test case skeleton with UUID v4 guids and sequential `testItemI | Argument key / value pattern | Emitted XML class | API context | | ------------------------------------ | ----------------------------- | ----------------------------------- | | `target` key | `class="uiTarget"` | UiWithScreen, UiWithRow | -| `locator` key | `class="uiLocator"` | UiDoAction, UiAssert, UiScrollToElement | +| `locator` key | `class="uiLocator"` | UiDoAction, UiAssert | | Value matches `{VarName}` or `{A.B}` | `class="variable"` + `` | Any step | | SetValues attributes | `class="valueList"/` | SetValues only | | All other values | `class="value" valueClass="string"` | Any step | @@ -713,7 +713,7 @@ Validates an XML test case for schema correctness (validity score) and best prac - **DATA-001** — `testCase` declares a `` element. CLI standalone execution does not bind CSV column variables; steps using variable references will resolve to null. Use `SetValues` (Test scope) steps instead, or add the test to a test plan. - **ASSERT-001** — An `AssertValues` step uses the `argument id="values"` (namedValues) format, which is designed for UI element attribute assertions. For Apex/SOQL result or variable comparisons this silently passes as `null=null`. Use separate `expectedValue`, `actualValue`, and `comparisonType` arguments instead. - **UI-TARGET-001** — A UiWithScreen or UiWithRow `target` argument uses the wrong XML class (e.g. `class="value"`). Must be `class="uiTarget"` or the screen binding is silently ignored at runtime. -- **UI-LOCATOR-001** — A UiDoAction, UiAssert, or UiScrollToElement `locator` argument uses the wrong XML class. Must be `class="uiLocator"` or Provar cannot resolve the element. +- **UI-LOCATOR-001** — A UiDoAction or UiAssert `locator` argument uses the wrong XML class. Must be `class="uiLocator"` or Provar cannot resolve the element. - **SETVALUES-STRUCTURE-001** (ERROR) — A `SetValues` step's `values` argument uses `class="value"` (plain string) instead of `class="valueList"` with `` children. This causes an immediate `ClassCastException` at runtime. - **VAR-REF-001** — An argument value looks like a variable reference (`{VarName}` or `{Obj.Field}`) but is stored as `class="value" valueClass="string"`. Provar will treat it as a literal string, not resolve the variable. Replace with `class="variable"` and `` elements. diff --git a/src/mcp/tools/testCaseGenerate.ts b/src/mcp/tools/testCaseGenerate.ts index dd9c256b..d7dff526 100644 --- a/src/mcp/tools/testCaseGenerate.ts +++ b/src/mcp/tools/testCaseGenerate.ts @@ -111,7 +111,7 @@ const StepSchema = z.object({ '(3) AssertValues: pass assertion arguments as flat key/value pairs; emitted as flat elements, NOT wrapped in valueList/namedValues. ' + '(4) target argument (UiWithScreen / UiWithRow): pass the sf:ui:target or ui:pageobject:target URI; ' + ' emitted as class="uiTarget" uri="...". ' + - '(5) locator argument (UiDoAction / UiAssert / UiScrollToElement): pass the locator URI; emitted as class="uiLocator" uri="...". ' + + '(5) locator argument (UiDoAction / UiAssert): pass the locator URI; emitted as class="uiLocator" uri="...". ' + 'All other string values use class="value" valueClass="string".' ), }); @@ -137,7 +137,7 @@ const TOOL_DESCRIPTION = [ 'If AssertValues uses namedValues-shaped content, validation reports warning ASSERT-001.', 'Variable references: pass values as "{VarName}" (braces); emitted as class="variable" .', 'target argument (UiWithScreen/UiWithRow): pass the URI value; emitted as class="uiTarget" uri="...".', - 'locator argument (UiDoAction/UiAssert/UiScrollToElement): pass the URI value; emitted as class="uiLocator" uri="...".', + 'locator argument (UiDoAction/UiAssert): pass the URI value; emitted as class="uiLocator" uri="...".', 'Cleanup warning: ApexDeleteObject steps near end of test will be skipped if an earlier step fails (stopOnError=false). Use a TearDown callable.', 'Validation: when validate_after_edit=true (default) the response includes a validation field and returns TESTCASE_INVALID if the generated XML fails structural checks.', 'Grounding: call provar.qualityhub.examples.retrieve before generating to get corpus examples for the scenario — correct XML structure for the step types you need.', @@ -285,11 +285,8 @@ function buildArgumentValue(key: string, val: string, indent: string, inNamedVal if (key === 'target' && (apiId.includes('UiWithScreen') || apiId.includes('UiWithRow'))) { return `${indent}`; } - // D2: 'locator' argument → class="uiLocator" (only for UiDoAction / UiAssert / UiScrollToElement). - if ( - key === 'locator' && - (apiId.includes('UiDoAction') || apiId.includes('UiAssert') || apiId.includes('UiScrollToElement')) - ) { + // D2: 'locator' argument → class="uiLocator" (only for UiDoAction / UiAssert). + if (key === 'locator' && (apiId.includes('UiDoAction') || apiId.includes('UiAssert'))) { return `${indent}`; } } diff --git a/src/mcp/tools/testCaseValidate.ts b/src/mcp/tools/testCaseValidate.ts index f872b0a2..8c364318 100644 --- a/src/mcp/tools/testCaseValidate.ts +++ b/src/mcp/tools/testCaseValidate.ts @@ -437,8 +437,8 @@ function validateApiCallArgs( } // UI-LOCATOR-001 (local rule, no direct backend equivalent): - // UiDoAction / UiAssert / UiScrollToElement locator argument must use class="uiLocator". - if (apiId.includes('UiDoAction') || apiId.includes('UiAssert') || apiId.includes('UiScrollToElement')) { + // UiDoAction / UiAssert locator argument must use class="uiLocator". + if (apiId.includes('UiDoAction') || apiId.includes('UiAssert')) { const locatorArg = getArgList(call).find((a) => (a['@_id'] as string | undefined) === 'locator'); if (locatorArg) { const valClass = (locatorArg['value'] as Record | undefined)?.['@_class'] as string | undefined; From c046e341c17520f73fea1e853dd421d8d930ca73 Mon Sep 17 00:00:00 2001 From: Michael Dailey Date: Fri, 1 May 2026 15:59:45 -0500 Subject: [PATCH 08/19] PDX-0: feat(ci): add Slack deploy notification to DeployManual workflow Posts version, tag, deployer, and auto-extracted change notes (feat/fix commits since previous tag) on successful publish. Falls back to GitHub Release body when available. Payload built via jq for safe escaping. Co-Authored-By: Claude Sonnet 4.6 --- .github/workflows/DeployManual.yml | 80 ++++++++++++++++++++++++++++++ 1 file changed, 80 insertions(+) diff --git a/.github/workflows/DeployManual.yml b/.github/workflows/DeployManual.yml index 4fd9c345..5f6894e1 100644 --- a/.github/workflows/DeployManual.yml +++ b/.github/workflows/DeployManual.yml @@ -15,6 +15,8 @@ jobs: steps: - name: Checkout uses: actions/checkout@v4 + with: + fetch-depth: 0 - name: Setup Node uses: actions/setup-node@v4 with: @@ -47,3 +49,81 @@ jobs: run: ./mcp-publisher login github-oidc - name: Publish to MCP Registry run: ./mcp-publisher publish + + - name: Build Slack notification payload + if: success() + env: + RELEASE_BODY: ${{ github.event.release.body }} + run: | + VERSION=$(node -p "require('./package.json').version") + TAG="${{ github.event.inputs.tag || 'latest' }}" + + # --- Determine change notes source --- + if [ -n "$RELEASE_BODY" ]; then + # GitHub Release body provided — use it verbatim + NOTES="$RELEASE_BODY" + else + # Auto-extract from git log since the previous tag + if [ "${{ github.event_name }}" = "release" ]; then + # For a release event HEAD is already the new tag; find the one before it + PREV=$(git tag --sort=-creatordate | awk "/^${GITHUB_REF_NAME}$/{found=1;next} found{print;exit}") + else + # For a manual dispatch use the most recent existing tag + PREV=$(git tag --sort=-creatordate | head -1) + fi + + RANGE="${PREV:+${PREV}..}HEAD" + + RAW=$(git log --pretty=format:"%s" $RANGE \ + | sed 's/^[A-Z][A-Z0-9]*-[0-9]*: //' \ + | grep -Ev "^(Merge |chore)" \ + | head -20) + + FEATS=$(printf '%s\n' "$RAW" | grep '^feat' | sed 's/^feat[^:]*: /• /' | head -8) + FIXES=$(printf '%s\n' "$RAW" | grep '^fix' | sed 's/^fix[^:]*: /• /' | head -8) + + NOTES="" + [ -n "$FEATS" ] && NOTES="*What's new:*"$'\n'"$FEATS" + if [ -n "$FIXES" ]; then + [ -n "$NOTES" ] && NOTES="${NOTES}"$'\n' + NOTES="${NOTES}*Fixes:*"$'\n'"$FIXES" + fi + [ -z "$NOTES" ] && NOTES="_No notable changes extracted._" + fi + + # --- Build Block Kit JSON payload via jq (handles escaping correctly) --- + NPM_URL="https://www.npmjs.com/package/@provartesting/provardx-cli/v/${VERSION}" + RUN_URL="${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}" + + jq -n \ + --arg ver "$VERSION" \ + --arg tag "$TAG" \ + --arg who "${{ github.actor }}" \ + --arg body "$NOTES" \ + --arg npm "$NPM_URL" \ + --arg run "$RUN_URL" \ + '{blocks:[ + {type:"header", + text:{type:"plain_text", text:("🚀 Provar MCP v"+$ver+" published"), emoji:true}}, + {type:"section", + fields:[ + {type:"mrkdwn", text:("*Tag:* `"+$tag+"`")}, + {type:"mrkdwn", text:("*By:* "+$who)} + ]}, + {type:"section", + text:{type:"mrkdwn", text:$body}}, + {type:"divider"}, + {type:"actions", + elements:[ + {type:"button", text:{type:"plain_text", text:"📦 npm", emoji:true}, url:$npm}, + {type:"button", text:{type:"plain_text", text:"🔗 GitHub Run", emoji:true}, url:$run} + ]} + ]}' > /tmp/slack_payload.json + + - name: Notify Slack + if: success() + uses: slackapi/slack-github-action@v2.0.0 + with: + webhook: ${{ secrets.SLACK_WEBHOOK_URL }} + webhook-type: incoming-webhook + payload-file-path: /tmp/slack_payload.json From 226885c241b1077d13d991eeb39c318cc1cfd862 Mon Sep 17 00:00:00 2001 From: Michael Dailey Date: Fri, 1 May 2026 16:07:00 -0500 Subject: [PATCH 09/19] PDX-0: fix: address PR #137 Copilot review comments - plan_name: tighten validation to reject dot-segments (..) via safe-name regex; previously plan_name=".." resolved to projectRoot - UI-TARGET-001 / UI-LOCATOR-001 / SETVALUES-STRUCTURE-001: fire when node is present but class attribute is absent, not only when class is wrong; distinguish via "(missing)" in error message - Add 5 tests covering the new missing-class and dot-segment cases Co-Authored-By: Claude Sonnet 4.6 --- src/mcp/tools/testCaseValidate.ts | 62 +++++++++++++---------- src/mcp/tools/testPlanTools.ts | 4 +- test/unit/mcp/testCaseValidate.test.ts | 69 ++++++++++++++++++++++++++ test/unit/mcp/testPlanTools.test.ts | 28 +++++++++++ 4 files changed, 134 insertions(+), 29 deletions(-) diff --git a/src/mcp/tools/testCaseValidate.ts b/src/mcp/tools/testCaseValidate.ts index 8c364318..1697faab 100644 --- a/src/mcp/tools/testCaseValidate.ts +++ b/src/mcp/tools/testCaseValidate.ts @@ -406,13 +406,15 @@ function validateApiCall(call: Record, issues: ValidationIssue[ function checkUiTarget(call: Record, apiId: string, stepName: string, issues: ValidationIssue[]): void { const targetArg = getArgList(call).find((a) => (a['@_id'] as string | undefined) === 'target'); if (!targetArg) return; - const valClass = (targetArg['value'] as Record | undefined)?.['@_class'] as string | undefined; - if (valClass && valClass !== 'uiTarget') { + const valueNode = targetArg['value'] as Record | undefined; + if (!valueNode) return; + const valClass = valueNode['@_class'] as string | undefined; + if (valClass !== 'uiTarget') { const apiLabel = apiId.includes('UiWithRow') ? 'UiWithRow' : 'UiWithScreen'; issues.push({ rule_id: 'UI-TARGET-001', severity: 'ERROR', - message: `${apiLabel} step "${stepName}" target argument uses class="${valClass}" — must be class="uiTarget".`, + message: `${apiLabel} step "${stepName}" target argument uses class="${valClass ?? '(missing)'}" — must be class="uiTarget".`, applies_to: 'apiCall', suggestion: 'Emit the target as: or uri="ui:pageobject:target?pageId=...". ' + @@ -441,17 +443,20 @@ function validateApiCallArgs( if (apiId.includes('UiDoAction') || apiId.includes('UiAssert')) { const locatorArg = getArgList(call).find((a) => (a['@_id'] as string | undefined) === 'locator'); if (locatorArg) { - const valClass = (locatorArg['value'] as Record | undefined)?.['@_class'] as string | undefined; - if (valClass && valClass !== 'uiLocator') { - issues.push({ - rule_id: 'UI-LOCATOR-001', - severity: 'ERROR', - message: `"${stepName}" locator argument uses class="${valClass}" — must be class="uiLocator".`, - applies_to: 'apiCall', - suggestion: - 'Emit the locator as: . ' + - 'In provar.testcase.generate the "locator" attribute is converted automatically.', - }); + const locatorNode = locatorArg['value'] as Record | undefined; + if (locatorNode) { + const valClass = locatorNode['@_class'] as string | undefined; + if (valClass !== 'uiLocator') { + issues.push({ + rule_id: 'UI-LOCATOR-001', + severity: 'ERROR', + message: `"${stepName}" locator argument uses class="${valClass ?? '(missing)'}" — must be class="uiLocator".`, + applies_to: 'apiCall', + suggestion: + 'Emit the locator as: . ' + + 'In provar.testcase.generate the "locator" attribute is converted automatically.', + }); + } } } } @@ -462,19 +467,22 @@ function validateApiCallArgs( if (apiId.includes('SetValues') && !apiId.includes('AssertValues')) { const valuesArg = getArgList(call).find((a) => (a['@_id'] as string | undefined) === 'values'); if (valuesArg) { - const valClass = (valuesArg['value'] as Record | undefined)?.['@_class'] as string | undefined; - if (valClass && valClass !== 'valueList') { - issues.push({ - rule_id: 'SETVALUES-STRUCTURE-001', - severity: 'ERROR', - message: `SetValues step "${stepName}" values argument uses class="${valClass}" — must use class="valueList" with children.`, - applies_to: 'apiCall', - suggestion: - 'Wrap variable assignments in: ' + - 'value' + - '. In provar.testcase.generate pass each variable as a flat key/value pair ' + - 'in attributes — the generator builds the valueList structure automatically.', - }); + const valuesNode = valuesArg['value'] as Record | undefined; + if (valuesNode) { + const valClass = valuesNode['@_class'] as string | undefined; + if (valClass !== 'valueList') { + issues.push({ + rule_id: 'SETVALUES-STRUCTURE-001', + severity: 'ERROR', + message: `SetValues step "${stepName}" values argument uses class="${valClass ?? '(missing)'}" — must use class="valueList" with children.`, + applies_to: 'apiCall', + suggestion: + 'Wrap variable assignments in: ' + + 'value' + + '. In provar.testcase.generate pass each variable as a flat key/value pair ' + + 'in attributes — the generator builds the valueList structure automatically.', + }); + } } } } diff --git a/src/mcp/tools/testPlanTools.ts b/src/mcp/tools/testPlanTools.ts index 716c4022..d165c4ca 100644 --- a/src/mcp/tools/testPlanTools.ts +++ b/src/mcp/tools/testPlanTools.ts @@ -105,13 +105,13 @@ export function registerTestPlanCreate(server: McpServer, config: ServerConfig): }; } - if (plan_name.includes('/') || plan_name.includes('\\') || path.isAbsolute(plan_name)) { + if (!/^[A-Za-z0-9][\w\- ]*$/.test(plan_name)) { return { isError: true, content: [ { type: 'text' as const, - text: JSON.stringify(makeError('INVALID_PLAN_NAME', `plan_name must be a simple directory name without path separators: "${plan_name}"`, requestId)), + text: JSON.stringify(makeError('INVALID_PLAN_NAME', `plan_name must start with a letter or digit and contain only letters, digits, underscores, hyphens, or spaces: "${plan_name}"`, requestId)), }, ], }; diff --git a/test/unit/mcp/testCaseValidate.test.ts b/test/unit/mcp/testCaseValidate.test.ts index 456fc09b..205f1052 100644 --- a/test/unit/mcp/testCaseValidate.test.ts +++ b/test/unit/mcp/testCaseValidate.test.ts @@ -359,6 +359,29 @@ describe('validateTestCase', () => { ); }); + it('fires when UiWithScreen target has no class attribute', () => { + const r = validateTestCase( + ` + + + + + + sf:ui:target?object=Account + + + + +` + ); + assert.ok( + r.issues.some((i) => i.rule_id === 'UI-TARGET-001'), + 'UI-TARGET-001 should fire when has no class attribute' + ); + const issue = r.issues.find((i) => i.rule_id === 'UI-TARGET-001')!; + assert.ok(issue.message.includes('(missing)'), `Message should note missing class: ${issue.message}`); + }); + it('also fires for UiWithRow steps with wrong target class', () => { const r = validateTestCase( ` @@ -427,6 +450,29 @@ describe('validateTestCase', () => { ); }); + it('fires when UiDoAction locator has no class attribute', () => { + const r = validateTestCase( + ` + + + + + + sf:ui:locator:label?label=Save + + + + +` + ); + assert.ok( + r.issues.some((i) => i.rule_id === 'UI-LOCATOR-001'), + 'UI-LOCATOR-001 should fire when has no class attribute' + ); + const issue = r.issues.find((i) => i.rule_id === 'UI-LOCATOR-001')!; + assert.ok(issue.message.includes('(missing)'), `Message should note missing class: ${issue.message}`); + }); + it('also fires for UiAssert steps with wrong locator class', () => { const r = validateTestCase( ` @@ -501,6 +547,29 @@ describe('validateTestCase', () => { ); }); + it('fires when SetValues values has no class attribute', () => { + const r = validateTestCase( + ` + + + + + + someText + + + + +` + ); + assert.ok( + r.issues.some((i) => i.rule_id === 'SETVALUES-STRUCTURE-001'), + 'SETVALUES-STRUCTURE-001 should fire when has no class attribute' + ); + const issue = r.issues.find((i) => i.rule_id === 'SETVALUES-STRUCTURE-001')!; + assert.ok(issue.message.includes('(missing)'), `Message should note missing class: ${issue.message}`); + }); + it('does not fire when SetValues has no values argument (self-closing)', () => { const r = validateTestCase( ` diff --git a/test/unit/mcp/testPlanTools.test.ts b/test/unit/mcp/testPlanTools.test.ts index 28f299f5..50021882 100644 --- a/test/unit/mcp/testPlanTools.test.ts +++ b/test/unit/mcp/testPlanTools.test.ts @@ -207,6 +207,34 @@ describe('provar.testplan.create', () => { assert.equal(parseText(result)['created'], true); }); + it('returns INVALID_PLAN_NAME for dot-segment plan_name (..)', () => { + makeProject(projectDir); + + const result = server.call('provar.testplan.create', { + project_path: projectDir, + plan_name: '..', + overwrite: false, + dry_run: false, + }); + + assert.equal(isError(result), true); + assert.equal(errorCode(result), 'INVALID_PLAN_NAME'); + }); + + it('returns INVALID_PLAN_NAME for plan_name with path separators', () => { + makeProject(projectDir); + + const result = server.call('provar.testplan.create', { + project_path: projectDir, + plan_name: 'sub/plan', + overwrite: false, + dry_run: false, + }); + + assert.equal(isError(result), true); + assert.equal(errorCode(result), 'INVALID_PLAN_NAME'); + }); + it('returns PATH_NOT_ALLOWED when project_path is outside allowedPaths', () => { const strictServer = new MockMcpServer(); registerAllTestPlanTools(strictServer as never, { allowedPaths: [tmpDir] }); From be056131910b83068b34d9275e891bccd1af90d1 Mon Sep 17 00:00:00 2001 From: Michael Dailey Date: Sat, 2 May 2026 21:30:11 -0500 Subject: [PATCH 10/19] PDX-0: fix(validate): use == null guards for self-closing elements; fix awk regex in deploy workflow - testCaseValidate.ts: change !valueNode/!locatorNode/!valuesNode truthiness guards to == null / != null checks so fast-xml-parser's empty-string '' for self-closing elements still triggers UI-TARGET-001, UI-LOCATOR-001, and SETVALUES-STRUCTURE-001 instead of being silently skipped - testCaseValidate.test.ts: add three self-closing tests (one per rule) to cover the '' edge case explicitly - DeployManual.yml: replace awk regex pattern match with -v tag / string equality so tag names like v1.2.3 are matched literally rather than as regexes Co-Authored-By: Claude Sonnet 4.6 --- .github/workflows/DeployManual.yml | 2 +- src/mcp/tools/testCaseValidate.ts | 6 +-- test/unit/mcp/testCaseValidate.test.ts | 69 ++++++++++++++++++++++++++ 3 files changed, 73 insertions(+), 4 deletions(-) diff --git a/.github/workflows/DeployManual.yml b/.github/workflows/DeployManual.yml index 5f6894e1..68d8c469 100644 --- a/.github/workflows/DeployManual.yml +++ b/.github/workflows/DeployManual.yml @@ -66,7 +66,7 @@ jobs: # Auto-extract from git log since the previous tag if [ "${{ github.event_name }}" = "release" ]; then # For a release event HEAD is already the new tag; find the one before it - PREV=$(git tag --sort=-creatordate | awk "/^${GITHUB_REF_NAME}$/{found=1;next} found{print;exit}") + PREV=$(git tag --sort=-creatordate | awk -v tag="${GITHUB_REF_NAME}" '$0==tag{found=1;next} found{print;exit}') else # For a manual dispatch use the most recent existing tag PREV=$(git tag --sort=-creatordate | head -1) diff --git a/src/mcp/tools/testCaseValidate.ts b/src/mcp/tools/testCaseValidate.ts index 1697faab..34a6691f 100644 --- a/src/mcp/tools/testCaseValidate.ts +++ b/src/mcp/tools/testCaseValidate.ts @@ -407,7 +407,7 @@ function checkUiTarget(call: Record, apiId: string, stepName: s const targetArg = getArgList(call).find((a) => (a['@_id'] as string | undefined) === 'target'); if (!targetArg) return; const valueNode = targetArg['value'] as Record | undefined; - if (!valueNode) return; + if (valueNode == null) return; const valClass = valueNode['@_class'] as string | undefined; if (valClass !== 'uiTarget') { const apiLabel = apiId.includes('UiWithRow') ? 'UiWithRow' : 'UiWithScreen'; @@ -444,7 +444,7 @@ function validateApiCallArgs( const locatorArg = getArgList(call).find((a) => (a['@_id'] as string | undefined) === 'locator'); if (locatorArg) { const locatorNode = locatorArg['value'] as Record | undefined; - if (locatorNode) { + if (locatorNode != null) { const valClass = locatorNode['@_class'] as string | undefined; if (valClass !== 'uiLocator') { issues.push({ @@ -468,7 +468,7 @@ function validateApiCallArgs( const valuesArg = getArgList(call).find((a) => (a['@_id'] as string | undefined) === 'values'); if (valuesArg) { const valuesNode = valuesArg['value'] as Record | undefined; - if (valuesNode) { + if (valuesNode != null) { const valClass = valuesNode['@_class'] as string | undefined; if (valClass !== 'valueList') { issues.push({ diff --git a/test/unit/mcp/testCaseValidate.test.ts b/test/unit/mcp/testCaseValidate.test.ts index 205f1052..279209b6 100644 --- a/test/unit/mcp/testCaseValidate.test.ts +++ b/test/unit/mcp/testCaseValidate.test.ts @@ -382,6 +382,29 @@ describe('validateTestCase', () => { assert.ok(issue.message.includes('(missing)'), `Message should note missing class: ${issue.message}`); }); + it('fires when UiWithScreen target uses self-closing (empty string from fast-xml-parser)', () => { + const r = validateTestCase( + ` + + + + + + + + + + +` + ); + assert.ok( + r.issues.some((i) => i.rule_id === 'UI-TARGET-001'), + 'UI-TARGET-001 should fire for self-closing (no class attribute)' + ); + const issue = r.issues.find((i) => i.rule_id === 'UI-TARGET-001')!; + assert.ok(issue.message.includes('(missing)'), `Message should note missing class: ${issue.message}`); + }); + it('also fires for UiWithRow steps with wrong target class', () => { const r = validateTestCase( ` @@ -473,6 +496,29 @@ describe('validateTestCase', () => { assert.ok(issue.message.includes('(missing)'), `Message should note missing class: ${issue.message}`); }); + it('fires when UiDoAction locator uses self-closing (empty string from fast-xml-parser)', () => { + const r = validateTestCase( + ` + + + + + + + + + + +` + ); + assert.ok( + r.issues.some((i) => i.rule_id === 'UI-LOCATOR-001'), + 'UI-LOCATOR-001 should fire for self-closing (no class attribute)' + ); + const issue = r.issues.find((i) => i.rule_id === 'UI-LOCATOR-001')!; + assert.ok(issue.message.includes('(missing)'), `Message should note missing class: ${issue.message}`); + }); + it('also fires for UiAssert steps with wrong locator class', () => { const r = validateTestCase( ` @@ -570,6 +616,29 @@ describe('validateTestCase', () => { assert.ok(issue.message.includes('(missing)'), `Message should note missing class: ${issue.message}`); }); + it('fires when SetValues values uses self-closing (empty string from fast-xml-parser)', () => { + const r = validateTestCase( + ` + + + + + + + + + + +` + ); + assert.ok( + r.issues.some((i) => i.rule_id === 'SETVALUES-STRUCTURE-001'), + 'SETVALUES-STRUCTURE-001 should fire for self-closing (no class attribute)' + ); + const issue = r.issues.find((i) => i.rule_id === 'SETVALUES-STRUCTURE-001')!; + assert.ok(issue.message.includes('(missing)'), `Message should note missing class: ${issue.message}`); + }); + it('does not fire when SetValues has no values argument (self-closing)', () => { const r = validateTestCase( ` From 25defa13e97857f93f3c2a559491cec0d20177e1 Mon Sep 17 00:00:00 2001 From: Michael Dailey Date: Mon, 4 May 2026 11:51:52 -0500 Subject: [PATCH 11/19] increment version --- package.json | 2 +- server.json | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/package.json b/package.json index 8f459891..c9ccd879 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "@provartesting/provardx-cli", "description": "A plugin for the Salesforce CLI to orchestrate testing activities and report quality metrics to Provar Quality Hub", - "version": "1.5.0-beta.12", + "version": "1.5.0-beta.13", "mcpName": "io.github.ProvarTesting/provar", "license": "BSD-3-Clause", "plugins": [ diff --git a/server.json b/server.json index c23758f0..96d72334 100644 --- a/server.json +++ b/server.json @@ -14,12 +14,12 @@ "url": "https://github.com/ProvarTesting/provardx-cli", "source": "github" }, - "version": "1.5.0-beta.12", + "version": "1.5.0-beta.13", "packages": [ { "registryType": "npm", "identifier": "@provartesting/provardx-cli", - "version": "1.5.0-beta.12", + "version": "1.5.0-beta.13", "transport": { "type": "stdio" }, From 54c7ee30994c6253f9668bff73a93a1133118c60 Mon Sep 17 00:00:00 2001 From: Michael Dailey Date: Mon, 4 May 2026 14:48:46 -0500 Subject: [PATCH 12/19] PDX-0: fix: address session 4 MCP feedback items A3/A5/E2/E3/E4/C1 RCA: Session 4 testing on beta.12 found six issues: wrong testCase XML structure (A3), missing ApexSoqlQuery arg-name docs (A5), no .planitem integrity check (E2), no inline-edit guidance for missing Edit page objects (E3), no ENOBUFS transport hint in testrun description (E4), no IDE duplicate-argument warning (C1). Fix: Fixed buildTestCaseXml to emit standalone="no", id="1", no name attr, child; TC_010 rule now requires id="1" literal; extracted checkTestCaseIdAndGuid helper to stay within ESLint complexity=20 limit; added .planitem check in readPlansDir/readPlanDirectory and surfaced plan_integrity_warnings in project.validate; added ApexSoqlQuery arg IDs, inline-edit fallback, IDE dup-arg warning, ENOBUFS hint to tool descriptions; updated unit tests and docs/mcp.md. Co-Authored-By: Claude Sonnet 4.6 --- docs/mcp.md | 58 +++++--- src/mcp/tools/automationTools.ts | 2 + src/mcp/tools/projectValidateFromPath.ts | 64 ++++++--- src/mcp/tools/testCaseGenerate.ts | 34 +++-- src/mcp/tools/testCaseValidate.ts | 92 +++++++----- src/services/projectValidation.ts | 135 ++++++++++++------ test/unit/mcp/projectValidateFromPath.test.ts | 100 +++++++++++-- test/unit/mcp/testCaseGenerate.test.ts | 49 ++++--- test/unit/mcp/testCaseValidate.test.ts | 23 ++- 9 files changed, 399 insertions(+), 158 deletions(-) diff --git a/docs/mcp.md b/docs/mcp.md index 3366c616..10d3e02e 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -632,6 +632,21 @@ 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. +**Generated `` element structure (Provar requirements):** + +```xml + + + + ... + +``` + +- `id` is always the integer literal `"1"` — Provar ignores any other value +- No `name` attribute on `` — Provar derives the name from the file name +- `` must appear before `` +- `standalone="no"` is required in the XML declaration + **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) @@ -639,13 +654,13 @@ Generates an XML test case skeleton with UUID v4 guids and sequential `testItemI **Argument XML conventions** (automatically applied by the generator): -| Argument key / value pattern | Emitted XML class | API context | -| ------------------------------------ | ----------------------------- | ----------------------------------- | -| `target` key | `class="uiTarget"` | UiWithScreen, UiWithRow | -| `locator` key | `class="uiLocator"` | UiDoAction, UiAssert | -| Value matches `{VarName}` or `{A.B}` | `class="variable"` + `` | Any step | -| SetValues attributes | `class="valueList"/` | SetValues only | -| All other values | `class="value" valueClass="string"` | Any step | +| Argument key / value pattern | Emitted XML class | API context | +| ------------------------------------ | ----------------------------------- | ----------------------- | +| `target` key | `class="uiTarget"` | UiWithScreen, UiWithRow | +| `locator` key | `class="uiLocator"` | UiDoAction, UiAssert | +| Value matches `{VarName}` or `{A.B}` | `class="variable"` + `` | Any step | +| SetValues attributes | `class="valueList"/` | SetValues only | +| All other values | `class="value" valueClass="string"` | Any step | AssertValues uses **flat** argument structure (`expectedValue`, `actualValue`, `comparisonType`) — not the `valueList`/namedValues format. @@ -654,7 +669,6 @@ AssertValues uses **flat** argument structure (`expectedValue`, `actualValue`, ` | 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`) | @@ -663,6 +677,15 @@ AssertValues uses **flat** argument structure (`expectedValue`, `actualValue`, ` | `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 | +**`ApexSoqlQuery` argument IDs** (common source of runtime errors — wrong names are silently accepted but fail at runtime): + +| Argument ID | Purpose | +| -------------------- | ------------------------------------------- | +| `soqlQuery` | The SOQL SELECT statement | +| `resultListName` | Variable name that receives the result list | +| `apexConnectionName` | Named Salesforce connection | +| `resultScope` | Optional scope (Test, Local, Global) | + **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`. @@ -793,17 +816,20 @@ Validates a Provar project directly from its directory on disk. Reads the plan/s **Output** (slim mode, `include_plan_details: false`) -| Field | Description | -| ---------------------- | ----------------------------------------------------- | -| `quality_score` | Project quality score (0–100) | -| `coverage_percent` | Percentage of test cases covered by at least one plan | -| `violation_summary` | Map of `rule_id → count` for all violations found | -| `plan_scores` | Array of `{ name, quality_score }` per plan | -| `uncovered_test_cases` | Uncovered test case paths (capped at `max_uncovered`) | -| `save_error` | Present only if the results file could not be written | +| Field | Description | +| ------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `quality_score` | Project quality score (0–100) | +| `coverage_percent` | Percentage of test cases covered by at least one plan | +| `violation_summary` | Map of `rule_id → count` for all violations found | +| `plan_scores` | Array of `{ name, quality_score }` per plan | +| `uncovered_test_cases` | Uncovered test case paths (capped at `max_uncovered`) | +| `save_error` | Present only if the results file could not be written | +| `plan_integrity_warnings` | Present when any plan or suite directory is missing a `.planitem` file — test instances in those directories are silently invisible to the Provar runner | When `include_plan_details: true`, the response additionally includes full `test_plans[]` with nested suite and per-test-case data. +**Plan integrity warnings:** `provar.project.validate` now walks every `plans/` subdirectory and checks that a `.planitem` file is present at both the plan level and each suite subfolder level. If any are missing, the response includes a `plan_integrity_warnings` array. Use `provar.testplan.create-suite` to create missing `.planitem` files without losing any existing test instances. + **Violation rule IDs:** PROJ-EMPTY-001, PROJ-DUP-001, PROJ-DUP-002, PROJ-CALLABLE-001, PROJ-CALLABLE-002, PROJ-CONN-001, PROJ-ENV-001, PROJ-ENV-002, PROJ-SECRET-001 **Error codes:** `NOT_A_PROJECT`, `AMBIGUOUS_PROJECT`, `PATH_NOT_FOUND`, `PATH_NOT_ALLOWED`, `PATH_TRAVERSAL` diff --git a/src/mcp/tools/automationTools.ts b/src/mcp/tools/automationTools.ts index 42b4038b..4972b783 100644 --- a/src/mcp/tools/automationTools.ts +++ b/src/mcp/tools/automationTools.ts @@ -395,6 +395,8 @@ export function registerAutomationTestRun(server: McpServer, config: ServerConfi 'Requires Provar to be installed locally and provarHome set correctly in the properties file.', 'Use provar.automation.setup first if Provar is not yet installed.', 'For grid/CI execution via Provar Quality Hub instead of running locally, use provar.qualityhub.testrun.', + 'Output buffer: a 50 MB maxBuffer is set so ENOBUFS on verbose Provar runs is now rare.', + 'If ENOBUFS still occurs (extremely verbose logging), run `sf provar automation test run --json` directly in the terminal and pipe or tail the output instead of retrying this tool.', 'Typical local AI loop: config.load → compile → testrun → inspect results.', ].join(' '), { diff --git a/src/mcp/tools/projectValidateFromPath.ts b/src/mcp/tools/projectValidateFromPath.ts index 5151f3cf..4ca33865 100644 --- a/src/mcp/tools/projectValidateFromPath.ts +++ b/src/mcp/tools/projectValidateFromPath.ts @@ -32,8 +32,7 @@ interface ViolationSummary { } function buildPlanSummary(plan: ValidatedPlan): PlanSummary { - const test_case_count = plan.suites.reduce((n, s) => n + s.test_cases.length, 0) - + plan.unplanned_test_cases.length; + const test_case_count = plan.suites.reduce((n, s) => n + s.test_cases.length, 0) + plan.unplanned_test_cases.length; return { name: plan.name, quality_score: plan.quality_score, @@ -69,9 +68,7 @@ function shapeResponse( const coverage = { ...coverageRest, uncovered_test_cases: uncovered_shown, - ...(uncovered_truncated - ? { uncovered_truncated: true, uncovered_total: uncovered_test_cases.length } - : {}), + ...(uncovered_truncated ? { uncovered_truncated: true, uncovered_total: uncovered_test_cases.length } : {}), }; if (includePlanDetails) { @@ -99,6 +96,9 @@ function shapeResponse( coverage, saved_to: result.saved_to, ...(result.save_error ? { save_error: result.save_error } : {}), + ...(result.plan_integrity_warnings && result.plan_integrity_warnings.length > 0 + ? { plan_integrity_warnings: result.plan_integrity_warnings } + : {}), hint: 'Set include_plan_details:true to get per-suite/test-case violations. project_violations and plan violations are grouped by rule_id in this view.', }; } @@ -119,22 +119,39 @@ export function registerProjectValidateFromPath(server: McpServer, config: Serve 'Pass include_plan_details:true to get full per-suite and per-test-case data.', 'By default saves a QH-compatible JSON report to', '{project_path}/provardx/validation/ (created if absent).', + 'Plan integrity: if any plan or suite directory is missing a .planitem file, the response includes a plan_integrity_warnings array.', + 'Test instances in those directories are silently ignored by the Provar runner — fix these before running tests.', 'IMPORTANT: Use this tool for whole-project validation —', 'DO NOT read individual test case files and pass XML content inline.', 'Pass a project_path and let this tool handle all file reading.', ].join(' '), { - project_path: z.string().describe('Absolute path to the Provar project root (the directory containing the .testproject file)'), - quality_threshold: z.number().min(0).max(100).optional().default(80).describe('Minimum quality score for a test case to be considered valid (default: 80)'), - save_results: z.boolean().optional().default(true).describe('Write a QH-compatible JSON report to provardx/validation/ (default: true)'), - results_dir: z.string().optional().describe('Override the output directory for the saved report (default: {project_path}/provardx/validation)'), + project_path: z + .string() + .describe('Absolute path to the Provar project root (the directory containing the .testproject file)'), + quality_threshold: z + .number() + .min(0) + .max(100) + .optional() + .default(80) + .describe('Minimum quality score for a test case to be considered valid (default: 80)'), + save_results: z + .boolean() + .optional() + .default(true) + .describe('Write a QH-compatible JSON report to provardx/validation/ (default: true)'), + results_dir: z + .string() + .optional() + .describe('Override the output directory for the saved report (default: {project_path}/provardx/validation)'), include_plan_details: z .boolean() .optional() .default(false) .describe( 'When true, include full per-suite and per-test-case violation data in the response. ' + - 'Default false to keep response small. Use only when you need to inspect specific test case failures.' + 'Default false to keep response small. Use only when you need to inspect specific test case failures.' ), max_uncovered: z .number() @@ -142,16 +159,28 @@ export function registerProjectValidateFromPath(server: McpServer, config: Serve .min(0) .optional() .default(20) - .describe('Maximum number of uncovered test case paths to include in the response (default: 20). Set to 0 for none, or a large number for all.'), + .describe( + 'Maximum number of uncovered test case paths to include in the response (default: 20). Set to 0 for none, or a large number for all.' + ), max_violations: z .number() .int() .min(0) .optional() .default(50) - .describe('When include_plan_details:true, caps project_violations returned (default: 50). Ignored in slim mode where violations are grouped by rule_id instead.'), + .describe( + 'When include_plan_details:true, caps project_violations returned (default: 50). Ignored in slim mode where violations are grouped by rule_id instead.' + ), }, - ({ project_path, quality_threshold, save_results, results_dir, include_plan_details, max_uncovered, max_violations }) => { + ({ + project_path, + quality_threshold, + save_results, + results_dir, + include_plan_details, + max_uncovered, + max_violations, + }) => { const requestId = makeRequestId(); log('info', 'provar.project.validate', { requestId, project_path, include_plan_details }); @@ -179,11 +208,12 @@ export function registerProjectValidateFromPath(server: McpServer, config: Serve }; } catch (err: unknown) { const error = err as Error & { code?: string }; - const code = error instanceof PathPolicyError - ? error.code - : error instanceof ProjectValidationError + const code = + error instanceof PathPolicyError + ? error.code + : error instanceof ProjectValidationError ? error.code - : (error.code ?? 'VALIDATE_ERROR'); + : error.code ?? 'VALIDATE_ERROR'; const isUserError = error instanceof PathPolicyError || error instanceof ProjectValidationError; const errResult = makeError(code, error.message, requestId, !isUserError); log('error', 'provar.project.validate failed', { requestId, error: error.message }); diff --git a/src/mcp/tools/testCaseGenerate.ts b/src/mcp/tools/testCaseGenerate.ts index d7dff526..a2947198 100644 --- a/src/mcp/tools/testCaseGenerate.ts +++ b/src/mcp/tools/testCaseGenerate.ts @@ -119,12 +119,14 @@ 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.', + 'Generated structure: with (id is always the integer literal "1" as required by the Provar runtime), a child, then .', '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.', + 'ApexSoqlQuery argument IDs: soqlQuery (the SOQL SELECT statement), resultListName (binds result list to a variable), apexConnectionName (named connection), resultScope (optional).', 'Data-driven note: only iterates rows when the test case runs via a test plan instance (.testinstance).', 'Running directly via the provardx testCase property resolves all data table variables as null.', 'Use provar.testplan.add-instance to wire into a plan for data-driven execution.', @@ -138,6 +140,13 @@ const TOOL_DESCRIPTION = [ 'Variable references: pass values as "{VarName}" (braces); emitted as class="variable" .', 'target argument (UiWithScreen/UiWithRow): pass the URI value; emitted as class="uiTarget" uri="...".', 'locator argument (UiDoAction/UiAssert): pass the URI value; emitted as class="uiLocator" uri="...".', + 'Edit page objects: action=Edit targets require a compiled page object for the SF object. ' + + 'If none exists in the project page-objects directory, the locator binding will fail at runtime. ' + + 'For objects without a compiled Edit page object, use inline edit instead: sfIleActivate to activate the field, ' + + 'set the value, then SaveEdit binding on the Record view screen.', + 'Provar IDE warning: opening a generated test case in Provar IDE injects empty elements for known parameter IDs. ' + + 'If the empty element appears after a populated one, the empty version wins at runtime. ' + + 'Always check for and remove duplicate empty arguments after any IDE open/save cycle before re-running.', 'Cleanup warning: ApexDeleteObject steps near end of test will be skipped if an earlier step fails (stopOnError=false). Use a TearDown callable.', 'Validation: when validate_after_edit=true (default) the response includes a validation field and returns TESTCASE_INVALID if the generated XML fails structural checks.', 'Grounding: call provar.qualityhub.examples.retrieve before generating to get corpus examples for the scenario — correct XML structure for the step types you need.', @@ -149,7 +158,6 @@ export function registerTestCaseGenerate(server: McpServer, config: ServerConfig TOOL_DESCRIPTION, { 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() @@ -299,11 +307,7 @@ function buildArgumentsXml(attributes: Record, baseIndent = ' const argLines = entries .map(([k, v]) => { const valueXml = buildArgumentValue(k, v, `${baseIndent} `, false, apiId); - return ( - `${baseIndent}\n` + - valueXml + '\n' + - `${baseIndent}` - ); + return `${baseIndent}\n` + valueXml + '\n' + `${baseIndent}`; }) .join('\n'); return `\n${baseIndent}\n${argLines}\n${baseIndent}\n${baseIndent.slice(0, -2)}`; @@ -325,7 +329,8 @@ function buildSetValuesXml(attributes: Record, baseIndent: strin `${i(0)}\n` + `${i(1)}\n` + `${i(2)}\n` + - namedValueLines + '\n' + + namedValueLines + + '\n' + `${i(2)}\n` + `${i(1)}\n` + `${i(0)}\n` + @@ -360,11 +365,9 @@ function buildFlatStepXml( 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(); @@ -378,10 +381,11 @@ function buildTestCaseXml(input: { stepLines = lines || ' '; } + // Provar requires: standalone="no", id="1" (integer literal), no name attr, before . return ( - '\n' + - `\n` + + '\n' + + `\n` + + ' \n' + ' \n' + stepLines + '\n \n' + @@ -406,7 +410,11 @@ function buildUiWithScreenXml( ' \n '; return ( ` ${buildArgumentsXml({ target: targetUri }, ' ', wrapperApiId).trimEnd()}${clausesXml}` + ` name="With page" testItemId="1">${buildArgumentsXml( + { target: targetUri }, + ' ', + wrapperApiId + ).trimEnd()}${clausesXml}` ); } diff --git a/src/mcp/tools/testCaseValidate.ts b/src/mcp/tools/testCaseValidate.ts index 34a6691f..2a84622f 100644 --- a/src/mcp/tools/testCaseValidate.ts +++ b/src/mcp/tools/testCaseValidate.ts @@ -185,6 +185,45 @@ export function validateTestCaseXml(filePath: string, config: ServerConfig): Tes return validateTestCase(fs.readFileSync(resolved, 'utf-8')); } +/** TC_010/TC_011: validate testCase id and guid attributes. */ +function checkTestCaseIdAndGuid(tcId: string | null, tcGuid: string | undefined, issues: ValidationIssue[]): void { + if (!tcId) { + issues.push({ + rule_id: 'TC_010', + severity: 'ERROR', + message: 'testCase missing required id attribute.', + applies_to: 'testCase', + suggestion: 'Add id="1" to testCase element (Provar requires the integer literal "1").', + }); + } else if (tcId !== '1') { + issues.push({ + rule_id: 'TC_010', + severity: 'ERROR', + message: `testCase id="${tcId}" is invalid — Provar requires id="1" (integer literal).`, + applies_to: 'testCase', + suggestion: 'Set id="1" on the testCase element. The unique identifier is the guid attribute, not id.', + }); + } + if (!tcGuid) { + issues.push({ + rule_id: 'TC_011', + severity: 'ERROR', + message: 'testCase missing required guid attribute.', + applies_to: 'testCase', + suggestion: 'Add guid attribute (UUID v4) to testCase element.', + }); + } else if (!UUID_V4_RE.test(tcGuid)) { + issues.push({ + rule_id: 'TC_012', + severity: 'ERROR', + message: `testCase guid "${tcGuid}" is not a valid UUID v4.`, + applies_to: 'testCase', + suggestion: + 'Replace with a valid UUID v4 — e.g. crypto.randomUUID(). The 4th segment must begin with 8, 9, a, or b.', + }); + } +} + /** Pure function — exported for unit testing */ export function validateTestCase(xmlContent: string, testName?: string): TestCaseValidationResult { const issues: ValidationIssue[] = []; @@ -242,33 +281,7 @@ export function validateTestCase(xmlContent: string, testName?: string): TestCas const tcName = (tc['@_name'] as string | undefined) ?? null; const tcGuid = tc['@_guid'] as string | undefined; - if (!tcId) { - issues.push({ - rule_id: 'TC_010', - severity: 'ERROR', - message: 'testCase missing required id attribute.', - applies_to: 'testCase', - suggestion: 'Add id attribute to testCase element.', - }); - } - if (!tcGuid) { - issues.push({ - rule_id: 'TC_011', - severity: 'ERROR', - message: 'testCase missing required guid attribute.', - applies_to: 'testCase', - suggestion: 'Add guid attribute (UUID v4) to testCase element.', - }); - } else if (!UUID_V4_RE.test(tcGuid)) { - issues.push({ - rule_id: 'TC_012', - severity: 'ERROR', - message: `testCase guid "${tcGuid}" is not a valid UUID v4.`, - applies_to: 'testCase', - suggestion: - 'Replace with a valid UUID v4 — e.g. crypto.randomUUID(). The 4th segment must begin with 8, 9, a, or b.', - }); - } + checkTestCaseIdAndGuid(tcId, tcGuid, issues); // TC_013 (registryId) is intentionally not checked here — registryId is a // Salesforce Quality Hub record ID assigned when a test case is registered in // the QH org. Local project files will never have this attribute, so checking @@ -323,7 +336,11 @@ export function validateTestCase(xmlContent: string, testName?: string): TestCas severity: 'WARNING', message: `Argument value "{${varMatch[1]}}" looks like a variable reference but is stored as a plain string — Provar will not resolve it at runtime.`, applies_to: 'argument', - suggestion: `Replace with . In provar.testcase.generate, use the {VarName} syntax in the attributes object — the generator converts it automatically.`, + suggestion: `Replace with . In provar.testcase.generate, use the {VarName} syntax in the attributes object — the generator converts it automatically.`, }); } @@ -403,7 +420,12 @@ function validateApiCall(call: Record, issues: ValidationIssue[ if (apiId) validateApiCallArgs(call, apiId, name, issues); } -function checkUiTarget(call: Record, apiId: string, stepName: string, issues: ValidationIssue[]): void { +function checkUiTarget( + call: Record, + apiId: string, + stepName: string, + issues: ValidationIssue[] +): void { const targetArg = getArgList(call).find((a) => (a['@_id'] as string | undefined) === 'target'); if (!targetArg) return; const valueNode = targetArg['value'] as Record | undefined; @@ -414,7 +436,9 @@ function checkUiTarget(call: Record, apiId: string, stepName: s issues.push({ rule_id: 'UI-TARGET-001', severity: 'ERROR', - message: `${apiLabel} step "${stepName}" target argument uses class="${valClass ?? '(missing)'}" — must be class="uiTarget".`, + message: `${apiLabel} step "${stepName}" target argument uses class="${ + valClass ?? '(missing)' + }" — must be class="uiTarget".`, applies_to: 'apiCall', suggestion: 'Emit the target as: or uri="ui:pageobject:target?pageId=...". ' + @@ -450,7 +474,9 @@ function validateApiCallArgs( issues.push({ rule_id: 'UI-LOCATOR-001', severity: 'ERROR', - message: `"${stepName}" locator argument uses class="${valClass ?? '(missing)'}" — must be class="uiLocator".`, + message: `"${stepName}" locator argument uses class="${ + valClass ?? '(missing)' + }" — must be class="uiLocator".`, applies_to: 'apiCall', suggestion: 'Emit the locator as: . ' + @@ -474,7 +500,9 @@ function validateApiCallArgs( issues.push({ rule_id: 'SETVALUES-STRUCTURE-001', severity: 'ERROR', - message: `SetValues step "${stepName}" values argument uses class="${valClass ?? '(missing)'}" — must use class="valueList" with children.`, + message: `SetValues step "${stepName}" values argument uses class="${ + valClass ?? '(missing)' + }" — must use class="valueList" with children.`, applies_to: 'apiCall', suggestion: 'Wrap variable assignments in: ' + diff --git a/src/services/projectValidation.ts b/src/services/projectValidation.ts index e54ccc97..9bb183c0 100644 --- a/src/services/projectValidation.ts +++ b/src/services/projectValidation.ts @@ -40,8 +40,8 @@ export class ProjectValidationError extends Error { export interface ProjectValidationOptions { project_path: string; quality_threshold?: number; // default 80 - save_results?: boolean; // default true (any value !== false means save) - results_dir?: string; // default '{project_path}/provardx/validation' + save_results?: boolean; // default true (any value !== false means save) + results_dir?: string; // default '{project_path}/provardx/validation' } export interface ValidatedTestCase { @@ -101,6 +101,8 @@ export interface ProjectValidationResult { uncovered_test_cases: string[]; }; saved_to: string | null; + /** Directories within plans/ that are missing a .planitem file and will be silently ignored by the Provar runner. */ + plan_integrity_warnings?: string[]; /** Set when save_results was requested but the write failed (disk full, permissions, etc.). */ save_error?: string; } @@ -153,7 +155,9 @@ export function resolveProjectRoot(givenPath: string): { root: string; candidate const sub = path.join(givenPath, entry.name); if (fs.existsSync(path.join(sub, '.testproject'))) candidates.push(sub); } - } catch { /* skip */ } + } catch { + /* skip */ + } if (candidates.length === 1) return { root: candidates[0], candidates: [] }; return { root: givenPath, candidates }; // caller handles 0 or multiple @@ -198,7 +202,8 @@ export function readProjectContext(projectPath: string): { const secretsFullPath = path.resolve(path.join(projectPath, secretsRelPath)); const projectPathResolved = path.resolve(projectPath); // Bounds check: only read secrets file if it's within the project directory - const secretsInBounds = secretsFullPath === projectPathResolved || secretsFullPath.startsWith(projectPathResolved + path.sep); + const secretsInBounds = + secretsFullPath === projectPathResolved || secretsFullPath.startsWith(projectPathResolved + path.sep); if (secretsInBounds && fs.existsSync(secretsFullPath)) { try { const secretsContent = fs.readFileSync(secretsFullPath, 'utf-8'); @@ -212,7 +217,9 @@ export function readProjectContext(projectPath: string): { const value = trimmed.slice(eqIdx + 1).trim(); if (value && !value.startsWith('ENC1(')) unencryptedSecretCount++; } - } catch { /* skip */ } + } catch { + /* skip */ + } } return { @@ -256,13 +263,13 @@ function resolveTestInstanceFull(instancePath: string, projectPath: string): Tes // Bounds check: only read test case files within the project directory const tcInBounds = tcFullPath === projResolved || tcFullPath.startsWith(projResolved + path.sep); // Derive name from the bounds-checked resolved path to prevent injection via crafted testCasePath - const tcName = tcInBounds - ? path.basename(tcFullPath, '.testcase') - : path.basename(testCasePath, '.testcase'); + const tcName = tcInBounds ? path.basename(tcFullPath, '.testcase') : path.basename(testCasePath, '.testcase'); if (tcInBounds && fs.existsSync(tcFullPath)) { try { xml_content = fs.readFileSync(tcFullPath, 'utf-8'); - } catch { /* xml_content stays undefined */ } + } catch { + /* xml_content stays undefined */ + } } return { testCase: { name: tcName, xml_content }, testCasePath, testCaseId }; @@ -283,7 +290,7 @@ function accumulateCoveredPath( testCasePath: string | null, testCaseId: string | null, coveredPaths: Set, - idMap: Map, + idMap: Map ): void { if (testCasePath) coveredPaths.add(testCasePath); if (testCaseId) { @@ -298,7 +305,7 @@ export function readSuiteDirectory( projectPath: string, depth = 0, coveredPaths?: Set, - idMap?: Map, + idMap?: Map ): TestSuiteInput { const testCases: TestCaseInput[] = []; const testSuites: TestSuiteInput[] = []; @@ -318,7 +325,9 @@ export function readSuiteDirectory( if (coveredPaths && idMap) accumulateCoveredPath(testCasePath, testCaseId, coveredPaths, idMap); } } - } catch { /* skip */ } + } catch { + /* skip */ + } return { name, test_cases: testCases, test_suites: testSuites }; } @@ -329,6 +338,7 @@ export function readPlanDirectory( projectPath: string, coveredPaths?: Set, idMap?: Map, + planIntegrityWarnings?: string[] ): TestPlanInput { const testCases: TestCaseInput[] = []; const testSuites: TestSuiteInput[] = []; @@ -339,6 +349,12 @@ export function readPlanDirectory( if (entry.name === 'node_modules') continue; const fullPath = path.join(planPath, entry.name); if (entry.isDirectory() && !entry.name.startsWith('.')) { + if (planIntegrityWarnings && !fs.existsSync(path.join(fullPath, '.planitem'))) { + const rel = path.relative(projectPath, fullPath).replace(/\\/g, '/'); + planIntegrityWarnings.push( + `${rel}/ is missing a .planitem file — test instances in this suite will be invisible to the runner.` + ); + } testSuites.push(readSuiteDirectory(fullPath, entry.name, projectPath, 0, coveredPaths, idMap)); } else if (entry.name.endsWith('.testinstance')) { const { testCase, testCasePath, testCaseId } = resolveTestInstanceFull(fullPath, projectPath); @@ -346,15 +362,22 @@ export function readPlanDirectory( if (coveredPaths && idMap) accumulateCoveredPath(testCasePath, testCaseId, coveredPaths, idMap); } } - } catch { /* skip */ } + } catch { + /* skip */ + } return { name, test_cases: testCases, test_suites: testSuites }; } -export function readPlansDir(projectPath: string): { plans: TestPlanInput[]; coveredPaths: Set } { +export function readPlansDir(projectPath: string): { + plans: TestPlanInput[]; + coveredPaths: Set; + planIntegrityWarnings: string[]; +} { const plansDir = path.join(projectPath, 'plans'); const coveredPaths = new Set(); - if (!fs.existsSync(plansDir)) return { plans: [], coveredPaths }; + const planIntegrityWarnings: string[] = []; + if (!fs.existsSync(plansDir)) return { plans: [], coveredPaths, planIntegrityWarnings }; // Build UUID→path map once so the plan walk can resolve testCaseId fallbacks // without a separate pass over the tests/ directory later. @@ -366,11 +389,18 @@ export function readPlansDir(projectPath: string): { plans: TestPlanInput[]; cov for (const entry of entries) { if (!entry.isDirectory() || entry.name.startsWith('.') || entry.name === 'node_modules') continue; const planPath = path.join(plansDir, entry.name); - plans.push(readPlanDirectory(planPath, entry.name, projectPath, coveredPaths, idMap)); + if (!fs.existsSync(path.join(planPath, '.planitem'))) { + planIntegrityWarnings.push( + `plans/${entry.name}/ is missing a .planitem file — this plan will not be recognised by the Provar runner.` + ); + } + plans.push(readPlanDirectory(planPath, entry.name, projectPath, coveredPaths, idMap, planIntegrityWarnings)); } - } catch { /* skip */ } + } catch { + /* skip */ + } - return { plans, coveredPaths }; + return { plans, coveredPaths, planIntegrityWarnings }; } /** @@ -388,8 +418,9 @@ function buildTestCaseIdMap(projectPath: string): Map { for (const entry of fs.readdirSync(dir, { withFileTypes: true })) { if (entry.name.startsWith('.') || entry.name === 'node_modules') continue; const fullPath = path.join(dir, entry.name); - if (entry.isDirectory()) { walk(fullPath); } - else if (entry.name.endsWith('.testcase')) { + if (entry.isDirectory()) { + walk(fullPath); + } else if (entry.name.endsWith('.testcase')) { try { const content = fs.readFileSync(fullPath, 'utf-8'); const rel = path.relative(projectPath, fullPath).replace(/\\/g, '/'); @@ -397,10 +428,14 @@ function buildTestCaseIdMap(projectPath: string): Map { const m = content.match(new RegExp(`${attr}=["']([^"']+)["']`)); if (m?.[1] && !idMap.has(m[1])) idMap.set(m[1], rel); } - } catch { /* skip */ } + } catch { + /* skip */ + } } } - } catch { /* skip */ } + } catch { + /* skip */ + } } walk(testsDir); return idMap; @@ -420,10 +455,13 @@ export function collectAllTestCaseNames(projectPath: string): string[] { try { for (const entry of fs.readdirSync(dir, { withFileTypes: true })) { if (entry.name.startsWith('.') || entry.name === 'node_modules') continue; - if (entry.isDirectory()) { walk(path.join(dir, entry.name)); } - else if (entry.name.endsWith('.testcase')) names.push(path.basename(entry.name, '.testcase')); + if (entry.isDirectory()) { + walk(path.join(dir, entry.name)); + } else if (entry.name.endsWith('.testcase')) names.push(path.basename(entry.name, '.testcase')); } - } catch { /* skip */ } + } catch { + /* skip */ + } } walk(testsDir); return names; @@ -446,8 +484,9 @@ export function collectCoveredPathsFromDisk(projectPath: string): Set { for (const entry of fs.readdirSync(dir, { withFileTypes: true })) { if (entry.name === 'node_modules') continue; const fullPath = path.join(dir, entry.name); - if (entry.isDirectory()) { walk(fullPath); } - else if (entry.name.endsWith('.testinstance')) { + if (entry.isDirectory()) { + walk(fullPath); + } else if (entry.name.endsWith('.testinstance')) { try { const content = fs.readFileSync(fullPath, 'utf-8'); // Primary: path-based match @@ -459,10 +498,14 @@ export function collectCoveredPathsFromDisk(projectPath: string): Set { const resolvedPath = idMap.get(idM[1]); if (resolvedPath) covered.add(resolvedPath); } - } catch { /* skip */ } + } catch { + /* skip */ + } } } - } catch { /* skip */ } + } catch { + /* skip */ + } } walk(plansDir); return covered; @@ -478,13 +521,16 @@ export function findUncoveredTestCases(projectPath: string, coveredPaths: Set, projectName: string ): string { - const targetDir = resultsDir - ? path.resolve(resultsDir) - : path.join(projectPath, 'provardx', 'validation'); + const targetDir = resultsDir ? path.resolve(resultsDir) : path.join(projectPath, 'provardx', 'validation'); fs.mkdirSync(targetDir, { recursive: true }); @@ -719,9 +769,7 @@ export function saveResults( * ambiguous project, not a Provar project directory). Lets unexpected I/O * errors propagate as-is. */ -export function validateProjectFromPath( - options: ProjectValidationOptions -): ProjectValidationResult { +export function validateProjectFromPath(options: ProjectValidationOptions): ProjectValidationResult { const { project_path, quality_threshold, save_results, results_dir } = options; const resolved = path.resolve(project_path); @@ -734,7 +782,9 @@ export function validateProjectFromPath( if (candidates.length > 1) { throw new ProjectValidationError( 'AMBIGUOUS_PROJECT', - `Multiple Provar projects found under "${resolved}". Specify the exact project directory: ${candidates.map((c) => path.basename(c)).join(', ')}` + `Multiple Provar projects found under "${resolved}". Specify the exact project directory: ${candidates + .map((c) => path.basename(c)) + .join(', ')}` ); } @@ -752,7 +802,7 @@ export function validateProjectFromPath( // 2. Read plan hierarchy from plans/ directory; covered paths are computed // as a byproduct of the walk — no second traversal needed. - const { plans: testPlans, coveredPaths } = readPlansDir(projectRoot); + const { plans: testPlans, coveredPaths, planIntegrityWarnings } = readPlansDir(projectRoot); // 3. Validate const input: ProjectInput = { @@ -837,6 +887,7 @@ export function validateProjectFromPath( uncovered_test_cases: uncoveredTestCases, }, saved_to: savedTo, + ...(planIntegrityWarnings.length > 0 ? { plan_integrity_warnings: planIntegrityWarnings } : {}), save_error: saveError, }; } diff --git a/test/unit/mcp/projectValidateFromPath.test.ts b/test/unit/mcp/projectValidateFromPath.test.ts index 6d6fa204..08e0a8eb 100644 --- a/test/unit/mcp/projectValidateFromPath.test.ts +++ b/test/unit/mcp/projectValidateFromPath.test.ts @@ -52,9 +52,10 @@ const TESTPROJECT_XML = ` .secrets `; -function makeXml(tcGuid: string, stepGuid: string, id: string): string { - return ` - +function makeXml(tcGuid: string, stepGuid: string): string { + return ` + + @@ -68,16 +69,14 @@ function writeFile(p: string, content: string): void { const G = { tc1: '550e8400-e29b-41d4-a716-446655440001', - s1: '550e8400-e29b-41d4-a716-446655440011', + s1: '550e8400-e29b-41d4-a716-446655440011', }; /** Build a minimal valid Provar project in the given directory */ function makeProject(root: string, planName = 'smoke', tcName = 'Login'): void { writeFile(path.join(root, '.testproject'), TESTPROJECT_XML); - writeFile(path.join(root, 'tests', `${tcName}.testcase`), - makeXml(G.tc1, G.s1, `tc-${tcName.toLowerCase()}`)); - writeFile(path.join(root, 'plans', planName, `${tcName}.testinstance`), - `testCasePath="tests/${tcName}.testcase"\n`); + writeFile(path.join(root, 'tests', `${tcName}.testcase`), makeXml(G.tc1, G.s1)); + writeFile(path.join(root, 'plans', planName, `${tcName}.testinstance`), `testCasePath="tests/${tcName}.testcase"\n`); } // ── Test setup ───────────────────────────────────────────────────────────────── @@ -211,8 +210,7 @@ describe('provar.project.validate (from path)', () => { }); const defaultResultsDir = path.join(tmpDir, 'provardx', 'validation'); - const exists = fs.existsSync(defaultResultsDir) && - fs.readdirSync(defaultResultsDir).length > 0; + const exists = fs.existsSync(defaultResultsDir) && fs.readdirSync(defaultResultsDir).length > 0; assert.equal(exists, false, 'No results file should be written when save_results=false'); }); @@ -306,7 +304,7 @@ describe('provar.project.validate (from path)', () => { const root = mktemp(); writeFile(path.join(root, '.testproject'), TESTPROJECT_XML); for (const name of ['Login', 'Logout', 'Search']) { - writeFile(path.join(root, 'tests', `${name}.testcase`), makeXml(G.tc1, G.s1, `tc-${name.toLowerCase()}`)); + writeFile(path.join(root, 'tests', `${name}.testcase`), makeXml(G.tc1, G.s1)); } writeFile(path.join(root, 'plans', 'smoke', 'Login.testinstance'), 'testCasePath="tests/Login.testcase"\n'); @@ -322,4 +320,84 @@ describe('provar.project.validate (from path)', () => { assert.equal(coverage['uncovered_truncated'], true); }); }); + + describe('plan integrity warnings (E2)', () => { + it('reports plan_integrity_warnings when a plan dir is missing .planitem', () => { + const root = mktemp(); + writeFile(path.join(root, '.testproject'), TESTPROJECT_XML); + writeFile(path.join(root, 'tests', 'Login.testcase'), makeXml(G.tc1, G.s1)); + // Plan dir exists but has no .planitem file + writeFile(path.join(root, 'plans', 'smoke', 'Login.testinstance'), 'testCasePath="tests/Login.testcase"\n'); + + const result = server.call('provar.project.validate', { + project_path: root, + save_results: false, + }); + + assert.equal(isError(result), false); + const body = parseText(result); + const warnings = body['plan_integrity_warnings'] as string[] | undefined; + assert.ok( + Array.isArray(warnings) && warnings.length > 0, + 'Expected plan_integrity_warnings for missing .planitem' + ); + assert.ok(warnings[0].includes('smoke'), 'Warning should name the plan'); + assert.ok(warnings[0].includes('.planitem'), 'Warning should mention .planitem'); + }); + + it('reports plan_integrity_warnings when a suite dir is missing .planitem', () => { + const root = mktemp(); + writeFile(path.join(root, '.testproject'), TESTPROJECT_XML); + writeFile(path.join(root, 'tests', 'Login.testcase'), makeXml(G.tc1, G.s1)); + // Plan has .planitem but a suite subdir does not + writeFile( + path.join(root, 'plans', 'smoke', '.planitem'), + '' + ); + writeFile( + path.join(root, 'plans', 'smoke', 'SuiteA', 'Login.testinstance'), + 'testCasePath="tests/Login.testcase"\n' + ); + + const result = server.call('provar.project.validate', { + project_path: root, + save_results: false, + }); + + assert.equal(isError(result), false); + const body = parseText(result); + const warnings = body['plan_integrity_warnings'] as string[] | undefined; + assert.ok( + Array.isArray(warnings) && warnings.length > 0, + 'Expected plan_integrity_warnings for suite missing .planitem' + ); + assert.ok( + warnings.some((w) => w.includes('SuiteA')), + 'Warning should name the suite directory' + ); + }); + + it('does NOT include plan_integrity_warnings when all .planitem files are present', () => { + const root = mktemp(); + writeFile(path.join(root, '.testproject'), TESTPROJECT_XML); + writeFile(path.join(root, 'tests', 'Login.testcase'), makeXml(G.tc1, G.s1)); + writeFile( + path.join(root, 'plans', 'smoke', '.planitem'), + '' + ); + writeFile(path.join(root, 'plans', 'smoke', 'Login.testinstance'), 'testCasePath="tests/Login.testcase"\n'); + + const result = server.call('provar.project.validate', { + project_path: root, + save_results: false, + }); + + assert.equal(isError(result), false); + const body = parseText(result); + assert.ok( + !('plan_integrity_warnings' in body), + 'No plan_integrity_warnings expected when .planitem files are present' + ); + }); + }); }); diff --git a/test/unit/mcp/testCaseGenerate.test.ts b/test/unit/mcp/testCaseGenerate.test.ts index e3729beb..fba71dc1 100644 --- a/test/unit/mcp/testCaseGenerate.test.ts +++ b/test/unit/mcp/testCaseGenerate.test.ts @@ -99,7 +99,7 @@ describe('provar.testcase.generate', () => { }); describe('generated XML content', () => { - it('contains root element with name attribute', () => { + it('generates correct element structure per Provar requirements', () => { const result = server.call('provar.testcase.generate', { test_case_name: 'Create Account', steps: [], @@ -108,8 +108,11 @@ describe('provar.testcase.generate', () => { }); const xml = parseText(result)['xml_content'] as string; + assert.ok(xml.includes('standalone="no"'), 'Expected standalone="no" in XML declaration'); assert.ok(xml.includes(''), 'Expected as first child of testCase'); }); it('contains element', () => { @@ -138,18 +141,19 @@ describe('provar.testcase.generate', () => { assert.ok(UUID_RE.test(guidMatch[1]), `Expected UUID v4, got: ${guidMatch[1]}`); }); - it('uses explicit test_case_id when provided', () => { - const myId = 'my-explicit-id-123'; + it('always emits id="1" regardless of test_case_name (Provar integer literal requirement)', () => { const result = server.call('provar.testcase.generate', { test_case_name: 'Explicit ID Test', - test_case_id: myId, steps: [], dry_run: true, overwrite: false, }); const xml = parseText(result)['xml_content'] as string; - assert.ok(xml.includes(`id="${myId}"`), 'Expected explicit id in XML'); + assert.ok(xml.includes('id="1"'), 'Expected id="1" literal in testCase element'); + // Ensure the testCase id attr specifically is "1", not a UUID. + // Use word-boundary regex to avoid matching registryId="" or guid="". + assert.ok(!xml.match(/\btestCase\b[^>]*?\bid="[0-9a-f]{8}-/), 'testCase id must not be a UUID'); }); it('includes steps with correct apiId and sequential testItemId', () => { @@ -220,7 +224,7 @@ describe('provar.testcase.generate', () => { assert.ok(xml.includes('TODO'), 'Expected TODO placeholder for empty steps'); }); - it('escapes XML special characters in test_case_name', () => { + it('does not embed test_case_name in XML (name attr removed per Provar spec)', () => { const result = server.call('provar.testcase.generate', { test_case_name: 'Test & "Escape" ', steps: [], @@ -229,10 +233,9 @@ describe('provar.testcase.generate', () => { }); const xml = parseText(result)['xml_content'] as string; - assert.ok(xml.includes('&'), 'Expected & escaped to &'); - assert.ok(xml.includes('"'), 'Expected " escaped to "'); - assert.ok(xml.includes('<'), 'Expected < escaped to <'); - assert.ok(xml.includes('>'), 'Expected > escaped to >'); + // Provar derives test name from the file name — the name attr is not written to testCase + assert.ok(!xml.includes('name="Test'), 'testCase must NOT have a name attribute'); + assert.ok(!xml.includes('Test &'), 'escaped name must not appear in XML'); }); it('escapes XML special characters in step api_id and name', () => { @@ -479,7 +482,10 @@ describe('provar.testcase.generate', () => { const xml = parseText(result)['xml_content'] as string; assert.ok(xml.includes('class="uiTarget"'), 'Expected class="uiTarget"'); assert.ok(xml.includes('uri="sf:ui:target?'), 'Expected uri attribute with sf:ui:target value'); - assert.ok(!xml.includes('valueClass="string">sf:ui:target'), 'Must NOT emit uiTarget URI as a plain string value'); + assert.ok( + !xml.includes('valueClass="string">sf:ui:target'), + 'Must NOT emit uiTarget URI as a plain string value' + ); }); it('emits class="uiLocator" uri="..." for the locator argument', () => { @@ -500,7 +506,10 @@ describe('provar.testcase.generate', () => { const xml = parseText(result)['xml_content'] as string; assert.ok(xml.includes('class="uiLocator"'), 'Expected class="uiLocator"'); assert.ok(xml.includes('uri="sf:ui:locator:'), 'Expected uri attribute with locator value'); - assert.ok(!xml.includes('valueClass="string">sf:ui:locator'), 'Must NOT emit locator URI as a plain string value'); + assert.ok( + !xml.includes('valueClass="string">sf:ui:locator'), + 'Must NOT emit locator URI as a plain string value' + ); }); it('uiTarget also applies inside UiWithScreen wrapper when target_uri is non-SF', () => { @@ -515,7 +524,10 @@ describe('provar.testcase.generate', () => { const xml = parseText(result)['xml_content'] as string; assert.ok(xml.includes('class="uiTarget"'), 'Wrapper UiWithScreen target should use uiTarget class'); - assert.ok(xml.includes('uri="ui:pageobject:target?pageId=pageobjects.LoginPage"'), 'URI should appear as attribute'); + assert.ok( + xml.includes('uri="ui:pageobject:target?pageId=pageobjects.LoginPage"'), + 'URI should appear as attribute' + ); }); }); @@ -542,10 +554,7 @@ describe('provar.testcase.generate', () => { assert.ok(xml.includes(''), 'Expected namedValue for testCaseName'); assert.ok(xml.includes(''), 'Expected namedValue for testType'); assert.ok(xml.includes(''), 'Expected argument id="values"'); - assert.ok( - !xml.includes('testCaseName|TC_New'), - 'Must NOT emit pipe-delimited string for SetValues' - ); + assert.ok(!xml.includes('testCaseName|TC_New'), 'Must NOT emit pipe-delimited string for SetValues'); }); it('AssertValues uses flat argument structure (not valueList)', () => { @@ -572,9 +581,7 @@ describe('provar.testcase.generate', () => { it('non-SetValues steps still use flat argument structure', () => { const result = server.call('provar.testcase.generate', { test_case_name: 'Flat Args Test', - steps: [ - { api_id: 'ApexCreateObject', name: 'Create record', attributes: { objectApiName: 'Opportunity' } }, - ], + steps: [{ api_id: 'ApexCreateObject', name: 'Create record', attributes: { objectApiName: 'Opportunity' } }], dry_run: true, overwrite: false, validate_after_edit: false, diff --git a/test/unit/mcp/testCaseValidate.test.ts b/test/unit/mcp/testCaseValidate.test.ts index 279209b6..40ecb3af 100644 --- a/test/unit/mcp/testCaseValidate.test.ts +++ b/test/unit/mcp/testCaseValidate.test.ts @@ -23,8 +23,9 @@ const GUID_TC = '550e8400-e29b-41d4-a716-446655440000'; const GUID_S1 = '6ba7b810-9dad-4000-8000-00c04fd430c8'; const GUID_S2 = '6ba7b811-9dad-4001-9001-00c04fd430c8'; -const VALID_TC = ` - +const VALID_TC = ` + + @@ -38,8 +39,8 @@ describe('validateTestCase', () => { assert.equal(r.is_valid, true); assert.equal(r.error_count, 0); assert.equal(r.step_count, 2); - assert.equal(r.test_case_id, 'test-001'); - assert.equal(r.test_case_name, 'My Test'); + assert.equal(r.test_case_id, '1'); + assert.equal(r.test_case_name, null); // name attr is absent per Provar spec }); }); @@ -81,6 +82,16 @@ describe('validateTestCase', () => { ); }); + it('TC_010: flags non-"1" id (e.g. UUID used as id)', () => { + const r = validateTestCase( + `` + ); + assert.ok( + r.issues.some((i) => i.rule_id === 'TC_010'), + 'Expected TC_010 for UUID used as id' + ); + }); + it('TC_011: flags missing guid', () => { const r = validateTestCase( '' @@ -841,8 +852,8 @@ describe('registerTestCaseValidate handler', () => { assert.equal(result['is_valid'], true); assert.equal(result['quality_score'], 90); // Metadata extracted from XML locally and merged into the API response - assert.equal(result['test_case_id'], 'test-001'); - assert.equal(result['test_case_name'], 'My Test'); + assert.equal(result['test_case_id'], '1'); // id="1" per Provar spec + assert.equal(result['test_case_name'], null); // name attr absent per Provar spec assert.equal(result['step_count'], 2); }); From 5eade0e098c055a9971a3571c2112d3a2d7d538f Mon Sep 17 00:00:00 2001 From: Michael Dailey Date: Mon, 4 May 2026 15:22:04 -0500 Subject: [PATCH 13/19] PDX-0: fix: pre-landing review fixes for fix/session4-feedback RCA: Post-commit review identified three issues: buildTestCaseIdMap included id in the attribute loop causing id="1" collisions across all test cases; readSuiteDirectory lacked planIntegrityWarnings propagation so .planitem checks only reached one suite depth; makeProject test fixture omitted .planitem making happy-path tests run against an integrity-warning project. Fix: Removed id from buildTestCaseIdMap attr loop (registryId and guid are UUID-unique, id="1" is not); extended readSuiteDirectory signature with planIntegrityWarnings and propagated it through recursive calls and the readPlanDirectory call site; added .planitem to makeProject fixture; added TC_010 negative assertion and nested-suite .planitem test. 843 tests passing. Co-Authored-By: Claude Sonnet 4.6 --- src/services/projectValidation.ts | 19 +++++++-- test/unit/mcp/projectValidateFromPath.test.ts | 41 +++++++++++++++++++ test/unit/mcp/testCaseValidate.test.ts | 5 +++ 3 files changed, 61 insertions(+), 4 deletions(-) diff --git a/src/services/projectValidation.ts b/src/services/projectValidation.ts index 9bb183c0..a1af1eff 100644 --- a/src/services/projectValidation.ts +++ b/src/services/projectValidation.ts @@ -305,7 +305,8 @@ export function readSuiteDirectory( projectPath: string, depth = 0, coveredPaths?: Set, - idMap?: Map + idMap?: Map, + planIntegrityWarnings?: string[] ): TestSuiteInput { const testCases: TestCaseInput[] = []; const testSuites: TestSuiteInput[] = []; @@ -318,7 +319,15 @@ export function readSuiteDirectory( if (entry.name === 'node_modules') continue; const fullPath = path.join(dirPath, entry.name); if (entry.isDirectory() && !entry.name.startsWith('.')) { - testSuites.push(readSuiteDirectory(fullPath, entry.name, projectPath, depth + 1, coveredPaths, idMap)); + if (planIntegrityWarnings && !fs.existsSync(path.join(fullPath, '.planitem'))) { + const rel = path.relative(projectPath, fullPath).replace(/\\/g, '/'); + planIntegrityWarnings.push( + `${rel}/ is missing a .planitem file — test instances in this suite will be invisible to the runner.` + ); + } + testSuites.push( + readSuiteDirectory(fullPath, entry.name, projectPath, depth + 1, coveredPaths, idMap, planIntegrityWarnings) + ); } else if (entry.name.endsWith('.testinstance')) { const { testCase, testCasePath, testCaseId } = resolveTestInstanceFull(fullPath, projectPath); if (testCase) testCases.push(testCase); @@ -355,7 +364,9 @@ export function readPlanDirectory( `${rel}/ is missing a .planitem file — test instances in this suite will be invisible to the runner.` ); } - testSuites.push(readSuiteDirectory(fullPath, entry.name, projectPath, 0, coveredPaths, idMap)); + testSuites.push( + readSuiteDirectory(fullPath, entry.name, projectPath, 0, coveredPaths, idMap, planIntegrityWarnings) + ); } else if (entry.name.endsWith('.testinstance')) { const { testCase, testCasePath, testCaseId } = resolveTestInstanceFull(fullPath, projectPath); if (testCase) testCases.push(testCase); @@ -424,7 +435,7 @@ function buildTestCaseIdMap(projectPath: string): Map { try { const content = fs.readFileSync(fullPath, 'utf-8'); const rel = path.relative(projectPath, fullPath).replace(/\\/g, '/'); - for (const attr of ['registryId', 'id', 'guid'] as const) { + for (const attr of ['registryId', 'guid'] as const) { const m = content.match(new RegExp(`${attr}=["']([^"']+)["']`)); if (m?.[1] && !idMap.has(m[1])) idMap.set(m[1], rel); } diff --git a/test/unit/mcp/projectValidateFromPath.test.ts b/test/unit/mcp/projectValidateFromPath.test.ts index 08e0a8eb..d920ac0a 100644 --- a/test/unit/mcp/projectValidateFromPath.test.ts +++ b/test/unit/mcp/projectValidateFromPath.test.ts @@ -77,6 +77,10 @@ function makeProject(root: string, planName = 'smoke', tcName = 'Login'): void { writeFile(path.join(root, '.testproject'), TESTPROJECT_XML); writeFile(path.join(root, 'tests', `${tcName}.testcase`), makeXml(G.tc1, G.s1)); writeFile(path.join(root, 'plans', planName, `${tcName}.testinstance`), `testCasePath="tests/${tcName}.testcase"\n`); + writeFile( + path.join(root, 'plans', planName, '.planitem'), + '' + ); } // ── Test setup ───────────────────────────────────────────────────────────────── @@ -399,5 +403,42 @@ describe('provar.project.validate (from path)', () => { 'No plan_integrity_warnings expected when .planitem files are present' ); }); + + it('reports plan_integrity_warnings for nested suite dir (depth ≥ 2) missing .planitem', () => { + const root = mktemp(); + writeFile(path.join(root, '.testproject'), TESTPROJECT_XML); + writeFile(path.join(root, 'tests', 'Login.testcase'), makeXml(G.tc1, G.s1)); + // Plan and SuiteA have .planitem, but SuiteA/SubSuite does not + writeFile( + path.join(root, 'plans', 'smoke', '.planitem'), + '' + ); + writeFile( + path.join(root, 'plans', 'smoke', 'SuiteA', '.planitem'), + '' + ); + // SubSuite exists but lacks .planitem — instances here would be invisible to runner + writeFile( + path.join(root, 'plans', 'smoke', 'SuiteA', 'SubSuite', 'Login.testinstance'), + 'testCasePath="tests/Login.testcase"\n' + ); + + const result = server.call('provar.project.validate', { + project_path: root, + save_results: false, + }); + + assert.equal(isError(result), false); + const body = parseText(result); + const warnings = body['plan_integrity_warnings'] as string[] | undefined; + assert.ok( + Array.isArray(warnings) && warnings.length > 0, + 'Expected plan_integrity_warnings for nested suite missing .planitem' + ); + assert.ok( + warnings.some((w) => w.includes('SubSuite')), + 'Warning should name the nested suite directory' + ); + }); }); }); diff --git a/test/unit/mcp/testCaseValidate.test.ts b/test/unit/mcp/testCaseValidate.test.ts index 40ecb3af..57f10bcf 100644 --- a/test/unit/mcp/testCaseValidate.test.ts +++ b/test/unit/mcp/testCaseValidate.test.ts @@ -92,6 +92,11 @@ describe('validateTestCase', () => { ); }); + it('TC_010: does not fire when id="1" (the correct literal)', () => { + const r = validateTestCase(VALID_TC); + assert.ok(!r.issues.some((i) => i.rule_id === 'TC_010'), 'TC_010 must not fire when id="1"'); + }); + it('TC_011: flags missing guid', () => { const r = validateTestCase( '' From 0e123a3ed0a584cfca5a3995402ca49a5f685506 Mon Sep 17 00:00:00 2001 From: Michael Dailey Date: Mon, 4 May 2026 15:29:24 -0500 Subject: [PATCH 14/19] PDX-0: fix: preserve id-attr UUID lookup for legacy projects in buildTestCaseIdMap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RCA: Adversarial review caught that removing id from the attribute scan breaks legacy Provar projects where testinstance testCaseId fields reference the old UUID stored in id — even though new generated test cases emit id="1", existing projects on disk still carry UUID ids. Fix: Restore id to the scan loop but skip the literal value "1" which is non-unique. Legacy UUID resolution is preserved; the id="1" collision that prompted the removal is eliminated by the value guard. Co-Authored-By: Claude Sonnet 4.6 --- src/services/projectValidation.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/services/projectValidation.ts b/src/services/projectValidation.ts index a1af1eff..d728ff77 100644 --- a/src/services/projectValidation.ts +++ b/src/services/projectValidation.ts @@ -435,9 +435,10 @@ function buildTestCaseIdMap(projectPath: string): Map { try { const content = fs.readFileSync(fullPath, 'utf-8'); const rel = path.relative(projectPath, fullPath).replace(/\\/g, '/'); - for (const attr of ['registryId', 'guid'] as const) { + for (const attr of ['registryId', 'id', 'guid'] as const) { const m = content.match(new RegExp(`${attr}=["']([^"']+)["']`)); - if (m?.[1] && !idMap.has(m[1])) idMap.set(m[1], rel); + // Skip id="1" — new generator emits this literal and it is not UUID-unique + if (m?.[1] && m[1] !== '1' && !idMap.has(m[1])) idMap.set(m[1], rel); } } catch { /* skip */ From a027c6fa5073ad42c36a46b983216642d9d17faa Mon Sep 17 00:00:00 2001 From: Michael Dailey Date: Mon, 4 May 2026 15:54:20 -0500 Subject: [PATCH 15/19] PDX-0: fix(coverage): exclude planitem-less dirs from coverage count; fix test fixtures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RCA: readPlansDir/readPlanDirectory/readSuiteDirectory counted test instances in directories missing .planitem as covered, inflating coverage score — Provar runner silently ignores those directories entirely. Fix: gate coveredPaths/idMap accumulation on presence of .planitem at each level; update test fixtures in both mcp and service test files to include .planitem so coverage assertions remain accurate. Co-Authored-By: Claude Sonnet 4.6 --- src/services/projectValidation.ts | 40 ++++++-- test/unit/services/projectValidation.test.ts | 96 +++++++++++++++----- 2 files changed, 107 insertions(+), 29 deletions(-) diff --git a/src/services/projectValidation.ts b/src/services/projectValidation.ts index d728ff77..2d26abc0 100644 --- a/src/services/projectValidation.ts +++ b/src/services/projectValidation.ts @@ -319,14 +319,23 @@ export function readSuiteDirectory( if (entry.name === 'node_modules') continue; const fullPath = path.join(dirPath, entry.name); if (entry.isDirectory() && !entry.name.startsWith('.')) { - if (planIntegrityWarnings && !fs.existsSync(path.join(fullPath, '.planitem'))) { + const subHasPlanItem = fs.existsSync(path.join(fullPath, '.planitem')); + if (planIntegrityWarnings && !subHasPlanItem) { const rel = path.relative(projectPath, fullPath).replace(/\\/g, '/'); planIntegrityWarnings.push( `${rel}/ is missing a .planitem file — test instances in this suite will be invisible to the runner.` ); } testSuites.push( - readSuiteDirectory(fullPath, entry.name, projectPath, depth + 1, coveredPaths, idMap, planIntegrityWarnings) + readSuiteDirectory( + fullPath, + entry.name, + projectPath, + depth + 1, + subHasPlanItem ? coveredPaths : undefined, + subHasPlanItem ? idMap : undefined, + planIntegrityWarnings + ) ); } else if (entry.name.endsWith('.testinstance')) { const { testCase, testCasePath, testCaseId } = resolveTestInstanceFull(fullPath, projectPath); @@ -358,14 +367,23 @@ export function readPlanDirectory( if (entry.name === 'node_modules') continue; const fullPath = path.join(planPath, entry.name); if (entry.isDirectory() && !entry.name.startsWith('.')) { - if (planIntegrityWarnings && !fs.existsSync(path.join(fullPath, '.planitem'))) { + const suiteHasPlanItem = fs.existsSync(path.join(fullPath, '.planitem')); + if (planIntegrityWarnings && !suiteHasPlanItem) { const rel = path.relative(projectPath, fullPath).replace(/\\/g, '/'); planIntegrityWarnings.push( `${rel}/ is missing a .planitem file — test instances in this suite will be invisible to the runner.` ); } testSuites.push( - readSuiteDirectory(fullPath, entry.name, projectPath, 0, coveredPaths, idMap, planIntegrityWarnings) + readSuiteDirectory( + fullPath, + entry.name, + projectPath, + 0, + suiteHasPlanItem ? coveredPaths : undefined, + suiteHasPlanItem ? idMap : undefined, + planIntegrityWarnings + ) ); } else if (entry.name.endsWith('.testinstance')) { const { testCase, testCasePath, testCaseId } = resolveTestInstanceFull(fullPath, projectPath); @@ -400,12 +418,22 @@ export function readPlansDir(projectPath: string): { for (const entry of entries) { if (!entry.isDirectory() || entry.name.startsWith('.') || entry.name === 'node_modules') continue; const planPath = path.join(plansDir, entry.name); - if (!fs.existsSync(path.join(planPath, '.planitem'))) { + const planHasPlanItem = fs.existsSync(path.join(planPath, '.planitem')); + if (!planHasPlanItem) { planIntegrityWarnings.push( `plans/${entry.name}/ is missing a .planitem file — this plan will not be recognised by the Provar runner.` ); } - plans.push(readPlanDirectory(planPath, entry.name, projectPath, coveredPaths, idMap, planIntegrityWarnings)); + plans.push( + readPlanDirectory( + planPath, + entry.name, + projectPath, + planHasPlanItem ? coveredPaths : undefined, + planHasPlanItem ? idMap : undefined, + planIntegrityWarnings + ) + ); } } catch { /* skip */ diff --git a/test/unit/services/projectValidation.test.ts b/test/unit/services/projectValidation.test.ts index 3e56d51a..2e42e95d 100644 --- a/test/unit/services/projectValidation.test.ts +++ b/test/unit/services/projectValidation.test.ts @@ -33,8 +33,8 @@ const VALID_TC_XML = (guid: string, stepGuid: string, id: string): string => ` @@ -72,13 +72,23 @@ function makeProject(root: string, planName = 'smoke', tcName = 'Login'): void { // plans//.testinstance const instancePath = path.join(root, 'plans', planName, `${tcName}.testinstance`); writeFile(instancePath, `testCasePath="tests/${tcName}.testcase"\n`); + + // plans//.planitem — required for runner to recognise this plan + writeFile( + path.join(root, 'plans', planName, '.planitem'), + '' + ); } // ── Cleanup ─────────────────────────────────────────────────────────────────── afterEach(() => { for (const dir of tempDirs.splice(0)) { - try { fs.rmSync(dir, { recursive: true, force: true }); } catch { /* skip */ } + try { + fs.rmSync(dir, { recursive: true, force: true }); + } catch { + /* skip */ + } } }); @@ -158,7 +168,11 @@ describe('readProjectContext', () => { } catch { // If chmod doesn't work (Windows), skip the test body — graceful skip } finally { - try { fs.chmodSync(tpPath, 0o644); } catch { /* skip */ } + try { + fs.chmodSync(tpPath, 0o644); + } catch { + /* skip */ + } } }); @@ -166,11 +180,12 @@ describe('readProjectContext', () => { const root = mktemp(); writeFile(path.join(root, '.testproject'), TESTPROJECT_XML); // 2 unencrypted values, 1 encrypted, 1 encryptor check line - writeFile(path.join(root, '.secrets'), + writeFile( + path.join(root, '.secrets'), 'Encryptor.check=ENC1(abc123)\n' + - 'password=plaintext\n' + - 'apiKey=ENC1(encrypted)\n' + - 'token=another_plaintext\n' + 'password=plaintext\n' + + 'apiKey=ENC1(encrypted)\n' + + 'token=another_plaintext\n' ); const { context } = readProjectContext(root); assert.equal(context.secretsPasswordSet, true); @@ -230,19 +245,49 @@ describe('resolveTestInstance', () => { // ── toQualityTier + toQualityGrade ──────────────────────────────────────────── describe('toQualityTier', () => { - it('returns S for score >= 95', () => { assert.equal(toQualityTier(100), 'S'); assert.equal(toQualityTier(95), 'S'); }); - it('returns A for score 85-94', () => { assert.equal(toQualityTier(90), 'A'); assert.equal(toQualityTier(85), 'A'); }); - it('returns B for score 75-84', () => { assert.equal(toQualityTier(80), 'B'); assert.equal(toQualityTier(75), 'B'); }); - it('returns C for score 65-74', () => { assert.equal(toQualityTier(70), 'C'); assert.equal(toQualityTier(65), 'C'); }); - it('returns D for score < 65', () => { assert.equal(toQualityTier(64), 'D'); assert.equal(toQualityTier(0), 'D'); }); + it('returns S for score >= 95', () => { + assert.equal(toQualityTier(100), 'S'); + assert.equal(toQualityTier(95), 'S'); + }); + it('returns A for score 85-94', () => { + assert.equal(toQualityTier(90), 'A'); + assert.equal(toQualityTier(85), 'A'); + }); + it('returns B for score 75-84', () => { + assert.equal(toQualityTier(80), 'B'); + assert.equal(toQualityTier(75), 'B'); + }); + it('returns C for score 65-74', () => { + assert.equal(toQualityTier(70), 'C'); + assert.equal(toQualityTier(65), 'C'); + }); + it('returns D for score < 65', () => { + assert.equal(toQualityTier(64), 'D'); + assert.equal(toQualityTier(0), 'D'); + }); }); describe('toQualityGrade', () => { - it('returns Excellent for score >= 95', () => { assert.equal(toQualityGrade(100), 'Excellent'); assert.equal(toQualityGrade(95), 'Excellent'); }); - it('returns Great for score 90-94', () => { assert.equal(toQualityGrade(90), 'Great'); assert.equal(toQualityGrade(92), 'Great'); }); - it('returns Good for score 80-89', () => { assert.equal(toQualityGrade(80), 'Good'); assert.equal(toQualityGrade(85), 'Good'); }); - it('returns Fair for score 70-79', () => { assert.equal(toQualityGrade(70), 'Fair'); assert.equal(toQualityGrade(75), 'Fair'); }); - it('returns Poor for score < 70', () => { assert.equal(toQualityGrade(69), 'Poor'); assert.equal(toQualityGrade(0), 'Poor'); }); + it('returns Excellent for score >= 95', () => { + assert.equal(toQualityGrade(100), 'Excellent'); + assert.equal(toQualityGrade(95), 'Excellent'); + }); + it('returns Great for score 90-94', () => { + assert.equal(toQualityGrade(90), 'Great'); + assert.equal(toQualityGrade(92), 'Great'); + }); + it('returns Good for score 80-89', () => { + assert.equal(toQualityGrade(80), 'Good'); + assert.equal(toQualityGrade(85), 'Good'); + }); + it('returns Fair for score 70-79', () => { + assert.equal(toQualityGrade(70), 'Fair'); + assert.equal(toQualityGrade(75), 'Fair'); + }); + it('returns Poor for score < 70', () => { + assert.equal(toQualityGrade(69), 'Poor'); + assert.equal(toQualityGrade(0), 'Poor'); + }); }); // ── validateProjectFromPath ─────────────────────────────────────────────────── @@ -372,7 +417,10 @@ describe('validateProjectFromPath', () => { assert.ok(result.coverage.uncovered_test_cases.some((p) => p.includes('Orphan'))); assert.equal(result.coverage.total_test_cases_on_disk, 2); // covered + uncovered must always sum to total - assert.equal(result.coverage.covered_by_plans + result.coverage.uncovered_count, result.coverage.total_test_cases_on_disk); + assert.equal( + result.coverage.covered_by_plans + result.coverage.uncovered_count, + result.coverage.total_test_cases_on_disk + ); }); it('coverage: UUID-based match marks test as covered when testCasePath is absent but testCaseId matches registryId', () => { @@ -390,7 +438,11 @@ describe('validateProjectFromPath', () => { // We simulate a mismatch by omitting testCasePath and relying solely on testCaseId writeFile( path.join(root, 'plans', 'smoke', 'UuidTest.testinstance'), - `testCaseId="${callableRegId}"\n` // no testCasePath — UUID fallback must cover it + `testCaseId="${callableRegId}"\n` // no testCasePath — UUID fallback must cover it + ); + writeFile( + path.join(root, 'plans', 'smoke', '.planitem'), + '' ); const result = validateProjectFromPath({ project_path: root, save_results: false }); assert.equal(result.coverage.covered_by_plans, 1, 'UUID-based match should count UuidTest as covered'); @@ -436,9 +488,7 @@ describe('buildQhReport', () => { // The saved report has the QH structure — verify by re-running with save_results: true const withSave = validateProjectFromPath({ project_path: root, save_results: true }); assert.ok(withSave.saved_to !== null); - const report = JSON.parse( - fs.readFileSync(path.join(root, withSave.saved_to), 'utf-8') - ) as Record; + const report = JSON.parse(fs.readFileSync(path.join(root, withSave.saved_to), 'utf-8')) as Record; assert.ok(report.reportInfo, 'missing reportInfo'); assert.ok(report.summary, 'missing summary'); assert.ok(report.context, 'missing context'); From 03c470d0b385fc094a8ca908bbb14b6bfafee7ac Mon Sep 17 00:00:00 2001 From: Michael Dailey Date: Mon, 4 May 2026 15:55:47 -0500 Subject: [PATCH 16/19] PDX-0: increment version to 1.5.0-beta.14 RCA: session 4 feedback fixes (A3 testCase XML, A5 ApexSoqlQuery docs, E2 planitem integrity, E3/E4/C1 tool description improvements) require a new beta release. Fix: bump package.json and server.json version fields from 1.5.0-beta.13 to 1.5.0-beta.14 to mark this release boundary. Co-Authored-By: Claude Sonnet 4.6 --- package.json | 2 +- server.json | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/package.json b/package.json index c9ccd879..44aa9b2a 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "@provartesting/provardx-cli", "description": "A plugin for the Salesforce CLI to orchestrate testing activities and report quality metrics to Provar Quality Hub", - "version": "1.5.0-beta.13", + "version": "1.5.0-beta.14", "mcpName": "io.github.ProvarTesting/provar", "license": "BSD-3-Clause", "plugins": [ diff --git a/server.json b/server.json index 96d72334..8515d5e0 100644 --- a/server.json +++ b/server.json @@ -14,12 +14,12 @@ "url": "https://github.com/ProvarTesting/provardx-cli", "source": "github" }, - "version": "1.5.0-beta.13", + "version": "1.5.0-beta.14", "packages": [ { "registryType": "npm", "identifier": "@provartesting/provardx-cli", - "version": "1.5.0-beta.13", + "version": "1.5.0-beta.14", "transport": { "type": "stdio" }, From 14a4f4d3be514a26aef06cb07d7634bbe7009354 Mon Sep 17 00:00:00 2001 From: Michael Dailey Date: Mon, 4 May 2026 16:06:08 -0500 Subject: [PATCH 17/19] PDX-0: feat(mcp): auto-update check at MCP startup (v1.5.0-beta.14) RCA: Users on stale plugin versions get silent failures or missing tools with no indication an update exists. Fix: Add checkForUpdate() called at startup before server.connect(); writes stderr notification and optionally auto-installs via --auto-update flag. --- docs/mcp.md | 33 ++- messages/sf.provar.mcp.start.md | 132 +++++++----- package.json | 2 +- server.json | 4 +- src/commands/provar/mcp/start.ts | 20 +- src/mcp/server.ts | 14 +- src/mcp/update/updateChecker.ts | 259 ++++++++++++++++++++++++ test/unit/mcp/updateChecker.test.ts | 302 ++++++++++++++++++++++++++++ 8 files changed, 699 insertions(+), 67 deletions(-) create mode 100644 src/mcp/update/updateChecker.ts create mode 100644 test/unit/mcp/updateChecker.test.ts diff --git a/docs/mcp.md b/docs/mcp.md index 3366c616..3a503e9f 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -143,9 +143,11 @@ The server communicates over **stdio** (standard input / output). It must be sta ### Flags -| Flag | Alias | Default | Description | -| ----------------- | ----- | ------------------------- | ----------------------------------------------------------------------------------------------------------------- | -| `--allowed-paths` | `-a` | Current working directory | Base directories that file-system tools are permitted to read and write. Repeat the flag to allow multiple paths. | +| Flag | Alias | Default | Description | +| ------------------- | ----- | ------------------------- | ------------------------------------------------------------------------------------------------------------------------------------ | +| `--allowed-paths` | `-a` | Current working directory | Base directories that file-system tools are permitted to read and write. Repeat the flag to allow multiple paths. | +| `--auto-update` | | false | Automatically installs the latest version at startup and restarts the MCP connection. Skipped if running from a development symlink. | +| `--no-update-check` | | false | Skip the startup update check. Also controlled by the `PROVAR_NO_UPDATE_CHECK` environment variable. | ```sh # Allow access to a specific project directory @@ -485,7 +487,16 @@ A lightweight sanity-check tool. Echoes back the message you send. Useful for ve | --------- | ------ | -------- | --------------------- | | `message` | string | no | Any text to echo back | -**Output** — `{ message: string }` +**Output** + +| Field | Type | Description | +| ----------------- | ------- | --------------------------------------------------------- | --------------------------------------------------- | +| `pong` | string | The echoed message | +| `ts` | string | ISO-8601 timestamp | +| `server` | string | Server name and version (e.g. `provar-mcp@1.5.0-beta.10`) | +| `updateAvailable` | boolean | Whether a newer version is available in the registry | +| `latestVersion` | string | null | Latest version found in the npm registry, or `null` | +| `updateCommand` | string | null | Command to run to update the plugin, or `null` | --- @@ -639,13 +650,13 @@ Generates an XML test case skeleton with UUID v4 guids and sequential `testItemI **Argument XML conventions** (automatically applied by the generator): -| Argument key / value pattern | Emitted XML class | API context | -| ------------------------------------ | ----------------------------- | ----------------------------------- | -| `target` key | `class="uiTarget"` | UiWithScreen, UiWithRow | -| `locator` key | `class="uiLocator"` | UiDoAction, UiAssert | -| Value matches `{VarName}` or `{A.B}` | `class="variable"` + `` | Any step | -| SetValues attributes | `class="valueList"/` | SetValues only | -| All other values | `class="value" valueClass="string"` | Any step | +| Argument key / value pattern | Emitted XML class | API context | +| ------------------------------------ | ----------------------------------- | ----------------------- | +| `target` key | `class="uiTarget"` | UiWithScreen, UiWithRow | +| `locator` key | `class="uiLocator"` | UiDoAction, UiAssert | +| Value matches `{VarName}` or `{A.B}` | `class="variable"` + `` | Any step | +| SetValues attributes | `class="valueList"/` | SetValues only | +| All other values | `class="value" valueClass="string"` | Any step | AssertValues uses **flat** argument structure (`expectedValue`, `actualValue`, `comparisonType`) — not the `valueList`/namedValues format. diff --git a/messages/sf.provar.mcp.start.md b/messages/sf.provar.mcp.start.md index eb8f0b32..5c8863b5 100644 --- a/messages/sf.provar.mcp.start.md +++ b/messages/sf.provar.mcp.start.md @@ -1,71 +1,107 @@ # summary + Start a local MCP server for Provar tools over stdio transport. # description + Launches a stateless MCP (Model Context Protocol) server that exposes Provar tools to AI assistants (Claude Desktop, Claude Code, Cursor) via stdio transport. All MCP JSON-RPC communication happens over stdout; all internal logging goes to stderr. Available tools: - Project & inspection: - - provar.project.inspect — inspect project folder inventory - - provar.project.validate — validate full project from disk: coverage, quality scores - - Page Object: - - provar.pageobject.generate — generate a Java Page Object skeleton - - provar.pageobject.validate — validate Page Object quality and naming - - Test Case: - - provar.testcase.generate — generate an XML test case skeleton - - provar.testcase.validate — validate test case XML (validity + best-practices scores) - - Test Suite / Plan: - - provar.testsuite.validate — validate test suite hierarchy - - provar.testplan.validate — validate test plan metadata completeness - - provar.testplan.create-suite — create a test suite under a plan - - provar.testplan.add-instance — add a test instance to a plan - - provar.testplan.remove-instance — remove a test instance from a plan - - Properties files: - - provar.properties.read — read a Provar properties file - - provar.properties.set — set a key in a Provar properties file - - provar.properties.validate — validate a properties file structure - - provar.properties.generate — generate a properties file skeleton - - Quality Hub (sf provar quality-hub wrappers): - - provar.qualityhub.connect — connect to a Quality Hub org - - provar.qualityhub.display — display connected org info - - provar.qualityhub.testrun — trigger a Quality Hub test run - - provar.qualityhub.testrun.report — poll test run status - - provar.qualityhub.testrun.abort — abort a running test run - - provar.qualityhub.testcase.retrieve — retrieve test case results - - provar.qualityhub.defect.create — create defects for failed test executions - - Automation (sf provar automation wrappers): - - provar.automation.setup — set up the Provar Automation runtime - - provar.automation.metadata.download — download Salesforce metadata - - provar.automation.compile — compile Provar test assets - - provar.automation.testrun — run Provar tests - - provar.automation.config.load — load a Provar configuration - - ANT build: - - provar.ant.generate — generate an ANT build.xml - - provar.ant.validate — validate an ANT build.xml - - Test result analysis: - - provar.testrun.rca — root cause analysis on a test result - - provar.testrun.report.locate — locate a test result report +Project & inspection: + +- provar.project.inspect — inspect project folder inventory +- provar.project.validate — validate full project from disk: coverage, quality scores + +Page Object: + +- provar.pageobject.generate — generate a Java Page Object skeleton +- provar.pageobject.validate — validate Page Object quality and naming + +Test Case: + +- provar.testcase.generate — generate an XML test case skeleton +- provar.testcase.validate — validate test case XML (validity + best-practices scores) +- provar.testcase.step.edit — atomically add or remove a single step in a test case + +Test Suite / Plan: + +- provar.testsuite.validate — validate test suite hierarchy +- provar.testplan.validate — validate test plan metadata completeness +- provar.testplan.create-suite — create a test suite under a plan +- provar.testplan.add-instance — add a test instance to a plan +- provar.testplan.remove-instance — remove a test instance from a plan + +Properties files: + +- provar.properties.read — read a Provar properties file +- provar.properties.set — set a key in a Provar properties file +- provar.properties.validate — validate a properties file structure +- provar.properties.generate — generate a properties file skeleton + +Quality Hub (sf provar quality-hub wrappers): + +- provar.qualityhub.connect — connect to a Quality Hub org +- provar.qualityhub.display — display connected org info +- provar.qualityhub.testrun — trigger a Quality Hub test run +- provar.qualityhub.testrun.report — poll test run status +- provar.qualityhub.testrun.abort — abort a running test run +- provar.qualityhub.testcase.retrieve — retrieve test case results +- provar.qualityhub.defect.create — create defects for failed test executions +- provar.qualityhub.examples.retrieve — retrieve corpus examples for a given step type + +Automation (sf provar automation wrappers): + +- provar.automation.setup — set up the Provar Automation runtime +- provar.automation.metadata.download — download Salesforce metadata +- provar.automation.compile — compile Provar test assets +- provar.automation.testrun — run Provar tests +- provar.automation.config.load — load a Provar configuration + +ANT build: + +- provar.ant.generate — generate an ANT build.xml +- provar.ant.validate — validate an ANT build.xml + +Test result analysis: + +- provar.testrun.rca — root cause analysis on a test result +- provar.testrun.report.locate — locate a test result report + +NitroX (Provar NitroX component tools): + +- provar.nitrox.discover — discover NitroX component metadata +- provar.nitrox.generate — generate a NitroX component +- provar.nitrox.patch — patch a NitroX component definition +- provar.nitrox.read — read a NitroX component definition +- provar.nitrox.validate — validate a NitroX component + +Connections: + +- provar.connection.list — list connections and named environments from .testproject For full tool documentation see docs/mcp.md in this repository. # flags.allowed-paths.summary + Allowed base directory paths for file operations. Defaults to current directory. # flags.auto-defects.summary + When enabled, testrun.report suggestions will prompt defect creation on failures. +# flags.auto-update.summary + +When enabled, automatically installs the latest version of the plugin and restarts the MCP connection on startup. + +# flags.no-update-check.summary + +Skip the update check at startup. Also controlled by the PROVAR_NO_UPDATE_CHECK environment variable. + # examples + - Start MCP server (accepts stdio connections from Claude Desktop / Cursor): <%= config.bin %> <%= command.id %> - Start with explicit allowed paths: diff --git a/package.json b/package.json index 8f459891..44aa9b2a 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "@provartesting/provardx-cli", "description": "A plugin for the Salesforce CLI to orchestrate testing activities and report quality metrics to Provar Quality Hub", - "version": "1.5.0-beta.12", + "version": "1.5.0-beta.14", "mcpName": "io.github.ProvarTesting/provar", "license": "BSD-3-Clause", "plugins": [ diff --git a/server.json b/server.json index c23758f0..8515d5e0 100644 --- a/server.json +++ b/server.json @@ -14,12 +14,12 @@ "url": "https://github.com/ProvarTesting/provardx-cli", "source": "github" }, - "version": "1.5.0-beta.12", + "version": "1.5.0-beta.14", "packages": [ { "registryType": "npm", "identifier": "@provartesting/provardx-cli", - "version": "1.5.0-beta.12", + "version": "1.5.0-beta.14", "transport": { "type": "stdio" }, diff --git a/src/commands/provar/mcp/start.ts b/src/commands/provar/mcp/start.ts index 338aabe1..91f2040b 100644 --- a/src/commands/provar/mcp/start.ts +++ b/src/commands/provar/mcp/start.ts @@ -10,6 +10,7 @@ import { Messages } from '@provartesting/provardx-plugins-utils'; import { StdioServerTransport } from '@modelcontextprotocol/sdk/server/stdio.js'; import { createProvarMcpServer } from '../../../mcp/server.js'; import { validateLicense, LicenseError } from '../../../mcp/licensing/index.js'; +import { checkForUpdate } from '../../../mcp/update/updateChecker.js'; Messages.importMessagesDirectoryFromMetaUrl(import.meta.url); const messages = Messages.loadMessages('@provartesting/provardx-cli', 'sf.provar.mcp.start'); @@ -36,6 +37,14 @@ export default class SfProvarMcpStart extends SfCommand { summary: messages.getMessage('flags.auto-defects.summary'), default: false, }), + 'auto-update': Flags.boolean({ + summary: messages.getMessage('flags.auto-update.summary'), + default: false, + }), + 'no-update-check': Flags.boolean({ + summary: messages.getMessage('flags.no-update-check.summary'), + default: false, + }), }; public async run(): Promise { @@ -51,9 +60,7 @@ export default class SfProvarMcpStart extends SfCommand { try { const result = await validateLicense(); if (result.offlineGrace) { - process.stderr.write( - '[provar-mcp] Warning: license validated from offline cache (last checked > 2h ago).\n' - ); + process.stderr.write('[provar-mcp] Warning: license validated from offline cache (last checked > 2h ago).\n'); } } catch (err: unknown) { if (err instanceof LicenseError) { @@ -62,7 +69,12 @@ export default class SfProvarMcpStart extends SfCommand { throw err; } - const server = createProvarMcpServer({ allowedPaths }); + const updateResult = await checkForUpdate({ + noUpdateCheck: flags['no-update-check'], + autoUpdate: flags['auto-update'], + }); + + const server = createProvarMcpServer({ allowedPaths, updateResult }); const transport = new StdioServerTransport(); // Connect hands stdin/stdout ownership to the SDK. diff --git a/src/mcp/server.ts b/src/mcp/server.ts index 27b889fb..adb2ad7f 100644 --- a/src/mcp/server.ts +++ b/src/mcp/server.ts @@ -38,6 +38,11 @@ import { registerAllPrompts } from './prompts/index.js'; export interface ServerConfig { allowedPaths: string[]; + updateResult?: { + updateAvailable: boolean; + latestVersion: string | null; + updateCommand: string | null; + }; } export function createProvarMcpServer(config: ServerConfig): McpServer { @@ -56,7 +61,14 @@ export function createProvarMcpServer(config: ServerConfig): McpServer { message: z.string().optional().default('ping').describe('Optional message to echo back'), }, ({ message }) => { - const result = { pong: message, ts: new Date().toISOString(), server: `provar-mcp@${SERVER_VERSION}` }; + const result = { + pong: message, + ts: new Date().toISOString(), + server: `provar-mcp@${SERVER_VERSION}`, + updateAvailable: config.updateResult?.updateAvailable ?? false, + latestVersion: config.updateResult?.latestVersion ?? null, + updateCommand: config.updateResult?.updateCommand ?? null, + }; return { content: [{ type: 'text' as const, text: JSON.stringify(result) }], structuredContent: result, diff --git a/src/mcp/update/updateChecker.ts b/src/mcp/update/updateChecker.ts new file mode 100644 index 00000000..f0e5afb6 --- /dev/null +++ b/src/mcp/update/updateChecker.ts @@ -0,0 +1,259 @@ +/* + * Copyright (c) 2024 Provar Limited. + * All rights reserved. + * Licensed under the BSD 3-Clause license. + * For full license text, see LICENSE.md file in the repo root or https://opensource.org/licenses/BSD-3-Clause + */ + +import fs from 'node:fs'; +import path from 'node:path'; +import os from 'node:os'; +import { fileURLToPath } from 'node:url'; +import { spawnSync } from 'node:child_process'; +import type { SpawnSyncOptions } from 'node:child_process'; +import { createRequire } from 'node:module'; +import { log } from '../logging/logger.js'; + +const requireJson = createRequire(import.meta.url); +const CURRENT_SERVER_VERSION: string = (requireJson('../../../package.json') as { version: string }).version; + +export interface CheckForUpdateResult { + currentVersion: string; + latestVersion: string | null; + updateAvailable: boolean; + updateCommand: string | null; + fromCache: boolean; +} + +interface UpdateCacheEntry { + checkedAt: number; + currentVersion: string; + latestVersion: string | null; + channel: string; +} + +const UPDATE_TTL_MS = 4 * 60 * 60 * 1_000; +const UPDATE_GRACE_MS = 48 * 60 * 60 * 1_000; + +const SPAWN_OPTS = { + stdio: ['ignore', 'pipe', 'pipe'] as const, + timeout: 30_000, + shell: process.platform === 'win32', + maxBuffer: 10 * 1_024 * 1_024, +} satisfies SpawnSyncOptions; + +const SEMVER_RE = /^\d+\.\d+\.\d+(-[a-zA-Z0-9.]+)?$/; + +function cacheDir(): string { + const provarHome = process.env['PROVAR_HOME'] ?? path.join(os.homedir(), 'Provar'); + return path.join(provarHome, '.cache'); +} + +function cacheFile(): string { + return path.join(cacheDir(), '.mcp-update-cache.json'); +} + +// Derives the dist-tag channel from the current version's prerelease identifier. +// '1.5.0-beta.10' → 'beta', '1.5.0' → 'latest' +// Same-channel assumption: we fetch dist-tags.{channel} and compare within that channel. +// Cross-label comparison (e.g. beta vs rc) is explicitly out of scope. +export function deriveChannel(version: string): string { + const hyphen = version.indexOf('-'); + if (hyphen === -1) return 'latest'; + return version.slice(hyphen + 1).split('.')[0] ?? 'latest'; +} + +export function isNewer(current: string, latest: string): boolean { + if (current === latest) return false; + if (!SEMVER_RE.test(latest)) return false; + const split = (v: string): readonly [number, number, number, number] => { + const [main, pre = ''] = v.split('-'); + const [major, minor, patch] = (main ?? '').split('.').map(Number); + // Infinity for stable versions so stable is always newer than any prerelease + const preN = pre ? parseInt(pre.split('.').pop() ?? '0', 10) : Infinity; + return [major, minor, patch, preN] as const; + }; + const [cm, cn, cp, cpN] = split(current); + const [lm, ln, lp, lpN] = split(latest); + for (const [c, l] of [ + [cm, lm], + [cn, ln], + [cp, lp], + [cpN, lpN], + ] as const) { + if (l > c) return true; + if (l < c) return false; + } + return false; +} + +export function detectInstallMethod(): 'sf-plugin' | 'npm-global' | 'linked' { + const modulePath = fileURLToPath(import.meta.url); + const sfDataDirs = [ + path.join('.local', 'share', 'sf'), + path.join('AppData', 'Local', 'sf'), + path.join('AppData', 'Roaming', 'sf'), + path.join('.local', 'share', 'sfdx'), + ]; + if (sfDataDirs.some((d) => modulePath.includes(d))) return 'sf-plugin'; + if (modulePath.includes('node_modules')) return 'npm-global'; + return 'linked'; +} + +function readUpdateCache(): UpdateCacheEntry | null { + try { + const raw = fs.readFileSync(cacheFile(), 'utf-8'); + return JSON.parse(raw) as UpdateCacheEntry; + } catch { + return null; + } +} + +function writeUpdateCache(entry: UpdateCacheEntry): void { + try { + fs.mkdirSync(cacheDir(), { recursive: true }); + const file = cacheFile(); + fs.writeFileSync(file, JSON.stringify(entry, null, 2), { encoding: 'utf-8', mode: 0o600 }); + try { + fs.chmodSync(file, 0o600); + } catch { + // best effort + } + } catch (err) { + log('warn', 'updateChecker: failed to write cache', { error: String(err) }); + } +} + +function buildUpdateCommand(latestVersion: string): string { + const method = detectInstallMethod(); + if (method === 'npm-global') { + return 'npm install -g @provartesting/provardx-cli@' + latestVersion; + } + return 'sf plugins install @provartesting/provardx-cli@' + latestVersion; +} + +function resultFromCache(cached: UpdateCacheEntry, currentVersion: string): CheckForUpdateResult { + const updateAvailable = cached.latestVersion ? isNewer(currentVersion, cached.latestVersion) : false; + const updateCommand = updateAvailable && cached.latestVersion ? buildUpdateCommand(cached.latestVersion) : null; + return { currentVersion, latestVersion: cached.latestVersion, updateAvailable, updateCommand, fromCache: true }; +} + +async function fetchLatestVersion(channel: string): Promise { + const controller = new AbortController(); + const timer = setTimeout(() => controller.abort(), 5_000); + try { + const resp = await fetch('https://registry.npmjs.org/@provartesting/provardx-cli', { + signal: controller.signal, + headers: { Accept: 'application/json' }, + }); + if (!resp.ok) { + log('warn', 'updateChecker: registry returned non-200', { status: resp.status }); + return null; + } + const data = (await resp.json()) as Record; + const distTags = data['dist-tags'] as Record | undefined; + const candidate = distTags?.[channel]; + if (!candidate) { + log('warn', 'updateChecker: dist-tag not found in registry', { channel }); + return null; + } + if (!SEMVER_RE.test(candidate)) { + log('warn', 'updateChecker: registry returned invalid semver', { candidate }); + return null; + } + return candidate; + } finally { + clearTimeout(timer); + } +} + +function applyAutoUpdate(latestVersion: string): void { + const method = detectInstallMethod(); + if (method === 'linked') { + process.stderr.write( + '[provar-mcp] Auto-update skipped: running from development link. Install manually:\n' + + ' sf plugins install @provartesting/provardx-cli@' + + latestVersion + + '\n' + ); + return; + } + const args = + method === 'sf-plugin' + ? ['plugins', 'install', '@provartesting/provardx-cli@' + latestVersion] + : ['install', '-g', '@provartesting/provardx-cli@' + latestVersion]; + const cmd = method === 'sf-plugin' ? 'sf' : 'npm'; + log('info', 'updateChecker: running auto-update', { cmd, args }); + const result = spawnSync(cmd, args, SPAWN_OPTS); + if (result.error != null || result.status !== 0 || result.signal != null) { + const detail = result.stderr?.toString().trim() ?? ''; + log('error', 'updateChecker: auto-update failed', { status: result.status, signal: result.signal, detail }); + } else { + process.stderr.write( + '[provar-mcp] Updated to ' + latestVersion + '. Restart your MCP connection to use the new version.\n' + ); + process.exit(0); + } +} + +export async function checkForUpdate(opts: { + noUpdateCheck: boolean; + autoUpdate: boolean; +}): Promise { + const currentVersion = CURRENT_SERVER_VERSION; + const earlyExit: CheckForUpdateResult = { + currentVersion, + latestVersion: null, + updateAvailable: false, + updateCommand: null, + fromCache: false, + }; + + if (process.env.NODE_ENV === 'test') return earlyExit; + if (process.env['PROVAR_NO_UPDATE_CHECK']) return earlyExit; + if (opts.noUpdateCheck) return earlyExit; + + const channel = deriveChannel(currentVersion); + const cached = readUpdateCache(); + + if (cached != null && Date.now() - cached.checkedAt < UPDATE_TTL_MS) { + log('info', 'updateChecker: cache hit', { channel, latestVersion: cached.latestVersion, fromCache: true }); + return resultFromCache(cached, currentVersion); + } + + let latestVersion: string | null = null; + try { + latestVersion = await fetchLatestVersion(channel); + } catch (err: unknown) { + const errMsg = err instanceof Error ? err.message : String(err); + log('warn', 'updateChecker: registry fetch failed', { error: errMsg }); + if (cached != null && Date.now() - cached.checkedAt < UPDATE_GRACE_MS) { + return resultFromCache(cached, currentVersion); + } + return earlyExit; + } + + writeUpdateCache({ checkedAt: Date.now(), currentVersion, latestVersion, channel }); + + const updateAvailable = latestVersion ? isNewer(currentVersion, latestVersion) : false; + const updateCommand = updateAvailable && latestVersion ? buildUpdateCommand(latestVersion) : null; + + if (updateAvailable && latestVersion) { + process.stderr.write( + '[provar-mcp] Update available: ' + + latestVersion + + ' (current: ' + + currentVersion + + ')\n' + + ' Run: ' + + (updateCommand ?? '') + + '\n' + ); + log('info', 'updateChecker: update available', { currentVersion, latestVersion, updateCommand }); + if (opts.autoUpdate) { + applyAutoUpdate(latestVersion); + } + } + + return { currentVersion, latestVersion, updateAvailable, updateCommand, fromCache: false }; +} diff --git a/test/unit/mcp/updateChecker.test.ts b/test/unit/mcp/updateChecker.test.ts new file mode 100644 index 00000000..66823710 --- /dev/null +++ b/test/unit/mcp/updateChecker.test.ts @@ -0,0 +1,302 @@ +/* + * Copyright (c) 2024 Provar Limited. + * All rights reserved. + * Licensed under the BSD 3-Clause license. + * For full license text, see LICENSE.md file in the repo root or https://opensource.org/licenses/BSD-3-Clause + */ + +import os from 'node:os'; +import path from 'node:path'; +import fs from 'node:fs'; +import { strict as assert } from 'node:assert'; +import { describe, it, beforeEach, afterEach } from 'mocha'; +import { checkForUpdate, isNewer, deriveChannel, detectInstallMethod } from '../../../src/mcp/update/updateChecker.js'; + +// ── Helpers ─────────────────────────────────────────────────────────────────── + +let tmpDir: string; +let origProvarHome: string | undefined; +let origNodeEnv: string | undefined; +let origProvarNoUpdate: string | undefined; +let origFetch: typeof globalThis.fetch; + +function writeFreshCache(data: object): void { + fs.mkdirSync(path.join(tmpDir, '.cache'), { recursive: true }); + fs.writeFileSync(path.join(tmpDir, '.cache', '.mcp-update-cache.json'), JSON.stringify(data), 'utf-8'); +} + +type FakeDistTags = Record; +interface FakeRegistryResponse { + 'dist-tags': FakeDistTags; +} + +function mockFetchOk(distTags: FakeDistTags): void { + const body: FakeRegistryResponse = { 'dist-tags': distTags }; + globalThis.fetch = (): Promise => + Promise.resolve({ + ok: true, + status: 200, + json: (): Promise => Promise.resolve(body), + } as unknown as Response); +} + +function mockFetchError(status: number): void { + globalThis.fetch = (): Promise => + Promise.resolve({ + ok: false, + status, + json: (): Promise> => Promise.resolve({}), + } as unknown as Response); +} + +function mockFetchThrow(err: Error): void { + globalThis.fetch = (): Promise => Promise.reject(err); +} + +// ── Setup ───────────────────────────────────────────────────────────────────── + +beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'provar-update-test-')); + origProvarHome = process.env['PROVAR_HOME']; + process.env['PROVAR_HOME'] = tmpDir; + origNodeEnv = process.env.NODE_ENV; + delete process.env.NODE_ENV; // bypass test fast-path for integration tests + origProvarNoUpdate = process.env['PROVAR_NO_UPDATE_CHECK']; + delete process.env['PROVAR_NO_UPDATE_CHECK']; + origFetch = globalThis.fetch; +}); + +afterEach(() => { + if (origNodeEnv !== undefined) { + process.env.NODE_ENV = origNodeEnv; + } else { + delete process.env.NODE_ENV; + } + if (origProvarNoUpdate !== undefined) { + process.env['PROVAR_NO_UPDATE_CHECK'] = origProvarNoUpdate; + } else { + delete process.env['PROVAR_NO_UPDATE_CHECK']; + } + if (origProvarHome !== undefined) { + process.env['PROVAR_HOME'] = origProvarHome; + } else { + delete process.env['PROVAR_HOME']; + } + globalThis.fetch = origFetch; + fs.rmSync(tmpDir, { recursive: true, force: true }); +}); + +// ── A. isNewer() ────────────────────────────────────────────────────────────── + +describe('isNewer', () => { + it('returns false for identical versions', () => { + assert.equal(isNewer('1.5.0', '1.5.0'), false); + }); + + it('stable is newer than prerelease', () => { + assert.equal(isNewer('1.5.0-beta.1', '1.5.0'), true); + }); + + it('stable is not older than prerelease', () => { + assert.equal(isNewer('1.5.0', '1.5.0-beta.1'), false); + }); + + it('higher numeric suffix is newer', () => { + assert.equal(isNewer('1.5.0-beta.2', '1.5.0-beta.10'), true); + }); + + it('lower numeric suffix is not newer', () => { + assert.equal(isNewer('1.5.0-beta.10', '1.5.0-beta.2'), false); + }); + + it('non-semver latest returns false', () => { + assert.equal(isNewer('any', 'not-semver'), false); + }); +}); + +// ── B. deriveChannel() ──────────────────────────────────────────────────────── + +describe('deriveChannel', () => { + it('derives beta from prerelease version', () => { + assert.equal(deriveChannel('1.5.0-beta.10'), 'beta'); + }); + + it('derives latest from stable version', () => { + assert.equal(deriveChannel('1.5.0'), 'latest'); + }); + + it('derives rc from rc version', () => { + assert.equal(deriveChannel('1.5.0-rc.1'), 'rc'); + }); +}); + +// ── C. detectInstallMethod() ────────────────────────────────────────────────── + +describe('detectInstallMethod', () => { + it('returns a valid install method', () => { + const method = detectInstallMethod(); + assert.ok(['sf-plugin', 'npm-global', 'linked'].includes(method)); + }); + + it('is deterministic', () => { + assert.equal(detectInstallMethod(), detectInstallMethod()); + }); +}); + +// ── D. checkForUpdate() integration ───────────────────────────────────────── + +describe('checkForUpdate', () => { + it('returns earlyExit when PROVAR_NO_UPDATE_CHECK is set', async () => { + process.env['PROVAR_NO_UPDATE_CHECK'] = '1'; + let fetchCalled = false; + globalThis.fetch = (): Promise => { + fetchCalled = true; + return Promise.resolve({} as Response); + }; + const result = await checkForUpdate({ noUpdateCheck: false, autoUpdate: false }); + assert.equal(result.updateAvailable, false); + assert.equal(result.latestVersion, null); + assert.equal(result.fromCache, false); + assert.equal(fetchCalled, false); + }); + + it('returns earlyExit when noUpdateCheck is true', async () => { + const result = await checkForUpdate({ noUpdateCheck: true, autoUpdate: false }); + assert.equal(result.updateAvailable, false); + assert.equal(result.latestVersion, null); + }); + + it('returns fromCache=true for fresh cache (<4h)', async () => { + writeFreshCache({ + checkedAt: Date.now() - 30 * 60 * 1_000, // 30 min ago + currentVersion: '1.5.0-beta.10', + latestVersion: '1.5.0-beta.10', + channel: 'beta', + }); + let fetchCalled = false; + globalThis.fetch = (): Promise => { + fetchCalled = true; + return Promise.resolve({} as Response); + }; + const result = await checkForUpdate({ noUpdateCheck: false, autoUpdate: false }); + assert.equal(result.fromCache, true); + assert.equal(fetchCalled, false); + }); + + it('fetches registry when cache is stale (>4h)', async () => { + writeFreshCache({ + checkedAt: Date.now() - 5 * 60 * 60 * 1_000, // 5 hours ago + currentVersion: '1.5.0-beta.10', + latestVersion: '1.5.0-beta.10', + channel: 'beta', + }); + mockFetchOk({ beta: '1.5.0-beta.11' }); + const result = await checkForUpdate({ noUpdateCheck: false, autoUpdate: false }); + assert.equal(result.fromCache, false); + assert.equal(result.latestVersion, '1.5.0-beta.11'); + }); + + it('returns updateAvailable=true when update is available', async () => { + mockFetchOk({ beta: '99.0.0-beta.1', latest: '99.0.0' }); + const result = await checkForUpdate({ noUpdateCheck: false, autoUpdate: false }); + assert.equal(result.updateAvailable, true); + assert.ok(result.updateCommand !== null); + }); + + it('returns updateAvailable=false when current equals latest', async () => { + const { currentVersion } = await checkForUpdate({ noUpdateCheck: true, autoUpdate: false }); + const channel = deriveChannel(currentVersion); + mockFetchOk({ [channel]: currentVersion }); + const result = await checkForUpdate({ noUpdateCheck: false, autoUpdate: false }); + assert.equal(result.updateAvailable, false); + }); + + it('returns updateAvailable=false on fetch abort (no throw)', async () => { + mockFetchThrow(new Error('The operation was aborted')); + const result = await checkForUpdate({ noUpdateCheck: false, autoUpdate: false }); + assert.equal(result.updateAvailable, false); + assert.equal(result.latestVersion, null); + }); + + it('returns updateAvailable=false on network error (no throw)', async () => { + mockFetchThrow(new TypeError('Failed to fetch')); + const result = await checkForUpdate({ noUpdateCheck: false, autoUpdate: false }); + assert.equal(result.updateAvailable, false); + }); + + it('returns updateAvailable=false on HTTP non-200 (no throw)', async () => { + mockFetchError(503); + const result = await checkForUpdate({ noUpdateCheck: false, autoUpdate: false }); + assert.equal(result.updateAvailable, false); + assert.equal(result.latestVersion, null); + }); + + it('returns latestVersion=null when dist-tag channel missing', async () => { + // Registry has 'latest' but not 'beta' — current version may be beta + mockFetchOk({ latest: '1.5.0' }); + const result = await checkForUpdate({ noUpdateCheck: false, autoUpdate: false }); + // No throw is the main assertion; updateAvailable depends on current version channel + assert.ok(result !== null); + }); + + it('treats corrupt cache as miss and fetches fresh (no throw)', async () => { + fs.mkdirSync(path.join(tmpDir, '.cache'), { recursive: true }); + fs.writeFileSync(path.join(tmpDir, '.cache', '.mcp-update-cache.json'), 'NOT-JSON', 'utf-8'); + mockFetchOk({ beta: '1.5.0-beta.11', latest: '1.5.0' }); + const result = await checkForUpdate({ noUpdateCheck: false, autoUpdate: false }); + assert.equal(result.fromCache, false); + }); + + it('returns updateAvailable=false when cache is >48h stale and fetch fails', async () => { + writeFreshCache({ + checkedAt: Date.now() - 50 * 60 * 60 * 1_000, // 50 hours ago + currentVersion: '1.5.0-beta.10', + latestVersion: '1.5.0-beta.10', + channel: 'beta', + }); + mockFetchThrow(new TypeError('Failed to fetch')); + const result = await checkForUpdate({ noUpdateCheck: false, autoUpdate: false }); + assert.equal(result.updateAvailable, false); + }); + + it('returns updateAvailable=false when registry returns invalid semver', async () => { + mockFetchOk({ beta: 'not-a-version', latest: 'also-bad' }); + const result = await checkForUpdate({ noUpdateCheck: false, autoUpdate: false }); + assert.equal(result.updateAvailable, false); + assert.equal(result.latestVersion, null); + }); + + it('writes cache file after successful fetch', async () => { + mockFetchOk({ beta: '1.5.0-beta.11', latest: '1.5.0' }); + await checkForUpdate({ noUpdateCheck: false, autoUpdate: false }); + const cacheFilePath = path.join(tmpDir, '.cache', '.mcp-update-cache.json'); + assert.ok(fs.existsSync(cacheFilePath), 'Cache file should have been written'); + const cacheData = JSON.parse(fs.readFileSync(cacheFilePath, 'utf-8')) as { checkedAt: number }; + assert.ok(cacheData.checkedAt > 0, 'Cache should have checkedAt'); + }); + + it('returns stale cache within 48h grace period when fetch fails', async () => { + writeFreshCache({ + checkedAt: Date.now() - 6 * 60 * 60 * 1_000, // 6 hours ago (stale but within 48h) + currentVersion: '1.5.0-beta.10', + latestVersion: '1.5.0-beta.10', + channel: 'beta', + }); + mockFetchThrow(new TypeError('Failed to fetch')); + const result = await checkForUpdate({ noUpdateCheck: false, autoUpdate: false }); + assert.equal(result.fromCache, true); + }); + + it('skips silently when PROVAR_NO_UPDATE_CHECK is set (no network)', async () => { + process.env['PROVAR_NO_UPDATE_CHECK'] = '1'; + let fetchCalled = false; + globalThis.fetch = (): Promise => { + fetchCalled = true; + return Promise.resolve({} as Response); + }; + const result = await checkForUpdate({ noUpdateCheck: false, autoUpdate: false }); + assert.equal(fetchCalled, false); + assert.equal(result.updateAvailable, false); + assert.equal(result.fromCache, false); + }); +}); From 20b75f7fb1015a8dee16bfe12867f83547c3e713 Mon Sep 17 00:00:00 2001 From: Michael Dailey Date: Mon, 4 May 2026 16:08:03 -0500 Subject: [PATCH 18/19] PDX-0: fix(coverage): null testCasePath when out-of-bounds to prevent coverage skew RCA: resolveTestInstanceFull returned the raw testCasePath even when tcInBounds=false, allowing crafted paths (e.g. ../other/TC.testcase) to be added to coveredPaths and skew coverage totals. Fix: return testCasePath: null when tcInBounds is false so out-of-bounds instance references never accumulate in coveredPaths; addressed per Copilot review comment on PR #139. Co-Authored-By: Claude Sonnet 4.6 --- src/services/projectValidation.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/services/projectValidation.ts b/src/services/projectValidation.ts index 2d26abc0..8ac751dc 100644 --- a/src/services/projectValidation.ts +++ b/src/services/projectValidation.ts @@ -272,7 +272,8 @@ function resolveTestInstanceFull(instancePath: string, projectPath: string): Tes } } - return { testCase: { name: tcName, xml_content }, testCasePath, testCaseId }; + // Only expose testCasePath when in-bounds — out-of-bounds paths must not affect coverage totals + return { testCase: { name: tcName, xml_content }, testCasePath: tcInBounds ? testCasePath : null, testCaseId }; } catch { return { testCase: null, testCasePath: null, testCaseId: null }; } From 9003362a98423ac4f28975861799d6c6a9f50cb7 Mon Sep 17 00:00:00 2001 From: Michael Dailey Date: Mon, 4 May 2026 16:43:28 -0500 Subject: [PATCH 19/19] PDX-0: fix(mcp): address PR #140 review comments RCA: Cache hit path skipped channel/version validation; auto-update flag docs said restart instead of exit-and-reconnect. Fix: Guard cache hit on channel+version match; fix ping table to 3-col with nullable types; align auto-update wording in docs and messages. --- docs/mcp.md | 26 +++++++++++++------------- messages/sf.provar.mcp.start.md | 2 +- src/mcp/update/updateChecker.ts | 7 ++++++- test/unit/mcp/updateChecker.test.ts | 9 ++++++--- 4 files changed, 26 insertions(+), 18 deletions(-) diff --git a/docs/mcp.md b/docs/mcp.md index 3a503e9f..748f35c3 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -143,11 +143,11 @@ The server communicates over **stdio** (standard input / output). It must be sta ### Flags -| Flag | Alias | Default | Description | -| ------------------- | ----- | ------------------------- | ------------------------------------------------------------------------------------------------------------------------------------ | -| `--allowed-paths` | `-a` | Current working directory | Base directories that file-system tools are permitted to read and write. Repeat the flag to allow multiple paths. | -| `--auto-update` | | false | Automatically installs the latest version at startup and restarts the MCP connection. Skipped if running from a development symlink. | -| `--no-update-check` | | false | Skip the startup update check. Also controlled by the `PROVAR_NO_UPDATE_CHECK` environment variable. | +| Flag | Alias | Default | Description | +| ------------------- | ----- | ------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| `--allowed-paths` | `-a` | Current working directory | Base directories that file-system tools are permitted to read and write. Repeat the flag to allow multiple paths. | +| `--auto-update` | | false | Automatically installs the latest version at startup and exits so the client reconnects with the new version. Skipped if running from a development symlink. | +| `--no-update-check` | | false | Skip the startup update check. Also controlled by the `PROVAR_NO_UPDATE_CHECK` environment variable. | ```sh # Allow access to a specific project directory @@ -489,14 +489,14 @@ A lightweight sanity-check tool. Echoes back the message you send. Useful for ve **Output** -| Field | Type | Description | -| ----------------- | ------- | --------------------------------------------------------- | --------------------------------------------------- | -| `pong` | string | The echoed message | -| `ts` | string | ISO-8601 timestamp | -| `server` | string | Server name and version (e.g. `provar-mcp@1.5.0-beta.10`) | -| `updateAvailable` | boolean | Whether a newer version is available in the registry | -| `latestVersion` | string | null | Latest version found in the npm registry, or `null` | -| `updateCommand` | string | null | Command to run to update the plugin, or `null` | +| Field | Type | Description | +| ----------------- | -------------- | --------------------------------------------------------- | +| `pong` | string | The echoed message | +| `ts` | string | ISO-8601 timestamp | +| `server` | string | Server name and version (e.g. `provar-mcp@1.5.0-beta.14`) | +| `updateAvailable` | boolean | Whether a newer version is available in the registry | +| `latestVersion` | string \| null | Latest version found in the npm registry, or `null` | +| `updateCommand` | string \| null | Command to run to update the plugin, or `null` | --- diff --git a/messages/sf.provar.mcp.start.md b/messages/sf.provar.mcp.start.md index 5c8863b5..ea3da344 100644 --- a/messages/sf.provar.mcp.start.md +++ b/messages/sf.provar.mcp.start.md @@ -94,7 +94,7 @@ When enabled, testrun.report suggestions will prompt defect creation on failures # flags.auto-update.summary -When enabled, automatically installs the latest version of the plugin and restarts the MCP connection on startup. +When enabled, automatically installs the latest version at startup and exits. The MCP client must reconnect to load the new version. # flags.no-update-check.summary diff --git a/src/mcp/update/updateChecker.ts b/src/mcp/update/updateChecker.ts index f0e5afb6..a77a5af8 100644 --- a/src/mcp/update/updateChecker.ts +++ b/src/mcp/update/updateChecker.ts @@ -216,7 +216,12 @@ export async function checkForUpdate(opts: { const channel = deriveChannel(currentVersion); const cached = readUpdateCache(); - if (cached != null && Date.now() - cached.checkedAt < UPDATE_TTL_MS) { + if ( + cached != null && + cached.channel === channel && + cached.currentVersion === currentVersion && + Date.now() - cached.checkedAt < UPDATE_TTL_MS + ) { log('info', 'updateChecker: cache hit', { channel, latestVersion: cached.latestVersion, fromCache: true }); return resultFromCache(cached, currentVersion); } diff --git a/test/unit/mcp/updateChecker.test.ts b/test/unit/mcp/updateChecker.test.ts index 66823710..1093c08d 100644 --- a/test/unit/mcp/updateChecker.test.ts +++ b/test/unit/mcp/updateChecker.test.ts @@ -167,11 +167,14 @@ describe('checkForUpdate', () => { }); it('returns fromCache=true for fresh cache (<4h)', async () => { + // Use the actual running version so the channel + currentVersion guard passes + const { currentVersion } = await checkForUpdate({ noUpdateCheck: true, autoUpdate: false }); + const channel = deriveChannel(currentVersion); writeFreshCache({ checkedAt: Date.now() - 30 * 60 * 1_000, // 30 min ago - currentVersion: '1.5.0-beta.10', - latestVersion: '1.5.0-beta.10', - channel: 'beta', + currentVersion, + latestVersion: currentVersion, + channel, }); let fetchCalled = false; globalThis.fetch = (): Promise => {