Skip to content

fix(mcp): resolve sf CLI for quality hub and defect tools (B1/B2a) #151

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mrdailey99 merged 3 commits into from
May 7, 2026
Merged

fix(mcp): resolve sf CLI for quality hub and defect tools (B1/B2a) #151

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7c5a97e
PDX-0: fix(mcp): resolve sf CLI for quality hub and defect tools (B1/…
mrdailey99 May 7, 2026
71e42e3
PDX-0: test(mcp): add direct unit tests for sfSpawn.ts
mrdailey99 May 7, 2026
bc9e11a
PDX-0: fix(mcp): address PR #151 review comments
mrdailey99 May 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
183 changes: 3 additions & 180 deletions src/mcp/tools/automationTools.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,187 +16,10 @@ import { log } from '../logging/logger.js';
import type { ServerConfig } from '../server.js';
import { assertPathAllowed, PathPolicyError } from '../security/pathPolicy.js';
import { parseJUnitResults } from './antTools.js';
import { sfSpawnHelper, SfNotFoundError } from './sfSpawn.js';
import { runSfCommand } from './sfSpawn.js';

// ── SF CLI discovery ──────────────────────────────────────────────────────────

/**
* Returns candidate sf CLI paths in common npm/nvm/volta install locations.
* Used as a fallback when `sf` is not in PATH.
*/
export function getSfCommonPaths(): string[] {
const home = os.homedir();
if (process.platform === 'win32') {
const appData = process.env['APPDATA'] ?? path.join(home, 'AppData', 'Roaming');
return [
path.join(appData, 'npm', 'sf.cmd'),
path.join('C:', 'Program Files', 'nodejs', 'sf.cmd'),
path.join('C:', 'Program Files (x86)', 'nodejs', 'sf.cmd'),
];
}
const candidates = [
'/usr/local/bin/sf',
path.join(home, '.npm-global', 'bin', 'sf'),
path.join(home, '.local', 'bin', 'sf'),
path.join(home, '.volta', 'bin', 'sf'),
];
// nvm — scan the three most-recently installed Node versions
const nvmBinDir = path.join(process.env['NVM_DIR'] ?? path.join(home, '.nvm'), 'versions', 'node');
if (fs.existsSync(nvmBinDir)) {
try {
for (const v of fs.readdirSync(nvmBinDir).sort().reverse().slice(0, 3)) {
candidates.push(path.join(nvmBinDir, v, 'bin', 'sf'));
}
} catch {
/* skip */
}
}
return candidates;
}

// ── Shared spawn helper ───────────────────────────────────────────────────────

const MAX_BUFFER = 50 * 1024 * 1024; // 50 MB — prevents ENOBUFS on verbose Provar runs

interface SpawnResult {
stdout: string;
stderr: string;
exitCode: number;
}

// Proactively resolve the sf executable path once on first use and cache it.
// This ensures sf is always found even when ENOENT is masked by other errors (e.g. ENOBUFS).
let cachedSfPath: string | null | undefined; // undefined = not yet probed

/**
* Exposed for testing only — pre-seeds the cached sf executable path, bypassing the probe spawn.
* Pass `undefined` to reset the cache so the next call triggers a fresh probe.
*/
export function setSfPathCacheForTesting(value: string | null | undefined): void {
cachedSfPath = value;
}

// Platform override used in tests so Windows-specific shell logic can be exercised on any OS.
let sfPlatformOverride: NodeJS.Platform | undefined;
/** Exposed for testing only — overrides process.platform for needsWindowsShell decisions. */
export function setSfPlatformForTesting(platform: NodeJS.Platform | undefined): void {
sfPlatformOverride = platform;
}

/**
* Returns true when spawning `executable` requires the Windows shell.
* On Windows, `.cmd` and `.bat` batch scripts cannot be executed directly by
* Node's spawnSync — they must be invoked through cmd.exe (i.e. shell: true).
* The bare name "sf" also needs this treatment on Windows because the file on
* disk is actually "sf.cmd" and Node won't auto-append the extension.
*
* The `platform` parameter defaults to `process.platform` and is exposed for
* unit testing so tests can verify both Windows and non-Windows behaviour
* without having to run on the corresponding OS.
*/
export function needsWindowsShell(executable: string, platform = process.platform): boolean {
if (platform !== 'win32') return false;
const lower = executable.toLowerCase();
return lower.endsWith('.cmd') || lower.endsWith('.bat') || !path.extname(lower);
}

function resolveSfExecutable(): string | null {
if (cachedSfPath !== undefined) return cachedSfPath;
const platform = sfPlatformOverride ?? process.platform;

// Two-phase probe avoids false-positives on Windows with shell:true.
// When shell:true is used, cmd.exe spawns successfully even when `sf` is
// missing — it exits non-zero with "not recognised" in stderr but sets no
// probe.error. Trying shell:false first catches both cases correctly.
//
// First attempt: shell:false (works on Linux/macOS; gives ENOENT on Windows if
// sf.cmd is on PATH but requires the shell).
const probe = sfSpawnHelper.spawnSync('sf', ['--version'], {
encoding: 'utf-8',
shell: false,
maxBuffer: 1024 * 1024,
});
if (!probe.error && probe.status === 0) {
cachedSfPath = 'sf';
return cachedSfPath;
}

// Windows fallback: retry with shell:true when the plain probe failed
// with ENOENT — meaning sf.cmd exists on PATH but can't run without the shell.
if (platform === 'win32' && (probe.error as NodeJS.ErrnoException | undefined)?.code === 'ENOENT') {
const probeShell = sfSpawnHelper.spawnSync('sf', ['--version'], {
encoding: 'utf-8',
shell: true,
maxBuffer: 1024 * 1024,
});
if (!probeShell.error && probeShell.status === 0) {
cachedSfPath = 'sf';
return cachedSfPath;
}
}

// Fall back to common install locations
for (const candidate of getSfCommonPaths()) {
if (fs.existsSync(candidate)) {
cachedSfPath = candidate;
return cachedSfPath;
}
}
cachedSfPath = null;
return null;
}

/**
* Reject shell metacharacters in an sf_path that will be executed via shell:true.
* On Windows, cmd.exe interprets & | ; < > ` ' " and newlines as shell syntax.
* A valid filesystem path should never contain these characters.
*/
function assertShellSafePath(sfPath: string): void {
if (/[&|;<>`'"\n\r]/.test(sfPath)) {
throw Object.assign(
new Error(
'sf_path contains characters that are unsafe for shell execution on Windows ' +
'(& | ; < > ` \' " or line-breaks). Provide an absolute filesystem path to the sf executable.'
),
{ code: 'INVALID_SF_PATH' }
);
}
}

function runSfCommand(args: string[], sfPath?: string): SpawnResult {
// Use explicit path if provided; otherwise use cached probe result
const executable = sfPath ?? resolveSfExecutable();
if (!executable) throw new SfNotFoundError();

const platform = sfPlatformOverride ?? process.platform;
const useShell = needsWindowsShell(executable, platform);

// Guard against injection when shell:true is used with a user-supplied path.
// Common install locations returned by resolveSfExecutable() are safe by construction.
if (useShell && sfPath) {
assertShellSafePath(sfPath);
}

const result = sfSpawnHelper.spawnSync(executable, args, {
encoding: 'utf-8',
shell: useShell,
maxBuffer: MAX_BUFFER,
});

if (result.error) {
const err = result.error as NodeJS.ErrnoException;
if (err.code === 'ENOENT') {
throw new SfNotFoundError(sfPath);
}
throw result.error;
}

return {
stdout: result.stdout ?? '',
stderr: result.stderr ?? '',
exitCode: result.status ?? 1,
};
}
// Re-export sf resolution helpers so existing test imports from automationTools continue to work
export { getSfCommonPaths, needsWindowsShell, setSfPathCacheForTesting, setSfPlatformForTesting } from './sfSpawn.js';

function handleSpawnError(
err: unknown,
Expand Down
70 changes: 35 additions & 35 deletions src/mcp/tools/defectTools.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ interface FailureContext {

// ── SF CLI helpers ─────────────────────────────────────────────────────────────

function runSfArgs(args: string[]): { stdout: string; stderr: string; exitCode: number } {
const { stdout, stderr, exitCode } = runSfCommand(args);
function runSfArgs(args: string[], sfPath?: string): { stdout: string; stderr: string; exitCode: number } {
const { stdout, stderr, exitCode } = runSfCommand(args, sfPath);
return { stdout, stderr, exitCode };
}

Expand All @@ -57,35 +57,22 @@ function formatSfCommandError(action: string, exitCode: number, stderr: string,
: `${action} failed with exit code ${exitCode}`;
}

function runQuery(soql: string, targetOrg: string): SfQueryResponse {
const { stdout, stderr, exitCode } = runSfArgs([
'data',
'query',
'--query',
soql,
'--target-org',
targetOrg,
'--json',
]);
function runQuery(soql: string, targetOrg: string, sfPath?: string): SfQueryResponse {
const { stdout, stderr, exitCode } = runSfArgs(
['data', 'query', '--query', soql, '--target-org', targetOrg, '--json'],
sfPath
);
if (exitCode !== 0) {
throw new Error(formatSfCommandError('Salesforce query', exitCode, stderr, stdout));
}
return JSON.parse(stdout) as SfQueryResponse;
}

function createRecord(sobject: string, values: string, targetOrg: string): string {
const { stdout, stderr, exitCode } = runSfArgs([
'data',
'create',
'record',
'--sobject',
sobject,
'--values',
values,
'--target-org',
targetOrg,
'--json',
]);
function createRecord(sobject: string, values: string, targetOrg: string, sfPath?: string): string {
const { stdout, stderr, exitCode } = runSfArgs(
['data', 'create', 'record', '--sobject', sobject, '--values', values, '--target-org', targetOrg, '--json'],
sfPath
);
if (exitCode !== 0) {
throw new Error(formatSfCommandError(`Failed to create ${sobject}`, exitCode, stderr, stdout));
}
Expand Down Expand Up @@ -115,12 +102,14 @@ export interface DefectCreateResult {
export function createDefectsForRun(
runId: string,
targetOrg: string,
failedTestFilter?: string[]
failedTestFilter?: string[],
sfPath?: string
): { created: DefectCreateResult[]; skipped: number; message: string } {
// Step 1: resolve job record ID from tracking ID
const jobQuery = runQuery(
`SELECT Id FROM provar__Test_Plan_Schedule_Job__c WHERE provar__Tracking_Id__c = '${soqlEscape(runId)}'`,
targetOrg
targetOrg,
sfPath
);
if (jobQuery.result.totalSize === 0) {
throw new Error(`No Test_Plan_Schedule_Job__c found with Tracking_Id__c = '${runId}'`);
Expand All @@ -133,7 +122,8 @@ export function createDefectsForRun(
FROM provar__Test_Cycle__c
WHERE provar__Test_Plan_Schedule_Job__c = '${soqlEscape(jobId)}'
LIMIT 1`,
targetOrg
targetOrg,
sfPath
);
const cycle = cycleQuery.result.records[0] ?? {};
const browser = safeText(cycle['provar__Web_Browser__c'], 100);
Expand All @@ -151,7 +141,8 @@ export function createDefectsForRun(
FROM provar__Test_Execution__c
WHERE provar__Test_Cycle__c = '${soqlEscape(cycleId)}'
AND provar__Status__c = 'Failed'`,
targetOrg
targetOrg,
sfPath
);

if (execQuery.result.totalSize === 0) {
Expand Down Expand Up @@ -185,7 +176,8 @@ export function createDefectsForRun(
AND provar__Result__c = 'Fail'
ORDER BY provar__Sequence_No__c ASC
LIMIT 1`,
targetOrg
targetOrg,
sfPath
);

const step = stepQuery.result.records[0] ?? {};
Expand Down Expand Up @@ -227,23 +219,23 @@ export function createDefectsForRun(
`provar__Description__c="${safeText(descLines, 2000)}" ` +
'provar__Status__c="Open"';

const defectId = createRecord('provar__Defect__c', defectValues, targetOrg);
const defectId = createRecord('provar__Defect__c', defectValues, targetOrg, sfPath);

// Step 5b: link TC → Defect
const tcDefectValues =
`provar__Defect__c="${defectId}" ` +
`provar__Test_Case__c="${testCaseId}"` +
(stepId ? ` provar__Test_Step__c="${stepId}"` : '');

const tcDefectId = createRecord('provar__Test_Case_Defect__c', tcDefectValues, targetOrg);
const tcDefectId = createRecord('provar__Test_Case_Defect__c', tcDefectValues, targetOrg, sfPath);

// Step 5c: link Execution → Defect (with step execution if available)
const execDefectValues =
`provar__Defect__c="${defectId}" ` +
`provar__Test_Execution__c="${executionId}"` +
(stepExecutionId ? ` provar__Test_Step_Execution__c="${stepExecutionId}"` : '');

const execDefectId = createRecord('provar__Test_Execution_Defect__c', execDefectValues, targetOrg);
const execDefectId = createRecord('provar__Test_Execution_Defect__c', execDefectValues, targetOrg, sfPath);

log('info', 'defect created', { defectId, tcDefectId, execDefectId, executionId });

Expand Down Expand Up @@ -281,14 +273,22 @@ export function registerQualityHubDefectCreate(server: McpServer): void {
.describe(
'Optional filter — list of Test_Case__c record ID substrings to restrict defect creation to specific failures'
),
sf_path: z
.string()
.optional()
.describe(
'Path to the sf CLI executable when not in PATH ' +
'(e.g. "C:\\\\Program Files\\\\sf\\\\bin\\\\sf.cmd" for the Windows standalone installer). ' +
'Leave unset to use auto-discovery.'
),
},
},
({ run_id, target_org, failed_tests }) => {
({ run_id, target_org, failed_tests, sf_path }) => {
const requestId = makeRequestId();
log('info', 'provar_qualityhub_defect_create', { requestId, run_id, target_org });

try {
const result = createDefectsForRun(run_id, target_org, failed_tests);
const result = createDefectsForRun(run_id, target_org, failed_tests, sf_path);
const response = { requestId, ...result };
return {
content: [{ type: 'text' as const, text: JSON.stringify(response) }],
Expand Down
Loading
Loading