chore(scripts): rename ticket-prefixed scripts + enforce naming convention#179
Merged
Conversation
…enforcement lint
RCA: Three scripts under scripts/ were named after the tickets that introduced them (pdx-481-trace.cjs, pdx-481-validate.cjs, pdx-482-validate.cjs). Ticket-prefixed filenames anchor the repo to internal Jira IDs, age poorly once the ticket closes, and surface in customer-visible artifacts (CI logs, PR diffs, repo browsing). The provar_testcase_generate STEPS_REQUIRED error message also referenced PDX-479 in the message body returned to the MCP client, leaking internal nomenclature into a customer-facing surface.
Fix: Renamed pdx-481-trace.cjs -> authoring-flow-trace.cjs, pdx-481-validate.cjs -> authoring-guidance-validate.cjs, pdx-482-validate.cjs -> construction-contract-validate.cjs (git mv preserves history). Updated each script's header comment, internal clientInfo identifiers, log strings, and console.log output to drop PDX-XXX references. Reworded the STEPS_REQUIRED error message in testCaseGenerate.ts to describe the contract violation in behavioural terms ("Constructing a test case requires the full step tree in a single call") instead of citing a ticket. Added scripts/lint-script-names.cjs which fails the lint chain if any file under scripts/ matches ^pdx[-_]?\d+ (case-insensitive); wired into wireit as a dependency of yarn lint. Documented the convention in CLAUDE.md.
Quality Orchestrator🟢 LOW · 🧪 Tests to Run · Running 1 of 51 tests
▶ Run commandnpx vitest run \
unit/mcp/testCaseGenerate.test.ts⚡ quality-orchestrator · |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR removes ticket-ID anchoring from script filenames and customer/LLM-facing messaging, and adds a lint gate to prevent reintroducing ticket-prefixed script names.
Changes:
- Renames three
scripts/pdx-*utilities to behavior-based names and updates their internal identifiers/log strings. - Updates the
provar_testcase_generateSTEPS_REQUIREDerror message to remove the customer-facingPDX-479reference. - Adds a wireit-wired script-name lint (
lint:script-names) and documents the naming convention inCLAUDE.md.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/mcp/tools/testCaseGenerate.ts | Scrubs ticket reference from STEPS_REQUIRED runtime-guard error message. |
| scripts/mcp-smoke.cjs | Updates smoke-test copy/paths and references to renamed validation script. |
| scripts/lint-script-names.cjs | Introduces a lint script to reject ticket-prefixed script filenames. |
| scripts/construction-contract-validate.cjs | Renamed + updated identifiers/messages to behavior-based naming. |
| scripts/authoring-guidance-validate.cjs | Renamed + updated identifiers/messages to behavior-based naming. |
| scripts/authoring-flow-trace.cjs | Renamed + updated identifiers/messages to behavior-based naming. |
| package.json | Wires lint:script-names into yarn lint via wireit. |
| CLAUDE.md | Documents the new script naming convention and enforcement mechanism. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+25
to
+29
| const offenders = fs | ||
| .readdirSync(SCRIPTS_DIR, { withFileTypes: true }) | ||
| .filter((e) => e.isFile()) | ||
| .map((e) => e.name) | ||
| .filter((name) => TICKET_PREFIX_RE.test(name)) |
Collaborator
Author
There was a problem hiding this comment.
Fixed in 82de452 — now recurses into subdirectories. Verified with three states: clean (exit 0), (exit 1, top-level), (exit 1, reports the full nested path). CLAUDE.md updated to call out nested coverage explicitly.
| "command": "node scripts/lint-script-names.cjs", | ||
| "files": [ | ||
| "scripts/lint-script-names.cjs", | ||
| "scripts/*" |
Collaborator
Author
There was a problem hiding this comment.
Fixed in 82de452 — wireit files glob is now scripts/**/*. Cache invalidation now triggers on changes anywhere under scripts/, matching the recursive walk in the lint check.
…view RCA: Copilot review on PR #179 flagged two related gaps. (1) scripts/lint-script-names.cjs used readdirSync without recursion, so a future scripts/tmp/pdx-NNN.cjs nested file would bypass the rule despite the documentation saying "under scripts/". (2) The wireit `files` glob for `lint:script-names` was `scripts/*`, which excludes subdirectories, so wireit's cache would not invalidate on changes to nested files even if the check itself were recursive. Fix: Rewrote the offender walk as a recursive `walk(dir)` that traverses subdirectories and reports relative paths from `scripts/`. Updated the wireit `files` glob to `scripts/**/*` so cache invalidation covers nested additions. Tightened CLAUDE.md wording to call out "anywhere under scripts/ (including nested subdirectories)" and gave a nested-path example in the rejected list. Validated three states: clean (exit 0), top-level offender (exit 1), nested offender (exit 1 reporting the full nested path).
… after PR #177 landed RCA: PR #177 (PDX-484 tool-title hardening) landed on develop while this cleanup branch was open. Both branches modified scripts/pdx-482-validate.cjs — this branch renamed it to scripts/construction-contract-validate.cjs and scrubbed its PDX-XXX header references, while PR #177 added a titleAssertions() helper and rewrote the header on the old filename. Git auto-merged the title-assertion content into the renamed file (rename detection worked) but conflicted on the header comment block. Fix: Kept the clean header from this branch (no PDX-XXX refs) and merged in the new title-contract-pass paragraph reworded without the PDX-484 prefix ("Title-contract pass:" instead of "PDX-484 — title contract"). Also scrubbed the "(PDX-484)" suffixes from titleAssertions() assertion labels and the "PDX-484:" prefix from the two comment lines that call titleAssertions(toolList, record) — keeping the script free of ticket IDs to match the convention. src/mcp/tools/testCaseGenerate.ts auto-merged cleanly: the STEPS_REQUIRED error-message scrub from this branch and the title field addition from PDX-484 coexist. Verified: yarn compile clean, yarn lint clean (includes lint:script-names), 1140 unit tests pass, construction-contract-validate.cjs 40/40 PASS (12 new title-contract assertions now exercised against the merged tool).
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
STEPS_REQUIREDerror message returned byprovar_testcase_generate— that message reaches the MCP client and is read by the LLM and end users.scripts/lint-script-names.cjsas a wireit-wired lint dependency that fails the build if any file underscripts/matches^pdx[-_]?\d+(case-insensitive). Document the convention inCLAUDE.mdso future contributors and agents see it.Renames (git mv — history preserved)
scripts/pdx-481-trace.cjsscripts/authoring-flow-trace.cjsscripts/pdx-481-validate.cjsscripts/authoring-guidance-validate.cjsscripts/pdx-482-validate.cjsscripts/construction-contract-validate.cjsEach renamed script's internal references updated: header comment,
clientInfo.name, andconsole.logoutput strings.Error message change
src/mcp/tools/testCaseGenerate.tsSTEPS_REQUIRED:This produces a contract-violating skeleton (the PDX-479 regression pattern) and is rejected.Constructing a test case requires the full step tree in a single call; an empty payload on the write path would produce a skeleton-only file.The
details.suggestionfield is unchanged — it never referenced a ticket — so all existing assertions (construction-contract-validate.cjs28/28, unit tests STEPS_REQUIRED block) continue to pass.Enforcement
scripts/lint-script-names.cjsexits non-zero with a clear remediation message when anyscripts/pdx-NNN-*file exists. Wired intoyarn lintvia wireit:Negative-tested by temporarily dropping a
pdx-999-fake-test.cjsintoscripts/— the check failed with exit 1 and printed the offender plus a rename hint.Test plan
yarn compile— cleannode_modules/.bin/nyc node_modules/.bin/mocha "test/**/*.test.ts"— 1136 passing (no change vs baseline; STEPS_REQUIRED tests assert ondetails.suggestionwhich I didn't touch)yarn lint— clean (runslint:script-namesfirst, then ESLint)node scripts/construction-contract-validate.cjs— 28/28 PASS via the new filenametouch scripts/pdx-999-fake-test.cjs && node scripts/lint-script-names.cjs→ exit 1 with offender listedWhy PDX-0
This is a chore/cleanup that doesn't change any product behaviour or fix a specific ticket. The renames are mechanical and the enforcement script is preventive — it stops the convention from drifting back.
Notes for reviewers
src/andtest/still contain PDX-XXX references in dev-facing places (test block headers, internal regression-guard comments). Those are intentional and aligned with the existing memory rule that source comments and PR descriptions are the right place for ticket IDs. This PR only touches surfaces that customers / LLMs see (error message bodies + script filenames + CI log strings).docs/mcp-pilot-guide.mdScenario 12 background remain untouched — flagging as a possible follow-up sweep if desired.🤖 Generated with Claude Code