fix(scm): keep git-status pacing across scheduler rebuilds

This commit is contained in:
Brennan Benson
2026-07-31 14:38:20 -07:00
parent cd2b62ed14
commit 6eece0b41e
4 changed files with 128 additions and 10 deletions
@@ -1,6 +1,8 @@
import { afterEach, describe, expect, it, vi } from 'vitest'
import {
createGitStatusRefreshPacing,
createGitStatusRefreshScheduler,
type GitStatusRefreshPacing,
type GitStatusRefreshReason
} from './git-status-refresh-scheduler'
@@ -19,7 +21,8 @@ async function flushMicrotasks(): Promise<void> {
}
function createScheduler(
task: (request: { reason: GitStatusRefreshReason; signal: AbortSignal }) => Promise<void>
task: (request: { reason: GitStatusRefreshReason; signal: AbortSignal }) => Promise<void>,
pacing?: GitStatusRefreshPacing
) {
return createGitStatusRefreshScheduler(task, {
safetyIntervalMs: 60_000,
@@ -29,7 +32,8 @@ function createScheduler(
idleMultiplier: 5,
changeSignalMultiplier: 1,
maxIntervalMs: 5 * 60_000
}
},
...(pacing ? { pacing } : {})
})
}
@@ -258,6 +262,56 @@ describe('createGitStatusRefreshScheduler', () => {
expect(task).toHaveBeenCalledTimes(2)
})
it('keeps the activity floor across scheduler recreation when pacing is shared', async () => {
vi.useFakeTimers()
const task = vi.fn(async () => {})
const pacing = createGitStatusRefreshPacing()
const first = createScheduler(task, pacing)
first.resumeSafety()
await flushMicrotasks()
expect(task).toHaveBeenCalledTimes(1)
first.dispose()
// A rebuild (execution-host/push-target change) must not grant a fresh
// immediate run; the floor from the previous run still applies.
const second = createScheduler(task, pacing)
second.resumeSafety()
await flushMicrotasks()
expect(task).toHaveBeenCalledTimes(1)
await vi.advanceTimersByTimeAsync(2999)
expect(task).toHaveBeenCalledTimes(1)
await vi.advanceTimersByTimeAsync(1)
expect(task).toHaveBeenCalledTimes(2)
})
it('keeps slow-scan backoff across scheduler recreation when pacing is shared', async () => {
vi.useFakeTimers()
const calls: ReturnType<typeof deferred>[] = []
const task = vi.fn(() => {
const call = deferred()
calls.push(call)
return call.promise
})
const pacing = createGitStatusRefreshPacing()
const first = createScheduler(task, pacing)
first.resumeSafety()
await vi.advanceTimersByTimeAsync(30_000)
calls[0]?.resolve()
await flushMicrotasks()
first.dispose()
const second = createScheduler(task, pacing)
second.resumeSafety()
await flushMicrotasks()
// The 30s scan duration still paces the next run: max(3s floor, 1x 30s).
await vi.advanceTimersByTimeAsync(29_999)
expect(task).toHaveBeenCalledTimes(1)
await vi.advanceTimersByTimeAsync(1)
expect(task).toHaveBeenCalledTimes(2)
})
it('cleans up debounce and safety timers on dispose', async () => {
vi.useFakeTimers()
const task = vi.fn(async () => {})
@@ -2,6 +2,18 @@ import { slowTaskRequiredIdleMs, type SlowTaskBackoffOptions } from './coalesced
export type GitStatusRefreshReason = 'activity' | 'safety'
// Why: run pacing must be shareable across scheduler instances — a rebuild
// (execution-host or push-target change) that resets it lets sustained change
// signals run git at the bare debounce, bypassing the #7983 floor.
export type GitStatusRefreshPacing = {
lastRunEndedAt: number
lastRunDurationMs: number
}
export function createGitStatusRefreshPacing(): GitStatusRefreshPacing {
return { lastRunEndedAt: -Infinity, lastRunDurationMs: 0 }
}
export type GitStatusRefreshScheduler = {
resumeSafety: () => void
pause: () => void
@@ -27,6 +39,7 @@ export function createGitStatusRefreshScheduler(
// task backoff when the previous scan itself was slow.
activityMinGapMs: number
slowTaskBackoff: SlowTaskBackoffOptions
pacing?: GitStatusRefreshPacing
}
): GitStatusRefreshScheduler {
let disposed = false
@@ -37,8 +50,7 @@ export function createGitStatusRefreshScheduler(
let activityTimerFiresAt = Infinity
let safetyTimer: ReturnType<typeof setTimeout> | null = null
let activeController: AbortController | null = null
let lastRunEndedAt = -Infinity
let lastRunDurationMs = 0
const pacing = options.pacing ?? createGitStatusRefreshPacing()
const clearActivityTimer = (): void => {
if (activityTimer !== null) {
@@ -56,7 +68,7 @@ export function createGitStatusRefreshScheduler(
const requiredActivityIdleMs = (): number =>
slowTaskRequiredIdleMs(
lastRunDurationMs,
pacing.lastRunDurationMs,
options.slowTaskBackoff.changeSignalMultiplier,
options.activityMinGapMs,
options.slowTaskBackoff.maxIntervalMs
@@ -68,7 +80,7 @@ export function createGitStatusRefreshScheduler(
const delay = Math.max(
options.safetyIntervalMs,
slowTaskRequiredIdleMs(
lastRunDurationMs,
pacing.lastRunDurationMs,
options.slowTaskBackoff.idleMultiplier,
0,
options.slowTaskBackoff.maxIntervalMs
@@ -91,7 +103,7 @@ export function createGitStatusRefreshScheduler(
return
}
const now = Date.now()
const delay = Math.max(minDelayMs, lastRunEndedAt + requiredActivityIdleMs() - now)
const delay = Math.max(minDelayMs, pacing.lastRunEndedAt + requiredActivityIdleMs() - now)
if (delay <= 0) {
startRun('activity')
return
@@ -134,12 +146,14 @@ export function createGitStatusRefreshScheduler(
// Status refresh errors are transient; the next signal or safety run retries.
})
.finally(() => {
lastRunEndedAt = Date.now()
pacing.lastRunEndedAt = Date.now()
// Why: cancelled scans never delivered a useful result, so their wall
// time must not stretch the next activity/safety gap. Otherwise hide →
// reveal after a slow abort waits out the aborted scan's full duration
// before the catch-up refresh can start.
lastRunDurationMs = controller.signal.aborted ? 0 : Math.max(0, lastRunEndedAt - startedAt)
pacing.lastRunDurationMs = controller.signal.aborted
? 0
: Math.max(0, pacing.lastRunEndedAt - startedAt)
if (activeController === controller) {
activeController = null
}
@@ -241,6 +241,42 @@ describe('useGitStatusPolling rerender stability', () => {
expect(refreshMock.mock.calls[1]?.[0].request.reuseLineStats).toBe(true)
})
it('keeps the activity floor when the execution host flaps for the same worktree', async () => {
await renderHook()
await flushMicrotasks()
// Mount refresh settles and stamps pacing for this worktree.
expect(refreshMock).toHaveBeenCalledTimes(1)
// Flap the resolved execution host back and forth. Each flip rebuilds the
// scheduler; pacing must survive or every flip grants an immediate run
// (the rc.3 storm: sustained git at the debounce floor).
await act(async () => {
useAppStore.setState({
worktreesByRepo: {
[REPO_ID]: [{ ...worktree, hostId: 'runtime:env-2' }],
[REPO_ID2]: [worktree2]
}
})
})
await flushMicrotasks()
await act(async () => {
useAppStore.setState({
worktreesByRepo: {
[REPO_ID]: [worktree],
[REPO_ID2]: [worktree2]
}
})
})
await flushMicrotasks()
expect(refreshMock).toHaveBeenCalledTimes(1)
await vi.advanceTimersByTimeAsync(2999)
expect(refreshMock).toHaveBeenCalledTimes(1)
await vi.advanceTimersByTimeAsync(1)
await flushMicrotasks()
expect(refreshMock).toHaveBeenCalledTimes(2)
})
it('aborts and rejects stale work when the execution host changes', async () => {
let resolveFirst!: () => void
const firstRefresh = new Promise<void>((resolve) => {
@@ -15,7 +15,9 @@ import { useGitStatusFileWatchRefresh } from './git-status-file-watch-refresh'
import { useGitStatusPushSignalRefresh } from './git-status-push-signal-refresh'
import { useStaleConflictOperationPolling } from './stale-conflict-operation-poll'
import {
createGitStatusRefreshPacing,
createGitStatusRefreshScheduler,
type GitStatusRefreshPacing,
type GitStatusRefreshReason,
type GitStatusRefreshScheduler
} from './git-status-refresh-scheduler'
@@ -162,8 +164,19 @@ export function useGitStatusPolling(options: { enabled?: boolean } = {}): void {
const statusRefreshGenerationRef = useRef(0)
const statusSchedulerRef = useRef<GitStatusRefreshScheduler | null>(null)
// Why: pacing is keyed by worktree, not by scheduler instance — the rebuilds
// below (execution-host/push-target changes) must not reset it, or a flapping
// host id lets sustained change signals run git at the bare debounce.
const statusPacingByWorktreeRef = useRef<Map<string, GitStatusRefreshPacing> | null>(null)
useEffect(() => {
const generation = ++statusRefreshGenerationRef.current
const pacingByWorktree = (statusPacingByWorktreeRef.current ??= new Map())
const pacingKey = `${activeWorktreeId}\0${worktreePath}`
let pacing = pacingByWorktree.get(pacingKey)
if (!pacing) {
pacing = createGitStatusRefreshPacing()
pacingByWorktree.set(pacingKey, pacing)
}
const scheduler = createGitStatusRefreshScheduler(
({ reason, signal }) =>
runFetchStatusRef.current({
@@ -179,7 +192,8 @@ export function useGitStatusPolling(options: { enabled?: boolean } = {}): void {
safetyIntervalMs: STATUS_SAFETY_INTERVAL_MS,
activityDebounceMs: STATUS_ACTIVITY_DEBOUNCE_MS,
activityMinGapMs: STATUS_ACTIVITY_MIN_GAP_MS,
slowTaskBackoff: SLOW_GIT_POLL_BACKOFF
slowTaskBackoff: SLOW_GIT_POLL_BACKOFF,
pacing
}
)
statusSchedulerRef.current = scheduler