Develop#201
Merged
Merged
Conversation
… prompt RCA: Autonomous /loop runs for ticket batches had no persisted, reusable scaffold, so each run was re-authored ad hoc and the workflow/decisions were not captured for future batches. Fix: Added docs/loops/ with loop-template.md (generic template, how-to, decisions, registry), _example-instance.md skeleton, and pdx-501-505.md ready-to-run prompt with step-scoped ComparisonType sets.
RCA: Copilot review found the loop docs referenced .claude/agents/dev-git-workflow.md, which is untracked in git and therefore absent in fresh checkouts and worktrees the loop creates, making the instructions impossible to follow. Fix: Replaced the untracked .claude/agents/dev-git-workflow.md reference with the tracked CLAUDE.md across loop-template.md and pdx-501-505.md; only .claude/commands/git-workflow.md + CLAUDE.md are now cited.
PDX-0: docs — reusable loop runner template + PDX-501..505 prompt
RCA: comparisonType is one Provar enum but each step type accepts only a subset; the step-reference, both best-practice rule strings, and docs/mcp.md listed a single flat/incorrect set, so agents emitted load-blocking values such as NotEqualTo on a UI Assert. Fix: scope the guidance into the AssertValues 16-value subset and the UI Assert 6-value subset matching Provar core ComparisonType exactly, in PROVAR_TEST_STEP_REFERENCE.md, both rule strings, and docs/mcp.md, plus schema-aware field-type notes (encrypted->null, rich-text wrapped, Contains direction).
…-sets PDX-501: scope comparisonType enum sets by step type
RCA: A comparisonType used outside the subset its step type allows (e.g. NotEqualTo on a UI Assert) is load-blocking, but nothing caught it locally — offline / local_fallback sessions had no comparisonType coverage at all, so the load-blocker slipped through. Fix: Add a context-aware COMPARISON-TYPE-001 ERROR-tier check in testCaseValidate that validates AssertValues against the 16-value subset and uiAttributeAssertion against the 6-value subset, sourced from a single shared comparisonTypeSets module; deep-walks nested steps. Adds unit tests, a smoke entry, and the docs/mcp.md error-code entry.
…type-validator PDX-502: add context-aware comparisonType enum validator
RCA: On Windows the sf launcher is a .cmd run via shell:true, where Node concatenates executable + args unquoted, so any space (auto-discovered C:\Program Files\sf install, an explicit sf_path, or a --properties-file value under a Provar Manager directory) split the command — breaking the default Windows install case. Fix: Pre-quote the executable and every argument with quoteWindowsToken on the win32 shell branch (auto-resolved paths included, not gated on sf_path); Node's join + cmd /s outer-quote strip preserves per-token quoting. Metacharacter rejection still applies to user-supplied paths. Adds win32 unit tests and a docs note.
PDX-504: quote sf executable and args for spaces on Windows
Contributor
There was a problem hiding this comment.
Pull request overview
This PR tightens Provar MCP validation and tooling by (1) adding a context-aware comparisonType enum validator that reflects Provar’s step-scoped subsets and (2) making Windows sf CLI spawning resilient to spaces in executable paths and argument values. It also updates documentation and rule text to align with the corrected “single enum, step-specific subsets” model, and adds internal loop-runner docs for batching related fixes.
Changes:
- Add
COMPARISON-TYPE-001(ERROR) validator for step-scopedcomparisonTypesubsets, plus unit tests and smoke coverage. - Improve Windows
sfinvocation with per-token quoting undershell:true, including auto-resolvedProgram Filesexecutables and spaced argument values. - Update docs/rules to document the correct per-step
comparisonTypesubsets and related semantics; add internal loop runner documentation.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| test/unit/mcp/testCaseValidate.test.ts | Adds unit tests covering context-aware comparisonType validation across AssertValues vs UI Assert. |
| test/unit/mcp/sfSpawn.test.ts | Adds Windows quoting tests to ensure spaced executables/args are not split under shell:true. |
| src/mcp/tools/testCaseValidate.ts | Implements recursive, context-aware comparisonType validation emitting COMPARISON-TYPE-001 at ERROR tier. |
| src/mcp/tools/sfSpawn.ts | Adds Windows token quoting logic when using shell:true to prevent splitting on spaces. |
| src/mcp/rules/provar_best_practices_rules.json | Updates rule text to describe the correct step-scoped comparisonType subsets and expected-value semantics. |
| src/mcp/rules/comparisonTypeSets.ts | Introduces canonical AssertValues/UI Assert comparisonType constants and sets as a shared source of truth. |
| scripts/mcp-smoke.cjs | Adds a smoke call that exercises the COMPARISON-TYPE-001 ERROR path. |
| docs/PROVAR_TEST_STEP_REFERENCE.md | Corrects/expands step reference docs for UI Assert vs AssertValues comparisonType subsets and notes. |
| docs/mcp.md | Documents step-scoped comparisonType subsets and adds COMPARISON-TYPE-001 to the validator rule list; notes Windows path-space handling. |
| docs/loops/pdx-501-505.md | Adds an internal loop prompt doc for driving a batch of PDX tickets (dogfooding fixes). |
| docs/loops/loop-template.md | Adds a reusable internal template/registry for loop prompts. |
| docs/loops/_example-instance.md | Adds an example internal loop instance scaffold. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| it('(c) does not split an argument value containing a space', () => { | ||
| setSfPathCacheForTesting('sf'); | ||
| const propsPath = 'C:\\Users\\mrdai\\git\\Provar Manager\\test-manager\\provardx-properties.json'; |
Collaborator
Author
| authored testcases: | ||
| • AssertValues / assertValuesComparison (apiId | ||
| com.provar.plugins.bundled.apis.AssertValues) — valid set from | ||
| C:\Users\mrdai\git\provar-manager-regression\tests\ComparisonTypes.testcase: |
Collaborator
Author
There was a problem hiding this comment.
Fixed in #204 — replaced with an OS- and user-agnostic path: ~/git/provar-manager-regression/tests/ComparisonTypes.testcase.
| LessThanOrEqualTo, IsPresent, IsEmpty, Matches, NotMatches, Contains, | ||
| NotContains, StartsWith, NotStartsWith, EndsWith, NotEndsWith (16) | ||
| • UiAssert / uiAttributeAssertion — valid set from the comparisonType | ||
| attributes in C:\Users\mrdai\git\provar-manager-regression\tests\SauceDemo Purchase Flow (Demo).testcase: |
Collaborator
Author
There was a problem hiding this comment.
Fixed in #204 — replaced with an OS- and user-agnostic path: ~/git/provar-manager-regression/tests/SauceDemo Purchase Flow (Demo).testcase.
…OT_FOUND RCA: When the @provartesting/provardx-cli sf plugin is not installed (no provar topic), automation tools failed opaquely with AUTOMATION_*_FAILED carrying "Command provar not found"; there was no dedicated code or probe so users could not tell the root cause was a missing plugin. Fix: Add ProvarPluginNotFoundError + isProvarPluginMissing detector + probeProvarTopic in sfSpawn, and a shared pre-flight in all five automation tools that returns PROVAR_PLUGIN_NOT_FOUND with a details.suggestion remediation (sf plugins install @provartesting/provardx-cli) instead of the generic failure. Adds unit tests and docs/mcp.md error-code entries.
…gin-not-found PDX-505: detect missing provar plugin with PROVAR_PLUGIN_NOT_FOUND
RCA: Develop sat at 1.5.2-beta.4 after the PDX-501..505 MCP fixes merged; 1.5.3 shipped from a release branch to main only and never back-merged, so develop needs to advance to the next version. Fix: Set version to 1.5.4 in package.json and both server.json fields (top-level "version" and packages[0].version) so the two stay identical per the repo version-sync convention.
PDX-0: chore: bump version to 1.5.4
… docs RCA: Copilot review on PR #201 flagged a hard-coded personal Windows username (mrdai) in an sfSpawn test path and two loop-doc example paths; personal/environment-specific paths are non-portable and embed identifying detail. Fix: Replace mrdai with the generic "username" in the win32 sfSpawn test (still a spaced Windows path, preserving the space-handling assertion) and switch the loop-doc example paths to OS- and user-agnostic ~/git/... form.
3 tasks
PDX-0: chore: portable, non-personal paths (PR #201 Copilot review)
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.