diff --git a/src/vs/platform/agentHost/OTEL.md b/src/vs/platform/agentHost/OTEL.md index af8e5c71056d5c..a343fafb4edf20 100644 --- a/src/vs/platform/agentHost/OTEL.md +++ b/src/vs/platform/agentHost/OTEL.md @@ -113,7 +113,7 @@ the deadline remain missing; the marker is not proof that a turn completed. | Host `timeToProviderDispatch` | Turn start to provider dispatch; absent if not observed | | Host `timeToFirstProgress`, `timeToFirstSubstantiveProgress` | Turn start to existing first visible/substantive progress boundaries; absent if not observed | | Host `sendStageWorkingDirectoryMs`, `sendStageModelSelectionMs`, `sendStageAttachmentsMs`, `sendStageContributionsMs` | Elapsed time in each existing pre-send stage that ran; an interrupted open stage retains its partial duration | -| Host `sendStageCheckpointMs` | **Residual critical-path wait** for the checkpoint after overlap with earlier preparation, not the entire checkpoint operation | +| Host `sendStageCheckpointMs` | **Residual critical-path wait** for the checkpoint after overlap with earlier preparation, not the entire checkpoint operation; absent whenever `chat.agentHost.experimental.deferTurnStartCheckpoint` is enabled and the provider supports a turn-start barrier — dispatch then never waits for the capture, even when it already completed — and the provider holds the turn's tools and hook commands until the capture completes | | Host `sendStageProviderPreparationMs` | **Residual critical-path wait** for provider turn preparation (`IAgentChats.prepareTurn`, enabled by `chat.agentHost.experimental.overlapProviderPreparation`) after it overlapped the earlier pre-send stages and the checkpoint capture; absent when preparation did not run. Provider stage marks made during preparation precede dispatch and are not reported as `providerStage*Ms` | | Host `providerStageQueueMs`, `providerStageClientMs`, `providerStageSnapshotMs`, `providerStageConfigMs`, `providerStageCreateMs`, `providerStageFinalizeMs`, `providerStagePersistMs`, `providerStageRefreshMs`, `providerStageTurnPrepareMs`, `providerStageModelResponseMs` | Sequential provider-marked stages between provider dispatch and first progress (chat queue wait, SDK client acquisition, customization snapshot, session config, SDK create/resume, post-create setup, session registration/persistence, live-session refresh, per-turn preparation, and SDK send until first progress). Only stages the provider ran are present; a turn ending before first progress retains its partial open stage. Copilot marks all of them; other providers currently mark none | | Host `hostRootTurnOrdinal`, `hostProcessAgeMs`, `titleGenerationStrategy` | Existing root ordinal and process age captured at turn start, and effective `activeAgent`, `utility`, or `deferred` strategy when observed | diff --git a/src/vs/platform/agentHost/common/agent.ts b/src/vs/platform/agentHost/common/agent.ts index ee300fc9d59cc6..d294415a2f640a 100644 --- a/src/vs/platform/agentHost/common/agent.ts +++ b/src/vs/platform/agentHost/common/agent.ts @@ -525,6 +525,14 @@ export interface IAgentChatContext { readonly hostInstructions?: readonly string[]; /** Records provider stage timing for the turn being sent; supplied only for a send. */ readonly sendStageRecorder?: IAgentProviderSendStageRecorder; + /** + * Settles once the turn-start checkpoint has been captured; supplied only + * for a send to a provider that sets {@link IAgentChats.supportsTurnStartBarrier} + * when the host dispatches without waiting for that capture. The provider + * must await it before running any tool of the turn, because tools can + * modify the working tree the checkpoint describes. + */ + readonly turnStartBarrier?: Promise; /** Whether the current turn is an automated Agent Merge repair turn. */ readonly agentMergeTurn?: boolean; } @@ -829,6 +837,13 @@ export interface IAgentPrepareChatResult { * the provider needs the owning session or storage scope. */ export interface IAgentChats { + /** + * Whether the provider honors {@link IAgentChatContext.turnStartBarrier} by + * running no tool of a turn before it settles. Only then may the host send + * a turn before its turn-start checkpoint has been captured. + */ + readonly supportsTurnStartBarrier?: boolean; + /** * Optional pre-send preparation, such as materializing a deferred provider * session, after the turn's model/agent selection and working directories diff --git a/src/vs/platform/agentHost/common/agentHostSchema.ts b/src/vs/platform/agentHost/common/agentHostSchema.ts index fc4bca0cc42aa8..a4e37ff7ed359a 100644 --- a/src/vs/platform/agentHost/common/agentHostSchema.ts +++ b/src/vs/platform/agentHost/common/agentHostSchema.ts @@ -578,6 +578,15 @@ export const AgentHostAutoAttachPullRequestsConfigKey = 'autoAttachPullRequests' */ export const AgentHostOverlapProviderPreparationConfigKey = 'overlapProviderPreparation'; +/** + * Root config key forwarded from the renderer's + * `chat.agentHost.experimental.deferTurnStartCheckpoint` setting. When `true`, + * a turn is sent to a provider that supports a turn-start barrier while its + * turn-start checkpoint is still being captured; the provider holds the + * turn's tools until the capture completes. + */ +export const AgentHostDeferTurnStartCheckpointConfigKey = 'deferTurnStartCheckpoint'; + // Root config key forwarded from the renderer when the `chat.agentSessions.migrateLegacyCopilotCli` // setting changes. When `true`, `listSessions` surfaces un-adopted extension-host Copilot CLI // sessions as adoptable agent-host sessions, and opening one adopts it in place. Experimental; off. @@ -943,6 +952,12 @@ export const platformRootSchema = createSchema({ description: localize('agentHost.config.overlapProviderPreparation.description', "Whether agents prepare their session for a turn while the turn-start checkpoint is captured, instead of after it."), default: false, }), + [AgentHostDeferTurnStartCheckpointConfigKey]: schemaProperty({ + type: 'boolean', + title: localize('agentHost.config.deferTurnStartCheckpoint.title', "Defer Turn-Start Checkpoint"), + description: localize('agentHost.config.deferTurnStartCheckpoint.description', "Whether turns are sent while their turn-start checkpoint is still being captured, with the agent's tools held until the capture completes."), + default: false, + }), [AgentHostMigrateLegacyCopilotCliEnabledConfigKey]: schemaProperty({ type: 'boolean', title: localize('agentHost.config.migrateLegacyCopilotCliEnabled.title', "Migrate Legacy Copilot CLI Sessions"), diff --git a/src/vs/platform/agentHost/common/agentHostStarter.config.contribution.ts b/src/vs/platform/agentHost/common/agentHostStarter.config.contribution.ts index 3329eb09ea27fb..553d3168ace63a 100644 --- a/src/vs/platform/agentHost/common/agentHostStarter.config.contribution.ts +++ b/src/vs/platform/agentHost/common/agentHostStarter.config.contribution.ts @@ -26,6 +26,7 @@ import { AgentHostCopilotMultiRootEnabledSettingId, AgentHostMarkdownPlanRichLinksEnabledSettingId, AgentHostOverlapProviderPreparationSettingId, + AgentHostDeferTurnStartCheckpointSettingId, AgentHostOTelCaptureContentSettingId, AgentHostOTelCaptureIdentitySettingId, AgentHostOTelDbSpanExporterEnabledSettingId, @@ -51,6 +52,7 @@ import { AgentHostMcpToolRoutingEnabledConfigKey, AgentHostMarkdownPlanRichLinksEnabledConfigKey, AgentHostOverlapProviderPreparationConfigKey, + AgentHostDeferTurnStartCheckpointConfigKey, AgentHostSystemProxyEnabledConfigKey, AgentHostWorkspaceSnapshotEnabledConfigKey, } from './agentHostSchema.js'; @@ -217,6 +219,15 @@ configurationRegistry.registerConfiguration({ experiment: { mode: 'auto' }, agentHost: { key: AgentHostOverlapProviderPreparationConfigKey }, }, + [AgentHostDeferTurnStartCheckpointSettingId]: { + type: 'boolean', + description: nls.localize('chat.agentHost.experimental.deferTurnStartCheckpoint', "When enabled, a turn is sent to the agent while its turn-start checkpoint is still being captured, and the agent's tools wait until the capture completes. This removes the checkpoint from the time to the first response."), + default: false, + scope: ConfigurationScope.APPLICATION, + tags: ['experimental', 'advanced'], + experiment: { mode: 'auto' }, + agentHost: { key: AgentHostDeferTurnStartCheckpointConfigKey }, + }, [AgentHostWorkspaceSnapshotEnabledSettingId]: { type: 'boolean', description: nls.localize('chat.experimental.workspaceSnapshot', "When enabled, the first turn of a new Copilot agent host chat includes a bounded file-name snapshot of its working directories, so the agent can orient itself without listing the workspace first. File contents are never included."), diff --git a/src/vs/platform/agentHost/common/agentService.ts b/src/vs/platform/agentHost/common/agentService.ts index 4006c3bc42d9d4..f0198ee8566ea0 100644 --- a/src/vs/platform/agentHost/common/agentService.ts +++ b/src/vs/platform/agentHost/common/agentService.ts @@ -138,6 +138,9 @@ export const AgentHostAutoAttachPullRequestsSettingId = 'chat.agentHost.experime /** Configuration key controlling whether providers prepare a turn alongside the turn-start checkpoint. */ export const AgentHostOverlapProviderPreparationSettingId = 'chat.agentHost.experimental.overlapProviderPreparation'; +/** Configuration key controlling whether turns are sent before their turn-start checkpoint is captured. */ +export const AgentHostDeferTurnStartCheckpointSettingId = 'chat.agentHost.experimental.deferTurnStartCheckpoint'; + /** * Configuration key gating multiple-working-directory support for the Copilot * agent-host provider. When `true`, the Copilot provider advertises the diff --git a/src/vs/platform/agentHost/node/agentHostCheckpointService.ts b/src/vs/platform/agentHost/node/agentHostCheckpointService.ts index 11daf5b90fbef1..c3a01e39059072 100644 --- a/src/vs/platform/agentHost/node/agentHostCheckpointService.ts +++ b/src/vs/platform/agentHost/node/agentHostCheckpointService.ts @@ -30,6 +30,8 @@ export class AgentHostCheckpointService extends Disposable implements IAgentHost */ private readonly _sequencer = new SequencerByKey(); private readonly _turnStartCheckpoints = new Map>(); + /** Settling markers of the turn-start captures queued per session key. */ + private readonly _pendingTurnStartCaptures = new Map>>(); constructor( @ISessionDataService private readonly _sessionDataService: ISessionDataService, @@ -75,7 +77,7 @@ export class AgentHostCheckpointService extends Disposable implements IAgentHost } // Check if the baseline ref already exists - const baselineCheckpointRef = await this.getBaselineCheckpoint(sessionUri, repositoryRootUri); + const baselineCheckpointRef = await this._readBaselineCheckpoint(sessionUri, repositoryRootUri); if (baselineCheckpointRef) { continue; } @@ -96,7 +98,31 @@ export class AgentHostCheckpointService extends Disposable implements IAgentHost } captureTurnStartCheckpoint(sessionUri: URI, chatUri: URI, turnId: string, workingDirectories: readonly URI[] | undefined): Promise { - return this._sequencer.queue(sessionUri.toString(), () => this._captureTurnStartCheckpoint(sessionUri, chatUri, turnId, workingDirectories)); + const sessionKey = sessionUri.toString(); + const capture = this._sequencer.queue(sessionKey, () => this._captureTurnStartCheckpoint(sessionUri, chatUri, turnId, workingDirectories)); + // Readers wait for in-flight captures: a turn can be sent before its capture + // completes, and the first capture of a session also creates its baseline. + let pending = this._pendingTurnStartCaptures.get(sessionKey); + if (!pending) { + pending = new Set(); + this._pendingTurnStartCaptures.set(sessionKey, pending); + } + const settled = capture.then(undefined, () => undefined); + pending.add(settled); + void settled.then(() => { + pending.delete(settled); + if (pending.size === 0 && this._pendingTurnStartCaptures.get(sessionKey) === pending) { + this._pendingTurnStartCaptures.delete(sessionKey); + } + }); + return capture; + } + + private async _whenTurnStartCapturesSettled(sessionUri: URI): Promise { + const pending = this._pendingTurnStartCaptures.get(sessionUri.toString()); + if (pending?.size) { + await Promise.all(pending); + } } private async _captureTurnStartCheckpoint(sessionUri: URI, chatUri: URI, turnId: string, workingDirectories: readonly URI[] | undefined): Promise { @@ -143,7 +169,7 @@ export class AgentHostCheckpointService extends Disposable implements IAgentHost // The baseline lookup is independent of the capture, so overlap it. const [tree, hasBaseline] = await Promise.all([ this._gitService.captureWorkingTreeAsTree(repositoryRootUri), - this.getBaselineCheckpoint(sessionUri, repositoryRootUri), + this._readBaselineCheckpoint(sessionUri, repositoryRootUri), ]); if (tree) { if (!hasBaseline) { @@ -233,7 +259,7 @@ export class AgentHostCheckpointService extends Disposable implements IAgentHost // Check if the baseline ref exists for this repository. If it // doesn't exist, we cannot capture a turn checkpoint for this repository. - const baselineCheckpointRef = await this.getBaselineCheckpoint(sessionUri, repositoryRootUri); + const baselineCheckpointRef = await this._readBaselineCheckpoint(sessionUri, repositoryRootUri); if (!baselineCheckpointRef) { continue; } @@ -306,6 +332,7 @@ export class AgentHostCheckpointService extends Disposable implements IAgentHost turnId: string, workingDirectory?: URI ): Promise<{ parent: string; current: string } | undefined> { + await this._whenTurnStartCapturesSettled(sessionUri); if (!workingDirectory) { const workingDirectories = this._agentConfigService.getEffectiveWorkingDirectories(sessionUri.toString()); if (!workingDirectories || workingDirectories.length === 0) { @@ -319,7 +346,7 @@ export class AgentHostCheckpointService extends Disposable implements IAgentHost const [currentCheckpointRef, previousCheckpointRef, baselineCheckpointRef] = await Promise.all([ ref.object.getTurnCheckpointRef(turnId), ref.object.getPreviousCheckpointRef(turnId), - this.getBaselineCheckpoint(sessionUri, workingDirectory) + this._readBaselineCheckpoint(sessionUri, workingDirectory) ]); if (!currentCheckpointRef || !baselineCheckpointRef) { return undefined; @@ -344,6 +371,12 @@ export class AgentHostCheckpointService extends Disposable implements IAgentHost } async getBaselineCheckpoint(sessionUri: URI, workingDirectory?: URI): Promise { + await this._whenTurnStartCapturesSettled(sessionUri); + return this._readBaselineCheckpoint(sessionUri, workingDirectory); + } + + /** Reads the baseline ref without waiting for pending captures; safe inside sequenced captures. */ + private async _readBaselineCheckpoint(sessionUri: URI, workingDirectory?: URI): Promise { if (!workingDirectory) { const workingDirectories = this._agentConfigService.getEffectiveWorkingDirectories(sessionUri.toString()); if (!workingDirectories || workingDirectories.length === 0) { @@ -437,7 +470,7 @@ export class AgentHostCheckpointService extends Disposable implements IAgentHost continue; } - const baselineCheckpointRef = await this.getBaselineCheckpoint(sessionUri, repositoryRootUri); + const baselineCheckpointRef = await this._readBaselineCheckpoint(sessionUri, repositoryRootUri); if (!baselineCheckpointRef) { continue; } @@ -478,7 +511,7 @@ export class AgentHostCheckpointService extends Disposable implements IAgentHost } private async _ensureBaselineCheckpoint(sessionUri: URI, repositoryRootUri: URI, tree: string): Promise { - if (await this.getBaselineCheckpoint(sessionUri, repositoryRootUri)) { + if (await this._readBaselineCheckpoint(sessionUri, repositoryRootUri)) { return; } diff --git a/src/vs/platform/agentHost/node/agentSideEffects.ts b/src/vs/platform/agentHost/node/agentSideEffects.ts index 206ce0230bf687..e0c658509e04cb 100644 --- a/src/vs/platform/agentHost/node/agentSideEffects.ts +++ b/src/vs/platform/agentHost/node/agentSideEffects.ts @@ -31,8 +31,8 @@ import { readAgentPermissionResponseMeta } from '../common/meta/agentPermissionR import { ITelemetryService } from '../../telemetry/common/telemetry.js'; import { logSettingExperimentTrigger } from '../../telemetry/common/experimentTrigger.js'; -import { AgentHostOverlapProviderPreparationConfigKey, platformRootSchema } from '../common/agentHostSchema.js'; -import { AgentHostOverlapProviderPreparationSettingId } from '../common/agentService.js'; +import { AgentHostDeferTurnStartCheckpointConfigKey, AgentHostOverlapProviderPreparationConfigKey, platformRootSchema } from '../common/agentHostSchema.js'; +import { AgentHostDeferTurnStartCheckpointSettingId, AgentHostOverlapProviderPreparationSettingId } from '../common/agentService.js'; import { CopilotCliVSCodeAssignmentContextKey } from '../common/copilotCliConfig.js'; import { ISessionDataService } from '../common/sessionDataService.js'; import { SessionConfigKey } from '../common/sessionConfigKeys.js'; @@ -250,8 +250,8 @@ export class AgentSideEffects extends Disposable { private readonly _pendingSessionCustomizationPublishes = new Map>(); private readonly _pendingMcpServerStarts = new NKeyMap(); private readonly _pendingCustomizationEnablementRefreshes = new Set(); - /** Set while a turn reached the overlap experiment's divergence before the assignment context arrived. */ - private _overlapExperimentTriggerPending = false; + /** Experiment settings whose trigger a turn reached before the assignment context arrived. */ + private readonly _pendingExperimentTriggers = new Set(); /** * Buffers signals whose `parentToolCallId` references a subagent @@ -305,11 +305,13 @@ export class AgentSideEffects extends Disposable { sendTurnMessage: options => void this._sendTurnMessage(options), })); this._register(this._agentConfigService.onDidRootConfigChange(() => { - if (this._overlapExperimentTriggerPending) { + if (this._pendingExperimentTriggers.size) { // Deferred so that the listener installing the forwarded assignment context on telemetry runs first. queueMicrotask(() => { - if (this._overlapExperimentTriggerPending && !this._store.isDisposed) { - this._reportOverlapExperimentTrigger(); + if (!this._store.isDisposed) { + for (const settingId of [...this._pendingExperimentTriggers]) { + this._reportExperimentTrigger(settingId); + } } }); } @@ -2026,17 +2028,35 @@ export class AgentSideEffects extends Disposable { await Promise.all(selectionUpdates); + // A provider that holds everything that can modify the working tree — + // tools and hook commands — behind a barrier can be sent the turn while + // the turn-start checkpoint is still being captured: the checkpoint + // still describes the tree the agent starts from. + let deferCheckpoint = false; + let turnStartBarrier: Promise | undefined; + if (checkpointCapture && agent.chats.supportsTurnStartBarrier) { + this._reportExperimentTrigger(AgentHostDeferTurnStartCheckpointSettingId); + deferCheckpoint = this._agentConfigService.getRootValue(platformRootSchema, AgentHostDeferTurnStartCheckpointConfigKey) === true; + turnStartBarrier = checkpointCapture.then(() => { }, err => { + if (deferCheckpoint) { + this._logService.warn(`[AgentSideEffects] Turn-start checkpoint failed for ${chat}; the turn continues without it`, err); + } + }); + } + // A provider can prepare the turn — e.g. materialize a deferred session // with the selection applied above — while attachments, contributions - // and the checkpoint capture run. Dispatch still waits for both, so the - // checkpoint keeps describing the tree the agent starts from. A failed - // preparation is only logged: `sendMessage` then prepares as usual and - // surfaces any error exactly as it would without the overlap. + // and the checkpoint capture run. Dispatch still waits for preparation, + // and the barrier keeps the session's hook commands from modifying the + // tree before the checkpoint is captured. A failed preparation is only + // logged: `sendMessage` then prepares as usual and surfaces any error + // exactly as it would without the overlap. let providerPreparation: Promise | undefined; if (agent.chats.prepareTurn) { - this._reportOverlapExperimentTrigger(); + this._reportExperimentTrigger(AgentHostOverlapProviderPreparationSettingId); if (this._agentConfigService.getRootValue(platformRootSchema, AgentHostOverlapProviderPreparationConfigKey) === true) { - providerPreparation = agent.chats.prepareTurn(chatUri, turnId, resolvedWorkingDirectories, clientOperationContext).catch(err => { + const prepareContext = turnStartBarrier ? { ...clientOperationContext, turnStartBarrier } : clientOperationContext; + providerPreparation = agent.chats.prepareTurn(chatUri, turnId, resolvedWorkingDirectories, prepareContext).catch(err => { this._logService.warn(`[AgentSideEffects] Turn preparation failed for ${chat}; sending will prepare again`, err); }); } @@ -2052,6 +2072,7 @@ export class AgentSideEffects extends Disposable { ...clientOperationContext, ...(turnTelemetryContext ? { turnTelemetryContext } : {}), ...(contribution.instructions?.length ? { hostInstructions: contribution.instructions } : {}), + ...(turnStartBarrier ? { turnStartBarrier } : {}), sendStageRecorder: this._turnTracker.createProviderStageRecorder(turnChannel, turnId), }; if (this._cancelledTurnIds.get(turnChannel)?.has(turnId)) { @@ -2064,7 +2085,7 @@ export class AgentSideEffects extends Disposable { this._turnTracker.markSendStage(turnChannel, turnId, 'providerPreparation'); await providerPreparation; } - if (checkpointCapture) { + if (checkpointCapture && !deferCheckpoint) { // Measures only what the checkpoint still costs the critical path // after overlapping the work above, not the capture's total cost. this._turnTracker.markSendStage(turnChannel, turnId, 'checkpoint'); @@ -2115,6 +2136,20 @@ export class AgentSideEffects extends Disposable { } } + /** + * Reports where an experiment's treatment and control diverge. Agent host telemetry + * only carries the assignment context that ExP attributes the event by once the + * workbench has forwarded it, so until then the trigger stays pending. + */ + private _reportExperimentTrigger(settingId: string): void { + if (typeof this._agentConfigService.getRootConfigValues?.()[CopilotCliVSCodeAssignmentContextKey] !== 'string') { + this._pendingExperimentTriggers.add(settingId); + return; + } + this._pendingExperimentTriggers.delete(settingId); + logSettingExperimentTrigger(this._telemetryService, settingId); + } + /** * Discards a turn-start checkpoint that was started concurrently with the * rest of the send path, for a turn that will never reach the provider. @@ -2128,18 +2163,6 @@ export class AgentSideEffects extends Disposable { * caller. It is the only cleanup on the failure path, where no such * cancellation discard exists. */ - /** - * Reports where overlapped and sequential provider preparation diverge. Agent host - * telemetry only carries the assignment context that ExP attributes the event by once - * the workbench has forwarded it, so until then the trigger stays pending. - */ - private _reportOverlapExperimentTrigger(): void { - this._overlapExperimentTriggerPending = typeof this._agentConfigService.getRootConfigValues?.()[CopilotCliVSCodeAssignmentContextKey] !== 'string'; - if (!this._overlapExperimentTriggerPending) { - logSettingExperimentTrigger(this._telemetryService, AgentHostOverlapProviderPreparationSettingId); - } - } - private async _discardPendingTurnStartCheckpoint(capture: Promise | undefined, sessionChannel: ProtocolURI, chatUri: URI, turnId: string): Promise { if (!capture) { return; diff --git a/src/vs/platform/agentHost/node/copilot/copilotAgent.ts b/src/vs/platform/agentHost/node/copilot/copilotAgent.ts index c462e6b08cd9c3..e2643b5da168f4 100644 --- a/src/vs/platform/agentHost/node/copilot/copilotAgent.ts +++ b/src/vs/platform/agentHost/node/copilot/copilotAgent.ts @@ -985,6 +985,11 @@ export class CopilotAgent extends Disposable implements IAgent { private readonly _activeClients = new ResourceMap(); /** Live session -> the turn whose `prepareTurn` launched it; dropped with the session. */ private readonly _preparedTurnLaunches = new WeakMap(); + /** + * Chat URI -> turn-start barrier of the turn being prepared or sent, until it + * settles. Hook commands run while that chat's session starts wait for it. + */ + private readonly _pendingTurnStartBarriers = new Map>(); /** * Last host-published customization snapshot per configuration scope (AGENTS.md section 8b). * Updated only from host call boundaries; absence is distinct from an empty list. @@ -3757,6 +3762,7 @@ export class CopilotAgent extends Disposable implements IAgent { * Chat-addressed surface for the chats within a session. */ readonly chats: IAgentChats = { + supportsTurnStartBarrier: true, prepareTurn: (chat: URI, turnId: string, workingDirectories: readonly URI[] | undefined, context: URI | IAgentChatContext): Promise => this._prepareTurn(chat, turnId, workingDirectories, context), createChat: (chat: URI, context: URI | IAgentChatContext, options?: IAgentCreateChatOptions): Promise => { this._noteHostCustomizations(context); @@ -4838,6 +4844,7 @@ export class CopilotAgent extends Disposable implements IAgent { } private async _sendMessageOnce(chat: URI, prompt: string, attachments?: readonly MessageAttachment[], turnId?: string, senderClientId?: string, clientType = AgentHostClientType.Unknown, workingDirectories?: readonly URI[], operationContext?: URI | IAgentChatContext, clientTelemetryContext?: IAgentHostClientTelemetryContext): Promise { + this._trackTurnStartBarrier(chat, operationContext); const context = this._resolveSendChatContext(chat, operationContext); const stageRecorder = operationContext && !URI.isUri(operationContext) ? operationContext.sendStageRecorder : undefined; stageRecorder?.mark('queue'); @@ -4878,7 +4885,7 @@ export class CopilotAgent extends Disposable implements IAgent { const sdkMode = this._resolveSdkMode(current.configurationResource); enterUnboundedPhase(); stageRecorder?.mark('turnPrepare'); - await entry.send(prompt, attachments, turnId, sdkMode, senderClientId, clientType, resolveAgentHostInstructions(operationContext), clientTelemetryContext, !!operationContext && !URI.isUri(operationContext) && operationContext.agentMergeTurn === true, stageRecorder); + await entry.send(prompt, attachments, turnId, sdkMode, senderClientId, clientType, resolveAgentHostInstructions(operationContext), clientTelemetryContext, !!operationContext && !URI.isUri(operationContext) && operationContext.agentMergeTurn === true, stageRecorder, operationContext && !URI.isUri(operationContext) ? operationContext.turnStartBarrier : undefined); } catch (err) { const errCode = (err as { code?: number })?.code; const errMsg = err instanceof Error ? err.message : String(err); @@ -5571,6 +5578,7 @@ export class CopilotAgent extends Disposable implements IAgent { * syncs and workspace scans the launch just completed. */ private async _prepareTurn(chat: URI, turnId: string, workingDirectories: readonly URI[] | undefined, operationContext: URI | IAgentChatContext): Promise { + this._trackTurnStartBarrier(chat, operationContext); const initial = this._resolveSendChatContext(chat, operationContext); await this._queueChat(initial.configurationId, initial.sequencerKey, 'prepareTurn', async () => { const current = this._resolveSendChatContext(chat, operationContext); @@ -6113,11 +6121,27 @@ export class CopilotAgent extends Disposable implements IAgent { serverToolHost: this._serverToolHost, onTurnEnded: () => this._onChatTurnEnded(), telemetryContext: () => this.getTelemetryContext(), + pendingTurnStartBarrier: () => this._pendingTurnStartBarriers.get(chatChannelUri.toString()), }, ); return agentSession; } + private _trackTurnStartBarrier(chat: URI, operationContext: URI | IAgentChatContext | undefined): void { + const barrier = operationContext && !URI.isUri(operationContext) ? operationContext.turnStartBarrier : undefined; + if (!barrier) { + return; + } + const key = chat.toString(); + this._pendingTurnStartBarriers.set(key, barrier); + const release = () => { + if (this._pendingTurnStartBarriers.get(key) === barrier) { + this._pendingTurnStartBarriers.delete(key); + } + }; + barrier.then(release, release); + } + /** Resolves root-configured MCP servers that must be disabled when the SDK session starts. */ private async _disabledRootMcpServers(session: URI, sessionId: string, snapshot: IActiveClientSnapshot): Promise { await this._customizationEnablementService.initializeSession(session.toString()); diff --git a/src/vs/platform/agentHost/node/copilot/copilotAgentSession.ts b/src/vs/platform/agentHost/node/copilot/copilotAgentSession.ts index 4b79bebfde3dbb..9e1726438defa3 100644 --- a/src/vs/platform/agentHost/node/copilot/copilotAgentSession.ts +++ b/src/vs/platform/agentHost/node/copilot/copilotAgentSession.ts @@ -535,6 +535,13 @@ export interface ICopilotAgentSessionOptions { * could dispose this session off the current stack. */ readonly onTurnEnded?: () => void; + /** + * Returns the turn-start barrier of a turn this chat is being prepared or + * sent for, if any. Lets hook commands that run while the session starts + * (before {@link CopilotAgentSession.send} sets the turn's barrier) wait + * for the turn-start checkpoint too. + */ + readonly pendingTurnStartBarrier?: () => Promise | undefined; /** * Platform used to compute the SDK sandbox policy. Defaults to @@ -1044,6 +1051,8 @@ export class CopilotAgentSession extends Disposable { private readonly _developmentErrorInjectionEnabled: boolean; private _dropLateRootTurnEvents = false; private _agentMergeTurn = false; + /** Settles once the current turn's turn-start checkpoint is captured; tools wait for it. */ + private _turnStartBarrier: Promise | undefined; /** MCP servers whose tools Agent Merge turns deny because they expose GitHub. */ private readonly _agentMergeRestrictedMcpServerNames: ReadonlySet; /** Monotonic 0-based ordinal assigned to each turn as it starts, for numeric `turnIndex` telemetry parity. */ @@ -1308,6 +1317,7 @@ export class CopilotAgentSession extends Disposable { private _detectInterruptedTurnOnRestore: boolean; /** Notifies the agent that this chat's turn ended. See {@link ICopilotAgentSessionOptions.onTurnEnded}. */ private readonly _onTurnEnded: () => void; + private readonly _pendingTurnStartBarrier: () => Promise | undefined; private readonly _shellManager: ShellManager | undefined; /** Streams runtime-executed shell output into output-only (non-pty) terminal channels. */ private readonly _nonPtyShellTerminals: NonPtyShellTerminalStreams; @@ -1443,6 +1453,7 @@ export class CopilotAgentSession extends Disposable { this._sandboxDiagnostics = this._register(this._instantiationService.createInstance(CopilotSandboxDiagnostics, this._ownerSessionUri.toString(), () => this._launchPlan.client.rpc.sandbox.getHostSupport())); this._detectInterruptedTurnOnRestore = options.launchPlan.kind === 'resume'; this._onTurnEnded = options.onTurnEnded ?? (() => { }); + this._pendingTurnStartBarrier = options.pendingTurnStartBarrier ?? (() => undefined); this._shellManager = options.shellManager; this._nonPtyShellTerminals = this._register(this._instantiationService.createInstance(NonPtyShellTerminalStreams, options.sessionUri, this._storageUri, options.chatChannelUri)); this._workingDirectory = options.workingDirectory; @@ -3092,9 +3103,20 @@ export class CopilotAgentSession extends Disposable { handlePreToolUse: input => this._handlePreToolUse(input), handlePostToolUse: input => this._handlePostToolUse(input), handleUserPromptSubmitted: () => this.handleUserPromptSubmitted(), + waitForTurnStartBarrier: () => this._waitForTurnStartBarrier(), }; } + /** + * Settles once the turn-start checkpoint of the turn being prepared or run + * is captured. Anything that can modify the working tree — tools and hook + * commands — waits for it so the checkpoint describes the tree the turn + * starts from. + */ + private async _waitForTurnStartBarrier(): Promise { + await (this._turnStartBarrier ?? this._pendingTurnStartBarrier()); + } + /** Resolves only matching, currently pending SDK authentication callbacks. */ async resolveMcpAuthentication(params: AuthenticateParams): Promise { let resolved = false; @@ -3415,12 +3437,13 @@ export class CopilotAgentSession extends Disposable { } } - async send(prompt: string, attachments?: readonly MessageAttachment[], turnId?: string, mode?: CopilotSdkMode, senderClientId?: string, clientType = AgentHostClientType.Unknown, hostInstructions?: readonly string[], clientContext = createUnknownAgentHostClientTelemetryContext(clientType), agentMergeTurn = false, stageRecorder?: IAgentProviderSendStageRecorder): Promise { + async send(prompt: string, attachments?: readonly MessageAttachment[], turnId?: string, mode?: CopilotSdkMode, senderClientId?: string, clientType = AgentHostClientType.Unknown, hostInstructions?: readonly string[], clientContext = createUnknownAgentHostClientTelemetryContext(clientType), agentMergeTurn = false, stageRecorder?: IAgentProviderSendStageRecorder, turnStartBarrier?: Promise): Promise { if (this._workingDirectoryMutationInProgress) { throw new Error('Cannot start a turn while the working directory is changing'); } this._resetAbortToken(); this._agentMergeTurn = agentMergeTurn; + this._turnStartBarrier = turnStartBarrier; if (turnId && this._currentTurn.value?.id !== turnId) { // Establish the `pending` turn for this message. Callers normally // call `resetTurnState` just before `send()`; this covers the @@ -5974,6 +5997,9 @@ export class CopilotAgentSession extends Disposable { private async _handlePreToolUse(input: PreToolUseHookInput): Promise { try { + // Any tool can modify the working tree, so none may run before the + // turn-start checkpoint that describes it has been captured. + await this._waitForTurnStartBarrier(); const restriction = this._agentMergeTurn ? getAgentMergeGitHubToolRestriction(input.toolName, input.toolArgs) ?? (isCopilotMcpToolName(input.toolName, this._agentMergeRestrictedMcpServerNames) ? AGENT_MERGE_GITHUB_TOOL_RESTRICTION : undefined) diff --git a/src/vs/platform/agentHost/node/copilot/copilotPluginConverters.ts b/src/vs/platform/agentHost/node/copilot/copilotPluginConverters.ts index f37cc0c47d1f05..6de073c7faf75f 100644 --- a/src/vs/platform/agentHost/node/copilot/copilotPluginConverters.ts +++ b/src/vs/platform/agentHost/node/copilot/copilotPluginConverters.ts @@ -384,8 +384,16 @@ export function toSdkHooks( readonly onPreToolUse: (input: PreToolUseHookInput) => Promise; readonly onPostToolUse: (input: PostToolUseHookInput) => Promise; readonly onUserPromptSubmitted?: () => { readonly additionalContext: string } | undefined; + /** Awaited before any plugin hook command runs, since a command can modify the working tree. */ + readonly beforeHookCommands?: () => Promise; }, ): SessionHooks { + const beforeCommands = async (commands: readonly IParsedHookCommand[] | undefined) => { + if (commands?.length) { + await editTrackingHooks?.beforeHookCommands?.(); + } + }; + // Group all commands by SDK handler key const commandsByKey = new Map(); for (const group of hookGroups) { @@ -408,6 +416,7 @@ export function toSdkHooks( if (internalResult !== undefined) { return internalResult; } + await beforeCommands(preToolCommands); return runHookCommands(preToolCommands, input); }; } @@ -417,6 +426,7 @@ export function toSdkHooks( if (postToolCommands?.length || editTrackingHooks) { hooks.onPostToolUse = async (input: PostToolUseHookInput) => { await editTrackingHooks?.onPostToolUse(input); + await beforeCommands(postToolCommands); return runHookCommands(postToolCommands, input); }; } @@ -425,6 +435,7 @@ export function toSdkHooks( const promptCommands = commandsByKey.get('onUserPromptSubmitted'); if (promptCommands?.length || editTrackingHooks?.onUserPromptSubmitted) { hooks.onUserPromptSubmitted = async (input: UserPromptSubmittedHookInput) => { + await beforeCommands(promptCommands); const stdin = JSON.stringify(input); for (const cmd of promptCommands ?? []) { try { @@ -441,6 +452,7 @@ export function toSdkHooks( const startCommands = commandsByKey.get('onSessionStart'); if (startCommands?.length) { hooks.onSessionStart = async (input: SessionStartHookInput) => { + await beforeCommands(startCommands); const stdin = JSON.stringify(input); for (const cmd of startCommands) { try { @@ -456,6 +468,7 @@ export function toSdkHooks( const endCommands = commandsByKey.get('onSessionEnd'); if (endCommands?.length) { hooks.onSessionEnd = async (input: SessionEndHookInput) => { + await beforeCommands(endCommands); const stdin = JSON.stringify(input); for (const cmd of endCommands) { try { @@ -471,6 +484,7 @@ export function toSdkHooks( const errorCommands = commandsByKey.get('onErrorOccurred'); if (errorCommands?.length) { hooks.onErrorOccurred = async (input: ErrorOccurredHookInput) => { + await beforeCommands(errorCommands); const stdin = JSON.stringify(input); for (const cmd of errorCommands) { try { diff --git a/src/vs/platform/agentHost/node/copilot/copilotSessionLauncher.ts b/src/vs/platform/agentHost/node/copilot/copilotSessionLauncher.ts index e64c62a0990144..1fcc02647e5000 100644 --- a/src/vs/platform/agentHost/node/copilot/copilotSessionLauncher.ts +++ b/src/vs/platform/agentHost/node/copilot/copilotSessionLauncher.ts @@ -221,6 +221,8 @@ export interface ICopilotSessionRuntime { handlePreToolUse(input: PreToolUseHookInput): Promise; handlePostToolUse(input: PostToolUseHookInput): Promise; handleUserPromptSubmitted(): { readonly additionalContext: string } | undefined; + /** Settles once the turn-start checkpoint is captured, so hook commands may modify the working tree. */ + waitForTurnStartBarrier(): Promise; // eslint-disable-next-line @typescript-eslint/no-explicit-any createClientSdkTools(toolSearchActive: boolean): Tool[]; // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -1127,6 +1129,7 @@ export class CopilotSessionLauncher implements ICopilotSessionLauncher { onPreToolUse: input => runtime.handlePreToolUse(input), onPostToolUse: input => runtime.handlePostToolUse(input), onUserPromptSubmitted: () => runtime.handleUserPromptSubmitted(), + beforeHookCommands: () => runtime.waitForTurnStartBarrier(), }), mcpServers, onExitPlanModeRequest: (request, invocation) => runtime.handleExitPlanModeRequest(request, invocation), diff --git a/src/vs/platform/agentHost/test/node/agentHostCheckpointService.test.ts b/src/vs/platform/agentHost/test/node/agentHostCheckpointService.test.ts index bf978639d64e1c..913073188196c1 100644 --- a/src/vs/platform/agentHost/test/node/agentHostCheckpointService.test.ts +++ b/src/vs/platform/agentHost/test/node/agentHostCheckpointService.test.ts @@ -159,6 +159,24 @@ suite('AgentHostCheckpointService', () => { ]); }); + test('baseline readers wait for an in-flight first turn-start capture that creates the baseline', async () => { + const captureStarted = new DeferredPromise(); + const releaseCapture = new DeferredPromise(); + const { chat, session, service, workingDirectory } = createTestService(async () => { + captureStarted.complete(); + await releaseCapture.p; + return 'tree-before-first-turn'; + }, { baseline: false, previous: false }); + + const capture = service.captureTurnStartCheckpoint(session, chat, 'turn-1', [workingDirectory]); + await captureStarted.p; + const baseline = service.getBaselineCheckpoint(session, workingDirectory); + releaseCapture.complete(); + await capture; + + assert.strictEqual(await baseline, buildCheckpointRefName(AgentSession.id(session), 0)); + }); + test('discard waits for an in-flight turn-start capture', async () => { const captureStarted = new DeferredPromise(); const releaseCapture = new DeferredPromise(); diff --git a/src/vs/platform/agentHost/test/node/agentSideEffects.test.ts b/src/vs/platform/agentHost/test/node/agentSideEffects.test.ts index 18e68a1e35fabc..cdc9f8f8e1dd35 100644 --- a/src/vs/platform/agentHost/test/node/agentSideEffects.test.ts +++ b/src/vs/platform/agentHost/test/node/agentSideEffects.test.ts @@ -39,9 +39,9 @@ import { IProductService } from '../../../product/common/productService.js'; import { ITelemetryService, TelemetryLevel } from '../../../telemetry/common/telemetry.js'; import { NullTelemetryService } from '../../../telemetry/common/telemetryUtils.js'; import { TestExperimentTriggerTelemetryService } from '../../../telemetry/test/common/experimentTriggerTestUtils.js'; -import { AgentHostOverlapProviderPreparationSettingId } from '../../common/agentService.js'; +import { AgentHostDeferTurnStartCheckpointSettingId, AgentHostOverlapProviderPreparationSettingId } from '../../common/agentService.js'; import { CopilotCliVSCodeAssignmentContextKey } from '../../common/copilotCliConfig.js'; -import { AgentHostGlobalAutoApproveEnabledConfigKey, AgentHostMarkdownPlanRichLinksEnabledConfigKey, AgentHostOverlapProviderPreparationConfigKey, AgentHostTelemetryLevelConfigKey, AgentHostWorkspaceSnapshotEnabledConfigKey, platformSessionSchema, telemetryLevelToAgentHostConfigValue } from '../../common/agentHostSchema.js'; +import { AgentHostGlobalAutoApproveEnabledConfigKey, AgentHostMarkdownPlanRichLinksEnabledConfigKey, AgentHostDeferTurnStartCheckpointConfigKey, AgentHostOverlapProviderPreparationConfigKey, AgentHostTelemetryLevelConfigKey, AgentHostWorkspaceSnapshotEnabledConfigKey, platformSessionSchema, telemetryLevelToAgentHostConfigValue } from '../../common/agentHostSchema.js'; import { AgentConfigurationService, IAgentConfigurationService } from '../../node/agentConfigurationService.js'; import { AgentHostTelemetryService } from '../../node/agentHostTelemetryService.js'; import { AgentHostClientConnectionService, IAgentHostClientConnectionService } from '../../node/agentHostClientConnectionService.js'; @@ -808,6 +808,151 @@ suite('AgentSideEffects', () => { }); }); + suite('deferred turn-start checkpoint', () => { + + const turnStarted = { + type: ActionType.ChatTurnStarted, + turnId: 'turn-1', + startedAt: '2025-01-01T00:00:00.000Z', + message: { text: 'hello', origin: { kind: MessageKind.User } }, + } as const; + + function createDeferringSideEffects(checkpointService: IAgentHostCheckpointService, telemetry: ITelemetryService = NullTelemetryService): AgentSideEffects { + const workingDirectory = URI.file('/wd'); + setupSession(workingDirectory.toString()); + const localSideEffects = createTestSideEffects(disposables, stateManager, { + getAgent: () => agent, + agents: agentList, + sessionDataService: createNullSessionDataService(), + resolveWorkingDirectoryBeforeSend: async () => [workingDirectory], + }, undefined, telemetry, new FakeChangesetService(), undefined, checkpointService); + disposables.add(localSideEffects.registerProgressListener(agent)); + return localSideEffects; + } + + function setRootConfig(values: Record): void { + stateManager.dispatchServerAction(ROOT_STATE_URI, { type: ActionType.RootConfigChanged, config: values }); + } + + /** Records the barrier each send receives, while still delivering the send to the mock agent. */ + function captureSendBarriers(): (Promise | undefined)[] { + const barriers: (Promise | undefined)[] = []; + const sendMessage = agent.chats.sendMessage; + agent.chats.sendMessage = (chat, prompt, workingDirectories, attachments, turnId, senderClientId, clientTypeOrContext, context) => { + const sendContext = context ?? clientTypeOrContext; + barriers.push(sendContext && typeof sendContext === 'object' && !URI.isUri(sendContext) ? sendContext.turnStartBarrier : undefined); + return sendMessage(chat, prompt, workingDirectories, attachments, turnId, senderClientId, clientTypeOrContext, context); + }; + return barriers; + } + + test('sends before the capture completes and hands the provider a barrier that settles with it', async () => { + const capture = new DeferredPromise(); + const localSideEffects = createDeferringSideEffects({ + ...NULL_CHECKPOINT_SERVICE, + captureTurnStartCheckpoint: () => capture.p, + }); + setRootConfig({ [AgentHostDeferTurnStartCheckpointConfigKey]: true }); + (agent.chats as { supportsTurnStartBarrier?: boolean }).supportsTurnStartBarrier = true; + const barriers = captureSendBarriers(); + + stateManager.dispatchServerAction(defaultChatUri, turnStarted); + localSideEffects.handleAction(defaultChatUri, turnStarted); + await waitForSendMessageCalls(1); + let barrierSettled = false; + void barriers[0]?.then(() => { barrierSettled = true; }); + await timeout(0); + const beforeCapture = barrierSettled; + capture.complete(); + await timeout(0); + + assert.deepStrictEqual({ hasBarrier: barriers[0] !== undefined, beforeCapture, afterCapture: barrierSettled }, { + hasBarrier: true, + beforeCapture: false, + afterCapture: true, + }); + }); + + test('defers only for providers that hold tools behind the barrier, and reports the trigger in both arms', async () => { + const results: Record = {}; + for (const [name, enabled, supportsBarrier] of [['enabled', true, true], ['disabled', false, true], ['unsupported', true, false]] as const) { + const capture = new DeferredPromise(); + const telemetry = new TestExperimentTriggerTelemetryService(); + const localSideEffects = createDeferringSideEffects({ + ...NULL_CHECKPOINT_SERVICE, + captureTurnStartCheckpoint: () => capture.p, + }, telemetry); + setRootConfig({ [AgentHostDeferTurnStartCheckpointConfigKey]: enabled, [CopilotCliVSCodeAssignmentContextKey]: 'assignment-context' }); + (agent.chats as { supportsTurnStartBarrier?: boolean }).supportsTurnStartBarrier = supportsBarrier; + const barriers = captureSendBarriers(); + const sendsBefore = agent.sendMessageCalls.length; + + stateManager.dispatchServerAction(defaultChatUri, turnStarted); + localSideEffects.handleAction(defaultChatUri, turnStarted); + await timeout(0); + const sentBeforeCapture = agent.sendMessageCalls.length > sendsBefore; + capture.complete(); + await waitForSendMessageCalls(sendsBefore + 1); + results[name] = { sentBeforeCapture, barrier: barriers[0] !== undefined, triggers: telemetry.triggers }; + stateManager.removeSession(sessionUri.toString()); + } + + const trigger = [`config.${AgentHostDeferTurnStartCheckpointSettingId}`]; + assert.deepStrictEqual(results, { + enabled: { sentBeforeCapture: true, barrier: true, triggers: trigger }, + disabled: { sentBeforeCapture: false, barrier: true, triggers: trigger }, + unsupported: { sentBeforeCapture: false, barrier: false, triggers: [] }, + }); + }); + + test('a failed deferred capture still settles the barrier and does not fail the turn', async () => { + const localSideEffects = createDeferringSideEffects({ + ...NULL_CHECKPOINT_SERVICE, + captureTurnStartCheckpoint: async () => { throw new Error('capture failed'); }, + }); + setRootConfig({ [AgentHostDeferTurnStartCheckpointConfigKey]: true }); + (agent.chats as { supportsTurnStartBarrier?: boolean }).supportsTurnStartBarrier = true; + const barriers = captureSendBarriers(); + + stateManager.dispatchServerAction(defaultChatUri, turnStarted); + localSideEffects.handleAction(defaultChatUri, turnStarted); + await waitForSendMessageCalls(1); + await barriers[0]; + + assert.notStrictEqual(stateManager.getChatState(defaultChatUri)?.turns.at(-1)?.state, TurnState.Error); + }); + + test('hands turn preparation the barrier so hook commands run while the session starts wait for the capture', async () => { + const capture = new DeferredPromise(); + const localSideEffects = createDeferringSideEffects({ + ...NULL_CHECKPOINT_SERVICE, + captureTurnStartCheckpoint: () => capture.p, + }); + setRootConfig({ [AgentHostOverlapProviderPreparationConfigKey]: true }); + (agent.chats as { supportsTurnStartBarrier?: boolean }).supportsTurnStartBarrier = true; + let prepareBarrier: Promise | undefined; + agent.chats.prepareTurn = async (_chat, _turnId, _workingDirectories, context) => { + prepareBarrier = URI.isUri(context) ? undefined : context.turnStartBarrier; + }; + + stateManager.dispatchServerAction(defaultChatUri, turnStarted); + localSideEffects.handleAction(defaultChatUri, turnStarted); + await timeout(0); + let barrierSettled = false; + void prepareBarrier?.then(() => { barrierSettled = true; }); + await timeout(0); + const beforeCapture = barrierSettled; + capture.complete(); + await waitForSendMessageCalls(1); + + assert.deepStrictEqual({ hasBarrier: prepareBarrier !== undefined, beforeCapture, afterCapture: barrierSettled }, { + hasBarrier: true, + beforeCapture: false, + afterCapture: true, + }); + }); + }); + test('preserves the original turn-start checkpoint identity across resume and completion', async () => { const workingDirectory = URI.file('/wd'); setupSession(workingDirectory.toString()); diff --git a/src/vs/platform/agentHost/test/node/copilotAgentSession.test.ts b/src/vs/platform/agentHost/test/node/copilotAgentSession.test.ts index 456bc3183f8cd3..ef547bebaa4d6e 100644 --- a/src/vs/platform/agentHost/test/node/copilotAgentSession.test.ts +++ b/src/vs/platform/agentHost/test/node/copilotAgentSession.test.ts @@ -1072,6 +1072,7 @@ async function createAgentSession(disposables: DisposableStore, options?: { restrictedTelemetryContextError?: Error; telemetryContext?: IAgentTelemetryContext; onTurnEnded?: () => void; + pendingTurnStartBarrier?: () => Promise | undefined; modelId?: string; enableDevelopmentErrorInjection?: boolean; resume?: boolean; @@ -1392,6 +1393,7 @@ async function createAgentSession(disposables: DisposableStore, options?: { serverToolHost: options?.serverToolHost, platform: options?.platform ?? 'linux', onTurnEnded: options?.onTurnEnded, + pendingTurnStartBarrier: options?.pendingTurnStartBarrier, enableDevelopmentErrorInjection: options?.enableDevelopmentErrorInjection ?? true, realpath: options?.realpath, controlPlaneRpcTimeoutMs: options?.controlPlaneRpcTimeoutMs, @@ -16274,6 +16276,43 @@ Use the attached image as context. assert.strictEqual(entry.args[0], '[Copilot:test-session-1] Failed in onPreToolUse: tool=edit'); }); + test('holds every tool until the turn-start barrier settles', async () => { + const capturedRuntime: { current?: ICopilotSessionRuntime } = {}; + const { session } = await createAgentSession(disposables, { captureRuntime: capturedRuntime }); + const barrier = new DeferredPromise(); + (session as unknown as { _turnStartBarrier: Promise | undefined })._turnStartBarrier = barrier.p; + + let settled = false; + const hook = capturedRuntime.current!.handlePreToolUse({ + sessionId: 'test-session-1', + timestamp: new Date(0), + workingDirectory: '/tmp', + toolName: 'bash', + toolArgs: { command: 'echo hi' }, + }).then(() => { settled = true; }); + await timeout(0); + const beforeBarrier = settled; + barrier.complete(); + await hook; + + assert.deepStrictEqual({ beforeBarrier, afterBarrier: settled }, { beforeBarrier: false, afterBarrier: true }); + }); + + test('hook commands wait for the pending turn-start barrier before the turn is sent', async () => { + const capturedRuntime: { current?: ICopilotSessionRuntime } = {}; + const barrier = new DeferredPromise(); + await createAgentSession(disposables, { captureRuntime: capturedRuntime, pendingTurnStartBarrier: () => barrier.p }); + + let settled = false; + const wait = capturedRuntime.current!.waitForTurnStartBarrier().then(() => { settled = true; }); + await timeout(0); + const beforeBarrier = settled; + barrier.complete(); + await wait; + + assert.deepStrictEqual({ beforeBarrier, afterBarrier: settled }, { beforeBarrier: false, afterBarrier: true }); + }); + test('denies GitHub fallback tools during Agent Merge turns', async () => { const capturedRuntime: { current?: ICopilotSessionRuntime } = {}; const { session } = await createAgentSession(disposables, { captureRuntime: capturedRuntime }); diff --git a/src/vs/platform/agentHost/test/node/copilotPluginConverters.test.ts b/src/vs/platform/agentHost/test/node/copilotPluginConverters.test.ts index bb960c1871a9f3..a1d10be3ab2ad0 100644 --- a/src/vs/platform/agentHost/test/node/copilotPluginConverters.test.ts +++ b/src/vs/platform/agentHost/test/node/copilotPluginConverters.test.ts @@ -7,6 +7,7 @@ import assert from 'assert'; import { writeFileSync, unlinkSync } from 'fs'; import { fileURLToPath } from 'url'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../base/test/common/utils.js'; +import { DeferredPromise, timeout } from '../../../../base/common/async.js'; import { DisposableStore } from '../../../../base/common/lifecycle.js'; import { Schemas } from '../../../../base/common/network.js'; import { URI } from '../../../../base/common/uri.js'; @@ -682,6 +683,38 @@ suite('copilotPluginConverters', () => { assert.strictEqual(input.prompt, 'Keep GitHub casing'); assert.deepStrictEqual(result, { additionalContext: 'Rename with exact casing' }); }); + + test('plugin hook commands wait for beforeHookCommands, internal-only hooks do not', async () => { + const { command, cleanup } = echoJsonCmd({}); + try { + const barrier = new DeferredPromise(); + const waits: string[] = []; + const tracking = (name: string) => ({ + onPreToolUse: async () => { }, + onPostToolUse: async () => { }, + onUserPromptSubmitted: () => undefined, + beforeHookCommands: () => { waits.push(name); return barrier.p; }, + }); + const withCommands = toSdkHooks([makeHookGroup('SessionStart', command), makeHookGroup('UserPromptSubmit', command)], tracking('commands')); + const internalOnly = toSdkHooks([], tracking('internal')); + const context = { timestamp: new Date(0), workingDirectory: '/', sessionId: 'test' }; + + let settled = 0; + const pending = [ + Promise.resolve(withCommands.onSessionStart!({ ...context, source: 'new' }, { sessionId: 'test' })).then(() => { settled++; }), + Promise.resolve(withCommands.onUserPromptSubmitted!({ ...context, prompt: 'hi' }, { sessionId: 'test' })).then(() => { settled++; }), + ]; + await internalOnly.onUserPromptSubmitted!({ ...context, prompt: 'hi' }, { sessionId: 'test' }); + await timeout(0); + const beforeRelease = settled; + barrier.complete(); + await Promise.all(pending); + + assert.deepStrictEqual({ waits, beforeRelease, afterRelease: settled }, { waits: ['commands', 'commands'], beforeRelease: 0, afterRelease: 2 }); + } finally { + cleanup(); + } + }); }); // ---- parsedPluginsEqual --------------------------------------------- diff --git a/src/vs/platform/agentHost/test/node/copilotSessionLauncher.test.ts b/src/vs/platform/agentHost/test/node/copilotSessionLauncher.test.ts index 7fbcf68ff49756..ab96f8b440b309 100644 --- a/src/vs/platform/agentHost/test/node/copilotSessionLauncher.test.ts +++ b/src/vs/platform/agentHost/test/node/copilotSessionLauncher.test.ts @@ -57,6 +57,7 @@ const testRuntime: ICopilotSessionRuntime = { handlePreToolUse: async () => { }, handlePostToolUse: async () => { }, handleUserPromptSubmitted: () => undefined, + waitForTurnStartBarrier: async () => { }, createClientSdkTools: () => [], createServerSdkTools: () => [], reloadExtensions: async () => { },