From c046e341c17520f73fea1e853dd421d8d930ca73 Mon Sep 17 00:00:00 2001 From: Michael Dailey Date: Fri, 1 May 2026 15:59:45 -0500 Subject: [PATCH 1/2] PDX-0: feat(ci): add Slack deploy notification to DeployManual workflow Posts version, tag, deployer, and auto-extracted change notes (feat/fix commits since previous tag) on successful publish. Falls back to GitHub Release body when available. Payload built via jq for safe escaping. Co-Authored-By: Claude Sonnet 4.6 --- .github/workflows/DeployManual.yml | 80 ++++++++++++++++++++++++++++++ 1 file changed, 80 insertions(+) diff --git a/.github/workflows/DeployManual.yml b/.github/workflows/DeployManual.yml index 4fd9c34..5f6894e 100644 --- a/.github/workflows/DeployManual.yml +++ b/.github/workflows/DeployManual.yml @@ -15,6 +15,8 @@ jobs: steps: - name: Checkout uses: actions/checkout@v4 + with: + fetch-depth: 0 - name: Setup Node uses: actions/setup-node@v4 with: @@ -47,3 +49,81 @@ jobs: run: ./mcp-publisher login github-oidc - name: Publish to MCP Registry run: ./mcp-publisher publish + + - name: Build Slack notification payload + if: success() + env: + RELEASE_BODY: ${{ github.event.release.body }} + run: | + VERSION=$(node -p "require('./package.json').version") + TAG="${{ github.event.inputs.tag || 'latest' }}" + + # --- Determine change notes source --- + if [ -n "$RELEASE_BODY" ]; then + # GitHub Release body provided — use it verbatim + NOTES="$RELEASE_BODY" + else + # Auto-extract from git log since the previous tag + if [ "${{ github.event_name }}" = "release" ]; then + # For a release event HEAD is already the new tag; find the one before it + PREV=$(git tag --sort=-creatordate | awk "/^${GITHUB_REF_NAME}$/{found=1;next} found{print;exit}") + else + # For a manual dispatch use the most recent existing tag + PREV=$(git tag --sort=-creatordate | head -1) + fi + + RANGE="${PREV:+${PREV}..}HEAD" + + RAW=$(git log --pretty=format:"%s" $RANGE \ + | sed 's/^[A-Z][A-Z0-9]*-[0-9]*: //' \ + | grep -Ev "^(Merge |chore)" \ + | head -20) + + FEATS=$(printf '%s\n' "$RAW" | grep '^feat' | sed 's/^feat[^:]*: /• /' | head -8) + FIXES=$(printf '%s\n' "$RAW" | grep '^fix' | sed 's/^fix[^:]*: /• /' | head -8) + + NOTES="" + [ -n "$FEATS" ] && NOTES="*What's new:*"$'\n'"$FEATS" + if [ -n "$FIXES" ]; then + [ -n "$NOTES" ] && NOTES="${NOTES}"$'\n' + NOTES="${NOTES}*Fixes:*"$'\n'"$FIXES" + fi + [ -z "$NOTES" ] && NOTES="_No notable changes extracted._" + fi + + # --- Build Block Kit JSON payload via jq (handles escaping correctly) --- + NPM_URL="https://www.npmjs.com/package/@provartesting/provardx-cli/v/${VERSION}" + RUN_URL="${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}" + + jq -n \ + --arg ver "$VERSION" \ + --arg tag "$TAG" \ + --arg who "${{ github.actor }}" \ + --arg body "$NOTES" \ + --arg npm "$NPM_URL" \ + --arg run "$RUN_URL" \ + '{blocks:[ + {type:"header", + text:{type:"plain_text", text:("🚀 Provar MCP v"+$ver+" published"), emoji:true}}, + {type:"section", + fields:[ + {type:"mrkdwn", text:("*Tag:* `"+$tag+"`")}, + {type:"mrkdwn", text:("*By:* "+$who)} + ]}, + {type:"section", + text:{type:"mrkdwn", text:$body}}, + {type:"divider"}, + {type:"actions", + elements:[ + {type:"button", text:{type:"plain_text", text:"📦 npm", emoji:true}, url:$npm}, + {type:"button", text:{type:"plain_text", text:"🔗 GitHub Run", emoji:true}, url:$run} + ]} + ]}' > /tmp/slack_payload.json + + - name: Notify Slack + if: success() + uses: slackapi/slack-github-action@v2.0.0 + with: + webhook: ${{ secrets.SLACK_WEBHOOK_URL }} + webhook-type: incoming-webhook + payload-file-path: /tmp/slack_payload.json From 226885c241b1077d13d991eeb39c318cc1cfd862 Mon Sep 17 00:00:00 2001 From: Michael Dailey Date: Fri, 1 May 2026 16:07:00 -0500 Subject: [PATCH 2/2] PDX-0: fix: address PR #137 Copilot review comments - 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 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 --- src/mcp/tools/testCaseValidate.ts | 62 +++++++++++++---------- src/mcp/tools/testPlanTools.ts | 4 +- test/unit/mcp/testCaseValidate.test.ts | 69 ++++++++++++++++++++++++++ test/unit/mcp/testPlanTools.test.ts | 28 +++++++++++ 4 files changed, 134 insertions(+), 29 deletions(-) diff --git a/src/mcp/tools/testCaseValidate.ts b/src/mcp/tools/testCaseValidate.ts index 8c36431..1697faa 100644 --- a/src/mcp/tools/testCaseValidate.ts +++ b/src/mcp/tools/testCaseValidate.ts @@ -406,13 +406,15 @@ function validateApiCall(call: Record, issues: ValidationIssue[ function checkUiTarget(call: Record, apiId: string, stepName: string, issues: ValidationIssue[]): void { const targetArg = getArgList(call).find((a) => (a['@_id'] as string | undefined) === 'target'); if (!targetArg) return; - const valClass = (targetArg['value'] as Record | undefined)?.['@_class'] as string | undefined; - if (valClass && valClass !== 'uiTarget') { + const valueNode = targetArg['value'] as Record | undefined; + if (!valueNode) return; + const valClass = valueNode['@_class'] as string | undefined; + if (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".`, + message: `${apiLabel} step "${stepName}" target argument uses class="${valClass ?? '(missing)'}" — must be class="uiTarget".`, applies_to: 'apiCall', suggestion: 'Emit the target as: or uri="ui:pageobject:target?pageId=...". ' + @@ -441,17 +443,20 @@ function validateApiCallArgs( 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 | 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: . ' + - 'In provar.testcase.generate the "locator" attribute is converted automatically.', - }); + const locatorNode = locatorArg['value'] as Record | undefined; + if (locatorNode) { + const valClass = locatorNode['@_class'] as string | undefined; + if (valClass !== 'uiLocator') { + issues.push({ + rule_id: 'UI-LOCATOR-001', + severity: 'ERROR', + message: `"${stepName}" locator argument uses class="${valClass ?? '(missing)'}" — must be class="uiLocator".`, + applies_to: 'apiCall', + suggestion: + 'Emit the locator as: . ' + + 'In provar.testcase.generate the "locator" attribute is converted automatically.', + }); + } } } } @@ -462,19 +467,22 @@ function validateApiCallArgs( 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 | undefined)?.['@_class'] as string | undefined; - if (valClass && valClass !== 'valueList') { - issues.push({ - rule_id: 'SETVALUES-STRUCTURE-001', - severity: 'ERROR', - message: `SetValues step "${stepName}" values argument uses class="${valClass}" — must use class="valueList" with children.`, - applies_to: 'apiCall', - suggestion: - 'Wrap variable assignments in: ' + - 'value' + - '. In provar.testcase.generate pass each variable as a flat key/value pair ' + - 'in attributes — the generator builds the valueList structure automatically.', - }); + const valuesNode = valuesArg['value'] as Record | undefined; + if (valuesNode) { + const valClass = valuesNode['@_class'] as string | undefined; + if (valClass !== 'valueList') { + issues.push({ + rule_id: 'SETVALUES-STRUCTURE-001', + severity: 'ERROR', + message: `SetValues step "${stepName}" values argument uses class="${valClass ?? '(missing)'}" — must use class="valueList" with children.`, + applies_to: 'apiCall', + suggestion: + 'Wrap variable assignments in: ' + + 'value' + + '. In provar.testcase.generate pass each variable as a flat key/value pair ' + + 'in attributes — the generator builds the valueList structure automatically.', + }); + } } } } diff --git a/src/mcp/tools/testPlanTools.ts b/src/mcp/tools/testPlanTools.ts index 716c402..d165c4c 100644 --- a/src/mcp/tools/testPlanTools.ts +++ b/src/mcp/tools/testPlanTools.ts @@ -105,13 +105,13 @@ export function registerTestPlanCreate(server: McpServer, config: ServerConfig): }; } - if (plan_name.includes('/') || plan_name.includes('\\') || path.isAbsolute(plan_name)) { + if (!/^[A-Za-z0-9][\w\- ]*$/.test(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)), + text: JSON.stringify(makeError('INVALID_PLAN_NAME', `plan_name must start with a letter or digit and contain only letters, digits, underscores, hyphens, or spaces: "${plan_name}"`, requestId)), }, ], }; diff --git a/test/unit/mcp/testCaseValidate.test.ts b/test/unit/mcp/testCaseValidate.test.ts index 456fc09..205f105 100644 --- a/test/unit/mcp/testCaseValidate.test.ts +++ b/test/unit/mcp/testCaseValidate.test.ts @@ -359,6 +359,29 @@ describe('validateTestCase', () => { ); }); + it('fires when UiWithScreen target has no class attribute', () => { + const r = validateTestCase( + ` + + + + + + sf:ui:target?object=Account + + + + +` + ); + assert.ok( + r.issues.some((i) => i.rule_id === 'UI-TARGET-001'), + 'UI-TARGET-001 should fire when has no class attribute' + ); + const issue = r.issues.find((i) => i.rule_id === 'UI-TARGET-001')!; + assert.ok(issue.message.includes('(missing)'), `Message should note missing class: ${issue.message}`); + }); + it('also fires for UiWithRow steps with wrong target class', () => { const r = validateTestCase( ` @@ -427,6 +450,29 @@ describe('validateTestCase', () => { ); }); + it('fires when UiDoAction locator has no class attribute', () => { + const r = validateTestCase( + ` + + + + + + sf:ui:locator:label?label=Save + + + + +` + ); + assert.ok( + r.issues.some((i) => i.rule_id === 'UI-LOCATOR-001'), + 'UI-LOCATOR-001 should fire when has no class attribute' + ); + const issue = r.issues.find((i) => i.rule_id === 'UI-LOCATOR-001')!; + assert.ok(issue.message.includes('(missing)'), `Message should note missing class: ${issue.message}`); + }); + it('also fires for UiAssert steps with wrong locator class', () => { const r = validateTestCase( ` @@ -501,6 +547,29 @@ describe('validateTestCase', () => { ); }); + it('fires when SetValues values has no class attribute', () => { + const r = validateTestCase( + ` + + + + + + someText + + + + +` + ); + assert.ok( + r.issues.some((i) => i.rule_id === 'SETVALUES-STRUCTURE-001'), + 'SETVALUES-STRUCTURE-001 should fire when has no class attribute' + ); + const issue = r.issues.find((i) => i.rule_id === 'SETVALUES-STRUCTURE-001')!; + assert.ok(issue.message.includes('(missing)'), `Message should note missing class: ${issue.message}`); + }); + it('does not fire when SetValues has no values argument (self-closing)', () => { const r = validateTestCase( ` diff --git a/test/unit/mcp/testPlanTools.test.ts b/test/unit/mcp/testPlanTools.test.ts index 28f299f..5002188 100644 --- a/test/unit/mcp/testPlanTools.test.ts +++ b/test/unit/mcp/testPlanTools.test.ts @@ -207,6 +207,34 @@ describe('provar.testplan.create', () => { assert.equal(parseText(result)['created'], true); }); + it('returns INVALID_PLAN_NAME for dot-segment plan_name (..)', () => { + makeProject(projectDir); + + const result = server.call('provar.testplan.create', { + project_path: projectDir, + plan_name: '..', + overwrite: false, + dry_run: false, + }); + + assert.equal(isError(result), true); + assert.equal(errorCode(result), 'INVALID_PLAN_NAME'); + }); + + it('returns INVALID_PLAN_NAME for plan_name with path separators', () => { + makeProject(projectDir); + + const result = server.call('provar.testplan.create', { + project_path: projectDir, + plan_name: 'sub/plan', + overwrite: false, + dry_run: false, + }); + + assert.equal(isError(result), true); + assert.equal(errorCode(result), 'INVALID_PLAN_NAME'); + }); + it('returns PATH_NOT_ALLOWED when project_path is outside allowedPaths', () => { const strictServer = new MockMcpServer(); registerAllTestPlanTools(strictServer as never, { allowedPaths: [tmpDir] });