mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 00:02:31 +00:00
fix(scm): keep git-status pacing across scheduler rebuilds (#11820)
* fix(scm): keep git-status pacing across scheduler rebuilds * fix(scm): order shared refresh pacing updates
This commit is contained in:
@@ -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,116 @@ 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('does not let an older disposed run erase replacement backoff', 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()
|
||||
first.dispose()
|
||||
|
||||
const second = createScheduler(task, pacing)
|
||||
second.resumeSafety()
|
||||
await vi.advanceTimersByTimeAsync(30_000)
|
||||
calls[1]?.resolve()
|
||||
await flushMicrotasks()
|
||||
|
||||
await vi.advanceTimersByTimeAsync(1)
|
||||
calls[0]?.resolve()
|
||||
await flushMicrotasks()
|
||||
second.signal()
|
||||
|
||||
await vi.advanceTimersByTimeAsync(29_998)
|
||||
expect(task).toHaveBeenCalledTimes(2)
|
||||
await vi.advanceTimersByTimeAsync(1)
|
||||
expect(task).toHaveBeenCalledTimes(3)
|
||||
})
|
||||
|
||||
it('lets an aborted old run pace repeated rebuilds until a replacement finishes', 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()
|
||||
first.dispose()
|
||||
|
||||
const second = createScheduler(task, pacing)
|
||||
second.resumeSafety()
|
||||
calls[0]?.resolve()
|
||||
await flushMicrotasks()
|
||||
second.dispose()
|
||||
|
||||
const third = createScheduler(task, pacing)
|
||||
third.resumeSafety()
|
||||
expect(task).toHaveBeenCalledTimes(2)
|
||||
await vi.advanceTimersByTimeAsync(2999)
|
||||
expect(task).toHaveBeenCalledTimes(2)
|
||||
await vi.advanceTimersByTimeAsync(1)
|
||||
expect(task).toHaveBeenCalledTimes(3)
|
||||
})
|
||||
|
||||
it('cleans up debounce and safety timers on dispose', async () => {
|
||||
vi.useFakeTimers()
|
||||
const task = vi.fn(async () => {})
|
||||
|
||||
@@ -2,6 +2,25 @@ 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
|
||||
nextRunId: number
|
||||
latestFinishedRunId: number
|
||||
}
|
||||
|
||||
export function createGitStatusRefreshPacing(): GitStatusRefreshPacing {
|
||||
return {
|
||||
lastRunEndedAt: -Infinity,
|
||||
lastRunDurationMs: 0,
|
||||
nextRunId: 0,
|
||||
latestFinishedRunId: 0
|
||||
}
|
||||
}
|
||||
|
||||
export type GitStatusRefreshScheduler = {
|
||||
resumeSafety: () => void
|
||||
pause: () => void
|
||||
@@ -27,6 +46,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 +57,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 +75,7 @@ export function createGitStatusRefreshScheduler(
|
||||
|
||||
const requiredActivityIdleMs = (): number =>
|
||||
slowTaskRequiredIdleMs(
|
||||
lastRunDurationMs,
|
||||
pacing.lastRunDurationMs,
|
||||
options.slowTaskBackoff.changeSignalMultiplier,
|
||||
options.activityMinGapMs,
|
||||
options.slowTaskBackoff.maxIntervalMs
|
||||
@@ -68,7 +87,7 @@ export function createGitStatusRefreshScheduler(
|
||||
const delay = Math.max(
|
||||
options.safetyIntervalMs,
|
||||
slowTaskRequiredIdleMs(
|
||||
lastRunDurationMs,
|
||||
pacing.lastRunDurationMs,
|
||||
options.slowTaskBackoff.idleMultiplier,
|
||||
0,
|
||||
options.slowTaskBackoff.maxIntervalMs
|
||||
@@ -91,7 +110,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
|
||||
@@ -121,6 +140,7 @@ export function createGitStatusRefreshScheduler(
|
||||
clearSafetyTimer()
|
||||
inFlight = true
|
||||
const startedAt = Date.now()
|
||||
const runId = ++pacing.nextRunId
|
||||
const controller = new AbortController()
|
||||
activeController = controller
|
||||
let result: Promise<void>
|
||||
@@ -134,12 +154,17 @@ export function createGitStatusRefreshScheduler(
|
||||
// Status refresh errors are transient; the next signal or safety run retries.
|
||||
})
|
||||
.finally(() => {
|
||||
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)
|
||||
if (runId > pacing.latestFinishedRunId) {
|
||||
pacing.latestFinishedRunId = runId
|
||||
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.
|
||||
pacing.lastRunDurationMs = controller.signal.aborted
|
||||
? 0
|
||||
: Math.max(0, pacing.lastRunEndedAt - startedAt)
|
||||
}
|
||||
if (activeController === controller) {
|
||||
activeController = null
|
||||
}
|
||||
|
||||
@@ -172,6 +172,13 @@ describe('useGitStatusPolling rerender stability', () => {
|
||||
// Should trigger an immediate poll on the new worktree (total 2 calls)
|
||||
// without having to wait for the 3000ms timer.
|
||||
expect(refreshMock).toHaveBeenCalledTimes(2)
|
||||
|
||||
await act(async () => {
|
||||
useAppStore.setState({ activeWorktreeId: WORKTREE_ID })
|
||||
})
|
||||
await flushMicrotasks()
|
||||
|
||||
expect(refreshMock).toHaveBeenCalledTimes(3)
|
||||
})
|
||||
|
||||
it('refreshes immediately when Source Control becomes visible', async () => {
|
||||
@@ -241,6 +248,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,22 @@ export function useGitStatusPolling(options: { enabled?: boolean } = {}): void {
|
||||
const statusRefreshGenerationRef = useRef(0)
|
||||
|
||||
const statusSchedulerRef = useRef<GitStatusRefreshScheduler | null>(null)
|
||||
// Why: pacing belongs to the current worktree, not to scheduler instances —
|
||||
// execution-host/push-target rebuilds must not reset it, or a flapping host id
|
||||
// lets sustained change signals run git at the bare debounce.
|
||||
const statusPacingRef = useRef<{
|
||||
key: string
|
||||
pacing: GitStatusRefreshPacing
|
||||
} | null>(null)
|
||||
useEffect(() => {
|
||||
const generation = ++statusRefreshGenerationRef.current
|
||||
const pacingKey = `${activeWorktreeId}\0${worktreePath}`
|
||||
let pacing =
|
||||
statusPacingRef.current?.key === pacingKey ? statusPacingRef.current.pacing : undefined
|
||||
if (!pacing) {
|
||||
pacing = createGitStatusRefreshPacing()
|
||||
statusPacingRef.current = { key: pacingKey, pacing }
|
||||
}
|
||||
const scheduler = createGitStatusRefreshScheduler(
|
||||
({ reason, signal }) =>
|
||||
runFetchStatusRef.current({
|
||||
@@ -179,7 +195,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
|
||||
|
||||
Reference in New Issue
Block a user