Adopt ISessionsProvider for remote agent hosts

[roblourens]

Co-authored-by: Copilot copilot@github.com

[Copilot]

Pull request overview

This PR updates the Sessions app’s remote agent host integration to use an ISessionsProvider-based implementation so remote agent-host sessions can be listed and managed consistently in the Sessions UI.

Changes:

• Implement a full RemoteAgentHostSessionsProvider with session caching, notifications-based updates, basic actions (delete/read), and initial send flow integration.
• Register one sessions provider per remote agent-host connection (and unwrap vscode-agent-host:// URIs back to original paths where needed).
• Add a Sessions command for adding remote agent hosts and introduce a new unit test suite for the provider.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSessionListController.ts	Filter sessionRemoved notifications by provider to avoid cross-provider removals.
src/vs/sessions/sessions.desktop.main.ts	Wire remote agent host actions into Sessions desktop main entrypoint.
src/vs/sessions/contrib/remoteAgentHost/browser/remoteAgentHostSessionsProvider.ts	New ISessionsProvider implementation for remote agent host connections (cache + send flow + actions).
src/vs/sessions/contrib/remoteAgentHost/browser/remoteAgentHostActions.ts	Add command/action to add a remote agent host from the Sessions app.
src/vs/sessions/contrib/remoteAgentHost/browser/remoteAgentHost.contribution.ts	Register a single sessions provider per connection and unwrap agent-host URIs when deriving working directories.
src/vs/sessions/contrib/remoteAgentHost/test/browser/remoteAgentHostSessionsProvider.test.ts	New tests for provider identity, caching, notifications, and send flow behavior.

Comments suppressed due to low confidence (9)

src/vs/sessions/contrib/remoteAgentHost/test/browser/remoteAgentHostSessionsProvider.test.ts:275

• The tests wait for async refreshes using setTimeout(..., 50). This makes the suite timing-dependent and potentially flaky under load/slow CI. Prefer waiting on an explicit signal (e.g. Event.toPromise(provider.onDidChangeSessions) / a deferred) or using fake timers so the test deterministically waits for the refresh completion.

		provider.getSessions();
		await new Promise(resolve => setTimeout(resolve, 50));

src/vs/sessions/contrib/remoteAgentHost/browser/remoteAgentHostSessionsProvider.ts:178

• _sessionCache is keyed only by the raw session ID, but this provider also accepts notifications/listings from arbitrary agent providers (and builds per-provider resource schemes). If two providers ever emit the same raw ID, entries will collide (and removals can delete the wrong session). Either include the provider in the cache key (e.g. provider+id or full session URI) or filter the provider to a single supported backend.

	/** Cache of adapted sessions, keyed by raw session ID. */
	private readonly _sessionCache = new Map<string, RemoteSessionAdapter>();

src/vs/sessions/contrib/remoteAgentHost/browser/remoteAgentHostSessionsProvider.ts:223

• The provider listens to notify/sessionAdded/notify/sessionRemoved without filtering by supported agent provider. In this codebase RemoteAgentHostContribution currently ignores non-copilot agents, so sessions from other providers could be surfaced here without a corresponding chat session contribution/handler to open them. Consider filtering to the supported provider(s) (or extending the contribution/provider to register handlers for all providers you intend to list).

		// Listen for session notifications from the connection
		this._register(this._connection.onDidNotification(n => {
			if (n.type === 'notify/sessionAdded') {
				this._handleSessionAdded(n.summary);
			} else if (n.type === 'notify/sessionRemoved') {
				this._handleSessionRemoved(n.session);
			}
		}));

src/vs/sessions/contrib/remoteAgentHost/test/browser/remoteAgentHostSessionsProvider.test.ts:187

• This test asserts the browse action label includes 'Browse', but the provider currently uses localize('folders', "Folders"). Update the expectation (or update the provider label) so they match; otherwise this test will fail deterministically.

		assert.strictEqual(provider.browseActions.length, 1);
		assert.ok(provider.browseActions[0].label.includes('Browse'));
		assert.strictEqual(provider.browseActions[0].providerId, provider.id);
	});

src/vs/sessions/contrib/remoteAgentHost/browser/remoteAgentHostSessionsProvider.ts:397

• In sendRequest, existingKeys is captured from _sessionCache without first ensuring the cache is populated with the current listSessions() result. If the cache hasn’t been initialized/refreshed yet, _waitForNewSession may treat pre-existing sessions as “new” and return the wrong session. Consider forcing an initial refresh (and awaiting it) before capturing existingKeys so delta detection is correct.

		// Track existing sessions before sending so we can detect the new one
		const existingKeys = new Set(this._sessionCache.keys());

		// Send request through the chat service, which delegates to the
		// AgentHostSessionHandler content provider for turn handling
		const result = await this._chatService.sendRequest(session.resource, query, sendOptions);
		if (result.kind === 'rejected') {

src/vs/sessions/contrib/remoteAgentHost/browser/remoteAgentHostSessionsProvider.ts:487

• _waitForNewSession returns a Promise that never resolves if the backend never produces a new session (e.g. send fails server-side without a notification). That can hang the UI and leaves an event listener attached indefinitely. Add a timeout and/or CancellationToken support, and ensure the listener is disposed on all paths.

	private async _waitForNewSession(existingKeys: Set<string>): Promise<ISessionData | undefined> {
		// First, try an immediate refresh
		await this._refreshSessions(CancellationToken.None);
		for (const [key, adapter] of this._sessionCache) {
			if (!existingKeys.has(key)) {
				return adapter;
			}
		}

		// If not found yet, wait for the next onDidChangeSessions event
		return new Promise<ISessionData | undefined>(resolve => {
			const listener = this._onDidChangeSessions.event(e => {
				const newSession = e.added.find(s => {
					const rawId = s.resource.path.substring(1);
					return !existingKeys.has(rawId);
				});
				if (newSession) {
					listener.dispose();
					resolve(newSession);
				}
			});
		});

src/vs/sessions/contrib/remoteAgentHost/browser/remoteAgentHostSessionsProvider.ts:151

• RemoteSessionAdapter._buildWorkspace sets requiresWorkspaceTrust: false, but other workspaces from this provider (resolveWorkspace, createNewSession, and folder browsing) set it to true. This inconsistency can skip the trust gate for sessions populated from workingDirectory. Align this value with the rest of the provider (or document why it is safe to disable trust here).

		return {
			label,
			icon: Codicon.remote,
			repositories: [{ uri, workingDirectory: undefined, detail: providerLabel, baseBranchProtected: undefined }],
			requiresWorkspaceTrust: false
		};

src/vs/sessions/contrib/remoteAgentHost/browser/remoteAgentHostSessionsProvider.ts:342

• setRead mutates the adapter’s isRead observable via a type assertion to a settable observable. This is brittle (it depends on the adapter implementation detail) and makes refactors risky. Prefer exposing a method on RemoteSessionAdapter (e.g. setRead(...)) or storing the underlying settable observable on the adapter in a way that doesn’t require casting.

	setRead(sessionId: string, read: boolean): void {
		const adapter = this._findAdapter(sessionId);
		if (adapter) {
			// Track read state locally since the protocol doesn't support it
			(adapter.isRead as ReturnType<typeof observableValue<boolean>>).set(read, undefined);
		}

src/vs/sessions/contrib/remoteAgentHost/test/browser/remoteAgentHostSessionsProvider.test.ts:177

• Several tests construct remote folder URIs with URI.parse('vscode-agent-host://auth/home/user/project'), but AGENT_HOST_SCHEME URIs are expected to encode the original scheme/authority in the path (e.g. /file/-/home/user/project, produced by agentHostUri(...) / toAgentHostUri(...)). Using the simplified URI risks missing bugs around real-world path encoding and authority handling (including Windows paths). Consider building URIs with the same helpers used in production code.

	test('resolveWorkspace builds workspace from URI', () => {
		const provider = createProvider(disposables, connection);
		const uri = URI.parse('vscode-agent-host://auth/home/user/project');
		const ws = provider.resolveWorkspace(uri);

		assert.strictEqual(ws.label, 'project');
		assert.strictEqual(ws.repositories.length, 1);
		assert.strictEqual(ws.repositories[0].uri.toString(), uri.toString());
	});