feat: native v2 workflows endpoint with pluggable stream protocols - #13307
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:
WalkthroughThis PR implements a comprehensive v2 workflow execution system using AG-UI streaming protocol. It replaces the legacy developer-API/API-key-only architecture with session-based authentication, multi-mode execution (sync/stream/background), and streaming SSE frames through pluggable adapters. The backend introduces stream adapters for protocol abstraction, AG-UI translator for event lifecycle, and background buffering with reattach support. The frontend shifts to native AG-UI event handlers, pure state reducers, and orchestrated flow bridge integration. Extensive tests validate adapter contracts, event sequences, endpoint behavior, IDOR enforcement, and end-to-end workflows. ChangesAG-UI V2 Workflow System
🎯 4 (Complex) | ⏱️ ~60 minutes Possibly Related PRs
Suggested Reviewers
🚥 Pre-merge checks | ✅ 7 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (7 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #13307 +/- ##
===========================================
- Coverage 53.12% 50.41% -2.72%
===========================================
Files 2033 619 -1414
Lines 184171 58166 -126005
Branches 26195 5052 -21143
===========================================
- Hits 97843 29324 -68519
+ Misses 85219 27732 -57487
- Partials 1109 1110 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR introduces an end-to-end AG-UI execution path for Langflow workflows by routing the frontend “run/build” flow through the new POST /api/v2/workflows endpoint, which can stream AG-UI events over SSE (and also supports sync/background modes). It adds a backend event translator and supporting API changes, plus a frontend bridge/reducers/tests to consume AG-UI while keeping the existing v1 build path intact when the feature flag is off.
Changes:
- Backend: Add AG-UI
RunAgentInputcontract handling forPOST /api/v2/workflows, including streaming SSE translation, background buffering + reattach, and updated auth dependency to avoid long-lived DB sessions. - Frontend: Add
@ag-ui/clientintegration (agent + bridge + reducers/hooks) and gate the new run path behindLANGFLOW_V2_WORKFLOWS_AGUI_ENABLED. - Tests: Add/reshape unit + integration coverage for the translator, request contract, and parsing logic; adjust existing v2 workflow tests accordingly.
Reviewed changes
Copilot reviewed 25 out of 27 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| uv.lock | Adds Python dependency resolution entries for ag-ui-protocol. |
| src/backend/base/pyproject.toml | Pins ag-ui-protocol==0.1.18 for backend AG-UI types/events. |
| src/backend/base/langflow/services/auth/utils.py | Adds get_current_user_for_workflow (session-or-API-key auth without holding a DB session). |
| src/backend/base/langflow/api/v2/workflow.py | Implements AG-UI RunAgentInput handling, SSE streaming translation, background buffering + /events reattach, and updates execution flow. |
| src/backend/base/langflow/api/v2/converters.py | Adds ParsedWorkflowRun and parse_run_agent_input; updates converters to accept inputs dict directly. |
| src/backend/base/langflow/api/v2/agui_translator.py | New translator mapping Langflow EventManager events to AG-UI protocol events. |
| src/backend/base/langflow/api/v2/workflow_reconstruction.py | Updates reconstruction to use new converter signature (inputs={}) instead of WorkflowExecutionRequest. |
| src/backend/tests/unit/api/v2/test_workflow.py | Reduces/refocuses v2 tests to status/stop/IDOR; defers AG-UI contract tests to dedicated module. |
| src/backend/tests/unit/api/v2/test_workflow_agui.py | New no-mocks endpoint tests for AG-UI request contract + mode dispatch + streaming/background behaviors. |
| src/backend/tests/unit/api/v2/test_run_agent_input.py | New unit tests for parse_run_agent_input. |
| src/backend/tests/unit/api/v2/test_converters.py | Updates converter tests for new function signatures. |
| src/backend/tests/unit/api/v2/test_agui_translator.py | New unit tests for AG-UI translator correctness and well-formed event streams. |
| src/frontend/package.json | Pins @ag-ui/client@0.0.53. |
| src/frontend/package-lock.json | Locks @ag-ui/* dependency tree for the frontend. |
| src/frontend/vite.config.mts | Exposes LANGFLOW_V2_WORKFLOWS_AGUI_ENABLED to the frontend build. |
| src/frontend/src/customization/feature-flags.ts | Adds ENABLE_V2_WORKFLOWS_AGUI feature flag. |
| src/frontend/src/stores/flowStore.ts | Routes buildFlow through runFlowAGUI when the flag is enabled. |
| src/frontend/src/controllers/API/agui/run-agent.ts | Adds AG-UI workflow agent wrapper + buildRunInput. |
| src/frontend/src/controllers/API/agui/run-flow-bridge.ts | Adds bridge to fold AG-UI events into existing flowStore methods. |
| src/frontend/src/controllers/API/agui/state.ts | Adds pure reducer for AG-UI STATE_SNAPSHOT/STATE_DELTA canvas state. |
| src/frontend/src/controllers/API/agui/chat.ts | Adds pure reducer for AG-UI TEXT_MESSAGE_* chat lifecycle events. |
| src/frontend/src/controllers/API/agui/use-run-flow.ts | Adds a React hook around the AG-UI agent run/abort lifecycle. |
| src/frontend/src/controllers/API/agui/tests/state.test.ts | Adds tests for AG-UI canvas-state reducer. |
| src/frontend/src/controllers/API/agui/tests/chat.test.ts | Adds tests for AG-UI chat reducer. |
| src/frontend/src/controllers/API/agui/tests/run-agent.test.ts | Adds tests for buildRunInput and agent construction. |
| src/frontend/tests/utils/withEventDeliveryModes.ts | Collapses the v1 event-delivery-mode matrix to a single run when AG-UI is enabled. |
| .secrets.baseline | Updates baseline metadata/line number due to file changes. |
Files not reviewed (1)
- src/frontend/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| queue: asyncio.Queue = asyncio.Queue() | ||
| event_manager = create_default_event_manager(queue) | ||
| translator = AGUITranslator(run_id=run_id, thread_id=thread_id) |
| """Run a background flow, buffer its AG-UI frames, and finalize job status.""" | ||
| fresh_background_tasks = BackgroundTasks() | ||
| errored = False | ||
| try: | ||
| async for frame in _agui_event_frames( | ||
| flow_id=flow.id, | ||
| flow_name=flow.name, | ||
| background_tasks=fresh_background_tasks, | ||
| parsed=parsed, | ||
| current_user=current_user, | ||
| run_id=parsed.run_id or job_id, | ||
| thread_id=parsed.session_id or str(flow.id), | ||
| ): | ||
| if b'"RUN_ERROR"' in frame: | ||
| errored = True | ||
| await bg_run.append(frame) | ||
| finally: | ||
| await bg_run.finish() | ||
| with contextlib.suppress(Exception): | ||
| await get_job_service().update_job_status( | ||
| job_id, | ||
| JobStatus.FAILED if errored else JobStatus.COMPLETED, | ||
| ) |
| def __init__(self, user_id: str) -> None: | ||
| self.user_id = user_id | ||
| self.frames: list[bytes] = [] | ||
| self.done = False | ||
| self._cond = asyncio.Condition() | ||
|
|
||
| async def append(self, frame: bytes) -> None: | ||
| async with self._cond: | ||
| self.frames.append(frame) | ||
| self._cond.notify_all() |
| def parse_run_agent_input(run_input: RunAgentInput) -> ParsedWorkflowRun: | ||
| """Extract Langflow run parameters from a strict AG-UI ``RunAgentInput``. | ||
|
|
||
| The AG-UI body carries Langflow-specific fields in ``forwardedProps``; the | ||
| user's chat input is the last user message; the session is the ``threadId``. | ||
|
|
||
| Args: | ||
| run_input: The AG-UI request body. | ||
|
|
||
| Returns: | ||
| ParsedWorkflowRun: the Langflow run parameters. | ||
| """ | ||
| forwarded = run_input.forwarded_props if isinstance(run_input.forwarded_props, dict) else {} | ||
|
|
||
| input_value = "" | ||
| for message in reversed(run_input.messages or []): | ||
| if getattr(message, "role", None) == "user": | ||
| input_value = getattr(message, "content", "") or "" | ||
| break | ||
|
|
||
| data = forwarded.get("data") if isinstance(forwarded.get("data"), dict) else None | ||
| files_value = forwarded.get("files") | ||
| files = list(files_value) if isinstance(files_value, list) and files_value else None | ||
| return ParsedWorkflowRun( | ||
| flow_id=forwarded.get("flow_id"), | ||
| tweaks=forwarded.get("tweaks") or {}, | ||
| input_value=input_value, | ||
| session_id=run_input.thread_id, | ||
| run_id=run_input.run_id, | ||
| mode=forwarded.get("mode", "stream"), | ||
| start_component_id=forwarded.get("start_component_id"), | ||
| stop_component_id=forwarded.get("stop_component_id"), | ||
| data=data, | ||
| files=files, | ||
| ) |
| const buildStatus = | ||
| AGUI_STATUS_TO_BUILD_STATUS[value.status] ?? BuildStatus.BUILDING; | ||
| flowStore.updateBuildStatus([nodeId], buildStatus); | ||
| nodeIds.add(nodeId); |
The bridge's next handler reacted to RUN_FINISHED / RUN_ERROR by updating flowStore state but did not unsubscribe or resolve the runFlowAGUI promise. Resolution relied on the AG-UI observable completing after the terminal event, which holds only when the SSE stream closes cleanly. A keepalive after RUN_FINISHED, a buffered chunk the reader has not consumed, or a server that does not close eagerly all leave the observable open. The canvas then stays on isBuilding=true and running-status nodes never revert. The fix mirrors the existing teardown in the error: and complete: callbacks: call subscription.unsubscribe() + finish() on the terminal event itself so resolution does not depend on the SSE stream closing. Both branches updated symmetrically since RUN_FINISHED has the same hang risk as RUN_ERROR. No new test scaffold for this fix: reproducing the hang requires a long-lived fake SSE response plus the real flowStore singleton (pulls @xyflow/react and friends), and the fix surface is four lines mirroring the documented complete: pattern. Evidence for the bug comes from the PR #13307 code review and the step 6 code-review pass that re-surfaced the same risk on the success branch. All 29 controllers/API/agui tests still pass.
…hem on stop Two related bugs in the in-memory _BACKGROUND_RUNS registry that re-attach reads from. Both surfaced in the PR #13307 review; both have to land together so the registry's eviction policy and its cleanup path agree. A4: _register_background_run used to pop the oldest entry by insertion order when the dict hit _MAX_BACKGROUND_RUNS. A long-running first job got evicted by the 101st short job mid-run; re-attach returned 404 while the still-buffering task appended into an orphaned _BackgroundRun. The new policy prefers evicting the oldest completed entry. If every slot is still running, evict the oldest anyway to keep the registry bounded and log a warning so the situation is visible. A5: stop_workflow revoked the buffer task but left the _BackgroundRun in the registry with done=False. Re-attach readers could hang on _cond.wait() indefinitely (the task that would have called finish() was cancelled mid-execution), and cancelled buffers occupied memory until LRU evicted them. New _clear_background_run helper pops the entry and calls await bg_run.finish() so waiters wake to a clean stream end. stop_workflow calls it after revoke_task. Four unit tests pin both behaviors: eviction prefers completed entries, eviction falls back to the oldest when every run is active, clear pops and finishes the buffer, and clear is a no-op for unknown job ids. All use real _BackgroundRun instances via monkeypatch on the module-level dict; no mocks.
…rtex builds A completed background job's GET status 500'd with 'No vertex builds found for job_id'. The background build path differed from the sync path twice: 1. generate_flow_events minted a fresh run_id instead of using job_id, so vertex builds were keyed by an id the status query never uses. Thread run_id through _stream_event_frames -> generate_flow_events and pass job_id from the background buffer so graph.run_id == job_id (the sync path already does graph.set_run_id(job_id)). 2. The SSE build loop (build_vertices) only persisted builds when log_builds was set and never passed job_id. Tie log_builds to job-tracked runs (run_id present) and pass job_id=graph.run_id on the persist call. Job-tracked runs also persist streaming terminal vertices so reconstruction is complete; the live build path (run_id is None) keeps its original behavior, so the v1 build path is unchanged. Test: a real background run polled to completion, then GET status asserts a reconstructed 200 (verified RED: 500 'No vertex builds found' before the fix). Covers the non-streaming flow. v1 build path unchanged (35 build tests pass); AG-UI suite 46 pass. LE-1389
Adopt 1.11.0's composed group-mixin Settings for lfx settings/base.py (all v2 settings already present in the new groups); port the Python 3.14 cors_origins ['*']->'*' fix into groups/security.py.
Suggestion: introduce an event-sourcing surface in
|
| Event | Payload |
|---|---|
message.start |
{ id, role, parent_message_id? } |
message.delta |
{ id, chunk } |
message.end |
{ id, final_text?, usage? } |
tool.start |
{ tool_call_id, message_id, name, args } |
tool.end |
{ tool_call_id, result | error } |
content.block |
{ id, type, data } for json / code / media |
- Components opt in via a new
Component.stream_message_v2helper. - Legacy
send_message/token/add_messagekeep working untouched — v1 frontend is unaffected. - Protocol adapters consume the explicit lifecycle 1:1 — no inference, no dedupe, no singular-open invariant — and naturally support parallel streams via id multiplexing.
This is the industry-standard pattern for multi-stream protocols:
- Anthropic Messages API — content-block
index - OpenAI Responses API —
item_id/output_index - AG-UI —
message_id,tool_call_id - LangChain
astream_events—run_id,parent_ids
Why now
- Cost is bounded: two event paths in
EventManagerduring a transition period. - Value compounds with every new adapter (AG-UI, OpenAI Responses, MCP, …).
- Unblocks parallel-agent flows on AG-UI without protocol violations.
Tactical bridge (this PR scope, optional)
Switch AGUITranslator from a singular _open_message_id: str | None to _open_message_ids: set[str] — preserves the dual-emission dedupe (the real LF-specific reason _emitted_text_message_ids exists) while removing the close-on-id-switch heuristic that causes the drops. ~10-line change; fixes the immediate AG-UI bug without waiting on the larger event-vocabulary work.
Out of scope
- Migrating persistence to event-sourcing wholesale (DB write becomes a consumer of the stream). Right model in the abstract; breaks v1 and every existing component. Not proposed here.
… of dropping them Parallel components stream tokens for different message ids interleaved. The translator tracked a single open message: the first foreign token closed the open message and tombstoned its id, so every later event for it was dropped and its remaining text never reached the client. Tokens for a message that cannot take the wire now buffer until the open message genuinely ends (its add_message finalizer), then flush in arrival order; complete messages landing mid-stream buffer the same way instead of interleaving a second START. end/error drain all buffers before the terminal event. The wire still carries at most one open text message, so the stream stays AG-UI-conformant.
… purge removed buffers A partial add_message re-fire (the agent path emits these at tool start/end for a message it is still streaming) was treated as the finalizer: it closed and tombstoned the id, so the post-tool answer was dropped. Only a non-partial add_message finalizes now; state defaults to complete, so payloads without properties are unchanged. remove_message now purges a buffered message and tombstones its id, so text the backend retracted is not flushed to the client later.
…ai/langflow into codex/v2-workflows-agui-merge # Conflicts: # src/backend/base/langflow/api/v2/agui_translator.py # src/backend/tests/unit/api/v2/test_agui_translator.py
|
Hey @dkaushik94, thanks for the really thoughtful write-up. Two parts here, and I want to answer both. On the tactical bridge: the drop you spotted is already fixed on the branch, and it went a bit further than the On the bigger idea: I think you're right that the inference tax is real and it compounds per adapter. An explicit |
ts-jest compiles with target es5; without downlevelIteration, [...set] and for...of over a Set/Map emit ES5 that yields nothing. That silently broke the AG-UI bridge tests: runningNodeIds spread, markRunningNodesFailed, and restoreOriginalBuildStatuses all iterated empty. Production (Vite/SWC, modern target) was never affected; only the ts-jest harness was. Fixes the 3 failing jest tests on this branch with no other suite changes (4994/4994 pass).
A branch component (If-Else, Conditional Router) reports its not-taken vertices in build_data.inactivated_vertices, but the AG-UI translator only emitted the branch node's own success/error status and dropped that list. The canvas seeds every planned node as pending from vertices_sorted; skipped vertices then get no build_start/end_vertex, so they stayed stuck on pending instead of rendering as inactive (the v1 build path marked them INACTIVE). The translator now appends an inactive STATE_DELTA op per inactivated vertex, and the frontend bridge maps the new inactive status to BuildStatus.INACTIVE and tears its edges down like a completed node. Fixes the If-Else regression in general-bugs-reset-flow-run.spec.ts.
build.py keeps reporting a conditionally-excluded vertex in inactivated_vertices on every subsequent end_vertex (the excluded set persists until the ConditionalRouter clears it), so the translator was putting the same inactive STATE_DELTA on the wire once per remaining vertex. Track emitted inactive nodes and skip re-emitting; drop a node from the set when it actually runs again (build_start/end_vertex) so a loop re-activation can still re-emit inactive later.
Cristhianzl
left a comment
There was a problem hiding this comment.
⛔ Blockers (resolve before merge)
B1 — workflow.py exceeds file-structure hard limits (LOC, classes, mixed responsibilities)
File: src/backend/base/langflow/api/v2/workflow.py (whole file, ~1,565 lines / ~1,270 code lines)
Issue: The PR grows this module by +977 lines to ~1,270 code lines. It holds 26 top-level symbols including two classes (_WorkflowEventQueue, _BackgroundRun) and four clearly different responsibility groups in one file: HTTP route handlers (execute_workflow, get_workflow_status, stop_workflow, reattach_workflow_events), request validation (_validate_flow_data_for_execution, _validate_output_ids, _reject_unsupported_sync_fields, _enforce_flow_data_override_owner), execution orchestration (execute_sync_workflow*, _stream_event_frames, execute_workflow_background), and an in-memory background-run buffer + registry (_BackgroundRun, _register_background_run, _finalize_job_status, _clear_background_run, _finish_cancelled_background_run).
Why it matters: The repo hard rules cap a code file at 500 LOC (600–700 only when SRP holds), 1 main class, and forbid mixing validate* with execution/persistence prefixes in one file. At 1,270 LOC with two stateful classes and four responsibility groups, none of those hold. This is the highest-traffic module in the PR (a new public endpoint + streaming), so the maintenance and review cost compounds. The hook that enforces the 500-LOC cap blocks at >700.
Suggested fix: Split by responsibility, e.g.:
workflow.py— route handlers only (the four@routerfunctions), delegating to the modules below.workflow_execution.py—execute_sync_workflow*,_build_run_inputs,_resolve_request_variables,_stream_event_frames,_execute_streaming_workflow.workflow_background.py—_BackgroundRun,_WorkflowEventQueue, registry helpers (_register_background_run,_finalize_job_status,_clear_background_run,_finish_cancelled_background_run,_buffer_background_run).workflow_validation.py— the_validate_*/_reject_*/_enforce_*guards.
Top-level symbols (grep)
class _WorkflowEventQueue: (647)
async def _stream_event_frames( (694)
class _BackgroundRun: (880)
async def execute_workflow( (252) async def get_workflow_status( (1219)
async def stop_workflow( (1378) async def reattach_workflow_events( (1494)
def _validate_flow_data_for_execution / _validate_output_ids / _reject_unsupported_sync_fields / _enforce_flow_data_override_owner
def _register_background_run / _finalize_job_status / _clear_background_run / _finish_cancelled_background_run
⚠️ Important (preferably this PR)
I1 — No execution timeout on stream / background / public modes (unbounded LLM runtime dependency)
File: src/backend/base/langflow/api/v2/workflow.py:319-360, :1067-1147; src/backend/base/langflow/api/v2/workflow_public.py:173-188
Issue: Only sync mode is wrapped in asyncio.wait_for(..., timeout=EXECUTION_TIMEOUT). stream, background, and the entire public endpoint drive generate_flow_events (which executes the graph, including any LLM components) with no upper bound on run time.
Why it matters: A flow that calls an LLM is a network call to a slow, non-deterministic external service. On the public endpoint an anonymous visitor can start a run that holds a server task (and a process-local buffer for background) indefinitely. With no per-run wall-clock ceiling, a hung upstream provider or a deliberately slow consumer ties up resources. The repo's AI-runtime-resilience rules require an explicit timeout on every path that runs an LLM, not just sync.
Suggested fix: Apply a wall-clock ceiling to the stream/background/public drive loop too (e.g. cancel run_task after a configurable STREAM_EXECUTION_TIMEOUT, emitting the adapter's terminal-error event). At minimum document why streaming is intentionally unbounded if that is the decision, and gate the public endpoint with a tighter cap.
I2 — Public streaming endpoint has no rate limiting and no per-run cost ceiling
File: src/backend/base/langflow/api/v2/workflow_public.py:65-188
Issue: POST /api/v2/workflows/public is anonymous (owner-impersonated) and, combined with I1, has no rate limit, no concurrency cap per client/IP, and no token/cost ceiling. The app wires slowapi globally (main.py) but no @limiter.limit is applied here.
Why it matters: This is a new unauthenticated, externally-reachable endpoint that executes flows under the owner's credentials and can invoke paid LLM calls. Without a rate limit it is an amplification and cost-exhaustion vector against the flow owner's provider keys. The security checklist lists "rate limiting on all public-facing endpoints" as a required control.
Suggested fix: Confirm whether v1 build_public_tmp carries a limit and reach at least parity; if neither has one, add a @limiter.limit(...) to the public endpoint (per client_id / IP) and a per-run cost/iteration ceiling. If the decision is to rely on an upstream gateway, capture that in the PR description.
I3 — Internal exception messages leaked to clients in error bodies
File: src/backend/base/langflow/api/v2/workflow.py:427-436, :375-384, :1256-1263, :1362-1370, :1477-1485
Issue: Several catch-all handlers embed str(exc) / f"{err!s}" / f"{e!s}" directly in the response message (e.g. "An unexpected error occurred: {err!s}", "Failed to retrieve job from database: {exc!s}", "Failed to stop job: {job_id} - {exc!s}").
Why it matters: "No internal details exposed in error messages to end users" is a general security control. Raw exception text from the DB layer or graph build can leak table names, file paths, driver internals, or stack-derived detail to an API client (and, via the streamed error event, to anonymous public-flow visitors). Note the public endpoint deliberately sanitizes the blocked-component message to "This flow cannot be executed." — the authenticated handlers are inconsistent with that posture.
Suggested fix: Log the full exc server-side (already partly done) and return a generic, code-tagged message to the client ("Internal server error" + the existing code), keeping the detailed string out of the body. Apply the same to the langflow/agui adapter error_events(str(error)) path that reaches the wire.
I4 — _resolve_request_variables / body globals reach component runtime without validation beyond length
File: src/backend/base/langflow/api/v2/workflow.py:230-241, :532-535; src/lfx/src/lfx/schema/workflow.py:245-254
Issue: Body globals (and X-LANGFLOW-GLOBAL-VAR-* headers) are merged and injected into graph.context["request_variables"] for every authenticated run. The only constraint is key/value length (GLOBAL_KEY_MAX_LEN / GLOBAL_VALUE_MAX_LEN). The header path in extract_global_variables_from_headers is not visible in this diff and is trusted as-is.
Why it matters: These values become component inputs at runtime (the trust-boundary question: "who controls each value, and could they lie?"). Caller-controlled globals that silently override a flow's configured global variables is a privilege/data-exposure surface if any component treats a global as trusted (e.g. a connection string or a path). The PR description and code don't state which globals are allowed to be overridden per-request.
Suggested fix: Document and, if needed, allowlist which global keys a request may set, or confirm in a Why: comment / PR ## Design Decisions that request-level globals are intentionally unrestricted and that no component treats a global as a trust boundary. This is the kind of assumption the comprehension audit asks to be captured durably.
💡 Recommended (can ship as a follow-up)
R1 — Misleading "commented out" comment over live dataframe code; ungated TODOs
File: src/backend/base/langflow/api/v2/converters.py:262-274, :366-367
Issue: The comment says "The following code is commented out pending further requirements analysis" immediately above a if output_type == "dataframe": block that is not commented out and actually executes. There are also two # TODO: Future scope comments with no ticket reference.
Why it matters: A comment that contradicts the code is worse than no comment — a future maintainer will trust the wrong statement. The repo rules forbid WHAT-comments and TODOs without a ticket.
Suggested fix: Delete the misleading comment, keep the executing dataframe branch (or remove it if truly unused), and either drop the TODOs or attach a tracking issue id.
R2 — Likely-dead legacy schema WorkflowExecutionRequest
File: src/lfx/src/lfx/schema/workflow.py:113-178
Issue: WorkflowExecutionRequest (with validate_execution_mode, background/stream booleans, flat inputs) is only referenced by the lazy __getattr__ export in lfx/schema/__init__.py; the new endpoint uses WorkflowRunRequest. No production caller in the diff constructs it.
Why it matters: YAGNI / dead-code: a ~65-line request model with its own validator that nothing executes invites confusion about which is the real request shape.
Suggested fix: If nothing outside tests uses it, remove it (and its export) in this PR or file a follow-up to retire it once the v1 build path is deleted (the PR already lists that as a follow-up).
R3 — agui adapter cancel_events routes cancellation through the generic error translation
File: src/backend/base/langflow/api/v2/adapters/agui.py:508-511
Issue: Cancellation emits a RUN_ERROR via translate("error", {"error": reason}). A user-initiated stop is then indistinguishable from a real failure on the wire (both become RUN_ERROR), and the buffer's terminal_error_type == "RUN_ERROR" will mark a cancelled job as errored unless the CANCELLED guard in _finalize_job_status wins the race.
Why it matters: Clients and analytics can't tell "user cancelled" from "run failed". The _finalize_job_status CANCELLED-protection comment acknowledges this is a race.
Suggested fix: Emit a distinct cancellation signal (an AG-UI CUSTOM langflow.cancelled event, or rely on the status row) so cancel and error are separable downstream; keep the RUN_ERROR only if AG-UI genuinely has no cancellation primitive and document that.
R4 — reattach_workflow_events 409 message reveals worker-affinity internals
File: src/backend/base/langflow/api/v2/workflow.py:1540-1552
Issue: The 409 body explains "Buffered events ... are not available on this worker ... route ... requests to the worker that accepted the background run."
Why it matters: Minor information disclosure about deployment topology to any caller. Low severity but inconsistent with the privacy-first posture elsewhere (404-on-cross-user).
Suggested fix: Keep the actionable hint ("use the status endpoint") but drop the worker-routing internals from the client-facing message; log them server-side instead.
🟢 Nice-to-have
N1 — WorkflowStreamEvent schema appears unused by the actual stream wire shape
File: src/lfx/src/lfx/schema/workflow.py:432-493
Issue: WorkflowStreamEvent (type/run_id/timestamp/raw_event) is advertised in the OpenAPI text/event-stream schema, but the real frames are adapter-specific ({"event","data"} for langflow, AG-UI events for agui). The documented schema doesn't match either wire shape.
Suggested fix: Either align the OpenAPI example with one real protocol's frame or annotate that the SSE body shape depends on stream_protocol.
N2 — parse_workflow_run_request is a thin field-copy that duplicates the request model
File: src/backend/base/langflow/api/v2/converters.py:72-98
Issue: ParsedWorkflowRun mirrors WorkflowRunRequest field-for-field; the parse step is mostly a rename layer (mode.value, run_id=None).
Suggested fix: Acceptable as a boundary DTO, but consider whether the route could carry WorkflowRunRequest plus a resolved run_id directly to cut one mapping layer. Low priority.
erichare
left a comment
There was a problem hiding this comment.
Reviewed the full incremental diff — backend v2 endpoint, stream adapters, AG-UI translator, converters, background-job reconstruction, the auth refactor, and the frontend AG-UI bridge — plus a dedicated security pass on the public endpoint and auth changes.
Security: the public path is clean on the RCE surface. validate_public_flow_no_code_execution() (blocking PythonREPL*/PythonCodeStructuredTool/Smart Transform plus transitive RunFlow/SubFlow/FlowTool) runs before any graph build; the endpoint is gated to PUBLIC flows only; PublicWorkflowRunRequest is extra="forbid" with no data/tweaks, so a caller can't inject nodes; session IDs stay namespaced; and get_current_user_for_workflow doesn't weaken token/active-user validation. No auth bypass found.
No CRITICAL issues. The inline notes below are correctness/robustness items. Frontend terminal-event teardown, the extra="forbid" wire body, unknown-protocol -> 422 across all modes, and stream-exception -> terminal-frame handling all checked out, and the background/translator tests are meaningful rather than shallow.
| @@ -69,12 +68,11 @@ async def reconstruct_workflow_response_from_job_id( | |||
|
|
|||
| # Create RunResponse and convert to WorkflowExecutionResponse | |||
| run_response = RunResponse(outputs=run_outputs_list, session_id=None) | |||
There was a problem hiding this comment.
[HIGH] Background status reconstruction always returns session_id: null. run_response_to_workflow_response echoes RunResponse.session_id into the API response, but reconstruct_workflow_response_from_job_id builds RunResponse(outputs=..., session_id=None). So GET /api/v2/workflows?job_id=... returns session_id: null for every completed background job, even though the run executed under a real session.
The schema documents this field as the handle to continue the same chat/memory thread — and background is exactly the mode where the client wasn't streaming and most needs to recover it. The session is known to the job/build rows; thread it through instead of hardcoding None.
There was a problem hiding this comment.
Hey @erichare, good catch. The session isn't on the job or vertex_build columns, but it's persisted inside the terminal message data, so reconstruction pulls it from there now. Data-only flows stay null since there's no thread to continue. Added an e2e test that runs a background flow and asserts the status echoes the real session.
| evict_key, | ||
| job_id, | ||
| ) | ||
| _BACKGROUND_RUNS.pop(evict_key, None) |
There was a problem hiding this comment.
[MEDIUM] Evicting a still-running background run orphans its buffer writer. The eviction here only pops the registry entry — it doesn't finish()/cancel the evicted run's _buffer_background_run coroutine, which holds a direct reference to the _BackgroundRun and keeps appending frames.
Under sustained load (>100 concurrent background jobs) the evicted buffer keeps growing with no registry entry and no reader able to find it, and a re-attach to that job now 409s even though the run is healthy — which partially defeats the bounded-memory guarantee this registry is meant to provide. When evicting a still-running entry, also cancel its queue job (or at least finish() the buffer) so the orphaned coroutine stops writing.
There was a problem hiding this comment.
Right, popping the entry without stopping the writer leaks. The fallback now cancels the evicted run's queue job so the buffer coroutine actually stops. Had to make _register_background_run async for it.
| return "unknown" | ||
|
|
||
|
|
||
| def build_component_output( |
There was a problem hiding this comment.
[MEDIUM] Per-component status is hardcoded COMPLETED. build_component_output sets status=JobStatus.COMPLETED for every terminal vertex; the only failure signal is the top-level error path when execute_with_status raises.
A graph that completes with a vertex that produced an error artifact without raising (a partial failure — the scenario the two-tier error handling advertises) still reports outputs[id].status == COMPLETED, so a client trusting per-component status gets false positives. Consider deriving status from the vertex valid flag, which the stream path already has.
There was a problem hiding this comment.
Fixed the hardcoded COMPLETED. Status comes from the error artifact / valid flag now, and the langflow adapter stops dropping valid so it lines up with what the agui translator already does. One caveat: I couldn't reproduce a non-raising partial failure reaching the sync converter on this branch (component errors raise and hit the top-level FAILED path), so this is really hardening the contract and fixing the stream/agui divergence, not an observed sync false-positive.
| # Resolve request-level variables: body ``globals`` plus the legacy | ||
| # X-LANGFLOW-GLOBAL-VAR-* headers (still used by the Responses API). | ||
| # Body globals win on conflict. | ||
| request_variables = _resolve_request_variables(parsed.globals, http_request) |
There was a problem hiding this comment.
[MEDIUM] Request-body globals is honored in sync mode only. This sync path merges parsed.globals into graph context, but the stream/background paths call generate_flow_events and never pass parsed.globals anywhere — so globals is silently dropped for mode=stream/background.
The field description makes no mode distinction ("Body globals always win over the legacy headers"). Either honor globals on all paths, or document the sync-only limitation in the field description the way output_ids already documents "Ignored for stream/background".
There was a problem hiding this comment.
I went with documenting the sync-only limitation like output_ids does. Honoring globals on stream/background means threading context through generate_flow_events and the shared build_graph_* helpers that the v1 build endpoint also uses, so I'd rather do that as its own change. Description now says honored in sync, ignored for stream/background.
| def cancel_events(self, reason: str) -> Iterable[StreamEvent]: | ||
| # AG-UI has no cancellation primitive in the local event model; route | ||
| # through the translator so any open text lifecycle is closed first. | ||
| return [_to_stream_event(e) for e in self._translator.translate("error", {"error": reason})] |
There was a problem hiding this comment.
[MEDIUM] User-cancel is replayed as RUN_ERROR in the AG-UI stream. cancel_events routes through translate("error", ...) -> RunErrorEvent. The job row correctly lands CANCELLED, but a re-attaching client sees a stream that ends in RUN_ERROR, indistinguishable from a genuine failure — the frontend bridge then fires "Workflow run failed" and marks nodes ERROR for a deliberate stop.
AG-UI has no cancel primitive, but emitting RUN_FINISHED after closing any open messages — or a CUSTOM cancel marker — would be more honest than RUN_ERROR. The langflow adapter has the same issue.
There was a problem hiding this comment.
Agreed, replaying a stop as RUN_ERROR was misleading. The agui path now closes any open text, emits a CUSTOM langflow.run.cancelled marker, then RUN_FINISHED, and the langflow adapter emits a cancelled event instead of error. So a re-attaching client can tell a stop from a real failure.
| "Mirrors the security posture of /api/v1/build_public_tmp." | ||
| ), | ||
| ) | ||
| async def execute_public_workflow( |
There was a problem hiding this comment.
[MEDIUM] No rate limiting + unbounded input on the unauthenticated public endpoint. This endpoint has no per-IP/per-flow rate limiting, and PublicWorkflowRunRequest.input_value/session_id have no max_length.
An anonymous caller can trigger concurrent flow executions (each running as the flow owner — real CPU/DB/LLM-credit cost) and post arbitrarily large strings that are held in memory and persisted to MessageTable. The per-run event queue is bounded (256) but the number of concurrent runs is not. This mirrors the v1 public endpoint, but v2 replicates the exposure rather than improving on it — suggest a configurable per-IP limit plus StringConstraints(max_length=...) on the public request fields, consistent with the 64 KB GlobalVarValue bound.
There was a problem hiding this comment.
Did both halves. input_value/session_id have max_length now (64KB / 256, matching GlobalVarValue), and there's a per-IP throttle with its own knob (public_flow_rate_limit_per_minute, default 20/min) so it doesn't borrow the login limit. It runs before any DB work.
| summary="Re-attach to a background run", | ||
| description="Replay the buffered protocol-native events for a background run and tail until it ends.", | ||
| ) | ||
| async def reattach_workflow_events( |
There was a problem hiding this comment.
[MEDIUM] reattach_workflow_events gates on the buffer owner only, bypassing the RBAC layer. When a local bg_run exists, access is gated solely by bg_run.user_id != str(current_user.id), unlike the sibling handlers in this file that route through ensure_flow_permission/share-aware fetch.
Under an authz plugin, a user holding a share on the flow (who can read its job status) can't re-attach to its event stream, and the owner check ignores RBAC entirely. Worth aligning with the other handlers, or adding a comment that stream re-attach is intentionally owner-only. (Access is correctly user-scoped, so this is a consistency/feature note, not an IDOR.)
There was a problem hiding this comment.
This one's deliberate so I added a comment. The live siblings (stop, active-status) are owner-only too; only the COMPLETED-status branch is share-aware because it reloads the flow, and a share-holder can still tail via the status endpoint. Making the live stream share-aware would mean touching stop and active-status to stay consistent. Do you think it's worth it, or is owner-only fine here?
- recover session_id for completed background jobs from the persisted terminal message instead of always returning null, so GET status can continue the same chat/memory thread - replay a user-cancel as a CUSTOM cancel marker + RUN_FINISHED (agui) and a `cancelled` terminal (langflow) instead of RUN_ERROR, so a re-attaching client no longer reads a deliberate stop as a failure - cancel the evicted still-running buffer writer when the background-run registry is full, so it stops appending into a run no reader can find - derive per-component status from the error artifact / valid flag instead of hardcoding COMPLETED, and stop the langflow adapter dropping `valid` - throttle the unauthenticated public endpoint per IP and bound its input_value/session_id length - document the sync-only scope of request-body globals - document that live event re-attach is intentionally owner-only
Splits the ~1.5k-line workflow.py into focused modules and folds in the execution-timeout and error-sanitization fixes from Cristhianzl's review of #13307. - B1: workflow.py now holds only the four route handlers. Validation guards move to workflow_validation, the sync/stream run loop to workflow_execution, and the durable background machinery to workflow_background (layered, acyclic). - I1: add workflow_execution_timeout (default 300) and apply a single wall-clock ceiling across sync, stream, background, and public via _stream_event_frames. A timeout becomes a sanitized terminal error and marks a background job failed. - I3: the route error handlers no longer echo raw exception text. They return a generic, code-tagged message and log the full exception server-side. - R1: remove the "commented out / future scope" comments that sat over live dataframe-extraction code in converters.py. - R4: drop the worker-routing internals from the reattach 409 message. Tests cover the timeout terminal-error path and the error-body sanitization, and the settings field-count guard is updated for the new setting.
|
Hey @Cristhianzl, thanks for the thorough review. Pushed a21d0d9 with the blockers addressed. B1 (split): workflow.py now holds just the four route handlers (~660 lines). The validation guards moved to I1 (timeouts): added a I3 (error leak): the route handlers don't put I2 and R3 landed in the earlier round on this PR: the public endpoint has a per-IP throttle with its own knob ( I also folded in two of the recommended ones since I was already in those files: R1 (removed the "commented out / future scope" comments sitting over live dataframe code) and R4 (the reattach 409 just points at the status endpoint now, no worker-routing detail). A few I'd like your read on: I4 (request globals): I added The streamed terminal error and the sync 200 body still return R2 (dead Full v2 suite is green, plus new tests for the timeout terminal-error path and the error-body sanitization. |
dkaushik94
left a comment
There was a problem hiding this comment.
LGTM. My concerns are addressed. I see others also contributed meaningfully, so I think we should be feeling great on this PR, Gabe!
Routes the playground and canvas through
POST /api/v2/workflows, a Langflow-native endpoint with a pluggable stream-protocol layer. Frontend pinsstream_protocol: "agui"so the AG-UI typed event stream keeps driving canvas + chat-view.flowStore.buildFlownow goes through the v2 endpoint unconditionally; the frontend v1 build branch is removed. The backendapi/v1/chat.pybuild pipeline still exists for now (retiring it is a follow-up below).On the diff size
It's large on paper, but ~2/3 of it is tests: 5,438 of the 8,173 added lines (19 test files). The production surface is ~2,600 lines, and most of it lands in new, additions-only v2 modules. Edits to existing files are small (
flowStore.tsis a net deletion of the v1 build path). Suggested read order:lfx/schema/workflow.py→api/v2/workflow.py→api/v2/adapters/→converters.py/agui_translator.py→ frontendcontrollers/API/agui/run-agent.ts+run-flow-bridge.ts→stores/flowStore.ts.Backend
The endpoint takes a native
WorkflowRunRequest(extra="forbid"):{ "flow_id": "...", "input_value": "...", "mode": "sync" | "stream" | "background", "stream_protocol": "langflow" | "agui", "session_id": "...", "tweaks": {...}, "data": {"nodes": [...], "edges": [...]}, "files": [...] }modedefaults tosync(one curl, one JSON back).stream_protocoldefaults tolangflow(passthrough of EventManager events as{"event": "...", "data": {...}}). Unknownstream_protocolreturns 422 with the available list for every mode, not just streaming.The sync (and status) response is flat. The text reply is surfaced at the top level as
output_text(the flow's single ChatOutput/TextOutput;nullwhen a flow has no single text output, so callers readoutputsinstead of the shortcut guessing which channel is the answer), andsession_idechoes the resolved session so chat/memory callers can continue the same thread (v1/runreturned this; v2 had dropped it). The per-component results stay inoutputs, keyed by component id. Both fields are additive, so the existingoutputscontract is unchanged.Stream dispatch goes through a
StreamAdapterregistry.langflowis passthrough;aguiwraps the existingAGUITranslator. Adding a third protocol is aregister_stream_adaptercall.Background mode buffers the chosen protocol's frames per job.
GET /api/v2/workflows/{job_id}/eventsre-attaches withLast-Event-ID./stopreleases the buffer and wakes waiters so they see a clean stream end instead of hanging. The in-memory registry prefers evicting completed runs over still-running ones so a long-running job's re-attach handle survives the 101st short job.Combined session-cookie-or-API-key auth that does not hold a DB session across the request (fixes a SQLite lock bug under the obvious
get_current_active_userchoice).266 v2 backend tests + 31 translator tests pass.
Frontend
@ag-ui/client@0.0.53pinned.controllers/API/agui/ships the run service (run-agent.ts), the React hook (use-run-flow.ts), the canvas-state reducer (state.ts), the chat reducer (chat.ts), and the bridge (run-flow-bridge.ts) thatflowStore.buildFlowdrives every run through.buildWorkflowRunRequestbuilds the native body.createWorkflowAgent({ body })patchesHttpAgent.requestIniton the instance so the wire body is the native shape instead of the AG-UIRunAgentInput. TheRunAgentInputpassed toagent.run()stays local for the client-side subscriber correlation pipeline and never reaches the network.The bridge folds AG-UI events into the same flow-store methods the v1 path used.
RUN_FINISHEDandRUN_ERRORtear down the subscription on the terminal event so a server-side keepalive after the run doesn't leave the canvas stuck onisBuilding=true.Side-channel CustomEvent (
langflow.event) carries the original v1 message payloads alongside the AG-UI translation, so the playground chat-view keeps consuming its familiar v1 shape. A follow-up rewrites chat-view onto AG-UITEXT_MESSAGE_*directly and retires the side-channel.50 frontend jest tests cover the builder, agent factory, wire-body capture, JSON-Patch state-delta parsing, the terminal-event contract, the e2e bridge through real stores, and
useRunFlow's concurrency lifecycle.CI
CI was green on the last run before merging the latest
release-1.10.0(98 jobs, 0 failures: backend matrix on Python 3.10 + 3.14 across LFX, CLI, integration, and unit groups 1 to 5, plus the 70-shard Playwright matrix). It's re-running now after that merge.Follow-ups
api/v1/chat.py::build_flow+ the v1 build pipelinecustomBuildUtils.tsand theeventDeliveryconfig the canvas no longer consultslangflow.eventside-channel by porting chat-view to AG-UITEXT_MESSAGE_*directlywithEventDeliveryModesat each Playwright call site and delete the shimPOST /api/v2/workflowsIntegrating from TypeScript / JavaScript
Plain
fetchexamples (Node 18+ and browser) for the three execution modes, matching the real v2 wire shapes.As a frontend/Node developer, I want to run any flow through one endpoint with sync, streaming, and background modes, so that I can pick the right execution shape per feature without learning a different API for each. The run should give a deterministic primary answer plus the full per-component map, stream tokens as they happen, let a long job disconnect/re-attach without losing events, read a stop as a cancellation (not a failure), and round-trip
session_idfor chat continuity.Setup
Sync — one-shot answer
Pin a specific output so
output.textis deterministic on multi-output flows:output_ids: ["ChatOutput-final"].Streaming (AG-UI) — live tokens + tool activity
streamreturns SSE.EventSourceonly does GET, so for a POST stream usefetch+ a small SSE reader:Prefer raw v1-shaped frames? Send
stream_protocol: "langflow"and readdata.event/data.data.Background — long job, resumable
Stopping a run
A stopped run's replayed stream ends in
CUSTOM langflow.run.cancelled+RUN_FINISHED(on thelangflowprotocol, acancelledevent), notRUN_ERROR, so thetail()loop treats a deliberate stop as a clean end.Notes
session_idacross calls; it's echoed back on every response (including completed background jobs).tweaks: { "OpenAIModel-x": { "temperature": 0.2 } }overrides component params without editing the flow.syncmode; ignored forstream/background.x-api-key; a sessionAuthorization: Bearer <token>works too.Endpoints
POST/api/v2/workflowsmode:sync/stream/background)GET/api/v2/workflows?job_id={id}GET/api/v2/workflows/{job_id}/eventsLast-Event-IDto resume)POST/api/v2/workflows/stop{ job_id })POST/api/v2/workflows/public