Skip to content

Commit 236fd9d

Browse files
RhysSullivanclaude
andcommitted
Report executions lost to a session reset
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
1 parent eaa1f3a commit 236fd9d

9 files changed

Lines changed: 984 additions & 73 deletions
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"executor": patch
3+
---
4+
5+
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.

apps/cloud/src/observability/observability.test.ts

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -385,15 +385,24 @@ describe("Durable Object platform reset noise", () => {
385385
expect(beforeSendWithOtelCorrelation(defect)).not.toBeNull();
386386
});
387387

388-
// The memory-limit reset is deliberately absent from the classifier: the
389-
// runtime blames the application for it, so it is a defect, not noise.
390-
it("keeps the memory-limit reset the classifier deliberately excludes", () => {
388+
// The storage-cache memory-limit variant stays absent from the classifier:
389+
// the runtime blames the application for it (un-awaited writes, an oversized
390+
// read), so it is a defect, not noise. Its plain sibling is a platform reset
391+
// and IS classified — the two are separated only by that qualifier.
392+
it("keeps the memory-limit variant the classifier deliberately excludes", () => {
391393
const memory = doInstrumentationEvent(
392394
"Durable Object's isolate exceeded its memory limit due to overflowing the storage cache. All objects in the isolate were reset.",
393395
);
394396
expect(beforeSendWithOtelCorrelation(memory)).not.toBeNull();
395397
});
396398

399+
it("drops the plain memory-limit reset as platform noise", () => {
400+
const memory = doInstrumentationEvent(
401+
"Durable Object's isolate exceeded its memory limit and was reset.",
402+
);
403+
expect(beforeSendWithOtelCorrelation(memory)).toBeNull();
404+
});
405+
397406
it("the hook the worker and DOs install drops the deploy reset", () => {
398407
const options = cloudSentryOptions({ SENTRY_DSN: "https://public@example.invalid/1" } as Env);
399408
const event = doInstrumentationEvent("Durable Object reset because its code was updated.");

packages/hosts/cloudflare/src/mcp/agent-session-durable-object.test.ts

Lines changed: 129 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,20 @@ type HarnessSession = {
153153
>;
154154
alarm: () => Promise<void>;
155155
ctx: MemoryStorage;
156+
currentSessionEpoch: () => Promise<number>;
157+
getStaleEpochStreamRequestIds: () => Promise<
158+
ReadonlyArray<{
159+
readonly streamId: string;
160+
readonly requestIds: ReadonlyArray<string | number>;
161+
readonly epoch: number;
162+
readonly currentEpoch: number;
163+
}>
164+
>;
165+
getStreamRequestIds: (streamId: string) => Promise<ReadonlyArray<string | number> | undefined>;
166+
setStreamRequestIds: (
167+
streamId: string,
168+
requestIds: ReadonlyArray<string | number>,
169+
) => Promise<void>;
156170
dbHandle: { readonly end: () => void } | null;
157171
engine: ExecutionEngine<Cause.YieldableError> | null;
158172
getConnections?: () => Iterable<unknown>;
@@ -266,7 +280,14 @@ const approval = {
266280
content: { approved: true },
267281
} satisfies ResumeResponse;
268282

269-
const makeHarnessSession = async (): Promise<HarnessSession> => {
283+
/**
284+
* `storage` is a parameter so a test can build a SECOND session on the same
285+
* durable storage — that is exactly what a Durable Object reset looks like from
286+
* storage's point of view: same keys, brand new instance.
287+
*/
288+
const makeHarnessSession = async (
289+
storage: MemoryStorage = new MemoryStorage(),
290+
): Promise<HarnessSession> => {
270291
const sessionId = "session-reconnect";
271292
const sessionMeta: SessionMeta = {
272293
organizationId: "org-1",
@@ -275,7 +296,6 @@ const makeHarnessSession = async (): Promise<HarnessSession> => {
275296
userId: "user-1",
276297
resource: defaultMcpResource,
277298
};
278-
const storage = new MemoryStorage();
279299
const server = makeServer();
280300
await server.connect(new StaleCloseTransport());
281301

@@ -1766,3 +1786,110 @@ describe("McpAgentSessionDOBase residency cap eviction", () => {
17661786
});
17671787
});
17681788
});
1789+
1790+
// The request-id ledger (`__mcp_stream_reqs__:<streamId>`, written by the
1791+
// patched McpAgent — see patches/agents@0.17.3.patch) is the only durable
1792+
// record that a POST is still owed a response. A row exists from the moment the
1793+
// request is accepted until its final response is written, so a row that
1794+
// outlives the incarnation which accepted it is a request nothing will ever
1795+
// answer: the isolate was reset mid-execute. Each row carries the epoch of the
1796+
// incarnation that wrote it, and that is what separates "stranded" from
1797+
// "legitimately still running" — a browser-approval pause holds a row open for
1798+
// minutes inside ONE incarnation and must never be swept.
1799+
describe("McpAgentSessionDOBase stranded-request ledger", () => {
1800+
const ledgerKey = (streamId: string) => `__mcp_stream_reqs__:${streamId}`;
1801+
1802+
it("stamps the accepting incarnation on every ledger row", async () => {
1803+
const session = await makeHarnessSession();
1804+
1805+
await session.setStreamRequestIds("stream-a", [1, "two"]);
1806+
1807+
expect(await session.ctx.storage.get(ledgerKey("stream-a"))).toEqual({
1808+
epoch: await session.currentSessionEpoch(),
1809+
requestIds: [1, "two"],
1810+
});
1811+
expect(
1812+
await session.getStreamRequestIds("stream-a"),
1813+
"readers still see a plain request-id list",
1814+
).toEqual([1, "two"]);
1815+
});
1816+
1817+
it("does not treat a row from the running incarnation as stranded", async () => {
1818+
const session = await makeHarnessSession();
1819+
1820+
// What a browser-approval pause looks like: accepted, unanswered, and
1821+
// legitimately going to stay that way for minutes.
1822+
await session.setStreamRequestIds("stream-paused", [9]);
1823+
1824+
expect(await session.getStaleEpochStreamRequestIds()).toEqual([]);
1825+
});
1826+
1827+
it("reports a row left by a previous incarnation as stranded", async () => {
1828+
const storage = new MemoryStorage();
1829+
const beforeReset = await makeHarnessSession(storage);
1830+
await beforeReset.setStreamRequestIds("stream-lost", [42]);
1831+
await beforeReset.setStreamRequestIds("stream-also-lost", ["abc"]);
1832+
1833+
// The reset: same durable storage, a brand new Durable Object instance.
1834+
const afterReset = await makeHarnessSession(storage);
1835+
await afterReset.setStreamRequestIds("stream-live", [100]);
1836+
1837+
const stranded = await afterReset.getStaleEpochStreamRequestIds();
1838+
1839+
// Order follows storage's key order, which this fake does not model, so
1840+
// the assertion is on the set.
1841+
expect(
1842+
[...stranded].sort((a, b) => a.streamId.localeCompare(b.streamId)),
1843+
"only the rows the dead incarnation accepted",
1844+
).toMatchObject([
1845+
{ streamId: "stream-also-lost", requestIds: ["abc"] },
1846+
{ streamId: "stream-lost", requestIds: [42] },
1847+
]);
1848+
for (const row of stranded) expect(row.epoch).toBeLessThan(row.currentEpoch);
1849+
});
1850+
1851+
it("reports a pre-epoch ledger row as stranded", async () => {
1852+
const session = await makeHarnessSession();
1853+
1854+
// The shape rows had before they carried an epoch. One can only have been
1855+
// written by an earlier deployment, so it reads as epoch 0 and is swept.
1856+
await session.ctx.storage.put(ledgerKey("stream-legacy"), [7]);
1857+
1858+
expect(await session.getStaleEpochStreamRequestIds()).toEqual([
1859+
{
1860+
currentEpoch: await session.currentSessionEpoch(),
1861+
epoch: 0,
1862+
requestIds: [7],
1863+
streamId: "stream-legacy",
1864+
},
1865+
]);
1866+
expect(
1867+
await session.getStreamRequestIds("stream-legacy"),
1868+
"and it is still readable as a request-id list",
1869+
).toEqual([7]);
1870+
});
1871+
1872+
it("holds the idle lease for a request the running incarnation still owes", async () => {
1873+
const session = await makeHarnessSession();
1874+
await session.setStreamRequestIds("stream-live", [1]);
1875+
1876+
await session.alarm();
1877+
1878+
expect(session.initialized, "live work keeps the runtime resident").toBe(true);
1879+
expect(session.ctx.alarm, "and re-arms the lease").toBeGreaterThan(0);
1880+
});
1881+
1882+
it("does not let a stranded row extend the idle lease", async () => {
1883+
const storage = new MemoryStorage();
1884+
const beforeReset = await makeHarnessSession(storage);
1885+
await beforeReset.setStreamRequestIds("stream-lost", [1]);
1886+
1887+
const afterReset = await makeHarnessSession(storage);
1888+
await afterReset.alarm();
1889+
1890+
expect(
1891+
afterReset.initialized,
1892+
"a request nothing will ever answer is dead work, not running work",
1893+
).toBe(false);
1894+
});
1895+
});

packages/hosts/cloudflare/src/mcp/agent-session-durable-object.ts

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,6 @@ const AGENTS_DESTROY_PENDING_KEY = "cf_agents_destroy_pending";
203203
const MCP_HTTP_METHOD_HEADER = "cf-mcp-method";
204204
const MCP_MESSAGE_HEADER = "cf-mcp-message";
205205
const MODEL_RESUME_FORWARD_TIMEOUT_MS = 10_000;
206-
const MCP_STREAM_REQS_KEY_PREFIX = "__mcp_stream_reqs__:";
207206
const approvalResponseKey = (executionId: string) => `approval-response:${executionId}`;
208207
const BrowserApprovalDecisionStorage = Schema.Struct({
209208
response: ResumeResponsePayload,
@@ -752,13 +751,20 @@ export abstract class McpAgentSessionDOBase<
752751
// which survives disposeIdleRuntime, so a later reconnect GET re-inits the
753752
// DO and replays it. Counting them would make every delivered-but-unacked
754753
// POST response pin the runtime alive indefinitely.
755-
const rows = await this.ctx.storage.list<readonly JsonRpcRequestId[]>({
756-
prefix: MCP_STREAM_REQS_KEY_PREFIX,
757-
limit: 1_000,
758-
});
754+
//
755+
// Rows stamped with an epoch older than this incarnation's are dead work,
756+
// not running work: the isolate that was going to produce their response
757+
// was reset, so nothing will ever answer them and they must not hold the
758+
// runtime open. The transport's orphan sweep tells the client and removes
759+
// the row on the next GET; until then they simply do not count.
760+
const [openStreams, currentEpoch] = await Promise.all([
761+
this.getOpenStreamRequestIds(),
762+
this.currentSessionEpoch(),
763+
]);
759764
let count = 0;
760-
for (const requestIds of rows.values()) {
761-
if (Array.isArray(requestIds)) count += requestIds.length;
765+
for (const stream of openStreams) {
766+
if (stream.epoch < currentEpoch) continue;
767+
count += stream.requestIds.length;
762768
}
763769
return count;
764770
}

0 commit comments

Comments
 (0)