test: deterministic remote-server teardown to kill #1667 afterEach hang (#1763)

* test: deterministic remote-server teardown to kill #1667 afterEach hang

The remote integration tests close their Hono HTTP server in afterEach with
`server.close(cb)`, which waits for all keep-alive sockets to go idle before
firing the callback. undici's global-fetch connection pool holds sockets open,
and under the full parallel/instrumented `npm run coverage` load they may not
go idle before the 30s hook timeout — intermittently failing the dead-transport
tests in remote-auth-branches.test.ts. The crash-on-startup / dead-session
tests are the worst case: they never disconnect, so the session and its dead
subprocess transport linger when the server closes.

Force the sockets closed with `server.closeAllConnections()` alongside close()
so the callback fires deterministically regardless of parallel load, in both
remote-auth-branches.test.ts and its sibling connect-crash.test.ts.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* server: unref the abandoned-session sweep timer

The transport-onclose path arms a 30s setTimeout to GC a session whose client
never opens the events stream. That sweep is best-effort cleanup, not work the
process must stay alive for — unref it so a pending timer can't keep the event
loop (or a test worker) alive for 30s after everything else has finished.

Also narrow closeAllConnections() behind an `in` guard in the two remote
teardown helpers: ServerType is Server | Http2Server and the method is
http1-only, so the direct call failed tsc.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

* test: extract shared closeHarnessServer teardown helper

Address review feedback on #1763: the deterministic close()+closeAllConnections
teardown (and its multi-line rationale) was duplicated across
remote-auth-branches.test.ts and connect-crash.test.ts. Extract it to a shared
closeHarnessServer(server) helper so the logic and its comment live in one
place.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XNdjEPKLG637X8YmhDiEk5

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Cliff Hall 2026-07-24 12:20:54 -04:00 • committed by GitHub
commit 40aaf47705
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 34 additions and 3 deletions

View file

@ -10,6 +10,7 @@ import { serve } from "@hono/node-server";
import type { ServerType } from "@hono/node-server";
import { createRemoteApp } from "@inspector/core/mcp/remote/node/server.js";
import type { MCPServerConfig } from "@inspector/core/mcp/types.js";
import { closeHarnessServer } from "./harnessTeardown.js";
interface Harness {
baseUrl: string;
@ -37,7 +38,7 @@ async function startServer(): Promise<Harness> {
}
async function teardown(h: Harness): Promise<void> {
await new Promise<void>((resolve) => h.server.close(() => resolve()));
await closeHarnessServer(h.server);
}
interface ParsedSseEvent {

View file

@ -0,0 +1,25 @@
import type { ServerType } from "@hono/node-server";
/**
* Deterministically close a Hono-served HTTP server in test teardown.
*
* Node's `server.close(cb)` stops accepting new connections and then waits for
* every existing keep-alive socket to go idle before firing its callback.
* undici's global-`fetch` connection pool holds sockets open, and under the
* full parallel/instrumented `npm run coverage` load they may not go idle
* before the 30s `afterEach` timeout — the #1667 teardown hang. Some of these
* suites' sessions (crash-on-startup / dead-transport) never disconnect, so the
* session and its dead subprocess transport also linger at close time.
*
* Force the pooled sockets closed with `closeAllConnections()` alongside
* `close()` so the callback fires regardless of load. `closeAllConnections` is
* declared only on http1's `Server` (absent on the `Http2Server` arm of
* `ServerType`); `serve()` returns an http1 server here, so the `in` narrow is
* always taken at runtime while keeping the union type-safe without a cast.
*/
export async function closeHarnessServer(server: ServerType): Promise<void> {
await new Promise<void>((resolve) => {
server.close(() => resolve());
if ("closeAllConnections" in server) server.closeAllConnections();
});
}

View file

@ -18,6 +18,7 @@ import type { ServerType } from "@hono/node-server";
import { createRemoteApp } from "@inspector/core/mcp/remote/node/server.js";
import { getTestMcpServerCommand } from "@modelcontextprotocol/inspector-test-server";
import type { MCPServerConfig } from "@inspector/core/mcp/types.js";
import { closeHarnessServer } from "./harnessTeardown.js";
interface Harness {
baseUrl: string;
@ -45,7 +46,7 @@ async function start(): Promise<Harness> {
}
async function stop(h: Harness): Promise<void> {
await new Promise<void>((resolve) => h.server.close(() => resolve()));
await closeHarnessServer(h.server);
}
async function connect(

View file

@ -633,12 +633,16 @@ export function createRemoteApp(
// the user. The endpoint cleans up on stream close; this TTL
// sweeps sessions whose client never connects at all.
if (!session.hasEventConsumer()) {
// This sweep is a best-effort GC of an abandoned session, not work
// the process must stay alive for — unref it so a pending timer
// can't keep the event loop (or a test worker) alive for 30s after
// everything else has finished.
setTimeout(() => {
const stale = sessions.get(sessionId);
if (stale && !stale.hasEventConsumer()) {
sessions.delete(sessionId);
}
}, 30_000);
}, 30_000).unref();
}
} else {
// Session not created yet - failed during start