From 8eee83ddb9c27782313284961f1a2e3d53a9b791 Mon Sep 17 00:00:00 2001 From: LurigeLars <201361928+LurigeLars@users.noreply.github.com> Date: Wed, 14 Oct 2026 11:39:44 +0200 Subject: [PATCH] fix(local-mcp): wait for concurrent Docker creation to become visible --- deploy/local-gateway/local-mcp/bridge.mjs | 14 +++++++---- .../local-gateway/local-mcp/stdio-proxy.mjs | 2 +- .../test/local-backend-lifecycle.test.mjs | 2 +- .../test/local-stdio-proxy.test.mjs | 23 ++++++++++--------- 4 files changed, 24 insertions(+), 17 deletions(-) diff --git a/deploy/local-gateway/local-mcp/bridge.mjs b/deploy/local-gateway/local-mcp/bridge.mjs index 95ba029..871fcfc 100644 --- a/deploy/local-gateway/local-mcp/bridge.mjs +++ b/deploy/local-gateway/local-mcp/bridge.mjs @@ -2,6 +2,7 @@ // Docker access is injected; this module never launches processes or reads secrets. import { createHash } from 'node:crypto'; +import { setTimeout } from 'node:timers/promises'; import { checkRequestPolicy, @@ -48,16 +49,21 @@ export function buildDockerRunArgs({ image, configFile, name }) { // A backend belongs to its pinned image/config, not to a short-lived MCP client. // shortcut: old image/config backends need explicit retirement after their clients exit. -export function ensureLocalBackend({ image, configFile, configText, docker }) { +export async function ensureLocalBackend({ image, configFile, configText, docker }) { const key = createHash('sha256').update(image).update('\0').update(configText).digest('hex').slice(0, 16); const name = `github-mcp-local-stdio-${key}`; const inspect = () => docker(['inspect', '--format', `{"image":{{json .Config.Image}},"shared":{{json (index .Config.Labels "${SHARED_CONTAINER_LABEL}")}},"running":{{json .State.Running}}}`, name]); let state = inspect(); if (state.status !== 0) { - docker(buildDockerRunArgs({ image, configFile, name })); - // A concurrent client may have won the name; inspect that same backend. - state = inspect(); + const run = docker(buildDockerRunArgs({ image, configFile, name })); + // Docker reserves the name before a concurrent create is visible to inspect. + for (let attempt = 0; attempt < 40; attempt++) { + state = inspect(); + if (state.status === 0) break; + if (run.status !== 0 && !/already in use/.test(run.stderr ?? '')) break; + await setTimeout(100); + } } if (state.status !== 0) throw new Error('failed to create the shared local GitHub MCP backend'); const backend = JSON.parse(state.stdout); diff --git a/deploy/local-gateway/local-mcp/stdio-proxy.mjs b/deploy/local-gateway/local-mcp/stdio-proxy.mjs index 6806a64..199ba7a 100644 --- a/deploy/local-gateway/local-mcp/stdio-proxy.mjs +++ b/deploy/local-gateway/local-mcp/stdio-proxy.mjs @@ -90,7 +90,7 @@ process.on('uncaughtException', () => { removeOrphanedBackends(); -const { name, endpoint } = ensureLocalBackend({ +const { name, endpoint } = await ensureLocalBackend({ image, configFile, configText: readFileSync(configFile, 'utf8'), docker, }); diff --git a/deploy/local-gateway/test/local-backend-lifecycle.test.mjs b/deploy/local-gateway/test/local-backend-lifecycle.test.mjs index 616409c..304bf09 100644 --- a/deploy/local-gateway/test/local-backend-lifecycle.test.mjs +++ b/deploy/local-gateway/test/local-backend-lifecycle.test.mjs @@ -26,7 +26,7 @@ test('concurrent and successive client processes reuse the same container, inclu import { ensureLocalBackend } from ${JSON.stringify(bridgeUrl)}; const configFile = ${JSON.stringify(configFile)}; const docker = args => spawnSync('docker', args, { encoding: 'utf8', windowsHide: true, timeout: 30000 }); - console.log(JSON.stringify(ensureLocalBackend({ image: ${JSON.stringify(image)}, configFile, + console.log(JSON.stringify(await ensureLocalBackend({ image: ${JSON.stringify(image)}, configFile, configText: readFileSync(configFile, 'utf8'), docker }))); `; const connect = async () => JSON.parse((await exec(process.execPath, ['--input-type=module', '-e', client], { windowsHide: true, timeout: 40_000 })).stdout); diff --git a/deploy/local-gateway/test/local-stdio-proxy.test.mjs b/deploy/local-gateway/test/local-stdio-proxy.test.mjs index cc622a4..fb7b3e9 100644 --- a/deploy/local-gateway/test/local-stdio-proxy.test.mjs +++ b/deploy/local-gateway/test/local-stdio-proxy.test.mjs @@ -44,7 +44,7 @@ test('docker run arguments never carry credentials and bind only to loopback', ( assert.deepEqual(args.slice(-3), ['http', '--port=8082', '--listen-host=0.0.0.0']); }); -test('successive clients reuse one backend and a stopped backend is started without recreation', () => { +test('successive clients reuse one backend and a stopped backend is started without recreation', async () => { const image = parsePinnedImage(compose); const calls = []; let exists = false; @@ -60,37 +60,38 @@ test('successive clients reuse one backend and a stopped backend is started with assert.fail('unexpected Docker operation: ' + args[0]); }; const connect = () => ensureLocalBackend({ image, configFile: 'github.env', configText: 'GITHUB_TOOLSETS=repos', docker }); - const first = connect(); - assert.deepEqual(connect(), first); + const first = await connect(); + assert.deepEqual(await connect(), first); running = false; - assert.deepEqual(connect(), first); + assert.deepEqual(await connect(), first); assert.equal(calls.filter(args => args[0] === 'run').length, 1); assert.equal(calls.filter(args => args[0] === 'start').length, 1); assert.equal(first.endpoint, 'http://127.0.0.1:49152/'); }); -test('a concurrent creator winning the shared name is reused', () => { +test('a concurrent name reservation is reused after inspect becomes available', async () => { const image = parsePinnedImage(compose); let inspections = 0; const docker = args => { - if (args[0] === 'inspect') return ++inspections === 1 + if (args[0] === 'inspect') return ++inspections <= 3 ? { status: 1 } : { status: 0, stdout: JSON.stringify({ image, shared: '1', running: true }) }; - if (args[0] === 'run') return { status: 125 }; + if (args[0] === 'run') return { status: 125, stderr: 'Conflict. The container name is already in use.' }; if (args[0] === 'port') return { status: 0, stdout: '127.0.0.1:49152\n' }; assert.fail('unexpected Docker operation: ' + args[0]); }; - assert.equal(ensureLocalBackend({ image, configFile: 'github.env', configText: '', docker }).endpoint, 'http://127.0.0.1:49152/'); + assert.equal((await ensureLocalBackend({ image, configFile: 'github.env', configText: '', docker })).endpoint, 'http://127.0.0.1:49152/'); + assert.equal(inspections, 4); }); -test('wrong backend identity and Docker failure fail closed', () => { +test('wrong backend identity and Docker failure fail closed', async () => { const image = parsePinnedImage(compose); for (const stdout of [JSON.stringify({ image: 'wrong-image', shared: '1', running: true }), JSON.stringify({ image, shared: '', running: true })]) { - assert.throws(() => ensureLocalBackend({ image, configFile: 'github.env', configText: '', + await assert.rejects(() => ensureLocalBackend({ image, configFile: 'github.env', configText: '', docker: args => { assert.equal(args[0], 'inspect'); return { status: 0, stdout }; }, }), /identity mismatch/); } - assert.throws(() => ensureLocalBackend({ image, configFile: 'github.env', configText: '', + await assert.rejects(() => ensureLocalBackend({ image, configFile: 'github.env', configText: '', docker: () => ({ status: 1 }), }), /failed to create/); });