diff --git a/src/mcp/tools/bestPracticesEngine.ts b/src/mcp/tools/bestPracticesEngine.ts index f7f23d9..7fdda7e 100644 --- a/src/mcp/tools/bestPracticesEngine.ts +++ b/src/mcp/tools/bestPracticesEngine.ts @@ -756,6 +756,15 @@ function validateUniqueResultNames(tc: XmlNode, rule: BPRule): BPViolation | nul // ── UiWithScreen target URI validator ───────────────────────────────────────── +// Recognised query-param keys on an `sf:ui:target?…` URI. A UiWithScreen SF +// target is treated as valid when it carries AT LEAST ONE of these keys. The set +// is corpus-verified against real Provar IDE test cases (CPQ / Billing / sales +// flows) — a target whose only keys are outside this set (e.g. `?foo=bar`) is +// flagged as having no recognised SF parameters. PDX-518: extended with the +// FlexiPage / Visualforce / record-type navigation params the IDE actually emits +// (`page`, `pageObject`, `flexiPage`, `flexiPath`, `recordType`, `listView`, +// `quickAction`, `relatedList`, `noOverride`) so activating the attribute reader +// does not false-positive on legitimate IDE-authored screens. const VALID_SF_UI_PARAMS = new Set([ 'object', 'action', @@ -766,22 +775,23 @@ const VALID_SF_UI_PARAMS = new Set([ 'lookup', 'fieldService', 'tab', + // Visualforce-page navigation (e.g. SBQQ__sb, blng__creditInvoice). + 'page', + 'pageObject', + // Lightning record-page (FlexiPage) targeting. + 'flexiPage', + 'flexiPath', + // Record-type-scoped navigation. + 'recordType', + // List-view, quick-action and related-list navigation. + 'listView', + 'quickAction', + 'relatedList', + // Standard-action override flag carried on `action=New&noOverride=true` forms; + // legitimate on its own as a recognised SF nav parameter. + 'noOverride', ]); -/** Read the "target" argument from a UiWithScreen call via the wrapper. */ -function getUiWithScreenTarget(call: XmlNode): string | undefined { - const argsNode = call['arguments'] as XmlNode | undefined; - if (!argsNode || typeof argsNode !== 'object') return undefined; - for (const arg of toArr(argsNode['argument'] as XmlNode | XmlNode[])) { - if (!arg || typeof arg !== 'object') continue; - if (arg['@_id'] !== 'target') continue; - const valElem = arg['value'] as XmlNode | undefined; - if (!valElem || typeof valElem !== 'object') return undefined; - return (valElem['#text'] as string | undefined) ?? undefined; - } - return undefined; -} - /** UI-SCREEN-TARGET — validate UiWithScreen target URIs (SF and page object formats). */ function validateUiWithScreenTarget(tc: XmlNode, rule: BPRule): BPViolation | null { const targetApiId = rule.check['apiId'] as string | undefined; @@ -792,7 +802,7 @@ function validateUiWithScreenTarget(tc: XmlNode, rule: BPRule): BPViolation | nu if (!apiId) continue; if (targetApiId && !apiId.includes(targetApiId)) continue; - const target = getUiWithScreenTarget(call); + const target = getUiWithScreenTargetUri(call); if (!target) continue; if (target.startsWith('sf:')) { diff --git a/test/unit/mcp/bestPracticesEngine.test.ts b/test/unit/mcp/bestPracticesEngine.test.ts index d2e12a1..8fe4c0d 100644 --- a/test/unit/mcp/bestPracticesEngine.test.ts +++ b/test/unit/mcp/bestPracticesEngine.test.ts @@ -206,6 +206,57 @@ describe('runBestPractices', () => { ); assert.ok(uwsViolation, 'Expected uiWithScreenTarget violation for missing pageobjects. prefix'); }); + + // PDX-518: real Provar IDE test cases store the target in the uri= ATTRIBUTE + // of , not in element #text. These tests + // exercise the attribute form so the rule actually fires on IDE-authored XML. + function buildUwsAttrXml(uri: string): string { + return ` + + + + + + + + + + +`; + } + + it('passes for a valid SF target in the uri attribute (sf:ui:target?object=Opportunity&action=New)', () => { + const result = runBestPractices( + buildUwsAttrXml('sf:ui:target?object=Opportunity&action=New&noOverride=true') + ); + const uwsViolation = result.violations.find((v) => v.rule_id.includes('UI-SCREEN-TARGET')); + assert.ok(!uwsViolation, `Expected no uiWithScreenTarget violation, got: ${uwsViolation?.message}`); + }); + + it('passes for a CPQ Visualforce-page SF target (sf:ui:target?page=SBQQ__sb&pageObject=pageobjects.EditQuote)', () => { + // PDX-518: real Salesforce CPQ test cases navigate to Visualforce pages via + // page=/pageObject= with no object/action key; these must NOT false-fire. + const result = runBestPractices( + buildUwsAttrXml('sf:ui:target?page=SBQQ__sb&pageObject=pageobjects.EditQuote') + ); + const uwsViolation = result.violations.find((v) => v.rule_id.includes('UI-SCREEN-TARGET')); + assert.ok( + !uwsViolation, + `Expected no uiWithScreenTarget violation for CPQ page target, got: ${uwsViolation?.message}` + ); + }); + + it('fires for an SF target in the uri attribute with no recognised SF params', () => { + const result = runBestPractices(buildUwsAttrXml('sf:ui:target?foo=bar')); + const uwsViolation = result.violations.find((v) => v.rule_id.includes('UI-SCREEN-TARGET')); + assert.ok(uwsViolation, 'Expected uiWithScreenTarget violation for SF target with no recognised params'); + }); + + it('fires for a page object target in the uri attribute with bad pageId prefix', () => { + const result = runBestPractices(buildUwsAttrXml('ui:pageobject:target?pageId=LoginPage')); + const uwsViolation = result.violations.find((v) => v.rule_id.includes('UI-SCREEN-TARGET')); + assert.ok(uwsViolation, 'Expected uiWithScreenTarget violation for missing pageobjects. prefix in uri attribute'); + }); }); // ── UI-NEST-STRUCT-001 — UI action nesting structure ──