Occasional "Failed to pop isolated storage stack frame" error in test environment

[mthadley]

Describe the bug

I've been seeing the occasional test failure with an error like this:

Failed to pop isolated storage stack frame in src/index.spec.ts's test "calls the hello-world tool".
In particular, we were unable to pop Durable Objects storage.
This usually means your Worker tried to access storage outside of a test, or some resources have not been disposed of properly.
Ensure you "await" all Promises that read or write to these services, and make sure you use the "using" keyword when passing data across JSRPC.
See https://developers.cloudflare.com/workers/testing/vitest-integration/known-issues/#isolated-storage for more details.

To Reproduce

Here's an example test that seems to trigger the issue inconsistently:

import { describe, expect, it } from 'vitest';
import { mcpTest as test } from './test/mcp-test.js';

describe('MCP server', () => {
  test('lists the hello-world tool', async ({ client }) => {
    const { tools } = await client.listTools();
    expect(tools).toEqual(
      expect.arrayContaining([
        expect.objectContaining({ name: 'hello-world' }),
      ]),
    );
  });
});

Along with the vitest fixture it is using:

import { Client } from '@modelcontextprotocol/sdk/client/index.js';
import { StreamableHTTPClientTransport } from '@modelcontextprotocol/sdk/client/streamableHttp.js';
import { SELF } from 'cloudflare:test';
import { test as base } from 'vitest';

interface McpFixtures {
  client: Client;
}

export const mcpTest = base.extend<McpFixtures>({
  client: async ({}, use) => {
    const transport = new StreamableHTTPClientTransport(
      new URL('https://example.com/mcp'),
      { fetch: SELF.fetch.bind(SELF) },
    );
    const client = new Client({ name: 'test-client', version: '0.1.0' });
    await client.connect(transport);

    await use(client);

    await client.close();
  },
});

Expected behavior

No sporadic test fialures.

Version:

This is with agents@0.4.1

Additional context

I did some (admitedly AI-assisted) poking around of the code and I was led towards these calls to updateProps:

agents/packages/agents/src/mcp/utils.ts

Line 245 in 839abc7

if (ctx.props) agent.updateProps(ctx.props as Record<string, unknown>);

A hypothesis is that these calls to updateProps, which calls this.ctx.storage.put are being dispatched, but never awaited. When the test environment is torn down, they are still in-flight, leading to the above errors.

[nathanschram]

I've been digging into this and I think the issue is in the SDK itself rather than your test code.

McpAgent.updateProps() is an async method that does await this.ctx.storage.put("props", props) internally (source). But in utils.ts, it gets called without await at three places - lines 245, 402, and 566:

// All three call sites have this pattern:
if (ctx.props) agent.updateProps(ctx.props as Record<string, unknown>);

That turns the storage.put() into a fire-and-forget write. When vitest-pool-workers tries to pop the isolated storage stack frame during teardown, it occasionally catches that write still in-flight - hence the intermittent error.

For comparison, onStart() at line 153 already does this correctly:

if (props) await this.updateProps(props);

The fix would be adding await at those three call sites. Worth noting this could also be a production concern - unawaited storage writes could silently fail if the DO gets evicted before they complete.

Happy to put up a PR for this if that'd be useful.

[threepointone]

thanks for digging in here, we should be able to take it from here. does applying these changes locally solve the problem for you?

[mattzcarey]

Thanks for this :)

[threepointone]

Taking this on, looks like we can just remove those calls since we've refactored it out a while ago with onStart's props field .

[nathanschram]

Nice, makes sense to just remove them entirely now that onStart handles it. Cheers!