diff --git a/.github/workflows/CI_Execution.yml b/.github/workflows/CI_Execution.yml index 35bb7ab..3d1558a 100644 --- a/.github/workflows/CI_Execution.yml +++ b/.github/workflows/CI_Execution.yml @@ -22,7 +22,7 @@ jobs: provardx-ci-execution: strategy: matrix: - os: ${{ fromJSON(inputs.OS && format('[{0}]', inputs.OS) || '["ubuntu-latest", "macos-latest"]') }} + os: ${{ fromJSON(inputs.OS && format('[{0}]', inputs.OS) || '["ubuntu-latest", "macos-latest", "windows-latest"]') }} nodeversion: [20] runs-on: ${{ matrix.os }} steps: @@ -99,6 +99,8 @@ jobs: run: | sf plugins link . yarn prepack + - name: Unit tests + run: npx mocha "test/**/*.test.ts" --timeout 30000 - name: MCP smoke test timeout-minutes: 5 env: diff --git a/.github/workflows/DeployManual.yml b/.github/workflows/DeployManual.yml index 68d8c46..10da189 100644 --- a/.github/workflows/DeployManual.yml +++ b/.github/workflows/DeployManual.yml @@ -65,18 +65,21 @@ jobs: 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 -v tag="${GITHUB_REF_NAME}" '$0==tag{found=1;next} found{print;exit}') + # Release event: HEAD is the new tag — find the nearest ancestor tag before it + PREV=$(git describe --tags --abbrev=0 HEAD^ 2>/dev/null || git tag --sort=-version:refname | tail -1) else - # For a manual dispatch use the most recent existing tag - PREV=$(git tag --sort=-creatordate | head -1) + # Manual dispatch: find the nearest ancestor tag from HEAD + # (git describe respects branch ancestry; avoids pulling in commits from sibling branches) + PREV=$(git describe --tags --abbrev=0 HEAD 2>/dev/null || true) fi RANGE="${PREV:+${PREV}..}HEAD" - RAW=$(git log --pretty=format:"%s" $RANGE \ + RAW=$(git log --pretty=format:"%s" "$RANGE" \ | sed 's/^[A-Z][A-Z0-9]*-[0-9]*: //' \ - | grep -Ev "^(Merge |chore)" \ + | grep -Ev "^(Merge |chore|bump|increment)" \ + | grep -Ev "\(ci\):" \ + | grep -Eiv "review comments?|merge conflict|feedback items?|test fixtures?|pre-landing|address session|address PR|session [0-9]|resolving merge|PR #[0-9]|dot\.notation|server\.tool|registerTool|bump version|increment version|testCasePath|planitem|uuid lookup|coverage (count|skew)|buildTestCase|awk regex|deploy workflow" \ | head -20) FEATS=$(printf '%s\n' "$RAW" | grep '^feat' | sed 's/^feat[^:]*: /• /' | head -8) diff --git a/docs/mcp-pilot-guide.md b/docs/mcp-pilot-guide.md index 7da00fc..a958918 100644 --- a/docs/mcp-pilot-guide.md +++ b/docs/mcp-pilot-guide.md @@ -502,6 +502,8 @@ PathPolicy: assertPathAllowed(filePath, allowedPaths) This check runs before every file read and write, including all path-type input fields — not just output file paths. Symlinks are dereferenced so that a symlink inside an allowed directory cannot escape containment. The allowed roots are set at server startup via `--allowed-paths` and cannot be changed while the server is running. +On **Windows**, all path comparisons are performed case-insensitively. `fs.realpathSync` does not always canonicalize drive-letter case (e.g. `c:\` vs `C:\`), so the policy normalizes both the candidate path and the allowed roots to lowercase before comparing. This means `C:\Projects\MyProject` and `c:\projects\myproject` are treated as the same path for containment purposes. + ### Audit log All tool invocations are logged to **stderr** with a unique `requestId` per call. The log format is structured JSON: diff --git a/docs/mcp.md b/docs/mcp.md index 5b25a7f..5024925 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -473,6 +473,8 @@ All file-system operations (read, write, generate) are restricted to the paths s Symlinks are resolved via `fs.realpathSync` before the containment check, so a symlink inside an allowed directory that points outside it cannot bypass the restriction. For tools that accept multiple path inputs (such as `provar_ant_generate`'s `provar_home`, `project_path`, and `results_path`), all path fields are validated before any file operation occurs — not just the output path. +On **Windows**, path comparisons are performed case-insensitively to account for the fact that `fs.realpathSync` does not always canonicalize drive-letter case (e.g. `c:\` vs `C:\`). This means `C:\Projects\my-project` and `c:\projects\my-project` are treated as equivalent when checking against `--allowed-paths`. + --- ## Available tools @@ -750,6 +752,7 @@ Validates an XML test case for schema correctness (validity score) and best prac - **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. +- **VAR-REF-002** — A `{VarName}` token is embedded inside a larger plain string (e.g. `SELECT Id FROM Account WHERE Id = '{AccountId}'`). Provar does not perform `{…}` interpolation in string values at runtime; the braces are emitted literally. Use `class="compound"` with `` children to split the literal text and variable references. In `provar_testcase_generate`, pass the value with `{VarName}` placeholders — the generator emits compound XML automatically. --- diff --git a/package.json b/package.json index 7204116..6048105 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.15", + "version": "1.5.0-beta.16", "mcpName": "io.github.ProvarTesting/provar", "license": "BSD-3-Clause", "plugins": [ diff --git a/server.json b/server.json index 3b20386..54eca32 100644 --- a/server.json +++ b/server.json @@ -14,19 +14,28 @@ "url": "https://github.com/ProvarTesting/provardx-cli", "source": "github" }, - "version": "1.5.0-beta.15", + "version": "1.5.0-beta.16", "packages": [ { "registryType": "npm", "identifier": "@provartesting/provardx-cli", - "version": "1.5.0-beta.15", + "version": "1.5.0-beta.16", "transport": { "type": "stdio" }, "runtimeArguments": [ - { "type": "positional", "value": "provar" }, - { "type": "positional", "value": "mcp" }, - { "type": "positional", "value": "start" }, + { + "type": "positional", + "value": "provar" + }, + { + "type": "positional", + "value": "mcp" + }, + { + "type": "positional", + "value": "start" + }, { "type": "named", "name": "--allowed-paths", diff --git a/src/mcp/security/pathPolicy.ts b/src/mcp/security/pathPolicy.ts index 00eeef1..c7a4656 100644 --- a/src/mcp/security/pathPolicy.ts +++ b/src/mcp/security/pathPolicy.ts @@ -44,12 +44,21 @@ export function assertPathAllowed(filePath: string, allowedPaths: string[]): voi try { resolved = fs.realpathSync(filePath); } catch { - // Path doesn't exist — resolve the parent (which should exist) to catch symlinks there - const parent = path.dirname(path.resolve(filePath)); + // Path doesn't exist — walk up the ancestor hierarchy to find the deepest existing directory, + // resolve symlinks there, then re-attach the non-existent tail segments. This handles macOS + // where os.tmpdir() returns /var/... (a symlink to /private/var/...) and intermediate dirs + // for a new output path may not yet exist. + const full = path.resolve(filePath); + let cur = full; + const tail: string[] = []; + while (!fs.existsSync(cur) && cur !== path.dirname(cur)) { + tail.unshift(path.basename(cur)); + cur = path.dirname(cur); + } try { - resolved = path.join(fs.realpathSync(parent), path.basename(filePath)); + resolved = path.join(fs.realpathSync(cur), ...tail); } catch { - resolved = path.resolve(filePath); + resolved = full; } } @@ -61,9 +70,18 @@ export function assertPathAllowed(filePath: string, allowedPaths: string[]): voi } }); + // Windows file paths are case-insensitive; fs.realpathSync does not always + // canonicalize drive-letter case (e.g. `c:\` vs `C:\`), so compare case-insensitively. + const isWindows = process.platform === 'win32'; + const normalizeForCompare = (p: string): string => (isWindows ? p.toLowerCase() : p); + const resolvedKey = normalizeForCompare(resolved); + if ( resolvedAllowed.length > 0 && - !resolvedAllowed.some((base) => resolved === base || resolved.startsWith(base + path.sep)) + !resolvedAllowed.some((base) => { + const baseKey = normalizeForCompare(base); + return resolvedKey === baseKey || resolvedKey.startsWith(baseKey + path.sep); + }) ) { throw new PathPolicyError( 'PATH_NOT_ALLOWED', diff --git a/src/mcp/tools/testCaseGenerate.ts b/src/mcp/tools/testCaseGenerate.ts index 9fd4d55..6310a81 100644 --- a/src/mcp/tools/testCaseGenerate.ts +++ b/src/mcp/tools/testCaseGenerate.ts @@ -150,6 +150,7 @@ const TOOL_DESCRIPTION = [ 'Cleanup warning: ApexDeleteObject steps near end of test will be skipped if an earlier step fails (stopOnError=false). Use a TearDown callable.', 'Validation: when validate_after_edit=true (default) the response includes a validation field and returns TESTCASE_INVALID if the generated XML fails structural checks.', 'Grounding: call provar_qualityhub_examples_retrieve before generating to get corpus examples for the scenario — correct XML structure for the step types you need.', + 'If the response has count: 0 with a warning field (API unavailable or not configured), fall back: read the provar://docs/step-reference MCP resource for step types and attribute formats, then continue.', ].join(' '); export function registerTestCaseGenerate(server: McpServer, config: ServerConfig): void { @@ -278,7 +279,29 @@ export function registerTestCaseGenerate(server: McpServer, config: ServerConfig // ── XML builder ─────────────────────────────────────────────────────────────── -// Build the element for a single argument (D2/D4 aware). +// F1/F3: build class="compound" for strings that mix literal text with {VarName} tokens. +function buildCompoundValue(val: string, indent: string): string { + const i = `${indent} `; + const parts: string[] = []; + const tokenRe = /\{([\w.]+)\}/g; + let last = 0; + let m: RegExpExecArray | null; + while ((m = tokenRe.exec(val)) !== null) { + const before = val.slice(last, m.index); + if (before) parts.push(`${i}${escapeXmlContent(before)}`); + const pathElements = m[1] + .split('.') + .map((p) => `${i} `) + .join('\n'); + parts.push(`${i}\n${pathElements}\n${i}`); + last = m.index + m[0].length; + } + const tail = val.slice(last); + if (tail) parts.push(`${i}${escapeXmlContent(tail)}`); + return `${indent}\n${i}\n${parts.join('\n')}\n${i}\n${indent}`; +} + +// Build the element for a single argument (D2/D4/F1 aware). // inNamedValues: when true (inside SetValues namedValues), skip uiTarget/uiLocator dispatch. // 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 { @@ -291,6 +314,10 @@ function buildArgumentValue(key: string, val: string, indent: string, inNamedVal .join('\n'); return `${indent}\n${pathElements}\n${indent}`; } + // F1/F3: {VarName} embedded in surrounding text → class="compound" with . + if (/\{[\w.]+\}/.test(val)) { + return buildCompoundValue(val, indent); + } if (!inNamedValues) { // D2: 'target' argument → class="uiTarget" (only for UiWithScreen / UiWithRow). if (key === 'target' && (apiId.includes('UiWithScreen') || apiId.includes('UiWithRow'))) { diff --git a/src/mcp/tools/testCaseStepTools.ts b/src/mcp/tools/testCaseStepTools.ts index 09eca65..5a24b75 100644 --- a/src/mcp/tools/testCaseStepTools.ts +++ b/src/mcp/tools/testCaseStepTools.ts @@ -98,6 +98,7 @@ export function registerTestCaseStepEdit(server: McpServer, config: ServerConfig 'Returns STEP_NOT_FOUND (with all_test_item_ids list) when the target step is absent.', 'Returns INVALID_STEP_XML when step_xml cannot be parsed or contains ≠1 elements.', 'Returns INVALID_XML_AFTER_EDIT (backup restored) when the mutated file fails validation.', + 'Grounding for step_xml: call provar_qualityhub_examples_retrieve for corpus examples of the step type you need; if the response has count: 0 with a warning field, fall back: read the provar://docs/step-reference MCP resource.', ].join(' '), inputSchema: { test_case_path: z.string().describe('Absolute path to the .testcase XML file; must be within --allowed-paths'), diff --git a/src/mcp/tools/testCaseValidate.ts b/src/mcp/tools/testCaseValidate.ts index cc2f4d5..7ef8d3d 100644 --- a/src/mcp/tools/testCaseValidate.ts +++ b/src/mcp/tools/testCaseValidate.ts @@ -47,7 +47,7 @@ export function registerTestCaseValidate(server: McpServer, config: ServerConfig { title: 'Validate Test Case', description: - 'Validate a Provar XML test case for structural correctness and quality. Checks XML declaration, root element, required attributes (guid UUID v4, testItemId integer), presence, and applies best-practice rules. When a Provar API key is configured (via sf provar auth login or PROVAR_API_KEY env var), calls the Quality Hub API for full 170-rule scoring. Falls back to local validation if no key is set or the API is unavailable. Returns validity_score (schema compliance), quality_score (best practices, 0–100), and validation_source indicating which ruleset was applied.', + 'Validate a Provar XML test case for structural correctness and quality. Checks XML declaration, root element, required attributes (guid UUID v4, testItemId integer), presence, and applies best-practice rules. When a Provar API key is configured (via sf provar auth login or PROVAR_API_KEY env var), calls the Quality Hub API for full 170-rule scoring. Falls back to local validation if no key is set or the API is unavailable. Returns validity_score (schema compliance), quality_score (best practices, 0–100), and validation_source indicating which ruleset was applied. When structural errors are returned, consult the provar://docs/step-reference MCP resource for correct step attribute schemas.', inputSchema: { content: z.string().optional().describe('XML content to validate directly (alias: xml)'), xml: z.string().optional().describe('XML content to validate — API-compatible alias for content'), @@ -329,23 +329,43 @@ 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.`, - }); + // VAR-REF-001 / VAR-REF-002: detect {VarName} tokens inside valueClass="string" elements. + // Provar does not interpolate {…} tokens in plain string values at runtime — they must use + // class="variable" (pure reference) or class="compound" (embedded in surrounding text). + const stringValueRe = /]+valueClass="string"[^>]*>([^<]+)<\/value>/g; + let stringMatch: RegExpExecArray | null; + while ((stringMatch = stringValueRe.exec(xmlContent)) !== null) { + const rawContent = stringMatch[1]; + if (!/\{[\w.]+\}/.test(rawContent)) continue; + const isPure = /^\{[\w.]+\}$/.test(rawContent.trim()); + const varNames = [...rawContent.matchAll(/\{([\w.]+)\}/g)].map((m) => m[1]); + if (isPure) { + const varName = varNames[0]; + issues.push({ + rule_id: 'VAR-REF-001', + severity: 'WARNING', + message: `Argument value "{${varName}}" 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.`, + }); + } else { + const preview = rawContent.length > 60 ? rawContent.slice(0, 57) + '…' : rawContent; + issues.push({ + rule_id: 'VAR-REF-002', + severity: 'WARNING', + message: `Argument value "${preview}" contains {${varNames.join( + '}, {' + )}} embedded in a plain string — Provar does not interpolate {…} tokens in string values at runtime.`, + applies_to: 'argument', + suggestion: + 'Use class="compound" with to split literal text and variable references at each {VarName} boundary. ' + + 'In provar_testcase_generate, pass the value with {VarName} placeholders in the attributes object — the generator emits compound XML automatically.', + }); + } } return finalize(issues, tcId, tcName, apiCalls.length, xmlContent, testName); diff --git a/src/mcp/tools/testPlanTools.ts b/src/mcp/tools/testPlanTools.ts index 8c01d9e..c53e65a 100644 --- a/src/mcp/tools/testPlanTools.ts +++ b/src/mcp/tools/testPlanTools.ts @@ -259,8 +259,9 @@ export function registerTestPlanAddInstance(server: McpServer, config: ServerCon }; } - // Resolve testcase absolute path - const absoluteTestCasePath = path.join(projectRoot, test_case_path); + // Resolve testcase absolute path — normalize backslashes so Windows-style paths work on macOS/Linux + const normalizedTestCasePath = toForwardSlashes(test_case_path); + const absoluteTestCasePath = path.join(projectRoot, normalizedTestCasePath); if (!fs.existsSync(absoluteTestCasePath)) { return { isError: true, @@ -274,7 +275,7 @@ export function registerTestPlanAddInstance(server: McpServer, config: ServerCon ], }; } - if (!test_case_path.endsWith('.testcase')) { + if (!normalizedTestCasePath.endsWith('.testcase')) { return { isError: true, content: [ @@ -331,7 +332,7 @@ export function registerTestPlanAddInstance(server: McpServer, config: ServerCon } // Determine filename and full path - const instanceFileName = path.basename(test_case_path, '.testcase') + '.testinstance'; + const instanceFileName = path.basename(normalizedTestCasePath, '.testcase') + '.testinstance'; const instanceFilePath = path.join(instanceDir, instanceFileName); if (!overwrite && fs.existsSync(instanceFilePath)) { @@ -354,7 +355,6 @@ export function registerTestPlanAddInstance(server: McpServer, config: ServerCon // Build XML const guid = randomUUID(); - const normalizedTestCasePath = toForwardSlashes(test_case_path); const xmlContent = buildTestInstanceXml(guid, testCaseId, normalizedTestCasePath); if (!dry_run) { diff --git a/test/unit/mcp/pathPolicy.test.ts b/test/unit/mcp/pathPolicy.test.ts index 01276d8..38aab73 100644 --- a/test/unit/mcp/pathPolicy.test.ts +++ b/test/unit/mcp/pathPolicy.test.ts @@ -2,7 +2,7 @@ import { strict as assert } from 'node:assert'; import fs from 'node:fs'; import os from 'node:os'; import path from 'node:path'; -import { describe, it, afterEach } from 'mocha'; +import { describe, it, afterEach, before } from 'mocha'; import { assertPathAllowed, PathPolicyError } from '../../../src/mcp/security/pathPolicy.js'; const tmp = os.tmpdir(); @@ -41,9 +41,7 @@ describe('pathPolicy', () => { }); it('allows nested paths inside allowedPaths', () => { - assert.doesNotThrow(() => - assertPathAllowed(path.join(tmp, 'a', 'b', 'c', 'file.xml'), [tmp]) - ); + assert.doesNotThrow(() => assertPathAllowed(path.join(tmp, 'a', 'b', 'c', 'file.xml'), [tmp])); }); it('rejects sibling directories that share a prefix', () => { @@ -63,7 +61,11 @@ describe('pathPolicy', () => { afterEach(() => { if (symlinkDir) { - try { fs.rmSync(symlinkDir, { recursive: true, force: true }); } catch { /* ignore */ } + try { + fs.rmSync(symlinkDir, { recursive: true, force: true }); + } catch { + /* ignore */ + } } }); @@ -98,4 +100,40 @@ describe('pathPolicy', () => { assert.doesNotThrow(() => assertPathAllowed(real, [allowedDir])); }); }); + + describe('Windows case-insensitive comparison', () => { + before(function () { + if (process.platform !== 'win32') this.skip(); + }); + + it('allows a path with lowercase drive letter when allowed has uppercase', () => { + // Reproduces GitHub Copilot bug: agent sends `c:\...` but allowed path is `C:\...` + const allowed = tmp; // typically `C:\Users\\AppData\Local\Temp` + const lowerDrive = allowed.charAt(0).toLowerCase() + allowed.slice(1); + assert.doesNotThrow(() => assertPathAllowed(path.join(lowerDrive, 'foo.java'), [allowed])); + }); + + it('allows a path with uppercase drive letter when allowed has lowercase', () => { + const upperDrive = tmp.charAt(0).toUpperCase() + tmp.slice(1); + const lowerAllowed = tmp.charAt(0).toLowerCase() + tmp.slice(1); + assert.doesNotThrow(() => assertPathAllowed(path.join(upperDrive, 'foo.java'), [lowerAllowed])); + }); + + it('allows a path with mixed-case directory segments', () => { + const mixed = tmp.toUpperCase(); + assert.doesNotThrow(() => assertPathAllowed(path.join(mixed, 'foo.java'), [tmp])); + }); + + it('still rejects a sibling path that differs only after the prefix (case-insensitive)', () => { + const allowed = path.join(tmp, 'myproject'); + const sibling = path.join(tmp.toUpperCase(), 'myproject-evil', 'secret.txt'); + try { + assertPathAllowed(sibling, [allowed]); + assert.fail('Expected PathPolicyError to be thrown'); + } catch (e) { + assert.ok(e instanceof PathPolicyError, 'Expected PathPolicyError'); + assert.equal(e.code, 'PATH_NOT_ALLOWED'); + } + }); + }); }); diff --git a/test/unit/mcp/testCaseGenerate.test.ts b/test/unit/mcp/testCaseGenerate.test.ts index e733482..6de289a 100644 --- a/test/unit/mcp/testCaseGenerate.test.ts +++ b/test/unit/mcp/testCaseGenerate.test.ts @@ -21,14 +21,17 @@ import type { ServerConfig } from '../../../src/mcp/server.js'; type ToolHandler = (args: Record) => unknown; class MockMcpServer { + public registrations: Array<{ name: string; description: string }> = []; private handlers = new Map(); public tool(name: string, _description: string, _schema: unknown, handler: ToolHandler): void { this.handlers.set(name, handler); } - public registerTool(name: string, _config: unknown, handler: ToolHandler): void { + public registerTool(name: string, config: unknown, handler: ToolHandler): void { this.handlers.set(name, handler); + const desc = (config as Record)['description']; + if (typeof desc === 'string') this.registrations.push({ name, description: desc }); } public call(name: string, args: Record): ReturnType { @@ -69,6 +72,23 @@ afterEach(() => { fs.rmSync(tmpDir, { recursive: true, force: true }); }); +// ── tool description ────────────────────────────────────────────────────────── + +describe('provar_testcase_generate description', () => { + it('references corpus tool and step-reference fallback', () => { + const reg = server.registrations.find((r) => r.name === 'provar_testcase_generate'); + assert.ok(reg, 'tool should be registered'); + assert.ok( + reg.description.includes('provar_qualityhub_examples_retrieve'), + 'description should reference corpus tool' + ); + assert.ok( + reg.description.includes('provar://docs/step-reference'), + 'description should include step-reference fallback' + ); + }); +}); + // ── provar_testcase_generate ─────────────────────────────────────────────────── describe('provar_testcase_generate', () => { @@ -675,6 +695,76 @@ describe('provar_testcase_generate', () => { }); }); + describe('F1 — Compound values for {VarName} embedded in surrounding text', () => { + it('"Hello {Name}" emits class="compound" with literal and variable parts', () => { + const result = server.call('provar_testcase_generate', { + test_case_name: 'Compound Value Test', + steps: [ + { + api_id: 'UiDoAction', + name: 'Enter greeting', + attributes: { value: 'Hello {Name}' }, + }, + ], + dry_run: true, + overwrite: false, + validate_after_edit: false, + }); + + assert.equal(isError(result), false); + const xml = parseText(result)['xml_content'] as string; + assert.ok(xml.includes('class="compound"'), 'Expected class="compound" for mixed value'); + assert.ok(xml.includes(''), 'Expected element'); + assert.ok(xml.includes('valueClass="string">Hello '), 'Expected literal prefix as string part'); + assert.ok(xml.includes(''), 'Expected element for the token'); + assert.ok(xml.includes(''), 'Expected '); + assert.ok(!xml.includes('valueClass="string">Hello {Name}'), 'Must NOT emit raw {Name} as string literal'); + }); + + it('"{A} and {B}" emits compound with two variable parts', () => { + const result = server.call('provar_testcase_generate', { + test_case_name: 'Multi-Var Compound Test', + steps: [ + { + api_id: 'UiDoAction', + name: 'Combine two vars', + attributes: { value: '{First} and {Last}' }, + }, + ], + dry_run: true, + overwrite: false, + validate_after_edit: false, + }); + + assert.equal(isError(result), false); + const xml = parseText(result)['xml_content'] as string; + assert.ok(xml.includes('class="compound"'), 'Expected compound for two variables'); + assert.ok(xml.includes(''), 'Expected path for First'); + assert.ok(xml.includes(''), 'Expected path for Last'); + }); + + it('pure {VarName} alone still uses class="variable" (not compound)', () => { + const result = server.call('provar_testcase_generate', { + test_case_name: 'Pure Var Test', + steps: [ + { + api_id: 'UiDoAction', + name: 'Pure var', + attributes: { value: '{AccountId}' }, + }, + ], + dry_run: true, + overwrite: false, + validate_after_edit: false, + }); + + assert.equal(isError(result), false); + const xml = parseText(result)['xml_content'] as string; + assert.ok(xml.includes('class="variable"'), 'Pure token should still use class="variable"'); + assert.ok(!xml.includes('class="compound"'), 'Should not emit compound for a pure variable token'); + }); + }); + describe('D7 — Cleanup warning for ApexDeleteObject', () => { it('includes cleanup warning when ApexDeleteObject is in the step list', () => { const result = server.call('provar_testcase_generate', { diff --git a/test/unit/mcp/testCaseStepTools.test.ts b/test/unit/mcp/testCaseStepTools.test.ts index 9e879b7..809cf3c 100644 --- a/test/unit/mcp/testCaseStepTools.test.ts +++ b/test/unit/mcp/testCaseStepTools.test.ts @@ -18,14 +18,17 @@ import { registerAllTestCaseStepTools } from '../../../src/mcp/tools/testCaseSte type ToolHandler = (args: Record) => unknown; class MockMcpServer { + public registrations: Array<{ name: string; description: string }> = []; private handlers = new Map(); public tool(name: string, _desc: string, _schema: unknown, handler: ToolHandler): void { this.handlers.set(name, handler); } - public registerTool(name: string, _config: unknown, handler: ToolHandler): void { + public registerTool(name: string, config: unknown, handler: ToolHandler): void { this.handlers.set(name, handler); + const desc = (config as Record)['description']; + if (typeof desc === 'string') this.registrations.push({ name, description: desc }); } public call(name: string, args: Record): ReturnType { @@ -78,6 +81,23 @@ afterEach(() => { fs.rmSync(tmpDir, { recursive: true, force: true }); }); +// ── tool description ────────────────────────────────────────────────────────── + +describe('provar_testcase_step_edit description', () => { + it('references corpus tool and step-reference fallback', () => { + const reg = server.registrations.find((r) => r.name === 'provar_testcase_step_edit'); + assert.ok(reg, 'tool should be registered'); + assert.ok( + reg.description.includes('provar_qualityhub_examples_retrieve'), + 'description should reference corpus tool' + ); + assert.ok( + reg.description.includes('provar://docs/step-reference'), + 'description should include step-reference fallback' + ); + }); +}); + // ── provar_testcase_step_edit ────────────────────────────────────────────────── describe('provar_testcase_step_edit', () => { diff --git a/test/unit/mcp/testCaseValidate.test.ts b/test/unit/mcp/testCaseValidate.test.ts index d735d7b..fa8ed15 100644 --- a/test/unit/mcp/testCaseValidate.test.ts +++ b/test/unit/mcp/testCaseValidate.test.ts @@ -788,6 +788,108 @@ describe('validateTestCase', () => { ); }); }); + + describe('VAR-REF-002', () => { + it('warns when {VarName} is embedded inside a larger string value', () => { + const r = validateTestCase( + ` + + + + + + SELECT Id FROM Account WHERE Id = '{AccountId}' + + + + +` + ); + assert.ok( + r.issues.some((i) => i.rule_id === 'VAR-REF-002'), + `Expected VAR-REF-002, got: ${JSON.stringify(r.issues.map((i) => i.rule_id))}` + ); + const issue = r.issues.find((i) => i.rule_id === 'VAR-REF-002')!; + assert.equal(issue.severity, 'WARNING'); + assert.ok(issue.message.includes('AccountId'), `Message should mention the variable: ${issue.message}`); + assert.ok(issue.suggestion?.includes('compound'), `Suggestion should mention compound: ${issue.suggestion}`); + }); + + it('warns for {NOW} system variable embedded in a SetValues string', () => { + const r = validateTestCase( + ` + + + + + + startDate=prefix_{NOW} + + + + +` + ); + assert.ok( + r.issues.some((i) => i.rule_id === 'VAR-REF-002'), + `Expected VAR-REF-002, got: ${JSON.stringify(r.issues.map((i) => i.rule_id))}` + ); + const issue = r.issues.find((i) => i.rule_id === 'VAR-REF-002')!; + assert.ok(issue.message.includes('NOW'), `Message should mention NOW: ${issue.message}`); + }); + + it('does NOT fire VAR-REF-002 for a pure {VarName} value (that is VAR-REF-001)', () => { + const r = validateTestCase( + ` + + + + + + {AccountId} + + + + +` + ); + assert.ok( + !r.issues.some((i) => i.rule_id === 'VAR-REF-002'), + 'VAR-REF-002 must not fire for a pure {VarName} value' + ); + assert.ok( + r.issues.some((i) => i.rule_id === 'VAR-REF-001'), + 'VAR-REF-001 should still fire for pure {VarName}' + ); + }); + + it('does NOT fire VAR-REF-002 for correct class="compound" XML', () => { + const r = validateTestCase( + ` + + + + + + + + SELECT Id FROM Account WHERE Id = ' + + ' + + + + + + +` + ); + assert.ok( + !r.issues.some((i) => i.rule_id === 'VAR-REF-002'), + 'VAR-REF-002 must not fire when class="compound" is used correctly' + ); + }); + }); }); // ── Handler-level tests (registerTestCaseValidate) ──────────────────────────── @@ -797,6 +899,7 @@ describe('registerTestCaseValidate handler', () => { // Cast to McpServer via unknown — safe because registerTestCaseValidate only calls server.tool(). class CapturingServer { public capturedHandler: ((args: Record) => Promise) | null = null; + public capturedDescription: string | null = null; // eslint-disable-next-line @typescript-eslint/no-explicit-any public tool(...args: any[]): void { this.capturedHandler = args[args.length - 1] as (args: Record) => Promise; @@ -804,6 +907,8 @@ describe('registerTestCaseValidate handler', () => { // eslint-disable-next-line @typescript-eslint/no-explicit-any public registerTool(...args: any[]): void { + const config = args[1] as { description?: string }; + if (config?.description) this.capturedDescription = config.description; this.capturedHandler = args[args.length - 1] as (args: Record) => Promise; } } @@ -935,3 +1040,26 @@ describe('validateTestCaseXml', () => { assert.throws(() => validateTestCaseXml(outside, makeConfig(tmpDir))); }); }); + +// ── tool description ────────────────────────────────────────────────────────── + +describe('provar_testcase_validate description', () => { + class DescriptionCapturingServer { + public capturedDescription: string | null = null; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + public registerTool(...args: any[]): void { + const config = args[1] as { description?: string }; + if (config?.description) this.capturedDescription = config.description; + } + } + + it('includes step-reference guidance', () => { + const srv = new DescriptionCapturingServer(); + registerTestCaseValidate(srv as unknown as McpServer, { allowedPaths: [] }); + assert.ok(srv.capturedDescription, 'description should be captured'); + assert.ok( + String(srv.capturedDescription).includes('provar://docs/step-reference'), + 'description should include step-reference guidance' + ); + }); +});