PDX-0: release v1.5.2 — Sessions 6/7 feedback (warnings, valueClass, org-describe)#190
Merged
Conversation
…-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>
…a inferSalesforceValueClass helper RCA: Provar runtime silently discards Salesforce date/datetime fields whose XML emits valueClass="string" — the failure surfaces only when a later validation rule reads the empty field. The existing buildArgumentValue dispatch in src/mcp/tools/testCaseGenerate.ts fell through to a hard-coded valueClass="string" fallback after handling variable / compound / uiTarget / uiLocator cases, so every literal date or boolean argument flowed out untyped. A prior helper (inferValueClass) introduced in 3c1545c that covered the boolean case alone was refactored away in 2d4240c, regressing literal-true/false handling too. Datetime, integer, and explicit org-describe-driven type hints were never covered. Fix: Introduce an exported inferSalesforceValueClass(key, val, fieldTypeHint?) helper that returns one of 'date' | 'datetime' | 'boolean' | 'integer' | 'string'. Detection order: explicit fieldTypeHint wins; then ISO-8601 datetime regex /^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}/; then ISO-8601 date regex /^\d{4}-\d{2}-\d{2}$/; then literal 'true'/'false'; then integer-only string /^-?\d+$/; else string. buildArgumentValue now calls this helper before the (now-removed) string fallback and emits <value class="value" valueClass="<inferred>">…</value>. The fieldTypeHint param is wired into the helper signature but not yet exposed on the MCP tool input schema — that exposure lands in PDX-492 H2b, which sits behind PDX-491 H2a (provar_org_describe). Until then callers omit the hint and detection falls through to the regex layer. Tool description and docs/mcp.md argument-conventions table updated to spell out the auto-detection rules. Tests: New PDX-493 — inferSalesforceValueClass helper block exercises the helper directly across all six branches (datetime with fractional seconds + zone, plain date, true, false, positive integer, negative integer, plain string), the edge case where a short numeric string like "12" must resolve to integer not date (date regex requires full ISO yyyy-mm-dd), strings that look like dates but miss the ISO format must stay string, and explicit fieldTypeHint wins over format detection in all three directions (string-shape + date hint, date-shape + string hint, integer-shape + boolean hint). A second PDX-493 — valueClass emission in generated XML block round-trips through provar_testcase_generate and asserts the inferred attribute reaches the emitted XML for date, datetime, boolean (true and false), positive integer, negative integer, and plain string. Validation: node_modules/.bin/nyc node_modules/.bin/mocha "test/**/*.test.ts" — 1172 passing / 0 failing; yarn compile clean; yarn lint clean (lint:script-names + lint). 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).
…run failures RCA: Consumers of provar_automation_testrun.steps[] and provar_testrun_rca.failures[] currently have no machine-friendly retry hint and must re-parse human-readable strings to decide whether a failure is transient or deterministic, so agents either retry locator failures forever or give up on a single ECONNRESET blip. Fix: Add optional error_category (INFRASTRUCTURE|ASSERTION|LOCATOR|TIMEOUT|OTHER) and retryable (boolean) fields to both FailureReport (rcaTools.ts) and JUnitStepResult (antTools.ts), populated by an additive pattern-match classifier; retryable=true only for INFRASTRUCTURE and TIMEOUT, undefined when no pattern matches — non-breaking — with tool descriptions and docs/mcp.md updated. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
RCA: provar_automation_testrun returned exitCode 0 with an empty steps[] array when a misspelled testCase/testCases key in provardx-properties.json caused the run to select nothing. Agents (and humans) had no signal to disambiguate "intentionally zero tests ran" from "typo silently dropped my entire selector", leading to wasted retry loops and downstream confusion when later tooling complained about missing results. Fix: Introduce a single JUnit introspection helper (introspectJUnit) that returns stepCount, parseWarning, and a resultsPathResolved flag. When sf exits 0 and the JUnit XML for the resolved results dir contains zero test cases, the response gains a warnings[] entry built via formatWarning(WARNING_CODES.RUN_001, ...) pointing the agent at the testCase/testCases spelling pitfall. The warning is additive — exitCode and isError remain unchanged. The helper is structured so future PDX-491 JUNIT-001 (expected-vs-actual mismatch) can read stepCount from the same struct without re-parsing. Updated the tool description, docs/mcp.md (RUN-001 output documentation), and added four targeted tests (positive, success-with-steps negative, non-zero-exit negative, unresolved-results-dir silent case).
…DATA-001) RCA: Provar's `<dataTable>` element only iterates row-by-row when the test case runs through a test-plan instance (`.testinstance`). When the project's provardx-properties.json references the test case directly via top-level `testCase` or `testCases`, the runtime silently ignores the data table and every variable reference resolves to null. The test run reports success against an empty row set — a silent-pass mode that AI agents and humans both miss until a downstream assertion fails for an unrelated reason. Prior to this change the validator emitted a generic DATA-001 advisory whenever a `<dataTable>` element was present, with no awareness of how the test would be wired at run time. That generic warning was also noisy in plan-mode projects where data-driven execution works correctly. Fix: Introduce `src/mcp/utils/testCasePlanMode.ts`, a resolver that consults the active `~/.sf/config.json` (`PROVARDX_PROPERTIES_FILE_PATH`) for the project's properties file, resolves `projectPath`, walks `plans/**/*.testinstance` for `testCasePath=` references to the file under validation, and returns `'direct' | 'plan' | 'unknown'`. The MCP handler in `registerTestCaseValidate` and the disk-backed `validateTestCaseXml` entry both call the resolver and pass the mode through `validateTestCase` via a new `ValidateTestCaseOptions` parameter. DATA-001 is now suppressed under plan mode, fires with the `formatWarning(WARNING_CODES.DATA_001, ...)` advisory under direct mode (recommending `provar_testplan_add-instance`), and falls back to the structural advisory under unknown mode (pure function called without file resolution — keeps existing callers' behaviour). The new helper, the new mode-aware emission helper (`maybeEmitDataTableWarning`), and the handler-level integration are covered by 12 new tests in `test/unit/mcp/testCasePlanMode.test.ts` and the PDX-489 block of `test/unit/mcp/testCaseValidate.test.ts`. Documentation: the tool description for `provar_testcase_validate` references the new behaviour, a new "Data-driven execution" subsection in `docs/mcp.md` explains the direct-vs-plan modes and the recommended workaround, the warning-codes table cross-links to it, and the DATA-001 entry in the validator section spells out the suppression rule. Thread E (testCaseValidate.ts) of the Sessions 6–7 plan; does not touch automationTools.ts or testCaseGenerate.ts per the per-thread file-ownership map (the `provar_automation_testrun` description-text reference noted in the original brief is deferred to Thread B which owns that file).
…DES test RCA: Copilot review on PR #182 flagged that the parity test only iterated the expected map and asserted forward-direction equality (expected key -> WARNING_CODES[key]). A new enum entry added without updating the test, or an accidentally-removed entry, would not fail the build. Bidirectional coverage was missing. Fix: Hoist the expected map to the describe scope so it is reusable. Add two new assertions: (a) Object.keys(WARNING_CODES).sort() deepEqual Object.keys(expected).sort() — guards against additions and omissions in the key set; (b) Object.values(WARNING_CODES).sort() deepEqual Object.values(expected).sort() — guards against value drift. Tests now fail loudly on silent enum drift in either direction.
…etadata cache) Introduces provar_org_describe, a read-only MCP tool that surfaces cached Salesforce describe data from the Provar IDE workspace .metadata directory so authoring tools (provar_testcase_generate) can produce field-correct data steps without a live SF API call. Workspace discovery walks three candidates in order: 1. <parent>/workspace-<basename>/ (sibling pattern) 2. <parent>/Provar_Workspaces/workspace-<name-dashes>/ 3. ~/Provar/workspace-<name-dashes>/ (user-home fallback) Returns a structured cache-miss response with details.suggestion when the connection cache is absent, so the agent can either prompt the user to load the connection in Provar IDE or fall back to inline field hints. Registered under the existing 'inspect' tool group. H2b (sibling thread) consumes this tool's output to populate generator hints. RCA: provar_testcase_generate had no source of truth for which fields on a Salesforce object are required and what their types are. Agents either guessed (producing brittle tests), hard-coded names, or called the live SF API (slow + auth-dependent). The Provar IDE already caches describe data on disk after a connection is loaded — this PR exposes that cache as a read-only MCP tool so the cache becomes useful outside the IDE. Fix: New tool src/mcp/tools/orgDescribeTools.ts with strict path-policy checks on both project_path and connection_name (separator/traversal rejected). Cache schema is one file per object (.json preferred, .xml / .object accepted) so the existing IDE writer needs no change. Cache miss returns a stable shape with suggestion rather than an isError response, so callers do not need a try/catch path. 14 unit tests cover all 7 plan scenarios (workspace discovery, fallback, cache miss, path policy, happy path, field_filter, objects filter). Two smoke entries cover happy + miss. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…o formatWarning() messages RCA: The "Warning codes" section in docs/mcp.md previously stated that "Warnings emitted programmatically follow the shape WARNING [<CODE>]: <message>". That claim is too broad: provar_properties_validate emits placeholder warnings (pre-existing, non-coded) whose message is a plain string with no WARNING [...] prefix. A reader looking at that line would reasonably expect every warning surface in the codebase to carry a code, then be surprised by the properties validator output and treat it as a regression. Fix: Reword the sentence to scope the structured shape to warnings built via formatWarning() — those have the WARNING [<CODE>]: <message> prefix and the optional Did you mean '<suggestion>'? suffix. Free-form warnings without a structured code (called out explicitly via the properties validator example) remain plain strings. Keeps the reference to src/mcp/utils/warningCodes.ts for the canonical enum. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ix root_cause_category count RCA: Copilot review on PR #186 flagged two doc/description inconsistencies — (1) the provar_testrun_rca tool description implied error_category/retryable appear in mode="failures" output, but mode="failures" only emits { testItemId, title, errorMessage } and the new fields live on FailureReport entries in mode="rca" only; (2) docs/mcp.md FailureReport table said root_cause_category is "One of 12 categories" while the enumerated list contains 17 buckets (and the new paragraph added in this PR explicitly says "17 buckets"). These are doc/description mismatches, not runtime behaviour bugs. Fix: Reword the rcaTools.ts tool description so error_category/retryable are explicitly scoped to failures[] entries in mode="rca", with a "NOT included in mode=failures output" clarifier so agents reading the description cold pick the right mode. Update docs/mcp.md root_cause_category row from "One of 12 categories (see table below)" to "One of 17 categories (see list below)" — count now matches the enumerated list (DRIVER_VERSION_MISMATCH, LOCATOR_STALE, TIMEOUT, ASSERTION_FAILED, CREDENTIAL_FAILURE, MISSING_CALLABLE, METADATA_CACHE, PAGE_OBJECT_COMPILE, CONNECTION_REFUSED, DATA_SETUP, LICENSE_INVALID, SALESFORCE_VALIDATION, SALESFORCE_PICKLIST, SALESFORCE_REFERENCE, SALESFORCE_ACCESS, SALESFORCE_TRIGGER, UNKNOWN = 17). Also corrected "see table below" → "see list below" since the categories are an inline comma-separated list, not a table. Verified with yarn compile (clean), yarn lint (clean), and full mocha suite (1169 passing). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… (integer → decimal) + anchor datetime regex
RCA: The H3 plan AC specified `valueClass="integer"` for integer-only strings, but
`docs/PROVAR_TEST_STEP_REFERENCE.md` lines 1338 and 1428 are explicit: numbers in
Provar test steps use `valueClass="decimal"` (covering both `42` and `3.14`). No
`integer` valueClass exists in the canonical reference grammar, so the initial
H3 implementation would have emitted XML that diverges from the Provar runtime
contract for every integer field — silently produced bad XML for any numeric
Salesforce field. Separately, the datetime detection regex
`/^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}/` was not end-anchored, so trailing
garbage after seconds (e.g. `2026-05-19T10:30:00not-a-zone`) was accepted as a
valid datetime and emitted as `valueClass="datetime"`. The canonical reference
permits optional fractional seconds and timezone — the regex must enforce that
shape explicitly.
Fix: In `inferSalesforceValueClass` (src/mcp/tools/testCaseGenerate.ts):
- Replace the `'integer'` arm of the return-type union with `'decimal'`. Both
the parameter and return type now use the canonical-reference set:
`'date' | 'datetime' | 'boolean' | 'decimal' | 'string'`.
- Broaden the numeric regex from `/^-?\d+$/` to `/^-?\d+(\.\d+)?$/` so it
matches integers and decimals; return `'decimal'` for both.
- Anchor the datetime regex end with explicit optional fractional-seconds and
timezone groups: `/^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}(\.\d+)?(Z|[+-]\d{2}:?\d{2})?$/`.
- Update doc-comment to spell out the canonical reference (lines 1338/1428).
Update tool description text (line ~155) to say "numeric string → decimal" and
explicitly call out that there is no separate `integer` valueClass. Update
`docs/mcp.md` argument-conventions table row to match (one merged numeric row
covering `42`, `-5`, `3.14`). Tests:
- Rewrite `integer` assertions in test/unit/mcp/testCaseGenerate.test.ts to
`decimal` (positive int, negative int, short numeric string).
- Add positive coverage for decimals (`3.14`, `-12.5`) at both helper and
XML-emission levels.
- Add datetime end-anchor coverage (numeric timezone offset accepted; trailing
garbage rejected as `string`).
Addresses Copilot review feedback on PR #183.
…erride + repair docs TOC RCA: Copilot review on PR #187 flagged a path-policy bypass in resolveTestCasePlanMode: the propertiesFilePathOverride parameter was returned directly from readPropertiesFilePath without going through assertPathAllowed, so a future caller could read a properties file outside the configured allowedPaths tree. A separate Copilot finding caught a broken docs TOC — adding the Data-driven execution top-level bullet left the NitroX, Quality Hub, and Org metadata bullets indented as if nested, misrepresenting the section hierarchy. Fix: Funnel both the override and the value read from ~/.sf/config.json through a shared enforceAllowed helper that resolves the path, calls assertPathAllowed, canonicalises symlinks via realpathSync when the file exists, and collapses any policy violation or missing file to null to preserve the resolver's best-effort silent-fallback contract. Re-leveled the docs/mcp.md TOC so NitroX and Quality Hub sit at the top level (matching their ## headings) with Org metadata nested under Quality Hub (matching its ### heading). Added two regression tests: override inside allowed paths is consulted normally; override outside allowed paths collapses to mode 'unknown' without throwing and without exposing the rejected path.
…s when JUnit data missing RCA: The RUN-001 zero-tests-executed warning fired whenever `resultsPathResolved === true && stepCount === 0`. That condition is also true when the results dir was located but contained no XML files, or when every XML file failed to parse — in those cases the tool has no data on which to claim "zero tests ran" yet would still emit RUN-001, misdirecting the agent toward a properties-file typo when the real problem was an unreadable/unwritten results dir. Fix: Extended parseJUnitResults to return parsedAny: boolean (true iff at least one JUnit XML file was located AND parsed without throwing). Tightened the RUN-001 gate in introspectJUnit/registerAutomationTestRun to require `resultsPathResolved && parsedAny && stepCount === 0`. The "no XML found" and "all XML unparseable" branches now stay silent on warnings[] and surface only through details.warning, as the docs already described. Aligned the in-tree tool description and docs/mcp.md to spell out the parsedAny requirement, and added/strengthened tests covering the missing-XML, malformed-XML, and empty-<testsuite> (legit RUN-001) cases. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…lag bug, exists-true on parse error, docs/tests RCA: Copilot review of PR #188 flagged six issues across correctness, security, and contract: (1) the .xml/.object fallback compared required==='true' as a string, but fast-xml-parser with parseTagValue=true (its default) coerces the value to boolean true, silently misclassifying required fields as nillable; (2) discoverWorkspace probed fs.existsSync/statSync against candidate dirs (including the ~/Provar home fallback) BEFORE any path-policy check, contradicting the project's --allowed-paths contract and potentially touching paths outside the policy; (3) when a cache file existed but failed to parse, readObject returned exists=false — indistinguishable from "object not cached", so callers could not detect corrupt/unsupported cache files; (4) docs/examples omitted the requestId field that the tool actually returns, making the documented shape drift from runtime; (5) unit tests covered only the .json cache path, leaving the legacy .xml and .object parsers (where the required-flag bug lived) untested; (6) the PATH_TRAVERSAL message read "contains path separators" but the validator also rejects bare ".." with no separators, so the message was inaccurate for that branch. Fix: (1) readXmlCacheFile now treats both boolean true and string "true" as required, so nillable is computed correctly regardless of parser config; (2) discoverWorkspace accepts allowedPaths and runs assertPathAllowed per candidate BEFORE fs.existsSync/statSync — a candidate outside policy is silently skipped (not a hard error) so discovery falls through to the next candidate naturally; (3) readObject parse failures now return exists=true with field_count=0 and a per-object error_message describing the parse failure, letting callers distinguish corrupt from missing; (4) docs/mcp.md adds requestId to the output table, adds it to both example responses, documents the new error_message field shape, and adds a third example showing the parse-error response; (5) added (h.1) .xml format test (regression guard for the required-flag bug), (h.2) .object format test, (i) parse-error test asserting exists=true + error_message, and (j) bare ".." connection_name test asserting the broadened message; (6) the PATH_TRAVERSAL message now reads "must not contain path separators or directory-traversal segments ('..')", covering both rejection conditions. 19/19 orgDescribe tests pass, full mocha 1174/1174, yarn lint clean, yarn compile clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…link canonicalisation RCA: macOS CI surfaced a test failure in resolveTestCasePlanMode's "direct" assertion. On Mac, os.tmpdir() returns /var/folders/... but /var is a symlink to /private/var. The Copilot-fix commit (e46b7a9) added fs.realpathSync to enforceAllowed for symlink canonicalisation — correct security behaviour — but the test compared result.propertiesFilePath against a path built from un-realpath'd os.tmpdir(), so the resolved /private/var/... did not match the expected /var/... and the assertion blew up on Mac CI (passed on Windows because /var is a regular dir there). Fix: Wrap the mkdtempSync return in fs.realpathSync inside beforeEach so the tmp root is already canonicalised. Every derived path (projectPath, testCasePath, propertiesPath) now uses the canonical /private/var/... form on Mac, matching what the resolver returns. No code change to the helper — its realpathSync behaviour is correct and stays. Windows behaviour unchanged because realpathSync is a no-op on paths without symlinks. Full mocha 1176 passing, yarn lint clean.
…s-enum PDX-485: chore(mcp) — shared warningCodes.ts enum
…a-strict-validator PDX-486: feat(mcp) — strict validator unknown-key detection (Thread A, PR 1/3)
…gory-retryable PDX-490: feat(mcp) — error_category + retryable on test-run failures (Thread D)
…direct-mode-warning PDX-489: feat(mcp) — DATA-001 warning for dataTable in direct testCase-mode (Thread E)
…b-zero-tests-guard PDX-486: feat(mcp) — testrun zero-tests guard (Thread B, PR 1/3)
…ime-valueclass PDX-493: feat(mcp) — date/datetime/boolean/integer valueClass dispatch (Thread C, PR 1/2)
…be-tool PDX-492: feat(mcp) — provar_org_describe tool, H2a (Thread F)
RCA: Develop accumulated 7 PRs from Sessions 6/7 backlog (PDX-485 thread-prep, PDX-486 VALIDATE-TYPO A+B, PDX-489 G3 DATA-001, PDX-490 G5 error_category, PDX-492 H2a provar_org_describe, PDX-493 H3 valueClass decimal alignment) since 1.5.1 was tagged. Per CLAUDE.md the develop branch tracks <major>.<minor>.<patch>-beta.<n>, so the next publishable version is 1.5.2-beta.1. Without this bump a publish from develop would clash with 1.5.1 on the registry. Fix: Bump package.json.version, server.json.version (top-level), and server.json.packages[0].version from 1.5.1 to 1.5.2-beta.1, keeping all three identical per the version-sync rule in CLAUDE.md. No source or test changes. yarn compile clean, yarn lint clean, full mocha 1245 passing / 0 failing.
…eta.1 PDX-0: chore(release) — bump version to 1.5.2-beta.1
RCA: Develop currently sits at 1.5.2-beta.1 after PR #189. The 1.5.2 release rolls up 7 PRs from the Sessions 6/7 backlog (PDX-485 warningCodes shared enum, PDX-486 VALIDATE-TYPO A+B, PDX-489 G3 DATA-001 plus path-safety hardening, PDX-490 G5 error_category + retryable, PDX-492 H2a provar_org_describe tool, PDX-493 H3 date/datetime/decimal valueClass). Per CLAUDE.md, the release branch drops the -beta.N suffix before the develop -> main merge so the release commit publishes 1.5.2 cleanly without a prerelease tag chain. Fix: Bump package.json.version, server.json.version, and server.json.packages[0].version from 1.5.2-beta.1 to 1.5.2 in lock-step per the version-sync rule. No source changes; release surface is exactly what landed on develop.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR prepares the v1.5.2 release of provardx-cli, rolling up the Sessions 6/7 MCP feedback batch: standardized warning codes, stricter properties validation warnings, improved testrun/JUnit introspection (including a zero-tests warning), richer failure classification, org-describe metadata access from local workspace cache, and valueClass inference improvements—plus docs/tests/smoke updates and the final version bump.
Changes:
- Introduces shared
WARNING_CODES+formatWarning()and wires new warning surfaces (SCHEMA-001, RUN-001, DATA-001) across MCP tools with expanded unit coverage. - Adds
provar_org_describeplus supporting workspace discovery / cache parsing, and enhances JUnit parsing outputs (parsedAny, step classification fields). - Updates documentation + smoke harness, and bumps
package.json+server.jsonto1.5.2.
Reviewed changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/unit/mcp/utils/warningCodes.test.ts | Adds unit coverage for warning code constants and formatWarning() behavior. |
| test/unit/mcp/testCaseValidate.test.ts | Extends DATA-001 tests, including handler-level plan-mode integration coverage. |
| test/unit/mcp/testCasePlanMode.test.ts | Adds tests for resolving direct vs plan vs unknown mode from properties + plans. |
| test/unit/mcp/testCaseGenerate.test.ts | Adds tests for inferSalesforceValueClass and emitted valueClass in generated XML. |
| test/unit/mcp/rcaTools.test.ts | Adds tests for error_category + retryable classification in RCA failures. |
| test/unit/mcp/propertiesTools.test.ts | Adds SCHEMA-001 unknown-key detection tests (incl. “did you mean” suggestion). |
| test/unit/mcp/orgDescribeTools.test.ts | Adds comprehensive tests for new provar_org_describe tool behavior and edge cases. |
| test/unit/mcp/automationTools.test.ts | Adds RUN-001 tests and clarifies behavior when JUnit data is absent/unparseable. |
| test/unit/mcp/antTools.test.ts | Extends JUnit parsing tests for parsedAny and step-level classification fields. |
| test/fixtures/properties/testcases-typo.json | Adds fixture for testCases typo regression coverage. |
| src/mcp/utils/warningCodes.ts | Introduces canonical warning codes + standard formatter. |
| src/mcp/utils/testCasePlanMode.ts | Adds resolver for determining direct vs plan execution mode for a testcase. |
| src/mcp/tools/testCaseValidate.ts | Adds plan-mode-aware DATA-001 emission (suppressed under plan mode). |
| src/mcp/tools/testCaseGenerate.ts | Adds inferSalesforceValueClass() and emits typed valueClass instead of always string. |
| src/mcp/tools/rcaTools.ts | Adds failure error_category + retryable fields and classifier helpers. |
| src/mcp/tools/propertiesTools.ts | Adds SCHEMA-001 unknown-key warnings with Levenshtein suggestions and exports canonical key sets. |
| src/mcp/tools/orgDescribeTools.ts | Adds new provar_org_describe tool reading Provar workspace .metadata cache. |
| src/mcp/tools/automationTools.ts | Adds JUnit introspection, RUN-001 warning logic, and enriches output with step metadata. |
| src/mcp/tools/antTools.ts | Adds parsedAny signal and step error classification (error_category, retryable). |
| src/mcp/server.ts | Registers org-describe tools under the inspect group. |
| server.json | Bumps MCP server version metadata to 1.5.2. |
| scripts/mcp-smoke.cjs | Adds smoke calls for provar_org_describe (cache miss + happy path). |
| package.json | Bumps package version to 1.5.2. |
| docs/mcp.md | Documents warning codes, new org-describe tool, data-driven execution semantics, and new fields/warnings. |
| docs/mcp-pilot-guide.md | Adds a new scenario documenting org-aware generation via provar_org_describe. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+104
to
+125
| * Returns the first candidate workspace that exists AND is within allowedPaths, or null. | ||
| * | ||
| * Path policy is enforced PER CANDIDATE before any filesystem call: a candidate that | ||
| * sits outside `--allowed-paths` is silently skipped (it is not an error — discovery | ||
| * just moves on to the next). This means we never call fs.existsSync / fs.statSync | ||
| * against directories that the operator has explicitly placed off-limits, including | ||
| * the user-home fallback (~/Provar/...) when home sits outside the policy. | ||
| * | ||
| * When allowedPaths is empty (unrestricted mode), assertPathAllowed is a no-op and | ||
| * all candidates are probed exactly as before. | ||
| */ | ||
| export function discoverWorkspace(projectPath: string, allowedPaths: string[] = []): string | null { | ||
| for (const candidate of workspaceCandidates(projectPath)) { | ||
| try { | ||
| assertPathAllowed(candidate, allowedPaths); | ||
| } catch { | ||
| // Candidate outside policy — skip without touching the filesystem. | ||
| continue; | ||
| } | ||
| try { | ||
| if (fs.existsSync(candidate) && fs.statSync(candidate).isDirectory()) { | ||
| return candidate; |
Comment on lines
1418
to
+1422
| ```json | ||
| "steps": [ | ||
| { "testItemId": "1", "title": "TC-Login-001-LoginAndVerify.testcase", "status": "pass" }, | ||
| { "testItemId": "2", "title": "TC-Login-002-ForgotPassword.testcase", "status": "fail", "errorMessage": "Execution failed: Element not found" } | ||
| { "testItemId": "2", "title": "TC-Login-002-ForgotPassword.testcase", "status": "fail", "errorMessage": "TimeoutException: page did not load", | ||
| "error_category": "TIMEOUT", "retryable": true } |
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
Release
v1.5.2. Rolls up the Sessions 6/7 feedback batch (PDX-485, PDX-486, PDX-489, PDX-490, PDX-492, PDX-493) plus the version bump. Drops the-beta.1suffix from develop's1.5.2-beta.1.What's in 1.5.2
warningCodes.tsenum +formatWarning()helper across all MCP toolstestCasestypo class) with Levenshtein "did you mean" suggestionsfexits 0 but JUnit shows zero executed testserror_category(INFRASTRUCTURE / ASSERTION / LOCATOR / TIMEOUT / OTHER) +retryablefields on test-run failures<dataTable>runs under directtestCasemode + path-safety hardening onpropertiesFilePathOverridevalueClass="date"/"datetime"/"boolean"/"decimal"dispatch viainferSalesforceValueClass(aligned withPROVAR_TEST_STEP_REFERENCE.md)provar_org_describetool — reads Provar workspace.metadatacache for required-field inspection without an org connection1.5.2-beta.1on develop1.5.2for releaseTest surface grew from
~1163to1245passing (+82 new tests).Release notes (draft for GitHub Release body)
ProvarDX v1.5.2 Release
Highlights
testCasesvstestCasebefore the run happens, and the testrun surfaces aRUN-001warning whensfreports success but zero tests actually ran.error_category+retryableso AI agents and CI can distinguish a flakyConnection resetfrom a real assertion failure without regexing the message.provar_org_describetool reads Provar's workspace.metadatacache to surface required fields, types, and existence — no live SF connection needed at generation time.valueClassfor dates and numbers — date/datetime values now emitvalueClass="date"/"datetime", and numbers usevalueClass="decimal"perPROVAR_TEST_STEP_REFERENCE.md. Stops the silent-discard failure mode where dates serialised as strings vanished without error.New
provar_org_describeMCP tool: reads<workspace>/.metadata/<connection>/*(JSON, XML, and legacy.objectformats) and returns object/field describe data. Gracefully handles cache-miss withdetails.suggestion. All paths run throughassertPathAllowed.provar_properties_validate) detects unknown keys at top-level,metadata.*, andenvironment.*with Levenshtein-based "did you mean" suggestions. Emits warnings (not errors) so future Provar additions don't break older MCP clients.provar_automation_testrun— gated onparsedAny && stepCount === 0so it only fires when the tool genuinely has JUnit data showing zero tests ran (not when JUnit data is missing/unparseable).<dataTable>is referenced via the top-leveltestCase/testCasesarray rather than a.testinstance(data-driven iteration silently fails in direct mode).error_category+retryableoptional fields onJUnitStepResultandFailureReport. Categories:INFRASTRUCTURE,ASSERTION,LOCATOR,TIMEOUT,OTHER. Retryable only for INFRASTRUCTURE and TIMEOUT.Fixed
valueClass="integer"was emitted for numeric values; canonical reference isvalueClass="decimal". All numeric strings (integer and decimal forms) now emit asdecimal.2026-05-19T10:30:00not-a-zonewas misclassified asdatetime. Now end-anchored with optional fractional seconds + timezone.valueClass="string"and discarded by Provar's runtime, surfacing only when a downstream validation rule failed. Now correctly typed.testCasePlanModeresolver bypassedassertPathAllowedon the properties-file override path (security regression). Now enforces policy + canonicalises viafs.realpathSyncto prevent symlink escape from the allowed-paths boundary..objectcache files compared against the string"true", butfast-xml-parserreturns booleantrueby default — required fields were misclassified as nillable. Now handles both forms.provar_properties_validateemitted "Warnings emitted programmatically follow the shapeWARNING [<CODE>]: <message>" too broadly — the validator also emits placeholder warnings without a code. Wording now scoped toformatWarning()output only.provar_testrun_rcatool description impliedmode="failures"returnserror_category/retryable, but those fields only appear onfailures[]inmode="rca". Description corrected.Internal / infrastructure
src/mcp/utils/warningCodes.tsshared enum +formatWarning()helper. All warning emission sites now reference the same canonical codes (SCHEMA-001,RUN-001,DATA-001,PROVARHOME-001,PARALLEL-001,JUNIT-001).discoverWorkspaceinprovar_org_describepolicy-checks every candidate dir before anyfscall, so the~/Provar/...home fallback never touches the filesystem when home sits outside--allowed-paths.Upgrade notes
warnings[]array). Existing callers see no behavior change unless they consume the new fields.valueClass="decimal"for numeric arguments is a generator-side change — newly generated test cases will produce the canonical XML; existing test cases on disk are unaffected.The
Publish to NPM and MCP Registryworkflow on v1.5.1 succeeded all the way through to the Slack notification step, but nothing posted to the Slack channel and release notes had to be copied manually. The Slack action ran (per the run log) and the payload was constructed correctly, but the message never appeared.Most likely root cause: Slack Block Kit
section.text.mrkdwnfield has a 3000-character limit. The v1.5.1 release body was ~1800 chars of markdown but expanded with\nliterals and a wide table that may have pushed the rendered text beyond the limit. Slack's webhook receiver rejects oversized blocks silently and returns 200 OK to the action, so theslack-github-action@v2step succeeded without raising an alarm.Secondary suspects:
mrkdwndoesn't render GitHub markdown tables — the v1.5.1 notes contained a table that may have triggered parser rejection of the whole block.errors: falseby default in this workflow, so any Slack-side rejection is swallowed.Recommended fixes (not in this PR — file as a follow-up):
NOTESto ~2900 chars in thejqcommand before constructing thebodyarg. Append a "see full notes on GitHub" link when truncated. Quick one-line fix in.github/workflows/DeployManual.ymlaround line 137.NOTESbefore passing to Slack (or convert to a bulleted list) — Block Kit'smrkdwndoesn't support them.errors: trueon theslackapi/slack-github-action@v2.0.0step so failed Slack posts surface as a CI failure instead of silent success.sectionblocks (each capped at 3000 chars).The v1.5.2 release will hit the same issue without intervention — recommend bundling the truncation fix into a follow-up PR before the next release that publishes notable changes.
Verification
package.jsonversion →1.5.2server.jsontop-levelversion→1.5.2server.jsonpackages[0].version→1.5.2yarn compileclean (verified on develop pre-merge)yarn lintclean (verified on develop pre-merge)chore/version-bump-1.5.2-beta.1pre-merge)yarn build && yarn test) passed on this branchPost-merge checklist
v1.5.2(paste the "Release notes" section above as the body)Publish to NPM and MCP Registryworkflow runs onrelease: publishedand publishes1.5.2with thelatesttag (NPM_TOKEN rotation may be needed — see PDX-0: chore(release) — bump version to 1.5.2-beta.1 #189 thread)1.5.2main → developto bring the1.5.2version onto develop, then bump develop to1.5.3-beta.1in a chore PR for the next iteration