fix(memory): stop cross-flow chat-history leak in legacy flows (LE-1929) - #14191
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughChangesThe PR adds ambient flow scoping for backend message retrieval, binds and resets flow context during graph component execution, and adds regression coverage. It also fixes React Flow edge migration references and stale generic-node dimension handling. Ambient flow-scoped message retrieval
React Flow persistence fixes
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Graph
participant ComponentExecution
participant MessageRetrieval
participant MessageStore
Graph->>ComponentExecution: execute graph component
ComponentExecution->>ComponentExecution: bind graph flow_id
ComponentExecution->>MessageRetrieval: retrieve messages without flow_id
MessageRetrieval->>MessageStore: query session and ambient flow_id
MessageStore-->>MessageRetrieval: scoped messages
ComponentExecution->>ComponentExecution: reset flow context
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 8 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (8 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 |
✅ Test Coverage AdvisorNo source changes detected without accompanying tests. Thanks for keeping coverage up! 🎉
|
github-actions
Bot
added
bug
![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
🤖 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 `@src/frontend/src/utils/reactflowUtils.ts`:
- Around line 128-130: Update the source-invalid removal paths in the edge
migration flow to use the retained edge reference `edgeInNewEdges` for filtering
and for the `filterHiddenFieldsEdges` call, rather than the original `edge`.
Ensure every later removal reflects any migrated `edgeInNewEdges.id` so broken
or hidden edges are actually removed from `newEdges`.
In `@src/lfx/src/lfx/interface/initialize/loading.py`:
- Around line 81-94: Update the flow-scope setup around build_custom_component
and build_component so set_current_flow_id is always called, passing flow_id
even when it is None. Continue resetting the returned token in the finally
block, preserving unscoped behavior when vertex.graph.flow_id is absent.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a01aa0a8-7dcb-4af4-899e-c32fd7c31ce7
📒 Files selected for processing (7)
src/backend/base/langflow/memory.pysrc/backend/tests/unit/test_message_flow_context_scoping.pysrc/frontend/src/utils/__tests__/reconcileNodeDimensions.test.tssrc/frontend/src/utils/__tests__/updatePathEdgeMigration.test.tssrc/frontend/src/utils/reactflowUtils.tssrc/lfx/src/lfx/interface/initialize/loading.pysrc/lfx/src/lfx/memory/flow_context.py
| // Hold by reference: the target block rewrites edge.id on migration, so a second find-by-id in | ||
| // the source block would miss it and skip the source-handle migration (DataFrame stays). LE-1929. | ||
| const edgeInNewEdges = newEdges.find((e) => e.id === edge.id); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Use the retained edge reference for every later removal.
After a handle migration rewrites edgeInNewEdges.id, the source-invalid paths still filter by the original edge.id (Lines 282 and 306), and filterHiddenFieldsEdges receives that stale edge (Line 312). Those paths then report a broken/hidden edge but leave it in newEdges.
Proposed fix
- newEdges = newEdges.filter((e) => e.id !== edge.id);
+ newEdges = newEdges.filter((e) => e !== edgeInNewEdges);
- newEdges = filterHiddenFieldsEdges(edge, newEdges, targetNode);
+ newEdges = filterHiddenFieldsEdges(
+ edgeInNewEdges ?? edge,
+ newEdges,
+ targetNode,
+ );Also applies to: 205-205
🤖 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 `@src/frontend/src/utils/reactflowUtils.ts` around lines 128 - 130, Update the
source-invalid removal paths in the edge migration flow to use the retained edge
reference `edgeInNewEdges` for filtering and for the `filterHiddenFieldsEdges`
call, rather than the original `edge`. Ensure every later removal reflects any
migrated `edgeInNewEdges.id` so broken or hidden edges are actually removed from
`newEdges`.
| flow_id = getattr(getattr(vertex, "graph", None), "flow_id", None) | ||
| flow_scope_token = set_current_flow_id(flow_id) if flow_id is not None else None | ||
| try: | ||
| with warnings.catch_warnings(): | ||
| warnings.filterwarnings("ignore", category=PydanticDeprecatedSince20) | ||
| if base_type == "custom_components": | ||
| return await build_custom_component(params=custom_params, custom_component=custom_component) | ||
| if base_type == "component": | ||
| return await build_component(params=custom_params, custom_component=custom_component) | ||
| msg = f"Base type {base_type} not found." | ||
| raise ValueError(msg) | ||
| finally: | ||
| if flow_scope_token is not None: | ||
| reset_current_flow_id(flow_scope_token) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Always shadow the ambient context, including with None.
Line 82 leaves a previously bound flow scope active when this vertex has no graph.flow_id. Nested execution can then retrieve messages under the outer flow’s ID instead of preserving the intended unscoped behavior.
Proposed fix
flow_id = getattr(getattr(vertex, "graph", None), "flow_id", None)
-flow_scope_token = set_current_flow_id(flow_id) if flow_id is not None else None
+flow_scope_token = set_current_flow_id(flow_id)
try:
...
finally:
- if flow_scope_token is not None:
- reset_current_flow_id(flow_scope_token)
+ reset_current_flow_id(flow_scope_token)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| flow_id = getattr(getattr(vertex, "graph", None), "flow_id", None) | |
| flow_scope_token = set_current_flow_id(flow_id) if flow_id is not None else None | |
| try: | |
| with warnings.catch_warnings(): | |
| warnings.filterwarnings("ignore", category=PydanticDeprecatedSince20) | |
| if base_type == "custom_components": | |
| return await build_custom_component(params=custom_params, custom_component=custom_component) | |
| if base_type == "component": | |
| return await build_component(params=custom_params, custom_component=custom_component) | |
| msg = f"Base type {base_type} not found." | |
| raise ValueError(msg) | |
| finally: | |
| if flow_scope_token is not None: | |
| reset_current_flow_id(flow_scope_token) | |
| flow_id = getattr(getattr(vertex, "graph", None), "flow_id", None) | |
| flow_scope_token = set_current_flow_id(flow_id) | |
| try: | |
| with warnings.catch_warnings(): | |
| warnings.filterwarnings("ignore", category=PydanticDeprecatedSince20) | |
| if base_type == "custom_components": | |
| return await build_custom_component(params=custom_params, custom_component=custom_component) | |
| if base_type == "component": | |
| return await build_component(params=custom_params, custom_component=custom_component) | |
| msg = f"Base type {base_type} not found." | |
| raise ValueError(msg) | |
| finally: | |
| reset_current_flow_id(flow_scope_token) |
🤖 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 `@src/lfx/src/lfx/interface/initialize/loading.py` around lines 81 - 94, Update
the flow-scope setup around build_custom_component and build_component so
set_current_flow_id is always called, passing flow_id even when it is None.
Continue resetting the returned token in the finally block, preserving unscoped
behavior when vertex.graph.flow_id is absent.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## release-1.11.0 #14191 +/- ##
==================================================
+ Coverage 60.84% 61.30% +0.45%
==================================================
Files 2441 2442 +1
Lines 238902 238980 +78
Branches 36073 36123 +50
==================================================
+ Hits 145368 146497 +1129
+ Misses 91778 90727 -1051
Partials 1756 1756
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
erichare
left a comment
erichare
left a comment
There was a problem hiding this comment.
Couple things @Cristhianzl
-
[P1] Ambient flow context can bleed into nested execution.
[get_instance_results](https://github.com/langflow-ai/langflow/blob/4ac2566059c55b051e30fe78c104ed254966bf79/src/lfx/src/lfx/interface/initialize/loading.py#L81-L94)skips setting the ContextVar when the inner vertex has noflow_id, leaving any outer flow scope active. Always set it—includingNone—and reset the token unconditionally. [Existing thread](#14191 (comment)). -
[P2] Migrated broken edges can survive cleanup. After target migration rewrites the retained edge ID, the source-invalid and hidden-field paths still filter using the original ID at
[reactflowUtils.ts](https://github.com/langflow-ai/langflow/blob/4ac2566059c55b051e30fe78c104ed254966bf79/src/frontend/src/utils/reactflowUtils.ts#L281-L312). My focused reproduction returnedbrokenEdges=1but incorrectly retainededges=1. Use the retained reference for every subsequent removal. [Existing thread](#14191 (comment)).
Blocking CI: Biome reports seven noExplicitAny errors in the new frontend tests. The Docker job was canceled after a runner shutdown, not a demonstrated Docker regression; other checks remain pending.
Validation: PR backend tests 6 passed; frontend tests 3 passed; git diff --check and synthetic merge-tree clean.
Frontend Unit Test Coverage ReportCoverage Summary
Unit Test Results
|
github-actions
Bot
added
bug
A graph without flow_id must run legacy-unscoped, not inherit an outer flow's ambient scope. Bind the ContextVar unconditionally (including None) and reset it unconditionally. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013CgXpMiyLTFSEueEzcrync
After the target migration rewrites the retained edge's id, the source-invalid and hidden-field removal paths still filtered by the original id and silently kept broken edges. Filter by the retained reference and pass it to filterHiddenFieldsEdges. Also type the new test fixtures (biome noExplicitAny). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013CgXpMiyLTFSEueEzcrync
2 participants
Summary
Fixes a cross-flow chat-history leak (issue #13059) that still affected legacy saved flows, plus two flow-editor rendering bugs surfaced while validating the same legacy flow.
1. Memory — cross-flow chat-history leak in legacy flows (primary)
MemoryComponent.retrieve_messageswas scoped byflow_idin #13087, but Langflow executes the frozen componentcodeembedded in each saved flow (lfx.interface.initialize.loading.instantiate_class→eval_custom_component_code), not the installed library. A flow saved before #13087 keeps the old unscopedretrieve_messagesand leaks another flow's history on a collidingsession_id— on any server version.Fix (defense-in-depth; frozen code untouched): an ambient
flow_idContextVar (lfx/memory/flow_context.py) is bound around component execution inget_instance_results;aget_messagesdefaults its scope from it only whenflow_id is None. Explicit callers and non-graph callers are unchanged — this extends the #13087 contract to the platform function the frozen code calls, instead of the (unchangeable) frozen code.Verified live on the reporter's actual legacy flow across
POST /api/v1/run/{flow}andPOST /api/v1/build/{flow}/flow(Playground): leaked before, no leak after.2. Frontend —
cleanEdgesskips the source-handle migration when both handles migrateWhen an edge needs both handles migrated (e.g.
DataFrame→Table, #11554), the target block rewroteedge.id, then the source block'sfind-by-original-id failed and silently skipped the source rewrite — leaving a kept-but-detached edge. Fixed by capturing the edge reference once and reusing it in both blocks.3. Frontend — legacy nodes render with detached edges (stale width)
Flows saved when the node card was
w-96(384px) keptwidth: 384; the card now rendersw-80(320px), so React Flow placed the right-side output handle ~64px past the visual dot (edges rendered detached, unfixable by dragging).processFlowNodesnow drops persistedwidth/height/measuredongenericNodes so React Flow re-measures the real size;noteNodes (user-resizable) are preserved.Testing
test_message_flow_context_scoping.py(incl. a graph-level e2e reproducing an old-style unscoped component); 115 existing memory tests green;ruff format/checkclean.updatePathEdgeMigration.test.ts,reconcileNodeDimensions.test.ts; 40 existing edge/type-compatibility tests green; biome clean.Backward compatibility
No component class or
name/input/output renames. Changes are additive and default to legacy behavior when the new context is absent, so existing flows and API consumers are unaffected.Summary by CodeRabbit
Bug Fixes
DataFrametoTable.Tests