Fix DeepgramBackend crashing the whole server on a bad-connection reconnect loop

[bgrozev]

Summary

• DeepgramBackend.close() was not idempotent. Found via the new integration test harness (Add integration test harness (container/worker x opus-backend x provider) #108) when testing a deliberately invalid DEEPGRAM_API_KEY: undici's WebSocket.close() can synchronously re-dispatch 'error'/'close' on a connection that never opened, and since DeepgramBackend's own 'error'/'close' listeners call close(), this recurses without bound and crashes the entire Node process with a stack overflow — not just the one session with bad credentials, every other active participant too.
• close() now checks/sets status = 'closed' before touching the WebSocket, mirroring the doClose() idempotency pattern OutgoingConnection/TranslatorConnection already use elsewhere in this codebase.
• Added a test that reproduces the crash by making the mock WebSocket's close() synchronously re-fire 'error' (matching the real undici behavior observed in the crash trace) — it throws RangeError: Maximum call stack size exceeded before this fix, passes after.
• Noted the pitfall in BACKENDS.md's backend-implementation checklist and CLAUDE.md's Deepgram notes, since any backend whose error/close handlers call close() themselves is exposed to the same trap.

[claude]

Code Review

Overview

This PR fixes a legitimate process-crashing bug: DeepgramBackend.close() was not idempotent, and with undici's WebSocket synchronously re-dispatching 'error'/'close' on a bad connection, the error/close listeners calling this.close() → this.ws.close() → re-fire → this.close() created a stack overflow that killed every active session, not just the one with bad credentials. The crash root cause analysis is accurate and well-documented.

---

What works well

• Core fix is correct — the if (this.status === 'closed') return guard at the top of close(), combined with this.ws = undefined before calling ws?.close(), correctly breaks the re-entry cycle. Clearing this.ws first ensures that if the 'error' handler fires again inside ws.close() and calls this.close() a second time (status momentarily back to 'failed'), the ws?.close() at the bottom is a no-op and the recursion terminates.
• wasConnected variable — captures the status before mutating it, so the CloseStream message is sent only for genuinely-connected sessions. Correct.
• Test — accurately reproduces the undici behavior by having the mock ws.close() synchronously re-fire 'error'. The closeCallCount === 1 assertion is the right property to check. Good comment explaining the real-world context.
• Documentation — BACKENDS.md checklist item and CLAUDE.md Deepgram note are both useful; this pitfall isn't obvious, so capturing it in both places is worth the verbosity.

---

Issue: resource leak in the server-initiated 'close' path

The 'close' event handler (line 162) pre-sets this.status = 'closed' before calling this.close():

ws.addEventListener('close', (event) => {
    ...
    this.status = 'closed';   // ← pre-set
    this.close();             // ← guard fires immediately; cleanup is skipped
    this.onClosed?.();
});

After the fix, close() sees status === 'closed' at line 222 and returns early — skipping clearInterval(this.keepAliveTimer) and the this.ws = undefined clear. The keepAlive setInterval keeps firing (harmlessly, since it checks this.status === 'connected' before acting), and this.ws remains pointing at the already-closed socket indefinitely.

Before the fix this wasn't a problem because close() had no guard and performed the cleanup regardless. After the fix the pre-set in the handler is now counterproductive.

Suggested fix — remove the pre-set from the 'close' handler and let close() own the status transition exclusively:

ws.addEventListener('close', (event) => {
    logger.info(
        `Deepgram WebSocket closed for tag ${this.tag}: code=${event.code} reason=${event.reason || 'none'} wasClean=${event.wasClean}`,
    );
    // No status mutation here — close() will set status = 'closed' and clear keepAlive/this.ws.
    // The idempotency guard in close() prevents re-entry if ws.close() re-fires 'close'.
    this.close();
    this.onClosed?.();
});

The guard in close() (if (this.status === 'closed') return) already handles the case where ws.close() re-fires 'close' synchronously — this.ws is set to undefined before ws?.close() is called, so any re-entry via 'close' → this.close() would find this.ws already undefined and be a no-op even without the status guard.

---

Minor: onError can fire twice (pre-existing, not introduced here)

When ws.close() synchronously re-fires 'error', the 'error' handler runs a second time and calls this.onError?.() again before this.close() returns early (via the null this.ws path). This means OutgoingConnection's error callback is invoked twice for the same failure event. This isn't new to this PR and the callback chain appears to be idempotent in practice, but it's now observable without a crash masking it.

---

Verdict

The crash fix is sound and urgently needed. The test and docs are high quality. The only concrete issue is the keepAlive timer leak in the server-initiated close path, which is a direct consequence of the 'close' handler's pre-set conflicting with the new guard. Worth fixing in this PR since the change is one-liner and the existing 'should be safe to call multiple times' test (line 702) plus the existing server-close tests should continue to pass once the pre-set is removed.