From ae690f696cdabf2ede46fa24ed593883828e73be Mon Sep 17 00:00:00 2001 From: Rob Lourens Date: Sun, 31 May 2026 13:10:14 -0700 Subject: [PATCH 1/3] sessions: recover empty Agent Host session list after a failed initial refresh The Agents-app local session list could stay empty on launch and only populate after the user started a new session. The provider's session cache is one-shot: `_refreshSessions()` ran once, but its blanket catch could not tell "listSessions() threw" (e.g. the agent throws AHP_AUTH_REQUIRED before its token is effective server-side, or a transient offline error) from "listSessions() returned []" (a genuinely empty, valid result). Both collapsed to an empty cache marked initialized, and nothing retried until an unrelated AHP event (SessionTurnComplete / sessionAdded) forced a re- which is whyfetch starting a new session "unstuck" the list. `_refreshSessions` now owns `_cacheInitialized` and sets it only on a successful list. On a thrown failure it logs and arms a capped exponential-backoff retry (1s -> 30s, reset on success), so a transient startup failure self-heals on its own. An empty successful list is still treated as a valid result that marks the cache initialized and arms no retry. Listing sessions still never pops a sign-in dialog; AHP_AUTH_REQUIRED is retried silently in the background. The local provider's auth autorun and the remote provider's setConnection no longer pre-set `_cacheInitialized`; clearConnection cancels any pending retry. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../browser/baseAgentHostSessionsProvider.ts | 64 ++++++++++++- .../browser/localAgentHostSessionsProvider.ts | 1 - .../localAgentHostSessionsProvider.test.ts | 93 +++++++++++++++++++ .../remoteAgentHostSessionsProvider.ts | 6 +- 4 files changed, 158 insertions(+), 6 deletions(-) diff --git a/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts b/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts index 3386bf7085f6d..6134ca6e40445 100644 --- a/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts +++ b/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts @@ -1106,6 +1106,22 @@ export abstract class BaseAgentHostSessionsProvider extends Disposable implement protected _cacheInitialized = false; + /** + * 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; + + private static readonly SESSION_REFRESH_RETRY_MIN_MS = 1_000; + private static readonly SESSION_REFRESH_RETRY_MAX_MS = 30_000; + constructor( @IChatSessionsService protected readonly _chatSessionsService: IChatSessionsService, @IChatService protected readonly _chatService: IChatService, @@ -2112,7 +2128,10 @@ 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. this._refreshSessions(); } @@ -2121,8 +2140,14 @@ export abstract class BaseAgentHostSessionsProvider extends Disposable implement if (!connection) { return; } + // Cancel any pending retry; this attempt supersedes it. + this._sessionRefreshRetry.clear(); 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 +2183,44 @@ 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); } } + /** + * 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 413996e8a51c8..f2129e7401d17 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 4a9e93cc0b458..cd09757b14bf0 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 d88635632fb9c..0a36b6ee78d5c 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(); } /** From 12baf68d5e10074983cfd51fdc631dd08120aca4 Mon Sep 17 00:00:00 2001 From: Rob Lourens Date: Sun, 31 May 2026 13:53:02 -0700 Subject: [PATCH 2/3] fix compile error: declare retry backoff statics before instance field The static SESSION_REFRESH_RETRY_MIN_MS was referenced by the _sessionRefreshRetryDelay instance field initializer but declared after it, causing a 'used before its initialization' compile error under the full tsc build. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../agentHost/browser/baseAgentHostSessionsProvider.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts b/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts index 6134ca6e40445..889b2b8047551 100644 --- a/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts +++ b/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts @@ -1106,6 +1106,9 @@ 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 @@ -1119,9 +1122,6 @@ export abstract class BaseAgentHostSessionsProvider extends Disposable implement /** Current backoff delay (ms) for the session-refresh retry. */ private _sessionRefreshRetryDelay = BaseAgentHostSessionsProvider.SESSION_REFRESH_RETRY_MIN_MS; - private static readonly SESSION_REFRESH_RETRY_MIN_MS = 1_000; - private static readonly SESSION_REFRESH_RETRY_MAX_MS = 30_000; - constructor( @IChatSessionsService protected readonly _chatSessionsService: IChatSessionsService, @IChatService protected readonly _chatService: IChatService, From c78d12135d67fa679616383b49daf5585950230c Mon Sep 17 00:00:00 2001 From: Rob Lourens Date: Sun, 31 May 2026 14:00:42 -0700 Subject: [PATCH 3/3] address review: guard _ensureSessionCache against redundant refreshes With _cacheInitialized now staying false until a successful listSessions, every synchronous getSessions()/getSessionByResource() during the failure window would call _ensureSessionCache() and launch a fresh listSessions(), bypassing the backoff and repeatedly hitting the agent/auth path. Add an in-flight flag and skip _ensureSessionCache when a refresh is already running or a backoff retry is already scheduled, so recovery only happens via the backoff timer. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../browser/baseAgentHostSessionsProvider.ts | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts b/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts index 889b2b8047551..2c980ea5769de 100644 --- a/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts +++ b/src/vs/sessions/contrib/providers/agentHost/browser/baseAgentHostSessionsProvider.ts @@ -1122,6 +1122,9 @@ export abstract class BaseAgentHostSessionsProvider extends Disposable implement /** 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, @@ -2131,7 +2134,14 @@ export abstract class BaseAgentHostSessionsProvider extends Disposable implement // `_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. + // 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(); } @@ -2142,6 +2152,7 @@ export abstract class BaseAgentHostSessionsProvider extends Disposable implement } // 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 @@ -2193,6 +2204,8 @@ export abstract class BaseAgentHostSessionsProvider extends Disposable implement // 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; } }