From c65d0656c9ef8fe52e2caabb901b8496132c1c7a Mon Sep 17 00:00:00 2001 From: "paseo-bot[bot]" <266920839+paseo-bot[bot]@users.noreply.github.com> Date: Wed, 4 Nov 2026 04:35:36 +0000 Subject: [PATCH] fix(server): keep large ignore listings from stalling Git observation Ignored subtree scans multiplied every observed entry by the number of ignored roots, blocking the main thread and expiring watcher subscriptions. Compile lexical roots once and check candidate ancestors, retaining path namespaces and ignore update barriers. --- .../file-observer/internal/observation.ts | 5 +- .../file-observer/internal/paths.test.ts | 28 ++++++- .../server/file-observer/internal/paths.ts | 5 ++ .../src/server/git-observation-paths.ts | 13 ++-- ...t-service.large-ignore.integration.test.ts | 76 +++++++++++++++++++ .../src/server/workspace-git-service.ts | 61 +++------------ packages/server/src/utils/path.test.ts | 29 +++++++ packages/server/src/utils/path.ts | 20 +++++ 8 files changed, 180 insertions(+), 57 deletions(-) create mode 100644 packages/server/src/server/workspace-git-service.large-ignore.integration.test.ts diff --git a/packages/server/src/server/file-observer/internal/observation.ts b/packages/server/src/server/file-observer/internal/observation.ts index f0c27a504a..b00ffd632f 100644 --- a/packages/server/src/server/file-observer/internal/observation.ts +++ b/packages/server/src/server/file-observer/internal/observation.ts @@ -41,6 +41,7 @@ class Observation implements ManagedObservation, ObservationHost { readonly root: string; readonly metrics: ObserverMetrics; private ignoredRoots: string[]; + private matchesIgnoredPath: (path: string) => boolean; private status: "open" | "failed" | "closed" = "open"; private readonly backend: ObservationBackend; private readonly pendingEvents = new Map(); @@ -61,6 +62,7 @@ class Observation implements ManagedObservation, ObservationHost { this.root = input.root; this.metrics = input.metrics; this.ignoredRoots = input.paths.normalizeIgnoredRoots(input.root, input.options.ignore ?? []); + this.matchesIgnoredPath = input.paths.createIgnoreMatcher(this.ignoredRoots); this.backend = input.createBackend(this); } @@ -89,6 +91,7 @@ class Observation implements ManagedObservation, ObservationHost { const next = this.input.paths.normalizeIgnoredRoots(this.root, paths); if (samePaths(this.ignoredRoots, next)) return; this.ignoredRoots = next; + this.matchesIgnoredPath = this.input.paths.createIgnoreMatcher(next); for (const path of this.pendingEvents.keys()) { if (this.isIgnored(path)) this.pendingEvents.delete(path); } @@ -120,7 +123,7 @@ class Observation implements ManagedObservation, ObservationHost { } isIgnored(path: string): boolean { - return this.ignoredRoots.some((root) => this.input.paths.isInside(root, path)); + return this.matchesIgnoredPath(path); } isPathInside(root: string, path: string): boolean { diff --git a/packages/server/src/server/file-observer/internal/paths.test.ts b/packages/server/src/server/file-observer/internal/paths.test.ts index 8399760ef6..1fd2febe9f 100644 --- a/packages/server/src/server/file-observer/internal/paths.test.ts +++ b/packages/server/src/server/file-observer/internal/paths.test.ts @@ -1,4 +1,4 @@ -import { win32 } from "node:path"; +import { posix, win32 } from "node:path"; import { describe, expect, test } from "vitest"; import { createObserverPaths } from "./paths.js"; @@ -40,3 +40,29 @@ describe("collapse", () => { expect(paths.collapse([parent, sibling, descendant]).sort()).toEqual([parent, sibling].sort()); }); }); + +describe("ignored root matching", () => { + test.each(["linux", "win32"] as const)( + "matches ancestors and sibling boundaries on %s", + (platform) => { + const paths = createObserverPaths(platform); + const root = platform === "win32" ? "C:\\repo" : "/repo"; + const api = platform === "win32" ? win32 : posix; + const ignored = api.join(root, "output"); + const matches = paths.createIgnoreMatcher([ignored]); + expect(matches(ignored)).toBe(true); + expect(matches(api.join(ignored, "nested", "file.txt"))).toBe(true); + expect(matches(api.join(root, "output-sibling", "file.txt"))).toBe(false); + expect(matches(root)).toBe(false); + expect(matches(api.join(root, "src", "file.txt"))).toBe(false); + if (platform === "win32") { + expect(matches(api.join(ignored, "nested", "file.txt").toUpperCase())).toBe(true); + } + }, + ); + + test("an empty ignore set accepts every path", () => { + const matches = createObserverPaths("linux").createIgnoreMatcher([]); + expect(matches("/repo/file.txt")).toBe(false); + }); +}); diff --git a/packages/server/src/server/file-observer/internal/paths.ts b/packages/server/src/server/file-observer/internal/paths.ts index 004b7ef298..8d75156ca9 100644 --- a/packages/server/src/server/file-observer/internal/paths.ts +++ b/packages/server/src/server/file-observer/internal/paths.ts @@ -1,9 +1,11 @@ import { posix, win32 } from "node:path"; +import { createPathContainmentMatcher } from "../../../utils/path.js"; export interface ObserverPaths { isInside(root: string, path: string): boolean; normalizeIgnoredRoots(root: string, paths: string[]): string[]; collapse(paths: string[]): string[]; + createIgnoreMatcher(roots: string[]): (path: string) => boolean; isExpectedWatchDisappearance(error: unknown): boolean; } @@ -50,6 +52,9 @@ export function createObserverPaths(platform: NodeJS.Platform): ObserverPaths { return { isInside, collapse, + createIgnoreMatcher(roots) { + return createPathContainmentMatcher(roots, platform === "win32"); + }, normalizeIgnoredRoots(root, paths) { const inside = paths .map((path) => pathApi.resolve(path)) diff --git a/packages/server/src/server/git-observation-paths.ts b/packages/server/src/server/git-observation-paths.ts index 1ef0a4e571..e37f0539f9 100644 --- a/packages/server/src/server/git-observation-paths.ts +++ b/packages/server/src/server/git-observation-paths.ts @@ -3,7 +3,11 @@ import { readlink } from "node:fs/promises"; import { promisify } from "node:util"; import pLimit from "p-limit"; import { dirname, resolve as resolvePath, sep } from "node:path"; -import { looksLikeDefiniteWindowsPath, normalizePathPreservingCase } from "../utils/path.js"; +import { + createPathContainmentMatcher, + looksLikeDefiniteWindowsPath, + normalizePathPreservingCase, +} from "../utils/path.js"; const nativeRealpath = promisify(realpath.native); const jsRealpath = promisify(realpath); @@ -77,15 +81,12 @@ export class GitObservationPaths { excluded.push(this.windows ? subtree.toLowerCase() : subtree); } } - const prefixes = excluded.map((root) => `${root}/`); + const isExcluded = createPathContainmentMatcher(excluded, this.windows); return (candidate) => { const normalized = this.normalizeEventPath(candidate); const comparable = this.windows ? normalized.toLowerCase() : normalized; if (this.aliases.includes(comparable)) return false; - for (let i = 0; i < excluded.length; i++) { - if (comparable === excluded[i] || comparable.startsWith(prefixes[i])) return false; - } - return true; + return !isExcluded(comparable); }; } diff --git a/packages/server/src/server/workspace-git-service.large-ignore.integration.test.ts b/packages/server/src/server/workspace-git-service.large-ignore.integration.test.ts new file mode 100644 index 0000000000..e55d71099f --- /dev/null +++ b/packages/server/src/server/workspace-git-service.large-ignore.integration.test.ts @@ -0,0 +1,76 @@ +import { execFile as execFileCallback, execFileSync } from "node:child_process"; +import { mkdir, mkdtemp, rm, writeFile } from "node:fs/promises"; +import path from "node:path"; +import { promisify } from "node:util"; +import { expect, test } from "vitest"; + +const execFile = promisify(execFileCallback); + +test("a large ignored-directory listing establishes a working-tree watcher before its deadline", async () => { + const artifacts = path.resolve(".artifacts"); + await mkdir(artifacts, { recursive: true }); + const root = await mkdtemp(path.join(artifacts, "large-ignore-")); + const repo = path.join(root, "repo"); + await mkdir(repo); + try { + execFileSync("git", ["init", "-q", repo]); + await writeFile(path.join(repo, ".gitignore"), "probes/\n"); + // An ignored parent containing force-tracked files cannot be excluded as a whole. + // Keep the observed directory count small enough for Linux's watcher limit. + for (let parent = 0; parent < 400; parent++) { + const directory = path.join(repo, "probes", `p${parent}`); + await mkdir(directory, { recursive: true }); + await writeFile(path.join(directory, "notes.md"), "tracked\n"); + await Promise.all( + Array.from({ length: 100 }, async (_, index) => { + const ignored = path.join(directory, `out${index}`); + await mkdir(ignored); + await writeFile(path.join(ignored, "data.bin"), "ignored\n"); + }), + ); + } + execFileSync("git", ["add", ".gitignore"], { cwd: repo }); + execFileSync("git", ["add", "-f", "--", "**/notes.md"], { cwd: repo }); + const listing = execFileSync( + "git", + ["ls-files", "-o", "-i", "--directory", "--exclude-standard"], + { cwd: repo, encoding: "utf8", maxBuffer: 8 * 1024 * 1024 }, + ); + expect(listing.trim().split("\n")).toHaveLength(40_000); + + // Bound the failing baseline in a child: timed-out subscriptions continue their + // startup scan until it settles, so awaiting disposal would hide the failure. + const source = ` + import pino from "pino"; + import { WorkspaceGitServiceImpl } from "./src/server/workspace-git-service.ts"; + const service = new WorkspaceGitServiceImpl({ + logger: pino({ enabled: false }), paseoHome: process.argv[2], + }); + let maxTickMs = 0; + let previous = performance.now(); + const timer = setInterval(() => { + const now = performance.now(); + maxTickMs = Math.max(maxTickMs, now - previous); + previous = now; + }, 10); + const watch = await service.requestWorkingTreeWatch(process.argv[1], () => {}); + const active = service.getMetrics().fileObserver.activeObservationCount; + await new Promise((resolve) => setTimeout(resolve, 100)); + clearInterval(timer); + console.log(JSON.stringify({ active, maxTickMs })); + if (active !== 1) process.exit(0); + watch.unsubscribe(); + await service.dispose(); + `; + const { stdout } = await execFile( + process.execPath, + ["--import", "tsx", "--input-type=module", "--eval", source, repo, path.join(root, "home")], + { cwd: path.resolve(import.meta.dirname, "../.."), timeout: 30_000, maxBuffer: 1024 * 1024 }, + ); + const measurement = JSON.parse(stdout.trim()); + expect(measurement.active).toBe(1); + expect(measurement.maxTickMs).toBeLessThan(2_000); + } finally { + await rm(root, { recursive: true, force: true }); + } +}, 60_000); diff --git a/packages/server/src/server/workspace-git-service.ts b/packages/server/src/server/workspace-git-service.ts index 295d579265..40cc1b5c15 100644 --- a/packages/server/src/server/workspace-git-service.ts +++ b/packages/server/src/server/workspace-git-service.ts @@ -42,7 +42,11 @@ import { type ForgeResolver, } from "../services/forge-resolver.js"; import { parseGitRevParsePath } from "../utils/git-rev-parse-path.js"; -import { isPathInsideRoot, looksLikeDefiniteWindowsPath } from "../utils/path.js"; +import { + createPathContainmentMatcher, + isPathInsideRoot, + looksLikeDefiniteWindowsPath, +} from "../utils/path.js"; import { createRunGitCommand, runGitCommand, @@ -1939,7 +1943,7 @@ export class WorkspaceGitServiceImpl implements WorkspaceGitService { if (target.knownDirectories.has(directory) || matchesWatchPath(directory)) { continue; } - // `directory` and `gitDir`/`ignoredDirectory` all derive from the same + // `directory` and `gitDir` derive from the same // `target.watchPath` used to build this subscription, and `directory` // itself is the same `dirname(event.path)` value already trusted as a // plain string key by `target.knownDirectories.has()` above — no realpath @@ -1950,14 +1954,7 @@ export class WorkspaceGitServiceImpl implements WorkspaceGitService { if (isPathInsideRoot(gitDir, directory)) { continue; } - let ignored = false; - for (const ignoredDirectory of target.ignoredDirectories) { - if (isPathInsideRoot(ignoredDirectory, directory)) { - ignored = true; - break; - } - } - if (ignored) { + if (!target.acceptsEvent(directory)) { continue; } target.knownDirectories.add(directory); @@ -2047,47 +2044,13 @@ export class WorkspaceGitServiceImpl implements WorkspaceGitService { }, delayMs); } - /** - * String-prefix containment, not `isRealpathInsideRoot`: every entry on - * both sides is a plain path built from `target.watchPath` by this same - * class (`dirname(event.path)` for `knownDirectories`, `resolve(watchPath, - * ...)` for `ignoredDirectories` in `loadIgnoredDirs`) — no realpath - * divergence to reconcile, per the same reasoning as - * `noteWorkingTreeDirectories`. Normalizing each ignored root once up front - * instead of recomputing per pair turns a 30k-known x 40-ignored scan from - * ~26s of synchronous realpath syscalls into a bounded number of cheap - * string comparisons. - * - * Case-folding still needs to happen for a Windows-looking root, same as - * `isPathInsideRoot`/`isRealpathInsideRoot` in `../utils/path.js` — a - * case-insensitive filesystem can report a known directory whose case - * diverges from the git-reported ignored root. Since both sides derive - * from the single `target.watchPath` for this target, whether to fold is - * one decision for the whole call (`looksLikeDefiniteWindowsPath` - * reused from `../utils/path.js`, not reimplemented here), not a per-pair - * one — so it costs nothing beyond the already-precomputed prefixes. - */ private pruneKnownDirectories(target: WorkingTreeWatchTarget): void { - if (target.ignoredDirectories.size === 0) { - return; - } - const foldCase = looksLikeDefiniteWindowsPath(target.watchPath); - const comparable = (path: string) => - foldCase ? path.replaceAll("\\", "/").toLowerCase() : path; - const ignoredPrefixes: string[] = []; - const foldedIgnoredExact = foldCase ? new Set() : null; - for (const ignoredDirectory of target.ignoredDirectories) { - const ignored = comparable(ignoredDirectory); - ignoredPrefixes.push(`${ignored}/`); - foldedIgnoredExact?.add(ignored); - } + const isIgnored = createPathContainmentMatcher( + target.ignoredDirectories, + looksLikeDefiniteWindowsPath(target.watchPath), + ); for (const directory of target.knownDirectories) { - const comparableDirectory = comparable(directory); - const comparableDirectoryWithSep = `${comparableDirectory}/`; - const isIgnored = - (foldedIgnoredExact ?? target.ignoredDirectories).has(comparableDirectory) || - ignoredPrefixes.some((prefix) => comparableDirectoryWithSep.startsWith(prefix)); - if (isIgnored) { + if (isIgnored(directory)) { target.knownDirectories.delete(directory); } } diff --git a/packages/server/src/utils/path.test.ts b/packages/server/src/utils/path.test.ts index bb279d6c2f..34399473c1 100644 --- a/packages/server/src/utils/path.test.ts +++ b/packages/server/src/utils/path.test.ts @@ -6,6 +6,7 @@ import { describe, expect, test } from "vitest"; import { areEquivalentPaths, createPathEquivalenceMatcher, + createPathContainmentMatcher, getRealpathAwareRelativePath, isPathInsideRoot, } from "./path.js"; @@ -97,3 +98,31 @@ describe("path equivalence", () => { }, ); }); + +describe("indexed path containment", () => { + test("preserves POSIX case and component boundaries", () => { + const matches = createPathContainmentMatcher(["/repo/Build/", "/repo/cache"], false); + expect(matches("/repo/Build/nested/file")).toBe(true); + expect(matches("/repo/cache")).toBe(true); + expect(matches("/repo/Build-sibling/file")).toBe(false); + expect(matches("/repo/build/file")).toBe(false); + expect(matches("/repo")).toBe(false); + }); + + test("normalizes Windows case, separators and extended namespaces", () => { + const matches = createPathContainmentMatcher( + ["C:/Repo/Build", String.raw`\\server\share\cache`], + true, + ); + expect(matches(String.raw`\\?\C:\REPO\build\file`)).toBe(true); + expect(matches(String.raw`\\?\UNC\SERVER\SHARE\Cache\file`)).toBe(true); + expect(matches("C:/Repo/Build-Sibling/file")).toBe(false); + expect(matches(String.raw`\\server\share\cache-sibling\file`)).toBe(false); + }); + + test("handles filesystem roots and empty sets", () => { + expect(createPathContainmentMatcher(["/"], false)("/repo/file")).toBe(true); + expect(createPathContainmentMatcher(["C:/"], true)("c:/repo/file")).toBe(true); + expect(createPathContainmentMatcher([], false)("/repo/file")).toBe(false); + }); +}); diff --git a/packages/server/src/utils/path.ts b/packages/server/src/utils/path.ts index 45cb8b01b5..d21a6a16ee 100644 --- a/packages/server/src/utils/path.ts +++ b/packages/server/src/utils/path.ts @@ -72,6 +72,26 @@ export function stopPathContainmentMetrics(): number { return checks; } +/** Compile lexical subtree membership without scanning unrelated roots per candidate. */ +export function createPathContainmentMatcher( + roots: Iterable, + compareAsWindows: boolean, +): (candidate: string) => boolean { + const pathApi = compareAsWindows ? nodePath.win32 : nodePath.posix; + const indexed = new Set( + Array.from(roots, (root) => normalizePathForComparison(root, compareAsWindows)), + ); + return (candidate) => { + let current = normalizePathForComparison(candidate, compareAsWindows); + for (;;) { + if (indexed.has(current)) return true; + const parent = pathApi.dirname(current); + if (parent === current) return false; + current = parent; + } + }; +} + export function isPathInsideRoot(root: string, candidate: string): boolean { if (pathContainmentChecks !== null) pathContainmentChecks += 1; return getRelativePathInsideRoot(root, candidate) !== null;