Skip to content

flaky: mcp/lifecycle.test.ts 'add() closes the old protocol session when replacing a server' times out under load #207

Closed

Description

@alltomatos

Summary

`bun run test` occasionally fails in `packages/opencode/test/mcp/lifecycle.test.ts`:

```
(fail) add() closes the old protocol session when replacing a server [5176.29ms]
error: replaced HTTP session was not aborted
at orElse (packages/opencode/test/lib/effect.ts:175:37)
```

Root cause

This is a legitimate-latency test-timing issue, not an app bug and not a "never completes" hang.

When `MCP.add()` replaces an existing server, `storeClient()` in `packages/opencode/src/mcp/index.ts` calls `previous.close()` on the old `StreamableHTTPClientTransport`. That `close()` synchronously calls `this._abortController.abort()` client-side (see the SDK's `client/streamableHttp.js`). Aborting the client-side fetch/controller does not synchronously abort the server's in-flight request — the mock test server in `lifecycleServer()` only increments `state.aborted` when the underlying HTTP request's `signal` fires its own `abort` event, which requires the socket teardown to actually propagate from client to server over the real network stack (even over loopback, this is scheduled asynchronously and its latency depends on runtime/OS scheduling, not on our code).

The test polls for `first.state.aborted > 0` with the default 5-second `pollWithTimeout` window (`test/lib/effect.ts`). Under CI/load, socket-teardown propagation occasionally exceeds 5s (observed at ~5.17s), so the poll times out even though the abort does eventually happen — same class of issue as #191 (tight subprocess timeouts causing flaky unit failures).

Fix

Widen the timeout for this poll (and the sibling polls in the same file that wait on the same kind of real-socket abort propagation: "disconnected HTTP session was not aborted" and "remote transport requests were not aborted") to 10 seconds, matching the precedent already used elsewhere in the suite (e.g. `test/session/prompt.test.ts`) for similar real async teardown waits. This is a test-only change; no production code changes are needed since the abort call itself is already correct and synchronous on the client side.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions