diff --git a/src/vs/workbench/contrib/positronPackages/browser/interfaces/positronPackagesService.ts b/src/vs/workbench/contrib/positronPackages/browser/interfaces/positronPackagesService.ts index b1839283a0d2..100c3b934fa3 100644 --- a/src/vs/workbench/contrib/positronPackages/browser/interfaces/positronPackagesService.ts +++ b/src/vs/workbench/contrib/positronPackages/browser/interfaces/positronPackagesService.ts @@ -64,14 +64,11 @@ export interface IPositronPackagesService { /** * Gets the installed packages for the active session. * @param token Optional cancellation token + * @param forceMetadata When `true`, recompute outdated metadata live for + * every package instead of reusing still-fresh cached state. Set for + * user-initiated refreshes so an explicit Refresh is always authoritative. */ - refreshPackages(token?: CancellationToken): Promise; - - /** - * Force refresh package metadata, clearing the cache. - * @param token Optional cancellation token - */ - refreshMetadata(token?: CancellationToken): Promise; + refreshPackages(token?: CancellationToken, forceMetadata?: boolean): Promise; /** * Install packages in the active session. diff --git a/src/vs/workbench/contrib/positronPackages/browser/positronPackages.contribution.ts b/src/vs/workbench/contrib/positronPackages/browser/positronPackages.contribution.ts index 63901b7c0d0d..c89400aab8f1 100644 --- a/src/vs/workbench/contrib/positronPackages/browser/positronPackages.contribution.ts +++ b/src/vs/workbench/contrib/positronPackages/browser/positronPackages.contribution.ts @@ -163,7 +163,6 @@ export const PACKAGES_UPDATE_COMMAND_ID = 'positronPackages.updatePackage'; export const PACKAGES_UPDATE_ALL_COMMAND_ID = 'positronPackages.updateAllPackages'; export const PACKAGES_UNINSTALL_COMMAND_ID = 'positronPackages.uninstallPackage'; export const PACKAGES_REFRESH_COMMAND_ID = 'positronPackages.refreshPackages'; -export const PACKAGES_REFRESH_METADATA_COMMAND_ID = 'positronPackages.refreshMetadata'; const PACKAGES_CATEGORY = nls.localize2('packages', 'Packages'); @@ -268,7 +267,9 @@ class RefreshPackagesAction extends Action2 { delay: 500 }, async () => { try { - return await service.refreshPackages(cts.token); + // User-initiated refresh: force a live outdated recompute so + // the pane can't keep showing stale cached indicators. + return await service.refreshPackages(cts.token, true /* forceMetadata */); } catch (error) { notifications.error(cleanErrorMessage(error)); throw error; @@ -626,49 +627,6 @@ class UninstallSelectedPackageAction extends Action2 { } } -class RefreshMetadataAction extends Action2 { - constructor() { - super({ - id: PACKAGES_REFRESH_METADATA_COMMAND_ID, - title: nls.localize2('refreshMetadata', 'Refresh Metadata'), - category: PACKAGES_CATEGORY, - f1: true, - precondition: ContextKeyExpr.and(POSITRON_PACKAGES_ENABLED, PACKAGES_CAN_RUN_ACTION), - menu: { - id: MenuId.ViewTitle, - when: PACKAGES_VIEW_VISIBLE, - group: 'packages_metadata', - order: 1 - } - }); - } - override async run(accessor: ServicesAccessor): Promise { - const service = accessor.get(IPositronPackagesService); - const notifications = accessor.get(INotificationService); - const progress = accessor.get(IProgressService); - - const cts = new CancellationTokenSource(); - - try { - await progress.withProgress({ - title: nls.localize('positronPackages.refreshingMetadata', 'Refreshing Package Metadata...'), - location: ProgressLocation.Notification, - cancellable: true, - delay: 500 - }, async () => { - try { - await service.refreshMetadata(cts.token); - } catch (error) { - notifications.error(cleanErrorMessage(error)); - throw error; - } - }, () => cts.cancel()); - } finally { - cts.dispose(true); - } - } -} - /** * Switches the Packages view to the expanded card layout. * Only visible in the view title when the view is currently showing compact rows. @@ -772,7 +730,6 @@ CommandsRegistry.registerCommand(PACKAGES_OPEN_COMMAND_ID, registerAction2(InstallPackageAction); registerAction2(RefreshPackagesAction); -registerAction2(RefreshMetadataAction); registerAction2(UninstallPackageAction); registerAction2(UpdatePackageAction); registerAction2(UpdateAllPackagesAction); diff --git a/src/vs/workbench/contrib/positronPackages/browser/positronPackagesInstance.ts b/src/vs/workbench/contrib/positronPackages/browser/positronPackagesInstance.ts index 8c605fbf3fc7..ef01f864abc0 100644 --- a/src/vs/workbench/contrib/positronPackages/browser/positronPackagesInstance.ts +++ b/src/vs/workbench/contrib/positronPackages/browser/positronPackagesInstance.ts @@ -18,8 +18,7 @@ export interface IPositronPackagesInstance { session: ILanguageRuntimeSession; attachRuntime(): void; detachRuntime(): void; - refreshPackages(token?: CancellationToken): Promise; - refreshMetadata(token?: CancellationToken): Promise; + refreshPackages(token?: CancellationToken, forceMetadata?: boolean): Promise; installPackages(packages: IPackageSpec[], token?: CancellationToken): Promise; uninstallPackages(packageNames: string[], token?: CancellationToken): Promise; updatePackages(packages: IPackageSpec[], token?: CancellationToken): Promise; @@ -182,39 +181,20 @@ export class PositronPackagesInstance extends Disposable implements IPositronPac return packageManager; } - async refreshPackages(token?: CancellationToken): Promise { + async refreshPackages(token?: CancellationToken, forceMetadata: boolean = false): Promise { const packageManager = this.getPackageManagerOrThrow(); const effectiveToken = token ?? CancellationToken.None; // Loading this._onDidChangeRefreshState.fire(true); try { - await this._refreshPackagesInternal(packageManager, effectiveToken); + await this._refreshPackagesInternal(packageManager, effectiveToken, forceMetadata); return this.packages; } finally { this._onDidChangeRefreshState.fire(false); } } - /** - * Force refresh metadata for all packages, clearing the cache first. - */ - async refreshMetadata(token?: CancellationToken): Promise { - const packageManager = this.getPackageManagerOrThrow(); - const effectiveToken = token ?? CancellationToken.None; - - if (!packageManager.getPackageMetadata || this._packages.length === 0) { - return; - } - - // Cancel any in-flight fetch before clearing the cache so a stale - // fetch from refreshPackages can't repopulate it after the clear. - this._metadataFetch?.cancel(); - this._metadataCache.clear(); - - await this._fetchAndMergeMetadata(packageManager, effectiveToken, true /* fetchAll */); - } - /** * Internal helper to refresh packages with two-stage metadata fetch. * Stage 1: Get basic packages and fire event immediately (with cached metadata). @@ -223,20 +203,23 @@ export class PositronPackagesInstance extends Disposable implements IPositronPac private async _refreshPackagesInternal( packageManager: ReturnType, token: CancellationToken, + forceMetadata: boolean = false, ): Promise { // Stage 1: Get basic package list and fire event (getter merges cached metadata) this._packages = await packageManager.getPackages(token); this._onDidRefreshPackagesInstance.fire(this.packages); - // Stage 2: Fetch metadata asynchronously (don't block). When the - // persisted entry has aged past its freshness window, refetch every - // package so a new upstream release surfaces even though nothing - // installed locally changed; otherwise only the packages without a - // fresh cache hit are fetched (and a fully-fresh warm start makes no - // network call at all). Use CancellationToken.None since this runs - // after the main operation completes. + // Stage 2: Fetch metadata asynchronously (don't block). Refetch every + // package when `forceMetadata` is set (a user-initiated refresh, which + // must be authoritative even inside the freshness window) or when the + // persisted entry has aged past its freshness window (so a new upstream + // release surfaces even though nothing installed locally changed); + // otherwise only the packages without a fresh cache hit are fetched + // (and a fully-fresh warm start makes no network call at all). Use + // CancellationToken.None since this runs after the main operation + // completes. if (packageManager.getPackageMetadata && this._packages.length > 0) { - const fetchAll = !this._cache.isFresh(this._runtimeId); + const fetchAll = forceMetadata || !this._cache.isFresh(this._runtimeId); this._fetchAndMergeMetadata(packageManager, CancellationToken.None, fetchAll); } } diff --git a/src/vs/workbench/contrib/positronPackages/browser/positronPackagesService.ts b/src/vs/workbench/contrib/positronPackages/browser/positronPackagesService.ts index 3331d78cd833..bfaefda047e4 100644 --- a/src/vs/workbench/contrib/positronPackages/browser/positronPackagesService.ts +++ b/src/vs/workbench/contrib/positronPackages/browser/positronPackagesService.ts @@ -218,11 +218,11 @@ export class PositronPackagesService extends Disposable implements IPositronPack this._onDidChangeItemSize.fire(itemSize); } - async refreshPackages(token?: CancellationToken): Promise { + async refreshPackages(token?: CancellationToken, forceMetadata?: boolean): Promise { const instance = this._activeInstance; if (instance) { return await Promise.race([ - instance.refreshPackages(token), + instance.refreshPackages(token, forceMetadata), timeout(TIMEOUT_REFRESH_MS).then(() => { throw new Error('Package refresh timed out'); }) ]); } @@ -230,15 +230,6 @@ export class PositronPackagesService extends Disposable implements IPositronPack throw new Error('No active session found.'); } - async refreshMetadata(token?: CancellationToken): Promise { - const instance = this._activeInstance; - if (instance) { - return await instance.refreshMetadata(token); - } - - throw new Error('No active session found.'); - } - async installPackages(packages: IPackageSpec[], token?: CancellationToken): Promise { const instance = this._activeInstance; if (instance) { diff --git a/src/vs/workbench/contrib/positronPackages/test/browser/positronPackagesInstance.vitest.ts b/src/vs/workbench/contrib/positronPackages/test/browser/positronPackagesInstance.vitest.ts index 99f7d3caf107..b72a9c2cd363 100644 --- a/src/vs/workbench/contrib/positronPackages/test/browser/positronPackagesInstance.vitest.ts +++ b/src/vs/workbench/contrib/positronPackages/test/browser/positronPackagesInstance.vitest.ts @@ -117,6 +117,31 @@ describe('PositronPackagesInstance disk-cache integration', () => { expect(getPackageMetadata).not.toHaveBeenCalled(); }); + it('forces a live refetch on a fresh, fully-covered entry when forceMetadata is set', async () => { + // Mirror of the "makes no network call" test above: same fresh, fully- + // covered entry, but forceMetadata flips it from re-rendering cache to a + // live refetch. The cache flags numpy as outdated; the repository has + // since caught up, so the live fetch reports it current. + seed({ + numpy: { version: '1.26.0', outdated: true, latestVersion: '2.0.0' }, + pandas: { version: '2.0.0', outdated: false }, + }, 1 * HOUR_MS); + getPackageMetadata.mockResolvedValue(new Map>([ + ['numpy', { outdated: false }], + ['pandas', { outdated: false }], + ])); + + const instance = makeInstance(); + const fires = waitForEvents(instance.onDidRefreshPackagesInstance, 2); + await instance.refreshPackages(CancellationToken.None, true /* forceMetadata */); + const [, stage2] = await fires; + + // The forced Stage 2 refetches every package (not just uncached ones, as + // a non-forced refresh of a fresh entry would) and clears the stale flag. + expect(getPackageMetadata).toHaveBeenCalledWith(['numpy', 'pandas'], expect.anything()); + expect(stage2.find(p => p.name === 'numpy')?.outdated).toBe(false); + }); + it('renders a stale entry then refetches every package', async () => { seed({ numpy: { version: '1.26.0', outdated: true, latestVersion: '2.0.0' } }, 25 * HOUR_MS);