diff --git a/.changeset/lost-execution-visibility.md b/.changeset/lost-execution-visibility.md new file mode 100644 index 0000000000..28e783ab13 --- /dev/null +++ b/.changeset/lost-execution-visibility.md @@ -0,0 +1,5 @@ +--- +"executor": patch +--- + +Report an MCP `execute` call that dies with a session reset as a JSON-RPC error instead of a silently closed stream. The front worker answers outstanding request ids when the session socket closes abnormally or a response deadline passes, and a rebuilt session answers ids stranded by a previous incarnation on the next stream. The plain memory-limit reset is now classified as transient. diff --git a/apps/cloud/src/observability/observability.test.ts b/apps/cloud/src/observability/observability.test.ts index b95ea61d7a..581377d12f 100644 --- a/apps/cloud/src/observability/observability.test.ts +++ b/apps/cloud/src/observability/observability.test.ts @@ -385,15 +385,24 @@ describe("Durable Object platform reset noise", () => { expect(beforeSendWithOtelCorrelation(defect)).not.toBeNull(); }); - // The memory-limit reset is deliberately absent from the classifier: the - // runtime blames the application for it, so it is a defect, not noise. - it("keeps the memory-limit reset the classifier deliberately excludes", () => { + // The storage-cache memory-limit variant stays absent from the classifier: + // the runtime blames the application for it (un-awaited writes, an oversized + // read), so it is a defect, not noise. Its plain sibling is a platform reset + // and IS classified — the two are separated only by that qualifier. + it("keeps the memory-limit variant the classifier deliberately excludes", () => { const memory = doInstrumentationEvent( "Durable Object's isolate exceeded its memory limit due to overflowing the storage cache. All objects in the isolate were reset.", ); expect(beforeSendWithOtelCorrelation(memory)).not.toBeNull(); }); + it("drops the plain memory-limit reset as platform noise", () => { + const memory = doInstrumentationEvent( + "Durable Object's isolate exceeded its memory limit and was reset.", + ); + expect(beforeSendWithOtelCorrelation(memory)).toBeNull(); + }); + it("the hook the worker and DOs install drops the deploy reset", () => { const options = cloudSentryOptions({ SENTRY_DSN: "https://public@example.invalid/1" } as Env); const event = doInstrumentationEvent("Durable Object reset because its code was updated."); 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 4c38d7a606..3dbae924b8 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 @@ -153,6 +153,20 @@ type HarnessSession = { >; alarm: () => Promise; ctx: MemoryStorage; + currentSessionEpoch: () => Promise; + getStaleEpochStreamRequestIds: () => Promise< + ReadonlyArray<{ + readonly streamId: string; + readonly requestIds: ReadonlyArray; + readonly epoch: number; + readonly currentEpoch: number; + }> + >; + getStreamRequestIds: (streamId: string) => Promise | undefined>; + setStreamRequestIds: ( + streamId: string, + requestIds: ReadonlyArray, + ) => Promise; dbHandle: { readonly end: () => void } | null; engine: ExecutionEngine | null; getConnections?: () => Iterable; @@ -266,7 +280,14 @@ const approval = { content: { approved: true }, } satisfies ResumeResponse; -const makeHarnessSession = async (): Promise => { +/** + * `storage` is a parameter so a test can build a SECOND session on the same + * durable storage — that is exactly what a Durable Object reset looks like from + * storage's point of view: same keys, brand new instance. + */ +const makeHarnessSession = async ( + storage: MemoryStorage = new MemoryStorage(), +): Promise => { const sessionId = "session-reconnect"; const sessionMeta: SessionMeta = { organizationId: "org-1", @@ -275,7 +296,6 @@ const makeHarnessSession = async (): Promise => { userId: "user-1", resource: defaultMcpResource, }; - const storage = new MemoryStorage(); const server = makeServer(); await server.connect(new StaleCloseTransport()); @@ -1766,3 +1786,110 @@ describe("McpAgentSessionDOBase residency cap eviction", () => { }); }); }); + +// The request-id ledger (`__mcp_stream_reqs__:`, written by the +// patched McpAgent — see patches/agents@0.17.3.patch) is the only durable +// record that a POST is still owed a response. A row exists from the moment the +// request is accepted until its final response is written, so a row that +// outlives the incarnation which accepted it is a request nothing will ever +// answer: the isolate was reset mid-execute. Each row carries the epoch of the +// incarnation that wrote it, and that is what separates "stranded" from +// "legitimately still running" — a browser-approval pause holds a row open for +// minutes inside ONE incarnation and must never be swept. +describe("McpAgentSessionDOBase stranded-request ledger", () => { + const ledgerKey = (streamId: string) => `__mcp_stream_reqs__:${streamId}`; + + it("stamps the accepting incarnation on every ledger row", async () => { + const session = await makeHarnessSession(); + + await session.setStreamRequestIds("stream-a", [1, "two"]); + + expect(await session.ctx.storage.get(ledgerKey("stream-a"))).toEqual({ + epoch: await session.currentSessionEpoch(), + requestIds: [1, "two"], + }); + expect( + await session.getStreamRequestIds("stream-a"), + "readers still see a plain request-id list", + ).toEqual([1, "two"]); + }); + + it("does not treat a row from the running incarnation as stranded", async () => { + const session = await makeHarnessSession(); + + // What a browser-approval pause looks like: accepted, unanswered, and + // legitimately going to stay that way for minutes. + await session.setStreamRequestIds("stream-paused", [9]); + + expect(await session.getStaleEpochStreamRequestIds()).toEqual([]); + }); + + it("reports a row left by a previous incarnation as stranded", async () => { + const storage = new MemoryStorage(); + const beforeReset = await makeHarnessSession(storage); + await beforeReset.setStreamRequestIds("stream-lost", [42]); + await beforeReset.setStreamRequestIds("stream-also-lost", ["abc"]); + + // The reset: same durable storage, a brand new Durable Object instance. + const afterReset = await makeHarnessSession(storage); + await afterReset.setStreamRequestIds("stream-live", [100]); + + const stranded = await afterReset.getStaleEpochStreamRequestIds(); + + // Order follows storage's key order, which this fake does not model, so + // the assertion is on the set. + expect( + [...stranded].sort((a, b) => a.streamId.localeCompare(b.streamId)), + "only the rows the dead incarnation accepted", + ).toMatchObject([ + { streamId: "stream-also-lost", requestIds: ["abc"] }, + { streamId: "stream-lost", requestIds: [42] }, + ]); + for (const row of stranded) expect(row.epoch).toBeLessThan(row.currentEpoch); + }); + + it("reports a pre-epoch ledger row as stranded", async () => { + const session = await makeHarnessSession(); + + // The shape rows had before they carried an epoch. One can only have been + // written by an earlier deployment, so it reads as epoch 0 and is swept. + await session.ctx.storage.put(ledgerKey("stream-legacy"), [7]); + + expect(await session.getStaleEpochStreamRequestIds()).toEqual([ + { + currentEpoch: await session.currentSessionEpoch(), + epoch: 0, + requestIds: [7], + streamId: "stream-legacy", + }, + ]); + expect( + await session.getStreamRequestIds("stream-legacy"), + "and it is still readable as a request-id list", + ).toEqual([7]); + }); + + it("holds the idle lease for a request the running incarnation still owes", async () => { + const session = await makeHarnessSession(); + await session.setStreamRequestIds("stream-live", [1]); + + await session.alarm(); + + expect(session.initialized, "live work keeps the runtime resident").toBe(true); + expect(session.ctx.alarm, "and re-arms the lease").toBeGreaterThan(0); + }); + + it("does not let a stranded row extend the idle lease", async () => { + const storage = new MemoryStorage(); + const beforeReset = await makeHarnessSession(storage); + await beforeReset.setStreamRequestIds("stream-lost", [1]); + + const afterReset = await makeHarnessSession(storage); + await afterReset.alarm(); + + expect( + afterReset.initialized, + "a request nothing will ever answer is dead work, not running work", + ).toBe(false); + }); +}); 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 993fab3783..578a64fe1d 100644 --- a/packages/hosts/cloudflare/src/mcp/agent-session-durable-object.ts +++ b/packages/hosts/cloudflare/src/mcp/agent-session-durable-object.ts @@ -203,7 +203,6 @@ const AGENTS_DESTROY_PENDING_KEY = "cf_agents_destroy_pending"; const MCP_HTTP_METHOD_HEADER = "cf-mcp-method"; const MCP_MESSAGE_HEADER = "cf-mcp-message"; const MODEL_RESUME_FORWARD_TIMEOUT_MS = 10_000; -const MCP_STREAM_REQS_KEY_PREFIX = "__mcp_stream_reqs__:"; const approvalResponseKey = (executionId: string) => `approval-response:${executionId}`; const BrowserApprovalDecisionStorage = Schema.Struct({ response: ResumeResponsePayload, @@ -752,13 +751,20 @@ export abstract class McpAgentSessionDOBase< // which survives disposeIdleRuntime, so a later reconnect GET re-inits the // DO and replays it. Counting them would make every delivered-but-unacked // POST response pin the runtime alive indefinitely. - const rows = await this.ctx.storage.list({ - prefix: MCP_STREAM_REQS_KEY_PREFIX, - limit: 1_000, - }); + // + // Rows stamped with an epoch older than this incarnation's are dead work, + // not running work: the isolate that was going to produce their response + // was reset, so nothing will ever answer them and they must not hold the + // runtime open. The transport's orphan sweep tells the client and removes + // the row on the next GET; until then they simply do not count. + const [openStreams, currentEpoch] = await Promise.all([ + this.getOpenStreamRequestIds(), + this.currentSessionEpoch(), + ]); let count = 0; - for (const requestIds of rows.values()) { - if (Array.isArray(requestIds)) count += requestIds.length; + for (const stream of openStreams) { + if (stream.epoch < currentEpoch) continue; + count += stream.requestIds.length; } return count; } diff --git a/packages/hosts/cloudflare/src/mcp/agents-post-stream-loss.test.ts b/packages/hosts/cloudflare/src/mcp/agents-post-stream-loss.test.ts new file mode 100644 index 0000000000..1681709ca6 --- /dev/null +++ b/packages/hosts/cloudflare/src/mcp/agents-post-stream-loss.test.ts @@ -0,0 +1,337 @@ +// Unit coverage for the POST bridge's lost-execution handling (see +// patches/agents@0.17.3.patch). +// +// The front worker answers a POST with HTTP 200 and an SSE body long before the +// Durable Object produces a result. When the DO is reset mid-execute (isolate +// memory/CPU limit, storage timeout, deploy) the bridge WebSocket closes, and +// the bridge used to simply close the writer: the client saw a cleanly +// terminated SSE stream carrying no JSON-RPC response, which is +// indistinguishable from "still working" until its own timeout fires. Same +// outcome if the DO never answers at all. +// +// Three properties are pinned here: +// 1. An abnormal WS close answers everything still outstanding with the +// -32010 session_reset error, then ends the stream. +// 2. The happy path is byte-identical — a delivered response leaves nothing +// outstanding, so nothing is synthesized. +// 3. The response deadline answers a stream the DO never replied on, and the +// constant that bounds it still clears the two source constants it has to +// track (it lives in a vendored dist and cannot import them). +import { readFileSync } from "node:fs"; + +import { afterEach, beforeEach, describe, expect, it, vi } from "@effect/vitest"; +import { MCP_POST_RESPONSE_DEADLINE_MS, McpAgent } from "agents/mcp"; +import { Effect, Option, Schema } from "effect"; + +import { PAUSED_APPROVAL_TIMEOUT_MS } from "@executor-js/host-mcp/tool-server"; + +const SESSION_RESET_ERROR_CODE = -32010; +/** Margin the patch adds on top of the two bounded waits. */ +const DEADLINE_MARGIN_MS = 60_000; + +type FakeWebSocket = EventTarget & { + accepted: boolean; + closeCode: number | undefined; + closeReason: string | undefined; + accept: () => void; + close: (code?: number, reason?: string) => void; + send: (message: string) => void; +}; + +const JsonRpcErrorFrame = Schema.Struct({ + error: Schema.Struct({ + code: Schema.Number, + data: Schema.Struct({ reason: Schema.String }), + message: Schema.String, + }), + id: Schema.Union([Schema.String, Schema.Number]), + jsonrpc: Schema.Literal("2.0"), +}); +const decodeJsonRpcErrorFrame = Schema.decodeUnknownOption( + Schema.fromJsonString(JsonRpcErrorFrame), +); + +/** Every `data:` payload in an SSE body that parses as a JSON-RPC error. */ +const errorFrames = (body: string): ReadonlyArray => + body.split("\n").flatMap((line) => { + if (!line.startsWith("data: ")) return []; + const decoded = decodeJsonRpcErrorFrame(line.slice("data: ".length)); + return Option.isSome(decoded) ? [decoded.value] : []; + }); + +const flushMicrotasks = async (): Promise => { + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); +}; + +const drainResponse = async (response: Response): Promise => { + const decoder = new TextDecoder(); + let body = ""; + + await Effect.runPromise( + Effect.ignore( + Effect.tryPromise({ + try: () => + response.body?.pipeTo( + new WritableStream({ + close: () => { + body += decoder.decode(); + }, + write: (chunk) => { + body += decoder.decode(chunk, { stream: true }); + }, + }), + ) ?? Promise.resolve(), + catch: () => undefined, + }), + ), + ); + + return body; +}; + +const makeExecutionContext = (): ExecutionContext => ({ + passThroughOnException: () => {}, + props: undefined, + waitUntil: () => {}, +}); + +const makeWebSocket = (): FakeWebSocket => { + const ws = new EventTarget() as FakeWebSocket; + ws.accepted = false; + ws.closeCode = undefined; + ws.closeReason = undefined; + ws.accept = () => { + ws.accepted = true; + }; + ws.close = (code?: number, reason?: string) => { + ws.closeCode = code; + ws.closeReason = reason; + }; + ws.send = () => {}; + return ws; +}; + +const makeNamespace = (ws: FakeWebSocket) => ({ + newUniqueId: () => ({ toString: () => "generated-session" }), + idFromName: (name: string) => ({ equals: () => true, name, toString: () => name }), + get: () => ({ + setName: async () => {}, + getInitializeRequest: async () => ({}), + fetch: async () => ({ webSocket: ws }), + }), +}); + +const postToBridge = async (body: unknown) => { + const ws = makeWebSocket(); + const handler = McpAgent.serve("/mcp", { + binding: "MCP_SESSION", + transport: "streamable-http", + }); + const response = await handler.fetch( + new Request("https://executor.sh/mcp", { + body: JSON.stringify(body), + headers: { + accept: "application/json, text/event-stream", + "content-type": "application/json", + "mcp-session-id": "session-1", + }, + method: "POST", + }), + { MCP_SESSION: makeNamespace(ws) }, + makeExecutionContext(), + ); + return { response, ws }; +}; + +const toolCall = (id: number | string) => ({ + id, + jsonrpc: "2.0" as const, + method: "tools/call", + params: { arguments: {}, name: "execute" }, +}); + +/** + * Deliver one `cf_mcp_agent_event` envelope exactly as the patched + * `writeSSEEvent` builds it, including the `respondedIds` field the bridge uses + * to retire outstanding ids. + */ +const emitResponse = (ws: FakeWebSocket, message: { readonly id: number | string }, close = true) => + ws.dispatchEvent( + new MessageEvent("message", { + data: JSON.stringify({ + close: close ? true : undefined, + event: `event: message\ndata: ${JSON.stringify(message)}\n\n`, + respondedIds: [message.id], + type: "cf_mcp_agent_event", + }), + }), + ); + +/** + * A Durable Object reset closes the bridge socket with no close code at all — + * `CloseEvent` is not reliably constructible across runtimes, so the shape the + * handler actually reads (`code` / `reason`) is attached directly. + */ +const emitAbnormalClose = (ws: FakeWebSocket, code?: number, reason?: string) => + ws.dispatchEvent(Object.assign(new Event("close"), { code, reason })); + +describe("POST bridge: lost-execution visibility", () => { + beforeEach(() => { + vi.useFakeTimers(); + vi.setSystemTime(0); + vi.spyOn(console, "warn").mockImplementation(() => {}); + }); + + afterEach(() => { + vi.restoreAllMocks(); + vi.useRealTimers(); + }); + + it("answers every outstanding request when the bridge socket closes abnormally", async () => { + const { response, ws } = await postToBridge([toolCall(1), toolCall("two")]); + const drained = drainResponse(response); + + emitAbnormalClose(ws); + const body = await drained; + + const frames = errorFrames(body); + expect( + frames.map((frame) => frame.id), + "one error per unanswered request id, preserving the id's JSON type", + ).toEqual([1, "two"]); + for (const frame of frames) { + expect(frame.error.code).toBe(SESSION_RESET_ERROR_CODE); + expect(frame.error.data.reason).toBe("session_reset"); + expect(frame.error.message).toContain("Execution lost"); + } + }); + + it("writes nothing extra when the response was delivered before the close", async () => { + const { response, ws } = await postToBridge(toolCall(1)); + const drained = drainResponse(response); + + const result = { id: 1, jsonrpc: "2.0" as const, result: { ok: true } }; + emitResponse(ws, result); + await flushMicrotasks(); + emitAbnormalClose(ws); + const body = await drained; + + expect(body, "the delivered result is the only frame on the wire").toBe( + `event: message\ndata: ${JSON.stringify(result)}\n\n`, + ); + expect(errorFrames(body)).toEqual([]); + expect(ws.closeCode, "the happy-path close is unchanged").toBe(1000); + expect(ws.closeReason).toBe("SSE response delivered"); + }); + + it("answers the request itself when the DO never responds before the deadline", async () => { + const { response, ws } = await postToBridge(toolCall(7)); + const drained = drainResponse(response); + + await vi.advanceTimersByTimeAsync(MCP_POST_RESPONSE_DEADLINE_MS - 1); + expect(errorFrames(await Promise.race([drained, Promise.resolve("")]))).toEqual([]); + + await vi.advanceTimersByTimeAsync(2); + const body = await drained; + + const frames = errorFrames(body); + expect(frames.map((frame) => frame.id)).toEqual([7]); + expect(frames[0]?.error.code).toBe(SESSION_RESET_ERROR_CODE); + expect(frames[0]?.error.data.reason).toBe("response_deadline"); + expect(vi.getTimerCount(), "the deadline and the keepalive are both disarmed").toBe(0); + + // A close arriving after the bridge already answered must not double-write. + emitAbnormalClose(ws); + await flushMicrotasks(); + expect(errorFrames(body).length).toBe(1); + }); + + it("arms no deadline for a body that owes no response", async () => { + const { response } = await postToBridge({ + jsonrpc: "2.0", + method: "notifications/initialized", + }); + + expect(response.status).toBe(202); + expect(vi.getTimerCount(), "a notifications-only POST leaves no timer behind").toBe(0); + }); +}); + +describe("POST response deadline: drift against the waits it must cover", () => { + /** + * Evaluate the small `a * b + c` expressions these constants are declared + * with. Anything outside digits, `_`, `*`, `+` and the named substitutions is + * refused, so a declaration that grows a new shape fails the test loudly + * instead of silently reading as `NaN`. + */ + const evaluate = (expression: string, known: Readonly>): number | null => { + let total = 0; + for (const term of expression.split("+")) { + let product = 1; + for (const rawFactor of term.split("*")) { + const factor = rawFactor.trim(); + const named = known[factor]; + if (named !== undefined) { + product *= named; + continue; + } + if (!/^\d[\d_]*$/.test(factor)) return null; + product *= Number(factor.replaceAll("_", "")); + } + total += product; + } + return total; + }; + + /** + * Read a constant out of another package's source. Deliberately a file read + * and not an import: `DEFAULT_TIMEOUT_MS` is private to the dynamic-worker + * runtime, the bridge constant that has to cover it lives in a vendored dist + * that can import neither, and this test exists precisely to notice when the + * two drift apart. + */ + const readConstant = ( + relativePath: string, + name: string, + known: Readonly> = {}, + ): number | null => { + const source = readFileSync(new URL(relativePath, import.meta.url), "utf8"); + const match = new RegExp(`\\b${name}\\s*=\\s*([^;\\n]+);`).exec(source); + return match?.[1] === undefined ? null : evaluate(match[1], known); + }; + + const RUNTIME_EXECUTOR = "../../../../kernel/runtime-dynamic-worker/src/executor.ts"; + const TOOL_SERVER = "../../../mcp/src/tool-server.ts"; + + it("covers the sandbox execution ceiling plus the browser-approval wait plus margin", () => { + const sandboxTimeoutMs = readConstant(RUNTIME_EXECUTOR, "DEFAULT_TIMEOUT_MS"); + expect(sandboxTimeoutMs, `DEFAULT_TIMEOUT_MS not readable from ${RUNTIME_EXECUTOR}`).not.toBe( + null, + ); + + // The scrape is kept honest by the one constant that IS a public export: + // if the regex ever stops reading these declarations correctly, this + // mismatch fires rather than the test passing on a wrong number. + const scrapedPausedApprovalMs = readConstant(TOOL_SERVER, "PAUSED_APPROVAL_TIMEOUT_MS"); + expect(scrapedPausedApprovalMs, "scraped value must match the real export").toBe( + PAUSED_APPROVAL_TIMEOUT_MS, + ); + + const browserApprovalWaitMs = readConstant(TOOL_SERVER, "BROWSER_APPROVAL_WAIT_TIMEOUT_MS", { + PAUSED_APPROVAL_TIMEOUT_MS, + }); + expect( + browserApprovalWaitMs, + `BROWSER_APPROVAL_WAIT_TIMEOUT_MS not readable from ${TOOL_SERVER}`, + ).not.toBe(null); + + const required = (sandboxTimeoutMs ?? 0) + (browserApprovalWaitMs ?? 0) + DEADLINE_MARGIN_MS; + expect( + MCP_POST_RESPONSE_DEADLINE_MS, + "the bridge would answer for a call that is still legitimately running — raise " + + "MCP_POST_RESPONSE_DEADLINE_MS in patches/agents@0.17.3.patch", + ).toBeGreaterThanOrEqual(required); + }); +}); diff --git a/packages/hosts/cloudflare/src/mcp/agents-sse-max-age.test.ts b/packages/hosts/cloudflare/src/mcp/agents-sse-max-age.test.ts index 5a603b47bf..eb8717ee41 100644 --- a/packages/hosts/cloudflare/src/mcp/agents-sse-max-age.test.ts +++ b/packages/hosts/cloudflare/src/mcp/agents-sse-max-age.test.ts @@ -1,5 +1,5 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "@effect/vitest"; -import { MAX_SSE_AGE_MS, McpAgent } from "agents/mcp"; +import { MAX_SSE_AGE_MS, MCP_POST_RESPONSE_DEADLINE_MS, McpAgent } from "agents/mcp"; import { Effect, Option, Schema } from "effect"; import { SESSION_TIMEOUT_MS } from "./session-alarm-policy"; @@ -377,12 +377,17 @@ describe("agents SSE max-age rotation", () => { await expect(drained).resolves.toContain(": max-age rotation, reconnect\n\n"); }); - it("does not rotate an in-flight POST response past max age", async () => { + // Max-age rotation applies only to GET streams. A POST is bounded by its own, + // much shorter response deadline (MCP_POST_RESPONSE_DEADLINE_MS — see + // agents-post-stream-loss.test.ts), so max age is never reachable on one; this + // pins that rotation stays out of the way for as long as a POST may legitimately + // run. + it("does not rotate an in-flight POST response", async () => { const { response, ws } = await openPostSse(); const drained = drainResponse(response); emitAgentEvent(ws, `event: message\ndata: {"jsonrpc":"2.0","id":1,"result":{}}\n\n`); - await vi.advanceTimersByTimeAsync(MAX_SSE_AGE_MS + KEEPALIVE_INTERVAL_MS * 4); + await vi.advanceTimersByTimeAsync(MCP_POST_RESPONSE_DEADLINE_MS - KEEPALIVE_INTERVAL_MS); await flushMicrotasks(); expect(ws.closeCode).toBeUndefined(); diff --git a/packages/hosts/cloudflare/src/mcp/durable-object-errors.test.ts b/packages/hosts/cloudflare/src/mcp/durable-object-errors.test.ts index ab939d3a3a..0b350a3506 100644 --- a/packages/hosts/cloudflare/src/mcp/durable-object-errors.test.ts +++ b/packages/hosts/cloudflare/src/mcp/durable-object-errors.test.ts @@ -91,11 +91,20 @@ describe("classifyDurableObjectError", () => { ).toEqual({ kind: "cpu_limit", disposition: "transient" }); }); - // The memory-limit reset is the CPU limit's sibling and is deliberately NOT - // classified: the runtime names the application as the cause (un-awaited - // writes, an oversized read), so a retry reproduces it. It has to keep being - // rethrown and reported rather than disappearing into a 503. - it("refuses to classify the sibling memory-limit reset as retryable", () => { + it("reads a plain memory-limit reset as transient", () => { + expect( + classifyDurableObjectError( + new Error("Durable Object's isolate exceeded its memory limit and was reset."), + ), + ).toEqual({ kind: "memory_limit", disposition: "transient" }); + }); + + // The storage-cache variant shares the "exceeded its memory limit" phrase but + // is a USER error the runtime names the cause of (un-awaited writes, an + // oversized read), so a retry reproduces it. It has to keep being rethrown and + // reported rather than disappearing into a 503 — the qualifier is the only + // thing separating it from the platform reset above. + it("refuses to classify the storage-cache memory-limit variant as retryable", () => { expect( classifyDurableObjectError( new Error( diff --git a/packages/hosts/cloudflare/src/mcp/durable-object-errors.ts b/packages/hosts/cloudflare/src/mcp/durable-object-errors.ts index 0eee14c2b6..5d0434aa26 100644 --- a/packages/hosts/cloudflare/src/mcp/durable-object-errors.ts +++ b/packages/hosts/cloudflare/src/mcp/durable-object-errors.ts @@ -41,6 +41,8 @@ export type DurableObjectFailureKind = | "concurrency_reset" /** An invocation ran past the per-invocation CPU ceiling; the object was reset. */ | "cpu_limit" + /** The isolate ran past its memory ceiling; every object in it was reset. */ + | "memory_limit" /** A generic platform blip: `internal error; reference = `. */ | "internal_error" /** The runtime itself flagged the error as retryable. */ @@ -68,6 +70,14 @@ export type DurableObjectFailure = { */ const MESSAGE_PATTERNS: ReadonlyArray<{ readonly fragment: string; + /** + * A second fragment that, when also present, *disqualifies* the match. One + * runtime phrase covers both a platform reset and an application defect, and + * the qualifier the runtime adds for the defect is the only thing separating + * them — so the entry names that qualifier rather than trying to spell out + * every benign variant of the shared phrase. + */ + readonly excludeFragment?: string; readonly failure: DurableObjectFailure; }> = [ { @@ -125,6 +135,28 @@ const MESSAGE_PATTERNS: ReadonlyArray<{ fragment: "exceeded its cpu time limit and was reset", failure: { kind: "cpu_limit", disposition: "transient" }, }, + { + // The CPU limit's sibling, matched on the short shared phrase rather than a + // full sentence because the runtime words its two memory-limit messages + // differently after it. + // + // `excludeFragment` carries what used to keep this bucket out of the list + // entirely: the runtime reuses "exceeded its memory limit" for a USER error + // that names its own cause — "… due to overflowing the storage cache. All + // objects in the isolate were reset." — too many un-awaited writes, or one + // oversized read. That variant reproduces on every retry, so calling it + // transient would bury an application defect behind a 503. It still falls + // through unclassified and keeps being rethrown and reported. + // + // What is left is the plain reset, transient for exactly the reasons the + // CPU limit is: nothing about the request caused it, durable storage is + // untouched, and the session id still routes. Leaving it unclassified is + // what let a memory-limit reset mid-execute fall out of the worker as an + // unhandled 500. + fragment: "exceeded its memory limit", + excludeFragment: "overflowing the storage cache", + failure: { kind: "memory_limit", disposition: "transient" }, + }, { // Only the bare blip. A reference id at the end of the message is NOT the // marker: the runtime also appends one to described faults such as the @@ -136,12 +168,6 @@ const MESSAGE_PATTERNS: ReadonlyArray<{ fragment: "internal error; reference =", failure: { kind: "internal_error", disposition: "transient" }, }, - // Not listed, on purpose: the sibling memory-limit reset ("Durable Object's - // isolate exceeded its memory limit due to overflowing the storage cache … - // All objects in the isolate were reset."). The runtime tags that one as a - // user error, and it names its own cause — too many un-awaited writes, or one - // oversized read. Retrying reproduces it, so calling it transient would bury - // an application defect behind a 503 instead of surfacing it. ]; /** @@ -175,7 +201,11 @@ const classifyOne = (error: unknown): DurableObjectFailure | null => { return { kind: "destroyed", disposition: "session_dead" }; } for (const pattern of MESSAGE_PATTERNS) { - if (normalized.includes(pattern.fragment)) return pattern.failure; + if (!normalized.includes(pattern.fragment)) continue; + if (pattern.excludeFragment !== undefined && normalized.includes(pattern.excludeFragment)) { + continue; + } + return pattern.failure; } } // Checked last: the message is the more specific signal, and the runtime sets diff --git a/patches/agents@0.17.3.patch b/patches/agents@0.17.3.patch index 0378a5a20d..7eec44acf0 100644 --- a/patches/agents@0.17.3.patch +++ b/patches/agents@0.17.3.patch @@ -4,11 +4,17 @@ index 0000000000000000000000000000000000000000..e69de29bb2d1d6434b8b29ae775ad8c2 diff --git a/node_modules/agents/.bun-tag-61b2f1517ab5ced4 b/.bun-tag-61b2f1517ab5ced4 new file mode 100644 index 0000000000000000000000000000000000000000..e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 +diff --git a/node_modules/agents/.bun-tag-61d29b56a6079f1d b/.bun-tag-61d29b56a6079f1d +new file mode 100644 +index 0000000000000000000000000000000000000000..e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 +diff --git a/node_modules/agents/.bun-tag-a6a47855632a2623 b/.bun-tag-a6a47855632a2623 +new file mode 100644 +index 0000000000000000000000000000000000000000..e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 diff --git a/node_modules/agents/.bun-tag-c0c639aa2299e502 b/.bun-tag-c0c639aa2299e502 new file mode 100644 index 0000000000000000000000000000000000000000..e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 diff --git a/dist/agent-tool-types-CNyE1iz_.d.ts b/dist/agent-tool-types-CNyE1iz_.d.ts -index 571eececebd5a1eaf7f2fbf5278801c4d34728ba..317e526489fcd3fd477a50525da0df666253bf0b 100644 +index 571eececebd5a1eaf7f2fbf5278801c4d34728ba..010811ae74ef2d27f12f7fb2506848f1939d91d6 100644 --- a/dist/agent-tool-types-CNyE1iz_.d.ts +++ b/dist/agent-tool-types-CNyE1iz_.d.ts @@ -480,7 +480,10 @@ declare class DurableObjectEventStore implements EventStore { @@ -23,31 +29,76 @@ index 571eececebd5a1eaf7f2fbf5278801c4d34728ba..317e526489fcd3fd477a50525da0df66 getStreamIdForEventId(eventId: EventId): Promise; replayEventsAfter( lastEventId: EventId, +@@ -538,6 +541,42 @@ declare abstract class McpAgent< + * @internal + */ + private static readonly STREAM_REQS_KEY_PREFIX; ++ /** ++ * Monotonic id for THIS Durable Object incarnation, bumped once per live ++ * instance. Ledger rows are stamped with it so a row that outlived the ++ * isolate which was going to answer it is recognisable as stale. ++ * ++ * @internal ++ */ ++ currentSessionEpoch(): Promise; ++ /** ++ * Ledger rows stamped by an earlier incarnation: requests that nothing will ++ * ever answer. Read-only — the caller writes the client's error first. ++ * ++ * @internal ++ */ ++ getStaleEpochStreamRequestIds(): Promise< ++ ReadonlyArray<{ ++ readonly streamId: string; ++ readonly requestIds: RequestId[]; ++ readonly epoch: number; ++ readonly currentEpoch: number; ++ }> ++ >; ++ /** ++ * Every POST stream still awaiting a response, with the incarnation that ++ * accepted it. Callers that mean "work still running" must drop rows whose ++ * `epoch` is below {@link currentSessionEpoch}. ++ * ++ * @internal ++ */ ++ getOpenStreamRequestIds(): Promise< ++ ReadonlyArray<{ ++ readonly streamId: string; ++ readonly requestIds: RequestId[]; ++ readonly epoch: number; ++ }> ++ >; + /** Persist the `requestIds` for a POST stream. @internal */ + setStreamRequestIds(streamId: string, requestIds: RequestId[]): Promise; + /** Read the persisted `requestIds` for a POST stream. @internal */ diff --git a/dist/mcp/index.d.ts b/dist/mcp/index.d.ts -index c8fad448e8797b89690a99d93490d1363851b225..77f9fe3f6f2375eadc9f7a2974f0d202fe75b3bd 100644 +index c8fad448e8797b89690a99d93490d1363851b225..d80f66f1532c29dda0a0477dc1ec9ef5fd1fca47 100644 --- a/dist/mcp/index.d.ts +++ b/dist/mcp/index.d.ts -@@ -29,6 +29,7 @@ import { +@@ -29,6 +29,8 @@ import { xt as MCPClientOAuthCallbackConfig, zt as ElicitResult } from "../agent-tool-types-CNyE1iz_.js"; +declare const MAX_SSE_AGE_MS = 1800000; ++declare const MCP_POST_RESPONSE_DEADLINE_MS = 660000; export { type ClearableEventStore, type CreateMcpHandlerOptions, -@@ -41,6 +42,7 @@ export { +@@ -41,6 +43,8 @@ export { type MCPConnectionResult, type MCPDiscoverResult, type MCPServerOptions, + MAX_SSE_AGE_MS, ++ MCP_POST_RESPONSE_DEADLINE_MS, MCP_SERVER_ID_MAX_LENGTH, McpAgent, type McpAuthContext, diff --git a/dist/mcp/index.js b/dist/mcp/index.js -index 1edcf0c8c9e67aa211ae515e7672cdf79912101e..cb04fc39d14db9f7fcdfa2613b1c5babd1219e17 100644 +index 1edcf0c8c9e67aa211ae515e7672cdf79912101e..231220c26b995a48e40dba993773c28786cca392 100644 --- a/dist/mcp/index.js +++ b/dist/mcp/index.js -@@ -28,13 +28,17 @@ import { WebStandardStreamableHTTPServerTransport } from "@modelcontextprotocol/ +@@ -28,13 +28,60 @@ import { WebStandardStreamableHTTPServerTransport } from "@modelcontextprotocol/ const KEEPALIVE_INTERVAL_MS = 25e3; /** SSE comment frame the parser drops before any event dispatch. */ const KEEPALIVE_FRAME = ": keepalive\n\n"; @@ -55,6 +106,49 @@ index 1edcf0c8c9e67aa211ae515e7672cdf79912101e..cb04fc39d14db9f7fcdfa2613b1c5bab +// well above Executor's 5 minute session idle timeout so active clients rarely +// rotate. +const MAX_SSE_AGE_MS = 30 * 60 * 1000; ++/** ++* JSON-RPC error code for "the isolate that was running this call is gone". ++* In the implementation-defined server range (-32000..-32099) and deliberately ++* NOT the -32001 the worker already uses for session/transport failures, so a ++* client can tell "the call you are waiting on died mid-flight, re-run it" from ++* "this session id is dead, reconnect". ++*/ ++const SESSION_RESET_ERROR_CODE = -32010; ++const SESSION_RESET_ERROR_MESSAGE = "Execution lost: the session was reset before it produced a result. Re-run the call."; ++/** ++* How long the POST bridge waits for the Durable Object to answer a JSON-RPC ++* request before answering for it. ++* ++* This file is a vendored dist and cannot import the two constants that ++* actually bound how long an answer may legitimately take, so this MUST be kept ++* at or above their sum: ++* ++* DEFAULT_TIMEOUT_MS packages/kernel/runtime-dynamic-worker/src/executor.ts ++* (5 min sandbox execution ceiling) ++* BROWSER_APPROVAL_WAIT_TIMEOUT_MS packages/hosts/mcp/src/tool-server.ts ++* (PAUSED_APPROVAL_TIMEOUT_MS + 1s = 4 min 1 s) ++* ++* plus ~60s of margin for DO restore, storage and edge latency. 11 minutes ++* clears 10 min 1 s. Drift is caught by ++* packages/hosts/cloudflare/src/mcp/agents-post-stream-loss.test.ts, ++* which recomputes the sum from those two source files. ++*/ ++const MCP_POST_RESPONSE_DEADLINE_MS = 11 * 60 * 1000; ++/** ++* The JSON-RPC error response sent for a request that will never get a real ++* answer. `reason` distinguishes the two ways we find out: `session_reset` ++* (the DO end of the bridge went away, or a ledger row outlived its ++* incarnation) and `response_deadline` (nothing came back in time). ++*/ ++const sessionResetErrorResponse = (id, reason) => ({ ++ error: { ++ code: SESSION_RESET_ERROR_CODE, ++ data: { reason }, ++ message: SESSION_RESET_ERROR_MESSAGE ++ }, ++ id, ++ jsonrpc: "2.0" ++}); /** * Start an SSE keepalive on `writer`. Returns a `clearInterval` handle * that the stream cleanup must invoke when the stream closes. @@ -67,7 +161,7 @@ index 1edcf0c8c9e67aa211ae515e7672cdf79912101e..cb04fc39d14db9f7fcdfa2613b1c5bab }, KEEPALIVE_INTERVAL_MS); return handle; } -@@ -180,10 +184,15 @@ const createStreamingHttpHandler = (basePath, namespace, options = {}) => { +@@ -180,10 +227,30 @@ const createStreamingHttpHandler = (basePath, namespace, options = {}) => { }); return new Response(body, { status: 404 }); } @@ -83,11 +177,26 @@ index 1edcf0c8c9e67aa211ae515e7672cdf79912101e..cb04fc39d14db9f7fcdfa2613b1c5bab + let __writeChain = Promise.resolve(); + let __sseClosed = false; + let keepAlive; ++ let __responseDeadline; ++ /** ++ * JSON-RPC ids this POST still owes the client a response for. ++ * Entries are removed as the DO reports responses (the ++ * `respondedIds` field writeSSEEvent puts on the envelope). ++ * ++ * A non-empty set when the bridge finishes abnormally is exactly ++ * the "lost execution" case: the client already holds a 200 with ++ * an open SSE body, so a silently ended stream leaves it waiting ++ * forever. Keyed by type+value because JSON-RPC ids may be ++ * strings or numbers and 1 must not match "1". ++ */ ++ const __outstanding = new Map(); ++ const __idKey = (id) => `${typeof id}:${String(id)}`; ++ let __finished = false; + const existingHeaders = {}; request.headers.forEach((value, key) => { existingHeaders[key] = value; }); -@@ -206,45 +215,99 @@ const createStreamingHttpHandler = (basePath, namespace, options = {}) => { +@@ -206,47 +273,159 @@ const createStreamingHttpHandler = (basePath, namespace, options = {}) => { jsonrpc: "2.0" }); return new Response(body, { status: 500 }); @@ -101,8 +210,10 @@ index 1edcf0c8c9e67aa211ae515e7672cdf79912101e..cb04fc39d14db9f7fcdfa2613b1c5bab + const __closeSse = () => { + if (__sseClosed) return; + __sseClosed = true; ++ __finished = true; + try { + clearInterval(keepAlive); ++ clearTimeout(__responseDeadline); + } catch {} + try { + ws.close(1013, "SSE client not draining"); @@ -112,8 +223,10 @@ index 1edcf0c8c9e67aa211ae515e7672cdf79912101e..cb04fc39d14db9f7fcdfa2613b1c5bab + const __markSseClientClosed = () => { + if (__sseClosed) return; + __sseClosed = true; ++ __finished = true; + try { + clearInterval(keepAlive); ++ clearTimeout(__responseDeadline); + } catch {} + writer.abort(new Error("SSE client disconnected")).catch(() => {}); + }; @@ -148,6 +261,44 @@ index 1edcf0c8c9e67aa211ae515e7672cdf79912101e..cb04fc39d14db9f7fcdfa2613b1c5bab - ws.addEventListener("message", (event) => { + }); + } ++ for (const msg of messages) if (isJSONRPCRequest(msg)) __outstanding.set(__idKey(msg.id), msg.id); ++ /** ++ * End the response stream when it will not end on its own, and ++ * answer whatever is still outstanding first. ++ * ++ * The Durable Object can be reset out from under an in-flight ++ * execute (isolate memory/CPU limit, storage timeout, deploy). The ++ * front worker has already returned HTTP 200 with an SSE body, so ++ * before this the WS close simply closed the writer and the client ++ * saw a cleanly terminated stream carrying no JSON-RPC response — ++ * indistinguishable from "still working" until its own timeout. ++ * ++ * The happy path is byte-identical: every id has been reported ++ * responded by then, so the set is empty and nothing is written. ++ */ ++ const __finishAbnormally = async (reason) => { ++ if (__finished) return; ++ __finished = true; ++ clearTimeout(__responseDeadline); ++ clearInterval(keepAlive); ++ if (__outstanding.size > 0) { ++ console.warn(JSON.stringify({ ++ event: "mcp_post_stream_lost", ++ outstandingCount: __outstanding.size, ++ reason, ++ sessionId ++ })); ++ for (const id of __outstanding.values()) __forwardSse(encoder.encode(`event: message\ndata: ${JSON.stringify(sessionResetErrorResponse(id, reason))}\n\n`)); ++ __outstanding.clear(); ++ } ++ const pending = __writeChain; ++ __sseClosed = true; ++ await pending.catch(() => {}); ++ await writer.close().catch(() => {}); ++ }; ++ __responseDeadline = setTimeout(() => { ++ __finishAbnormally("response_deadline").catch(console.error); ++ }, MCP_POST_RESPONSE_DEADLINE_MS); + ws.addEventListener("message", (event) => { async function onMessage(event) { try { @@ -163,8 +314,11 @@ index 1edcf0c8c9e67aa211ae515e7672cdf79912101e..cb04fc39d14db9f7fcdfa2613b1c5bab + const data = typeof event.data === "string" ? event.data : new TextDecoder().decode(event.data); + const message = JSON.parse(data); + if (message.type !== "cf_mcp_agent_event") return; ++ if (Array.isArray(message.respondedIds)) for (const respondedId of message.respondedIds) __outstanding.delete(__idKey(respondedId)); + const writePromise = __forwardSse(encoder.encode(message.event)); + if (message.close) { ++ __finished = true; ++ clearTimeout(__responseDeadline); + clearInterval(keepAlive); + await writePromise; + await writer.close(); @@ -195,9 +349,9 @@ index 1edcf0c8c9e67aa211ae515e7672cdf79912101e..cb04fc39d14db9f7fcdfa2613b1c5bab - await writer.close().catch(() => {}); + ws.addEventListener("error", (error) => { + async function onError(_error) { -+ __sseClosed = true; -+ clearInterval(keepAlive); -+ await writer.close().catch(() => {}); ++ // The DO end of the bridge failed. Nothing still ++ // outstanding will ever be answered. ++ await __finishAbnormally("session_reset"); } onError(error).catch(console.error); }); @@ -205,15 +359,31 @@ index 1edcf0c8c9e67aa211ae515e7672cdf79912101e..cb04fc39d14db9f7fcdfa2613b1c5bab - async function onClose() { - clearInterval(keepAlive); - await writer.close().catch(() => {}); -+ ws.addEventListener("close", () => { -+ async function onClose() { -+ __sseClosed = true; -+ clearInterval(keepAlive); -+ await writer.close().catch(() => {}); ++ ws.addEventListener("close", (event) => { ++ async function onClose(closeEvent) { ++ // 1000/"SSE response delivered" is this bridge's OWN ++ // happy-path close coming back round. Every other code — ++ // including no code at all, which is what a reset Durable ++ // Object produces — means the far end went away, so ++ // anything unanswered gets a session_reset error. The ++ // outstanding set makes this safe either way: after a ++ // normal delivery it is empty and nothing is written. ++ if (closeEvent?.code === 1000 && closeEvent?.reason === "SSE response delivered") { ++ __finished = true; ++ clearTimeout(__responseDeadline); ++ __sseClosed = true; ++ clearInterval(keepAlive); ++ await writer.close().catch(() => {}); ++ return; ++ } ++ await __finishAbnormally("session_reset"); } - onClose().catch(console.error); +- onClose().catch(console.error); ++ onClose(event).catch(console.error); }); -@@ -279,10 +342,16 @@ const createStreamingHttpHandler = (basePath, namespace, options = {}) => { + return new Response(readable, { + headers: { +@@ -279,10 +458,16 @@ const createStreamingHttpHandler = (basePath, namespace, options = {}) => { id: null, jsonrpc: "2.0" }), { status: 400 }); @@ -234,7 +404,7 @@ index 1edcf0c8c9e67aa211ae515e7672cdf79912101e..cb04fc39d14db9f7fcdfa2613b1c5bab props: ctx.props, jurisdiction: options.jurisdiction }); -@@ -306,27 +375,116 @@ const createStreamingHttpHandler = (basePath, namespace, options = {}) => { +@@ -306,27 +491,116 @@ const createStreamingHttpHandler = (basePath, namespace, options = {}) => { if (!ws) { await writer.close(); return new Response("Failed to establish WS to DO", { status: 500 }); @@ -366,7 +536,7 @@ index 1edcf0c8c9e67aa211ae515e7672cdf79912101e..cb04fc39d14db9f7fcdfa2613b1c5bab return new Response(readable, { headers: { "Cache-Control": "no-cache", -@@ -389,10 +547,16 @@ const createLegacySseHandler = (basePath, namespace, options = {}) => { +@@ -389,10 +663,16 @@ const createLegacySseHandler = (basePath, namespace, options = {}) => { const url = new URL(request.url); if (request.method === "GET" && basePattern.test(url)) { const sessionId = url.searchParams.get("sessionId") || namespace.newUniqueId().toString(); @@ -387,7 +557,7 @@ index 1edcf0c8c9e67aa211ae515e7672cdf79912101e..cb04fc39d14db9f7fcdfa2613b1c5bab endpointUrl.pathname = encodeURI(`${basePath}/message`); endpointUrl.searchParams.set("sessionId", sessionId); const endpointMessage = `event: endpoint\ndata: ${endpointUrl.pathname + endpointUrl.search + endpointUrl.hash}\n\n`; -@@ -414,35 +578,94 @@ const createLegacySseHandler = (basePath, namespace, options = {}) => { +@@ -414,35 +694,94 @@ const createLegacySseHandler = (basePath, namespace, options = {}) => { console.error("Failed to establish WebSocket connection"); await writer.close(); return new Response("Failed to establish WebSocket connection", { status: 500 }); @@ -500,15 +670,16 @@ index 1edcf0c8c9e67aa211ae515e7672cdf79912101e..cb04fc39d14db9f7fcdfa2613b1c5bab console.error("Error closing SSE connection:", error); } } -@@ -586,6 +809,7 @@ var StreamableHTTPServerTransport = class { +@@ -586,6 +925,8 @@ var StreamableHTTPServerTransport = class { constructor(options) { this._started = false; this._streamResponseIds = /* @__PURE__ */ new Map(); + this._replayInFlight = /* @__PURE__ */ new Map(); ++ this._sweptStaleStreams = /* @__PURE__ */ new Set(); const { agent } = getCurrentAgent(); if (!agent) throw new Error("McpAgent was not found in Transport constructor"); this._agent = agent; -@@ -627,23 +851,74 @@ var StreamableHTTPServerTransport = class { +@@ -627,23 +968,146 @@ var StreamableHTTPServerTransport = class { const resumedStreamId = await this._eventStore.getStreamIdForEventId?.(lastEventId); if (resumedStreamId) { const resumeState = { streamId: resumedStreamId }; @@ -535,6 +706,14 @@ index 1edcf0c8c9e67aa211ae515e7672cdf79912101e..cb04fc39d14db9f7fcdfa2613b1c5bab + const ackStreamIds = []; + const replayedResponse = await this.replayEvents(lastEventId); + if (resumedStreamId !== STANDALONE_STREAM_ID && replayedResponse) ackStreamIds.push(resumedStreamId); ++ // Nothing to replay on a POST stream means one of two things: the ++ // work is still running in THIS incarnation, in which case the ++ // stream must stay attached so the response routes here, or the ++ // incarnation that owned it is gone and no response will ever ++ // exist. Only the second is swept, and only for the stream this ++ // cursor names. Before this, that case fell through with no close ++ // frame and the recovery GET hung open on keepalives. ++ if (resumedStreamId !== STANDALONE_STREAM_ID && !replayedResponse && (await this.failStaleEpochRequests(agent, connection, resumedStreamId)).length > 0) ackStreamIds.push(resumedStreamId); + // Last-Event-ID identifies one disconnected stream. Keep replay + // scoped to that stream: mixing another POST's response into this + // recovery stream can make a client stop reconnecting before the @@ -565,12 +744,17 @@ index 1edcf0c8c9e67aa211ae515e7672cdf79912101e..cb04fc39d14db9f7fcdfa2613b1c5bab }; connection.setState(standaloneState); + const replayedStreamIds = await this.replayUndeliveredResponsesOnFreshGet(agent, connection); ++ // The standalone stream is also where orphans go. A POST stranded by a ++ // reset may have no client left holding its cursor (its own recovery GET ++ // never came), so every stale-epoch id is answered here instead. Both ++ // sweeps delete the row, so an id is answered at most once. ++ const staleStreamIds = await this.failStaleEpochRequests(agent, connection); + // A GET without Last-Event-ID has no identified stream to preserve, so + // retain the existing fallback that drains completed POST responses. + // Same delivery-confirmed clearing as the resume branch above. When -+ // nothing was replayed no close frame is sent and this stays the -+ // session's long-lived standalone listener. -+ if (replayedStreamIds.length > 0) this.sendReplayComplete(connection, replayedStreamIds); ++ // nothing was replayed or swept no close frame is sent and this stays ++ // the session's long-lived standalone listener. ++ if (replayedStreamIds.length > 0 || staleStreamIds.length > 0) this.sendReplayComplete(connection, [...replayedStreamIds, ...staleStreamIds]); + } + /** + * Ask the Worker bridge to flush everything queued on `connection`, @@ -586,10 +770,69 @@ index 1edcf0c8c9e67aa211ae515e7672cdf79912101e..cb04fc39d14db9f7fcdfa2613b1c5bab + ackStreamIds, + close: true + })); ++ } ++ /** ++ * Answer every request left stranded by a PREVIOUS Durable Object ++ * incarnation, and retire its ledger row. ++ * ++ * A `__mcp_stream_reqs__:` row exists only while a POST's requests ++ * are unanswered — writing the final response deletes it. Each row is stamped ++ * with the epoch of the incarnation that accepted the request, so a row whose ++ * epoch is older than the running one is proof that the isolate which was ++ * going to produce that response no longer exists (memory/CPU limit, storage ++ * timeout, deploy). No code path will ever answer it. ++ * ++ * Same-epoch rows are deliberately left alone: a browser-approval pause ++ * legitimately holds a row open for minutes inside one incarnation, and that ++ * work is still live. ++ * ++ * `onlyStreamId` scopes the sweep to the stream a Last-Event-ID cursor ++ * identifies, so a recovery GET is never handed an unrelated request's error ++ * (see the replay-scoping note in handleGetRequest). ++ * ++ * Returns the stream ids it swept, so the caller can send the close frame ++ * that makes the client's recovery GET end instead of hanging on keepalives. ++ */ ++ async failStaleEpochRequests(agent, connection, onlyStreamId) { ++ if (typeof agent.getStaleEpochStreamRequestIds !== "function") return []; ++ const stale = await agent.getStaleEpochStreamRequestIds(); ++ const sweptStreamIds = []; ++ for (const row of stale) { ++ if (onlyStreamId !== void 0 && row.streamId !== onlyStreamId) continue; ++ // Claimed synchronously, before the first await below: a client may ++ // hold two listeners (mcp-remote does), so two GETs can read the same ++ // rows and interleave at the delete. Without this the client would ++ // receive the same error response twice for one request id. ++ if (this._sweptStaleStreams.has(row.streamId)) continue; ++ this._sweptStaleStreams.add(row.streamId); ++ console.warn(JSON.stringify({ ++ event: "mcp_stale_epoch_requests_failed", ++ currentEpoch: row.currentEpoch, ++ requestIds: row.requestIds, ++ rowEpoch: row.epoch, ++ streamId: row.streamId ++ })); ++ for (const requestId of row.requestIds) try { ++ // No event id on purpose: this error belongs to a stream whose ++ // event log is not this connection's cursor space, and it must ++ // not be replayed to a later reconnect as that stream's history. ++ this.writeSSEEvent(connection, sessionResetErrorResponse(requestId, "session_reset")); ++ } catch (error) { ++ this.onerror?.(error); ++ } ++ // Dropped now rather than on the delivery ack (unlike a replayed ++ // response, which is idempotent and worth retrying): a surviving row ++ // would be re-swept and re-errored on every later GET. A client that ++ // missed this write gets the same answer from the POST bridge's own ++ // close/deadline synthesis. ++ await agent.deleteStreamRequestIds(row.streamId); ++ sweptStreamIds.push(row.streamId); ++ } ++ return sweptStreamIds; } /** * Close any connection (other than `selfId`) currently bound to -@@ -651,6 +926,9 @@ var StreamableHTTPServerTransport = class { +@@ -651,6 +1115,9 @@ var StreamableHTTPServerTransport = class { * Closing rather than mutating sibling state mirrors how the SDK's * single `_streamMapping` entry gives last-writer-wins for free, and * keeps `send()` from routing to a stale bridge. @@ -599,7 +842,7 @@ index 1edcf0c8c9e67aa211ae515e7672cdf79912101e..cb04fc39d14db9f7fcdfa2613b1c5bab */ supersedePriorStreamConnections(agent, selfId, streamId) { for (const other of agent.getConnections()) { -@@ -664,12 +942,14 @@ var StreamableHTTPServerTransport = class { +@@ -664,12 +1131,14 @@ var StreamableHTTPServerTransport = class { * Only used when resumability is enabled */ async replayEvents(lastEventId) { @@ -615,7 +858,7 @@ index 1edcf0c8c9e67aa211ae515e7672cdf79912101e..cb04fc39d14db9f7fcdfa2613b1c5bab this.writeSSEEvent(connection, message, eventId); } catch (error) { this.onerror?.(error); -@@ -678,6 +958,45 @@ var StreamableHTTPServerTransport = class { +@@ -678,6 +1147,45 @@ var StreamableHTTPServerTransport = class { } catch (error) { this.onerror?.(error); } @@ -661,12 +904,16 @@ index 1edcf0c8c9e67aa211ae515e7672cdf79912101e..cb04fc39d14db9f7fcdfa2613b1c5bab } /** * Writes an event to the SSE stream with proper formatting -@@ -689,10 +1008,65 @@ var StreamableHTTPServerTransport = class { +@@ -689,10 +1197,69 @@ var StreamableHTTPServerTransport = class { return connection.send(JSON.stringify({ type: "cf_mcp_agent_event", event: eventData, + eventId, + streamId: eventId ? eventId.slice(0, eventId.lastIndexOf(":")) : void 0, ++ // Lets the POST bridge retire the request ids it is holding without ++ // re-parsing the SSE frame it is forwarding. An id still outstanding ++ // when that bridge's WS dies is what it synthesizes an error for. ++ respondedIds: isJSONRPCResultResponse(message) || isJSONRPCErrorResponse(message) ? [message.id] : void 0, close })); } @@ -727,7 +974,7 @@ index 1edcf0c8c9e67aa211ae515e7672cdf79912101e..cb04fc39d14db9f7fcdfa2613b1c5bab * Handles POST requests containing JSON-RPC messages */ async handlePostRequest(req, parsedBody) { -@@ -733,6 +1107,22 @@ var StreamableHTTPServerTransport = class { +@@ -733,6 +1300,22 @@ var StreamableHTTPServerTransport = class { }; connection.setState(postState); if (this._eventStore) await agent.setStreamRequestIds(streamId, requestIds); @@ -750,7 +997,7 @@ index 1edcf0c8c9e67aa211ae515e7672cdf79912101e..cb04fc39d14db9f7fcdfa2613b1c5bab for (const message of messages) { if (this.messageInterceptor) { if (await this.messageInterceptor(message, { -@@ -760,7 +1150,22 @@ var StreamableHTTPServerTransport = class { +@@ -760,7 +1343,22 @@ var StreamableHTTPServerTransport = class { * when the originating WS has dropped. */ async sendOnStream(agent, streamId, relatedIds, liveConnection, message, requestId) { @@ -774,7 +1021,7 @@ index 1edcf0c8c9e67aa211ae515e7672cdf79912101e..cb04fc39d14db9f7fcdfa2613b1c5bab let shouldClose = false; if (isJSONRPCResultResponse(message) || isJSONRPCErrorResponse(message)) { let responseIds = this._streamResponseIds.get(streamId); -@@ -777,9 +1182,11 @@ var StreamableHTTPServerTransport = class { +@@ -777,9 +1375,11 @@ var StreamableHTTPServerTransport = class { } catch (error) { this.onerror?.(error); } @@ -788,7 +1035,7 @@ index 1edcf0c8c9e67aa211ae515e7672cdf79912101e..cb04fc39d14db9f7fcdfa2613b1c5bab } } async send(message, options) { -@@ -798,14 +1205,19 @@ var StreamableHTTPServerTransport = class { +@@ -798,14 +1398,19 @@ var StreamableHTTPServerTransport = class { * * Sent on exactly one stream, per MCP: "the server MUST send each of * its JSON-RPC messages on only one of the connected streams; it MUST @@ -812,7 +1059,7 @@ index 1edcf0c8c9e67aa211ae515e7672cdf79912101e..cb04fc39d14db9f7fcdfa2613b1c5bab if (standalone) this.writeSSEEvent(standalone, message, eventId); } /** -@@ -861,12 +1273,10 @@ var StreamableHTTPServerTransport = class { +@@ -861,12 +1466,10 @@ var StreamableHTTPServerTransport = class { * * ## Lifecycle * @@ -829,7 +1076,7 @@ index 1edcf0c8c9e67aa211ae515e7672cdf79912101e..cb04fc39d14db9f7fcdfa2613b1c5bab * * Standalone GET stream events (`_GET_stream`) are *not* cleared * automatically; they accumulate for the lifetime of the DO. Bounded -@@ -893,12 +1303,34 @@ var DurableObjectEventStore = class DurableObjectEventStore { +@@ -893,12 +1496,34 @@ var DurableObjectEventStore = class DurableObjectEventStore { } async storeEvent(streamId, message) { if (streamId.includes(":")) throw new Error(`DurableObjectEventStore: streamId must not contain ':' (got ${JSON.stringify(streamId)})`); @@ -864,7 +1111,7 @@ index 1edcf0c8c9e67aa211ae515e7672cdf79912101e..cb04fc39d14db9f7fcdfa2613b1c5bab return eventId; } async getStreamIdForEventId(eventId) { -@@ -915,9 +1347,59 @@ var DurableObjectEventStore = class DurableObjectEventStore { +@@ -915,9 +1540,59 @@ var DurableObjectEventStore = class DurableObjectEventStore { start: startKey, limit: DurableObjectEventStore.REPLAY_LIMIT }); @@ -925,7 +1172,7 @@ index 1edcf0c8c9e67aa211ae515e7672cdf79912101e..cb04fc39d14db9f7fcdfa2613b1c5bab /** * Drop the event log for a single stream. Called by the transport * immediately after a POST's final response has been written to the -@@ -973,6 +1455,13 @@ DurableObjectEventStore.EVENT_KEY_PREFIX = "__mcp_event__:"; +@@ -973,6 +1648,13 @@ DurableObjectEventStore.EVENT_KEY_PREFIX = "__mcp_event__:"; DurableObjectEventStore.SEQ_PAD = 16; DurableObjectEventStore.DELETE_CHUNK = 128; DurableObjectEventStore.REPLAY_LIMIT = 1e3; @@ -939,7 +1186,106 @@ index 1edcf0c8c9e67aa211ae515e7672cdf79912101e..cb04fc39d14db9f7fcdfa2613b1c5bab //#endregion //#region src/mcp/client-transports.ts /** -@@ -1381,6 +1870,48 @@ var McpAgent = class McpAgent extends Agent { +@@ -1355,6 +2037,26 @@ function experimental_createMcpHandler(server, options = {}) { + } + //#endregion + //#region src/mcp/index.ts ++/** ++* Normalize a `__mcp_stream_reqs__:` ledger row. ++* ++* Rows were a bare `requestIds` array before they carried an epoch. A row still ++* in that shape can only have been written by an earlier deployment — that is, ++* by an earlier incarnation — so epoch 0, below every real epoch (they start at ++* 1), is exactly the right reading: the orphan sweep retires it. No data ++* migration is needed because a row's whole lifetime is one unanswered POST. ++*/ ++function readStreamRequestIdsRow(value) { ++ if (Array.isArray(value)) return { ++ epoch: 0, ++ requestIds: value ++ }; ++ if (value !== null && typeof value === "object" && Array.isArray(value.requestIds)) return { ++ epoch: typeof value.epoch === "number" ? value.epoch : 0, ++ requestIds: value.requestIds ++ }; ++ return void 0; ++} + var McpAgent = class McpAgent extends Agent { + constructor(..._args) { + super(..._args); +@@ -1369,18 +2071,121 @@ var McpAgent = class McpAgent extends Agent { + async getInitializeRequest() { + return this.ctx.storage.get("initializeRequest"); + } +- /** Persist the `requestIds` for a POST stream. @internal */ ++ /** ++ * Monotonic id for THIS Durable Object incarnation, bumped exactly once per ++ * live instance the first time anything asks for it (the promise is ++ * memoized on the instance, so concurrent first callers share one bump). ++ * ++ * A reset — isolate memory/CPU limit, storage timeout, deploy — throws the ++ * instance away and constructs a new one, so the next read bumps again and ++ * every ledger row written by the dead incarnation becomes recognisably ++ * stale. ++ * ++ * Deliberately NOT hung off onStart()/init(): both re-run inside a single ++ * incarnation (a restore-after-idle-dispose calls onStart again), which ++ * would strand still-running work as "stale". onStart does force the first ++ * read, so the stored value is current before any request handler runs and ++ * readers outside this class can compare against it. ++ * @internal ++ */ ++ currentSessionEpoch() { ++ this._sessionEpoch ??= (async () => { ++ const stored = await this.ctx.storage.get(McpAgent.SESSION_EPOCH_KEY); ++ const next = (typeof stored === "number" && Number.isFinite(stored) ? stored : 0) + 1; ++ await this.ctx.storage.put(McpAgent.SESSION_EPOCH_KEY, next); ++ return next; ++ })(); ++ return this._sessionEpoch; ++ } ++ /** Persist the `requestIds` for a POST stream, stamped with the incarnation ++ * that accepted them so a reset leaves a recognisable orphan. @internal */ + async setStreamRequestIds(streamId, requestIds) { +- await this.ctx.storage.put(`${McpAgent.STREAM_REQS_KEY_PREFIX}${streamId}`, requestIds); ++ const epoch = await this.currentSessionEpoch(); ++ await this.ctx.storage.put(`${McpAgent.STREAM_REQS_KEY_PREFIX}${streamId}`, { ++ epoch, ++ requestIds ++ }); + } + /** Read the persisted `requestIds` for a POST stream. @internal */ + async getStreamRequestIds(streamId) { +- return this.ctx.storage.get(`${McpAgent.STREAM_REQS_KEY_PREFIX}${streamId}`); ++ return readStreamRequestIdsRow(await this.ctx.storage.get(`${McpAgent.STREAM_REQS_KEY_PREFIX}${streamId}`))?.requestIds; ++ } ++ /** ++ * Ledger rows stamped by an earlier incarnation: requests dispatched into an ++ * isolate that no longer exists, so nothing will ever produce a response for ++ * them. Read-only — the caller must write the client's error before it ++ * deletes anything. @internal ++ */ ++ async getStaleEpochStreamRequestIds() { ++ const currentEpoch = await this.currentSessionEpoch(); ++ const rows = await this.ctx.storage.list({ ++ prefix: McpAgent.STREAM_REQS_KEY_PREFIX, ++ limit: 1e3 ++ }); ++ const stale = []; ++ for (const [key, value] of rows) { ++ const row = readStreamRequestIdsRow(value); ++ if (!row || row.requestIds.length === 0) continue; ++ if (row.epoch >= currentEpoch) continue; ++ stale.push({ ++ currentEpoch, ++ epoch: row.epoch, ++ requestIds: row.requestIds, ++ streamId: key.slice(McpAgent.STREAM_REQS_KEY_PREFIX.length) ++ }); ++ } ++ return stale; + } + /** Drop the persisted `requestIds` for a POST stream. @internal */ async deleteStreamRequestIds(streamId) { await this.ctx.storage.delete(`${McpAgent.STREAM_REQS_KEY_PREFIX}${streamId}`); } @@ -962,10 +1308,14 @@ index 1edcf0c8c9e67aa211ae515e7672cdf79912101e..cb04fc39d14db9f7fcdfa2613b1c5bab + prefix: McpAgent.STREAM_REQS_KEY_PREFIX, + limit: 1e3 + }); -+ return [...rows].flatMap(([key, requestIds]) => Array.isArray(requestIds) && requestIds.length > 0 ? [{ -+ streamId: key.slice(McpAgent.STREAM_REQS_KEY_PREFIX.length), -+ requestIds -+ }] : []); ++ return [...rows].flatMap(([key, value]) => { ++ const row = readStreamRequestIdsRow(value); ++ return row && row.requestIds.length > 0 ? [{ ++ epoch: row.epoch, ++ streamId: key.slice(McpAgent.STREAM_REQS_KEY_PREFIX.length), ++ requestIds: row.requestIds ++ }] : []; ++ }); + } + async acknowledgeDeliveredStream(streamId) { + await this.deleteStreamRequestIds(streamId); @@ -988,7 +1338,39 @@ index 1edcf0c8c9e67aa211ae515e7672cdf79912101e..cb04fc39d14db9f7fcdfa2613b1c5bab /** * Reverse lookup: find which POST stream a given `requestId` belongs * to, and return the stream's full `requestIds` list in the same -@@ -1516,23 +2047,36 @@ var McpAgent = class McpAgent extends Agent { +@@ -1407,10 +2212,14 @@ var McpAgent = class McpAgent extends Agent { + limit: STREAM_REQS_SCAN_LIMIT + }); + if (rows.size === STREAM_REQS_SCAN_LIMIT) console.warn(`McpAgent: getStreamForRequestId hit the ${STREAM_REQS_SCAN_LIMIT}-key scan cap; stale __mcp_stream_reqs__ entries may be accumulating from abandoned POSTs`); +- for (const [key, requestIds] of rows) if (requestIds?.includes(requestId)) return { +- streamId: key.slice(McpAgent.STREAM_REQS_KEY_PREFIX.length), +- requestIds +- }; ++ for (const [key, value] of rows) { ++ const row = readStreamRequestIdsRow(value); ++ if (!row?.requestIds.includes(requestId)) continue; ++ return { ++ streamId: key.slice(McpAgent.STREAM_REQS_KEY_PREFIX.length), ++ requestIds: row.requestIds ++ }; ++ } + } + /** Read the transport type for this agent. + * This relies on the naming scheme being `sse:${sessionId}`, +@@ -1498,6 +2307,12 @@ var McpAgent = class McpAgent extends Agent { + } + /** Sets up the MCP transport and server every time the Agent is started.*/ + async onStart(props) { ++ // Force the incarnation epoch before anything else can read or write the ++ // request-id ledger. partyserver runs onStart inside ++ // blockConcurrencyWhile at every entry point, so from here on the stored ++ // epoch IS this incarnation's, and readers outside this class (the DO's ++ // idle-lease accounting) can compare row epochs against storage directly. ++ await this.currentSessionEpoch(); + if (props) await this.updateProps(props); + else this.props = await this.ctx.storage.get("props"); + await this.init(); +@@ -1516,23 +2331,36 @@ var McpAgent = class McpAgent extends Agent { return; } break; @@ -1040,14 +1422,15 @@ index 1edcf0c8c9e67aa211ae515e7672cdf79912101e..cb04fc39d14db9f7fcdfa2613b1c5bab } } } -@@ -1697,7 +2241,8 @@ var McpAgent = class McpAgent extends Agent { +@@ -1697,7 +2525,9 @@ var McpAgent = class McpAgent extends Agent { } }; McpAgent.STREAM_REQS_KEY_PREFIX = "__mcp_stream_reqs__:"; +McpAgent.UNDELIVERED_STREAM_KEY_PREFIX = "__mcp_undelivered_stream__:"; ++McpAgent.SESSION_EPOCH_KEY = "__mcp_session_epoch__"; //#endregion -export { DurableObjectEventStore, ElicitRequestSchema, MCP_SERVER_ID_MAX_LENGTH, McpAgent, RPCClientTransport, RPCServerTransport, RPC_DO_PREFIX, SSEEdgeClientTransport, StreamableHTTPEdgeClientTransport, WorkerTransport, createMcpHandler, experimental_createMcpHandler, getMcpAuthContext, normalizeServerId }; -+export { DurableObjectEventStore, ElicitRequestSchema, MAX_SSE_AGE_MS, MCP_SERVER_ID_MAX_LENGTH, McpAgent, RPCClientTransport, RPCServerTransport, RPC_DO_PREFIX, SSEEdgeClientTransport, StreamableHTTPEdgeClientTransport, WorkerTransport, createMcpHandler, experimental_createMcpHandler, getMcpAuthContext, normalizeServerId }; ++export { DurableObjectEventStore, ElicitRequestSchema, MAX_SSE_AGE_MS, MCP_POST_RESPONSE_DEADLINE_MS, MCP_SERVER_ID_MAX_LENGTH, McpAgent, RPCClientTransport, RPCServerTransport, RPC_DO_PREFIX, SSEEdgeClientTransport, StreamableHTTPEdgeClientTransport, WorkerTransport, createMcpHandler, experimental_createMcpHandler, getMcpAuthContext, normalizeServerId }; //# sourceMappingURL=index.js.map \ No newline at end of file