Merge pull request #1341 from oriontech-me/fix/issue-1340-mcp-server-reopen-by-id

fix(mcp): re-open the flow under test by id in mcp-server (#1340)

Author: Victor-w-Madeira

docs/mcp/server/mcp-server.md

@@ -121,10 +121,15 @@ round-tripped; then deletes it.
 4. Settings → MCP Servers → Edit: assert `command` is `npx` and `args[0]` is the
    sequential-thinking package, then **edit `args[0]`** to
    `@modelcontextprotocol/server-everything` (server **B**) and save.
-5. Return to the flow, re-select the server on the node, and assert the tool
-   list now exposes `echo-0-option` — the refresh, not the cached A list.
-6. Delete the server; assert it is gone; re-register it as **A** again and assert
-   the node's tool list is back to `sequentialthinking-0-option`.
+5. Return to the flow **by id** (`openFlowById`), re-select the server on the
+   node, and assert the tool list now exposes `echo-0-option` — the refresh, not
+   the cached A list.
+6. Delete the server; assert it is gone; re-register it as **A** again, return to
+   the flow by id, and assert the node's tool list is back to
+   `sequentialthinking-0-option`.
+
+Both re-opens address the flow by **id**, never by the card whose name contains
+"New Flow" (#1340) — see the note below.
 
 ### 6 — `Streamable HTTP MCP server with server-everything should load tools correctly`
 
@@ -248,6 +253,35 @@ Unchanged by #1091 (no stdio surface). Derives the project's own
 
 ## Notes *(optional)*
 
+- **#1340 — test 5 re-opened a flow by NAME, and it opened the wrong one.** Both
+  re-opens clicked the first `list-card` whose name contained "New Flow".
+  Langflow names every blank flow "New Flow"/"New Flow (N)", so under
+  `fullyParallel` the shared project holds one per worker and `.first()` resolves
+  whichever the list puts first. Measured on nightly `1.12.0.dev18`: in isolation
+  the test's own flow ranks first and the click is correct (which is why this
+  never appeared in the daily history — no recorded failure on this test), but
+  seeding **one** competing `New Flow …` in the same project before the list
+  fetch is enough to flip it — the rendered order became
+  `["New Flow probeB-…", "New Flow (1)", "Basic Prompting"]`, the click opened
+  the competitor, and the test then died on the `text="MCP Tools"` wait at 30 s,
+  blaming the node for a flow it was never in. The same locator, in
+  `auto-save-off.spec.ts`, cost two dailies before it was diagnosed (#1336). Both
+  re-opens now use `openFlowById` (#1214), the repo's by-id entry, which also
+  seeds the assistant-onboarding flag and gates on the flow being writable —
+  neither of which the card click did (#1005). The flow id is read AFTER the
+  blank-flow navigation, never before it: the bootstrap parks the page on a
+  placeholder flow Langflow deletes as soon as the modal navigates elsewhere
+  (#490/#681).
+- **Pre-existing flake, NOT introduced by #1340: `openAddMcpServerModal`.** This
+  test fails roughly 1 run in 3 locally at
+  `helpers/mcp/open-add-mcp-server-modal.ts:10` (`mcp-server-dropdown`,
+  `locator.click: Timeout 3000ms exceeded`) — the #1335 signature, in a second
+  file. Confirmed by a control run of the unmodified spec: same 2/3, same step.
+  Raising that budget to 30 s locally did not help under `--workers=2+`, where
+  the dropdown simply never becomes clickable; a 4-worker burst of this spec
+  fails 3/4 there, always before the re-open. That budget belongs to #1335 and is
+  deliberately untouched here — it is a shared MCP helper with other callers.
+
 - **Why `npx` and not `uvx` for the servers that must really start.** Before
   #1091 tests 1/2/5 registered `uvx mcp-server-fetch` / `mcp-server-time`.
   Splitting those into `command` + `args` gets past the new validation but the

tests/tests-automations/regression/mcp/server/mcp-server.spec.ts

@@ -6,6 +6,7 @@ import { openAddMcpServerModal } from "../../../../helpers/mcp/open-add-mcp-serv
 import { zoomOut } from "../../../../helpers/ui/zoom-out";
 import { getAuthToken } from "../../../../helpers/auth/get-auth-token";
 import { deleteFlow } from "../../../../helpers/flows/delete-flow";
+import { openFlowById } from "../../../../helpers/flows/open-flow-by-id";
 
 /**
  * Add-MCP-Server modal: stdio / HTTP registration, field persistence and tool
@@ -790,7 +791,48 @@ test(
     await page.waitForSelector('[data-testid="blank-flow"]', {
       timeout: 30000,
     });
+    // The flow under test has to be addressed by id from here on (#1340), and
+    // the id has to satisfy BOTH sources — neither alone is enough here:
+    //
+    //  - `page.url()` alone is the documented trap. `awaitBootstrapTest` reaches
+    //    the templates modal through "New Flow", so before the blank-flow
+    //    navigation the URL still carries the bootstrap PLACEHOLDER — the flow
+    //    Langflow deletes the moment the modal navigates elsewhere, and the one
+    //    authoring-conventions Pattern A warns about (#681/#505).
+    //  - the tracked `POST /flows` 201 ids alone do not say which flow the editor
+    //    ended up on: this page creates the placeholder AND the blank flow, so
+    //    picking one means trusting arrival order of two async body reads, and
+    //    the wrong pick is precisely the id that gets deleted.
+    //
+    // So: poll until the editor's URL carries an id this page is known to have
+    // created and that is not the placeholder. A transient or client-only id
+    // cannot satisfy the membership test, and a blank-flow click that never
+    // navigates fails HERE, naming the cause, instead of surfacing later as an
+    // unattributed timeout. Measured on nightly 1.12.0.dev18: the click issues
+    // its own `POST /flows` 201 and the URL changes every time (5/5) — the
+    // placeholder is never reused — so this is about attribution, not a defect.
+    const placeholderId = new URL(page.url()).pathname.match(
+      /\/flow\/([0-9a-f-]{36})/,
+    )?.[1];
     await page.getByTestId("blank-flow").click();
+    const editorFlowId = () =>
+      new URL(page.url()).pathname.match(/\/flow\/([0-9a-f-]{36})/)?.[1];
+    await expect
+      .poll(
+        () => {
+          const id = editorFlowId();
+          return !!id && id !== placeholderId && createdFlowIds.includes(id);
+        },
+        {
+          timeout: 30000,
+          message:
+            "the blank-flow click never landed the editor on a newly created " +
+            "flow: the URL still holds the bootstrap placeholder, or its id is " +
+            "not among this page's POST /api/v1/flows 201 responses",
+        },
+      )
+      .toBe(true);
+    const flowUnderTest = editorFlowId()!;
     await page.getByTestId("sidebar-nav-mcp").click();
     await page.waitForSelector(
       '[data-testid="add-component-button-lf-starter_project"]',
@@ -955,17 +997,16 @@ test(
 
     await awaitBootstrapTest(page, { skipModal: true });
 
-    // The /flows a11y refactor (Langflow #13891) makes `flow-name-div`
-    // `pointer-events-none`; open the flow via the card's overlay button.
-    const flowOpenButton = page
-      .getByTestId("list-card")
-      .filter({
-        has: page.getByTestId("flow-name-div").filter({ hasText: "New Flow" }),
-      })
-      .getByTestId("list-card-open-button")
-      .first();
-    await flowOpenButton.waitFor({ state: "visible", timeout: 10000 });
-    await flowOpenButton.click();
+    // By id, never a name-filtered `list-card` + `.first()` (#1340). Langflow
+    // names every blank flow "New Flow"/"New Flow (N)", so under `fullyParallel`
+    // that filter resolves whichever card the shared project's list puts first.
+    // Measured on nightly 1.12.0.dev18: seeding ONE competing "New Flow …" in
+    // this project before the list fetch is enough — the click opened the
+    // competitor, and the test then died on the `text="MCP Tools"` wait below,
+    // blaming the node for a flow it was never in. `openFlowById` also seeds the
+    // assistant-onboarding flag and gates on the flow being writable, which the
+    // card click never did (#1214/#1005).
+    await openFlowById(page, flowUnderTest);
 
     // Wait for the MCP Tools component to be visible on canvas
     await page.waitForSelector('text="MCP Tools"', {
@@ -1076,16 +1117,8 @@ test(
 
     await awaitBootstrapTest(page, { skipModal: true });
 
-    // See note above: open the flow via the card's overlay button.
-    const flowOpenButton2 = page
-      .getByTestId("list-card")
-      .filter({
-        has: page.getByTestId("flow-name-div").filter({ hasText: "New Flow" }),
-      })
-      .getByTestId("list-card-open-button")
-      .first();
-    await flowOpenButton2.waitFor({ state: "visible", timeout: 10000 });
-    await flowOpenButton2.click();
+    // See note above: by id, not by name.
+    await openFlowById(page, flowUnderTest);
 
     // Wait for the MCP Tools component to be visible on canvas
     await page.waitForSelector('text="MCP Tools"', {