diff --git a/UnityMcpBridge/UnityMcpServer~/src/telemetry.py b/UnityMcpBridge/UnityMcpServer~/src/telemetry.py index 3141d1b..7399107 100644 --- a/UnityMcpBridge/UnityMcpServer~/src/telemetry.py +++ b/UnityMcpBridge/UnityMcpServer~/src/telemetry.py @@ -17,6 +17,7 @@ from urllib.parse import urlparse from dataclasses import dataclass, asdict from typing import Optional, Dict, Any, List from pathlib import Path +import importlib try: import httpx @@ -61,11 +62,29 @@ class TelemetryRecord: class TelemetryConfig: """Telemetry configuration""" def __init__(self): - # Check environment variables for opt-out - self.enabled = not self._is_disabled() + # Prefer config file, then allow env overrides + server_config = None + for modname in ( + "UnityMcpBridge.UnityMcpServer~.src.config", + "UnityMcpBridge.UnityMcpServer.src.config", + "src.config", + "config", + ): + try: + mod = importlib.import_module(modname) + server_config = getattr(mod, "config", None) + if server_config is not None: + break + except Exception: + continue + + # Determine enabled flag: config -> env DISABLE_* opt-out + cfg_enabled = True if server_config is None else bool(getattr(server_config, "telemetry_enabled", True)) + self.enabled = cfg_enabled and not self._is_disabled() # Telemetry endpoint (Cloud Run default; override via env) - default_ep = "https://unity-mcp-telemetry-375728817078.us-central1.run.app/telemetry/events" + cfg_default = None if server_config is None else getattr(server_config, "telemetry_endpoint", None) + default_ep = cfg_default or "https://unity-mcp-telemetry-375728817078.us-central1.run.app/telemetry/events" self.default_endpoint = default_ep self.endpoint = self._validated_endpoint( os.environ.get("UNITY_MCP_TELEMETRY_ENDPOINT", default_ep), @@ -142,20 +161,31 @@ class TelemetryCollector: def _load_persistent_data(self): """Load UUID and milestones from disk""" + # Load customer UUID try: - # Load customer UUID if self.config.uuid_file.exists(): - self._customer_uuid = self.config.uuid_file.read_text().strip() + self._customer_uuid = self.config.uuid_file.read_text(encoding="utf-8").strip() or str(uuid.uuid4()) else: self._customer_uuid = str(uuid.uuid4()) - self.config.uuid_file.write_text(self._customer_uuid) - - # Load milestones - if self.config.milestones_file.exists(): - self._milestones = json.loads(self.config.milestones_file.read_text()) - except Exception as e: - logger.warning(f"Failed to load telemetry data: {e}") + try: + self.config.uuid_file.write_text(self._customer_uuid, encoding="utf-8") + if os.name == "posix": + os.chmod(self.config.uuid_file, 0o600) + except OSError as e: + logger.debug(f"Failed to persist customer UUID: {e}", exc_info=True) + except OSError as e: + logger.debug(f"Failed to load customer UUID: {e}", exc_info=True) self._customer_uuid = str(uuid.uuid4()) + + # Load milestones (failure here must not affect UUID) + try: + if self.config.milestones_file.exists(): + content = self.config.milestones_file.read_text(encoding="utf-8") + self._milestones = json.loads(content) or {} + if not isinstance(self._milestones, dict): + self._milestones = {} + except (OSError, json.JSONDecodeError, ValueError) as e: + logger.debug(f"Failed to load milestones: {e}", exc_info=True) self._milestones = {} def _save_milestones(self): diff --git a/UnityMcpBridge/UnityMcpServer~/src/tools/manage_script.py b/UnityMcpBridge/UnityMcpServer~/src/tools/manage_script.py index 149f06e..99c33bb 100644 --- a/UnityMcpBridge/UnityMcpServer~/src/tools/manage_script.py +++ b/UnityMcpBridge/UnityMcpServer~/src/tools/manage_script.py @@ -409,13 +409,14 @@ def register_manage_script_tools(mcp: FastMCP): @mcp.tool(description=( "Validate a C# script and return diagnostics.\n\n" - "Args: uri, level=('basic'|'standard').\n" + "Args: uri, level=('basic'|'standard'), include_diagnostics (bool, optional).\n" "- basic: quick syntax checks.\n" "- standard: deeper checks (performance hints, common pitfalls).\n" + "- include_diagnostics: when true, returns full diagnostics and summary; default returns counts only.\n" )) @telemetry_tool("validate_script") def validate_script( - ctx: Context, uri: str, level: str = "basic" + ctx: Context, uri: str, level: str = "basic", include_diagnostics: bool = False ) -> Dict[str, Any]: """Validate a C# script and return diagnostics.""" name, directory = _split_uri(uri) @@ -431,9 +432,11 @@ def register_manage_script_tools(mcp: FastMCP): } resp = send_command_with_retry("manage_script", params) if isinstance(resp, dict) and resp.get("success"): - diags = resp.get("data", {}).get("diagnostics", []) - warnings = sum(1 for d in diags if str(d.get("severity", "")).lower() in ("warning",)) + diags = resp.get("data", {}).get("diagnostics", []) or [] + warnings = sum(1 for d in diags if str(d.get("severity", "")).lower() == "warning") errors = sum(1 for d in diags if str(d.get("severity", "")).lower() in ("error", "fatal")) + if include_diagnostics: + return {"success": True, "data": {"diagnostics": diags, "summary": {"warnings": warnings, "errors": errors}}} return {"success": True, "data": {"warnings": warnings, "errors": errors}} return resp if isinstance(resp, dict) else {"success": False, "message": str(resp)} @@ -573,7 +576,6 @@ def register_manage_script_tools(mcp: FastMCP): @mcp.tool(description=( "Get manage_script capabilities (supported ops, limits, and guards).\n\n" - "Args:\n- random_string: required parameter (any string value)\n\n" "Returns:\n- ops: list of supported structured ops\n- text_ops: list of supported text ops\n- max_edit_payload_bytes: server edit payload cap\n- guards: header/using guard enabled flag\n" )) @telemetry_tool("manage_script_capabilities") diff --git a/tests/test_telemetry_endpoint_validation.py b/tests/test_telemetry_endpoint_validation.py new file mode 100644 index 0000000..8ba0d27 --- /dev/null +++ b/tests/test_telemetry_endpoint_validation.py @@ -0,0 +1,56 @@ +import os +import importlib + +def test_endpoint_rejects_non_http(tmp_path, monkeypatch): + # Point data dir to temp to avoid touching real files + monkeypatch.setenv("XDG_DATA_HOME", str(tmp_path)) + monkeypatch.setenv("UNITY_MCP_TELEMETRY_ENDPOINT", "file:///etc/passwd") + + telemetry = importlib.import_module("UnityMcpBridge.UnityMcpServer~.src.telemetry") + importlib.reload(telemetry) + + tc = telemetry.TelemetryCollector() + # Should have fallen back to default endpoint + assert tc.config.endpoint == tc.config.default_endpoint + +def test_config_preferred_then_env_override(tmp_path, monkeypatch): + # Simulate config telemetry endpoint + monkeypatch.setenv("XDG_DATA_HOME", str(tmp_path)) + monkeypatch.delenv("UNITY_MCP_TELEMETRY_ENDPOINT", raising=False) + + # Patch config.telemetry_endpoint via import mocking + import importlib + cfg_mod = importlib.import_module("UnityMcpBridge.UnityMcpServer~.src.config") + old_endpoint = cfg_mod.config.telemetry_endpoint + cfg_mod.config.telemetry_endpoint = "https://example.com/telemetry" + try: + telemetry = importlib.import_module("UnityMcpBridge.UnityMcpServer~.src.telemetry") + importlib.reload(telemetry) + tc = telemetry.TelemetryCollector() + assert tc.config.endpoint == "https://example.com/telemetry" + + # Env should override config + monkeypatch.setenv("UNITY_MCP_TELEMETRY_ENDPOINT", "https://override.example/ep") + importlib.reload(telemetry) + tc2 = telemetry.TelemetryCollector() + assert tc2.config.endpoint == "https://override.example/ep" + finally: + cfg_mod.config.telemetry_endpoint = old_endpoint + +def test_uuid_preserved_on_malformed_milestones(tmp_path, monkeypatch): + monkeypatch.setenv("XDG_DATA_HOME", str(tmp_path)) + + telemetry = importlib.import_module("UnityMcpBridge.UnityMcpServer~.src.telemetry") + importlib.reload(telemetry) + + tc1 = telemetry.TelemetryCollector() + first_uuid = tc1._customer_uuid + + # Write malformed milestones + tc1.config.milestones_file.write_text("{not-json}", encoding="utf-8") + + # Reload collector; UUID should remain same despite bad milestones + importlib.reload(telemetry) + tc2 = telemetry.TelemetryCollector() + assert tc2._customer_uuid == first_uuid +