Develop#137
Open
mrdailey99 wants to merge 8 commits intomainfrom
Open
Conversation
D1: add provar.testplan.create tool — creates plans/{name}/ directory and
root .planitem; previously agents had to create plan files manually.
D2: emit class="uiTarget" uri="..." for the 'target' argument and
class="uiLocator" uri="..." for 'locator' argument in XML generation;
previously both were written as valueClass="string" causing a
ClassCastException in the Provar runtime.
D3: SetValues / AssertValues steps now emit <value class="valueList"
mutable="Mutable"><namedValues>...</namedValues></value> for the
'values' argument; previously a pipe-delimited string was written,
causing an immediate ClassCastException.
D4: attribute values matching {VarName} or {Obj.Field} are emitted as
class="variable" with <path element="..."/> children; previously
emitted as plain string literals which the runtime rejects.
D7: provar.testcase.generate now includes a cleanup warning when
ApexDeleteObject steps are present, explaining that cleanup steps
after a failure point are skipped when stopOnError=false.
Also updates StepSchema.attributes description and TOOL_DESCRIPTION to
document all four special value conventions for agent grounding.
Tests: 19 new assertions across testCaseGenerate.test.ts and
testPlanTools.test.ts covering happy path, dry_run, and error cases.
Smoke test: +1 entry for provar.testplan.create (TOTAL_EXPECTED 50->51).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…TRUCTURE-001, VAR-REF-001 rules Addresses Session 3 feedback D2 (uiTarget class), D3 (SetValues valueList structure), and D4 (variable reference detection). Mirrors quality-hub-agents backend rule IDs where they exist; VAR-REF-001 fills a gap present in both local and backend validation.
D1: add assertPathAllowed(planDir) in testPlanTools create-plan path D2: skip uiTarget/uiLocator dispatch inside SetValues namedValues context D3: remove AssertValues from SET_VALUES_APIS; update test to expect flat args Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… rules Add docs for UI-TARGET-001, UI-LOCATOR-001, SETVALUES-STRUCTURE-001, VAR-REF-001 Document generator XML argument conventions (uiTarget/uiLocator/variable/valueList) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Separate SetValues/AssertValues in StepSchema and TOOL_DESCRIPTION
- Replace SET_VALUES_APIS Set with resolvedApiId.includes('SetValues') (mirrors validator)
- Make target/locator dispatch API-aware (pass resolvedApiId through buildArgumentsXml)
- Fix buildUiWithScreenXml to pass wrapperApiId so target emits uiTarget class
- Validate plan_name has no path separators to prevent path.join injection
- Add UiScrollToElement to UI-LOCATOR-001 docs and argument convention table
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…vidence) UiScrollToElement has zero usage in the internal test corpus and no documented locator argument. The inclusion was speculative — reverting to UiDoAction/UiAssert only. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
feat(validate): Wave 3 — XML generator correctness + 4 new local validator rules
Contributor
There was a problem hiding this comment.
Pull request overview
This PR expands the MCP toolset around Provar test authoring by adding a new test plan creation tool and tightening test case XML generation/validation to better match Provar runtime expectations.
Changes:
- Add
provar.testplan.createMCP tool (and corresponding unit + smoke tests). - Enhance
provar.testcase.generate/validateTestCaseto emit/validate correct XML structures foruiTarget,uiLocator,SetValuesvalueList, and{Var}variable references, plus add anApexDeleteObjectcleanup warning. - Bump package/server versions to
1.5.0-beta.12and update docs.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| test/unit/mcp/testPlanTools.test.ts | Adds coverage for the new provar.testplan.create tool and updates “register all tools” expectations. |
| test/unit/mcp/testCaseValidate.test.ts | Adds new validation rule tests (UI-TARGET-001, UI-LOCATOR-001, SETVALUES-STRUCTURE-001, VAR-REF-001). |
| test/unit/mcp/testCaseGenerate.test.ts | Adds generator tests for uiTarget/uiLocator, SetValues valueList structure, variable refs, and cleanup warnings. |
| src/mcp/tools/testPlanTools.ts | Implements registerTestPlanCreate and wires it into registerAllTestPlanTools. |
| src/mcp/tools/testCaseValidate.ts | Adds argument-structure validation rules and a regex-based warning for {Var} literals stored as strings. |
| src/mcp/tools/testCaseGenerate.ts | Updates argument XML emission to support uiTarget/uiLocator, variable refs, SetValues valueList/namedValues, and adds cleanup warning. |
| server.json | Version bump to 1.5.0-beta.12. |
| scripts/mcp-smoke.cjs | Adds a smoke call for provar.testplan.create and updates expected response count. |
| package.json | Version bump to 1.5.0-beta.12. |
| docs/mcp.md | Documents new generator conventions and new validation rules. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+108
to
+121
| if (plan_name.includes('/') || plan_name.includes('\\') || path.isAbsolute(plan_name)) { | ||
| return { | ||
| isError: true, | ||
| content: [ | ||
| { | ||
| type: 'text' as const, | ||
| text: JSON.stringify(makeError('INVALID_PLAN_NAME', `plan_name must be a simple directory name without path separators: "${plan_name}"`, requestId)), | ||
| }, | ||
| ], | ||
| }; | ||
| } | ||
| const planDir = path.join(projectRoot, 'plans', plan_name); | ||
| assertPathAllowed(planDir, config.allowedPaths); | ||
| const planItemPath = path.join(planDir, '.planitem'); |
Comment on lines
+410
to
+415
| if (valClass && valClass !== 'uiTarget') { | ||
| const apiLabel = apiId.includes('UiWithRow') ? 'UiWithRow' : 'UiWithScreen'; | ||
| issues.push({ | ||
| rule_id: 'UI-TARGET-001', | ||
| severity: 'ERROR', | ||
| message: `${apiLabel} step "${stepName}" target argument uses class="${valClass}" — must be class="uiTarget".`, |
Comment on lines
+441
to
+456
| if (apiId.includes('UiDoAction') || apiId.includes('UiAssert')) { | ||
| const locatorArg = getArgList(call).find((a) => (a['@_id'] as string | undefined) === 'locator'); | ||
| if (locatorArg) { | ||
| const valClass = (locatorArg['value'] as Record<string, unknown> | undefined)?.['@_class'] as string | undefined; | ||
| if (valClass && valClass !== 'uiLocator') { | ||
| issues.push({ | ||
| rule_id: 'UI-LOCATOR-001', | ||
| severity: 'ERROR', | ||
| message: `"${stepName}" locator argument uses class="${valClass}" — must be class="uiLocator".`, | ||
| applies_to: 'apiCall', | ||
| suggestion: | ||
| 'Emit the locator as: <value class="uiLocator" uri="sf:ui:locator:..."/>. ' + | ||
| 'In provar.testcase.generate the "locator" attribute is converted automatically.', | ||
| }); | ||
| } | ||
| } |
Comment on lines
+462
to
478
| if (apiId.includes('SetValues') && !apiId.includes('AssertValues')) { | ||
| const valuesArg = getArgList(call).find((a) => (a['@_id'] as string | undefined) === 'values'); | ||
| if (valuesArg) { | ||
| const valClass = (valuesArg['value'] as Record<string, unknown> | undefined)?.['@_class'] as string | undefined; | ||
| if (valClass && valClass !== 'valueList') { | ||
| issues.push({ | ||
| rule_id: 'ASSERT-001', | ||
| severity: 'WARNING', | ||
| message: `AssertValues step "${ | ||
| name ?? '(unnamed)' | ||
| }" uses namedValues format (argument id="values") — designed for UI element attribute assertions. For Apex/SOQL result or variable comparisons this silently passes as null=null.`, | ||
| rule_id: 'SETVALUES-STRUCTURE-001', | ||
| severity: 'ERROR', | ||
| message: `SetValues step "${stepName}" values argument uses class="${valClass}" — must use class="valueList" with <namedValues> children.`, | ||
| applies_to: 'apiCall', | ||
| suggestion: | ||
| 'Use separate expectedValue, actualValue, and comparisonType arguments for variable or Apex result comparisons.', | ||
| 'Wrap variable assignments in: <value class="valueList" mutable="Mutable"><namedValues>' + | ||
| '<namedValue name="varName"><value class="value" valueClass="string">value</value></namedValue>' + | ||
| '</namedValues></value>. In provar.testcase.generate pass each variable as a flat key/value pair ' + | ||
| 'in attributes — the generator builds the valueList structure automatically.', | ||
| }); | ||
| } |
mrdailey99
added a commit
that referenced
this pull request
May 1, 2026
- plan_name: tighten validation to reject dot-segments (..) via safe-name regex; previously plan_name=".." resolved to projectRoot - UI-TARGET-001 / UI-LOCATOR-001 / SETVALUES-STRUCTURE-001: fire when <value> node is present but class attribute is absent, not only when class is wrong; distinguish via "(missing)" in error message - Add 5 tests covering the new missing-class and dot-segment cases Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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.
No description provided.