diff --git a/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts b/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts index 3386bf7085f6d6..2c980ea5769de6 100644 --- a/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts +++ b/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts @@ -1106,6 +1106,25 @@ export abstract class BaseAgentHostSessionsProvider extends Disposable implement protected _cacheInitialized = false; + private static readonly SESSION_REFRESH_RETRY_MIN_MS = 1_000; + private static readonly SESSION_REFRESH_RETRY_MAX_MS = 30_000; + + /** + * Backoff timer that retries {@link _refreshSessions} after a failed + * attempt. A failed initial list (e.g. the agent threw + * `AHP_AUTH_REQUIRED` because its token wasn't yet effective server-side, + * or a transient offline/network error) must not leave the session list + * permanently empty. The timer is armed only on failure and cancelled on + * the next successful refresh. + */ + private readonly _sessionRefreshRetry = this._register(new MutableDisposable()); + + /** Current backoff delay (ms) for the session-refresh retry. */ + private _sessionRefreshRetryDelay = BaseAgentHostSessionsProvider.SESSION_REFRESH_RETRY_MIN_MS; + + /** True while a {@link _refreshSessions} call is awaiting `listSessions()`. */ + private _sessionRefreshInFlight = false; + constructor( @IChatSessionsService protected readonly _chatSessionsService: IChatSessionsService, @IChatService protected readonly _chatService: IChatService, @@ -2112,7 +2131,17 @@ export abstract class BaseAgentHostSessionsProvider extends Disposable implement if (this._cacheInitialized) { return; } - this._cacheInitialized = true; + // `_refreshSessions` owns `_cacheInitialized` — it flips it to `true` + // only once `listSessions()` actually returns. A call that races + // before the connection/auth is ready will fail and arm a retry + // rather than permanently pinning an empty cache. Don't launch a new + // refresh while one is already in flight or a backoff retry is already + // scheduled — otherwise every synchronous `getSessions()` during the + // failure window would hammer the agent/auth path and bypass the + // backoff. + if (this._sessionRefreshInFlight || this._sessionRefreshRetry.value) { + return; + } this._refreshSessions(); } @@ -2121,8 +2150,15 @@ export abstract class BaseAgentHostSessionsProvider extends Disposable implement if (!connection) { return; } + // Cancel any pending retry; this attempt supersedes it. + this._sessionRefreshRetry.clear(); + this._sessionRefreshInFlight = true; try { const sessions = await connection.listSessions(); + // A successful return (even an empty list) means the cache is + // authoritative. Mark it initialized and reset the backoff. + this._cacheInitialized = true; + this._sessionRefreshRetryDelay = BaseAgentHostSessionsProvider.SESSION_REFRESH_RETRY_MIN_MS; const currentKeys = new Set(); const added: ISession[] = []; const changed: ISession[] = []; @@ -2158,11 +2194,46 @@ export abstract class BaseAgentHostSessionsProvider extends Disposable implement if (added.length > 0 || removed.length > 0 || changed.length > 0) { this._onDidChangeSessions.fire({ added, removed, changed }); } - } catch { - // Connection may not be ready yet + } catch (err) { + // The connection / agent may not be ready yet — e.g. the agent + // throws `AHP_AUTH_REQUIRED` until its token is effective + // server-side, or there's a transient offline/network error. We + // must NOT mark the cache initialized (that would conflate a + // failure with a genuinely-empty success and never recover), and + // we deliberately do NOT pop a sign-in dialog just to render the + // list. Instead, retry silently in the background with backoff. + this._logService.trace(`[AgentHostSessionsProvider] listSessions failed; scheduling retry: ${err}`); + this._scheduleSessionRefreshRetry(announceExistingAsAdded); + } finally { + this._sessionRefreshInFlight = false; } } + /** + * Arm a backoff retry of {@link _refreshSessions}. Used after a failed + * refresh so a transient startup failure self-heals without requiring an + * unrelated AHP event (a turn completing, a session being added) to force + * a re-fetch. Cancelled on the next successful refresh. + */ + private _scheduleSessionRefreshRetry(announceExistingAsAdded: boolean): void { + const delay = this._sessionRefreshRetryDelay; + this._sessionRefreshRetryDelay = Math.min(delay * 2, BaseAgentHostSessionsProvider.SESSION_REFRESH_RETRY_MAX_MS); + this._sessionRefreshRetry.value = disposableTimeout(() => { + this._refreshSessions(announceExistingAsAdded); + }, delay); + } + + /** + * Cancel any pending session-refresh retry and reset the backoff. Called + * by subclasses when the connection goes away (the stale timer would + * otherwise fire against a dead connection and no-op). + */ + protected _cancelSessionRefreshRetry(): void { + this._sessionRefreshRetry.clear(); + this._sessionRefreshRetryDelay = BaseAgentHostSessionsProvider.SESSION_REFRESH_RETRY_MIN_MS; + } + + private async _waitForNewSession(existingKeys: Set): Promise { await this._refreshSessions(); for (const [key, cached] of this._sessionCache) { diff --git a/src/vs/sessions/contrib/providers/agentHost/browser/localAgentHostSessionsProvider.ts b/src/vs/sessions/contrib/providers/agentHost/browser/localAgentHostSessionsProvider.ts index 413996e8a51c85..f2129e7401d17a 100644 --- a/src/vs/sessions/contrib/providers/agentHost/browser/localAgentHostSessionsProvider.ts +++ b/src/vs/sessions/contrib/providers/agentHost/browser/localAgentHostSessionsProvider.ts @@ -99,7 +99,6 @@ export class LocalAgentHostSessionsProvider extends BaseAgentHostSessionsProvide if (this._agentHostService.authenticationPending.read(reader)) { return; } - this._cacheInitialized = true; this._refreshSessions(); })); } diff --git a/src/vs/sessions/contrib/providers/agentHost/test/browser/localAgentHostSessionsProvider.test.ts b/src/vs/sessions/contrib/providers/agentHost/test/browser/localAgentHostSessionsProvider.test.ts index 4a9e93cc0b4582..cd09757b14bf05 100644 --- a/src/vs/sessions/contrib/providers/agentHost/test/browser/localAgentHostSessionsProvider.test.ts +++ b/src/vs/sessions/contrib/providers/agentHost/test/browser/localAgentHostSessionsProvider.test.ts @@ -86,7 +86,19 @@ class MockAgentHostService extends mock() { return this._nextSeq++; } + /** + * Number of upcoming `listSessions()` calls that should reject, used to + * simulate the agent throwing `AHP_AUTH_REQUIRED` (or a transient offline + * error) before its token is effective server-side. Decremented per call. + */ + public failListSessionsCount = 0; + public listSessionsCallCount = 0; override async listSessions(): Promise { + this.listSessionsCallCount++; + if (this.failListSessionsCount > 0) { + this.failListSessionsCount--; + throw new Error('AHP_AUTH_REQUIRED'); + } return [...this._sessions.values()]; } @@ -606,6 +618,87 @@ suite('LocalAgentHostSessionsProvider', () => { }); })); + test('recovers an empty list when the initial listSessions fails, without needing a new session', () => runWithFakedTimers({ useFakeTimers: true }, async () => { + // Fresh launch: the agent throws on the first listSessions() (e.g. + // AHP_AUTH_REQUIRED before its token is effective, or a transient + // offline error). The sessions really exist on the host. + agentHost.failListSessionsCount = 1; + agentHost.addSession(createSession('heal-1', { summary: 'First' })); + agentHost.addSession(createSession('heal-2', { summary: 'Second' })); + + const provider = createProvider(disposables, agentHost); + const changes: ISessionChangeEvent[] = []; + disposables.add(provider.onDidChangeSessions(e => changes.push(e))); + + // The eager refresh fires and fails; nothing is cached yet. + await timeout(0); + assert.strictEqual(changes.length, 0, 'no event should fire after a failed initial refresh'); + assert.strictEqual(provider.getSessions().length, 0, 'cache stays empty after a failed initial refresh'); + + // The backoff retry (min 1s) fires on its own — no SessionTurnComplete + // or sessionAdded needed — and the list self-heals. + await timeout(1_100); + + assert.deepStrictEqual({ + eventCount: changes.length, + added: changes[0]?.added.map(s => s.title.get()).sort(), + cachedTitles: provider.getSessions().map(s => s.title.get()).sort(), + }, { + eventCount: 1, + added: ['First', 'Second'], + cachedTitles: ['First', 'Second'], + }); + })); + + test('a successful empty listSessions arms no retry', () => runWithFakedTimers({ useFakeTimers: true }, async () => { + // No sessions on the host: listSessions() succeeds with []. This is a + // valid result, not a failure — the cache should be marked initialized + // and no background retry should be scheduled. + const provider = createProvider(disposables, agentHost); + const changes: ISessionChangeEvent[] = []; + disposables.add(provider.onDidChangeSessions(e => changes.push(e))); + + await timeout(0); + const callsAfterEagerLoad = agentHost.listSessionsCallCount; + assert.strictEqual(callsAfterEagerLoad, 1, 'exactly one eager listSessions call'); + + // Advance well past the max backoff window; no retry should fire. + await timeout(60_000); + + assert.strictEqual(agentHost.listSessionsCallCount, callsAfterEagerLoad, 'no retry should be scheduled after a successful empty list'); + assert.strictEqual(changes.length, 0, 'no change event for an empty list'); + assert.strictEqual(provider.getSessions().length, 0); + })); + + test('retries with backoff until listSessions succeeds', () => runWithFakedTimers({ useFakeTimers: true }, async () => { + // First two attempts fail, third succeeds. Verifies the retry keeps + // re-arming rather than giving up after a single failed attempt. + agentHost.failListSessionsCount = 2; + agentHost.addSession(createSession('backoff-1', { summary: 'Only' })); + + const provider = createProvider(disposables, agentHost); + const changes: ISessionChangeEvent[] = []; + disposables.add(provider.onDidChangeSessions(e => changes.push(e))); + + await timeout(0); + assert.strictEqual(provider.getSessions().length, 0, 'empty after first failure'); + + // First retry (~1s) — still failing. + await timeout(1_100); + assert.strictEqual(provider.getSessions().length, 0, 'empty after second failure'); + + // Second retry (~2s backoff) — now succeeds. + await timeout(2_200); + + assert.deepStrictEqual({ + eventCount: changes.length, + cachedTitles: provider.getSessions().map(s => s.title.get()).sort(), + }, { + eventCount: 1, + cachedTitles: ['Only'], + }); + })); + test('uses project metadata as workspace group source', () => runWithFakedTimers({ useFakeTimers: true }, async () => { const projectUri = URI.file('/home/user/vscode'); const workingDirectory = URI.file('/tmp/copilot-worktrees/vscode-feature'); diff --git a/src/vs/sessions/contrib/providers/remoteAgentHost/browser/remoteAgentHostSessionsProvider.ts b/src/vs/sessions/contrib/providers/remoteAgentHost/browser/remoteAgentHostSessionsProvider.ts index d88635632fb9c9..0a36b6ee78d5cd 100644 --- a/src/vs/sessions/contrib/providers/remoteAgentHost/browser/remoteAgentHostSessionsProvider.ts +++ b/src/vs/sessions/contrib/providers/remoteAgentHost/browser/remoteAgentHostSessionsProvider.ts @@ -398,8 +398,9 @@ export class RemoteAgentHostSessionsProvider extends BaseAgentHostSessionsProvid this._attachConnectionListeners(connection, this._connectionListeners); - // Always refresh sessions when a connection is (re)established - this._cacheInitialized = true; + // Always refresh sessions when a connection is (re)established. + // `_refreshSessions` owns `_cacheInitialized` (set on a successful + // list) and arms a backoff retry if the first attempt fails. this._refreshSessions(wasUnpublished); } @@ -438,6 +439,7 @@ export class RemoteAgentHostSessionsProvider extends BaseAgentHostSessionsProvid // triggers a full list refresh (which will reconcile against the // persisted entries we keep on disk). this._cacheInitialized = false; + this._cancelSessionRefreshRetry(); } /**