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;