mirror of
https://github.com/microsoft/agent-framework.git
synced 2026-06-16 21:04:09 +08:00
Python: [BREAKING] Upgrade github-copilot-sdk to v1.0.0 (stable) (#6292)
* Python: Upgrade github-copilot-sdk to v1.0.0 (stable) Upgrade agent-framework-github-copilot from github-copilot-sdk 1.0.0b2 to the stable 1.0.0 release, adapting to all breaking API changes. Source changes (_agent.py): - SubprocessConfig removed: use RuntimeConnection.for_stdio(path=...) + CopilotClient kwargs (connection, log_level, base_directory) - Import paths: copilot.generated.session_events -> copilot.session_events - Settings: copilot_home -> base_directory (env GITHUB_COPILOT_BASE_DIRECTORY) - Default deny handler: PermissionDecisionUserNotAvailable() (from copilot.generated.rpc) Test changes: - Updated imports and client-construction assertions (kwargs-based) - Permission handler tests use concrete decision types (PermissionDecisionApproveOnce, PermissionDecisionDeniedInteractivelyByUser) Sample changes: - Permission handlers use PermissionHandler.approve_all or sync approve_and_log pattern (v1.0.0 protocol v3 dispatch is incompatible with blocking input() in permission handlers) - Function approval sample uses asyncio.to_thread for interactive prompts - Simplified imports across all samples Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address PR review: scope permission handlers, widen type, add test - Shell sample: only approve kind='shell', deny others - URL sample: only approve kind='url', deny others - Use getattr() for kind-specific attributes to satisfy pyright - Widen PermissionHandlerType to accept async handlers (matches SDK) - Add test for _deny_all_permissions return value Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix validation script and strengthen test assertion - Update scripts/sample_validation/create_dynamic_workflow_executor.py to use copilot.session_events imports and PermissionHandler.approve_all - Assert isinstance(result, PermissionDecisionUserNotAvailable) instead of stringly-typed kind check Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add integration tests for GitHubCopilotAgent Add 6 integration tests mirroring .NET coverage: - Basic non-streaming response - Streaming response - Function tool invocation - Session context (multi-turn) - Session resume by ID - Shell command execution Tests require COPILOT_GITHUB_TOKEN env var (skipped otherwise). Each test cleans up its Copilot session via delete_session. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
Giles Odigwe
committed by
GitHub
Unverified
GitHub
parent
f970a699d8
commit
fe08574a7c
@@ -37,9 +37,10 @@ from agent_framework.exceptions import AgentException
|
||||
from agent_framework.observability import AgentTelemetryLayer
|
||||
|
||||
try:
|
||||
from copilot import CopilotClient, CopilotSession, SubprocessConfig
|
||||
from copilot.generated.session_events import PermissionRequest, SessionEvent, SessionEventType
|
||||
from copilot import CopilotClient, CopilotSession, RuntimeConnection
|
||||
from copilot.generated.rpc import PermissionDecisionUserNotAvailable
|
||||
from copilot.session import MCPServerConfig, PermissionRequestResult, ProviderConfig, SystemMessageConfig
|
||||
from copilot.session_events import PermissionRequest, SessionEvent, SessionEventType
|
||||
from copilot.tools import Tool as CopilotTool
|
||||
from copilot.tools import ToolInvocation, ToolResult
|
||||
except ImportError as _copilot_import_error:
|
||||
@@ -57,8 +58,10 @@ else:
|
||||
DEFAULT_TIMEOUT_SECONDS: float = 60.0
|
||||
"""Default timeout in seconds for Copilot requests."""
|
||||
|
||||
PermissionHandlerType = Callable[[PermissionRequest, dict[str, str]], PermissionRequestResult]
|
||||
"""Type for permission request handlers."""
|
||||
PermissionHandlerType = Callable[
|
||||
[PermissionRequest, dict[str, str]], "PermissionRequestResult | Awaitable[PermissionRequestResult]"
|
||||
]
|
||||
"""Type for permission request handlers. Supports both sync and async callbacks."""
|
||||
|
||||
|
||||
FunctionApprovalCallback = Callable[[Content], "bool | Awaitable[bool]"]
|
||||
@@ -121,7 +124,7 @@ def _deny_all_permissions(
|
||||
_invocation: dict[str, str],
|
||||
) -> PermissionRequestResult:
|
||||
"""Default permission handler that denies all requests."""
|
||||
return PermissionRequestResult()
|
||||
return PermissionDecisionUserNotAvailable()
|
||||
|
||||
|
||||
class GitHubCopilotSettings(TypedDict, total=False):
|
||||
@@ -140,9 +143,9 @@ class GitHubCopilotSettings(TypedDict, total=False):
|
||||
Can be set via environment variable GITHUB_COPILOT_TIMEOUT.
|
||||
log_level: CLI log level.
|
||||
Can be set via environment variable GITHUB_COPILOT_LOG_LEVEL.
|
||||
copilot_home: Directory where the CLI stores session state, configuration,
|
||||
base_directory: Directory where the CLI stores session state, configuration,
|
||||
and other persistent data. Can be set via environment variable
|
||||
GITHUB_COPILOT_COPILOT_HOME. Defaults to ~/.copilot when not set.
|
||||
GITHUB_COPILOT_BASE_DIRECTORY. Defaults to ~/.copilot when not set.
|
||||
Only applicable when the SDK spawns the CLI process (ignored when
|
||||
connecting to an external server via a pre-configured client).
|
||||
"""
|
||||
@@ -151,7 +154,7 @@ class GitHubCopilotSettings(TypedDict, total=False):
|
||||
model: str | None
|
||||
timeout: float | None
|
||||
log_level: str | None
|
||||
copilot_home: str | None
|
||||
base_directory: str | None
|
||||
|
||||
|
||||
class GitHubCopilotOptions(TypedDict, total=False):
|
||||
@@ -314,7 +317,7 @@ class RawGitHubCopilotAgent(BaseAgent, Generic[OptionsT]):
|
||||
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)
|
||||
copilot_home = opts.pop("copilot_home", None)
|
||||
base_directory = opts.pop("base_directory", None)
|
||||
|
||||
self._settings = load_settings(
|
||||
GitHubCopilotSettings,
|
||||
@@ -323,7 +326,7 @@ class RawGitHubCopilotAgent(BaseAgent, Generic[OptionsT]):
|
||||
model=model,
|
||||
timeout=timeout,
|
||||
log_level=log_level,
|
||||
copilot_home=copilot_home,
|
||||
base_directory=base_directory,
|
||||
env_file_path=env_file_path,
|
||||
env_file_encoding=env_file_encoding,
|
||||
)
|
||||
@@ -362,14 +365,16 @@ class RawGitHubCopilotAgent(BaseAgent, Generic[OptionsT]):
|
||||
if self._client is None:
|
||||
cli_path = self._settings.get("cli_path") or None
|
||||
log_level = self._settings.get("log_level") or None
|
||||
copilot_home = self._settings.get("copilot_home") or None
|
||||
base_directory = self._settings.get("base_directory") or None
|
||||
|
||||
subprocess_kwargs: dict[str, Any] = {"cli_path": cli_path}
|
||||
client_kwargs: dict[str, Any] = {}
|
||||
if cli_path:
|
||||
client_kwargs["connection"] = RuntimeConnection.for_stdio(path=cli_path)
|
||||
if log_level:
|
||||
subprocess_kwargs["log_level"] = log_level
|
||||
if copilot_home:
|
||||
subprocess_kwargs["copilot_home"] = copilot_home
|
||||
self._client = CopilotClient(SubprocessConfig(**subprocess_kwargs))
|
||||
client_kwargs["log_level"] = log_level
|
||||
if base_directory:
|
||||
client_kwargs["base_directory"] = base_directory
|
||||
self._client = CopilotClient(**client_kwargs)
|
||||
|
||||
try:
|
||||
await self._client.start()
|
||||
|
||||
@@ -24,7 +24,7 @@ classifiers = [
|
||||
]
|
||||
dependencies = [
|
||||
"agent-framework-core>=1.6.0,<2",
|
||||
"github-copilot-sdk>=1.0.0b2,<=1.0.0b2; python_version >= '3.11'",
|
||||
"github-copilot-sdk>=1.0.0,<2; python_version >= '3.11'",
|
||||
]
|
||||
|
||||
[tool.uv]
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
|
||||
# ruff: noqa: E402
|
||||
|
||||
import os
|
||||
import unittest.mock
|
||||
from datetime import datetime, timezone
|
||||
from typing import Any
|
||||
@@ -20,9 +21,11 @@ from agent_framework import (
|
||||
ContextProvider,
|
||||
HistoryProvider,
|
||||
Message,
|
||||
tool,
|
||||
)
|
||||
from agent_framework.exceptions import AgentException
|
||||
from copilot.generated.session_events import (
|
||||
from copilot.session import PermissionHandler
|
||||
from copilot.session_events import (
|
||||
Data,
|
||||
SessionEvent,
|
||||
SessionEventType,
|
||||
@@ -308,27 +311,27 @@ class TestGitHubCopilotAgentLifecycle:
|
||||
)
|
||||
await agent.start()
|
||||
|
||||
call_args = MockClient.call_args[0][0]
|
||||
assert call_args.cli_path == "/custom/path"
|
||||
assert call_args.log_level == "debug"
|
||||
kwargs = MockClient.call_args.kwargs
|
||||
assert kwargs["connection"].path == "/custom/path"
|
||||
assert kwargs["log_level"] == "debug"
|
||||
|
||||
async def test_start_passes_copilot_home_to_subprocess_config(self) -> None:
|
||||
"""Test that copilot_home is passed through to SubprocessConfig."""
|
||||
async def test_start_passes_base_directory_to_client(self) -> None:
|
||||
"""Test that base_directory is passed through to CopilotClient."""
|
||||
with patch("agent_framework_github_copilot._agent.CopilotClient") as MockClient:
|
||||
mock_client = MagicMock()
|
||||
mock_client.start = AsyncMock()
|
||||
MockClient.return_value = mock_client
|
||||
|
||||
agent: GitHubCopilotAgent[GitHubCopilotOptions] = GitHubCopilotAgent(
|
||||
default_options={"copilot_home": "/custom/copilot/home"}
|
||||
default_options={"base_directory": "/custom/copilot/home"}
|
||||
)
|
||||
await agent.start()
|
||||
|
||||
call_args = MockClient.call_args[0][0]
|
||||
assert call_args.copilot_home == "/custom/copilot/home"
|
||||
kwargs = MockClient.call_args.kwargs
|
||||
assert kwargs["base_directory"] == "/custom/copilot/home"
|
||||
|
||||
async def test_start_copilot_home_not_set_when_unspecified(self) -> None:
|
||||
"""Test that copilot_home is not included in SubprocessConfig when not specified."""
|
||||
async def test_start_base_directory_not_set_when_unspecified(self) -> None:
|
||||
"""Test that base_directory is not included in client kwargs when not specified."""
|
||||
with patch("agent_framework_github_copilot._agent.CopilotClient") as MockClient:
|
||||
mock_client = MagicMock()
|
||||
mock_client.start = AsyncMock()
|
||||
@@ -337,14 +340,14 @@ class TestGitHubCopilotAgentLifecycle:
|
||||
agent = GitHubCopilotAgent()
|
||||
await agent.start()
|
||||
|
||||
call_args = MockClient.call_args[0][0]
|
||||
assert call_args.copilot_home is None
|
||||
kwargs = MockClient.call_args.kwargs
|
||||
assert "base_directory" not in kwargs
|
||||
|
||||
async def test_start_copilot_home_from_env_variable(self) -> None:
|
||||
"""Test that copilot_home can be set via GITHUB_COPILOT_COPILOT_HOME env variable."""
|
||||
async def test_start_base_directory_from_env_variable(self) -> None:
|
||||
"""Test that base_directory can be set via GITHUB_COPILOT_BASE_DIRECTORY env variable."""
|
||||
with (
|
||||
patch("agent_framework_github_copilot._agent.CopilotClient") as MockClient,
|
||||
patch.dict("os.environ", {"GITHUB_COPILOT_COPILOT_HOME": "/env/copilot/home"}),
|
||||
patch.dict("os.environ", {"GITHUB_COPILOT_BASE_DIRECTORY": "/env/copilot/home"}),
|
||||
):
|
||||
mock_client = MagicMock()
|
||||
mock_client.start = AsyncMock()
|
||||
@@ -353,8 +356,8 @@ class TestGitHubCopilotAgentLifecycle:
|
||||
agent = GitHubCopilotAgent()
|
||||
await agent.start()
|
||||
|
||||
call_args = MockClient.call_args[0][0]
|
||||
assert call_args.copilot_home == "/env/copilot/home"
|
||||
kwargs = MockClient.call_args.kwargs
|
||||
assert kwargs["base_directory"] == "/env/copilot/home"
|
||||
|
||||
|
||||
class TestGitHubCopilotAgentRun:
|
||||
@@ -1053,11 +1056,11 @@ class TestGitHubCopilotAgentSessionManagement:
|
||||
mock_session: MagicMock,
|
||||
) -> None:
|
||||
"""Test that resumed session config includes tools and permission handler."""
|
||||
from copilot.generated.session_events import PermissionRequest
|
||||
from copilot.session import PermissionRequestResult
|
||||
from copilot.session import PermissionDecisionApproveOnce, PermissionRequestResult
|
||||
from copilot.session_events import PermissionRequest
|
||||
|
||||
def my_handler(request: PermissionRequest, context: dict[str, str]) -> PermissionRequestResult:
|
||||
return PermissionRequestResult(kind="approved")
|
||||
return PermissionDecisionApproveOnce()
|
||||
|
||||
def my_tool(arg: str) -> str:
|
||||
"""A test tool."""
|
||||
@@ -1869,6 +1872,15 @@ class TestGitHubCopilotAgentErrorHandling:
|
||||
class TestGitHubCopilotAgentPermissions:
|
||||
"""Test cases for permission handling."""
|
||||
|
||||
def test_deny_all_permissions_returns_user_not_available(self) -> None:
|
||||
"""Test that the default deny handler returns PermissionDecisionUserNotAvailable."""
|
||||
from copilot.generated.rpc import PermissionDecisionUserNotAvailable
|
||||
|
||||
from agent_framework_github_copilot._agent import _deny_all_permissions
|
||||
|
||||
result = _deny_all_permissions(MagicMock(), {})
|
||||
assert isinstance(result, PermissionDecisionUserNotAvailable)
|
||||
|
||||
def test_no_permission_handler_when_not_provided(self) -> None:
|
||||
"""Test that no handler is set when on_permission_request is not provided."""
|
||||
agent = GitHubCopilotAgent()
|
||||
@@ -1876,13 +1888,14 @@ class TestGitHubCopilotAgentPermissions:
|
||||
|
||||
def test_permission_handler_set_when_provided(self) -> None:
|
||||
"""Test that a handler is set when on_permission_request is provided."""
|
||||
from copilot.generated.session_events import PermissionRequest
|
||||
from copilot.session import PermissionRequestResult
|
||||
from copilot.generated.rpc import PermissionDecisionDeniedInteractivelyByUser
|
||||
from copilot.session import PermissionDecisionApproveOnce, PermissionRequestResult
|
||||
from copilot.session_events import PermissionRequest
|
||||
|
||||
def approve_shell(request: PermissionRequest, context: dict[str, str]) -> PermissionRequestResult:
|
||||
if request.kind == "shell":
|
||||
return PermissionRequestResult(kind="approved")
|
||||
return PermissionRequestResult(kind="denied-interactively-by-user")
|
||||
return PermissionDecisionApproveOnce()
|
||||
return PermissionDecisionDeniedInteractivelyByUser()
|
||||
|
||||
agent: GitHubCopilotAgent[GitHubCopilotOptions] = GitHubCopilotAgent(
|
||||
default_options={"on_permission_request": approve_shell}
|
||||
@@ -1895,13 +1908,14 @@ class TestGitHubCopilotAgentPermissions:
|
||||
mock_session: MagicMock,
|
||||
) -> None:
|
||||
"""Test that session config includes permission handler when provided."""
|
||||
from copilot.generated.session_events import PermissionRequest
|
||||
from copilot.session import PermissionRequestResult
|
||||
from copilot.generated.rpc import PermissionDecisionDeniedInteractivelyByUser
|
||||
from copilot.session import PermissionDecisionApproveOnce, PermissionRequestResult
|
||||
from copilot.session_events import PermissionRequest
|
||||
|
||||
def approve_shell_read(request: PermissionRequest, context: dict[str, str]) -> PermissionRequestResult:
|
||||
if request.kind in ("shell", "read"):
|
||||
return PermissionRequestResult(kind="approved")
|
||||
return PermissionRequestResult(kind="denied-interactively-by-user")
|
||||
return PermissionDecisionApproveOnce()
|
||||
return PermissionDecisionDeniedInteractivelyByUser()
|
||||
|
||||
agent: GitHubCopilotAgent[GitHubCopilotOptions] = GitHubCopilotAgent(
|
||||
client=mock_client,
|
||||
@@ -2705,3 +2719,163 @@ class TestGitHubCopilotAgentContextProviders:
|
||||
assert call_kwargs.get("tools") is not None
|
||||
tool_names = [t.name for t in call_kwargs["tools"]]
|
||||
assert "load_skill" in tool_names
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Integration tests — require COPILOT_GITHUB_TOKEN env var
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
skip_if_copilot_integration_tests_disabled = pytest.mark.skipif(
|
||||
os.getenv("COPILOT_GITHUB_TOKEN", "") == "",
|
||||
reason="No COPILOT_GITHUB_TOKEN provided; skipping integration tests.",
|
||||
)
|
||||
|
||||
|
||||
@tool(approval_mode="never_require")
|
||||
def get_weather(location: str) -> str:
|
||||
"""Get the weather for a given location."""
|
||||
return f"The weather in {location} is sunny with a high of 25C."
|
||||
|
||||
|
||||
@pytest.mark.flaky
|
||||
@pytest.mark.integration
|
||||
@skip_if_copilot_integration_tests_disabled
|
||||
async def test_integration_run_with_simple_prompt_returns_response() -> None:
|
||||
"""Integration test: basic non-streaming response."""
|
||||
agent = GitHubCopilotAgent(
|
||||
instructions="You are a helpful assistant. Keep your answers short.",
|
||||
default_options={"on_permission_request": PermissionHandler.approve_all},
|
||||
)
|
||||
|
||||
async with agent:
|
||||
session = agent.create_session()
|
||||
response = await agent.run("What is 2 + 2? Answer with just the number.", session=session)
|
||||
|
||||
assert response is not None
|
||||
assert len(response.messages) > 0
|
||||
assert "4" in response.text
|
||||
|
||||
if session.service_session_id and agent._client:
|
||||
await agent._client.delete_session(session.service_session_id)
|
||||
|
||||
|
||||
@pytest.mark.flaky
|
||||
@pytest.mark.integration
|
||||
@skip_if_copilot_integration_tests_disabled
|
||||
async def test_integration_run_streaming_returns_updates() -> None:
|
||||
"""Integration test: streaming response yields updates."""
|
||||
agent = GitHubCopilotAgent(
|
||||
instructions="You are a helpful assistant. Keep your answers short.",
|
||||
default_options={"on_permission_request": PermissionHandler.approve_all},
|
||||
)
|
||||
|
||||
async with agent:
|
||||
session = agent.create_session()
|
||||
updates = []
|
||||
async for chunk in agent.run("Count from 1 to 5.", stream=True, session=session):
|
||||
updates.append(chunk)
|
||||
|
||||
assert len(updates) > 0
|
||||
full_text = "".join(u.text for u in updates if u.text)
|
||||
assert len(full_text) > 0
|
||||
|
||||
if session.service_session_id and agent._client:
|
||||
await agent._client.delete_session(session.service_session_id)
|
||||
|
||||
|
||||
@pytest.mark.flaky
|
||||
@pytest.mark.integration
|
||||
@skip_if_copilot_integration_tests_disabled
|
||||
async def test_integration_run_with_function_tool_invokes_tool() -> None:
|
||||
"""Integration test: function tool is invoked by the agent."""
|
||||
agent = GitHubCopilotAgent(
|
||||
instructions="You are a helpful weather agent. Use the get_weather tool to answer weather questions.",
|
||||
tools=[get_weather],
|
||||
default_options={"on_permission_request": PermissionHandler.approve_all},
|
||||
)
|
||||
|
||||
async with agent:
|
||||
session = agent.create_session()
|
||||
response = await agent.run("What's the weather like in Seattle?", session=session)
|
||||
|
||||
assert response is not None
|
||||
assert len(response.messages) > 0
|
||||
assert any(word in response.text.lower() for word in ["sunny", "25", "weather", "seattle"])
|
||||
|
||||
if session.service_session_id and agent._client:
|
||||
await agent._client.delete_session(session.service_session_id)
|
||||
|
||||
|
||||
@pytest.mark.flaky
|
||||
@pytest.mark.integration
|
||||
@skip_if_copilot_integration_tests_disabled
|
||||
async def test_integration_run_with_session_maintains_context() -> None:
|
||||
"""Integration test: session maintains conversation context across turns."""
|
||||
agent = GitHubCopilotAgent(
|
||||
instructions="You are a helpful assistant. Keep your answers short.",
|
||||
default_options={"on_permission_request": PermissionHandler.approve_all},
|
||||
)
|
||||
|
||||
async with agent:
|
||||
session = agent.create_session()
|
||||
|
||||
response1 = await agent.run("My name is Alice.", session=session)
|
||||
assert response1 is not None
|
||||
|
||||
response2 = await agent.run("What is my name?", session=session)
|
||||
|
||||
assert response2 is not None
|
||||
assert "alice" in response2.text.lower()
|
||||
|
||||
if session.service_session_id and agent._client:
|
||||
await agent._client.delete_session(session.service_session_id)
|
||||
|
||||
|
||||
@pytest.mark.flaky
|
||||
@pytest.mark.integration
|
||||
@skip_if_copilot_integration_tests_disabled
|
||||
async def test_integration_run_with_session_resume_continues_conversation() -> None:
|
||||
"""Integration test: session can be resumed by ID."""
|
||||
agent = GitHubCopilotAgent(
|
||||
instructions="You are a helpful assistant. Keep your answers short.",
|
||||
default_options={"on_permission_request": PermissionHandler.approve_all},
|
||||
)
|
||||
|
||||
async with agent:
|
||||
session1 = agent.create_session()
|
||||
await agent.run("Remember this number: 42.", session=session1)
|
||||
|
||||
session_id = session1.service_session_id
|
||||
assert session_id is not None
|
||||
|
||||
session2 = AgentSession()
|
||||
session2.service_session_id = session_id
|
||||
|
||||
response = await agent.run("What number did I ask you to remember?", session=session2)
|
||||
|
||||
assert response is not None
|
||||
assert "42" in response.text
|
||||
|
||||
if agent._client:
|
||||
await agent._client.delete_session(session_id)
|
||||
|
||||
|
||||
@pytest.mark.flaky
|
||||
@pytest.mark.integration
|
||||
@skip_if_copilot_integration_tests_disabled
|
||||
async def test_integration_run_with_shell_permissions_executes_command() -> None:
|
||||
"""Integration test: shell commands can be executed with permission handler."""
|
||||
agent = GitHubCopilotAgent(
|
||||
instructions="You are a helpful assistant that can execute shell commands.",
|
||||
default_options={"on_permission_request": PermissionHandler.approve_all},
|
||||
)
|
||||
|
||||
async with agent:
|
||||
session = agent.create_session()
|
||||
response = await agent.run("Run a shell command to print 'hello world'", session=session)
|
||||
|
||||
assert response is not None
|
||||
assert "hello" in response.text.lower()
|
||||
|
||||
if session.service_session_id and agent._client:
|
||||
await agent._client.delete_session(session.service_session_id)
|
||||
|
||||
Reference in New Issue
Block a user