From 6184ba433691ad01a7614ba35c6401054e56727f Mon Sep 17 00:00:00 2001 From: Maruthan G Date: Fri, 17 Apr 2026 00:46:07 +0530 Subject: [PATCH 1/2] fix(chat): guard agent-sessions reveal against transient tree-map desync (#309891) --- .../agentSessions/agentSessionsControl.ts | 30 +++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/src/vs/workbench/contrib/chat/browser/agentSessions/agentSessionsControl.ts b/src/vs/workbench/contrib/chat/browser/agentSessions/agentSessionsControl.ts index 2642c55a7e3cc..eaacd1e24f72b 100644 --- a/src/vs/workbench/contrib/chat/browser/agentSessions/agentSessionsControl.ts +++ b/src/vs/workbench/contrib/chat/browser/agentSessions/agentSessionsControl.ts @@ -153,7 +153,7 @@ export class AgentSessionsControl extends Disposable implements IAgentSessionsCo const matchingSession = this.agentSessionsService.model.getSession(resource); if (matchingSession && this.sessionsList?.hasNode(matchingSession)) { - if (this.sessionsList.getRelativeTop(matchingSession) === null) { + if (this.tryGetRelativeTop(matchingSession) === null) { this.sessionsList.reveal(matchingSession, 0.5); // only reveal when not already visible } @@ -162,6 +162,32 @@ export class AgentSessionsControl extends Disposable implements IAgentSessionsCo } } + /** + * Safely queries the sessions list for the relative top of an element. + * + * `hasNode()` and `getRelativeTop()` consult different internal maps in the + * compressible async data tree. During an async refresh there is a microtask + * gap where the async tree's node map has been updated but the underlying + * compressed object tree model has not (or vice versa). In that window + * `hasNode()` can return `true` while `getRelativeTop()` throws + * `TreeError [AgentSessionsView] Tree element not found` (issue #309891). + * + * Treat such a failure as "not currently visible" so that callers fall back + * to `reveal()`, which is safe to call when the element is in the data + * source even if the view has not caught up yet. + */ + private tryGetRelativeTop(session: IAgentSession): number | null { + if (!this.sessionsList) { + return null; + } + + try { + return this.sessionsList.getRelativeTop(session); + } catch { + return null; + } + } + private create(container: HTMLElement): void { this.sessionsContainer = append(container, $('.agent-sessions-viewer')); @@ -837,7 +863,7 @@ export class AgentSessionsControl extends Disposable implements IAgentSessionsCo return false; } - if (this.sessionsList.getRelativeTop(session) === null) { + if (this.tryGetRelativeTop(session) === null) { this.sessionsList.reveal(session, 0.5); // only reveal when not already visible } From de1d8b8acc6f002059f69e46794e7aaf18288304 Mon Sep 17 00:00:00 2001 From: Maruthan G Date: Fri, 17 Apr 2026 14:25:17 +0530 Subject: [PATCH 2/2] fix(asyncDataTree): treat TreeError in getRelativeTop as 'not visible' (#309891) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per @bpasero's review on #310831 — moving the guard from the agentSessionsControl call site into the underlying base widget so all AsyncDataTree consumers benefit. During an async refresh there is a microtask gap where AsyncDataTree.nodes (consulted by hasNode) and the underlying tree model's node map can be out of sync, so getDataNode or tree.getRelativeTop can throw TreeError even though hasNode returned true. Treat that transient TreeError as "not currently visible" by returning null — consistent with the existing semantic that null means the element is in the data source but not in view, so callers fall back to reveal() which is safe to call across the gap. Reverts the call-site try/catch in agentSessionsControl now that the base widget handles it. --- src/vs/base/browser/ui/tree/asyncDataTree.ts | 15 +++++++++- .../agentSessions/agentSessionsControl.ts | 30 ++----------------- 2 files changed, 16 insertions(+), 29 deletions(-) diff --git a/src/vs/base/browser/ui/tree/asyncDataTree.ts b/src/vs/base/browser/ui/tree/asyncDataTree.ts index 9d6d200f68f11..22f920e8bbd17 100644 --- a/src/vs/base/browser/ui/tree/asyncDataTree.ts +++ b/src/vs/base/browser/ui/tree/asyncDataTree.ts @@ -1042,7 +1042,20 @@ export class AsyncDataTree implements IDisposable } getRelativeTop(element: T): number | null { - return this.tree.getRelativeTop(this.getDataNode(element)); + // During async refresh there is a microtask gap where this.nodes (used + // by hasNode) and the underlying tree's node map can be out of sync, + // so getDataNode or tree.getRelativeTop may throw TreeError even + // though hasNode returned true. Treat that as "not currently visible" + // — the existing semantic of returning null already covers callers + // that fall back to reveal(). + try { + return this.tree.getRelativeTop(this.getDataNode(element)); + } catch (err) { + if (err instanceof TreeError) { + return null; + } + throw err; + } } // Tree navigation diff --git a/src/vs/workbench/contrib/chat/browser/agentSessions/agentSessionsControl.ts b/src/vs/workbench/contrib/chat/browser/agentSessions/agentSessionsControl.ts index eaacd1e24f72b..2642c55a7e3cc 100644 --- a/src/vs/workbench/contrib/chat/browser/agentSessions/agentSessionsControl.ts +++ b/src/vs/workbench/contrib/chat/browser/agentSessions/agentSessionsControl.ts @@ -153,7 +153,7 @@ export class AgentSessionsControl extends Disposable implements IAgentSessionsCo const matchingSession = this.agentSessionsService.model.getSession(resource); if (matchingSession && this.sessionsList?.hasNode(matchingSession)) { - if (this.tryGetRelativeTop(matchingSession) === null) { + if (this.sessionsList.getRelativeTop(matchingSession) === null) { this.sessionsList.reveal(matchingSession, 0.5); // only reveal when not already visible } @@ -162,32 +162,6 @@ export class AgentSessionsControl extends Disposable implements IAgentSessionsCo } } - /** - * Safely queries the sessions list for the relative top of an element. - * - * `hasNode()` and `getRelativeTop()` consult different internal maps in the - * compressible async data tree. During an async refresh there is a microtask - * gap where the async tree's node map has been updated but the underlying - * compressed object tree model has not (or vice versa). In that window - * `hasNode()` can return `true` while `getRelativeTop()` throws - * `TreeError [AgentSessionsView] Tree element not found` (issue #309891). - * - * Treat such a failure as "not currently visible" so that callers fall back - * to `reveal()`, which is safe to call when the element is in the data - * source even if the view has not caught up yet. - */ - private tryGetRelativeTop(session: IAgentSession): number | null { - if (!this.sessionsList) { - return null; - } - - try { - return this.sessionsList.getRelativeTop(session); - } catch { - return null; - } - } - private create(container: HTMLElement): void { this.sessionsContainer = append(container, $('.agent-sessions-viewer')); @@ -863,7 +837,7 @@ export class AgentSessionsControl extends Disposable implements IAgentSessionsCo return false; } - if (this.tryGetRelativeTop(session) === null) { + if (this.sessionsList.getRelativeTop(session) === null) { this.sessionsList.reveal(session, 0.5); // only reveal when not already visible }