Skip to content

Python: [BREAKING] Guard Copilot runtime connections - #9611

Open
Roger Barreto (rogerbarreto) wants to merge 3 commits into
microsoft:mainfrom
rogerbarreto:i744r889-invocations-python
Open

Roger Barreto (rogerbarreto) wants to merge 3 commits into
microsoft:mainfrom
rogerbarreto:i744r889-invocations-python

Conversation

@rogerbarreto

@rogerbarreto Roger Barreto (rogerbarreto) commented Oct 22, 2026 •

Copy link
Copy Markdown
Member

Motivation & Context

Connection option names are agent-specific. Reserving provider and mcp_servers across the Invocations host rejects supported Copilot runtime configuration and settings produced by trusted application callbacks. Put default connection protection in the Copilot agent, where those options are interpreted, while preserving an explicit application-owned compatibility path.

Description & Review Guide

  • What are the major changes? Add constructor-only allow_runtime_connection_overrides=False to GitHubCopilotAgent and RawGitHubCopilotAgent. Reject per-run provider and mcp_servers before Copilot SDK startup and when assembling session configuration; retain developer defaults and ordinary runtime options. Remove the host-wide connection-name restriction. Map AgentInvalidRequestException to sanitized HTTP 400 or typed SSE client errors. Add regression coverage, migration documentation, and a proposed ADR. HTTP regressions isolate AgentServer SDK telemetry so they do not contaminate later tests' process-wide logging state.
  • What is the impact of these changes? Breaking default behavior: existing Copilot callers supplying runtime connection configuration must move it to default_options or construct the agent with allow_runtime_connection_overrides=True. Opted-in runtime connections keep their previous precedence. The constructor flag cannot be enabled through incoming run options. Deploy the updated Copilot package alongside the host; older agent versions retain their existing behavior. Other agents keep ownership of their option semantics.
  • What do you want reviewers to focus on? The agent-owned trust boundary, explicit compatibility opt-in, preservation of application defaults, raw/wrapped and streaming/non-streaming behavior, session creation/resumption, and HTTP/SSE error classification without leaking provider details.

Local checks passed on Windows with Python 3.12.10. Commands below were run from python; typing tasks used the restored worktree environment, pinned uv 0.13.1, and UV_NO_SYNC=true.

  • .\.venv\Scripts\python.exe -m ruff check --no-fix packages\foundry_hosting packages\github_copilot - passed.
  • .\.venv\Scripts\python.exe -m ruff format --check packages\foundry_hosting packages\github_copilot - passed; 30 files already formatted.
  • .\.venv\Scripts\python.exe -m pyright --project packages\github_copilot\pyproject.toml --pythonpath .venv\Scripts\python.exe - passed; zero errors or warnings.
  • .\.venv\Scripts\python.exe -m pyright --project packages\foundry_hosting\pyproject.toml --pythonpath .venv\Scripts\python.exe - passed; zero errors or warnings.
  • .\.venv\Scripts\uv.exe run --no-sync poe test-typing -P github_copilot - passed for mypy, pyrefly, ty, zuban, and Pyright, including after the SDK telemetry-isolation fix.
  • .\.venv\Scripts\uv.exe run --no-sync poe test-typing -P foundry_hosting - passed for mypy, pyrefly, ty, zuban, and Pyright.
  • .\.venv\Scripts\python.exe -m pytest packages\foundry_hosting\tests\test_invocations.py packages\github_copilot\tests\test_github_copilot_agent.py -m 'not integration' -o addopts='' -p no:cacheprovider --tb=short -q - passed; 488 tests, six integration tests deselected. The same 488 regressions also passed with Copilot tests running before Invocations after the isolation fix.
  • .\.venv\Scripts\python.exe -m scripts.check_md_code_blocks packages\github_copilot\README.md packages\foundry_hosting\README.md packages\github_copilot\AGENTS.md ..\docs\decisions\0046-python-copilot-runtime-connections.md --no-glob - passed under the repository's configured snippet exclusions.

Published-head GitHub CI for 3a65ab60fefec2e84709e953f30b6e1cdf89aa51 passed the Merge Gatekeeper, all configured Python unit-test matrix jobs (Python 3.10-3.15 on Linux and 3.10-3.14 on Windows), package checks, test typing, samples/Markdown, and the full-workspace coverage gate.

The separate API-compatibility job still fails on the existing Griffe crash resolving the unchanged trusted-base EdgeGroup.builtin_type alias; #9614 proposes an upstream fix. The merge gate explicitly excludes this check. Local reproduction with the trusted base's pinned Griffe 2.4.0 is blocked because the configured package feed only offers versions through 2.3.2. Publication proceeded with this known failure explicitly acknowledged; this is not a claim that every check is green. Live integrations are not claimed as passed.

Related Issue

N/A - revises the compatibility approach following review on this existing PR. No separately agreed implementation issue is linked; that checklist item remains unchecked. #9614 addresses the independent API-comparison failure, not this connection policy.

AI Assistance

  • No material AI assistance was used.
  • This is an AI-assisted contribution. I reviewed, understood, and verified all submitted content and accept responsibility for it.

AI assistance details: Implementation, regression tests, migration documentation, proposed ADR, validation, and this PR description.

Contribution Checklist

  • The code builds clean without any errors or warnings
  • All unit tests pass, and I have added new tests where possible
  • The PR follows the Contribution Guidelines
  • This PR links to an agreed issue with no competing open PR, or the Related Issue section documents a trivial-change or repository-automation exception.
  • This is not a breaking change. If it is a breaking change, add the breaking change label (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.

Keep connection settings in application configuration while retaining
caller generation options and application defaults.
Copilot AI balanced review requested due to automatic review settings October 22, 2026 18:04
@agent-framework-automation agent-framework-automation Bot added documentation Usage: [Issues, PRs], Target: documentation in the code base and learn docs python Usage: [Issues, PRs], Target: Python labels Oct 22, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The narrow change reuses existing validation, has matching regression coverage and documentation, and has no unresolved findings.

No open issues 🎉

What changed in this PR

This PR reinforces the Foundry Invocations configuration boundary: connection settings remain application-owned, while callers can supply per-turn generation options.

Changes:

  • Add mcp_servers and provider to the existing agent-controlled option validation.
  • Extend rejection tests across streaming modes, wire formats, and unsupported-options policies, and verify application defaults remain unchanged.
  • Document where trusted connection configuration belongs.
File Description
python/​packages/​foundry_hosting/​tests/​test_invocations.py Extend validation coverage and verify connection defaults are preserved.
python/​packages/​foundry_hosting/​README.md Clarify the application-owned connection configuration boundary.
python/​packages/​foundry_hosting/​agent_framework_foundry_hosting/​_invocations.py Reject caller-supplied mcp_servers and provider options.

Review effort: Balanced

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MAF Automated Review — Iteration 1

Result: Findings reported
Scope: full PR (1 commit(s)): 7a37342aaeb6
Model: gpt-5.6-sol

Overview

This PR extends Invocations option validation to reject mcp_servers and provider before agent creation or storage access, with broad coverage across request, parser, hook, streaming, wire-format, and unsupported-option paths. The ordering and application-default preservation guards are strong. However, reserving provider for every hosted agent breaks an existing public per-run option contract used by GitHubCopilotAgent, so the change needs explicit compatibility handling or breaking-change treatment.

Reviewed the supplied pull-request change set across correctness, security/reliability, architecture, and failure behavior.
1 verified finding remained after source verification (1 medium) across 1 file. Details are attached to the affected lines below.

Affected areas: python/packages/foundry_hosting/agent_framework_foundry_hosting/_invocations.py

Comment thread python/packages/foundry_hosting/agent_framework_foundry_hosting/_invocations.py Outdated
Keep connection ownership in Copilot agents without reserving option
names across every Invocations implementation.

BREAKING CHANGE: configure connections in default_options or construct
Copilot agents with allow_runtime_connection_overrides=True.

Refs microsoft#9611
@rogerbarreto Roger Barreto (rogerbarreto) changed the title Python: Preserve Invocations connection ownership Python: [BREAKING] Guard Copilot runtime connections Oct 22, 2026
@agent-framework-automation agent-framework-automation Bot added the breaking change Usage: [PRs], Target: all PRs that introduce changes that are not backward compatible label Oct 22, 2026
Host construction configures global SDK telemetry and contaminates later
Responses log-sanitization tests. Mock SDK tracing for this HTTP test
only, matching Foundry's existing isolation fixture.

Refs microsoft#9611

This branch was successfully deployed

1 active deployment
github-app-auth — 3a65ab60 Deployed Oct 22, 2026 by rogerbarreto via add_label #25985
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking change Usage: [PRs], Target: all PRs that introduce changes that are not backward compatible documentation Usage: [Issues, PRs], Target: documentation in the code base and learn docs python Usage: [Issues, PRs], Target: Python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants