fix: handle multiple EndTurn tools with Union type for allow_fail#1231
Open
fix: handle multiple EndTurn tools with Union type for allow_fail#1231
Conversation
fixes infinite loop and "multiple endturn tools" warning when using allow_fail=True **root cause** - when a task has allow_fail=True, it creates two endturn tools: MarkTaskSuccessful and MarkTaskFailed - previous code (line 186) set output_type=type(None) when >1 endturn tool detected - this caused the agent to not validate its output as an EndTurn, leading to infinite loop **solution** - use Union[Tool1, Tool2] as output_type when multiple endturn tools exist - this allows pydantic_ai to properly validate the agent's response as one of the union types - the model can now choose between marking the task successful or failed **testing** - tested with allow_fail=True - tasks can now properly succeed OR fail - no more infinite loops - no more "multiple endturn tools" warnings - 58/59 existing tests pass (1 failure due to API connection issue, not our change) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
- test that tasks with allow_fail complete without infinite loop - test that allow_fail creates both MarkTaskSuccessful and MarkTaskFailed tools - test that multiple EndTurn tools result in Union output type 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR fixes an infinite loop issue that occurred when using tasks with allow_fail=True, which creates multiple EndTurn tools. The solution replaces the previous workaround of setting output_type=type(None) with a proper Union type that allows the model to choose between success and failure tools.
- Replaced
type(None)withUnion[Tool1, Tool2]for multiple EndTurn tools to enable proper validation - Removed the warning about multiple EndTurn tools since this is now properly handled
- Added comprehensive test coverage for the allow_fail functionality
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/marvin/agents/agent.py | Updated logic to use Union type for multiple EndTurn tools instead of type(None) |
| tests/basic/tasks/test_tasks.py | Added new test class with 3 tests covering allow_fail functionality and regression testing |
Comments suppressed due to low confidence (1)
tests/basic/tasks/test_tasks.py:1
- The test accesses private attribute
_output_typeand doesn't verify the Union contains the expected tool types. Consider using public APIs if available, and assert that the Union contains the specific MarkTaskSuccessful and MarkTaskFailed tool types.
import enum
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
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
1 participant
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.
summary
fixes infinite loop and "multiple endturn tools" warning when using
allow_fail=Trueroot cause
allow_fail=True, it creates two endturn tools:MarkTaskSuccessfulandMarkTaskFailedoutput_type=type(None)when >1 endturn tool detectedsolution
Union[Tool1, Tool2]as output_type when multiple endturn tools existtesting
tests/basic/tasks/test_tasks.py::TestAllowFail:related
🤖 Generated with Claude Code