diff --git a/.oxfmtrc.jsonc b/.oxfmtrc.jsonc index e4f44c346..7135624e6 100644 --- a/.oxfmtrc.jsonc +++ b/.oxfmtrc.jsonc @@ -1,14 +1,6 @@ { "$schema": "./node_modules/oxfmt/configuration_schema.json", - "ignorePatterns": [ - ".changeset/*.md", - "**/__generated__/**", - "schema.graphql", - "pnpm-lock.yaml", - // Vendored verbatim from upstream so `curl | diff` can prove it has not - // drifted; reformatting its inline script would defeat that check. - "scripts/lib/inspector-sandbox-proxy.html" - ], + "ignorePatterns": [".changeset/*.md", "**/__generated__/**", "schema.graphql", "pnpm-lock.yaml"], "printWidth": 100, "tabWidth": 2, "semi": true, diff --git a/packages/mcp/README.md b/packages/mcp/README.md index 30b727397..6922532a3 100644 --- a/packages/mcp/README.md +++ b/packages/mcp/README.md @@ -510,8 +510,6 @@ A view's document is read once, when the app's sandbox iframe mounts, and never Two things must be true for that reopen to show new markup, and both are handled for you. The server has to read the view from disk rather than the copy inlined at build time, and it has to actually receive `TRANSCEND_MCP_DEV_VIEWS` — which exporting it cannot achieve, because the Inspector spawns a stdio server with an allowlisted environment (`HOME`, `LOGNAME`, `PATH`, `SHELL`, `TERM`, `USER`) plus only what its `-e KEY=VALUE` flag supplied. `pnpm mcp:inspect` passes the flag. The same allowlist drops API credentials, and we deliberately leave them out rather than exposing them in a command line every local process can read, so use `--http` when a tool needs to reach the Transcend API: there we spawn the server ourselves and it inherits the environment normally. -One workaround runs before launch. v2's published tarball omits `clients/web/static/sandbox_proxy.html`, the document that hosts the app's iframe, so opening an app renders `Sandbox not loaded: ENOENT ...` inside the app frame — a missing file that looks like a broken view. `pnpm mcp:inspect` restores it from a vendored copy of upstream's file and logs a line when it does. It is a workaround for [inspector#1859](https://github.com/modelcontextprotocol/inspector/issues/1859), tracked by a `TODO` in [`scripts/lib/mcp-app-dev.ts`](../../scripts/lib/mcp-app-dev.ts) to delete once a release ships the file; an install that already has the document is left alone. Because the Inspector reads it once at startup, an instance that was already running when the file appeared keeps serving the error — restart it. - #### When a view does not appear, check the capability gate first This is the failure that looks exactly like a broken view. A tool's `_meta.ui` is only attached when the client declared the MCP Apps extension: diff --git a/scripts/inspector-sandbox-proxy.test.ts b/scripts/inspector-sandbox-proxy.test.ts deleted file mode 100644 index 985b3e9e1..000000000 --- a/scripts/inspector-sandbox-proxy.test.ts +++ /dev/null @@ -1,95 +0,0 @@ -import { createHash } from 'node:crypto'; -import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; -import { tmpdir } from 'node:os'; -import { join } from 'node:path'; -import { fileURLToPath } from 'node:url'; - -import { afterEach, describe, expect, it } from 'vitest'; - -import { restoreSandboxProxy, SandboxProxyOutcome } from './lib/mcp-app-dev.ts'; - -const VENDORED_PROXY = fileURLToPath( - new URL('./lib/inspector-sandbox-proxy.html', import.meta.url), -); -const PROXY_PATH = join('clients', 'web', 'static', 'sandbox_proxy.html'); - -const temporaryDirs: string[] = []; - -/** Creates a directory shaped like an Inspector install, minus the proxy. */ -function fakeInstall({ withWebClient = true } = {}): string { - const dir = mkdtempSync(join(tmpdir(), 'inspector-install-')); - temporaryDirs.push(dir); - if (withWebClient) mkdirSync(join(dir, 'clients', 'web', 'dist'), { recursive: true }); - return dir; -} - -afterEach(() => { - let dir = temporaryDirs.pop(); - while (dir !== undefined) { - rmSync(dir, { force: true, recursive: true }); - dir = temporaryDirs.pop(); - } -}); - -describe('restoreSandboxProxy', () => { - it('writes the proxy document when the published package omitted it', () => { - const installDir = fakeInstall(); - - expect(restoreSandboxProxy(installDir)).toBe(SandboxProxyOutcome.Written); - expect(readFileSync(join(installDir, PROXY_PATH), 'utf8')).toBe( - readFileSync(VENDORED_PROXY, 'utf8'), - ); - }); - - it('leaves an existing document untouched', () => { - // The whole point of the workaround is to fill a gap, so a release that - // ships its own proxy — or a newer one than ours — must win. - const installDir = fakeInstall(); - const target = join(installDir, PROXY_PATH); - mkdirSync(join(installDir, 'clients', 'web', 'static'), { recursive: true }); - writeFileSync(target, '

upstream

'); - - expect(restoreSandboxProxy(installDir)).toBe(SandboxProxyOutcome.Present); - expect(readFileSync(target, 'utf8')).toBe('

upstream

'); - }); - - it('creates nothing when the directory is not an Inspector install', () => { - const installDir = fakeInstall({ withWebClient: false }); - - expect(restoreSandboxProxy(installDir)).toBe(SandboxProxyOutcome.Unrecognized); - expect(existsSync(join(installDir, 'clients'))).toBe(false); - }); -}); - -describe('the vendored proxy document', () => { - // Nothing imports this file, so only a test notices if it is deleted as dead - // weight or truncated. These are the two things it has to be: a participant in - // the bridge protocol, whose method names are its contract with the - // Inspector's web client, and the isolation boundary that makes restoring - // upstream's document — rather than improvising a replacement — the right - // call. The hash below pins the bytes; this case is what says which part - // broke. - it('is a document speaking the sandbox bridge protocol, and denying same-origin access', () => { - const html = readFileSync(VENDORED_PROXY, 'utf8'); - - expect(html.startsWith('')).toBe(true); - expect(html).toContain('ui/notifications/sandbox-proxy-ready'); - expect(html).toContain('ui/notifications/sandbox-resource-ready'); - expect(html).toContain('allow-scripts allow-forms'); - expect(html).toMatch(/toLowerCase\(\) !== "allow-same-origin"/); - }); - - it('is byte-identical to upstream', () => { - // This document is a security boundary we did not write, so it should only - // ever change by deliberately re-copying upstream's. Editing it in place — - // to satisfy a linter, or to "just fix" something — would silently change - // what isolates an untrusted view, so make that a failing test instead. - // Update the hash when re-copying, from: - // - // curl -s https://raw.githubusercontent.com/modelcontextprotocol/inspector/main/clients/web/static/sandbox_proxy.html \ - // | shasum -a 256 - const digest = createHash('sha256').update(readFileSync(VENDORED_PROXY)).digest('hex'); - - expect(digest).toBe('895cebc62dce32428350a77af0a433faf3fbba4943cef5f49a28b9ed223f9d99'); - }); -}); diff --git a/scripts/lib/inspector-sandbox-proxy.html b/scripts/lib/inspector-sandbox-proxy.html deleted file mode 100644 index 2f4336e8a..000000000 --- a/scripts/lib/inspector-sandbox-proxy.html +++ /dev/null @@ -1,237 +0,0 @@ - - - - - - - MCP-UI Proxy - - - - - - diff --git a/scripts/lib/mcp-app-dev.ts b/scripts/lib/mcp-app-dev.ts index 4ecfa54c2..c734ac214 100644 --- a/scripts/lib/mcp-app-dev.ts +++ b/scripts/lib/mcp-app-dev.ts @@ -1,12 +1,12 @@ import { spawn, type ChildProcess } from 'node:child_process'; -import { copyFileSync, existsSync, mkdirSync, readdirSync, readFileSync } from 'node:fs'; +import { existsSync, readdirSync, readFileSync } from 'node:fs'; import { dirname, join, resolve } from 'node:path'; import { fileURLToPath } from 'node:url'; import { discoverMcpAppViews, MCP_APP_OUT_DIR, type McpAppView } from '../../vite.config.base.ts'; import { logger } from '../logger.ts'; -/** Directory holding this file, used to locate assets that ship beside it. */ +/** Directory holding this file, used to derive the repository root. */ const scriptsLibDir = dirname(fileURLToPath(import.meta.url)); /** Repository root, derived from this file rather than the working directory. */ @@ -42,185 +42,11 @@ export const EXAMPLES_PACKAGE = '@transcend-io/mcp-server-examples'; /** * The Inspector release `pnpm mcp:inspect` runs. * - * Pinned to the major because {@link restoreSandboxProxy} reaches into the install's - * `clients/web/static` layout, and because a client that stopped declaring + * Pinned to the major because a client that stopped declaring * `extensions["io.modelcontextprotocol/ui"]` would silently withhold every view. */ export const INSPECTOR_SPEC = '@modelcontextprotocol/inspector@2'; -/** Package the spec above resolves to. */ -const INSPECTOR_PACKAGE_NAME = '@modelcontextprotocol/inspector'; - -/** - * Path the Inspector's web client reads the app sandbox document from, relative - * to its install directory. - */ -const SANDBOX_PROXY_PATH = join('clients', 'web', 'static', 'sandbox_proxy.html'); - -/** Our copy of the document, kept byte-identical to upstream's. */ -const VENDORED_SANDBOX_PROXY = join(scriptsLibDir, 'inspector-sandbox-proxy.html'); - -/** Outcome of {@link restoreSandboxProxy}. */ -export const SandboxProxyOutcome = { - /** Already present */ - Present: 'present', - /** Vendored copy written */ - Written: 'written', - /** Not an Inspector install */ - Unrecognized: 'unrecognized', -} as const; - -/** Union of {@link SandboxProxyOutcome} values. */ -export type SandboxProxyOutcome = (typeof SandboxProxyOutcome)[keyof typeof SandboxProxyOutcome]; - -/** - * Writes the sandbox proxy document into an Inspector install that is missing it. - * - * TODO(ZEL-8153): https://github.com/modelcontextprotocol/inspector/issues/1859 — - * delete this, the vendored document, and its call site once a release ships the - * file. Tracking ticket: https://linear.app/transcend/issue/ZEL-8153 - * The published v2 tarball's `files` list covers `clients/web/build` and - * `clients/web/dist` but not `clients/web/static`, so the one document the Apps - * tab needs is absent. The web server reads it at startup, swallows the ENOENT, - * and substitutes its own error page, which then renders *inside the app frame* - * as "Sandbox not loaded: ENOENT ...". Every other tab works, so it looks like a - * broken view rather than a missing file. Upstream shipped and fixed the same - * omission once before in v1 (issue #1113, for `server/static`). - * - * Restoring the file rather than working around it is deliberate: the proxy is - * the security boundary for app rendering — it holds the untrusted view at an - * opaque origin, strips `allow-same-origin` from anything a server asks for, and - * relays bridge messages between host and view. A substitute of our own would - * make this loop diverge from real hosts on exactly the axis the Inspector is - * here to check, so the vendored copy is upstream's file verbatim — down to the - * bytes, which is why the formatter is told to skip it and a test pins its hash. - * Compare it against upstream with: - * - * ```bash - * curl -s https://raw.githubusercontent.com/modelcontextprotocol/inspector/main/clients/web/static/sandbox_proxy.html \ - * | diff -u - scripts/lib/inspector-sandbox-proxy.html - * ``` - * - * @param installDir - Root of an Inspector installation - * @returns Whether the document was already there, written, or the directory did - * not look like an Inspector install - */ -export function restoreSandboxProxy(installDir: string): SandboxProxyOutcome { - // Absent `clients/web` this is not the layout the fix was written against, so - // creating directories would be guessing at someone else's package. - if (!existsSync(join(installDir, 'clients', 'web'))) { - return SandboxProxyOutcome.Unrecognized; - } - - const target = join(installDir, SANDBOX_PROXY_PATH); - if (existsSync(target)) return SandboxProxyOutcome.Present; - - mkdirSync(dirname(target), { recursive: true }); - copyFileSync(VENDORED_SANDBOX_PROXY, target); - return SandboxProxyOutcome.Written; -} - -/** - * Locates the directory `npx` installed a package into. - * - * Derived from the child's own `PATH` rather than by globbing `~/.npm/_npx`, - * because npm decides where that cache lives — it moves with `npm_config_cache`, - * and sandboxes relocate it wholesale. Running the probe under the same spec we - * are about to launch is what guarantees we patch the install that will be used. - * - * @param spec - Package spec to resolve, e.g. `pkg@2` - * @param packageName - Package to find inside the install - * @returns The package directory, or undefined if it could not be located - */ -async function resolveNpxPackageDir( - spec: string, - packageName: string, -): Promise { - const probe = ` - const path = require('node:path'); - const fs = require('node:fs'); - const segments = ${JSON.stringify(packageName.split('/'))}; - for (const dir of (process.env.PATH || '').split(path.delimiter)) { - if (path.basename(dir) !== '.bin') continue; - if (path.basename(path.dirname(dir)) !== 'node_modules') continue; - const manifest = path.join(path.dirname(dir), ...segments, 'package.json'); - if (fs.existsSync(manifest)) { - process.stdout.write(path.dirname(manifest)); - break; - } - } - `; - - const stdout = await new Promise((resolvePromise, reject) => { - const child = spawn('npx', ['-y', `--package=${spec}`, 'node', '-e', probe], { - cwd: repoRoot, - env: process.env, - // npm prints install and peer-dependency warnings to stderr that say - // nothing about whether the probe worked, so keep them out of the way. - stdio: ['ignore', 'pipe', 'pipe'], - shell: false, - }); - - let output = ''; - let errors = ''; - child.stdout?.on('data', (chunk: Buffer) => { - output += chunk.toString(); - }); - child.stderr?.on('data', (chunk: Buffer) => { - errors += chunk.toString(); - }); - child.on('error', reject); - child.on('exit', (code) => { - if (code === 0) resolvePromise(output.trim()); - else reject(new Error(`Resolving ${spec} failed with exit code ${code}. ${errors.trim()}`)); - }); - }); - - return stdout === '' ? undefined : stdout; -} - -/** - * Makes sure the Inspector can render an app before we hand it a server that - * serves one. - * - * Warns rather than throws on every failure path. This works around someone - * else's packaging bug, and the Inspector is still useful for tools, resources, - * and the handshake even when the Apps tab cannot paint — refusing to launch over - * it would be a worse outcome than a rendered error the warning explains. See - * {@link restoreSandboxProxy} for the removal condition. - * - * @param spec - Inspector spec about to be launched - */ -export async function ensureInspectorSandboxProxy(spec: string): Promise { - try { - const installDir = await resolveNpxPackageDir(spec, INSPECTOR_PACKAGE_NAME); - if (installDir === undefined) { - logger.log( - `Could not locate the ${spec} install to check its app sandbox document. ` + - 'If the app frame shows "Sandbox not loaded", that is why.', - ); - return; - } - - const outcome = restoreSandboxProxy(installDir); - if (outcome === SandboxProxyOutcome.Written) { - logger.log( - `Restored the missing app sandbox document in ${spec} ` + - '(upstream inspector issue 1859); the Apps tab would render an ENOENT without it.', - ); - } else if (outcome === SandboxProxyOutcome.Unrecognized) { - logger.log( - `The ${spec} install has an unfamiliar layout, so its app sandbox document was left alone.`, - ); - } - } catch (error) { - logger.log( - `Could not check the app sandbox document in ${spec}: ` + - `${error instanceof Error ? error.message : String(error)}`, - ); - } -} - /** Environment variable that makes servers read views from disk per request. */ export const DEV_VIEWS_ENV_VAR = 'TRANSCEND_MCP_DEV_VIEWS'; diff --git a/scripts/mcp-inspect.ts b/scripts/mcp-inspect.ts index 921f4c1a8..4b4fbbbdf 100644 --- a/scripts/mcp-inspect.ts +++ b/scripts/mcp-inspect.ts @@ -24,7 +24,6 @@ import { buildTarget, DEV_VIEWS_ENV_VAR, discoverMcpPackages, - ensureInspectorSandboxProxy, INSPECTOR_SPEC, inspectorEnvArgs, installShutdownHandlers, @@ -72,8 +71,7 @@ async function main(): Promise { loadSecretEnv(); installShutdownHandlers(); - // Overlapped because the sandbox check costs an `npx` resolution the build hides. - await Promise.all([buildTarget(target), ensureInspectorSandboxProxy(INSPECTOR_SPEC)]); + await buildTarget(target); // Set here for the server we spawn under `--http`; a stdio server is handed it // explicitly below, because the Inspector does not pass our environment on.