Skip to content

fix(server): turning off provider update checks no longer hides new models - #19343

Closed
t3dotgg wants to merge 1 commit into
mainfrom
t3/fix-13244
Closed

t3dotgg wants to merge 1 commit into
mainfrom
t3/fix-13244

Conversation

@t3dotgg

@t3dotgg t3dotgg commented Oct 14, 2026 •

Copy link
Copy Markdown
Member

Turning off Provider update checks also stopped T3 Code from fetching the model catalog, even on an explicit Refresh provider status. New Claude models that only exist in the remote catalog stayed hidden until the switch was turned back on. The setting says it only checks provider CLIs for newer versions.

Fix

ModelManifest.refresh no longer reads enableProviderUpdateChecks. The switch now only gates CLI version lookups, which are unchanged. The catalog fetch keeps its existing TTL and retry timers, so there are no new network calls compared to having the switch on. There is no setting that turns off all network access, so nothing else needed to gate this fetch.

The triage on the issue called the gate deliberate and offered two fixes: copy that states the coupling, or decoupling. #14549 was closed for lacking approval of the behavior change. This PR takes the decoupling option. That behavior change still needs maintainer approval before it merges.

Proof

  • Replaced the ModelManifest.test.ts test that asserted no fetch with the switch off. The new test asserts that refresh and forceRefresh both return the remote catalog with the switch off. It fails on main and passes with this change.
  • vp test run src/provider/ModelManifest.test.ts: 12/12 pass. apps/server typecheck passes. Lint is clean.

Fixes #13244

Reviewed with sol-loop: 1 round with GPT-6.1 Sol.

Model and harness: Claude Opus 5.5 in Claude Code via T3 Code.

🤖 Generated with Claude Code

… model catalog

enableProviderUpdateChecks also returned before ModelManifest fetched
model-manifest.json, including an explicit Refresh provider status. New
Claude models that only exist in the remote catalog stayed hidden.

The switch now only gates CLI version lookups.

Fixes #13244

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:S 10-29 changed lines (additions + deletions). labels Oct 14, 2026
@juliusmarminge juliusmarminge added the macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews label Oct 14, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 14, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at b6a00d4

Macroscope's review found this PR approvable — This is a focused server bug fix that decouples model-catalog refreshes from CLI version checks without changing defaults or cache/retry behavior. A targeted test verifies that disabled CLI checks still allow the remote catalog to refresh and expose new models.

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

@coderabbitai

coderabbitai Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Path: .coderabbit.config.ts
  • Review profile: CHILL
  • Plan: Team
  • Run ID: ce378511-a72c-4f5e-811b-2b559cf9b667
📥 Commits

Reviewing files that changed from the base of the PR and between 9a01524 and b6a00d4.

📒 Files selected for processing: 2
  • apps/server/src/provider/ModelManifest.test.ts
  • apps/server/src/provider/ModelManifest.ts

Included review availability: This review used your included allowance. 1 included review remains after this review. Your included PR review attempts over the past 7 days set your current allowance at 10 reviews per hour.

📝 Walkthrough

Provider catalog refreshes now attempt a remote fetch regardless of the provider update-check setting, after freshness and retry checks pass. Updated tests verify that refresh and forceRefresh return the remote manifest when the setting is disabled.

Changes: Provider catalog refresh

Layer / File(s) Summary
Ungated catalog refresh
apps/server/src/provider/ModelManifest.ts, apps/server/src/provider/ModelManifest.test.ts
Removed the settings dependency and update-check gate from catalog refreshes. Tests now expect refresh and forceRefresh to return the remote manifest and make two requests when provider update checks are disabled.

Priority: ➖ Normal

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

Change: Bug fix · Severity of issue fixed: Low

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to b6a00

Disabling CLI version checks no longer prevents catalog refreshes, and the supplied review context identifies no material regression. No merge-blocking risk is evident.

Pre-merge checks: Passed 3 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Description check Warning The description explains the problem, implementation, scope, and verification results. However, the required explicit maintainer approval is missing; the description states that approval is still need… Add a link to the triaged issue or discussion with an explicit maintainer approval comment for this behavior change. If the change qualifies as an obvious bug fix without prior approval, explain that exemption instead.
Passed Passed checks: 3 passed
Check name Status Explanation
Linked Issues check Passed The change satisfies the coding requirement in #13244. ModelManifest.ts no longer reads or applies enableProviderUpdateChecks before the catalog fetch. The existing freshness and retry checks rema…
Out of Scope Changes check Passed The implementation change is limited to decoupling the remote model-catalog fetch from the CLI version-check setting. The test update verifies the behavior required by #13244. No unrelated source or t…
Title check Passed The title clearly and concisely describes the main change: disabling provider update checks no longer hides new models.
Full details: Description check

Explanation

The description explains the problem, implementation, scope, and verification results. However, the required explicit maintainer approval is missing; the description states that approval is still needed before merge.

  • Fix all pre-merge checks with AI
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

@github-actions

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire 4.9 KiB 4.9 KiB 0 B (0.0%) 6.8 KiB ✅
Codex Thread snapshot wire 3.8 KiB 3.8 KiB 0 B (0.0%) 4.9 KiB ✅
Codex Live turn WebSocket wire 1.2 KiB 1.2 KiB 0 B (0.0%) 2.0 KiB ✅
Codex Live turn WebSocket decoded 20.8 KiB 20.8 KiB 0 B (0.0%) 29.3 KiB ✅
Codex Live turn messages 1 1 0 (0.0%) 8 ✅
Claude Total thread wire 5.0 KiB 5.0 KiB −24 B (−0.5%) 6.8 KiB ✅
Claude Thread snapshot wire 3.8 KiB 3.8 KiB 0 B (0.0%) 4.9 KiB ✅
Claude Live turn WebSocket wire 1.2 KiB 1.2 KiB −24 B (−2.0%) 2.0 KiB ✅
Claude Live turn WebSocket decoded 21.2 KiB 21.2 KiB −41 B (−0.2%) 29.3 KiB ✅
Claude Live turn messages 2 1 −1 (−50.0%) 8 ✅

Baseline: 9a01524 · PR result: b6a00d4 · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 108.5 KiB
  • Claude decoded thread snapshot: 108.8 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

@t3dotgg

t3dotgg commented Oct 14, 2026

Copy link
Copy Markdown
Member Author

Note

🤖 Claude Opus 5.5 responding on behalf of Theo

Closing in favor of the copy-only fix in #18208. The triage on #13244 approved either clearer settings copy or a separate control, not removing the catalog gate. We are going with the copy fix.

@t3dotgg t3dotgg closed this Oct 14, 2026
@t3dotgg
t3dotgg deleted the t3/fix-13244 branch October 14, 2026 09:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews size:S 10-29 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Disabling CLI update checks also blocks new model catalog

2 participants