From 20ea5dc72cda6def95a6208272f51a46b3b6a489 Mon Sep 17 00:00:00 2001 From: Osvaldo Ortega Date: Mon, 24 Aug 2026 17:05:10 -0700 Subject: [PATCH 1/4] Keep locally provisioned sessions out of the list until they are real A provider that creates a session remotely has to seed it into its cache before connecting, so a later discovery pass reconciles against it instead of creating a second entry. But the caller is still showing an optimistic placeholder row for that same session until the first turn is dispatched, so publishing the seed straight away listed the session twice for as long as the environment took to wake (~8s observed). Retiring the placeholder earlier is not an option: swapping when the session merely exists hands the UI a session it cannot open yet and the view falls back to the new-session screen. So the seed is now withheld from `getSessions` instead, and revealed as the placeholder is retired. The reveal is silent because the list re-reads on any change, letting a single event swap both rows rather than listing both for a frame. Withheld sessions stay reachable by resource, so opening one still works. Separately, such a session could be evicted before it existed on the host. Connecting triggers a reconciliation that prunes anything cached but not listed, and the session id is minted before the host materializes the session, so that first listing can legitimately omit it. Evicting there dropped the row the user was looking at and bounced the view to the new-session screen, re-adding the session seconds later. Sessions seeded as provisional now resist that eviction until the host lists them. The base provider already made this exemption for a sibling case, so that is generalized into an overridable hook rather than adding a parallel mechanism. The protection is bounded: without a deadline, a session the host will never list becomes a permanent row that only a reload clears, trading a visible flash for an invisible one. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../browser/baseAgentHostSessionsProvider.ts | 17 +++ .../browser/copilotChatSessionsProvider.ts | 39 ++++-- .../copilotChatSessionsProvider.test.ts | 44 ++++++- .../cloudSandboxAgentHostContribution.ts | 13 +- .../remoteAgentHostSessionsProvider.ts | 94 +++++++++++++- .../cloudSandboxAgentHostContribution.test.ts | 34 ++++- .../remoteAgentHostSessionsProvider.test.ts | 116 ++++++++++++++++++ 7 files changed, 332 insertions(+), 25 deletions(-) diff --git a/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts b/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts index 07de3b0d5c169..aaf4112539565 100644 --- a/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts +++ b/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts @@ -5253,6 +5253,7 @@ export abstract class BaseAgentHostSessionsProvider extends Disposable implement } const removed: ISession[] = []; + this._onHostListedSessions(currentKeys); // Some hosts briefly omit the just-sent eager session from listSessions. // Keep the pending session visible until sendRequest graduates it. const pendingRawId = this._pendingSession?.resource.path.replace(/^\//, ''); @@ -5272,6 +5273,9 @@ export abstract class BaseAgentHostSessionsProvider extends Disposable implement if (key === pendingRawId) { continue; } + if (!this._isSessionEvictable(key)) { + continue; + } if (!evictUnlistedAgents && !listedAgentProviders.has(cached.agentProvider)) { continue; } @@ -5304,6 +5308,19 @@ export abstract class BaseAgentHostSessionsProvider extends Disposable implement } } + /** + * Whether a cached session the host did not list may be evicted. Subclasses + * override this to protect a session that exists but that the host has not + * materialized yet — evicting one drops the row the user may be looking at, + * sending the view back to the new-session screen. + */ + protected _isSessionEvictable(_rawId: string): boolean { + return true; + } + + /** Raw ids the host listed, reported before eviction runs so subclasses can retire protections. */ + protected _onHostListedSessions(_rawIds: ReadonlySet): void { } + /** * Arm a backoff retry of {@link _refreshSessions}. Used after a failed * refresh so a transient startup failure self-heals without requiring an diff --git a/src/vs/sessions/contrib/providers/copilotChatSessions/browser/copilotChatSessionsProvider.ts b/src/vs/sessions/contrib/providers/copilotChatSessions/browser/copilotChatSessionsProvider.ts index 796f58e39ab30..de7b06d9597d6 100644 --- a/src/vs/sessions/contrib/providers/copilotChatSessions/browser/copilotChatSessionsProvider.ts +++ b/src/vs/sessions/contrib/providers/copilotChatSessions/browser/copilotChatSessionsProvider.ts @@ -16,6 +16,7 @@ import { URI } from '../../../../../base/common/uri.js'; import { ICommandService } from '../../../../../platform/commands/common/commands.js'; import { IDialogService } from '../../../../../platform/dialogs/common/dialogs.js'; import { IInstantiationService } from '../../../../../platform/instantiation/common/instantiation.js'; +import { AgentSession } from '../../../../../platform/agentHost/common/agent.js'; import { getAgentSessionPullRequestUri, IAgentSession } from '../../../../../workbench/contrib/chat/browser/agentSessions/agentSessionsModel.js'; import { getRepositoryName } from '../../../../../workbench/contrib/chat/browser/agentSessions/agentSessionsViewer.js'; import { IAgentSessionsService } from '../../../../../workbench/contrib/chat/browser/agentSessions/agentSessionsService.js'; @@ -55,7 +56,7 @@ import { IUriIdentityService } from '../../../../../platform/uriIdentity/common/ import { IAgentHostEnablementService } from '../../../../../platform/agentHost/common/agentHostEnablementService.js'; import { isCloudSandboxEnabled } from '../../../../../platform/agentHost/common/cloudSandboxAgentHost.js'; import { getWorkbenchContribution } from '../../../../../workbench/common/contributions.js'; -import { CloudSandboxAgentHostContribution } from '../../remoteAgentHost/browser/cloudSandboxAgentHostContribution.js'; +import { CloudSandboxAgentHostContribution, type ICloudSandboxProvisionedSession } from '../../remoteAgentHost/browser/cloudSandboxAgentHostContribution.js'; /** Copilot Cloud session type - cloud-hosted agent. */ export const CopilotCloudSessionType: ISessionType = { @@ -2103,11 +2104,12 @@ export class CopilotChatSessionsProvider extends Disposable implements ISessions const placeholder = this._chatToSession(session); this._onDidChangeSessions.fire({ added: [placeholder], removed: [], changed: [] }); + let provisioned: ICloudSandboxProvisionedSession | undefined; try { - const provisioned = await this._getCloudSandboxContribution().provisionSession({ + provisioned = await this._getCloudSandboxContribution().provisionSession({ repoNwo, - // No `baseRef`: cloud sessions have no branch picker, so Mission Control picks the - // repository's default branch — the same branch the server-run cloud agent uses. + // No `baseRef`: cloud sessions have no branch picker, and the Copilot app and CLI + // create sandbox tasks without one too, leaving the branch to Mission Control. prompt: options.query, }, CancellationToken.None); @@ -2116,14 +2118,19 @@ export class CopilotChatSessionsProvider extends Disposable implements ISessions const chat = provisioned.session.mainChat.get(); const committed = await provisioned.provider.sendRequest(provisioned.session.sessionId, chat.resource, options); - this._sessionCache.delete(session.resource.toString()); - this._invalidateGroupingCaches(); - this._sessionGroupCache.delete(session.sessionId); - this._clearCurrentNewSessionIfMatch(session); - this._onDidReplaceSession.fire({ from: placeholder, to: committed }); + // Retire the placeholder only once the turn is dispatched. Swapping earlier — when the + // session merely exists — hands the UI a session it cannot open yet, and the view falls + // back to the new-session screen. Revealing without announcing lets the replace event + // swap the two rows in one pass, rather than listing both for a frame. + this._publishSandboxSession(provisioned, { announce: false }); + this._retirePlaceholder(session, placeholder, committed); return committed; } catch (error) { this.logService.error(`[CopilotChatSessionsProvider] Failed to start cloud sandbox session for ${repoNwo}:`, error); + // The sandbox outlives a failed first turn, so list it rather than leaving it invisible. + if (provisioned) { + this._publishSandboxSession(provisioned); + } this._sessionCache.delete(session.resource.toString()); this._invalidateGroupingCaches(); this._sessionGroupCache.delete(session.sessionId); @@ -2134,6 +2141,20 @@ export class CopilotChatSessionsProvider extends Disposable implements ISessions } } + /** Reveal the sandbox session that {@link CloudSandboxAgentHostContribution.provisionSession} withheld from listings. */ + private _publishSandboxSession(provisioned: ICloudSandboxProvisionedSession, options?: { announce?: boolean }): void { + provisioned.provider.publishWithheldSession(AgentSession.id(provisioned.session.resource), options); + } + + /** Retire the optimistic placeholder in favour of the session that now exists. */ + private _retirePlaceholder(session: RemoteNewSession, placeholder: ISession, committed: ISession): void { + this._sessionCache.delete(session.resource.toString()); + this._invalidateGroupingCaches(); + this._sessionGroupCache.delete(session.sessionId); + this._clearCurrentNewSessionIfMatch(session); + this._onDidReplaceSession.fire({ from: placeholder, to: committed }); + } + async sendRequest(sessionId: string, chatResource: URI, options: ISendRequestOptions): Promise { const newSession = this._newSessions.get(sessionId); if (newSession) { diff --git a/src/vs/sessions/contrib/providers/copilotChatSessions/test/browser/copilotChatSessionsProvider.test.ts b/src/vs/sessions/contrib/providers/copilotChatSessions/test/browser/copilotChatSessionsProvider.test.ts index 2b9f5154acc20..40fc9eb08c6ef 100644 --- a/src/vs/sessions/contrib/providers/copilotChatSessions/test/browser/copilotChatSessionsProvider.test.ts +++ b/src/vs/sessions/contrib/providers/copilotChatSessions/test/browser/copilotChatSessionsProvider.test.ts @@ -1857,19 +1857,26 @@ suite('CopilotChatSessionsProvider', () => { } /** A provisioned session whose provider immediately commits the send. */ - function provisionedSession(): ICloudSandboxProvisionedSession { - const committed = upcastPartial({ sessionId: 'agenthost:sess-new' }); + function provisionedSession(sendRequest?: () => Promise): ICloudSandboxProvisionedSession & { published: string[] } { + const committed = upcastPartial({ + sessionId: 'agenthost:sess-new', + resource: URI.parse('agent-host-copilot:/sess-new'), + }); const sandboxSession = upcastPartial({ sessionId: 'agenthost:sess-new', + resource: URI.parse('agent-host-copilot:/sess-new'), mainChat: constObservable(upcastPartial({ resource: URI.parse('agent-host-copilot:/sess-new') })), }); + const published: string[] = []; return { taskId: 'task-new', sessionId: 'sess-new', environmentId: 'env-new', session: sandboxSession, + published, provider: upcastPartial({ - sendRequest: async () => committed, + sendRequest: sendRequest ?? (async () => committed), + publishWithheldSession: (rawId: string) => { published.push(rawId); }, }) as RemoteAgentHostSessionsProvider, }; } @@ -1887,7 +1894,7 @@ suite('CopilotChatSessionsProvider', () => { assert.deepStrictEqual({ committed: committed.sessionId, - // The repo comes from the workspace root; no baseRef, so MC picks the default branch. + // The repo comes from the workspace root; no baseRef, matching the Copilot app. provisionRequests, // The prompt must not also go through the server-run cloud agent. cloudSends, @@ -1913,6 +1920,35 @@ suite('CopilotChatSessionsProvider', () => { assert.deepStrictEqual({ provisionRequests, cloudSends }, { provisionRequests: [], cloudSends: ['fix it'] }); }); + test('reveals the withheld sandbox session as it retires the placeholder', async () => { + const provisioned = provisionedSession(); + const { provider } = createSandboxProvider({ provision: async () => provisioned }); + const sessionInfo = provider.createNewSession(repoWorkspace, CopilotCloudSessionType.id); + const session = provider.getSession(sessionInfo.sessionId)!; + session.setUseSandbox(true); + + // The sandbox session is seeded before connecting so a discovery pass reconciles + // against it, but it must stay out of the list until the placeholder goes away — + // otherwise both rows show for as long as the sandbox takes to wake. + await provider.sendRequest(sessionInfo.sessionId, session.mainChat.get().resource, { query: 'fix it' }); + + assert.deepStrictEqual(provisioned.published, ['sess-new']); + }); + + test('a failed send still reveals the sandbox session it already provisioned', async () => { + const provisioned = provisionedSession(async () => { throw new Error('send failed'); }); + const { provider } = createSandboxProvider({ provision: async () => provisioned }); + const sessionInfo = provider.createNewSession(repoWorkspace, CopilotCloudSessionType.id); + const session = provider.getSession(sessionInfo.sessionId)!; + session.setUseSandbox(true); + + await assert.rejects(() => provider.sendRequest(sessionInfo.sessionId, session.mainChat.get().resource, { query: 'fix it' })); + + // The sandbox outlives the failed turn, so leaving it withheld would hide a session + // that really exists. + assert.deepStrictEqual(provisioned.published, ['sess-new']); + }); + test('a failed provision removes the placeholder instead of stranding it in the list', async () => { const { provider } = createSandboxProvider(); const sessionInfo = provider.createNewSession(repoWorkspace, CopilotCloudSessionType.id); diff --git a/src/vs/sessions/contrib/providers/remoteAgentHost/browser/cloudSandboxAgentHostContribution.ts b/src/vs/sessions/contrib/providers/remoteAgentHost/browser/cloudSandboxAgentHostContribution.ts index 25ad9bc98d3ac..8c465b2b55b77 100644 --- a/src/vs/sessions/contrib/providers/remoteAgentHost/browser/cloudSandboxAgentHostContribution.ts +++ b/src/vs/sessions/contrib/providers/remoteAgentHost/browser/cloudSandboxAgentHostContribution.ts @@ -352,7 +352,13 @@ export class CloudSandboxAgentHostContribution extends Disposable implements IWo modifiedTime: now, summary: name, ...(project ? { project } : {}), - }]); + }], { + // The caller shows a placeholder row for this session until it sends the first + // turn, so listing the seed now would duplicate that row. This also protects the + // session from the first `listSessions` after connecting, which the host can + // answer before it has materialized the session. + provisional: true, + }); await this.connect({ environmentId: created.environmentId, sessionId: created.sessionId, name }); @@ -364,8 +370,9 @@ export class CloudSandboxAgentHostContribution extends Disposable implements IWo } // The adapter `seedSessions` created addresses the session by its raw id, which is the - // session id Mission Control just returned. - const session = provider.getSessions().find(candidate => AgentSession.id(candidate.resource) === created.sessionId); + // session id Mission Control just returned. Look it up through the cache rather than + // `getSessions`, which withholds it from listings until the caller publishes it. + const session = provider.getCachedSession(created.sessionId); if (!session) { throw new Error(`Provisioned sandbox session ${created.sessionId} did not surface on its provider`); } diff --git a/src/vs/sessions/contrib/providers/remoteAgentHost/browser/remoteAgentHostSessionsProvider.ts b/src/vs/sessions/contrib/providers/remoteAgentHost/browser/remoteAgentHostSessionsProvider.ts index b48c0741e6b77..b2c7f43e8962f 100644 --- a/src/vs/sessions/contrib/providers/remoteAgentHost/browser/remoteAgentHostSessionsProvider.ts +++ b/src/vs/sessions/contrib/providers/remoteAgentHost/browser/remoteAgentHostSessionsProvider.ts @@ -94,6 +94,18 @@ export interface ISessionSchemeAlias { readonly backend: string; } +/** Options for {@link RemoteAgentHostSessionsProvider.seedSessions}. */ +export interface ISeedSessionsOptions { + /** + * Mark the seeded session as freshly provisioned by this client, meaning the host + * may not have materialized it yet. It stays out of + * {@link RemoteAgentHostSessionsProvider.getSessions} until + * {@link RemoteAgentHostSessionsProvider.publishWithheldSession}, and resists + * eviction until the host lists it. Only affects sessions the call creates. + */ + readonly provisional?: boolean; +} + /** * Sessions provider for a remote agent host connection. A thin subclass of * {@link BaseAgentHostSessionsProvider} that adds the connection-lifecycle @@ -172,6 +184,24 @@ export class RemoteAgentHostSessionsProvider extends BaseAgentHostSessionsProvid */ private _unpublished = false; + /** + * Seeded sessions kept out of {@link getSessions} because the caller is still showing + * a placeholder row for them; listing both would show the session twice until the + * sandbox wakes. They stay reachable by resource, so opening one still works. + */ + private readonly _withheldSessions = new Set(); + + /** + * Raw id → deadline for locally provisioned sessions the host has not listed yet. + * Mission Control creates the task before the host materializes the session, so the + * first `listSessions` after connecting can legitimately omit it, and evicting it + * there drops the row the user is looking at. The deadline stops a session the host + * will never list from becoming a permanent row. + */ + private readonly _provisionalSessions = new Map(); + + /** How long a provisional session resists eviction while the host has not listed it. */ + static readonly PROVISIONAL_GRACE_MS = 2 * 60_000; constructor( config: IRemoteAgentHostSessionsProviderConfig, @@ -264,7 +294,13 @@ export class RemoteAgentHostSessionsProvider extends BaseAgentHostSessionsProvid } override getSessions(): ISession[] { - return this._unpublished ? [] : super.getSessions(); + if (this._unpublished) { + return []; + } + const sessions = super.getSessions(); + return this._withheldSessions.size === 0 + ? sessions + : sessions.filter(session => !this._withheldSessions.has(AgentSession.id(session.resource))); } protected override mapWorkingDirectoryUri(uri: URI): URI { @@ -349,7 +385,7 @@ export class RemoteAgentHostSessionsProvider extends BaseAgentHostSessionsProvid * filling it in on a later pass is what makes retrying worth anything. Opening a seeded session * triggers `connectOnDemand`, after which `_refreshSessions` reconciles against the host. */ - seedSessions(metas: readonly IAgentSessionMetadata[]): void { + seedSessions(metas: readonly IAgentSessionMetadata[], options?: ISeedSessionsOptions): void { const added: ISession[] = []; const changed: ISession[] = []; for (const rawMeta of metas) { @@ -366,13 +402,65 @@ export class RemoteAgentHostSessionsProvider extends BaseAgentHostSessionsProvid } const adapter = this.createAdapter(meta); this._sessionCache.set(rawId, adapter); - added.push(adapter); + if (options?.provisional) { + this._withheldSessions.add(rawId); + this._provisionalSessions.set(rawId, Date.now() + RemoteAgentHostSessionsProvider.PROVISIONAL_GRACE_MS); + } else { + added.push(adapter); + } } if (added.length > 0 || changed.length > 0) { this._onDidChangeSessions.fire({ added, removed: [], changed }); } } + protected override _isSessionEvictable(rawId: string): boolean { + const deadline = this._provisionalSessions.get(rawId); + if (deadline === undefined) { + return true; + } + if (Date.now() >= deadline) { + this._provisionalSessions.delete(rawId); + return true; + } + return false; + } + + protected override _onHostListedSessions(rawIds: ReadonlySet): void { + if (this._provisionalSessions.size === 0) { + return; + } + for (const rawId of rawIds) { + this._provisionalSessions.delete(rawId); + } + } + + /** + * Look up a cached session by raw id, **including** ones withheld from + * {@link getSessions}. Callers that seeded a session and need to act on it before + * it is listed should use this rather than scanning {@link getSessions}. + */ + getCachedSession(rawId: string): ISession | undefined { + return this._sessionCache.get(rawId); + } + + /** + * Reveal a session seeded as `provisional`, so {@link getSessions} starts returning it. + * + * Pass `announce: false` when the caller immediately fires its own change event covering + * this session: the list re-reads {@link getSessions} on any change, so a single event can + * both drop a placeholder row and reveal this one. + */ + publishWithheldSession(rawId: string, options?: { announce?: boolean }): void { + if (!this._withheldSessions.delete(rawId)) { + return; + } + const session = this._sessionCache.get(rawId); + if (session && options?.announce !== false) { + this._onDidChangeSessions.fire({ added: [session], removed: [], changed: [] }); + } + } + /** * Map a host-reported session URI onto the UI scheme, so the session routes to the agent's * content provider. The raw id is preserved, so cache keys are unaffected. diff --git a/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/cloudSandboxAgentHostContribution.test.ts b/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/cloudSandboxAgentHostContribution.test.ts index 8f366d6ca3d6c..b30fcaae60281 100644 --- a/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/cloudSandboxAgentHostContribution.test.ts +++ b/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/cloudSandboxAgentHostContribution.test.ts @@ -37,10 +37,12 @@ import { ISessionsProvider } from '../../../../../services/sessions/common/sessi import { ISessionsProvidersService } from '../../../../../services/sessions/browser/sessionsProvidersService.js'; import { CloudSandboxAgentHostContribution } from '../../browser/cloudSandboxAgentHostContribution.js'; import { IRemoteAgentHostConnectionCustomizationService } from '../../browser/remoteAgentHostConnectionCustomization.js'; -import { IRemoteAgentHostSessionsProviderConfig, RemoteAgentHostSessionsProvider } from '../../browser/remoteAgentHostSessionsProvider.js'; +import { IRemoteAgentHostSessionsProviderConfig, RemoteAgentHostSessionsProvider, type ISeedSessionsOptions } from '../../browser/remoteAgentHostSessionsProvider.js'; class StubProvider extends mock() { readonly seeded: IAgentSessionMetadata[] = []; + /** Raw ids seeded as provisional, mirroring the real provider's listing gate. */ + readonly withheld = new Set(); disposed = false; override readonly id: string; @@ -55,19 +57,39 @@ class StubProvider extends mock() { * the project backfill on an already-seeded session — that path is covered against the real * provider in `remoteAgentHostSessionsProvider.test.ts`. */ - override seedSessions(metas: readonly IAgentSessionMetadata[]): void { + override seedSessions(metas: readonly IAgentSessionMetadata[], options?: ISeedSessionsOptions): void { for (const meta of metas) { - if (!this.seeded.some(seen => seen.session.toString() === meta.session.toString())) { - this.seeded.push(meta); + if (this.seeded.some(seen => seen.session.toString() === meta.session.toString())) { + continue; + } + this.seeded.push(meta); + if (options?.provisional) { + this.withheld.add(AgentSession.id(meta.session)); } } } /** Surfaces each seed under the UI resource scheme, which is what keys the raw session id. */ override getSessions(): ISession[] { - return this.seeded.map(meta => upcastPartial({ + return this.seeded + .filter(meta => !this.withheld.has(AgentSession.id(meta.session))) + .map(meta => this._toSession(meta)); + } + + /** Reaches withheld seeds too, which is the whole point of the cache accessor. */ + override getCachedSession(rawId: string): ISession | undefined { + const meta = this.seeded.find(seen => AgentSession.id(seen.session) === rawId); + return meta ? this._toSession(meta) : undefined; + } + + override publishWithheldSession(rawId: string): void { + this.withheld.delete(rawId); + } + + private _toSession(meta: IAgentSessionMetadata): ISession { + return upcastPartial({ resource: URI.from({ scheme: 'agent-host-copilot', path: `/${AgentSession.id(meta.session)}` }), - })); + }); } override setConnectionStatus(): void { } diff --git a/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/remoteAgentHostSessionsProvider.test.ts b/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/remoteAgentHostSessionsProvider.test.ts index f2fa33ee6752a..e84706888a3dd 100644 --- a/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/remoteAgentHostSessionsProvider.test.ts +++ b/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/remoteAgentHostSessionsProvider.test.ts @@ -1312,6 +1312,122 @@ suite('RemoteAgentHostSessionsProvider', () => { }); })); + /** Force a session refresh the way the host does: a turn-complete action on a known session. */ + async function refreshViaTurnComplete(connection: MockAgentConnection, rawId: string): Promise { + connection.fireAction({ + channel: buildDefaultChatUri(AgentSession.uri('copilotcli', rawId).toString()), + action: { type: ActionType.ChatTurnComplete, turnId: 'turn-refresh', duration: 1 }, + serverSeq: 1, + origin: undefined, + } as ActionEnvelope); + await timeout(0); + } + + test('a provisional session survives a host listing that does not know it yet', () => runWithFakedTimers({ useFakeTimers: true }, async () => { + // Mission Control creates the sandbox task before the host materializes the session, so the + // first listing after connecting can legitimately omit it. Evicting it there drops the row + // the user is looking at and bounces the view to the new-session screen. + connection.addSession(createSession('other-1', { summary: 'Someone else' })); + const provider = createProvider(disposables, connection, { isWebPlatform: false, omitHostFromWorkspaceLabel: true }); + provider.seedSessions([{ + session: AgentSession.uri('copilotcli', 'provisional-1'), + startTime: 0, + modifiedTime: 0, + summary: 'Just provisioned', + }], { provisional: true }); + provider.publishWithheldSession('provisional-1'); + + await timeout(0); + const survivedUnknown = provider.getSessions().map(s => AgentSession.id(s.resource)).sort(); + + // Once the host knows it, it reconciles like any other session. + connection.addSession(createSession('provisional-1', { summary: 'Just provisioned' })); + await refreshViaTurnComplete(connection, 'other-1'); + const afterHostKnows = provider.getSessions().map(s => AgentSession.id(s.resource)).sort(); + + assert.deepStrictEqual({ survivedUnknown, afterHostKnows }, { + survivedUnknown: ['other-1', 'provisional-1'], + afterHostKnows: ['other-1', 'provisional-1'], + }); + })); + + test('a provisional session the host never lists is evicted once its grace period ends', () => runWithFakedTimers({ useFakeTimers: true }, async () => { + connection.addSession(createSession('other-1', { summary: 'Someone else' })); + const provider = createProvider(disposables, connection, { isWebPlatform: false, omitHostFromWorkspaceLabel: true }); + provider.seedSessions([{ + session: AgentSession.uri('copilotcli', 'never-listed'), + startTime: 0, + modifiedTime: 0, + summary: 'Never materialized', + }], { provisional: true }); + provider.publishWithheldSession('never-listed'); + await timeout(0); + + // The protection is bounded so a session the host will never list cannot become a + // permanent row that only a reload clears. + await timeout(RemoteAgentHostSessionsProvider.PROVISIONAL_GRACE_MS + 1); + await refreshViaTurnComplete(connection, 'other-1'); + + assert.deepStrictEqual(provider.getSessions().map(s => AgentSession.id(s.resource)), ['other-1']); + })); + + test('a withheld seed is cached and openable but stays out of the sessions list until published', () => runWithFakedTimers({ useFakeTimers: true }, async () => { + const provider = createProvider(disposables, new MockAgentConnection(), { noConnection: true, isWebPlatform: false, omitHostFromWorkspaceLabel: true }); + const announced: string[][] = []; + disposables.add(provider.onDidChangeSessions(e => announced.push(e.added.map(s => s.sessionId)))); + + provider.seedSessions([{ + session: AgentSession.uri('copilotcli', 'withheld-1'), + startTime: 0, + modifiedTime: 0, + summary: 'Withheld Session', + }], { provisional: true }); + + const whileWithheld = { + listed: provider.getSessions().map(s => AgentSession.id(s.resource)), + // Reachable by id so the caller that seeded it can still act on it, and openable by + // resource so a swap into it does not land on a session the UI cannot resolve. + cached: AgentSession.id(provider.getCachedSession('withheld-1')!.resource), + announced: announced.length, + }; + + provider.publishWithheldSession('withheld-1'); + + assert.deepStrictEqual({ + whileWithheld, + listedAfterPublish: provider.getSessions().map(s => AgentSession.id(s.resource)), + announcedAfterPublish: announced, + }, { + whileWithheld: { listed: [], cached: 'withheld-1', announced: 0 }, + listedAfterPublish: ['withheld-1'], + announcedAfterPublish: [['agenthost-localhost__4321:remote-localhost__4321-copilotcli:/withheld-1']], + }); + })); + + test('publishing with announce:false lists the session without firing its own event', () => runWithFakedTimers({ useFakeTimers: true }, async () => { + const provider = createProvider(disposables, new MockAgentConnection(), { noConnection: true, isWebPlatform: false, omitHostFromWorkspaceLabel: true }); + provider.seedSessions([{ + session: AgentSession.uri('copilotcli', 'withheld-2'), + startTime: 0, + modifiedTime: 0, + summary: 'Withheld Session', + }], { provisional: true }); + + const announced: string[][] = []; + disposables.add(provider.onDidChangeSessions(e => announced.push(e.added.map(s => s.sessionId)))); + // The caller fires its own replace event covering this session, so a second event here + // would list the new row a frame before the placeholder row disappears. + provider.publishWithheldSession('withheld-2', { announce: false }); + + assert.deepStrictEqual({ + listed: provider.getSessions().map(s => AgentSession.id(s.resource)), + announced, + }, { + listed: ['withheld-2'], + announced: [], + }); + })); + test('seedSessions never overwrites a project the host already reported', () => runWithFakedTimers({ useFakeTimers: true }, async () => { connection.addSession(createSession('authoritative-1', { summary: 'Authoritative', From 0b715a0b721400eedce874a547f1dda627dd4374 Mon Sep 17 00:00:00 2001 From: Osvaldo Ortega Date: Mon, 24 Aug 2026 19:47:53 -0700 Subject: [PATCH 2/4] Do not strand a provisional session that never finishes connecting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two ways the provisional guard could leave a session worse off than no guard at all. A failure after the seed — a rejected connect, or teardown while the sandbox wakes — returned before the caller received a provider, so nothing could publish the seed. Discovery does not rescue it either: a later pass finds the entry already in the cache and only backfills its project. The session existed remotely and was invisible until reload. It is now published on the way out, while the provider is still the registered one. The grace period also started at seed time, before connecting. Waking a sandbox can take minutes, so a slow wake burned the whole period before the host had said anything, and the first listing that omitted the session met an already-expired deadline — evicting it immediately and recreating the disappearance the guard exists to prevent. The clock now starts when a connected host first omits the session, which is what the grace is actually measuring. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../cloudSandboxAgentHostContribution.ts | 11 ++++++ .../remoteAgentHostSessionsProvider.ts | 38 ++++++++++++------- .../cloudSandboxAgentHostContribution.test.ts | 21 ++++++++++ .../remoteAgentHostSessionsProvider.test.ts | 35 ++++++++++++++++- 4 files changed, 91 insertions(+), 14 deletions(-) diff --git a/src/vs/sessions/contrib/providers/remoteAgentHost/browser/cloudSandboxAgentHostContribution.ts b/src/vs/sessions/contrib/providers/remoteAgentHost/browser/cloudSandboxAgentHostContribution.ts index 8c465b2b55b77..fbc9d911934de 100644 --- a/src/vs/sessions/contrib/providers/remoteAgentHost/browser/cloudSandboxAgentHostContribution.ts +++ b/src/vs/sessions/contrib/providers/remoteAgentHost/browser/cloudSandboxAgentHostContribution.ts @@ -335,6 +335,7 @@ export class CloudSandboxAgentHostContribution extends Disposable implements IWo throw new CancellationError(); } this._provisioning.add(address); + let seededProvider: RemoteAgentHostSessionsProvider | undefined; try { this._ensureProvider({ environmentId: created.environmentId, sessionId: created.sessionId, taskId: created.taskId, name }); @@ -359,6 +360,7 @@ export class CloudSandboxAgentHostContribution extends Disposable implements IWo // answer before it has materialized the session. provisional: true, }); + seededProvider = provider; await this.connect({ environmentId: created.environmentId, sessionId: created.sessionId, name }); @@ -377,6 +379,15 @@ export class CloudSandboxAgentHostContribution extends Disposable implements IWo throw new Error(`Provisioned sandbox session ${created.sessionId} did not surface on its provider`); } return { ...created, provider, session }; + } catch (error) { + // The task exists remotely from `createSession` onward, so a failure after the seed + // must not leave it withheld: the caller never receives a provider to publish it with, + // and a later discovery pass only backfills the entry already in the cache. Withheld + // and unreachable is worse than listed early. + if (seededProvider && this._providerInstances.get(address) === seededProvider) { + seededProvider.publishWithheldSession(created.sessionId); + } + throw error; } finally { this._provisioning.delete(address); } diff --git a/src/vs/sessions/contrib/providers/remoteAgentHost/browser/remoteAgentHostSessionsProvider.ts b/src/vs/sessions/contrib/providers/remoteAgentHost/browser/remoteAgentHostSessionsProvider.ts index b2c7f43e8962f..c8f8efc20cb5b 100644 --- a/src/vs/sessions/contrib/providers/remoteAgentHost/browser/remoteAgentHostSessionsProvider.ts +++ b/src/vs/sessions/contrib/providers/remoteAgentHost/browser/remoteAgentHostSessionsProvider.ts @@ -195,12 +195,16 @@ export class RemoteAgentHostSessionsProvider extends BaseAgentHostSessionsProvid * Raw id → deadline for locally provisioned sessions the host has not listed yet. * Mission Control creates the task before the host materializes the session, so the * first `listSessions` after connecting can legitimately omit it, and evicting it - * there drops the row the user is looking at. The deadline stops a session the host - * will never list from becoming a permanent row. + * there drops the row the user is looking at. + * + * The value is the deadline after which eviction resumes, or `undefined` while the + * clock has not started. It starts when a connected host first omits the session — + * not at seed time, because waking a sandbox can take minutes and would otherwise + * burn the whole grace period before the host has said anything at all. */ - private readonly _provisionalSessions = new Map(); + private readonly _provisionalSessions = new Map(); - /** How long a provisional session resists eviction while the host has not listed it. */ + /** How long a provisional session resists eviction after the host first omits it. */ static readonly PROVISIONAL_GRACE_MS = 2 * 60_000; constructor( @@ -404,7 +408,8 @@ export class RemoteAgentHostSessionsProvider extends BaseAgentHostSessionsProvid this._sessionCache.set(rawId, adapter); if (options?.provisional) { this._withheldSessions.add(rawId); - this._provisionalSessions.set(rawId, Date.now() + RemoteAgentHostSessionsProvider.PROVISIONAL_GRACE_MS); + // No deadline yet: the clock starts when the host first omits it. + this._provisionalSessions.set(rawId, undefined); } else { added.push(adapter); } @@ -415,23 +420,30 @@ export class RemoteAgentHostSessionsProvider extends BaseAgentHostSessionsProvid } protected override _isSessionEvictable(rawId: string): boolean { - const deadline = this._provisionalSessions.get(rawId); - if (deadline === undefined) { + if (!this._provisionalSessions.has(rawId)) { return true; } - if (Date.now() >= deadline) { - this._provisionalSessions.delete(rawId); - return true; + const deadline = this._provisionalSessions.get(rawId); + if (deadline === undefined || Date.now() < deadline) { + return false; } - return false; + this._provisionalSessions.delete(rawId); + return true; } protected override _onHostListedSessions(rawIds: ReadonlySet): void { if (this._provisionalSessions.size === 0) { return; } - for (const rawId of rawIds) { - this._provisionalSessions.delete(rawId); + for (const [rawId, deadline] of [...this._provisionalSessions]) { + if (rawIds.has(rawId)) { + // The host knows it, so it reconciles like any other session from here on. + this._provisionalSessions.delete(rawId); + } else if (deadline === undefined) { + // First time a connected host has omitted it: start the grace period now, so a + // slow sandbox wake does not consume it before the host has answered at all. + this._provisionalSessions.set(rawId, Date.now() + RemoteAgentHostSessionsProvider.PROVISIONAL_GRACE_MS); + } } } diff --git a/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/cloudSandboxAgentHostContribution.test.ts b/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/cloudSandboxAgentHostContribution.test.ts index b30fcaae60281..189f9f9315ec9 100644 --- a/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/cloudSandboxAgentHostContribution.test.ts +++ b/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/cloudSandboxAgentHostContribution.test.ts @@ -304,6 +304,27 @@ suite('CloudSandboxAgentHostContribution provisioning', () => { }); }); + test('publishes the seeded session when connecting fails, so it is not withheld forever', async () => { + // The task exists remotely once `createSession` returns. If a later failure leaves the seed + // withheld, nothing clears it: the caller never gets a provider to publish with, and a + // later discovery pass only backfills the entry already in the cache. + const harness = await createContribution(store, []); + harness.onConnect = async () => { + throw new Error('relay unavailable'); + }; + + await assert.rejects(() => harness.contribution.provisionSession({ prompt: 'fix it' }, CancellationToken.None)); + + const provider = harness.contribution.stubProviders.get(cloudSandboxAddress('env-new')); + assert.deepStrictEqual({ + withheld: [...(provider?.withheld ?? [])], + listed: provider?.getSessions().map(s => AgentSession.id(s.resource)), + }, { + withheld: [], + listed: ['sess-new'], + }); + }); + test('rejects when the feature is disabled while the sandbox is waking', async () => { // Connecting waits out the VM boot, which is long enough for the setting to change. // Returning a provider that teardown has already disposed would send into nothing. diff --git a/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/remoteAgentHostSessionsProvider.test.ts b/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/remoteAgentHostSessionsProvider.test.ts index e84706888a3dd..0b290f2bcde14 100644 --- a/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/remoteAgentHostSessionsProvider.test.ts +++ b/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/remoteAgentHostSessionsProvider.test.ts @@ -1363,12 +1363,45 @@ suite('RemoteAgentHostSessionsProvider', () => { provider.publishWithheldSession('never-listed'); await timeout(0); + // The first listing that omits it starts the clock; it is still protected here. + await refreshViaTurnComplete(connection, 'other-1'); + const afterFirstOmission = provider.getSessions().map(s => AgentSession.id(s.resource)).sort(); + // The protection is bounded so a session the host will never list cannot become a // permanent row that only a reload clears. await timeout(RemoteAgentHostSessionsProvider.PROVISIONAL_GRACE_MS + 1); await refreshViaTurnComplete(connection, 'other-1'); - assert.deepStrictEqual(provider.getSessions().map(s => AgentSession.id(s.resource)), ['other-1']); + assert.deepStrictEqual({ + afterFirstOmission, + afterGrace: provider.getSessions().map(s => AgentSession.id(s.resource)), + }, { + afterFirstOmission: ['never-listed', 'other-1'], + afterGrace: ['other-1'], + }); + })); + + test('a slow connection does not consume the grace period before the host answers', () => runWithFakedTimers({ useFakeTimers: true }, async () => { + // Waking a sandbox can take minutes. If the clock ran from the seed, the first listing + // would meet an already-expired deadline and evict immediately — the disappearance this + // guard exists to prevent. + connection.addSession(createSession('other-1', { summary: 'Someone else' })); + // Seeded before connecting, exactly as provisioning does it: no listing can arrive until + // the sandbox is awake. + const provider = createProvider(disposables, connection, { noConnection: true, isWebPlatform: false, omitHostFromWorkspaceLabel: true }); + provider.seedSessions([{ + session: AgentSession.uri('copilotcli', 'slow-wake'), + startTime: 0, + modifiedTime: 0, + summary: 'Slow to wake', + }], { provisional: true }); + provider.publishWithheldSession('slow-wake'); + + await timeout(RemoteAgentHostSessionsProvider.PROVISIONAL_GRACE_MS * 2); + provider.setConnection(connection); + await timeout(0); + + assert.deepStrictEqual(provider.getSessions().map(s => AgentSession.id(s.resource)).sort(), ['other-1', 'slow-wake']); })); test('a withheld seed is cached and openable but stays out of the sessions list until published', () => runWithFakedTimers({ useFakeTimers: true }, async () => { From 628e1c45692735b006ca99deed06414364606df9 Mon Sep 17 00:00:00 2001 From: Osvaldo Ortega Date: Mon, 24 Aug 2026 20:03:45 -0700 Subject: [PATCH 3/4] Trim comments to the repository's length limits Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../browser/baseAgentHostSessionsProvider.ts | 6 ++-- .../browser/copilotChatSessionsProvider.ts | 8 ++--- .../cloudSandboxAgentHostContribution.ts | 15 +++------ .../remoteAgentHostSessionsProvider.ts | 33 +++++++------------ .../cloudSandboxAgentHostContribution.test.ts | 5 ++- .../remoteAgentHostSessionsProvider.test.ts | 4 +-- 6 files changed, 23 insertions(+), 48 deletions(-) diff --git a/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts b/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts index aaf4112539565..8bdc6e7d05037 100644 --- a/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts +++ b/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts @@ -5309,10 +5309,8 @@ export abstract class BaseAgentHostSessionsProvider extends Disposable implement } /** - * Whether a cached session the host did not list may be evicted. Subclasses - * override this to protect a session that exists but that the host has not - * materialized yet — evicting one drops the row the user may be looking at, - * sending the view back to the new-session screen. + * Whether a cached session the host did not list may be evicted. Subclasses override this to + * protect a session that exists but that the host has not materialized yet. */ protected _isSessionEvictable(_rawId: string): boolean { return true; diff --git a/src/vs/sessions/contrib/providers/copilotChatSessions/browser/copilotChatSessionsProvider.ts b/src/vs/sessions/contrib/providers/copilotChatSessions/browser/copilotChatSessionsProvider.ts index de7b06d9597d6..3318c9cad038a 100644 --- a/src/vs/sessions/contrib/providers/copilotChatSessions/browser/copilotChatSessionsProvider.ts +++ b/src/vs/sessions/contrib/providers/copilotChatSessions/browser/copilotChatSessionsProvider.ts @@ -2108,8 +2108,7 @@ export class CopilotChatSessionsProvider extends Disposable implements ISessions try { provisioned = await this._getCloudSandboxContribution().provisionSession({ repoNwo, - // No `baseRef`: cloud sessions have no branch picker, and the Copilot app and CLI - // create sandbox tasks without one too, leaving the branch to Mission Control. + // No `baseRef`: cloud sessions have no branch picker; Mission Control chooses. prompt: options.query, }, CancellationToken.None); @@ -2118,10 +2117,7 @@ export class CopilotChatSessionsProvider extends Disposable implements ISessions const chat = provisioned.session.mainChat.get(); const committed = await provisioned.provider.sendRequest(provisioned.session.sessionId, chat.resource, options); - // Retire the placeholder only once the turn is dispatched. Swapping earlier — when the - // session merely exists — hands the UI a session it cannot open yet, and the view falls - // back to the new-session screen. Revealing without announcing lets the replace event - // swap the two rows in one pass, rather than listing both for a frame. + // Retire only once the turn is dispatched; swapping earlier bounces the view home. this._publishSandboxSession(provisioned, { announce: false }); this._retirePlaceholder(session, placeholder, committed); return committed; diff --git a/src/vs/sessions/contrib/providers/remoteAgentHost/browser/cloudSandboxAgentHostContribution.ts b/src/vs/sessions/contrib/providers/remoteAgentHost/browser/cloudSandboxAgentHostContribution.ts index fbc9d911934de..701cadd9ae34d 100644 --- a/src/vs/sessions/contrib/providers/remoteAgentHost/browser/cloudSandboxAgentHostContribution.ts +++ b/src/vs/sessions/contrib/providers/remoteAgentHost/browser/cloudSandboxAgentHostContribution.ts @@ -354,10 +354,8 @@ export class CloudSandboxAgentHostContribution extends Disposable implements IWo summary: name, ...(project ? { project } : {}), }], { - // The caller shows a placeholder row for this session until it sends the first - // turn, so listing the seed now would duplicate that row. This also protects the - // session from the first `listSessions` after connecting, which the host can - // answer before it has materialized the session. + // The caller still shows a placeholder row for this session, and the host may not + // have materialized it yet. provisional: true, }); seededProvider = provider; @@ -372,18 +370,15 @@ export class CloudSandboxAgentHostContribution extends Disposable implements IWo } // The adapter `seedSessions` created addresses the session by its raw id, which is the - // session id Mission Control just returned. Look it up through the cache rather than - // `getSessions`, which withholds it from listings until the caller publishes it. + // session id Mission Control just returned, and `getSessions` withholds it until + // the caller publishes it. const session = provider.getCachedSession(created.sessionId); if (!session) { throw new Error(`Provisioned sandbox session ${created.sessionId} did not surface on its provider`); } return { ...created, provider, session }; } catch (error) { - // The task exists remotely from `createSession` onward, so a failure after the seed - // must not leave it withheld: the caller never receives a provider to publish it with, - // and a later discovery pass only backfills the entry already in the cache. Withheld - // and unreachable is worse than listed early. + // The task exists remotely, and nothing else clears a withheld seed. if (seededProvider && this._providerInstances.get(address) === seededProvider) { seededProvider.publishWithheldSession(created.sessionId); } diff --git a/src/vs/sessions/contrib/providers/remoteAgentHost/browser/remoteAgentHostSessionsProvider.ts b/src/vs/sessions/contrib/providers/remoteAgentHost/browser/remoteAgentHostSessionsProvider.ts index c8f8efc20cb5b..9726d440bfb03 100644 --- a/src/vs/sessions/contrib/providers/remoteAgentHost/browser/remoteAgentHostSessionsProvider.ts +++ b/src/vs/sessions/contrib/providers/remoteAgentHost/browser/remoteAgentHostSessionsProvider.ts @@ -97,11 +97,9 @@ export interface ISessionSchemeAlias { /** Options for {@link RemoteAgentHostSessionsProvider.seedSessions}. */ export interface ISeedSessionsOptions { /** - * Mark the seeded session as freshly provisioned by this client, meaning the host - * may not have materialized it yet. It stays out of - * {@link RemoteAgentHostSessionsProvider.getSessions} until - * {@link RemoteAgentHostSessionsProvider.publishWithheldSession}, and resists - * eviction until the host lists it. Only affects sessions the call creates. + * Mark the seeded session as freshly provisioned by this client, so it stays out of + * {@link RemoteAgentHostSessionsProvider.getSessions} and resists eviction until the host + * lists it. Only affects sessions the call creates. */ readonly provisional?: boolean; } @@ -185,22 +183,15 @@ export class RemoteAgentHostSessionsProvider extends BaseAgentHostSessionsProvid private _unpublished = false; /** - * Seeded sessions kept out of {@link getSessions} because the caller is still showing - * a placeholder row for them; listing both would show the session twice until the - * sandbox wakes. They stay reachable by resource, so opening one still works. + * Seeded sessions kept out of {@link getSessions} because the caller is still showing a + * placeholder row for them. They stay reachable by resource, so opening one still works. */ private readonly _withheldSessions = new Set(); /** - * Raw id → deadline for locally provisioned sessions the host has not listed yet. - * Mission Control creates the task before the host materializes the session, so the - * first `listSessions` after connecting can legitimately omit it, and evicting it - * there drops the row the user is looking at. - * - * The value is the deadline after which eviction resumes, or `undefined` while the - * clock has not started. It starts when a connected host first omits the session — - * not at seed time, because waking a sandbox can take minutes and would otherwise - * burn the whole grace period before the host has said anything at all. + * Raw id → deadline after which eviction resumes, or `undefined` while the clock has not + * started. It starts when a connected host first omits the session, not at seed time, because + * waking a sandbox can take minutes. */ private readonly _provisionalSessions = new Map(); @@ -440,17 +431,15 @@ export class RemoteAgentHostSessionsProvider extends BaseAgentHostSessionsProvid // The host knows it, so it reconciles like any other session from here on. this._provisionalSessions.delete(rawId); } else if (deadline === undefined) { - // First time a connected host has omitted it: start the grace period now, so a - // slow sandbox wake does not consume it before the host has answered at all. + // Start the grace period now, so a slow wake does not consume it beforehand. this._provisionalSessions.set(rawId, Date.now() + RemoteAgentHostSessionsProvider.PROVISIONAL_GRACE_MS); } } } /** - * Look up a cached session by raw id, **including** ones withheld from - * {@link getSessions}. Callers that seeded a session and need to act on it before - * it is listed should use this rather than scanning {@link getSessions}. + * Look up a cached session by raw id, **including** ones withheld from {@link getSessions}, + * which callers that seeded a session need before it is listed. */ getCachedSession(rawId: string): ISession | undefined { return this._sessionCache.get(rawId); diff --git a/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/cloudSandboxAgentHostContribution.test.ts b/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/cloudSandboxAgentHostContribution.test.ts index 189f9f9315ec9..aee6ceb46dd11 100644 --- a/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/cloudSandboxAgentHostContribution.test.ts +++ b/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/cloudSandboxAgentHostContribution.test.ts @@ -305,9 +305,8 @@ suite('CloudSandboxAgentHostContribution provisioning', () => { }); test('publishes the seeded session when connecting fails, so it is not withheld forever', async () => { - // The task exists remotely once `createSession` returns. If a later failure leaves the seed - // withheld, nothing clears it: the caller never gets a provider to publish with, and a - // later discovery pass only backfills the entry already in the cache. + // The task exists remotely once `createSession` returns, and nothing else clears a + // withheld seed. const harness = await createContribution(store, []); harness.onConnect = async () => { throw new Error('relay unavailable'); diff --git a/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/remoteAgentHostSessionsProvider.test.ts b/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/remoteAgentHostSessionsProvider.test.ts index 0b290f2bcde14..5b29fdc4380b3 100644 --- a/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/remoteAgentHostSessionsProvider.test.ts +++ b/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/remoteAgentHostSessionsProvider.test.ts @@ -1324,9 +1324,7 @@ suite('RemoteAgentHostSessionsProvider', () => { } test('a provisional session survives a host listing that does not know it yet', () => runWithFakedTimers({ useFakeTimers: true }, async () => { - // Mission Control creates the sandbox task before the host materializes the session, so the - // first listing after connecting can legitimately omit it. Evicting it there drops the row - // the user is looking at and bounces the view to the new-session screen. + // The first listing after connecting can legitimately omit a just-minted session. connection.addSession(createSession('other-1', { summary: 'Someone else' })); const provider = createProvider(disposables, connection, { isWebPlatform: false, omitHostFromWorkspaceLabel: true }); provider.seedSessions([{ From d073cbf9b02ee273029bd7fcf3118ab32273a3b7 Mon Sep 17 00:00:00 2001 From: Osvaldo Ortega Date: Mon, 24 Aug 2026 20:57:36 -0700 Subject: [PATCH 4/4] Move the sandbox-only session handling into its own provider The withheld/provisional handling lived on RemoteAgentHostSessionsProvider, which SSH and tunnel connections also use, even though only the cloud sandbox calls any of it. Those connections carried ~95 lines of state and API that mean nothing to them. It now lives in CloudSandboxSessionsProvider, wired through the contribution's existing provider-construction seam, and RemoteAgentHostSessionsProvider is byte-identical to before this change. What stays in the base provider is the pair of eviction hooks, which default to "evictable" and a no-op. Eviction happens inside `_refreshSessions`, so a subclass can only influence it from there, and the base already made this exact exemption for its own pending session. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../copilotChatSessionsProvider.test.ts | 6 +- .../cloudSandboxAgentHostContribution.ts | 21 +- .../browser/cloudSandboxSessionsProvider.ts | 110 +++++++++ .../remoteAgentHostSessionsProvider.ts | 95 +------- .../cloudSandboxAgentHostContribution.test.ts | 27 ++- .../remoteAgentHostSessionsProvider.test.ts | 216 ++++++++++-------- 6 files changed, 261 insertions(+), 214 deletions(-) create mode 100644 src/vs/sessions/contrib/providers/remoteAgentHost/browser/cloudSandboxSessionsProvider.ts diff --git a/src/vs/sessions/contrib/providers/copilotChatSessions/test/browser/copilotChatSessionsProvider.test.ts b/src/vs/sessions/contrib/providers/copilotChatSessions/test/browser/copilotChatSessionsProvider.test.ts index 40fc9eb08c6ef..98acf549a1c21 100644 --- a/src/vs/sessions/contrib/providers/copilotChatSessions/test/browser/copilotChatSessionsProvider.test.ts +++ b/src/vs/sessions/contrib/providers/copilotChatSessions/test/browser/copilotChatSessionsProvider.test.ts @@ -39,7 +39,7 @@ import { ChatModelSource, GITHUB_REMOTE_FILE_SCHEME, IChat, ISession, SessionSta import { CloudSandboxEnabledSettingId, type ICloudSandboxCreateSessionRequest } from '../../../../../../platform/agentHost/common/cloudSandboxAgentHost.js'; import { RemoteAgentHostsEnabledSettingId } from '../../../../../../platform/agentHost/common/remoteAgentHostService.js'; import { CloudSandboxAgentHostContribution, type ICloudSandboxProvisionedSession } from '../../../remoteAgentHost/browser/cloudSandboxAgentHostContribution.js'; -import { RemoteAgentHostSessionsProvider } from '../../../remoteAgentHost/browser/remoteAgentHostSessionsProvider.js'; +import { CloudSandboxSessionsProvider } from '../../../remoteAgentHost/browser/cloudSandboxSessionsProvider.js'; import { ChatConfiguration, ChatPermissionLevel } from '../../../../../../workbench/contrib/chat/common/constants.js'; import { CopilotChatSessionsProvider, COPILOT_PROVIDER_ID, CopilotCloudSessionType, ICopilotChatSession } from '../../browser/copilotChatSessionsProvider.js'; import { ILogService, NullLogService } from '../../../../../../platform/log/common/log.js'; @@ -1874,10 +1874,10 @@ suite('CopilotChatSessionsProvider', () => { environmentId: 'env-new', session: sandboxSession, published, - provider: upcastPartial({ + provider: upcastPartial({ sendRequest: sendRequest ?? (async () => committed), publishWithheldSession: (rawId: string) => { published.push(rawId); }, - }) as RemoteAgentHostSessionsProvider, + }) as CloudSandboxSessionsProvider, }; } diff --git a/src/vs/sessions/contrib/providers/remoteAgentHost/browser/cloudSandboxAgentHostContribution.ts b/src/vs/sessions/contrib/providers/remoteAgentHost/browser/cloudSandboxAgentHostContribution.ts index 701cadd9ae34d..202a887c1c847 100644 --- a/src/vs/sessions/contrib/providers/remoteAgentHost/browser/cloudSandboxAgentHostContribution.ts +++ b/src/vs/sessions/contrib/providers/remoteAgentHost/browser/cloudSandboxAgentHostContribution.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ // Surfaces Copilot cloud sandbox (copilot-developer-cli) sessions as native agent-host sessions. -// Owns a RemoteAgentHostSessionsProvider per sandbox environment, connects on demand via +// Owns a CloudSandboxSessionsProvider per sandbox environment, connects on demand via // CloudSandboxAgentHostService, and wires the live connection to the provider so the native session // machinery can enumerate and render the host's sessions. @@ -44,7 +44,8 @@ import { CloudSandboxReadOnlySessionHandler } from './cloudSandboxReadOnlySessio import { IAgentHostFilterService } from '../../../../services/agentHostFilter/common/agentHostFilter.js'; import { ISession } from '../../../../services/sessions/common/session.js'; import { ISessionsProvidersService } from '../../../../services/sessions/browser/sessionsProvidersService.js'; -import { ISessionSchemeAlias, IRemoteAgentHostSessionsProviderConfig, RemoteAgentHostSessionsProvider } from './remoteAgentHostSessionsProvider.js'; +import { ISessionSchemeAlias, IRemoteAgentHostSessionsProviderConfig } from './remoteAgentHostSessionsProvider.js'; +import { CloudSandboxSessionsProvider } from './cloudSandboxSessionsProvider.js'; import { IRemoteAgentHostConnectionCustomizationService } from './remoteAgentHostConnectionCustomization.js'; import { createCloudSandboxConnectionCustomization, isCloudSandboxConnectionAddress } from './cloudSandboxConnectionCustomization.js'; import { watchForIncompatibleNotifications } from './remoteHostOptions.js'; @@ -91,7 +92,7 @@ function discoveredSessionProject(repoName: string | undefined): IAgentSessionMe * for the caller to send the first turn into it. */ export interface ICloudSandboxProvisionedSession extends ICloudSandboxCreatedSession { - readonly provider: RemoteAgentHostSessionsProvider; + readonly provider: CloudSandboxSessionsProvider; readonly session: ISession; } @@ -99,7 +100,7 @@ export class CloudSandboxAgentHostContribution extends Disposable implements IWo static readonly ID = 'workbench.contrib.cloudSandboxAgentHost'; /** Provider instances keyed by connection address (`cloudsandbox:`). */ - private readonly _providerInstances = new Map(); + private readonly _providerInstances = new Map(); private readonly _providerStores = this._register(new DisposableMap()); /** Environment metadata keyed by connection address, for on-demand reconnect. */ private readonly _environments = new Map(); @@ -335,7 +336,7 @@ export class CloudSandboxAgentHostContribution extends Disposable implements IWo throw new CancellationError(); } this._provisioning.add(address); - let seededProvider: RemoteAgentHostSessionsProvider | undefined; + let seededProvider: CloudSandboxSessionsProvider | undefined; try { this._ensureProvider({ environmentId: created.environmentId, sessionId: created.sessionId, taskId: created.taskId, name }); @@ -345,7 +346,7 @@ export class CloudSandboxAgentHostContribution extends Disposable implements IWo } const now = Date.now(); const project = discoveredSessionProject(request.repoNwo); - provider.seedSessions([{ + provider.seedProvisionalSession({ // Same identity discovery seeds under: Mission Control issues the session as // `ahp-session:/` and the host lists that id back, so this reconciles on connect. session: AgentSession.uri(CLOUD_SANDBOX_AGENT_PROVIDER, created.sessionId), @@ -353,10 +354,6 @@ export class CloudSandboxAgentHostContribution extends Disposable implements IWo modifiedTime: now, summary: name, ...(project ? { project } : {}), - }], { - // The caller still shows a placeholder row for this session, and the host may not - // have materialized it yet. - provisional: true, }); seededProvider = provider; @@ -679,8 +676,8 @@ export class CloudSandboxAgentHostContribution extends Disposable implements IWo /** * Provider construction seam so tests can observe each provider's configuration. */ - protected _instantiateProvider(config: IRemoteAgentHostSessionsProviderConfig): RemoteAgentHostSessionsProvider { - return this._instantiationService.createInstance(RemoteAgentHostSessionsProvider, config); + protected _instantiateProvider(config: IRemoteAgentHostSessionsProviderConfig): CloudSandboxSessionsProvider { + return this._instantiationService.createInstance(CloudSandboxSessionsProvider, config); } /** Wire each live connection to its provider so session enumeration runs. */ diff --git a/src/vs/sessions/contrib/providers/remoteAgentHost/browser/cloudSandboxSessionsProvider.ts b/src/vs/sessions/contrib/providers/remoteAgentHost/browser/cloudSandboxSessionsProvider.ts new file mode 100644 index 0000000000000..f226e0cda678a --- /dev/null +++ b/src/vs/sessions/contrib/providers/remoteAgentHost/browser/cloudSandboxSessionsProvider.ts @@ -0,0 +1,110 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { AgentSession, type IAgentSessionMetadata } from '../../../../../platform/agentHost/common/agent.js'; +import type { ISession } from '../../../../services/sessions/common/session.js'; +import { RemoteAgentHostSessionsProvider } from './remoteAgentHostSessionsProvider.js'; + +/** + * Sessions provider for a Copilot cloud sandbox. + * + * Adds the handling for sessions this client provisioned but the host has not materialized yet: + * Mission Control mints the session id and returns it before the sandbox is even awake, so such a + * session is real, addressable, and unknown to the host all at once. + */ +export class CloudSandboxSessionsProvider extends RemoteAgentHostSessionsProvider { + + /** + * Provisional sessions kept out of {@link getSessions} because the caller is still showing a + * placeholder row for them. They stay reachable by resource, so opening one still works. + */ + private readonly _withheldSessions = new Set(); + + /** + * Raw id → deadline after which eviction resumes, or `undefined` while the clock has not + * started. It starts when a connected host first omits the session, not at seed time, because + * waking a sandbox can take minutes. + */ + private readonly _provisionalSessions = new Map(); + + /** How long a provisional session resists eviction after the host first omits it. */ + static readonly PROVISIONAL_GRACE_MS = 2 * 60_000; + + /** + * Seed a session this client just provisioned. It is cached so a later discovery pass + * reconciles against it rather than adding a second entry, but stays out of the sessions list + * until {@link publishWithheldSession} and resists eviction until the host lists it. + */ + seedProvisionalSession(rawMeta: IAgentSessionMetadata): void { + const meta = this._adoptSessionMeta(rawMeta); + const rawId = AgentSession.id(meta.session); + if (this._sessionCache.has(rawId)) { + return; + } + this._sessionCache.set(rawId, this.createAdapter(meta)); + this._withheldSessions.add(rawId); + // No deadline yet: the clock starts when the host first omits it. + this._provisionalSessions.set(rawId, undefined); + } + + /** + * Reveal a session seeded by {@link seedProvisionalSession}, so {@link getSessions} returns it. + * + * Pass `announce: false` when the caller immediately fires its own change event covering this + * session: the list re-reads {@link getSessions} on any change, so a single event can both drop + * a placeholder row and reveal this one. + */ + publishWithheldSession(rawId: string, options?: { announce?: boolean }): void { + if (!this._withheldSessions.delete(rawId)) { + return; + } + const session = this._sessionCache.get(rawId); + if (session && options?.announce !== false) { + this._onDidChangeSessions.fire({ added: [session], removed: [], changed: [] }); + } + } + + /** + * Look up a cached session by raw id, **including** ones withheld from {@link getSessions}, + * which callers that seeded a session need before it is listed. + */ + getCachedSession(rawId: string): ISession | undefined { + return this._sessionCache.get(rawId); + } + + override getSessions(): ISession[] { + const sessions = super.getSessions(); + return this._withheldSessions.size === 0 + ? sessions + : sessions.filter(session => !this._withheldSessions.has(AgentSession.id(session.resource))); + } + + protected override _isSessionEvictable(rawId: string): boolean { + if (!this._provisionalSessions.has(rawId)) { + return true; + } + const deadline = this._provisionalSessions.get(rawId); + if (deadline === undefined || Date.now() < deadline) { + return false; + } + this._provisionalSessions.delete(rawId); + return true; + } + + protected override _onHostListedSessions(rawIds: ReadonlySet): void { + if (this._provisionalSessions.size === 0) { + return; + } + for (const [rawId, deadline] of [...this._provisionalSessions]) { + if (rawIds.has(rawId)) { + // The host knows it, so it reconciles like any other session from here on. + this._provisionalSessions.delete(rawId); + } else if (deadline === undefined) { + // Start the grace period now, so a slow wake does not consume it beforehand. + this._provisionalSessions.set(rawId, Date.now() + CloudSandboxSessionsProvider.PROVISIONAL_GRACE_MS); + } + } + } +} diff --git a/src/vs/sessions/contrib/providers/remoteAgentHost/browser/remoteAgentHostSessionsProvider.ts b/src/vs/sessions/contrib/providers/remoteAgentHost/browser/remoteAgentHostSessionsProvider.ts index 9726d440bfb03..b48c0741e6b77 100644 --- a/src/vs/sessions/contrib/providers/remoteAgentHost/browser/remoteAgentHostSessionsProvider.ts +++ b/src/vs/sessions/contrib/providers/remoteAgentHost/browser/remoteAgentHostSessionsProvider.ts @@ -94,16 +94,6 @@ export interface ISessionSchemeAlias { readonly backend: string; } -/** Options for {@link RemoteAgentHostSessionsProvider.seedSessions}. */ -export interface ISeedSessionsOptions { - /** - * Mark the seeded session as freshly provisioned by this client, so it stays out of - * {@link RemoteAgentHostSessionsProvider.getSessions} and resists eviction until the host - * lists it. Only affects sessions the call creates. - */ - readonly provisional?: boolean; -} - /** * Sessions provider for a remote agent host connection. A thin subclass of * {@link BaseAgentHostSessionsProvider} that adds the connection-lifecycle @@ -182,21 +172,6 @@ export class RemoteAgentHostSessionsProvider extends BaseAgentHostSessionsProvid */ private _unpublished = false; - /** - * Seeded sessions kept out of {@link getSessions} because the caller is still showing a - * placeholder row for them. They stay reachable by resource, so opening one still works. - */ - private readonly _withheldSessions = new Set(); - - /** - * Raw id → deadline after which eviction resumes, or `undefined` while the clock has not - * started. It starts when a connected host first omits the session, not at seed time, because - * waking a sandbox can take minutes. - */ - private readonly _provisionalSessions = new Map(); - - /** How long a provisional session resists eviction after the host first omits it. */ - static readonly PROVISIONAL_GRACE_MS = 2 * 60_000; constructor( config: IRemoteAgentHostSessionsProviderConfig, @@ -289,13 +264,7 @@ export class RemoteAgentHostSessionsProvider extends BaseAgentHostSessionsProvid } override getSessions(): ISession[] { - if (this._unpublished) { - return []; - } - const sessions = super.getSessions(); - return this._withheldSessions.size === 0 - ? sessions - : sessions.filter(session => !this._withheldSessions.has(AgentSession.id(session.resource))); + return this._unpublished ? [] : super.getSessions(); } protected override mapWorkingDirectoryUri(uri: URI): URI { @@ -380,7 +349,7 @@ export class RemoteAgentHostSessionsProvider extends BaseAgentHostSessionsProvid * filling it in on a later pass is what makes retrying worth anything. Opening a seeded session * triggers `connectOnDemand`, after which `_refreshSessions` reconciles against the host. */ - seedSessions(metas: readonly IAgentSessionMetadata[], options?: ISeedSessionsOptions): void { + seedSessions(metas: readonly IAgentSessionMetadata[]): void { const added: ISession[] = []; const changed: ISession[] = []; for (const rawMeta of metas) { @@ -397,71 +366,13 @@ export class RemoteAgentHostSessionsProvider extends BaseAgentHostSessionsProvid } const adapter = this.createAdapter(meta); this._sessionCache.set(rawId, adapter); - if (options?.provisional) { - this._withheldSessions.add(rawId); - // No deadline yet: the clock starts when the host first omits it. - this._provisionalSessions.set(rawId, undefined); - } else { - added.push(adapter); - } + added.push(adapter); } if (added.length > 0 || changed.length > 0) { this._onDidChangeSessions.fire({ added, removed: [], changed }); } } - protected override _isSessionEvictable(rawId: string): boolean { - if (!this._provisionalSessions.has(rawId)) { - return true; - } - const deadline = this._provisionalSessions.get(rawId); - if (deadline === undefined || Date.now() < deadline) { - return false; - } - this._provisionalSessions.delete(rawId); - return true; - } - - protected override _onHostListedSessions(rawIds: ReadonlySet): void { - if (this._provisionalSessions.size === 0) { - return; - } - for (const [rawId, deadline] of [...this._provisionalSessions]) { - if (rawIds.has(rawId)) { - // The host knows it, so it reconciles like any other session from here on. - this._provisionalSessions.delete(rawId); - } else if (deadline === undefined) { - // Start the grace period now, so a slow wake does not consume it beforehand. - this._provisionalSessions.set(rawId, Date.now() + RemoteAgentHostSessionsProvider.PROVISIONAL_GRACE_MS); - } - } - } - - /** - * Look up a cached session by raw id, **including** ones withheld from {@link getSessions}, - * which callers that seeded a session need before it is listed. - */ - getCachedSession(rawId: string): ISession | undefined { - return this._sessionCache.get(rawId); - } - - /** - * Reveal a session seeded as `provisional`, so {@link getSessions} starts returning it. - * - * Pass `announce: false` when the caller immediately fires its own change event covering - * this session: the list re-reads {@link getSessions} on any change, so a single event can - * both drop a placeholder row and reveal this one. - */ - publishWithheldSession(rawId: string, options?: { announce?: boolean }): void { - if (!this._withheldSessions.delete(rawId)) { - return; - } - const session = this._sessionCache.get(rawId); - if (session && options?.announce !== false) { - this._onDidChangeSessions.fire({ added: [session], removed: [], changed: [] }); - } - } - /** * Map a host-reported session URI onto the UI scheme, so the session routes to the agent's * content provider. The raw id is preserved, so cache keys are unaffected. diff --git a/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/cloudSandboxAgentHostContribution.test.ts b/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/cloudSandboxAgentHostContribution.test.ts index aee6ceb46dd11..fdbcd3a674efb 100644 --- a/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/cloudSandboxAgentHostContribution.test.ts +++ b/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/cloudSandboxAgentHostContribution.test.ts @@ -37,9 +37,10 @@ import { ISessionsProvider } from '../../../../../services/sessions/common/sessi import { ISessionsProvidersService } from '../../../../../services/sessions/browser/sessionsProvidersService.js'; import { CloudSandboxAgentHostContribution } from '../../browser/cloudSandboxAgentHostContribution.js'; import { IRemoteAgentHostConnectionCustomizationService } from '../../browser/remoteAgentHostConnectionCustomization.js'; -import { IRemoteAgentHostSessionsProviderConfig, RemoteAgentHostSessionsProvider, type ISeedSessionsOptions } from '../../browser/remoteAgentHostSessionsProvider.js'; +import { IRemoteAgentHostSessionsProviderConfig } from '../../browser/remoteAgentHostSessionsProvider.js'; +import { CloudSandboxSessionsProvider } from '../../browser/cloudSandboxSessionsProvider.js'; -class StubProvider extends mock() { +class StubProvider extends mock() { readonly seeded: IAgentSessionMetadata[] = []; /** Raw ids seeded as provisional, mirroring the real provider's listing gate. */ readonly withheld = new Set(); @@ -57,18 +58,22 @@ class StubProvider extends mock() { * the project backfill on an already-seeded session — that path is covered against the real * provider in `remoteAgentHostSessionsProvider.test.ts`. */ - override seedSessions(metas: readonly IAgentSessionMetadata[], options?: ISeedSessionsOptions): void { + override seedSessions(metas: readonly IAgentSessionMetadata[]): void { for (const meta of metas) { - if (this.seeded.some(seen => seen.session.toString() === meta.session.toString())) { - continue; - } - this.seeded.push(meta); - if (options?.provisional) { - this.withheld.add(AgentSession.id(meta.session)); + if (!this.seeded.some(seen => seen.session.toString() === meta.session.toString())) { + this.seeded.push(meta); } } } + override seedProvisionalSession(meta: IAgentSessionMetadata): void { + if (this.seeded.some(seen => seen.session.toString() === meta.session.toString())) { + return; + } + this.seeded.push(meta); + this.withheld.add(AgentSession.id(meta.session)); + } + /** Surfaces each seed under the UI resource scheme, which is what keys the raw session id. */ override getSessions(): ISession[] { return this.seeded @@ -104,10 +109,10 @@ class StubProvider extends mock() { class TestCloudSandboxContribution extends CloudSandboxAgentHostContribution { readonly stubProviders = new Map(); - protected override _instantiateProvider(config: IRemoteAgentHostSessionsProviderConfig): RemoteAgentHostSessionsProvider { + protected override _instantiateProvider(config: IRemoteAgentHostSessionsProviderConfig): CloudSandboxSessionsProvider { const stub = new StubProvider(config); this.stubProviders.set(config.address, stub); - return stub as unknown as RemoteAgentHostSessionsProvider; + return stub as unknown as CloudSandboxSessionsProvider; } } diff --git a/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/remoteAgentHostSessionsProvider.test.ts b/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/remoteAgentHostSessionsProvider.test.ts index 5b29fdc4380b3..35e75880ea092 100644 --- a/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/remoteAgentHostSessionsProvider.test.ts +++ b/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/remoteAgentHostSessionsProvider.test.ts @@ -33,8 +33,9 @@ import { IChatService, type ChatSendResult, type IChatSendRequestOptions } from import { IChatSessionsService } from '../../../../../../workbench/contrib/chat/common/chatSessionsService.js'; import { ILanguageModelsService } from '../../../../../../workbench/contrib/chat/common/languageModels.js'; import { ISessionChangeEvent } from '../../../../../services/sessions/common/sessionsProvider.js'; -import { ChatModelSource, SessionStatus } from '../../../../../services/sessions/common/session.js'; +import { ChatModelSource, SessionStatus, type ISession } from '../../../../../services/sessions/common/session.js'; import { RemoteAgentHostSessionsProvider, type IRemoteAgentHostSessionsProviderConfig } from '../../browser/remoteAgentHostSessionsProvider.js'; +import { CloudSandboxSessionsProvider } from '../../browser/cloudSandboxSessionsProvider.js'; import { ILabelService } from '../../../../../../platform/label/common/label.js'; import { ILogService, NullLogService } from '../../../../../../platform/log/common/log.js'; import { IGitHubService } from '../../../../github/browser/githubService.js'; @@ -194,7 +195,7 @@ function createSession(id: string, opts?: { provider?: string; summary?: string; }; } -function createProvider(disposables: DisposableStore, connection: MockAgentConnection, overrides?: { address?: string; preferenceKey?: string; connectionName?: string | undefined; sendRequest?: (resource: URI, message: string, options?: IChatSendRequestOptions) => Promise; openSession?: boolean; storageService?: IStorageService; noConnection?: boolean; isWebPlatform?: boolean; workspaceTrusted?: boolean; omitHostFromWorkspaceLabel?: boolean; workspaceTypeIcon?: ThemeIcon }): RemoteAgentHostSessionsProvider { +function createProvider(disposables: DisposableStore, connection: MockAgentConnection, overrides?: { address?: string; preferenceKey?: string; connectionName?: string | undefined; sendRequest?: (resource: URI, message: string, options?: IChatSendRequestOptions) => Promise; openSession?: boolean; storageService?: IStorageService; noConnection?: boolean; isWebPlatform?: boolean; workspaceTrusted?: boolean; omitHostFromWorkspaceLabel?: boolean; workspaceTypeIcon?: ThemeIcon; ctor?: typeof RemoteAgentHostSessionsProvider }): RemoteAgentHostSessionsProvider { const instantiationService = disposables.add(new TestInstantiationService()); instantiationService.stub(IFileDialogService, {}); @@ -253,11 +254,12 @@ function createProvider(disposables: DisposableStore, connection: MockAgentConne workspaceTypeIcon: overrides?.workspaceTypeIcon, }; + const baseCtor = overrides?.ctor ?? RemoteAgentHostSessionsProvider; const providerCtor = overrides?.isWebPlatform !== undefined - ? class extends RemoteAgentHostSessionsProvider { + ? class extends baseCtor { protected override get isWebPlatform(): boolean { return overrides.isWebPlatform!; } } - : RemoteAgentHostSessionsProvider; + : baseCtor; const provider = disposables.add(instantiationService.createInstance(providerCtor, config)); if (!overrides?.noConnection) { provider.setConnection(connection); @@ -1312,6 +1314,95 @@ suite('RemoteAgentHostSessionsProvider', () => { }); })); + test('seedSessions never overwrites a project the host already reported', () => runWithFakedTimers({ useFakeTimers: true }, async () => { + connection.addSession(createSession('authoritative-1', { + summary: 'Authoritative', + project: { uri: URI.parse('vscode-agent-host://localhost__4321/home/user/real?_ah%3DeyJzY2hlbWUiOiJmaWxlIn0'), displayName: 'real-repo' }, + })); + const provider = createProvider(disposables, connection, { isWebPlatform: false, omitHostFromWorkspaceLabel: true }); + provider.getSessions(); + await timeout(0); + + provider.seedSessions([{ + session: AgentSession.uri('copilotcli', 'authoritative-1'), + startTime: 0, + modifiedTime: 0, + summary: 'Stale Seed', + project: { uri: URI.parse('https://github.com/someone/stale'), displayName: 'someone/stale' }, + }]); + + assert.deepStrictEqual({ + label: provider.getSessions()[0].workspace.get()?.label, + title: provider.getSessions()[0].title.get(), + }, { + label: 'real-repo', + title: 'Authoritative', + }); + })); + + test('non-web: omitHostFromWorkspaceLabel drops the [host] suffix so sessions group by repository', () => runWithFakedTimers({ useFakeTimers: true }, async () => { + const projectUri = URI.parse('vscode-agent-host://localhost__4321/home/user/vscode?_ah%3DeyJzY2hlbWUiOiJmaWxlIn0'); + connection.addSession(createSession('sandbox-1', { + summary: 'Sandbox Session', + project: { uri: projectUri, displayName: 'osortega/simple-server' }, + })); + + const provider = createProvider(disposables, connection, { isWebPlatform: false, omitHostFromWorkspaceLabel: true }); + provider.getSessions(); + await timeout(0); + + assert.deepStrictEqual({ + session: provider.getSessions()[0].workspace.get()?.label, + browsed: provider.resolveWorkspace(URI.parse('vscode-agent-host://localhost__4321/home/user/project'))?.label, + }, { + session: 'osortega/simple-server', + browsed: 'project', + }); + })); + + test('workspaceTypeIcon reaches the built workspace, and is absent by default', () => runWithFakedTimers({ useFakeTimers: true }, async () => { + connection.addSession(createSession('sandbox-icon', { + summary: 'Sandbox Session', + project: { uri: URI.parse('https://github.com/osortega/simple-server'), displayName: 'osortega/simple-server' }, + })); + + const withIcon = createProvider(disposables, connection, { isWebPlatform: false, workspaceTypeIcon: Codicon.package }); + const withoutIcon = createProvider(disposables, new MockAgentConnection(), { isWebPlatform: false, noConnection: true }); + withIcon.getSessions(); + await timeout(0); + + assert.deepStrictEqual({ + declared: withIcon.getSessions()[0].workspace.get()?.typeIcon?.id, + // Other hosts leave it unset so the icon stays inferred from the workspace shape. + browsed: withoutIcon.resolveWorkspace(URI.parse('vscode-agent-host://localhost__4321/home/user/project'))?.typeIcon, + }, { + declared: Codicon.package.id, + browsed: undefined, + }); + })); + +}); + +suite('CloudSandboxSessionsProvider provisional sessions', () => { + + const disposables = new DisposableStore(); + let connection: MockAgentConnection; + + setup(() => { + connection = new MockAgentConnection(); + }); + + teardown(() => { + disposables.clear(); + }); + + ensureNoDisposablesAreLeakedInTestSuite(); + + /** The sandbox provider, built on the same mocks as the remote provider it extends. */ + function createSandboxProvider(store: DisposableStore, conn: MockAgentConnection, overrides?: { noConnection?: boolean; isWebPlatform?: boolean; omitHostFromWorkspaceLabel?: boolean }): CloudSandboxSessionsProvider { + return createProvider(store, conn, { ...overrides, ctor: CloudSandboxSessionsProvider }) as CloudSandboxSessionsProvider; + } + /** Force a session refresh the way the host does: a turn-complete action on a known session. */ async function refreshViaTurnComplete(connection: MockAgentConnection, rawId: string): Promise { connection.fireAction({ @@ -1326,22 +1417,22 @@ suite('RemoteAgentHostSessionsProvider', () => { test('a provisional session survives a host listing that does not know it yet', () => runWithFakedTimers({ useFakeTimers: true }, async () => { // The first listing after connecting can legitimately omit a just-minted session. connection.addSession(createSession('other-1', { summary: 'Someone else' })); - const provider = createProvider(disposables, connection, { isWebPlatform: false, omitHostFromWorkspaceLabel: true }); - provider.seedSessions([{ + const provider = createSandboxProvider(disposables, connection, { isWebPlatform: false, omitHostFromWorkspaceLabel: true }); + provider.seedProvisionalSession({ session: AgentSession.uri('copilotcli', 'provisional-1'), startTime: 0, modifiedTime: 0, summary: 'Just provisioned', - }], { provisional: true }); + }); provider.publishWithheldSession('provisional-1'); await timeout(0); - const survivedUnknown = provider.getSessions().map(s => AgentSession.id(s.resource)).sort(); + const survivedUnknown = provider.getSessions().map((s: ISession) => AgentSession.id(s.resource)).sort(); // Once the host knows it, it reconciles like any other session. connection.addSession(createSession('provisional-1', { summary: 'Just provisioned' })); await refreshViaTurnComplete(connection, 'other-1'); - const afterHostKnows = provider.getSessions().map(s => AgentSession.id(s.resource)).sort(); + const afterHostKnows = provider.getSessions().map((s: ISession) => AgentSession.id(s.resource)).sort(); assert.deepStrictEqual({ survivedUnknown, afterHostKnows }, { survivedUnknown: ['other-1', 'provisional-1'], @@ -1351,28 +1442,28 @@ suite('RemoteAgentHostSessionsProvider', () => { test('a provisional session the host never lists is evicted once its grace period ends', () => runWithFakedTimers({ useFakeTimers: true }, async () => { connection.addSession(createSession('other-1', { summary: 'Someone else' })); - const provider = createProvider(disposables, connection, { isWebPlatform: false, omitHostFromWorkspaceLabel: true }); - provider.seedSessions([{ + const provider = createSandboxProvider(disposables, connection, { isWebPlatform: false, omitHostFromWorkspaceLabel: true }); + provider.seedProvisionalSession({ session: AgentSession.uri('copilotcli', 'never-listed'), startTime: 0, modifiedTime: 0, summary: 'Never materialized', - }], { provisional: true }); + }); provider.publishWithheldSession('never-listed'); await timeout(0); // The first listing that omits it starts the clock; it is still protected here. await refreshViaTurnComplete(connection, 'other-1'); - const afterFirstOmission = provider.getSessions().map(s => AgentSession.id(s.resource)).sort(); + const afterFirstOmission = provider.getSessions().map((s: ISession) => AgentSession.id(s.resource)).sort(); // The protection is bounded so a session the host will never list cannot become a // permanent row that only a reload clears. - await timeout(RemoteAgentHostSessionsProvider.PROVISIONAL_GRACE_MS + 1); + await timeout(CloudSandboxSessionsProvider.PROVISIONAL_GRACE_MS + 1); await refreshViaTurnComplete(connection, 'other-1'); assert.deepStrictEqual({ afterFirstOmission, - afterGrace: provider.getSessions().map(s => AgentSession.id(s.resource)), + afterGrace: provider.getSessions().map((s: ISession) => AgentSession.id(s.resource)), }, { afterFirstOmission: ['never-listed', 'other-1'], afterGrace: ['other-1'], @@ -1386,36 +1477,36 @@ suite('RemoteAgentHostSessionsProvider', () => { connection.addSession(createSession('other-1', { summary: 'Someone else' })); // Seeded before connecting, exactly as provisioning does it: no listing can arrive until // the sandbox is awake. - const provider = createProvider(disposables, connection, { noConnection: true, isWebPlatform: false, omitHostFromWorkspaceLabel: true }); - provider.seedSessions([{ + const provider = createSandboxProvider(disposables, connection, { noConnection: true, isWebPlatform: false, omitHostFromWorkspaceLabel: true }); + provider.seedProvisionalSession({ session: AgentSession.uri('copilotcli', 'slow-wake'), startTime: 0, modifiedTime: 0, summary: 'Slow to wake', - }], { provisional: true }); + }); provider.publishWithheldSession('slow-wake'); - await timeout(RemoteAgentHostSessionsProvider.PROVISIONAL_GRACE_MS * 2); + await timeout(CloudSandboxSessionsProvider.PROVISIONAL_GRACE_MS * 2); provider.setConnection(connection); await timeout(0); - assert.deepStrictEqual(provider.getSessions().map(s => AgentSession.id(s.resource)).sort(), ['other-1', 'slow-wake']); + assert.deepStrictEqual(provider.getSessions().map((s: ISession) => AgentSession.id(s.resource)).sort(), ['other-1', 'slow-wake']); })); test('a withheld seed is cached and openable but stays out of the sessions list until published', () => runWithFakedTimers({ useFakeTimers: true }, async () => { - const provider = createProvider(disposables, new MockAgentConnection(), { noConnection: true, isWebPlatform: false, omitHostFromWorkspaceLabel: true }); + const provider = createSandboxProvider(disposables, new MockAgentConnection(), { noConnection: true, isWebPlatform: false, omitHostFromWorkspaceLabel: true }); const announced: string[][] = []; disposables.add(provider.onDidChangeSessions(e => announced.push(e.added.map(s => s.sessionId)))); - provider.seedSessions([{ + provider.seedProvisionalSession({ session: AgentSession.uri('copilotcli', 'withheld-1'), startTime: 0, modifiedTime: 0, summary: 'Withheld Session', - }], { provisional: true }); + }); const whileWithheld = { - listed: provider.getSessions().map(s => AgentSession.id(s.resource)), + listed: provider.getSessions().map((s: ISession) => AgentSession.id(s.resource)), // Reachable by id so the caller that seeded it can still act on it, and openable by // resource so a swap into it does not land on a session the UI cannot resolve. cached: AgentSession.id(provider.getCachedSession('withheld-1')!.resource), @@ -1426,7 +1517,7 @@ suite('RemoteAgentHostSessionsProvider', () => { assert.deepStrictEqual({ whileWithheld, - listedAfterPublish: provider.getSessions().map(s => AgentSession.id(s.resource)), + listedAfterPublish: provider.getSessions().map((s: ISession) => AgentSession.id(s.resource)), announcedAfterPublish: announced, }, { whileWithheld: { listed: [], cached: 'withheld-1', announced: 0 }, @@ -1436,13 +1527,13 @@ suite('RemoteAgentHostSessionsProvider', () => { })); test('publishing with announce:false lists the session without firing its own event', () => runWithFakedTimers({ useFakeTimers: true }, async () => { - const provider = createProvider(disposables, new MockAgentConnection(), { noConnection: true, isWebPlatform: false, omitHostFromWorkspaceLabel: true }); - provider.seedSessions([{ + const provider = createSandboxProvider(disposables, new MockAgentConnection(), { noConnection: true, isWebPlatform: false, omitHostFromWorkspaceLabel: true }); + provider.seedProvisionalSession({ session: AgentSession.uri('copilotcli', 'withheld-2'), startTime: 0, modifiedTime: 0, summary: 'Withheld Session', - }], { provisional: true }); + }); const announced: string[][] = []; disposables.add(provider.onDidChangeSessions(e => announced.push(e.added.map(s => s.sessionId)))); @@ -1451,7 +1542,7 @@ suite('RemoteAgentHostSessionsProvider', () => { provider.publishWithheldSession('withheld-2', { announce: false }); assert.deepStrictEqual({ - listed: provider.getSessions().map(s => AgentSession.id(s.resource)), + listed: provider.getSessions().map((s: ISession) => AgentSession.id(s.resource)), announced, }, { listed: ['withheld-2'], @@ -1459,71 +1550,4 @@ suite('RemoteAgentHostSessionsProvider', () => { }); })); - test('seedSessions never overwrites a project the host already reported', () => runWithFakedTimers({ useFakeTimers: true }, async () => { - connection.addSession(createSession('authoritative-1', { - summary: 'Authoritative', - project: { uri: URI.parse('vscode-agent-host://localhost__4321/home/user/real?_ah%3DeyJzY2hlbWUiOiJmaWxlIn0'), displayName: 'real-repo' }, - })); - const provider = createProvider(disposables, connection, { isWebPlatform: false, omitHostFromWorkspaceLabel: true }); - provider.getSessions(); - await timeout(0); - - provider.seedSessions([{ - session: AgentSession.uri('copilotcli', 'authoritative-1'), - startTime: 0, - modifiedTime: 0, - summary: 'Stale Seed', - project: { uri: URI.parse('https://github.com/someone/stale'), displayName: 'someone/stale' }, - }]); - - assert.deepStrictEqual({ - label: provider.getSessions()[0].workspace.get()?.label, - title: provider.getSessions()[0].title.get(), - }, { - label: 'real-repo', - title: 'Authoritative', - }); - })); - - test('non-web: omitHostFromWorkspaceLabel drops the [host] suffix so sessions group by repository', () => runWithFakedTimers({ useFakeTimers: true }, async () => { - const projectUri = URI.parse('vscode-agent-host://localhost__4321/home/user/vscode?_ah%3DeyJzY2hlbWUiOiJmaWxlIn0'); - connection.addSession(createSession('sandbox-1', { - summary: 'Sandbox Session', - project: { uri: projectUri, displayName: 'osortega/simple-server' }, - })); - - const provider = createProvider(disposables, connection, { isWebPlatform: false, omitHostFromWorkspaceLabel: true }); - provider.getSessions(); - await timeout(0); - - assert.deepStrictEqual({ - session: provider.getSessions()[0].workspace.get()?.label, - browsed: provider.resolveWorkspace(URI.parse('vscode-agent-host://localhost__4321/home/user/project'))?.label, - }, { - session: 'osortega/simple-server', - browsed: 'project', - }); - })); - - test('workspaceTypeIcon reaches the built workspace, and is absent by default', () => runWithFakedTimers({ useFakeTimers: true }, async () => { - connection.addSession(createSession('sandbox-icon', { - summary: 'Sandbox Session', - project: { uri: URI.parse('https://github.com/osortega/simple-server'), displayName: 'osortega/simple-server' }, - })); - - const withIcon = createProvider(disposables, connection, { isWebPlatform: false, workspaceTypeIcon: Codicon.package }); - const withoutIcon = createProvider(disposables, new MockAgentConnection(), { isWebPlatform: false, noConnection: true }); - withIcon.getSessions(); - await timeout(0); - - assert.deepStrictEqual({ - declared: withIcon.getSessions()[0].workspace.get()?.typeIcon?.id, - // Other hosts leave it unset so the icon stays inferred from the workspace shape. - browsed: withoutIcon.resolveWorkspace(URI.parse('vscode-agent-host://localhost__4321/home/user/project'))?.typeIcon, - }, { - declared: Codicon.package.id, - browsed: undefined, - }); - })); - });