From 0b9aee11a313e4d9afe056575766730913c9f5a9 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Mon, 10 Aug 2026 18:01:27 -0400 Subject: [PATCH] fix: only absorb the teardown's own rejection in settleInFlight MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review feedback: the blanket rejection handler also converted a genuine `callTool` failure into a pass. That matters here because these tests assert on the progress notifications, which arrive before the result — so a call that emitted all three progress events and then failed would have satisfied every assertion and been swallowed. Narrow the handler to absorb only `SdkError` / `CONNECTION_CLOSED` and re-throw anything else. The handler is still attached at call time, so an unexpected rejection stays *handled* and surfaces as an ordinary test failure at the `await inFlight` after `disconnect()` rather than as another unhandled rejection. Fulfillment is still accepted: whether the call beats the teardown is a race, so asserting either outcome would reintroduce the original bug as a flake. Verified the narrowed predicate is actually exercised rather than dead code — both call sites reject with SdkError/CONNECTION_CLOSED on every run. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01HDdo1rNRVQnVRRdsqSrvG3 --- .../integration/mcp/inspectorClient.test.ts | 21 +++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/clients/web/src/test/integration/mcp/inspectorClient.test.ts b/clients/web/src/test/integration/mcp/inspectorClient.test.ts index 3586e51f..9f5379a7 100644 --- a/clients/web/src/test/integration/mcp/inspectorClient.test.ts +++ b/clients/web/src/test/integration/mcp/inspectorClient.test.ts @@ -107,14 +107,27 @@ async function getTool(client: InspectorClient, name: string): Promise { * * Attach the handler at call time (not after the assertions) so there is no * window in which the rejection can escape, and `await` the returned promise - * after `disconnect()` so teardown stays ordered. The call may legitimately - * either settle before the teardown or reject with it, so neither outcome is - * asserted here. + * after `disconnect()` so teardown stays ordered. + * + * Only the teardown's own `CONNECTION_CLOSED` rejection is absorbed. Plain + * fulfillment is fine too — whether the call beats the teardown is a race, so + * asserting either outcome would turn this straight back into a flake. Any + * *other* rejection is re-thrown, so a call that fails for a real reason still + * fails the test instead of passing on the strength of the progress + * notifications it managed to emit first. */ function settleInFlight(call: Promise): Promise { return call.then( () => undefined, - () => undefined, + (error: unknown) => { + if ( + error instanceof SdkError && + error.code === SdkErrorCode.ConnectionClosed + ) { + return; + } + throw error; + }, ); }