fix(auth): preserve per-model cooldown windows by siewcapital · Pull Request #45113 · openclaw/openclaw

siewcapital · 2026-03-13T13:14:25Z

Summary

Problem: auth-profile cooldowns were being made model-aware for rate limits, but an active model-scoped cooldown could still be rewritten by a later transient failure during the same window.
Why it matters: that rewrite widened the cooldown back to profile-wide behavior and could block same-provider fallback models again.
What changed: store the triggering model on rate-limit cooldowns, use model-aware cooldown checks in fallback and embedded-runner paths, and preserve cooldown metadata when an active cooldown window is intentionally kept immutable.
What did NOT change (scope boundary): auth/billing-style profile-wide failures still remain profile-wide, and this PR does not change model-id normalization behavior beyond the existing string comparison.

Change Type (select all)

Scope (select all touched areas)

Linked Issue/PR

User-visible / Behavior Changes

Rate-limit cooldowns on one model no longer block fallback to a different model on the same auth profile, and later transient failures during that active cooldown window no longer collapse the cooldown back to profile-wide behavior.

Security Impact (required)

New permissions/capabilities? No
Secrets/tokens handling changed? No
New/changed network calls? No
Command/tool execution surface changed? No
Data access scope changed? No

Repro + Verification

Environment

OS: macOS
Runtime/container: local workspace
Model/provider: auth-profile cooldown + fallback paths
Integration/channel (if any): N/A
Relevant config (redacted): auth profiles with same-provider multi-model fallback

Steps

Put an auth profile into a rate-limit cooldown for model A.
Run fallback/model-selection for model B on the same provider/profile.
Trigger a later transient cooldown-class failure during the active window.

Expected

Model B is not blocked just because model A was rate-limited.
The active cooldown keeps its original model-scoped metadata until expiry.

Actual

Verified by regression tests after this patch.

Evidence

Attach at least one:

Failing test/log before + passing after
Trace/log snippets
Screenshot/recording
Perf numbers (if relevant)

Human Verification (required)

What you personally verified (not just CI), and how:

Verified scenarios: model-aware cooldown check, preserved active cooldown metadata, same-provider fallback remains available when the stored cooldown is for a different model.
Edge cases checked: non-rate-limit cooldowns still block other models; active cooldown windows still do not extend on retry.
What you did not verify: the *.e2e.test.ts file via the default pnpm test entrypoint, because this repo excludes E2E tests from that filter.

Review Conversations

I replied to or resolved every bot review conversation I addressed in this PR.
I left unresolved only the conversations that still need reviewer or maintainer judgment.

Compatibility / Migration

Backward compatible? Yes
Config/env changes? No
Migration needed? No

Failure Recovery (if this breaks)

How to disable/revert this change quickly: revert this PR or switch back to provider-wide cooldown behavior by removing the model-aware cooldown checks.
Files/config to restore: src/agents/auth-profiles/usage.ts, src/agents/model-fallback.ts, src/agents/pi-embedded-runner/run.ts
Known bad symptoms reviewers should watch for: same-provider fallback models being skipped after a rate limit on a different model, or cooldown metadata changing mid-window.

Risks and Mitigations

List only real risks for this PR. Add/remove entries as needed. If none, write None.

Risk: model IDs are compared as strings, so alias/canonical mismatches would not count as the same model.
Mitigation: current callers already pass normalized model IDs; this PR keeps that assumption explicit and covered by focused tests.

greptile-apps · 2026-03-13T13:17:48Z

Greptile Summary

This PR correctly addresses the root bug where a later transient failure could overwrite a model-scoped rate-limit cooldown and revert it to profile-wide behavior. The approach — storing cooldownReason/cooldownModel on the stats, preserving that metadata when an active window is kept immutable, and threading forModel/modelId through the three call-sites — is clean and well-tested for the happy paths.

One logic issue was found:

isProfileInCooldown model-aware bypass overrides disabledUntil (src/agents/auth-profiles/usage.ts lines 58–67): the early return false fires before the combined unusableUntil (which includes disabledUntil) is ever evaluated. A profile that is billing-disabled but later accumulates a rate-limit cooldownReason for model A can incorrectly appear available to model B. The fix is to guard the bypass with !isActiveUnusableWindow(stats.disabledUntil, ts).

One style/hygiene observation:

clearExpiredCooldowns leaves stale cooldownReason/cooldownModel after cooldownUntil is cleared (lines 224–227). No immediate incorrect behavior today, but it makes the on-disk store inconsistent and could silently interact with any future reader that checks cooldownReason without first confirming the window is active.

Confidence Score: 3/5

The core fix is sound, but there is a logic bug where the model-aware bypass can override an active disabledUntil, potentially allowing requests to reach a billing-disabled profile.
The metadata-preservation and same-provider fallback improvements work correctly for the targeted scenarios and are well-tested. However, the early return in isProfileInCooldown is too broad: it skips the unusableUntil guard entirely, meaning a profile with both an active disabledUntil (billing/auth) and a subsequently written cooldownReason = "rate_limit" would pass the availability check for a different model. This is a real correctness issue even if the exact trigger sequence is uncommon in practice.
src/agents/auth-profiles/usage.ts — specifically the isProfileInCooldown early-return block and the clearExpiredCooldowns cleanup section.

Comments Outside Diff (1)

src/agents/auth-profiles/usage.ts, line 224-227 (link)

clearExpiredCooldowns leaves stale cooldownReason/cooldownModel after expiry

When cooldownUntil is cleared, the companion cooldownReason and cooldownModel fields are left behind. This doesn't cause an immediate functional bug today — because once unusableUntil is null both the model-aware bypass and the final return evaluate to false — but it makes the on-disk store inconsistent (window gone, metadata still present) and would quietly interact with any future reader that inspects cooldownReason without first confirming the window is active.

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/agents/auth-profiles/usage.ts
Line: 224-227

Comment:
**`clearExpiredCooldowns` leaves stale `cooldownReason`/`cooldownModel` after expiry**

When `cooldownUntil` is cleared, the companion `cooldownReason` and `cooldownModel` fields are left behind. This doesn't cause an immediate functional bug today — because once `unusableUntil` is null both the model-aware bypass and the final `return` evaluate to `false` — but it makes the on-disk store inconsistent (window gone, metadata still present) and would quietly interact with any future reader that inspects `cooldownReason` without first confirming the window is active.



How can I resolve this? If you propose a fix, please make it concise.

Prompt To Fix All With AI

This is a comment left during a code review.
Path: src/agents/auth-profiles/usage.ts
Line: 58-67

Comment:
**Model-aware bypass silently overrides an active `disabledUntil`**

The early return at lines 58–67 fires before `unusableUntil` (which combines `cooldownUntil` **and** `disabledUntil`) is ever tested. This means a profile that is genuinely disabled (e.g. billing or `auth_permanent`) can incorrectly appear available to model B if its `cooldownReason` was subsequently set to `"rate_limit"` for model A.

Concrete sequence that produces the wrong answer:

1. Billing failure → `disabledUntil = T+5h`, `disabledReason = "billing"`, `cooldownReason = undefined` (cleared by the billing branch in `computeNextProfileUsageStats`).
2. Profile is retried anyway (e.g. via a transient-probe path or a concurrent worker) and gets a rate-limit for model A → the `else` branch sets `cooldownUntil`, `cooldownReason = "rate_limit"`, `cooldownModel = "model-A"`. Because the spread `{ ...params.existing, ... }` in `computeNextProfileUsageStats` is used, `disabledUntil` from step 1 is **inherited** into the new stats.
3. Model B asks `isProfileInCooldown` → the guard at line 58 fires, returns `false` → profile appears available despite the still-active `disabledUntil`.

The fix is to skip the model-aware shortcut whenever `disabledUntil` is itself active:

```suggestion
  if (
    !isActiveUnusableWindow(stats.disabledUntil, ts) &&
    stats.cooldownReason === "rate_limit" &&
    typeof forModel === "string" &&
    forModel.trim().length > 0 &&
    typeof stats.cooldownModel === "string" &&
    stats.cooldownModel.trim().length > 0 &&
    stats.cooldownModel.trim() !== forModel.trim()
  ) {
    return false;
  }
```

`isActiveUnusableWindow` is already defined in this file so no new helper is needed.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: src/agents/auth-profiles/usage.ts
Line: 224-227

Comment:
**`clearExpiredCooldowns` leaves stale `cooldownReason`/`cooldownModel` after expiry**

When `cooldownUntil` is cleared, the companion `cooldownReason` and `cooldownModel` fields are left behind. This doesn't cause an immediate functional bug today — because once `unusableUntil` is null both the model-aware bypass and the final `return` evaluate to `false` — but it makes the on-disk store inconsistent (window gone, metadata still present) and would quietly interact with any future reader that inspects `cooldownReason` without first confirming the window is active.

```suggestion
    if (cooldownExpired) {
      stats.cooldownUntil = undefined;
      stats.cooldownReason = undefined;
      stats.cooldownModel = undefined;
      profileMutated = true;
    }
```

How can I resolve this? If you propose a fix, please make it concise.

_{Last reviewed commit: dd78c52}

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: dd78c52f17

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

siewcapital · 2026-03-14T05:08:19Z

Pushed 87c4111.

This follow-up does two things in src/agents/auth-profiles/usage.ts:

guard the model-specific rate-limit bypass behind an inactive disabledUntil
clear cooldownReason and cooldownModel when an expired cooldown window is removed

Added regressions in src/agents/auth-profiles/usage.test.ts for both cases.

…mit message Combines ideas from PRs openclaw#45113, openclaw#31962, and openclaw#45763 to address three cooldown-related issues: 1. Stepped cooldown (30s → 1m → 5m cap) replaces the aggressive exponential formula (1m → 5m → 25m → 1h) that locked out providers for far longer than the actual API rate-limit window. 2. Per-model cooldown scoping: rate_limit cooldowns now record which model triggered them. When a different model on the same auth profile is requested, the cooldown is bypassed — so one model hitting a 429 no longer blocks all other models on the same provider. 3. FallbackSummaryError with soonest-expiry countdown: when all candidates are exhausted, the user sees a clear message like '⚠️ Rate-limited — ready in ~28s' instead of a generic failure. Files changed: - types.ts: add cooldownReason/cooldownModel to ProfileUsageStats - usage.ts: stepped formula, model-aware isProfileInCooldown, modelId threading through computeNextProfileUsageStats/markAuthProfileFailure - model-fallback.ts: FallbackSummaryError class, model-aware availability check, soonestCooldownExpiry computation - pi-embedded-runner/run.ts: thread modelId into failure recording - agent-runner-execution.ts: buildCopilotCooldownMessage helper, rate-limit detection branch in error handler - usage.test.ts: update expected cooldown value (60s → 30s)

altaywtf · 2026-03-25T19:10:19Z

Closing this as a duplicate to keep the discussion and landed fix in one place.

What shipped instead:

fix: per-model cooldown scope, stepped backoff, and user-facing rate-limit message #49834 — fix: per-model cooldown scope, stepped backoff, and user-facing rate-limit message #49834
landed commit: 8440122 (8440122)

Why this is duplicate:

This PR and fix: per-model cooldown scope, stepped backoff, and user-facing rate-limit message #49834 both implement the same core per-model cooldown fix across the same paths: src/agents/auth-profiles/usage.ts, src/agents/model-fallback.ts, and src/agents/pi-embedded-runner/run.ts.
The landed fix also carries the related test coverage and the follow-up hardening/UX work that was needed before merge.

If there is any behavior here that you think is still missing from #49834 on current main, please call it out there and we can follow up explicitly.

Thank you for the contribution, @siewcapital.

fix(auth): preserve per-model cooldown windows

dd78c52

openclaw-barnacle Bot added agents Agent runtime and tooling size: M labels Mar 13, 2026

greptile-apps Bot reviewed Mar 13, 2026

View reviewed changes

Comment thread src/agents/auth-profiles/usage.ts

chatgpt-codex-connector Bot reviewed Mar 13, 2026

View reviewed changes

Comment thread src/agents/auth-profiles/usage.ts

This comment was marked as spam.

Sign in to view

fix(auth): honor disabled windows in cooldown checks

87c4111

kiranvk-2011 mentioned this pull request Mar 18, 2026

fix: per-model cooldown scope, stepped backoff, and user-facing rate-limit message #49834

Merged

altaywtf closed this Mar 25, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(auth): preserve per-model cooldown windows#45113

fix(auth): preserve per-model cooldown windows#45113
siewcapital wants to merge 2 commits into
openclaw:mainfrom
siewcapital:codex/fix-per-model-cooldown

siewcapital commented Mar 13, 2026

Uh oh!

greptile-apps Bot commented Mar 13, 2026 •

edited

Loading

Comments Outside Diff (1)

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

Uh oh!

This comment was marked as spam.

siewcapital commented Mar 14, 2026

Uh oh!

altaywtf commented Mar 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Uh oh!

Conversation

siewcapital commented Mar 13, 2026

Summary

Change Type (select all)

Scope (select all touched areas)

Linked Issue/PR

User-visible / Behavior Changes

Security Impact (required)

Repro + Verification

Environment

Steps

Expected

Actual

Evidence

Human Verification (required)

Review Conversations

Compatibility / Migration

Failure Recovery (if this breaks)

Risks and Mitigations

Uh oh!

greptile-apps Bot commented Mar 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 3/5

Comments Outside Diff (1)

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

This comment was marked as spam.

siewcapital commented Mar 14, 2026

Uh oh!

altaywtf commented Mar 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

greptile-apps Bot commented Mar 13, 2026 •

edited

Loading