diff --git a/src/main/ipc/parcel-watcher-shallow-subscription.ts b/src/main/ipc/parcel-watcher-shallow-subscription.ts index 1918fda6a9d..13319281535 100644 --- a/src/main/ipc/parcel-watcher-shallow-subscription.ts +++ b/src/main/ipc/parcel-watcher-shallow-subscription.ts @@ -1,4 +1,4 @@ -import { watch, type FSWatcher } from 'node:fs' +import { statSync, watch, type FSWatcher } from 'node:fs' import { stat } from 'node:fs/promises' import { join } from 'node:path' import type { Event as ParcelWatcherEvent } from '@parcel/watcher' @@ -110,6 +110,15 @@ export function startShallowWatcher( } } + const directoryIdentitySync = (parent: string): string | null => { + try { + const entry = statSync(join(rootPath, parent)) + return entry.isDirectory() ? `${entry.dev}:${entry.ino}` : null + } catch { + return null + } + } + const directoryIdentity = async (parent: string): Promise => { try { const entry = await stat(join(rootPath, parent)) @@ -148,13 +157,14 @@ export function startShallowWatcher( rebindTimer.unref?.() for (const [parent, fileNames] of pathsByDirectory) { + // Why: read identity BEFORE binding. If the directory is replaced in the gap, + // the recorded identity is stale and the first sweep rebinds — the harmless + // direction. Reading after would pin the dead inode's watcher to the new + // identity, and the sweep would then never rebind it. + const identityBeforeBind = directoryIdentitySync(parent) watchDirectory(parent, fileNames) - if (watchers.has(parent)) { - void directoryIdentity(parent).then((identity) => { - if (!disposed && identity !== null) { - boundIdentities.set(parent, identity) - } - }) + if (watchers.has(parent) && identityBeforeBind !== null) { + boundIdentities.set(parent, identityBeforeBind) } } diff --git a/src/main/ipc/worktree-git-common-primary-watch.ts b/src/main/ipc/worktree-git-common-primary-watch.ts index 9f380c41b76..f853ce08b90 100644 --- a/src/main/ipc/worktree-git-common-primary-watch.ts +++ b/src/main/ipc/worktree-git-common-primary-watch.ts @@ -15,6 +15,14 @@ const PRIMARY_WATCH_OPTIONS = { include: PRIMARY_CHECKOUT_METADATA_FILES } +// Why: the shallow watcher can stop reporting without ever erroring — a lossy +// notification path (network mount, inotify queue overflow in the shared child), +// a dropped event batch, or a binding that went deaf between rebind sweeps. None +// of those raise, so the error-driven fallback never fires. A bounded re-stat is +// the only thing that turns "silently stale forever" into "stale for one tick". +// At 15 ticks this is ~0.2 stats/s/repo against the 3/s the old poll cost. +const PRIMARY_BACKSTOP_TICKS = 15 + function primaryMetadataEvents(commonDirPath: string): WorktreeBasePollEvent[] { return PRIMARY_CHECKOUT_METADATA_FILES.map((name) => ({ type: 'update', @@ -34,6 +42,7 @@ export async function startGitCommonPrimaryWatch( let disposed = false let watcher: WatcherProcessSubscription | null = null let statusRefPolling: WorktreeBaseSubscription | null = null + let backstopPolling: WorktreeBaseSubscription | null = null let fallback: WorktreeBaseSubscription | null = null const fallbackFlight = createSingleFlight() @@ -47,20 +56,26 @@ export async function startGitCommonPrimaryWatch( visibility, onFullScan ).then(async (nextFallback) => { - if (disposed || watcher || statusRefPolling) { + // `statusRefPolling` deliberately excluded: it covers only status refs, + // so its presence is not evidence that primary metadata is covered. + if (disposed || watcher) { await nextFallback.unsubscribe() return } + void stopWatcherSidePolling() fallback = nextFallback }) ) - const stopStatusRefPolling = async (): Promise => { + const stopWatcherSidePolling = async (): Promise => { const current = statusRefPolling + const currentBackstop = backstopPolling statusRefPolling = null - if (current) { - await current.unsubscribe().catch(() => {}) - } + backstopPolling = null + await Promise.all([ + current?.unsubscribe().catch(() => {}), + currentBackstop?.unsubscribe().catch(() => {}) + ]) } const handleWatcherError = (error: Error): void => { @@ -76,7 +91,7 @@ export async function startGitCommonPrimaryWatch( if (current) { void current.unsubscribe().catch(() => {}) } - void stopStatusRefPolling() + void stopWatcherSidePolling() .then(() => startFallback()) .catch(() => {}) } @@ -108,15 +123,41 @@ export async function startGitCommonPrimaryWatch( } ) if (watcher && !disposed && !fallbackFlight.pending()) { - statusRefPolling = await startGitCommonPrimaryPolling( - commonDirPath, - getStatusRefPaths, - onEvents, - pollIntervalMs, - visibility, - undefined, - false - ) + const [nextStatusRefPolling, nextBackstop] = await Promise.all([ + startGitCommonPrimaryPolling( + commonDirPath, + getStatusRefPaths, + onEvents, + pollIntervalMs, + visibility, + undefined, + false + ), + startGitCommonPrimaryPolling( + commonDirPath, + () => [], + onEvents, + pollIntervalMs * PRIMARY_BACKSTOP_TICKS, + visibility, + undefined, + true + ) + ]) + // Why: a terminal watch error can land while the two polls above are still + // starting. handleWatcherError already ran its teardown against nulls, so + // adopting these now would strand the repo with status-ref coverage only. + if (disposed || !watcher) { + await Promise.all([ + nextStatusRefPolling.unsubscribe().catch(() => {}), + nextBackstop.unsubscribe().catch(() => {}) + ]) + if (!disposed) { + await startFallback() + } + } else { + statusRefPolling = nextStatusRefPolling + backstopPolling = nextBackstop + } } } catch (error) { handleWatcherError(error instanceof Error ? error : new Error(String(error))) @@ -130,7 +171,7 @@ export async function startGitCommonPrimaryWatch( if (current) { await current.unsubscribe().catch(() => {}) } - await stopStatusRefPolling() + await stopWatcherSidePolling() await fallbackFlight.pending()?.catch(() => {}) if (fallback) { await fallback.unsubscribe().catch(() => {}) diff --git a/src/main/ipc/worktree-git-common-watch-reconciliation.ts b/src/main/ipc/worktree-git-common-watch-reconciliation.ts index fbf75a5e05b..662ea13a375 100644 --- a/src/main/ipc/worktree-git-common-watch-reconciliation.ts +++ b/src/main/ipc/worktree-git-common-watch-reconciliation.ts @@ -60,12 +60,22 @@ export function createGitCommonWatchReconciliation({ const rootWasReplaced = events.some((event) => event.type === 'delete' && event.path === worktreesDir) && events.some((event) => event.type === 'create' && event.path === worktreesDir) - const coarseRootReplacement = events.some( - (event) => - event.type === 'create' && - event.path !== worktreesDir && - dirname(event.path) === worktreesDir + // Why: this backstop lags the native stream by up to 15 ticks, so it + // routinely reports entry creates the stream already delivered. Only + // treat them as a replacement when the root itself was also recreated + // — otherwise every ordinary `git worktree add` would tear down a + // healthy stream and open a deaf window while it resubscribes. + const rootRecreated = events.some( + (event) => event.type === 'create' && event.path === worktreesDir ) + const coarseRootReplacement = + rootRecreated && + events.some( + (event) => + event.type === 'create' && + event.path !== worktreesDir && + dirname(event.path) === worktreesDir + ) if (rootWasReplaced || coarseRootReplacement) { onRootReplacement() } diff --git a/src/main/ipc/worktree-git-common-watch.test.ts b/src/main/ipc/worktree-git-common-watch.test.ts index abe94d0b8d2..650697b2008 100644 --- a/src/main/ipc/worktree-git-common-watch.test.ts +++ b/src/main/ipc/worktree-git-common-watch.test.ts @@ -1,5 +1,5 @@ import { afterEach, describe, expect, it, vi } from 'vitest' -import { appendFile, mkdtemp, mkdir, realpath, rm, writeFile } from 'node:fs/promises' +import { appendFile, mkdtemp, mkdir, realpath, rm, stat, writeFile } from 'node:fs/promises' import type * as NodeFsPromises from 'node:fs/promises' import { performance as nodePerformance } from 'node:perf_hooks' import { tmpdir } from 'node:os' @@ -52,15 +52,22 @@ async function replaceWorktreesRoot( worktreesDir: string, retainedEntry: string ): Promise { + const previousInode = (await stat(worktreesDir)).ino await rm(worktreesDir, { recursive: true }) - // Linux reuses just-deleted directory inodes. Park each freed inode in a - // sibling that is HELD for the rest of the test — releasing it would return it - // to the free list, so a later replacement could land back on it and look - // unchanged to reconciliation. The dirs are siblings of `worktrees`, which no - // poll or watch path enumerates. - const inodeReservation = join(commonDir, `worktrees-replacement-inode-${++inodeReservationCount}`) - await mkdir(inodeReservation) - await mkdir(retainedEntry, { recursive: true }) + // Linux reuses freed directory inodes, and removing the tree frees several at + // once, so the recreated root can land back on its own old inode and look + // unchanged to reconciliation. Park inodes in HELD siblings until the root + // genuinely differs — verifying beats guessing how many to reserve. The + // siblings live next to `worktrees`, which no poll or watch path enumerates. + for (let attempt = 0; attempt < 16; attempt++) { + await mkdir(retainedEntry, { recursive: true }) + if ((await stat(worktreesDir)).ino !== previousInode) { + return + } + await rm(worktreesDir, { recursive: true }) + await mkdir(join(commonDir, `worktrees-inode-hold-${++inodeReservationCount}`)) + } + throw new Error('could not obtain a fresh inode for the replaced worktrees root') } function createVisibilityHarness(): { @@ -233,7 +240,7 @@ describe('worktree git-common narrow watch (local native platforms)', () => { }) }) - it('leaves primary metadata stats to the shallow child on the healthy path', async () => { + it('keeps primary metadata off the per-tick path but re-stats it on the backstop', async () => { installSubscribeMock() const commonDir = await makeCommonDir(true) const headPath = join(commonDir, 'HEAD') @@ -242,14 +249,37 @@ describe('worktree git-common narrow watch (local native platforms)', () => { writeFile(headPath, 'ref: refs/heads/main'), writeFile(configPath, '[core]\n') ]) - statCalls.length = 0 const received: WorktreeBasePollEvent[][] = [] await startWatch(commonDir, received) - - expect(statCalls).not.toContain(headPath) - expect(statCalls).not.toContain(configPath) expect(primarySubscription().dir).toBe(commonDir) + + // The shallow child owns the fast path: ordinary ticks must not re-stat these. + statCalls.length = 0 + await vi + .waitFor( + () => { + expect(statCalls.length).toBeGreaterThan(0) + }, + { timeout: 2_000 } + ) + .catch(() => {}) + const duringOrdinaryTicks = statCalls.filter( + (path) => path === headPath || path === configPath + ).length + + // ...but a bounded backstop must still re-stat them, or a watcher that goes + // silently lossy would leave primary metadata stale forever. + statCalls.length = 0 + await vi.waitFor( + () => { + expect(statCalls).toContain(headPath) + expect(statCalls).toContain(configPath) + }, + { timeout: POLL_MS * RECONCILIATION_TICKS * 6 } + ) + const backstopSamples = statCalls.filter((path) => path === headPath).length + expect(duringOrdinaryTicks).toBeLessThan(backstopSamples * RECONCILIATION_TICKS) }) it('polls only the accepted upstream ref and rebinds without synthetic events', async () => { @@ -376,9 +406,12 @@ describe('worktree git-common narrow watch (local native platforms)', () => { ), [] ) + // Narrow fallback + selected-ref polling + primary backstop, with the narrow + // existence poll retired. Waiting on the exact count is what guarantees the + // fallback has baselined before the entry below is created. await vi.waitFor(() => { expect(narrowSubscription().unsubscribe).toHaveBeenCalledOnce() - expect(visibility.listenerCount()).toBe(3) + expect(visibility.listenerCount()).toBe(4) }) const entryPath = join(worktreesDir, 'fallback-entry') @@ -547,8 +580,9 @@ describe('worktree git-common narrow watch (local native platforms)', () => { visibility.source ) - // Narrow existence + selected-ref polling each park on window visibility. - expect(visibility.listenerCount()).toBe(2) + // Narrow existence, selected-ref polling, and the primary backstop each park + // on window visibility. + expect(visibility.listenerCount()).toBe(3) await watch.unsubscribe() expect(visibility.listenerCount()).toBe(0) })