From 3109dfc3fc3a3f554d4164cabc042ef54c371007 Mon Sep 17 00:00:00 2001 From: Rob Lourens Date: Sat, 25 Apr 2026 16:50:30 -0700 Subject: [PATCH 1/8] Fix reasoning text rendering when restoring agent host sessions (#312559) * Fix: preserve reasoning order when restoring agent host sessions When re-opening an agent host session, thinking text was rendered up front instead of being properly interspersed with tool calls. The SDK's `getMessages()` history bundles reasoning into each `assistant.message` as `reasoningText` rather than emitting it as a separate event. `_buildTurnsFromMessages` was reading `content` and tool requests but ignoring `reasoningText` entirely, so reasoning was either dropped or came from a different code path that did not honor stream order. Fix: when handling an `assistant.message`, push a Reasoning response part (from `reasoningText`) immediately before the Markdown part. This matches the EH CLI's history-replay shape and keeps reasoning, content, and tool calls interleaved in their original order. Also applied to subagent inner messages. (Written by Copilot) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Update ScriptedMockAgent to use SessionHistoryEvent Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../platform/agentHost/common/agentService.ts | 13 ++++++- .../platform/agentHost/node/agentService.ts | 37 ++++++++++++++----- .../agentHost/node/copilot/copilotAgent.ts | 6 +-- .../node/copilot/copilotAgentSession.ts | 4 +- .../node/copilot/mapSessionEvents.ts | 6 +-- .../agentHost/test/node/agentService.test.ts | 34 +++++++++++++++++ .../platform/agentHost/test/node/mockAgent.ts | 10 ++--- 7 files changed, 87 insertions(+), 23 deletions(-) diff --git a/src/vs/platform/agentHost/common/agentService.ts b/src/vs/platform/agentHost/common/agentService.ts index d50e7c2e5f236..786b973bb6bf7 100644 --- a/src/vs/platform/agentHost/common/agentService.ts +++ b/src/vs/platform/agentHost/common/agentService.ts @@ -340,6 +340,17 @@ export interface IAgentReasoningEvent extends IAgentProgressEventBase { readonly content: string; } +/** + * The set of events returned by {@link IAgent.getSessionMessages} when + * reconstructing a session's history. Reasoning is carried inline on + * {@link IAgentMessageEvent.reasoningText} rather than as a separate event. + */ +export type SessionHistoryEvent = + | IAgentMessageEvent + | IAgentToolStartEvent + | IAgentToolCompleteEvent + | IAgentSubagentStartedEvent; + /** A steering message was consumed (sent to the model). */ export interface IAgentSteeringConsumedEvent extends IAgentProgressEventBase { readonly type: 'steering_consumed'; @@ -452,7 +463,7 @@ export interface IAgent { setPendingMessages?(session: URI, steeringMessage: PendingMessage | undefined, queuedMessages: readonly PendingMessage[]): void; /** Retrieve all session events/messages for reconstruction. */ - getSessionMessages(session: URI): Promise<(IAgentMessageEvent | IAgentToolStartEvent | IAgentToolCompleteEvent | IAgentSubagentStartedEvent)[]>; + getSessionMessages(session: URI): Promise; /** Dispose a session, freeing resources. */ disposeSession(session: URI): Promise; diff --git a/src/vs/platform/agentHost/node/agentService.ts b/src/vs/platform/agentHost/node/agentService.ts index abf84f27c3cd5..631b8e0dc7cec 100644 --- a/src/vs/platform/agentHost/node/agentService.ts +++ b/src/vs/platform/agentHost/node/agentService.ts @@ -14,7 +14,7 @@ import { FileSystemProviderErrorCode, IFileService, toFileSystemProviderErrorCod import { InstantiationService } from '../../instantiation/common/instantiationService.js'; import { ServiceCollection } from '../../instantiation/common/serviceCollection.js'; import { ILogService } from '../../log/common/log.js'; -import { AgentProvider, AgentSession, IAgent, IAgentCreateSessionConfig, IAgentMessageEvent, IAgentResolveSessionConfigParams, IAgentService, IAgentSessionConfigCompletionsParams, IAgentSessionMetadata, IAgentSubagentStartedEvent, IAgentToolCompleteEvent, IAgentToolStartEvent, AuthenticateParams, AuthenticateResult } from '../common/agentService.js'; +import { AgentProvider, AgentSession, IAgent, IAgentCreateSessionConfig, IAgentResolveSessionConfigParams, IAgentService, IAgentSessionConfigCompletionsParams, IAgentSessionMetadata, IAgentSubagentStartedEvent, IAgentToolStartEvent, SessionHistoryEvent, AuthenticateParams, AuthenticateResult } from '../common/agentService.js'; import { ISessionDataService } from '../common/sessionDataService.js'; import { ActionType, ActionEnvelope, INotification, RootAction, SessionAction, TerminalAction, isSessionAction } from '../common/state/sessionActions.js'; import type { CreateTerminalParams, ResolveSessionConfigResult, SessionConfigCompletionsResult } from '../common/state/protocol/commands.js'; @@ -690,7 +690,7 @@ export class AgentService extends Disposable implements IAgentService { * closes it. */ private _buildTurnsFromMessages( - messages: readonly (IAgentMessageEvent | IAgentToolStartEvent | IAgentToolCompleteEvent | IAgentSubagentStartedEvent)[], + messages: readonly SessionHistoryEvent[], ): Turn[] { const turns: Turn[] = []; // Track subagent metadata by parent tool call ID so we can inject @@ -737,6 +737,16 @@ export class AgentService extends Disposable implements IAgentService { currentTurn = startTurn(msg.messageId, ''); } + // Reasoning is bundled onto the assistant message and + // logically precedes its content/tool calls. + if (msg.reasoningText) { + currentTurn.responseParts.push({ + kind: ResponsePartKind.Reasoning, + id: generateUuid(), + content: msg.reasoningText, + }); + } + if (msg.content) { currentTurn.responseParts.push({ kind: ResponsePartKind.Markdown, @@ -821,7 +831,7 @@ export class AgentService extends Disposable implements IAgentService { * tool calls. */ private _buildSubagentTurns( - parentMessages: readonly (IAgentMessageEvent | IAgentToolStartEvent | IAgentToolCompleteEvent | IAgentSubagentStartedEvent)[], + parentMessages: readonly SessionHistoryEvent[], parentToolCallId: string, childSessionUri: string, ): Turn[] { @@ -903,12 +913,21 @@ export class AgentService extends Disposable implements IAgentService { kind: ResponsePartKind.ToolCall, toolCall: tc, }); - } else if (msg.type === 'message' && msg.role === 'assistant' && msg.content) { - responseParts.push({ - kind: ResponsePartKind.Markdown, - id: generateUuid(), - content: msg.content, - }); + } else if (msg.type === 'message' && msg.role === 'assistant') { + if (msg.reasoningText) { + responseParts.push({ + kind: ResponsePartKind.Reasoning, + id: generateUuid(), + content: msg.reasoningText, + }); + } + if (msg.content) { + responseParts.push({ + kind: ResponsePartKind.Markdown, + id: generateUuid(), + content: msg.content, + }); + } } } diff --git a/src/vs/platform/agentHost/node/copilot/copilotAgent.ts b/src/vs/platform/agentHost/node/copilot/copilotAgent.ts index de102fdedde7f..4e69d0bb97c79 100644 --- a/src/vs/platform/agentHost/node/copilot/copilotAgent.ts +++ b/src/vs/platform/agentHost/node/copilot/copilotAgent.ts @@ -23,7 +23,7 @@ import { IFileService } from '../../../files/common/files.js'; import { IInstantiationService } from '../../../instantiation/common/instantiation.js'; import { ILogService } from '../../../log/common/log.js'; import { IAgentPluginManager, ISyncedCustomization } from '../../common/agentPluginManager.js'; -import { AgentSession, IAgent, IAgentAttachment, IAgentCreateSessionConfig, IAgentCreateSessionResult, IAgentDescriptor, IAgentDeltaEvent, IAgentMessageEvent, IAgentModelInfo, IAgentProgressEvent, IAgentResolveSessionConfigParams, IAgentSessionConfigCompletionsParams, IAgentSessionMetadata, IAgentSessionProjectInfo, IAgentSubagentStartedEvent, IAgentToolCompleteEvent, IAgentToolStartEvent } from '../../common/agentService.js'; +import { AgentSession, IAgent, IAgentAttachment, IAgentCreateSessionConfig, IAgentCreateSessionResult, IAgentDescriptor, IAgentDeltaEvent, IAgentMessageEvent, IAgentModelInfo, IAgentProgressEvent, IAgentResolveSessionConfigParams, IAgentSessionConfigCompletionsParams, IAgentSessionMetadata, IAgentSessionProjectInfo, SessionHistoryEvent } from '../../common/agentService.js'; import { AutoApproveLevel, ISchemaProperty, createSchema, platformSessionSchema, schemaProperty } from '../../common/agentHostSchema.js'; import { SessionConfigKey } from '../../common/sessionConfigKeys.js'; import { ISessionDataService, SESSION_DB_FILENAME } from '../../common/sessionDataService.js'; @@ -121,7 +121,7 @@ function buildWorktreeAnnouncementText(branchName: string): string { ) + '\n\n'; } -type AgentMessageOrEvent = IAgentMessageEvent | IAgentToolStartEvent | IAgentToolCompleteEvent | IAgentSubagentStartedEvent; +type AgentMessageOrEvent = SessionHistoryEvent; /** * Returns a copy of `messages` where `announcement` has been prepended to @@ -727,7 +727,7 @@ export class CopilotAgent extends Disposable implements IAgent { // No SDK-level enqueue is needed. } - async getSessionMessages(session: URI): Promise<(IAgentMessageEvent | IAgentToolStartEvent | IAgentToolCompleteEvent | IAgentSubagentStartedEvent)[]> { + async getSessionMessages(session: URI): Promise { const sessionId = AgentSession.id(session); const entry = this._sessions.get(sessionId) ?? await this._resumeSession(sessionId).catch(err => { this._logService.warn(`[Copilot:${sessionId}] Failed to resume session for message lookup`, err); diff --git a/src/vs/platform/agentHost/node/copilot/copilotAgentSession.ts b/src/vs/platform/agentHost/node/copilot/copilotAgentSession.ts index 5ff06ce1f3c39..e8575fab5c7d8 100644 --- a/src/vs/platform/agentHost/node/copilot/copilotAgentSession.ts +++ b/src/vs/platform/agentHost/node/copilot/copilotAgentSession.ts @@ -17,7 +17,7 @@ import { INativeEnvironmentService } from '../../../environment/common/environme import { IFileService } from '../../../files/common/files.js'; import { IInstantiationService } from '../../../instantiation/common/instantiation.js'; import { ILogService } from '../../../log/common/log.js'; -import { IAgentAttachment, IAgentMessageEvent, IAgentProgressEvent, IAgentSubagentStartedEvent, IAgentToolCompleteEvent, IAgentToolStartEvent } from '../../common/agentService.js'; +import { IAgentAttachment, IAgentProgressEvent, SessionHistoryEvent } from '../../common/agentService.js'; import { stripRedundantCdPrefix } from '../../common/commandLineHelpers.js'; import { ISessionDatabase, ISessionDataService } from '../../common/sessionDataService.js'; import type { FileEdit, ToolDefinition } from '../../common/state/protocol/state.js'; @@ -331,7 +331,7 @@ export class CopilotAgentSession extends Disposable { } } - async getMessages(): Promise<(IAgentMessageEvent | IAgentToolStartEvent | IAgentToolCompleteEvent | IAgentSubagentStartedEvent)[]> { + async getMessages(): Promise { const events = await this._wrapper.session.getMessages(); let db: ISessionDatabase | undefined; try { diff --git a/src/vs/platform/agentHost/node/copilot/mapSessionEvents.ts b/src/vs/platform/agentHost/node/copilot/mapSessionEvents.ts index 551ff04b11346..706b72969042a 100644 --- a/src/vs/platform/agentHost/node/copilot/mapSessionEvents.ts +++ b/src/vs/platform/agentHost/node/copilot/mapSessionEvents.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import { URI } from '../../../../base/common/uri.js'; -import { IAgentMessageEvent, IAgentSubagentStartedEvent, IAgentToolCompleteEvent, IAgentToolStartEvent } from '../../common/agentService.js'; +import { SessionHistoryEvent } from '../../common/agentService.js'; import { stripRedundantCdPrefix } from '../../common/commandLineHelpers.js'; import { IFileEditRecord, ISessionDatabase } from '../../common/sessionDataService.js'; import { ToolResultContentType, type ToolResultContent } from '../../common/state/sessionState.js'; @@ -90,8 +90,8 @@ export async function mapSessionEvents( db: ISessionDatabase | undefined, events: readonly ISessionEvent[], workingDirectory?: URI, -): Promise<(IAgentMessageEvent | IAgentToolStartEvent | IAgentToolCompleteEvent | IAgentSubagentStartedEvent)[]> { - const result: (IAgentMessageEvent | IAgentToolStartEvent | IAgentToolCompleteEvent | IAgentSubagentStartedEvent)[] = []; +): Promise { + const result: SessionHistoryEvent[] = []; const toolInfoByCallId = new Map | undefined; rewrittenArgs?: string }>(); // Collect all tool call IDs for edit tools so we can batch-query the database diff --git a/src/vs/platform/agentHost/test/node/agentService.test.ts b/src/vs/platform/agentHost/test/node/agentService.test.ts index c847ffbf14df2..fb82ce83ce6c0 100644 --- a/src/vs/platform/agentHost/test/node/agentService.test.ts +++ b/src/vs/platform/agentHost/test/node/agentService.test.ts @@ -375,6 +375,40 @@ suite('AgentService (node dispatcher)', () => { assert.strictEqual(tc.confirmed, ToolCallConfirmationReason.NotNeeded); }); + test('interleaves reasoning, markdown, and tool calls in stream order on resume', async () => { + service.registerProvider(copilotAgent); + const { session } = await copilotAgent.createSession(); + const sessions = await copilotAgent.listSessions(); + const sessionResource = sessions[0].session; + + copilotAgent.sessionMessages = [ + { type: 'message', session, role: 'user', messageId: 'u-1', content: 'Hello', toolRequests: [] }, + { type: 'message', session, role: 'assistant', messageId: 'a-1', content: 'Reply A', reasoningText: 'Thinking A', toolRequests: [{ toolCallId: 'tc-1', name: 'shell' }] }, + { type: 'tool_start', session, toolCallId: 'tc-1', toolName: 'shell', displayName: 'Shell', invocationMessage: 'Running...' }, + { type: 'tool_complete', session, toolCallId: 'tc-1', result: { success: true, pastTenseMessage: 'Ran', content: [{ type: ToolResultContentType.Text, text: 'ok' }] } }, + { type: 'message', session, role: 'assistant', messageId: 'a-2', content: 'Reply B', reasoningText: 'Thinking B', toolRequests: [] }, + ]; + + await service.restoreSession(sessionResource); + + const state = service.stateManager.getSessionState(sessionResource.toString()); + assert.ok(state); + const turn = state!.turns[0]; + const summary = turn.responseParts.map(p => { + if (p.kind === ResponsePartKind.Reasoning) { return ['reasoning', p.content]; } + if (p.kind === ResponsePartKind.Markdown) { return ['markdown', p.content]; } + if (p.kind === ResponsePartKind.ToolCall) { return ['toolCall', p.toolCall.toolCallId]; } + return ['other']; + }); + assert.deepStrictEqual(summary, [ + ['reasoning', 'Thinking A'], + ['markdown', 'Reply A'], + ['toolCall', 'tc-1'], + ['reasoning', 'Thinking B'], + ['markdown', 'Reply B'], + ]); + }); + test('flushes interrupted turns', async () => { service.registerProvider(copilotAgent); const { session } = await copilotAgent.createSession(); diff --git a/src/vs/platform/agentHost/test/node/mockAgent.ts b/src/vs/platform/agentHost/test/node/mockAgent.ts index dab37c38259d9..1388b069ebcd2 100644 --- a/src/vs/platform/agentHost/test/node/mockAgent.ts +++ b/src/vs/platform/agentHost/test/node/mockAgent.ts @@ -9,7 +9,7 @@ import { observableValue } from '../../../../base/common/observable.js'; import type { IAuthorizationProtectedResourceMetadata } from '../../../../base/common/oauth.js'; import { URI } from '../../../../base/common/uri.js'; import { type ISyncedCustomization } from '../../common/agentPluginManager.js'; -import { AgentSession, type AgentProvider, type IAgent, type IAgentAttachment, type IAgentCreateSessionConfig, type IAgentCreateSessionResult, type IAgentDescriptor, type IAgentMessageEvent, type IAgentModelInfo, type IAgentProgressEvent, type IAgentResolveSessionConfigParams, type IAgentSessionConfigCompletionsParams, type IAgentSessionMetadata, type IAgentSubagentStartedEvent, type IAgentToolCompleteEvent, type IAgentToolStartEvent } from '../../common/agentService.js'; +import { AgentSession, type AgentProvider, type IAgent, type IAgentAttachment, type IAgentCreateSessionConfig, type IAgentCreateSessionResult, type IAgentDescriptor, type IAgentModelInfo, type IAgentProgressEvent, type IAgentResolveSessionConfigParams, type IAgentSessionConfigCompletionsParams, type IAgentSessionMetadata, type SessionHistoryEvent } from '../../common/agentService.js'; import { ProtectedResourceMetadata, type ModelSelection } from '../../common/state/protocol/state.js'; import type { ResolveSessionConfigResult, SessionConfigCompletionsResult } from '../../common/state/protocol/commands.js'; import { CustomizationStatus, ToolResultContentType, type CustomizationRef, type PendingMessage, type ToolCallResult } from '../../common/state/sessionState.js'; @@ -48,7 +48,7 @@ export class MockAgent implements IAgent { customizations: CustomizationRef[] = []; /** Configurable return value for getSessionMessages. */ - sessionMessages: (IAgentMessageEvent | IAgentToolStartEvent | IAgentToolCompleteEvent | IAgentSubagentStartedEvent)[] = []; + sessionMessages: SessionHistoryEvent[] = []; /** Optional overrides applied to session metadata from listSessions. */ sessionMetadataOverrides: Partial> = {}; @@ -100,7 +100,7 @@ export class MockAgent implements IAgent { this.setPendingMessagesCalls.push({ session, steeringMessage, queuedMessages }); } - async getSessionMessages(_session: URI): Promise<(IAgentMessageEvent | IAgentToolStartEvent | IAgentToolCompleteEvent | IAgentSubagentStartedEvent)[]> { + async getSessionMessages(_session: URI): Promise { return this.sessionMessages; } @@ -190,7 +190,7 @@ export class ScriptedMockAgent implements IAgent { * Message history for the pre-existing session: a single user→assistant * turn with a tool call. */ - private readonly _preExistingMessages: (IAgentMessageEvent | IAgentToolStartEvent | IAgentToolCompleteEvent)[] = [ + private readonly _preExistingMessages: SessionHistoryEvent[] = [ { type: 'message', role: 'user', session: PRE_EXISTING_SESSION_URI, messageId: 'h-msg-1', content: 'What files are here?' }, { type: 'tool_start', session: PRE_EXISTING_SESSION_URI, toolCallId: 'h-tc-1', toolName: 'list_files', displayName: 'List Files', invocationMessage: 'Listing files...' }, { type: 'tool_complete', session: PRE_EXISTING_SESSION_URI, toolCallId: 'h-tc-1', result: { pastTenseMessage: 'Listed files', content: [{ type: ToolResultContentType.Text, text: 'file1.ts\nfile2.ts' }], success: true } satisfies ToolCallResult }, @@ -611,7 +611,7 @@ export class ScriptedMockAgent implements IAgent { } } - async getSessionMessages(session: URI): Promise<(IAgentMessageEvent | IAgentToolStartEvent | IAgentToolCompleteEvent)[]> { + async getSessionMessages(session: URI): Promise { if (session.toString() === PRE_EXISTING_SESSION_URI.toString()) { return this._preExistingMessages; } From 0cb2f1d44640817dc625b9e407d9376125f473b4 Mon Sep 17 00:00:00 2001 From: Bhavya U Date: Sat, 25 Apr 2026 17:00:29 -0700 Subject: [PATCH 2/8] Fix empty detailedOutcome in summarization failure telemetry (#312552) Fall back to response.type when response.reason is undefined so detailedOutcome is always populated for non-success outcomes. --- .../prompts/node/agent/summarizedConversationHistory.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/extensions/copilot/src/extension/prompts/node/agent/summarizedConversationHistory.tsx b/extensions/copilot/src/extension/prompts/node/agent/summarizedConversationHistory.tsx index 7e38db57951e0..bb87c144f1e14 100644 --- a/extensions/copilot/src/extension/prompts/node/agent/summarizedConversationHistory.tsx +++ b/extensions/copilot/src/extension/prompts/node/agent/summarizedConversationHistory.tsx @@ -778,8 +778,8 @@ class ConversationHistorySummarizer { private async handleSummarizationResponse(response: ChatResponse, mode: SummaryMode, elapsedTime: number, promptTypes?: string): Promise> { if (response.type !== ChatFetchResponseType.Success) { const outcome = response.type; - this.sendSummarizationTelemetry(outcome, response.requestId, this.props.endpoint.model, mode, elapsedTime, undefined, response.reason); - this.logInfo(`Summarization request failed. ${response.type} ${response.reason}`, mode); + this.sendSummarizationTelemetry(outcome, response.requestId, this.props.endpoint.model, mode, elapsedTime, undefined, response.reason ?? response.type); + this.logInfo(`Summarization request failed. ${response.type} ${response.reason ?? response.type}`, mode); if (response.type === ChatFetchResponseType.Canceled) { throw new CancellationError(); } From 13e28599a96b80e86768459cf3f5cce5b8e31572 Mon Sep 17 00:00:00 2001 From: Rob Lourens Date: Sat, 25 Apr 2026 18:45:12 -0700 Subject: [PATCH 3/8] Cache listSessions result in AgentHostSessionListController (#312563) * Cache listSessions result in AgentHostSessionListController The workbench-side session list controller previously called listSessions() on every refresh, even though it already maintains an in-memory cache that's kept in sync via notify/sessionAdded, notify/sessionRemoved, and notify/sessionSummaryChanged. refresh() is fired on workspace folder change, trust change, availability change, and provider change, so it ran far more often than the underlying list actually changed. Add a _cacheValid flag, set on first successful listSessions(). After that, refresh() skips the RPC and just re-emits the cached items. Failures leave the cache invalid so the next refresh retries. Cache lifetime is tied to the controller, which is created per agent registration and disposed on connection replacement, so reconnect implicitly drops the cache. This matches the AHP rule that notifications are not replayed on reconnect, and brings the workbench controller in line with the analogous one-shot caching in BaseAgentHostSessionsProvider on the sessions-app side. (Written by Copilot) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Invalidate session list cache on agent host restart After a listSessions() success the cache is valid and refresh() skips the RPC. But since AHP notifications are not replayed on reconnect, the in-memory list would be stale after an agent host restart if the cache were never reset. Add resetCache() on AgentHostSessionListController, and wire it to onAgentHostStart in AgentHostContribution so the next refresh() following a restart re-fetches via listSessions(). Also make the MockAgentHostService.onAgentHostStart fireable in tests and add a test covering the reset-on-restart path. (Written by Copilot) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../agentHost/agentHostChatContribution.ts | 9 +++ .../agentHostSessionListController.ts | 24 ++++++ .../agentHostChatContribution.test.ts | 77 ++++++++++++++++++- 3 files changed, 109 insertions(+), 1 deletion(-) diff --git a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostChatContribution.ts b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostChatContribution.ts index 557129199caf2..6f1c9568a0f90 100644 --- a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostChatContribution.ts +++ b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostChatContribution.ts @@ -55,6 +55,8 @@ export class AgentHostContribution extends Disposable implements IWorkbenchContr private readonly _agentRegistrations = this._register(new DisposableMap()); /** Model providers keyed by agent provider, for pushing model updates. */ private readonly _modelProviders = new Map(); + /** List controllers keyed by agent provider, for cache resets on reconnect. */ + private readonly _listControllers = new Map(); /** Dedupes redundant `authenticate` RPCs when the resolved token hasn't changed. */ private readonly _authTokenCache = new AgentHostAuthTokenCache(); @@ -100,8 +102,13 @@ export class AgentHostContribution extends Disposable implements IWorkbenchContr // Clear the auth cache whenever the local agent host (re)starts so the // first post-restart authenticate RPC is never skipped as "unchanged". + // Also reset each list controller's session cache so the next refresh + // re-fetches via listSessions() rather than serving a stale in-memory list. this._register(this._agentHostService.onAgentHostStart(() => { this._authTokenCache.clear(); + for (const controller of this._listControllers.values()) { + controller.resetCache(); + } })); // Process initial root state if already available @@ -164,6 +171,8 @@ export class AgentHostContribution extends Disposable implements IWorkbenchContr // Session list controller const listController = store.add(this._instantiationService.createInstance(AgentHostSessionListController, sessionType, agent.provider, this._loggedConnection!, undefined, 'local')); + this._listControllers.set(agent.provider, listController); + store.add({ dispose: () => this._listControllers.delete(agent.provider) }); store.add(this._chatSessionsService.registerChatSessionItemController(sessionType, listController)); // Customization sync provider + bundler + observable diff --git a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSessionListController.ts b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSessionListController.ts index cb8e2d5fad5f1..55e5da5cbbf5f 100644 --- a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSessionListController.ts +++ b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSessionListController.ts @@ -76,6 +76,17 @@ export class AgentHostSessionListController extends Disposable implements IChatS private _items: IChatSessionItem[] = []; /** Cached full summaries per session so partial updates can be applied. */ private readonly _cachedSummaries = new Map(); + /** + * Once `listSessions()` has succeeded, the in-memory list is kept in + * sync by `notify/sessionAdded`, `notify/sessionRemoved`, and + * `notify/sessionSummaryChanged`. Subsequent `refresh()` calls then + * just re-emit the cached items instead of re-issuing the RPC. + * + * Lifetime: the controller is created per agent registration and + * disposed when the registration is torn down (e.g. on connection + * replacement), so this flag naturally resets on reconnect. + */ + private _cacheValid = false; constructor( private readonly _sessionType: string, @@ -132,11 +143,23 @@ export class AgentHostSessionListController extends Disposable implements IChatS })); } + /** Reset the list-sessions cache so the next {@link refresh} re-fetches from the agent host. */ + resetCache(): void { + this._cacheValid = false; + } + get items(): readonly IChatSessionItem[] { return this._items; } async refresh(_token: CancellationToken): Promise { + if (this._cacheValid) { + // Cache is kept in sync by notify/sessionAdded, + // notify/sessionRemoved, and notify/sessionSummaryChanged. No + // need to round-trip through the agent host on every refresh. + this._onDidChangeChatSessionItems.fire({ addedOrUpdated: this._items }); + return; + } try { const sessions = await this._connection.listSessions(); const filtered = sessions.filter(s => AgentSession.provider(s.session) === this._provider); @@ -168,6 +191,7 @@ export class AgentHostSessionListController extends Disposable implements IChatS diffs: s.diffs, }); }); + this._cacheValid = true; } catch { this._cachedSummaries.clear(); this._items = []; diff --git a/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostChatContribution.test.ts b/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostChatContribution.test.ts index 9f3fdcb020388..d5f5bd33f2c2d 100644 --- a/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostChatContribution.test.ts +++ b/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostChatContribution.test.ts @@ -62,7 +62,12 @@ class MockAgentHostService extends mock() { private readonly _onDidNotification = new Emitter(); override readonly onDidNotification = this._onDidNotification.event; override readonly onAgentHostExit = Event.None; - override readonly onAgentHostStart = Event.None; + private readonly _onAgentHostStart = new Emitter(); + override readonly onAgentHostStart = this._onAgentHostStart.event; + + fireAgentHostStart(): void { + this._onAgentHostStart.fire(); + } private readonly _authenticationPending: ISettableObservable = observableValue('authenticationPending', false); override readonly authenticationPending: IObservable = this._authenticationPending; @@ -667,6 +672,76 @@ suite('AgentHostChatContribution', () => { assert.strictEqual(listController.items.length, 1); assert.strictEqual(listController.items[0].archived, true); }); + + test('refresh skips listSessions RPC after first successful call', async () => { + const { listController, agentHostService } = createContribution(disposables); + + agentHostService.addSession({ session: AgentSession.uri('copilot', 'aaa'), startTime: 1000, modifiedTime: 2000, summary: 'My session' }); + + let listCalls = 0; + const originalListSessions = agentHostService.listSessions.bind(agentHostService); + agentHostService.listSessions = async () => { listCalls++; return originalListSessions(); }; + + await listController.refresh(CancellationToken.None); + assert.strictEqual(listCalls, 1); + assert.strictEqual(listController.items.length, 1); + + // Subsequent refresh should not re-fetch — the cache is kept in + // sync via notify/sessionAdded etc. + await listController.refresh(CancellationToken.None); + await listController.refresh(CancellationToken.None); + assert.strictEqual(listCalls, 1); + assert.strictEqual(listController.items.length, 1); + }); + + test('refresh retries listSessions if the first call failed', async () => { + const { listController, agentHostService } = createContribution(disposables); + + let listCalls = 0; + const originalListSessions = agentHostService.listSessions.bind(agentHostService); + agentHostService.listSessions = async () => { + listCalls++; + if (listCalls === 1) { + throw new Error('fail'); + } + return originalListSessions(); + }; + + agentHostService.addSession({ session: AgentSession.uri('copilot', 'aaa'), startTime: 1000, modifiedTime: 2000, summary: 'My session' }); + + await listController.refresh(CancellationToken.None); + assert.strictEqual(listCalls, 1); + assert.strictEqual(listController.items.length, 0); + + // Failure must not mark the cache valid; the next refresh retries. + await listController.refresh(CancellationToken.None); + assert.strictEqual(listCalls, 2); + assert.strictEqual(listController.items.length, 1); + }); + + test('agent host restart invalidates cache so next refresh re-fetches', async () => { + const { listController, agentHostService } = createContribution(disposables); + + agentHostService.addSession({ session: AgentSession.uri('copilot', 'aaa'), startTime: 1000, modifiedTime: 2000, summary: 'Before restart' }); + + let listCalls = 0; + const originalListSessions = agentHostService.listSessions.bind(agentHostService); + agentHostService.listSessions = async () => { listCalls++; return originalListSessions(); }; + + await listController.refresh(CancellationToken.None); + assert.strictEqual(listCalls, 1); + + // Subsequent refresh uses cache — no new RPC. + await listController.refresh(CancellationToken.None); + assert.strictEqual(listCalls, 1); + + // Directly resetting the cache (as onAgentHostStart does) must cause + // the next refresh to re-fetch. + listController.resetCache(); + + await listController.refresh(CancellationToken.None); + assert.strictEqual(listCalls, 2); + }); }); // ---- Session ID resolution in _invokeAgent -------------------------- From 7ac607f6a9505ec4f988541433147386adb0be8f Mon Sep 17 00:00:00 2001 From: Rob Lourens Date: Sat, 25 Apr 2026 18:46:03 -0700 Subject: [PATCH 4/8] Render copilot skill invocations as a tool call (#312557) * Render copilot skill invocations as a tool call The Copilot SDK now exposes a skill tool that the model uses to load SKILL.md files. Until now this surfaced in the agent host as the default tool UI, which doesn't fit. This change does three things: - Hide the raw `skill` tool call (added to HIDDEN_TOOL_NAMES). - Synthesize a `tool_start`/`tool_complete` pair from the SDK's `skill.invoked` lifecycle event so we can show 'Reading [name]' with a link to the skill file. Wired in both the live event path and the history-replay path so reconnect renders identically. - Filter out the synthetic `user.message` events the SDK sometimes injects with skill content (data.source !== 'user'). Client side, the chat adapter now tags markdown links pointing at a SKILL.md file with `?vscodeLinkType=skill` so they render as the inline skill pill instead of a plain markdown anchor. Doing this on the client (rather than at the agent host) keeps the agent host host-agnostic and also upgrades older sessions and other providers that emit SKILL.md links. The link rewriter also now preserves the original link text rather than always collapsing to `[](newHref)`, so a skill named 'plan' shows up as 'Reading [plan]' rather than 'Reading [SKILL.md]'. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address review comments and fix CI snapshots - Add escapeMarkdownLinkLabel helper in htmlContent.ts: only escapes '\' and ']' so link labels rendered without re-parsing markdown (e.g. the chat skill pill) don't show literal backslashes for safe characters like '-' or '.'. - Use it in synthesizeSkillToolEvents and stateToProgressAdapter's rewriteLinkTokenRaw instead of the over-eager escapeMarkdownSyntaxTokens. - Hash the path/name fallback in getSkillSyntheticToolCallId so the synthesized toolCallId never contains '/' (which breaks ChatResponseResource.createUri paths). - Update copilotToolDisplay/mapSessionEvents test snapshots to match the actual emitted strings ('Reading skill [plan](...)' / 'Read skill [plan](...)') and the new hash-based fallback id. - Add htmlContent unit tests for escapeMarkdownLinkLabel. (Written by Copilot) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/vs/base/common/htmlContent.ts | 12 +++ src/vs/base/test/common/htmlContent.test.ts | 23 ++++- .../node/copilot/copilotAgentSession.ts | 13 ++- .../node/copilot/copilotToolDisplay.ts | 98 ++++++++++++++++++- .../node/copilot/mapSessionEvents.ts | 45 ++++++++- .../test/node/copilotToolDisplay.test.ts | 48 ++++++++- .../test/node/mapSessionEvents.test.ts | 62 ++++++++++++ .../agentHost/stateToProgressAdapter.ts | 43 ++++++-- .../stateToProgressAdapter.test.ts | 12 +-- 9 files changed, 335 insertions(+), 21 deletions(-) diff --git a/src/vs/base/common/htmlContent.ts b/src/vs/base/common/htmlContent.ts index a9b07f6e0b83e..31acfa88384dc 100644 --- a/src/vs/base/common/htmlContent.ts +++ b/src/vs/base/common/htmlContent.ts @@ -155,6 +155,18 @@ export function escapeMarkdownSyntaxTokens(text: string): string { return text.replace(/[\\`*_{}[\]()#+\-!~]/g, '\\$&'); // CodeQL [SM02383] Backslash is escaped in the character class } +/** + * Escapes only the characters that would break out of markdown link text + * (`[label](url)`) syntax: `\` and `]`. Use this when the escaped string is + * displayed as the visible label of a link, since renderers that extract the + * link text without re-parsing markdown (e.g. the chat inline anchor / skill + * pill) would otherwise show full `escapeMarkdownSyntaxTokens` backslashes + * (`\-`, `\.`, ...) verbatim. + */ +export function escapeMarkdownLinkLabel(text: string): string { + return text.replace(/[\\\]]/g, '\\$&'); +} + /** * @see https://github.com/microsoft/vscode/issues/193746 */ diff --git a/src/vs/base/test/common/htmlContent.test.ts b/src/vs/base/test/common/htmlContent.test.ts index 82a84971a523c..e3894cd19dc05 100644 --- a/src/vs/base/test/common/htmlContent.test.ts +++ b/src/vs/base/test/common/htmlContent.test.ts @@ -3,7 +3,7 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ import assert from 'assert'; -import { appendEscapedMarkdownInlineCode } from '../../common/htmlContent.js'; +import { appendEscapedMarkdownInlineCode, escapeMarkdownLinkLabel } from '../../common/htmlContent.js'; import { ensureNoDisposablesAreLeakedInTestSuite } from './utils.js'; suite('htmlContent', () => { @@ -37,4 +37,25 @@ suite('htmlContent', () => { assert.strictEqual(appendEscapedMarkdownInlineCode('``'), '``` `` ```'); }); }); + + suite('escapeMarkdownLinkLabel', () => { + test('passes plain text through unchanged', () => { + assert.strictEqual(escapeMarkdownLinkLabel('hello'), 'hello'); + assert.strictEqual(escapeMarkdownLinkLabel(''), ''); + assert.strictEqual(escapeMarkdownLinkLabel('heap-snapshot-analysis'), 'heap-snapshot-analysis'); + assert.strictEqual(escapeMarkdownLinkLabel('foo.bar_baz'), 'foo.bar_baz'); + }); + + test('escapes only `\\` and `]`', () => { + assert.strictEqual(escapeMarkdownLinkLabel('a]b'), 'a\\]b'); + assert.strictEqual(escapeMarkdownLinkLabel('a\\b'), 'a\\\\b'); + assert.strictEqual(escapeMarkdownLinkLabel(']]'), '\\]\\]'); + }); + + test('does not escape characters that are safe in link text', () => { + // these would be escaped by escapeMarkdownSyntaxTokens but must + // pass through here since they render literally inside `[...]`. + assert.strictEqual(escapeMarkdownLinkLabel('a*b_c#d-e.f!g~h+i(j)k{l}m'), 'a*b_c#d-e.f!g~h+i(j)k{l}m'); + }); + }); }); diff --git a/src/vs/platform/agentHost/node/copilot/copilotAgentSession.ts b/src/vs/platform/agentHost/node/copilot/copilotAgentSession.ts index e8575fab5c7d8..23cc1ce9131e4 100644 --- a/src/vs/platform/agentHost/node/copilot/copilotAgentSession.ts +++ b/src/vs/platform/agentHost/node/copilot/copilotAgentSession.ts @@ -24,7 +24,7 @@ import type { FileEdit, ToolDefinition } from '../../common/state/protocol/state import { SessionInputAnswerState, SessionInputAnswerValueKind, SessionInputQuestionKind, SessionInputResponseKind, ToolResultContentType, type PendingMessage, type SessionInputAnswer, type SessionInputRequest, type ToolCallResult, type ToolResultContent } from '../../common/state/sessionState.js'; import { CopilotSessionWrapper } from './copilotSessionWrapper.js'; import type { ShellManager } from './copilotShellTools.js'; -import { getEditFilePath, getInvocationMessage, getPastTenseMessage, getPermissionDisplay, getShellLanguage, getSubagentMetadata, getToolDisplayName, getToolInputString, getToolKind, isEditTool, isHiddenTool, isShellTool, tryStringify, type ITypedPermissionRequest } from './copilotToolDisplay.js'; +import { getEditFilePath, getInvocationMessage, getPastTenseMessage, getPermissionDisplay, getShellLanguage, getSubagentMetadata, getToolDisplayName, getToolInputString, getToolKind, isEditTool, isHiddenTool, isShellTool, synthesizeSkillToolEvents, tryStringify, type ITypedPermissionRequest } from './copilotToolDisplay.js'; import { FileEditTracker } from './fileEditTracker.js'; import { mapSessionEvents } from './mapSessionEvents.js'; import { buildPendingEditContentUri } from './pendingEditContentStore.js'; @@ -794,6 +794,17 @@ export class CopilotAgentSession extends Disposable { this._onDidSessionProgress.fire({ session, type: 'idle' }); })); + // The SDK emits a `skill` tool call (which we hide) and a richer + // `skill.invoked` event with the resolved SKILL.md path. Synthesize a + // tool-start/complete pair from the latter so the UI can render a + // clickable file link, matching the `view`-tool display style. + this._register(wrapper.onSkillInvoked(e => { + this._logService.info(`[Copilot:${sessionId}] Skill invoked: ${e.data.name} (${e.data.path})`); + const { start, complete } = synthesizeSkillToolEvents(session, e.data, e.id); + this._onDidSessionProgress.fire(start); + this._onDidSessionProgress.fire(complete); + })); + this._register(wrapper.onSubagentStarted(e => { this._logService.info(`[Copilot:${sessionId}] Subagent started: toolCallId=${e.data.toolCallId}, agent=${e.data.agentName}`); this._onDidSessionProgress.fire({ diff --git a/src/vs/platform/agentHost/node/copilot/copilotToolDisplay.ts b/src/vs/platform/agentHost/node/copilot/copilotToolDisplay.ts index a1428617afff2..2d061529b7601 100644 --- a/src/vs/platform/agentHost/node/copilot/copilotToolDisplay.ts +++ b/src/vs/platform/agentHost/node/copilot/copilotToolDisplay.ts @@ -6,9 +6,10 @@ import type { PermissionRequest } from '@github/copilot-sdk'; import { hasKey } from '../../../../base/common/types.js'; import { URI } from '../../../../base/common/uri.js'; -import { appendEscapedMarkdownInlineCode } from '../../../../base/common/htmlContent.js'; +import { appendEscapedMarkdownInlineCode, escapeMarkdownLinkLabel } from '../../../../base/common/htmlContent.js'; +import { hash } from '../../../../base/common/hash.js'; import { localize } from '../../../../nls.js'; -import type { IAgentToolReadyEvent } from '../../common/agentService.js'; +import type { IAgentToolCompleteEvent, IAgentToolReadyEvent, IAgentToolStartEvent } from '../../common/agentService.js'; import { stripRedundantCdPrefix } from '../../common/commandLineHelpers.js'; import { StringOrMarkdown } from '../../common/state/protocol/state.js'; import { basename } from '../../../../base/common/resources.js'; @@ -55,6 +56,7 @@ const enum CopilotToolName { WebFetch = 'web_fetch', AskUser = 'ask_user', ReportIntent = 'report_intent', + Skill = 'skill', } /** Parameters for the `bash` / `powershell` shell tools. */ @@ -170,9 +172,16 @@ const SUBAGENT_TOOL_NAMES: ReadonlySet = new Set([ /** * Tools that should not be shown to the user. These are internal tools * used by the CLI for its own purposes (e.g., reporting intent to the model). + * + * `skill` is hidden because the SDK already emits a richer `skill.invoked` + * lifecycle event with the resolved skill file path; the agent session + * synthesizes a tool-start/complete pair from that event so the UI can + * render a clickable file link instead of just the skill name. See + * {@link synthesizeSkillToolEvents}. */ const HIDDEN_TOOL_NAMES: ReadonlySet = new Set([ CopilotToolName.ReportIntent, + CopilotToolName.Skill, ]); /** @@ -398,6 +407,91 @@ export function getPastTenseMessage(toolName: string, displayName: string, param } } +// ============================================================================= +// Skill event synthesis +// +// The Copilot SDK emits a `skill` tool call (which we hide) and, separately, a +// `skill.invoked` lifecycle event with the resolved skill file path. We turn +// the latter into a synthesized tool-start/complete pair so clients can render +// a clickable file link to the SKILL.md the agent loaded -- matching the +// existing `view`-tool display style. Live and replay paths share this helper +// so they stay in lock-step (see also the mirrored-pair gotcha for tool-call +// display in this file). +// ============================================================================= + +/** Subset of the SDK's `skill.invoked` payload that the synth helper needs. */ +export interface ICopilotSkillInvokedData { + readonly name: string; + readonly path?: string; + readonly description?: string; +} + +/** + * Builds a stable synthetic tool call id for a `skill.invoked` event so + * reconnect/replay produces the same id as the original live emit. The id + * is used unencoded as a path segment (e.g. by `ChatResponseResource.createUri`), + * so it must not contain characters like `/` -- we hash any fallback values + * that could carry filesystem paths or arbitrary text. + */ +export function getSkillSyntheticToolCallId(eventId: string | undefined, data: ICopilotSkillInvokedData): string { + if (eventId) { + return `synth-skill-${eventId}`; + } + const seed = data.path ?? data.name; + return `synth-skill-${hash(seed).toString(16)}`; +} + +/** + * Synthesizes the `tool_start` and `tool_complete` agent progress events that + * represent a successful `skill.invoked` lifecycle event. Used by both the + * live session handler and the history-replay mapper so the two paths render + * identically. + */ +export function synthesizeSkillToolEvents( + session: URI, + data: ICopilotSkillInvokedData, + eventId: string | undefined, +): { start: IAgentToolStartEvent; complete: IAgentToolCompleteEvent } { + const toolCallId = getSkillSyntheticToolCallId(eventId, data); + const displayName = localize('toolName.skill', "Read Skill"); + // Use the skill name as the link text rather than the basename: every skill + // file is named SKILL.md, so `Reading skill [plan]` reads better than the + // always-identical `Reading skill [SKILL.md]`. The client may further upgrade + // this link to a rich pill based on the `SKILL.md` basename. Skill names and + // paths come from the SDK / agent host and are escaped to prevent markdown + // injection from a malicious skill author. + // Escape only the characters that would break out of markdown link text + // syntax (`\` and `]`); a full markdown escape would leave visible + // backslashes in renderers (like the skill pill) that extract link text + // without re-parsing markdown. + const escapedName = escapeMarkdownLinkLabel(data.name); + const skillLink = data.path ? `[${escapedName}](${URI.file(data.path)})` : undefined; + const invocationMessage: StringOrMarkdown = skillLink + ? md(localize('toolInvoke.skill', "Reading skill {0}", skillLink)) + : localize('toolInvoke.skillName', "Reading skill {0}", data.name); + const pastTenseMessage: StringOrMarkdown = skillLink + ? md(localize('toolComplete.skill', "Read skill {0}", skillLink)) + : localize('toolComplete.skillName', "Read skill {0}", data.name); + const start: IAgentToolStartEvent = { + session, + type: 'tool_start', + toolCallId, + toolName: CopilotToolName.Skill, + displayName, + invocationMessage, + }; + const complete: IAgentToolCompleteEvent = { + session, + type: 'tool_complete', + toolCallId, + result: { + success: true, + pastTenseMessage, + }, + }; + return { start, complete }; +} + export function getToolInputString(toolName: string, parameters: Record | undefined, rawArguments: string | undefined): string | undefined { if (!parameters && !rawArguments) { return undefined; diff --git a/src/vs/platform/agentHost/node/copilot/mapSessionEvents.ts b/src/vs/platform/agentHost/node/copilot/mapSessionEvents.ts index 706b72969042a..5a306612eb492 100644 --- a/src/vs/platform/agentHost/node/copilot/mapSessionEvents.ts +++ b/src/vs/platform/agentHost/node/copilot/mapSessionEvents.ts @@ -8,7 +8,7 @@ import { SessionHistoryEvent } from '../../common/agentService.js'; import { stripRedundantCdPrefix } from '../../common/commandLineHelpers.js'; import { IFileEditRecord, ISessionDatabase } from '../../common/sessionDataService.js'; import { ToolResultContentType, type ToolResultContent } from '../../common/state/sessionState.js'; -import { getInvocationMessage, getPastTenseMessage, getShellLanguage, getSubagentMetadata, getToolDisplayName, getToolInputString, getToolKind, isEditTool, isHiddenTool } from './copilotToolDisplay.js'; +import { getInvocationMessage, getPastTenseMessage, getShellLanguage, getSubagentMetadata, getToolDisplayName, getToolInputString, getToolKind, isEditTool, isHiddenTool, synthesizeSkillToolEvents } from './copilotToolDisplay.js'; import { buildSessionDbUri } from './fileEditTracker.js'; function tryStringify(value: unknown): string | undefined { @@ -58,11 +58,45 @@ export interface ISessionEventMessage { reasoningText?: string; encryptedContent?: string; parentToolCallId?: string; + /** + * Origin of this message. The SDK sets this to a non-`'user'` value + * (e.g. `'skill-pdf'`) for messages it injects on behalf of a skill or + * other internal mechanism. We filter those out so they don't render + * as user turns. + */ + source?: string; }; } +/** Minimal event shape for `skill.invoked`, used to synthesize a tool-style render. */ +export interface ISessionEventSkillInvoked { + type: 'skill.invoked'; + id?: string; + data: { + name: string; + path?: string; + description?: string; + }; +} + +/** + * Returns true if the event is a SDK-injected `user.message` that should not + * be shown to the user (e.g. skill-content injection). + * + * The SDK marks these via a non-`'user'` `source` field. Older sessions + * persisted before `source` existed will not be filtered; that is accepted + * leakage rather than guessed-at content sniffing. + */ +export function isSyntheticUserMessage(event: ISessionEvent): boolean { + if (event.type !== 'user.message') { + return false; + } + const source = (event as ISessionEventMessage).data?.source; + return !!source && source.toLowerCase() !== 'user'; +} + /** Minimal event shape for session history mapping. */ -export type ISessionEvent = ISessionEventToolStart | ISessionEventToolComplete | ISessionEventMessage | ISessionEventSubagentStarted | { type: string; data?: unknown }; +export type ISessionEvent = ISessionEventToolStart | ISessionEventToolComplete | ISessionEventMessage | ISessionEventSubagentStarted | ISessionEventSkillInvoked | { type: string; data?: unknown }; export interface ISessionEventSubagentStarted { type: 'subagent.started'; @@ -143,6 +177,9 @@ export async function mapSessionEvents( // Second pass: build result events for (const e of events) { if (e.type === 'assistant.message' || e.type === 'user.message') { + if (isSyntheticUserMessage(e)) { + continue; + } const d = (e as ISessionEventMessage).data; result.push({ session, @@ -253,6 +290,10 @@ export async function mapSessionEvents( agentDisplayName: d.agentDisplayName, agentDescription: d.agentDescription, }); + } else if (e.type === 'skill.invoked') { + const skillEvent = e as ISessionEventSkillInvoked; + const { start, complete } = synthesizeSkillToolEvents(session, skillEvent.data, skillEvent.id); + result.push(start, complete); } } return result; diff --git a/src/vs/platform/agentHost/test/node/copilotToolDisplay.test.ts b/src/vs/platform/agentHost/test/node/copilotToolDisplay.test.ts index 86d2c10c713ff..07d9471f38bca 100644 --- a/src/vs/platform/agentHost/test/node/copilotToolDisplay.test.ts +++ b/src/vs/platform/agentHost/test/node/copilotToolDisplay.test.ts @@ -6,7 +6,7 @@ import assert from 'assert'; import { URI } from '../../../../base/common/uri.js'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../base/test/common/utils.js'; -import { getInvocationMessage, getPastTenseMessage, getPermissionDisplay, getShellLanguage, getToolInputString, getToolKind, type ITypedPermissionRequest } from '../../node/copilot/copilotToolDisplay.js'; +import { getInvocationMessage, getPastTenseMessage, getPermissionDisplay, getShellLanguage, getToolInputString, getToolKind, isHiddenTool, synthesizeSkillToolEvents, type ITypedPermissionRequest } from '../../node/copilot/copilotToolDisplay.js'; suite('getPermissionDisplay — cd-prefix stripping', () => { @@ -293,3 +293,49 @@ suite('copilotToolDisplay — write_/read_ shell tools', () => { }); }); }); + +suite('skill events', () => { + + ensureNoDisposablesAreLeakedInTestSuite(); + + const session = URI.parse('agent://copilot/test'); + + test('hides the raw `skill` tool call and synthesizes a tool-start/complete pair from `skill.invoked`', () => { + const withPath = synthesizeSkillToolEvents( + session, + { name: 'plan', path: '/abs/repo/skills/plan/SKILL.md' }, + 'evt-123', + ); + const noPath = synthesizeSkillToolEvents( + session, + { name: 'plan' }, + undefined, + ); + + assert.deepStrictEqual({ + skillIsHidden: isHiddenTool('skill'), + withPathToolCallId: withPath.start.toolCallId, + withPathSameIdOnComplete: withPath.start.toolCallId === withPath.complete.toolCallId, + withPathToolName: withPath.start.toolName, + withPathDisplayName: withPath.start.displayName, + withPathInvocation: withPath.start.invocationMessage, + withPathPastTense: withPath.complete.result.pastTenseMessage, + withPathSuccess: withPath.complete.result.success, + noPathToolCallId: noPath.start.toolCallId, + noPathInvocation: noPath.start.invocationMessage, + noPathPastTense: noPath.complete.result.pastTenseMessage, + }, { + skillIsHidden: true, + withPathToolCallId: 'synth-skill-evt-123', + withPathSameIdOnComplete: true, + withPathToolName: 'skill', + withPathDisplayName: 'Read Skill', + withPathInvocation: { markdown: 'Reading skill [plan](file:///abs/repo/skills/plan/SKILL.md)' }, + withPathPastTense: { markdown: 'Read skill [plan](file:///abs/repo/skills/plan/SKILL.md)' }, + withPathSuccess: true, + noPathToolCallId: 'synth-skill-2108d652', + noPathInvocation: 'Reading skill plan', + noPathPastTense: 'Read skill plan', + }); + }); +}); diff --git a/src/vs/platform/agentHost/test/node/mapSessionEvents.test.ts b/src/vs/platform/agentHost/test/node/mapSessionEvents.test.ts index 046fde01db6fc..d5105061035fb 100644 --- a/src/vs/platform/agentHost/test/node/mapSessionEvents.test.ts +++ b/src/vs/platform/agentHost/test/node/mapSessionEvents.test.ts @@ -254,6 +254,68 @@ suite('mapSessionEvents', () => { }); }); + // ---- Skill events --------------------------------------------------- + + suite('skill events', () => { + + test('synthesizes tool start/complete from skill.invoked and filters synthetic skill-injected user messages', async () => { + const events: ISessionEvent[] = [ + { + type: 'tool.execution_start', + data: { toolCallId: 'tc-skill', toolName: 'skill', arguments: { skill: 'plan' } }, + }, + { + type: 'tool.execution_complete', + data: { toolCallId: 'tc-skill', success: true }, + }, + { + type: 'skill.invoked', + id: 'evt-42', + data: { name: 'plan', path: '/abs/repo/skills/plan/SKILL.md' }, + }, + { + type: 'user.message', + data: { messageId: 'msg-skill', content: '', source: 'skill-plan' }, + }, + { + type: 'assistant.message', + data: { messageId: 'msg-1', content: 'ok' }, + }, + ]; + + const result = await mapSessionEvents(session, undefined, events); + + assert.deepStrictEqual({ + count: result.length, + types: result.map(r => r.type), + skillStart: result[0], + skillComplete: result[1], + assistantRole: (result[2] as { role: string }).role, + }, { + count: 3, + types: ['tool_start', 'tool_complete', 'message'], + skillStart: { + session, + type: 'tool_start', + toolCallId: 'synth-skill-evt-42', + toolName: 'skill', + displayName: 'Read Skill', + invocationMessage: { markdown: 'Reading skill [plan](file:///abs/repo/skills/plan/SKILL.md)' }, + }, + skillComplete: { + session, + type: 'tool_complete', + toolCallId: 'synth-skill-evt-42', + result: { + success: true, + pastTenseMessage: { markdown: 'Read skill [plan](file:///abs/repo/skills/plan/SKILL.md)' }, + }, + }, + assistantRole: 'assistant', + }); + }); + }); + // ---- cd-prefix rewriting -------------------------------------------- suite('cd-prefix rewriting', () => { diff --git a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/stateToProgressAdapter.ts b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/stateToProgressAdapter.ts index d29664a24bd0c..0234eacc1e399 100644 --- a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/stateToProgressAdapter.ts +++ b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/stateToProgressAdapter.ts @@ -3,7 +3,7 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import { IMarkdownString, MarkdownString } from '../../../../../../base/common/htmlContent.js'; +import { escapeMarkdownLinkLabel, IMarkdownString, MarkdownString } from '../../../../../../base/common/htmlContent.js'; import { marked, type Token, type Tokens, type TokensList } from '../../../../../../base/common/marked/marked.js'; import { URI } from '../../../../../../base/common/uri.js'; import { ToolCallStatus, TurnState, ResponsePartKind, getToolFileEdits, getToolOutputText, getToolSubagentContent, type ActiveTurn, type ICompletedToolCall, type ToolCallState, type Turn, FileEditKind, ToolResultContentType, type ToolResultContent } from '../../../../../../platform/agentHost/common/state/sessionState.js'; @@ -411,11 +411,13 @@ export function rewriteMarkdownLinks(markdown: string, connectionAuthority: stri * or returns `undefined` if the token should be left alone (external * scheme or unparseable URI). * - * The output collapses to the canonical inline form `[](newHref)` (or - * `![](newHref)` for images) — the chat renderer has richer handling for - * empty-text agent-host links, so preserving the original label isn't - * useful. This also means autolinks (``) and reference-style links - * (`[text][ref]`) are normalized into the inline form. + * The output is the canonical inline form `[text](newHref)` (or + * `![text](newHref)` for images). When the original link has no display + * text the chat renderer falls back to its file-widget rendering, but a + * non-empty label (e.g. a skill name) is preserved so the renderer shows + * it instead of the URI's basename. This also means autolinks (``) + * and reference-style links (`[text][ref]`) are normalized into the + * inline form. */ function rewriteLinkTokenRaw(token: Tokens.Link | Tokens.Image, connectionAuthority: string): string | undefined { let parsed: URI; @@ -428,9 +430,34 @@ function rewriteLinkTokenRaw(token: Tokens.Link | Tokens.Image, connectionAuthor if (!scheme || EXTERNAL_LINK_SCHEMES.has(scheme)) { return undefined; } - const newHref = toAgentHostUri(parsed, connectionAuthority).toString(); + let agentHostUri = toAgentHostUri(parsed, connectionAuthority); + // VS-Code-specific: links pointing at a `SKILL.md` file are rendered as a + // rich skill pill rather than a plain markdown link. The chat renderer's + // inline anchor widget keys off the `vscodeLinkType` query parameter (see + // `chatInlineAnchorWidget.ts`), so we tag the URI here on the client side + // rather than at the agent host. We do this whether or not the link came + // in pre-tagged so older sessions and other agent providers also benefit. + if (isSkillFileUri(parsed) && !agentHostUri.query.includes('vscodeLinkType=')) { + const existing = agentHostUri.query; + agentHostUri = agentHostUri.with({ query: existing ? `${existing}&vscodeLinkType=skill` : 'vscodeLinkType=skill' }); + } const prefix = token.type === 'image' ? '![' : '['; - return `${prefix}](${newHref})`; + // Escape only the characters that would break out of markdown link text + // syntax (`\` and `]`). A full markdown escape would leave visible + // backslashes in renderers (like the skill pill) that extract link text + // without re-parsing markdown. + const text = escapeMarkdownLinkLabel(token.text ?? ''); + return `${prefix}${text}](${agentHostUri.toString()})`; +} + +/** + * Returns true when the URI's basename is `SKILL.md` (case-insensitive). + * Used to tag skill links so the chat renderer shows the rich skill pill + * instead of a plain markdown anchor. + */ +function isSkillFileUri(uri: URI): boolean { + const name = basename(uri); + return name.toLowerCase() === 'skill.md'; } /** diff --git a/src/vs/workbench/contrib/chat/test/browser/agentSessions/stateToProgressAdapter.test.ts b/src/vs/workbench/contrib/chat/test/browser/agentSessions/stateToProgressAdapter.test.ts index dcd44ed099333..1888763ae14ff 100644 --- a/src/vs/workbench/contrib/chat/test/browser/agentSessions/stateToProgressAdapter.test.ts +++ b/src/vs/workbench/contrib/chat/test/browser/agentSessions/stateToProgressAdapter.test.ts @@ -244,8 +244,8 @@ suite('stateToProgressAdapter', () => { if (response.type !== 'response') { return; } const part = response.parts[0] as IChatMarkdownContent; assert.deepStrictEqual(part.content.value, - 'See [](vscode-agent-host://my-host/file/-/a/b.ts), ' + - '[](vscode-agent-host://my-host/agenthost-content/-/s/x), ' + + 'See [local](vscode-agent-host://my-host/file/-/a/b.ts), ' + + '[content](vscode-agent-host://my-host/agenthost-content/-/s/x), ' + '[external](https://example.com) and ' + '[rel](./foo.md).' ); @@ -270,8 +270,8 @@ suite('stateToProgressAdapter', () => { assert.strictEqual(response.type, 'response'); if (response.type !== 'response') { return; } const value = (response.parts[0] as IChatMarkdownContent).content.value; - assert.ok(value.includes('[](vscode-agent-host://my-host/file/-/a.ts)')); - assert.ok(value.includes('[](vscode-agent-host://my-host/file/-/c.ts)')); + assert.ok(value.includes('[real](vscode-agent-host://my-host/file/-/a.ts)')); + assert.ok(value.includes('[another](vscode-agent-host://my-host/file/-/c.ts)')); // The link inside the fenced code block must NOT be rewritten. assert.ok(value.includes('[fake](file:///b.ts)')); assert.ok(!value.includes('[fake](vscode-agent-host')); @@ -289,7 +289,7 @@ suite('stateToProgressAdapter', () => { if (response.type !== 'response') { return; } const value = (response.parts[0] as IChatMarkdownContent).content.value; assert.strictEqual(value, - 'Real [](vscode-agent-host://my-host/file/-/a.ts) and literal `[two](file:///b.ts)` here.' + 'Real [one](vscode-agent-host://my-host/file/-/a.ts) and literal `[two](file:///b.ts)` here.' ); }); @@ -415,7 +415,7 @@ suite('stateToProgressAdapter', () => { assert.ok(invocation.pastTenseMessage); assert.strictEqual(typeof invocation.pastTenseMessage, 'object'); const value = (invocation.pastTenseMessage as { value: string }).value; - assert.strictEqual(value, 'Read [](vscode-agent-host://ssh__macbook-air/file/-/path/to/foo.ts)'); + assert.strictEqual(value, 'Read [foo.ts](vscode-agent-host://ssh__macbook-air/file/-/path/to/foo.ts)'); }); test('finalizes terminal tool with output and exit code', () => { From 9c40178fa90046d33a0d5e3c6c4ebcde954f08a1 Mon Sep 17 00:00:00 2001 From: kevin-m-kent <38162246+kevin-m-kent@users.noreply.github.com> Date: Sat, 25 Apr 2026 22:33:57 -0400 Subject: [PATCH 5/8] Add cache-stable mode for vscode_renameSymbol and vscode_listCodeUsages (experimental) (#312568) * Add cache-stable mode for vscode_renameSymbol and vscode_listCodeUsages These two tools today emit a description that embeds the live list of registered language IDs and return undefined when no providers are registered. As language extensions activate during an agent turn (and sometimes deactivate), the tools' description bytes change and the tools themselves can disappear from / reappear in the tool array. Both effects shift bytes inside the tools array prefix and break the provider-side prompt cache mid-turn. Add an experimental setting, `chat.experimental.symbolTools.cacheStable`, that when enabled: - Always registers both tools, regardless of whether any provider is currently registered. - Uses a static modelDescription that does not include the language list. - Skips re-firing onDidUpdateToolData on provider changes (no re-registration churn). Tool behavior at invoke time is unchanged: the tools already return an error when the file's language has no provider, and the static description tells the model to expect that. Default is false so no behavior change without explicit opt-in or experiment assignment. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Update RenameTool/UsagesTool tests for new IConfigurationService dependency Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add unit tests for symbol tools cache-stable mode Addresses Copilot PR review: covers RenameTool/UsagesTool getToolData() behavior when chat.experimental.symbolTools.cacheStable is enabled: - tool data is returned even with no providers registered - modelDescription does not include 'Currently supported for' list - modelDescription is byte-stable across provider registrations Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Kevin Kent Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../contrib/chat/browser/chat.contribution.ts | 9 ++++ .../contrib/chat/browser/tools/renameTool.ts | 49 +++++++++++++++-- .../contrib/chat/browser/tools/usagesTool.ts | 49 +++++++++++++++-- .../contrib/chat/common/constants.ts | 11 ++++ .../test/browser/tools/renameTool.test.ts | 54 ++++++++++++++++++- .../test/browser/tools/usagesTool.test.ts | 49 ++++++++++++++++- 6 files changed, 208 insertions(+), 13 deletions(-) diff --git a/src/vs/workbench/contrib/chat/browser/chat.contribution.ts b/src/vs/workbench/contrib/chat/browser/chat.contribution.ts index eb5cdfe9b82e3..ed276d53fb860 100644 --- a/src/vs/workbench/contrib/chat/browser/chat.contribution.ts +++ b/src/vs/workbench/contrib/chat/browser/chat.contribution.ts @@ -387,6 +387,15 @@ configurationRegistry.registerConfiguration({ default: 'word', tags: ['experimental'], }, + [ChatConfiguration.SymbolToolsCacheStable]: { + type: 'boolean', + description: nls.localize('chat.experimental.symbolTools.cacheStable', "When enabled, the rename and list-code-usages tools are always registered with a static description (no per-language list). Stabilizes the tools-array bytes across requests so prompt caches survive language-extension activations mid-turn. Tool behavior is unchanged: unsupported languages still produce an error at invocation time."), + default: false, + tags: ['experimental'], + experiment: { + mode: 'startup' + } + }, 'chat.detectParticipant.enabled': { type: 'boolean', description: nls.localize('chat.detectParticipant.enabled', "Enables chat participant autodetection for panel chat."), diff --git a/src/vs/workbench/contrib/chat/browser/tools/renameTool.ts b/src/vs/workbench/contrib/chat/browser/tools/renameTool.ts index 0ddb27fa1ffcf..3828c4f36b9c9 100644 --- a/src/vs/workbench/contrib/chat/browser/tools/renameTool.ts +++ b/src/vs/workbench/contrib/chat/browser/tools/renameTool.ts @@ -18,11 +18,13 @@ import { ITextModelService } from '../../../../../editor/common/services/resolve import { ILanguageService } from '../../../../../editor/common/languages/language.js'; import { rename } from '../../../../../editor/contrib/rename/browser/rename.js'; import { localize } from '../../../../../nls.js'; +import { IConfigurationService } from '../../../../../platform/configuration/common/configuration.js'; import { ContextKeyExpr } from '../../../../../platform/contextkey/common/contextkey.js'; import { IInstantiationService } from '../../../../../platform/instantiation/common/instantiation.js'; import { IWorkspaceContextService } from '../../../../../platform/workspace/common/workspace.js'; import { IWorkbenchContribution } from '../../../../common/contributions.js'; import { IChatService } from '../../common/chatService/chatService.js'; +import { ChatConfiguration } from '../../common/constants.js'; import { ChatModel } from '../../common/model/chatModel.js'; import { CountTokensCallback, ILanguageModelToolsService, IPreparedToolInvocation, IToolData, IToolImpl, IToolInvocation, IToolInvocationPreparationContext, IToolResult, ToolDataSource, ToolProgress } from '../../common/tools/languageModelToolsService.js'; import { createToolSimpleTextResult } from '../../common/tools/builtinTools/toolHelpers.js'; @@ -47,6 +49,17 @@ IMPORTANT: The file and line do NOT need to be the definition of the symbol. Any If the tool returns an error, retry with corrected input - ensure the file path is correct, the line content matches the actual file content, and the symbol name appears in that line.`; +/** + * Static description used when the {@link ChatConfiguration.SymbolToolsCacheStable} + * experiment is enabled. Identical to {@link BaseModelDescription} plus a single + * sentence describing the unsupported-language behavior. Crucially, this string + * does NOT depend on the set of registered rename providers, so it stays + * byte-stable across requests as language extensions activate during a turn. + */ +const StaticModelDescription = BaseModelDescription + ` + +If the file's language has no rename provider registered, the tool returns an error.`; + export class RenameTool extends Disposable implements IToolImpl { private readonly _onDidUpdateToolData = this._store.add(new Emitter()); @@ -59,17 +72,32 @@ export class RenameTool extends Disposable implements IToolImpl { @IWorkspaceContextService private readonly _workspaceContextService: IWorkspaceContextService, @IChatService private readonly _chatService: IChatService, @IBulkEditService private readonly _bulkEditService: IBulkEditService, + @IConfigurationService private readonly _configurationService: IConfigurationService, ) { super(); - this._store.add(Event.debounce( - this._languageFeaturesService.renameProvider.onDidChange, - () => { }, - 2000 - )((() => this._onDidUpdateToolData.fire()))); + // In cache-stable mode the tool's wire bytes don't depend on the set + // of registered rename providers, so we don't need to re-fire the + // update event on provider changes. Skipping this subscription + // avoids unnecessary tool re-registration churn as well. + if (!this._isCacheStable()) { + this._store.add(Event.debounce( + this._languageFeaturesService.renameProvider.onDidChange, + () => { }, + 2000 + )((() => this._onDidUpdateToolData.fire()))); + } + } + + private _isCacheStable(): boolean { + return this._configurationService.getValue(ChatConfiguration.SymbolToolsCacheStable) === true; } getToolData(): IToolData | undefined { + if (this._isCacheStable()) { + return this._getStaticToolData(); + } + const languageIds = this._languageFeaturesService.renameProvider.registeredLanguageIds; if (languageIds.size === 0) { @@ -87,6 +115,17 @@ export class RenameTool extends Disposable implements IToolImpl { const niceNames = sorted.map(id => this._languageService.getLanguageName(id) ?? id); userDescription = localize('tool.rename.userDescriptionWithLanguages', 'Rename a symbol across the workspace ({0})', niceNames.join(', ')); } + return this._buildToolData(modelDescription, userDescription); + } + + private _getStaticToolData(): IToolData { + return this._buildToolData( + StaticModelDescription, + localize('tool.rename.userDescription', 'Rename a symbol across the workspace'), + ); + } + + private _buildToolData(modelDescription: string, userDescription: string): IToolData { return { id: RenameToolId, toolReferenceName: 'rename', diff --git a/src/vs/workbench/contrib/chat/browser/tools/usagesTool.ts b/src/vs/workbench/contrib/chat/browser/tools/usagesTool.ts index 6f8f708ee0154..c446f871fd082 100644 --- a/src/vs/workbench/contrib/chat/browser/tools/usagesTool.ts +++ b/src/vs/workbench/contrib/chat/browser/tools/usagesTool.ts @@ -21,11 +21,13 @@ import { ITextModelService } from '../../../../../editor/common/services/resolve import { ILanguageService } from '../../../../../editor/common/languages/language.js'; import { getDefinitionsAtPosition, getImplementationsAtPosition, getReferencesAtPosition } from '../../../../../editor/contrib/gotoSymbol/browser/goToSymbol.js'; import { localize } from '../../../../../nls.js'; +import { IConfigurationService } from '../../../../../platform/configuration/common/configuration.js'; import { ContextKeyExpr } from '../../../../../platform/contextkey/common/contextkey.js'; import { IInstantiationService } from '../../../../../platform/instantiation/common/instantiation.js'; import { IWorkspaceContextService } from '../../../../../platform/workspace/common/workspace.js'; import { IWorkbenchContribution } from '../../../../common/contributions.js'; import { ISearchService, QueryType, resultIsMatch } from '../../../../services/search/common/search.js'; +import { ChatConfiguration } from '../../common/constants.js'; import { CountTokensCallback, ILanguageModelToolsService, IPreparedToolInvocation, IToolData, IToolImpl, IToolInvocation, IToolInvocationPreparationContext, IToolResult, ToolDataSource, ToolProgress, } from '../../common/tools/languageModelToolsService.js'; import { createToolSimpleTextResult } from '../../common/tools/builtinTools/toolHelpers.js'; import { errorResult, findLineNumber, findSymbolColumn, ISymbolToolInput, resolveToolUri } from './toolHelpers.js'; @@ -44,6 +46,17 @@ IMPORTANT: The file and line do NOT need to be the definition of the symbol. Any If the tool returns an error, retry with corrected input - ensure the file path is correct, the line content matches the actual file content, and the symbol name appears in that line.`; +/** + * Static description used when the {@link ChatConfiguration.SymbolToolsCacheStable} + * experiment is enabled. Identical to {@link BaseModelDescription} plus a single + * sentence describing the unsupported-language behavior. Crucially, this string + * does NOT depend on the set of registered reference providers, so it stays + * byte-stable across requests as language extensions activate during a turn. + */ +const StaticModelDescription = BaseModelDescription + ` + +If the file's language has no reference provider registered, the tool returns an error.`; + export class UsagesTool extends Disposable implements IToolImpl { private readonly _onDidUpdateToolData = this._store.add(new Emitter()); @@ -56,17 +69,32 @@ export class UsagesTool extends Disposable implements IToolImpl { @ISearchService private readonly _searchService: ISearchService, @ITextModelService private readonly _textModelService: ITextModelService, @IWorkspaceContextService private readonly _workspaceContextService: IWorkspaceContextService, + @IConfigurationService private readonly _configurationService: IConfigurationService, ) { super(); - this._store.add(Event.debounce( - this._languageFeaturesService.referenceProvider.onDidChange, - () => { }, - 2000 - )((() => this._onDidUpdateToolData.fire()))); + // In cache-stable mode the tool's wire bytes don't depend on the set + // of registered reference providers, so we don't re-fire the update + // event on provider changes. Skipping this subscription also avoids + // unnecessary tool re-registration churn. + if (!this._isCacheStable()) { + this._store.add(Event.debounce( + this._languageFeaturesService.referenceProvider.onDidChange, + () => { }, + 2000 + )((() => this._onDidUpdateToolData.fire()))); + } + } + + private _isCacheStable(): boolean { + return this._configurationService.getValue(ChatConfiguration.SymbolToolsCacheStable) === true; } getToolData(): IToolData | undefined { + if (this._isCacheStable()) { + return this._getStaticToolData(); + } + const languageIds = this._languageFeaturesService.referenceProvider.registeredLanguageIds; if (languageIds.size === 0) { @@ -85,6 +113,17 @@ export class UsagesTool extends Disposable implements IToolImpl { userDescription = localize('tool.usages.userDescriptionWithLanguages', 'Find references, definitions, and implementations of a symbol ({0})', niceNames.join(', ')); } + return this._buildToolData(modelDescription, userDescription); + } + + private _getStaticToolData(): IToolData { + return this._buildToolData( + StaticModelDescription, + localize('tool.usages.userDescription', 'Find references, definitions, and implementations of a symbol'), + ); + } + + private _buildToolData(modelDescription: string, userDescription: string): IToolData { return { id: UsagesToolId, toolReferenceName: 'usages', diff --git a/src/vs/workbench/contrib/chat/common/constants.ts b/src/vs/workbench/contrib/chat/common/constants.ts index 613f5cfeaeee7..dea889b8f3730 100644 --- a/src/vs/workbench/contrib/chat/common/constants.ts +++ b/src/vs/workbench/contrib/chat/common/constants.ts @@ -78,6 +78,17 @@ export enum ChatConfiguration { IncrementalRendering = 'chat.experimental.incrementalRendering.enabled', IncrementalRenderingStyle = 'chat.experimental.incrementalRendering.animationStyle', IncrementalRenderingBuffering = 'chat.experimental.incrementalRendering.buffering', + + /** + * When enabled, `vscode_renameSymbol` and `vscode_listCodeUsages` are always + * registered with a static, language-list-free description. This makes the + * tools array byte-stable across rounds even as language extensions activate + * mid-turn, which significantly improves prompt-cache hit rates on agent + * conversations. Behavior is unchanged: the tools still error on + * unsupported languages at invocation time. Behind an A/B flag for + * controlled rollout. + */ + SymbolToolsCacheStable = 'chat.experimental.symbolTools.cacheStable', } /** diff --git a/src/vs/workbench/contrib/chat/test/browser/tools/renameTool.test.ts b/src/vs/workbench/contrib/chat/test/browser/tools/renameTool.test.ts index fabf66ac855a7..61e269e901975 100644 --- a/src/vs/workbench/contrib/chat/test/browser/tools/renameTool.test.ts +++ b/src/vs/workbench/contrib/chat/test/browser/tools/renameTool.test.ts @@ -16,7 +16,9 @@ import { ILanguageService } from '../../../../../../editor/common/languages/lang import { createTextModel } from '../../../../../../editor/test/common/testTextModel.js'; import { IWorkspaceContextService, IWorkspaceFolder } from '../../../../../../platform/workspace/common/workspace.js'; import { IBulkEditService, IBulkEditResult } from '../../../../../../editor/browser/services/bulkEditService.js'; +import { TestConfigurationService } from '../../../../../../platform/configuration/test/common/testConfigurationService.js'; import { RenameTool } from '../../../browser/tools/renameTool.js'; +import { ChatConfiguration } from '../../../common/constants.js'; import { IChatService } from '../../../common/chatService/chatService.js'; import { IToolInvocation, IToolResult, IToolResultTextPart, ToolProgress } from '../../../common/tools/languageModelToolsService.js'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../../base/test/common/utils.js'; @@ -105,7 +107,7 @@ suite('RenameTool', () => { return { getLanguageName: (id: string) => id } as unknown as ILanguageService; } - function createTool(textModelService: ITextModelService, options?: { bulkEditService?: IBulkEditService }): RenameTool { + function createTool(textModelService: ITextModelService, options?: { bulkEditService?: IBulkEditService; configurationService?: TestConfigurationService }): RenameTool { return new RenameTool( langFeatures, createMockLanguageService(), @@ -113,6 +115,7 @@ suite('RenameTool', () => { createMockWorkspaceService(), createMockChatService(), options?.bulkEditService ?? createMockBulkEditService(), + options?.configurationService ?? new TestConfigurationService(), ); } @@ -151,6 +154,55 @@ suite('RenameTool', () => { const data = tool.getToolData(); assert.ok(data?.modelDescription.includes('all languages')); }); + + suite('cache-stable mode', () => { + function createCacheStableTool(textModelService: ITextModelService) { + const configurationService = new TestConfigurationService(); + configurationService.setUserConfiguration(ChatConfiguration.SymbolToolsCacheStable, true); + return disposables.add(createTool(textModelService, { configurationService })); + } + + test('returns tool data even when no providers are registered', () => { + const tool = createCacheStableTool(createMockTextModelService(null!)); + const data = tool.getToolData(); + assert.ok(data, 'expected getToolData() to return data with no providers registered'); + }); + + test('description does not include a per-language list', () => { + const model = disposables.add(createTextModel('', 'typescript', undefined, testUri)); + const tool = createCacheStableTool(createMockTextModelService(model)); + disposables.add(langFeatures.renameProvider.register('typescript', { + provideRenameEdits: () => ({ edits: [] }), + })); + const data = tool.getToolData(); + assert.ok(data, 'expected getToolData() to return data'); + assert.ok(!data!.modelDescription.includes('Currently supported for'), + `expected modelDescription to not list languages, got: ${data!.modelDescription}`); + assert.ok(!data!.modelDescription.includes('typescript'), + 'expected modelDescription to not include any specific language id'); + assert.ok(!data!.modelDescription.includes('all languages'), + 'expected modelDescription to not mention "all languages"'); + }); + + test('description is identical regardless of which providers are registered', () => { + const tool1 = createCacheStableTool(createMockTextModelService(null!)); + const data1 = tool1.getToolData(); + + const model = disposables.add(createTextModel('', 'typescript', undefined, testUri)); + const tool2 = createCacheStableTool(createMockTextModelService(model)); + disposables.add(langFeatures.renameProvider.register('typescript', { + provideRenameEdits: () => ({ edits: [] }), + })); + disposables.add(langFeatures.renameProvider.register('python', { + provideRenameEdits: () => ({ edits: [] }), + })); + const data2 = tool2.getToolData(); + + assert.ok(data1 && data2); + assert.strictEqual(data1!.modelDescription, data2!.modelDescription, + 'expected modelDescription to be byte-stable across provider registrations'); + }); + }); }); suite('invoke', () => { diff --git a/src/vs/workbench/contrib/chat/test/browser/tools/usagesTool.test.ts b/src/vs/workbench/contrib/chat/test/browser/tools/usagesTool.test.ts index 28c14f3b1ad24..9747eb96767c1 100644 --- a/src/vs/workbench/contrib/chat/test/browser/tools/usagesTool.test.ts +++ b/src/vs/workbench/contrib/chat/test/browser/tools/usagesTool.test.ts @@ -17,7 +17,9 @@ import { ILanguageService } from '../../../../../../editor/common/languages/lang import { createTextModel } from '../../../../../../editor/test/common/testTextModel.js'; import { IWorkspaceContextService, IWorkspaceFolder } from '../../../../../../platform/workspace/common/workspace.js'; import { FileMatch, ISearchComplete, ISearchService, ITextQuery, OneLineRange, TextSearchMatch } from '../../../../../services/search/common/search.js'; +import { TestConfigurationService } from '../../../../../../platform/configuration/test/common/testConfigurationService.js'; import { UsagesTool } from '../../../browser/tools/usagesTool.js'; +import { ChatConfiguration } from '../../../common/constants.js'; import { IToolInvocation, IToolResult, IToolResultTextPart, ToolProgress } from '../../../common/tools/languageModelToolsService.js'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../../base/test/common/utils.js'; @@ -96,8 +98,8 @@ suite('UsagesTool', () => { return { getLanguageName: (id: string) => id } as unknown as ILanguageService; } - function createTool(textModelService: ITextModelService, workspaceService: IWorkspaceContextService, options?: { modelService?: IModelService; searchService?: ISearchService }): UsagesTool { - return new UsagesTool(langFeatures, createMockLanguageService(), options?.modelService ?? createMockModelService(), options?.searchService ?? createMockSearchService(), textModelService, workspaceService); + function createTool(textModelService: ITextModelService, workspaceService: IWorkspaceContextService, options?: { modelService?: IModelService; searchService?: ISearchService; configurationService?: TestConfigurationService }): UsagesTool { + return new UsagesTool(langFeatures, createMockLanguageService(), options?.modelService ?? createMockModelService(), options?.searchService ?? createMockSearchService(), textModelService, workspaceService, options?.configurationService ?? new TestConfigurationService()); } setup(() => { @@ -131,6 +133,49 @@ suite('UsagesTool', () => { const data = tool.getToolData(); assert.ok(data?.modelDescription.includes('all languages')); }); + + suite('cache-stable mode', () => { + function createCacheStableTool(textModelService: ITextModelService) { + const configurationService = new TestConfigurationService(); + configurationService.setUserConfiguration(ChatConfiguration.SymbolToolsCacheStable, true); + return disposables.add(createTool(textModelService, createMockWorkspaceService(), { configurationService })); + } + + test('returns tool data even when no providers are registered', () => { + const tool = createCacheStableTool(createMockTextModelService(null!)); + const data = tool.getToolData(); + assert.ok(data, 'expected getToolData() to return data with no providers registered'); + }); + + test('description does not include a per-language list', () => { + const model = disposables.add(createTextModel('', 'typescript', undefined, testUri)); + const tool = createCacheStableTool(createMockTextModelService(model)); + disposables.add(langFeatures.referenceProvider.register('typescript', { provideReferences: () => [] })); + const data = tool.getToolData(); + assert.ok(data, 'expected getToolData() to return data'); + assert.ok(!data!.modelDescription.includes('Currently supported for'), + `expected modelDescription to not list languages, got: ${data!.modelDescription}`); + assert.ok(!data!.modelDescription.includes('typescript'), + 'expected modelDescription to not include any specific language id'); + assert.ok(!data!.modelDescription.includes('all languages'), + 'expected modelDescription to not mention "all languages"'); + }); + + test('description is identical regardless of which providers are registered', () => { + const tool1 = createCacheStableTool(createMockTextModelService(null!)); + const data1 = tool1.getToolData(); + + const model = disposables.add(createTextModel('', 'typescript', undefined, testUri)); + const tool2 = createCacheStableTool(createMockTextModelService(model)); + disposables.add(langFeatures.referenceProvider.register('typescript', { provideReferences: () => [] })); + disposables.add(langFeatures.referenceProvider.register('python', { provideReferences: () => [] })); + const data2 = tool2.getToolData(); + + assert.ok(data1 && data2); + assert.strictEqual(data1!.modelDescription, data2!.modelDescription, + 'expected modelDescription to be byte-stable across provider registrations'); + }); + }); }); suite('invoke', () => { From cf1f60059c9945ad418e561072ea9f4c299ffbc4 Mon Sep 17 00:00:00 2001 From: Bhavya U Date: Sat, 25 Apr 2026 20:50:42 -0700 Subject: [PATCH 6/8] Enable inline history summarization and GPT-5.5 tool search (#312570) --- extensions/copilot/package.json | 2 +- .../configuration/common/configurationService.ts | 2 +- .../endpoint/common/chatModelCapabilities.ts | 15 ++++++++++----- .../test/node/chatModelCapabilities.spec.ts | 6 +++++- 4 files changed, 17 insertions(+), 8 deletions(-) diff --git a/extensions/copilot/package.json b/extensions/copilot/package.json index 83f76c63df883..79dd3b42ff176 100644 --- a/extensions/copilot/package.json +++ b/extensions/copilot/package.json @@ -4507,7 +4507,7 @@ }, "github.copilot.chat.agentHistorySummarizationInline": { "type": "boolean", - "default": false, + "default": true, "markdownDescription": "%github.copilot.config.agentHistorySummarizationInline%", "tags": [ "advanced", diff --git a/extensions/copilot/src/platform/configuration/common/configurationService.ts b/extensions/copilot/src/platform/configuration/common/configurationService.ts index 4ce1093b53469..e50b1be311b76 100644 --- a/extensions/copilot/src/platform/configuration/common/configurationService.ts +++ b/extensions/copilot/src/platform/configuration/common/configurationService.ts @@ -647,7 +647,7 @@ export namespace ConfigKey { export const InstantApplyShortModelName = defineAndMigrateExpSetting('chat.advanced.instantApply.shortContextModelName', 'chat.instantApply.shortContextModelName', CHAT_MODEL.SHORT_INSTANT_APPLY); export const InstantApplyShortContextLimit = defineAndMigrateExpSetting('chat.advanced.instantApply.shortContextLimit', 'chat.instantApply.shortContextLimit', 8000); - export const AgentHistorySummarizationInline = defineAndMigrateExpSetting('chat.advanced.agentHistorySummarizationInline', 'chat.agentHistorySummarizationInline', false); + export const AgentHistorySummarizationInline = defineAndMigrateExpSetting('chat.advanced.agentHistorySummarizationInline', 'chat.agentHistorySummarizationInline', true); export const PromptFileContext = defineAndMigrateExpSetting('chat.advanced.promptFileContextProvider.enabled', 'chat.promptFileContextProvider.enabled', true); export const DefaultToolsGrouped = defineAndMigrateExpSetting('chat.advanced.tools.defaultToolsGrouped', 'chat.tools.defaultToolsGrouped', false); export const Gpt5AlternativePatch = defineAndMigrateExpSetting('chat.advanced.gpt5AlternativePatch', 'chat.gpt5AlternativePatch', false); diff --git a/extensions/copilot/src/platform/endpoint/common/chatModelCapabilities.ts b/extensions/copilot/src/platform/endpoint/common/chatModelCapabilities.ts index f3801ae2d5e55..89a243606faef 100644 --- a/extensions/copilot/src/platform/endpoint/common/chatModelCapabilities.ts +++ b/extensions/copilot/src/platform/endpoint/common/chatModelCapabilities.ts @@ -397,15 +397,14 @@ export function getVerbosityForModelSync(model: IChatEndpoint): 'low' | 'medium' * - Claude Opus 4.5 (claude-opus-4-5-* or claude-opus-4.5-*) * - Claude Opus 4.6 (claude-opus-4-6-* or claude-opus-4.6-*) * - Claude Opus 4.7 (claude-opus-4-7-* or claude-opus-4.7-*) - * - OpenAI gpt-5.4 (gpt-5.4-*), but only when the `ResponsesApiToolSearchEnabled` setting is enabled + * - OpenAI gpt-5.4/gpt-5.5, but only when the `ResponsesApiToolSearchEnabled` setting is enabled */ export function modelSupportsToolSearch(modelId: string, configurationService?: IConfigurationService, experimentationService?: IExperimentationService): boolean { - const lower = modelId.toLowerCase(); - if (isGpt54(lower)) { + const normalized = modelId.toLowerCase().replace(/\./g, '-'); + if (isResponsesApiToolSearchModelId(normalized)) { return !!configurationService && !!experimentationService && isResponsesApiToolSearchEnabled(modelId, configurationService, experimentationService); } - const normalized = lower.replace(/\./g, '-'); return normalized.startsWith('claude-sonnet-4-5') || normalized.startsWith('claude-sonnet-4-6') || normalized.startsWith('claude-opus-4-5') || @@ -414,12 +413,18 @@ export function modelSupportsToolSearch(modelId: string, configurationService?: isHiddenModelG(modelId); } +function isResponsesApiToolSearchModelId(normalizedModelId: string): boolean { + return normalizedModelId.startsWith('gpt-5-4') || normalizedModelId.startsWith('gpt-5-5') || normalizedModelId.startsWith('gpt5-5'); +} + export function isResponsesApiToolSearchEnabled( endpoint: IChatEndpoint | string, configurationService: IConfigurationService, experimentationService: IExperimentationService, ): boolean { - return isGpt54(endpoint) && configurationService.getExperimentBasedConfig(ConfigKey.ResponsesApiToolSearchEnabled, experimentationService); + const modelId = typeof endpoint === 'string' ? endpoint : endpoint.model; + const normalized = modelId.toLowerCase().replace(/\./g, '-'); + return isResponsesApiToolSearchModelId(normalized) && configurationService.getExperimentBasedConfig(ConfigKey.ResponsesApiToolSearchEnabled, experimentationService); } /** diff --git a/extensions/copilot/src/platform/endpoint/test/node/chatModelCapabilities.spec.ts b/extensions/copilot/src/platform/endpoint/test/node/chatModelCapabilities.spec.ts index 705b0b18bc2c6..a3e700cace342 100644 --- a/extensions/copilot/src/platform/endpoint/test/node/chatModelCapabilities.spec.ts +++ b/extensions/copilot/src/platform/endpoint/test/node/chatModelCapabilities.spec.ts @@ -66,7 +66,7 @@ describe('modelSupportsToolSearch', () => { expect(modelSupportsToolSearch('claude-3-opus')).toBe(false); }); - test('supports OpenAI gpt-5.4 models when the setting is enabled', () => { + test('supports OpenAI gpt-5.4 and gpt-5.5 models when the setting is enabled', () => { const configurationService = { getExperimentBasedConfig: (key: unknown) => key === ConfigKey.ResponsesApiToolSearchEnabled, } as unknown as IConfigurationService; @@ -74,7 +74,11 @@ describe('modelSupportsToolSearch', () => { expect(modelSupportsToolSearch('gpt-5.4', configurationService, experimentationService)).toBe(true); expect(modelSupportsToolSearch('gpt-5.4-preview', configurationService, experimentationService)).toBe(true); + expect(modelSupportsToolSearch('gpt-5.5', configurationService, experimentationService)).toBe(true); + expect(modelSupportsToolSearch('gpt-5.5-preview', configurationService, experimentationService)).toBe(true); + expect(modelSupportsToolSearch('gpt5.5-preview', configurationService, experimentationService)).toBe(true); expect(modelSupportsToolSearch('gpt-5.4')).toBe(false); + expect(modelSupportsToolSearch('gpt-5.5')).toBe(false); }); test('rejects other non-Claude models', () => { From b35a44fffb6fc77f9207480c4e7732fb7e738414 Mon Sep 17 00:00:00 2001 From: Rob Lourens Date: Sat, 25 Apr 2026 21:18:46 -0700 Subject: [PATCH 7/8] Restore empty link text for non-skill agent-host links (#312571) * Restore empty link text for non-skill agent-host links PR #312557 changed all agent-host link rewriting to preserve the original markdown label, which made every read-tool link render as a plain blue markdown link instead of the rich inline anchor / file widget. Only skill links need a non-empty label (so the skill pill can show the skill name); restore the empty-label behavior for everything else. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address Copilot review: preserve image alt text, add skill/image tests Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../agentHost/stateToProgressAdapter.ts | 31 +++++++----- .../stateToProgressAdapter.test.ts | 49 ++++++++++++++++--- 2 files changed, 62 insertions(+), 18 deletions(-) diff --git a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/stateToProgressAdapter.ts b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/stateToProgressAdapter.ts index 0234eacc1e399..bcd7031563688 100644 --- a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/stateToProgressAdapter.ts +++ b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/stateToProgressAdapter.ts @@ -411,13 +411,15 @@ export function rewriteMarkdownLinks(markdown: string, connectionAuthority: stri * or returns `undefined` if the token should be left alone (external * scheme or unparseable URI). * - * The output is the canonical inline form `[text](newHref)` (or - * `![text](newHref)` for images). When the original link has no display - * text the chat renderer falls back to its file-widget rendering, but a - * non-empty label (e.g. a skill name) is preserved so the renderer shows - * it instead of the URI's basename. This also means autolinks (``) - * and reference-style links (`[text][ref]`) are normalized into the - * inline form. + * The output collapses to the canonical inline form `[](newHref)` (or + * `![](newHref)` for images) — the chat renderer has richer handling for + * empty-text agent-host links (rendering them as a file widget), so + * preserving the original label isn't useful for most links. The one + * exception is skill links (URIs whose basename is `SKILL.md`), where the + * skill name is preserved as the label so the skill pill renderer can + * display it instead of the always-identical `SKILL.md` basename. This + * also means autolinks (``) and reference-style links + * (`[text][ref]`) are normalized into the inline form. */ function rewriteLinkTokenRaw(token: Tokens.Link | Tokens.Image, connectionAuthority: string): string | undefined { let parsed: URI; @@ -431,22 +433,27 @@ function rewriteLinkTokenRaw(token: Tokens.Link | Tokens.Image, connectionAuthor return undefined; } let agentHostUri = toAgentHostUri(parsed, connectionAuthority); + const isSkill = isSkillFileUri(parsed); // VS-Code-specific: links pointing at a `SKILL.md` file are rendered as a // rich skill pill rather than a plain markdown link. The chat renderer's // inline anchor widget keys off the `vscodeLinkType` query parameter (see // `chatInlineAnchorWidget.ts`), so we tag the URI here on the client side // rather than at the agent host. We do this whether or not the link came // in pre-tagged so older sessions and other agent providers also benefit. - if (isSkillFileUri(parsed) && !agentHostUri.query.includes('vscodeLinkType=')) { + if (isSkill && !agentHostUri.query.includes('vscodeLinkType=')) { const existing = agentHostUri.query; agentHostUri = agentHostUri.with({ query: existing ? `${existing}&vscodeLinkType=skill` : 'vscodeLinkType=skill' }); } const prefix = token.type === 'image' ? '![' : '['; + // Preserve the label for skill links (so the skill pill renderer can show + // the skill name) and for image alt text (accessibility — the inline + // anchor widget only applies to links, not images). For all other + // agent-host links, leave the text empty so the chat renderer's inline + // anchor widget takes over with its rich file-widget rendering. // Escape only the characters that would break out of markdown link text - // syntax (`\` and `]`). A full markdown escape would leave visible - // backslashes in renderers (like the skill pill) that extract link text - // without re-parsing markdown. - const text = escapeMarkdownLinkLabel(token.text ?? ''); + // syntax (`\` and `]`); a full markdown escape would leave visible + // backslashes in the skill pill which extracts text without re-parsing. + const text = isSkill || token.type === 'image' ? escapeMarkdownLinkLabel(token.text ?? '') : ''; return `${prefix}${text}](${agentHostUri.toString()})`; } diff --git a/src/vs/workbench/contrib/chat/test/browser/agentSessions/stateToProgressAdapter.test.ts b/src/vs/workbench/contrib/chat/test/browser/agentSessions/stateToProgressAdapter.test.ts index 1888763ae14ff..ad66f2a97f348 100644 --- a/src/vs/workbench/contrib/chat/test/browser/agentSessions/stateToProgressAdapter.test.ts +++ b/src/vs/workbench/contrib/chat/test/browser/agentSessions/stateToProgressAdapter.test.ts @@ -244,8 +244,8 @@ suite('stateToProgressAdapter', () => { if (response.type !== 'response') { return; } const part = response.parts[0] as IChatMarkdownContent; assert.deepStrictEqual(part.content.value, - 'See [local](vscode-agent-host://my-host/file/-/a/b.ts), ' + - '[content](vscode-agent-host://my-host/agenthost-content/-/s/x), ' + + 'See [](vscode-agent-host://my-host/file/-/a/b.ts), ' + + '[](vscode-agent-host://my-host/agenthost-content/-/s/x), ' + '[external](https://example.com) and ' + '[rel](./foo.md).' ); @@ -270,8 +270,8 @@ suite('stateToProgressAdapter', () => { assert.strictEqual(response.type, 'response'); if (response.type !== 'response') { return; } const value = (response.parts[0] as IChatMarkdownContent).content.value; - assert.ok(value.includes('[real](vscode-agent-host://my-host/file/-/a.ts)')); - assert.ok(value.includes('[another](vscode-agent-host://my-host/file/-/c.ts)')); + assert.ok(value.includes('[](vscode-agent-host://my-host/file/-/a.ts)')); + assert.ok(value.includes('[](vscode-agent-host://my-host/file/-/c.ts)')); // The link inside the fenced code block must NOT be rewritten. assert.ok(value.includes('[fake](file:///b.ts)')); assert.ok(!value.includes('[fake](vscode-agent-host')); @@ -289,10 +289,47 @@ suite('stateToProgressAdapter', () => { if (response.type !== 'response') { return; } const value = (response.parts[0] as IChatMarkdownContent).content.value; assert.strictEqual(value, - 'Real [one](vscode-agent-host://my-host/file/-/a.ts) and literal `[two](file:///b.ts)` here.' + 'Real [](vscode-agent-host://my-host/file/-/a.ts) and literal `[two](file:///b.ts)` here.' ); }); + test('preserves label and tags vscodeLinkType=skill for SKILL.md links', () => { + const turn = createTurn({ + responseParts: [{ + kind: ResponsePartKind.Markdown, + id: 'md-skill', + content: 'Loaded [plan](file:///abs/repo/skills/plan/SKILL.md) and [other](file:///abs/repo/foo.ts).', + }], + }); + + const history = rawTurnsToHistory(URI.file('/'), [turn], 'p', 'my-host'); + const response = history[1]; + assert.strictEqual(response.type, 'response'); + if (response.type !== 'response') { return; } + const value = (response.parts[0] as IChatMarkdownContent).content.value; + assert.strictEqual(value, + 'Loaded [plan](vscode-agent-host://my-host/file/-/abs/repo/skills/plan/SKILL.md?vscodeLinkType%3Dskill) ' + + 'and [](vscode-agent-host://my-host/file/-/abs/repo/foo.ts).' + ); + }); + + test('preserves alt text for image tokens', () => { + const turn = createTurn({ + responseParts: [{ + kind: ResponsePartKind.Markdown, + id: 'md-image', + content: 'See ![diagram](file:///a/b.png).', + }], + }); + + const history = rawTurnsToHistory(URI.file('/'), [turn], 'p', 'my-host'); + const response = history[1]; + assert.strictEqual(response.type, 'response'); + if (response.type !== 'response') { return; } + const value = (response.parts[0] as IChatMarkdownContent).content.value; + assert.strictEqual(value, 'See ![diagram](vscode-agent-host://my-host/file/-/a/b.png).'); + }); + test('error turn produces error message in history', () => { const turn = createTurn({ state: TurnState.Error, @@ -415,7 +452,7 @@ suite('stateToProgressAdapter', () => { assert.ok(invocation.pastTenseMessage); assert.strictEqual(typeof invocation.pastTenseMessage, 'object'); const value = (invocation.pastTenseMessage as { value: string }).value; - assert.strictEqual(value, 'Read [foo.ts](vscode-agent-host://ssh__macbook-air/file/-/path/to/foo.ts)'); + assert.strictEqual(value, 'Read [](vscode-agent-host://ssh__macbook-air/file/-/path/to/foo.ts)'); }); test('finalizes terminal tool with output and exit code', () => { From 1fa1b7af5c190606cdd5e8fe5e5f1ca4fad47e00 Mon Sep 17 00:00:00 2001 From: Rob Lourens Date: Sat, 25 Apr 2026 21:19:05 -0700 Subject: [PATCH 8/8] agent-host: surface session git state via SessionState._meta (#312543) * agent-host: surface session git state via SessionState._meta The agent host process now computes per-session git state (branch, GitHub remote, ahead/behind, uncommitted changes) for sessions that have a working directory and publishes it through the protocol's per-session `_meta`. The Agents app reads it from `SessionState._meta` (not `SessionSummary`) and surfaces it via `ISessionWorkspace`, lighting up existing UI in the Changes view (e.g. ahead/behind indicators, branch info). Highlights: - New `AgentHostGitService` (server-side) computes git state by shelling out to git; refreshed on session open and after each turn. - New `SessionMetaChanged` action propagates `_meta` deltas without a full list refresh. - Client-side `AgentHostSessionAdapter` retains `_project` / `_workingDirectory` / `_meta` so the workspace observable can be rebuilt when only `_meta` changes. - `baseBranchProtected` is computed client-side from `git.branchProtection` config, so the workspace shape no longer carries it as a field on `ISessionRepository`. - `update()` only overwrites `_meta` when the source actually provides one (SessionSummary feeds have no `_meta` field), so the polling refresh path no longer clobbers good state pushed via `SessionState`. (Written by Copilot) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address Copilot review feedback - Doc updates: reference SessionState._meta (not SessionSummary._meta) in agent service interface and the setMeta delta path. - changesViewModel.activeSessionHasGitRepositoryObs now derives the git-backed signal from surfaced git state on the workspace's first repository (uncommittedChanges/incomingChanges/outgoingChanges/ upstreamBranchName) rather than from mere workspace presence, so we don't enable git-specific UI for non-git working directories. (Written by Copilot) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Render branch name in changes view tree root (Written by Copilot) Plumb 'branchName' through: - ISessionRepository (new optional field) - buildAgentHostSessionWorkspace + agentHostSessionWorkspaceKey (gitFields) - ChangesViewModel.activeSessionStateObs (fall back to workspace repo) - ChangesViewPane.getTreeRootInfo: only render parens when branchName known Also subscribe to activeSessionStateObs in the changes-tree autorun so the root rebuilds once branchName arrives asynchronously. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Show branch name in changes tree root even for non-worktree sessions (Written by Copilot) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: wire AgentHostGitService into AgentService at construction time Previously AgentService was constructed without a git service (the optional _gitService parameter was never passed), so _attachGitState always bailed with 'hasGitService=false' and no branch name was ever computed. The fix creates AgentHostGitService before AgentService and passes it as the fifth constructor argument. Also removes debug logging added during investigation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * harden: make IAgentHostGitService a required AgentService dep The optional `_gitService?` parameter was the root cause of git metadata never reaching the client: `agentHostMain.ts` and `agentHostServerMain.ts` constructed AgentService without passing it, and `_attachGitState` silently bailed at runtime. Making the dep required forces all callers (now and in the future) to wire it correctly at compile time. Also adds a regression test for the `subscribe()` lazy-fire path, which was the intended client-visible mechanism for surfacing branch name on sessions that already exist in the state manager. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Revert: only show branch name for worktree sessions Branch name on non-worktree sessions wasn't a goal. Restores the original guard so `(branch)` only appears when the session has a worktree. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * cleanups * Address Copilot review feedback (Written by Copilot) - changesViewModel: fall back to workspaceRepository.baseBranchName so agent-host sessions get branch protection UI when only the workspace repo carries baseBranchName - changesView: add comment explaining the intentional dependency read on activeSessionStateObs in the tree-update autorun - agentService: dedupe _ skip setSessionMeta when theattachGitState newly computed git state equals the current _meta.git, avoiding action churn after every turn Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../platform/agentHost/common/agentService.ts | 10 +- .../agentHost/common/state/sessionState.ts | 92 ++++++++++ .../agentHost/node/agentHostGitService.ts | 141 +++++++++++++++ .../platform/agentHost/node/agentHostMain.ts | 5 +- .../agentHost/node/agentHostServerMain.ts | 5 +- .../agentHost/node/agentHostStateManager.ts | 17 +- .../platform/agentHost/node/agentService.ts | 67 ++++++- .../agentHost/node/agentSideEffects.ts | 10 +- .../test/common/sessionTestHelpers.ts | 20 +++ .../agentHostGitService.integrationTest.ts | 127 +++++++++++++ .../test/node/agentHostGitService.test.ts | 89 ++++++++- .../agentHost/test/node/agentService.test.ts | 170 ++++++++++++++++-- .../test/node/agentSideEffects.test.ts | 10 +- .../agentHost/test/node/copilotAgent.test.ts | 1 + .../test/node/copilotGitProject.test.ts | 1 + .../platform/agentHost/test/node/mockAgent.ts | 12 +- .../common/agentHostSessionWorkspace.ts | 71 +++++++- .../browser/baseAgentHostSessionsProvider.ts | 80 ++++++++- .../browser/localAgentHostSessionsProvider.ts | 16 +- .../localAgentHostSessionsProvider.test.ts | 3 + .../contrib/changes/browser/changesView.ts | 3 + .../changes/browser/changesViewModel.ts | 38 ++-- .../browser/sessionWorkspacePicker.test.ts | 2 +- .../sessionsConfigurationService.test.ts | 1 - .../browser/copilotChatSessionsProvider.ts | 19 +- .../remoteAgentHostSessionsProvider.ts | 16 +- .../remoteAgentHostSessionsProvider.test.ts | 3 + .../sessionsTerminalContribution.test.ts | 2 - .../services/sessions/common/session.ts | 14 +- .../agentHostSessionListController.ts | 11 +- .../agentHostChatContribution.test.ts | 22 +++ 31 files changed, 1002 insertions(+), 76 deletions(-) create mode 100644 src/vs/platform/agentHost/test/node/agentHostGitService.integrationTest.ts diff --git a/src/vs/platform/agentHost/common/agentService.ts b/src/vs/platform/agentHost/common/agentService.ts index 786b973bb6bf7..0b8959824d279 100644 --- a/src/vs/platform/agentHost/common/agentService.ts +++ b/src/vs/platform/agentHost/common/agentService.ts @@ -15,7 +15,7 @@ import type { CreateTerminalParams, ResolveSessionConfigResult, SessionConfigCom import { ProtectedResourceMetadata, type ConfigSchema, type FileEdit, type ModelSelection, type SessionActiveClient, type ToolDefinition } from './state/protocol/state.js'; import type { ActionEnvelope, INotification, RootAction, SessionAction, TerminalAction } from './state/sessionActions.js'; import type { ResourceCopyParams, ResourceCopyResult, ResourceDeleteParams, ResourceDeleteResult, ResourceListResult, ResourceMoveParams, ResourceMoveResult, ResourceReadResult, ResourceWriteParams, ResourceWriteResult, IStateSnapshot } from './state/sessionProtocol.js'; -import { AttachmentType, ComponentToState, SessionInputResponseKind, SessionStatus, StateComponents, type CustomizationRef, type PendingMessage, type RootState, type SessionInputAnswer, type SessionInputRequest, type ToolCallResult, type ToolResultContent, type PolicyState, type StringOrMarkdown } from './state/sessionState.js'; +import { AttachmentType, ComponentToState, SessionInputResponseKind, SessionStatus, StateComponents, type CustomizationRef, type PendingMessage, type RootState, type SessionInputAnswer, type SessionInputRequest, type SessionMeta, type ToolCallResult, type ToolResultContent, type PolicyState, type StringOrMarkdown } from './state/sessionState.js'; // IPC contract between the renderer and the agent host utility process. // Defines all serializable event types, the IAgent provider interface, @@ -87,6 +87,14 @@ export interface IAgentSessionMetadata { readonly isRead?: boolean; readonly isArchived?: boolean; readonly diffs?: readonly FileEdit[]; + /** + * Side-channel metadata mirroring {@link SessionState._meta}, propagated + * to clients via per-session state subscriptions. + * Producers SHOULD use namespaced keys; consumers MUST ignore unknown + * keys. Use the typed accessors in `sessionState.ts` (e.g. + * `readSessionGitState`) for well-known slots. + */ + readonly _meta?: SessionMeta; } export interface IAgentSessionProjectInfo { diff --git a/src/vs/platform/agentHost/common/state/sessionState.ts b/src/vs/platform/agentHost/common/state/sessionState.ts index 59fbe08fce4fa..896be3ca489b9 100644 --- a/src/vs/platform/agentHost/common/state/sessionState.ts +++ b/src/vs/platform/agentHost/common/state/sessionState.ts @@ -267,3 +267,95 @@ export type ComponentToState = { [StateComponents.Session]: SessionState; [StateComponents.Terminal]: TerminalState; }; + +// ---- SessionMeta accessors ------------------------------------------------- + +/** + * VS Code-side alias for the protocol's open `_meta` property bag on + * {@link SessionState}. Keys SHOULD be namespaced (e.g. `git`, `vscode.foo`) + * to avoid collisions; values MUST be JSON-serializable. + */ +export type SessionMeta = Record; + +/** + * Reserved key under {@link SessionMeta} for the well-known git-state + * payload. Value at this key, when present, MUST be shaped like + * {@link ISessionGitState}. This is a VS Code-specific convention layered + * on top of the protocol's generic `_meta` bag — the protocol itself does + * not know about git state. + */ +export const SESSION_META_GIT_KEY = 'git'; + +/** + * Git state of a session's working directory, carried under + * {@link SessionMeta} at {@link SESSION_META_GIT_KEY}. Used by clients to + * drive source-control affordances (e.g. PR/merge buttons in the Agents + * app). + * + * All fields are optional — agents that do not track a particular field + * should omit it rather than send a placeholder, so clients can distinguish + * "unknown" from "known to be zero". + */ +export interface ISessionGitState { + /** Whether the working directory has a `github.com` git remote. */ + readonly hasGitHubRemote?: boolean; + /** Current branch name. */ + readonly branchName?: string; + /** Base branch the work targets (e.g. `main`). */ + readonly baseBranchName?: string; + /** Upstream tracking branch (e.g. `origin/feature`). */ + readonly upstreamBranchName?: string; + /** Number of commits the upstream branch has ahead of the local branch. */ + readonly incomingChanges?: number; + /** Number of commits the local branch has ahead of the upstream branch. */ + readonly outgoingChanges?: number; + /** Number of files with uncommitted changes. */ + readonly uncommittedChanges?: number; +} + +/** + * Reads the well-known git-state payload from {@link SessionMeta}, if + * present. Returns `undefined` when the meta bag is absent or the value at + * the git key is not a plain object (e.g. an array or a primitive). + * Individual fields with wrong types are silently dropped so partial state + * still propagates. + */ +export function readSessionGitState(meta: SessionMeta | undefined): ISessionGitState | undefined { + const value = meta?.[SESSION_META_GIT_KEY]; + if (!value || typeof value !== 'object' || Array.isArray(value)) { + return undefined; + } + const raw = value as Record; + const result: { + hasGitHubRemote?: boolean; + branchName?: string; + baseBranchName?: string; + upstreamBranchName?: string; + incomingChanges?: number; + outgoingChanges?: number; + uncommittedChanges?: number; + } = {}; + if (typeof raw['hasGitHubRemote'] === 'boolean') { result.hasGitHubRemote = raw['hasGitHubRemote']; } + if (typeof raw['branchName'] === 'string') { result.branchName = raw['branchName']; } + if (typeof raw['baseBranchName'] === 'string') { result.baseBranchName = raw['baseBranchName']; } + if (typeof raw['upstreamBranchName'] === 'string') { result.upstreamBranchName = raw['upstreamBranchName']; } + if (typeof raw['incomingChanges'] === 'number') { result.incomingChanges = raw['incomingChanges']; } + if (typeof raw['outgoingChanges'] === 'number') { result.outgoingChanges = raw['outgoingChanges']; } + if (typeof raw['uncommittedChanges'] === 'number') { result.uncommittedChanges = raw['uncommittedChanges']; } + return result; +} + +/** + * Returns a new {@link SessionMeta} with the git-state payload set to + * `gitState`, or with the git slot removed if `gitState` is `undefined`. + * Returns `undefined` if the result would be empty. + */ +export function withSessionGitState(meta: SessionMeta | undefined, gitState: ISessionGitState | undefined): SessionMeta | undefined { + const next: { [key: string]: unknown } = { ...meta }; + if (gitState !== undefined) { + next[SESSION_META_GIT_KEY] = gitState; + } else { + delete next[SESSION_META_GIT_KEY]; + } + return Object.keys(next).length > 0 ? next : undefined; +} diff --git a/src/vs/platform/agentHost/node/agentHostGitService.ts b/src/vs/platform/agentHost/node/agentHostGitService.ts index ce73918591628..865ce104d7816 100644 --- a/src/vs/platform/agentHost/node/agentHostGitService.ts +++ b/src/vs/platform/agentHost/node/agentHostGitService.ts @@ -6,6 +6,7 @@ import * as cp from 'child_process'; import { URI } from '../../../base/common/uri.js'; import { createDecorator } from '../../instantiation/common/instantiation.js'; +import type { ISessionGitState } from '../common/state/sessionState.js'; export const IAgentHostGitService = createDecorator('agentHostGitService'); @@ -19,6 +20,13 @@ export interface IAgentHostGitService { getWorktreeRoots(workingDirectory: URI): Promise; addWorktree(repositoryRoot: URI, worktree: URI, branchName: string, startPoint: string): Promise; removeWorktree(repositoryRoot: URI, worktree: URI): Promise; + /** + * Computes the {@link ISessionGitState} for the working directory by + * shelling out to `git`. Returns undefined if the directory is not a + * git work tree. Called on session open and after each turn completes + * so the UI always reflects current branch/remote/change state. + */ + getSessionGitState(workingDirectory: URI): Promise; } function getCommonBranchPriority(branch: string): number { @@ -106,6 +114,63 @@ export class AgentHostGitService implements IAgentHostGitService { await this._runGit(repositoryRoot, ['worktree', 'remove', '--force', worktree.fsPath], { timeout: 30_000, throwOnError: true }); } + async getSessionGitState(workingDirectory: URI): Promise { + return this._computeSessionGitState(workingDirectory); + } + + private async _computeSessionGitState(workingDirectory: URI): Promise { + // Bail fast if not inside a git work tree. + const inside = await this._runGit(workingDirectory, ['rev-parse', '--is-inside-work-tree']); + if (inside?.trim() !== 'true') { + return undefined; + } + + // Run all probes in parallel. Each handles its own errors and returns + // undefined on failure so we can populate fields independently. + const [ + statusOutput, + remotesOutput, + defaultBranchRef, + ] = await Promise.all([ + this._runGit(workingDirectory, ['status', '-b', '--porcelain=v2']), + this._runGit(workingDirectory, ['remote', '-v']), + this._runGit(workingDirectory, ['symbolic-ref', '--quiet', 'refs/remotes/origin/HEAD']), + ]); + + const status = parseGitStatusV2(statusOutput); + const hasGitHubRemote = parseHasGitHubRemote(remotesOutput); + const baseBranchName = parseDefaultBranchRef(defaultBranchRef); + + // `git status -b --porcelain=v2` only emits ahead/behind counts when the + // branch has an upstream tracking ref. For agent-host worktrees the + // branch is typically created locally with no upstream, so the user can + // have committed work that we'd otherwise report as 0 outgoing changes + // and the "Create PR" button would never appear. Fall back to counting + // commits relative to the base branch — that matches what the user + // actually cares about for "is there work to PR?". + let outgoingChanges = status.outgoingChanges; + if (outgoingChanges === undefined && baseBranchName && status.branchName && status.branchName !== baseBranchName) { + const ahead = await this._runGit(workingDirectory, ['rev-list', '--count', `${baseBranchName}..HEAD`]); + const parsed = ahead === undefined ? NaN : Number(ahead.trim()); + if (Number.isFinite(parsed)) { + outgoingChanges = parsed; + } + } + + const result: ISessionGitState = { + hasGitHubRemote, + branchName: status.branchName, + baseBranchName, + upstreamBranchName: status.upstreamBranchName, + incomingChanges: status.incomingChanges, + outgoingChanges, + uncommittedChanges: status.uncommittedChanges, + }; + // Strip undefined fields so the resulting object is the same regardless + // of which probes succeeded — easier to compare in tests. + return stripUndefined(result); + } + private _runGit(workingDirectory: URI, args: readonly string[], options?: { readonly timeout?: number; readonly throwOnError?: boolean }): Promise { return new Promise((resolve, reject) => { cp.execFile('git', [...args], { cwd: workingDirectory.fsPath, timeout: options?.timeout ?? 5000 }, (error, stdout, stderr) => { @@ -122,3 +187,79 @@ export class AgentHostGitService implements IAgentHostGitService { }); } } + +/** + * Parses output of `git status -b --porcelain=v2`. The format is documented + * at https://git-scm.com/docs/git-status. We care about a few header lines: + * + * # branch.head + * # branch.upstream + * # branch.ab + - + * + * and the count of non-header lines (one per changed entry). + * + * Exported for tests. + */ +export function parseGitStatusV2(output: string | undefined): { + branchName?: string; + upstreamBranchName?: string; + outgoingChanges?: number; + incomingChanges?: number; + uncommittedChanges?: number; +} { + if (!output) { + return {}; + } + let branchName: string | undefined; + let upstreamBranchName: string | undefined; + let outgoingChanges: number | undefined; + let incomingChanges: number | undefined; + let uncommittedChanges = 0; + for (const rawLine of output.split(/\r?\n/g)) { + const line = rawLine.trimEnd(); + if (!line) { continue; } + if (line.startsWith('# branch.head ')) { + const head = line.substring('# branch.head '.length).trim(); + // `(detached)` is what git emits for a detached HEAD. Treat as no branch. + branchName = head === '(detached)' ? undefined : head; + } else if (line.startsWith('# branch.upstream ')) { + upstreamBranchName = line.substring('# branch.upstream '.length).trim(); + } else if (line.startsWith('# branch.ab ')) { + const m = /^# branch\.ab \+(\d+) -(\d+)$/.exec(line); + if (m) { + outgoingChanges = Number(m[1]); + incomingChanges = Number(m[2]); + } + } else if (!line.startsWith('#')) { + uncommittedChanges++; + } + } + return { branchName, upstreamBranchName, outgoingChanges, incomingChanges, uncommittedChanges }; +} + +/** Exported for tests. */ +export function parseHasGitHubRemote(remotesOutput: string | undefined): boolean | undefined { + if (remotesOutput === undefined) { + return undefined; + } + if (!remotesOutput.trim()) { + return false; + } + return /github\.com[:\/]/i.test(remotesOutput); +} + +/** Exported for tests. */ +export function parseDefaultBranchRef(symbolicRefOutput: string | undefined): string | undefined { + const ref = symbolicRefOutput?.trim(); + if (!ref) { return undefined; } + const prefix = 'refs/remotes/origin/'; + return ref.startsWith(prefix) ? ref.substring(prefix.length) : ref; +} + +function stripUndefined(obj: T): T { + const out: Record = {}; + for (const [k, v] of Object.entries(obj)) { + if (v !== undefined) { out[k] = v; } + } + return out as T; +} diff --git a/src/vs/platform/agentHost/node/agentHostMain.ts b/src/vs/platform/agentHost/node/agentHostMain.ts index 7a862c9699e62..5fb0af528b3fc 100644 --- a/src/vs/platform/agentHost/node/agentHostMain.ts +++ b/src/vs/platform/agentHost/node/agentHostMain.ts @@ -89,7 +89,8 @@ function startAgentHost(): void { // Create the real service implementation that lives in this process let agentService: AgentService; try { - agentService = new AgentService(logService, fileService, sessionDataService, productService); + const gitService = new AgentHostGitService(); + agentService = new AgentService(logService, fileService, sessionDataService, productService, gitService); const pluginManager = new AgentPluginManager(URI.file(environmentService.userDataPath), fileService, logService); const diServices = new ServiceCollection(); diServices.set(INativeEnvironmentService, environmentService); @@ -102,7 +103,7 @@ function startAgentHost(): void { diServices.set(IAgentHostTerminalManager, agentService.terminalManager); const instantiationService = new InstantiationService(diServices); - diServices.set(IAgentHostGitService, instantiationService.createInstance(AgentHostGitService)); + diServices.set(IAgentHostGitService, gitService); agentService.registerProvider(instantiationService.createInstance(CopilotAgent)); } catch (err) { logService.error('Failed to create AgentService', err); diff --git a/src/vs/platform/agentHost/node/agentHostServerMain.ts b/src/vs/platform/agentHost/node/agentHostServerMain.ts index 9b09da0f7ea26..cb5b930d7424c 100644 --- a/src/vs/platform/agentHost/node/agentHostServerMain.ts +++ b/src/vs/platform/agentHost/node/agentHostServerMain.ts @@ -163,7 +163,8 @@ async function main(): Promise { const sessionDataService = new SessionDataService(URI.file(environmentService.userDataPath), fileService, logService); // Create the agent service (owns AgentHostStateManager + AgentSideEffects internally) - const agentService = new AgentService(logService, fileService, sessionDataService, productService); + const gitService = new AgentHostGitService(); + const agentService = new AgentService(logService, fileService, sessionDataService, productService, gitService); disposables.add(agentService); // Register agents @@ -179,8 +180,8 @@ async function main(): Promise { diServices.set(IAgentPluginManager, pluginManager); diServices.set(IDiffComputeService, disposables.add(new NodeWorkerDiffComputeService(logService))); diServices.set(IAgentHostTerminalManager, agentService.terminalManager); + diServices.set(IAgentHostGitService, gitService); const instantiationService = new InstantiationService(diServices); - diServices.set(IAgentHostGitService, instantiationService.createInstance(AgentHostGitService)); const copilotAgent = disposables.add(instantiationService.createInstance(CopilotAgent)); agentService.registerProvider(copilotAgent); log('CopilotAgent registered'); diff --git a/src/vs/platform/agentHost/node/agentHostStateManager.ts b/src/vs/platform/agentHost/node/agentHostStateManager.ts index 9bb890e45f401..9ef374d36a116 100644 --- a/src/vs/platform/agentHost/node/agentHostStateManager.ts +++ b/src/vs/platform/agentHost/node/agentHostStateManager.ts @@ -10,7 +10,7 @@ import { ILogService } from '../../log/common/log.js'; import { ActionType, NotificationType, ActionEnvelope, ActionOrigin, INotification, SessionAction, RootAction, StateAction, isRootAction, isSessionAction, type TerminalAction } from '../common/state/sessionActions.js'; import type { IStateSnapshot } from '../common/state/sessionProtocol.js'; import { rootReducer, sessionReducer } from '../common/state/sessionReducers.js'; -import { createRootState, createSessionState, SessionLifecycle, type RootState, type SessionState, type SessionSummary, type Turn, type URI, ROOT_STATE_URI } from '../common/state/sessionState.js'; +import { createRootState, createSessionState, SessionLifecycle, type RootState, type SessionMeta, type SessionState, type SessionSummary, type Turn, type URI, ROOT_STATE_URI } from '../common/state/sessionState.js'; import { IPermissionsValue, platformRootSchema } from '../common/agentHostSchema.js'; import { SessionConfigKey } from '../common/sessionConfigKeys.js'; @@ -215,6 +215,21 @@ export class AgentHostStateManager extends Disposable { }); } + // ---- Session meta ------------------------------------------------------- + + /** + * Replaces `state._meta` on a session by dispatching a + * {@link ActionType.SessionMetaChanged} action so the change flows + * through the action envelope (and thus to all live subscribers). + * + * The full `_meta` object is replaced (not merged) so callers stay in + * control of the convention for their own keys; use the `withSessionXxx` + * helpers in `sessionState.ts` to combine slots. + */ + setSessionMeta(session: URI, meta: SessionMeta | undefined): void { + this.dispatchServerAction({ type: ActionType.SessionMetaChanged, session, _meta: meta }); + } + // ---- Turn tracking ------------------------------------------------------ /** diff --git a/src/vs/platform/agentHost/node/agentService.ts b/src/vs/platform/agentHost/node/agentService.ts index 631b8e0dc7cec..553f65cf3a228 100644 --- a/src/vs/platform/agentHost/node/agentService.ts +++ b/src/vs/platform/agentHost/node/agentService.ts @@ -7,6 +7,7 @@ import { decodeBase64, VSBuffer } from '../../../base/common/buffer.js'; import { toErrorMessage } from '../../../base/common/errorMessage.js'; import { Emitter } from '../../../base/common/event.js'; import { Disposable, DisposableStore } from '../../../base/common/lifecycle.js'; +import { equals as objectEquals } from '../../../base/common/objects.js'; import { observableValue } from '../../../base/common/observable.js'; import { URI } from '../../../base/common/uri.js'; import { generateUuid } from '../../../base/common/uuid.js'; @@ -19,13 +20,14 @@ import { ISessionDataService } from '../common/sessionDataService.js'; import { ActionType, ActionEnvelope, INotification, RootAction, SessionAction, TerminalAction, isSessionAction } from '../common/state/sessionActions.js'; import type { CreateTerminalParams, ResolveSessionConfigResult, SessionConfigCompletionsResult } from '../common/state/protocol/commands.js'; import { AhpErrorCodes, AHP_SESSION_NOT_FOUND, ContentEncoding, JSON_RPC_INTERNAL_ERROR, ProtocolError, type DirectoryEntry, type ResourceCopyParams, type ResourceCopyResult, type ResourceDeleteParams, type ResourceDeleteResult, type ResourceListResult, type ResourceMoveParams, type ResourceMoveResult, type ResourceReadResult, type ResourceWriteParams, type ResourceWriteResult, type IStateSnapshot } from '../common/state/sessionProtocol.js'; -import { ResponsePartKind, SessionStatus, ToolCallConfirmationReason, ToolCallStatus, ToolResultContentType, TurnState, buildSubagentSessionUri, parseSubagentSessionUri, type ResponsePart, type SessionConfigState, type ISessionFileDiff, type SessionSummary, type ToolCallCompletedState, type ToolResultSubagentContent, type Turn } from '../common/state/sessionState.js'; +import { ResponsePartKind, SessionStatus, ToolCallConfirmationReason, ToolCallStatus, ToolResultContentType, TurnState, buildSubagentSessionUri, parseSubagentSessionUri, readSessionGitState, withSessionGitState, type ResponsePart, type SessionConfigState, type ISessionFileDiff, type SessionSummary, type ToolCallCompletedState, type ToolResultSubagentContent, type Turn } from '../common/state/sessionState.js'; import { IProductService } from '../../product/common/productService.js'; import { AgentConfigurationService, IAgentConfigurationService } from './agentConfigurationService.js'; import { AgentSideEffects } from './agentSideEffects.js'; import { AgentHostTerminalManager, type IAgentHostTerminalManager } from './agentHostTerminalManager.js'; import { ISessionDbUriFields, parseSessionDbUri } from './copilot/fileEditTracker.js'; import { AgentHostStateManager } from './agentHostStateManager.js'; +import { IAgentHostGitService } from './agentHostGitService.js'; /** * The agent service implementation that runs inside the agent-host utility @@ -88,6 +90,7 @@ export class AgentService extends Disposable implements IAgentService { private readonly _fileService: IFileService, private readonly _sessionDataService: ISessionDataService, private readonly _productService: IProductService, + private readonly _gitService: IAgentHostGitService, ) { super(); this._logService.info('AgentService initialized'); @@ -109,6 +112,10 @@ export class AgentService extends Disposable implements IAgentService { getAgent: session => this._findProviderForSession(session), sessionDataService: this._sessionDataService, agents: this._agents, + onTurnComplete: session => { + const workingDirStr = this._stateManager.getSessionState(session)?.summary.workingDirectory; + this._attachGitState(URI.parse(session), workingDirStr ? URI.parse(workingDirStr) : undefined); + }, })); // Terminal management — the terminal manager listens to the state @@ -307,9 +314,44 @@ export class AgentService extends Disposable implements IAgentService { } this._stateManager.dispatchServerAction({ type: ActionType.SessionReady, session: session.toString() }); + // Lazily compute git state for sessions with a working directory; + // attaches under `state._meta.git` once ready. + this._attachGitState(session, created.workingDirectory ?? config?.workingDirectory); + return session; } + /** + * Fire-and-forget probe that resolves the session's git state for its + * working directory (if any) and merges it into `state._meta.git` via + * the state manager. Failures are logged; sessions simply remain without + * git state. + */ + private _attachGitState(session: URI, workingDirectory: URI | undefined): void { + if (!workingDirectory) { + return; + } + this._gitService.getSessionGitState(workingDirectory).then( + gitState => { + if (!gitState) { + return; + } + const sessionKey = session.toString(); + const current = this._stateManager.getSessionState(sessionKey)?._meta; + // Skip the action if the computed git state hasn't changed; this is + // called after every turn, so deduping avoids needless action churn. + if (objectEquals(readSessionGitState(current), gitState)) { + return; + } + const next = withSessionGitState(current, gitState); + this._stateManager.setSessionMeta(sessionKey, next); + }, + e => { + this._logService.warn(`[AgentService] Failed to compute git state for ${session}`, e); + }, + ); + } + private _persistConfigValues(session: URI, values: Record): void { let ref; try { @@ -406,6 +448,19 @@ export class AgentService extends Disposable implements IAgentService { if (!snapshot) { throw new Error(`Cannot subscribe to unknown resource: ${resourceStr}`); } + + // Ensure git state has been computed for this session. When the snapshot + // already existed (e.g. seeded by list query, or restored earlier), the + // restore path that normally calls `_attachGitState` is skipped — so + // trigger it lazily here for the first subscriber. `_attachGitState` + // is async and updates `_meta.git` once ready, which clients see via + // the normal state-update stream. + const sessionState = this._stateManager.getSessionState(resourceStr); + if (sessionState && readSessionGitState(sessionState._meta) === undefined) { + const wd = sessionState.summary?.workingDirectory; + this._attachGitState(resource, wd ? URI.parse(wd) : undefined); + } + return snapshot; } @@ -556,6 +611,12 @@ export class AgentService extends Disposable implements IAgentService { this._stateManager.restoreSession(summary, turns); + // Restore persisted `_meta` (e.g. git state) onto the new session + // state. This dispatches a SessionMetaChanged action. + if (meta._meta) { + this._stateManager.setSessionMeta(sessionStr, meta._meta); + } + // Resolve the session config so clients (e.g. the running-session // auto-approve picker) can render session-mutable properties for // sessions that were not created in the current process lifetime. @@ -573,6 +634,10 @@ export class AgentService extends Disposable implements IAgentService { } this._logService.info(`[AgentService] Restored session ${sessionStr} with ${turns.length} turns`); + + // Lazily compute git state for sessions with a working directory; + // attaches under `state._meta.git` once ready. + this._attachGitState(session, meta.workingDirectory); } async resourceRead(uri: URI): Promise { diff --git a/src/vs/platform/agentHost/node/agentSideEffects.ts b/src/vs/platform/agentHost/node/agentSideEffects.ts index 241048fffc449..ea66d5083d475 100644 --- a/src/vs/platform/agentHost/node/agentSideEffects.ts +++ b/src/vs/platform/agentHost/node/agentSideEffects.ts @@ -47,6 +47,12 @@ export interface IAgentSideEffectsOptions { readonly agents: IObservable; /** Session data service for cleaning up per-session data on disposal. */ readonly sessionDataService: ISessionDataService; + /** + * Called after each top-level session turn completes so git state can be + * refreshed and published via `SessionMetaChanged`. Subagent turns are + * excluded — only the parent session URI is passed. + */ + readonly onTurnComplete: (session: ProtocolURI) => void; } /** A progress event that was deferred because its subagent session does not exist yet. */ @@ -296,11 +302,13 @@ export class AgentSideEffects extends Disposable { } // After a turn completes (idle event), flush any pending debounced - // diff computation and compute final diffs immediately. + // diff computation and compute final diffs immediately, then refresh + // git state so the toolbar buttons reflect post-turn repository state. if (e.type === 'idle') { this._cancelDebouncedDiffComputation(sessionKey); this._computeSessionDiffs(sessionKey, turnId); this._tryConsumeNextQueuedMessage(sessionKey); + this._options.onTurnComplete(sessionKey as ProtocolURI); } // Steering message was consumed by the agent — remove from protocol state diff --git a/src/vs/platform/agentHost/test/common/sessionTestHelpers.ts b/src/vs/platform/agentHost/test/common/sessionTestHelpers.ts index 467b5427c7646..435a5a203fe5a 100644 --- a/src/vs/platform/agentHost/test/common/sessionTestHelpers.ts +++ b/src/vs/platform/agentHost/test/common/sessionTestHelpers.ts @@ -155,6 +155,26 @@ export function encodeString(text: string): Uint8Array { return new TextEncoder().encode(text); } +/** + * Returns a no-op {@link IAgentHostGitService} suitable for tests that + * exercise the {@link AgentService} but don't care about git state. + * Tests that DO care about git state should pass their own implementation. + */ +export function createNoopGitService(): import('../../node/agentHostGitService.js').IAgentHostGitService { + return { + _serviceBrand: undefined, + isInsideWorkTree: async () => false, + getCurrentBranch: async () => undefined, + getDefaultBranch: async () => undefined, + getBranches: async () => [], + getRepositoryRoot: async () => undefined, + getWorktreeRoots: async () => [], + addWorktree: async () => { }, + removeWorktree: async () => { }, + getSessionGitState: async () => undefined, + }; +} + function createReference(object: T): IReference { return { object, diff --git a/src/vs/platform/agentHost/test/node/agentHostGitService.integrationTest.ts b/src/vs/platform/agentHost/test/node/agentHostGitService.integrationTest.ts new file mode 100644 index 0000000000000..a19bddeee7e15 --- /dev/null +++ b/src/vs/platform/agentHost/test/node/agentHostGitService.integrationTest.ts @@ -0,0 +1,127 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +/** + * Integration tests for {@link AgentHostGitService} that spawn real `git` against + * temporary on-disk repositories. Kept out of the unit-test suite because they + * require `git` on PATH and do real filesystem and process work — same split as + * the git extension (pure parser tests in `git.test.ts`, on-disk tests in + * `smoke.test.ts`). + * + * Run via `scripts/test-integration.sh`. + */ + +import assert from 'assert'; +import * as cp from 'child_process'; +import { mkdtempSync, rmSync } from 'fs'; +import { tmpdir } from 'os'; +import { join } from '../../../../base/common/path.js'; +import { URI } from '../../../../base/common/uri.js'; +import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../base/test/common/utils.js'; +import { AgentHostGitService } from '../../node/agentHostGitService.js'; + +suite('AgentHostGitService - getSessionGitState (real git)', () => { + ensureNoDisposablesAreLeakedInTestSuite(); + + // Skip the on-disk git tests when `git` is not on PATH (e.g. minimal CI). + const hasGit = (() => { + try { cp.execFileSync('git', ['--version'], { stdio: 'ignore' }); return true; } catch { return false; } + })(); + + let tmpRoot: string | undefined; + let svc: AgentHostGitService | undefined; + + setup(() => { + tmpRoot = undefined; + svc = new AgentHostGitService(); + }); + + teardown(() => { + if (tmpRoot) { + rmSync(tmpRoot, { recursive: true, force: true }); + } + }); + + function initRepo(opts?: { remote?: string; baseBranch?: string }): string { + tmpRoot = mkdtempSync(join(tmpdir(), 'agent-host-git-')); + const env = { ...process.env, GIT_AUTHOR_NAME: 't', GIT_AUTHOR_EMAIL: 't@t', GIT_COMMITTER_NAME: 't', GIT_COMMITTER_EMAIL: 't@t' }; + const run = (...args: string[]) => cp.execFileSync('git', args, { cwd: tmpRoot!, env, stdio: 'pipe' }); + run('init', '-q', '-b', opts?.baseBranch ?? 'main'); + run('commit', '-q', '--allow-empty', '-m', 'initial'); + if (opts?.remote) { + run('remote', 'add', 'origin', opts.remote); + } + return tmpRoot!; + } + + (hasGit ? test : test.skip)('returns undefined for a non-git directory', async () => { + const dir = mkdtempSync(join(tmpdir(), 'agent-host-nongit-')); + tmpRoot = dir; + const result = await svc!.getSessionGitState(URI.file(dir)); + assert.strictEqual(result, undefined); + }); + + (hasGit ? test : test.skip)('reports branch, github remote and clean state for a fresh repo', async () => { + const dir = initRepo({ remote: 'https://github.com/owner/repo.git' }); + const result = await svc!.getSessionGitState(URI.file(dir)); + assert.ok(result, 'expected git state'); + assert.strictEqual(result.branchName, 'main'); + assert.strictEqual(result.hasGitHubRemote, true); + assert.strictEqual(result.uncommittedChanges, 0); + // No upstream configured for the fresh local branch. + assert.strictEqual(result.upstreamBranchName, undefined); + assert.strictEqual(result.outgoingChanges, undefined); + assert.strictEqual(result.incomingChanges, undefined); + }); + + (hasGit ? test : test.skip)('counts uncommitted changes', async () => { + const dir = initRepo({ remote: 'git@gitlab.com:owner/repo.git' }); + const fs = await import('fs/promises'); + await fs.writeFile(join(dir, 'a.txt'), 'hello'); + await fs.writeFile(join(dir, 'b.txt'), 'world'); + const result = await svc!.getSessionGitState(URI.file(dir)); + assert.ok(result); + assert.strictEqual(result.uncommittedChanges, 2); + assert.strictEqual(result.hasGitHubRemote, false); + }); + + (hasGit ? test : test.skip)('reports outgoingChanges relative to base branch when local branch has no upstream', async () => { + // Create a bare "remote" repo and set up the working repo so that + // `refs/remotes/origin/HEAD` exists (required for baseBranchName parsing). + const remoteDir = mkdtempSync(join(tmpdir(), 'agent-host-remote-')); + const env = { ...process.env, GIT_AUTHOR_NAME: 't', GIT_AUTHOR_EMAIL: 't@t', GIT_COMMITTER_NAME: 't', GIT_COMMITTER_EMAIL: 't@t' }; + try { + cp.execFileSync('git', ['init', '-q', '--bare', '-b', 'main'], { cwd: remoteDir, env, stdio: 'pipe' }); + tmpRoot = mkdtempSync(join(tmpdir(), 'agent-host-git-')); + const run = (...args: string[]) => cp.execFileSync('git', args, { cwd: tmpRoot!, env, stdio: 'pipe' }); + run('init', '-q', '-b', 'main'); + run('commit', '-q', '--allow-empty', '-m', 'initial'); + run('remote', 'add', 'origin', `https://github.com/owner/repo.git`); + // Use a separate "upload" remote pointing at the bare repo to populate + // the origin/main remote-tracking ref without changing the GitHub URL + // we're testing for hasGitHubRemote detection. + run('remote', 'add', 'tmp', remoteDir); + run('push', '-q', 'tmp', 'main:main'); + // Create the origin/main ref locally without any network round-trip. + run('update-ref', 'refs/remotes/origin/main', 'refs/heads/main'); + run('symbolic-ref', 'refs/remotes/origin/HEAD', 'refs/remotes/origin/main'); + + // Branch off and add two commits without setting an upstream. + run('checkout', '-q', '-b', 'feature', '--no-track'); + run('commit', '-q', '--allow-empty', '-m', 'one'); + run('commit', '-q', '--allow-empty', '-m', 'two'); + + const result = await svc!.getSessionGitState(URI.file(tmpRoot!)); + assert.ok(result, 'expected git state'); + assert.strictEqual(result.branchName, 'feature'); + assert.strictEqual(result.baseBranchName, 'main'); + assert.strictEqual(result.upstreamBranchName, undefined); + assert.strictEqual(result.outgoingChanges, 2); + assert.strictEqual(result.uncommittedChanges, 0); + } finally { + rmSync(remoteDir, { recursive: true, force: true }); + } + }); +}); diff --git a/src/vs/platform/agentHost/test/node/agentHostGitService.test.ts b/src/vs/platform/agentHost/test/node/agentHostGitService.test.ts index 4bf2f2ec0b461..25a0c66bdfe28 100644 --- a/src/vs/platform/agentHost/test/node/agentHostGitService.test.ts +++ b/src/vs/platform/agentHost/test/node/agentHostGitService.test.ts @@ -5,7 +5,7 @@ import assert from 'assert'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../base/test/common/utils.js'; -import { getBranchCompletions } from '../../node/agentHostGitService.js'; +import { getBranchCompletions, parseDefaultBranchRef, parseGitStatusV2, parseHasGitHubRemote } from '../../node/agentHostGitService.js'; suite('AgentHostGitService', () => { ensureNoDisposablesAreLeakedInTestSuite(); @@ -30,4 +30,91 @@ suite('AgentHostGitService', () => { ['main', 'master', 'maintenance'], ); }); + + suite('parseGitStatusV2', () => { + test('parses a clean checkout with upstream', () => { + const out = [ + '# branch.oid 0123456789abcdef0123456789abcdef01234567', + '# branch.head main', + '# branch.upstream origin/main', + '# branch.ab +0 -0', + ].join('\n'); + assert.deepStrictEqual(parseGitStatusV2(out), { + branchName: 'main', + upstreamBranchName: 'origin/main', + outgoingChanges: 0, + incomingChanges: 0, + uncommittedChanges: 0, + }); + }); + + test('parses a dirty branch ahead and behind upstream', () => { + const out = [ + '# branch.oid 0123456789abcdef0123456789abcdef01234567', + '# branch.head feature', + '# branch.upstream origin/feature', + '# branch.ab +3 -2', + '1 .M N... 100644 100644 100644 abc abc src/a.ts', + '2 R. N... 100644 100644 100644 abc abc R100 src/b.ts\tsrc/old-b.ts', + '? src/untracked.ts', + ].join('\n'); + assert.deepStrictEqual(parseGitStatusV2(out), { + branchName: 'feature', + upstreamBranchName: 'origin/feature', + outgoingChanges: 3, + incomingChanges: 2, + uncommittedChanges: 3, + }); + }); + + test('treats (detached) HEAD as no branch and omits upstream/ab when absent', () => { + const out = [ + '# branch.oid 0123456789abcdef0123456789abcdef01234567', + '# branch.head (detached)', + ].join('\n'); + assert.deepStrictEqual(parseGitStatusV2(out), { + branchName: undefined, + upstreamBranchName: undefined, + outgoingChanges: undefined, + incomingChanges: undefined, + uncommittedChanges: 0, + }); + }); + + test('returns empty object for undefined input', () => { + assert.deepStrictEqual(parseGitStatusV2(undefined), {}); + }); + }); + + suite('parseHasGitHubRemote', () => { + test('detects ssh github remote', () => { + assert.strictEqual(parseHasGitHubRemote('origin\tgit@github.com:owner/repo.git (fetch)\n'), true); + }); + test('detects https github remote', () => { + assert.strictEqual(parseHasGitHubRemote('origin\thttps://github.com/owner/repo.git (fetch)\n'), true); + }); + test('returns false for non-github remotes', () => { + assert.strictEqual(parseHasGitHubRemote('origin\thttps://gitlab.com/owner/repo.git (fetch)\n'), false); + }); + test('returns false when there are no remotes', () => { + assert.strictEqual(parseHasGitHubRemote(''), false); + }); + test('returns undefined when probe failed (output absent)', () => { + assert.strictEqual(parseHasGitHubRemote(undefined), undefined); + }); + }); + + suite('parseDefaultBranchRef', () => { + test('strips refs/remotes/origin/ prefix', () => { + assert.strictEqual(parseDefaultBranchRef('refs/remotes/origin/main\n'), 'main'); + }); + test('returns the ref as-is when prefix is not present', () => { + assert.strictEqual(parseDefaultBranchRef('main'), 'main'); + }); + test('returns undefined for empty/missing output', () => { + assert.strictEqual(parseDefaultBranchRef(undefined), undefined); + assert.strictEqual(parseDefaultBranchRef(' '), undefined); + }); + }); }); + diff --git a/src/vs/platform/agentHost/test/node/agentService.test.ts b/src/vs/platform/agentHost/test/node/agentService.test.ts index fb82ce83ce6c0..e0f531fed291c 100644 --- a/src/vs/platform/agentHost/test/node/agentService.test.ts +++ b/src/vs/platform/agentHost/test/node/agentService.test.ts @@ -22,9 +22,9 @@ import { ActionType, ActionEnvelope } from '../../common/state/sessionActions.js import { SessionActiveClient, ResponsePartKind, SessionLifecycle, ToolCallConfirmationReason, ToolCallStatus, ToolResultContentType, TurnState, buildSubagentSessionUri, type MarkdownResponsePart, type ToolCallCompletedState, type ToolCallResponsePart } from '../../common/state/sessionState.js'; import { IProductService } from '../../../product/common/productService.js'; import { AgentService } from '../../node/agentService.js'; -import { MockAgent } from './mockAgent.js'; +import { MockAgent, ScriptedMockAgent } from './mockAgent.js'; import { mapSessionEvents, type ISessionEvent } from '../../node/copilot/mapSessionEvents.js'; -import { createSessionDataService } from '../common/sessionTestHelpers.js'; +import { createNoopGitService, createSessionDataService } from '../common/sessionTestHelpers.js'; /** * Loads a JSONL fixture of raw Copilot SDK events, runs them through @@ -55,9 +55,10 @@ suite('AgentService (node dispatcher)', () => { let service: AgentService; let copilotAgent: MockAgent; let fileService: FileService; + let nullSessionDataService: ISessionDataService; setup(async () => { - const nullSessionDataService: ISessionDataService = { + nullSessionDataService = { _serviceBrand: undefined, getSessionDataDir: () => URI.parse('inmemory:/session-data'), getSessionDataDirById: () => URI.parse('inmemory:/session-data'), @@ -74,7 +75,7 @@ suite('AgentService (node dispatcher)', () => { await fileService.createFolder(URI.from({ scheme: Schemas.inMemory, path: '/testDir' })); await fileService.writeFile(URI.from({ scheme: Schemas.inMemory, path: '/testDir/file.txt' }), VSBuffer.fromString('hello')); - service = disposables.add(new AgentService(new NullLogService(), fileService, nullSessionDataService, { _serviceBrand: undefined } as IProductService)); + service = disposables.add(new AgentService(new NullLogService(), fileService, nullSessionDataService, { _serviceBrand: undefined } as IProductService, createNoopGitService())); copilotAgent = new MockAgent('copilot'); disposables.add(toDisposable(() => copilotAgent.dispose())); }); @@ -127,6 +128,31 @@ suite('AgentService (node dispatcher)', () => { assert.strictEqual(AgentSession.provider(session), 'copilot'); }); + test('honors requested session URI', async () => { + service.registerProvider(copilotAgent); + + const requestedSession = AgentSession.uri('copilot', 'requested-session'); + const session = await service.createSession({ provider: 'copilot', session: requestedSession }); + assert.strictEqual(session.toString(), requestedSession.toString()); + }); + + test('scripted mock agent honors requested session URI', async () => { + const agent = new ScriptedMockAgent(); + disposables.add(toDisposable(() => agent.dispose())); + + const requestedSession = AgentSession.uri('mock', 'requested-session'); + const result = await agent.createSession({ session: requestedSession }); + const sessions = await agent.listSessions(); + + assert.deepStrictEqual({ + created: result.session.toString(), + listed: sessions.some(s => s.session.toString() === requestedSession.toString()), + }, { + created: requestedSession.toString(), + listed: true, + }); + }); + test('uses default provider when none specified', async () => { service.registerProvider(copilotAgent); @@ -206,7 +232,7 @@ suite('AgentService (node dispatcher)', () => { // Manually add the session to the mock (agent as unknown as { _sessions: Map })._sessions.set(sessionId, sessionUri); - const svc = disposables.add(new AgentService(new NullLogService(), fileService, sessionDataService, { _serviceBrand: undefined } as IProductService)); + const svc = disposables.add(new AgentService(new NullLogService(), fileService, sessionDataService, { _serviceBrand: undefined } as IProductService, createNoopGitService())); svc.registerProvider(agent); const sessions = await svc.listSessions(); @@ -242,6 +268,130 @@ suite('AgentService (node dispatcher)', () => { assert.strictEqual(sessions[0].summary, 'User first message'); }); + test('createSession attaches git state into state _meta when working directory is present', async () => { + const workingDirectory = URI.file('/workspace/repo'); + const gitState = { + hasGitHubRemote: true, + branchName: 'feature/x', + baseBranchName: 'main', + upstreamBranchName: 'origin/feature/x', + incomingChanges: 1, + outgoingChanges: 2, + uncommittedChanges: 3, + }; + const calls: string[] = []; + const gitService = { + _serviceBrand: undefined, + isInsideWorkTree: async () => true, + getCurrentBranch: async () => undefined, + getDefaultBranch: async () => undefined, + getBranches: async () => [], + getRepositoryRoot: async () => undefined, + getWorktreeRoots: async () => [], + addWorktree: async () => { }, + removeWorktree: async () => { }, + getSessionGitState: async (uri: URI) => { calls.push(uri.fsPath); return gitState; }, + }; + const localService = disposables.add(new AgentService(new NullLogService(), fileService, nullSessionDataService, { _serviceBrand: undefined } as IProductService, gitService)); + const agent = new MockAgent('copilot'); + disposables.add(toDisposable(() => agent.dispose())); + agent.resolvedWorkingDirectory = workingDirectory; + agent.sessionMetadataOverrides = { workingDirectory }; + localService.registerProvider(agent); + + const session = await localService.createSession({ provider: 'copilot' }); + + // _attachGitState is fire-and-forget; drain microtasks until the + // git service's promise has resolved and setSessionMeta has run. + for (let i = 0; i < 5; i++) { + await Promise.resolve(); + } + + const sessions = await localService.listSessions(); + assert.strictEqual(sessions.length, 1); + assert.deepStrictEqual(calls, [workingDirectory.fsPath]); + assert.deepStrictEqual( + localService.stateManager.getSessionState(session.toString())?._meta, + { git: gitState }, + ); + }); + + test('createSession skips git overlay when no working directory or no git state', async () => { + const gitService = { + _serviceBrand: undefined, + isInsideWorkTree: async () => false, + getCurrentBranch: async () => undefined, + getDefaultBranch: async () => undefined, + getBranches: async () => [], + getRepositoryRoot: async () => undefined, + getWorktreeRoots: async () => [], + addWorktree: async () => { }, + removeWorktree: async () => { }, + getSessionGitState: async () => undefined, + }; + const localService = disposables.add(new AgentService(new NullLogService(), fileService, nullSessionDataService, { _serviceBrand: undefined } as IProductService, gitService)); + const agent = new MockAgent('copilot'); + disposables.add(toDisposable(() => agent.dispose())); + // No resolvedWorkingDirectory set on the mock. + localService.registerProvider(agent); + + const session = await localService.createSession({ provider: 'copilot' }); + for (let i = 0; i < 5; i++) { + await Promise.resolve(); + } + const sessions = await localService.listSessions(); + + assert.strictEqual(sessions.length, 1); + assert.strictEqual(localService.stateManager.getSessionState(session.toString())?._meta, undefined); + }); + + test('subscribe lazily attaches git state when an existing session has no _meta.git', async () => { + // Regression test: previously AgentService was constructed without + // a git service, so _attachGitState always bailed and `_meta.git` + // was never populated. This test ensures the lazy-fire path on + // subscribe() actually invokes the git service and writes git + // state into the session's `_meta`. + const workingDirectory = URI.file('/workspace/repo'); + const gitState = { + hasGitHubRemote: false, + branchName: 'feature/lazy', + baseBranchName: 'main', + upstreamBranchName: undefined, + incomingChanges: 0, + outgoingChanges: 0, + uncommittedChanges: 0, + }; + const calls: string[] = []; + const gitService = createNoopGitService(); + gitService.getSessionGitState = async (uri: URI) => { calls.push(uri.fsPath); return gitState; }; + const localService = disposables.add(new AgentService(new NullLogService(), fileService, nullSessionDataService, { _serviceBrand: undefined } as IProductService, gitService)); + const agent = new MockAgent('copilot'); + disposables.add(toDisposable(() => agent.dispose())); + agent.resolvedWorkingDirectory = workingDirectory; + agent.sessionMetadataOverrides = { workingDirectory }; + localService.registerProvider(agent); + + // Seed a session and clear its _meta so subscribe must lazily + // recompute git state. + const session = await localService.createSession({ provider: 'copilot' }); + for (let i = 0; i < 5; i++) { + await Promise.resolve(); + } + localService.stateManager.setSessionMeta(session.toString(), undefined); + calls.length = 0; + + await localService.subscribe(session); + for (let i = 0; i < 5; i++) { + await Promise.resolve(); + } + + assert.deepStrictEqual(calls, [workingDirectory.fsPath]); + assert.deepStrictEqual( + localService.stateManager.getSessionState(session.toString())?._meta, + { git: gitState }, + ); + }); + test('createSession stores live session config', async () => { service.registerProvider(copilotAgent); @@ -560,7 +710,7 @@ suite('AgentService (node dispatcher)', () => { const sessionDataService = createSessionDataService(sessionDb); const localAgent = new MockAgent('copilot'); disposables.add(toDisposable(() => localAgent.dispose())); - const localService = disposables.add(new AgentService(new NullLogService(), fileService, sessionDataService, { _serviceBrand: undefined } as IProductService)); + const localService = disposables.add(new AgentService(new NullLogService(), fileService, sessionDataService, { _serviceBrand: undefined } as IProductService, createNoopGitService())); localService.registerProvider(localAgent); await localService.createSession({ provider: 'copilot', config: { autoApprove: 'autoApprove' } }); @@ -578,7 +728,7 @@ suite('AgentService (node dispatcher)', () => { const sessionDataService = createSessionDataService(sessionDb); const localAgent = new MockAgent('copilot'); disposables.add(toDisposable(() => localAgent.dispose())); - const localService = disposables.add(new AgentService(new NullLogService(), fileService, sessionDataService, { _serviceBrand: undefined } as IProductService)); + const localService = disposables.add(new AgentService(new NullLogService(), fileService, sessionDataService, { _serviceBrand: undefined } as IProductService, createNoopGitService())); localService.registerProvider(localAgent); await localService.createSession({ provider: 'copilot' }); @@ -594,7 +744,7 @@ suite('AgentService (node dispatcher)', () => { const sessionDataService = createSessionDataService(sessionDb); const localAgent = new MockAgent('copilot'); disposables.add(toDisposable(() => localAgent.dispose())); - const localService = disposables.add(new AgentService(new NullLogService(), fileService, sessionDataService, { _serviceBrand: undefined } as IProductService)); + const localService = disposables.add(new AgentService(new NullLogService(), fileService, sessionDataService, { _serviceBrand: undefined } as IProductService, createNoopGitService())); localService.registerProvider(localAgent); // Create a session on the agent backend (no config) so listSessions can find it @@ -627,7 +777,7 @@ suite('AgentService (node dispatcher)', () => { const sessionDataService = createSessionDataService(sessionDb); const localAgent = new MockAgent('copilot'); disposables.add(toDisposable(() => localAgent.dispose())); - const localService = disposables.add(new AgentService(new NullLogService(), fileService, sessionDataService, { _serviceBrand: undefined } as IProductService)); + const localService = disposables.add(new AgentService(new NullLogService(), fileService, sessionDataService, { _serviceBrand: undefined } as IProductService, createNoopGitService())); localService.registerProvider(localAgent); const session = await localService.createSession({ provider: 'copilot', config: { autoApprove: 'autoApprove' } }); @@ -654,7 +804,7 @@ suite('AgentService (node dispatcher)', () => { const sessionDataService = createSessionDataService(sessionDb); const localAgent = new MockAgent('copilot'); disposables.add(toDisposable(() => localAgent.dispose())); - const localService = disposables.add(new AgentService(new NullLogService(), fileService, sessionDataService, { _serviceBrand: undefined } as IProductService)); + const localService = disposables.add(new AgentService(new NullLogService(), fileService, sessionDataService, { _serviceBrand: undefined } as IProductService, createNoopGitService())); localService.registerProvider(localAgent); const { session } = await localAgent.createSession(); diff --git a/src/vs/platform/agentHost/test/node/agentSideEffects.test.ts b/src/vs/platform/agentHost/test/node/agentSideEffects.test.ts index 72bd6e6c971e5..2485516dc9943 100644 --- a/src/vs/platform/agentHost/test/node/agentSideEffects.test.ts +++ b/src/vs/platform/agentHost/test/node/agentSideEffects.test.ts @@ -27,7 +27,7 @@ import { AgentService } from '../../node/agentService.js'; import { AgentSideEffects, IAgentSideEffectsOptions } from '../../node/agentSideEffects.js'; import { SessionDatabase } from '../../node/sessionDatabase.js'; import { AgentHostStateManager } from '../../node/agentHostStateManager.js'; -import { createNullSessionDataService, createSessionDataService } from '../common/sessionTestHelpers.js'; +import { createNoopGitService, createNullSessionDataService, createSessionDataService } from '../common/sessionTestHelpers.js'; import { MockAgent } from './mockAgent.js'; // ---- Tests ------------------------------------------------------------------ @@ -97,6 +97,7 @@ suite('AgentSideEffects', () => { getAgent: () => agent, agents: agentList, sessionDataService: createNullSessionDataService(), + onTurnComplete: () => { }, }); }); @@ -151,6 +152,7 @@ suite('AgentSideEffects', () => { getAgent: () => undefined, agents: emptyAgents, sessionDataService: {} as ISessionDataService, + onTurnComplete: () => { }, }); const envelopes: ActionEnvelope[] = []; @@ -1228,6 +1230,7 @@ suite('AgentSideEffects', () => { getAgent: () => localAgent, agents: observableValue('agents', [localAgent]), sessionDataService, + onTurnComplete: () => { }, }); localStateManager.createSession({ @@ -1256,7 +1259,7 @@ suite('AgentSideEffects', () => { const sessionDataService = createSessionDataService(sessionDb); const localAgent = new MockAgent(); disposables.add(toDisposable(() => localAgent.dispose())); - const localService = disposables.add(new AgentService(new NullLogService(), fileService, sessionDataService, { _serviceBrand: undefined } as IProductService)); + const localService = disposables.add(new AgentService(new NullLogService(), fileService, sessionDataService, { _serviceBrand: undefined } as IProductService, createNoopGitService())); localService.registerProvider(localAgent); // Create a session on the agent backend @@ -1276,7 +1279,7 @@ suite('AgentSideEffects', () => { const sessionDataService = createSessionDataService(sessionDb); const localAgent = new MockAgent(); disposables.add(toDisposable(() => localAgent.dispose())); - const localService = disposables.add(new AgentService(new NullLogService(), fileService, sessionDataService, { _serviceBrand: undefined } as IProductService)); + const localService = disposables.add(new AgentService(new NullLogService(), fileService, sessionDataService, { _serviceBrand: undefined } as IProductService, createNoopGitService())); localService.registerProvider(localAgent); // Create a session on the agent backend @@ -1309,6 +1312,7 @@ suite('AgentSideEffects', () => { getAgent: () => localAgent, agents: observableValue('agents', [localAgent]), sessionDataService, + onTurnComplete: () => { }, }); const session = localStateManager.createSession({ diff --git a/src/vs/platform/agentHost/test/node/copilotAgent.test.ts b/src/vs/platform/agentHost/test/node/copilotAgent.test.ts index f4f73c5a9d936..e7b6b177a2e9a 100644 --- a/src/vs/platform/agentHost/test/node/copilotAgent.test.ts +++ b/src/vs/platform/agentHost/test/node/copilotAgent.test.ts @@ -55,6 +55,7 @@ class TestAgentHostGitService implements IAgentHostGitService { this.addedWorktrees.push({ repositoryRoot, worktree, branchName, startPoint }); } async removeWorktree(): Promise { } + async getSessionGitState(): Promise { return undefined; } } class TestAgentHostTerminalManager implements IAgentHostTerminalManager { diff --git a/src/vs/platform/agentHost/test/node/copilotGitProject.test.ts b/src/vs/platform/agentHost/test/node/copilotGitProject.test.ts index 8129d781195d9..89b219bd1a72a 100644 --- a/src/vs/platform/agentHost/test/node/copilotGitProject.test.ts +++ b/src/vs/platform/agentHost/test/node/copilotGitProject.test.ts @@ -24,6 +24,7 @@ class TestAgentHostGitService implements IAgentHostGitService { async getWorktreeRoots(): Promise { return this.worktreeRoots; } async addWorktree(): Promise { } async removeWorktree(): Promise { } + async getSessionGitState(): Promise { return undefined; } } suite('Copilot Git Project', () => { diff --git a/src/vs/platform/agentHost/test/node/mockAgent.ts b/src/vs/platform/agentHost/test/node/mockAgent.ts index 1388b069ebcd2..649ce829d788b 100644 --- a/src/vs/platform/agentHost/test/node/mockAgent.ts +++ b/src/vs/platform/agentHost/test/node/mockAgent.ts @@ -77,9 +77,9 @@ export class MockAgent implements IAgent { /** Optional override for the working directory returned by createSession. */ resolvedWorkingDirectory: URI | undefined; - async createSession(_config?: IAgentCreateSessionConfig): Promise { - const rawId = `${this.id}-session-${this._nextId++}`; - const session = AgentSession.uri(this.id, rawId); + async createSession(config?: IAgentCreateSessionConfig): Promise { + const session = config?.session ?? AgentSession.uri(this.id, `${this.id}-session-${this._nextId++}`); + const rawId = AgentSession.id(session); this._sessions.set(rawId, session); return { session, project: mockProject(this.id), workingDirectory: this.resolvedWorkingDirectory }; } @@ -240,9 +240,9 @@ export class ScriptedMockAgent implements IAgent { })); } - async createSession(_config?: IAgentCreateSessionConfig): Promise { - const rawId = `mock-session-${this._nextId++}`; - const session = AgentSession.uri('mock', rawId); + async createSession(config?: IAgentCreateSessionConfig): Promise { + const session = config?.session ?? AgentSession.uri('mock', `mock-session-${this._nextId++}`); + const rawId = AgentSession.id(session); this._sessions.set(rawId, session); return { session, project: mockProject(this.id) }; } diff --git a/src/vs/sessions/common/agentHostSessionWorkspace.ts b/src/vs/sessions/common/agentHostSessionWorkspace.ts index af6f0350253c0..0e283bc585382 100644 --- a/src/vs/sessions/common/agentHostSessionWorkspace.ts +++ b/src/vs/sessions/common/agentHostSessionWorkspace.ts @@ -4,9 +4,12 @@ *--------------------------------------------------------------------------------------------*/ import { Codicon } from '../../base/common/codicons.js'; +import { match as matchGlob } from '../../base/common/glob.js'; import { extUri, basename } from '../../base/common/resources.js'; import { ThemeIcon } from '../../base/common/themables.js'; import { URI } from '../../base/common/uri.js'; +import type { ISessionGitState } from '../../platform/agentHost/common/state/sessionState.js'; +import { IConfigurationService } from '../../platform/configuration/common/configuration.js'; import { ISessionWorkspace } from '../services/sessions/common/session.js'; export interface IAgentHostSessionProjectSummary { @@ -19,21 +22,81 @@ export interface IAgentHostSessionWorkspaceOptions { readonly fallbackIcon: ThemeIcon; readonly requiresWorkspaceTrust: boolean; readonly description?: string; + /** + * Configured `git.branchProtection` glob patterns. Used to compute + * `baseBranchProtected` on the resulting repository. + */ + readonly branchProtectionPatterns?: readonly string[]; +} + +/** + * Returns true when `branchName` matches any of the configured + * `git.branchProtection` glob patterns. + */ +export function matchesAnyBranchProtectionPattern(branchName: string, patterns: readonly string[] | undefined): boolean { + if (!patterns) { + return false; + } + for (const pattern of patterns) { + const trimmed = pattern.trim(); + if (trimmed && matchGlob(trimmed, branchName)) { + return true; + } + } + return false; +} + +/** + * Reads `git.branchProtection` from configuration and normalizes the result + * into an array of trimmed, non-empty pattern strings. + */ +export function readBranchProtectionPatterns(configurationService: IConfigurationService): readonly string[] { + const raw = configurationService.getValue('git.branchProtection') ?? []; + const list = Array.isArray(raw) ? raw : [raw]; + return list + .map(p => typeof p === 'string' ? p.trim() : '') + .filter(p => p !== ''); } export function agentHostSessionWorkspaceKey(workspace: ISessionWorkspace | undefined): string | undefined { const repository = workspace?.repositories[0]; - return workspace && repository ? `${workspace.label}\n${extUri.getComparisonKey(repository.uri)}\n${repository.workingDirectory ? extUri.getComparisonKey(repository.workingDirectory) : ''}` : undefined; + if (!workspace || !repository) { + return undefined; + } + return [ + workspace.label, + extUri.getComparisonKey(repository.uri), + repository.workingDirectory ? extUri.getComparisonKey(repository.workingDirectory) : '', + repository.branchName ?? '', + repository.baseBranchName ?? '', + String(repository.baseBranchProtected ?? ''), + String(repository.hasGitHubRemote ?? ''), + repository.upstreamBranchName ?? '', + String(repository.incomingChanges ?? ''), + String(repository.outgoingChanges ?? ''), + String(repository.uncommittedChanges ?? ''), + ].join('\n'); } -export function buildAgentHostSessionWorkspace(project: IAgentHostSessionProjectSummary | undefined, workingDirectory: URI | undefined, options: IAgentHostSessionWorkspaceOptions): ISessionWorkspace | undefined { +export function buildAgentHostSessionWorkspace(project: IAgentHostSessionProjectSummary | undefined, workingDirectory: URI | undefined, options: IAgentHostSessionWorkspaceOptions, gitState?: ISessionGitState): ISessionWorkspace | undefined { + const baseBranchName = gitState?.baseBranchName; + const baseBranchProtected = baseBranchName !== undefined + ? matchesAnyBranchProtectionPattern(baseBranchName, options.branchProtectionPatterns) + : undefined; + const hasGitHubRemote = gitState?.hasGitHubRemote; + const upstreamBranchName = gitState?.upstreamBranchName; + const incomingChanges = gitState?.incomingChanges; + const outgoingChanges = gitState?.outgoingChanges; + const uncommittedChanges = gitState?.uncommittedChanges; + const branchName = gitState?.branchName; + const gitFields = { branchName, baseBranchName, baseBranchProtected, hasGitHubRemote, upstreamBranchName, incomingChanges, outgoingChanges, uncommittedChanges }; if (project) { const repositoryWorkingDirectory = extUri.isEqual(workingDirectory, project.uri) ? undefined : workingDirectory; return { label: options.providerLabel ? `${project.displayName} [${options.providerLabel}]` : project.displayName, description: options.description, icon: Codicon.repo, - repositories: [{ uri: project.uri, workingDirectory: repositoryWorkingDirectory, detail: undefined, baseBranchName: undefined, baseBranchProtected: undefined }], + repositories: [{ uri: project.uri, workingDirectory: repositoryWorkingDirectory, detail: undefined, ...gitFields }], requiresWorkspaceTrust: options.requiresWorkspaceTrust, }; } @@ -47,7 +110,7 @@ export function buildAgentHostSessionWorkspace(project: IAgentHostSessionProject label: options.providerLabel ? `${folderName} [${options.providerLabel}]` : folderName, description: options.description, icon: options.fallbackIcon, - repositories: [{ uri: workingDirectory, workingDirectory: undefined, detail: undefined, baseBranchName: undefined, baseBranchProtected: undefined }], + repositories: [{ uri: workingDirectory, workingDirectory: undefined, detail: undefined, ...gitFields }], requiresWorkspaceTrust: options.requiresWorkspaceTrust, }; } diff --git a/src/vs/sessions/contrib/agentHost/browser/baseAgentHostSessionsProvider.ts b/src/vs/sessions/contrib/agentHost/browser/baseAgentHostSessionsProvider.ts index 3ef9840244325..6c09eeabe5523 100644 --- a/src/vs/sessions/contrib/agentHost/browser/baseAgentHostSessionsProvider.ts +++ b/src/vs/sessions/contrib/agentHost/browser/baseAgentHostSessionsProvider.ts @@ -19,7 +19,7 @@ import { ResolveSessionConfigResult } from '../../../../platform/agentHost/commo import { NotificationType } from '../../../../platform/agentHost/common/state/protocol/notifications.js'; import { FileEdit, ModelSelection, RootConfigState, RootState, SessionState, SessionSummary, SessionStatus as ProtocolSessionStatus } from '../../../../platform/agentHost/common/state/protocol/state.js'; import { ActionType, isSessionAction } from '../../../../platform/agentHost/common/state/sessionActions.js'; -import { StateComponents } from '../../../../platform/agentHost/common/state/sessionState.js'; +import { readSessionGitState, StateComponents, type ISessionGitState } from '../../../../platform/agentHost/common/state/sessionState.js'; import { ChatViewPaneTarget, IChatWidgetService } from '../../../../workbench/contrib/chat/browser/chat.js'; import { IChatSendRequestOptions, IChatService } from '../../../../workbench/contrib/chat/common/chatService/chatService.js'; import { IChatSessionFileChange, IChatSessionsService } from '../../../../workbench/contrib/chat/common/chatSessionsService.js'; @@ -48,7 +48,7 @@ export interface IAgentHostAdapterOptions { /** Loading observable wired to the provider's authentication-pending state. */ readonly loading: IObservable; /** Builds the session workspace from session metadata; provider-specific (icon, providerLabel, requiresWorkspaceTrust). */ - readonly buildWorkspace: (project: IAgentSessionMetadata['project'], workingDirectory: URI | undefined) => ISessionWorkspace | undefined; + readonly buildWorkspace: (project: IAgentSessionMetadata['project'], workingDirectory: URI | undefined, gitState: ISessionGitState | undefined) => ISessionWorkspace | undefined; /** Optional URI mapping for diff entries (remote uses `toAgentHostUri`; local uses identity). */ readonly mapDiffUri?: (uri: URI) => URI; } @@ -87,6 +87,13 @@ export class AgentHostSessionAdapter implements ISession { readonly agentProvider: string; + // Retained so we can rebuild `workspace` when only `_meta` changes via + // a `SessionMetaChanged` action dispatched on session open (without a full + // list refresh). See `_applySessionMetaFromState` / `setMeta`. + private _project: IAgentSessionMetadata['project']; + private _workingDirectory: URI | undefined; + private _meta: IAgentSessionMetadata['_meta']; + constructor( metadata: IAgentSessionMetadata, providerId: string, @@ -113,7 +120,12 @@ export class AgentHostSessionAdapter implements ISession { this.modelId = observableValue('modelId', metadata.model ? `${resourceScheme}:${metadata.model.id}` : undefined); this.lastTurnEnd = observableValue('lastTurnEnd', metadata.modifiedTime ? new Date(metadata.modifiedTime) : undefined); this.description = observableValue('description', _options.description); - this.workspace = observableValue('workspace', _options.buildWorkspace(metadata.project, metadata.workingDirectory)); + this._project = metadata.project; + this._workingDirectory = metadata.workingDirectory; + this._meta = metadata._meta; + const initialGitState = readSessionGitState(this._meta); + const initialWorkspace = _options.buildWorkspace(this._project, this._workingDirectory, initialGitState); + this.workspace = observableValue('workspace', initialWorkspace); this.loading = _options.loading; if (metadata.isRead === false) { @@ -177,7 +189,17 @@ export class AgentHostSessionAdapter implements ISession { didChange = true; } - const workspace = this._options.buildWorkspace(metadata.project, metadata.workingDirectory); + this._project = metadata.project; + this._workingDirectory = metadata.workingDirectory; + // Only update `_meta` when the source actually provides one. `update()` + // is fed from SessionSummary (via `listSessions`/`sessionAdded` paths) + // which has no `_meta` field, so an undefined value here means "not + // included" rather than "cleared". `_meta` (e.g. git state) flows in + // exclusively via `setMeta` from `SessionState` subscription updates. + if (metadata._meta !== undefined) { + this._meta = metadata._meta; + } + const workspace = this._options.buildWorkspace(this._project, this._workingDirectory, readSessionGitState(this._meta)); if (agentHostSessionWorkspaceKey(workspace) !== agentHostSessionWorkspaceKey(this.workspace.get())) { this.workspace.set(workspace, undefined); didChange = true; @@ -207,6 +229,22 @@ export class AgentHostSessionAdapter implements ISession { return didChange; } + + /** + * Apply a `SessionState._meta` delta (fed from `_applySessionMetaFromState`) + * and rebuild the workspace if the git state changed. Returns `true` iff + * the workspace actually changed. + */ + setMeta(meta: IAgentSessionMetadata['_meta']): boolean { + this._meta = meta; + const gitState = readSessionGitState(this._meta); + const workspace = this._options.buildWorkspace(this._project, this._workingDirectory, gitState); + if (agentHostSessionWorkspaceKey(workspace) === agentHostSessionWorkspaceKey(this.workspace.get())) { + return false; + } + this.workspace.set(workspace, undefined); + return true; + } } // ============================================================================ @@ -425,6 +463,10 @@ export abstract class BaseAgentHostSessionsProvider extends Disposable implement this._ensureSessionCache(); for (const cached of this._sessionCache.values()) { if (cached.resource.toString() === resource.toString()) { + // Opening a session: subscribe to its AHP state so that + // `_meta` (e.g. lazy git state computed by the agent host) + // flows into the cached adapter. + this._ensureSessionStateSubscription(cached.sessionId); return cached; } } @@ -1027,12 +1069,38 @@ export abstract class BaseAgentHostSessionsProvider extends Disposable implement const ref = connection.getSubscription(StateComponents.Session, sessionUri); const store = new DisposableStore(); store.add(ref); - store.add(ref.object.onDidChange(state => this._seedRunningConfigFromState(sessionId, state))); + store.add(ref.object.onDidChange(state => { + this._applySessionStateUpdate(sessionId, state); + })); this._sessionStateSubscriptions.set(sessionId, store); const value = ref.object.value; if (value && !(value instanceof Error)) { - this._seedRunningConfigFromState(sessionId, value); + this._applySessionStateUpdate(sessionId, value); + } + } + + /** + * Fan-out for AHP `SessionState` snapshots: keeps both the running + * session config and the cached adapter's `_meta` (e.g. git state) in + * sync. + */ + private _applySessionStateUpdate(sessionId: string, state: SessionState): void { + this._seedRunningConfigFromState(sessionId, state); + this._applySessionMetaFromState(sessionId, state); + } + + private _applySessionMetaFromState(sessionId: string, state: SessionState): void { + const rawId = this._rawIdFromChatId(sessionId); + if (!rawId) { + return; + } + const cached = this._sessionCache.get(rawId); + if (!cached) { + return; + } + if (cached.setMeta(state._meta)) { + this._onDidChangeSessions.fire({ added: [], removed: [], changed: [cached] }); } } diff --git a/src/vs/sessions/contrib/agentHost/browser/localAgentHostSessionsProvider.ts b/src/vs/sessions/contrib/agentHost/browser/localAgentHostSessionsProvider.ts index dfe47123109b0..bee9a06960f22 100644 --- a/src/vs/sessions/contrib/agentHost/browser/localAgentHostSessionsProvider.ts +++ b/src/vs/sessions/contrib/agentHost/browser/localAgentHostSessionsProvider.ts @@ -12,6 +12,8 @@ import { ThemeIcon } from '../../../../base/common/themables.js'; import { URI } from '../../../../base/common/uri.js'; import { localize } from '../../../../nls.js'; import { IAgentConnection, IAgentHostService, type IAgentSessionMetadata } from '../../../../platform/agentHost/common/agentService.js'; +import type { ISessionGitState } from '../../../../platform/agentHost/common/state/sessionState.js'; +import { IConfigurationService } from '../../../../platform/configuration/common/configuration.js'; import { IFileDialogService } from '../../../../platform/dialogs/common/dialogs.js'; import { ILabelService } from '../../../../platform/label/common/label.js'; import { IChatWidgetService } from '../../../../workbench/contrib/chat/browser/chat.js'; @@ -19,7 +21,7 @@ import { IChatService } from '../../../../workbench/contrib/chat/common/chatServ import { IChatSessionsService } from '../../../../workbench/contrib/chat/common/chatSessionsService.js'; import { ILanguageModelsService } from '../../../../workbench/contrib/chat/common/languageModels.js'; import { BaseAgentHostSessionsProvider } from './baseAgentHostSessionsProvider.js'; -import { buildAgentHostSessionWorkspace } from '../../../common/agentHostSessionWorkspace.js'; +import { buildAgentHostSessionWorkspace, readBranchProtectionPatterns } from '../../../common/agentHostSessionWorkspace.js'; import { ISessionWorkspace, ISessionWorkspaceBrowseAction } from '../../../services/sessions/common/session.js'; import { toAgentHostUri } from '../../../../platform/agentHost/common/agentHostUri.js'; import { LOCAL_AGENT_HOST_PROVIDER_ID } from '../../../common/agentHostSessionsProvider.js'; @@ -52,6 +54,7 @@ export class LocalAgentHostSessionsProvider extends BaseAgentHostSessionsProvide @IChatWidgetService chatWidgetService: IChatWidgetService, @ILanguageModelsService languageModelsService: ILanguageModelsService, @ILabelService private readonly _labelService: ILabelService, + @IConfigurationService private readonly _configurationService: IConfigurationService, ) { super(chatSessionsService, chatService, chatWidgetService, languageModelsService); @@ -116,10 +119,11 @@ export class LocalAgentHostSessionsProvider extends BaseAgentHostSessionsProvide protected _adapterOptions() { return { description: this._localDescription, - buildWorkspace: (project: IAgentSessionMetadata['project'], workingDirectory: URI | undefined) => { + buildWorkspace: (project: IAgentSessionMetadata['project'], workingDirectory: URI | undefined, gitState: ISessionGitState | undefined) => { const uriForDescription = project?.uri ?? workingDirectory; const description = uriForDescription ? this._labelService.getUriLabel(dirname(uriForDescription), { relative: false }) : undefined; - return buildAgentHostSessionWorkspace(project, workingDirectory, { providerLabel: this._localLabel, fallbackIcon: Codicon.folder, requiresWorkspaceTrust: true, description }); + const branchProtectionPatterns = readBranchProtectionPatterns(this._configurationService); + return LocalAgentHostSessionsProvider.buildWorkspace(project, workingDirectory, this._localLabel, gitState, description, branchProtectionPatterns); }, }; } @@ -139,6 +143,10 @@ export class LocalAgentHostSessionsProvider extends BaseAgentHostSessionsProvide // -- Workspaces ---------------------------------------------------------- + static buildWorkspace(project: IAgentSessionMetadata['project'], workingDirectory: URI | undefined, providerLabel: string, gitState: ISessionGitState | undefined, description?: string, branchProtectionPatterns?: readonly string[]): ISessionWorkspace | undefined { + return buildAgentHostSessionWorkspace(project, workingDirectory, { providerLabel, fallbackIcon: Codicon.folder, requiresWorkspaceTrust: true, description, branchProtectionPatterns }, gitState); + } + resolveWorkspace(repositoryUri: URI): ISessionWorkspace | undefined { if (repositoryUri.scheme !== Schemas.file) { return undefined; @@ -149,7 +157,7 @@ export class LocalAgentHostSessionsProvider extends BaseAgentHostSessionsProvide description: this._labelService.getUriLabel(dirname(repositoryUri), { relative: false }), group: this.label, icon: Codicon.folder, - repositories: [{ uri: repositoryUri, workingDirectory: undefined, detail: undefined, baseBranchName: undefined, baseBranchProtected: undefined }], + repositories: [{ uri: repositoryUri, workingDirectory: undefined, detail: undefined, baseBranchName: undefined }], requiresWorkspaceTrust: true, }; } diff --git a/src/vs/sessions/contrib/agentHost/test/browser/localAgentHostSessionsProvider.test.ts b/src/vs/sessions/contrib/agentHost/test/browser/localAgentHostSessionsProvider.test.ts index 2aadbca6d3cae..d1832a54c6d49 100644 --- a/src/vs/sessions/contrib/agentHost/test/browser/localAgentHostSessionsProvider.test.ts +++ b/src/vs/sessions/contrib/agentHost/test/browser/localAgentHostSessionsProvider.test.ts @@ -20,6 +20,8 @@ import { NotificationType } from '../../../../../platform/agentHost/common/state import { SessionLifecycle, type AgentInfo, type ModelSelection, type RootState, type SessionConfigState, type SessionState } from '../../../../../platform/agentHost/common/state/protocol/state.js'; import { SessionStatus as ProtocolSessionStatus, StateComponents } from '../../../../../platform/agentHost/common/state/sessionState.js'; import { ActionType, type ActionEnvelope, type INotification } from '../../../../../platform/agentHost/common/state/sessionActions.js'; +import { IConfigurationService } from '../../../../../platform/configuration/common/configuration.js'; +import { TestConfigurationService } from '../../../../../platform/configuration/test/common/testConfigurationService.js'; import { IFileDialogService } from '../../../../../platform/dialogs/common/dialogs.js'; import { TestInstantiationService } from '../../../../../platform/instantiation/test/common/instantiationServiceMock.js'; import { IChatWidget, IChatWidgetService } from '../../../../../workbench/contrib/chat/browser/chat.js'; @@ -195,6 +197,7 @@ function createProvider(disposables: DisposableStore, agentHostService: MockAgen const instantiationService = disposables.add(new TestInstantiationService()); instantiationService.stub(IAgentHostService, agentHostService); + instantiationService.stub(IConfigurationService, new TestConfigurationService()); instantiationService.stub(IFileDialogService, {}); instantiationService.stub(IChatSessionsService, { getChatSessionContribution: (chatSessionType: string) => contributions.find(c => c.type === chatSessionType), diff --git a/src/vs/sessions/contrib/changes/browser/changesView.ts b/src/vs/sessions/contrib/changes/browser/changesView.ts index 6ad3277cfa3ac..03014b01c17e6 100644 --- a/src/vs/sessions/contrib/changes/browser/changesView.ts +++ b/src/vs/sessions/contrib/changes/browser/changesView.ts @@ -639,6 +639,9 @@ export class ChangesViewPane extends ViewPane { const changes = changesObs.read(reader); const viewMode = this.viewModel.viewModeObs.read(reader); const isLoading = this.viewModel.activeSessionIsLoadingObs.read(reader); + // Read session state so this autorun re-runs when git state (e.g. branch name) + // arrives asynchronously, since the tree root label depends on it. + this.viewModel.activeSessionStateObs.read(reader); if (!this.tree || isLoading) { return; diff --git a/src/vs/sessions/contrib/changes/browser/changesViewModel.ts b/src/vs/sessions/contrib/changes/browser/changesViewModel.ts index d1b0e4a8715f5..c26af162775bf 100644 --- a/src/vs/sessions/contrib/changes/browser/changesViewModel.ts +++ b/src/vs/sessions/contrib/changes/browser/changesViewModel.ts @@ -139,7 +139,20 @@ export class ChangesViewModel extends Disposable { this.activeSessionHasGitRepositoryObs = derived(reader => { const sessionType = this.activeSessionTypeObs.read(reader); const metadata = this._activeSessionMetadataObs.read(reader); - return sessionType === COPILOT_CLOUD_SESSION_TYPE || metadata?.repositoryPath !== undefined; + if (sessionType === COPILOT_CLOUD_SESSION_TYPE || metadata?.repositoryPath !== undefined) { + return true; + } + + // Fall back to reading details from repo on the session management service session + const activeSession = this.sessionManagementService.activeSession.read(reader); + const workspace = activeSession?.workspace.read(reader); + const repository = workspace?.repositories[0]; + return repository !== undefined && ( + repository.uncommittedChanges !== undefined || + repository.incomingChanges !== undefined || + repository.outgoingChanges !== undefined || + repository.upstreamBranchName !== undefined + ); }); // Active session first checkpoint ref @@ -388,9 +401,14 @@ export class ChangesViewModel extends Disposable { // Session state const workspaceRepository = workspace?.repositories[0]; const hasGitRepository = this.activeSessionHasGitRepositoryObs.read(reader); - const branchName = (sessionMetadata?.branchName ?? sessionMetadata?.branch) as string | undefined; - const baseBranchName = (sessionMetadata?.baseBranchName ?? sessionMetadata?.baseBranch) as string | undefined; - const isMergeBaseBranchProtected = workspaceRepository?.baseBranchProtected; + const branchName = (sessionMetadata?.branchName ?? sessionMetadata?.branch) as string | undefined + ?? workspaceRepository?.branchName; + const baseBranchName = (sessionMetadata?.baseBranchName ?? sessionMetadata?.baseBranch) as string | undefined + ?? workspaceRepository?.baseBranchName; + + // Fall back to reading details from repo on the session management service session + const isMergeBaseBranchProtected = (sessionMetadata?.baseBranchProtected as boolean | undefined) + ?? workspaceRepository?.baseBranchProtected; const isolationMode = workspaceRepository?.workingDirectory === undefined ? IsolationMode.Workspace : IsolationMode.Worktree; @@ -402,12 +420,12 @@ export class ChangesViewModel extends Disposable { (gitHubInfo.pullRequest.icon?.id === Codicon.gitPullRequestDraft.id || gitHubInfo.pullRequest.icon?.id === Codicon.gitPullRequest.id); - // Repository state - const hasGitHubRemote = (sessionMetadata?.hasGitHubRemote as boolean | undefined) === true; - const upstreamBranchName = sessionMetadata?.upstreamBranchName as string | undefined; - const incomingChanges = (sessionMetadata?.incomingChanges as number | undefined) ?? 0; - const outgoingChanges = (sessionMetadata?.outgoingChanges as number | undefined) ?? 0; - const uncommittedChanges = (sessionMetadata?.uncommittedChanges as number | undefined) ?? 0; + // Fall back to reading details from repo on the session management service session + const hasGitHubRemote = (sessionMetadata?.hasGitHubRemote as boolean | undefined) ?? workspaceRepository?.hasGitHubRemote ?? false; + const upstreamBranchName = (sessionMetadata?.upstreamBranchName as string | undefined) ?? workspaceRepository?.upstreamBranchName; + const incomingChanges = (sessionMetadata?.incomingChanges as number | undefined) ?? workspaceRepository?.incomingChanges ?? 0; + const outgoingChanges = (sessionMetadata?.outgoingChanges as number | undefined) ?? workspaceRepository?.outgoingChanges ?? 0; + const uncommittedChanges = (sessionMetadata?.uncommittedChanges as number | undefined) ?? workspaceRepository?.uncommittedChanges ?? 0; return { isolationMode, diff --git a/src/vs/sessions/contrib/chat/test/browser/sessionWorkspacePicker.test.ts b/src/vs/sessions/contrib/chat/test/browser/sessionWorkspacePicker.test.ts index f09d19059bd9d..79b25e2f2744e 100644 --- a/src/vs/sessions/contrib/chat/test/browser/sessionWorkspacePicker.test.ts +++ b/src/vs/sessions/contrib/chat/test/browser/sessionWorkspacePicker.test.ts @@ -50,7 +50,7 @@ function createMockProvider(id: string, opts?: { resolveWorkspace: (uri: URI): ISessionWorkspace => ({ label: uri.path.substring(1) || uri.path, icon: Codicon.folder, - repositories: [{ uri, workingDirectory: undefined, detail: undefined, baseBranchName: undefined, baseBranchProtected: undefined }], + repositories: [{ uri, workingDirectory: undefined, detail: undefined, baseBranchName: undefined }], requiresWorkspaceTrust: false, }), onDidChangeSessions: Event.None, diff --git a/src/vs/sessions/contrib/chat/test/browser/sessionsConfigurationService.test.ts b/src/vs/sessions/contrib/chat/test/browser/sessionsConfigurationService.test.ts index 83e627c025ec4..0d691444e24fd 100644 --- a/src/vs/sessions/contrib/chat/test/browser/sessionsConfigurationService.test.ts +++ b/src/vs/sessions/contrib/chat/test/browser/sessionsConfigurationService.test.ts @@ -32,7 +32,6 @@ function makeSession(opts: { repository?: URI; worktree?: URI } = {}): ISession workingDirectory: opts.worktree, detail: undefined, baseBranchName: undefined, - baseBranchProtected: undefined, }], requiresWorkspaceTrust: false, } : undefined; diff --git a/src/vs/sessions/contrib/copilotChatSessions/browser/copilotChatSessionsProvider.ts b/src/vs/sessions/contrib/copilotChatSessions/browser/copilotChatSessionsProvider.ts index c75cd66c3b8f7..afca3ffd79788 100644 --- a/src/vs/sessions/contrib/copilotChatSessions/browser/copilotChatSessionsProvider.ts +++ b/src/vs/sessions/contrib/copilotChatSessions/browser/copilotChatSessionsProvider.ts @@ -1112,14 +1112,13 @@ class AgentSessionAdapter implements ICopilotChatSession { } private _buildWorkspace(session: IAgentSession): ISessionWorkspace | undefined { - const [repoUri, worktreeUri, branchName, baseBranchName, baseBranchProtected] = this._extractRepositoryFromMetadata(session); + const [repoUri, worktreeUri, branchName, baseBranchName] = this._extractRepositoryFromMetadata(session); const repository: ISessionRepository = { uri: repoUri ?? URI.parse('unknown:///'), workingDirectory: worktreeUri, detail: branchName, baseBranchName, - baseBranchProtected, }; return { @@ -1134,10 +1133,10 @@ class AgentSessionAdapter implements ICopilotChatSession { * Extract repository/worktree information from session metadata. * Mirrors the logic in sessionsManagementService.getRepositoryFromMetadata(). */ - private _extractRepositoryFromMetadata(session: IAgentSession): [URI | undefined, URI | undefined, string | undefined, string | undefined, boolean | undefined] { + private _extractRepositoryFromMetadata(session: IAgentSession): [URI | undefined, URI | undefined, string | undefined, string | undefined] { const metadata = session.metadata; if (!metadata) { - return [undefined, undefined, undefined, undefined, undefined]; + return [undefined, undefined, undefined, undefined]; } if (session.providerType === AgentSessionProviders.Cloud) { @@ -1147,13 +1146,13 @@ class AgentSessionAdapter implements ICopilotChatSession { authority: 'github', path: `/${metadata.owner}/${metadata.name}/${encodeURIComponent(branch)}` }); - return [repositoryUri, undefined, undefined, undefined, undefined]; + return [repositoryUri, undefined, undefined, undefined]; } // Background/CLI sessions: check workingDirectoryPath first const workingDirectoryPath = metadata?.workingDirectoryPath as string | undefined; if (workingDirectoryPath) { - return [URI.file(workingDirectoryPath), undefined, undefined, undefined, undefined]; + return [URI.file(workingDirectoryPath), undefined, undefined, undefined]; } // Fall back to repositoryPath + worktreePath @@ -1165,14 +1164,12 @@ class AgentSessionAdapter implements ICopilotChatSession { const worktreeBranchName = metadata?.branchName as string | undefined; const worktreeBaseBranchName = metadata?.baseBranchName as string | undefined; - const worktreeBaseBranchProtected = metadata?.baseBranchProtected as boolean | undefined; return [ URI.isUri(repositoryPathUri) ? repositoryPathUri : undefined, URI.isUri(worktreePathUri) ? worktreePathUri : undefined, worktreeBranchName, worktreeBaseBranchName, - worktreeBaseBranchProtected, ]; } } @@ -2160,7 +2157,7 @@ export class CopilotChatSessionsProvider extends Disposable implements ISessions return { label: this._labelFromUri(uri), icon: this._iconFromUri(uri), - repositories: [{ uri, workingDirectory: undefined, detail: undefined, baseBranchName: undefined, baseBranchProtected: undefined }], + repositories: [{ uri, workingDirectory: undefined, detail: undefined, baseBranchName: undefined }], requiresWorkspaceTrust: true }; } @@ -2174,7 +2171,7 @@ export class CopilotChatSessionsProvider extends Disposable implements ISessions return { label: this._labelFromUri(uri), icon: this._iconFromUri(uri), - repositories: [{ uri, workingDirectory: undefined, detail: undefined, baseBranchName: undefined, baseBranchProtected: undefined }], + repositories: [{ uri, workingDirectory: undefined, detail: undefined, baseBranchName: undefined }], requiresWorkspaceTrust: false, }; } @@ -2192,7 +2189,7 @@ export class CopilotChatSessionsProvider extends Disposable implements ISessions ? localize('copilotProvider.workspaceGroupRepositories', "Repositories") : localize('copilotProvider.workspaceGroupFolders', "Folders"), icon: this._iconFromUri(repositoryUri), - repositories: [{ uri: repositoryUri, workingDirectory: undefined, detail: undefined, baseBranchName: undefined, baseBranchProtected: undefined }], + repositories: [{ uri: repositoryUri, workingDirectory: undefined, detail: undefined, baseBranchName: undefined }], requiresWorkspaceTrust: repositoryUri.scheme !== GITHUB_REMOTE_FILE_SCHEME }; } diff --git a/src/vs/sessions/contrib/remoteAgentHost/browser/remoteAgentHostSessionsProvider.ts b/src/vs/sessions/contrib/remoteAgentHost/browser/remoteAgentHostSessionsProvider.ts index 959c2e4b72a88..355a3f363ad46 100644 --- a/src/vs/sessions/contrib/remoteAgentHost/browser/remoteAgentHostSessionsProvider.ts +++ b/src/vs/sessions/contrib/remoteAgentHost/browser/remoteAgentHostSessionsProvider.ts @@ -17,9 +17,11 @@ import { localize } from '../../../../nls.js'; import { agentHostUri } from '../../../../platform/agentHost/common/agentHostFileSystemProvider.js'; import { AGENT_HOST_SCHEME, agentHostAuthority, toAgentHostUri } from '../../../../platform/agentHost/common/agentHostUri.js'; import { AgentSession, type IAgentConnection, type IAgentSessionMetadata } from '../../../../platform/agentHost/common/agentService.js'; +import type { ISessionGitState } from '../../../../platform/agentHost/common/state/sessionState.js'; import { IRemoteAgentHostService, RemoteAgentHostConnectionStatus } from '../../../../platform/agentHost/common/remoteAgentHostService.js'; import { IFileDialogService } from '../../../../platform/dialogs/common/dialogs.js'; import { ILabelService } from '../../../../platform/label/common/label.js'; +import { IConfigurationService } from '../../../../platform/configuration/common/configuration.js'; import { INotificationService } from '../../../../platform/notification/common/notification.js'; import { IStorageService, StorageScope, StorageTarget } from '../../../../platform/storage/common/storage.js'; import { IChatWidgetService } from '../../../../workbench/contrib/chat/browser/chat.js'; @@ -27,7 +29,7 @@ import { IChatService } from '../../../../workbench/contrib/chat/common/chatServ import { IChatSessionsService } from '../../../../workbench/contrib/chat/common/chatSessionsService.js'; import { ILanguageModelsService } from '../../../../workbench/contrib/chat/common/languageModels.js'; import { AgentHostSessionAdapter, BaseAgentHostSessionsProvider } from '../../agentHost/browser/baseAgentHostSessionsProvider.js'; -import { buildAgentHostSessionWorkspace } from '../../../common/agentHostSessionWorkspace.js'; +import { buildAgentHostSessionWorkspace, readBranchProtectionPatterns } from '../../../common/agentHostSessionWorkspace.js'; import { ISession, ISessionType, ISessionWorkspace, ISessionWorkspaceBrowseAction } from '../../../services/sessions/common/session.js'; import { remoteAgentHostSessionTypeId } from '../common/remoteAgentHostSessionType.js'; @@ -196,6 +198,7 @@ export class RemoteAgentHostSessionsProvider extends BaseAgentHostSessionsProvid @ILanguageModelsService languageModelsService: ILanguageModelsService, @IRemoteAgentHostService private readonly _remoteAgentHostService: IRemoteAgentHostService, @ILabelService private readonly _labelService: ILabelService, + @IConfigurationService private readonly _configurationService: IConfigurationService, ) { super(chatSessionsService, chatService, chatWidgetService, languageModelsService); @@ -258,10 +261,11 @@ export class RemoteAgentHostSessionsProvider extends BaseAgentHostSessionsProvid const web = this.isWebPlatform; return { description: web ? undefined : new MarkdownString().appendText(this.label), - buildWorkspace: (project: IAgentSessionMetadata['project'], workingDirectory: URI | undefined) => { + buildWorkspace: (project: IAgentSessionMetadata['project'], workingDirectory: URI | undefined, gitState: ISessionGitState | undefined) => { const uriForDescription = project?.uri ?? workingDirectory; const description = uriForDescription ? this._labelService.getUriLabel(dirname(uriForDescription), { relative: false }) : undefined; - return buildAgentHostSessionWorkspace(project, workingDirectory, { providerLabel: web ? undefined : this.label, fallbackIcon: Codicon.remote, requiresWorkspaceTrust: false, description }); + const branchProtectionPatterns = readBranchProtectionPatterns(this._configurationService); + return RemoteAgentHostSessionsProvider.buildWorkspace(project, workingDirectory, web ? undefined : this.label, gitState, description, branchProtectionPatterns); }, }; } @@ -505,6 +509,10 @@ export class RemoteAgentHostSessionsProvider extends BaseAgentHostSessionsProvid // -- Workspaces ---------------------------------------------------------- + static buildWorkspace(project: IAgentSessionMetadata['project'], workingDirectory: URI | undefined, providerLabel: string | undefined, gitState: ISessionGitState | undefined, description?: string, branchProtectionPatterns?: readonly string[]): ISessionWorkspace | undefined { + return buildAgentHostSessionWorkspace(project, workingDirectory, { providerLabel, fallbackIcon: Codicon.remote, requiresWorkspaceTrust: false, description, branchProtectionPatterns }, gitState); + } + private _buildWorkspaceFromUri(uri: URI): ISessionWorkspace { const folderName = basename(uri) || uri.path; return { @@ -512,7 +520,7 @@ export class RemoteAgentHostSessionsProvider extends BaseAgentHostSessionsProvid description: this._labelService.getUriLabel(dirname(uri), { relative: false }), group: this.label, icon: Codicon.remote, - repositories: [{ uri, workingDirectory: undefined, detail: undefined, baseBranchName: undefined, baseBranchProtected: undefined }], + repositories: [{ uri, workingDirectory: undefined, detail: undefined, baseBranchName: undefined }], requiresWorkspaceTrust: true, }; } diff --git a/src/vs/sessions/contrib/remoteAgentHost/test/browser/remoteAgentHostSessionsProvider.test.ts b/src/vs/sessions/contrib/remoteAgentHost/test/browser/remoteAgentHostSessionsProvider.test.ts index 83487264714ec..a8d2ef44c2564 100644 --- a/src/vs/sessions/contrib/remoteAgentHost/test/browser/remoteAgentHostSessionsProvider.test.ts +++ b/src/vs/sessions/contrib/remoteAgentHost/test/browser/remoteAgentHostSessionsProvider.test.ts @@ -19,6 +19,8 @@ import { SessionLifecycle, type AgentInfo, type ModelSelection, type RootState, import { ActionType, type ActionEnvelope, type INotification } from '../../../../../platform/agentHost/common/state/sessionActions.js'; import { SessionStatus as ProtocolSessionStatus, StateComponents } from '../../../../../platform/agentHost/common/state/sessionState.js'; import type { IAgentSubscription } from '../../../../../platform/agentHost/common/state/agentSubscription.js'; +import { IConfigurationService } from '../../../../../platform/configuration/common/configuration.js'; +import { TestConfigurationService } from '../../../../../platform/configuration/test/common/testConfigurationService.js'; import { IFileDialogService } from '../../../../../platform/dialogs/common/dialogs.js'; import { TestInstantiationService } from '../../../../../platform/instantiation/test/common/instantiationServiceMock.js'; import { INotificationService } from '../../../../../platform/notification/common/notification.js'; @@ -181,6 +183,7 @@ function createProvider(disposables: DisposableStore, connection: MockAgentConne const instantiationService = disposables.add(new TestInstantiationService()); instantiationService.stub(IFileDialogService, {}); + instantiationService.stub(IConfigurationService, new TestConfigurationService()); instantiationService.stub(INotificationService, { error: () => { } }); instantiationService.stub(IChatSessionsService, { getChatSessionContribution: () => ({ type: 'remote-test-copilot', name: 'test', displayName: 'Test', description: 'test', icon: undefined }), diff --git a/src/vs/sessions/contrib/terminal/test/browser/sessionsTerminalContribution.test.ts b/src/vs/sessions/contrib/terminal/test/browser/sessionsTerminalContribution.test.ts index ed396d300ee11..9e2ec230e58d7 100644 --- a/src/vs/sessions/contrib/terminal/test/browser/sessionsTerminalContribution.test.ts +++ b/src/vs/sessions/contrib/terminal/test/browser/sessionsTerminalContribution.test.ts @@ -56,7 +56,6 @@ function makeAgentSession(opts: { workingDirectory: opts.worktree, detail: undefined, baseBranchName: undefined, - baseBranchProtected: undefined, } : undefined; const chat: IChat = { resource: URI.parse('file:///session'), @@ -106,7 +105,6 @@ function makeNonAgentSession(opts: { repository?: URI; worktree?: URI; providerT workingDirectory: opts.worktree, detail: undefined, baseBranchName: undefined, - baseBranchProtected: undefined, } : undefined; const chat: IChat = { resource: URI.parse('file:///session'), diff --git a/src/vs/sessions/services/sessions/common/session.ts b/src/vs/sessions/services/sessions/common/session.ts index 4cd65187d6b69..1c798a0bedf97 100644 --- a/src/vs/sessions/services/sessions/common/session.ts +++ b/src/vs/sessions/services/sessions/common/session.ts @@ -88,10 +88,22 @@ export interface ISessionRepository { readonly workingDirectory: URI | undefined; /** Provider-chosen display detail (e.g., branch name, host name). */ readonly detail: string | undefined; + /** Current branch name. */ + readonly branchName?: string; /** Name of the base branch. */ readonly baseBranchName: string | undefined; /** Whether the base branch is protected (drives PR vs merge workflow). */ - readonly baseBranchProtected: boolean | undefined; + readonly baseBranchProtected?: boolean; + /** Whether the repository has a github.com remote. */ + readonly hasGitHubRemote?: boolean; + /** Upstream tracking branch name (e.g. `origin/feature`). */ + readonly upstreamBranchName?: string; + /** Number of commits the upstream branch is ahead of the local branch. */ + readonly incomingChanges?: number; + /** Number of commits the local branch is ahead of the upstream branch. */ + readonly outgoingChanges?: number; + /** Number of files with uncommitted changes. */ + readonly uncommittedChanges?: number; } /** diff --git a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSessionListController.ts b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSessionListController.ts index 55e5da5cbbf5f..796284f131512 100644 --- a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSessionListController.ts +++ b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSessionListController.ts @@ -236,14 +236,17 @@ export class AgentHostSessionListController extends Disposable implements IChatS }; } - private _buildMetadata(workingDirectory?: URI): { readonly [key: string]: unknown } | undefined { - if (!this._description) { + private _buildMetadata(workingDirectory: URI | undefined): { readonly [key: string]: unknown } | undefined { + if (!this._description && !workingDirectory) { return undefined; } - const result: { [key: string]: unknown } = { remoteAgentHost: this._description }; + const result: { [key: string]: unknown } = {}; + if (this._description) { + result.remoteAgentHost = this._description; + } if (workingDirectory) { result.workingDirectoryPath = workingDirectory.fsPath; } - return result; + return Object.keys(result).length > 0 ? result : undefined; } } diff --git a/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostChatContribution.test.ts b/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostChatContribution.test.ts index d5f5bd33f2c2d..9eeb1dff0ecc7 100644 --- a/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostChatContribution.test.ts +++ b/src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostChatContribution.test.ts @@ -2128,6 +2128,28 @@ suite('AgentHostChatContribution', () => { assert.strictEqual(controller.items[0].description, undefined); }); + test('list controller surfaces only working directory in metadata (git state is now per-session state, not summary)', async () => { + const { instantiationService, agentHostService } = createTestServices(disposables); + + const controller = disposables.add(instantiationService.createInstance( + AgentHostSessionListController, 'agent-host-copilot', 'copilot', agentHostService, undefined, 'local')); + + const workingDirectory = URI.file('/repo/work'); + agentHostService.addSession({ + session: AgentSession.uri('copilot', 'sess-git'), + startTime: 1000, + modifiedTime: 2000, + summary: 'With git', + workingDirectory, + }); + await controller.refresh(CancellationToken.None); + + assert.strictEqual(controller.items.length, 1); + assert.deepStrictEqual(controller.items[0].metadata, { + workingDirectoryPath: workingDirectory.fsPath, + }); + }); + test('handler works with any IAgentConnection, not just IAgentHostService', () => runWithFakedTimers({ useFakeTimers: true }, async () => { const { instantiationService, agentHostService, chatAgentService } = createTestServices(disposables);