fix(sidebar): isolate runtime reconnect refreshes (#11472)

This commit is contained in:
Brennan Benson 2026-07-29 20:10:44 -07:00 committed by GitHub
parent bf894ef150
commit f0eca5fe32
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 1291 additions and 240 deletions

File diff suppressed because it is too large Load Diff

View File

@ -3,7 +3,7 @@
import type { CSSProperties, ReactNode } from 'react'
import { renderToStaticMarkup } from 'react-dom/server'
import { tmpdir } from 'node:os'
import { cleanup, render, waitFor } from '@testing-library/react'
import { cleanup, render } from '@testing-library/react'
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import { getDefaultSettings } from '../../../../shared/constants'
import type { GlobalSettings } from '../../../../shared/types'
@ -196,38 +196,28 @@ describe('Sidebar', () => {
expect(fetchAllWorktrees).toHaveBeenCalledTimes(1)
})
it('does not scan when runtime hosts come online during the startup refresh', async () => {
it('does not scan all hosts when runtime connection status flaps', () => {
setSidebarState(getDefaultSettings(tmpdir()))
const fetchAllWorktrees = vi.fn().mockResolvedValue(undefined)
mocks.state = {
...mocks.state,
fetchAllWorktrees,
fetchWorktreeLineage: vi.fn().mockResolvedValue(undefined),
runtimeStatusByEnvironmentId: new Map(),
startupWorktreeRefreshCompleted: false
startupWorktreeRefreshCompleted: true
}
const view = render(sidebarElement())
mocks.state = {
...mocks.state,
runtimeStatusByEnvironmentId: new Map([['runtime-a', { status: 'connected' }]])
for (let index = 0; index < 5; index += 1) {
mocks.state = {
...mocks.state,
runtimeStatusByEnvironmentId: new Map([
['runtime-a', { status: index % 2 === 0 ? 'connected' : null }]
])
}
view.rerender(sidebarElement())
}
view.rerender(sidebarElement())
expect(fetchAllWorktrees).not.toHaveBeenCalled()
mocks.state = { ...mocks.state, startupWorktreeRefreshCompleted: true }
view.rerender(sidebarElement())
expect(fetchAllWorktrees).not.toHaveBeenCalled()
mocks.state = {
...mocks.state,
runtimeStatusByEnvironmentId: new Map([
['runtime-a', { status: 'connected' }],
['runtime-b', { status: 'connected' }]
])
}
view.rerender(sidebarElement())
await waitFor(() => expect(fetchAllWorktrees).toHaveBeenCalledTimes(1))
})
describe('companion board mutual exclusion', () => {

View File

@ -14,7 +14,6 @@ import { cn } from '@/lib/utils'
import { FolderPlus, Loader2 } from 'lucide-react'
import { useSidebarProjectDrop } from './useSidebarProjectDrop'
import { useWorkspaceBoardPanel } from './useWorkspaceBoardPanel'
import { createSingleFlightCoalescer, type SingleFlightCoalescer } from './single-flight-coalescer'
import { resolveLeftSidebarStyleVariables } from '@/lib/left-sidebar-appearance'
import { useSystemPrefersDark } from '@/components/terminal-pane/use-system-prefers-dark'
import { lazyWithRetry } from '@/lib/lazy-with-retry'
@ -87,45 +86,6 @@ function Sidebar({
}
}, [repoCount, startupWorktreeRefreshCompleted, fetchAllWorktrees])
// Why: a runtime host coming online/offline must refresh the sidebar so its
// worktrees appear/drop, the same way SSH state changes already refetch. Only
// the manual connect button refetched before, so the list went stale until the
// user forced a refetch (e.g. via Add Project). React to the set of online
// runtime envs (a host has a status entry once it is connected).
const runtimeStatusByEnvironmentId = useAppStore((s) => s.runtimeStatusByEnvironmentId)
const fetchWorktreeLineage = useAppStore((s) => s.fetchWorktreeLineage)
const onlineRuntimeEnvKey = React.useMemo(
() =>
// Why: tolerate an absent map — a partial/hydrating store can leave this
// undefined, and a thrown selector would crash the whole sidebar render.
[...(runtimeStatusByEnvironmentId?.entries() ?? [])]
.filter(([, entry]) => Boolean(entry?.status))
.map(([id]) => id)
.sort()
.join(','),
[runtimeStatusByEnvironmentId]
)
const previousOnlineRuntimeEnvKeyRef = React.useRef<string | null>(null)
// Coalesce staggered wake reconnects so K hosts can't fire K sidebar remounts and freeze (#8539).
const reconnectRefreshRef = React.useRef<SingleFlightCoalescer | null>(null)
if (reconnectRefreshRef.current === null) {
reconnectRefreshRef.current = createSingleFlightCoalescer(() =>
fetchAllWorktrees().then(() => fetchWorktreeLineage())
)
}
useEffect(() => {
const previousOnlineRuntimeEnvKey = previousOnlineRuntimeEnvKeyRef.current
previousOnlineRuntimeEnvKeyRef.current = onlineRuntimeEnvKey
if (
previousOnlineRuntimeEnvKey === null ||
previousOnlineRuntimeEnvKey === onlineRuntimeEnvKey ||
!startupWorktreeRefreshCompleted
) {
return
}
reconnectRefreshRef.current?.request()
}, [onlineRuntimeEnvKey, startupWorktreeRefreshCompleted])
useEffect(() => {
if (!sidebarOpen && workspaceBoardRenderedOpen) {
closeWorkspaceBoard()

View File

@ -1,141 +0,0 @@
import { describe, expect, it } from 'vitest'
import { createSingleFlightCoalescer } from './single-flight-coalescer'
/** Resolve after all queued microtasks and timers have drained. */
const flush = () => new Promise<void>((resolve) => setTimeout(resolve, 0))
type Deferred = {
promise: Promise<void>
resolve: () => void
reject: (e: unknown) => void
}
const deferred = (): Deferred => {
let resolve!: () => void
let reject!: (e: unknown) => void
const promise = new Promise<void>((res, rej) => {
resolve = () => res()
reject = rej
})
return { promise, resolve, reject }
}
describe('createSingleFlightCoalescer', () => {
it('runs the task immediately on the first request', async () => {
let calls = 0
const c = createSingleFlightCoalescer(async () => {
calls++
})
c.request()
await flush()
expect(calls).toBe(1)
})
it('collapses a burst of requests during an in-flight run into ONE trailing run', async () => {
// This is the #8539 wake-storm guard: K near-simultaneous reconnect triggers must
// not produce K refreshes — only the leading run + a single trailing run.
let calls = 0
const gates: Deferred[] = []
const c = createSingleFlightCoalescer(() => {
calls++
const d = deferred()
gates.push(d)
return d.promise
})
c.request() // leading run starts
await flush()
expect(calls).toBe(1)
// 10 staggered reconnects land while the first refresh is still in flight.
for (let i = 0; i < 10; i++) {
c.request()
}
await flush()
expect(calls).toBe(1) // still just the one in-flight run
gates[0].resolve() // leading run settles -> exactly one trailing run
await flush()
expect(calls).toBe(2)
gates[1].resolve() // trailing run settles; nothing pending
await flush()
expect(calls).toBe(2)
})
it('runs each request that arrives while idle (no coalescing without contention)', async () => {
let calls = 0
const c = createSingleFlightCoalescer(async () => {
calls++
})
c.request()
await flush()
c.request()
await flush()
c.request()
await flush()
expect(calls).toBe(3)
})
it('chains multiple bursts: each in-flight window yields at most one trailing run', async () => {
let calls = 0
const gates: Deferred[] = []
const c = createSingleFlightCoalescer(() => {
calls++
const d = deferred()
gates.push(d)
return d.promise
})
c.request()
await flush()
c.request() // pending during run 1
gates[0].resolve()
await flush()
expect(calls).toBe(2) // trailing run 2 started
c.request() // pending during run 2
c.request()
gates[1].resolve()
await flush()
expect(calls).toBe(3) // one trailing run 3
gates[2].resolve()
await flush()
expect(calls).toBe(3)
})
it('does not wedge when the task throws synchronously', async () => {
let calls = 0
const c = createSingleFlightCoalescer(() => {
calls++
if (calls === 1) {
throw new Error('sync boom')
}
return Promise.resolve()
})
c.request()
await flush()
expect(calls).toBe(1)
// A later request must still run (inFlight was released despite the throw).
c.request()
await flush()
expect(calls).toBe(2)
})
it('still runs the trailing run after a rejected task', async () => {
let calls = 0
const gates: Deferred[] = []
const c = createSingleFlightCoalescer(() => {
calls++
const d = deferred()
gates.push(d)
return d.promise
})
c.request()
await flush()
c.request() // pending
gates[0].reject(new Error('async boom'))
await flush()
expect(calls).toBe(2) // trailing run still fires
gates[1].resolve()
await flush()
})
})

View File

@ -1,36 +0,0 @@
// Single-flight: runs `task` on the first request; requests during a run collapse into one
// trailing re-run. Stops staggered wake reconnects firing K sidebar refreshes at once (#8539).
export type SingleFlightCoalescer = {
request: () => void
}
export function createSingleFlightCoalescer(task: () => Promise<unknown>): SingleFlightCoalescer {
let inFlight = false
let pending = false
const run = (): void => {
inFlight = true
// Microtask-defer so a sync throw in `task` can't wedge inFlight.
Promise.resolve()
.then(task)
.catch(() => {})
.finally(() => {
inFlight = false
if (!pending) {
return
}
pending = false
run()
})
}
return {
request: () => {
if (inFlight) {
pending = true
return
}
run()
}
}
}

View File

@ -23,7 +23,7 @@ describe('crash-reporting shared helpers', () => {
const longStack = [
'Error: boom',
...Array.from(
{ length: 80 },
{ length: 200 },
(_, index) => `at Component${index} (/Users/alice/project/src/file-${index}.tsx:1:1)`
)
].join('\n')
@ -48,6 +48,15 @@ describe('crash-reporting shared helpers', () => {
expect(
String(sanitizeCrashReportDetails({ error_stack: longStack }).error_stack).length
).toBeGreaterThan(240)
expect(String(sanitizeCrashReportDetails({ errorStack: longStack }).errorStack).length).toBe(
4_003
)
expect(
String(sanitizeCrashReportDetails({ componentStack: longStack }).componentStack).length
).toBe(4_003)
expect(String(sanitizeCrashReportDetails({ description: longStack }).description).length).toBe(
243
)
})
it('sanitizes breadcrumb data and caps to the latest thirty entries', () => {

View File

@ -183,7 +183,8 @@ export function sanitizeCrashReportString(
}
function maxDetailStringLengthForKey(key: string): number {
return /(?:^|_)(?:stack|component_stack|error_stack)$/i.test(key)
const normalizedKey = key.replace(/([a-z0-9])([A-Z])/g, '$1_$2')
return /(?:^|_)(?:stack|component_stack|error_stack)$/i.test(normalizedKey)
? MAX_STACK_DETAIL_LENGTH
: MAX_STRING_DETAIL_LENGTH
}