consolidate file-list cap and timeout into listAllFiles

Author: charlesvien

packages/git/src/queries.test.ts

@@ -13,6 +13,7 @@ import {
   getChangedFilesDetailed,
   getGitBusyState,
   getLinkedWorktreeMainPath,
+  listAllFiles,
   remoteBranchExists,
   splitUnifiedDiffByFile,
 } from "./queries";
@@ -597,3 +598,44 @@ describe("getLinkedWorktreeMainPath", () => {
     expect(getLinkedWorktreeMainPath(worktreeDir)).toBeNull();
   });
 });
+
+describe("listAllFiles", () => {
+  let repoDir: string;
+
+  afterEach(async () => {
+    if (repoDir) {
+      await rm(repoDir, { recursive: true, force: true });
+    }
+  });
+
+  it("combines tracked and untracked files uncapped by default", async () => {
+    repoDir = await setupRepo();
+    await writeFile(path.join(repoDir, "untracked.txt"), "content");
+
+    const files = await listAllFiles(repoDir);
+
+    expect(files.sort()).toEqual(["file.txt", "untracked.txt"]);
+  });
+
+  it("truncates to maxFiles", async () => {
+    repoDir = await setupRepo();
+    const git = createGitClient(repoDir);
+    await writeFile(path.join(repoDir, "b.txt"), "content");
+    await writeFile(path.join(repoDir, "c.txt"), "content");
+    await git.add(["b.txt", "c.txt"]);
+    await git.commit("add more files");
+
+    const files = await listAllFiles(repoDir, { maxFiles: 2 });
+
+    expect(files.length).toBe(2);
+  });
+
+  it("keeps untracked files over tracked ones when truncating", async () => {
+    repoDir = await setupRepo();
+    await writeFile(path.join(repoDir, "untracked.txt"), "content");
+
+    const files = await listAllFiles(repoDir, { maxFiles: 1 });
+
+    expect(files).toEqual(["untracked.txt"]);
+  });
+});

packages/git/src/queries.ts

@@ -1181,15 +1181,39 @@ export async function listUntrackedFiles(
   );
 }
 
+export interface ListAllFilesOptions {
+  maxFiles?: number;
+  timeoutMs?: number;
+}
+
 export async function listAllFiles(
   baseDir: string,
-  options?: CreateGitClientOptions,
+  options?: ListAllFilesOptions,
 ): Promise<string[]> {
-  const [tracked, untracked] = await Promise.all([
-    listFiles(baseDir, options),
-    listUntrackedFiles(baseDir, options),
-  ]);
-  return [...tracked, ...untracked];
+  const { maxFiles, timeoutMs } = options ?? {};
+  const controller =
+    timeoutMs !== undefined ? new AbortController() : undefined;
+  const timer =
+    controller && timeoutMs !== undefined
+      ? setTimeout(() => controller.abort(), timeoutMs)
+      : undefined;
+  try {
+    const [tracked, untracked] = await Promise.all([
+      listFiles(baseDir, { abortSignal: controller?.signal }).catch(
+        (): string[] => [],
+      ),
+      listUntrackedFiles(baseDir, { abortSignal: controller?.signal }).catch(
+        (): string[] => [],
+      ),
+    ]);
+    const combined = untracked.concat(tracked);
+    if (maxFiles !== undefined && combined.length > maxFiles) {
+      combined.splice(maxFiles);
+    }
+    return combined;
+  } finally {
+    if (timer) clearTimeout(timer);
+  }
 }
 
 // Tracked + untracked files containing `pattern` (literal, case-insensitive).

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

@@ -5,26 +5,20 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
 
 vi.mock("@posthog/git/queries", () => ({
   getChangedFiles: vi.fn(async () => new Set<string>()),
-  listFiles: vi.fn(async () => []),
-  listUntrackedFiles: vi.fn(async () => []),
+  listAllFiles: vi.fn(async () => []),
 }));
 
-import {
-  getChangedFiles,
-  listFiles,
-  listUntrackedFiles,
-} from "@posthog/git/queries";
+import { getChangedFiles, listAllFiles } from "@posthog/git/queries";
 import { FsService } from "./service";
 
 describe("FsService.listRepoFiles", () => {
   it("derives directory entries alongside files", async () => {
     vi.mocked(getChangedFiles).mockResolvedValue(new Set());
-    vi.mocked(listFiles).mockResolvedValue([
+    vi.mocked(listAllFiles).mockResolvedValue([
       "a.ts",
       "src/b.ts",
       "src/sub/c.ts",
     ]);
-    vi.mocked(listUntrackedFiles).mockResolvedValue([]);
 
     const service = new FsService();
     const entries = await service.listRepoFiles("/repo");
@@ -40,12 +34,11 @@ describe("FsService.listRepoFiles", () => {
 
   it("filters directories and files by query substring", async () => {
     vi.mocked(getChangedFiles).mockResolvedValue(new Set());
-    vi.mocked(listFiles).mockResolvedValue([
+    vi.mocked(listAllFiles).mockResolvedValue([
       "a.ts",
       "src/b.ts",
       "src/sub/c.ts",
     ]);
-    vi.mocked(listUntrackedFiles).mockResolvedValue([]);
 
     const service = new FsService();
     const entries = await service.listRepoFiles("/repo", "sub");
@@ -56,26 +49,26 @@ describe("FsService.listRepoFiles", () => {
     ]);
   });
 
-  it("caps file list at MAX_REPO_FILES when repo is very large", async () => {
+  it("passes the file cap and timeout through to listAllFiles", async () => {
     vi.mocked(getChangedFiles).mockResolvedValue(new Set());
-    const bigList = Array.from({ length: 60_000 }, (_, i) => `file${i}.ts`);
-    vi.mocked(listFiles).mockResolvedValue(bigList);
-    vi.mocked(listUntrackedFiles).mockResolvedValue([]);
+    vi.mocked(listAllFiles).mockResolvedValue([]);
 
     const service = new FsService();
-    const entries = await service.listRepoFiles("/repo");
+    await service.listRepoFiles("/repo");
 
-    expect(entries.length).toBe(50_000);
+    expect(listAllFiles).toHaveBeenCalledWith("/repo", {
+      maxFiles: 50_000,
+      timeoutMs: 8_000,
+    });
   });
 
-  it("total entries can exceed MAX_REPO_FILES when derived directories are included", async () => {
+  it("total entries can exceed the file cap when derived directories are included", async () => {
     vi.mocked(getChangedFiles).mockResolvedValue(new Set());
-    const bigList = Array.from(
-      { length: 60_000 },
+    const cappedList = Array.from(
+      { length: 50_000 },
       (_, i) => `src/sub${i}/file.ts`,
     );
-    vi.mocked(listFiles).mockResolvedValue(bigList);
-    vi.mocked(listUntrackedFiles).mockResolvedValue([]);
+    vi.mocked(listAllFiles).mockResolvedValue(cappedList);
 
     const service = new FsService();
     const entries = await service.listRepoFiles("/repo");
@@ -84,17 +77,6 @@ describe("FsService.listRepoFiles", () => {
     expect(fileEntries.length).toBe(50_000);
     expect(entries.length).toBeGreaterThan(50_000);
   });
-
-  it("omits untracked files when git ls-files --others is aborted", async () => {
-    vi.mocked(getChangedFiles).mockResolvedValue(new Set());
-    vi.mocked(listFiles).mockResolvedValue(["tracked.ts"]);
-    vi.mocked(listUntrackedFiles).mockRejectedValue(new Error("AbortError"));
-
-    const service = new FsService();
-    const entries = await service.listRepoFiles("/repo");
-
-    expect(entries.some((e) => e.path === "tracked.ts")).toBe(true);
-  });
 });
 
 describe("FsService repo file IO", () => {

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

@@ -1,10 +1,6 @@
 import fs from "node:fs/promises";
 import path from "node:path";
-import {
-  getChangedFiles,
-  listFiles,
-  listUntrackedFiles,
-} from "@posthog/git/queries";
+import { getChangedFiles, listAllFiles } from "@posthog/git/queries";
 import { injectable } from "inversify";
 import type { BoundedReadResult, DirectoryEntry, FileEntry } from "./schemas";
 
@@ -13,7 +9,7 @@ export class FsService {
   private static readonly CACHE_TTL = 30000;
   private static readonly READ_REPO_FILES_CONCURRENCY = 24;
   private static readonly MAX_REPO_FILES = 50_000;
-  private static readonly UNTRACKED_TIMEOUT_MS = 8_000;
+  private static readonly LIST_FILES_TIMEOUT_MS = 8_000;
   private cache = new Map<string, { files: FileEntry[]; timestamp: number }>();
 
   async listDirectory(dirPath: string): Promise<DirectoryEntry[]> {
@@ -49,7 +45,7 @@ export class FsService {
       const changedFiles = await getChangedFiles(repoPath);
 
       if (query?.trim()) {
-        const allFiles = await this.fetchAllFiles(repoPath);
+        const allFiles = await this.listAllFilesBounded(repoPath);
         const directories = this.deriveDirectories(allFiles);
         const lowerQuery = query.toLowerCase();
         const matchingDirs = directories.filter((d) =>
@@ -70,7 +66,7 @@ export class FsService {
         return limit ? cached.files.slice(0, limit) : cached.files;
       }
 
-      const files = await this.fetchAllFiles(repoPath);
+      const files = await this.listAllFilesBounded(repoPath);
       const directories = this.deriveDirectories(files);
       const entries = [
         ...this.toDirectoryEntries(directories),
@@ -227,27 +223,11 @@ export class FsService {
     }));
   }
 
-  private async fetchAllFiles(repoPath: string): Promise<string[]> {
-    const controller = new AbortController();
-    const timer = setTimeout(
-      () => controller.abort(),
-      FsService.UNTRACKED_TIMEOUT_MS,
-    );
-    try {
-      const [tracked, untracked] = await Promise.all([
-        listFiles(repoPath),
-        listUntrackedFiles(repoPath, { abortSignal: controller.signal }).catch(
-          () => [],
-        ),
-      ]);
-      const combined = tracked.concat(untracked);
-      if (combined.length > FsService.MAX_REPO_FILES) {
-        combined.length = FsService.MAX_REPO_FILES;
-      }
-      return combined;
-    } finally {
-      clearTimeout(timer);
-    }
+  private listAllFilesBounded(repoPath: string): Promise<string[]> {
+    return listAllFiles(repoPath, {
+      maxFiles: FsService.MAX_REPO_FILES,
+      timeoutMs: FsService.LIST_FILES_TIMEOUT_MS,
+    });
   }
 
   private deriveDirectories(files: string[]): string[] {