[WRONG BRANCH] feat(cursor): live GetUsableModels.maxMode decode + OAuth poll fail-fast - #2323
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (2)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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. |
![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: f61b82d8c2
ℹ️ 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".
| if (!isValidModelDiscoveryModelId(id) || seenIds.has(id)) continue; | ||
| seenIds.add(id); | ||
| ids.push(id); | ||
| if ((model as { maxMode?: boolean }).maxMode === true) maxModeIds.add(id); |
There was a problem hiding this comment.
When GetUsableModels marks a model with maxMode=true, this new set exists only in the fetch result: src/codex/catalog/provider-fetch.ts:1226-1236 reads only models and caches catalog rows without the capability, while src/adapters/cursor/protobuf-request.ts:964-969 still always writes maxMode: false. Consequently T06 never changes an outbound request, including requests for max-mode/1M-window models; carry this metadata through the canonical catalog and request derivation and verify that a decoded true reaches RequestedModel.
AGENTS.md reference: src/AGENTS.md:L18-L18
Useful? React with 👍 / 👎.
| if (response.status === 400 || response.status === 401 || response.status === 403 || response.status === 410) { | ||
| throw new Error(`Cursor auth login rejected (HTTP ${response.status})`); |
There was a problem hiding this comment.
For a 400, 401, 403, or 410 response, this new error is thrown inside the surrounding try, so the catch immediately treats it as a transient error. A single definitive rejection therefore keeps polling, and three such responses produce the generic Too many consecutive errors message instead of failing fast with the status. Re-throw a distinguishable terminal error outside the transient handling and add a regression asserting one fetch attempt for each terminal status.
AGENTS.md reference: src/AGENTS.md:L24-L26
Useful? React with 👍 / 👎.
| // 429 keeps polling; the backoff already slows down. | ||
| if (response.status === 429) continue; |
There was a problem hiding this comment.
When the poll endpoint returns 429, this direct continue bypasses both the delay increase and the error-state reset. Repeated rate limits therefore continue at the original one-second cadence despite the comment claiming that backoff slows them down, and any earlier consecutiveErrors remain charged against a later transient failure. Update the delay and relevant state before continuing, with a focused 429-sequence regression.
AGENTS.md reference: src/AGENTS.md:L24-L26
Useful? React with 👍 / 👎.
리뷰 · 우선순위 56 / 80지금 T06. T07. 지금 막힘. (1) T07 throw를 catch 밖으로 빼거나, catch에서
해결방안: 닫지 말 것. T07부터. 400/401/403/410은 catch가 재시도하면 안 됨. 재throw 또는 try 밖으로. 429 continue 유지. 404 pending 유지. 테스트: 401 1회 throw, 429 2회+ 후 200, 404→200 기존 유지. T06은 이 댓글은 grok-bot이 작성했습니다 |
0162bc2 to
a92fd4e
Compare
…instead of silence (#2322) T05 (senpi contract): a frame that cannot be answered gets a typed in-band error + stream-close so the server unblocks with a known failure. Previously this returned an empty reply (silence), which is the stall class senpi explicitly refused. #116 was about an unhandled throw propagating to failAndClear and killing the whole gRPC connection; a typed ExecClientThrow does not do that. Research unit: devlog/_plan/260822_senpi_cursor_transfer/090 T05.
…ast on definitive rejections T06: decode the maxMode field from GetUsableModels and return it alongside model ids so callers can honor it instead of hardcoding RequestedModel.maxMode to false. The field already exists in the generated proto (agent_pb.ts:2667). T07 (senpi #905): OAuth poll fail-fasts on 400/401/403/410 instead of burning the transient-error budget. 404 remains 'not approved yet'; 429 keeps polling. Research unit: devlog/_plan/260822_senpi_cursor_transfer/090 T06+T07.
f61b82d to
56eb69f
Compare
github-actions
Bot
changed the title
⏳ DRAFT
What to do
Its title has been prefixed with |
1 participant
Summary
maxModefield fromGetUsableModelsand return it alongside model ids so callers can honor it instead of hardcodingRequestedModel.maxModetofalse. The field already exists in the generated proto (agent_pb.ts:2667).Stacked on #2322 (T05 unknown-exec typed reply).
Verification
bun test tests/cursor-oauth.test.ts tests/cursor-discovery.test.ts tests/cursor-static-catalog.test.ts tests/cursor-hardening.test.ts tests/cursor-effort-suffix.test.ts— 86 pass / 0 failbun run typecheck— exit 0Checklist