Route Python hooks.invoke through generated client-global handler

CLI 1.0.71 promoted hooks.invoke to a client-global RPC method with a
generated HooksHandler interface. The handwritten SDK still registered its
own hooks.invoke handler, which only avoided colliding with the generated
one because global handlers were skipped when no LLM/telemetry adapter was
set.

Make the wiring intentional: add _HooksAdapter implementing the generated
HooksHandler protocol (routing HookInvokeRequest.sessionId to the matching
session's dispatcher), always register the client-global handlers with the
hooks adapter, and remove the redundant handwritten hooks.invoke
registrations and dead client-level handler.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: ae226b7f-caf9-46f3-b8c5-b9a21c5d7951

Author: SteveSandersonMS

python/copilot/client.py

@@ -73,6 +73,8 @@
     RemoteSessionMode,
     ServerRpc,
     _ConnectResult,
+    _HookInvokeRequest,
+    _HookInvokeResponse,
     from_datetime,
     register_client_global_api_handlers,
     register_client_session_api_handlers,
@@ -479,6 +481,25 @@ async def event(self, params: GitHubTelemetryNotification) -> None:
             logger.warning("Error handling gitHubTelemetry.event notification", exc_info=True)
 
 
+class _HooksAdapter:
+    """Adapts session-scoped hook dispatch to the generated ``HooksHandler`` protocol.
+
+    ``hooks.invoke`` is a client-global RPC method whose payload carries a
+    ``sessionId``. This adapter routes each invocation to the matching session's
+    registered hook handlers.
+    """
+
+    def __init__(self, get_session: Callable[[str], CopilotSession | None]) -> None:
+        self._get_session = get_session
+
+    async def invoke(self, params: _HookInvokeRequest) -> _HookInvokeResponse:
+        session = self._get_session(params.session_id)
+        if session is None:
+            raise ValueError(f"unknown session {params.session_id}")
+        output = await session._handle_hooks_invoke(params.hook_type.value, params.input)
+        return _HookInvokeResponse(output=output)
+
+
 @dataclass
 class _CopilotClientOptions:
     """Internal configuration carrier used by :class:`CopilotClient`.
@@ -4072,7 +4093,6 @@ def handle_notification(method: str, params: dict):
         self._client.set_request_handler(
             "autoModeSwitch.request", self._handle_auto_mode_switch_request
         )
-        self._client.set_request_handler("hooks.invoke", self._handle_hooks_invoke)
         self._client.set_request_handler(
             "systemMessage.transform", self._handle_system_message_transform
         )
@@ -4192,7 +4212,6 @@ def handle_notification(method: str, params: dict):
         self._client.set_request_handler(
             "autoModeSwitch.request", self._handle_auto_mode_switch_request
         )
-        self._client.set_request_handler("hooks.invoke", self._handle_hooks_invoke)
         self._client.set_request_handler(
             "systemMessage.transform", self._handle_system_message_transform
         )
@@ -4282,16 +4301,19 @@ def _register_client_global_handlers(self) -> None:
         github_telemetry_adapter = None
         if self._on_github_telemetry is not None:
             github_telemetry_adapter = _GitHubTelemetryAdapter(self._on_github_telemetry)
-        if llm_inference_adapter is None and github_telemetry_adapter is None:
-            return
         register_client_global_api_handlers(
             self._client,
             ClientGlobalApiHandlers(
+                hooks=_HooksAdapter(self._get_session),
                 llm_inference=llm_inference_adapter,
                 git_hub_telemetry=github_telemetry_adapter,
             ),
         )
 
+    def _get_session(self, session_id: str) -> CopilotSession | None:
+        with self._sessions_lock:
+            return self._sessions.get(session_id)
+
     async def _set_llm_inference_provider(self) -> None:
         if self._request_handler is None or self._rpc is None:
             return
@@ -4364,34 +4386,6 @@ async def _handle_auto_mode_switch_request(self, params: dict) -> dict:
         response = await session._handle_auto_mode_switch_request(params)
         return {"response": response}
 
-    async def _handle_hooks_invoke(self, params: dict) -> dict:
-        """
-        Handle a hooks invocation from the CLI server.
-
-        Args:
-            params: The hooks invocation parameters from the server.
-
-        Returns:
-            A dict containing the hook output.
-
-        Raises:
-            ValueError: If the request payload is invalid.
-        """
-        session_id = params.get("sessionId")
-        hook_type = params.get("hookType")
-        input_data = params.get("input")
-
-        if not session_id or not hook_type:
-            raise ValueError("invalid hooks invoke payload")
-
-        with self._sessions_lock:
-            session = self._sessions.get(session_id)
-        if not session:
-            raise ValueError(f"unknown session {session_id}")
-
-        output = await session._handle_hooks_invoke(hook_type, input_data)
-        return {"output": output}
-
     async def _handle_system_message_transform(self, params: dict) -> dict:
         """Handle a systemMessage.transform request from the CLI server."""
         session_id = params.get("sessionId")

python/test_client.py

@@ -2616,12 +2616,33 @@ async def on_telemetry(notification):
             await client.force_stop()
 
     @pytest.mark.asyncio
-    async def test_event_handler_not_registered_without_option(self):
+    async def test_event_not_forwarded_without_option(self):
+        # Client-global handlers are always registered (so that hooks.invoke works),
+        # but without the on_github_telemetry option the telemetry adapter is inert:
+        # incoming events must not be forwarded to any callback.
         client = CopilotClient(connection=RuntimeConnection.for_stdio(path=CLI_PATH))
         await client.start()
 
         try:
-            assert "gitHubTelemetry.event" not in client._client.notification_method_handlers
-            assert "gitHubTelemetry.event" not in client._client.request_handlers
+            assert client._on_github_telemetry is None
+
+            # Dispatching a telemetry event is a harmless no-op when not opted in.
+            client._client._handle_message(
+                {
+                    "jsonrpc": "2.0",
+                    "method": "gitHubTelemetry.event",
+                    "params": {
+                        "sessionId": "sess-no-telemetry",
+                        "restricted": False,
+                        "event": {
+                            "kind": "tool_call_executed",
+                            "metrics": {"duration_ms": 1.0},
+                            "properties": {"tool": "shell"},
+                            "session_id": "sess-no-telemetry",
+                        },
+                    },
+                }
+            )
+            await asyncio.sleep(0)
         finally:
             await client.force_stop()