fix(cursor): retry a discovery stream that ends before HTTP/2 headers - #3102
Conversation
(cherry picked from commit 46125ea)
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. |
|
✅ Deterministic PR hygiene checks passed. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 10 included reviews per hour; 5 remain after this review. 📝 WalkthroughWalkthroughThe Cursor HTTP/2 adapter now classifies pre-header stream termination as a transport error. The added test confirms that model discovery retries the failed request and returns the model from the next successful attempt. ChangesCursor pre-header retry
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to The change retries HTTP/2 discovery streams that end before response headers, preventing intermittent catalog failures while leaving authenticated and other HTTP errors unchanged. Focused tests and type checking pass, so no actionable merge-blocking risk remains. Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Linked Issues checkExplanation The implementation addresses issue Full details: Out of Scope Changes checkExplanation The changes are limited to the Cursor HTTP/2 discovery adapter and its regression test. Both files directly support the linked issue and PR objective. No unrelated code or behavior changes are present.
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
리뷰 · 우선순위 60 / 80이 PR은 Cursor 라이브 모델 발견이 HTTP/2에서 응답 헤더보다 먼저 스트림이 끝날 때, 실패를 잘못 분류하던 구멍을 고칩니다. 지금 고치는 코드는 생산 한 줄입니다. HTTP/1.1 길은 이미 다릅니다. 테스트는 이 패치는 @terrytan95 의 #3052를 지금 라인 265 - 메인테이너의 판단이 필요한 지점
너의 추천 이 댓글은 grok-bot이 작성했습니다 |
|
Administrator bypass record\n\nOwner authorization was explicit in Codex session 01a05a34-1e3a-73f2-8607-15e517cbec11 on 2026-09-01 KST. Exact head: 2b11e98. Current exact-head check rollup: 0 failing, 0 pending. Independent A-gate review found no unresolved code blocker. Using the maintain/admin pull_request bypass documented in MAINTAINERS.md because the author cannot approve their own PR. This bypass does not waive CI or security review. |
Summary
Carries #3052 (author @terrytan95) rebased onto current
dev. The patch is one production line and needs no changes.Cursor discovery over HTTP/2 starts with
status = 0and only assigns it onresponse. When a stream ends before headers arrive — which real Cursor endpoints do intermittently —req.on("end")fell through every status branch to{ error: "http", detail: "HTTP unknown" }. The retry set istimeout|transport(src/adapters/cursor/live-models.ts:54), sohttpwas never retried and the catalog recorded a discovery failure for a connection whose very next request succeeds. The HTTP/1.1 path already maps a fetch failure totransportat:194-196; this makes the HTTP/2 path agree.Closes #3051.
Verification
The test is not a stub: it stands up a real HTTP/2 server, closes the first stream with
NGHTTP2_NO_ERRORbefore responding, and asserts both that a second request happened and that the model list came back. Mutation-checked — deleting the single production line gives 41 pass / 1 fail, exactlyretries an HTTP/2 stream that ends before response headers, then restored to 42/0.The deterministic-auth test directly above it is the control: 401/403 and non-2xx statuses stay non-retryable, so this widens retry only to the pre-header case.
Checklist
bun x tsc --noEmitcleanTriaged in the 2026-08-31 non-priority-70 bug round; supersedes #3052 only by rebase.
Summary by CodeRabbit