From 2b42bca733676a1807ee28295654b0018119a491 Mon Sep 17 00:00:00 2001 From: JJ Lee Date: Tue, 8 Sep 2026 16:32:57 -0400 Subject: [PATCH] fix(sidebar): empty environment children from cache poisoning + watcher gap (#903) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two bugs combined to make resolved (non-workspace) environment nodes expand with nothing beneath them: 1. Cache poisoning: readDirectoryEntries swallows errors and returns []. The webview cached [] (truthy), so all !childrenCache[path] guards evaluated false, permanently suppressing re-requests. Fix: never cache empty arrays — delete the key instead, leaving undefined (falsy) so future renders re-request children. 2. Watcher gap: setupWatcher uses createFileSystemWatcher('**/*') which is workspace-scoped, and the handler checks getWorkspaceFolder(uri) which drops non-workspace URIs. Resolved environments (source: 'resolved', auto-surfaced from the registry) are not workspace folders, so filesystem changes inside them never invalidated the poisoned cache. Fix: create per-path FileSystemWatcher using RelativePattern for resolved environment paths. Watchers are refreshed on roots changes, deduped, debounced (300ms), and disposed cleanly. Also adds console.warn to readDirectoryEntries catch block for future diagnosis visibility. --- packages/extension/src/sidebar_view.ts | 80 +++++++++++++++++++++-- packages/extension/src/sidebar_webview.ts | 14 +++- 2 files changed, 88 insertions(+), 6 deletions(-) diff --git a/packages/extension/src/sidebar_view.ts b/packages/extension/src/sidebar_view.ts index 1b5c6db7..e3a58707 100644 --- a/packages/extension/src/sidebar_view.ts +++ b/packages/extension/src/sidebar_view.ts @@ -11,7 +11,7 @@ import * as vscode from "vscode"; import * as path from "node:path"; import * as fs from "node:fs"; import * as os from "node:os"; -import { handleSidebarMessage, type SidebarMessageHandlers, type SidebarDownMessage, type FileOpRequest, type FileOpResult, type TreeEntry } from "./sidebar_bridge"; +import { handleSidebarMessage, type SidebarMessageHandlers, type SidebarDownMessage, type FileOpRequest, type FileOpResult, type TreeEntry, type TreeRoot } from "./sidebar_bridge"; import { SidebarTreeService, type RawDirEntry } from "./sidebar_tree_service"; import { ChatPanel } from "./chat_panel"; import { detectProjectType } from "./project/detect"; @@ -293,6 +293,9 @@ export class SidebarViewProvider implements vscode.WebviewViewProvider { private fsPendingProjectTypeChange = false; private workspaceSub?: vscode.Disposable; private gitSubs: vscode.Disposable[] = []; + /** Watchers for resolved (non-workspace) environment directories (#903). */ + private resolvedEnvWatchers: vscode.FileSystemWatcher[] = []; + private resolvedEnvDebounceTimer?: ReturnType; private treeService: SidebarTreeService; private globalState?: { get(key: string, fallback?: unknown): unknown; update(key: string, value: unknown): Thenable }; @@ -363,6 +366,8 @@ export class SidebarViewProvider implements vscode.WebviewViewProvider { // Schedule a git-status push so colors survive the DOM wipe // that renderRoots() causes in the webview. queueMicrotask(() => this.pushGitStatus()); + // Refresh watchers for resolved (non-workspace) environments (#903) + this.refreshResolvedEnvWatchers(roots); return roots; }, getChildren: async (p) => { @@ -405,15 +410,20 @@ export class SidebarViewProvider implements vscode.WebviewViewProvider { // Refresh when workspace folders change. this.workspaceSub = vscode.workspace.onDidChangeWorkspaceFolders(() => { invalidateEnvironmentCache(); - this.postDown({ kind: "roots", roots: this.treeService.getRoots() }); + const roots = this.treeService.getRoots(); + this.postDown({ kind: "roots", roots }); + this.refreshResolvedEnvWatchers(roots); this.pushGitStatus(); }); webviewView.onDidDispose(() => { clearTimeout(this.fsDebounceTimer); + clearTimeout(this.resolvedEnvDebounceTimer); this.fsPendingFolders.clear(); this.fsPendingProjectTypeChange = false; this.watcher?.dispose(); + for (const w of this.resolvedEnvWatchers) w.dispose(); + this.resolvedEnvWatchers = []; this.workspaceSub?.dispose(); for (const sub of this.gitSubs) sub.dispose(); this.gitSubs = []; @@ -511,7 +521,9 @@ export class SidebarViewProvider implements vscode.WebviewViewProvider { // webview's fs-changed handler already performs. if (this.fsPendingProjectTypeChange) { invalidateEnvironmentCache(); - this.postDown({ kind: "roots", roots: this.treeService.getRoots() }); + const roots = this.treeService.getRoots(); + this.postDown({ kind: "roots", roots }); + this.refreshResolvedEnvWatchers(roots); queueMicrotask(() => this.pushGitStatus()); this.fsPendingProjectTypeChange = false; } @@ -588,6 +600,63 @@ export class SidebarViewProvider implements vscode.WebviewViewProvider { } } + /** + * Create file watchers for resolved (non-workspace) environment directories + * so that filesystem changes inside them produce fs-changed messages and + * invalidate the webview's children cache (#903). + * + * The main setupWatcher only covers workspace folders (createFileSystemWatcher + * with a bare glob is workspace-scoped, and the onFsEvent handler explicitly + * checks getWorkspaceFolder). Resolved environments are auto-surfaced from + * the registry and are NOT workspace folders, so changes inside them are + * invisible without these per-path watchers. + * + * Uses RelativePattern(Uri.file(envPath), "**\/*") which works for non- + * workspace paths on macOS (FSEvents) and Windows. On Linux the watcher is + * best-effort per the VS Code API contract. + */ + private refreshResolvedEnvWatchers(roots: TreeRoot[]): void { + const newPaths = new Set( + roots.filter((r) => r.source === "resolved").map((r) => r.path), + ); + + // Fast path: no change in the set of resolved paths → nothing to do. + const currentPaths = new Set(this.resolvedEnvWatchers.map((w) => (w as any).__envPath as string)); + if (newPaths.size === currentPaths.size && [...newPaths].every((p) => currentPaths.has(p))) { + return; + } + + // Dispose old watchers + for (const w of this.resolvedEnvWatchers) w.dispose(); + this.resolvedEnvWatchers = []; + + for (const envPath of newPaths) { + try { + const pattern = new vscode.RelativePattern(vscode.Uri.file(envPath), "**/*"); + const watcher = vscode.workspace.createFileSystemWatcher(pattern); + // Tag the watcher so the fast-path check above can compare sets + (watcher as any).__envPath = envPath; + + const handler = () => { + // Debounce to avoid flooding, same as the main watcher + clearTimeout(this.resolvedEnvDebounceTimer); + this.resolvedEnvDebounceTimer = setTimeout(() => { + this.postDown({ kind: "fs-changed", folder: envPath }); + }, 300); + }; + + watcher.onDidCreate(handler); + watcher.onDidChange(handler); + watcher.onDidDelete(handler); + this.resolvedEnvWatchers.push(watcher); + } catch { + // Watcher creation failed (e.g., OS-level watch limits) — graceful + // degradation: Fix 1 (no-cache-empty) still allows re-requests on + // expand toggles and webview recreation. + } + } + } + private buildHtml( webview: vscode.Webview, nonce: string, @@ -1056,7 +1125,10 @@ async function readDirectoryEntries(dir: string): Promise { name, type: type === vscode.FileType.Directory ? "directory" as const : "file" as const, })); - } catch { + } catch (err) { + // Log so transient failures are visible — a swallowed empty [] here is + // the entry point for the cache-poison bug (#903). + console.warn(`[sidebar] readDirectoryEntries failed for ${dir}:`, err); return []; } } diff --git a/packages/extension/src/sidebar_webview.ts b/packages/extension/src/sidebar_webview.ts index 3df76f2e..425c5d38 100644 --- a/packages/extension/src/sidebar_webview.ts +++ b/packages/extension/src/sidebar_webview.ts @@ -1895,7 +1895,17 @@ function createIconEl(icon: string): HTMLElement { } case "children": { - childrenCache[msg.path] = msg.entries ?? []; + // Only cache non-empty results. An empty [] from a transient + // readDirectory failure is truthy and would permanently suppress + // re-requests (every guard uses !childrenCache[path], and ![] is + // false). Leaving the key absent (undefined) lets future renders + // and expand toggles re-request children. (#903) + const childEntries = msg.entries ?? []; + if (childEntries.length > 0) { + childrenCache[msg.path] = childEntries; + } else { + delete childrenCache[msg.path]; + } // Find the container for this path and render children const container = treeRoot?.querySelector(`[data-path="${CSS.escape(msg.path)}"] > .children`); if (container) { @@ -1904,7 +1914,7 @@ function createIconEl(icon: string): HTMLElement { // focused input, firing blur synchronously — suppress cancel. const hasInlineEdit = activeInlineEdit?.tempRow && activeInlineEdit.path === msg.path; if (hasInlineEdit) inlineEditRerendering = true; - renderChildren(container as HTMLElement, msg.entries ?? [], depth); + renderChildren(container as HTMLElement, childEntries, depth); inlineEditRerendering = false; // Re-insert the inline edit temp row after children are rendered if (hasInlineEdit && activeInlineEdit?.tempRow) {