Repository navigation
fix(responses): allow 90 seconds for Codex WebSocket response prelude - #4084
Conversation
|
Warning Review limit reachedNext included review available in 21 minutes. View limit detailsLimit details: You’ve used all 10 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (2)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (4)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe fixed Codex WebSocket response-prelude timeout increases from 30 to 90 seconds. A regression test covers responses that begin after 30 seconds. Server and transport documentation describe the updated deadline and fallback behavior. ChangesCodex WebSocket response-prelude timeout
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to Codex WebSocket requests can now wait up to 90 seconds for their first response event, preventing premature failures for slow multi-image requests while retaining a bounded deadline and avoiding duplicate resends. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
✅ Deterministic PR hygiene checks passed. |
|
@codex review |
|
@coderabbitai review |
✅ Action performedReview finished.
|
✅ READY
Review readiness checklist
✅ 4/4 boxes ticked. This pull request is already Ready for Review. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
@lidge-jun @Ingwannu This is a narrow proposed fix for #4083（premature timeouts during multi-image work）: in multi-image work, a healthy upstream response can begin after the current 30-second deadline, causing repeated client reconnects. The patch changes one runtime constant to 90 seconds, adds a deterministic regression, and documents the bound. It does not add configuration or change replay behavior. Could you confirm whether raising the default is a direction you want to accept? The PR remains draft. The focused WebSocket tests, typecheck, privacy scan, and docs build passed; the description explicitly records the incomplete local full-suite attempt and does not claim those failures are unrelated to this patch. Codex and CodeRabbit reviews have been requested. The unchanged Cross-platform CI is running on this exact PR commit in the contributor fork: https://github.com/S0RYUASUKA/opencodex/actions/runs/34315214926. If upstream workflows require approval, please approve them when appropriate. I will address valid findings within this fix's scope; the full local-CI and readiness boxes remain unchecked. |
![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: c46bd23aff
ℹ️ 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".
리뷰 · 우선순위 58 / 80이 PR은 #4083을 닫기 위한 최소 패치입니다. 지금 설명의 핵심을 초등학생에게도 말하면 이렇습니다. “서버가 대답을 시작했는지”를 재는 초시계가 지금은 30초짜리인데, 사진이 많은 정상적인 일도 가끔 30초가 넘어서야 시작됩니다. 시계가 먼저 울리면 우리는 작업을 취소해 버리고, 앱은 같은 일을 처음부터 다시 보냅니다. 시계를 90초짜리로 바꾸면 그런 정상 느린 시작은 살리고, 진짜로 아무 응답이 없는 경우에는 최대 1분만 더 기다립니다. 이건 모델이 글을 다 쓰는 전체 시간 제한이 아니고, 변경 범위는 작습니다.
메인테이너의 판단이 필요한 지점
너의 추천
이 댓글은 grok-bot이 작성했습니다 |
The new paragraph claimed the 90-second prelude deadline is not configurable through connectTimeoutMs. The constant is not tunable, but the effective wait is: core.ts passes connectTimeoutMs (default 200s) as the header-timeout budget to fetchWithHeaderTimeout, which aborts the combined signal the WS exchange runs under, and codex-ws-exchange.ts cancels an already-sent create on that abort before the prelude timer fires. An operator who lowers connectTimeoutMs below 90 seconds keeps the shorter deadline, so state it as the shorter of the two. Addresses the Codex review finding on this PR.
lidge-jun
left a comment
lidge-jun
left a comment
There was a problem hiding this comment.
Maintainer integration review. Exact-head CI at dec8306: Cross-platform CI success, enforce-target success (run 34370880192; the later cancelled run is a concurrency-group duplicate at the same SHA), hygiene and labeler success. Raises the WebSocket response-prelude cap to 90s so a slow multi-image start is not truncated. Original author @S0RYUASUKA; branch brought current by merge from dev, authorship preserved.
…e-90s fix(responses): allow 90 seconds for Codex WebSocket response prelude
2 participants
Summary
Closes #4083（多图慢请求被 30 秒响应开始上限截断）。
多图工作中，连续查看截图、对照图片会使后续请求携带累积的图片上下文，正常响应开始前的等待也可能接近或超过 30 秒。固定 30 秒上限对这类请求过短，容易造成“超时断流 → 客户端重连并重发大输入 → 再次超时”的反复中断。
将 canonical ChatGPT WebSocket 发送 create 后等待响应开始的固定上限从 30 秒提高到 90 秒，为正常的多图慢请求留出余量，减少过早断流。含多图上下文的真实探针曾在
send()返回后 30.085 秒才收到正常response.created，证明原上限确实会截断本可继续的请求。实现仅修改共享常量一行，沿用现有定时器、取消、内存限制和发送后不通过 HTTP 重发的处理。新增一项确定性回归检查：28 秒收到控制帧、31 秒收到正常响应，最终完成且只发送一次。同步架构说明和运行配置文档。
关联事项 #3976（让响应开始等待时间可配置）。本改动只提高固定上限，不实现可配置功能，因此不以该事项作为自动关闭目标。
90 秒为有界缓解值，未证明是最佳阈值。真正无响应的请求最多多等待 60 秒；控制帧不会无限续期。这是响应开始前的上限，不是模型回答完成的时限。
Verification
dev：8026405d9a527085b3c972dc8630abf8fe3b0441；Windows 11 build 26200.8875，Bun 1.4.2。codex websocket response prelude timed out。bun run test tests/responses/ws-upstream.test.ts tests/responses/ws-upstream-reuse.test.ts：153 通过、1 跳过、0 失败，包含既有的到期退出且不重发检查。bun run typecheck：通过。bun run privacy:scan：通过。cd docs-site && bun install --frozen-lockfile && bun run build：通过，425 页。git diff --check：通过。bun run test:changed：以同一origin/dev为基线启动，运行近 6 分钟仍未给出最终结果后主动停止；状态为未完成，不计为通过。未将未完成原因归为代码缺陷，也未扩大到无关测试修复。另外，现场安装版 2.48.0 / Bun 1.4.0 的同一常量修改已通过真实计时时序检查：跨过 30 秒的响应正常完成，无消息时约 90.001 秒超时。真实本机
/v1/responses多图请求约 18.44 秒开始、25.56 秒 completed；该样本没有跨过 30 秒，且未执行模型返回的工具，不等同于原会话任务完整恢复。当前贡献分支未部署到生产服务。正常推送前检查已尝试：类型检查通过、GUI lint 因未修改 GUI 跳过；全套测试出现账号选择事件流的管理认证、订阅释放和提交后通知相关失败后主动停止，没有完整结果。尚未做未修改基线对照，不能认定这些失败与本补丁无关。按草稿贡献流程使用
git push --no-verify发布分支，并通过 fork 的正式 Cross-platform CI 补充验证；本地全套检查框保持未勾选，不能声明已可合并。Checklist
Maintainer shepherding update (2026-09-09)
dec8306e0is the contributor'sc46bd23afwith currentdev(91db6c2f2) merged in, plus one documentation commit. The branch was 106 commits behinddev, which theenforce-targetreadiness gate treats as a disproved "latest dev" claim.devwas merged in rather than rebased, so the contributor's commits and authorship are preserved.The open Codex P2 finding on
docs-site/src/content/docs/reference/configuration/server.mdis fixed and its thread resolved. That paragraph claimed the 90-second prelude deadline "is not configurable throughconnectTimeoutMs". The constant itself is not tunable, but the effective wait is:src/server/responses/core.tspassesconfig.connectTimeoutMs ?? 200_000as the header-timeout budget tofetchWithHeaderTimeout(src/server/responses/fetch-helpers.ts:177), which composes it into theAbortSignal.any([abortSignal, timeout.signal])that the WebSocket exchange runs under, andsrc/server/responses/codex-ws-exchange.ts:169-170cancels an already-sent create on that abort before the prelude timer at line 214 fires. The documentation andstructure/04_transports-and-sidecars.mdnow describe the deadline as the shorter of the 90-second bound andconnectTimeoutMs.Local checks were NOT RUN for this update — no product test suite, no typecheck, no build, no lint, no
bun install— per maintainer instruction. Exact-head repository CI is the only verification gate: run 34366687424 atdec8306e0finished with the aggregatecicheck SUCCESS, 24 successful checks, 2 conditional skips (the Windows shard matrix and the macOS control lane), and no failures. The "All CI tests are green on my local testing" box below is ticked on the strength of that green exact-head repository CI run, which supersedes the local suite; it does not attest to a local run.Review readiness checklist
Summary by CodeRabbit
Bug Fixes
Documentation