Skip to content

fix(codex): recover WebSocket model refusals across pool accounts - #5486

Closed
luvs01 wants to merge 4 commits into
lidge-jun:devfrom
luvs01:fix/astra-refusal-recovery
Closed

luvs01 wants to merge 4 commits into
lidge-jun:devfrom
luvs01:fix/astra-refusal-recovery

Conversation

@luvs01

@luvs01 luvs01 commented Sep 22, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • Follow-up to [Bug]: v2.57.0 still selects Free account for Sol/Astra after roster refresh #4906. A precommit Codex WebSocket model refusal is projected into HTTP 400 with error.message, but the pool recovery detector read only HTTP's detail envelope. The exact unsupported-model refusal therefore skipped both the alternate-account attempt and remembered denial evidence, allowing the same account to be selected again.
  • Accept either unambiguous envelope while retaining HTTP 400, the complete refusal sentence, requested/wire model identity, bounded body reads and all existing account-selector, uploaded-file and send-budget restrictions. No plan-name entitlement rule, quota threshold change, model substitution or unbounded retry is added.
  • Regressions cover the actual WebSocket projection, one alternate and next-turn avoidance, recovery to the caller-owned main credential, stopping after a refused alternate, malformed/competing envelopes, and fill-first with quota switching disabled and 94% main headroom.

Verification

The diagnostic run IDs below refer to contributor-side executions; they are not upstream required-check results.

  • Before the production fix, all three new reproductions failed: envelope extraction, WebSocket projection-to-recovery, and real server alternate dispatch. The same cases passed after the fix.
  • Focused recovery checks: 4 tests / 28 assertions passed; main-account and bounded-retry server checks: 3 tests / 14 assertions passed on pinned Bun 1.4.0.
  • Typecheck, structure checks, privacy scan and git diff --check passed. Import-connected bun run test:changed selected 238/1502 files but exhausted its 900-second budget (exit 124), including Windows ACL timeouts and spend-ledger fixture owner conflicts; no broad-suite pass is claimed.
  • Hosted Linux (source diagnostic run 35687818509, job 106618320451) and hosted Windows (source diagnostic run 35687818509, job 106618320600) each checked out exact head 6443712af4fc7440728555d78340297c5109d2ba with Bun 1.4.0 and passed the complete four regression files: 276 pass, one existing skip, zero failures. The Free/monthly-100 and main/weekly-6 selection cases pass on both platforms.
  • Independent read-only review found no blocking issue. The running package was not replaced; no deployed fix is claimed before integration and release.
  • The first full CI reported the repository's file-size ratchet on server-auth.test.ts. The follow-up extracts its existing pool retry harness into tests/helpers/codex-pool-retry.ts, reducing the parent to 4,447 lines without raising any cap or moving test declarations. The moved function bodies and all 101 direct test declarations were compared unchanged; five focused server checks pass after extraction. Final hosted verification includes the file-size contract.
  • Windows exposed a race in the existing stalled-body fixture: its 5.1-second producer timer could finish before a delayed 5-second body inspection. The fixture now delegates to the real bounded reader and releases the suffix only after observing its real timeout; production deadlines are unchanged. Both previously failing Windows cases pass on the final revision.
  • Source diagnostic head 2149c4b2a921c152226e0cdca6865cd9cdad8a1a: Linux (source diagnostic run 35689902777, job 106624464136) and Windows (source diagnostic run 35689902777, job 106624464264) each passed all five focused files including the file-size contract: 285 pass, one existing skip, zero failures, on Bun 1.4.0. These scoped results do not claim a full repository suite pass.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults. Explicit maintainer review is requested before integration.

@coderabbitai

coderabbitai Bot commented Sep 22, 2026

Copy link
Copy Markdown
Contributor

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@luvs01
luvs01 requested a review from Ingwannu September 22, 2026 04:41
@github-actions

Copy link
Copy Markdown
Contributor

✅ Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the bug Something isn't working label Sep 22, 2026
@lidge-jun

Copy link
Copy Markdown
Owner

리뷰 · 우선순위 60 / 80

이 PR은 Codex 풀이 “이 계정은 이 모델을 못 쓴다”는 거절을 보고도 다른 계정으로 안 넘어가던 구멍을 막습니다. 예전 #4906에서 HTTP detail 문장은 잡았는데, WebSocket 쪽은 같은 거절을 error.message 봉투로 바꿔 HTTP 400에 실어 보냅니다. 복구 감지기(codexUnsupportedModelFromDetail)는 detail만 봐서, Astra처럼 계정에 묶인 모델 거절이 대체 계정 시도도 안 하고 거절 기록도 안 남긴 채 같은 계정을 또 골랐습니다. 이번 수정은 detail과 error.message 중 하나만 있을 때만 읽고, 문장 전체가 맞고 HTTP 400일 때만 모델 id를 뽑습니다. 둘 다 있거나 문장 앞뒤에 군더더기가 있으면 거절하지 않습니다. 계획 이름·쿼터 문턱·모델 바꿔치기·무한 재시도는 건드리지 않습니다. structure 문서와 단위·서버·선택 회귀 테스트가 같이 왔고, base는 dev입니다.

라인 - tests/server/server-auth.test.ts — CI test 4/4가 실패합니다. file-size ratchet이 이 파일을 GREW로 잡았습니다. 기준선은 4589줄인데 이번 커밋은 4642줄입니다. 새로 넣은 Astra 봉투 복구 테스트 세 개가 그 파일에 붙으면서 난 실패입니다. 기능 테스트 자체 실패가 아닙니다.
라인 - codexUnsupportedModelFromDetail의 hasDetail === hasError — 키가 둘 다 있으면 무조건 버립니다. 의 WS 투영은 error만 보내고 테스트도 그걸 고정합니다. 나중에 업스트림이 detail과 error를 같이 실어 보내면, 문장이 같아도 복구가 다시 꺼집니다.
라인 - tests/codex-integration/codex-account-selection-preferences.test.ts의 fill-first·threshold 0 케이스 — 봉투 파서 수정과 직접 연결된 건 아니고, 거절 증거가 자동 커서/메인 선택에 어떻게 쓰이는지 보강입니다. 범위는 이해되지만, 실패 경로는 봉투 쪽과 선택 쪽이 갈라져 있습니다.

메인테이너의 판단이 필요한 지점

큰 테스트 파일에 줄을 더 붙일지, 새 서버 복구 테스트를 더 작은 파일로 빼서 ratchet을 통과시킬지, 아니면 기준선만 올리는 예외를 줄지 정해야 합니다. 보안·auth 경로를 건드리는 풀 재시도라 작성자가 메인테이너 확인을 요청한 점도 맞습니다. 같은 WebSocket error.message 거절 복구를 다루는 열린 중복 PR은 보이지 않습니다. #5024(요청 소유 메인)와는 겹치는 면이 있어도 이 봉투 구멍과는 다른 일입니다.

너의 추천

복구 방향과 봉투 XOR·문장 전체 매칭은 맞습니다. 머지 전에 server-auth.test.ts 성장을 정리해 ratchet을 통과시키세요. 새 케이스는 전용 작은 테스트 파일로 옮기는 편이 기준선 올리기보다 낫습니다. types/config 쪼개기·중복 닫기·preview deploy는 이번 변경과 무관합니다.

이 댓글은 grok-bot이 작성했습니다

luvs01 and others added 3 commits September 22, 2026 13:55
A deferred settle can land after server.stop has released the spend-ledger
owner: the journal is already closed, the outstanding sends have nobody to
book against, and the thrown SpendLedgerOwnerError escapes as an unhandled
rejection that kills the process. That crashed 'an unsafe configured key id
is omitted with one id-free warning' in the refusal-recovery windows run
when a client-cancel drain fired mid-test. Send records die with the
discarded ledger; non-ownership failures still propagate.

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@luvs01

luvs01 commented Sep 22, 2026

Copy link
Copy Markdown
Collaborator Author

Consolidated into #5504 in native Stack #5505.

Source head: 3dd0dfdb6830ddcd33612b77b17da0083f719ff9. Replacement head: 271fde4a4bd59b9f11b3e781d639d62edef1707f. All 4 unique non-merge source commit(s) were matched to their cherry-picked replacement commits using stable Git patch IDs; attribution and provenance are preserved.

Closing this standalone PR as a duplicate review entry at the author's request. The change is not merged or released: the replacement remains draft, with remaining integration/CI/security-review work tracked there. Original branches are retained. The maintainer-cancelled CI is not treated as a pass.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants