Skip to content

Python: fix(core): return MiddlewareTermination(result=...) from agent and chat middleware - #9747

Open
Somay kousis (Somay-kousis) wants to merge 2 commits into
microsoft:mainfrom
Somay-kousis:fix/middleware-termination-result
Open

Somay kousis (Somay-kousis) wants to merge 2 commits into
microsoft:mainfrom
Somay-kousis:fix/middleware-termination-result

Conversation

@Somay-kousis

Copy link
Copy Markdown
Contributor

Motivation & Context

A middleware that raises MiddlewareTermination(result=response) should get response back from Agent.run, as it does when it sets context.result. The agent and chat pipelines suppressed the exception and dropped result=: agent middleware returned no response, and for chat middleware Agent.run crashed with AttributeError: 'NoneType' object has no attribute 'usage_details'. Details in #9471.

Description & Review Guide

  • What are the major changes? AgentMiddlewarePipeline.execute and ChatMiddlewarePipeline.execute catch MiddlewareTermination and use exc.result when context.result was not set, the same rule the function pipeline already follows in _tools.py. The result then goes through the existing post-processing (stream buffering etc.) like a context.result would.
  • What is the impact of these changes? result= works in all three pipelines. A context.result set by the middleware still wins. On a streaming run, a non-streaming result now fails with a clear ValueError: Expected ResponseStream for streaming instead of an AttributeError. A bare MiddlewareTermination with no result is unchanged, as the issue scoped it out.
  • What do you want reviewers to focus on? Whether a plain response passed on a streaming run should be converted to a stream (for example with to_updates()) instead of raising; I kept it out to stay in the issue's scope.

Tests: TestMiddlewareTerminationResult in test_middleware.py (agent and chat pipelines, streaming and non-streaming) and TestMiddlewareTerminationResultWithAgent in test_middleware_with_agent.py (Agent.run end to end): with _middleware.py from main all 8 fail, with the change they pass. test_context_result_wins_over_termination_result pins the precedence (it passes on main too, and fails if exc.result is made to win). tests/core: 7047 passed, 21 skipped. Ruff is clean, and the core typing and test-typing tasks (pyrefly, ty, zuban, pyright, mypy) pass.

Same as the function pipeline: a middleware that raises after call_next() already has the model's answer in context.result, so result= does not replace it.

Related Issue

Fixes #9471

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: AI tools helped find the bug and write the code, the tests and this description. I reviewed all of it and ran the repro, the core tests, and the lint and typing checks myself.

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.

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.

🟡 Changes recommended

The shared test helpers must move into a non-test support module to satisfy the repository’s mandatory helper-layout requirement.

1 open issue
What changed in this PR

Fixes #9471 by preserving MiddlewareTermination(result=...) in Python’s agent and chat middleware pipelines, matching the function pipeline’s behavior.

Changes:

  • Use the exception’s result when context.result is unset, preserving existing precedence and post-processing.
  • Add pipeline tests for streaming, non-streaming, and result precedence.
  • Add end-to-end Agent.run tests confirming termination returns a response without calling the model.
File Description
python/​packages/​core/​tests/​core/​test_middleware.py Adds response factories and pipeline regression tests.
python/​packages/​core/​tests/​core/​test_middleware_with_agent.py Adds end-to-end termination-result tests.
python/​packages/​core/​agent_framework/​_middleware.py Preserves exception results in agent and chat pipelines.

Review effort: Balanced

from agent_framework._sessions import InMemoryHistoryProvider

from .conftest import MockBaseChatClient, MockChatClient
from .test_middleware import _TERMINATION_TEXT, _termination_result

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done in ca09565: TERMINATION_TEXT and termination_result now live in tests/core/utils.py and both test files import them from there. Same tests, all pass.

…utils.py

CODING_STANDARD.md asks for non-fixture helpers in a helper module, not
imported from another test module.

This branch was previously deployed

1 inactive deployment
github-app-auth — ca095655 Deployed Oct 27, 2026 by Somay-kousis via add_label #26259
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

python Usage: [Issues, PRs], Target: Python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Python: [Bug]: MiddlewareTermination(result=...) is dropped by agent and chat middleware; Agent.run crashes for chat middleware

2 participants