telemetry: prefer config with env override; validate scheme; robust load\n\n- TelemetryConfig reads config.telemetry_enabled/endpoint, env can override\n- Validate endpoint scheme; revalidate on send\n- Split UUID/milestones load error handling\n- Add tests for config precedence, scheme validation, UUID preservation\n- validate_script: optional include_diagnostics with documented behavior

Author: dsarno

UnityMcpBridge/UnityMcpServer~/src/telemetry.py

@@ -17,6 +17,7 @@
 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 @@ def __init__(self):
 
     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):

UnityMcpBridge/UnityMcpServer~/src/tools/manage_script.py

@@ -409,13 +409,14 @@ def delete_script(ctx: Context, uri: str) -> Dict[str, Any]:
 
     @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 validate_script(
         }
         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 manage_script(
 
     @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")

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
+