From acda360cf02d75edbb76cf11c6e95f91c1de99f9 Mon Sep 17 00:00:00 2001 From: Sameh Khamis Date: Wed, 7 Oct 2026 15:12:44 -0700 Subject: [PATCH] fix(mcp): serialize shared panel key repair and credit report --- CHANGELOG.md | 14 +++---- docs/CURRENT-STATE.md | 13 ++++++ server/mcp/panelKey.js | 52 ++++++++++++++++++----- src/tools/mcpPanelKey.test.mjs | 76 ++++++++++++++++++++++++++++++++++ 4 files changed, 138 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f27b8d911..3f2e585e9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -59,13 +59,13 @@ ## [Unreleased] -- Keep the globe panel working when an MCP client starts more than one - server process. Each stdio server made its own panel key, so a client that - read the panel page from one process and sent its requests to another - (seen in Claude Desktop) got "Only the God's Eye View panel may make this - request". Stdio servers of one install now share a key, created once in - `.gev-cache/`; a server that cannot store it keeps a key of its own, as - before (#927). +- Stdio servers from one install share a panel key so a page read from one + process can make requests through another. Concurrent malformed-key repairs + now select one winner; unavailable storage or a busy repair retains the + logged per-process fallback. Thanks to [Jibran Tahir](https://github.com/jibraaan) + for #958, [MarvinNL046](https://github.com/MarvinNL046) for the report and + proposed diagnosis in #927, and [kvnloo](https://github.com/kvnloo) for identifying + the repair race. Windows/Claude Desktop Cowork confirmation remains outstanding. - Street Level: a street-level imagery layer modelled on the iD editor's photo overlay, with Mapillary as its first provider (free client token, CC BY-SA 4.0 diff --git a/docs/CURRENT-STATE.md b/docs/CURRENT-STATE.md index 730586df9..ee3bfbdb2 100644 --- a/docs/CURRENT-STATE.md +++ b/docs/CURRENT-STATE.md @@ -14,6 +14,19 @@ new views from the page that frames it; framing is off unless `GEV_EMBED_FRAME_ANCESTORS` allows the framing page. MCP leads with the tools that find what to show. See [tools and the MCP server](TOOLS.md). +Stdio processes from one install share the panel key in +`.gev-cache/mcp-panel-key`, resolved from the checkout rather than the client's +working directory. First creation is atomic; malformed-file repairs select one +winner and concurrent processes re-read it. A busy repair waits at most two +seconds before falling back. If the cache cannot be read/written or repair remains +busy, stderr reports the fallback to a per-process key; cross-process panels can +still fail in that mode. An abandoned `.repair-lock` is left intact: after +confirming no repair process is running, remove it to allow repair on restart. +HTTP MCP servers retain per-server keys. This key is a panel routing guard, not +authentication. Large-response continuations remain process-local and require +consistent tool-call routing. Two-process reproduction is covered locally; +Windows/Claude Desktop Cowork confirmation remains outstanding (#927). + ## Tools and local MCP server — October 1, 2026 `gods-eye-view/tools` defines queries that answer questions from the app's data, diff --git a/server/mcp/panelKey.js b/server/mcp/panelKey.js index 6dd42c46c..8b34657f7 100644 --- a/server/mcp/panelKey.js +++ b/server/mcp/panelKey.js @@ -20,17 +20,52 @@ function readKey(file) { return KEY_PATTERN.test(text) ? text : null; } +/** Serialize malformed-file repair; ordinary first creation still uses link(). */ +function repairKey(file, tmp, key) { + const lock = `${file}.repair-lock`; + const deadline = Date.now() + 2000; + const pause = new Int32Array(new SharedArrayBuffer(4)); + let fd; + for (;;) { + try { + fd = fs.openSync(lock, 'wx', 0o600); + break; + } catch (error) { + if (error.code !== 'EEXIST') throw error; + // Another repair may already have published a usable key. + const winner = readKey(file); + if (winner) return winner; + if (Date.now() >= deadline) { + throw Object.assign(new Error('panel key repair is busy'), { + code: 'ETIMEDOUT', + }); + } + Atomics.wait(pause, 0, 0, 10); + } + } + try { + // A repair completed between our earlier read and acquiring the lock. + const winner = readKey(file); + if (winner) return winner; + fs.renameSync(tmp, file); + return key; + } finally { + fs.closeSync(fd); + fs.rmSync(lock, { force: true }); + } +} + /** * The panel key every stdio server of this install shares. * * A client can start more than one server process for one connection: Claude - * Desktop has been seen serving the globe panel's page from one and sending - * the page's `panel_request` calls to the other. A key made per process then - * never matches, and the panel reports that only it may make the request - * (#927). The first process to run creates the key, without replacing one + * Desktop was reported to start two processes; serving the panel's page from + * one and sending its `panel_request` calls to the other reproduces #927. + * Per-process keys then mismatch, and the panel refuses the request. + * The first process to run creates the key, without replacing one * another process created at the same moment; the rest read it. When the file - * cannot be read or written, the process falls back to a key of its own, - * which is what every process used before. + * cannot be read or written, or repair stays busy for two seconds, the process + * falls back to its own key, as every process did before. * * As before, the key keeps `panel_request` from clients that only list it; it * is not access control. @@ -71,10 +106,7 @@ export function sharedPanelKey({ if (error.code !== 'EEXIST') throw error; const winner = readKey(file); if (winner) return winner; - // A malformed file left by something else: replace it. - fs.renameSync(tmp, file); - tmp = null; - return key; + return repairKey(file, tmp, key); } } catch (error) { log(notShared(error)); diff --git a/src/tools/mcpPanelKey.test.mjs b/src/tools/mcpPanelKey.test.mjs index c2bddac73..d70fdb3e2 100644 --- a/src/tools/mcpPanelKey.test.mjs +++ b/src/tools/mcpPanelKey.test.mjs @@ -122,3 +122,79 @@ test('processes starting at the same moment agree on one key', async (t) => { assert.equal(new Set(keys).size, 1); assert.equal(fs.readFileSync(file, 'utf8').trim(), keys[0]); }); + +test('concurrent malformed-file repairs return the same persisted key', async (t) => { + const file = tempKeyFile(t); + fs.mkdirSync(path.dirname(file), { recursive: true }); + fs.writeFileSync(file, 'not a key\n'); + const script = ` + import fs from 'node:fs'; + const { sharedPanelKey } = await import(${JSON.stringify(PANEL_KEY_MODULE)}); + const [file, marker, other] = process.argv.slice(1); + const original = fs.readFileSync; + let reads = 0; + fs.readFileSync = function (target, ...args) { + const text = original.call(this, target, ...args); + if (target === file && ++reads === 2) { + // Both processes capture the actual malformed contents before either + // can replace them. The old unchecked rename returns two different keys. + fs.writeFileSync(marker, 'ready'); + const deadline = Date.now() + 5000; + while (!fs.existsSync(other)) { + if (Date.now() >= deadline) throw new Error('repair barrier timed out'); + Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, 5); + } + } + return text; + }; + process.stdout.write(sharedPanelKey({ file })); + `; + const run = (index) => + new Promise((resolve, reject) => { + const marker = (id) => path.join(path.dirname(file), `ready-${id}`); + const child = spawn( + process.execPath, + [ + '--input-type=module', + '-e', + script, + file, + marker(index), + marker(1 - index), + ], + { stdio: ['ignore', 'pipe', 'inherit'] }, + ); + t.after(() => child.kill()); + let out = ''; + child.stdout.on('data', (chunk) => (out += chunk)); + child.on('error', reject); + child.on('close', (code) => + code === 0 ? resolve(out) : reject(new Error(`exit ${code}`)), + ); + }); + const keys = await Promise.all([run(0), run(1)]); + assert.equal(new Set(keys).size, 1); + assert.equal(fs.readFileSync(file, 'utf8').trim(), keys[0]); + assert.equal(fs.existsSync(`${file}.repair-lock`), false); + assert.equal( + fs.readdirSync(path.dirname(file)).some((name) => name.endsWith('.tmp')), + false, + ); +}); + +test('an unavailable repair lock falls back without hanging or removing its owner’s lock', (t) => { + const file = tempKeyFile(t); + fs.mkdirSync(path.dirname(file), { recursive: true }); + fs.writeFileSync(file, 'not a key\n'); + fs.writeFileSync(`${file}.repair-lock`, 'held'); + const logged = []; + const key = sharedPanelKey({ file, log: (line) => logged.push(line) }); + assert.match(key, /^[A-Za-z0-9_-]{43}$/); + assert.match(logged[0], /ETIMEDOUT/); + assert.equal(fs.readFileSync(file, 'utf8'), 'not a key\n'); + assert.equal(fs.readFileSync(`${file}.repair-lock`, 'utf8'), 'held'); + assert.deepEqual(fs.readdirSync(path.dirname(file)).sort(), [ + 'mcp-panel-key', + 'mcp-panel-key.repair-lock', + ]); +});