From 71f4e194b71dd6363651d5235ab1d3684a104c7b Mon Sep 17 00:00:00 2001 From: Eli Pinkerton Date: Wed, 14 Oct 2026 20:51:57 +0000 Subject: [PATCH 1/3] Fix Unity bridge readiness and frontend connections Managed clients could not drive the host editor because reverse tunnels shadowed loopback traffic and connection settings diverged. Verify the serving HTTP instance before publishing the port, keep configuration sync asynchronous, preserve the optional token, and route managed frontends through the pinned bridge. Validation: full non-Unity gate passed (10 steps), strict pyright passed, eight subprocess bridge contracts and all sync/doctor contracts passed. The live doctor discovered 160 tools and drove the configured host project with zero failures or warnings. npm pack dry-run passed. No shipped C# changed. Host Unity 6000.5.2f1 on Windows: PlayMode 15/15 and AssetProcessorRelevanceTests 5/5 passed. Full EditMode twice passed 382/384; the same unrelated performance-driver contracts failed and are tracked in #240. Windows wildcard coexistence was observed in the prior host investigation; this session executes the subprocess regressions on Linux and leaves Windows execution to CI. Revert this commit to restore the previous connection behavior. Fixes #239. Refs #171, #114, #226, #240. --- .devcontainer/README.md | 5 +- .devcontainer/devcontainer.json | 6 + .env.local.example | 9 +- .llm/context.md | 3 +- .llm/mcp/README.md | 27 ++- .llm/mcp/configure.mjs | 68 ++++--- .llm/mcp/mcp-doctor.mjs | 76 ++++---- .llm/mcp/unity-mcp-host.mjs | 180 +++++++++++++----- docs/performance.md.meta | 2 + scripts/harness_scope.py | 1 + scripts/tests/test_mcp_sync.py | 31 ++- scripts/tests/test_unity_mcp_doctor.py | 53 ++++-- scripts/tests/test_unity_mcp_host.py | 218 ++++++++++++++++++++++ scripts/tests/test_unity_mcp_host.py.meta | 2 + scripts/unity_mcp_doctor.py | 78 +++++--- 15 files changed, 585 insertions(+), 174 deletions(-) create mode 100644 docs/performance.md.meta create mode 100644 scripts/tests/test_unity_mcp_host.py create mode 100644 scripts/tests/test_unity_mcp_host.py.meta diff --git a/.devcontainer/README.md b/.devcontainer/README.md index 98aa6dae..ec8a009a 100644 --- a/.devcontainer/README.md +++ b/.devcontainer/README.md @@ -80,7 +80,10 @@ npm run unity:mcp:host # bridges `unity mcp` on http://0.0.0.0:9020/ unity mcp configure ``` -Container agents connect to `http://host.docker.internal:9020/mcp` (configured automatically by `npm run mcp:sync`). Full details: `.llm/mcp/README.md`. +Container agents connect to `http://host.docker.internal:/mcp` (default 9020, configured automatically by `npm run mcp:sync`). Automatic port forwarding, restoration, and +forwarding on URL open are disabled: these create host-to-container tunnels, while Unity needs container-to-host access. A tunnel on the bridge port can shadow the Windows host +listener and return no HTTP bytes. If an older session still forwards a bridge port, stop it in VS Code's Ports view, then reload the window. Explicit forwarding for container +services remains available in that view. Full details: `.llm/mcp/README.md`. ## Host networking note (Linux) diff --git a/.devcontainer/devcontainer.json b/.devcontainer/devcontainer.json index 345219bc..e0accb1e 100644 --- a/.devcontainer/devcontainer.json +++ b/.devcontainer/devcontainer.json @@ -63,6 +63,12 @@ "sst-dev.opencode" ], "settings": { + // Unity runs on the host. Reverse tunnels can shadow its + // wildcard bridge listener with a broken loopback listener. + // Keep deliberate forwarding available through the Ports view. + "remote.autoForwardPorts": false, + "remote.restoreForwardedPorts": false, + "remote.forwardOnOpen": false, // ============================================================ // Appearance // ============================================================ diff --git a/.env.local.example b/.env.local.example index f536fbf1..924b0be0 100644 --- a/.env.local.example +++ b/.env.local.example @@ -44,9 +44,9 @@ CONTEXT7_API_KEY= # --------------------------------------------------------------------------- # Unity MCP bridge — shared token between Unity editors, host tooling, and -# devcontainer agents. `npm run mcp:sync` auto-generates a secure value if -# missing; keep host and container copies identical (same .env.local when the -# package folder is shared). +# devcontainer agents. Optional: set before starting the bridge, then keep +# host and container copies identical (same .env.local when shared). Sync +# preserves this choice and does not create or rotate the token. # --------------------------------------------------------------------------- UNITY_MCP_TOKEN= @@ -63,8 +63,7 @@ UNITY_MCP_TOKEN= # UNITY_PROJECT_PATH=C:\Path\To\UnityProject # Optional overrides (sensible defaults apply): -# UNITY_MCP_TCP_HOST=host.docker.internal # how container agents reach host editors -# UNITY_MCP_BIND=loopback # or `all` (requires UNITY_MCP_TOKEN) +# UNITY_MCP_HTTP_URL=http://host.docker.internal:9020/mcp # endpoint override read by sync and both doctors # UNITY_MCP_HTTP_PORT=9020 # npm run unity:mcp:host HTTP bridge port; when busy the bridge picks the next free port and persists the choice back to this file # UNITY_MCP_TIMEOUT_MS=30000 # editor command timeout diff --git a/.llm/context.md b/.llm/context.md index 800062d1..dfaca585 100644 --- a/.llm/context.md +++ b/.llm/context.md @@ -216,4 +216,5 @@ runs everything, including the Prettier Markdown check, on ubuntu and windows. C gitignored and must never be committed. - Secrets live in gitignored `.env.local` (template: `.env.local.example`); config files reference env vars instead of embedding credentials. - Unity MCP: the **official Unity CLI** MCP server (`unity mcp`, ~140 tools). Container agents reach it through the host HTTP bridge (`npm run unity:mcp:host` → - `host.docker.internal:`, default 9020 via `UNITY_MCP_HTTP_PORT`); host GUI clients use `unity mcp configure ` directly. See [mcp/README.md](./mcp/README.md). + `host.docker.internal:`, default 9020 via `UNITY_MCP_HTTP_PORT`); all six managed frontends share this bridge. Unmanaged host clients can use `unity mcp configure ` + directly. See [mcp/README.md](./mcp/README.md). diff --git a/.llm/mcp/README.md b/.llm/mcp/README.md index 8f8d8a0b..590a9126 100644 --- a/.llm/mcp/README.md +++ b/.llm/mcp/README.md @@ -84,11 +84,11 @@ first-run approval prompt. The isolated Claude state directories used by the `cl the same trust + approvals by `python3 .devcontainer/ai_backends.py install` (and on every launcher start), because Claude only reads that state from the config dir its `CLAUDE_CONFIG_DIR` points at. -Windsurf and other host GUI clients are not writable from the container; run the official configurator on the host instead (see below). +Unmanaged Windsurf and other host GUI clients are not writable from the container; run the official configurator on the host instead (see below). -`fetch`/`git` are only registered when `uvx` is on PATH (baked into the devcontainer image). Cursor's project config only carries host-executable entries (it omits `zai-image`, -whose script path is container-local). Merging preserves servers configured by other tools. The Z.ai credential is read from `ZAI_API_KEY` or `Z_AI_API_KEY`; `npm run mcp:sync` -mirrors whichever exists into the other inside `.env.local`. +`fetch`/`git` are only registered when `uvx` is on PATH (baked into the devcontainer image). Cursor's project config uses the same pinned Unity HTTP bridge and only carries +host-executable stdio entries (it omits `zai-image`, whose script path is container-local). Merging preserves servers configured by other tools. The Z.ai credential is read from +`ZAI_API_KEY` or `Z_AI_API_KEY`; `npm run mcp:sync` mirrors whichever exists into the other inside `.env.local`. ## Unity MCP server (official Unity CLI) @@ -101,7 +101,7 @@ materials, animation, packages, tests, builds, plus an arbitrary C# `eval` tool) # 1. Install the pipeline package into the Unity project cd /path/to/UnityProject && unity pipeline install -# 2. Point host-side GUI clients (Cursor, Windsurf, Claude Desktop, …) at it +# 2. For unmanaged host GUI clients (Windsurf, Claude Desktop, …) only unity mcp configure # `unity mcp configure --list` for all ``` @@ -115,9 +115,15 @@ npm run unity:mcp:host # host: spawns `unity mcp`, serves http://0.0.0.0:9020/ ``` Container agents connect to `http://host.docker.internal:9020/mcp` (written automatically by `npm run mcp:sync`). The bridge is transparent: JSON-RPC requests are forwarded to the -CLI with per-request id remapping, notifications are passed through, and a crashed CLI is respawned lazily on the next request. +CLI with per-request id remapping, client notifications are passed through, and a crashed CLI is respawned lazily on the next request. Startup also verifies a proxy-free local HTTP +roundtrip against a unique bridge instance ID before updating client configs. `/healthz` includes `bridgePid` and `instanceId`; `pid` identifies the lazy Unity CLI child. A failed +self-check exits with code 1. Port persistence uses asynchronous file I/O so it cannot block HTTP handling. -Environment (prefer a gitignored `.env.local` at the workspace root): +Environment (process variables override the gitignored workspace `.env.local`): + +Keep the bridge URL, port, and token in the workspace `.env.local` so host and container clients read the same settings. The legacy host file `~/.unity-mcp/.env.local` is only a +fallback for CLI launch settings (`UNITY_CLI`, `UNITY_MCP_PROJECT`, `UNITY_PROJECT_PATH`, `UNITY_MCP_CLI_ARGS`); workspace values take precedence. Home-only transport settings are +ignored because frontend sync cannot read a Windows host home from the container. | Variable | Purpose | | ---------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------- | @@ -125,6 +131,7 @@ Environment (prefer a gitignored `.env.local` at the workspace root): | `UNITY_MCP_CLI_ARGS` | Extra args appended after `mcp` (e.g. `--instance host:port`) | | `UNITY_MCP_PROJECT` | `--project-path` value; must be the project **root** (`UNITY_PROJECT_PATH` alias honored; non-root values self-heal to the enclosing project) | | `UNITY_MCP_HTTP_PORT` | Bridge port (default 9020). When busy, the bridge binds the next free port, persists the choice here, and re-syncs client configs (`UNITY_MCP_AUTO_SYNC=0` disables) | +| `UNITY_MCP_HTTP_URL` | Endpoint override read from process env or `.env.local` by sync and both doctors | | `UNITY_MCP_TOKEN` | Optional bearer token required by the bridge when set | | `UNITY_MCP_TIMEOUT_MS` | Per-request timeout (default 180000) | @@ -160,3 +167,9 @@ Security: the bridge listens on all interfaces by default (Linux Docker cannot r The previous hand-rolled `UnityMcpBridge.cs` (10 tools) is deprecated. If it was installed into the Unity project, delete `/Assets/Editor/UnityMcpBridge.cs` (and its `.meta`) — it is no longer installed or served by anything in this repo. + +### Diagnose connection failures + +Run `npm run mcp:sync`, restart the host bridge and agent sessions, then run `npm run unity:mcp:doctor` from the container. The doctor checks initialization, tool discovery, and +the pinned editor. If `/healthz` times out, check the host URL and remove forwarded Unity bridge ports in VS Code's Ports view: reverse tunnels can intercept host traffic. Connect +directly to the host bridge. Match health's `bridgePid` to the listener owner; health bypasses authentication and does not start Unity. diff --git a/.llm/mcp/configure.mjs b/.llm/mcp/configure.mjs index 08684e68..42be3df6 100644 --- a/.llm/mcp/configure.mjs +++ b/.llm/mcp/configure.mjs @@ -15,10 +15,9 @@ // Unity uses the official Unity CLI MCP server (`unity mcp`, ~140 tools on a // typical project). Container frontends reach it through the host HTTP bridge // (npm run unity:mcp:host → http://host.docker.internal:/mcp, port from -// UNITY_MCP_HTTP_PORT); host-side GUI clients (Cursor/Windsurf/Claude Desktop) -// should use the CLI directly via `unity mcp configure ` on the host. +// UNITY_MCP_HTTP_PORT). Every managed frontend shares the pinned bridge; +// unmanaged host GUI clients can use `unity mcp configure `. -import crypto from "node:crypto"; import fs from "node:fs"; import os from "node:os"; import path from "node:path"; @@ -97,22 +96,10 @@ function hasCommand(command) { } function ensureUnityMcpToken() { - const existing = env("UNITY_MCP_TOKEN"); - if (existing) return existing; - if (CHECK_ONLY) return "(generated)"; - const token = crypto.randomBytes(24).toString("hex"); - const envLocalPath = path.join(WORKSPACE_ROOT, ".env.local"); - let content = ""; - try { - content = fs.readFileSync(envLocalPath, "utf8"); - } catch { - content = "# Machine-local credentials (gitignored).\n"; - } - if (content.length > 0 && !content.endsWith("\n")) content += "\n"; - content += `\n# Auto-generated by configure.mjs on ${new Date().toISOString()}\nUNITY_MCP_TOKEN=${token}\n`; - fs.mkdirSync(path.dirname(envLocalPath), { recursive: true }); - fs.writeFileSync(envLocalPath, content, { mode: 0o600 }); - return token; + // Authentication is a host choice. Sync must not introduce a token after + // the bridge has already started without one, or require an absent env + // variable in clients that reject missing bearer_token_env_var values. + return env("UNITY_MCP_TOKEN"); } // The Z.ai credential circulates under two names: the MCP configs and the @@ -195,7 +182,7 @@ const UNITY_MCP_PORT = Number(env("UNITY_MCP_HTTP_PORT")) || 9020; // container agents — so it is never the default. Override the whole URL with // UNITY_MCP_HTTP_URL for exotic topologies. const UNITY_MCP_URL = - process.env.UNITY_MCP_HTTP_URL || + env("UNITY_MCP_HTTP_URL") || `http://host.docker.internal:${UNITY_MCP_PORT}/mcp`; // VS Code spawns MCP servers from the extension host, whose environment is @@ -211,20 +198,25 @@ function vscodeEnvWrapper(command) { return ["bash", "-c", `${VSCODE_ENV_WRAPPER_PREFIX}; exec ${command}`]; } +function shellQuote(value) { + return "'" + value.replaceAll("'", "'\\''") + "'"; +} + function buildCatalog() { const servers = new Map(); + const unityAuth = Boolean(env("UNITY_MCP_TOKEN")); servers.set("unity", { summary: `Unity official CLI MCP (unity mcp, ~140 tools) via HTTP bridge ${UNITY_MCP_URL}`, claude: { type: "http", url: UNITY_MCP_URL, - headers: { + ...(unityAuth && { headers: { Authorization: "Bearer ${UNITY_MCP_TOKEN:-}", - }, + } }), }, codex: { url: UNITY_MCP_URL, - bearer_token_env_var: "UNITY_MCP_TOKEN", + ...(unityAuth && { bearer_token_env_var: "UNITY_MCP_TOKEN" }), }, opencode: { type: "remote", @@ -232,30 +224,29 @@ function buildCatalog() { enabled: true, oauth: false, timeout: 300000, - headers: { + ...(unityAuth && { headers: { Authorization: "Bearer {env:UNITY_MCP_TOKEN}", - }, + } }), }, nanocoder: { transport: "http", url: UNITY_MCP_URL, - headers: { + ...(unityAuth && { headers: { Authorization: "Bearer ${UNITY_MCP_TOKEN:-}", - }, + } }), }, vscode: { // stdio wrapper (mcp-remote) instead of native remote HTTP: the // extension host's frozen environment cannot provide // ${env:UNITY_MCP_TOKEN}, but the wrapper sources .env.local. command: vscodeEnvWrapper( - `npx -y mcp-remote ${UNITY_MCP_URL} --header "Authorization:\${UNITY_MCP_TOKEN}"`, + `npx -y mcp-remote ${shellQuote(UNITY_MCP_URL)}${unityAuth ? ' --header "Authorization: Bearer ${UNITY_MCP_TOKEN}"' : ''}`, ), }, - // Cursor runs on the host, so it can launch the CLI directly (stdio) - // without going through the HTTP bridge. `unity mcp configure cursor` - // on the host rewrites this with the absolute CLI path if `unity` is - // not on the host PATH. - cursor: { command: "unity", args: ["mcp"] }, + cursor: { + url: UNITY_MCP_URL, + ...(unityAuth && { headers: { Authorization: "Bearer ${env:UNITY_MCP_TOKEN}" } }), + }, }); servers.set("github", { summary: "GitHub platform (repos, PRs, issues, actions, security…) via hosted MCP, all toolsets", @@ -788,7 +779,12 @@ function main() { managedCodex[name] = definition.codex; managedOpencode[name] = opencodeServerV2(definition.opencode); managedNanocoder[name] = definition.nanocoder; - managedVscode[name] = definition.vscode; + // VS Code requires a command string plus args. OpenCode's command + // array shape is not a valid VS Code stdio server definition. + const vscode = definition.vscode; + managedVscode[name] = Array.isArray(vscode.command) + ? { ...vscode, command: vscode.command[0], args: vscode.command.slice(1) } + : vscode; if (definition.cursor) managedCursor[name] = definition.cursor; } @@ -921,7 +917,7 @@ function main() { console.log(" (fetch/git skipped: uvx not found on PATH)"); } console.log( - ` UNITY_MCP_TOKEN: ${unityToken && unityToken !== "(generated)" ? "configured" : "OPEN (no token)"}`, + ` UNITY_MCP_TOKEN: ${unityToken ? "configured" : "OPEN (no token)"}`, ); const describeZai = (status) => { if (status === "missing") return "MISSING (zai-* servers will fail auth)"; @@ -953,7 +949,7 @@ function main() { ` Unity MCP: official Unity CLI (~140 tools) via ${UNITY_MCP_URL} — start the bridge with \`npm run unity:mcp:host\` on the host.`, ); console.log( - " Host GUI clients (Cursor/Windsurf/Claude Desktop): run `unity mcp configure ` on the host for native stdio.", + " Managed Cursor shares the bridge; unmanaged host clients can run `unity mcp configure ` for native stdio.", ); console.log(""); console.log(" Restart your agent CLIs (or run /mcp) to pick up changes."); diff --git a/.llm/mcp/mcp-doctor.mjs b/.llm/mcp/mcp-doctor.mjs index 786e8ba2..efb02ac9 100644 --- a/.llm/mcp/mcp-doctor.mjs +++ b/.llm/mcp/mcp-doctor.mjs @@ -431,52 +431,50 @@ await spawnProbe("zai-image", "node", [".llm/mcp/zai-image-mcp.mjs"], { // --------------------------------------------------------------------------- console.log("--- Unity bridge ---"); const port = Number(env("UNITY_MCP_HTTP_PORT")) || 9020; -const bridgeBase = `http://host.docker.internal:${port}`; +const bridgeUrl = env("UNITY_MCP_HTTP_URL") || `http://host.docker.internal:${port}/mcp`; +const bridgeBase = bridgeUrl.slice(0, bridgeUrl.lastIndexOf("/")); try { const res = await fetch(`${bridgeBase}/healthz`, { signal: AbortSignal.timeout(5000), }); - const body = await res.json(); - if (body.ok) { - ok(`unity bridge: ${bridgeBase}/healthz ok=true`); - } else { - // The CLI child is spawned lazily on the first request; drive one to - // prove the stack end-to-end instead of warning on the idle state. - const token = env("UNITY_MCP_TOKEN"); - const mcpRes = await fetch(`${bridgeBase}/mcp`, { - method: "POST", - headers: { - "content-type": "application/json", - accept: "application/json, text/event-stream", - ...(token ? { authorization: `Bearer ${token}` } : {}), + await res.json(); + if (!res.ok) throw new Error(`health HTTP ${res.status}`); + // Always drive initialize: a live CLI pid does not prove authentication + // or JSON-RPC works for this client. + const token = env("UNITY_MCP_TOKEN"); + const mcpRes = await fetch(bridgeUrl, { + method: "POST", + headers: { + "content-type": "application/json", + accept: "application/json, text/event-stream", + ...(token ? { authorization: `Bearer ${token}` } : {}), + }, + body: JSON.stringify({ + jsonrpc: "2.0", + id: 1, + method: "initialize", + params: { + protocolVersion: "2025-03-26", + capabilities: {}, + clientInfo: { name: "mcp-doctor", version: "0.1.0" }, }, - body: JSON.stringify({ - jsonrpc: "2.0", - id: 1, - method: "initialize", - params: { - protocolVersion: "2025-03-26", - capabilities: {}, - clientInfo: { name: "mcp-doctor", version: "0.1.0" }, - }, - }), - signal: AbortSignal.timeout(30000), - }); - const mcpBody = await mcpRes.json(); - if (mcpBody?.result?.serverInfo) { - ok( - `unity bridge: ${bridgeBase}/mcp initialize OK ` + - `serverInfo=${JSON.stringify(mcpBody.result.serverInfo)} (child was lazily idle)`, - ); - } else { - fail( - `unity bridge responded but MCP initialize failed: ${JSON.stringify(mcpBody).slice(0, 200)}`, - ); - } + }), + signal: AbortSignal.timeout(30000), + }); + const mcpBody = await mcpRes.json(); + if (mcpBody?.result?.serverInfo) { + ok( + `unity bridge: ${bridgeBase}/mcp initialize OK ` + + `serverInfo=${JSON.stringify(mcpBody.result.serverInfo)}`, + ); + } else { + fail( + `unity bridge responded but MCP initialize failed: ${JSON.stringify(mcpBody).slice(0, 200)}`, + ); } -} catch { +} catch (error) { fail( - `unity bridge not reachable on host.docker.internal:${port}. On the host run: ` + + `unity bridge failed at ${bridgeBase}: ${error.message}. On the host run: ` + "npm run unity:mcp:host (then unity:mcp:doctor for depth).", ); } diff --git a/.llm/mcp/unity-mcp-host.mjs b/.llm/mcp/unity-mcp-host.mjs index 55394d7a..b67787bc 100644 --- a/.llm/mcp/unity-mcp-host.mjs +++ b/.llm/mcp/unity-mcp-host.mjs @@ -47,12 +47,12 @@ import crypto from "node:crypto"; import fs from "node:fs"; import os from "node:os"; import path from "node:path"; -import { spawn, spawnSync } from "node:child_process"; +import { spawn } from "node:child_process"; import { fileURLToPath } from "node:url"; const SCRIPT_DIR = path.dirname(fileURLToPath(import.meta.url)); const WORKSPACE_ROOT = path.resolve(SCRIPT_DIR, "..", ".."); -const REQUEST_TIMEOUT_MS = Number(process.env.UNITY_MCP_TIMEOUT_MS || 180000); +const INSTANCE_ID = crypto.randomUUID(); function readEnvLocal(filePath) { const vars = {}; @@ -62,12 +62,12 @@ function readEnvLocal(filePath) { } catch { return vars; } - for (const rawLine of text.split(/\r?\n/)) { + for (const rawLine of text.replace(/^\uFEFF/, "").split(/\r?\n/)) { const line = rawLine.trim(); if (!line || line.startsWith("#")) continue; const eq = line.indexOf("="); if (eq <= 0) continue; - const key = line.slice(0, eq).trim(); + const key = line.slice(0, eq).trim().replace(/^export\s+/, ""); let value = line.slice(eq + 1).trim(); if ( (value.startsWith('"') && value.endsWith('"')) || @@ -80,10 +80,17 @@ function readEnvLocal(filePath) { return vars; } +// Only host launch settings may fall back to the legacy home file. Shared +// transport settings must match configure.mjs and the container workspace. +const hostEnvLocal = readEnvLocal(path.join(os.homedir(), ".unity-mcp", ".env.local")); const envLocal = { + UNITY_CLI: hostEnvLocal.UNITY_CLI, + UNITY_MCP_PROJECT: hostEnvLocal.UNITY_MCP_PROJECT, + UNITY_PROJECT_PATH: hostEnvLocal.UNITY_PROJECT_PATH, + UNITY_MCP_CLI_ARGS: hostEnvLocal.UNITY_MCP_CLI_ARGS, ...readEnvLocal(path.join(WORKSPACE_ROOT, ".env.local")), - ...readEnvLocal(path.join(os.homedir(), ".unity-mcp", ".env.local")), }; +const REQUEST_TIMEOUT_MS = Number(process.env.UNITY_MCP_TIMEOUT_MS || envLocal.UNITY_MCP_TIMEOUT_MS || 180000); // Preferred HTTP port: process env wins, then .env.local, then the historical // default of 9020. When the preferred port is busy the bridge binds the next @@ -108,7 +115,7 @@ function timingSafeEqual(a, b) { // CLI process management // --------------------------------------------------------------------------- -const cliCommand = process.env.UNITY_CLI || "unity"; +const cliCommand = process.env.UNITY_CLI || envLocal.UNITY_CLI || "unity"; const cliArgs = ["mcp"]; function isUnityProjectRoot(dir) { @@ -179,8 +186,9 @@ if (!projectPath) { } } if (projectPath) cliArgs.push("--project-path", projectPath); -if (process.env.UNITY_MCP_CLI_ARGS) { - cliArgs.push(...process.env.UNITY_MCP_CLI_ARGS.split(" ").filter(Boolean)); +const extraCliArgs = process.env.UNITY_MCP_CLI_ARGS || envLocal.UNITY_MCP_CLI_ARGS; +if (extraCliArgs) { + cliArgs.push(...extraCliArgs.split(" ").filter(Boolean)); } let child = null; @@ -212,10 +220,22 @@ function spawnChild() { } lastSpawnAt = Date.now(); failPending("Unity CLI restarted; retry the request."); - child = spawn(cliCommand, cliArgs, { + stdoutBuffer = ""; + const spawned = spawn(cliCommand, cliArgs, { stdio: ["pipe", "pipe", "pipe"], env: process.env, }); + child = spawned; + // A closed pipe must fail the current requests instead of crashing the + // HTTP bridge with an unhandled EPIPE. Old children cannot clear a newer + // child's state when their exit events arrive late. + spawned.stdin.on("error", (error) => { + if (child !== spawned) return; + child = null; + stdoutBuffer = ""; + failPending(`Unity CLI input closed: ${error.message}; retry the request.`); + spawned.kill(); + }); child.stdout.setEncoding("utf8"); child.stderr.setEncoding("utf8"); child.stdout.on("data", (chunk) => { @@ -259,12 +279,15 @@ function spawnChild() { process.stderr.write(`[unity mcp] ${chunk}`); }); child.on("exit", (code, signal) => { + if (child !== spawned) return; child = null; + stdoutBuffer = ""; failPending( `Unity CLI exited (code=${code ?? "null"}${signal ? `, signal=${signal}` : ""}); retry the request.`, ); }); child.on("error", (error) => { + if (child !== spawned) return; process.stderr.write(`[unity-mcp] failed to spawn ${cliCommand}: ${error.message}\n`); const failedChild = child; child = null; @@ -304,17 +327,16 @@ function forwardRequest(message) { pending.set(internalId, { resolve, timer, originalId: message.id }); child.stdin.write(`${JSON.stringify({ ...message, id: internalId })}\n`); } catch (error) { - if (internalId) pending.delete(internalId); + if (internalId) { + const entry = pending.get(internalId); + if (entry) clearTimeout(entry.timer); + pending.delete(internalId); + } resolve(jsonRpcError(message.id, -32000, error?.message || String(error))); } }); } -function ensureChild() { - if (!child || !child.stdin.writable) spawnChild(); - return child; -} - // --------------------------------------------------------------------------- // HTTP MCP endpoint (stateless streamable-HTTP subset: POST + DELETE) // --------------------------------------------------------------------------- @@ -350,8 +372,13 @@ function startHttpServer(port, host, onListening) { response.writeHead(200, { "content-type": "application/json" }); response.end( JSON.stringify({ + bridge: "dataviz-unity-mcp", + instanceId: INSTANCE_ID, + bridgePid: process.pid, ok: Boolean(child && child.exitCode === null), cli: `${cliCommand} ${cliArgs.join(" ")}`.trim(), + projectPath, + authRequired: Boolean(token), pid: child?.pid ?? null, }), ); @@ -361,14 +388,6 @@ function startHttpServer(port, host, onListening) { response.writeHead(404).end(); return; } - if (request.method === "DELETE") { - response.writeHead(204).end(); - return; - } - if (request.method !== "POST") { - response.writeHead(405, { allow: "POST, DELETE" }).end(); - return; - } if (token) { const authorization = request.headers.authorization || ""; const provided = authorization.startsWith("Bearer ") @@ -380,6 +399,14 @@ function startHttpServer(port, host, onListening) { return; } } + if (request.method === "DELETE") { + response.writeHead(204).end(); + return; + } + if (request.method !== "POST") { + response.writeHead(405, { allow: "POST, DELETE" }).end(); + return; + } const chunks = []; request.on("data", (chunk) => chunks.push(chunk)); request.on("end", () => { @@ -393,18 +420,10 @@ function startHttpServer(port, host, onListening) { ); return; } - try { - ensureChild(); - } catch (error) { - response.writeHead(200, { "content-type": "application/json" }); - response.end(JSON.stringify(jsonRpcError(null, -32000, error.message))); - return; - } handleRpcMessage(message) .then((result) => { if (result === null) { - response.writeHead(202, { "content-type": "application/json" }); - response.end("{}"); + response.writeHead(202).end(); return; } response.writeHead(200, { "content-type": "application/json" }); @@ -424,7 +443,23 @@ function startHttpServer(port, host, onListening) { console.log(`[unity-mcp] HTTP MCP server listening on http://${displayedHost}:${port}/mcp`); console.log(`[unity-mcp] Health: http://${displayedHost}:${port}/healthz`); console.log(`[unity-mcp] Auth: ${token ? "bearer token required" : "OPEN (no UNITY_MCP_TOKEN configured)"}`); - if (onListening) onListening(); + // A listen callback proves only that a socket was bound. Check the + // actual HTTP response and instance identity before publishing configs. + verifyLocalHttp(port, host) + .then(async () => { + console.log(`[unity-mcp] Local HTTP self-check passed (bridge PID ${process.pid}, instance ${INSTANCE_ID}).`); + if (onListening) await onListening(); + console.log("[unity-mcp] Startup complete."); + }) + .catch((error) => { + console.error(`[unity-mcp] Startup failed: ${error.message}`); + console.error("[unity-mcp] Check the port's owning process and VS Code Ports view for a reverse tunnel. Unity has not been verified."); + process.exitCode = 1; + failPending("Bridge startup failed."); + child?.kill(); + server.closeAllConnections(); + server.close(); + }); }); const shutdown = () => { try { @@ -443,15 +478,56 @@ function startHttpServer(port, host, onListening) { // Startup: idempotent start, dynamic port fallback, persistence, client sync // --------------------------------------------------------------------------- -function isPortFree(port, host) { +function verifyLocalHttp(port, host) { + const hostname = host === "0.0.0.0" ? "127.0.0.1" : host === "::" ? "::1" : host; + return new Promise((resolve, reject) => { + // Core http bypasses HTTP_PROXY, ALL_PROXY, and fetch dispatchers. + const request = http.get({ + hostname, port, path: "/healthz", agent: false, + signal: AbortSignal.timeout(3000), + }, (response) => { + let body = ""; + response.setEncoding("utf8"); + response.on("error", reject); + response.on("data", (chunk) => { body += chunk; }); + response.on("end", () => { + try { + const health = JSON.parse(body); + if (response.statusCode !== 200 || health.instanceId !== INSTANCE_ID) { + throw new Error("the loopback endpoint did not answer as this bridge instance"); + } + resolve(); + } catch (error) { + reject(new Error(`Local HTTP self-check failed on ${hostname}:${port}: ${error.message}`)); + } + }); + }); + request.on("error", (error) => reject(new Error( + `Local HTTP self-check failed on ${hostname}:${port}: ${error.message}`, + ))); + }); +} + +function canBindPort(port, host) { return new Promise((resolve) => { const probe = net.createServer(); probe.once("error", () => resolve(false)); probe.once("listening", () => probe.close(() => resolve(true))); - probe.listen(port, host); + probe.listen({ port, host, exclusive: true }); }); } +// Windows can bind a wildcard socket beside a loopback-only listener. The +// specific listener then receives loopback traffic instead of this bridge. +// Match DxCommandTerminal's check: reserve both addresses before choosing a port. +async function isPortFree(port, host) { + if (!(await canBindPort(port, host))) return false; + if (host === "0.0.0.0" || host === "::" || host === "localhost") { + return canBindPort(port, "127.0.0.1"); + } + return true; +} + // Detect an already-running unity-mcp bridge (ours) so re-running the npm // script is a no-op instead of a second instance or an EADDRINUSE crash. async function probeExistingBridge(port) { @@ -461,7 +537,8 @@ async function probeExistingBridge(port) { }); if (!response.ok) return false; const body = await response.json(); - return typeof body?.cli === "string" && body.cli.startsWith("unity mcp"); + return body?.bridge === "dataviz-unity-mcp" || + (typeof body?.cli === "string" && /(?:^|[/\\])unity(?:\.exe)? mcp(?: |$)/.test(body.cli)); } catch { return false; } @@ -471,12 +548,12 @@ async function probeExistingBridge(port) { // and every generated frontend config follow it. Idempotent; a no-op when the // recorded value already matches or when the port is the 9020 default and no // override exists. -function persistPort(port) { +async function persistPort(port) { const envLocalPath = path.join(WORKSPACE_ROOT, ".env.local"); const line = `UNITY_MCP_HTTP_PORT=${port}`; let content = ""; try { - content = fs.readFileSync(envLocalPath, "utf8"); + content = await fs.promises.readFile(envLocalPath, "utf8"); } catch { content = "# Machine-local credentials (gitignored).\n"; } @@ -490,7 +567,7 @@ function persistPort(port) { content += `\n# Auto-persisted by unity-mcp-host.mjs (dynamic port) on ${new Date().toISOString()}\n${line}\n`; } try { - fs.writeFileSync(envLocalPath, content, "utf8"); + await fs.promises.writeFile(envLocalPath, content, "utf8"); } catch (error) { process.stderr.write( `[unity-mcp] Could not persist port to .env.local: ${error.message}\n`, @@ -502,12 +579,19 @@ function persistPort(port) { // Re-run the universal configurator so every agentic frontend config picks up // the new unity bridge port. Cross-platform: runs configure.mjs directly. -function syncClientConfigs() { - const result = spawnSync(process.execPath, [path.join(SCRIPT_DIR, "configure.mjs")], { - cwd: WORKSPACE_ROOT, - stdio: "inherit", +function syncClientConfigs(port) { + // Configuration can stall on host filesystem or command discovery. Keep + // the HTTP event loop responsive while it runs, with a bounded lifetime. + return new Promise((resolve) => { + const sync = spawn(process.execPath, [path.join(SCRIPT_DIR, "configure.mjs")], { + cwd: WORKSPACE_ROOT, + stdio: "inherit", + env: { ...process.env, UNITY_MCP_HTTP_PORT: String(port) }, + timeout: 30000, + }); + sync.once("error", () => resolve(false)); + sync.once("exit", (code) => resolve(code === 0)); }); - return !result.error && result.status === 0; } async function main() { @@ -577,17 +661,17 @@ Environment: port = allocated; } - startHttpServer(port, host, () => { - if (!persistPort(port)) return; + startHttpServer(port, host, async () => { + if (!(await persistPort(port))) return; console.log( `[unity-mcp] Persisted UNITY_MCP_HTTP_PORT=${port} to .env.local so client configs follow.`, ); - if (process.env.UNITY_MCP_AUTO_SYNC === "0") { + if ((process.env.UNITY_MCP_AUTO_SYNC || envLocal.UNITY_MCP_AUTO_SYNC) === "0") { console.log("[unity-mcp] AUTO_SYNC disabled; run `npm run mcp:sync` to update MCP clients."); return; } console.log("[unity-mcp] Syncing MCP client configs across agentic frontends…"); - if (syncClientConfigs()) { + if (await syncClientConfigs(port)) { console.log("[unity-mcp] MCP client configs updated (unity URL follows the new port)."); } else { console.log("[unity-mcp] ⚠️ Config sync failed; run `npm run mcp:sync` manually."); diff --git a/docs/performance.md.meta b/docs/performance.md.meta new file mode 100644 index 00000000..a068007b --- /dev/null +++ b/docs/performance.md.meta @@ -0,0 +1,2 @@ +fileFormatVersion: 2 +guid: 13bf2f913ca953845971d1e25975d626 diff --git a/scripts/harness_scope.py b/scripts/harness_scope.py index 1f91496c..bc0dd302 100644 --- a/scripts/harness_scope.py +++ b/scripts/harness_scope.py @@ -59,6 +59,7 @@ # companion on the same docs contract; unknown metadata stays fail-safe. "scripts/tests/test_build_docs.py.meta": ("test_build_docs.py",), "scripts/unity_mcp_doctor.py": ("test_unity_mcp_doctor.py",), + ".llm/mcp/unity-mcp-host.mjs": ("test_unity_mcp_host.py",), ".llm/mcp/unity-mcp-doctor.mjs": ("test_unity_mcp_doctor.py",), "scripts/run_ci_lints.py": ("test_run_ci_lints.py",), "scripts/run_ci_lints.py.meta": ("test_run_ci_lints.py",), diff --git a/scripts/tests/test_mcp_sync.py b/scripts/tests/test_mcp_sync.py index 2adf2a62..86e9aa64 100644 --- a/scripts/tests/test_mcp_sync.py +++ b/scripts/tests/test_mcp_sync.py @@ -37,7 +37,7 @@ # it so the sandboxed configure.mjs run and these assertions stay in lockstep. UNITY_MCP_HTTP_PORT = os.environ.get("UNITY_MCP_HTTP_PORT") or "9020" -SCRUBBED_KEYS = ("GITHUB_PERSONAL_ACCESS_TOKEN", "GITHUB_MCP_PAT", "GITHUB_TOKEN", "ZAI_API_KEY", "Z_AI_API_KEY") +SCRUBBED_KEYS = ("GITHUB_PERSONAL_ACCESS_TOKEN", "GITHUB_MCP_PAT", "GITHUB_TOKEN", "ZAI_API_KEY", "Z_AI_API_KEY", "UNITY_MCP_TOKEN", "UNITY_MCP_HTTP_URL") LEGACY_OPENCODE_JSON = """{ @@ -265,7 +265,7 @@ def case_generated_configs_contain_no_literal_secrets() -> None: ) assert_file_contains(sandbox, ".mcp.json", '"X-MCP-Toolsets": "all"', "claude github enables all toolsets") assert_file_contains(sandbox, "codex-home/config.toml", '"X-MCP-Toolsets" = "all"', "codex github enables all toolsets") - assert_file_contains(sandbox, ".cursor/mcp.json", '"command": "unity"', "cursor unity launches the official CLI (host stdio)") + assert_eq("cursor shares the pinned bridge", sandbox.load_json(".mcp.json")["mcpServers"]["unity"]["url"], sandbox.load_json(".cursor/mcp.json")["mcpServers"]["unity"]["url"]) assert_file_contains(sandbox, "opencode.json", "Bearer {env:ZAI_API_KEY}", "opencode zai auth uses env placeholder") assert_file_contains( sandbox, @@ -291,6 +291,32 @@ def case_generated_configs_contain_no_literal_secrets() -> None: ) +def case_unity_auth_and_url_are_consistent() -> None: + sandbox = Sandbox() + try: + sandbox.path('.env.local').write_text('UNITY_MCP_HTTP_URL=http://bridge:9988/mcp\n', encoding='utf-8') + sandbox.run_sync() + assert_eq('open bridge sync succeeds', 0, sandbox.last_exit, sandbox.run_failure_context()) + assert_true('UNITY_MCP_TOKEN=' not in sandbox.path('.env.local').read_text(encoding='utf-8'), 'sync does not change host authentication') + assert_true('bearer_token_env_var = "UNITY_MCP_TOKEN"' not in sandbox.path('codex-home/config.toml').read_text(encoding='utf-8'), 'open bridge does not require a token in Codex') + for relative, key in [('.mcp.json', 'mcpServers'), ('.cursor/mcp.json', 'mcpServers'), ('.nanocoder/mcp.json', 'mcpServers')]: + entry = sandbox.load_json(relative)[key]['unity'] + assert_eq(f'{relative} honors local endpoint', 'http://bridge:9988/mcp', entry['url']) + assert_true('headers' not in entry, f'{relative} omits auth for open bridge') + sandbox.path('.env.local').write_text('UNITY_MCP_HTTP_URL=http://bridge:9988/mcp\nUNITY_MCP_TOKEN=fixture-token\n', encoding='utf-8') + sandbox.run_sync() + assert_eq('authenticated bridge sync succeeds', 0, sandbox.last_exit, sandbox.run_failure_context()) + vscode = sandbox.load_json('.vscode/mcp.json')['servers']['unity']['args'][-1] + for name, definition in sandbox.load_json('.vscode/mcp.json')['servers'].items(): + if 'command' in definition: + assert_true(isinstance(definition['command'], str), f'VS Code {name} has a command string') + assert_true('Authorization: Bearer ${UNITY_MCP_TOKEN}' in vscode, 'VS Code sends the bearer scheme') + assert_true('fixture-token' not in vscode, 'VS Code loads credentials at spawn') + assert_file_contains(sandbox, 'codex-home/config.toml', 'bearer_token_env_var = "UNITY_MCP_TOKEN"', 'authenticated Codex uses token reference') + finally: + sandbox.destroy() + + def case_wrapper_forwards_workspace_flags_and_exit_status() -> None: root = new_temp_root(prefix="mcp sync wrapper-") try: @@ -370,6 +396,7 @@ def main() -> int: if missing: print(" SKIP: node is required to drive the configurator.") return 1 if failure_count() else 0 + run_case('case_unity_auth_and_url_are_consistent', case_unity_auth_and_url_are_consistent) run_case("case_wrapper_forwards_workspace_flags_and_exit_status", case_wrapper_forwards_workspace_flags_and_exit_status) for case in CASES: run_case(case.__name__, case) diff --git a/scripts/tests/test_unity_mcp_doctor.py b/scripts/tests/test_unity_mcp_doctor.py index 6770d197..148ffecb 100644 --- a/scripts/tests/test_unity_mcp_doctor.py +++ b/scripts/tests/test_unity_mcp_doctor.py @@ -50,10 +50,11 @@ def request(self, url: str, headers: Dict[str, str], payload: Optional[JsonObjec self.calls.append(payload) if method == self.error_method: raise OSError('offline fixture') - return self.responses[method] + return self.responses.get(method, {}) def run_doctor(self, environment: Optional[Dict[str, str]] = None) -> tuple[Doctor, int, str]: - env = {'UNITY_MCP_PROJECT': str(self.project)} if environment is None else environment + env = {'UNITY_MCP_PROJECT': str(self.project)} if environment is None else dict(environment) + env.setdefault('DATAVIZ_IN_CONTAINER', '0') doctor = Doctor(self.root, self.root / 'home', env, self.command, self.request) stream = io.StringIO() with contextlib.redirect_stdout(stream): @@ -64,10 +65,10 @@ def test_healthy_roundtrip_and_token_precedence(self) -> None: (self.root / '.env.local').write_text('UNITY_MCP_TOKEN=local-secret\nUNITY_MCP_HTTP_PORT=9001', encoding='utf-8') doctor, code, output = self.run_doctor({'UNITY_MCP_PROJECT': str(self.project), 'UNITY_MCP_TOKEN': 'process-secret', 'UNITY_MCP_HTTP_PORT': '9002'}) self.assertEqual((doctor.failures, doctor.warnings, code), (0, 0, 0)) - self.assertEqual([call['method'] for call in self.calls], ['initialize', 'tools/list', 'tools/call']) - self.assertEqual([call['id'] for call in self.calls], [1, 2, 3]) + self.assertEqual([call['method'] for call in self.calls], ['initialize', 'notifications/initialized', 'tools/list', 'tools/call']) + self.assertEqual([call['id'] for call in self.calls if 'id' in call], [1, 2, 3]) self.assertEqual(self.calls[0]['params'], {'protocolVersion': '2025-03-26', 'capabilities': {}, 'clientInfo': {'name': 'unity-mcp-doctor', 'version': '0.1.0'}}) - self.assertEqual(self.calls[2]['params'], {'name': 'editor_status', 'arguments': {}}) + self.assertEqual(self.calls[3]['params'], {'name': 'editor_status', 'arguments': {}}) self.assertEqual(self.headers[0], {}) self.assertTrue(all(headers['authorization'] == 'Bearer process-secret' for headers in self.headers[1:])) self.assertTrue(all(url.startswith('http://127.0.0.1:9002/') for url in self.urls)) @@ -77,10 +78,11 @@ def test_healthy_roundtrip_and_token_precedence(self) -> None: def test_project_and_environment_precedence(self) -> None: home = self.root / 'home' / '.unity-mcp' home.mkdir(parents=True) - (self.root / '.env.local').write_text('UNITY_MCP_PROJECT=missing\nUNITY_MCP_HTTP_PORT=9001', encoding='utf-8') - (home / '.env.local').write_text(f'UNITY_MCP_PROJECT="{self.project}"\nUNITY_MCP_HTTP_PORT=9003', encoding='utf-8') + (self.root / '.env.local').write_text(f'UNITY_MCP_PROJECT="{self.project}"\nUNITY_MCP_HTTP_PORT=9001', encoding='utf-8') + (home / '.env.local').write_text('UNITY_MCP_PROJECT=missing\nUNITY_MCP_HTTP_PORT=9003\nUNITY_MCP_TOKEN=stale-home-token', encoding='utf-8') self.assertEqual(self.run_doctor({})[0].project, self.project) - self.assertTrue(self.urls[0].startswith('http://127.0.0.1:9003/')) + self.assertTrue(self.urls[0].startswith('http://127.0.0.1:9001/')) + self.assertTrue(all('authorization' not in headers for headers in self.headers)) self.assertEqual(self.run_doctor({'UNITY_PROJECT_PATH': str(self.project / 'Packages')})[0].project, self.project) doctor, code, output = self.run_doctor({'UNITY_MCP_PROJECT': str(self.root / 'missing'), 'UNITY_PROJECT_PATH': str(self.project)}) self.assertEqual(code, 1) @@ -99,7 +101,7 @@ def test_nested_pin_and_enclosing_fallback_warn(self) -> None: self.assertIsNone(enclosing_project(self.root)) def test_offline_and_rpc_failure_severity(self) -> None: - for method, code, failures, warnings in [('health', 0, 0, 0), ('initialize', 1, 1, 0), ('tools/list', 1, 1, 0), ('tools/call', 0, 0, 1)]: + for method, code, failures, warnings in [('health', 1, 1, 0), ('initialize', 1, 1, 0), ('tools/list', 1, 1, 0), ('tools/call', 1, 1, 0)]: with self.subTest(method=method): self.error_method = method doctor, result, _ = self.run_doctor() @@ -117,16 +119,16 @@ def test_missing_manifest_and_editor_canary_warn(self) -> None: self.manifest.unlink() self.responses['tools/call'] = {'error': {'message': 'not running'}} doctor, code, _ = self.run_doctor() - self.assertEqual((code, doctor.failures, doctor.warnings), (0, 0, 2)) + self.assertEqual((code, doctor.failures, doctor.warnings), (1, 1, 1)) self.manifest.write_text('{"dependencies":{}}', encoding='utf-8') self.assertEqual(self.run_doctor()[1], 1) def test_cli_failure_is_reported_without_stopping_bridge_checks(self) -> None: - doctor = Doctor(self.root, self.root, {'UNITY_MCP_PROJECT': str(self.project)}, self.raise_command_error, self.request) + doctor = Doctor(self.root, self.root, {'UNITY_MCP_PROJECT': str(self.project), 'DATAVIZ_IN_CONTAINER': '0'}, self.raise_command_error, self.request) with contextlib.redirect_stdout(io.StringIO()): code = doctor.run() self.assertEqual((code, doctor.failures, doctor.warnings), (1, 1, 1)) - self.assertEqual(len(self.calls), 3) + self.assertEqual(len(self.calls), 4) def raise_command_error(self, arguments: List[str]) -> str: raise OSError('missing') @@ -145,9 +147,34 @@ def test_http_transport_encodes_authenticated_json(self) -> None: def test_environment_parser_preserves_literal_values(self) -> None: path = self.root / '.env.local' self.assertEqual(read_env_local(path), {}) - path.write_text('# comment\n bad\n=invalid\n A = "x=y"\nB=\'quoted\'\nC=literal # text', encoding='utf-8') + path.write_text('\ufeff# comment\n bad\n=invalid\n export A = "x=y"\nB=\'quoted\'\nC=literal # text', encoding='utf-8') self.assertEqual(read_env_local(path), {'A': 'x=y', 'B': 'quoted', 'C': 'literal # text'}) + def test_container_uses_host_bridge_and_windows_project_paths(self) -> None: + self.responses['tools/call'] = {'result': {'content': [{'text': json.dumps({'projectPath': 'C:/Code/Project'})}]}} + doctor, code, output = self.run_doctor({'DATAVIZ_IN_CONTAINER': '1', 'UNITY_MCP_PROJECT': 'c:\\code\\project'}) + self.assertEqual((code, doctor.failures), (0, 0)) + self.assertTrue(self.urls[0].startswith('http://host.docker.internal:9020/')) + self.assertNotIn('Unity CLI on PATH', output) + + def test_custom_endpoint_and_rpc_errors_fail(self) -> None: + for method in ['initialize', 'tools/list', 'tools/call']: + with self.subTest(method=method): + original = self.responses[method] + self.responses[method] = {'error': {'code': -32000, 'message': 'CLI unavailable'}} + doctor, code, output = self.run_doctor({'DATAVIZ_IN_CONTAINER': '1', 'UNITY_MCP_HTTP_URL': 'http://bridge:9999/custom'}) + self.assertEqual(code, 1) + self.assertTrue(doctor.failures) + self.assertIn('CLI unavailable', output) + self.assertEqual(self.urls[0], 'http://bridge:9999/healthz') + self.responses[method] = original + + def test_editor_tool_error_and_missing_server_info_fail(self) -> None: + self.responses['tools/call'] = {'result': {'isError': True, 'content': [{'text': 'not running'}]}} + self.assertEqual(self.run_doctor()[1], 1) + self.responses['initialize'] = {'result': {}} + self.assertEqual(self.run_doctor()[1], 1) + if __name__ == '__main__': unittest.main() diff --git a/scripts/tests/test_unity_mcp_host.py b/scripts/tests/test_unity_mcp_host.py new file mode 100644 index 00000000..6ab6b788 --- /dev/null +++ b/scripts/tests/test_unity_mcp_host.py @@ -0,0 +1,218 @@ +#!/usr/bin/env python3 +"""Exercise the real HTTP bridge with an isolated, deterministic CLI process.""" +from __future__ import annotations + +import concurrent.futures +import json +import os +import shutil +import socket +import subprocess +import sys +import tempfile +import time +import unittest +from pathlib import Path +from typing import Any, Dict, Optional +from urllib.error import HTTPError +from urllib.request import Request, urlopen + +sys.dont_write_bytecode = True +REPO = Path(__file__).resolve().parents[2] +FAKE_CLI = r""" +const readline = require('node:readline'); +readline.createInterface({ input: process.stdin }).on('line', line => { + const message = JSON.parse(line); + if (message.method === 'crash') { + process.stdout.write('partial output'); + process.exit(1); + } + if (message.id === undefined) return; + const result = message.method === 'initialize' + ? { serverInfo: { name: 'fixture' }, capabilities: {}, protocolVersion: '2025-03-26' } + : { method: message.method, params: message.params }; + setTimeout(() => process.stdout.write(JSON.stringify({ jsonrpc: '2.0', id: message.id, result }) + '\n'), 5); +}); +""" + + +@unittest.skipUnless(shutil.which('node'), 'node is required') +class HostTests(unittest.TestCase): + def setUp(self) -> None: + self.temp = tempfile.TemporaryDirectory(prefix='unity-mcp-host-') + self.addCleanup(self.temp.cleanup) + self.root = Path(self.temp.name) + script_dir = self.root / '.llm' / 'mcp' + script_dir.mkdir(parents=True) + self.script = script_dir / 'unity-mcp-host.mjs' + shutil.copyfile(REPO / '.llm' / 'mcp' / self.script.name, self.script) + (self.root / 'mcp').write_text(FAKE_CLI, encoding='utf-8') + self.home = self.root / 'home' + self.home.mkdir() + self.project = self.root / 'Project' + (self.project / 'ProjectSettings').mkdir(parents=True) + (self.project / 'ProjectSettings' / 'ProjectVersion.txt').write_text('6000.5', encoding='utf-8') + with socket.socket() as listener: + listener.bind(('127.0.0.1', 0)) + self.port = listener.getsockname()[1] + self.env = {key: value for key, value in os.environ.items() if not key.startswith('UNITY_')} + self.env.update(HOME=str(self.home), USERPROFILE=str(self.home), UNITY_MCP_AUTO_SYNC='0') + self.env_file = self.root / '.env.local' + self.env_file.write_text(f'\ufeffexport UNITY_CLI="{shutil.which("node")}"\nUNITY_MCP_PROJECT="{self.project}"\nUNITY_MCP_TOKEN=fixture-secret\nUNITY_MCP_TIMEOUT_MS=1500\n', encoding='utf-8') + self.log = (self.root / 'bridge.log').open('w+', encoding='utf-8') + self.addCleanup(self.log.close) + self.launch() + self.addCleanup(self.stop) + + def launch(self, command: Optional[list[str]] = None) -> None: + offset = len(self.logs()) + self.process = subprocess.Popen(command or self.command(), cwd=self.root, env=self.env, stdout=self.log, stderr=self.log) + deadline = time.monotonic() + 10 + while 'Startup complete.' not in self.logs()[offset:]: + if self.process.poll() is not None or deadline <= time.monotonic(): + self.stop() + self.fail('Bridge did not start: ' + self.logs()[offset:]) + time.sleep(0.02) + + def command(self) -> list[str]: + return [str(shutil.which('node')), str(self.script), '--host', '127.0.0.1', '--port', str(self.port)] + + def logs(self) -> str: + self.log.flush() + return (self.root / 'bridge.log').read_text(encoding='utf-8') + + def stop(self) -> None: + self.process.terminate() + try: + self.process.wait(timeout=5) + except subprocess.TimeoutExpired: + self.process.kill() + self.process.wait(timeout=5) + + def http(self, route: str, payload: Optional[Dict[str, Any]] = None, token: str = 'fixture-secret', method: Optional[str] = None) -> tuple[int, Any]: + headers = {'content-type': 'application/json', 'authorization': f'Bearer {token}'} + data = None if payload is None else json.dumps(payload).encode() + request = Request(f'http://127.0.0.1:{self.port}{route}', data=data, headers=headers, method=method) + with urlopen(request, timeout=5) as response: + body = response.read() + return response.status, json.loads(body) if body else None + + def rpc(self, method: str, identifier: int = 7) -> Any: + return self.http('/mcp', {'jsonrpc': '2.0', 'id': identifier, 'method': method, 'params': {}})[1] + + def test_auth_notifications_and_parallel_request_ids(self) -> None: + health = self.http('/healthz')[1] + self.assertEqual(health['bridge'], 'dataviz-unity-mcp') + self.assertEqual(health['bridgePid'], self.process.pid) + self.assertTrue(health['instanceId']) + self.assertEqual(health['projectPath'], str(self.project)) + self.assertTrue(health['authRequired']) + self.assertFalse(health['ok']) + for method in ['POST', 'DELETE']: + with self.assertRaises(HTTPError) as raised: + self.http('/mcp', {}, token='wrong', method=method) + self.assertEqual(raised.exception.code, 401) + self.assertEqual(self.rpc('initialize')['result']['serverInfo']['name'], 'fixture') + self.assertEqual(self.http('/mcp', {'jsonrpc': '2.0', 'method': 'notifications/initialized'}), (202, None)) + methods = ['first', 'second', 'third'] + with concurrent.futures.ThreadPoolExecutor() as executor: + results = list(executor.map(self.rpc, methods)) + self.assertEqual([result['id'] for result in results], [7, 7, 7]) + self.assertEqual([result['result']['method'] for result in results], methods) + + def test_cli_crash_does_not_poison_restarted_stdout(self) -> None: + self.rpc('initialize') + result = self.rpc('crash') + self.assertIn('error', result) + retry = self.rpc('initialize', identifier=23) + self.assertEqual(retry['id'], 23) + self.assertIn('error', retry) + time.sleep(1.05) + self.assertEqual(self.rpc('initialize')['result']['serverInfo']['name'], 'fixture') + self.assertTrue(self.http('/healthz')[1]['ok']) + + def test_workspace_transport_settings_override_legacy_host_file(self) -> None: + self.stop() + legacy = self.home / '.unity-mcp' + legacy.mkdir() + (legacy / '.env.local').write_text('UNITY_MCP_TOKEN=stale-home-token\nUNITY_MCP_HTTP_PORT=1\nUNITY_MCP_PROJECT=missing', encoding='utf-8') + self.launch() + health = self.http('/healthz')[1] + self.assertEqual(health['projectPath'], str(self.project)) + self.assertEqual(self.rpc('initialize')['result']['serverInfo']['name'], 'fixture') + self.stop() + self.env_file.write_text(self.env_file.read_text(encoding='utf-8').replace('UNITY_MCP_TOKEN=fixture-secret', ''), encoding='utf-8') + self.launch() + self.assertFalse(self.http('/healthz')[1]['authRequired']) + self.assertEqual(self.rpc('initialize')['result']['serverInfo']['name'], 'fixture') + + def test_default_wildcard_bind_verifies_its_own_http_instance(self) -> None: + self.stop() + self.env.update(HTTP_PROXY='http://127.0.0.1:1', ALL_PROXY='http://127.0.0.1:1') + command = self.command() + del command[2:4] # Exercise the production default: all interfaces. + self.launch(command) + self.assertIn(f'Local HTTP self-check passed (bridge PID {self.process.pid},', self.logs()) + health = self.http('/healthz')[1] + self.assertEqual(health['bridgePid'], self.process.pid) + self.assertIn(health['instanceId'], self.logs()) + self.assertIsNone(health['pid']) # Health does not start Unity. + + def test_loopback_occupant_forces_wildcard_bridge_to_another_port(self) -> None: + self.stop() + with socket.socket() as occupant: + occupant.bind(('127.0.0.1', 0)) + self.port = occupant.getsockname()[1] + occupant.listen(8) + offset = len(self.logs()) + command = self.command() + del command[2:4] + self.launch(command) + self.assertIn(f'Port {self.port} is busy', self.logs()[offset:]) + selected = next(line.split('=', 1)[1] for line in self.env_file.read_text(encoding='utf-8').splitlines() if line.startswith('UNITY_MCP_HTTP_PORT=')) + self.assertNotEqual(int(selected), self.port) + self.port = int(selected) + self.assertEqual(self.http('/healthz')[1]['bridgePid'], self.process.pid) + self.assertEqual(self.rpc('initialize')['result']['serverInfo']['name'], 'fixture') + + def test_failed_self_check_exits_before_publishing_port(self) -> None: + self.stop() + preload = self.root / 'redirect-health.mjs' + preload.write_text("import http from 'node:http'; const get = http.get; http.get = (options, callback) => get({ ...options, path: '/wrong-endpoint' }, callback);", encoding='utf-8') + env = dict(self.env, NODE_OPTIONS=f'--import={preload.as_uri()}') + before = self.env_file.read_text(encoding='utf-8') + result = subprocess.run(self.command(), cwd=self.root, env=env, capture_output=True, text=True, timeout=8) + self.assertEqual(result.returncode, 1, result.stdout + result.stderr) + self.assertIn('Local HTTP self-check failed', result.stderr) + self.assertNotIn('Local HTTP self-check passed', result.stdout) + self.assertEqual(before, self.env_file.read_text(encoding='utf-8')) + + def test_repeated_start_recognizes_absolute_cli_path(self) -> None: + before = self.env_file.read_text(encoding='utf-8') + result = subprocess.run(self.command(), cwd=self.root, env=self.env, capture_output=True, text=True, timeout=5) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertIn('already running', result.stdout) + self.assertEqual(before, self.env_file.read_text(encoding='utf-8')) + + def test_slow_config_sync_keeps_http_responsive_and_uses_selected_port(self) -> None: + self.stop() + configure = self.script.parent / 'configure.mjs' + configure.write_text("import fs from 'node:fs'; fs.writeFileSync('sync-port', process.env.UNITY_MCP_HTTP_PORT); setTimeout(() => process.exit(0), 1800);", encoding='utf-8') + self.env['UNITY_MCP_AUTO_SYNC'] = '1' + self.env['UNITY_MCP_HTTP_PORT'] = '9020' + self.env_file.write_text(self.env_file.read_text(encoding='utf-8').replace(f'UNITY_MCP_HTTP_PORT={self.port}', 'UNITY_MCP_HTTP_PORT=9020'), encoding='utf-8') + self.process = subprocess.Popen(self.command(), cwd=self.root, env=self.env, stdout=self.log, stderr=self.log) + deadline = time.monotonic() + 5 + port_file = self.root / 'sync-port' + while not port_file.exists() and time.monotonic() < deadline: + time.sleep(0.02) + self.assertTrue(port_file.exists(), self.logs()) + start = time.monotonic() + self.assertEqual(self.http('/healthz')[0], 200) + self.assertLess(time.monotonic() - start, 1) + self.assertEqual(port_file.read_text(encoding='utf-8'), str(self.port)) + time.sleep(1.8) + + +if __name__ == '__main__': + unittest.main() diff --git a/scripts/tests/test_unity_mcp_host.py.meta b/scripts/tests/test_unity_mcp_host.py.meta new file mode 100644 index 00000000..25ffddd5 --- /dev/null +++ b/scripts/tests/test_unity_mcp_host.py.meta @@ -0,0 +1,2 @@ +fileFormatVersion: 2 +guid: d6dad881b5134f8787c31586f544f842 diff --git a/scripts/unity_mcp_doctor.py b/scripts/unity_mcp_doctor.py index 30c776a3..8959b84b 100644 --- a/scripts/unity_mcp_doctor.py +++ b/scripts/unity_mcp_doctor.py @@ -4,6 +4,7 @@ import io import json +import ntpath import os import subprocess import sys @@ -30,11 +31,13 @@ def read_env_local(path: Path) -> Dict[str, str]: except (OSError, UnicodeError): return {} result: Dict[str, str] = {} - for raw in text.splitlines(): + for raw in text.lstrip("\ufeff").splitlines(): line = raw.strip() if not line or line.startswith('#') or '=' not in line: continue key, value = (part.strip() for part in line.split('=', 1)) + if key.startswith('export '): + key = key[7:].strip() if len(value) >= 2 and value[0] == value[-1] and value[0] in "\"'": value = value[1:-1] if key: @@ -55,13 +58,16 @@ def enclosing_project(start: Path) -> Optional[Path]: def unity_command(arguments: List[str]) -> str: - return subprocess.check_output(['unity', *arguments], text=True, encoding='utf-8').strip() + local = {**read_env_local(Path.home() / '.unity-mcp' / '.env.local'), **read_env_local(REPO_ROOT / '.env.local')} + executable = os.environ.get('UNITY_CLI') or local.get('UNITY_CLI') or 'unity' + return subprocess.check_output([executable, *arguments], text=True, encoding='utf-8').strip() def request_json(url: str, headers: Dict[str, str], payload: Optional[JsonObject]) -> JsonObject: data = None if payload is None else json.dumps(payload).encode('utf-8') with urlopen(Request(url, data=data, headers=headers), timeout=30) as response: - return object_value(cast(Json, json.load(response))) + body = response.read() + return object_value(cast(Json, json.loads(body))) if body else {} class Doctor: @@ -72,12 +78,17 @@ def __init__( ) -> None: self.root = root self.environment = environment - self.local = {**read_env_local(root / '.env.local'), **read_env_local(home / '.unity-mcp' / '.env.local')} + host_local = read_env_local(home / '.unity-mcp' / '.env.local') + self.local = {key: value for key, value in host_local.items() + if key in ('UNITY_CLI', 'UNITY_MCP_PROJECT', 'UNITY_PROJECT_PATH', 'UNITY_MCP_CLI_ARGS')} + self.local.update(read_env_local(root / '.env.local')) self.command = command self.request = request self.failures = 0 self.warnings = 0 self.project: Optional[Path] = None + container_flag = environment.get('DATAVIZ_IN_CONTAINER') + self.remote = container_flag == '1' if container_flag is not None else Path('/.dockerenv').exists() or Path('/run/.containerenv').exists() def fail(self, message: str) -> None: self.failures += 1 @@ -148,14 +159,17 @@ def check_editor(self) -> None: def check_bridge(self) -> None: port = self.environment.get('UNITY_MCP_HTTP_PORT') or self.local.get('UNITY_MCP_HTTP_PORT') or '9020' - base = f'http://127.0.0.1:{port}' + host = 'host.docker.internal' if self.remote else '127.0.0.1' + endpoint = (self.environment.get('UNITY_MCP_HTTP_URL') or self.local.get('UNITY_MCP_HTTP_URL') or f'http://{host}:{port}/mcp').rstrip('/') + base = endpoint.rsplit('/', 1)[0] try: health = self.request(base + '/healthz', {}, None) self.ok(f"Bridge healthz: ok={str(health.get('ok')).lower()} cli={json.dumps(health.get('cli'))}") - except (OSError, ValueError): - print(f'ℹ️ Bridge not reachable on {base} — start it on the host with: npm run unity:mcp:host') + except (OSError, ValueError) as error: + self.fail(f'Bridge HTTP health failed on {base}: {error}. Check the host /healthz URL, bridge port, and firewall; ' + 'start it on the host with: npm run unity:mcp:host') return - headers = {'content-type': 'application/json'} + headers = {'content-type': 'application/json', 'accept': 'application/json, text/event-stream'} token = self.environment.get('UNITY_MCP_TOKEN') or self.local.get('UNITY_MCP_TOKEN') or '' if token: headers['authorization'] = f'Bearer {token}' @@ -165,17 +179,25 @@ def check_bridge(self) -> None: 'Restart the bridge with UNITY_MCP_PROJECT set.') def rpc(identifier: int, method: str, params: JsonObject) -> JsonObject: - return self.request(base + '/mcp', headers, {'jsonrpc': '2.0', 'id': identifier, 'method': method, 'params': params}) + result = self.request(endpoint, headers, {'jsonrpc': '2.0', 'id': identifier, 'method': method, 'params': params}) + if 'error' in result: + raise ValueError(json.dumps(result['error'])) + if not isinstance(result.get('result'), dict): + raise ValueError(f'{method} returned no result object') + return result try: init = rpc(1, 'initialize', {'protocolVersion': '2025-03-26', 'capabilities': {}, 'clientInfo': {'name': 'unity-mcp-doctor', 'version': '0.1.0'}}) + if not object_value(init.get('result')).get('serverInfo'): + raise ValueError('initialize returned no serverInfo') server = object_value(init.get('result')).get('serverInfo', init.get('error')) self.ok(f'MCP initialize: server={json.dumps(server)}') except (OSError, ValueError) as error: self.fail(f'MCP initialize failed: {error}') return try: + self.request(endpoint, headers, {'jsonrpc': '2.0', 'method': 'notifications/initialized'}) tools = object_value(rpc(2, 'tools/list', {}).get('result')).get('tools') count = len(tools) if isinstance(tools, list) else 0 if count == 0: @@ -186,6 +208,8 @@ def rpc(identifier: int, method: str, params: JsonObject) -> JsonObject: self.fail(f'tools/list failed: {error}') try: call = rpc(3, 'tools/call', {'name': 'editor_status', 'arguments': {}}) + if object_value(call.get('result')).get('isError'): + raise ValueError('editor_status returned isError=true') content = object_value(call.get('result')).get('content') text = object_value(content[0]).get('text') if isinstance(content, list) and content else json.dumps(call.get('error')) text = text if isinstance(text, str) else json.dumps(text) @@ -195,26 +219,36 @@ def rpc(identifier: int, method: str, params: JsonObject) -> JsonObject: status = {} driven = status.get('projectPath') if isinstance(driven, str) and driven: - actual = Path(driven).resolve() - if self.project and actual == self.project: - self.ok(f'editor_status drives the pinned project: {actual}') + expected = (self.environment.get('UNITY_MCP_PROJECT') or self.environment.get('UNITY_PROJECT_PATH') + or self.local.get('UNITY_MCP_PROJECT') or self.local.get('UNITY_PROJECT_PATH') or '') if self.remote else str(self.project or '') + # Windows host paths are opaque to Linux. Compare them with + # Windows rules instead of resolving C:\\... under /workspaces. + normalize = ntpath.normcase if ntpath.splitdrive(driven)[0] else os.path.normpath + actual = normalize(driven) + if expected and actual == normalize(expected): + self.ok(f'editor_status drives the pinned project: {driven}') + elif not expected and self.remote: + self.ok(f'editor_status drives host project: {driven}') else: self.fail(f"WRONG EDITOR: the MCP server drives '{actual}' but the pinned project is " - f"'{self.project or ''}'. Restart the bridge after fixing UNITY_MCP_PROJECT.") + f"'{expected}'. Restart the bridge after fixing UNITY_MCP_PROJECT.") else: - self.warn('editor_status did not return a project (editor for the pinned project likely not running): ' + self.fail('editor_status did not return a project (editor for the pinned project likely not running): ' + ' '.join(text[:200].split())) except (OSError, ValueError) as error: - self.warn(f'editor_status canary failed: {error}') + self.fail(f'editor_status canary failed: {error}') def run(self) -> int: - try: - self.ok(f"Unity CLI on PATH: {self.command(['--version'])}") - except (OSError, subprocess.SubprocessError): - self.fail('Unity CLI not runnable (`unity --version` failed). Install it or set UNITY_CLI; ' - 'the bridge spawns `unity mcp`.') - self.check_project() - self.check_editor() + if self.remote: + print('ℹ️ Container client: Unity CLI and project paths belong to the host; checking through the bridge.') + else: + try: + self.ok(f"Unity CLI on PATH: {self.command(['--version'])}") + except (OSError, subprocess.SubprocessError): + self.fail('Unity CLI not runnable (`unity --version` failed). Install it or set UNITY_CLI; ' + 'the bridge spawns `unity mcp`.') + self.check_project() + self.check_editor() self.check_bridge() return self.finish() From b32384da7e6b6bb1d879903939b5d166a651edef Mon Sep 17 00:00:00 2001 From: Eli Pinkerton Date: Wed, 14 Oct 2026 20:56:34 +0000 Subject: [PATCH 2/3] Fix bridge fixture cleanup on Windows Windows terminates the bridge parent without stopping the fixture CLI child. The child keeps its current directory open, so temporary-directory cleanup fails even when the assertions pass. Stop the fixture process tree before removing its files; keep every behavior assertion. Validation: scoped fast check, strict pyright, and full non-Unity gate pass. Windows CI on PR #241 exposed this cleanup failure and will validate the repair. Refs #239. --- scripts/tests/test_unity_mcp_host.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/scripts/tests/test_unity_mcp_host.py b/scripts/tests/test_unity_mcp_host.py index 6ab6b788..0d81d6b7 100644 --- a/scripts/tests/test_unity_mcp_host.py +++ b/scripts/tests/test_unity_mcp_host.py @@ -82,7 +82,12 @@ def logs(self) -> str: return (self.root / 'bridge.log').read_text(encoding='utf-8') def stop(self) -> None: - self.process.terminate() + if os.name == 'nt' and self.process.poll() is None: + # Windows terminate kills only the parent, leaving the CLI child + # holding the fixture directory and log open. + subprocess.run(['taskkill', '/PID', str(self.process.pid), '/T', '/F'], capture_output=True, check=True) + else: + self.process.terminate() try: self.process.wait(timeout=5) except subprocess.TimeoutExpired: From 0cb1293af5382465ea53bc6c916333ef4e95256a Mon Sep 17 00:00:00 2001 From: Eli Pinkerton Date: Wed, 14 Oct 2026 21:10:22 +0000 Subject: [PATCH 3/3] Fix Unity doctor path and endpoint normalization Equivalent Windows project pins and endpoint URLs could fail healthy bridge diagnostics. Normalize Windows dot segments and trailing separators before comparing case, and strip endpoint slashes before constructing the JS health URL. Regression tests fail before the repair for three Windows pin variants and the trailing-slash JS probe. All 13 doctor tests, strict pyright, the scoped fast check, and the full 39-file harness gate now pass. The live doctor confirms the correct host editor with zero failures or warnings. No Unity C# changed; the two separately recorded performance-driver failures remain on #240. Address both review findings on PR #241. Refs #239, #171. --- .llm/mcp/mcp-doctor.mjs | 2 +- scripts/tests/test_unity_mcp_doctor.py | 25 +++++++++++++++++++++---- scripts/unity_mcp_doctor.py | 2 +- 3 files changed, 23 insertions(+), 6 deletions(-) diff --git a/.llm/mcp/mcp-doctor.mjs b/.llm/mcp/mcp-doctor.mjs index efb02ac9..27ee2518 100644 --- a/.llm/mcp/mcp-doctor.mjs +++ b/.llm/mcp/mcp-doctor.mjs @@ -431,7 +431,7 @@ await spawnProbe("zai-image", "node", [".llm/mcp/zai-image-mcp.mjs"], { // --------------------------------------------------------------------------- console.log("--- Unity bridge ---"); const port = Number(env("UNITY_MCP_HTTP_PORT")) || 9020; -const bridgeUrl = env("UNITY_MCP_HTTP_URL") || `http://host.docker.internal:${port}/mcp`; +const bridgeUrl = (env("UNITY_MCP_HTTP_URL") || `http://host.docker.internal:${port}/mcp`).replace(/\/+$/, ""); const bridgeBase = bridgeUrl.slice(0, bridgeUrl.lastIndexOf("/")); try { const res = await fetch(`${bridgeBase}/healthz`, { diff --git a/scripts/tests/test_unity_mcp_doctor.py b/scripts/tests/test_unity_mcp_doctor.py index 148ffecb..e3e2230c 100644 --- a/scripts/tests/test_unity_mcp_doctor.py +++ b/scripts/tests/test_unity_mcp_doctor.py @@ -5,6 +5,8 @@ import contextlib import io import json +import shutil +import subprocess import sys import tempfile import unittest @@ -152,10 +154,25 @@ def test_environment_parser_preserves_literal_values(self) -> None: def test_container_uses_host_bridge_and_windows_project_paths(self) -> None: self.responses['tools/call'] = {'result': {'content': [{'text': json.dumps({'projectPath': 'C:/Code/Project'})}]}} - doctor, code, output = self.run_doctor({'DATAVIZ_IN_CONTAINER': '1', 'UNITY_MCP_PROJECT': 'c:\\code\\project'}) - self.assertEqual((code, doctor.failures), (0, 0)) - self.assertTrue(self.urls[0].startswith('http://host.docker.internal:9020/')) - self.assertNotIn('Unity CLI on PATH', output) + for pin in [r'c:\code\project', 'C:/Code/Project/', r'C:\Code\.\Project', r'C:\Code\other\..\Project']: + with self.subTest(pin=pin): + doctor, code, output = self.run_doctor({'DATAVIZ_IN_CONTAINER': '1', 'UNITY_MCP_PROJECT': pin}) + self.assertEqual((code, doctor.failures), (0, 0), output) + self.assertTrue(self.urls[0].startswith('http://host.docker.internal:9020/')) + self.assertNotIn('Unity CLI on PATH', output) + + @unittest.skipUnless(shutil.which('node'), 'node is required') + def test_js_doctor_normalizes_endpoint_before_health_probe(self) -> None: + source = (Path(__file__).resolve().parents[2] / '.llm/mcp/mcp-doctor.mjs').read_text(encoding='utf-8') + section = source.split('console.log("--- Unity bridge ---");', 1)[1].split('} // end SKIP_NETWORK_PROBES guard', 1)[0] + for endpoint in ['http://bridge:9021/mcp', 'http://bridge:9021/mcp///']: + with self.subTest(endpoint=endpoint): + bootstrap = ('const urls=[]; const env=name=>name==="UNITY_MCP_HTTP_URL"?' + json.dumps(endpoint) + ':"";' + 'const ok=()=>{}; const fail=message=>{throw Error(message)};' + 'const fetch=async url=>{urls.push(url);return {ok:true,json:async()=>({result:{serverInfo:{name:"fixture"}}})}};') + result = subprocess.run([str(shutil.which('node')), '--input-type=module'], input=bootstrap + section + '\nconsole.log(JSON.stringify(urls));', + capture_output=True, text=True, encoding='utf-8', timeout=10, check=True) + self.assertEqual(json.loads(result.stdout.splitlines()[-1]), ['http://bridge:9021/healthz', 'http://bridge:9021/mcp']) def test_custom_endpoint_and_rpc_errors_fail(self) -> None: for method in ['initialize', 'tools/list', 'tools/call']: diff --git a/scripts/unity_mcp_doctor.py b/scripts/unity_mcp_doctor.py index 8959b84b..8c3b1f6b 100644 --- a/scripts/unity_mcp_doctor.py +++ b/scripts/unity_mcp_doctor.py @@ -223,7 +223,7 @@ def rpc(identifier: int, method: str, params: JsonObject) -> JsonObject: or self.local.get('UNITY_MCP_PROJECT') or self.local.get('UNITY_PROJECT_PATH') or '') if self.remote else str(self.project or '') # Windows host paths are opaque to Linux. Compare them with # Windows rules instead of resolving C:\\... under /workspaces. - normalize = ntpath.normcase if ntpath.splitdrive(driven)[0] else os.path.normpath + normalize: Callable[[str], str] = (lambda value: ntpath.normcase(ntpath.normpath(value))) if ntpath.splitdrive(driven)[0] else os.path.normpath actual = normalize(driven) if expected and actual == normalize(expected): self.ok(f'editor_status drives the pinned project: {driven}')