Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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();
Comment thread
roblourens marked this conversation as resolved.
}

Expand All @@ -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<string>();
const added: ISession[] = [];
const changed: ISession[] = [];
Expand Down Expand Up @@ -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<string>): Promise<ISession | undefined> {
await this._refreshSessions();
for (const [key, cached] of this._sessionCache) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,6 @@ export class LocalAgentHostSessionsProvider extends BaseAgentHostSessionsProvide
if (this._agentHostService.authenticationPending.read(reader)) {
return;
}
this._cacheInitialized = true;
this._refreshSessions();
}));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,19 @@ class MockAgentHostService extends mock<IAgentHostService>() {
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<IAgentSessionMetadata[]> {
this.listSessionsCallCount++;
if (this.failListSessionsCount > 0) {
this.failListSessionsCount--;
throw new Error('AHP_AUTH_REQUIRED');
}
return [...this._sessions.values()];
}

Expand Down Expand Up @@ -606,6 +618,87 @@ suite('LocalAgentHostSessionsProvider', () => {
});
}));

test('recovers an empty list when the initial listSessions fails, without needing a new session', () => runWithFakedTimers<void>({ 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');
Comment thread
roblourens marked this conversation as resolved.

// 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<void>({ 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<void>({ 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');
Comment thread
roblourens marked this conversation as resolved.

// 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<void>({ useFakeTimers: true }, async () => {
const projectUri = URI.file('/home/user/vscode');
const workingDirectory = URI.file('/tmp/copilot-worktrees/vscode-feature');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down Expand Up @@ -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();
}

/**
Expand Down
Loading