Skip to content

Commit 7a42ee6

Browse files
authored
fix: Move folder cleanup and orphaned worktree pruning to main process (#1541)
## Problem Folder cleanup logic (removing deleted folders, pruning orphaned worktrees) ran in the renderer via useEffect. ## Changes 1. Move deleted-folder removal and orphaned worktree cleanup into FoldersService.initialize 2. Remove cleanupOrphanedWorktrees tRPC endpoint and associated schemas 3. Simplify useFolders hook by removing the init-on-mount side effect 4. Change cleanupOrphanedWorktrees return type to void (result was unused) ## How did you test this? Manually
1 parent 5fd4618 commit 7a42ee6

File tree

5 files changed

+179
-93
lines changed

5 files changed

+179
-93
lines changed

apps/code/src/main/services/folders/schemas.ts

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -29,20 +29,6 @@ export const updateFolderAccessedInput = z.object({
2929
folderId: z.string(),
3030
});
3131

32-
export const cleanupOrphanedWorktreesInput = z.object({
33-
mainRepoPath: z.string(),
34-
});
35-
36-
export const cleanupOrphanedWorktreesOutput = z.object({
37-
deleted: z.array(z.string()),
38-
errors: z.array(
39-
z.object({
40-
path: z.string(),
41-
error: z.string(),
42-
}),
43-
),
44-
});
45-
4632
export type RegisteredFolder = z.infer<typeof registeredFolderWithExistsSchema>;
4733
export type GetFoldersOutput = z.infer<typeof getFoldersOutput>;
4834
export type AddFolderInput = z.infer<typeof addFolderInput>;
@@ -51,12 +37,6 @@ export type RemoveFolderInput = z.infer<typeof removeFolderInput>;
5137
export type UpdateFolderAccessedInput = z.infer<
5238
typeof updateFolderAccessedInput
5339
>;
54-
export type CleanupOrphanedWorktreesInput = z.infer<
55-
typeof cleanupOrphanedWorktreesInput
56-
>;
57-
export type CleanupOrphanedWorktreesOutput = z.infer<
58-
typeof cleanupOrphanedWorktreesOutput
59-
>;
6040

6141
export const repositoryLookupResult = z
6242
.object({

apps/code/src/main/services/folders/service.test.ts

Lines changed: 135 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,137 @@ describe("FoldersService", () => {
128128
vi.clearAllMocks();
129129
});
130130

131+
describe("initialize", () => {
132+
function createService() {
133+
return new FoldersService(
134+
mockRepositoryRepo as unknown as IRepositoryRepository,
135+
mockWorkspaceRepo as unknown as IWorkspaceRepository,
136+
mockWorktreeRepo as unknown as IWorktreeRepository,
137+
);
138+
}
139+
140+
it("removes folders that no longer exist on disk", async () => {
141+
mockRepositoryRepo.findAll.mockReturnValue([
142+
{
143+
id: "folder-1",
144+
path: "/gone/project",
145+
lastAccessedAt: "2024-01-01T00:00:00.000Z",
146+
createdAt: "2024-01-01T00:00:00.000Z",
147+
updatedAt: "2024-01-01T00:00:00.000Z",
148+
},
149+
]);
150+
mockExistsSync.mockReturnValue(false);
151+
mockRepositoryRepo.findById.mockReturnValue({
152+
id: "folder-1",
153+
path: "/gone/project",
154+
});
155+
mockWorkspaceRepo.findAllByRepositoryId.mockReturnValue([]);
156+
157+
createService();
158+
await vi.waitFor(() => {
159+
expect(mockRepositoryRepo.delete).toHaveBeenCalledWith("folder-1");
160+
});
161+
});
162+
163+
it("cleans up orphaned worktrees for each existing folder", async () => {
164+
mockRepositoryRepo.findAll.mockReturnValue([
165+
{
166+
id: "folder-1",
167+
path: "/home/user/project-a",
168+
lastAccessedAt: "2024-01-01T00:00:00.000Z",
169+
createdAt: "2024-01-01T00:00:00.000Z",
170+
updatedAt: "2024-01-01T00:00:00.000Z",
171+
},
172+
{
173+
id: "folder-2",
174+
path: "/home/user/project-b",
175+
lastAccessedAt: "2024-01-01T00:00:00.000Z",
176+
createdAt: "2024-01-01T00:00:00.000Z",
177+
updatedAt: "2024-01-01T00:00:00.000Z",
178+
},
179+
]);
180+
mockExistsSync.mockReturnValue(true);
181+
mockWorktreeRepo.findAll.mockReturnValue([]);
182+
mockWorktreeManager.cleanupOrphanedWorktrees.mockResolvedValue({
183+
deleted: [],
184+
errors: [],
185+
});
186+
187+
createService();
188+
await vi.waitFor(() => {
189+
expect(
190+
mockWorktreeManager.cleanupOrphanedWorktrees,
191+
).toHaveBeenCalledTimes(2);
192+
});
193+
});
194+
195+
it("continues if one folder removal fails", async () => {
196+
mockRepositoryRepo.findAll.mockReturnValue([
197+
{
198+
id: "folder-1",
199+
path: "/gone/a",
200+
lastAccessedAt: "2024-01-01T00:00:00.000Z",
201+
createdAt: "2024-01-01T00:00:00.000Z",
202+
updatedAt: "2024-01-01T00:00:00.000Z",
203+
},
204+
{
205+
id: "folder-2",
206+
path: "/gone/b",
207+
lastAccessedAt: "2024-01-01T00:00:00.000Z",
208+
createdAt: "2024-01-01T00:00:00.000Z",
209+
updatedAt: "2024-01-01T00:00:00.000Z",
210+
},
211+
]);
212+
mockExistsSync.mockReturnValue(false);
213+
mockRepositoryRepo.findById
214+
.mockReturnValueOnce({ id: "folder-1", path: "/gone/a" })
215+
.mockReturnValueOnce({ id: "folder-2", path: "/gone/b" });
216+
mockWorkspaceRepo.findAllByRepositoryId.mockReturnValue([]);
217+
mockRepositoryRepo.delete
218+
.mockImplementationOnce(() => {
219+
throw new Error("db error");
220+
})
221+
.mockImplementationOnce(() => undefined);
222+
223+
createService();
224+
await vi.waitFor(() => {
225+
expect(mockRepositoryRepo.delete).toHaveBeenCalledTimes(2);
226+
expect(mockRepositoryRepo.delete).toHaveBeenCalledWith("folder-2");
227+
});
228+
});
229+
230+
it("continues if one worktree cleanup fails", async () => {
231+
mockRepositoryRepo.findAll.mockReturnValue([
232+
{
233+
id: "folder-1",
234+
path: "/home/user/project-a",
235+
lastAccessedAt: "2024-01-01T00:00:00.000Z",
236+
createdAt: "2024-01-01T00:00:00.000Z",
237+
updatedAt: "2024-01-01T00:00:00.000Z",
238+
},
239+
{
240+
id: "folder-2",
241+
path: "/home/user/project-b",
242+
lastAccessedAt: "2024-01-01T00:00:00.000Z",
243+
createdAt: "2024-01-01T00:00:00.000Z",
244+
updatedAt: "2024-01-01T00:00:00.000Z",
245+
},
246+
]);
247+
mockExistsSync.mockReturnValue(true);
248+
mockWorktreeRepo.findAll.mockReturnValue([]);
249+
mockWorktreeManager.cleanupOrphanedWorktrees
250+
.mockRejectedValueOnce(new Error("cleanup error"))
251+
.mockResolvedValueOnce({ deleted: [], errors: [] });
252+
253+
createService();
254+
await vi.waitFor(() => {
255+
expect(
256+
mockWorktreeManager.cleanupOrphanedWorktrees,
257+
).toHaveBeenCalledTimes(2);
258+
});
259+
});
260+
});
261+
131262
describe("getFolders", () => {
132263
it("returns empty array when no folders registered", async () => {
133264
mockRepositoryRepo.findAll.mockReturnValue([]);
@@ -318,11 +449,11 @@ describe("FoldersService", () => {
318449
errors: [],
319450
});
320451

321-
const result =
322-
await service.cleanupOrphanedWorktrees("/home/user/project");
452+
await service.cleanupOrphanedWorktrees("/home/user/project");
323453

324-
expect(result.deleted).toHaveLength(1);
325-
expect(result.errors).toHaveLength(0);
454+
expect(mockWorktreeManager.cleanupOrphanedWorktrees).toHaveBeenCalledWith(
455+
[],
456+
);
326457
});
327458

328459
it("excludes associated worktrees from cleanup", async () => {

apps/code/src/main/services/folders/service.ts

Lines changed: 43 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,7 @@ import { MAIN_TOKENS } from "../../di/tokens";
2626
import { getMainWindow } from "../../trpc/context";
2727
import { logger } from "../../utils/logger";
2828
import { getWorktreeLocation } from "../settingsStore";
29-
import type {
30-
CleanupOrphanedWorktreesOutput,
31-
RegisteredFolder,
32-
} from "./schemas";
29+
import type { RegisteredFolder } from "./schemas";
3330

3431
const log = logger.scope("folders-service");
3532

@@ -42,7 +39,46 @@ export class FoldersService {
4239
private readonly workspaceRepo: IWorkspaceRepository,
4340
@inject(MAIN_TOKENS.WorktreeRepository)
4441
private readonly worktreeRepo: IWorktreeRepository,
45-
) {}
42+
) {
43+
this.initialize().catch((err) => {
44+
log.error("Folders initialization failed", err);
45+
});
46+
}
47+
48+
private async initialize(): Promise<void> {
49+
const folders = await this.getFolders();
50+
51+
const deletedFolders = folders.filter((f) => !f.exists);
52+
if (deletedFolders.length > 0) {
53+
let removed = 0;
54+
for (const folder of deletedFolders) {
55+
try {
56+
await this.removeFolder(folder.id);
57+
removed++;
58+
} catch (err) {
59+
log.error(`Failed to remove deleted folder ${folder.path}:`, err);
60+
}
61+
}
62+
if (removed > 0) {
63+
log.info(`Removed ${removed} deleted folder(s)`);
64+
}
65+
}
66+
67+
const existingFolders = folders.filter((f) => f.exists);
68+
const results = await Promise.allSettled(
69+
existingFolders.map((folder) =>
70+
this.cleanupOrphanedWorktrees(folder.path),
71+
),
72+
);
73+
for (const [i, result] of results.entries()) {
74+
if (result.status === "rejected") {
75+
log.error(
76+
`Failed to cleanup orphaned worktrees for ${existingFolders[i].path}:`,
77+
result.reason,
78+
);
79+
}
80+
}
81+
}
4682

4783
async getFolders(): Promise<(RegisteredFolder & { exists: boolean })[]> {
4884
const repos = this.repositoryRepo.findAll();
@@ -190,16 +226,14 @@ export class FoldersService {
190226
this.repositoryRepo.updateLastAccessed(folderId);
191227
}
192228

193-
async cleanupOrphanedWorktrees(
194-
mainRepoPath: string,
195-
): Promise<CleanupOrphanedWorktreesOutput> {
229+
async cleanupOrphanedWorktrees(mainRepoPath: string): Promise<void> {
196230
const worktreeBasePath = getWorktreeLocation();
197231
const manager = new WorktreeManager({ mainRepoPath, worktreeBasePath });
198232

199233
const allWorktrees = this.worktreeRepo.findAll();
200234
const associatedWorktreePaths = allWorktrees.map((wt) => wt.path);
201235

202-
return await manager.cleanupOrphanedWorktrees(associatedWorktreePaths);
236+
await manager.cleanupOrphanedWorktrees(associatedWorktreePaths);
203237
}
204238

205239
getRepositoryByRemoteUrl(

apps/code/src/main/trpc/routers/folders.ts

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,6 @@ import { MAIN_TOKENS } from "../../di/tokens";
33
import {
44
addFolderInput,
55
addFolderOutput,
6-
cleanupOrphanedWorktreesInput,
7-
cleanupOrphanedWorktreesOutput,
86
getFoldersOutput,
97
getRepositoryByRemoteUrlInput,
108
removeFolderInput,
@@ -41,13 +39,6 @@ export const foldersRouter = router({
4139
return getService().updateFolderAccessed(input.folderId);
4240
}),
4341

44-
cleanupOrphanedWorktrees: publicProcedure
45-
.input(cleanupOrphanedWorktreesInput)
46-
.output(cleanupOrphanedWorktreesOutput)
47-
.mutation(({ input }) => {
48-
return getService().cleanupOrphanedWorktrees(input.mainRepoPath);
49-
}),
50-
5142
clearAllData: publicProcedure.mutation(() => {
5243
return getService().clearAllData();
5344
}),

apps/code/src/renderer/features/folders/hooks/useFolders.ts

Lines changed: 1 addition & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,12 @@
11
import type { RegisteredFolder } from "@main/services/folders/schemas";
2-
import { useFocusStore } from "@renderer/stores/focusStore";
32
import { trpc, trpcClient, useTRPC } from "@renderer/trpc";
43
import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query";
5-
import { logger } from "@utils/logger";
64
import { queryClient } from "@utils/queryClient";
7-
import { useCallback, useEffect, useMemo, useRef } from "react";
8-
9-
const log = logger.scope("folders");
5+
import { useCallback, useMemo } from "react";
106

117
export function useFolders() {
128
const trpcReact = useTRPC();
139
const queryClient = useQueryClient();
14-
const hasInitialized = useRef(false);
1510

1611
const { data: folders = [], isLoading } = useQuery(
1712
trpcReact.folders.getFolders.queryOptions(undefined, {
@@ -24,46 +19,6 @@ export function useFolders() {
2419
[folders],
2520
);
2621

27-
useEffect(() => {
28-
if (hasInitialized.current || isLoading || folders.length === 0) return;
29-
hasInitialized.current = true;
30-
31-
const deletedFolders = folders.filter((f) => f.exists === false);
32-
if (deletedFolders.length > 0) {
33-
Promise.all(
34-
deletedFolders.map((folder) =>
35-
trpcClient.folders.removeFolder
36-
.mutate({ folderId: folder.id })
37-
.catch((err) =>
38-
log.error(`Failed to remove deleted folder ${folder.path}:`, err),
39-
),
40-
),
41-
).then(() => {
42-
void queryClient.invalidateQueries(
43-
trpcReact.folders.getFolders.pathFilter(),
44-
);
45-
});
46-
}
47-
48-
for (const folder of existingFolders) {
49-
useFocusStore
50-
.getState()
51-
.restore(folder.path)
52-
.catch((error) => {
53-
log.error(`Failed to restore focus state for ${folder.path}:`, error);
54-
});
55-
56-
trpcClient.folders.cleanupOrphanedWorktrees
57-
.mutate({ mainRepoPath: folder.path })
58-
.catch((error) => {
59-
log.error(
60-
`Failed to cleanup orphaned worktrees for ${folder.path}:`,
61-
error,
62-
);
63-
});
64-
}
65-
}, [folders, existingFolders, isLoading, queryClient, trpcReact]);
66-
6722
const addFolderMutation = useMutation(
6823
trpcReact.folders.addFolder.mutationOptions({
6924
onSuccess: () => {
@@ -177,11 +132,6 @@ export const foldersApi = {
177132
async updateFolderAccessed(folderId: string) {
178133
return trpcClient.folders.updateFolderAccessed.mutate({ folderId });
179134
},
180-
async cleanupOrphanedWorktrees(mainRepoPath: string) {
181-
return trpcClient.folders.cleanupOrphanedWorktrees.mutate({
182-
mainRepoPath,
183-
});
184-
},
185135
getFolderByPath(folders: RegisteredFolder[], path: string) {
186136
return folders.find((f) => f.path === path);
187137
},

0 commit comments

Comments
 (0)