[iscekic]

Change

createCachedFetch (packages/web-shared/src/lib/cached-fetch.ts) previously ran the fetcher once per concurrent caller once an entry's TTL had expired, and retried a failing fetcher on every call with no backoff. Both are now fixed in the helper, so every existing consumer (11 call sites: preferred models, gateway model metadata caches, routing table, routing config, request-logging opt-ins, unavailable models, auto-free config, blacklist domains, providers catalogue, public snowflake reports) inherits the behaviour:

• Shared in-flight refresh. The pending promise is stored and returned to callers that arrive while a refresh is in flight, cleared in a finally block — the same pattern as createModelsByProviderIndexLoader. An expired entry can no longer produce N concurrent underlying queries.
• Failure backoff. A failed refresh is negatively cached for FAILURE_BACKOFF_MS (30s default, exported). Callers keep receiving the last-known-good or default value and no new fetch is attempted until the window passes. Callers can pass { failureBackoffMs: 0 } to restore retry-every-call; the focused unit tests use that option to keep TTL/refresh semantics deterministic.

No call-site changes were needed: every consumer already serves the stale/default value when the fetcher throws, so the backoff only replaces an unthrottled failing retry with a bounded one.

Verification

• jest suites: 32 tests pass across cached-fetch, preferred-models, request-logging-opt-ins, and unavailable-models (the suites that exercise this helper directly or through consumers). cached-fetch.test.ts gained three tests: N concurrent callers after TTL expiry run the fetcher once; a throwing fetcher runs once across many calls inside the backoff window and again at the deadline; the default value is served for the window when nothing is cached.
• pnpm --filter @kilocode/web-shared run typecheck passes; oxlint reports 0 warnings/errors on the changed files; oxfmt --list-different clean.
• End-to-end against local Postgres in a Next.js dev server: a route calling two real createCachedFetch consumers with their production TTLs was hit by 20 concurrent requests; Postgres statement logging (enabled via ALTER SYSTEM SET log_statement='all') captured exactly one select "openrouter" from "models_by_provider" ... and one select "preferred_models" from "ai_gateway_config" ... for the bursts. A pre-fix baseline of 20 independent (uncached) calls against the same sequence counted 20 query executions. Statement logging was disabled and the seeded row and throwaway route removed afterwards.
• A real /api/v1/providers request returned HTTP 200 with the seeded payload through the fixed helper.

Proxy answers

The owner's proxy gave these answers in the owner's place.

1. Question: Backoff scope: createCachedFetch gets a failure backoff. Should the backoff apply to every existing caller by default, or be opt-in per caller?
   Answer: Default for all callers (30s), opt-out per caller
   Confidence: 0.75 · Precedent: Fix the root cause and the class of the issue: session 7f3470e0ba6f, "fix class of issue preventing progress"
2. Question: API shape: Should the backoff window be configurable, and is 30 seconds right?
   Answer: Optional 4th arg { failureBackoffMs }, default 30s, constant exported
   Confidence: 0.70 · Precedent: Fix the root cause and the class of the issue: session 00eb2a1d3e4b, "if anything repeats the same mistake over and over again, fix the class"
3. Question: Clock in tests: How should the backoff be made deterministic in tests?
   Answer: Keep Date.now() and jest.spyOn(Date, 'now') as the existing test file does
   Confidence: 0.75 · Precedent: Do the least work possible with no drop in quality: session 7f3470e0ba6f, "do the least amount of work possible, with no drop in quality"
4. Question: Backoff conflict: The direct-byok model-list suite pins that a failed read is retried on the very next call (7 tests fail with the new default 30s backoff because a second read is now suppressed inside the backoff window). How should the conflict be resolved?
   Answer: Rewrite the direct-byok tests
   Confidence: 0.55 · Precedent: Fix the root cause and the class of the issue: session 00eb2a1d3e4b, "if anything repeats the same mistake over and over again, fix the class". Also: replace old paths instead of keeping them alongside: session ad6a3f589364, "alongside? why not instead?"

[kilo-code-bot]

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Executive Summary

The incremental change fixes the prior warning by storing failed refreshes with at: 0, and moving in-flight cleanup to the awaiting caller also fixes a permanent stuck-entry path when the fetcher throws synchronously.

Notes

• cached-fetch.ts:70 no longer refreshes the entry timestamp on failure, so the failureBackoffMs window is honored regardless of ttlMs, and failureBackoffMs: 0 restores retry-on-next-call.
• cached-fetch.ts:59-80 now assigns the refresh promise before attaching the caller-side finally. Previously a synchronous fetcher() throw let the inner finally run before inFlight was assigned, leaving a settled promise permanently cached as in-flight.
• The new cached-fetch.test.ts:171 case covers repeated synchronous failures across backoff windows and recovery.
• No memory/retention issue found: inFlight is cleared in finally and no other reference outlives a refresh.

Files Reviewed (2 files)

• packages/web-shared/src/lib/cached-fetch.ts
• packages/web-shared/src/lib/cached-fetch.test.ts

Previous Review Summaries (2 snapshots, latest commit e4734c0)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit e4734c0)

Status: No Issues Found | Recommendation: Merge

Executive Summary

The follow-up stops a failed refresh from looking fresh (at: 0), so the documented failureBackoffMs window is now honored exactly instead of being masked by ttlMs; the rewritten direct-byok suite verifies the backoff semantics deterministically.

Files Reviewed (2 files)

• packages/web-shared/src/lib/cached-fetch.ts
• packages/web-shared/src/lib/ai-gateway/providers/direct-byok/model-list.test.ts

Previous review (commit 5c629b3)

Status: 1 Issue Found | Recommendation: Address before merge

Executive Summary

The shared in-flight refresh and never-rejecting promise are correct, but re-storing cached.at on failure makes the effective refresh suppression max(failureBackoffMs, ttlMs), delaying recovery far past the documented 30s window for long-TTL consumers.

Overview

Severity	Count
CRITICAL	0
WARNING	1
SUGGESTION	0

Issue Details (click to expand)

WARNING

File	Line	Issue
packages/web-shared/src/lib/cached-fetch.ts	67	On failure the stale/default value is re-stored with at: Date.now(), so the TTL check keeps it "fresh" for the full ttlMs. The backoff deadline is only honored when failureBackoffMs > ttlMs. With the 30s default this delays recovery to 5 min for the routing table (TTL 5 min) and up to 1 hour for public Snowflake reports (IN_MEMORY_CACHE_TTL_MS = 3600 * 1000), and it breaks the documented failureBackoffMs: 0 opt-out when ttlMs > 0.

Files Reviewed (2 files)

• packages/web-shared/src/lib/cached-fetch.ts - 1 issue
• packages/web-shared/src/lib/cached-fetch.test.ts - 0 issues

Fix these issues in Kilo Cloud

---

_{Reviewed by deepseek-v4.1-flash-1017 · Input: 34.9K · Output: 4.8K · Cached: 172.4K}

_{Review guidance: REVIEW.md from base branch main}

[iscekic]

bot: Fixed in e4734c0. The failure path now stores the stale/default value with at: 0 instead of Date.now(), so the first check (now - cached.at < ttlMs) can no longer satisfy the freshness condition with a still-fresh last-known-good value and mask the backoff window. The refresh suppression is exactly failureBackoffMs after a failure, and the stored value does not become artificially younger (once the window passes, the next call refreshes normally). The direct-byok model-list tests were rewritten per the owner's decision to assert the backoff semantics directly.

[chrarnoldus]

bot: Looks good. The shared in-flight refresh, the never-rejecting shared promise, and storing the failure fallback with at: 0 so the backoff window is honored exactly all check out. I checked every current caller: all fetchers are async functions or return promises from async functions, plain data, no request-scoped APIs, so sharing one refresh is safe.

One non-blocking hardening suggestion: inFlight = (async () => { ... finally { inFlight = null } })() pins inFlight forever if a fetcher ever throws synchronously. The IIFE runs its catch/finally synchronously before the assignment, so inFlight ends up holding an already-settled promise and every later call returns it without fetching again. Reproduced with a plain () => { throw ... } fetcher: the fetcher is never called again, even after the TTL and backoff. No current caller can hit this (all fetchers are async), but it's a shared helper. Assigning first avoids it:

const refresh = (async (): Promise<T> => { /* try/catch without finally */ })();
inFlight = refresh;
void refresh.finally(() => {
  if (inFlight === refresh) inFlight = null;
});
return await refresh;

plus a test with a synchronously throwing fetcher.

[iscekic]

bot: Addressed the synchronous-throw suggestion in e3a8db3. The initiating caller now assigns the refresh promise before awaiting it and clears inFlight in its own finally block. Added a regression covering repeated synchronous failures, suppression inside each backoff window, and successful recovery at the deadline. The pre-fix probe stayed on fallback with one attempt; the fixed probe recovered on attempt two. Verification: 10 cached-fetch tests passed, targeted TypeScript and lint checks passed, and a loopback HTTP smoke run served two fallback requests followed by 20 concurrent recovery requests sharing one upstream read.