fix: refresh now surfaces externally-generated files in new pakages (#1026)

wenytang-ms · web-flow · commit 7be414f3ad0a · 2026-06-05T09:56:05.000+08:00
* Fix #914: refresh now surfaces externally-generated files in new packages The source package-root load only did a shallow (DEPTH_ONE) refresh, so files written to disk by external generators into brand-new packages never appeared in the Java Projects view, even after Refresh. Manual Refresh now deep-refreshes (DEPTH_INFINITE) and closes the package root so the Java Model re-enumerates its package-fragment list. Auto-refresh passes the changed resource URI to the server via PackageParams.syncPaths, which scopes the refresh to the nearest existing ancestor subtree instead of the whole source root, falling back to a full deep refresh when a path cannot be resolved. - PackageParams: add syncPaths field + accessors - PackageCommand: targeted refresh branch + findNearestExistingResource helper - packageRootNode: stash pending sync paths, snapshot/clear in loadData - syncHandler: onDidCreate stashes the created URI (cycle-safe NodeKind check) - add E2E plans for manual deep refresh and targeted auto-refresh * test: make #914 E2E assertions deterministic to fix CI false negatives CI runs the autotest harness with the LLM screenshot judge enabled, which the local --no-llm runs skipped. Four state-check steps sat on a wait action while asserting a state that was already true (the package/class had appeared in an earlier refresh step). The LLM judge diffed the before/after screenshots, saw no visual delta, and failed the steps -- even though the deterministic verifyTreeItem DOM assertion passed. This produced a spurious 18/20 in CI vs 20/20 locally. Remove the natural-language �erify: field from those pure state-check steps and rely solely on the deterministic �erifyTreeItem assertion, which is the actual behaviour under test. Steps where a visible change genuinely occurs within their own window keep their �erify:. * test: drop LLM verify on file-write/refresh steps and trim comments (#914) The Windows-UI run still failed (19/20): the LLM screenshot judge could not confirm the insertLineInFile step because the written file is not visible in the collapsed Explorer (autoReveal off), a flaky judgment that passed on Linux but failed on Windows. Remove �erify: from all steps with no reliable visual signal (file writes, manual refresh, view focus); keep it only on expandTreeItem steps, which produce a clear visual change. Deterministic verifyTreeItem remains the real assertion. Also trim the verbose header/inline comments. * test: add deterministic negative baseline to #914 plans Each plan only asserted the new package was PRESENT after refresh, never that it was ABSENT before — the missing "before" half of the before/after comparison. Add a deterministic verifyTreeItem (visible: false) baseline proving the package does not exist prior to the file write, so the later appearance is a genuine refresh-driven transition rather than a pre-existing or leftover node. Uses the DOM check (waitForTreeItemGone), so no LLM-judge flakiness. Verified locally: manual 21/21, auto-refresh 16/16. * fix: address PR review - restore syncPaths on failure, dedup refresh targets - packageRootNode: restore the pending syncPaths snapshot if getPackageData rejects so a transient server error does not drop a targeted refresh - PackageCommand: dedup resolved refresh targets via LinkedHashSet so multiple files in the same new package only refresh the subtree once - e2e plans: align Maven section id to lowercase 'maven' for consistency
diff --git a/jdtls.ext/com.microsoft.jdtls.ext.core/src/com/microsoft/jdtls/ext/core/PackageCommand.java b/jdtls.ext/com.microsoft.jdtls.ext.core/src/com/microsoft/jdtls/ext/core/PackageCommand.java
@@ -16,6 +16,7 @@
 import java.util.Collections;
 import java.util.EnumMap;
 import java.util.HashMap;
+import java.util.LinkedHashSet;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
@@ -371,7 +372,7 @@ private static List<PackageNode> getPackageRootChildren(PackageParams query, IPr
                 throw new CoreException(
                         new Status(IStatus.ERROR, JdtlsExtActivator.PLUGIN_ID, String.format("No package root found for %s", query.getPath())));
             }
-            List<Object> result = getPackageFragmentRootContent(packageRoot, query.isHierarchicalView(), pm);
+            List<Object> result = getPackageFragmentRootContent(packageRoot, query.isHierarchicalView(), query.getSyncPaths(), pm);
             ResourceSet resourceSet = new ResourceSet(result, query.isHierarchicalView());
             ResourceVisitor visitor = new JavaResourceVisitor(packageRoot.getJavaProject());
             resourceSet.accept(visitor);
@@ -508,8 +509,62 @@ private static List<PackageNode> getFolderChildren(PackageParams query, IProgres
      * @param pm the progress monitor
      */
     public static List<Object> getPackageFragmentRootContent(IPackageFragmentRoot root, boolean isHierarchicalView, IProgressMonitor pm) throws CoreException {
+        return getPackageFragmentRootContent(root, isHierarchicalView, null, pm);
+    }
+
+    public static List<Object> getPackageFragmentRootContent(IPackageFragmentRoot root, boolean isHierarchicalView, List<String> syncPaths, IProgressMonitor pm) throws CoreException {
         ArrayList<Object> result = new ArrayList<>();
-        refreshLocal(root.getResource(), pm);
+        IResource rootResource = root.getResource();
+        if (rootResource instanceof IContainer && rootResource.exists()
+                && root.getKind() == IPackageFragmentRoot.K_SOURCE) {
+            // Packages created out-of-band (e.g. by code generators or
+            // refactor-moves that write straight to disk) are otherwise never
+            // surfaced. A shallow DEPTH_ONE refresh only syncs the source root's
+            // immediate children and never discovers brand-new nested package
+            // folders. Even a DEPTH_INFINITE resource refresh is not enough on
+            // its own: the Java Model keeps a cached list of package fragments
+            // for the root, so closing it forces getChildren() below to rebuild
+            // that list from the freshly refreshed resource tree.
+            //
+            // On auto-refresh the client passes the changed resource URIs in
+            // syncPaths so we only deep-refresh those subtrees instead of the
+            // whole source tree. If any path cannot be resolved to an existing
+            // resource inside this root we conservatively fall back to a full
+            // DEPTH_INFINITE refresh so no package is ever missed.
+            // See https://github.com/microsoft/vscode-java-dependency/issues/914
+            boolean refreshedTargets = false;
+            if (syncPaths != null && !syncPaths.isEmpty()) {
+                Set<IResource> targets = new LinkedHashSet<>();
+                boolean allResolved = true;
+                for (String syncPath : syncPaths) {
+                    IResource target = findNearestExistingResource(syncPath, (IContainer) rootResource);
+                    if (target == null) {
+                        allResolved = false;
+                        break;
+                    }
+                    // Multiple changed paths can resolve to the same existing
+                    // ancestor (e.g. several new files in one new package); the
+                    // set keeps each distinct subtree so it is refreshed once.
+                    targets.add(target);
+                }
+                if (allResolved && !targets.isEmpty()) {
+                    for (IResource target : targets) {
+                        refreshLocal(target, IResource.DEPTH_INFINITE, pm);
+                    }
+                    refreshedTargets = true;
+                }
+            }
+            if (!refreshedTargets) {
+                refreshLocal(rootResource, IResource.DEPTH_INFINITE, pm);
+            }
+            try {
+                root.close();
+            } catch (JavaModelException e) {
+                JdtlsExtActivator.log(e);
+            }
+        } else {
+            refreshLocal(rootResource, IResource.DEPTH_ONE, pm);
+        }
         if (isHierarchicalView) {
             Map<String, IJavaElement> map = new HashMap<>();
             for (IJavaElement child : root.getChildren()) {
@@ -599,13 +654,56 @@ public static IJavaProject getJavaProject(String projectUri) {
     }
 
     private static void refreshLocal(IResource resource, IProgressMonitor monitor) {
+        refreshLocal(resource, IResource.DEPTH_ONE, monitor);
+    }
+
+    private static void refreshLocal(IResource resource, int depth, IProgressMonitor monitor) {
         if (resource == null || !resource.exists()) {
             return;
         }
         try {
-            resource.refreshLocal(IResource.DEPTH_ONE, monitor);
+            resource.refreshLocal(depth, monitor);
         } catch (CoreException e) {
             JdtlsExtActivator.log(e);
         }
     }
+
+    /**
+     * Resolve a changed resource URI to the nearest ancestor that already exists
+     * in the workspace resource tree and lives inside the given source root.
+     * Used to scope an auto-refresh to only the affected subtree. Returns null
+     * when the URI cannot be mapped to a resource within the root, in which case
+     * the caller falls back to a full refresh so no package is missed.
+     */
+    private static IResource findNearestExistingResource(String uriStr, IContainer root) {
+        if (StringUtils.isBlank(uriStr) || root == null) {
+            return null;
+        }
+        try {
+            URI uri = JDTUtils.toURI(uriStr);
+            if (uri == null) {
+                return null;
+            }
+            IWorkspaceRoot wsRoot = ResourcesPlugin.getWorkspace().getRoot();
+            IResource resource = null;
+            IFile[] files = wsRoot.findFilesForLocationURI(uri);
+            if (files.length > 0) {
+                resource = files[0];
+            } else {
+                IContainer[] containers = wsRoot.findContainersForLocationURI(uri);
+                if (containers.length > 0) {
+                    resource = containers[0];
+                }
+            }
+            while (resource != null && !resource.exists()) {
+                resource = resource.getParent();
+            }
+            if (resource != null && root.getFullPath().isPrefixOf(resource.getFullPath())) {
+                return resource;
+            }
+        } catch (Exception e) {
+            JdtlsExtActivator.logException("Failed to resolve sync path " + uriStr, e);
+        }
+        return null;
+    }
 }
diff --git a/jdtls.ext/com.microsoft.jdtls.ext.core/src/com/microsoft/jdtls/ext/core/PackageParams.java b/jdtls.ext/com.microsoft.jdtls.ext.core/src/com/microsoft/jdtls/ext/core/PackageParams.java
@@ -11,6 +11,8 @@
 
 package com.microsoft.jdtls.ext.core;
 
+import java.util.List;
+
 import com.microsoft.jdtls.ext.core.model.NodeKind;
 
 /**
@@ -31,6 +33,14 @@ public class PackageParams {
 
     private boolean isHierarchicalView;
 
+    /**
+     * Optional list of resource URIs (sent by the client on auto-refresh) that
+     * have just changed on disk. When present, the server only refreshes the
+     * affected subtrees instead of deeply refreshing the whole source root.
+     * See https://github.com/microsoft/vscode-java-dependency/issues/914
+     */
+    private List<String> syncPaths;
+
     public PackageParams() {
     }
 
@@ -97,4 +107,12 @@ public void setRootPath(String rootPath) {
         this.rootPath = rootPath;
     }
 
+    public List<String> getSyncPaths() {
+        return syncPaths;
+    }
+
+    public void setSyncPaths(List<String> syncPaths) {
+        this.syncPaths = syncPaths;
+    }
+
 }
diff --git a/src/syncHandler.ts b/src/syncHandler.ts
@@ -90,7 +90,18 @@ class SyncHandler implements Disposable {
         }));
 
         this.disposables.push(watcher.onDidCreate((uri: Uri) => {
-            this.refresh(this.getParentNodeInExplorer(uri));
+            const node: ExplorerNode | undefined = this.getParentNodeInExplorer(uri);
+            // When the created resource lands in a package that is not currently
+            // rendered, getParentNodeInExplorer resolves to the source root. Tell
+            // that root which path changed so the server can refresh only that
+            // subtree instead of deeply refreshing the whole source tree. Gate on
+            // the node kind (not instanceof) to avoid importing PackageRootNode
+            // here, which would create a module cycle and break activation.
+            // See https://github.com/microsoft/vscode-java-dependency/issues/914
+            if (node instanceof DataNode && node.nodeData?.kind === NodeKind.PackageRoot) {
+                (node as unknown as { pendingSyncPaths: Set<string> }).pendingSyncPaths.add(uri.toString());
+            }
+            this.refresh(node);
         }));
 
         this.disposables.push(watcher.onDidDelete((uri: Uri) => {
diff --git a/src/views/packageRootNode.ts b/src/views/packageRootNode.ts
@@ -16,6 +16,14 @@ import { NodeFactory } from "./nodeFactory";
 
 export class PackageRootNode extends DataNode {
 
+    /**
+     * Resource URIs reported by the file watcher as newly created under this
+     * source root since the last load. Consumed on the next loadData() so the
+     * server can scope its filesystem refresh to only the changed subtrees.
+     * See https://github.com/microsoft/vscode-java-dependency/issues/914
+     */
+    public pendingSyncPaths: Set<string> = new Set<string>();
+
     constructor(nodeData: INodeData, parent: DataNode, protected _project: ProjectNode) {
         super(nodeData, parent);
     }
@@ -25,13 +33,28 @@ export class PackageRootNode extends DataNode {
     }
 
     protected async loadData(): Promise<INodeData[]> {
-        return Jdtls.getPackageData({
-            kind: NodeKind.PackageRoot,
-            projectUri: this._project.nodeData.uri,
-            rootPath: this.nodeData.path,
-            handlerIdentifier: this.nodeData.handlerIdentifier,
-            isHierarchicalView: Settings.isHierarchicalView(),
-        });
+        let syncPaths: string[] | undefined;
+        if (this.pendingSyncPaths.size) {
+            // Snapshot and clear synchronously before the async server call so
+            // watcher events arriving during the await are not lost.
+            syncPaths = Array.from(this.pendingSyncPaths);
+            this.pendingSyncPaths.clear();
+        }
+        try {
+            return await Jdtls.getPackageData({
+                kind: NodeKind.PackageRoot,
+                projectUri: this._project.nodeData.uri,
+                rootPath: this.nodeData.path,
+                handlerIdentifier: this.nodeData.handlerIdentifier,
+                isHierarchicalView: Settings.isHierarchicalView(),
+                syncPaths,
+            });
+        } catch (error) {
+            // Restore the snapshot so a transient server error does not drop the
+            // pending paths; the next refresh will retry the targeted sync.
+            syncPaths?.forEach((path) => this.pendingSyncPaths.add(path));
+            throw error;
+        }
     }
 
     protected createChildNodeList(): ExplorerNode[] {
diff --git a/test/e2e-plans/java-dep-autorefresh-targeted.yaml b/test/e2e-plans/java-dep-autorefresh-targeted.yaml
@@ -0,0 +1,105 @@
+# Validates the targeted (scoped) auto-refresh optimization for issue #914.
+# Companion to java-dep-refresh-generated-files.yaml (which tests manual Refresh).
+#
+# On auto-refresh the extension passes the changed URI to the server
+# (PackageParams.syncPaths); the server refreshes only the affected subtree (the
+# nearest existing ancestor) instead of the whole source root, then closes the
+# package root to rebuild its package-fragment list. This plan does NOT call
+# java.view.package.refresh — it relies solely on the FileSystemWatcher, so it
+# exercises the syncPaths path. Auto-refresh is on by default.
+#
+# Tree layout and verify policy are the same as the manual-Refresh plan: compact
+# virtualized tree, deterministic verifyTreeItem assertions, no `verify:` on the
+# file-write step (no reliable visual signal for the LLM judge).
+#
+# Usage:
+#   npx autotest run test/e2e-plans/java-dep-autorefresh-targeted.yaml \
+#       --override extensionPath=<path-to-this-repo>
+
+name: "Java Dependency — Auto-refresh surfaces a new package (targeted, #914)"
+description: |
+  Validates the targeted (scoped) auto-refresh optimization for issue #914: a
+  .java file written by an external generator into a brand-new sub-package must
+  appear in the Java Projects view via the file-watcher auto-refresh alone (no
+  manual Refresh), which routes the changed URI through PackageParams.syncPaths.
+
+setup:
+  extension: "redhat.java"
+  vscodeVersion: "stable"
+  workspace: "../maven"
+  timeout: 180
+  settings:
+    java.configuration.checkProjectSettingsExclusions: false
+    workbench.startupEditor: "none"
+    explorer.autoReveal: false
+
+steps:
+  - id: "ls-ready"
+    action: "waitForLanguageServer"
+    timeout: 180
+
+  # Free vertical space so the Java Projects tree is not virtualized.
+  - id: "close-aux-bar"
+    action: "executeVSCodeCommand workbench.action.closeAuxiliaryBar"
+
+  - id: "collapse-outline"
+    action: "collapseSidebarSection OUTLINE"
+
+  - id: "collapse-timeline"
+    action: "collapseSidebarSection TIMELINE"
+
+  - id: "collapse-explorer-folders"
+    action: "collapseSidebarSection maven"
+
+  - id: "focus-java-projects"
+    action: "executeVSCodeCommand javaProjectExplorer.focus"
+
+  - id: "wait-tree-load"
+    action: "wait 3 seconds"
+
+  # The source root must be expanded so a PackageRootNode exists to target.
+  - id: "expand-project"
+    action: "expandTreeItem my-app"
+    verify: "my-app project expanded"
+
+  - id: "expand-source-root"
+    action: "expandTreeItem src/main/java"
+    verify: "source root src/main/java expanded"
+
+  - id: "baseline-existing-pkg"
+    action: "wait 1 seconds"
+    verifyTreeItem:
+      name: "com.mycompany.app"
+      visible: true
+
+  # Negative baseline: the brand-new package must be ABSENT before the file is
+  # written, so check-new-pkg-autorefresh later observes a genuine appearance
+  # driven by the watcher, not a pre-existing node.
+  - id: "baseline-new-pkg-absent"
+    action: "wait 1 seconds"
+    verifyTreeItem:
+      name: "com.mycompany.app.autogen"
+      visible: false
+    timeout: 5
+
+  # Write a file straight to disk into a brand-new sub-package.
+  - id: "gen-file-new-pkg"
+    action: "insertLineInFile src/main/java/com/mycompany/app/autogen/Gen914AutoNewPkg.java 1 package com.mycompany.app.autogen;\n\npublic class Gen914AutoNewPkg {\n}\n"
+
+  - id: "dismiss-overlay"
+    action: "pressKey Escape"
+
+  # Let the file watcher + debounced auto-refresh fire. No manual Refresh.
+  - id: "wait-for-auto-refresh"
+    action: "wait 6 seconds"
+
+  - id: "reexpand-source-root"
+    action: "expandTreeItem src/main/java"
+
+  # The brand-new package appears via auto-refresh alone.
+  - id: "check-new-pkg-autorefresh"
+    action: "wait 1 seconds"
+    verifyTreeItem:
+      name: "com.mycompany.app.autogen"
+      visible: true
+    timeout: 20
diff --git a/test/e2e-plans/java-dep-refresh-generated-files.yaml b/test/e2e-plans/java-dep-refresh-generated-files.yaml
