Skip to content

Commit b67c3e1

Browse files
osortegaCopilot
andauthored
Enhance cloud sandbox UX for remote connections (#334644)
* Agent Host changes for agents/cloud-sandbox-ux-enhancement * 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> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent e3c3c45 commit b67c3e1

14 files changed

Lines changed: 768 additions & 167 deletions

src/vs/sessions/browser/parts/chatGroupView.ts

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -230,30 +230,41 @@ export class ChatGroupView extends Disposable implements ISerializableView {
230230
if (archived) {
231231
const action = getChatSessionArchiveActionPresentation(this._archiveActionWording.read(reader)).unarchive;
232232
return {
233-
message: localize('sessionReadOnlyBanner.archived', "Archived sessions are read-only."),
234-
action: {
235-
label: action.title.value,
236-
run: () => this._commandService.executeCommand(UNARCHIVE_SESSION_COMMAND_ID, context.session),
233+
archived: true,
234+
content: {
235+
message: localize('sessionReadOnlyBanner.archived', "Archived sessions are read-only."),
236+
action: {
237+
label: action.title.value,
238+
run: () => this._commandService.executeCommand(UNARCHIVE_SESSION_COMMAND_ID, context.session),
239+
},
237240
},
238241
};
239242
}
240-
return { message: localize('sessionReadOnlyBanner.message', "This chat is read-only") };
243+
return { archived: false, content: { message: localize('sessionReadOnlyBanner.message', "This chat is read-only") } };
241244
});
242245

243246
const surface = derived<IChatGroupSurface>(reader => {
244247
const readOnly = readOnlyContent.read(reader);
245-
if (readOnly) {
246-
return { banner: readOnly, recovery: undefined };
248+
if (readOnly?.archived) {
249+
return { banner: readOnly.content, recovery: undefined };
247250
}
248251

252+
// Keep the banner while history loads to avoid flashing the centered recovery state.
249253
const view = currentView.read(reader);
250-
const recovery = view?.hasVisibleTranscriptContent.read(reader)
254+
const transcriptSettled = view === undefined || !view.isLoadingTranscript.read(reader);
255+
const recovery = !transcriptSettled || view?.hasVisibleTranscriptContent.read(reader)
251256
? undefined
252257
: this._connection.recoveryContent.read(reader);
253258
if (recovery) {
254259
return { banner: undefined, recovery };
255260
}
256-
return { banner: this._connection.bannerContent.read(reader), recovery: undefined };
261+
// Explain connection-related read-only state before falling back to the generic notice.
262+
const connectionBanner = this._connection.bannerContent.read(reader);
263+
if (connectionBanner) {
264+
return { banner: connectionBanner, recovery: undefined };
265+
}
266+
267+
return { banner: readOnly?.content, recovery: undefined };
257268
});
258269

259270
this._contextDisposables.add(autorun(reader => {

src/vs/sessions/browser/parts/chatView.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,14 @@ export abstract class AbstractChatView extends Disposable implements ISerializab
7272
*/
7373
readonly hasVisibleTranscriptContent: IObservable<boolean> = constObservable(false);
7474

75+
/**
76+
* Whether this view is still resolving its chat model, during which
77+
* {@link hasVisibleTranscriptContent} is not yet meaningful — it reads `false` for a transcript
78+
* that simply has not arrived yet as well as for one that does not exist. Views that never load
79+
* a model report `false`, since for them the answer is already final.
80+
*/
81+
readonly isLoadingTranscript: IObservable<boolean> = constObservable(false);
82+
7583
/**
7684
* Show the given chat in this view. The default implementation is a
7785
* no-op; subclasses that host a chat widget (e.g. `ChatView`) override

src/vs/sessions/browser/parts/media/remoteHostUnavailableEmptyState.css

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
}
1919

2020
.remote-host-unavailable-empty-state.hidden,
21+
.remote-host-unavailable-empty-state-description.hidden,
2122
.remote-host-unavailable-empty-state-progress.hidden,
2223
.remote-host-unavailable-empty-state-action.hidden,
2324
.remote-host-unavailable-empty-state-auto-connect.hidden {

src/vs/sessions/browser/parts/remoteHostUnavailableEmptyState.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,8 @@ import { defaultButtonStyles, defaultCheckboxStyles } from '../../../platform/th
1515

1616
export interface IRemoteHostUnavailableEmptyStateContent {
1717
readonly title: string;
18-
readonly description: string;
18+
/** Omitted when the title already says everything, leaving the title and action to speak. */
19+
readonly description?: string;
1920
readonly progress?: string;
2021
readonly action?: {
2122
readonly label: string;
@@ -59,7 +60,7 @@ export class RemoteHostUnavailableEmptyState extends Disposable {
5960
icon.appendChild(renderIcon(Codicon.debugDisconnect));
6061

6162
this._title = dom.append(this.domNode, dom.$('h2.remote-host-unavailable-empty-state-title'));
62-
this._description = dom.append(this.domNode, dom.$('p.remote-host-unavailable-empty-state-description'));
63+
this._description = dom.append(this.domNode, dom.$('p.remote-host-unavailable-empty-state-description.hidden'));
6364
this._progress = dom.append(this.domNode, dom.$('p.remote-host-unavailable-empty-state-progress.hidden'));
6465
// Connect progress changes while the user waits (waiting → download
6566
// percentage), so announce it politely rather than leaving it silent.
@@ -85,6 +86,8 @@ export class RemoteHostUnavailableEmptyState extends Disposable {
8586
this._autoConnectListener.clear();
8687
this.domNode.classList.toggle('hidden', !content);
8788
if (!content) {
89+
this._description.textContent = '';
90+
this._description.classList.add('hidden');
8891
this._progress.textContent = '';
8992
this._progress.classList.add('hidden');
9093
this._actionContainer.classList.add('hidden');
@@ -98,7 +101,8 @@ export class RemoteHostUnavailableEmptyState extends Disposable {
98101

99102
this.domNode.setAttribute('aria-label', content.title);
100103
this._title.textContent = content.title;
101-
this._description.textContent = content.description;
104+
this._description.textContent = content.description ?? '';
105+
this._description.classList.toggle('hidden', !content.description);
102106
this._progress.textContent = content.progress ?? '';
103107
this._progress.classList.toggle('hidden', !content.progress);
104108
if (!content.action) {

src/vs/sessions/browser/parts/sessionRemoteConnection.ts

Lines changed: 69 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,10 @@ import { Disposable, MutableDisposable } from '../../../base/common/lifecycle.js
1010
import { autorun, derived, derivedObservableWithCache, IObservable, IReader, observableSignal, observableValue, transaction } from '../../../base/common/observable.js';
1111
import { localize } from '../../../nls.js';
1212
import { ILogService } from '../../../platform/log/common/log.js';
13-
import { isAgentHostProvider } from '../../common/agentHostSessionsProvider.js';
13+
import { IAgentHostConnectionLabels, isAgentHostProvider } from '../../common/agentHostSessionsProvider.js';
1414
import { SessionRemoteConnectionFailureReason, SessionRemoteConnectionStatus } from '../../services/sessions/common/session.js';
1515
import { IActiveSession } from '../../services/sessions/common/sessionsManagement.js';
16+
import { ISessionsProvider } from '../../services/sessions/common/sessionsProvider.js';
1617
import { ISessionsProvidersService } from '../../services/sessions/browser/sessionsProvidersService.js';
1718
import { IRemoteHostUnavailableEmptyStateContent } from './remoteHostUnavailableEmptyState.js';
1819
import { ISessionReadOnlyBannerContent } from './sessionReadOnlyBanner.js';
@@ -63,6 +64,13 @@ export class SessionRemoteConnection extends Disposable {
6364
* `connect()` joins an in-flight dial rather than starting a second one.
6465
*/
6566
private readonly _autoConnected = observableValue<IActiveSession | undefined>(this, undefined);
67+
/**
68+
* The session a connect has already been started for, however it ended. Distinguishes a host
69+
* that has never been dialled — where the action reads as "Connect" — from one that was tried
70+
* and did not come up, where it reads as "Retry". Latched separately from {@link _attempt},
71+
* which clears once an attempt settles and so cannot answer "has this been tried at all".
72+
*/
73+
private readonly _connectAttempted = observableValue<IActiveSession | undefined>(this, undefined);
6674
/**
6775
* Whether the host should be started without waiting for a click. Gated on a
6876
* stopped host the provider can start, and on not having tried yet — one
@@ -170,6 +178,7 @@ export class SessionRemoteConnection extends Disposable {
170178
this._session.set(session, tx);
171179
this._attempt.set(undefined, tx);
172180
this._autoConnected.set(undefined, tx);
181+
this._connectAttempted.set(undefined, tx);
173182
});
174183
}
175184

@@ -197,7 +206,10 @@ export class SessionRemoteConnection extends Disposable {
197206

198207
const statusBefore = session.remoteConnectionStatus?.get();
199208
this._logService.info(`[SessionRemoteConnection] connect: starting ${provider.remoteAddress ?? provider.id}, statusBefore=${statusBefore?.kind}`);
200-
this._attempt.set({ kind: 'active', session, message: undefined, statusBefore }, undefined);
209+
transaction(tx => {
210+
this._attempt.set({ kind: 'active', session, message: undefined, statusBefore }, tx);
211+
this._connectAttempted.set(session, tx);
212+
});
201213
const remoteAddress = provider.remoteAddress;
202214
if (remoteAddress && provider.onDidReportConnectProgress) {
203215
this._progressListener.value = provider.onDidReportConnectProgress(progress => {
@@ -255,7 +267,26 @@ export class SessionRemoteConnection extends Disposable {
255267
: status;
256268
}
257269

258-
private _getRemoteHostConnectProgress(session: IActiveSession, status: SessionRemoteConnectionStatus | undefined, reader: IReader): string | undefined {
270+
private _getConnectionLabels(provider: ISessionsProvider | undefined): IAgentHostConnectionLabels {
271+
if (provider && isAgentHostProvider(provider) && provider.connectionLabels) {
272+
return provider.connectionLabels;
273+
}
274+
const hostLabel = provider?.label ?? localize('sessionRemoteHost.unknown', "The remote host");
275+
return {
276+
unavailableTitle: localize('sessionRemoteHost.disconnectedTitle', "Cannot Connect to {0}", hostLabel),
277+
unavailableDescription: localize('sessionRemoteHost.disconnected', "Cannot reach {0}.", hostLabel),
278+
unavailable: localize('sessionRemoteHost.disconnected', "Cannot reach {0}.", hostLabel),
279+
connectingTitle: localize('sessionRemoteHost.connectingTitle', "Connecting to {0}", hostLabel),
280+
connectingDescription: localize('sessionRemoteHost.startingDescription', "Starting {0}.", hostLabel),
281+
connecting: localize('sessionRemoteHost.waitingForConnection', "Waiting for agent host connection..."),
282+
reconnecting: localize('sessionRemoteHost.reconnecting', "Reconnecting to {0}...", hostLabel),
283+
reconnectingIn: seconds => localize('sessionRemoteHost.reconnectingIn', "Reconnecting to {0} in {1}s", hostLabel, seconds),
284+
incompatibleTitle: localize('sessionRemoteHost.incompatibleTitle', "Cannot Connect to {0}", hostLabel),
285+
incompatible: localize('sessionRemoteHost.incompatibleDescription', "{0} is incompatible with this version of Visual Studio Code.", hostLabel),
286+
};
287+
}
288+
289+
private _getRemoteHostConnectProgress(session: IActiveSession, status: SessionRemoteConnectionStatus | undefined, labels: IAgentHostConnectionLabels, reader: IReader): string | undefined {
259290
const attempt = this._attempt.read(reader);
260291
if (attempt?.kind !== 'active' || attempt.session !== session || status?.kind === 'connected') {
261292
return undefined;
@@ -271,9 +302,7 @@ export class SessionRemoteConnection extends Disposable {
271302
const settling = status?.kind === 'connecting'
272303
|| status?.kind === 'reconnecting'
273304
|| isSameRemoteConnectionStatus(status, attempt.statusBefore);
274-
return settling
275-
? localize('sessionRemoteHost.waitingForConnection', "Waiting for agent host connection...")
276-
: undefined;
305+
return settling ? labels.connecting : undefined;
277306
}
278307

279308
private _getRemoteHostUnavailableContent(reader: IReader): IRemoteHostUnavailableEmptyStateContent | undefined {
@@ -285,16 +314,17 @@ export class SessionRemoteConnection extends Disposable {
285314
const status = this._getEffectiveStatus(reader);
286315
const provider = this._sessionsProvidersService.getProvider(session.providerId);
287316
const hostLabel = provider?.label ?? localize('sessionRemoteHost.unknown', "The remote host");
317+
const labels = this._getConnectionLabels(provider);
288318
if (status?.kind === 'connected') {
289319
return undefined;
290320
}
291321
if (status?.kind === 'incompatible') {
292322
return {
293-
title: localize('sessionRemoteHost.incompatibleTitle', "Cannot Connect to {0}", hostLabel),
294-
description: localize('sessionRemoteHost.incompatibleDescription', "{0} is incompatible with this version of Visual Studio Code.", hostLabel),
323+
title: labels.incompatibleTitle,
324+
description: labels.incompatible,
295325
};
296326
}
297-
const progressMessage = this._getRemoteHostConnectProgress(session, status, reader);
327+
const progressMessage = this._getRemoteHostConnectProgress(session, status, labels, reader);
298328
const autoConnectPending = this._autoConnectPending.read(reader);
299329
const canStartHost = !!provider && isAgentHostProvider(provider) && !!provider.connect;
300330
const autoConnect = canStartHost && provider.autoConnect
@@ -310,11 +340,12 @@ export class SessionRemoteConnection extends Disposable {
310340
&& attempt.session === session
311341
&& attempt.statusBefore?.kind === 'disconnected'
312342
&& attempt.statusBefore.reason === SessionRemoteConnectionFailureReason.HostNotRunning);
313-
if (progressMessage || autoConnectPending) {
343+
// Include externally started connects; reconnecting keeps its delayed countdown banner.
344+
if (progressMessage || autoConnectPending || status?.kind === 'connecting') {
314345
return {
315-
title: localize('sessionRemoteHost.connectingTitle', "Connecting to {0}", hostLabel),
316-
description: localize('sessionRemoteHost.startingDescription', "Starting {0}.", hostLabel),
317-
progress: progressMessage ?? localize('sessionRemoteHost.waitingForConnection', "Waiting for agent host connection..."),
346+
title: labels.connectingTitle,
347+
description: labels.connectingDescription,
348+
progress: progressMessage ?? labels.connecting,
318349
autoConnect: startedFromStoppedHost ? autoConnect : undefined,
319350
};
320351
}
@@ -335,14 +366,14 @@ export class SessionRemoteConnection extends Disposable {
335366
};
336367
}
337368
return {
338-
title: localize('sessionRemoteHost.disconnectedTitle', "Cannot Connect to {0}", hostLabel),
339-
description: localize('sessionRemoteHost.disconnected', "Cannot reach {0}.", hostLabel),
369+
title: labels.unavailableTitle,
370+
description: labels.unavailableDescription,
340371
// An unreachable host is often transient — a dropped tunnel, a sleeping
341372
// machine — so offer a manual retry even though there is nothing local
342373
// to start and so nothing to do automatically.
343374
action: canStartHost
344375
? {
345-
label: localize('sessionRemoteHost.retry', "Retry"),
376+
label: this._connectActionLabel(session, reader),
346377
run: () => this.connect(),
347378
}
348379
: undefined,
@@ -358,17 +389,18 @@ export class SessionRemoteConnection extends Disposable {
358389
const status = this._getEffectiveStatus(reader);
359390
const provider = this._sessionsProvidersService.getProvider(session.providerId);
360391
const hostLabel = provider?.label ?? localize('sessionRemoteHost.unknown', "The remote host");
361-
const progressMessage = this._getRemoteHostConnectProgress(session, status, reader);
392+
const labels = this._getConnectionLabels(provider);
393+
const progressMessage = this._getRemoteHostConnectProgress(session, status, labels, reader);
362394
// Mirror the recovery state: while an automatic start is pending the
363395
// action must not appear, even for the frame before the attempt registers.
364-
if (progressMessage || this._autoConnectPending.read(reader)) {
396+
if (progressMessage || this._autoConnectPending.read(reader) || status?.kind === 'connecting') {
365397
return {
366398
icon: Codicon.sync,
367-
message: progressMessage ?? localize('sessionRemoteHost.waitingForConnection', "Waiting for agent host connection..."),
399+
message: progressMessage ?? labels.connecting,
368400
};
369401
}
370402

371-
if (!status || status.kind === 'connected' || status.kind === 'connecting') {
403+
if (!status || status.kind === 'connected') {
372404
return undefined;
373405
}
374406

@@ -377,7 +409,7 @@ export class SessionRemoteConnection extends Disposable {
377409
// so this banner is the only place the incompatibility can be explained.
378410
return {
379411
icon: Codicon.debugDisconnect,
380-
message: localize('sessionRemoteHost.incompatibleDescription', "{0} is incompatible with this version of Visual Studio Code.", hostLabel),
412+
message: labels.incompatible,
381413
};
382414
}
383415

@@ -386,13 +418,11 @@ export class SessionRemoteConnection extends Disposable {
386418
return this._reconnectingBannerVisible.read(reader)
387419
? {
388420
icon: Codicon.sync,
389-
message: seconds === undefined
390-
? localize('sessionRemoteHost.reconnecting', "Reconnecting to {0}...", hostLabel)
391-
: localize('sessionRemoteHost.reconnectingIn', "Reconnecting to {0} in {1}s", hostLabel, seconds),
421+
message: seconds === undefined ? labels.reconnecting : labels.reconnectingIn(seconds),
392422
// The banner is a live region, so the per-second countdown would
393423
// otherwise queue an announcement every tick. Keep the spoken
394424
// text stable and let only the visible text count down.
395-
ariaLabel: localize('sessionRemoteHost.reconnecting', "Reconnecting to {0}...", hostLabel),
425+
ariaLabel: labels.reconnecting,
396426
action: seconds !== undefined && provider && isAgentHostProvider(provider) && provider.reconnectNow
397427
? {
398428
label: localize('sessionRemoteHost.tryNow', "Try Now"),
@@ -416,15 +446,27 @@ export class SessionRemoteConnection extends Disposable {
416446
}
417447
return {
418448
icon: Codicon.debugDisconnect,
419-
message: localize('sessionRemoteHost.disconnected', "Cannot reach {0}.", hostLabel),
449+
message: labels.unavailable,
420450
// A dropped tunnel or a sleeping machine is usually transient, so a
421451
// manual retry is worth offering even with nothing local to start.
422452
action: provider && isAgentHostProvider(provider) && provider.connect
423453
? {
424-
label: localize('sessionRemoteHost.retry', "Retry"),
454+
label: this._connectActionLabel(session, reader),
425455
run: () => this.connect(),
426456
}
427457
: undefined,
428458
};
429459
}
460+
461+
/**
462+
* The wording for the action that establishes the connection. A host that has never been
463+
* dialled from this view is offered a plain "Connect": presenting it as a retry would imply an
464+
* attempt the user never made, and for hosts that must be resumed rather than merely reached,
465+
* the first dial is a deliberate choice rather than a recovery.
466+
*/
467+
private _connectActionLabel(session: IActiveSession, reader: IReader): string {
468+
return this._connectAttempted.read(reader) === session
469+
? localize('sessionRemoteHost.retry', "Retry")
470+
: localize('sessionRemoteHost.connect', "Connect");
471+
}
430472
}

src/vs/sessions/common/agentHostSessionsProvider.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,20 @@ export interface IAgentHostAutoConnect {
4646
setEnabled(enabled: boolean): void;
4747
}
4848

49+
/** Localized labels shared by connection banners and recovery screens. */
50+
export interface IAgentHostConnectionLabels {
51+
readonly unavailableTitle: string;
52+
readonly unavailableDescription?: string;
53+
readonly unavailable: string;
54+
readonly connectingTitle: string;
55+
readonly connectingDescription?: string;
56+
readonly connecting: string;
57+
readonly reconnecting: string;
58+
reconnectingIn(seconds: number): string;
59+
readonly incompatibleTitle: string;
60+
readonly incompatible: string;
61+
}
62+
4963
/**
5064
* Declares that a provider is one of many interchangeable members of a single
5165
* user-facing host. Members collapse into one `IAgentHostFilterEntry` that
@@ -153,6 +167,9 @@ export interface IAgentHostSessionsProvider extends ISessionsProvider {
153167
*/
154168
readonly autoConnect?: IAgentHostAutoConnect;
155169

170+
/** Optional labels for providers whose display name does not name the host. */
171+
readonly connectionLabels?: IAgentHostConnectionLabels;
172+
156173
/**
157174
* When `true`, the workspace picker keeps this provider's browse
158175
* action(s) enabled even while {@link connectionStatus} reports

0 commit comments

Comments
 (0)