Python: compose GitHub Copilot session hooks correctly

python/packages/github_copilot/README.md

@@ -20,7 +20,8 @@ When you register a `FunctionTool` declared with `approval_mode="always_require"
 do **not** supply your own `on_pre_tool_use` hook, `GitHubCopilotAgent` installs a default
 `on_pre_tool_use` hook that returns `"ask"` for that tool and defers (`None`) for all other
 tools. The `"ask"` decision routes to your `on_permission_request` handler, where you
-approve or deny the call:
+approve or deny the call. Other session hooks, such as `on_post_tool_use` or
+`on_session_start`, are preserved alongside this default approval hook:
 
 ```python
 from agent_framework import tool

python/packages/github_copilot/agent_framework_github_copilot/_agent.py

@@ -1505,17 +1505,16 @@ def _build_session_hooks(
         ``approval_mode="always_require"`` is delegated to the Copilot SDK's native
         ``on_pre_tool_use`` hook:
 
-        - If the caller supplies their own session hooks -- either the SDK-native
-          ``hooks`` dict or the convenience ``on_pre_tool_use`` handler (via per-run
-          ``options`` or ``default_options``) -- those take precedence and are used
-          as-is. When both are given, the explicit ``hooks`` dict wins for any key it
-          defines and the ``on_pre_tool_use`` shortcut fills in that key otherwise. A
-          warning is logged naming any approval-required tool that will therefore not
-          be automatically gated, since the caller's hooks are responsible for
-          enforcing approval.
+        - If the caller supplies their own ``on_pre_tool_use`` handler -- either in
+          the SDK-native ``hooks`` dict or through the convenience option (via per-run
+          ``options`` or ``default_options``) -- it takes precedence. When both are
+          given, the explicit ``hooks`` dict wins for any key it defines and the
+          shortcut fills in that key otherwise. A warning names any approval-required
+          tool that the caller's pre-tool-use handler is responsible for gating.
         - Otherwise, when any approval-required tool is present, a default hook is
           installed that returns ``"ask"`` for those tools (routing the decision to
           ``on_permission_request``) and defers (``None``) for all other tools.
+          Unrelated caller hooks are preserved and do not replace this default.
         - The default hook is **not** installed when the deprecated
           ``on_function_approval`` callback is configured: in that case approval is
           enforced inside the tool handler (see :meth:`_tool_to_copilot_tool`) to
@@ -1532,12 +1531,12 @@ def _build_session_hooks(
             The hooks to register for the session, or ``None`` if none are needed.
         """
         user_hook: PreToolUseHandler | None = options.get("on_pre_tool_use") or self._on_pre_tool_use
-        caller_hooks: Mapping[str, Any] | None = options.get("hooks")
+        caller_hooks: SessionHooks | None = options.get("hooks")
 
         # Combine caller-provided hooks: the SDK-native ``hooks`` dict plus the
         # convenience ``on_pre_tool_use`` shortcut. The explicit dict wins for the
         # keys it defines; the shortcut only fills in ``on_pre_tool_use`` otherwise.
-        combined: dict[str, Any] = {}
+        combined: SessionHooks = {}
         if user_hook is not None:
             combined["on_pre_tool_use"] = user_hook
         if caller_hooks:
@@ -1547,7 +1546,7 @@ def _build_session_hooks(
             tool.name for tool in all_tools if isinstance(tool, FunctionTool) and tool.approval_mode == "always_require"
         }
 
-        if combined:
+        if combined.get("on_pre_tool_use") is not None:
             if approval_required_names:
                 logger.warning(
                     "Custom session hooks are configured, so %d approval-required tool(s) (%s) "
@@ -1556,15 +1555,15 @@ def _build_session_hooks(
                     len(approval_required_names),
                     ", ".join(sorted(approval_required_names)),
                 )
-            return cast("SessionHooks", combined)
+            return combined
 
         if not approval_required_names:
-            return None
+            return combined or None
 
         # The deprecated on_function_approval callback enforces approval in the tool
         # handler; don't also install the default ask-hook (which would double-gate).
         if self._function_approval_handler is not None:
-            return None
+            return combined or None
 
         def default_pre_tool_use(
             hook_input: Mapping[str, Any],
@@ -1580,7 +1579,8 @@ def default_pre_tool_use(
                 }
             return None
 
-        return {"on_pre_tool_use": default_pre_tool_use}
+        combined["on_pre_tool_use"] = default_pre_tool_use
+        return combined
 
     def _session_run_lock(self, service_session_id: str) -> asyncio.Lock:
         """Return the lock that serializes runs on one Copilot service session."""

python/packages/github_copilot/tests/test_github_copilot_agent.py

@@ -2959,26 +2959,47 @@ def dangerous(path: str) -> str:
         hooks = agent._build_session_hooks([dangerous], {"on_pre_tool_use": runtime_hook})  # type: ignore[reportPrivateUsage]
         assert hooks == {"on_pre_tool_use": runtime_hook}
 
-    async def test_default_hook_forwarded_to_create_session(
+    @pytest.mark.parametrize("streaming", [False, True])
+    async def test_default_hook_forwarded_with_unrelated_hooks(
         self,
         mock_client: MagicMock,
         mock_session: MagicMock,
         assistant_message_event: SessionEvent,
+        assistant_delta_event: SessionEvent,
+        session_idle_event: SessionEvent,
+        streaming: bool,
     ) -> None:
-        """An always_require tool causes the default hook to be forwarded to the SDK session."""
+        """Unrelated hooks are forwarded without suppressing approval in either run path."""
         mock_session.send_and_wait.return_value = assistant_message_event
+        post_tool_use = MagicMock()
 
         @tool(approval_mode="always_require")
         def dangerous(path: str) -> str:
             """A tool that requires human approval."""
             return f"deleted {path}"
 
-        agent = GitHubCopilotAgent(client=mock_client, tools=[dangerous])
-        await agent.run("hello")
+        agent = GitHubCopilotAgent(
+            client=mock_client,
+            tools=[dangerous],
+            default_options=cast(Any, {"hooks": {"on_post_tool_use": post_tool_use}}),
+        )
+        if streaming:
+            events = [assistant_delta_event, session_idle_event]
+
+            def mock_on(handler: Any) -> Any:
+                for event in events:
+                    handler(event)
+                return lambda: None
 
-        hooks = mock_client.create_session.call_args.kwargs["hooks"]
-        assert hooks is not None
-        assert "on_pre_tool_use" in hooks
+            mock_session.on = mock_on
+            await agent.run("hello", stream=True).get_final_response()
+        else:
+            await agent.run("hello")
+
+        session_hooks = mock_client.create_session.call_args.kwargs["hooks"]
+        assert session_hooks["on_post_tool_use"] is post_tool_use
+        decision = session_hooks["on_pre_tool_use"](pre_tool_use_input("dangerous"), {"session_id": "s"})
+        assert decision["permissionDecision"] == "ask"
 
 
 class TestGitHubCopilotAgentDeprecatedFunctionApproval: