Skip to content

Commit 51dc310

Browse files
Copilotalexr00
Copilot
and
alexr00
committed
Add proactive stale worktree cleanup on refresh
When the Refresh button is clicked in the PR sidebar, check if any tracked folder managers reference root URIs that no longer exist on disk (e.g., removed worktree directories). Remove those stale entries before refreshing the tree. Also: - PrsTreeModel now fires data change on repo removal so tree refreshes - ReviewsManager cleans up orphaned review managers when folder repos change Co-authored-by: alexr00 <38270282+alexr00@users.noreply.github.com>
1 parent 56eaa56 commit 51dc310

File tree

5 files changed

+107
-12
lines changed

5 files changed

+107
-12
lines changed

‎src/github/repositoriesManager.ts‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,24 @@ export class RepositoriesManager extends Disposable {
134134
}
135135
}
136136

137+
async removeMissingRepos(): Promise<void> {
138+
const managersToRemove: FolderRepositoryManager[] = [];
139+
for (const manager of this._folderManagers) {
140+
const uri = manager.repository.rootUri;
141+
if (uri.scheme === 'file') {
142+
try {
143+
await vscode.workspace.fs.stat(uri);
144+
} catch {
145+
managersToRemove.push(manager);
146+
}
147+
}
148+
}
149+
for (const manager of managersToRemove) {
150+
Logger.appendLine(`Removing stale repository ${manager.repository.rootUri} (path no longer exists).`, RepositoriesManager.ID);
151+
this.removeRepo(manager.repository);
152+
}
153+
}
154+
137155
getManagerForIssueModel(issueModel: IssueModel | undefined): FolderRepositoryManager | undefined {
138156
if (issueModel === undefined) {
139157
return undefined;

‎src/test/github/repositoriesManager.test.ts‎

Lines changed: 68 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
* Licensed under the MIT License. See License.txt in the project root for license information.
44
*--------------------------------------------------------------------------------------------*/
55

6-
import { Uri } from 'vscode';
6+
import * as vscode from 'vscode';
77
import { SinonSandbox, createSandbox } from 'sinon';
88
import { default as assert } from 'assert';
99

@@ -47,15 +47,15 @@ describe('RepositoriesManager', function () {
4747
describe('removeRepo', function () {
4848
it('removes only the specified repository when it is not at the last position', function () {
4949
const repo1 = new MockRepository();
50-
repo1.rootUri = Uri.file('/repo1');
50+
repo1.rootUri = vscode.Uri.file('/repo1');
5151
repo1.addRemote('origin', 'git@github.com:aaa/bbb');
5252

5353
const repo2 = new MockRepository();
54-
repo2.rootUri = Uri.file('/repo2');
54+
repo2.rootUri = vscode.Uri.file('/repo2');
5555
repo2.addRemote('origin', 'git@github.com:ccc/ddd');
5656

5757
const repo3 = new MockRepository();
58-
repo3.rootUri = Uri.file('/repo3');
58+
repo3.rootUri = vscode.Uri.file('/repo3');
5959
repo3.addRemote('origin', 'git@github.com:eee/fff');
6060

6161
reposManager.insertFolderManager(new FolderRepositoryManager(0, context, repo1, telemetry, new GitApiImpl(reposManager), credentialStore, createPrHelper, mockThemeWatcher));
@@ -75,11 +75,11 @@ describe('RepositoriesManager', function () {
7575

7676
it('removes only the specified repository when it is at the last position', function () {
7777
const repo1 = new MockRepository();
78-
repo1.rootUri = Uri.file('/repo1');
78+
repo1.rootUri = vscode.Uri.file('/repo1');
7979
repo1.addRemote('origin', 'git@github.com:aaa/bbb');
8080

8181
const repo2 = new MockRepository();
82-
repo2.rootUri = Uri.file('/repo2');
82+
repo2.rootUri = vscode.Uri.file('/repo2');
8383
repo2.addRemote('origin', 'git@github.com:ccc/ddd');
8484

8585
reposManager.insertFolderManager(new FolderRepositoryManager(0, context, repo1, telemetry, new GitApiImpl(reposManager), credentialStore, createPrHelper, mockThemeWatcher));
@@ -96,15 +96,15 @@ describe('RepositoriesManager', function () {
9696

9797
it('removes only the middle repository leaving others intact', function () {
9898
const repo1 = new MockRepository();
99-
repo1.rootUri = Uri.file('/repo1');
99+
repo1.rootUri = vscode.Uri.file('/repo1');
100100
repo1.addRemote('origin', 'git@github.com:aaa/bbb');
101101

102102
const repo2 = new MockRepository();
103-
repo2.rootUri = Uri.file('/repo2');
103+
repo2.rootUri = vscode.Uri.file('/repo2');
104104
repo2.addRemote('origin', 'git@github.com:ccc/ddd');
105105

106106
const repo3 = new MockRepository();
107-
repo3.rootUri = Uri.file('/repo3');
107+
repo3.rootUri = vscode.Uri.file('/repo3');
108108
repo3.addRemote('origin', 'git@github.com:eee/fff');
109109

110110
reposManager.insertFolderManager(new FolderRepositoryManager(0, context, repo1, telemetry, new GitApiImpl(reposManager), credentialStore, createPrHelper, mockThemeWatcher));
@@ -123,11 +123,11 @@ describe('RepositoriesManager', function () {
123123

124124
it('does nothing when removing a repo that is not tracked', function () {
125125
const repo1 = new MockRepository();
126-
repo1.rootUri = Uri.file('/repo1');
126+
repo1.rootUri = vscode.Uri.file('/repo1');
127127
repo1.addRemote('origin', 'git@github.com:aaa/bbb');
128128

129129
const unknownRepo = new MockRepository();
130-
unknownRepo.rootUri = Uri.file('/unknown');
130+
unknownRepo.rootUri = vscode.Uri.file('/unknown');
131131

132132
reposManager.insertFolderManager(new FolderRepositoryManager(0, context, repo1, telemetry, new GitApiImpl(reposManager), credentialStore, createPrHelper, mockThemeWatcher));
133133

@@ -139,4 +139,61 @@ describe('RepositoriesManager', function () {
139139
assert.strictEqual(reposManager.folderManagers[0].repository.rootUri.toString(), repo1.rootUri.toString());
140140
});
141141
});
142+
143+
describe('removeMissingRepos', function () {
144+
it('removes folder managers whose root URIs no longer exist on disk', async function () {
145+
const repo1 = new MockRepository();
146+
repo1.rootUri = vscode.Uri.file('/existing-repo');
147+
repo1.addRemote('origin', 'git@github.com:aaa/bbb');
148+
149+
const repo2 = new MockRepository();
150+
repo2.rootUri = vscode.Uri.file('/removed-worktree');
151+
repo2.addRemote('origin', 'git@github.com:aaa/bbb');
152+
153+
reposManager.insertFolderManager(new FolderRepositoryManager(0, context, repo1, telemetry, new GitApiImpl(reposManager), credentialStore, createPrHelper, mockThemeWatcher));
154+
reposManager.insertFolderManager(new FolderRepositoryManager(1, context, repo2, telemetry, new GitApiImpl(reposManager), credentialStore, createPrHelper, mockThemeWatcher));
155+
156+
assert.strictEqual(reposManager.folderManagers.length, 2);
157+
158+
// Stub vscode.workspace.fs.stat to succeed for repo1 and fail for repo2
159+
const statStub = sinon.stub(vscode.workspace.fs, 'stat');
160+
statStub.callsFake(async (uri: vscode.Uri) => {
161+
if (uri.fsPath === repo2.rootUri.fsPath) {
162+
throw vscode.FileSystemError.FileNotFound(uri);
163+
}
164+
return { type: vscode.FileType.Directory, ctime: 0, mtime: 0, size: 0 };
165+
});
166+
167+
await reposManager.removeMissingRepos();
168+
169+
assert.strictEqual(reposManager.folderManagers.length, 1);
170+
assert.strictEqual(reposManager.folderManagers[0].repository.rootUri.toString(), repo1.rootUri.toString());
171+
});
172+
173+
it('keeps all repos when all paths exist on disk', async function () {
174+
const repo1 = new MockRepository();
175+
repo1.rootUri = vscode.Uri.file('/repo1');
176+
repo1.addRemote('origin', 'git@github.com:aaa/bbb');
177+
178+
const repo2 = new MockRepository();
179+
repo2.rootUri = vscode.Uri.file('/repo2');
180+
repo2.addRemote('origin', 'git@github.com:ccc/ddd');
181+
182+
reposManager.insertFolderManager(new FolderRepositoryManager(0, context, repo1, telemetry, new GitApiImpl(reposManager), credentialStore, createPrHelper, mockThemeWatcher));
183+
reposManager.insertFolderManager(new FolderRepositoryManager(1, context, repo2, telemetry, new GitApiImpl(reposManager), credentialStore, createPrHelper, mockThemeWatcher));
184+
185+
const statStub = sinon.stub(vscode.workspace.fs, 'stat');
186+
statStub.resolves({ type: vscode.FileType.Directory, ctime: 0, mtime: 0, size: 0 });
187+
188+
await reposManager.removeMissingRepos();
189+
190+
assert.strictEqual(reposManager.folderManagers.length, 2);
191+
});
192+
193+
it('does nothing when there are no folder managers', async function () {
194+
assert.strictEqual(reposManager.folderManagers.length, 0);
195+
await reposManager.removeMissingRepos();
196+
assert.strictEqual(reposManager.folderManagers.length, 0);
197+
});
198+
});
142199
});

‎src/view/prsTreeDataProvider.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,8 @@ export class PullRequestsTreeDataProvider extends Disposable implements vscode.T
6262
this.refreshAllQueryResults(true);
6363
}
6464
}));
65-
this._register(vscode.commands.registerCommand('pr.refreshList', _ => {
65+
this._register(vscode.commands.registerCommand('pr.refreshList', async _ => {
66+
await this._reposManager.removeMissingRepos();
6667
this.prsTreeModel.forceClearCache();
6768
this.refreshAllQueryResults(true);
6869
}));

‎src/view/prsTreeModel.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,8 @@ export class PrsTreeModel extends Disposable {
132132
if (changed.added) {
133133
repoEvents(changed.added);
134134
this._onDidChangeData.fire(changed.added);
135+
} else {
136+
this._onDidChangeData.fire();
135137
}
136138
}));
137139

‎src/view/reviewsManager.ts‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,23 @@ export class ReviewsManager extends Disposable {
6868
this._prsTreeDataProvider.initialize(this._reviewManagers.map(manager => manager.reviewModel), this._notificationsManager);
6969
}
7070
}));
71+
72+
this._register(this._reposManager.onDidChangeFolderRepositories(changed => {
73+
if (!changed.added) {
74+
this.removeOrphanedReviewManagers();
75+
}
76+
}));
77+
}
78+
79+
private removeOrphanedReviewManagers(): void {
80+
const folderManagerUris = new Set(this._reposManager.folderManagers.map(m => m.repository.rootUri.toString()));
81+
for (let i = this._reviewManagers.length - 1; i >= 0; i--) {
82+
const manager = this._reviewManagers[i];
83+
if (!folderManagerUris.has(manager.repository.rootUri.toString())) {
84+
this._reviewManagers.splice(i, 1);
85+
manager.dispose();
86+
}
87+
}
7188
}
7289

7390
async provideTextDocumentContent(uri: vscode.Uri): Promise<string | undefined> {

0 commit comments

Comments
 (0)