From 16d06a4f82681a924067b96e4bc0f7f9d517f145 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=A8=8B=E5=BA=8F=E5=91=98=E9=98=BF=E6=B1=9F=28Relakkes?= =?UTF-8?q?=29?= Date: Fri, 10 Jul 2026 04:04:59 +0800 Subject: [PATCH] fix(desktop): prevent overlapping API polls #993 Coalesce concurrent session-list scans and keep desktop pollers from stacking requests while the local server is slow. Tested: bun run check:server (1489 pass) Tested: desktop focused tests, typecheck, and production build Scope-risk: moderate --- desktop/src/api/client.test.ts | 27 ++++ desktop/src/api/client.ts | 8 +- .../src/components/layout/Sidebar.test.tsx | 18 +++ desktop/src/components/layout/Sidebar.tsx | 2 +- ...ScheduledTaskDesktopNotifications.test.tsx | 91 ++++++++++- .../useScheduledTaskDesktopNotifications.ts | 21 ++- src/server/__tests__/sessions.test.ts | 142 ++++++++++++++++++ src/server/services/sessionService.ts | 72 +++++++-- 8 files changed, 361 insertions(+), 20 deletions(-) diff --git a/desktop/src/api/client.test.ts b/desktop/src/api/client.test.ts index 7ee05058..e2223d2f 100644 --- a/desktop/src/api/client.test.ts +++ b/desktop/src/api/client.test.ts @@ -166,4 +166,31 @@ describe('api diagnostics reporting', () => { const body = JSON.parse(String((init as RequestInit).body)) expect(body.type).toBe('client_window_error') }) + + it('bounds raw diagnostics requests when the local server is unresponsive', async () => { + vi.useFakeTimers() + let signal: AbortSignal | undefined + const fetchMock = vi.spyOn(globalThis, 'fetch') + fetchMock.mockImplementation((_url, init) => { + signal = init?.signal ?? undefined + expect(signal).toBeInstanceOf(AbortSignal) + return new Promise((resolve) => { + signal?.addEventListener('abort', () => { + resolve(new Response(null, { status: 503 })) + }) + }) + }) + + const request = rawRecordDiagnosticEvent({ + type: 'client_api_request_failed', + severity: 'warn', + summary: 'server stalled', + }) + + await vi.advanceTimersByTimeAsync(5_000) + await request + + expect(signal?.aborted).toBe(true) + expect(fetchMock).toHaveBeenCalledTimes(1) + }) }) diff --git a/desktop/src/api/client.ts b/desktop/src/api/client.ts index fcb2367d..7999e551 100644 --- a/desktop/src/api/client.ts +++ b/desktop/src/api/client.ts @@ -11,6 +11,7 @@ let baseUrl = DEFAULT_BASE_URL let authToken: string | null = null const DIAGNOSTICS_PATH = '/api/diagnostics/events' const DEFAULT_REQUEST_TIMEOUT_MS = 120_000 +const DIAGNOSTICS_REQUEST_TIMEOUT_MS = 5_000 function getErrorMessage(status: number, body: unknown) { if (body && typeof body === 'object' && 'message' in body && typeof body.message === 'string') { @@ -133,11 +134,16 @@ export function rawRecordDiagnosticEvent(event: { sessionId?: string details?: unknown }) { + const controller = new AbortController() + const timeout = setTimeout(() => controller.abort(), DIAGNOSTICS_REQUEST_TIMEOUT_MS) return fetch(`${baseUrl}${DIAGNOSTICS_PATH}`, { method: 'POST', headers: buildHeaders(), body: JSON.stringify(event), - }).catch(() => undefined) + signal: controller.signal, + }) + .catch(() => undefined) + .finally(() => clearTimeout(timeout)) } function buildHeaders(): Record { diff --git a/desktop/src/components/layout/Sidebar.test.tsx b/desktop/src/components/layout/Sidebar.test.tsx index 99465d3a..b058f593 100644 --- a/desktop/src/components/layout/Sidebar.test.tsx +++ b/desktop/src/components/layout/Sidebar.test.tsx @@ -1260,6 +1260,24 @@ describe('Sidebar', () => { expect(fetchSessions).toHaveBeenCalledTimes(3) }) + it('does not overlap automatic session refreshes when the previous request is still pending', async () => { + vi.useFakeTimers() + fetchSessions.mockReturnValue(new Promise(() => {})) + + render() + await act(async () => { + await Promise.resolve() + }) + expect(fetchSessions).toHaveBeenCalledTimes(1) + + await act(async () => { + vi.advanceTimersByTime(90_000) + await Promise.resolve() + }) + + expect(fetchSessions).toHaveBeenCalledTimes(1) + }) + it('does not poll for session changes while the document is hidden', async () => { vi.useFakeTimers() const originalVisibility = document.visibilityState diff --git a/desktop/src/components/layout/Sidebar.tsx b/desktop/src/components/layout/Sidebar.tsx index edfda0c1..c6080879 100644 --- a/desktop/src/components/layout/Sidebar.tsx +++ b/desktop/src/components/layout/Sidebar.tsx @@ -1264,7 +1264,7 @@ function useSessionListAutoRefresh(fetchSessions: () => Promise): () => Pr document.addEventListener('visibilitychange', refreshIfVisible) const timer = window.setInterval(() => { if (!isDocumentVisible()) return - void refreshSessions(true) + void refreshSessions() }, SESSION_LIST_AUTO_REFRESH_MS) return () => { diff --git a/desktop/src/hooks/useScheduledTaskDesktopNotifications.test.tsx b/desktop/src/hooks/useScheduledTaskDesktopNotifications.test.tsx index de6a415a..164bee64 100644 --- a/desktop/src/hooks/useScheduledTaskDesktopNotifications.test.tsx +++ b/desktop/src/hooks/useScheduledTaskDesktopNotifications.test.tsx @@ -60,8 +60,94 @@ describe('useScheduledTaskDesktopNotifications', () => { expect(getRecentRunsMock).not.toHaveBeenCalled() resolveReady() - await vi.waitFor(() => expect(getRecentRunsMock).toHaveBeenCalledTimes(1)) + await vi.waitFor(() => expect(listMock).toHaveBeenCalledTimes(1)) + expect(getRecentRunsMock).not.toHaveBeenCalled() + }) + + it('does not overlap scheduled-task polls while the previous task request is pending', async () => { + let resolveTasks: (value: { tasks: [] }) => void = () => {} + listMock.mockReturnValue(new Promise<{ tasks: [] }>((resolve) => { + resolveTasks = resolve + })) + + render() + await vi.waitFor(() => expect(listMock).toHaveBeenCalledTimes(1)) + + await vi.advanceTimersByTimeAsync(90_000) expect(listMock).toHaveBeenCalledTimes(1) + expect(getRecentRunsMock).not.toHaveBeenCalled() + + resolveTasks({ tasks: [] }) + await vi.runAllTicks() + }) + + it('does not overlap recent-run requests while the previous run poll is pending', async () => { + listMock.mockResolvedValue({ + tasks: [{ + id: 'task-1', + name: 'Daily review', + cron: '* * * * *', + prompt: 'review', + enabled: true, + createdAt: 1, + notification: { enabled: true, channels: ['desktop'] }, + }], + }) + let resolveRuns: (value: { runs: [] }) => void = () => {} + getRecentRunsMock.mockReturnValue(new Promise<{ runs: [] }>((resolve) => { + resolveRuns = resolve + })) + + render() + await vi.waitFor(() => expect(getRecentRunsMock).toHaveBeenCalledTimes(1)) + + await vi.advanceTimersByTimeAsync(90_000) + expect(listMock).toHaveBeenCalledTimes(1) + expect(getRecentRunsMock).toHaveBeenCalledTimes(1) + + resolveRuns({ runs: [] }) + await vi.runAllTicks() + }) + + it('notifies the first completed desktop task created after an empty initial poll', async () => { + const desktopTask = { + id: 'task-new', + name: 'New task', + cron: '* * * * *', + prompt: 'review', + enabled: true, + createdAt: 1, + notification: { enabled: true, channels: ['desktop'] }, + } + listMock + .mockResolvedValueOnce({ tasks: [] }) + .mockResolvedValue({ tasks: [desktopTask] }) + getRecentRunsMock.mockResolvedValue({ + runs: [{ + id: 'run-new-task', + taskId: 'task-new', + taskName: 'New task', + startedAt: '2026-05-03T00:01:00.000Z', + completedAt: '2026-05-03T00:01:01.000Z', + status: 'completed', + prompt: 'review', + output: 'done', + }], + }) + + render() + await vi.waitFor(() => expect(listMock).toHaveBeenCalledTimes(1)) + expect(getRecentRunsMock).not.toHaveBeenCalled() + + await vi.advanceTimersByTimeAsync(30_000) + await vi.waitFor(() => expect(notifyDesktopMock).toHaveBeenCalledTimes(1)) + + expect(notifyDesktopMock).toHaveBeenCalledWith({ + dedupeKey: 'scheduled-task:run-new-task', + title: '定时任务 New task', + body: '完成: done', + target: { type: 'scheduled' }, + }) }) it('does not notify old runs on first poll and notifies new desktop-enabled task runs later', async () => { @@ -198,9 +284,10 @@ describe('useScheduledTaskDesktopNotifications', () => { }) render() - await vi.waitFor(() => expect(getRecentRunsMock).toHaveBeenCalledTimes(1)) + await vi.waitFor(() => expect(listMock).toHaveBeenCalledTimes(1)) await vi.advanceTimersByTimeAsync(30_000) + expect(getRecentRunsMock).not.toHaveBeenCalled() expect(notifyDesktopMock).not.toHaveBeenCalled() }) diff --git a/desktop/src/hooks/useScheduledTaskDesktopNotifications.ts b/desktop/src/hooks/useScheduledTaskDesktopNotifications.ts index 358a4394..8b4e3e2e 100644 --- a/desktop/src/hooks/useScheduledTaskDesktopNotifications.ts +++ b/desktop/src/hooks/useScheduledTaskDesktopNotifications.ts @@ -70,17 +70,26 @@ export function useScheduledTaskDesktopNotifications(): void { let stopped = false let initialized = false let interval: number | undefined + let pollInFlight = false const poll = async () => { + if (stopped || pollInFlight) return + pollInFlight = true try { - const [{ tasks }, { runs }] = await Promise.all([ - tasksApi.list(), - tasksApi.getRecentRuns(50), - ]) + const { tasks } = await tasksApi.list() + if (stopped) return + + const desktopTasks = tasks.filter(hasDesktopNotification) + if (desktopTasks.length === 0) { + initialized = true + return + } + + const { runs } = await tasksApi.getRecentRuns(50) if (stopped) return const notifiedRunIds = readNotifiedRunIds() - const pendingRuns = collectDesktopNotifiableRuns(tasks, runs, notifiedRunIds) + const pendingRuns = collectDesktopNotifiableRuns(desktopTasks, runs, notifiedRunIds) if (!initialized) { for (const run of pendingRuns) notifiedRunIds.add(run.id) @@ -106,6 +115,8 @@ export function useScheduledTaskDesktopNotifications(): void { if (typeof console !== 'undefined') { console.warn('[scheduledTaskNotifications] failed to poll task runs:', err) } + } finally { + pollInFlight = false } } diff --git a/src/server/__tests__/sessions.test.ts b/src/server/__tests__/sessions.test.ts index 32402aab..854e7d18 100644 --- a/src/server/__tests__/sessions.test.ts +++ b/src/server/__tests__/sessions.test.ts @@ -649,6 +649,148 @@ describe('SessionService', () => { expect(scanCount).toBe(5) }) + it('should coalesce concurrent session list scans for the same query', async () => { + for (let i = 0; i < 3; i++) { + const id = `2400000${i.toString(16)}-bbbb-cccc-dddd-eeeeeeeeeeee` + await writeSessionFile('-tmp-concurrent-session-list', id, [ + makeSnapshotEntry(), + makeUserEntry(`Concurrent message ${i}`), + ]) + } + + const serviceWithSpy = service as unknown as { + scanSessionListSummary: (...args: unknown[]) => Promise + } + const originalScanSessionListSummary = serviceWithSpy.scanSessionListSummary.bind(service) + let scanCount = 0 + let releaseFirstScan: () => void = () => {} + let markFirstScanStarted: () => void = () => {} + const firstScanStarted = new Promise((resolve) => { + markFirstScanStarted = resolve + }) + const firstScanGate = new Promise((resolve) => { + releaseFirstScan = resolve + }) + + serviceWithSpy.scanSessionListSummary = async (...args) => { + scanCount += 1 + if (scanCount === 1) { + markFirstScanStarted() + await firstScanGate + } + return originalScanSessionListSummary(...args) + } + + const first = service.listSessions({ limit: 3, offset: 0 }) + await firstScanStarted + const second = service.listSessions({ limit: 3, offset: 0 }) + await new Promise((resolve) => setTimeout(resolve, 10)) + releaseFirstScan() + + const [firstResult, secondResult] = await Promise.all([first, second]) + + expect(firstResult).toEqual(secondResult) + expect(scanCount).toBe(3) + }) + + it('should coalesce file summary scans across concurrent pagination queries', async () => { + for (let i = 0; i < 3; i++) { + const id = `2420000${i.toString(16)}-bbbb-cccc-dddd-eeeeeeeeeeee` + await writeSessionFile('-tmp-concurrent-session-pages', id, [ + makeSnapshotEntry(), + makeUserEntry(`Concurrent page message ${i}`), + ]) + } + + const serviceWithSpy = service as unknown as { + scanSessionListSummary: (...args: unknown[]) => Promise + } + const originalScanSessionListSummary = serviceWithSpy.scanSessionListSummary.bind(service) + let scanCount = 0 + let releaseFirstScan: () => void = () => {} + let markFirstScanStarted: () => void = () => {} + const firstScanStarted = new Promise((resolve) => { + markFirstScanStarted = resolve + }) + const firstScanGate = new Promise((resolve) => { + releaseFirstScan = resolve + }) + + serviceWithSpy.scanSessionListSummary = async (...args) => { + scanCount += 1 + if (scanCount === 1) { + markFirstScanStarted() + await firstScanGate + } + return originalScanSessionListSummary(...args) + } + + const sidebarRequest = service.listSessions({ limit: 400, offset: 0 }) + await firstScanStarted + const tabRestoreRequest = service.listSessions({ limit: 200, offset: 0 }) + await new Promise((resolve) => setTimeout(resolve, 10)) + releaseFirstScan() + + const [sidebarResult, tabRestoreResult] = await Promise.all([ + sidebarRequest, + tabRestoreRequest, + ]) + + expect(sidebarResult.sessions).toHaveLength(3) + expect(tabRestoreResult.sessions).toHaveLength(3) + expect(scanCount).toBe(3) + }) + + it('should not reuse or cache a list scan started before session metadata changes', async () => { + const sessionId = '24500000-bbbb-cccc-dddd-eeeeeeeeeeee' + await writeSessionFile('-tmp-invalidated-session-list', sessionId, [ + makeSnapshotEntry(), + makeUserEntry('Original title'), + ]) + + const serviceWithSpy = service as unknown as { + scanSessionListSummary: (...args: unknown[]) => Promise + } + const originalScanSessionListSummary = serviceWithSpy.scanSessionListSummary.bind(service) + let scanCount = 0 + let releaseFirstScan: () => void = () => {} + let markFirstScanStarted: () => void = () => {} + const firstScanStarted = new Promise((resolve) => { + markFirstScanStarted = resolve + }) + const firstScanGate = new Promise((resolve) => { + releaseFirstScan = resolve + }) + + serviceWithSpy.scanSessionListSummary = async (...args) => { + scanCount += 1 + const summary = await originalScanSessionListSummary(...args) + if (scanCount === 1) { + markFirstScanStarted() + await firstScanGate + } + return summary + } + + const staleRequest = service.listSessions({ limit: 10, offset: 0 }) + await firstScanStarted + await service.renameSession(sessionId, 'Renamed while scanning') + + const freshRequest = service.listSessions({ limit: 10, offset: 0 }) + await new Promise((resolve) => setTimeout(resolve, 10)) + expect(scanCount).toBe(2) + + const freshResult = await freshRequest + expect(freshResult.sessions[0]?.title).toBe('Renamed while scanning') + + releaseFirstScan() + const staleResult = await staleRequest + expect(staleResult.sessions[0]?.title).toBe('Original title') + + const cachedResult = await service.listSessions({ limit: 10, offset: 0 }) + expect(cachedResult.sessions[0]?.title).toBe('Renamed while scanning') + }) + it('should reuse unchanged file summaries after the list response cache is cleared', async () => { const sessionFiles: Array<{ id: string; filePath: string }> = [] for (let i = 0; i < 3; i++) { diff --git a/src/server/services/sessionService.ts b/src/server/services/sessionService.ts index 4496c860..86338d33 100644 --- a/src/server/services/sessionService.ts +++ b/src/server/services/sessionService.ts @@ -374,7 +374,13 @@ export class SessionService { expiresAt: number result: { sessions: SessionListItem[]; total: number } }>() + private readonly sessionListRequests = new Map< + string, + Promise<{ sessions: SessionListItem[]; total: number }> + >() + private sessionListCacheGeneration = 0 private readonly sessionListSummaryCache = new Map() + private readonly sessionListSummaryRequests = new Map>() private sessionListCacheKey(options?: { project?: string @@ -397,6 +403,7 @@ export class SessionService { private invalidateSessionListCache(): void { this.sessionListCache.clear() + this.sessionListCacheGeneration += 1 } private cloneSessionListSummary(summary: SessionListSummary): SessionListSummary { @@ -477,13 +484,27 @@ export class SessionService { return this.cloneSessionListSummary(cached.summary) } - const summary = await this.scanSessionListSummary(filePath, projectDir, stat) - this.sessionListSummaryCache.set(filePath, { - mtimeMs: stat.mtimeMs, - size: stat.size, - summary: this.cloneSessionListSummary(summary), - }) - return summary + const requestKey = `${filePath}:${stat.mtimeMs}:${stat.size}` + const inFlight = this.sessionListSummaryRequests.get(requestKey) + if (inFlight) { + return this.cloneSessionListSummary(await inFlight) + } + + const request = this.scanSessionListSummary(filePath, projectDir, stat) + this.sessionListSummaryRequests.set(requestKey, request) + try { + const summary = await request + this.sessionListSummaryCache.set(filePath, { + mtimeMs: stat.mtimeMs, + size: stat.size, + summary: this.cloneSessionListSummary(summary), + }) + return this.cloneSessionListSummary(summary) + } finally { + if (this.sessionListSummaryRequests.get(requestKey) === request) { + this.sessionListSummaryRequests.delete(requestKey) + } + } } // -------------------------------------------------------------------------- @@ -2340,6 +2361,33 @@ export class SessionService { return this.cloneSessionListResult(cached.result) } + const cacheGeneration = this.sessionListCacheGeneration + const requestKey = `${cacheGeneration}:${cacheKey}` + const inFlight = this.sessionListRequests.get(requestKey) + if (inFlight) { + return this.cloneSessionListResult(await inFlight) + } + + const request = this.loadSessionList(options, cacheKey, cacheGeneration) + this.sessionListRequests.set(requestKey, request) + try { + return this.cloneSessionListResult(await request) + } finally { + if (this.sessionListRequests.get(requestKey) === request) { + this.sessionListRequests.delete(requestKey) + } + } + } + + private async loadSessionList( + options: { + project?: string + limit?: number + offset?: number + } | undefined, + cacheKey: string, + cacheGeneration: number, + ): Promise<{ sessions: SessionListItem[]; total: number }> { const sessionFiles = await this.discoverSessionFiles(options?.project) const filesWithStats = (await Promise.all(sessionFiles.map(async (sessionFile) => { try { @@ -2417,10 +2465,12 @@ export class SessionService { } const result = { sessions: items, total } - this.sessionListCache.set(cacheKey, { - expiresAt: Date.now() + this.sessionListCacheTtlMs, - result: this.cloneSessionListResult(result), - }) + if (cacheGeneration === this.sessionListCacheGeneration) { + this.sessionListCache.set(cacheKey, { + expiresAt: Date.now() + this.sessionListCacheTtlMs, + result: this.cloneSessionListResult(result), + }) + } return result }