Skip to content

fix(server): retry a failed provider version lookup on the next probe - #11165

Closed
Mnigos wants to merge 1 commit into
pingdotgg:mainfrom
Mnigos:latest-version-retry
Closed

Mnigos wants to merge 1 commit into
pingdotgg:mainfrom
Mnigos:latest-version-retry

Conversation

@Mnigos

@Mnigos Mnigos commented Sep 11, 2026 •

Copy link
Copy Markdown
Contributor

The provider update prompt depends on resolveLatestProviderVersion, which asks the npm registry for the latest version under a 4 s timeout. When that single request times out (a cold DNS lookup on a filtering resolver can take over 10 s) or fails, the null result is cached for the full one-hour TTL, so deriveVersionAdvisory stays unknown and the "update available" toast never appears, even though the next lookup would take well under a second.

A failed or timed-out lookup is now cached for one minute instead of an hour, so the next provider probe retries it. Successful lookups keep the hour-long cache, and the request timeout is unchanged.

Verification

  • New test in providerMaintenance.test.ts: a 503 from the registry yields null with a 60 s cache entry, the failure is still served from cache inside that minute, and after the test clock passes it a 200 response resolves 2.0.0 with the hour-long entry.
  • vp test run apps/server/src/provider/providerMaintenance.test.ts: the new test and the existing cache tests pass. Two npm-ownership tests in that file fail identically on an untouched main checkout on this machine (they depend on the local npm prefix), so they are unrelated.
  • Server typecheck and lint on the touched files are clean.

Fixes #11163. Implemented with Claude Code (Claude Fable 5.1).

Summary by CodeRabbit

  • Bug Fixes
    • Failed npm registry lookups are retried after one minute instead of being cached for the full hour.
    • Successful version lookups continue to be cached for one hour, ensuring update notifications remain timely.

@cursor

cursor Bot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Bugbot is paused — on-demand spend limit reached

Bugbot uses usage-based billing for this team and has hit its on-demand spend limit.

A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:XS 0-9 changed lines (additions + deletions). labels Sep 11, 2026
@macroscopeapp

macroscopeapp Bot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at f9a59e6

Macroscope's review found this PR approvable — This narrowly scoped fix changes only failed provider-version lookups to retry after one minute instead of remaining cached for an hour; successful lookups remain cached for an hour. A focused regression test covers the retry and recovery behavior.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Sep 11, 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: CHILL

Plan: Advanced

Run ID: 1e70782f-971a-490e-8f57-416b3b37297f

📥 Commits

Reviewing files that changed from the base of the PR and between c52b8d9 and f9a59e6.

📒 Files selected for processing (2)
  • apps/server/src/provider/providerMaintenance.test.ts
  • apps/server/src/provider/providerMaintenance.ts

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


📝 Walkthrough

Walkthrough

Failed npm registry lookups now expire from the latest-version cache after 60 seconds. Successful lookups retain a one-hour expiry. Tests verify the failed lookup, retry window, and later successful lookup.

Changes

Provider version retry behavior

Layer / File(s) Summary
Cache retry policy and validation
apps/server/src/provider/providerMaintenance.ts, apps/server/src/provider/providerMaintenance.test.ts
Null latest-version results now use a 60-second cache expiry, while successful results use the existing one-hour expiry. Tests verify cached failure behavior and successful retry after the window.

Priority: ⬇️ Low

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

Severity of issue fixed: Low

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to f9a59

Failed provider version checks retry after one minute while successful results remain cached for an hour. No actionable merge risk remains.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: retrying failed provider version lookups sooner.
Description check ✅ Passed The description explains the problem, the one-minute failure cache, the unchanged successful cache and timeout behavior, and the test verification. It does not use the template headings or checklist, …
Linked Issues check ✅ Passed The implementation meets issue #11163 by caching failed or timed-out npm registry lookups for one minute, preserving the one-hour cache for successful lookups, and adding coverage for retry behavior a…
Out of Scope Changes check ✅ Passed The changes are limited to provider version cache expiry logic and its focused test. No unrelated code changes are identified.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2…
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@juliusmarminge

Copy link
Copy Markdown
Member

Thanks for the PR. We're not taking changes to the orchestration and provider layers right now: that part of the server is being rewritten for V2, and merging into the current code would either conflict with or be thrown away by that work.

Closing for now. If this is still an issue once V2 lands, please reopen (or open a fresh PR against the new code) and we'll take a proper look.

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

Labels

size:XS 0-9 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Provider update prompt never shows when npm registry lookup exceeds the hardcoded 4s timeout

2 participants