fix(kosong): add idle timeout to streaming response loop - #1052
fix(kosong): add idle timeout to streaming response loop#1052itxaiohanglover wants to merge 3 commits into
Conversation
The for-await loop in generate() waits forever when the connection stalls without closing. Add a 60s idle timeout that aborts the stream if no chunks arrive. Fixes MoonshotAI#1050
🦋 Changeset detectedLatest commit: 614f2cd 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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cd4d356d5d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| }; | ||
| resetIdleTimer(); | ||
|
|
||
| try { |
There was a problem hiding this comment.
Close the try block with catch/finally
This new try is never closed with a catch or finally, so generate.ts is no longer parseable; tsc -p packages/kosong/tsconfig.json --noEmit --pretty false reports TS1472: 'catch' or 'finally' expected and the package build is blocked. Add the intended finally that clears idleTimer or remove the try.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed — added finally block to close the try and clean up the idle timer. Also added a changeset.
| const resetIdleTimer = () => { | ||
| if (idleTimer) clearTimeout(idleTimer); | ||
| idleTimer = setTimeout(() => { | ||
| stream.return?.(undefined); |
There was a problem hiding this comment.
Abort the iterator rather than the stream wrapper
This timeout calls return on the StreamedMessage wrapper, but the current provider stream classes expose only [Symbol.asyncIterator]() and keep the actual async generator private (for example KimiStreamedMessage at packages/kosong/src/providers/kimi.ts:218-255, and the OpenAI/Anthropic/Google variants follow the same pattern). In those normal streaming paths stream.return is undefined, so when no chunk arrives the timer fires but does not unblock the pending for await next() call, leaving the stalled connection hung.
Useful? React with 👍 / 👎.
Codex P1: the try block was never closed with catch/finally. Added finally block after the for-await loop to clear the idle timer.
Phase 1 — Non-PR issues (5): - MoonshotAI#94: Read tool status now reports both MAX_LINES and MAX_BYTES when both limits are hit (was: else-if chain only reported the first). Fixed in v1 (agent-core) and v2 (agent-core-v2). Added test for dual-limit case. - MoonshotAI#250: KIMI_MODEL_* env config no longer gives OpenAI/Anthropic models the Kimi default capabilities (image_in, thinking). DEFAULT_CAPABILITIES is now a per-provider-type map: kimi gets ['image_in','thinking'], openai/anthropic get []. Fixed in v1 (env-model.ts) and v2 (envOverlay.ts). Added 2 tests. - MoonshotAI#1972: Skill descriptions exceeding the 250-char model listing limit now emit a warning via onWarning during loadRoots, so the user knows the model sees a truncated version. Added 2 tests (warns / does not warn). - MoonshotAI#1982: Added disabled_skills config denylist. config.toml now accepts disabled_skills = ['skill-name']. Matching skills (case-insensitive) are hidden from getSkill, getPluginSkill, listSkills, listInvocableSkills, register, loadRoots, and the TUI slash menu. Wired through SessionSkillConfig, resolveSessionSkillConfig, and TOML serialization. Added 3 tests. - MoonshotAI#2224: Plugin-qualified skill names (e.g. 'superpowers:systematic-debugging') are now resolved by the native Skill tool. Falls back to getPluginSkill when bare-name getSkill misses and the name contains ':'. Fixed in v1 (skill-tool.ts) and v2 (skillTool.ts). Added 2 tests. Phase 2 — PR-matching issues (5): - MoonshotAI#1008 (PR MoonshotAI#1096): Web UI session archive relabeled from 'Archive' to 'Delete'. Confirm message now states the action is irreversible (en + zh). - MoonshotAI#1106 (PR MoonshotAI#1108): WebSearch and FetchURL tools now forward the abort signal from ExecutableToolContext through to fetch() calls. Updated provider interfaces (WebSearchProvider, UrlFetcher) and all 3 provider implementations (MoonshotWebSearch, MoonshotFetchURL, LocalFetchURL). Updated 2 tests. - MoonshotAI#1050 (PR MoonshotAI#1052): kosong streaming response loop now has an idle timeout (default 60s, configurable via idleTimeoutMs). Replaced bare for-await with manual iterator + Promise.race against timeout. Throws APITimeoutError on idle. Fixed in v1 (kosong/generate.ts) and v2 (agent-core-v2/generate.ts). Added 9 tests. - MoonshotAI#1148 (PR MoonshotAI#1208): max_tokens/max_output_tokens now clamped to 128k ceiling in openai-legacy generate() and openai-responses withMaxCompletionTokens() + generate(). Kimi provider left unchanged (upstream clamping is intentional). Added 4 tests. - MoonshotAI#1273 (PR MoonshotAI#1276): Added KimiCore.dispose() which iterates all live sessions and calls session.close() via Promise.allSettled. Wired into CoreProcessService.dispose(). Idempotent. Added 5 tests. Skipped (already fixed in fork): MoonshotAI#1539, MoonshotAI#244, MoonshotAI#1218. Checklist: .tmp/upstream-tracking/ISSUE-RESOLUTION-CHECKLIST.md
Fixes #1050
Problem
When a model stream stalls — connection remains ESTABLISHED but no bytes arrive —
packages/kosong/src/generate.tswaits forever inside itsfor await (const part of stream)loop. There is no chunk-level idle timeout, so the UI stays in "thinking" state indefinitely.What changed
Added a 60-second idle timeout around the streaming loop:
resetIdleTimer()is called before the loop starts and on every chunk receivedstream.return()to abort the streamfinallyblock to avoid leaksAbortSignalhandling is unchanged — the idle timeout is an additional safety netValidation
finallyblock ensures no timer leak on any exit path