Skip to content

Commit e024cb4

Browse files
fix(fs): cap file list and abort untracked scan to prevent session init timeout on large repos (#2819)
Co-authored-by: Charles Vien <charles.v@posthog.com>
1 parent 53a3ce1 commit e024cb4

4 files changed

Lines changed: 119 additions & 8 deletions

File tree

packages/git/src/queries.test.ts

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {
1313
getChangedFilesDetailed,
1414
getGitBusyState,
1515
getLinkedWorktreeMainPath,
16+
listAllFiles,
1617
remoteBranchExists,
1718
splitUnifiedDiffByFile,
1819
} from "./queries";
@@ -597,3 +598,53 @@ describe("getLinkedWorktreeMainPath", () => {
597598
expect(getLinkedWorktreeMainPath(worktreeDir)).toBeNull();
598599
});
599600
});
601+
602+
describe("listAllFiles", () => {
603+
let repoDir: string;
604+
605+
afterEach(async () => {
606+
if (repoDir) {
607+
await rm(repoDir, { recursive: true, force: true });
608+
}
609+
});
610+
611+
it("combines tracked and untracked files uncapped by default", async () => {
612+
repoDir = await setupRepo();
613+
await writeFile(path.join(repoDir, "untracked.txt"), "content");
614+
615+
const files = await listAllFiles(repoDir);
616+
617+
expect(files.sort()).toEqual(["file.txt", "untracked.txt"]);
618+
});
619+
620+
it("truncates to maxFiles", async () => {
621+
repoDir = await setupRepo();
622+
const git = createGitClient(repoDir);
623+
await writeFile(path.join(repoDir, "b.txt"), "content");
624+
await writeFile(path.join(repoDir, "c.txt"), "content");
625+
await git.add(["b.txt", "c.txt"]);
626+
await git.commit("add more files");
627+
628+
const files = await listAllFiles(repoDir, { maxFiles: 2 });
629+
630+
expect(files.length).toBe(2);
631+
});
632+
633+
it("keeps tracked files over untracked ones when truncating", async () => {
634+
repoDir = await setupRepo();
635+
await writeFile(path.join(repoDir, "untracked.txt"), "content");
636+
637+
const files = await listAllFiles(repoDir, { maxFiles: 1 });
638+
639+
expect(files).toEqual(["file.txt"]);
640+
});
641+
642+
it("returns tracked files when the untracked scan times out", async () => {
643+
repoDir = await setupRepo();
644+
await writeFile(path.join(repoDir, "untracked.txt"), "content");
645+
646+
const files = await listAllFiles(repoDir, { timeoutMs: 0 });
647+
648+
expect(files).toContain("file.txt");
649+
});
650+
});

packages/git/src/queries.ts

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1181,15 +1181,37 @@ export async function listUntrackedFiles(
11811181
);
11821182
}
11831183

1184+
export interface ListAllFilesOptions {
1185+
maxFiles?: number;
1186+
timeoutMs?: number;
1187+
}
1188+
11841189
export async function listAllFiles(
11851190
baseDir: string,
1186-
options?: CreateGitClientOptions,
1191+
options?: ListAllFilesOptions,
11871192
): Promise<string[]> {
1188-
const [tracked, untracked] = await Promise.all([
1189-
listFiles(baseDir, options),
1190-
listUntrackedFiles(baseDir, options),
1191-
]);
1192-
return [...tracked, ...untracked];
1193+
const { maxFiles, timeoutMs } = options ?? {};
1194+
const controller =
1195+
timeoutMs !== undefined ? new AbortController() : undefined;
1196+
const timer =
1197+
controller && timeoutMs !== undefined
1198+
? setTimeout(() => controller.abort(), timeoutMs)
1199+
: undefined;
1200+
try {
1201+
const [tracked, untracked] = await Promise.all([
1202+
listFiles(baseDir).catch((): string[] => []),
1203+
listUntrackedFiles(baseDir, { abortSignal: controller?.signal }).catch(
1204+
(): string[] => [],
1205+
),
1206+
]);
1207+
const combined = tracked.concat(untracked);
1208+
if (maxFiles !== undefined && combined.length > maxFiles) {
1209+
combined.splice(maxFiles);
1210+
}
1211+
return combined;
1212+
} finally {
1213+
if (timer) clearTimeout(timer);
1214+
}
11931215
}
11941216

11951217
// Tracked + untracked files containing `pattern` (literal, case-insensitive).

packages/workspace-server/src/services/fs/service.test.ts

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,35 @@ describe("FsService.listRepoFiles", () => {
4848
{ path: "src/sub/c.ts", kind: "file" },
4949
]);
5050
});
51+
52+
it("passes the file cap and timeout through to listAllFiles", async () => {
53+
vi.mocked(getChangedFiles).mockResolvedValue(new Set());
54+
vi.mocked(listAllFiles).mockResolvedValue([]);
55+
56+
const service = new FsService();
57+
await service.listRepoFiles("/repo");
58+
59+
expect(listAllFiles).toHaveBeenCalledWith("/repo", {
60+
maxFiles: 50_000,
61+
timeoutMs: 8_000,
62+
});
63+
});
64+
65+
it("total entries can exceed the file cap when derived directories are included", async () => {
66+
vi.mocked(getChangedFiles).mockResolvedValue(new Set());
67+
const cappedList = Array.from(
68+
{ length: 50_000 },
69+
(_, i) => `src/sub${i}/file.ts`,
70+
);
71+
vi.mocked(listAllFiles).mockResolvedValue(cappedList);
72+
73+
const service = new FsService();
74+
const entries = await service.listRepoFiles("/repo");
75+
76+
const fileEntries = entries.filter((e) => e.kind === "file");
77+
expect(fileEntries.length).toBe(50_000);
78+
expect(entries.length).toBeGreaterThan(50_000);
79+
});
5180
});
5281

5382
describe("FsService repo file IO", () => {

packages/workspace-server/src/services/fs/service.ts

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ import type { BoundedReadResult, DirectoryEntry, FileEntry } from "./schemas";
88
export class FsService {
99
private static readonly CACHE_TTL = 30000;
1010
private static readonly READ_REPO_FILES_CONCURRENCY = 24;
11+
private static readonly MAX_REPO_FILES = 50_000;
12+
private static readonly LIST_FILES_TIMEOUT_MS = 8_000;
1113
private cache = new Map<string, { files: FileEntry[]; timestamp: number }>();
1214

1315
async listDirectory(dirPath: string): Promise<DirectoryEntry[]> {
@@ -43,7 +45,7 @@ export class FsService {
4345
const changedFiles = await getChangedFiles(repoPath);
4446

4547
if (query?.trim()) {
46-
const allFiles = await listAllFiles(repoPath);
48+
const allFiles = await this.listAllFilesBounded(repoPath);
4749
const directories = this.deriveDirectories(allFiles);
4850
const lowerQuery = query.toLowerCase();
4951
const matchingDirs = directories.filter((d) =>
@@ -64,7 +66,7 @@ export class FsService {
6466
return limit ? cached.files.slice(0, limit) : cached.files;
6567
}
6668

67-
const files = await listAllFiles(repoPath);
69+
const files = await this.listAllFilesBounded(repoPath);
6870
const directories = this.deriveDirectories(files);
6971
const entries = [
7072
...this.toDirectoryEntries(directories),
@@ -221,6 +223,13 @@ export class FsService {
221223
}));
222224
}
223225

226+
private listAllFilesBounded(repoPath: string): Promise<string[]> {
227+
return listAllFiles(repoPath, {
228+
maxFiles: FsService.MAX_REPO_FILES,
229+
timeoutMs: FsService.LIST_FILES_TIMEOUT_MS,
230+
});
231+
}
232+
224233
private deriveDirectories(files: string[]): string[] {
225234
const dirs = new Set<string>();
226235
for (const file of files) {

0 commit comments

Comments
 (0)