fix(daemon): release the pending IPC slot when write throws - #1045

Thanks for this, and especially for writing the regression test first and correcting the "hang" claim in the issue. I verified it: the test fails on main with exactly the - 1 / + 0 you reported and passes on the branch. The fix itself is correct and idiomatic.

One thing needs changing before merge.

The changeset states a mechanism that isn't real

Both the issue and the changeset say write() throws synchronously when the socket is already closed. It doesn't. On Node 22.15:

• write after destroy(): returns false, no throw, no error event
• write after end(): returns false, no throw, async ERR_STREAM_WRITE_AFTER_END

net.Socket.prototype.write only throws synchronously when the chunk isn't a string or ArrayBufferView. So the repro steps in #1040 never hit this path. The closed-socket case was already safe via handleClose(), which rejects and clears every pending slot on the socket's close event.

The guard is still worth having, but the realistic sync-throw source in that block is JSON.stringify on circular params, not write. Since the changeset ships verbatim into the published changelog, I'd rather it didn't assert the wrong cause. Suggested wording:

Fixed a leaked pending slot in the daemon IPC client. If serializing or writing a request threw synchronously, the request's entry stayed in the pending map for the lifetime of the client, one per failed request. The slot is now released and the request rejected explicitly. Closes #1040.

Suggestions (non-blocking)

source/daemon/ipc.ts:248 - the ?. is now dead and hides the same bug class. request() already throws at line 244 if !this.socket, and the executor runs synchronously, so it can't be null here. But if it ever were, ?. would skip the write and leave the slot pending with no rejection, which is exactly the leak you're fixing. Capturing it first makes the guard total:

const socket = this.socket;
const id = this.nextId++;
return new Promise((resolve, reject) => {
  this.pending.set(id, {resolve, reject});
  try {
    socket.write(`${JSON.stringify({id, method, params} satisfies IpcRequest)}\n`);
  } catch (error) { ... }
});

Test stub restoration. Overwriting internals.socket.write survives the finally only because Writable.end() with no chunk doesn't route through the public write. Restoring the original in finally would decouple the test from that internal.

Optional second case. A test that destroys the socket, fires a request, and asserts the promise rejects and pending drains via handleClose would cover the path the issue actually describes. Nothing in the suite pins that today.

Adjacent, please don't fix here

Two things I noticed while reading, both pre-existing and better as separate issues if you're interested:

• No request timeout. If the daemon accepts the connection but never responds and never closes the socket, the slot leaks and the promise never settles. That's the real route to the "hangs" symptom in the issue title.
• connect() (source/daemon/ipc.ts:204) never removes its s.once('error', reject) on success. Post-connect, the first socket error is swallowed by a no-op reject and consumes the listener, so a second error has no listener and takes the process down.