From abaf3591baf735cfb11f6799ad58a12d1db5c2a5 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Wed, 22 Jul 2026 16:16:49 -0700 Subject: [PATCH] fix(main): don't fabricate worktree deletions on a transient git-common readdir failure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to always-enumerating the worktrees dir: the readdir catch-all treated ANY error as an authoritative empty listing, so a transient failure (EIO/ESTALE/EMFILE, network/SSH hiccup) emitted a false delete for every linked worktree (and a false create next tick) — and enumerating every tick widened that exposure. Only ENOENT (dir truly absent) now yields an empty listing; other errors retain the known entries so per-entry stats still run and a real removal surfaces as that entry's own stat miss. Adds a regression test (readdir → ENOTDIR) asserting no false delete. Co-authored-by: Orca --- src/main/ipc/worktree-git-common-polling.ts | 14 +++++++--- .../ipc/worktree-git-common-watch.test.ts | 26 +++++++++++++++++++ 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/src/main/ipc/worktree-git-common-polling.ts b/src/main/ipc/worktree-git-common-polling.ts index c3414430ca8..ed0043b3c5b 100644 --- a/src/main/ipc/worktree-git-common-polling.ts +++ b/src/main/ipc/worktree-git-common-polling.ts @@ -143,9 +143,17 @@ async function snapshotGitCommon( entryPaths = entries .filter((entry) => entry.isDirectory()) .map((entry) => join(worktreesDir, entry.name)) - } catch { - // Missing worktrees dir is normal for repos without linked worktrees. - entryPaths = [] + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ENOENT') { + // Dir genuinely absent (no linked worktrees, or all removed) → authoritative empty listing. + entryPaths = [] + } else { + // Why: a TRANSIENT readdir failure (EIO/ESTALE/EMFILE, network/SSH hiccup) must not masquerade as + // "every worktree removed" — that would emit false delete events (and false creates next tick). + // Reuse the known entries so per-entry stats still run; a real removal surfaces as that entry's own + // stat miss (handled in snapshotGitCommonEntry), and the next successful readdir catches any add. + entryPaths = previous ? [...previous.entries.keys()] : [] + } } const entries = new Map() diff --git a/src/main/ipc/worktree-git-common-watch.test.ts b/src/main/ipc/worktree-git-common-watch.test.ts index f37fe816827..e4c508e45ff 100644 --- a/src/main/ipc/worktree-git-common-watch.test.ts +++ b/src/main/ipc/worktree-git-common-watch.test.ts @@ -337,6 +337,10 @@ describe('worktree git-common polling gate (non-darwin)', () => { await vi.waitFor(() => { expect(received.flat()).toContainEqual({ type: 'create', path: entry }) }) + // The add is caught by the every-tick listing, NOT the 15-tick index backstop: detection lands well + // before a backstop could fire, so onFullScan must not have run. (On the old gated impl a coarse-FS + // signature collision would have deferred this to the backstop.) + expect(fullScans).not.toHaveBeenCalled() await rm(entry, { recursive: true }) await vi.waitFor(() => { @@ -344,6 +348,28 @@ describe('worktree git-common polling gate (non-darwin)', () => { }) }) + it('does not fabricate worktree deletions when the readdir fails non-ENOENT (transient)', async () => { + // Why: a transient readdir failure (EIO/ESTALE/EMFILE/ENOTDIR, network/SSH hiccup) must not be read + // as "every linked worktree removed". Simulate a non-ENOENT failure by replacing the worktrees dir + // with a file so readdir throws ENOTDIR; the known entry must NOT be reported deleted. On the old + // catch-all (entryPaths = []) this emitted a false delete for every entry. + const commonDir = await makePollingCommonDir() + const entry = join(commonDir, 'worktrees', 'keep') + await mkdir(entry) + await writeFile(join(entry, 'HEAD'), 'ref: refs/heads/main') + const received: WorktreeBasePollEvent[][] = [] + await startPollingWatch(commonDir, received) + + await new Promise((resolve) => setTimeout(resolve, POLL_MS * 2)) + const worktreesDir = join(commonDir, 'worktrees') + await rm(worktreesDir, { recursive: true, force: true }) + await writeFile(worktreesDir, 'not-a-dir') + await new Promise((resolve) => setTimeout(resolve, POLL_MS * 4)) + + expect(received.flat()).not.toContainEqual({ type: 'delete', path: entry }) + await rm(worktreesDir, { force: true }) + }) + it('detects an in-place structural (HEAD) write on a known entry every tick, without the index backstop', async () => { // Why: a raw HEAD/gitdir/config.worktree rewrite does not bump the entry-dir mtime, so the // structural leaves are re-stat'd every tick (never gated) — the change surfaces within one tick