Skip to content

fix(codex): let clear-cooldown lift an avoidance the cooldown outlived - #4397

Merged
lidge-jun merged 1 commit into
devfrom
codex/260912-routing-avoid-lapsed
Sep 12, 2026
Merged

fix(codex): let clear-cooldown lift an avoidance the cooldown outlived#4397
lidge-jun merged 1 commit into
devfrom
codex/260912-routing-avoid-lapsed

Conversation

@lidge-jun

@lidge-jun lidge-jun commented Sep 12, 2026

Copy link
Copy Markdown
Owner

Summary

  • Let clearCodexAccountCooldown act on a live avoidance window when the cooldown it belonged to has already lapsed. The two durations differ by design — the cooldown caps at fifteen minutes, the window a refusal announces runs up to six hours — so the point where an operator reaches for this escape hatch is normally after the cooldown expired and only the window is still holding the account out of rotation. The call returned early there, reported cleared: false, and left selection passing the account over for hours.
  • Regression test for exactly that window: the cooldown is confirmed lapsed, selection is confirmed to be avoiding the account, and the call has to both report success and return the account to selection.

Verification

  • Follow-up to fix(codex): apply a quota refusal's avoidance window to the main login #4396, which fixed the three avoidance gaps the regression sweep found. This is the fourth, raised in review on that PR against the same escape-hatch contract.
  • The existing contract is unchanged otherwise: failure counters and softAvoidUntil still survive, and an account with neither a live cooldown nor a live window still returns false, so the route still cannot be used to tell whether an account exists.
  • Local tests, build, typecheck and install: NOT RUN under the standing restriction. Hosted CI is the gate.

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.

Summary by CodeRabbit

  • Bug Fixes

    • Clearing an expired account cooldown now also removes any remaining quota-avoidance period, allowing the account to become selectable again.
    • Cooldown clearing now accurately reports whether any active restriction was removed.
  • Tests

    • Added coverage for clearing lingering quota avoidance after a cooldown expires.

clearCodexAccountCooldown returned before it looked at quotaAvoidUntil whenever
cooldownUntil had already passed. The two durations have different lengths on
purpose — the cooldown caps at fifteen minutes, the announced window runs up to
six hours — so the moment an operator actually reaches for this escape hatch is
usually after the cooldown lapsed and only the window is still keeping the
account out of rotation. The call was a no-op there, and the dashboard reported
cleared: false while selection kept passing the account over for hours.

Treat a live avoidance window as its own reason to act, and keep the existing
contract otherwise: failure counters and softAvoid still survive.
@lidge-jun
lidge-jun requested a review from Ingwannu as a code owner September 12, 2026 11:26
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 12, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-12T11:30:03.883157Z bd981da PR opened
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

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

Copy link
Copy Markdown
Contributor

Deterministic PR hygiene checks passed.

@coderabbitai

coderabbitai Bot commented Sep 12, 2026

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 5a7abddd-34c8-4171-95a5-4c4d0f1f0606

📥 Commits

Reviewing files that changed from the base of the PR and between 4b6849c and bd981da.

📒 Files selected for processing (2)
  • src/codex/routing.ts
  • tests/codex-integration/codex-routing.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

The change updates clearCodexAccountCooldown to clear live quota avoidance after a hard cooldown expires. A new integration test verifies that the account becomes selectable again.

Changes

Codex routing cooldown clearing

Layer / File(s) Summary
Clear quota avoidance state
src/codex/routing.ts, tests/codex-integration/codex-routing.test.ts
clearCodexAccountCooldown now clears a future quotaAvoidUntil value and returns false only when neither cooldown state is active. The integration test covers a lapsed cooldown with remaining Spark avoidance and verifies account selection after clearing.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to bd981

The change clears lingering quota avoidance after a cooldown expires and includes regression coverage for account reselection. No merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: allowing clear-cooldown to remove an avoidance window that outlasted the cooldown.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/260912-routing-avoid-lapsed

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.

@lidge-jun

Copy link
Copy Markdown
Owner Author

리뷰 · 우선순위 73 / 80

이 PR은 방금 dev에 들어간 #4396의 네 번째 구멍이다. #4396은 메인 로그인 선택에 quotaAvoidUntil을 넣고, 수동 선택 리셋과 clearCodexAccountCooldown이 회피 창을 같이 지우도록 고쳤다. 그런데 그 쿨다운 클리어 함수는 여전히 “지금 살아있는 쿨다운이 있을 때만” 본문을 탔다. 쿨다운은 설계상 최대 15분이고, 거절이 발표한 회피 창은 최대 6시간이다. 운영자가 대시보드에서 이 탈출 버튼을 누르는 순간은 보통 쿨다운이 이미 끝난 뒤, 회피 창만 계정을 골라내지 못하게 잡고 있는 때다. 그 순간에는 함수가 바로 null을 반환해 cleared: false가 되고, 선택 쪽은 몇 시간 더 그 계정을 건너뛴다. 탈출구가 정작 필요한 순간에 잠겨 있던 셈이다.

지금 devsrc/codex/routing.ts를 보면 clearCodexAccountCooldown 안의 clear 헬퍼가 cooldownUntil > now일 때만 필드를 빼고, 그때 quotaAvoidUntil도 같이 버린다. 이번 변경은 조건을 liveCooldown || liveAvoidance로 바꾼다. 둘 다 없으면 예전처럼 null이라서, 쿨다운도 회피도 없는 계정에 대해 false를 돌려 계정 존재 여부를 알려주지 않는 계약은 그대로다. 실패 카운터와 softAvoidUntil은 여전히 ...rest에 남는다. “쿼터 창이 움직였다”는 말이지 “이 계정은 건강하다”는 말이 아니라는 #4396 때의 의도도 유지된다. 계정 전역 upstreamHealthquotaScopedHealth 양쪽 맵에 같은 clear를 돌리는 구조도 그대로라, 스코프만 회피가 남아 있는 reset-derived 429 케이스도 같은 탈출구로 풀린다.

테스트는 그 장면을 그대로 재현한다. 429로 회피 창을 세운 뒤 16분이 지나 쿨다운은 이미 끝났고(isCodexAccountInCooldown가 false), 선택은 다른 계정(b)으로 간다. 그다음 clearCodexAccountCooldowntrue를 주고, 곧이어 같은 스파크 레인에서 계정 a가 다시 뽑힌다. #4396이 남긴 “쿨다운이 살아 있을 때만 회피도 지운다” 회귀를 닫는 최소 변경이다. types.ts/config.ts 분리 캠페인과도 무관하고, Hermes·remote-workspace 캐리와도 겹치지 않는다. 현재 기차가 막 올린 쿼터 회피(#4368+#4396)의 탈출구 계약을 완성하는 짧은 후속이다.

라인 1079 근처 clearCodexAccountCooldown JSDoc - 반환 설명을 “쿨다운도 회피 창도 없으면 false”로 고친 것은 맞다. 다만 함수 이름은 여전히 clearCodexAccountCooldown라서, 문서만 읽고 호출하는 쪽은 “쿨다운만 푼다”고 오해할 여지가 남는다.

라인 1083~1087 clear 헬퍼 - liveCooldown과 liveAvoidance를 각각 검사한 뒤 둘 다 없으면 null이다 흐름이 읽기 쉽다. 회피만 살아 있을 때도 cooldownGeneration을 올리고 lastProbeAt을 찍는 동작은 프로브 리스/세대 불변식과 맞춰 두었고, 쿨다운만 있을 때와 같은 경로라 분기 폭이 크지 않다.

경로 tests/codex-integration/codex-routing.test.ts “clearing a lapsed cooldown still lifts the avoidance it left behind” - 쿨다운 만료 확인 → 선택 회피 확인 → clear 성공 → 재선택까지 한 테스트에 묶여 있어 회귀 의도가 분명하다. softAvoid·실패 카운터가 남는지는 기존 “lifts a live cooldown but keeps failure history” 테스트가 계속 커버한다.

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

  • 함수 이름을 clearCodexAccountCooldown으로 둘지, 회피까지 푼다는 뜻을 이름에 넣을지(호출부·대시보드 카피까지 같이 손볼지)
  • 이 탈출구를 GUI/문서에서 “쿨다운 해제”로만 표기할지, “쿨다운·회피 해제”로 고칠지
  • #4396과 같은 기차에 바로 올릴지, 아니면 짧은 단독 핫픽스로 먼저 넣을지

너의 추천
CI 초록이면 dev에 바로 머지하는 쪽으로 간다. #4396이 연 탈출구 계약을 실제로 쓸 수 있게 만드는 최소 수정이고, 범위도 routing.ts와 회귀 테스트 한 개뿐이다. 이름 변경·카피 정리는 원하면 후속 이슈로 빼도 된다. 지금 당장 막아야 할 동작은 이 PR이 이미 고친다.

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

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: bd981dac93

ℹ️ 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".

Comment thread src/codex/routing.ts
Comment on lines +1088 to +1090
const avoidUntil = health.quotaAvoidUntil;
const liveAvoidance = typeof avoidUntil === "number" && Number.isFinite(avoidUntil) && avoidUntil > now;
if (!liveCooldown && !liveAvoidance) return null;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Update the owned routing docs with this behavior

This changes shared Codex routing and the operator-facing clear-cooldown contract: a live avoidance window now makes the operation succeed after the hard cooldown expires. However, none of the structure documents mapped to src/codex/ were updated, so documents such as structure/catalog.md and structure/gui-and-management-api.md still omit the new selection and management semantics; update the applicable owned documentation in this change.

AGENTS.md reference: src/AGENTS.md:L10-L11

Useful? React with 👍 / 👎.

@lidge-jun

Copy link
Copy Markdown
Owner Author

Maintainer integration decision (dev only, per MAINTAINERS.md). Exact head bd981dac9311630d7553cab33a3ae50ceae54760: every check green. Fourth and last gap in the #4368 avoidance contract, raised in review on #4396. Local tests, build, typecheck and install: NOT RUN; hosted CI is the gate.

@lidge-jun
lidge-jun merged commit f2f1783 into dev Sep 12, 2026
31 checks passed
@lidge-jun
lidge-jun deleted the codex/260912-routing-avoid-lapsed branch September 12, 2026 11:35
Vocllum pushed a commit to Vocllum/opencodex that referenced this pull request Sep 12, 2026
lidge-jun#4397 made a live avoidance window its own reason for the clear-cooldown
operation to act, which changes the operator-facing contract. structure/INDEX.md
maps src/codex/ to this document, so the SSOT has to say that a lapsed cooldown
with a live window still reports a clear, and that an account with neither still
reports no change.
cgq0816 pushed a commit to cgq0816/opencodex that referenced this pull request Sep 12, 2026
Product tree is dev at 7bfb2ad. Only package.json differs, carrying the
stable channel version 2.52.0.

This promotion follows a CI repair and a regression sweep: dev had drifted 79
commits past its last finished run and the one run allowed to complete had
failed. lidge-jun#4390 fixed an integration test that never denied anything and had been
red on dev since lidge-jun#4342; lidge-jun#4396/lidge-jun#4397/lidge-jun#4398 closed four gaps in the quota
avoidance contract lidge-jun#4368 introduced; lidge-jun#4403 made a port fixture deterministic.
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.

1 participant