fix: stop 34 tools forcing an approval prompt on every call (#1288)

Server/src/services/tools/debug_request_context.py

@@ -19,6 +19,9 @@
     annotations=ToolAnnotations(
         title="Debug Request Context",
         readOnlyHint=True,
+        destructiveHint=False,
+        idempotentHint=True,
+        openWorldHint=False,
     ),
 )
 async def debug_request_context(ctx: Context) -> dict[str, Any]:

Server/src/services/tools/find_gameobjects.py

@@ -5,6 +5,7 @@
 from typing import Annotated, Any, Literal
 
 from fastmcp import Context
+from mcp.types import ToolAnnotations
 from pydantic import Field
 from services.registry import mcp_for_unity_tool
 from services.tools import get_unity_instance_from_context
@@ -21,7 +22,16 @@
         "Then use mcpforunity://scene/gameobject/{id} resource for full data, "
         "or mcpforunity://scene/gameobject/{id}/components for component details. "
         "For CRUD operations (create/modify/delete), use manage_gameobject instead."
-    )
+    ),
+    annotations=ToolAnnotations(
+        title="Find GameObjects",
+        # Not readOnly: preflight(refresh_if_dirty=True) below can trigger an
+        # asset refresh and domain reload.
+        readOnlyHint=False,
+        destructiveHint=False,
+        idempotentHint=True,
+        openWorldHint=False,
+    ),
 )
 async def find_gameobjects(
     ctx: Context,

Server/src/services/tools/find_in_file.py

@@ -71,6 +71,9 @@ def _split_uri(uri: str) -> tuple[str, str]:
     annotations=ToolAnnotations(
         title="Find in File",
         readOnlyHint=True,
+        destructiveHint=False,
+        idempotentHint=True,
+        openWorldHint=False,
     ),
 )
 async def find_in_file(

Server/src/services/tools/manage_components.py

@@ -5,6 +5,7 @@
 from typing import Annotated, Any, Literal, Optional
 
 from fastmcp import Context
+from mcp.types import ToolAnnotations
 from services.registry import mcp_for_unity_tool
 from services.tools import get_unity_instance_from_context
 from transport.unity_transport import send_with_unity_instance
@@ -20,7 +21,12 @@
         "For READING component data, use the mcpforunity://scene/gameobject/{id}/components resource "
         "or mcpforunity://scene/gameobject/{id}/component/{name} for a single component. "
         "For creating/deleting GameObjects themselves, use manage_gameobject instead."
-    )
+    ),
+    annotations=ToolAnnotations(
+        title="Manage Components",
+        readOnlyHint=False,
+        destructiveHint=True,
+    ),
 )
 async def manage_components(
     ctx: Context,

Server/src/services/tools/manage_editor.py

@@ -13,6 +13,8 @@
     description="Controls and queries the Unity editor's state and settings. Read-only actions: telemetry_status, telemetry_ping. Modifying actions: play, pause, stop, set_active_tool, add_tag, remove_tag, add_layer, remove_layer, deploy_package, restore_package, undo, redo. For prefab editing (open/save/close prefab stage), use manage_prefabs. deploy_package copies the configured MCPForUnity source folder into the project's installed package location (triggers recompile, no confirmation dialog). restore_package reverts to the pre-deployment backup. undo/redo perform Unity editor undo/redo and return the affected group name.",
     annotations=ToolAnnotations(
         title="Manage Editor",
+        readOnlyHint=False,
+        destructiveHint=True,
     ),
 )
 async def manage_editor(

Server/src/services/tools/manage_script.py

@@ -480,6 +480,9 @@ async def _verify_delete():
     annotations=ToolAnnotations(
         title="Validate Script",
         readOnlyHint=True,
+        destructiveHint=False,
+        idempotentHint=True,
+        openWorldHint=False,
     ),
 )
 async def validate_script(
@@ -626,6 +629,9 @@ async def _verify_mutation():
     annotations=ToolAnnotations(
         title="Manage Script Capabilities",
         readOnlyHint=True,
+        destructiveHint=False,
+        idempotentHint=True,
+        openWorldHint=False,
     ),
 )
 async def manage_script_capabilities(ctx: Context) -> dict[str, Any]:
@@ -658,6 +664,9 @@ async def manage_script_capabilities(ctx: Context) -> dict[str, Any]:
     annotations=ToolAnnotations(
         title="Get SHA",
         readOnlyHint=True,
+        destructiveHint=False,
+        idempotentHint=True,
+        openWorldHint=False,
     ),
 )
 async def get_sha(

Server/src/services/tools/manage_tools.py

@@ -36,6 +36,9 @@
     annotations=ToolAnnotations(
         title="Manage Tools",
         readOnlyHint=False,
+        destructiveHint=False,
+        idempotentHint=True,
+        openWorldHint=False,
     ),
 )
 async def manage_tools(

Server/src/services/tools/read_console.py

@@ -24,6 +24,12 @@ def _strip_stacktrace_from_list(items: list) -> None:
     description="Gets messages from or clears the Unity Editor console. Defaults to 10 most recent entries. Use page_size/cursor for paging. Note: For maximum client compatibility, pass count as a quoted string (e.g., '5'). The 'get' action is read-only; 'clear' modifies ephemeral UI state (not project data).",
     annotations=ToolAnnotations(
         title="Read Console",
+        # 'clear' wipes the ephemeral Editor console buffer only — Unity still
+        # mirrors every entry to the Editor log file — so nothing is destroyed.
+        readOnlyHint=False,
+        destructiveHint=False,
+        idempotentHint=True,
+        openWorldHint=False,
     ),
 )
 async def read_console(

Server/src/services/tools/run_tests.py

@@ -243,6 +243,9 @@ def _coerce_string_list(value) -> list[str] | None:
     annotations=ToolAnnotations(
         title="Get Test Job",
         readOnlyHint=True,
+        destructiveHint=False,
+        idempotentHint=True,
+        openWorldHint=False,
     ),
 )
 async def get_test_job(

Server/src/services/tools/set_active_instance.py

@@ -17,6 +17,11 @@
     description="Set the active Unity instance for this client/session. Accepts Name@hash, hash prefix, or port number (stdio only).",
     annotations=ToolAnnotations(
         title="Set Active Instance",
+        # Changes session-local routing only; touches nothing in the project.
+        readOnlyHint=False,
+        destructiveHint=False,
+        idempotentHint=True,
+        openWorldHint=False,
     ),
 )
 async def set_active_instance(

Server/tests/test_tool_annotations.py

@@ -0,0 +1,150 @@
+"""Annotation guard: every MCP tool must state its safety hints explicitly.
+
+MCP clients gate a tool behind a human approval prompt when it is not read-only
+and not explicitly non-destructive. ``destructiveHint`` defaults to *true* when
+omitted, so forgetting it is the dangerous direction -- a harmless read gets an
+approval prompt on every call.
+
+That is exactly how #1288 happened: PR #480's own description claimed hints for
+``read_console``, ``manage_editor`` and ``set_active_instance``, but the merged
+diff set only ``title``. The spec default silently supplied ``destructiveHint:
+true`` and nobody noticed for a year.
+
+So this guard requires ``title`` and ``destructiveHint`` to be stated outright,
+and pins the set of tools that are safe to auto-approve. ``readOnlyHint`` is not
+required: omitted it means *false*, which is the safe direction and is already
+true of every tool that leaves it unset.
+"""
+from pathlib import Path
+
+import pytest
+
+import services.tools as tools_package
+from services.registry import get_registered_tools
+from utils.module_discovery import discover_modules
+
+
+# Tools a client may run without prompting. Two kinds live here:
+#   - read-only: observe Unity, change nothing.
+#   - non-destructive: change only ephemeral or session-local state.
+# Adding a name here asserts that an agent may call it unattended. Do not add a
+# tool that writes to the project, and do not set readOnlyHint on a tool whose
+# body calls preflight(refresh_if_dirty=True) -- that can trigger a domain reload.
+READ_ONLY = {
+    "debug_request_context",
+    "find_in_file",
+    "get_sha",
+    "get_test_job",
+    "manage_script_capabilities",
+    "unity_docs",
+    "unity_reflect",
+    "validate_script",
+}
+
+NON_DESTRUCTIVE = {
+    # preflight(refresh_if_dirty=True) at find_gameobjects.py can refresh assets,
+    # so it is not read-only -- but it never destroys anything.
+    "find_gameobjects",
+    # 'clear' empties the ephemeral Editor console buffer; Unity still mirrors
+    # every entry to the Editor log file on disk.
+    "read_console",
+    # Session-local routing only.
+    "set_active_instance",
+    # Toggles which tools are visible to this session.
+    "manage_tools",
+    # Reads counters / starts a profiler session; writes no project asset.
+    "manage_profiler",
+    # Generate into a staging area; the import step is a separate tool.
+    "generate_audio",
+    "generate_image",
+    "generate_model",
+    "import_model",
+    "import_model_file",
+}
+
+AUTO_APPROVABLE = READ_ONLY | NON_DESTRUCTIVE
+
+
+def _hint(annotations, field: str):
+    """Read one hint, treating 'not stated' as None.
+
+    The real ToolAnnotations is a pydantic model with every field defaulting to
+    None, but tests/integration/conftest.py substitutes a stub that only sets
+    the kwargs actually passed. getattr with a default reads the same answer
+    from either, so this guard means the same thing whatever ran before it.
+    """
+    return getattr(annotations, field, None)
+
+
+@pytest.fixture(scope="module")
+def tools() -> dict:
+    # Import every tool module so its @mcp_for_unity_tool decorator runs. Going
+    # through discover_modules rather than register_all_tools keeps this off
+    # FastMCP, which tests/integration/conftest.py replaces with a stub for the
+    # whole session.
+    list(discover_modules(Path(tools_package.__file__).parent, tools_package.__package__))
+    return {t["name"]: t for t in get_registered_tools()}
+
+
+def test_every_tool_declares_its_hints(tools):
+    missing = []
+    for name, tool in sorted(tools.items()):
+        annotations = tool["kwargs"].get("annotations")
+        if annotations is None:
+            missing.append(f"{name}: no annotations= at all")
+            continue
+        if not _hint(annotations, "title"):
+            missing.append(f"{name}: no title")
+        if _hint(annotations, "destructiveHint") is None:
+            missing.append(f"{name}: destructiveHint not stated (defaults to True)")
+    assert not missing, (
+        "Every tool must state title and destructiveHint explicitly:\n  "
+        + "\n  ".join(missing)
+    )
+
+
+def test_auto_approvable_tools_are_not_gated(tools):
+    """A tool in AUTO_APPROVABLE must actually serialize as auto-approvable."""
+    gated = []
+    for name in sorted(AUTO_APPROVABLE):
+        assert name in tools, f"{name} is in AUTO_APPROVABLE but is not a registered tool"
+        annotations = tools[name]["kwargs"]["annotations"]
+        read_only = _hint(annotations, "readOnlyHint")
+        destructive = _hint(annotations, "destructiveHint")
+        if not read_only and destructive is not False:
+            gated.append(f"{name}: readOnlyHint={read_only} destructiveHint={destructive}")
+    assert not gated, (
+        "These tools are listed as safe to auto-approve but a spec-compliant "
+        "client would still prompt for them:\n  " + "\n  ".join(gated)
+    )
+
+
+def test_read_only_set_is_exact(tools):
+    """readOnlyHint=True is a promise the tool cannot mutate anything. Pin it."""
+    actual = {
+        name
+        for name, tool in tools.items()
+        if _hint(tool["kwargs"]["annotations"], "readOnlyHint") is True
+    }
+    assert actual == READ_ONLY, (
+        "The read-only tool set changed. Newly read-only: "
+        f"{sorted(actual - READ_ONLY)}; no longer read-only: {sorted(READ_ONLY - actual)}. "
+        "Update READ_ONLY only after confirming the tool truly mutates nothing -- "
+        "including via preflight(refresh_if_dirty=True), which can trigger a domain reload."
+    )
+
+
+def test_mutating_tools_stay_gated(tools):
+    """Anything outside AUTO_APPROVABLE must keep prompting."""
+    ungated = []
+    for name, tool in sorted(tools.items()):
+        if name in AUTO_APPROVABLE:
+            continue
+        annotations = tool["kwargs"]["annotations"]
+        if _hint(annotations, "readOnlyHint") or _hint(annotations, "destructiveHint") is False:
+            ungated.append(name)
+    assert not ungated, (
+        "These tools write to the Unity project but are marked auto-approvable: "
+        f"{ungated}. Either they belong in AUTO_APPROVABLE with a comment saying why, "
+        "or the annotation is wrong."
+    )

Server/tests/test_tool_test_symmetry.py

@@ -43,10 +43,17 @@ def _tool_modules() -> list[str]:
     return mods
 
 
+# Registry-wide guards name many tools at once to assert one cross-cutting
+# property. That is not coverage of any individual tool, so they don't count
+# here -- otherwise one such file would silently satisfy this guard for every
+# tool it happens to mention.
+NOT_COVERAGE = {"test_tool_test_symmetry.py", "test_tool_annotations.py"}
+
+
 def _is_referenced(stem: str) -> bool:
     pattern = re.compile(rf"\b{re.escape(stem)}\b")
     for test_file in TESTS_DIR.rglob("test_*.py"):
-        if test_file.resolve() == Path(__file__).resolve():
+        if test_file.name in NOT_COVERAGE:
             continue
         if pattern.search(test_file.read_text(encoding="utf-8")):
             return True