fix(flow-functionality): re-open the flow under test by id, not by name (#1336) - #1339
Merged
Merged
Conversation
…me (#1336) The auto-save-off spec re-opened "the just-created flow" by clicking the first `list-card` whose name contained "New Flow". Langflow names every blank flow "New Flow"/"New Flow (N)", so under `fullyParallel` the list holds one per worker and that filter routinely resolved another worker's flow. Proved on nightly 1.12.0.dev18 by logging the page's own network: the page created `8e767306` and `ee8e0ab9`, and the re-open landed on `164b3c19` — an id it never created. When that flow's real owner ran its id-scoped cleanup, the save PATCH came back 404, so the editor never navigated back to the list and the next re-open burned its 45 s on a card with no reason to exist. That is the 2026-07-22 / 2026-08-06 daily signature, reproduced 3/8 at --workers=4. Selecting the card by id does not fix it either: the flows list is paginated at 12 and ordered by `updated_at DESC`, and under load this test's own card is routinely off page 1 (measured: 12 of 12 slots taken by fresher flows). So the old spec's "success" depended on some other worker's blank flow being on top — which also made the discard assertion (`div-generic-node` count === 0) vacuous whenever it opened a stranger's fresh flow. Re-open by URL instead, waiting for `GET /api/v1/flows/{id}` and the canvas to mount. The id is read AFTER the blank-flow navigation, never before it: the bootstrap reaches the templates modal through "New Flow", which parks the page on a placeholder flow Langflow deletes as soon as the modal navigates elsewhere (#490/#681 from the other side). Each exit now asserts the editor was actually left. That is the attribution this flake lacked: a failed save leaves the editor in place, which surfaced 45 s later as a card-click timeout naming neither the step nor the cause. Verified by forcing the save PATCH to 500 — the failure now lands on the exit. Quarantine lifted: `test.fixme` removed and `@stable` restored.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a recurrent flake in the auto-save-off @stable flow test by ensuring the spec always re-opens the exact flow it created (by flow id) rather than relying on ambiguous, paginated list-card selection by name.
Changes:
- Replaces “reopen by flows-list card filtered by
New Flow” with “reopen by/flow/<id>and wait forGET /api/v1/flows/<id>+ canvas mount”. - Adds
editorFlowIdcross-check andexpectLeftEditorassertions so “wrong flow” and “failed exit” failures surface at the correct step. - Restores
@stableon the spec and updates the spec doc’s validated version and narrative accordingly.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| tests/tests-automations/regression/flow-functionality/auto-save-off.spec.ts | Reopens the flow under test by id (URL) with response-based readiness, adds editor-exit assertions, and restores @stable. |
| docs/flow-functionality/auto-save-off.md | Updates last-validated version and documents the id-based reopen + exit assertions rationale for #1336. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1336. Closes #1336.
Problem
The
@stablemanual-save test flaked on the 2026-07-22 and 2026-08-06 dailies with the same signature —locator.click: Timeout 45000ms exceededre-opening "the just-created flow" through itslist-card-open-button. 45 s on a card the test itself created is not a tight wait, and the product is fine here: the spec was driving another worker's flow.reopenNewFlowclicked the firstlist-cardwhose name contained"New Flow". Langflow names every blank flowNew Flow/New Flow (N), so underfullyParallelthe list holds one per worker. Proved on nightly1.12.0.dev18by logging the page's own network:With the save 404'd the editor never navigates back to the list, so the next re-open waits out its full 45 s on a card that has no reason to exist. That is exactly the CI artifact's state: the failure screenshot is the canvas, not the flows list, with the flow saved and those two 404s in the advisory log. Reproduced 3/8 at
--workers=4 --retries=0.Two further findings shaped the fix:
updated_at DESC(pageSize = 12inhomePage,stmt.order_by(Flow.updated_at.desc())inprojects.py). Under load this test's own card is routinely off page 1 — measured at the failure: 12 of 12 slots taken by fresher flows. So the old spec's "success" depended on some other worker's blank flow being on top, which also made the discard assertion (div-generic-nodecount=== 0) vacuous whenever it opened a stranger's fresh blank flow.awaitBootstrapTestreaches the templates modal through the "New Flow" entry point, which parks the page on a placeholder flow that Langflow deletes as soon as the modal navigates elsewhere (flow-builder-welcome-mount.tsx). Readingpage.url()before the click captures that placeholder — Create agent-tool-name-validation.spec.ts — invalid tool name blocked with clear message #490/Validate & promote flow settings — §12.2 #681 from the other side, and the first draft of this fix hit it.Fix
/flow/<id>), waiting forGET /api/v1/flows/<id>to resolve and the canvas to mount. Immune to pagination, ordering and to workers next door. The wait on the flow GET also stops thediv-generic-nodecounts from reading a canvas that simply had not rendered yet.expectLeftEditor). This is the attribution the flake lacked: a failed save leaves the editor in place, and that surfaced 45 s later as a card-click timeout naming neither the step nor the cause.editorFlowIdcross-checks the URL id against the ids this page created, so "the spec is driving somebody else's flow" fails where it happens instead of six steps later.test.fixmeremoved,@stablerestored.Rejected alternatives: keeping the card click scoped by the card's own id-scoped
flow-name-<uuid>testid (does not survive pagination — the card is absent, not mis-selected); keying on the exact flow name (names are unique per user viaUniqueConstraint("user_id", "name"), but the card is still off page 1); narrowing the list via its search box (adds a debounced server filter as a new failure surface to a spec whose subject is persistence).Scope note
Every assertion in the test is unchanged in kind and strictly stronger in practice — the discard count, the two
title-*visibility checks and the finalcount === 2now run against the flow this test actually saved. The re-open is a full reload rather than an SPA route change, which proves server-side persistence more strictly; the "open a flow from its list card" path stays covered by the specs whose subject it is (bulk-actions,mcp-server).Descriptive, out of scope: under an artificial 8× same-spec burst,
POST /api/v1/flows/andDELETE /api/v1/flows/returned 500 ("An internal error occurred while creating the flow") — probable unique-name race. Never observed in serial execution.Validation (nightly
1.12.0.dev18,--retries=0)npm run typecheck✅ ·npx eslint <spec>✅ (0 errors; 15 pre-existing style warnings)--workers=1 --repeat-each=5, 55 s) ✅--workers=4 --repeat-each=8), twice ✅--trace=on✅ (13 s, trace captured — this spec is not in the hanging family)🚨 Backend Erroron the serial runs ✅GET /api/v1/flows/: 28 before → 28 after a run (zero leak); the pre-existing orphan was purged ✅expect(chatInputNode).toBe(0)→toBe(1)⇒ failed (Expected: 1, Received: 0)expect(nodeCount).toBe(2)→toBe(3)⇒ failed (Expected: 3, Received: 2)PATCH /api/v1/flows/{id}forced to 500 ⇒ failed atexpectLeftEditoron the second exit, not 45 s later on a card clickgrep FFMUT→ 0 hits) + final green run ✅Follow-up filed for the same
hasText: "New Flow"hazard still present inmcp/server/mcp-server.spec.ts(lines 963, 1083).