From 5d9303ac5b8a403c9a154c3f7eec90990b18cea0 Mon Sep 17 00:00:00 2001 From: Michael Dailey <49916244+mrdailey99@users.noreply.github.com> Date: Tue, 12 May 2026 16:29:40 -0500 Subject: [PATCH 1/2] 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. --- README.md | 2 +- docs/NITROX_CATALOG_SOURCE.json | 4 ++-- docs/mcp.md | 2 +- package.json | 3 ++- src/mcp/tools/nitroXTools.ts | 13 +++++++++++-- 5 files changed, 17 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 6057946..d601254 100644 --- a/README.md +++ b/README.md @@ -100,7 +100,7 @@ sf provar auth login claude mcp add provar -s user -- sf provar mcp start --allowed-paths /path/to/your/provar/project ``` -📖 **[docs/mcp.md](https://github.com/ProvarTesting/provardx-cli/blob/main/docs/mcp.md) — full setup, all 35+ tools, 7 MCP prompts, troubleshooting.** +📖 **[docs/mcp.md](https://github.com/ProvarTesting/provardx-cli/blob/main/docs/mcp.md) — full setup, all 35+ tools, 11 MCP prompts, troubleshooting.** --- diff --git a/docs/NITROX_CATALOG_SOURCE.json b/docs/NITROX_CATALOG_SOURCE.json index 08ab05b..ff330f0 100644 --- a/docs/NITROX_CATALOG_SOURCE.json +++ b/docs/NITROX_CATALOG_SOURCE.json @@ -1,6 +1,6 @@ { - "repo": "https://github.com/ProvarTesting/factPackages", "branch": "main", "commitSha": null, - "fetchedAt": null + "fetchedAt": null, + "schemasUpdated": null } diff --git a/docs/mcp.md b/docs/mcp.md index 57ed89a..592d8f4 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -1816,7 +1816,7 @@ The SF Hosted MCP uses per-user OAuth 2.0, respects field-level security and sha ## MCP Prompts -The Provar MCP server registers **7 MCP prompts** that pre-wire the tool chain into guided workflows. AI clients that support MCP prompts can invoke them directly by name instead of manually orchestrating the underlying tool sequence. **Important:** prompts that need to list, read, or write local project files (for example, `.testcase` files used by `provar.loop.fix` and `provar.loop.coverage`) also require a client with its own workspace/file tools, such as Claude Code or another MCP-compatible client with local file access configured; MCP prompt support alone is not sufficient for those workflows. +The Provar MCP server registers **11 MCP prompts** that pre-wire the tool chain into guided workflows. AI clients that support MCP prompts can invoke them directly by name instead of manually orchestrating the underlying tool sequence. **Important:** prompts that need to list, read, or write local project files (for example, `.testcase` files used by `provar.loop.fix` and `provar.loop.coverage`) also require a client with its own workspace/file tools, such as Claude Code or another MCP-compatible client with local file access configured; MCP prompt support alone is not sufficient for those workflows. --- diff --git a/package.json b/package.json index d0112bc..ceff6e6 100644 --- a/package.json +++ b/package.json @@ -40,7 +40,8 @@ "node": ">=18.0.0 <25.0.0" }, "bin": { - "provardx": "./bin/mcp-start.js" + "provardx": "./bin/mcp-start.js", + "provardx-cli": "./bin/mcp-start.js" }, "files": [ "/bin/mcp-start.js", diff --git a/src/mcp/tools/nitroXTools.ts b/src/mcp/tools/nitroXTools.ts index febddb2..eb07392 100644 --- a/src/mcp/tools/nitroXTools.ts +++ b/src/mcp/tools/nitroXTools.ts @@ -66,7 +66,16 @@ function getFactComponentValidator(): ValidateFunction | null { function ajvErrorToIssue(err: ErrorObject): NitroXIssue { const keyword = err.keyword.replace(/([a-z])([A-Z])/g, '$1_$2').toUpperCase(); const instancePath = err.instancePath; - const appliesTo = instancePath ? instancePath.replace(/^\//, '').replace(/\//g, '.') : 'root'; + // For additionalProperties/required, instancePath points to the parent object; + // the actual property name is in err.params. + const params = err.params as Record; + const extraProp = + err.keyword === 'additionalProperties' ? (params['additionalProperty'] as string | undefined) : undefined; + const missingProp = err.keyword === 'required' ? (params['missingProperty'] as string | undefined) : undefined; + const leafProp = extraProp ?? missingProp; + + const basePath = instancePath ? instancePath.replace(/^\//, '').replace(/\//g, '.') : 'root'; + const appliesTo = leafProp ? (basePath === 'root' ? leafProp : `${basePath}.${leafProp}`) : basePath; const pathParts = instancePath.split('/').filter(Boolean); const severity: 'ERROR' | 'WARNING' = ['REQUIRED', 'TYPE'].includes(keyword) ? 'ERROR' : 'WARNING'; const issue: NitroXIssue = { @@ -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; } From b6b302b30b08a8492484f4f7be2450f7809b59f3 Mon Sep 17 00:00:00 2001 From: Michael Dailey Date: Tue, 12 May 2026 16:32:39 -0500 Subject: [PATCH 2/2] 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. --- test/unit/mcp/nitroXTools.test.ts | 33 +++++++++++++++++++++++++------ 1 file changed, 27 insertions(+), 6 deletions(-) diff --git a/test/unit/mcp/nitroXTools.test.ts b/test/unit/mcp/nitroXTools.test.ts index 514f0bf..c03c660 100644 --- a/test/unit/mcp/nitroXTools.test.ts +++ b/test/unit/mcp/nitroXTools.test.ts @@ -422,6 +422,7 @@ describe('nitroXTools', () => { let extraPropsValidator!: ValidateFunction; let typeViolationValidator!: ValidateFunction; let permissiveValidator!: ValidateFunction; + let requiredValidator!: ValidateFunction; before(async () => { const mod = await import('../../../src/mcp/tools/nitroXTools.js'); @@ -453,19 +454,39 @@ describe('nitroXTools', () => { fieldDetailsElement: { type: 'boolean' }, }, }); + + requiredValidator = ajv.compile({ + type: 'object', + required: ['componentId'], + properties: { componentId: { type: 'string' } }, + }); }); - it('NX_SCHEMA_ADDITIONAL_PROPERTIES: extra property surfaces as WARNING', () => { - // Schema only allows componentId; passing an extra field should produce a schema issue + it('NX_SCHEMA_ADDITIONAL_PROPERTIES: extra property surfaces as WARNING with correct field and applies_to', () => { const result = validateFn({ componentId: VALID_UUID, _extraProp: true }, extraPropsValidator); - assert.ok(result.issues.some((i) => i.rule_id === 'NX_SCHEMA_ADDITIONAL_PROPERTIES')); - assert.equal(result.issues.find((i) => i.rule_id === 'NX_SCHEMA_ADDITIONAL_PROPERTIES')?.severity, 'WARNING'); + const issue = result.issues.find((i) => i.rule_id === 'NX_SCHEMA_ADDITIONAL_PROPERTIES'); + assert.ok(issue, 'expected NX_SCHEMA_ADDITIONAL_PROPERTIES issue'); + assert.equal(issue?.severity, 'WARNING'); + assert.equal(issue?.field, '_extraProp'); + assert.equal(issue?.applies_to, '_extraProp'); + }); + + it('NX_SCHEMA_REQUIRED: missing required property surfaces as ERROR with correct field and applies_to', () => { + const result = validateFn({}, requiredValidator); + const issue = result.issues.find((i) => i.rule_id === 'NX_SCHEMA_REQUIRED'); + assert.ok(issue, 'expected NX_SCHEMA_REQUIRED issue'); + assert.equal(issue?.severity, 'ERROR'); + assert.equal(issue?.field, 'componentId'); + assert.equal(issue?.applies_to, 'componentId'); }); it('NX_SCHEMA_TYPE: wrong property type surfaces as ERROR', () => { - // Schema expects pageStructureElement to be boolean; passing a string should produce a type error + // instancePath points directly to the offending property; field derives from pathParts const result = validateFn({ ...VALID_ROOT, pageStructureElement: 'yes' }, typeViolationValidator); - assert.ok(result.issues.some((i) => i.rule_id === 'NX_SCHEMA_TYPE' && i.severity === 'ERROR')); + const issue = result.issues.find((i) => i.rule_id === 'NX_SCHEMA_TYPE'); + assert.ok(issue && issue.severity === 'ERROR'); + assert.equal(issue?.field, 'pageStructureElement'); + assert.equal(issue?.applies_to, 'pageStructureElement'); }); it('valid object matching schema produces no NX_SCHEMA_ issues', () => {