Skip to content

PDX-486: feat(mcp) — strict validator unknown-key detection (Thread A, PR 1/3)#184

Open
mrdailey99 wants to merge 2 commits into
developfrom
fix/PDX-486-validate-typo-a-strict-validator
Open

PDX-486: feat(mcp) — strict validator unknown-key detection (Thread A, PR 1/3)#184
mrdailey99 wants to merge 2 commits into
developfrom
fix/PDX-486-validate-typo-a-strict-validator

Conversation

@mrdailey99
Copy link
Copy Markdown
Collaborator

Context

A user typed testCases (plural) instead of testCase (singular) in a
provardx-properties.json file. provar_properties_validate accepted it
(the validator only checked required fields and enum values — unknown keys
were silently ignored). The subsequent provar_automation_testrun exited 0
with zero tests executed and falsely reported "tests run successfully." Two
safety nets failed in series.

This PR is VALIDATE-TYPO-A — the first half of the fix. It plugs the
validator gap.

The companion VALIDATE-TYPO-B (Thread B) adds the RUN-001 zero-tests
guard on provar_automation_testrun, and a final small Thread B commit
will wire validateProperties into provar_automation_config_load so
SCHEMA-001 surfaces before the shared sf config is written. See the umbrella
ticket PDX-486 for the full story.

This is Thread A, PR 1 of 3 in the parallel sequencing from
.claude/plans/file-c-users-mrdai-git-provar-manager-re-compressed-thunder.md.
The next two Thread A PRs (PDX-488 PROVARHOME-001 and PDX-494 PARALLEL-001)
will extend the canonical key sets exported here.

Depends on #182 (PDX-485 thread-prep: shared src/mcp/utils/warningCodes.ts).

Why

  • The validator was the first safety net that failed. Strict unknown-key
    detection closes it without breaking older clients (warning, not error).
  • The exported canonical key sets give sibling Thread A PRs a clean
    extension point — six threads referencing the same enum names converge
    cleanly; six threads inventing their own do not.
  • A "Did you mean ..." suggestion (Levenshtein ≤ 2, no new dependency)
    turns a silent failure into an obvious one for both humans and agents.

What changed

  • src/mcp/tools/propertiesTools.ts
    • Exports CANONICAL_TOP_LEVEL_KEYS, CANONICAL_METADATA_KEYS,
      CANONICAL_ENVIRONMENT_KEYS sourced from the existing required-field
      constants plus the documented optional fields and template defaults.
    • Adds inline levenshtein() and closestCanonicalKey() helpers.
    • validateProperties now emits SCHEMA-001 warnings for any unknown
      key at top-level, metadata.*, or environment.*.
    • provar_properties_validate tool description updated to call out the
      testCasestestCase example explicitly.
  • test/unit/mcp/propertiesTools.test.ts — 7 new test cases (top-level,
    metadata, environment, no-near-match, is_valid stays true, fixture round-trip).
  • test/fixtures/properties/testcases-typo.json — shared regression fixture
    with the testCases typo (Thread B and the smoke harness will reference it).
  • docs/mcp.mdprovar_properties_validate section updated with SCHEMA-001
    description, classic-typo example, and clarified output shape (errors
    vs warnings arrays; is_valid only flips on errors).

Out of scope (sibling PRs)

  • RUN-001 zero-tests guard on provar_automation_testrun — VALIDATE-TYPO-B (Thread B).
  • validateProperties wired into provar_automation_config_load — final Thread B commit.
  • Smoke-harness assertion for the typo fixture — VALIDATE-TYPO-B.
  • PROVARHOME-001 — PDX-488 (Thread A, PR 2).
  • PARALLEL-001 — PDX-494 (Thread A, PR 3).
  • package.json / server.json beta bump — sweeper PR after the batch lands.

Test plan

  • yarn compile clean (TypeScript)
  • yarn lint clean (ESLint + script-name lint)
  • node_modules/.bin/nyc node_modules/.bin/mocha "test/**/*.test.ts" — 1166 passing, 1 pending (unchanged from baseline)
  • 37 propertiesTools.test.ts tests pass, including the 7 new SCHEMA-001 cases
  • Husky commit-msg hook passes (RCA + Fix sections in body)
  • Reviewer: confirm canonical key sets cover everything in docs/mcp.md provar_properties_set schema (cross-reference lines 1082–1097)
  • Reviewer: confirm extension point is suitable for PDX-488 / PDX-494 (the exported CANONICAL_* constants)

Risk

Low. Warning-only behaviour — is_valid stays true on files that have
unknown keys, so no existing-caller breakage. Canonical sets are additive
(future Provar versions that add keys produce one extra warning until those
keys are added to the canonical set; no test run breakage).

mrdailey99 and others added 2 commits May 19, 2026 15:33
…-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>
…01 warning)

RCA: A user typo of 'testCases' (plural) instead of 'testCase' (singular) in
provardx-properties.json slipped past provar_properties_validate because the
validator only checked required fields and enum values — unknown keys were
silently accepted. The subsequent test run exited 0 with zero tests executed,
falsely reporting success. Two safety nets failed in series: the lenient
validator was the first.

Fix: Extend validateProperties to emit a SCHEMA-001 warning for any unknown
top-level, metadata.*, or environment.* key. Canonical key sets are exported
constants (CANONICAL_TOP_LEVEL_KEYS / CANONICAL_METADATA_KEYS /
CANONICAL_ENVIRONMENT_KEYS) so sibling PRs PDX-488 (PROVARHOME-001) and
PDX-494 (PARALLEL-001) can extend them. A minimal inline Levenshtein helper
suggests the closest canonical key within distance 2 ("Did you mean
'testCase'?"). Unknown keys are warnings (not errors) so additive Provar
schema versions do not break older MCP clients. The validate tool description
now references the testCases → testCase example explicitly. A regression
fixture lives at test/fixtures/properties/testcases-typo.json and is shared
with the VALIDATE-TYPO-B half (sibling PR, Thread B).

Out of scope for this PR (Thread A, PR 1 of 3): the testrun zero-tests guard
(VALIDATE-TYPO-B, Thread B), the config_load wiring (final Thread B commit),
PROVARHOME-001 (PDX-488), and PARALLEL-001 (PDX-494).

Depends on #182 (PDX-485 thread-prep: shared warningCodes.ts enum).
Copilot AI review requested due to automatic review settings May 19, 2026 21:17
@github-actions
Copy link
Copy Markdown

Quality Orchestrator

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


🧪 Tests to Run · Running 2 of 52 tests

  • unit/mcp/propertiesTools.test.ts
  • unit/mcp/utils/warningCodes.test.ts
▶ Run command
npx vitest run \
  unit/mcp/propertiesTools.test.ts \
  unit/mcp/utils/warningCodes.test.ts

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

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds strict unknown-key detection to provar_properties_validate (SCHEMA-001) so schema typos like testCases vs testCase are surfaced as warnings with “Did you mean …?” suggestions, preventing silent no-op test runs later in the workflow.

Changes:

  • Introduces shared WARNING_CODES + formatWarning() utility and documents warning codes in docs/mcp.md.
  • Exports canonical properties key sets and adds Levenshtein-based “closest key” suggestion + SCHEMA-001 warning emission for unknown keys in provardx-properties.json.
  • Adds unit tests and a regression fixture covering SCHEMA-001 behavior and suggestion/no-suggestion cases.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/mcp/utils/warningCodes.ts Adds canonical warning codes + standardized warning formatter used by validators/tools.
src/mcp/tools/propertiesTools.ts Implements canonical key sets + SCHEMA-001 unknown-key detection with Levenshtein suggestions.
test/unit/mcp/utils/warningCodes.test.ts Unit coverage for warning codes mapping and formatter behavior.
test/unit/mcp/propertiesTools.test.ts Adds SCHEMA-001 validation test cases (top-level/metadata/environment, suggestion threshold, fixture).
test/fixtures/properties/testcases-typo.json Regression fixture containing the testCases typo for shared reuse.
docs/mcp.md Documents warning codes + updates provar_properties_validate docs for SCHEMA-001 and output shape.

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

Comment thread docs/mcp.md
| `RUN-001` | `provar_automation_testrun` and friends | Test run produced no executable results — check input selection |
| `JUNIT-001` | report / RCA tooling | JUnit results file is missing, empty, or not parseable |

Warnings emitted programmatically follow the shape `WARNING [<CODE>]: <message>` — and when a typo is detected, the message is suffixed with `Did you mean '<suggestion>'?`. See `src/mcp/utils/warningCodes.ts` for the canonical enum.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants