PDX-486: feat(mcp) — testrun zero-tests guard (Thread B, PR 1/3)#185
Merged
mrdailey99 merged 3 commits intoMay 20, 2026
Merged
Conversation
RCA: provar_automation_testrun returned exitCode 0 with an empty steps[] array when a misspelled testCase/testCases key in provardx-properties.json caused the run to select nothing. Agents (and humans) had no signal to disambiguate "intentionally zero tests ran" from "typo silently dropped my entire selector", leading to wasted retry loops and downstream confusion when later tooling complained about missing results. Fix: Introduce a single JUnit introspection helper (introspectJUnit) that returns stepCount, parseWarning, and a resultsPathResolved flag. When sf exits 0 and the JUnit XML for the resolved results dir contains zero test cases, the response gains a warnings[] entry built via formatWarning(WARNING_CODES.RUN_001, ...) pointing the agent at the testCase/testCases spelling pitfall. The warning is additive — exitCode and isError remain unchanged. The helper is structured so future PDX-491 JUNIT-001 (expected-vs-actual mismatch) can read stepCount from the same struct without re-parsing. Updated the tool description, docs/mcp.md (RUN-001 output documentation), and added four targeted tests (positive, success-with-steps negative, non-zero-exit negative, unresolved-results-dir silent case).
Quality Orchestrator🟢 LOW · 🧪 Tests to Run · Running 2 of 53 tests
▶ Run commandnpx vitest run \
unit/mcp/antTools.test.ts \
unit/mcp/automationTools.test.ts⚡ quality-orchestrator · |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds a standardized warning-code surface to MCP and introduces a RUN-001 “zero tests executed” guard on provar_automation_testrun, so consumers can distinguish “success but nothing ran” from a normal successful run with results.
Changes:
- Added
WARNING_CODES+formatWarning()utility and corresponding unit tests. - Added JUnit introspection helper + success-path
warnings[]emission forRUN-001when step count is zero. - Updated MCP docs and
automationToolstests to document/cover the newwarnings[]output shape.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/mcp/utils/warningCodes.ts |
Introduces shared warning code constants and canonical formatter. |
src/mcp/tools/automationTools.ts |
Adds introspectJUnit() and emits RUN-001 warning on zero-step successful testruns. |
test/unit/mcp/utils/warningCodes.test.ts |
Adds unit coverage for code mapping and formatter behavior. |
test/unit/mcp/automationTools.test.ts |
Adds unit scenarios for the RUN-001 warning behavior. |
docs/mcp.md |
Documents warning codes and adds warnings[]/RUN-001 to testrun tool docs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+344
to
+349
| if (junit.resultsPathResolved && junit.stepCount === 0) { | ||
| const warningStr = formatWarning(WARNING_CODES.RUN_001, ZERO_TESTS_MESSAGE); | ||
| // Append rather than overwrite so future warning emitters (e.g. JUNIT-001 mismatch | ||
| // in PDX-491) can coexist on the same response without stepping on each other. | ||
| const existing = response['warnings'] as string[] | undefined; | ||
| response['warnings'] = existing ? existing.concat(warningStr) : [warningStr]; |
Comment on lines
+1411
to
+1416
| Each entry represents one test case. `status` is `"pass"`, `"fail"`, or `"skip"`. If the results directory cannot be located or contains no JUnit XML, `details.warning` explains why and `steps` is absent. | ||
|
|
||
| **Zero-tests guard (RUN-001):** when the sf command exits 0 but the JUnit report contains zero executed test cases, the response includes a `warnings[]` array containing a [`RUN-001`](#warning-codes) message. This is almost always a typo such as `testCase` vs `testCases` (or some other unknown key) in `provardx-properties.json` — the run silently selected nothing. The warning is additive and never flips `exitCode` or sets `isError`; the failure surface remains driven by the underlying `sf` exit code. | ||
|
|
||
| **Error codes:** `AUTOMATION_TESTRUN_FAILED`, `SF_NOT_FOUND` | ||
| **Warning codes:** `RUN-001` (zero tests executed despite success) |
Comment on lines
+239
to
+246
| describe('RUN-001 zero-tests-executed warning', () => { | ||
| const expectedWarning = formatWarning( | ||
| WARNING_CODES.RUN_001, | ||
| 'Test run exited successfully but zero tests were executed. ' + | ||
| 'Check the testCase / testCases (note spelling) field in provardx-properties.json.' | ||
| ); | ||
|
|
||
| it('fires when exit is 0 and JUnit step count is 0 (empty <testsuite>)', () => { |
…s when JUnit data missing RCA: The RUN-001 zero-tests-executed warning fired whenever `resultsPathResolved === true && stepCount === 0`. That condition is also true when the results dir was located but contained no XML files, or when every XML file failed to parse — in those cases the tool has no data on which to claim "zero tests ran" yet would still emit RUN-001, misdirecting the agent toward a properties-file typo when the real problem was an unreadable/unwritten results dir. Fix: Extended parseJUnitResults to return parsedAny: boolean (true iff at least one JUnit XML file was located AND parsed without throwing). Tightened the RUN-001 gate in introspectJUnit/registerAutomationTestRun to require `resultsPathResolved && parsedAny && stepCount === 0`. The "no XML found" and "all XML unparseable" branches now stay silent on warnings[] and surface only through details.warning, as the docs already described. Aligned the in-tree tool description and docs/mcp.md to spell out the parsedAny requirement, and added/strengthened tests covering the missing-XML, malformed-XML, and empty-<testsuite> (legit RUN-001) cases. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
mrdailey99
commented
May 20, 2026
Collaborator
Author
mrdailey99
left a comment
mrdailey99
left a comment
There was a problem hiding this comment.
Addressed in 3ca9379: tightened RUN-001 to require parsedAny; added missing-XML and malformed-XML negative tests; docs aligned.
13 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
When
provar_automation_testrunexits 0 but no tests actually executed — most often because theprovardx-properties.jsonhastestCaseinstead oftestCases(or vice versa, or any unknown selector key) — the response previously contained an emptysteps[]array and no other signal. Agents (and humans) had no way to distinguish "intentionally zero tests ran" from "typo silently dropped my entire selector," leading to wasted retry loops and downstream confusion when later tooling tried to read results.This PR is Thread B, PR 1 of 3 in the VALIDATE-TYPO / propagation track. It introduces the
RUN-001zero-tests guard onprovar_automation_testrun.Why
testCase↔testCases).WARNING_CODESenum from PDX-485 (PDX-485: chore(mcp) — shared warningCodes.ts enum #182) in production tooling.introspectJUnit) so the futureJUNIT-001mismatch warning (PDX-491, Thread B PR 3) can extend it cleanly without re-parsing the XML.What changed
src/mcp/tools/automationTools.tsintrospectJUnit(config)helper returning{ steps, stepCount, parseWarning, resultsPathResolved }— single read-point for downstream warning emitters.formatWarning(WARNING_CODES.RUN_001, …)entry to awarnings[]array whenresultsPathResolved && stepCount === 0. Strictly additive —exitCodeandisErrorare untouched.testCase/testCasesspelling pitfall.docs/mcp.md— documents the newwarnings[]field on the testrun output and referencesRUN-001.test/unit/mcp/automationTools.test.ts— four new tests covering: fires on empty<testsuite>, does not fire when steps ≥ 1, does not fire on non-zero exit, does not fire when results dir is unresolved. The expected warning string is built viaformatWarning(no inline duplicates).Out of scope (sibling PRs)
provar_automation_config_loadpropagatingSCHEMA-001— scheduled for Thread B PR 3 stitch commit after both halves merge.properties_fileinline param (PDX-487) — Thread B PR 2.introspectJUnit).Test plan
yarn compileclean.yarn lintclean.node_modules/.bin/nyc node_modules/.bin/mocha "test/**/*.test.ts") — 1163 passing, 1 pending, 0 failing.automationTools.test.ts— 80/80 passing, includes the four new RUN-001 scenarios.Dependencies
chore/PDX-485-warning-codes-enum) — usesWARNING_CODES.RUN_001andformatWarningfromsrc/mcp/utils/warningCodes.ts. PDX-485 is already merged into this branch.testcases-typo.jsonfixture. This PR uses its own inline JUnit XML fixtures to avoid coupling.