Skip to content

Fix MCP session teardown races behind two flaky cloud e2e tests - #2060

Draft
RhysSullivan wants to merge 2 commits into
mainfrom
fix/mcp-session-teardown-races
Draft

RhysSullivan wants to merge 2 commits into
mainfrom
fix/mcp-session-teardown-races

Conversation

@RhysSullivan

@RhysSullivan RhysSullivan commented Sep 18, 2026

Copy link
Copy Markdown
Collaborator

Two cloud e2e tests failed intermittently on main with no related code change. Both trace to the session Durable Object's teardown and restore paths, and one to a test-timing assumption.

mcp-destroyed-session-envelope — in-flight requests answered with a Vite 500 (no such table: cf_agents_mcp_servers). The agents SDK's destroy() drops its tables and deleteAll()s storage (removing the destroy-pending marker) before it aborts the isolate from a setTimeout(0). An owner check landing in that gap saw no marker, still held the cached session meta, and tried to restore the runtime, which starts by reading the dropped table. The DO now records destroying before any await in destroy() and answers terminated from it, same as the durable marker.

mcp-session-cap-eviction — timed out at 180s. Two causes:

  • Test: the scenario only proves anything if the sessions it opens are still resident when the cap is crossed, and the e2e idle timeout is 3s. Opening 34 sessions one at a time did not fit inside that on CI, and neither did opening four at a time (3.5s on a loaded runner — this PR's first CI run). The scenario no longer depends on open throughput. It opens the first cap sessions in waves, and after every wave touches every session opened so far with a JSON-RPC ping, which re-arms the idle alarm through the owner check every request goes through; a session is never left untouched for longer than one open+touch tick (measured and printed next to the window). By the cap arithmetic, admission cap + 1 is the first that can evict one of these sessions whatever else is resident (the comment in the test derives it), so the remaining 10 are then opened untouched at full width and the first of them forces the eviction. Opens and touches never overlap and every wave is at most MAX_CONCURRENT_BUILDS wide (now exported from session-build-semaphore.ts and imported by the test), as are the cleanup DELETEs (which restore a disposed runtime before destroying it), so nothing the scenario does ever waits in that queue. The elapsed-time check is a diagnostic line now, not an assertion.
  • Product: the 3-minute cleanup. Once the idle-disposed objects had been evicted from memory, each DELETE's validateMcpSessionOwner restored the runtime by calling onStart directly, which leaves PartyServer's own "started" state unset; the following fetch re-ran onStart under blockConcurrencyWhile, tearing down and rebuilding the runtime with the input gate held. Every restore through an RPC on a fresh instance paid for two cold builds. RPC restores now go through __unsafe_ensureInitialized (the SDK's own gate for RPC entry points) on a fresh instance, and restore in place otherwise.

Observed, not fixed here. The same CI runs showed 30s blockConcurrencyWhile resets (32 concurrency_reset per run). Span timings point at the build semaphore's hand-off rather than the double build: every init that queued at session-build-semaphore.ts sat out the full 10s queue timeout, and the ones granted a slot from another Durable Object's request context did not finish before the 30s reset. That is tracked in #2063; the scenario stays out of the queue so it cannot hit it.

Verified locally: DO unit tests (51), host-cloudflare and e2e typechecks, oxfmt, oxlint, and both e2e files against a full cloud boot (vitest globalsetup, MCP_SESSION_TIMEOUT_MS=3000, motel span store). The cap-eviction scenario, fresh boot each time:

  • paired with mcp-destroyed-session-envelope first, three boots: kept 24 sessions resident through 6 open+touch ticks in 1681 / 1829 / 2061 ms (longest tick 394 / 444 / 539 ms against the 3000 ms window, 84 touches each); the 10 crossing opens took 355 / 359 / 430 ms; both files passed every time.
  • alone: 2827 ms, longest tick 541 ms; passed.
  • alone with MCP_SESSION_TIMEOUT_MS=1000, so the touched phase is twice the window: 2035 ms, longest tick 419 ms; passed.
  • Span timeline from one paired boot (motel sqlite): no disposal during the 24 touched opens, the first cap disposals 12 ms after the 25th init, one eviction per crossing open, residency never above 24.
  • The design before this one (a concurrent keep-alive fiber sharing the build width) failed 1 of 4 paired boots the same way CI did: a touch is a full authenticated request (~190 ms locally), so a round over 24 sessions at the width left for it approached the window.

@github-actions

github-actions Bot commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

Cloudflare preview

Console https://executor-preview-pr-2060.executor-e2e.workers.dev
MCP https://executor-preview-pr-2060.executor-e2e.workers.dev/mcp
Deployed commit b5daec8

Sign-in is Cloudflare Access (one-time PIN to an allowed email). The preview has its own database and encryption key; it is destroyed when this PR closes.

@pkg-pr-new

pkg-pr-new Bot commented Sep 18, 2026

Copy link
Copy Markdown

Open in StackBlitz

@executor-js/cli

npm i https://pkg.pr.new/@executor-js/cli@2060

@executor-js/config

npm i https://pkg.pr.new/@executor-js/config@2060

@executor-js/execution

npm i https://pkg.pr.new/@executor-js/execution@2060

@executor-js/sdk

npm i https://pkg.pr.new/@executor-js/sdk@2060

@executor-js/codemode-core

npm i https://pkg.pr.new/@executor-js/codemode-core@2060

@executor-js/runtime-quickjs

npm i https://pkg.pr.new/@executor-js/runtime-quickjs@2060

@executor-js/plugin-file-secrets

npm i https://pkg.pr.new/@executor-js/plugin-file-secrets@2060

@executor-js/plugin-graphql

npm i https://pkg.pr.new/@executor-js/plugin-graphql@2060

@executor-js/plugin-keychain

npm i https://pkg.pr.new/@executor-js/plugin-keychain@2060

@executor-js/plugin-mcp

npm i https://pkg.pr.new/@executor-js/plugin-mcp@2060

@executor-js/plugin-onepassword

npm i https://pkg.pr.new/@executor-js/plugin-onepassword@2060

@executor-js/plugin-openapi

npm i https://pkg.pr.new/@executor-js/plugin-openapi@2060

executor

npm i https://pkg.pr.new/executor@2060

commit: b5daec8

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Sep 18, 2026

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
executor-marketing b5daec8 Commit Preview URL

Branch Preview URL
Sep 18 2026, 07:51 PM

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Sep 18, 2026

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Updated (UTC)
✅ Deployment successful!
View logs
executor-cloud b5daec8 Sep 18 2026, 07:52 PM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant