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.
This commit is contained in:
paseo-bot[bot]
2026-11-04 04:35:36 +00:00
parent 67628b48fe
commit c65d0656c9
8 changed files with 180 additions and 57 deletions
@@ -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<string, FileChange>();
@@ -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 {
@@ -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);
});
});
@@ -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))
@@ -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);
};
}
@@ -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);
@@ -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<string>() : 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);
}
}
+29
View File
@@ -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);
});
});
+20
View File
@@ -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<string>,
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;