fix: only absorb the teardown's own rejection in settleInFlight

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) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HDdo1rNRVQnVRRdsqSrvG3
This commit is contained in:
cliffhall 2026-08-10 18:01:27 -04:00
commit 0b9aee11a3

View file

@ -107,14 +107,27 @@ async function getTool(client: InspectorClient, name: string): Promise<Tool> {
*
* 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<unknown>): Promise<void> {
return call.then(
() => undefined,
() => undefined,
(error: unknown) => {
if (
error instanceof SdkError &&
error.code === SdkErrorCode.ConnectionClosed
) {
return;
}
throw error;
},
);
}