fix(crew): await async before/after_kickoff_callbacks in akickoff#6500
Open
nolanchic wants to merge 1 commit into
Open
fix(crew): await async before/after_kickoff_callbacks in akickoff#6500nolanchic wants to merge 1 commit into
nolanchic wants to merge 1 commit into
Conversation
akickoff invoked before/after_kickoff_callbacks without an awaitable check, so async callables were silently dropped: an async after-callback replaced the crew result with an unawaited coroutine, and async before-callbacks never ran because prepare_kickoff is synchronous. - Add aprepare_kickoff (async counterpart of prepare_kickoff) that awaits callbacks returning an awaitable, matching task/step_callback handling. Shared body extracted into _prepare_kickoff_impl to avoid duplication. - In akickoff, await the result of each after_kickoff_callback when it is awaitable. Sync kickoff and kickoff_async (to_thread wrapper) are unchanged. Fixes crewAIInc#6481
|
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 (3)
📝 WalkthroughWalkthroughThis PR adds async-aware handling for ChangesAsync kickoff callback support
Sequence Diagram(s)sequenceDiagram
participant Caller
participant Crew as Crew.akickoff()
participant Utils as aprepare_kickoff
participant BeforeCB as before_kickoff_callbacks
participant AfterCB as after_kickoff_callbacks
Caller->>Crew: akickoff(inputs)
Crew->>Utils: await aprepare_kickoff(inputs)
Utils->>BeforeCB: await callback(inputs)
BeforeCB-->>Utils: normalized inputs
Utils-->>Crew: prepared state
Crew->>Crew: execute tasks
Crew->>AfterCB: after_callback(result)
AfterCB-->>Crew: result (possibly coroutine)
Crew->>Crew: if isawaitable(result): await result
Crew-->>Caller: CrewOutput
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 |
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
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.
Description
Crew.akickoff()is a native async path, butbefore_kickoff_callbacksandafter_kickoff_callbackswere invoked without any awaitable check — silently dropping async callables:after_kickoff_callbacks:result = after_callback(result)returned an unawaited coroutine when the callback wasasync, so the crew result was silently replaced with a coroutine object (never executed, never awaited).before_kickoff_callbacks: invoked inside the synchronousprepare_kickoff(), which has no async support — async before-callbacks never ran, and any blocking I/O inside a sync before-callback stalled the event loop.This is inconsistent with
task_callbackandstep_callback, which correctly checkinspect.isawaitablein the async path.Changes
crews/utils.py: addasync def aprepare_kickoff(...)— the async counterpart ofprepare_kickoff. It awaits before-callbacks that return an awaitable (sync callbacks run unchanged). The shared body is extracted into_prepare_kickoff_impl(...)so the two entry points stay DRY and behavior is identical apart from callback awaiting.crew.py:akickoffnow callsawait aprepare_kickoff(...)instead ofprepare_kickoff(...), and awaits each after-callback result when it is awaitable.Behavior preserved
kickoff()(sync) — unchanged, still usesprepare_kickoff.kickoff_async()— unchanged; it wraps the synckickoffinasyncio.to_thread, so all callbacks already run off the event loop.akickoff— unchanged (anisawaitablecheck is a no-op for them).Test plan
test_akickoff_awaits_async_after_callbacks— async after-callback is awaited; result stays aCrewOutput(was a coroutine before).test_akickoff_applies_async_after_callback_result— async after-callback's returnedCrewOutputreplaces the result.test_akickoff_awaits_async_before_callbacks— async before-callback is awaited and runs.test_akickoff_applies_async_before_callback_inputs— async before-callback's returned inputs flow into the crew.uv run pytest lib/crewai/tests/crew/test_async_crew.py lib/crewai/tests/test_crew.py lib/crewai/tests/test_checkpoint.py→ all pass (sync kickoff + kickoff_async callbacks unaffected).uv run ruff check lib/→ clean.uv run mypy lib/crewai/src/crewai/crew.py lib/crewai/src/crewai/crews/utils.py→ clean (strict).Fixes #6481.