diff --git a/.github/dependabot.yml b/.github/dependabot.yml index 456482128f61dd..a2aa2419434320 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -14,5 +14,7 @@ updates: directory: "/extensions/markdown-language-features" schedule: interval: "daily" + time: "16:00" + timezone: "America/Los_Angeles" allow: - dependency-name: "@vscode/markdown-editor" diff --git a/.github/instructions/css-best-practices.instructions.md b/.github/instructions/css-best-practices.instructions.md index 9b9e0696677908..f83a0ceb4b0931 100644 --- a/.github/instructions/css-best-practices.instructions.md +++ b/.github/instructions/css-best-practices.instructions.md @@ -8,3 +8,4 @@ applyTo: "**/*.css" ## Selectors - Avoid `:has()` selectors. Because their result depends on descendant state, DOM mutations can invalidate styles on ancestors and cause expensive style recalculation, especially when selectors are broadly scoped. Instead, represent the state explicitly with a class or data attribute on the smallest container you own, and scope selectors to that marker. Add and remove the marker together with the state it represents. +- Never match the `class` attribute by substring (`[class*="…"]`, `[class^="…"]`, `[class$="…"]`). A single such selector anywhere in the workbench stylesheet defeats Blink's per-class invalidation: every `classList` change then forces a style recalculation for that element, even when no rule references the class that changed. Measured on a 3.7k-node workbench, the ten `[class*="monaco-decoration-itemColor"]` selectors in the Modern UI tab styles alone made a full style recalculation 2.4x slower. When a class carries a generated suffix, have the code that applies it also set a stable marker class (see `DECORATION_LABEL_COLOR_CLASS`) and match that instead. diff --git a/.vscode-test.js b/.vscode-test.js index 9eb863743449b2..7cdb29fc7c639c 100644 --- a/.vscode-test.js +++ b/.vscode-test.js @@ -57,6 +57,11 @@ const extensions = [ workspaceFolder: path.join(os.tmpdir(), `confeditout-${Math.floor(Math.random() * 100000)}`), mocha: { timeout: 60_000 } }, + { + label: 'npm', + workspaceFolder: path.join(os.tmpdir(), `npmout-${Math.floor(Math.random() * 100000)}`), + mocha: { timeout: 60_000 } + }, { label: 'github-authentication', workspaceFolder: path.join(os.tmpdir(), `msft-auth-${Math.floor(Math.random() * 100000)}`), diff --git a/extensions/npm/src/features/npmViewParser.ts b/extensions/npm/src/features/npmViewParser.ts new file mode 100644 index 00000000000000..662d3be66350ff --- /dev/null +++ b/extensions/npm/src/features/npmViewParser.ts @@ -0,0 +1,40 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +export interface ViewPackageInfo { + description: string; + version?: string; + time?: string; + homepage?: string; + installedVersion?: string; +} + +export interface NpmViewRecord { + description?: string; + version?: string; + homepage?: string; + time?: { [version: string]: string }; + 'dist-tags.latest'?: string; +} + +/** + * Parses the output of `npm view --json`. npm 12+ always returns an array `[{...}]`, + * even for a single package, while older versions return the object directly. + */ +export function parseNpmViewOutput(stdout: string): ViewPackageInfo | undefined { + try { + const parsed = JSON.parse(stdout) as NpmViewRecord | NpmViewRecord[]; + const content = Array.isArray(parsed) ? parsed[0] : parsed; + const version = content['dist-tags.latest'] || content.version; + return { + description: content.description ?? '', + version, + time: version ? content.time?.[version] : undefined, + homepage: content.homepage + }; + } catch (e) { + return undefined; + } +} \ No newline at end of file diff --git a/extensions/npm/src/features/packageJSONContribution.ts b/extensions/npm/src/features/packageJSONContribution.ts index bf131a205790f8..f62289f45c31ce 100644 --- a/extensions/npm/src/features/packageJSONContribution.ts +++ b/extensions/npm/src/features/packageJSONContribution.ts @@ -11,6 +11,7 @@ import { Location } from 'jsonc-parser'; import type * as cp from 'child_process'; import { dirname } from 'path'; import { fromNow } from './date'; +import { parseNpmViewOutput, ViewPackageInfo } from './npmViewParser'; const LIMIT = 40; @@ -325,21 +326,7 @@ export class PackageJSONContribution implements IJSONContribution { private async npmView(npmCommandPath: string, pack: string, resource: Uri | undefined): Promise { const args = ['view', '--json', '--', pack, 'description', 'dist-tags.latest', 'homepage', 'version', 'time']; const stdout = await this.runNpmCommand(npmCommandPath, args, resource); - if (stdout) { - try { - const content = JSON.parse(stdout); - const version = content['dist-tags.latest'] || content['version']; - return { - description: content['description'], - version, - time: content.time?.[version], - homepage: content['homepage'] - }; - } catch (e) { - // ignore - } - } - return undefined; + return stdout ? parseNpmViewOutput(stdout) : undefined; } private async npmjsView(pack: string): Promise { @@ -429,11 +416,3 @@ interface SearchPackageInfo { version?: string; links?: { homepage?: string }; } - -interface ViewPackageInfo { - description: string; - version?: string; - time?: string; - homepage?: string; - installedVersion?: string; -} diff --git a/extensions/npm/src/test/index.ts b/extensions/npm/src/test/index.ts new file mode 100644 index 00000000000000..af83ef27ab9e95 --- /dev/null +++ b/extensions/npm/src/test/index.ts @@ -0,0 +1,42 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import * as path from 'path'; +import * as testRunner from '../../../../test/integration/electron/testrunner'; + +const options: import('mocha').MochaOptions = { + ui: 'tdd', + color: true, + timeout: 60000 +}; + +// These integration tests is being run in multiple environments (electron, web, remote) +// so we need to set the suite name based on the environment as the suite name is used +// for the test results file name +let suite = ''; +if (process.env.VSCODE_BROWSER) { + suite = `${process.env.VSCODE_BROWSER} Browser Integration Npm Tests`; +} else if (process.env.REMOTE_VSCODE) { + suite = 'Remote Integration Npm Tests'; +} else { + suite = 'Integration Npm Tests'; +} + +if (process.env.BUILD_ARTIFACTSTAGINGDIRECTORY || process.env.GITHUB_WORKSPACE) { + options.reporter = 'mocha-multi-reporters'; + options.reporterOptions = { + reporterEnabled: 'spec, mocha-junit-reporter', + mochaJunitReporterReporterOptions: { + testsuitesTitle: `${suite} ${process.platform}`, + mochaFile: path.join( + process.env.BUILD_ARTIFACTSTAGINGDIRECTORY || process.env.GITHUB_WORKSPACE || __dirname, + `test-results/${process.platform}-${process.arch}-${suite.toLowerCase().replace(/[^\w]/g, '-')}-results.xml`) + } + }; +} + +testRunner.configure(options); + +export = testRunner; \ No newline at end of file diff --git a/extensions/npm/src/test/npmViewParser.test.ts b/extensions/npm/src/test/npmViewParser.test.ts new file mode 100644 index 00000000000000..541cc325f3ce7f --- /dev/null +++ b/extensions/npm/src/test/npmViewParser.test.ts @@ -0,0 +1,105 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import * as assert from 'assert'; +import { NpmViewRecord, parseNpmViewOutput } from '../features/npmViewParser'; + +const npmViewOutput: NpmViewRecord = { + description: 'React is a JavaScript library for building user interfaces.', + 'dist-tags.latest': '19.1.0', + homepage: 'https://react.dev/', + version: '19.1.0', + time: { + '19.1.0': '2025-05-20T20:58:48.397Z', + '0.0.0-experimental-98e8ed76': '2026-07-25T21:39:01.123Z' + } +}; + +suite('npmViewParser', () => { + + test('parses object output (npm <= 11)', () => { + const info = parseNpmViewOutput(JSON.stringify(npmViewOutput)); + assert.ok(info); + assert.strictEqual(info!.description, 'React is a JavaScript library for building user interfaces.'); + assert.strictEqual(info!.version, '19.1.0'); + assert.strictEqual(info!.time, '2025-05-20T20:58:48.397Z'); + assert.strictEqual(info!.homepage, 'https://react.dev/'); + }); + + test('parses array output (npm 12+)', () => { + const info = parseNpmViewOutput(JSON.stringify([npmViewOutput])); + assert.ok(info); + assert.strictEqual(info!.description, 'React is a JavaScript library for building user interfaces.'); + assert.strictEqual(info!.version, '19.1.0'); + assert.strictEqual(info!.time, '2025-05-20T20:58:48.397Z'); + assert.strictEqual(info!.homepage, 'https://react.dev/'); + }); + + test('prefers dist-tags.latest over version', () => { + const info = parseNpmViewOutput(JSON.stringify({ + 'dist-tags.latest': '19.1.0', + version: '0.0.0-experimental-98e8ed76', + time: { + '19.1.0': '2025-05-20T20:58:48.397Z', + '0.0.0-experimental-98e8ed76': '2026-07-25T21:39:01.123Z' + } + })); + assert.ok(info); + assert.strictEqual(info!.version, '19.1.0'); + assert.strictEqual(info!.time, '2025-05-20T20:58:48.397Z'); + assert.notStrictEqual(info!.time, '2026-07-25T21:39:01.123Z'); + }); + + test('uses the first element when the array contains multiple packages', () => { + const first = { ...npmViewOutput, description: 'first package' }; + const second = { ...npmViewOutput, description: 'second package' }; + const info = parseNpmViewOutput(JSON.stringify([first, second])); + assert.ok(info); + assert.strictEqual(info!.description, 'first package'); + }); + + test('falls back to the version field when dist-tags.latest is missing', () => { + const info = parseNpmViewOutput(JSON.stringify([ + { version: '0.0.0-experimental-98e8ed76', time: { '0.0.0-experimental-98e8ed76': '2026-07-25T21:39:01.123Z' } } + ])); + assert.ok(info); + assert.strictEqual(info!.version, '0.0.0-experimental-98e8ed76'); + assert.strictEqual(info!.time, '2026-07-25T21:39:01.123Z'); + assert.strictEqual(info!.description, ''); + assert.strictEqual(info!.homepage, undefined); + }); + + test('returns undefined version and time when neither field is present', () => { + const info = parseNpmViewOutput(JSON.stringify({ description: 'React is a JavaScript library for building user interfaces.' })); + assert.ok(info); + assert.strictEqual(info!.version, undefined); + assert.strictEqual(info!.time, undefined); + }); + + test('returns empty description when description is missing', () => { + const info = parseNpmViewOutput(JSON.stringify({ 'dist-tags.latest': '19.1.0' })); + assert.ok(info); + assert.strictEqual(info!.description, ''); + assert.strictEqual(info!.version, '19.1.0'); + }); + + test('returns undefined time when the resolved version has no matching time entry', () => { + const info = parseNpmViewOutput(JSON.stringify({ 'dist-tags.latest': '19.1.0', time: { '18.3.1': '2024-04-26T09:39:52.159Z' } })); + assert.ok(info); + assert.strictEqual(info!.version, '19.1.0'); + assert.strictEqual(info!.time, undefined); + }); + + test('returns undefined for invalid JSON', () => { + assert.strictEqual(parseNpmViewOutput('not json'), undefined); + assert.strictEqual(parseNpmViewOutput('{'), undefined); + assert.strictEqual(parseNpmViewOutput(''), undefined); + }); + + test('returns undefined for non-object output', () => { + assert.strictEqual(parseNpmViewOutput('null'), undefined); + assert.strictEqual(parseNpmViewOutput('[]'), undefined); + }); +}); \ No newline at end of file diff --git a/extensions/theme-abyss/themes/abyss-color-theme.json b/extensions/theme-abyss/themes/abyss-color-theme.json index 92a6a035e514b4..d3cc805170b94d 100644 --- a/extensions/theme-abyss/themes/abyss-color-theme.json +++ b/extensions/theme-abyss/themes/abyss-color-theme.json @@ -446,6 +446,7 @@ "surface.border": "#00000000", "modernActivityBar.activeBackground": "#08286b", "modernActivityBar.hoverBackground": "#08286b87", + "modernActivityBar.background": "#00000000", }, "semanticHighlighting": true } diff --git a/extensions/theme-defaults/themes/dark_vs.json b/extensions/theme-defaults/themes/dark_vs.json index feafeaf232ba8c..bfeee6a7382295 100644 --- a/extensions/theme-defaults/themes/dark_vs.json +++ b/extensions/theme-defaults/themes/dark_vs.json @@ -39,6 +39,7 @@ "surface.border": "#252526", "modernActivityBar.activeBackground": "#1E1E1E", "modernActivityBar.hoverBackground": "#1E1E1E66", + "modernActivityBar.background": "#00000000", }, "tokenColors": [ { diff --git a/extensions/theme-monokai-dimmed/themes/dimmed-monokai-color-theme.json b/extensions/theme-monokai-dimmed/themes/dimmed-monokai-color-theme.json index c31d568a805188..063559b7fa4375 100644 --- a/extensions/theme-monokai-dimmed/themes/dimmed-monokai-color-theme.json +++ b/extensions/theme-monokai-dimmed/themes/dimmed-monokai-color-theme.json @@ -70,6 +70,9 @@ "agentsCard.border": "#00000000", // modern ui "surface.border": "#00000000", + "modernActivityBar.background": "#00000000", + "modernActivityBar.activeBackground": "#353535", + "modernActivityBar.hoverBackground": "#35353566", }, "tokenColors": [ { diff --git a/extensions/theme-monokai/themes/monokai-color-theme.json b/extensions/theme-monokai/themes/monokai-color-theme.json index 6074878f362bc3..38077fe1ddf275 100644 --- a/extensions/theme-monokai/themes/monokai-color-theme.json +++ b/extensions/theme-monokai/themes/monokai-color-theme.json @@ -107,6 +107,7 @@ "terminal.ansiBrightWhite": "#f8f8f2", // modern ui "surface.border": "#272822", + "modernActivityBar.background": "#00000000", }, "tokenColors": [ { diff --git a/extensions/theme-red/themes/Red-color-theme.json b/extensions/theme-red/themes/Red-color-theme.json index 1b25b98f91e067..8dae8f4f91f232 100644 --- a/extensions/theme-red/themes/Red-color-theme.json +++ b/extensions/theme-red/themes/Red-color-theme.json @@ -68,6 +68,9 @@ "surface.border": "#00000000", // "agentsBottomPanel.border": "#00000000", // "agentsCard.border": "#00000000", + "modernActivityBar.background": "#00000000", + "modernActivityBar.activeBackground": "#580000", + "modernActivityBar.hoverBackground": "#58000087", }, "tokenColors": [ { diff --git a/extensions/theme-solarized-dark/themes/solarized-dark-color-theme.json b/extensions/theme-solarized-dark/themes/solarized-dark-color-theme.json index d64a80c9b60282..ea1cb6b35365ae 100644 --- a/extensions/theme-solarized-dark/themes/solarized-dark-color-theme.json +++ b/extensions/theme-solarized-dark/themes/solarized-dark-color-theme.json @@ -528,6 +528,7 @@ "surface.border": "#00222c", "modernActivityBar.activeBackground": "#005A6F", "modernActivityBar.hoverBackground": "#005A6F87", + "modernActivityBar.background": "#00000000", }, "semanticHighlighting": true } diff --git a/extensions/theme-solarized-light/themes/solarized-light-color-theme.json b/extensions/theme-solarized-light/themes/solarized-light-color-theme.json index e177e14b069fa4..8e052e3c714e79 100644 --- a/extensions/theme-solarized-light/themes/solarized-light-color-theme.json +++ b/extensions/theme-solarized-light/themes/solarized-light-color-theme.json @@ -506,6 +506,7 @@ // modern ui "surface.border": "#ddd6c1", "modernActivityBar.activeBackground": "#DFCA88", + "modernActivityBar.background": "#00000000", }, "semanticHighlighting": true } diff --git a/extensions/theme-tomorrow-night-blue/themes/tomorrow-night-blue-color-theme.json b/extensions/theme-tomorrow-night-blue/themes/tomorrow-night-blue-color-theme.json index eb10ac4714824f..f36e847b45ca66 100644 --- a/extensions/theme-tomorrow-night-blue/themes/tomorrow-night-blue-color-theme.json +++ b/extensions/theme-tomorrow-night-blue/themes/tomorrow-night-blue-color-theme.json @@ -64,6 +64,7 @@ "agentsCard.border": "#00000000", // modern ui "surface.border": "#00000000", + "modernActivityBar.background": "#00000000", }, "tokenColors": [ { diff --git a/package-lock.json b/package-lock.json index 0b3e10047d838a..53691fbc1a7043 100644 --- a/package-lock.json +++ b/package-lock.json @@ -23,7 +23,7 @@ "@microsoft/mxc-sdk": "0.7.0", "@parcel/watcher": "^2.5.6", "@types/semver": "^7.5.8", - "@vscode/codicons": "^0.0.46-36", + "@vscode/codicons": "^0.0.46-37", "@vscode/copilot-api": "^0.5.2", "@vscode/deviceid": "^0.1.1", "@vscode/diff": "0.0.2-7", @@ -4401,9 +4401,9 @@ } }, "node_modules/@vscode/codicons": { - "version": "0.0.46-36", - "resolved": "https://registry.npmjs.org/@vscode/codicons/-/codicons-0.0.46-36.tgz", - "integrity": "sha512-K030Z2AGo4P1gIZfT8X6dNyjficRhA3mDbBfuTObHY4M8+QdyIocc7c2nLfLhZC629oUVVPflJery0CjSfrrUw==", + "version": "0.0.46-37", + "resolved": "https://registry.npmjs.org/@vscode/codicons/-/codicons-0.0.46-37.tgz", + "integrity": "sha512-VOQ7QqRz9N/NmU7nCcwRBkWzjMScQLddRDr1nAXXIebYs4JL6Qez19V+p5q4GyH4FbehGXOSVYIAtG09HAdvuA==", "license": "CC-BY-4.0" }, "node_modules/@vscode/component-explorer": { diff --git a/package.json b/package.json index 3d3716499032fd..88d1d445c6253b 100644 --- a/package.json +++ b/package.json @@ -111,7 +111,7 @@ "@microsoft/mxc-sdk": "0.7.0", "@parcel/watcher": "^2.5.6", "@types/semver": "^7.5.8", - "@vscode/codicons": "^0.0.46-36", + "@vscode/codicons": "^0.0.46-37", "@vscode/copilot-api": "^0.5.2", "@vscode/deviceid": "^0.1.1", "@vscode/diff": "0.0.2-7", diff --git a/remote/web/package-lock.json b/remote/web/package-lock.json index dfc99095482670..becf0ef8430050 100644 --- a/remote/web/package-lock.json +++ b/remote/web/package-lock.json @@ -10,7 +10,7 @@ "dependencies": { "@microsoft/1ds-core-js": "^3.2.13", "@microsoft/1ds-post-js": "^3.2.13", - "@vscode/codicons": "^0.0.46-36", + "@vscode/codicons": "^0.0.46-37", "@vscode/iconv-lite-umd": "0.7.1", "@vscode/tree-sitter-wasm": "^0.3.1", "@vscode/vscode-languagedetection": "1.0.23", @@ -73,9 +73,9 @@ "integrity": "sha512-n1VPsljTSkthsAFYdiWfC+DKzK2WwcRp83Y1YAqdX552BstvsDjft9YXppjUzp11BPsapDoO1LDgrDB0XVsfNQ==" }, "node_modules/@vscode/codicons": { - "version": "0.0.46-36", - "resolved": "https://registry.npmjs.org/@vscode/codicons/-/codicons-0.0.46-36.tgz", - "integrity": "sha512-K030Z2AGo4P1gIZfT8X6dNyjficRhA3mDbBfuTObHY4M8+QdyIocc7c2nLfLhZC629oUVVPflJery0CjSfrrUw==", + "version": "0.0.46-37", + "resolved": "https://registry.npmjs.org/@vscode/codicons/-/codicons-0.0.46-37.tgz", + "integrity": "sha512-VOQ7QqRz9N/NmU7nCcwRBkWzjMScQLddRDr1nAXXIebYs4JL6Qez19V+p5q4GyH4FbehGXOSVYIAtG09HAdvuA==", "license": "CC-BY-4.0" }, "node_modules/@vscode/iconv-lite-umd": { diff --git a/remote/web/package.json b/remote/web/package.json index 999e2729d9df08..7da53e358296b0 100644 --- a/remote/web/package.json +++ b/remote/web/package.json @@ -5,7 +5,7 @@ "dependencies": { "@microsoft/1ds-core-js": "^3.2.13", "@microsoft/1ds-post-js": "^3.2.13", - "@vscode/codicons": "^0.0.46-36", + "@vscode/codicons": "^0.0.46-37", "@vscode/iconv-lite-umd": "0.7.1", "@vscode/tree-sitter-wasm": "^0.3.1", "@vscode/vscode-languagedetection": "1.0.23", diff --git a/src/vs/base/common/codiconsLibrary.ts b/src/vs/base/common/codiconsLibrary.ts index 988f9ce3a08bf4..52aa9eded74824 100644 --- a/src/vs/base/common/codiconsLibrary.ts +++ b/src/vs/base/common/codiconsLibrary.ts @@ -762,4 +762,7 @@ export const codiconsLibrary = { xai: register('xai', 0xecec), arrowCircleUpSparkle: register('arrow-circle-up-sparkle', 0xeced), closeSmall: register('close-small', 0xecee), + bookCompact: register('book-compact', 0xecef), + micOff: register('mic-off', 0xecf0), + micOffCompact: register('mic-off-compact', 0xecf1), } as const; diff --git a/src/vs/platform/agentHost/browser/remoteAgentHostProtocolClient.ts b/src/vs/platform/agentHost/browser/remoteAgentHostProtocolClient.ts index 5841fa4199feb4..996ea883e692e0 100644 --- a/src/vs/platform/agentHost/browser/remoteAgentHostProtocolClient.ts +++ b/src/vs/platform/agentHost/browser/remoteAgentHostProtocolClient.ts @@ -41,7 +41,7 @@ import { ILoadEstimator, LoadEstimator } from '../../../base/parts/ipc/common/ip import { ITelemetryService, TelemetryLevel, TELEMETRY_CRASH_REPORTER_SETTING_ID, TELEMETRY_OLD_SETTING_ID, TELEMETRY_SETTING_ID } from '../../telemetry/common/telemetry.js'; import { getTelemetryLevel } from '../../telemetry/common/telemetryUtils.js'; import { AgentHostAutoApprovePolicyRestrictedConfigKey, AgentHostTelemetryLevelConfigKey, AgentHostTerminalAutoApproveEnabledConfigKey, AgentHostTerminalAutoApproveRulesConfigKey, AgentHostDisableRepoInfoTelemetryConfigKey, getAgentHostTerminalAutoApproveRulesConfig, GLOBAL_AUTO_APPROVE_SETTING_ID, TERMINAL_AUTO_APPROVE_ENABLED_SETTING_ID, TERMINAL_AUTO_APPROVE_SETTING_ID, TERMINAL_IGNORE_DEFAULT_AUTO_APPROVE_RULES_SETTING_ID, DISABLE_REPO_INFO_TELEMETRY_SETTING_ID, telemetryLevelToAgentHostConfigValue } from '../common/agentHostSchema.js'; -import { formatAgentHostConfigurationSyncValueForLog, getAgentHostConfigurationSyncEntries, resolveAgentHostConfigurationSyncPatch, resolveAgentHostConfigurationSyncValue } from '../common/agentHostConfigurationSync.js'; +import { formatAgentHostConfigurationSyncValueForLog, getAgentHostConfigurationSyncEntries, getAgentHostConfigurationSyncTarget, resolveAgentHostConfigurationSyncPatch, resolveAgentHostConfigurationSyncValue } from '../common/agentHostConfigurationSync.js'; import { managedPermissionsConfigurationIds, resolveManagedSettingsPermissions, type IAgentHostManagedSettingsPermissions } from '../common/agentHostManagedSettings.js'; import { AgentHostClientConnectionKind, toAgentHostClientMeta } from '../common/agentHostTelemetry.js'; import type { OtlpExportLogsParams } from '../common/state/protocol/channels-otlp/notifications.js'; @@ -353,7 +353,7 @@ export class RemoteAgentHostProtocolClient extends Disposable implements IAgentC const patch: Record = {}; // These keys are host-level and last-writer-wins across windows. const mirrored: string[] = []; - for (const entry of getAgentHostConfigurationSyncEntries(this._resourceIdentity === LOCAL_AGENT_HOST_RESOURCE_IDENTITY)) { + for (const entry of getAgentHostConfigurationSyncEntries(getAgentHostConfigurationSyncTarget(this._resourceIdentity))) { if (!e.affectsConfiguration(entry.settingId)) { continue; } @@ -818,7 +818,7 @@ export class RemoteAgentHostProtocolClient extends Disposable implements IAgentC * settings contributed by an extension rather than by core. */ private _forwardClientConfig(includeManagedSettings = true): void { - this._dispatchRootConfig(resolveAgentHostConfigurationSyncPatch(this._configurationService, this._resourceIdentity === LOCAL_AGENT_HOST_RESOURCE_IDENTITY)); + this._dispatchRootConfig(resolveAgentHostConfigurationSyncPatch(this._configurationService, getAgentHostConfigurationSyncTarget(this._resourceIdentity))); this._updateTelemetryLevel(); this._updateTerminalAutoApproveEnabled(); this._updateTerminalAutoApproveRules(); diff --git a/src/vs/platform/agentHost/common/agentHostConfigurationSync.ts b/src/vs/platform/agentHost/common/agentHostConfigurationSync.ts index 5519df0b6e735c..c1c3e8f47a661c 100644 --- a/src/vs/platform/agentHost/common/agentHostConfigurationSync.ts +++ b/src/vs/platform/agentHost/common/agentHostConfigurationSync.ts @@ -3,9 +3,12 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ +import { Schemas } from '../../../base/common/network.js'; +import { URI } from '../../../base/common/uri.js'; import { IConfigurationService } from '../../configuration/common/configuration.js'; -import { Extensions as ConfigurationExtensions, IAgentHostConfigurationSync, IConfigurationPropertySchema, IConfigurationRegistry } from '../../configuration/common/configurationRegistry.js'; +import { AgentHostConfigurationSyncScope, Extensions as ConfigurationExtensions, IAgentHostConfigurationSync, IConfigurationPropertySchema, IConfigurationRegistry } from '../../configuration/common/configurationRegistry.js'; import { Registry } from '../../registry/common/platform.js'; +import { AgentHostResourceIdentity, LOCAL_AGENT_HOST_RESOURCE_IDENTITY } from './agentHostResourceService.js'; function getRegistry(): IConfigurationRegistry { return Registry.as(ConfigurationExtensions.Configuration); @@ -100,17 +103,40 @@ export interface IAgentHostConfigurationSyncEntry { readonly sync: IAgentHostConfigurationSync; } +export const enum AgentHostConfigurationSyncTarget { + Local, + RemoteExtensionHost, + Remote, +} + +export function getAgentHostConfigurationSyncTarget(identity: AgentHostResourceIdentity): AgentHostConfigurationSyncTarget { + if (identity === LOCAL_AGENT_HOST_RESOURCE_IDENTITY) { + return AgentHostConfigurationSyncTarget.Local; + } + return URI.parse(identity).scheme === Schemas.vscodeRemote + ? AgentHostConfigurationSyncTarget.RemoteExtensionHost + : AgentHostConfigurationSyncTarget.Remote; +} + +function includesTarget(scope: AgentHostConfigurationSyncScope | undefined, target: AgentHostConfigurationSyncTarget): boolean { + switch (scope ?? AgentHostConfigurationSyncScope.All) { + case AgentHostConfigurationSyncScope.All: + return true; + case AgentHostConfigurationSyncScope.Local: + return target === AgentHostConfigurationSyncTarget.Local; + case AgentHostConfigurationSyncScope.Ambient: + return target === AgentHostConfigurationSyncTarget.Local || target === AgentHostConfigurationSyncTarget.RemoteExtensionHost; + } +} + /** * Returns every setting that declares agent-host mirroring and applies to a host - * of this locality. - * - * @param isLocalAgentHost Whether the target host runs on the user's own - * machine. Entries marked `localOnly` are omitted for remote hosts. + * of this target kind. */ -export function getAgentHostConfigurationSyncEntries(isLocalAgentHost: boolean): IAgentHostConfigurationSyncEntry[] { +export function getAgentHostConfigurationSyncEntries(target: AgentHostConfigurationSyncTarget): IAgentHostConfigurationSyncEntry[] { const entries: IAgentHostConfigurationSyncEntry[] = []; for (const [settingId, sync] of getRegistry().getAgentHostSyncConfigurations()) { - if (sync.localOnly && !isLocalAgentHost) { + if (!includesTarget(sync.scope, target)) { continue; } entries.push({ settingId, sync }); @@ -150,9 +176,9 @@ export function formatAgentHostConfigurationSyncValueForLog(settingId: string, v * connect and reconnect, where the host may be a freshly restarted process that * has none of these values. */ -export function resolveAgentHostConfigurationSyncPatch(configurationService: IConfigurationService, isLocalAgentHost: boolean): Record { +export function resolveAgentHostConfigurationSyncPatch(configurationService: IConfigurationService, target: AgentHostConfigurationSyncTarget): Record { const patch: Record = {}; - for (const entry of getAgentHostConfigurationSyncEntries(isLocalAgentHost)) { + for (const entry of getAgentHostConfigurationSyncEntries(target)) { const value = resolveAgentHostConfigurationSyncValue(configurationService, entry); // A setting with no value in any global layer and no registered default has // nothing to mirror; leave the key absent so a previously stored host value diff --git a/src/vs/platform/agentHost/common/agentHostSchema.ts b/src/vs/platform/agentHost/common/agentHostSchema.ts index 3b80a66b77937d..0d69985c8e5e7e 100644 --- a/src/vs/platform/agentHost/common/agentHostSchema.ts +++ b/src/vs/platform/agentHost/common/agentHostSchema.ts @@ -460,6 +460,38 @@ export const AgentHostAutoReplyAnswer = 'The user is not available to answer you /** Root config key forwarded from the renderer for automatic OS system proxy discovery. */ export const AgentHostSystemProxyEnabledConfigKey = 'systemProxyEnabled'; +/** + * Independently synchronized proxy settings retain their VS Code `http.*` + * names, matching other flat namespaced root keys such as `agentMerge.*`. + */ +export const AgentHostProxyConfigKey = { + Proxy: 'http.proxy', + ProxyKerberosServicePrincipal: 'http.proxyKerberosServicePrincipal', + NoProxy: 'http.noProxy', +} as const; + +const agentHostProxyConfigDefinition = { + [AgentHostProxyConfigKey.Proxy]: schemaProperty({ + type: 'string', + title: localize('agentHost.config.httpProxy.title', "HTTP Proxy"), + description: localize('agentHost.config.httpProxy.description', "The proxy URL used by network requests from the Agent Host."), + }), + [AgentHostProxyConfigKey.ProxyKerberosServicePrincipal]: schemaProperty({ + type: 'string', + title: localize('agentHost.config.httpProxyKerberosServicePrincipal.title', "HTTP Proxy Kerberos Service Principal"), + description: localize('agentHost.config.httpProxyKerberosServicePrincipal.description', "The Kerberos service principal used to authenticate with the HTTP proxy."), + }), + [AgentHostProxyConfigKey.NoProxy]: schemaProperty({ + type: 'array', + title: localize('agentHost.config.httpNoProxy.title', "HTTP No Proxy"), + description: localize('agentHost.config.httpNoProxy.description', "Domain names that bypass the configured HTTP proxy."), + items: { type: 'string', title: localize('agentHost.config.httpNoProxy.item.title', "Domain") }, + default: [], + }), +}; + +export const agentHostProxyConfigSchema = createSchema(agentHostProxyConfigDefinition); + /** Root config key forwarded from the renderer for active-agent title generation. */ export const AgentHostActiveAgentTitleGenerationConfigKey = 'activeAgentTitleGeneration'; @@ -678,6 +710,7 @@ const mcpServersValueProperties: Record = { }; export const platformRootSchema = createSchema({ + ...agentHostProxyConfigDefinition, [SessionConfigKey.Permissions]: permissionsProperty, [AgentHostDisableRepoInfoTelemetryConfigKey]: schemaProperty({ type: 'boolean', diff --git a/src/vs/platform/agentHost/common/agentHostStarter.config.contribution.ts b/src/vs/platform/agentHost/common/agentHostStarter.config.contribution.ts index ea4342d284cd0d..d97f41265134f2 100644 --- a/src/vs/platform/agentHost/common/agentHostStarter.config.contribution.ts +++ b/src/vs/platform/agentHost/common/agentHostStarter.config.contribution.ts @@ -6,7 +6,7 @@ import * as nls from '../../../nls.js'; import { IPolicyData } from '../../../base/common/defaultAccount.js'; import { PolicyCategory } from '../../../base/common/policy.js'; -import { ConfigurationScope, Extensions as ConfigurationExtensions, IConfigurationPropertySchema, IConfigurationRegistry } from '../../configuration/common/configurationRegistry.js'; +import { AgentHostConfigurationSyncScope, ConfigurationScope, Extensions as ConfigurationExtensions, IConfigurationPropertySchema, IConfigurationRegistry } from '../../configuration/common/configurationRegistry.js'; import { COPILOT_OTEL_CAPTURE_CONTENT_KEY, COPILOT_OTEL_ENABLED_KEY, COPILOT_OTEL_ENDPOINT_KEY, COPILOT_OTEL_HEADERS_KEY, COPILOT_OTEL_LOCK_CAPTURE_CONTENT_KEY, COPILOT_OTEL_PROTOCOL_KEY, COPILOT_OTEL_RESOURCE_ATTRIBUTES_KEY, COPILOT_OTEL_SERVICE_NAME_KEY, managedSettingValue } from '../../policy/common/copilotManagedSettings.js'; import product from '../../product/common/product.js'; import { Registry } from '../../registry/common/platform.js'; @@ -248,7 +248,7 @@ configurationRegistry.registerConfiguration({ default: false, tags: ['experimental', 'advanced'], experiment: { mode: 'startup' }, - agentHost: { key: AgentHostByokModelsEnabledConfigKey, localOnly: true }, + agentHost: { key: AgentHostByokModelsEnabledConfigKey, scope: AgentHostConfigurationSyncScope.Local }, }, [AgentHostCodexAgentEnabledSettingId]: { type: 'boolean', diff --git a/src/vs/platform/agentHost/node/agentConfigurationService.ts b/src/vs/platform/agentHost/node/agentConfigurationService.ts index 1ecc2ba1b027a7..291cbd7a775765 100644 --- a/src/vs/platform/agentHost/node/agentConfigurationService.ts +++ b/src/vs/platform/agentHost/node/agentConfigurationService.ts @@ -16,7 +16,7 @@ import { getAgentCustomizationSettingsEntries, getProviderBackedRootConfigKeys, import { copilotCliConfigSchema } from '../common/copilotCliConfig.js'; import { agentMergeRootConfigSchema } from '../common/agentMerge.js'; import { sandboxConfigSchema } from '../common/sandboxConfigSchema.js'; -import type { ISchema, SchemaDefinition, SchemaValue } from '../common/agentHostSchema.js'; +import { agentHostProxyConfigSchema, type ISchema, type SchemaDefinition, type SchemaValue } from '../common/agentHostSchema.js'; import { ProtocolError } from '../common/state/sessionProtocol.js'; import { ActionType, type ActionOrigin } from '../common/state/sessionActions.js'; import { isAhpChatChannel, parseSubagentSessionUri, ROOT_STATE_URI, type URI as ProtocolURI } from '../common/state/sessionState.js'; @@ -411,6 +411,7 @@ export class AgentConfigurationService extends Disposable implements IAgentConfi ...sandboxConfigSchema.validateOrDefault(parsed, {}), ...copilotCliConfigSchema.validateOrDefault(parsed, {}), ...agentMergeRootConfigSchema.validateOrDefault(parsed, {}), + ...agentHostProxyConfigSchema.validateOrDefault(parsed, {}), }; } catch (err) { const code = err && typeof err === 'object' && hasKey(err, { code: true }) ? String(err.code) : undefined; diff --git a/src/vs/platform/agentHost/node/agentHostBootstrap.ts b/src/vs/platform/agentHost/node/agentHostBootstrap.ts index 3baef2a365984c..2e76e8adcc16c0 100644 --- a/src/vs/platform/agentHost/node/agentHostBootstrap.ts +++ b/src/vs/platform/agentHost/node/agentHostBootstrap.ts @@ -4,14 +4,8 @@ *--------------------------------------------------------------------------------------------*/ import { DisposableStore } from '../../../base/common/lifecycle.js'; -import { joinPath } from '../../../base/common/resources.js'; -import { IConfigurationService } from '../../configuration/common/configuration.js'; -import { ConfigurationService } from '../../configuration/common/configurationService.js'; -import { INativeEnvironmentService } from '../../environment/common/environment.js'; -import { IFileService } from '../../files/common/files.js'; import { ServiceCollection } from '../../instantiation/common/serviceCollection.js'; import { ILogService } from '../../log/common/log.js'; -import { IPolicyService, NullPolicyService } from '../../policy/common/policy.js'; import { IRequestService } from '../../request/common/request.js'; import { AgentHostProxyResolver, IAgentHostProxyResolver } from './agentHostProxyResolver.js'; import { AgentHostRequestService } from './agentHostRequestService.js'; @@ -22,41 +16,25 @@ export interface IAgentHostNetworkServices { } /** - * Register `IPolicyService`, `IConfigurationService`, `IAgentHostProxyResolver`, - * and `IRequestService` into the agent host's DI container — the services that - * `IAgentSdkDownloader` (and proxy-aware network diagnostics) depend on. + * Register `IAgentHostProxyResolver` and `IRequestService` into the agent host's + * DI container — the services that `IAgentSdkDownloader` (and proxy-aware + * network diagnostics) depend on. * * Used by both entry points (`agentHostMain.ts` and `agentHostServerMain.ts`) * to avoid drift between them. The order of registration matters because - * `RequestService` injects `IConfigurationService`; consumers (the downloader - * itself, and through it `ClaudeAgentSdkService` / `CodexAgent`) must be - * constructed AFTER this call. - * - * Reads the default profile's `settings.json` from `` — - * the same file the workbench writes user settings to. Initialization is - * async because the settings file is read off disk. - * - * `NullPolicyService` matches the pattern used by sibling node-side processes - * (server, CLI). Enterprise policy enforcement happens in the main process and - * lands in `settings.json`; we don't re-resolve it here. `RequestService` runs - * in `'local'` mode because the agent host runs on the user's machine. + * Consumers (the downloader itself, and through it `ClaudeAgentSdkService` / + * `CodexAgent`) must be constructed AFTER this call. The resolver is bound to + * `IAgentConfigurationService` after `AgentService` creates the host-owned + * configuration service. */ -export async function registerAgentHostNetworkServices( +export function registerAgentHostNetworkServices( diServices: ServiceCollection, - fileService: IFileService, - environmentService: INativeEnvironmentService, logService: ILogService, disposables: DisposableStore, -): Promise { - const policyService = new NullPolicyService(); - diServices.set(IPolicyService, policyService); - const settingsResource = joinPath(environmentService.appSettingsHome, 'settings.json'); - const configurationService = disposables.add(new ConfigurationService(settingsResource, fileService, policyService, logService)); - await configurationService.initialize(); - diServices.set(IConfigurationService, configurationService); - const proxyResolver = disposables.add(new AgentHostProxyResolver(configurationService, logService)); +): IAgentHostNetworkServices { + const proxyResolver = disposables.add(new AgentHostProxyResolver(logService)); diServices.set(IAgentHostProxyResolver, proxyResolver); - const requestService = disposables.add(new AgentHostRequestService(configurationService, environmentService, logService, proxyResolver)); + const requestService = disposables.add(new AgentHostRequestService(logService, proxyResolver)); diServices.set(IRequestService, requestService); return { proxyResolver, requestService }; } diff --git a/src/vs/platform/agentHost/node/agentHostMain.ts b/src/vs/platform/agentHost/node/agentHostMain.ts index ff51ce682eb1b4..48672a1123dcb7 100644 --- a/src/vs/platform/agentHost/node/agentHostMain.ts +++ b/src/vs/platform/agentHost/node/agentHostMain.ts @@ -176,7 +176,7 @@ async function startAgentHost(): Promise { diServices.set(IFileService, fileService); diServices.set(ISessionDataService, sessionDataService); diServices.set(IProductService, productService); - const networkServices = await registerAgentHostNetworkServices(diServices, fileService, environmentService, logService, disposables); + const networkServices = registerAgentHostNetworkServices(diServices, logService, disposables); proxyResolver = networkServices.proxyResolver; const fetchFn = proxyResolver.fetch.bind(proxyResolver); const telemetryService = await createAgentHostTelemetryService({ environmentService, productService, fileService, loggerService, logService, disposables, fetchFn, requestService: networkServices.requestService }); @@ -209,14 +209,16 @@ async function startAgentHost(): Promise { logsHome: environmentService.logsHome, tmpDir: environmentService.tmpDir, }); - const networkDiagnosticsService = instantiationService.createInstance(NetworkDiagnosticsService); - diServices.set(INetworkDiagnosticsService, networkDiagnosticsService); - agentService.setNetworkDiagnosticsService(networkDiagnosticsService); diServices.set(IAgentService, agentService); diServices.set(IAgentHostStateManager, agentService.stateManager); // Narrow host seams providers consume instead of the whole state manager. diServices.set(IAgentHostPromptCache, agentService.promptCache); diServices.set(IAgentHostSessionTitleSignal, agentService.sessionTitleSignal); + diServices.set(IAgentConfigurationService, agentService.configurationService); + proxyResolver.bindConfigurationService(agentService.configurationService, true); + const networkDiagnosticsService = instantiationService.createInstance(NetworkDiagnosticsService); + diServices.set(INetworkDiagnosticsService, networkDiagnosticsService); + agentService.setNetworkDiagnosticsService(networkDiagnosticsService); const pluginManager = new AgentPluginManager(URI.file(environmentService.userDataPath), fileService, logService); diServices.set(IAgentPluginManager, pluginManager); const diffComputeService = disposables.add(new NodeWorkerDiffComputeService(logService)); @@ -227,7 +229,6 @@ async function startAgentHost(): Promise { diServices.set(IEditSurvivalReporterFactory, instantiationService.createInstance(EditSurvivalReporterFactory)); diServices.set(IAgentHostTerminalManager, agentService.terminalManager); - diServices.set(IAgentConfigurationService, agentService.configurationService); diServices.set(IAgentHostStorageService, agentService.storageService); diServices.set(IAgentHostCustomizationEnablementService, agentService.customizationEnablementService); diServices.set(IAgentHostManagedSettingsService, agentService.managedSettingsService); diff --git a/src/vs/platform/agentHost/node/agentHostProxyResolver.ts b/src/vs/platform/agentHost/node/agentHostProxyResolver.ts index 127948f1117397..c0607fec515b0e 100644 --- a/src/vs/platform/agentHost/node/agentHostProxyResolver.ts +++ b/src/vs/platform/agentHost/node/agentHostProxyResolver.ts @@ -3,17 +3,22 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import { LogLevel as ProxyLogLevel, ProxyAgentParams, ProxySupportSetting, createFetchPatch, createProxyAuthorizationLookup, createProxyResolver, loadSystemCertificates } from '@vscode/proxy-agent'; +import { LogLevel as ProxyLogLevel, ProxyAgentParams, createFetchPatch, createProxyAuthorizationLookup, createProxyResolver, loadSystemCertificates } from '@vscode/proxy-agent'; import { Emitter, Event } from '../../../base/common/event.js'; -import { Disposable, IDisposable, toDisposable } from '../../../base/common/lifecycle.js'; -import { IConfigurationService } from '../../configuration/common/configuration.js'; +import { Disposable, IDisposable, MutableDisposable, toDisposable } from '../../../base/common/lifecycle.js'; +import { equals } from '../../../base/common/objects.js'; import { createDecorator } from '../../instantiation/common/instantiation.js'; import { ILogService, LogLevel } from '../../log/common/log.js'; import { AuthInfo, Credentials, systemCertificatesNodeDefault } from '../../request/common/request.js'; +import { lookupKerberosAuthorization } from '../../request/node/requestService.js'; import { IAgentHostClientProxyConnection } from '../common/agentHostClientProxyChannel.js'; +import { AgentHostProxyConfigKey, agentHostProxyConfigSchema } from '../common/agentHostSchema.js'; +import { IAgentConfigurationService } from './agentConfigurationService.js'; export const IAgentHostProxyResolver = createDecorator('agentHostProxyResolver'); +type AgentHostProxyConfigurationKey = keyof typeof agentHostProxyConfigSchema.definition & string; + /** * Node-side registry of renderer {@link IAgentHostClientProxyConnection}s keyed * by client id. Populated by the agent host's connection lifecycle (one entry @@ -28,10 +33,19 @@ export interface IAgentHostProxyResolver { readonly _serviceBrand: undefined; readonly onDidRegisterConnection: Event; + readonly onDidChangeConfiguration: Event; /** Register a renderer connection. Disposing the result removes it. */ register(clientId: string, connection: IAgentHostClientProxyConnection): IDisposable; + /** + * Binds the Agent Host configuration after the orchestrator has initialized it. + * Local hosts mark mirrored values transient; remote hosts persist manual values. + */ + bindConfigurationService(configurationService: IAgentConfigurationService, transient: boolean): void; + + getConfigurationValue(key: AgentHostProxyConfigurationKey): T | undefined; + /** * Resolve the proxy URL for `url` (e.g. `http://host:port`), or `undefined` * for a direct connection. Reuses `@vscode/proxy-agent`'s `resolveProxyURL` @@ -51,19 +65,49 @@ export class AgentHostProxyResolver extends Disposable implements IAgentHostProx private readonly _onDidRegisterConnection = this._register(new Emitter()); readonly onDidRegisterConnection = this._onDidRegisterConnection.event; + private readonly _onDidChangeConfiguration = this._register(new Emitter()); + readonly onDidChangeConfiguration = this._onDidChangeConfiguration.event; + private readonly _configurationListener = this._register(new MutableDisposable()); private readonly _connections = new Map(); + private _configurationService: IAgentConfigurationService | undefined; + private _configurationValues: Record = {}; private _proxyResolver: ReturnType | undefined; private _proxyAgentParams: ProxyAgentParams | undefined; private _fetch: typeof globalThis.fetch | undefined; - constructor( - @IConfigurationService private readonly _configurationService: IConfigurationService, - @ILogService private readonly _logService: ILogService, - ) { + constructor(@ILogService private readonly _logService: ILogService) { super(); } + bindConfigurationService(configurationService: IAgentConfigurationService, transient: boolean): void { + this._configurationService = configurationService; + if (transient) { + configurationService.publishRootTransientValues?.(Object.fromEntries( + Object.values(AgentHostProxyConfigKey).map(key => [key, undefined]) + )); + } + this._configurationValues = this._readConfigurationValues(); + this._configurationListener.value = configurationService.onDidRootConfigChange(() => { + const values = this._readConfigurationValues(); + if (!equals(this._configurationValues, values)) { + this._configurationValues = values; + this._onDidChangeConfiguration.fire(); + } + }); + } + + getConfigurationValue(key: AgentHostProxyConfigurationKey): T | undefined { + if (!this._configurationService) { + return undefined; + } + return this._configurationService.getRootValue(agentHostProxyConfigSchema, key) as T | undefined; + } + + private _readConfigurationValues(): Record { + return Object.fromEntries(Object.values(AgentHostProxyConfigKey).map(key => [key, this.getConfigurationValue(key)])); + } + register(clientId: string, connection: IAgentHostClientProxyConnection): IDisposable { const hadConnections = this._connections.size > 0; this._connections.set(clientId, connection); @@ -92,9 +136,6 @@ export class AgentHostProxyResolver extends Disposable implements IAgentHostProx private _getProxyResolver(): ReturnType { if (!this._proxyResolver) { // Mirror `workbench/api/node/proxyResolver.ts`. - const config = (key: string): T | undefined => this._configurationService.getValue(key); - const systemCertificatesV2 = () => config('http.experimental.systemCertificatesV2') ?? false; - const systemCertificates = () => !!config('http.systemCertificates'); const params: ProxyAgentParams = { // The host proxy resolution runs in VS Code: reverse-call a connected // renderer, whose IRequestService.resolveProxy hits the Electron @@ -105,16 +146,16 @@ export class AgentHostProxyResolver extends Disposable implements IAgentHostProx lookupAuthorization: authInfo => this._hostLookupAuthorization(authInfo), lookupKerberosAuthorization: url => this._hostLookupKerberosAuthorization(url), }), - getProxyURL: () => config('http.proxy'), - getProxySupport: () => config('http.proxySupport') || 'off', - getNoProxyConfig: () => config('http.noProxy') || [], - isAdditionalFetchSupportEnabled: () => config('http.fetchAdditionalSupport') ?? true, - isWebSocketPatchEnabled: () => config('http.webSocketAdditionalSupport') ?? true, - addCertificatesV1: () => !systemCertificatesV2() && systemCertificates(), - addCertificatesV2: () => systemCertificatesV2() && systemCertificates(), - loadSystemCertificatesFromNode: () => config('http.systemCertificatesNode') ?? systemCertificatesNodeDefault, + getProxyURL: () => this.getConfigurationValue(AgentHostProxyConfigKey.Proxy), + getProxySupport: () => 'override', + getNoProxyConfig: () => this.getConfigurationValue(AgentHostProxyConfigKey.NoProxy) || [], + isAdditionalFetchSupportEnabled: () => true, + isWebSocketPatchEnabled: () => true, + addCertificatesV1: () => true, + addCertificatesV2: () => false, + loadSystemCertificatesFromNode: () => systemCertificatesNodeDefault, loadAdditionalCertificates: async () => loadSystemCertificates({ - loadSystemCertificatesFromNode: () => config('http.systemCertificatesNode') ?? systemCertificatesNodeDefault, + loadSystemCertificatesFromNode: () => systemCertificatesNodeDefault, log: this._logService, }), log: this._logService, @@ -135,7 +176,7 @@ export class AgentHostProxyResolver extends Disposable implements IAgentHostProx // when the agent host is local (i.e., on the same machine as // the client). isUseHostProxyEnabled: () => this._connections.size > 0, - getNetworkInterfaceCheckInterval: () => (config('http.experimental.networkInterfaceCheckInterval') ?? 300) * 1000, + getNetworkInterfaceCheckInterval: () => 300 * 1000, env: process.env, }; this._proxyAgentParams = params; @@ -174,6 +215,16 @@ export class AgentHostProxyResolver extends Disposable implements IAgentHostProx // This renderer could not serve the lookup; try the next one. } } - return undefined; + try { + const spn = this.getConfigurationValue(AgentHostProxyConfigKey.ProxyKerberosServicePrincipal); + return `Negotiate ${await this._lookupKerberosAuthorization(url, spn)}`; + } catch (error) { + this._logService.debug('AgentHostProxyResolver#lookupKerberosAuthorization Kerberos authentication failed', error); + return undefined; + } + } + + protected _lookupKerberosAuthorization(url: string, spn: string | undefined): Promise { + return lookupKerberosAuthorization(url, spn, this._logService, 'AgentHostProxyResolver#lookupKerberosAuthorization'); } } diff --git a/src/vs/platform/agentHost/node/agentHostRequestService.ts b/src/vs/platform/agentHost/node/agentHostRequestService.ts index 6843c0597fabad..4e51d42a7ea9bd 100644 --- a/src/vs/platform/agentHost/node/agentHostRequestService.ts +++ b/src/vs/platform/agentHost/node/agentHostRequestService.ts @@ -3,16 +3,17 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ +import { loadSystemCertificates } from '@vscode/proxy-agent'; import { newWriteableBufferStream, VSBuffer, VSBufferWriteableStream } from '../../../base/common/buffer.js'; import { timeout } from '../../../base/common/async.js'; import { CancellationToken } from '../../../base/common/cancellation.js'; import { CancellationError, isCancellationError } from '../../../base/common/errors.js'; import { IDisposable } from '../../../base/common/lifecycle.js'; import { IHeaders, IRequestContext, IRequestOptions } from '../../../base/parts/request/common/request.js'; -import { IConfigurationService } from '../../configuration/common/configuration.js'; -import { INativeEnvironmentService } from '../../environment/common/environment.js'; import { ILogService } from '../../log/common/log.js'; -import { RequestService } from '../../request/node/requestService.js'; +import { AbstractRequestService, AuthInfo, Credentials, systemCertificatesNodeDefault } from '../../request/common/request.js'; +import { lookupKerberosAuthorization } from '../../request/node/requestService.js'; +import { AgentHostProxyConfigKey } from '../common/agentHostSchema.js'; import { IAgentHostProxyResolver } from './agentHostProxyResolver.js'; const TRANSIENT_ERROR_CODES = new Set([ @@ -41,15 +42,15 @@ function isTransientError(error: unknown): boolean { * certificate settings. The base {@link RequestService} remains unchanged for * all other Node consumers. */ -export class AgentHostRequestService extends RequestService { +export class AgentHostRequestService extends AbstractRequestService { + + declare readonly _serviceBrand: undefined; constructor( - @IConfigurationService configurationService: IConfigurationService, - @INativeEnvironmentService environmentService: INativeEnvironmentService, @ILogService logService: ILogService, @IAgentHostProxyResolver private readonly _proxyResolver: IAgentHostProxyResolver, ) { - super('local', configurationService, environmentService, logService); + super(logService); } override request(options: IRequestOptions, token: CancellationToken): Promise { @@ -60,6 +61,27 @@ export class AgentHostRequestService extends RequestService { return this._proxyResolver.resolveProxy(url); } + override async lookupAuthorization(_authInfo: AuthInfo): Promise { + return undefined; + } + + override async lookupKerberosAuthorization(url: string): Promise { + try { + const spn = this._proxyResolver.getConfigurationValue(AgentHostProxyConfigKey.ProxyKerberosServicePrincipal); + return `Negotiate ${await lookupKerberosAuthorization(url, spn, this.logService, 'AgentHostRequestService#lookupKerberosAuthorization')}`; + } catch (error) { + this.logService.debug('AgentHostRequestService#lookupKerberosAuthorization Kerberos authentication failed', error); + return undefined; + } + } + + override loadCertificates(): Promise { + return loadSystemCertificates({ + loadSystemCertificatesFromNode: () => systemCertificatesNodeDefault, + log: this.logService, + }); + } + private async _request(options: IRequestOptions, token: CancellationToken): Promise { const maxRetries = 3; let lastError: Error | undefined; diff --git a/src/vs/platform/agentHost/node/agentHostServerMain.ts b/src/vs/platform/agentHost/node/agentHostServerMain.ts index 9f03ee18a6c6fe..72572d78b59a4a 100644 --- a/src/vs/platform/agentHost/node/agentHostServerMain.ts +++ b/src/vs/platform/agentHost/node/agentHostServerMain.ts @@ -249,7 +249,7 @@ async function main(): Promise { diServices.set(ILogService, logService); diServices.set(IFileService, fileService); diServices.set(ISessionDataService, sessionDataService); - const networkServices = await registerAgentHostNetworkServices(diServices, fileService, environmentService, logService, disposables); + const networkServices = registerAgentHostNetworkServices(diServices, logService, disposables); const proxyResolver = networkServices.proxyResolver; const fetchFn = proxyResolver.fetch.bind(proxyResolver); const telemetryService = await createAgentHostTelemetryService({ environmentService, productService, fileService, loggerService, logService, disposables, disableTelemetry: options.quiet, fetchFn, requestService: networkServices.requestService }); @@ -275,6 +275,8 @@ async function main(): Promise { diServices.set(IAgentHostPromptCache, agentService.promptCache); diServices.set(IAgentHostSessionTitleSignal, agentService.sessionTitleSignal); diServices.set(IAgentHostManagedSettingsService, agentService.managedSettingsService); + diServices.set(IAgentConfigurationService, agentService.configurationService); + proxyResolver.bindConfigurationService(agentService.configurationService, false); const networkDiagnosticsService = instantiationService.createInstance(NetworkDiagnosticsService); diServices.set(INetworkDiagnosticsService, networkDiagnosticsService); agentService.setNetworkDiagnosticsService(networkDiagnosticsService); @@ -294,7 +296,6 @@ async function main(): Promise { agentService.setEditAttributionService(editAttributionService); diServices.set(IEditSurvivalReporterFactory, instantiationService.createInstance(EditSurvivalReporterFactory)); diServices.set(IAgentHostTerminalManager, agentService.terminalManager); - diServices.set(IAgentConfigurationService, agentService.configurationService); const editArcReporterService = disposables.add(instantiationService.createInstance(EditArcReporterService, undefined)); diServices.set(IEditArcReporterService, editArcReporterService); diServices.set(IAgentHostCompletions, agentService.completionsService); diff --git a/src/vs/platform/agentHost/node/copilot/copilotAgent.ts b/src/vs/platform/agentHost/node/copilot/copilotAgent.ts index 555abb2acabf12..6c2725d39c5ed9 100644 --- a/src/vs/platform/agentHost/node/copilot/copilotAgent.ts +++ b/src/vs/platform/agentHost/node/copilot/copilotAgent.ts @@ -40,7 +40,7 @@ import { createPricingMetaFromBilling, hasLongContextSurcharge, normalizeCAPIBil import { createAgentModelByokMeta } from '../../common/agentModelByokMeta.js'; import { AgentHostConfigKey, agentHostCustomizationConfigSchema, DEFAULT_SESSION_CUSTOMIZATION_DISCOVERY_MODE, toContainerCustomization } from '../../common/agentHostCustomizationConfig.js'; import { CopilotCliConfigKey, CopilotCliVSCodeAssignmentContextKey, copilotCliConfigSchema, DEFAULT_COPILOT_RUBBER_DUCK_ENABLED, type CopilotSdkLogLevelSetting } from '../../common/copilotCliConfig.js'; -import { AgentHostAutoApprovePolicyRestrictedConfigKey, AgentHostByokModelsEnabledConfigKey, AgentHostMcpServersConfigKey, AgentHostCopilotMultiRootEnabledConfigKey, AgentHostSessionSyncEnabledConfigKey, AgentHostSystemProxyEnabledConfigKey, AgentHostMigrateLegacyCopilotCliEnabledConfigKey, AutoApproveLevel, SessionMode, migrateLegacyAutopilotConfig, platformRootSchema, platformSessionSchema, type AgentHostMcpServers } from '../../common/agentHostSchema.js'; +import { AgentHostAutoApprovePolicyRestrictedConfigKey, AgentHostByokModelsEnabledConfigKey, AgentHostMcpServersConfigKey, AgentHostCopilotMultiRootEnabledConfigKey, AgentHostSessionSyncEnabledConfigKey, AgentHostSystemProxyEnabledConfigKey, AgentHostMigrateLegacyCopilotCliEnabledConfigKey, AgentHostProxyConfigKey, agentHostProxyConfigSchema, AutoApproveLevel, SessionMode, migrateLegacyAutopilotConfig, platformRootSchema, platformSessionSchema, type AgentHostMcpServers } from '../../common/agentHostSchema.js'; import { IAgentPluginManager, ISyncedCustomization } from '../../common/agentPluginManager.js'; import { decodeProviderData, encodeProviderData, type IPersistedChat } from '../agentChatBackings.js'; import { prepareSideChatPrompt, sliceSideChatTurns } from '../agentPeerChats.js'; @@ -671,6 +671,7 @@ export class CopilotAgent extends Disposable implements IAgent { private _proxyRefresh: Promise | undefined; private _proxyResolutionGeneration = 0; private _appliedProxy: string | undefined; + private _appliedProxyKerberosSpn: string | undefined; /** * Reasons for a client restart that is parked until every chat is idle. See * {@link _requestClientRestart}; drained by {@link _applyPendingClientRestart}. @@ -798,6 +799,7 @@ export class CopilotAgent extends Disposable implements IAgent { ? new AgentHostGitHubTelemetryRouter(this._telemetryService) : undefined; this._register(this._proxyResolver.onDidRegisterConnection(() => this._refreshProxy())); + this._register(this._proxyResolver.onDidChangeConfiguration(() => this._refreshProxy())); this.onDidCustomizationsChange = this._plugins.onDidChange; // Mirror host-owned titles under the SDK conversation id used by the agent's turn spans. this._register(sessionTitleSignal.onDidChangeSessionTitle(({ provider, session, title }) => { @@ -4309,6 +4311,11 @@ export class CopilotAgent extends Disposable implements IAgent { } this._logService.info('[Copilot] Resolved CAPI proxy and forwarded HTTP_PROXY/HTTPS_PROXY to Copilot SDK'); } + const kerberosSpn = env['COPILOT_PROXY_KERBEROS_SPN'] || this._configurationService.getRootValue(agentHostProxyConfigSchema, AgentHostProxyConfigKey.ProxyKerberosServicePrincipal); + this._appliedProxyKerberosSpn = kerberosSpn; + if (kerberosSpn && !env['COPILOT_PROXY_KERBEROS_SPN']) { + env['COPILOT_PROXY_KERBEROS_SPN'] = kerberosSpn; + } } private async _resolveProxyForSdk(env: Record = process.env): Promise { @@ -4348,7 +4355,8 @@ export class CopilotAgent extends Disposable implements IAgent { } this._resolvedProxy = proxy; const effectiveProxy = this._isSystemProxyEnabled() ? proxy : undefined; - if (effectiveProxy === this._appliedProxy) { + const effectiveKerberosSpn = process.env['COPILOT_PROXY_KERBEROS_SPN'] || this._configurationService.getRootValue(agentHostProxyConfigSchema, AgentHostProxyConfigKey.ProxyKerberosServicePrincipal); + if (effectiveProxy === this._appliedProxy && effectiveKerberosSpn === this._appliedProxyKerberosSpn) { return; } if (this._clientStarting) { @@ -4360,11 +4368,11 @@ export class CopilotAgent extends Disposable implements IAgent { // A newer proxy resolution (or the client start we just awaited) // may have already superseded this one; re-check both so we don't // restart based on a stale comparison. - if (generation !== this._proxyResolutionGeneration || effectiveProxy === this._appliedProxy) { + if (generation !== this._proxyResolutionGeneration || (effectiveProxy === this._appliedProxy && effectiveKerberosSpn === this._appliedProxyKerberosSpn)) { return; } } - await this._requestClientRestart(`CAPI proxy changed (${this._appliedProxy ?? '(none)'} -> ${effectiveProxy ?? '(none)'})`); + await this._requestClientRestart(`CAPI proxy configuration changed (${this._appliedProxy ?? '(none)'} -> ${effectiveProxy ?? '(none)'})`); }).catch(error => this._logService.error('[Copilot] Failed to refresh CAPI proxy', error)); this._proxyRefresh = refresh; void refresh.finally(() => { diff --git a/src/vs/platform/agentHost/node/networkDiagnosticsService.ts b/src/vs/platform/agentHost/node/networkDiagnosticsService.ts index 63d4e396bbec60..007829bc4ddac5 100644 --- a/src/vs/platform/agentHost/node/networkDiagnosticsService.ts +++ b/src/vs/platform/agentHost/node/networkDiagnosticsService.ts @@ -6,13 +6,14 @@ import { lookup } from 'dns'; import { streamToBuffer } from '../../../base/common/buffer.js'; import { CancellationToken } from '../../../base/common/cancellation.js'; -import { IConfigurationService } from '../../configuration/common/configuration.js'; import { createDecorator } from '../../instantiation/common/instantiation.js'; import { ILogService } from '../../log/common/log.js'; import { IProductService } from '../../product/common/productService.js'; import { IRequestService, NO_FETCH_TELEMETRY } from '../../request/common/request.js'; import { IAgentHostNetworkEndpoint } from '../common/agent.js'; import { IAgentHostDnsResult, IAgentHostNetworkDiagnosticsInfo, IAgentHostNetworkFetchResult } from '../common/agentService.js'; +import { AgentHostProxyConfigKey, agentHostProxyConfigSchema } from '../common/agentHostSchema.js'; +import { IAgentConfigurationService } from './agentConfigurationService.js'; import { IAgentHostProxyResolver } from './agentHostProxyResolver.js'; export const INetworkDiagnosticsService = createDecorator('networkDiagnosticsService'); @@ -44,10 +45,10 @@ const MAX_BODY_CHARS = 64 * 1024; * Proxy-related environment variables surfaced in the diagnostics report so a * mismatch between the OS/config proxy and an explicit env override is visible. */ -const PROXY_ENV_KEYS = ['HTTPS_PROXY', 'https_proxy', 'HTTP_PROXY', 'http_proxy', 'ALL_PROXY', 'all_proxy', 'NO_PROXY', 'no_proxy'] as const; +const PROXY_ENV_KEYS = ['HTTPS_PROXY', 'https_proxy', 'HTTP_PROXY', 'http_proxy', 'ALL_PROXY', 'all_proxy', 'NO_PROXY', 'no_proxy', 'COPILOT_PROXY_KERBEROS_SPN'] as const; -/** VS Code `http.*` proxy settings surfaced alongside the env vars. */ -const PROXY_CONFIG_KEYS = ['http.proxy', 'http.proxyStrictSSL', 'http.proxySupport', 'http.noProxy'] as const; +/** Agent Host `http.*` proxy settings surfaced alongside the env vars. */ +const PROXY_CONFIG_KEYS = [AgentHostProxyConfigKey.Proxy, AgentHostProxyConfigKey.NoProxy, AgentHostProxyConfigKey.ProxyKerberosServicePrincipal] as const; export class NetworkDiagnosticsService implements INetworkDiagnosticsService { @@ -56,7 +57,7 @@ export class NetworkDiagnosticsService implements INetworkDiagnosticsService { constructor( @IRequestService private readonly _requestService: IRequestService, @IAgentHostProxyResolver private readonly _proxyResolver: IAgentHostProxyResolver, - @IConfigurationService private readonly _configurationService: IConfigurationService, + @IAgentConfigurationService private readonly _configurationService: IAgentConfigurationService, @IProductService private readonly _productService: IProductService, @ILogService private readonly _logService: ILogService, ) { } @@ -72,7 +73,7 @@ export class NetworkDiagnosticsService implements INetworkDiagnosticsService { const proxySettings: Record = {}; for (const key of PROXY_CONFIG_KEYS) { - const value = this._configurationService.getValue(key); + const value = this._configurationService.getRootValue(agentHostProxyConfigSchema, key); if (value === undefined || value === '' || (Array.isArray(value) && value.length === 0)) { continue; } diff --git a/src/vs/platform/agentHost/test/common/agentHostConfigurationSync.test.ts b/src/vs/platform/agentHost/test/common/agentHostConfigurationSync.test.ts index cef88f67fda5ce..f80b8a2ac115a8 100644 --- a/src/vs/platform/agentHost/test/common/agentHostConfigurationSync.test.ts +++ b/src/vs/platform/agentHost/test/common/agentHostConfigurationSync.test.ts @@ -6,12 +6,15 @@ import assert from 'assert'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../base/test/common/utils.js'; import { IConfigurationService, IConfigurationValue } from '../../../configuration/common/configuration.js'; -import { Extensions as ConfigurationExtensions, IConfigurationRegistry } from '../../../configuration/common/configurationRegistry.js'; +import { AgentHostConfigurationSyncScope, Extensions as ConfigurationExtensions, IConfigurationRegistry } from '../../../configuration/common/configurationRegistry.js'; import { Registry } from '../../../registry/common/platform.js'; -import { formatAgentHostConfigurationSyncValueForLog, getAgentHostConfigurationSyncEntries, getGlobalConfigurationValue, inspectValue, resolveAgentHostConfigurationSyncPatch } from '../../common/agentHostConfigurationSync.js'; +import '../../../request/common/request.js'; +import { AgentHostConfigurationSyncTarget, formatAgentHostConfigurationSyncValueForLog, getAgentHostConfigurationSyncEntries, getAgentHostConfigurationSyncTarget, getGlobalConfigurationValue, inspectValue, resolveAgentHostConfigurationSyncPatch } from '../../common/agentHostConfigurationSync.js'; +import { LOCAL_AGENT_HOST_RESOURCE_IDENTITY } from '../../common/agentHostResourceService.js'; const ALL_HOSTS_SETTING = 'test.agentHostSync.allHosts'; -const LOCAL_ONLY_SETTING = 'test.agentHostSync.localOnly'; +const LOCAL_SETTING = 'test.agentHostSync.local'; +const AMBIENT_SETTING = 'test.agentHostSync.ambient'; const HIDDEN_SETTING = 'test.agentHostSync.hidden'; const UNSYNCED_SETTING = 'test.agentHostSync.unsynced'; const ENUM_SETTING = 'test.agentHostSync.enum'; @@ -43,10 +46,15 @@ suite('AgentHostConfigurationSync', () => { default: true, agentHost: { key: 'allHostsValue' }, }, - [LOCAL_ONLY_SETTING]: { + [LOCAL_SETTING]: { type: 'boolean' as const, default: false, - agentHost: { key: 'localOnlyValue', localOnly: true }, + agentHost: { key: 'localValue', scope: AgentHostConfigurationSyncScope.Local }, + }, + [AMBIENT_SETTING]: { + type: 'boolean' as const, + default: false, + agentHost: { key: 'ambientValue', scope: AgentHostConfigurationSyncScope.Ambient }, }, [HIDDEN_SETTING]: { type: 'boolean' as const, @@ -143,42 +151,112 @@ suite('AgentHostConfigurationSync', () => { test('builds a patch applying transforms, including for hidden settings', () => { const configurationService = createConfigurationService({ [ALL_HOSTS_SETTING]: { defaultValue: true }, - [LOCAL_ONLY_SETTING]: { defaultValue: false, userValue: true }, + [LOCAL_SETTING]: { defaultValue: false, userValue: true }, + [AMBIENT_SETTING]: { defaultValue: false, userValue: true }, [HIDDEN_SETTING]: { defaultValue: false, userValue: true }, }); - const patch = resolveAgentHostConfigurationSyncPatch(configurationService, true); + const patch = resolveAgentHostConfigurationSyncPatch(configurationService, AgentHostConfigurationSyncTarget.Local); assert.deepStrictEqual({ allHostsValue: patch.allHostsValue, - localOnlyValue: patch.localOnlyValue, + localValue: patch.localValue, + ambientValue: patch.ambientValue, hiddenValue: patch.hiddenValue, }, { allHostsValue: true, - localOnlyValue: true, + localValue: true, + ambientValue: true, hiddenValue: 'on', }); }); - test('omits localOnly settings for a remote host', () => { + test('applies local and ambient scopes to the corresponding host targets', () => { const configurationService = createConfigurationService({ [ALL_HOSTS_SETTING]: { defaultValue: true }, - [LOCAL_ONLY_SETTING]: { defaultValue: false, userValue: true }, + [LOCAL_SETTING]: { defaultValue: false, userValue: true }, + [AMBIENT_SETTING]: { defaultValue: false, userValue: true }, }); - const patch = resolveAgentHostConfigurationSyncPatch(configurationService, false); + const scopedValues = (target: AgentHostConfigurationSyncTarget) => { + const patch = resolveAgentHostConfigurationSyncPatch(configurationService, target); + return { + allHostsValue: patch.allHostsValue, + localValue: patch.localValue, + ambientValue: patch.ambientValue, + }; + }; + assert.deepStrictEqual({ + local: scopedValues(AgentHostConfigurationSyncTarget.Local), + remoteExtensionHost: scopedValues(AgentHostConfigurationSyncTarget.RemoteExtensionHost), + remote: scopedValues(AgentHostConfigurationSyncTarget.Remote), + }, { + local: { allHostsValue: true, localValue: true, ambientValue: true }, + remoteExtensionHost: { allHostsValue: true, localValue: undefined, ambientValue: true }, + remote: { allHostsValue: true, localValue: undefined, ambientValue: undefined }, + }); + }); + test('classifies local, remote-extension ambient, and explicit remote identities', () => { assert.deepStrictEqual({ - allHostsValue: patch.allHostsValue, - mirroredKeys: Object.keys(patch).filter(key => key === 'localOnlyValue'), + local: getAgentHostConfigurationSyncTarget(LOCAL_AGENT_HOST_RESOURCE_IDENTITY), + remoteExtensionHost: getAgentHostConfigurationSyncTarget('vscode-remote://ssh-remote+host'), + remote: getAgentHostConfigurationSyncTarget('ssh://host'), }, { - allHostsValue: true, - mirroredKeys: [], + local: AgentHostConfigurationSyncTarget.Local, + remoteExtensionHost: AgentHostConfigurationSyncTarget.RemoteExtensionHost, + remote: AgentHostConfigurationSyncTarget.Remote, + }); + }); + + test('mirrors HTTP proxy settings only to ambient Agent Hosts', () => { + const allProxySettingIds = [ + 'http.proxy', + 'http.proxyKerberosServicePrincipal', + 'http.noProxy', + 'http.proxySupport', + 'http.systemCertificates', + 'http.systemCertificatesNode', + 'http.experimental.systemCertificatesV2', + 'http.fetchAdditionalSupport', + 'http.webSocketAdditionalSupport', + 'http.experimental.networkInterfaceCheckInterval', + ]; + const syncedProxySettingIds = ['http.proxy', 'http.proxyKerberosServicePrincipal', 'http.noProxy']; + const local = getAgentHostConfigurationSyncEntries(AgentHostConfigurationSyncTarget.Local) + .filter(entry => allProxySettingIds.includes(entry.settingId)) + .map(entry => [entry.settingId, entry.sync.key]); + const remoteExtensionHost = getAgentHostConfigurationSyncEntries(AgentHostConfigurationSyncTarget.RemoteExtensionHost) + .filter(entry => allProxySettingIds.includes(entry.settingId)) + .map(entry => [entry.settingId, entry.sync.key]); + const remote = getAgentHostConfigurationSyncEntries(AgentHostConfigurationSyncTarget.Remote) + .filter(entry => allProxySettingIds.includes(entry.settingId)) + .map(entry => entry.settingId); + + assert.deepStrictEqual({ local, remoteExtensionHost, remote }, { + local: syncedProxySettingIds.map(settingId => [settingId, settingId]), + remoteExtensionHost: syncedProxySettingIds.map(settingId => [settingId, settingId]), + remote: [], + }); + }); + + test('clears unset local proxy strings with empty values', () => { + const configurationService = createConfigurationService({}); + const patch = resolveAgentHostConfigurationSyncPatch(configurationService, AgentHostConfigurationSyncTarget.Local); + + assert.deepStrictEqual({ + proxy: patch['http.proxy'], + proxyKerberosServicePrincipal: patch['http.proxyKerberosServicePrincipal'], + noProxy: patch['http.noProxy'], + }, { + proxy: '', + proxyKerberosServicePrincipal: '', + noProxy: [], }); }); test('only settings declaring `agentHost` are mirrored', () => { - const settingIds = getAgentHostConfigurationSyncEntries(true).map(entry => entry.settingId); + const settingIds = getAgentHostConfigurationSyncEntries(AgentHostConfigurationSyncTarget.Local).map(entry => entry.settingId); assert.deepStrictEqual({ hasSynced: settingIds.includes(ALL_HOSTS_SETTING), @@ -222,7 +300,7 @@ suite('AgentHostConfigurationSync', () => { assert.deepStrictEqual({ hidden: getGlobalConfigurationValue(configurationService, HIDDEN_SETTING), visible: getGlobalConfigurationValue(configurationService, ALL_HOSTS_SETTING), - mirrored: Object.keys(resolveAgentHostConfigurationSyncPatch(configurationService, true)).includes('hiddenValue'), + mirrored: Object.keys(resolveAgentHostConfigurationSyncPatch(configurationService, AgentHostConfigurationSyncTarget.Local)).includes('hiddenValue'), }, { hidden: false, visible: true, @@ -252,9 +330,9 @@ suite('AgentHostConfigurationSync', () => { }; registry.registerConfiguration(node); - const whileRegistered = getAgentHostConfigurationSyncEntries(true).map(entry => entry.settingId); + const whileRegistered = getAgentHostConfigurationSyncEntries(AgentHostConfigurationSyncTarget.Local).map(entry => entry.settingId); registry.deregisterConfigurations([node]); - const afterDeregister = getAgentHostConfigurationSyncEntries(true).map(entry => entry.settingId); + const afterDeregister = getAgentHostConfigurationSyncEntries(AgentHostConfigurationSyncTarget.Local).map(entry => entry.settingId); assert.deepStrictEqual({ registeredVisible: whileRegistered.includes('test.agentHostSync.transient'), diff --git a/src/vs/platform/agentHost/test/electron-browser/remoteAgentHostProtocolClient.test.ts b/src/vs/platform/agentHost/test/electron-browser/remoteAgentHostProtocolClient.test.ts index d23daac7168911..271a2a72575cd4 100644 --- a/src/vs/platform/agentHost/test/electron-browser/remoteAgentHostProtocolClient.test.ts +++ b/src/vs/platform/agentHost/test/electron-browser/remoteAgentHostProtocolClient.test.ts @@ -34,7 +34,7 @@ import { ITelemetryService, TelemetryLevel } from '../../../telemetry/common/tel import { NullTelemetryService } from '../../../telemetry/common/telemetryUtils.js'; import { AgentHostDisableRepoInfoTelemetryConfigKey, AgentHostTelemetryLevelConfigKey, AgentHostTerminalAutoApproveRulesConfigKey, DISABLE_REPO_INFO_TELEMETRY_SETTING_ID, GLOBAL_AUTO_APPROVE_SETTING_ID, telemetryLevelToAgentHostConfigValue, TERMINAL_AUTO_APPROVE_ENABLED_SETTING_ID, TERMINAL_AUTO_APPROVE_SETTING_ID, TERMINAL_IGNORE_DEFAULT_AUTO_APPROVE_RULES_SETTING_ID, type AgentHostTerminalAutoApproveRules } from '../../common/agentHostSchema.js'; import { AgentHostMapLegacySettingsToManagedSettingsSettingId } from '../../common/agentHostManagedSettings.js'; -import { Extensions as ConfigurationExtensions, IConfigurationRegistry } from '../../../configuration/common/configurationRegistry.js'; +import { AgentHostConfigurationSyncScope, Extensions as ConfigurationExtensions, IConfigurationRegistry } from '../../../configuration/common/configurationRegistry.js'; import { Registry } from '../../../registry/common/platform.js'; // Settings used to exercise declarative agent-host mirroring. Registered by this @@ -46,6 +46,10 @@ const SYNC_SETTING_A = 'test.remoteAgentHostProtocolClient.syncA'; const SYNC_CONFIG_KEY_A = 'testSyncValueA'; const SYNC_SETTING_B = 'test.remoteAgentHostProtocolClient.syncB'; const SYNC_CONFIG_KEY_B = 'testSyncValueB'; +const SYNC_LOCAL_SETTING = 'test.remoteAgentHostProtocolClient.syncLocal'; +const SYNC_LOCAL_CONFIG_KEY = 'testSyncLocal'; +const SYNC_AMBIENT_SETTING = 'test.remoteAgentHostProtocolClient.syncAmbient'; +const SYNC_AMBIENT_CONFIG_KEY = 'testSyncAmbient'; const syncTestConfigurationNode = { id: 'testRemoteAgentHostProtocolClientSync', @@ -61,6 +65,16 @@ const syncTestConfigurationNode = { default: false, agentHost: { key: SYNC_CONFIG_KEY_B }, }, + [SYNC_LOCAL_SETTING]: { + type: 'boolean' as const, + default: true, + agentHost: { key: SYNC_LOCAL_CONFIG_KEY, scope: AgentHostConfigurationSyncScope.Local }, + }, + [SYNC_AMBIENT_SETTING]: { + type: 'boolean' as const, + default: true, + agentHost: { key: SYNC_AMBIENT_CONFIG_KEY, scope: AgentHostConfigurationSyncScope.Ambient }, + }, }, }; import type { Implementation } from '../../common/state/protocol/common/commands.js'; @@ -137,6 +151,19 @@ function findRootConfigValue(messages: readonly ProtocolTransportMessage[], conf return getRootConfig(findRootConfigNotification(messages, configKey))[configKey]; } +function findOptionalRootConfigValue(messages: readonly ProtocolTransportMessage[], configKey: string): RootConfigValue { + for (const message of messages) { + if (!hasKey(message, { method: true }) || message.method !== 'dispatchAction') { + continue; + } + const params = (message as JsonRpcNotification).params as ITestRootConfigNotificationParams | undefined; + if (params?.action?.type === ActionType.RootConfigChanged && params.action.config && hasKey(params.action.config, { [configKey]: true })) { + return params.action.config[configKey]; + } + } + return undefined; +} + class TestProtocolTransport extends Disposable implements IProtocolTransport { constructor(readonly clientConnectionKind?: AgentHostClientConnectionKind) { super(); @@ -1100,6 +1127,37 @@ suite('RemoteAgentHostProtocolClient', () => { }); }); + test('applies local and ambient configuration scopes to the target Agent Host', async () => { + const local = createClientForIdentity(LOCAL_AGENT_HOST_RESOURCE_IDENTITY); + const remoteExtensionHost = createClientForIdentity('vscode-remote://ssh-remote+host'); + const remote = createClient(); + + await Promise.all([ + connectClient(local.client, local.transport), + connectClient(remoteExtensionHost.client, remoteExtensionHost.transport), + connectClient(remote.client, remote.transport), + ]); + + assert.deepStrictEqual({ + local: { + local: findRootConfigValue(local.transport.sentMessages, SYNC_LOCAL_CONFIG_KEY), + ambient: findRootConfigValue(local.transport.sentMessages, SYNC_AMBIENT_CONFIG_KEY), + }, + remoteExtensionHost: { + local: findOptionalRootConfigValue(remoteExtensionHost.transport.sentMessages, SYNC_LOCAL_CONFIG_KEY), + ambient: findRootConfigValue(remoteExtensionHost.transport.sentMessages, SYNC_AMBIENT_CONFIG_KEY), + }, + remote: { + local: findOptionalRootConfigValue(remote.transport.sentMessages, SYNC_LOCAL_CONFIG_KEY), + ambient: findOptionalRootConfigValue(remote.transport.sentMessages, SYNC_AMBIENT_CONFIG_KEY), + }, + }, { + local: { local: true, ambient: true }, + remoteExtensionHost: { local: undefined, ambient: true }, + remote: { local: undefined, ambient: undefined }, + }); + }); + test('forwards the repo-info telemetry debug switch on connect and change', async () => { const configurationService = new TestConfigurationService({ [DISABLE_REPO_INFO_TELEMETRY_SETTING_ID]: true }); const { client, transport } = createClient(disposables.add(new TestProtocolTransport()), createPermissionService(), undefined, new NullLogService(), configurationService); diff --git a/src/vs/platform/agentHost/test/node/agentConfigurationService.test.ts b/src/vs/platform/agentHost/test/node/agentConfigurationService.test.ts index 73cb886b7acf4c..1402fe5b17f368 100644 --- a/src/vs/platform/agentHost/test/node/agentConfigurationService.test.ts +++ b/src/vs/platform/agentHost/test/node/agentConfigurationService.test.ts @@ -11,7 +11,7 @@ import { join } from '../../../../base/common/path.js'; import { URI } from '../../../../base/common/uri.js'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../base/test/common/utils.js'; import { NullLogService } from '../../../log/common/log.js'; -import { createSchema, schemaProperty } from '../../common/agentHostSchema.js'; +import { AgentHostProxyConfigKey, createSchema, schemaProperty } from '../../common/agentHostSchema.js'; import { AGENT_CUSTOMIZATION_SETTINGS_META_KEY, getAgentCustomizationSettingsEntries } from '../../common/agentCustomizationSettings.js'; import type { RootConfigState } from '../../common/state/protocol/state.js'; import { ActionType } from '../../common/state/sessionActions.js'; @@ -264,6 +264,26 @@ suite('AgentConfigurationService', () => { fs.rmSync(directory, { recursive: true, force: true }); }); + test('loads manually configured proxy settings from persisted Agent Host config', () => { + const directory = fs.mkdtempSync(join(os.tmpdir(), 'agent-config-')); + const resource = URI.file(join(directory, 'agent-host-config.json')); + fs.writeFileSync(resource.fsPath, JSON.stringify({ + [AgentHostProxyConfigKey.Proxy]: 'http://proxy.example:8080', + [AgentHostProxyConfigKey.NoProxy]: ['localhost'], + })); + const localManager = disposables.add(new AgentHostStateManager(new NullLogService())); + const localService = disposables.add(new AgentConfigurationService(localManager, new NullLogService(), resource)); + + assert.deepStrictEqual({ + proxy: localService.getRootConfigValues?.()[AgentHostProxyConfigKey.Proxy], + noProxy: localService.getRootConfigValues?.()[AgentHostProxyConfigKey.NoProxy], + }, { + proxy: 'http://proxy.example:8080', + noProxy: ['localhost'], + }); + fs.rmSync(directory, { recursive: true, force: true }); + }); + test('seeds provider configuration into the initial root snapshot', () => { const localManager = disposables.add(new AgentHostStateManager(new NullLogService())); disposables.add(new AgentConfigurationService(localManager, new NullLogService(), undefined, [{ diff --git a/src/vs/platform/agentHost/test/node/agentHostBootstrap.test.ts b/src/vs/platform/agentHost/test/node/agentHostBootstrap.test.ts index b4b2eaf3343dc9..a44d3b0b6a7201 100644 --- a/src/vs/platform/agentHost/test/node/agentHostBootstrap.test.ts +++ b/src/vs/platform/agentHost/test/node/agentHostBootstrap.test.ts @@ -4,52 +4,28 @@ *--------------------------------------------------------------------------------------------*/ import assert from 'assert'; -import { VSBuffer } from '../../../../base/common/buffer.js'; -import { URI } from '../../../../base/common/uri.js'; import { DisposableStore } from '../../../../base/common/lifecycle.js'; -import { Schemas } from '../../../../base/common/network.js'; -import { joinPath } from '../../../../base/common/resources.js'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../base/test/common/utils.js'; -import { IConfigurationService } from '../../../configuration/common/configuration.js'; -import { ConfigurationService } from '../../../configuration/common/configurationService.js'; -import { NativeEnvironmentService } from '../../../environment/node/environmentService.js'; -import { FileService } from '../../../files/common/fileService.js'; -import { InMemoryFileSystemProvider } from '../../../files/common/inMemoryFilesystemProvider.js'; -import { OPTIONS, parseArgs } from '../../../environment/node/argv.js'; import { NullLogService } from '../../../log/common/log.js'; -import product from '../../../product/common/product.js'; import { ServiceCollection } from '../../../instantiation/common/serviceCollection.js'; +import { IRequestService } from '../../../request/common/request.js'; import { registerAgentHostNetworkServices } from '../../node/agentHostBootstrap.js'; - -class TestEnvironmentService extends NativeEnvironmentService { - override get appSettingsHome(): URI { - return URI.from({ scheme: Schemas.file, path: '/User' }); - } -} - -function createFileService(disposables: DisposableStore): FileService { - const fileService = disposables.add(new FileService(new NullLogService())); - const provider = disposables.add(new InMemoryFileSystemProvider()); - disposables.add(fileService.registerProvider(Schemas.file, provider)); - return fileService; -} +import { IAgentHostProxyResolver } from '../../node/agentHostProxyResolver.js'; suite('agentHostBootstrap', () => { const disposables = ensureNoDisposablesAreLeakedInTestSuite(); - test('loads configuration from appSettingsHome', async () => { + test('registers network services without reading VS Code settings', () => { const testDisposables = disposables.add(new DisposableStore()); - const environmentService = new TestEnvironmentService(parseArgs(['--force-disable-user-env'], OPTIONS), { _serviceBrand: undefined, ...product }); - const fileService = createFileService(testDisposables); - - await fileService.createFolder(environmentService.appSettingsHome); - await fileService.writeFile(joinPath(environmentService.appSettingsHome, 'settings.json'), VSBuffer.fromString('{ "http.proxy": "http://proxy.example:8080" }')); - const services = new ServiceCollection(); - await registerAgentHostNetworkServices(services, fileService, environmentService, new NullLogService(), testDisposables); - - const configurationService = services.get(IConfigurationService); - assert.ok(configurationService instanceof ConfigurationService); - assert.strictEqual(configurationService.getValue('http.proxy'), 'http://proxy.example:8080'); + const networkServices = registerAgentHostNetworkServices(services, new NullLogService(), testDisposables); + + assert.deepStrictEqual({ + proxyResolver: services.get(IAgentHostProxyResolver) === networkServices.proxyResolver, + requestService: services.get(IRequestService) === networkServices.requestService, + }, { + proxyResolver: true, + requestService: true, + }); }); }); diff --git a/src/vs/platform/agentHost/test/node/agentHostRequestService.test.ts b/src/vs/platform/agentHost/test/node/agentHostRequestService.test.ts index 37c2a43be888d0..044d7f22ddbb8f 100644 --- a/src/vs/platform/agentHost/test/node/agentHostRequestService.test.ts +++ b/src/vs/platform/agentHost/test/node/agentHostRequestService.test.ts @@ -8,15 +8,16 @@ import { streamToBuffer } from '../../../../base/common/buffer.js'; import { CancellationToken, CancellationTokenSource } from '../../../../base/common/cancellation.js'; import { isCancellationError } from '../../../../base/common/errors.js'; import { Event } from '../../../../base/common/event.js'; -import { Disposable } from '../../../../base/common/lifecycle.js'; +import { Disposable, type DisposableStore } from '../../../../base/common/lifecycle.js'; import { IChannel } from '../../../../base/parts/ipc/common/ipc.js'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../base/test/common/utils.js'; -import { TestConfigurationService } from '../../../configuration/test/common/testConfigurationService.js'; -import type { INativeEnvironmentService } from '../../../environment/common/environment.js'; import { NullLogService } from '../../../log/common/log.js'; import { IProductService } from '../../../product/common/productService.js'; import { AuthInfo, IRequestService } from '../../../request/common/request.js'; import { AgentHostClientProxyChannel, createAgentHostClientProxyConnection, type IAgentHostClientProxyConnection } from '../../common/agentHostClientProxyChannel.js'; +import { AgentHostProxyConfigKey } from '../../common/agentHostSchema.js'; +import { AgentConfigurationService, type IAgentConfigurationService } from '../../node/agentConfigurationService.js'; +import { AgentHostStateManager } from '../../node/agentHostStateManager.js'; import { AgentHostProxyResolver, IAgentHostProxyResolver } from '../../node/agentHostProxyResolver.js'; import { AgentHostRequestService } from '../../node/agentHostRequestService.js'; import { NetworkDiagnosticsService } from '../../node/networkDiagnosticsService.js'; @@ -24,6 +25,7 @@ import { NetworkDiagnosticsService } from '../../node/networkDiagnosticsService. class TestProxyResolver implements IAgentHostProxyResolver { declare readonly _serviceBrand: undefined; readonly onDidRegisterConnection = Event.None; + readonly onDidChangeConfiguration = Event.None; lastInput: string | URL | Request | undefined; lastInit: RequestInit | undefined; @@ -33,6 +35,12 @@ class TestProxyResolver implements IAgentHostProxyResolver { return Disposable.None; } + bindConfigurationService(_configurationService: IAgentConfigurationService, _transient: boolean): void { } + + getConfigurationValue(_key: string): T | undefined { + return undefined; + } + resolveProxy(_url: string): Promise { return Promise.resolve('http://proxy.example:8080'); } @@ -44,11 +52,26 @@ class TestProxyResolver implements IAgentHostProxyResolver { } } +class TestAgentHostProxyResolver extends AgentHostProxyResolver { + kerberosLookup: { url: string; spn: string | undefined } | undefined; + + protected override async _lookupKerberosAuthorization(url: string, spn: string | undefined): Promise { + this.kerberosLookup = { url, spn }; + return 'token'; + } +} + +function createAgentConfigurationService(disposables: Pick): AgentConfigurationService { + const logService = new NullLogService(); + const stateManager = disposables.add(new AgentHostStateManager(logService)); + return disposables.add(new AgentConfigurationService(stateManager, logService)); +} + suite('AgentHostProxyResolver', () => { const disposables = ensureNoDisposablesAreLeakedInTestSuite(); test('fires when the first connection registers and after all connections reconnect', () => { - const resolver = disposables.add(new AgentHostProxyResolver(new TestConfigurationService(), new NullLogService())); + const resolver = disposables.add(new AgentHostProxyResolver(new NullLogService())); let registrations = 0; disposables.add(resolver.onDidRegisterConnection(() => registrations++)); const connection: IAgentHostClientProxyConnection = { @@ -71,18 +94,62 @@ suite('AgentHostProxyResolver', () => { afterReconnect: 2, }); }); + + test('reads manually configured proxy settings from Agent Host configuration', async () => { + const resolver = disposables.add(new AgentHostProxyResolver(new NullLogService())); + const configurationService = createAgentConfigurationService(disposables); + let configurationChanges = 0; + disposables.add(resolver.onDidChangeConfiguration(() => configurationChanges++)); + resolver.bindConfigurationService(configurationService, false); + configurationService.updateRootConfig({ [AgentHostProxyConfigKey.Proxy]: 'http://proxy.example:8080' }); + + assert.deepStrictEqual({ + proxy: await resolver.resolveProxy('https://example.com'), + configurationChanges, + }, { + proxy: 'http://proxy.example:8080', + configurationChanges: 1, + }); + }); + + test('clears persisted proxy values when binding local mirrored configuration', () => { + const resolver = disposables.add(new AgentHostProxyResolver(new NullLogService())); + const configurationService = createAgentConfigurationService(disposables); + configurationService.updateRootConfig({ [AgentHostProxyConfigKey.Proxy]: 'http://stale-proxy.example:8080' }); + + resolver.bindConfigurationService(configurationService, true); + + assert.strictEqual(configurationService.getRootConfigValues?.()[AgentHostProxyConfigKey.Proxy], undefined); + }); + + test('uses manually configured Kerberos authentication without a renderer bridge', async () => { + const resolver = disposables.add(new TestAgentHostProxyResolver(new NullLogService())); + const configurationService = createAgentConfigurationService(disposables); + resolver.bindConfigurationService(configurationService, false); + configurationService.updateRootConfig({ [AgentHostProxyConfigKey.ProxyKerberosServicePrincipal]: 'HTTP/proxy.example' }); + + const authorization = await (resolver as unknown as { + _hostLookupKerberosAuthorization(url: string): Promise; + })._hostLookupKerberosAuthorization('http://proxy.example:8080'); + + assert.deepStrictEqual({ + authorization, + lookup: resolver.kerberosLookup, + }, { + authorization: 'Negotiate token', + lookup: { + url: 'http://proxy.example:8080', + spn: 'HTTP/proxy.example', + }, + }); + }); }); suite('AgentHostRequestService', () => { const disposables = ensureNoDisposablesAreLeakedInTestSuite(); function createService(proxyResolver: TestProxyResolver): AgentHostRequestService { - const environmentService = { - args: { 'force-disable-user-env': true }, - } as unknown as INativeEnvironmentService; return disposables.add(new AgentHostRequestService( - new TestConfigurationService(), - environmentService, new NullLogService(), proxyResolver, )); @@ -231,7 +298,48 @@ suite('AgentHostRequestService', () => { }); suite('NetworkDiagnosticsService', () => { - ensureNoDisposablesAreLeakedInTestSuite(); + const disposables = ensureNoDisposablesAreLeakedInTestSuite(); + + test('reports the configured and environment Kerberos proxy SPN', async () => { + const configurationService = createAgentConfigurationService(disposables); + configurationService.updateRootConfig({ + [AgentHostProxyConfigKey.ProxyKerberosServicePrincipal]: 'HTTP/configured.proxy', + }); + const previous = process.env['COPILOT_PROXY_KERBEROS_SPN']; + process.env['COPILOT_PROXY_KERBEROS_SPN'] = 'HTTP/environment.proxy'; + const service = new NetworkDiagnosticsService( + { + _serviceBrand: undefined, + onDidCompleteRequest: Event.None, + request: async () => { throw new Error('not implemented'); }, + resolveProxy: async () => undefined, + lookupAuthorization: async () => undefined, + lookupKerberosAuthorization: async () => undefined, + loadCertificates: async () => [], + }, + new TestProxyResolver(), + configurationService, + { version: 'test' } as IProductService, + new NullLogService(), + ); + try { + const result = await service.getInfo([]); + + assert.deepStrictEqual({ + setting: result.proxySettings[AgentHostProxyConfigKey.ProxyKerberosServicePrincipal], + environment: result.proxyEnv['COPILOT_PROXY_KERBEROS_SPN'], + }, { + setting: 'HTTP/configured.proxy', + environment: 'HTTP/environment.proxy', + }); + } finally { + if (previous === undefined) { + delete process.env['COPILOT_PROXY_KERBEROS_SPN']; + } else { + process.env['COPILOT_PROXY_KERBEROS_SPN'] = previous; + } + } + }); test('includes nested proxy response errors', async () => { const proxyError = new Error('Proxy response (407)'); @@ -246,10 +354,11 @@ suite('NetworkDiagnosticsService', () => { loadCertificates: async () => [], }; const proxyResolver = new TestProxyResolver(); + const configurationService = createAgentConfigurationService(disposables); const service = new NetworkDiagnosticsService( requestService, proxyResolver, - new TestConfigurationService(), + configurationService, { version: 'test' } as IProductService, new NullLogService(), ); diff --git a/src/vs/platform/agentHost/test/node/copilotAgent.test.ts b/src/vs/platform/agentHost/test/node/copilotAgent.test.ts index 3701fe3f97f0bd..dbc8dae9cf4dd4 100644 --- a/src/vs/platform/agentHost/test/node/copilotAgent.test.ts +++ b/src/vs/platform/agentHost/test/node/copilotAgent.test.ts @@ -36,7 +36,7 @@ import { NullTelemetryService, NullTelemetryServiceShape } from '../../../teleme import { AgentHostTelemetryService } from '../../node/agentHostTelemetryService.js'; import { CopilotCliConfigKey, CopilotCliVSCodeAssignmentContextKey } from '../../common/copilotCliConfig.js'; import { AgentHostConfigKey } from '../../common/agentHostCustomizationConfig.js'; -import { AgentHostAutoApprovePolicyRestrictedConfigKey, AgentHostByokModelsEnabledConfigKey, AgentHostCopilotMultiRootEnabledConfigKey, AgentHostMigrateLegacyCopilotCliEnabledConfigKey, AgentHostSystemProxyEnabledConfigKey } from '../../common/agentHostSchema.js'; +import { AgentHostAutoApprovePolicyRestrictedConfigKey, AgentHostByokModelsEnabledConfigKey, AgentHostCopilotMultiRootEnabledConfigKey, AgentHostMigrateLegacyCopilotCliEnabledConfigKey, AgentHostProxyConfigKey, AgentHostSystemProxyEnabledConfigKey } from '../../common/agentHostSchema.js'; import { IAgentPluginManager, ISyncedCustomization } from '../../common/agentPluginManager.js'; import { getTelemetryChatSessionId } from '../../common/agentTelemetryCorrelation.js'; import { AgentSession, GITHUB_COPILOT_PROTECTED_RESOURCE, type AgentSignal, type IAgentChatContext, type IAgentChatMetadata, type IAgentCreateChatForkSource, type IAgentCreateChatOptions, type IAgentCreateChatResult, type IAgentCreateSessionConfig, type IAgentDiscoveredChat, type IAgentMaterializeChatEvent, type IAgentSpawnChatEvent } from '../../common/agent.js'; @@ -711,6 +711,8 @@ class TestProxyResolver implements IAgentHostProxyResolver { declare readonly _serviceBrand: undefined; private readonly _onDidRegisterConnection = new Emitter(); readonly onDidRegisterConnection = this._onDidRegisterConnection.event; + private readonly _onDidChangeConfiguration = new Emitter(); + readonly onDidChangeConfiguration = this._onDidChangeConfiguration.event; private readonly _connections = new Map(); resolveProxyCalls = 0; resolvedProxy: string | undefined; @@ -729,6 +731,16 @@ class TestProxyResolver implements IAgentHostProxyResolver { }); } + bindConfigurationService(_configurationService: IAgentConfigurationService, _transient: boolean): void { } + + getConfigurationValue(_key: string): T | undefined { + return undefined; + } + + fireConfigurationChange(): void { + this._onDidChangeConfiguration.fire(); + } + async resolveProxy(_url: string): Promise { this.resolveProxyCalls++; await this.resolveProxyGate; @@ -3389,6 +3401,111 @@ suite('CopilotAgent', () => { } }); + test('refreshes the proxy when Agent Host proxy configuration changes', async () => { + const client = new TestCopilotClient([]); + const proxyResolver = new TestProxyResolver(); + const proxy = 'http://configured-proxy.example:8080'; + const { agent } = createTestAgentContext(disposables, { copilotClient: client, proxyResolver }); + try { + await agent.listChatsToMigrate(); + proxyResolver.resolvedProxy = proxy; + proxyResolver.fireConfigurationChange(); + for (let i = 0; i < 20 && client.stopCallCount < 1; i++) { + await timeout(0); + } + await agent.listChatsToMigrate(); + + assert.deepStrictEqual({ + startCallCount: client.startCallCount, + stopCallCount: client.stopCallCount, + resolveProxyCalls: proxyResolver.resolveProxyCalls, + httpProxy: getCreatedClientOptions(agent).at(-1)?.env?.['HTTP_PROXY'], + httpsProxy: getCreatedClientOptions(agent).at(-1)?.env?.['HTTPS_PROXY'], + }, { + startCallCount: 2, + stopCallCount: 1, + resolveProxyCalls: 3, + httpProxy: proxy, + httpsProxy: proxy, + }); + } finally { + await disposeAgent(agent); + } + }); + + test('forwards the configured Kerberos proxy SPN to the Copilot runtime', async () => { + const client = new TestCopilotClient([]); + const kerberosSpn = 'HTTP/proxy.example'; + const { agent } = createTestAgentContext(disposables, { + copilotClient: client, + rootConfig: { [AgentHostProxyConfigKey.ProxyKerberosServicePrincipal]: kerberosSpn }, + }); + try { + await agent.listChatsToMigrate(); + + assert.strictEqual(getCreatedClientOptions(agent).at(-1)?.env?.['COPILOT_PROXY_KERBEROS_SPN'], kerberosSpn); + } finally { + await disposeAgent(agent); + } + }); + + test('preserves an explicit Kerberos proxy SPN environment override', async () => { + const client = new TestCopilotClient([]); + const configuredSpn = 'HTTP/configured.proxy'; + const environmentSpn = 'HTTP/environment.proxy'; + const previous = process.env['COPILOT_PROXY_KERBEROS_SPN']; + process.env['COPILOT_PROXY_KERBEROS_SPN'] = environmentSpn; + const { agent } = createTestAgentContext(disposables, { + copilotClient: client, + rootConfig: { [AgentHostProxyConfigKey.ProxyKerberosServicePrincipal]: configuredSpn }, + }); + try { + await agent.listChatsToMigrate(); + + assert.strictEqual(getCreatedClientOptions(agent).at(-1)?.env?.['COPILOT_PROXY_KERBEROS_SPN'], environmentSpn); + } finally { + if (previous === undefined) { + delete process.env['COPILOT_PROXY_KERBEROS_SPN']; + } else { + process.env['COPILOT_PROXY_KERBEROS_SPN'] = previous; + } + await disposeAgent(agent); + } + }); + + test('restarts the Copilot runtime when the Kerberos proxy SPN changes', async () => { + const client = new TestCopilotClient([]); + const proxyResolver = new TestProxyResolver(); + const initialSpn = 'HTTP/initial.proxy'; + const changedSpn = 'HTTP/changed.proxy'; + const { agent, configurationService } = createTestAgentContext(disposables, { + copilotClient: client, + proxyResolver, + rootConfig: { [AgentHostProxyConfigKey.ProxyKerberosServicePrincipal]: initialSpn }, + }); + try { + await agent.listChatsToMigrate(); + configurationService.updateRootConfig({ [AgentHostProxyConfigKey.ProxyKerberosServicePrincipal]: changedSpn }); + proxyResolver.fireConfigurationChange(); + for (let i = 0; i < 20 && client.stopCallCount < 1; i++) { + await timeout(0); + } + await agent.listChatsToMigrate(); + + assert.deepStrictEqual({ + startCallCount: client.startCallCount, + stopCallCount: client.stopCallCount, + kerberosSpn: getCreatedClientOptions(agent).at(-1)?.env?.['COPILOT_PROXY_KERBEROS_SPN'], + }, { + startCallCount: 2, + stopCallCount: 1, + kerberosSpn: changedSpn, + }); + } finally { + await disposeAgent(agent); + } + }); + test('resolves the proxy on first client start without a bridge', async () => { const client = new TestCopilotClient([]); const proxyResolver = new TestProxyResolver(); diff --git a/src/vs/platform/configuration/common/configurationRegistry.ts b/src/vs/platform/configuration/common/configurationRegistry.ts index 27f1b11f05e2a5..ed1315b68f1374 100644 --- a/src/vs/platform/configuration/common/configurationRegistry.ts +++ b/src/vs/platform/configuration/common/configurationRegistry.ts @@ -61,13 +61,17 @@ export interface IAgentHostConfigurationSync { */ readonly transform?: (value: unknown) => unknown; - /** - * When `true`, the value is only mirrored to a local agent host, and never to - * a remote one. Use for settings that describe the client's own machine — - * filesystem paths, machine identity — which are meaningless on a remote - * host. Defaults to `false`, mirroring to every agent host. - */ - readonly localOnly?: boolean; + /** Which Agent Host targets receive this setting. Defaults to {@link AgentHostConfigurationSyncScope.All}. */ + readonly scope?: AgentHostConfigurationSyncScope; +} + +export const enum AgentHostConfigurationSyncScope { + /** Mirror to every Agent Host connection. */ + All = 'all', + /** Mirror only to the local utility-process Agent Host. */ + Local = 'local', + /** Mirror to the ambient Agent Host, whether local or colocated with a remote extension host. */ + Ambient = 'ambient', } export interface IConfigurationRegistry { diff --git a/src/vs/platform/request/common/request.ts b/src/vs/platform/request/common/request.ts index 81c6ad5c502ab3..b5d0e36baca605 100644 --- a/src/vs/platform/request/common/request.ts +++ b/src/vs/platform/request/common/request.ts @@ -10,7 +10,7 @@ import { Emitter, Event } from '../../../base/common/event.js'; import { Disposable } from '../../../base/common/lifecycle.js'; import { IHeaders, IRequestContext, IRequestOptions } from '../../../base/parts/request/common/request.js'; import { localize } from '../../../nls.js'; -import { ConfigurationScope, Extensions, IConfigurationNode, IConfigurationRegistry } from '../../configuration/common/configurationRegistry.js'; +import { AgentHostConfigurationSyncScope, ConfigurationScope, Extensions, IConfigurationNode, IConfigurationRegistry } from '../../configuration/common/configurationRegistry.js'; import { createDecorator } from '../../instantiation/common/instantiation.js'; import { ILogService } from '../../log/common/log.js'; import { Registry } from '../../registry/common/platform.js'; @@ -273,7 +273,8 @@ function registerProxyConfigurations(useHostProxy = true, useHostProxyDefault = type: 'string', pattern: '^(https?|socks|socks4a?|socks5h?)://([^:]*(:[^@]*)?@)?([^:]+|\\[[:0-9a-fA-F]+\\])(:\\d+)?/?$|^$', markdownDescription: localize('proxy', "The proxy setting to use. If not set, will be inherited from the `http_proxy` and `https_proxy` environment variables. When during [remote development](https://aka.ms/vscode-remote) the {0} setting is disabled this setting can be configured in the local and the remote settings separately.", '`#http.useLocalProxyConfiguration#`'), - restricted: true + restricted: true, + agentHost: { key: 'http.proxy', scope: AgentHostConfigurationSyncScope.Ambient, transform: value => typeof value === 'string' ? value : '' }, }, 'http.proxyStrictSSL': { type: 'boolean', @@ -284,13 +285,15 @@ function registerProxyConfigurations(useHostProxy = true, useHostProxyDefault = 'http.proxyKerberosServicePrincipal': { type: 'string', markdownDescription: localize('proxyKerberosServicePrincipal', "Overrides the principal service name for Kerberos authentication with the HTTP proxy. A default based on the proxy hostname is used when this is not set. When during [remote development](https://aka.ms/vscode-remote) the {0} setting is disabled this setting can be configured in the local and the remote settings separately.", '`#http.useLocalProxyConfiguration#`'), - restricted: true + restricted: true, + agentHost: { key: 'http.proxyKerberosServicePrincipal', scope: AgentHostConfigurationSyncScope.Ambient, transform: value => typeof value === 'string' ? value : '' }, }, 'http.noProxy': { type: 'array', items: { type: 'string' }, markdownDescription: localize('noProxy', "Specifies domain names for which proxy settings should be ignored for HTTP/HTTPS requests. When during [remote development](https://aka.ms/vscode-remote) the {0} setting is disabled this setting can be configured in the local and the remote settings separately.", '`#http.useLocalProxyConfiguration#`'), - restricted: true + restricted: true, + agentHost: { key: 'http.noProxy', scope: AgentHostConfigurationSyncScope.Ambient, transform: value => Array.isArray(value) ? value : [] }, }, 'http.proxyAuthorization': { type: ['null', 'string'], diff --git a/src/vs/sessions/browser/parts/chatGroupsView.ts b/src/vs/sessions/browser/parts/chatGroupsView.ts index bb7ff054f77d62..5ef6dcd2a0c7f5 100644 --- a/src/vs/sessions/browser/parts/chatGroupsView.ts +++ b/src/vs/sessions/browser/parts/chatGroupsView.ts @@ -102,6 +102,7 @@ export class ChatGroupsView extends Themable { private _lastSessionActiveChatId: string | undefined; private _lastLayout: { readonly width: number; readonly height: number; readonly top: number; readonly left: number } | undefined; + private _gridDidLayout = false; constructor( @IThemeService themeService: IThemeService, @@ -129,6 +130,7 @@ export class ChatGroupsView extends Themable { this._groupDisposables.clearAndDisposeAll(); this._currentSessionStore = store; this._grid = undefined; + this._gridDidLayout = false; this._groups = []; this._activeGroup = undefined; this._restoreAssignment = undefined; @@ -163,7 +165,6 @@ export class ChatGroupsView extends Themable { store.add(this._instantiationService.createInstance(ChatGroupDropTarget, this.element, dropDelegate)); store.add(autorun(reader => this._reconcile(reader))); - this._applyLayout(); } @@ -560,7 +561,7 @@ export class ChatGroupsView extends Themable { } private _findAdjacentGroup(reference: IGroupEntry): IGroupEntry | undefined { - if (this._grid && this._lastLayout) { + if (this._grid && this._gridDidLayout) { for (const direction of [Direction.Right, Direction.Left, Direction.Down, Direction.Up]) { const neighbor = this._grid.getNeighborViews(reference.view, direction)[0]; const group = neighbor && this._groups.find(candidate => candidate.view === neighbor); @@ -829,7 +830,10 @@ export class ChatGroupsView extends Themable { } const { width, height, top, left } = this._lastLayout; size(this.element, width, height); - this._grid?.layout(width, height, top, left); + if (this._grid) { + this._grid.layout(width, height, top, left); + this._gridDidLayout = true; + } } private get _separatorBorder(): Color { diff --git a/src/vs/sessions/contrib/github/browser/createSessionFromPullRequestAction.ts b/src/vs/sessions/contrib/github/browser/createSessionFromPullRequestAction.ts index 229ebdb7c5c44e..15556d940b7c85 100644 --- a/src/vs/sessions/contrib/github/browser/createSessionFromPullRequestAction.ts +++ b/src/vs/sessions/contrib/github/browser/createSessionFromPullRequestAction.ts @@ -25,7 +25,7 @@ import { ISessionsPartService } from '../../../services/sessions/browser/session import { ISessionsService } from '../../../services/sessions/browser/sessionsService.js'; import { CLOSE_MOBILE_SIDEBAR_DRAWER_COMMAND_ID } from '../../../browser/workbench.js'; import { Menus } from '../../../browser/menus.js'; -import { ISessionSection, SessionSectionHasNonCloudRepositoryContext, SessionSectionTypeContext } from '../../sessions/browser/views/sessionsList.js'; +import { ISessionSection, SessionSectionHasGitHubRepositoryContext, SessionSectionHasNonCloudRepositoryContext, SessionSectionTypeContext } from '../../sessions/browser/views/sessionsList.js'; import { IGitHubService } from './githubService.js'; import { IGitHubPullRequestSummary } from '../common/types.js'; import { createPullRequestBootstrapPrompt, createPullRequestContextAttachment, createPullRequestQuickPickItems, createPullRequestSessionMetadata, getExistingPullRequests, getGitHubRepositoryFromRemotes, hasExistingPullRequest, IPullRequestQuickPickItem, mergePullRequestSummaries, pullRequestMatchesQuery, resolvePullRequestSessionRepository } from './pullRequestPicker.js'; @@ -46,8 +46,9 @@ registerAction2(class CreateSessionFromPullRequestAction extends Action2 { order: 2, when: ContextKeyExpr.and( ChatContextKeys.enabled, - ContextKeyExpr.equals(SessionSectionTypeContext.key, 'workspace'), + SessionSectionHasGitHubRepositoryContext, SessionSectionHasNonCloudRepositoryContext, + SessionSectionTypeContext.isEqualTo('workspace') ), }, }); diff --git a/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts b/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts index a8e87e896589fc..343ce59417b1c5 100644 --- a/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts +++ b/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts @@ -62,6 +62,87 @@ import { createSessionOutputObs, ISessionOutputObs } from './agentHostSessionFil const STORAGE_KEY_REMEMBERED_SESSION_CONFIG_VALUES = 'sessions.agentHost.sessionConfigPicker.selectedValues'; const UNSAFE_SESSION_CONFIG_KEYS = new Set(['__proto__', 'constructor', 'prototype']); +const SESSION_CHANGE_NOTIFICATION_DEBOUNCE_MS = 50; + +function mergeSessionChangeEvents(events: readonly ISessionChangeEvent[]): ISessionChangeEvent { + const changes = new Map(); + for (const event of events) { + for (const session of event.added) { + changes.set(session.sessionId, { removed: changes.get(session.sessionId)?.removed, added: session }); + } + for (const session of event.removed) { + changes.set(session.sessionId, { removed: session }); + } + for (const session of event.changed) { + const change = changes.get(session.sessionId); + if (change?.removed && !change.added) { + continue; + } + if (change?.added) { + change.added = session; + } else { + changes.set(session.sessionId, { changed: session }); + } + } + } + + const added: ISession[] = []; + const removed: ISession[] = []; + const changed: ISession[] = []; + for (const change of changes.values()) { + if (change.added) { + added.push(change.added); + } + if (change.removed) { + removed.push(change.removed); + } + if (change.changed) { + changed.push(change.changed); + } + } + return { + added, + removed, + changed, + }; +} + +function debounceSessionChangeEvents(notifications: Event, immediate: Event, disposable: DisposableStore): Event { + const event: Event = (listener, thisArgs) => { + const store = new DisposableStore(); + let pending: ISessionChangeEvent[] | undefined; + store.add(toDisposable(() => { + pending?.splice(0); + pending = undefined; + })); + + const takePending = (event?: ISessionChangeEvent): ISessionChangeEvent | undefined => { + if (!pending?.length) { + pending = undefined; + return event; + } + const events = pending?.splice(0) ?? []; + pending = undefined; + if (event) { + events.push(event); + } + return mergeSessionChangeEvents(events); + }; + const debounced = Event.debounce(notifications, (events, event) => { + pending = events ?? []; + pending.push(event); + return pending; + }, SESSION_CHANGE_NOTIFICATION_DEBOUNCE_MS, false, false, undefined, store); + const onDebounced = Event.filter( + Event.map(debounced, () => takePending(), store), + (event): event is ISessionChangeEvent => event !== undefined, + store, + ); + store.add(Event.any(onDebounced, Event.map(immediate, event => takePending(event) ?? event, store))(listener, thisArgs)); + return store; + }; + return Event.map(event, event => event, disposable); +} // Well-known config chips whose last-resolved schemas are cached and seeded into // new drafts, so they stay visible (disabled) while a draft re-resolves rather @@ -2303,7 +2384,9 @@ export abstract class BaseAgentHostSessionsProvider extends Disposable implement readonly onDidChangeSessionTypes: Event = this._onDidChangeSessionTypes.event; protected readonly _onDidChangeSessions = this._register(new Emitter()); - readonly onDidChangeSessions: Event = this._onDidChangeSessions.event; + private readonly _onDidChangeSessionsFromNotifications = this._register(new Emitter()); + private readonly _onDidChangeSessionsImmediately = Event.any(this._onDidChangeSessions.event, this._onDidChangeSessionsFromNotifications.event); + readonly onDidChangeSessions = debounceSessionChangeEvents(this._onDidChangeSessionsFromNotifications.event, this._onDidChangeSessions.event, this._store); protected readonly _onDidReplaceSession = this._register(new Emitter<{ readonly from: ISession; readonly to: ISession }>()); readonly onDidReplaceSession: Event<{ readonly from: ISession; readonly to: ISession }> = this._onDidReplaceSession.event; @@ -2544,7 +2627,7 @@ export abstract class BaseAgentHostSessionsProvider extends Disposable implement // opts in via `_enableSessionCachePersistence` (which sets the storage // key). They are safe to register unconditionally because they only act // at event time and read the key lazily. - this._register(this._onDidChangeSessions.event(e => { + this._register(this._onDidChangeSessionsImmediately(e => { if (!this._shouldTrackSessionCacheChanges()) { return; } @@ -5247,7 +5330,7 @@ export abstract class BaseAgentHostSessionsProvider extends Disposable implement const waitDisposables = new DisposableStore(); try { const sessionPromise = new Promise((resolve) => { - waitDisposables.add(this._onDidChangeSessions.event(e => { + waitDisposables.add(this._onDidChangeSessionsImmediately(e => { // Prefer this send's own id within the batch before falling // back to an acceptable novel session. const exact = e.added.find(s => s.resource.path.substring(1) === ownRawId && matches(ownRawId, s.resource.scheme)); @@ -5334,7 +5417,7 @@ export abstract class BaseAgentHostSessionsProvider extends Disposable implement const existing = this._sessionCache.get(rawId); if (existing) { if (this.updateAdapter(existing, meta)) { - this._onDidChangeSessions.fire({ added: [], removed: [], changed: [existing] }); + this._onDidChangeSessionsFromNotifications.fire({ added: [], removed: [], changed: [existing] }); } this._syncActiveClient(); return; @@ -5342,7 +5425,7 @@ export abstract class BaseAgentHostSessionsProvider extends Disposable implement const cached = this.createAdapter(meta); this._sessionCache.set(rawId, cached); - this._onDidChangeSessions.fire({ added: [cached], removed: [], changed: [] }); + this._onDidChangeSessionsFromNotifications.fire({ added: [cached], removed: [], changed: [] }); this._syncActiveClient(); } @@ -5350,7 +5433,7 @@ export abstract class BaseAgentHostSessionsProvider extends Disposable implement const rawId = AgentSession.id(session); const cached = this._removeCachedSession(rawId); if (cached) { - this._onDidChangeSessions.fire({ added: [], removed: [cached], changed: [] }); + this._onDidChangeSessionsFromNotifications.fire({ added: [], removed: [cached], changed: [] }); cached.dispose(); } this._syncActiveClient(); @@ -5477,7 +5560,7 @@ export abstract class BaseAgentHostSessionsProvider extends Disposable implement } if (didChange) { - this._onDidChangeSessions.fire({ added: [], removed: [], changed: [cached] }); + this._onDidChangeSessionsFromNotifications.fire({ added: [], removed: [], changed: [cached] }); } }); diff --git a/src/vs/sessions/contrib/providers/agentHost/test/browser/localAgentHostSessionsProvider.test.ts b/src/vs/sessions/contrib/providers/agentHost/test/browser/localAgentHostSessionsProvider.test.ts index 75ef9b1c93ee8d..6da5b109fad681 100644 --- a/src/vs/sessions/contrib/providers/agentHost/test/browser/localAgentHostSessionsProvider.test.ts +++ b/src/vs/sessions/contrib/providers/agentHost/test/browser/localAgentHostSessionsProvider.test.ts @@ -681,16 +681,18 @@ suite('LocalAgentHostSessionsProvider', () => { assert.deepStrictEqual(provider.sessionTypes, []); }); - test('rebinds session types when Agent Host starts with a new root subscription', () => { + test('rebinds session types when Agent Host starts with a new root subscription', () => runWithFakedTimers({ useFakeTimers: true }, async () => { agentHost.clearRootState(); const provider = createProvider(disposables, agentHost); let addedSessions = 0; disposables.add(provider.onDidChangeSessions(event => addedSessions += event.added.length)); + await timeout(0); agentHost.replaceRootStateOnStart([ { provider: 'copilotcli', displayName: 'Copilot', description: '', models: [] } as AgentInfo, ]); fireSessionAdded(agentHost, 'after-rebind'); + await timeout(100); assert.deepStrictEqual({ sessionTypes: provider.sessionTypes.map(type => ({ id: type.id, label: type.label })), @@ -701,15 +703,17 @@ suite('LocalAgentHostSessionsProvider', () => { rootStateListenerCount: 1, addedSessions: 1, }); - }); + })); - test('does not duplicate listeners when Agent Host starts after listeners bind', () => { + test('does not duplicate listeners when Agent Host starts after listeners bind', () => runWithFakedTimers({ useFakeTimers: true }, async () => { const provider = createProvider(disposables, agentHost); let addedSessions = 0; disposables.add(provider.onDidChangeSessions(event => addedSessions += event.added.length)); + await timeout(0); agentHost.fireAgentHostStart(); fireSessionAdded(agentHost, 'after-start'); + await timeout(100); assert.deepStrictEqual({ rootStateListenerCount: agentHost.rootStateListenerCount, @@ -718,7 +722,7 @@ suite('LocalAgentHostSessionsProvider', () => { rootStateListenerCount: 1, addedSessions: 1, }); - }); + })); test('reports no session types when rootState advertises no agents', () => { agentHost.setAgents([]); @@ -898,20 +902,90 @@ suite('LocalAgentHostSessionsProvider', () => { // ---- Session listing via notifications ------- - test('onDidChangeSessions fires when session added notification arrives', () => { + test('batches session added and removed notifications', () => runWithFakedTimers({ useFakeTimers: true }, async () => { const provider = createProvider(disposables, agentHost); + await timeout(0); + fireSessionAdded(agentHost, 'remove-1'); + fireSessionAdded(agentHost, 'remove-2'); + fireSessionAdded(agentHost, 'replace'); + const replacedSession = provider.getSessions().find(session => AgentSession.id(session.resource) === 'replace'); + const changes: ISessionChangeEvent[] = []; disposables.add(provider.onDidChangeSessions(e => changes.push(e))); - fireSessionAdded(agentHost, 'notif-1', { title: 'Notif Session' }); + fireSessionAdded(agentHost, 'add-1'); + fireSessionAdded(agentHost, 'add-2'); + fireSessionAdded(agentHost, 'transient'); + fireSessionRemoved(agentHost, 'remove-1'); + fireSessionRemoved(agentHost, 'remove-2'); + fireSessionRemoved(agentHost, 'transient'); + fireSessionRemoved(agentHost, 'replace'); + fireSessionAdded(agentHost, 'replace'); - assert.strictEqual(changes.length, 1); - assert.strictEqual(changes[0].added.length, 1); - assert.strictEqual(changes[0].added[0].title.get(), 'Notif Session'); - }); + const eventCountBeforeDebounce = changes.length; + const cachedBeforeDebounce = provider.getSessions().map(session => AgentSession.id(session.resource)).sort(); + await timeout(100); + + assert.deepStrictEqual({ + eventCountBeforeDebounce, + events: changes.map(change => ({ + added: change.added.map(session => AgentSession.id(session.resource)).sort(), + removed: change.removed.map(session => AgentSession.id(session.resource)).sort(), + changed: change.changed.map(session => AgentSession.id(session.resource)).sort(), + })), + replacement: { + addedIsOriginal: changes[0]?.added.find(session => AgentSession.id(session.resource) === 'replace') === replacedSession, + removedIsOriginal: changes[0]?.removed.find(session => AgentSession.id(session.resource) === 'replace') === replacedSession, + }, + cachedBeforeDebounce, + }, { + eventCountBeforeDebounce: 0, + events: [{ + added: ['add-1', 'add-2', 'replace'], + removed: ['remove-1', 'remove-2', 'replace', 'transient'], + changed: [], + }], + replacement: { + addedIsOriginal: false, + removedIsOriginal: true, + }, + cachedBeforeDebounce: ['add-1', 'add-2', 'replace'], + }); + })); + + test('immediate session changes flush pending notification batches', () => runWithFakedTimers({ useFakeTimers: true }, async () => { + const provider = createProvider(disposables, agentHost); + await timeout(0); + const changes: ISessionChangeEvent[] = []; + disposables.add(provider.onDidChangeSessions(e => changes.push(e))); - test('session removed notification clears cache and metadata', () => { + fireSessionAdded(agentHost, 'deleted-before-debounce'); + const session = provider.getSessions().find(session => AgentSession.id(session.resource) === 'deleted-before-debounce'); + assert.ok(session); + await provider.deleteSession(session.sessionId); + const eventsAfterDelete = changes.length; + await timeout(100); + + assert.deepStrictEqual({ + eventsAfterDelete, + events: changes.map(change => ({ + added: change.added.map(session => AgentSession.id(session.resource)), + removed: change.removed.map(session => AgentSession.id(session.resource)), + changed: change.changed.map(session => AgentSession.id(session.resource)), + })), + }, { + eventsAfterDelete: 1, + events: [{ + added: [], + removed: ['deleted-before-debounce'], + changed: [], + }], + }); + })); + + test('session removed notification clears cache and metadata', () => runWithFakedTimers({ useFakeTimers: true }, async () => { const provider = createProvider(disposables, agentHost); + await timeout(0); fireSessionAdded(agentHost, 'to-remove', { title: 'Removed' }); const metadata = Reflect.get(provider, '_metaByRawId') as Map; @@ -919,6 +993,7 @@ suite('LocalAgentHostSessionsProvider', () => { disposables.add(provider.onDidChangeSessions(e => changes.push(e))); fireSessionRemoved(agentHost, 'to-remove'); + await timeout(100); assert.deepStrictEqual({ removed: changes[0]?.removed.length, @@ -929,19 +1004,21 @@ suite('LocalAgentHostSessionsProvider', () => { session: undefined, metadata: undefined, }); - }); + })); - test('identical session added notification is ignored', () => { + test('identical session added notification is ignored', () => runWithFakedTimers({ useFakeTimers: true }, async () => { const provider = createProvider(disposables, agentHost); + await timeout(0); const changes: ISessionChangeEvent[] = []; disposables.add(provider.onDidChangeSessions(e => changes.push(e))); const timestamp = new Date(0).toISOString(); fireSessionAdded(agentHost, 'dup-sess', { title: 'Dup', createdAt: timestamp, modifiedAt: timestamp }); fireSessionAdded(agentHost, 'dup-sess', { title: 'Dup', createdAt: timestamp, modifiedAt: timestamp }); + await timeout(100); assert.strictEqual(changes.length, 1); - }); + })); test('removing non-existent session is no-op', () => { const provider = createProvider(disposables, agentHost); @@ -985,6 +1062,7 @@ suite('LocalAgentHostSessionsProvider', () => { fireSessionSummaryChanged(agentHost, 'worktree-upsert', { _meta: { git: { branchName: 'agents/worktree-session', baseBranchName: 'main' } }, }); + await timeout(100); const current = provider.getSessions()[0]!; const currentWorkspace = current.workspace.get()!; @@ -999,7 +1077,7 @@ suite('LocalAgentHostSessionsProvider', () => { originalWorkingDirectory: originalWorkingDirectory.toString(), workingDirectory: worktreeWorkingDirectory, branchName: 'agents/worktree-session', - changedEvents: [[true], [true]], + changedEvents: [[true]], }); })); @@ -3868,7 +3946,7 @@ suite('LocalAgentHostSessionsProvider', () => { }); })); - test('deleteSession does not remove a session twice when the host also notifies', async () => { + test('deleteSession does not remove a session twice when the host also notifies', () => runWithFakedTimers({ useFakeTimers: true }, async () => { const provider = createProvider(disposables, agentHost); fireSessionAdded(agentHost, 'delete-notified', { title: 'Delete Notified' }); const target = provider.getSessions().find(s => s.title.get() === 'Delete Notified'); @@ -3879,6 +3957,7 @@ suite('LocalAgentHostSessionsProvider', () => { agentHost.onDisposeSession = session => fireSessionRemoved(agentHost, AgentSession.id(session)); await provider.deleteSession(target.sessionId); + await timeout(100); assert.deepStrictEqual({ disposedSessions: agentHost.disposedSessions.length, @@ -3889,7 +3968,7 @@ suite('LocalAgentHostSessionsProvider', () => { removedEvents: 1, session: undefined, }); - }); + })); test('deleteSessions disposes all sessions and removes them from cache', async () => { const provider = createProvider(disposables, agentHost); diff --git a/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/remoteAgentHostSessionsProvider.test.ts b/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/remoteAgentHostSessionsProvider.test.ts index 6cc807aa886751..f2fa33ee6752a1 100644 --- a/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/remoteAgentHostSessionsProvider.test.ts +++ b/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/remoteAgentHostSessionsProvider.test.ts @@ -459,17 +459,19 @@ suite('RemoteAgentHostSessionsProvider', () => { // ---- Session listing via notifications ------- - test('onDidChangeSessions fires when session added notification arrives', () => { + test('onDidChangeSessions fires when session added notification arrives', () => runWithFakedTimers({ useFakeTimers: true }, async () => { const provider = createProvider(disposables, connection); + await timeout(0); const changes: ISessionChangeEvent[] = []; disposables.add(provider.onDidChangeSessions((e: ISessionChangeEvent) => changes.push(e))); fireSessionAdded(connection, 'notif-1', { title: 'Notif Session' }); + await timeout(100); assert.strictEqual(changes.length, 1); assert.strictEqual(changes[0].added.length, 1); assert.strictEqual(changes[0].added[0].title.get(), 'Notif Session'); - }); + })); test('session added notifications ingest any advertised agent provider', () => runWithFakedTimers({ useFakeTimers: true }, async () => { connection.setAgents([ @@ -491,30 +493,34 @@ suite('RemoteAgentHostSessionsProvider', () => { ); })); - test('session removed notification removes from cache', () => { + test('session removed notification removes from cache', () => runWithFakedTimers({ useFakeTimers: true }, async () => { const provider = createProvider(disposables, connection); + await timeout(0); fireSessionAdded(connection, 'to-remove', { title: 'Removed' }); const changes: ISessionChangeEvent[] = []; disposables.add(provider.onDidChangeSessions((e: ISessionChangeEvent) => changes.push(e))); fireSessionRemoved(connection, 'to-remove'); + await timeout(100); assert.strictEqual(changes.length, 1); assert.strictEqual(changes[0].removed.length, 1); - }); + })); - test('duplicate session added notification is ignored', () => { + test('duplicate session added notification is ignored', () => runWithFakedTimers({ useFakeTimers: true }, async () => { const provider = createProvider(disposables, connection); + await timeout(0); const changes: ISessionChangeEvent[] = []; disposables.add(provider.onDidChangeSessions((e: ISessionChangeEvent) => changes.push(e))); const timestamp = new Date(0).toISOString(); fireSessionAdded(connection, 'dup-sess', { title: 'Dup', createdAt: timestamp, modifiedAt: timestamp }); fireSessionAdded(connection, 'dup-sess', { title: 'Dup', createdAt: timestamp, modifiedAt: timestamp }); + await timeout(100); assert.strictEqual(changes.length, 1); - }); + })); test('uses project metadata as workspace group source', () => runWithFakedTimers({ useFakeTimers: true }, async () => { const projectUri = URI.parse('vscode-agent-host://localhost__4321/home/user/vscode?_ah%3DeyJzY2hlbWUiOiJmaWxlIn0'); diff --git a/src/vs/sessions/test/browser/chatGroupsView.test.ts b/src/vs/sessions/test/browser/chatGroupsView.test.ts index 5fe62d0aebf9f5..1638a2f14a5f0f 100644 --- a/src/vs/sessions/test/browser/chatGroupsView.test.ts +++ b/src/vs/sessions/test/browser/chatGroupsView.test.ts @@ -25,6 +25,7 @@ import { IActiveSession, ISessionsManagementService } from '../../services/sessi class TestChatView extends AbstractChatView { private readonly _focusTarget = mainWindow.document.createElement('button'); + layoutCount = 0; constructor(readonly kind: ChatViewKind) { super(); @@ -36,7 +37,9 @@ class TestChatView extends AbstractChatView { return {}; } - protected doLayout(): void { } + protected doLayout(): void { + this.layoutCount++; + } focus(): void { this._focusTarget.focus(); @@ -44,12 +47,20 @@ class TestChatView extends AbstractChatView { } class TestChatViewFactory extends mock() { + readonly views: TestChatView[] = []; + override createNewChatView(isNewChatInSession: boolean): AbstractChatView { - return new TestChatView(isNewChatInSession ? 'newChatInSession' : 'newSession'); + return this._createView(isNewChatInSession ? 'newChatInSession' : 'newSession'); } override createChatView(): AbstractChatView { - return new TestChatView('chat'); + return this._createView('chat'); + } + + private _createView(kind: ChatViewKind): TestChatView { + const view = new TestChatView(kind); + this.views.push(view); + return view; } } @@ -137,6 +148,7 @@ class TestSessionsService extends mock() { interface IChatGroupsHarness { readonly instantiationService: TestInstantiationService; readonly sessionsService: TestSessionsService; + readonly chatViewFactory: TestChatViewFactory; readonly view: ChatGroupsView; } @@ -144,7 +156,8 @@ function createHarness(disposables: Pick): IChatGroupsHa const store = disposables.add(new DisposableStore()); const instantiationService = workbenchInstantiationService(undefined, store); const sessionsService = new TestSessionsService(); - instantiationService.stub(IChatViewFactory, new TestChatViewFactory()); + const chatViewFactory = new TestChatViewFactory(); + instantiationService.stub(IChatViewFactory, chatViewFactory); instantiationService.stub(ISessionsService, sessionsService); instantiationService.stub(ISessionsManagementService, new class extends mock() { override readonly onDidChangeSessions = Event.None; @@ -158,13 +171,46 @@ function createHarness(disposables: Pick): IChatGroupsHa const view = store.add(instantiationService.createInstance(ChatGroupsView)); mainWindow.document.body.appendChild(view.element); store.add(toDisposable(() => view.element.remove())); - return { instantiationService, sessionsService, view }; + return { instantiationService, sessionsService, chatViewFactory, view }; } suite('Sessions - ChatGroupsView', () => { const disposables = ensureNoDisposablesAreLeakedInTestSuite(); const options = { renderSessionTypePickerInControls: constObservable(false) }; + test('opens a session with an active child chat after initial layout', () => { + const { view, chatViewFactory } = createHarness(disposables); + const main = createChat('main'); + const child = createChat('child', SessionStatus.Completed, main.resource); + const session = new TestActiveSession([main, child]); + session.activeChat.set(child, undefined); + view.layout(800, 600, 0, 0); + + view.setSession(session, options); + view.focus(); + + const renderedView = view.element.querySelector('.chat-view'); + const renderedChatView = chatViewFactory.views.find(createdView => createdView.kind === 'chat'); + const transientComposerView = chatViewFactory.views.find(createdView => createdView.kind === 'newChatInSession'); + assert.deepStrictEqual({ + renderedKind: renderedView?.dataset.kind, + renderedWidth: renderedView?.style.width, + renderedLayoutCount: renderedChatView?.layoutCount, + transientLayoutCount: transientComposerView?.layoutCount ?? 0, + focusedKind: view.element.ownerDocument.activeElement?.closest('.chat-view')?.dataset.kind, + activeTab: view.element.querySelector('.chat-composite-bar-tab.active')?.dataset.chatResource, + tabs: Array.from(view.element.querySelectorAll('.chat-composite-bar-tab')).map(tab => tab.dataset.chatResource), + }, { + renderedKind: 'chat', + renderedWidth: '800px', + renderedLayoutCount: 1, + transientLayoutCount: 0, + focusedKind: 'chat', + activeTab: child.resource.toString(), + tabs: [main.resource.toString(), child.resource.toString()], + }); + }); + test('focusing another group updates the session active chat', () => { const { sessionsService, view } = createHarness(disposables); const main = createChat('main'); diff --git a/src/vs/workbench/api/browser/mainThreadDocumentsAndEditors.ts b/src/vs/workbench/api/browser/mainThreadDocumentsAndEditors.ts index de8a7fa95e9f56..a16c68f6f7039e 100644 --- a/src/vs/workbench/api/browser/mainThreadDocumentsAndEditors.ts +++ b/src/vs/workbench/api/browser/mainThreadDocumentsAndEditors.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import { Event } from '../../../base/common/event.js'; -import { combinedDisposable, DisposableStore, DisposableMap } from '../../../base/common/lifecycle.js'; +import { combinedDisposable, Disposable, DisposableMap, DisposableStore } from '../../../base/common/lifecycle.js'; import { ICodeEditor, isCodeEditor, isDiffEditor, IActiveCodeEditor } from '../../../editor/browser/editorBrowser.js'; import { ICodeEditorService } from '../../../editor/browser/services/codeEditorService.js'; import { IEditor } from '../../../editor/common/editorCommon.js'; @@ -274,13 +274,12 @@ class MainThreadDocumentAndEditorStateComputer { } @extHostCustomer -export class MainThreadDocumentsAndEditors implements IMainThreadEditorLocator { +export class MainThreadDocumentsAndEditors extends Disposable implements IMainThreadEditorLocator { - private readonly _toDispose = new DisposableStore(); private readonly _proxy: ExtHostDocumentsAndEditorsShape; private readonly _mainThreadDocuments: MainThreadDocuments; private readonly _mainThreadEditors: MainThreadTextEditors; - private readonly _textEditors = new Map(); + private readonly _textEditors = this._register(new DisposableMap()); constructor( extHostContext: IExtHostContext, @@ -300,20 +299,17 @@ export class MainThreadDocumentsAndEditors implements IMainThreadEditorLocator { @IConfigurationService configurationService: IConfigurationService, @IQuickDiffModelService quickDiffModelService: IQuickDiffModelService ) { + super(); this._proxy = extHostContext.getProxy(ExtHostContext.ExtHostDocumentsAndEditors); - this._mainThreadDocuments = this._toDispose.add(new MainThreadDocuments(extHostContext, this._modelService, this._textFileService, fileService, textModelResolverService, environmentService, uriIdentityService, workingCopyFileService, pathService)); + this._mainThreadDocuments = this._register(new MainThreadDocuments(extHostContext, this._modelService, this._textFileService, fileService, textModelResolverService, environmentService, uriIdentityService, workingCopyFileService, pathService)); extHostContext.set(MainContext.MainThreadDocuments, this._mainThreadDocuments); - this._mainThreadEditors = this._toDispose.add(new MainThreadTextEditors(this, extHostContext, codeEditorService, this._editorService, this._editorGroupService, configurationService, quickDiffModelService, uriIdentityService)); + this._mainThreadEditors = this._register(new MainThreadTextEditors(this, extHostContext, codeEditorService, this._editorService, this._editorGroupService, configurationService, quickDiffModelService, uriIdentityService)); extHostContext.set(MainContext.MainThreadTextEditors, this._mainThreadEditors); // It is expected that the ctor of the state computer calls our `_onDelta`. - this._toDispose.add(new MainThreadDocumentAndEditorStateComputer(delta => this._onDelta(delta), _modelService, codeEditorService, this._editorService, paneCompositeService)); - } - - dispose(): void { - this._toDispose.dispose(); + this._register(new MainThreadDocumentAndEditorStateComputer(delta => this._onDelta(delta), _modelService, codeEditorService, this._editorService, paneCompositeService)); } private _onDelta(delta: DocumentAndEditorStateDelta): void { @@ -337,8 +333,7 @@ export class MainThreadDocumentsAndEditors implements IMainThreadEditorLocator { for (const { id } of delta.removedEditors) { const mainThreadEditor = this._textEditors.get(id); if (mainThreadEditor) { - mainThreadEditor.dispose(); - this._textEditors.delete(id); + this._textEditors.deleteAndDispose(id); removedEditors.push(id); } } diff --git a/src/vs/workbench/api/test/browser/mainThreadDocumentsAndEditors.test.ts b/src/vs/workbench/api/test/browser/mainThreadDocumentsAndEditors.test.ts index e60b2fbc335ecc..7d5260f95cbe65 100644 --- a/src/vs/workbench/api/test/browser/mainThreadDocumentsAndEditors.test.ts +++ b/src/vs/workbench/api/test/browser/mainThreadDocumentsAndEditors.test.ts @@ -40,6 +40,7 @@ import { IQuickDiffModelService } from '../../../contrib/scm/browser/quickDiffMo import { ITextEditorDiffInformation } from '../../../../platform/editor/common/editor.js'; import { ITreeSitterLibraryService } from '../../../../editor/common/services/treeSitter/treeSitterLibraryService.js'; import { TestTreeSitterLibraryService } from '../../../../editor/test/common/services/testTreeSitterLibraryService.js'; +import { URI } from '../../../../base/common/uri.js'; suite('MainThreadDocumentsAndEditors', () => { @@ -48,6 +49,8 @@ suite('MainThreadDocumentsAndEditors', () => { let modelService: ModelService; let codeEditorService: TestCodeEditorService; let textFileService: ITextFileService; + let mainThreadDocumentsAndEditors: MainThreadDocumentsAndEditors; + let propertyChanges: number; const deltas: IDocumentsAndEditorsDelta[] = []; function myCreateTestCodeEditor(model: ITextModel | undefined): ITestCodeEditor { @@ -63,6 +66,7 @@ suite('MainThreadDocumentsAndEditors', () => { disposables = new DisposableStore(); deltas.length = 0; + propertyChanges = 0; const configService = new TestConfigurationService(); configService.setUserConfiguration('editor', { 'detectIndentation': false }); const dialogService = new TestDialogService(); @@ -105,10 +109,11 @@ suite('MainThreadDocumentsAndEditors', () => { override onDidChangeFileSystemProviderRegistrations = Event.None; }; - new MainThreadDocumentsAndEditors( + mainThreadDocumentsAndEditors = disposables.add(new MainThreadDocumentsAndEditors( SingleProxyRPCProtocol({ $acceptDocumentsAndEditorsDelta: (delta: IDocumentsAndEditorsDelta) => { deltas.push(delta); }, - $acceptEditorDiffInformation: (id: string, diffInformation: ITextEditorDiffInformation | undefined) => { } + $acceptEditorDiffInformation: (id: string, diffInformation: ITextEditorDiffInformation | undefined) => { }, + $acceptEditorPropertiesChanged: () => { propertyChanges++; } }), modelService, textFileService, @@ -139,7 +144,7 @@ suite('MainThreadDocumentsAndEditors', () => { return undefined; } } - ); + )); }); teardown(() => { @@ -261,6 +266,58 @@ suite('MainThreadDocumentsAndEditors', () => { model.dispose(); }); + test('fires expected add/remove events on editor lifecycle', () => { + deltas.length = 0; + + const removedEditorEventsFromService: string[] = []; + const removedViaEditorDispose: string[] = []; + + const model = modelService.createModel('farboo', null); + const editor = myCreateTestCodeEditor(model); + const editorId = `${editor.getId()},${model.id}`; + + disposables.add(codeEditorService.onCodeEditorRemove((editorToRemove) => { + removedEditorEventsFromService.push(editorToRemove.getId()); + })); + disposables.add(editor.onDidDispose(() => { + removedViaEditorDispose.push(editor.getId()); + })); + + assert.strictEqual(deltas.length, 2); + + const addedDocumentDelta = deltas.find((delta) => delta.addedDocuments?.length === 1); + assert.ok(addedDocumentDelta); + assert.strictEqual(URI.revive(addedDocumentDelta.addedDocuments![0].uri).toString(), model.uri.toString()); + assert.strictEqual(addedDocumentDelta.addedEditors, undefined); + assert.strictEqual(addedDocumentDelta.removedEditors, undefined); + assert.strictEqual(addedDocumentDelta.removedDocuments, undefined); + + const addedEditorDelta = deltas.find((delta) => delta.addedEditors?.some((editorDto) => editorDto.id === editorId)); + assert.ok(addedEditorDelta); + assert.strictEqual(mainThreadDocumentsAndEditors.getIdOfCodeEditor(editor), editorId); + assert.strictEqual(addedEditorDelta.addedEditors?.length, 1); + assert.strictEqual(addedEditorDelta.addedEditors![0].id, editorId); + assert.strictEqual(URI.revive(addedEditorDelta.addedEditors![0].documentUri).toString(), model.uri.toString()); + + editor.dispose(); + + assert.deepStrictEqual(removedViaEditorDispose, [editor.getId()]); + assert.deepStrictEqual(removedEditorEventsFromService, [editor.getId()]); + const removedEditorDelta = deltas.find((delta) => delta.removedEditors?.includes(editorId)); + assert.ok(removedEditorDelta); + assert.deepStrictEqual(removedEditorDelta.removedEditors, [editorId]); + assert.strictEqual(removedEditorDelta.removedDocuments, undefined); + + assert.strictEqual(mainThreadDocumentsAndEditors.getIdOfCodeEditor(editor), undefined); + assert.strictEqual(mainThreadDocumentsAndEditors.getEditor(editorId), undefined); + + model.dispose(); + + const removedDocumentDelta = deltas.find((delta) => delta.removedDocuments?.some((uri) => URI.revive(uri).toString() === model.uri.toString())); + assert.ok(removedDocumentDelta); + assert.deepStrictEqual(removedDocumentDelta.removedDocuments?.map((uri) => URI.revive(uri).toString()), [model.uri.toString()]); + }); + test('editor with dispos-ed/-ing model', () => { const model = modelService.createModel('farboo', null); const editor = myCreateTestCodeEditor(model); @@ -281,4 +338,29 @@ suite('MainThreadDocumentsAndEditors', () => { editor.dispose(); model.dispose(); }); + + test('dispose removes editor listeners', () => { + const model = modelService.createModel('farboo', null); + const editor = myCreateTestCodeEditor(model); + const mainThreadTextEditor = mainThreadDocumentsAndEditors.getEditor(`${editor.getId()},${model.id}`); + assert.ok(mainThreadTextEditor); + + let directPropertyChanges = 0; + disposables.add(mainThreadTextEditor.onPropertiesChanged(() => directPropertyChanges++)); + const propertyChangesBeforeUpdate = propertyChanges; + const directPropertyChangesBeforeUpdate = directPropertyChanges; + editor.updateOptions({ lineNumbers: 'off' }); + assert.ok(propertyChanges > propertyChangesBeforeUpdate); + assert.ok(directPropertyChanges > directPropertyChangesBeforeUpdate); + const propertyChangesAfterUpdate = propertyChanges; + const directPropertyChangesAfterUpdate = directPropertyChanges; + + mainThreadDocumentsAndEditors.dispose(); + editor.updateOptions({ lineNumbers: 'on' }); + assert.strictEqual(propertyChanges, propertyChangesAfterUpdate); + assert.strictEqual(directPropertyChanges, directPropertyChangesAfterUpdate); + + editor.dispose(); + model.dispose(); + }); }); diff --git a/src/vs/workbench/browser/labels.ts b/src/vs/workbench/browser/labels.ts index a414a3d0fcfbb0..8617e1deabc92c 100644 --- a/src/vs/workbench/browser/labels.ts +++ b/src/vs/workbench/browser/labels.ts @@ -12,7 +12,7 @@ import { IWorkspaceContextService } from '../../platform/workspace/common/worksp import { IConfigurationService } from '../../platform/configuration/common/configuration.js'; import { IModelService } from '../../editor/common/services/model.js'; import { ITextFileService } from '../services/textfile/common/textfiles.js'; -import { IDecoration, IDecorationsService, IResourceDecorationChangeEvent } from '../services/decorations/common/decorations.js'; +import { DECORATION_LABEL_COLOR_CLASS, IDecoration, IDecorationsService, IResourceDecorationChangeEvent } from '../services/decorations/common/decorations.js'; import { Schemas } from '../../base/common/network.js'; import { FileKind, FILES_ASSOCIATIONS_CONFIG } from '../../platform/files/common/files.js'; import { ITextModel } from '../../editor/common/model.js'; @@ -705,7 +705,7 @@ class ResourceLabelWidget extends IconLabel { } if (this.options.fileDecorations.colors) { - iconLabelOptions.extraClasses.push(decoration.labelClassName); + iconLabelOptions.extraClasses.push(DECORATION_LABEL_COLOR_CLASS, decoration.labelClassName); } if (this.options.fileDecorations.badges) { diff --git a/src/vs/workbench/browser/parts/activitybar/activitybarPart.ts b/src/vs/workbench/browser/parts/activitybar/activitybarPart.ts index ae33b9f8e79a78..f5d6a69b845fb0 100644 --- a/src/vs/workbench/browser/parts/activitybar/activitybarPart.ts +++ b/src/vs/workbench/browser/parts/activitybar/activitybarPart.ts @@ -14,7 +14,7 @@ import { IInstantiationService, ServicesAccessor } from '../../../../platform/in import { DisposableStore, MutableDisposable } from '../../../../base/common/lifecycle.js'; import { ToggleSidebarPositionAction, ToggleSidebarVisibilityAction } from '../../actions/layoutActions.js'; import { IThemeService, IColorTheme, registerThemingParticipant } from '../../../../platform/theme/common/themeService.js'; -import { ACTIVITY_BAR_BACKGROUND, ACTIVITY_BAR_BORDER, ACTIVITY_BAR_FOREGROUND, ACTIVITY_BAR_ACTIVE_BORDER, ACTIVITY_BAR_BADGE_BACKGROUND, ACTIVITY_BAR_BADGE_FOREGROUND, ACTIVITY_BAR_INACTIVE_FOREGROUND, ACTIVITY_BAR_ACTIVE_BACKGROUND, ACTIVITY_BAR_DRAG_AND_DROP_BORDER, ACTIVITY_BAR_ACTIVE_FOCUS_BORDER } from '../../../common/theme.js'; +import { ACTIVITY_BAR_BACKGROUND, ACTIVITY_BAR_BORDER, ACTIVITY_BAR_FOREGROUND, ACTIVITY_BAR_ACTIVE_BORDER, ACTIVITY_BAR_BADGE_BACKGROUND, ACTIVITY_BAR_BADGE_FOREGROUND, ACTIVITY_BAR_INACTIVE_FOREGROUND, ACTIVITY_BAR_ACTIVE_BACKGROUND, ACTIVITY_BAR_DRAG_AND_DROP_BORDER, ACTIVITY_BAR_ACTIVE_FOCUS_BORDER, MODERN_ACTIVITY_BAR_BACKGROUND, MODERN_ACTIVITY_BAR_INACTIVE_BACKGROUND } from '../../../common/theme.js'; import { activeContrastBorder, contrastBorder, focusBorder } from '../../../../platform/theme/common/colorRegistry.js'; import { addDisposableListener, append, EventType, isAncestor, $, clearNode } from '../../../../base/browser/dom.js'; import { assertReturnsDefined } from '../../../../base/common/types.js'; @@ -40,6 +40,7 @@ import { IExtensionService } from '../../../services/extensions/common/extension import { IWorkbenchEnvironmentService } from '../../../services/environment/common/environmentService.js'; import { IViewsService } from '../../../services/views/common/viewsService.js'; import { SwitchCompositeViewAction } from '../compositeBarActions.js'; +import { IHostService } from '../../../services/host/browser/host.js'; export class ActivitybarPart extends Part { @@ -104,6 +105,7 @@ export class ActivitybarPart extends Part { private readonly compositeBar = this._register(new MutableDisposable()); private content: HTMLElement | undefined; private _isCompact: boolean; + private isInactive: boolean; constructor( private readonly location: ViewContainerLocation, @@ -113,10 +115,12 @@ export class ActivitybarPart extends Part { @IThemeService themeService: IThemeService, @IStorageService storageService: IStorageService, @IConfigurationService private readonly configurationService: IConfigurationService, + @IHostService private readonly hostService: IHostService, ) { super(Parts.ACTIVITYBAR_PART, { hasTitle: false }, themeService, storageService, layoutService); this._isCompact = this.configurationService.getValue(LayoutSettings.ACTIVITY_BAR_COMPACT) ?? false; + this.isInactive = !this.hostService.hasFocus; this._register(this.configurationService.onDidChangeConfiguration(e => { if (e.affectsConfiguration(LayoutSettings.ACTIVITY_BAR_COMPACT)) { @@ -132,8 +136,25 @@ export class ActivitybarPart extends Part { this.updateCompactStyle(); this.recreateCompositeBar(); this._onDidChange.fire(undefined); + if (this.element) { + this.updateStyles(); + } } })); + + this._register(this.hostService.onDidChangeFocus(focused => this.setInactive(!focused))); + this._register(this.hostService.onDidChangeActiveWindow(windowId => this.setInactive(windowId !== mainWindow.vscodeWindowId))); + } + + private setInactive(inactive: boolean): void { + if (this.isInactive === inactive) { + return; + } + + this.isInactive = inactive; + if (this.element) { + this.updateStyles(); + } } private updateCompactStyle(): void { @@ -228,7 +249,11 @@ export class ActivitybarPart extends Part { super.updateStyles(); const container = assertReturnsDefined(this.getContainer()); - const background = this.getColor(ACTIVITY_BAR_BACKGROUND) || ''; + let backgroundColor = ACTIVITY_BAR_BACKGROUND; + if (this.configurationService.getValue(LayoutSettings.MODERN_UI) === true) { + backgroundColor = this.isInactive ? MODERN_ACTIVITY_BAR_INACTIVE_BACKGROUND : MODERN_ACTIVITY_BAR_BACKGROUND; + } + const background = this.getColor(backgroundColor) || ''; container.style.backgroundColor = background; const borderColor = this.getColor(ACTIVITY_BAR_BORDER) || this.getColor(contrastBorder) || ''; diff --git a/src/vs/workbench/common/theme.ts b/src/vs/workbench/common/theme.ts index 3667118f25146c..3db14376cd7982 100644 --- a/src/vs/workbench/common/theme.ts +++ b/src/vs/workbench/common/theme.ts @@ -718,6 +718,10 @@ export const MODERN_EDITOR_TAB_SELECTED_ACTION_BACKGROUND = registerColor('moder // < --- Modern Activity Bar --- > +export const MODERN_ACTIVITY_BAR_BACKGROUND = registerColor('modernActivityBar.background', ACTIVITY_BAR_BACKGROUND, localize('modernActivityBarBackground', "Background color of the Activity bar in the default side position when the modern UI is enabled.")); + +export const MODERN_ACTIVITY_BAR_INACTIVE_BACKGROUND = registerColor('modernActivityBar.inactiveBackground', MODERN_ACTIVITY_BAR_BACKGROUND, localize('modernActivityBarInactiveBackground', "Background color of the Activity bar in an inactive window when it is in the default side position and the modern UI is enabled.")); + export const MODERN_ACTIVITY_BAR_ACTIVE_BACKGROUND = registerColor('modernActivityBar.activeBackground', MODERN_TAB_ACTIVE_BACKGROUND, localize('modernActivityBarActiveBackground', "Background color of active Activity bar items in the default side position when the modern UI is enabled.")); export const MODERN_ACTIVITY_BAR_ACTIVE_FOREGROUND = registerColor('modernActivityBar.activeForeground', MODERN_TAB_ACTIVE_FOREGROUND, localize('modernActivityBarActiveForeground', "Foreground color of active Activity bar items in the default side position when the modern UI is enabled.")); diff --git a/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/media/chatInlineAnchorWidget.css b/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/media/chatInlineAnchorWidget.css index 5f14c5bd2b11bf..39fcd7ef98b55d 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/media/chatInlineAnchorWidget.css +++ b/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/media/chatInlineAnchorWidget.css @@ -45,6 +45,9 @@ } .show-file-icons.chat-inline-anchor-widget .icon::before { + --file-icon-mask-position: center; + --file-icon-mask-size: contain; + display: inline-block; line-height: 100%; overflow: hidden; diff --git a/src/vs/workbench/contrib/files/browser/views/explorerView.ts b/src/vs/workbench/contrib/files/browser/views/explorerView.ts index 89afdf54753331..3c8c088e3d74d6 100644 --- a/src/vs/workbench/contrib/files/browser/views/explorerView.ts +++ b/src/vs/workbench/contrib/files/browser/views/explorerView.ts @@ -12,7 +12,7 @@ import { IFilesConfiguration, ExplorerFolderContext, FilesExplorerFocusedContext import { FileCopiedContext, NEW_FILE_COMMAND_ID, NEW_FOLDER_COMMAND_ID } from '../fileActions.js'; import * as DOM from '../../../../../base/browser/dom.js'; import { IWorkbenchLayoutService } from '../../../../services/layout/browser/layoutService.js'; -import { IWorkspaceContextService, WorkbenchState } from '../../../../../platform/workspace/common/workspace.js'; +import { isUntitledWorkspace, IWorkspace, IWorkspaceContextService, WorkbenchState } from '../../../../../platform/workspace/common/workspace.js'; import { IConfigurationService, IConfigurationChangeEvent } from '../../../../../platform/configuration/common/configuration.js'; import { IKeybindingService } from '../../../../../platform/keybinding/common/keybinding.js'; import { IInstantiationService, ServicesAccessor } from '../../../../../platform/instantiation/common/instantiation.js'; @@ -38,7 +38,7 @@ import { IAsyncDataTreeViewState } from '../../../../../base/browser/ui/tree/asy import { FuzzyScore } from '../../../../../base/common/filters.js'; import { IClipboardService } from '../../../../../platform/clipboard/common/clipboardService.js'; import { IFileService, FileSystemProviderCapabilities } from '../../../../../platform/files/common/files.js'; -import { IDisposable } from '../../../../../base/common/lifecycle.js'; +import { IDisposable, toDisposable } from '../../../../../base/common/lifecycle.js'; import { Event } from '../../../../../base/common/event.js'; import { IViewDescriptorService } from '../../../../common/views.js'; import { IViewsService } from '../../../../services/views/common/viewsService.js'; @@ -54,6 +54,7 @@ import { ResourceMap } from '../../../../../base/common/map.js'; import { AbstractTreePart } from '../../../../../base/browser/ui/tree/abstractTree.js'; import { IHoverService } from '../../../../../platform/hover/browser/hover.js'; import { IAccessibilityService } from '../../../../../platform/accessibility/common/accessibility.js'; +import { IEnvironmentService } from '../../../../../platform/environment/common/environment.js'; function hasExpandedRootChild(tree: WorkbenchCompressibleAsyncDataTree, treeInput: ExplorerItem[]): boolean { @@ -149,7 +150,32 @@ export interface IExplorerViewPaneOptions extends IViewPaneOptions { delegate: IExplorerViewContainerDelegate; } +/** + * Marks the Explorer pane header as showing a name the user chose. + */ +export const PRESERVE_WORKSPACE_NAME_CASE_CLASS = 'preserve-workspace-name-case'; + +/** + * Marks the part hosting the Explorer as showing a name the user chose in its + * merged (single view) title. + */ +export const PRESERVE_MERGED_WORKSPACE_NAME_CASE_CLASS = 'preserve-merged-workspace-name-case'; + +/** + * Whether the Explorer title shows a name the user provided and therefore has to + * be rendered with its original casing. Untitled workspaces show a generated + * label and empty workbenches show a static label, so both keep the default casing. + */ +export function shouldPreserveWorkspaceNameCase(workbenchState: WorkbenchState, workspace: IWorkspace, environmentService: IEnvironmentService): boolean { + if (workbenchState === WorkbenchState.EMPTY) { + return false; + } + + return !workspace.configuration || !isUntitledWorkspace(workspace.configuration, environmentService); +} + export class ExplorerView extends ViewPane implements IExplorerView { + static readonly TREE_VIEW_STATE_STORAGE_KEY: string = 'workbench.explorer.treeViewState'; private tree!: WorkbenchCompressibleAsyncDataTree; @@ -182,6 +208,7 @@ export class ExplorerView extends ViewPane implements IExplorerView { private dragHandler!: DelayedDragHandler; private _autoReveal: boolean | 'force' | 'focusNoScroll' = false; private readonly delegate: IExplorerViewContainerDelegate | undefined; + private workspaceTitleContainer: HTMLElement | undefined; override get singleViewPaneContainerTitle(): string { return this.name; @@ -211,7 +238,8 @@ export class ExplorerView extends ViewPane implements IExplorerView { @IUriIdentityService private readonly uriIdentityService: IUriIdentityService, @ICommandService private readonly commandService: ICommandService, @IOpenerService openerService: IOpenerService, - @IAccessibilityService private readonly accessibilityService: IAccessibilityService + @IAccessibilityService private readonly accessibilityService: IAccessibilityService, + @IEnvironmentService private readonly environmentService: IEnvironmentService ) { super(options, keybindingService, contextMenuService, configurationService, contextKeyService, viewDescriptorService, instantiationService, openerService, themeService, hoverService); @@ -231,8 +259,8 @@ export class ExplorerView extends ViewPane implements IExplorerView { this.viewHasSomeCollapsibleRootItem = ViewHasSomeCollapsibleRootItemContext.bindTo(contextKeyService); this.viewVisibleContextKey = FoldersViewVisibleContext.bindTo(contextKeyService); - this.explorerService.registerView(this); + this._register(toDisposable(() => this.clearWorkspaceTitleContainer())); } get autoReveal() { @@ -255,9 +283,24 @@ export class ExplorerView extends ViewPane implements IExplorerView { // noop } + override get headerVisible(): boolean { + return super.headerVisible; + } + + override set headerVisible(visible: boolean) { + super.headerVisible = visible; + this.updateWorkspaceTitleCase(); + } + override setVisible(visible: boolean): void { + if (!visible) { + this.clearWorkspaceTitleContainer(); + } this.viewVisibleContextKey.set(visible); super.setVisible(visible); + if (visible) { + this.updateWorkspaceTitleContainer(); + } } @memoize private get fileCopiedContextKey(): IContextKey { @@ -285,11 +328,35 @@ export class ExplorerView extends ViewPane implements IExplorerView { titleElement.setAttribute('aria-label', this.ariaHeaderLabel); }; - this._register(this.contextService.onDidChangeWorkspaceName(setHeader)); + this._register(this.contextService.onDidChangeWorkspaceName(() => { + setHeader(); + this.updateWorkspaceTitleCase(); + })); + this._register(this.contextService.onDidChangeWorkbenchState(() => this.updateWorkspaceTitleCase())); this._register(this.labelService.onDidChangeFormatters(setHeader)); setHeader(); } + private updateWorkspaceTitleContainer(): void { + const workspaceTitleContainer = DOM.findParentWithClass(this.element, 'part') ?? undefined; + if (this.workspaceTitleContainer !== workspaceTitleContainer) { + this.workspaceTitleContainer?.classList.remove(PRESERVE_MERGED_WORKSPACE_NAME_CASE_CLASS); + this.workspaceTitleContainer = workspaceTitleContainer; + } + this.updateWorkspaceTitleCase(); + } + + private clearWorkspaceTitleContainer(): void { + this.workspaceTitleContainer?.classList.remove(PRESERVE_MERGED_WORKSPACE_NAME_CASE_CLASS); + this.workspaceTitleContainer = undefined; + } + + private updateWorkspaceTitleCase(): void { + const preserveWorkspaceNameCase = shouldPreserveWorkspaceNameCase(this.contextService.getWorkbenchState(), this.contextService.getWorkspace(), this.environmentService); + this.element.classList.toggle(PRESERVE_WORKSPACE_NAME_CASE_CLASS, preserveWorkspaceNameCase); + this.workspaceTitleContainer?.classList.toggle(PRESERVE_MERGED_WORKSPACE_NAME_CASE_CLASS, preserveWorkspaceNameCase && this.isVisible() && !this.headerVisible); + } + protected override layoutBody(height: number, width: number): void { super.layoutBody(height, width); this.tree.layout(height, width); diff --git a/src/vs/workbench/contrib/files/test/browser/explorerView.test.ts b/src/vs/workbench/contrib/files/test/browser/explorerView.test.ts index 7a531dd70f1035..68b271724a40f3 100644 --- a/src/vs/workbench/contrib/files/test/browser/explorerView.test.ts +++ b/src/vs/workbench/contrib/files/test/browser/explorerView.test.ts @@ -7,7 +7,7 @@ import assert from 'assert'; import { Emitter } from '../../../../../base/common/event.js'; import { ensureNoDisposablesAreLeakedInTestSuite, toResource } from '../../../../../base/test/common/utils.js'; import { ExplorerItem } from '../../common/explorerModel.js'; -import { getContext } from '../../browser/views/explorerView.js'; +import { getContext, shouldPreserveWorkspaceNameCase } from '../../browser/views/explorerView.js'; import { listInvalidItemForeground } from '../../../../../platform/theme/common/colorRegistry.js'; import { CompressedNavigationController } from '../../browser/views/explorerViewer.js'; import * as dom from '../../../../../base/browser/dom.js'; @@ -15,6 +15,10 @@ import { DisposableStore } from '../../../../../base/common/lifecycle.js'; import { provideDecorations } from '../../browser/views/explorerDecorationsProvider.js'; import { TestConfigurationService } from '../../../../../platform/configuration/test/common/testConfigurationService.js'; import { NullFilesConfigurationService, TestFileService } from '../../../../test/common/workbenchTestServices.js'; +import { TestEnvironmentService } from '../../../../test/browser/workbenchTestServices.js'; +import { URI } from '../../../../../base/common/uri.js'; +import { IWorkspace, WorkbenchState } from '../../../../../platform/workspace/common/workspace.js'; +import { joinPath } from '../../../../../base/common/resources.js'; suite('Files - ExplorerView', () => { @@ -69,6 +73,27 @@ suite('Files - ExplorerView', () => { }); }); + test('preserves workspace name case only for user named workspaces', async function () { + const untitledWorkspacesHome = TestEnvironmentService.untitledWorkspacesHome; + function workspace(configuration: URI | null): IWorkspace { + return { id: 'test', folders: [], configuration }; + } + + assert.deepStrictEqual({ + empty: shouldPreserveWorkspaceNameCase(WorkbenchState.EMPTY, workspace(null), TestEnvironmentService), + folder: shouldPreserveWorkspaceNameCase(WorkbenchState.FOLDER, workspace(null), TestEnvironmentService), + untitled: shouldPreserveWorkspaceNameCase(WorkbenchState.WORKSPACE, workspace(joinPath(untitledWorkspacesHome, '1234', 'workspace.json')), TestEnvironmentService), + untitledDifferentCase: shouldPreserveWorkspaceNameCase(WorkbenchState.WORKSPACE, workspace(joinPath(untitledWorkspacesHome.with({ path: untitledWorkspacesHome.path.toUpperCase() }), '1234', 'workspace.json')), TestEnvironmentService), + named: shouldPreserveWorkspaceNameCase(WorkbenchState.WORKSPACE, workspace(URI.file('/some/path/myWorkspace.code-workspace')), TestEnvironmentService), + }, { + empty: false, + folder: true, + untitled: false, + untitledDifferentCase: false, + named: true, + }); + }); + test('compressed navigation controller', async function () { const container = $('.file'); const label = $('.label'); diff --git a/src/vs/workbench/contrib/modernUI/README.md b/src/vs/workbench/contrib/modernUI/README.md index b401d357f21274..e0969fb07cddf4 100644 --- a/src/vs/workbench/contrib/modernUI/README.md +++ b/src/vs/workbench/contrib/modernUI/README.md @@ -22,6 +22,8 @@ Modern UI uses the standard workbench color theme system. Theme authors can use | `modernEditorTab.hoverActionBackground` | Opaque background of actions on hovered Modern UI editor tabs | `modernEditorTab.hoverBackground` composited over `editor.background` | | `modernEditorTab.hoverForeground` | Foreground of hovered Modern UI editor tabs | `modernTab.hoverForeground` | | `modernEditorTab.selectedActionBackground` | Opaque background of actions on selected Modern UI editor tabs | `tab.selectedBackground` composited over `editor.background` | +| `modernActivityBar.background` | Background of the Modern UI activity bar | `activityBar.background` | +| `modernActivityBar.inactiveBackground` | Background of the Modern UI activity bar in an inactive window | `modernActivityBar.background` | | `modernActivityBar.activeBackground` | Background of active Modern UI activity bar items in the default side position | `modernTab.activeBackground` | | `modernActivityBar.activeForeground` | Foreground of active Modern UI activity bar items in the default side position | `modernTab.activeForeground` | | `modernActivityBar.hoverBackground` | Background of hovered Modern UI activity bar items in the default side position | `modernTab.hoverBackground` | @@ -52,6 +54,8 @@ Activity bar items in non-default top or bottom positions use the `modernTab.*` "modernEditorTab.hoverActionBackground": "#323232", "modernEditorTab.hoverForeground": "#ffffff", "modernEditorTab.selectedActionBackground": "#454545", + "modernActivityBar.background": "#181818", + "modernActivityBar.inactiveBackground": "#202020", "modernActivityBar.activeBackground": "#3d3d3d", "modernActivityBar.activeForeground": "#f0f0f0", "modernActivityBar.hoverBackground": "#292929", diff --git a/src/vs/workbench/contrib/modernUI/browser/media/fontRamp.css b/src/vs/workbench/contrib/modernUI/browser/media/fontRamp.css index 8e0d12e1ab6a82..a1a8fc73898ba4 100644 --- a/src/vs/workbench/contrib/modernUI/browser/media/fontRamp.css +++ b/src/vs/workbench/contrib/modernUI/browser/media/fontRamp.css @@ -134,15 +134,11 @@ } /* - * File Explorer titles — respect the real casing of the workspace / folder name - * (user input). `text-transform: none` overrides both the base `uppercase` rule - * and the `capitalize` override above. System labels such as "Untitled - * (Workspace)" are already title-cased in source, so they still render - * correctly; only user-provided folder names (e.g. "vscode-dev") are left - * untouched. + * Named File Explorer workspace titles preserve the casing supplied by the + * user. Untitled workspaces continue to use the standard capitalized header. */ -.modern-ui.monaco-workbench .part:not(.editor)[data-active-composite="workbench.view.explorer"] > .title > .title-label h2, -.modern-ui .monaco-pane-view .pane > .pane-header > .icon.codicon-explorer-view-icon + .title { +.modern-ui.monaco-workbench .part.preserve-merged-workspace-name-case > .title > .title-label h2, +.modern-ui .pane.preserve-workspace-name-case > .pane-header > .icon.codicon-explorer-view-icon + .title { text-transform: none; } diff --git a/src/vs/workbench/contrib/modernUI/browser/media/padding.css b/src/vs/workbench/contrib/modernUI/browser/media/padding.css index e8b3e82eaebede..a634c001e609bb 100644 --- a/src/vs/workbench/contrib/modernUI/browser/media/padding.css +++ b/src/vs/workbench/contrib/modernUI/browser/media/padding.css @@ -259,6 +259,11 @@ overflow: visible; } +/* Remove classic 1px top border that shifts 32px pills off-center (#331013) */ +.modern-ui.monaco-workbench .part.panel.bottom .composite.title { + border-top: none; +} + .modern-ui.monaco-workbench .part.basepanel > .composite.title > .title-actions { min-width: 0; } diff --git a/src/vs/workbench/contrib/modernUI/browser/media/tabs.css b/src/vs/workbench/contrib/modernUI/browser/media/tabs.css index e8bad7c5910f0b..3921e331f769a1 100644 --- a/src/vs/workbench/contrib/modernUI/browser/media/tabs.css +++ b/src/vs/workbench/contrib/modernUI/browser/media/tabs.css @@ -89,7 +89,8 @@ row-gap: 0; } -.modern-ui-tabs.monaco-workbench .part.editor > .content .editor-group-container > .title .tabs-container > .tab:not(.selected):hover { +.modern-ui-tabs.monaco-workbench .part.editor > .content .editor-group-container.active > .title .tabs-container > .tab:not(.selected):hover, +.modern-ui-tabs.monaco-workbench .part.editor > .content .editor-group-container:not(.active) > .title .tabs-container > .tab:not(.selected):hover { background-color: transparent !important; } @@ -902,8 +903,14 @@ * Skip labels that carry a file-decoration color class so git/problem * decoration colors still apply to the entire tab label (name + description), * matching classic UI behavior. See https://github.com/microsoft/vscode/issues/325246 + * + * `.monaco-decoration-itemColor` is the stable marker the resource label sets + * next to the generated, per-color decoration class (DECORATION_LABEL_COLOR_CLASS + * in vs/workbench/services/decorations/common/decorations.ts). Matching the + * hashed class with `[class*="..."]` instead would make every class mutation in + * the workbench pay style invalidation for these rules. */ -.modern-ui-tabs .part.editor > .content .editor-group-container.active > .title .tabs-container > .tab:not(.active) .tab-label:not([class*="monaco-decoration-itemColor"]) a { +.modern-ui-tabs .part.editor > .content .editor-group-container.active > .title .tabs-container > .tab:not(.active) .tab-label:not(.monaco-decoration-itemColor) a { color: var(--modern-ui-editor-tab-inactive-foreground) !important; } @@ -911,12 +918,12 @@ color: var(--modern-ui-editor-tab-inactive-foreground) !important; } -.modern-ui-tabs .part.editor > .content .editor-group-container:not(.active) > .title .tabs-container > .tab:not(.active) .tab-label:not([class*="monaco-decoration-itemColor"]) a, +.modern-ui-tabs .part.editor > .content .editor-group-container:not(.active) > .title .tabs-container > .tab:not(.active) .tab-label:not(.monaco-decoration-itemColor) a, .modern-ui-tabs .modern-ui-editor-tab-group:not(.modern-ui-editor-tab-group-active) .modern-ui-editor-tab:not(.active) { color: var(--modern-ui-editor-tab-unfocused-inactive-foreground) !important; } -.modern-ui-tabs .part.editor > .content .editor-group-container.active > .title .tabs-container > .tab:hover:not(.selected):not(.active) .tab-label:not([class*="monaco-decoration-itemColor"]) a, +.modern-ui-tabs .part.editor > .content .editor-group-container.active > .title .tabs-container > .tab:hover:not(.selected):not(.active) .tab-label:not(.monaco-decoration-itemColor) a, .modern-ui-tabs .part.editor > .content .editor-group-container.active > .title .tabs-container > .tab:hover:not(.selected):not(.active) > .tab-actions .action-label { color: var(--modern-ui-editor-tab-hover-foreground) !important; } @@ -925,13 +932,13 @@ color: var(--modern-ui-editor-tab-hover-foreground) !important; } -.modern-ui-tabs .part.editor > .content .editor-group-container:not(.active) > .title .tabs-container > .tab:hover:not(.selected):not(.active) .tab-label:not([class*="monaco-decoration-itemColor"]) a, +.modern-ui-tabs .part.editor > .content .editor-group-container:not(.active) > .title .tabs-container > .tab:hover:not(.selected):not(.active) .tab-label:not(.monaco-decoration-itemColor) a, .modern-ui-tabs .part.editor > .content .editor-group-container:not(.active) > .title .tabs-container > .tab:hover:not(.selected):not(.active) > .tab-actions .action-label, .modern-ui-tabs .modern-ui-editor-tab-group:not(.modern-ui-editor-tab-group-active) .modern-ui-editor-tab:hover:not(.active) { color: var(--modern-ui-editor-tab-unfocused-hover-foreground) !important; } -.modern-ui-tabs .part.editor > .content .editor-group-container.active > .title .tabs-container > .tab.active .tab-label:not([class*="monaco-decoration-itemColor"]) a, +.modern-ui-tabs .part.editor > .content .editor-group-container.active > .title .tabs-container > .tab.active .tab-label:not(.monaco-decoration-itemColor) a, .modern-ui-tabs .part.editor > .content .editor-group-container.active > .title .tabs-container > .tab.active > .tab-actions .action-label { color: var(--modern-ui-editor-tab-active-foreground) !important; } @@ -940,20 +947,20 @@ color: var(--modern-ui-editor-tab-active-foreground) !important; } -.modern-ui-tabs .part.editor > .content .editor-group-container:not(.active) > .title .tabs-container > .tab.active .tab-label:not([class*="monaco-decoration-itemColor"]) a, +.modern-ui-tabs .part.editor > .content .editor-group-container:not(.active) > .title .tabs-container > .tab.active .tab-label:not(.monaco-decoration-itemColor) a, .modern-ui-tabs .part.editor > .content .editor-group-container:not(.active) > .title .tabs-container > .tab.active > .tab-actions .action-label, .modern-ui-tabs .modern-ui-editor-tab-group:not(.modern-ui-editor-tab-group-active) .modern-ui-editor-tab.active { color: var(--modern-ui-editor-tab-unfocused-active-foreground) !important; } -.modern-ui-tabs .part.editor > .content .editor-group-container > .title .tabs-container > .tab.selected:not(.active) .tab-label:not([class*="monaco-decoration-itemColor"]) a { +.modern-ui-tabs .part.editor > .content .editor-group-container > .title .tabs-container > .tab.selected:not(.active) .tab-label:not(.monaco-decoration-itemColor) a { color: var(--vscode-tab-selectedForeground) !important; } /* Ensure decoration color cascades to every part of the tab label. */ -.modern-ui-tabs .part.editor > .content .editor-group-container > .title .tabs-container > .tab .tab-label[class*="monaco-decoration-itemColor"] a, -.modern-ui-tabs .part.editor > .content .editor-group-container > .title .tabs-container > .tab .tab-label[class*="monaco-decoration-itemColor"] .label-description, -.modern-ui-tabs .part.editor > .content .editor-group-container > .title .tabs-container > .tab .tab-label[class*="monaco-decoration-itemColor"] .monaco-highlighted-label { +.modern-ui-tabs .part.editor > .content .editor-group-container > .title .tabs-container > .tab .tab-label.monaco-decoration-itemColor a, +.modern-ui-tabs .part.editor > .content .editor-group-container > .title .tabs-container > .tab .tab-label.monaco-decoration-itemColor .label-description, +.modern-ui-tabs .part.editor > .content .editor-group-container > .title .tabs-container > .tab .tab-label.monaco-decoration-itemColor .monaco-highlighted-label { color: inherit !important; } diff --git a/src/vs/workbench/contrib/modernUI/test/browser/modernUI.contribution.test.ts b/src/vs/workbench/contrib/modernUI/test/browser/modernUI.contribution.test.ts index 2e1c4c74e3f9ed..c756ba0ea19262 100644 --- a/src/vs/workbench/contrib/modernUI/test/browser/modernUI.contribution.test.ts +++ b/src/vs/workbench/contrib/modernUI/test/browser/modernUI.contribution.test.ts @@ -17,9 +17,13 @@ import { Registry } from '../../../../../platform/registry/common/platform.js'; import { editorBackground, Extensions as ColorRegistryExtensions, IColorRegistry, listHoverBackground, listHoverForeground, listInactiveSelectionBackground, listInactiveSelectionForeground, oneOf, opaque } from '../../../../../platform/theme/common/colorRegistry.js'; import { foreground } from '../../../../../platform/theme/common/colors/baseColors.js'; import { Extensions as ThemeServiceExtensions, IThemingRegistry } from '../../../../../platform/theme/common/themeService.js'; -import { EDITOR_BORDER, MODERN_ACTIVITY_BAR_ACTIVE_BACKGROUND, MODERN_ACTIVITY_BAR_ACTIVE_FOREGROUND, MODERN_ACTIVITY_BAR_HOVER_BACKGROUND, MODERN_ACTIVITY_BAR_HOVER_FOREGROUND, MODERN_EDITOR_TAB_ACTIVE_ACTION_BACKGROUND, MODERN_EDITOR_TAB_ACTIVE_BACKGROUND, MODERN_EDITOR_TAB_ACTIVE_FOREGROUND, MODERN_EDITOR_TAB_ACTIVE_HOVER_ACTION_BACKGROUND, MODERN_EDITOR_TAB_ACTIVE_HOVER_BACKGROUND, MODERN_EDITOR_TAB_HOVER_ACTION_BACKGROUND, MODERN_EDITOR_TAB_HOVER_BACKGROUND, MODERN_EDITOR_TAB_HOVER_FOREGROUND, MODERN_EDITOR_TAB_INACTIVE_BACKGROUND, MODERN_EDITOR_TAB_SELECTED_ACTION_BACKGROUND, MODERN_TAB_ACTIVE_BACKGROUND, MODERN_TAB_ACTIVE_FOREGROUND, MODERN_TAB_HOVER_BACKGROUND, MODERN_TAB_HOVER_FOREGROUND, SURFACE_BORDER, TAB_ACTIVE_BACKGROUND, TAB_ACTIVE_BORDER, TAB_ACTIVE_BORDER_TOP, TAB_ACTIVE_FOREGROUND, TAB_BORDER, TAB_HOVER_BACKGROUND, TAB_HOVER_BORDER, TAB_HOVER_FOREGROUND, TAB_INACTIVE_BACKGROUND, TAB_INACTIVE_FOREGROUND, TAB_LAST_PINNED_BORDER, TAB_SELECTED_BACKGROUND, TAB_UNFOCUSED_HOVER_BACKGROUND } from '../../../../common/theme.js'; +import { EDITOR_BORDER, MODERN_ACTIVITY_BAR_ACTIVE_BACKGROUND, MODERN_ACTIVITY_BAR_ACTIVE_FOREGROUND, MODERN_ACTIVITY_BAR_BACKGROUND, MODERN_ACTIVITY_BAR_HOVER_BACKGROUND, MODERN_ACTIVITY_BAR_HOVER_FOREGROUND, MODERN_ACTIVITY_BAR_INACTIVE_BACKGROUND, MODERN_EDITOR_TAB_ACTIVE_ACTION_BACKGROUND, MODERN_EDITOR_TAB_ACTIVE_BACKGROUND, MODERN_EDITOR_TAB_ACTIVE_FOREGROUND, MODERN_EDITOR_TAB_ACTIVE_HOVER_ACTION_BACKGROUND, MODERN_EDITOR_TAB_ACTIVE_HOVER_BACKGROUND, MODERN_EDITOR_TAB_HOVER_ACTION_BACKGROUND, MODERN_EDITOR_TAB_HOVER_BACKGROUND, MODERN_EDITOR_TAB_HOVER_FOREGROUND, MODERN_EDITOR_TAB_INACTIVE_BACKGROUND, MODERN_EDITOR_TAB_SELECTED_ACTION_BACKGROUND, MODERN_TAB_ACTIVE_BACKGROUND, MODERN_TAB_ACTIVE_FOREGROUND, MODERN_TAB_HOVER_BACKGROUND, MODERN_TAB_HOVER_FOREGROUND, SURFACE_BORDER, TAB_ACTIVE_BACKGROUND, TAB_ACTIVE_BORDER, TAB_ACTIVE_BORDER_TOP, TAB_ACTIVE_FOREGROUND, TAB_BORDER, TAB_HOVER_BACKGROUND, TAB_HOVER_BORDER, TAB_HOVER_FOREGROUND, TAB_INACTIVE_BACKGROUND, TAB_INACTIVE_FOREGROUND, TAB_LAST_PINNED_BORDER, TAB_SELECTED_BACKGROUND, TAB_UNFOCUSED_HOVER_BACKGROUND } from '../../../../common/theme.js'; import { TestEnvironmentService, TestLayoutService } from '../../../../test/browser/workbenchTestServices.js'; import { LayoutSettings } from '../../../../services/layout/browser/layoutService.js'; +import { PRESERVE_MERGED_WORKSPACE_NAME_CASE_CLASS, PRESERVE_WORKSPACE_NAME_CASE_CLASS, shouldPreserveWorkspaceNameCase } from '../../../files/browser/views/explorerView.js'; +import { URI } from '../../../../../base/common/uri.js'; +import { joinPath } from '../../../../../base/common/resources.js'; +import { WorkbenchState } from '../../../../../platform/workspace/common/workspace.js'; import { ColorThemeData } from '../../../../services/themes/common/colorThemeData.js'; import { generateColorThemeCSS } from '../../../../services/themes/browser/colorThemeCss.js'; import '../../../../browser/parts/activitybar/media/activityaction.css'; @@ -282,11 +286,22 @@ suite('ModernUIContribution', () => { const paneHeader = appendElement(appendElement(paneView, 'pane'), 'pane-header'); const paneTitle = appendElement(paneHeader, 'title'); - const explorerPart = appendElement(layoutService.mainContainer, 'part'); - explorerPart.dataset.activeComposite = 'workbench.view.explorer'; + const explorerPart = appendElement(layoutService.mainContainer, `part ${PRESERVE_MERGED_WORKSPACE_NAME_CASE_CLASS}`); + explorerPart.dataset.activeComposite = 'workbench.views.service.sidebar.custom'; const explorerTitleLabel = appendElement(appendElement(explorerPart, 'title'), 'title-label'); const explorerTitle = document.createElement('h2'); explorerTitleLabel.appendChild(explorerTitle); + const explorerPaneHeader = appendElement(appendElement(appendElement(explorerPart, 'monaco-pane-view'), `pane ${PRESERVE_WORKSPACE_NAME_CASE_CLASS}`), 'pane-header'); + appendElement(explorerPaneHeader, 'icon codicon-explorer-view-icon'); + const explorerPaneTitle = appendElement(explorerPaneHeader, 'title'); + const multiViewPart = appendElement(layoutService.mainContainer, 'part'); + multiViewPart.dataset.activeComposite = 'workbench.views.service.sidebar.multiView'; + const multiViewTitleLabel = appendElement(appendElement(multiViewPart, 'title'), 'title-label'); + const multiViewTitle = document.createElement('h2'); + multiViewTitleLabel.appendChild(multiViewTitle); + const multiViewExplorerPaneHeader = appendElement(appendElement(appendElement(multiViewPart, 'monaco-pane-view'), `pane ${PRESERVE_WORKSPACE_NAME_CASE_CLASS}`), 'pane-header'); + appendElement(multiViewExplorerPaneHeader, 'icon codicon-explorer-view-icon'); + const multiViewExplorerPaneTitle = appendElement(multiViewExplorerPaneHeader, 'title'); const extensionsPart = appendElement(layoutService.mainContainer, 'part'); const extensionsTitleLabel = appendElement(appendElement(extensionsPart, 'title'), 'title-label'); const extensionsTitle = document.createElement('h2'); @@ -300,6 +315,9 @@ suite('ModernUIContribution', () => { classApplied: layoutService.mainContainer.classList.contains('modern-ui-uppercase-view-headers'), paneTitleTransform: targetWindow.getComputedStyle(paneTitle).textTransform, explorerTitleTransform: targetWindow.getComputedStyle(explorerTitle).textTransform, + explorerPaneTitleTransform: targetWindow.getComputedStyle(explorerPaneTitle).textTransform, + multiViewTitleTransform: targetWindow.getComputedStyle(multiViewTitle).textTransform, + multiViewExplorerPaneTitleTransform: targetWindow.getComputedStyle(multiViewExplorerPaneTitle).textTransform, extensionsTitleTransform: targetWindow.getComputedStyle(extensionsTitle).textTransform, panelTabTransform: targetWindow.getComputedStyle(panelTab).textTransform, layoutCount: layoutService.layoutCount, @@ -318,6 +336,9 @@ suite('ModernUIContribution', () => { classApplied: layoutService.mainContainer.classList.contains('modern-ui-uppercase-view-headers'), paneTitleTransform: targetWindow.getComputedStyle(paneTitle).textTransform, explorerTitleTransform: targetWindow.getComputedStyle(explorerTitle).textTransform, + explorerPaneTitleTransform: targetWindow.getComputedStyle(explorerPaneTitle).textTransform, + multiViewTitleTransform: targetWindow.getComputedStyle(multiViewTitle).textTransform, + multiViewExplorerPaneTitleTransform: targetWindow.getComputedStyle(multiViewExplorerPaneTitle).textTransform, extensionsTitleTransform: targetWindow.getComputedStyle(extensionsTitle).textTransform, panelTabTransform: targetWindow.getComputedStyle(panelTab).textTransform, layoutCount: layoutService.layoutCount, @@ -326,6 +347,9 @@ suite('ModernUIContribution', () => { classApplied: false, paneTitleTransform: 'capitalize', explorerTitleTransform: 'none', + explorerPaneTitleTransform: 'none', + multiViewTitleTransform: 'capitalize', + multiViewExplorerPaneTitleTransform: 'none', extensionsTitleTransform: 'capitalize', panelTabTransform: 'capitalize', layoutCount: 0, @@ -333,12 +357,72 @@ suite('ModernUIContribution', () => { classApplied: true, paneTitleTransform: 'uppercase', explorerTitleTransform: 'uppercase', + explorerPaneTitleTransform: 'uppercase', + multiViewTitleTransform: 'uppercase', + multiViewExplorerPaneTitleTransform: 'uppercase', extensionsTitleTransform: 'uppercase', panelTabTransform: 'uppercase', layoutCount: 0, }); }); + test('Explorer title casing follows the workspace name decision', () => { + const root = document.createElement('div'); + root.className = 'monaco-workbench modern-ui'; + + function createExplorerTitles(workbenchState: WorkbenchState, configuration: URI | null) { + const preserveCase = shouldPreserveWorkspaceNameCase(workbenchState, { id: 'test', folders: [], configuration }, TestEnvironmentService); + const part = appendElement(root, preserveCase ? `part ${PRESERVE_MERGED_WORKSPACE_NAME_CASE_CLASS}` : 'part'); + const mergedTitle = document.createElement('h2'); + appendElement(appendElement(part, 'title'), 'title-label').appendChild(mergedTitle); + const paneHeader = appendElement(appendElement(appendElement(part, 'monaco-pane-view'), preserveCase ? `pane ${PRESERVE_WORKSPACE_NAME_CASE_CLASS}` : 'pane'), 'pane-header'); + appendElement(paneHeader, 'icon codicon-explorer-view-icon'); + const paneTitle = appendElement(paneHeader, 'title'); + return { mergedTitle, paneTitle }; + } + + const untitled = createExplorerTitles(WorkbenchState.WORKSPACE, joinPath(TestEnvironmentService.untitledWorkspacesHome, '1234', 'workspace.json')); + const named = createExplorerTitles(WorkbenchState.WORKSPACE, URI.file('/some/path/myWorkspace.code-workspace')); + const folder = createExplorerTitles(WorkbenchState.FOLDER, null); + + document.body.appendChild(root); + store.add(toDisposable(() => root.remove())); + const targetWindow = getWindow(root); + const transforms = () => ({ + untitledMerged: targetWindow.getComputedStyle(untitled.mergedTitle).textTransform, + untitledPane: targetWindow.getComputedStyle(untitled.paneTitle).textTransform, + namedMerged: targetWindow.getComputedStyle(named.mergedTitle).textTransform, + namedPane: targetWindow.getComputedStyle(named.paneTitle).textTransform, + folderMerged: targetWindow.getComputedStyle(folder.mergedTitle).textTransform, + folderPane: targetWindow.getComputedStyle(folder.paneTitle).textTransform, + }); + + const defaultTransforms = transforms(); + root.classList.add('modern-ui-uppercase-view-headers'); + + assert.deepStrictEqual({ + defaultTransforms, + uppercaseTransforms: transforms(), + }, { + defaultTransforms: { + untitledMerged: 'capitalize', + untitledPane: 'capitalize', + namedMerged: 'none', + namedPane: 'none', + folderMerged: 'none', + folderPane: 'none', + }, + uppercaseTransforms: { + untitledMerged: 'uppercase', + untitledPane: 'uppercase', + namedMerged: 'uppercase', + namedPane: 'uppercase', + folderMerged: 'uppercase', + folderPane: 'uppercase', + }, + }); + }); + test('pane composite actions fill regular and Agents headers', () => { const regularRoot = document.createElement('div'); regularRoot.className = 'monaco-workbench modern-ui modern-ui-tabs'; @@ -393,6 +477,33 @@ suite('ModernUIContribution', () => { }); }); + test('panel title tabs drop the classic 1px title border so the 32px pills center in the 32px bar', () => { + const root = document.createElement('div'); + root.className = 'monaco-workbench modern-ui modern-ui-tabs'; + document.body.appendChild(root); + store.add(toDisposable(() => root.remove())); + + const { actionItem } = createCompositeAction(root, 32, true); + actionItem.closest('.part')?.classList.add('panel', 'basepanel', 'bottom'); + actionItem.closest('.title')?.classList.add('composite', 'has-composite-bar'); + + const title = actionItem.closest('.title')!; + const classicBorder = document.createElement('style'); + classicBorder.textContent = '.monaco-workbench .part.panel.bottom .composite.title { border-top: 1px solid; }'; + root.prepend(classicBorder); + const targetWindow = getWindow(title); + // Assert only what this fix owns; other layout values would be brittle. + assert.deepStrictEqual({ + titleBorderTopWidth: targetWindow.getComputedStyle(title).borderTopWidth, + titleBorderTopStyle: targetWindow.getComputedStyle(title).borderTopStyle, + actionItemBorderTop: targetWindow.getComputedStyle(actionItem).borderTopWidth, + }, { + titleBorderTopWidth: '0px', + titleBorderTopStyle: 'none', + actionItemBorderTop: '4px', + }); + }); + test('pane composite actions use regular label weight', () => { const regularRoot = document.createElement('div'); regularRoot.className = 'monaco-workbench modern-ui modern-ui-tabs'; @@ -468,7 +579,7 @@ suite('ModernUIContribution', () => { const targetWindow = getWindow(root); assert.deepStrictEqual({ - activityColorsRegistered: [MODERN_ACTIVITY_BAR_ACTIVE_BACKGROUND, MODERN_ACTIVITY_BAR_ACTIVE_FOREGROUND, MODERN_ACTIVITY_BAR_HOVER_BACKGROUND, MODERN_ACTIVITY_BAR_HOVER_FOREGROUND].map(id => colorRegistry.getColors().some(color => color.id === id)), + activityColorsRegistered: [MODERN_ACTIVITY_BAR_BACKGROUND, MODERN_ACTIVITY_BAR_INACTIVE_BACKGROUND, MODERN_ACTIVITY_BAR_ACTIVE_BACKGROUND, MODERN_ACTIVITY_BAR_ACTIVE_FOREGROUND, MODERN_ACTIVITY_BAR_HOVER_BACKGROUND, MODERN_ACTIVITY_BAR_HOVER_FOREGROUND].map(id => colorRegistry.getColors().some(color => color.id === id)), indicatorBackground: targetWindow.getComputedStyle(indicator).backgroundColor, activityLabelColor: targetWindow.getComputedStyle(activityLabel).color, horizontalIndicatorBackground: targetWindow.getComputedStyle(horizontalAction.indicator).backgroundColor, @@ -483,7 +594,7 @@ suite('ModernUIContribution', () => { headerOverflow: targetWindow.getComputedStyle(header).overflow, footerBorderWidth: targetWindow.getComputedStyle(footer).borderTopWidth, }, { - activityColorsRegistered: [true, true, true, true], + activityColorsRegistered: [true, true, true, true, true, true], indicatorBackground: 'rgb(18, 52, 86)', activityLabelColor: 'rgb(171, 205, 239)', horizontalIndicatorBackground: 'rgb(101, 67, 33)', @@ -546,6 +657,19 @@ suite('ModernUIContribution', () => { }); }); + test('inherits the customized activity bar background when inactive', () => { + const theme = ColorThemeData.createUnloadedTheme('vs-dark'); + theme.setCustomColors({ [MODERN_ACTIVITY_BAR_BACKGROUND]: '#123456' }); + + assert.deepStrictEqual({ + background: theme.getColor(MODERN_ACTIVITY_BAR_BACKGROUND)?.toString(), + inactiveBackground: theme.getColor(MODERN_ACTIVITY_BAR_INACTIVE_BACKGROUND)?.toString(), + }, { + background: '#123456', + inactiveBackground: '#123456', + }); + }); + test('hides collapsed primary side bar grips without hiding constrained auxiliary sash grips', () => { const root = document.createElement('div'); root.className = 'monaco-workbench modern-ui nosidebar nopanel'; diff --git a/src/vs/workbench/contrib/workspace/browser/workspaceTrustEditor.ts b/src/vs/workbench/contrib/workspace/browser/workspaceTrustEditor.ts index 2c82bae817c3bd..aa8ea0539ae1c3 100644 --- a/src/vs/workbench/contrib/workspace/browser/workspaceTrustEditor.ts +++ b/src/vs/workbench/contrib/workspace/browser/workspaceTrustEditor.ts @@ -341,14 +341,18 @@ class WorkspaceTrustedUrisTable extends Disposable { } async delete(item: ITrustedUriItem) { - this.table.focusNext(); - await this.workspaceTrustManagementService.setUrisTrust([item.uri], false); - - if (this.table.getFocus().length === 0) { - this.table.focusLast(); + const index = this.table.indexOf(item); + if (index < this.table.length - 1) { + this.table.setFocus([index + 1]); + } else if (index > 0) { + this.table.setFocus([index - 1]); + } else { + this.table.setFocus([]); } - this._onDelete.fire(item); this.table.domFocus(); + + await this.workspaceTrustManagementService.setUrisTrust([item.uri], false); + this._onDelete.fire(item); } async edit(item: ITrustedUriItem, usePickerIfPossible?: boolean) { diff --git a/src/vs/workbench/services/decorations/common/decorations.ts b/src/vs/workbench/services/decorations/common/decorations.ts index 6ffd62cd03e1e7..d5f5158aa59298 100644 --- a/src/vs/workbench/services/decorations/common/decorations.ts +++ b/src/vs/workbench/services/decorations/common/decorations.ts @@ -13,6 +13,16 @@ import { ThemeIcon } from '../../../../base/common/themables.js'; export const IDecorationsService = createDecorator('IFileDecorationsService'); +/** + * Stable marker class set on a label alongside the generated, per-color + * {@link IDecoration.labelClassName}. Because that class carries a hashed + * suffix, styling "labels that carry a decoration color" would otherwise need a + * `[class*="..."]` substring match. Substring matches on `class` make every + * class mutation in the workbench pay style invalidation, so use this marker + * instead. + */ +export const DECORATION_LABEL_COLOR_CLASS = 'monaco-decoration-itemColor'; + export interface IDecorationData { readonly weight?: number; readonly color?: ColorIdentifier; diff --git a/src/vs/workbench/services/themes/browser/fileIconThemeData.ts b/src/vs/workbench/services/themes/browser/fileIconThemeData.ts index e6138fb1a67073..48811cc86d26f4 100644 --- a/src/vs/workbench/services/themes/browser/fileIconThemeData.ts +++ b/src/vs/workbench/services/themes/browser/fileIconThemeData.ts @@ -417,7 +417,7 @@ export class FileIconThemeLoader { // When usesCurrentColor is set, image icons are rendered as masks filled with the current text color const imageIconStyle = (iconPath: css.CssFragment): css.CssFragment => iconThemeDocument.usesCurrentColor - ? css.inline`background-color: currentColor; background-image: none; mask: ${iconPath} no-repeat left center; mask-size: 16px; -webkit-mask: ${iconPath} no-repeat left center; -webkit-mask-size: 16px;` + ? css.inline`background-color: currentColor; background-image: none; mask: ${iconPath} no-repeat var(--file-icon-mask-position, left center); mask-size: var(--file-icon-mask-size, 16px); -webkit-mask: ${iconPath} no-repeat var(--file-icon-mask-position, left center); -webkit-mask-size: var(--file-icon-mask-size, 16px);` : css.inline`background-image: ${iconPath};`; for (const defId in selectorByDefinitionId) { diff --git a/src/vs/workbench/services/themes/test/browser/fileIconThemeData.test.ts b/src/vs/workbench/services/themes/test/browser/fileIconThemeData.test.ts index 7f62d8266099ab..9481e4959af069 100644 --- a/src/vs/workbench/services/themes/test/browser/fileIconThemeData.test.ts +++ b/src/vs/workbench/services/themes/test/browser/fileIconThemeData.test.ts @@ -44,10 +44,10 @@ suite('FileIconThemeData', () => { assert.deepStrictEqual({ backgroundColor: content?.includes('background-color: currentColor'), mask: content?.includes('mask: url('), - maskPlacement: content?.includes('no-repeat left center'), - maskSize: content?.includes('mask-size: 16px'), + maskPlacement: content?.includes('no-repeat var(--file-icon-mask-position, left center)'), + maskSize: content?.includes('mask-size: var(--file-icon-mask-size, 16px)'), webkitMask: content?.includes('-webkit-mask: url('), - webkitMaskSize: content?.includes('-webkit-mask-size: 16px'), + webkitMaskSize: content?.includes('-webkit-mask-size: var(--file-icon-mask-size, 16px)'), noBackgroundImage: content?.includes('background-image: none') }, { backgroundColor: true, diff --git a/src/vs/workbench/services/workspaces/common/workspaceTrust.ts b/src/vs/workbench/services/workspaces/common/workspaceTrust.ts index cc10c260ddf150..1a7904c50fa2b5 100644 --- a/src/vs/workbench/services/workspaces/common/workspaceTrust.ts +++ b/src/vs/workbench/services/workspaces/common/workspaceTrust.ts @@ -625,7 +625,7 @@ export class WorkspaceTrustManagementService extends Disposable implements IWork } async setUrisTrust(uris: URI[], trusted: boolean): Promise { - this.doSetUrisTrust(await Promise.all(uris.map(uri => this.getCanonicalUri(uri))), trusted); + await this.doSetUrisTrust(await Promise.all(uris.map(uri => this.getCanonicalUri(uri))), trusted); } getTrustedUris(): URI[] { diff --git a/src/vs/workbench/services/workspaces/test/common/workspaceTrust.test.ts b/src/vs/workbench/services/workspaces/test/common/workspaceTrust.test.ts index 8921cb040808a9..8ca3b973fec4c9 100644 --- a/src/vs/workbench/services/workspaces/test/common/workspaceTrust.test.ts +++ b/src/vs/workbench/services/workspaces/test/common/workspaceTrust.test.ts @@ -13,7 +13,7 @@ import { TestInstantiationService } from '../../../../../platform/instantiation/ import { NullLogService } from '../../../../../platform/log/common/log.js'; import { IRemoteAuthorityResolverService } from '../../../../../platform/remote/common/remoteAuthorityResolver.js'; import { IStorageService, StorageScope, StorageTarget } from '../../../../../platform/storage/common/storage.js'; -import { IWorkspaceContextService } from '../../../../../platform/workspace/common/workspace.js'; +import { IWorkspaceContextService, toWorkspaceFolder } from '../../../../../platform/workspace/common/workspace.js'; import { IWorkspaceTrustEnablementService, IWorkspaceTrustInfo } from '../../../../../platform/workspace/common/workspaceTrust.js'; import { Workspace } from '../../../../../platform/workspace/test/common/testWorkspace.js'; import { Memento } from '../../../../common/memento.js'; @@ -159,6 +159,96 @@ suite('Workspace Trust', () => { assert.strictEqual(true, (await testObject.getUriTrustInfo(sameFolderDifferentMeta)).trusted); }); + test('setWorkspaceTrust waits for trust transition participants before resolving', async () => { + await configurationService.setUserConfiguration('security', getUserSettings(true, true)); + workspaceService.setWorkspace(new Workspace('folder-workspace', [toWorkspaceFolder(URI.parse('file:///Folder'))])); + const testObject = await initializeTestObject(); + + let releaseParticipant!: () => void; + const participantCanComplete = new Promise(resolve => releaseParticipant = resolve); + + let participantStartedResolve!: () => void; + const participantStarted = new Promise(resolve => participantStartedResolve = resolve); + + let participantStartedFlag = false; + let participantCompleted = false; + let trustChangeEventFired = false; + + const participantCompletedPromise = new Promise(resolve => { + store.add(testObject.addWorkspaceTrustTransitionParticipant({ + async participate(trusted: boolean): Promise { + if (trusted) { + participantStartedFlag = true; + participantStartedResolve(); + await participantCanComplete; + participantCompleted = true; + resolve(); + } + } + })); + }); + + store.add(testObject.onDidChangeTrust(trusted => { + if (trusted) { + trustChangeEventFired = true; + } + })); + + await testObject.setWorkspaceTrust(false); + assert.deepStrictEqual({ + trusted: testObject.isWorkspaceTrusted(), + participantStarted: participantStartedFlag, + participantCompleted, + trustChangeEventFired + }, { + trusted: false, + participantStarted: false, + participantCompleted: false, + trustChangeEventFired: false + }); + + const setWorkspaceTrustPromise = testObject.setWorkspaceTrust(true); + let setWorkspaceTrustResolved = false; + setWorkspaceTrustPromise.then(() => setWorkspaceTrustResolved = true); + + try { + await participantStarted; + await Promise.resolve(); + + assert.deepStrictEqual({ + setWorkspaceTrustResolved, + trusted: testObject.isWorkspaceTrusted(), + participantStarted: participantStartedFlag, + participantCompleted, + trustChangeEventFired + }, { + setWorkspaceTrustResolved: false, + trusted: true, + participantStarted: true, + participantCompleted: false, + trustChangeEventFired: false + }); + } finally { + releaseParticipant(); + await participantCompletedPromise; + } + + await setWorkspaceTrustPromise; + await Promise.resolve(); + + assert.deepStrictEqual({ + setWorkspaceTrustResolved, + trusted: testObject.isWorkspaceTrusted(), + participantCompleted, + trustChangeEventFired + }, { + setWorkspaceTrustResolved: true, + trusted: true, + participantCompleted: true, + trustChangeEventFired: true + }); + }); + async function initializeTestObject(): Promise { const workspaceTrustManagementService = store.add(instantiationService.createInstance(WorkspaceTrustManagementService)); await workspaceTrustManagementService.workspaceTrustInitialized; diff --git a/src/vs/workbench/test/browser/componentFixtures/editor/editorTabBar.fixture.css b/src/vs/workbench/test/browser/componentFixtures/editor/editorTabBar.fixture.css index 8685bf9238a639..b173ee30c69962 100644 --- a/src/vs/workbench/test/browser/componentFixtures/editor/editorTabBar.fixture.css +++ b/src/vs/workbench/test/browser/componentFixtures/editor/editorTabBar.fixture.css @@ -7,7 +7,7 @@ background-color: var(--modern-ui-editor-tab-hover-background); } -.modern-ui-tabs .part.editor > .content .editor-group-container.active > .title .tabs-container > .tab.fixture-hover:not(.selected):not(.active) .tab-label:not([class*="monaco-decoration-itemColor"]) a, +.modern-ui-tabs .part.editor > .content .editor-group-container.active > .title .tabs-container > .tab.fixture-hover:not(.selected):not(.active) .tab-label:not(.monaco-decoration-itemColor) a, .modern-ui-tabs .part.editor > .content .editor-group-container.active > .title .tabs-container > .tab.fixture-hover:not(.selected):not(.active) > .tab-actions .action-label { color: var(--modern-ui-editor-tab-hover-foreground) !important; } diff --git a/src/vs/workbench/test/browser/parts/activitybar/activitybarPart.test.ts b/src/vs/workbench/test/browser/parts/activitybar/activitybarPart.test.ts index 1cf981b57cbfdd..eda6fd12cf993f 100644 --- a/src/vs/workbench/test/browser/parts/activitybar/activitybarPart.test.ts +++ b/src/vs/workbench/test/browser/parts/activitybar/activitybarPart.test.ts @@ -7,9 +7,9 @@ import assert from 'assert'; import { DisposableStore } from '../../../../../base/common/lifecycle.js'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../base/test/common/utils.js'; import { TestConfigurationService } from '../../../../../platform/configuration/test/common/testConfigurationService.js'; -import { TestThemeService } from '../../../../../platform/theme/test/common/testThemeService.js'; +import { TestColorTheme, TestThemeService } from '../../../../../platform/theme/test/common/testThemeService.js'; import { TestStorageService } from '../../../common/workbenchTestServices.js'; -import { TestLayoutService } from '../../workbenchTestServices.js'; +import { TestHostService, TestLayoutService } from '../../workbenchTestServices.js'; import { ActivitybarPart } from '../../../../browser/parts/activitybar/activitybarPart.js'; import { IViewSize } from '../../../../../base/browser/ui/grid/grid.js'; import { LayoutSettings, Parts, Position } from '../../../../services/layout/browser/layoutService.js'; @@ -21,6 +21,7 @@ import { IPaneComposite } from '../../../../common/panecomposite.js'; import { Extensions, PaneCompositeDescriptor } from '../../../../browser/panecomposite.js'; import { IInstantiationService } from '../../../../../platform/instantiation/common/instantiation.js'; import { ViewContainerLocation } from '../../../../common/views.js'; +import { ACTIVITY_BAR_BACKGROUND, MODERN_ACTIVITY_BAR_BACKGROUND, MODERN_ACTIVITY_BAR_INACTIVE_BACKGROUND } from '../../../../common/theme.js'; class StubPaneCompositePart implements IPaneCompositePart { declare readonly _serviceBrand: undefined; @@ -73,14 +74,15 @@ suite('ActivitybarPart', () => { disposables.clear(); }); - function createActivitybarPart(compact: boolean, floatingPanelsEnabled = false, sideBarPosition = Position.LEFT): { part: ActivitybarPart; configService: TestConfigurationService; layoutService: TestFloatingPanelsLayoutService } { + function createActivitybarPart(compact: boolean, floatingPanelsEnabled = false, sideBarPosition = Position.LEFT, colors: { [id: string]: string | undefined } = {}): { part: ActivitybarPart; configService: TestConfigurationService; layoutService: TestFloatingPanelsLayoutService; hostService: TestHostService } { const configService = new TestConfigurationService({ [LayoutSettings.ACTIVITY_BAR_COMPACT]: compact, [LayoutSettings.MODERN_UI]: floatingPanelsEnabled, }); const storageService = disposables.add(new TestStorageService()); - const themeService = new TestThemeService(); + const themeService = new TestThemeService(new TestColorTheme(colors)); const layoutService = new TestFloatingPanelsLayoutService(); + const hostService = new TestHostService(); layoutService.floatingPanelsEnabled = floatingPanelsEnabled; layoutService.sideBarPosition = sideBarPosition; @@ -100,9 +102,10 @@ suite('ActivitybarPart', () => { themeService, storageService, configService, + hostService, )); - return { part, configService, layoutService }; + return { part, configService, layoutService, hostService }; } function fireConfigChange(configService: TestConfigurationService, key: string): void { @@ -352,6 +355,33 @@ suite('ActivitybarPart', () => { assert.strictEqual(el.classList.contains('compact'), false); }); + test('uses the inactive background only for inactive Modern UI windows', () => { + const { part, configService, hostService } = createActivitybarPart(false, true, Position.LEFT, { + [ACTIVITY_BAR_BACKGROUND]: '#123456', + [MODERN_ACTIVITY_BAR_BACKGROUND]: '#abcdef', + [MODERN_ACTIVITY_BAR_INACTIVE_BACKGROUND]: '#654321', + }); + const el = document.createElement('div'); + fixture.appendChild(el); + part.create(el); + + const activeModernBackground = el.style.backgroundColor; + hostService.setFocus(false); + const inactiveModernBackground = el.style.backgroundColor; + configService.setUserConfiguration(LayoutSettings.MODERN_UI, false); + fireConfigChange(configService, LayoutSettings.MODERN_UI); + + assert.deepStrictEqual({ + activeModernBackground, + inactiveModernBackground, + inactiveClassicBackground: el.style.backgroundColor, + }, { + activeModernBackground: 'rgb(171, 205, 239)', + inactiveModernBackground: 'rgb(101, 67, 33)', + inactiveClassicBackground: 'rgb(18, 52, 86)', + }); + }); + // --- toJSON ------------------------------------------------------------ test('toJSON returns correct part type', () => {