diff --git a/dist/fix.js b/dist/fix.js index 7bd985a..c3918c4 100644 --- a/dist/fix.js +++ b/dist/fix.js @@ -94,11 +94,22 @@ export async function planPinFixes(root, canonical) { if (server.canonicalIdentity === canonicalServer.canonicalIdentity) { continue; } + // Reproduce the canonical surface's *raw* shape, not the joined + // display command. Splitting `command` on spaces dropped every + // argument whenever the canonical config used a single inline + // command string ("npx -y pkg@1.2.3"), silently rewriting the + // target down to just "npx". `rawCommand` is the verbatim + // `command` value; `args` is the verbatim args array (if any). + if (canonicalServer.rawCommand === undefined && canonicalServer.args === undefined) { + // Remote (url-only) server or otherwise nothing launchable to + // copy — fix pin only aligns command/args. + continue; + } fixes.push({ file: server.file, surface: server.surfaceId, server: server.name, - canonicalCommand: canonicalServer.command.split(' ')[0], + canonicalCommand: canonicalServer.rawCommand, canonicalArgs: canonicalServer.args, description: `Align "${server.name}" command/args to ${canonical} in ${server.file}` }); @@ -215,7 +226,7 @@ class JsonLineEditor { return this.lines.join('\n'); } alignCommandAndArgs(serverName, canonicalCommand, canonicalArgs) { - if (!canonicalCommand && !canonicalArgs) { + if (canonicalCommand === undefined && canonicalArgs === undefined) { return { ok: false, reason: 'canonical surface has neither command nor args to copy' }; } const original = this.text(); @@ -256,15 +267,28 @@ class JsonLineEditor { depth -= 1; } } + // Shape-mismatch guard: the canonical surface folds its arguments + // into the command string (no separate args array) but the target + // keeps a separate args array. Rewriting the command while leaving + // the target's args untouched would yield a duplicated, corrupt + // launch line, so refuse rather than "helpfully" break the config. + if (canonicalArgs === undefined && argsLine !== undefined) { + return { + ok: false, + reason: 'canonical folds args into the command string but the target keeps a separate "args" array; align manually to avoid a corrupt launch' + }; + } + // Validate every edit is applicable BEFORE mutating anything, so a + // partially-applicable fix never leaves the file half-rewritten. + const commandRegex = /("command"\s*:\s*)"[^"]*"/; + const argsRegex = /("args"\s*:\s*)\[[^\]]*\]/; if (canonicalCommand !== undefined) { if (commandLine === undefined) { return { ok: false, reason: 'target server has no "command" field; insertion not supported in v1' }; } - const commandRegex = /("command"\s*:\s*)"[^"]*"/; if (!commandRegex.test(this.lines[commandLine])) { return { ok: false, reason: 'could not locate command value token on its line' }; } - this.lines[commandLine] = this.lines[commandLine].replace(commandRegex, `$1"${escapeJsonString(canonicalCommand)}"`); } if (canonicalArgs !== undefined) { if (argsLine === undefined) { @@ -273,10 +297,15 @@ class JsonLineEditor { if (argsClosingLine !== argsLine) { return { ok: false, reason: 'multi-line "args" array; not supported in v1' }; } - const argsRegex = /("args"\s*:\s*)\[[^\]]*\]/; if (!argsRegex.test(this.lines[argsLine])) { return { ok: false, reason: 'could not locate args array token on its line' }; } + } + // All checks passed — apply the edits. + if (canonicalCommand !== undefined && commandLine !== undefined) { + this.lines[commandLine] = this.lines[commandLine].replace(commandRegex, `$1"${escapeJsonString(canonicalCommand)}"`); + } + if (canonicalArgs !== undefined && argsLine !== undefined) { const renderedArgs = canonicalArgs.map((arg) => `"${escapeJsonString(arg)}"`).join(', '); this.lines[argsLine] = this.lines[argsLine].replace(argsRegex, `$1[${renderedArgs}]`); } diff --git a/dist/parsers/mcp.js b/dist/parsers/mcp.js index ec972bb..208fa64 100644 --- a/dist/parsers/mcp.js +++ b/dist/parsers/mcp.js @@ -66,6 +66,7 @@ async function readMcpServers(root, config) { servers.push({ name, command, + rawCommand: raw.command, canonicalIdentity: normalizeMcpCommand({ command: raw.command, args: raw.args, diff --git a/src/fix.ts b/src/fix.ts index a749c6e..73d0e36 100644 --- a/src/fix.ts +++ b/src/fix.ts @@ -157,11 +157,22 @@ export async function planPinFixes(root: string, canonical: SurfaceId): Promise< if (server.canonicalIdentity === canonicalServer.canonicalIdentity) { continue; } + // Reproduce the canonical surface's *raw* shape, not the joined + // display command. Splitting `command` on spaces dropped every + // argument whenever the canonical config used a single inline + // command string ("npx -y pkg@1.2.3"), silently rewriting the + // target down to just "npx". `rawCommand` is the verbatim + // `command` value; `args` is the verbatim args array (if any). + if (canonicalServer.rawCommand === undefined && canonicalServer.args === undefined) { + // Remote (url-only) server or otherwise nothing launchable to + // copy — fix pin only aligns command/args. + continue; + } fixes.push({ file: server.file, surface: server.surfaceId, server: server.name, - canonicalCommand: canonicalServer.command.split(' ')[0], + canonicalCommand: canonicalServer.rawCommand, canonicalArgs: canonicalServer.args, description: `Align "${server.name}" command/args to ${canonical} in ${server.file}` }); @@ -300,7 +311,7 @@ class JsonLineEditor { canonicalCommand: string | undefined, canonicalArgs: string[] | undefined ): { ok: true } | { ok: false; reason: string } { - if (!canonicalCommand && !canonicalArgs) { + if (canonicalCommand === undefined && canonicalArgs === undefined) { return { ok: false, reason: 'canonical surface has neither command nor args to copy' }; } @@ -343,17 +354,30 @@ class JsonLineEditor { } } + // Shape-mismatch guard: the canonical surface folds its arguments + // into the command string (no separate args array) but the target + // keeps a separate args array. Rewriting the command while leaving + // the target's args untouched would yield a duplicated, corrupt + // launch line, so refuse rather than "helpfully" break the config. + if (canonicalArgs === undefined && argsLine !== undefined) { + return { + ok: false, + reason: 'canonical folds args into the command string but the target keeps a separate "args" array; align manually to avoid a corrupt launch' + }; + } + + // Validate every edit is applicable BEFORE mutating anything, so a + // partially-applicable fix never leaves the file half-rewritten. + const commandRegex = /("command"\s*:\s*)"[^"]*"/; + const argsRegex = /("args"\s*:\s*)\[[^\]]*\]/; if (canonicalCommand !== undefined) { if (commandLine === undefined) { return { ok: false, reason: 'target server has no "command" field; insertion not supported in v1' }; } - const commandRegex = /("command"\s*:\s*)"[^"]*"/; if (!commandRegex.test(this.lines[commandLine])) { return { ok: false, reason: 'could not locate command value token on its line' }; } - this.lines[commandLine] = this.lines[commandLine].replace(commandRegex, `$1"${escapeJsonString(canonicalCommand)}"`); } - if (canonicalArgs !== undefined) { if (argsLine === undefined) { return { ok: false, reason: 'target server has no "args" field; insertion not supported in v1' }; @@ -361,10 +385,16 @@ class JsonLineEditor { if (argsClosingLine !== argsLine) { return { ok: false, reason: 'multi-line "args" array; not supported in v1' }; } - const argsRegex = /("args"\s*:\s*)\[[^\]]*\]/; if (!argsRegex.test(this.lines[argsLine])) { return { ok: false, reason: 'could not locate args array token on its line' }; } + } + + // All checks passed — apply the edits. + if (canonicalCommand !== undefined && commandLine !== undefined) { + this.lines[commandLine] = this.lines[commandLine].replace(commandRegex, `$1"${escapeJsonString(canonicalCommand)}"`); + } + if (canonicalArgs !== undefined && argsLine !== undefined) { const renderedArgs = canonicalArgs.map((arg) => `"${escapeJsonString(arg)}"`).join(', '); this.lines[argsLine] = this.lines[argsLine].replace(argsRegex, `$1[${renderedArgs}]`); } diff --git a/src/parsers/mcp.ts b/src/parsers/mcp.ts index 1d27d11..9c8866d 100644 --- a/src/parsers/mcp.ts +++ b/src/parsers/mcp.ts @@ -105,6 +105,7 @@ async function readMcpServers( servers.push({ name, command, + rawCommand: raw.command, canonicalIdentity: normalizeMcpCommand({ command: raw.command, args: raw.args, diff --git a/src/types.ts b/src/types.ts index 9544675..28ab759 100644 --- a/src/types.ts +++ b/src/types.ts @@ -44,6 +44,16 @@ export interface McpServer { name: string; /** Human-readable launch string. Used in messages/matrix rows only. */ command: string; + /** + * The `command` value exactly as authored in the config, before `args` + * / `url` were joined into the display `command`. Preserved so `fix pin` + * can faithfully reproduce the canonical raw shape — a single inline + * command string (`"npx -y pkg@1.2.3"`) versus a bare command with a + * separate `args` array — instead of guessing by splitting the joined + * display string (which dropped every argument). Undefined for remote + * (url-only) servers that carry no command. + */ + rawCommand?: string; /** * Canonical identity of the launch command from agent-gov-core's * normalizeMcpCommand, computed *without* env. Two servers with the same diff --git a/test/cli-output.test.mjs b/test/cli-output.test.mjs index 53f15e7..1bdb754 100644 --- a/test/cli-output.test.mjs +++ b/test/cli-output.test.mjs @@ -1204,6 +1204,72 @@ test('CLI fix pin dry-run does not modify files', async () => { assert.equal(after, before); }); +test('CLI fix pin preserves a canonical inline command string instead of dropping its args', async () => { + // Regression: the canonical surface authors the launch as a single + // inline command string with no separate args array. The old plan did + // canonicalServer.command.split(' ')[0], which copied only "npx" and + // silently dropped "-y @modelcontextprotocol/server-github@1.2.3". + const src = join(testDir, 'fixtures', 'fix-pin-inline-command'); + const repo = await mkdtemp(join(tmpdir(), 'policymesh-fix-pin-inline-')); + try { + await copyFixture(src, repo); + + const { stdout } = await execFileAsync( + process.execPath, + ['dist/index.js', 'fix', 'pin', '--repo', repo, '--canonical', 'root_mcp', '--write'], + { cwd: packageRoot } + ); + assert.match(stdout, /Applied 1 edit/); + + const updated = JSON.parse(await readFile(join(repo, '.cursor', 'mcp.json'), 'utf8')); + // The full canonical command survives — package and flags intact. + assert.equal(updated.mcpServers.github.command, 'npx -y @modelcontextprotocol/server-github@1.2.3'); + // No stray args array was introduced. + assert.equal(updated.mcpServers.github.args, undefined); + + const { stdout: auditOut } = await execFileAsync( + process.execPath, + ['dist/index.js', 'audit', '--repo', repo, '--format', 'json'], + { cwd: packageRoot } + ); + const report = JSON.parse(auditOut); + assert.equal( + report.findings.some((f) => f.kind === 'policy_mesh.mcp_command_mismatch'), + false + ); + } finally { + await rm(repo, { recursive: true, force: true }); + } +}); + +test('CLI fix pin skips, not corrupts, when canonical folds args but the target keeps a separate args array', async () => { + // Canonical = inline command string; target = bare command + args + // array. Rewriting the command while leaving the target args would + // produce "npx -y pkg@1.2.3 -y pkg@2.0.0". We refuse instead. + const src = join(testDir, 'fixtures', 'fix-pin-shape-mismatch'); + const repo = await mkdtemp(join(tmpdir(), 'policymesh-fix-pin-mismatch-')); + try { + await copyFixture(src, repo); + const cursorPath = join(repo, '.cursor', 'mcp.json'); + const before = await readFile(cursorPath, 'utf8'); + + const { stdout } = await execFileAsync( + process.execPath, + ['dist/index.js', 'fix', 'pin', '--repo', repo, '--canonical', 'root_mcp', '--write'], + { cwd: packageRoot } + ); + + assert.match(stdout, /Applied 0 edit\(s\), skipped 1/); + assert.match(stdout, /align manually to avoid a corrupt launch/); + + // The target file is byte-for-byte untouched. + const after = await readFile(cursorPath, 'utf8'); + assert.equal(after, before); + } finally { + await rm(repo, { recursive: true, force: true }); + } +}); + test('CLI fix dry-run lists planned enabled-state edits without modifying files', async () => { const repo = join(testDir, 'fixtures', 'fix-enabled-mismatch'); const before = await readFile(join(repo, '.cursor', 'mcp.json'), 'utf8'); diff --git a/test/fixtures/fix-pin-inline-command/.cursor/mcp.json b/test/fixtures/fix-pin-inline-command/.cursor/mcp.json new file mode 100644 index 0000000..1022fe4 --- /dev/null +++ b/test/fixtures/fix-pin-inline-command/.cursor/mcp.json @@ -0,0 +1,7 @@ +{ + "mcpServers": { + "github": { + "command": "npx -y @modelcontextprotocol/server-github@2.0.0" + } + } +} diff --git a/test/fixtures/fix-pin-inline-command/.mcp.json b/test/fixtures/fix-pin-inline-command/.mcp.json new file mode 100644 index 0000000..e8ef416 --- /dev/null +++ b/test/fixtures/fix-pin-inline-command/.mcp.json @@ -0,0 +1,7 @@ +{ + "mcpServers": { + "github": { + "command": "npx -y @modelcontextprotocol/server-github@1.2.3" + } + } +} diff --git a/test/fixtures/fix-pin-shape-mismatch/.cursor/mcp.json b/test/fixtures/fix-pin-shape-mismatch/.cursor/mcp.json new file mode 100644 index 0000000..e687be1 --- /dev/null +++ b/test/fixtures/fix-pin-shape-mismatch/.cursor/mcp.json @@ -0,0 +1,8 @@ +{ + "mcpServers": { + "github": { + "command": "npx", + "args": ["-y", "@modelcontextprotocol/server-github@2.0.0"] + } + } +} diff --git a/test/fixtures/fix-pin-shape-mismatch/.mcp.json b/test/fixtures/fix-pin-shape-mismatch/.mcp.json new file mode 100644 index 0000000..e8ef416 --- /dev/null +++ b/test/fixtures/fix-pin-shape-mismatch/.mcp.json @@ -0,0 +1,7 @@ +{ + "mcpServers": { + "github": { + "command": "npx -y @modelcontextprotocol/server-github@1.2.3" + } + } +}