From 631adccc2cf698d30949920140117da1bcfe2760 Mon Sep 17 00:00:00 2001 From: Osvaldo Ortega Date: Fri, 4 Sep 2026 15:33:11 -0700 Subject: [PATCH 1/2] Agent Host changes for agents/cloud-sandbox-ux-enhancement --- .../sessions/browser/parts/chatGroupView.ts | 29 ++- src/vs/sessions/browser/parts/chatView.ts | 8 + .../media/remoteHostUnavailableEmptyState.css | 1 + .../parts/remoteHostUnavailableEmptyState.ts | 10 +- .../browser/parts/sessionRemoteConnection.ts | 96 ++++++--- .../common/agentHostSessionsProvider.ts | 17 ++ .../sessions/contrib/chat/browser/chatView.ts | 18 +- .../cloudSandboxAgentHostContribution.ts | 186 +++++++++-------- .../cloudSandboxReadOnlySessionHandler.ts | 7 +- .../remoteAgentHostSessionsProvider.ts | 45 +++-- .../cloudSandboxAgentHostContribution.test.ts | 147 +++++++++++++- .../remoteAgentHostSessionsProvider.test.ts | 30 ++- .../test/browser/chatGroupsView.test.ts | 187 +++++++++++++++++- ...remoteHostUnavailableEmptyState.fixture.ts | 17 ++ 14 files changed, 645 insertions(+), 153 deletions(-) diff --git a/src/vs/sessions/browser/parts/chatGroupView.ts b/src/vs/sessions/browser/parts/chatGroupView.ts index a7b02e35bb1cdc..46eecd46948e19 100644 --- a/src/vs/sessions/browser/parts/chatGroupView.ts +++ b/src/vs/sessions/browser/parts/chatGroupView.ts @@ -230,30 +230,41 @@ export class ChatGroupView extends Disposable implements ISerializableView { if (archived) { const action = getChatSessionArchiveActionPresentation(this._archiveActionWording.read(reader)).unarchive; return { - message: localize('sessionReadOnlyBanner.archived', "Archived sessions are read-only."), - action: { - label: action.title.value, - run: () => this._commandService.executeCommand(UNARCHIVE_SESSION_COMMAND_ID, context.session), + archived: true, + content: { + message: localize('sessionReadOnlyBanner.archived', "Archived sessions are read-only."), + action: { + label: action.title.value, + run: () => this._commandService.executeCommand(UNARCHIVE_SESSION_COMMAND_ID, context.session), + }, }, }; } - return { message: localize('sessionReadOnlyBanner.message', "This chat is read-only") }; + return { archived: false, content: { message: localize('sessionReadOnlyBanner.message', "This chat is read-only") } }; }); const surface = derived(reader => { const readOnly = readOnlyContent.read(reader); - if (readOnly) { - return { banner: readOnly, recovery: undefined }; + if (readOnly?.archived) { + return { banner: readOnly.content, recovery: undefined }; } + // Keep the banner while history loads to avoid flashing the centered recovery state. const view = currentView.read(reader); - const recovery = view?.hasVisibleTranscriptContent.read(reader) + const transcriptSettled = view === undefined || !view.isLoadingTranscript.read(reader); + const recovery = !transcriptSettled || view?.hasVisibleTranscriptContent.read(reader) ? undefined : this._connection.recoveryContent.read(reader); if (recovery) { return { banner: undefined, recovery }; } - return { banner: this._connection.bannerContent.read(reader), recovery: undefined }; + // Explain connection-related read-only state before falling back to the generic notice. + const connectionBanner = this._connection.bannerContent.read(reader); + if (connectionBanner) { + return { banner: connectionBanner, recovery: undefined }; + } + + return { banner: readOnly?.content, recovery: undefined }; }); this._contextDisposables.add(autorun(reader => { diff --git a/src/vs/sessions/browser/parts/chatView.ts b/src/vs/sessions/browser/parts/chatView.ts index 4474612d85caa9..75c02ac4875ab7 100644 --- a/src/vs/sessions/browser/parts/chatView.ts +++ b/src/vs/sessions/browser/parts/chatView.ts @@ -72,6 +72,14 @@ export abstract class AbstractChatView extends Disposable implements ISerializab */ readonly hasVisibleTranscriptContent: IObservable = constObservable(false); + /** + * Whether this view is still resolving its chat model, during which + * {@link hasVisibleTranscriptContent} is not yet meaningful — it reads `false` for a transcript + * that simply has not arrived yet as well as for one that does not exist. Views that never load + * a model report `false`, since for them the answer is already final. + */ + readonly isLoadingTranscript: IObservable = constObservable(false); + /** * Show the given chat in this view. The default implementation is a * no-op; subclasses that host a chat widget (e.g. `ChatView`) override diff --git a/src/vs/sessions/browser/parts/media/remoteHostUnavailableEmptyState.css b/src/vs/sessions/browser/parts/media/remoteHostUnavailableEmptyState.css index 719e7ecc78bfdc..f9b4273c6264f6 100644 --- a/src/vs/sessions/browser/parts/media/remoteHostUnavailableEmptyState.css +++ b/src/vs/sessions/browser/parts/media/remoteHostUnavailableEmptyState.css @@ -18,6 +18,7 @@ } .remote-host-unavailable-empty-state.hidden, +.remote-host-unavailable-empty-state-description.hidden, .remote-host-unavailable-empty-state-progress.hidden, .remote-host-unavailable-empty-state-action.hidden, .remote-host-unavailable-empty-state-auto-connect.hidden { diff --git a/src/vs/sessions/browser/parts/remoteHostUnavailableEmptyState.ts b/src/vs/sessions/browser/parts/remoteHostUnavailableEmptyState.ts index ffcb92b8a7f15d..8f5125f8ec8ba2 100644 --- a/src/vs/sessions/browser/parts/remoteHostUnavailableEmptyState.ts +++ b/src/vs/sessions/browser/parts/remoteHostUnavailableEmptyState.ts @@ -15,7 +15,8 @@ import { defaultButtonStyles, defaultCheckboxStyles } from '../../../platform/th export interface IRemoteHostUnavailableEmptyStateContent { readonly title: string; - readonly description: string; + /** Omitted when the title already says everything, leaving the title and action to speak. */ + readonly description?: string; readonly progress?: string; readonly action?: { readonly label: string; @@ -59,7 +60,7 @@ export class RemoteHostUnavailableEmptyState extends Disposable { icon.appendChild(renderIcon(Codicon.debugDisconnect)); this._title = dom.append(this.domNode, dom.$('h2.remote-host-unavailable-empty-state-title')); - this._description = dom.append(this.domNode, dom.$('p.remote-host-unavailable-empty-state-description')); + this._description = dom.append(this.domNode, dom.$('p.remote-host-unavailable-empty-state-description.hidden')); this._progress = dom.append(this.domNode, dom.$('p.remote-host-unavailable-empty-state-progress.hidden')); // Connect progress changes while the user waits (waiting → download // percentage), so announce it politely rather than leaving it silent. @@ -85,6 +86,8 @@ export class RemoteHostUnavailableEmptyState extends Disposable { this._autoConnectListener.clear(); this.domNode.classList.toggle('hidden', !content); if (!content) { + this._description.textContent = ''; + this._description.classList.add('hidden'); this._progress.textContent = ''; this._progress.classList.add('hidden'); this._actionContainer.classList.add('hidden'); @@ -98,7 +101,8 @@ export class RemoteHostUnavailableEmptyState extends Disposable { this.domNode.setAttribute('aria-label', content.title); this._title.textContent = content.title; - this._description.textContent = content.description; + this._description.textContent = content.description ?? ''; + this._description.classList.toggle('hidden', !content.description); this._progress.textContent = content.progress ?? ''; this._progress.classList.toggle('hidden', !content.progress); if (!content.action) { diff --git a/src/vs/sessions/browser/parts/sessionRemoteConnection.ts b/src/vs/sessions/browser/parts/sessionRemoteConnection.ts index e9b094ecb2ac3a..9a8a79ffe66a7a 100644 --- a/src/vs/sessions/browser/parts/sessionRemoteConnection.ts +++ b/src/vs/sessions/browser/parts/sessionRemoteConnection.ts @@ -10,9 +10,10 @@ import { Disposable, MutableDisposable } from '../../../base/common/lifecycle.js import { autorun, derived, derivedObservableWithCache, IObservable, IReader, observableSignal, observableValue, transaction } from '../../../base/common/observable.js'; import { localize } from '../../../nls.js'; import { ILogService } from '../../../platform/log/common/log.js'; -import { isAgentHostProvider } from '../../common/agentHostSessionsProvider.js'; +import { IAgentHostConnectionLabels, isAgentHostProvider } from '../../common/agentHostSessionsProvider.js'; import { SessionRemoteConnectionFailureReason, SessionRemoteConnectionStatus } from '../../services/sessions/common/session.js'; import { IActiveSession } from '../../services/sessions/common/sessionsManagement.js'; +import { ISessionsProvider } from '../../services/sessions/common/sessionsProvider.js'; import { ISessionsProvidersService } from '../../services/sessions/browser/sessionsProvidersService.js'; import { IRemoteHostUnavailableEmptyStateContent } from './remoteHostUnavailableEmptyState.js'; import { ISessionReadOnlyBannerContent } from './sessionReadOnlyBanner.js'; @@ -63,6 +64,13 @@ export class SessionRemoteConnection extends Disposable { * `connect()` joins an in-flight dial rather than starting a second one. */ private readonly _autoConnected = observableValue(this, undefined); + /** + * The session a connect has already been started for, however it ended. Distinguishes a host + * that has never been dialled — where the action reads as "Connect" — from one that was tried + * and did not come up, where it reads as "Retry". Latched separately from {@link _attempt}, + * which clears once an attempt settles and so cannot answer "has this been tried at all". + */ + private readonly _connectAttempted = observableValue(this, undefined); /** * Whether the host should be started without waiting for a click. Gated on a * stopped host the provider can start, and on not having tried yet — one @@ -170,6 +178,7 @@ export class SessionRemoteConnection extends Disposable { this._session.set(session, tx); this._attempt.set(undefined, tx); this._autoConnected.set(undefined, tx); + this._connectAttempted.set(undefined, tx); }); } @@ -197,7 +206,10 @@ export class SessionRemoteConnection extends Disposable { const statusBefore = session.remoteConnectionStatus?.get(); this._logService.info(`[SessionRemoteConnection] connect: starting ${provider.remoteAddress ?? provider.id}, statusBefore=${statusBefore?.kind}`); - this._attempt.set({ kind: 'active', session, message: undefined, statusBefore }, undefined); + transaction(tx => { + this._attempt.set({ kind: 'active', session, message: undefined, statusBefore }, tx); + this._connectAttempted.set(session, tx); + }); const remoteAddress = provider.remoteAddress; if (remoteAddress && provider.onDidReportConnectProgress) { this._progressListener.value = provider.onDidReportConnectProgress(progress => { @@ -255,7 +267,26 @@ export class SessionRemoteConnection extends Disposable { : status; } - private _getRemoteHostConnectProgress(session: IActiveSession, status: SessionRemoteConnectionStatus | undefined, reader: IReader): string | undefined { + private _getConnectionLabels(provider: ISessionsProvider | undefined): IAgentHostConnectionLabels { + if (provider && isAgentHostProvider(provider) && provider.connectionLabels) { + return provider.connectionLabels; + } + const hostLabel = provider?.label ?? localize('sessionRemoteHost.unknown', "The remote host"); + return { + unavailableTitle: localize('sessionRemoteHost.disconnectedTitle', "Cannot Connect to {0}", hostLabel), + unavailableDescription: localize('sessionRemoteHost.disconnected', "Cannot reach {0}.", hostLabel), + unavailable: localize('sessionRemoteHost.disconnected', "Cannot reach {0}.", hostLabel), + connectingTitle: localize('sessionRemoteHost.connectingTitle', "Connecting to {0}", hostLabel), + connectingDescription: localize('sessionRemoteHost.startingDescription', "Starting {0}.", hostLabel), + connecting: localize('sessionRemoteHost.waitingForConnection', "Waiting for agent host connection..."), + reconnecting: localize('sessionRemoteHost.reconnecting', "Reconnecting to {0}...", hostLabel), + reconnectingIn: seconds => localize('sessionRemoteHost.reconnectingIn', "Reconnecting to {0} in {1}s", hostLabel, seconds), + incompatibleTitle: localize('sessionRemoteHost.incompatibleTitle', "Cannot Connect to {0}", hostLabel), + incompatible: localize('sessionRemoteHost.incompatibleDescription', "{0} is incompatible with this version of Visual Studio Code.", hostLabel), + }; + } + + private _getRemoteHostConnectProgress(session: IActiveSession, status: SessionRemoteConnectionStatus | undefined, labels: IAgentHostConnectionLabels, reader: IReader): string | undefined { const attempt = this._attempt.read(reader); if (attempt?.kind !== 'active' || attempt.session !== session || status?.kind === 'connected') { return undefined; @@ -271,9 +302,7 @@ export class SessionRemoteConnection extends Disposable { const settling = status?.kind === 'connecting' || status?.kind === 'reconnecting' || isSameRemoteConnectionStatus(status, attempt.statusBefore); - return settling - ? localize('sessionRemoteHost.waitingForConnection', "Waiting for agent host connection...") - : undefined; + return settling ? labels.connecting : undefined; } private _getRemoteHostUnavailableContent(reader: IReader): IRemoteHostUnavailableEmptyStateContent | undefined { @@ -285,16 +314,17 @@ export class SessionRemoteConnection extends Disposable { const status = this._getEffectiveStatus(reader); const provider = this._sessionsProvidersService.getProvider(session.providerId); const hostLabel = provider?.label ?? localize('sessionRemoteHost.unknown', "The remote host"); + const labels = this._getConnectionLabels(provider); if (status?.kind === 'connected') { return undefined; } if (status?.kind === 'incompatible') { return { - title: localize('sessionRemoteHost.incompatibleTitle', "Cannot Connect to {0}", hostLabel), - description: localize('sessionRemoteHost.incompatibleDescription', "{0} is incompatible with this version of Visual Studio Code.", hostLabel), + title: labels.incompatibleTitle, + description: labels.incompatible, }; } - const progressMessage = this._getRemoteHostConnectProgress(session, status, reader); + const progressMessage = this._getRemoteHostConnectProgress(session, status, labels, reader); const autoConnectPending = this._autoConnectPending.read(reader); const canStartHost = !!provider && isAgentHostProvider(provider) && !!provider.connect; const autoConnect = canStartHost && provider.autoConnect @@ -310,11 +340,12 @@ export class SessionRemoteConnection extends Disposable { && attempt.session === session && attempt.statusBefore?.kind === 'disconnected' && attempt.statusBefore.reason === SessionRemoteConnectionFailureReason.HostNotRunning); - if (progressMessage || autoConnectPending) { + // Include externally started connects; reconnecting keeps its delayed countdown banner. + if (progressMessage || autoConnectPending || status?.kind === 'connecting') { return { - title: localize('sessionRemoteHost.connectingTitle', "Connecting to {0}", hostLabel), - description: localize('sessionRemoteHost.startingDescription', "Starting {0}.", hostLabel), - progress: progressMessage ?? localize('sessionRemoteHost.waitingForConnection', "Waiting for agent host connection..."), + title: labels.connectingTitle, + description: labels.connectingDescription, + progress: progressMessage ?? labels.connecting, autoConnect: startedFromStoppedHost ? autoConnect : undefined, }; } @@ -335,14 +366,14 @@ export class SessionRemoteConnection extends Disposable { }; } return { - title: localize('sessionRemoteHost.disconnectedTitle', "Cannot Connect to {0}", hostLabel), - description: localize('sessionRemoteHost.disconnected', "Cannot reach {0}.", hostLabel), + title: labels.unavailableTitle, + description: labels.unavailableDescription, // An unreachable host is often transient — a dropped tunnel, a sleeping // machine — so offer a manual retry even though there is nothing local // to start and so nothing to do automatically. action: canStartHost ? { - label: localize('sessionRemoteHost.retry', "Retry"), + label: this._connectActionLabel(session, reader), run: () => this.connect(), } : undefined, @@ -358,17 +389,18 @@ export class SessionRemoteConnection extends Disposable { const status = this._getEffectiveStatus(reader); const provider = this._sessionsProvidersService.getProvider(session.providerId); const hostLabel = provider?.label ?? localize('sessionRemoteHost.unknown', "The remote host"); - const progressMessage = this._getRemoteHostConnectProgress(session, status, reader); + const labels = this._getConnectionLabels(provider); + const progressMessage = this._getRemoteHostConnectProgress(session, status, labels, reader); // Mirror the recovery state: while an automatic start is pending the // action must not appear, even for the frame before the attempt registers. - if (progressMessage || this._autoConnectPending.read(reader)) { + if (progressMessage || this._autoConnectPending.read(reader) || status?.kind === 'connecting') { return { icon: Codicon.sync, - message: progressMessage ?? localize('sessionRemoteHost.waitingForConnection', "Waiting for agent host connection..."), + message: progressMessage ?? labels.connecting, }; } - if (!status || status.kind === 'connected' || status.kind === 'connecting') { + if (!status || status.kind === 'connected') { return undefined; } @@ -377,7 +409,7 @@ export class SessionRemoteConnection extends Disposable { // so this banner is the only place the incompatibility can be explained. return { icon: Codicon.debugDisconnect, - message: localize('sessionRemoteHost.incompatibleDescription', "{0} is incompatible with this version of Visual Studio Code.", hostLabel), + message: labels.incompatible, }; } @@ -386,13 +418,11 @@ export class SessionRemoteConnection extends Disposable { return this._reconnectingBannerVisible.read(reader) ? { icon: Codicon.sync, - message: seconds === undefined - ? localize('sessionRemoteHost.reconnecting', "Reconnecting to {0}...", hostLabel) - : localize('sessionRemoteHost.reconnectingIn', "Reconnecting to {0} in {1}s", hostLabel, seconds), + message: seconds === undefined ? labels.reconnecting : labels.reconnectingIn(seconds), // The banner is a live region, so the per-second countdown would // otherwise queue an announcement every tick. Keep the spoken // text stable and let only the visible text count down. - ariaLabel: localize('sessionRemoteHost.reconnecting', "Reconnecting to {0}...", hostLabel), + ariaLabel: labels.reconnecting, action: seconds !== undefined && provider && isAgentHostProvider(provider) && provider.reconnectNow ? { label: localize('sessionRemoteHost.tryNow', "Try Now"), @@ -416,15 +446,27 @@ export class SessionRemoteConnection extends Disposable { } return { icon: Codicon.debugDisconnect, - message: localize('sessionRemoteHost.disconnected', "Cannot reach {0}.", hostLabel), + message: labels.unavailable, // A dropped tunnel or a sleeping machine is usually transient, so a // manual retry is worth offering even with nothing local to start. action: provider && isAgentHostProvider(provider) && provider.connect ? { - label: localize('sessionRemoteHost.retry', "Retry"), + label: this._connectActionLabel(session, reader), run: () => this.connect(), } : undefined, }; } + + /** + * The wording for the action that establishes the connection. A host that has never been + * dialled from this view is offered a plain "Connect": presenting it as a retry would imply an + * attempt the user never made, and for hosts that must be resumed rather than merely reached, + * the first dial is a deliberate choice rather than a recovery. + */ + private _connectActionLabel(session: IActiveSession, reader: IReader): string { + return this._connectAttempted.read(reader) === session + ? localize('sessionRemoteHost.retry', "Retry") + : localize('sessionRemoteHost.connect', "Connect"); + } } diff --git a/src/vs/sessions/common/agentHostSessionsProvider.ts b/src/vs/sessions/common/agentHostSessionsProvider.ts index fa99397f2e590e..03431dea0596e0 100644 --- a/src/vs/sessions/common/agentHostSessionsProvider.ts +++ b/src/vs/sessions/common/agentHostSessionsProvider.ts @@ -46,6 +46,20 @@ export interface IAgentHostAutoConnect { setEnabled(enabled: boolean): void; } +/** Localized labels shared by connection banners and recovery screens. */ +export interface IAgentHostConnectionLabels { + readonly unavailableTitle: string; + readonly unavailableDescription?: string; + readonly unavailable: string; + readonly connectingTitle: string; + readonly connectingDescription?: string; + readonly connecting: string; + readonly reconnecting: string; + reconnectingIn(seconds: number): string; + readonly incompatibleTitle: string; + readonly incompatible: string; +} + /** * Declares that a provider is one of many interchangeable members of a single * user-facing host. Members collapse into one `IAgentHostFilterEntry` that @@ -153,6 +167,9 @@ export interface IAgentHostSessionsProvider extends ISessionsProvider { */ readonly autoConnect?: IAgentHostAutoConnect; + /** Optional labels for providers whose display name does not name the host. */ + readonly connectionLabels?: IAgentHostConnectionLabels; + /** * When `true`, the workspace picker keeps this provider's browse * action(s) enabled even while {@link connectionStatus} reports diff --git a/src/vs/sessions/contrib/chat/browser/chatView.ts b/src/vs/sessions/contrib/chat/browser/chatView.ts index 901307e62e6a05..a44de4ec665b3d 100644 --- a/src/vs/sessions/contrib/chat/browser/chatView.ts +++ b/src/vs/sessions/contrib/chat/browser/chatView.ts @@ -181,6 +181,7 @@ export class ChatView extends AbstractChatView { private readonly _currentChatResourceObs = observableValue(this, undefined); private readonly _currentSessionObs = observableValue(this, undefined); override readonly hasVisibleTranscriptContent = observableValue(this, false); + override readonly isLoadingTranscript = observableValue(this, false); private _historyKey: string | undefined; /** Whether this view currently represents the active session. */ @@ -470,6 +471,15 @@ export class ChatView extends AbstractChatView { this._loadChat(resource, this._currentSessionObs.get()); } + /** + * Drives the widget's loading affordance and the observable mirror of it together, so a + * consumer reading {@link isLoadingTranscript} can never disagree with what the widget shows. + */ + private _setLoading(isLoading: boolean): void { + this._widget.setLoading(isLoading); + this.isLoadingTranscript.set(isLoading, undefined); + } + private _loadChat(resource: URI, session: ISession | undefined, previousChatResource?: URI, previousSession?: ISession): void { // Cancel any in-flight load for the previous chat and start a fresh one. this._loadCts.value?.cancel(); @@ -479,7 +489,7 @@ export class ChatView extends AbstractChatView { const cts = new CancellationTokenSource(); this._loadCts.value = cts; const token = cts.token; - this._widget.setLoading(true); + this._setLoading(true); // Capture the input draft before the load window opens so text typed // during loading is preserved when the model binds. See #325323. @@ -494,7 +504,7 @@ export class ChatView extends AbstractChatView { this.sessionOpenTelemetryService.modelBindFailed(session.resource, resource); } if (isCurrentChat && isCurrentLoad) { - this._widget.setLoading(false); + this._setLoading(false); } this.logService.trace(`[ChatView] setChat abandoned uri=${resource.toString()}`); return; @@ -507,7 +517,7 @@ export class ChatView extends AbstractChatView { if (widgetViewState) { this._widget.restoreViewState(widgetViewState); } - this._widget.setLoading(false); + this._setLoading(false); if (session) { this.sessionOpenTelemetryService.modelBound(session.resource, resource); } @@ -526,7 +536,7 @@ export class ChatView extends AbstractChatView { if (isEqual(this._currentChatResource, resource) && this._loadCts.value === cts) { // might have changed while we were waiting, only reset if it is still the same this._currentChatResource = undefined; this._currentChatResourceObs.set(undefined, undefined); - this._widget.setLoading(false); + this._setLoading(false); } if (!token.isCancellationRequested && this._loadCts.value === cts && session) { this.sessionOpenTelemetryService.modelBindFailed(session.resource, resource); diff --git a/src/vs/sessions/contrib/providers/remoteAgentHost/browser/cloudSandboxAgentHostContribution.ts b/src/vs/sessions/contrib/providers/remoteAgentHost/browser/cloudSandboxAgentHostContribution.ts index aecff61bf5893f..d31209dc19426b 100644 --- a/src/vs/sessions/contrib/providers/remoteAgentHost/browser/cloudSandboxAgentHostContribution.ts +++ b/src/vs/sessions/contrib/providers/remoteAgentHost/browser/cloudSandboxAgentHostContribution.ts @@ -44,7 +44,7 @@ import { IWorkbenchContribution } from '../../../../../workbench/common/contribu import { ChatSessionsExtensions, IAsyncChatSessionActivationRegistry, IChatSessionsService } from '../../../../../workbench/contrib/chat/common/chatSessionsService.js'; import { CloudSandboxReadOnlySessionHandler } from './cloudSandboxReadOnlySessionHandler.js'; import { IAgentHostFilterService } from '../../../../services/agentHostFilter/common/agentHostFilter.js'; -import { IAgentHostGroup } from '../../../../common/agentHostSessionsProvider.js'; +import { IAgentHostConnectionLabels, IAgentHostGroup } from '../../../../common/agentHostSessionsProvider.js'; import { ISession } from '../../../../services/sessions/common/session.js'; import { ISessionsProvidersService } from '../../../../services/sessions/browser/sessionsProvidersService.js'; import { IAgentHostSessionSchemeAlias } from '../../../../../platform/agentHost/common/agentHostConnectionsService.js'; @@ -78,6 +78,18 @@ const CLOUD_SANDBOX_HOST_GROUP: IAgentHostGroup = { connectable: false, }; +/** Names the environment rather than the task used as the provider's display name. */ +const CLOUD_SANDBOX_CONNECTION_LABELS: IAgentHostConnectionLabels = { + unavailableTitle: localize('cloudSandbox.offlineTitle', "Environment Offline"), + unavailable: localize('cloudSandbox.offline', "Environment offline."), + connectingTitle: localize('cloudSandbox.connectingTitle', "Connecting to the Environment"), + connecting: localize('cloudSandbox.connecting', "Connecting..."), + reconnecting: localize('cloudSandbox.reconnecting', "Reconnecting..."), + reconnectingIn: seconds => localize('cloudSandbox.reconnectingIn', "Reconnecting in {0}s", seconds), + incompatibleTitle: localize('cloudSandbox.incompatibleTitle', "Cannot Connect to the Environment"), + incompatible: localize('cloudSandbox.incompatible', "This environment is incompatible with this version of Visual Studio Code."), +}; + /** A discovered sandbox environment we can create a provider for. */ interface ICloudSandboxEnvironment { readonly environmentId: string; @@ -134,8 +146,6 @@ export class CloudSandboxAgentHostContribution extends Disposable implements IWo * Disposed when the environment becomes reachable again. */ private readonly _readOnlyHandlers = this._register(new DisposableMap()); - /** Live handler instances, so an already-open session can be settled read-only in place. */ - private readonly _readOnlyInstances = new Map(); /** * Cancelled when the feature is disabled (or the contribution is disposed), so in-flight * discovery and connects abort instead of committing state after teardown has run. @@ -454,56 +464,41 @@ export class CloudSandboxAgentHostContribution extends Disposable implements IWo } /** - * Async-activation hook for a sandbox session type: establish the relay connection on demand, - * then resolve once the host advertises the agent backing this session type (its content - * provider is registered), so the chat can load. Returns false if the environment is unknown, - * the connection fails, or the agent never appears. + * Async-activation hook for a sandbox session type: make the session openable, either by + * connecting to an environment that is already online — resolving once the host advertises the + * agent backing this session type, so the chat can load — or by serving its persisted history + * read-only. Returns false only when the environment is unknown, or nothing can be shown at + * all: no live host and no history to fall back on. */ - private async _waitForActivation(sessionType: string): Promise { + protected async _waitForActivation(sessionType: string): Promise { const address = this._findAddressForSessionType(sessionType); const env = address ? this._environments.get(address) : undefined; if (!address || !env) { return false; } - // Both start before any `await` so they overlap: `/connect` blocks on the compute resume and - // can occupy its whole budget while the transcript already sits ready. - const connecting = this.connect({ environmentId: env.environmentId, sessionId: env.sessionId, name: env.name }); - // Settled into a value so the race below can inspect it without an unhandled rejection. - const connectOutcome = connecting.then(() => undefined, (error: unknown) => error ?? new Error('connect failed')); - const prefetchedHistory = this._prefetchHistoryIfDormant(env); - - if (prefetchedHistory) { - // Whichever lands first decides what the user sees. The connect keeps running either - // way: if it lands later, `onDidChangeConnections` drops the stand-in. - const historyFirst = await Promise.race([ - connectOutcome.then(() => undefined), - prefetchedHistory, - ]); - if (historyFirst && this._isEnabled() && !this._enabledCts.token.isCancellationRequested) { - this._logService.info(`${LOG_PREFIX} History for ${address} arrived before the connect settled; opening it now.`); - const opened = this._activateReadOnly(sessionType, address, env, prefetchedHistory); - // On screen but undecided: a failed connect disables the composer in place. - void connectOutcome.then(connectError => { - if (connectError !== undefined && this._isEnabled() && !this._enabledCts.token.isCancellationRequested) { - this._logService.info(`${LOG_PREFIX} Connect for ${address} failed after the session opened; settling it read-only.`); - this._settleReadOnly(sessionType, address); - } - }); - return opened; - } + // Resuming is a side effect the user has to ask for. Mission Control wakes the environment + // behind `/connect`, and it cannot say in advance whether a dormant one will come back, so + // opening a session must not gamble minutes of wake on the guess. Read the environment's + // state first and dial only what is already online; anything else opens from history with + // the connection banner offering the connect. + // + // Without a task there is no history to serve, and refusing to open would leave the + // session unreachable entirely — so fall through to the connect, which is the only way + // such a session can show anything at all. + if (env.taskId && !await this._isEnvironmentOnline(env)) { + this._logService.info(`${LOG_PREFIX} Environment for ${address} is not online; serving history and leaving the connect to the user.`); + return this._activateReadOnly(sessionType, address, env, this._fetchTaskHistory(env)); } - const connectError = await connectOutcome; + const connectError = await this + .connect({ environmentId: env.environmentId, sessionId: env.sessionId, name: env.name }) + .then(() => undefined, (error: unknown) => error ?? new Error('connect failed')); if (connectError !== undefined) { this._logService.warn(`${LOG_PREFIX} connect-on-open failed for ${address}: ${connectError instanceof Error ? connectError.message : String(connectError)}`); // Serve history whatever the reason: `/connect` fails in several ways for a deleted // sandbox, so gating on any one of them would leave the rest with no history. if (this._isEnabled() && !this._enabledCts.token.isCancellationRequested) { - const opened = this._activateReadOnly(sessionType, address, env, prefetchedHistory); - if (opened) { - this._settleReadOnly(sessionType, address); - } - return opened; + return this._activateReadOnly(sessionType, address, env, this._fetchTaskHistory(env)); } return false; } @@ -525,40 +520,48 @@ export class CloudSandboxAgentHostContribution extends Disposable implements IWo } /** - * Persisted history for an environment that is not currently online, or `undefined` when it is - * online, has no task, or the read failed. + * Whether the environment currently has a daemon listening on the relay. Only `online` does, so + * only `online` can be dialled without triggering a resume. * - * `status` cannot predict whether a dormant environment will wake — suspended and deleted both - * read `offline` — but it does say cheaply that this open is on the slow path. Never rejects. + * An unreadable record answers `false`: this gates a side effect the user has not asked for, so + * a Mission Control blip must cost a click rather than an unrequested wake. */ - private _prefetchHistoryIfDormant(env: ICloudSandboxEnvironment): Promise | undefined { + private async _isEnvironmentOnline(env: ICloudSandboxEnvironment): Promise { + try { + const record = await this._apiService.getEnvironment(env.environmentId, this._enabledCts.token); + return record.status === 'online'; + } catch (error) { + this._logService.trace(`${LOG_PREFIX} Could not read the state of ${env.environmentId}; treating it as not online: ${error instanceof Error ? error.message : String(error)}`); + return false; + } + } + + /** + * A task's persisted history, or `undefined` when there is no task or the read failed. Served + * by Mission Control rather than the sandbox, so it stays readable while the environment is + * asleep or gone. Never rejects. + */ + private _fetchTaskHistory(env: ICloudSandboxEnvironment): Promise | undefined { const taskId = env.taskId; if (!taskId) { return undefined; } const token = this._enabledCts.token; - return (async () => { - try { - const record = await this._apiService.getEnvironment(env.environmentId, token); - if (record.status === 'online') { - return undefined; - } - this._logService.trace(`${LOG_PREFIX} Environment ${env.environmentId} is '${record.status}'; prefetching history in case the connect does not land.`); - return await this._apiService.getSessionHistory(taskId, token); - } catch (error) { - this._logService.trace(`${LOG_PREFIX} History prefetch for ${env.environmentId} did not complete: ${error instanceof Error ? error.message : String(error)}`); - return undefined; - } - })(); + return this._apiService.getSessionHistory(taskId, token).catch((error: unknown) => { + this._logService.trace(`${LOG_PREFIX} History read for ${env.environmentId} did not complete: ${error instanceof Error ? error.message : String(error)}`); + return undefined; + }); } /** - * Register a content provider that serves this session from replayed history. + * Register a content provider that serves this session from replayed history, read-only. * - * Deliberately does *not* mark the session read-only: this also runs while a connect is in - * flight and the environment may yet wake — callers settle it via {@link _settleReadOnly}. - * Returns `true` once registered, which is what lets `canResolveChatSession` proceed, or `false` - * when there is no task to read history from. + * Only ever registered when the environment is not connected — dormant, or a connect that just + * failed — so the transcript is real but there is nothing to send to. A connect that later + * lands drops this stand-in and hands the session to the live handler. + * + * Returns `true` once registered, which is what lets `canResolveChatSession` proceed, or + * `false` when there is no task to read history from. */ private _activateReadOnly(sessionType: string, address: string, env: ICloudSandboxEnvironment, prefetchedHistory?: Promise): boolean { if (this._readOnlyHandlers.has(sessionType)) { @@ -583,37 +586,19 @@ export class CloudSandboxAgentHostContribution extends Disposable implements IWo connectionAuthority: agentHostAuthority(address), prefetchedHistory, })); + handler.markReadOnly(); store.add(this._chatSessionsService.registerChatSessionContentProvider(sessionType, handler)); this._readOnlyHandlers.set(sessionType, store); - this._readOnlyInstances.set(sessionType, handler); - store.add(toDisposable(() => this._readOnlyInstances.delete(sessionType))); this._logService.info(`${LOG_PREFIX} Serving ${sessionType} from Mission Control history.`); return true; } - /** - * Settle a history-backed session as read-only once the connect has failed. Sessions already on - * screen observe this and disable their composer in place, without needing a reopen. - */ - private _settleReadOnly(sessionType: string, address: string): void { - const handler = this._readOnlyInstances.get(sessionType); - if (!handler) { - // The live handler owns this session type, so there is nothing being served from - // history to settle — and forcing the host read-only here would be wrong. - return; - } - handler.markReadOnly(); - // The transcript is real, but there is no host left to send to. - this._providerInstances.get(address)?.setReadOnly(true); - } - /** * Drop any read-only stand-in for an address so the live handler can own the session type. * Registering two content providers for one session type throws, so this must run before a * connection is established rather than after. */ private _clearReadOnly(address: string): void { - this._providerInstances.get(address)?.setReadOnly(false); const authority = agentHostAuthority(address); for (const sessionType of [...this._readOnlyHandlers.keys()]) { if (findRemoteAgentHostSessionTypeAuthority(sessionType, [authority]) === authority) { @@ -653,6 +638,13 @@ export class CloudSandboxAgentHostContribution extends Disposable implements IWo throw new CancellationError(); } return result; + } catch (error) { + // Settle the status here rather than waiting for a connections-changed event: a + // wake that exhausts its retry budget fails before any transport entry exists, so + // no such event is coming and the provider would sit at `connecting` forever — + // a permanent spinner with no way back to the connect action. + this._settleFailedConnect(address); + throw error; } finally { this._pendingConnects.delete(address); } @@ -661,6 +653,24 @@ export class CloudSandboxAgentHostContribution extends Disposable implements IWo return attempt; } + /** + * Return a provider to a state the user can act on after its connect failed. Defers to the + * service when it has something live to report, so a failure that raced a successful dial does + * not overwrite a good status, and leaves `incompatible` alone since redialing cannot fix it. + */ + private _settleFailedConnect(address: string): void { + const provider = this._providerInstances.get(address); + if (!provider) { + return; + } + const connectionInfo = this._remoteAgentHostService.connections.find(c => c.address === address); + if (connectionInfo) { + provider.setConnectionStatus(connectionInfo.status); + } else if (!RemoteAgentHostConnectionStatus.isIncompatible(provider.connectionStatus.get())) { + provider.setConnectionStatus(RemoteAgentHostConnectionStatus.disconnected); + } + } + private _isEnabled(): boolean { return isCloudSandboxEnabled(this._configurationService); } @@ -701,6 +711,12 @@ export class CloudSandboxAgentHostContribution extends Disposable implements IWo omitHostFromWorkspaceLabel: true, // A sandbox is a disposable remote environment, not a checkout on disk. workspaceTypeIcon: Codicon.package, + // The sandbox has to be resumed before it can be sent to, and a resume is not + // guaranteed to succeed, so an offline session is read-only until it reconnects + // rather than accepting input that would queue against an environment that may + // never come back. + readOnlyWhenDisconnected: true, + connectionLabels: CLOUD_SANDBOX_CONNECTION_LABELS, hostGroup: CLOUD_SANDBOX_HOST_GROUP, }); store.add(provider); @@ -740,6 +756,12 @@ export class CloudSandboxAgentHostContribution extends Disposable implements IWo const connectionInfo = this._remoteAgentHostService.connections.find(c => c.address === address); if (connectionInfo) { provider.setConnectionStatus(connectionInfo.status); + } else if (this._pendingConnects.has(address)) { + // A connect is in flight but has not reached `reconnect()` yet, so the service has + // no entry to report: waking an environment can spend minutes minting credentials + // beforehand. Any unrelated connection change would otherwise land here and reset + // the wake to `disconnected`, flipping the chat to a failure it has not had. + continue; } else if (!RemoteAgentHostConnectionStatus.isIncompatible(provider.connectionStatus.get())) { provider.setConnectionStatus(RemoteAgentHostConnectionStatus.disconnected); } diff --git a/src/vs/sessions/contrib/providers/remoteAgentHost/browser/cloudSandboxReadOnlySessionHandler.ts b/src/vs/sessions/contrib/providers/remoteAgentHost/browser/cloudSandboxReadOnlySessionHandler.ts index aa1a8231e4dd25..657aa29a3e729a 100644 --- a/src/vs/sessions/contrib/providers/remoteAgentHost/browser/cloudSandboxReadOnlySessionHandler.ts +++ b/src/vs/sessions/contrib/providers/remoteAgentHost/browser/cloudSandboxReadOnlySessionHandler.ts @@ -58,10 +58,11 @@ class ReadOnlyChatSession extends Disposable implements IChatSession { } /** - * Content provider for cloud sandbox sessions whose environment is gone. + * Content provider for cloud sandbox sessions served from Mission Control's persisted history + * rather than a live host — a dormant environment, or one whose connect failed. * - * Registered only after a connect attempt has failed terminally, and disposed as soon as a real - * connection is established, so it never shadows the live handler. + * Registered only while no connection exists, and disposed as soon as one is established, so it + * never shadows the live handler. */ export class CloudSandboxReadOnlySessionHandler extends Disposable implements IChatSessionContentProvider { diff --git a/src/vs/sessions/contrib/providers/remoteAgentHost/browser/remoteAgentHostSessionsProvider.ts b/src/vs/sessions/contrib/providers/remoteAgentHost/browser/remoteAgentHostSessionsProvider.ts index 9a8a93f1f4cda8..3597da7e187bca 100644 --- a/src/vs/sessions/contrib/providers/remoteAgentHost/browser/remoteAgentHostSessionsProvider.ts +++ b/src/vs/sessions/contrib/providers/remoteAgentHost/browser/remoteAgentHostSessionsProvider.ts @@ -35,7 +35,7 @@ import { IChatWidgetService } from '../../../../../workbench/contrib/chat/browse import { IChatService } from '../../../../../workbench/contrib/chat/common/chatService/chatService.js'; import { IChatSessionsService } from '../../../../../workbench/contrib/chat/common/chatSessionsService.js'; import { ILanguageModelsService } from '../../../../../workbench/contrib/chat/common/languageModels.js'; -import { IAgentHostAutoConnect, IAgentHostConnectProgress, IAgentHostGroup } from '../../../../common/agentHostSessionsProvider.js'; +import { IAgentHostAutoConnect, IAgentHostConnectProgress, IAgentHostConnectionLabels, IAgentHostGroup } from '../../../../common/agentHostSessionsProvider.js'; import { buildAgentHostSessionWorkspace, readBranchProtectionPatterns } from '../../../../common/agentHostSessionWorkspace.js'; import { IGitHubInfo, ISession, ISessionType, ISessionWorkspace, ISessionWorkspaceBrowseAction, SESSION_WORKSPACE_GROUP_REMOTE } from '../../../../services/sessions/common/session.js'; import { ISessionsService } from '../../../../services/sessions/browser/sessionsService.js'; @@ -76,6 +76,7 @@ export interface IRemoteAgentHostSessionsProviderConfig { readonly onDidReportConnectProgress?: Event; /** Optional kind-scoped policy for automatically starting the host. */ readonly autoConnect?: IAgentHostAutoConnect; + readonly connectionLabels?: IAgentHostConnectionLabels; /** * Set when the host addresses sessions under a scheme that differs from its agent provider, as * the cloud sandbox host does (sessions are `ahp-session:/` while the agent is `copilot`). @@ -90,6 +91,13 @@ export interface IRemoteAgentHostSessionsProviderConfig { readonly omitHostFromWorkspaceLabel?: boolean; /** Type icon for this host's workspaces. See {@link ISessionWorkspace.typeIcon}. */ readonly workspaceTypeIcon?: ThemeIcon; + /** + * Forces this host's sessions read-only whenever it is not connected. Set by hosts that cannot + * accept work offline — a cloud sandbox has to be resumed before it can be sent to, so a + * composer would take input that nothing will ever deliver. Hosts left with the default keep + * their sessions writable while disconnected, queuing the input for the reconnect. + */ + readonly readOnlyWhenDisconnected?: boolean; /** See {@link IAgentHostAdapterOptions.defaultChangesetKind}. */ readonly defaultChangesetKind?: ChangesetKind.Branch | ChangesetKind.Uncommitted | ChangesetKind.Session; /** @@ -135,16 +143,20 @@ export class RemoteAgentHostSessionsProvider extends BaseAgentHostSessionsProvid readonly canConnectOnDemand: boolean; readonly onDidReportConnectProgress: Event | undefined; readonly autoConnect?: IAgentHostAutoConnect; + readonly connectionLabels?: IAgentHostConnectionLabels; readonly automations: ISessionsProviderAutomations; private readonly _automationStore: ReconnectableAgentHostAutomationStore; private readonly _connectionStatus = observableValue('connectionStatus', RemoteAgentHostConnectionStatus.disconnected); /** - * Forces this host's sessions read-only. Distinct from `disconnected`: a disconnected host may - * come back, so its sessions stay writable and queue on reconnect, whereas this marks a host - * that is gone and whose sessions exist only as replayed history. + * Whether every session on this host is read-only, which hides the composer. + * + * Only ever true for hosts configured with + * {@link IRemoteAgentHostSessionsProviderConfig.readOnlyWhenDisconnected}, and only while they + * are disconnected: elsewhere a dropped host keeps its sessions writable so the input queues + * for the reconnect. */ - private readonly _readOnly = observableValue('providerReadOnly', false); + private readonly _readOnly: IObservable; readonly connectionStatus: IObservable = this._connectionStatus; protected override get remoteConnectionStatus(): IObservable { @@ -241,7 +253,19 @@ export class RemoteAgentHostSessionsProvider extends BaseAgentHostSessionsProvid this._devContainerWorktreeScope = config.devContainerWorktreeScope; this.onDidReportConnectProgress = config.onDidReportConnectProgress; this.autoConnect = config.autoConnect; + this.connectionLabels = config.connectionLabels; this.canConnectOnDemand = !!config.connectOnDemand; + this._readOnly = config.readOnlyWhenDisconnected + // Deliberately not `!isConnected`: that would include `reconnecting`, which is the + // protocol client restoring a dropped transport while the host itself is up. Input + // sent then is delivered once the transport returns, so hiding the composer would + // interrupt a conversation mid-sentence over a blip the user should not notice. + ? derived(this, reader => { + const status = this._connectionStatus.read(reader); + return RemoteAgentHostConnectionStatus.isDisconnected(status) + || RemoteAgentHostConnectionStatus.isIncompatible(status); + }) + : constObservable(false); this._register(this._onDidChangeResourceLabelHomes(() => this.updateResourceLabelHomes())); this.updateResourceLabelHomes(); const displayName = config.name || config.address; @@ -501,17 +525,6 @@ export class RemoteAgentHostSessionsProvider extends BaseAgentHostSessionsProvid this._connectionStatus.set(status, undefined); } - /** - * Forces every session on this host to be read-only. - * - * Set when the host is permanently unreachable and its sessions are being served from - * persisted history: the conversation is genuine, but there is no host left to send to, so the - * composer must be hidden rather than accept input that can never be delivered. - */ - setReadOnly(readOnly: boolean): void { - this._readOnly.set(readOnly, undefined); - } - /** * Seed discovered session summaries into the cache so they surface in the sessions list * **before** a connection is established (lazy discovery). 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 d2d3a582abf481..11ea97bc0b5ad2 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 @@ -12,18 +12,25 @@ import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../../base/ import { URI } from '../../../../../../base/common/uri.js'; import { AgentSession } from '../../../../../../platform/agentHost/common/agent.js'; import { IAgentSessionMetadata } from '../../../../../../platform/agentHost/common/agentService.js'; +import { agentHostAuthority } from '../../../../../../platform/agentHost/common/agentHostUri.js'; +import { remoteAgentHostSessionTypeId } from '../../../../../../platform/agentHost/common/agentHostSessionType.js'; +import { IReplayedTaskHistory } from '../../../../../../platform/agentHost/common/taskEventReplay.js'; import { + CLOUD_SANDBOX_AGENT_PROVIDER, CloudSandboxEnabledSettingId, ICloudSandboxAgentHostService, ICloudSandboxApiService, cloudSandboxAddress, + type CloudSandboxEnvironmentStatus, type ICloudSandboxConnectOptions, type ICloudSandboxCreateSessionRequest, type ICloudSandboxCreatedSession, type ICloudSandboxDiscoveredSession, type ICloudSandboxDiscoveryResult, + type ICloudSandboxEnvironment as ICloudSandboxEnvironmentRecord, } from '../../../../../../platform/agentHost/common/cloudSandboxAgentHost.js'; -import { IRemoteAgentHostService, RemoteAgentHostsEnabledSettingId } from '../../../../../../platform/agentHost/common/remoteAgentHostService.js'; +import { IRemoteAgentHostService, RemoteAgentHostConnectionStatus, RemoteAgentHostsEnabledSettingId } from '../../../../../../platform/agentHost/common/remoteAgentHostService.js'; +import { IObservable, observableValue } from '../../../../../../base/common/observable.js'; import { IConfigurationService } from '../../../../../../platform/configuration/common/configuration.js'; import { TestConfigurationService } from '../../../../../../platform/configuration/test/common/testConfigurationService.js'; import { TestInstantiationService } from '../../../../../../platform/instantiation/test/common/instantiationServiceMock.js'; @@ -45,6 +52,10 @@ class StubProvider extends mock() { readonly seeded: IAgentSessionMetadata[] = []; /** Raw ids seeded as provisional, mirroring the real provider's listing gate. */ readonly withheld = new Set(); + /** Every connection status pushed onto this provider, in order. */ + readonly statuses: string[] = []; + private readonly _status = observableValue('stubStatus', RemoteAgentHostConnectionStatus.disconnected); + override readonly connectionStatus: IObservable = this._status; disposed = false; override readonly id: string; @@ -98,9 +109,10 @@ class StubProvider extends mock() { }); } - override setConnectionStatus(): void { } - - override setReadOnly(): void { } + override setConnectionStatus(status: RemoteAgentHostConnectionStatus): void { + this.statuses.push(status.kind); + this._status.set(status, undefined); + } override dispose(): void { this.disposed = true; @@ -110,6 +122,15 @@ class StubProvider extends mock() { class TestCloudSandboxContribution extends CloudSandboxAgentHostContribution { readonly stubProviders = new Map(); + /** + * Drives the async activation the chat service performs on open. Called directly rather than + * through the global activation registry, which also holds the generic remote-agent-host + * activator for the same session type. + */ + activate(sessionType: string): Promise { + return this._waitForActivation(sessionType); + } + protected override _instantiateProvider(config: IRemoteAgentHostSessionsProviderConfig): CloudSandboxSessionsProvider { const stub = new StubProvider(config); this.stubProviders.set(config.address, stub); @@ -141,6 +162,12 @@ interface ITestHarness { runDiscovery(): Promise; /** Runs while a `connect` is in flight, for testing what can race with it. */ onConnect?: () => Promise; + /** The state Mission Control reports for an environment. Defaults to `offline`. */ + environmentStatus: CloudSandboxEnvironmentStatus; + /** Session types currently served from replayed history. */ + readonly readOnlySessionTypes: string[]; + /** Drives the async activation the chat service performs when a session is opened. */ + activate(environmentId: string): Promise; readonly created: ICloudSandboxCreateSessionRequest[]; readonly connectedTo: string[]; /** Host groups currently declared to the filter service. */ @@ -160,21 +187,34 @@ async function createContribution(store: Pick, sessions: }): Promise { const discoveryHandlers: (() => Promise)[] = []; const hostGroups: IAgentHostGroup[] = []; + const readOnlySessionTypes: string[] = []; const instantiationService = store.add(new TestInstantiationService()); const created: ICloudSandboxCreateSessionRequest[] = []; const connectedTo: string[] = []; const harness: ITestHarness = { discovered: sessions, + environmentStatus: 'offline', + readOnlySessionTypes, created, connectedTo, hostGroups, runDiscovery: async () => { await Promise.all(discoveryHandlers.map(handler => handler())); }, + activate: async (environmentId: string) => { + const sessionType = remoteAgentHostSessionTypeId(agentHostAuthority(cloudSandboxAddress(environmentId)), CLOUD_SANDBOX_AGENT_PROVIDER); + return harness.contribution.activate(sessionType); + }, } as ITestHarness; instantiationService.stub(ICloudSandboxApiService, new class extends mock() { override async listSessions(_token: CancellationToken): Promise { return { kind: 'complete', sessions: harness.discovered }; } + override async getEnvironment(id: string): Promise { + return { id, status: harness.environmentStatus }; + } + override async getSessionHistory(): Promise { + return { sessions: [], truncated: false }; + } override async createSession(request: ICloudSandboxCreateSessionRequest): Promise { created.push(request); return options?.createSession @@ -192,6 +232,9 @@ async function createContribution(store: Pick, sessions: instantiationService.stub(IRemoteAgentHostService, new class extends mock() { override readonly onDidChangeConnections = Event.None; override readonly connections = []; + // No live protocol client is modelled, so activation stops once the connect has been made + // rather than going on to wait for the host to advertise its agents. + override getConnection() { return undefined; } override async removeRemoteAgentHost(): Promise { } }()); instantiationService.stub(IRemoteAgentHostConnectionCustomizationService, new class extends mock() { @@ -223,7 +266,18 @@ async function createContribution(store: Pick, sessions: override readonly onDidRegisterAuthenticationProvider = Event.None; }()); instantiationService.stub(INotificationService, new class extends mock() { }()); - instantiationService.stub(IChatSessionsService, new class extends mock() { }()); + instantiationService.stub(IChatSessionsService, new class extends mock() { + override getContentProviderSchemes(): string[] { return [...readOnlySessionTypes]; } + override registerChatSessionContentProvider(sessionType: string): IDisposable { + readOnlySessionTypes.push(sessionType); + return toDisposable(() => { + const index = readOnlySessionTypes.indexOf(sessionType); + if (index >= 0) { + readOnlySessionTypes.splice(index, 1); + } + }); + } + }()); instantiationService.stub(ILogService, new NullLogService()); const contribution = store.add(instantiationService.createInstance(TestCloudSandboxContribution)); @@ -273,6 +327,22 @@ suite('CloudSandboxAgentHostContribution', () => { assert.strictEqual(provider?.seeded[0]?.project, undefined); }); + test('supplies environment connection labels independently of the task name', async () => { + const { contribution } = await createContribution(store, [discoveredSession({ name: 'hi' })]); + const labels = contribution.stubProviders.get(cloudSandboxAddress('env-1'))?.config.connectionLabels; + + assert.deepStrictEqual(labels && { ...labels, reconnectingIn: labels.reconnectingIn(5) }, { + unavailableTitle: 'Environment Offline', + unavailable: 'Environment offline.', + connectingTitle: 'Connecting to the Environment', + connecting: 'Connecting...', + reconnecting: 'Reconnecting...', + reconnectingIn: 'Reconnecting in 5s', + incompatibleTitle: 'Cannot Connect to the Environment', + incompatible: 'This environment is incompatible with this version of Visual Studio Code.', + }); + }); + test('opts sandbox providers out of the [host] workspace-label suffix', async () => { // Each sandbox is its own provider named after its task, so the suffix would put every // session in a workspace group of one. @@ -314,6 +384,73 @@ suite('CloudSandboxAgentHostContribution', () => { assert.deepStrictEqual([...hostGroups], []); }); + + test('serves a dormant environment from history instead of waking it to open a session', async () => { + // Resuming costs minutes and Mission Control cannot say in advance whether a dormant + // environment will come back, so opening a session must not gamble that on the user's + // behalf. The session still opens — from replayed history — and the connect is offered. + const harness = await createContribution(store, [discoveredSession()]); + harness.environmentStatus = 'offline'; + + const opened = await harness.activate('env-1'); + + assert.deepStrictEqual({ opened, connectedTo: harness.connectedTo, servedFromHistory: harness.readOnlySessionTypes.length }, { + opened: true, + connectedTo: [], + servedFromHistory: 1, + }); + }); + + test('connects when opening a session on an environment that is already online', async () => { + const harness = await createContribution(store, [discoveredSession()]); + harness.environmentStatus = 'online'; + + await harness.activate('env-1'); + + assert.deepStrictEqual({ connectedTo: harness.connectedTo, servedFromHistory: harness.readOnlySessionTypes.length }, { + connectedTo: ['env-1'], + servedFromHistory: 0, + }); + }); + + test('does not wake an environment whose state could not be read', async () => { + // A Mission Control blip must cost a click, not an unrequested resume. + const harness = await createContribution(store, [discoveredSession()]); + harness.environmentStatus = 'degraded'; + + const opened = await harness.activate('env-1'); + + assert.deepStrictEqual({ opened, connectedTo: harness.connectedTo }, { opened: true, connectedTo: [] }); + }); + + test('connects a dormant environment that has no history to fall back on', async () => { + // Without a task there is nothing to serve read-only, so refusing to connect would leave + // the session unopenable rather than merely offline. The harness models no live protocol + // client, so the dial itself is what this asserts. + const harness = await createContribution(store, [discoveredSession({ taskId: undefined })]); + harness.environmentStatus = 'offline'; + + await harness.activate('env-1'); + + assert.deepStrictEqual({ connectedTo: harness.connectedTo, servedFromHistory: harness.readOnlySessionTypes.length }, { + connectedTo: ['env-1'], + servedFromHistory: 0, + }); + }); + + test('settles the status after a failed connect so the connect action comes back', async () => { + // A wake that exhausts its retry budget fails before any transport entry exists, so no + // connections-changed event follows. Left alone the provider would sit at `connecting` + // forever: a permanent spinner, a permanently hidden composer, and no way to retry. + const harness = await createContribution(store, [discoveredSession()]); + harness.environmentStatus = 'online'; + harness.onConnect = () => Promise.reject(new Error('Timed out waiting for sandbox environment to wake.')); + + await harness.activate('env-1'); + + const provider = harness.contribution.stubProviders.get(cloudSandboxAddress('env-1')); + assert.deepStrictEqual(provider?.statuses, ['connecting', 'disconnected']); + }); }); suite('CloudSandboxAgentHostContribution provisioning', () => { 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 630048ad813d3b..84dd0e8a31e9eb 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 @@ -39,7 +39,7 @@ 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, SessionRemoteConnectionFailureReason, SessionStatus, type ISession } from '../../../../../services/sessions/common/session.js'; +import { ChatInteractivity, ChatModelSource, SessionRemoteConnectionFailureReason, 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'; @@ -242,7 +242,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; localAgentHostService?: IAgentHostService; noConnection?: boolean; isWebPlatform?: boolean; workspaceTrusted?: boolean; omitHostFromWorkspaceLabel?: boolean; workspaceTypeIcon?: ThemeIcon; sessionSchemeAlias?: IAgentHostSessionSchemeAlias; defaultChangesetKind?: IRemoteAgentHostSessionsProviderConfig['defaultChangesetKind']; sessionResolutionPolicies?: Array<{ authority: string; policy: IAgentHostSessionResolutionPolicy }>; devContainerWorktreeScope?: string; ctor?: typeof RemoteAgentHostSessionsProvider; labelService?: ILabelService; defaultDirectory?: string }): 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; localAgentHostService?: IAgentHostService; noConnection?: boolean; isWebPlatform?: boolean; workspaceTrusted?: boolean; omitHostFromWorkspaceLabel?: boolean; workspaceTypeIcon?: ThemeIcon; sessionSchemeAlias?: IAgentHostSessionSchemeAlias; defaultChangesetKind?: IRemoteAgentHostSessionsProviderConfig['defaultChangesetKind']; sessionResolutionPolicies?: Array<{ authority: string; policy: IAgentHostSessionResolutionPolicy }>; devContainerWorktreeScope?: string; readOnlyWhenDisconnected?: boolean; ctor?: typeof RemoteAgentHostSessionsProvider; labelService?: ILabelService; defaultDirectory?: string }): RemoteAgentHostSessionsProvider { const instantiationService = disposables.add(new TestInstantiationService()); instantiationService.stub(IFileDialogService, {}); @@ -307,6 +307,7 @@ function createProvider(disposables: DisposableStore, connection: MockAgentConne sessionSchemeAlias: overrides?.sessionSchemeAlias, defaultChangesetKind: overrides?.defaultChangesetKind, devContainerWorktreeScope: overrides?.devContainerWorktreeScope, + readOnlyWhenDisconnected: overrides?.readOnlyWhenDisconnected, }; const baseCtor = overrides?.ctor ?? RemoteAgentHostSessionsProvider; @@ -483,6 +484,31 @@ suite('RemoteAgentHostSessionsProvider', () => { }); }); + test('keeps the composer during a self-healing reconnect on a host that is read-only when disconnected', () => { + // `reconnecting` is the protocol client restoring a dropped transport while the host is + // still up, so input sent then is delivered once it returns. Treating it as read-only + // would pull the composer — and the user's focus — mid-conversation over a blip. + const provider = createProvider(disposables, connection, { readOnlyWhenDisconnected: true }); + provider.setConnectionStatus(RemoteAgentHostConnectionStatus.connected); + provider.seedSessions([createSession('sandbox-session')]); + const chat = provider.getSessions()[0].mainChat.get(); + const interactivity = [chat.interactivity.get()]; + + provider.setConnectionStatus(RemoteAgentHostConnectionStatus.reconnecting); + interactivity.push(chat.interactivity.get()); + provider.setConnectionStatus(RemoteAgentHostConnectionStatus.disconnected); + interactivity.push(chat.interactivity.get()); + provider.setConnectionStatus(RemoteAgentHostConnectionStatus.connected); + interactivity.push(chat.interactivity.get()); + + assert.deepStrictEqual(interactivity, [ + ChatInteractivity.Full, + ChatInteractivity.Full, + ChatInteractivity.ReadOnly, + ChatInteractivity.Full, + ]); + }); + test('does not present an active chat as busy while its remote host is unavailable', () => { const provider = createProvider(disposables, connection); provider.setConnectionStatus(RemoteAgentHostConnectionStatus.connected); diff --git a/src/vs/sessions/test/browser/chatGroupsView.test.ts b/src/vs/sessions/test/browser/chatGroupsView.test.ts index 99046a3919d614..1380a9b3156ab7 100644 --- a/src/vs/sessions/test/browser/chatGroupsView.test.ts +++ b/src/vs/sessions/test/browser/chatGroupsView.test.ts @@ -25,7 +25,7 @@ import { workbenchInstantiationService } from '../../../workbench/test/browser/w import { AbstractChatView, ChatViewKind, IChatViewOptions } from '../../browser/parts/chatView.js'; import { ChatGroupsView } from '../../browser/parts/chatGroupsView.js'; import { SessionFocusedChatIsRenameTargetContext } from '../../common/contextkeys.js'; -import { type IAgentHostAutoConnect, type IAgentHostConnectProgress, IAgentHostSessionsProvider } from '../../common/agentHostSessionsProvider.js'; +import { type IAgentHostAutoConnect, type IAgentHostConnectProgress, type IAgentHostConnectionLabels, IAgentHostSessionsProvider } from '../../common/agentHostSessionsProvider.js'; import { IChatViewFactory } from '../../services/chatView/browser/chatViewFactory.js'; import { ISessionsProvidersService } from '../../services/sessions/browser/sessionsProvidersService.js'; import { ISessionsPartService } from '../../services/sessions/browser/sessionsPartService.js'; @@ -37,6 +37,7 @@ import { ISessionsProvider } from '../../services/sessions/common/sessionsProvid class TestChatView extends AbstractChatView { private readonly _focusTarget = mainWindow.document.createElement('button'); override readonly hasVisibleTranscriptContent = observableValue(this, false); + override readonly isLoadingTranscript = observableValue(this, false); layoutCount = 0; primary = false; @@ -200,6 +201,7 @@ class TestAgentHostProvider extends mock() { connectCalls = 0; reconnectNowCalls = 0; connectGate: Promise | undefined; + override connectionLabels: IAgentHostConnectionLabels | undefined; override async connect(): Promise { this.connectCalls++; @@ -271,7 +273,7 @@ function readRemoteHostUnavailableState(view: ChatGroupsView): { readonly visibl return { visible: !state?.classList.contains('hidden'), title: state?.querySelector('.remote-host-unavailable-empty-state-title')?.textContent ?? undefined, - description: state?.querySelector('.remote-host-unavailable-empty-state-description')?.textContent ?? undefined, + description: state?.querySelector('.remote-host-unavailable-empty-state-description:not(.hidden)')?.textContent ?? undefined, progress: state?.querySelector('.remote-host-unavailable-empty-state-progress:not(.hidden)')?.textContent ?? undefined, action: action && !action.classList.contains('hidden') ? action.textContent ?? undefined : undefined, actionHidden: action?.classList.contains('hidden') ?? true, @@ -856,6 +858,187 @@ suite('Sessions - ChatGroupsView', () => { }); }); + test('offers a plain connect before the first attempt and a retry once one has failed', async () => { + const { sessionsProvidersService, view } = createHarness(disposables); + const provider = new TestAgentHostProvider(); + const connect = new DeferredPromise(); + provider.connectGate = connect.p; + sessionsProvidersService.provider = provider; + // A host that reports no specific reason has not been established as stoppable, which is + // the shape a cloud sandbox arrives in: dormant, and resumable only by asking. + const session = new TestActiveSession([createChat('main')], undefined, true, provider.id, { kind: 'disconnected', reason: SessionRemoteConnectionFailureReason.Unknown }); + const status = session.remoteConnectionStatus; + assert.ok(status); + view.setSession(session, options); + const beforeAnyAttempt = readRemoteHostUnavailableState(view).action; + + view.element.querySelector('.remote-host-unavailable-empty-state-action .monaco-button')?.click(); + const whileConnecting = readRemoteHostUnavailableState(view); + + const originalErrorHandler = errorHandler.getUnexpectedErrorHandler(); + setUnexpectedErrorHandler(() => { }); + try { + connect.error(new Error('Expected sandbox resume failure')); + await Promise.resolve(); + await Promise.resolve(); + } finally { + setUnexpectedErrorHandler(originalErrorHandler); + } + + assert.deepStrictEqual({ + beforeAnyAttempt, + whileConnecting: { visible: whileConnecting.visible, title: whileConnecting.title, progress: whileConnecting.progress }, + afterFailure: readRemoteHostUnavailableState(view).action, + connectCalls: provider.connectCalls, + }, { + beforeAnyAttempt: 'Connect', + whileConnecting: { visible: true, title: 'Connecting to WSL: Ubuntu', progress: 'Waiting for agent host connection...' }, + afterFailure: 'Retry', + connectCalls: 1, + }); + }); + + test('shows the connecting state for a connection the view did not start itself', () => { + const { chatViewFactory, sessionsProvidersService, view } = createHarness(disposables); + const provider = new TestAgentHostProvider(); + sessionsProvidersService.provider = provider; + // Nothing in this view asked for the connect, so there is no attempt to hang progress on. + // The wait is real either way and must not present as a blank chat. + const session = new TestActiveSession([createChat('main')], undefined, true, provider.id, { kind: 'connecting' }); + const status = session.remoteConnectionStatus; + assert.ok(status); + view.setSession(session, options); + const emptyTranscript = readRemoteHostUnavailableState(view); + + const withTranscript = new TestActiveSession([createChat('existing')], undefined, true, provider.id, { kind: 'connecting' }); + view.setSession(withTranscript, options); + chatViewFactory.views[chatViewFactory.views.length - 1].hasVisibleTranscriptContent.set(true, undefined); + + assert.deepStrictEqual({ + emptyTranscript: { visible: emptyTranscript.visible, title: emptyTranscript.title, progress: emptyTranscript.progress, action: emptyTranscript.action }, + withTranscript: readBanner(view), + connectCalls: provider.connectCalls, + }, { + emptyTranscript: { visible: true, title: 'Connecting to WSL: Ubuntu', progress: 'Waiting for agent host connection...', action: undefined }, + withTranscript: { visible: true, message: 'Waiting for agent host connection...', action: undefined }, + connectCalls: 0, + }); + }); + + test('explains a read-only chat by its connection state rather than the generic notice', () => { + const { chatViewFactory, sessionsProvidersService, view } = createHarness(disposables); + const provider = new TestAgentHostProvider(); + sessionsProvidersService.provider = provider; + // A host that cannot queue work offline reports its sessions read-only *because* it is + // disconnected, so the connection banner is what explains the missing composer. + const chat = createChat('main'); + chat.interactivity.set(ChatInteractivity.ReadOnly, undefined); + const session = new TestActiveSession([chat], undefined, true, provider.id, { kind: 'disconnected', reason: SessionRemoteConnectionFailureReason.Unknown }); + view.setSession(session, options); + chatViewFactory.views[chatViewFactory.views.length - 1].hasVisibleTranscriptContent.set(true, undefined); + const disconnected = readBanner(view); + + // Archiving is a deliberate act on the session, so it keeps explaining itself. + session.isArchived.set(true, undefined); + + assert.deepStrictEqual({ disconnected, archived: readBanner(view).message }, { + disconnected: { visible: true, message: 'Cannot reach WSL: Ubuntu.', action: 'Connect' }, + archived: 'Archived sessions are read-only.', + }); + }); + + test('prefers the banner over the centered state while the transcript is still loading', () => { + const { chatViewFactory, sessionsProvidersService, view } = createHarness(disposables); + const provider = new TestAgentHostProvider(); + sessionsProvidersService.provider = provider; + // A session whose history is still in flight reports no transcript yet, which is not the + // same as having none. Committing to the full-pane state here would flash it and then + // collapse to the banner the moment the history lands. + const session = new TestActiveSession([createChat('main')], undefined, true, provider.id, { kind: 'disconnected', reason: SessionRemoteConnectionFailureReason.Unknown }); + view.setSession(session, options); + const currentView = () => chatViewFactory.views[chatViewFactory.views.length - 1]; + currentView().isLoadingTranscript.set(true, undefined); + const whileLoading = { state: readRemoteHostUnavailableState(view).visible, banner: readBanner(view) }; + + // History arrived: the transcript owns the pane and the banner keeps explaining the host. + transaction(tx => { + currentView().isLoadingTranscript.set(false, tx); + currentView().hasVisibleTranscriptContent.set(true, tx); + }); + const withTranscript = { state: readRemoteHostUnavailableState(view).visible, banner: readBanner(view).visible }; + + // A genuinely empty session settles the other way, so the centered state is not lost. + currentView().hasVisibleTranscriptContent.set(false, undefined); + + assert.deepStrictEqual({ whileLoading, withTranscript, settledEmpty: readRemoteHostUnavailableState(view).visible }, { + whileLoading: { state: false, banner: { visible: true, message: 'Cannot reach WSL: Ubuntu.', action: 'Connect' } }, + withTranscript: { state: false, banner: true }, + settledEmpty: true, + }); + }); + + test('uses provider connection labels for recovery states and banners', () => { + const { chatViewFactory, sessionsProvidersService, view } = createHarness(disposables); + const provider = new TestAgentHostProvider(); + provider.connectionLabels = { + unavailableTitle: 'Environment Offline', + unavailable: 'Environment offline.', + connectingTitle: 'Connecting to the Environment', + connecting: 'Connecting...', + reconnecting: 'Reconnecting...', + reconnectingIn: seconds => `Reconnecting in ${seconds}s`, + incompatibleTitle: 'Cannot Connect to the Environment', + incompatible: 'This environment is incompatible with this version of Visual Studio Code.', + }; + sessionsProvidersService.provider = provider; + const session = new TestActiveSession([createChat('main')], undefined, true, provider.id, { kind: 'disconnected', reason: SessionRemoteConnectionFailureReason.Unknown }); + const status = session.remoteConnectionStatus; + assert.ok(status); + view.setSession(session, options); + const offlineState = readRemoteHostUnavailableState(view); + + status.set({ kind: 'connecting' }, undefined); + const connectingState = readRemoteHostUnavailableState(view); + + // With a transcript on screen the banner carries the same wording. + chatViewFactory.views[chatViewFactory.views.length - 1].hasVisibleTranscriptContent.set(true, undefined); + const connectingBanner = readBanner(view).message; + status.set({ kind: 'disconnected', reason: SessionRemoteConnectionFailureReason.Unknown }, undefined); + const offlineBanner = readBanner(view); + + status.set({ kind: 'incompatible' }, undefined); + const incompatibleBanner = readBanner(view); + chatViewFactory.views[chatViewFactory.views.length - 1].hasVisibleTranscriptContent.set(false, undefined); + const incompatibleState = readRemoteHostUnavailableState(view); + status.set({ kind: 'connected' }, undefined); + + assert.deepStrictEqual({ + offline: { title: offlineState.title, description: offlineState.description, action: offlineState.action }, + connecting: { title: connectingState.title, description: connectingState.description, progress: connectingState.progress }, + connectingBanner, + offlineBanner, + incompatible: { title: incompatibleState.title, description: incompatibleState.description, action: incompatibleState.action }, + incompatibleBanner, + connected: { recoveryVisible: readRemoteHostUnavailableState(view).visible, bannerVisible: readBanner(view).visible }, + }, { + offline: { title: 'Environment Offline', description: undefined, action: 'Connect' }, + connecting: { title: 'Connecting to the Environment', description: undefined, progress: 'Connecting...' }, + connectingBanner: 'Connecting...', + offlineBanner: { visible: true, message: 'Environment offline.', action: 'Connect' }, + incompatible: { + title: 'Cannot Connect to the Environment', + description: 'This environment is incompatible with this version of Visual Studio Code.', + action: undefined, + }, + incompatibleBanner: { + visible: true, + message: 'This environment is incompatible with this version of Visual Studio Code.', + action: undefined, + }, + connected: { recoveryVisible: false, bannerVisible: false }, + }); + }); + test('automatically starts the host again when it drops after an earlier automatic start', async () => { const { chatViewFactory, sessionsProvidersService, view } = createHarness(disposables); const provider = new TestAgentHostProvider(); diff --git a/src/vs/sessions/test/browser/remoteHostUnavailableEmptyState.fixture.ts b/src/vs/sessions/test/browser/remoteHostUnavailableEmptyState.fixture.ts index 93198287022c54..ad19510470e4b1 100644 --- a/src/vs/sessions/test/browser/remoteHostUnavailableEmptyState.fixture.ts +++ b/src/vs/sessions/test/browser/remoteHostUnavailableEmptyState.fixture.ts @@ -44,6 +44,23 @@ export default defineThemedFixtureGroup({ path: 'sessions/remoteHostUnavailable/ progress: 'Downloading server (80%)', }), }), + + // A host that supplies its own wording, and whose heading needs no description under it. + EnvironmentOffline: defineComponentFixture({ + labels: { kind: 'screenshot' }, + render: context => renderUnavailableState(context, { + title: 'Environment Offline', + action: { label: 'Connect', run: () => { } }, + }), + }), + + EnvironmentConnecting: defineComponentFixture({ + labels: { kind: 'screenshot' }, + render: context => renderUnavailableState(context, { + title: 'Connecting to the Environment', + progress: 'Connecting...', + }), + }), }); function renderUnavailableState({ container, disposableStore }: ComponentFixtureContext, content: IRemoteHostUnavailableEmptyStateContent): void { From c254bc1faad056e7f67a99abc3ea5769622b3e01 Mon Sep 17 00:00:00 2001 From: Osvaldo Ortega Date: Fri, 4 Sep 2026 16:44:35 -0700 Subject: [PATCH 2/2] Fix sandbox activation races and initial connection interactivity Reject stale activation results after cancellation, feature teardown, or provider replacement. Keep initial sandbox connections read-only while preserving input during self-healing reconnects. Add regression coverage for review feedback on #334644, including actual environment lookup failures. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../cloudSandboxAgentHostContribution.ts | 72 ++++----- .../remoteAgentHostSessionsProvider.ts | 20 +-- .../cloudSandboxAgentHostContribution.test.ts | 141 +++++++++++++++++- .../remoteAgentHostSessionsProvider.test.ts | 42 +++--- 4 files changed, 192 insertions(+), 83 deletions(-) diff --git a/src/vs/sessions/contrib/providers/remoteAgentHost/browser/cloudSandboxAgentHostContribution.ts b/src/vs/sessions/contrib/providers/remoteAgentHost/browser/cloudSandboxAgentHostContribution.ts index d31209dc19426b..b0820787decfd5 100644 --- a/src/vs/sessions/contrib/providers/remoteAgentHost/browser/cloudSandboxAgentHostContribution.ts +++ b/src/vs/sessions/contrib/providers/remoteAgentHost/browser/cloudSandboxAgentHostContribution.ts @@ -463,44 +463,45 @@ export class CloudSandboxAgentHostContribution extends Disposable implements IWo return authority ? byAuthority.get(authority) : undefined; } - /** - * Async-activation hook for a sandbox session type: make the session openable, either by - * connecting to an environment that is already online — resolving once the host advertises the - * agent backing this session type, so the chat can load — or by serving its persisted history - * read-only. Returns false only when the environment is unknown, or nothing can be shown at - * all: no live host and no history to fall back on. - */ + /** Opens an online environment through its host, or an offline session from persisted history. */ protected async _waitForActivation(sessionType: string): Promise { const address = this._findAddressForSessionType(sessionType); const env = address ? this._environments.get(address) : undefined; - if (!address || !env) { + const provider = address ? this._providerInstances.get(address) : undefined; + if (!address || !env || !provider) { return false; } - // Resuming is a side effect the user has to ask for. Mission Control wakes the environment - // behind `/connect`, and it cannot say in advance whether a dormant one will come back, so - // opening a session must not gamble minutes of wake on the guess. Read the environment's - // state first and dial only what is already online; anything else opens from history with - // the connection banner offering the connect. - // - // Without a task there is no history to serve, and refusing to open would leave the - // session unreachable entirely — so fall through to the connect, which is the only way - // such a session can show anything at all. - if (env.taskId && !await this._isEnvironmentOnline(env)) { + const token = this._enabledCts.token; + const isCurrentActivation = () => { + const current = !token.isCancellationRequested + && this._isEnabled() + && this._environments.has(address) + && this._providerInstances.get(address) === provider; + if (!current) { + this._logService.trace(`${LOG_PREFIX} Abandoning activation for ${address} after teardown.`); + } + return current; + }; + + // Without a task there is no history fallback, so connecting is the only way to open it. + const shouldConnect = !env.taskId || await this._isEnvironmentOnline(env, token); + if (!isCurrentActivation()) { + return false; + } + if (!shouldConnect) { this._logService.info(`${LOG_PREFIX} Environment for ${address} is not online; serving history and leaving the connect to the user.`); - return this._activateReadOnly(sessionType, address, env, this._fetchTaskHistory(env)); + return this._activateReadOnly(sessionType, address, env, this._fetchTaskHistory(env, token)); } const connectError = await this .connect({ environmentId: env.environmentId, sessionId: env.sessionId, name: env.name }) .then(() => undefined, (error: unknown) => error ?? new Error('connect failed')); + if (!isCurrentActivation()) { + return false; + } if (connectError !== undefined) { this._logService.warn(`${LOG_PREFIX} connect-on-open failed for ${address}: ${connectError instanceof Error ? connectError.message : String(connectError)}`); - // Serve history whatever the reason: `/connect` fails in several ways for a deleted - // sandbox, so gating on any one of them would leave the rest with no history. - if (this._isEnabled() && !this._enabledCts.token.isCancellationRequested) { - return this._activateReadOnly(sessionType, address, env, this._fetchTaskHistory(env)); - } - return false; + return this._activateReadOnly(sessionType, address, env, this._fetchTaskHistory(env, token)); } const authority = agentHostAuthority(address); while (true) { @@ -519,16 +520,10 @@ export class CloudSandboxAgentHostContribution extends Disposable implements IWo } } - /** - * Whether the environment currently has a daemon listening on the relay. Only `online` does, so - * only `online` can be dialled without triggering a resume. - * - * An unreadable record answers `false`: this gates a side effect the user has not asked for, so - * a Mission Control blip must cost a click rather than an unrequested wake. - */ - private async _isEnvironmentOnline(env: ICloudSandboxEnvironment): Promise { + /** An unreadable record must not trigger an automatic resume. */ + private async _isEnvironmentOnline(env: ICloudSandboxEnvironment, token: CancellationToken): Promise { try { - const record = await this._apiService.getEnvironment(env.environmentId, this._enabledCts.token); + const record = await this._apiService.getEnvironment(env.environmentId, token); return record.status === 'online'; } catch (error) { this._logService.trace(`${LOG_PREFIX} Could not read the state of ${env.environmentId}; treating it as not online: ${error instanceof Error ? error.message : String(error)}`); @@ -536,17 +531,12 @@ export class CloudSandboxAgentHostContribution extends Disposable implements IWo } } - /** - * A task's persisted history, or `undefined` when there is no task or the read failed. Served - * by Mission Control rather than the sandbox, so it stays readable while the environment is - * asleep or gone. Never rejects. - */ - private _fetchTaskHistory(env: ICloudSandboxEnvironment): Promise | undefined { + /** Reads history from Mission Control without connecting to the sandbox. */ + private _fetchTaskHistory(env: ICloudSandboxEnvironment, token: CancellationToken): Promise | undefined { const taskId = env.taskId; if (!taskId) { return undefined; } - const token = this._enabledCts.token; return this._apiService.getSessionHistory(taskId, token).catch((error: unknown) => { this._logService.trace(`${LOG_PREFIX} History read for ${env.environmentId} did not complete: ${error instanceof Error ? error.message : String(error)}`); return undefined; diff --git a/src/vs/sessions/contrib/providers/remoteAgentHost/browser/remoteAgentHostSessionsProvider.ts b/src/vs/sessions/contrib/providers/remoteAgentHost/browser/remoteAgentHostSessionsProvider.ts index 3597da7e187bca..eea4e91f11b958 100644 --- a/src/vs/sessions/contrib/providers/remoteAgentHost/browser/remoteAgentHostSessionsProvider.ts +++ b/src/vs/sessions/contrib/providers/remoteAgentHost/browser/remoteAgentHostSessionsProvider.ts @@ -91,12 +91,7 @@ export interface IRemoteAgentHostSessionsProviderConfig { readonly omitHostFromWorkspaceLabel?: boolean; /** Type icon for this host's workspaces. See {@link ISessionWorkspace.typeIcon}. */ readonly workspaceTypeIcon?: ThemeIcon; - /** - * Forces this host's sessions read-only whenever it is not connected. Set by hosts that cannot - * accept work offline — a cloud sandbox has to be resumed before it can be sent to, so a - * composer would take input that nothing will ever deliver. Hosts left with the default keep - * their sessions writable while disconnected, queuing the input for the reconnect. - */ + /** Keeps unavailable and initially connecting sessions read-only, but permits self-healing reconnects. */ readonly readOnlyWhenDisconnected?: boolean; /** See {@link IAgentHostAdapterOptions.defaultChangesetKind}. */ readonly defaultChangesetKind?: ChangesetKind.Branch | ChangesetKind.Uncommitted | ChangesetKind.Session; @@ -148,14 +143,6 @@ export class RemoteAgentHostSessionsProvider extends BaseAgentHostSessionsProvid private readonly _automationStore: ReconnectableAgentHostAutomationStore; private readonly _connectionStatus = observableValue('connectionStatus', RemoteAgentHostConnectionStatus.disconnected); - /** - * Whether every session on this host is read-only, which hides the composer. - * - * Only ever true for hosts configured with - * {@link IRemoteAgentHostSessionsProviderConfig.readOnlyWhenDisconnected}, and only while they - * are disconnected: elsewhere a dropped host keeps its sessions writable so the input queues - * for the reconnect. - */ private readonly _readOnly: IObservable; readonly connectionStatus: IObservable = this._connectionStatus; @@ -256,13 +243,10 @@ export class RemoteAgentHostSessionsProvider extends BaseAgentHostSessionsProvid this.connectionLabels = config.connectionLabels; this.canConnectOnDemand = !!config.connectOnDemand; this._readOnly = config.readOnlyWhenDisconnected - // Deliberately not `!isConnected`: that would include `reconnecting`, which is the - // protocol client restoring a dropped transport while the host itself is up. Input - // sent then is delivered once the transport returns, so hiding the composer would - // interrupt a conversation mid-sentence over a blip the user should not notice. ? derived(this, reader => { const status = this._connectionStatus.read(reader); return RemoteAgentHostConnectionStatus.isDisconnected(status) + || RemoteAgentHostConnectionStatus.isConnecting(status) || RemoteAgentHostConnectionStatus.isIncompatible(status); }) : constObservable(false); 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 11ea97bc0b5ad2..6d2ecbe33675ad 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 @@ -4,7 +4,9 @@ *--------------------------------------------------------------------------------------------*/ import assert from 'assert'; +import { DeferredPromise } from '../../../../../../base/common/async.js'; import { CancellationToken } from '../../../../../../base/common/cancellation.js'; +import { CancellationError } from '../../../../../../base/common/errors.js'; import { Event } from '../../../../../../base/common/event.js'; import { Disposable, DisposableStore, IDisposable, toDisposable } from '../../../../../../base/common/lifecycle.js'; import { mock, upcastPartial } from '../../../../../../base/test/common/mock.js'; @@ -31,7 +33,7 @@ import { } from '../../../../../../platform/agentHost/common/cloudSandboxAgentHost.js'; import { IRemoteAgentHostService, RemoteAgentHostConnectionStatus, RemoteAgentHostsEnabledSettingId } from '../../../../../../platform/agentHost/common/remoteAgentHostService.js'; import { IObservable, observableValue } from '../../../../../../base/common/observable.js'; -import { IConfigurationService } from '../../../../../../platform/configuration/common/configuration.js'; +import { ConfigurationTarget, IConfigurationService } from '../../../../../../platform/configuration/common/configuration.js'; import { TestConfigurationService } from '../../../../../../platform/configuration/test/common/testConfigurationService.js'; import { TestInstantiationService } from '../../../../../../platform/instantiation/test/common/instantiationServiceMock.js'; import { ILogService, NullLogService } from '../../../../../../platform/log/common/log.js'; @@ -156,6 +158,7 @@ const GITHUB_SANDBOX_GROUP: IAgentHostGroup = { interface ITestHarness { readonly contribution: TestCloudSandboxContribution; readonly configurationService: TestConfigurationService; + setEnabled(enabled: boolean): Promise; /** Discovery's answer, mutable so a test can change what a later pass reports. */ discovered: readonly ICloudSandboxDiscoveredSession[]; /** Runs a discovery pass and waits for it to reconcile. */ @@ -170,6 +173,7 @@ interface ITestHarness { activate(environmentId: string): Promise; readonly created: ICloudSandboxCreateSessionRequest[]; readonly connectedTo: string[]; + readonly historyRequests: string[]; /** Host groups currently declared to the filter service. */ readonly hostGroups: IAgentHostGroup[]; } @@ -182,6 +186,7 @@ interface ITestHarness { async function createContribution(store: Pick, sessions: readonly ICloudSandboxDiscoveredSession[], options?: { /** Task Mission Control returns from `createSession`, or a rejection. */ readonly createSession?: () => Promise; + readonly getEnvironment?: (id: string, token: CancellationToken) => Promise; /** Whether the sandbox feature settings start on. Defaults to `true`. */ readonly enabled?: boolean; }): Promise { @@ -191,13 +196,24 @@ async function createContribution(store: Pick, sessions: const instantiationService = store.add(new TestInstantiationService()); const created: ICloudSandboxCreateSessionRequest[] = []; const connectedTo: string[] = []; + const historyRequests: string[] = []; const harness: ITestHarness = { discovered: sessions, environmentStatus: 'offline', readOnlySessionTypes, created, connectedTo, + historyRequests, hostGroups, + setEnabled: async (enabled: boolean) => { + await configurationService.setUserConfiguration(CloudSandboxEnabledSettingId, enabled); + configurationService.onDidChangeConfigurationEmitter.fire({ + affectsConfiguration: key => key === CloudSandboxEnabledSettingId, + affectedKeys: new Set([CloudSandboxEnabledSettingId]), + change: { keys: [CloudSandboxEnabledSettingId], overrides: [] }, + source: ConfigurationTarget.USER, + }); + }, runDiscovery: async () => { await Promise.all(discoveryHandlers.map(handler => handler())); }, activate: async (environmentId: string) => { const sessionType = remoteAgentHostSessionTypeId(agentHostAuthority(cloudSandboxAddress(environmentId)), CLOUD_SANDBOX_AGENT_PROVIDER); @@ -209,10 +225,14 @@ async function createContribution(store: Pick, sessions: override async listSessions(_token: CancellationToken): Promise { return { kind: 'complete', sessions: harness.discovered }; } - override async getEnvironment(id: string): Promise { + override async getEnvironment(id: string, token: CancellationToken): Promise { + if (options?.getEnvironment) { + return options.getEnvironment(id, token); + } return { id, status: harness.environmentStatus }; } - override async getSessionHistory(): Promise { + override async getSessionHistory(taskId: string): Promise { + historyRequests.push(taskId); return { sessions: [], truncated: false }; } override async createSession(request: ICloudSandboxCreateSessionRequest): Promise { @@ -414,13 +434,120 @@ suite('CloudSandboxAgentHostContribution', () => { }); test('does not wake an environment whose state could not be read', async () => { - // A Mission Control blip must cost a click, not an unrequested resume. - const harness = await createContribution(store, [discoveredSession()]); - harness.environmentStatus = 'degraded'; + const harness = await createContribution(store, [discoveredSession()], { + getEnvironment: async () => { throw new Error('Expected environment lookup failure'); }, + }); const opened = await harness.activate('env-1'); - assert.deepStrictEqual({ opened, connectedTo: harness.connectedTo }, { opened: true, connectedTo: [] }); + assert.deepStrictEqual({ + opened, + connectedTo: harness.connectedTo, + historyRequests: harness.historyRequests, + servedFromHistory: harness.readOnlySessionTypes.length, + }, { opened: true, connectedTo: [], historyRequests: ['task-1'], servedFromHistory: 1 }); + }); + + for (const reenable of [false, true]) { + test(`abandons a cancelled environment lookup when the feature is ${reenable ? 're-enabled' : 'disabled'}`, async () => { + const environment = new DeferredPromise(); + const requestedToken = new DeferredPromise(); + const harness = await createContribution(store, [discoveredSession()], { + getEnvironment: (_id, token) => { + void requestedToken.complete(token); + return environment.p; + }, + }); + const activation = harness.activate('env-1'); + const token = await requestedToken.p; + + await harness.setEnabled(false); + if (reenable) { + await harness.setEnabled(true); + await harness.runDiscovery(); + } + await environment.error(new CancellationError()); + + assert.deepStrictEqual({ + opened: await activation, + cancelled: token.isCancellationRequested, + connectedTo: harness.connectedTo, + historyRequests: harness.historyRequests, + readOnlySessionTypes: harness.readOnlySessionTypes, + }, { opened: false, cancelled: true, connectedTo: [], historyRequests: [], readOnlySessionTypes: [] }); + }); + } + + for (const status of ['online', 'offline'] as const) { + test(`does not reactivate a removed environment after a late ${status} record`, async () => { + const environment = new DeferredPromise(); + const harness = await createContribution(store, [discoveredSession()], { + getEnvironment: () => environment.p, + }); + const activation = harness.activate('env-1'); + harness.discovered = []; + await harness.runDiscovery(); + await environment.complete({ id: 'env-1', status }); + + assert.deepStrictEqual({ + opened: await activation, + connectedTo: harness.connectedTo, + historyRequests: harness.historyRequests, + readOnlySessionTypes: harness.readOnlySessionTypes, + }, { opened: false, connectedTo: [], historyRequests: [], readOnlySessionTypes: [] }); + }); + } + + test('does not register old history against a replacement provider at the same address', async () => { + const environment = new DeferredPromise(); + const harness = await createContribution(store, [discoveredSession()], { + getEnvironment: () => environment.p, + }); + const activation = harness.activate('env-1'); + harness.discovered = []; + await harness.runDiscovery(); + harness.discovered = [discoveredSession({ taskId: 'task-2' })]; + await harness.runDiscovery(); + await environment.complete({ id: 'env-1', status: 'offline' }); + + assert.deepStrictEqual({ + opened: await activation, + historyRequests: harness.historyRequests, + readOnlySessionTypes: harness.readOnlySessionTypes, + }, { opened: false, historyRequests: [], readOnlySessionTypes: [] }); + }); + + test('keeps activation valid across a discovery refresh of the same provider', async () => { + const environment = new DeferredPromise(); + const harness = await createContribution(store, [discoveredSession()], { + getEnvironment: () => environment.p, + }); + const activation = harness.activate('env-1'); + await harness.runDiscovery(); + await environment.complete({ id: 'env-1', status: 'offline' }); + + assert.deepStrictEqual({ + opened: await activation, + connectedTo: harness.connectedTo, + historyRequests: harness.historyRequests, + }, { opened: true, connectedTo: [], historyRequests: ['task-1'] }); + }); + + test('does not restore history after an old connect fails across disable and re-enable', async () => { + const harness = await createContribution(store, [discoveredSession()]); + harness.environmentStatus = 'online'; + harness.onConnect = async () => { + await harness.setEnabled(false); + await harness.setEnabled(true); + await harness.runDiscovery(); + throw new Error('Expected connection failure after teardown'); + }; + + assert.deepStrictEqual({ + opened: await harness.activate('env-1'), + historyRequests: harness.historyRequests, + readOnlySessionTypes: harness.readOnlySessionTypes, + }, { opened: false, historyRequests: [], readOnlySessionTypes: [] }); }); test('connects a dormant environment that has no history to fall back on', async () => { 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 84dd0e8a31e9eb..eb407dbd179a6c 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 @@ -484,28 +484,36 @@ suite('RemoteAgentHostSessionsProvider', () => { }); }); - test('keeps the composer during a self-healing reconnect on a host that is read-only when disconnected', () => { - // `reconnecting` is the protocol client restoring a dropped transport while the host is - // still up, so input sent then is delivered once it returns. Treating it as read-only - // would pull the composer — and the user's focus — mid-conversation over a blip. + test('keeps initial connections read-only but permits self-healing reconnects', () => { const provider = createProvider(disposables, connection, { readOnlyWhenDisconnected: true }); - provider.setConnectionStatus(RemoteAgentHostConnectionStatus.connected); provider.seedSessions([createSession('sandbox-session')]); const chat = provider.getSessions()[0].mainChat.get(); - const interactivity = [chat.interactivity.get()]; - - provider.setConnectionStatus(RemoteAgentHostConnectionStatus.reconnecting); - interactivity.push(chat.interactivity.get()); - provider.setConnectionStatus(RemoteAgentHostConnectionStatus.disconnected); - interactivity.push(chat.interactivity.get()); - provider.setConnectionStatus(RemoteAgentHostConnectionStatus.connected); - interactivity.push(chat.interactivity.get()); + const statuses = [ + RemoteAgentHostConnectionStatus.disconnected, + RemoteAgentHostConnectionStatus.connecting, + RemoteAgentHostConnectionStatus.connected, + RemoteAgentHostConnectionStatus.reconnecting, + RemoteAgentHostConnectionStatus.disconnected, + RemoteAgentHostConnectionStatus.connecting, + RemoteAgentHostConnectionStatus.disconnected, + RemoteAgentHostConnectionStatus.incompatible('Protocol version mismatch', ['1']), + RemoteAgentHostConnectionStatus.connected, + ]; + const interactivity = statuses.map(status => { + provider.setConnectionStatus(status); + return { status: status.kind, interactivity: chat.interactivity.get() }; + }); assert.deepStrictEqual(interactivity, [ - ChatInteractivity.Full, - ChatInteractivity.Full, - ChatInteractivity.ReadOnly, - ChatInteractivity.Full, + { status: 'disconnected', interactivity: ChatInteractivity.ReadOnly }, + { status: 'connecting', interactivity: ChatInteractivity.ReadOnly }, + { status: 'connected', interactivity: ChatInteractivity.Full }, + { status: 'reconnecting', interactivity: ChatInteractivity.Full }, + { status: 'disconnected', interactivity: ChatInteractivity.ReadOnly }, + { status: 'connecting', interactivity: ChatInteractivity.ReadOnly }, + { status: 'disconnected', interactivity: ChatInteractivity.ReadOnly }, + { status: 'incompatible', interactivity: ChatInteractivity.ReadOnly }, + { status: 'connected', interactivity: ChatInteractivity.Full }, ]); });