Skip to content

chore: port CodeRabbit review fixes from upstream PR - #5

Merged
François (Lp-Francois) merged 6 commits into
mainfrom
chore/port-coderabbit-fixes
Aug 14, 2026
Merged

chore: port CodeRabbit review fixes from upstream PR#5
François (Lp-Francois) merged 6 commits into
mainfrom
chore/port-coderabbit-fixes

Conversation

@Lp-Francois

Copy link
Copy Markdown

Ports the CodeRabbit-driven fixes from the upstream PR (yuseferi#19) back to this fork.

Changes

  • fix: clear the discovery timeout timer (withTimeout helper) so a completed discovery doesn't leave a pending timer holding a short-lived process open.
  • fix: throttle background cache refresh to a 5-minute interval so a burst of session.created events can't generate repeated health-check/discovery traffic.
  • feat: expire model cache entries older than 7 days; adds readModelCacheSavedAt used by the throttle.
  • fix: write the cache atomically (temp file + renameSync) so readers never see a partial JSON file.
  • refactor: mergeModels returns the ids it actually added; injectedModelIds stores only those (not every provider key). Uses Object.hasOwn so null/false/0 entries aren't treated as absent.
  • refactor: call homedir() once in cacheDir().

The info-log removal from PR #4 is preserved (the cache-load console.log was intentionally dropped during the port).

Testing

  • npm run typecheck passes.

Bound cache entries by savedAt so a permanently-gone proxy no longer
serves the same stale model list forever. Add readModelCacheSavedAt so
callers can throttle background refreshes on still-fresh caches.
Prevents readers from observing a partially-written JSON file when two
OpenCode processes refresh the same baseURL concurrently.
Extract a withTimeout helper that clears its setTimeout once the race
settles, so a completed discovery doesn't leave a pending timer holding
a short-lived process open.
Skip revalidation when the cache's savedAt is within REFRESH_MIN_INTERVAL_MS
so a burst of session.created events can't generate repeated health checks
and discovery traffic. refreshInFlight still guards concurrent runs.
mergeModels now returns the ids it actually added, and injectedModelIds
stores exactly those instead of every provider key (which included
user-curated entries). Also use Object.hasOwn so entries that are null,
false, or 0 aren't treated as absent.
@Lp-Francois
François (Lp-Francois) merged commit 2114908 into main Aug 14, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant