Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion .github/workflows/CI_Execution.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand Down
15 changes: 9 additions & 6 deletions .github/workflows/DeployManual.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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 tagfind 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)
Comment on lines +68 to +73
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)
Expand Down
2 changes: 2 additions & 0 deletions docs/mcp-pilot-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
3 changes: 3 additions & 0 deletions docs/mcp.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 `<namedValues>` 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 `<path>` 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 `<parts>` 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.

---

Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -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": [
Expand Down
19 changes: 14 additions & 5 deletions server.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
28 changes: 23 additions & 5 deletions src/mcp/security/pathPolicy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}

Expand All @@ -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',
Expand Down
29 changes: 28 additions & 1 deletion src/mcp/tools/testCaseGenerate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -278,7 +279,29 @@ export function registerTestCaseGenerate(server: McpServer, config: ServerConfig

// ── XML builder ───────────────────────────────────────────────────────────────

// Build the <value> 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}<value valueClass="string">${escapeXmlContent(before)}</value>`);
const pathElements = m[1]
.split('.')
.map((p) => `${i} <path element="${escapeXmlAttr(p)}"/>`)
.join('\n');
parts.push(`${i}<variable>\n${pathElements}\n${i}</variable>`);
last = m.index + m[0].length;
}
const tail = val.slice(last);
if (tail) parts.push(`${i}<value valueClass="string">${escapeXmlContent(tail)}</value>`);
return `${indent}<value class="compound">\n${i}<parts>\n${parts.join('\n')}\n${i}</parts>\n${indent}</value>`;
}

// Build the <value> 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 {
Expand All @@ -291,6 +314,10 @@ function buildArgumentValue(key: string, val: string, indent: string, inNamedVal
.join('\n');
return `${indent}<value class="variable">\n${pathElements}\n${indent}</value>`;
}
// F1/F3: {VarName} embedded in surrounding text → class="compound" with <parts>.
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'))) {
Expand Down
1 change: 1 addition & 0 deletions src/mcp/tools/testCaseStepTools.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 <apiCall> 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'),
Expand Down
56 changes: 38 additions & 18 deletions src/mcp/tools/testCaseValidate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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), <steps> 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), <steps> 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'),
Expand Down Expand Up @@ -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 = /<value[^>]+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 <value class="variable"><path element="${varMatch[1]
.split('.')
.join(
'"/><path element="'
)}"/></value>. 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 = /<value[^>]+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 <value class="variable"><path element="${varName
.split('.')
.join(
'"/><path element="'
)}"/></value>. 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 <parts> 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);
Expand Down
10 changes: 5 additions & 5 deletions src/mcp/tools/testPlanTools.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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: [
Expand Down Expand Up @@ -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)) {
Expand All @@ -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) {
Expand Down
Loading
Loading