Skip to content

Commit 4ac2566

Browse files
committed
fix(memory): scope legacy flows by ambient flow_id
1 parent f685d29 commit 4ac2566

7 files changed

Lines changed: 418 additions & 11 deletions

File tree

src/backend/base/langflow/memory.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,11 @@ async def aget_messages(
138138
Returns:
139139
List[Data]: A list of Data objects representing the retrieved messages.
140140
"""
141+
if flow_id is None:
142+
# Default to the executing graph's flow_id so old saved flows (frozen code calls this without one) cannot surface another flow's history on a colliding session_id (issue #13059). # noqa: E501
143+
from lfx.memory.flow_context import coerce_flow_id, get_current_flow_id
144+
145+
flow_id = coerce_flow_id(get_current_flow_id())
141146
async with session_scope() as session:
142147
stmt = _get_variable_query(
143148
sender, sender_name, session_id, context_id, order_by, order, flow_id, limit, user_id=user_id
Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,157 @@
1+
"""Regression tests: ambient flow-scope defaults ``aget_messages`` by ``flow_id`` (issue #13059).
2+
3+
Langflow executes the *frozen* component ``code`` embedded in each saved flow, not the installed
4+
library version (``lfx.interface.initialize.loading.instantiate_class`` -> ``eval_custom_component_code``).
5+
A flow saved before PR #13087 therefore carries the old ``retrieve_messages`` that calls
6+
``aget_messages`` WITHOUT ``flow_id`` and leaks chat history across flows on a colliding
7+
``session_id`` — even when running on a patched server.
8+
9+
Defense-in-depth: the engine binds the executing graph's ``flow_id`` to a ContextVar so
10+
``aget_messages`` can default the scope. This closes the leak for old frozen flows without:
11+
- touching frozen code (impossible),
12+
- changing callers that pass ``flow_id`` explicitly (the default only applies when it is ``None``),
13+
- changing callers outside a graph run (the ContextVar is unset -> identical to today).
14+
"""
15+
16+
from typing import cast
17+
from uuid import uuid4
18+
19+
from langflow.memory import aadd_messages, aget_messages
20+
from langflow.schema.message import Message
21+
from lfx.components.input_output import ChatOutput
22+
from lfx.components.models_and_agents.memory import MemoryComponent
23+
from lfx.graph.graph.base import Graph
24+
from lfx.memory.flow_context import reset_current_flow_id, set_current_flow_id
25+
from lfx.schema.data import Data
26+
27+
28+
async def _store(session_id: str, flow_id, text: str) -> None:
29+
msg = Message(text=text, sender="User", sender_name="User", session_id=session_id)
30+
await aadd_messages([msg], flow_id=flow_id)
31+
32+
33+
async def test_aget_messages_defaults_flow_id_from_context(client): # noqa: ARG001
34+
"""The reproduction: two flows share a session_id; the ambient scope isolates them.
35+
36+
Simulates old frozen code calling ``aget_messages(session_id=...)`` with no flow_id while
37+
Flow B's graph is executing. The ambient flow scope must prevent Flow A's row from leaking.
38+
"""
39+
flow_a, flow_b = uuid4(), uuid4()
40+
session_id = "shared-session-13059"
41+
await _store(session_id, flow_a, "secret from A")
42+
await _store(session_id, flow_b, "hello from B")
43+
44+
token = set_current_flow_id(flow_b)
45+
try:
46+
scoped = await aget_messages(session_id=session_id)
47+
finally:
48+
reset_current_flow_id(token)
49+
50+
assert [m.text for m in scoped] == ["hello from B"]
51+
52+
53+
async def test_explicit_flow_id_overrides_context(client): # noqa: ARG001
54+
"""An explicitly-passed flow_id must win over the ambient context (no silent override)."""
55+
flow_a, flow_b = uuid4(), uuid4()
56+
session_id = "shared-session-13059-explicit"
57+
await _store(session_id, flow_a, "secret from A")
58+
await _store(session_id, flow_b, "hello from B")
59+
60+
token = set_current_flow_id(flow_b)
61+
try:
62+
a_msgs = await aget_messages(session_id=session_id, flow_id=flow_a)
63+
finally:
64+
reset_current_flow_id(token)
65+
66+
assert [m.text for m in a_msgs] == ["secret from A"]
67+
68+
69+
async def test_no_context_preserves_legacy_unscoped(client): # noqa: ARG001
70+
"""With no ambient scope and no explicit flow_id, behavior is unchanged (both rows)."""
71+
flow_a, flow_b = uuid4(), uuid4()
72+
session_id = "shared-session-13059-legacy"
73+
await _store(session_id, flow_a, "secret from A")
74+
await _store(session_id, flow_b, "hello from B")
75+
76+
unscoped = await aget_messages(session_id=session_id)
77+
78+
assert len(unscoped) == 2
79+
80+
81+
async def test_context_accepts_string_flow_id(client): # noqa: ARG001
82+
"""``graph.flow_id`` is commonly a ``str``; the default path must coerce it, not crash.
83+
84+
``MessageTable.flow_id`` is UUID-typed; on SQLite comparing it to a raw string makes the
85+
``Uuid`` bind processor call ``value.hex`` and raise. The default must coerce str -> UUID.
86+
"""
87+
flow_a, flow_b = uuid4(), uuid4()
88+
session_id = "shared-session-13059-str"
89+
await _store(session_id, flow_a, "secret from A")
90+
await _store(session_id, flow_b, "hello from B")
91+
92+
token = set_current_flow_id(str(flow_b))
93+
try:
94+
scoped = await aget_messages(session_id=session_id)
95+
finally:
96+
reset_current_flow_id(token)
97+
98+
assert [m.text for m in scoped] == ["hello from B"]
99+
100+
101+
async def test_invalid_context_flow_id_degrades_to_unscoped(client): # noqa: ARG001
102+
"""A non-UUID ambient flow_id (synthetic/test graphs) must degrade, never crash retrieval."""
103+
flow_a, flow_b = uuid4(), uuid4()
104+
session_id = "shared-session-13059-badctx"
105+
await _store(session_id, flow_a, "secret from A")
106+
await _store(session_id, flow_b, "hello from B")
107+
108+
token = set_current_flow_id("not-a-uuid")
109+
try:
110+
result = await aget_messages(session_id=session_id)
111+
finally:
112+
reset_current_flow_id(token)
113+
114+
assert len(result) == 2
115+
116+
117+
class _OldStyleMemory(MemoryComponent):
118+
"""Simulates a flow saved before PR #13087: its frozen ``retrieve_messages`` omits ``flow_id``.
119+
120+
We cannot recreate frozen bytes in a unit test, so we reproduce the one behavior that matters:
121+
the internal-memory retrieve calls ``aget_messages`` with no ``flow_id``. The engine's ambient
122+
flow scope must still isolate it.
123+
"""
124+
125+
name = "OldStyleMemory"
126+
127+
async def retrieve_messages(self) -> Data:
128+
from langflow.memory import aget_messages as backend_aget_messages
129+
130+
stored = await backend_aget_messages(session_id=self.session_id, order="DESC")
131+
return cast("Data", list(reversed(stored)))
132+
133+
134+
async def test_graph_execution_binds_flow_scope_end_to_end(client): # noqa: ARG001
135+
"""End-to-end: running Flow B's graph must not surface Flow A's message via unscoped frozen code.
136+
137+
This is the real reproduction path — ``get_instance_results`` binds ``graph.flow_id`` so the
138+
old-style component's unscoped ``aget_messages`` call is defaulted to Flow B's scope.
139+
"""
140+
flow_a, flow_b = uuid4(), uuid4()
141+
session_id = "shared-session-13059-e2e"
142+
await _store(session_id, flow_a, "SECRET_FROM_A")
143+
await _store(session_id, flow_b, "hello from B")
144+
145+
probe = _OldStyleMemory(_id="old_memory")
146+
probe.set(session_id=session_id, n_messages=100, order="Ascending")
147+
chat_output = ChatOutput(_id="chat_output")
148+
chat_output.set(input_value=probe.retrieve_messages_as_text)
149+
150+
graph = Graph(probe, chat_output, flow_id=str(flow_b))
151+
async for _ in graph.async_start():
152+
pass
153+
154+
rendered = chat_output.get_output_by_method(chat_output.message_response).value
155+
text = rendered.text if hasattr(rendered, "text") else str(rendered)
156+
assert "SECRET_FROM_A" not in text, "Flow A's message leaked into Flow B's graph run"
157+
assert "hello from B" in text
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
/**
2+
* LE-1929: flows saved when the node card was `w-96` (384px) kept `width: 384`. React Flow then
3+
* uses that stale width to place the right-side output handle ~64px past the actual `w-80` (320px)
4+
* card, so edges render detached from the handle. Reconciling drops the persisted dimensions so
5+
* React Flow re-measures the real DOM size. noteNodes are user-resizable and must be preserved.
6+
*/
7+
8+
import { reconcileStaleGenericNodeDimensions } from "../reactflowUtils";
9+
10+
describe("reconcileStaleGenericNodeDimensions", () => {
11+
it("drops stale width/height/measured from genericNodes so React Flow re-measures", () => {
12+
const nodes: any[] = [
13+
{
14+
id: "Memory-1",
15+
type: "genericNode",
16+
width: 384,
17+
height: 366,
18+
measured: { width: 384, height: 366 },
19+
position: { x: 0, y: 0 },
20+
data: {},
21+
},
22+
];
23+
24+
reconcileStaleGenericNodeDimensions(nodes as any);
25+
26+
expect(nodes[0].width).toBeUndefined();
27+
expect(nodes[0].height).toBeUndefined();
28+
expect(nodes[0].measured).toBeUndefined();
29+
});
30+
31+
it("preserves dimensions on noteNodes (they are user-resizable)", () => {
32+
const nodes: any[] = [
33+
{
34+
id: "note-1",
35+
type: "noteNode",
36+
width: 500,
37+
height: 300,
38+
measured: { width: 500, height: 300 },
39+
position: { x: 0, y: 0 },
40+
data: {},
41+
},
42+
];
43+
44+
reconcileStaleGenericNodeDimensions(nodes as any);
45+
46+
expect(nodes[0].width).toBe(500);
47+
expect(nodes[0].height).toBe(300);
48+
expect(nodes[0].measured).toEqual({ width: 500, height: 300 });
49+
});
50+
});
Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
/**
2+
* Reproduction for LE-1929: updating an outdated saved flow must not drop edges whose
3+
* output type was renamed (DataFrame -> Table, #11554).
4+
*
5+
* Flow B (Memory Chatbot, legacy) has a Memory."dataframe" output that was typed ["DataFrame"]
6+
* when saved but is ["Table"] in the current library. When the user clicks "Update All", the
7+
* node definition is swapped to the current one (Table) while the edge still stores the old
8+
* DataFrame handle. cleanEdges must migrate + keep the edge, not drop it.
9+
*/
10+
11+
import { cleanEdges, scapedJSONStringfy } from "../reactflowUtils";
12+
13+
const stringify = (obj: object): string => scapedJSONStringfy(obj);
14+
15+
describe("LE-1929 update-path edge migration (DataFrame -> Table)", () => {
16+
it("keeps the Memory->TypeConverter edge after the Memory output migrates to Table", () => {
17+
// Memory node AFTER "Update All": dataframe output is now typed Table.
18+
const memoryNode: any = {
19+
id: "Memory-8X8Cq",
20+
type: "genericNode",
21+
data: {
22+
id: "Memory-8X8Cq",
23+
type: "Memory",
24+
selected_output: "dataframe",
25+
node: {
26+
display_name: "Message History",
27+
template: {},
28+
outputs: [
29+
{
30+
name: "dataframe",
31+
display_name: "Table",
32+
types: ["Table"],
33+
selected: "Table",
34+
},
35+
],
36+
},
37+
},
38+
};
39+
40+
// TypeConverter node AFTER update: input_data accepts the migrated types.
41+
const typeConverterNode: any = {
42+
id: "TypeConverterComponent-koSIz",
43+
type: "genericNode",
44+
data: {
45+
id: "TypeConverterComponent-koSIz",
46+
type: "TypeConverterComponent",
47+
node: {
48+
display_name: "Type Convert",
49+
template: {
50+
input_data: {
51+
type: "other",
52+
input_types: ["Message", "Data", "JSON", "DataFrame", "Table"],
53+
display_name: "Input",
54+
},
55+
},
56+
outputs: [],
57+
},
58+
},
59+
};
60+
61+
// Edge as SAVED in the legacy flow: source handle still says DataFrame.
62+
const sourceHandle = stringify({
63+
dataType: "Memory",
64+
id: "Memory-8X8Cq",
65+
name: "dataframe",
66+
output_types: ["DataFrame"],
67+
});
68+
const targetHandle = stringify({
69+
fieldName: "input_data",
70+
id: "TypeConverterComponent-koSIz",
71+
inputTypes: ["Message", "Data", "DataFrame"],
72+
type: "other",
73+
});
74+
75+
const edge: any = {
76+
id:
77+
"xy-edge__Memory-8X8Cq" +
78+
sourceHandle +
79+
"-TypeConverter" +
80+
targetHandle,
81+
source: "Memory-8X8Cq",
82+
target: "TypeConverterComponent-koSIz",
83+
sourceHandle,
84+
targetHandle,
85+
data: {
86+
sourceHandle: {
87+
dataType: "Memory",
88+
id: "Memory-8X8Cq",
89+
name: "dataframe",
90+
output_types: ["DataFrame"],
91+
},
92+
targetHandle: {
93+
fieldName: "input_data",
94+
id: "TypeConverterComponent-koSIz",
95+
inputTypes: ["Message", "Data", "DataFrame"],
96+
type: "other",
97+
},
98+
},
99+
};
100+
101+
const result = cleanEdges([memoryNode, typeConverterNode], [edge]);
102+
103+
expect(result.brokenEdges.length).toBe(0);
104+
expect(result.edges.length).toBe(1);
105+
// The kept edge must have its source handle rewritten to the migrated Table type.
106+
expect(result.edges[0].sourceHandle).toContain("Table");
107+
});
108+
});

src/frontend/src/utils/reactflowUtils.ts

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,9 @@ export function cleanEdges(nodes: AllNodeType[], edges: EdgeType[]) {
125125
// check if the source and target handle still exists
126126
const sourceHandle = edge.sourceHandle; //right
127127
const targetHandle = edge.targetHandle; //left
128+
// Hold by reference: the target block rewrites edge.id on migration, so a second find-by-id in
129+
// the source block would miss it and skip the source-handle migration (DataFrame stays). LE-1929.
130+
const edgeInNewEdges = newEdges.find((e) => e.id === edge.id);
128131
if (targetHandle) {
129132
const targetHandleObject: targetHandleType = scapeJSONParse(targetHandle);
130133
const field = targetHandleObject.fieldName;
@@ -199,15 +202,14 @@ export function cleanEdges(nodes: AllNodeType[], edges: EdgeType[]) {
199202
isAdvanced) &&
200203
!isLoopInput
201204
) {
202-
newEdges = newEdges.filter((e) => e.id !== edge.id);
205+
newEdges = newEdges.filter((e) => e !== edgeInNewEdges);
203206
brokenEdges.push(generateAlertObject(sourceNode, targetNode, edge));
204207
} else if (
205208
targetHandlesMatchResult &&
206209
expectedTargetHandle !== targetHandle
207210
) {
208211
// Handles match via migration but IDs differ — update edge to use current types
209212
// so React Flow can find the DOM handle
210-
const edgeInNewEdges = newEdges.find((e) => e.id === edge.id);
211213
if (edgeInNewEdges) {
212214
edgeInNewEdges.targetHandle = expectedTargetHandle;
213215
if (edgeInNewEdges.data) {
@@ -284,7 +286,6 @@ export function cleanEdges(nodes: AllNodeType[], edges: EdgeType[]) {
284286
expectedSourceHandle !== sourceHandle
285287
) {
286288
// Handles match via migration but IDs differ — update edge to use current types
287-
const edgeInNewEdges = newEdges.find((e) => e.id === edge.id);
288289
if (edgeInNewEdges) {
289290
edgeInNewEdges.sourceHandle = expectedSourceHandle;
290291
if (edgeInNewEdges.data) {
@@ -1902,6 +1903,24 @@ export function processFlowNodes(flow: FlowType) {
19021903
flow.data.nodes = nodes;
19031904
flow.data.edges = edges;
19041905
}
1906+
reconcileStaleGenericNodeDimensions(flow.data.nodes);
1907+
}
1908+
1909+
/**
1910+
* Drop persisted width/height on genericNodes so React Flow re-measures the real DOM size.
1911+
*
1912+
* The component card is a fixed Tailwind width (`w-80`/`w-48`). Flows saved when that width was
1913+
* larger (`w-96` = 384px) kept `width: 384`, which React Flow then uses to place the right-side
1914+
* output handle — landing it ~64px past the visual dot so edges render detached (LE-1929). Only
1915+
* genericNodes are content-sized; noteNodes are user-resizable and keep their dimensions.
1916+
*/
1917+
export function reconcileStaleGenericNodeDimensions(nodes: AllNodeType[]) {
1918+
for (const node of nodes) {
1919+
if (node.type !== "genericNode") continue;
1920+
delete node.width;
1921+
delete node.height;
1922+
delete node.measured;
1923+
}
19051924
}
19061925

19071926
export function expandGroupNode(

0 commit comments

Comments
 (0)