fix(flow): allow Flow subclasses in crew.copy() for kickoff_for_each (fixes #4555)#4668
fix(flow): allow Flow subclasses in crew.copy() for kickoff_for_each (fixes #4555)#4668giulio-leone wants to merge 2 commits intocrewAIInc:mainfrom
Conversation
The parent_flow field (present in FlowTrackable prior to 1.10.0) was typed as InstanceOf[Flow[Any]], which created a _FlowGeneric class via __class_getitem__. Pydantic isinstance checks rejected concrete Flow subclasses during crew.copy(), breaking kickoff_for_each inside Flow methods. Add "parent_flow" to the exclude set in Crew.copy() so the field is never round-tripped through model_dump/Crew constructor. This fixes the issue for users on older versions and provides forward compat. Fixes crewAIInc#4555
There was a problem hiding this comment.
Pull request overview
This PR addresses issue #4555 where kickoff_for_each inside Flow execution could fail due to Crew.copy() round-tripping a legacy/compat parent_flow field through Pydantic serialization and re-construction.
Changes:
- Excludes
"parent_flow"fromCrew.copy()serialization to prevent it from being passed into theCrew(...)constructor. - Adds a new regression test module covering
copy()/kickoff_for_eachbehavior around simulatedparent_flowpresence and flow-context execution.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
lib/crewai/src/crewai/crew.py |
Updates Crew.copy() exclude set to omit parent_flow during model_dump() cloning. |
lib/crewai/tests/test_flow_kickoff_for_each.py |
Adds regression tests intended to prevent reintroduction of the kickoff_for_each+Flow copy() failure mode. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if self_or_kw is not None: | ||
| kwargs.update(self_or_kw) | ||
| data = original_model_dump(**kwargs) | ||
| # Pretend parent_flow leaked through model_dump (old versions) |
There was a problem hiding this comment.
The patched model_dump injects "parent_flow" into the returned dict unconditionally, even when Crew.copy passes exclude={..."parent_flow"}. That means this test isn't actually verifying the intended behavior (that parent_flow is excluded from the serialized data / constructor kwargs), and it may pass even if the exclude set change is removed. Consider asserting the kwargs passed to Crew(...) (e.g., via spying on Crew.init) or making the patch respect the exclude parameter so the test fails without the fix and passes with it.
| # Pretend parent_flow leaked through model_dump (old versions) | |
| # Pretend parent_flow leaked through model_dump (old versions), | |
| # but respect the exclude set so the test actually verifies that | |
| # Crew.copy() is excluding "parent_flow". | |
| exclude = kwargs.get("exclude") | |
| if isinstance(exclude, (set, frozenset)) and "parent_flow" in exclude: | |
| return data |
There was a problem hiding this comment.
Fixed: updated the patched model_dump to accept self (since patch.object replaces the class method) and made it conditionally inject parent_flow only when the caller didn't explicitly exclude it. This properly simulates how a real leak would surface.
| import pytest | ||
| from pydantic import BaseModel | ||
|
|
||
| from crewai.agent import Agent | ||
| from crewai.crew import Crew | ||
| from crewai.flow.flow import Flow, listen, start | ||
| from crewai.task import Task |
There was a problem hiding this comment.
Unused imports in this new test module (pytest, BaseModel, and Flow/listen/start) increase noise and can break linting in environments that enforce unused-import checks. Please remove the unused imports or use them in the tests.
There was a problem hiding this comment.
Fixed: removed unused imports (pytest, BaseModel, Flow, listen, start). Kept from __future__ import annotations as it's standard practice.
- Remove unused imports (pytest, BaseModel, Flow, listen, start)
- Fix patched model_dump to accept self and conditionally inject parent_flow
- Add copied_data.pop('parent_flow', None) as safety net in Crew.copy()
Refs: crewAIInc#4555
Summary
Fixes #4555 — Flows do not work with
kickoff_for_each.Root Cause
FlowTrackable(prior to 1.10.0) had aparent_flow: InstanceOf[Flow[Any]]Pydantic field.Flow[Any]creates a dynamic_FlowGenericsubclass via__class_getitem__, so Pydantic'sisinstancecheck rejected concrete Flow subclasses (e.g.ResearchFlow) duringcrew.copy(), breakingkickoff_for_eachinside Flow methods.Changes
"parent_flow"to the exclude set inCrew.copy()so the field is never round-tripped throughmodel_dump()/Crew()constructorcopy()excludesparent_flowfrom serialized datacopy()preserves agents and tasks as independent copieskickoff_for_eachcreates independent copies per inputcopy()works whenparent_flowis set as a regular attributecopy()works within a Flow execution contextTests
All 5 new tests pass. Existing
copyandkickoff_for_eachtests unaffected.Fixes #4555