From 2660f43f39c224e4fa559e4386be05b2af00c8db Mon Sep 17 00:00:00 2001 From: MaxDNG Date: Mon, 20 Apr 2026 15:50:33 +0200 Subject: [PATCH 1/2] Fix: Re-parent pulled-up directory children to ensure proper checkbox refresh When the tree has multiple top-level directories, a temporary DirectoryTreeNode is created to build the tree, then its children are pulled up to the parent. However, the children's parent pointers still referenced the phantom root, causing processCheckboxUpdates to refresh an invisible node, so checkbox state never updated visually. Re-parent all pulled-up children to the actual container node (FilesCategoryNode, PullRequestNode, or CommitNode) so ancestor walks target visible tree nodes. --- src/view/treeNodes/commitNode.ts | 1 + src/view/treeNodes/filesCategoryNode.ts | 1 + src/view/treeNodes/pullRequestNode.ts | 1 + 3 files changed, 3 insertions(+) diff --git a/src/view/treeNodes/commitNode.ts b/src/view/treeNodes/commitNode.ts index d67aa49438..ee95b34a12 100644 --- a/src/view/treeNodes/commitNode.ts +++ b/src/view/treeNodes/commitNode.ts @@ -111,6 +111,7 @@ export class CommitNode extends TreeNode implements vscode.TreeItem { dirNode.finalize(); if (dirNode.label === '') { // nothing on the root changed, pull children to parent + dirNode._children.forEach(child => { child.parent = this; }); result.push(...dirNode._children); } else { result.push(dirNode); diff --git a/src/view/treeNodes/filesCategoryNode.ts b/src/view/treeNodes/filesCategoryNode.ts index 189b060c22..077974c8c5 100644 --- a/src/view/treeNodes/filesCategoryNode.ts +++ b/src/view/treeNodes/filesCategoryNode.ts @@ -93,6 +93,7 @@ export class FilesCategoryNode extends TreeNode implements vscode.TreeItem { if (dirNode.label === '') { // nothing on the root changed, pull children to parent this.directories = dirNode._children; + this.directories.forEach(child => { child.parent = this; }); } else { this.directories = [dirNode]; } diff --git a/src/view/treeNodes/pullRequestNode.ts b/src/view/treeNodes/pullRequestNode.ts index 7bedc0cc03..f9ddf00328 100644 --- a/src/view/treeNodes/pullRequestNode.ts +++ b/src/view/treeNodes/pullRequestNode.ts @@ -105,6 +105,7 @@ export class PRNode extends TreeNode implements vscode.CommentingRangeProvider2 dirNode.finalize(); if (dirNode.label === '') { // nothing on the root changed, pull children to parent + dirNode._children.forEach(child => { child.parent = this; }); result.push(...dirNode._children); } else { result.push(dirNode); From 6d0b6f93635dfe4944220727cc1c94d39c56998e Mon Sep 17 00:00:00 2001 From: MaxDNG Date: Mon, 20 Apr 2026 15:58:19 +0200 Subject: [PATCH 2/2] test: add regression tests for parent pointer invariant after tree building Add tests to verify that pulled-up directory children are correctly re-parented to the container node, not the phantom root. This ensures processCheckboxUpdates can walk ancestors and find visible tree nodes for refresh() calls. Tests cover: - getParent() correctly returns undefined after re-parenting - ancestor walk stops at topmost visible directory, not phantom root --- .../view/treeNodes/directoryTreeNode.test.ts | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/src/test/view/treeNodes/directoryTreeNode.test.ts b/src/test/view/treeNodes/directoryTreeNode.test.ts index 3945b469b5..47fa9e5f50 100644 --- a/src/test/view/treeNodes/directoryTreeNode.test.ts +++ b/src/test/view/treeNodes/directoryTreeNode.test.ts @@ -206,4 +206,57 @@ describe('DirectoryTreeNode', function () { assert.strictEqual(rootDir.checkboxState?.state, vscode.TreeItemCheckboxState.Unchecked); }); }); + + describe('parent pointer after phantom-root pull-up', function () { + // When a tree has multiple top-level directories, a temporary DirectoryTreeNode + // with label='' (phantom root) is used to build the tree, then its children are + // pulled up to the actual container. If children are not re-parented, getParent() + // still returns the phantom DirectoryTreeNode and processCheckboxUpdates fires + // refresh() on an invisible node, so checkbox state never updates visually. + + it('getParent() returns undefined after child is re-parented to the container', function () { + const container = createMockParent(); + const phantomRoot = new DirectoryTreeNode(container, ''); + const subDir = new DirectoryTreeNode(phantomRoot, 'src'); + + // Before re-parenting: parent is the phantom root DirectoryTreeNode + assert.ok(subDir.getParent() instanceof DirectoryTreeNode, 'sanity: should point to phantom root before re-parenting'); + + // Simulate the pull-up re-parenting fix applied in filesCategoryNode / pullRequestNode / commitNode + subDir.parent = container; + + // After re-parenting: container is not a TreeNode, so getParent() returns undefined. + // This means the ancestor walk in processCheckboxUpdates stops here and refresh() + // is called on the visible subDir node, not the invisible phantom root. + assert.strictEqual(subDir.getParent(), undefined, 'after re-parenting, getParent() should not return the phantom root'); + }); + + it('ancestor walk stops at the topmost visible directory after re-parenting', function () { + const container = createMockParent(); + const phantomRoot = new DirectoryTreeNode(container, ''); + const topDir = new DirectoryTreeNode(phantomRoot, 'cloud'); + const subDir = new DirectoryTreeNode(topDir, 'helm'); + const file = new MockFileNode(subDir); + file.checkboxState = { state: vscode.TreeItemCheckboxState.Checked }; + + (subDir._children as any[]).push(file); + topDir._children.push(subDir); + + // Re-parent: simulate fix + topDir.parent = container; + + // Walk ancestors from file, mimicking processCheckboxUpdates + const ancestors: TreeNode[] = []; + let current = file.getParent(); + while (current instanceof DirectoryTreeNode) { + ancestors.push(current); + current = current.getParent(); + } + + // Should find subDir and topDir, but NOT the phantom root + assert.strictEqual(ancestors.length, 2); + assert.strictEqual(ancestors[0], subDir); + assert.strictEqual(ancestors[1], topDir); + }); + }); });