Python: [BREAKING] Graduate tool approval middleware out of experimental#7106
Conversation
There was a problem hiding this comment.
Pull request overview
Graduates the Python tool-approval middleware surface from experimental to stable so downstream users can adopt it without opting into experimental warnings, while keeping other harness components under ExperimentalFeature.HARNESS.
Changes:
- Removed
@experimental(feature_id=ExperimentalFeature.HARNESS)decorators (and the now-unused import) fromToolApprovalRule,ToolApprovalState, andToolApprovalMiddleware. - Updated core Python documentation to remove the “Experimental” qualifier for
ToolApprovalMiddleware.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| python/packages/core/AGENTS.md | Updates the harness documentation to describe ToolApprovalMiddleware as non-experimental. |
| python/packages/core/agent_framework/_harness/_tool_approval.py | Removes experimental staging decorators/imports from the tool-approval rule/state/middleware types. |
Python Test Coverage Report •
Python Unit Test Overview
|
||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Automated Code Review
Reviewers: 5 | Confidence: 94%
✓ Correctness
This is a clean, mechanical removal of three
@experimentaldecorators and their now-unused import from _tool_approval.py, graduating ToolApprovalRule, ToolApprovalState, and ToolApprovalMiddleware to stable. The import removal is safe—no other references to experimental or ExperimentalFeature remain in the file. The ExperimentalFeature.HARNESS enum value is still used by numerous other harness modules (_file_access, _agent, _loop, _file_memory, _todo, _background_agents, _memory), so nothing is left dangling. The AGENTS.md documentation update correctly removes the 'Experimental' qualifier. No correctness issues found.
✓ Security Reliability
This PR cleanly removes the
@experimentaldecorators from three classes (ToolAprovalRule, ToolApprovalState, ToolApprovalMiddleware) and their associated import. The grep confirms no remaining references toexperimentalorExperimentalFeaturein the file after the change. There are no security or reliability concerns — the change is purely removing decorator-based warnings and updating documentation. No resource leaks, injection risks, or unhandled failure modes are introduced.
✓ Test Coverage
The PR correctly removes
@experimentaldecorators from ToolApprovalRule, ToolApprovalState, and ToolApprovalMiddleware. Functional test coverage for these classes is solid (13 tests in test_harness_tool_approval.py). However, unlike the analogous todo harness graduation (which added test_todo_harness_graduated_classes_are_not_experimental in test_harness_todo.py:369), this PR does not add a regression test asserting these classes no longer carry feature_id metadata. This is the established pattern in the codebase for graduating experimental features.
✓ Failure Modes
This is a clean, minimal change that removes three
@experimentaldecorators and their associated import from _tool_approval.py, graduating ToolApprovalRule, ToolApprovalState, and ToolApprovalMiddleware to stable status. The import removal is safe (no remaining references in the file), and the ExperimentalFeature.HARNESS enum value is correctly preserved for other harness symbols that remain experimental. No silent failure modes, lost errors, or operational issues are introduced.
✓ Design Approach
I did not find a design-approach issue in this diff. The three symbols being graduated are already treated as normal public API in the package exports and harness tests, and the removed decorator only adds warning/docstring/metadata behavior rather than functional semantics, so this change is consistent with the stated goal of graduating just the tool-approval surface.
Suggestions
- Add a graduation regression test (following the pattern in test_harness_todo.py:369-379) asserting that ToolApprovalRule, ToolApprovalState, and ToolApprovalMiddleware no longer carry
__feature_id__or experimental docstring warnings. This guards against accidental re-introduction of the experimental decorator.
Automated review by westey-m's agents
Motivation & Context
The Microsoft Agent Framework harness depends on several building-block features
that are still marked experimental. As part of releasing those dependencies (see
#6964), the tool approval middleware and its supporting types are now stable
enough to graduate out of experimental status so downstream users can rely on
them without opting into experimental warnings.
Description & Review Guide
What are the major changes?
@experimental(feature_id=ExperimentalFeature.HARNESS)decoratorsfrom the graduated tool-approval types (
ToolApprovalRule,ToolApprovalState,ToolApprovalMiddleware) in_harness/_tool_approval.py, along with the now-unused_feature_stageimport.
packages/core/AGENTS.mdtodrop the "Experimental" qualifier from
ToolApprovalMiddleware.What is the impact of these changes?
experimental warnings.
breaking so preview users of the experimental harness are notified; no
released API is broken.
ExperimentalFeature.HARNESSid is intentionally retained (stillreferenced by other harness symbols that remain experimental).
What do you want reviewers to focus on?
ToolApprovalRule,ToolApprovalState,ToolApprovalMiddleware, and the already-undecorated helper functions andToolApprovalRuleCallbackalias) is the intended graduation scope.Related Issue
Related to #6964
Contribution Checklist
breaking changelabel (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.