Python: Fix HandoffBuilder dropping function-level middleware when cloning agents#5220
Python: Fix HandoffBuilder dropping function-level middleware when cloning agents#5220moonbox3 wants to merge 4 commits intomicrosoft:mainfrom
Conversation
…ents (microsoft#5173) _clone_chat_agent() was using agent.agent_middleware (agent-level only) instead of agent.middleware (all types), which silently dropped any function middleware registered on the original agent. Changed to use agent.middleware to preserve all middleware types (agent, function, and chat) during cloning. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
moonbox3
left a comment
There was a problem hiding this comment.
Automated Code Review
Reviewers: 4 | Confidence: 95% | Result: All clear
Reviewed: Correctness, Security Reliability, Test Coverage, Design Approach
Automated review by moonbox3's agents
Python Test Coverage Report •
Python Unit Test Overview
|
||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Pull request overview
Fixes a handoff workflow cloning bug where HandoffAgentExecutor._clone_chat_agent() only copied agent-level middleware (agent.agent_middleware) and silently dropped function-level middleware, preventing user @function_middleware from running during handoffs.
Changes:
- Update handoff agent cloning to pass
agent.middleware(all middleware types) instead ofagent.agent_middleware. - Add a regression test intended to ensure middleware survives the
HandoffBuilder.build()cloning path.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| python/packages/orchestrations/agent_framework_orchestrations/_handoff.py | Fix clone construction to preserve the original agent’s full middleware list. |
| python/packages/orchestrations/tests/test_handoff.py | Adds a regression test for middleware preservation in handoff agent cloning. |
The test used isinstance(m, FunctionMiddleware) which matched _AutoHandoffMiddleware (always appended during build) instead of the user's @function_middleware decorator. Assert directly that tracking_middleware is present in the cloned agent's middleware list. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…lder drops function-level middleware when cloning agents
moonbox3
left a comment
There was a problem hiding this comment.
Automated Code Review
Reviewers: 4 | Confidence: 95% | Result: All clear
Reviewed: Correctness, Security Reliability, Test Coverage, Design Approach
Automated review by moonbox3's agents
| description=agent.description, | ||
| context_providers=agent.context_providers, | ||
| middleware=agent.agent_middleware, | ||
| middleware=agent.middleware, |
There was a problem hiding this comment.
these changes make me wonder if we need to do a copy method in the agent, we've had multiple regressions here because it is not clear that a change to the agent also might need a change here, and while the public api's of the agent are stable now, we still might need to make internal changes like what went wrong here. WDYT?
Motivation and Context
HandoffAgentExecutor._clone_chat_agent()usedagent.agent_middleware(agent-level only) instead ofagent.middleware(all types), silently discarding any@function_middlewareregistered on the original agent so it never executed during handoff workflows.Fixes #5173
Description
The root cause was in
_handoff.pyline 290: the clone constructor was passedagent.agent_middleware, which only contains agent-level middleware aftercategorize_middleware()splits the list duringAgent.__init__. The fix changes this toagent.middleware, which holds the full original middleware list including both agent and function middleware. A regression test verifies that function middleware survives the cloning step performed byHandoffBuilder.build().Contribution Checklist