From 04a6100ac03fa22eeedb5fed97b7cf6b15c4f669 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Wed, 15 Jul 2026 17:33:48 -0700 Subject: [PATCH] Fix aborted git status scans being mistaken for completed empty results MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - An aborted scan/numstat pass now always rejects instead of silently resolving, so a cancelled request can't look like a valid empty status. - Stop clearing the line-stats cache key on abort, since an aborted pass never wrote to it — clearing was evicting a concurrent scan's healthy snapshot and forcing a redundant numstat recompute. --- src/main/git/status.ts | 5 ++++ src/relay/git-handler-status-ops.ts | 5 ++++ .../git-status-line-stats-cache.test.ts | 28 +++++++++++++++++++ src/shared/git-status-line-stats-cache.ts | 4 ++- 4 files changed, 41 insertions(+), 1 deletion(-) diff --git a/src/main/git/status.ts b/src/main/git/status.ts index 4b5bede9e78..60ab59c8802 100644 --- a/src/main/git/status.ts +++ b/src/main/git/status.ts @@ -315,6 +315,8 @@ async function runGetStatus( didHitLimit = stoppedEarly statusSucceeded = true } catch (error) { + // Why: an aborted scan must reject, not resolve — swallowing here would let + // a cancelled request be mistaken for a completed (empty) status result. if (options.signal?.aborted) { throw error } @@ -557,6 +559,9 @@ async function runNumstat( ) return parseNumstat(stdout) } catch (error) { + // Why: an aborted pass must reject so a cancelled scan is never treated as + // a completed one; only a genuine (non-abort) numstat failure degrades to + // uncounted rows below. if (options.signal?.aborted) { throw error } diff --git a/src/relay/git-handler-status-ops.ts b/src/relay/git-handler-status-ops.ts index 4a8ad75b351..823c788bf51 100644 --- a/src/relay/git-handler-status-ops.ts +++ b/src/relay/git-handler-status-ops.ts @@ -168,6 +168,8 @@ export async function getStatusOp( } } } catch (error) { + // Why: an aborted scan must reject, not resolve — swallowing here would let + // a cancelled request be mistaken for a completed (empty) status result. if (options.signal?.aborted) { throw error } @@ -218,6 +220,9 @@ async function runNumstat( ) return parseNumstat(stdout) } catch (error) { + // Why: an aborted pass must reject so a cancelled scan is never treated as + // a completed one; only a genuine (non-abort) numstat failure degrades to + // uncounted rows below. if (signal?.aborted) { throw error } diff --git a/src/shared/git-status-line-stats-cache.test.ts b/src/shared/git-status-line-stats-cache.test.ts index fa17d0d6079..05040e28e83 100644 --- a/src/shared/git-status-line-stats-cache.test.ts +++ b/src/shared/git-status-line-stats-cache.test.ts @@ -5,6 +5,7 @@ import { clearGitStatusLineStatsCache, clearGitStatusLineStatsCacheKey, GIT_STATUS_LINE_STATS_CACHE_MAX_AGE_MS, + reuseOrRecomputeGitStatusLineStats, storeGitStatusLineStats } from './git-status-line-stats-cache' @@ -109,6 +110,33 @@ describe('git status line stats cache', () => { expect(entries[0]?.added).toBe(3) }) + it('keeps a prior good snapshot reusable when an overlapping recompute is aborted', async () => { + // A completed scan stored healthy counts. + storeGitStatusLineStats({ cacheKey: 'native\0/repo', head: 'head-1', entries: cachedEntries }) + + // A later scan begins and recomputes but is aborted before it can store. + const abortedWrite = beginGitStatusLineStatsCacheWrite('native\0/repo') + await reuseOrRecomputeGitStatusLineStats({ + cacheKey: 'native\0/repo', + head: 'head-1', + entries: [{ path: 'src/a.ts', status: 'modified', area: 'unstaged' }], + writeToken: abortedWrite, + reuse: false, + isAborted: () => true, + recompute: async () => true + }) + + // The aborted pass persisted nothing, so it must not evict the healthy + // snapshot — a following safety read still reuses the stored counts. + const entries: { path: string; status: string; area: string; added?: number }[] = [ + { path: 'src/a.ts', status: 'modified', area: 'unstaged' } + ] + expect( + applyCachedGitStatusLineStats({ cacheKey: 'native\0/repo', head: 'head-1', entries }) + ).toBe(true) + expect(entries[0]?.added).toBe(3) + }) + it('rejects stale cache writes and writes started before invalidation', () => { const olderWrite = beginGitStatusLineStatsCacheWrite('native\0/repo') const latestWrite = beginGitStatusLineStatsCacheWrite('native\0/repo') diff --git a/src/shared/git-status-line-stats-cache.ts b/src/shared/git-status-line-stats-cache.ts index 1d7fe81926b..75aefd9795b 100644 --- a/src/shared/git-status-line-stats-cache.ts +++ b/src/shared/git-status-line-stats-cache.ts @@ -196,7 +196,9 @@ export async function reuseOrRecomputeGitStatusLineStats(input: { } const complete = await input.recompute() if (input.isAborted()) { - clearGitStatusLineStatsCacheKey(input.cacheKey, input.writeToken) + // Why: an aborted pass never reached storeGitStatusLineStats, so there is + // nothing partial to undo; clearing here would instead evict a concurrent + // scan's healthy snapshot and force a redundant numstat on the next read. return } if (!complete) {