From 9f0df633607a695e0953a25d114e8cc398831d4f Mon Sep 17 00:00:00 2001 From: Giles Odigwe Date: Thu, 16 Jul 2026 08:16:42 -0700 Subject: [PATCH 1/3] Python: forward GitHubCopilotOptions verbatim to create_session Refactor the GitHub Copilot agent to forward the full options dict to the Copilot SDK's create_session/resume_session instead of hand-mapping a fixed subset. GitHubCopilotOptions stays as the curated, typed surface, but any other create_session parameter (reasoning_effort, context_tier, enable_citations, ...) is now passed through verbatim. Unknown keys surface as TypeError from the SDK instead of being silently dropped. De-duplicates the near-identical _create_session/_resume_session bodies into a shared _build_session_kwargs helper. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: f9d016d5-4d8c-43f0-a8fb-f3cf3d1ad7eb --- .../agent_framework_github_copilot/_agent.py | 124 ++++++++---------- .../tests/test_github_copilot_agent.py | 66 +++++----- 2 files changed, 86 insertions(+), 104 deletions(-) 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 d8ea5536a41..e136fe0a362 100644 --- a/python/packages/github_copilot/agent_framework_github_copilot/_agent.py +++ b/python/packages/github_copilot/agent_framework_github_copilot/_agent.py @@ -162,7 +162,16 @@ class GitHubCopilotSettings(TypedDict, total=False): class GitHubCopilotOptions(TypedDict, total=False): - """GitHub Copilot-specific options.""" + """GitHub Copilot-specific options. + + The keys below have first-class typing and inline documentation because they are + the commonly used options. They are **not** an exhaustive list: any other + parameter accepted by the Copilot SDK's ``create_session`` (for example + ``reasoning_effort``, ``context_tier``, ``enable_citations``, ``available_tools``, + ``memory``, ...) may also be supplied and is forwarded verbatim to the SDK. An + unrecognized parameter name surfaces as a ``TypeError`` from the SDK, so typos are + caught rather than silently ignored. + """ system_message: SystemMessageConfig """System message configuration for the session. Use mode 'append' to add to the default @@ -355,11 +364,6 @@ def __init__( timeout = opts.pop("timeout", None) log_level = opts.pop("log_level", None) on_permission_request: PermissionHandlerType | None = opts.pop("on_permission_request", None) - 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) - skill_directories: list[str] | None = opts.pop("skill_directories", None) - disabled_skills: list[str] | None = opts.pop("disabled_skills", None) on_pre_tool_use: PreToolUseHandler | None = opts.pop("on_pre_tool_use", None) on_function_approval: FunctionApprovalCallback | None = opts.pop("on_function_approval", None) base_directory = opts.pop("base_directory", None) @@ -397,11 +401,9 @@ def __init__( self._permission_handler = on_permission_request self._on_pre_tool_use: PreToolUseHandler | None = on_pre_tool_use self._function_approval_handler: FunctionApprovalCallback | None = on_function_approval - self._mcp_servers = mcp_servers - self._provider = provider - self._instruction_directories = instruction_directories - self._skill_directories = skill_directories - self._disabled_skills = disabled_skills + # Remaining options (e.g. mcp_servers, provider, instruction_directories, + # skill_directories, disabled_skills, and any other create_session parameter) + # are forwarded verbatim to the Copilot SDK by _build_session_kwargs. self._default_options = opts self._started = False @@ -1107,6 +1109,47 @@ async def _get_or_create_session( except Exception as ex: raise AgentException(f"Failed to create GitHub Copilot session: {ex}") from ex + def _build_session_kwargs( + self, + streaming: bool, + runtime_options: dict[str, Any] | None, + ) -> dict[str, Any]: + """Assemble keyword arguments for ``create_session`` / ``resume_session``. + + Options are layered: the agent's ``default_options`` first, then per-run + ``runtime_options`` which override them. Every key is forwarded verbatim to + the Copilot SDK, so any ``create_session`` parameter is supported without a + dedicated mapping here (an unknown name surfaces as a ``TypeError`` from the + SDK). A few keys are handled specially because they need a secure default + (``on_permission_request`` defaults to denying all requests) or transforming: + ``tools`` are merged with the agent's tools and converted to SDK tools, and + approval callbacks are turned into ``hooks``. + + Args: + streaming: Whether to enable streaming for the session. + runtime_options: Runtime options that take precedence over default_options. + + Returns: + The keyword arguments to splat into the SDK session factory. + """ + opts = runtime_options or {} + + # Passthrough layer: agent defaults first, per-run options override. + kwargs: dict[str, Any] = {**self._default_options, **opts} + + # Merge agent-level and per-run tools, then convert to SDK tools. + all_tools = list(self._tools or []) + list(opts.get("tools") or []) + kwargs["tools"] = self._prepare_tools(all_tools) if all_tools else None + + kwargs["streaming"] = streaming + kwargs["model"] = opts.get("model") or self._settings.get("model") or None + kwargs["on_permission_request"] = ( + opts.get("on_permission_request") or self._permission_handler or _deny_all_permissions + ) + kwargs["hooks"] = self._build_session_hooks(all_tools, opts) + + return kwargs + async def _create_session( self, streaming: bool, @@ -1121,34 +1164,7 @@ async def _create_session( if not self._client: raise RuntimeError("GitHub Copilot client not initialized. Call start() first.") - opts = runtime_options or {} - model = opts.get("model") or self._settings.get("model") or None - system_message = opts.get("system_message") or self._default_options.get("system_message") or None - permission_handler: PermissionHandlerType = ( - opts.get("on_permission_request") or self._permission_handler or _deny_all_permissions - ) - mcp_servers = opts.get("mcp_servers") or self._mcp_servers or None - provider = opts.get("provider") or self._provider or None - instruction_directories = opts.get("instruction_directories", self._instruction_directories) - skill_directories = opts.get("skill_directories", self._skill_directories) - disabled_skills = opts.get("disabled_skills", self._disabled_skills) - 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, - streaming=streaming, - model=model or None, - system_message=system_message or None, - tools=tools or None, - mcp_servers=mcp_servers or None, - provider=provider or None, - instruction_directories=instruction_directories, - skill_directories=skill_directories, - disabled_skills=disabled_skills, - hooks=hooks, - ) + return await self._client.create_session(**self._build_session_kwargs(streaming, runtime_options)) async def _resume_session( self, @@ -1166,35 +1182,7 @@ async def _resume_session( if not self._client: raise RuntimeError("GitHub Copilot client not initialized. Call start() first.") - opts = runtime_options or {} - model = opts.get("model") or self._settings.get("model") or None - system_message = opts.get("system_message") or self._default_options.get("system_message") or None - permission_handler: PermissionHandlerType = ( - opts.get("on_permission_request") or self._permission_handler or _deny_all_permissions - ) - mcp_servers = opts.get("mcp_servers") or self._mcp_servers or None - provider = opts.get("provider") or self._provider or None - instruction_directories = opts.get("instruction_directories", self._instruction_directories) - skill_directories = opts.get("skill_directories", self._skill_directories) - disabled_skills = opts.get("disabled_skills", self._disabled_skills) - 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, - on_permission_request=permission_handler, - streaming=streaming, - model=model or None, - system_message=system_message or None, - tools=tools or None, - mcp_servers=mcp_servers or None, - provider=provider or None, - instruction_directories=instruction_directories, - skill_directories=skill_directories, - disabled_skills=disabled_skills, - hooks=hooks, - ) + return await self._client.resume_session(session_id, **self._build_session_kwargs(streaming, runtime_options)) class GitHubCopilotAgent( # type: ignore[misc] 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 1ad6900b353..9267ef307e1 100644 --- a/python/packages/github_copilot/tests/test_github_copilot_agent.py +++ b/python/packages/github_copilot/tests/test_github_copilot_agent.py @@ -240,34 +240,34 @@ def test_default_options_returns_independent_copy(self) -> None: assert agent._settings.get("model") == "gpt-5.1-mini" def test_init_stores_instruction_directories(self) -> None: - """Test that instruction_directories are stored on the agent instance.""" + """Test that instruction_directories are retained for passthrough to the SDK.""" agent = GitHubCopilotAgent(default_options=copilot_options({"instruction_directories": ["/my/instructions"]})) - assert agent._instruction_directories == ["/my/instructions"] # type: ignore + assert agent._default_options.get("instruction_directories") == ["/my/instructions"] def test_init_without_instruction_directories(self) -> None: - """Test that instruction_directories default to None when not provided.""" + """Test that instruction_directories are absent when not provided.""" agent = GitHubCopilotAgent() - assert agent._instruction_directories is None # type: ignore + assert "instruction_directories" not in agent._default_options def test_init_stores_skill_directories(self) -> None: - """Test that skill_directories are stored on the agent instance.""" + """Test that skill_directories are retained for passthrough to the SDK.""" agent = GitHubCopilotAgent(default_options=copilot_options({"skill_directories": ["/my/skills"]})) - assert agent._skill_directories == ["/my/skills"] # type: ignore + assert agent._default_options.get("skill_directories") == ["/my/skills"] def test_init_without_skill_directories(self) -> None: - """Test that skill_directories default to None when not provided.""" + """Test that skill_directories are absent when not provided.""" agent = GitHubCopilotAgent() - assert agent._skill_directories is None # type: ignore + assert "skill_directories" not in agent._default_options def test_init_stores_disabled_skills(self) -> None: - """Test that disabled_skills are stored on the agent instance.""" + """Test that disabled_skills are retained for passthrough to the SDK.""" agent = GitHubCopilotAgent(default_options=copilot_options({"disabled_skills": ["skill-a"]})) - assert agent._disabled_skills == ["skill-a"] # type: ignore + assert agent._default_options.get("disabled_skills") == ["skill-a"] def test_init_without_disabled_skills(self) -> None: - """Test that disabled_skills default to None when not provided.""" + """Test that disabled_skills are absent when not provided.""" agent = GitHubCopilotAgent() - assert agent._disabled_skills is None # type: ignore + assert "disabled_skills" not in agent._default_options class TestGitHubCopilotAgentLifecycle: @@ -1085,16 +1085,10 @@ async def test_session_resumed_for_same_session( mock_client.create_session.assert_called_once() mock_client.resume_session.assert_called_once_with( mock_session.session_id, - on_permission_request=unittest.mock.ANY, + tools=unittest.mock.ANY, streaming=unittest.mock.ANY, model=unittest.mock.ANY, - system_message=unittest.mock.ANY, - tools=unittest.mock.ANY, - mcp_servers=unittest.mock.ANY, - provider=unittest.mock.ANY, - instruction_directories=unittest.mock.ANY, - skill_directories=unittest.mock.ANY, - disabled_skills=unittest.mock.ANY, + on_permission_request=unittest.mock.ANY, hooks=unittest.mock.ANY, ) @@ -1265,7 +1259,7 @@ async def test_instruction_directories_none_when_not_specified( mock_client: MagicMock, mock_session: MagicMock, ) -> None: - """Test that instruction_directories is None when not specified.""" + """Test that instruction_directories is omitted when not specified.""" agent = GitHubCopilotAgent(client=mock_client) await agent.start() @@ -1273,7 +1267,7 @@ async def test_instruction_directories_none_when_not_specified( call_args = mock_client.create_session.call_args config = call_args.kwargs - assert config["instruction_directories"] is None + assert "instruction_directories" not in config async def test_instruction_directories_empty_list_clears_defaults( self, @@ -1359,7 +1353,7 @@ async def test_skill_directories_none_when_not_specified( mock_client: MagicMock, mock_session: MagicMock, ) -> None: - """Test that skill_directories is None when not specified.""" + """Test that skill_directories is omitted when not specified.""" agent = GitHubCopilotAgent(client=mock_client) await agent.start() @@ -1367,7 +1361,7 @@ async def test_skill_directories_none_when_not_specified( call_args = mock_client.create_session.call_args config = call_args.kwargs - assert config["skill_directories"] is None + assert "skill_directories" not in config async def test_skill_directories_empty_list_clears_defaults( self, @@ -1453,7 +1447,7 @@ async def test_disabled_skills_none_when_not_specified( mock_client: MagicMock, mock_session: MagicMock, ) -> None: - """Test that disabled_skills is None when not specified.""" + """Test that disabled_skills is omitted when not specified.""" agent = GitHubCopilotAgent(client=mock_client) await agent.start() @@ -1461,7 +1455,7 @@ async def test_disabled_skills_none_when_not_specified( call_args = mock_client.create_session.call_args config = call_args.kwargs - assert config["disabled_skills"] is None + assert "disabled_skills" not in config async def test_disabled_skills_empty_list_clears_defaults( self, @@ -1594,7 +1588,7 @@ async def test_session_config_excludes_mcp_servers_when_not_set( call_args = mock_client.create_session.call_args config = call_args.kwargs - assert config["mcp_servers"] is None + assert "mcp_servers" not in config class TestGitHubCopilotAgentProvider: @@ -1660,7 +1654,7 @@ async def test_session_config_excludes_provider_when_not_set( self, mock_client: MagicMock, ) -> None: - """Test that provider is None in session config when not set.""" + """Test that provider is omitted from session config when not set.""" agent = GitHubCopilotAgent(client=mock_client) await agent.start() @@ -1668,13 +1662,13 @@ async def test_session_config_excludes_provider_when_not_set( call_args = mock_client.create_session.call_args config = call_args.kwargs - assert config["provider"] is None + assert "provider" not in config async def test_resume_session_excludes_provider_when_not_set( self, mock_client: MagicMock, ) -> None: - """Test that provider is None in resume session config when not set.""" + """Test that provider is omitted from resume session config when not set.""" agent = GitHubCopilotAgent(client=mock_client) await agent.start() @@ -1685,7 +1679,7 @@ async def test_resume_session_excludes_provider_when_not_set( call_args = mock_client.resume_session.call_args config = call_args.kwargs - assert config["provider"] is None + assert "provider" not in config async def test_runtime_provider_takes_precedence( self, @@ -1721,11 +1715,11 @@ async def test_runtime_provider_takes_precedence( assert config["provider"]["type"] == "openai" assert config["provider"]["base_url"] == "https://runtime.openai.com" - async def test_provider_not_leaked_into_default_options( + async def test_provider_retained_in_default_options( self, mock_client: MagicMock, ) -> None: - """Test that provider is popped from opts and not left in _default_options.""" + """Test that provider is retained in _default_options for passthrough to the SDK.""" from copilot.session import ProviderConfig provider: ProviderConfig = { @@ -1739,9 +1733,9 @@ async def test_provider_not_leaked_into_default_options( default_options=copilot_options({"provider": provider, "model": "gpt-5"}), ) - assert "provider" not in agent._default_options - assert agent._provider is not None - assert agent._provider["type"] == "azure" + # model is consumed into settings; provider rides through default_options. + assert "model" not in agent._default_options + assert agent._default_options["provider"]["type"] == "azure" async def test_provider_coexists_with_other_options( self, From 164093a64ab4947ea111f40bb710c89600858a43 Mon Sep 17 00:00:00 2001 From: Giles Odigwe Date: Thu, 16 Jul 2026 09:59:29 -0700 Subject: [PATCH 2/3] Python: address review feedback on GHCP options passthrough - Strip agent-internal/client-level keys (on_pre_tool_use, on_function_approval, timeout, cli_path, log_level, base_directory) from the forwarded kwargs so they cannot leak into create_session/resume_session and raise TypeError. - Source caller tools from the merged options layer so tools supplied via default_options are honored instead of silently dropped. - Honor a caller-supplied native 'hooks' dict in _build_session_hooks (composing with the on_pre_tool_use shortcut) instead of unconditionally overwriting it. - Validate mock create_session/resume_session calls against the real SDK signatures in tests so invalid kwargs surface as TypeError, and add regression tests for the passthrough contract. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: f9d016d5-4d8c-43f0-a8fb-f3cf3d1ad7eb --- .../agent_framework_github_copilot/_agent.py | 51 +++++--- .../tests/test_github_copilot_agent.py | 116 +++++++++++++++++- 2 files changed, 149 insertions(+), 18 deletions(-) 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 e136fe0a362..fd58f9d4cfe 100644 --- a/python/packages/github_copilot/agent_framework_github_copilot/_agent.py +++ b/python/packages/github_copilot/agent_framework_github_copilot/_agent.py @@ -1001,7 +1001,7 @@ async def handler(invocation: ToolInvocation) -> ToolResult: def _build_session_hooks( self, all_tools: Sequence[ToolTypes | CopilotTool], - opts: Mapping[str, Any], + options: Mapping[str, Any], ) -> SessionHooks | None: """Build the ``SessionHooks`` to pass to the Copilot SDK for this session. @@ -1009,11 +1009,14 @@ 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 ``on_pre_tool_use`` (via per-run ``options`` - or ``default_options``), it takes precedence and is returned unchanged. A + - 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 hook is responsible for enforcing - approval. + be automatically gated, since the caller's hooks are 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. @@ -1021,32 +1024,43 @@ def _build_session_hooks( ``on_function_approval`` callback is configured: in that case approval is enforced inside the tool handler (see :meth:`_tool_to_copilot_tool`) to preserve backward-compatible behavior. - - When there are no approval-required tools and no caller hook, ``None`` is + - When there are no approval-required tools and no caller hooks, ``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``. + options: The merged session options (``default_options`` overlaid with + per-run ``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 + 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") + + # 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] = {} + if user_hook is not None: + combined["on_pre_tool_use"] = user_hook + if caller_hooks: + combined.update(caller_hooks) 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 combined: 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 " + "Custom session hooks are configured, so %d approval-required tool(s) (%s) " + "will not be automatically gated by GitHubCopilotAgent. The custom hooks are 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} + return cast("SessionHooks", combined) if not approval_required_names: return None @@ -1137,8 +1151,9 @@ def _build_session_kwargs( # Passthrough layer: agent defaults first, per-run options override. kwargs: dict[str, Any] = {**self._default_options, **opts} - # Merge agent-level and per-run tools, then convert to SDK tools. - all_tools = list(self._tools or []) + list(opts.get("tools") or []) + # Merge agent-level tools with any caller-supplied tools (from default_options + # or per-run options, the latter winning) and convert to SDK tools. + all_tools = list(self._tools or []) + list(kwargs.get("tools") or []) kwargs["tools"] = self._prepare_tools(all_tools) if all_tools else None kwargs["streaming"] = streaming @@ -1146,7 +1161,13 @@ def _build_session_kwargs( kwargs["on_permission_request"] = ( opts.get("on_permission_request") or self._permission_handler or _deny_all_permissions ) - kwargs["hooks"] = self._build_session_hooks(all_tools, opts) + kwargs["hooks"] = self._build_session_hooks(all_tools, kwargs) + + # Strip agent-internal and client-level keys that are consumed here or in the + # run methods (and settings) but are NOT valid create_session parameters, so + # they don't leak through the passthrough layer and raise TypeError. + for key in ("on_pre_tool_use", "on_function_approval", "timeout", "cli_path", "log_level", "base_directory"): + kwargs.pop(key, None) return kwargs 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 9267ef307e1..6f0089d9be8 100644 --- a/python/packages/github_copilot/tests/test_github_copilot_agent.py +++ b/python/packages/github_copilot/tests/test_github_copilot_agent.py @@ -2,6 +2,7 @@ # ruff: noqa: E402 +import inspect import os import unittest.mock from collections.abc import Sequence @@ -91,12 +92,34 @@ def mock_session() -> MagicMock: @pytest.fixture def mock_client(mock_session: MagicMock) -> MagicMock: - """Create a mock CopilotClient.""" + """Create a mock CopilotClient. + + ``create_session`` / ``resume_session`` validate their call arguments against the + real SDK signatures, so that agent-internal or otherwise invalid keyword arguments + (which a permissive mock would silently accept) surface as ``TypeError`` here just + as they would against the real client. + """ + from copilot import CopilotClient + + def _validating(method_name: str) -> "AsyncMock": + real = getattr(CopilotClient, method_name) + sig = inspect.signature(real) + params = list(sig.parameters.values()) + if params and params[0].name == "self": + sig = sig.replace(parameters=params[1:]) + + def _side_effect(*args: Any, **kwargs: Any) -> MagicMock: + # Raises TypeError if args/kwargs are not valid for the real signature. + sig.bind(*args, **kwargs) + return mock_session + + return AsyncMock(side_effect=_side_effect) + client = MagicMock() client.start = AsyncMock() client.stop = AsyncMock(return_value=[]) - client.create_session = AsyncMock(return_value=mock_session) - client.resume_session = AsyncMock(return_value=mock_session) + client.create_session = _validating("create_session") + client.resume_session = _validating("resume_session") return client @@ -1783,6 +1806,93 @@ def my_tool(arg: str) -> str: assert config["tools"] is not None +class TestGitHubCopilotAgentOptionsPassthrough: + """Regression tests for the options-passthrough contract of _build_session_kwargs.""" + + async def test_arbitrary_option_forwarded_verbatim( + self, + mock_client: MagicMock, + ) -> None: + """An option without a dedicated mapping is forwarded verbatim to create_session.""" + agent = GitHubCopilotAgent( + client=mock_client, + default_options=cast(Any, {"reasoning_effort": "high", "context_tier": "large"}), + ) + await agent.start() + + await agent._get_or_create_session(AgentSession()) # type: ignore[reportPrivateUsage] + + config = mock_client.create_session.call_args.kwargs + assert config["reasoning_effort"] == "high" + assert config["context_tier"] == "large" + + async def test_tools_from_default_options_are_honored( + self, + mock_client: MagicMock, + ) -> None: + """Tools supplied via default_options are converted and forwarded, not dropped.""" + from copilot.tools import Tool as CopilotTool + + passthrough_tool = CopilotTool( + name="passthrough", + description="A pre-built SDK tool supplied through default_options.", + handler=AsyncMock(), + parameters={"type": "object", "properties": {}}, + ) + + agent = GitHubCopilotAgent( + client=mock_client, + default_options=cast(Any, {"tools": [passthrough_tool]}), + ) + await agent.start() + + await agent._get_or_create_session(AgentSession()) # type: ignore[reportPrivateUsage] + + config = mock_client.create_session.call_args.kwargs + assert config["tools"] is not None + assert any(t.name == "passthrough" for t in config["tools"]) + + async def test_caller_hooks_forwarded_verbatim( + self, + mock_client: MagicMock, + ) -> None: + """A caller-supplied native ``hooks`` dict is forwarded instead of being clobbered.""" + + def my_pre_tool_use(_input: Any, _context: Any) -> Any: + return None + + hooks = {"on_pre_tool_use": my_pre_tool_use} + agent = GitHubCopilotAgent(client=mock_client, default_options=cast(Any, {"hooks": hooks})) + await agent.start() + + await agent._get_or_create_session(AgentSession()) # type: ignore[reportPrivateUsage] + + config = mock_client.create_session.call_args.kwargs + assert config["hooks"]["on_pre_tool_use"] is my_pre_tool_use + + async def test_internal_keys_do_not_leak_to_create_session( + self, + mock_client: MagicMock, + mock_session: MagicMock, + assistant_message_event: SessionEvent, + ) -> None: + """Agent-internal / client-level keys must not be forwarded to create_session.""" + mock_session.send_and_wait.return_value = assistant_message_event + + def runtime_hook(_input: Any, _context: Any) -> Any: + return None + + agent = GitHubCopilotAgent(client=mock_client) + # timeout and on_pre_tool_use are consumed by the agent, not create_session. + await agent.run("hello", options=cast(Any, {"timeout": 30, "on_pre_tool_use": runtime_hook})) + + config = mock_client.create_session.call_args.kwargs + for leaked in ("timeout", "on_pre_tool_use", "on_function_approval", "cli_path", "log_level", "base_directory"): + assert leaked not in config + # on_pre_tool_use is still honored via the hooks parameter. + assert config["hooks"]["on_pre_tool_use"] is runtime_hook + + class TestGitHubCopilotAgentToolConversion: """Test cases for tool conversion.""" From 18dd6fb25d3a3680743de4d078e7f33172ba100e Mon Sep 17 00:00:00 2001 From: Giles Odigwe Date: Fri, 17 Jul 2026 14:59:55 -0700 Subject: [PATCH 3/3] Python: avoid redundant re-read of model in _build_session_kwargs model is popped from default_options into settings at init, so a per-run model already lands in the merged kwargs. Keep that value when present and only fall back to the resolved setting otherwise, instead of re-reading opts. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: f9d016d5-4d8c-43f0-a8fb-f3cf3d1ad7eb --- .../github_copilot/agent_framework_github_copilot/_agent.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) 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 fd58f9d4cfe..6b28079168e 100644 --- a/python/packages/github_copilot/agent_framework_github_copilot/_agent.py +++ b/python/packages/github_copilot/agent_framework_github_copilot/_agent.py @@ -1157,7 +1157,10 @@ def _build_session_kwargs( kwargs["tools"] = self._prepare_tools(all_tools) if all_tools else None kwargs["streaming"] = streaming - kwargs["model"] = opts.get("model") or self._settings.get("model") or None + # model may already be present from per-run options (merged above); otherwise fall + # back to the resolved setting (which carries the default_options / env model). + if not kwargs.get("model"): + kwargs["model"] = self._settings.get("model") or None kwargs["on_permission_request"] = ( opts.get("on_permission_request") or self._permission_handler or _deny_all_permissions )