fix(ai): widen Alibaba first-event timeout - #3740
Conversation
Long-context Alibaba Token Plan responses can arrive beyond the old 300s watchdog, while Completions setup still inherited the generic 120s SDK timeout. Align the provider-specific layers at 600s and preserve the canonical typed timeout so the existing no-replay policy can terminate it. Lore-id: b7a4c91e Constraint: caller and environment timeout overrides must keep precedence Rejected: raise the global first-event timeout | delays failures for unrelated providers Confidence: high Scope-risk: narrow Reversibility: easy Tested: packages/ai check; 43 timeout regressions; Alibaba no-replay session tests Not-tested: full packages/ai suite on Windows | existing EBUSY temp-directory cleanup failures after 1929 passes
![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: 7698bf54e1
ℹ️ 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".
| const envSdkTimeoutMs = getStreamFirstEventTimeoutMs( | ||
| getOpenAIStreamIdleTimeoutMs(), | ||
| getProviderFirstEventTimeoutFallbackMs(model.provider), |
There was a problem hiding this comment.
When the public streamOpenAICompletions transport is called directly for Alibaba with streamFirstEventTimeoutMs below 600,000 and the server stalls before returning headers, this fallback makes envSdkTimeoutMs 600,000 and the subsequent Math.max prevents the caller's override from taking effect. Because the iterator watchdog is armed only after headers arrive, a requested 60-second timeout can therefore hang for ten minutes (and potentially longer with SDK retries), contrary to the documented override contract. Use the explicit nonzero override as the pre-header SDK timeout rather than flooring it at the provider default.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 3327361. Provider-specific fallbacks now apply only when the caller does not pin a value; an explicit nonzero Alibaba/Kimi override is passed to the pre-header SDK timeout even when shorter. Added a direct Alibaba setup regression, while the existing generic OpenAI delayed-setup contract remains green (44 focused tests + package check).
Yeachan-Heo
left a comment
Yeachan-Heo
left a comment
There was a problem hiding this comment.
Verdict: APPROVE
Exact head reviewed: 7698bf54e133e8e9e33ce357fe6cc5b9bd77d64c against base 44b35ceaf041fa5a24207e7e24a91be0bdba1329.
Red-team result: no blocking defect found in the Alibaba first-event timeout widening. The 600-second provider fallback is centralized and applied to both inner OpenAI transports and the lazy outer wrapper; explicit caller and environment overrides retain precedence. The Completions path also normalizes the pre-header OpenAI SDK timeout into the canonical stream_first_event_timeout fact, closing the before-headers gap described by the PR. Focused timeout tests cover hanging, delayed, semantic-progress, and caller-abort paths.
CI was exact-head green/CLEAN at review time.
—
[repo owner's gaebal-gajae (clawdbot) 🦞]
The new provider-specific SDK fallback could override an explicitly shorter caller window and leave Alibaba setup waiting for 600 seconds. Let explicit values beat slow-provider fallbacks while preserving the established generic OpenAI pre-header floor. Lore-id: 4e0c7a92 Constraint: preserve generic OpenAI delayed-setup behavior Rejected: apply exact caller timeouts to every OpenAI completion | changes the established generic SDK floor contract Confidence: high Scope-risk: narrow Reversibility: easy Tested: 44 timeout regressions; packages/ai check
Yeachan-Heo
left a comment
Yeachan-Heo
left a comment
There was a problem hiding this comment.
Verdict: APPROVE / MERGE_READY
Exact-head revalidation completed against the current PR head and current dev base.
- PR head:
332736123b3d647473d6fb8eaf1e88a6d12ec5ff - Current
devbase:44b35ceaf041fa5a24207e7e24a91be0bdba1329 - Reviewed the full 8-file diff, including the follow-up commit
fix(ai): preserve slow-provider timeout overrides.
Red-team result: no blocking defect found. The follow-up correctly makes a shorter explicit Alibaba caller timeout beat the provider's 600-second SDK fallback, while preserving 0 as disable. The effective timeout remains aligned across the SDK pre-header bound, provider semantic-event watchdog, and lazy outer wrapper; unrelated providers retain their existing behavior. The Alibaba pre-stream SDK timeout normalization remains typed as stream_first_event_timeout and is covered for both Completions and Responses.
Evidence:
- Current-head GitHub CI has no non-passing checks; all completed checks are
SUCCESSand only platform/path-inapplicable jobs areSKIPPED. - The current-head focused test additions cover delayed setup beyond the old 120-second Completions bound, shorter explicit Alibaba caller timeout, environment override normalization, Alibaba Responses setup timeout normalization, caller aborts, no-progress streams, and lazy-wrapper behavior.
- Current-head diff is clean under
git diff --checkper the PR verification record. - No source files were modified and no merge was performed in this review worktree.
—
[repo owner's gaebal-gajae (clawdbot) 🦞]
2 participants
Problem
Alibaba Token Plan long-context requests have produced successful first semantic events at 264?311 seconds. The existing 300-second Alibaba watchdog therefore sits inside the normal observed TTFT range and can abort a legitimate response at the boundary.
There was also a shorter path on OpenAI Completions: the SDK request timeout is armed before response headers, while the iterator watchdog only starts after
create()returns. That SDK timeout inherited the generic 120-second stream window instead of Alibaba's provider budget. When it fired, the OpenAI SDK surfacedAPIConnectionTimeoutError: Request timed out.without the canonicalstream_first_event_timeouttransport fact, so the session layer could treat the failure as unknown/retryable and replay the request.Change
0still disables the first-event watchdog).APIConnectionTimeoutErrors on both OpenAI transports toFirstEventTimeoutError, preserving the typed no-replay transport contract.Why 600 seconds
600 seconds leaves useful headroom above the observed 264?311-second range without widening the global default or delaying failures for unrelated providers. Explicit caller/environment timeout settings still win.
Verification
bun test packages/ai/test/openai-first-event-timeout.test.ts packages/ai/test/register-builtins.test.ts packages/ai/test/stream-timeout-defaults.test.ts? 44 passbun --cwd=packages/ai run check? pass (Biome 446 files + package TypeScript check)EBUSYwhile deleting its temp directorypackages/airun reached 1,929 pass / 337 skip; 172 failures were a Windows SQLite/temp-directoryEBUSYcleanup cascade also reproduced by an unrelated auth-only testgit diff --check origin/dev...HEAD? passRelated
This is complementary to #3708. That PR addresses ownership races between the outer lazy watchdog and provider-owned raw-event watchdogs. It does not change Alibaba's provider budget, the Completions pre-header SDK timeout, or typed normalization of SDK connection timeouts; those remain necessary if #3708 lands.