fix(flow-functionality): enter the flow through openFlowById (#1342) - #1343
Merged
Victor-w-Madeira merged 1 commit intoAug 7, 2026
Merged
Conversation
#1336's fix re-opened the flow under test with a local `page.goto('/flow/{id}')` plus a canvas wait. That works and was validated, but it is the fourth copy of the block `helpers/flows/open-flow-by-id.ts` (#1214) was extracted to stop — added, ironically, by the PR that was fixing a parallel-safety bug of exactly the kind that helper documents. Two guarantees the copy did not have, both of which apply here: - the onboarding overlay cannot appear. Upstream arms the assistant tooltip at canvas mount + 10 s and paints it over the canvas-controls bar, which is what `adjustScreenView` clicks (#1220's measurement). This test calls it four times across a ~15-25 s run. The CI artifact that opened #1336 shows that tooltip painted over the canvas. - the editor is not handed back while POST /api/v1/authz/me/permissions is still in flight — the #1005 window in which a mutation is silently swallowed. This spec adds a component immediately after two of the three re-opens. The seed is called at the top of the test rather than left to the helper's own call: `addInitScript` only applies to loads that follow it, and the first editing phase (two `adjustScreenView` calls plus the on-canvas save) happens before any re-open. One thing deliberately does NOT come from the helper. The wait on the flow's own GET /api/v1/flows/{id} stays, because `openFlowById` returns on `canvas_controls_dropdown` + writability and neither implies the graph has been applied — and the discard assertion (`div-generic-node` count === 0) is the one check that passes VACUOUSLY on a canvas that has not painted its nodes yet.
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Updates the “auto-save off” regression coverage to use the shared openFlowById navigation helper and proactively seed the assistant onboarding flag to prevent UI interference/flakiness during repeated re-opens.
Changes:
- Replace direct
page.goto('/flow/{id}')+ selector wait withopenFlowById, while preserving an explicit wait forGET /api/v1/flows/{id}to ensure the graph is applied before node-count assertions. - Seed assistant onboarding state at the start of the test to avoid late-appearing overlays blocking
adjustScreenViewinteractions. - Refresh the accompanying doc to reflect the new helper-based re-open flow and the rationale for the added gating.
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 | Uses openFlowById for consistent re-open behavior and seeds onboarding early to reduce UI flake. |
| docs/flow-functionality/auto-save-off.md | Documents the helper-based re-open path and the onboarding seeding rationale. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Victor-w-Madeira
deleted the
fix/issue-1342-auto-save-off-use-open-flow-by-id
branch
August 7, 2026 00:03
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 #1342. Closes #1342.
Problem
#1336's fix re-opened the flow under test with a local
page.goto('/flow/{id}')plus a canvas wait. It works and it is validated — but it is the fourth copy of the blockhelpers/flows/open-flow-by-id.ts(#1214) was extracted to stop, added by the very PR that was fixing a parallel-safety bug of the kind that helper documents. Self-reported debt, not a review catch.Two guarantees the copy did not have, both live for this spec:
openFlowByIdseeds the discovery flag before the document load. Upstream arms the assistant tooltip at canvas mount + 10 s and paints it over the canvas-controls bar — which is exactly whatadjustScreenViewclicks (Measure whether the two remaining dismissOnboardingIfPresent probes actually catch the tooltip #1220's measurement, not an inference). This test callsadjustScreenViewfour times across a ~15–25 s run, and the CI artifact that opened [Daily #1330] auto-save-off — reopening the just-created flow's card intermittently times out at 45 s (recurrent 2×) #1336 shows that tooltip painted over the canvas.openFlowByIdrefuses to hand back an editor whilePOST /api/v1/authz/me/permissionsis still in flight — the renameFlow residual flake: read-only save button and missing flow_name header (~7%, not the #995 clobber) #1005 window in which a mutation is silently swallowed. This spec adds a component immediately after two of the three re-opens.Fix
Both re-opens now enter through
openFlowById. Two deliberate details:addInitScriptapplies only to loads that follow it, and the first editing phase — twoadjustScreenViewcalls plus the on-canvas save click — happens before any re-open, inside the mount + 10 s window.GET /api/v1/flows/{id}stays. This is the part the issue flagged as needing care and it is load-bearing:openFlowByIdreturns oncanvas_controls_dropdown+ writability, and neither implies the graph has been applied. The discard assertion (div-generic-nodecount=== 0) is the one check that passes vacuously on a canvas that has not painted its nodes yet, so the count is ordered after that GET. The response promise is registered before the helper's navigation and awaited after it.Scope note
No assertion changed; no change to the exit path (
expectLeftEditoris untouched —leaveFlowEditoris not applicable here, it is built for autosave mode where the blocker dialog renders no buttons, whereas this spec must deliberately click "Exit Anyway" / "Save And Exit", which is the behaviour under test).Validation (nightly
1.12.0.dev18,--retries=0)npm run typecheck✅ ·npx eslint <spec>✅ (0 errors)🚨 Backend Error✅--workers=4 --repeat-each=8— the burst that produced 3/8 failures before [Daily #1330] auto-save-off — reopening the just-created flow's card intermittently times out at 45 s (recurrent 2×) #1336's fix ✅New Floworphans ✅expect(chatInputNode).toBe(0)→toBe(1)⇒ failed (Expected: 1, Received: 0) — the discard assertion the issue asked to re-prove is still falsifiable through the new entryexpect(nodeCount).toBe(2)→toBe(3)⇒ failed (Expected: 3, Received: 2)canvas_controls_dropdownassertion (element(s) not found), with the 404 loggedgrep→ 0 hits) + final green run ✅Stated rather than claimed: the placement of
seedAssistantDiscoveredat the top of the test is not independently falsifiable by a test — the tooltip fires at mount + 10 s and only sometimes intercepts, so no deterministic mutation proves it. It rests on #1220's measurement (a 282×32 px opaque element atz-index: 40,pointer-events: auto, over the canvas-controls bar) plus the fact that this spec is still clicking that bar past the 10 s mark.