Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/backend/base/langflow/memory.py
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,11 @@ async def aget_messages(
Returns:
List[Data]: A list of Data objects representing the retrieved messages.
"""
if flow_id is None:
# 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
from lfx.memory.flow_context import coerce_flow_id, get_current_flow_id

flow_id = coerce_flow_id(get_current_flow_id())
async with session_scope() as session:
stmt = _get_variable_query(
sender, sender_name, session_id, context_id, order_by, order, flow_id, limit, user_id=user_id
Expand Down
157 changes: 157 additions & 0 deletions src/backend/tests/unit/test_message_flow_context_scoping.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,157 @@
"""Regression tests: ambient flow-scope defaults ``aget_messages`` by ``flow_id`` (issue #13059).

Langflow executes the *frozen* component ``code`` embedded in each saved flow, not the installed
library version (``lfx.interface.initialize.loading.instantiate_class`` -> ``eval_custom_component_code``).
A flow saved before PR #13087 therefore carries the old ``retrieve_messages`` that calls
``aget_messages`` WITHOUT ``flow_id`` and leaks chat history across flows on a colliding
``session_id`` — even when running on a patched server.

Defense-in-depth: the engine binds the executing graph's ``flow_id`` to a ContextVar so
``aget_messages`` can default the scope. This closes the leak for old frozen flows without:
- touching frozen code (impossible),
- changing callers that pass ``flow_id`` explicitly (the default only applies when it is ``None``),
- changing callers outside a graph run (the ContextVar is unset -> identical to today).
"""

from typing import cast
from uuid import uuid4

from langflow.memory import aadd_messages, aget_messages
from langflow.schema.message import Message
from lfx.components.input_output import ChatOutput
from lfx.components.models_and_agents.memory import MemoryComponent
from lfx.graph.graph.base import Graph
from lfx.memory.flow_context import reset_current_flow_id, set_current_flow_id
from lfx.schema.data import Data


async def _store(session_id: str, flow_id, text: str) -> None:
msg = Message(text=text, sender="User", sender_name="User", session_id=session_id)
await aadd_messages([msg], flow_id=flow_id)


async def test_aget_messages_defaults_flow_id_from_context(client): # noqa: ARG001
"""The reproduction: two flows share a session_id; the ambient scope isolates them.

Simulates old frozen code calling ``aget_messages(session_id=...)`` with no flow_id while
Flow B's graph is executing. The ambient flow scope must prevent Flow A's row from leaking.
"""
flow_a, flow_b = uuid4(), uuid4()
session_id = "shared-session-13059"
await _store(session_id, flow_a, "secret from A")
await _store(session_id, flow_b, "hello from B")

token = set_current_flow_id(flow_b)
try:
scoped = await aget_messages(session_id=session_id)
finally:
reset_current_flow_id(token)

assert [m.text for m in scoped] == ["hello from B"]


async def test_explicit_flow_id_overrides_context(client): # noqa: ARG001
"""An explicitly-passed flow_id must win over the ambient context (no silent override)."""
flow_a, flow_b = uuid4(), uuid4()
session_id = "shared-session-13059-explicit"
await _store(session_id, flow_a, "secret from A")
await _store(session_id, flow_b, "hello from B")

token = set_current_flow_id(flow_b)
try:
a_msgs = await aget_messages(session_id=session_id, flow_id=flow_a)
finally:
reset_current_flow_id(token)

assert [m.text for m in a_msgs] == ["secret from A"]


async def test_no_context_preserves_legacy_unscoped(client): # noqa: ARG001
"""With no ambient scope and no explicit flow_id, behavior is unchanged (both rows)."""
flow_a, flow_b = uuid4(), uuid4()
session_id = "shared-session-13059-legacy"
await _store(session_id, flow_a, "secret from A")
await _store(session_id, flow_b, "hello from B")

unscoped = await aget_messages(session_id=session_id)

assert len(unscoped) == 2


async def test_context_accepts_string_flow_id(client): # noqa: ARG001
"""``graph.flow_id`` is commonly a ``str``; the default path must coerce it, not crash.

``MessageTable.flow_id`` is UUID-typed; on SQLite comparing it to a raw string makes the
``Uuid`` bind processor call ``value.hex`` and raise. The default must coerce str -> UUID.
"""
flow_a, flow_b = uuid4(), uuid4()
session_id = "shared-session-13059-str"
await _store(session_id, flow_a, "secret from A")
await _store(session_id, flow_b, "hello from B")

token = set_current_flow_id(str(flow_b))
try:
scoped = await aget_messages(session_id=session_id)
finally:
reset_current_flow_id(token)

assert [m.text for m in scoped] == ["hello from B"]


async def test_invalid_context_flow_id_degrades_to_unscoped(client): # noqa: ARG001
"""A non-UUID ambient flow_id (synthetic/test graphs) must degrade, never crash retrieval."""
flow_a, flow_b = uuid4(), uuid4()
session_id = "shared-session-13059-badctx"
await _store(session_id, flow_a, "secret from A")
await _store(session_id, flow_b, "hello from B")

token = set_current_flow_id("not-a-uuid")
try:
result = await aget_messages(session_id=session_id)
finally:
reset_current_flow_id(token)

assert len(result) == 2


class _OldStyleMemory(MemoryComponent):
"""Simulates a flow saved before PR #13087: its frozen ``retrieve_messages`` omits ``flow_id``.

We cannot recreate frozen bytes in a unit test, so we reproduce the one behavior that matters:
the internal-memory retrieve calls ``aget_messages`` with no ``flow_id``. The engine's ambient
flow scope must still isolate it.
"""

name = "OldStyleMemory"

async def retrieve_messages(self) -> Data:
from langflow.memory import aget_messages as backend_aget_messages

stored = await backend_aget_messages(session_id=self.session_id, order="DESC")
return cast("Data", list(reversed(stored)))


async def test_graph_execution_binds_flow_scope_end_to_end(client): # noqa: ARG001
"""End-to-end: running Flow B's graph must not surface Flow A's message via unscoped frozen code.

This is the real reproduction path — ``get_instance_results`` binds ``graph.flow_id`` so the
old-style component's unscoped ``aget_messages`` call is defaulted to Flow B's scope.
"""
flow_a, flow_b = uuid4(), uuid4()
session_id = "shared-session-13059-e2e"
await _store(session_id, flow_a, "SECRET_FROM_A")
await _store(session_id, flow_b, "hello from B")

probe = _OldStyleMemory(_id="old_memory")
probe.set(session_id=session_id, n_messages=100, order="Ascending")
chat_output = ChatOutput(_id="chat_output")
chat_output.set(input_value=probe.retrieve_messages_as_text)

graph = Graph(probe, chat_output, flow_id=str(flow_b))
async for _ in graph.async_start():
pass

rendered = chat_output.get_output_by_method(chat_output.message_response).value
text = rendered.text if hasattr(rendered, "text") else str(rendered)
assert "SECRET_FROM_A" not in text, "Flow A's message leaked into Flow B's graph run"
assert "hello from B" in text
50 changes: 50 additions & 0 deletions src/frontend/src/utils/__tests__/reconcileNodeDimensions.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/**
* LE-1929: flows saved when the node card was `w-96` (384px) kept `width: 384`. React Flow then
* uses that stale width to place the right-side output handle ~64px past the actual `w-80` (320px)
* card, so edges render detached from the handle. Reconciling drops the persisted dimensions so
* React Flow re-measures the real DOM size. noteNodes are user-resizable and must be preserved.
*/

import { reconcileStaleGenericNodeDimensions } from "../reactflowUtils";

describe("reconcileStaleGenericNodeDimensions", () => {
it("drops stale width/height/measured from genericNodes so React Flow re-measures", () => {
const nodes: any[] = [
{
id: "Memory-1",
type: "genericNode",
width: 384,
height: 366,
measured: { width: 384, height: 366 },
position: { x: 0, y: 0 },
data: {},
},
];

reconcileStaleGenericNodeDimensions(nodes as any);

expect(nodes[0].width).toBeUndefined();
expect(nodes[0].height).toBeUndefined();
expect(nodes[0].measured).toBeUndefined();
});

it("preserves dimensions on noteNodes (they are user-resizable)", () => {
const nodes: any[] = [
{
id: "note-1",
type: "noteNode",
width: 500,
height: 300,
measured: { width: 500, height: 300 },
position: { x: 0, y: 0 },
data: {},
},
];

reconcileStaleGenericNodeDimensions(nodes as any);

expect(nodes[0].width).toBe(500);
expect(nodes[0].height).toBe(300);
expect(nodes[0].measured).toEqual({ width: 500, height: 300 });
});
});
108 changes: 108 additions & 0 deletions src/frontend/src/utils/__tests__/updatePathEdgeMigration.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
/**
* Reproduction for LE-1929: updating an outdated saved flow must not drop edges whose
* output type was renamed (DataFrame -> Table, #11554).
*
* Flow B (Memory Chatbot, legacy) has a Memory."dataframe" output that was typed ["DataFrame"]
* when saved but is ["Table"] in the current library. When the user clicks "Update All", the
* node definition is swapped to the current one (Table) while the edge still stores the old
* DataFrame handle. cleanEdges must migrate + keep the edge, not drop it.
*/

import { cleanEdges, scapedJSONStringfy } from "../reactflowUtils";

const stringify = (obj: object): string => scapedJSONStringfy(obj);

describe("LE-1929 update-path edge migration (DataFrame -> Table)", () => {
it("keeps the Memory->TypeConverter edge after the Memory output migrates to Table", () => {
// Memory node AFTER "Update All": dataframe output is now typed Table.
const memoryNode: any = {
id: "Memory-8X8Cq",
type: "genericNode",
data: {
id: "Memory-8X8Cq",
type: "Memory",
selected_output: "dataframe",
node: {
display_name: "Message History",
template: {},
outputs: [
{
name: "dataframe",
display_name: "Table",
types: ["Table"],
selected: "Table",
},
],
},
},
};

// TypeConverter node AFTER update: input_data accepts the migrated types.
const typeConverterNode: any = {
id: "TypeConverterComponent-koSIz",
type: "genericNode",
data: {
id: "TypeConverterComponent-koSIz",
type: "TypeConverterComponent",
node: {
display_name: "Type Convert",
template: {
input_data: {
type: "other",
input_types: ["Message", "Data", "JSON", "DataFrame", "Table"],
display_name: "Input",
},
},
outputs: [],
},
},
};

// Edge as SAVED in the legacy flow: source handle still says DataFrame.
const sourceHandle = stringify({
dataType: "Memory",
id: "Memory-8X8Cq",
name: "dataframe",
output_types: ["DataFrame"],
});
const targetHandle = stringify({
fieldName: "input_data",
id: "TypeConverterComponent-koSIz",
inputTypes: ["Message", "Data", "DataFrame"],
type: "other",
});

const edge: any = {
id:
"xy-edge__Memory-8X8Cq" +
sourceHandle +
"-TypeConverter" +
targetHandle,
source: "Memory-8X8Cq",
target: "TypeConverterComponent-koSIz",
sourceHandle,
targetHandle,
data: {
sourceHandle: {
dataType: "Memory",
id: "Memory-8X8Cq",
name: "dataframe",
output_types: ["DataFrame"],
},
targetHandle: {
fieldName: "input_data",
id: "TypeConverterComponent-koSIz",
inputTypes: ["Message", "Data", "DataFrame"],
type: "other",
},
},
};

const result = cleanEdges([memoryNode, typeConverterNode], [edge]);

expect(result.brokenEdges.length).toBe(0);
expect(result.edges.length).toBe(1);
// The kept edge must have its source handle rewritten to the migrated Table type.
expect(result.edges[0].sourceHandle).toContain("Table");
});
});
25 changes: 22 additions & 3 deletions src/frontend/src/utils/reactflowUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,9 @@ export function cleanEdges(nodes: AllNodeType[], edges: EdgeType[]) {
// check if the source and target handle still exists
const sourceHandle = edge.sourceHandle; //right
const targetHandle = edge.targetHandle; //left
// 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);
Comment on lines +128 to +130

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 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`.

if (targetHandle) {
const targetHandleObject: targetHandleType = scapeJSONParse(targetHandle);
const field = targetHandleObject.fieldName;
Expand Down Expand Up @@ -199,15 +202,14 @@ export function cleanEdges(nodes: AllNodeType[], edges: EdgeType[]) {
isAdvanced) &&
!isLoopInput
) {
newEdges = newEdges.filter((e) => e.id !== edge.id);
newEdges = newEdges.filter((e) => e !== edgeInNewEdges);
brokenEdges.push(generateAlertObject(sourceNode, targetNode, edge));
} else if (
targetHandlesMatchResult &&
expectedTargetHandle !== targetHandle
) {
// Handles match via migration but IDs differ — update edge to use current types
// so React Flow can find the DOM handle
const edgeInNewEdges = newEdges.find((e) => e.id === edge.id);
if (edgeInNewEdges) {
edgeInNewEdges.targetHandle = expectedTargetHandle;
if (edgeInNewEdges.data) {
Expand Down Expand Up @@ -284,7 +286,6 @@ export function cleanEdges(nodes: AllNodeType[], edges: EdgeType[]) {
expectedSourceHandle !== sourceHandle
) {
// Handles match via migration but IDs differ — update edge to use current types
const edgeInNewEdges = newEdges.find((e) => e.id === edge.id);
if (edgeInNewEdges) {
edgeInNewEdges.sourceHandle = expectedSourceHandle;
if (edgeInNewEdges.data) {
Expand Down Expand Up @@ -1902,6 +1903,24 @@ export function processFlowNodes(flow: FlowType) {
flow.data.nodes = nodes;
flow.data.edges = edges;
}
reconcileStaleGenericNodeDimensions(flow.data.nodes);
}

/**
* Drop persisted width/height on genericNodes so React Flow re-measures the real DOM size.
*
* The component card is a fixed Tailwind width (`w-80`/`w-48`). Flows saved when that width was
* larger (`w-96` = 384px) kept `width: 384`, which React Flow then uses to place the right-side
* output handle — landing it ~64px past the visual dot so edges render detached (LE-1929). Only
* genericNodes are content-sized; noteNodes are user-resizable and keep their dimensions.
*/
export function reconcileStaleGenericNodeDimensions(nodes: AllNodeType[]) {
for (const node of nodes) {
if (node.type !== "genericNode") continue;
delete node.width;
delete node.height;
delete node.measured;
}
}

export function expandGroupNode(
Expand Down
Loading
Loading