From 854557a48ab98caa5a658b10abe952858ea2d212 Mon Sep 17 00:00:00 2001 From: Dmitriy Vasyura Date: Fri, 14 Aug 2026 11:53:17 -0700 Subject: [PATCH 01/19] chat: fix signed-in model prompt on startup --- .../sessionTypeAuthRequirement.test.ts | 3 ++ .../agentHostSignedOutModelsNotification.ts | 12 +++-- ...entHostSignedOutModelsNotification.test.ts | 45 +++++++++++++++++-- 3 files changed, 54 insertions(+), 6 deletions(-) diff --git a/src/vs/sessions/contrib/providers/agentHost/test/browser/sessionTypeAuthRequirement.test.ts b/src/vs/sessions/contrib/providers/agentHost/test/browser/sessionTypeAuthRequirement.test.ts index c3f84e1b45171c..d77f00bcb3650c 100644 --- a/src/vs/sessions/contrib/providers/agentHost/test/browser/sessionTypeAuthRequirement.test.ts +++ b/src/vs/sessions/contrib/providers/agentHost/test/browser/sessionTypeAuthRequirement.test.ts @@ -72,6 +72,7 @@ suite('Agent Host - session type auth requirement', () => { const ready = { allowSignedOutWhenUsable: true, accountResolved: true, + entitlementResolved: true, signedIn: false, hasCopilotHarness: true, hasModels: false, @@ -84,6 +85,7 @@ suite('Agent Host - session type auth requirement', () => { loading: getSignedOutModelsNotificationState({ ...ready, localModelsLoaded: false }), loadingPastGracePeriod: getSignedOutModelsNotificationState({ ...ready, localModelsLoaded: false, gracePeriodElapsed: true }), accountUnresolved: getSignedOutModelsNotificationState({ ...ready, accountResolved: false }), + entitlementUnresolved: getSignedOutModelsNotificationState({ ...ready, entitlementResolved: false }), harnessUnavailable: getSignedOutModelsNotificationState({ ...ready, hasCopilotHarness: false }), visible: getSignedOutModelsNotificationState(ready), modelsAvailable: getSignedOutModelsNotificationState({ ...ready, hasModels: true }), @@ -94,6 +96,7 @@ suite('Agent Host - session type auth requirement', () => { loading: SignedOutModelsNotificationState.Waiting, loadingPastGracePeriod: SignedOutModelsNotificationState.Visible, accountUnresolved: SignedOutModelsNotificationState.Hidden, + entitlementUnresolved: SignedOutModelsNotificationState.Hidden, harnessUnavailable: SignedOutModelsNotificationState.Hidden, visible: SignedOutModelsNotificationState.Visible, modelsAvailable: SignedOutModelsNotificationState.Hidden, diff --git a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSignedOutModelsNotification.ts b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSignedOutModelsNotification.ts index 8e7fdf0c1fd295..0c7950f6c0700e 100644 --- a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSignedOutModelsNotification.ts +++ b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSignedOutModelsNotification.ts @@ -12,7 +12,7 @@ import { IConfigurationService } from '../../../../../../platform/configuration/ import { IContextKeyService } from '../../../../../../platform/contextkey/common/contextkey.js'; import { IDefaultAccountService } from '../../../../../../platform/defaultAccount/common/defaultAccount.js'; import { IWorkbenchContribution } from '../../../../../common/contributions.js'; -import { ChatEntitlementContextKeys, IChatEntitlementService } from '../../../../../services/chat/common/chatEntitlementService.js'; +import { ChatEntitlement, ChatEntitlementContextKeys, IChatEntitlementService } from '../../../../../services/chat/common/chatEntitlementService.js'; import { IExtensionService } from '../../../../../services/extensions/common/extensions.js'; import { hasVisibleByokModelsTargetingSessionType } from '../sessionTypeAvailability.js'; import { ChatSetupDialogVisibleContext } from '../../chatSetup/chatSetup.js'; @@ -45,6 +45,7 @@ export const enum SignedOutModelsNotificationState { export function getSignedOutModelsNotificationState(options: { readonly allowSignedOutWhenUsable: boolean; readonly accountResolved: boolean; + readonly entitlementResolved: boolean; readonly signedIn: boolean; readonly hasCopilotHarness: boolean; readonly hasModels: boolean; @@ -52,7 +53,7 @@ export function getSignedOutModelsNotificationState(options: { readonly gracePeriodElapsed: boolean; readonly setupDialogVisible: boolean; }): SignedOutModelsNotificationState { - if (options.setupDialogVisible || !options.allowSignedOutWhenUsable || !options.accountResolved || options.signedIn || !options.hasCopilotHarness || options.hasModels) { + if (options.setupDialogVisible || !options.allowSignedOutWhenUsable || !options.accountResolved || !options.entitlementResolved || options.signedIn || !options.hasCopilotHarness || options.hasModels) { return SignedOutModelsNotificationState.Hidden; } // Readiness is the fast path; the grace period bounds it because a vendor named @@ -107,6 +108,7 @@ export class AgentHostSignedOutModelsNotificationContribution extends Disposable } }); this._register(Event.any( + this._chatEntitlementService.onDidChangeEntitlement, this._languageModelsService.onDidChangeLanguageModels, this._languageModelsService.onDidChangeModelVisibility, this._languageModelsConfigurationService.onDidChangeLanguageModelGroups, @@ -145,7 +147,10 @@ export class AgentHostSignedOutModelsNotificationContribution extends Disposable private _update(): void { // Local BYOK readiness is shared by both harnesses; the Agent Host's own bridged catalog is covered by the grace period. const allowSignedOutWhenUsable = this._configurationService.getValue(AgentHostAllowSignedOutWhenUsableSettingId) === true; - const signedIn = this._defaultAccountService.currentDefaultAccount !== null; + const entitlement = this._chatEntitlementService.entitlement; + const entitlementResolved = entitlement !== ChatEntitlement.Unresolved; + const signedIn = this._defaultAccountService.currentDefaultAccount !== null + || (entitlementResolved && entitlement !== ChatEntitlement.Unknown); const configuredByokVendors = new Set(this._languageModelsConfigurationService.getLanguageModelsProviderGroups() .map(group => group.vendor) .filter(vendor => vendor !== COPILOT_VENDOR_ID)); @@ -164,6 +169,7 @@ export class AgentHostSignedOutModelsNotificationContribution extends Disposable const state = getSignedOutModelsNotificationState({ allowSignedOutWhenUsable, accountResolved: this._accountResolved, + entitlementResolved, signedIn, hasCopilotHarness, hasModels, diff --git a/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostSignedOutModelsNotification.test.ts b/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostSignedOutModelsNotification.test.ts index 8437177de3406c..00cd8d1d2c6f41 100644 --- a/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostSignedOutModelsNotification.test.ts +++ b/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostSignedOutModelsNotification.test.ts @@ -15,7 +15,7 @@ import { IContextKeyService } from '../../../../../../platform/contextkey/common import { MockContextKeyService } from '../../../../../../platform/keybinding/test/common/mockKeybindingService.js'; import { IDefaultAccountService } from '../../../../../../platform/defaultAccount/common/defaultAccount.js'; import { TestInstantiationService } from '../../../../../../platform/instantiation/test/common/instantiationServiceMock.js'; -import { IChatEntitlementService } from '../../../../../services/chat/common/chatEntitlementService.js'; +import { ChatEntitlement, IChatEntitlementService } from '../../../../../services/chat/common/chatEntitlementService.js'; import { IExtensionService } from '../../../../../services/extensions/common/extensions.js'; import { AgentHostSignedOutModelsNotificationContribution } from '../../../browser/agentSessions/agentHost/agentHostSignedOutModelsNotification.js'; import { type IChatInputNotification, IChatInputNotificationService } from '../../../browser/widget/input/chatInputNotificationService.js'; @@ -83,6 +83,26 @@ suite('AgentHostSignedOutModelsNotification', () => { fixture.clock.restore(); }); + test('never shows while a signed-in entitlement resolves after startup', async () => { + const fixture = await createFixture({ entitlement: ChatEntitlement.Unresolved }); + const shownWhileUnresolved = fixture.notifications.isShown(); + + fixture.setEntitlement(ChatEntitlement.Pro); + + assert.deepStrictEqual([shownWhileUnresolved, fixture.notifications.isShown()], [false, false]); + fixture.clock.restore(); + }); + + test('shows once an unresolved entitlement resolves signed out', async () => { + const fixture = await createFixture({ entitlement: ChatEntitlement.Unresolved }); + const shownWhileUnresolved = fixture.notifications.isShown(); + + fixture.setEntitlement(ChatEntitlement.Unknown); + + assert.deepStrictEqual([shownWhileUnresolved, fixture.notifications.isShown()], [false, true]); + fixture.clock.restore(); + }); + test('gives a later wait its own grace period instead of the remainder of an earlier one', async () => { const fixture = await createFixture({ configuredVendors: ['anthropic'], resolvedVendors: [] }); assert.strictEqual(fixture.notifications.isShown(), false); @@ -119,12 +139,13 @@ suite('AgentHostSignedOutModelsNotification', () => { fixture.clock.restore(); }); - async function createFixture(options: { configuredVendors?: string[]; resolvedVendors?: string[] } = {}) { + async function createFixture(options: { configuredVendors?: string[]; resolvedVendors?: string[]; entitlement?: ChatEntitlement } = {}) { const clock = sinon.useFakeTimers({ shouldAdvanceTime: false }); const notifications = new TestChatInputNotificationService(); const languageModels = new TestLanguageModelsService(options.resolvedVendors ?? ['anthropic']); const languageModelsConfiguration = new TestLanguageModelsConfigurationService(options.configuredVendors ?? []); const account = new TestDefaultAccountService(); + const chatEntitlement = new TestChatEntitlementService(options.entitlement ?? ChatEntitlement.Unknown); const configuration = new TestConfigurationService(); configuration.setUserConfiguration(AgentHostAllowSignedOutWhenUsableSettingId, true); @@ -138,7 +159,7 @@ suite('AgentHostSignedOutModelsNotification', () => { rootState: new TestRootStateSubscription({ agents: [{ provider: 'copilotcli' }] } as RootState), }); instantiationService.stub(IConfigurationService, configuration); - instantiationService.stub(IChatEntitlementService, { clientByokEnabled: true }); + instantiationService.stub(IChatEntitlementService, chatEntitlement); instantiationService.stub(IContextKeyService, store.add(new MockContextKeyService())); instantiationService.stub(IExtensionService, { whenInstalledExtensionsRegistered: () => Promise.resolve(true) }); @@ -153,6 +174,7 @@ suite('AgentHostSignedOutModelsNotification', () => { resolveVendor: (vendor: string) => languageModels.resolveVendor(vendor), addConfiguredVendor: (vendor: string) => languageModelsConfiguration.addVendor(vendor), signIn: () => account.setSignedIn(), + setEntitlement: (entitlement: ChatEntitlement) => chatEntitlement.setEntitlement(entitlement), }; } }); @@ -248,6 +270,23 @@ class TestDefaultAccountService implements Partial { } } +class TestChatEntitlementService implements Partial { + declare readonly _serviceBrand: undefined; + private readonly _onDidChangeEntitlement = new Emitter(); + readonly onDidChangeEntitlement = this._onDidChangeEntitlement.event; + readonly clientByokEnabled = true; + + constructor(private _entitlement: ChatEntitlement) { } + + get entitlement(): ChatEntitlement { + return this._entitlement; + } + setEntitlement(entitlement: ChatEntitlement): void { + this._entitlement = entitlement; + this._onDidChangeEntitlement.fire(); + } +} + class TestRootStateSubscription { readonly onDidChange = Event.None; readonly onDidError = Event.None; From 07c969b921cf594ac5539b159305b4a470f26931 Mon Sep 17 00:00:00 2001 From: ulugbekna Date: Mon, 17 Aug 2026 11:06:04 +0200 Subject: [PATCH 02/19] chat: fix: stop capitalizing localized customization group headers `text-transform: capitalize` on the shared group header re-cased strings that were already cased correctly, and got them wrong: "Included Based on Context" rendered as "Included Based On Context" and "Loaded on Demand" as "Loaded On Demand", against this repo's own rule that short prepositions stay lowercase, and "Built-in" rendered as "Built-In". Per-word capitalization is also not a transform that survives translation, so the rule was wrong for every locale rather than just awkward in English. The header is shared, so this reaches every customizations tab -- Agents, Skills, Instructions, Hooks, Prompts, Plugins and MCP Servers. Every group label on those tabs is already a correctly cased localized string, so the only rendered differences are the three the transform was getting wrong: four screenshot baselines change, on the Instructions and MCP Servers tabs. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../aiCustomization/media/aiCustomizationManagement.css | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/vs/workbench/contrib/chat/browser/aiCustomization/media/aiCustomizationManagement.css b/src/vs/workbench/contrib/chat/browser/aiCustomization/media/aiCustomizationManagement.css index 275c364052e427..76a32e562bc146 100644 --- a/src/vs/workbench/contrib/chat/browser/aiCustomization/media/aiCustomizationManagement.css +++ b/src/vs/workbench/contrib/chat/browser/aiCustomization/media/aiCustomizationManagement.css @@ -479,9 +479,10 @@ min-width: 0; } +/* No text-transform: these labels are localized strings that are already cased correctly, and +per-word capitalization does not survive translation. */ .ai-customization-group-header .group-label { font-weight: var(--vscode-agents-fontWeight-semiBold); - text-transform: capitalize; color: var(--vscode-sideBarSectionHeader-foreground, var(--vscode-foreground)); overflow: hidden; text-overflow: ellipsis; From 38be91b69151e61c8b8091e8a5b9c699b532f07d Mon Sep 17 00:00:00 2001 From: ulugbekna Date: Mon, 17 Aug 2026 11:06:17 +0200 Subject: [PATCH 03/19] mcp: fix: make Show Output clickable on a failing MCP server `updateStatus` began by clearing the row's action disposables and emptying its actions node, then rebuilt them. It runs from an autorun over the server's connection state, and an erroring server re-runs it about twice a second while producing byte-identical content: measured in a real Code OSS build, 9-10 rebuilds per 5 seconds, every one of them a no-op. A DOM node replaced between mousedown and mouseup never receives the click, so the inline `Show Output` button did nothing on precisely the rows that needed it -- the failing ones, which are the only rows that offer it at all. The row's actions are now rebuilt only when something about them changed. `getMcpStatusRenderSignature` reduces them to a comparable value covering both what they render and what they act on; leaving anything out would drop an update that matters, so it is a pure exported function whose test fails to compile if a field is added without being covered. The list re-splices on every customizations change, so `renderElement` would otherwise undo this by clearing the actions itself. It now keys on the row's content identity rather than the entry object, which is recreated on every refresh and therefore says nothing about whether this is the same row. Both guards are pinned by tests that drive the renderer itself, since that is the only place the failure is observable: an erroring server's update is fired repeatedly with unchanged content, and the button must be the same node afterwards, still attached, and still reach `showMcpServerLog` when clicked. A third test asserts a real status change does still rebuild, so the guard cannot be satisfied by never updating. Reverting either guard fails them. `McpServerItemRenderer` is exported for this. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../browser/aiCustomization/mcpListWidget.ts | 115 ++++++++++++- .../aiCustomization/mcpListWidget.test.ts | 161 +++++++++++++++++- 2 files changed, 268 insertions(+), 8 deletions(-) diff --git a/src/vs/workbench/contrib/chat/browser/aiCustomization/mcpListWidget.ts b/src/vs/workbench/contrib/chat/browser/aiCustomization/mcpListWidget.ts index 92b45cae6fbcf8..18c56f19b64f04 100644 --- a/src/vs/workbench/contrib/chat/browser/aiCustomization/mcpListWidget.ts +++ b/src/vs/workbench/contrib/chat/browser/aiCustomization/mcpListWidget.ts @@ -162,12 +162,22 @@ interface IMcpServerItemTemplateData { readonly actions: HTMLElement; readonly elementDisposables: DisposableStore; readonly actionDisposables: DisposableStore; + /** Which row the actions currently belong to, so a recycled template cannot reuse another row's. */ + renderedRowKey?: string; + /** What the actions currently show, so an unchanged status does not rebuild them. */ + renderedStatusSignature?: string; } /** * Renderer for local MCP server list items. */ -class McpServerItemRenderer implements IListRenderer { +/** + * Renderer for local MCP server list items. + * + * Exported for testing: the guard that keeps a row's actions alive across no-op updates is only + * observable by driving the renderer itself. + */ +export class McpServerItemRenderer implements IListRenderer { readonly templateId = 'mcpServerItem'; constructor( @@ -206,8 +216,18 @@ class McpServerItemRenderer implements IListRenderer Promise) => this.agentHostCustomizationService.showMcpServerLog(activeSessionResource, activeSessionServer.id, beforeShow) : undefined; @@ -354,7 +397,7 @@ class McpServerItemRenderer implements IListRenderer = {}): AgentHostMcpServer { @@ -508,6 +517,156 @@ suite('mcpListWidget', () => { }); }); + suite('getMcpStatusRenderSignature', () => { + const base: IMcpStatusRenderInput = { + rowKey: 'server:mcp.config.workspace/notion:0', + label: 'notion', + state: McpServerStatus.Error, + statusLabel: 'Error', + statusClassName: 'error', + statusIconId: 'error', + activeSessionServerId: 'session-1/notion', + logOutputChannelId: 'mcp.session-1.notion', + localServerId: 'mcp.config.workspace/notion', + activeSessionResource: 'vscode-agent-session:///session-1', + }; + + // A different, and differently-typed-where-possible, value for every field. The mapped type + // is what makes this a barrier: a field added to the input fails to compile until it is + // given a value here, and the test below then proves the signature actually covers it. + const changed: { [K in keyof IMcpStatusRenderInput]-?: IMcpStatusRenderInput[K] } = { + rowKey: 'server:mcp.config.user/notion:0', + label: 'Notion', + state: McpServerStatus.Ready, + statusLabel: 'Running', + statusClassName: 'running', + statusIconId: 'check', + activeSessionServerId: 'session-1/other', + logOutputChannelId: 'mcp.session-1.other', + localServerId: 'mcp.config.user/notion', + activeSessionResource: 'vscode-agent-session:///session-2', + }; + + const fields = Object.keys(base) as (keyof IMcpStatusRenderInput)[]; + + test('the same row state produces the same signature', () => { + assert.strictEqual(getMcpStatusRenderSignature({ ...base }), getMcpStatusRenderSignature({ ...base })); + }); + + test('changing any covered value changes the signature', () => { + const baseline = getMcpStatusRenderSignature(base); + const missed = fields.filter(field => getMcpStatusRenderSignature({ ...base, [field]: changed[field] }) === baseline); + + assert.deepStrictEqual(missed, []); + }); + + test('clearing any optional value changes the signature', () => { + const baseline = getMcpStatusRenderSignature(base); + // `rowKey` and `label` are always present; everything else can legitimately go away, + // e.g. when a server loses its active-session twin. + const clearable = fields.filter(field => field !== 'rowKey' && field !== 'label'); + const missed = clearable.filter(field => getMcpStatusRenderSignature({ ...base, [field]: undefined }) === baseline); + + assert.deepStrictEqual(missed, []); + }); + }); + + suite('row actions survive no-op updates', () => { + // The signature tests above only cover the pure helper, so they would still pass if the + // early return in `updateStatus` or the row guard in `renderElement` were removed. These + // drive the renderer itself, which is the only place the reported failure is observable: + // an erroring server re-runs the status update about twice a second, and a button node + // replaced between mousedown and mouseup never receives the click. + function createRenderer(server: AgentHostMcpServer) { + const store = new DisposableStore(); + const onDidChangeCustomizations = store.add(new Emitter()); + const sessionResource = URI.parse('vscode-agent-session:///session-1'); + let servers: AgentHostMcpServer[] = [server]; + const shownLogs: string[] = []; + + const agentHostCustomizationService = { + getMcpServers: () => servers, + onDidChangeCustomizations: onDidChangeCustomizations.event, + showMcpServerLog: async (_resource: URI, serverId: string) => { shownLogs.push(serverId); }, + } as unknown as IAgentHostCustomizationService; + const customizationHarnessService = { + activeSessionResource: observableValue('activeSessionResource', sessionResource), + } as unknown as ICustomizationHarnessService; + const renderer = new McpServerItemRenderer( + async () => { }, + { isSessionsWindow: true } as IAICustomizationWorkspaceService, + { plugins: observableValue('plugins', []) } as unknown as IAgentPluginService, + { setupManagedHover: () => Disposable.None } as unknown as IHoverService, + agentHostCustomizationService, + customizationHarnessService, + { showChannel: async () => { } } as unknown as IOutputService, + ); + + const container = document.createElement('div'); + const templateData = renderer.renderTemplate(container); + store.add({ dispose: () => renderer.disposeTemplate(templateData) }); + + return { + store, + templateData, + shownLogs, + render: () => renderer.renderElement(createBuiltinActiveSessionMcpEntries([server])[0], 0, templateData), + notifyUnchanged: () => onDidChangeCustomizations.fire(), + setServers: (next: AgentHostMcpServer[]) => { servers = next; }, + actionNode: () => templateData.actions.firstElementChild, + }; + } + + const erroring = () => createAgentHostServer({ id: 'server-1', status: McpServerStatus.Error, state: { kind: McpServerStatus.Error, error: { errorType: 'spawn', message: 'failed to start' } } }); + + test('the Show Output button stays the same clickable node across repeated identical updates', () => { + const ctx = createRenderer(erroring()); + disposables.add(ctx.store); + ctx.render(); + + const button = ctx.actionNode(); + assert.ok(button, 'expected an action for an erroring server'); + + // What the autorun does in production while a server sits in error. + for (let i = 0; i < 10; i++) { + ctx.notifyUnchanged(); + } + + assert.strictEqual(ctx.actionNode(), button, 'the button was replaced by an update that changed nothing'); + assert.strictEqual(button.parentElement, ctx.templateData.actions, 'the button was detached from the row'); + + (button as HTMLElement).click(); + + assert.deepStrictEqual(ctx.shownLogs, ['server-1']); + }); + + test('re-rendering the same row keeps its actions, so a list refresh cannot swallow a click', () => { + // Entries are recreated on every refresh, and the list re-splices every visible row on + // any customizations change, so the guard has to key on content rather than identity. + const ctx = createRenderer(erroring()); + disposables.add(ctx.store); + ctx.render(); + const button = ctx.actionNode(); + + ctx.render(); + + assert.strictEqual(ctx.actionNode(), button, 'a re-render of the same row rebuilt its actions'); + }); + + test('a real status change still rebuilds the actions', () => { + const ctx = createRenderer(erroring()); + disposables.add(ctx.store); + ctx.render(); + const button = ctx.actionNode(); + + // Recovering from error drops the Show Output action entirely. + ctx.setServers([createAgentHostServer({ id: 'server-1', status: McpServerStatus.Ready, state: { kind: McpServerStatus.Ready } })]); + ctx.notifyUnchanged(); + + assert.notStrictEqual(ctx.actionNode(), button, 'the actions were not rebuilt for a changed status'); + }); + }); + suite('inline actions', () => { test('authentication receives the active session and server without opening the row', () => { const sessionResource = URI.parse('vscode-agent-session:///session-1'); From 0fff929643a69b24724d9bf96afb4f9892800b6a Mon Sep 17 00:00:00 2001 From: ulugbekna Date: Mon, 17 Aug 2026 11:06:31 +0200 Subject: [PATCH 04/19] mcp: fix: show one row per agent MCP server, not one per customization A session can carry two customizations for a single MCP server. The agent host publishes the declaration as a child of whatever declared it -- a plugin, or the .mcp.json VS Code syncs into the agent -- and separately mints a top-level customization for any server the SDK reports before that child can be resolved by name. `McpCustomizationController._applyOne` never retires the minted entry once the child becomes resolvable ("Once promoted to a top-level entry, stay top-level for the session"), so both remain in state: notion -> file:///.../vscode-synced-customization-.../.mcp.json#mcp=notion state: stopped <- the declaration notion -> mcp-top-level:copilotcli::notion state: ready, channel: mcp:// <- the live one Every consumer of getMcpServers saw both, so the servers list rendered the same server twice with contradictory status. It was worse than a repeat: the list's matcher only matches when exactly one candidate answers a key, so with two copies the server's local row could not adopt either, and both fell through as extra rows. getMcpServers now drops a child that a top-level customization already speaks for. The top-level copy wins because it is the one the host treats as live: it carries the running state and channel, and its id is what the host resolves for lifecycle and enablement. Position in the tree is the signal, not the shape of the minted id, which is the host's own business -- and not the absence of an owning plugin either, since a directory-declared child has none. That last one is a live trap rather than a hypothetical, so it has its own test. Nothing else is collapsed. Two plugins that each declare a server named `search` stay two rows, because they are two servers. Only the presentation path dedupes; log, diagnostics and id lookups still walk every customization, so an id from either copy continues to resolve. The host-side repair is tracked in #330893. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../agentHostCustomizationService.ts | 71 ++++++++++++++---- .../agentHostMcpServerCustomizations.test.ts | 75 +++++++++++++++++++ 2 files changed, 132 insertions(+), 14 deletions(-) create mode 100644 src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostMcpServerCustomizations.test.ts diff --git a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostCustomizationService.ts b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostCustomizationService.ts index a55b21623956e3..dd6c574cce01e3 100644 --- a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostCustomizationService.ts +++ b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostCustomizationService.ts @@ -184,7 +184,7 @@ export abstract class AbstractAgentHostCustomizationService extends Disposable i if (!target) { return []; } - return this._flattenMcpServers(target.customizations) + return getPresentableMcpServerCustomizations(target.customizations) .map(({ server, plugin }): IAgentHostMcpServer => ({ id: this._scopedMcpServerId(sessionResource, server.id), name: server.name, @@ -208,7 +208,7 @@ export abstract class AbstractAgentHostCustomizationService extends Disposable i if (!target) { return Promise.resolve(); } - const entry = this._flattenMcpServers(target.customizations).find(({ server }) => this._scopedMcpServerId(sessionResource, server.id) === serverId); + const entry = flattenMcpServerCustomizations(target.customizations).find(({ server }) => this._scopedMcpServerId(sessionResource, server.id) === serverId); if (!entry) { return Promise.resolve(); } @@ -226,7 +226,7 @@ export abstract class AbstractAgentHostCustomizationService extends Disposable i */ private _trackMcpDiagnostics(sessionResource: URI, target: IAgentHostCustomizationTarget): void { this._mcpDiagnosticSessions.add(sessionResource); - for (const { server, plugin } of this._flattenMcpServers(target.customizations)) { + for (const { server, plugin } of flattenMcpServerCustomizations(target.customizations)) { this._mcpLogRegistry.record({ sessionResource, rawId: server.id, name: server.name, enabled: isCustomizationEnabled(server) && (!plugin || isCustomizationEnabled(plugin)), state: server.state }); } } @@ -238,7 +238,7 @@ export abstract class AbstractAgentHostCustomizationService extends Disposable i if (!target) { continue; } - for (const { server, plugin } of this._flattenMcpServers(target.customizations)) { + for (const { server, plugin } of flattenMcpServerCustomizations(target.customizations)) { this._mcpLogRegistry.record({ sessionResource, rawId: server.id, name: server.name, enabled: isCustomizationEnabled(server) && (!plugin || isCustomizationEnabled(plugin)), state: server.state }); } } @@ -327,17 +327,8 @@ export abstract class AbstractAgentHostCustomizationService extends Disposable i this._onDidChangeCustomizations.fire(); } - private _flattenMcpServers(customizations: readonly Customization[]): readonly { readonly server: McpServerCustomization; readonly plugin?: PluginCustomization }[] { - return customizations.flatMap(customization => customization.type === CustomizationType.McpServer - ? [{ server: customization }] - : customization.children?.filter(child => child.type === CustomizationType.McpServer).map(server => ({ - server, - plugin: customization.type === CustomizationType.Plugin ? customization : undefined, - })) ?? []); - } - private _findMcpServer(customizations: readonly Customization[], serverId: string): McpServerCustomization | undefined { - for (const { server } of this._flattenMcpServers(customizations)) { + for (const { server } of flattenMcpServerCustomizations(customizations)) { if (server.id === serverId || this._isScopedMcpServerIdForRawId(serverId, server.id)) { return server; } @@ -368,6 +359,58 @@ export abstract class AbstractAgentHostCustomizationService extends Disposable i } } +/** One MCP server customization, with the position it was published at. */ +export interface IMcpServerCustomizationEntry { + readonly server: McpServerCustomization; + /** + * The plugin that declares this server. Absent both for a server published at the top level + * and for one declared by a {@link CustomizationType.Directory} container, so it says nothing + * about where in the tree the server sits -- use {@link isTopLevel} for that. + */ + readonly plugin?: PluginCustomization; + /** Whether the agent host published this server as a customization of the session itself. */ + readonly isTopLevel: boolean; +} + +/** Every MCP server customization in a session, including duplicates of the same server. */ +export function flattenMcpServerCustomizations(customizations: readonly Customization[]): readonly IMcpServerCustomizationEntry[] { + return customizations.flatMap((customization): IMcpServerCustomizationEntry[] => customization.type === CustomizationType.McpServer + ? [{ server: customization, isTopLevel: true }] + : customization.children?.filter(child => child.type === CustomizationType.McpServer).map(server => ({ + server, + plugin: customization.type === CustomizationType.Plugin ? customization : undefined, + isTopLevel: false, + })) ?? []); +} + +/** + * The MCP servers to *show* for a session: one entry per server. + * + * A session can carry two customizations for one server: the declaration, published as a child of + * whatever declared it, and a top-level entry the agent host mints for a server the SDK reports + * before that child resolves by name. A child is dropped when a top-level customization already + * speaks for its name, because the top-level copy is the one the host keeps live and resolves for + * lifecycle and enablement. + * + * Tree position is the signal, not the shape of the minted id and not the absence of an owning + * plugin -- a directory-declared child has none either. Only presentation dedupes; lookups + * elsewhere walk every customization, so an id from either copy still resolves. Servers of the + * same name from different containers are left alone, because they are different servers. + */ +export function getPresentableMcpServerCustomizations(customizations: readonly Customization[]): readonly IMcpServerCustomizationEntry[] { + const entries = flattenMcpServerCustomizations(customizations); + const topLevelNames = new Set(); + for (const entry of entries) { + if (entry.isTopLevel) { + topLevelNames.add(entry.server.name); + } + } + if (topLevelNames.size === 0) { + return entries; + } + return entries.filter(entry => entry.isTopLevel || !topLevelNames.has(entry.server.name)); +} + class WorkbenchAgentHostCustomizationService extends AbstractAgentHostCustomizationService { private readonly _sessionStateSubscriptions = this._register(new DisposableResourceMap }>()); diff --git a/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostMcpServerCustomizations.test.ts b/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostMcpServerCustomizations.test.ts new file mode 100644 index 00000000000000..8da0e59cbc931a --- /dev/null +++ b/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostMcpServerCustomizations.test.ts @@ -0,0 +1,75 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import assert from 'assert'; +import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../../base/test/common/utils.js'; +import { CustomizationType, McpServerStatus, type Customization, type DirectoryCustomization, type McpServerCustomization, type McpServerState, type PluginCustomization } from '../../../../../../platform/agentHost/common/state/protocol/state.js'; +import { getPresentableMcpServerCustomizations } from '../../../browser/agentSessions/agentHost/agentHostCustomizationService.js'; + +suite('agent host MCP server customizations', () => { + + ensureNoDisposablesAreLeakedInTestSuite(); + + function mcpServer(id: string, name: string, state: McpServerState = { kind: McpServerStatus.Stopped }): McpServerCustomization { + return { type: CustomizationType.McpServer, id, uri: `file:///${encodeURIComponent(id)}`, name, state }; + } + + function plugin(id: string, children: McpServerCustomization[]): PluginCustomization { + return { type: CustomizationType.Plugin, id, uri: `file:///${encodeURIComponent(id)}`, name: id, children }; + } + + function directory(id: string, children: McpServerCustomization[]): DirectoryCustomization { + return { type: CustomizationType.Directory, id, uri: `file:///${encodeURIComponent(id)}`, name: id, enabled: true, writable: true, contents: CustomizationType.McpServer, children }; + } + + function shown(customizations: readonly Customization[]): [string, string][] { + return getPresentableMcpServerCustomizations(customizations).map(({ server }) => [server.name, server.id]); + } + + test('a server a plugin declares and the host also minted is shown once, as the live copy', () => { + // The shape an agent host actually publishes: the synced `.mcp.json` declares the server + // and never leaves `stopped`, while the minted top-level entry is the one it keeps current. + const declaration = mcpServer('file:///synced/.mcp.json#mcp=notion', 'notion'); + const live = mcpServer('mcp-top-level:copilotcli:session:notion', 'notion', { kind: McpServerStatus.Ready }); + + assert.deepStrictEqual(shown([plugin('synced', [declaration]), live]), [ + ['notion', 'mcp-top-level:copilotcli:session:notion'], + ]); + }); + + test('a server a directory declares and the host also minted is shown once, as the live copy', () => { + // A directory-declared child carries no owning plugin, so "has no plugin" cannot stand in + // for "is top-level": doing so would let this child claim the name and survive shadowing. + const declaration = mcpServer('file:///.mcp.json#mcp=notion', 'notion'); + const live = mcpServer('mcp-top-level:copilotcli:session:notion', 'notion', { kind: McpServerStatus.Ready }); + + assert.deepStrictEqual(shown([directory('mcp-config', [declaration]), live]), [ + ['notion', 'mcp-top-level:copilotcli:session:notion'], + ]); + }); + + test('a server only a container declares is kept, and so is one only the host minted', () => { + const declared = mcpServer('file:///synced/.mcp.json#mcp=cleanshot', 'cleanshot'); + const inDirectory = mcpServer('file:///.mcp.json#mcp=playwright', 'playwright'); + const minted = mcpServer('mcp-top-level:copilotcli:session:notion', 'notion', { kind: McpServerStatus.Ready }); + + // Source order is preserved, exactly as it was before duplicates were collapsed. + assert.deepStrictEqual(shown([plugin('synced', [declared]), directory('mcp-config', [inDirectory]), minted]), [ + ['cleanshot', 'file:///synced/.mcp.json#mcp=cleanshot'], + ['playwright', 'file:///.mcp.json#mcp=playwright'], + ['notion', 'mcp-top-level:copilotcli:session:notion'], + ]); + }); + + test('two plugins declaring the same name stay two servers, because they are two servers', () => { + assert.deepStrictEqual(shown([ + plugin('a', [mcpServer('file:///a/.mcp.json#mcp=search', 'search')]), + plugin('b', [mcpServer('file:///b/.mcp.json#mcp=search', 'search')]), + ]), [ + ['search', 'file:///a/.mcp.json#mcp=search'], + ['search', 'file:///b/.mcp.json#mcp=search'], + ]); + }); +}); From 518720d54da77e8a9b5b087b26c00e3b515a5f79 Mon Sep 17 00:00:00 2001 From: n-gist <58081918+n-gist@users.noreply.github.com> Date: Mon, 17 Aug 2026 22:19:07 +0500 Subject: [PATCH 05/19] fix languages.getDiagnostics() problem-matchers diagnostics duplication (#290278) --- src/vs/workbench/api/browser/mainThreadDiagnostics.ts | 10 ++-------- src/vs/workbench/api/common/extHostDiagnostics.ts | 6 +++++- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/src/vs/workbench/api/browser/mainThreadDiagnostics.ts b/src/vs/workbench/api/browser/mainThreadDiagnostics.ts index d4d46de4755a98..00658bf710df89 100644 --- a/src/vs/workbench/api/browser/mainThreadDiagnostics.ts +++ b/src/vs/workbench/api/browser/mainThreadDiagnostics.ts @@ -58,14 +58,8 @@ export class MainThreadDiagnostics implements MainThreadDiagnosticsShape { const data: [UriComponents, IMarkerData[]][] = []; for (const resource of resources) { const allMarkerData = this._markerService.read({ resource, ignoreResourceFilters: true }); - if (allMarkerData.length === 0) { - data.push([resource, []]); - } else { - const foreignMarkerData = allMarkerData.filter(marker => marker?.origin !== this.extHostId); - if (foreignMarkerData.length > 0) { - data.push([resource, foreignMarkerData]); - } - } + const markerData = allMarkerData.filter(marker => marker.origin !== this.extHostId); + data.push([resource, markerData]); } if (data.length > 0) { this._proxy.$acceptMarkersChange(data); diff --git a/src/vs/workbench/api/common/extHostDiagnostics.ts b/src/vs/workbench/api/common/extHostDiagnostics.ts index 1b61328f362f46..7d3043a4d144a8 100644 --- a/src/vs/workbench/api/common/extHostDiagnostics.ts +++ b/src/vs/workbench/api/common/extHostDiagnostics.ts @@ -361,7 +361,11 @@ export class ExtHostDiagnostics implements ExtHostDiagnosticsShape { } for (const [uri, markers] of data) { - this._mirrorCollection.set(URI.revive(uri), markers.map(converter.Diagnostic.to)); + if (markers.length === 0) { + this._mirrorCollection.delete(URI.revive(uri)); + } else { + this._mirrorCollection.set(URI.revive(uri), markers.map(converter.Diagnostic.to)); + } } } } From 4ccdb71a3e51e6098d5bb7096cedf00415a3e3e2 Mon Sep 17 00:00:00 2001 From: Sandeep Somavarapu Date: Mon, 17 Aug 2026 20:20:40 +0200 Subject: [PATCH 06/19] sessions: Synchronize live last turn changes (#331244) * sessions: Synchronize live last turn changes Use active-turn chat edits for Last Turn Changes until the finalized host changeset is available, and route the live changes pill to the same changeset. Keep historical turn selections workspace-scoped. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * sessions: Address live turn review feedback Keep live turn changes until the finalized host changeset is ready, tighten workspace filtering, and prune per-chat observable cache entries. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/vs/sessions/SESSIONS.md | 11 +- .../chat/browser/sessionTurnChanges.ts | 46 +++++--- .../test/browser/sessionTurnChanges.test.ts | 55 +++++++-- .../agentHost/AGENT_HOST_SESSIONS_PROVIDER.md | 2 +- .../browser/agentHostSessionChangesets.ts | 36 +++++- .../browser/agentHostSessionFiles.ts | 43 +++++-- .../browser/baseAgentHostSessionsProvider.ts | 42 ++++++- .../localAgentHostSessionsProvider.test.ts | 111 +++++++++++++++++- 8 files changed, 286 insertions(+), 60 deletions(-) diff --git a/src/vs/sessions/SESSIONS.md b/src/vs/sessions/SESSIONS.md index e5af91bb8be081..18cda062f4a779 100644 --- a/src/vs/sessions/SESSIONS.md +++ b/src/vs/sessions/SESSIONS.md @@ -180,12 +180,11 @@ provider supports it. Turn-level file changes open through `IChatResponseFileChangesService`. The Editor workbench opens a standalone multi-diff, while the Agents Window selects -the canonical Changes editor. The active-turn pill uses a transient selection -backed by the viewed chat's live `lastTurnChanges` observable so streamed edits -appear before turn completion. The completed latest-response pill selects the -provider's moving last-turn changeset, which follows the most recently modified -chat; historical turns and other completed chats use a transient selection -backed by their exact per-turn changes. +the canonical Changes editor. The active-turn pill selects the provider's moving +Last Turn Changes changeset, which follows the active turn and then remains on +that turn after completion. Historical turns and completed chats that are no +longer the session's most recent use transient selections backed by their exact +per-turn changes. Presentation and layout of changes are documented in [LAYOUT.md](LAYOUT.md). Provider translation and transport details belong in the relevant provider diff --git a/src/vs/sessions/contrib/chat/browser/sessionTurnChanges.ts b/src/vs/sessions/contrib/chat/browser/sessionTurnChanges.ts index c9031f2f418eb2..2c0079d6fc5ee2 100644 --- a/src/vs/sessions/contrib/chat/browser/sessionTurnChanges.ts +++ b/src/vs/sessions/contrib/chat/browser/sessionTurnChanges.ts @@ -4,13 +4,14 @@ *--------------------------------------------------------------------------------------------*/ import { constObservable, derived, IObservable } from '../../../../base/common/observable.js'; -import { isEqual } from '../../../../base/common/resources.js'; +import { extUriBiasedIgnorePathCase, isEqual } from '../../../../base/common/resources.js'; import { URI } from '../../../../base/common/uri.js'; import { localize } from '../../../../nls.js'; import { AbstractChatResponseFileChangesService, IChatResponseFileChangesOpenContext } from '../../../../workbench/contrib/chat/browser/chatResponseFileChangesService.js'; import { IEditSessionEntryDiff } from '../../../../workbench/contrib/chat/common/editing/chatEditingService.js'; import { IEditorService } from '../../../../workbench/services/editor/common/editorService.js'; import { IAgentWorkbenchLayoutService } from '../../../browser/workbench.js'; +import { isAgentHostProviderId } from '../../../common/agentHostSessionsProvider.js'; import { ISessionsService } from '../../../services/sessions/browser/sessionsService.js'; import { IChat, ISession, ISessionChangeset, ISessionFileChange, TURN_CHANGES_CHANGESET_ID } from '../../../services/sessions/common/session.js'; import { ISessionsManagementService } from '../../../services/sessions/common/sessionsManagement.js'; @@ -43,9 +44,13 @@ export class SessionsChatResponseFileChangesService extends AbstractChatResponse } if (context.isLastTurn) { if (requestId === undefined) { - const changes = owner.chat.lastTurnChanges; - if (changes) { - this._openTransientLastTurnChanges(owner.session, owner.chat.resource.toString(), changes); + if (isAgentHostProviderId(owner.session.providerId)) { + void this._openSessionTurnChanges(owner.session); + } else { + const changes = owner.chat.lastTurnChanges; + if (changes) { + this._openTransientLastTurnChanges(owner.session, owner.chat.resource.toString(), changes); + } } return; } @@ -55,7 +60,7 @@ export class SessionsChatResponseFileChangesService extends AbstractChatResponse return; } - const changes = this._getSessionFileChanges(chatResource, requestId); + const changes = this._getSessionFileChanges(owner.session, chatResource, requestId); if (changes) { this._openTransientLastTurnChanges(owner.session, requestId, changes); } @@ -74,7 +79,7 @@ export class SessionsChatResponseFileChangesService extends AbstractChatResponse id: `${TURN_CHANGES_CHANGESET_ID}:${requestId}`, label: localize('historicalTurnChanges.label', "Turn Changes"), description: localize('historicalTurnChanges.description', "Changes from the selected chat turn."), - changes: this._toSessionFileChanges(changes), + changes: this._toSessionFileChanges(owner.session, changes), }); } @@ -105,25 +110,32 @@ export class SessionsChatResponseFileChangesService extends AbstractChatResponse return isEqual(mostRecentChat?.resource ?? session.mainChat.get().resource, chat.resource); } - private _getSessionFileChanges(chatResource: URI, requestId: string): IObservable | undefined { + private _getSessionFileChanges(session: ISession, chatResource: URI, requestId: string): IObservable | undefined { const changes = this.getChangesForRequest(chatResource, requestId); - return changes ? this._toSessionFileChanges(changes) : undefined; + return changes ? this._toSessionFileChanges(session, changes) : undefined; } - private _toSessionFileChanges(changes: IObservable): IObservable { - return derived(reader => changes.read(reader).map((diff): ISessionFileChange => ({ - uri: diff.modifiedURI, - originalUri: isEqual(diff.originalURI, diff.modifiedURI) ? undefined : diff.originalURI, - modifiedUri: diff.isDeleted ? undefined : diff.modifiedSnapshotURI ?? diff.modifiedURI, - insertions: diff.added, - deletions: diff.removed, - }))); + private _toSessionFileChanges(session: ISession, changes: IObservable): IObservable { + return derived(reader => { + const workspace = session.workspace?.read(reader); + const workspaceFolders = workspace?.folders.flatMap(folder => [folder.root, folder.workingDirectory]) ?? []; + return changes.read(reader) + .filter(diff => workspaceFolders.some(folder => + extUriBiasedIgnorePathCase.isEqualOrParent(folder.with({ path: diff.modifiedURI.path }), folder))) + .map((diff): ISessionFileChange => ({ + uri: diff.modifiedURI, + originalUri: isEqual(diff.originalURI, diff.modifiedURI) ? undefined : diff.originalURI, + modifiedUri: diff.isDeleted ? undefined : diff.modifiedSnapshotURI ?? diff.modifiedURI, + insertions: diff.added, + deletions: diff.removed, + })); + }); } private _openTransientLastTurnChanges(session: ISession, id: string, changes: IObservable): void { void this._openSessionTurnChanges(session, { id: `${TURN_CHANGES_CHANGESET_ID}:${id}`, - label: localize('lastTurnChanges.label', "Last Turn Changes"), + label: localize('lastTurnChanges.label', "Turn Changes"), description: localize('lastTurnChanges.description', "Changes from the viewed chat's last turn."), changes, }); diff --git a/src/vs/sessions/contrib/chat/test/browser/sessionTurnChanges.test.ts b/src/vs/sessions/contrib/chat/test/browser/sessionTurnChanges.test.ts index 78228324098652..b0228ab8539f90 100644 --- a/src/vs/sessions/contrib/chat/test/browser/sessionTurnChanges.test.ts +++ b/src/vs/sessions/contrib/chat/test/browser/sessionTurnChanges.test.ts @@ -12,7 +12,7 @@ import { isIChatSessionFileChange2 } from '../../../../../workbench/contrib/chat import { IEditorService } from '../../../../../workbench/services/editor/common/editorService.js'; import { IAgentWorkbenchLayoutService } from '../../../../browser/workbench.js'; import { ISessionsService } from '../../../../services/sessions/browser/sessionsService.js'; -import { IChat, ISessionFileChange, ISessionTurnFileChange, TURN_CHANGES_CHANGESET_ID } from '../../../../services/sessions/common/session.js'; +import { IChat, ISessionFileChange, ISessionFolder, ISessionTurnFileChange, ISessionWorkspace, TURN_CHANGES_CHANGESET_ID } from '../../../../services/sessions/common/session.js'; import { IActiveSession, ISessionsManagementService } from '../../../../services/sessions/common/sessionsManagement.js'; import { ISessionChangesEditorOptions, ISessionChangesService } from '../../../changes/browser/sessionChangesService.js'; import { SessionsChatResponseFileChangesService } from '../../browser/sessionTurnChanges.js'; @@ -20,7 +20,7 @@ import { SessionsChatResponseFileChangesService } from '../../browser/sessionTur suite('SessionTurnChanges', () => { const disposables = ensureNoDisposablesAreLeakedInTestSuite(); - test('activates the session and opens live input-pill changes', () => { + test('activates the session and selects Last Turn Changes from the live input pill', () => { const chatResource = URI.parse('chat:session'); const lastTurnChanges = observableValue('lastTurnChanges', [{ uri: URI.file('/workspace/first.ts'), @@ -37,6 +37,7 @@ suite('SessionTurnChanges', () => { }); const session = upcastPartial({ resource: URI.parse('agent-host:session'), + providerId: 'local-agent-host', chats: constObservable([chat]), mainChat: constObservable(chat), }); @@ -93,14 +94,23 @@ suite('SessionTurnChanges', () => { calls: [ { showSession: session.resource.toString(), preserveFocus: true }, { revealEditorPartExplicitly: true }, - { openChangesEditor: session.resource.toString(), changesetId: 'turn:chat:session' }, + { openChangesEditor: session.resource.toString(), changesetId: TURN_CHANGES_CHANGESET_ID }, ], - selectedChanges: ['file:///workspace/second.ts'], + selectedChanges: undefined, }); }); test('opens exact historical request changes as a transient changeset', () => { - const session = upcastPartial({ resource: URI.parse('agent-host:session') }); + const workspaceFolder = URI.file('/workspace'); + const session = upcastPartial({ + resource: URI.parse('agent-host:session'), + workspace: constObservable(upcastPartial({ + folders: [upcastPartial({ + root: workspaceFolder, + workingDirectory: workspaceFolder, + })], + })), + }); const chatResource = URI.parse('chat:session'); const calls: object[] = []; const sessionsManagementService = new class extends mock() { @@ -167,6 +177,15 @@ suite('SessionTurnChanges', () => { identical: false, isFinal: true, isBusy: false, + }, { + originalURI: URI.parse('agenthost:/snapshots/outside-before'), + modifiedURI: URI.file('/outside/ignored.ts'), + added: 1, + removed: 1, + quitEarly: false, + identical: false, + isFinal: true, + isBusy: false, }]), })); @@ -265,8 +284,16 @@ suite('SessionTurnChanges', () => { resource: URI.parse('chat:newer'), updatedAt: constObservable(new Date('2026-08-13T11:00:00Z')), }); + const workspaceFolder = URI.file('/workspace'); const session = upcastPartial({ resource: URI.parse('agent-host:session'), + providerId: 'local-agent-host', + workspace: constObservable(upcastPartial({ + folders: [upcastPartial({ + root: workspaceFolder, + workingDirectory: workspaceFolder, + })], + })), chats: constObservable([chat, newerChat]), mainChat: constObservable(chat), }); @@ -284,11 +311,13 @@ suite('SessionTurnChanges', () => { new class extends mock() { override async openChangesEditor(_sessionResource: URI, options?: ISessionChangesEditorOptions): Promise { const selection = options?.changesetSelection; - if (selection?.kind === 'transient') { + if (selection) { selections.push({ - id: selection.changeset.id, - label: selection.changeset.label, - uris: selection.changeset.changes.get().map(change => isIChatSessionFileChange2(change) ? change.uri.toString() : undefined), + id: selection.kind === 'transient' ? selection.changeset.id : selection.id, + label: selection.kind === 'transient' ? selection.changeset.label : undefined, + uris: selection.kind === 'transient' + ? selection.changeset.changes.get().map(change => isIChatSessionFileChange2(change) ? change.uri.toString() : undefined) + : undefined, }); } return undefined; @@ -316,12 +345,12 @@ suite('SessionTurnChanges', () => { assert.deepStrictEqual(selections, [{ id: 'turn:request', - label: 'Last Turn Changes', + label: 'Turn Changes', uris: ['file:///workspace/response.ts'], }, { - id: 'turn:chat:older', - label: 'Last Turn Changes', - uris: ['file:///workspace/input.ts'], + id: TURN_CHANGES_CHANGESET_ID, + label: undefined, + uris: undefined, }]); }); diff --git a/src/vs/sessions/contrib/providers/agentHost/AGENT_HOST_SESSIONS_PROVIDER.md b/src/vs/sessions/contrib/providers/agentHost/AGENT_HOST_SESSIONS_PROVIDER.md index afdd90f7ff09c1..11b0acce48a947 100644 --- a/src/vs/sessions/contrib/providers/agentHost/AGENT_HOST_SESSIONS_PROVIDER.md +++ b/src/vs/sessions/contrib/providers/agentHost/AGENT_HOST_SESSIONS_PROVIDER.md @@ -114,7 +114,7 @@ A single agent host session uses several distinct identifiers: ## Architecture -- **`AgentHostSessionAdapter`** (`baseAgentHostSessionsProvider.ts`) is the `ISession` implementation. It wraps an `IAgentSessionMetadata` from the backend and exposes the observable session surface (`status`, `title`, `workspace`, `mainChat`, `mode`, …). The base provider keeps a `_sessionCache` of adapters keyed by `rawId`. Adapter capabilities derive from a shared provider-to-capabilities lookup, so one root-state event listener and one catalog scan serve the entire cache; an adapter observes that lookup only after receiving a chat catalog that may need reapplication. Root-state errors and disconnects clear the lookup so stale capabilities are not retained. +- **`AgentHostSessionAdapter`** (`baseAgentHostSessionsProvider.ts`) is the `ISession` implementation. It wraps an `IAgentSessionMetadata` from the backend and exposes the observable session surface (`status`, `title`, `workspace`, `mainChat`, `mode`, …). Client-side changesets may provide an optional `changes` observable; the common changeset implementation uses any defined array from it and otherwise falls back to the host changeset channel. Last Turn Changes supplies the live chat-output reduction while `activeTurn` exists, then returns `undefined` so the finalized host changeset resumes after completion. The base provider keeps a `_sessionCache` of adapters keyed by `rawId`. Adapter capabilities derive from a shared provider-to-capabilities lookup, so one root-state event listener and one catalog scan serve the entire cache; an adapter observes that lookup only after receiving a chat catalog that may need reapplication. Root-state errors and disconnects clear the lookup so stale capabilities are not retained. - **`NewSession`** is a disposable draft (pre-creation) session. Several can be in flight simultaneously; the management layer tears down superseded drafts via `deleteNewSession`. A draft eagerly creates its backend session once authentication settles, then **graduates** into a committed `AgentHostSessionAdapter` on first send. - The base provider is abstract; concrete providers supply: `connection`, `authenticationPending`, `resourceSchemeForProvider`, `_formatSessionTypeLabel`, `_adapterOptions` (workspace builder), `resolveWorkspace`, and optionally `_diffUriMapper`. diff --git a/src/vs/sessions/contrib/providers/agentHost/browser/agentHostSessionChangesets.ts b/src/vs/sessions/contrib/providers/agentHost/browser/agentHostSessionChangesets.ts index f3bea7d62d3ddb..fcd4052afcb800 100644 --- a/src/vs/sessions/contrib/providers/agentHost/browser/agentHostSessionChangesets.ts +++ b/src/vs/sessions/contrib/providers/agentHost/browser/agentHostSessionChangesets.ts @@ -31,6 +31,14 @@ const enum ChangesetKind { Compare = 'compare-turns', } +export interface IAgentHostChangeset extends Changeset { + /** + * Optional authoritative changes. `undefined` falls back to the changeset + * channel; an array, including an empty one, is used as-is. + */ + readonly changes?: IObservable; +} + /** * Returns the workspace file URI that identifies a change, matching the * convention used by {@link sessionFileChangesEqual}: the `uri` of a @@ -74,7 +82,7 @@ export function createChangesets( sessionUri: URI, options: IAgentHostAdapterOptions, isActiveSessionObs: IObservable, - changesets: readonly Changeset[] | undefined + changesets: readonly IAgentHostChangeset[] | undefined, ): readonly ISessionChangeset[] { if (!changesets) { return []; @@ -223,7 +231,7 @@ abstract class AbstractAgentHostChangeset implements ISessionChangeset { private readonly _changesetFilesObs: IObservable; constructor( - changeset: Changeset, + changeset: IAgentHostChangeset & { isDefault: boolean }, private readonly _options: IAgentHostAdapterOptions, private readonly _dialogService: IDialogService, ) { @@ -231,7 +239,24 @@ abstract class AbstractAgentHostChangeset implements ISessionChangeset { review: changeset.capabilities?.review !== undefined } satisfies ISessionChangesetCapabilities; + const providedChangesObs = derivedObservableWithCache(this, (reader, lastValue) => { + const providedChanges = changeset.changes?.read(reader); + if (providedChanges !== undefined) { + return providedChanges; + } + if (lastValue === undefined) { + return undefined; + } + const changesetState = this.changesetStateObs.read(reader).read(reader); + return changesetState && !(changesetState instanceof Error) && changesetState.status === ChangesetStatus.Ready + ? undefined + : lastValue; + }); + this.isLoadingChanges = derived(reader => { + if (providedChangesObs.read(reader) !== undefined) { + return false; + } const changesetState = this.changesetStateObs.read(reader).read(reader); // If the changeset state is `undefined`, it means that the first snapshot @@ -289,7 +314,8 @@ abstract class AbstractAgentHostChangeset implements ISessionChangeset { }); this.changes = derivedOpts({ equalsFn: sessionFileChangesEqual }, reader => { - return this._filterChanges(changesObs.read(reader) ?? [], reader); + const changes = providedChangesObs.read(reader) ?? changesObs.read(reader) ?? []; + return this._filterChanges(changes, reader); }); const operationsObs = derivedObservableWithCache(this, (reader, lastValue) => { @@ -413,7 +439,7 @@ class AgentHostChangeset extends AbstractAgentHostChangeset { constructor( options: IAgentHostAdapterOptions, isActiveSessionObs: IObservable, - changesetSummary: Changeset & { isDefault: boolean }, + changesetSummary: IAgentHostChangeset & { isDefault: boolean }, @IDialogService dialogService: IDialogService, ) { super(changesetSummary, options, dialogService); @@ -457,7 +483,7 @@ class AgentHostLastTurnChangeset extends AbstractAgentHostChangeset { sessionUri: URI, options: IAgentHostAdapterOptions, isActiveSessionObs: IObservable, - changesetSummary: Changeset & { isDefault: boolean }, + changesetSummary: IAgentHostChangeset & { isDefault: boolean }, @IDialogService dialogService: IDialogService, ) { super(changesetSummary, options, dialogService); diff --git a/src/vs/sessions/contrib/providers/agentHost/browser/agentHostSessionFiles.ts b/src/vs/sessions/contrib/providers/agentHost/browser/agentHostSessionFiles.ts index 77daace85fb030..7bb982dce3558c 100644 --- a/src/vs/sessions/contrib/providers/agentHost/browser/agentHostSessionFiles.ts +++ b/src/vs/sessions/contrib/providers/agentHost/browser/agentHostSessionFiles.ts @@ -112,16 +112,27 @@ export function createSessionOutputObs( constObservable(sessionUri), ); + const lastTurnChangesByChat = new Map>(); + const pruneLastTurnChanges = (chatUris: readonly URI[]): readonly URI[] => { + const chatKeys = new Set(chatUris.map(uri => uri.toString())); + for (const key of lastTurnChangesByChat.keys()) { + if (!chatKeys.has(key)) { + lastTurnChangesByChat.delete(key); + } + } + return chatUris; + }; + // All chat URIs in the session (default chat + any peer chats). File edits // can be produced by any chat, so we union edits across all of them. const chatUrisObs = derivedOpts({ equalsFn: (a, b) => a.length === b.length && a.every((u, i) => isEqual(u, b[i])) }, reader => { if (!enabledObs.read(reader)) { - return []; + return pruneLastTurnChanges([]); } const sessionState = sessionStateObs.read(reader).read(reader); const defaultChatUri = URI.parse(buildDefaultChatUri(sessionUri)); if (!sessionState || sessionState instanceof Error) { - return [defaultChatUri]; + return pruneLastTurnChanges([defaultChatUri]); } const uris = new Map(); @@ -130,7 +141,7 @@ export function createSessionOutputObs( const uri = URI.parse(chat.resource); uris.set(uri.toString(), uri); } - return [...uris.values()]; + return pruneLastTurnChanges([...uris.values()]); }); // One observable of parsed edits per chat, subscribing to that chat's state. @@ -174,15 +185,23 @@ export function createSessionOutputObs( return reduceSessionFiles(allEdits, folderRoots); }); - const getLastTurnChanges = (chatUri: URI): IObservable => - derivedOpts({ equalsFn: sessionTurnFileChangesEqual }, reader => { - const folderRoots = getWorkspaceAndWorktreeRoots(workspaceObs.read(reader)); - const chatEdits = editsPerChatObs.read(reader).find(entry => isEqual(entry.chatUri, chatUri)); - if (chatEdits) { - return reduceTurnChanges(chatEdits.edits.read(reader).lastTurnEdits, folderRoots, cache); - } - return []; - }); + // The active-turn changeset requests this reactively, so reuse one observable per chat. + const getLastTurnChanges = (chatUri: URI): IObservable => { + const key = chatUri.toString(); + let changes = lastTurnChangesByChat.get(key); + if (!changes) { + changes = derivedOpts({ equalsFn: sessionTurnFileChangesEqual }, reader => { + const folderRoots = getWorkspaceAndWorktreeRoots(workspaceObs.read(reader)); + const chatEdits = editsPerChatObs.read(reader).find(entry => isEqual(entry.chatUri, chatUri)); + if (chatEdits) { + return reduceTurnChanges(chatEdits.edits.read(reader).lastTurnEdits, folderRoots, cache); + } + return []; + }); + lastTurnChangesByChat.set(key, changes); + } + return changes; + }; return { externalFiles, getLastTurnChanges }; } diff --git a/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts b/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts index fcc9f5dff085a9..0e31e689670943 100644 --- a/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts +++ b/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts @@ -30,7 +30,7 @@ import type { IAgentSubscription } from '../../../../../platform/agentHost/commo import { ResolveSessionConfigResult, type SessionConfigPropertySchema } from '../../../../../platform/agentHost/common/state/protocol/commands.js'; import { AgentCustomization, ChangesSummary, ChatInteractivity as ProtocolChatInteractivity, ChatOriginKind as ProtocolChatOriginKind, type ClientPluginCustomization, Customization, CustomizationEnablementKind, CustomizationType, type CustomizationEnablement, ModelSelection, SessionStatus as ProtocolSessionStatus, RootConfigState, RootState, SessionState, SessionSummary, type Changeset } from '../../../../../platform/agentHost/common/state/protocol/state.js'; import { ActionType, isChatAction, isSessionAction, NotificationType } from '../../../../../platform/agentHost/common/state/sessionActions.js'; -import { AgentCapabilities, AgentInfo, buildChatUri, buildDefaultChatUri, getSessionRelatedPullRequestUrls, isDefaultChatUri, isSessionStatusArchived, isSessionStatusRead, parseChatUri, readSessionEhcliAdoptable, readSessionExternal, readSessionGitHubState, readSessionGitState, readSessionMultiRootMetadata, readSessionSourceControlState, readSessionWorkspaceless, ROOT_STATE_URI, SESSION_META_MULTI_ROOT_KEY, SessionMeta, SessionSourceControlOutcome, StateComponents, withSessionExternal, withSessionGitHubState, withSessionMultiRootMetadata, withSessionStatusFlag, withSessionWorkspaceless, type ChatSummary, type ISessionGitHubState, type ISessionGitState, type ISessionMultiRootMetadata } from '../../../../../platform/agentHost/common/state/sessionState.js'; +import { AgentCapabilities, AgentInfo, buildChatUri, buildDefaultChatUri, getSessionRelatedPullRequestUrls, isDefaultChatUri, isSessionStatusArchived, isSessionStatusRead, parseChatUri, readSessionEhcliAdoptable, readSessionExternal, readSessionGitHubState, readSessionGitState, readSessionMultiRootMetadata, readSessionSourceControlState, readSessionWorkspaceless, ROOT_STATE_URI, SESSION_META_MULTI_ROOT_KEY, SessionMeta, SessionSourceControlOutcome, StateComponents, withSessionExternal, withSessionGitHubState, withSessionMultiRootMetadata, withSessionStatusFlag, withSessionWorkspaceless, type ChatState, type ChatSummary, type ISessionGitHubState, type ISessionGitState, type ISessionMultiRootMetadata } from '../../../../../platform/agentHost/common/state/sessionState.js'; import { IConfigurationService } from '../../../../../platform/configuration/common/configuration.js'; import { IInstantiationService } from '../../../../../platform/instantiation/common/instantiation.js'; import { ILogService } from '../../../../../platform/log/common/log.js'; @@ -50,14 +50,14 @@ import { getRegisteredLanguageModels, resolveConfiguredModel, resolveModelIdenti import { buildMutableConfigSchema, IAgentHostMcpServer, IAgentHostSessionsProvider, resolvedConfigsEqual } from '../../../../common/agentHostSessionsProvider.js'; import { agentHostSessionWorkspaceKey } from '../../../../common/agentHostSessionWorkspace.js'; import { isSessionConfigComplete } from '../../../../common/sessionConfig.js'; -import { ChatInteractivity, ChatOriginKind, DEFAULT_CHAT_CAPABILITIES, effectiveChatInteractivity, IChat, IChatCapabilities, IGitHubInfo, IGitHubIssueRef, IGitHubPullRequestRef, ISession, ISessionAgentRef, ISessionCapabilities, ISessionChangeset, ISessionChangesSummary, ISessionFile, ISessionFileChange, ISessionTurnFileChange, ISessionType, ISessionWorkspace, ISessionWorkspaceBrowseAction, ISideChatSelection, sessionFileChangesEqual, sessionWorkspaceEqual, SessionStatus, SessionTypeAuthRequirement, toSessionId } from '../../../../services/sessions/common/session.js'; +import { ChatInteractivity, ChatOriginKind, DEFAULT_CHAT_CAPABILITIES, effectiveChatInteractivity, IChat, IChatCapabilities, IGitHubInfo, IGitHubIssueRef, IGitHubPullRequestRef, ISession, ISessionAgentRef, ISessionCapabilities, ISessionChangeset, ISessionChangesSummary, ISessionFile, ISessionFileChange, ISessionTurnFileChange, ISessionType, ISessionWorkspace, ISessionWorkspaceBrowseAction, ISideChatSelection, sessionFileChangesEqual, sessionWorkspaceEqual, SessionStatus, SessionTypeAuthRequirement, toSessionId, TURN_CHANGES_CHANGESET_ID } from '../../../../services/sessions/common/session.js'; import { ISessionsService } from '../../../../services/sessions/browser/sessionsService.js'; import { IDeleteChatOptions, ISendRequestOptions, ISessionChangeEvent, ISessionModelPickerOptions, ISessionModelsSnapshot, ISessionsProviderCreateSessionOptions, ISessionWorktreeConfiguration } from '../../../../services/sessions/common/sessionsProvider.js'; import { IGitHubService } from '../../../github/browser/githubService.js'; import { computeSessionPullRequestIcon } from '../../../github/browser/pullRequestIconStatus.js'; import { IPullRequestIconCache } from '../../../github/browser/pullRequestIconCache.js'; import { mapProtocolStatus } from './agentHostDiffs.js'; -import { createChangesets } from './agentHostSessionChangesets.js'; +import { createActiveSessionSubscriptionObs, createChangesets, IAgentHostChangeset, selectMostRecentChatUri } from './agentHostSessionChangesets.js'; import { createSessionOutputObs, ISessionOutputObs } from './agentHostSessionFiles.js'; const STORAGE_KEY_REMEMBERED_SESSION_CONFIG_VALUES = 'sessions.agentHost.sessionConfigPicker.selectedValues'; @@ -627,6 +627,8 @@ export class AgentHostSessionAdapter extends Disposable implements ISession { * that chat's own last turn. */ private readonly _sessionOutput: ISessionOutputObs; + /** Live workspace changes while the session's most recent chat has an active turn. */ + private readonly _currentTurnChanges: IObservable; /** * Independent title override for the default chat tab. `undefined` means the * default chat inherits the session title; a non-empty value means the user @@ -876,6 +878,7 @@ export class AgentHostSessionAdapter extends Disposable implements ISession { this._sessionOutputCache, ); this._sessionOutput = sessionOutput; + this._currentTurnChanges = this._createCurrentTurnChangesObservable(); this.externalChanges = sessionOutput.externalFiles; const mainChat: IChat = { @@ -1468,10 +1471,41 @@ export class AgentHostSessionAdapter extends Disposable implements ISession { return; } - const changesets = createChangesets(this.backendUri, this._options, this.isActiveSessionObs, changesetsMetadata); + const changesets = createChangesets(this.backendUri, this._options, this.isActiveSessionObs, changesetsMetadata.map(changeset => ({ + ...changeset, + changes: changeset.changeKind === TURN_CHANGES_CHANGESET_ID + ? this._currentTurnChanges + : undefined, + } satisfies IAgentHostChangeset))); this.changesets.set(changesets, undefined); } + + private _createCurrentTurnChangesObservable(): IObservable { + const sessionStateObs = createActiveSessionSubscriptionObs( + this._options, + this.isActiveSessionObs, + StateComponents.Session, + constObservable(this.backendUri), + ); + const mostRecentChatUriObs = derivedOpts({ equalsFn: isEqual }, reader => { + return selectMostRecentChatUri(sessionStateObs.read(reader).read(reader), this.backendUri); + }); + const chatStateObs = createActiveSessionSubscriptionObs( + this._options, + this.isActiveSessionObs, + StateComponents.Chat, + mostRecentChatUriObs, + ); + return derived(reader => { + const chatState = chatStateObs.read(reader).read(reader); + if (!chatState || chatState instanceof Error || !chatState.activeTurn) { + return undefined; + } + return this._sessionOutput.getLastTurnChanges(mostRecentChatUriObs.read(reader)).read(reader) + .filter(change => !change.isOutsideWorkspace); + }); + } } /** 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 5071b7560f9767..bb5910f5ac6b9c 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 @@ -20,7 +20,7 @@ import { AgentHostCodexAgentEnabledSettingId, IAgentHostService } from '../../.. import type { IAgentSubscription } from '../../../../../../platform/agentHost/common/state/agentSubscription.js'; import type { ResolveSessionConfigResult } from '../../../../../../platform/agentHost/common/state/protocol/commands.js'; import { ChatInteractivity as ProtocolChatInteractivity, ChatOriginKind as ProtocolChatOriginKind, CustomizationEnablementKind, CustomizationLoadStatus, CustomizationType, McpServerStatus, MessageKind, SessionLifecycle, type AgentCustomization, type AgentInfo, type ChangesSummary, type Customization, type RootState, type SessionActiveClient, type SessionConfigState, type SessionState, type SessionSummary } from '../../../../../../platform/agentHost/common/state/protocol/state.js'; -import { buildChatUri, buildDefaultChatUri, buildSubagentChatUri, ChangesetStatus, SessionSourceControlOutcome, SessionStatus as ProtocolSessionStatus, StateComponents, withSessionEhcliAdoptable, withSessionGitHubState, withSessionGitState, withSessionMultiRootMetadata, withSessionSourceControlState, withSessionWorkspaceless, type ChangesetState, type ChatState, type ChatSummary } from '../../../../../../platform/agentHost/common/state/sessionState.js'; +import { buildChatUri, buildDefaultChatUri, buildSubagentChatUri, ChangesetStatus, ResponsePartKind, SessionSourceControlOutcome, SessionStatus as ProtocolSessionStatus, StateComponents, ToolCallConfirmationReason, ToolCallStatus, ToolResultContentType, TurnState, withSessionEhcliAdoptable, withSessionGitHubState, withSessionGitState, withSessionMultiRootMetadata, withSessionSourceControlState, withSessionWorkspaceless, type ChangesetState, type ChatState, type ChatSummary } from '../../../../../../platform/agentHost/common/state/sessionState.js'; import { ActionType, NotificationType, type ActionEnvelope, type IRootConfigChangedAction, type ChatAction, type SessionAction, type TerminalAction, type INotification, type ClientAnnotationsAction } from '../../../../../../platform/agentHost/common/state/sessionActions.js'; import { SessionConfigKey } from '../../../../../../platform/agentHost/common/sessionConfigKeys.js'; import { ConfigurationTarget, IConfigurationService } from '../../../../../../platform/configuration/common/configuration.js'; @@ -40,7 +40,7 @@ import { ChatModeKind } from '../../../../../../workbench/contrib/chat/common/co import { ILanguageModelsService, type ILanguageModelChatMetadata } from '../../../../../../workbench/contrib/chat/common/languageModels.js'; import type { IChatModel, IChatModelInputState, IInputModel } from '../../../../../../workbench/contrib/chat/common/model/chatModel.js'; import { ISessionChangeEvent } from '../../../../../services/sessions/common/sessionsProvider.js'; -import { ChatInteractivity, ChatOriginKind, getChatCapabilities, ISession, SessionStatus } from '../../../../../services/sessions/common/session.js'; +import { ChatInteractivity, ChatOriginKind, getChatCapabilities, ISession, SessionStatus, TURN_CHANGES_CHANGESET_ID } from '../../../../../services/sessions/common/session.js'; import { IActiveSession } from '../../../../../services/sessions/common/sessionsManagement.js'; import { ISessionsService } from '../../../../../services/sessions/browser/sessionsService.js'; import { IAgentCustomizationScope, IAgentHostActiveClientService } from '../../../../../../workbench/contrib/chat/browser/agentSessions/agentHost/agentHostActiveClientService.js'; @@ -4747,6 +4747,113 @@ suite('LocalAgentHostSessionsProvider', () => { assert.strictEqual(session!.workspace.get(), undefined); })); + test('Last Turn Changes uses live chat edits before the host changeset updates', () => runWithFakedTimers({ useFakeTimers: true }, async () => { + const workingDirectory = URI.file('/repo'); + agentHost.addSession(createSession('live-turn-changes', { summary: 'Live Turn Changes', workingDirectory })); + const activeSession = observableValue('activeSession', undefined); + const provider = createProvider(disposables, agentHost, undefined, { activeSession }); + provider.getSessions(); + await timeout(0); + + const session = provider.getSessions().find(candidate => candidate.title.get() === 'Live Turn Changes'); + assert.ok(session); + activeSession.set(session as IActiveSession, undefined); + + const sessionUri = AgentSession.uri('copilotcli', 'live-turn-changes').toString(); + const chatUri = buildDefaultChatUri(sessionUri); + agentHost.setSessionState('live-turn-changes', 'copilotcli', { + provider: 'copilotcli', + title: 'Live Turn Changes', + status: ProtocolSessionStatus.InProgress, + lifecycle: SessionLifecycle.Ready, + activeClients: [], + defaultChat: chatUri, + chats: [{ + resource: chatUri, + title: 'Default', + status: ProtocolSessionStatus.InProgress, + modifiedAt: new Date(0).toISOString(), + }], + workingDirectories: [workingDirectory.toString()], + }); + assert.ok(session instanceof AgentHostSessionAdapter); + session.updateChangesets([{ + label: 'Last Turn Changes', + uriTemplate: `${sessionUri}/changeset/turn/{turnId}`, + changeKind: 'turn', + }]); + const changedFile = URI.file('/repo/live.ts'); + const externalFile = URI.file('/outside/ignored.ts'); + agentHost.setChatState(chatUri, { + resource: chatUri, + title: 'Default', + status: ProtocolSessionStatus.InProgress, + modifiedAt: new Date().toISOString(), + turns: [], + activeTurn: { + id: 'active-turn', + startedAt: new Date().toISOString(), + message: { text: 'Edit live.ts', origin: { kind: MessageKind.User } }, + responseParts: [{ + kind: ResponsePartKind.ToolCall, + toolCall: { + status: ToolCallStatus.Completed, + toolCallId: 'edit-live', + toolName: 'create', + displayName: 'Create File', + invocationMessage: 'Creating live.ts', + success: true, + pastTenseMessage: 'Created live.ts', + confirmed: ToolCallConfirmationReason.NotNeeded, + content: [{ + type: ToolResultContentType.FileEdit, + after: { uri: changedFile.toString(), content: { uri: changedFile.toString() } }, + diff: { added: 1, removed: 0 }, + }, { + type: ToolResultContentType.FileEdit, + after: { uri: externalFile.toString(), content: { uri: externalFile.toString() } }, + diff: { added: 1, removed: 0 }, + }], + }, + }], + usage: undefined, + }, + }); + + const changeset = session!.changesets.get()?.find(candidate => candidate.id === TURN_CHANGES_CHANGESET_ID); + assert.deepStrictEqual({ + isLoading: changeset?.isLoadingChanges.get(), + changes: changeset?.changes.get().map(change => isIChatSessionFileChange2(change) ? change.uri.toString() : change.modifiedUri.toString()), + }, { + isLoading: false, + changes: [changedFile.toString()], + }); + + const changesetUri = `${sessionUri}/changeset/turn/active-turn`; + agentHost.setChangesetState(changesetUri, { status: ChangesetStatus.Computing, files: [] }); + agentHost.setChatState(chatUri, { + resource: chatUri, + title: 'Default', + status: ProtocolSessionStatus.Idle, + modifiedAt: new Date().toISOString(), + turns: [{ + id: 'active-turn', + message: { text: 'Edit live.ts', origin: { kind: MessageKind.User } }, + responseParts: [], + usage: undefined, + state: TurnState.Complete, + }], + }); + const whileComputing = changeset?.changes.get().map(change => isIChatSessionFileChange2(change) ? change.uri.toString() : change.modifiedUri.toString()); + agentHost.setChangesetState(changesetUri, { status: ChangesetStatus.Ready, files: [] }); + const afterReady = changeset?.changes.get().map(change => isIChatSessionFileChange2(change) ? change.uri.toString() : change.modifiedUri.toString()); + + assert.deepStrictEqual({ whileComputing, afterReady }, { + whileComputing: [changedFile.toString()], + afterReady: [], + }); + })); + test('session adapter uses raw ID as fallback title', () => runWithFakedTimers({ useFakeTimers: true }, async () => { agentHost.addSession(createSession('abcdef1234567890')); From 90dde50c2f56c711c16dc79b6225a979300fb6f4 Mon Sep 17 00:00:00 2001 From: vritant24 Date: Mon, 17 Aug 2026 11:42:20 -0700 Subject: [PATCH 07/19] chat: describe BYOK Responses failures that omit error details A provider can end a Responses stream with a terminal event whose error object omits the code and message the API contract requires. Serializing that object produced {"code":0,"message":"","metadata":{}}, which the BYOK endpoint promotes to the entire user-facing reason, so an outage and a malformed request looked identical and neither users nor maintainers could tell them apart. Describe such failures instead, naming the terminal event, response status, and response id so the failure stays correlatable with the provider, and omit absent metadata keys so serialization cannot collapse them to an empty object. Keep the fetcher's original reason when a stream error carries no message, so a hollow struct never replaces an actionable one. The mapping predates the reports; the Agent Host BYOK proxy moving from Chat Completions to Responses in 1.132.0 is what first exposed it in the Agents Window. This restores diagnosability rather than stopping the upstream failure, so it does not close the report on its own. Related to #330408 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../src/extension/byok/node/openAIEndpoint.ts | 4 +- .../byok/node/test/openAIEndpoint.spec.ts | 59 +++++++++++++ .../platform/endpoint/node/responsesApi.ts | 60 +++++++++++-- .../endpoint/node/test/responsesApi.spec.ts | 87 ++++++++++++++++++- .../copilotByokResponses.integrationTest.ts | 49 +++++++++-- 5 files changed, 245 insertions(+), 14 deletions(-) diff --git a/extensions/copilot/src/extension/byok/node/openAIEndpoint.ts b/extensions/copilot/src/extension/byok/node/openAIEndpoint.ts index 7193675b8be904..deba6a0d0c4461 100644 --- a/extensions/copilot/src/extension/byok/node/openAIEndpoint.ts +++ b/extensions/copilot/src/extension/byok/node/openAIEndpoint.ts @@ -25,7 +25,9 @@ function hydrateBYOKErrorMessages(response: ChatResponse): ChatResponse { type: response.type, requestId: response.requestId, serverRequestId: response.serverRequestId, - reason: JSON.stringify(response.streamError), + // A stream error carrying no message has no diagnostic value, so keep the + // original reason rather than replacing it with a hollow serialized struct. + reason: response.streamError.message ? JSON.stringify(response.streamError) : response.reason, }; } else if (response.type === ChatFetchResponseType.RateLimited) { return { diff --git a/extensions/copilot/src/extension/byok/node/test/openAIEndpoint.spec.ts b/extensions/copilot/src/extension/byok/node/test/openAIEndpoint.spec.ts index 131759488be572..716e2135ff399a 100644 --- a/extensions/copilot/src/extension/byok/node/test/openAIEndpoint.spec.ts +++ b/extensions/copilot/src/extension/byok/node/test/openAIEndpoint.spec.ts @@ -489,6 +489,65 @@ describe('OpenAIEndpoint - Reasoning Properties', () => { expect(parentRequestSpy.mock.calls[0][0].ignoreStatefulMarker).toBe(false); }); + // Regression for https://github.com/microsoft/vscode/issues/330408 + // + // BYOK promotes the serialized stream error to the whole user-facing reason. A + // message-less error must not displace the reason the fetcher already computed, + // otherwise the user is left with an empty struct and nothing to act on. + it('issue #330408: keeps the original reason when a stream error carries no message', async () => { + const endpoint = instaService.createInstance(OpenAIEndpoint, + modelMetadata, + 'test-api-key', + 'https://api.openai.com/v1/responses'); + const parentResponse: ChatResponse = { + type: ChatFetchResponseType.Failed, + requestId: 'request-id', + serverRequestId: 'server-request-id', + reason: 'Server error. Stream terminated', + streamError: { code: 0, message: '', metadata: {} }, + }; + vi.spyOn(ChatEndpoint.prototype, 'makeChatRequest2').mockResolvedValue(parentResponse); + + const response = await endpoint.makeChatRequest2( + createMakeRequestOptions([ + { + role: Raw.ChatRole.User, + content: [{ type: Raw.ChatCompletionContentPartKind.Text, text: 'hello' }] + } + ]), + CancellationToken.None, + ); + + expect(response.type === ChatFetchResponseType.Failed && response.reason).toBe('Server error. Stream terminated'); + }); + + it('surfaces the serialized stream error when it carries a message', async () => { + const endpoint = instaService.createInstance(OpenAIEndpoint, + modelMetadata, + 'test-api-key', + 'https://api.openai.com/v1/responses'); + const parentResponse: ChatResponse = { + type: ChatFetchResponseType.Failed, + requestId: 'request-id', + serverRequestId: 'server-request-id', + reason: 'Server error. Stream terminated', + streamError: { code: 0, message: 'something broke', metadata: { code: 'server_error' } }, + }; + vi.spyOn(ChatEndpoint.prototype, 'makeChatRequest2').mockResolvedValue(parentResponse); + + const response = await endpoint.makeChatRequest2( + createMakeRequestOptions([ + { + role: Raw.ChatRole.User, + content: [{ type: Raw.ChatCompletionContentPartKind.Text, text: 'hello' }] + } + ]), + CancellationToken.None, + ); + + expect(response.type === ChatFetchResponseType.Failed && response.reason).toBe('{"code":0,"message":"something broke","metadata":{"code":"server_error"}}'); + }); + it('disables marker reuse and store for ZDR Responses requests', () => { const endpoint = instaService.createInstance(OpenAIEndpoint, { diff --git a/extensions/copilot/src/platform/endpoint/node/responsesApi.ts b/extensions/copilot/src/platform/endpoint/node/responsesApi.ts index 469a736338b053..ca47568cdd37cb 100644 --- a/extensions/copilot/src/platform/endpoint/node/responsesApi.ts +++ b/extensions/copilot/src/platform/endpoint/node/responsesApi.ts @@ -1102,20 +1102,66 @@ function extractFilterReasonFromContentFilters(filters: CapiContentFilterEntry[] return undefined; } +/** + * Identifying details for the terminal Responses event that carried an error. + * Used to describe failures whose error object omits the fields the API + * contract requires. + */ +interface IResponsesErrorContext { + /** The terminal SSE event that carried the error, e.g. `response.failed`. */ + readonly eventType: string; + /** `id` from the response envelope, so a user report stays correlatable upstream. */ + readonly responseId?: string; + /** `status` from the response envelope, when present. */ + readonly responseStatus?: string; +} + +function toResponsesErrorContext(eventType: string, response?: Pick): IResponsesErrorContext { + return { + eventType, + responseId: response?.id || undefined, + responseStatus: response?.status || undefined, + }; +} + +/** + * Describe a terminal error that carries no usable message. Names only the + * event, status, response id, and provider code so the failure stays + * diagnosable and correlatable without exposing prompt content. + */ +function describeUninformativeResponsesError(code: string | undefined, context: IResponsesErrorContext): string { + const details = [ + `event: ${context.eventType}`, + ...(context.responseStatus ? [`status: ${context.responseStatus}`] : []), + ...(context.responseId ? [`response: ${context.responseId}`] : []), + ].join(', '); + return code + ? `The model provider reported a failed response with code '${code}' and no error message (${details}).` + : `The model provider reported a failed response without any error details (${details}).`; +} + /** * Map a Responses-API `response.error` (string-coded per the OpenAI SDK) onto * our {@link APIErrorResponse} shape (numeric `code`). We can't preserve the * string code in `code`, so we stash it in `metadata.code` for BYOK diagnostics - * (which `JSON.stringify` the whole struct). + * (which `JSON.stringify` the whole struct). Providers do terminate streams with + * an error object that omits `code`/`message` entirely, so those are described + * rather than serialized as an empty struct that tells the user nothing. */ -function mapResponsesApiError(err: OpenAI.Responses.ResponseError | null | undefined): APIErrorResponse | undefined { +function mapResponsesApiError(err: OpenAI.Responses.ResponseError | null | undefined, context: IResponsesErrorContext): APIErrorResponse | undefined { if (!err) { return undefined; } + const code = typeof err.code === 'string' && err.code ? err.code : undefined; + const message = typeof err.message === 'string' && err.message ? err.message : undefined; return { code: 0, - message: err.message ?? '', - metadata: { code: err.code }, + message: message ?? describeUninformativeResponsesError(code, context), + // Omit absent keys so `JSON.stringify` cannot collapse metadata to `{}`. + metadata: { + ...(code ? { code } : {}), + ...(context.responseId ? { responseId: context.responseId } : {}), + }, }; } @@ -1206,7 +1252,7 @@ export class OpenAIResponsesProcessor { return this.buildTerminalCompletion( { output: [] } as unknown as CapiResponseTerminalEvent['response'], FinishedCompletionReason.ServerError, - { error: mapResponsesApiError({ code: chunk.code, message: chunk.message } as OpenAI.Responses.ResponseError) } + { error: mapResponsesApiError({ code: chunk.code, message: chunk.message } as OpenAI.Responses.ResponseError, toResponsesErrorContext('error')) } ); case 'response.output_text.delta': { const capiChunk: CapiResponsesTextDeltaEvent = chunk; @@ -1428,13 +1474,13 @@ export class OpenAIResponsesProcessor { } return this.buildTerminalCompletion(incomplete, finishReason, { filterReason, - error: mapResponsesApiError(incomplete.error), + error: mapResponsesApiError(incomplete.error, toResponsesErrorContext('response.incomplete', incomplete)), }); } case 'response.failed': { const failed = chunk.response as CapiResponseTerminalEvent['response']; return this.buildTerminalCompletion(failed, FinishedCompletionReason.ServerError, { - error: mapResponsesApiError(failed.error), + error: mapResponsesApiError(failed.error, toResponsesErrorContext('response.failed', failed)), }); } } diff --git a/extensions/copilot/src/platform/endpoint/node/test/responsesApi.spec.ts b/extensions/copilot/src/platform/endpoint/node/test/responsesApi.spec.ts index 96dc9a8791c703..e2313d916b05c2 100644 --- a/extensions/copilot/src/platform/endpoint/node/test/responsesApi.spec.ts +++ b/extensions/copilot/src/platform/endpoint/node/test/responsesApi.spec.ts @@ -2000,7 +2000,92 @@ describe('processResponseFromChatEndpoint terminal events', () => { expect(completion.error).toEqual({ code: 0, message: 'something broke', - metadata: { code: 'internal_error' }, + metadata: { code: 'internal_error', responseId: 'resp_failed' }, + }); + }); + + // Regression for https://github.com/microsoft/vscode/issues/330408 + // + // A provider can terminate a Responses stream with `response.failed` while sending an + // error object that omits the `code`/`message` the API contract requires. Serializing + // that struct verbatim produced `{"code":0,"message":"","metadata":{}}`, which the BYOK + // endpoint surfaces as the entire user-facing reason — leaving no way to tell an outage + // from a malformed request. The failure must still be described and correlatable. + it('issue #330408: describes a response.failed event whose error omits code and message', async () => { + const failedEvent = { + type: 'response.failed', + response: { + id: 'resp_failed', + model: 'gpt-5-mini', + created_at: 123, + status: 'failed', + error: {}, + output: [], + }, + }; + + const [completion] = await runStream(`data: ${JSON.stringify(failedEvent)}\n\n`); + + expect({ + finishReason: completion.finishReason, + error: completion.error, + }).toEqual({ + finishReason: FinishedCompletionReason.ServerError, + error: { + code: 0, + message: `The model provider reported a failed response without any error details (event: response.failed, status: failed, response: resp_failed).`, + metadata: { responseId: 'resp_failed' }, + }, + }); + }); + + it('issue #330408: describes a terminal error that carries a code but no message', async () => { + const failedEvent = { + type: 'response.failed', + response: { + id: 'resp_failed', + model: 'gpt-5-mini', + created_at: 123, + status: 'failed', + error: { code: 'server_error' }, + output: [], + }, + }; + + const [completion] = await runStream(`data: ${JSON.stringify(failedEvent)}\n\n`); + + expect(completion.error).toEqual({ + code: 0, + message: `The model provider reported a failed response with code 'server_error' and no error message (event: response.failed, status: failed, response: resp_failed).`, + metadata: { code: 'server_error', responseId: 'resp_failed' }, + }); + }); + + it('issue #330408: describes a response.incomplete event whose error omits code and message', async () => { + const incompleteEvent = { + type: 'response.incomplete', + response: { + id: 'resp_incomplete', + model: 'gpt-5-mini', + created_at: 123, + status: 'incomplete', + error: {}, + output: [], + }, + }; + + const [completion] = await runStream(`data: ${JSON.stringify(incompleteEvent)}\n\n`); + + expect({ + finishReason: completion.finishReason, + error: completion.error, + }).toEqual({ + finishReason: FinishedCompletionReason.ServerError, + error: { + code: 0, + message: `The model provider reported a failed response without any error details (event: response.incomplete, status: incomplete, response: resp_incomplete).`, + metadata: { responseId: 'resp_incomplete' }, + }, }); }); diff --git a/src/vs/platform/agentHost/test/node/providerIntegration/copilotByokResponses.integrationTest.ts b/src/vs/platform/agentHost/test/node/providerIntegration/copilotByokResponses.integrationTest.ts index 7c6e8e96244fc4..e9d5c498462638 100644 --- a/src/vs/platform/agentHost/test/node/providerIntegration/copilotByokResponses.integrationTest.ts +++ b/src/vs/platform/agentHost/test/node/providerIntegration/copilotByokResponses.integrationTest.ts @@ -6,7 +6,7 @@ import assert from 'assert'; import { mkdtemp, rm } from 'fs/promises'; import { tmpdir } from 'os'; -import { CopilotClient } from '@github/copilot-sdk'; +import { CopilotClient, defineTool } from '@github/copilot-sdk'; import { Emitter } from '../../../../../base/common/event.js'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../base/test/common/utils.js'; import { NullLogService } from '../../../../log/common/log.js'; @@ -20,10 +20,11 @@ suite('Agent Host Provider Integration - Copilot BYOK Responses', function () { const store = ensureNoDisposablesAreLeakedInTestSuite(); - test('bundled SDK consumes structured reasoning and text from the proxy', async function () { + test('bundled SDK preserves parameterless tools, tool continuations, reasoning, and text', async function () { this.timeout(120_000); const sessionId = 'byok-responses-integration'; + const parameterlessToolName = 'parameterless_tool'; const baseDirectory = await mkdtemp(`${tmpdir()}/byok-responses-sdk-`); const models = store.add(new Emitter()); const registry = new ByokLmBridgeRegistry(); @@ -31,9 +32,15 @@ suite('Agent Host Provider Integration - Copilot BYOK Responses', function () { const registration = registry.register('client', { chat: async request => { captured.push(request); - if (captured.length > 1) { + if (captured.length === 2) { return { responseId: 'resp_provider_2', + output: [{ type: 'message', content: [{ type: 'text', text: 'hello' }] }], + }; + } + if (captured.length > 2) { + return { + responseId: 'resp_provider_3', output: [{ type: 'message', content: [{ type: 'text', text: 'second' }] }], }; } @@ -41,7 +48,7 @@ suite('Agent Host Provider Integration - Copilot BYOK Responses', function () { responseId: 'resp_provider', output: [ { type: 'reasoning', id: 'rs_provider', summary: ['considered options'], encryptedContent: 'opaque' }, - { type: 'message', content: [{ type: 'text', text: 'hello' }] }, + { type: 'function_call', callId: 'call_provider', name: parameterlessToolName, argumentsJson: '{}' }, ], usage: { inputTokens: 1, outputTokens: 2, reasoningTokens: 1 }, }; @@ -69,7 +76,13 @@ suite('Agent Host Provider Integration - Copilot BYOK Responses', function () { sessionId, model: 'test-model', reasoningEffort: 'medium', - availableTools: [], + availableTools: [parameterlessToolName], + tools: [defineTool(parameterlessToolName, { + description: 'A parameterless integration test tool.', + defer: 'never', + handler: async () => 'ok', + skipPermission: true, + })], provider: { type: 'openai', wireApi: 'responses', @@ -83,6 +96,8 @@ suite('Agent Host Provider Integration - Copilot BYOK Responses', function () { const result = await session.sendAndWait({ prompt: 'Reply exactly hello.' }, 30_000); const secondResult = await session.sendAndWait({ prompt: 'Reply exactly second.' }, 30_000); const replayedReasoning = captured[1]?.input.find(item => item.type === 'reasoning'); + const toolOutput = captured[1]?.input.find(item => item.type === 'function_call_output'); + const parameterlessTool = captured[0]?.tools?.find(tool => tool.name === parameterlessToolName); assert.deepStrictEqual({ result: result?.type === 'assistant.message' ? result.data.content : undefined, @@ -94,6 +109,16 @@ suite('Agent Host Provider Integration - Copilot BYOK Responses', function () { inputTypes: captured[0]?.input.map(item => item.type), reasoningEffort: captured[0]?.reasoningEffort, }, + parameterlessTool: parameterlessTool?.type === 'function' ? { + type: parameterlessTool.type, + name: parameterlessTool.name, + parametersSchema: parameterlessTool.parametersSchema, + } : undefined, + toolContinuation: { + previousResponseId: captured[1]?.previousResponseId, + inputTypes: captured[1]?.input.map(item => item.type), + toolOutput, + }, replayedReasoning, }, { result: 'hello', @@ -105,6 +130,20 @@ suite('Agent Host Provider Integration - Copilot BYOK Responses', function () { inputTypes: ['message'], reasoningEffort: 'medium', }, + parameterlessTool: { + type: 'function', + name: parameterlessToolName, + parametersSchema: { type: 'object', properties: {} }, + }, + toolContinuation: { + previousResponseId: undefined, + inputTypes: ['message', 'reasoning', 'function_call', 'function_call_output'], + toolOutput: { + type: 'function_call_output', + callId: 'call_provider', + output: 'ok', + }, + }, replayedReasoning: { type: 'reasoning', id: 'rs_provider', From bd96693dc67a9122c69c9e8f80816813029a344f Mon Sep 17 00:00:00 2001 From: vritant24 Date: Mon, 17 Aug 2026 11:56:07 -0700 Subject: [PATCH 08/19] chat: localize BYOK failure descriptions and drop unrelated coverage Route both synthesized failure descriptions through @vscode/l10n with placeholders for the provider code and diagnostic details, matching the localized endpoint error pattern in imageLimits.ts. These strings reach the user as the BYOK failure reason, so a non-English install should not receive untranslated text. The bracketed identifiers stay verbatim so they remain greppable and pasteable into a provider support request. Revert the Agent Host integration test expansion. It exercised parameterless tool schemas and tool-call continuations, which ruled out two alternative causes while investigating, but no production tool handling changes here and that coverage does not validate this fix. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../platform/endpoint/node/responsesApi.ts | 7 ++- .../copilotByokResponses.integrationTest.ts | 49 ++----------------- 2 files changed, 10 insertions(+), 46 deletions(-) diff --git a/extensions/copilot/src/platform/endpoint/node/responsesApi.ts b/extensions/copilot/src/platform/endpoint/node/responsesApi.ts index ca47568cdd37cb..3988b2ef147011 100644 --- a/extensions/copilot/src/platform/endpoint/node/responsesApi.ts +++ b/extensions/copilot/src/platform/endpoint/node/responsesApi.ts @@ -3,6 +3,7 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ +import * as l10n from '@vscode/l10n'; import { Raw } from '@vscode/prompt-tsx'; import type { OpenAI } from 'openai'; import { Response } from '../../../platform/networking/common/fetcherService'; @@ -1130,14 +1131,16 @@ function toResponsesErrorContext(eventType: string, response?: Pick()); const registry = new ByokLmBridgeRegistry(); @@ -32,15 +31,9 @@ suite('Agent Host Provider Integration - Copilot BYOK Responses', function () { const registration = registry.register('client', { chat: async request => { captured.push(request); - if (captured.length === 2) { + if (captured.length > 1) { return { responseId: 'resp_provider_2', - output: [{ type: 'message', content: [{ type: 'text', text: 'hello' }] }], - }; - } - if (captured.length > 2) { - return { - responseId: 'resp_provider_3', output: [{ type: 'message', content: [{ type: 'text', text: 'second' }] }], }; } @@ -48,7 +41,7 @@ suite('Agent Host Provider Integration - Copilot BYOK Responses', function () { responseId: 'resp_provider', output: [ { type: 'reasoning', id: 'rs_provider', summary: ['considered options'], encryptedContent: 'opaque' }, - { type: 'function_call', callId: 'call_provider', name: parameterlessToolName, argumentsJson: '{}' }, + { type: 'message', content: [{ type: 'text', text: 'hello' }] }, ], usage: { inputTokens: 1, outputTokens: 2, reasoningTokens: 1 }, }; @@ -76,13 +69,7 @@ suite('Agent Host Provider Integration - Copilot BYOK Responses', function () { sessionId, model: 'test-model', reasoningEffort: 'medium', - availableTools: [parameterlessToolName], - tools: [defineTool(parameterlessToolName, { - description: 'A parameterless integration test tool.', - defer: 'never', - handler: async () => 'ok', - skipPermission: true, - })], + availableTools: [], provider: { type: 'openai', wireApi: 'responses', @@ -96,8 +83,6 @@ suite('Agent Host Provider Integration - Copilot BYOK Responses', function () { const result = await session.sendAndWait({ prompt: 'Reply exactly hello.' }, 30_000); const secondResult = await session.sendAndWait({ prompt: 'Reply exactly second.' }, 30_000); const replayedReasoning = captured[1]?.input.find(item => item.type === 'reasoning'); - const toolOutput = captured[1]?.input.find(item => item.type === 'function_call_output'); - const parameterlessTool = captured[0]?.tools?.find(tool => tool.name === parameterlessToolName); assert.deepStrictEqual({ result: result?.type === 'assistant.message' ? result.data.content : undefined, @@ -109,16 +94,6 @@ suite('Agent Host Provider Integration - Copilot BYOK Responses', function () { inputTypes: captured[0]?.input.map(item => item.type), reasoningEffort: captured[0]?.reasoningEffort, }, - parameterlessTool: parameterlessTool?.type === 'function' ? { - type: parameterlessTool.type, - name: parameterlessTool.name, - parametersSchema: parameterlessTool.parametersSchema, - } : undefined, - toolContinuation: { - previousResponseId: captured[1]?.previousResponseId, - inputTypes: captured[1]?.input.map(item => item.type), - toolOutput, - }, replayedReasoning, }, { result: 'hello', @@ -130,20 +105,6 @@ suite('Agent Host Provider Integration - Copilot BYOK Responses', function () { inputTypes: ['message'], reasoningEffort: 'medium', }, - parameterlessTool: { - type: 'function', - name: parameterlessToolName, - parametersSchema: { type: 'object', properties: {} }, - }, - toolContinuation: { - previousResponseId: undefined, - inputTypes: ['message', 'reasoning', 'function_call', 'function_call_output'], - toolOutput: { - type: 'function_call_output', - callId: 'call_provider', - output: 'ok', - }, - }, replayedReasoning: { type: 'reasoning', id: 'rs_provider', From 93344fb7b750f5c52bd95e570754cb90e47ed2a8 Mon Sep 17 00:00:00 2001 From: Ryan Ewen Date: Thu, 13 Aug 2026 14:35:47 -0400 Subject: [PATCH 09/19] Do not present a failed tool call as a successful one Two ways a failed browser tool still read as success. `ChatToolInvocation.didExecuteTool` only replaced the past-tense message when the result carried one of its own, so a tool that failed kept the message prepared before it ran. A screenshot that failed with "No browser page found" still rendered as "Captured browser screenshot". Drop the prepared message on failure and let the error stand on its own. `invokeFunctionResultToToolResult` recorded an evaluated-code error only in `toolResultDetails.isError`, so the tool call itself was still reported as successful. Set `toolResultError` as well. --- .../tools/browserToolHelpers.ts | 4 ++++ .../chatProgressTypes/chatToolInvocation.ts | 5 +++++ .../chat/test/common/model/chatModel.test.ts | 21 +++++++++++++++++++ 3 files changed, 30 insertions(+) diff --git a/src/vs/workbench/contrib/browserView/electron-browser/tools/browserToolHelpers.ts b/src/vs/workbench/contrib/browserView/electron-browser/tools/browserToolHelpers.ts index 8974c358f4fc01..18617fb8ad8b39 100644 --- a/src/vs/workbench/contrib/browserView/electron-browser/tools/browserToolHelpers.ts +++ b/src/vs/workbench/contrib/browserView/electron-browser/tools/browserToolHelpers.ts @@ -180,6 +180,10 @@ export function invokeFunctionResultToToolResult(result: IInvokeFunctionResult, content.push({ kind: 'text', value: result.summary }); return { content, + // The evaluated code reported an error, so the call failed. Without this + // only `toolResultDetails.isError` is set, which leaves the tool call + // itself reported as a success. + ...(result.error ? { toolResultError: result.error } : {}), ...(code ? { toolResultDetails: { input: code, diff --git a/src/vs/workbench/contrib/chat/common/model/chatProgressTypes/chatToolInvocation.ts b/src/vs/workbench/contrib/chat/common/model/chatProgressTypes/chatToolInvocation.ts index 13aa48ff30369d..31bb4ece43b3ab 100644 --- a/src/vs/workbench/contrib/chat/common/model/chatProgressTypes/chatToolInvocation.ts +++ b/src/vs/workbench/contrib/chat/common/model/chatProgressTypes/chatToolInvocation.ts @@ -366,6 +366,11 @@ export class ChatToolInvocation implements IChatToolInvocation { } if (result?.toolResultMessage) { this.pastTenseMessage = result.toolResultMessage; + } else if (result?.toolResultError) { + // The past-tense message was prepared before the tool ran and states + // what it would have done, so keeping it on a failure reads as though + // the work succeeded. Drop it and let the error stand on its own. + this.pastTenseMessage = undefined; } else if (this._progress.get().message) { this.pastTenseMessage = this._progress.get().message; } diff --git a/src/vs/workbench/contrib/chat/test/common/model/chatModel.test.ts b/src/vs/workbench/contrib/chat/test/common/model/chatModel.test.ts index e6e923ef695b6f..d9d8aefcf01dc2 100644 --- a/src/vs/workbench/contrib/chat/test/common/model/chatModel.test.ts +++ b/src/vs/workbench/contrib/chat/test/common/model/chatModel.test.ts @@ -1755,6 +1755,27 @@ suite('ChatResponseModel', () => { assert.strictEqual(completedNotifications, 1); }); + test('a failed tool drops the past-tense message prepared before it ran; a successful one keeps it', async () => { + const make = () => new ChatToolInvocation({ invocationMessage: 'Capturing browser screenshot', pastTenseMessage: 'Captured browser screenshot' }, { + id: 'screenshotPage', + modelDescription: 'Capture a browser screenshot', + displayName: 'Screenshot Page', + source: ToolDataSource.Internal, + }, 'tool-call-1', undefined, {}, {}); + + // The prepared message states what the tool would have done, so a failure + // must not keep it: it would read as though the screenshot was captured. + const failed = make(); + await failed.didExecuteTool({ content: [], toolResultError: 'No browser page found' }, true); + const succeeded = make(); + await succeeded.didExecuteTool({ content: [] }, true); + + assert.deepStrictEqual({ failed: failed.pastTenseMessage, succeeded: succeeded.pastTenseMessage }, { + failed: undefined, + succeeded: 'Captured browser screenshot', + }); + }); + test('hasActiveRequest reflects last request isIncomplete', async () => { const model = testDisposables.add(instantiationService.createInstance(ChatModel, undefined, { initialLocation: ChatAgentLocation.Chat, canUseTools: true })); From dd4ee3114c438fd5b198a98f5627361e31b64522 Mon Sep 17 00:00:00 2001 From: Ryan Ewen Date: Thu, 13 Aug 2026 17:01:55 -0400 Subject: [PATCH 10/19] Condense comments per review --- .../browserView/electron-browser/tools/browserToolHelpers.ts | 3 --- .../chat/common/model/chatProgressTypes/chatToolInvocation.ts | 4 +--- .../contrib/chat/test/common/model/chatModel.test.ts | 2 -- 3 files changed, 1 insertion(+), 8 deletions(-) diff --git a/src/vs/workbench/contrib/browserView/electron-browser/tools/browserToolHelpers.ts b/src/vs/workbench/contrib/browserView/electron-browser/tools/browserToolHelpers.ts index 18617fb8ad8b39..2e72b160e83f32 100644 --- a/src/vs/workbench/contrib/browserView/electron-browser/tools/browserToolHelpers.ts +++ b/src/vs/workbench/contrib/browserView/electron-browser/tools/browserToolHelpers.ts @@ -180,9 +180,6 @@ export function invokeFunctionResultToToolResult(result: IInvokeFunctionResult, content.push({ kind: 'text', value: result.summary }); return { content, - // The evaluated code reported an error, so the call failed. Without this - // only `toolResultDetails.isError` is set, which leaves the tool call - // itself reported as a success. ...(result.error ? { toolResultError: result.error } : {}), ...(code ? { toolResultDetails: { diff --git a/src/vs/workbench/contrib/chat/common/model/chatProgressTypes/chatToolInvocation.ts b/src/vs/workbench/contrib/chat/common/model/chatProgressTypes/chatToolInvocation.ts index 31bb4ece43b3ab..753b185208b202 100644 --- a/src/vs/workbench/contrib/chat/common/model/chatProgressTypes/chatToolInvocation.ts +++ b/src/vs/workbench/contrib/chat/common/model/chatProgressTypes/chatToolInvocation.ts @@ -367,9 +367,7 @@ export class ChatToolInvocation implements IChatToolInvocation { if (result?.toolResultMessage) { this.pastTenseMessage = result.toolResultMessage; } else if (result?.toolResultError) { - // The past-tense message was prepared before the tool ran and states - // what it would have done, so keeping it on a failure reads as though - // the work succeeded. Drop it and let the error stand on its own. + // The prepared message states what the tool would have done; on failure it would read as success. this.pastTenseMessage = undefined; } else if (this._progress.get().message) { this.pastTenseMessage = this._progress.get().message; diff --git a/src/vs/workbench/contrib/chat/test/common/model/chatModel.test.ts b/src/vs/workbench/contrib/chat/test/common/model/chatModel.test.ts index d9d8aefcf01dc2..aeb5ad72f15bd5 100644 --- a/src/vs/workbench/contrib/chat/test/common/model/chatModel.test.ts +++ b/src/vs/workbench/contrib/chat/test/common/model/chatModel.test.ts @@ -1763,8 +1763,6 @@ suite('ChatResponseModel', () => { source: ToolDataSource.Internal, }, 'tool-call-1', undefined, {}, {}); - // The prepared message states what the tool would have done, so a failure - // must not keep it: it would read as though the screenshot was captured. const failed = make(); await failed.didExecuteTool({ content: [], toolResultError: 'No browser page found' }, true); const succeeded = make(); From c517c08be6a59ce6037da6cb390e28044b7b9c81 Mon Sep 17 00:00:00 2001 From: Ryan Ewen Date: Mon, 17 Aug 2026 15:07:17 -0400 Subject: [PATCH 11/19] Name the failure instead of dropping the label MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review feedback: the codebase sets a past-tense message for a failed tool call rather than clearing it — the codex mapper does it per result, and the agent host does it for client tool calls that outlive their client. Doing it in ChatToolInvocation changed the past-tense behavior of every tool that reports an error in order to fix browser ones. The browser helpers now supply the failure label alongside the error, so the shared model is untouched. These tools declare only an invocationMessage and no pastTenseMessage, which is why a failed call reused the present-tense one and read as a success. The error check is on presence rather than truthiness, so an empty message from `throw ''` or `new Error()` still reports a failure. Coverage moves to browserToolHelpers, which nothing exercised before. --- .../tools/browserToolHelpers.ts | 16 ++++++- .../tools/browserToolHelpers.test.ts | 42 +++++++++++++++++++ .../chatProgressTypes/chatToolInvocation.ts | 3 -- .../chat/test/common/model/chatModel.test.ts | 19 --------- 4 files changed, 56 insertions(+), 24 deletions(-) create mode 100644 src/vs/workbench/contrib/browserView/test/electron-browser/tools/browserToolHelpers.test.ts diff --git a/src/vs/workbench/contrib/browserView/electron-browser/tools/browserToolHelpers.ts b/src/vs/workbench/contrib/browserView/electron-browser/tools/browserToolHelpers.ts index 2e72b160e83f32..a23348dbf18782 100644 --- a/src/vs/workbench/contrib/browserView/electron-browser/tools/browserToolHelpers.ts +++ b/src/vs/workbench/contrib/browserView/electron-browser/tools/browserToolHelpers.ts @@ -162,6 +162,17 @@ export async function playwrightInvoke( } } +/** + * Past-tense label for a browser tool call that failed. + * + * These tools declare only an `invocationMessage`, so on completion the + * present-tense label is reused verbatim and a failed call reads as a + * successful one ("Capturing browser screenshot"). Naming the failure keeps + * the completed state honest, as the agent host already does for client tool + * calls and the codex mapper does for its own results. + */ +const failedMessage = localize('browser.actionFailed', "Browser action failed"); + /** * Convert an {@link IInvokeFunctionResult} to an {@link IToolResult}, * including any {@link IInvokeFunctionResult.deferredResultId}. @@ -180,7 +191,7 @@ export function invokeFunctionResultToToolResult(result: IInvokeFunctionResult, content.push({ kind: 'text', value: result.summary }); return { content, - ...(result.error ? { toolResultError: result.error } : {}), + ...(result.error !== undefined ? { toolResultError: result.error || failedMessage, toolResultMessage: failedMessage } : {}), ...(code ? { toolResultDetails: { input: code, @@ -188,7 +199,7 @@ export function invokeFunctionResultToToolResult(result: IInvokeFunctionResult, output: result.result || result.error ? [{ type: 'embed' as const, isText: true, value: JSON.stringify(result.result ?? result.error, null, 2) }] : [], - isError: !!result.error, + isError: result.error !== undefined, }, } : {}), }; @@ -198,6 +209,7 @@ export function errorResult(message: string): IToolResult { return { content: [{ kind: 'text', value: message }], toolResultError: message, + toolResultMessage: failedMessage, }; } diff --git a/src/vs/workbench/contrib/browserView/test/electron-browser/tools/browserToolHelpers.test.ts b/src/vs/workbench/contrib/browserView/test/electron-browser/tools/browserToolHelpers.test.ts new file mode 100644 index 00000000000000..d19e5a0a9ee5a7 --- /dev/null +++ b/src/vs/workbench/contrib/browserView/test/electron-browser/tools/browserToolHelpers.test.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 assert from 'assert'; +import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../../base/test/common/utils.js'; +import { errorResult, invokeFunctionResultToToolResult } from '../../../electron-browser/tools/browserToolHelpers.js'; + +suite('browserToolHelpers - failure reporting', () => { + ensureNoDisposablesAreLeakedInTestSuite(); + + test('a failed invocation reports the failure and names it in the completed state', () => { + const result = invokeFunctionResultToToolResult({ error: 'No browser page found', summary: 'Screenshot failed' }); + + assert.strictEqual(result.toolResultError, 'No browser page found'); + assert.ok(result.toolResultMessage, 'a failed call must not reuse the present-tense invocation message'); + }); + + test('an empty error message still reports a failure', () => { + // `throw ''` and `new Error()` both produce one, and a falsy check would + // report the call as successful. + const result = invokeFunctionResultToToolResult({ error: '', summary: 'Screenshot failed' }); + + assert.ok(result.toolResultError, 'an empty error message is still a failure'); + assert.ok(result.toolResultMessage); + }); + + test('a successful invocation reports neither', () => { + const result = invokeFunctionResultToToolResult({ result: 'ok', summary: 'Captured screenshot' }); + + assert.strictEqual(result.toolResultError, undefined); + assert.strictEqual(result.toolResultMessage, undefined); + }); + + test('errorResult reports the failure and names it', () => { + const result = errorResult('No page ID provided.'); + + assert.strictEqual(result.toolResultError, 'No page ID provided.'); + assert.ok(result.toolResultMessage); + }); +}); diff --git a/src/vs/workbench/contrib/chat/common/model/chatProgressTypes/chatToolInvocation.ts b/src/vs/workbench/contrib/chat/common/model/chatProgressTypes/chatToolInvocation.ts index 753b185208b202..13aa48ff30369d 100644 --- a/src/vs/workbench/contrib/chat/common/model/chatProgressTypes/chatToolInvocation.ts +++ b/src/vs/workbench/contrib/chat/common/model/chatProgressTypes/chatToolInvocation.ts @@ -366,9 +366,6 @@ export class ChatToolInvocation implements IChatToolInvocation { } if (result?.toolResultMessage) { this.pastTenseMessage = result.toolResultMessage; - } else if (result?.toolResultError) { - // The prepared message states what the tool would have done; on failure it would read as success. - this.pastTenseMessage = undefined; } else if (this._progress.get().message) { this.pastTenseMessage = this._progress.get().message; } diff --git a/src/vs/workbench/contrib/chat/test/common/model/chatModel.test.ts b/src/vs/workbench/contrib/chat/test/common/model/chatModel.test.ts index aeb5ad72f15bd5..e6e923ef695b6f 100644 --- a/src/vs/workbench/contrib/chat/test/common/model/chatModel.test.ts +++ b/src/vs/workbench/contrib/chat/test/common/model/chatModel.test.ts @@ -1755,25 +1755,6 @@ suite('ChatResponseModel', () => { assert.strictEqual(completedNotifications, 1); }); - test('a failed tool drops the past-tense message prepared before it ran; a successful one keeps it', async () => { - const make = () => new ChatToolInvocation({ invocationMessage: 'Capturing browser screenshot', pastTenseMessage: 'Captured browser screenshot' }, { - id: 'screenshotPage', - modelDescription: 'Capture a browser screenshot', - displayName: 'Screenshot Page', - source: ToolDataSource.Internal, - }, 'tool-call-1', undefined, {}, {}); - - const failed = make(); - await failed.didExecuteTool({ content: [], toolResultError: 'No browser page found' }, true); - const succeeded = make(); - await succeeded.didExecuteTool({ content: [] }, true); - - assert.deepStrictEqual({ failed: failed.pastTenseMessage, succeeded: succeeded.pastTenseMessage }, { - failed: undefined, - succeeded: 'Captured browser screenshot', - }); - }); - test('hasActiveRequest reflects last request isIncomplete', async () => { const model = testDisposables.add(instantiationService.createInstance(ChatModel, undefined, { initialLocation: ChatAgentLocation.Chat, canUseTools: true })); From be2aeccba1a5853e07d22d23f0419a6e2a4b4cee Mon Sep 17 00:00:00 2001 From: Benjamin Christopher Simmonds <44439583+benibenj@users.noreply.github.com> Date: Mon, 17 Aug 2026 22:26:43 +0200 Subject: [PATCH 12/19] agentHost: make startup session discovery registry-first (#331176) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * agentHost: make startup session discovery registry-first (#331155) A 2026-08-17 Insiders reproduction with 941 SDK sessions and 1,437 per-session databases (~946 MB) showed the ~60s delay is not migration — `sessionRegistryBackfilled:copilotcli` was already true and migration was skipped. The time went to per-session SQLite work during discovery and listing: classification took 66.9s (113 external, 342 adoptable extension-host, 486 already known), registration of the 113 emitted candidates another 21.9s, and the first two AHP `listSessions` calls 109.8s and 91.0s, with a third 73.2s window after discovery. Each measured phase is addressed: - Classification (66.9s): discovery is now registry-first. `IAgent` gains an optional `setKnownSessionsFilter` seam that `AgentService` installs at provider registration; it answers, in one registry query for the whole candidate set, which sessions the host already owns. `CopilotAgent` drops those candidates instead of opening a session database each, so the 486 already-known sessions cost no DB opens. Tombstoned sessions are absent from the registry and therefore never reported as known, so an explicitly deleted session still reaches `register`, whose atomic tombstone check declines it. Provenance of a registered row stays owned by the explicit create/restore paths. - Classification (the 342 adoptable rows): while migrate-legacy is off, adoptable extension-host candidates are never emitted, so their Git-touching project resolution is now skipped entirely instead of being computed and then filtered away. `_emitCopilotChats` keeps its filter as a re-check, since the setting can flip mid-pass. - Registration (21.9s): `_registerDiscoveredChats` rejects an already-registered candidate with unchanged provenance before `_isChatBacking()` or any other per-session I/O. - Listing (109.8s / 91.0s / 73.2s): `listSessions()` coalesces concurrent computations per external-sessions mode, so the burst a multi-window restore produces shares one registry traversal. The shared entry records the registry epoch it started at and every registry mutation invalidates it, so a caller arriving after a mutation starts a fresh pass rather than joining a possibly pre-mutation one. Each caller gets its own array; rejections are shared only with callers already waiting. - `_readStoredSessionMetadata` / `_readSessionMetadata` now issue one bulk `getMetadataObject()` query instead of nine and six single-key reads, shrinking the cost of the fallback path that runs when no host filter is installed. Deferred deliberately: seeding a newly discovered external session as read still creates its database purely to hold one flag. Dropping the write without a durable default would flip every discovered external session to unread, because the list overlay only applies `IsRead` when the key is present. The correct default belongs on the registry row and is left to the registry list-projection change; the reasoning is recorded on `_initializeExternalSessionReadState`. Removal of `_awaitInitialProviderMigration()` is likewise not attempted: the reproduction proves migration was already skipped. Tests assert call/open counts rather than wall-clock thresholds: registry-known candidates cause zero session DB opens; disabled adoptable candidates resolve no projects; re-registering a known discovered chat performs no per-session I/O; the known-sessions filter reports registered sessions only and leaves tombstones to registration; concurrent list calls share one computation but not their arrays; a mutation during an in-flight list is not served from it; and stored metadata is read with a single bulk query. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * agentHost: address discovery review feedback Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * agentHost: update discovery perf fixture Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/vs/platform/agentHost/AGENTS.md | 4 + src/vs/platform/agentHost/common/agent.ts | 6 + .../platform/agentHost/node/agentService.ts | 61 +++++++++- .../agentHost/node/copilot/copilotAgent.ts | 87 ++++++++----- .../agentHost/test/node/agentService.test.ts | 114 ++++++++++++++++++ .../agentHost/test/node/copilotAgent.test.ts | 108 +++++++++++++++++ 6 files changed, 345 insertions(+), 35 deletions(-) diff --git a/src/vs/platform/agentHost/AGENTS.md b/src/vs/platform/agentHost/AGENTS.md index 0c8b0470247a41..4447cd745403d8 100644 --- a/src/vs/platform/agentHost/AGENTS.md +++ b/src/vs/platform/agentHost/AGENTS.md @@ -213,10 +213,14 @@ a chat URI. New provider code must consume the seams. Providers own discovery lifecycle and push unknown chats with provider-classified provenance through `onDidDiscoverChats`. Claude, Codex, and Copilot classify their unknown native chats as external, except that Copilot keeps an unknown *legacy extension-host* chat internal because it is adoptable in place rather than someone else's session. Agent Service preserves that classification when it additively registers the event payload. Every provider starts one memoized initial attempt when the first discovery-event listener is attached; that attempt retries internally, but once it settles it is not re-armed by SDK readiness, so the only later trigger is an explicit one (for Copilot, the migrate-legacy toggle). Ordinary list refreshes never enumerate provider catalogs. External discovery has no migration marker or Copilot migrate-legacy gate; only the adoptable legacy extension-host half of Copilot's payload is withheld while migrate-legacy is off. Discovery never prunes a registry row when a provider later omits it and filters subagents and marked internal chat backings. +Discovery is registry-first: Agent Service hands each provider an optional `setKnownSessionsFilter` seam that answers, for a whole candidate set in one registry query, which sessions the host already owns. A provider drops those candidates before any per-session database open, and Copilot additionally skips adoptable legacy classification work (project/Git resolution) while migrate-legacy is off, since those candidates would not be emitted. Agent Service in turn rejects an already-registered candidate before `_isChatBacking()` or any other per-session I/O; provenance of a registered row stays owned by the explicit create/restore paths. Tombstoned sessions are absent from the registry and therefore never reported as known, so an explicitly deleted session still reaches `register`, whose atomic tombstone check declines it. + Claude and Codex each use one memoized initial path: resolve/download the SDK, enumerate once, classify the native catalog by stored session metadata, then emit only unknown chats as `external: true`. Provider session databases no longer persist a provider-local external property; legacy `claude.external` and `codex.external` values are recognized only as evidence that a chat was known. An empty or absent sidecar remains unknown. If a provider cannot enumerate yet, its initial discovery attempt emits nothing; once ready, it emits the resulting chats through `onDidDiscoverChats`. Registry provenance is projected into `IAgentSessionMetadata._meta` with `readSessionExternal` / `withSessionExternal`, and the normal AHP listSessions round trip carries it to the Sessions provider. There is no external-specific UI behavior. +`listSessions()` coalesces concurrent computations per external-sessions mode, so the burst of calls a multi-window restore produces shares one registry traversal instead of one per window. The shared entry records the registry epoch it started at and is invalidated by every registry mutation, so a caller arriving after a mutation starts a fresh pass; each caller receives its own array. + Legacy registry migration remains a separate `listChatsToMigrate()` contract. It returns only chats known from non-empty provider session metadata, without external provenance, and is gated by durable per-provider/global migration markers. Agent Host writes `agentHost.workspaceless` as either `true` or `false` into every session it creates. Agent Service classifies each migration candidate itself: marker presence means internal, while absence means a known external chat. Provider-private discovery helpers name their concrete source: Claude uses `_listClaudeCodeChats()` / `_emitClaudeCodeChats()`, Codex uses `_listCodexChats()` / `_emitCodexChats()`, and Copilot uses `_discoverCopilotChats()` / `_emitCopilotChats()`. Providers filter known session metadata before emitting; Agent Service still performs the authoritative additive registry write and atomic tombstone check. Copilot treats the existence of a per-session database (under `{userDataPath}/agentSessionData`, never the shared Copilot home) as "known", which also keeps peer-chat backings out of the payload; it additionally drops a chat whose SDK context carries no working directory, because `_doResumeSession` requires one and a discovered chat has no other source for it. diff --git a/src/vs/platform/agentHost/common/agent.ts b/src/vs/platform/agentHost/common/agent.ts index 7ad4c5a3b77b36..d955f78957ad24 100644 --- a/src/vs/platform/agentHost/common/agent.ts +++ b/src/vs/platform/agentHost/common/agent.ts @@ -129,6 +129,9 @@ export interface IAgentDiscoveredChat extends IAgentChatMetadata { readonly external: boolean; } +/** Returns the candidate session URI keys already present in the host registry. */ +export type IAgentKnownSessionsFilter = (sessions: readonly URI[]) => Promise>; + export interface IAgentSessionMetadata extends Omit { readonly session: URI; } @@ -1091,6 +1094,9 @@ export interface IAgent { /** Provides chats that are ready to be registered as Agent Host sessions. */ readonly onDidDiscoverChats: Event; + /** Lets discovery drop registered candidates before per-session I/O. */ + setKnownSessionsFilter?(filter: IAgentKnownSessionsFilter): void; + // ---- Legacy migration --------------------------------------------------- /** Optional adoption hook for providers with a predecessor-owned on-disk format. */ diff --git a/src/vs/platform/agentHost/node/agentService.ts b/src/vs/platform/agentHost/node/agentService.ts index bbb5839ece1acf..ed73139a9efd39 100644 --- a/src/vs/platform/agentHost/node/agentService.ts +++ b/src/vs/platform/agentHost/node/agentService.ts @@ -972,6 +972,7 @@ export class AgentService extends Disposable implements IAgentService { this._logService.info(`Registering agent provider: ${provider.id}`); this._providers.set(provider.id, provider); provider.setServerToolHost?.(this._serverToolHost); + provider.setKnownSessionsFilter?.(sessions => this._filterKnownSessions(sessions)); void this._authService.replay(provider); // Deterministic subagent membership ordering: apply a spawned subagent's // catalog membership (via the spawn-channel handlers) BEFORE @@ -1357,10 +1358,17 @@ export class AgentService extends Disposable implements IAgentService { const discoveryLimiter = new Limiter(4); let suppressed = 0; let registeredExternal = false; + let alreadyRegistered = 0; const results = await Promise.all(chats.map(({ external, ...metadata }) => discoveryLimiter.queue(async () => { const sessionMetadata = this._toSessionMetadata(metadata); const session = sessionMetadata.session; try { + // Matching registry entries need no per-session I/O. + const known = existing.get(session.toString()); + if (known !== undefined && known === external) { + alreadyRegistered++; + return false; + } if (isSubagentSession(session.toString()) || await this._isChatBacking(session)) { suppressed++; return false; @@ -1371,6 +1379,7 @@ export class AgentService extends Disposable implements IAgentService { `discovery registration for ${session.toString()}`, ); if (registered) { + this._invalidateSessionList(); if (external && existing.get(session.toString()) !== true) { await this._initializeExternalSessionReadState(session); } @@ -1393,7 +1402,7 @@ export class AgentService extends Disposable implements IAgentService { if (registeredExternal) { this._queueSessionListReconciliation(); } - this._logService.info(`[AgentService] discovery for provider ${provider.id}: ${chats.length} candidate(s) (${chats.filter(chat => chat.external).length} external), ${registered} registered, ${suppressed} suppressed as subagent/chat backing`); + this._logService.info(`[AgentService] discovery for provider ${provider.id}: ${chats.length} candidate(s) (${chats.filter(chat => chat.external).length} external), ${registered} registered, ${alreadyRegistered} already registered, ${suppressed} suppressed as subagent/chat backing`); return registered > 0; } @@ -1428,6 +1437,7 @@ export class AgentService extends Disposable implements IAgentService { } const registered = await this._sessionRegistry.register(identity.session, identity, { checkTombstone: true }); if (registered) { + this._invalidateSessionList(); const metadata = sessions[index]; if (identity.external && existing.get(identity.session.toString()) !== true) { await this._initializeExternalSessionReadState(identity.session); @@ -1446,6 +1456,7 @@ export class AgentService extends Disposable implements IAgentService { } } + /** Seeds external sessions as read. Avoiding this DB requires a durable registry default. */ private async _initializeExternalSessionReadState(session: URI): Promise { const ref = this._sessionDataService.openDatabase(session); try { @@ -1493,10 +1504,22 @@ export class AgentService extends Disposable implements IAgentService { } } + /** Returns registered candidates. Tombstones remain candidates so registration can reject them atomically. */ + private async _filterKnownSessions(sessions: readonly URI[]): Promise> { + const registered = new Set((await this._listRegisteredSessions()).map(entry => entry.session.toString())); + const known = new Set(); + for (const session of sessions) { + const key = session.toString(); + if (registered.has(key)) { + known.add(key); + } + } + return known; + } + /** * Whether a session is marked as an internal chat backing, either durably - * (its own metadata) or in-process (its durable marker write kept failing - * in `createChat`; see `_unpersistedChatBackings`). + * or in `_unpersistedChatBackings`. */ private async _isChatBacking(session: URI): Promise { if (this._unpersistedChatBackings.has(session.toString())) { @@ -1516,7 +1539,36 @@ export class AgentService extends Disposable implements IAgentService { return false; } } + /** In-flight list computations, shared per mode until they settle or the registry changes. */ + private readonly _inFlightListSessions = new Map }>(); + + private _registryEpoch = 0; + + private _invalidateSessionList(): void { + this._registryEpoch++; + this._inFlightListSessions.clear(); + } + async listSessions(mode = this._getExternalSessionsMode()): Promise { + const epoch = this._registryEpoch; + const inFlight = this._inFlightListSessions.get(mode); + if (inFlight && inFlight.epoch === epoch) { + // Callers own their array; the shared result must not be mutable by one of them. + return [...await inFlight.promise]; + } + const promise = this._computeSessions(mode); + const entry = { epoch, promise }; + this._inFlightListSessions.set(mode, entry); + const clear = () => { + if (this._inFlightListSessions.get(mode) === entry) { + this._inFlightListSessions.delete(mode); + } + }; + void promise.then(clear, clear); + return [...await promise]; + } + + private async _computeSessions(mode: AgentHostExternalSessionsMode): Promise { this._logService.trace('[AgentService] listSessions called'); // The first list waits for registration-time legacy migration if it is still in flight. await this._awaitInitialProviderMigration(); @@ -2074,6 +2126,7 @@ export class AgentService extends Disposable implements IAgentService { () => this._sessionRegistry.register(session, { provider: provider.id, startTime: Date.now(), source: 'explicit' }, { checkTombstone: false }), `registration for ${session.toString()}`, ); + this._invalidateSessionList(); } catch (err) { await this._rollbackProviderSession(provider, session); throw err; @@ -3240,6 +3293,7 @@ export class AgentService extends Disposable implements IAgentService { () => this._sessionRegistry.unregister(session), `unregistration for ${session.toString()}`, ); + this._invalidateSessionList(); if (provider) { this._sessionToProvider.delete(session.toString()); this._clearDownloadProgressInterest(session.toString()); @@ -4492,6 +4546,7 @@ export class AgentService extends Disposable implements IAgentService { // up-front tombstone would, before any state-manager mutation. throw new ProtocolError(AHP_SESSION_NOT_FOUND, `Session was explicitly deleted: ${sessionStr}`); } + this._invalidateSessionList(); this._stateManager.restoreSession(summary, mergedTurns, { draft: restoredDraft, defaultChatTitle }); this._serverToolHost.advertise(sessionStr); diff --git a/src/vs/platform/agentHost/node/copilot/copilotAgent.ts b/src/vs/platform/agentHost/node/copilot/copilotAgent.ts index 6dd14f82446da1..e37cb43c334dec 100644 --- a/src/vs/platform/agentHost/node/copilot/copilotAgent.ts +++ b/src/vs/platform/agentHost/node/copilot/copilotAgent.ts @@ -44,7 +44,7 @@ import { AgentHostAutoApprovePolicyRestrictedConfigKey, AgentHostMcpServersConfi import { IAgentPluginManager, ISyncedCustomization } from '../../common/agentPluginManager.js'; import { decodeProviderData, encodeProviderData, type IPersistedChat } from '../agentChatBackings.js'; import { prepareSideChatPrompt, sliceSideChatTurns } from '../agentPeerChats.js'; -import { AgentChatOperationContext, AgentSession, AgentSignal, AuthenticateParams, IActiveClient, IAgent, IAgentChatAdoptionResult, IAgentChatConfigCompletionsParams, IAgentChatContext, IAgentChatDataChange, IAgentChatMetadata, IAgentChats, IAgentLegacyChat, IAgentCreateChatOptions, IAgentCreateChatResult, IAgentDescriptor, IAgentDiscoveredChat, IAgentHostManagedSettingsSnapshot, IAgentHostNetworkEndpoint, IAgentMaterializeChatEvent, IAgentModelInfo, IAgentResolveChatConfigParams, IAgentSessionProjectInfo, IAgentSpawnChatEvent, IMcpNotification, SubagentChatSignal, resolveAgentChatContext, resolveAgentHostCustomizations, resolveAgentHostInstructions, resolveSubagentChatParent } from '../../common/agent.js'; +import { AgentChatOperationContext, AgentSession, AgentSignal, AuthenticateParams, IActiveClient, IAgent, IAgentChatAdoptionResult, IAgentChatConfigCompletionsParams, IAgentChatContext, IAgentChatDataChange, IAgentChatMetadata, IAgentChats, IAgentLegacyChat, IAgentCreateChatOptions, IAgentCreateChatResult, IAgentDescriptor, IAgentDiscoveredChat, IAgentHostManagedSettingsSnapshot, IAgentHostNetworkEndpoint, IAgentKnownSessionsFilter, IAgentMaterializeChatEvent, IAgentModelInfo, IAgentResolveChatConfigParams, IAgentSessionProjectInfo, IAgentSpawnChatEvent, IMcpNotification, SubagentChatSignal, resolveAgentChatContext, resolveAgentHostCustomizations, resolveAgentHostInstructions, resolveSubagentChatParent } from '../../common/agent.js'; import { getReasoningEffortDescription, getReasoningEffortLabel, resolveDefaultReasoningEffort } from '../../common/reasoningEffort.js'; import type { IAgentServerToolHost } from '../../common/agentServerTools.js'; import { IAgentHostOTelService } from '../../common/otel/agentHostOTelService.js'; @@ -2137,6 +2137,12 @@ export class CopilotAgent extends Disposable implements IAgent { private _copilotChatDiscovery: Promise | undefined; + private _knownSessionsFilter: IAgentKnownSessionsFilter | undefined; + + setKnownSessionsFilter(filter: IAgentKnownSessionsFilter): void { + this._knownSessionsFilter = filter; + } + /** * One memoized initial discovery attempt, mirroring Claude and Codex. The * CLI client may still be starting when the first discovery listener @@ -2201,8 +2207,8 @@ export class CopilotAgent extends Disposable implements IAgent { * creator provenance. External chats must also have repository metadata * and have been modified within the last seven days. * - * A chat counts as already known when it has a per-session database, which - * also keeps peer-chat backings out of the result. A chat the SDK reports + * Registered chats are filtered by the host, with stored metadata as a + * fallback when no host filter is installed. A chat the SDK reports * without a working directory is skipped: {@link _doResumeSession} requires * one and a discovered chat has no other source for it (Agent Host writes * no metadata for it beyond the read marker), so it would surface as a row @@ -2220,6 +2226,12 @@ export class CopilotAgent extends Disposable implements IAgent { if (!sessions) { return undefined; } + // Filter registered candidates with one registry query. + const knownSessions = this._knownSessionsFilter + ? await this._knownSessionsFilter(sessions.map(s => AgentSession.uri(this.id, s.sessionId))) + : undefined; + // Skip project resolution for adoptable chats that will not be emitted. + const emitAdoptable = this._isMigrateLegacyCopilotCliEnabled(); const projectLimiter = new Limiter(4); const metadataLimiter = new Limiter(4); const projectByContext = new Map>(); @@ -2233,7 +2245,7 @@ export class CopilotAgent extends Disposable implements IAgent { const mapped = await Promise.all(sessions.map(s => metadataLimiter.queue(async () => { const session = AgentSession.uri(this.id, s.sessionId); try { - if (await this._readStoredSessionMetadata(session)) { + if (knownSessions ? knownSessions.has(session.toString()) : !!(await this._readStoredSessionMetadata(session))) { known++; return undefined; } @@ -2242,6 +2254,9 @@ export class CopilotAgent extends Disposable implements IAgent { return undefined; } const adoptable = await this._isExtensionHostCliSession(s.sessionId); + if (adoptable && !emitAdoptable) { + return undefined; + } const modifiedTime = new Date(s.modifiedTime).getTime(); if (!adoptable) { const clientName = s.isRemote ? undefined : s.clientName; @@ -2276,7 +2291,7 @@ export class CopilotAgent extends Disposable implements IAgent { }))); const chats = mapped.filter((chat): chat is IAgentDiscoveredChat => chat !== undefined); const external = chats.filter(chat => chat.external).length; - this._logService.info(`[Copilot] Chat discovery: ${sessions.length} SDK session(s) -> ${external} external, ${chats.length - external} adoptable legacy extension-host, ${known} already known to Agent Host, ${withoutWorkingDirectory} without a working directory, ${unsupportedClientName} with unsupported or missing client name, ${outsideImportWindow} outside the import window, ${withoutRepository} without repository metadata, ${failed} failed to classify`); + this._logService.info(`[Copilot] Chat discovery: ${sessions.length} SDK session(s) -> ${external} external, ${chats.length - external} adoptable legacy extension-host, ${known} already known to Agent Host, ${withoutWorkingDirectory} without a working directory, ${unsupportedClientName} with unsupported or missing client name, ${outsideImportWindow} outside the import window, ${withoutRepository} without repository metadata, ${failed} failed to classify (adopt legacy extension-host chats: ${emitAdoptable})`); return chats; } @@ -4581,22 +4596,24 @@ export class CopilotAgent extends Disposable implements IAgent { return {}; } try { - const [model, agent, cwd, cwds, customizationDirectory, workspaceless] = await Promise.all([ - ref.object.getMetadata(CopilotAgent._META_MODEL), - ref.object.getMetadata(CopilotAgent._META_AGENT), - ref.object.getMetadata(CopilotAgent._META_CWD), - ref.object.getMetadata(CopilotAgent._META_CWDS), - ref.object.getMetadata(CopilotAgent._META_CUSTOMIZATION_DIRECTORY), - ref.object.getMetadata(AH_META_WORKSPACELESS_DB_KEY), - ]); + const m = await ref.object.getMetadataObject({ + [CopilotAgent._META_MODEL]: true, + [CopilotAgent._META_AGENT]: true, + [CopilotAgent._META_CWD]: true, + [CopilotAgent._META_CWDS]: true, + [CopilotAgent._META_CUSTOMIZATION_DIRECTORY]: true, + [AH_META_WORKSPACELESS_DB_KEY]: true, + }); + const cwd = m[CopilotAgent._META_CWD]; + const customizationDirectory = m[CopilotAgent._META_CUSTOMIZATION_DIRECTORY]; const workingDirectory = cwd ? URI.parse(cwd) : undefined; return { - model: this._parseModelSelection(model), - agent: this._parseAgentSelection(agent), + model: this._parseModelSelection(m[CopilotAgent._META_MODEL]), + agent: this._parseAgentSelection(m[CopilotAgent._META_AGENT]), workingDirectory, - workingDirectories: this._parseWorkingDirectories(cwds, workingDirectory), + workingDirectories: this._parseWorkingDirectories(m[CopilotAgent._META_CWDS], workingDirectory), customizationDirectory: customizationDirectory ? URI.parse(customizationDirectory) : undefined, - workspaceless: workspaceless === 'true', + workspaceless: m[AH_META_WORKSPACELESS_DB_KEY] === 'true', }; } finally { ref.dispose(); @@ -4609,27 +4626,33 @@ export class CopilotAgent extends Disposable implements IAgent { return undefined; } try { - const [model, agent, cwd, cwds, customizationDirectory, resolved, uri, displayName, workspaceless] = await Promise.all([ - ref.object.getMetadata(CopilotAgent._META_MODEL), - ref.object.getMetadata(CopilotAgent._META_AGENT), - ref.object.getMetadata(CopilotAgent._META_CWD), - ref.object.getMetadata(CopilotAgent._META_CWDS), - ref.object.getMetadata(CopilotAgent._META_CUSTOMIZATION_DIRECTORY), - ref.object.getMetadata(CopilotAgent._META_PROJECT_RESOLVED), - ref.object.getMetadata(CopilotAgent._META_PROJECT_URI), - ref.object.getMetadata(CopilotAgent._META_PROJECT_DISPLAY_NAME), - ref.object.getMetadata(AH_META_WORKSPACELESS_DB_KEY), - ]); - if ([model, agent, cwd, cwds, customizationDirectory, resolved, uri, displayName, workspaceless].every(value => value === undefined)) { + const m = await ref.object.getMetadataObject({ + [CopilotAgent._META_MODEL]: true, + [CopilotAgent._META_AGENT]: true, + [CopilotAgent._META_CWD]: true, + [CopilotAgent._META_CWDS]: true, + [CopilotAgent._META_CUSTOMIZATION_DIRECTORY]: true, + [CopilotAgent._META_PROJECT_RESOLVED]: true, + [CopilotAgent._META_PROJECT_URI]: true, + [CopilotAgent._META_PROJECT_DISPLAY_NAME]: true, + [AH_META_WORKSPACELESS_DB_KEY]: true, + }); + const cwd = m[CopilotAgent._META_CWD]; + const customizationDirectory = m[CopilotAgent._META_CUSTOMIZATION_DIRECTORY]; + const resolved = m[CopilotAgent._META_PROJECT_RESOLVED]; + const uri = m[CopilotAgent._META_PROJECT_URI]; + const displayName = m[CopilotAgent._META_PROJECT_DISPLAY_NAME]; + const workspaceless = m[AH_META_WORKSPACELESS_DB_KEY]; + if ([m[CopilotAgent._META_MODEL], m[CopilotAgent._META_AGENT], cwd, m[CopilotAgent._META_CWDS], customizationDirectory, resolved, uri, displayName, workspaceless].every(value => value === undefined)) { return { resolved: false }; } const workingDirectory = cwd ? URI.parse(cwd) : undefined; const project = uri && displayName ? { uri: URI.parse(uri), displayName } : undefined; return { - model: this._parseModelSelection(model), - agent: this._parseAgentSelection(agent), + model: this._parseModelSelection(m[CopilotAgent._META_MODEL]), + agent: this._parseAgentSelection(m[CopilotAgent._META_AGENT]), workingDirectory, - workingDirectories: this._parseWorkingDirectories(cwds, workingDirectory), + workingDirectories: this._parseWorkingDirectories(m[CopilotAgent._META_CWDS], workingDirectory), customizationDirectory: customizationDirectory ? URI.parse(customizationDirectory) : undefined, project, resolved: resolved === 'true' || project !== undefined, diff --git a/src/vs/platform/agentHost/test/node/agentService.test.ts b/src/vs/platform/agentHost/test/node/agentService.test.ts index 346ba52f636b41..e8855d05dad1f4 100644 --- a/src/vs/platform/agentHost/test/node/agentService.test.ts +++ b/src/vs/platform/agentHost/test/node/agentService.test.ts @@ -3069,6 +3069,120 @@ suite('AgentService (node dispatcher)', () => { }); }); + test('re-registering a known discovered chat performs no per-session database I/O', async () => { + const perSession = createPerSessionDataService(); + const svc = disposables.add(new AgentService(new NullLogService(), fileService, perSession.service, { _serviceBrand: undefined } as IProductService, createNoopGitService())); + const agent = disposables.add(new MockAgent('copilot')); + svc.registerProvider(agent); + const session = AgentSession.uri('copilot', 'known-discovered'); + const register = (svc as unknown as { _registerDiscoveredChats(provider: IAgent, chats: readonly IAgentDiscoveredChat[]): Promise })._registerDiscoveredChats.bind(svc); + await register(agent, [discoveredChat(session)]); + + const opened: string[] = []; + const service = perSession.service as { tryOpenDatabase(session: URI): Promise }; + const originalTryOpen = service.tryOpenDatabase; + service.tryOpenDatabase = async (s: URI) => { + opened.push(s.toString()); + return originalTryOpen.call(perSession.service, s); + }; + try { + const changed = await register(agent, [discoveredChat(session)]); + + assert.deepStrictEqual({ changed, opened }, { changed: false, opened: [] }); + } finally { + service.tryOpenDatabase = originalTryOpen; + } + }); + + test('the known-sessions filter reports registered sessions only, leaving tombstones to registration', async () => { + const svc = disposables.add(new AgentService(new NullLogService(), fileService, createSessionDataService(), { _serviceBrand: undefined } as IProductService, createNoopGitService())); + const agent = disposables.add(new MockAgent('copilot')); + svc.registerProvider(agent); + const registered = AgentSession.uri('copilot', 'filter-registered'); + const deleted = AgentSession.uri('copilot', 'filter-deleted'); + const unknown = AgentSession.uri('copilot', 'filter-unknown'); + const register = (svc as unknown as { _registerDiscoveredChats(provider: IAgent, chats: readonly IAgentDiscoveredChat[]): Promise })._registerDiscoveredChats.bind(svc); + await register(agent, [discoveredChat(registered), discoveredChat(deleted)]); + await (svc as unknown as { _sessionRegistry: AgentSessionRegistry })._sessionRegistry.unregister(deleted); + + const known = await (svc as unknown as { _filterKnownSessions(sessions: readonly URI[]): Promise> })._filterKnownSessions([registered, deleted, unknown]); + const reRegistered = await register(agent, [discoveredChat(deleted)]); + + assert.deepStrictEqual({ + known: [...known], + reRegistered, + sessions: (await (svc as unknown as { _sessionRegistry: AgentSessionRegistry })._sessionRegistry.list()).map(entry => entry.session.toString()), + }, { + known: [registered.toString()], + reRegistered: false, + sessions: [registered.toString()], + }); + }); + + test('concurrent listSessions calls share one computation and never share their result array', async () => { + const svc = disposables.add(new AgentService(new NullLogService(), fileService, createSessionDataService(), { _serviceBrand: undefined } as IProductService, createNoopGitService())); + const agent = disposables.add(new MockAgent('copilot')); + svc.registerProvider(agent); + await svc.createSession({ provider: 'copilot' }); + let computations = 0; + const inner = svc as unknown as { _computeSessions(mode: AgentHostExternalSessionsMode): Promise }; + const original = inner._computeSessions; + inner._computeSessions = async mode => { + computations++; + return original.call(svc, mode); + }; + + const [first, second] = await Promise.all([svc.listSessions(), svc.listSessions()]); + const sharedComputations = computations; + first.length = 0; + const third = await svc.listSessions(); + + assert.deepStrictEqual({ + sharedComputations, + + computations, + secondIntact: second.length, + thirdIntact: third.length, + distinctArrays: first !== second, + }, { + sharedComputations: 1, + computations: 2, + secondIntact: 1, + thirdIntact: 1, + distinctArrays: true, + }); + }); + + test('a registry mutation during an in-flight list is not served from the shared computation', async () => { + const svc = disposables.add(new AgentService(new NullLogService(), fileService, createSessionDataService(), { _serviceBrand: undefined } as IProductService, createNoopGitService())); + const agent = disposables.add(new MockAgent('copilot')); + svc.registerProvider(agent); + const gate = new DeferredPromise(); + const inner = svc as unknown as { _computeSessions(mode: AgentHostExternalSessionsMode): Promise }; + const original = inner._computeSessions; + let computations = 0; + inner._computeSessions = async mode => { + computations++; + await gate.p; + return original.call(svc, mode); + }; + + const stale = svc.listSessions(); + await svc.createSession({ provider: 'copilot' }); + const fresh = svc.listSessions(); + gate.complete(); + + assert.deepStrictEqual({ + computations, + stale: (await stale).length, + fresh: (await fresh).length, + }, { + computations: 2, + stale: 1, + fresh: 1, + }); + }); + test('explicitly created sessions are registered as non-external', async () => { service.registerProvider(copilotAgent); const session = await service.createSession({ provider: 'copilot' }); diff --git a/src/vs/platform/agentHost/test/node/copilotAgent.test.ts b/src/vs/platform/agentHost/test/node/copilotAgent.test.ts index 88aa23c0c5684b..63e5eb2242ffb1 100644 --- a/src/vs/platform/agentHost/test/node/copilotAgent.test.ts +++ b/src/vs/platform/agentHost/test/node/copilotAgent.test.ts @@ -5297,6 +5297,114 @@ suite('CopilotAgent', () => { await disposeAgent(agent); } }); + + test('registry-known candidates are dropped without opening any session database', async () => { + const userHome = URI.file(await fs.mkdtemp(`${os.tmpdir()}/known-discovery-home-`)); + const workingDirectory = await fs.mkdtemp(`${os.tmpdir()}/known-discovery-cwd-`); + const tryOpened: string[] = []; + class CountingSessionDataService extends TestSessionDataService { + override async tryOpenDatabase(session: URI): Promise | undefined> { + tryOpened.push(AgentSession.id(session)); + return super.tryOpenDatabase(session); + } + } + const sessionDataService = disposables.add(new CountingSessionDataService()); + const client = new TestCopilotClient([ + sdkSession('known-a', workingDirectory), + sdkSession('known-b', workingDirectory), + sdkSession('fresh', workingDirectory, { clientName: 'github/cli', repository: 'owner/repository', modifiedTime: new Date() }), + ]); + const { agent } = createTestAgentContext(disposables, { sessionDataService, copilotClient: client, userHome }); + const filterCalls: string[][] = []; + agent.setKnownSessionsFilter(async sessions => { + filterCalls.push(sessions.map(s => AgentSession.id(s))); + return new Set([ + AgentSession.uri('copilotcli', 'known-a').toString(), + AgentSession.uri('copilotcli', 'known-b').toString(), + ]); + }); + try { + assert.deepStrictEqual({ + discovered: await collectDiscoveredChats(agent), + filterCalls, + tryOpened, + }, { + discovered: [{ id: 'fresh', external: true, adoptable: false }], + filterCalls: [['known-a', 'known-b', 'fresh']], + tryOpened: [], + }); + } finally { + await fs.rm(userHome.fsPath, { recursive: true, force: true }); + await fs.rm(workingDirectory, { recursive: true, force: true }); + await disposeAgent(agent); + } + }); + + test('does not resolve projects for adoptable chats that migration will not emit', async () => { + const userHome = URI.file(await fs.mkdtemp(`${os.tmpdir()}/adoptable-skip-home-`)); + const workingDirectory = await fs.mkdtemp(`${os.tmpdir()}/adoptable-skip-cwd-`); + const sessionDataService = disposables.add(new TestSessionDataService()); + const repositoryRootCalls: string[] = []; + class CountingGitService extends TestAgentHostGitService { + override async getRepositoryRoot(workingDirectory?: URI): Promise { + repositoryRootCalls.push(workingDirectory?.fsPath ?? ''); + return super.getRepositoryRoot(); + } + } + const gitService = new CountingGitService(); + const client = new TestCopilotClient([sdkSession('ehcli-skipped', workingDirectory)]); + const { agent } = createTestAgentContext(disposables, { sessionDataService, copilotClient: client, userHome, gitService }); + try { + await writeExtensionHostMarker(userHome, 'ehcli-skipped'); + + assert.deepStrictEqual({ + discovered: await collectDiscoveredChats(agent), + repositoryRootCalls, + }, { + discovered: [], + repositoryRootCalls: [], + }); + } finally { + await fs.rm(userHome.fsPath, { recursive: true, force: true }); + await fs.rm(workingDirectory, { recursive: true, force: true }); + await disposeAgent(agent); + } + }); + test('reads stored session metadata with a single bulk metadata query', async () => { + const userHome = URI.file(await fs.mkdtemp(`${os.tmpdir()}/bulk-metadata-home-`)); + const workingDirectory = await fs.mkdtemp(`${os.tmpdir()}/bulk-metadata-cwd-`); + const calls: string[] = []; + class CountingSessionDataService extends TestSessionDataService { + override async tryOpenDatabase(session: URI): Promise | undefined> { + const ref = await super.tryOpenDatabase(session); + if (!ref) { + return ref; + } + const db = ref.object; + const object = { + getMetadata: (key: string) => { calls.push(`getMetadata:${key}`); return db.getMetadata(key); }, + getMetadataObject: (keys: Record) => { calls.push('getMetadataObject'); return db.getMetadataObject(keys); }, + } as unknown as SessionDatabase; + return { object, dispose: () => ref.dispose() }; + } + } + const sessionDataService = disposables.add(new CountingSessionDataService()); + const session = AgentSession.uri('copilotcli', 'bulk-metadata'); + const db = sessionDataService.openDatabase(session); + await db.object.setMetadata('copilot.workingDirectory', URI.file(workingDirectory).toString()); + db.dispose(); + const client = new TestCopilotClient([sdkSession('bulk-metadata', workingDirectory)]); + const { agent } = createTestAgentContext(disposables, { sessionDataService, copilotClient: client, userHome }); + try { + await collectDiscoveredChats(agent); + + assert.deepStrictEqual(calls, ['getMetadataObject']); + } finally { + await fs.rm(userHome.fsPath, { recursive: true, force: true }); + await fs.rm(workingDirectory, { recursive: true, force: true }); + await disposeAgent(agent); + } + }); }); suite('createChat fork', () => { From 1e3ed44201658c36b0bce2bba29d7a6379f41950 Mon Sep 17 00:00:00 2001 From: Sandeep Somavarapu Date: Mon, 17 Aug 2026 23:01:33 +0200 Subject: [PATCH 13/19] agentHost: Scope server tools to exact chats (#331307) * agentHost: scope server tools to exact chats Pass exact Agent Host chat channels through every provider and derive owning session context centrally for session-scoped tools. This fixes rename_chat targeting for Claude and Codex peer chats while preserving Agent Merge and feedback behavior.\n\nFixes #331165\n\nCo-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * agentHost: test Claude peer server tool routing Capture Claude SDK server-tool handlers in the provider test double and verify a materialized peer chat forwards its exact Agent Host chat URI.\n\nCo-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../agentHost/common/agentServerTools.ts | 11 ++-- .../node/claude/claudeAgentSession.ts | 2 +- .../node/claude/claudeServerToolMcpServer.ts | 4 +- .../agentHost/node/codex/codexAgent.ts | 25 ++------- .../node/shared/agentFeedbackServerTools.ts | 8 +-- .../node/shared/agentMergeServerTools.ts | 10 ++-- .../node/shared/agentServerToolHost.ts | 36 ++++++++---- .../node/shared/sessionServerTools.ts | 15 ++--- .../node/agentFeedbackServerTools.test.ts | 21 +++---- .../test/node/agentMergeServerTools.test.ts | 35 ++++++++++++ .../agentHost/test/node/claudeAgent.test.ts | 36 ++++++++++++ .../node/claudeServerToolMcpServer.test.ts | 18 +++--- .../test/node/codex/codexCreateChat.test.ts | 26 ++++----- .../test/node/sessionServerTools.test.ts | 56 +++++++++++++++---- 14 files changed, 204 insertions(+), 99 deletions(-) diff --git a/src/vs/platform/agentHost/common/agentServerTools.ts b/src/vs/platform/agentHost/common/agentServerTools.ts index 73c333af7d642e..7b99c8f28cc7dd 100644 --- a/src/vs/platform/agentHost/common/agentServerTools.ts +++ b/src/vs/platform/agentHost/common/agentServerTools.ts @@ -17,7 +17,8 @@ import type { ToolDefinition, URI } from './state/sessionState.js'; * never hard-code any specific tool — they read {@link definitions} / * {@link toolNames} and route through {@link executeTool}. * - * `sessionUri` is the session's protocol URI. + * Tool invocation methods take the exact Agent Host chat channel URI. The host + * resolves its owning session for session-scoped state and tools. */ export interface IAgentServerToolHost { /** Every server tool definition across the contributed groups. */ @@ -39,20 +40,20 @@ export interface IAgentServerToolHost { canRequireConfirmation(toolName: string): boolean; /** * Whether {@link toolName} needs to prompt for *this* invocation, given the - * current state of {@link sessionUri}. Lets a tool that normally confirms + * current state of {@link chatUri}. Lets a tool that normally confirms * run silently when it has nothing to confirm. Defaults to * {@link canRequireConfirmation} when the owning group has no * session-specific condition. * * Providers must consult this before prompting or executing the tool. */ - requiresConfirmation(sessionUri: URI, toolName: string): boolean; + requiresConfirmation(chatUri: URI, toolName: string): boolean; /** - * Executes a server tool against the session's state, dispatching any + * Executes a server tool for the exact chat that invoked it, dispatching any * resulting actions, and returns the textual tool result for the agent. * * @throws if {@link toolName} is not a known server tool or the arguments * are invalid. */ - executeTool(sessionUri: URI, toolName: string, rawArgs: unknown): string | Promise; + executeTool(chatUri: URI, toolName: string, rawArgs: unknown): string | Promise; } diff --git a/src/vs/platform/agentHost/node/claude/claudeAgentSession.ts b/src/vs/platform/agentHost/node/claude/claudeAgentSession.ts index 7328078e0195a3..8eac456ee500ee 100644 --- a/src/vs/platform/agentHost/node/claude/claudeAgentSession.ts +++ b/src/vs/platform/agentHost/node/claude/claudeAgentSession.ts @@ -777,7 +777,7 @@ export class ClaudeAgentSession extends Disposable { ): Promise<{ mcpServers: Record | undefined; allowedTools: readonly string[] | undefined }> { const clientServers = await buildClientMcpServers(this.toolDiff, this._pendingClientToolCalls, this._sdkService); const serverToolServer = serverToolHost - ? await buildServerToolMcpServer(serverToolHost, resource.toString(), this._sdkService) + ? await buildServerToolMcpServer(serverToolHost, this._chatChannelUri.toString(), this._sdkService) : undefined; const mcpServers = (!clientServers && !serverToolServer) ? undefined diff --git a/src/vs/platform/agentHost/node/claude/claudeServerToolMcpServer.ts b/src/vs/platform/agentHost/node/claude/claudeServerToolMcpServer.ts index 084e3bbf134a3e..1f5794857df478 100644 --- a/src/vs/platform/agentHost/node/claude/claudeServerToolMcpServer.ts +++ b/src/vs/platform/agentHost/node/claude/claudeServerToolMcpServer.ts @@ -56,7 +56,7 @@ export function extractServerToolName(toolName: string): string | undefined { */ export async function buildServerToolMcpServer( host: IAgentServerToolHost, - sessionUri: string, + chatUri: string, sdk: IClaudeAgentSdkService, ): Promise { const tools = await Promise.all(host.definitions.map(def => sdk.tool( @@ -65,7 +65,7 @@ export async function buildServerToolMcpServer( jsonSchemaToZodRawShape(def.inputSchema), async args => { try { - const text = await host.executeTool(sessionUri, def.name, args); + const text = await host.executeTool(chatUri, def.name, args); return { content: [{ type: 'text' as const, text }] }; } catch (error) { const message = error instanceof Error ? error.message : String(error); diff --git a/src/vs/platform/agentHost/node/codex/codexAgent.ts b/src/vs/platform/agentHost/node/codex/codexAgent.ts index 8f44be68826597..45f59370b9ad29 100644 --- a/src/vs/platform/agentHost/node/codex/codexAgent.ts +++ b/src/vs/platform/agentHost/node/codex/codexAgent.ts @@ -2080,22 +2080,6 @@ export class CodexAgent extends Disposable implements IAgent { })); } - /** - * The scope Codex hands {@link IAgentServerToolHost} for a session's - * server-tool confirmation/execution: the same host-supplied - * configuration scope {@link IAgentServerToolHost.advertise} was called - * with for this chat (see {@link _configScope}), never the runtime's own - * identity. A peer chat's runtime is keyed by its own thread id (e.g. - * `codex:/`) once materialized, which is neither the addressed - * AH session nor the chat channel and — critically — not the scope the - * host indexes its per-session tool state under. Falls back to the - * runtime's own URI only when the chat has never been tracked (there is - * no better scope to route through). - */ - private _serverToolScope(session: ICodexSession): URI { - return session.chatChannel ? this._configScope(session.chatChannel) : session.sessionUri; - } - private async _handleDynamicToolCallRpc(params: DynamicToolCallParams): Promise> { const sessionId = this._sessionIdByThreadId.get(params.threadId); const session = sessionId ? this._sessions.get(sessionId) : undefined; @@ -2109,8 +2093,11 @@ export class CodexAgent extends Disposable implements IAgent { const host = this._serverToolHost; if (host && params.namespace === null && host.toolNames.includes(params.tool)) { try { - const scope = this._serverToolScope(session).toString(); - if (host.requiresConfirmation(scope, params.tool)) { + const chatChannel = session.chatChannel?.toString(); + if (!chatChannel) { + return { result: this._toolFailure(`No chat channel for server tool ${params.tool}`) }; + } + if (host.requiresConfirmation(chatChannel, params.tool)) { const entry = session.mapState.itemToToolCall.get(params.callId); if (!entry) { return { result: this._toolFailure(`No pending server tool call for ${params.tool} (callId ${params.callId})`) }; @@ -2129,7 +2116,7 @@ export class CodexAgent extends Disposable implements IAgent { return { result: this._toolFailure(`Server tool ${params.tool} was not approved`) }; } } - const text = host.executeTool(scope, params.tool, params.arguments); + const text = host.executeTool(chatChannel, params.tool, params.arguments); return { result: { contentItems: [{ type: 'inputText', text: await text }], success: true } }; } catch (err) { return { result: this._toolFailure(`Server tool ${params.tool} failed: ${err instanceof Error ? err.message : String(err)}`) }; diff --git a/src/vs/platform/agentHost/node/shared/agentFeedbackServerTools.ts b/src/vs/platform/agentHost/node/shared/agentFeedbackServerTools.ts index 533cebc3f2b1db..1e36dd9a233bad 100644 --- a/src/vs/platform/agentHost/node/shared/agentFeedbackServerTools.ts +++ b/src/vs/platform/agentHost/node/shared/agentFeedbackServerTools.ts @@ -653,17 +653,17 @@ export const feedbackServerToolGroup: IServerToolGroup = { canRequireConfirmation(toolName): boolean { return feedbackToolRequiresConfirmation(toolName); }, - requiresConfirmation(stateManager, chatUri, toolName): boolean { + requiresConfirmation(stateManager, context, toolName): boolean { if (!feedbackToolRequiresConfirmation(toolName)) { return false; } - return hasRevealableComments(getFeedbackToolState(stateManager, chatUri).state); + return hasRevealableComments(getFeedbackToolState(stateManager, context.chatUri).state); }, getDisplay(toolName, args, result): IServerToolDisplay | undefined { return getFeedbackToolDisplay(toolName, args, result); }, - execute(stateManager, chatUri, toolName, rawArgs): string { - const { mainSessionUri, annotationsUri, state } = getFeedbackToolState(stateManager, chatUri); + execute(stateManager, context, toolName, rawArgs): string { + const { mainSessionUri, annotationsUri, state } = getFeedbackToolState(stateManager, context.chatUri); const outcome = applyFeedbackTool(state, mainSessionUri, toolName, rawArgs); for (const action of outcome.actions) { stateManager.dispatchServerAction(annotationsUri, action); diff --git a/src/vs/platform/agentHost/node/shared/agentMergeServerTools.ts b/src/vs/platform/agentHost/node/shared/agentMergeServerTools.ts index 356ce8219dca7b..f8b907443d5ebd 100644 --- a/src/vs/platform/agentHost/node/shared/agentMergeServerTools.ts +++ b/src/vs/platform/agentHost/node/shared/agentMergeServerTools.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import { localize } from '../../../../nls.js'; -import type { ToolDefinition, URI } from '../../common/state/sessionState.js'; +import type { ToolDefinition } from '../../common/state/sessionState.js'; import type { AgentHostStateManager } from '../agentHostStateManager.js'; import type { IServerToolDisplay, IServerToolDisplayResult, IServerToolGroup } from './agentServerToolHost.js'; @@ -62,17 +62,17 @@ export function createAgentMergeServerToolGroup(accessor?: IAgentMergeToolAccess return { definitions, isEnabled: toolName => accessor?.isEnabled() === true && definitions.some(definition => definition.name === toolName), - execute: (_stateManager: AgentHostStateManager, sessionUri: URI, toolName: string, rawArgs: unknown) => { + execute: (_stateManager: AgentHostStateManager, context, toolName: string, rawArgs: unknown) => { if (!accessor) { throw new Error('Agent Merge tools are not available without an Agent Merge controller.'); } switch (toolName) { case readAgentMergeCIToolName: - return accessor.readFailedCI(sessionUri); + return accessor.readFailedCI(context.sessionUri); case replyToAgentMergeReviewThreadToolName: { const args = asRecord(rawArgs, toolName); return accessor.replyToReviewThread( - sessionUri, + context.sessionUri, requiredString(args.threadId, 'threadId', toolName), requiredString(args.body, 'body', toolName), optionalBoolean(args.resolve, 'resolve', toolName) ?? true, @@ -81,7 +81,7 @@ export function createAgentMergeServerToolGroup(accessor?: IAgentMergeToolAccess case rerunAgentMergeWorkflowToolName: { const args = asRecord(rawArgs, toolName); return accessor.rerunFailedWorkflow( - sessionUri, + context.sessionUri, requiredString(args.runId, 'runId', toolName), optionalBoolean(args.failedJobsOnly, 'failedJobsOnly', toolName) ?? true, ); diff --git a/src/vs/platform/agentHost/node/shared/agentServerToolHost.ts b/src/vs/platform/agentHost/node/shared/agentServerToolHost.ts index d9e13839a35b08..73afe258872722 100644 --- a/src/vs/platform/agentHost/node/shared/agentServerToolHost.ts +++ b/src/vs/platform/agentHost/node/shared/agentServerToolHost.ts @@ -5,7 +5,7 @@ import type { IAgentServerToolHost } from '../../common/agentServerTools.js'; import { ActionType } from '../../common/state/protocol/common/actions.js'; -import type { StringOrMarkdown, ToolDefinition, URI } from '../../common/state/sessionState.js'; +import { parseRequiredSessionUriFromChatUri, type StringOrMarkdown, type ToolDefinition, type URI } from '../../common/state/sessionState.js'; import type { AgentHostStateManager } from '../agentHostStateManager.js'; /** @@ -36,6 +36,11 @@ export interface IServerToolDisplay { readonly pastTenseMessage?: StringOrMarkdown; } +export interface IServerToolExecutionContext { + readonly sessionUri: URI; + readonly chatUri: URI; +} + /** * A group of related server tools owned and executed by the agent host. Each * group bundles the {@link ToolDefinition}s it advertises with an executor @@ -65,12 +70,12 @@ export interface IServerToolGroup { canRequireConfirmation?(toolName: string): boolean; /** * Whether {@link toolName} needs to prompt for the invocation currently - * being made against {@link sessionUri}. Implement this for + * being made in {@link IServerToolExecutionContext.chatUri}. Implement this for * state-dependent confirmation (e.g. nothing to confirm yet) while keeping * {@link canRequireConfirmation} stable for provider allow-lists. Absent * falls back to {@link canRequireConfirmation}. */ - requiresConfirmation?(stateManager: AgentHostStateManager, sessionUri: URI, toolName: string): boolean; + requiresConfirmation?(stateManager: AgentHostStateManager, context: IServerToolExecutionContext, toolName: string): boolean; /** * Executes {@link toolName} (one of this group's {@link definitions}) * against the session's state, dispatching any resulting actions through @@ -80,7 +85,7 @@ export interface IServerToolGroup { * @throws if {@link toolName} is not owned by this group or the arguments * are invalid. */ - execute(stateManager: AgentHostStateManager, sessionUri: URI, toolName: string, rawArgs: unknown): string | Promise; + execute(stateManager: AgentHostStateManager, context: IServerToolExecutionContext, toolName: string, rawArgs: unknown): string | Promise; /** * Display strings for {@link toolName} (one of this group's @@ -151,29 +156,36 @@ export class AgentServerToolHost implements IAgentServerToolHost { return group?.isEnabled(toolName) === true && (group.canRequireConfirmation?.(toolName) ?? false); } - requiresConfirmation(sessionUri: URI, toolName: string): boolean { + requiresConfirmation(chatUri: URI, toolName: string): boolean { const group = this._groupByToolName.get(toolName); - if (group && !this._isEnabledForSession(group, sessionUri, toolName)) { + if (group && !this._isEnabledForSession(group, chatUri, toolName)) { return false; } - return group?.requiresConfirmation?.(this._stateManager, sessionUri, toolName) + return group?.requiresConfirmation?.(this._stateManager, this._executionContext(chatUri), toolName) ?? group?.canRequireConfirmation?.(toolName) ?? false; } - executeTool(sessionUri: URI, toolName: string, rawArgs: unknown): string | Promise { + executeTool(chatUri: URI, toolName: string, rawArgs: unknown): string | Promise { const group = this._groupByToolName.get(toolName); if (!group) { throw new Error(`Unknown server tool: ${toolName}`); } - if (!this._isEnabledForSession(group, sessionUri, toolName)) { + if (!this._isEnabledForSession(group, chatUri, toolName)) { throw new Error(`Server tool "${toolName}" is disabled.`); } - return group.execute(this._stateManager, sessionUri, toolName, rawArgs); + return group.execute(this._stateManager, this._executionContext(chatUri), toolName, rawArgs); + } + + private _executionContext(chatUri: URI): IServerToolExecutionContext { + return { + sessionUri: parseRequiredSessionUriFromChatUri(chatUri), + chatUri, + }; } - private _isEnabledForSession(group: IServerToolGroup, sessionUri: URI, toolName: string): boolean { - const advertisedTools = this._stateManager.getSessionState(sessionUri)?.serverTools; + private _isEnabledForSession(group: IServerToolGroup, chatUri: URI, toolName: string): boolean { + const advertisedTools = this._stateManager.getSessionState(chatUri)?.serverTools; return advertisedTools ? advertisedTools.some(tool => tool.name === toolName) : group.isEnabled(toolName); diff --git a/src/vs/platform/agentHost/node/shared/sessionServerTools.ts b/src/vs/platform/agentHost/node/shared/sessionServerTools.ts index c6872a31e73304..402f4050ec927b 100644 --- a/src/vs/platform/agentHost/node/shared/sessionServerTools.ts +++ b/src/vs/platform/agentHost/node/shared/sessionServerTools.ts @@ -1210,20 +1210,21 @@ export function createSessionServerToolGroup(accessor?: ISessionServerToolAccess getDisplay(toolName: string, args: unknown, result?: IServerToolDisplayResult): IServerToolDisplay | undefined { return getSessionToolDisplay(toolName, args, result); }, - async execute(_stateManager: AgentHostStateManager, sessionUri: ProtocolURI, toolName: string, rawArgs: unknown): Promise { + async execute(_stateManager: AgentHostStateManager, context, toolName: string, rawArgs: unknown): Promise { if (!accessor) { throw new Error(`Session server tool "${toolName}" cannot run: the group was built without a session accessor.`); } + const currentChannel = context.chatUri; switch (toolName) { case SessionServerToolName.ListSessions: return serializeSessions(filterSessions(await accessor.listSessions(), getListSessionsArgs(rawArgs))); case SessionServerToolName.GetCurrentSession: - return serializeCurrentSession(currentSessionUri(sessionUri), await accessor.listSessions()); + return serializeCurrentSession(currentSessionUri(currentChannel), await accessor.listSessions()); case SessionServerToolName.CreateSession: { if (createdSessionCount >= maxCreatedSessions) { throw new Error(`Refusing to create more than ${maxCreatedSessions} sessions from server tools in this process.`); } - const result = await applyCreateSessionTool(accessor, rawArgs, URI.parse(sessionUri)); + const result = await applyCreateSessionTool(accessor, rawArgs, URI.parse(currentChannel)); createdSessionCount++; return formatCreateSessionResult(result); } @@ -1231,24 +1232,24 @@ export function createSessionServerToolGroup(accessor?: ISessionServerToolAccess if (createdChatCount >= maxCreatedChats) { throw new Error(`Refusing to create more than ${maxCreatedChats} chats from server tools in this process.`); } - const result = await applyCreateChatTool(accessor, rawArgs, URI.parse(sessionUri)); + const result = await applyCreateChatTool(accessor, rawArgs, URI.parse(currentChannel)); createdChatCount++; return formatCreateChatResult(result); } case SessionServerToolName.RenameChat: - return applyRenameChatTool(accessor, rawArgs, sessionUri); + return applyRenameChatTool(accessor, rawArgs, currentChannel); case SessionServerToolName.SendMessage: { if (sentMessageCount >= maxSentMessages) { throw new Error(`Refusing to send more than ${maxSentMessages} messages from server tools in this process.`); } - const result = await applySendMessageTool(accessor, rawArgs, sessionUri); + const result = await applySendMessageTool(accessor, rawArgs, currentChannel); sentMessageCount++; return result; } case SessionServerToolName.GetSessionContext: return applyGetSessionContextTool(accessor, rawArgs); case SessionServerToolName.DeleteSession: - return applyDeleteSessionTool(accessor, rawArgs, currentSessionUri(sessionUri)); + return applyDeleteSessionTool(accessor, rawArgs, currentSessionUri(currentChannel)); default: throw new Error(`Unknown session server tool: ${toolName}`); } diff --git a/src/vs/platform/agentHost/test/node/agentFeedbackServerTools.test.ts b/src/vs/platform/agentHost/test/node/agentFeedbackServerTools.test.ts index 1a8a65127b8515..42e1aec7d4148a 100644 --- a/src/vs/platform/agentHost/test/node/agentFeedbackServerTools.test.ts +++ b/src/vs/platform/agentHost/test/node/agentFeedbackServerTools.test.ts @@ -9,7 +9,7 @@ import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../base/test/c import { NullLogService } from '../../../log/common/log.js'; import { feedbackAnnotationEntryMeta, FEEDBACK_ANNOTATION_META_KEY, readFeedbackAnnotationEntryAuthor, type IFeedbackAnnotationMeta } from '../../common/meta/agentFeedbackAnnotations.js'; import { ActionType } from '../../common/state/protocol/common/actions.js'; -import { Annotation, AnnotationsState, SessionStatus, SessionSummary, buildChatUri } from '../../common/state/sessionState.js'; +import { Annotation, AnnotationsState, SessionStatus, SessionSummary, buildChatUri, buildDefaultChatUri } from '../../common/state/sessionState.js'; import { buildAnnotationsUri } from '../../common/annotationsUri.js'; import { AgentHostStateManager } from '../../node/agentHostStateManager.js'; import { AgentServerToolHost } from '../../node/shared/agentServerToolHost.js'; @@ -386,7 +386,7 @@ suite('AgentFeedbackServerTools', () => { teardown(() => disposables.dispose()); test('executeTool round-trips a comment into the annotation state', () => { - host.executeTool(sessionResource, addCommentToolName, { + host.executeTool(buildDefaultChatUri(sessionResource), addCommentToolName, { resourceUri: fileUri, range: { startLineNumber: 1, startColumn: 1, endLineNumber: 1, endColumn: 2 }, text: 'hello', @@ -404,7 +404,7 @@ suite('AgentFeedbackServerTools', () => { annotation: annotation('reply-target', 'accepted', false, 'original'), }); - await host.executeTool(sessionResource, replyToCommentToolName, { + await host.executeTool(buildDefaultChatUri(sessionResource), replyToCommentToolName, { commentId: 'reply-target', text: 'agent reply', }); @@ -442,7 +442,7 @@ suite('AgentFeedbackServerTools', () => { annotation: annotation('auto-submit', 'created', false, 'submit me', 'prReview'), }); - const result = await host.executeTool(sessionResource, viewUnreviewedCommentsToolName, {}); + const result = await host.executeTool(buildDefaultChatUri(sessionResource), viewUnreviewedCommentsToolName, {}); const state = manager.getSnapshot(annotationsUri)!.state as AnnotationsState; const meta = state.annotations[0]._meta?.[FEEDBACK_ANNOTATION_META_KEY] as IFeedbackAnnotationMeta; @@ -486,29 +486,30 @@ suite('AgentFeedbackServerTools', () => { test('requiresConfirmation only prompts when comments can be revealed', async () => { const annotationsUri = buildAnnotationsUri(sessionResource); const chatUri = buildChatUri(sessionResource, 'peer-chat-1'); - const empty = host.requiresConfirmation(sessionResource, viewUnreviewedCommentsToolName); + const defaultChatUri = buildDefaultChatUri(sessionResource); + const empty = host.requiresConfirmation(defaultChatUri, viewUnreviewedCommentsToolName); manager.dispatchServerAction(annotationsUri, { type: ActionType.AnnotationsSet, annotation: annotation('accepted', 'accepted', false, 'already accepted', 'prReview'), }); - const acceptedOnly = host.requiresConfirmation(sessionResource, viewUnreviewedCommentsToolName); + const acceptedOnly = host.requiresConfirmation(defaultChatUri, viewUnreviewedCommentsToolName); manager.dispatchServerAction(annotationsUri, { type: ActionType.AnnotationsSet, annotation: annotation('created', 'created', false, 'new comment', 'codeReview'), }); - const created = host.requiresConfirmation(sessionResource, viewUnreviewedCommentsToolName); + const created = host.requiresConfirmation(defaultChatUri, viewUnreviewedCommentsToolName); const peerChat = host.requiresConfirmation(chatUri, viewUnreviewedCommentsToolName); - await host.executeTool(sessionResource, viewUnreviewedCommentsToolName, {}); - const delivered = host.requiresConfirmation(sessionResource, viewUnreviewedCommentsToolName); + await host.executeTool(defaultChatUri, viewUnreviewedCommentsToolName, {}); + const delivered = host.requiresConfirmation(defaultChatUri, viewUnreviewedCommentsToolName); manager.dispatchServerAction(annotationsUri, { type: ActionType.AnnotationsSet, annotation: annotation('pending', 'accepted', false, 'selected comment', 'prReview', true), }); - const pendingSelection = host.requiresConfirmation(sessionResource, viewUnreviewedCommentsToolName); + const pendingSelection = host.requiresConfirmation(defaultChatUri, viewUnreviewedCommentsToolName); assert.deepStrictEqual({ empty, diff --git a/src/vs/platform/agentHost/test/node/agentMergeServerTools.test.ts b/src/vs/platform/agentHost/test/node/agentMergeServerTools.test.ts index 3353b945a32b4f..7139e8551219f8 100644 --- a/src/vs/platform/agentHost/test/node/agentMergeServerTools.test.ts +++ b/src/vs/platform/agentHost/test/node/agentMergeServerTools.test.ts @@ -5,7 +5,11 @@ import * as assert from 'assert'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../base/test/common/utils.js'; +import { NullLogService } from '../../../log/common/log.js'; +import { buildChatUri, SessionStatus } from '../../common/state/sessionState.js'; +import { AgentHostStateManager } from '../../node/agentHostStateManager.js'; import { createAgentMergeServerToolGroup, readAgentMergeCIToolName, replyToAgentMergeReviewThreadToolName, rerunAgentMergeWorkflowToolName, type IAgentMergeToolAccessor } from '../../node/shared/agentMergeServerTools.js'; +import { AgentServerToolHost } from '../../node/shared/agentServerToolHost.js'; suite('Agent Merge server tools', () => { ensureNoDisposablesAreLeakedInTestSuite(); @@ -31,4 +35,35 @@ suite('Agent Merge server tools', () => { withoutAccessor: false, }); }); + + test('resolves the owning session for a tool invoked from a peer chat', async () => { + const sessionUri = 'copilot:/merge-session'; + const chatUri = buildChatUri(sessionUri, 'peer'); + let receivedSession: string | undefined; + const stateManager = new AgentHostStateManager(new NullLogService()); + stateManager.createSession({ + resource: sessionUri, + provider: 'copilot', + title: 'Agent Merge', + status: SessionStatus.Idle, + createdAt: new Date(0).toISOString(), + modifiedAt: new Date(0).toISOString(), + }); + const host = new AgentServerToolHost(stateManager, [ + createAgentMergeServerToolGroup({ + isEnabled: () => true, + readFailedCI: async session => { + receivedSession = session; + return 'result'; + }, + replyToReviewThread: async () => '', + rerunFailedWorkflow: async () => '', + }), + ]); + + const result = await host.executeTool(chatUri, readAgentMergeCIToolName, {}); + + assert.deepStrictEqual({ result, receivedSession }, { result: 'result', receivedSession: sessionUri }); + stateManager.dispose(); + }); }); diff --git a/src/vs/platform/agentHost/test/node/claudeAgent.test.ts b/src/vs/platform/agentHost/test/node/claudeAgent.test.ts index abc82e1abb1ac6..809a500edd6756 100644 --- a/src/vs/platform/agentHost/test/node/claudeAgent.test.ts +++ b/src/vs/platform/agentHost/test/node/claudeAgent.test.ts @@ -665,6 +665,7 @@ class FakeClaudeAgentSdkService implements IClaudeAgentSdkService { readonly description: string; readonly inputSchema: Record; }> = []; + readonly toolHandlers = new Map Promise>(); readonly createSdkMcpServerCalls: Array<{ readonly name: string; readonly toolNames: readonly string[]; @@ -677,6 +678,7 @@ class FakeClaudeAgentSdkService implements IClaudeAgentSdkService { _handler: (args: any, extra: unknown) => Promise, ): Promise> { this.toolCalls.push({ name, description, inputSchema }); + this.toolHandlers.set(name, _handler); return { name } as unknown as SdkMcpToolDefinition; } @@ -10305,6 +10307,40 @@ suite('ClaudeAgent — host seams', () => { const disposables = ensureNoDisposablesAreLeakedInTestSuite(); + test('a peer chat server tool executes against its exact Agent Host chat channel', async () => { + const { agent, sdk } = createTestContext(disposables); + const toolName = 'peer_server_tool'; + let executedChatUri: string | undefined; + agent.setServerToolHost({ + definitions: [{ name: toolName, inputSchema: { type: 'object', properties: {} } }], + toolNames: [toolName], + advertise: () => { }, + canRequireConfirmation: () => false, + requiresConfirmation: () => false, + executeTool: chatUri => { + executedChatUri = chatUri; + return 'done'; + }, + }); + await agent.authenticate(GITHUB_COPILOT_PROTECTED_RESOURCE.resource, 'tok'); + const created = await createSession(agent, { workingDirectories: [URI.file('/work')] }); + const peerChat = URI.parse(buildChatUri(created.session.toString(), 'peer-server-tool')); + const peerCreated = await agent.chats.createChat(peerChat, { configurationResource: created.session, resource: peerChat }, { ...resolvedChatOptions() }); + const peerSdkId = AgentSession.id(peerCreated!.backingSession!); + sdk.nextQueryMessages = [makeSystemInitMessage(peerSdkId), makeResultSuccess(peerSdkId)]; + + await agent.chats.sendMessage(peerChat, 'hi', undefined, undefined, 'turn-1', undefined, undefined, chatContext(peerChat)); + const result = await sdk.toolHandlers.get(toolName)!({}, undefined); + + assert.deepStrictEqual({ + executedChatUri, + result, + }, { + executedChatUri: peerChat.toString(), + result: { content: [{ type: 'text', text: 'done' }] }, + }); + }); + test('a subagent chat resolves its spawn edge only from the host-supplied origin', async () => { const { agent, sdk, stateManager } = createTestContext(disposables); await agent.authenticate(GITHUB_COPILOT_PROTECTED_RESOURCE.resource, 'tok'); diff --git a/src/vs/platform/agentHost/test/node/claudeServerToolMcpServer.test.ts b/src/vs/platform/agentHost/test/node/claudeServerToolMcpServer.test.ts index a14b4111c7b68b..a7a3b6800e81fe 100644 --- a/src/vs/platform/agentHost/test/node/claudeServerToolMcpServer.test.ts +++ b/src/vs/platform/agentHost/test/node/claudeServerToolMcpServer.test.ts @@ -8,7 +8,7 @@ import type { CallToolResult } from '@modelcontextprotocol/sdk/types.js'; import type { McpSdkServerConfigWithInstance } from '@anthropic-ai/claude-agent-sdk'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../base/test/common/utils.js'; import type { IAgentServerToolHost } from '../../common/agentServerTools.js'; -import type { ToolDefinition } from '../../common/state/sessionState.js'; +import { buildChatUri, type ToolDefinition } from '../../common/state/sessionState.js'; import type { IClaudeAgentSdkService } from '../../node/claude/claudeAgentSdkService.js'; import { buildServerToolMcpServer, @@ -44,7 +44,7 @@ const fakeToolDefinitions: readonly ToolDefinition[] = [ class FakeServerToolHost implements IAgentServerToolHost { readonly definitions: readonly ToolDefinition[] = fakeToolDefinitions; readonly toolNames: readonly string[] = fakeToolDefinitions.map(def => def.name); - readonly executions: Array<{ sessionUri: string; toolName: string; rawArgs: unknown }> = []; + readonly executions: Array<{ chatUri: string; toolName: string; rawArgs: unknown }> = []; result = 'ok'; error: Error | undefined; @@ -54,8 +54,8 @@ class FakeServerToolHost implements IAgentServerToolHost { requiresConfirmation(_sessionUri: string, _toolName: string): boolean { return false; } - executeTool(sessionUri: string, toolName: string, rawArgs: unknown): string { - this.executions.push({ sessionUri, toolName, rawArgs }); + executeTool(chatUri: string, toolName: string, rawArgs: unknown): string { + this.executions.push({ chatUri, toolName, rawArgs }); if (this.error) { throw this.error; } @@ -67,12 +67,12 @@ suite('claudeServerToolMcpServer / buildServerToolMcpServer', () => { ensureNoDisposablesAreLeakedInTestSuite(); - const sessionUri = 'claude:/server-tool-session'; + const chatUri = buildChatUri('claude:/server-tool-session', 'peer'); test('registers every server tool on the server-tool MCP server', async () => { const { sdk, recorded } = makeSdk(); const host = new FakeServerToolHost(); - const server = await buildServerToolMcpServer(host, sessionUri, sdk); + const server = await buildServerToolMcpServer(host, chatUri, sdk); assert.deepStrictEqual({ serverName: server.name, toolNames: recorded.map(t => t.name).sort(), @@ -86,7 +86,7 @@ suite('claudeServerToolMcpServer / buildServerToolMcpServer', () => { const { sdk, recorded } = makeSdk(); const host = new FakeServerToolHost(); host.result = 'listed 2 comments'; - await buildServerToolMcpServer(host, sessionUri, sdk); + await buildServerToolMcpServer(host, chatUri, sdk); const handler = recorded.find(t => t.name === 'serverToolA')!.handler; const result = await handler({ foo: 'bar' }, undefined); @@ -95,7 +95,7 @@ suite('claudeServerToolMcpServer / buildServerToolMcpServer', () => { executions: host.executions, result, }, { - executions: [{ sessionUri, toolName: 'serverToolA', rawArgs: { foo: 'bar' } }], + executions: [{ chatUri, toolName: 'serverToolA', rawArgs: { foo: 'bar' } }], result: { content: [{ type: 'text', text: 'listed 2 comments' }] }, }); }); @@ -104,7 +104,7 @@ suite('claudeServerToolMcpServer / buildServerToolMcpServer', () => { const { sdk, recorded } = makeSdk(); const host = new FakeServerToolHost(); host.error = new Error('boom'); - await buildServerToolMcpServer(host, sessionUri, sdk); + await buildServerToolMcpServer(host, chatUri, sdk); const result = await recorded[0]!.handler({}, undefined); assert.deepStrictEqual(result, { content: [{ type: 'text', text: 'boom' }], isError: true }); diff --git a/src/vs/platform/agentHost/test/node/codex/codexCreateChat.test.ts b/src/vs/platform/agentHost/test/node/codex/codexCreateChat.test.ts index 5cdfa5dd8b1d6f..1541a77ef7356e 100644 --- a/src/vs/platform/agentHost/test/node/codex/codexCreateChat.test.ts +++ b/src/vs/platform/agentHost/test/node/codex/codexCreateChat.test.ts @@ -264,23 +264,21 @@ function createThrowingAdvertiseServerToolHost(message: string): IAgentServerToo const PEER_TEST_TOOL_NAME = 'peer_test_tool'; /** - * Records the exact scope Codex hands {@link IAgentServerToolHost.requiresConfirmation} - * and {@link IAgentServerToolHost.executeTool} for a single server tool - * ({@link PEER_TEST_TOOL_NAME}). `advertise` is inert here: only the - * execute/confirmation scope is under test. + * Records the exact chat channel Codex hands the server-tool host for a single + * server tool ({@link PEER_TEST_TOOL_NAME}). */ -function createRecordingCallScopeServerToolHost(calls: { readonly method: 'requiresConfirmation' | 'executeTool'; readonly scope: string }[]): IAgentServerToolHost { +function createRecordingChatServerToolHost(calls: { readonly method: 'requiresConfirmation' | 'executeTool'; readonly chatUri: string }[]): IAgentServerToolHost { return { definitions: [{ name: PEER_TEST_TOOL_NAME, description: 'test', inputSchema: { type: 'object' } }], toolNames: [PEER_TEST_TOOL_NAME], advertise: () => { }, canRequireConfirmation: () => false, - requiresConfirmation: (scope, toolName) => { - calls.push({ method: 'requiresConfirmation', scope: scope.toString() }); + requiresConfirmation: (chatUri, toolName) => { + calls.push({ method: 'requiresConfirmation', chatUri: chatUri.toString() }); return false; }, - executeTool: (scope, toolName) => { - calls.push({ method: 'executeTool', scope: scope.toString() }); + executeTool: (chatUri, _toolName, _rawArgs) => { + calls.push({ method: 'executeTool', chatUri: chatUri.toString() }); return 'tool result'; }, }; @@ -1195,10 +1193,10 @@ suite('CodexAgent exact chat routing', () => { } }); - test('a peer chat\'s server-tool call routes execute/confirmation through the host-addressed scope, never the peer runtime\'s own thread identity', async () => { + test('a peer chat\'s server-tool call uses its exact Agent Host chat channel', async () => { const agent = await createAgent(disposables, { sdkResolvableWithoutDownload: true }); - const calls: { readonly method: 'requiresConfirmation' | 'executeTool'; readonly scope: string }[] = []; - agent.setServerToolHost(createRecordingCallScopeServerToolHost(calls)); + const calls: { readonly method: 'requiresConfirmation' | 'executeTool'; readonly chatUri: string }[] = []; + agent.setServerToolHost(createRecordingChatServerToolHost(calls)); const peer = disposables.add(createTestPeer()); connectPeer(agent, peer); @@ -1252,8 +1250,8 @@ suite('CodexAgent exact chat routing', () => { // session nor the chat channel — must never reach the host. peerRuntimeUri: AgentSession.uri('codex', 'peer-thread').toString(), calls: [ - { method: 'requiresConfirmation', scope: sessionUri.toString() }, - { method: 'executeTool', scope: sessionUri.toString() }, + { method: 'requiresConfirmation', chatUri: peerChat.toString() }, + { method: 'executeTool', chatUri: peerChat.toString() }, ], toolSucceeded: true, }); diff --git a/src/vs/platform/agentHost/test/node/sessionServerTools.test.ts b/src/vs/platform/agentHost/test/node/sessionServerTools.test.ts index 1d4fe3291ae798..a08428c58384ab 100644 --- a/src/vs/platform/agentHost/test/node/sessionServerTools.test.ts +++ b/src/vs/platform/agentHost/test/node/sessionServerTools.test.ts @@ -49,6 +49,10 @@ suite('SessionServerTools', () => { return { session: URI.parse(`copilot:/${id}`), startTime: 0, modifiedTime: 0, status: status | SessionStatus.IsRead, workingDirectories: dir ? [dir] : undefined, summary: `title-${id}` }; } + function executionContext(sessionUri: string) { + return { sessionUri, chatUri: buildDefaultChatUri(sessionUri) }; + } + function createAccessor(overrides?: Partial & { onCreate?: (config: IAgentCreateSessionConfig) => void; onPrompt?: (session: URI, chat: URI, prompt: string) => void; onCreateChat?: (session: URI, chat: URI, options?: { title?: string; model?: ModelSelection }) => void; onRenameChat?: (session: URI, chat: URI, title: string) => void; onDelete?: (session: URI) => void; depths?: Map }): ISessionServerToolAccessor { const depths = overrides?.depths ?? new Map(); return { @@ -274,7 +278,7 @@ suite('SessionServerTools', () => { const accessor = createAccessor({ onCreate: c => { created = c; }, onPrompt: (_s, chat, prompt) => { prompted = { chat, prompt }; } }); const group = createSessionServerToolGroup(accessor); - const text = await group.execute(stateManager, 'copilot:/caller', SessionServerToolName.CreateSession, { workspace: workspace.toString(), prompt: 'do it', model: 'gpt-4o' }); + const text = await group.execute(stateManager, executionContext('copilot:/caller'), SessionServerToolName.CreateSession, { workspace: workspace.toString(), prompt: 'do it', model: 'gpt-4o' }); assert.deepStrictEqual(created, { workingDirectories: [workspace], provider: 'copilot', model: { id: 'gpt-4o' } }); assert.strictEqual(prompted?.prompt, 'do it'); @@ -306,7 +310,7 @@ suite('SessionServerTools', () => { const group = createSessionServerToolGroup(accessor); const store = new DisposableStore(); const stateManager = store.add(new AgentHostStateManager(new NullLogService())); - await group.execute(stateManager, source.toString(), SessionServerToolName.CreateSession, { workspace: workspace.toString(), prompt: 'do it' }); + await group.execute(stateManager, { sessionUri: 'copilot:/caller', chatUri: source.toString() }, SessionServerToolName.CreateSession, { workspace: workspace.toString(), prompt: 'do it' }); assert.deepStrictEqual({ creationSource: creationSource?.toString(), @@ -349,7 +353,7 @@ suite('SessionServerTools', () => { const store = new DisposableStore(); const stateManager = store.add(new AgentHostStateManager(new NullLogService())); const group = createSessionServerToolGroup(createAccessor()); - const text = await group.execute(stateManager, 'copilot:/caller', SessionServerToolName.ListSessions, {}); + const text = await group.execute(stateManager, executionContext('copilot:/caller'), SessionServerToolName.ListSessions, {}); assert.deepStrictEqual(JSON.parse(text).sessions.map((s: { session: string }) => s.session), ['copilot:/s1']); store.dispose(); }); @@ -367,7 +371,7 @@ suite('SessionServerTools', () => { const sessions = [idle, needsInput, elsewhere, archived, withPr, inheritedPr]; const group = createSessionServerToolGroup(createAccessor({ listSessions: async () => sessions })); - const ids = async (args: object) => JSON.parse(await group.execute(stateManager, 'copilot:/caller', SessionServerToolName.ListSessions, args)).sessions.map((s: { session: string }) => s.session); + const ids = async (args: object) => JSON.parse(await group.execute(stateManager, executionContext('copilot:/caller'), SessionServerToolName.ListSessions, args)).sessions.map((s: { session: string }) => s.session); assert.deepStrictEqual({ byStatus: await ids({ status: ['inputNeeded'] }), @@ -432,13 +436,13 @@ suite('SessionServerTools', () => { const args = { workspace: workspace.toString(), prompt: 'go' }; // From a top-level (depth 0) session, the created session is stamped depth 1. - await group.execute(stateManager, 'copilot:/caller', SessionServerToolName.CreateSession, args); + await group.execute(stateManager, executionContext('copilot:/caller'), SessionServerToolName.CreateSession, args); assert.strictEqual(depths.get('copilot:/new'), 1); // A session already at the max spawn depth may not create further sessions. depths.set('copilot:/deep', 3); await assert.rejects( - async () => { await group.execute(stateManager, 'copilot:/deep', SessionServerToolName.CreateSession, args); }, + async () => { await group.execute(stateManager, executionContext('copilot:/deep'), SessionServerToolName.CreateSession, args); }, /recursion limit/, ); store.dispose(); @@ -452,9 +456,9 @@ suite('SessionServerTools', () => { const group = createSessionServerToolGroup(createAccessor({ createSession: async () => URI.parse(`copilot:/s${n++}`) })); const args = { workspace: workspace.toString(), prompt: 'go' }; for (let i = 0; i < 25; i++) { - await group.execute(stateManager, 'copilot:/caller', SessionServerToolName.CreateSession, args); + await group.execute(stateManager, executionContext('copilot:/caller'), SessionServerToolName.CreateSession, args); } - await assert.rejects(async () => { await group.execute(stateManager, 'copilot:/caller', SessionServerToolName.CreateSession, args); }, /more than 25 sessions/); + await assert.rejects(async () => { await group.execute(stateManager, executionContext('copilot:/caller'), SessionServerToolName.CreateSession, args); }, /more than 25 sessions/); store.dispose(); }); @@ -538,6 +542,7 @@ suite('SessionServerTools', () => { listSessions: async () => [sessionMeta('s1', SessionStatus.Idle, workspace)], onRenameChat: (session, chat, title) => { renamed = { session, chat, title }; }, }); + const peer = buildChatUri('copilot:/s1', 'peer'); const defaultChat = buildDefaultChatUri('copilot:/s1'); assert.strictEqual(await applyRenameChatTool(accessor, { title: 'Default Focus' }, defaultChat), 'Renamed chat to "Default Focus".'); @@ -552,6 +557,35 @@ suite('SessionServerTools', () => { }), { chat: 'agent-host-session://copilot/s1?chat=missing', title: 'Ignored' }), /known non-default chat/); }); + test('rename_chat uses the invoking chat while server-tool state remains session-scoped', async () => { + const stateManager = new AgentHostStateManager(new NullLogService()); + const session = 'copilot:/s1'; + const peer = buildChatUri(session, 'peer'); + stateManager.createSession({ + resource: session, + provider: 'copilot', + title: 'Session', + status: SessionStatus.Idle, + createdAt: new Date(0).toISOString(), + modifiedAt: new Date(0).toISOString(), + }); + let renamedChat: string | undefined; + const host = new AgentServerToolHost(stateManager, [ + createSessionServerToolGroup(createAccessor({ + onRenameChat: (_session, chat) => { renamedChat = chat.toString(); }, + })), + ]); + host.advertise(session); + + const result = await host.executeTool(peer, SessionServerToolName.RenameChat, { title: 'Peer Focus' }); + + assert.deepStrictEqual({ result, renamedChat }, { + result: 'Renamed chat to "Peer Focus".', + renamedChat: peer, + }); + stateManager.dispose(); + }); + test('repeated rename tool calls each apply their requested title', async () => { let renameCalls = 0; const accessor = createAccessor({ @@ -687,11 +721,11 @@ suite('SessionServerTools', () => { const stateManager = store.add(new AgentHostStateManager(new NullLogService())); const sessions = [sessionMeta('s1', SessionStatus.Idle, workspace)]; const withCtx = createSessionServerToolGroup(createAccessor({ listSessions: async () => sessions, getChatContext: async () => snapshot })); - const live = JSON.parse(await withCtx.execute(stateManager, 'copilot:/caller', SessionServerToolName.GetSessionContext, { session: 'copilot:/s1' })); + const live = JSON.parse(await withCtx.execute(stateManager, executionContext('copilot:/caller'), SessionServerToolName.GetSessionContext, { session: 'copilot:/s1' })); assert.strictEqual(live.transcript.length, 2); const cold = createSessionServerToolGroup(createAccessor({ listSessions: async () => sessions, getChatContext: async () => undefined })); - assert.deepStrictEqual(JSON.parse(await cold.execute(stateManager, 'copilot:/caller', SessionServerToolName.GetSessionContext, { session: 'copilot:/s1' })), { + assert.deepStrictEqual(JSON.parse(await cold.execute(stateManager, executionContext('copilot:/caller'), SessionServerToolName.GetSessionContext, { session: 'copilot:/s1' })), { session: 'copilot:/s1', openLink: 'agent-host-session://copilot/s1', detail: 'summary', transcript: [], hasMoreHistory: false, truncated: false, }); store.dispose(); @@ -711,7 +745,7 @@ suite('SessionServerTools', () => { const group = createSessionServerToolGroup(createAccessor({ listSessions: async () => [sessionMeta('s1', SessionStatus.Idle, workspace)] })); // Tool call runs on the session's default chat channel; the tool resolves the owning session. const chatChannel = buildDefaultChatUri('copilot:/s1'); - const text = await group.execute(stateManager, chatChannel, SessionServerToolName.GetCurrentSession, {}); + const text = await group.execute(stateManager, { sessionUri: 'copilot:/s1', chatUri: chatChannel }, SessionServerToolName.GetCurrentSession, {}); const parsed = JSON.parse(text); assert.strictEqual(parsed.session, 'copilot:/s1'); assert.strictEqual(parsed.openLink, 'agent-host-session://copilot/s1'); From d02bfbbc5cebbd342d9c5bbc240ffe3edd5f4043 Mon Sep 17 00:00:00 2001 From: Bhavya U Date: Mon, 17 Aug 2026 14:11:41 -0700 Subject: [PATCH 14/19] Show Anthropic refusals correctly (#331315) * Fix Anthropic refusal error handling Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Handle refusal edge cases Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Simplify refusal error handling Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Avoid logging refusal explanations Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../extension/intents/node/toolCallingLoop.ts | 1 + .../node/toolCallingLoopAutopilot.spec.ts | 5 ++ .../extension/prompt/node/chatMLFetcher.ts | 7 ++ .../node/defaultIntentRequestHandler.ts | 6 ++ .../src/extension/xtab/node/xtabProvider.ts | 1 + .../src/platform/chat/common/commonTypes.ts | 11 +++ .../src/platform/endpoint/node/messagesApi.ts | 50 ++++++------ .../endpoint/test/node/messagesApi.spec.ts | 77 +++++++++++++++++-- .../src/platform/networking/common/openai.ts | 5 ++ 9 files changed, 129 insertions(+), 34 deletions(-) diff --git a/extensions/copilot/src/extension/intents/node/toolCallingLoop.ts b/extensions/copilot/src/extension/intents/node/toolCallingLoop.ts index ca06839833225c..9f770c47c259ec 100644 --- a/extensions/copilot/src/extension/intents/node/toolCallingLoop.ts +++ b/extensions/copilot/src/extension/intents/node/toolCallingLoop.ts @@ -994,6 +994,7 @@ export abstract class ToolCallingLoop { expect(loop.testShouldAutoRetry(mockResponse(ChatFetchResponseType.OffTopic))).toBe(false); }); + it('should not retry on Refusal', () => { + const loop = createLoop('autopilot'); + expect(loop.testShouldAutoRetry(mockResponse(ChatFetchResponseType.Refusal))).toBe(false); + }); + it('should not retry on Success', () => { const loop = createLoop('autoApprove'); expect(loop.testShouldAutoRetry(mockResponse(ChatFetchResponseType.Success))).toBe(false); diff --git a/extensions/copilot/src/extension/prompt/node/chatMLFetcher.ts b/extensions/copilot/src/extension/prompt/node/chatMLFetcher.ts index fe61da7906416f..73e58be2f1ecab 100644 --- a/extensions/copilot/src/extension/prompt/node/chatMLFetcher.ts +++ b/extensions/copilot/src/extension/prompt/node/chatMLFetcher.ts @@ -1918,6 +1918,13 @@ export class ChatMLFetcherImpl extends AbstractChatMLFetcher { requestId: requestId, serverRequestId: result.requestId.headerRequestId, }; + case FinishedCompletionReason.Refusal: + return { + type: ChatFetchResponseType.Refusal, + reason: 'Model declined to respond.', + requestId: requestId, + serverRequestId: result.requestId.headerRequestId, + }; case FinishedCompletionReason.Length: return { type: ChatFetchResponseType.Length, diff --git a/extensions/copilot/src/extension/prompt/node/defaultIntentRequestHandler.ts b/extensions/copilot/src/extension/prompt/node/defaultIntentRequestHandler.ts index 0e1b673b29135f..4793fbb30728e2 100644 --- a/extensions/copilot/src/extension/prompt/node/defaultIntentRequestHandler.ts +++ b/extensions/copilot/src/extension/prompt/node/defaultIntentRequestHandler.ts @@ -544,6 +544,12 @@ export class DefaultIntentRequestHandler { this.turn.setResponse(TurnStatus.Filtered, undefined, baseModelTelemetry.properties.messageId, chatResult); return chatResult; } + case ChatFetchResponseType.Refusal: { + const errorDetails = await this.getErrorDetails(fetchResult); + const chatResult = { errorDetails, metadata: metadataFragment }; + this.turn.setResponse(TurnStatus.Filtered, undefined, baseModelTelemetry.properties.messageId, chatResult); + return chatResult; + } case ChatFetchResponseType.PromptFiltered: { const errorDetails = await this.getErrorDetails(fetchResult); const chatResult = { errorDetails, metadata: { ...metadataFragment, filterReason: FilterReason.Prompt } }; diff --git a/extensions/copilot/src/extension/xtab/node/xtabProvider.ts b/extensions/copilot/src/extension/xtab/node/xtabProvider.ts index 196594e24197ee..daf5b42d5605ee 100644 --- a/extensions/copilot/src/extension/xtab/node/xtabProvider.ts +++ b/extensions/copilot/src/extension/xtab/node/xtabProvider.ts @@ -1759,6 +1759,7 @@ export function mapChatFetcherErrorToNoNextEditReason(fetchError: ChatFetchError case ChatFetchResponseType.OffTopic: case ChatFetchResponseType.Filtered: case ChatFetchResponseType.PromptFiltered: + case ChatFetchResponseType.Refusal: case ChatFetchResponseType.Length: case ChatFetchResponseType.RateLimited: case ChatFetchResponseType.QuotaExceeded: diff --git a/extensions/copilot/src/platform/chat/common/commonTypes.ts b/extensions/copilot/src/platform/chat/common/commonTypes.ts index d3a200abfbb5f0..c21c21f43507a4 100644 --- a/extensions/copilot/src/platform/chat/common/commonTypes.ts +++ b/extensions/copilot/src/platform/chat/common/commonTypes.ts @@ -98,6 +98,7 @@ export enum ChatFetchResponseType { Filtered = 'filtered', FilteredRetry = 'filteredRetry', PromptFiltered = 'promptFiltered', + Refusal = 'refusal', Length = 'length', RateLimited = 'rateLimited', QuotaExceeded = 'quotaExceeded', @@ -142,6 +143,10 @@ export type ChatFetchError = * We requested conversation, but the prompt was filtered by RAI. */ | { type: ChatFetchResponseType.PromptFiltered; reason: string; reasonDetail?: string; category: FilterReason; requestId: string; serverRequestId: string | undefined } + /** + * We requested conversation, but the model declined to answer. + */ + | { type: ChatFetchResponseType.Refusal; reason: string; reasonDetail?: string; requestId: string; serverRequestId: string | undefined } /** * We requested conversation, but the response was too long. */ @@ -452,6 +457,12 @@ function getErrorDetailsFromChatFetchErrorInner(fetchResult: ChatFetchError, cop level: ChatErrorLevel.Info, }; break; + case ChatFetchResponseType.Refusal: + details = { + message: l10n.t(`Sorry, the model declined to complete this request. Please rephrase your prompt.`), + level: ChatErrorLevel.Info, + }; + break; case ChatFetchResponseType.AgentUnauthorized: details = { message: l10n.t(`Sorry, something went wrong.`) }; break; diff --git a/extensions/copilot/src/platform/endpoint/node/messagesApi.ts b/extensions/copilot/src/platform/endpoint/node/messagesApi.ts index 109d8d622e487e..a5ba356868ccc6 100644 --- a/extensions/copilot/src/platform/endpoint/node/messagesApi.ts +++ b/extensions/copilot/src/platform/endpoint/node/messagesApi.ts @@ -3,7 +3,7 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import { ContentBlockParam, DocumentBlockParam, ImageBlockParam, MessageParam, RedactedThinkingBlockParam, TextBlockParam, ThinkingBlockParam, ToolReferenceBlockParam, ToolResultBlockParam } from '@anthropic-ai/sdk/resources'; +import { ContentBlockParam, DocumentBlockParam, ImageBlockParam, MessageParam, RedactedThinkingBlockParam, RefusalStopDetails, TextBlockParam, ThinkingBlockParam, ToolReferenceBlockParam, ToolResultBlockParam } from '@anthropic-ai/sdk/resources'; import { Raw } from '@vscode/prompt-tsx'; import { Response } from '../../../platform/networking/common/fetcherService'; import { AsyncIterableObject } from '../../../util/vs/base/common/async'; @@ -137,11 +137,7 @@ interface AnthropicStreamEvent { signature?: string; stop_reason?: string; stop_sequence?: string; - stop_details?: { - category?: string; - explanation?: string; - type?: string; - }; + stop_details?: RefusalStopDetails | null; }; copilot_annotations?: { IPCodeCitations?: AnthropicIPCodeCitation[]; @@ -790,7 +786,7 @@ interface AnthropicCompletionState { function mapStopReason(stopReason: string | null | undefined): FinishedCompletionReason { switch (stopReason) { case 'refusal': - return FinishedCompletionReason.ClientDone; + return FinishedCompletionReason.Refusal; case 'max_tokens': case 'model_context_window_exceeded': return FinishedCompletionReason.Length; @@ -889,6 +885,7 @@ type AnthropicNonStreamingResponse = )[]; model: string; stop_reason: string | null; + stop_details?: RefusalStopDetails | null; usage: { input_tokens: number; output_tokens: number; @@ -987,23 +984,9 @@ export async function processNonStreamingResponseFromMessagesEndpoint( } } - // Report text and tool calls to finishedCb so callers that rely on - // the callback (e.g. for OTEL tracing, progress, langModelServer SSE - // forwarding) see the complete response — matching the streaming path. - const delta: IResponseDelta = { - text: textContent, - ...(toolCalls.length > 0 ? { - copilotToolCalls: toolCalls.map(tc => ({ - id: tc.id, - name: tc.name, - arguments: tc.arguments, - })), - } : {}), - }; - await finishCallback(textContent, 0, delta); - if (parsed.stop_reason === 'refusal') { - logService.warn(`[messagesAPI] non-streaming: Refusal received for model ${parsed.model}`); + const category = parsed.stop_details?.category ?? 'unknown'; + logService.warn(`[messagesAPI] non-streaming: Refusal received: category='${category}' for model ${parsed.model}`); /* __GDPR__ "messagesApi.refusal" : { @@ -1018,11 +1001,24 @@ export async function processNonStreamingResponseFromMessagesEndpoint( { requestId, model: parsed.model, - category: 'unknown', + category, } ); } + // There are no incremental deltas here, so callback-only consumers need the whole response. + const delta: IResponseDelta = { + text: textContent, + ...(toolCalls.length > 0 ? { + copilotToolCalls: toolCalls.map(tc => ({ + id: tc.id, + name: tc.name, + arguments: tc.arguments, + })), + } : {}), + }; + await finishCallback(textContent, 0, delta); + const usage = parsed.usage; const completion = buildAnthropicCompletion({ model: parsed.model, @@ -1091,7 +1087,7 @@ export class AnthropicMessagesProcessor { private copilotUsage?: { total_nano_aiu: number }; private contextManagementResponse?: ContextManagementResponse; private stopReason: string | undefined; - private stopDetails?: { category?: string; explanation?: string; type?: string }; + private stopDetails?: RefusalStopDetails; constructor( private readonly telemetryData: TelemetryData, @@ -1292,7 +1288,7 @@ export class AnthropicMessagesProcessor { if (chunk.context_management) { this.contextManagementResponse = chunk.context_management; // Report context management via delta so it gets logged to request logger - return onProgress({ + onProgress({ text: '', contextManagement: chunk.context_management }); @@ -1404,5 +1400,3 @@ export class AnthropicMessagesProcessor { } } } - - diff --git a/extensions/copilot/src/platform/endpoint/test/node/messagesApi.spec.ts b/extensions/copilot/src/platform/endpoint/test/node/messagesApi.spec.ts index bf22a589647a82..5ff085661f7d0e 100644 --- a/extensions/copilot/src/platform/endpoint/test/node/messagesApi.spec.ts +++ b/extensions/copilot/src/platform/endpoint/test/node/messagesApi.spec.ts @@ -23,6 +23,14 @@ import { ConfigKey, IConfigurationService } from '../../../configuration/common/ import { IExperimentationService } from '../../../telemetry/common/nullExperimentationService'; import { InMemoryConfigurationService } from '../../../configuration/test/common/inMemoryConfigurationService'; +class RecordingLogService extends TestLogService { + readonly warnings: string[] = []; + + override warn(message: string): void { + this.warnings.push(message); + } +} + function assertContentArray(content: MessageParam['content']): ContentBlockParam[] { expect(Array.isArray(content)).toBe(true); return content as ContentBlockParam[]; @@ -1972,7 +1980,8 @@ suite('processNonStreamingResponseFromMessagesEndpoint', () => { expect(results[0].message.content).toHaveLength(0); }); - test('maps refusal stop_reason to ClientDone', async () => { + test('maps refusal stop_reason to Refusal, keeping any text the model did produce', async () => { + const explanation = 'API integrators: configure a fallback model.\n'; const response = createNonStreamingResponse({ id: 'msg_refusal', type: 'message', @@ -1980,21 +1989,34 @@ suite('processNonStreamingResponseFromMessagesEndpoint', () => { content: [{ type: 'text', text: 'refused' }], model: 'claude-sonnet-4-20250514', stop_reason: 'refusal', + stop_details: { type: 'refusal', category: 'cyber', explanation }, usage: { input_tokens: 10, output_tokens: 5 }, }); const telemetryData = TelemetryData.createAndMarkAsIssued(); + const deltas: IResponseDelta[] = []; + const logService = new RecordingLogService(); const completions = await processNonStreamingResponseFromMessagesEndpoint( new NullTelemetryService(), - new TestLogService(), + logService, response, - async () => undefined, + async (_text, _idx, delta) => { deltas.push(delta); return undefined; }, telemetryData, ); const results = []; for await (const c of completions) { results.push(c); } - expect(results[0].finishReason).toBe('DONE'); + expect({ + finishReason: results[0].finishReason, + content: results[0].message.content, + copilotErrors: deltas.flatMap(d => d.copilotErrors ?? []), + loggedExplanation: logService.warnings.some(message => message.includes(explanation)), + }).toEqual({ + finishReason: 'refusal', + content: [{ type: Raw.ChatCompletionContentPartKind.Text, text: 'refused' }], + copilotErrors: [], + loggedExplanation: false, + }); }); test('reports tool calls through finishCallback delta', async () => { @@ -2084,13 +2106,13 @@ suite('processResponseFromMessagesEndpoint routing', () => { }); suite('AnthropicMessagesProcessor streaming cache_creation', () => { - function makeProcessor(): AnthropicMessagesProcessor { + function makeProcessor(logService: TestLogService = new TestLogService()): AnthropicMessagesProcessor { return new AnthropicMessagesProcessor( TelemetryData.createAndMarkAsIssued(), 'req-1', 'gh-req-1', '', - new TestLogService(), + logService, new NullTelemetryService(), ); } @@ -2271,4 +2293,47 @@ suite('AnthropicMessagesProcessor streaming cache_creation', () => { expect(completion!.usage?.completion_tokens).toBe(2024); expect(completion!.usage?.completion_tokens_details?.reasoning_tokens).toBe(639); }); + + test('refusal stop_reason maps to Refusal even when it arrives alongside context management', () => { + const logService = new RecordingLogService(); + const processor = makeProcessor(logService); + const deltas: IResponseDelta[] = []; + const capture: FinishedCallback = async (_text, _idx, delta) => { deltas.push(delta); return undefined; }; + const explanation = 'API integrators: configure a fallback model.\n'; + + processor.push({ + type: 'message_start', + message: { + id: 'msg_refusal_stream', + type: 'message', + role: 'assistant', + content: [], + model: 'claude-sonnet-4-20250514', + stop_reason: null, + stop_sequence: null, + usage: { input_tokens: 5, output_tokens: 0 }, + }, + }, capture); + + processor.push({ + type: 'message_delta', + delta: { type: 'message_delta', stop_reason: 'refusal', stop_details: { type: 'refusal', category: 'cyber', explanation } }, + usage: { output_tokens: 0, input_tokens: 5 }, + context_management: { applied_edits: [] }, + }, capture); + + const completion = processor.push({ type: 'message_stop' }, capture); + + expect({ + finishReason: completion!.finishReason, + contextManagement: deltas.find(d => d.contextManagement)?.contextManagement, + copilotErrors: deltas.flatMap(d => d.copilotErrors ?? []), + loggedExplanation: logService.warnings.some(message => message.includes(explanation)), + }).toEqual({ + finishReason: 'refusal', + contextManagement: { applied_edits: [] }, + copilotErrors: [], + loggedExplanation: false, + }); + }); }); diff --git a/extensions/copilot/src/platform/networking/common/openai.ts b/extensions/copilot/src/platform/networking/common/openai.ts index 33faf49031931a..ccebb5cd4998cd 100644 --- a/extensions/copilot/src/platform/networking/common/openai.ts +++ b/extensions/copilot/src/platform/networking/common/openai.ts @@ -249,6 +249,11 @@ export enum FinishedCompletionReason { * Reason generated by the server. See https://platform.openai.com/docs/guides/gpt/chat-completions-api */ ContentFilter = 'content_filter', + /** + * Reason generated by the server. The model itself declined, as opposed to {@link ContentFilter} + * where a separate system blocked the response. + */ + Refusal = 'refusal', /** * Reason generated by the server (CAPI). Happens when the stream cannot be completed and the server must terminate the response. */ From 89634ad2a7096e5b2737805bd3baeda4a0d8b497 Mon Sep 17 00:00:00 2001 From: Megan Rogge Date: Mon, 17 Aug 2026 17:18:27 -0400 Subject: [PATCH 15/19] Allow dictation without Copilot entitlement (#331321) * Allow local dictation without Copilot entitlement Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d367acf7-ee13-488e-bf9f-4aef2d274190 * Allow cloud dictation without paid entitlement Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d367acf7-ee13-488e-bf9f-4aef2d274190 --------- Copilot-Session: d367acf7-ee13-488e-bf9f-4aef2d274190 --- .../speechToText/chatSpeechToTextService.ts | 8 +++----- .../test/browser/chatSpeechToTextService.test.ts | 14 ++++++++++++-- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/src/vs/workbench/contrib/chat/browser/speechToText/chatSpeechToTextService.ts b/src/vs/workbench/contrib/chat/browser/speechToText/chatSpeechToTextService.ts index 87537e9b4f9b5d..9bc53533ecc67e 100644 --- a/src/vs/workbench/contrib/chat/browser/speechToText/chatSpeechToTextService.ts +++ b/src/vs/workbench/contrib/chat/browser/speechToText/chatSpeechToTextService.ts @@ -36,7 +36,7 @@ import { IPromptsService } from '../../common/promptSyntax/service/promptsServic import { createPcmCaptureNode } from '../pcmCaptureWorklet.js'; import { getMediaCaptureWindow } from '../voiceClient/micCaptureService.js'; import { resolveDictationLanguage } from './dictationLanguage.js'; -import { ChatEntitlement, IChatEntitlementService, isProUser } from '../../../../services/chat/common/chatEntitlementService.js'; +import { ChatEntitlement, IChatEntitlementService } from '../../../../services/chat/common/chatEntitlementService.js'; export const IChatSpeechToTextService = createDecorator('chatSpeechToTextService'); @@ -135,7 +135,7 @@ const LLM_CLEANUP_MODEL_SELECTOR = { vendor: 'copilot', id: 'copilot-utility-sma type DictationBackend = 'nemo' | 'mai'; export function isDictationEntitled(entitlement: ChatEntitlement, isInternal: boolean, usesMai: boolean): boolean { - return isProUser(entitlement) && (!usesMai || entitlement !== ChatEntitlement.Enterprise || isInternal); + return !usesMai || entitlement !== ChatEntitlement.Enterprise || isInternal; } /** How long to wait for the voice websocket to connect before failing an MAI session. */ @@ -708,9 +708,7 @@ export class ChatSpeechToTextService extends Disposable implements IChatSpeechTo this._activeBackend = backend; if (!this._isEntitledForBackend(backend)) { - this._notificationService.warn(backend === 'mai' && this._chatEntitlementService.entitlement === ChatEntitlement.Enterprise - ? localize('chatStt.maiEnterpriseUnavailable', "Cloud speech-to-text is not available for GitHub Copilot Enterprise accounts.") - : localize('chatStt.requiresPaidPlan', "Dictation requires a paid GitHub Copilot plan.")); + this._notificationService.warn(localize('chatStt.maiEnterpriseUnavailable', "Cloud speech-to-text is not available for GitHub Copilot Enterprise accounts.")); return; } diff --git a/src/vs/workbench/contrib/chat/test/browser/chatSpeechToTextService.test.ts b/src/vs/workbench/contrib/chat/test/browser/chatSpeechToTextService.test.ts index b809dc81c7ac3d..23ca11babba6be 100644 --- a/src/vs/workbench/contrib/chat/test/browser/chatSpeechToTextService.test.ts +++ b/src/vs/workbench/contrib/chat/test/browser/chatSpeechToTextService.test.ts @@ -31,17 +31,27 @@ suite('ChatSpeechToTextService', () => { ensureNoDisposablesAreLeakedInTestSuite(); - test('requires a paid plan and restricts MAI for external Enterprise users', () => { + test('allows dictation without a paid plan and restricts MAI for external Enterprise users', () => { assert.deepStrictEqual({ + signedOutLocal: isDictationEntitled(ChatEntitlement.Unknown, false, false), + byokLocal: isDictationEntitled(ChatEntitlement.Unavailable, false, false), freeLocal: isDictationEntitled(ChatEntitlement.Free, false, false), proLocal: isDictationEntitled(ChatEntitlement.Pro, false, false), + signedOutMai: isDictationEntitled(ChatEntitlement.Unknown, false, true), + byokMai: isDictationEntitled(ChatEntitlement.Unavailable, false, true), + freeMai: isDictationEntitled(ChatEntitlement.Free, false, true), proMai: isDictationEntitled(ChatEntitlement.Pro, false, true), enterpriseLocal: isDictationEntitled(ChatEntitlement.Enterprise, false, false), enterpriseMai: isDictationEntitled(ChatEntitlement.Enterprise, false, true), internalEnterpriseMai: isDictationEntitled(ChatEntitlement.Enterprise, true, true), }, { - freeLocal: false, + signedOutLocal: true, + byokLocal: true, + freeLocal: true, proLocal: true, + signedOutMai: true, + byokMai: true, + freeMai: true, proMai: true, enterpriseLocal: true, enterpriseMai: false, From dee145bca3da48cef1af9938f65523122037f4af Mon Sep 17 00:00:00 2001 From: Megan Rogge Date: Mon, 17 Aug 2026 17:22:44 -0400 Subject: [PATCH 16/19] Scope dictation indicator to owning chat (#331221) * Scope dictation indicator to owning chat Fixes #331206 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d367acf7-ee13-488e-bf9f-4aef2d274190 * Track dictation editor ownership changes Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d367acf7-ee13-488e-bf9f-4aef2d274190 * Register dictation service in inline chat fixtures Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d367acf7-ee13-488e-bf9f-4aef2d274190 --------- Copilot-Session: d367acf7-ee13-488e-bf9f-4aef2d274190 --- .../contrib/chat/browser/newChatInput.ts | 25 +++++++++---- .../speechToText/dictationActionViewItem.ts | 6 ++- .../browser/speechToText/dictationSession.ts | 37 +++++++++++++++---- .../voiceInputModeActionViewItem.ts | 11 ++++-- .../browser/widget/input/chatInputPart.ts | 22 ++++++++++- .../test/browser/dictationSession.test.ts | 15 +++++++- .../chat/chatFixtureUtils.ts | 7 ++++ .../editor/inlineChatZoneWidget.fixture.ts | 7 ++++ 8 files changed, 107 insertions(+), 23 deletions(-) diff --git a/src/vs/sessions/contrib/chat/browser/newChatInput.ts b/src/vs/sessions/contrib/chat/browser/newChatInput.ts index 06a5b96bc25dc3..f9a774641275cd 100644 --- a/src/vs/sessions/contrib/chat/browser/newChatInput.ts +++ b/src/vs/sessions/contrib/chat/browser/newChatInput.ts @@ -10,7 +10,7 @@ import { StandardKeyboardEvent } from '../../../../base/browser/keyboardEvent.js import { Gesture, EventType as TouchEventType } from '../../../../base/browser/touch.js'; import { CancellationToken, CancellationTokenSource } from '../../../../base/common/cancellation.js'; import { Codicon } from '../../../../base/common/codicons.js'; -import { Emitter } from '../../../../base/common/event.js'; +import { Emitter, Event } from '../../../../base/common/event.js'; import { KeyCode, KeyMod } from '../../../../base/common/keyCodes.js'; import { Disposable, DisposableStore, MutableDisposable, thenRegisterOrDispose, toDisposable } from '../../../../base/common/lifecycle.js'; import { URI } from '../../../../base/common/uri.js'; @@ -101,14 +101,14 @@ import { handleTerminalCommandPaste, isTerminalCommandInput } from '../../../../ import { IChatPasteTargetService } from '../../../../workbench/contrib/chat/browser/chat.js'; import { NewChatInputPasteTarget } from './newChatInputPasteTarget.js'; import { getChatSessionType } from '../../../../workbench/contrib/chat/common/model/chatUri.js'; -import { ChatSpeechToTextState, DictationSettingId, IChatSpeechToTextService, isDictationActiveOnSurface } from '../../../../workbench/contrib/chat/browser/speechToText/chatSpeechToTextService.js'; +import { ChatSpeechToTextState, DictationSettingId, IChatSpeechToTextService } from '../../../../workbench/contrib/chat/browser/speechToText/chatSpeechToTextService.js'; import { setupDictationMicGlow } from '../../../../workbench/contrib/chat/browser/speechToText/dictationMicGlow.js'; import { IDictationOnboardingService } from '../../../../workbench/contrib/chat/browser/speechToText/dictationOnboarding.js'; import { ChatVoiceInputModeAction, VoiceInputModeActionViewItem } from '../../../../workbench/contrib/chat/browser/voiceInputMode/voiceInputModeActionViewItem.js'; import { IVoiceInputModeService } from '../../../../workbench/contrib/chat/browser/voiceInputMode/voiceInputMode.js'; import { toAction } from '../../../../base/common/actions.js'; import { runDictationShortcut } from '../../../../workbench/contrib/chat/browser/actions/chatSpeechToTextActions.js'; -import { notifyDictationSubmitted } from '../../../../workbench/contrib/chat/browser/speechToText/dictationSession.js'; +import { isDictationActiveForEditor, notifyDictationSubmitted, onDidChangeDictationEditor } from '../../../../workbench/contrib/chat/browser/speechToText/dictationSession.js'; import { combineVoiceInput } from '../../../../workbench/contrib/chat/browser/voiceClient/voiceInputUtils.js'; import { ChatContextKeys } from '../../../../workbench/contrib/chat/common/actions/chatContextKeys.js'; import { DictationDownloadRing, getDictationDownloadHoverMarkdown, getDictationPreparingLabel } from '../../../../workbench/contrib/chat/browser/speechToText/dictationDownloadRing.js'; @@ -1032,6 +1032,11 @@ export class NewChatInputWidget extends Disposable implements IHistoryNavigation private _createVoiceInputModePill(toolbar: HTMLElement, inputContainer: HTMLElement): void { const pillContainer = dom.append(toolbar, dom.$('.sessions-chat-voice-input-mode')); const isVoiceInputActive = derived(this, reader => isEqual(this.newChatVoiceTargetService.currentVoiceInputResource.read(reader), NEW_CHAT_VOICE_SENTINEL)); + const isDictationInputActive = observableFromEvent( + this, + Event.any(this.chatSpeechToTextService.onDidChangeState, this.chatSpeechToTextService.onDidChangePreparingModel, onDidChangeDictationEditor), + () => isDictationActiveForEditor(this._editor), + ); const isVoiceSessionActive = derived(this, reader => isNewChatVoiceSessionActive( this.voiceSessionController.isConnected.read(reader), this.voiceSessionController.isConnecting.read(reader), @@ -1050,6 +1055,7 @@ export class NewChatInputWidget extends Disposable implements IHistoryNavigation // chat widget (this composer isn't an `IChatWidget`). toggleDictation: () => { void this.toggleDictation(); }, isActive: isVoiceInputActive, + isDictationActive: isDictationInputActive, isVoiceActive: isVoiceSessionActive, })); pill.render(pillContainer); @@ -1078,6 +1084,11 @@ export class NewChatInputWidget extends Disposable implements IHistoryNavigation private _createSpeechToTextButton(container: HTMLElement, onDidChangeVisibility: (visible: boolean) => void): void { const sttService = this.chatSpeechToTextService; + const isDictationInputActive = observableFromEvent( + this, + Event.any(sttService.onDidChangeState, sttService.onDidChangePreparingModel, onDidChangeDictationEditor), + () => isDictationActiveForEditor(this._editor), + ); const button = dom.append(container, dom.$('.sessions-chat-stt-button')); button.tabIndex = 0; @@ -1089,16 +1100,16 @@ export class NewChatInputWidget extends Disposable implements IHistoryNavigation // (which invites the user to click to cancel) so this composer matches // the main chat toolbar affordance. Idle gets the richer description // naming the configured dictation model. - content: sttService.currentSurface === 'chat' && sttService.isPreparingModel + content: isDictationInputActive.get() && sttService.isPreparingModel ? getDictationDownloadHoverMarkdown(sttService) - : (isDictationActiveOnSurface(sttService, 'chat') ? stopLabel : getDictationHoverMarkdown(micLabel, this.configurationService)), + : (isDictationInputActive.get() ? stopLabel : getDictationHoverMarkdown(micLabel, this.configurationService)), position: { hoverPosition: HoverPosition.BELOW }, appearance: { showPointer: true } }))); const downloadRing = this._register(new MutableDisposable()); const renderState = () => { - const active = isDictationActiveOnSurface(sttService, 'chat'); + const active = isDictationActiveForEditor(this._editor); const preparing = active && sttService.isPreparingModel; // Only the active Recording state should read as "recording" (filled // mic). Once the user stops, the service enters Transcribing while it @@ -1135,7 +1146,7 @@ export class NewChatInputWidget extends Disposable implements IHistoryNavigation this._register(sttService.onDidChangeState(renderState)); this._register(sttService.onDidChangePreparingModel(renderState)); this._register(sttService.onDidChangeDownloadingModel(renderState)); - this._register(setupDictationMicGlow(button, sttService, this.accessibilityService, undefined, this.themeService)); + this._register(setupDictationMicGlow(button, sttService, this.accessibilityService, isDictationInputActive, this.themeService)); const updateVisibility = () => { // Mirror the `MenuId.ChatExecute` dictation gate: hide while diff --git a/src/vs/workbench/contrib/chat/browser/speechToText/dictationActionViewItem.ts b/src/vs/workbench/contrib/chat/browser/speechToText/dictationActionViewItem.ts index 868ff62a6ef117..22f2785a23641d 100644 --- a/src/vs/workbench/contrib/chat/browser/speechToText/dictationActionViewItem.ts +++ b/src/vs/workbench/contrib/chat/browser/speechToText/dictationActionViewItem.ts @@ -5,6 +5,7 @@ import { IAccessibilityService } from '../../../../../platform/accessibility/common/accessibility.js'; import { IManagedHoverContent } from '../../../../../base/browser/ui/hover/hover.js'; +import { IObservable } from '../../../../../base/common/observable.js'; import { MenuItemAction } from '../../../../../platform/actions/common/actions.js'; import { IMenuEntryActionViewItemOptions, MenuEntryActionViewItem } from '../../../../../platform/actions/browser/menuEntryActionViewItem.js'; import { ICommandService } from '../../../../../platform/commands/common/commands.js'; @@ -33,6 +34,7 @@ export class DictationActionViewItem extends MenuEntryActionViewItem { constructor( action: MenuItemAction, options: IMenuEntryActionViewItemOptions | undefined, + private readonly _isActive: IObservable, @ICommandService private readonly _commandService: ICommandService, @IConfigurationService private readonly _configurationService: IConfigurationService, @IKeybindingService keybindingService: IKeybindingService, @@ -54,11 +56,11 @@ export class DictationActionViewItem extends MenuEntryActionViewItem { () => getDictationContextMenuActions(this._commandService, this._configurationService, this._keybindingService, this._action.id), this._contextMenuService, )); - this._register(setupDictationMicGlow(container, this._speechToTextService, this._dictationAccessibilityService, undefined, this._dictationThemeService)); + this._register(setupDictationMicGlow(container, this._speechToTextService, this._dictationAccessibilityService, this._isActive, this._dictationThemeService)); } protected override getHoverContents(): IManagedHoverContent { - if (this._speechToTextService.isPreparingModel) { + if (this._isActive.get() && this._speechToTextService.isPreparingModel) { return getDictationDownloadHoverContent(this._speechToTextService); } return getDictationHoverContent(this.getTooltip() ?? '', this._configurationService); diff --git a/src/vs/workbench/contrib/chat/browser/speechToText/dictationSession.ts b/src/vs/workbench/contrib/chat/browser/speechToText/dictationSession.ts index 06ca3166e06ca9..edd0201cc59a5c 100644 --- a/src/vs/workbench/contrib/chat/browser/speechToText/dictationSession.ts +++ b/src/vs/workbench/contrib/chat/browser/speechToText/dictationSession.ts @@ -4,6 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import './media/dictationSession.css'; +import { Emitter } from '../../../../../base/common/event.js'; import { DisposableStore, toDisposable } from '../../../../../base/common/lifecycle.js'; import { ICodeEditor } from '../../../../../editor/browser/editorBrowser.js'; import { EditorOption } from '../../../../../editor/common/config/editorOptions.js'; @@ -294,6 +295,16 @@ interface IActiveDictation { * (toggle action, hold-to-talk, and the sessions composer button). */ let _active: IActiveDictation | undefined; +const _onDidChangeDictationEditor = new Emitter(); +/** Fires when the active or finalizing dictation editor changes. */ +export const onDidChangeDictationEditor = _onDidChangeDictationEditor.event; + +function setActiveDictation(active: IActiveDictation | undefined): void { + if (_active !== active) { + _active = active; + _onDidChangeDictationEditor.fire(); + } +} /** * The in-flight {@link stopDictation} finalization, if any. `stopDictation` @@ -303,6 +314,13 @@ let _active: IActiveDictation | undefined; */ let _finalizing: { readonly editor: ICodeEditor; readonly promise: Promise } | undefined; +function setFinalizingDictation(finalizing: typeof _finalizing): void { + if (_finalizing !== finalizing) { + _finalizing = finalizing; + _onDidChangeDictationEditor.fire(); + } +} + /** True while a dictation is in progress. */ export function isDictating(): boolean { return !!_active; @@ -313,6 +331,11 @@ export function activeDictationEditor(): ICodeEditor | undefined { return _active?.editor; } +/** Whether `editor` owns the active or finalizing dictation session. */ +export function isDictationActiveForEditor(editor: ICodeEditor): boolean { + return _active?.editor === editor || _finalizing?.editor === editor; +} + /** Start dictating into `editor`, rendering the transcript live. */ export async function startDictation(service: IChatSpeechToTextService, editor: ICodeEditor, window: Window & typeof globalThis, logService: ILogService, surface: ChatDictationSurface = 'chat'): Promise { // Already dictating into this exact editor: nothing to do (callers toggle @@ -406,7 +429,7 @@ export async function startDictation(service: IChatSpeechToTextService, editor: // If the service ends the session on its own (e.g. the model failed // to load and it surfaced an error), drop the stale active reference // so the toolbar and glow reflect that dictation is no longer running. - _active = undefined; + setActiveDictation(undefined); disposables.dispose(); return; } @@ -416,13 +439,13 @@ export async function startDictation(service: IChatSpeechToTextService, editor: // composer is closed); cancel dictation instead of leaving the microphone // and local transcription running against a dead editor. disposables.add(editor.onDidDispose(() => cancelDictation())); - _active = { service, editor, inserter, disposables, logService, surface }; + setActiveDictation({ service, editor, inserter, disposables, logService, surface }); try { await service.start(window, surface); } catch { // Acquisition/connection failure is surfaced by the service. if (_active?.service === service) { - _active = undefined; + setActiveDictation(undefined); } disposables.dispose(); } @@ -438,14 +461,14 @@ export async function stopDictation(): Promise { await _finalizing?.promise; return; } - _active = undefined; + setActiveDictation(undefined); const promise = finalizeDictation(active); - _finalizing = { editor: active.editor, promise }; + setFinalizingDictation({ editor: active.editor, promise }); try { await promise; } finally { if (_finalizing?.promise === promise) { - _finalizing = undefined; + setFinalizingDictation(undefined); } } } @@ -498,7 +521,7 @@ export function cancelDictation(): void { if (!active) { return; } - _active = undefined; + setActiveDictation(undefined); // Remove any live transcript already written to the editor so Escape leaves // the input exactly as it was before dictation started. active.inserter.revert(); diff --git a/src/vs/workbench/contrib/chat/browser/voiceInputMode/voiceInputModeActionViewItem.ts b/src/vs/workbench/contrib/chat/browser/voiceInputMode/voiceInputModeActionViewItem.ts index 4e1d91b7e4d231..954c4a03c003b3 100644 --- a/src/vs/workbench/contrib/chat/browser/voiceInputMode/voiceInputModeActionViewItem.ts +++ b/src/vs/workbench/contrib/chat/browser/voiceInputMode/voiceInputModeActionViewItem.ts @@ -309,6 +309,8 @@ export interface IVoiceInputModePillOptions { readonly toggleDictation?: () => void; /** Whether this is the focused or last-focused chat input that owns live state. */ readonly isActive?: IObservable; + /** Whether the dictation session belongs to this input. Defaults to `isActive`. */ + readonly isDictationActive?: IObservable; /** Whether the shared Voice Mode transport belongs to this input. */ readonly isVoiceActive?: IObservable; /** Claim Voice Mode for this host instead of targeting the last focused chat session. */ @@ -414,7 +416,7 @@ export class VoiceInputModeActionViewItem extends BaseActionViewItem { this._dictationCell.setAttribute('role', 'button'); this._dictationIcon = dom.append(this._dictationCell, dom.$('span.chat-voice-input-mode-icon')); this._register(this.hoverService.setupManagedHover(getDefaultHoverDelegate('element'), this._dictationCell, - () => this.chatSpeechToTextService.isPreparingModel + () => (this._options?.isDictationActive?.get() ?? this._options?.isActive?.get() ?? true) && this.chatSpeechToTextService.isPreparingModel ? getDictationDownloadHoverContent(this.chatSpeechToTextService) : getDictationHoverContent(this._getLabelWithKeybinding(localize('voiceInputMode.dictation', "Dictation"), DICTATION_TOGGLE_COMMAND_ID), this.configurationService))); this._register(dom.addDisposableListener(this._dictationCell, dom.EventType.CLICK, e => { @@ -427,7 +429,7 @@ export class VoiceInputModeActionViewItem extends BaseActionViewItem { () => getDictationContextMenuActions(this.commandService, this.configurationService, this.keybindingService, DICTATION_TOGGLE_COMMAND_ID), this.contextMenuService, )); - this._register(setupDictationMicGlow(this._dictationCell, this.chatSpeechToTextService, this.accessibilityService, this._options?.isActive, this.themeService)); + this._register(setupDictationMicGlow(this._dictationCell, this.chatSpeechToTextService, this.accessibilityService, this._options?.isDictationActive ?? this._options?.isActive, this.themeService)); // --- Voice cell: a single waveform that transforms across states (no glyph). --- this._voiceCell = dom.append(this._reel, dom.$('button.monaco-segmented-icon-toggle-cell.chat-voice-input-mode-cell.voice')); @@ -532,6 +534,7 @@ export class VoiceInputModeActionViewItem extends BaseActionViewItem { const handsFree = simHandsFree ?? this.voiceInputModeService.handsFree.read(reader); const sim = this.voiceInputModeService.simulatedVoiceState.read(reader); const isActive = sim !== undefined || (this._options?.isActive?.read(reader) ?? true); + const isDictationActive = sim !== undefined || (this._options?.isDictationActive?.read(reader) ?? isActive); const isVoiceActive = sim !== undefined || (this._options?.isVoiceActive?.read(reader) ?? isActive); // Resolve the effective state — a simulation override wins over live state. @@ -547,7 +550,7 @@ export class VoiceInputModeActionViewItem extends BaseActionViewItem { listening = sim === 'listening'; speaking = sim === 'speaking'; } else { - isDictating = isActive && dictationActive.read(reader); + isDictating = isDictationActive && dictationActive.read(reader); connected = isVoiceActive && this.voiceSessionController.isConnected.read(reader); // A reconnect is a connect in progress as far as this pill is concerned: // without it the pill renders its idle state while the socket is retrying. @@ -561,7 +564,7 @@ export class VoiceInputModeActionViewItem extends BaseActionViewItem { const voiceOn = connected || connecting; this._voiceLive = voiceLive; // First-use model download/load (real state only; simulations never prepare). - const dictationBusy = sim === undefined && isActive && dictationPreparing.read(reader); + const dictationBusy = sim === undefined && isDictationActive && dictationPreparing.read(reader); // The dedicated listen (start/stop speaking) toggle shows in manual // (non-hands-free) connected voice mode. In hands-free mode the auto-listen diff --git a/src/vs/workbench/contrib/chat/browser/widget/input/chatInputPart.ts b/src/vs/workbench/contrib/chat/browser/widget/input/chatInputPart.ts index 2c96df9c4a775e..85fc7c8a58237a 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/input/chatInputPart.ts +++ b/src/vs/workbench/contrib/chat/browser/widget/input/chatInputPart.ts @@ -117,8 +117,9 @@ import { ChatVoiceInputModeAction, VoiceInputModeActionViewItem } from '../../vo import { ChatSpeechToTextConnectingAction, ChatSpeechToTextPreparingAction, ToggleChatSpeechToTextAction } from '../../actions/chatSpeechToTextActions.js'; import { DictationActionViewItem } from '../../speechToText/dictationActionViewItem.js'; import { DictationDownloadActionViewItem } from '../../speechToText/dictationDownloadActionViewItem.js'; +import { ChatSpeechToTextState, IChatSpeechToTextService } from '../../speechToText/chatSpeechToTextService.js'; import { IDictationOnboardingService } from '../../speechToText/dictationOnboarding.js'; -import { notifyDictationSubmitted } from '../../speechToText/dictationSession.js'; +import { isDictationActiveForEditor, notifyDictationSubmitted, onDidChangeDictationEditor } from '../../speechToText/dictationSession.js'; import { VoiceModeActionViewItem } from '../../voiceClient/voiceModeActionViewItem.js'; import { IVoiceSessionController } from '../../voiceClient/voiceSessionController.js'; import { AgentSessionProviders, AgentSessionTarget, getAgentSessionProvider } from '../../agentSessions/agentSessions.js'; @@ -785,6 +786,7 @@ export class ChatInputPart extends Disposable implements IHistoryNavigationWidge @IChatSessionsService private readonly chatSessionsService: IChatSessionsService, @IChatContextService private readonly chatContextService: IChatContextService, @IAgentSessionsService private readonly agentSessionsService: IAgentSessionsService, + @IChatSpeechToTextService private readonly speechToTextService: IChatSpeechToTextService, @IDictationOnboardingService private readonly dictationOnboardingService: IDictationOnboardingService, @IChatInputNoticeHubService private readonly chatInputNoticeHubService: IChatInputNoticeHubService, @IWorkspaceContextService private readonly workspaceContextService: IWorkspaceContextService, @@ -3173,6 +3175,21 @@ export class ChatInputPart extends Disposable implements IHistoryNavigationWidge this.updateInputEditorFontFamily(); this._register(addDisposableListener(this._inputEditorElement, dom.EventType.PASTE, e => this.handleTerminalCommandPaste(e), true)); + const dictationRecording = ChatContextKeys.speechToTextRecording.bindTo(this.contextKeyService); + const dictationPreparing = ChatContextKeys.speechToTextPreparing.bindTo(this.contextKeyService); + const isDictationInputActive = observableFromEvent( + this, + Event.any(this.speechToTextService.onDidChangeState, this.speechToTextService.onDidChangePreparingModel, onDidChangeDictationEditor), + () => isDictationActiveForEditor(this._inputEditor), + ); + const updateDictationContextKeys = () => { + const active = isDictationActiveForEditor(this._inputEditor); + dictationRecording.set(active && this.speechToTextService.state === ChatSpeechToTextState.Recording); + dictationPreparing.set(active && this.speechToTextService.isPreparingModel); + }; + this._register(Event.any(this.speechToTextService.onDidChangeState, this.speechToTextService.onDidChangePreparingModel, onDidChangeDictationEditor)(updateDictationContextKeys)); + updateDictationContextKeys(); + SuggestController.get(this._inputEditor)?.forceRenderingAbove(); options.overflowWidgetsDomNode?.classList.add('hideSuggestTextIcons'); this._inputEditorElement.classList.add('hideSuggestTextIcons'); @@ -3411,6 +3428,7 @@ export class ChatInputPart extends Disposable implements IHistoryNavigationWidge if (action.id === ChatVoiceInputModeAction.ID) { return this.instantiationService.createInstance(VoiceInputModeActionViewItem, action, { isActive: isVoiceInputActive, + isDictationActive: isDictationInputActive, isVoiceActive: isVoiceSessionActive, activateVoiceMode: isOmniInput ? () => { this.voiceSessionController.takeOmniInputOwnership(dom.getWindow(toolbarsContainer)); @@ -3433,7 +3451,7 @@ export class ChatInputPart extends Disposable implements IHistoryNavigationWidge return this.instantiationService.createInstance(DictationDownloadActionViewItem, action, options); } if (action.id === ToggleChatSpeechToTextAction.ID && action instanceof MenuItemAction) { - return this.instantiationService.createInstance(DictationActionViewItem, action, options); + return this.instantiationService.createInstance(DictationActionViewItem, action, options, isDictationInputActive); } // Voice Mode mic button: add a right-click context menu (Select // Microphone / Disable Voice Mode) mirroring dictation. While diff --git a/src/vs/workbench/contrib/chat/test/browser/dictationSession.test.ts b/src/vs/workbench/contrib/chat/test/browser/dictationSession.test.ts index df03d8c855a27a..02e14783e8d525 100644 --- a/src/vs/workbench/contrib/chat/test/browser/dictationSession.test.ts +++ b/src/vs/workbench/contrib/chat/test/browser/dictationSession.test.ts @@ -15,7 +15,7 @@ import { createTestCodeEditor } from '../../../../../editor/test/browser/testCod import { createTextModel } from '../../../../../editor/test/common/testTextModel.js'; import { NullLogService } from '../../../../../platform/log/common/log.js'; import { ChatDictationSurface, ChatSpeechToTextState, IChatDictationTranscript, IChatSpeechToTextService, isDictationActiveOnSurface } from '../../browser/speechToText/chatSpeechToTextService.js'; -import { isDictating, startDictation, stopDictation, stopDictationForEditor } from '../../browser/speechToText/dictationSession.js'; +import { isDictating, isDictationActiveForEditor, onDidChangeDictationEditor, startDictation, stopDictation, stopDictationForEditor } from '../../browser/speechToText/dictationSession.js'; suite('DictationSession', () => { @@ -145,13 +145,18 @@ suite('DictationSession', () => { const { service, onDidUpdateTranscript, blockStop } = createService('hello world', true); const model = store.add(createTextModel('')); const editor = store.add(createTestCodeEditor(model)); + const otherEditor = store.add(createTestCodeEditor(store.add(createTextModel('')))); await startDictation(service, editor, mainWindow, new NullLogService()); onDidUpdateTranscript.fire({ text: 'hello world', finalizedText: '' }); + const ownershipChanges: boolean[] = []; + store.add(onDidChangeDictationEditor(() => ownershipChanges.push(isDictationActiveForEditor(editor)))); // The first submit begins finalizing but blocks inside stopAndTranscribe. const release = blockStop(); const firstStop = stopDictationForEditor(editor); + const ownsFinalizingDictation = isDictationActiveForEditor(editor); + const otherOwnsFinalizingDictation = isDictationActiveForEditor(otherEditor); // A second submit for the same editor arrives mid-finalization; it must // await the in-flight finalization rather than returning early. let secondResolved = false; @@ -163,12 +168,20 @@ suite('DictationSession', () => { await Promise.all([firstStop, secondStop]); assert.deepStrictEqual({ + ownsFinalizingDictation, + otherOwnsFinalizingDictation, + ownershipChanges, secondResolvedWhileBlocked, secondResolvedAfterFinal: secondResolved, + ownsCompletedDictation: isDictationActiveForEditor(editor), value: editor.getValue(), }, { + ownsFinalizingDictation: true, + otherOwnsFinalizingDictation: false, + ownershipChanges: [false, true, false], secondResolvedWhileBlocked: false, secondResolvedAfterFinal: true, + ownsCompletedDictation: false, value: 'hello world', }); }); diff --git a/src/vs/workbench/test/browser/componentFixtures/chat/chatFixtureUtils.ts b/src/vs/workbench/test/browser/componentFixtures/chat/chatFixtureUtils.ts index 59edb07dd6d270..f3f5f0df793df0 100644 --- a/src/vs/workbench/test/browser/componentFixtures/chat/chatFixtureUtils.ts +++ b/src/vs/workbench/test/browser/componentFixtures/chat/chatFixtureUtils.ts @@ -58,6 +58,7 @@ import { IChatContextPickService } from '../../../../contrib/chat/browser/attach import { IChatContextService } from '../../../../contrib/chat/browser/contextContrib/chatContextService.js'; import { IChatImageCarouselService } from '../../../../contrib/chat/browser/chatImageCarouselService.js'; import { IChatInputNotification, IChatInputNotificationService } from '../../../../contrib/chat/browser/widget/input/chatInputNotificationService.js'; +import { ChatSpeechToTextState, IChatSpeechToTextService } from '../../../../contrib/chat/browser/speechToText/chatSpeechToTextService.js'; import { IDictationOnboardingService } from '../../../../contrib/chat/browser/speechToText/dictationOnboarding.js'; import { IChatInputNoticeHubService } from '../../../../contrib/chat/browser/widget/input/chatInputNoticeHub.js'; import { ChatSubmitRequestHandlerService, IChatSubmitRequestHandlerService } from '../../../../contrib/chat/browser/chatSubmitRequestHandlerService.js'; @@ -235,6 +236,12 @@ export function registerChatFixtureServices(reg: ServiceRegistration, options: I override readonly isVisible = false; override registerHost() { return Disposable.None; } }()); + reg.defineInstance(IChatSpeechToTextService, new class extends mock() { + override readonly onDidChangeState = Event.None; + override readonly onDidChangePreparingModel = Event.None; + override readonly state = ChatSpeechToTextState.Idle; + override readonly isPreparingModel = false; + }()); reg.defineInstance(IVoiceModeOnboardingService, new class extends mock() { override readonly isVisible = false; override registerHost() { return Disposable.None; } diff --git a/src/vs/workbench/test/browser/componentFixtures/editor/inlineChatZoneWidget.fixture.ts b/src/vs/workbench/test/browser/componentFixtures/editor/inlineChatZoneWidget.fixture.ts index 716ffe17e8f5c3..59c8921f7c0fb2 100644 --- a/src/vs/workbench/test/browser/componentFixtures/editor/inlineChatZoneWidget.fixture.ts +++ b/src/vs/workbench/test/browser/componentFixtures/editor/inlineChatZoneWidget.fixture.ts @@ -29,6 +29,7 @@ import { ChatEntitlement, IChatEntitlementService } from '../../../../services/c import { IVoiceModeOnboardingService } from '../../../../contrib/agentsVoice/browser/voiceModeOnboarding.js'; import { IChatInputNotificationService } from '../../../../contrib/chat/browser/widget/input/chatInputNotificationService.js'; import { IDictationOnboardingService } from '../../../../contrib/chat/browser/speechToText/dictationOnboarding.js'; +import { ChatSpeechToTextState, IChatSpeechToTextService } from '../../../../contrib/chat/browser/speechToText/chatSpeechToTextService.js'; import { IChatInputNoticeHubService } from '../../../../contrib/chat/browser/widget/input/chatInputNoticeHub.js'; import { IPathService } from '../../../../services/path/common/pathService.js'; import { IChatWidgetService, IChatAccessibilityService } from '../../../../contrib/chat/browser/chat.js'; @@ -203,6 +204,12 @@ function renderInlineChatZoneWidget({ container, disposableStore, theme }: Compo override acceptRequest() { } override acceptResponse() { } }()); + reg.defineInstance(IChatSpeechToTextService, new class extends mock() { + override readonly onDidChangeState = Event.None; + override readonly onDidChangePreparingModel = Event.None; + override readonly state = ChatSpeechToTextState.Idle; + override readonly isPreparingModel = false; + }()); reg.defineInstance(IChatSlashCommandService, new class extends mock() { override readonly onDidChangeCommands = Event.None; override getCommands() { return []; } From a85c9b1d5588d5c3a593fcb3876365eba666f3d4 Mon Sep 17 00:00:00 2001 From: Justin Chen <54879025+justschen@users.noreply.github.com> Date: Mon, 17 Aug 2026 14:24:52 -0700 Subject: [PATCH 17/19] fix chat jump on pet window blur (#331278) --- .../chat/browser/widget/chatPetWidget.ts | 6 ++++- .../contrib/chat/browser/widget/chatWidget.ts | 12 +++------- .../test/browser/widget/chatPetWidget.test.ts | 24 +++++++++---------- 3 files changed, 20 insertions(+), 22 deletions(-) diff --git a/src/vs/workbench/contrib/chat/browser/widget/chatPetWidget.ts b/src/vs/workbench/contrib/chat/browser/widget/chatPetWidget.ts index 1d99011cd189e1..bc20099291d8f2 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/chatPetWidget.ts +++ b/src/vs/workbench/contrib/chat/browser/widget/chatPetWidget.ts @@ -443,8 +443,12 @@ export function getChatPetBaseState(hasActiveRequest: boolean, needsInput: boole return 'idle'; } +export function shouldReserveChatPetSpace(enabled: boolean, isLatestFocusedWidget: boolean): boolean { + return enabled && isLatestFocusedWidget; +} + export function isChatPetVisible(enabled: boolean, isLatestFocusedWidget: boolean, windowFocused = true): boolean { - return enabled && isLatestFocusedWidget && windowFocused; + return shouldReserveChatPetSpace(enabled, isLatestFocusedWidget) && windowFocused; } export function isChatPetWindowActive(applicationFocused: boolean, activeWindowId: number, targetWindowId: number): boolean { diff --git a/src/vs/workbench/contrib/chat/browser/widget/chatWidget.ts b/src/vs/workbench/contrib/chat/browser/widget/chatWidget.ts index c24468b0697d1d..ea7aa4058aaa20 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/chatWidget.ts +++ b/src/vs/workbench/contrib/chat/browser/widget/chatWidget.ts @@ -51,7 +51,6 @@ import { IThemeService } from '../../../../../platform/theme/common/themeService import { SaveReason } from '../../../../common/editor.js'; import { ChatEntitlementContextKeys, IChatEntitlementService } from '../../../../services/chat/common/chatEntitlementService.js'; import { IEditorService } from '../../../../services/editor/common/editorService.js'; -import { IHostService } from '../../../../services/host/browser/host.js'; import { ILifecycleService } from '../../../../services/lifecycle/common/lifecycle.js'; import { checkModeOption } from '../../common/chat.js'; import { IChatAgentAttachmentCapabilities, IChatAgentCommand, IChatAgentData, IChatAgentService } from '../../common/participants/chatAgents.js'; @@ -101,7 +100,7 @@ import { getChatSessionType } from '../../common/model/chatUri.js'; import { ICustomizationHarnessService } from '../../common/customizationHarnessService.js'; import { CHAT_READ_ONLY_BANNER_HEIGHT, ChatReadOnlyBanner } from './chatReadOnlyBanner.js'; import { IChatSubmitRequestHandlerService } from '../chatSubmitRequestHandlerService.js'; -import { ChatPetWidget, isChatPetVisible, isChatPetWindowActive } from './chatPetWidget.js'; +import { ChatPetWidget, shouldReserveChatPetSpace } from './chatPetWidget.js'; import { IChatPetService } from '../chatPetService.js'; import { stopDictationForEditor } from '../speechToText/dictationSession.js'; import { ChatContentMarkdownRenderer } from './chatContentMarkdownRenderer.js'; @@ -536,7 +535,6 @@ export class ChatWidget extends Disposable implements IChatWidget { @IChatSubmitRequestHandlerService private readonly chatSubmitRequestHandlerService: IChatSubmitRequestHandlerService, @IChatPetService private readonly chatPetService: IChatPetService, @IAgentHostService private readonly _agentHostService: IAgentHostService, - @IHostService private readonly hostService: IHostService, ) { super(); @@ -958,18 +956,14 @@ export class ChatWidget extends Disposable implements IChatWidget { const petHost = this.inputPart.element; const inputHasContent = observableFromEvent(this, this.inputEditor.onDidChangeModelContent, () => this.inputEditor.getValue().length > 0); const targetWindow = dom.getWindow(this.container); - const targetWindowId = dom.getWindowId(targetWindow); const isLatestFocusedWidgetInWindow = observableValue(this, this.chatWidgetService.lastFocusedWidget === this); - const applicationFocused = observableFromEvent(this, this.hostService.onDidChangeFocus, () => this.hostService.hasFocus); - const activeWindowId = observableFromEvent(this, this.hostService.onDidChangeActiveWindow, windowId => windowId ?? dom.getWindowId(dom.getActiveWindow())); - const windowFocused = derived(this, reader => isChatPetWindowActive(applicationFocused.read(reader), activeWindowId.read(reader), targetWindowId)); this._register(this.chatWidgetService.onDidChangeFocusedWidget(focusedWidget => { if (focusedWidget && dom.getWindow(focusedWidget.domNode) === targetWindow) { isLatestFocusedWidgetInWindow.set(focusedWidget === this, undefined); } })); - const petVisible = derived(this, reader => isChatPetVisible(this.chatPetService.enabled.read(reader), isLatestFocusedWidgetInWindow.read(reader), windowFocused.read(reader))); - this._register(autorun(reader => this.container.classList.toggle('chat-pet-enabled', petVisible.read(reader)))); + const petSpaceReserved = derived(this, reader => shouldReserveChatPetSpace(this.chatPetService.enabled.read(reader), isLatestFocusedWidgetInWindow.read(reader))); + this._register(autorun(reader => this.container.classList.toggle('chat-pet-enabled', petSpaceReserved.read(reader)))); const petWidget = this._register(this.instantiationService.createInstance(ChatPetWidget, petHost, inputContainer ?? petHost, petMovementBounds ?? parent, this._viewModelObs.map(viewModel => viewModel?.model), inputHasContent, isLatestFocusedWidgetInWindow, this.inputEditor.onDidChangeModelContent)); petWidget.setPlatformTopProvider(() => this.inputPart.getChatPetPlatformTop()); } diff --git a/src/vs/workbench/contrib/chat/test/browser/widget/chatPetWidget.test.ts b/src/vs/workbench/contrib/chat/test/browser/widget/chatPetWidget.test.ts index 165c644836e058..bd8e65f607f1c1 100644 --- a/src/vs/workbench/contrib/chat/test/browser/widget/chatPetWidget.test.ts +++ b/src/vs/workbench/contrib/chat/test/browser/widget/chatPetWidget.test.ts @@ -9,7 +9,7 @@ import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../../base/ import { NullTelemetryServiceShape } from '../../../../../../platform/telemetry/common/telemetryUtils.js'; import { TestStorageService } from '../../../../../test/common/workbenchTestServices.js'; import { ChatPetService, getChatPetVariant } from '../../../browser/chatPetService.js'; -import { CHAT_PET_CONFIRMATION_ATTENTION_DURATION, CHAT_PET_ICON_TRANSFORMATION_CHANCE, CHAT_PET_IDLE_SLEEP_DELAY, CHAT_PET_WALL_IMPACT_DURATION, CHAT_PET_YAPPING_CHANCE, ChatPetBlinkController, ChatPetDirectionChangeController, ChatPetFacingController, ChatPetHopController, advanceChatPetThrow, doesChatPetStateBlink, doesChatPetStateTrackCursor, getChatPetAnimationFrame, getChatPetBaseState, getChatPetBlinkDelay, getChatPetBuddyName, getChatPetClickInteraction, getChatPetDefaultHorizontalPosition, getChatPetDragPosition, getChatPetFallDuration, getChatPetFallTarget, getChatPetFrameDurations, getChatPetGazeDirection, getChatPetHorizontalPosition, getChatPetPlatformTop, getChatPetRelativeHorizontalPosition, getChatPetRenderedState, getChatPetRespawnFrameDurations, getChatPetRestoredHorizontalPosition, getChatPetScale, getChatPetSpeechFrameDurations, getChatPetSpriteName, getChatPetThrowLanding, getChatPetThrowRotation, getChatPetThrowVelocity, getChatPetVerticalOffset, getChatPetWallReboundVelocity, getChatPetWideSpriteHorizontalOffset, isChatPetImageSource, isChatPetKeyboardInteractionEnabled, isChatPetVisible, isChatPetWindowActive, shouldPlaceChatPetSpeechBubbleLeft, shouldSettleChatPetThrow } from '../../../browser/widget/chatPetWidget.js'; +import { CHAT_PET_CONFIRMATION_ATTENTION_DURATION, CHAT_PET_ICON_TRANSFORMATION_CHANCE, CHAT_PET_IDLE_SLEEP_DELAY, CHAT_PET_WALL_IMPACT_DURATION, CHAT_PET_YAPPING_CHANCE, ChatPetBlinkController, ChatPetDirectionChangeController, ChatPetFacingController, ChatPetHopController, advanceChatPetThrow, doesChatPetStateBlink, doesChatPetStateTrackCursor, getChatPetAnimationFrame, getChatPetBaseState, getChatPetBlinkDelay, getChatPetBuddyName, getChatPetClickInteraction, getChatPetDefaultHorizontalPosition, getChatPetDragPosition, getChatPetFallDuration, getChatPetFallTarget, getChatPetFrameDurations, getChatPetGazeDirection, getChatPetHorizontalPosition, getChatPetPlatformTop, getChatPetRelativeHorizontalPosition, getChatPetRenderedState, getChatPetRespawnFrameDurations, getChatPetRestoredHorizontalPosition, getChatPetScale, getChatPetSpeechFrameDurations, getChatPetSpriteName, getChatPetThrowLanding, getChatPetThrowRotation, getChatPetThrowVelocity, getChatPetVerticalOffset, getChatPetWallReboundVelocity, getChatPetWideSpriteHorizontalOffset, isChatPetImageSource, isChatPetKeyboardInteractionEnabled, isChatPetVisible, isChatPetWindowActive, shouldPlaceChatPetSpeechBubbleLeft, shouldReserveChatPetSpace, shouldSettleChatPetThrow } from '../../../browser/widget/chatPetWidget.js'; suite('ChatPetWidget', () => { @@ -225,19 +225,19 @@ suite('ChatPetWidget', () => { assert.strictEqual(CHAT_PET_CONFIRMATION_ATTENTION_DURATION, 2_000); }); - test('only shows in the active window and latest focused chat widget when enabled', () => { + test('only shows in the active window but reserves space in each window\'s latest focused chat', () => { assert.deepStrictEqual([ - isChatPetVisible(false, false, false), - isChatPetVisible(false, true, true), - isChatPetVisible(true, false, true), - isChatPetVisible(true, true, false), - isChatPetVisible(true, true, true), + { visible: isChatPetVisible(false, false, false), spaceReserved: shouldReserveChatPetSpace(false, false) }, + { visible: isChatPetVisible(false, true, true), spaceReserved: shouldReserveChatPetSpace(false, true) }, + { visible: isChatPetVisible(true, false, true), spaceReserved: shouldReserveChatPetSpace(true, false) }, + { visible: isChatPetVisible(true, true, false), spaceReserved: shouldReserveChatPetSpace(true, true) }, + { visible: isChatPetVisible(true, true, true), spaceReserved: shouldReserveChatPetSpace(true, true) }, ], [ - false, - false, - false, - false, - true, + { visible: false, spaceReserved: false }, + { visible: false, spaceReserved: false }, + { visible: false, spaceReserved: false }, + { visible: false, spaceReserved: true }, + { visible: true, spaceReserved: true }, ]); }); From 52a062804df503ab0567702137a1087997e7715d Mon Sep 17 00:00:00 2001 From: Ben Villalobos Date: Mon, 17 Aug 2026 14:54:44 -0700 Subject: [PATCH 18/19] Automations: Render quick chats like workspace chats (#331306) * Render quick chats consistently in automation history * Announce quick chats in automation history * Test quick chat presentation in flat session lists * Clarify session status timestamp behavior * signing commit --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/vs/sessions/SESSIONS_LIST.md | 6 +- .../sessions/browser/views/automationsView.ts | 1 + .../sessions/browser/views/sessionsList.ts | 86 ++++++++++++------- .../test/browser/sessionsList.test.ts | 60 ++++++++++++- 4 files changed, 118 insertions(+), 35 deletions(-) diff --git a/src/vs/sessions/SESSIONS_LIST.md b/src/vs/sessions/SESSIONS_LIST.md index 0ff064584020fa..5fcc9fbae68f77 100644 --- a/src/vs/sessions/SESSIONS_LIST.md +++ b/src/vs/sessions/SESSIONS_LIST.md @@ -36,7 +36,7 @@ Each session row displays: - **Type icon** (regular sessions only) — folder/worktree/cloud icon indicating the workspace kind; omitted for quick chats - **Workspace badge** — workspace label rendered inline after the folder/worktree/cloud type icon. It is hidden only when a workspace section header already carries the same label; date, custom-group, Pinned, and Done rows show it unless live status temporarily hides row details. - **Diff stats** (regular sessions only) — `+insertions −deletions` when the session has pending changes; omitted for quick chats -- **Status description or timestamp** (regular sessions only) — InProgress/NeedsInput/Error show a status message, otherwise a relative timestamp; quick chats show none of this (their compact spinner status icon already conveys "in progress", and diff stats/timestamps are omitted for their more compact row) +- **Status description or timestamp** — InProgress and NeedsInput show a status message instead of a timestamp; Error shows both, and other terminal states show a relative timestamp. Compact quick-chat rows in the primary Sessions list omit this second row; automation history presents quick-chat-backed runs as regular history rows with timestamps. - **Approval row** (optional) — pending agent approvals with an "Allow" button Quick-chat rows (`.session-item.quick-chat`, driven by the reactive `ISession.isQuickChat` observable) are single-line entries: the details (second) row is hidden entirely and its content is never built — smaller icon, one line of title only, tighter row height (see `SessionsTreeDelegate.ITEM_HEIGHT_QUICK_CHAT`). Regular sessions keep the standard two-line row (title + details row). @@ -45,7 +45,7 @@ Continuous row animations preserve their existing appearance while limiting rend `SessionsFlatList` reuses the same session row renderer for sectionless surfaces, including the approval row and dynamic row height updates. Consumers that size their own container listen for content-height changes and relayout the list. When embedded inside another hover, consumers disable row hovers so moving over the list does not replace the parent hover. -Automation run history uses `SessionsFlatList` for runs backed by a live session. Pending and running runs without a resolved session use a lightweight `Working...` row; date grouping and run actions remain owned by the Automations view. +Automation run history uses `SessionsFlatList` for runs backed by a live session. Quick-chat-backed runs use the regular two-line history-row presentation so all run entries have consistent height and status-icon sizing; their details row shows the Chats icon, a `Chat` badge in place of a workspace label, and the timestamp while continuing to omit diff metadata. Pending and running runs without a resolved session use a lightweight `Working...` row; date grouping and run actions remain owned by the Automations view. ### Grouping @@ -59,7 +59,7 @@ Sessions are organized into sections with fixed priority: 5. Done/Archived ← always last, not reorderable ``` -The **Chats** section holds workspace-less quick-chat sessions, detected via the `isQuickChatSession(session)` helper (which reads the session's own `ISession.isQuickChat` observable — **not** `workspace === undefined`, which can be transiently undefined for workspace-bound sessions too). It renders **inside the Sessions list directly below the Pinned section** (above the workspace/date groups) in **both** grouping modes — quick chats are neither a workspace nor a date bucket, so they are partitioned out of workspace/date grouping and rendered as their own entry right after Pinned. The section is **always visible** (even with no quick chats) whenever a provider advertises `supportsQuickChats` — subject to the `sessions.list.showEmptyDefaultGroups` setting (default `true`; when `false` the empty Chats section is hidden). Both the Pinned and Chats section headers carry a **leading icon** (`Codicon.pinned` for Pinned, `Codicon.commentDiscussion` for Chats) and share the standard section-header font/styling (the two headers look consistent — no prominent top-title variant). The Chats header shows the chat icon, the label "Chats", and a **"+" New Quick Chat** action in its section toolbar (also bound to **Cmd+K Cmd+N**) — the *only* create affordance for quick chats (Cmd+N always creates a new **session**, not a quick chat; there is no quick-chat action in the top Sessions header). The bulk archive/mark-done action is not offered on the Chats section. When the section has **no quick chats**, it shows a muted, centered **"No chats" placeholder row** (a synthetic non-session list item, like the "show more" rows) instead of an empty section. A pinned quick chat still appears in Pinned (pin wins), and an archived one still goes to Done (archive wins). Quick-chat rows never show a per-row chat/PR glyph as their **status icon** (their identity is already conveyed by the Chats section, the Pinned section, or a custom group), have no type icon in the details row, and carry no workspace badge/diff stats/timestamp (see Session Row above). +The **Chats** section holds workspace-less quick-chat sessions, detected via the `isQuickChatSession(session)` helper (which reads the session's own `ISession.isQuickChat` observable — **not** `workspace === undefined`, which can be transiently undefined for workspace-bound sessions too). It renders **inside the Sessions list directly below the Pinned section** (above the workspace/date groups) in **both** grouping modes — quick chats are neither a workspace nor a date bucket, so they are partitioned out of workspace/date grouping and rendered as their own entry right after Pinned. The section is **always visible** (even with no quick chats) whenever a provider advertises `supportsQuickChats` — subject to the `sessions.list.showEmptyDefaultGroups` setting (default `true`; when `false` the empty Chats section is hidden). Both the Pinned and Chats section headers carry a **leading icon** (`Codicon.pinned` for Pinned, `Codicon.commentDiscussion` for Chats) and share the standard section-header font/styling (the two headers look consistent — no prominent top-title variant). The Chats header shows the chat icon, the label "Chats", and a **"+" New Quick Chat** action in its section toolbar (also bound to **Cmd+K Cmd+N**) — the *only* create affordance for quick chats (Cmd+N always creates a new **session**, not a quick chat; there is no quick-chat action in the top Sessions header). The bulk archive/mark-done action is not offered on the Chats section. When the section has **no quick chats**, it shows a muted, centered **"No chats" placeholder row** (a synthetic non-session list item, like the "show more" rows) instead of an empty section. A pinned quick chat still appears in Pinned (pin wins), and an archived one still goes to Done (archive wins). Compact quick-chat rows never show a per-row chat/PR glyph as their **status icon** (their identity is already conveyed by the Chats section, the Pinned section, or a custom group), have no type icon in the details row, and carry no workspace badge/diff stats/timestamp (see Session Row above). Automation history uses the regular-row exception described above. Each quick chat is its **own single-chat session** (New Quick Chat = a new session per create), so it occupies one list row like any other session — there are no chat-level (`IChat`) rows. A quick chat is pinned/grouped/archived as a whole session: a pinned quick chat appears in Pinned (pin wins), an archived one goes to Done (archive wins). The earlier "single quick-chat container session whose peer `IChat`s become their own rows" model was descoped. diff --git a/src/vs/sessions/contrib/sessions/browser/views/automationsView.ts b/src/vs/sessions/contrib/sessions/browser/views/automationsView.ts index 73b97b5c0ad719..c79664fd2303f1 100644 --- a/src/vs/sessions/contrib/sessions/browser/views/automationsView.ts +++ b/src/vs/sessions/contrib/sessions/browser/views/automationsView.ts @@ -727,6 +727,7 @@ class AutomationHistorySection extends Disposable { const list = disposables.add(this.instantiationService.createInstance(SessionsFlatList, entry.listContainer, { showSessionHover: false, alwaysConsumeMouseWheel: false, + useCompactQuickChatRows: false, toolbarMenuId: Menus.AutomationsHistoryItem, markSessionReadOnOpen: false, approvalModel: this.approvalModel, diff --git a/src/vs/sessions/contrib/sessions/browser/views/sessionsList.ts b/src/vs/sessions/contrib/sessions/browser/views/sessionsList.ts index ded3a1bc6f626c..4449ab870feeb7 100644 --- a/src/vs/sessions/contrib/sessions/browser/views/sessionsList.ts +++ b/src/vs/sessions/contrib/sessions/browser/views/sessionsList.ts @@ -223,6 +223,7 @@ class SessionsTreeDelegate implements IListVirtualDelegate { private readonly _isPhone: () => boolean, private readonly _approvalRowMaxLines: number = DEFAULT_APPROVAL_ROW_MAX_LINES, private readonly _ciFixModel: ISessionCIFixModel | undefined = undefined, + private readonly _useCompactQuickChatRows = true, ) { } getHeight(element: SessionListItem): number { @@ -239,7 +240,7 @@ class SessionsTreeDelegate implements IListVirtualDelegate { let height: number; if (this._isPhone()) { height = SessionsTreeDelegate.ITEM_HEIGHT_PHONE; - } else if (isQuickChatSession(element as ISession)) { + } else if (this._useCompactQuickChatRows && isQuickChatSession(element as ISession)) { height = SessionsTreeDelegate.ITEM_HEIGHT_QUICK_CHAT; } else { height = SessionsTreeDelegate.ITEM_HEIGHT; @@ -392,7 +393,7 @@ class SessionItemRenderer implements ITreeRenderer = this._onDidApproveSession.event; constructor( - private readonly options: { grouping: () => SessionsGrouping; isPinned: (session: ISession) => boolean; isRenderedInCustomGroup?: (session: ISession) => boolean; visibleSessions: IObservable; getMultiSelectedSessions: (session: ISession) => ISession[]; showHover: boolean; approvalRowMaxLines: number; toolbarMenuId: MenuId | undefined; handleToolbarAction?: (action: IAction, session: ISession) => boolean | Promise; onDidRequestRename?: (session: ISession) => void }, + private readonly options: { grouping: () => SessionsGrouping; isPinned: (session: ISession) => boolean; isRenderedInCustomGroup?: (session: ISession) => boolean; visibleSessions: IObservable; getMultiSelectedSessions: (session: ISession) => ISession[]; showHover: boolean; useCompactQuickChatRows: boolean; approvalRowMaxLines: number; toolbarMenuId: MenuId | undefined; handleToolbarAction?: (action: IAction, session: ISession) => boolean | Promise; onDidRequestRename?: (session: ISession) => void }, private readonly approvalModel: AgentSessionApprovalModel | undefined, private readonly ciFixModel: ISessionCIFixModel | undefined, private readonly instantiationService: IInstantiationService, @@ -598,8 +599,7 @@ class SessionItemRenderer implements ITreeRenderer { @@ -622,9 +620,8 @@ class SessionItemRenderer implements ITreeRenderer 0)) { + if (!isQuickChat && !hideDetails && (changesSummary || changes.length > 0)) { let insertions = 0, deletions = 0; if (changesSummary) { @@ -1333,10 +1336,17 @@ class SessionPlaceholderRenderer implements ITreeRenderer SessionsGrouping; + readonly isPinned: (session: ISession) => boolean; + readonly isRenderedInCustomGroup?: (session: ISession) => boolean; + readonly includeQuickChatIdentity?: boolean; +} + class SessionsAccessibilityProvider { constructor( private readonly automationStatus?: IObservable, - private readonly workspaceBadgeOptions?: { grouping: () => SessionsGrouping; isPinned: (session: ISession) => boolean; isRenderedInCustomGroup?: (session: ISession) => boolean }, + private readonly options?: ISessionsAccessibilityProviderOptions, ) { } getWidgetAriaLabel(): string { @@ -1386,22 +1396,27 @@ class SessionsAccessibilityProvider { return derived(this, reader => { const title = element.title.read(reader); const updated = fromNow(element.updatedAt.read(reader), true); - let label = element.worktreePending?.read(reader) - ? localize('sessionItemWorktreePendingAria', "{0}, creating worktree, updated {1}", title, updated) - : localize('sessionItemAria', "{0}, updated {1}", title, updated); + let label: string; + if (this.options?.includeQuickChatIdentity && element.isQuickChat?.read(reader)) { + label = localize('sessionItemQuickChatAria', "{0}, chat, updated {1}", title, updated); + } else if (element.worktreePending?.read(reader)) { + label = localize('sessionItemWorktreePendingAria', "{0}, creating worktree, updated {1}", title, updated); + } else { + label = localize('sessionItemAria', "{0}, updated {1}", title, updated); + } const status = element.status.read(reader); const workspace = element.workspace.read(reader); const workspaceLabel = workspace ? getWorkspaceBadgeLabel(workspace) : undefined; if ( - this.workspaceBadgeOptions && + this.options && status !== SessionStatus.InProgress && status !== SessionStatus.NeedsInput && workspaceLabel && ( - this.workspaceBadgeOptions.grouping() !== SessionsGrouping.Workspace || - this.workspaceBadgeOptions.isPinned(element) || + this.options.grouping() !== SessionsGrouping.Workspace || + this.options.isPinned(element) || element.isArchived.read(reader) || - this.workspaceBadgeOptions.isRenderedInCustomGroup?.(element) + this.options.isRenderedInCustomGroup?.(element) ) ) { label = localize('sessionItemWorkspaceAria', "{0}, in {1}", label, workspaceLabel); @@ -1971,6 +1986,7 @@ export class SessionsList extends Disposable implements ISessionsList { visibleSessions: this._sessionsService.visibleSessions, getMultiSelectedSessions: s => this.getMultiSelectedSessions(s), showHover: true, + useCompactQuickChatRows: true, approvalRowMaxLines: DEFAULT_APPROVAL_ROW_MAX_LINES, toolbarMenuId: SessionItemToolbarMenuId, onDidRequestRename: session => { @@ -3698,6 +3714,11 @@ export interface ISessionsFlatListOptions { * consumed by the embedded tree. Defaults to `true` (standard list behavior). */ readonly alwaysConsumeMouseWheel?: boolean; + /** + * Whether quick chats use the compact single-line presentation. Defaults to + * `true`; consumers showing homogeneous history entries can opt into regular rows. + */ + readonly useCompactQuickChatRows?: boolean; } /** @@ -3745,6 +3766,7 @@ export class SessionsFlatList extends Disposable { // the instantiation service so this file's single suppressed import stays // the only reference. See the note on the `IAgentSessionsService` import. const agentSessionsService = instantiationService.invokeFunction(accessor => accessor.get(IAgentSessionsService)); + const useCompactQuickChatRows = this.options.useCompactQuickChatRows ?? true; const sessionRenderer = new SessionItemRenderer( { @@ -3753,6 +3775,7 @@ export class SessionsFlatList extends Disposable { visibleSessions: this._sessionsService.visibleSessions, getMultiSelectedSessions: s => [s], showHover: this.options.showSessionHover ?? true, + useCompactQuickChatRows, approvalRowMaxLines: this.options.approvalRowMaxLines ?? DEFAULT_APPROVAL_ROW_MAX_LINES, toolbarMenuId: this.options.toolbarMenuId ?? SessionItemToolbarMenuId, handleToolbarAction: this.options.onToolbarAction, @@ -3768,7 +3791,7 @@ export class SessionsFlatList extends Disposable { voicePlaybackService, ); - this._delegate = new SessionsTreeDelegate(approvalModel, () => false, this.options.approvalRowMaxLines ?? DEFAULT_APPROVAL_ROW_MAX_LINES, this.options.ciFixModel); + this._delegate = new SessionsTreeDelegate(approvalModel, () => false, this.options.approvalRowMaxLines ?? DEFAULT_APPROVAL_ROW_MAX_LINES, this.options.ciFixModel, useCompactQuickChatRows); this.tree = this._register(instantiationService.createInstance( WorkbenchObjectTree, @@ -3780,6 +3803,7 @@ export class SessionsFlatList extends Disposable { accessibilityProvider: new SessionsAccessibilityProvider(undefined, { grouping: () => SessionsGrouping.Date, isPinned: session => this._sessionsListModelService.isSessionPinned(session), + includeQuickChatIdentity: !useCompactQuickChatRows, }), identityProvider: { getId: (element: SessionListItem) => (element as ISession).resource.toString(), diff --git a/src/vs/sessions/contrib/sessions/test/browser/sessionsList.test.ts b/src/vs/sessions/contrib/sessions/test/browser/sessionsList.test.ts index b51555ae7074c2..bd194f1485d9b3 100644 --- a/src/vs/sessions/contrib/sessions/test/browser/sessionsList.test.ts +++ b/src/vs/sessions/contrib/sessions/test/browser/sessionsList.test.ts @@ -23,7 +23,7 @@ import { IAutomationService } from '../../../../../workbench/contrib/chat/common import { ICustomViewService } from '../../../../services/customView/browser/customViewService.js'; import { IChat, ISession, SessionStatus } from '../../../../services/sessions/common/session.js'; import { ISessionsManagementService } from '../../../../services/sessions/common/sessionsManagement.js'; -import { computeReorderSortChanges, groupByDate, groupByWorkspace, groupSessionsForList, ISessionSection, limitSessionsForList, SessionSectionRenderer, SessionsList, sortSessions, SessionsGrouping, SessionsSorting } from '../../browser/views/sessionsList.js'; +import { computeReorderSortChanges, groupByDate, groupByWorkspace, groupSessionsForList, ISessionSection, limitSessionsForList, SessionSectionRenderer, SessionsFlatList, SessionsList, sortSessions, SessionsGrouping, SessionsSorting } from '../../browser/views/sessionsList.js'; import { createListHarness, createTestSession } from './sessionsListTestUtils.js'; import '../../browser/views/sessionsViewActions.js'; @@ -724,6 +724,64 @@ suite('Sessions - SessionsList', () => { }); }); + suite('SessionsFlatList quick-chat presentation', () => { + + function renderQuickChat(useCompactQuickChatRows: boolean) { + const quickChat = createTestSession('Investigate failure', { isQuickChat: true }).session; + const harness = createListHarness(disposables, [quickChat]); + const container = harness.createContainer(); + const list = harness.store.add(harness.instantiationService.createInstance(SessionsFlatList, container, { + showSessionHover: false, + useCompactQuickChatRows, + onSessionOpen: () => { }, + })); + list.setSessions([quickChat]); + const contentHeight = list.getContentHeight(); + list.layout(contentHeight, 400); + + const item = container.querySelector('.session-item'); + assert.ok(item); + return { + usesStandardRowHeight: contentHeight === list.getRowHeight(), + isShorterThanStandardRow: contentHeight < list.getRowHeight(), + hasCompactClass: item.classList.contains('quick-chat'), + hasChatIcon: item.querySelector('.session-details-icon > .codicon')?.classList.contains('codicon-comment-discussion') ?? false, + badge: item.querySelector('.session-badge')?.textContent ?? undefined, + time: item.querySelector('.session-time')?.textContent ?? undefined, + hasDiff: !!item.querySelector('.session-diff'), + ariaLabel: item.closest('.monaco-list-row')?.getAttribute('aria-label') ?? null, + }; + } + + test('renders compact and regular quick-chat rows consistently', () => { + assert.deepStrictEqual({ + compact: renderQuickChat(true), + regular: renderQuickChat(false), + }, { + compact: { + usesStandardRowHeight: false, + isShorterThanStandardRow: true, + hasCompactClass: true, + hasChatIcon: false, + badge: undefined, + time: undefined, + hasDiff: false, + ariaLabel: 'Investigate failure, updated now', + }, + regular: { + usesStandardRowHeight: true, + isShorterThanStandardRow: false, + hasCompactClass: false, + hasChatIcon: true, + badge: 'Chat', + time: 'now', + hasDiff: false, + ariaLabel: 'Investigate failure, chat, updated now', + }, + }); + }); + }); + suite('computeReorderSortChanges', () => { const NOW = 1_000_000; const STEP = 60_000; From 9db4d181d1c9483310f5854e759c500f44744123 Mon Sep 17 00:00:00 2001 From: Sandeep Somavarapu Date: Tue, 18 Aug 2026 00:04:24 +0200 Subject: [PATCH 19/19] agentHost: Speed up rename chat tool (#331324) * agenthost: speed up rename chat tool Avoid enumerating every session when resolving a rename target. Add a targeted registry lookup so rename validation and persistence remain awaited while lookup cost stays constant. Fixes #331110 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * agenthost: share live session metadata overlay Reuse one helper for targeted session lookup and listSessions so both paths apply live state consistently. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * agenthost: make rename chat non-blocking Return to the agent before session lookup and persistence complete. Serialize queued renames and log deferred failures while retaining targeted session resolution. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * agenthost: update merged rename test Wait for the background rename independently while expecting the immediate tool result. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * agenthost: simplify background rename Start each rename independently without a sequencer and keep the existing tool description unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../agentHost/node/agentHostDatabase.ts | 15 ++ .../platform/agentHost/node/agentService.ts | 79 +++++++---- .../agentHost/node/agentSessionRegistry.ts | 27 ++++ .../node/shared/sessionServerTools.ts | 33 ++++- .../agentHost/test/node/agentService.test.ts | 61 +++++--- .../test/node/agentSessionRegistry.test.ts | 29 ++++ .../test/node/sessionServerTools.test.ts | 130 ++++++++++++++---- 7 files changed, 293 insertions(+), 81 deletions(-) diff --git a/src/vs/platform/agentHost/node/agentHostDatabase.ts b/src/vs/platform/agentHost/node/agentHostDatabase.ts index e7f811ce78c71e..acdb0efe78dde7 100644 --- a/src/vs/platform/agentHost/node/agentHostDatabase.ts +++ b/src/vs/platform/agentHost/node/agentHostDatabase.ts @@ -51,6 +51,7 @@ export interface IAgentHostDatabase extends IDisposable { /** Atomically tombstones and removes a session so concurrent backfill cannot re-register it. */ tombstoneAndUnregisterSession(session: string): Promise; updateSessionExternal(updates: readonly IAgentHostDatabaseExternalUpdate[]): Promise; + getSession(session: string): Promise; listSessions(): Promise; isSessionRegistryEmpty(): Promise; /** @@ -258,6 +259,20 @@ export class AgentHostDatabase implements IAgentHostDatabase { })); } + async getSession(session: string): Promise { + const row = await get(await this._ensureDatabase(), 'SELECT session_uri, provider, start_time, external, registration_source FROM sessions WHERE session_uri = ?', [session]); + if (!row) { + return undefined; + } + return { + session: row.session_uri as string, + provider: row.provider as AgentProvider, + startTime: row.start_time as number, + external: row.external === null || row.external === undefined ? undefined : row.external === 1, + source: row.registration_source as AgentSessionRegistrationSource, + }; + } + async isSessionRegistryEmpty(): Promise { const row = await get(await this._ensureDatabase(), 'SELECT 1 AS present FROM sessions LIMIT 1', []); return row === undefined; diff --git a/src/vs/platform/agentHost/node/agentService.ts b/src/vs/platform/agentHost/node/agentService.ts index ed73139a9efd39..a01a8c9403ea64 100644 --- a/src/vs/platform/agentHost/node/agentService.ts +++ b/src/vs/platform/agentHost/node/agentService.ts @@ -1062,6 +1062,7 @@ export class AgentService extends Disposable implements IAgentService { return { isActiveAgentTitleGenerationEnabled: () => this._isActiveAgentTitleGenerationEnabled(), listSessions: () => this.listSessions(), + getSession: session => this._getSessionMetadata(session), createSession: config => this.createSession(config), getModels: () => { const models: IAgentModelInfo[] = []; @@ -1076,6 +1077,7 @@ export class AgentService extends Disposable implements IAgentService { ? { ...(options.title !== undefined ? { title: options.title } : {}), ...(options.model !== undefined ? { model: options.model } : {}) } : undefined), renameChat: (session, chat, title) => this._renameChatFromTool(session, chat, title), + reportToolError: (toolName, error) => this._logService.error(`[AgentService] ${toolName} failed after the tool returned: ${toErrorMessage(error)}`), deleteSession: session => this.disposeSession(session), getChatContext: (session, chatId) => this._getChatContext(session, chatId), // Reads the `create_session` spawn depth from a session's `_meta` (0 when absent). @@ -1247,6 +1249,52 @@ export class AgentService extends Disposable implements IAgentService { }; } + private async _getSessionMetadata(session: URI): Promise { + const registered = await this._sessionRegistry.get(session, entry => this._migrateRegisteredSession(entry)); + if (!registered) { + return undefined; + } + const agent = this._providers.get(registered.provider); + const liveSummary = this._stateManager.getSessionSummary(session.toString()); + if (liveSummary) { + const metadata = (liveSummary.workingDirectories === undefined && agent + ? await this._registeredSessionMetadata(agent, session, registered.external) + : undefined) ?? { + session, + startTime: registered.startTime, + modifiedTime: Date.parse(liveSummary.modifiedAt), + }; + return this._withLiveSessionMetadata(metadata, liveSummary); + } + if (!agent) { + return undefined; + } + return this._registeredSessionMetadata(agent, session, registered.external); + } + + private _withLiveSessionMetadata(metadata: IAgentSessionMetadata, liveSummary: SessionSummary): IAgentSessionMetadata { + let _meta = liveSummary._meta !== undefined || metadata._meta !== undefined + ? { ...metadata._meta, ...liveSummary._meta } + : undefined; + _meta = withSessionMultiRootMetadata(_meta, readSessionMultiRootMetadata(liveSummary._meta) ?? readSessionMultiRootMetadata(metadata._meta)); + return { + ...metadata, + summary: liveSummary.title || metadata.summary, + status: liveSummary.status, + activity: liveSummary.activity, + modifiedTime: Date.parse(liveSummary.modifiedAt), + project: liveSummary.project + ? { uri: URI.parse(liveSummary.project.uri), displayName: liveSummary.project.displayName } + : metadata.project, + workingDirectories: liveSummary.workingDirectories !== undefined + ? liveSummary.workingDirectories.map(directory => URI.parse(directory)) + : metadata.workingDirectories, + changes: liveSummary.changes ?? metadata.changes, + changesets: this._stateManager.getSessionState(metadata.session.toString())?.changesets ?? metadata.changesets, + ...(_meta !== undefined ? { _meta } : {}), + }; + } + /** * Awaits legacy migration started at provider registration. Provider-owned * discovery is independent and surfaces unknown chats additively. @@ -1711,36 +1759,7 @@ export class AgentService extends Disposable implements IAgentService { const withStatus = result.map(s => { const liveSummary = this._stateManager.getSessionSummary(s.session.toString()); if (liveSummary) { - // Overlay the live `_meta` over the DB-derived value. The live - // `_meta` is the freshest source (e.g. the GitHub state is - // published here as soon as a PR is created), so a freshly-created - // session that has not yet persisted its state to its session - // database still reports it here. Keep the DB value as the base so - // any keys absent from the live `_meta` are preserved. - let _meta = liveSummary._meta !== undefined || s._meta !== undefined - ? { ...s._meta, ...liveSummary._meta } - : undefined; - _meta = withSessionMultiRootMetadata(_meta, readSessionMultiRootMetadata(liveSummary._meta) ?? readSessionMultiRootMetadata(s._meta)); - const liveWorkingDirs = liveSummary.workingDirectories; - return { - ...s, - summary: liveSummary.title || s.summary, - // Supersedes the flags folded in above: the state manager seeded - // them from the same database on restore and has applied every - // mutation since. - status: liveSummary.status, - activity: liveSummary.activity, - modifiedTime: Date.parse(liveSummary.modifiedAt), - project: liveSummary.project - ? { uri: URI.parse(liveSummary.project.uri), displayName: liveSummary.project.displayName } - : s.project, - workingDirectories: liveWorkingDirs !== undefined - ? liveWorkingDirs.map(d => URI.parse(d)) - : s.workingDirectories, - changes: liveSummary.changes ?? s.changes, - changesets: this._stateManager.getSessionState(s.session.toString())?.changesets ?? s.changesets, - ...(_meta !== undefined ? { _meta } : {}), - }; + return this._withLiveSessionMetadata(s, liveSummary); } return s; }); diff --git a/src/vs/platform/agentHost/node/agentSessionRegistry.ts b/src/vs/platform/agentHost/node/agentSessionRegistry.ts index a339332098a482..ea4c00a5431e23 100644 --- a/src/vs/platform/agentHost/node/agentSessionRegistry.ts +++ b/src/vs/platform/agentHost/node/agentSessionRegistry.ts @@ -112,6 +112,33 @@ export class AgentSessionRegistry extends Disposable { return result; } + /** Returns the session registered under `session`, or `undefined` when it is unknown. */ + async get(session: URI, migrate?: RegisteredSessionMigration): Promise { + const stored = await this._database.getSession(session.toString()); + if (!stored) { + return undefined; + } + const entry: IStoredRegisteredSession = { + session: URI.parse(stored.session), + provider: stored.provider, + startTime: stored.startTime, + external: stored.external, + source: stored.source, + }; + const migrated = await migrate?.(entry); + if (migrated) { + await this._database.updateSessionExternal([{ session: migrated.session.toString(), external: migrated.external }]); + return migrated; + } + if (entry.external === undefined) { + throw new Error(`Session migration did not resolve registry entry ${entry.session.toString()}`); + } + return { + ...entry, + external: entry.external, + }; + } + /** Whether the registry has ever been populated. Retained for compatibility. */ async isEmpty(): Promise { return this._database.isSessionRegistryEmpty(); diff --git a/src/vs/platform/agentHost/node/shared/sessionServerTools.ts b/src/vs/platform/agentHost/node/shared/sessionServerTools.ts index 402f4050ec927b..abfce0bacacfe4 100644 --- a/src/vs/platform/agentHost/node/shared/sessionServerTools.ts +++ b/src/vs/platform/agentHost/node/shared/sessionServerTools.ts @@ -3,8 +3,8 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import { URI } from '../../../../base/common/uri.js'; import type { Mutable } from '../../../../base/common/types.js'; +import { URI } from '../../../../base/common/uri.js'; import { localize } from '../../../../nls.js'; import { AgentSession, type AgentProvider, type IAgentCreateSessionConfig, type IAgentModelInfo, type IAgentSessionMetadata } from '../../common/agent.js'; import { SessionStatus } from '../../common/state/protocol/channels-session/state.js'; @@ -210,12 +210,14 @@ export interface IResolvedCreateSessionArgs { export interface ISessionServerToolAccessor { readonly isActiveAgentTitleGenerationEnabled: () => boolean; readonly listSessions: () => Promise; + readonly getSession: (session: URI) => Promise; readonly createSession: (config: IAgentCreateSessionConfig) => Promise; readonly getModels: () => readonly IAgentModelInfo[]; readonly getCreationDefaults: (source: URI) => ISessionCreationDefaults | undefined; readonly startPrompt: (session: URI, chat: URI, prompt: string) => Promise; readonly createChat: (session: URI, chat: URI, options?: { title?: string; model?: ModelSelection }) => Promise; readonly renameChat: (session: URI, chat: URI, title: string) => Promise; + readonly reportToolError: (toolName: SessionServerToolName, error: unknown) => void; readonly deleteSession: (session: URI) => Promise; /** Reads a point-in-time snapshot of a session's chat conversation (default chat, or a specific chat by id). */ readonly getChatContext: (session: URI, chatId?: string) => Promise; @@ -855,11 +857,30 @@ export function getRenameChatArgs(rawArgs: unknown, sessions: readonly IAgentSes return { session, chat: currentChat, title, chatId: parsed.chatId }; } +function getRenameChatSession(rawArgs: unknown, currentChannel?: ProtocolURI): URI { + const args = (rawArgs ?? {}) as IRenameChatArgs; + const chatInput = getOptionalString(args.chat, 'chat', SessionServerToolName.RenameChat); + if (chatInput !== undefined) { + const session = parseOpenSessionLinkUri(chatInput); + if (!session) { + throw new Error(`Invalid ${SessionServerToolName.RenameChat} input: chat must be an agent-host-session:// link targeting a known chat.`); + } + return session; + } + if (!currentChannel || !currentChatUri(currentChannel)) { + throw new Error(`Invalid ${SessionServerToolName.RenameChat} input: chat must target a known chat, or the tool must run inside that chat.`); + } + return currentSessionUri(currentChannel); +} + export async function applyRenameChatTool(accessor: ISessionServerToolAccessor, rawArgs: unknown, currentChannel?: ProtocolURI): Promise { - const sessions = await accessor.listSessions(); - const { session, chat, title } = getRenameChatArgs(rawArgs, sessions, currentChannel); - const result = await accessor.renameChat(session, chat, title); - return `Renamed chat to "${result.title}".`; + void (async () => { + const targetSession = getRenameChatSession(rawArgs, currentChannel); + const metadata = await accessor.getSession(targetSession); + const { session, chat, title } = getRenameChatArgs(rawArgs, metadata ? [metadata] : [], currentChannel); + await accessor.renameChat(session, chat, title); + })().catch(error => accessor.reportToolError(SessionServerToolName.RenameChat, error)); + return 'Renaming chat.'; } interface ISendMessageArgs { @@ -1158,7 +1179,7 @@ function getSessionToolDisplay(toolName: string, _args: unknown, _result?: IServ return { displayName: localize('toolName.renameChat', "Rename Chat"), invocationMessage: localize('toolInvoke.renameChat', "Renaming chat"), - pastTenseMessage: localize('toolComplete.renameChat', "Updated chat name"), + pastTenseMessage: localize('toolComplete.renameChat', "Requested chat rename"), }; case SessionServerToolName.SendMessage: return { diff --git a/src/vs/platform/agentHost/test/node/agentService.test.ts b/src/vs/platform/agentHost/test/node/agentService.test.ts index e8855d05dad1f4..84c4a811fb7ec3 100644 --- a/src/vs/platform/agentHost/test/node/agentService.test.ts +++ b/src/vs/platform/agentHost/test/node/agentService.test.ts @@ -283,6 +283,11 @@ class TransientRegistryWriteDatabase implements IAgentHostDatabase { : session); } + async getSession(session: string): Promise { + const value = this._sessions.get(session); + return value && this._sessionsWithoutExternal.has(session) ? { ...value, external: undefined } : value; + } + async isSessionRegistryEmpty(): Promise { return this._sessions.size === 0; } @@ -9766,6 +9771,17 @@ suite('AgentService (node dispatcher)', () => { suite('rename server tools', () => { test('rename_chat replaces live and persisted default and peer chat titles', async () => { + class RecordingTitleDatabase extends TestSessionDatabase { + readonly finalRenamePersisted = new DeferredPromise(); + finalRenameKey: string | undefined; + + override async setMetadataValues(values: Readonly>): Promise { + await super.setMetadataValues(values); + if (this.finalRenameKey && values[this.finalRenameKey] === 'Complete replacement peer chat title') { + await this.finalRenamePersisted.complete(); + } + } + } class ServerToolAgent extends MockAgent { serverToolHost: IAgentServerToolHost | undefined; @@ -9774,7 +9790,7 @@ suite('AgentService (node dispatcher)', () => { } } - const db = new TestSessionDatabase(); + const db = new RecordingTitleDatabase(); const localService = disposables.add(new AgentService(new NullLogService(), fileService, createSessionDataService(db), { _serviceBrand: undefined } as IProductService, createNoopGitService())); localService.configurationService.updateRootConfig({ [AgentHostActiveAgentTitleGenerationConfigKey]: true }); const agent = disposables.add(new ServerToolAgent('copilot')); @@ -9783,6 +9799,7 @@ suite('AgentService (node dispatcher)', () => { const sessionUri = session.toString(); const defaultChat = buildDefaultChatUri(session); const peerChat = buildChatUri(sessionUri, 'peer-rename'); + db.finalRenameKey = `customChatTitle:${peerChat}`; localService.stateManager.dispatchServerAction(sessionUri, { type: ActionType.SessionTitleChanged, title: 'Previous user title' }); await db.setMetadata('customTitle', 'Previous user title'); await db.setMetadata('customTitleSource', 'user'); @@ -9805,6 +9822,8 @@ suite('AgentService (node dispatcher)', () => { chat: `agent-host-session://copilot/${AgentSession.id(session)}?chat=peer-rename`, title: 'Complete replacement peer chat title', }); + await db.finalRenamePersisted.p; + await timeout(0); assert.deepStrictEqual({ singleChatResult, @@ -9820,9 +9839,9 @@ suite('AgentService (node dispatcher)', () => { persistedChatTitle: await db.getMetadata(`customChatTitle:${peerChat}`), persistedChatSource: await db.getMetadata(`customChatTitleSource:${peerChat}`), }, { - singleChatResult: 'Renamed chat to "Single-chat title".', - multiChatDefaultResult: 'Renamed chat to "Complete replacement default chat title".', - chatResult: 'Renamed chat to "Complete replacement peer chat title".', + singleChatResult: 'Renaming chat.', + multiChatDefaultResult: 'Renaming chat.', + chatResult: 'Renaming chat.', liveSessionTitle: 'Multi-chat session title', liveDefaultChatTitle: 'Complete replacement default chat title', liveChatTitle: 'Complete replacement peer chat title', @@ -9837,8 +9856,14 @@ suite('AgentService (node dispatcher)', () => { test('rename failures preserve live state and both persisted metadata values', async () => { class FailingTitleDatabase extends TestSessionDatabase { + readonly allFailuresObserved = new DeferredPromise(); + private failureCount = 0; + override async setMetadataValues(values: Readonly>): Promise { if (Object.keys(values).some(key => key.startsWith('customTitle') || key.startsWith('customChatTitle'))) { + if (++this.failureCount === 3) { + await this.allFailuresObserved.complete(); + } throw new Error('title persistence failed'); } return super.setMetadataValues(values); @@ -9865,10 +9890,7 @@ suite('AgentService (node dispatcher)', () => { await db.setMetadata('customTitle', 'Original session'); await db.setMetadata('customTitleSource', 'user'); - await assert.rejects( - async () => agent.serverToolHost!.executeTool(defaultChat, SessionServerToolName.RenameChat, { title: 'Session-backed title will fail' }), - /title persistence failed/, - ); + const sessionResult = await agent.serverToolHost!.executeTool(defaultChat, SessionServerToolName.RenameChat, { title: 'Session-backed title will fail' }); localService.stateManager.addChat(sessionUri, peerChat, { title: 'Original chat' }); await db.setMetadata(`customChatTitle:${defaultChat}`, 'Original session'); @@ -9876,18 +9898,16 @@ suite('AgentService (node dispatcher)', () => { await db.setMetadata(`customChatTitle:${peerChat}`, 'Original chat'); await db.setMetadata(`customChatTitleSource:${peerChat}`, 'user'); - await assert.rejects( - async () => agent.serverToolHost!.executeTool(defaultChat, SessionServerToolName.RenameChat, { title: 'Chat-backed title will fail' }), - /title persistence failed/, - ); - await assert.rejects( - async () => agent.serverToolHost!.executeTool(buildDefaultChatUri(session), SessionServerToolName.RenameChat, { - chat: `agent-host-session://copilot/${AgentSession.id(session)}?chat=peer-failure`, - title: 'Chat will fail', - }), - /title persistence failed/, - ); + const defaultChatResult = await agent.serverToolHost!.executeTool(defaultChat, SessionServerToolName.RenameChat, { title: 'Chat-backed title will fail' }); + const peerChatResult = await agent.serverToolHost!.executeTool(buildDefaultChatUri(session), SessionServerToolName.RenameChat, { + chat: `agent-host-session://copilot/${AgentSession.id(session)}?chat=peer-failure`, + title: 'Chat will fail', + }); + await db.allFailuresObserved.p; assert.deepStrictEqual({ + sessionResult, + defaultChatResult, + peerChatResult, liveSession: localService.stateManager.getSessionState(sessionUri)?.title, sessionTitle: await db.getMetadata('customTitle'), sessionSource: await db.getMetadata('customTitleSource'), @@ -9898,6 +9918,9 @@ suite('AgentService (node dispatcher)', () => { chatTitle: await db.getMetadata(`customChatTitle:${peerChat}`), chatSource: await db.getMetadata(`customChatTitleSource:${peerChat}`), }, { + sessionResult: 'Renaming chat.', + defaultChatResult: 'Renaming chat.', + peerChatResult: 'Renaming chat.', liveSession: 'Original session', sessionTitle: 'Original session', sessionSource: 'user', diff --git a/src/vs/platform/agentHost/test/node/agentSessionRegistry.test.ts b/src/vs/platform/agentHost/test/node/agentSessionRegistry.test.ts index b41893121b594a..263ae937520bf1 100644 --- a/src/vs/platform/agentHost/test/node/agentSessionRegistry.test.ts +++ b/src/vs/platform/agentHost/test/node/agentSessionRegistry.test.ts @@ -78,6 +78,11 @@ class TestAgentHostDatabase implements IAgentHostDatabase { return [...this.sessions.values()]; } + async getSession(session: string): Promise { + this._throwReadFailure(); + return this.sessions.get(session); + } + async isSessionRegistryEmpty(): Promise { this._throwReadFailure(); return this.sessions.size === 0; @@ -195,6 +200,30 @@ suite('AgentSessionRegistry', () => { ], }); }); + + test('get reads only the requested session', async () => { + const testDatabase = new TestAgentHostDatabase(); + database = testDatabase; + testDatabase.sessions.set(a.toString(), { session: a.toString(), provider: 'copilot', startTime: 1, external: false, source: 'explicit' }); + testDatabase.sessions.set(b.toString(), { session: b.toString(), provider: 'claude', startTime: 2, external: false, source: 'explicit' }); + const registry = createRegistry(); + + const [entry, missing] = await Promise.all([ + registry.get(b), + registry.get(AgentSession.uri('copilot', 'missing')), + ]); + + assert.deepStrictEqual({ + listCalls: testDatabase.listCalls, + entry: entry && { session: entry.session.toString(), provider: entry.provider }, + missing, + }, { + listCalls: 0, + entry: { session: b.toString(), provider: 'claude' }, + missing: undefined, + }); + }); + const registerRestored = (registry: AgentSessionRegistry, session: typeof a, provider: 'copilot' | 'claude', startTime: number) => registry.register(session, { provider, startTime, source: 'restore' }, { checkTombstone: true }); const registerDiscovered = (registry: AgentSessionRegistry, session: typeof a, provider: 'copilot' | 'claude', startTime: number) => diff --git a/src/vs/platform/agentHost/test/node/sessionServerTools.test.ts b/src/vs/platform/agentHost/test/node/sessionServerTools.test.ts index a08428c58384ab..0f193b8d1a0fb0 100644 --- a/src/vs/platform/agentHost/test/node/sessionServerTools.test.ts +++ b/src/vs/platform/agentHost/test/node/sessionServerTools.test.ts @@ -4,6 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import assert from 'assert'; +import { DeferredPromise } from '../../../../base/common/async.js'; import { DisposableStore } from '../../../../base/common/lifecycle.js'; import { URI } from '../../../../base/common/uri.js'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../base/test/common/utils.js'; @@ -58,12 +59,14 @@ suite('SessionServerTools', () => { return { isActiveAgentTitleGenerationEnabled: overrides?.isActiveAgentTitleGenerationEnabled ?? (() => true), listSessions: overrides?.listSessions ?? (async () => [sessionMeta('s1', SessionStatus.InProgress, workspace)]), + getSession: overrides?.getSession ?? (async session => session.toString() === 'copilot:/s1' ? sessionMeta('s1', SessionStatus.InProgress, workspace) : undefined), createSession: overrides?.createSession ?? (async config => { overrides?.onCreate?.(config); return URI.parse('copilot:/new'); }), getModels: overrides?.getModels ?? (() => [model]), getCreationDefaults: overrides?.getCreationDefaults ?? (() => undefined), startPrompt: overrides?.startPrompt ?? (async (session, chat, prompt) => { overrides?.onPrompt?.(session, chat, prompt); }), createChat: overrides?.createChat ?? (async (session, chat, options) => { overrides?.onCreateChat?.(session, chat, options); }), renameChat: overrides?.renameChat ?? (async (session, chat, title) => { overrides?.onRenameChat?.(session, chat, title); return { title }; }), + reportToolError: overrides?.reportToolError ?? (() => { }), deleteSession: overrides?.deleteSession ?? (async session => { overrides?.onDelete?.(session); }), getChatContext: overrides?.getChatContext ?? (async () => undefined), getSessionSpawnDepth: overrides?.getSessionSpawnDepth ?? (session => depths.get(session.toString()) ?? 0), @@ -125,7 +128,7 @@ suite('SessionServerTools', () => { ); assert.strictEqual( await host.executeTool(buildDefaultChatUri(enabledSession), SessionServerToolName.RenameChat, { title: 'Enabled' }), - 'Renamed chat to "Enabled".', + 'Renaming chat.', ); assert.deepStrictEqual({ disabledTools: stateManager.getSessionState(disabledSession)?.serverTools?.map(tool => tool.name), @@ -166,7 +169,7 @@ suite('SessionServerTools', () => { assert.strictEqual( await host.executeTool(buildDefaultChatUri(session), SessionServerToolName.RenameChat, { title: 'Still enabled' }), - 'Renamed chat to "Still enabled".', + 'Renaming chat.', ); stateManager.dispose(); }); @@ -536,25 +539,92 @@ suite('SessionServerTools', () => { assert.throws(() => getRenameChatArgs({ chat: 'agent-host-session://copilot/s2?chat=c9', session: 'copilot:/s1', title: 'Mismatch' }, sessions), /must match/); }); - test('rename_chat always forwards the addressed default or peer chat', async () => { - let renamed: { session: URI; chat: URI; title: string } | undefined; + test('rename_chat returns before resolving the target session', async () => { + const getSessionStarted = new DeferredPromise(); + const releaseGetSession = new DeferredPromise(); + const renameCompleted = new DeferredPromise(); const accessor = createAccessor({ - listSessions: async () => [sessionMeta('s1', SessionStatus.Idle, workspace)], - onRenameChat: (session, chat, title) => { renamed = { session, chat, title }; }, + getSession: async session => { + await getSessionStarted.complete(session); + await releaseGetSession.p; + return sessionMeta('s1', SessionStatus.Idle, workspace); + }, + renameChat: async (_session, _chat, _title) => { + await renameCompleted.complete(); + return { title: 'Peer Focus' }; + }, + }); + const peer = buildChatUri('copilot:/s1', 'peer'); + const result = await applyRenameChatTool(accessor, { title: 'Peer Focus' }, peer); + const targetSession = await getSessionStarted.p; + assert.deepStrictEqual({ + result, + targetSession: targetSession.toString(), + renameCompletedBeforeRelease: renameCompleted.isSettled, + }, { + result: 'Renaming chat.', + targetSession: 'copilot:/s1', + renameCompletedBeforeRelease: false, + }); + await releaseGetSession.complete(); + await renameCompleted.p; + }); + + test('rename_chat gets only target sessions and forwards addressed default or peer chats', async () => { + let listSessionsCalls = 0; + const renames: { session: string; chat: string; title: string }[] = []; + const allRenamesCompleted = new DeferredPromise(); + const accessor = createAccessor({ + listSessions: async () => { + listSessionsCalls++; + return []; + }, + getSession: async session => session.toString() === 'copilot:/s1' ? sessionMeta('s1', SessionStatus.Idle, workspace) : undefined, + renameChat: async (session, chat, title) => { + renames.push({ session: session.toString(), chat: chat.toString(), title }); + if (renames.length === 3) { + await allRenamesCompleted.complete(); + } + return { title }; + }, }); const peer = buildChatUri('copilot:/s1', 'peer'); const defaultChat = buildDefaultChatUri('copilot:/s1'); - assert.strictEqual(await applyRenameChatTool(accessor, { title: 'Default Focus' }, defaultChat), 'Renamed chat to "Default Focus".'); - assert.deepStrictEqual({ session: renamed?.session.toString(), chat: renamed?.chat.toString(), title: renamed?.title }, { session: 'copilot:/s1', chat: defaultChat, title: 'Default Focus' }); - assert.strictEqual(await applyRenameChatTool(accessor, { title: 'Peer Focus' }, peer), 'Renamed chat to "Peer Focus".'); - assert.deepStrictEqual({ session: renamed?.session.toString(), chat: renamed?.chat.toString(), title: renamed?.title }, { session: 'copilot:/s1', chat: peer, title: 'Peer Focus' }); - assert.strictEqual(await applyRenameChatTool(accessor, { chat: 'agent-host-session://copilot/s1?chat=peer', title: 'Updated Focus' }), 'Renamed chat to "Updated Focus".'); - assert.deepStrictEqual({ session: renamed?.session.toString(), chat: renamed?.chat.toString(), title: renamed?.title }, { session: 'copilot:/s1', chat: peer, title: 'Updated Focus' }); - await assert.rejects(() => applyRenameChatTool(createAccessor({ - listSessions: async () => [sessionMeta('s1', SessionStatus.Idle, workspace)], + const results = await Promise.all([ + applyRenameChatTool(accessor, { title: 'Default Focus' }, defaultChat), + applyRenameChatTool(accessor, { title: 'Peer Focus' }, peer), + applyRenameChatTool(accessor, { chat: 'agent-host-session://copilot/s1?chat=peer', title: 'Updated Focus' }), + ]); + await allRenamesCompleted.p; + assert.deepStrictEqual({ results, renames, listSessionsCalls }, { + results: ['Renaming chat.', 'Renaming chat.', 'Renaming chat.'], + renames: [ + { session: 'copilot:/s1', chat: defaultChat, title: 'Default Focus' }, + { session: 'copilot:/s1', chat: peer, title: 'Peer Focus' }, + { session: 'copilot:/s1', chat: peer, title: 'Updated Focus' }, + ], + listSessionsCalls: 0, + }); + }); + + test('rename_chat reports background failures', async () => { + const reportedError = new DeferredPromise<{ toolName: SessionServerToolName; error: unknown }>(); + const result = await applyRenameChatTool(createAccessor({ + getSession: async () => sessionMeta('s1', SessionStatus.Idle, workspace), renameChat: async () => { throw new Error('Invalid rename_chat input: chat must match a known non-default chat.'); }, - }), { chat: 'agent-host-session://copilot/s1?chat=missing', title: 'Ignored' }), /known non-default chat/); + reportToolError: (toolName, error) => { void reportedError.complete({ toolName, error }); }, + }), { chat: 'agent-host-session://copilot/s1?chat=missing', title: 'Ignored' }); + const failure = await reportedError.p; + assert.deepStrictEqual({ + result, + toolName: failure.toolName, + error: failure.error instanceof Error ? failure.error.message : failure.error, + }, { + result: 'Renaming chat.', + toolName: SessionServerToolName.RenameChat, + error: 'Invalid rename_chat input: chat must match a known non-default chat.', + }); }); test('rename_chat uses the invoking chat while server-tool state remains session-scoped', async () => { @@ -569,39 +639,47 @@ suite('SessionServerTools', () => { createdAt: new Date(0).toISOString(), modifiedAt: new Date(0).toISOString(), }); - let renamedChat: string | undefined; + const renamedChat = new DeferredPromise(); const host = new AgentServerToolHost(stateManager, [ createSessionServerToolGroup(createAccessor({ - onRenameChat: (_session, chat) => { renamedChat = chat.toString(); }, + onRenameChat: (_session, chat) => { void renamedChat.complete(chat.toString()); }, })), ]); host.advertise(session); const result = await host.executeTool(peer, SessionServerToolName.RenameChat, { title: 'Peer Focus' }); - assert.deepStrictEqual({ result, renamedChat }, { - result: 'Renamed chat to "Peer Focus".', + assert.deepStrictEqual({ result, renamedChat: await renamedChat.p }, { + result: 'Renaming chat.', renamedChat: peer, }); stateManager.dispose(); }); test('repeated rename tool calls each apply their requested title', async () => { - let renameCalls = 0; + const bothRenamesStarted = new DeferredPromise(); + const releaseFirstRename = new DeferredPromise(); + const titles: string[] = []; const accessor = createAccessor({ - listSessions: async () => [sessionMeta('s1', SessionStatus.Idle, workspace)], renameChat: async (_session, _chat, title) => { - renameCalls++; + titles.push(title); + if (titles.length === 1) { + await releaseFirstRename.p; + } else { + await bothRenamesStarted.complete(); + } return { title }; }, }); const first = await applyRenameChatTool(accessor, { chat: 'agent-host-session://copilot/s1', title: 'Named Once' }); const second = await applyRenameChatTool(accessor, { chat: 'agent-host-session://copilot/s1', title: 'Renamed Again' }); - assert.deepStrictEqual({ first, second, renameCalls }, { - first: 'Renamed chat to "Named Once".', - second: 'Renamed chat to "Renamed Again".', - renameCalls: 2, + await bothRenamesStarted.p; + assert.deepStrictEqual({ first, second, titles }, { + first: 'Renaming chat.', + second: 'Renaming chat.', + titles: ['Named Once', 'Renamed Again'], }); + await releaseFirstRename.complete(); }); test('create_chat inherits the calling chat model when no override is provided', async () => {