PDX-489: feat(mcp) — DATA-001 warning for dataTable in direct testCase-mode (Thread E) by mrdailey99 · Pull Request #187 · ProvarTesting/provardx-cli

mrdailey99 · 2026-05-19T21:25:43Z

Summary

Adds project-context-aware emission of the DATA-001 validator warning: now fires only when <dataTable> is present AND the project's provardx-properties.json references the test case via top-level testCase / testCases rather than via a .testinstance inside plans/.
New helper src/mcp/utils/testCasePlanMode.ts resolves the active properties file (via ~/.sf/config.json → PROVARDX_PROPERTIES_FILE_PATH), reads projectPath, then determines whether the test case is referenced directly, via a plan instance, or unknown (no project context). Resolution is best-effort and silent — any read or path-policy failure collapses to mode: 'unknown', where the validator falls back to the structural advisory.
DATA-001 messaging:
- direct → WARNING [DATA-001]: <dataTable> only iterates when run through a test plan instance. Direct testCase-mode execution will resolve all data-driven variables as null. Add this test to a plan via provar_testplan_add-instance. (via formatWarning(WARNING_CODES.DATA_001, ...)).
- plan → suppressed entirely (data-driven iteration works under plan mode).
- unknown → existing structural advisory, preserving backward compatibility for pure-function callers.
Tool description for provar_testcase_validate references the new behaviour.
docs/mcp.md: new "Data-driven execution" subsection cross-links provar_testcase_validate, provar_testcase_generate, provar_automation_testrun, and provar_testplan_add-instance; refreshes the DATA-001 row in the warning-codes table; updates the DATA-001 entry in the validator section.
Tests: 8 new unit tests for resolveTestCasePlanMode (direct / testCases plural / plan-wins / unknown / missing config / outside allowed paths) and 8 new DATA-001 tests in the validator (handler-level integration through validateTestCaseXml covers testCase, testCases, plan-instance, no-dataTable; pure-function tests cover the four planMode permutations).

Dependencies

Depends on PDX-485: chore(mcp) — shared warningCodes.ts enum #182 (chore/PDX-485-warning-codes-enum) for src/mcp/utils/warningCodes.ts (WARNING_CODES.DATA_001 and formatWarning). That branch is merged into this PR so it can build standalone; rebase on develop once PDX-485: chore(mcp) — shared warningCodes.ts enum #182 lands.

Scope notes (Thread E)

Per the per-thread file-ownership map in the Sessions 6–7 plan, this PR owns src/mcp/tools/testCaseValidate.ts and does not touch propertiesTools.ts, automationTools.ts, or testCaseGenerate.ts. The original brief mentioned adding a one-liner about the dataTable limitation to the provar_testcase_generate and provar_automation_testrun .describe() text — testCaseGenerate.ts already carries that note (lines 140–142), and the automationTools.ts change is deferred to Thread B which owns that file.

No external customer-facing docs (docs/provar-mcp-public-docs.md, docs/university-of-provar-mcp-course.md) were edited per CLAUDE.md.

Test plan

yarn compile clean
yarn lint clean (including lint:script-names)
node_modules/.bin/nyc node_modules/.bin/mocha "test/**/*.test.ts" — 1174 passing, 1 pending (matches pre-change baseline)
New tests fire correctly for direct, testCases-plural, plan, and no-dataTable scenarios
node scripts/mcp-smoke.cjs — runs locally only with an activated Provar license; verified that no validator regressions appear in unit suite

🤖 Generated with Claude Code

…-thread feedback codes RCA: Six sibling Provar MCP thread PRs (validation, properties, automation, RCA, JUnit, parallel-mode tuning) each independently emit warnings with ad-hoc string prefixes — `WARNING:`, `[provarHome]`, `WARN —` — producing inconsistent surface area for AI agents downstream and making cross-tool typo guidance ("Did you mean...?") harder to standardise. PDX-485 wants a single canonical enum the threads can import, so warning codes are coined once, formatted once, and documented once. Fix: Adds src/mcp/utils/warningCodes.ts exporting WARNING_CODES (PROVARHOME-001, DATA-001, PARALLEL-001, SCHEMA-001, RUN-001, JUNIT-001), a WarningCode type derived from the enum, and a formatWarning(code, message, suggestion?) helper that emits `WARNING [<CODE>]: <message>` and appends ` Did you mean '<suggestion>'?` when a suggestion is provided. No call sites are touched in this PR — surface area is intentionally minimal so the six sibling thread PRs can import and adopt without merge conflicts. docs/mcp.md gains a new "Warning codes" reference table linked from the table of contents; per-row meanings are placeholders that subsequent thread PRs will refine. Tests: New test/unit/mcp/utils/warningCodes.test.ts covers (1) each WARNING_CODES key maps to its expected wire string, (2) formatWarning without a suggestion returns the prefixed message exactly, (3) formatWarning with a suggestion appends the "Did you mean" suffix exactly, (4) an empty-string suggestion is treated as no suggestion. Validation: yarn compile clean, yarn lint clean, full mocha 1159 passing / 0 failing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…DATA-001) RCA: Provar's `<dataTable>` element only iterates row-by-row when the test case runs through a test-plan instance (`.testinstance`). When the project's provardx-properties.json references the test case directly via top-level `testCase` or `testCases`, the runtime silently ignores the data table and every variable reference resolves to null. The test run reports success against an empty row set — a silent-pass mode that AI agents and humans both miss until a downstream assertion fails for an unrelated reason. Prior to this change the validator emitted a generic DATA-001 advisory whenever a `<dataTable>` element was present, with no awareness of how the test would be wired at run time. That generic warning was also noisy in plan-mode projects where data-driven execution works correctly. Fix: Introduce `src/mcp/utils/testCasePlanMode.ts`, a resolver that consults the active `~/.sf/config.json` (`PROVARDX_PROPERTIES_FILE_PATH`) for the project's properties file, resolves `projectPath`, walks `plans/**/*.testinstance` for `testCasePath=` references to the file under validation, and returns `'direct' | 'plan' | 'unknown'`. The MCP handler in `registerTestCaseValidate` and the disk-backed `validateTestCaseXml` entry both call the resolver and pass the mode through `validateTestCase` via a new `ValidateTestCaseOptions` parameter. DATA-001 is now suppressed under plan mode, fires with the `formatWarning(WARNING_CODES.DATA_001, ...)` advisory under direct mode (recommending `provar_testplan_add-instance`), and falls back to the structural advisory under unknown mode (pure function called without file resolution — keeps existing callers' behaviour). The new helper, the new mode-aware emission helper (`maybeEmitDataTableWarning`), and the handler-level integration are covered by 12 new tests in `test/unit/mcp/testCasePlanMode.test.ts` and the PDX-489 block of `test/unit/mcp/testCaseValidate.test.ts`. Documentation: the tool description for `provar_testcase_validate` references the new behaviour, a new "Data-driven execution" subsection in `docs/mcp.md` explains the direct-vs-plan modes and the recommended workaround, the warning-codes table cross-links to it, and the DATA-001 entry in the validator section spells out the suppression rule. Thread E (testCaseValidate.ts) of the Sessions 6–7 plan; does not touch automationTools.ts or testCaseGenerate.ts per the per-thread file-ownership map (the `provar_automation_testrun` description-text reference noted in the original brief is deferred to Thread B which owns that file).

github-actions · 2026-05-19T21:26:07Z

Quality Orchestrator

🟢 LOW · 16 / 100 · Touches: utils. All changed files have mapped tests.

🧪 Tests to Run · Running 3 of 53 tests

unit/mcp/testCaseValidate.test.ts
unit/mcp/testCasePlanMode.test.ts
unit/mcp/utils/warningCodes.test.ts

▶ Run command

npx vitest run \
  unit/mcp/testCaseValidate.test.ts \
  unit/mcp/testCasePlanMode.test.ts \
  unit/mcp/utils/warningCodes.test.ts

_{⚡ quality-orchestrator · /qo stub <file> · qo analyze-local}

Copilot

Pull request overview

This PR refines how the MCP provar_testcase_validate validator surfaces the DATA-001 warning by making it project-context-aware (direct testCase-mode vs plan-instance mode), and introduces shared warning-code formatting utilities plus supporting docs/tests.

Changes:

Added resolveTestCasePlanMode helper to infer whether a test case is referenced directly (testCase/testCases) or via a plan .testinstance, with best-effort fallback to unknown.
Updated test case validation to (a) suppress DATA-001 in plan mode, (b) emit a standardized WARNING [DATA-001]: ... message in direct mode, and (c) preserve the prior structural advisory in unknown mode.
Expanded docs and unit tests to cover the new warning behavior and plan-mode resolver.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
`src/mcp/tools/testCaseValidate.ts`	Threads resolved plan mode into validation so DATA-001 messaging/suppression is context-aware and uses shared warning formatting.
`src/mcp/utils/testCasePlanMode.ts`	New helper that resolves project properties context and determines direct/plan/unknown execution mode.
`src/mcp/utils/warningCodes.ts`	New shared `WARNING_CODES` enum and `formatWarning` helper for consistent warning text formatting.
`test/unit/mcp/testCaseValidate.test.ts`	Adds unit + handler-level integration tests validating DATA-001 behavior across plan modes.
`test/unit/mcp/testCasePlanMode.test.ts`	New unit tests for `resolveTestCasePlanMode` across direct/plan/unknown and policy/error fallbacks.
`test/unit/mcp/utils/warningCodes.test.ts`	New unit tests for `WARNING_CODES` mapping and `formatWarning` behavior (including empty suggestion).
`docs/mcp.md`	Documents warning codes, DATA-001 behavior, and adds a new “Data-driven execution” section + TOC updates.

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

+function readPropertiesFilePath(opts: ResolveOptions): string | null {
+  if (opts.propertiesFilePathOverride) {
+    return opts.propertiesFilePathOverride;
+  }


  - [provar_testplan_create-suite](#provar_testplan_create-suite)
  - [provar_testplan_remove-instance](#provar_testplan_remove-instance)
+- [Data-driven execution](#data-driven-execution)
  - [NitroX — Hybrid Model page objects](#nitrox--hybrid-model-page-objects)
    - [provar_nitrox_discover](#provar_nitrox_discover)
    - [provar_nitrox_read](#provar_nitrox_read)


…erride + repair docs TOC RCA: Copilot review on PR #187 flagged a path-policy bypass in resolveTestCasePlanMode: the propertiesFilePathOverride parameter was returned directly from readPropertiesFilePath without going through assertPathAllowed, so a future caller could read a properties file outside the configured allowedPaths tree. A separate Copilot finding caught a broken docs TOC — adding the Data-driven execution top-level bullet left the NitroX, Quality Hub, and Org metadata bullets indented as if nested, misrepresenting the section hierarchy. Fix: Funnel both the override and the value read from ~/.sf/config.json through a shared enforceAllowed helper that resolves the path, calls assertPathAllowed, canonicalises symlinks via realpathSync when the file exists, and collapses any policy violation or missing file to null to preserve the resolver's best-effort silent-fallback contract. Re-leveled the docs/mcp.md TOC so NitroX and Quality Hub sit at the top level (matching their ## headings) with Org metadata nested under Quality Hub (matching its ### heading). Added two regression tests: override inside allowed paths is consulted normally; override outside allowed paths collapses to mode 'unknown' without throwing and without exposing the rejected path.

mrdailey99

Addressed in e46b7a9: assertPathAllowed now enforced on override path with safe null fallback; TOC re-leveled.

…link canonicalisation RCA: macOS CI surfaced a test failure in resolveTestCasePlanMode's "direct" assertion. On Mac, os.tmpdir() returns /var/folders/... but /var is a symlink to /private/var. The Copilot-fix commit (e46b7a9) added fs.realpathSync to enforceAllowed for symlink canonicalisation — correct security behaviour — but the test compared result.propertiesFilePath against a path built from un-realpath'd os.tmpdir(), so the resolved /private/var/... did not match the expected /var/... and the assertion blew up on Mac CI (passed on Windows because /var is a regular dir there). Fix: Wrap the mkdtempSync return in fs.realpathSync inside beforeEach so the tmp root is already canonicalised. Every derived path (projectPath, testCasePath, propertiesPath) now uses the canonical /private/var/... form on Mac, matching what the resolver returns. No code change to the helper — its realpathSync behaviour is correct and stays. Windows behaviour unchanged because realpathSync is a no-op on paths without symlinks. Full mocha 1176 passing, yarn lint clean.

mrdailey99 · 2026-05-20T13:36:06Z

macOS CI fix pushed in dbe4f37.

Root cause: Test fixture root came from os.tmpdir() → /var/folders/... on Mac. The new enforceAllowed helper from e46b7a9 calls fs.realpathSync for symlink canonicalisation (correct security behaviour), which on Mac resolves /var → /private/var. The test asserted equality against the un-realpath'd expected path, so the resolved result diverged from expectation only on Mac.

Fix: Wrap mkdtempSync in fs.realpathSync inside beforeEach, so the tmp root is already canonicalised before any derived path is constructed. Implementation code unchanged — its realpathSync is the right behaviour and stays. Windows is unaffected because realpathSync is a no-op on paths without symlinks.

Verified locally: full mocha 1176 passing / 0 failing, yarn lint clean.

mrdailey99 and others added 2 commits May 19, 2026 15:33

Copilot AI review requested due to automatic review settings May 19, 2026 21:25

Copilot started reviewing on behalf of mrdailey99 May 19, 2026 21:26 View session

Copilot AI reviewed May 19, 2026

View reviewed changes

mrdailey99 commented May 20, 2026

View reviewed changes

mrdailey99 merged commit 00423b9 into develop May 20, 2026
4 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PDX-489: feat(mcp) — DATA-001 warning for dataTable in direct testCase-mode (Thread E)#187

PDX-489: feat(mcp) — DATA-001 warning for dataTable in direct testCase-mode (Thread E)#187
mrdailey99 merged 4 commits into
developfrom
feature/PDX-489-datatable-direct-mode-warning

mrdailey99 commented May 19, 2026

Uh oh!

github-actions Bot commented May 19, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

mrdailey99 left a comment

Uh oh!

mrdailey99 commented May 20, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

mrdailey99 commented May 19, 2026

Summary

Dependencies

Scope notes (Thread E)

Test plan

Uh oh!

github-actions Bot commented May 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Quality Orchestrator

🧪 Tests to Run · Running 3 of 53 tests

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

mrdailey99 left a comment

Choose a reason for hiding this comment

Uh oh!

mrdailey99 commented May 20, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions Bot commented May 19, 2026 •

edited

Loading