From 34dfed49f105345603b05ea809d14ccbfb0d85f5 Mon Sep 17 00:00:00 2001 From: harveysang Date: Sun, 11 Oct 2026 09:51:30 +0800 Subject: [PATCH] fix(DMY-66): recognize receipt-owned HTTP MCP upgrades The public installer recognized only stdio args and rejected native HTTP entries created by earlier OpenGUI installs. Verify the existing native receipt fingerprint before migration, remove stale HTTP fields, and cover upgrades plus preserved actionable conflicts. --- workbuddy-plugin/package.json | 2 +- workbuddy-plugin/scripts/install-local.mjs | 14 ++++- .../scripts/test-mcp-conflict.mjs | 63 +++++++++++++++++++ workbuddy-plugin/src/installation.ts | 3 +- workbuddy-plugin/tests/installation.spec.ts | 6 ++ 5 files changed, 84 insertions(+), 4 deletions(-) create mode 100644 workbuddy-plugin/scripts/test-mcp-conflict.mjs 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,