You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix(server): stale provider update results clear once the provider is up to date - #24338
After a provider update ends as unchanged or failed, the result message stays under the provider row until the server restarts. This holds even after the install is fixed outside T3 Code and Refresh providers shows the provider is current. The row then shows 0.164.0 · Latest 0.164.0 together with "Update command completed, but T3 Code still detects an outdated provider version.", which contradicts it (https://git.cafe/pingdotgg/t3code/issues/24330).
ProviderRegistry keeps update results in maintenanceActionStatesRef, and applyProviderUpdateState attaches the stored result to every refreshed snapshot. Nothing removes the entry on refresh. It is only dropped when the instance disappears or when a new update replaces it.
Change
A new retireStaleUpdateState runs in syncProvider, on snapshots that come from a provider source (refresh and stream). When such a snapshot reports versionAdvisory.status === "current", an unchanged or failed update result for that instance is deleted from maintenanceActionStatesRef, so it does not come back on a later refresh. A later update writes a fresh entry as before.
It checks for current and not for "not behind_latest". A probe publishes unknown until the latest-version check finishes, so "not behind" would clear the result of a provider that is still outdated.
It does not run in setProviderMaintenanceActionState. That path re-applies the stored snapshot, whose advisory may already read current, and it would discard the result the update runner has just recorded.
queued, running and succeeded results are kept. A provider that is still behind_latest or unknown keeps its unchanged or failed result.
Known side effect: a failed result on a provider that was already current (for example a targeted version install) is also cleared at the next current snapshot.
Known gap: "Provider updated." (succeeded) still stays, as the reporter also mentions for Claude. It is true after the update and has no "no longer outdated" moment to key on, so I left it alone. It would need its own rule.
The change is limited to apps/server/src/provider/ProviderRegistry.ts and its test file.
Scope and approval
Fixes https://git.cafe/pingdotgg/t3code/issues/24330, a labeled bug report with steps to reproduce and the cause traced to this code path. It has no maintainer comment yet. The report suggests this direction (drop a terminal unchanged/failed state once the provider is no longer outdated) and leaves the choice to maintainers. This change does the first option and keeps the behavior of every provider that is still outdated.
Verification
This is server state with no UI change, so the proof is a test in the module's existing test file. The new update results across refreshes tests use the real ProviderRegistry layer over a fake provider source. They set an update result, switch the source snapshot to a given advisory status, call refresh, and check the snapshot's updateState:
retired: unchanged + current, failed + current
kept: unchanged or failed + behind_latest, unchanged + unknown, queued + current, running + current
a retired result does not come back after a later unknown re-probe, and a later running update still reports
Commands run in the worktree:
vp test run apps/server/src/provider/ProviderRegistry.test.ts -t "update results across refreshes" without the source change: 3 failed, 5 passed. The failures are unchanged + current, failed + current, and the "does not come back" test.
The same command with the change: 8 passed. After reverting only ProviderRegistry.ts again: the same 3 fail.
vp test run apps/server/src/provider/ProviderRegistry.test.ts apps/server/src/provider/providerMaintenanceRunner.test.ts: 63 passed (45 and 18).
vp lint and vp fmt --check on the two changed files: no findings.
vp run --filter t3 typecheck: exit 0.
vp run check:changed: exit 0 (format, lint, knip, typecheck, tests).
Not checked: I did not run the server or the app, so I did not see the settings row itself before and after. The behavior is covered by the tests above only.
… up to date
A finished update that reported "unchanged" or "failed" stayed attached to the provider until the server restarted, even after the install was fixed and a refresh showed the provider current. A refreshed snapshot that reports the provider current now retires those results; queued, running and succeeded results, and any provider still behind or not yet checked, are left alone.
Macroscope's review found this PR approvable — This is a narrowly scoped server bug fix that retires only stale unchanged/failed update results after a provider is confirmed current. Focused tests cover the affected refresh states and preserve active update reporting.
You can add or adjust custom eligibility rules. Learn more.
No actionable comments were generated in the recent review.
Walkthrough
ProviderRegistry now stores maintenance update state with an optional target version. During provider synchronization, it removes eligible state when the refreshed provider reports a current version advisory. Tests cover retirement and retention conditions.
Changes: Provider update state
Layer / File(s)
Summary
Store update state with target version apps/server/src/provider/ProviderRegistry.ts, apps/server/src/provider/providerMaintenanceRunner.ts
ProviderRegistry stores update state with an optional target version. The maintenance runner passes the requested target version when it updates that state.
Retire eligible results during refresh apps/server/src/provider/ProviderRegistry.ts, apps/server/src/provider/ProviderRegistry.test.ts
syncProvider removes stored update state under the specified current-advisory conditions. Tests cover retirement, retained states, and refresh behavior.
Stale provider update messages now clear once a provider is current, and the follow-up preserves failed installs of a requested version. No merge-blocking risk is evident.
Pre-merge checks: 4
Check name
Status
Explanation
Title check
The title names the main change: stale provider update results (unchanged or failed) are cleared once the provider reports it is up to date. It matches the changeset, which retires such update states …
Description check
The description follows the template sections (Problem, Change, Scope, Verification) and fills each one with specific content. It describes the bug, the change in ProviderRegistry.ts, the reason for t…
Linked Issues check
Issue #24330 asks that a stale terminal update result clear or hide once refresh shows the provider is no longer outdated. At head 59cf6ad, the syncProvider path in ProviderRegistry.ts deletes the…
Out of Scope Changes check
The head adds an optional targetVersion to setProviderMaintenanceActionState, changes the stored entry to a single update state plus target version, and passes targetVersion from `providerMainte…
Full details: Title check
Explanation
The title names the main change: stale provider update results (unchanged or failed) are cleared once the provider reports it is up to date. It matches the changeset, which retires such update states when the version advisory is current. It uses the conventional commit form with the server scope. Length is 80 characters, which is acceptable.
Full details: Description check
Explanation
The description follows the template sections (Problem, Change, Scope, Verification) and fills each one with specific content. It describes the bug, the change in ProviderRegistry.ts, the reason for the chosen advisory check, and the known side effects and gaps. It links the issue, explains why no maintainer request exists, and lists the commands run, with results. It also states what was not checked. The template's conventional-commit title guidance and agent attribution line are satisfied by the title and the 'Claude Sonnet 5.5 (xhigh) via T3 Code' line. No UI screenshots are needed because the change has no UI impact, and the author explains this. Minor gap: the Scope section cites a labeled bug report but the template asks for a link to the issue that shows the bug, which is provided.
Full details: Linked Issues check
Explanation
Issue #24330 asks that a stale terminal update result clear or hide once refresh shows the provider is no longer outdated. At head 59cf6ad, the syncProvider path in ProviderRegistry.ts deletes the stored entry when the incoming snapshot has versionAdvisory.status === "current", and the entry has no targetVersion and its update status is unchanged or failed. Other statuses (behind_latest, unknown, queued, running, succeeded) are kept, so the still-outdated case is unchanged. Because the stored entry is removed from the map, later refreshes do not re-attach it. The issue treats the succeeded ("Provider updated.") message as harmless, so keeping it is not an unmet requirement. The test file changes described in the raw summary cover the retire and retain cases. This assessment relies on the diff and the summary of tests; I did not run the tests.
Full details: Out of Scope Changes check
Explanation
The head adds an optional targetVersion to setProviderMaintenanceActionState, changes the stored entry to a single update state plus target version, and passes targetVersion from providerMaintenanceRunner.ts. These changes support the retirement rule: a recommended-version install that has a targetVersion is kept when the provider reads current. Without this, the retirement rule would clear failed targeted installs, which the PR description listed as a side effect. The change to the setter also removes the action-keyed object, which is a refactor needed for the new state shape. The production changes stay in ProviderRegistry.ts and providerMaintenanceRunner.ts, with tests in ProviderRegistry.test.ts. The PR description still describes the earlier side effect and file list, but the current code no longer has that side effect, so the description is stale rather than the code being out of scope.
Finishing Touches
Generate unit tests (beta)
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.
thanks @fixfon. your source-snapshot retirement rule and refresh regression tests address the stale result correctly. i kept your commit, merged current main, and pushed a focused follow-up to this branch (4e4abb7fe694238c4606f285291ae5313df84629).
preserve failed and unchanged recommended-version installs by carrying the requested version privately from the maintenance runner into the registry. latest-version status cannot prove that a recommended-version install succeeded.
inline the retirement rule and simplify the update-only state setter. no wire contract or client changes.
extend existing tests for recommended-version retention, succeeded/unknown retention, and runner target propagation. no new test files.
verification: current main failed three refresh regression cases; the original implementation failed both recommended-version cases; the runner failed three target propagation cases before the fix. all 69 focused registry/runner tests now pass. blacksmith vp run check:changed passed format, lint, knip, graphql, affected typechecks and tests. the worker uses a shallow source snapshot, so its local-only history was connected to the fetched base for change detection. both independent gpt-6.1-sol reviews found no issues on this head.
linux chrome exercised the real settings → provider update → server maintenance runner → disposable claude wrapper path. the wrapper delegates capabilities/auth to the installed cli, fails only the installer, and controls its reported version. after failure, a current refresh followed by another outdated probe restores the old error on main; this head keeps it cleared. screenshots use matching data and viewport. the short recording is slowed for readability and makes no timing claim.
unverified: real provider upgrades, recommended-version installs in a live client (covered by registry and runner tests), other provider binaries, electron/mobile, windows/macos and remote/relay/tunnel sessions. the shared server rule applies to all callers and clients. the displayed claude authentication warning is expected on this vm.
all 28 reported checks are green on this head; cafe reports mergeable. coderabbit reports no actionable findings. the existing macroscope approval is on the contributor's earlier head, so no new macroscope approval is claimed. the two independent reviews above cover the final head. left open and unmerged for maria.
verified as-is at 4e4abb7fe694238c4606f285291ae5313df84629. no code changes or pushes.
ran github/main (975b3dc568) in a separate worktree first, then the exact pr head, with vp i and one background dev server at a time. drove the actual web client using the repo Playwright dependency and /usr/bin/google-chrome. created a project for a real git repository and threads through the app. real Codex gpt-6.1-sol turns completed on main and the pr, including a fresh thread after an app-driven provider upgrade. no mocks, wrappers, fixtures, or seeded data.
used an actual npm installation of Codex 0.164.0 in a disposable prefix. making its installation directory read-only caused the app update command to fail with real npm EACCES. repaired permissions and installed 0.165.0 externally, then clicked refresh. main retained the failed result despite reporting current. the pr removed it. changing back to 0.164.0 and refreshing restored the stale error on main, while the pr kept it cleared. the screenshots show that matching final state at the same viewport; main has newer settings navigation.
blacksmith passed all 69 registry/maintenance-runner tests and vp run check:changed 7d3208cbea86b91574366bb6e87d05f93c975127 (format, lint, knip, graphql, affected typechecks, tests). restored shallow worker history and passed the comparison base explicitly. all 30 reported pr checks are successful; gitcafe still reports a pending current-base conflict observation, so this is no claim of merge readiness.
inspected the captures and verified uploaded video playback and seeking in t3. the recording trims the external-install wait and makes no timing claim. stopped both task dev servers and the blacksmith worker.
Not verified
native iOS/Android, Electron, macOS/Windows, remote/relay/tunnel connections, other provider binaries, and the unchanged installer outcome through the live app. recommended-version failure retention was covered by the existing registry and runner tests, but not a live targeted install. the displayed Claude authentication warning is unrelated; Claude is not authenticated on this VM.
gitcafe renders these external media embeds as links. reopened the comment, checked each image against its uploaded bytes, and opened the video directly to verify playback and seeking. inline media rendering in gitcafe is unavailable.
Resolve conflicts with main's refreshCompatibility split in ProviderRegistry
and take main's providerMaintenanceRunner test. Provide the registry mutator
and crypto layers the new update-state tests now need, and drop the
redundant retired-result test.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
Real web app, Chromium, Linux, 1600×1000, light theme; separate fresh server state on each commit. A disposable provider executable supplied an updater that succeeded without changing version. No model API was called.
Update the outdated 0.162.0 installation and observe the unchanged-version warning.
Externally repair the same installation to latest native-channel version 0.167.2.
Without restarting the server, refresh providers, wait for completion, then refresh again.
Confirm the current version and close Settings to inspect the retained update notice.
Both commits display the current version. Base retains “Codex still needs an update”; head clears it after the first repair refresh and it stays absent after the second. On these commits the stale result is visible in the sidebar; the current-version row's update-details control disappears on both.
Private environment labels, installation paths and an unrelated model entry are redacted. Initial recording masks also cover transient updater output containing the installation path. Final versions and warnings remain visible.
AI-assisted; verified with the tests and tools listed above.
Not shown:
The failed-exit updater variant was not captured; this proof exercises the default updater succeeding without changing version.
Released provider installers, pinned recommended-version updates, and desktop/mobile clients were not exercised; the proof covers controlled installation behavior through the real web app and server.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bugfixFix PR opened by an issue lead; enters the PR looppr:gate-failedA land gate failedsize:M30-99 changed lines (additions + deletions).vouch:unvouchedAuthor is not yet trusted in the VOUCHED list.
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
github-actions
Bot
added
vouch:unvouched
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
juliusmarminge
added
bugfix
Problem
After a provider update ends as
unchangedorfailed, the result message stays under the provider row until the server restarts. This holds even after the install is fixed outside T3 Code and Refresh providers shows the provider is current. The row then shows0.164.0 · Latest 0.164.0together with "Update command completed, but T3 Code still detects an outdated provider version.", which contradicts it (https://git.cafe/pingdotgg/t3code/issues/24330).ProviderRegistrykeeps update results inmaintenanceActionStatesRef, andapplyProviderUpdateStateattaches the stored result to every refreshed snapshot. Nothing removes the entry on refresh. It is only dropped when the instance disappears or when a new update replaces it.Change
A new
retireStaleUpdateStateruns insyncProvider, on snapshots that come from a provider source (refresh and stream). When such a snapshot reportsversionAdvisory.status === "current", anunchangedorfailedupdate result for that instance is deleted frommaintenanceActionStatesRef, so it does not come back on a later refresh. A later update writes a fresh entry as before.currentand not for "notbehind_latest". A probe publishesunknownuntil the latest-version check finishes, so "not behind" would clear the result of a provider that is still outdated.setProviderMaintenanceActionState. That path re-applies the stored snapshot, whose advisory may already readcurrent, and it would discard the result the update runner has just recorded.queued,runningandsucceededresults are kept. A provider that is stillbehind_latestorunknownkeeps itsunchangedorfailedresult.Known side effect: a
failedresult on a provider that was alreadycurrent(for example a targeted version install) is also cleared at the nextcurrentsnapshot.Known gap: "Provider updated." (
succeeded) still stays, as the reporter also mentions for Claude. It is true after the update and has no "no longer outdated" moment to key on, so I left it alone. It would need its own rule.The change is limited to
apps/server/src/provider/ProviderRegistry.tsand its test file.Scope and approval
Fixes https://git.cafe/pingdotgg/t3code/issues/24330, a labeled
bugreport with steps to reproduce and the cause traced to this code path. It has no maintainer comment yet. The report suggests this direction (drop a terminalunchanged/failedstate once the provider is no longer outdated) and leaves the choice to maintainers. This change does the first option and keeps the behavior of every provider that is still outdated.Verification
This is server state with no UI change, so the proof is a test in the module's existing test file. The new
update results across refreshestests use the realProviderRegistrylayer over a fake provider source. They set an update result, switch the source snapshot to a given advisory status, callrefresh, and check the snapshot'supdateState:unchanged+current,failed+currentunchangedorfailed+behind_latest,unchanged+unknown,queued+current,running+currentunknownre-probe, and a laterrunningupdate still reportsCommands run in the worktree:
vp test run apps/server/src/provider/ProviderRegistry.test.ts -t "update results across refreshes"without the source change: 3 failed, 5 passed. The failures areunchanged+current,failed+current, and the "does not come back" test.ProviderRegistry.tsagain: the same 3 fail.vp test run apps/server/src/provider/ProviderRegistry.test.ts apps/server/src/provider/providerMaintenanceRunner.test.ts: 63 passed (45 and 18).vp lintandvp fmt --checkon the two changed files: no findings.vp run --filter t3 typecheck: exit 0.vp run check:changed: exit 0 (format, lint, knip, typecheck, tests).Not checked: I did not run the server or the app, so I did not see the settings row itself before and after. The behavior is covered by the tests above only.
Claude Sonnet 5.5 (xhigh) via T3 Code.
Fixes #24330