fix(mcp): delete the flow and the project this spec leaks every run (#1376) - #1379
Merged
Victor-w-Madeira merged 2 commits intoAug 8, 2026
Merged
Conversation
…1376) Measured on 1.12.0.dev20 over three consecutive runs of this file, counting GET /api/v1/flows/?get_all=true and GET /api/v1/projects/ around each: flows 34 -> 35 -> 36 -> 37 (+1 every run, unbounded) projects 1 -> 2 -> 2 -> 2 (+1, then flat) The two halves fail differently and the second is the one worth naming. Test 2 opens the Basic Prompting template, which creates a flow nothing deletes — that leak is monotonic and plainly visible. Test 1 creates TWO projects and only ever deletes the one it renames, and from run 2 on that leak reads as zero only because cleanOldFolders at the top of test 1 sweeps the PREVIOUS run's leftover. It is not absent, it is absorbed — and #1363 is precisely the incident where the absorption stopped working: with the sweep silently deleting nothing, New Project (N) accumulated inside a single test's own retries. The file had no afterEach at all. It now tracks what it creates and deletes exactly that, id-scoped, never a global sweep (#515): flows through a response listener, because the leaked one is a side effect of opening a starter template rather than an explicit step, and both project ids from createProjectThroughSidebar — including the one test 1 already deleted through the UI, where deleteProject treats the 404 as the desired end state. cleanOldFolders stays. It guards against OTHER runs' leftovers; this stops the file from producing them. After: 4 consecutive runs, 0 delta on both counts, 2 passed each.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses a resource-leak in the MCP starter-projects E2E spec by adding id-scoped teardown that deletes flows and projects created during the tests, avoiding reliance on cross-run/global cleanup.
Changes:
- Added flow tracking via a
page.on("response")listener to capture flow ids created as side-effects (e.g., opening starter templates). - Added
afterEachteardown that navigates away from the editor and deletes captured flow/project ids via API calls. - Captured and tracked both project ids created in test 1 so neither is left behind.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+96
to
+118
| function trackCreatedFlows(page: Page): void { | ||
| page.on("response", (response) => { | ||
| if (response.request().method() !== "POST" || response.status() !== 201) { | ||
| return; | ||
| } | ||
| if (new URL(response.url()).pathname.replace(/\/$/, "") !== "/api/v1/flows") { | ||
| return; | ||
| } | ||
| response | ||
| .json() | ||
| .then((body: { id?: string }) => { | ||
| if (body?.id) createdFlowIds.add(body.id); | ||
| }) | ||
| .catch(() => {}); | ||
| }); | ||
| } | ||
|
|
||
| test.afterEach(async ({ page, request }) => { | ||
| const flowIds = [...createdFlowIds]; | ||
| const projectIds = [...createdProjectIds]; | ||
| createdFlowIds.clear(); | ||
| createdProjectIds.clear(); | ||
| if (flowIds.length === 0 && projectIds.length === 0) return; |
| // `about:blank` rather than `/` so teardown adds no backend traffic of its own. | ||
| await page.goto("about:blank").catch(() => {}); | ||
|
|
||
| const headers = { Authorization: await getAuthToken(request) }; |
Both points from the review are real, and the shared helper #1108 built (tests/helpers/flows/track-created-flows.ts) already answers both — so the fix is to use it, not to patch the local copy. 1. Race. The listener reads the creation body asynchronously, so the id lands a tick after the 201, while afterEach snapshotted and cleared the Set immediately — an id resolving late was dropped and the flow leaked anyway. The tracker holds its pending reads and `cleanup` awaits `settle()` before snapshotting. #1108's own header measures this: of the 51 hand-copies, ONE settles. This was about to be the 52nd copy, on the wrong side of that count. 2. getAuthToken throws once its retry budget is spent (a backend wedged at teardown, #1077), and an unguarded await in an afterEach makes that a HOOK error — failing an otherwise-green test, which is exactly what the comment two lines above it promised would not happen. The tracker catches it, names it, and carries on unauthenticated rather than degrading silently to an empty token (#1086). The project half, which has no shared equivalent, now does the same thing explicitly. Also gained by not hand-rolling: `unmountEditorForCleanup` (#1288) instead of a bare goto whose synchronous throw would skip the deletes, dedup at capture, and failed creations recorded (#1114). The tracker unmounts only when it has flows to delete, and test 1 has none while still holding projects, so the navigation is repeated before the project deletes — deleting a project under a mounted home view makes it refetch a folder that is already gone (#1023). Re-validated: 3 runs, 0 delta on flows and projects, 2 passed each. Force-fail re-done against the new wiring (FF-H, cleanup call and project tracking removed): the leak returns, 26 -> 29 -> 30 flows. typecheck, lint, 552 unit tests green.
Victor-w-Madeira
deleted the
fix/issue-1376-mcp-starter-projects-cleanup
branch
August 8, 2026 07:20
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 #1376.
Problem
Found while working #1363 and deliberately kept out of that PR — a cleanup defect, not an addressing one.
mcp-server-starter-projects.spec.tshad noafterEachat all. Measured on1.12.0.dev20over three consecutive runs of the file, countingGET /api/v1/flows/?get_all=trueandGET /api/v1/projects/around each:Every sibling in
core-functionality/project-management/measured0in the same sweep, so this file is the outlier, not the norm.POST /api/v1/flows/creates, and nothing deletes it. Monotonic and plainly visible.renamed_project.Why the project half reads as zero and is not
From run 2 onwards the project count is flat, which looks like "no leak". It is not:
cleanOldFolders(page)at the top of test 1 sweeps the previous run's leftover. The leak is not absent, it is absorbed — and #1363 is precisely the incident where the absorption stopped working. When upstream re-keyed the sidebar testid, that sweep silently deleted nothing andNew Project (N)accumulated inside a single test's own retries.So the blast radius of this leak is "whatever
cleanOldFoldershappens to still be able to do", which is not a property a spec should depend on.Fix
The file now tracks what it creates and deletes exactly that, id-scoped — never a global sweep, which wipes what other workers are building (#515).
awaitBootstrapTest's empty-instance branch can seed one too (project-management: residual 404s after the destructive lane — an editor polling a deleted flow, and a folder refetched after its own deletion #1023).createProjectThroughSidebar, including the one test 1 already deleted through the UI —deleteProjecttreats that404as the desired end state and retries the500the endpoint answers under contention ([Daily #962] api-folders DELETE returns 500 instead of 204 (recurrent) #965).about:blankbefore deleting: test 2 ends inside the flow editor, and deleting a flow underneath a mounted editor makes it keep polling one that no longer exists — 404s the fixture logs as🚨 Backend Error, an artifact of teardown order rather than a defect (project-management: residual 404s after the destructive lane — an editor polling a deleted flow, and a folder refetched after its own deletion #1023).cleanOldFoldersstays. It guards against other runs' leftovers; this stops the file from producing them. The two are not redundant, and the comment in the file says so explicitly so the next reader does not delete one of them.Scope note
No assertion changed. Both tests' bodies are untouched apart from the two
trackCreatedFlows(page)calls and capturing the second project's id. What this PR adds is teardown.Validation (nightly
1.12.0.dev20,--workers=1 --retries=0)npm run typecheck✅ ·npm run lint✅ (0 errors) ·npm run test:units✅ 531/5312 passedeach (~18 s), zero orphan warnings. Re-verified after rebasing onto mergedmain: 2 more runs, same result.🚨 Backend Errorintroduced. The two advisory entries that appear are pre-existing and belong to test 2, which this PR does not touch: the409 Server already exists.is that test's own assertion, and the intermittent500on the bulkDELETE /api/v1/flows/issqlite3.OperationalError: database is lockedin the cascade delete — the filed [Daily #962] api-folders DELETE returns 500 instead of 204 (recurrent) #965/LE-2020 contention class, read from the container traceback. Neither is issued by this hook, which deletes per-id throughrequest, not through the page.Force-fail — executed
37 → 38then38 → 40flows, projects1 → 2and flat againA behavioural force-fail rather than an assertion one, because the defect is invisible to every assertion in the file — which is the whole reason it survived this long. Revert proven:
grep FF_MUTATIONovertests/returns 0, followed by 4 green runs at 0 delta.Dependencies
None — pure UI plus REST. No LLM, no provider key.
🤖 Generated with Claude Code