From f0e1b2569a78e14c80f6dc54b2c2c08d27357b0c Mon Sep 17 00:00:00 2001 From: Michael Dailey Date: Thu, 14 May 2026 16:31:08 -0500 Subject: [PATCH] PDX-478: fix(mcp): parse connections.connection[] and resolve env via associations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RCA: provar_connection_list returned 0 connections on every real Provar project (surveyed 7) because parseConnectionList traversed connectionClass.connection[] but the actual .testproject XML nests each under a wrapper. parseEnvironmentList separately read @connectionName and @url attributes that do not exist on elements — the connection link is in , keyed to the connection's @id, and per-env URLs live on the matching connection's entry. The existing test fixture used the flattened (broken) XML shape that the buggy code expected, so CI never caught either bug. Fix: Traverse connectionClass.connections.connection[]. Build a connection-id-to-info map containing the default URL (the connectionUrl with no @envId) and a map of envId-to-URL overrides. Resolve environment.connection by mapping associations.association[0].@connectionId back to the connection name via that map, and use the env-specific URL when env.@guid matches a connectionUrl.@envId. Handle empty (string-empty per real Provar XML) gracefully. Replace the fixture to mirror real .testproject XML and add tests for env→connection resolution, env-specific URL via @guid, default-URL selection, and empty-associations. Verified non-zero connection counts and correct env→connection resolution across 7 real projects (provar-manager-regression, Agentforce, ExperienceCloud, FinancialServices, PQP, ProvarDXGrid, TrialProject). All 1109 unit tests pass; 54 smoke responses pass. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/mcp/tools/connectionTools.ts | 113 ++++++++++++++++++++------ test/unit/mcp/connectionTools.test.ts | 95 ++++++++++++++++++---- 2 files changed, 168 insertions(+), 40 deletions(-) diff --git a/src/mcp/tools/connectionTools.ts b/src/mcp/tools/connectionTools.ts index f1d1d14e..398b2407 100644 --- a/src/mcp/tools/connectionTools.ts +++ b/src/mcp/tools/connectionTools.ts @@ -59,7 +59,12 @@ const TP_PARSER = new XMLParser({ ignoreAttributes: false, attributeNamePrefix: '@_', parseAttributeValue: false, - isArray: (name): boolean => name === 'connectionClass' || name === 'connection' || name === 'environment', + isArray: (name): boolean => + name === 'connectionClass' || + name === 'connection' || + name === 'environment' || + name === 'connectionUrl' || + name === 'association', }); class XmlParseError extends Error { @@ -77,33 +82,73 @@ function parseTestProjectXml(content: string): Record { return raw !== null && typeof raw === 'object' ? (raw as Record) : {}; } -function parseConnectionList(content: string): ConnectionEntry[] { - const tp = parseTestProjectXml(content); +interface ConnectionInfo { + name: string; + className: string; + defaultUrl?: string; + urlsByEnvId: Map; +} + +function buildConnectionMap(tp: Record): Map { + const map = new Map(); const cc = tp['connectionClasses']; - if (!cc || typeof cc !== 'object') return []; + if (!cc || typeof cc !== 'object') return map; const classesRaw = (cc as Record)['connectionClass']; - if (!classesRaw) return []; - const classes = classesRaw as Array>; + if (!Array.isArray(classesRaw)) return map; - const connections: ConnectionEntry[] = []; - for (const cls of classes) { + for (const cls of classesRaw as Array>) { const className = cls['@_name'] as string | undefined; if (!className) continue; - const connsRaw = cls['connection']; - if (!connsRaw) continue; + // Real .testproject XML nests each inside a wrapper. + const connsWrap = cls['connections'] as Record | undefined; + const connsRaw = connsWrap?.['connection']; + if (!Array.isArray(connsRaw)) continue; for (const conn of connsRaw as Array>) { - const connName = conn['@_name'] as string | undefined; - if (!connName) continue; - const url = conn['@_url'] as string | undefined; - connections.push({ - name: connName, - type: classToType(className), - ...(url ? { url } : {}), - sso_configured: className === 'sso', - }); + const id = conn['@_id'] as string | undefined; + const name = conn['@_name'] as string | undefined; + if (!name) continue; + + let defaultUrl: string | undefined; + const urlsByEnvId = new Map(); + const urlsWrap = conn['connectionUrls'] as Record | undefined; + const urlsRaw = urlsWrap?.['connectionUrl']; + if (Array.isArray(urlsRaw)) { + for (const u of urlsRaw as Array>) { + const url = u['@_url'] as string | undefined; + if (!url) continue; + const envId = u['@_envId'] as string | undefined; + // The base entry (no @_envId) is the connection's default URL; + // entries with @_envId are environment-specific overrides keyed by env GUID. + if (envId) urlsByEnvId.set(envId, url); + else if (defaultUrl === undefined) defaultUrl = url; + } + } + + const info: ConnectionInfo = { name, className, defaultUrl, urlsByEnvId }; + if (id) map.set(id, info); + // Also key by name so name-based lookups (e.g. legacy callers) still work. + map.set(`name:${name}`, info); } } + return map; +} + +function parseConnectionList(content: string): ConnectionEntry[] { + const tp = parseTestProjectXml(content); + const map = buildConnectionMap(tp); + const connections: ConnectionEntry[] = []; + const seen = new Set(); + for (const info of map.values()) { + if (seen.has(info)) continue; + seen.add(info); + connections.push({ + name: info.name, + type: classToType(info.className), + ...(info.defaultUrl ? { url: info.defaultUrl } : {}), + sso_configured: info.className === 'sso', + }); + } return connections; } @@ -113,18 +158,38 @@ function parseEnvironmentList(content: string): EnvironmentEntry[] { if (!envSection || typeof envSection !== 'object') return []; const envsRaw = (envSection as Record)['environment']; - if (!envsRaw) return []; + if (!Array.isArray(envsRaw)) return []; + const connectionMap = buildConnectionMap(tp); const environments: EnvironmentEntry[] = []; for (const env of envsRaw as Array>) { const name = env['@_name'] as string | undefined; if (!name) continue; - const connection = env['@_connectionName'] as string | undefined; - const url = env['@_url'] as string | undefined; + const envGuid = env['@_guid'] as string | undefined; + + let connectionName = ''; + let envUrl: string | undefined; + // associations may be missing, an empty string (no associations), or an object wrapping an array. + const assocs = env['associations']; + if (assocs !== null && typeof assocs === 'object') { + const assocsRaw = (assocs as Record)['association']; + if (Array.isArray(assocsRaw) && assocsRaw.length > 0) { + const first = assocsRaw[0] as Record; + const connId = first['@_connectionId'] as string | undefined; + if (connId) { + const info = connectionMap.get(connId); + if (info) { + connectionName = info.name; + if (envGuid) envUrl = info.urlsByEnvId.get(envGuid); + } + } + } + } + environments.push({ name, - connection: connection ?? '', - ...(url ? { url } : {}), + connection: connectionName, + ...(envUrl ? { url: envUrl } : {}), }); } return environments; diff --git a/test/unit/mcp/connectionTools.test.ts b/test/unit/mcp/connectionTools.test.ts index 1dc0c1b4..6dbecb82 100644 --- a/test/unit/mcp/connectionTools.test.ts +++ b/test/unit/mcp/connectionTools.test.ts @@ -53,27 +53,62 @@ function writeTestProject(dir: string, content: string): void { // ── .testproject fixture content ────────────────────────────────────────────── +// Mirrors the real .testproject XML shape: +// connectionClass → connections → connection → connectionUrls → connectionUrl +// environment → associations → association[@connectionId] +// The pre-PDX-478 fixture used a flattened shape that did not exist in real +// projects, which is how the parser bugs slipped through CI. const BASIC_TEST_PROJECT = ` - - - - + + + + + + + + + + + + + - - + + + + + + + - - + + + + + + + - - + + + + + + + + + + + + + `; @@ -151,25 +186,53 @@ describe('provar_connection_list', () => { assert.equal(sfConn['sso_configured'], false); }); - it('returns environments with name, connection, and url', () => { + it('resolves environment.connection via associations[@connectionId]', () => { writeTestProject(tmpDir, BASIC_TEST_PROJECT); const result = server.call('provar_connection_list', { project_path: tmpDir }); const environments = parseText(result)['environments'] as Array>; assert.ok(Array.isArray(environments)); - assert.equal(environments.length, 2); + assert.equal(environments.length, 3); const qa = environments.find((e) => e['name'] === 'QA'); assert.ok(qa); assert.equal(qa['connection'], 'MyOrg'); - assert.equal(qa['url'], 'https://qa.example.com'); }); - it('returns environment without url when not present', () => { + it('returns environment-specific url when a connectionUrl has @envId matching env @guid', () => { + writeTestProject(tmpDir, BASIC_TEST_PROJECT); + const result = server.call('provar_connection_list', { project_path: tmpDir }); + const environments = parseText(result)['environments'] as Array>; + const qa = environments.find((e) => e['name'] === 'QA'); + assert.ok(qa); + assert.equal(qa['url'], 'sfdc://user@example.com.qa;environment=SANDBOX'); + }); + + it('omits url on environment when no per-env connectionUrl exists', () => { writeTestProject(tmpDir, BASIC_TEST_PROJECT); const result = server.call('provar_connection_list', { project_path: tmpDir }); const environments = parseText(result)['environments'] as Array>; const uat = environments.find((e) => e['name'] === 'UAT'); assert.ok(uat); - assert.equal(uat['url'], undefined, 'UAT has no url attribute'); + assert.equal(uat['url'], undefined, 'UAT has no @envId-matched connectionUrl'); + }); + + it("handles environments with empty gracefully (no crash, connection='')", () => { + writeTestProject(tmpDir, BASIC_TEST_PROJECT); + const result = server.call('provar_connection_list', { project_path: tmpDir }); + assert.equal(isError(result), false); + const environments = parseText(result)['environments'] as Array>; + const noAssoc = environments.find((e) => e['name'] === 'NoAssoc'); + assert.ok(noAssoc); + assert.equal(noAssoc['connection'], ''); + assert.equal(noAssoc['url'], undefined); + }); + + it('connection.url uses the default connectionUrl (entry without @envId)', () => { + writeTestProject(tmpDir, BASIC_TEST_PROJECT); + const result = server.call('provar_connection_list', { project_path: tmpDir }); + const connections = parseText(result)['connections'] as Array>; + const myOrg = connections.find((c) => c['name'] === 'MyOrg'); + assert.ok(myOrg); + assert.equal(myOrg['url'], 'sfdc://user@example.com;environment=SANDBOX'); }); it('returns summary with correct counts', () => { @@ -177,7 +240,7 @@ describe('provar_connection_list', () => { const result = server.call('provar_connection_list', { project_path: tmpDir }); const summary = parseText(result)['summary'] as Record; assert.equal(summary['connection_count'], 4); - assert.equal(summary['environment_count'], 2); + assert.equal(summary['environment_count'], 3); }); it('returns empty arrays for project with no connections or environments', () => {