PDX-518: fix(mcp): read UiWithScreen target from uri attribute

[mrdailey99]

Summary

UI-SCREEN-TARGET-001 (validateUiWithScreenTarget in src/mcp/tools/bestPracticesEngine.ts) read the UiWithScreen screen target only from the element #text. Real Provar IDE test cases store the target in the uri= attribute of <value class="uiTarget" uri="sf:ui:target?object=...&action=..."/>, not in #text. As a result the rule effectively never fired on IDE-authored test cases — it only saw a target on hand-authored #text XML.

The fix switches the rule to the attribute-aware reader getUiWithScreenTargetUri (added by PDX-516), which reads the uri= attribute first and falls back to #text for hand-authored XML. The now-unused #text-only getUiWithScreenTarget helper (its only caller was this rule) is deleted, consolidating to a single reader.

Jira

https://provartesting.atlassian.net/browse/PDX-518

Changes

• src/mcp/tools/bestPracticesEngine.ts
  • validateUiWithScreenTarget now reads the target via getUiWithScreenTargetUri(call) (attribute-aware: uri= first, #text fallback) instead of the #text-only getUiWithScreenTarget(call).
  • Deleted the dead getUiWithScreenTarget helper — confirmed it had no other callers.
• test/unit/mcp/bestPracticesEngine.test.ts
  • Added 3 tests exercising the real <value class="uiTarget" uri="..."/> attribute form: a VALID sf target (object/action) does NOT fire; an sf target with no recognised params FIRES; a page-object target with a bad pageId prefix FIRES.
  • Existing #text-form tests remain green (the fallback still works).

Verification

• Confirmed object and action are both in VALID_SF_UI_PARAMS, so a valid real target (sf:ui:target?object=Opportunity&action=New&noOverride=true) produces NO violation.
• Ran the validator (throwaway harness) against the real reference test case OpportunityCreationByClaude.testcase, which carries uri=-attribute targets (...?object=Opportunity&action=ObjectHome, ...&action=New&noOverride=true, ...&action=View&...): 0 UI-SCREEN-TARGET-001 violations — the rule now SEES the attribute targets and does NOT over-fire on valid ones.
• No committed test/fixtures/*.testcase started firing UI-SCREEN-TARGET-001 from enabling the attribute reader.

Test plan

• yarn compile — clean
• node_modules/.bin/nyc node_modules/.bin/mocha "test/**/*.test.ts" — 1510 passing, 1 pending
• node scripts/mcp-smoke.cjs — 30 deterministic tools PASS; only the known provar_automation_* / provar_testrun_* env-timeout flakes vary between runs (smoke spawns the globally-installed package, not this build)
• yarn lint — clean

Customer-facing validator-behaviour note

This makes a previously-dormant rule actually fire on IDE-authored test cases. UI-SCREEN-TARGET-001 now validates the uri= attribute targets that the Provar IDE writes, where before it only validated hand-authored #text targets. Valid IDE targets are unaffected (no new false positives — verified against a real Opportunity creation test case), but malformed IDE-authored targets that were previously invisible to the rule will now be flagged. Flagging this so the Provar team can update external docs (docs/provar-mcp-public-docs.md, docs/university-of-provar-mcp-course.md) if needed. No internal docs/mcp.md change was required — that file does not describe this rule's target-reading internals.

🤖 Generated with Claude Code

[github-actions]

Quality Orchestrator

🟢 LOW · 2 / 100 · All changed files have mapped tests.

---

🧪 Tests to Run · Running 1 of 57 tests

• unit/mcp/bestPracticesEngine.test.ts

▶ Run command

npx vitest run \
  unit/mcp/bestPracticesEngine.test.ts

---

_{⚡ quality-orchestrator  ·  /qo stub <file>  ·  qo analyze-local}