diff --git a/docs/mcp.md b/docs/mcp.md index a3798c4..b350704 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -2038,13 +2038,14 @@ Read cached Salesforce describe data for one connection from the Provar workspac **Prerequisite:** the project must have been opened in Provar IDE at least once with the named connection loaded, and (for the chosen test environment) the relevant objects expanded so their metadata is written to disk. If the cache is missing, the tool returns a structured response with `details.suggestion` rather than an error. -**Workspace discovery heuristic** — the tool walks candidate directories in this order and uses the first one that exists: +**Workspace discovery heuristic** — the tool walks candidate directories in this order and uses the first one that is a Provar workspace (i.e. contains a `.metadata` directory): -1. `/workspace-/` — sibling workspace pattern (default for Provar IDE in this workspace layout). -2. `/Provar_Workspaces/workspace-/` — shared `Provar_Workspaces` directory. -3. `~/Provar/workspace-/` — user-home fallback. +1. `/` — the project's parent directory (the project lives inside its workspace; default for the Provar IDE layout). +2. `/workspace-/` — sibling-workspace layout (kept for backward compatibility with earlier releases). +3. `/Provar_Workspaces/workspace-/` — shared `Provar_Workspaces` directory. +4. `~/Provar/workspace-/` — user-home fallback. -`` is the project's basename with whitespace collapsed to single dashes and lowercased: `"My Project"` → `"my-project"`. +`` is the project directory's name verbatim (e.g. `"MyProject"`). A candidate is skipped unless it contains a `.metadata` directory, so candidate 1 (which almost always exists) falls through to the fallbacks when the parent is not itself a workspace. Each candidate is also checked against `--allowed-paths` before any filesystem access; candidates outside the policy are silently skipped. **Cache layout resolution (within the discovered workspace).** The tool prefers the Provar IDE SfObject cache and falls back to the legacy/native layout: @@ -2064,7 +2065,7 @@ Read cached Salesforce describe data for one connection from the Provar workspac | Output field | Description | | ------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | `requestId` | UUID for this invocation. Echoed in MCP server logs for cross-correlation. Consistent with the rest of the MCP tool surface. | -| `workspace_path` | Absolute resolved path to the discovered workspace, or `null` when none of the three candidate directories exists (or all candidates were outside `--allowed-paths`). | +| `workspace_path` | Absolute resolved path to the discovered workspace, or `null` when none of the four candidate directories is a Provar workspace (contains `.metadata`), or all candidates were outside `--allowed-paths`. | | `cache_age_ms` | `mtime` delta in milliseconds of the connection cache directory, or `null` when the cache is missing. | | `objects[]` | Array of `{ name, exists, required_fields, field_count, error_message? }`. `exists` is `true` (cache file present), `false` (requested but not cached), or `null` (cache miss — the whole `.metadata/` directory is absent). | | `objects[].error_message` | Present **only** when a cache file existed but failed to parse (`exists: true, field_count: 0`). Lets the agent distinguish a corrupt / unsupported cache file from a missing one. | diff --git a/package.json b/package.json index 96cb040..934ce8f 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "@provartesting/provardx-cli", "description": "A plugin for the Salesforce CLI to orchestrate testing activities and report quality metrics to Provar Quality Hub", - "version": "1.6.3", + "version": "1.6.4", "mcpName": "io.github.ProvarTesting/provar", "license": "BSD-3-Clause", "plugins": [ diff --git a/server.json b/server.json index c32307e..b269900 100644 --- a/server.json +++ b/server.json @@ -14,12 +14,12 @@ "url": "https://github.com/ProvarTesting/provardx-cli", "source": "github" }, - "version": "1.6.3", + "version": "1.6.4", "packages": [ { "registryType": "npm", "identifier": "@provartesting/provardx-cli", - "version": "1.6.3", + "version": "1.6.4", "transport": { "type": "stdio" }, diff --git a/src/mcp/tools/orgDescribeTools.ts b/src/mcp/tools/orgDescribeTools.ts index 1184c87..2b9a08e 100644 --- a/src/mcp/tools/orgDescribeTools.ts +++ b/src/mcp/tools/orgDescribeTools.ts @@ -78,44 +78,48 @@ interface CachedObject { // ── Workspace discovery ─────────────────────────────────────────────────────── -/** - * Normalise a project basename for use in fallback workspace dir names: - * "My Project Path " → "my-project-path". - */ -export function projectNameDashes(projectPath: string): string { - return path.basename(projectPath).trim().replace(/\s+/g, '-').toLowerCase(); -} - /** * Build the ordered list of candidate workspace directories. - * First existing wins. - * 1. /workspace-/ — sibling workspace pattern. - * 2. /Provar_Workspaces/workspace-/ - * 3. ~/Provar/workspace-/ — user-home fallback. + * The first candidate that is a real Provar workspace (contains `.metadata`) wins. + * 1. / — the project lives INSIDE its workspace (the Provar IDE layout). + * 2. /workspace-/ — sibling-workspace layout (kept for backward compatibility). + * 3. /Provar_Workspaces/workspace-/ — shared Provar_Workspaces directory. + * 4. ~/Provar/workspace-/ — user-home fallback. + * + * The parent and sibling candidates are BOTH probed: parent is the current IDE layout, + * the sibling is the layout an earlier release assumed. Probing both keeps existing + * sibling-layout workspaces discoverable while preferring the parent layout when both exist. */ export function workspaceCandidates(projectPath: string): string[] { const resolved = path.resolve(projectPath); const parent = path.dirname(resolved); const basename = path.basename(resolved); - const dashes = projectNameDashes(resolved); return [ + parent, path.join(parent, `workspace-${basename}`), - path.join(parent, 'Provar_Workspaces', `workspace-${dashes}`), - path.join(os.homedir(), 'Provar', `workspace-${dashes}`), + path.join(parent, 'Provar_Workspaces', `workspace-${basename}`), + path.join(os.homedir(), 'Provar', `workspace-${basename}`), ]; } /** - * Returns the first candidate workspace that exists AND is within allowedPaths, or null. + * Returns the first candidate that is a real Provar workspace AND is within + * allowedPaths, or null. + * + * A candidate qualifies only if it is a directory that contains a `.metadata` + * subdirectory — the marker of a Provar/Eclipse workspace. This matters because + * candidate 1 is the project's parent directory, which almost always exists: the + * `.metadata` requirement lets discovery fall through to the sibling-workspace, + * Provar_Workspaces, and ~/Provar fallbacks when the parent is not itself a workspace. * * Path policy is enforced PER CANDIDATE before any filesystem call: a candidate that * sits outside `--allowed-paths` is silently skipped (it is not an error — discovery * just moves on to the next). This means we never call fs.existsSync / fs.statSync * against directories that the operator has explicitly placed off-limits, including - * the user-home fallback (~/Provar/...) when home sits outside the policy. + * the user-home fallback (~/Provar/workspace-/) when home sits outside the policy. * * When allowedPaths is empty (unrestricted mode), assertPathAllowed is a no-op and - * all candidates are probed exactly as before. + * all candidates are probed. */ export function discoverWorkspace(projectPath: string, allowedPaths: string[] = []): string | null { for (const candidate of workspaceCandidates(projectPath)) { @@ -125,12 +129,10 @@ export function discoverWorkspace(projectPath: string, allowedPaths: string[] = // Candidate outside policy — skip without touching the filesystem. continue; } - try { - if (fs.existsSync(candidate) && fs.statSync(candidate).isDirectory()) { - return candidate; - } - } catch { - // Permission errors etc. — skip and try next candidate + // A candidate qualifies only if it holds a `.metadata` subdirectory; this + // also implies the candidate itself is an existing directory. + if (isExistingDir(path.join(candidate, '.metadata'))) { + return candidate; } } return null; @@ -521,8 +523,9 @@ export function registerOrgDescribe(server: McpServer, config: ServerConfig): vo 'Read cached Salesforce describe data for one connection from the Provar workspace .metadata cache.', 'Prerequisite: the project must have been opened in Provar IDE at least once with the named connection loaded', '(and, for the named test environment, the relevant objects expanded) — this tool is read-only and does NOT trigger a metadata download.', - 'Workspace discovery tries in order: /workspace-, ', - '/Provar_Workspaces/workspace-, then ~/Provar/workspace-.', + 'Workspace discovery uses the first of these directories that contains a .metadata dir (and is within --allowed-paths):', + ' (project inside its workspace), /workspace-,', + '/Provar_Workspaces/workspace-, then ~/Provar/workspace-.', 'Within the workspace it prefers the Provar IDE SfObject cache at', '.metadata/.plugins/com.provar.eclipse.ui///SfObject/.xml', '(environment defaults to "default"; if the requested environment is absent it falls back to any cached environment),', diff --git a/test/unit/mcp/orgDescribeTools.test.ts b/test/unit/mcp/orgDescribeTools.test.ts index 2ac588f..aac8361 100644 --- a/test/unit/mcp/orgDescribeTools.test.ts +++ b/test/unit/mcp/orgDescribeTools.test.ts @@ -14,7 +14,6 @@ import { describe, it, beforeEach, afterEach } from 'mocha'; import { registerOrgDescribe, discoverWorkspace, - projectNameDashes, workspaceCandidates, } from '../../../src/mcp/tools/orgDescribeTools.js'; import type { ServerConfig } from '../../../src/mcp/server.js'; @@ -163,11 +162,14 @@ beforeEach(() => { // Use realpathSync to canonicalise the path on macOS (/var → /private/var) so // assertPathAllowed comparisons match the realpath the policy resolves to. tmpRoot = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'org-describe-test-'))); - projectPath = path.join(tmpRoot, 'MyProject'); + // The project lives INSIDE its workspace directory; discovery resolves the workspace + // as the project's parent (/). Naming the parent workspace-MyProject lets the + // per-test fixtures keep writing caches under /workspace-MyProject. + projectPath = path.join(tmpRoot, 'workspace-MyProject', 'MyProject'); fs.mkdirSync(projectPath, { recursive: true }); server = new MockMcpServer(); - // tmpRoot must be allowed so both the project path and any sibling workspace + // tmpRoot must be allowed so both the project path and any workspace // candidate (also under tmpRoot) pass the path policy check. config = { allowedPaths: [tmpRoot] }; registerOrgDescribe(server as never, config); @@ -177,40 +179,61 @@ afterEach(() => { fs.rmSync(tmpRoot, { recursive: true, force: true }); }); -// ── projectNameDashes / workspaceCandidates ─────────────────────────────────── - -describe('projectNameDashes', () => { - it('lowercases and replaces whitespace with single dashes', () => { - assert.equal(projectNameDashes('/x/My Project Path'), 'my-project-path'); - assert.equal(projectNameDashes('/x/ Spaced Name '), 'spaced-name'); - }); -}); +// ── workspaceCandidates ─────────────────────────────────────────────────────── describe('workspaceCandidates', () => { - it('returns three candidates in expected order', () => { + it('returns four candidates in order: parent, sibling, Provar_Workspaces, home', () => { const cands = workspaceCandidates('/Users/alice/projects/My Project'); - assert.equal(cands.length, 3); + assert.equal(cands.length, 4); + assert.equal(cands[0], path.resolve('/Users/alice/projects'), `Expected project parent first, got: ${cands[0]}`); assert.ok( - cands[0].endsWith(`${path.sep}workspace-My Project`), - `Expected sibling workspace first, got: ${cands[0]}` + cands[1].endsWith(`${path.sep}workspace-My Project`) && !cands[1].includes('Provar_Workspaces'), + `Expected sibling /workspace- second, got: ${cands[1]}` ); assert.ok( - cands[1].includes('Provar_Workspaces') && cands[1].endsWith('workspace-my-project'), - `Expected Provar_Workspaces second, got: ${cands[1]}` + cands[2].includes('Provar_Workspaces') && cands[2].endsWith('workspace-My Project'), + `Expected Provar_Workspaces third, got: ${cands[2]}` ); - assert.ok(cands[2].endsWith(`${path.sep}Provar${path.sep}workspace-my-project`)); + assert.ok(cands[3].endsWith(`${path.sep}Provar${path.sep}workspace-My Project`)); }); }); -// ── (a) Workspace discovery — sibling pattern ───────────────────────────────── +// ── (a) Workspace discovery — project parent ────────────────────────────────── describe('provar_org_describe — workspace discovery', () => { - it('(a) finds the sibling workspace at /workspace-', () => { - // /workspace-MyProject is the sibling pattern - const siblingWorkspace = path.join(tmpRoot, 'workspace-MyProject'); - const connectionDir = path.join(siblingWorkspace, '.metadata', 'MyOrg'); + it('(a) finds the workspace at the project parent (/)', () => { + // The project's parent dir (/workspace-MyProject) is the workspace. + const workspace = path.join(tmpRoot, 'workspace-MyProject'); + const connectionDir = path.join(workspace, '.metadata', 'MyOrg'); + writeJsonCache(connectionDir, 'Account', [{ name: 'Name', type: 'string', defaultValue: null, nillable: false }]); + + const result = server.call('provar_org_describe', { + project_path: projectPath, + connection_name: 'MyOrg', + }); + + assert.equal(isError(result), false); + const body = parseText(result); + assert.equal(body['workspace_path'], workspace, 'should discover the project-parent workspace'); + const objects = body['objects'] as Array<{ name: string; exists: boolean | null; field_count: number }>; + assert.equal(objects.length, 1); + assert.equal(objects[0].name, 'Account'); + assert.equal(objects[0].exists, true); + assert.equal(objects[0].field_count, 1); + }); + + it('(a2) falls back to the sibling /workspace- workspace (back-compat)', () => { + // Back-compat layout: the project's parent is NOT itself a workspace, but a sibling + // dir named workspace- is. beforeEach leaves the parent (/workspace-MyProject) + // without a .metadata dir, so candidate 1 (parent) falls through to candidate 2 (sibling). + const parent = path.dirname(projectPath); // /workspace-MyProject + const sibling = path.join(parent, 'workspace-MyProject'); // /workspace- + const connectionDir = path.join(sibling, '.metadata', 'MyOrg'); writeJsonCache(connectionDir, 'Account', [{ name: 'Name', type: 'string', defaultValue: null, nillable: false }]); + // Sanity: the parent must NOT be a workspace here, otherwise candidate 1 would win. + assert.equal(fs.existsSync(path.join(parent, '.metadata')), false, 'parent must not have .metadata for this test'); + const result = server.call('provar_org_describe', { project_path: projectPath, connection_name: 'MyOrg', @@ -218,7 +241,7 @@ describe('provar_org_describe — workspace discovery', () => { assert.equal(isError(result), false); const body = parseText(result); - assert.equal(body['workspace_path'], siblingWorkspace, 'should discover sibling workspace'); + assert.equal(body['workspace_path'], sibling, 'should discover the sibling workspace- workspace'); const objects = body['objects'] as Array<{ name: string; exists: boolean | null; field_count: number }>; assert.equal(objects.length, 1); assert.equal(objects[0].name, 'Account'); @@ -226,14 +249,14 @@ describe('provar_org_describe — workspace discovery', () => { assert.equal(objects[0].field_count, 1); }); - it('(b) falls back to user-home workspace when sibling missing (via override)', () => { + it('(b) falls back to user-home workspace when project parent is not a workspace (via override)', () => { // Stand in for ~/Provar by using a HOME override. The tool uses os.homedir(), // and we override $HOME for this test only. Set the home to a tmp dir so the // path is inside allowed paths. const fakeHome = path.join(tmpRoot, 'fakehome'); fs.mkdirSync(fakeHome, { recursive: true }); - const homeWorkspace = path.join(fakeHome, 'Provar', 'workspace-myproject'); + const homeWorkspace = path.join(fakeHome, 'Provar', 'workspace-MyProject'); const connectionDir = path.join(homeWorkspace, '.metadata', 'MyOrg'); writeJsonCache(connectionDir, 'Contact', [ { name: 'LastName', type: 'string', defaultValue: null, nillable: false }, @@ -270,20 +293,17 @@ describe('provar_org_describe — workspace discovery', () => { }); it('discoverWorkspace skips candidates outside allowedPaths without touching the filesystem', () => { - // Create a sibling workspace that DOES exist on disk but lies outside the allowed root. - // We force this by creating a parallel tmp tree and only allowing tmpRoot. - // The sibling pattern resolves to /workspace-; the parent of - // projectPath is tmpRoot, so the sibling itself is inside the allowed set — - // which is what we want for the happy case. To exercise the policy skip we use - // a separate "outside" project path whose sibling candidate lives outside. + // Create a real workspace (with .metadata) that DOES exist on disk but lies outside + // the allowed root. The project parent (candidate 1) resolves to outsideRoot, so it + // would otherwise qualify — but it must be skipped because it is outside policy. const outsideRoot = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'org-describe-outside-'))); try { const outsideProject = path.join(outsideRoot, 'OtherProject'); fs.mkdirSync(outsideProject, { recursive: true }); - const outsideSibling = path.join(outsideRoot, 'workspace-OtherProject'); - fs.mkdirSync(outsideSibling, { recursive: true }); + // outsideRoot is the project parent (candidate 1); make it a genuine workspace. + fs.mkdirSync(path.join(outsideRoot, '.metadata'), { recursive: true }); - // With only tmpRoot allowed, discoverWorkspace MUST NOT return the outside sibling + // With only tmpRoot allowed, discoverWorkspace MUST NOT return the outside workspace // even though it exists on disk. assert.equal(discoverWorkspace(outsideProject, [tmpRoot]), null); } finally { @@ -296,7 +316,7 @@ describe('provar_org_describe — workspace discovery', () => { describe('provar_org_describe — cache miss', () => { it('(c) returns suggestion when workspace exists but .metadata/ absent', () => { - // Create the sibling workspace dir but NOT the connection subdir + // The project parent is the workspace; create its .metadata but NOT the connection subdir const siblingWorkspace = path.join(tmpRoot, 'workspace-MyProject'); fs.mkdirSync(path.join(siblingWorkspace, '.metadata'), { recursive: true });