Python: Fixing issue #1366 - Thread corruption when max_iterations is reached.#4234
Merged
alliscode merged 7 commits intomicrosoft:mainfrom Feb 25, 2026
Merged
Conversation
When the function invocation loop exhausts max_iterations while the model keeps requesting tools, the failsafe code path (calling the model with tool_choice='none' and prepending fcc_messages) was unreachable because 'if response is not None: return response' short-circuited before it. The fix removes the premature return so the failsafe always runs after loop exhaustion, making a final model call with tool_choice='none' to produce a clean text answer and prepending accumulated fcc_messages from prior iterations. This matches the existing pattern used by the error threshold and max_function_calls paths. Also unskips test_max_iterations_limit and test_streaming_max_iterations_limit which were previously skipped with 'needs investigation in unified API'. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…n.py Apply ruff format to fix multi-line string concatenation and function call formatting issues flagged by the linter. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
github-actions
bot
changed the title
Member
Python Test Coverage Report •
Python Unit Test Overview
|
||||||||||||||||||||||||||||||
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request fixes a critical bug in the Python agent framework where exhausting max_iterations during tool invocation would return a response with orphaned function calls (FunctionCallContent without matching FunctionResultContent), corrupting thread state and causing subsequent API calls to fail with 400 errors.
Changes:
- Fixed the function invocation loop to make a final model call with
tool_choice="none"when max_iterations is exhausted, preventing orphaned function calls and ensuring complete conversation history - Re-enabled previously skipped tests for max_iterations behavior in both non-streaming and streaming modes
- Added comprehensive regression tests specifically targeting thread corruption scenarios
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
python/packages/core/agent_framework/_tools.py |
Removed premature return when max_iterations exhausted; both non-streaming (lines 2195-2203) and streaming (lines 2334-2342) paths now fall through to failsafe logic that calls model with tool_choice="none" and preserves all messages |
python/packages/core/tests/core/test_function_invocation_logic.py |
Removed duplicate @pytest.mark.skip decorators from max_iterations tests, re-enabling validation of the fixed behavior |
python/packages/core/tests/core/test_issue_1366_thread_corruption.py |
Added new comprehensive test file with 4 tests covering orphaned function call detection, failsafe invocation, message preservation, and full agent thread integrity |
python/packages/core/tests/core/test_issue_1366_thread_corruption.py
Outdated
Show resolved
Hide resolved
python/packages/core/tests/core/test_issue_1366_thread_corruption.py
Outdated
Show resolved
Hide resolved
alliscode
commented
Feb 25, 2026
TaoChenOSU
approved these changes
Feb 25, 2026
…ock behavior in test - Add explicit function_invocation_configuration['enabled'] check to the 'Maximum iterations reached' log condition in both non-streaming and streaming paths, making intent clearer when function invocation is disabled. - Add comment in test_thread_safe_after_max_iterations_with_agent explaining that the failsafe response (tool_choice='none') is provided automatically by the mock client, not from run_responses.
alliscode
changed the title
moonbox3
reviewed
Feb 25, 2026
python/packages/core/tests/core/test_issue_1366_thread_corruption.py
Outdated
Show resolved
Hide resolved
- Remove issue microsoft#1366 references from _tools.py comments - Move regression tests from standalone test_issue_1366_thread_corruption.py into test_function_invocation_logic.py alongside existing max_iterations tests - Clean up test docstrings to describe behavior generically - Delete the standalone issue-specific test file
Member
Author
|
Closes #1366 |
moonbox3
approved these changes
Feb 25, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request fixes a critical bug in the agent framework where exhausting
max_iterationsduring tool invocation could corrupt the thread state by skipping a final model call and losing conversation history. The fix ensures that, when the iteration limit is reached, a final model call is made withtool_choice="none"and all accumulated messages are preserved, restoring thread integrity. The changes are minimal, well-tested, and follow existing code patterns.Bug fix in agent tool invocation loop:
Updated
python/packages/core/agent_framework/_tools.pyto remove the prematurereturn responsewhenmax_iterationsis reached in both non-streaming and streaming paths. Instead, the code now logs the event and falls through to the existing failsafe logic, making a final model call withtool_choice="none"and prepending all accumulated function call/result messages (fcc_messages). This ensures a clean text response and complete conversation history. [1] [2]Refactored related conditions and log messages for consistency and clarity, matching the style of the
max_function_callslimit handling. [1] [2]Test coverage improvements:
Removed unnecessary
@pytest.mark.skipdecorators fromtest_max_iterations_limitandtest_streaming_max_iterations_limitintest_function_invocation_logic.py, re-enabling these tests to validate the fixed behavior. [1] [2]Added a new targeted test file (
test_issue_1366_thread_corruption.py) to cover orphaned function call detection, failsafe invocation, message preservation, and full agent run thread integrity.Documentation and quality review:
Contribution Checklist