From b9a9098c1632b3c3653bde966fd1636f7e8dc0ec Mon Sep 17 00:00:00 2001 From: Eli Pinkerton Date: Mon, 19 Oct 2026 02:24:30 +0000 Subject: [PATCH 1/2] Fix Unity bridge reuse across project pins Startup could claim success while tools still targeted another Unity project. Require filesystem identity before reusing a verified bridge, and fail clearly without changing its process or client configuration. Accept equivalent paths and explicitly unpinned repeats; reject unknown ownership. Regression: three real HTTP fixture mismatch cases failed before the fix and pass after it. Scoped fast check, 11-step local gate, and strict Pyright pass. Unity 6000.5.2f1 on Windows passes 395 EditMode and 15 PlayMode tests. Windows path variants will also run in CI; macOS was unavailable. Fixes #301 --- .llm/mcp/README.md | 9 ++-- scripts/tests/test_unity_mcp_host.py | 46 +++++++++++++++++++++ scripts/tests/test_unity_mcp_host_python.py | 15 ++++++- scripts/unity_mcp_host.py | 23 +++++++++-- 4 files changed, 86 insertions(+), 7 deletions(-) diff --git a/.llm/mcp/README.md b/.llm/mcp/README.md index 00caea2..57c6f03 100644 --- a/.llm/mcp/README.md +++ b/.llm/mcp/README.md @@ -147,9 +147,12 @@ Security: the bridge listens on all interfaces by default (Linux Docker cannot r - **`unity run unity:mcp:host` fails with "Not a Unity project".** That invoked the Unity CLI, not the npm script: `unity run ` expects a project path, so it treats `unity:mcp:host` as one. The bridge is an npm script — start it with `npm run unity:mcp:host`. -- **Port already in use / dynamic ports.** Re-running the bridge is a no-op while a healthy instance holds the port; if something else owns it, the bridge binds the next free port, - persists `UNITY_MCP_HTTP_PORT` to `.env.local`, and re-syncs every frontend config automatically. No fixed port is required anywhere: `mcp_configure.py`, the devcontainer health - check, and the doctors all read the persisted value. +- **Port already in use / dynamic ports.** Re-running the bridge is a no-op while an instance for the same project holds the port; if something else owns it, the bridge binds the + next free port, persists `UNITY_MCP_HTTP_PORT` to `.env.local`, and re-syncs every frontend config automatically. No fixed port is required anywhere: `mcp_configure.py`, the + devcontainer health check, and the doctors all read the persisted value. +- **Bridge project mismatch.** Startup rejects reuse when the running bridge has a different project pin, including pinned/unpinned differences or unknown ownership. It leaves the + running bridge and client settings unchanged. Use another `--port` or stop the old bridge before restarting. Project roots compare by filesystem identity: trailing separators, + `..`, and symlink aliases match; Windows case and slash variants match when they name the same directory. Two explicitly unpinned instances can share a bridge. - **`npm run mcp:sync` on the host.** It runs the typed Python driver `scripts/mcp_configure.py`. Codex persistence and workspace cleanup use `scripts/mcp_codex_config.py`; JSON persistence uses `scripts/mcp_json_config.py`. Credential aliases use `scripts/mcp_aliases.py`, preserving BOM and line endings and emitting statuses only. The managed server catalog uses `scripts/mcp_catalog.py`. The npm commands use uv; standalone entry points also support Python 3.9+. Container hooks use `sync_mcp.py` diff --git a/scripts/tests/test_unity_mcp_host.py b/scripts/tests/test_unity_mcp_host.py index 79d2d8a..0b934c3 100644 --- a/scripts/tests/test_unity_mcp_host.py +++ b/scripts/tests/test_unity_mcp_host.py @@ -470,6 +470,52 @@ def test_repeated_start_recognizes_absolute_cli_path(self) -> None: self.assertIn('already running', result.stdout) self.assertEqual(before, self.env_file.read_text(encoding='utf-8')) + def test_repeated_start_requires_matching_project_ownership(self) -> None: + other = self.root / 'OtherProject' + (other / 'ProjectSettings').mkdir(parents=True) + (other / 'ProjectSettings/ProjectVersion.txt').write_text('6000.5', encoding='utf-8') + for existing_pin, requested_pin in ((self.project, other), (None, self.project), (self.project, None)): + with self.subTest(existing_pin=existing_pin, requested_pin=requested_pin): + self.stop() + content = f'UNITY_CLI="{PYTHON}"\nUNITY_MCP_TOKEN=fixture-secret\n' + if existing_pin is not None: + content += f'UNITY_MCP_PROJECT="{existing_pin}"\n' + self.env_file.write_text(content, encoding='utf-8') + self.launch() + before = self.env_file.read_bytes() + original = self.http('/healthz')[1] + # Override the file pin; an invalid root resolves to an unpinned invocation. + pin = str(requested_pin) if requested_pin is not None else str(self.root / 'Missing') + result = subprocess.run(self.command(), cwd=self.root, env={**self.env, 'UNITY_MCP_PROJECT': pin}, + capture_output=True, text=True, timeout=5) + self.assertEqual(result.returncode, 1, result.stdout + result.stderr) + self.assertIn('project mismatch', result.stderr) + self.assertNotIn('already running', result.stdout) + self.assertEqual(self.env_file.read_bytes(), before) + current = self.http('/healthz')[1] + self.assertEqual(current['instanceId'], original['instanceId']) + self.assertEqual(current['projectPath'], original['projectPath']) + self.assertEqual(self.rpc('initialize')['result']['serverInfo']['name'], 'fixture') + + def test_repeated_start_accepts_equivalent_project_spellings(self) -> None: + aliases = [str(self.project) + os.sep, str(self.project / 'ProjectSettings' / '..')] + if os.name == 'nt': + aliases.extend((str(self.project).swapcase(), str(self.project).replace('\\', '/'))) + else: + alias = self.root / 'ProjectAlias' + alias.symlink_to(self.project, target_is_directory=True) + aliases.append(str(alias)) + before = self.env_file.read_bytes() + original = self.http('/healthz')[1] + for alias in aliases: + with self.subTest(alias=alias): + result = subprocess.run(self.command(), cwd=self.root, env={**self.env, 'UNITY_MCP_PROJECT': alias}, + capture_output=True, text=True, timeout=5) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertIn('already running', result.stdout) + self.assertEqual(self.http('/healthz')[1]['instanceId'], original['instanceId']) + self.assertEqual(self.env_file.read_bytes(), before) + def test_slow_config_sync_keeps_http_responsive_and_uses_selected_port(self) -> None: self.stop() configure = self.root / 'scripts/mcp_configure.py' diff --git a/scripts/tests/test_unity_mcp_host_python.py b/scripts/tests/test_unity_mcp_host_python.py index 852689f..a2852c5 100644 --- a/scripts/tests/test_unity_mcp_host_python.py +++ b/scripts/tests/test_unity_mcp_host_python.py @@ -45,7 +45,20 @@ def test_docker_route_requires_the_same_instance_before_publication(self) -> Non def test_intercepted_existing_bridge_is_not_reused(self) -> None: with patch.object(host, 'read_health', return_value={'bridge': 'dataviz-unity-mcp', 'instanceId': 'existing'}), \ patch.object(host, 'verify_docker_host_route', side_effect=host.DockerHostRouteError('intercepted')): - self.assertFalse(host.existing_bridge(9020, '0.0.0.0')) + self.assertFalse(host.existing_bridge(9020, '0.0.0.0', '')) + + def test_existing_bridge_requires_known_project_ownership(self) -> None: + for project in (None, 42, 'relative/project', '/missing/project'): + with self.subTest(project=project), \ + patch.object(host, 'read_health', return_value={'bridge': 'dataviz-unity-mcp', 'projectPath': project}), \ + patch.object(host, 'verify_docker_host_route'): + with self.assertRaisesRegex(ValueError, 'project mismatch'): + host.existing_bridge(9020, '127.0.0.1', '') + + def test_unpinned_bridge_can_be_reused_by_unpinned_startup(self) -> None: + with patch.object(host, 'read_health', return_value={'bridge': 'dataviz-unity-mcp', 'projectPath': ''}), \ + patch.object(host, 'verify_docker_host_route'): + self.assertTrue(host.existing_bridge(9020, '127.0.0.1', '')) def test_failed_local_check_never_publishes_and_closes_the_candidate(self) -> None: server = Mock() diff --git a/scripts/unity_mcp_host.py b/scripts/unity_mcp_host.py index 73885dd..a42130f 100644 --- a/scripts/unity_mcp_host.py +++ b/scripts/unity_mcp_host.py @@ -346,15 +346,32 @@ def port_free(port: int, host: str) -> bool: return True -def existing_bridge(port: int, host: str) -> bool: +def same_project(requested: str, actual: object) -> bool: + if not isinstance(actual, str): + return False + if not requested or not actual: + return requested == actual + if not os.path.isabs(actual): + return False + try: + return os.path.samefile(requested, actual) + except (OSError, ValueError): + return False + + +def existing_bridge(port: int, host: str, project: str) -> bool: try: health = read_health(port, '127.0.0.1') if health.get('bridge') != 'dataviz-unity-mcp' and not re.search(r'(?:^|[/\\])unity(?:\.exe)? mcp(?: |$)', str(health.get('cli', ''))): return False verify_docker_host_route(port, host, health.get('instanceId', '')) - return True except (OSError, ValueError): return False + if not same_project(project, health.get('projectPath')): + raise ValueError(f"Bridge project mismatch on port {port}: requested {project or ''!r}, " + f"running {health.get('projectPath', '') or ''!r}. " + 'The existing bridge is unchanged. Use a different --port or stop that bridge before restarting.') + return True def persist_port(workspace: Path, port: int) -> bool: @@ -382,7 +399,7 @@ def persist_port(workspace: Path, port: int) -> bool: def serve(workspace: Path, settings: BridgeSettings, preferred: int, host: str, done: threading.Event) -> int: - if existing_bridge(preferred, host): + if existing_bridge(preferred, host, settings['projectPath']): log(f'[unity-mcp] A healthy unity-mcp bridge is already running on port {preferred}; nothing to do.') return 0 for port in range(preferred, min(preferred + 100, 65535) + 1): From 30605b7de95874254063c0f7ec183b7a1977a2ff Mon Sep 17 00:00:00 2001 From: Eli Pinkerton Date: Mon, 19 Oct 2026 02:27:58 +0000 Subject: [PATCH 2/2] Clarify unknown Unity bridge ownership errors A null or malformed health project field does not prove that a bridge is unpinned. Report unknown ownership distinctly so the recovery diagnostic matches the rejection reason. The focused regression, fast checks, strict Pyright, and the 11-step local gate pass. Refs #301. --- scripts/tests/test_unity_mcp_host_python.py | 3 ++- scripts/unity_mcp_host.py | 6 ++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/scripts/tests/test_unity_mcp_host_python.py b/scripts/tests/test_unity_mcp_host_python.py index a2852c5..55fa1eb 100644 --- a/scripts/tests/test_unity_mcp_host_python.py +++ b/scripts/tests/test_unity_mcp_host_python.py @@ -52,8 +52,9 @@ def test_existing_bridge_requires_known_project_ownership(self) -> None: with self.subTest(project=project), \ patch.object(host, 'read_health', return_value={'bridge': 'dataviz-unity-mcp', 'projectPath': project}), \ patch.object(host, 'verify_docker_host_route'): - with self.assertRaisesRegex(ValueError, 'project mismatch'): + with self.assertRaisesRegex(ValueError, 'project mismatch') as raised: host.existing_bridge(9020, '127.0.0.1', '') + self.assertIn(f"running {project if isinstance(project, str) else ''!r}", str(raised.exception)) def test_unpinned_bridge_can_be_reused_by_unpinned_startup(self) -> None: with patch.object(host, 'read_health', return_value={'bridge': 'dataviz-unity-mcp', 'projectPath': ''}), \ diff --git a/scripts/unity_mcp_host.py b/scripts/unity_mcp_host.py index a42130f..6df1a99 100644 --- a/scripts/unity_mcp_host.py +++ b/scripts/unity_mcp_host.py @@ -367,9 +367,11 @@ def existing_bridge(port: int, host: str, project: str) -> bool: verify_docker_host_route(port, host, health.get('instanceId', '')) except (OSError, ValueError): return False - if not same_project(project, health.get('projectPath')): + actual = health.get('projectPath') + if not same_project(project, actual): + running = (actual or '') if isinstance(actual, str) else '' raise ValueError(f"Bridge project mismatch on port {port}: requested {project or ''!r}, " - f"running {health.get('projectPath', '') or ''!r}. " + f"running {running!r}. " 'The existing bridge is unchanged. Use a different --port or stop that bridge before restarting.') return True