fix(agent-core-v2): detect and retry stalled LLM response streams - #3073
fix(agent-core-v2): detect and retry stalled LLM response streams#30737Sageer wants to merge 8 commits into
Conversation
🦋 Changeset detectedLatest commit: 3b2c6e9 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 |
|
@codex review |
![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: 075d0633f4
ℹ️ 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".
| { | ||
| ...options, | ||
| auth, | ||
| signal: watchdog?.signal ?? signal, |
There was a problem hiding this comment.
When the Google GenAI provider has returned its stream but the iterator then stops yielding, passing the watchdog signal here does not settle generate(): google-genai.ts races the signal only while acquiring generateContentStream() (lines 797–806), while its subsequent for await loop checks the signal only after another chunk arrives (lines 593–605). Consequently the timeout aborts its controller but no error reaches this catch, so no LLMStreamStalledError or retry occurs and Google streaming requests can still hang indefinitely; race each pending iterator read against the signal or otherwise make that provider's stream consumption abort-aware.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5b28a44822
ℹ️ 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".
| timeout = setTimeout(() => { | ||
| timeout = undefined; | ||
| didIdleTimeout = true; | ||
| controller.abort(abortError()); | ||
| }, currentTimeoutMs); |
There was a problem hiding this comment.
When either new timeout is configured above 2147483647 ms, Node clamps this setTimeout delay to 1 ms, so a user attempting to allow a long-running request instead causes nearly every request to be classified as stalled immediately. Both config fields currently accept any non-negative integer; reject oversized values or use the existing setClampedTimeout helper.
Useful? React with 👍 / 👎.
| const maxAttempts = Math.max( | ||
| this.config.get<LoopControl>(LOOP_CONTROL_SECTION)?.maxAttemptsPerStep ?? | ||
| DEFAULT_MAX_RETRY_ATTEMPTS, | ||
| error instanceof LLMStreamStalledError | ||
| ? loopControl?.maxStallAttemptsPerStep ?? DEFAULT_MAX_STALL_ATTEMPTS_PER_STEP | ||
| : loopControl?.maxAttemptsPerStep ?? DEFAULT_MAX_RETRY_ATTEMPTS, |
There was a problem hiding this comment.
When a step encounters mixed transient failures, failedAttempts includes every prior retryable error before this smaller stall limit is applied. For example, after two 5xx failures, the first subsequent stall makes the shared count 3 and exhausts the default stall budget immediately, so that stalled request is not retried at all despite the separate stall budget. Track stall failures separately while continuing to enforce the overall attempt cap.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 11e58591ae
ℹ️ 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".
| throw convertGoogleGenAIError(error); | ||
| } finally { | ||
| try { | ||
| void iterator.return?.()?.catch(() => {}); |
There was a problem hiding this comment.
When the stalled operation is the pending iterator.next() call, the new race lets the caller fail but this fire-and-forget return() does not cancel the read: native async generators queue return() behind the unresolved next(). Because generateContentStream(params) is not supplied the watchdog signal, the underlying Google HTTP stream can therefore remain open indefinitely, and each automatic retry can leave another request and connection behind. Make the transport itself abort-aware or use a cancellation primitive that can interrupt the pending read.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7b1b98feaa
ℹ️ 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".
| compactionTriggerRatio: z.number().min(0.5).max(0.99).optional(), | ||
| firstOutputTimeoutMs: z.number().int().min(0).max(MAX_TIMER_DELAY_MS).optional(), | ||
| streamIdleTimeoutMs: z.number().int().min(0).max(MAX_TIMER_DELAY_MS).optional(), | ||
| maxStallAttemptsPerStep: z.number().int().min(0).optional(), |
There was a problem hiding this comment.
When max_stall_attempts_per_step is configured to a very large integer, the first stall passes that value to retryBackoffDelays(attemptBudget), which eagerly allocates attemptBudget - 1 entries even though the overall max_attempts_per_step limit may still be only 10. Values such as 1000000000 are accepted by both the schema and environment parser, so a single stalled request can exhaust memory before retrying; bound this field, clamp it to the effective overall budget, or calculate only the current delay.
Useful? React with 👍 / 👎.
|
@codex review |
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cfe470ed5e
ℹ️ 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".
| const maxAttempts = Math.max(loopControl?.maxAttemptsPerStep ?? DEFAULT_MAX_RETRY_ATTEMPTS, 1); | ||
| let failedAttempt = this.failedAttempts; | ||
| let attemptBudget = maxAttempts; | ||
| if (error instanceof LLMStreamStalledError) { |
There was a problem hiding this comment.
When a request emits assistant or thinking parts and then exceeds stream_idle_timeout_ms, this branch classifies it for retry after those parts have already been published as deltas. The retry path emits turn.step.retrying and starts another numbered step, but the kap-server transcript projector only updates the old step header and never deletes its existing frames, so the incomplete first attempt remains visible—often as a permanently running step—beside the successful retry output. Retract or explicitly terminate the stalled attempt's streamed frames before requeueing the driver.
Useful? React with 👍 / 👎.
| beforeEach(() => { | ||
| vi.useFakeTimers(); | ||
| }); |
There was a problem hiding this comment.
The new watchdog suite globally enables vi.useFakeTimers() (and the added abort, retry, and requester suites do likewise), violating the agent-core-v2 test skill's boundary-discipline rule that timers be controlled through documented knobs, an injected clock, or a manual tick rather than fake timers. This is particularly important here because the implementation depends on real Node timer clamping and abort/event-loop ordering, which virtual timers can model differently; expose a scheduler/clock seam and drive these tests through that public control instead.
Useful? React with 👍 / 👎.
|
@codex review |
|
Codex Review: Didn't find any major issues. Another round soon, please! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
2 participants
Related Issue
None — the problem is explained below. See also #1917, which covers the sibling problem of SDK-internal hidden retries.
Problem
When a provider stops producing bytes mid-stream — or never sends the first byte — a model request can hang forever:
generate()only checks the abort signal at part boundaries, so a stalled stream never settles.What changed
Adds an application-layer liveness watchdog for model requests in
agent-core-v2:createIdleTimeoutAbortSignal(_base/utils/abort.ts): a resettable idle-timeout signal that links to the parent signal and aborts only its own internal controller on expiry.ModelRequesterImpl.runRequestwraps eachgenerate()attempt (inside the auth-refresh callback, so a 401 resend gets fresh timers): a first-output timeout runs until the first part arrives, then a stream-idle timeout resets on every part. On expiry it aborts only the internal controller and rethrows a typed, retryableLLMStreamStalledErrorcarrying phase / elapsed / idle diagnostics. A user cancel always wins the race, and when both thresholds are0the watchdog is bypassed entirely — the provider receives the original signal object.AgentStepRetryServicegives stall errors their own bounded budget (max_stall_attempts_per_step, default3) instead of the generic 10 attempts; the existing backoff andturn.step.retryingpipeline is reused unchanged.loop_controlfieldsfirst_output_timeout_ms(default180000),stream_idle_timeout_ms(default120000), andmax_stall_attempts_per_step(default3), withKIMI_LOOP_*env bindings. Thresholds flow fromAgentLLMRequesterServicedown throughModelRequestParams.flowchart TD A[AgentLLMRequesterService<br/>reads loop_control thresholds] --> B[ModelRequesterImpl.runRequest] B --> C{watchdog armed?<br/>threshold > 0} C -- no --> D[generate with original signal] C -- yes --> E[generate with idle-timeout signal] E -->|part arrives| E E -->|idle timeout| F[abort internal controller<br/>throw LLMStreamStalledError] F --> G[AgentStepRetryService<br/>bounded budget: 3 attempts] G --> B E -->|user cancel| H[cancelled, never retried]Tests cover: first-output expiry, per-part idle reset, the typed stall error (retryable, never an
AbortError), user-cancel precedence, full timer cleanup on every path, the0-threshold bypass, the separate stall budget, and the config/env wiring. User docs (config-files,env-vars, both locales) are updated in this PR.Checklist
gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update.