fix: avoid spurious unhandledRejection when transport fails mid-sendRequest

[zahalzel]

Summary

Connection.sendRequest can leak an unhandled promise rejection when the underlying transport fails during the await this.#sendMessage(...) call. The caller's await does observe the rejection, but Node's rejection tracker has already fired unhandledRejection (followed by PromiseRejectionHandledWarning) because responsePromise had no handler at the moment #close() rejected it.

The fix attaches a noop .catch to responsePromise immediately after it's created, so the rejection is considered handled when raised. The rejection is still delivered to the caller verbatim via return responsePromise.

Root cause

Inside sendRequest:

const responsePromise = new Promise((resolve, reject) => {
    this.#pendingResponses.set(id, { resolve, reject });
});
await this.#sendMessage({ ... });   // ← caller is blocked here
return responsePromise;              //   by the time we reach this, #close()
                                     //   has already rejected responsePromise

Race:

1. responsePromise is created; {resolve, reject} stored in #pendingResponses.
2. await #sendMessage(...) yields.
3. Transport fails (e.g., child stdout closes because spawn ENOENT'd) → #receive() exits → #close(err) iterates pending responses and rejects responsePromise.
4. The async function's adoption of responsePromise (via return responsePromise) happens one macrotask later when #sendMessage's write queue finally settles.
5. Between (3) and (4) there is a macrotask boundary, so Node fires unhandledRejection at the end of macrotask (3) — the handler isn't attached until macrotask (4).

Symptom

Downstream tests using strict unhandled-rejection policies (vitest default, node --unhandled-rejections=strict) fail:

(node:NNN) PromiseRejectionHandledWarning: Promise rejection was handled
asynchronously (rejection id: N)

Minimal repro

Save as repro.mjs in a scratch dir with @agentclientprotocol/sdk@0.19.0 installed:

import { spawn } from 'node:child_process';
import { Readable, Writable } from 'node:stream';
import { ClientSideConnection, PROTOCOL_VERSION } from '@agentclientprotocol/sdk';

process.on('unhandledRejection', (err) => {
    console.log('UNHANDLED:', err?.message ?? String(err));
    process.exitCode = 1;
});

const child = spawn('definitely-not-a-real-binary-xyz', [], {
    stdio: ['pipe', 'pipe', 'pipe'],
});
child.on('error', () => {});

const readable = Readable.toWeb(child.stdout);
const writable = Writable.toWeb(child.stdin);

const connection = new ClientSideConnection(
    () => ({
        async writeTextFile() { return {}; },
        async readTextFile() { return { content: '' }; },
        async requestPermission() {
            return { outcome: { outcome: 'selected', optionId: 'allow' } };
        },
        async sessionUpdate() {},
    }),
    { readable, writable },
);

try {
    await connection.initialize({
        protocolVersion: PROTOCOL_VERSION,
        clientCapabilities: { fs: { readTextFile: true, writeTextFile: true } },
    });
} catch (err) {
    console.log('initialize rejected (expected):', err.message);
}

Before this patch (on main):

UNHANDLED: ACP connection closed
initialize rejected (expected): ACP connection closed
(node:NNN) PromiseRejectionHandledWarning: Promise rejection was handled asynchronously

(process exits 1)

After this patch:

initialize rejected (expected): ACP connection closed

(process exits 0)

Tests

• All existing tests pass (npm test): 95 tests across 4 files.
• I attempted to add a regression test in acp.test.ts, but vitest's internal unhandled-rejection tracking makes it tricky to observe this specific race from inside a describe block — process.on('unhandledRejection') listeners registered inside a test don't reliably see the rejection because vitest's own handlers intercept first. I can add one if you have a pattern for catching this kind of thing in the existing suite; otherwise the manual repro above is the clearest demonstration.

Why this fix and not restructuring sendRequest

Alternatives considered:

1. Reverse the order — register in #pendingResponses after await #sendMessage. Wrong: messages can arrive before the write call returns, so the response map needs the entry before we send.
2. Catch inside #close — the rejection already has a handler at the .reject() call site (the promise itself); the issue is purely that no external handler was attached yet.
3. Promise.withResolvers + explicit tracking — more invasive for the same effect.

The noop .catch is the minimal change: it marks the promise as "will be handled" without changing any caller-visible behavior. The caller's await still sees the rejection through return responsePromise.

Downstream

This is blocking agent-loom PR #62 (adds an AcpClient wrapper around the SDK for driving copilot --acp). Loom currently deletes the spawn-based missing-binary test case as a workaround; if this lands I'd like to restore it.

Happy to iterate on the test approach or the comment wording.