provider update advisories#2312
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
size:XXL
justsomelegs
changed the title
ApprovabilityVerdict: Needs human review Diff is too large for automated approval analysis. A human reviewer should evaluate this PR. You can customize Macroscope's approvability policy. Learn more. |
|
just merged a large refactor on how drivers work and is registerred so this will need to be updated, i can review it when it is |
justsomelegs
force-pushed
the
feature/provider-update-advisories
branch
from
aeb88d0 to
9c34d30
Compare
|
@juliusmarminge this has been rebased now :) |
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
github-actions
Bot
added
vouch:trusted
|
where is the update button? 
|
|
and there should be some "dismiss until next update" or something so it doesn't reprompt every load: CleanShot.2026-05-02.at.00.05.13.mp4 |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| }); | ||
| return yield* upsertProviders(nextProviders, { | ||
| persist: false, | ||
| }); |
There was a problem hiding this comment.
Redundant double-application of update state in upsertProviders
Low Severity
setProviderUpdateState applies applyProviderUpdateState to the matching providers at line 398, then passes the result into upsertProviders which immediately applies applyProviderUpdateState again at line 323-329. Both reads resolve the same updateStatesRef value (already updated in step one), producing identical results. The first application in setProviderUpdateState is unnecessary since upsertProviders unconditionally re-applies it.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit c6ed36f. Configure here.
| const providerUpdateCandidateByInstanceId = useMemo( | ||
| () => new Map(providerUpdateCandidates.map((candidate) => [candidate.instanceId, candidate])), | ||
| [providerUpdateCandidates], | ||
| ); |
There was a problem hiding this comment.
Multi-instance providers only show update button on one card
Medium Severity
providerUpdateCandidateByInstanceId is built from providerUpdateCandidates, which is deduped by driver (one representative per driver). The map is keyed by the representative's instanceId. When rendering each row, providerUpdateCandidateByInstanceId.get(liveProvider.instanceId) will return undefined for any non-representative instance of the same driver. Users with multiple instances of the same provider (e.g. codex_personal and codex) will only see the Update button on one card, making the feature invisible for the other instances.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit c6ed36f. Configure here.
|
@juliusmarminge do you want me to fix these issue presented by Bugbot? don't mess with anything if your mid doing something or have already done it |
- Resolve provider lifecycles through Effect-based path handling - Support npm, bun, pnpm, Homebrew, and native update commands - Refresh update advisories and launch UI messaging
|
nevermind haha |
| export function isProviderUpdateNotificationDismissed( | ||
| dismissalKey: string | null | undefined, | ||
| ): boolean { | ||
| if (!dismissalKey) { | ||
| return false; | ||
| } | ||
| return readProviderUpdateDismissals().keys.includes(dismissalKey); | ||
| } |
There was a problem hiding this comment.
🟢 Low src/providerUpdateDismissal.ts:41
isProviderUpdateNotificationDismissed checks dismissalKey against stored keys without trimming, but dismissProviderUpdateNotification trims before storing. If a caller dismisses " key " and later checks " key ", the check returns false because the stored key is "key".
export function isProviderUpdateNotificationDismissed(
dismissalKey: string | null | undefined,
): boolean {
if (!dismissalKey) {
return false;
}
- return readProviderUpdateDismissals().keys.includes(dismissalKey);
+ return readProviderUpdateDismissals().keys.includes(dismissalKey.trim());
}🤖 Copy this AI Prompt to have your agent fix this:
In file apps/web/src/providerUpdateDismissal.ts around lines 41-48:
`isProviderUpdateNotificationDismissed` checks `dismissalKey` against stored keys without trimming, but `dismissProviderUpdateNotification` trims before storing. If a caller dismisses `" key "` and later checks `" key "`, the check returns `false` because the stored key is `"key"`.
Evidence trail:
apps/web/src/providerUpdateDismissal.ts lines 43-48 (isProviderUpdateNotificationDismissed - no trim), lines 50-59 (dismissProviderUpdateNotification - trims at line 51), lines 78-80 (useDismissedProviderUpdateNotificationKeys also trims). Test file at apps/web/src/providerUpdateDismissal.test.ts shows callers use clean strings like 'opencode:1.14.33'.
| models: input.models, | ||
| slashCommands: [...(input.slashCommands ?? [])], | ||
| skills: [...(input.skills ?? [])], | ||
| ...(versionAdvisory ? { versionAdvisory } : {}), |
There was a problem hiding this comment.
Dead code: driver parameter never passed by callers
Low Severity
The new optional driver parameter on buildServerProvider and its associated versionAdvisory computation are never used. All callers in ClaudeProvider.ts, CodexProvider.ts, CursorProvider.ts, and OpenCodeProvider.ts continue to call buildServerProvider without passing driver. The createProviderVersionAdvisory branch is unreachable dead code that adds confusion about where version advisories are sourced (they actually come exclusively from the enrichSnapshot callbacks).
Reviewed by Cursor Bugbot for commit 1589e67. Configure here.
- Support instance-scoped update lifecycle resolution and state - Add update metadata for Claude, Codex, Cursor, and OpenCode - Expand provider updater tests for targeted instance updates - Co-authored-by: codex <codex@users.noreply.github.com>
There was a problem hiding this comment.
🟢 Low
Line 143 in 544cddd
Each WebSocket connection instantiates its own providerUpdater with isolated runningTargetsRef and updateLocks semaphores. This allows two different clients to trigger simultaneous updates to the same provider, bypassing the intended cross-connection locking. For example, concurrent npm install -g commands from different clients will race instead of being serialized. Consider moving the providerUpdater (and its mutable state) outside makeWsRpcLayer so a single instance is shared across all connections.
🤖 Copy this AI Prompt to have your agent fix this:
In file apps/server/src/ws.ts around line 143:
Each WebSocket connection instantiates its own `providerUpdater` with isolated `runningTargetsRef` and `updateLocks` semaphores. This allows two different clients to trigger simultaneous updates to the same provider, bypassing the intended cross-connection locking. For example, concurrent `npm install -g` commands from different clients will race instead of being serialized. Consider moving the `providerUpdater` (and its mutable state) outside `makeWsRpcLayer` so a single instance is shared across all connections.
Evidence trail:
apps/server/src/ws.ts lines 143-156 (makeWsRpcLayer creates providerUpdater per call), apps/server/src/ws.ts lines 1140-1162 (makeWsRpcLayer called per WebSocket connection inside request handler), apps/server/src/provider/providerUpdater.ts lines 112-117 (runningTargetsRef and updateLocks created inside makeProviderUpdater), apps/server/src/provider/providerUpdater.ts lines 22-29 (SHARED_UPDATE_LOCK_KEYS showing intent to share locks), apps/server/src/ws.ts line 791 (providerUpdater.updateProvider used in RPC handler)
- Move native update path checks into drivers - Default providers to manual-only lifecycles - Update tests for the new lifecycle API Co-authored-by: codex <codex@users.noreply.github.com>
- Update provider lifecycle test fixtures to use generic package-tool names - Keep update-command resolution coverage aligned with the renamed providers
- Rename provider update lifecycle types and resolvers - Update provider snapshots and registry wiring - Add advisory maintenance coordinator and tests
- Replace driver-scoped update state with per-instance maintenance action state - Resolve update capabilities and progress through the target instance only - Update provider updater and tests for the new registry API
|
@justsomelegs mind trying it out now? made some changers |
|
@juliusmarminge LGTM, the only things i changed were the toast to track the providers exact provider |
|
yea made some architectural changes since i think this flow will eventually be extended to support installing drivers from and adding a proper onboarding etc |
- Add Effect-based maintenance command runner with process spawning - Allow dynamic update lock keys and preserve provider update advisories - Update tests to cover the new runner and command flow
- move maintenance runner behind an Effect service - add shared stream text collection for command output - broaden tests around command execution and update locking
- Switch provider maintenance/version advisory helpers to Effect.fn - Preserve npm/pnpm symlink detection while improving Effect test coverage
- Thread HttpClient through provider maintenance and snapshot refreshes - Switch provider update visibility to ISO timestamps - Add and update tests for maintenance and launch notification logic Co-authored-by: codex <codex@users.noreply.github.com>
- propagate maintenance capabilities into cursor snapshot enrichment - surface queued update state while another provider update runs - reset provider version cache between tests
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 4 total unresolved issues (including 3 from previous reviews).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 0950b79. Configure here.
| const client = yield* HttpClient.HttpClient; | ||
| const request = HttpClientRequest.get( | ||
| `https://registry.npmjs.org/${encodeURIComponent(packageName)}/latest`, | ||
| ).pipe(HttpClientRequest.setHeader("accept", "application/json")); |
There was a problem hiding this comment.
npm registry URL double-encodes @ in scoped packages
Low Severity
fetchNpmLatestVersion uses encodeURIComponent(packageName) for the npm registry URL, which encodes @ to %40 for scoped packages like @openai/codex. The npm registry convention is to keep @ literal and only encode the / (e.g., /@openai%2fcodex). While many servers decode %40 correctly, some CDN/proxy layers in front of the registry may not, potentially causing failed version lookups that silently return null.
Reviewed by Cursor Bugbot for commit 0950b79. Configure here.
2 participants
What Changed
Compared to
main, this PR adds provider update advisories and update-state UI for installed providers.Server-side:
Web-side:
UpdateactionUp to date.Also includes tests covering the update lifecycle, updater behavior, notification logic, and the rebased provider registry fixture change.
Why
maindoes not have provider update advisories or a dedicated way to surface provider update state in the UI.This PR adds that missing flow so users can:
The toast handles the action. The sidebar pill handles ongoing state and final status.
UI Changes
Compared to
main:Before:
After:
UpdateactionUp to date.after a successful updateupdate.demo.vid.mp4
Checklist
Note
Add provider update advisories and one-click update support for Claude, Codex, Cursor, and OpenCode
providerMaintenancemodule that resolves update capabilities per provider (npm, Homebrew, native), fetches latest versions from npm with TTL caching, and generatesversionAdvisoryobjects attached to provider snapshots.ProviderMaintenanceRunnerservice and aserver.updateProviderRPC endpoint that spawns update commands, captures bounded stdout/stderr with timeout, and returns structured results.ProviderInstanceCardto show an update popover when a version advisory exists, with a one-click 'Update now' button, loading state, and copy-to-clipboard for the update command.SidebarProviderUpdatePillin the sidebar footer and aProviderUpdateLaunchNotificationtoast at the root layout to surface update notifications with auto-dismiss and dismissal persistence via localStorage./settings/general#providersnow auto-scrolls to the providers section.Macroscope summarized 0950b79.
Note
Medium Risk
Adds new maintenance/update execution paths (spawning provider update commands, tracking volatile update state, and fetching latest versions from npm), which could affect provider lifecycle behavior and snapshot persistence if mis-handled.
Overview
Introduces a provider maintenance system that computes per-provider
maintenanceCapabilitiesand aversionAdvisory(including latest-version checks via npm registry) and wires it into provider snapshot enrichment for Claude, Codex, Cursor, and OpenCode.Adds a server-side update runner (
ProviderMaintenanceRunner) plus a new WS RPC (serverUpdateProvider) to execute one-click update commands with concurrency/lock coordination, timeout + output truncation, and volatileupdateStatetracking (explicitly excluded from the on-disk provider status cache).Adds web UI logic/components to surface update availability and progress via toasts and a sidebar pill, and persists dismissal keys in client settings (
dismissedProviderUpdateNotificationKeys), with comprehensive new tests around maintenance resolution, update execution, and notification behavior.Reviewed by Cursor Bugbot for commit 0950b79. Bugbot is set up for automated code reviews on this repo. Configure here.