From e7222a560bdb29ff69dcea08a25a0b472d861a61 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Wed, 15 Jul 2026 17:33:17 -0700 Subject: [PATCH] feat(git-status): batch, cancel, and cache git status polling to cut idl - Add a single duty-cycled refresh scheduler (activity debounce + 60s safety timer) replacing multiple overlapping intervals, so status polling no longer runs near-continuously on large repos (#7983). - Let safety refreshes reuse cached numstat line counts instead of re-running diff --numstat every cycle, invalidated by head change, known mutations, and a bounded TTL. - Thread AbortSignal/request-token cancellation through IPC, RPC, and relay layers so a superseded or backgrounded git:status call is killed instead of finishing wastefully. - Fix automatic upstream/status apply ordering so a slow, older refresh can no longer clobber a newer result, and so an earlier refresh still applies when a later one fails. --- src/main/git/status.test.ts | 110 ++++++ src/main/git/status.ts | 73 +++- src/main/ipc/filesystem.test.ts | 75 +++- src/main/ipc/filesystem.ts | 63 +-- .../ipc/sender-scoped-request-cancellation.ts | 50 +++ src/main/providers/ssh-git-provider.test.ts | 16 + src/main/providers/ssh-git-provider.ts | 4 +- src/main/providers/types.ts | 1 + src/main/runtime/rpc/methods/git-params.ts | 3 +- src/main/runtime/rpc/methods/git.test.ts | 22 ++ src/main/runtime/rpc/methods/git.ts | 10 +- src/preload/api-types.ts | 3 + src/preload/index.ts | 4 + src/relay/git-handler-ops.ts | 8 +- src/relay/git-handler-status-ops.test.ts | 47 +++ src/relay/git-handler-status-ops.ts | 69 +++- src/relay/git-handler.ts | 8 +- .../right-sidebar/coalesced-poll-runner.ts | 18 +- .../git-status-refresh-ordering.ts | 100 +++++ .../git-status-refresh-scheduler.test.ts | 246 ++++++++++++ .../git-status-refresh-scheduler.ts | 210 ++++++++++ .../right-sidebar/git-status-refresh.test.ts | 91 +++++ .../right-sidebar/git-status-refresh.ts | 114 +++--- .../stale-conflict-operation-poll.ts | 113 ++++++ .../useGitStatusPolling.rerender.test.ts | 143 ++++++- .../right-sidebar/useGitStatusPolling.test.ts | 24 +- .../right-sidebar/useGitStatusPolling.ts | 368 ++++++++---------- .../abortable-runtime-environment-call.ts | 65 ++++ .../src/runtime/runtime-git-client.test.ts | 88 +++++ .../src/runtime/runtime-git-client.ts | 64 ++- .../src/runtime/runtime-rpc-client.test.ts | 50 ++- .../src/runtime/runtime-rpc-client.ts | 55 +-- src/renderer/src/web/web-preload-api.ts | 12 +- .../src/web/web-runtime-client.test.ts | 43 ++ src/renderer/src/web/web-runtime-client.ts | 33 +- .../git-status-line-stats-cache.test.ts | 152 ++++++++ src/shared/git-status-line-stats-cache.ts | 235 +++++++++++ src/shared/git-uncommitted-line-stats.ts | 8 +- 38 files changed, 2378 insertions(+), 420 deletions(-) create mode 100644 src/main/ipc/sender-scoped-request-cancellation.ts create mode 100644 src/renderer/src/components/right-sidebar/git-status-refresh-ordering.ts create mode 100644 src/renderer/src/components/right-sidebar/git-status-refresh-scheduler.test.ts create mode 100644 src/renderer/src/components/right-sidebar/git-status-refresh-scheduler.ts create mode 100644 src/renderer/src/components/right-sidebar/stale-conflict-operation-poll.ts create mode 100644 src/renderer/src/runtime/abortable-runtime-environment-call.ts create mode 100644 src/shared/git-status-line-stats-cache.test.ts create mode 100644 src/shared/git-status-line-stats-cache.ts diff --git a/src/main/git/status.test.ts b/src/main/git/status.test.ts index 3f7cbfd9f3a..8af536a31a6 100644 --- a/src/main/git/status.test.ts +++ b/src/main/git/status.test.ts @@ -1414,6 +1414,116 @@ describe('getStatus', () => { ]) }) + it('reuses unchanged line stats only when the safety hint is present', async () => { + readFileMock.mockResolvedValue('gitdir: /repo/.git/worktrees/feature\n') + existsSyncMock.mockReturnValue(false) + gitExecFileAsyncMock.mockImplementation((args: string[]) => { + if (args.includes('status')) { + return Promise.resolve({ + stdout: + '# branch.oid head-1\n' + '1 .M N... 100644 100644 100644 aaaa aaaa src/unstaged.ts\n' + }) + } + if (args.includes('--numstat')) { + return Promise.resolve({ stdout: '3\t4\tsrc/unstaged.ts\n' }) + } + return Promise.resolve({ stdout: '' }) + }) + + await getStatus('/repo') + const reused = await getStatus('/repo', { reuseLineStats: true }) + await getStatus('/repo') + + expect(reused.entries).toEqual([ + { path: 'src/unstaged.ts', status: 'modified', area: 'unstaged', added: 3, removed: 4 } + ]) + expect( + gitExecFileAsyncMock.mock.calls.filter(([args]) => args.includes('--numstat')) + ).toHaveLength(2) + }) + + it('recomputes after a scan whose numstat failed instead of pinning missing counts', async () => { + readFileMock.mockResolvedValue('gitdir: /repo/.git/worktrees/feature\n') + existsSyncMock.mockReturnValue(false) + let failNumstat = true + gitExecFileAsyncMock.mockImplementation((args: string[]) => { + if (args.includes('status')) { + return Promise.resolve({ + stdout: + '# branch.oid head-1\n' + '1 .M N... 100644 100644 100644 aaaa aaaa src/unstaged.ts\n' + }) + } + if (args.includes('--numstat')) { + if (failNumstat) { + failNumstat = false + return Promise.reject(new Error('transient index.lock')) + } + return Promise.resolve({ stdout: '3\t4\tsrc/unstaged.ts\n' }) + } + return Promise.resolve({ stdout: '' }) + }) + + const failed = await getStatus('/repo') + const reused = await getStatus('/repo', { reuseLineStats: true }) + + expect(failed.entries[0]?.added).toBeUndefined() + expect(reused.entries).toEqual([ + { path: 'src/unstaged.ts', status: 'modified', area: 'unstaged', added: 3, removed: 4 } + ]) + }) + + it('invalidates safety reuse for a new head and for known mutations', async () => { + readFileMock.mockResolvedValue('gitdir: /repo/.git/worktrees/feature\n') + existsSyncMock.mockReturnValue(false) + let head = 'head-1' + gitExecFileAsyncMock.mockImplementation((args: string[]) => { + if (args.includes('status')) { + return Promise.resolve({ + stdout: + `# branch.oid ${head}\n` + '1 .M N... 100644 100644 100644 aaaa aaaa src/unstaged.ts\n' + }) + } + if (args.includes('--numstat')) { + return Promise.resolve({ stdout: '3\t4\tsrc/unstaged.ts\n' }) + } + return Promise.resolve({ stdout: '' }) + }) + + await getStatus('/repo') + head = 'head-2' + await getStatus('/repo', { reuseLineStats: true }) + await stageFile('/repo', 'src/unstaged.ts') + await getStatus('/repo', { reuseLineStats: true }) + + expect( + gitExecFileAsyncMock.mock.calls.filter(([args]) => args.includes('--numstat')) + ).toHaveLength(3) + }) + + it('isolates line-stat reuse between WSL distributions', async () => { + readFileMock.mockResolvedValue('gitdir: /repo/.git/worktrees/feature\n') + existsSyncMock.mockReturnValue(false) + gitExecFileAsyncMock.mockImplementation((args: string[]) => { + if (args.includes('status')) { + return Promise.resolve({ + stdout: '1 .M N... 100644 100644 100644 aaaa aaaa src/unstaged.ts\n' + }) + } + if (args.includes('--numstat')) { + return Promise.resolve({ stdout: '3\t4\tsrc/unstaged.ts\n' }) + } + return Promise.resolve({ stdout: '' }) + }) + + await getStatus('/repo', { wslDistro: 'ubuntu' }) + await getStatus('/repo', { wslDistro: 'debian', reuseLineStats: true }) + await getStatus('/repo', { wslDistro: 'ubuntu', reuseLineStats: true }) + + expect( + gitExecFileAsyncMock.mock.calls.filter(([args]) => args.includes('--numstat')) + ).toHaveLength(2) + }) + it('attaches numstat counts for literal paths containing rename markers', async () => { readFileMock.mockResolvedValue('gitdir: /repo/.git/worktrees/feature\n') existsSyncMock.mockReturnValue(false) diff --git a/src/main/git/status.ts b/src/main/git/status.ts index ddbdd1b2ca4..4b5bede9e78 100644 --- a/src/main/git/status.ts +++ b/src/main/git/status.ts @@ -51,6 +51,12 @@ import { InFlightPromiseDedupe, stableInFlightKey } from '../../shared/in-flight import type { GitRuntimeOptions } from './git-runtime-options' import { gitOptionsForWorktree } from './git-runtime-options' import { parseGitRevListFirstParentOid } from '../../shared/git-rev-list-output' +import { + beginGitStatusLineStatsCacheWrite, + clearGitStatusLineStatsCache, + clearGitStatusLineStatsCacheKey, + reuseOrRecomputeGitStatusLineStats +} from '../../shared/git-status-line-stats-cache' const MAX_GIT_SHOW_BYTES = 10 * 1024 * 1024 const MAX_STAGED_COMMIT_CONTEXT_BYTES = MAX_GIT_SHOW_BYTES @@ -97,6 +103,7 @@ const statusReadsInFlight = new Map>() export function invalidateGitReadCaches(): void { gitDiffReadDedupe.clear() statusReadsInFlight.clear() + clearGitStatusLineStatsCache() clearSubmodulePathsCache() resolvedUpstreamNameCache.clear() } @@ -195,6 +202,7 @@ export function getEffectiveUpstreamStatusGenerationCountForTests(): number { export type GetStatusOptions = GitRuntimeOptions & { includeIgnored?: boolean + reuseLineStats?: boolean /** * Max changed-file entries before git is stopped and the result is marked * `didHitLimit`. Defaults to DEFAULT_GIT_STATUS_LIMIT; 0 disables the cap. @@ -243,6 +251,7 @@ function getStatusReadKey(worktreePath: string, options: GetStatusOptions): stri worktreePath, options.wslDistro ?? '', options.includeIgnored === true, + options.reuseLineStats === true, options.bypassEffectiveUpstreamNegativeCache === true, limit ].join('\0') @@ -252,6 +261,8 @@ async function runGetStatus( worktreePath: string, options: GetStatusOptions = {} ): Promise { + const lineStatsCacheKey = getStatusLineStatsCacheKey(worktreePath, options) + const lineStatsWriteToken = beginGitStatusLineStatsCacheWrite(lineStatsCacheKey) let effectiveUpstreamStatus: GitUpstreamStatus | undefined let statusSucceeded = false // Why: a negative/fractional/NaN limit would trigger spurious early-stop or @@ -303,7 +314,10 @@ async function runGetStatus( } didHitLimit = stoppedEarly statusSucceeded = true - } catch { + } catch (error) { + if (options.signal?.aborted) { + throw error + } // Not a git repo or git not available } @@ -334,11 +348,17 @@ async function runGetStatus( options ) try { + // Why: the probe promise and its name/negative caches are shared by + // concurrent status reads, so one caller's abort must not reject the + // shared probe or evict warm cache state for the others. The probe is + // small and its cached result stays useful, so run it unbound from + // this request's signal. + const { signal: _requestSignal, ...sharedProbeOptions } = options effectiveUpstreamStatus = await readOrProbeEffectiveUpstreamStatus( cacheKey, worktreePath, branchName, - options, + sharedProbeOptions, options.bypassEffectiveUpstreamNegativeCache === true ) } catch { @@ -355,7 +375,17 @@ async function runGetStatus( // running numstat over a huge change set would reintroduce the cost the limit // exists to avoid, matching how a "huge" repo disables extra git features. if (!didHitLimit) { - await attachLineStats(worktreePath, entries, options) + await reuseOrRecomputeGitStatusLineStats({ + cacheKey: lineStatsCacheKey, + head, + entries, + writeToken: lineStatsWriteToken, + reuse: options.reuseLineStats === true, + isAborted: () => options.signal?.aborted === true, + recompute: () => attachLineStats(worktreePath, entries, options) + }) + } else { + clearGitStatusLineStatsCacheKey(lineStatsCacheKey, lineStatsWriteToken) } return { @@ -382,6 +412,12 @@ async function runGetStatus( } } +function getStatusLineStatsCacheKey(worktreePath: string, options: GitRuntimeOptions = {}): string { + // Why: identical path strings can address different Linux filesystems in + // different WSL distros, so derived stats must follow Git's execution host. + return `${options.wslDistro ?? 'native'}\0${worktreePath}` +} + /** * Resolve a submodule's own worktree path from a parent worktree + relative * submodule path, rejecting anything that escapes the parent. Shared by the @@ -505,7 +541,7 @@ async function runNumstat( worktreePath: string, cached: boolean, options: GitRuntimeOptions = {} -): Promise> { +): Promise | null> { try { const { stdout } = await gitExecFileAsync( [ @@ -520,20 +556,25 @@ async function runNumstat( { ...gitOptionsForWorktree(worktreePath, options), env: gitOptionalLocksDisabledEnv() } ) return parseNumstat(stdout) - } catch { + } catch (error) { + if (options.signal?.aborted) { + throw error + } // Why: a numstat failure (e.g. transient lock) should leave rows without - // counts rather than break the whole status refresh. - return new Map() + // counts rather than break the whole status refresh. Null (vs an empty + // map) tells the caller the pass is incomplete and must not be cached. + return null } } +/** Returns false when a numstat pass failed, so callers skip caching it. */ async function attachLineStats( worktreePath: string, entries: GitStatusEntry[], options: GitRuntimeOptions = {} -): Promise { +): Promise { if (entries.length === 0) { - return + return true } const hasStaged = entries.some((entry) => entry.area === 'staged') const hasUnstaged = entries.some((entry) => entry.area === 'unstaged') @@ -544,18 +585,19 @@ async function attachLineStats( const [stagedStats, unstagedStats, untrackedStats] = await Promise.all([ hasStaged ? runNumstat(worktreePath, true, options) : Promise.resolve(emptyStats), hasUnstaged ? runNumstat(worktreePath, false, options) : Promise.resolve(emptyStats), - collectUntrackedAdditions(worktreePath, untrackedPaths) + collectUntrackedAdditions(worktreePath, untrackedPaths, options.signal) ]) for (const entry of entries) { applyLineStats( entry, entry.area === 'staged' - ? stagedStats.get(entry.path) + ? (stagedStats ?? emptyStats).get(entry.path) : entry.area === 'unstaged' - ? unstagedStats.get(entry.path) + ? (unstagedStats ?? emptyStats).get(entry.path) : untrackedStats.get(entry.path) ) } + return stagedStats !== null && unstagedStats !== null } function getShortBranchName(branch: string | undefined): string | null { @@ -753,7 +795,12 @@ async function probeOrRevalidateEffectiveUpstreamStatus( cached.upstreamName ) return { status, probedSameNameOriginRef: false } - } catch { + } catch (error) { + // Why: an aborted probe says nothing about the ref; evicting the warm + // name cache here would force a pointless full re-resolve next scan. + if (options.signal?.aborted) { + throw error + } // Ref deleted or repo state changed — fall through to a full re-resolve. resolvedUpstreamNameCache.delete(cacheKey) } diff --git a/src/main/ipc/filesystem.test.ts b/src/main/ipc/filesystem.test.ts index 2e9d1843988..c1c319c55f1 100644 --- a/src/main/ipc/filesystem.test.ts +++ b/src/main/ipc/filesystem.test.ts @@ -1148,6 +1148,72 @@ describe('registerFilesystemHandlers', () => { }) }) + it('forwards line-stat reuse through local and SSH git status IPC', async () => { + registerWorktreeRootsForRepo(store as never, 'repo-1', [REPO_PATH, WORKTREE_FEATURE_PATH]) + getStatusMock.mockResolvedValue({ entries: [], conflictOperation: 'unknown' }) + const sshProvider = { + getStatus: vi.fn().mockResolvedValue({ entries: [], conflictOperation: 'unknown' }) + } + getSshGitProviderMock.mockReturnValue(sshProvider) + registerFilesystemHandlers(store as never) + + await handlers.get('git:status')!(null, { + worktreePath: WORKTREE_FEATURE_PATH, + reuseLineStats: true + }) + await handlers.get('git:status')!(null, { + worktreePath: '/remote/repo', + connectionId: 'ssh-1', + reuseLineStats: true + }) + + expect(getStatusMock).toHaveBeenCalledWith(WORKTREE_FEATURE_PATH, { + includeIgnored: false, + reuseLineStats: true + }) + expect(sshProvider.getStatus).toHaveBeenCalledWith('/remote/repo', { + includeIgnored: false, + reuseLineStats: true + }) + }) + + it('aborts tokenized local status without crossing renderer boundaries', async () => { + registerWorktreeRootsForRepo(store as never, 'repo-1', [REPO_PATH, WORKTREE_FEATURE_PATH]) + const statusSignals: AbortSignal[] = [] + getStatusMock.mockImplementation( + (_worktreePath: string, options: { signal?: AbortSignal }) => + new Promise((_resolve, reject) => { + if (options.signal) { + statusSignals.push(options.signal) + options.signal.addEventListener('abort', () => reject(new Error('aborted')), { + once: true + }) + } + }) + ) + registerFilesystemHandlers(store as never) + + const firstEvent = { sender: { id: 7 } } + const secondEvent = { sender: { id: 8 } } + const firstRequest = handlers.get('git:status')!(firstEvent, { + worktreePath: WORKTREE_FEATURE_PATH, + requestToken: 'status-1' + }) as Promise + const secondRequest = handlers.get('git:status')!(secondEvent, { + worktreePath: WORKTREE_FEATURE_PATH, + requestToken: 'status-1' + }) as Promise + await vi.waitFor(() => expect(statusSignals).toHaveLength(2)) + await handlers.get('git:cancelStatus')!(firstEvent, { requestToken: 'status-1' }) + + expect(statusSignals[0]?.aborted).toBe(true) + expect(statusSignals[1]?.aborted).toBe(false) + await expect(firstRequest).rejects.toThrow('aborted') + + await handlers.get('git:cancelStatus')!(secondEvent, { requestToken: 'status-1' }) + await expect(secondRequest).rejects.toThrow('aborted') + }) + it('checks ignored paths through local and SSH git providers', async () => { registerWorktreeRootsForRepo(store as never, 'repo-1', [REPO_PATH, WORKTREE_FEATURE_PATH]) checkIgnoredPathsMock.mockResolvedValue(['dist/bundle.js']) @@ -2364,20 +2430,23 @@ describe('registerFilesystemHandlers', () => { registerFilesystemHandlers(store as never) - const pending = handlers.get('fs:listFiles')!(null, { + // Why: cancellation keys are scoped to the issuing webContents, so the + // cancel must come from the same sender as the listing request. + const senderEvent = { sender: { id: 7 } } + const pending = handlers.get('fs:listFiles')!(senderEvent, { rootPath: '/home/user/repo', connectionId: 'conn-1', requestToken: 'token-1' }) as Promise expect(capturedSignal?.aborted).toBe(false) - await handlers.get('fs:cancelListFiles')!(null, { requestToken: 'token-1' }) + await handlers.get('fs:cancelListFiles')!(senderEvent, { requestToken: 'token-1' }) expect(capturedSignal?.aborted).toBe(true) await expect(pending).rejects.toThrow('listing cancelled') // Unknown or already-settled tokens are a no-op, not an error. expect(() => - handlers.get('fs:cancelListFiles')!(null, { requestToken: 'unknown' }) + handlers.get('fs:cancelListFiles')!(senderEvent, { requestToken: 'unknown' }) ).not.toThrow() }) }) diff --git a/src/main/ipc/filesystem.ts b/src/main/ipc/filesystem.ts index 3918f8d3ea4..931c28c401f 100644 --- a/src/main/ipc/filesystem.ts +++ b/src/main/ipc/filesystem.ts @@ -122,6 +122,7 @@ import { getRuntimePathBasename } from '../../shared/cross-platform-path' import type { LocalProjectWorktreeGitOptions } from '../project-runtime-git-options' import { registerLocalLogTailHandlers } from './local-log-tail' import { localLogFileIdentity } from '../ai-vault/local-log-tail-reader' +import { createSenderScopedRequestCancellations } from './sender-scoped-request-cancellation' // Why: Monaco has large-file optimizations like VS Code; blocking at 5MB makes // ordinary JSON/log files inaccessible before the editor can degrade features. @@ -1072,11 +1073,11 @@ export function registerFilesystemHandlers( // Why #7721: keyed by renderer-generated token so a workspace switch can // abort the previous workspace's full-tree scan (SSH relays otherwise stack // scans that starve interactive fs.readDir/fs.stat past their 30s timeout). - const listFilesCancellations = new Map() + const listFilesCancellations = createSenderScopedRequestCancellations() ipcMain.handle( 'fs:listFiles', async ( - _event, + event, args: { rootPath: string connectionId?: string @@ -1084,10 +1085,7 @@ export function registerFilesystemHandlers( requestToken?: string } ): Promise => { - const controller = args.requestToken ? new AbortController() : null - if (controller && args.requestToken) { - listFilesCancellations.set(args.requestToken, controller) - } + const controller = listFilesCancellations.begin(event, args.requestToken) try { if (args.connectionId) { const provider = getSshFilesystemProvider(args.connectionId) @@ -1108,53 +1106,66 @@ export function registerFilesystemHandlers( } return await listQuickOpenFiles(args.rootPath, store, args.excludePaths, controller?.signal) } finally { - if (args.requestToken) { - listFilesCancellations.delete(args.requestToken) - } + listFilesCancellations.finish(event, args.requestToken, controller) } } ) - ipcMain.handle('fs:cancelListFiles', (_event, args: { requestToken: string }): void => { - // Why: best-effort — the entry is gone once the listing settles. - listFilesCancellations.get(args.requestToken)?.abort() + ipcMain.handle('fs:cancelListFiles', (event, args: { requestToken: string }): void => { + listFilesCancellations.cancel(event, args.requestToken) }) // ─── Git operations ───────────────────────────────────── + const gitStatusCancellations = createSenderScopedRequestCancellations() ipcMain.handle( 'git:status', async ( - _event, + event, args: { worktreePath: string connectionId?: string includeIgnored?: boolean bypassEffectiveUpstreamNegativeCache?: boolean + reuseLineStats?: boolean + requestToken?: string } ): Promise => { + const controller = gitStatusCancellations.begin(event, args.requestToken) const options = { includeIgnored: args.includeIgnored ?? false, + ...(args.reuseLineStats === true ? { reuseLineStats: true } : {}), ...(args.bypassEffectiveUpstreamNegativeCache === true ? { bypassEffectiveUpstreamNegativeCache: true } - : {}) + : {}), + ...(controller ? { signal: controller.signal } : {}) } - if (args.connectionId) { - const provider = getSshGitProvider(args.connectionId) - if (!provider) { - throw new Error(SSH_GIT_PROVIDER_UNAVAILABLE_MESSAGE) + try { + if (args.connectionId) { + const provider = getSshGitProvider(args.connectionId) + if (!provider) { + throw new Error(SSH_GIT_PROVIDER_UNAVAILABLE_MESSAGE) + } + // Why: awaiting here keeps the cancellation token registered until + // the remote request settles instead of running finally immediately. + return await provider.getStatus(args.worktreePath, options) } - return provider.getStatus(args.worktreePath, options) + const worktreePath = await resolveRegisteredWorktreePath(args.worktreePath, store) + const gitOptions = getLocalGitOptionsForRegisteredWorktree( + store, + args.worktreePath, + worktreePath + ) + return await getStatus(worktreePath, { ...options, ...gitOptions }) + } finally { + gitStatusCancellations.finish(event, args.requestToken, controller) } - const worktreePath = await resolveRegisteredWorktreePath(args.worktreePath, store) - const gitOptions = getLocalGitOptionsForRegisteredWorktree( - store, - args.worktreePath, - worktreePath - ) - return getStatus(worktreePath, { ...options, ...gitOptions }) } ) + ipcMain.handle('git:cancelStatus', (event, args: { requestToken: string }): void => { + gitStatusCancellations.cancel(event, args.requestToken) + }) + // Why: the parent status only reports one gitlink row per submodule. When the // user expands a dirty submodule, this fetches the inner per-file changes by // running a plain status inside the submodule's own worktree (read-only). diff --git a/src/main/ipc/sender-scoped-request-cancellation.ts b/src/main/ipc/sender-scoped-request-cancellation.ts new file mode 100644 index 00000000000..9700be9c3da --- /dev/null +++ b/src/main/ipc/sender-scoped-request-cancellation.ts @@ -0,0 +1,50 @@ +import type { IpcMainInvokeEvent } from 'electron' + +export type SenderScopedRequestCancellations = { + /** Registers a cancellable request; aborts any previous request that reused the token. */ + begin: (event: IpcMainInvokeEvent, requestToken: string | undefined) => AbortController | null + /** Removes the registration once the request settles (no-op if it was replaced). */ + finish: ( + event: IpcMainInvokeEvent, + requestToken: string | undefined, + controller: AbortController | null + ) => void + /** Best-effort abort from the issuing webContents; a settled request is gone. */ + cancel: (event: IpcMainInvokeEvent, requestToken: string) => void +} + +/** + * Registry for renderer-cancellable IPC requests. Keys are scoped to the + * issuing webContents so one window's token can never cancel another window's + * request, and reusing a token aborts the previous request before the new one + * registers. + */ +export function createSenderScopedRequestCancellations(): SenderScopedRequestCancellations { + const controllers = new Map() + const keyFor = (event: IpcMainInvokeEvent, requestToken: string): string => + `${event.sender.id}\0${requestToken}` + return { + begin: (event, requestToken) => { + if (!requestToken) { + return null + } + const key = keyFor(event, requestToken) + controllers.get(key)?.abort() + const controller = new AbortController() + controllers.set(key, controller) + return controller + }, + finish: (event, requestToken, controller) => { + if (!requestToken || !controller) { + return + } + const key = keyFor(event, requestToken) + if (controllers.get(key) === controller) { + controllers.delete(key) + } + }, + cancel: (event, requestToken) => { + controllers.get(keyFor(event, requestToken))?.abort() + } + } +} diff --git a/src/main/providers/ssh-git-provider.test.ts b/src/main/providers/ssh-git-provider.test.ts index eb8383a3f60..d4a275a89ba 100644 --- a/src/main/providers/ssh-git-provider.test.ts +++ b/src/main/providers/ssh-git-provider.test.ts @@ -97,6 +97,22 @@ describe('SshGitProvider', () => { }) }) + it('getStatus forwards line-stat reuse and cancellation to the relay', async () => { + const controller = new AbortController() + mux.request.mockResolvedValue({ entries: [], conflictOperation: 'unknown' }) + + await provider.getStatus('/home/user/repo', { + reuseLineStats: true, + signal: controller.signal + }) + + expect(mux.request).toHaveBeenCalledWith( + 'git.status', + { worktreePath: '/home/user/repo', reuseLineStats: true }, + { signal: controller.signal } + ) + }) + it('getSubmoduleStatus sends git.submoduleStatus request', async () => { const statusResult = { entries: [], conflictOperation: 'unknown' } mux.request.mockResolvedValue(statusResult) diff --git a/src/main/providers/ssh-git-provider.ts b/src/main/providers/ssh-git-provider.ts index 4bca707a0fb..1aa1cd271a2 100644 --- a/src/main/providers/ssh-git-provider.ts +++ b/src/main/providers/ssh-git-provider.ts @@ -106,10 +106,12 @@ export class SshGitProvider implements IGitProvider { const upstreamCacheBypassArgs = options?.bypassEffectiveUpstreamNegativeCache ? { bypassEffectiveUpstreamNegativeCache: true } : {} + const lineStatsReuseArgs = options?.reuseLineStats ? { reuseLineStats: true } : {} const request = { worktreePath, ...includeIgnoredArgs, - ...upstreamCacheBypassArgs + ...upstreamCacheBypassArgs, + ...lineStatsReuseArgs } return (await (options?.signal ? this.mux.request('git.status', request, { signal: options.signal }) diff --git a/src/main/providers/types.ts b/src/main/providers/types.ts index 66642e38ad1..e11e86e1866 100644 --- a/src/main/providers/types.ts +++ b/src/main/providers/types.ts @@ -306,6 +306,7 @@ export type TerminalArtifactAccessOptions = { export type GitProviderStatusOptions = { includeIgnored?: boolean bypassEffectiveUpstreamNegativeCache?: boolean + reuseLineStats?: boolean signal?: AbortSignal } diff --git a/src/main/runtime/rpc/methods/git-params.ts b/src/main/runtime/rpc/methods/git-params.ts index 8f9d511dfca..7e13e584c42 100644 --- a/src/main/runtime/rpc/methods/git-params.ts +++ b/src/main/runtime/rpc/methods/git-params.ts @@ -9,7 +9,8 @@ export const WorktreeSelector = z.object({ export const GitStatusParams = WorktreeSelector.extend({ includeIgnored: z.boolean().optional(), - bypassEffectiveUpstreamNegativeCache: z.boolean().optional() + bypassEffectiveUpstreamNegativeCache: z.boolean().optional(), + reuseLineStats: z.boolean().optional() }) export const GitCheckIgnored = WorktreeSelector.extend({ diff --git a/src/main/runtime/rpc/methods/git.test.ts b/src/main/runtime/rpc/methods/git.test.ts index bb69dfc5958..1655bb0b3d5 100644 --- a/src/main/runtime/rpc/methods/git.test.ts +++ b/src/main/runtime/rpc/methods/git.test.ts @@ -80,6 +80,28 @@ describe('git RPC methods', () => { }) }) + it('forwards line-stat reuse and request cancellation for status requests', async () => { + const controller = new AbortController() + const runtime = { + getRuntimeId: () => 'test-runtime', + getRuntimeGitStatus: vi.fn().mockResolvedValue({ + entries: [], + conflictOperation: 'unknown' + }) + } as unknown as OrcaRuntimeService + const dispatcher = new RpcDispatcher({ runtime, methods: GIT_METHODS }) + + await dispatcher.dispatch( + makeRequest('git.status', { worktree: 'id:wt-1', reuseLineStats: true }), + { signal: controller.signal } + ) + + expect(runtime.getRuntimeGitStatus).toHaveBeenCalledWith('id:wt-1', { + reuseLineStats: true, + signal: controller.signal + }) + }) + it('returns ignored paths for selected explorer rows', async () => { const runtime = { getRuntimeId: () => 'test-runtime', diff --git a/src/main/runtime/rpc/methods/git.ts b/src/main/runtime/rpc/methods/git.ts index 1e4923f672a..c4ccdf071b6 100644 --- a/src/main/runtime/rpc/methods/git.ts +++ b/src/main/runtime/rpc/methods/git.ts @@ -88,10 +88,12 @@ export const GIT_METHODS: RpcMethod[] = [ defineMethod({ name: 'git.status', params: GitStatusParams, - handler: async (params, { runtime }) => { + handler: async (params, { runtime, signal }) => { const options = params.includeIgnored === undefined && - params.bypassEffectiveUpstreamNegativeCache === undefined + params.bypassEffectiveUpstreamNegativeCache === undefined && + params.reuseLineStats === undefined && + signal === undefined ? undefined : { ...(params.includeIgnored === undefined @@ -99,7 +101,9 @@ export const GIT_METHODS: RpcMethod[] = [ : { includeIgnored: params.includeIgnored }), ...(params.bypassEffectiveUpstreamNegativeCache === true ? { bypassEffectiveUpstreamNegativeCache: true } - : {}) + : {}), + ...(params.reuseLineStats === true ? { reuseLineStats: true } : {}), + ...(signal ? { signal } : {}) } return options === undefined ? runtime.getRuntimeGitStatus(params.worktree) diff --git a/src/preload/api-types.ts b/src/preload/api-types.ts index 9fd010a8695..f7e89a3c650 100644 --- a/src/preload/api-types.ts +++ b/src/preload/api-types.ts @@ -2512,7 +2512,10 @@ export type PreloadApi = { connectionId?: string includeIgnored?: boolean bypassEffectiveUpstreamNegativeCache?: boolean + reuseLineStats?: boolean + requestToken?: string }) => Promise + cancelStatus: (args: { requestToken: string }) => Promise submoduleStatus: (args: { worktreePath: string submodulePath: string diff --git a/src/preload/index.ts b/src/preload/index.ts index 9e5b7e419e1..462ab02f280 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -2943,7 +2943,11 @@ const api = { connectionId?: string includeIgnored?: boolean bypassEffectiveUpstreamNegativeCache?: boolean + reuseLineStats?: boolean + requestToken?: string }): Promise => ipcRenderer.invoke('git:status', args), + cancelStatus: (args: { requestToken: string }): Promise => + ipcRenderer.invoke('git:cancelStatus', args), submoduleStatus: (args: { worktreePath: string submodulePath: string diff --git a/src/relay/git-handler-ops.ts b/src/relay/git-handler-ops.ts index 0bb862fbcea..395e31badb3 100644 --- a/src/relay/git-handler-ops.ts +++ b/src/relay/git-handler-ops.ts @@ -16,7 +16,13 @@ import { readWorkingDiffFile } from './git-working-file-read' export type GitExec = ( args: string[], cwd: string, - opts?: { maxBuffer?: number; disableOptionalLocks?: boolean; stdin?: string; timeout?: number } + opts?: { + maxBuffer?: number + disableOptionalLocks?: boolean + signal?: AbortSignal + stdin?: string + timeout?: number + } ) => Promise<{ stdout: string; stderr: string }> export type GitBufferExec = (args: string[], cwd: string) => Promise diff --git a/src/relay/git-handler-status-ops.test.ts b/src/relay/git-handler-status-ops.test.ts index 044fa82bf6a..4a7494d0956 100644 --- a/src/relay/git-handler-status-ops.test.ts +++ b/src/relay/git-handler-status-ops.test.ts @@ -6,6 +6,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import type { GitExec } from './git-handler-ops' import { getStatusOp } from './git-handler-status-ops' import { clearNoEffectiveUpstreamStatusCache } from './git-status-upstream-negative-cache' +import { clearGitStatusLineStatsCache } from '../shared/git-status-line-stats-cache' const LARGE_STATUS_ENTRY_COUNT = 150_000 @@ -26,6 +27,7 @@ describe('getStatusOp', () => { beforeEach(() => { clearNoEffectiveUpstreamStatusCache() + clearGitStatusLineStatsCache() tmpDir = mkdtempSync(path.join(tmpdir(), 'relay-git-status-')) }) @@ -79,6 +81,51 @@ describe('getStatusOp', () => { expect(result.entries).toHaveLength(5) }) + it('reuses unchanged line stats only for hinted safety reads', async () => { + const statusOutput = `${buildBranchStatusOutput('head-1', '(detached)')}\n1 .M N... 100644 100644 100644 aaaa aaaa src/a.ts` + const git = vi.fn(async (args) => { + if (args.includes('status')) { + return { stdout: statusOutput, stderr: '' } + } + if (args.includes('diff')) { + return { stdout: '3\t2\tsrc/a.ts\n', stderr: '' } + } + throw new Error(`Unexpected git command: ${args.join(' ')}`) + }) + + await getStatusOp(git, { worktreePath: tmpDir }) + const reused = await getStatusOp(git, { worktreePath: tmpDir, reuseLineStats: true }) + await getStatusOp(git, { worktreePath: tmpDir }) + + expect(reused.entries).toContainEqual( + expect.objectContaining({ path: 'src/a.ts', added: 3, removed: 2 }) + ) + expect(git.mock.calls.filter(([args]) => args.includes('diff'))).toHaveLength(2) + }) + + it('forwards the request abort signal to status and numstat subprocesses', async () => { + const controller = new AbortController() + const git = vi.fn(async (args) => { + if (args.includes('status')) { + return { + stdout: '1 .M N... 100644 100644 100644 aaaa aaaa src/a.ts', + stderr: '' + } + } + if (args.includes('diff')) { + return { stdout: '1\t0\tsrc/a.ts\n', stderr: '' } + } + throw new Error(`Unexpected git command: ${args.join(' ')}`) + }) + + await getStatusOp(git, { worktreePath: tmpDir }, { signal: controller.signal }) + + expect(git.mock.calls).not.toHaveLength(0) + for (const [, , options] of git.mock.calls) { + expect(options?.signal).toBe(controller.signal) + } + }) + it('caches no-effective-upstream probes across status polls for the same head', async () => { const git = vi.fn(async (args) => { if (args.includes('status')) { diff --git a/src/relay/git-handler-status-ops.ts b/src/relay/git-handler-status-ops.ts index 4f48ec9b6da..4a8ad75b351 100644 --- a/src/relay/git-handler-status-ops.ts +++ b/src/relay/git-handler-status-ops.ts @@ -20,6 +20,11 @@ import { type GitLineStats } from '../shared/git-uncommitted-line-stats' import { DEFAULT_GIT_STATUS_LIMIT } from '../shared/git-status-limit' +import { + beginGitStatusLineStatsCacheWrite, + clearGitStatusLineStatsCacheKey, + reuseOrRecomputeGitStatusLineStats +} from '../shared/git-status-line-stats-cache' export async function resolveGitDir(worktreePath: string): Promise { const dotGitPath = path.join(worktreePath, '.git') @@ -58,7 +63,8 @@ export async function detectConflictOperation(worktreePath: string): Promise + params: Record, + options: { signal?: AbortSignal } = {} ): Promise<{ entries: Record[] conflictOperation: string @@ -70,6 +76,8 @@ export async function getStatusOp( statusLength?: number }> { const worktreePath = params.worktreePath as string + const lineStatsCacheKey = `relay\0${worktreePath}` + const lineStatsWriteToken = beginGitStatusLineStatsCacheWrite(lineStatsCacheKey) const includeIgnored = params.includeIgnored === true // Why: reject non-finite/negative limits so the cap guard stays reliable // (NaN would silently disable capping; negatives would over-truncate). @@ -106,7 +114,8 @@ export async function getStatusOp( const { stdout } = await git(statusArgs, worktreePath, { // Why: status polling is read-like; avoid refreshing the index and racing // terminal Git commands on `.git/worktrees/*/index.lock`. - disableOptionalLocks: true + disableOptionalLocks: true, + signal: options.signal }) const parsed = parseStatusOutput(stdout) head = parsed.head @@ -134,6 +143,9 @@ export async function getStatusOp( const branchName = getShortBranchName(branch) if (branchName) { try { + // Why: this probe coalesces across concurrent status reads, so one + // request's abort must not reject the shared in-flight promise for + // the others; the probe is small and its cached result stays useful. upstreamStatus = await readOrProbeNoEffectiveUpstreamStatus( { worktreePath, branchName, upstreamName: upstreamStatus?.upstreamName }, (args) => git(args, worktreePath), @@ -155,7 +167,10 @@ export async function getStatusOp( } } } - } catch { + } catch (error) { + if (options.signal?.aborted) { + throw error + } // not a git repo or git not available } @@ -165,7 +180,17 @@ export async function getStatusOp( // running numstat over a huge change set would reintroduce the cost the limit // exists to avoid. if (!didHitLimit) { - await attachLineStats(git, worktreePath, entries) + await reuseOrRecomputeGitStatusLineStats({ + cacheKey: lineStatsCacheKey, + head, + entries, + writeToken: lineStatsWriteToken, + reuse: params.reuseLineStats === true, + isAborted: () => options.signal?.aborted === true, + recompute: () => attachLineStats(git, worktreePath, entries, options.signal) + }) + } else { + clearGitStatusLineStatsCacheKey(lineStatsCacheKey, lineStatsWriteToken) } return { @@ -182,29 +207,36 @@ export async function getStatusOp( async function runNumstat( git: GitExec, worktreePath: string, - cached: boolean -): Promise> { + cached: boolean, + signal?: AbortSignal +): Promise | null> { try { const { stdout } = await git( ['-c', 'core.quotePath=false', 'diff', ...(cached ? ['--cached'] : []), '--numstat', '-M'], worktreePath, - { disableOptionalLocks: true } + { disableOptionalLocks: true, signal } ) return parseNumstat(stdout) - } catch { + } catch (error) { + if (signal?.aborted) { + throw error + } // Why: a numstat failure should leave rows without counts rather than break - // the whole status refresh. - return new Map() + // the whole status refresh. Null (vs an empty map) tells the caller the + // pass is incomplete and must not be cached. + return null } } +/** Returns false when a numstat pass failed, so callers skip caching it. */ async function attachLineStats( git: GitExec, worktreePath: string, - entries: Record[] -): Promise { + entries: Record[], + signal?: AbortSignal +): Promise { if (entries.length === 0) { - return + return true } const hasStaged = entries.some((entry) => entry.area === 'staged') const hasUnstaged = entries.some((entry) => entry.area === 'unstaged') @@ -213,21 +245,22 @@ async function attachLineStats( .map((entry) => entry.path as string) const emptyStats = new Map() const [stagedStats, unstagedStats, untrackedStats] = await Promise.all([ - hasStaged ? runNumstat(git, worktreePath, true) : Promise.resolve(emptyStats), - hasUnstaged ? runNumstat(git, worktreePath, false) : Promise.resolve(emptyStats), - collectUntrackedAdditions(worktreePath, untrackedPaths) + hasStaged ? runNumstat(git, worktreePath, true, signal) : Promise.resolve(emptyStats), + hasUnstaged ? runNumstat(git, worktreePath, false, signal) : Promise.resolve(emptyStats), + collectUntrackedAdditions(worktreePath, untrackedPaths, signal) ]) for (const entry of entries) { const filePath = entry.path as string applyLineStats( entry as { added?: number; removed?: number }, entry.area === 'staged' - ? stagedStats.get(filePath) + ? (stagedStats ?? emptyStats).get(filePath) : entry.area === 'unstaged' - ? unstagedStats.get(filePath) + ? (unstagedStats ?? emptyStats).get(filePath) : untrackedStats.get(filePath) ) } + return stagedStats !== null && unstagedStats !== null } function getShortBranchName(branch: string | undefined): string | null { diff --git a/src/relay/git-handler.ts b/src/relay/git-handler.ts index 510dd60f84b..83b00819756 100644 --- a/src/relay/git-handler.ts +++ b/src/relay/git-handler.ts @@ -76,6 +76,7 @@ import { import { GitResponseStreamRegistry } from './git-response-stream' import { GIT_RESPONSE_STREAM_THRESHOLD } from './protocol' import { endSubprocessStdin } from '../shared/subprocess-stdin-write' +import { clearGitStatusLineStatsCache } from '../shared/git-status-line-stats-cache' const execFileAsync = promisify(execFile) const MAX_GIT_BUFFER = 10 * 1024 * 1024 @@ -192,7 +193,7 @@ export class GitHandler { } private registerHandlers(): void { - this.dispatcher.onRequest('git.status', (p) => this.getStatus(p)) + this.dispatcher.onRequest('git.status', (p, context) => this.getStatus(p, context)) this.dispatcher.onRequest('git.submoduleStatus', (p) => this.getSubmoduleStatus(p)) this.dispatcher.onRequest('git.checkIgnored', (p) => this.checkIgnored(p)) this.dispatcher.onRequest('git.history', (p) => this.history(p)) @@ -281,6 +282,7 @@ export class GitHandler { private clearGitMutationReadCaches(): void { this.gitDiffReadDedupe.clear() + clearGitStatusLineStatsCache() clearSubmodulePathsCache(this.submodulePathsCache) } @@ -342,9 +344,9 @@ export class GitHandler { return stdout } - private async getStatus(params: Record) { + private async getStatus(params: Record, context: RequestContext) { this.gitDiffReadDedupe.clear() - return getStatusOp(this.git.bind(this), params) + return getStatusOp(this.git.bind(this), params, { signal: context.signal }) } // Why: the parent status only lists a single gitlink row per submodule. The diff --git a/src/renderer/src/components/right-sidebar/coalesced-poll-runner.ts b/src/renderer/src/components/right-sidebar/coalesced-poll-runner.ts index 1040ed279e2..5c2c443997f 100644 --- a/src/renderer/src/components/right-sidebar/coalesced-poll-runner.ts +++ b/src/renderer/src/components/right-sidebar/coalesced-poll-runner.ts @@ -25,6 +25,17 @@ export type SlowTaskBackoffOptions = { // so a fixed idle gap makes polling effectively continuous (#7983). Scale the // gap with the previous run's duration — aggressively for evidence-free timer // ticks, mildly for change signals — capped so results never go minutes-stale. +// Shared with the git-status refresh scheduler so both pacing machines keep +// the same guarantee: sustained triggers can never run the task back-to-back. +export function slowTaskRequiredIdleMs( + lastRunDurationMs: number, + multiplier: number, + minIntervalMs: number, + maxIntervalMs: number +): number { + return Math.max(minIntervalMs, Math.min(lastRunDurationMs * multiplier, maxIntervalMs)) +} + export function createCoalescedPollRunner( task: () => Promise, options?: { @@ -49,7 +60,12 @@ export function createCoalescedPollRunner( const multiplier = trigger?.changeSignal ? backoff.changeSignalMultiplier : backoff.idleMultiplier - return Math.max(minIntervalMs, Math.min(lastRunDurationMs * multiplier, backoff.maxIntervalMs)) + return slowTaskRequiredIdleMs( + lastRunDurationMs, + multiplier, + minIntervalMs, + backoff.maxIntervalMs + ) } const clearScheduledRun = (): void => { diff --git a/src/renderer/src/components/right-sidebar/git-status-refresh-ordering.ts b/src/renderer/src/components/right-sidebar/git-status-refresh-ordering.ts new file mode 100644 index 00000000000..9f4c45e0c17 --- /dev/null +++ b/src/renderer/src/components/right-sidebar/git-status-refresh-ordering.ts @@ -0,0 +1,100 @@ +const MAX_REFRESH_ORDERING_WORKTREES = 1024 +const strictUpstreamRefreshGenerationByWorktree = new Map() +const automaticRefreshGenerationByWorktree = new Map() +// Why: automatic ordering is latest-APPLIED-wins, not latest-started-wins. A +// later-started refresh that fails (or never applies) must not veto an earlier +// refresh's good result, so the fence only advances when a result is applied. +const lastAppliedAutomaticGenerationByWorktree = new Map() +const automaticUpstreamRefreshInFlightByWorktree = new Map() + +export type AutomaticRefreshOrder = { + strictGeneration: number + automaticGeneration: number +} + +function trimRefreshOrderingState(): void { + for (const worktreeId of strictUpstreamRefreshGenerationByWorktree.keys()) { + if (strictUpstreamRefreshGenerationByWorktree.size <= MAX_REFRESH_ORDERING_WORKTREES) { + break + } + if (automaticUpstreamRefreshInFlightByWorktree.has(worktreeId)) { + continue + } + strictUpstreamRefreshGenerationByWorktree.delete(worktreeId) + } +} + +export function beginAutomaticUpstreamRefresh(worktreeId: string): AutomaticRefreshOrder { + automaticUpstreamRefreshInFlightByWorktree.set( + worktreeId, + (automaticUpstreamRefreshInFlightByWorktree.get(worktreeId) ?? 0) + 1 + ) + const automaticGeneration = (automaticRefreshGenerationByWorktree.get(worktreeId) ?? 0) + 1 + automaticRefreshGenerationByWorktree.set(worktreeId, automaticGeneration) + return { + strictGeneration: strictUpstreamRefreshGenerationByWorktree.get(worktreeId) ?? 0, + automaticGeneration + } +} + +export function finishAutomaticUpstreamRefresh(worktreeId: string): void { + const count = automaticUpstreamRefreshInFlightByWorktree.get(worktreeId) ?? 0 + if (count <= 1) { + automaticUpstreamRefreshInFlightByWorktree.delete(worktreeId) + automaticRefreshGenerationByWorktree.delete(worktreeId) + lastAppliedAutomaticGenerationByWorktree.delete(worktreeId) + } else { + automaticUpstreamRefreshInFlightByWorktree.set(worktreeId, count - 1) + } + trimRefreshOrderingState() +} + +export function shouldApplyAutomaticUpstreamRefresh( + worktreeId: string, + order: AutomaticRefreshOrder, + shouldApply?: () => boolean +): boolean { + return ( + (strictUpstreamRefreshGenerationByWorktree.get(worktreeId) ?? 0) === order.strictGeneration && + order.automaticGeneration >= (lastAppliedAutomaticGenerationByWorktree.get(worktreeId) ?? 0) && + (shouldApply?.() ?? true) + ) +} + +/** + * Check-and-claim variant used immediately before applying state. On success + * it advances the applied fence so an older concurrent refresh that settles + * later cannot overwrite this refresh's newer result. + */ +export function claimAutomaticUpstreamRefreshApply( + worktreeId: string, + order: AutomaticRefreshOrder, + shouldApply?: () => boolean +): boolean { + if (!shouldApplyAutomaticUpstreamRefresh(worktreeId, order, shouldApply)) { + return false + } + lastAppliedAutomaticGenerationByWorktree.set( + worktreeId, + Math.max( + lastAppliedAutomaticGenerationByWorktree.get(worktreeId) ?? 0, + order.automaticGeneration + ) + ) + return true +} + +export function beginStrictUpstreamRefresh(worktreeId: string): void { + strictUpstreamRefreshGenerationByWorktree.set( + worktreeId, + (strictUpstreamRefreshGenerationByWorktree.get(worktreeId) ?? 0) + 1 + ) + trimRefreshOrderingState() +} + +export function clearGitStatusRefreshOrderingStateForTests(): void { + strictUpstreamRefreshGenerationByWorktree.clear() + automaticRefreshGenerationByWorktree.clear() + lastAppliedAutomaticGenerationByWorktree.clear() + automaticUpstreamRefreshInFlightByWorktree.clear() +} diff --git a/src/renderer/src/components/right-sidebar/git-status-refresh-scheduler.test.ts b/src/renderer/src/components/right-sidebar/git-status-refresh-scheduler.test.ts new file mode 100644 index 00000000000..f1f7dc9dadd --- /dev/null +++ b/src/renderer/src/components/right-sidebar/git-status-refresh-scheduler.test.ts @@ -0,0 +1,246 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' +import { + createGitStatusRefreshScheduler, + type GitStatusRefreshReason +} from './git-status-refresh-scheduler' + +function deferred(): { promise: Promise; resolve: () => void } { + let resolve = (): void => {} + const promise = new Promise((settle) => { + resolve = settle + }) + return { promise, resolve } +} + +async function flushMicrotasks(): Promise { + for (let index = 0; index < 5; index += 1) { + await Promise.resolve() + } +} + +function createScheduler( + task: (request: { reason: GitStatusRefreshReason; signal: AbortSignal }) => Promise +) { + return createGitStatusRefreshScheduler(task, { + safetyIntervalMs: 60_000, + activityDebounceMs: 125, + activityMinGapMs: 3000, + slowTaskBackoff: { + idleMultiplier: 5, + changeSignalMultiplier: 1, + maxIntervalMs: 5 * 60_000 + } + }) +} + +describe('createGitStatusRefreshScheduler', () => { + afterEach(() => { + vi.useRealTimers() + }) + + it('refreshes immediately on resume and uses one 60-second safety timeout', async () => { + vi.useFakeTimers() + const reasons: GitStatusRefreshReason[] = [] + const task = vi.fn(async ({ reason }: { reason: GitStatusRefreshReason }) => { + reasons.push(reason) + }) + const scheduler = createScheduler(task) + + scheduler.resumeSafety() + expect(task).toHaveBeenCalledTimes(1) + await flushMicrotasks() + + await vi.advanceTimersByTimeAsync(59_999) + expect(task).toHaveBeenCalledTimes(1) + await vi.advanceTimersByTimeAsync(1) + + expect(task).toHaveBeenCalledTimes(2) + expect(reasons).toEqual(['activity', 'safety']) + }) + + it('coalesces activity signals and restarts the safety horizon', async () => { + vi.useFakeTimers() + const task = vi.fn(async () => {}) + const scheduler = createScheduler(task) + + scheduler.resumeSafety() + await flushMicrotasks() + await vi.advanceTimersByTimeAsync(59_000) + + scheduler.signal() + scheduler.signal() + scheduler.signal() + await vi.advanceTimersByTimeAsync(124) + expect(task).toHaveBeenCalledTimes(1) + await vi.advanceTimersByTimeAsync(1) + expect(task).toHaveBeenCalledTimes(2) + await flushMicrotasks() + + await vi.advanceTimersByTimeAsync(59_999) + expect(task).toHaveBeenCalledTimes(2) + await vi.advanceTimersByTimeAsync(1) + expect(task).toHaveBeenCalledTimes(3) + }) + + it('serializes in-flight work and retains one trailing activity refresh', async () => { + vi.useFakeTimers() + const calls: ReturnType[] = [] + let active = 0 + let maxActive = 0 + const task = vi.fn(() => { + const call = deferred() + calls.push(call) + active += 1 + maxActive = Math.max(maxActive, active) + return call.promise.finally(() => { + active -= 1 + }) + }) + const scheduler = createScheduler(task) + + scheduler.resumeSafety() + scheduler.signal() + scheduler.signal() + scheduler.refreshNow() + expect(task).toHaveBeenCalledTimes(1) + + calls[0]?.resolve() + await flushMicrotasks() + // Why: the trailing refresh respects the 3s activity floor from run end. + expect(task).toHaveBeenCalledTimes(1) + await vi.advanceTimersByTimeAsync(3000) + expect(task).toHaveBeenCalledTimes(2) + expect(maxActive).toBe(1) + + calls[1]?.resolve() + await flushMicrotasks() + await vi.advanceTimersByTimeAsync(60_000) + expect(task).toHaveBeenCalledTimes(3) + }) + + it('spaces sustained signal bursts by the activity floor instead of running back-to-back', async () => { + vi.useFakeTimers() + const calls: ReturnType[] = [] + const task = vi.fn(() => { + const call = deferred() + calls.push(call) + return call.promise + }) + const scheduler = createScheduler(task) + + scheduler.resumeSafety() + scheduler.signal() + calls[0]?.resolve() + await flushMicrotasks() + expect(task).toHaveBeenCalledTimes(1) + + await vi.advanceTimersByTimeAsync(2999) + expect(task).toHaveBeenCalledTimes(1) + await vi.advanceTimersByTimeAsync(1) + expect(task).toHaveBeenCalledTimes(2) + + // A signal mid-flight plus immediate settle still waits out the floor. + scheduler.signal() + calls[1]?.resolve() + await flushMicrotasks() + expect(task).toHaveBeenCalledTimes(2) + await vi.advanceTimersByTimeAsync(3000) + expect(task).toHaveBeenCalledTimes(3) + }) + + it('stretches activity and safety pacing after a slow scan', async () => { + vi.useFakeTimers() + const calls: ReturnType[] = [] + const task = vi.fn(() => { + const call = deferred() + calls.push(call) + return call.promise + }) + const scheduler = createScheduler(task) + + scheduler.resumeSafety() + // The first scan takes 30 seconds; a signal arrives while it is running. + scheduler.signal() + await vi.advanceTimersByTimeAsync(30_000) + calls[0]?.resolve() + await flushMicrotasks() + expect(task).toHaveBeenCalledTimes(1) + + // Trailing activity waits max(3s floor, 1x scan duration) = 30s. + await vi.advanceTimersByTimeAsync(29_999) + expect(task).toHaveBeenCalledTimes(1) + await vi.advanceTimersByTimeAsync(1) + expect(task).toHaveBeenCalledTimes(2) + + // The second scan is also slow; the next safety waits max(60s, 5x) = 150s. + await vi.advanceTimersByTimeAsync(30_000) + calls[1]?.resolve() + await flushMicrotasks() + await vi.advanceTimersByTimeAsync(149_999) + expect(task).toHaveBeenCalledTimes(2) + await vi.advanceTimersByTimeAsync(1) + expect(task).toHaveBeenCalledTimes(3) + }) + + it('aborts on pause and catches up after the obsolete request settles', async () => { + vi.useFakeTimers() + const calls: ReturnType[] = [] + const signals: AbortSignal[] = [] + const task = vi.fn(({ signal }: { signal: AbortSignal }) => { + const call = deferred() + calls.push(call) + signals.push(signal) + return call.promise + }) + const scheduler = createScheduler(task) + + scheduler.resumeSafety() + scheduler.pause() + expect(signals[0]?.aborted).toBe(true) + + scheduler.resumeSafety() + expect(task).toHaveBeenCalledTimes(1) + calls[0]?.resolve() + await flushMicrotasks() + + // Why: the reveal catch-up rides the paced activity lane (3s floor). + await vi.advanceTimersByTimeAsync(3000) + expect(task).toHaveBeenCalledTimes(2) + expect(signals[1]?.aborted).toBe(false) + calls[1]?.resolve() + await flushMicrotasks() + }) + + it('keeps huge-status signal mode active without a safety timeout', async () => { + vi.useFakeTimers() + const task = vi.fn(async () => {}) + const scheduler = createScheduler(task) + + scheduler.resumeSafety() + await flushMicrotasks() + scheduler.suspendSafety() + await vi.advanceTimersByTimeAsync(120_000) + expect(task).toHaveBeenCalledTimes(1) + + scheduler.signal() + await vi.advanceTimersByTimeAsync(125) + expect(task).toHaveBeenCalledTimes(2) + await flushMicrotasks() + await vi.advanceTimersByTimeAsync(120_000) + expect(task).toHaveBeenCalledTimes(2) + }) + + it('cleans up debounce and safety timers on dispose', async () => { + vi.useFakeTimers() + const task = vi.fn(async () => {}) + const scheduler = createScheduler(task) + + scheduler.resumeSafety() + await flushMicrotasks() + scheduler.signal() + scheduler.dispose() + await vi.advanceTimersByTimeAsync(120_000) + + expect(task).toHaveBeenCalledTimes(1) + }) +}) diff --git a/src/renderer/src/components/right-sidebar/git-status-refresh-scheduler.ts b/src/renderer/src/components/right-sidebar/git-status-refresh-scheduler.ts new file mode 100644 index 00000000000..1fab24ff107 --- /dev/null +++ b/src/renderer/src/components/right-sidebar/git-status-refresh-scheduler.ts @@ -0,0 +1,210 @@ +import { slowTaskRequiredIdleMs, type SlowTaskBackoffOptions } from './coalesced-poll-runner' + +export type GitStatusRefreshReason = 'activity' | 'safety' + +export type GitStatusRefreshScheduler = { + resumeSafety: () => void + pause: () => void + suspendSafety: () => void + signal: () => void + refreshNow: () => void + dispose: () => void +} + +type RefreshTask = (request: { + reason: GitStatusRefreshReason + signal: AbortSignal +}) => Promise + +export function createGitStatusRefreshScheduler( + task: RefreshTask, + options: { + safetyIntervalMs: number + activityDebounceMs: number + // Why: sustained signal streams (busy agent terminals, file churn) must + // never run status scans back-to-back (#7983). Activity runs keep this + // minimum idle gap since the previous run's end, scaled up by the slow + // task backoff when the previous scan itself was slow. + activityMinGapMs: number + slowTaskBackoff: SlowTaskBackoffOptions + } +): GitStatusRefreshScheduler { + let disposed = false + let safetyEnabled = false + let inFlight = false + let pendingActivity = false + let activityTimer: ReturnType | null = null + let activityTimerFiresAt = Infinity + let safetyTimer: ReturnType | null = null + let activeController: AbortController | null = null + let lastRunEndedAt = -Infinity + let lastRunDurationMs = 0 + + const clearActivityTimer = (): void => { + if (activityTimer !== null) { + clearTimeout(activityTimer) + activityTimer = null + activityTimerFiresAt = Infinity + } + } + const clearSafetyTimer = (): void => { + if (safetyTimer !== null) { + clearTimeout(safetyTimer) + safetyTimer = null + } + } + + const requiredActivityIdleMs = (): number => + slowTaskRequiredIdleMs( + lastRunDurationMs, + options.slowTaskBackoff.changeSignalMultiplier, + options.activityMinGapMs, + options.slowTaskBackoff.maxIntervalMs + ) + + const scheduleSafetyRun = (): void => { + // Why: slow scans stretch the safety horizon (idle-lane backoff) so a + // tens-of-seconds status scan never approaches a continuous git duty cycle. + const delay = Math.max( + options.safetyIntervalMs, + slowTaskRequiredIdleMs( + lastRunDurationMs, + options.slowTaskBackoff.idleMultiplier, + 0, + options.slowTaskBackoff.maxIntervalMs + ) + ) + safetyTimer = setTimeout(() => { + safetyTimer = null + startRun('safety') + }, delay) + } + + const scheduleActivityRun = (minDelayMs: number): void => { + if (disposed) { + return + } + if (inFlight) { + // Why: the trigger may describe a mutation after the running Git + // snapshot, so one trailing refresh is required for freshness. + pendingActivity = true + return + } + const now = Date.now() + const delay = Math.max(minDelayMs, lastRunEndedAt + requiredActivityIdleMs() - now) + if (delay <= 0) { + startRun('activity') + return + } + const firesAt = now + delay + if (activityTimer !== null) { + // Why: an earlier-eligible trigger may pull a pending run forward, but a + // later debounce window must never push an already-scheduled run back. + if (firesAt >= activityTimerFiresAt) { + return + } + clearActivityTimer() + } + activityTimerFiresAt = firesAt + activityTimer = setTimeout(() => { + activityTimer = null + activityTimerFiresAt = Infinity + startRun('activity') + }, delay) + } + + const startRun = (reason: GitStatusRefreshReason): void => { + if (disposed || inFlight) { + return + } + clearActivityTimer() + clearSafetyTimer() + inFlight = true + const startedAt = Date.now() + const controller = new AbortController() + activeController = controller + let result: Promise + try { + result = task({ reason, signal: controller.signal }) + } catch (error) { + result = Promise.reject(error) + } + void result + .catch(() => { + // Status refresh errors are transient; the next signal or safety run retries. + }) + .finally(() => { + lastRunEndedAt = Date.now() + lastRunDurationMs = Math.max(0, lastRunEndedAt - startedAt) + if (activeController === controller) { + activeController = null + } + inFlight = false + if (disposed) { + return + } + if (pendingActivity) { + pendingActivity = false + scheduleActivityRun(0) + return + } + if (safetyEnabled) { + scheduleSafetyRun() + } + }) + } + + return { + resumeSafety: () => { + if (disposed) { + return + } + const wasEnabled = safetyEnabled + safetyEnabled = true + if (wasEnabled) { + return + } + clearSafetyTimer() + if (inFlight) { + // Why: a request aborted while hidden cannot serve as reveal catch-up; + // retain one fresh run and wait for the old transport to settle. + pendingActivity ||= activeController?.signal.aborted === true + return + } + scheduleActivityRun(0) + }, + pause: () => { + safetyEnabled = false + pendingActivity = false + clearActivityTimer() + clearSafetyTimer() + activeController?.abort() + }, + suspendSafety: () => { + safetyEnabled = false + clearSafetyTimer() + }, + signal: () => { + if (disposed) { + return + } + clearSafetyTimer() + scheduleActivityRun(options.activityDebounceMs) + }, + refreshNow: () => { + if (disposed) { + return + } + clearSafetyTimer() + scheduleActivityRun(0) + }, + dispose: () => { + disposed = true + safetyEnabled = false + pendingActivity = false + clearActivityTimer() + clearSafetyTimer() + activeController?.abort() + } + } +} diff --git a/src/renderer/src/components/right-sidebar/git-status-refresh.test.ts b/src/renderer/src/components/right-sidebar/git-status-refresh.test.ts index 5fe28efa4be..3d9427ac1a4 100644 --- a/src/renderer/src/components/right-sidebar/git-status-refresh.test.ts +++ b/src/renderer/src/components/right-sidebar/git-status-refresh.test.ts @@ -233,6 +233,97 @@ describe('refreshGitStatusForWorktree', () => { expect(deps.setUpstreamStatus).toHaveBeenCalledWith('wt-race', strictStatus.upstreamStatus) }) + it('does not let an older automatic status overwrite the latest automatic result', async () => { + const olderStatus = deferred() + const latestStatus: GitStatusResult = { + entries: [{ path: 'latest.ts', status: 'modified', area: 'unstaged' }], + conflictOperation: 'unknown', + head: 'latest' + } + const gitStatus = vi + .fn() + .mockReturnValueOnce(olderStatus.promise) + .mockResolvedValueOnce(latestStatus) + vi.stubGlobal('window', { api: { git: { status: gitStatus } } }) + const deps = makeDeps() + + const olderRefresh = refreshGitStatusForWorktree({ + worktreeId: 'wt-automatic-race', + worktreePath: '/repo', + deps + }) + await vi.waitFor(() => expect(gitStatus).toHaveBeenCalledTimes(1)) + await refreshGitStatusForWorktree({ + worktreeId: 'wt-automatic-race', + worktreePath: '/repo', + deps + }) + olderStatus.resolve({ + entries: [{ path: 'older.ts', status: 'modified', area: 'unstaged' }], + conflictOperation: 'unknown', + head: 'older' + }) + await olderRefresh + + expect(deps.setGitStatus).toHaveBeenCalledTimes(1) + expect(deps.setGitStatus).toHaveBeenCalledWith('wt-automatic-race', latestStatus) + }) + + it('applies an earlier automatic result when a later automatic refresh fails', async () => { + const olderStatus = deferred() + const gitStatus = vi + .fn() + .mockReturnValueOnce(olderStatus.promise) + .mockRejectedValueOnce(new Error('transient index.lock')) + vi.stubGlobal('window', { api: { git: { status: gitStatus } } }) + const deps = makeDeps() + + const olderRefresh = refreshGitStatusForWorktree({ + worktreeId: 'wt-failed-veto', + worktreePath: '/repo', + deps + }) + await vi.waitFor(() => expect(gitStatus).toHaveBeenCalledTimes(1)) + await expect( + refreshGitStatusForWorktree({ + worktreeId: 'wt-failed-veto', + worktreePath: '/repo', + deps + }) + ).rejects.toThrow('transient index.lock') + + const freshStatus: GitStatusResult = { + entries: [{ path: 'fresh.ts', status: 'modified', area: 'unstaged' }], + conflictOperation: 'unknown', + head: 'fresh' + } + olderStatus.resolve(freshStatus) + await olderRefresh + + expect(deps.setGitStatus).toHaveBeenCalledWith('wt-failed-veto', freshStatus) + }) + + it('does not apply a status result after its liveness guard expires', async () => { + const status: GitStatusResult = { + entries: [], + conflictOperation: 'unknown', + head: 'stale' + } + vi.stubGlobal('window', { api: { git: { status: vi.fn().mockResolvedValue(status) } } }) + const deps = makeDeps() + + await refreshGitStatusForWorktree({ + worktreeId: 'wt-stale', + worktreePath: '/repo', + deps, + request: { shouldApply: () => false } + }) + + expect(deps.setGitStatus).not.toHaveBeenCalled() + expect(deps.updateWorktreeGitIdentity).not.toHaveBeenCalled() + expect(deps.fetchUpstreamStatus).not.toHaveBeenCalled() + }) + it('does not let an older automatic explicit upstream fetch overwrite a strict result', async () => { const automaticFetch = deferred() const strictStatus: GitStatusResult = { diff --git a/src/renderer/src/components/right-sidebar/git-status-refresh.ts b/src/renderer/src/components/right-sidebar/git-status-refresh.ts index d8dd54e5764..27e8fa30c35 100644 --- a/src/renderer/src/components/right-sidebar/git-status-refresh.ts +++ b/src/renderer/src/components/right-sidebar/git-status-refresh.ts @@ -11,6 +11,15 @@ import type { GitUpstreamStatus, GlobalSettings } from '../../../../shared/types' +import { + beginAutomaticUpstreamRefresh, + beginStrictUpstreamRefresh, + claimAutomaticUpstreamRefreshApply, + clearGitStatusRefreshOrderingStateForTests, + finishAutomaticUpstreamRefresh, + shouldApplyAutomaticUpstreamRefresh, + type AutomaticRefreshOrder +} from './git-status-refresh-ordering' export type GitStatusRefreshDeps = { setGitStatus: (worktreeId: string, status: GitStatusResult) => void @@ -31,44 +40,6 @@ export type GitStatusRefreshDeps = { ) => Promise } -const MAX_REFRESH_ORDERING_WORKTREES = 1024 -const strictUpstreamRefreshGenerationByWorktree = new Map() -const automaticUpstreamRefreshInFlightByWorktree = new Map() - -function trimRefreshOrderingState(): void { - for (const worktreeId of strictUpstreamRefreshGenerationByWorktree.keys()) { - if (strictUpstreamRefreshGenerationByWorktree.size <= MAX_REFRESH_ORDERING_WORKTREES) { - break - } - if (automaticUpstreamRefreshInFlightByWorktree.has(worktreeId)) { - continue - } - strictUpstreamRefreshGenerationByWorktree.delete(worktreeId) - } -} - -function beginAutomaticUpstreamRefresh(worktreeId: string): number { - automaticUpstreamRefreshInFlightByWorktree.set( - worktreeId, - (automaticUpstreamRefreshInFlightByWorktree.get(worktreeId) ?? 0) + 1 - ) - return strictUpstreamRefreshGenerationByWorktree.get(worktreeId) ?? 0 -} - -function finishAutomaticUpstreamRefresh(worktreeId: string): void { - const count = automaticUpstreamRefreshInFlightByWorktree.get(worktreeId) ?? 0 - if (count <= 1) { - automaticUpstreamRefreshInFlightByWorktree.delete(worktreeId) - } else { - automaticUpstreamRefreshInFlightByWorktree.set(worktreeId, count - 1) - } - trimRefreshOrderingState() -} - -function shouldApplyAutomaticUpstreamRefresh(worktreeId: string, startGeneration: number): boolean { - return (strictUpstreamRefreshGenerationByWorktree.get(worktreeId) ?? 0) === startGeneration -} - async function fetchAndApplyAutomaticUpstreamStatus({ settings, worktreeId, @@ -76,7 +47,8 @@ async function fetchAndApplyAutomaticUpstreamStatus({ connectionId, pushTarget, deps, - startGeneration + order, + shouldApply }: { settings?: Pick | null worktreeId: string @@ -84,8 +56,12 @@ async function fetchAndApplyAutomaticUpstreamStatus({ connectionId?: string pushTarget?: GitPushTarget deps: GitStatusRefreshDeps - startGeneration: number + order: AutomaticRefreshOrder + shouldApply?: () => boolean }): Promise { + if (!shouldApplyAutomaticUpstreamRefresh(worktreeId, order, shouldApply)) { + return null + } const upstreamStatus = await deps.fetchUpstreamStatus( worktreeId, worktreePath, @@ -110,24 +86,15 @@ async function fetchAndApplyAutomaticUpstreamStatus({ } return null } - if (!shouldApplyAutomaticUpstreamRefresh(worktreeId, startGeneration)) { + if (!claimAutomaticUpstreamRefreshApply(worktreeId, order, shouldApply)) { return null } deps.setUpstreamStatus(worktreeId, upstreamStatus) return upstreamStatus } -function beginStrictUpstreamRefresh(worktreeId: string): void { - strictUpstreamRefreshGenerationByWorktree.set( - worktreeId, - (strictUpstreamRefreshGenerationByWorktree.get(worktreeId) ?? 0) + 1 - ) - trimRefreshOrderingState() -} - export function clearGitStatusRefreshOrderingForTests(): void { - strictUpstreamRefreshGenerationByWorktree.clear() - automaticUpstreamRefreshInFlightByWorktree.clear() + clearGitStatusRefreshOrderingStateForTests() clearAutomaticPushTargetUpstreamStatusCache() } @@ -137,7 +104,8 @@ export async function refreshGitStatusForWorktree({ worktreePath, connectionId, pushTarget, - deps + deps, + request }: { settings?: Pick | null worktreeId: string @@ -145,17 +113,30 @@ export async function refreshGitStatusForWorktree({ connectionId?: string pushTarget?: GitPushTarget deps: GitStatusRefreshDeps + request?: { + reuseLineStats?: boolean + signal?: AbortSignal + shouldApply?: () => boolean + } }): Promise { - const upstreamStartGeneration = beginAutomaticUpstreamRefresh(worktreeId) + const refreshOrder = beginAutomaticUpstreamRefresh(worktreeId) try { - const status = (await getRuntimeGitStatus({ - settings, - worktreeId, - worktreePath, - connectionId - })) as GitStatusResult + const status = (await getRuntimeGitStatus( + { + settings, + worktreeId, + worktreePath, + connectionId + }, + request + ? { + ...(request.reuseLineStats === true ? { reuseLineStats: true } : {}), + ...(request.signal ? { signal: request.signal } : {}) + } + : undefined + )) as GitStatusResult - if (!shouldApplyAutomaticUpstreamRefresh(worktreeId, upstreamStartGeneration)) { + if (!claimAutomaticUpstreamRefreshApply(worktreeId, refreshOrder, request?.shouldApply)) { return } @@ -192,7 +173,8 @@ export async function refreshGitStatusForWorktree({ connectionId, pushTarget, deps, - startGeneration: upstreamStartGeneration + order: refreshOrder, + shouldApply: request?.shouldApply }) if (upstreamStatus) { // Why: explicit publish-target comparison can spawn several git @@ -219,11 +201,14 @@ export async function refreshGitStatusForWorktree({ worktreePath, connectionId, deps, - startGeneration: upstreamStartGeneration + order: refreshOrder, + shouldApply: request?.shouldApply }) return } - deps.setUpstreamStatus(worktreeId, status.upstreamStatus) + if (claimAutomaticUpstreamRefreshApply(worktreeId, refreshOrder, request?.shouldApply)) { + deps.setUpstreamStatus(worktreeId, status.upstreamStatus) + } return } await fetchAndApplyAutomaticUpstreamStatus({ @@ -233,7 +218,8 @@ export async function refreshGitStatusForWorktree({ connectionId, pushTarget, deps, - startGeneration: upstreamStartGeneration + order: refreshOrder, + shouldApply: request?.shouldApply }) } finally { finishAutomaticUpstreamRefresh(worktreeId) diff --git a/src/renderer/src/components/right-sidebar/stale-conflict-operation-poll.ts b/src/renderer/src/components/right-sidebar/stale-conflict-operation-poll.ts new file mode 100644 index 00000000000..08a925961fe --- /dev/null +++ b/src/renderer/src/components/right-sidebar/stale-conflict-operation-poll.ts @@ -0,0 +1,113 @@ +import { useEffect, useMemo } from 'react' +import type { GitConflictOperation, Repo } from '../../../../shared/types' +import { isGitRepoKind } from '../../../../shared/repo-kind' +import { getConnectionId } from '@/lib/connection-context' +import { getRuntimeGitConflictOperation } from '@/runtime/runtime-git-client' +import { createCoalescedPollRunner, type SlowTaskBackoffOptions } from './coalesced-poll-runner' +import { installWindowVisibilityInterval, isWindowVisible } from '@/lib/window-visibility-interval' +import { getRightSidebarWorktreeRuntimeSettings } from './file-explorer-runtime-owner' + +const CONFLICT_POLL_INTERVAL_MS = 3000 + +/** + * Polls the conflict operation of non-active worktrees whose sidebar badge + * still shows merge/rebase/cherry-pick, so the badge clears when the operation + * finishes. This is a lightweight fs-only check (no git status), so it stays + * cheap even with many worktrees. + */ +export function useStaleConflictOperationPolling(args: { + enabled: boolean + activeWorktreeId: string | null + allWorktrees: { id: string; path: string; repoId: string }[] + repoMap: Map> + conflictOperationByWorktree: Record + setConflictOperation: (worktreeId: string, operation: GitConflictOperation) => void + isConnectionReady: (connectionId: string | null | undefined) => boolean + slowTaskBackoff: SlowTaskBackoffOptions +}): void { + const { + enabled, + activeWorktreeId, + allWorktrees, + repoMap, + conflictOperationByWorktree, + setConflictOperation, + isConnectionReady, + slowTaskBackoff + } = args + // Why: only non-active worktrees with a known conflict operation need this + // probe — the full git status refresh already covers the active worktree. + const staleConflictWorktrees = useMemo(() => { + const result: { id: string; path: string }[] = [] + for (const [worktreeId, op] of Object.entries(conflictOperationByWorktree)) { + if (worktreeId === activeWorktreeId || op === 'unknown') { + continue + } + const worktree = allWorktrees.find((entry) => entry.id === worktreeId) + if (worktree) { + const repo = repoMap.get(worktree.repoId) + if (repo && !isGitRepoKind(repo)) { + continue + } + result.push({ id: worktree.id, path: worktree.path }) + } + } + return result + }, [allWorktrees, conflictOperationByWorktree, activeWorktreeId, repoMap]) + useEffect(() => { + if (!enabled) { + return + } + if (staleConflictWorktrees.length === 0) { + return + } + + const pollStale = async (): Promise => { + // Why: a backoff-deferred run can fire long after the window hides; skip + // the probe instead of running SSH/RPC work nobody can see. The + // becoming-visible run catches up via the change-signal lane. + if (!isWindowVisible()) { + return + } + for (const { id, path } of staleConflictWorktrees) { + try { + const connectionId = getConnectionId(id) ?? undefined + // Why: after explicit SSH disconnect the provider is intentionally + // gone; keep remote polling quiet until the target reconnects. + if (!isConnectionReady(connectionId)) { + continue + } + const op = (await getRuntimeGitConflictOperation({ + settings: getRightSidebarWorktreeRuntimeSettings(id), + worktreeId: id, + worktreePath: path, + connectionId + })) as GitConflictOperation + setConflictOperation(id, op) + } catch { + // ignore — worktree may have been removed + } + } + } + + // Why: remote conflict probes can exceed the 3s interval. Keep one poll in + // flight, coalesce skipped ticks into one trailing pass, and back off after + // slow probe chains so stale badges catch up without stacking SSH/RPC work. + const pollRunner = createCoalescedPollRunner(pollStale, { + slowTaskBackoff + }) + // Why: conflict badges are visible sidebar state; keep them fresh in + // visible unfocused windows, but do not poll disconnected hidden windows. + // The becoming-visible run rides the short-backoff lane so badges catch + // up promptly after a hidden stretch. + const stopVisiblePoll = installWindowVisibilityInterval({ + run: () => pollRunner.run(), + runOnVisible: () => pollRunner.run({ changeSignal: true }), + intervalMs: CONFLICT_POLL_INTERVAL_MS + }) + return () => { + pollRunner.dispose() + stopVisiblePoll() + } + }, [enabled, staleConflictWorktrees, setConflictOperation, isConnectionReady, slowTaskBackoff]) +} diff --git a/src/renderer/src/components/right-sidebar/useGitStatusPolling.rerender.test.ts b/src/renderer/src/components/right-sidebar/useGitStatusPolling.rerender.test.ts index 1d80a6f8e4a..83595ee7600 100644 --- a/src/renderer/src/components/right-sidebar/useGitStatusPolling.rerender.test.ts +++ b/src/renderer/src/components/right-sidebar/useGitStatusPolling.rerender.test.ts @@ -10,6 +10,8 @@ import { makeOpenFile, makeWorktree, TEST_REPO } from '@/store/slices/store-test import { ORCA_WORKTREE_FILE_CHANGE_EVENT } from '@/hooks/worktree-file-change-event' import { useGitStatusPolling } from './useGitStatusPolling' +globalThis.IS_REACT_ACT_ENVIRONMENT = true + // Mock the refresh boundary so we count invocations precisely without // triggering real IPC cascades (upstream probes, store mutations, etc.). const refreshMock = vi.hoisted(() => vi.fn()) @@ -50,7 +52,7 @@ function HookProbe(): null { return null } -async function renderHook(): Promise { +async function renderHook(): Promise { const container = document.createElement('div') document.body.appendChild(container) const root = createRoot(container) @@ -58,6 +60,7 @@ async function renderHook(): Promise { await act(async () => { root.render(createElement(HookProbe)) }) + return root } async function flushMicrotasks(): Promise { @@ -99,7 +102,7 @@ describe('useGitStatusPolling rerender stability', () => { vi.useRealTimers() }) - it('keeps the poll runner stable when openFiles changes mid-cooldown', async () => { + it('keeps the refresh scheduler stable when openFiles changes mid-signal debounce', async () => { // Spy on addEventListener so we can guard that the file-watch listener // registered before emitting — otherwise the test would prove nothing. const addSpy = vi.spyOn(window, 'addEventListener') @@ -111,10 +114,8 @@ describe('useGitStatusPolling rerender stability', () => { // run() once at install time). expect(refreshMock).toHaveBeenCalledTimes(1) - // Rerender: change openFiles in the store. This gives runFetchStatus a new - // identity (it lists openFiles in its useCallback deps). With the old - // useMemo([runFetchStatus]) the runner would be recreated here, resetting - // lastRunEndedAt to -Infinity and bypassing the cooldown. + // Rerender: change openFiles in the store while the file-watch/scheduler + // debounce is pending. The active-worktree scheduler must survive it. await act(async () => { useAppStore.setState({ openFiles: [makeOpenFile({ id: `${WORKTREE_PATH}/a.ts`, worktreeId: WORKTREE_ID })] @@ -139,18 +140,15 @@ describe('useGitStatusPolling rerender stability', () => { }) ) - // Advance past the 125 ms file-watch debounce. fetchStatus is called, but - // the runner's cooldown (POLL_INTERVAL_MS = 3000 ms from first run end) - // should suppress the second refresh. + // Advance past the 125 ms file-watch debounce. The scheduler coalesces the + // signal and paces it behind the 3 s anti-churn floor from the mount run. await vi.advanceTimersByTimeAsync(200) expect(refreshMock).toHaveBeenCalledTimes(1) - // Mid-cooldown: still no second refresh. - await vi.advanceTimersByTimeAsync(1300) + await vi.advanceTimersByTimeAsync(2799) expect(refreshMock).toHaveBeenCalledTimes(1) - // After the full 3 s cooldown the trailing refresh fires. - await vi.advanceTimersByTimeAsync(1500) + await vi.advanceTimersByTimeAsync(1) await flushMicrotasks() expect(refreshMock).toHaveBeenCalledTimes(2) @@ -175,4 +173,123 @@ describe('useGitStatusPolling rerender stability', () => { // without having to wait for the 3000ms timer. expect(refreshMock).toHaveBeenCalledTimes(2) }) + + it('refreshes immediately when Source Control becomes visible', async () => { + useAppStore.setState({ rightSidebarOpen: false }) + await renderHook() + await flushMicrotasks() + expect(refreshMock).toHaveBeenCalledTimes(1) + + // Let the 3 s anti-churn floor from the mount refresh elapse first. + await vi.advanceTimersByTimeAsync(3000) + await act(async () => { + useAppStore.setState({ rightSidebarOpen: true }) + }) + await flushMicrotasks() + + expect(refreshMock).toHaveBeenCalledTimes(2) + }) + + it('refreshes immediately when the Files tab becomes visible', async () => { + useAppStore.setState({ rightSidebarOpen: false, rightSidebarTab: 'explorer' }) + await renderHook() + await flushMicrotasks() + expect(refreshMock).toHaveBeenCalledTimes(1) + + // Let the 3 s anti-churn floor from the mount refresh elapse first. + await vi.advanceTimersByTimeAsync(3000) + await act(async () => { + useAppStore.setState({ rightSidebarOpen: true }) + }) + await flushMicrotasks() + + expect(refreshMock).toHaveBeenCalledTimes(2) + }) + + it('refreshes immediately when an SSH execution host reconnects', async () => { + useAppStore.setState({ + repos: [{ ...repo, connectionId: 'ssh-1' }], + sshConnectionStates: new Map([ + ['ssh-1', { status: 'disconnected', error: null, reconnectAttempt: 0 }] + ]) + } as Partial) + await renderHook() + await flushMicrotasks() + expect(refreshMock).not.toHaveBeenCalled() + + await act(async () => { + useAppStore.setState({ + sshConnectionStates: new Map([ + ['ssh-1', { status: 'connected', error: null, reconnectAttempt: 0 }] + ]) + } as Partial) + }) + await flushMicrotasks() + + expect(refreshMock).toHaveBeenCalledTimes(1) + }) + + it('marks only the 60-second safety refresh for line-stat reuse', async () => { + await renderHook() + await flushMicrotasks() + + expect(refreshMock.mock.calls[0]?.[0].request.reuseLineStats).toBeUndefined() + await vi.advanceTimersByTimeAsync(60_000) + await flushMicrotasks() + + expect(refreshMock).toHaveBeenCalledTimes(2) + expect(refreshMock.mock.calls[1]?.[0].request.reuseLineStats).toBe(true) + }) + + it('aborts and rejects stale work when the execution host changes', async () => { + let resolveFirst!: () => void + const firstRefresh = new Promise((resolve) => { + resolveFirst = resolve + }) + let firstRequest!: { signal: AbortSignal; shouldApply: () => boolean } + refreshMock.mockImplementationOnce( + (args: { request: { signal: AbortSignal; shouldApply: () => boolean } }) => { + firstRequest = args.request + return firstRefresh + } + ) + await renderHook() + expect(refreshMock).toHaveBeenCalledTimes(1) + + await act(async () => { + useAppStore.setState({ + settings: { activeRuntimeEnvironmentId: 'env-2' } as AppState['settings'] + }) + }) + await flushMicrotasks() + + expect(firstRequest.signal.aborted).toBe(true) + expect(firstRequest.shouldApply()).toBe(false) + expect(refreshMock).toHaveBeenCalledTimes(2) + resolveFirst() + await flushMicrotasks() + }) + + it('aborts and rejects stale work on unmount', async () => { + let resolveFirst!: () => void + const firstRefresh = new Promise((resolve) => { + resolveFirst = resolve + }) + let firstRequest!: { signal: AbortSignal; shouldApply: () => boolean } + refreshMock.mockImplementationOnce( + (args: { request: { signal: AbortSignal; shouldApply: () => boolean } }) => { + firstRequest = args.request + return firstRefresh + } + ) + const root = await renderHook() + + act(() => root.unmount()) + roots.splice(roots.indexOf(root), 1) + + expect(firstRequest.signal.aborted).toBe(true) + expect(firstRequest.shouldApply()).toBe(false) + resolveFirst() + await flushMicrotasks() + }) }) diff --git a/src/renderer/src/components/right-sidebar/useGitStatusPolling.test.ts b/src/renderer/src/components/right-sidebar/useGitStatusPolling.test.ts index 44efe18be8b..9b226fb012a 100644 --- a/src/renderer/src/components/right-sidebar/useGitStatusPolling.test.ts +++ b/src/renderer/src/components/right-sidebar/useGitStatusPolling.test.ts @@ -238,7 +238,7 @@ describe('useGitStatusPolling', () => { expect(globalThis.setInterval).not.toHaveBeenCalled() }) - it('uses a slower git status cadence when only terminal branch detection needs polling', async () => { + it('uses the safety scheduler instead of an interval for terminal-only branch detection', async () => { const { gitStatus } = await usePollingOnce( { entries: [], @@ -255,7 +255,7 @@ describe('useGitStatusPolling', () => { ) expect(gitStatus).toHaveBeenCalledTimes(1) - expect(globalThis.setInterval).toHaveBeenCalledWith(expect.any(Function), 30_000) + expect(globalThis.setInterval).not.toHaveBeenCalled() }) it('does not install the visible git status poll while disabled', async () => { @@ -411,6 +411,7 @@ describe('useGitStatusPolling', () => { await vi.advanceTimersByTimeAsync(125) expect(gitStatus).toHaveBeenCalledTimes(1) + // Why: evidence refreshes keep the 3s anti-churn floor from the last run. await vi.advanceTimersByTimeAsync(2875) await vi.waitFor(() => expect(gitStatus).toHaveBeenCalledTimes(2)) expect(window.api.fs.watchWorktree).not.toHaveBeenCalled() @@ -652,6 +653,7 @@ describe('useGitStatusPolling', () => { await vi.advanceTimersByTimeAsync(65) expect(gitStatus).toHaveBeenCalledTimes(callsBeforeDebounceFires) + // Why: evidence refreshes keep the 3s anti-churn floor from the last run. await vi.advanceTimersByTimeAsync(2875) await vi.waitFor(() => expect(gitStatus).toHaveBeenCalledTimes(callsBeforeDebounceFires + 1)) @@ -728,7 +730,6 @@ describe('useGitStatusPolling', () => { it('does not overlap slow visible git status polls and runs one trailing refresh', async () => { vi.resetModules() vi.useFakeTimers() - let intervalCallback: (() => void) | null = null let resolveFirst!: (value: GitStatusResult) => void const firstStatus = new Promise((resolve) => { resolveFirst = resolve @@ -804,27 +805,22 @@ describe('useGitStatusPolling', () => { addEventListener: vi.fn(), removeEventListener: vi.fn() }) - vi.stubGlobal( - 'setInterval', - vi.fn((callback: () => void) => { - intervalCallback = callback - return 1 - }) - ) + vi.stubGlobal('setInterval', vi.fn()) vi.stubGlobal('clearInterval', vi.fn()) const { useGitStatusPolling: runPolling } = await import('./useGitStatusPolling') GitStatusPollingHarness({ runPolling }) await vi.waitFor(() => expect(gitStatus).toHaveBeenCalledTimes(1)) - expect(intervalCallback).toBeTypeOf('function') - const tick = intervalCallback as unknown as () => void - tick() - tick() + const onChanged = window.api.worktrees.onChanged as ReturnType + const handleRepoSignal = onChanged.mock.calls[0][0] as (payload: { repoId: string }) => void + handleRepoSignal({ repoId: repo.id }) + handleRepoSignal({ repoId: repo.id }) expect(gitStatus).toHaveBeenCalledTimes(1) resolveFirst(status) await vi.waitFor(() => expect(state.setGitStatus).toHaveBeenCalledTimes(1)) + // Why: the trailing refresh respects the 3s anti-churn floor after settle. await vi.advanceTimersByTimeAsync(3000) await vi.waitFor(() => expect(gitStatus).toHaveBeenCalledTimes(2)) await vi.waitFor(() => expect(state.setGitStatus).toHaveBeenCalledTimes(2)) diff --git a/src/renderer/src/components/right-sidebar/useGitStatusPolling.ts b/src/renderer/src/components/right-sidebar/useGitStatusPolling.ts index acb8d464cbb..37e8f8f63c5 100644 --- a/src/renderer/src/components/right-sidebar/useGitStatusPolling.ts +++ b/src/renderer/src/components/right-sidebar/useGitStatusPolling.ts @@ -1,13 +1,11 @@ -import { useCallback, useEffect, useMemo, useRef } from 'react' +import { useCallback, useEffect, useRef } from 'react' import { useAppStore } from '@/store' import { useAllWorktrees, useRepoById, useRepoMap, useWorktreeById } from '@/store/selectors' -import type { GitConflictOperation } from '../../../../shared/types' import { isGitRepoKind } from '../../../../shared/repo-kind' import { getConnectionId } from '@/lib/connection-context' -import { getRuntimeGitConflictOperation } from '@/runtime/runtime-git-client' +import { getExecutionHostIdForWorktree } from '@/lib/worktree-runtime-owner' import { refreshGitStatusForWorktree } from './git-status-refresh' -import { type CoalescedPollRunner, createCoalescedPollRunner } from './coalesced-poll-runner' -import { installWindowVisibilityInterval, isWindowVisible } from '@/lib/window-visibility-interval' +import { isWindowVisible } from '@/lib/window-visibility-interval' import { hasInteractiveActiveGitStatusConsumer, shouldPollActiveGitStatus @@ -15,18 +13,20 @@ import { import { getRightSidebarWorktreeRuntimeSettings } from './file-explorer-runtime-owner' import { useGitStatusFileWatchRefresh } from './git-status-file-watch-refresh' import { useGitStatusPushSignalRefresh } from './git-status-push-signal-refresh' +import { useStaleConflictOperationPolling } from './stale-conflict-operation-poll' +import { + createGitStatusRefreshScheduler, + type GitStatusRefreshReason, + type GitStatusRefreshScheduler +} from './git-status-refresh-scheduler' -const MIN_STATUS_REFRESH_INTERVAL_MS = 3000 -const INTERACTIVE_STATUS_POLL_INTERVAL_MS = MIN_STATUS_REFRESH_INTERVAL_MS -// Why: file-watch refreshes cover content changes and push signals (repo -// metadata watch, shell command completion) cover branch switches; the -// terminal-only poll is a last-resort backstop for shells without either. -const TERMINAL_ONLY_STATUS_POLL_INTERVAL_MS = 30_000 -// Why: on a large monorepo one status refresh can take tens of seconds, so the -// fixed 3s gap kept a git process running almost continuously while the -// workspace was idle (#7983). Evidence-free ticks wait 5x the previous poll -// duration (~1/6 duty cycle); change signals wait only 1x so real changes in a -// slow repo still surface promptly; the cap bounds worst-case staleness. +const STATUS_SAFETY_INTERVAL_MS = 60_000 +const STATUS_ACTIVITY_DEBOUNCE_MS = 125 +// Why: evidence-driven status refreshes must keep the pre-scheduler floor so +// sustained terminal/file signals can never run git back-to-back (#7983). +const STATUS_ACTIVITY_MIN_GAP_MS = 3000 +// Why: status scans and remote conflict probes can take longer than their +// timers; duration-aware spacing prevents a slow task from running nonstop. const SLOW_GIT_POLL_BACKOFF = { idleMultiplier: 5, changeSignalMultiplier: 1, @@ -37,6 +37,9 @@ export function useGitStatusPolling(options: { enabled?: boolean } = {}): void { const enabled = options.enabled ?? true const activeWorktreeId = useAppStore((s) => s.activeWorktreeId) const activeWorktree = useWorktreeById(activeWorktreeId) + const activeExecutionHostId = useAppStore((s) => + getExecutionHostIdForWorktree(s, activeWorktreeId) + ) const allWorktrees = useAllWorktrees() const updateWorktreeGitIdentity = useAppStore((s) => s.updateWorktreeGitIdentity) const setGitStatus = useAppStore((s) => s.setGitStatus) @@ -87,143 +90,167 @@ export function useGitStatusPolling(options: { enabled?: boolean } = {}): void { canFetchActiveWorktreeGitStatus && !!activeWorktreeId && !gitStatusHugeByWorktree?.[activeWorktreeId] - const activeStatusPollIntervalMs = hasInteractiveActiveGitStatusConsumer( + const activeStatusPollScope = shouldPollActiveWorktreeGitStatus + ? `${activeExecutionHostId}\0${activeWorktreeId}\0${worktreePath}` + : null + // Why: opening any git-status consumer (Source Control, Files, Checks, or an + // editor file) must refresh promptly, matching the pre-scheduler behavior + // where the interactive interval flip re-ran an immediate poll. + const interactiveConsumerVisible = hasInteractiveActiveGitStatusConsumer( activeGitStatusPollingArgs ) - ? INTERACTIVE_STATUS_POLL_INTERVAL_MS - : TERMINAL_ONLY_STATUS_POLL_INTERVAL_MS - const activeStatusPollScope = shouldPollActiveWorktreeGitStatus ? activeWorktreeId : null - // Why: build a list of non-active worktrees that still have a known conflict - // operation (merge/rebase/cherry-pick). These need lightweight polling so - // their sidebar badges clear when the operation finishes — the full git status - // poll only covers the active worktree. - const staleConflictWorktrees = useMemo(() => { - const result: { id: string; path: string }[] = [] - for (const [worktreeId, op] of Object.entries(conflictOperationByWorktree)) { - if (worktreeId === activeWorktreeId || op === 'unknown') { - continue + const runFetchStatus = useCallback( + async (request: { + reason: GitStatusRefreshReason + signal: AbortSignal + shouldApply: () => boolean + }) => { + // Why: eligibility can change between timer dispatch and task start. + // Avoid launching work that its liveness guard would have to discard. + if ( + request.signal.aborted || + !isWindowVisible() || + !canFetchActiveWorktreeGitStatus || + !activeWorktreeId || + !worktreePath + ) { + return } - const worktree = allWorktrees.find((entry) => entry.id === worktreeId) - if (worktree) { - const repo = repoMap.get(worktree.repoId) - if (repo && !isGitRepoKind(repo)) { - continue - } - result.push({ id: worktree.id, path: worktree.path }) + try { + const connectionId = getConnectionId(activeWorktreeId) ?? undefined + await refreshGitStatusForWorktree({ + settings: getRightSidebarWorktreeRuntimeSettings(activeWorktreeId), + worktreeId: activeWorktreeId, + worktreePath, + connectionId, + pushTarget: activePushTarget, + deps: { + setGitStatus, + updateWorktreeGitIdentity, + setUpstreamStatus, + fetchUpstreamStatus + }, + request: { + ...(request.reason === 'safety' ? { reuseLineStats: true } : {}), + signal: request.signal, + shouldApply: request.shouldApply + } + }) + } catch { + // ignore } - } - return result - }, [allWorktrees, conflictOperationByWorktree, activeWorktreeId, repoMap]) + }, + [ + activePushTarget, + activeWorktreeId, + fetchUpstreamStatus, + canFetchActiveWorktreeGitStatus, + worktreePath, + setGitStatus, + setUpstreamStatus, + updateWorktreeGitIdentity + ] + ) - const runFetchStatus = useCallback(async () => { - // Why: a backoff-deferred run can fire long after the window hides; skip - // the scan instead of running tens of seconds of git work nobody can see. - // The becoming-visible run catches up via the change-signal lane. - if (!isWindowVisible()) { - return - } - if (!canFetchActiveWorktreeGitStatus || !activeWorktreeId || !worktreePath) { - return - } - try { - const connectionId = getConnectionId(activeWorktreeId) ?? undefined - await refreshGitStatusForWorktree({ - settings: getRightSidebarWorktreeRuntimeSettings(activeWorktreeId), - worktreeId: activeWorktreeId, - worktreePath, - connectionId, - pushTarget: activePushTarget, - deps: { - setGitStatus, - updateWorktreeGitIdentity, - setUpstreamStatus, - fetchUpstreamStatus - } - }) - } catch { - // ignore - } - }, [ - activePushTarget, - activeWorktreeId, - fetchUpstreamStatus, - canFetchActiveWorktreeGitStatus, - worktreePath, - setGitStatus, - setUpstreamStatus, - updateWorktreeGitIdentity - ]) - - // Why: the runner must survive rerenders so `lastRunEndedAt` and `inFlight` - // are never reset by a UI-state change mid-burst (e.g. openFiles update while - // git is still running). A ref keeps one runner per active-worktree lifetime; - // `runFetchStatusRef` lets the runner always call the latest closure without - // being recreated. The runner is disposed and replaced only when the active - // worktree changes or the hook unmounts. + // Why: the scheduler must survive harmless store rerenders so its in-flight, + // trailing-signal, and safety-horizon state remain authoritative. const runFetchStatusRef = useRef(runFetchStatus) runFetchStatusRef.current = runFetchStatus + const canApplyScheduledStatusRef = useRef(canFetchActiveWorktreeGitStatus) + canApplyScheduledStatusRef.current = canFetchActiveWorktreeGitStatus + const statusRefreshGenerationRef = useRef(0) - const statusPollRunnerRef = useRef(null) + const statusSchedulerRef = useRef(null) useEffect(() => { - const runner = createCoalescedPollRunner(() => runFetchStatusRef.current(), { - minIntervalMs: MIN_STATUS_REFRESH_INTERVAL_MS, - slowTaskBackoff: SLOW_GIT_POLL_BACKOFF - }) - statusPollRunnerRef.current = runner + const generation = ++statusRefreshGenerationRef.current + const scheduler = createGitStatusRefreshScheduler( + ({ reason, signal }) => + runFetchStatusRef.current({ + reason, + signal, + shouldApply: () => + statusRefreshGenerationRef.current === generation && + canApplyScheduledStatusRef.current && + !signal.aborted && + isWindowVisible() + }), + { + safetyIntervalMs: STATUS_SAFETY_INTERVAL_MS, + activityDebounceMs: STATUS_ACTIVITY_DEBOUNCE_MS, + activityMinGapMs: STATUS_ACTIVITY_MIN_GAP_MS, + slowTaskBackoff: SLOW_GIT_POLL_BACKOFF + } + ) + statusSchedulerRef.current = scheduler return () => { - runner.dispose() - statusPollRunnerRef.current = null - } - }, [activeWorktreeId]) - - const fetchStatus = useCallback(() => { - statusPollRunnerRef.current?.run() - }, []) - - // Why: file-watch and push signals carry evidence something changed, so they - // take the runner's short-backoff lane instead of evidence-free tick pacing. - const fetchStatusOnChangeSignal = useCallback(() => { - statusPollRunnerRef.current?.run({ changeSignal: true }) - }, []) - - // Why: while huge, the visibility interval below is not installed, and push - // signals are dropped while hidden — e.g. an agent committing behind a - // minimized window. Catch up on reveal so the huge flag can still clear. - const hugeStatusPauseActive = canFetchActiveWorktreeGitStatus && !activeStatusPollScope - useEffect(() => { - if ( - !hugeStatusPauseActive || - typeof document === 'undefined' || - typeof document.addEventListener !== 'function' - ) { - return - } - const catchUpOnReveal = (): void => { - if (isWindowVisible()) { - fetchStatusOnChangeSignal() + statusRefreshGenerationRef.current += 1 + scheduler.dispose() + if (statusSchedulerRef.current === scheduler) { + statusSchedulerRef.current = null } } - document.addEventListener('visibilitychange', catchUpOnReveal) - return () => { - document.removeEventListener('visibilitychange', catchUpOnReveal) - } - }, [hugeStatusPauseActive, fetchStatusOnChangeSignal]) + }, [activeExecutionHostId, activeWorktreeId, worktreePath]) useEffect(() => { - if (!activeStatusPollScope) { + const reconcile = (catchUp: boolean): void => { + const scheduler = statusSchedulerRef.current + if (!scheduler) { + return + } + if (!canFetchActiveWorktreeGitStatus || !isWindowVisible()) { + scheduler.pause() + return + } + if (activeStatusPollScope) { + scheduler.resumeSafety() + return + } + scheduler.suspendSafety() + if (catchUp) { + scheduler.refreshNow() + } + } + reconcile(false) + if (typeof document === 'undefined' || typeof document.addEventListener !== 'function') { return } - // Why: this root-level poll should pause while hidden, but visible - // unfocused windows still need fresh status for second-display workflows. - // Change signals are dropped while hidden, so the becoming-visible run - // rides the short-backoff lane to catch up on anything that was missed. - return installWindowVisibilityInterval({ - run: fetchStatus, - runOnVisible: fetchStatusOnChangeSignal, - intervalMs: activeStatusPollIntervalMs - }) - }, [activeStatusPollIntervalMs, activeStatusPollScope, fetchStatus, fetchStatusOnChangeSignal]) + const handleVisibilityChange = (): void => reconcile(isWindowVisible()) + document.addEventListener('visibilitychange', handleVisibilityChange) + return () => { + document.removeEventListener('visibilitychange', handleVisibilityChange) + } + }, [activeStatusPollScope, canFetchActiveWorktreeGitStatus]) + + const previousConsumerVisibilityRef = useRef({ + worktreeId: activeWorktreeId, + visible: interactiveConsumerVisible, + canFetch: canFetchActiveWorktreeGitStatus + }) + useEffect(() => { + const previous = previousConsumerVisibilityRef.current + if ( + interactiveConsumerVisible && + previous.worktreeId === activeWorktreeId && + !previous.visible && + previous.canFetch && + canFetchActiveWorktreeGitStatus && + isWindowVisible() + ) { + statusSchedulerRef.current?.refreshNow() + } + previousConsumerVisibilityRef.current = { + worktreeId: activeWorktreeId, + visible: interactiveConsumerVisible, + canFetch: canFetchActiveWorktreeGitStatus + } + }, [activeWorktreeId, canFetchActiveWorktreeGitStatus, interactiveConsumerVisible]) + + // Why: file, terminal, and metadata evidence share one scheduler window so + // a single Git operation cannot fan out into several status subprocesses. + const fetchStatusOnChangeSignal = useCallback(() => { + statusSchedulerRef.current?.signal() + }, []) useGitStatusFileWatchRefresh({ activeConnectionId, @@ -247,63 +274,14 @@ export function useGitStatusPolling(options: { enabled?: boolean } = {}): void { fetchStatus: fetchStatusOnChangeSignal }) - // Why: poll conflict operation for non-active worktrees that have a stale - // non-unknown operation. This is a lightweight fs-only check (no git status) - // so it won't cause performance issues even with many worktrees. - useEffect(() => { - if (!enabled) { - return - } - if (staleConflictWorktrees.length === 0) { - return - } - - const pollStale = async (): Promise => { - // Why: a backoff-deferred run can fire long after the window hides; skip - // the probe instead of running SSH/RPC work nobody can see. The - // becoming-visible run catches up via the change-signal lane. - if (!isWindowVisible()) { - return - } - for (const { id, path } of staleConflictWorktrees) { - try { - const connectionId = getConnectionId(id) ?? undefined - // Why: after explicit SSH disconnect the provider is intentionally - // gone; keep remote polling quiet until the target reconnects. - if (!isConnectionReady(connectionId)) { - continue - } - const op = (await getRuntimeGitConflictOperation({ - settings: getRightSidebarWorktreeRuntimeSettings(id), - worktreeId: id, - worktreePath: path, - connectionId - })) as GitConflictOperation - setConflictOperation(id, op) - } catch { - // ignore — worktree may have been removed - } - } - } - - // Why: remote conflict probes can exceed the 3s interval. Keep one poll in - // flight, coalesce skipped ticks into one trailing pass, and back off after - // slow probe chains so stale badges catch up without stacking SSH/RPC work. - const pollRunner = createCoalescedPollRunner(pollStale, { - slowTaskBackoff: SLOW_GIT_POLL_BACKOFF - }) - // Why: conflict badges are visible sidebar state; keep them fresh in - // visible unfocused windows, but do not poll disconnected hidden windows. - // The becoming-visible run rides the short-backoff lane so badges catch - // up promptly after a hidden stretch. - const stopVisiblePoll = installWindowVisibilityInterval({ - run: () => pollRunner.run(), - runOnVisible: () => pollRunner.run({ changeSignal: true }), - intervalMs: MIN_STATUS_REFRESH_INTERVAL_MS - }) - return () => { - pollRunner.dispose() - stopVisiblePoll() - } - }, [enabled, staleConflictWorktrees, setConflictOperation, isConnectionReady]) + useStaleConflictOperationPolling({ + enabled, + activeWorktreeId, + allWorktrees, + repoMap, + conflictOperationByWorktree, + setConflictOperation, + isConnectionReady, + slowTaskBackoff: SLOW_GIT_POLL_BACKOFF + }) } diff --git a/src/renderer/src/runtime/abortable-runtime-environment-call.ts b/src/renderer/src/runtime/abortable-runtime-environment-call.ts new file mode 100644 index 00000000000..88b9dc9d6fb --- /dev/null +++ b/src/renderer/src/runtime/abortable-runtime-environment-call.ts @@ -0,0 +1,65 @@ +import type { RuntimeRpcResponse } from '../../../shared/runtime-rpc-envelope' + +export function createRuntimeRpcAbortError(): Error { + const error = new Error('Runtime request aborted') + error.name = 'AbortError' + return error +} + +export async function callAbortableRuntimeEnvironment( + environmentId: string, + method: string, + params: unknown, + timeoutMs: number | undefined, + signal: AbortSignal +): Promise> { + if (signal.aborted) { + throw createRuntimeRpcAbortError() + } + // Why: the one-shot runtime call bridge cannot cancel host work; the + // subscription bridge closes its request context when we unsubscribe. + return new Promise((resolve, reject) => { + let handle: { unsubscribe: () => void } | null = null + let settled = false + // Why: the subscription transport's own timeout only bounds subscription + // start, not the response. Keep the one-shot call path's response deadline + // so a connected-but-unresponsive runtime cannot stall the caller forever. + const deadline = + timeoutMs === undefined + ? null + : setTimeout(() => { + finish(() => reject(new Error(`Runtime request timed out before ${method} completed`))) + }, timeoutMs) + const finish = (complete: () => void): void => { + if (settled) { + return + } + settled = true + if (deadline !== null) { + clearTimeout(deadline) + } + signal.removeEventListener('abort', onAbort) + handle?.unsubscribe() + complete() + } + const onAbort = (): void => finish(() => reject(createRuntimeRpcAbortError())) + signal.addEventListener('abort', onAbort, { once: true }) + void window.api.runtimeEnvironments + .subscribe( + { selector: environmentId, method, params, timeoutMs }, + { + onResponse: (response) => finish(() => resolve(response)), + onError: (error) => finish(() => reject(new Error(error.message))), + onClose: () => + finish(() => reject(new Error(`Runtime request closed before ${method} completed`))) + } + ) + .then((subscription) => { + handle = subscription + if (settled) { + subscription.unsubscribe() + } + }) + .catch((error) => finish(() => reject(error))) + }) +} diff --git a/src/renderer/src/runtime/runtime-git-client.test.ts b/src/renderer/src/runtime/runtime-git-client.test.ts index 64c12946a4b..63b70a48029 100644 --- a/src/renderer/src/runtime/runtime-git-client.test.ts +++ b/src/renderer/src/runtime/runtime-git-client.test.ts @@ -23,6 +23,7 @@ import { import { clearRuntimeCompatibilityCacheForTests } from './runtime-rpc-client' const gitStatus = vi.fn() +const gitCancelStatus = vi.fn() const gitCheckIgnored = vi.fn() const gitSubmoduleStatus = vi.fn() const gitDiff = vi.fn() @@ -44,6 +45,8 @@ const runtimeCall = vi.fn() beforeEach(() => { clearRuntimeCompatibilityCacheForTests() gitStatus.mockReset() + gitCancelStatus.mockReset() + gitCancelStatus.mockResolvedValue(undefined) gitCheckIgnored.mockReset() gitSubmoduleStatus.mockReset() gitDiff.mockReset() @@ -68,6 +71,7 @@ beforeEach(() => { api: { git: { status: gitStatus, + cancelStatus: gitCancelStatus, checkIgnored: gitCheckIgnored, submoduleStatus: gitSubmoduleStatus, diff: gitDiff, @@ -204,6 +208,39 @@ describe('runtime git client', () => { }) }) + it('forwards line-stat reuse to local status and cancels tokenized work on abort', async () => { + const controller = new AbortController() + let resolveStatus!: (value: { entries: never[]; conflictOperation: string }) => void + gitStatus.mockImplementation( + () => + new Promise((resolve) => { + resolveStatus = resolve + }) + ) + + const request = getRuntimeGitStatus( + { + settings: { activeRuntimeEnvironmentId: null }, + worktreeId: 'wt-1', + worktreePath: '/repo' + }, + { reuseLineStats: true, signal: controller.signal } + ) + await vi.waitFor(() => expect(gitStatus).toHaveBeenCalled()) + const statusArgs = gitStatus.mock.calls[0]?.[0] as { requestToken?: string } + controller.abort() + resolveStatus({ entries: [], conflictOperation: 'unknown' }) + await request + + expect(gitStatus).toHaveBeenCalledWith({ + worktreePath: '/repo', + connectionId: undefined, + reuseLineStats: true, + requestToken: expect.any(String) + }) + expect(gitCancelStatus).toHaveBeenCalledWith({ requestToken: statusArgs.requestToken }) + }) + it('checks ignored paths through local git IPC', async () => { gitCheckIgnored.mockResolvedValue(['dist/bundle.js']) @@ -408,6 +445,57 @@ describe('runtime git client', () => { }) }) + it('keeps safety (reuse) refreshes on the pooled call transport even with a signal', async () => { + const controller = new AbortController() + runtimeEnvironmentCall.mockResolvedValue({ + id: 'rpc-1', + ok: true, + result: { entries: [], conflictOperation: 'unknown' }, + _meta: { runtimeId: 'remote-runtime' } + }) + + await getRuntimeGitStatus( + { + settings: { activeRuntimeEnvironmentId: 'env-1' }, + worktreeId: 'wt-1', + worktreePath: '/repo' + }, + { reuseLineStats: true, signal: controller.signal } + ) + + expect(runtimeEnvironmentCall).toHaveBeenCalledWith({ + selector: 'env-1', + method: 'git.status', + params: { worktree: 'id:wt-1', reuseLineStats: true }, + timeoutMs: 15_000 + }) + }) + + it('forwards line-stat reuse through the active runtime environment', async () => { + runtimeEnvironmentCall.mockResolvedValue({ + id: 'rpc-1', + ok: true, + result: { entries: [], conflictOperation: 'unknown' }, + _meta: { runtimeId: 'remote-runtime' } + }) + + await getRuntimeGitStatus( + { + settings: { activeRuntimeEnvironmentId: 'env-1' }, + worktreeId: 'wt-1', + worktreePath: '/repo' + }, + { reuseLineStats: true } + ) + + expect(runtimeEnvironmentCall).toHaveBeenCalledWith({ + selector: 'env-1', + method: 'git.status', + params: { worktree: 'id:wt-1', reuseLineStats: true }, + timeoutMs: 15_000 + }) + }) + it('checks ignored paths through the active runtime environment', async () => { runtimeEnvironmentCall.mockResolvedValue({ id: 'rpc-1', diff --git a/src/renderer/src/runtime/runtime-git-client.ts b/src/renderer/src/runtime/runtime-git-client.ts index 972e2b7fdcb..12779449d91 100644 --- a/src/renderer/src/runtime/runtime-git-client.ts +++ b/src/renderer/src/runtime/runtime-git-client.ts @@ -133,20 +133,30 @@ export function getRuntimeGitScope( export async function getRuntimeGitStatus( context: RuntimeGitContext, - options?: { includeIgnored?: boolean; bypassEffectiveUpstreamNegativeCache?: boolean } + options?: { + includeIgnored?: boolean + bypassEffectiveUpstreamNegativeCache?: boolean + reuseLineStats?: boolean + signal?: AbortSignal + } ): Promise { const target = getActiveRuntimeTarget(context.settings) const includeIgnoredArgs = options?.includeIgnored ? { includeIgnored: true } : {} const upstreamCacheBypassArgs = options?.bypassEffectiveUpstreamNegativeCache ? { bypassEffectiveUpstreamNegativeCache: true } : {} + const lineStatsReuseArgs = options?.reuseLineStats ? { reuseLineStats: true } : {} if (target.kind === 'local' || !context.worktreeId) { - return window.api.git.status({ - worktreePath: resolveLocalWorktreePath(context), - connectionId: context.connectionId, - ...includeIgnoredArgs, - ...upstreamCacheBypassArgs - }) + return callLocalGitStatus( + { + worktreePath: resolveLocalWorktreePath(context), + connectionId: context.connectionId, + ...includeIgnoredArgs, + ...upstreamCacheBypassArgs, + ...lineStatsReuseArgs + }, + options?.signal + ) } return callRuntimeRpc( target, @@ -154,12 +164,48 @@ export async function getRuntimeGitStatus( { worktree: toRuntimeWorktreeSelector(context.worktreeId), ...includeIgnoredArgs, - ...upstreamCacheBypassArgs + ...upstreamCacheBypassArgs, + ...lineStatsReuseArgs }, - { timeoutMs: 15_000 } + { + timeoutMs: 15_000, + // Why: an abort signal forces the dedicated per-request subscription + // socket (one WebSocket + E2EE handshake per request). The safety + // refresh is the idle steady state, so it stays on the pooled call + // transport: its stale result is rejected by the caller's liveness + // guard and the 15s timeout bounds it. Activity refreshes keep the + // abortable transport so pause/host switches cancel real remote work. + ...(options?.reuseLineStats ? {} : { signal: options?.signal }) + } ) } +let nextGitStatusRequestToken = 0 + +async function callLocalGitStatus( + args: Parameters[0], + signal?: AbortSignal +): Promise { + if (!signal) { + return window.api.git.status(args) + } + if (signal.aborted) { + const error = new Error('Git status request aborted') + error.name = 'AbortError' + throw error + } + const requestToken = `git-status-${Date.now()}-${++nextGitStatusRequestToken}` + const cancel = (): void => { + void window.api.git.cancelStatus({ requestToken }).catch(() => {}) + } + signal.addEventListener('abort', cancel, { once: true }) + try { + return await window.api.git.status({ ...args, requestToken }) + } finally { + signal.removeEventListener('abort', cancel) + } +} + export async function getRuntimeGitSubmoduleStatus( context: RuntimeGitContext, submodulePath: string, diff --git a/src/renderer/src/runtime/runtime-rpc-client.test.ts b/src/renderer/src/runtime/runtime-rpc-client.test.ts index 0bf2ac4f8f0..c9481d2746c 100644 --- a/src/renderer/src/runtime/runtime-rpc-client.test.ts +++ b/src/renderer/src/runtime/runtime-rpc-client.test.ts @@ -20,15 +20,20 @@ import { const runtimeCall = vi.fn() const runtimeEnvironmentCall = vi.fn() +const runtimeEnvironmentSubscribe = vi.fn() beforeEach(() => { clearRuntimeCompatibilityCacheForTests() runtimeCall.mockReset() runtimeEnvironmentCall.mockReset() + runtimeEnvironmentSubscribe.mockReset() vi.stubGlobal('window', { api: { runtime: { call: runtimeCall }, - runtimeEnvironments: { call: runtimeEnvironmentCall } + runtimeEnvironments: { + call: runtimeEnvironmentCall, + subscribe: runtimeEnvironmentSubscribe + } } }) }) @@ -109,6 +114,49 @@ describe('runtime RPC client routing', () => { expect(runtimeCall).not.toHaveBeenCalled() }) + it('uses an abortable subscription for paired-runtime requests', async () => { + const controller = new AbortController() + const unsubscribe = vi.fn() + runtimeEnvironmentSubscribe.mockResolvedValue({ unsubscribe, sendBinary: vi.fn() }) + + const request = callRuntimeRpc( + { kind: 'environment', environmentId: 'env-1' }, + 'status.get', + undefined, + { signal: controller.signal } + ) + await vi.waitFor(() => expect(runtimeEnvironmentSubscribe).toHaveBeenCalled()) + controller.abort() + + await expect(request).rejects.toMatchObject({ name: 'AbortError' }) + await vi.waitFor(() => expect(unsubscribe).toHaveBeenCalledTimes(1)) + expect(runtimeEnvironmentCall).not.toHaveBeenCalled() + }) + + it('rejects an abortable paired-runtime request at the response deadline', async () => { + vi.useFakeTimers() + try { + const controller = new AbortController() + const unsubscribe = vi.fn() + runtimeEnvironmentSubscribe.mockResolvedValue({ unsubscribe, sendBinary: vi.fn() }) + + const request = callRuntimeRpc( + { kind: 'environment', environmentId: 'env-1' }, + 'status.get', + undefined, + { signal: controller.signal, timeoutMs: 15_000 } + ) + await vi.waitFor(() => expect(runtimeEnvironmentSubscribe).toHaveBeenCalled()) + const rejection = expect(request).rejects.toThrow('timed out before status.get completed') + await vi.advanceTimersByTimeAsync(15_000) + + await rejection + await vi.waitFor(() => expect(unsubscribe).toHaveBeenCalledTimes(1)) + } finally { + vi.useRealTimers() + } + }) + it('preflights remote runtime compatibility before non-status calls', async () => { runtimeEnvironmentCall.mockImplementation(({ method }: { method: string }) => { const result = diff --git a/src/renderer/src/runtime/runtime-rpc-client.ts b/src/renderer/src/runtime/runtime-rpc-client.ts index e9b58f9d694..1f024c1a67f 100644 --- a/src/renderer/src/runtime/runtime-rpc-client.ts +++ b/src/renderer/src/runtime/runtime-rpc-client.ts @@ -4,20 +4,22 @@ import type { RuntimeStatus } from '../../../shared/runtime-types' import type { RuntimeCapability } from '../../../shared/protocol-version' import { withBrowserPaneUiRuntimeRpcSource } from '../../../shared/runtime-rpc-feature-interaction-source' import { assertRuntimeStatusCompatible } from './runtime-protocol-compat' +import { + callAbortableRuntimeEnvironment, + createRuntimeRpcAbortError +} from './abortable-runtime-environment-call' export type RuntimeClientTarget = { kind: 'local' } | { kind: 'environment'; environmentId: string } const RUNTIME_COMPATIBILITY_CACHE_MAX = 32 const RECENT_RUNTIME_COMPATIBILITY_FAILURE_TTL_MS = 60_000 -// Why: a saved environment can restart into a different Orca version without -// changing ids; capability verdicts must eventually follow that version change. +// Why: capability verdicts must eventually follow a saved environment's version changes. const RUNTIME_CAPABILITY_STATUS_TTL_MS = 60_000 type RuntimeCompatibilityCacheEntry = { check: Promise failedAt: number | null - // True only once status.get settled and proved compatible. Stays false while - // the probe is in flight, so a recovery clear can drop a doomed pending probe. + // False while probing so recovery can drop a doomed pending compatibility check. provenCompatible: boolean status: RuntimeStatus | null statusCheckedAt: number | null @@ -73,34 +75,38 @@ export async function callRuntimeRpc( timeoutMs?: number suppressFeatureInteraction?: boolean reuseRecentCompatibilityFailure?: boolean + signal?: AbortSignal } = {} ): Promise { if (target.kind === 'environment' && method !== 'status.get') { await ensureRuntimeEnvironmentCompatible(target.environmentId, options) } - const nextParams = addFeatureInteractionSource(params, options) + if (options.signal?.aborted) { + throw createRuntimeRpcAbortError() + } + const nextParams = options.suppressFeatureInteraction + ? withBrowserPaneUiRuntimeRpcSource(params) + : params const response = target.kind === 'local' ? await window.api.runtime.call({ method, params: nextParams }) - : await window.api.runtimeEnvironments.call({ - selector: target.environmentId, - method, - params: nextParams, - timeoutMs: options.timeoutMs - }) + : options.signal + ? await callAbortableRuntimeEnvironment( + target.environmentId, + method, + nextParams, + options.timeoutMs, + options.signal + ) + : await window.api.runtimeEnvironments.call({ + selector: target.environmentId, + method, + params: nextParams, + timeoutMs: options.timeoutMs + }) return unwrapRuntimeRpcResult(response as RuntimeRpcResponse) } -function addFeatureInteractionSource( - params: unknown, - options: { suppressFeatureInteraction?: boolean } -): unknown { - if (!options.suppressFeatureInteraction) { - return params - } - return withBrowserPaneUiRuntimeRpcSource(params) -} - async function ensureRuntimeEnvironmentCompatible( environmentId: string, options: { timeoutMs?: number; reuseRecentCompatibilityFailure?: boolean } = {} @@ -187,11 +193,8 @@ function rememberRuntimeEnvironmentCompatibility( } } -// Why: a live status.get answer proves any cached compatibility verdict that is -// not a settled success is stale. Drop settled failures AND still-pending probes -// (a probe queued on the dropped connection is doomed, and a reachability- -// triggered refresh must not coalesce onto it) so the refresh re-probes. Only -// proven-compatible successes stay cached. +// Why: a live status answer invalidates failures and pending probes from the +// dropped connection; only proven-compatible successes remain reusable. export function clearRecentRuntimeCompatibilityFailure(environmentId: string): void { const trimmed = environmentId.trim() if (!trimmed) { diff --git a/src/renderer/src/web/web-preload-api.ts b/src/renderer/src/web/web-preload-api.ts index 0d104751b17..c8c5bb8900d 100644 --- a/src/renderer/src/web/web-preload-api.ts +++ b/src/renderer/src/web/web-preload-api.ts @@ -1634,13 +1634,21 @@ function createFileApi(): NonNullable['fs']> { function createGitApi(): NonNullable['git']> { return { - status: async ({ worktreePath, includeIgnored }) => { + status: async ({ + worktreePath, + includeIgnored, + bypassEffectiveUpstreamNegativeCache, + reuseLineStats + }) => { const worktree = await resolveRuntimeWorktreeByPath(worktreePath) return callRuntimeResult('git.status', { worktree: toRuntimeWorktreeSelector(worktree.id), - includeIgnored + includeIgnored, + bypassEffectiveUpstreamNegativeCache, + reuseLineStats }) }, + cancelStatus: async () => {}, submoduleStatus: async ({ worktreePath, submodulePath, area }) => { const worktree = await resolveRuntimeWorktreeByPath(worktreePath) return callRuntimeResult('git.submoduleStatus', { diff --git a/src/renderer/src/web/web-runtime-client.test.ts b/src/renderer/src/web/web-runtime-client.test.ts index 8f7e500c3d3..33c1ad9bb86 100644 --- a/src/renderer/src/web/web-runtime-client.test.ts +++ b/src/renderer/src/web/web-runtime-client.test.ts @@ -205,6 +205,49 @@ describe('WebRuntimeClient', () => { } }) + it('delivers plain unary responses to their subscription instead of dropping them', async () => { + const client = new WebRuntimeClient({ + v: 2, + endpoint: 'ws://127.0.0.1:6768', + deviceToken: 'token', + publicKeyB64: Buffer.alloc(32).toString('base64') + }) + try { + const keyPair = generateKeyPair() + const sharedKey = deriveSharedKey(keyPair.secretKey, keyPair.publicKey) + const internals = client as unknown as { + state: string + sharedKey: Uint8Array | null + subscriptions: Map< + string, + { callbacks: { onResponse: (response: unknown) => void; onClose?: () => void } } + > + handleSocketMessage: (rawData: unknown, sourceWs?: unknown) => Promise + } + internals.state = 'connected' + internals.sharedKey = sharedKey + const onResponse = vi.fn() + internals.subscriptions.set('req-1', { callbacks: { onResponse } }) + + // A unary RPC (e.g. abortable git.status) answered over the subscription + // bridge: not streaming and neither an end nor a scrollback frame. + const response = { + id: 'req-1', + ok: true, + result: { entries: [], conflictOperation: 'unknown' }, + _meta: { runtimeId: 'runtime-1' } + } + await internals.handleSocketMessage(encrypt(JSON.stringify(response), sharedKey)) + + expect(onResponse).toHaveBeenCalledWith(response) + // The caller owns the unsubscribe; a plain reply must not close it. + expect(internals.subscriptions.has('req-1')).toBe(true) + internals.subscriptions.delete('req-1') + } finally { + client.close() + } + }) + it('keeps file watches on the owning WebSocket instead of opening child clients', async () => { const client = new WebRuntimeClient({ v: 2, diff --git a/src/renderer/src/web/web-runtime-client.ts b/src/renderer/src/web/web-runtime-client.ts index f94cf04476e..6ab1d821cbb 100644 --- a/src/renderer/src/web/web-runtime-client.ts +++ b/src/renderer/src/web/web-runtime-client.ts @@ -511,9 +511,14 @@ export class WebRuntimeClient { } const subscription = this.subscriptions.get(response.id) - if (subscription && isSubscriptionResponse(response)) { - subscription.callbacks.onResponse(response) - if (response.ok && isEndResult(response.result)) { + if (subscription) { + // Why: a unary RPC carried over the subscription bridge (e.g. an + // abortable git.status) answers with a plain response frame. Every frame + // for a subscribed id belongs to that subscription, so deliver it + // instead of dropping it on the pending-call map. + const subscriptionResponse = response as RuntimeRpcResponse + subscription.callbacks.onResponse(subscriptionResponse) + if (subscriptionResponse.ok && isEndResult(subscriptionResponse.result)) { this.subscriptions.delete(response.id) subscription.callbacks.onClose?.() } @@ -765,24 +770,6 @@ export class WebRuntimeClient { } } -function isSubscriptionResponse( - response: RuntimeRpcResponse | Record -): response is RuntimeRpcResponse { - if (!('ok' in response)) { - return false - } - if (response.ok === false) { - return true - } - if (response.ok === false) { - return true - } - const success = response as RuntimeRpcResponse & { ok: true; streaming?: true } - return ( - success.streaming === true || isEndResult(success.result) || isScrollbackResult(success.result) - ) -} - function isRuntimeFailureResponse( response: RuntimeRpcResponse | Record ): response is RuntimeRpcResponse & { ok: false } { @@ -815,10 +802,6 @@ function isEndResult(value: unknown): value is { type: 'end' } { return !!value && typeof value === 'object' && (value as { type?: unknown }).type === 'end' } -function isScrollbackResult(value: unknown): value is { type: 'scrollback' } { - return !!value && typeof value === 'object' && (value as { type?: unknown }).type === 'scrollback' -} - async function websocketPayloadToUint8( value: unknown ): Promise | null> { diff --git a/src/shared/git-status-line-stats-cache.test.ts b/src/shared/git-status-line-stats-cache.test.ts new file mode 100644 index 00000000000..fa17d0d6079 --- /dev/null +++ b/src/shared/git-status-line-stats-cache.test.ts @@ -0,0 +1,152 @@ +import { beforeEach, describe, expect, it } from 'vitest' +import { + applyCachedGitStatusLineStats, + beginGitStatusLineStatsCacheWrite, + clearGitStatusLineStatsCache, + clearGitStatusLineStatsCacheKey, + GIT_STATUS_LINE_STATS_CACHE_MAX_AGE_MS, + storeGitStatusLineStats +} from './git-status-line-stats-cache' + +const cachedEntries = [ + { + path: 'src/a.ts', + status: 'modified', + area: 'unstaged', + added: 3, + removed: 2 + } +] + +describe('git status line stats cache', () => { + beforeEach(() => { + clearGitStatusLineStatsCache() + }) + + it('reuses counts only for an exact head and status-entry identity', () => { + storeGitStatusLineStats({ cacheKey: 'native\0/repo', head: 'head-1', entries: cachedEntries }) + const matchingEntries = [{ path: 'src/a.ts', status: 'modified', area: 'unstaged' }] + + expect( + applyCachedGitStatusLineStats({ + cacheKey: 'native\0/repo', + head: 'head-1', + entries: matchingEntries + }) + ).toBe(true) + expect(matchingEntries).toEqual(cachedEntries) + + expect( + applyCachedGitStatusLineStats({ + cacheKey: 'native\0/repo', + head: 'head-2', + entries: [{ path: 'src/a.ts', status: 'modified', area: 'unstaged' }] + }) + ).toBe(false) + }) + + it('invalidates a matching status identity at the bounded age', () => { + storeGitStatusLineStats({ + cacheKey: 'native\0/repo', + head: 'head-1', + entries: cachedEntries, + now: 1_000 + }) + + expect( + applyCachedGitStatusLineStats({ + cacheKey: 'native\0/repo', + head: 'head-1', + entries: [{ path: 'src/a.ts', status: 'modified', area: 'unstaged' }], + now: 1_000 + GIT_STATUS_LINE_STATS_CACHE_MAX_AGE_MS + }) + ).toBe(false) + }) + + it('isolates execution-host keys and supports mutation invalidation', () => { + storeGitStatusLineStats({ + cacheKey: 'wsl:ubuntu\0/repo', + head: 'head-1', + entries: cachedEntries + }) + + expect( + applyCachedGitStatusLineStats({ + cacheKey: 'wsl:debian\0/repo', + head: 'head-1', + entries: [{ path: 'src/a.ts', status: 'modified', area: 'unstaged' }] + }) + ).toBe(false) + + clearGitStatusLineStatsCacheKey('wsl:ubuntu\0/repo') + expect( + applyCachedGitStatusLineStats({ + cacheKey: 'wsl:ubuntu\0/repo', + head: 'head-1', + entries: [{ path: 'src/a.ts', status: 'modified', area: 'unstaged' }] + }) + ).toBe(false) + }) + + it('lets an earlier recompute store after a later reuse-only read began', () => { + const recomputeWrite = beginGitStatusLineStatsCacheWrite('native\0/repo') + // A hinted safety read begins later, reuses, and never stores; it must not + // starve the older recompute's store. + beginGitStatusLineStatsCacheWrite('native\0/repo') + storeGitStatusLineStats({ + cacheKey: 'native\0/repo', + head: 'head-1', + entries: cachedEntries, + writeToken: recomputeWrite + }) + + 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') + storeGitStatusLineStats({ + cacheKey: 'native\0/repo', + head: 'head-1', + entries: [{ ...cachedEntries[0], added: 8 }], + writeToken: latestWrite + }) + storeGitStatusLineStats({ + cacheKey: 'native\0/repo', + head: 'head-1', + entries: cachedEntries, + writeToken: olderWrite + }) + 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(8) + + const invalidatedWrite = beginGitStatusLineStatsCacheWrite('native\0/other') + clearGitStatusLineStatsCache() + storeGitStatusLineStats({ + cacheKey: 'native\0/other', + head: 'head-1', + entries: cachedEntries, + writeToken: invalidatedWrite + }) + expect( + applyCachedGitStatusLineStats({ + cacheKey: 'native\0/other', + head: 'head-1', + entries: [{ path: 'src/a.ts', status: 'modified', area: 'unstaged' }] + }) + ).toBe(false) + }) +}) diff --git a/src/shared/git-status-line-stats-cache.ts b/src/shared/git-status-line-stats-cache.ts new file mode 100644 index 00000000000..1d7fe81926b --- /dev/null +++ b/src/shared/git-status-line-stats-cache.ts @@ -0,0 +1,235 @@ +type GitStatusLineStatsEntry = { + path?: unknown + status?: unknown + area?: unknown + oldPath?: unknown + conflictKind?: unknown + conflictStatus?: unknown + conflictStatusSource?: unknown + submodule?: { + commitChanged?: boolean + trackedChanges?: boolean + untrackedChanges?: boolean + } + added?: number + removed?: number +} + +type CachedLineStats = { + identity: string + storedAt: number + stats: { added?: number; removed?: number }[] +} + +export type GitStatusLineStatsWriteToken = { + cacheKey: string + globalGeneration: number + keyGeneration: number + beginSeq: number +} + +// Why: the TTL is the sole staleness backstop when file contents change while +// the porcelain identity stays "modified" (added/removed are excluded from the +// reuse identity), so a missed watcher signal pins counts for at most this long. +export const GIT_STATUS_LINE_STATS_CACHE_MAX_AGE_MS = 2 * 60_000 +const GIT_STATUS_LINE_STATS_CACHE_MAX_ENTRIES = 128 +const GIT_STATUS_LINE_STATS_WRITE_KEYS_MAX_ENTRIES = 1024 +const lineStatsByWorktree = new Map() +// Why: mutation invalidation must retire scans that began before it. A scan +// captures these generations at begin; a mismatch at store/clear time means an +// invalidation happened mid-scan and the derived stats may be pre-mutation. +let globalInvalidationGeneration = 0 +const keyInvalidationGenerationByWorktree = new Map() +// Why: overlapping recomputes must resolve latest-begun-wins without letting a +// reuse-only read (which never stores) starve an older recompute's store. +const lastStoredBeginSeqByWorktree = new Map() +let nextBeginSeq = 0 + +// Why: wall-clock steps (NTP, VM resume) must not extend or shrink the TTL. +const monotonicNowMs = (): number => performance.now() + +function bumpBoundedKeyMap(map: Map, cacheKey: string, value: number): void { + map.delete(cacheKey) + map.set(cacheKey, value) + while (map.size > GIT_STATUS_LINE_STATS_WRITE_KEYS_MAX_ENTRIES) { + const oldestKey = map.keys().next().value + if (oldestKey === undefined) { + return + } + map.delete(oldestKey) + } +} + +export function beginGitStatusLineStatsCacheWrite(cacheKey: string): GitStatusLineStatsWriteToken { + return { + cacheKey, + globalGeneration: globalInvalidationGeneration, + keyGeneration: keyInvalidationGenerationByWorktree.get(cacheKey) ?? 0, + beginSeq: ++nextBeginSeq + } +} + +function isWriteTokenCurrent(token: GitStatusLineStatsWriteToken): boolean { + return ( + token.globalGeneration === globalInvalidationGeneration && + token.keyGeneration === (keyInvalidationGenerationByWorktree.get(token.cacheKey) ?? 0) && + token.beginSeq >= (lastStoredBeginSeqByWorktree.get(token.cacheKey) ?? 0) + ) +} + +function createInputIdentity(head: string | undefined, entries: GitStatusLineStatsEntry[]): string { + return JSON.stringify([ + head ?? null, + entries.map((entry) => [ + entry.path, + entry.status, + entry.area, + entry.oldPath ?? null, + entry.conflictKind ?? null, + entry.conflictStatus ?? null, + entry.conflictStatusSource ?? null, + entry.submodule?.commitChanged ?? null, + entry.submodule?.trackedChanges ?? null, + entry.submodule?.untrackedChanges ?? null + ]) + ]) +} + +function trimLineStatsCache(now: number): void { + for (const [key, cached] of lineStatsByWorktree) { + if (now - cached.storedAt >= GIT_STATUS_LINE_STATS_CACHE_MAX_AGE_MS) { + lineStatsByWorktree.delete(key) + } + } + while (lineStatsByWorktree.size > GIT_STATUS_LINE_STATS_CACHE_MAX_ENTRIES) { + const oldestKey = lineStatsByWorktree.keys().next().value + if (oldestKey === undefined) { + return + } + lineStatsByWorktree.delete(oldestKey) + } +} + +export function applyCachedGitStatusLineStats(input: { + cacheKey: string + head?: string + entries: GitStatusLineStatsEntry[] + now?: number +}): boolean { + const now = input.now ?? monotonicNowMs() + const cached = lineStatsByWorktree.get(input.cacheKey) + if (!cached) { + return false + } + if ( + now - cached.storedAt >= GIT_STATUS_LINE_STATS_CACHE_MAX_AGE_MS || + cached.identity !== createInputIdentity(input.head, input.entries) + ) { + lineStatsByWorktree.delete(input.cacheKey) + return false + } + + lineStatsByWorktree.delete(input.cacheKey) + lineStatsByWorktree.set(input.cacheKey, cached) + input.entries.forEach((entry, index) => { + const stats = cached.stats[index] + if (stats?.added !== undefined) { + entry.added = stats.added + } + if (stats?.removed !== undefined) { + entry.removed = stats.removed + } + }) + return true +} + +export function storeGitStatusLineStats(input: { + cacheKey: string + head?: string + entries: GitStatusLineStatsEntry[] + now?: number + writeToken?: GitStatusLineStatsWriteToken +}): void { + const writeToken = input.writeToken ?? beginGitStatusLineStatsCacheWrite(input.cacheKey) + if (!isWriteTokenCurrent(writeToken)) { + return + } + bumpBoundedKeyMap(lastStoredBeginSeqByWorktree, input.cacheKey, writeToken.beginSeq) + const now = input.now ?? monotonicNowMs() + lineStatsByWorktree.delete(input.cacheKey) + lineStatsByWorktree.set(input.cacheKey, { + identity: createInputIdentity(input.head, input.entries), + storedAt: now, + stats: input.entries.map((entry) => ({ + ...(entry.added === undefined ? {} : { added: entry.added }), + ...(entry.removed === undefined ? {} : { removed: entry.removed }) + })) + }) + trimLineStatsCache(now) +} + +/** + * Shared post-status line-stat step for every host that executes Git. Reuses + * the cached snapshot only for hinted safety reads; otherwise recomputes and + * stores. `recompute` returns false when the counts are incomplete (e.g. a + * transient numstat failure) so a failed pass never replaces or pins a + * snapshot, and the previous good counts stay reusable. + */ +export async function reuseOrRecomputeGitStatusLineStats(input: { + cacheKey: string + head?: string + entries: GitStatusLineStatsEntry[] + writeToken: GitStatusLineStatsWriteToken + reuse: boolean + isAborted: () => boolean + recompute: () => Promise +}): Promise { + if ( + input.reuse && + applyCachedGitStatusLineStats({ + cacheKey: input.cacheKey, + head: input.head, + entries: input.entries + }) + ) { + return + } + const complete = await input.recompute() + if (input.isAborted()) { + clearGitStatusLineStatsCacheKey(input.cacheKey, input.writeToken) + return + } + if (!complete) { + return + } + storeGitStatusLineStats({ + cacheKey: input.cacheKey, + head: input.head, + entries: input.entries, + writeToken: input.writeToken + }) +} + +export function clearGitStatusLineStatsCache(): void { + globalInvalidationGeneration += 1 + lineStatsByWorktree.clear() + keyInvalidationGenerationByWorktree.clear() + lastStoredBeginSeqByWorktree.clear() +} + +export function clearGitStatusLineStatsCacheKey( + cacheKey: string, + writeToken?: GitStatusLineStatsWriteToken +): void { + if (writeToken !== undefined && !isWriteTokenCurrent(writeToken)) { + return + } + if (writeToken === undefined) { + bumpBoundedKeyMap( + keyInvalidationGenerationByWorktree, + cacheKey, + (keyInvalidationGenerationByWorktree.get(cacheKey) ?? 0) + 1 + ) + } + lineStatsByWorktree.delete(cacheKey) +} diff --git a/src/shared/git-uncommitted-line-stats.ts b/src/shared/git-uncommitted-line-stats.ts index 3fe4c5cc99d..f62e34c1a28 100644 --- a/src/shared/git-uncommitted-line-stats.ts +++ b/src/shared/git-uncommitted-line-stats.ts @@ -178,10 +178,16 @@ function rememberUntrackedStats( // We count their contents directly to show an additions magnitude. export async function collectUntrackedAdditions( worktreePath: string, - untrackedPaths: readonly string[] + untrackedPaths: readonly string[], + signal?: AbortSignal ): Promise> { const result = new Map() for (let i = 0; i < untrackedPaths.length; i += UNTRACKED_READ_CONCURRENCY) { + // Why: an aborted refresh discards its result, so stop reading here rather + // than keep burning (possibly remote-host) file I/O after cancellation. + if (signal?.aborted) { + return result + } const chunk = untrackedPaths.slice(i, i + UNTRACKED_READ_CONCURRENCY) await Promise.all( chunk.map(async (relativePath) => {