fix(core): add SSE stream idle watchdog to abort hung streams (Fixes #4177) - #5330

@qwen-code /triage

Maintainer verification — local merge build (🔴 not mergeable as-is)

Verified the 3-way merge result (current origin/main e234cf6 + this PR head 0ae2a54) on macOS (Darwin arm64, Node v22.22.2). The PR is up to date with main (diff: a new streamIdleWatchdog.ts + geminiChat.ts integration, +95/-4). Two blocking problems: CI's Test jobs are red, and the watchdog is never actually invoked — so the #4177 fix does not run.

1. 🔴 The watchdog is plumbed but never invoked — the fix doesn't function

createStreamIdleWatchdog() returns { next, cleanup }, where next(streamStep) is the mechanism: it races each stream step against the idle timeout and aborts on stall. But in geminiChat.ts the watchdog object is only created (2611), passed to processStreamResponse (2668), and cleanup-attempted (2670) — there is no streamWatchdog.next(...) call anywhere. processStreamResponse receives it as _streamWatchdog (underscore = intentionally unused) and iterates the stream directly (for await (const chunk of streamResponse)), bypassing the watchdog entirely.

I confirmed the watchdog unit itself is correct (fake-timer test): next() on a stalled promise rejects with STREAM_IDLE_TIMEOUT and aborts after the timeout; next() on a resolved step clears both timers. So the building block works — it's just never wired into the stream loop. As written, a hung SSE stream is not detected or aborted by this watchdog.

(This compiles cleanly precisely because the unused params are _-prefixed, so no-unused-vars stays quiet — which is why it passed lint/typecheck while doing nothing.)

2. 🔴 CI Test jobs fail — geminiChat.test.ts not updated for the new abortSignal

The integration now passes abortSignal: streamAbortController.signal into the generateContentStream config. The existing assertion should call generateContentStream with the correct parameters checks the exact config and was not updated, so it fails on the extra field.

This matches the PR's CI (Test macOS + ubuntu: fail).

3. ⚠️ Cleanup sits in a catch that can never fire

try {
  return this.processStreamResponse(model, streamResponse, streamWatchdog, cleanupAbortLink);
} catch (error) {
  streamWatchdog?.cleanup(); cleanupAbortLink?.(); throw error;
}

processStreamResponse is an async generator — calling it returns the generator synchronously without running the body, so this catch only catches synchronous construction errors (there are none). Stream/iteration errors happen later, during consumption, and never reach here. Net effect: cleanupAbortLink() (which removes the abort listener added by linkAbortSignal) is never called on the normal path or on iteration errors → a per-stream abort-listener leak. Cleanup needs to live inside the generator (e.g. a finally) or wrap the iteration.

4. ⚠️ Format + tests

• prettier --check packages/core/src/core/streamIdleWatchdog.ts reports style issues (the new file isn't formatted). eslint passes.
• No tests are added for the watchdog. A hung-stream test (fake timers, a stalled next()) would have caught that the watchdog never fires.

What it needs before merge

1. Wire streamWatchdog.next(...) into the stream loop in processStreamResponse (replace the bare for await with a manual loop that races each streamResponse.next() through the watchdog, or wrap streamResponse in a watchdog generator). Drop the _ once it's actually used.
2. Move cleanup into a finally inside the generator so cleanupAbortLink() / cleanup() always run (the current try/catch around the generator construction is dead).
3. Update geminiChat.test.ts for the new abortSignal config field (turn the failing assertion green).
4. prettier --write the new file; add a watchdog unit test (the timeout-fires / timers-cleared cases).

Verdict

The watchdog primitive is well-designed and verified to work in isolation, but as integrated it is inert (never invoked) and the PR is red on CI (a test wasn't updated for the new abortSignal), plus a dead-catch cleanup leak and an unformatted file. 🔴 Changes requested — wire next() into the loop, fix the cleanup placement, and update the test; then re-verify that a stalled stream actually aborts.

_{Verified by maintainer @wenshao: 3-way merge build (main e234cf6 + PR 0ae2a54) + whole-file usage grep (watchdog next() never called) + a fake-timer unit test of the watchdog (passes in isolation) + geminiChat.test.ts on main (192/192) vs merge (191/192). Env: Darwin arm64, Node v22.22.2.}

try {
  return this.processStreamResponse(model, streamResponse, streamWatchdog, cleanupAbortLink);
} catch (error) {
  streamWatchdog?.cleanup(); cleanupAbortLink?.(); throw error;
}