From 156357c8335f00c26e22456cc1bb838357d1cf3f Mon Sep 17 00:00:00 2001 From: Alexandru Dima Date: Mon, 17 Aug 2026 12:37:34 +0200 Subject: [PATCH 1/6] agentHost: observe capabilities lazily in session adapters (#330853) Every cached AgentHostSessionAdapter eagerly subscribed to the shared agent-capabilities observable, so a window restoring hundreds of sessions installed hundreds of observers and tripped the listener leak detector. Most of those observers had nothing to do: the autorun only re-applies a chat catalog, and an adapter that never received one has no catalog to reconcile. Install the observer on the first applyChatCatalog call instead, so only adapters with catalog state to reapply subscribe. Late-hydrating capabilities still re-expand a collapsed peer catalog. Found while self-hosting Insiders. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../agentHost/AGENT_HOST_SESSIONS_PROVIDER.md | 2 +- .../browser/baseAgentHostSessionsProvider.ts | 28 +++++---- .../localAgentHostSessionsProvider.test.ts | 58 ++++++++++++++++++- 3 files changed, 70 insertions(+), 18 deletions(-) diff --git a/src/vs/sessions/contrib/providers/agentHost/AGENT_HOST_SESSIONS_PROVIDER.md b/src/vs/sessions/contrib/providers/agentHost/AGENT_HOST_SESSIONS_PROVIDER.md index 4ad91ac70a785b..9967c4e6ef3b45 100644 --- a/src/vs/sessions/contrib/providers/agentHost/AGENT_HOST_SESSIONS_PROVIDER.md +++ b/src/vs/sessions/contrib/providers/agentHost/AGENT_HOST_SESSIONS_PROVIDER.md @@ -114,7 +114,7 @@ A single agent host session uses several distinct identifiers: ## Architecture -- **`AgentHostSessionAdapter`** (`baseAgentHostSessionsProvider.ts`) is the `ISession` implementation. It wraps an `IAgentSessionMetadata` from the backend and exposes the observable session surface (`status`, `title`, `workspace`, `mainChat`, `mode`, …). The base provider keeps a `_sessionCache` of adapters keyed by `rawId`. Adapter capabilities derive from a shared provider-to-capabilities lookup, so one root-state event listener and one catalog scan serve the entire cache; root-state errors and disconnects clear the lookup so stale capabilities are not retained. +- **`AgentHostSessionAdapter`** (`baseAgentHostSessionsProvider.ts`) is the `ISession` implementation. It wraps an `IAgentSessionMetadata` from the backend and exposes the observable session surface (`status`, `title`, `workspace`, `mainChat`, `mode`, …). The base provider keeps a `_sessionCache` of adapters keyed by `rawId`. Adapter capabilities derive from a shared provider-to-capabilities lookup, so one root-state event listener and one catalog scan serve the entire cache; an adapter observes that lookup only after receiving a chat catalog that may need reapplication. Root-state errors and disconnects clear the lookup so stale capabilities are not retained. - **`NewSession`** is a disposable draft (pre-creation) session. Several can be in flight simultaneously; the management layer tears down superseded drafts via `deleteNewSession`. A draft eagerly creates its backend session once authentication settles, then **graduates** into a committed `AgentHostSessionAdapter` on first send. - The base provider is abstract; concrete providers supply: `connection`, `authenticationPending`, `resourceSchemeForProvider`, `_formatSessionTypeLabel`, `_adapterOptions` (workspace builder), `resolveWorkspace`, and optionally `_diffUriMapper`. diff --git a/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts b/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts index 9318d6e2748df2..fcc9f5dff085a9 100644 --- a/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts +++ b/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts @@ -655,9 +655,10 @@ export class AgentHostSessionAdapter extends Disposable implements ISession { /** * The last {@link SessionState} applied to the chat catalog, retained so the * catalog can be re-reconciled when {@link capabilities} change after the - * fact (see the capability autorun in the constructor). + * fact. */ private _lastCatalogState: SessionState | undefined; + private readonly _chatCatalogCapabilitiesObserver = this._register(new MutableDisposable()); private readonly _rawId: string; private readonly _resourceScheme: string; @@ -915,19 +916,6 @@ export class AgentHostSessionAdapter extends Disposable implements ISession { supportsDelete: true, }; }); - - // Re-apply the chat catalog when advertised capabilities change (e.g. the - // agent host's root state arrives after the session's first state update). - // Without this, a multi-chat session whose state was processed while - // `supportsMultipleChats` was still `false` would stay collapsed to - // `[defaultChat]` until the next session-state update. - this._register(autorun(reader => { - this.capabilities.read(reader); - const state = this._lastCatalogState; - if (state) { - this._applyChatCatalog(state); - } - })); } /** @@ -948,7 +936,17 @@ export class AgentHostSessionAdapter extends Disposable implements ISession { */ applyChatCatalog(state: SessionState): void { this._lastCatalogState = state; - this._applyChatCatalog(state); + if (this._chatCatalogCapabilitiesObserver.value) { + this._applyChatCatalog(state); + } else { + this._chatCatalogCapabilitiesObserver.value = autorun(reader => { + this.capabilities.read(reader); + const currentState = this._lastCatalogState; + if (currentState) { + this._applyChatCatalog(currentState); + } + }); + } } private _applyChatCatalog(state: SessionState): void { diff --git a/src/vs/sessions/contrib/providers/agentHost/test/browser/localAgentHostSessionsProvider.test.ts b/src/vs/sessions/contrib/providers/agentHost/test/browser/localAgentHostSessionsProvider.test.ts index 87d17d5cfeb24c..5071b7560f9767 100644 --- a/src/vs/sessions/contrib/providers/agentHost/test/browser/localAgentHostSessionsProvider.test.ts +++ b/src/vs/sessions/contrib/providers/agentHost/test/browser/localAgentHostSessionsProvider.test.ts @@ -9,7 +9,7 @@ import { DeferredPromise, raceTimeout, timeout } from '../../../../../../base/co import { Codicon } from '../../../../../../base/common/codicons.js'; import { Emitter, Event } from '../../../../../../base/common/event.js'; import { DisposableMap, DisposableStore, ImmortalReference, toDisposable, type IReference } from '../../../../../../base/common/lifecycle.js'; -import { autorun, constObservable, ISettableObservable, observableValue, type IObservable } from '../../../../../../base/common/observable.js'; +import { autorun, constObservable, ISettableObservable, observableFromEvent, observableValue, type IObservable } from '../../../../../../base/common/observable.js'; import { URI } from '../../../../../../base/common/uri.js'; import { isEqual } from '../../../../../../base/common/resources.js'; import { mock } from '../../../../../../base/test/common/mock.js'; @@ -45,7 +45,7 @@ import { IActiveSession } from '../../../../../services/sessions/common/sessions import { ISessionsService } from '../../../../../services/sessions/browser/sessionsService.js'; import { IAgentCustomizationScope, IAgentHostActiveClientService } from '../../../../../../workbench/contrib/chat/browser/agentSessions/agentHost/agentHostActiveClientService.js'; import { LocalAgentHostSessionsProvider } from '../../browser/localAgentHostSessionsProvider.js'; -import { AgentHostSessionAdapter } from '../../browser/baseAgentHostSessionsProvider.js'; +import { AgentHostSessionAdapter, type IAgentHostAdapterOptions } from '../../browser/baseAgentHostSessionsProvider.js'; import { IAutomationStorageService } from '../../../../automations/common/automationStorageService.js'; import { TestAutomationStorageService } from '../../../../automations/test/browser/automationTestUtils.js'; import { ILabelService } from '../../../../../../platform/label/common/label.js'; @@ -4216,6 +4216,60 @@ suite('LocalAgentHostSessionsProvider', () => { }); }); + test('session adapters observe capabilities only after receiving a chat catalog', () => { + let listenerCount = 0; + let agentCapabilities = new Map([['copilotcli', {}]]); + const capabilitiesChanged = disposables.add(new Emitter({ + onDidAddListener: () => listenerCount++, + onWillRemoveListener: () => listenerCount--, + })); + const capabilitiesObs = observableFromEvent(disposables, capabilitiesChanged.event, () => agentCapabilities); + const instantiationService = disposables.add(new TestInstantiationService()); + instantiationService.stub(IGitHubService, new class extends mock() { }); + instantiationService.stub(ISessionsService, new class extends mock() { + override readonly activeSession = constObservable(undefined); + }); + instantiationService.stub(IPullRequestIconCache, new class extends mock() { }); + const options: IAgentHostAdapterOptions = { + icon: Codicon.copilot, + loading: constObservable(false), + buildWorkspace: () => undefined, + instantiationService, + getConnection: () => undefined, + agentCapabilities: capabilitiesObs, + }; + const adapters = Array.from({ length: 200 }, (_, index) => disposables.add(instantiationService.createInstance( + AgentHostSessionAdapter, + createSession(`lazy-capabilities-${index}`), + 'local-agent-host', + 'agent-host-copilotcli', + 'copilotcli', + options, + ))); + const sessionUri = AgentSession.uri('copilotcli', 'lazy-capabilities-0').toString(); + const defaultChat = buildDefaultChatUri(sessionUri); + const peerChat = buildChatUri(sessionUri, 'peer-1'); + + const listenerCountBeforeCatalog = listenerCount; + adapters[0].applyChatCatalog(makeState([ + makeChatSummary(defaultChat, ''), + makeChatSummary(peerChat, 'Peer'), + ], { defaultChat })); + const listenerCountAfterCatalog = listenerCount; + agentCapabilities = new Map([['copilotcli', { multipleChats: { fork: true } }]]); + capabilitiesChanged.fire(); + + assert.deepStrictEqual({ + listenerCountBeforeCatalog, + listenerCountAfterCatalog, + chatFragmentsAfterHydration: adapters[0].chats.get().map(chat => chat.resource.fragment), + }, { + listenerCountBeforeCatalog: 0, + listenerCountAfterCatalog: 1, + chatFragmentsAfterHydration: ['', 'peer-1'], + }); + }); + test('forkChat forwards the source chat and turn to the host and surfaces a new peer chat', () => runWithFakedTimers({ useFakeTimers: true }, async () => { const provider = createProvider(disposables, agentHost); const session = setupMultiChatSession(provider, 'multi-fork'); From ea32f8070d88542fe5873b213c33f0d9456fee15 Mon Sep 17 00:00:00 2001 From: Benjamin Christopher Simmonds <44439583+benibenj@users.noreply.github.com> Date: Mon, 17 Aug 2026 13:56:26 +0200 Subject: [PATCH 2/6] agentHost: address agent merge review feedback (#331158) * agentHost: address agent merge review feedback Follow-ups to PR #331010 review comments: - Rename settings from `chat.agentHost.agentMerge.*` to `chat.agentMerge.*`. - Reject client writes to host-owned `agentMerge.controller` session config so a forged controller state cannot drive a native merge. - Reconcile injected autonomy configuration every cycle and roll back keys that policy later revokes; never widen configuration while a turn is active. - Revalidate the merge target before starting a turn and before merging, and refresh live state, config and top-level comments inside the merge step. - Split `AgentMergeRepairAction` out of `AgentMergeAction` so repair paths cannot express a merge. - Carry all feedback comments per review thread and bound the prompt with explicit caps and an aggregate character budget. - Fail closed when fork head provenance is missing, and ignore refs from a different GitHub host. - Cancel the controller turn when the runtime stops. - Split the enable/disable command preconditions with a dedicated context key and make the configure quick pick's reset a title button. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * agentHost: address agent merge PR review feedback - Preserve host-written session config across a client `SessionConfigChanged` with `replace: true`. Omitting `agentMerge.controller` previously cleared the bound target, comment watermark and attempt budgets, which bypassed the authorization boundary that explicit-write rejection was meant to enforce. - Canonicalize pull request web hosts to their API host when checking that the credential matches. GitHub Enterprise Cloud serves `tenant.ghe.com` from `api.tenant.ghe.com`, so comparing the web host rejected every GHE Cloud pull request. The derivation now reuses `deriveGitHubEndpoints`. - Refresh top-level comments inside `prepareMerge`, last, before the snapshot and token are captured. Refreshing them in the controller beforehand left a window across `prepareMerge`'s own authoritative refreshes in which a new maintainer comment could be missed; refreshing afterwards would invalidate the preparation generation. A comment landing after capture now invalidates the preparation. - Migrate the legacy `chat.agentHost.agentMerge.*` setting ids to their new `chat.agentMerge.*` names so an explicit opt-out is not silently discarded. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../agentHostStarter.config.contribution.ts | 22 +-- .../platform/agentHost/common/agentMerge.ts | 97 +++++++--- .../agentHost/node/agentMergeController.ts | 183 ++++++++++++++---- .../platform/agentHost/node/agentService.ts | 70 ++++++- .../agentHost/test/common/agentMerge.test.ts | 90 ++++++--- .../test/node/agentMergeController.test.ts | 87 ++++++++- .../agentHost/test/node/agentService.test.ts | 72 +++++++ .../common/pullRequestMutationService.ts | 7 +- .../agentHost/AGENT_HOST_SESSIONS_PROVIDER.md | 10 +- .../agentHost/browser/agentMergeActions.ts | 107 ++++++++-- .../chat/browser/chat.shared.contribution.ts | 19 ++ 11 files changed, 628 insertions(+), 136 deletions(-) diff --git a/src/vs/platform/agentHost/common/agentHostStarter.config.contribution.ts b/src/vs/platform/agentHost/common/agentHostStarter.config.contribution.ts index 7b6176788d4400..ad435cffd08b2a 100644 --- a/src/vs/platform/agentHost/common/agentHostStarter.config.contribution.ts +++ b/src/vs/platform/agentHost/common/agentHostStarter.config.contribution.ts @@ -102,7 +102,7 @@ configurationRegistry.registerConfiguration({ properties: { [AgentMergeSettingId.Enabled]: { type: 'boolean', - description: nls.localize('chat.agentHost.agentMerge.enabled', "Enables the experimental Agent Merge controller and its commands. Agent Merge can monitor an agent session's pull request, ask the agent to address selected blockers, and optionally merge the pull request when it is ready."), + description: nls.localize('chat.agentMerge.enabled', "Enables the experimental Agent Merge controller and its commands. Agent Merge can monitor an agent session's pull request, ask the agent to address selected blockers, and optionally merge the pull request when it is ready."), default: product.quality !== 'stable', scope: ConfigurationScope.APPLICATION, tags: ['experimental'], @@ -110,7 +110,7 @@ configurationRegistry.registerConfiguration({ }, [AgentMergeSettingId.AddressReviews]: { type: 'boolean', - description: nls.localize('chat.agentHost.agentMerge.addressReviews', "Controls whether enabled Agent Merge sessions address unresolved review threads, changes-requested reviews, and new pull request comments from repository maintainers or the Copilot pull request reviewer."), + description: nls.localize('chat.agentMerge.addressReviews', "Controls whether enabled Agent Merge sessions address unresolved review threads, changes-requested reviews, and new pull request comments from repository maintainers or the Copilot pull request reviewer."), default: true, scope: ConfigurationScope.APPLICATION, tags: ['experimental'], @@ -118,7 +118,7 @@ configurationRegistry.registerConfiguration({ }, [AgentMergeSettingId.FixCI]: { type: 'boolean', - description: nls.localize('chat.agentHost.agentMerge.fixCI', "Controls whether enabled Agent Merge sessions ask the agent to fix failed required CI checks."), + description: nls.localize('chat.agentMerge.fixCI', "Controls whether enabled Agent Merge sessions ask the agent to fix failed required CI checks."), default: true, scope: ConfigurationScope.APPLICATION, tags: ['experimental'], @@ -126,7 +126,7 @@ configurationRegistry.registerConfiguration({ }, [AgentMergeSettingId.ResolveConflicts]: { type: 'boolean', - description: nls.localize('chat.agentHost.agentMerge.resolveConflicts', "Controls whether enabled Agent Merge sessions ask the agent to update branches that are behind or resolve merge conflicts."), + description: nls.localize('chat.agentMerge.resolveConflicts', "Controls whether enabled Agent Merge sessions ask the agent to update branches that are behind or resolve merge conflicts."), default: true, scope: ConfigurationScope.APPLICATION, tags: ['experimental'], @@ -134,7 +134,7 @@ configurationRegistry.registerConfiguration({ }, [AgentMergeSettingId.MergePullRequest]: { type: 'boolean', - description: nls.localize('chat.agentHost.agentMerge.mergePullRequest', "Controls whether the Agent Host automatically merges or enqueues pull requests for enabled Agent Merge sessions after all selected maintenance work is complete."), + description: nls.localize('chat.agentMerge.mergePullRequest', "Controls whether the Agent Host automatically merges or enqueues pull requests for enabled Agent Merge sessions after all selected maintenance work is complete."), default: false, scope: ConfigurationScope.APPLICATION, tags: ['experimental'], @@ -144,12 +144,12 @@ configurationRegistry.registerConfiguration({ type: 'string', enum: ['auto', 'squash', 'merge', 'rebase'], enumDescriptions: [ - nls.localize('chat.agentHost.agentMerge.mergeMethod.auto', "Uses the first repository-compatible method in this order: squash, merge commit, rebase."), - nls.localize('chat.agentHost.agentMerge.mergeMethod.squash', "Uses squash merge when the repository permits it."), - nls.localize('chat.agentHost.agentMerge.mergeMethod.merge', "Uses a merge commit when the repository permits it."), - nls.localize('chat.agentHost.agentMerge.mergeMethod.rebase', "Uses rebase merge when the repository permits it."), + nls.localize('chat.agentMerge.mergeMethod.auto', "Uses the first repository-compatible method in this order: squash, merge commit, rebase."), + nls.localize('chat.agentMerge.mergeMethod.squash', "Uses squash merge when the repository permits it."), + nls.localize('chat.agentMerge.mergeMethod.merge', "Uses a merge commit when the repository permits it."), + nls.localize('chat.agentMerge.mergeMethod.rebase', "Uses rebase merge when the repository permits it."), ], - description: nls.localize('chat.agentHost.agentMerge.mergeMethod', "Controls the native merge method used by Agent Merge."), + description: nls.localize('chat.agentMerge.mergeMethod', "Controls the native merge method used by Agent Merge."), default: 'auto', scope: ConfigurationScope.APPLICATION, tags: ['experimental'], @@ -157,7 +157,7 @@ configurationRegistry.registerConfiguration({ }, [AgentMergeSettingId.ReplyAttribution]: { type: 'boolean', - description: nls.localize('chat.agentHost.agentMerge.replyAttribution', "Controls whether review-thread replies posted by Agent Merge include an automated-reply attribution note."), + description: nls.localize('chat.agentMerge.replyAttribution', "Controls whether review-thread replies posted by Agent Merge include an automated-reply attribution note."), default: true, scope: ConfigurationScope.APPLICATION, tags: ['experimental'], diff --git a/src/vs/platform/agentHost/common/agentMerge.ts b/src/vs/platform/agentHost/common/agentMerge.ts index c385885b4e37a7..171eb71d0b6cee 100644 --- a/src/vs/platform/agentHost/common/agentMerge.ts +++ b/src/vs/platform/agentHost/common/agentMerge.ts @@ -19,16 +19,23 @@ export const AgentMergeConfigKey = { } as const; export const AgentMergeSettingId = { - Enabled: 'chat.agentHost.agentMerge.enabled', - AddressReviews: 'chat.agentHost.agentMerge.addressReviews', - FixCI: 'chat.agentHost.agentMerge.fixCI', - ResolveConflicts: 'chat.agentHost.agentMerge.resolveConflicts', - MergePullRequest: 'chat.agentHost.agentMerge.mergePullRequest', - MergeMethod: 'chat.agentHost.agentMerge.mergeMethod', - ReplyAttribution: 'chat.agentHost.agentMerge.replyAttribution', + Enabled: 'chat.agentMerge.enabled', + AddressReviews: 'chat.agentMerge.addressReviews', + FixCI: 'chat.agentMerge.fixCI', + ResolveConflicts: 'chat.agentMerge.resolveConflicts', + MergePullRequest: 'chat.agentMerge.mergePullRequest', + MergeMethod: 'chat.agentMerge.mergeMethod', + ReplyAttribution: 'chat.agentMerge.replyAttribution', } as const; -export type AgentMergeAction = 'addressReviews' | 'fixCI' | 'resolveConflicts' | 'mergePullRequest'; +/** + * Work the agent itself can be asked to perform. Merging is deliberately absent: + * it is executed by the host, never delegated to a model. + */ +export type AgentMergeRepairAction = 'addressReviews' | 'fixCI' | 'resolveConflicts'; + +/** A user-authorizable Agent Merge action, including the host-executed merge. */ +export type AgentMergeAction = AgentMergeRepairAction | 'mergePullRequest'; export type AgentMergeMethod = 'auto' | 'squash' | 'merge' | 'rebase'; export interface AgentMergeActions { @@ -61,6 +68,11 @@ export interface AgentMergeReviewThreadContext { readonly id: string; readonly path?: string; readonly line?: number; + /** Authorized comments in the thread, oldest first, so later follow-ups are visible. */ + readonly comments: readonly AgentMergeFeedbackComment[]; +} + +export interface AgentMergeFeedbackComment { readonly author?: string; readonly body: string; } @@ -146,7 +158,7 @@ export type AgentMergeGateResult = | { readonly kind: 'indeterminate'; readonly reason: string } | { readonly kind: 'terminal' } | { readonly kind: 'noWork'; readonly waitingOnChecks: boolean; readonly fingerprint: string } - | { readonly kind: 'prompt'; readonly actions: readonly AgentMergeAction[]; readonly fingerprint: string; readonly context: AgentMergePromptContext } + | { readonly kind: 'prompt'; readonly actions: readonly AgentMergeRepairAction[]; readonly fingerprint: string; readonly context: AgentMergePromptContext } | { readonly kind: 'merge'; readonly fingerprint: string }; export interface AgentMergePromptContext { @@ -156,14 +168,23 @@ export interface AgentMergePromptContext { readonly baseRef: string; readonly headRef: string; readonly reviewThreads: readonly AgentMergeReviewThreadContext[]; - readonly reviewSummaries: readonly string[]; - readonly newComments: readonly string[]; + readonly reviewSummaries: readonly AgentMergeFeedbackComment[]; + readonly newComments: readonly AgentMergeFeedbackComment[]; readonly failedChecks: readonly string[]; readonly behind: boolean; readonly conflicting: boolean; readonly commentWatermark: string; } +/** Caps that keep an autonomous prompt bounded regardless of pull request size. */ +const maximumReviewThreads = 10; +const maximumCommentsPerThread = 5; +const maximumReviewSummaries = 5; +const maximumNewComments = 5; +const maximumFailedChecks = 20; +const maximumFeedbackBodyLength = 1_000; +const maximumFeedbackBudget = 20_000; + const maintainerAssociations = new Set(['OWNER', 'MEMBER', 'COLLABORATOR']); const copilotPullRequestReviewerId = '175728472'; const copilotPullRequestReviewerLogins = new Set(['copilot', 'copilot-pull-request-reviewer[bot]']); @@ -247,6 +268,28 @@ function isRecord(value: unknown): value is Record { return typeof value === 'object' && value !== null && !Array.isArray(value); } +/** + * Truncates feedback bodies against one shared character budget so a + * comment-heavy pull request cannot grow the autonomous prompt without bound. + */ +class FeedbackBudget { + + private _remaining: number; + + constructor(budget: number) { + this._remaining = budget; + } + + take(author: string | undefined, body: string | undefined): AgentMergeFeedbackComment { + const text = (body ?? '').slice(0, Math.max(0, Math.min(maximumFeedbackBodyLength, this._remaining))); + this._remaining -= text.length; + return { + ...(author ? { author } : {}), + body: text, + }; + } +} + export function evaluateAgentMerge(snapshot: PullRequestSnapshot, configuration: AgentMergeConfiguration, commentWatermark: string): AgentMergeGateResult { const core = snapshot.core; if (core.status !== 'ready' || !core.complete || !core.value) { @@ -281,7 +324,7 @@ export function evaluateAgentMerge(snapshot: PullRequestSnapshot, configuration: const mergeability = snapshot.mergeability.value!; const behind = mergeability.mergeStateStatus?.toUpperCase() === 'BEHIND'; const conflicting = mergeability.mergeable === 'CONFLICTING'; - const actions: AgentMergeAction[] = []; + const actions: AgentMergeRepairAction[] = []; if (configuration.addressReviews && (reviewThreads.length > 0 || changesRequested.length > 0 || newComments.length > 0)) { actions.push('addressReviews'); } @@ -292,25 +335,29 @@ export function evaluateAgentMerge(snapshot: PullRequestSnapshot, configuration: actions.push('resolveConflicts'); } + const budget = new FeedbackBudget(maximumFeedbackBudget); const context: AgentMergePromptContext = { pullRequestUrl: core.value.url, title: core.value.title, headSha: core.value.headSha, baseRef: core.value.baseRef, headRef: core.value.headRef, - reviewThreads: reviewThreads.slice(0, 20).map(thread => { - const comment = thread.comments.find(candidate => isAgentMergeFeedbackAuthor(candidate.author)); - return { - id: thread.id, - ...(thread.path ? { path: thread.path } : {}), - ...(thread.line !== undefined ? { line: thread.line } : {}), - ...(comment?.author?.login ? { author: comment.author.login } : {}), - body: (comment?.body ?? '').slice(0, 1_000), - }; - }), - reviewSummaries: changesRequested.map(review => review.body ?? `Changes requested by ${review.author?.login ?? 'reviewer'}`), - newComments: newComments.map(comment => comment.body ?? `Comment by ${comment.author?.login ?? 'reviewer'}`), - failedChecks: checks.failed.map(check => check.name), + reviewThreads: reviewThreads.slice(0, maximumReviewThreads).map(thread => ({ + id: thread.id, + ...(thread.path ? { path: thread.path } : {}), + ...(thread.line !== undefined ? { line: thread.line } : {}), + comments: thread.comments + .filter(comment => isAgentMergeFeedbackAuthor(comment.author)) + .slice(-maximumCommentsPerThread) + .map(comment => budget.take(comment.author?.login, comment.body)), + })), + reviewSummaries: changesRequested + .slice(-maximumReviewSummaries) + .map(review => budget.take(review.author?.login, review.body)), + newComments: newComments + .slice(-maximumNewComments) + .map(comment => budget.take(comment.author?.login, comment.body)), + failedChecks: checks.failed.slice(0, maximumFailedChecks).map(check => check.name), behind, conflicting, commentWatermark: newComments.reduce((latest, comment) => comment.createdAt && comment.createdAt > latest ? comment.createdAt : latest, commentWatermark), diff --git a/src/vs/platform/agentHost/node/agentMergeController.ts b/src/vs/platform/agentHost/node/agentMergeController.ts index 258d903b2d6e91..711c04b4bbe8d8 100644 --- a/src/vs/platform/agentHost/node/agentMergeController.ts +++ b/src/vs/platform/agentHost/node/agentMergeController.ts @@ -14,8 +14,9 @@ import { IGitHubService } from '../../github/common/githubService.js'; import { PullRequestRef, PullRequestSnapshot, PullRequestSubscription } from '../../github/common/githubPullRequestService.js'; import { GitHubRequestError } from '../../github/common/githubTransport.js'; import { ILogService } from '../../log/common/log.js'; -import { AgentMergeAction, AgentMergeConfigKey, AgentMergeConfiguration, AgentMergePromptContext, AgentMergeSessionState, agentMergeRootConfigSchema, defaultAgentMergeConfiguration, evaluateAgentMerge, readAgentMergeSessionState, resolveAgentMergeConfiguration } from '../common/agentMerge.js'; +import { AgentMergeConfigKey, AgentMergeConfiguration, AgentMergePromptContext, AgentMergeRepairAction, AgentMergeSessionState, AgentMergeTarget, agentMergeRootConfigSchema, defaultAgentMergeConfiguration, evaluateAgentMerge, readAgentMergeSessionState, resolveAgentMergeConfiguration } from '../common/agentMerge.js'; import { IAgentHostGitStateService } from '../common/agentHostGitStateService.js'; +import { deriveGitHubEndpoints } from '../common/githubEndpoints.js'; import { SessionConfigKey } from '../common/sessionConfigKeys.js'; import { ActionType } from '../common/state/protocol/common/actions.js'; import { AuthRequiredReason } from '../common/state/sessionActions.js'; @@ -27,12 +28,12 @@ import { IAgentMergeTurnContext } from './agentMergeTools.js'; const snapshotDebounce = 30_000; const backstopInterval = 10 * 60_000; -const maximumPromptCommentLength = 2_000; const maximumRepeatedPromptCount = 3; const maximumTotalPromptCount = 6; interface IAgentMergeControllerOptions { readonly startTurn: (session: string, turnId: string, prompt: string) => boolean; + readonly cancelTurn: (session: string, turnId: string) => void; readonly getAutonomousSessionConfig: (session: string, config: Readonly>) => Record | undefined; } @@ -162,7 +163,11 @@ export class AgentMergeController extends Disposable { this._stopRuntime(session); return; } - this._ensureInjectedConfiguration(session, agentMerge); + // Widening approvals mid-turn would hand extra capability to a turn this + // controller does not own, so injection waits for an idle session. + if (!this._stateManager.hasActiveTurn(session)) { + this._reconcileInjectedConfiguration(session, agentMerge); + } let runtime = this._runtimes.get(session); if (!runtime) { runtime = new AgentMergeRuntime(session, () => this._queueEvaluation(session)); @@ -176,25 +181,44 @@ export class AgentMergeController extends Disposable { return this._configurationService.getRootValue(agentMergeRootConfigSchema, AgentMergeConfigKey.Enabled) ?? false; } - private _ensureInjectedConfiguration(session: string, agentMerge: AgentMergeSessionState): void { - if (agentMerge.injectedConfiguration) { - return; - } + /** + * Applies the provider's current autonomous configuration, recomputing it every + * cycle so a tightened managed policy revokes elevated approvals it previously + * granted. The originally observed user values are preserved for restore. + */ + private _reconcileInjectedConfiguration(session: string, agentMerge: AgentMergeSessionState): void { const values = this._configurationService.getSessionConfigValues(session) ?? {}; - const applied = this._options.getAutonomousSessionConfig(session, values); - if (!applied || Object.keys(applied).length === 0) { + const injected = agentMerge.injectedConfiguration; + const applied = this._options.getAutonomousSessionConfig(session, values) ?? {}; + if (!injected && Object.keys(applied).length === 0) { this._logService.debug(`[AgentMergeController] Provider did not select autonomous session configuration: session=${session}`); return; } + const previous: Record = {}; - for (const key of Object.keys(applied)) { - previous[key] = values[key]; + const patch: Record = {}; + for (const [key, value] of Object.entries(applied)) { + previous[key] = injected && Object.hasOwn(injected.previous, key) ? injected.previous[key] : values[key]; + if (!structuralEquals(values[key], value)) { + patch[key] = value; + } + } + // A key the provider no longer selects (e.g. policy revoked it) is rolled + // back, but only while it still holds the value this controller applied. + for (const [key, appliedValue] of Object.entries(injected?.applied ?? {})) { + if (!Object.hasOwn(applied, key) && structuralEquals(values[key], appliedValue)) { + patch[key] = injected!.previous[key]; + } + } + + const nextInjected = Object.keys(applied).length > 0 ? { previous, applied } : undefined; + if (Object.keys(patch).length === 0 && structuralEquals(injected, nextInjected)) { + return; } - const injectedConfiguration = { previous, applied }; - this._logService.info(`[AgentMergeController] Applying provider-selected autonomous session configuration: session=${session}, keys=${Object.keys(applied).sort().join(',')}`); + this._logService.info(`[AgentMergeController] Reconciled autonomous session configuration: session=${session}, applied=${Object.keys(applied).sort().join(',') || 'none'}, changed=${Object.keys(patch).sort().join(',') || 'none'}`); this._configurationService.updateSessionConfig(session, { - [SessionConfigKey.AgentMergeController]: toControllerState(agentMerge, { injectedConfiguration }), - ...applied, + [SessionConfigKey.AgentMergeController]: toControllerState(agentMerge, { injectedConfiguration: nextInjected }), + ...patch, }); } @@ -306,6 +330,10 @@ export class AgentMergeController extends Disposable { if (!this._isCurrentRuntime(session, runtime)) { return; } + if (!ref) { + this._disable(session, agentMerge, 'the bound pull request belongs to a different GitHub host than the signed-in account'); + return; + } const subscription = await this._ensureSubscription(session, runtime, ref); if (!subscription || !this._isCurrentRuntime(session, runtime)) { return; @@ -355,9 +383,10 @@ export class AgentMergeController extends Disposable { commentWatermark: gate.context.commentWatermark, }; if (!this._isCurrentRuntime(session, runtime) + || !this._isTargetStillCurrent(session, target) || this._stateManager.hasActiveTurn(session) || !this._options.startTurn(session, turnId, buildAgentMergePrompt(gate.actions, gate.context))) { - this._logService.debug(`[AgentMergeController] Repair turn was not claimed because the session became busy or stopped: session=${session}`); + this._logService.debug(`[AgentMergeController] Repair turn was not claimed because the session became busy, retargeted, or stopped: session=${session}`); runtime.backstopScheduler.schedule(); return; } @@ -387,9 +416,15 @@ export class AgentMergeController extends Disposable { } } - private async _resolveRef(parsed: { readonly owner: string; readonly repo: string; readonly number: number }, signal: AbortSignal): Promise { + private async _resolveRef(parsed: IParsedPullRequestUrl, signal: AbortSignal): Promise { const credential = await this._gitHubService.credentials.getCredential(signal); - return { ...credential.account, ...parsed }; + // The bound pull request URL carries its own host: after a restore or an + // endpoint switch the same owner/repo/number can name a different GitHub + // instance, which must never be acted on with this account's credential. + if (credential.account.host.toLowerCase() !== parsed.apiHost.toLowerCase()) { + return undefined; + } + return { ...credential.account, owner: parsed.owner, repo: parsed.repo, number: parsed.number }; } private async _ensureSubscription(session: string, runtime: AgentMergeRuntime, ref: PullRequestRef): Promise { @@ -452,12 +487,33 @@ export class AgentMergeController extends Disposable { private _canRepairFork(snapshot: PullRequestSnapshot): boolean { const core = snapshot.core.value; - if (!core?.headRepositoryNameWithOwner || core.headRepositoryNameWithOwner.toLowerCase() === core.repositoryNameWithOwner.toLowerCase()) { + if (!core) { + return false; + } + if (!core.headRepositoryNameWithOwner) { + // Without head provenance the host cannot establish whether pushes to the + // pull request branch are permitted, so it waits for complete state. + return false; + } + if (core.headRepositoryNameWithOwner.toLowerCase() === core.repositoryNameWithOwner.toLowerCase()) { return true; } return core.maintainerCanModify === true; } + /** Whether the session still sits on the branch and pull request this run was authorized for. */ + private _isTargetStillCurrent(session: string, target: AgentMergeTarget): boolean { + const state = this._stateManager.getSessionState(session); + if (!this._hasTargetBranch(state, target.branchName)) { + return false; + } + if (!target.pullRequestUrl) { + return true; + } + const pullRequestUrl = getSessionRelatedPullRequestUrls(readSessionGitHubState(state?._meta))[0]; + return !pullRequestUrl || pullRequestUrl.toLowerCase() === target.pullRequestUrl.toLowerCase(); + } + private async _merge(session: string, runtime: AgentMergeRuntime, ref: PullRequestRef, snapshot: PullRequestSnapshot, configuration: AgentMergeConfiguration, agentMerge: AgentMergeSessionState): Promise { const headSha = snapshot.core.value?.headSha; if (!headSha) { @@ -467,11 +523,31 @@ export class AgentMergeController extends Disposable { } const preparation = await this._gitHubService.mutations.prepareMerge(ref, headSha, runtime.abortController.signal); this._logService.debug(`[AgentMergeController] Native merge preparation completed: session=${session}`); - if (!this._isCurrentRuntime(session, runtime) || this._stateManager.hasActiveTurn(session) || !this._hasTargetBranch(this._stateManager.getSessionState(session), agentMerge.target!.branchName)) { + if (!this._isCurrentRuntime(session, runtime) || this._stateManager.hasActiveTurn(session)) { + runtime.backstopScheduler.schedule(); + return; + } + // Authorization can be withdrawn while preparation is in flight, so the + // merge is re-authorized against live state rather than the captured copy. + const currentState = readAgentMergeSessionState(this._stateManager.getSessionState(session)?.config?.values); + const currentTarget = currentState?.target; + if (!currentState?.enabled + || !currentTarget + || !this._isTargetStillCurrent(session, currentTarget) + || currentTarget.pullRequestUrl !== agentMerge.target?.pullRequestUrl) { + this._logService.info(`[AgentMergeController] Native merge abandoned because authorization or target changed: session=${session}`); runtime.backstopScheduler.schedule(); return; } - const freshGate = evaluateAgentMerge(preparation.snapshot, configuration, agentMerge.target!.commentWatermark); + const currentConfiguration = this._getConfiguration(currentState); + if (!currentConfiguration.mergePullRequest) { + this._logService.info(`[AgentMergeController] Native merge abandoned because automatic merge was switched off: session=${session}`); + runtime.backstopScheduler.schedule(); + return; + } + // `prepareMerge` captures an authoritative snapshot of every fragment the gate + // reads, with top-level comments refreshed last, so it is re-evaluated as-is. + const freshGate = evaluateAgentMerge(preparation.snapshot, currentConfiguration, currentTarget.commentWatermark); if (freshGate.kind !== 'merge') { this._logService.info(`[AgentMergeController] Native merge aborted after fresh readiness check: session=${session}, outcome=${freshGate.kind}`); this._schedule(session, 0); @@ -479,7 +555,7 @@ export class AgentMergeController extends Disposable { } const authorization = { confirmed: true as const, - authorizationId: `${agentMerge.target!.enabledAt}:${agentMerge.target!.pullRequestUrl}`, + authorizationId: `${currentTarget.enabledAt}:${currentTarget.pullRequestUrl}`, }; if (preparation.snapshot.mergeability.value!.mergeQueueRequired) { const result = await this._gitHubService.mutations.enqueue(preparation, authorization, runtime.abortController.signal); @@ -487,7 +563,7 @@ export class AgentMergeController extends Disposable { runtime.backstopScheduler.schedule(); return; } - const method = resolveMergeMethod(configuration.mergeMethod, preparation.snapshot.mergeability.value!.allowedMergeMethods); + const method = resolveMergeMethod(currentConfiguration.mergeMethod, preparation.snapshot.mergeability.value!.allowedMergeMethods); if (!method) { this._logService.warn(`[AgentMergeController] No allowed merge method is available for ${session}`); runtime.backstopScheduler.schedule(); @@ -495,7 +571,7 @@ export class AgentMergeController extends Disposable { } const result = await this._gitHubService.mutations.merge(preparation, { method, authorization }, runtime.abortController.signal); this._logService.info(`[AgentMergeController] Pull request merged natively: session=${session}, method=${method}, outcome=${result.outcome}`); - this._disable(session, agentMerge, 'the pull request was merged'); + this._disable(session, currentState, 'the pull request was merged'); } private async _completeTurn(session: string): Promise { @@ -563,7 +639,16 @@ export class AgentMergeController extends Disposable { } private _stopRuntime(session: string): void { - this._activeTurns.delete(session); + // A repair turn started by this controller must not keep running with the + // elevated capabilities that Agent Merge granted it. + const context = this._activeTurns.get(session); + if (context) { + this._activeTurns.delete(session); + if (this._stateManager.getSessionState(session)?.activeTurn?.id === context.turnId) { + this._logService.info(`[AgentMergeController] Cancelling repair turn because Agent Merge stopped: session=${session}, turn=${context.turnId}`); + this._options.cancelTurn(session, context.turnId); + } + } if (this._runtimes.has(session)) { this._runtimes.deleteAndDispose(session); this._logService.debug(`[AgentMergeController] Disposed session runtime: session=${session}`); @@ -595,7 +680,15 @@ export class AgentMergeController extends Disposable { } } -function parsePullRequestUrl(value: string): { readonly owner: string; readonly repo: string; readonly number: number } | undefined { +interface IParsedPullRequestUrl { + readonly owner: string; + readonly repo: string; + readonly number: number; + /** REST API host the credential account must match (`api.github.com` for github.com). */ + readonly apiHost: string; +} + +export function parsePullRequestUrl(value: string): IParsedPullRequestUrl | undefined { let url: URL; try { url = new URL(value); @@ -604,9 +697,18 @@ function parsePullRequestUrl(value: string): { readonly owner: string; readonly } const match = /^\/(?[^/]+)\/(?[^/]+)\/pull\/(?\d+)\/?$/.exec(url.pathname); const number = Number(match?.groups?.number); - return match?.groups && Number.isSafeInteger(number) && number > 0 - ? { owner: match.groups.owner, repo: match.groups.repo, number } - : undefined; + if (!match?.groups || !Number.isSafeInteger(number) || number <= 0) { + return undefined; + } + const host = url.host.toLowerCase(); + return { + owner: match.groups.owner, + repo: match.groups.repo, + number, + // Derived rather than hard-coded so GitHub Enterprise Cloud web hosts + // (`tenant.ghe.com`) canonicalize to the `api.` host the credential reports. + apiHost: new URL(deriveGitHubEndpoints(`${url.protocol}//${host}`).apiBaseUri).host.toLowerCase(), + }; } function sameRef(left: PullRequestRef, right: PullRequestRef): boolean { @@ -633,13 +735,13 @@ function resolveMergeMethod(configured: AgentMergeConfiguration['mergeMethod'], return (['SQUASH', 'MERGE', 'REBASE'] as const).find(method => allowed.includes(method)); } -function buildAgentMergePrompt(actions: readonly AgentMergeAction[], context: AgentMergePromptContext): string { +function buildAgentMergePrompt(actions: readonly AgentMergeRepairAction[], context: AgentMergePromptContext): string { const actionLabels = actions.map(action => { switch (action) { - case 'addressReviews': return 'address review feedback'; case 'fixCI': return 'fix failed required CI checks'; case 'resolveConflicts': return 'resolve conflicts or update the behind branch'; - case 'mergePullRequest': return 'merge the pull request'; + case 'addressReviews': + default: return 'address review feedback'; } }); const details = [ @@ -648,8 +750,8 @@ function buildAgentMergePrompt(actions: readonly AgentMergeAction[], context: Ag `Head: ${context.headRef} (${context.headSha})`, `Base: ${context.baseRef}`, `Unresolved authorized review threads:\n${formatReviewThreads(context.reviewThreads)}`, - `Changes-requested reviews: ${truncatePromptItems(context.reviewSummaries)}`, - `New authorized comments: ${truncatePromptItems(context.newComments)}`, + `Changes-requested reviews: ${formatFeedbackComments(context.reviewSummaries)}`, + `New authorized comments: ${formatFeedbackComments(context.newComments)}`, `Failed required checks: ${context.failedChecks.join(', ') || 'none'}`, `Behind base: ${context.behind ? 'yes' : 'no'}`, `Conflicting: ${context.conflicting ? 'yes' : 'no'}`, @@ -668,11 +770,15 @@ function buildAgentMergePrompt(actions: readonly AgentMergeAction[], context: Ag ].join('\n'); } -function truncatePromptItems(values: readonly string[]): string { - if (values.length === 0) { +function formatFeedbackComments(comments: AgentMergePromptContext['newComments']): string { + if (comments.length === 0) { return 'none'; } - return values.map(value => value.slice(0, maximumPromptCommentLength)).join('\n---\n'); + return comments.map(formatFeedbackComment).join('\n---\n'); +} + +function formatFeedbackComment(comment: { readonly author?: string; readonly body: string }): string { + return `${comment.author ? `${comment.author}: ` : ''}${comment.body || '(no body)'}`; } function formatReviewThreads(threads: AgentMergePromptContext['reviewThreads']): string { @@ -682,8 +788,7 @@ function formatReviewThreads(threads: AgentMergePromptContext['reviewThreads']): return threads.map(thread => [ `Thread ${thread.id}`, ...(thread.path ? [`File: ${thread.path}${thread.line !== undefined ? `:${thread.line}` : ''}`] : []), - ...(thread.author ? [`Author: ${thread.author}`] : []), - `Feedback: ${thread.body || '(no body)'}`, + `Feedback:\n${thread.comments.map(formatFeedbackComment).join('\n') || '(no body)'}`, ].join('\n')).join('\n---\n'); } diff --git a/src/vs/platform/agentHost/node/agentService.ts b/src/vs/platform/agentHost/node/agentService.ts index 4f5beca0d3f520..c0cbb1ceb7ea4f 100644 --- a/src/vs/platform/agentHost/node/agentService.ts +++ b/src/vs/platform/agentHost/node/agentService.ts @@ -36,7 +36,7 @@ import type { CompletionsParams, CompletionsResult, CreateTerminalParams, Resolv import type { InvokeChangesetOperationParams, InvokeChangesetOperationResult } from '../common/state/protocol/channels-changeset/commands.js'; import { AhpErrorCodes, AHP_SESSION_NOT_FOUND, ContentEncoding, JSON_RPC_INTERNAL_ERROR, ProtocolError, ResourceChangeType, ResourceType, ResourceWriteMode, type CreateResourceWatchParams, type CreateResourceWatchResult, type DirectoryEntry, type ResourceCopyParams, type ResourceCopyResult, type ResourceDeleteParams, type ResourceDeleteResult, type ResourceListResult, type ResourceMkdirParams, type ResourceMkdirResult, type ResourceMoveParams, type ResourceMoveResult, type ResourceReadResult, type ResourceResolveParams, type ResourceResolveResult, type ResourceWatchState, type ResourceWriteParams, type ResourceWriteResult, type IStateSnapshot } from '../common/state/sessionProtocol.js'; import { ChangesSummary, ChatInteractivity, ChatOriginKind, MessageAttachmentKind, type Annotation, type AnnotationEntry, type AnnotationsState, type ChatOrigin, type Customization, type Message, type MessageAttachment, type MessageResourceAttachment } from '../common/state/protocol/state.js'; -import type { ChatPendingMessageSetAction, ChatTurnStartedAction } from '../common/state/protocol/actions.js'; +import type { ChatPendingMessageSetAction, ChatTurnStartedAction, SessionConfigChangedAction } from '../common/state/protocol/actions.js'; import { ISessionGitHubState, ISessionGitState, MessageKind, ResponsePartKind, SESSION_META_GITHUB_KEY, SESSION_META_GIT_KEY, SESSION_META_MULTI_ROOT_KEY, SESSION_META_SOURCE_CONTROL_KEY, readSessionSpawnDepth, withSessionSpawnDepth, SessionLifecycle, SessionStatus, ToolCallStatus, ToolResultContentType, AH_META_WORKSPACELESS_DB_KEY, AH_META_IS_ARCHIVED_DB_KEY, AH_META_IS_DONE_DB_KEY, AH_META_IS_READ_DB_KEY, buildChatUri, buildDefaultChatUri, buildResourceWatchChannelUri, buildSubagentChatUri, buildSubagentSessionUriPrefix, hostBuildInfoFromProduct, isAhpChatChannel, isDefaultChatUri, isSubagentChatUri, isSubagentSession, parseChatUri, parseDefaultChatUri, parseRequiredSessionUriFromChatUri, parseResourceWatchChannelUri, parseSessionMultiRootMetadata, parseSubagentSessionUri, readSessionExternal, readSessionGitHubState, readSessionGitState, readSessionMultiRootMetadata, readSessionSourceControlState, readSessionWorkspaceless, withSessionExternal, withSessionGitHubState, withSessionGitState, withSessionMultiRootMetadata, withSessionSourceControlState, withSessionStatusFlag, withSessionWorkspaceless, readSessionEhcliAdoptable, type ISessionSourceControlState, type SessionConfigState, type SessionSummary, type ToolResultSubagentContent, type Turn, type UsageInfo, chatStorageUri, hasReportedUsage } from '../common/state/sessionState.js'; import { readToolCallMeta } from '../common/meta/agentToolCallMeta.js'; import { IProductService } from '../../product/common/productService.js'; @@ -86,6 +86,7 @@ import { AgentHostChangesetOperationService } from './agentHostChangesetOperatio import { AgentHostGitStateService } from './agentHostGitStateService.js'; import { AgentHostGitHubEndpointService, IAgentHostGitHubEndpointService } from './agentHostGitHubEndpointService.js'; import { AgentMergeController } from './agentMergeController.js'; +import { AgentMergeConfigKey, agentMergeRootConfigSchema } from '../common/agentMerge.js'; import { AgentMergeTools } from './agentMergeTools.js'; import { ITelemetryService } from '../../telemetry/common/telemetry.js'; import { NullTelemetryService } from '../../telemetry/common/telemetryUtils.js'; @@ -153,6 +154,15 @@ const HOST_OWNED_SESSION_CONFIG_KEYS = [ SessionConfigKey.WorktreeBranchTrack, ] as const; +/** + * Host-owned session config a client may never write. These carry Agent Merge + * authorization state (bound pull request, feedback watermark, attempt budgets) + * that the host derives itself. + */ +const HOST_WRITTEN_SESSION_CONFIG_KEYS = [ + SessionConfigKey.AgentMergeController, +] as const; + function omitHostOwnedSessionConfig(config: Record): Record { const result = { ...config }; for (const key of HOST_OWNED_SESSION_CONFIG_KEYS) { @@ -574,7 +584,7 @@ export class AgentService extends Disposable implements IAgentService { this._configurationService = configurationService; let externalSessionsMode = this._getExternalSessionsMode(); this._lastMigrateLegacyEnabled = this._isMigrateLegacyEnabled(); - let agentMergeEnabled: boolean | undefined; + let agentMergeEnabled = this._isAgentMergeEnabled(); this._register(configurationService.onDidRootConfigChange(() => { const nextMode = this._getExternalSessionsMode(); if (nextMode !== externalSessionsMode) { @@ -584,13 +594,13 @@ export class AgentService extends Disposable implements IAgentService { } // Agent Merge tools are only advertised while the feature is on, so a // toggle has to reach sessions that were advertised under the old value. - const nextAgentMergeEnabled = this._agentMergeController.isEnabled(); - if (agentMergeEnabled !== undefined && nextAgentMergeEnabled !== agentMergeEnabled) { + const nextAgentMergeEnabled = this._isAgentMergeEnabled(); + if (nextAgentMergeEnabled !== agentMergeEnabled) { + agentMergeEnabled = nextAgentMergeEnabled; for (const session of this._stateManager.getSessionUris()) { this._serverToolHost.advertise(session); } } - agentMergeEnabled = nextAgentMergeEnabled; this._onMigrateLegacySettingChanged(); })); const fileMonitorService = _fileMonitorService ?? this._register(new AgentHostFileMonitorService(this._fileService, this._logService)); @@ -650,6 +660,7 @@ export class AgentService extends Disposable implements IAgentService { services.set(IAgentHostGitStateService, this._gitStateService); this._agentMergeController = this._register(instantiationService.createInstance(AgentMergeController, { startTurn: (session, turnId, prompt) => this._startAgentMergePrompt(session, turnId, prompt), + cancelTurn: (session, turnId) => this._cancelAgentMergePrompt(session, turnId), getAutonomousSessionConfig: (session, config) => this._findProviderForSession(session)?.getAutonomousSessionConfig?.(config), })); @@ -1115,6 +1126,17 @@ export class AgentService extends Disposable implements IAgentService { return true; } + /** + * Cancels a repair turn this host started for Agent Merge, so a stopped or + * revoked controller cannot leave an autonomous turn running. + */ + private _cancelAgentMergePrompt(session: string, turnId: string): void { + const chat = buildDefaultChatUri(session).toString(); + const action = { type: ActionType.ChatTurnCancelled, turnId, duration: 0 } as const; + this._stateManager.dispatchServerAction(chat, action); + this._sideEffects.handleAction(chat, action); + } + /** * Reads a point-in-time snapshot of a session's chat conversation for the * `get_session_context` server tool. Targets the session's default chat, or a @@ -1783,6 +1805,10 @@ export class AgentService extends Disposable implements IAgentService { return this._configurationService.getRootValue(platformRootSchema, AgentHostMigrateLegacyCopilotCliEnabledConfigKey) === true; } + private _isAgentMergeEnabled(): boolean { + return this._configurationService.getRootValue(agentMergeRootConfigSchema, AgentMergeConfigKey.Enabled) === true; + } + /** Retracts un-opened adoptable-legacy entries when migration is turned off (deletes no data). */ private _onMigrateLegacySettingChanged(): void { const enabled = this._isMigrateLegacyEnabled(); @@ -3740,12 +3766,46 @@ export class AgentService extends Disposable implements IAgentService { return resolveSessionWorkingDirectoryAction(action, state.workingDirectories, capability.immutablePrimary === true); } + /** + * Carries host-written session config through a client replacement. A client + * may legitimately replace its own config wholesale, but omitting a host-owned + * key must not clear it, since that would reset Agent Merge authorization state. + */ + private _withPreservedHostWrittenSessionConfig(session: string, action: SessionConfigChangedAction): SessionConfigChangedAction { + const values = this._stateManager.getSessionState(session)?.config?.values; + if (!values) { + return action; + } + let preserved: Record | undefined; + for (const key of HOST_WRITTEN_SESSION_CONFIG_KEYS) { + if (Object.hasOwn(values, key)) { + preserved ??= {}; + preserved[key] = values[key]; + } + } + return preserved ? { ...action, config: { ...action.config, ...preserved } } : action; + } + private _dispatchActionNow(channel: string, sessionChannel: string, action: SessionAction | ChatAction | TerminalAction | ClientChangesetAction | ClientAnnotationsAction | IRootConfigChangedAction, clientId: string, clientSeq: number, clientContext: IAgentHostClientTelemetryContext): void { const origin = { clientId, clientSeq }; if (action.type === ActionType.ChatTurnStarted && this._isTurnIdUsedByAnotherChat(sessionChannel, channel, action.turnId)) { this._stateManager.rejectClientAction(channel, action, origin, 'Turn id is already used by another chat in this session.'); return; } + // Host-owned session config carries merge authorization (bound pull request, + // watermark, attempt budgets), so a client must never be able to write it, and + // a wholesale replacement must not drop it either. + if (action.type === ActionType.SessionConfigChanged) { + const configAction = action as SessionConfigChangedAction; + const forbidden = HOST_WRITTEN_SESSION_CONFIG_KEYS.filter(key => Object.hasOwn(configAction.config, key)); + if (forbidden.length > 0) { + this._stateManager.rejectClientAction(channel, action, origin, `Session config keys are host-owned and cannot be set by a client: ${forbidden.join(', ')}.`); + return; + } + if (configAction.replace) { + action = this._withPreservedHostWrittenSessionConfig(sessionChannel, configAction); + } + } if (action.type === ActionType.SessionWorkingDirectorySet || action.type === ActionType.SessionWorkingDirectoryRemoved) { if (clientContext.clientType !== AgentHostClientType.EditorWindow) { this._stateManager.rejectClientAction(channel, action, origin, 'Session working-directory actions require an Editor Window client.'); diff --git a/src/vs/platform/agentHost/test/common/agentMerge.test.ts b/src/vs/platform/agentHost/test/common/agentMerge.test.ts index 567148ff9ec8d0..bda060b221b311 100644 --- a/src/vs/platform/agentHost/test/common/agentMerge.test.ts +++ b/src/vs/platform/agentHost/test/common/agentMerge.test.ts @@ -28,7 +28,11 @@ suite('Agent Merge gate', () => { isResolved: false, path: 'src/example.ts', line: 42, - comments: [{ id: 'comment-1', author: { login: 'maintainer', association: 'MEMBER' }, body: 'Please fix this' }], + comments: [ + { id: 'comment-1', author: { login: 'maintainer', association: 'MEMBER' }, body: 'Please fix this' }, + { id: 'comment-2', author: { login: 'outsider', association: 'CONTRIBUTOR' }, body: 'Unauthorized' }, + { id: 'comment-3', author: { login: 'maintainer', association: 'MEMBER' }, body: 'Actually rename it instead' }, + ], }], topLevelComments: [ { id: 'old', author: { login: 'maintainer', association: 'OWNER' }, body: 'Old', createdAt: '2026-08-01T00:00:00.000Z' }, @@ -40,40 +44,34 @@ suite('Agent Merge gate', () => { ], }); + // Every authorized comment is carried so a later follow-up cannot be lost. + const expectedContext = { + pullRequestUrl: 'https://github.com/octo/repo/pull/1', + title: 'Change', + headSha: 'head', + baseRef: 'main', + headRef: 'feature', + reviewThreads: [{ + id: 'thread-1', + path: 'src/example.ts', + line: 42, + comments: [ + { author: 'maintainer', body: 'Please fix this' }, + { author: 'maintainer', body: 'Actually rename it instead' }, + ], + }], + reviewSummaries: [], + newComments: [], + failedChecks: ['Build'], + behind: false, + conflicting: false, + commentWatermark: '2026-08-02T00:00:00.000Z', + }; assert.deepStrictEqual(evaluateAgentMerge(snapshot, configuration, '2026-08-02T00:00:00.000Z'), { kind: 'prompt', actions: ['addressReviews', 'fixCI'], - fingerprint: JSON.stringify({ - actions: ['addressReviews', 'fixCI'], - context: { - pullRequestUrl: 'https://github.com/octo/repo/pull/1', - title: 'Change', - headSha: 'head', - baseRef: 'main', - headRef: 'feature', - reviewThreads: [{ id: 'thread-1', path: 'src/example.ts', line: 42, author: 'maintainer', body: 'Please fix this' }], - reviewSummaries: [], - newComments: [], - failedChecks: ['Build'], - behind: false, - conflicting: false, - commentWatermark: '2026-08-02T00:00:00.000Z', - }, - }), - context: { - pullRequestUrl: 'https://github.com/octo/repo/pull/1', - title: 'Change', - headSha: 'head', - baseRef: 'main', - headRef: 'feature', - reviewThreads: [{ id: 'thread-1', path: 'src/example.ts', line: 42, author: 'maintainer', body: 'Please fix this' }], - reviewSummaries: [], - newComments: [], - failedChecks: ['Build'], - behind: false, - conflicting: false, - commentWatermark: '2026-08-02T00:00:00.000Z', - }, + fingerprint: JSON.stringify({ actions: ['addressReviews', 'fixCI'], context: expectedContext }), + context: expectedContext, }); }); @@ -120,6 +118,34 @@ suite('Agent Merge gate', () => { }); }); + test('keeps feedback bounded for a comment-heavy pull request', () => { + const result = evaluateAgentMerge(readySnapshot({ + reviewThreads: Array.from({ length: 40 }, (_, index) => ({ + id: `thread-${index}`, + isResolved: false, + comments: Array.from({ length: 20 }, (_, comment) => ({ + id: `comment-${index}-${comment}`, + author: { login: 'maintainer', association: 'MEMBER' }, + body: 'x'.repeat(5_000), + })), + })), + }), configuration, '2026-08-02T00:00:00.000Z'); + + const context = result.kind === 'prompt' ? result.context : undefined; + const totalBodyLength = (context?.reviewThreads ?? []) + .flatMap(thread => thread.comments) + .reduce((total, comment) => total + comment.body.length, 0); + assert.deepStrictEqual({ + threads: context?.reviewThreads.length, + commentsPerThread: context?.reviewThreads[0]?.comments.length, + withinBudget: totalBodyLength <= 20_000, + }, { + threads: 10, + commentsPerThread: 5, + withinBudget: true, + }); + }); + test('keeps client and controller state in separate config values', () => { assert.deepStrictEqual(readAgentMergeSessionState({ [SessionConfigKey.AgentMerge]: { enabled: true, overrides: { fixCI: false } }, diff --git a/src/vs/platform/agentHost/test/node/agentMergeController.test.ts b/src/vs/platform/agentHost/test/node/agentMergeController.test.ts index 26ecbcbbe9e03d..149dba8e090e0a 100644 --- a/src/vs/platform/agentHost/test/node/agentMergeController.test.ts +++ b/src/vs/platform/agentHost/test/node/agentMergeController.test.ts @@ -13,11 +13,11 @@ import { AgentHostAutoApprovePolicyRestrictedConfigKey, platformRootSchema, plat import { IAgentHostGitStateService } from '../../common/agentHostGitStateService.js'; import { SessionConfigKey } from '../../common/sessionConfigKeys.js'; import { ActionType } from '../../common/state/protocol/common/actions.js'; -import { SessionStatus, type SessionSummary } from '../../common/state/sessionState.js'; +import { SessionStatus, buildDefaultChatUri, MessageKind, type SessionSummary } from '../../common/state/sessionState.js'; import { IGitHubService } from '../../../github/common/githubService.js'; import { AgentConfigurationService } from '../../node/agentConfigurationService.js'; import { AgentHostGitHubEndpointService } from '../../node/agentHostGitHubEndpointService.js'; -import { AgentMergeController } from '../../node/agentMergeController.js'; +import { AgentMergeController, parsePullRequestUrl } from '../../node/agentMergeController.js'; import { AgentHostStateManager } from '../../node/agentHostStateManager.js'; let sessionCounter = 0; @@ -40,6 +40,7 @@ suite('AgentMergeController', () => { disposables.add(new AgentMergeController( { startTurn: () => false, + cancelTurn: () => { }, getAutonomousSessionConfig: () => ({ [SessionConfigKey.Mode]: 'autopilot', [SessionConfigKey.AutoApprove]: 'assisted', @@ -158,6 +159,65 @@ suite('AgentMergeController', () => { }); }); + test('tightened managed policy revokes an already elevated approval', () => { + const { stateManager, configurationService, session } = createControllerHarness(disposables); + configurationService.updateSessionConfig(session, { + [SessionConfigKey.Mode]: 'interactive', + [SessionConfigKey.AutoApprove]: 'default', + [SessionConfigKey.AgentMerge]: { enabled: true }, + }); + stateManager.dispatchServerAction(session, { type: ActionType.SessionReady }); + const elevated = configurationService.getSessionConfigValues(session)?.[SessionConfigKey.AutoApprove]; + + // Policy revokes the elevated level while the session stays enabled. + configurationService.updateRootConfig({ [AgentHostAutoApprovePolicyRestrictedConfigKey]: true }); + + const values = configurationService.getSessionConfigValues(session); + assert.deepStrictEqual({ + elevated, + mode: values?.[SessionConfigKey.Mode], + autoApprove: values?.[SessionConfigKey.AutoApprove], + injected: readAgentMergeSessionState(values)?.injectedConfiguration, + }, { + elevated: 'assisted', + mode: 'autopilot', + autoApprove: 'default', + injected: { + previous: { [SessionConfigKey.Mode]: 'interactive' }, + applied: { [SessionConfigKey.Mode]: 'autopilot' }, + }, + }); + }); + + test('does not widen approvals while a turn is active', () => { + const { stateManager, configurationService, session } = createControllerHarness(disposables); + configurationService.updateSessionConfig(session, { + [SessionConfigKey.Mode]: 'interactive', + [SessionConfigKey.AutoApprove]: 'default', + }); + stateManager.dispatchServerAction(session, { type: ActionType.SessionReady }); + stateManager.dispatchServerAction(buildDefaultChatUri(session), { + type: ActionType.ChatTurnStarted, + turnId: 'user-turn', + startedAt: new Date().toISOString(), + message: { text: 'hello', origin: { kind: MessageKind.User } }, + }); + configurationService.updateSessionConfig(session, { + [SessionConfigKey.AgentMerge]: { enabled: true }, + }); + + const values = configurationService.getSessionConfigValues(session); + assert.deepStrictEqual({ + mode: values?.[SessionConfigKey.Mode], + autoApprove: values?.[SessionConfigKey.AutoApprove], + injected: readAgentMergeSessionState(values)?.injectedConfiguration, + }, { + mode: 'interactive', + autoApprove: 'default', + injected: undefined, + }); + }); + function createControllerHarness(disposables: ReturnType): { readonly stateManager: AgentHostStateManager; readonly configurationService: AgentConfigurationService; @@ -175,6 +235,7 @@ suite('AgentMergeController', () => { disposables.add(new AgentMergeController( { startTurn: () => false, + cancelTurn: () => { }, getAutonomousSessionConfig: () => configurationService.getRootValue(platformRootSchema, AgentHostAutoApprovePolicyRestrictedConfigKey) === true ? { [SessionConfigKey.Mode]: 'autopilot' } : { @@ -197,6 +258,28 @@ suite('AgentMergeController', () => { }); return { stateManager, configurationService, session }; } + + test('resolves the API host a credential must match for every GitHub deployment', () => { + assert.deepStrictEqual({ + dotCom: parsePullRequestUrl('https://github.com/octo/repo/pull/1')?.apiHost, + www: parsePullRequestUrl('https://www.github.com/octo/repo/pull/1')?.apiHost, + // GitHub Enterprise Cloud serves its API from an `api.` subdomain, which is + // the host the credential reports; comparing the web host rejects every PR. + enterpriseCloud: parsePullRequestUrl('https://tenant.ghe.com/octo/repo/pull/1')?.apiHost, + enterpriseServer: parsePullRequestUrl('https://ghe.corp.example/octo/repo/pull/1')?.apiHost, + parsed: parsePullRequestUrl('https://tenant.ghe.com/octo/repo/pull/42'), + notAPullRequest: parsePullRequestUrl('https://github.com/octo/repo/issues/1'), + notAUrl: parsePullRequestUrl('octo/repo#1'), + }, { + dotCom: 'api.github.com', + www: 'api.github.com', + enterpriseCloud: 'api.tenant.ghe.com', + enterpriseServer: 'ghe.corp.example', + parsed: { owner: 'octo', repo: 'repo', number: 42, apiHost: 'api.tenant.ghe.com' }, + notAPullRequest: undefined, + notAUrl: undefined, + }); + }); }); function summary(resource: string): SessionSummary { diff --git a/src/vs/platform/agentHost/test/node/agentService.test.ts b/src/vs/platform/agentHost/test/node/agentService.test.ts index a36afffbb092a3..39be736a2de592 100644 --- a/src/vs/platform/agentHost/test/node/agentService.test.ts +++ b/src/vs/platform/agentHost/test/node/agentService.test.ts @@ -1423,6 +1423,78 @@ suite('AgentService (node dispatcher)', () => { }); }); + test('rejects client writes to host-owned Agent Merge controller state', async () => { + const svc = disposables.add(new AgentService(new NullLogService(), fileService, createSessionDataService(new TestSessionDatabase()), { _serviceBrand: undefined } as IProductService, createNoopGitService())); + const agent = new MockAgent('copilot'); + disposables.add(toDisposable(() => agent.dispose())); + svc.registerProvider(agent); + const session = await svc.createSession({ provider: 'copilot' }); + const envelopePromise = Event.toPromise(Event.filter(svc.onDidAction, envelope => envelope.origin?.clientSeq === 1)); + + // A forged target would otherwise authorize a native merge of any pull request. + svc.dispatchAction(session.toString(), { + type: ActionType.SessionConfigChanged, + config: { + [SessionConfigKey.AgentMergeController]: { + target: { branchName: 'main', pullRequestUrl: 'https://github.com/octo/repo/pull/1', enabledAt: '2026-01-01T00:00:00.000Z', commentWatermark: '2026-01-01T00:00:00.000Z' }, + }, + }, + }, 'agents-window-client', 1, AgentHostClientType.AgentsWindow); + const envelope = await envelopePromise; + + assert.deepStrictEqual({ + rejectionReason: envelope.rejectionReason, + controllerState: svc.stateManager.getSessionState(session.toString())?.config?.values[SessionConfigKey.AgentMergeController], + }, { + rejectionReason: `Session config keys are host-owned and cannot be set by a client: ${SessionConfigKey.AgentMergeController}.`, + controllerState: undefined, + }); + }); + + test('preserves host-owned Agent Merge controller state across a client config replacement', async () => { + const svc = disposables.add(new AgentService(new NullLogService(), fileService, createSessionDataService(new TestSessionDatabase()), { _serviceBrand: undefined } as IProductService, createNoopGitService())); + const agent = new MockAgent('copilot'); + disposables.add(toDisposable(() => agent.dispose())); + svc.registerProvider(agent); + const controllerState = { target: { branchName: 'main', pullRequestUrl: 'https://github.com/octo/repo/pull/1', enabledAt: '2026-01-01T00:00:00.000Z', commentWatermark: '2026-01-01T00:00:00.000Z' } }; + const session = await svc.createSession({ provider: 'copilot', config: { [SessionConfigKey.AgentMergeController]: controllerState } }); + const envelopePromise = Event.toPromise(Event.filter(svc.onDidAction, envelope => envelope.origin?.clientSeq === 1)); + + // A wholesale replacement that omits the key must not clear the binding, + // which would otherwise reset the watermark and attempt budgets. + svc.dispatchAction(session.toString(), { + type: ActionType.SessionConfigChanged, + config: { [SessionConfigKey.AgentMerge]: { enabled: true } }, + replace: true, + }, 'agents-window-client', 1, AgentHostClientType.AgentsWindow); + const envelope = await envelopePromise; + + assert.deepStrictEqual({ + rejectionReason: envelope.rejectionReason, + controllerState: svc.stateManager.getSessionState(session.toString())?.config?.values[SessionConfigKey.AgentMergeController], + }, { + rejectionReason: undefined, + controllerState, + }); + }); + + test('accepts client writes to the client-owned Agent Merge enablement value', async () => { + const svc = disposables.add(new AgentService(new NullLogService(), fileService, createSessionDataService(new TestSessionDatabase()), { _serviceBrand: undefined } as IProductService, createNoopGitService())); + const agent = new MockAgent('copilot'); + disposables.add(toDisposable(() => agent.dispose())); + svc.registerProvider(agent); + const session = await svc.createSession({ provider: 'copilot' }); + const envelopePromise = Event.toPromise(Event.filter(svc.onDidAction, envelope => envelope.origin?.clientSeq === 1)); + + svc.dispatchAction(session.toString(), { + type: ActionType.SessionConfigChanged, + config: { [SessionConfigKey.AgentMerge]: { enabled: true } }, + }, 'agents-window-client', 1, AgentHostClientType.AgentsWindow); + const envelope = await envelopePromise; + + assert.strictEqual(envelope.rejectionReason, undefined); + }); + test('accepts a working-directory mutation synchronously', async () => { const { svc, session, primary, secondary } = await createDynamicWorkingDirectorySession(); const added = URI.file('/workspace/added'); diff --git a/src/vs/platform/github/common/pullRequestMutationService.ts b/src/vs/platform/github/common/pullRequestMutationService.ts index 444cff78160036..d6a21db7e6016f 100644 --- a/src/vs/platform/github/common/pullRequestMutationService.ts +++ b/src/vs/platform/github/common/pullRequestMutationService.ts @@ -345,7 +345,7 @@ export class PullRequestMutationService extends Disposable implements IPullReque } const subscription = this._resources.subscribePullRequest(ref, { priority: 'interactive', - conversation: { submittedReviews: true, reviewThreads: true }, + conversation: { topLevelComments: true, submittedReviews: true, reviewThreads: true }, checks: { required: true, includeOptional: true }, mergeability: true, }); @@ -358,6 +358,11 @@ export class PullRequestMutationService extends Disposable implements IPullReque subscription.refresh('reviewThreads', cancellation.tokenSource.token, { authoritative: true }), subscription.refresh('mergeability', cancellation.tokenSource.token, { authoritative: true }), ]); + // Refreshed last so that a comment posted while the fragments above were + // in flight is still part of the captured snapshot. Callers gate merges on + // new maintainer comments, and a comment that lands after this point bumps + // the resource generation, which invalidates the preparation. + await subscription.refresh('topLevelComments', cancellation.tokenSource.token, { authoritative: true }); if (signal.aborted) { throw signal.reason ?? new Error('Merge preparation was cancelled'); } diff --git a/src/vs/sessions/contrib/providers/agentHost/AGENT_HOST_SESSIONS_PROVIDER.md b/src/vs/sessions/contrib/providers/agentHost/AGENT_HOST_SESSIONS_PROVIDER.md index 9967c4e6ef3b45..b161b658b545bf 100644 --- a/src/vs/sessions/contrib/providers/agentHost/AGENT_HOST_SESSIONS_PROVIDER.md +++ b/src/vs/sessions/contrib/providers/agentHost/AGENT_HOST_SESSIONS_PROVIDER.md @@ -69,17 +69,17 @@ Pull-request identity uses the Agent Host's configured GitHub host. Never canoni Agent Merge is a provider-neutral Agent Host controller. Copilot, Claude, and Codex use the same persisted session state, pull-request subscription, readiness gate, system-initiated repair turn, scoped GitHub tools, and native merge executor. The controller does not encode provider approval keys: the owning `IAgent` selects the provider-native autonomous configuration to apply and the controller records and conditionally restores that patch. -The client owns only enablement and per-session action overrides under session config. The host owns the bound branch and pull request, comment watermark, injected permission-mode state, deduplication fingerprint, and retry count under a separate host-only config value. Both values survive session restore; provider config never receives either value. +The client owns only enablement and per-session action overrides under session config. The host owns the bound branch and pull request, comment watermark, injected permission-mode state, deduplication fingerprint, and retry count under a separate host-only config value. That host-owned value carries merge authorization, so client-dispatched writes to it are rejected at the action dispatch boundary, and a client config replacement that omits it preserves rather than clears it. Both values survive session restore; provider config never receives either value. Agent Merge's session config names are well-known `SessionConfigKey` values alongside the other platform-consumed keys. `AgentService` applies host session config through one contribution pipeline with separate worktree and Agent Merge contributors, so provider resolution remains isolated and future host-owned features can extend the pipeline without overloading an existing feature-specific wrapper. -Global `chat.agentHost.agentMerge.*` settings are mirrored into Agent Host root config and apply live. A session can override address-reviews, fix-CI, resolve-conflicts, and merge actions, or reset to the global defaults. Enabling Agent Merge injects Autopilot mode and Assisted approvals unless managed policy forbids elevated approval modes. Disabling restores each value only while it still equals the injected value, preserving later manual changes. +Global `chat.agentMerge.*` settings are mirrored into Agent Host root config and apply live. A session can override address-reviews, fix-CI, resolve-conflicts, and merge actions, or reset to the global defaults. While a session is enabled and idle, the controller reconciles the provider-selected autonomous configuration on every cycle, so a tightened managed policy revokes an approval level it previously granted. Injection never happens during an active turn, so a turn the controller does not own is never widened. Disabling restores each value only while it still equals the injected value, preserving later manual changes. -The controller subscribes to the reusable platform GitHub service at background priority and uses a 10-minute safety backstop. A pure, fail-closed gate starts a model turn only for authorized maintainer or Copilot review feedback, failed required checks, conflicts, or a behind branch. Authorized unresolved inline threads include bounded body, file, line, author, and thread identity in the prompt so the agent can act without an unrestricted GitHub read surface. Pending checks do not start a turn. Repeated identical work and total autonomous repair attempts are bounded. Git-state refreshes immediately revalidate the bound branch, including after asynchronous PR attachment and merge preparation; a branch or pull-request identity change disables the controller and requires explicit re-enablement. +The controller subscribes to the reusable platform GitHub service at background priority and uses a 10-minute safety backstop. A pure, fail-closed gate starts a model turn only for authorized maintainer or Copilot review feedback, failed required checks, conflicts, or a behind branch. Authorized unresolved inline threads include every authorized comment with file, line, and author, bounded per comment and by one aggregate budget, so the agent sees the requested change without an unrestricted GitHub read surface and without an unbounded prompt. Pending checks do not start a turn. A pull request whose head repository provenance is unknown fails closed. Repeated identical work and total autonomous repair attempts are bounded. Git-state refreshes immediately revalidate the bound branch, and branch and pull-request identity are revalidated after every asynchronous step; a branch or pull-request identity change disables the controller and requires explicit re-enablement. The bound pull request's host must match the signed-in account, so the same owner/repository/number on another GitHub instance is never acted on. -Repair turns receive only pull-request-bound tools for failed CI details, attributed review-thread replies and resolution, and failed-workflow reruns. Those tools are advertised only while Agent Merge is enabled, so a host with the feature off exposes no Agent Merge surface to any provider. Pull-request feedback and CI content remain untrusted. The agent is never authorized to merge. The controller claims a turn only while no chat in the session has an active turn, performs a fresh readiness check, and then directly merges or enqueues through the GitHub service. +Repair turns receive only pull-request-bound tools for failed CI details, attributed review-thread replies and resolution, and failed-workflow reruns. Those tools are advertised only while Agent Merge is enabled, so a host with the feature off exposes no Agent Merge surface to any provider. Pull-request feedback and CI content remain untrusted. The agent is never authorized to merge. The controller claims a turn only while no chat in the session has an active turn, and cancels a repair turn it started when Agent Merge stops. Before merging it re-reads live enablement, configuration, and target, then directly merges or enqueues through the GitHub service. Merge preparation captures an authoritative snapshot of every fragment the gate reads and refreshes top-level comments last, so a merge cannot race newly posted maintainer feedback. -The Agents Window initially exposes command-palette actions only: enable, disable, and configure the active Agent Host session. Configure uses an accessible multi-select Quick Pick and includes reset-to-global-defaults behavior. +The Agents Window initially exposes command-palette actions only: enable, disable, and configure the active Agent Host session. Enable and disable are offered only when they apply to the active session. Configure uses an accessible multi-select Quick Pick whose title button resets the session to the global defaults, so resetting can never silently override a visible selection. Diagnostics use the `AgentMergeController`, `AgentMergeTools`, and `AgentMergeActions` log prefixes. Lifecycle and outcomes are logged at info/debug level, repeated evaluation details at trace level, and rejected or exhausted operations at warning/error level. Logs include session and turn identifiers plus counts and enum-like outcomes, but never pull-request comment bodies, CI log contents, prompt text, credentials, or local paths. diff --git a/src/vs/sessions/contrib/providers/agentHost/browser/agentMergeActions.ts b/src/vs/sessions/contrib/providers/agentHost/browser/agentMergeActions.ts index 5301e2221e1c09..e3accebddae8d7 100644 --- a/src/vs/sessions/contrib/providers/agentHost/browser/agentMergeActions.ts +++ b/src/vs/sessions/contrib/providers/agentHost/browser/agentMergeActions.ts @@ -4,14 +4,19 @@ *--------------------------------------------------------------------------------------------*/ import { localize, localize2 } from '../../../../../nls.js'; +import { Codicon } from '../../../../../base/common/codicons.js'; +import { Disposable, DisposableStore, MutableDisposable } from '../../../../../base/common/lifecycle.js'; +import { autorun } from '../../../../../base/common/observable.js'; +import { ThemeIcon } from '../../../../../base/common/themables.js'; import { Action2, registerAction2 } from '../../../../../platform/actions/common/actions.js'; import { AgentMergeAction, AgentMergeConfiguration, AgentMergeSessionOverrides, AgentMergeSettingId, defaultAgentMergeConfiguration, resolveAgentMergeConfiguration } from '../../../../../platform/agentHost/common/agentMerge.js'; import { IConfigurationService } from '../../../../../platform/configuration/common/configuration.js'; -import { ContextKeyExpr } from '../../../../../platform/contextkey/common/contextkey.js'; +import { ContextKeyExpr, IContextKeyService, RawContextKey } from '../../../../../platform/contextkey/common/contextkey.js'; import { ServicesAccessor } from '../../../../../platform/instantiation/common/instantiation.js'; import { INotificationService } from '../../../../../platform/notification/common/notification.js'; import { IQuickInputService, IQuickPickItem } from '../../../../../platform/quickinput/common/quickInput.js'; import { ILogService } from '../../../../../platform/log/common/log.js'; +import { IWorkbenchContribution, registerWorkbenchContribution2, WorkbenchPhase } from '../../../../../workbench/common/contributions.js'; import { IsSessionsWindowContext } from '../../../../../workbench/common/contextkeys.js'; import { ChatContextKeys } from '../../../../../workbench/contrib/chat/common/actions/chatContextKeys.js'; import { ANY_AGENT_HOST_PROVIDER_RE, isAgentHostProvider } from '../../../../common/agentHostSessionsProvider.js'; @@ -27,9 +32,49 @@ const agentMergeCommandPrecondition = ContextKeyExpr.and( ContextKeyExpr.equals(`config.${AgentMergeSettingId.Enabled}`, true), ); +/** Whether Agent Merge is currently enabled on the active session. */ +const AgentMergeSessionEnabledContext = new RawContextKey('sessionAgentMergeEnabled', false, { + type: 'boolean', + description: localize('sessionAgentMergeEnabled', "True when Agent Merge is enabled for the active agent session."), +}); + +/** + * Mirrors the active session's Agent Merge enablement into a context key so the + * command palette only offers the action that actually applies. + */ +class AgentMergeContextContribution extends Disposable implements IWorkbenchContribution { + + static readonly ID = 'sessions.contrib.agentMergeContext'; + + constructor( + @IContextKeyService contextKeyService: IContextKeyService, + @ISessionsService sessionsService: ISessionsService, + @ISessionsProvidersService sessionsProvidersService: ISessionsProvidersService, + ) { + super(); + const enabledKey = AgentMergeSessionEnabledContext.bindTo(contextKeyService); + const providerListener = this._register(new MutableDisposable()); + this._register(autorun(reader => { + const session = sessionsService.activeSession.read(reader); + const provider = session && sessionsProvidersService.getProvider(session.providerId); + const agentHostProvider = provider && isAgentHostProvider(provider) ? provider : undefined; + const update = () => enabledKey.set( + !!session && !!agentHostProvider && agentHostProvider.getAgentMergeSessionState(session.sessionId)?.enabled === true, + ); + providerListener.value = agentHostProvider?.onDidChangeSessionConfig(changed => { + if (changed === session?.sessionId) { + update(); + } + }); + update(); + })); + } +} + +registerWorkbenchContribution2(AgentMergeContextContribution.ID, AgentMergeContextContribution, WorkbenchPhase.AfterRestored); + interface IAgentMergeActionPick extends IQuickPickItem { - readonly action?: AgentMergeAction; - readonly reset?: boolean; + readonly action: AgentMergeAction; } abstract class AgentMergeActionBase extends Action2 { @@ -47,7 +92,7 @@ registerAction2(class EnableAgentMergeAction extends AgentMergeActionBase { id: 'sessions.agentHost.agentMerge.enable', title: localize2('agentMerge.enable', "Enable Agent Merge for Active Session"), f1: true, - precondition: agentMergeCommandPrecondition, + precondition: ContextKeyExpr.and(agentMergeCommandPrecondition, AgentMergeSessionEnabledContext.negate()), }); } @@ -70,7 +115,7 @@ registerAction2(class DisableAgentMergeAction extends AgentMergeActionBase { id: 'sessions.agentHost.agentMerge.disable', title: localize2('agentMerge.disable', "Disable Agent Merge for Active Session"), f1: true, - precondition: agentMergeCommandPrecondition, + precondition: ContextKeyExpr.and(agentMergeCommandPrecondition, AgentMergeSessionEnabledContext), }); } @@ -114,26 +159,56 @@ registerAction2(class ConfigureAgentMergeAction extends AgentMergeActionBase { { action: 'fixCI', label: localize('agentMerge.action.fixCI', "Fix CI Failures"), picked: effective.fixCI }, { action: 'resolveConflicts', label: localize('agentMerge.action.resolveConflicts', "Resolve Conflicts and Behind Branches"), picked: effective.resolveConflicts }, { action: 'mergePullRequest', label: localize('agentMerge.action.mergePullRequest', "Automatically Merge When Ready"), picked: effective.mergePullRequest }, - { reset: true, label: localize('agentMerge.action.reset', "Reset to Global Defaults"), description: localize('agentMerge.action.reset.description', "Remove all action overrides for this session") }, ]; - const selected = await quickInputService.pick(picks, { - canPickMany: true, - placeHolder: localize('agentMerge.action.select', "Select actions Agent Merge may perform for this session"), - }); - if (!selected) { + const result = await pickAgentMergeActions(quickInputService, picks); + if (!result) { return; } - const reset = selected.some(item => item.reset); - const selectedActions = new Set(selected.flatMap(item => item.action ? [item.action] : [])); - const overrides = reset ? undefined : toOverrides(selectedActions); + const overrides = result.reset ? undefined : toOverrides(result.actions); await active.provider.setAgentMergeOverrides(active.session.sessionId, overrides); - logService.info(`[AgentMergeActions] Action overrides updated: session=${active.session.sessionId}, provider=${active.session.providerId}, reset=${reset}, enabledActions=${[...selectedActions].sort().join(',') || 'none'}`); - notificationService.info(reset + logService.info(`[AgentMergeActions] Action overrides updated: session=${active.session.sessionId}, provider=${active.session.providerId}, reset=${result.reset}, enabledActions=${[...result.actions].sort().join(',') || 'none'}`); + notificationService.info(result.reset ? localize('agentMerge.action.reset.complete', "Agent Merge now follows the global action defaults for this session.") : localize('agentMerge.action.updated', "Agent Merge actions were updated for the active session.")); } }); +/** + * Selects the session's authorized actions. Reset is a title button rather than + * a pick so it can never silently override an explicit multi-selection. + */ +function pickAgentMergeActions( + quickInputService: IQuickInputService, + picks: readonly IAgentMergeActionPick[], +): Promise<{ readonly reset: boolean; readonly actions: ReadonlySet } | undefined> { + const store = new DisposableStore(); + return new Promise(resolve => { + const quickPick = store.add(quickInputService.createQuickPick()); + quickPick.title = localize('agentMerge.action.title', "Agent Merge"); + quickPick.placeholder = localize('agentMerge.action.select', "Select actions Agent Merge may perform for this session"); + quickPick.canSelectMany = true; + quickPick.items = picks; + quickPick.selectedItems = picks.filter(pick => pick.picked); + quickPick.buttons = [{ + iconClass: ThemeIcon.asClassName(Codicon.discard), + tooltip: localize('agentMerge.action.reset', "Reset to Global Defaults"), + }]; + store.add(quickPick.onDidTriggerButton(() => { + resolve({ reset: true, actions: new Set() }); + quickPick.hide(); + })); + store.add(quickPick.onDidAccept(() => { + resolve({ reset: false, actions: new Set(quickPick.selectedItems.map(item => item.action)) }); + quickPick.hide(); + })); + store.add(quickPick.onDidHide(() => { + resolve(undefined); + store.dispose(); + })); + quickPick.show(); + }); +} + function getGlobalConfiguration(configurationService: IConfigurationService): AgentMergeConfiguration { return { addressReviews: configurationService.getValue(AgentMergeSettingId.AddressReviews) ?? defaultAgentMergeConfiguration.addressReviews, diff --git a/src/vs/workbench/contrib/chat/browser/chat.shared.contribution.ts b/src/vs/workbench/contrib/chat/browser/chat.shared.contribution.ts index 5ffc957fc12c62..aa9d945f7c448e 100644 --- a/src/vs/workbench/contrib/chat/browser/chat.shared.contribution.ts +++ b/src/vs/workbench/contrib/chat/browser/chat.shared.contribution.ts @@ -18,6 +18,7 @@ import '../../../../platform/agentHost/common/agentHostEnablementService.js'; import { AgentHostMapLegacySettingsToManagedSettingsSettingId } from '../../../../platform/agentHost/common/agentHostManagedSettings.js'; import { AgentHostAutoReplyEnabledConfigKey, AgentHostEditAutoApprovePatternsConfigKey, AgentHostExternalSessionsMode, AgentHostGlobalAutoApproveEnabledConfigKey, AgentHostMigrateLegacyCopilotCliEnabledConfigKey, AgentHostSessionSyncEnabledConfigKey, AgentHostShowExternalSessionsConfigKey } from '../../../../platform/agentHost/common/agentHostSchema.js'; import '../../../../platform/agentHost/common/agentHostStarter.config.contribution.js'; +import { AgentMergeSettingId } from '../../../../platform/agentHost/common/agentMerge.js'; import { AgentHostAhpJsonlLoggingSettingId, AgentHostAllowSignedOutWhenUsableSettingId, AgentHostSdkSandboxEnabledSettingId, AgentHostSdkSandboxWindowsEnabledSettingId, CodexPreferAgentHostEditorSettingId } from '../../../../platform/agentHost/common/agentService.js'; import { AgentHostCopilotModelCapabilityOverridesSettingId, AgentHostCopilotSdkLogLevelSettingId, AgentHostCustomTerminalToolEnabledSettingId, AgentHostOpus48PromptEnabledSettingId, AgentHostReasoningEffortOverrideSettingId, AgentHostReasoningSummaryEnabledSettingId, AgentHostToolSearchDeferThresholdSettingId, AgentHostToolSearchEnabledSettingId, copilotSdkLogLevelSettingValues } from '../../../../platform/agentHost/common/copilotCliConfig.js'; import { DEFAULT_EDIT_AUTO_APPROVE_PATTERNS, mergeChatEditAutoApprovePatterns } from '../../../../platform/chat/common/chatSettings.js'; @@ -2445,6 +2446,24 @@ Registry.as(Extensions.ConfigurationMigration). [ChatConfiguration.PluginLocations, { value }] ]) }, + // Agent Merge settings dropped the `agentHost` segment from their ids. Without + // this an explicit opt-out (for example `fixCI: false`) would silently revert to + // the permissive default for sessions that already have Agent Merge enabled. + ...Object.values(AgentMergeSettingId).map(settingId => { + const legacyKey = settingId.replace(/^chat\./, 'chat.agentHost.'); + return { + key: legacyKey, + migrateFn: (value: unknown, accessor: (key: string) => unknown): ConfigurationKeyValuePairs => { + const pairs: ConfigurationKeyValuePairs = [[legacyKey, { value: undefined }]]; + // Never clobber an explicitly configured new key (e.g. after settings + // sync brought both keys across versions). + if (accessor(settingId) === undefined) { + pairs.push([settingId, { value }]); + } + return pairs; + } + }; + }), { // The on-device dictation runtime moved to Foundry Local; the old // transformers.js/onnxruntime model IDs no longer resolve and would fail From 165b64a77256f817048eaefa343af7e222c08e11 Mon Sep 17 00:00:00 2001 From: Lee Murray Date: Mon, 17 Aug 2026 13:40:39 +0100 Subject: [PATCH 3/6] Modern UI: Update surface border color to use opaque defaults and add corresponding tests (#331177) Update surface border color to use opaque defaults and add corresponding tests Co-authored-by: mrleemurray --- src/vs/workbench/common/theme.ts | 4 +-- .../styleOverrides.contribution.test.ts | 25 +++++++++++++++++++ 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/src/vs/workbench/common/theme.ts b/src/vs/workbench/common/theme.ts index 98d436e872a3a6..3667118f25146c 100644 --- a/src/vs/workbench/common/theme.ts +++ b/src/vs/workbench/common/theme.ts @@ -676,8 +676,8 @@ export const SURFACE_BACKGROUND = registerColor('surface.background', { export const SURFACE_FOREGROUND = registerColor('surface.foreground', SIDE_BAR_FOREGROUND, localize('surfaceForeground', "Foreground color of framed container surfaces (\"cards\"), such as the floating workbench panels in the modern layout.")); export const SURFACE_BORDER = registerColor('surface.border', { - dark: transparent(foreground, 0.1), - light: transparent(foreground, 0.1), + dark: opaque(transparent(foreground, 0.1), SURFACE_BACKGROUND), + light: opaque(transparent(foreground, 0.1), SURFACE_BACKGROUND), hcDark: contrastBorder, hcLight: contrastBorder }, localize('surfaceBorder', "Border color of framed container surfaces (\"cards\"), such as the floating workbench panels in the modern layout.")); diff --git a/src/vs/workbench/contrib/styleOverrides/test/browser/styleOverrides.contribution.test.ts b/src/vs/workbench/contrib/styleOverrides/test/browser/styleOverrides.contribution.test.ts index 2c3a66c0c485a4..310198f7ee3556 100644 --- a/src/vs/workbench/contrib/styleOverrides/test/browser/styleOverrides.contribution.test.ts +++ b/src/vs/workbench/contrib/styleOverrides/test/browser/styleOverrides.contribution.test.ts @@ -425,6 +425,31 @@ suite('StyleOverridesContribution', () => { }); }); + test('uses opaque surface border defaults', () => { + const darkTheme = ColorThemeData.createUnloadedTheme('vs-dark'); + const lightTheme = ColorThemeData.createUnloadedTheme('vs'); + const darkSurfaceBorder = darkTheme.getColor(SURFACE_BORDER); + const darkEditorBorder = darkTheme.getColor(EDITOR_BORDER); + const lightSurfaceBorder = lightTheme.getColor(SURFACE_BORDER); + const lightEditorBorder = lightTheme.getColor(EDITOR_BORDER); + + assert.deepStrictEqual({ + darkSurfaceBorderIsOpaque: darkSurfaceBorder?.isOpaque(), + darkEditorBorderIsOpaque: darkEditorBorder?.isOpaque(), + darkEditorBorderMatchesSurface: darkEditorBorder?.equals(darkSurfaceBorder ?? null), + lightSurfaceBorderIsOpaque: lightSurfaceBorder?.isOpaque(), + lightEditorBorderIsOpaque: lightEditorBorder?.isOpaque(), + lightEditorBorderMatchesSurface: lightEditorBorder?.equals(lightSurfaceBorder ?? null), + }, { + darkSurfaceBorderIsOpaque: true, + darkEditorBorderIsOpaque: true, + darkEditorBorderMatchesSurface: true, + lightSurfaceBorderIsOpaque: true, + lightEditorBorderIsOpaque: true, + lightEditorBorderMatchesSurface: true, + }); + }); + test('hides collapsed primary side bar grips without hiding constrained auxiliary sash grips', () => { const root = document.createElement('div'); root.className = 'monaco-workbench style-override nosidebar nopanel'; From e0d62973af9cbe86ee5d99419c6ad384d489b207 Mon Sep 17 00:00:00 2001 From: Benjamin Christopher Simmonds <44439583+benibenj@users.noreply.github.com> Date: Mon, 17 Aug 2026 15:11:50 +0200 Subject: [PATCH 4/6] agentHost: Filter external Copilot sessions (#331187) * agentHost: Filter external Copilot sessions Only discover standalone Copilot CLI and GitHub Copilot app sessions that have repository metadata and were modified within the last seven days. Preserve legacy extension-host adoption and cover accepted and rejected metadata boundaries. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * agentHost: Fix Copilot discovery gating test Gate the raw session-list RPC used by external discovery so the migration-toggle test no longer waits on the obsolete convenience listSessions seam. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * agentHost: Format Copilot discovery test Apply the repository TypeScript formatter to the raw session-list test fixture. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../agentHost/node/copilot/copilotAgent.ts | 49 +++-- .../agentHost/test/node/copilotAgent.test.ts | 167 +++++++++++++++--- .../agentHost/AGENT_HOST_SESSIONS_PROVIDER.md | 2 + 3 files changed, 185 insertions(+), 33 deletions(-) diff --git a/src/vs/platform/agentHost/node/copilot/copilotAgent.ts b/src/vs/platform/agentHost/node/copilot/copilotAgent.ts index cb864567209e05..6dd14f82446da1 100644 --- a/src/vs/platform/agentHost/node/copilot/copilotAgent.ts +++ b/src/vs/platform/agentHost/node/copilot/copilotAgent.ts @@ -536,6 +536,8 @@ export function resolveCopilotOtlpMetricsEndpoint(endpoint: string, protocol: 'h /** `origin` value written by the VS Code extension-host Copilot CLI feature. */ const EXTENSION_HOST_CLI_MARKER_ORIGIN = 'vscode'; +const COPILOT_EXTERNAL_SESSION_CLIENT_NAMES = new Set(['github/cli', 'github/autopilot']); +const COPILOT_EXTERNAL_SESSION_MAX_AGE_MS = 7 * 24 * 60 * 60 * 1000; /** * Shape of the `vscode.metadata.json` marker written next to a Copilot CLI @@ -570,6 +572,7 @@ const NANO_AIU_PER_CREDIT = 1_000_000_000; */ export class CopilotAgent extends Disposable implements IAgent { readonly id = 'copilotcli' as const; + protected readonly _now = Date.now; private readonly _onDidChatProgress = this._register(new Emitter()); readonly onDidChatProgress = this._onDidChatProgress.event; @@ -2088,7 +2091,7 @@ export class CopilotAgent extends Disposable implements IAgent { } async listChatsToMigrate(): Promise { - const sessions = await this._listSdkSessions('chats to migrate'); + const sessions = await this._listSdkSessions('chats to migrate', client => client.listSessions()); if (!sessions) { return undefined; } @@ -2192,9 +2195,11 @@ export class CopilotAgent extends Disposable implements IAgent { * * - a legacy extension-host Copilot CLI chat is *internal* and adoptable in * place (see {@link ensureChatAdopted}), so it keeps `external: false`; - * - anything else was produced by another client sharing the same Copilot - * home (the standalone CLI, the GitHub Copilot app, another editor) and - * is therefore `external: true`. + * - a non-adoptable chat is external only when its persisted `clientName` + * identifies the standalone CLI or GitHub Copilot app. This value records + * the runtime client that created or last resumed the chat, not immutable + * creator provenance. External chats must also have repository metadata + * and have been modified within the last seven days. * * A chat counts as already known when it has a per-session database, which * also keeps peer-chat backings out of the result. A chat the SDK reports @@ -2211,15 +2216,19 @@ export class CopilotAgent extends Disposable implements IAgent { * authoritative empty result. */ private async _discoverCopilotChats(): Promise { - const sessions = await this._listSdkSessions('discoverable chats'); + const sessions = await this._listSdkSessions('discoverable chats', async client => (await client.rpc.sessions.list({})).sessions); if (!sessions) { return undefined; } const projectLimiter = new Limiter(4); const metadataLimiter = new Limiter(4); const projectByContext = new Map>(); + const earliestExternalModifiedTime = this._now() - COPILOT_EXTERNAL_SESSION_MAX_AGE_MS; let known = 0; let withoutWorkingDirectory = 0; + let unsupportedClientName = 0; + let outsideImportWindow = 0; + let withoutRepository = 0; let failed = 0; const mapped = await Promise.all(sessions.map(s => metadataLimiter.queue(async () => { const session = AgentSession.uri(this.id, s.sessionId); @@ -2228,18 +2237,34 @@ export class CopilotAgent extends Disposable implements IAgent { known++; return undefined; } - if (typeof s.context?.workingDirectory !== 'string') { + if (typeof s.context?.cwd !== 'string') { withoutWorkingDirectory++; return undefined; } const adoptable = await this._isExtensionHostCliSession(s.sessionId); + const modifiedTime = new Date(s.modifiedTime).getTime(); + if (!adoptable) { + const clientName = s.isRemote ? undefined : s.clientName; + if (clientName === undefined || !COPILOT_EXTERNAL_SESSION_CLIENT_NAMES.has(clientName)) { + unsupportedClientName++; + return undefined; + } + if (!Number.isFinite(modifiedTime) || modifiedTime < earliestExternalModifiedTime) { + outsideImportWindow++; + return undefined; + } + if (typeof s.context.repository !== 'string' || s.context.repository.trim().length === 0) { + withoutRepository++; + return undefined; + } + } return { chat: URI.parse(buildDefaultChatUri(session)), - startTime: s.startTime.getTime(), - modifiedTime: s.modifiedTime.getTime(), + startTime: new Date(s.startTime).getTime(), + modifiedTime, project: await this._resolveSessionProject(s.context, projectLimiter, projectByContext), summary: s.summary, - workingDirectories: [URI.file(s.context.workingDirectory)], + workingDirectories: [URI.file(s.context.cwd)], _meta: adoptable ? withSessionEhcliAdoptable(undefined) : undefined, external: !adoptable, } satisfies IAgentDiscoveredChat; @@ -2251,14 +2276,14 @@ export class CopilotAgent extends Disposable implements IAgent { }))); const chats = mapped.filter((chat): chat is IAgentDiscoveredChat => chat !== undefined); const external = chats.filter(chat => chat.external).length; - this._logService.info(`[Copilot] Chat discovery: ${sessions.length} SDK session(s) -> ${external} external, ${chats.length - external} adoptable legacy extension-host, ${known} already known to Agent Host, ${withoutWorkingDirectory} without a working directory, ${failed} failed to classify`); + this._logService.info(`[Copilot] Chat discovery: ${sessions.length} SDK session(s) -> ${external} external, ${chats.length - external} adoptable legacy extension-host, ${known} already known to Agent Host, ${withoutWorkingDirectory} without a working directory, ${unsupportedClientName} with unsupported or missing client name, ${outsideImportWindow} outside the import window, ${withoutRepository} without repository metadata, ${failed} failed to classify`); return chats; } - private async _listSdkSessions(reason: string): Promise> | undefined> { + private async _listSdkSessions(reason: string, listSessions: (client: CopilotClient) => Promise): Promise { this._logService.info(`[Copilot] Listing ${reason}...`); try { - const sessions = await this._retryAfterClosedConnection('listSessions', client => client.listSessions()); + const sessions = await this._retryAfterClosedConnection('listSessions', listSessions); this._logService.info(`[Copilot] Listed ${sessions.length} SDK session(s) for ${reason}`); return sessions; } catch (err) { diff --git a/src/vs/platform/agentHost/test/node/copilotAgent.test.ts b/src/vs/platform/agentHost/test/node/copilotAgent.test.ts index 1b0d5e6422403c..88aa23c0c5684b 100644 --- a/src/vs/platform/agentHost/test/node/copilotAgent.test.ts +++ b/src/vs/platform/agentHost/test/node/copilotAgent.test.ts @@ -432,11 +432,22 @@ interface ITestCopilotModelInfo { interface ITestCopilotClient extends Pick { readonly rpc: { - readonly sessions: { readonly fork: CopilotClient['rpc']['sessions']['fork'] }; + readonly sessions: { + readonly fork: CopilotClient['rpc']['sessions']['fork']; + readonly list: CopilotClient['rpc']['sessions']['list']; + }; readonly models: { readonly list: CopilotModelsList }; }; } +type TestCopilotSessionMetadata = Awaited>[number] & { readonly clientName?: string }; + +interface ITestCopilotSessionOptions { + readonly clientName?: string; + readonly repository?: string; + readonly modifiedTime?: Date; +} + function toSdkModelInfo(model: ITestCopilotModelInfo): CopilotModelInfo { return { id: model.id, @@ -462,7 +473,31 @@ function toSdkModelInfo(model: ITestCopilotModelInfo): CopilotModelInfo { class TestCopilotClient implements ITestCopilotClient { readonly rpc: ITestCopilotClient['rpc'] = { - sessions: { fork: async () => ({ sessionId: 'forked-session' }) }, + sessions: { + fork: async () => ({ sessionId: 'forked-session' }), + list: async () => { + this.sessionListStarted?.complete(); + await this.sessionListGate; + return { + sessions: this._sessions.map(session => ({ + sessionId: session.sessionId, + startTime: session.startTime.toISOString(), + modifiedTime: session.modifiedTime.toISOString(), + summary: session.summary, + clientName: session.clientName, + isRemote: false, + ...(session.context ? { + context: { + cwd: session.context.workingDirectory, + gitRoot: session.context.gitRoot, + repository: session.context.repository, + branch: session.context.branch, + } + } : {}), + })) + }; + }, + }, models: { list: async params => { this.modelListRequests.push(params); @@ -483,6 +518,8 @@ class TestCopilotClient implements ITestCopilotClient { startGate: Promise | undefined; startError: Error | undefined; listSessionCallCount = 0; + sessionListStarted: DeferredPromise | undefined; + sessionListGate: Promise | undefined; readonly modelListRequests: Parameters[0][] = []; readonly modelListErrors: Error[] = []; /** When set, `models.list` records its request then blocks on this until resolved. */ @@ -494,7 +531,7 @@ class TestCopilotClient implements ITestCopilotClient { readonly deletedSessionIds: string[] = []; constructor( - private readonly _sessions: Awaited>, + private readonly _sessions: TestCopilotSessionMetadata[], private readonly _models: readonly ITestCopilotModelInfo[] = [], ) { } @@ -734,6 +771,7 @@ class TestableCopilotAgent extends CopilotAgent { readonly resumeCalls: string[] = []; readonly createdClientOptions: CopilotClientOptions[] = []; lastClientOptions: CopilotClientOptions | undefined; + protected override readonly _now: () => number; // Keep model-refresh retries effectively instant in tests. protected override readonly _modelRefreshBaseDelayMs = 1; @@ -741,6 +779,7 @@ class TestableCopilotAgent extends CopilotAgent { constructor( private readonly _copilotClient: ITestCopilotClient, + now: () => number, @ILogService logService: ILogService, @IInstantiationService instantiationService: IInstantiationService, @ISessionDataService sessionDataService: ISessionDataService, @@ -759,6 +798,7 @@ class TestableCopilotAgent extends CopilotAgent { @ICopilotApiService copilotApiService: ICopilotApiService, ) { super(logService, instantiationService, sessionDataService, gitService, configurationService, sessionTitleSignal, managedSettingsService, gitHubEndpointService, otelService, completions, NULL_CHECKPOINT_SERVICE, NULL_REVIEW_SERVICE, customizationEnablementService, environmentService, byokBridgeRegistry, telemetryService, copilotApiService, proxyResolver); + this._now = now; } protected override _createCopilotClient(options: CopilotClientOptions): CopilotClient { @@ -812,7 +852,7 @@ function getCreatedClientOptions(agent: CopilotAgent): readonly CopilotClientOpt return agent.createdClientOptions; } -function createTestAgentContext(disposables: Pick, options?: { sessionDataService?: ISessionDataService; copilotClient?: ITestCopilotClient; useRealResumePath?: boolean; gitService?: TestAgentHostGitService; environmentServiceRegistration?: 'native' | 'none'; pluginManager?: IAgentPluginManager; fileService?: FileService; copilotApiService?: ICopilotApiService; gitHubEndpointService?: IAgentHostGitHubEndpointService; telemetryService?: ITelemetryService; userHome?: URI; logService?: ILogService; proxyResolver?: IAgentHostProxyResolver; byokBridgeRegistry?: IByokLmBridgeRegistry; otelService?: IAgentHostOTelService; rootConfig?: Record }): { agent: CopilotAgent; instantiationService: IInstantiationService; configurationService: IAgentConfigurationService; managedSettingsService: IAgentHostManagedSettingsService; fileService: FileService; stateManager: AgentHostStateManager } { +function createTestAgentContext(disposables: Pick, options?: { sessionDataService?: ISessionDataService; copilotClient?: ITestCopilotClient; useRealResumePath?: boolean; gitService?: TestAgentHostGitService; environmentServiceRegistration?: 'native' | 'none'; pluginManager?: IAgentPluginManager; fileService?: FileService; copilotApiService?: ICopilotApiService; gitHubEndpointService?: IAgentHostGitHubEndpointService; telemetryService?: ITelemetryService; userHome?: URI; logService?: ILogService; proxyResolver?: IAgentHostProxyResolver; byokBridgeRegistry?: IByokLmBridgeRegistry; otelService?: IAgentHostOTelService; rootConfig?: Record; now?: () => number }): { agent: CopilotAgent; instantiationService: IInstantiationService; configurationService: IAgentConfigurationService; managedSettingsService: IAgentHostManagedSettingsService; fileService: FileService; stateManager: AgentHostStateManager } { const services = new ServiceCollection(); const logService = options?.logService ?? new NullLogService(); const fileService = options?.fileService ?? disposables.add(new FileService(logService)); @@ -879,7 +919,9 @@ function createTestAgentContext(disposables: Pick, optio const instantiationService: IInstantiationService = disposables.add(new InstantiationService(services)); services.set(IInstantiationService, instantiationService); const agent = options?.copilotClient - ? instantiationService.createInstance(options.useRealResumePath ? ResumePathCopilotAgent : TestableCopilotAgent, options.copilotClient) + ? options.useRealResumePath + ? instantiationService.createInstance(ResumePathCopilotAgent, options.copilotClient) + : instantiationService.createInstance(TestableCopilotAgent, options.copilotClient, options.now ?? Date.now) : instantiationService.createInstance(CopilotAgent); return { agent, instantiationService, configurationService: configService, managedSettingsService, fileService, stateManager }; } @@ -934,14 +976,20 @@ function withoutUndefinedProperties(metadata: IAgentChatMetadata): Record>[number] { +function sdkSession(sessionId: string, cwd?: string, options?: ITestCopilotSessionOptions): TestCopilotSessionMetadata { return { sessionId, startTime: new Date(1000), - modifiedTime: new Date(2000), + modifiedTime: options?.modifiedTime ?? new Date(2000), summary: `SDK ${sessionId}`, isRemote: false, - ...(cwd ? { context: { workingDirectory: cwd } } : {}), + ...(cwd ? { + context: { + workingDirectory: cwd, + ...(options?.repository !== undefined ? { repository: options.repository } : {}), + } + } : {}), + ...(options?.clientName !== undefined ? { clientName: options.clientName } : {}), }; } @@ -4863,14 +4911,9 @@ suite('CopilotAgent', () => { const sessionId = 'disabled-during-migration-event'; const listStarted = new DeferredPromise(); const releaseList = new DeferredPromise(); - class GatedListClient extends TestCopilotClient { - override async listSessions(): ReturnType { - listStarted.complete(); - await releaseList.p; - return super.listSessions(); - } - } - const client = new GatedListClient([sdkSession(sessionId, workingDirectory)]); + const client = new TestCopilotClient([sdkSession(sessionId, workingDirectory)]); + client.sessionListStarted = listStarted; + client.sessionListGate = releaseList.p; await writeExtensionHostMarker(userHome, sessionId); const { agent, configurationService } = createTestAgentContext(disposables, { copilotClient: client, @@ -5063,11 +5106,15 @@ suite('CopilotAgent', () => { suite('external chat discovery', () => { - test('surfaces an SDK session created by another Copilot client as external', async () => { + test('surfaces a standalone Copilot CLI SDK session as external', async () => { const userHome = URI.file(await fs.mkdtemp(`${os.tmpdir()}/external-discovery-home-`)); const workingDirectory = await fs.mkdtemp(`${os.tmpdir()}/external-discovery-cwd-`); const sessionDataService = disposables.add(new TestSessionDataService()); - const client = new TestCopilotClient([sdkSession('external-cli', workingDirectory)]); + const client = new TestCopilotClient([sdkSession('external-cli', workingDirectory, { + clientName: 'github/cli', + repository: 'owner/repository', + modifiedTime: new Date(), + })]); // Migration stays off: external discovery must not depend on it. const { agent } = createTestAgentContext(disposables, { sessionDataService, copilotClient: client, userHome }); try { @@ -5085,7 +5132,11 @@ suite('CopilotAgent', () => { const userHome = URI.file(await fs.mkdtemp(`${os.tmpdir()}/external-origin-home-`)); const workingDirectory = await fs.mkdtemp(`${os.tmpdir()}/external-origin-cwd-`); const sessionDataService = disposables.add(new TestSessionDataService()); - const client = new TestCopilotClient([sdkSession('other-origin', workingDirectory)]); + const client = new TestCopilotClient([sdkSession('other-origin', workingDirectory, { + clientName: 'github/autopilot', + repository: 'owner/repository', + modifiedTime: new Date(), + })]); const { agent } = createTestAgentContext(disposables, { sessionDataService, copilotClient: client, userHome }); try { // The GitHub Copilot app writes the same sidecar with `origin: 'other'`. @@ -5101,6 +5152,80 @@ suite('CopilotAgent', () => { } }); + test('does not surface SDK sessions with an unknown or missing client name', async () => { + const userHome = URI.file(await fs.mkdtemp(`${os.tmpdir()}/unsupported-client-discovery-home-`)); + const workingDirectory = await fs.mkdtemp(`${os.tmpdir()}/unsupported-client-discovery-cwd-`); + const sessionDataService = disposables.add(new TestSessionDataService()); + const client = new TestCopilotClient([ + sdkSession('unknown-client', workingDirectory, { clientName: 'other/client', repository: 'owner/repository', modifiedTime: new Date() }), + sdkSession('missing-client', workingDirectory, { repository: 'owner/repository', modifiedTime: new Date() }), + ]); + const { agent } = createTestAgentContext(disposables, { sessionDataService, copilotClient: client, userHome }); + try { + assert.deepStrictEqual(await collectDiscoveredChats(agent), []); + } finally { + await fs.rm(userHome.fsPath, { recursive: true, force: true }); + await fs.rm(workingDirectory, { recursive: true, force: true }); + await disposeAgent(agent); + } + }); + + test('surfaces only sessions modified within the seven-day boundary', async () => { + const userHome = URI.file(await fs.mkdtemp(`${os.tmpdir()}/age-boundary-discovery-home-`)); + const workingDirectory = await fs.mkdtemp(`${os.tmpdir()}/age-boundary-discovery-cwd-`); + const sessionDataService = disposables.add(new TestSessionDataService()); + const now = Date.UTC(2026, 7, 17, 12); + const sevenDaysAgo = now - 7 * 24 * 60 * 60 * 1000; + const client = new TestCopilotClient([ + sdkSession('at-boundary', workingDirectory, { clientName: 'github/cli', repository: 'owner/repository', modifiedTime: new Date(sevenDaysAgo) }), + sdkSession('outside-boundary', workingDirectory, { clientName: 'github/cli', repository: 'owner/repository', modifiedTime: new Date(sevenDaysAgo - 1) }), + ]); + const { agent } = createTestAgentContext(disposables, { sessionDataService, copilotClient: client, userHome, now: () => now }); + try { + assert.deepStrictEqual(await collectDiscoveredChats(agent), [ + { id: 'at-boundary', external: true, adoptable: false }, + ]); + } finally { + await fs.rm(userHome.fsPath, { recursive: true, force: true }); + await fs.rm(workingDirectory, { recursive: true, force: true }); + await disposeAgent(agent); + } + }); + + test('does not surface a session with missing repository metadata', async () => { + const userHome = URI.file(await fs.mkdtemp(`${os.tmpdir()}/missing-repository-discovery-home-`)); + const workingDirectory = await fs.mkdtemp(`${os.tmpdir()}/missing-repository-discovery-cwd-`); + const sessionDataService = disposables.add(new TestSessionDataService()); + const client = new TestCopilotClient([ + sdkSession('missing-repository', workingDirectory, { clientName: 'github/cli', modifiedTime: new Date() }), + ]); + const { agent } = createTestAgentContext(disposables, { sessionDataService, copilotClient: client, userHome }); + try { + assert.deepStrictEqual(await collectDiscoveredChats(agent), []); + } finally { + await fs.rm(userHome.fsPath, { recursive: true, force: true }); + await fs.rm(workingDirectory, { recursive: true, force: true }); + await disposeAgent(agent); + } + }); + + test('does not surface a repository-less session', async () => { + const userHome = URI.file(await fs.mkdtemp(`${os.tmpdir()}/repository-less-discovery-home-`)); + const workingDirectory = await fs.mkdtemp(`${os.tmpdir()}/repository-less-discovery-cwd-`); + const sessionDataService = disposables.add(new TestSessionDataService()); + const client = new TestCopilotClient([ + sdkSession('repository-less', workingDirectory, { clientName: 'github/autopilot', repository: '', modifiedTime: new Date() }), + ]); + const { agent } = createTestAgentContext(disposables, { sessionDataService, copilotClient: client, userHome }); + try { + assert.deepStrictEqual(await collectDiscoveredChats(agent), []); + } finally { + await fs.rm(userHome.fsPath, { recursive: true, force: true }); + await fs.rm(workingDirectory, { recursive: true, force: true }); + await disposeAgent(agent); + } + }); + test('keeps a legacy extension-host chat internal and adoptable', async () => { const userHome = URI.file(await fs.mkdtemp(`${os.tmpdir()}/adoptable-discovery-home-`)); const workingDirectory = await fs.mkdtemp(`${os.tmpdir()}/adoptable-discovery-cwd-`); @@ -5158,8 +5283,8 @@ suite('CopilotAgent', () => { } const sessionDataService = disposables.add(new FailingSessionDataService()); const client = new TestCopilotClient([ - sdkSession('corrupt', workingDirectory), - sdkSession('healthy', workingDirectory), + sdkSession('corrupt', workingDirectory, { clientName: 'github/cli', repository: 'owner/repository', modifiedTime: new Date() }), + sdkSession('healthy', workingDirectory, { clientName: 'github/cli', repository: 'owner/repository', modifiedTime: new Date() }), ]); const { agent } = createTestAgentContext(disposables, { sessionDataService, copilotClient: client, userHome }); try { diff --git a/src/vs/sessions/contrib/providers/agentHost/AGENT_HOST_SESSIONS_PROVIDER.md b/src/vs/sessions/contrib/providers/agentHost/AGENT_HOST_SESSIONS_PROVIDER.md index b161b658b545bf..afdd90f7ff09c1 100644 --- a/src/vs/sessions/contrib/providers/agentHost/AGENT_HOST_SESSIONS_PROVIDER.md +++ b/src/vs/sessions/contrib/providers/agentHost/AGENT_HOST_SESSIONS_PROVIDER.md @@ -137,6 +137,8 @@ The **only** per-provider difference is the storage key: local uses the fixed `l Provider-native sessions discovered outside the Agent Host carry `_meta.external`. `chat.agentSessions.showExternal` controls whether the catalog publishes none, all, the last 24 hours, or the last 7 days (the default). Configuration changes publish or unpublish matching summaries immediately, including restored sessions, while retaining live Agent Host state so a later settings change can surface them again. The state manager distinguishes a summary retained as the diff baseline from one actually published through `root/sessionAdded`; restoring a filtered session records the former without implying the latter, and hidden summary changes advance that baseline without emitting root notifications. +Copilot discovery includes external SDK sessions only when their persisted `clientName` is exactly `github/cli` or `github/autopilot`, their persisted context includes non-empty repository metadata, and they were modified within the last seven days. Unknown and missing client names, repository-less sessions, and older sessions are excluded. `clientName` identifies the runtime client that created or last resumed the session, not immutable creator provenance. + Both the regular VS Code agent sessions list and the Agents Window Sessions list expose this setting as an `External` submenu directly below their provider filters. The checked option follows the effective configuration value, and selecting an option writes the user setting. The first external session opened in the Agents window shows a profile-scoped, one-time banner at the top of the chat. Its picker deliberately starts on a disabled placeholder rather than the effective seven-day default. Saving updates the setting; saving or closing records dismissal in profile storage. A setting that excludes the open session requires confirmation before the update is applied. From 7ee8d7166be54ef580b425c7b5d82fd8ea8a743d Mon Sep 17 00:00:00 2001 From: Benjamin Christopher Simmonds <44439583+benibenj@users.noreply.github.com> Date: Mon, 17 Aug 2026 15:12:48 +0200 Subject: [PATCH 5/6] Add recent external sessions filter (#331181) * Add recent external sessions filter Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address recent session review feedback Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Update session lifecycle test for external default Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../agentHost/common/agentHostSchema.ts | 4 +- .../agentHost/node/agentHostStateManager.ts | 5 +- .../platform/agentHost/node/agentService.ts | 75 +++++++- .../agentHost/test/node/agentService.test.ts | 172 +++++++++++++++++- .../sessionLifecycle.integrationTest.ts | 19 ++ src/vs/platform/chat/common/chatSettings.ts | 1 + .../chat/browser/externalSessionBanner.ts | 28 ++- .../browser/externalSessionBanner.test.ts | 30 ++- .../externalSessionsFilterMenu.ts | 1 + .../chat/browser/chat.shared.contribution.ts | 9 +- .../externalSessionsFilterMenu.test.ts | 13 +- 11 files changed, 316 insertions(+), 41 deletions(-) diff --git a/src/vs/platform/agentHost/common/agentHostSchema.ts b/src/vs/platform/agentHost/common/agentHostSchema.ts index 5f680626548e73..ad13e7f3091696 100644 --- a/src/vs/platform/agentHost/common/agentHostSchema.ts +++ b/src/vs/platform/agentHost/common/agentHostSchema.ts @@ -759,8 +759,8 @@ export const platformRootSchema = createSchema({ type: 'string', title: localize('agentHost.config.showExternalSessions.title', "Show External Agent Sessions"), description: localize('agentHost.config.showExternalSessions.description', "Controls whether sessions created outside the Agent Host are included in the session catalog."), - enum: [ChatExternalSessionsMode.None, ChatExternalSessionsMode.All, ChatExternalSessionsMode.Last24Hours, ChatExternalSessionsMode.Last7Days], - default: ChatExternalSessionsMode.Last7Days, + enum: [ChatExternalSessionsMode.None, ChatExternalSessionsMode.Recent, ChatExternalSessionsMode.Last24Hours, ChatExternalSessionsMode.Last7Days, ChatExternalSessionsMode.All], + default: ChatExternalSessionsMode.None, }), [AgentHostCopilotMultiRootEnabledConfigKey]: schemaProperty({ type: 'boolean', diff --git a/src/vs/platform/agentHost/node/agentHostStateManager.ts b/src/vs/platform/agentHost/node/agentHostStateManager.ts index 10febffd02a753..5cdcaa9cd30712 100644 --- a/src/vs/platform/agentHost/node/agentHostStateManager.ts +++ b/src/vs/platform/agentHost/node/agentHostStateManager.ts @@ -10,7 +10,7 @@ import { equals } from '../../../base/common/objects.js'; import { ILogService } from '../../log/common/log.js'; import { createDecorator } from '../../instantiation/common/instantiation.js'; import { TelemetryLevel } from '../../telemetry/common/telemetry.js'; -import { ActionType, ActionEnvelope, ActionOrigin, INotification, IRootConfigChangedAction, SessionAction, ChatAction, RootAction, StateAction, TerminalAction, ChangesetAction, ClientChangesetAction, AnnotationsAction, ClientAnnotationsAction, isRootAction, isSessionAction, isChatAction, isChangesetAction, isAnnotationsAction, type AuthRequiredParams, type ProgressParams } from '../common/state/sessionActions.js'; +import { ActionType, ActionEnvelope, ActionOrigin, INotification, IRootConfigChangedAction, SessionAction, ChatAction, RootAction, StateAction, TerminalAction, ChangesetAction, ClientChangesetAction, AnnotationsAction, ClientAnnotationsAction, isRootAction, isSessionAction, isChatAction, isChangesetAction, isAnnotationsAction, type AuthRequiredParams, type ProgressParams, type SessionSummaryChangedParams } from '../common/state/sessionActions.js'; import type { IStateSnapshot } from '../common/state/sessionProtocol.js'; import { rootReducer, sessionReducer, chatReducer, changesetReducer, annotationsReducer } from '../common/state/sessionReducers.js'; import { createRootState, createSessionState, createChatState, createDefaultChatSummary, chatSummaryFromState, buildDefaultChatUri, parseDefaultChatUri, parseRequiredSessionUriFromChatUri, parseSubagentSessionUri, isAhpChatChannel, isDefaultChatUri, mergeSessionWithDefaultChat, isAhpRootChannel, SessionLifecycle, withHostBuildInfo, type Changeset, type ChangesetState, type AnnotationsState, type ChatState, type ChatSummary, type Customization, type ISessionWithDefaultChat, type Message, type RootState, type SessionConfigState, type SessionMeta, type SessionState, type SessionSummary, type Turn, type URI, ROOT_STATE_URI, ChangesetStatus, IHostBuildInfo, SessionStatus } from '../common/state/sessionState.js'; @@ -280,6 +280,8 @@ export class AgentHostStateManager extends Disposable { private readonly _onDidChangeSessionWorkingDirectories = this._register(new Emitter<{ session: string }>()); readonly onDidChangeSessionWorkingDirectories: Event<{ session: string }> = this._onDidChangeSessionWorkingDirectories.event; + private readonly _onDidChangeSessionSummary = this._register(new Emitter<{ session: string; changes: SessionSummaryChangedParams['changes'] }>()); + readonly onDidChangeSessionSummary: Event<{ session: string; changes: SessionSummaryChangedParams['changes'] }> = this._onDidChangeSessionSummary.event; constructor( @ILogService private readonly _logService: ILogService, @@ -309,6 +311,7 @@ export class AgentHostStateManager extends Disposable { return entry ? this._toSummary(session, entry) : undefined; }, (session, changes) => { + this._onDidChangeSessionSummary.fire({ session, changes }); if (this._publishedSessionSummaries.has(session)) { this._onDidEmitNotification.fire({ type: 'root/sessionSummaryChanged', diff --git a/src/vs/platform/agentHost/node/agentService.ts b/src/vs/platform/agentHost/node/agentService.ts index c0cbb1ceb7ea4f..bbb5839ece1acf 100644 --- a/src/vs/platform/agentHost/node/agentService.ts +++ b/src/vs/platform/agentHost/node/agentService.ts @@ -117,6 +117,8 @@ import { AgentHostCheckpointService } from './agentHostCheckpointService.js'; * provider-side session, worktree, and on-disk state. */ const SESSION_GC_GRACE_MS = 30_000; +const DAY_MS = 24 * 60 * 60 * 1000; +const RECENT_EXTERNAL_SESSION_LIMIT = 2; type AgentHostLegacyMigrationEvent = { provider: string; @@ -576,6 +578,15 @@ export class AgentService extends Disposable implements IAgentService { this._register(this._stateManager.onDidEmitEnvelope(e => this._trackPendingSubagentChatFromEnvelope(e))); this._register(this._stateManager.onDidEmitEnvelope(e => this._persistAnnotations(e))); this._register(this._stateManager.onDidEmitNotification(e => this._onDidNotification.fire(e))); + this._register(this._stateManager.onDidChangeSessionSummary(({ session, changes }) => { + const meta = this._stateManager.getSessionSummary(session)?._meta; + if (changes.modifiedAt !== undefined + && this._getExternalSessionsMode() === AgentHostExternalSessionsMode.Recent + && readSessionExternal(meta) + && !readSessionEhcliAdoptable(meta)) { + this._queueSessionListReconciliation(); + } + })); // Build a local instantiation scope so downstream components can // consume {@link IAgentConfigurationService} (and later {@link ILogService}) @@ -1345,6 +1356,7 @@ export class AgentService extends Disposable implements IAgentService { const existing = new Map((await this._listRegisteredSessions()).map(session => [session.session.toString(), session.external])); const discoveryLimiter = new Limiter(4); let suppressed = 0; + let registeredExternal = false; const results = await Promise.all(chats.map(({ external, ...metadata }) => discoveryLimiter.queue(async () => { const sessionMetadata = this._toSessionMetadata(metadata); const session = sessionMetadata.session; @@ -1363,7 +1375,11 @@ export class AgentService extends Disposable implements IAgentService { await this._initializeExternalSessionReadState(session); } existing.set(session.toString(), external); - await this._announceSurfacedSession({ ...sessionMetadata, _meta: withSessionExternal(sessionMetadata._meta, external) }, provider.id); + if (external && !readSessionEhcliAdoptable(sessionMetadata._meta)) { + registeredExternal = true; + } else { + await this._announceSurfacedSession({ ...sessionMetadata, _meta: withSessionExternal(sessionMetadata._meta, external) }, provider.id); + } } else { this._logService.trace(`[AgentService] discovery: ${session.toString()} was not registered (tombstoned)`); } @@ -1374,6 +1390,9 @@ export class AgentService extends Disposable implements IAgentService { } }))); const registered = results.filter(changed => changed).length; + if (registeredExternal) { + this._queueSessionListReconciliation(); + } this._logService.info(`[AgentService] discovery for provider ${provider.id}: ${chats.length} candidate(s) (${chats.filter(chat => chat.external).length} external), ${registered} registered, ${suppressed} suppressed as subagent/chat backing`); return registered > 0; } @@ -1401,6 +1420,7 @@ export class AgentService extends Disposable implements IAgentService { const external = await this._isExternalProviderChat(s.session); return { session: s.session, provider: provider.id, startTime: s.startTime, external, source: external ? 'discovery' : 'restore' }; }))); + let registeredExternal = false; for (let index = 0; index < identities.length; index++) { const identity = identities[index]; if (!identity) { @@ -1413,10 +1433,17 @@ export class AgentService extends Disposable implements IAgentService { await this._initializeExternalSessionReadState(identity.session); } existing.set(identity.session.toString(), identity.external); - await this._announceSurfacedSession({ ...metadata, _meta: withSessionExternal(metadata._meta, identity.external) }, provider.id); + if (identity.external && !readSessionEhcliAdoptable(metadata._meta)) { + registeredExternal = true; + } else { + await this._announceSurfacedSession({ ...metadata, _meta: withSessionExternal(metadata._meta, identity.external) }, provider.id); + } } } await this._sessionRegistry.markProviderBackfilled(provider.id); + if (registeredExternal) { + this._queueSessionListReconciliation(); + } } private async _initializeExternalSessionReadState(session: URI): Promise { @@ -1710,11 +1737,15 @@ export class AgentService extends Disposable implements IAgentService { }); } const combined = additions.length > 0 ? [...withStatus, ...additions] : withStatus; + const now = this._now(); + const recentSessionKeys = mode === AgentHostExternalSessionsMode.Recent + ? this._getRecentSessionKeys(combined, now) + : undefined; const visible: IAgentSessionMetadata[] = []; // Adoptable-legacy rows are withheld by migrate-legacy, not by the external mode. let hiddenByExternalMode = 0; for (const session of combined) { - if (this._shouldIncludeSession(session, mode)) { + if (this._shouldIncludeSession(session, mode, now, recentSessionKeys)) { visible.push(session); } else if (!readSessionEhcliAdoptable(session._meta)) { hiddenByExternalMode++; @@ -1748,10 +1779,33 @@ export class AgentService extends Disposable implements IAgentService { } private _getExternalSessionsMode(): AgentHostExternalSessionsMode { - return this._configurationService.getRootValue(platformRootSchema, AgentHostShowExternalSessionsConfigKey) ?? AgentHostExternalSessionsMode.Last7Days; - } - - private _shouldIncludeSession(session: IAgentSessionMetadata, mode = this._getExternalSessionsMode()): boolean { + return this._configurationService.getRootValue(platformRootSchema, AgentHostShowExternalSessionsConfigKey) ?? AgentHostExternalSessionsMode.None; + } + + private _getRecentSessionKeys(sessions: readonly IAgentSessionMetadata[], now: number): ReadonlySet { + const recentExternalSessions = sessions + .filter(session => readSessionExternal(session._meta) + && !readSessionEhcliAdoptable(session._meta) + && session.modifiedTime >= now - 7 * DAY_MS) + .sort((a, b) => { + const timeDifference = b.modifiedTime - a.modifiedTime; + if (timeDifference !== 0) { + return timeDifference; + } + const aKey = a.session.toString(); + const bKey = b.session.toString(); + return aKey < bKey ? -1 : aKey > bKey ? 1 : 0; + }) + .slice(0, RECENT_EXTERNAL_SESSION_LIMIT); + return new Set(recentExternalSessions.map(session => session.session.toString())); + } + + private _shouldIncludeSession( + session: IAgentSessionMetadata, + mode = this._getExternalSessionsMode(), + now = this._now(), + recentSessionKeys?: ReadonlySet, + ): boolean { // While migration is off, un-adopted adoptable-legacy sessions belong to the extension-host provider — exclude so a refresh cannot re-surface an unopenable row. if (readSessionEhcliAdoptable(session._meta) && !this._isMigrateLegacyEnabled()) { return false; @@ -1760,12 +1814,15 @@ export class AgentService extends Disposable implements IAgentService { return true; } switch (mode) { + case AgentHostExternalSessionsMode.Recent: + return session.modifiedTime >= now - 7 * DAY_MS + && (recentSessionKeys === undefined || recentSessionKeys.has(session.session.toString())); case AgentHostExternalSessionsMode.All: return true; case AgentHostExternalSessionsMode.Last24Hours: - return session.modifiedTime >= this._now() - 24 * 60 * 60 * 1000; + return session.modifiedTime >= now - DAY_MS; case AgentHostExternalSessionsMode.Last7Days: - return session.modifiedTime >= this._now() - 7 * 24 * 60 * 60 * 1000; + return session.modifiedTime >= now - 7 * DAY_MS; case AgentHostExternalSessionsMode.None: return false; } diff --git a/src/vs/platform/agentHost/test/node/agentService.test.ts b/src/vs/platform/agentHost/test/node/agentService.test.ts index 39be736a2de592..346ba52f636b41 100644 --- a/src/vs/platform/agentHost/test/node/agentService.test.ts +++ b/src/vs/platform/agentHost/test/node/agentService.test.ts @@ -2699,6 +2699,7 @@ suite('AgentService (node dispatcher)', () => { test('listSessions discovers provider-native sessions as external and restore preserves provenance', async () => { const db = new TestSessionDatabase(); const svc = disposables.add(new AgentService(new NullLogService(), fileService, createSessionDataService(db), { _serviceBrand: undefined } as IProductService, createNoopGitService())); + svc.configurationService.updateRootConfig({ [AgentHostShowExternalSessionsConfigKey]: AgentHostExternalSessionsMode.All }); const agent = new MockAgent('copilot'); disposables.add(toDisposable(() => agent.dispose())); @@ -2749,23 +2750,161 @@ suite('AgentService (node dispatcher)', () => { svc.registerProvider(agent); const listedByMode: Record = { + [AgentHostExternalSessionsMode.Recent]: [], [AgentHostExternalSessionsMode.None]: [], [AgentHostExternalSessionsMode.All]: [], [AgentHostExternalSessionsMode.Last24Hours]: [], [AgentHostExternalSessionsMode.Last7Days]: [], }; + const listedByDefault = (await svc.listSessions()).map(session => AgentSession.id(session.session)).sort(); let clientSeq = 1; - for (const mode of [AgentHostExternalSessionsMode.None, AgentHostExternalSessionsMode.All, AgentHostExternalSessionsMode.Last24Hours, AgentHostExternalSessionsMode.Last7Days]) { + for (const mode of [AgentHostExternalSessionsMode.Recent, AgentHostExternalSessionsMode.None, AgentHostExternalSessionsMode.All, AgentHostExternalSessionsMode.Last24Hours, AgentHostExternalSessionsMode.Last7Days]) { setExternalSessionsMode(svc, mode, clientSeq++); await waitForSessionListReconciliation(svc); listedByMode[mode] = (await svc.listSessions()).map(session => AgentSession.id(session.session)).sort(); } - assert.deepStrictEqual(listedByMode, { - [AgentHostExternalSessionsMode.None]: [], - [AgentHostExternalSessionsMode.All]: ['at-24-hours', 'at-7-days', 'older-than-24-hours', 'older-than-7-days', 'recent'], - [AgentHostExternalSessionsMode.Last24Hours]: ['at-24-hours', 'recent'], - [AgentHostExternalSessionsMode.Last7Days]: ['at-24-hours', 'at-7-days', 'older-than-24-hours', 'recent'], + assert.deepStrictEqual({ listedByDefault, listedByMode }, { + listedByDefault: [], + listedByMode: { + [AgentHostExternalSessionsMode.Recent]: ['at-24-hours', 'recent'], + [AgentHostExternalSessionsMode.None]: [], + [AgentHostExternalSessionsMode.All]: ['at-24-hours', 'at-7-days', 'older-than-24-hours', 'older-than-7-days', 'recent'], + [AgentHostExternalSessionsMode.Last24Hours]: ['at-24-hours', 'recent'], + [AgentHostExternalSessionsMode.Last7Days]: ['at-24-hours', 'at-7-days', 'older-than-24-hours', 'recent'], + }, + }); + }); + + test('recent replaces the oldest visible external session when a newer session is discovered', async () => { + const now = Date.now(); + const svc = createExternalSessionService(() => now); + setExternalSessionsMode(svc, AgentHostExternalSessionsMode.Recent, 1); + await waitForSessionListReconciliation(svc); + const agent = disposables.add(new TimedExternalAgent('copilot')); + const first = agent.addSession('first', now - 1); + const second = agent.addSession('second', now - 2); + svc.registerProvider(agent); + await svc.listSessions(); + await waitForSessionListReconciliation(svc); + + const notifications: string[] = []; + disposables.add(svc.onDidNotification(notification => { + if (notification.type === NotificationType.SessionAdded) { + notifications.push(`add:${AgentSession.id(URI.parse(notification.summary.resource))}`); + } else if (notification.type === NotificationType.SessionRemoved) { + notifications.push(`remove:${AgentSession.id(URI.parse(notification.session))}`); + } + })); + + const newest = agent.addSession('newest', now); + await (svc as unknown as { _registerDiscoveredChats(provider: IAgent, chats: readonly IAgentDiscoveredChat[]): Promise })._registerDiscoveredChats(agent, [{ + chat: URI.parse(buildDefaultChatUri(newest)), + startTime: now, + modifiedTime: now, + external: true, + }]); + await waitForSessionListReconciliation(svc); + + assert.deepStrictEqual({ + visible: (await svc.listSessions()).map(session => AgentSession.id(session.session)).sort(), + notifications, + }, { + visible: [AgentSession.id(first), AgentSession.id(newest)].sort(), + notifications: ['add:newest', `remove:${AgentSession.id(second)}`], + }); + }); + + test('external discovery reconciles against a mode change that completes while registration is in flight', async () => { + const now = Date.now(); + const svc = createExternalSessionService(() => now); + setExternalSessionsMode(svc, AgentHostExternalSessionsMode.All, 1); + await waitForSessionListReconciliation(svc); + const agent = disposables.add(new TimedExternalAgent('copilot')); + svc.registerProvider(agent); + await svc.listSessions(); + + const first = agent.addSession('first', now); + const second = agent.addSession('second', now - 1); + const third = agent.addSession('third', now - 2); + const registry = (svc as unknown as { _sessionRegistry: AgentSessionRegistry })._sessionRegistry; + const originalRegister = registry.register.bind(registry); + const registrationGate = new DeferredPromise(); + let registrationsStarted = 0; + registry.register = async (session, sessionOptions, registerOptions) => { + registrationsStarted++; + await registrationGate.p; + return originalRegister(session, sessionOptions, registerOptions); + }; + + const notifications: string[] = []; + disposables.add(svc.onDidNotification(notification => { + if (notification.type === NotificationType.SessionAdded) { + notifications.push(`add:${AgentSession.id(URI.parse(notification.summary.resource))}`); + } + })); + const registration = (svc as unknown as { _registerDiscoveredChats(provider: IAgent, chats: readonly IAgentDiscoveredChat[]): Promise })._registerDiscoveredChats(agent, [ + { chat: URI.parse(buildDefaultChatUri(first)), startTime: now, modifiedTime: now, external: true }, + { chat: URI.parse(buildDefaultChatUri(second)), startTime: now - 1, modifiedTime: now - 1, external: true }, + { chat: URI.parse(buildDefaultChatUri(third)), startTime: now - 2, modifiedTime: now - 2, external: true }, + ]); + for (let attempt = 0; attempt < 20 && registrationsStarted < 3; attempt++) { + await timeout(0); + } + + setExternalSessionsMode(svc, AgentHostExternalSessionsMode.Recent, 2); + await waitForSessionListReconciliation(svc); + registrationGate.complete(); + await registration; + await waitForSessionListReconciliation(svc); + + assert.deepStrictEqual({ + visible: (await svc.listSessions()).map(session => AgentSession.id(session.session)).sort(), + notifications: notifications.sort(), + }, { + visible: ['first', 'second'], + notifications: ['add:first', 'add:second'], + }); + }); + + test('recent reconciles clients when a hidden external session becomes more recent', async () => { + const now = Date.now(); + const svc = createExternalSessionService(() => now); + setExternalSessionsMode(svc, AgentHostExternalSessionsMode.Recent, 1); + await waitForSessionListReconciliation(svc); + const agent = disposables.add(new TimedExternalAgent('copilot')); + const first = agent.addSession('first', now - 1); + const second = agent.addSession('second', now - 2); + const third = agent.addSession('third', now - 3); + svc.registerProvider(agent); + await svc.listSessions(); + await waitForSessionListReconciliation(svc); + await svc.restoreSession(third); + + const notifications: string[] = []; + disposables.add(svc.onDidNotification(notification => { + if (notification.type === NotificationType.SessionAdded) { + notifications.push(`add:${AgentSession.id(URI.parse(notification.summary.resource))}`); + } else if (notification.type === NotificationType.SessionRemoved) { + notifications.push(`remove:${AgentSession.id(URI.parse(notification.session))}`); + } + })); + + svc.stateManager.dispatchServerAction(buildDefaultChatUri(third), { + type: ActionType.ChatTurnStarted, + turnId: 'turn-third', + startedAt: new Date(now).toISOString(), + message: { text: 'Update', origin: { kind: MessageKind.User } }, + }); + await timeout(150); + await waitForSessionListReconciliation(svc); + + assert.deepStrictEqual({ + visible: (await svc.listSessions()).map(session => AgentSession.id(session.session)).sort(), + notifications, + }, { + visible: [AgentSession.id(first), AgentSession.id(third)].sort(), + notifications: ['add:third', `remove:${AgentSession.id(second)}`], }); }); @@ -2802,6 +2941,8 @@ suite('AgentService (node dispatcher)', () => { test('unpublishes and republishes a restored external session as the configured mode changes', async () => { const now = Date.now(); const svc = createExternalSessionService(() => now); + setExternalSessionsMode(svc, AgentHostExternalSessionsMode.All, 1); + await waitForSessionListReconciliation(svc); const agent = disposables.add(new TimedExternalAgent('copilot')); const session = agent.addSession('restored-external', now); const notifications: string[] = []; @@ -2817,10 +2958,10 @@ suite('AgentService (node dispatcher)', () => { await svc.restoreSession(session); notifications.length = 0; - setExternalSessionsMode(svc, AgentHostExternalSessionsMode.None, 1); + setExternalSessionsMode(svc, AgentHostExternalSessionsMode.None, 2); await waitForSessionListReconciliation(svc); const hidden = (await svc.listSessions()).map(entry => entry.session.toString()); - setExternalSessionsMode(svc, AgentHostExternalSessionsMode.All, 2); + setExternalSessionsMode(svc, AgentHostExternalSessionsMode.All, 3); await waitForSessionListReconciliation(svc); assert.deepStrictEqual({ @@ -3193,6 +3334,7 @@ suite('AgentService (node dispatcher)', () => { } } const svc = disposables.add(new AgentService(new NullLogService(), fileService, createSessionDataService(), { _serviceBrand: undefined } as IProductService, createNoopGitService())); + svc.configurationService.updateRootConfig({ [AgentHostShowExternalSessionsConfigKey]: AgentHostExternalSessionsMode.All }); const agent = disposables.add(new GatedListAgent('copilot')); svc.registerProvider(agent); const legacy = AgentSession.uri('copilot', 'legacy-concurrent'); @@ -3242,6 +3384,7 @@ suite('AgentService (node dispatcher)', () => { } const db = new TestSessionDatabase(); const svc = disposables.add(new AgentService(new NullLogService(), fileService, createSessionDataService(db), { _serviceBrand: undefined } as IProductService, createNoopGitService())); + svc.configurationService.updateRootConfig({ [AgentHostShowExternalSessionsConfigKey]: AgentHostExternalSessionsMode.All }); const agent = disposables.add(new TransientListFailureAgent('copilot')); svc.registerProvider(agent); const legacy = AgentSession.uri('copilot', 'legacy-session'); @@ -3262,6 +3405,7 @@ suite('AgentService (node dispatcher)', () => { test('a late-registered provider gets its own native discovery pass', async () => { const svc = disposables.add(new AgentService(new NullLogService(), fileService, createSessionDataService(), { _serviceBrand: undefined } as IProductService, createNoopGitService())); + svc.configurationService.updateRootConfig({ [AgentHostShowExternalSessionsConfigKey]: AgentHostExternalSessionsMode.All }); const early = disposables.add(new MockAgent('copilot')); svc.registerProvider(early); @@ -3399,6 +3543,7 @@ suite('AgentService (node dispatcher)', () => { } } const svc = disposables.add(new AgentService(new NullLogService(), fileService, createSessionDataService(), { _serviceBrand: undefined } as IProductService, createNoopGitService())); + svc.configurationService.updateRootConfig({ [AgentHostShowExternalSessionsConfigKey]: AgentHostExternalSessionsMode.All }); const providerA = disposables.add(new CountingAgent('copilot')); const providerB = disposables.add(new FailingThenRecoveringAgent('other')); @@ -3449,6 +3594,7 @@ suite('AgentService (node dispatcher)', () => { } } const svc = disposables.add(new AgentService(new NullLogService(), fileService, createSessionDataService(), { _serviceBrand: undefined } as IProductService, createNoopGitService())); + svc.configurationService.updateRootConfig({ [AgentHostShowExternalSessionsConfigKey]: AgentHostExternalSessionsMode.All }); const agent = disposables.add(new NotYetEnumerableAgent('copilot')); const originalListExternalChats = agent.listExternalChats.bind(agent); (agent as unknown as { listExternalChats: () => Promise }).listExternalChats = async () => { @@ -3724,6 +3870,7 @@ suite('AgentService (node dispatcher)', () => { // Simulate an old database whose legacy one-time marker is set. await db.markSessionRegistryBackfilled(); const svc = disposables.add(new AgentService(new NullLogService(), fileService, createSessionDataService(), { _serviceBrand: undefined } as IProductService, createNoopGitService(), undefined, undefined, undefined, undefined, undefined, undefined, undefined, undefined, db)); + svc.configurationService.updateRootConfig({ [AgentHostShowExternalSessionsConfigKey]: AgentHostExternalSessionsMode.All }); const agent = disposables.add(new CountingAgent('copilot')); const legacy = AgentSession.uri('copilot', 'old-db-native-session'); (agent as unknown as { _sessions: Map })._sessions.set(AgentSession.id(legacy), legacy); @@ -3937,6 +4084,7 @@ suite('AgentService (node dispatcher)', () => { (agent as unknown as { _sessions: Map })._sessions.set(sessionId, sessionUri); const svc = disposables.add(new AgentService(new NullLogService(), fileService, sessionDataService, { _serviceBrand: undefined } as IProductService, createNoopGitService())); + svc.configurationService.updateRootConfig({ [AgentHostShowExternalSessionsConfigKey]: AgentHostExternalSessionsMode.All }); svc.registerProvider(agent); const sessions = await svc.listSessions(); @@ -3982,6 +4130,7 @@ suite('AgentService (node dispatcher)', () => { (agent as unknown as { _sessions: Map })._sessions.set(sessionId, sessionUri); const svc = disposables.add(new AgentService(new NullLogService(), fileService, sessionDataService, { _serviceBrand: undefined } as IProductService, createNoopGitService())); + svc.configurationService.updateRootConfig({ [AgentHostShowExternalSessionsConfigKey]: AgentHostExternalSessionsMode.All }); svc.registerProvider(agent); const sessions = await svc.listSessions(); @@ -4004,6 +4153,7 @@ suite('AgentService (node dispatcher)', () => { }; (agent as unknown as { _sessions: Map })._sessions.set(sessionId, sessionUri); const svc = disposables.add(new AgentService(new NullLogService(), fileService, createSessionDataService(db), { _serviceBrand: undefined } as IProductService, createNoopGitService())); + svc.configurationService.updateRootConfig({ [AgentHostShowExternalSessionsConfigKey]: AgentHostExternalSessionsMode.All }); svc.registerProvider(agent); const sessions = await svc.listSessions(); @@ -4024,6 +4174,7 @@ suite('AgentService (node dispatcher)', () => { disposables.add(toDisposable(() => agent.dispose())); (agent as unknown as { _sessions: Map })._sessions.set(sessionId, sessionUri); const svc = disposables.add(new AgentService(new NullLogService(), fileService, createSessionDataService(db), { _serviceBrand: undefined } as IProductService, createNoopGitService())); + svc.configurationService.updateRootConfig({ [AgentHostShowExternalSessionsConfigKey]: AgentHostExternalSessionsMode.All }); svc.registerProvider(agent); const sessions = await svc.listSessions(); @@ -4041,6 +4192,7 @@ suite('AgentService (node dispatcher)', () => { }; (agent as unknown as { _sessions: Map })._sessions.set(sessionId, sessionUri); const svc = disposables.add(new AgentService(new NullLogService(), fileService, nullSessionDataService, { _serviceBrand: undefined } as IProductService, createNoopGitService())); + svc.configurationService.updateRootConfig({ [AgentHostShowExternalSessionsConfigKey]: AgentHostExternalSessionsMode.All }); svc.registerProvider(agent); const sessions = await svc.listSessions(); @@ -4066,6 +4218,7 @@ suite('AgentService (node dispatcher)', () => { return []; }; const svc = disposables.add(new AgentService(new NullLogService(), fileService, createSessionDataService(db), { _serviceBrand: undefined } as IProductService, gitService)); + svc.configurationService.updateRootConfig({ [AgentHostShowExternalSessionsConfigKey]: AgentHostExternalSessionsMode.All }); svc.registerProvider(agent); const sessions = await svc.listSessions(); @@ -4100,6 +4253,7 @@ suite('AgentService (node dispatcher)', () => { gitService.getWorktreeRoots = async () => [primaryRoot, linkedCheckout, sessionWorktree]; const sessionDataService = createSessionDataService(db); const svc = disposables.add(new AgentService(new NullLogService(), fileService, sessionDataService, { _serviceBrand: undefined } as IProductService, gitService)); + svc.configurationService.updateRootConfig({ [AgentHostShowExternalSessionsConfigKey]: AgentHostExternalSessionsMode.All }); svc.setWorktreeIsolation(disposables.add(new WorktreeIsolation( { generateBranchName: async () => 'agents/test' }, gitService, @@ -4146,6 +4300,7 @@ suite('AgentService (node dispatcher)', () => { gitService.getDefaultBranch = async () => ({ name: 'main', startPoint: 'main' }); const sessionDataService = createSessionDataService(db); const svc = disposables.add(new AgentService(new NullLogService(), fileService, sessionDataService, { _serviceBrand: undefined } as IProductService, gitService)); + svc.configurationService.updateRootConfig({ [AgentHostShowExternalSessionsConfigKey]: AgentHostExternalSessionsMode.All }); svc.setWorktreeIsolation(disposables.add(new WorktreeIsolation( { generateBranchName: async () => 'agents/test' }, gitService, @@ -10028,6 +10183,7 @@ suite('AgentService (node dispatcher)', () => { { type: 'message', session, role: 'assistant', messageId: 'msg-2', content: 'Hi', toolRequests: [] }, ]; await service.restoreSession(sessionResource); + await (service as unknown as { _sessionListReconciliation: Promise })._sessionListReconciliation; agent.events.length = 0; service.addSubscriber(sessionResource, 'client-1'); service.unsubscribe(sessionResource, 'client-1'); diff --git a/src/vs/platform/agentHost/test/node/protocol/sessionLifecycle.integrationTest.ts b/src/vs/platform/agentHost/test/node/protocol/sessionLifecycle.integrationTest.ts index eafd05292e32e4..074390d8670810 100644 --- a/src/vs/platform/agentHost/test/node/protocol/sessionLifecycle.integrationTest.ts +++ b/src/vs/platform/agentHost/test/node/protocol/sessionLifecycle.integrationTest.ts @@ -14,6 +14,7 @@ import { PROTOCOL_VERSION } from '../../../common/state/protocol/version/registr import type { ListSessionsResult } from '../../../common/state/sessionProtocol.js'; import { buildDefaultChatUri, ResponsePartKind, ROOT_STATE_URI, SessionStatus, type MarkdownResponsePart, type ISessionWithDefaultChat, type ToolCallResponsePart } from '../../../common/state/sessionState.js'; import { AgentHostSessionReleaseGraceMsEnvVar } from '../../../common/agentService.js'; +import { AgentHostExternalSessionsMode, AgentHostShowExternalSessionsConfigKey } from '../../../common/agentHostSchema.js'; import { PRE_EXISTING_SESSION_URI } from '../mockAgent.js'; import { createAndSubscribeSession, @@ -129,7 +130,25 @@ suite('Protocol WebSocket — Session Lifecycle', function () { // through the server's handleCreateSession -- simulating a session // from a previous server lifetime. const preExistingUri = PRE_EXISTING_SESSION_URI.toString(); + client.notify('dispatchAction', { + channel: ROOT_STATE_URI, + clientSeq: 1, + action: { + type: 'root/configChanged', + config: { [AgentHostShowExternalSessionsConfigKey]: AgentHostExternalSessionsMode.All }, + }, + }); + await client.call('ping'); const list = await client.call('listSessions', { channel: ROOT_STATE_URI }); + client.notify('dispatchAction', { + channel: ROOT_STATE_URI, + clientSeq: 2, + action: { + type: 'root/configChanged', + config: { [AgentHostShowExternalSessionsConfigKey]: AgentHostExternalSessionsMode.None }, + }, + }); + await client.call('ping'); const preExisting = list.items.find(s => s.resource === preExistingUri); assert.ok(preExisting, 'listSessions should include the pre-existing session'); diff --git a/src/vs/platform/chat/common/chatSettings.ts b/src/vs/platform/chat/common/chatSettings.ts index decb13aa7fa914..397749566d3eaf 100644 --- a/src/vs/platform/chat/common/chatSettings.ts +++ b/src/vs/platform/chat/common/chatSettings.ts @@ -10,6 +10,7 @@ export const ChatEditAutoApproveSettingId = 'chat.tools.edits.autoApprove'; export type ChatEditAutoApprovePatterns = Readonly>; export const enum ChatExternalSessionsMode { + Recent = 'recent', None = 'none', All = 'all', Last24Hours = 'last24Hours', diff --git a/src/vs/sessions/contrib/chat/browser/externalSessionBanner.ts b/src/vs/sessions/contrib/chat/browser/externalSessionBanner.ts index 333c27971487fa..965cb50ec3dba7 100644 --- a/src/vs/sessions/contrib/chat/browser/externalSessionBanner.ts +++ b/src/vs/sessions/contrib/chat/browser/externalSessionBanner.ts @@ -41,8 +41,10 @@ interface IExternalSessionBannerOptions { readonly onDidDismissWithFocus?: () => void; } -export function willExternalSessionBeHidden(mode: ChatExternalSessionsMode, updatedAt: Date, now: number): boolean { +export function shouldConfirmExternalSessionVisibilityChange(mode: ChatExternalSessionsMode, updatedAt: Date, now: number): boolean { switch (mode) { + case ChatExternalSessionsMode.Recent: + return true; case ChatExternalSessionsMode.None: return true; case ChatExternalSessionsMode.All: @@ -55,9 +57,20 @@ export function willExternalSessionBeHidden(mode: ChatExternalSessionsMode, upda } export function getExternalSessionVisibilityConfirmation(mode: ChatExternalSessionsMode, updatedAt: Date, now: number, productName: string): IConfirmation { - const message = localize('externalSessionBanner.confirm.message', "This session will no longer appear in {0}", productName); + const message = mode === ChatExternalSessionsMode.Recent + ? localize('externalSessionBanner.confirm.recent.message', "This session may no longer appear in {0}", productName) + : localize('externalSessionBanner.confirm.message', "This session will no longer appear in {0}", productName); const primaryButton = localize({ key: 'externalSessionBanner.confirm.save', comment: ['&& denotes a mnemonic'] }, "&&Save Anyway"); + if (mode === ChatExternalSessionsMode.Recent) { + return { + type: 'warning', + message, + detail: localize('externalSessionBanner.confirm.recent.detail', "Only the 2 most recently updated external sessions from the last 7 days will be shown. Are you sure you want to save this change?"), + primaryButton, + }; + } + if (mode === ChatExternalSessionsMode.None) { return { type: 'warning', @@ -208,6 +221,13 @@ export class ExternalSessionBanner extends Disposable { description: localize('externalSessionBanner.select.none.description', "Do not show sessions created in another application."), }, }, + { + mode: ChatExternalSessionsMode.Recent, + item: { + text: localize('externalSessionBanner.select.recent', "Recent"), + description: localize('externalSessionBanner.select.recent.description', "Show the 2 most recently updated external sessions from the last 7 days."), + }, + }, { mode: ChatExternalSessionsMode.Last24Hours, item: { @@ -219,7 +239,7 @@ export class ExternalSessionBanner extends Disposable { mode: ChatExternalSessionsMode.Last7Days, item: { text: localize('externalSessionBanner.select.last7Days', "Last 7 Days"), - description: localize('externalSessionBanner.select.last7Days.description', "Show external sessions updated in the last 7 days. This is the default."), + description: localize('externalSessionBanner.select.last7Days.description', "Show external sessions updated in the last 7 days."), }, }, { @@ -274,7 +294,7 @@ export class ExternalSessionBanner extends Disposable { try { const now = Date.now(); const updatedAt = session.updatedAt.get(); - if (willExternalSessionBeHidden(mode, updatedAt, now)) { + if (shouldConfirmExternalSessionVisibilityChange(mode, updatedAt, now)) { const confirmation = await this._dialogService.confirm(getExternalSessionVisibilityConfirmation(mode, updatedAt, now, this._productService.nameShort)); if (!confirmation.confirmed) { return; diff --git a/src/vs/sessions/contrib/chat/test/browser/externalSessionBanner.test.ts b/src/vs/sessions/contrib/chat/test/browser/externalSessionBanner.test.ts index 66a1ab03513fe6..c5841d767e591a 100644 --- a/src/vs/sessions/contrib/chat/test/browser/externalSessionBanner.test.ts +++ b/src/vs/sessions/contrib/chat/test/browser/externalSessionBanner.test.ts @@ -6,7 +6,7 @@ import assert from 'assert'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../base/test/common/utils.js'; import { ChatExternalSessionsMode } from '../../../../../platform/chat/common/chatSettings.js'; -import { getExternalSessionVisibilityConfirmation, willExternalSessionBeHidden } from '../../browser/externalSessionBanner.js'; +import { getExternalSessionVisibilityConfirmation, shouldConfirmExternalSessionVisibilityChange } from '../../browser/externalSessionBanner.js'; suite('Sessions - External Session Banner', () => { ensureNoDisposablesAreLeakedInTestSuite(); @@ -16,13 +16,15 @@ suite('Sessions - External Session Banner', () => { const now = Date.UTC(2026, 7, 16, 12); assert.deepStrictEqual({ - none: willExternalSessionBeHidden(ChatExternalSessionsMode.None, new Date(now), now), - all: willExternalSessionBeHidden(ChatExternalSessionsMode.All, new Date(0), now), - at24Hours: willExternalSessionBeHidden(ChatExternalSessionsMode.Last24Hours, new Date(now - day), now), - olderThan24Hours: willExternalSessionBeHidden(ChatExternalSessionsMode.Last24Hours, new Date(now - day - 1), now), - at7Days: willExternalSessionBeHidden(ChatExternalSessionsMode.Last7Days, new Date(now - 7 * day), now), - olderThan7Days: willExternalSessionBeHidden(ChatExternalSessionsMode.Last7Days, new Date(now - 7 * day - 1), now), + recent: shouldConfirmExternalSessionVisibilityChange(ChatExternalSessionsMode.Recent, new Date(now), now), + none: shouldConfirmExternalSessionVisibilityChange(ChatExternalSessionsMode.None, new Date(now), now), + all: shouldConfirmExternalSessionVisibilityChange(ChatExternalSessionsMode.All, new Date(0), now), + at24Hours: shouldConfirmExternalSessionVisibilityChange(ChatExternalSessionsMode.Last24Hours, new Date(now - day), now), + olderThan24Hours: shouldConfirmExternalSessionVisibilityChange(ChatExternalSessionsMode.Last24Hours, new Date(now - day - 1), now), + at7Days: shouldConfirmExternalSessionVisibilityChange(ChatExternalSessionsMode.Last7Days, new Date(now - 7 * day), now), + olderThan7Days: shouldConfirmExternalSessionVisibilityChange(ChatExternalSessionsMode.Last7Days, new Date(now - 7 * day - 1), now), }, { + recent: true, none: true, all: false, at24Hours: false, @@ -46,4 +48,18 @@ suite('Sessions - External Session Banner', () => { } ); }); + + test('warns that recent may hide the open session', () => { + const now = Date.UTC(2026, 7, 16, 12); + + assert.deepStrictEqual( + getExternalSessionVisibilityConfirmation(ChatExternalSessionsMode.Recent, new Date(now), now, 'Code - OSS'), + { + type: 'warning', + message: 'This session may no longer appear in Code - OSS', + detail: 'Only the 2 most recently updated external sessions from the last 7 days will be shown. Are you sure you want to save this change?', + primaryButton: '&&Save Anyway', + } + ); + }); }); diff --git a/src/vs/workbench/contrib/chat/browser/agentSessions/externalSessionsFilterMenu.ts b/src/vs/workbench/contrib/chat/browser/agentSessions/externalSessionsFilterMenu.ts index 98f027be079c9c..603d6cec71614e 100644 --- a/src/vs/workbench/contrib/chat/browser/agentSessions/externalSessionsFilterMenu.ts +++ b/src/vs/workbench/contrib/chat/browser/agentSessions/externalSessionsFilterMenu.ts @@ -14,6 +14,7 @@ import { ChatConfiguration } from '../../common/constants.js'; const externalSessionOptions = [ { mode: ChatExternalSessionsMode.None, title: localize2('agentSessions.filter.external.none', "None") }, + { mode: ChatExternalSessionsMode.Recent, title: localize2('agentSessions.filter.external.recent', "Recent") }, { mode: ChatExternalSessionsMode.Last24Hours, title: localize2('agentSessions.filter.external.last24Hours', "Last 24 Hours") }, { mode: ChatExternalSessionsMode.Last7Days, title: localize2('agentSessions.filter.external.last7Days', "Last 7 Days") }, { mode: ChatExternalSessionsMode.All, title: localize2('agentSessions.filter.external.all', "All") }, diff --git a/src/vs/workbench/contrib/chat/browser/chat.shared.contribution.ts b/src/vs/workbench/contrib/chat/browser/chat.shared.contribution.ts index aa9d945f7c448e..d5c5284df09017 100644 --- a/src/vs/workbench/contrib/chat/browser/chat.shared.contribution.ts +++ b/src/vs/workbench/contrib/chat/browser/chat.shared.contribution.ts @@ -398,14 +398,15 @@ configurationRegistry.registerConfiguration({ }, [ChatConfiguration.ShowExternalAgentSessions]: { type: 'string', - enum: [AgentHostExternalSessionsMode.None, AgentHostExternalSessionsMode.All, AgentHostExternalSessionsMode.Last24Hours, AgentHostExternalSessionsMode.Last7Days], + enum: [AgentHostExternalSessionsMode.None, AgentHostExternalSessionsMode.Recent, AgentHostExternalSessionsMode.Last24Hours, AgentHostExternalSessionsMode.Last7Days, AgentHostExternalSessionsMode.All], enumDescriptions: [ nls.localize('chat.agentSessions.showExternal.none', "Only shows sessions created by the Agent Host."), - nls.localize('chat.agentSessions.showExternal.all', "Shows all sessions discovered from supported external agent applications."), + nls.localize('chat.agentSessions.showExternal.recent', "Shows the 2 most recently updated external sessions from the last 7 days."), nls.localize('chat.agentSessions.showExternal.last24Hours', "Shows external sessions updated in the last 24 hours."), - nls.localize('chat.agentSessions.showExternal.last7Days', "Shows external sessions updated in the last 7 days. This is the default."), + nls.localize('chat.agentSessions.showExternal.last7Days', "Shows external sessions updated in the last 7 days."), + nls.localize('chat.agentSessions.showExternal.all', "Shows all sessions discovered from supported external agent applications."), ], - default: AgentHostExternalSessionsMode.Last7Days, + default: AgentHostExternalSessionsMode.None, markdownDescription: nls.localize('chat.agentSessions.showExternal', "Controls which external agent sessions, created outside VS Code's Agent Host, are shown."), agentHost: { key: AgentHostShowExternalSessionsConfigKey }, }, diff --git a/src/vs/workbench/contrib/chat/test/browser/agentSessions/externalSessionsFilterMenu.test.ts b/src/vs/workbench/contrib/chat/test/browser/agentSessions/externalSessionsFilterMenu.test.ts index 2268f9b2186648..ea545123c3450a 100644 --- a/src/vs/workbench/contrib/chat/test/browser/agentSessions/externalSessionsFilterMenu.test.ts +++ b/src/vs/workbench/contrib/chat/test/browser/agentSessions/externalSessionsFilterMenu.test.ts @@ -40,10 +40,10 @@ suite('External Sessions Filter Menu', () => { }, options: options.map(item => ({ title: typeof item.command.title === 'string' ? item.command.title : item.command.title.value, - checkedForLast7Days: getToggledExpression(item.command.toggled)?.evaluate({ + checkedForRecent: getToggledExpression(item.command.toggled)?.evaluate({ getValue: (key: string) => ( key === `config.${ChatConfiguration.ShowExternalAgentSessions}` - ? ChatExternalSessionsMode.Last7Days + ? ChatExternalSessionsMode.Recent : undefined ) as T, }), @@ -55,10 +55,11 @@ suite('External Sessions Filter Menu', () => { submenu: submenuId.id, }, options: [ - { title: 'None', checkedForLast7Days: false }, - { title: 'Last 24 Hours', checkedForLast7Days: false }, - { title: 'Last 7 Days', checkedForLast7Days: true }, - { title: 'All', checkedForLast7Days: false }, + { title: 'None', checkedForRecent: false }, + { title: 'Recent', checkedForRecent: true }, + { title: 'Last 24 Hours', checkedForRecent: false }, + { title: 'Last 7 Days', checkedForRecent: false }, + { title: 'All', checkedForRecent: false }, ], }); }); From da71f9583b692f815b533b88432d433afd66acf9 Mon Sep 17 00:00:00 2001 From: Lee Murray Date: Mon, 17 Aug 2026 16:50:35 +0100 Subject: [PATCH 6/6] Modern UI: Add CSS styles to persist tab actions when action space is reserved (#331175) * Add CSS styles to persist tab actions when action space is reserved * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: mrleemurray Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- .../styleOverrides/browser/media/tabs.css | 14 +++++++++ .../styleOverrides.contribution.test.ts | 29 +++++++++++++++++++ 2 files changed, 43 insertions(+) diff --git a/src/vs/workbench/contrib/styleOverrides/browser/media/tabs.css b/src/vs/workbench/contrib/styleOverrides/browser/media/tabs.css index 8fde437ed79877..e3a8477b7d96bf 100644 --- a/src/vs/workbench/contrib/styleOverrides/browser/media/tabs.css +++ b/src/vs/workbench/contrib/styleOverrides/browser/media/tabs.css @@ -528,6 +528,20 @@ opacity: 1; } +/* When the reserved action column is always present, keep the action visible and clickable so + * the column never reads as a blank gap (https://github.com/microsoft/vscode/issues/329605). */ +.modern-ui-tabs.monaco-workbench .part.editor > .content .editor-group-container > .title.tab-actions-reserve-space .tabs-container > .tab:not(.sticky-compact):not(.close-action-off) > .tab-actions { + pointer-events: auto; +} + +.modern-ui-tabs.monaco-workbench .part.editor > .content .editor-group-container.active > .title.tab-actions-reserve-space .tabs-container > .tab:not(.sticky-compact):not(.close-action-off) > .tab-actions .action-label { + opacity: 1; +} + +.modern-ui-tabs.monaco-workbench .part.editor > .content .editor-group-container:not(.active) > .title.tab-actions-reserve-space .tabs-container > .tab:not(.sticky-compact):not(.close-action-off):not(:hover) > .tab-actions .action-label:not(:focus) { + opacity: 0.5; +} + .modern-ui-tabs.monaco-workbench .part.editor > .content .editor-group-container > .title .tabs-container > .tab.dirty:not(.close-action-off):hover > .tab-actions .action-label.codicon-close::before, .modern-ui-tabs.monaco-workbench .part.editor > .content .editor-group-container > .title .tabs-container > .tab.dirty > .tab-actions .action-label.codicon-close:focus::before { content: var(--vscode-icon-close-content); diff --git a/src/vs/workbench/contrib/styleOverrides/test/browser/styleOverrides.contribution.test.ts b/src/vs/workbench/contrib/styleOverrides/test/browser/styleOverrides.contribution.test.ts index 310198f7ee3556..1626131f407125 100644 --- a/src/vs/workbench/contrib/styleOverrides/test/browser/styleOverrides.contribution.test.ts +++ b/src/vs/workbench/contrib/styleOverrides/test/browser/styleOverrides.contribution.test.ts @@ -664,6 +664,35 @@ suite('StyleOverridesContribution', () => { }); }); + test('persists tab actions when action space is reserved', () => { + const root = document.createElement('div'); + root.className = 'monaco-workbench modern-ui-tabs'; + document.body.appendChild(root); + store.add(toDisposable(() => root.remove())); + + const content = appendElement(appendElement(root, 'part editor'), 'content'); + const createTab = (groupClassName: string, titleClassName: string): HTMLElement => { + const title = appendElement(appendElement(content, groupClassName), titleClassName); + const tab = appendElement(appendElement(title, 'tabs-container'), 'tab'); + return appendElement(appendElement(tab, 'tab-actions'), 'action-label'); + }; + + const reservedActive = createTab('editor-group-container active', 'title tab-actions-reserve-space'); + const reservedInactiveGroup = createTab('editor-group-container', 'title tab-actions-reserve-space'); + const transientActive = createTab('editor-group-container active', 'title'); + + const targetWindow = getWindow(root); + assert.deepStrictEqual({ + reservedActive: { opacity: targetWindow.getComputedStyle(reservedActive).opacity, pointerEvents: targetWindow.getComputedStyle(reservedActive.parentElement!).pointerEvents }, + reservedInactiveGroup: { opacity: targetWindow.getComputedStyle(reservedInactiveGroup).opacity, pointerEvents: targetWindow.getComputedStyle(reservedInactiveGroup.parentElement!).pointerEvents }, + transientActive: { opacity: targetWindow.getComputedStyle(transientActive).opacity, pointerEvents: targetWindow.getComputedStyle(transientActive.parentElement!).pointerEvents }, + }, { + reservedActive: { opacity: '1', pointerEvents: 'auto' }, + reservedInactiveGroup: { opacity: '0.5', pointerEvents: 'auto' }, + transientActive: { opacity: '0', pointerEvents: 'none' }, + }); + }); + test('uses legacy color customizations for Modern UI editor tabs only', () => { const theme = ColorThemeData.createUnloadedTheme('vs-dark', { [editorBackground]: '#000000',