From b4a60a58d506ef051b05d5b57ba6d4b7273b33c8 Mon Sep 17 00:00:00 2001 From: Michael Dailey Date: Wed, 13 May 2026 11:13:18 -0500 Subject: [PATCH 1/2] PDX-472: feat(mcp): add fields param for field masking (Thread C) RCA: Iterative fix-validate loops emit full inventory and CLI raw output on every call, compounding token cost with no standard way for agents to request a narrower response. Fix: Add fields= and detail=summary|standard|full params to provar_project_inspect, provar_connection_list, provar_qualityhub_display, and provar_qualityhub_testcase_retrieve. Implement shared utilities fieldMask.ts and detailLevel.ts. Standard default preserves backward compatibility. Co-Authored-By: Claude Sonnet 4.6 --- src/mcp/tools/connectionTools.ts | 22 ++- src/mcp/tools/projectInspect.ts | 41 +++++- src/mcp/tools/qualityHubTools.ts | 70 +++++++++- src/mcp/utils/fieldMask.ts | 77 +++++++++++ test/unit/mcp/connectionTools.test.ts | 53 ++++++++ test/unit/mcp/fieldMask.test.ts | 136 +++++++++++++++++++ test/unit/mcp/projectInspect.test.ts | 186 ++++++++++++++++++++++++++ test/unit/mcp/qualityHubTools.test.ts | 108 +++++++++++++++ 8 files changed, 685 insertions(+), 8 deletions(-) create mode 100644 src/mcp/utils/fieldMask.ts create mode 100644 test/unit/mcp/fieldMask.test.ts create mode 100644 test/unit/mcp/projectInspect.test.ts diff --git a/src/mcp/tools/connectionTools.ts b/src/mcp/tools/connectionTools.ts index 21559ef5..255394ce 100644 --- a/src/mcp/tools/connectionTools.ts +++ b/src/mcp/tools/connectionTools.ts @@ -16,6 +16,7 @@ import { assertPathAllowed, PathPolicyError } from '../security/pathPolicy.js'; import { makeError, makeRequestId } from '../schemas/common.js'; import { log } from '../logging/logger.js'; import { desc } from './descHelper.js'; +import { maskFields, parseFieldsParam } from '../utils/fieldMask.js'; // ── Types ───────────────────────────────────────────────────────────────────── @@ -155,9 +156,20 @@ export function registerConnectionList(server: McpServer, config: ServerConfig): 'string, absolute path to project root' ) ), + fields: z + .string() + .optional() + .describe( + desc( + 'Comma-separated list of top-level response keys to retain (e.g. "connections,summary"). ' + + 'Supports dot notation for nested filtering (e.g. "connections.name,connections.type"). ' + + 'Unknown field names are silently ignored. Omit for full response.', + 'string, optional; comma-separated keys to keep (supports dot notation)' + ) + ), }, }, - ({ project_path }) => { + ({ project_path, fields }) => { const requestId = makeRequestId(); log('info', 'provar_connection_list', { requestId, project_path }); @@ -195,7 +207,7 @@ export function registerConnectionList(server: McpServer, config: ServerConfig): const connections = parseConnectionList(content); const environments = parseEnvironmentList(content); - const result = { + let result: Record = { requestId, project_path: resolvedPath, connections, @@ -205,6 +217,12 @@ export function registerConnectionList(server: McpServer, config: ServerConfig): environment_count: environments.length, }, }; + + const fieldList = parseFieldsParam(fields); + if (fieldList) { + result = maskFields(result, fieldList) as Record; + } + return { content: [{ type: 'text' as const, text: JSON.stringify(result) }], structuredContent: result, diff --git a/src/mcp/tools/projectInspect.ts b/src/mcp/tools/projectInspect.ts index 8114af87..f6b700ea 100644 --- a/src/mcp/tools/projectInspect.ts +++ b/src/mcp/tools/projectInspect.ts @@ -15,6 +15,10 @@ import { assertPathAllowed, PathPolicyError } from '../security/pathPolicy.js'; import { makeError, makeRequestId } from '../schemas/common.js'; import { log } from '../logging/logger.js'; import { desc } from './descHelper.js'; +import { applyDetailLevel, type DetailLevel } from '../utils/detailLevel.js'; +import { maskFields, parseFieldsParam } from '../utils/fieldMask.js'; + +const INSPECT_SUMMARY_FIELDS = ['requestId', 'project_path', 'provar_home', 'summary']; export function registerProjectInspect(server: McpServer, config: ServerConfig): void { server.registerTool( @@ -45,9 +49,31 @@ export function registerProjectInspect(server: McpServer, config: ServerConfig): 'string, absolute path to project root' ) ), + detail: z + .enum(['summary', 'standard', 'full']) + .optional() + .default('standard') + .describe( + desc( + 'Response verbosity: "summary" returns only requestId, project_path, provar_home, and the summary object; ' + + '"standard" (default) returns the full inventory; "full" is identical to standard for this tool.', + 'enum summary|standard|full, optional; default standard' + ) + ), + fields: z + .string() + .optional() + .describe( + desc( + 'Comma-separated list of top-level keys to retain (e.g. "test_case_files,summary"). ' + + 'Supports dot notation for nested filtering (e.g. "test_project.connections"). ' + + 'Unknown field names are silently ignored. Applied after the detail filter.', + 'string, optional; comma-separated keys to keep (supports dot notation)' + ) + ), }, }, - ({ project_path }) => { + ({ project_path, detail, fields }) => { const requestId = makeRequestId(); log('info', 'provar_project_inspect', { requestId, project_path }); @@ -60,7 +86,18 @@ export function registerProjectInspect(server: McpServer, config: ServerConfig): return { isError: true, content: [{ type: 'text' as const, text: JSON.stringify(err) }] }; } - const result = buildProjectInventory(resolved, requestId); + let result = buildProjectInventory(resolved, requestId); + + const detailLevel = (detail ?? 'standard') as DetailLevel; + if (detailLevel !== 'standard') { + result = applyDetailLevel(result, detailLevel, INSPECT_SUMMARY_FIELDS); + } + + const fieldList = parseFieldsParam(fields); + if (fieldList) { + result = maskFields(result, fieldList) as typeof result; + } + return { content: [{ type: 'text' as const, text: JSON.stringify(result) }], structuredContent: result, diff --git a/src/mcp/tools/qualityHubTools.ts b/src/mcp/tools/qualityHubTools.ts index 2f4bf620..ca5921ea 100644 --- a/src/mcp/tools/qualityHubTools.ts +++ b/src/mcp/tools/qualityHubTools.ts @@ -10,6 +10,8 @@ import { z } from 'zod'; import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; import { makeError, makeRequestId } from '../schemas/common.js'; import { log } from '../logging/logger.js'; +import { applyDetailLevel, type DetailLevel } from '../utils/detailLevel.js'; +import { maskFields, parseFieldsParam } from '../utils/fieldMask.js'; import { runSfCommand } from './sfSpawn.js'; import { desc } from './descHelper.js'; @@ -31,6 +33,8 @@ function handleSpawnError( }; } +const QH_SUMMARY_FIELDS = ['requestId', 'exitCode']; + // ── Tool: provar_qualityhub_connect ─────────────────────────────────────────── export function registerQualityHubConnect(server: McpServer): void { @@ -131,9 +135,24 @@ export function registerQualityHubDisplay(server: McpServer): void { 'string, optional; path to sf CLI executable' ) ), + detail: z + .enum(['summary', 'standard', 'full']) + .optional() + .default('standard') + .describe( + 'Response verbosity: "summary" returns only requestId and exitCode; ' + + '"standard" (default) returns requestId, exitCode, stdout, and stderr.' + ), + fields: z + .string() + .optional() + .describe( + 'Comma-separated list of response keys to retain (e.g. "exitCode,stdout"). ' + + 'Unknown field names are silently ignored. Applied after the detail filter.' + ), }, }, - ({ target_org, flags, sf_path }) => { + ({ target_org, flags, sf_path, detail, fields }) => { const requestId = makeRequestId(); log('info', 'provar_qualityhub_display', { requestId, target_org }); @@ -142,7 +161,12 @@ export function registerQualityHubDisplay(server: McpServer): void { if (target_org) args.splice(3, 0, '--target-org', target_org); const result = runSfCommand(args, sf_path); - const response = { requestId, exitCode: result.exitCode, stdout: result.stdout, stderr: result.stderr }; + let response: Record = { + requestId, + exitCode: result.exitCode, + stdout: result.stdout, + stderr: result.stderr, + }; if (result.exitCode !== 0) { return { @@ -156,6 +180,15 @@ export function registerQualityHubDisplay(server: McpServer): void { }; } + const detailLevel = (detail ?? 'standard') as DetailLevel; + if (detailLevel !== 'standard') { + response = applyDetailLevel(response, detailLevel, QH_SUMMARY_FIELDS); + } + const fieldList = parseFieldsParam(fields); + if (fieldList) { + response = maskFields(response, fieldList) as Record; + } + return { content: [{ type: 'text' as const, text: JSON.stringify(response) }], structuredContent: response }; } catch (err) { return handleSpawnError(err, requestId, 'provar_qualityhub_display'); @@ -441,9 +474,24 @@ export function registerQualityHubTestcaseRetrieve(server: McpServer): void { 'string, optional; path to sf CLI executable' ) ), + detail: z + .enum(['summary', 'standard', 'full']) + .optional() + .default('standard') + .describe( + 'Response verbosity: "summary" returns only requestId and exitCode; ' + + '"standard" (default) returns requestId, exitCode, stdout, and stderr.' + ), + fields: z + .string() + .optional() + .describe( + 'Comma-separated list of response keys to retain (e.g. "exitCode,stdout"). ' + + 'Unknown field names are silently ignored. Applied after the detail filter.' + ), }, }, - ({ target_org, flags, sf_path }) => { + ({ target_org, flags, sf_path, detail, fields }) => { const requestId = makeRequestId(); log('info', 'provar_qualityhub_testcase_retrieve', { requestId, target_org }); @@ -452,7 +500,12 @@ export function registerQualityHubTestcaseRetrieve(server: McpServer): void { ['provar', 'quality-hub', 'testcase', 'retrieve', '--target-org', target_org, ...flags], sf_path ); - const response = { requestId, exitCode: result.exitCode, stdout: result.stdout, stderr: result.stderr }; + let response: Record = { + requestId, + exitCode: result.exitCode, + stdout: result.stdout, + stderr: result.stderr, + }; if (result.exitCode !== 0) { return { @@ -466,6 +519,15 @@ export function registerQualityHubTestcaseRetrieve(server: McpServer): void { }; } + const detailLevel = (detail ?? 'standard') as DetailLevel; + if (detailLevel !== 'standard') { + response = applyDetailLevel(response, detailLevel, QH_SUMMARY_FIELDS); + } + const fieldList = parseFieldsParam(fields); + if (fieldList) { + response = maskFields(response, fieldList) as Record; + } + return { content: [{ type: 'text' as const, text: JSON.stringify(response) }], structuredContent: response }; } catch (err) { return handleSpawnError(err, requestId, 'provar_qualityhub_testcase_retrieve'); diff --git a/src/mcp/utils/fieldMask.ts b/src/mcp/utils/fieldMask.ts new file mode 100644 index 00000000..9ea0e964 --- /dev/null +++ b/src/mcp/utils/fieldMask.ts @@ -0,0 +1,77 @@ +/* + * Copyright (c) 2024 Provar Limited. + * All rights reserved. + * Licensed under the BSD 3-Clause license. + * For full license text, see LICENSE.md file in the repo root or https://opensource.org/licenses/BSD-3-Clause + */ + +/** + * Mask an object (or array of objects) to retain only the specified fields. + * + * - Top-level keys: `"name"` keeps only the `name` property + * - Dot notation: `"steps.action"` keeps the `steps` array but only `action` within each element + * - Unknown field names are silently ignored — never an error + * - Arrays: masking is applied to every element + * + * @param obj Source object or array (typed as unknown; cast internally, never through any) + * @param fields Parsed field list — each entry is a dot-path string + */ +export function maskFields(obj: unknown, fields: string[]): unknown { + if (Array.isArray(obj)) { + return obj.map((item) => maskFields(item, fields)); + } + + if (obj === null || typeof obj !== 'object') { + return obj; + } + + const source = obj as Record; + + // Group fields: topLevelKeys contains every key to retain. + // dotFields[key] holds the sub-paths to drill into for that key. + const topLevelKeys = new Set(); + const dotFields: Record = {}; + + for (const field of fields) { + const dotIdx = field.indexOf('.'); + if (dotIdx === -1) { + topLevelKeys.add(field); + } else { + const top = field.slice(0, dotIdx); + const rest = field.slice(dotIdx + 1); + topLevelKeys.add(top); + if (!dotFields[top]) dotFields[top] = []; + dotFields[top].push(rest); + } + } + + const result: Record = {}; + for (const key of topLevelKeys) { + if (!(key in source)) continue; // silently ignore unknown fields + const subPaths = dotFields[key]; + if (subPaths) { + const val = source[key]; + // Dot-path into a primitive can't be narrowed; omit rather than leak the whole value. + if (Array.isArray(val) || (val !== null && typeof val === 'object')) { + result[key] = maskFields(val, subPaths); + } + } else { + result[key] = source[key]; + } + } + + return result; +} + +/** + * Parse a comma-separated fields string into a trimmed, non-empty field list. + * Returns null when the string is absent or blank (caller should skip masking). + */ +export function parseFieldsParam(fields: string | undefined): string[] | null { + if (!fields) return null; + const parsed = fields + .split(',') + .map((f) => f.trim()) + .filter(Boolean); + return parsed.length > 0 ? parsed : null; +} diff --git a/test/unit/mcp/connectionTools.test.ts b/test/unit/mcp/connectionTools.test.ts index cf5ec3a6..1dc0c1b4 100644 --- a/test/unit/mcp/connectionTools.test.ts +++ b/test/unit/mcp/connectionTools.test.ts @@ -190,6 +190,59 @@ describe('provar_connection_list', () => { }); }); + describe('fields param (sparse field masking)', () => { + it('retains only specified top-level keys when fields is provided', () => { + writeTestProject(tmpDir, BASIC_TEST_PROJECT); + const result = server.call('provar_connection_list', { + project_path: tmpDir, + fields: 'connections,summary', + }); + assert.equal(isError(result), false); + const body = parseText(result); + assert.ok('connections' in body, 'connections should be retained'); + assert.ok('summary' in body, 'summary should be retained'); + assert.ok(!('environments' in body), 'environments should be masked out'); + assert.ok(!('requestId' in body), 'requestId should be masked out'); + }); + + it('omitting fields returns the full response', () => { + writeTestProject(tmpDir, BASIC_TEST_PROJECT); + const result = server.call('provar_connection_list', { project_path: tmpDir }); + const body = parseText(result); + assert.ok('connections' in body); + assert.ok('environments' in body); + assert.ok('requestId' in body); + }); + + it('silently ignores unknown field names', () => { + writeTestProject(tmpDir, BASIC_TEST_PROJECT); + const result = server.call('provar_connection_list', { + project_path: tmpDir, + fields: 'connections,ghost_field', + }); + assert.equal(isError(result), false); + const body = parseText(result); + assert.ok('connections' in body); + assert.ok(!('ghost_field' in body)); + }); + + it('supports dot notation to narrow connection entries', () => { + writeTestProject(tmpDir, BASIC_TEST_PROJECT); + const result = server.call('provar_connection_list', { + project_path: tmpDir, + fields: 'connections.name,connections.type', + }); + assert.equal(isError(result), false); + const body = parseText(result); + const connections = body['connections'] as Array>; + assert.ok(Array.isArray(connections)); + assert.ok('name' in connections[0], 'name should be retained'); + assert.ok('type' in connections[0], 'type should be retained'); + assert.ok(!('url' in connections[0]), 'url should be masked out'); + assert.ok(!('sso_configured' in connections[0]), 'sso_configured should be masked out'); + }); + }); + describe('error cases', () => { it('returns CONNECTION_FILE_NOT_FOUND when .testproject is missing', () => { const result = server.call('provar_connection_list', { project_path: tmpDir }); diff --git a/test/unit/mcp/fieldMask.test.ts b/test/unit/mcp/fieldMask.test.ts new file mode 100644 index 00000000..5e6e86ee --- /dev/null +++ b/test/unit/mcp/fieldMask.test.ts @@ -0,0 +1,136 @@ +/* + * Copyright (c) 2024 Provar Limited. + * All rights reserved. + * Licensed under the BSD 3-Clause license. + * For full license text, see LICENSE.md file in the repo root or https://opensource.org/licenses/BSD-3-Clause + */ + +import { strict as assert } from 'node:assert'; +import { describe, it } from 'mocha'; +import { maskFields, parseFieldsParam } from '../../../src/mcp/utils/fieldMask.js'; + +// ── maskFields ──────────────────────────────────────────────────────────────── + +describe('maskFields', () => { + describe('top-level field selection', () => { + it('retains only the specified top-level keys', () => { + const obj = { id: '1', name: 'Test', status: 'PASS', steps: [{ action: 'click' }] }; + const result = maskFields(obj, ['id', 'name']) as Record; + assert.deepEqual(result, { id: '1', name: 'Test' }); + }); + + it('silently ignores unknown field names', () => { + const obj = { id: '1', name: 'Test' }; + const result = maskFields(obj, ['id', 'nonexistent']) as Record; + assert.deepEqual(result, { id: '1' }); + }); + + it('returns empty object when all fields are unknown', () => { + const obj = { id: '1', name: 'Test' }; + const result = maskFields(obj, ['foo', 'bar']) as Record; + assert.deepEqual(result, {}); + }); + }); + + describe('dot notation for nested fields', () => { + it('retains the parent key with only specified sub-fields', () => { + const obj = { steps: [{ action: 'click', element: 'button', wait: 500 }] }; + const result = maskFields(obj, ['steps.action']) as Record; + const steps = result['steps'] as Array>; + assert.ok(Array.isArray(steps)); + assert.deepEqual(steps[0], { action: 'click' }); + }); + + it('supports multiple dot-notation paths under the same parent', () => { + const obj = { steps: [{ action: 'click', element: 'button', wait: 500 }] }; + const result = maskFields(obj, ['steps.action', 'steps.element']) as Record; + const steps = result['steps'] as Array>; + assert.deepEqual(steps[0], { action: 'click', element: 'button' }); + }); + + it('mixes top-level and dot-notation fields', () => { + const obj = { id: '1', name: 'Test', steps: [{ action: 'click', wait: 500 }] }; + const result = maskFields(obj, ['id', 'steps.action']) as Record; + assert.equal(result['id'], '1'); + const steps = result['steps'] as Array>; + assert.deepEqual(steps[0], { action: 'click' }); + }); + + it('silently ignores unknown dot-notation sub-fields', () => { + const obj = { steps: [{ action: 'click' }] }; + const result = maskFields(obj, ['steps.action', 'steps.ghost']) as Record; + const steps = result['steps'] as Array>; + assert.deepEqual(steps[0], { action: 'click' }); + }); + }); + + describe('array handling', () => { + it('applies masking to every element of a top-level array', () => { + const arr = [ + { name: 'A', type: 'sf', extra: true }, + { name: 'B', type: 'ui', extra: false }, + ]; + const result = maskFields(arr, ['name', 'type']) as Array>; + assert.equal(result.length, 2); + assert.deepEqual(result[0], { name: 'A', type: 'sf' }); + assert.deepEqual(result[1], { name: 'B', type: 'ui' }); + }); + + it('handles empty arrays without error', () => { + const result = maskFields([], ['name']); + assert.deepEqual(result, []); + }); + }); + + describe('edge cases', () => { + it('passes through primitive values unchanged', () => { + assert.equal(maskFields('hello', ['x']), 'hello'); + assert.equal(maskFields(42, ['x']), 42); + assert.equal(maskFields(null, ['x']), null); + }); + + it('handles objects with numeric or boolean values', () => { + const obj = { count: 5, active: true, name: 'Test' }; + const result = maskFields(obj, ['count', 'active']) as Record; + assert.deepEqual(result, { count: 5, active: true }); + }); + + it('handles a field that exists but has a null value', () => { + const obj = { id: '1', extra: null }; + const result = maskFields(obj, ['extra']) as Record; + assert.deepEqual(result, { extra: null }); + }); + }); +}); + +// ── parseFieldsParam ────────────────────────────────────────────────────────── + +describe('parseFieldsParam', () => { + it('returns null when undefined', () => { + assert.equal(parseFieldsParam(undefined), null); + }); + + it('returns null for blank string', () => { + assert.equal(parseFieldsParam(''), null); + assert.equal(parseFieldsParam(' '), null); + }); + + it('trims whitespace around entries', () => { + const result = parseFieldsParam('id , name , status'); + assert.deepEqual(result, ['id', 'name', 'status']); + }); + + it('filters out empty tokens from trailing commas', () => { + const result = parseFieldsParam('id,name,'); + assert.deepEqual(result, ['id', 'name']); + }); + + it('returns a single-item array for one field', () => { + assert.deepEqual(parseFieldsParam('name'), ['name']); + }); + + it('preserves dot notation intact', () => { + const result = parseFieldsParam('connections.name,connections.type'); + assert.deepEqual(result, ['connections.name', 'connections.type']); + }); +}); diff --git a/test/unit/mcp/projectInspect.test.ts b/test/unit/mcp/projectInspect.test.ts new file mode 100644 index 00000000..f6be6349 --- /dev/null +++ b/test/unit/mcp/projectInspect.test.ts @@ -0,0 +1,186 @@ +/* + * Copyright (c) 2024 Provar Limited. + * All rights reserved. + * Licensed under the BSD 3-Clause license. + * For full license text, see LICENSE.md file in the repo root or https://opensource.org/licenses/BSD-3-Clause + */ + +/* eslint-disable camelcase */ +import { strict as assert } from 'node:assert'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { describe, it, beforeEach, afterEach } from 'mocha'; +import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; +import { registerProjectInspect } from '../../../src/mcp/tools/projectInspect.js'; +import type { ServerConfig } from '../../../src/mcp/server.js'; + +// ── Minimal McpServer mock ──────────────────────────────────────────────────── + +type ToolHandler = (args: Record) => unknown; + +class MockMcpServer { + private handlers = new Map(); + + public registerTool(name: string, _config: unknown, handler: ToolHandler): void { + this.handlers.set(name, handler); + } + + public call(name: string, args: Record): ReturnType { + const h = this.handlers.get(name); + if (!h) throw new Error(`Tool not registered: ${name}`); + return h(args); + } +} + +// ── Helpers ─────────────────────────────────────────────────────────────────── + +function parseText(result: unknown): Record { + const r = result as { content: Array<{ type: string; text: string }> }; + return JSON.parse(r.content[0].text) as Record; +} + +function isError(result: unknown): boolean { + return (result as { isError?: boolean }).isError === true; +} + +// ── Test setup ──────────────────────────────────────────────────────────────── + +let tmpDir: string; +let server: MockMcpServer; +let config: ServerConfig; + +beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'inspect-test-')); + server = new MockMcpServer(); + config = { allowedPaths: [tmpDir] }; + registerProjectInspect(server as unknown as McpServer, config); +}); + +afterEach(() => { + fs.rmSync(tmpDir, { recursive: true, force: true }); +}); + +// ── provar_project_inspect — detail param ───────────────────────────────────── + +describe('provar_project_inspect — detail param', () => { + it('standard (default) returns all top-level fields including test_case_files', () => { + const result = server.call('provar_project_inspect', { project_path: tmpDir }); + assert.equal(isError(result), false); + const body = parseText(result); + assert.ok('test_case_files' in body, 'standard should include test_case_files'); + assert.ok('summary' in body, 'standard should include summary'); + assert.ok('requestId' in body, 'standard should include requestId'); + }); + + it('summary retains only requestId, project_path, provar_home, and summary', () => { + const result = server.call('provar_project_inspect', { project_path: tmpDir, detail: 'summary' }); + assert.equal(isError(result), false); + const body = parseText(result); + const keys = Object.keys(body); + assert.ok(keys.includes('requestId'), 'summary must include requestId'); + assert.ok(keys.includes('project_path'), 'summary must include project_path'); + assert.ok(keys.includes('summary'), 'summary must include summary'); + assert.ok(!keys.includes('test_case_files'), 'summary must not include test_case_files'); + assert.ok(!keys.includes('ant_build_files'), 'summary must not include ant_build_files'); + assert.ok(!keys.includes('test_project'), 'summary must not include test_project'); + }); + + it('full returns all fields (same as standard for this tool)', () => { + const resultFull = server.call('provar_project_inspect', { project_path: tmpDir, detail: 'full' }); + const resultStd = server.call('provar_project_inspect', { project_path: tmpDir, detail: 'standard' }); + const full = parseText(resultFull); + const std = parseText(resultStd); + // Both should have the same keys (requestId will differ — compare key sets only) + assert.deepEqual(Object.keys(full).sort(), Object.keys(std).sort()); + }); + + it('omitting detail defaults to standard behaviour', () => { + const withDefault = server.call('provar_project_inspect', { project_path: tmpDir }); + const withStandard = server.call('provar_project_inspect', { project_path: tmpDir, detail: 'standard' }); + const a = Object.keys(parseText(withDefault)).sort(); + const b = Object.keys(parseText(withStandard)).sort(); + assert.deepEqual(a, b, 'omitting detail should match explicit standard'); + }); +}); + +// ── provar_project_inspect — fields param ───────────────────────────────────── + +describe('provar_project_inspect — fields param', () => { + it('retains only specified top-level keys', () => { + const result = server.call('provar_project_inspect', { + project_path: tmpDir, + fields: 'test_case_files,summary', + }); + const body = parseText(result); + assert.ok('test_case_files' in body); + assert.ok('summary' in body); + assert.ok(!('requestId' in body), 'requestId should be masked out'); + assert.ok(!('test_project' in body), 'test_project should be masked out'); + }); + + it('omitting fields returns full response', () => { + const result = server.call('provar_project_inspect', { project_path: tmpDir }); + const body = parseText(result); + assert.ok('requestId' in body); + assert.ok('summary' in body); + }); + + it('silently ignores unknown field names', () => { + const result = server.call('provar_project_inspect', { + project_path: tmpDir, + fields: 'summary,ghost_field', + }); + assert.equal(isError(result), false); + const body = parseText(result); + assert.ok('summary' in body); + assert.ok(!('ghost_field' in body)); + }); + + it('supports dot notation for nested field selection', () => { + const result = server.call('provar_project_inspect', { + project_path: tmpDir, + fields: 'summary.test_case_count,summary.coverage_percent', + }); + assert.equal(isError(result), false); + const body = parseText(result); + const summary = body['summary'] as Record; + assert.ok('test_case_count' in summary, 'test_case_count should be retained'); + assert.ok('coverage_percent' in summary, 'coverage_percent should be retained'); + assert.ok(!('provardx_properties_count' in summary), 'unspecified summary keys should be dropped'); + }); + + it('composes detail=summary with fields for fine-grained trimming', () => { + const result = server.call('provar_project_inspect', { + project_path: tmpDir, + detail: 'summary', + fields: 'summary', + }); + const body = parseText(result); + assert.ok('summary' in body); + assert.ok(!('requestId' in body), 'fields filter should further narrow after detail'); + }); +}); + +// ── provar_project_inspect — path-policy errors (unchanged) ─────────────────── + +describe('provar_project_inspect — path policy', () => { + it('returns PATH_NOT_ALLOWED when project_path is outside allowed paths', () => { + const strictServer = new MockMcpServer(); + registerProjectInspect(strictServer as unknown as McpServer, { allowedPaths: [tmpDir] }); + const result = strictServer.call('provar_project_inspect', { + project_path: path.join(os.tmpdir(), 'some-other-project'), + }); + assert.equal(isError(result), true); + const code = parseText(result)['error_code'] as string; + assert.ok(code === 'PATH_NOT_ALLOWED' || code === 'PATH_TRAVERSAL', `Unexpected code: ${code}`); + }); + + it('returns PATH_NOT_FOUND when project path does not exist', () => { + const result = server.call('provar_project_inspect', { + project_path: path.join(tmpDir, 'nonexistent-dir'), + }); + assert.equal(isError(result), true); + assert.equal(parseText(result)['error_code'], 'PATH_NOT_FOUND'); + }); +}); diff --git a/test/unit/mcp/qualityHubTools.test.ts b/test/unit/mcp/qualityHubTools.test.ts index 11274d2f..edbe4801 100644 --- a/test/unit/mcp/qualityHubTools.test.ts +++ b/test/unit/mcp/qualityHubTools.test.ts @@ -163,6 +163,58 @@ describe('qualityHubTools', () => { }); }); + // ── provar_qualityhub_display — detail + fields ──────────────────────────── + + describe('provar_qualityhub_display — detail param', () => { + it('standard (default) returns requestId, exitCode, stdout, stderr', () => { + spawnStub.returns(makeSpawnResult('display output', '', 0)); + const result = server.call('provar_qualityhub_display', { flags: [] }); + const body = parseBody(result); + assert.ok('requestId' in body); + assert.ok('exitCode' in body); + assert.ok('stdout' in body); + assert.ok('stderr' in body); + }); + + it('summary returns only requestId and exitCode', () => { + spawnStub.returns(makeSpawnResult('display output', '', 0)); + const result = server.call('provar_qualityhub_display', { flags: [], detail: 'summary' }); + const body = parseBody(result); + assert.ok('requestId' in body, 'summary must include requestId'); + assert.ok('exitCode' in body, 'summary must include exitCode'); + assert.ok(!('stdout' in body), 'summary must not include stdout'); + assert.ok(!('stderr' in body), 'summary must not include stderr'); + }); + + it('full returns same fields as standard', () => { + spawnStub.returns(makeSpawnResult('display output', '', 0)); + const full = parseBody(server.call('provar_qualityhub_display', { flags: [], detail: 'full' })); + const std = parseBody(server.call('provar_qualityhub_display', { flags: [], detail: 'standard' })); + assert.deepEqual(Object.keys(full).sort(), Object.keys(std).sort()); + }); + }); + + describe('provar_qualityhub_display — fields param', () => { + it('retains only specified keys', () => { + spawnStub.returns(makeSpawnResult('display output', '', 0)); + const result = server.call('provar_qualityhub_display', { flags: [], fields: 'exitCode,stdout' }); + const body = parseBody(result); + assert.ok('exitCode' in body); + assert.ok('stdout' in body); + assert.ok(!('requestId' in body)); + assert.ok(!('stderr' in body)); + }); + + it('silently ignores unknown fields', () => { + spawnStub.returns(makeSpawnResult('ok', '', 0)); + const result = server.call('provar_qualityhub_display', { flags: [], fields: 'exitCode,ghost' }); + assert.equal(isError(result), false); + const body = parseBody(result); + assert.ok('exitCode' in body); + assert.ok(!('ghost' in body)); + }); + }); + // ── provar_qualityhub_testrun ─────────────────────────────────────────────── describe('provar_qualityhub_testrun', () => { @@ -396,6 +448,62 @@ describe('qualityHubTools', () => { }); }); + // ── provar_qualityhub_testcase_retrieve — detail + fields ───────────────────── + + describe('provar_qualityhub_testcase_retrieve — detail param', () => { + it('standard (default) returns requestId, exitCode, stdout, stderr', () => { + spawnStub.returns(makeSpawnResult('[]', '', 0)); + const result = server.call('provar_qualityhub_testcase_retrieve', { target_org: 'myorg', flags: [] }); + const body = parseBody(result); + assert.ok('requestId' in body); + assert.ok('exitCode' in body); + assert.ok('stdout' in body); + assert.ok('stderr' in body); + }); + + it('summary returns only requestId and exitCode', () => { + spawnStub.returns(makeSpawnResult('[]', '', 0)); + const result = server.call('provar_qualityhub_testcase_retrieve', { + target_org: 'myorg', + flags: [], + detail: 'summary', + }); + const body = parseBody(result); + assert.ok('requestId' in body, 'summary must include requestId'); + assert.ok('exitCode' in body, 'summary must include exitCode'); + assert.ok(!('stdout' in body), 'summary must not include stdout'); + assert.ok(!('stderr' in body), 'summary must not include stderr'); + }); + }); + + describe('provar_qualityhub_testcase_retrieve — fields param', () => { + it('retains only specified keys', () => { + spawnStub.returns(makeSpawnResult('[]', '', 0)); + const result = server.call('provar_qualityhub_testcase_retrieve', { + target_org: 'myorg', + flags: [], + fields: 'exitCode,stdout', + }); + const body = parseBody(result); + assert.ok('exitCode' in body); + assert.ok('stdout' in body); + assert.ok(!('requestId' in body)); + }); + + it('silently ignores unknown field names', () => { + spawnStub.returns(makeSpawnResult('[]', '', 0)); + const result = server.call('provar_qualityhub_testcase_retrieve', { + target_org: 'myorg', + flags: [], + fields: 'exitCode,nope', + }); + assert.equal(isError(result), false); + const body = parseBody(result); + assert.ok('exitCode' in body); + assert.ok(!('nope' in body)); + }); + }); + // ── sf_path threading ───────────────────────────────────────────────────────── describe('sf_path threading', () => { From 8e850cae90f8cecef10befd8e61cd62db0a59b8f Mon Sep 17 00:00:00 2001 From: Michael Dailey Date: Wed, 13 May 2026 15:06:28 -0500 Subject: [PATCH 2/2] PDX-477: docs(mcp): document fields and detail params for PR #167 field masking tools RCA: CLAUDE.md requires docs updates for every PR that adds or modifies tool parameters; PR #167 added fields/detail to 4 tools without updating docs/mcp.md Fix: Added fields and detail input param rows for provar_project_inspect, provar_connection_list, provar_qualityhub_display, and provar_qualityhub_testcase_retrieve; updated output descriptions to note detail/fields behaviour Co-Authored-By: Claude Sonnet 4.6 --- docs/mcp.md | 40 +++++++++++++++++++------------- src/mcp/tools/connectionTools.ts | 2 +- src/mcp/tools/projectInspect.ts | 2 +- 3 files changed, 26 insertions(+), 18 deletions(-) diff --git a/docs/mcp.md b/docs/mcp.md index 50ad161b..b5d26408 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -561,14 +561,17 @@ Inspects a Provar project folder and returns a structured inventory of all key p **Input** -| Parameter | Type | Required | Description | -| -------------- | ------ | -------- | ---------------------------------------- | -| `project_path` | string | yes | Absolute path to the Provar project root | +| Parameter | Type | Required | Description | +| -------------- | --------------------------------- | -------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `project_path` | string | yes | Absolute path to the Provar project root | +| `detail` | `summary` \| `standard` \| `full` | no | Response verbosity. `"summary"` returns only `requestId`, `project_path`, `provar_home`, and `summary`. `"standard"` (default) returns full inventory. `"full"` is identical to `"standard"` for this tool. | +| `fields` | string | no | Comma-separated top-level keys to retain (e.g. `"test_case_files,summary"`). Supports dot notation for nested filtering (e.g. `"test_project.connections"`). Unknown field names are silently ignored. Applied after the `detail` filter. | **Output** — JSON object containing: | Field | Description | | ----------------------------- | ------------------------------------------------------------------------------------------------------------------------------------ | +| `requestId` | Unique identifier for this request (always present, including in `detail="summary"` responses) | | `provar_home` | The Provar installation path, or `null` if not found | | `provar_home_source` | Where the value came from: `"PROVAR_HOME environment variable"`, `"provardx-properties.json ()"`, or `"ANT build file ()"` | | `provardx_properties_files` | Relative paths to any `provardx-properties.json` files found (ProvarDX CLI run configs) | @@ -607,9 +610,10 @@ Lists all connections and named environments defined in the project's `.testproj **Input** -| Parameter | Type | Required | Description | -| -------------- | ------ | -------- | ----------------------------------------------------------------- | -| `project_path` | string | yes | Absolute path to the Provar project root (within `allowed-paths`) | +| Parameter | Type | Required | Description | +| -------------- | ------ | -------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `project_path` | string | yes | Absolute path to the Provar project root (within `allowed-paths`) | +| `fields` | string | no | Comma-separated response keys to retain (e.g. `"connections,summary"`). Supports dot notation (e.g. `"connections.name,connections.type"`). Unknown fields are silently ignored. | **Output** @@ -1227,12 +1231,14 @@ Displays information about the currently connected Quality Hub org. Invokes `sf **Input** -| Parameter | Type | Required | Description | -| ------------ | -------- | -------- | ------------------------------------------ | -| `target_org` | string | no | SF CLI org alias (uses default if omitted) | -| `flags` | string[] | no | Additional raw CLI flags | +| Parameter | Type | Required | Description | +| ------------ | --------------------------------- | -------- | -------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `target_org` | string | no | SF CLI org alias (uses default if omitted) | +| `flags` | string[] | no | Additional raw CLI flags | +| `detail` | `summary` \| `standard` \| `full` | no | Response verbosity. `"summary"` returns only `requestId` and `exitCode`. `"standard"` (default) returns `requestId`, `exitCode`, `stdout`, and `stderr`. | +| `fields` | string | no | Comma-separated response keys to retain (e.g. `"exitCode,stdout"`). Unknown fields are silently ignored. Applied after the `detail` filter. | -**Output** — `{ requestId, exitCode, stdout, stderr }` +**Output** — `{ requestId, exitCode, stdout, stderr }`. Use `detail="summary"` to reduce to `{ requestId, exitCode }` only, or pass `fields` to select specific keys. --- @@ -1297,12 +1303,14 @@ Retrieves test cases from Quality Hub by user story or metadata component. Invok **Input** -| Parameter | Type | Required | Description | -| ------------ | -------- | -------- | ------------------------------------------------------------------------------------ | -| `target_org` | string | yes | SF CLI org alias or username | -| `flags` | string[] | no | Additional raw CLI flags (e.g. `["--issues", "US-123", "--test-project", "MyProj"]`) | +| Parameter | Type | Required | Description | +| ------------ | --------------------------------- | -------- | -------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `target_org` | string | yes | SF CLI org alias or username | +| `flags` | string[] | no | Additional raw CLI flags (e.g. `["--issues", "US-123", "--test-project", "MyProj"]`) | +| `detail` | `summary` \| `standard` \| `full` | no | Response verbosity. `"summary"` returns only `requestId` and `exitCode`. `"standard"` (default) returns `requestId`, `exitCode`, `stdout`, and `stderr`. | +| `fields` | string | no | Comma-separated response keys to retain (e.g. `"exitCode,stdout"`). Unknown fields are silently ignored. Applied after the `detail` filter. | -**Output** — `{ requestId, exitCode, stdout, stderr }` +**Output** — `{ requestId, exitCode, stdout, stderr }`. Use `detail="summary"` to reduce to `{ requestId, exitCode }` only, or pass `fields` to select specific keys. **Error codes:** `QH_RETRIEVE_FAILED`, `SF_NOT_FOUND` diff --git a/src/mcp/tools/connectionTools.ts b/src/mcp/tools/connectionTools.ts index 255394ce..f1d1d14e 100644 --- a/src/mcp/tools/connectionTools.ts +++ b/src/mcp/tools/connectionTools.ts @@ -15,8 +15,8 @@ import type { ServerConfig } from '../server.js'; import { assertPathAllowed, PathPolicyError } from '../security/pathPolicy.js'; import { makeError, makeRequestId } from '../schemas/common.js'; import { log } from '../logging/logger.js'; -import { desc } from './descHelper.js'; import { maskFields, parseFieldsParam } from '../utils/fieldMask.js'; +import { desc } from './descHelper.js'; // ── Types ───────────────────────────────────────────────────────────────────── diff --git a/src/mcp/tools/projectInspect.ts b/src/mcp/tools/projectInspect.ts index f6b700ea..b26e8c14 100644 --- a/src/mcp/tools/projectInspect.ts +++ b/src/mcp/tools/projectInspect.ts @@ -14,9 +14,9 @@ import type { ServerConfig } from '../server.js'; import { assertPathAllowed, PathPolicyError } from '../security/pathPolicy.js'; import { makeError, makeRequestId } from '../schemas/common.js'; import { log } from '../logging/logger.js'; -import { desc } from './descHelper.js'; import { applyDetailLevel, type DetailLevel } from '../utils/detailLevel.js'; import { maskFields, parseFieldsParam } from '../utils/fieldMask.js'; +import { desc } from './descHelper.js'; const INSPECT_SUMMARY_FIELDS = ['requestId', 'project_path', 'provar_home', 'summary'];