fix(kosong,agent-core-v2): watchdog and retry silent LLM stream stalls#1799
fix(kosong,agent-core-v2): watchdog and retry silent LLM stream stalls#1799askie wants to merge 3 commits into
Conversation
The OpenAI-compatible client used for streaming completions clears its
request timeout as soon as response headers arrive (`openai/src/client.ts`
wraps the fetch in `try { … } finally { clearTimeout(timeout) }` and the
fetch promise resolves on header receipt). Once headers are back the
streaming body has no read timeout of its own, so a mid-stream stall
blocks `for await` forever and hangs the whole turn — including cases
where the assistant has already streamed a reasoning delta and then
falls silent, and where the process holds pooled TCP connections that a
NAT/LB has half-closed.
Add an inactivity watchdog around the streamed message iterator: after
`KIMI_STREAM_IDLE_TIMEOUT_MS` (default 180 s) without a new chunk, cancel
the underlying stream and throw `StreamIdleTimeoutError`. The error now
extends `APITimeoutError`, so `isRetryableGenerateError` recognises it
and the loop's `stepRetry` plugin re-drives the failed step automatically;
users see the same recoverable behaviour as for any transient network
timeout instead of a permanent hang. The error message carries the
elapsed stream time and, when available, the Kimi `x-trace-id` for
server-side triage.
🦋 Changeset detectedLatest commit: cd11470 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Closing to run local verification first — will reopen after the fix is validated on the reporter's machine. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9312f94828
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // The dangling next() will reject once the stream is cancelled below; | ||
| // swallow it so it never surfaces as an unhandled rejection. | ||
| next.catch(() => {}); | ||
| await cancelStream(stream); |
There was a problem hiding this comment.
In the real provider streams (KimiStreamedMessage, OpenAILegacyStreamedMessage, etc.) cancellation is on the async iterator returned above, not on the StreamedMessage object; those classes do not expose cancel or return. When this new timeout path runs, await cancelStream(stream) is therefore a no-op, leaving the stalled iterator.next() and its HTTP socket open while retry starts another request. Please close iterator.return?.() here, and preferably from a finally so early exits are also closed.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in cd11470. You're right that the provider stream classes expose no cancellation handle, so cancelStream(stream) was a no-op on every real provider. generate() now owns a watchdog AbortController merged with the caller's signal via AbortSignal.any and hands the merged signal to the provider; on timeout the watchdog aborts it, which providers forward to the HTTP client — the stalled connection is actually torn down. The iterator is additionally closed via iterator.return?.() from a finally block so early exits are covered too; the call is deliberately not awaited because on a stalled stream return() stays pending until the transport abort settles the dangling read. New tests assert both the signal abort (with the timeout error as reason) and the return() call.
| /** | ||
| * Guard the gap between streamed chunks with an inactivity deadline. | ||
| * | ||
| * The provider SSE stream has no read timeout of its own — the HTTP client's | ||
| * request timeout is cleared as soon as response headers arrive (see |
There was a problem hiding this comment.
packages/agent-core-v2/AGENTS.md says comments in this package must live solely in the top-of-file /** */ block and never beside functions, methods, or statements. This new body comment (and the inline timeout comment below) violates the local convention, so move any necessary module-level context into the file header or let the code/tests carry it.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in cd11470 — the body comments are gone; module-level context now lives in the top-of-file /** */ header per this package's convention.
The ACP path (kimi-code TUI + `kimi acp` → @moonshot-ai/kimi-code-sdk → @moonshot-ai/agent-core → agent-core/turn/kosong-llm.ts) drives its LLM stream through kosong.generate(), not agent-core-v2.generate(). The earlier watchdog on agent-core-v2 fired only on the kap-server path (@moonshot-ai/agent-core-v2), so the ACP session that hung mid-stream still had no read-side timeout. Live proof: session 6871de3a-3499-4e94-94f7-39585a347f1b silently stalled after the first `think` chunk at 02:13:47 on a bundle that contained the agent-core-v2 watchdog symbols — no StreamIdleTimeoutError ever surfaced. Port the identical inter-chunk deadline guard into kosong: wrap the `for await (const part of stream)` loop with `withStreamIdleTimeout()` that races each iterator.next() against a `setTimeout` reject; on timeout, cancel the stream and throw a `StreamIdleTimeoutError` (extends kosong `APITimeoutError`) carrying the elapsed time and the `x-trace-id`. The existing `isRetryableGenerateError()` classifies it as retryable, so agent-core's `chatWithRetry()` re-drives the failed step via the standard `step.retrying` event — same recovery path the kap-server watchdog already exercises. Watchdog window shares the same `KIMI_STREAM_IDLE_TIMEOUT_MS` knob (default 180000 ms) so operators tune both stacks in lockstep. Test: packages/kosong/test/stream-idle-timeout.test.ts covers cancel, retryable classification with traceId propagation, and healthy-stream pass-through.
…atchdog fires Provider stream classes expose no cancellation handle, so the previous cancelStream() call was a no-op on every real provider and each stall leaked its HTTP connection until the server gave up. The watchdog now owns an AbortController merged with the caller's signal via AbortSignal.any and aborts it on timeout, so the stalled request is actually torn down. The provider iterator is additionally closed from a finally block (fire-and-forget: awaiting return() on a stalled stream would re-introduce the hang) so generator cleanup also runs on consumer throws and early exits. In agent-core-v2, comments move to the top-of-file header per that package's convention.
4a3880d to
cd11470
Compare
askie
changed the title
1 participant
Related Issue
Resolve #1798
Problem
A streaming completion whose body goes silent mid-flight blocks the whole turn forever. The OpenAI-compatible client only guards the header phase —
openai/src/client.tsfetchWithTimeoutwraps the fetch intry { … } finally { clearTimeout(timeout) }and the fetch promise resolves once headers arrive, so the abort timer is cleared before the body is consumed. From that point on,for await (const part of stream)has no deadline of its own; if the socket goes idle (rate-limit disconnect, LB/NAT half-close, provider-side stall) the loop never advances, no error is thrown, and the CLI status bar keeps showing an in-flight turn.Full evidence — 11/161 loop requests hung under concurrent use, always after partial content had already streamed — is in the linked issue.
What changed
Add an inactivity watchdog around the streamed message iterator, in both stream loops shipped in the CLI bundle:
packages/kosong/src/generate.ts— the ACP path (kimi-code TUI /kimi acp→ kimi-code-sdk → agent-core → kosong), which is where the linked issue's hangs were observed.packages/agent-core-v2/src/app/llmProtocol/generate.ts— the kap-server path, which has the same failure signature.Mechanics (identical in both):
stream[Symbol.asyncIterator]()with an async generator that races eachnext()against a timer. Default deadline is180_000ms, tunable viaKIMI_STREAM_IDLE_TIMEOUT_MS.generate()now owns a watchdogAbortControllermerged with the caller's signal viaAbortSignal.any, and hands the merged signal to the provider. On timeout the watchdog aborts it, which providers forward to the HTTP client — so the stalled connection is actually torn down instead of leaking until the server gives up on it. (Provider stream classes expose no cancellation handle of their own, so signal-based teardown is the only path that reaches the socket.)iterator.return?.()from afinallyblock, so generator cleanup also runs on consumer throws and early exits. The call is deliberately not awaited: on a stalled streamreturn()stays pending until the transport abort settles the dangling read, and awaiting it would re-introduce the hang.StreamIdleTimeoutErrorextendsAPITimeoutError, soisRetryableGenerateErrorrecognises it and the loop's existingstepRetryplugin transparently re-drives the failed step — no new retry pathway is introduced.x-trace-idwhen the response headers exposed one, so operators can correlate a hung turn with a server-side request without patchingfetch.This is the smallest change that eliminates the silent-hang failure mode without altering the happy path (a healthy stream simply never trips the deadline) and reuses the existing retry taxonomy rather than adding a parallel one.
Tests (
packages/kosong/test/stream-idle-timeout.test.ts,packages/agent-core-v2/test/app/llmProtocol/stream-idle-timeout.test.ts) cover:thinkdelta throwsStreamIdleTimeoutErrorand cancels the underlying stream.APITimeoutErrorandisRetryableGenerateErrorreturnstrue, so step-retry will drive recovery.return()is invoked.Verification beyond unit tests
The patched build has been running as the reporter's daily driver. Since the rebuild, 254 streamed completions completed with zero silent hangs; every request/usage-record gap in the session logs was attributable to a user interrupt or an in-flight request, none to a stall.
Checklist
gen-changesetsskill, or this PR needs no changeset. (patch @moonshot-ai/kimi-code)gen-docsskill, or this PR needs no doc update.