PDX-476: fix(mcp): address Copilot review follow-ups from PR #161#162
Merged
Conversation
RCA: Four issues identified in post-merge Copilot review: AJV error mapping for additionalProperties/required used instancePath (parent object) instead of err.params, NITROX_CATALOG_SOURCE.json had an exposed repo field and missing schemasUpdated key, the provardx-cli bin alias was absent making npx resolution ambiguous, and prompt counts in README/docs were stale at 7 (now 11). Fix: ajvErrorToIssue now special-cases additionalProperties and required keywords to pull field/applies_to from err.params; committed JSON normalised to stable shape; package.json adds provardx-cli bin alias; README.md and docs/mcp.md updated to 11 MCP prompts.
Quality Orchestrator🟢 LOW · 🧪 Tests to Run · Running 1 of 43 tests
▶ Run commandnpx vitest run \
unit/mcp/nitroXTools.test.ts⚡ quality-orchestrator · |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses follow-ups from PR #161 by improving NitroX AJV schema validation issue mapping, stabilizing NitroX catalog source metadata shape, ensuring npx -y @provartesting/provardx-cli resolves a predictable bin name, and updating documentation to reflect the current number of MCP prompts.
Changes:
- Update
ajvErrorToIssueto derivefield/applies_tofromerr.paramsfor AJVadditionalPropertiesandrequirederrors. - Normalize
docs/NITROX_CATALOG_SOURCE.jsonto a stable/public shape (removerepo, addschemasUpdated: null). - Add a
provardx-clibin alias and refresh docs prompt counts (7 → 11).
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/mcp/tools/nitroXTools.ts | Improves AJV error → NitroX issue mapping for additionalProperties/required keywords. |
| README.md | Updates documented MCP prompt count to 11. |
| package.json | Adds provardx-cli bin alias pointing to the MCP start script. |
| docs/NITROX_CATALOG_SOURCE.json | Stabilizes catalog source metadata shape (remove repo, add schemasUpdated). |
| docs/mcp.md | Updates documented MCP prompt count to 11. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
69
to
88
| @@ -75,7 +84,7 @@ function ajvErrorToIssue(err: ErrorObject): NitroXIssue { | |||
| message: `Schema: ${instancePath || 'root'} — ${err.message ?? 'validation failed'}`, | |||
| applies_to: appliesTo, | |||
| }; | |||
| if (pathParts.length > 0) issue.field = pathParts[pathParts.length - 1]; | |||
| issue.field = leafProp ?? (pathParts.length > 0 ? pathParts[pathParts.length - 1] : undefined); | |||
| return issue; | |||
mrdailey99
added a commit
that referenced
this pull request
May 12, 2026
…talog source shape, bin alias, prompt count + test coverage (#164) * PDX-476: fix(mcp): address Copilot review follow-ups from PR #161 (#162) RCA: Four issues identified in post-merge Copilot review: AJV error mapping for additionalProperties/required used instancePath (parent object) instead of err.params, NITROX_CATALOG_SOURCE.json had an exposed repo field and missing schemasUpdated key, the provardx-cli bin alias was absent making npx resolution ambiguous, and prompt counts in README/docs were stale at 7 (now 11). Fix: ajvErrorToIssue now special-cases additionalProperties and required keywords to pull field/applies_to from err.params; committed JSON normalised to stable shape; package.json adds provardx-cli bin alias; README.md and docs/mcp.md updated to 11 MCP prompts. * PDX-476: test(mcp): assert field/applies_to for AJV additionalProperties and required errors RCA: Copilot review noted that the new ajvErrorToIssue params-based mapping for additionalProperties and required keywords had no test coverage for the field and applies_to values, leaving the mapping free to silently regress. Fix: Expanded NX_SCHEMA_ADDITIONAL_PROPERTIES test to assert field and applies_to equal the extra property name; added NX_SCHEMA_REQUIRED case asserting field and applies_to equal the missing property name; added applies_to/field assertions to the NX_SCHEMA_TYPE test.
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.
Summary
ajvErrorToIssueto useerr.params.additionalProperty/missingPropertyfor accuratefield/applies_toonadditionalPropertiesandrequiredAJV errorsdocs/NITROX_CATALOG_SOURCE.jsonto stable shape: removerepofield, addschemasUpdated: nullprovardx-clibin alias inpackage.jsonsonpx -y @provartesting/provardx-cliresolves unambiguously by package nameJira
https://provartesting.atlassian.net/browse/PDX-476
Test plan
yarn compilepassesyarn test:onlypasses (954 passing)yarn lintpassesnode scripts/mcp-smoke.cjspassesChanges
src/mcp/tools/nitroXTools.ts: special-caseadditionalProperties/requiredkeywords inajvErrorToIssueto pull property names fromerr.paramsdocs/NITROX_CATALOG_SOURCE.json: removerepo, addschemasUpdated: nullpackage.json: addprovardx-clibin alias pointing to./bin/mcp-start.jsREADME.md,docs/mcp.md: update prompt count from 7 to 11