feat(mcp): Wave 1 inner-loop improvements — divergence warning, JUnit steps, validateTestCaseXml export by mrdailey99 · Pull Request #133 · ProvarTesting/provardx-cli

mrdailey99 · 2026-04-24T21:06:51Z

Summary

Three targeted improvements to tighten the MCP agent debug loop (Wave 1):

A1 — provar.properties.read disk divergence warning: When the file being read differs from the one registered in ~/.sf/config.json, the tool now surfaces which path/resultsPath/provarHome fields differ so agents can self-correct without a manual inspection step. Path-policy guard added before reading the active file.
B1 — provar.automation.testrun JUnit step output: After each run, the tool parses JUnit XML from the results directory and returns a structured steps[] array (name, status, failure message). Agents get per-step pass/fail data without a separate file-read round-trip.
B4 prep — validateTestCaseXml export: Extracts the core XML validation logic from the MCP tool handler into a named export so Wave 2/3 features (test case generation, step editing) can validate in-process without an extra tool call.

Commits

SHA	Scope	Description
`a3820aa`	feat	A1 — disk divergence warning in provar.properties.read
`67aa066`	feat	B1 — JUnit structured step output in provar.automation.testrun
`81f4803`	feat	B4 prep — validateTestCaseXml export
`8fdfd04`	docs	docs/mcp.md updates for all three changes
`822eff7`	chore	version bump → 1.5.0-beta.10

Adversarial review findings fixed

Three CRITICAL issues caught and resolved before merge:

Path-policy bypass (propertiesTools.ts): Active properties file was read without assertPathAllowed() guard — added try/catch wrapper that silently skips if the path is outside allowed paths.
Silent partial data corruption (antTools.ts): When some JUnit XML files parsed and others failed, the partial data was returned without any warning. Refactored to track parseFailures count and emit a warning even when parsedAny=true.
Spread crash on large arrays (automationTools.ts): Math.max(...indices) would blow the call stack on result sets with thousands of indices — replaced with indices.reduce((a, b) => (a > b ? a : b), 0).

Test coverage

743 passing (was 729 before this branch — +14 tests)
New test suites: parseJUnitResults (6 tests in antTools.test.ts), validateTestCaseXml (3 tests in testCaseValidate.test.ts), JUnit enrichment (3 tests in automationTools.test.ts), divergence warning (2 tests in propertiesTools.test.ts)
Error code rename: FILE_NOT_FOUND → PROPERTIES_FILE_NOT_FOUND (breaking for any caller checking the exact code string — intentional, old code was ambiguous)

Docs

docs/mcp.md updated for:

PROPERTIES_FILE_NOT_FOUND error code (renamed from FILE_NOT_FOUND)
divergence_warning field in provar.properties.read response
steps[] array shape in provar.automation.testrun response

External docs flag: provar-mcp-public-docs.md / university-of-provar-mcp-course.md need manual update for the divergence warning and JUnit steps features (public-facing behaviour changes).

Test plan

node_modules/.bin/nyc node_modules/.bin/mocha "test/**/*.test.ts" → 743 passing
yarn compile → clean TypeScript
node scripts/mcp-smoke.cjs → all PASS (no new tools; count unchanged)
Manual: read a properties file that differs from the sf-config active path → divergence_warning present in response
Manual: run a test suite with failures → steps[] populated with failure details

🤖 Generated with Claude Code

…ad (A1) RCA: Agents silently used stale config values when reading a different properties file than the one registered in sf config, causing downstream test run failures. Fix: Compare provarHome/projectPath/resultsPath between active sf config and requested file, surface details.warning on divergence. Also renames FILE_NOT_FOUND -> PROPERTIES_FILE_NOT_FOUND and guards active-file path with assertPathAllowed. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…on.testrun (B1) RCA: After a test run agents had no structured per-test-case result data, only raw stdout, making failure diagnosis require manual log parsing. Fix: Parse JUnit XML files in the results directory post-run; return steps[] array with testItemId, title, status, and errorMessage on both success and failure responses. Handles bare <testsuite> and <testsuites> wrapper roots, partial parse failures, and falls back to details.warning when no valid XML is found. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…rep) RCA: Wave 2 (test case generation) and Wave 3 (step editing) both need to validate XML after mutations without spawning a separate MCP tool call round-trip. Fix: Export validateTestCaseXml(filePath, config) from testCaseValidate.ts; it enforces path policy, reads the file, and returns the same TestCaseValidationResult shape as the tool handler. Throws TESTCASE_FILE_NOT_FOUND on missing file. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

… code rename RCA: Three behaviour changes in this wave lacked documentation: provar.properties.read gained a divergence warning + error code rename (FILE_NOT_FOUND → PROPERTIES_FILE_NOT_FOUND), and provar.automation.testrun now returns a structured steps[] array from JUnit XML. Fix: Update docs/mcp.md to reflect PROPERTIES_FILE_NOT_FOUND error code, divergence_warning field, and the steps[] shape returned by provar.automation.testrun. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

RCA: This PR introduces new behaviour (divergence warning, JUnit step output, validateTestCaseXml export) that warrants a beta version bump per the project's publish policy. Fix: Increment beta suffix from beta.9 to beta.10 in package.json and both version fields in server.json to keep them in sync. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

This PR tightens the MCP inner-loop for Provar debugging by improving configuration self-diagnosis, enriching test run output with structured JUnit step results, and exporting core test case XML validation for reuse by upcoming features.

Changes:

Add a divergence warning to provar.properties.read when the disk file differs from the active SF-registered properties file on key fields.
Enrich provar.automation.testrun responses with structured steps[] parsed from JUnit XML in the results directory.
Export validateTestCaseXml() to validate test case files directly from disk under path policy.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
`src/mcp/tools/propertiesTools.ts`	Adds active-vs-disk divergence detection and updates missing-file error handling for properties read.
`src/mcp/tools/automationTools.ts`	Adds JUnit step enrichment logic for testrun output and test-only results-path override.
`src/mcp/tools/antTools.ts`	Introduces `parseJUnitResults()` to parse JUnit XML into structured step results.
`src/mcp/tools/testCaseValidate.ts`	Exports `validateTestCaseXml()` for disk-based validation under path policy.
`test/unit/mcp/propertiesTools.test.ts`	Updates error-code expectations and adds divergence-warning coverage.
`test/unit/mcp/automationTools.test.ts`	Adds tests for `steps[]` enrichment and warning behavior when XML is missing/malformed.
`test/unit/mcp/antTools.test.ts`	Adds unit tests for `parseJUnitResults()` behavior and edge cases.
`test/unit/mcp/testCaseValidate.test.ts`	Adds tests for `validateTestCaseXml()` (success, missing file, path-policy violation).
`docs/mcp.md`	Documents divergence warning + testrun steps output (needs a couple of accuracy tweaks).
`package.json` / `server.json`	Version bump to `1.5.0-beta.10`.

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

Copilot · 2026-04-24T21:11:20Z

+
+The `stdout` field is filtered before returning: Java schema-validator lines (`com.networknt.schema.*`) and stale logger-lock `SEVERE` warnings are stripped. If any lines were suppressed, `output_lines_suppressed` contains the count.
+
+After each run, the tool scans the results directory for `JUnit.xml` and adds a `steps` array when results are found:


Docs say the tool scans the results directory for JUnit.xml, but the implementation (parseJUnitResults) currently scans all non-hidden *.xml files. Update the docs to match the actual behavior (e.g., "scans the results directory for JUnit XML files").

Suggested change

After each run, the tool scans the results directory for `JUnit.xml` and adds a `steps` array when results are found:

After each run, the tool scans the results directory for JUnit XML files and adds a `steps` array when results are found:

Copilot · 2026-04-24T21:11:20Z

+**Output** — `{ file_path, content[, details.warning] }` where `content` is the parsed JSON object. `details.warning` is present when the file diverges from the active sf config.

-**Error codes:** `FILE_NOT_FOUND`, `MALFORMED_JSON`, `PATH_NOT_ALLOWED`
+**Error codes:** `PROPERTIES_FILE_NOT_FOUND`, `MALFORMED_JSON`, `PATH_NOT_ALLOWED`


The provar.properties.read docs section was updated but still doesn’t match the actual response/error surface: the tool includes a requestId in responses, and path-policy violations can yield PATH_TRAVERSAL as well as PATH_NOT_ALLOWED. Consider documenting requestId in the output shape and including PATH_TRAVERSAL in the listed error codes for consistency with the rest of the doc.

Copilot · 2026-04-24T21:11:21Z

+    const sfConfig = JSON.parse(fs.readFileSync(sfConfigPath, 'utf-8')) as Record<string, unknown>;
+    const propFilePath = sfConfig['PROVARDX_PROPERTIES_FILE_PATH'] as string | undefined;
+    if (!propFilePath || !fs.existsSync(propFilePath)) return null;
+    const props = JSON.parse(fs.readFileSync(propFilePath, 'utf-8')) as Record<string, unknown>;
+    const resultsBase = props['resultsPath'] as string | undefined;


readResultsPathFromSfConfig() reads ~/.sf/config.json, then reads/parses the properties file and (via parseJUnitResults) reads XML files from resultsPath, but none of these paths are checked against the MCP server's allowedPaths. Because resultsPath comes from a JSON file, a properties file inside allowedPaths could point resultsPath outside allowed roots and this enrichment would still read from disk. Consider threading ServerConfig into registerAutomationTestRun/readResultsPathFromSfConfig and using assertPathAllowed on propFilePath and the resolved results directory (or skipping enrichment with a warning when outside allowedPaths).

Copilot · 2026-04-24T21:11:21Z

+                type: 'text' as const,
+                text: JSON.stringify(
+                  makeError(
+                    'FILE_NOT_FOUND',


provar.properties.read now returns PROPERTIES_FILE_NOT_FOUND, but provar.properties.set still returns FILE_NOT_FOUND for the same missing-properties-file condition. This makes error handling inconsistent across the properties tools and contradicts the PR description that the old code was ambiguous. Consider standardizing on PROPERTIES_FILE_NOT_FOUND across read/set/validate (or documenting why only read is different).

Suggested change

'FILE_NOT_FOUND',

'PROPERTIES_FILE_NOT_FOUND',

Copilot · 2026-04-24T21:11:21Z

+              content: [
+                {
+                  type: 'text' as const,
+                  text: JSON.stringify(makeError('FILE_NOT_FOUND', `File not found: ${resolved}`, requestId)),


provar.properties.validate still returns FILE_NOT_FOUND when the properties file is missing, while provar.properties.read uses PROPERTIES_FILE_NOT_FOUND. Consider aligning the error code across the properties tool surface so callers can handle missing properties consistently.

Suggested change

text: JSON.stringify(makeError('FILE_NOT_FOUND', `File not found: ${resolved}`, requestId)),

text: JSON.stringify(makeError('PROPERTIES_FILE_NOT_FOUND', `File not found: ${resolved}`, requestId)),

RCA: Five issues flagged by code review: (1) readResultsPathFromSfConfig read sf config and results dir without path-policy guards, allowing properties files inside allowedPaths to redirect reads outside; (2-3) provar.properties.set and .validate still emitted FILE_NOT_FOUND while .read used PROPERTIES_FILE_NOT_FOUND, making error handling inconsistent; (4-5) docs/mcp.md referred to a specific "JUnit.xml" filename (impl scans all *.xml) and the .properties.read output/error sections were missing requestId and PATH_TRAVERSAL. Fix: Thread ServerConfig into readResultsPathFromSfConfig and registerAutomationTestRun; add assertPathAllowed guards on propFilePath and resultsDir before reading. Standardise to PROPERTIES_FILE_NOT_FOUND across all three properties tools (set, validate, read). Fix docs: "JUnit XML files", add requestId to output shape, add PATH_TRAVERSAL to properties.read error codes, align set/validate error codes in docs. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

mrdailey99 and others added 5 commits April 24, 2026 16:01

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

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

Copilot AI reviewed Apr 24, 2026

View reviewed changes

mrdailey99 merged commit 56d9640 into develop Apr 24, 2026
8 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(mcp): Wave 1 inner-loop improvements — divergence warning, JUnit steps, validateTestCaseXml export#133

feat(mcp): Wave 1 inner-loop improvements — divergence warning, JUnit steps, validateTestCaseXml export#133
mrdailey99 merged 6 commits intodevelopfrom
feature/wave1-debug-loop-fixes

mrdailey99 commented Apr 24, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 24, 2026

Uh oh!

Copilot AI Apr 24, 2026

Uh oh!

Copilot AI Apr 24, 2026

Uh oh!

Copilot AI Apr 24, 2026

Uh oh!

Copilot AI Apr 24, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants


		The `stdout` field is filtered before returning: Java schema-validator lines (`com.networknt.schema.*`) and stale logger-lock `SEVERE` warnings are stripped. If any lines were suppressed, `output_lines_suppressed` contains the count.

		After each run, the tool scans the results directory for `JUnit.xml` and adds a `steps` array when results are found:

	text: JSON.stringify(makeError('FILE_NOT_FOUND', `File not found: ${resolved}`, requestId)),
	text: JSON.stringify(makeError('PROPERTIES_FILE_NOT_FOUND', `File not found: ${resolved}`, requestId)),

Conversation

mrdailey99 commented Apr 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Commits

Adversarial review findings fixed

Test coverage

Docs

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mrdailey99 commented Apr 24, 2026 •

edited

Loading