From 5c718d178f30558ee12e7b287525f0440bed573c Mon Sep 17 00:00:00 2001 From: Giles Odigwe Date: Thu, 25 Jun 2026 13:45:21 -0700 Subject: [PATCH 1/2] Python: align GitHub Copilot approval to SDK on_pre_tool_use hook Replace the bespoke on_function_approval enforcement in the GitHub Copilot provider with the Copilot SDK's native on_pre_tool_use hook. When no caller hook is supplied, a default hook returns 'ask' for approval_mode='always_require' tools (routed to on_permission_request) and defers others; a caller-supplied on_pre_tool_use takes precedence and logs a warning for any unenforced approval tool. Fixes #6746 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- python/packages/github_copilot/README.md | 40 +++ .../agent_framework_github_copilot/_agent.py | 200 +++++++------- .../tests/test_github_copilot_agent.py | 254 +++++++++--------- .../github_copilot_with_function_approval.py | 156 +++++------ 4 files changed, 347 insertions(+), 303 deletions(-) diff --git a/python/packages/github_copilot/README.md b/python/packages/github_copilot/README.md index 87b4ea5d3e3..03c4f3d20fd 100644 --- a/python/packages/github_copilot/README.md +++ b/python/packages/github_copilot/README.md @@ -9,3 +9,43 @@ pip install agent-framework-github-copilot --pre ## GitHub Copilot Agent The GitHub Copilot agent enables integration with GitHub Copilot, allowing you to interact with Copilot's agentic capabilities through the Agent Framework. + +## Tool approval (`approval_mode="always_require"`) + +The GitHub Copilot SDK owns the tool-calling loop for this provider, so approval for +custom function tools is enforced through the SDK's native pre-execution hook rather +than the standard Agent Framework approval round-trip. + +When you register a `FunctionTool` declared with `approval_mode="always_require"` and you +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: + +```python +from agent_framework import tool +from agent_framework.github import GitHubCopilotAgent, GitHubCopilotOptions +from copilot.session import PermissionHandler + + +@tool(approval_mode="always_require") +def delete_file(path: str) -> str: + """Delete a file.""" + ... + + +agent = GitHubCopilotAgent( + tools=[delete_file], + # The "ask" decision is routed here; approve or deny the call. + default_options=GitHubCopilotOptions(on_permission_request=PermissionHandler.approve_all), +) +``` + +> **⚠️ If you provide your own `on_pre_tool_use` hook**, it takes precedence and the agent +> does **not** install its default approval hook. In that case **you are fully responsible** +> for enforcing approval — including for any `approval_mode="always_require"` tool (e.g. by +> returning a `"deny"` or `"ask"` decision). The agent logs a warning naming any +> approval-required tool that your hook must handle. +> +> Note: with the default (deny-all) permission handler, an `always_require` tool is denied +> unless you wire an approving `on_permission_request`. diff --git a/python/packages/github_copilot/agent_framework_github_copilot/_agent.py b/python/packages/github_copilot/agent_framework_github_copilot/_agent.py index 921231a973a..a041c834eff 100644 --- a/python/packages/github_copilot/agent_framework_github_copilot/_agent.py +++ b/python/packages/github_copilot/agent_framework_github_copilot/_agent.py @@ -4,7 +4,6 @@ import asyncio import contextlib -import inspect import logging import sys from collections.abc import AsyncIterable, Awaitable, Callable, Mapping, MutableMapping, Sequence @@ -39,7 +38,15 @@ try: from copilot import CopilotClient, CopilotSession, RuntimeConnection from copilot.generated.rpc import PermissionDecisionUserNotAvailable - from copilot.session import MCPServerConfig, PermissionRequestResult, ProviderConfig, SystemMessageConfig + from copilot.session import ( + MCPServerConfig, + PermissionRequestResult, + PreToolUseHandler, + PreToolUseHookOutput, + ProviderConfig, + SessionHooks, + SystemMessageConfig, + ) from copilot.session_events import PermissionRequest, SessionEvent, SessionEventType from copilot.tools import Tool as CopilotTool from copilot.tools import ToolInvocation, ToolResult @@ -64,58 +71,6 @@ """Type for permission request handlers. Supports both sync and async callbacks.""" -FunctionApprovalCallback = Callable[[Content], "bool | Awaitable[bool]"] -"""Callback invoked by the agent before executing a FunctionTool that requires approval. - -The callback receives a ``FunctionCallContent`` describing the pending call -(``name``, ``arguments``, and a synthetic ``call_id``) and must return ``True`` -to allow execution or ``False`` to deny it. Both synchronous and ``await``-able -return values are supported. - -The Copilot CLI manages its own tool-calling loop, so the framework cannot -round-trip a ``FunctionApprovalRequestContent`` / ``FunctionApprovalResponseContent`` -pair the way the standard chat-client pipeline does. This callback is the -agent-level enforcement point for tools declared with -``approval_mode="always_require"``: when no callback is configured the agent -denies these calls by default. - -Note: this is independent of ``on_permission_request``, which gates the -Copilot SDK's *built-in* shell/file actions; ``on_function_approval`` gates -agent-framework ``FunctionTool`` calls. -""" - - -async def _resolve_function_approval( - callback: FunctionApprovalCallback | None, - func_tool: FunctionTool, - arguments: Mapping[str, Any] | None, -) -> bool: - """Run the agent-level approval callback for a pending tool call. - - Returns ``True`` only when ``callback`` is configured and explicitly returns - a truthy value. A missing callback or any callback failure is treated as a - denial so the secure-by-default policy holds even if the user code raises. - """ - if callback is None: - return False - request = Content.from_function_call( - call_id=f"af-copilot-approval::{func_tool.name}", - name=func_tool.name, - arguments=None if arguments is None else dict(arguments), - ) - try: - outcome = callback(request) - if inspect.isawaitable(outcome): - outcome = await outcome - except Exception: - logger.exception( - "on_function_approval callback raised for tool '%s'; denying execution.", - func_tool.name, - ) - return False - return bool(outcome) - - logger = logging.getLogger("agent_framework.github_copilot") @@ -205,13 +160,21 @@ class GitHubCopilotOptions(TypedDict, total=False): base_directory: str """Directory where the CLI stores session state, configuration, and other persistent data.""" - on_function_approval: FunctionApprovalCallback - """Approval callback for ``FunctionTool`` instances declared with - ``approval_mode="always_require"``. The callback is awaited (sync or async) - inside the SDK tool-handler before the tool is executed; a falsy return - value denies the call. If omitted, calls to such tools are denied with an - explanatory message returned to the model. This is independent of - ``on_permission_request``, which gates the Copilot SDK's built-in actions.""" + on_pre_tool_use: PreToolUseHandler + """Pre-tool-use hook handler for the Copilot SDK. + + Called by the Copilot SDK before any tool is executed. The handler receives a + ``PreToolUseHookInput`` and a context dict, and returns a ``PreToolUseHookOutput`` + (or ``None`` to defer). Returning ``{"permissionDecision": "ask"}`` routes the + decision to ``on_permission_request``; ``"allow"`` / ``"deny"`` gate the call + directly. + + If you do **not** supply this hook, the agent installs a default ``on_pre_tool_use`` + hook that returns ``"ask"`` for ``FunctionTool`` instances declared with + ``approval_mode="always_require"`` (deferring all other tools), so those tools are + gated through ``on_permission_request``. If you **do** supply your own hook, it + takes precedence and **you** are responsible for enforcing approval for any + ``always_require`` tool; the agent logs a warning naming such tools.""" OptionsT = TypeVar( @@ -319,7 +282,7 @@ def __init__( mcp_servers: dict[str, MCPServerConfig] | None = opts.pop("mcp_servers", None) provider: ProviderConfig | None = opts.pop("provider", None) instruction_directories: list[str] | None = opts.pop("instruction_directories", None) - on_function_approval: FunctionApprovalCallback | None = opts.pop("on_function_approval", None) + on_pre_tool_use: PreToolUseHandler | None = opts.pop("on_pre_tool_use", None) base_directory = opts.pop("base_directory", None) self._settings = load_settings( @@ -336,7 +299,7 @@ def __init__( self._tools = normalize_tools(tools) self._permission_handler = on_permission_request - self._function_approval_handler: FunctionApprovalCallback | None = on_function_approval + self._on_pre_tool_use: PreToolUseHandler | None = on_pre_tool_use self._mcp_servers = mcp_servers self._provider = provider self._instruction_directories = instruction_directories @@ -516,12 +479,6 @@ async def _run_impl( session = self.create_session() opts: dict[str, Any] = dict(options) if options else {} - if "on_function_approval" in opts: - raise ValueError( - "on_function_approval is a security-sensitive option and must be set " - "via default_options at agent construction time. It cannot be overridden " - "per run." - ) timeout = opts.get("timeout") or self._settings.get("timeout") or DEFAULT_TIMEOUT_SECONDS input_messages = normalize_messages(messages) @@ -605,12 +562,6 @@ async def _stream_updates( session = self.create_session() opts: dict[str, Any] = dict(options) if options else {} - if "on_function_approval" in opts: - raise ValueError( - "on_function_approval is a security-sensitive option and must be set " - "via default_options at agent construction time. It cannot be overridden " - "per run." - ) input_messages = normalize_messages(messages) @@ -792,34 +743,16 @@ def _prepare_tools( return copilot_tools def _tool_to_copilot_tool(self, ai_func: FunctionTool) -> CopilotTool: - """Convert an FunctionTool to a Copilot SDK tool.""" - approval_handler = self._function_approval_handler - requires_approval = ai_func.approval_mode == "always_require" + """Convert an FunctionTool to a Copilot SDK tool. + + Approval for tools declared with ``approval_mode="always_require"`` is enforced + by the Copilot SDK's native ``on_pre_tool_use`` hook (see + :meth:`_build_session_hooks`), not inside this handler. + """ async def handler(invocation: ToolInvocation) -> ToolResult: args: dict[str, Any] = invocation.arguments or {} try: - if requires_approval and not await _resolve_function_approval(approval_handler, ai_func, args): - deny_text = ( - f"Tool '{ai_func.name}' requires human approval " - "(approval_mode='always_require') and the request was denied." - if approval_handler is not None - else ( - f"Tool '{ai_func.name}' requires human approval " - "(approval_mode='always_require') but no on_function_approval " - "callback is configured on the agent; the request was denied." - ) - ) - logger.info( - "Denying execution of tool '%s' (approval_mode='always_require', %s)", - ai_func.name, - "callback denied" if approval_handler is not None else "no callback configured", - ) - return ToolResult( - text_result_for_llm=deny_text, - result_type="failure", - error="approval_denied", - ) if ai_func.input_model: args_instance = ai_func.input_model(**args) result = await ai_func.invoke(arguments=args_instance) @@ -850,6 +783,71 @@ async def handler(invocation: ToolInvocation) -> ToolResult: parameters=ai_func.parameters(), ) + def _build_session_hooks( + self, + all_tools: Sequence[ToolTypes | CopilotTool], + opts: Mapping[str, Any], + ) -> SessionHooks | None: + """Build the ``SessionHooks`` to pass to the Copilot SDK for this session. + + Approval enforcement for ``FunctionTool`` instances declared with + ``approval_mode="always_require"`` is delegated to the Copilot SDK's native + ``on_pre_tool_use`` hook: + + - If the caller supplies their own ``on_pre_tool_use`` (via per-run ``options`` + or ``default_options``), it takes precedence and is returned unchanged. A + warning is logged naming any approval-required tool that will therefore not + be automatically gated, since the caller's hook is responsible for enforcing + approval. + - 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. + - When there are no approval-required tools and no caller hook, ``None`` is + returned so no hooks are registered. + + Args: + all_tools: The full set of tools resolved for the session. + opts: Runtime options that take precedence over ``default_options``. + + Returns: + The hooks to register for the session, or ``None`` if none are needed. + """ + user_hook: PreToolUseHandler | None = opts.get("on_pre_tool_use") or self._on_pre_tool_use + + approval_required_names = { + tool.name for tool in all_tools if isinstance(tool, FunctionTool) and tool.approval_mode == "always_require" + } + + if user_hook is not None: + if approval_required_names: + logger.warning( + "A custom 'on_pre_tool_use' hook is configured, so %d approval-required tool(s) (%s) " + "will not be automatically gated by GitHubCopilotAgent. The custom hook is responsible " + "for enforcing approval (for example, by returning a 'deny' or 'ask' decision).", + len(approval_required_names), + ", ".join(sorted(approval_required_names)), + ) + return {"on_pre_tool_use": user_hook} + + if not approval_required_names: + return None + + def default_pre_tool_use( + hook_input: Mapping[str, Any], + _context: Mapping[str, str], + ) -> PreToolUseHookOutput | None: + tool_name = hook_input.get("toolName") + if tool_name in approval_required_names: + return { + "permissionDecision": "ask", + "permissionDecisionReason": ( + f"Tool '{tool_name}' is marked as requiring approval (approval_mode='always_require')." + ), + } + return None + + return {"on_pre_tool_use": default_pre_tool_use} + async def _get_or_create_session( self, agent_session: AgentSession, @@ -907,6 +905,7 @@ async def _create_session( instruction_directories = opts.get("instruction_directories", self._instruction_directories) all_tools = list(self._tools or []) + list(opts.get("tools") or []) tools = self._prepare_tools(all_tools) if all_tools else None + hooks = self._build_session_hooks(all_tools, opts) return await self._client.create_session( on_permission_request=permission_handler, @@ -917,6 +916,7 @@ async def _create_session( mcp_servers=mcp_servers or None, provider=provider or None, instruction_directories=instruction_directories, + hooks=hooks, ) async def _resume_session( @@ -946,6 +946,7 @@ async def _resume_session( instruction_directories = opts.get("instruction_directories", self._instruction_directories) all_tools = list(self._tools or []) + list(opts.get("tools") or []) tools = self._prepare_tools(all_tools) if all_tools else None + hooks = self._build_session_hooks(all_tools, opts) return await self._client.resume_session( session_id, @@ -957,6 +958,7 @@ async def _resume_session( mcp_servers=mcp_servers or None, provider=provider or None, instruction_directories=instruction_directories, + hooks=hooks, ) diff --git a/python/packages/github_copilot/tests/test_github_copilot_agent.py b/python/packages/github_copilot/tests/test_github_copilot_agent.py index 39a7eef387b..63b24858e82 100644 --- a/python/packages/github_copilot/tests/test_github_copilot_agent.py +++ b/python/packages/github_copilot/tests/test_github_copilot_agent.py @@ -4,7 +4,7 @@ import os import unittest.mock -from collections.abc import Awaitable, Callable, Sequence +from collections.abc import Sequence from datetime import datetime, timezone from typing import Any, cast from unittest.mock import AsyncMock, MagicMock, patch @@ -955,6 +955,7 @@ async def test_session_resumed_for_same_session( mcp_servers=unittest.mock.ANY, provider=unittest.mock.ANY, instruction_directories=unittest.mock.ANY, + hooks=unittest.mock.ANY, ) async def test_session_config_includes_model( @@ -1645,186 +1646,151 @@ async def tool_handler(invocation: Any) -> Any: class TestGitHubCopilotAgentFunctionApproval: - """Tests that ``approval_mode='always_require'`` is enforced at the agent boundary.""" + """Tests that ``approval_mode='always_require'`` is gated via the SDK ``on_pre_tool_use`` hook.""" - async def test_handler_denies_when_no_callback_configured( + def test_default_hook_asks_for_approval_required_tool( self, mock_client: MagicMock, ) -> None: - """Approval-required tool must be denied without executing when no callback is set.""" - from agent_framework import tool - - invocations: list[Any] = [] + """The default hook returns 'ask' for always_require tools and defers others.""" @tool(approval_mode="always_require") def dangerous(path: str) -> str: """A tool that requires human approval.""" - invocations.append(path) return f"deleted {path}" + @tool + def safe(x: int) -> str: + """A tool that does not require approval.""" + return f"safe={x}" + agent = GitHubCopilotAgent(client=mock_client) - copilot_tool = agent._tool_to_copilot_tool(dangerous) # type: ignore[reportPrivateUsage] + hooks = agent._build_session_hooks([dangerous, safe], {}) # type: ignore[reportPrivateUsage] - handler = cast("Callable[[ToolInvocation], Awaitable[ToolResult]]", copilot_tool.handler) - result = await handler(ToolInvocation(arguments={"path": "/critical"})) + assert hooks is not None + hook = hooks["on_pre_tool_use"] - assert invocations == [] - assert result.result_type == "failure" - assert result.error == "approval_denied" - assert "no on_function_approval callback is configured" in result.text_result_for_llm + approval_decision = hook({"toolName": "dangerous"}, {"session_id": "s"}) + assert approval_decision == { + "permissionDecision": "ask", + "permissionDecisionReason": ( + "Tool 'dangerous' is marked as requiring approval (approval_mode='always_require')." + ), + } - async def test_handler_denies_when_callback_returns_false( + assert hook({"toolName": "safe"}, {"session_id": "s"}) is None + + def test_no_hook_when_no_approval_required_tools( self, mock_client: MagicMock, ) -> None: - """Falsy callback return value must deny the call and skip execution.""" - from agent_framework import Content, tool - - invocations: list[Any] = [] - seen: list[Content] = [] + """No approval-required tools and no user hook means no hooks are installed.""" - def deny(call: Content) -> bool: - seen.append(call) - return False - - @tool(approval_mode="always_require") - def dangerous(path: str) -> str: - """A tool that requires human approval.""" - invocations.append(path) - return f"deleted {path}" - - agent = GitHubCopilotAgent( - client=mock_client, - default_options=copilot_options({"on_function_approval": deny}), - ) - copilot_tool = agent._tool_to_copilot_tool(dangerous) # type: ignore[reportPrivateUsage] - - handler = cast("Callable[[ToolInvocation], Awaitable[ToolResult]]", copilot_tool.handler) - result = await handler(ToolInvocation(arguments={"path": "/critical"})) + @tool + def safe(x: int) -> str: + """A tool that does not require approval.""" + return f"safe={x}" - assert invocations == [] - assert len(seen) == 1 - assert seen[0].type == "function_call" - assert seen[0].name == "dangerous" # type: ignore[attr-defined] - assert seen[0].arguments == {"path": "/critical"} # type: ignore[attr-defined] - assert result.result_type == "failure" - assert result.error == "approval_denied" + agent = GitHubCopilotAgent(client=mock_client) + assert agent._build_session_hooks([safe], {}) is None # type: ignore[reportPrivateUsage] - async def test_handler_executes_when_callback_returns_true( + def test_user_hook_takes_precedence_and_warns( self, mock_client: MagicMock, + caplog: pytest.LogCaptureFixture, ) -> None: - """Truthy callback return value must allow the tool to execute normally.""" - from agent_framework import Content, tool + """A caller-supplied on_pre_tool_use takes precedence and triggers a warning.""" - def approve(call: Content) -> bool: - return True + def user_hook(_input: Any, _context: Any) -> Any: + return {"permissionDecision": "allow"} @tool(approval_mode="always_require") - def guarded(x: int) -> str: + def dangerous(path: str) -> str: """A tool that requires human approval.""" - return f"result={x}" + return f"deleted {path}" agent = GitHubCopilotAgent( client=mock_client, - default_options=copilot_options({"on_function_approval": approve}), + default_options=copilot_options({"on_pre_tool_use": user_hook}), ) - copilot_tool = agent._tool_to_copilot_tool(guarded) # type: ignore[reportPrivateUsage] - handler = cast("Callable[[ToolInvocation], Awaitable[ToolResult]]", copilot_tool.handler) - result = await handler(ToolInvocation(arguments={"x": 42})) + with caplog.at_level("WARNING", logger="agent_framework.github_copilot"): + hooks = agent._build_session_hooks([dangerous], {}) # type: ignore[reportPrivateUsage] - assert result.result_type == "success" - assert result.text_result_for_llm == "result=42" + assert hooks == {"on_pre_tool_use": user_hook} + assert any("dangerous" in record.message and record.levelname == "WARNING" for record in caplog.records) - async def test_handler_supports_async_callback( + def test_user_hook_no_warning_without_approval_tools( self, mock_client: MagicMock, + caplog: pytest.LogCaptureFixture, ) -> None: - """Async callback must be awaited and respected.""" - from agent_framework import Content, tool + """A caller hook with no approval-required tools is preserved without a warning.""" - async def approve(call: Content) -> bool: - return True + def user_hook(_input: Any, _context: Any) -> Any: + return None - @tool(approval_mode="always_require") - def guarded(x: int) -> str: - """A tool that requires human approval.""" - return f"async={x}" + @tool + def safe(x: int) -> str: + """A tool that does not require approval.""" + return f"safe={x}" agent = GitHubCopilotAgent( client=mock_client, - default_options=copilot_options({"on_function_approval": approve}), + default_options=copilot_options({"on_pre_tool_use": user_hook}), ) - copilot_tool = agent._tool_to_copilot_tool(guarded) # type: ignore[reportPrivateUsage] - handler = cast("Callable[[ToolInvocation], Awaitable[ToolResult]]", copilot_tool.handler) - result = await handler(ToolInvocation(arguments={"x": 7})) + with caplog.at_level("WARNING", logger="agent_framework.github_copilot"): + hooks = agent._build_session_hooks([safe], {}) # type: ignore[reportPrivateUsage] - assert result.result_type == "success" - assert result.text_result_for_llm == "async=7" + assert hooks == {"on_pre_tool_use": user_hook} + assert not any(record.levelname == "WARNING" for record in caplog.records) - async def test_callback_failure_denies_safely( + def test_runtime_on_pre_tool_use_overrides_default_options( self, mock_client: MagicMock, ) -> None: - """A callback that raises must result in denial, not in tool execution.""" - from agent_framework import Content, tool + """A per-run on_pre_tool_use option takes precedence over default_options.""" - invocations: list[Any] = [] + def default_hook(_input: Any, _context: Any) -> Any: + return None - def boom(call: Content) -> bool: - raise RuntimeError("nope") + def runtime_hook(_input: Any, _context: Any) -> Any: + return {"permissionDecision": "deny"} @tool(approval_mode="always_require") - def dangerous(x: int) -> str: + def dangerous(path: str) -> str: """A tool that requires human approval.""" - invocations.append(x) - return f"x={x}" + return f"deleted {path}" agent = GitHubCopilotAgent( client=mock_client, - default_options=copilot_options({"on_function_approval": boom}), + default_options=copilot_options({"on_pre_tool_use": default_hook}), ) - copilot_tool = agent._tool_to_copilot_tool(dangerous) # type: ignore[reportPrivateUsage] - handler = cast("Callable[[ToolInvocation], Awaitable[ToolResult]]", copilot_tool.handler) - result = await handler(ToolInvocation(arguments={"x": 1})) + hooks = agent._build_session_hooks([dangerous], {"on_pre_tool_use": runtime_hook}) # type: ignore[reportPrivateUsage] + assert hooks == {"on_pre_tool_use": runtime_hook} - assert invocations == [] - assert result.result_type == "failure" - assert result.error == "approval_denied" - - async def test_handler_does_not_invoke_callback_for_never_require( + async def test_default_hook_forwarded_to_create_session( self, mock_client: MagicMock, + mock_session: MagicMock, + assistant_message_event: SessionEvent, ) -> None: - """Tools without approval_mode='always_require' must not trigger the callback.""" - from agent_framework import Content, tool - - callback_calls: list[Any] = [] - - def approve(call: Content) -> bool: - callback_calls.append(call) - return True - - @tool - def safe(x: int) -> str: - """A tool that does not require approval.""" - return f"safe={x}" + """An always_require tool causes the default hook to be forwarded to the SDK session.""" + mock_session.send_and_wait.return_value = assistant_message_event - agent = GitHubCopilotAgent( - client=mock_client, - default_options=copilot_options({"on_function_approval": approve}), - ) - copilot_tool = agent._tool_to_copilot_tool(safe) # type: ignore[reportPrivateUsage] + @tool(approval_mode="always_require") + def dangerous(path: str) -> str: + """A tool that requires human approval.""" + return f"deleted {path}" - handler = cast("Callable[[ToolInvocation], Awaitable[ToolResult]]", copilot_tool.handler) - result = await handler(ToolInvocation(arguments={"x": 5})) + agent = GitHubCopilotAgent(client=mock_client, tools=[dangerous]) + await agent.run("hello") - assert callback_calls == [] - assert result.result_type == "success" - assert result.text_result_for_llm == "safe=5" + hooks = mock_client.create_session.call_args.kwargs["hooks"] + assert hooks is not None + assert "on_pre_tool_use" in hooks class TestGitHubCopilotAgentErrorHandling: @@ -2491,22 +2457,54 @@ async def after_run( assert observed_options.get("timeout") == 120 - async def test_runtime_on_function_approval_rejected(self, mock_client: MagicMock) -> None: - """Passing on_function_approval at runtime must raise rather than be silently ignored.""" + async def test_runtime_on_pre_tool_use_forwarded( + self, + mock_client: MagicMock, + mock_session: MagicMock, + assistant_message_event: SessionEvent, + ) -> None: + """Passing on_pre_tool_use at runtime is accepted and forwarded to the session.""" + mock_session.send_and_wait.return_value = assistant_message_event + + def runtime_hook(_input: Any, _context: Any) -> Any: + return {"permissionDecision": "deny"} + agent = GitHubCopilotAgent(client=mock_client) - with pytest.raises(ValueError, match="on_function_approval"): - await agent.run("hello", options=cast(Any, {"on_function_approval": lambda _c: True})) + await agent.run("hello", options=cast(Any, {"on_pre_tool_use": runtime_hook})) + + hooks = mock_client.create_session.call_args.kwargs["hooks"] + assert hooks == {"on_pre_tool_use": runtime_hook} + + async def test_runtime_on_pre_tool_use_forwarded_streaming( + self, + mock_client: MagicMock, + mock_session: MagicMock, + assistant_delta_event: SessionEvent, + session_idle_event: SessionEvent, + ) -> None: + """Passing on_pre_tool_use at runtime is accepted on the streaming path too.""" + events = [assistant_delta_event, session_idle_event] + + def mock_on(handler: Any) -> Any: + for event in events: + handler(event) + return lambda: None + + mock_session.on = mock_on + + def runtime_hook(_input: Any, _context: Any) -> Any: + return {"permissionDecision": "deny"} - async def test_runtime_on_function_approval_rejected_streaming(self, mock_client: MagicMock) -> None: - """Passing on_function_approval at runtime must raise on the streaming path too.""" agent = GitHubCopilotAgent(client=mock_client) - with pytest.raises(ValueError, match="on_function_approval"): - async for _ in agent.run( - "hello", - stream=True, - options=cast(Any, {"on_function_approval": lambda _c: True}), - ): - pass + async for _ in agent.run( + "hello", + stream=True, + options=cast(Any, {"on_pre_tool_use": runtime_hook}), + ): + pass + + hooks = mock_client.create_session.call_args.kwargs["hooks"] + assert hooks == {"on_pre_tool_use": runtime_hook} async def test_provider_tools_forwarded_to_session( self, diff --git a/python/samples/02-agents/providers/github_copilot/github_copilot_with_function_approval.py b/python/samples/02-agents/providers/github_copilot/github_copilot_with_function_approval.py index 272aa2acb9a..72645dba38d 100644 --- a/python/samples/02-agents/providers/github_copilot/github_copilot_with_function_approval.py +++ b/python/samples/02-agents/providers/github_copilot/github_copilot_with_function_approval.py @@ -3,23 +3,23 @@ """ GitHub Copilot Agent with Function Approval -This sample demonstrates how to enforce ``approval_mode="always_require"`` on a -``FunctionTool`` when using ``GitHubCopilotAgent``. Because the Copilot CLI -runs its own tool-calling loop, the standard agent-framework approval -round-trip (``FunctionApprovalRequestContent`` → ``FunctionApprovalResponseContent``) -is not available — the agent instead awaits an ``on_function_approval`` -callback inside the tool handler before executing the tool. - -Key points: -- ``on_function_approval`` is set on ``GitHubCopilotOptions`` and receives a - ``FunctionCallContent`` describing the pending call. It must return ``True`` - to allow execution or ``False`` to deny it. Async callbacks are also - supported. -- If no callback is configured, calls to ``always_require`` tools are denied - by default and the model receives an explanatory error so it can react. -- This callback is independent of ``on_permission_request``, which gates the - Copilot SDK's *built-in* shell/file actions; ``on_function_approval`` gates - agent-framework ``FunctionTool`` calls. +This sample demonstrates how ``approval_mode="always_require"`` on a +``FunctionTool`` is enforced when using ``GitHubCopilotAgent``. Because the +Copilot CLI runs its own tool-calling loop, approval is enforced through the +Copilot SDK's native pre-execution hook (``on_pre_tool_use``) rather than the +standard agent-framework approval round-trip. + +How it works: +- When you register a tool declared with ``approval_mode="always_require"`` and + you do **not** supply your own ``on_pre_tool_use`` hook, the agent 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. With the default deny-all permission handler, + such a tool is therefore denied unless you wire an approving handler. +- If you supply your own ``on_pre_tool_use`` hook, it takes precedence and you + are responsible for enforcing approval; the agent logs a warning naming any + approval-required tool that your hook must handle. Environment variables (optional): - GITHUB_COPILOT_CLI_PATH: Path to the Copilot CLI executable. @@ -30,15 +30,27 @@ from random import randrange from typing import Annotated -from agent_framework import Content, tool +from agent_framework import tool from agent_framework.github import GitHubCopilotAgent, GitHubCopilotOptions -from copilot.session import PermissionHandler -from dotenv import load_dotenv - -load_dotenv() - - -# Always-require tool: execution must be gated by on_function_approval. +from copilot.generated.rpc import PermissionDecisionReject +from copilot.session import ( + PermissionHandler, + PermissionRequestResult, + PreToolUseHookInput, + PreToolUseHookOutput, +) +from copilot.session_events import PermissionRequest + +# Keep the demos driven by the function tool: tell the model to rely on the tool +# and not fall back to web browsing when the tool is unavailable. +INSTRUCTIONS = ( + "You are a helpful weather assistant. Always answer weather questions by calling the " + "get_weather_detail tool. Do not browse the web or use any other source." +) + + +# Always-require tool: execution is gated by the default on_pre_tool_use hook, +# which routes the decision to on_permission_request. @tool(approval_mode="always_require") def get_weather_detail(location: Annotated[str, "The city and state, e.g. San Francisco, CA"]) -> str: """Get a detailed weather report for a location.""" @@ -49,42 +61,25 @@ def get_weather_detail(location: Annotated[str, "The city and state, e.g. San Fr ) -async def prompt_for_approval(call: Content) -> bool: - """Async approval callback that prompts the user interactively. - - The callback receives a ``FunctionCallContent`` so the operator can review - the tool name and arguments before deciding. Returning ``True`` allows the - call; returning ``False`` denies it and a tool-error is returned to the - model. - - Uses ``asyncio.to_thread`` so the event loop is not blocked by ``input()``. - """ - print(f"\n [Function Approval Request]\n Tool: {call.name}\n Arguments: {call.arguments}") - response = (await asyncio.to_thread(input, " Approve this tool call? (y/n): ")).strip().lower() - return response in ("y", "yes") - +def approve_all_requests(request: PermissionRequest, context: dict[str, str]) -> PermissionRequestResult: + """Permission handler that approves every request, including the gated tool.""" + print(f"\n [Permission requested: {request.kind}] -> approved") + return PermissionHandler.approve_all(request, context) -def auto_approve(call: Content) -> bool: - """Synchronous approval callback that always approves. - Use a sync callback for simple, non-blocking decisions that don't require - I/O (e.g. checking an allow-list of tool names). - """ - print(f"\n [Function Approval Request]\n Tool: {call.name}\n Arguments: {call.arguments}") - print(" -> Auto-approved") - return True +def deny_all_requests(request: PermissionRequest, _context: dict[str, str]) -> PermissionRequestResult: + """Permission handler that denies every request.""" + print(f"\n [Permission requested: {request.kind}] -> denied") + return PermissionDecisionReject(feedback="Denied by the operator's policy.") -async def run_with_interactive_callback() -> None: - """Demonstrates an interactive approval prompt before tool execution.""" - print("\n=== GitHub Copilot Agent: interactive approval callback ===") +async def run_with_approval() -> None: + """The approval-required tool runs because on_permission_request approves it.""" + print("\n=== GitHub Copilot Agent: approval-required tool (approved) ===") agent: GitHubCopilotAgent[GitHubCopilotOptions] = GitHubCopilotAgent( - instructions="You are a helpful weather assistant.", + instructions=INSTRUCTIONS, tools=[get_weather_detail], - default_options=GitHubCopilotOptions( - on_function_approval=prompt_for_approval, - on_permission_request=PermissionHandler.approve_all, - ), + default_options=GitHubCopilotOptions(on_permission_request=approve_all_requests), ) async with agent: query = "Give me the detailed weather for Seattle." @@ -93,39 +88,48 @@ async def run_with_interactive_callback() -> None: print(f"Agent: {result}") -async def run_with_auto_approve_callback() -> None: - """Demonstrates a synchronous callback that always approves.""" - print("\n=== GitHub Copilot Agent: synchronous auto-approve callback ===") +async def run_with_denial() -> None: + """The approval-required tool is blocked because on_permission_request denies it.""" + print("\n=== GitHub Copilot Agent: approval-required tool (denied) ===") agent: GitHubCopilotAgent[GitHubCopilotOptions] = GitHubCopilotAgent( - instructions="You are a helpful weather assistant.", + instructions=INSTRUCTIONS, tools=[get_weather_detail], - default_options=GitHubCopilotOptions( - on_function_approval=auto_approve, - on_permission_request=PermissionHandler.approve_all, - ), + default_options=GitHubCopilotOptions(on_permission_request=deny_all_requests), ) async with agent: - query = "Give me the detailed weather for Tokyo." + query = "Give me the detailed weather for Paris." print(f"User: {query}") result = await agent.run(query) print(f"Agent: {result}") -async def run_without_callback() -> None: - """Default-deny demonstration. +async def run_with_custom_hook() -> None: + """A caller-supplied on_pre_tool_use hook takes precedence over the default. - With no ``on_function_approval`` configured, the always-require tool is - refused and the model receives an explanatory error, so it can apologise - or try a different approach instead of silently failing. + When you provide your own hook you own approval enforcement entirely, so the + agent does not install its default ask-hook and logs a warning naming any + ``always_require`` tool it will no longer auto-gate. Here the custom hook + approves the tool directly by returning ``"allow"`` — note that, unlike the + default ``"ask"`` flow, this does not route through ``on_permission_request`` + (so no permission request is raised for the tool). """ - print("\n=== GitHub Copilot Agent: no callback configured (deny by default) ===") + print("\n=== GitHub Copilot Agent: custom on_pre_tool_use hook (takes precedence) ===") + + def my_pre_tool_use(hook_input: PreToolUseHookInput, _context: dict[str, str]) -> PreToolUseHookOutput | None: + if hook_input.get("toolName") == "get_weather_detail": + return {"permissionDecision": "allow", "permissionDecisionReason": "Allowed by custom policy."} + return None + agent: GitHubCopilotAgent[GitHubCopilotOptions] = GitHubCopilotAgent( - instructions="You are a helpful weather assistant.", + instructions=INSTRUCTIONS, tools=[get_weather_detail], - default_options=GitHubCopilotOptions(on_permission_request=PermissionHandler.approve_all), + default_options=GitHubCopilotOptions( + on_pre_tool_use=my_pre_tool_use, + on_permission_request=approve_all_requests, + ), ) async with agent: - query = "Give me the detailed weather for Paris." + query = "Give me the detailed weather for Tokyo." print(f"User: {query}") result = await agent.run(query) print(f"Agent: {result}") @@ -133,9 +137,9 @@ async def run_without_callback() -> None: async def main() -> None: print("=== GitHub Copilot Agent: Function approval enforcement ===") - await run_with_interactive_callback() - await run_with_auto_approve_callback() - await run_without_callback() + await run_with_approval() + await run_with_denial() + await run_with_custom_hook() if __name__ == "__main__": From a41cebb9b25973d10b98682111b7799b12a356fc Mon Sep 17 00:00:00 2001 From: Giles Odigwe Date: Thu, 25 Jun 2026 14:00:57 -0700 Subject: [PATCH 2/2] Fix type-checker errors and restore load_dotenv in sample Use a complete PreToolUseHookInput in on_pre_tool_use hook tests so pyright/pyrefly/ty/zuban no longer report missing required TypedDict keys. Restore load_dotenv() in the function-approval sample for consistency with the other GitHub Copilot samples (PR review feedback). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../tests/test_github_copilot_agent.py | 17 ++++++++++++++--- .../github_copilot_with_function_approval.py | 6 ++++-- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/python/packages/github_copilot/tests/test_github_copilot_agent.py b/python/packages/github_copilot/tests/test_github_copilot_agent.py index 63b24858e82..de62d723b82 100644 --- a/python/packages/github_copilot/tests/test_github_copilot_agent.py +++ b/python/packages/github_copilot/tests/test_github_copilot_agent.py @@ -25,7 +25,7 @@ tool, ) from agent_framework.exceptions import AgentException -from copilot.session import PermissionHandler +from copilot.session import PermissionHandler, PreToolUseHookInput from copilot.session_events import ( Data, SessionEvent, @@ -43,6 +43,17 @@ def copilot_options(options: GitHubCopilotOptions) -> GitHubCopilotOptions: return options +def pre_tool_use_input(tool_name: str) -> PreToolUseHookInput: + """Build a complete PreToolUseHookInput for exercising on_pre_tool_use hooks in tests.""" + return { + "sessionId": "test-session", + "timestamp": datetime.now(timezone.utc), + "workingDirectory": ".", + "toolName": tool_name, + "toolArgs": {}, + } + + def create_session_event( event_type: SessionEventType, content: str | None = None, @@ -1670,7 +1681,7 @@ def safe(x: int) -> str: assert hooks is not None hook = hooks["on_pre_tool_use"] - approval_decision = hook({"toolName": "dangerous"}, {"session_id": "s"}) + approval_decision = hook(pre_tool_use_input("dangerous"), {"session_id": "s"}) assert approval_decision == { "permissionDecision": "ask", "permissionDecisionReason": ( @@ -1678,7 +1689,7 @@ def safe(x: int) -> str: ), } - assert hook({"toolName": "safe"}, {"session_id": "s"}) is None + assert hook(pre_tool_use_input("safe"), {"session_id": "s"}) is None def test_no_hook_when_no_approval_required_tools( self, diff --git a/python/samples/02-agents/providers/github_copilot/github_copilot_with_function_approval.py b/python/samples/02-agents/providers/github_copilot/github_copilot_with_function_approval.py index 72645dba38d..0502be7ea18 100644 --- a/python/samples/02-agents/providers/github_copilot/github_copilot_with_function_approval.py +++ b/python/samples/02-agents/providers/github_copilot/github_copilot_with_function_approval.py @@ -40,9 +40,11 @@ PreToolUseHookOutput, ) from copilot.session_events import PermissionRequest +from dotenv import load_dotenv + +load_dotenv() + -# Keep the demos driven by the function tool: tell the model to rely on the tool -# and not fall back to web browsing when the tool is unavailable. INSTRUCTIONS = ( "You are a helpful weather assistant. Always answer weather questions by calling the " "get_weather_detail tool. Do not browse the web or use any other source."