From 459fbb8aa5f4fd78dfe5b87702bb5e0002049fec Mon Sep 17 00:00:00 2001 From: Vijay Upadya <41652029+vijayupadya@users.noreply.github.com> Date: Thu, 1 Oct 2026 14:04:07 -0700 Subject: [PATCH 1/2] agent host: send turns before the turn-start checkpoint is captured (experiment) Behind the experiment-controlled setting chat.agentHost.experimental.deferTurnStartCheckpoint (default off), a turn is sent to a provider that sets IAgentChats.supportsTurnStartBarrier while its turn-start checkpoint is still being captured. The pending capture is passed as IAgentChatContext.turnStartBarrier, and Copilot holds every tool in its onPreToolUse hook until it settles, so tools still cannot modify the working tree before the checkpoint describes it. Providers without the flag keep waiting before dispatch. Checkpoint readers (getBaselineCheckpoint, getTurnCheckpointPair) wait for a session's in-flight turn-start captures, since a session's first capture also creates its baseline. A failed deferred capture is logged and the turn continues. The experiment trigger fires in both arms once a turn with a checkpoint reaches a provider that supports the barrier. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/vs/platform/agentHost/OTEL.md | 2 +- src/vs/platform/agentHost/common/agent.ts | 15 +++ .../agentHost/common/agentHostSchema.ts | 15 +++ .../agentHostStarter.config.contribution.ts | 11 ++ .../platform/agentHost/common/agentService.ts | 3 + .../node/agentHostCheckpointService.ts | 47 +++++-- .../agentHost/node/agentSideEffects.ts | 60 +++++---- .../agentHost/node/copilot/copilotAgent.ts | 3 +- .../node/copilot/copilotAgentSession.ts | 11 +- .../node/agentHostCheckpointService.test.ts | 18 +++ .../test/node/agentSideEffects.test.ts | 119 +++++++++++++++++- .../test/node/copilotAgentSession.test.ts | 22 ++++ 12 files changed, 293 insertions(+), 33 deletions(-) diff --git a/src/vs/platform/agentHost/OTEL.md b/src/vs/platform/agentHost/OTEL.md index af8e5c71056d5c..dc38e7ab8dea74 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 when the turn was sent before its capture completed (`chat.agentHost.experimental.deferTurnStartCheckpoint`), in which case the provider holds the turn's tools 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..b27bd820e480f7 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); + } } }); } @@ -2034,7 +2036,7 @@ export class AgentSideEffects extends Disposable { // 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 => { this._logService.warn(`[AgentSideEffects] Turn preparation failed for ${chat}; sending will prepare again`, err); @@ -2048,10 +2050,24 @@ export class AgentSideEffects extends Disposable { const resolvedAttachments = await this._resolveChatAttachments(message.attachments); this._turnTracker.markSendStage(turnChannel, turnId, 'contributions'); const contribution = await this._chatContributions.outgoingTurn({ session: sessionChannel, chat, message, turnId, workingDirectories: resolvedWorkingDirectories }); + // A provider that holds the turn's tools behind a barrier can be sent the + // turn while the turn-start checkpoint is still being captured. Its tools + // cannot modify the working tree before the capture completes, so the + // checkpoint still describes the tree the agent starts from. + let turnStartBarrier: Promise | undefined; + if (checkpointCapture && agent.chats.supportsTurnStartBarrier) { + this._reportExperimentTrigger(AgentHostDeferTurnStartCheckpointSettingId); + if (this._agentConfigService.getRootValue(platformRootSchema, AgentHostDeferTurnStartCheckpointConfigKey) === true) { + turnStartBarrier = checkpointCapture.catch(err => { + this._logService.warn(`[AgentSideEffects] Turn-start checkpoint failed for ${chat}; the turn continues without it`, err); + }); + } + } const sendContext = { ...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 +2080,7 @@ export class AgentSideEffects extends Disposable { this._turnTracker.markSendStage(turnChannel, turnId, 'providerPreparation'); await providerPreparation; } - if (checkpointCapture) { + if (checkpointCapture && !turnStartBarrier) { // 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 +2131,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 +2158,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..fc44080b3adce3 100644 --- a/src/vs/platform/agentHost/node/copilot/copilotAgent.ts +++ b/src/vs/platform/agentHost/node/copilot/copilotAgent.ts @@ -3757,6 +3757,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); @@ -4878,7 +4879,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); diff --git a/src/vs/platform/agentHost/node/copilot/copilotAgentSession.ts b/src/vs/platform/agentHost/node/copilot/copilotAgentSession.ts index 4b79bebfde3dbb..24f66aede05394 100644 --- a/src/vs/platform/agentHost/node/copilot/copilotAgentSession.ts +++ b/src/vs/platform/agentHost/node/copilot/copilotAgentSession.ts @@ -1044,6 +1044,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. */ @@ -3415,12 +3417,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 +5977,12 @@ 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. + const turnStartBarrier = this._turnStartBarrier; + if (turnStartBarrier) { + await turnStartBarrier; + } 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/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..0b554e97bb8cb3 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,121 @@ 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: false, 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('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..b9a4ddbdb09f11 100644 --- a/src/vs/platform/agentHost/test/node/copilotAgentSession.test.ts +++ b/src/vs/platform/agentHost/test/node/copilotAgentSession.test.ts @@ -16274,6 +16274,28 @@ 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('denies GitHub fallback tools during Agent Merge turns', async () => { const capturedRuntime: { current?: ICopilotSessionRuntime } = {}; const { session } = await createAgentSession(disposables, { captureRuntime: capturedRuntime }); From 87a965ed9469dd742f726e810f925354c9278f5a Mon Sep 17 00:00:00 2001 From: Vijay Upadya <41652029+vijayupadya@users.noreply.github.com> Date: Thu, 1 Oct 2026 16:34:39 -0700 Subject: [PATCH 2/2] agent host: hold plugin hook commands behind the turn-start barrier Plugin hook commands (SessionStart, UserPromptSubmit, ErrorOccurred, ...) can modify the working tree, so they now wait for the turn-start checkpoint like tools do. The barrier is also handed to prepareTurn and tracked per chat, so hooks that run while a session starts (before send installs the turn's barrier) wait too. Clarify when sendStageCheckpointMs is absent. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/vs/platform/agentHost/OTEL.md | 2 +- .../agentHost/node/agentSideEffects.ts | 43 +++++++++++-------- .../agentHost/node/copilot/copilotAgent.ts | 23 ++++++++++ .../node/copilot/copilotAgentSession.ts | 25 +++++++++-- .../node/copilot/copilotPluginConverters.ts | 14 ++++++ .../node/copilot/copilotSessionLauncher.ts | 3 ++ .../test/node/agentSideEffects.test.ts | 32 +++++++++++++- .../test/node/copilotAgentSession.test.ts | 17 ++++++++ .../test/node/copilotPluginConverters.test.ts | 33 ++++++++++++++ .../test/node/copilotSessionLauncher.test.ts | 1 + 10 files changed, 168 insertions(+), 25 deletions(-) diff --git a/src/vs/platform/agentHost/OTEL.md b/src/vs/platform/agentHost/OTEL.md index dc38e7ab8dea74..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; absent when the turn was sent before its capture completed (`chat.agentHost.experimental.deferTurnStartCheckpoint`), in which case the provider holds the turn's tools until the capture completes | +| 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/node/agentSideEffects.ts b/src/vs/platform/agentHost/node/agentSideEffects.ts index b27bd820e480f7..e0c658509e04cb 100644 --- a/src/vs/platform/agentHost/node/agentSideEffects.ts +++ b/src/vs/platform/agentHost/node/agentSideEffects.ts @@ -2028,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._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); }); } @@ -2050,19 +2068,6 @@ export class AgentSideEffects extends Disposable { const resolvedAttachments = await this._resolveChatAttachments(message.attachments); this._turnTracker.markSendStage(turnChannel, turnId, 'contributions'); const contribution = await this._chatContributions.outgoingTurn({ session: sessionChannel, chat, message, turnId, workingDirectories: resolvedWorkingDirectories }); - // A provider that holds the turn's tools behind a barrier can be sent the - // turn while the turn-start checkpoint is still being captured. Its tools - // cannot modify the working tree before the capture completes, so the - // checkpoint still describes the tree the agent starts from. - let turnStartBarrier: Promise | undefined; - if (checkpointCapture && agent.chats.supportsTurnStartBarrier) { - this._reportExperimentTrigger(AgentHostDeferTurnStartCheckpointSettingId); - if (this._agentConfigService.getRootValue(platformRootSchema, AgentHostDeferTurnStartCheckpointConfigKey) === true) { - turnStartBarrier = checkpointCapture.catch(err => { - this._logService.warn(`[AgentSideEffects] Turn-start checkpoint failed for ${chat}; the turn continues without it`, err); - }); - } - } const sendContext = { ...clientOperationContext, ...(turnTelemetryContext ? { turnTelemetryContext } : {}), @@ -2080,7 +2085,7 @@ export class AgentSideEffects extends Disposable { this._turnTracker.markSendStage(turnChannel, turnId, 'providerPreparation'); await providerPreparation; } - if (checkpointCapture && !turnStartBarrier) { + 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'); diff --git a/src/vs/platform/agentHost/node/copilot/copilotAgent.ts b/src/vs/platform/agentHost/node/copilot/copilotAgent.ts index fc44080b3adce3..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. @@ -4839,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'); @@ -5572,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); @@ -6114,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 24f66aede05394..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 @@ -1310,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; @@ -1445,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; @@ -3094,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; @@ -5979,10 +5999,7 @@ export class CopilotAgentSession extends Disposable { try { // Any tool can modify the working tree, so none may run before the // turn-start checkpoint that describes it has been captured. - const turnStartBarrier = this._turnStartBarrier; - if (turnStartBarrier) { - await turnStartBarrier; - } + 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/agentSideEffects.test.ts b/src/vs/platform/agentHost/test/node/agentSideEffects.test.ts index 0b554e97bb8cb3..cdc9f8f8e1dd35 100644 --- a/src/vs/platform/agentHost/test/node/agentSideEffects.test.ts +++ b/src/vs/platform/agentHost/test/node/agentSideEffects.test.ts @@ -900,7 +900,7 @@ suite('AgentSideEffects', () => { const trigger = [`config.${AgentHostDeferTurnStartCheckpointSettingId}`]; assert.deepStrictEqual(results, { enabled: { sentBeforeCapture: true, barrier: true, triggers: trigger }, - disabled: { sentBeforeCapture: false, barrier: false, triggers: trigger }, + disabled: { sentBeforeCapture: false, barrier: true, triggers: trigger }, unsupported: { sentBeforeCapture: false, barrier: false, triggers: [] }, }); }); @@ -921,6 +921,36 @@ suite('AgentSideEffects', () => { 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 () => { diff --git a/src/vs/platform/agentHost/test/node/copilotAgentSession.test.ts b/src/vs/platform/agentHost/test/node/copilotAgentSession.test.ts index b9a4ddbdb09f11..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, @@ -16296,6 +16298,21 @@ Use the attached image as context. 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 () => { },