From bc0e8737210881ad6841a0d15adeb179856250c9 Mon Sep 17 00:00:00 2001 From: thesiti92 Date: Sat, 10 Oct 2026 21:34:16 -0400 Subject: [PATCH 1/3] Add @parcel/watcher Prebuilt per platform; its install script only builds from source, so it stays off. --- packages/review/package.json | 1 + pnpm-lock.yaml | 163 +++++++++++++++++++++++++++++++++++ pnpm-workspace.yaml | 3 + 3 files changed, 167 insertions(+) diff --git a/packages/review/package.json b/packages/review/package.json index d61b6905f..d3dd5bc59 100644 --- a/packages/review/package.json +++ b/packages/review/package.json @@ -67,6 +67,7 @@ "@hono/node-server": "2.0.12", "@img/sharp-wasm32": "0.35.5", "@modelcontextprotocol/sdk": "1.31.0", + "@parcel/watcher": "catalog:", "commander": "14.0.3", "fuzzysort": "4.0.2", "hono": "4.13.7", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index b26488872..007d21c94 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -6,6 +6,9 @@ settings: catalogs: default: + '@parcel/watcher': + specifier: 2.6.0 + version: 2.6.0 zod: specifier: 4.6.5 version: 4.6.5 @@ -247,6 +250,9 @@ importers: '@modelcontextprotocol/sdk': specifier: 1.31.0 version: 1.31.0(zod@4.6.5) + '@parcel/watcher': + specifier: 'catalog:' + version: 2.6.0 commander: specifier: 14.0.3 version: 14.0.3 @@ -1256,6 +1262,88 @@ packages: resolution: {integrity: sha512-vZAzaUQkwgdN62RHgPFzfCsiBI6SDJMUdUlBGpJK0V++UHCMUk7UeJseygOhE/wOUSgV3ccE4ORkgab3C3MC6g==} engines: {node: ^12.22.0 || ^14.17.0 || >=16.0.0} + '@parcel/watcher-android-arm64@2.6.0': + resolution: {integrity: sha512-trgpLSCKRC/huFjXX/Smh+0sWe4+YtKfktIToiMl59ghz7z+qkH6kMvNnUbLyRs9N11t8l4svSCs1+5B3rOAhA==} + engines: {node: '>= 10.0.0'} + cpu: [arm64] + os: [android] + + '@parcel/watcher-darwin-arm64@2.6.0': + resolution: {integrity: sha512-Y3QV0gl7Q1zbfueunkWIERICbEojQFCgpyG7YqOGNFLsckXyI1xu9mAIUpKY9QBYzBtSkN8dBPwd3yiAO9ovMw==} + engines: {node: '>= 10.0.0'} + cpu: [arm64] + os: [darwin] + + '@parcel/watcher-darwin-x64@2.6.0': + resolution: {integrity: sha512-Ohv6OpzhUfKYD7Beb8kDvG0jbIxORCYY1JRdZnaBtnjjkJxgD7ZVL0nw2sCYd0yTMKTvz3nnTnOF3cDifK+kvw==} + engines: {node: '>= 10.0.0'} + cpu: [x64] + os: [darwin] + + '@parcel/watcher-freebsd-x64@2.6.0': + resolution: {integrity: sha512-5HmXvDgs8VK+74jF9y9/2FE3/OnlcKmc56tjmSrEuZjpSZOGL+fvAu+HKJBdPs9uwoP2hE6TlSUpXZ/C5jUFmQ==} + engines: {node: '>= 10.0.0'} + cpu: [x64] + os: [freebsd] + + '@parcel/watcher-linux-arm-glibc@2.6.0': + resolution: {integrity: sha512-Ps/hui3A+vMbjdqlqAowK2ZL8+BO8dBjxeWXj6npTBs3jx4wWmbPpaLuqwrQrSqIVMCnpWo238bJ1U37GhQOYg==} + engines: {node: '>= 10.0.0'} + cpu: [arm] + os: [linux] + libc: [glibc] + + '@parcel/watcher-linux-arm-musl@2.6.0': + resolution: {integrity: sha512-9c6AUHgHoG+IY88MRIHupztQiQnrbqHYQjkM2btA+Bf/wQnQMuiD0Wfk1EVv3TlNT3x41uU71rn6E4xh/+zvkw==} + engines: {node: '>= 10.0.0'} + cpu: [arm] + os: [linux] + libc: [musl] + + '@parcel/watcher-linux-arm64-glibc@2.6.0': + resolution: {integrity: sha512-yHRqS2owEXe6Hic9z6Mh1ECsCd+ODVOGvZDyciqRd21+v+o+DnXMOrw50DSpIG2sb8GPEaPPmfeCAWKPJdq46g==} + engines: {node: '>= 10.0.0'} + cpu: [arm64] + os: [linux] + libc: [glibc] + + '@parcel/watcher-linux-arm64-musl@2.6.0': + resolution: {integrity: sha512-WhB2e/V7rqdHHWZusBSPuy5Ei8S6lSz6FE5TKKQz5h3a0O+C+mhY7vxU9b/stqvMb8beLnPY82ZrFTLKs+SrKA==} + engines: {node: '>= 10.0.0'} + cpu: [arm64] + os: [linux] + libc: [musl] + + '@parcel/watcher-linux-x64-glibc@2.6.0': + resolution: {integrity: sha512-ulGE6x6Oz6iAwg75T8YQSoguBWasniIbX+QWpaYPcCnDOpdWX3k+4xbEYPZVLxOuoJI+svJJPD3sEj8G7lrQ3A==} + engines: {node: '>= 10.0.0'} + cpu: [x64] + os: [linux] + libc: [glibc] + + '@parcel/watcher-linux-x64-musl@2.6.0': + resolution: {integrity: sha512-tkBYKt7YQrjIJWYDnto2YgO8MRkjlMTSNoRHzsXinBqbLdeOM3L32wPZJvIZxqaLMfSlS/4sUjH/6STVP/XDLw==} + engines: {node: '>= 10.0.0'} + cpu: [x64] + os: [linux] + libc: [musl] + + '@parcel/watcher-win32-arm64@2.6.0': + resolution: {integrity: sha512-gIZAP23jaHjGWasY/TY6yL7NHFClf0Ga7FN+iINvk+KN94rhm94lYZhFsbYFNcA04/onvGD9kKmiJLJB2HbNwQ==} + engines: {node: '>= 10.0.0'} + cpu: [arm64] + os: [win32] + + '@parcel/watcher-win32-x64@2.6.0': + resolution: {integrity: sha512-cA+/pXV2YkfxlIcXOQ5fSWqAzzPyD78/x5qbK/I0vUkrlYHA8TIz+MXjAbGouguKVSI4bOmkTSJ1/poVSsgt+A==} + engines: {node: '>= 10.0.0'} + cpu: [x64] + os: [win32] + + '@parcel/watcher@2.6.0': + resolution: {integrity: sha512-7FNeNl8NCE7aINx7WXiKQrPYZWC/hvrTsmk6zmxbI7LTXE7hVek/n8AfVgpe2y82zl3w0HvCHN0bVKMBoJcC0w==} + engines: {node: '>= 10.0.0'} + '@pinojs/redact@0.4.0': resolution: {integrity: sha512-k2ENnmBugE/rzQfEcdWHcCY+/FM3VLzH9cYEsbdsoqrvzAKRhUZeRNhAZvB8OitQJ1TBed3yqWtdjzS6wJKBwg==} @@ -2383,6 +2471,14 @@ packages: engines: {node: ^12.20.0 || ^14.13.1 || >=16.0.0} hasBin: true + is-extglob@2.1.1: + resolution: {integrity: sha512-SbKbANkN603Vi4jEZv49LeVJMn4yGwsbzZworEoyEiutsN3nJYdbO36zfhGJ6QEDpOZIFkDtnq5JRxmvl3jsoQ==} + engines: {node: '>=0.10.0'} + + is-glob@4.0.3: + resolution: {integrity: sha512-xelSayHH36ZgE7ZWhli7pW34hNbNl8Ojv5KVmkJD4hBdD3th8Tfk9vYasLM+mXWOZhFkgZfxhLSnrwRr4elSSg==} + engines: {node: '>=0.10.0'} + is-in-ssh@1.0.0: resolution: {integrity: sha512-jYa6Q9rH90kR1vKB6NM7qqd1mge3Fx4Dhw5TVlK1MUBqhEOuCagrEHMevNuCcbECmXZ0ThXkRm+Ymr51HwEPAw==} engines: {node: '>=20'} @@ -2718,6 +2814,9 @@ packages: resolution: {integrity: sha512-NMPBRMJgiQHjbd8phG3Vebdx4kZ1H121rbl5IkMqeOsahptB9BKo/d7oJ3zTXqTgagn2bWlNSXkh0QUGM31RYg==} engines: {node: '>=18'} + node-addon-api@7.1.1: + resolution: {integrity: sha512-5m3bsyrjFWE1xf7nz7YXdN4udnVtXK6/Yfgn5qnahL6bCkf2yKt4k3nuTKAtT4r3IG8JNR2ncsIMdZuAzJjHQQ==} + node-releases@2.0.56: resolution: {integrity: sha512-x0InOIyzgdk+eyaWaRJFH5snEtiImgBgblZ2CyPrLmqqcuMQkEvcDPHbzqbD8eDsSeJbVOjn+crzyzHaM4D+/A==} engines: {node: '>=18'} @@ -4058,6 +4157,62 @@ snapshots: '@oxlint/plugins@1.63.0': {} + '@parcel/watcher-android-arm64@2.6.0': + optional: true + + '@parcel/watcher-darwin-arm64@2.6.0': + optional: true + + '@parcel/watcher-darwin-x64@2.6.0': + optional: true + + '@parcel/watcher-freebsd-x64@2.6.0': + optional: true + + '@parcel/watcher-linux-arm-glibc@2.6.0': + optional: true + + '@parcel/watcher-linux-arm-musl@2.6.0': + optional: true + + '@parcel/watcher-linux-arm64-glibc@2.6.0': + optional: true + + '@parcel/watcher-linux-arm64-musl@2.6.0': + optional: true + + '@parcel/watcher-linux-x64-glibc@2.6.0': + optional: true + + '@parcel/watcher-linux-x64-musl@2.6.0': + optional: true + + '@parcel/watcher-win32-arm64@2.6.0': + optional: true + + '@parcel/watcher-win32-x64@2.6.0': + optional: true + + '@parcel/watcher@2.6.0': + dependencies: + detect-libc: 2.1.2 + is-glob: 4.0.3 + node-addon-api: 7.1.1 + picomatch: 4.0.7 + optionalDependencies: + '@parcel/watcher-android-arm64': 2.6.0 + '@parcel/watcher-darwin-arm64': 2.6.0 + '@parcel/watcher-darwin-x64': 2.6.0 + '@parcel/watcher-freebsd-x64': 2.6.0 + '@parcel/watcher-linux-arm-glibc': 2.6.0 + '@parcel/watcher-linux-arm-musl': 2.6.0 + '@parcel/watcher-linux-arm64-glibc': 2.6.0 + '@parcel/watcher-linux-arm64-musl': 2.6.0 + '@parcel/watcher-linux-x64-glibc': 2.6.0 + '@parcel/watcher-linux-x64-musl': 2.6.0 + '@parcel/watcher-win32-arm64': 2.6.0 + '@parcel/watcher-win32-x64': 2.6.0 + '@pinojs/redact@0.4.0': {} '@polka/url@1.0.0-next.29': {} @@ -4982,6 +5137,12 @@ snapshots: is-docker@3.0.0: {} + is-extglob@2.1.1: {} + + is-glob@4.0.3: + dependencies: + is-extglob: 2.1.1 + is-in-ssh@1.0.0: {} is-inside-container@1.0.0: @@ -5462,6 +5623,8 @@ snapshots: dependencies: content-type: 2.1.0 + node-addon-api@7.1.1: {} + node-releases@2.0.56: {} object-assign@4.1.1: {} diff --git a/pnpm-workspace.yaml b/pnpm-workspace.yaml index 4d79af1db..4e7940e34 100644 --- a/pnpm-workspace.yaml +++ b/pnpm-workspace.yaml @@ -9,6 +9,7 @@ packages: - "apps/*" catalog: + "@parcel/watcher": "2.6.0" zod: "4.6.5" catalogMode: strict @@ -62,3 +63,5 @@ ignoredOptionalDependencies: allowBuilds: esbuild: true + # Ships prebuilt binaries; its install script only builds from source. + "@parcel/watcher": false From 7270c289887d9a9cf812a9acdadc6baf2b30ba6f Mon Sep 17 00:00:00 2001 From: thesiti92 Date: Sat, 10 Oct 2026 21:34:16 -0400 Subject: [PATCH 2/3] Watch worktrees with parcel On Linux, recursive fs.watch adds an inotify watch per file and per directory, including every managed review checkout under .git/dev-fast. Parcel watches directories only. The checkout watch skips .git, and the git directory watch skips objects, logs, lfs and dev-fast. --- .../review/src/review-api/local-data.test.ts | 42 ++++---- packages/review/src/review-api/local-data.ts | 100 +++++++++++------- .../review/src/review-api/worktree-events.ts | 3 + 3 files changed, 86 insertions(+), 59 deletions(-) diff --git a/packages/review/src/review-api/local-data.test.ts b/packages/review/src/review-api/local-data.test.ts index dc23e1d71..ffb50f28e 100644 --- a/packages/review/src/review-api/local-data.test.ts +++ b/packages/review/src/review-api/local-data.test.ts @@ -1,7 +1,6 @@ import { execFileSync, spawnSync } from "node:child_process"; import { randomUUID } from "node:crypto"; import { - type FSWatcher, appendFileSync, existsSync, mkdirSync, @@ -11,7 +10,6 @@ import { renameSync, rmSync, symlinkSync, - watch, writeFileSync, } from "node:fs"; import { tmpdir } from "node:os"; @@ -24,6 +22,7 @@ import { REVIEW_CLIENT_HEADER, REVIEW_CLIENT_REMOTE, } from "@dev.fast/review-protocol"; +import { type SubscribeCallback, subscribe } from "@parcel/watcher"; import { rangeAnchor } from "@review/lens-selection"; import { createGlobalReviewServer } from "@review/server/desktop-server.js"; import { Hono } from "hono"; @@ -60,6 +59,9 @@ const git = (...args: string[]) => const spawns: string[][] = []; +/** Parcel delivers events late, in batches. */ +const watcherLatency = () => new Promise((resolve) => setTimeout(resolve, 200)); + const recordSpawns = () => { spawns.length = 0; setLocalVcsCommandObserver({ @@ -2658,7 +2660,7 @@ it("leaves other checkouts alone while a nested worktree works", async () => { writeFileSync(path.join(sibling, "node_modules/built.js"), "built\n"); writeFileSync(path.join(sibling, source.file), "export const edited = 1;\n"); execFileSync("git", ["add", source.file], { cwd: sibling }); - await new Promise((resolve) => setTimeout(resolve, 100)); + await watcherLatency(); await local.store.refreshWorktrees(); expect(local.store.read(siblingReview).pins!.worktreeRevision).not.toBe( @@ -2691,14 +2693,14 @@ it("follows its base branch when only the ref moves", async () => { writeFileSync(path.join(linked, "linked.ts"), "export {};\n"); linkedGit("add", "."); linkedGit("-c", "commit.gpgsign=false", "commit", "-qm", "Linked"); - await new Promise((resolve) => setTimeout(resolve, 100)); + await watcherLatency(); await local.store.refreshWorktrees(); const head = linkedGit("rev-parse", "HEAD"); expect(local.store.read(reviewId).pins!.base).not.toBe(head); // Nothing in the linked checkout changes: only main moves. git("update-ref", "refs/heads/main", head); - await new Promise((resolve) => setTimeout(resolve, 100)); + await watcherLatency(); await local.store.refreshWorktrees(); expect(local.store.read(reviewId).pins!.base).toBe(head); @@ -2743,7 +2745,7 @@ it("reads current working source across authored versions, commits and retargeti path.join(repository, "example.ts"), "export const live = 2;\n", ); - await new Promise((resolve) => setTimeout(resolve, 50)); + await watcherLatency(); await local.store.refreshWorktrees(); const current = local.store.read(result.reviewId); expect( @@ -2762,7 +2764,7 @@ it("reads current working source across authored versions, commits and retargeti expect(git("diff", "--cached")).toBe(beforeIndex); git("add", "."); git("commit", "-qm", "Save changes"); - await new Promise((resolve) => setTimeout(resolve, 50)); + await watcherLatency(); await local.store.refreshWorktrees(); expect( await local.data.changes(local.store.read(result.reviewId).pins!), @@ -2857,7 +2859,7 @@ describe("worktree base", () => { const forkPoint = local.store.read(reviewId).pins!.base; const refresh = async () => { - await new Promise((resolve) => setTimeout(resolve, 50)); + await watcherLatency(); await local.store.refreshWorktrees(); return local.store.read(reviewId).pins!.base; @@ -2950,7 +2952,7 @@ it("reads symlink text and an unborn repository without following external links "-qm", "First", ]); - await new Promise((resolve) => setTimeout(resolve, 50)); + await watcherLatency(); await local.store.refreshWorktrees(); expect(local.store.read(result.reviewId).pins!.head).not.toBe( snapshot.pins!.head, @@ -2997,7 +2999,7 @@ it("keeps authored coordinates fixed as live source changes and warns only on un path.join(repository, "range.ts"), "// inserted\nconst first = 1;\nconst second = 2;\nconst third = 3;\n", ); - await new Promise((resolve) => setTimeout(resolve, 50)); + await watcherLatency(); await local.store.refreshWorktrees(); expect(local.store.read(result.reviewId).document[0]).toMatchObject({ source: "head/range.ts#L2", @@ -3334,7 +3336,7 @@ it("recovers persisted stale source and clears flags on set_target", async () => ).rejects.toThrow("exceeds the pinned file"); writeFileSync(path.join(repository, "recover.ts"), original); git("add", "-N", "recover.ts"); - await new Promise((resolve) => setTimeout(resolve, 50)); + await watcherLatency(); await local.store.refreshWorktrees(); expect(local.store.read(created.reviewId).staleSources).toEqual([]); expect(local.store.read(created.reviewId).document[0]).toMatchObject({ @@ -3429,14 +3431,13 @@ it("does not report clean tracked symlinks and submodules as modified", async () it("continues capturing after watchers fail and stop emitting changes", async () => { await local.data.close(); await local.store.close(); - const watchers: FSWatcher[] = []; + const callbacks: SubscribeCallback[] = []; local = openLocalReviewStore(database, { - watch: ((...args: Parameters) => { - const watcher = watch(...args); - watchers.push(watcher); + subscribe: (dir, callback, options) => { + callbacks.push(callback); - return watcher; - }) as typeof watch, + return subscribe(dir, callback, options); + }, }); const created = await local.store.execute( @@ -3451,11 +3452,8 @@ it("continues capturing after watchers fail and stop emitting changes", async () }), ); - for (const watcher of watchers) - watcher.emit( - "error", - Object.assign(new Error("Watch limit"), { code: "ENOSPC" }), - ); + for (const callback of callbacks) + callback(new Error("No space left on device"), []); for (const value of ["first change\n", "second change\n"]) { writeFileSync(path.join(repository, "example.ts"), value); diff --git a/packages/review/src/review-api/local-data.ts b/packages/review/src/review-api/local-data.ts index a9665b34b..df1e0d42c 100644 --- a/packages/review/src/review-api/local-data.ts +++ b/packages/review/src/review-api/local-data.ts @@ -1,5 +1,5 @@ import { execFile } from "node:child_process"; -import { type FSWatcher, existsSync, watch } from "node:fs"; +import { existsSync } from "node:fs"; import { mkdir, readFile, realpath, stat, writeFile } from "node:fs/promises"; import path from "node:path"; import { promisify } from "node:util"; @@ -30,6 +30,7 @@ import type { StructuralDiffEvent, } from "@dev.fast/review-protocol"; import { writePrivateJsonAtomic } from "@dev.fast/trace-core"; +import type { AsyncSubscription, subscribe } from "@parcel/watcher"; import { textIncludesQuote } from "@review/evidence.js"; import { isMissingFileError } from "@review/fs-utils.js"; import { reviewManagedCheckoutRoot } from "@review/review-checkout-paths.js"; @@ -88,6 +89,7 @@ import { import { traceSchema } from "./trace-schema.js"; import { ReviewWorkspaces } from "./workspaces.js"; import { + INERT_GIT_PATHS, gitDirectoryPrefix, gitEventMatters, rootEventMatters, @@ -569,7 +571,7 @@ export class LocalReviewData { blobReaderIdleTimeoutMs?: number; workspaceDatabase?: string; manageWorkspaces?: boolean; - watch?: typeof watch; + subscribe?: typeof subscribe; /** gh, git and GitHub API access for pull request targets. */ pullRequests?: PullRequestDeps; } = {}, @@ -640,7 +642,7 @@ export class LocalReviewData { { epoch: number; inspectedEpoch: number; - watchers: FSWatcher[]; + subscriptions: Map; healthy: boolean; inspection?: Omit>, "files">; tracked?: Set; @@ -652,7 +654,7 @@ export class LocalReviewData { private forgetWorktree(repositoryId: string) { const entry = this.worktrees.get(repositoryId); - if (entry) for (const watcher of entry.watchers) watcher.close(); + if (entry) void unwatch(entry); this.worktrees.delete(repositoryId); } @@ -663,12 +665,13 @@ export class LocalReviewData { entry = { epoch: 0, inspectedEpoch: -1, - watchers: [], + subscriptions: new Map(), healthy: true, forks: new Map(), }; this.worktrees.set(repositoryId, entry); const state = entry; + const current = () => this.worktrees.get(repositoryId) === state; const common = await gitCommonDir(vcs.rootPath); // jj snapshots untracked files, so every event can matter there. @@ -677,49 +680,60 @@ export class LocalReviewData { ? await gitDirectoryPrefix(vcs.rootPath, common) : null; - const roots = new Map([ - [ - vcs.rootPath, - (name: string) => - prefix === null || rootEventMatters(name, state.tracked), - ], - ]); - - if (common) - roots.set( - common, - (name) => prefix === null || gitEventMatters(name, prefix), - ); - - for (const [root, matters] of roots) { + // Linux watches each directory, so skip what cannot matter. + const watch = async ( + root: string, + ignore: string[], + matters: (name: string) => boolean, + ) => { try { - const watcher = (this.options.watch ?? watch)( + const subscription = await ( + this.options.subscribe ?? + (await import("@parcel/watcher")).subscribe + )( root, - { recursive: true }, - (_event, name) => { - if (name === null || matters(String(name))) state.epoch++; + (error, events) => { + if (error) { + state.healthy = false; + const failed = state.subscriptions.get(root); + + if (failed) void release(failed); + state.subscriptions.delete(root); + } else if ( + events.some((event) => matters(path.relative(root, event.path))) + ) + state.epoch++; }, + { ignore }, ); - watcher.on("error", () => { - state.epoch++; - state.inspectedEpoch = -1; - state.healthy = false; - watcher.close(); - }); - watcher.unref(); - state.watchers.push(watcher); + if (current()) state.subscriptions.set(root, subscription); + else void release(subscription); } catch { state.healthy = false; } - } + }; + + await Promise.all([ + watch( + vcs.rootPath, + common ? [path.join(vcs.rootPath, ".git")] : [], + (name) => prefix === null || rootEventMatters(name, state.tracked), + ), + common && + watch( + common, + INERT_GIT_PATHS.map((directory) => path.join(common, directory)), + (name) => prefix === null || gitEventMatters(name, prefix), + ), + ]); } if ( entry.inspection && entry.inspectedEpoch === entry.epoch && entry.healthy && - entry.watchers.length + entry.subscriptions.size ) return { ...entry.inspection, forks: entry.forks }; @@ -742,8 +756,7 @@ export class LocalReviewData { this.closed = true; await this.workspaceManager?.close(); - for (const entry of this.worktrees.values()) - for (const watcher of entry.watchers) watcher.close(); + await Promise.all([...this.worktrees.values()].map(unwatch)); this.worktrees.clear(); const readers = [...this.readers.values()]; @@ -1809,12 +1822,25 @@ function inputError(run: () => T): T { } } +async function unwatch(entry: { + subscriptions: Map; +}) { + const subscriptions = [...entry.subscriptions.values()]; + + entry.subscriptions.clear(); + await Promise.all(subscriptions.map(release)); +} + +/** Linux fails to unwatch a directory that is already gone. */ +const release = (subscription: AsyncSubscription) => + subscription.unsubscribe().catch(() => {}); + export function openLocalReviewStore( databasePath: string, options: { blobReaderIdleTimeoutMs?: number; manageWorkspaces?: boolean; - watch?: typeof watch; + subscribe?: typeof subscribe; pullRequests?: PullRequestDeps; } = {}, ) { diff --git a/packages/review/src/review-api/worktree-events.ts b/packages/review/src/review-api/worktree-events.ts index bc6c55fa1..80f15d0ea 100644 --- a/packages/review/src/review-api/worktree-events.ts +++ b/packages/review/src/review-api/worktree-events.ts @@ -41,6 +41,9 @@ export function rootEventMatters( const CHECKOUT_STATE = /^(?:index|[A-Z_]+|(?:rebase-merge|rebase-apply|sequencer|refs\/(?:bisect|worktree|rewritten))(?:\/.*)?)$/; +/** Objects, reflogs, LFS objects and Review's own checkouts; never watched. */ +export const INERT_GIT_PATHS = ["objects", "logs", "lfs", "dev-fast"]; + /** Objects, reflogs, locks and other checkouts' state cannot change this one. */ export function gitEventMatters(name: string, prefix: string): boolean { const file = normalize(name); From f9a369e7f15f678ecdf9f134300c5f58e01f4074 Mon Sep 17 00:00:00 2001 From: thesiti92 Date: Sat, 10 Oct 2026 21:34:17 -0400 Subject: [PATCH 3/3] Skip gitignored directories when watching The checkout watch ignores what git reports as ignored, such as node_modules. A new directory inside a tracked one, or a .gitignore or info/exclude change, rereads that list and resubscribes. Fixes #1073. --- .../review/src/review-api/local-data.test.ts | 54 ++++++++++++ packages/review/src/review-api/local-data.ts | 87 ++++++++++++++++--- .../review/src/review-api/worktree-events.ts | 43 +++++++++ 3 files changed, 172 insertions(+), 12 deletions(-) diff --git a/packages/review/src/review-api/local-data.test.ts b/packages/review/src/review-api/local-data.test.ts index ffb50f28e..024aa55ff 100644 --- a/packages/review/src/review-api/local-data.test.ts +++ b/packages/review/src/review-api/local-data.test.ts @@ -6,6 +6,8 @@ import { mkdirSync, mkdtempSync, readFileSync, + readdirSync, + readlinkSync, realpathSync, renameSync, rmSync, @@ -2673,6 +2675,58 @@ it("leaves other checkouts alone while a nested worktree works", async () => { ).toEqual([]); }); +const inotifyWatches = () => + readdirSync("/proc/self/fd").reduce((count, fd) => { + try { + if (readlinkSync(`/proc/self/fd/${fd}`) !== "anon_inode:inotify") + return count; + + return ( + count + + readFileSync(`/proc/self/fdinfo/${fd}`, "utf8") + .split("\n") + .filter((line) => line.startsWith("inotify wd")).length + ); + } catch { + return count; + } + }, 0); + +it.runIf(process.platform === "linux")( + "watches only tracked directories on Linux", + async () => { + const tree = (root: string) => { + for (let i = 0; i < 200; i++) + mkdirSync(path.join(repository, root, `${i}`, "nested"), { + recursive: true, + }); + }; + + writeFileSync( + path.join(repository, ".gitignore"), + "node_modules/\ntarget/\n", + ); + tree("node_modules"); + tree(".git/dev-fast/reviews/other"); + const before = inotifyWatches(); + + await local.store.execute( + command({ + type: "create", + title: "Main", + target: { kind: "worktree", repositoryId: pins.repositoryId }, + }), + ); + await local.store.refreshWorktrees(); + expect(inotifyWatches() - before).toBeLessThan(50); + + tree("target"); + await vi.waitFor(() => expect(inotifyWatches() - before).toBeLessThan(50), { + timeout: 5000, + }); + }, +); + it("follows its base branch when only the ref moves", async () => { const linked = path.join(directory, "linked"); git("worktree", "add", "-q", "-b", "linked", linked); diff --git a/packages/review/src/review-api/local-data.ts b/packages/review/src/review-api/local-data.ts index df1e0d42c..3e1914d49 100644 --- a/packages/review/src/review-api/local-data.ts +++ b/packages/review/src/review-api/local-data.ts @@ -1,5 +1,5 @@ import { execFile } from "node:child_process"; -import { existsSync } from "node:fs"; +import { existsSync, statSync } from "node:fs"; import { mkdir, readFile, realpath, stat, writeFile } from "node:fs/promises"; import path from "node:path"; import { promisify } from "node:util"; @@ -30,7 +30,11 @@ import type { StructuralDiffEvent, } from "@dev.fast/review-protocol"; import { writePrivateJsonAtomic } from "@dev.fast/trace-core"; -import type { AsyncSubscription, subscribe } from "@parcel/watcher"; +import type { + AsyncSubscription, + Event as WatcherEvent, + subscribe, +} from "@parcel/watcher"; import { textIncludesQuote } from "@review/evidence.js"; import { isMissingFileError } from "@review/fs-utils.js"; import { reviewManagedCheckoutRoot } from "@review/review-checkout-paths.js"; @@ -90,8 +94,10 @@ import { traceSchema } from "./trace-schema.js"; import { ReviewWorkspaces } from "./workspaces.js"; import { INERT_GIT_PATHS, + atTrackedEdge, gitDirectoryPrefix, gitEventMatters, + ignoredDirectories, rootEventMatters, trackedDirectories, } from "./worktree-events.js"; @@ -646,6 +652,8 @@ export class LocalReviewData { healthy: boolean; inspection?: Omit>, "files">; tracked?: Set; + /** Rereads ignored directories after one may have appeared. */ + ignoring?: NodeJS.Timeout; /** Fork points by base ref, valid for the inspected epoch. */ forks: Map>; } @@ -684,7 +692,7 @@ export class LocalReviewData { const watch = async ( root: string, ignore: string[], - matters: (name: string) => boolean, + matters: (name: string, event: WatcherEvent) => boolean, ) => { try { const subscription = await ( @@ -700,31 +708,84 @@ export class LocalReviewData { if (failed) void release(failed); state.subscriptions.delete(root); } else if ( - events.some((event) => matters(path.relative(root, event.path))) + // Every event, since matters also notices new ignored directories. + events + .map((event) => + matters(path.relative(root, event.path), event), + ) + .includes(true) ) state.epoch++; }, { ignore }, ); - if (current()) state.subscriptions.set(root, subscription); - else void release(subscription); + if (!current()) return void release(subscription); + const previous = state.subscriptions.get(root); + + state.subscriptions.set(root, subscription); + + if (previous) void release(previous); } catch { state.healthy = false; } }; + const rootIgnore = async () => + [ + ...(common ? [".git"] : []), + ...(await ignoredDirectories( + vcs.rootPath, + vcs.kind === "jj" ? (common ?? undefined) : undefined, + )), + ].map((directory) => path.join(vcs.rootPath, directory)); + + let ignored = await rootIgnore(); + + const refreshIgnored = () => { + clearTimeout(state.ignoring); + state.ignoring = setTimeout(async () => { + const next = await rootIgnore(); + + if (!current() || next.join("\0") === ignored.join("\0")) return; + ignored = next; + + // Parcel keeps watches an overlapping subscription shares, so + // unsubscribe first and treat the gap as a change. + const previous = state.subscriptions.get(vcs.rootPath); + + state.subscriptions.delete(vcs.rootPath); + + if (previous) await release(previous); + await watch(vcs.rootPath, ignored, rootMatters); + state.epoch++; + }, 1000); + state.ignoring.unref(); + }; + + const rootMatters = (name: string, event: WatcherEvent) => { + if ( + path.basename(name) === ".gitignore" || + (event.type === "create" && + atTrackedEdge(name, state.tracked) && + statSync(event.path, { throwIfNoEntry: false })?.isDirectory()) + ) + refreshIgnored(); + + return prefix === null || rootEventMatters(name, state.tracked); + }; + await Promise.all([ - watch( - vcs.rootPath, - common ? [path.join(vcs.rootPath, ".git")] : [], - (name) => prefix === null || rootEventMatters(name, state.tracked), - ), + watch(vcs.rootPath, ignored, rootMatters), common && watch( common, INERT_GIT_PATHS.map((directory) => path.join(common, directory)), - (name) => prefix === null || gitEventMatters(name, prefix), + (name) => { + if (name === path.join("info", "exclude")) refreshIgnored(); + + return prefix === null || gitEventMatters(name, prefix); + }, ), ]); } @@ -1824,7 +1885,9 @@ function inputError(run: () => T): T { async function unwatch(entry: { subscriptions: Map; + ignoring?: NodeJS.Timeout; }) { + clearTimeout(entry.ignoring); const subscriptions = [...entry.subscriptions.values()]; entry.subscriptions.clear(); diff --git a/packages/review/src/review-api/worktree-events.ts b/packages/review/src/review-api/worktree-events.ts index 80f15d0ea..09de21d18 100644 --- a/packages/review/src/review-api/worktree-events.ts +++ b/packages/review/src/review-api/worktree-events.ts @@ -37,6 +37,49 @@ export function rootEventMatters( return !tracked || tracked.has(parent(file)); } +/** A directory appearing at the edge of the tracked tree may be newly ignored. */ +export function atTrackedEdge( + name: string, + tracked: Set | undefined, +): boolean { + const file = normalize(name); + + return !!tracked && tracked.has(parent(file)) && !tracked.has(file); +} + +/** Wholly ignored directories, so watchers can skip them. */ +export async function ignoredDirectories( + rootPath: string, + gitDirectory?: string, +): Promise { + try { + const { stdout } = await exec( + "git", + [ + "-C", + rootPath, + ...(gitDirectory + ? ["--git-dir", gitDirectory, "--work-tree", rootPath] + : []), + "ls-files", + "-z", + "--others", + "--ignored", + "--exclude-standard", + "--directory", + ], + { maxBuffer: 32 * 1024 * 1024 }, + ); + + return stdout + .split("\0") + .filter((entry) => entry.endsWith("/") && entry !== ".jj/") + .map((entry) => entry.slice(0, -1)); + } catch { + return []; + } +} + // A checkout's own state, per gitrepository-layout(5). const CHECKOUT_STATE = /^(?:index|[A-Z_]+|(?:rebase-merge|rebase-apply|sequencer|refs\/(?:bisect|worktree|rewritten))(?:\/.*)?)$/;