Feature/wave3 editing tools#138
Open
mrdailey99 wants to merge 2 commits intodevelopfrom
Open
Conversation
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 <noreply@anthropic.com>
- 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>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR tightens validation for test plan creation inputs, expands local test case validation to flag missing XML class attributes (not just wrong ones), and enhances the manual deploy workflow with full git history and Slack release notifications.
Changes:
- Reject unsafe/invalid
plan_namevalues inprovar.testplan.create(e.g., dot segments and path separators) and add unit coverage. - Update
validateTestCaserules to also error when required<value>nodes are missingclassattributes, with new unit tests. - Update the DeployManual GitHub Actions workflow to fetch full history/tags and post a Slack notification with extracted release notes.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
test/unit/mcp/testPlanTools.test.ts |
Adds tests to ensure invalid plan_name inputs are rejected. |
test/unit/mcp/testCaseValidate.test.ts |
Adds tests ensuring missing class attributes now trigger the expected validation rules. |
src/mcp/tools/testPlanTools.ts |
Replaces path-separator checks with a stricter plan_name allowlist regex + updated error message. |
src/mcp/tools/testCaseValidate.ts |
Expands validation rules to treat missing class as an error and improves messages to indicate “(missing)”. |
.github/workflows/DeployManual.yml |
Fetches full git history/tags and builds/sends a Slack Block Kit notification payload after publish. |
Comments suppressed due to low confidence (1)
src/mcp/tools/testPlanTools.ts:119
- The plan_name regex currently allows trailing spaces (e.g. "MyPlan ") because the pattern ends with
[\w\- ]*. On Windows/NTFS this can lead to surprising behavior (trailing spaces often get normalized/stripped), causing the returnedplan_dirto differ from the actual directory created. Consider trimmingplan_nameand/or tightening validation to disallow leading/trailing whitespace (and possibly consecutive whitespace) before creating the directory.
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 start with a letter or digit and contain only letters, digits, underscores, hyphens, or spaces: "${plan_name}"`, requestId)),
},
],
};
}
const planDir = path.join(projectRoot, 'plans', plan_name);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+409
to
+411
| const valueNode = targetArg['value'] as Record<string, unknown> | undefined; | ||
| if (!valueNode) return; | ||
| const valClass = valueNode['@_class'] as string | undefined; |
Comment on lines
+446
to
+448
| const locatorNode = locatorArg['value'] as Record<string, unknown> | undefined; | ||
| if (locatorNode) { | ||
| const valClass = locatorNode['@_class'] as string | undefined; |
Comment on lines
+470
to
+472
| const valuesNode = valuesArg['value'] as Record<string, unknown> | undefined; | ||
| if (valuesNode) { | ||
| const valClass = valuesNode['@_class'] as string | undefined; |
| # 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}") |
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.