fix(mcp): repair the swallowed sidebar add behind the MCP-server flake (#1335) - #1344
Conversation
`addComponentFromSidebar` could only be used by a tab that has a search box, because it unconditionally fills `sidebar-search-input` before clicking. The MCP tab (`sidebar-nav-mcp`) has no such box — its entries are added straight from the list by testid — so every MCP spec clicked bare and inherited none of the swallowed-click repair. That is what #1335 turned out to be. Measured on nightly 1.12.0.dev17 with an instrumented probe: 4 of 8 first clicks on the MCP tab produced no node within 12 s, and all 4 were repaired by an identical second click — the same class as #1304's 4/20 on the Components tab, and worse here. A landed add rendered its node in 91-108 ms, so the budget is only ever paid by a genuine drop. Split into `addComponentFromSidebarWithoutSearch` rather than made an optional third argument: 27 call sites pass the search term positionally, and the two tabs differ in the post-failure evidence there is to report, not merely in whether one line runs. `searchTerm`/`searchValue` become nullable so the message can say "<none on this tab>" — `sidebar search input: ""` is a real observation on the Components tab (the input was reset) and must not read the same as "there is no input". `addComponentFromSidebar` itself is unchanged behaviourally: it delegates to the same `addWithRepair` with a non-null term, so none of the 27 callers move. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…1335) `openAddMcpServerModal` decided its branch from a snap read: `isVisible({ timeout: 1000 })` looks like a 1 s wait, but Playwright IGNORES that option (`@deprecated This option is ignored`), so the probe returned in ~1 ms and committed to the dropdown branch whenever the widget had not painted yet — then spent its whole 3 s budget on a locator that, in the no-servers case, never appears at all. The component renders exactly ONE of two mutually exclusive entry points (verified in the 1.12.0.dev17 bundle, a single ternary): `add-mcp-server-simple-button` once the servers query resolves to an empty list, and `mcp-server-dropdown` otherwise — including while that query is in flight, where it renders DISABLED with a "Loading servers…" label. Waiting for either first and branching on the settled state removes both halves of that, and the disabled state is asserted separately so an unclearing load reports itself as "still disabled" instead of a bare click timeout. The attribution matters as much as the wait. The old 3 s `click()` produced a call log of one line — `waiting for getByTestId('mcp-server-dropdown')` — which reads as "the dropdown is late" and led #1335 to propose raising the budget to 15-30 s. The failing attempt's own error-context snapshot showed an empty `application "Flow canvas"` with "Minimize all" disabled: there was no MCP component node, so neither entry point could ever render and no budget would have helped. The failure now reports the canvas node count first and splits on it — 0 nodes points at the swallowed add and says a wait cannot fix it; a populated canvas points at a Langflow change to the component's server field. `.or()` without `.first()`: the branches are mutually exclusive, so the #599 caveat cannot apply and a build rendering both would fail loudly as a strict-mode violation. The 15 s budget is generous relative to its cost — the entry point was visible 6-15 ms after the node landed in 8 of 8 runs, so it is only ever paid on the way to a failure. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Applies the two helper fixes at the four call sites (one in `mcp-server-tab`, three in `mcp-server`) and lifts the quarantine `938c547` installed: `test.fixme` back to `test`, `@stable` restored. Validated on nightly 1.12.0.dev18, all `--workers=1 --retries=0`: mcp-server-tab.spec.ts --repeat-each=5 5 passed (1.8m) mcp-server.spec.ts full file 5 passed, 1 skipped, 1 failed Neither the skip nor the failure is this change's. The skip is "change mode of MCP tools", quarantined for #1266. The failure is "Streamable HTTP MCP server with server-everything" — not `@stable` (auto-removed in `cb3082d`), already filed as #1332, and its body is byte-identical here and on `origin/main`: this commit's only edits to the file are one import and three call swaps, none inside that test. It fails on both sides, but **not at the same point**, and the difference is recorded rather than smoothed over. Measured back-to-back against the same backend, `--grep server-everything`, three attempts per side: origin/main 3/3 fail at the `add-mcp-server-button` visibility check (`element(s) not found` after 15 s) — #1332's exact signature this branch 3/3 fail later, at the `toolsCount` poll (null after 60 s) Both signatures map to already-open issues: the first to #1332, the second to the `?action_count=true` slowness behind #1266 — that endpoint answers in **7.3 s for two servers** on this instance, so a 60 s poll does not close once the test adds a third that points back at Langflow itself. What is NOT established is why the failure point splits so cleanly by side when the test body does not differ; the likeliest reason is that line 1207 carries the same `isVisible({ timeout })` ignored-option race this commit's helper fixes elsewhere, making the outcome genuinely state-dependent. Left as an observation for #1332, not a claim. `mcp-server-tab` also gains the cleanup it never had. The claim its spec doc carried — "no flows are left behind by design" — was false: the test creates TWO flows per run and registered a fresh `test_server_<random>` every time, deleting neither. 14 orphan `test_server_*` registrations had accumulated on the local nightly. Those are not merely litter: their count decides which branch the widget under test renders (an empty list shows `add-mcp-server-simple-button`, a populated one `mcp-server-dropdown`), so leaving them behind quietly stopped the spec from ever taking the empty-list path again. And since `?action_count=true` scales with that count, they were also feeding #1266. After the fix, 5 consecutive runs left 0 orphans (measured). Cleanup is id-scoped, never a wipe (#553). Flow ids come from `POST /api/v1/flows` 201 bodies, not from the canvas URL, which still holds the stale bootstrap id (#681). The server name is tracked BEFORE the registration request, so a run that dies mid-add still removes a server the backend did create; servers are listed first so a name that never registered does not produce a 404 delete, and a failed server delete warns rather than throws so the flow cleanup below still runs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Force-fail outputThe add was removed from the spec on purpose so the canvas stayed empty. On nightly 1.12.0.dev18, Compare with what the same state produced before this PR — the entire call log: That one line is why #1335 was filed as a 3 s budget too short and proposed raising it to 15–30 s. The budget was never the cause. Environment after validationMeasured against the API after 8 runs across this branch and
Note for #1332
|
Fixing the MCP-tab click left `mcp-server-tab.spec.ts` at 4 of 5, and the one failure was not the #1335 signature at all. It landed 30 s later on `waitForSelector: generic-node-title-arrangement` — the node created by the API Request **drag** at the top of the test, which Langflow had swallowed. That is the same mis-attribution #1335 was filed under (a wait blamed for a gesture that never registered), one surface over, and the click repair does not reach it. Measured on nightly 1.12.0.dev18: bare drag + repaired click 4 / 5 (the 1 failure is the drag) repaired drag + repaired click 10 / 10 The gesture is RE-ISSUED, not swapped for a click. Dragging a component out of the sidebar is an interaction Langflow ships; a spec that quietly stops exercising it stops covering it. The comment this replaces — "use dragTo which is more reliable than click on add-component-button" — predates the #1304 repair and its premise no longer holds: neither gesture is reliable bare, both are reliable repaired. `swallowedAddMessage` gains a `gesture` field rather than a second message function, because everything else it reports is identical. It defaults to "click", so the 34 existing call sites are byte-identical (pinned by a unit assertion — a drifting default would have all of them reporting a drag they never performed). Three things do differ and are branched: the trigger names `dragTo() ... onto the canvas`; the evidence quotes **1/5 on dev18** rather than #1304's 4/20, which was measured on the click surface and would be a number attributed to a surface it was never taken on; and there is no "+" button to report visibility for, so it reads "sidebar entry still visible". Also corrects two stale counts in the same file's docblocks: the helper is reached by 34 call sites across 27 spec files, not the 23 recorded when it was written. Verified: 10 passed (6.6m), --repeat-each=10 --workers=1 --retries=0, 0 orphan flows and 0 orphan MCP servers left behind. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Update: a second sample found a second surfaceThe click fix measured 5/5 here. An independent Line 113 is the wait; the gesture that failed is line 109-111, the Now repaired by
Two things worth noting for review. The interval alone does not separate 9/10 from 10/10 — what carries the claim is the measured cause (the drag was observed swallowed, and re-issuing it once repairs it, same as #1304's click). And the gesture is re-issued rather than swapped for a click, because dragging out of the sidebar is an interaction Langflow ships and a spec that quietly stops exercising it stops covering it. Orphans after those 10 runs: 0 |
The impacted-specs lane, run locally (GitHub Actions is in a
|
Closes #1335.
Verdict: test-side, but not the one the issue proposed
#1335 read the failure as a wait-strategy problem —
mcp-server-dropdownnot clickable within the shared helper's 3 s budget — and its investigation directive proposed raising that budget to 15–30 s. The budget was never the cause, and raising it would have fixed nothing.The failing attempt's own
error-context.mdshows an emptyapplication "Flow canvas"with "Minimize all" disabled: there was no MCP component on the canvas at all. Both of the add-server widget's entry points hang off that node, so with no node there is nothing to wait for and no budget can help. The sidebar add had been swallowed — the #1304 class.Measured on nightly 1.12.0.dev17 with an instrumented probe:
The same class as #1304's 4/20 on the Components tab, and worse here. The product is not exonerated by this PR — the swallowed click is a real Langflow defect (#1304) and this only makes the suite survive it and name it.
What changed
1.
addComponentFromSidebarWithoutSearch— the repair helper could only serve tabs that have a search box, because it unconditionally fillssidebar-search-input. The MCP tab has none, so every MCP spec clicked bare and inherited no repair. Split into its own export rather than an optional third argument: 34 call sites across 27 files pass the term positionally, and the two tabs differ in the post-failure evidence there is to report, not merely in whether one line runs.searchTerm/searchValuebecome nullable so the message can distinguish<none on this tab>from""(a real observation on the Components tab — the input was reset).addComponentFromSidebaris behaviourally unchanged; it delegates to the sameaddWithRepairwith a non-null term.2.
openAddMcpServerModal— decided its branch from a snap read.isVisible({ timeout: 1000 })looks like a 1 s wait, but Playwright ignores that option, so the probe returned in ~1 ms and committed to the dropdown branch whenever the widget had not painted — then spent its whole 3 s budget on a locator that, in the no-servers case, never appears at all. It now waits for either entry point (.or(), no.first()— the branches are mutually exclusive, so a build rendering both fails loudly as a strict-mode violation) and branches on the settled state. The disabled "Loading servers…" state is asserted separately, so an unclearing load says "still disabled" instead of producing a bare click timeout.3. The failure now names the cause. The old 3 s
click()produced a call log of exactly one line —waiting for getByTestId('mcp-server-dropdown')— which reads as "the dropdown is late" and is what sent the issue after the budget. The new failure reports the canvas node count first and splits on it: 0 nodes points at the swallowed add and says a wait cannot fix it; a populated canvas points at a Langflow change to the component's server field. Covered by unit tests on both branches.4. Cleanup that never existed. The spec doc claimed "no flows are left behind by design". False: the test creates two flows per run and registered a fresh
test_server_<random>every time, deleting neither — 14 orphantest_server_*registrations had accumulated locally. Not merely litter: their count decides which branch the widget under test renders, so leaving them behind quietly stopped the spec from ever taking the empty-list path again. It also feeds #1266 —GET /api/v2/mcp/servers?action_count=truetakes 7.3 s for two servers on this instance. Cleanup is id-scoped, never a wipe (#553); flow ids come fromPOST /api/v1/flows201 bodies, not the canvas URL, which holds the stale bootstrap id (#681).5. Quarantine lifted —
test.fixme→test,@stablerestored, per938c547's stated deliverable.Validation
Nightly 1.12.0.dev18, all
--workers=1 --retries=0:Orphans after 10 consecutive runs: 0
test_server_*and 0New Flow*(measured against the API).The click fix alone was not enough, and the second sample is why
Fixing the MCP-tab click measured 5/5 here. An independent
--repeat-each=5on the same branch came back 4/5 — and the one failure was not the #1335 signature. It landed 30 s later onwaitForSelector: generic-node-title-arrangement: the API Request drag at the top of the test had been swallowed. Same product defect (#1304), a surface the click repair does not reach, and the same mis-attribution #1335 was filed under — a wait blamed for a gesture that never registered.dragComponentFromSidebarre-issues the drag once. The gesture is re-issued, not swapped for a click: dragging out of the sidebar is an interaction Langflow ships, and a spec that quietly stops exercising it stops covering it. The comment it replaces — "use dragTo which is more reliable than click on add-component-button" — predates the #1304 repair; neither gesture is reliable bare, both are reliable repaired.swallowedAddMessagegained agesturefield rather than a second function. It defaults to"click", so the 34 existing call sites are byte-identical — pinned by a unit assertion, since a drifting default would have all of them reporting a drag they never performed. What does differ is branched: the trigger namesdragTo() … onto the canvas; the evidence quotes 1/5 on dev18 rather than #1304's 4/20, which was measured on the click surface; and there is no"+"button, so it reads "sidebar entry still visible".Force-fail — the add was removed on purpose so the canvas stayed empty; the failure is the attributed message naming the node count and pointing at the repair helper, not a
mcp-server-dropdowntimeout. (See the run output quoted in the PR discussion.)The one failure is not this change's
Streamable HTTP MCP server with server-everything— not@stable(auto-removed incb3082d), already filed as #1332, and its body is byte-identical here and onorigin/main: this PR's only edits to that file are one import and three call swaps, none inside that test.It fails on both sides, but not at the same point, measured back-to-back against the same backend,
--grep server-everything, three attempts per side:origin/mainadd-mcp-server-buttonvisibility —element(s) not foundafter 15 s (#1332's exact signature)toolsCountpoll — null after 60 s (the?action_count=trueslowness behind #1266)Both signatures map to already-open issues. What is not established is why the point splits so cleanly by side when the test body does not differ; the likeliest reason is that its line 1207 carries the same ignored-
isVisible({ timeout })race this PR fixes elsewhere, making the outcome state-dependent. Recorded as an observation for #1332 rather than a claim.Reach of the shared helpers (#1335's last deliverable)
helpers/mcp/open-add-mcp-server-modal.ts→ exactly 2 specs:mcp-server-tab.spec.ts,mcp-server.spec.ts. Both validated above.helpers/flows/add-component-from-sidebar.ts→ 34 call sites across 27 files, none of which move: the change is additive andaddComponentFromSidebar's behaviour is identical for a non-null term (pinned by the unit lane).🤖 Generated with Claude Code