fix: add missing ExecutorContext methods to AgentExecutor for human_input support#6352
fix: add missing ExecutorContext methods to AgentExecutor for human_input support#6352C1-BA-B1-F3 wants to merge 2 commits into
Conversation
…nput support AgentExecutor was missing three methods required by the ExecutorContext protocol: _invoke_loop, _ainvoke_loop, and _format_feedback_message. This caused Task(human_input=True) to crash with AttributeError after the default executor was swapped from CrewAgentExecutor to AgentExecutor in 1.15.0. Fixes crewAIInc#6347 Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
![corridor-security[bot]](https://avatars.githubusercontent.com/in/1177339?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary: This PR adds missing human input feedback loop methods to AgentExecutor, including sync/async re-invocation and feedback message formatting. No exploitable security vulnerabilities were identified in the added code.
Risk: Low risk. The changes do not introduce new public endpoints, authentication or authorization logic, filesystem/network access, subprocess execution, or other new attacker-facing security boundaries.
📝 WalkthroughWalkthroughAgentExecutor now resets transient execution state through a shared helper, adds sync and async human-feedback rerun helpers with feedback message formatting, and extends tests for state cleanup and protocol coverage. ChangesHuman feedback handling
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
lib/crewai/tests/agents/test_agent_executor.py (1)
2406-2459: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRegression tests only assert method presence, not feedback-loop behavior.
These tests guard against the original
AttributeError(good), but none exercise_invoke_loop/_ainvoke_loopactually re-running the flow and returning an improvedAgentFinish. The state-reset concern raised onagent_executor.py(carried-overiterations) would not be caught here. Consider adding a test that runs the feedback loop with a stubbed LLM across more than one prior iteration to lock in correct rerun semantics.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/crewai/tests/agents/test_agent_executor.py` around lines 2406 - 2459, The regression tests currently only verify that AgentExecutor exposes _invoke_loop, _ainvoke_loop, and _format_feedback_message, but they do not validate the feedback-loop behavior itself. Add a test that exercises the actual rerun path in AgentExecutor by stubbing the LLM and invoking _invoke_loop and/or _ainvoke_loop after prior iterations, then assert the flow resets state correctly and returns the expected improved AgentFinish. Use the existing AgentExecutor helpers and the ExecutorContext feedback methods to locate the behavior under test.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/crewai/src/crewai/experimental/agent_executor.py`:
- Around line 3172-3199: The feedback rerun in `_invoke_loop` and
`_ainvoke_loop` is only resetting part of the executor state, so prior
iterations and tool state leak into the second run. Update both methods to
mirror the full reset used by `invoke()`, including `state.iterations`,
`state.current_answer`, `state.use_native_tools`, and
`state.pending_tool_calls`, along with the existing flags. Make sure the async
path in `_ainvoke_loop` applies the same state reset before `kickoff_async()` so
`check_max_iterations` and `initialize_reasoning` behave as if this is a fresh
execution.
---
Nitpick comments:
In `@lib/crewai/tests/agents/test_agent_executor.py`:
- Around line 2406-2459: The regression tests currently only verify that
AgentExecutor exposes _invoke_loop, _ainvoke_loop, and _format_feedback_message,
but they do not validate the feedback-loop behavior itself. Add a test that
exercises the actual rerun path in AgentExecutor by stubbing the LLM and
invoking _invoke_loop and/or _ainvoke_loop after prior iterations, then assert
the flow resets state correctly and returns the expected improved AgentFinish.
Use the existing AgentExecutor helpers and the ExecutorContext feedback methods
to locate the behavior under test.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 9e5debdb-775d-4e72-a2ce-2c6b2d0ce918
📒 Files selected for processing (2)
lib/crewai/src/crewai/experimental/agent_executor.pylib/crewai/tests/agents/test_agent_executor.py
|
I independently checked the rerun reset path on this branch and the state leak is reproducible on With prior feedback-loop state seeded before {
"iterations": 3,
"current_answer": "AgentFinish",
"is_finished": False,
"use_native_tools": True,
"pending_tool_calls": [{"id": "stale"}],
}The async path behaves the same before That matters because I also tried the minimal local reset before self.state.iterations = 0
self.state.current_answer = None
self.state.is_finished = False
self.state.use_native_tools = False
self.state.pending_tool_calls = []After that temporary patch, the sync and async harnesses both observed clean state at rerun start, and the focused PR tests still passed: I would add a regression test that seeds stale |
|
@samrusani Great catch! You're absolutely right — the state leak across feedback-loop retries is a real issue. The I'll add the minimal reset you suggested and include a regression test that seeds stale state and asserts clean reset on rerun. Thanks for the detailed reproduction and the patch suggestion! |
|
Implemented the state reset follow-up in What changed:
Validation run:
The full |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
lib/crewai/tests/agents/test_agent_executor.py (1)
961-1016: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd the async mirror for the state-reset regression.
_ainvoke_loop()has separate control flow and was changed alongside_invoke_loop(), but this regression only exercises the sync rerun path. A mirrored async test would prevent the async human-feedback path from drifting.🧪 Suggested async companion test
def test_invoke_loop_clears_transient_state_between_sequential_runs( self, mock_dependencies ): """Feedback reruns must not inherit pipeline state from prior runs.""" executor = _build_executor(**mock_dependencies) @@ assert executor.state.current_answer is None assert executor.state.use_native_tools is False assert executor.state.pending_tool_calls == [] + + `@pytest.mark.asyncio` + async def test_ainvoke_loop_clears_transient_state_between_sequential_runs( + self, mock_dependencies + ): + """Async feedback reruns must not inherit pipeline state from prior runs.""" + executor = _build_executor(**mock_dependencies) + start_states = [] + outputs = ["First result", "Second result"] + + async def mock_kickoff_side_effect(): + start_states.append( + ( + executor.state.iterations, + executor.state.current_answer, + executor.state.use_native_tools, + list(executor.state.pending_tool_calls), + ) + ) + executor.state.iterations = 3 + executor.state.use_native_tools = True + executor.state.pending_tool_calls.append("tool_call") + executor.state.current_answer = AgentFinish( + thought="final thinking", + output=outputs.pop(0), + text="complete", + ) + + executor.state.iterations = 9 + executor.state.current_answer = AgentAction( + thought="stale", tool="search", tool_input="query", text="action" + ) + executor.state.use_native_tools = True + executor.state.pending_tool_calls.append("stale_tool_call") + + with patch.object( + AgentExecutor, "kickoff_async", side_effect=mock_kickoff_side_effect + ): + first_result = await executor._ainvoke_loop() + assert first_result.output == "First result" + assert executor.state.iterations == 0 + assert executor.state.current_answer is None + assert executor.state.use_native_tools is False + assert executor.state.pending_tool_calls == [] + + second_result = await executor._ainvoke_loop() + + assert second_result.output == "Second result" + assert start_states == [ + (0, None, False, []), + (0, None, False, []), + ] + assert executor.state.iterations == 0 + assert executor.state.current_answer is None + assert executor.state.use_native_tools is False + assert executor.state.pending_tool_calls == []🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/crewai/tests/agents/test_agent_executor.py` around lines 961 - 1016, Add an async regression test mirroring test_invoke_loop_clears_transient_state_between_sequential_runs so _ainvoke_loop() is covered too. Use the same AgentExecutor state-reset setup and assertions, but drive the async path with the async kickoff method or equivalent patch on AgentExecutor._ainvoke_loop() to verify iterations, current_answer, use_native_tools, and pending_tool_calls are cleared between sequential feedback reruns.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@lib/crewai/tests/agents/test_agent_executor.py`:
- Around line 961-1016: Add an async regression test mirroring
test_invoke_loop_clears_transient_state_between_sequential_runs so
_ainvoke_loop() is covered too. Use the same AgentExecutor state-reset setup and
assertions, but drive the async path with the async kickoff method or equivalent
patch on AgentExecutor._ainvoke_loop() to verify iterations, current_answer,
use_native_tools, and pending_tool_calls are cleared between sequential feedback
reruns.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 2aa658ce-4206-445b-b29d-1da678adfb31
📒 Files selected for processing (2)
lib/crewai/src/crewai/experimental/agent_executor.pylib/crewai/tests/agents/test_agent_executor.py
|
Rechecked current head The runtime fix looks correct now: I also tried the async companion regression locally by mirroring the existing sync test: seed stale state, patch Focused validation with that temporary async test added: So this looks like a coverage-only follow-up rather than a remaining runtime bug, but I agree the async mirror is worth adding because |
2 participants
Summary
Fixes #6347
Task(human_input=True)crashes withAttributeError: 'AgentExecutor' object has no attribute '_format_feedback_message'after the default executor was swapped fromCrewAgentExecutortoAgentExecutorin 1.15.0.Root Cause
AgentExecutoris cast toExecutorContextin_handle_human_feedback(), but three protocol methods were never implemented:_format_feedback_message(feedback)SyncHumanInputProvider._handle_regular_feedback()_invoke_loop()SyncHumanInputProvider._handle_regular_feedback()_ainvoke_loop()AsyncExecutorContextprotocolFix
Added all three missing methods to
AgentExecutorinsrc/crewai/experimental/agent_executor.py:_invoke_loop()— resets execution state and re-runs the flow viakickoff()_ainvoke_loop()— async version usingkickoff_async()_format_feedback_message()— formats feedback usingI18N_DEFAULTTest Plan
TestAgentExecutorHumanInputProtocoltest class with 5 tests verifying:_format_feedback_message()returns a proper dict with feedback contentExecutorContextprotocol methods are present onAgentExecutorVerification
All tests pass ✅
Summary by CodeRabbit
Bug Fixes
Tests