diff --git a/workbuddy-plugin/package.json b/workbuddy-plugin/package.json index cf067e2..14371d3 100644 --- a/workbuddy-plugin/package.json +++ b/workbuddy-plugin/package.json @@ -31,7 +31,7 @@ "test": "vitest run", "check": "npm test && npm run build && node scripts/validate.mjs", "pack:release": "npm run check && node scripts/package.mjs", - "smoke:packed": "node scripts/test-preflight.mjs && node scripts/test-install.mjs && node scripts/smoke-packed.mjs && node scripts/test-release-installer.mjs", + "smoke:packed": "node scripts/test-preflight.mjs && node scripts/test-install.mjs && node scripts/test-mcp-conflict.mjs && node scripts/smoke-packed.mjs && node scripts/test-release-installer.mjs", "test:browser": "npm run build && node scripts/test-wall-browser.mjs", "test:native": "npm run build && node scripts/test-native.mjs", "test:host": "npm run build && node scripts/test-workbuddy-host.mjs" diff --git a/workbuddy-plugin/scripts/install-local.mjs b/workbuddy-plugin/scripts/install-local.mjs index dad2132..182bf08 100644 --- a/workbuddy-plugin/scripts/install-local.mjs +++ b/workbuddy-plugin/scripts/install-local.mjs @@ -50,8 +50,18 @@ for (const path of targets) await assertOwnedPath(path) const existingMcp = JSON.parse(original[0] ?? '{}').mcpServers?.opengui if (existingMcp) { const knownEntrypoints = [packageDir, previous.packageDir].filter(Boolean).map(path => join(path, 'lib', 'mcp.js')) - assert(Array.isArray(existingMcp.args) && existingMcp.args.length === 1 && knownEntrypoints.includes(existingMcp.args[0]), - 'MCP_CONFLICT: an unrecognized opengui server already exists; preserve it and resolve the conflict explicitly') + const ownedStdio = (!existingMcp.type || existingMcp.type === 'stdio') && Array.isArray(existingMcp.args) + && existingMcp.args.length === 1 && knownEntrypoints.includes(existingMcp.args[0]) + // Native installations record the exact server, including its authorization. + // A matching URL alone cannot establish ownership of a third-party entry. + const ownedHttp = previous.configRoot === root && previous.transport === 'http' && existingMcp.type === 'http' + && previous.nativeServerSha256 === createHash('sha256').update(JSON.stringify(existingMcp)).digest('hex') + assert(ownedStdio || ownedHttp, + `MCP_CONFLICT: unrecognized or edited opengui server in ${targets[0]}. No configuration files changed.\n` + + `Back up and inspect the configuration before retrying:\ncp ${quote(targets[0])} ${quote(`${targets[0]}.before-opengui-conflict-${Date.now()}`)}\n` + + `\${EDITOR:-vi} ${quote(targets[0])}\n` + + 'To keep the existing server, rename its mcpServers key from opengui to an unused name (for example opengui-existing); ' + + 'or explicitly remove only that entry if it is no longer needed. Save, then rerun the installation command.') } const values = [ JSON.stringify(mergeMcpConfig(JSON.parse(original[0] ?? '{}'), node, join(packageDir, 'lib', 'mcp.js')), null, 2) + '\n', diff --git a/workbuddy-plugin/scripts/test-mcp-conflict.mjs b/workbuddy-plugin/scripts/test-mcp-conflict.mjs new file mode 100644 index 0000000..6e2ec84 --- /dev/null +++ b/workbuddy-plugin/scripts/test-mcp-conflict.mjs @@ -0,0 +1,63 @@ +import assert from 'node:assert/strict' +import { createHash } from 'node:crypto' +import { execFileSync, spawnSync } from 'node:child_process' +import { mkdtemp, mkdir, readFile, realpath, rm, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { fileURLToPath } from 'node:url' + +const temporary = await realpath(await mkdtemp(join(tmpdir(), 'opengui-conflict-'))) +const script = fileURLToPath(new URL('./install-local.mjs', import.meta.url)) +const hash = value => createHash('sha256').update(value).digest('hex') +try { + const root = join(temporary, "host with spaces and 'quote") + const state = join(root, 'opengui') + const pkg = join(state, 'packages/new/node_modules/opengui-mcp') + await mkdir(join(pkg, 'lib'), { recursive: true }) + await writeFile(join(pkg, 'package.json'), JSON.stringify({ name: 'opengui-mcp', version: '0.4.0' })) + await writeFile(join(pkg, 'lib/host-hook.js'), '// Fixture only\n') + await writeFile(join(pkg, 'lib/opengui-SKILL.md'), 'fixture skill\n') + const receiptPath = join(state, `local-install-${hash(root).slice(0, 16)}.json`) + const http = { type: 'http', timeout: 120000, disabled: false, url: 'http://127.0.0.1:61785/mcp', headers: { Authorization: 'Bearer fixture-only' } } + const receipt = { configRoot: root, packageDir: join(state, 'packages/old/node_modules/opengui-mcp'), transport: 'http', nativeServerSha256: hash(JSON.stringify(http)), hookCommands: [] } + const args = [script, '--config-root', root, '--state-root', state, '--package-dir', pkg] + const seed = async (server, previous = receipt) => { + await writeFile(join(root, 'mcp.json'), JSON.stringify({ mcpServers: { other: { command: 'retained' }, opengui: server } })) + await writeFile(join(root, 'settings.json'), '{"user":"retained"}\n') + await mkdir(join(root, 'skills/opengui'), { recursive: true }) + await writeFile(join(root, 'skills/opengui/SKILL.md'), 'old skill\n') + await writeFile(receiptPath, JSON.stringify(previous)) + } + await seed(http) + const installed = JSON.parse(execFileSync(process.execPath, args, { encoding: 'utf8', stdio: 'pipe' })) + const migrated = JSON.parse(await readFile(join(root, 'mcp.json'), 'utf8')) + assert.equal(installed.status, 'CONFIG_WRITTEN') + assert.equal(migrated.mcpServers.opengui.type, 'stdio') + assert.deepEqual(migrated.mcpServers.opengui.args, [join(pkg, 'lib/mcp.js')]) + assert.equal(migrated.mcpServers.opengui.url, undefined) + assert.equal(migrated.mcpServers.opengui.headers, undefined) + assert.equal(migrated.mcpServers.other.command, 'retained') + assert.equal(JSON.parse(execFileSync(process.execPath, args, { encoding: 'utf8', stdio: 'pipe' })).status, 'ALREADY_CONFIGURED') + await seed({ type: 'stdio', command: process.execPath, args: [join(receipt.packageDir, 'lib/mcp.js')] }, { ...receipt, transport: 'stdio' }) + execFileSync(process.execPath, args, { stdio: 'pipe' }) + for (const [server, previous] of [ + [{ command: 'third-party', args: [] }, {}], + [http, {}], + [{ ...http, url: 'http://127.0.0.1:61786/mcp' }, receipt], + [{ ...http, headers: { Authorization: 'Bearer user-edit' } }, receipt], + [http, { ...receipt, configRoot: join(temporary, 'other-host') }], + [http, { ...receipt, nativeServerSha256: undefined }], + ]) { + await seed(server, previous) + const paths = [join(root, 'mcp.json'), join(root, 'settings.json'), join(root, 'skills/opengui/SKILL.md'), receiptPath] + const before = await Promise.all(paths.map(path => readFile(path, 'utf8'))) + const result = spawnSync(process.execPath, args, { encoding: 'utf8' }) + assert.notEqual(result.status, 0) + assert.match(result.stderr, /MCP_CONFLICT/) + assert.match(result.stderr, /cp /) + assert.match(result.stderr, /EDITOR/) + assert.match(result.stderr, /rename.*opengui/s) + assert.deepEqual(await Promise.all(paths.map(path => readFile(path, 'utf8'))), before) + } + console.log('PASS: HTTP receipt migration, stdio upgrade, reinstall, and six preserved actionable conflicts') +} finally { await rm(temporary, { recursive: true, force: true }) } diff --git a/workbuddy-plugin/src/installation.ts b/workbuddy-plugin/src/installation.ts index 561f225..d37a9c7 100644 --- a/workbuddy-plugin/src/installation.ts +++ b/workbuddy-plugin/src/installation.ts @@ -18,8 +18,9 @@ export function mergeHostHooks(settings: Record, command: strin export function mergeMcpConfig(config: Record, node: string, entrypoint: string): Record { const servers = (config.mcpServers ?? {}) as Record + const { url: _url, headers: _headers, ...retained } = (servers.opengui ?? {}) as Record return { ...config, mcpServers: { ...servers, opengui: { - ...((servers.opengui ?? {}) as Record), type: 'stdio', command: node, + ...retained, type: 'stdio', command: node, args: [entrypoint], timeout: 120000, disabled: false, } } } } diff --git a/workbuddy-plugin/tests/installation.spec.ts b/workbuddy-plugin/tests/installation.spec.ts index 3acb7a1..2ca8d22 100644 --- a/workbuddy-plugin/tests/installation.spec.ts +++ b/workbuddy-plugin/tests/installation.spec.ts @@ -13,6 +13,12 @@ describe('scoped WorkBuddy installation', () => { expect(Object.keys(updated.hooks as object).sort()).toEqual([...HOST_HOOK_EVENTS].sort()) expect(JSON.stringify(updated)).not.toContain('permissionDecision') }) + it('removes HTTP transport fields when migrating an owned server to stdio', () => { + const original = { mcpServers: { opengui: { type: 'http', url: 'http://127.0.0.1:61785/mcp', headers: { Authorization: 'Bearer fixture' }, extra: true } } } + const updated = mergeMcpConfig(original, '/managed/node', '/immutable/lib/mcp.js') + expect(updated.mcpServers).toEqual({ opengui: { extra: true, type: 'stdio', command: '/managed/node', args: ['/immutable/lib/mcp.js'], timeout: 120000, disabled: false } }) + expect(original.mcpServers.opengui.headers.Authorization).toBe('Bearer fixture') + }) it('changes only the OpenGUI MCP entry and preserves its unrelated settings', () => { const original = { extra: true, mcpServers: { other: { command: 'production' }, opengui: { command: 'old', env: { KEEP: 'local' } } } } expect(mergeMcpConfig(original, '/managed/node', '/immutable/lib/mcp.js')).toEqual({ extra: true, mcpServers: { other: original.mcpServers.other,