From dc1f35ff7b2e560b99e42eef457122c6694492ac Mon Sep 17 00:00:00 2001 From: Dhruvi Sompura Date: Fri, 17 Jul 2026 16:50:37 -0700 Subject: [PATCH 01/14] Make positronPackages.updatePackage agent-invocable --- .../browser/agentPackageArgs.ts | 13 +++++++++++++ .../browser/positronPackages.contribution.ts | 13 +++++++++++-- .../test/browser/agentPackageArgs.vitest.ts | 18 ++++++++++++++++++ 3 files changed, 42 insertions(+), 2 deletions(-) create mode 100644 src/vs/workbench/contrib/positronPackages/browser/agentPackageArgs.ts create mode 100644 src/vs/workbench/contrib/positronPackages/test/browser/agentPackageArgs.vitest.ts diff --git a/src/vs/workbench/contrib/positronPackages/browser/agentPackageArgs.ts b/src/vs/workbench/contrib/positronPackages/browser/agentPackageArgs.ts new file mode 100644 index 00000000000..ef1c268b418 --- /dev/null +++ b/src/vs/workbench/contrib/positronPackages/browser/agentPackageArgs.ts @@ -0,0 +1,13 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (C) 2026 Posit Software, PBC. All rights reserved. + * Licensed under the Elastic License 2.0. See LICENSE.txt for license information. + *--------------------------------------------------------------------------------------------*/ + +/** + * Normalize the version an agent passes to `positronPackages.updatePackage`. + * The agent uses `'latest'` for "the newest available version"; the packages + * service treats an undefined version the same way, so map it here. + */ +export function normalizeAgentTargetVersion(version: string | undefined): string | undefined { + return version === 'latest' ? undefined : version; +} diff --git a/src/vs/workbench/contrib/positronPackages/browser/positronPackages.contribution.ts b/src/vs/workbench/contrib/positronPackages/browser/positronPackages.contribution.ts index 7c4e584ac2a..6753bea3437 100644 --- a/src/vs/workbench/contrib/positronPackages/browser/positronPackages.contribution.ts +++ b/src/vs/workbench/contrib/positronPackages/browser/positronPackages.contribution.ts @@ -32,6 +32,7 @@ import { IPositronPackagesService } from './interfaces/positronPackagesService.j import { PACKAGE_METADATA_CACHE_ENABLED_SETTING, PACKAGE_METADATA_CACHE_MAX_AGE_HOURS_DEFAULT, PACKAGE_METADATA_CACHE_MAX_AGE_HOURS_SETTING } from './packageMetadataCache.js'; import { PACKAGES_CAN_RUN_ACTION, PACKAGES_HAS_SELECTION, PACKAGES_VIEW_VISIBLE, POSITRON_PACKAGES_ITEM_SIZE, POSITRON_PACKAGES_VIEW_ID } from './positronPackagesContextKeys.js'; import { installPackage, uninstallPackage, updatePackage } from './positronPackagesQuickPick.js'; +import { normalizeAgentTargetVersion } from './agentPackageArgs.js'; import { PositronPackagesService } from './positronPackagesService.js'; import { PositronPackagesView } from './positronPackagesView.js'; @@ -453,6 +454,14 @@ class UpdatePackageAction extends Action2 { category: PACKAGES_CATEGORY, f1: true, precondition: ContextKeyExpr.and(POSITRON_PACKAGES_ENABLED, PACKAGES_CAN_RUN_ACTION), + metadata: { + description: nls.localize('positron.updatePackage.description', "Update an installed package in the active runtime session."), + agentCompatible: true, + args: [ + { name: 'name', description: "Name of the package to update.", schema: { type: 'string' } }, + { name: 'version', isOptional: true, description: "Target version, or 'latest' for the newest available version. When omitted, a version picker opens.", schema: { type: 'string' } }, + ], + }, }); } override async run(accessor: ServicesAccessor, ...args: unknown[]): Promise { @@ -478,7 +487,7 @@ class UpdatePackageAction extends Action2 { return service.searchPackageVersions(pkg, cts.token); }; - const performUpdate = async (pkg: string, version: string): Promise => { + const performUpdate = async (pkg: string, version?: string): Promise => { await progress.withProgress({ title: nls.localize('positronPackages.updatingPackages', 'Updating Packages...'), location: ProgressLocation.Notification, @@ -508,7 +517,7 @@ class UpdatePackageAction extends Action2 { const argPackage = typeof args.at(0) === 'string' ? args.at(0) as string : undefined; const argVersion = typeof args.at(1) === 'string' ? args.at(1) as string : undefined; if (argPackage && argVersion) { - await performUpdate(argPackage, argVersion); + await performUpdate(argPackage, normalizeAgentTargetVersion(argVersion)); return; } await updatePackage(accessor, performSearch, performSearchVersions, performUpdate, argPackage, cts); diff --git a/src/vs/workbench/contrib/positronPackages/test/browser/agentPackageArgs.vitest.ts b/src/vs/workbench/contrib/positronPackages/test/browser/agentPackageArgs.vitest.ts new file mode 100644 index 00000000000..6449c5e6f28 --- /dev/null +++ b/src/vs/workbench/contrib/positronPackages/test/browser/agentPackageArgs.vitest.ts @@ -0,0 +1,18 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (C) 2026 Posit Software, PBC. All rights reserved. + * Licensed under the Elastic License 2.0. See LICENSE.txt for license information. + *--------------------------------------------------------------------------------------------*/ + +/// + +import { normalizeAgentTargetVersion } from '../../browser/agentPackageArgs.js'; + +describe('normalizeAgentTargetVersion', () => { + it('maps the agent version contract onto the packages-service version contract', () => { + expect({ + latest: normalizeAgentTargetVersion('latest'), + explicit: normalizeAgentTargetVersion('1.2.3'), + missing: normalizeAgentTargetVersion(undefined), + }).toEqual({ latest: undefined, explicit: '1.2.3', missing: undefined }); + }); +}); From b4269e180de381298d85106bc7d5f1d790f546ba Mon Sep 17 00:00:00 2001 From: Dhruvi Sompura Date: Fri, 17 Jul 2026 17:44:38 -0700 Subject: [PATCH 02/14] Make positronPackages.installPackage agent-invocable --- .../browser/positronPackages.contribution.ts | 29 +++++++++++-------- 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/src/vs/workbench/contrib/positronPackages/browser/positronPackages.contribution.ts b/src/vs/workbench/contrib/positronPackages/browser/positronPackages.contribution.ts index 6753bea3437..70f1daedef6 100644 --- a/src/vs/workbench/contrib/positronPackages/browser/positronPackages.contribution.ts +++ b/src/vs/workbench/contrib/positronPackages/browser/positronPackages.contribution.ts @@ -304,7 +304,15 @@ class InstallPackageAction extends Action2 { when: PACKAGES_VIEW_VISIBLE, group: 'packages', order: 1 - } + }, + metadata: { + description: nls.localize('positron.installPackage.description', "Install a package in the active runtime session."), + agentCompatible: true, + args: [ + { name: 'name', description: "Name of the package to install.", schema: { type: 'string' } }, + { name: 'version', isOptional: true, description: "Target version, or 'latest' for the newest available version. When omitted, the newest version is installed.", schema: { type: 'string' } }, + ], + }, }); } override async run(accessor: ServicesAccessor, ...args: unknown[]): Promise { @@ -327,10 +335,6 @@ class InstallPackageAction extends Action2 { }; const performInstall = async (pkg: string, version?: string): Promise => { - if (!version) { - throw new Error('No version specified.'); - } - await progress.withProgress({ title: nls.localize('positronPackages.installingPackages', 'Installing Packages...'), location: ProgressLocation.Notification, @@ -353,15 +357,16 @@ class InstallPackageAction extends Action2 { }, () => cts.cancel()); }; - // When a package name and version are both provided (e.g. the detail - // editor's Install button), install that version directly. Only a real - // string is treated as the package name -- menu invocations (e.g. the - // view-title overflow "Install Package") pass a context object as arg0, - // which must fall through to the search quick-pick. + // When a package name is provided (e.g. the detail editor's Install + // button, or an agent), install it directly, installing the latest + // version when none is given. Only a real string is treated as the + // package name -- menu invocations (e.g. the view-title overflow + // "Install Package") pass a context object as arg0, which must fall + // through to the search quick-pick. const argPackage = typeof args.at(0) === 'string' ? args.at(0) as string : undefined; const argVersion = typeof args.at(1) === 'string' ? args.at(1) as string : undefined; - if (argPackage && argVersion) { - await performInstall(argPackage, argVersion); + if (argPackage) { + await performInstall(argPackage, normalizeAgentTargetVersion(argVersion)); return; } From c51f71a9d8945f8de49a1f9cae65da80e621db68 Mon Sep 17 00:00:00 2001 From: Dhruvi Sompura Date: Fri, 17 Jul 2026 17:51:59 -0700 Subject: [PATCH 03/14] Fix stale install comment in packageDetail after installPackage change --- .../positronPackages/browser/components/packageDetail.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/vs/workbench/contrib/positronPackages/browser/components/packageDetail.tsx b/src/vs/workbench/contrib/positronPackages/browser/components/packageDetail.tsx index 7b9200fc1c8..eb1a7cb26d0 100644 --- a/src/vs/workbench/contrib/positronPackages/browser/components/packageDetail.tsx +++ b/src/vs/workbench/contrib/positronPackages/browser/components/packageDetail.tsx @@ -170,8 +170,8 @@ export const PackageDetail = (props: PackageDetailProps) => { case 'install': // Reinstall the previously-installed version directly. `pkg` resolves // to the last-known package after an uninstall, so `pkg.version` is the - // version that was installed. If we have no last-known package, pass no - // version and let the command fall through to the version quick-pick. + // version that was installed; with no last-known package, the latest is + // installed. services.commandService.executeCommand('positronPackages.installPackage', packageName, pkg?.version); break; case 'help': From d3034645be306847d9d0e33129698348f68d810c Mon Sep 17 00:00:00 2001 From: Dhruvi Sompura Date: Fri, 24 Jul 2026 18:08:58 -0700 Subject: [PATCH 04/14] Use single quotes for non-externalized arg descriptions --- .../browser/positronPackages.contribution.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/vs/workbench/contrib/positronPackages/browser/positronPackages.contribution.ts b/src/vs/workbench/contrib/positronPackages/browser/positronPackages.contribution.ts index 70f1daedef6..1457e58c7a3 100644 --- a/src/vs/workbench/contrib/positronPackages/browser/positronPackages.contribution.ts +++ b/src/vs/workbench/contrib/positronPackages/browser/positronPackages.contribution.ts @@ -309,8 +309,8 @@ class InstallPackageAction extends Action2 { description: nls.localize('positron.installPackage.description', "Install a package in the active runtime session."), agentCompatible: true, args: [ - { name: 'name', description: "Name of the package to install.", schema: { type: 'string' } }, - { name: 'version', isOptional: true, description: "Target version, or 'latest' for the newest available version. When omitted, the newest version is installed.", schema: { type: 'string' } }, + { name: 'name', description: 'Name of the package to install.', schema: { type: 'string' } }, + { name: 'version', isOptional: true, description: 'Target version, or \'latest\' for the newest available version. When omitted, the newest version is installed.', schema: { type: 'string' } }, ], }, }); @@ -463,8 +463,8 @@ class UpdatePackageAction extends Action2 { description: nls.localize('positron.updatePackage.description', "Update an installed package in the active runtime session."), agentCompatible: true, args: [ - { name: 'name', description: "Name of the package to update.", schema: { type: 'string' } }, - { name: 'version', isOptional: true, description: "Target version, or 'latest' for the newest available version. When omitted, a version picker opens.", schema: { type: 'string' } }, + { name: 'name', description: 'Name of the package to update.', schema: { type: 'string' } }, + { name: 'version', isOptional: true, description: 'Target version, or \'latest\' for the newest available version. When omitted, a version picker opens.', schema: { type: 'string' } }, ], }, }); From ff044921db793863276c41b44f66c8b14d5d6101 Mon Sep 17 00:00:00 2001 From: Dhruvi Sompura Date: Fri, 24 Jul 2026 18:37:37 -0700 Subject: [PATCH 05/14] Match sibling localize id namespace for package command descriptions --- .../positronPackages/browser/positronPackages.contribution.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/vs/workbench/contrib/positronPackages/browser/positronPackages.contribution.ts b/src/vs/workbench/contrib/positronPackages/browser/positronPackages.contribution.ts index 1457e58c7a3..05aaf51d902 100644 --- a/src/vs/workbench/contrib/positronPackages/browser/positronPackages.contribution.ts +++ b/src/vs/workbench/contrib/positronPackages/browser/positronPackages.contribution.ts @@ -306,7 +306,7 @@ class InstallPackageAction extends Action2 { order: 1 }, metadata: { - description: nls.localize('positron.installPackage.description', "Install a package in the active runtime session."), + description: nls.localize('positronPackages.installPackage.description', "Install a package in the active runtime session."), agentCompatible: true, args: [ { name: 'name', description: 'Name of the package to install.', schema: { type: 'string' } }, @@ -460,7 +460,7 @@ class UpdatePackageAction extends Action2 { f1: true, precondition: ContextKeyExpr.and(POSITRON_PACKAGES_ENABLED, PACKAGES_CAN_RUN_ACTION), metadata: { - description: nls.localize('positron.updatePackage.description', "Update an installed package in the active runtime session."), + description: nls.localize('positronPackages.updatePackage.description', "Update an installed package in the active runtime session."), agentCompatible: true, args: [ { name: 'name', description: 'Name of the package to update.', schema: { type: 'string' } }, From 05a24cd00e32536effdba7d7d0a55ebe9df0616a Mon Sep 17 00:00:00 2001 From: Dhruvi Sompura Date: Tue, 28 Jul 2026 09:53:08 -0700 Subject: [PATCH 06/14] Extract package version ordering into its own module sortVersionsDescending moves verbatim out of positronPackagesQuickPick into a new packageVersions module, so a headless caller can reuse it without depending on the quick-pick's IQuickInputService and Delayer. Adds newestAvailableVersion, which picks the version to use when a caller asks for the newest one. It prefers the newest stable release over a newer prerelease, matching what the package managers install by default: with 1.8, 1.9 and 2.0.0rc1 available, pip install installs 1.9, not the release candidate. Backends disagree on ordering (R returns one version, PyPI ascending, conda newest-first), so the list is sorted here rather than trusted. Neither function was tested before; both are now. Co-Authored-By: Claude Opus 5 (1M context) --- .../browser/packageVersions.ts | 47 +++++++++++ .../browser/positronPackagesQuickPick.ts | 20 +---- .../test/browser/packageVersions.vitest.ts | 78 +++++++++++++++++++ 3 files changed, 126 insertions(+), 19 deletions(-) create mode 100644 src/vs/workbench/contrib/positronPackages/browser/packageVersions.ts create mode 100644 src/vs/workbench/contrib/positronPackages/test/browser/packageVersions.vitest.ts diff --git a/src/vs/workbench/contrib/positronPackages/browser/packageVersions.ts b/src/vs/workbench/contrib/positronPackages/browser/packageVersions.ts new file mode 100644 index 00000000000..8920fdce423 --- /dev/null +++ b/src/vs/workbench/contrib/positronPackages/browser/packageVersions.ts @@ -0,0 +1,47 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (C) 2026 Posit Software, PBC. All rights reserved. + * Licensed under the Elastic License 2.0. See LICENSE.txt for license information. + *--------------------------------------------------------------------------------------------*/ + +import * as semver from '../../../../base/common/semver/semver.js'; + +/** + * Sort version strings in descending order (newest first). + * Uses semver comparison when possible, falls back to string comparison. + */ +export function sortVersionsDescending(versions: string[]): string[] { + return [...versions].sort((a, b) => { + const aSemver = semver.valid(a, true) ? a : semver.coerce(a); + const bSemver = semver.valid(b, true) ? b : semver.coerce(b); + + if (aSemver && bSemver) { + return semver.rcompare(aSemver, bSemver, true); + } + + // Fall back to simple string comparison + return a < b ? 1 : a > b ? -1 : 0; + }); +} + +/** + * Pick the version to use when the caller asked for the newest available one. + * + * Prefers the newest stable release over a newer prerelease, matching what the + * package managers install by default: with versions 1.8, 1.9 and 2.0.0rc1, + * `pip install ` installs 1.9, not the release candidate. + * + * Backends disagree on ordering -- R returns a single version, PyPI returns + * ascending order and does not filter prereleases, conda returns newest-first -- + * so the list is sorted here rather than trusted. + * + * Falls back to the newest of everything when a package has published only + * prereleases, so a caller always gets a version if one exists. + */ +export function newestAvailableVersion(versions: string[]): string | undefined { + const sorted = sortVersionsDescending(versions); + const stable = sorted.find(version => { + const parsed = semver.valid(version, true) || semver.coerce(version); + return parsed ? !semver.prerelease(parsed, true) : true; + }); + return stable ?? sorted.at(0); +} diff --git a/src/vs/workbench/contrib/positronPackages/browser/positronPackagesQuickPick.ts b/src/vs/workbench/contrib/positronPackages/browser/positronPackagesQuickPick.ts index 72d133a4bcf..9a0441df368 100644 --- a/src/vs/workbench/contrib/positronPackages/browser/positronPackagesQuickPick.ts +++ b/src/vs/workbench/contrib/positronPackages/browser/positronPackagesQuickPick.ts @@ -10,7 +10,7 @@ import { DisposableStore, IDisposable } from '../../../../base/common/lifecycle. import { localize } from '../../../../nls.js'; import { ServicesAccessor } from '../../../../platform/instantiation/common/instantiation.js'; import { IInputBox, IQuickInput, IQuickInputButton, IQuickInputService, IQuickPickItem, QuickPickItem } from '../../../../platform/quickinput/common/quickInput.js'; -import * as semver from '../../../../base/common/semver/semver.js'; +import { sortVersionsDescending } from './packageVersions.js'; /** * Debounce interval (ms) before firing a package search as the user types. Long @@ -56,24 +56,6 @@ interface PackageSearchResult { detail?: string; } -/** - * Sort version strings in descending order (newest first). - * Uses semver comparison when possible, falls back to string comparison. - */ -function sortVersionsDescending(versions: string[]): string[] { - return [...versions].sort((a, b) => { - const aSemver = semver.valid(a, true) ? a : semver.coerce(a); - const bSemver = semver.valid(b, true) ? b : semver.coerce(b); - - if (aSemver && bSemver) { - return semver.rcompare(aSemver, bSemver, true); - } - - // Fall back to simple string comparison - return a < b ? 1 : a > b ? -1 : 0; - }); -} - export const updatePackage = async ( accessor: ServicesAccessor, performGetPackages: (q: string) => Promise, diff --git a/src/vs/workbench/contrib/positronPackages/test/browser/packageVersions.vitest.ts b/src/vs/workbench/contrib/positronPackages/test/browser/packageVersions.vitest.ts new file mode 100644 index 00000000000..42b97ab8e4b --- /dev/null +++ b/src/vs/workbench/contrib/positronPackages/test/browser/packageVersions.vitest.ts @@ -0,0 +1,78 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (C) 2026 Posit Software, PBC. All rights reserved. + * Licensed under the Elastic License 2.0. See LICENSE.txt for license information. + *--------------------------------------------------------------------------------------------*/ + +/// + +import { newestAvailableVersion, sortVersionsDescending } from '../../browser/packageVersions.js'; + +describe('sortVersionsDescending', () => { + it('sorts newest first, using semver rather than string ordering', () => { + expect(sortVersionsDescending(['1.1.2', '1.1.10', '1.1.4'])).toEqual(['1.1.10', '1.1.4', '1.1.2']); + }); + + it('sorts two-component versions numerically', () => { + expect(sortVersionsDescending(['2.0', '2.1', '2.10'])).toEqual(['2.10', '2.1', '2.0']); + }); + + it('ranks a prerelease below the matching stable release', () => { + expect(sortVersionsDescending(['1.26.4', '2.0.0rc1', '2.0.0'])).toEqual(['2.0.0', '2.0.0rc1', '1.26.4']); + }); + + it('handles the single-version and empty lists that R returns', () => { + expect({ single: sortVersionsDescending(['1.1.4']), empty: sortVersionsDescending([]) }) + .toEqual({ single: ['1.1.4'], empty: [] }); + }); + + it('falls back to string comparison when neither version parses', () => { + expect(sortVersionsDescending(['alpha', 'beta'])).toEqual(['beta', 'alpha']); + }); + + it('does not mutate its input', () => { + const versions = ['1.0.0', '2.0.0']; + sortVersionsDescending(versions); + expect(versions).toEqual(['1.0.0', '2.0.0']); + }); + + // Versions that differ only by a suffix semver.coerce drops compare equal, so + // the backend's own ordering decides. Pinned as current behavior, not as the + // ideal: R patch levels are moot because R returns a single version, and PyPI + // post-releases are rare. + it('cannot separate versions that differ only by a coerce-dropped suffix', () => { + expect({ + rPatchLevel: sortVersionsDescending(['1.0-3', '1.0-10']), + pypiPostRelease: sortVersionsDescending(['1.0', '1.0.post1']), + }).toEqual({ + rPatchLevel: ['1.0-3', '1.0-10'], + pypiPostRelease: ['1.0', '1.0.post1'], + }); + }); +}); + +describe('newestAvailableVersion', () => { + it('prefers the newest stable release over a newer prerelease', () => { + // What `pip install ` would resolve to for this version list. + expect(newestAvailableVersion(['1.8', '1.9', '2.0.0rc1'])).toBe('1.9'); + }); + + it('picks the newest version when all of them are stable', () => { + expect(newestAvailableVersion(['1.1.2', '1.1.10', '1.1.4'])).toBe('1.1.10'); + }); + + it('takes PyPI ascending order into account', () => { + expect(newestAvailableVersion(['1.26.4', '2.0.0', '2.1.3'])).toBe('2.1.3'); + }); + + it('falls back to the newest prerelease when a package has only prereleases', () => { + expect(newestAvailableVersion(['1.0.0rc1', '1.0.0rc2'])).toBe('1.0.0rc2'); + }); + + it('returns the only version R reports', () => { + expect(newestAvailableVersion(['1.1.4'])).toBe('1.1.4'); + }); + + it('returns undefined when no versions are available', () => { + expect(newestAvailableVersion([])).toBeUndefined(); + }); +}); From 4c080d062b81d6a591730b9f0a68bc210486f2f8 Mon Sep 17 00:00:00 2001 From: Dhruvi Sompura Date: Tue, 28 Jul 2026 09:53:32 -0700 Subject: [PATCH 07/14] Resolve 'latest' and return structured results from install and update The agent contract these commands advertised did not hold. 'latest' was mapped to an undefined version, but undefined is not universally "newest": pip and uv-outside-a-project reject a missing version on update outright, and on install they write a bare package name into a requirements file with no --upgrade, so an already-installed package resolves as satisfied and never moves. pip is the default manager for Pyenv, Global, System and VirtualEnv environments, so an agent following the documented contract failed on the common case. 'latest' is now resolved to a concrete version against the session's repositories before the service is called, so every backend receives an explicit target. This also fixes install's already-satisfied no-op. Both commands now return a structured result instead of void. Every failure path notified the user and returned void, which validateAndExecute reports as success, so the assistant was told a failed install succeeded. The two bugs compounded: an agent updating a package to latest in a pip session failed inside the service, the error went to a toast it could not see, and it reported success. The returns metadata documents the shape. The agent path never throws; the interactive path keeps its existing rethrow, which only it can now reach. That distinction matters because the Command Palette turns a thrown command error into a modal dialog. Neither dispatch guard changes, so no existing caller is affected. Install's guard is reverted to requiring both a name and a version, restoring the version-picker fallback from #14608. 'latest' is the agent's signal for "newest", and no human caller passes it, so argument shape alone distinguishes the two callers. Correspondingly, version is no longer marked optional in the agent metadata: omitting it installs nothing and opens a picker that waits for a human. isOptional is descriptive only, so this does not affect menu or UI callers. Co-Authored-By: Claude Opus 5 (1M context) --- .../browser/agentPackageArgs.ts | 13 -- .../browser/components/packageDetail.tsx | 4 +- .../browser/positronPackages.contribution.ts | 220 +++++++++++++++--- .../test/browser/agentPackageArgs.vitest.ts | 18 -- 4 files changed, 192 insertions(+), 63 deletions(-) delete mode 100644 src/vs/workbench/contrib/positronPackages/browser/agentPackageArgs.ts delete mode 100644 src/vs/workbench/contrib/positronPackages/test/browser/agentPackageArgs.vitest.ts diff --git a/src/vs/workbench/contrib/positronPackages/browser/agentPackageArgs.ts b/src/vs/workbench/contrib/positronPackages/browser/agentPackageArgs.ts deleted file mode 100644 index ef1c268b418..00000000000 --- a/src/vs/workbench/contrib/positronPackages/browser/agentPackageArgs.ts +++ /dev/null @@ -1,13 +0,0 @@ -/*--------------------------------------------------------------------------------------------- - * Copyright (C) 2026 Posit Software, PBC. All rights reserved. - * Licensed under the Elastic License 2.0. See LICENSE.txt for license information. - *--------------------------------------------------------------------------------------------*/ - -/** - * Normalize the version an agent passes to `positronPackages.updatePackage`. - * The agent uses `'latest'` for "the newest available version"; the packages - * service treats an undefined version the same way, so map it here. - */ -export function normalizeAgentTargetVersion(version: string | undefined): string | undefined { - return version === 'latest' ? undefined : version; -} diff --git a/src/vs/workbench/contrib/positronPackages/browser/components/packageDetail.tsx b/src/vs/workbench/contrib/positronPackages/browser/components/packageDetail.tsx index eb1a7cb26d0..7b9200fc1c8 100644 --- a/src/vs/workbench/contrib/positronPackages/browser/components/packageDetail.tsx +++ b/src/vs/workbench/contrib/positronPackages/browser/components/packageDetail.tsx @@ -170,8 +170,8 @@ export const PackageDetail = (props: PackageDetailProps) => { case 'install': // Reinstall the previously-installed version directly. `pkg` resolves // to the last-known package after an uninstall, so `pkg.version` is the - // version that was installed; with no last-known package, the latest is - // installed. + // version that was installed. If we have no last-known package, pass no + // version and let the command fall through to the version quick-pick. services.commandService.executeCommand('positronPackages.installPackage', packageName, pkg?.version); break; case 'help': diff --git a/src/vs/workbench/contrib/positronPackages/browser/positronPackages.contribution.ts b/src/vs/workbench/contrib/positronPackages/browser/positronPackages.contribution.ts index 05aaf51d902..b1c46b69d61 100644 --- a/src/vs/workbench/contrib/positronPackages/browser/positronPackages.contribution.ts +++ b/src/vs/workbench/contrib/positronPackages/browser/positronPackages.contribution.ts @@ -4,6 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import { CancellationToken, CancellationTokenSource } from '../../../../base/common/cancellation.js'; +import { isCancellationError } from '../../../../base/common/errors.js'; import { removeAnsiEscapeCodes } from '../../../../base/common/strings.js'; import { Codicon } from '../../../../base/common/codicons.js'; import { KeyCode, KeyMod } from '../../../../base/common/keyCodes.js'; @@ -32,7 +33,7 @@ import { IPositronPackagesService } from './interfaces/positronPackagesService.j import { PACKAGE_METADATA_CACHE_ENABLED_SETTING, PACKAGE_METADATA_CACHE_MAX_AGE_HOURS_DEFAULT, PACKAGE_METADATA_CACHE_MAX_AGE_HOURS_SETTING } from './packageMetadataCache.js'; import { PACKAGES_CAN_RUN_ACTION, PACKAGES_HAS_SELECTION, PACKAGES_VIEW_VISIBLE, POSITRON_PACKAGES_ITEM_SIZE, POSITRON_PACKAGES_VIEW_ID } from './positronPackagesContextKeys.js'; import { installPackage, uninstallPackage, updatePackage } from './positronPackagesQuickPick.js'; -import { normalizeAgentTargetVersion } from './agentPackageArgs.js'; +import { newestAvailableVersion } from './packageVersions.js'; import { PositronPackagesService } from './positronPackagesService.js'; import { PositronPackagesView } from './positronPackagesView.js'; @@ -179,6 +180,76 @@ function cleanErrorMessage(error: unknown): string { return removeAnsiEscapeCodes(message); } +/** + * The version an agent passes to ask for the newest available version. Human + * callers never pass it: the quick-pick and the package detail editor either + * supply a concrete version or no version at all. + */ +const LATEST_VERSION = 'latest'; + +/** + * Outcome of resolving a target version, before the install or update runs. + */ +type VersionResolution = + | { readonly ok: true; readonly version: string } + | { readonly ok: false; readonly message: string }; + +/** + * Resolves the version for a direct (non-quick-pick) install or update. A + * concrete version passes through untouched; `'latest'` is resolved against the + * repositories configured for the session, so the backend always receives an + * explicit target. + * + * Resolving is required, not merely tidier. pip and uv-outside-a-project reject + * a missing version on update outright, and on install they write a bare package + * name into a requirements file with no `--upgrade`, so an already-installed + * package resolves as satisfied and never moves to the newest version. + * + * Failures are notified here so both commands report them identically. + */ +async function resolveTargetVersion( + service: IPositronPackagesService, + notifications: INotificationService, + name: string, + version: string, + token: CancellationToken +): Promise { + if (version.toLowerCase() !== LATEST_VERSION) { + return { ok: true, version }; + } + + const canceled: VersionResolution = { + ok: false, + message: nls.localize('positronPackages.versionLookupCanceled', "Finding the latest version of '{0}' was canceled.", name) + }; + + let versions: string[]; + try { + versions = await service.searchPackageVersions(name, token); + } catch (e) { + if (isCancellationError(e) || token.isCancellationRequested) { + return canceled; + } + const message = cleanErrorMessage(e); + notifications.error(message); + return { ok: false, message }; + } + + // A canceled lookup resolves to an empty list rather than throwing, so check + // the token before reporting the package as missing from the repositories. + if (token.isCancellationRequested) { + return canceled; + } + + const latest = newestAvailableVersion(versions); + if (!latest) { + const message = nls.localize('positronPackages.noVersionsAvailable', "No available versions of '{0}' were found. Check the package name and the repositories configured for this session.", name); + notifications.error(message); + return { ok: false, message }; + } + return { ok: true, version: latest }; +} + /** * Shows a notification suggesting the user restart their session after a package operation. * @@ -291,7 +362,22 @@ class RefreshPackagesAction extends Action2 { } -class InstallPackageAction extends Action2 { +/** + * Result of the installPackage command, returned to programmatic callers + * (notably agents via `positron.ai.validateAndExecuteCommand`). + */ +interface IInstallPackageResult { + installed: boolean; + name?: string; + version?: string; + message?: string; +} + +/** + * Installs a package into the active session. Exported so its argument handling + * and result can be unit-tested. + */ +export class InstallPackageAction extends Action2 { constructor() { super({ id: PACKAGES_INSTALL_COMMAND_ID, @@ -306,16 +392,17 @@ class InstallPackageAction extends Action2 { order: 1 }, metadata: { - description: nls.localize('positronPackages.installPackage.description', "Install a package in the active runtime session."), + description: nls.localize('positronPackages.installPackage.description', "Install a package into the active runtime session's environment (R library, Python environment). Requires a running interpreter session. A session restart may be required before the newly installed package can be loaded."), agentCompatible: true, args: [ - { name: 'name', description: 'Name of the package to install.', schema: { type: 'string' } }, - { name: 'version', isOptional: true, description: 'Target version, or \'latest\' for the newest available version. When omitted, the newest version is installed.', schema: { type: 'string' } }, + { name: 'name', description: 'Name of the package to install, as the package repository knows it (for example: dplyr, pandas).', schema: { type: 'string' } }, + { name: 'version', description: 'Version to install: either an exact version (for example: 1.1.4) or \'latest\' for the newest version available to the session. Base R installs ignore this and always install the current release from the repository.', schema: { type: 'string' } }, ], + returns: 'An object with installed, name, version, and message. installed is true when the package was installed, with name and version confirming what was requested (version is the concrete version resolved when \'latest\' was passed); when installed is false, message explains why (no versions available for the package, a failed version lookup, a failed install, or a canceled install).', }, }); } - override async run(accessor: ServicesAccessor, ...args: unknown[]): Promise { + override async run(accessor: ServicesAccessor, ...args: unknown[]): Promise { const service = accessor.get(IPositronPackagesService); const notifications = accessor.get(INotificationService); const progress = accessor.get(IProgressService); @@ -334,6 +421,10 @@ class InstallPackageAction extends Action2 { return await service.searchPackageVersions(pkg, cts.token); }; + // Captured rather than returned, so the quick-pick helper's callback + // signature stays unchanged. + let outcome: IInstallPackageResult | undefined; + const performInstall = async (pkg: string, version?: string): Promise => { await progress.withProgress({ title: nls.localize('positronPackages.installingPackages', 'Installing Packages...'), @@ -351,26 +442,48 @@ class InstallPackageAction extends Action2 { nls.localize('positronPackages.operationInstalled', 'installed'), [pkg] ); + outcome = { installed: true, name: pkg, version }; } catch (e) { - notifications.error(cleanErrorMessage(e)); + if (isCancellationError(e) || cts.token.isCancellationRequested) { + // The user canceled the progress notification; an error + // toast would be wrong. + outcome = { + installed: false, + name: pkg, + version, + message: nls.localize('positronPackages.installCanceled', "The install of '{0}' was canceled.", pkg) + }; + return; + } + const message = cleanErrorMessage(e); + notifications.error(message); + outcome = { installed: false, name: pkg, version, message }; } }, () => cts.cancel()); }; - // When a package name is provided (e.g. the detail editor's Install - // button, or an agent), install it directly, installing the latest - // version when none is given. Only a real string is treated as the - // package name -- menu invocations (e.g. the view-title overflow - // "Install Package") pass a context object as arg0, which must fall - // through to the search quick-pick. - const argPackage = typeof args.at(0) === 'string' ? args.at(0) as string : undefined; - const argVersion = typeof args.at(1) === 'string' ? args.at(1) as string : undefined; - if (argPackage) { - await performInstall(argPackage, normalizeAgentTargetVersion(argVersion)); - return; + // When a package name and version are both provided (e.g. the detail + // editor's Install button, or an agent), install that version + // directly. Only a real string is treated as the package name -- + // menu invocations (e.g. the view-title overflow "Install Package") + // pass a context object as arg0, which must fall through to the + // search quick-pick, as must a name with no version. + const argPackage = typeof args.at(0) === 'string' ? (args.at(0) as string).trim() || undefined : undefined; + const argVersion = typeof args.at(1) === 'string' ? (args.at(1) as string).trim() || undefined : undefined; + if (argPackage && argVersion) { + const resolved = await resolveTargetVersion(service, notifications, argPackage, argVersion, cts.token); + if (!resolved.ok) { + return { installed: false, name: argPackage, message: resolved.message }; + } + await performInstall(argPackage, resolved.version); + return outcome ?? { installed: false, name: argPackage, version: resolved.version }; } await installPackage(accessor, performSearch, performSearchVersions, performInstall, cts); + return outcome ?? { + installed: false, + message: nls.localize('positronPackages.noPackageSelected', "No package was selected.") + }; } catch (error) { notifications.error(cleanErrorMessage(error)); throw error; @@ -451,7 +564,22 @@ class UninstallPackageAction extends Action2 { } } -class UpdatePackageAction extends Action2 { +/** + * Result of the updatePackage command, returned to programmatic callers + * (notably agents via `positron.ai.validateAndExecuteCommand`). + */ +interface IUpdatePackageResult { + updated: boolean; + name?: string; + version?: string; + message?: string; +} + +/** + * Updates an installed package in the active session. Exported so its argument + * handling and result can be unit-tested. + */ +export class UpdatePackageAction extends Action2 { constructor() { super({ id: PACKAGES_UPDATE_COMMAND_ID, @@ -460,16 +588,17 @@ class UpdatePackageAction extends Action2 { f1: true, precondition: ContextKeyExpr.and(POSITRON_PACKAGES_ENABLED, PACKAGES_CAN_RUN_ACTION), metadata: { - description: nls.localize('positronPackages.updatePackage.description', "Update an installed package in the active runtime session."), + description: nls.localize('positronPackages.updatePackage.description', "Update a package that is already installed in the active runtime session to a different version. Requires a running interpreter session. A session restart may be required before the new version is loaded."), agentCompatible: true, args: [ - { name: 'name', description: 'Name of the package to update.', schema: { type: 'string' } }, - { name: 'version', isOptional: true, description: 'Target version, or \'latest\' for the newest available version. When omitted, a version picker opens.', schema: { type: 'string' } }, + { name: 'name', description: 'Name of the installed package to update (for example: dplyr, pandas).', schema: { type: 'string' } }, + { name: 'version', description: 'Version to update to: either an exact version (for example: 1.1.4) or \'latest\' for the newest version available to the session. Base R updates ignore this and always install the current release from the repository.', schema: { type: 'string' } }, ], + returns: 'An object with updated, name, version, and message. updated is true when the package was updated, with name and version confirming what was requested (version is the concrete version resolved when \'latest\' was passed); when updated is false, message explains why (no versions available for the package, a failed version lookup, a failed update, or a canceled update).', }, }); } - override async run(accessor: ServicesAccessor, ...args: unknown[]): Promise { + override async run(accessor: ServicesAccessor, ...args: unknown[]): Promise { const service = accessor.get(IPositronPackagesService); const notifications = accessor.get(INotificationService); const progress = accessor.get(IProgressService); @@ -492,6 +621,10 @@ class UpdatePackageAction extends Action2 { return service.searchPackageVersions(pkg, cts.token); }; + // Captured rather than returned, so the quick-pick helper's callback + // signature stays unchanged. + let outcome: IUpdatePackageResult | undefined; + const performUpdate = async (pkg: string, version?: string): Promise => { await progress.withProgress({ title: nls.localize('positronPackages.updatingPackages', 'Updating Packages...'), @@ -509,23 +642,50 @@ class UpdatePackageAction extends Action2 { nls.localize('positronPackages.operationUpdated', 'updated'), [pkg] ); + outcome = { updated: true, name: pkg, version }; } catch (e) { - notifications.error(cleanErrorMessage(e)); + if (isCancellationError(e) || cts.token.isCancellationRequested) { + // The user canceled the progress notification; an error + // toast would be wrong. + outcome = { + updated: false, + name: pkg, + version, + message: nls.localize('positronPackages.updateCanceled', "The update of '{0}' was canceled.", pkg) + }; + return; + } + const message = cleanErrorMessage(e); + notifications.error(message); + outcome = { updated: false, name: pkg, version, message }; } }, () => cts.cancel()); }; // Only treat a real string as the package name (menu invocations pass a // context object as arg0). When both a package name and a target version - // are given (e.g. the detail editor's Update button), update directly - // without prompting; otherwise fall through to the quick-pick flow. - const argPackage = typeof args.at(0) === 'string' ? args.at(0) as string : undefined; - const argVersion = typeof args.at(1) === 'string' ? args.at(1) as string : undefined; + // are given (e.g. the detail editor's Update button, or an agent), + // update directly without prompting; otherwise fall through to the + // quick-pick flow, which is how the packages list's Update button picks + // a version. + const argPackage = typeof args.at(0) === 'string' ? (args.at(0) as string).trim() || undefined : undefined; + const argVersion = typeof args.at(1) === 'string' ? (args.at(1) as string).trim() || undefined : undefined; if (argPackage && argVersion) { - await performUpdate(argPackage, normalizeAgentTargetVersion(argVersion)); - return; + const resolved = await resolveTargetVersion(service, notifications, argPackage, argVersion, cts.token); + if (!resolved.ok) { + return { updated: false, name: argPackage, message: resolved.message }; + } + await performUpdate(argPackage, resolved.version); + return outcome ?? { updated: false, name: argPackage, version: resolved.version }; } await updatePackage(accessor, performSearch, performSearchVersions, performUpdate, argPackage, cts); + return outcome ?? { + updated: false, + name: argPackage, + message: argPackage + ? nls.localize('positronPackages.noVersionSelected', "No version was selected.") + : nls.localize('positronPackages.noPackageSelectedForUpdate', "No package was selected.") + }; } catch (error) { notifications.error(cleanErrorMessage(error)); throw error; diff --git a/src/vs/workbench/contrib/positronPackages/test/browser/agentPackageArgs.vitest.ts b/src/vs/workbench/contrib/positronPackages/test/browser/agentPackageArgs.vitest.ts deleted file mode 100644 index 6449c5e6f28..00000000000 --- a/src/vs/workbench/contrib/positronPackages/test/browser/agentPackageArgs.vitest.ts +++ /dev/null @@ -1,18 +0,0 @@ -/*--------------------------------------------------------------------------------------------- - * Copyright (C) 2026 Posit Software, PBC. All rights reserved. - * Licensed under the Elastic License 2.0. See LICENSE.txt for license information. - *--------------------------------------------------------------------------------------------*/ - -/// - -import { normalizeAgentTargetVersion } from '../../browser/agentPackageArgs.js'; - -describe('normalizeAgentTargetVersion', () => { - it('maps the agent version contract onto the packages-service version contract', () => { - expect({ - latest: normalizeAgentTargetVersion('latest'), - explicit: normalizeAgentTargetVersion('1.2.3'), - missing: normalizeAgentTargetVersion(undefined), - }).toEqual({ latest: undefined, explicit: '1.2.3', missing: undefined }); - }); -}); From 2afc479799c9aadee54c656459f327219f5ffc13 Mon Sep 17 00:00:00 2001 From: Dhruvi Sompura Date: Tue, 28 Jul 2026 09:53:43 -0700 Subject: [PATCH 08/14] Test install and update argument handling and results Covers what had no coverage: the dispatch decision and how an outcome becomes a result. The two action classes are exported for this, matching LookupHelpTopic; the quick-pick module is mocked because its real MultiStepInput needs a live QuickInputService and a 300ms debounce, and this change does not touch it. The regression tests that matter are a failed install reporting installed: false rather than success, 'latest' always reaching the service as a concrete version, and a name-only invocation still opening the picker. Co-Authored-By: Claude Opus 5 (1M context) --- .../browser/installAndUpdatePackage.vitest.ts | 357 ++++++++++++++++++ 1 file changed, 357 insertions(+) create mode 100644 src/vs/workbench/contrib/positronPackages/test/browser/installAndUpdatePackage.vitest.ts diff --git a/src/vs/workbench/contrib/positronPackages/test/browser/installAndUpdatePackage.vitest.ts b/src/vs/workbench/contrib/positronPackages/test/browser/installAndUpdatePackage.vitest.ts new file mode 100644 index 00000000000..9a0d0db769c --- /dev/null +++ b/src/vs/workbench/contrib/positronPackages/test/browser/installAndUpdatePackage.vitest.ts @@ -0,0 +1,357 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (C) 2026 Posit Software, PBC. All rights reserved. + * Licensed under the Elastic License 2.0. See LICENSE.txt for license information. + *--------------------------------------------------------------------------------------------*/ + +/// + +import { CancellationError } from '../../../../../base/common/errors.js'; +import { ICommandService } from '../../../../../platform/commands/common/commands.js'; +import { INotificationService } from '../../../../../platform/notification/common/notification.js'; +import { IProgress, IProgressService, IProgressStep, Progress } from '../../../../../platform/progress/common/progress.js'; +import { createTestContainer } from '../../../../../test/vitest/positronTestContainer.js'; +import { stubInterface } from '../../../../../test/vitest/stubInterface.js'; +import { ILanguageRuntimeSession, IRuntimeSessionService } from '../../../../services/runtimeSession/common/runtimeSessionService.js'; +import { IPositronPackagesService } from '../../browser/interfaces/positronPackagesService.js'; + +// The real quick-pick flow needs a live QuickInputService plus a 300ms debounce, +// and this change does not touch it. Mocking it keeps these tests on what did +// change: the dispatch decision, and how an outcome becomes a result. +const { installPackageMock, updatePackageMock } = vi.hoisted(() => ({ + installPackageMock: vi.fn(), + updatePackageMock: vi.fn(), +})); +vi.mock('../../browser/positronPackagesQuickPick.js', () => ({ + installPackage: installPackageMock, + updatePackage: updatePackageMock, + uninstallPackage: vi.fn(), +})); + +const { InstallPackageAction, UpdatePackageAction } = await import('../../browser/positronPackages.contribution.js'); + +describe('packages install and update commands', () => { + const ctx = createTestContainer() + .withWorkbenchServices() + .build(); + + let installPackages: ReturnType>; + let updatePackages: ReturnType>; + let searchPackageVersions: ReturnType>; + let error: ReturnType>; + let prompt: ReturnType>; + + /** + * Wires the services `run()` reads. The session is passed explicitly so that + * "no active session" cannot be confused with "not specified". + */ + function stubServices(activeSession: ILanguageRuntimeSession | undefined): void { + ctx.instantiationService.stub(IPositronPackagesService, stubInterface({ + activeSession, + installPackages, + updatePackages, + searchPackageVersions, + searchPackages: vi.fn().mockResolvedValue([]), + refreshPackages: vi.fn().mockResolvedValue([]), + })); + ctx.instantiationService.stub(INotificationService, stubInterface({ error, prompt })); + ctx.instantiationService.stub(IRuntimeSessionService, stubInterface({ + restartSession: vi.fn(), + })); + ctx.instantiationService.stub(ICommandService, stubInterface({ + executeCommand: vi.fn(), + })); + stubProgress(); + } + + /** + * Runs the task immediately. `cancel` fires the cancel handler first, which is + * what the user clicking Cancel on the progress notification does. + */ + function stubProgress(cancel = false): void { + ctx.instantiationService.stub(IProgressService, stubInterface({ + withProgress: ( + _options: unknown, + task: (progress: IProgress) => Promise, + onDidCancel?: (choice?: number) => void + ): Promise => { + if (cancel) { + onDidCancel?.(); + } + return task(Progress.None); + }, + })); + } + + beforeEach(() => { + installPackages = vi.fn().mockResolvedValue(undefined); + updatePackages = vi.fn().mockResolvedValue(undefined); + searchPackageVersions = vi.fn().mockResolvedValue(['1.8', '1.9', '2.0.0rc1']); + error = vi.fn(); + prompt = vi.fn(); + installPackageMock.mockReset().mockResolvedValue(undefined); + updatePackageMock.mockReset().mockResolvedValue(undefined); + stubServices(stubInterface({ sessionId: 'session-1' })); + }); + + async function runInstall(...args: unknown[]) { + const action = new InstallPackageAction(); + return ctx.instantiationService.invokeFunction(accessor => action.run(accessor, ...args)); + } + + async function runUpdate(...args: unknown[]) { + const action = new UpdatePackageAction(); + return ctx.instantiationService.invokeFunction(accessor => action.run(accessor, ...args)); + } + + describe('InstallPackageAction', () => { + it('installs an exact version without opening a picker or looking up versions', async () => { + const result = await runInstall('dplyr', '1.1.4'); + + expect(result).toEqual({ installed: true, name: 'dplyr', version: '1.1.4' }); + expect(installPackages).toHaveBeenCalledWith([{ name: 'dplyr', version: '1.1.4' }], expect.anything()); + expect(searchPackageVersions).not.toHaveBeenCalled(); + expect(installPackageMock).not.toHaveBeenCalled(); + }); + + it('resolves \'latest\' to the newest stable version', async () => { + const result = await runInstall('dplyr', 'latest'); + + expect(result).toEqual({ installed: true, name: 'dplyr', version: '1.9' }); + expect(installPackages).toHaveBeenCalledWith([{ name: 'dplyr', version: '1.9' }], expect.anything()); + }); + + it('accepts \'latest\' in any case, and trims both arguments', async () => { + const result = await runInstall(' dplyr ', ' Latest '); + + expect(result).toEqual({ installed: true, name: 'dplyr', version: '1.9' }); + expect(installPackages).toHaveBeenCalledWith([{ name: 'dplyr', version: '1.9' }], expect.anything()); + }); + + it('reports a failed install instead of reporting success', async () => { + installPackages.mockRejectedValue(new Error('no such package')); + + const result = await runInstall('nope', '1.0.0'); + + expect(result).toEqual({ installed: false, name: 'nope', version: '1.0.0', message: 'no such package' }); + expect(error).toHaveBeenCalledWith('no such package'); + expect(prompt).not.toHaveBeenCalled(); + }); + + it('reports a missing session as a failure', async () => { + installPackages.mockRejectedValue(new Error('No active session found.')); + + const result = await runInstall('dplyr', '1.1.4'); + + expect(result).toEqual({ installed: false, name: 'dplyr', version: '1.1.4', message: 'No active session found.' }); + }); + + it('reports when no versions are available for \'latest\'', async () => { + searchPackageVersions.mockResolvedValue([]); + + const result = await runInstall('nope', 'latest'); + + expect(result).toEqual({ + installed: false, + name: 'nope', + message: `No available versions of 'nope' were found. Check the package name and the repositories configured for this session.`, + }); + expect(installPackages).not.toHaveBeenCalled(); + expect(error).toHaveBeenCalled(); + }); + + it('reports a failed version lookup for \'latest\'', async () => { + searchPackageVersions.mockRejectedValue(new Error('network down')); + + const result = await runInstall('dplyr', 'latest'); + + expect(result).toEqual({ installed: false, name: 'dplyr', message: 'network down' }); + expect(installPackages).not.toHaveBeenCalled(); + expect(error).toHaveBeenCalledWith('network down'); + }); + + it('reports a canceled version lookup without an error notification', async () => { + searchPackageVersions.mockRejectedValue(new CancellationError()); + + const result = await runInstall('dplyr', 'latest'); + + expect(result).toEqual({ + installed: false, + name: 'dplyr', + message: `Finding the latest version of 'dplyr' was canceled.`, + }); + expect(error).not.toHaveBeenCalled(); + }); + + it('reports a canceled install without an error notification', async () => { + stubProgress(true); + installPackages.mockRejectedValue(new CancellationError()); + + const result = await runInstall('dplyr', '1.1.4'); + + expect(result).toEqual({ + installed: false, + name: 'dplyr', + version: '1.1.4', + message: `The install of 'dplyr' was canceled.`, + }); + expect(error).not.toHaveBeenCalled(); + }); + + it('suggests a session restart after a successful install', async () => { + await runInstall('dplyr', '1.1.4'); + + expect(prompt).toHaveBeenCalled(); + }); + + it('does not suggest a restart when there is no active session', async () => { + stubServices(undefined); + + const result = await runInstall('dplyr', '1.1.4'); + + expect(result).toEqual({ installed: true, name: 'dplyr', version: '1.1.4' }); + expect(prompt).not.toHaveBeenCalled(); + }); + + it('opens the search quick-pick when a name is given with no version', async () => { + const result = await runInstall('dplyr'); + + expect(installPackageMock).toHaveBeenCalledOnce(); + expect(installPackages).not.toHaveBeenCalled(); + expect(result).toEqual({ installed: false, message: 'No package was selected.' }); + }); + + it('opens the search quick-pick when a menu context object is passed as arg0', async () => { + await runInstall({ id: 'menu-context' }); + + expect(installPackageMock).toHaveBeenCalledOnce(); + expect(installPackages).not.toHaveBeenCalled(); + }); + + it('returns the outcome of an install chosen in the quick-pick', async () => { + // The helper's fourth argument is the performInstall callback. + installPackageMock.mockImplementation(async (_accessor, _search, _versions, performInstall) => + performInstall('tibble', '3.2.1')); + + const result = await runInstall(); + + expect(result).toEqual({ installed: true, name: 'tibble', version: '3.2.1' }); + expect(installPackages).toHaveBeenCalledWith([{ name: 'tibble', version: '3.2.1' }], expect.anything()); + }); + + it('reports a failed install chosen in the quick-pick', async () => { + installPackages.mockRejectedValue(new Error('boom')); + installPackageMock.mockImplementation(async (_accessor, _search, _versions, performInstall) => + performInstall('tibble', '3.2.1')); + + const result = await runInstall(); + + expect(result).toEqual({ installed: false, name: 'tibble', version: '3.2.1', message: 'boom' }); + }); + }); + + describe('UpdatePackageAction', () => { + it('updates to an exact version without opening a picker or looking up versions', async () => { + const result = await runUpdate('dplyr', '1.1.4'); + + expect(result).toEqual({ updated: true, name: 'dplyr', version: '1.1.4' }); + expect(updatePackages).toHaveBeenCalledWith([{ name: 'dplyr', version: '1.1.4' }], expect.anything()); + expect(searchPackageVersions).not.toHaveBeenCalled(); + expect(updatePackageMock).not.toHaveBeenCalled(); + }); + + it('resolves \'latest\' to a concrete version, never undefined', async () => { + // pip and uv-outside-a-project reject a missing version on update, so + // the version reaching the service must always be concrete. + const result = await runUpdate('dplyr', 'latest'); + + expect(result).toEqual({ updated: true, name: 'dplyr', version: '1.9' }); + expect(updatePackages).toHaveBeenCalledWith([{ name: 'dplyr', version: '1.9' }], expect.anything()); + }); + + it('accepts \'latest\' in any case, and trims both arguments', async () => { + const result = await runUpdate(' dplyr ', ' LATEST '); + + expect(result).toEqual({ updated: true, name: 'dplyr', version: '1.9' }); + expect(updatePackages).toHaveBeenCalledWith([{ name: 'dplyr', version: '1.9' }], expect.anything()); + }); + + it('reports a failed update instead of reporting success', async () => { + updatePackages.mockRejectedValue(new Error('A version is required to update \'dplyr\'.')); + + const result = await runUpdate('dplyr', '1.1.4'); + + expect(result).toEqual({ + updated: false, + name: 'dplyr', + version: '1.1.4', + message: `A version is required to update 'dplyr'.`, + }); + expect(error).toHaveBeenCalledWith(`A version is required to update 'dplyr'.`); + expect(prompt).not.toHaveBeenCalled(); + }); + + it('reports when no versions are available for \'latest\'', async () => { + searchPackageVersions.mockResolvedValue([]); + + const result = await runUpdate('nope', 'latest'); + + expect(result).toEqual({ + updated: false, + name: 'nope', + message: `No available versions of 'nope' were found. Check the package name and the repositories configured for this session.`, + }); + expect(updatePackages).not.toHaveBeenCalled(); + }); + + it('reports a failed version lookup for \'latest\'', async () => { + searchPackageVersions.mockRejectedValue(new Error('network down')); + + const result = await runUpdate('dplyr', 'latest'); + + expect(result).toEqual({ updated: false, name: 'dplyr', message: 'network down' }); + expect(updatePackages).not.toHaveBeenCalled(); + }); + + it('reports a canceled update without an error notification', async () => { + stubProgress(true); + updatePackages.mockRejectedValue(new CancellationError()); + + const result = await runUpdate('dplyr', '1.1.4'); + + expect(result).toEqual({ + updated: false, + name: 'dplyr', + version: '1.1.4', + message: `The update of 'dplyr' was canceled.`, + }); + expect(error).not.toHaveBeenCalled(); + }); + + it('routes a name-only invocation to the version picker with the package pre-selected', async () => { + const result = await runUpdate('dplyr'); + + expect(updatePackageMock).toHaveBeenCalledOnce(); + // The helper's fifth argument is the pre-selected package. + expect(updatePackageMock.mock.calls[0][4]).toBe('dplyr'); + expect(updatePackages).not.toHaveBeenCalled(); + expect(result).toEqual({ updated: false, name: 'dplyr', message: 'No version was selected.' }); + }); + + it('routes a menu context object to the package picker', async () => { + const result = await runUpdate({ id: 'menu-context' }); + + expect(updatePackageMock.mock.calls[0][4]).toBeUndefined(); + expect(result).toEqual({ updated: false, message: 'No package was selected.' }); + }); + + it('returns the outcome of an update chosen in the quick-pick', async () => { + updatePackageMock.mockImplementation(async (_accessor, _search, _versions, performUpdate) => + performUpdate('dplyr', '1.1.10')); + + const result = await runUpdate('dplyr'); + + expect(result).toEqual({ updated: true, name: 'dplyr', version: '1.1.10' }); + expect(updatePackages).toHaveBeenCalledWith([{ name: 'dplyr', version: '1.1.10' }], expect.anything()); + }); + }); +}); From fc22855e9b7260c0137a8ccc3d64263929639551 Mon Sep 17 00:00:00 2001 From: Dhruvi Sompura Date: Tue, 28 Jul 2026 11:10:22 -0700 Subject: [PATCH 09/14] Detect prerelease versions semver cannot parse newestAvailableVersion skipped three-component prereleases like 2.0.0rc1 but not two-component ones like 1.0a2 and 2.0b1, or dot-separated dev builds like 1.0.0.dev1. semver.valid rejects those spellings and semver.coerce then drops the suffix, so they read as the stable 1.0.0 and were installed in preference to an older stable release. With 0.9 and 1.0a2 available it picked 1.0a2, where pip install would give 0.9. The version string is now checked directly as well as through semver. A trailing digit group is required, which keeps out three things that only look similar: conda's letter-suffixed builds, R's patch levels, and post-releases, which are newer than the release they follow rather than prereleases of it. Co-Authored-By: Claude Opus 5 (1M context) --- .../browser/packageVersions.ts | 34 +++++++++-- .../test/browser/packageVersions.vitest.ts | 57 +++++++++++++++++++ 2 files changed, 86 insertions(+), 5 deletions(-) diff --git a/src/vs/workbench/contrib/positronPackages/browser/packageVersions.ts b/src/vs/workbench/contrib/positronPackages/browser/packageVersions.ts index 8920fdce423..4aed3befb28 100644 --- a/src/vs/workbench/contrib/positronPackages/browser/packageVersions.ts +++ b/src/vs/workbench/contrib/positronPackages/browser/packageVersions.ts @@ -23,6 +23,34 @@ export function sortVersionsDescending(versions: string[]): string[] { }); } +/** + * Prerelease and development suffixes, as PEP 440 and semver spell them: + * `2.0.0rc1`, `1.0a2`, `2.0.0-rc.1`, `1.0.0.dev1`, `3.0.0alpha1`. + * + * A trailing digit group is required, which is what keeps three things that only + * look similar out: conda's letter-suffixed builds (`1.1.1c`), R's patch levels + * (`1.0-3`), and PEP 440 post-releases (`1.0.post1`, which is *newer* than + * `1.0`, not a prerelease). + */ +const PRERELEASE_SUFFIX = /[-_.]?(?:a|b|c|rc|alpha|beta|pre|preview|dev)[-_.]?\d+$/i; + +/** + * Whether a version string names a prerelease. + * + * The string is tested directly as well as through semver, because semver alone + * is not enough: `semver.valid` rejects two-component versions like `1.0a2` and + * `2.0b1`, and `semver.coerce` then drops the suffix, so they would read as the + * stable `1.0.0` and `2.0.0`. + */ +function isPrerelease(version: string): boolean { + const trimmed = version.trim(); + if (PRERELEASE_SUFFIX.test(trimmed)) { + return true; + } + const parsed = semver.valid(trimmed, true) || semver.coerce(trimmed); + return parsed ? !!semver.prerelease(parsed, true) : false; +} + /** * Pick the version to use when the caller asked for the newest available one. * @@ -39,9 +67,5 @@ export function sortVersionsDescending(versions: string[]): string[] { */ export function newestAvailableVersion(versions: string[]): string | undefined { const sorted = sortVersionsDescending(versions); - const stable = sorted.find(version => { - const parsed = semver.valid(version, true) || semver.coerce(version); - return parsed ? !semver.prerelease(parsed, true) : true; - }); - return stable ?? sorted.at(0); + return sorted.find(version => !isPrerelease(version)) ?? sorted.at(0); } diff --git a/src/vs/workbench/contrib/positronPackages/test/browser/packageVersions.vitest.ts b/src/vs/workbench/contrib/positronPackages/test/browser/packageVersions.vitest.ts index 42b97ab8e4b..cecda07c813 100644 --- a/src/vs/workbench/contrib/positronPackages/test/browser/packageVersions.vitest.ts +++ b/src/vs/workbench/contrib/positronPackages/test/browser/packageVersions.vitest.ts @@ -68,6 +68,63 @@ describe('newestAvailableVersion', () => { expect(newestAvailableVersion(['1.0.0rc1', '1.0.0rc2'])).toBe('1.0.0rc2'); }); + // semver alone does not catch these: `semver.valid` rejects two-component + // versions and `semver.coerce` then drops the suffix, so `1.0a2` would read + // as the stable `1.0.0` and be installed over the older stable release. + it('skips every prerelease spelling in favor of an older stable release', () => { + expect({ + threeComponentRc: newestAvailableVersion(['1.9', '2.0.0rc1']), + threeComponentAlpha: newestAvailableVersion(['1.9.0', '2.0.0a2']), + threeComponentBeta: newestAvailableVersion(['1.9.0', '2.0.0b1']), + twoComponentAlpha: newestAvailableVersion(['0.9', '1.0a2']), + twoComponentBeta: newestAvailableVersion(['1.9', '2.0b1']), + dottedDev: newestAvailableVersion(['0.9.0', '1.0.0.dev1']), + runTogetherDev: newestAvailableVersion(['0.9.0', '1.0.0dev1']), + semverDotted: newestAvailableVersion(['1.9.0', '2.0.0-rc.1']), + spelledAlpha: newestAvailableVersion(['1.9.0', '3.0.0alpha1']), + pep440C: newestAvailableVersion(['0.9', '1.0c1']), + }).toEqual({ + threeComponentRc: '1.9', + threeComponentAlpha: '1.9.0', + threeComponentBeta: '1.9.0', + twoComponentAlpha: '0.9', + twoComponentBeta: '1.9', + dottedDev: '0.9.0', + runTogetherDev: '0.9.0', + semverDotted: '1.9.0', + spelledAlpha: '1.9.0', + pep440C: '0.9', + }); + }); + + // A post-release is newer than the release it follows, and R's patch level is + // not a prerelease at all, so neither may be skipped. + it('does not mistake post-releases or R patch levels for prereleases', () => { + expect({ + postRelease: newestAvailableVersion(['1.0.post1']), + rPatchLevel: newestAvailableVersion(['1.0-3']), + calendarVersion: newestAvailableVersion(['2024.1.1']), + }).toEqual({ + postRelease: '1.0.post1', + rPatchLevel: '1.0-3', + calendarVersion: '2024.1.1', + }); + }); + + // conda's letter-suffixed builds have no trailing digits, so the suffix check + // passes them through, but semver reads `1.1.1c` as the prerelease `1.1.1-c` + // and it loses to plain `1.1.1`. Rare and low-stakes: the fallback still + // returns it when it is the only version. + it('prefers a plain release over a conda letter-suffixed build', () => { + expect({ + alongsidePlain: newestAvailableVersion(['1.1.1', '1.1.1c']), + onItsOwn: newestAvailableVersion(['1.1.1c']), + }).toEqual({ + alongsidePlain: '1.1.1', + onItsOwn: '1.1.1c', + }); + }); + it('returns the only version R reports', () => { expect(newestAvailableVersion(['1.1.4'])).toBe('1.1.4'); }); From 719191320c635f8ba4b5c3cc1de64e90d7079cdf Mon Sep 17 00:00:00 2001 From: Dhruvi Sompura Date: Tue, 28 Jul 2026 11:39:07 -0700 Subject: [PATCH 10/14] Compare every numeric group when resolving the newest version newestAvailableVersion reused the version picker's comparison, which runs versions through semver.coerce and so truncates anything past three segments. Four-segment versions tied and the tie fell back to PyPI's ascending order, putting the older one first: with 1.2.3.4 and 1.2.3.5 available it picked 1.2.3.4. PEP 440 epochs were unparseable for the same reason, so 3.0 beat 1!2.0. Resolving a version automatically now uses its own comparison, which keeps every numeric group and reads the epoch. R patch levels sort correctly as a side effect (1.0-10 over 1.0-3), though R reports a single version so nothing depended on it. The picker keeps its existing ordering. A truncated sort is a cosmetic problem when a human is choosing from the whole list, but silently installs the wrong version when nothing is choosing. Co-Authored-By: Claude Opus 5 (1M context) --- .../browser/packageVersions.ts | 48 ++++++++++++++++++- .../test/browser/packageVersions.vitest.ts | 30 ++++++++++++ 2 files changed, 77 insertions(+), 1 deletion(-) diff --git a/src/vs/workbench/contrib/positronPackages/browser/packageVersions.ts b/src/vs/workbench/contrib/positronPackages/browser/packageVersions.ts index 4aed3befb28..b041bc49ffd 100644 --- a/src/vs/workbench/contrib/positronPackages/browser/packageVersions.ts +++ b/src/vs/workbench/contrib/positronPackages/browser/packageVersions.ts @@ -8,6 +8,11 @@ import * as semver from '../../../../base/common/semver/semver.js'; /** * Sort version strings in descending order (newest first). * Uses semver comparison when possible, falls back to string comparison. + * + * This drives the version quick-pick, where the user picks from the whole list, + * so its ordering is deliberately left as-is. `newestAvailableVersion` does not + * use it: choosing a version automatically needs the more precise comparison + * below, because `semver.coerce` truncates anything past three segments. */ export function sortVersionsDescending(versions: string[]): string[] { return [...versions].sort((a, b) => { @@ -51,6 +56,47 @@ function isPrerelease(version: string): boolean { return parsed ? !!semver.prerelease(parsed, true) : false; } +/** + * Split a version into the parts that decide which of two versions is newer: a + * PEP 440 epoch, and the numeric groups that follow it. + * + * Every numeric group is kept, however many there are, so four-segment versions + * (`1.2.3.4`) and R patch levels (`1.0-3`) compare on their real values rather + * than being truncated to three segments. Digits inside a suffix count too, + * which is what orders `1.0.post1` above `1.0` and `1.0.0rc2` above `1.0.0rc1`. + */ +function releaseKey(version: string): { epoch: number; segments: number[] } { + const trimmed = version.trim(); + const epochSeparator = trimmed.indexOf('!'); + const epoch = epochSeparator === -1 ? 0 : Number(trimmed.slice(0, epochSeparator)) || 0; + const release = epochSeparator === -1 ? trimmed : trimmed.slice(epochSeparator + 1); + const segments = (release.match(/\d+/g) ?? []).map(Number); + return { epoch, segments }; +} + +/** + * Compare two versions, newest first. A higher epoch always wins; otherwise the + * numeric groups are compared in order, treating a missing group as zero so + * `1.2` and `1.2.0` are equal. Versions with no digits at all fall back to + * string comparison. + */ +function compareVersionsDescending(a: string, b: string): number { + const aKey = releaseKey(a); + const bKey = releaseKey(b); + if (aKey.epoch !== bKey.epoch) { + return bKey.epoch - aKey.epoch; + } + const length = Math.max(aKey.segments.length, bKey.segments.length); + for (let i = 0; i < length; i++) { + const aSegment = aKey.segments[i] ?? 0; + const bSegment = bKey.segments[i] ?? 0; + if (aSegment !== bSegment) { + return bSegment - aSegment; + } + } + return a < b ? 1 : a > b ? -1 : 0; +} + /** * Pick the version to use when the caller asked for the newest available one. * @@ -66,6 +112,6 @@ function isPrerelease(version: string): boolean { * prereleases, so a caller always gets a version if one exists. */ export function newestAvailableVersion(versions: string[]): string | undefined { - const sorted = sortVersionsDescending(versions); + const sorted = [...versions].sort(compareVersionsDescending); return sorted.find(version => !isPrerelease(version)) ?? sorted.at(0); } diff --git a/src/vs/workbench/contrib/positronPackages/test/browser/packageVersions.vitest.ts b/src/vs/workbench/contrib/positronPackages/test/browser/packageVersions.vitest.ts index cecda07c813..5cf5be70a96 100644 --- a/src/vs/workbench/contrib/positronPackages/test/browser/packageVersions.vitest.ts +++ b/src/vs/workbench/contrib/positronPackages/test/browser/packageVersions.vitest.ts @@ -132,4 +132,34 @@ describe('newestAvailableVersion', () => { it('returns undefined when no versions are available', () => { expect(newestAvailableVersion([])).toBeUndefined(); }); + + // Every numeric group counts, so these do not get truncated to three + // segments the way semver.coerce would truncate them. The quick-pick's + // sortVersionsDescending still does truncate; only automatic selection uses + // the precise comparison. + it('compares every numeric group rather than the first three', () => { + expect({ + fourSegment: newestAvailableVersion(['1.2.3.4', '1.2.3.5']), + rPatchLevel: newestAvailableVersion(['1.0-3', '1.0-10']), + calendarVersion: newestAvailableVersion(['2024.1.1', '2024.1.2']), + missingGroupIsZero: newestAvailableVersion(['1.2', '1.2.1']), + }).toEqual({ + fourSegment: '1.2.3.5', + rPatchLevel: '1.0-10', + calendarVersion: '2024.1.2', + missingGroupIsZero: '1.2.1', + }); + }); + + it('ranks a higher PEP 440 epoch above a larger release number', () => { + expect(newestAvailableVersion(['3.0', '1!2.0'])).toBe('1!2.0'); + }); + + it('ranks a post-release above the release it follows', () => { + expect(newestAvailableVersion(['1.0', '1.0.post1'])).toBe('1.0.post1'); + }); + + it('falls back to string comparison for versions with no digits', () => { + expect(newestAvailableVersion(['alpha', 'beta'])).toBe('beta'); + }); }); From eddf2caa4e25292a9c3e58f87530f84a95087db0 Mon Sep 17 00:00:00 2001 From: Dhruvi Sompura Date: Tue, 28 Jul 2026 14:58:27 -0700 Subject: [PATCH 11/14] Cover malformed arguments to install and update An agent can send the wrong shape: a missing version, a missing name, a number where a string belongs, an empty string, or nothing at all. None of those install or update anything unintended -- every one falls through to the quick-pick and reports a false result. Pinned so that stays true, along with extra trailing arguments being ignored. Co-Authored-By: Claude Opus 5 (1M context) --- .../browser/installAndUpdatePackage.vitest.ts | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/src/vs/workbench/contrib/positronPackages/test/browser/installAndUpdatePackage.vitest.ts b/src/vs/workbench/contrib/positronPackages/test/browser/installAndUpdatePackage.vitest.ts index 9a0d0db769c..a52fb4d7024 100644 --- a/src/vs/workbench/contrib/positronPackages/test/browser/installAndUpdatePackage.vitest.ts +++ b/src/vs/workbench/contrib/positronPackages/test/browser/installAndUpdatePackage.vitest.ts @@ -227,6 +227,35 @@ describe('packages install and update commands', () => { expect(installPackages).not.toHaveBeenCalled(); }); + // Every one of these falls through to the quick-pick rather than + // installing something unintended. A caller with no user in front of it + // gets the no-selection result once the picker is dismissed. + it('never installs anything when the arguments are malformed', async () => { + const results = []; + for (const args of [ + [''], // empty name + ['dplyr', ''], // empty version + [' ', '1.1.4'], // whitespace-only name + [undefined, '1.1.4'], // version with no name + ['dplyr', 1.14], // non-string version + [42, '1.1.4'], // non-string name + [null, null], // nulls + [], // no arguments at all (the palette) + ]) { + results.push(await runInstall(...args)); + } + + expect(installPackages).not.toHaveBeenCalled(); + expect(results.every(r => r.installed === false)).toBe(true); + }); + + it('ignores extra arguments beyond name and version', async () => { + const result = await runInstall('dplyr', '1.1.4', 'unexpected', { more: true }); + + expect(result).toEqual({ installed: true, name: 'dplyr', version: '1.1.4' }); + expect(installPackages).toHaveBeenCalledWith([{ name: 'dplyr', version: '1.1.4' }], expect.anything()); + }); + it('returns the outcome of an install chosen in the quick-pick', async () => { // The helper's fourth argument is the performInstall callback. installPackageMock.mockImplementation(async (_accessor, _search, _versions, performInstall) => @@ -344,6 +373,25 @@ describe('packages install and update commands', () => { expect(result).toEqual({ updated: false, message: 'No package was selected.' }); }); + it('never updates anything when the arguments are malformed', async () => { + const results = []; + for (const args of [ + [''], + ['dplyr', ''], + [' ', '1.1.4'], + [undefined, '1.1.4'], + ['dplyr', 1.14], + [42, '1.1.4'], + [null, null], + [], + ]) { + results.push(await runUpdate(...args)); + } + + expect(updatePackages).not.toHaveBeenCalled(); + expect(results.every(r => r.updated === false)).toBe(true); + }); + it('returns the outcome of an update chosen in the quick-pick', async () => { updatePackageMock.mockImplementation(async (_accessor, _search, _versions, performUpdate) => performUpdate('dplyr', '1.1.10')); From 7cc170b96ff8408687986fd47fe2ba86b68d52a8 Mon Sep 17 00:00:00 2001 From: Dhruvi Sompura Date: Tue, 28 Jul 2026 15:14:57 -0700 Subject: [PATCH 12/14] Correct the palette requirement in the agentCompatible docs The comment said an agent-compatible command must also be palette-exposed so a user could run it themselves. Nothing enforces that: agentCompatible alone decides what getAgentAllowedCommands returns, and a test covers a command that is advertised without being in the palette. Keeps the intent, which is still worth stating as a preference: putting an agent-compatible command in the palette means anything an agent can do, a user can do for themselves. Also stops naming f1 as the only route in -- the view focus commands reach the palette through a MenuId.CommandPalette entry instead. Co-Authored-By: Claude Opus 5 (1M context) --- src/vs/platform/commands/common/commands.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/vs/platform/commands/common/commands.ts b/src/vs/platform/commands/common/commands.ts index ed3828b0d4a..4e90986b0c8 100644 --- a/src/vs/platform/commands/common/commands.ts +++ b/src/vs/platform/commands/common/commands.ts @@ -56,8 +56,13 @@ export interface ICommandMetadata { // --- Start Positron --- /** * When true, this command is exposed to AI agents via - * `positron.ai.getAgentAllowedCommands()`. The command must also be - * palette-exposed (`f1: true`) so a user could run it themselves. + * `positron.ai.getAgentAllowedCommands()`. + * + * Prefer exposing the command in the command palette as well (usually + * `f1: true`), so that anything an agent can do, a user can do for + * themselves. That is a preference rather than a requirement: this flag + * alone decides what is advertised, so a command outside the palette is + * still exposed to agents. */ readonly agentCompatible?: boolean; // --- End Positron --- From 0c0b7c277b8c8c827b5df17b0b72ee1a87595340 Mon Sep 17 00:00:00 2001 From: Dhruvi Sompura Date: Tue, 28 Jul 2026 16:03:54 -0700 Subject: [PATCH 13/14] Report a missing PyPI project as no versions, not a JSON error searchPyPIVersions called response.json() without checking the response. PyPI answers 404 for a project that does not exist with the plain-text body '404 Not Found', which parses as the number 404 followed by unexpected text, so the caller got 'Unexpected non-whitespace character after JSON at position 4' instead of a usable answer. A name that does not exist is routine, not exceptional -- any caller can pass one that was typed or guessed -- so a 404 now returns no versions. The packages commands already turn that into "No available versions of '' were found. Check the package name and the repositories configured for this session." Other non-OK responses throw naming the status, so a PyPI outage does not masquerade as a missing package. The bug was unreachable until now: searchPackageVersions was only called from the version quick-pick, which runs after the user has picked a real package out of search results. Resolving 'latest' calls it with a name supplied by an agent, which may not exist at all. Co-Authored-By: Claude Opus 5 (1M context) --- .../client/positron/packages/pypiSearch.ts | 15 +++++++++ .../src/test/positron/pypiSearch.unit.test.ts | 32 +++++++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/extensions/positron-python/src/client/positron/packages/pypiSearch.ts b/extensions/positron-python/src/client/positron/packages/pypiSearch.ts index 7ebdfd559e9..ef236d9e97a 100644 --- a/extensions/positron-python/src/client/positron/packages/pypiSearch.ts +++ b/extensions/positron-python/src/client/positron/packages/pypiSearch.ts @@ -255,6 +255,21 @@ export async function searchPyPIVersions( headers: { Accept: 'application/vnd.pypi.simple.v1+json' }, signal: createAbortSignal(token), }); + + // A project that does not exist is a routine outcome, not a failure: any + // caller can pass a name that was typed or guessed. PyPI answers 404 with + // the plain-text body `404 Not Found`, so the response must be checked + // before parsing -- feeding that body to `json()` throws an opaque + // "Unexpected non-whitespace character after JSON" instead. + if (response.status === 404) { + return []; + } + if (!response.ok) { + throw new Error( + `Could not look up versions of '${name}' on PyPI (HTTP ${response.status}).`, + ); + } + const json = (await response.json()) as { versions?: string[]; files?: PyPIFile[] }; const versions = json.versions ?? []; const files = json.files ?? []; diff --git a/extensions/positron-python/src/test/positron/pypiSearch.unit.test.ts b/extensions/positron-python/src/test/positron/pypiSearch.unit.test.ts index 3b291a4c5a8..41bfe1777a0 100644 --- a/extensions/positron-python/src/test/positron/pypiSearch.unit.test.ts +++ b/extensions/positron-python/src/test/positron/pypiSearch.unit.test.ts @@ -202,6 +202,38 @@ suite('searchPyPIVersions', () => { expect(result).to.deep.equal(['1.0']); }); + test('reports no versions for a project that does not exist', async () => { + // PyPI answers 404 with the plain-text body `404 Not Found`. Parsing that + // as JSON throws "Unexpected non-whitespace character after JSON at + // position 4", which used to reach the user verbatim. + fetchStub.resolves({ + ok: false, + status: 404, + json: () => Promise.reject(new SyntaxError('Unexpected non-whitespace character after JSON at position 4')), + } as unknown as Response); + + const result = await searchPyPIVersions('definitely-not-a-real-package-xyz', async () => ({})); + + expect(result).to.deep.equal([]); + }); + + test('throws a readable error when PyPI is unavailable', async () => { + fetchStub.resolves({ + ok: false, + status: 503, + json: () => Promise.reject(new SyntaxError('Unexpected token')), + } as unknown as Response); + + let message = ''; + try { + await searchPyPIVersions('pkg', async () => ({})); + } catch (e) { + message = e instanceof Error ? e.message : String(e); + } + + expect(message).to.match(/HTTP 503/); + }); + test('maps files to the longest matching version (1.0 vs 1.0.1)', async () => { fetchStub.resolves( makeVersionsResponse( From 771a0a2978a573dc6d67278d1858907c24b8f7f1 Mon Sep 17 00:00:00 2001 From: Dhruvi Sompura Date: Tue, 28 Jul 2026 17:28:04 -0700 Subject: [PATCH 14/14] Format pypiSearch with prettier The positron-python extension runs prettier --check in CI, and it wants the throw on one line. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/client/positron/packages/pypiSearch.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/extensions/positron-python/src/client/positron/packages/pypiSearch.ts b/extensions/positron-python/src/client/positron/packages/pypiSearch.ts index ef236d9e97a..d9c3e00ed06 100644 --- a/extensions/positron-python/src/client/positron/packages/pypiSearch.ts +++ b/extensions/positron-python/src/client/positron/packages/pypiSearch.ts @@ -265,9 +265,7 @@ export async function searchPyPIVersions( return []; } if (!response.ok) { - throw new Error( - `Could not look up versions of '${name}' on PyPI (HTTP ${response.status}).`, - ); + throw new Error(`Could not look up versions of '${name}' on PyPI (HTTP ${response.status}).`); } const json = (await response.json()) as { versions?: string[]; files?: PyPIFile[] };