From baf18e387663b01b49a5a1eb69826ea7b4c85bb5 Mon Sep 17 00:00:00 2001 From: Rhys Sullivan <39114868+RhysSullivan@users.noreply.github.com> Date: Fri, 18 Sep 2026 12:02:05 -0700 Subject: [PATCH 1/2] Fix MCP session teardown races behind two flaky cloud e2e tests --- .changeset/mcp-session-teardown-races.md | 5 + e2e/cloud/mcp-session-cap-eviction.test.ts | 39 ++++- .../mcp/agent-session-durable-object.test.ts | 150 ++++++++++++++++++ .../src/mcp/agent-session-durable-object.ts | 79 ++++++++- 4 files changed, 259 insertions(+), 14 deletions(-) create mode 100644 .changeset/mcp-session-teardown-races.md diff --git a/.changeset/mcp-session-teardown-races.md b/.changeset/mcp-session-teardown-races.md new file mode 100644 index 0000000000..e93092637a --- /dev/null +++ b/.changeset/mcp-session-teardown-races.md @@ -0,0 +1,5 @@ +--- +"@executor-js/cloudflare": patch +--- + +Answer owner checks with the reconnect verdict while a session Durable Object is mid-destroy, and initialize a fresh session instance through PartyServer's gate when an RPC restores its runtime, so the next request does not rebuild it. diff --git a/e2e/cloud/mcp-session-cap-eviction.test.ts b/e2e/cloud/mcp-session-cap-eviction.test.ts index 8103eef5cd..fdd3b04ace 100644 --- a/e2e/cloud/mcp-session-cap-eviction.test.ts +++ b/e2e/cloud/mcp-session-cap-eviction.test.ts @@ -25,6 +25,7 @@ import { Effect, Option, Schedule, Schema } from "effect"; import { scenario } from "../src/scenario"; import { Mcp, Target, Telemetry } from "../src/services"; import type { Identity } from "../src/target"; +import { configuredMcpSessionTimeoutMs } from "../setup/mcp-session-timeouts"; import { E2E_MCP_RESIDENT_RUNTIME_SOFT_CAP } from "../setup/resident-runtime-cap"; const PROTOCOL_VERSION = "2025-03-26"; @@ -35,6 +36,23 @@ const JSON_AND_SSE = "application/json, text/event-stream"; // sessions cross it that at least one eviction targets a session opened here. const SESSIONS_TO_OPEN = E2E_MCP_RESIDENT_RUNTIME_SOFT_CAP + 10; +// The cap only trips if the sessions opened here are still RESIDENT when the +// last of them is admitted. A session that reaches the target's idle timeout +// first (MCP_SESSION_TIMEOUT_MS, squeezed to a few seconds for e2e) gives its +// runtime back and leaves the count, and a batch that idles out as fast as it +// is opened never reaches the cap at all — no eviction, nothing to assert on. +// Opening one session at a time took ~60ms per session on a quiet runner and +// ~270ms under CI load, which crosses the e2e idle window well before the +// 34th session. A few opens in flight at once keep the whole batch inside it. +// +// Exactly as many as the isolate builds at once (MAX_CONCURRENT_BUILDS in +// apps/cloud/src/mcp/session-build-semaphore.ts), no more: an admission that +// has to wait at that semaphore is handed its slot from the releasing +// session's request context, and a cold build resumed that way does not +// complete in workerd — it sits until the queue's 10s timeout, or is reset at +// the 30s `blockConcurrencyWhile` limit. Four in flight never queue. +const OPEN_CONCURRENCY = 4; + const emailOf = (identity: Identity): string => identity.credentials?.email ?? identity.label; const mcpHeaders = (bearer: string, sessionId?: string) => ({ @@ -230,12 +248,10 @@ scenario( const openedSessionIds: string[] = []; const scenarioBody = Effect.gen(function* () { - // Open more sessions than the cap allows. Keep admission sequential: - // the cloud e2e database is one serialized PGlite instance, and this - // scenario exercises resident eviction rather than concurrent cold - // builds. None of the sessions run any work, so every one is immediately - // eviction-eligible — crossing the cap must pick at least one and tear it - // down through its own stub. + // Open more sessions than the cap allows. None of the sessions run any + // work, so every one is immediately eviction-eligible — crossing the cap + // must pick at least one and tear it down through its own stub. + const openStartedAt = Date.now(); const sessionIds = yield* Effect.forEach( Array.from({ length: SESSIONS_TO_OPEN }, (_, index) => index), (index) => @@ -244,11 +260,20 @@ scenario( openedSessionIds.push(sessionId); }), ), - { concurrency: 1 }, + { concurrency: OPEN_CONCURRENCY }, ); + const openTookMs = Date.now() - openStartedAt; expect(sessionIds.length, "every session opened").toBe(SESSIONS_TO_OPEN); expect(new Set(sessionIds).size, "every session got a distinct id").toBe(SESSIONS_TO_OPEN); + // Coverage precondition (see OPEN_CONCURRENCY): a batch that outlasted + // the idle window may have shed its first sessions before the cap was + // crossed, and the span search below would then report "no eviction" + // for a reason that has nothing to do with eviction. Say so directly. + expect( + openTookMs, + `opening ${SESSIONS_TO_OPEN} sessions must finish inside the target's ${configuredMcpSessionTimeoutMs()}ms idle window, or the first ones are disposed before the cap is reached`, + ).toBeLessThan(configuredMcpSessionTimeoutMs()); // ---- a real cap eviction fired, against a session opened here ------- // Same span the idle path emits (`mcp.session.idle_runtime_dispose`); diff --git a/packages/hosts/cloudflare/src/mcp/agent-session-durable-object.test.ts b/packages/hosts/cloudflare/src/mcp/agent-session-durable-object.test.ts index 813f4d5c40..af8e9e8f79 100644 --- a/packages/hosts/cloudflare/src/mcp/agent-session-durable-object.test.ts +++ b/packages/hosts/cloudflare/src/mcp/agent-session-durable-object.test.ts @@ -168,7 +168,9 @@ type HarnessSession = { requestIds: ReadonlyArray, ) => Promise; dbHandle: { readonly end: () => void } | null; + destroy: () => Promise; engine: ExecutionEngine | null; + ensurePartyServerInitialized: () => Promise; getConnections?: () => Iterable; getSessionId: () => string; initialized: boolean; @@ -317,6 +319,11 @@ const makeHarnessSession = async ( session.server = server; session.sessionMeta = sessionMeta; session.sessionTimeoutMs = () => 1; + // The harness has no PartyServer private state to consult. Model an + // instance PartyServer has already started — the state every in-place + // restore below runs in — so the gate is a no-op; the fresh-instance case + // installs its own model where it matters. + session.ensurePartyServerInitialized = async () => undefined; session.runMcpAgentOnStart = async () => { const restored = session.server ?? makeServer(); session.server = restored; @@ -1941,3 +1948,146 @@ describe("McpAgentSessionDOBase stranded-request ledger", () => { ).toBe(false); }); }); + +describe("McpAgentSessionDOBase owner check during the destroy alarm", () => { + /** + * The agents SDK's `destroy()` is what runs between "the durable + * destroy-pending marker is gone" and "the isolate is aborted": it + * `deleteAll()`s storage (taking the marker with it) and only then aborts + * from a `setTimeout(0)`. Stand in for it on the SDK prototype so the test + * can hold the object in exactly that gap — the real one also disposes SDK + * internals the harness never constructed. + */ + const withSdkDestroy = async ( + replacement: (this: HarnessSession) => Promise, + run: () => Promise, + ): Promise => { + let proto: object | null = Object.getPrototypeOf(McpAgentSessionDOBase.prototype); + while (proto && !Object.hasOwn(proto, "destroy")) proto = Object.getPrototypeOf(proto); + if (!proto) throw new Error("agents SDK prototype chain has no destroy()"); + const original = Reflect.get(proto, "destroy"); + Reflect.set(proto, "destroy", replacement); + try { + await run(); + } finally { + Reflect.set(proto, "destroy", original); + } + }; + + it("answers terminated, not a restore, once destroy has wiped the marker", async () => { + const session = await makeHarnessSession(); + await session.ctx.storage.put("cf_agents_destroy_pending", true); + let restoreAttempts = 0; + session.runMcpAgentOnStart = async () => { + restoreAttempts += 1; + // What the SDK's own `onStart` does first on a destroyed object: read a + // table `destroy()` has already dropped. + throw new Error("no such table: cf_agents_mcp_servers: SQLITE_ERROR"); + }; + const wiped = makeDeferred(); + const release = makeDeferred(); + + await withSdkDestroy( + async function (this: HarnessSession) { + await this.ctx.storage.deleteAll(); + wiped.resolve(); + await release.promise; + }, + async () => { + const destroying = session.destroy(); + await wiped.promise; + expect( + await session.ctx.storage.get("cf_agents_destroy_pending"), + "precondition: the durable marker is already gone", + ).toBeUndefined(); + expect(session.initialized, "precondition: the runtime is already torn down").toBe(false); + + const verdict = await session.validateMcpSessionOwner( + { accountId: "user-1", organizationId: "org-1" }, + defaultMcpResource, + ); + + expect(verdict).toBe("terminated"); + expect(restoreAttempts, "a dying object never tries to rebuild its runtime").toBe(0); + release.resolve(); + await destroying; + }, + ); + }); +}); + +describe("McpAgentSessionDOBase restore from an RPC entry point", () => { + /** + * PartyServer's gate, as the harness sees it: runs `onStart` exactly once + * per instance and remembers that it did. `started` is the private state + * the real class keeps and this one cannot read. + */ + const modelPartyServerGate = (session: HarnessSession, started: boolean) => { + const gate = { runs: 0, started }; + session.ensurePartyServerInitialized = async () => { + if (gate.started) return; + gate.runs += 1; + await session.onStart(); + gate.started = true; + }; + return gate; + }; + + const coldInstance = (session: HarnessSession): void => { + session.initialized = false; + session.engine = null; + session.dbHandle = null; + delete session.server; + }; + + it("initializes a fresh instance through PartyServer's gate, so the next fetch does not restart it", async () => { + const session = await makeHarnessSession(); + coldInstance(session); + const gate = modelPartyServerGate(session, false); + let onStartCalls = 0; + const restore = session.runMcpAgentOnStart; + session.runMcpAgentOnStart = async () => { + onStartCalls += 1; + await restore(); + }; + + await expect( + session.validateMcpSessionOwner( + { accountId: "user-1", organizationId: "org-1" }, + defaultMcpResource, + ), + ).resolves.toBe("ok"); + expect(gate.runs, "the restore went through the gate").toBe(1); + expect(onStartCalls).toBe(1); + expect(session.initialized).toBe(true); + + // The SDK's fetch path for the same session: `setName` re-enters the gate. + // A started instance passes straight through instead of rebuilding. + await session.ensurePartyServerInitialized(); + expect(onStartCalls, "no second build under the input gate").toBe(1); + expect(session.initialized).toBe(true); + }); + + it("restores in place when PartyServer already started the instance", async () => { + const session = await makeHarnessSession(); + const gate = modelPartyServerGate(session, true); + let onStartCalls = 0; + const restore = session.runMcpAgentOnStart; + session.runMcpAgentOnStart = async () => { + onStartCalls += 1; + await restore(); + }; + + await session.alarm(); + expect(session.initialized, "precondition: idle disposal emptied the runtime").toBe(false); + + await expect( + session.validateMcpSessionOwner( + { accountId: "user-1", organizationId: "org-1" }, + defaultMcpResource, + ), + ).resolves.toBe("ok"); + expect(gate.runs, "a started instance is not re-gated").toBe(0); + expect(onStartCalls, "the runtime is rebuilt directly").toBe(1); + }); +}); diff --git a/packages/hosts/cloudflare/src/mcp/agent-session-durable-object.ts b/packages/hosts/cloudflare/src/mcp/agent-session-durable-object.ts index 5a90dc5978..9b0a6eafec 100644 --- a/packages/hosts/cloudflare/src/mcp/agent-session-durable-object.ts +++ b/packages/hosts/cloudflare/src/mcp/agent-session-durable-object.ts @@ -344,6 +344,21 @@ export abstract class McpAgentSessionDOBase< * second time. */ private disposingRuntime: Promise | null = null; + /** + * Set the instant `destroy()` begins, before it wipes anything, and never + * cleared: this instance is on its way out and every answer it still gives + * must say so. + * + * The durable `cf_agents_destroy_pending` marker is not enough on its own. + * The agents SDK's `destroy()` drops its own tables and `deleteAll()`s + * storage — taking that marker with it — and only then aborts the isolate, + * from a `setTimeout(0)` after an `await`. A request that reaches this + * instance in that gap sees no marker, still holds the cached session meta, + * finds no runtime, and tries to restore one — which the SDK's `onStart` + * begins by reading a table that no longer exists, so the request dies as + * an unclassified `no such table` error instead of the reconnect verdict. + */ + private destroying = false; private onStartPromise: Promise | null = null; private lastActivityMs = 0; private resolvedSessionName: string | undefined = undefined; @@ -1366,13 +1381,56 @@ export abstract class McpAgentSessionDOBase< const sessionMeta = yield* self.loadSessionMeta(); if (!sessionMeta) return false; - yield* Effect.promise(() => self.onStart()).pipe( - Effect.withSpan("McpSessionDO.restore_runtime_for_approval"), - ); + yield* self + .restoreRuntimeForRpc() + .pipe(Effect.withSpan("McpSessionDO.restore_runtime_for_approval")); return self.initialized && !!self.engine; }).pipe(Effect.withSpan("McpSessionDO.ensure_runtime_for_approval")); } + /** + * Bring the runtime back for a request that entered through one of this + * class's own RPC methods rather than through the agents SDK's fetch path. + * + * PartyServer runs `onStart` exactly once per Durable Object instance, from + * inside `blockConcurrencyWhile`, and remembers that it did in private + * state this class cannot read. Every SDK entry point (`fetch`, `alarm`, + * `setName`, the SDK's own RPC methods) goes through that gate first. A + * direct `this.onStart()` from an RPC method restores the runtime but + * leaves PartyServer's state at "never started" — so on a FRESH instance + * (the object evicted from memory after its idle disposal, then woken by + * this RPC) the very next `fetch` for the same session re-enters the gate, + * finds the instance unstarted, and runs `onStart` again: tearing down the + * runtime this RPC just built and rebuilding it with the object's input gate + * held for the whole build. Under load that rebuild ran into the platform's + * `blockConcurrencyWhile` limit and reset the object, turning every + * request after a long idle into a ~30s stall ending in a 503. + * + * So a fresh instance is initialized the way the SDK initializes its own + * RPC entry points — `__unsafe_ensureInitialized`, PartyServer's escape + * hatch for exactly this — which runs `onStart` under the gate once and + * marks the instance started. On an instance PartyServer already started + * whose runtime was since disposed in place, that call is a no-op, and the + * runtime is restored directly as before; no later gate re-runs it. + */ + private restoreRuntimeForRpc(): Effect.Effect { + const self = this; + return Effect.gen(function* () { + yield* Effect.promise(() => self.ensurePartyServerInitialized()); + if (self.initialized) return; + yield* Effect.promise(() => self.onStart()); + }); + } + + /** + * PartyServer's own initialization gate — see {@link restoreRuntimeForRpc}. + * A seam only so a unit harness built without PartyServer's private state + * can stand in for it; production never overrides this. + */ + protected ensurePartyServerInitialized(): Promise { + return this.__unsafe_ensureInitialized(); + } + private startRuntimeFromOnStart(props?: McpSessionProps): Effect.Effect { const self = this; return Effect.gen(function* () { @@ -1614,7 +1672,11 @@ export abstract class McpAgentSessionDOBase< const destroyPending = yield* Effect.promise(() => self.ctx.storage.get(AGENTS_DESTROY_PENDING_KEY), ); - if (destroyPending === true) return "terminated" as const; + // Both signals, because they cover different halves of the teardown: + // the durable marker covers the second before the destroy alarm + // fires, the in-memory flag covers the alarm's own run — after the + // marker has been wiped, before the isolate is aborted. + if (destroyPending === true || self.destroying) return "terminated" as const; const sessionMeta = yield* self.loadSessionMeta(); if (!sessionMeta) return "not_found" as const; if (self.initialized) { @@ -1622,9 +1684,9 @@ export abstract class McpAgentSessionDOBase< .bestEffortBookkeeping("validate_owner.mark_activity", () => self.markActivity()) .pipe(Effect.withSpan("McpSessionDO.markActivity")); } else { - yield* Effect.promise(() => self.onStart()).pipe( - Effect.withSpan("McpSessionDO.restore_transport_runtime"), - ); + yield* self + .restoreRuntimeForRpc() + .pipe(Effect.withSpan("McpSessionDO.restore_transport_runtime")); } const ownerMatches = identity.accountId === sessionMeta.userId && @@ -1771,6 +1833,9 @@ export abstract class McpAgentSessionDOBase< } override async destroy(): Promise { + // Before any await: an owner check that lands during the teardown below + // must already read this as terminated (see `destroying`). + this.destroying = true; await this.cleanup(); await super.destroy(); } From b5daec8655d155d5e7d8f66f2554b3c1cd7ca808 Mon Sep 17 00:00:00 2001 From: Rhys Sullivan <39114868+RhysSullivan@users.noreply.github.com> Date: Fri, 18 Sep 2026 12:50:03 -0700 Subject: [PATCH 2/2] Keep cap-eviction sessions resident with touch waves instead of open throughput --- apps/cloud/src/mcp/session-build-semaphore.ts | 8 +- e2e/cloud/mcp-session-cap-eviction.test.ts | 165 ++++++++++++++---- .../src/mcp/agent-session-durable-object.ts | 10 +- 3 files changed, 148 insertions(+), 35 deletions(-) diff --git a/apps/cloud/src/mcp/session-build-semaphore.ts b/apps/cloud/src/mcp/session-build-semaphore.ts index 4e778deb2f..74b2cd49e3 100644 --- a/apps/cloud/src/mcp/session-build-semaphore.ts +++ b/apps/cloud/src/mcp/session-build-semaphore.ts @@ -13,7 +13,13 @@ * Deliberately dependency-free: a tiny promise-chain queue, not a library. */ -const MAX_CONCURRENT_BUILDS = 4; +/** + * Exported for the one consumer outside this module that has to agree with + * it: the cap-eviction e2e scenario (e2e/cloud/mcp-session-cap-eviction.test.ts) + * holds its own request concurrency to exactly this width, so nothing it + * opens ever waits in the queue below. + */ +export const MAX_CONCURRENT_BUILDS = 4; /** * Max time a build waits in the FIFO queue for a slot before proceeding diff --git a/e2e/cloud/mcp-session-cap-eviction.test.ts b/e2e/cloud/mcp-session-cap-eviction.test.ts index fdd3b04ace..f037ba3a91 100644 --- a/e2e/cloud/mcp-session-cap-eviction.test.ts +++ b/e2e/cloud/mcp-session-cap-eviction.test.ts @@ -22,6 +22,7 @@ import { expect, it } from "@effect/vitest"; import { Effect, Option, Schedule, Schema } from "effect"; +import { MAX_CONCURRENT_BUILDS } from "../../apps/cloud/src/mcp/session-build-semaphore"; import { scenario } from "../src/scenario"; import { Mcp, Target, Telemetry } from "../src/services"; import type { Identity } from "../src/target"; @@ -36,22 +37,54 @@ const JSON_AND_SSE = "application/json, text/event-stream"; // sessions cross it that at least one eviction targets a session opened here. const SESSIONS_TO_OPEN = E2E_MCP_RESIDENT_RUNTIME_SOFT_CAP + 10; +// Every request below that can start a cold runtime build is held to the +// isolate's own build width, so none of them ever waits in the FIFO queue at +// apps/cloud/src/mcp/session-build-semaphore.ts. That is an `initialize`, a +// keep-alive touch of a session the cap has evicted (the owner check +// restores it before the request is forwarded), and a cleanup DELETE of a +// session whose runtime was disposed (same restore, then the destroy). +// +// A build that does wait there is handed its slot from the releasing +// session's request context, and in the CI runs that first failed this +// scenario no build resumed that way finished: every queued init sat out the +// queue's full 10s timeout, and the ones granted a slot were reset at the +// 30s `blockConcurrencyWhile` limit. That is observed, most likely the +// semaphore hand-off itself, and tracked separately in +// https://github.com/UsefulSoftwareCo/executor/issues/2063. It is not what +// this scenario pins, so the scenario stays out of the queue entirely: opens +// and touches never overlap, the batch alternates a wave of one with a wave +// of the other, and each wave is at most this wide. +const COLD_BUILD_CONCURRENCY = MAX_CONCURRENT_BUILDS; + // The cap only trips if the sessions opened here are still RESIDENT when the -// last of them is admitted. A session that reaches the target's idle timeout +// next one is admitted. A session that reaches the target's idle timeout // first (MCP_SESSION_TIMEOUT_MS, squeezed to a few seconds for e2e) gives its // runtime back and leaves the count, and a batch that idles out as fast as it // is opened never reaches the cap at all — no eviction, nothing to assert on. -// Opening one session at a time took ~60ms per session on a quiet runner and -// ~270ms under CI load, which crosses the e2e idle window well before the -// 34th session. A few opens in flight at once keep the whole batch inside it. +// Open throughput is not something to rely on for that (34 opens took 3.5s +// on a loaded CI runner, against a 3s window), so the batch keeps them +// resident itself: after every wave of opens, every session opened so far is +// touched with a `ping`, which marks it active and re-arms its idle alarm +// through the owner check every request with a session id goes through. A +// touch is a full authenticated request, so the touch wave is the slow half +// of a tick, and one tick is the most any session goes untouched; the +// scenario measures the longest one and prints it next to the window. // -// Exactly as many as the isolate builds at once (MAX_CONCURRENT_BUILDS in -// apps/cloud/src/mcp/session-build-semaphore.ts), no more: an admission that -// has to wait at that semaphore is handed its slot from the releasing -// session's request context, and a cold build resumed that way does not -// complete in workerd — it sits until the queue's 10s timeout, or is reset at -// the 30s `blockConcurrencyWhile` limit. Four in flight never queue. -const OPEN_CONCURRENCY = 4; +// Touching stops once this many sessions are open, which is before the cap +// can evict any of THESE. Once an isolate is at the cap, every admission +// evicts its least-recently-active resident. With M sessions left over from +// earlier scenarios (older than these, so evicted first) the cap is reached +// at admission cap − M + 1, admissions up to cap evict the M leftovers, and +// admission cap + 1 is the first that can pick one of these — and it is the +// very next open, landing while every session from the last touch wave is +// fresh. Touching past that point would only trade one eviction for +// another: a touch of a session the cap has just evicted restores it, and +// that admission evicts the next candidate. A leftover that is NOT evictable +// (a paused execution keeps its runtime resident) moves the first pick of +// one of these earlier, into the touched phase; the next touch wave then +// restores it, which is a cold build like any other, and is why touch waves +// are held to the same width as open waves. +const KEEP_RESIDENT_WHILE_OPENING = E2E_MCP_RESIDENT_RUNTIME_SOFT_CAP; const emailOf = (identity: Identity): string => identity.credentials?.email ?? identity.label; @@ -207,6 +240,37 @@ const openSession = async ( return sessionId; }; +/** + * The cheapest request that keeps a session resident: a JSON-RPC `ping`, + * answered by the MCP server's protocol layer without touching a tool. The + * idle alarm is re-armed by the owner check the router runs before any + * request with a session id is forwarded, so a served ping is all that is + * needed. The documented restart envelope is tolerated too — the platform + * reset the session's object underneath the batch, and it restores itself + * on its next request — the same transient `openSession` tolerates. + * Anything else is a real failure: it stops the keep-alive, which fails the + * scenario with it. + */ +const touchSession = async ( + mcpUrl: string, + bearer: string, + sessionId: string, + id: number, +): Promise => { + const response = await postJson( + mcpUrl, + bearer, + { jsonrpc: "2.0" as const, id: `keep-alive-${id}`, method: "ping" }, + sessionId, + ); + const body = await response.text(); + if (response.status === 200 || isRestartResponse(response.status, body)) return; + // oxlint-disable-next-line executor/no-error-constructor -- boundary: e2e keep-alive precondition. + throw new Error( + `keep-alive ping of ${sessionId} was not served: ${response.status} ${body.slice(0, 200)}`, + ); +}; + const executeBody = (id: string, code: string) => ({ jsonrpc: "2.0" as const, id, @@ -248,32 +312,67 @@ scenario( const openedSessionIds: string[] = []; const scenarioBody = Effect.gen(function* () { + const openWave = (wave: ReadonlyArray) => + Effect.forEach( + wave, + (index) => + Effect.promise(() => + openSession(target.mcpUrl, bearer, `session-${index}`, (sessionId) => { + openedSessionIds.push(sessionId); + }), + ), + { concurrency: COLD_BUILD_CONCURRENCY }, + ); + let touches = 0; + // `suspend`: the sessions to touch are whichever are open when the + // wave runs, not when this is built. + const touchEveryOpenSession = Effect.suspend(() => + Effect.forEach( + [...openedSessionIds], + (sessionId) => + Effect.promise(() => { + touches += 1; + return touchSession(target.mcpUrl, bearer, sessionId, touches); + }), + { concurrency: COLD_BUILD_CONCURRENCY, discard: true }, + ), + ); + // Open more sessions than the cap allows. None of the sessions run any // work, so every one is immediately eviction-eligible — crossing the cap // must pick at least one and tear it down through its own stub. - const openStartedAt = Date.now(); - const sessionIds = yield* Effect.forEach( - Array.from({ length: SESSIONS_TO_OPEN }, (_, index) => index), - (index) => - Effect.promise(() => - openSession(target.mcpUrl, bearer, `session-${index}`, (sessionId) => { - openedSessionIds.push(sessionId); - }), - ), - { concurrency: OPEN_CONCURRENCY }, + const indices = Array.from({ length: SESSIONS_TO_OPEN }, (_, index) => index); + const sessionIds: string[] = []; + + // Kept resident (see KEEP_RESIDENT_WHILE_OPENING): a wave of opens, + // then a wave of touches over everything open so far, and again. + let ticks = 0; + let longestTickMs = 0; + const keptStartedAt = Date.now(); + for (let from = 0; from < KEEP_RESIDENT_WHILE_OPENING; from += COLD_BUILD_CONCURRENCY) { + const tickStartedAt = Date.now(); + const to = Math.min(from + COLD_BUILD_CONCURRENCY, KEEP_RESIDENT_WHILE_OPENING); + sessionIds.push(...(yield* openWave(indices.slice(from, to)))); + yield* touchEveryOpenSession; + ticks += 1; + longestTickMs = Math.max(longestTickMs, Date.now() - tickStartedAt); + } + const keptTookMs = Date.now() - keptStartedAt; + + // Crossing the cap: the rest, untouched. The first of these admissions + // is the one that must evict a session opened above. + const crossingStartedAt = Date.now(); + sessionIds.push(...(yield* openWave(indices.slice(KEEP_RESIDENT_WHILE_OPENING)))); + const crossingTookMs = Date.now() - crossingStartedAt; + + // Diagnostic only: the scenario no longer depends on the batch beating + // the idle window, but the figures show how much room the keep-alive had. + console.info( + `[cap-eviction] kept ${KEEP_RESIDENT_WHILE_OPENING} sessions resident through ${ticks} open+touch ticks in ${keptTookMs}ms (longest tick ${longestTickMs}ms against a ${configuredMcpSessionTimeoutMs()}ms idle window, ${touches} touches); the ${SESSIONS_TO_OPEN - KEEP_RESIDENT_WHILE_OPENING} opens that cross the cap took ${crossingTookMs}ms`, ); - const openTookMs = Date.now() - openStartedAt; expect(sessionIds.length, "every session opened").toBe(SESSIONS_TO_OPEN); expect(new Set(sessionIds).size, "every session got a distinct id").toBe(SESSIONS_TO_OPEN); - // Coverage precondition (see OPEN_CONCURRENCY): a batch that outlasted - // the idle window may have shed its first sessions before the cap was - // crossed, and the span search below would then report "no eviction" - // for a reason that has nothing to do with eviction. Say so directly. - expect( - openTookMs, - `opening ${SESSIONS_TO_OPEN} sessions must finish inside the target's ${configuredMcpSessionTimeoutMs()}ms idle window, or the first ones are disposed before the cap is reached`, - ).toBeLessThan(configuredMcpSessionTimeoutMs()); // ---- a real cap eviction fired, against a session opened here ------- // Same span the idle path emits (`mcp.session.idle_runtime_dispose`); @@ -357,7 +456,11 @@ scenario( }); await closed.text(); }).pipe(Effect.ignore), - { concurrency: 8, discard: true }, + // A DELETE of a session whose runtime was disposed in the meantime + // restores it before the destroy (the owner check runs first), so + // this is a wave of cold builds too — held to the same width as + // every other wave above (see COLD_BUILD_CONCURRENCY). + { concurrency: COLD_BUILD_CONCURRENCY, discard: true }, ), ), ), diff --git a/packages/hosts/cloudflare/src/mcp/agent-session-durable-object.ts b/packages/hosts/cloudflare/src/mcp/agent-session-durable-object.ts index 9b0a6eafec..605e69aa1b 100644 --- a/packages/hosts/cloudflare/src/mcp/agent-session-durable-object.ts +++ b/packages/hosts/cloudflare/src/mcp/agent-session-durable-object.ts @@ -1402,9 +1402,13 @@ export abstract class McpAgentSessionDOBase< * this RPC) the very next `fetch` for the same session re-enters the gate, * finds the instance unstarted, and runs `onStart` again: tearing down the * runtime this RPC just built and rebuilding it with the object's input gate - * held for the whole build. Under load that rebuild ran into the platform's - * `blockConcurrencyWhile` limit and reset the object, turning every - * request after a long idle into a ~30s stall ending in a 503. + * held for the whole build. That second build is the confirmed defect: + * every restore through an RPC on a fresh instance paid for two cold + * builds, the second with the object's input gate closed. The CI runs that + * surfaced it also showed builds reset at the platform's 30s + * `blockConcurrencyWhile` limit; that stall is observed alongside it, not + * explained by it — most likely the cloud app's build-semaphore hand-off, + * tracked in https://github.com/UsefulSoftwareCo/executor/issues/2063. * * So a fresh instance is initialized the way the SDK initializes its own * RPC entry points — `__unsafe_ensureInitialized`, PartyServer's escape