fix(agent): implement ExecutorContext protocol on AgentExecutor for human_input support (#6347)#6372
Open
ThryLox wants to merge 3 commits into
Open
Conversation
…uman_input support (crewAIInc#6347) In crewAI 1.15.0, AgentExecutor became the default executor class. However, when Task(human_input=True) or ask_for_human_input=True is enabled, human feedback handlers cast AgentExecutor to ExecutorContext and invoke _format_feedback_message, _invoke_loop, and _ainvoke_loop. Because AgentExecutor lacked these methods, execution crashed with AttributeError. This commit implements all required ExecutorContext and AsyncExecutorContext protocol methods on AgentExecutor and adds a regression contract test.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds ChangesHuman Input Protocol Implementation
Suggested Reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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: 2
🧹 Nitpick comments (1)
lib/crewai/tests/agents/test_agent_executor.py (1)
2399-2405: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThis regression test is too weak to protect the restored HITL flow.
hasattronly catches another missing-method regression. It will still pass if_invoke_loop()immediately re-hitsmax_iteror if planning-mode reruns ignore the appended feedback, so it does not cover the behavior this PR is restoring.🤖 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 2399 - 2405, The current regression test only checks that AgentExecutor still exposes helper methods, so it won’t catch broken HITL behavior in the restored flow. Strengthen test_agent_executor_implements_human_input_protocol in the AgentExecutor test suite to assert the actual human-in-the-loop path: verify _invoke_loop and/or _ainvoke_loop consume appended feedback, continue execution beyond max_iter when feedback is provided, and preserve planning-mode rerun behavior. Use the existing symbols _format_feedback_message, _invoke_loop, _ainvoke_loop, and _is_training_mode to drive a behavior-based assertion rather than simple hasattr checks.
🤖 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 3199-3210: Planning-mode reruns are still losing the human
feedback because the `generate_plan()` / `StepExecutor.execute()` path only uses
task text and dependency results, not the appended feedback message from
`_format_feedback_message()`. Update the planning execution flow in
`AgentExecutor` so the rerun preserves and forwards the latest feedback from
`self.state.messages` (or the equivalent stored context) into plan generation
and step execution, ensuring planned agents see the same feedback that
non-planning runs receive.
- Around line 3212-3220: The _invoke_loop flow is re-entering with a stale
iteration counter, so `check_max_iterations()` can immediately force another
final answer before new feedback is used. Reset `state.iterations` to the
initial value before calling `kickoff()` in `_invoke_loop` (and the related
wrapper path in the same area) so the agent can make a fresh LLM call. Keep the
existing finish validation after `kickoff()` and preserve the `AgentFinish`
check.
---
Nitpick comments:
In `@lib/crewai/tests/agents/test_agent_executor.py`:
- Around line 2399-2405: The current regression test only checks that
AgentExecutor still exposes helper methods, so it won’t catch broken HITL
behavior in the restored flow. Strengthen
test_agent_executor_implements_human_input_protocol in the AgentExecutor test
suite to assert the actual human-in-the-loop path: verify _invoke_loop and/or
_ainvoke_loop consume appended feedback, continue execution beyond max_iter when
feedback is provided, and preserve planning-mode rerun behavior. Use the
existing symbols _format_feedback_message, _invoke_loop, _ainvoke_loop, and
_is_training_mode to drive a behavior-based assertion rather than simple hasattr
checks.
🪄 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: ab3fdd0f-a3c2-47b6-b12c-deca91e285ee
📒 Files selected for processing (2)
lib/crewai/src/crewai/experimental/agent_executor.pylib/crewai/tests/agents/test_agent_executor.py
Comment on lines
+3199
to
+3210
| def _format_feedback_message(self, feedback: str) -> LLMMessage: | ||
| """Format feedback as a message for the LLM. | ||
|
|
||
| Args: | ||
| feedback: User feedback string. | ||
|
|
||
| Returns: | ||
| Formatted message dict. | ||
| """ | ||
| return format_message_for_llm( | ||
| I18N_DEFAULT.slice("feedback_instructions").format(feedback=feedback) | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Planning-mode reruns still drop the human feedback.
The provider appends _format_feedback_message(feedback) to context.messages, but a planning-enabled rerun goes through generate_plan() and StepExecutor.execute() using task text/dependency results only. Nothing on that path reads self.state.messages, so planned agents can satisfy the protocol surface while ignoring the feedback entirely.
Also applies to: 3212-3230
🤖 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/src/crewai/experimental/agent_executor.py` around lines 3199 -
3210, Planning-mode reruns are still losing the human feedback because the
`generate_plan()` / `StepExecutor.execute()` path only uses task text and
dependency results, not the appended feedback message from
`_format_feedback_message()`. Update the planning execution flow in
`AgentExecutor` so the rerun preserves and forwards the latest feedback from
`self.state.messages` (or the equivalent stored context) into plan generation
and step execution, ensuring planned agents see the same feedback that
non-planning runs receive.
…and strengthen contract tests
There was a problem hiding this comment.
🧹 Nitpick comments (2)
lib/crewai/tests/agents/test_agent_executor.py (2)
2417-2421: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert the reset happens before
kickoff().This only proves
iterations == 0after_invoke_loop()returns. Becausecurrent_answeris pre-seeded andkickoff()is a no-op, an implementation that resets afterkickoff()would still pass. Captureexecutor.state.iterationsinside akickoffside effect and setcurrent_answerthere so the test validates the intended ordering.🤖 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 2417 - 2421, The _invoke_loop() test currently only checks executor.state.iterations after the method finishes, so it can miss the reset ordering relative to kickoff(). Update the test around AgentExecutor._invoke_loop by using a kickoff side effect on the patched kickoff method to capture executor.state.iterations at call time and assign current_answer there, so the assertion verifies iterations is reset before kickoff() runs rather than afterward.
2399-2405: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a behavioral test for
_ainvoke_loop, not justhasattr.The async half of the contract is still effectively untested here. Since the PR restores
AsyncExecutorContext, mirror the reset-and-rerun assertion for_ainvoke_loop()so an async regression does not slip past on a simple method-presence check.🤖 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 2399 - 2405, The current test only checks that AgentExecutor exposes _ainvoke_loop, but it does not verify the async behavior. Update test_agent_executor_implements_human_input_protocol to mirror the existing reset-and-rerun pattern for _invoke_loop by actually exercising AgentExecutor._ainvoke_loop() and asserting the expected rerun/reset behavior, so the async contract is covered rather than just method presence.
🤖 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 2417-2421: The _invoke_loop() test currently only checks
executor.state.iterations after the method finishes, so it can miss the reset
ordering relative to kickoff(). Update the test around
AgentExecutor._invoke_loop by using a kickoff side effect on the patched kickoff
method to capture executor.state.iterations at call time and assign
current_answer there, so the assertion verifies iterations is reset before
kickoff() runs rather than afterward.
- Around line 2399-2405: The current test only checks that AgentExecutor exposes
_ainvoke_loop, but it does not verify the async behavior. Update
test_agent_executor_implements_human_input_protocol to mirror the existing
reset-and-rerun pattern for _invoke_loop by actually exercising
AgentExecutor._ainvoke_loop() and asserting the expected rerun/reset behavior,
so the async contract is covered rather than just method presence.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 500f6c10-1efc-4bb4-bb7c-522627fb158f
📒 Files selected for processing (2)
lib/crewai/src/crewai/experimental/agent_executor.pylib/crewai/tests/agents/test_agent_executor.py
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/crewai/src/crewai/experimental/agent_executor.py
…ync _ainvoke_loop contract test
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 #6347 by implementing the required
ExecutorContextandAsyncExecutorContextprotocol methods onAgentExecutor.Root Cause
In crewAI 1.15.0,
AgentExecutorbecame the default agent executor. However, whenTask(human_input=True)orask_for_human_input=Trueis enabled, the human input provider (SyncHumanInputProvider/AsyncHumanInputProvider) casts the executor toExecutorContext/AsyncExecutorContextand calls_format_feedback_message,_invoke_loop, and_ainvoke_loop. BecauseAgentExecutorlacked implementations for these methods, execution crashed with anAttributeError.Fix
Implemented
_format_feedback_message,_invoke_loop, and_ainvoke_looponAgentExecutorinlib/crewai/src/crewai/experimental/agent_executor.pyand added a protocol contract regression test intest_agent_executor.py.Summary by CodeRabbit