diff --git a/src/renderer/src/store/github/check-actions.ts b/src/renderer/src/store/github/check-actions.ts index d29a3720d38..51b9a891f1c 100644 --- a/src/renderer/src/store/github/check-actions.ts +++ b/src/renderer/src/store/github/check-actions.ts @@ -12,7 +12,11 @@ import { } from './cache-identity' import { isFresh, withBoundedCacheEntry } from './cache-policy' import { debouncedSaveCache } from './cache-persistence' -import { inflightChecksRequests } from './request-coordination' +import { + inflightChecksRequests, + nextProviderRequestId, + ownsInflightRequest +} from './request-coordination' import { getGitHubRepoSourceSettings, getGitHubWorkItemRequestContext } from './work-item-routing' export const createCheckActions = ( @@ -87,18 +91,26 @@ export const createCheckActions = ( return cachedChecks } - const inflightRequest = inflightChecksRequests.get(inflightKey) - if (inflightRequest) { - if ( - (options?.force && !inflightRequest.force) || - (options?.noCache && !inflightRequest.noCache) - ) { - await inflightRequest.promise.catch(() => {}) - } else { + let waitedForUpgrade = false + for (;;) { + const inflightRequest = inflightChecksRequests.get(inflightKey) + if (!inflightRequest) { + break + } + const weakerThanRequested = + (options?.force && !inflightRequest.force) || (options?.noCache && !inflightRequest.noCache) + if (!weakerThanRequested) { return inflightRequest.promise } + // Why: wait out one weaker request so peers can share the upgrade, but never twice — a steady stream of weaker callers would otherwise starve this one forever. + if (waitedForUpgrade) { + break + } + waitedForUpgrade = true + await inflightRequest.promise.catch(() => {}) } + const requestId = nextProviderRequestId() const request = (async () => { try { const requestContext = getGitHubWorkItemRequestContext( @@ -131,6 +143,12 @@ export const createCheckActions = ( noCache: Boolean(options?.force || options?.noCache), sourceContext: options?.sourceContext })) as PRCheckDetail[]) + // Why: the bounded upgrade wait can leave us running beside a stronger request for this + // key. Both bypass gh's cache here, but the later-started one holds the newer run state — + // let only the key's current owner write checksCache and the PR status it derives. + if (!ownsInflightRequest(inflightChecksRequests, inflightKey, requestId)) { + return checks + } set((s) => { const nextState: Partial = { checksCache: withBoundedCacheEntry(s.checksCache, cacheKey, { @@ -168,13 +186,16 @@ export const createCheckActions = ( return latestCached.data } return [] - } finally { + } + })().finally(() => { + if (ownsInflightRequest(inflightChecksRequests, inflightKey, requestId)) { inflightChecksRequests.delete(inflightKey) } - })() + }) inflightChecksRequests.set(inflightKey, { promise: request, + requestId, force: Boolean(options?.force), noCache: Boolean(options?.force || options?.noCache) }) diff --git a/src/renderer/src/store/github/project-actions.ts b/src/renderer/src/store/github/project-actions.ts index f4c5fec3100..68fc2b0a37a 100644 --- a/src/renderer/src/store/github/project-actions.ts +++ b/src/renderer/src/store/github/project-actions.ts @@ -18,6 +18,8 @@ import { withBoundedCacheEntry, WORK_ITEMS_CACHE_TTL } from './cache-policy' import { acquireProviderRequestSlot as acquireWorkItemSlot, inflightProjectViewRequests, + nextProviderRequestId, + ownsInflightRequest, releaseProviderRequestSlot as releaseWorkItemSlot } from './request-coordination' import { @@ -57,16 +59,25 @@ export const createProjectActions = ( } } - const existing = inflightProjectViewRequests.get(requestKey) - if (existing) { + let waitedForUpgrade = false + for (;;) { + const existing = inflightProjectViewRequests.get(requestKey) + if (!existing) { + break + } // Why: a forcing caller must not dedupe to a non-forcing in-flight request; wait for it to settle, then issue a fresh forced call (mirrors fetchWorkItems). - if (options?.force && !existing.force) { - await existing.promise.catch(() => {}) - } else { + if (!options?.force || existing.force) { return existing.promise } + // Why: wait out one weaker request so peers can share the upgrade, but never twice — a steady stream of weaker callers would otherwise starve this one forever. + if (waitedForUpgrade) { + break + } + waitedForUpgrade = true + await existing.promise.catch(() => {}) } + const requestId = nextProviderRequestId() const request = (async (): Promise => { await acquireWorkItemSlot() try { @@ -79,6 +90,12 @@ export const createProjectActions = ( { timeoutMs: 60_000 } ) : await window.api.gh.getProjectViewTable(args) + // Why: the bounded upgrade wait can leave us running beside a stronger request for this + // key, so neither write below may land once it owns the key — a late non-OK reply would + // otherwise stamp its error over the fresher table (or over a newer error) at the known key. + if (!ownsInflightRequest(inflightProjectViewRequests, requestKey, requestId)) { + return envelope + } if (envelope.ok) { const table = envelope.data const key = projectViewCacheKey( @@ -119,12 +136,16 @@ export const createProjectActions = ( } } finally { releaseWorkItemSlot() + } + })().finally(() => { + if (ownsInflightRequest(inflightProjectViewRequests, requestKey, requestId)) { inflightProjectViewRequests.delete(requestKey) } - })() + }) inflightProjectViewRequests.set(requestKey, { promise: request, + requestId, force: Boolean(options?.force) }) return request diff --git a/src/renderer/src/store/github/provider-request-coalescing.test.ts b/src/renderer/src/store/github/provider-request-coalescing.test.ts new file mode 100644 index 00000000000..f012999ce65 --- /dev/null +++ b/src/renderer/src/store/github/provider-request-coalescing.test.ts @@ -0,0 +1,343 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { GitHubWorkItem, ListWorkItemsResult } from '../../../../shared/github/work-item-types' +import type { PRCheckDetail } from '../../../../shared/github/check-types' +import type { GitHubProjectTable } from '../../../../shared/github/project-types' +import type { GetProjectViewTableResult } from '../../../../shared/github/project-result-types' +import type { FetchOptions } from './cache-model' +import { projectViewCacheKey, projectViewRequestKey } from './cache-identity' +import { inflightProjectViewRequests } from './request-coordination' +import { + createTestStore, + mockApi, + resetRemoteRuntimeMocks +} from '../slices/github-slice-test-harness' + +function workItems(title: string): ListWorkItemsResult { + return { + items: [ + { + id: 'issue-1', + type: 'issue', + number: 1, + title, + state: 'open', + url: 'https://example.test/1', + labels: [], + updatedAt: '2026-09-25T00:00:00Z', + author: null, + repoId: 'repo-1' + } + ], + sources: { issues: null, prs: null, originCandidate: null, upstreamCandidate: null } + } +} + +const projectViewRequest = { + owner: 'acme', + ownerType: 'organization' as const, + projectNumber: 1, + viewId: 'view-1' +} +const projectViewCacheKey1 = projectViewCacheKey('organization', 'acme', 1, 'view-1') + +function projectTable(title: string): GitHubProjectTable { + return { + project: { + id: 'project-1', + owner: 'acme', + ownerType: 'organization', + number: 1, + title, + url: 'https://github.com/orgs/acme/projects/1' + }, + selectedView: { + id: 'view-1', + number: 1, + name: 'Table', + layout: 'TABLE_LAYOUT', + filter: '', + fields: [], + groupByFields: [], + sortByFields: [] + }, + rows: [], + totalCount: 0, + parentFieldDropped: false + } +} + +const strongerWorkItemOptions: FetchOptions[] = [ + { force: true }, + { force: true, noCache: true }, + { force: true, requireComplete: true } +] + +describe('GitHub provider request upgrade coalescing', () => { + beforeEach(() => { + vi.clearAllMocks() + resetRemoteRuntimeMocks() + }) + + it.each(strongerWorkItemOptions)('coalesces twenty work-item upgrades: %j', async (options) => { + const store = createTestStore() + const weak = Promise.withResolvers>() + const fresh = Promise.withResolvers>() + mockApi.gh.listWorkItems.mockReturnValueOnce(weak.promise).mockReturnValue(fresh.promise) + const first = store.getState().fetchWorkItems('repo-1', '/repo', 24, '') + const settled = vi.fn() + const followers = Array.from({ length: 20 }, () => + store.getState().fetchWorkItems('repo-1', '/repo', 24, '', options).then(settled) + ) + weak.resolve(workItems('weak')) + await first + await vi.waitFor(() => + expect(mockApi.gh.listWorkItems.mock.calls.length).toBeGreaterThanOrEqual(2) + ) + expect(settled).not.toHaveBeenCalled() + fresh.resolve(workItems('fresh')) + await Promise.all(followers) + expect(mockApi.gh.listWorkItems).toHaveBeenCalledTimes(2) + expect(settled).toHaveBeenCalledTimes(20) + expect(settled.mock.calls.every(([rows]) => rows[0].title === 'fresh')).toBe(true) + expect(mockApi.gh.listWorkItems).toHaveBeenLastCalledWith({ + repoPath: '/repo', + repoId: 'repo-1', + limit: 24, + query: undefined, + ...(options.noCache ? { noCache: true } : {}) + }) + }) + + it('never joins a weaker replacement and never waits out more than one', async () => { + const store = createTestStore() + const weak = Promise.withResolvers>() + const forced = Promise.withResolvers>() + const strict = Promise.withResolvers>() + mockApi.gh.listWorkItems + .mockReturnValueOnce(weak.promise) + .mockReturnValueOnce(forced.promise) + .mockReturnValue(strict.promise) + const first = store.getState().fetchWorkItems('repo-1', '/repo', 24, '') + const forcedFetch = store.getState().fetchWorkItems('repo-1', '/repo', 24, '', { force: true }) + const strictSettled = vi.fn() + const strictFollowers = Array.from({ length: 20 }, () => + store + .getState() + .fetchWorkItems('repo-1', '/repo', 24, '', { + force: true, + noCache: true, + requireComplete: true + }) + .then(strictSettled) + ) + weak.resolve(workItems('weak')) + await first + // Why: the strict callers must reach the bridge without waiting out the weaker + // replacement too — the upgrade wait is bounded, so a repeating weaker refresh + // can never starve them. They still share exactly one strict request. + await vi.waitFor(() => expect(mockApi.gh.listWorkItems).toHaveBeenCalledTimes(3)) + expect(mockApi.gh.listWorkItems).toHaveBeenLastCalledWith( + expect.objectContaining({ noCache: true }) + ) + expect(strictSettled).not.toHaveBeenCalled() + strict.resolve(workItems('strict')) + await Promise.all(strictFollowers) + forced.resolve(workItems('forced')) + await expect(forcedFetch).resolves.toEqual(workItems('forced').items) + expect(mockApi.gh.listWorkItems).toHaveBeenCalledTimes(3) + expect(strictSettled).toHaveBeenCalledTimes(20) + expect(strictSettled.mock.calls.every(([rows]) => rows[0].title === 'strict')).toBe(true) + }) + + it('keeps an invalidated request from removing its replacement dedupe entry', async () => { + const store = createTestStore() + const stale = Promise.withResolvers>() + const fresh = Promise.withResolvers>() + mockApi.gh.listWorkItems.mockReturnValueOnce(stale.promise).mockReturnValue(fresh.promise) + const first = store.getState().fetchWorkItems('repo-1', '/repo', 24, '') + store.getState().evictGitHubRepoCaches('repo-1', '/repo') + const replacement = store.getState().fetchWorkItems('repo-1', '/repo', 24, '', { force: true }) + stale.resolve(workItems('stale')) + await first + const joined = store.getState().fetchWorkItems('repo-1', '/repo', 24, '', { force: true }) + fresh.resolve(workItems('fresh')) + await expect(Promise.all([replacement, joined])).resolves.toEqual([ + workItems('fresh').items, + workItems('fresh').items + ]) + expect(mockApi.gh.listWorkItems).toHaveBeenCalledTimes(2) + }) + + it('rejects partial results for every complete-result waiter and allows a later retry', async () => { + const store = createTestStore() + const weak = Promise.withResolvers>() + const partial = { + ...workItems('partial'), + errors: { issues: { type: 'network_error' as const, message: 'offline' } } + } + mockApi.gh.listWorkItems.mockReturnValueOnce(weak.promise).mockResolvedValue(partial) + const first = store.getState().fetchWorkItems('repo-1', '/repo', 24, '') + const followers = Array.from({ length: 20 }, () => + store.getState().fetchWorkItems('repo-1', '/repo', 24, '', { + force: true, + requireComplete: true + }) + ) + const settled = Promise.allSettled(followers) + weak.resolve(workItems('weak')) + await first + expect((await settled).every((result) => result.status === 'rejected')).toBe(true) + expect(mockApi.gh.listWorkItems).toHaveBeenCalledTimes(2) + mockApi.gh.listWorkItems.mockResolvedValue(workItems('recovered')) + await expect( + store.getState().fetchWorkItems('repo-1', '/repo', 24, '', { + force: true, + requireComplete: true + }) + ).resolves.toEqual(workItems('recovered').items) + expect(mockApi.gh.listWorkItems).toHaveBeenCalledTimes(3) + }) + + it.each([{ force: true }, { noCache: true }])( + 'coalesces twenty check upgrades: %j', + async (options) => { + const store = createTestStore() + const weak = Promise.withResolvers() + const fresh = Promise.withResolvers() + mockApi.gh.prChecks.mockReturnValueOnce(weak.promise).mockReturnValue(fresh.promise) + const first = store.getState().fetchPRChecks('/repo', 1, 'main', 'sha') + const settled = vi.fn() + const followers = Array.from({ length: 20 }, () => + store.getState().fetchPRChecks('/repo', 1, 'main', 'sha', undefined, options).then(settled) + ) + weak.resolve([]) + await first + await vi.waitFor(() => + expect(mockApi.gh.prChecks.mock.calls.length).toBeGreaterThanOrEqual(2) + ) + expect(settled).not.toHaveBeenCalled() + const checks: PRCheckDetail[] = [ + { name: 'fresh', status: 'completed', conclusion: 'success', url: null } + ] + fresh.resolve(checks) + await Promise.all(followers) + expect(mockApi.gh.prChecks).toHaveBeenCalledTimes(2) + expect(settled).toHaveBeenCalledTimes(20) + expect(settled.mock.calls.every(([rows]) => rows === checks)).toBe(true) + } + ) + + // Why: the bounded wait lets a force-only request (which still allows gh's own cache to answer) + // run beside a noCache one. Whichever settles last used to win the cache unconditionally. + it('keeps a late weaker work-item reply from burying the stronger result', async () => { + const store = createTestStore() + const weak = Promise.withResolvers>() + const forced = Promise.withResolvers>() + const strict = Promise.withResolvers>() + mockApi.gh.listWorkItems + .mockReturnValueOnce(weak.promise) + .mockReturnValueOnce(forced.promise) + .mockReturnValue(strict.promise) + const first = store.getState().fetchWorkItems('repo-1', '/repo', 24, '') + const forcedFetch = store.getState().fetchWorkItems('repo-1', '/repo', 24, '', { force: true }) + const strictFetch = store + .getState() + .fetchWorkItems('repo-1', '/repo', 24, '', { force: true, noCache: true }) + weak.resolve(workItems('weak')) + await first + await vi.waitFor(() => expect(mockApi.gh.listWorkItems).toHaveBeenCalledTimes(3)) + + strict.resolve(workItems('fresh')) + await expect(strictFetch).resolves.toEqual(workItems('fresh').items) + forced.resolve(workItems('gh-cached')) + await expect(forcedFetch).resolves.toEqual(workItems('gh-cached').items) + + expect(store.getState().getCachedWorkItems('repo-1', 24, '', '/repo')).toEqual( + workItems('fresh').items + ) + // Why: the stale payload must not be what isFresh hands back for the rest of the TTL either. + await expect(store.getState().fetchWorkItems('repo-1', '/repo', 24, '')).resolves.toEqual( + workItems('fresh').items + ) + expect(mockApi.gh.listWorkItems).toHaveBeenCalledTimes(3) + }) + + it('keeps a superseded check reply from rewinding the cached run state', async () => { + const store = createTestStore() + const plain = Promise.withResolvers() + const noCacheOnly = Promise.withResolvers() + const forcedOnly = Promise.withResolvers() + mockApi.gh.prChecks + .mockReturnValueOnce(plain.promise) + .mockReturnValueOnce(noCacheOnly.promise) + .mockReturnValue(forcedOnly.promise) + const first = store.getState().fetchPRChecks('/repo', 1, 'main', 'sha') + // Why: force and noCache are incomparable, so each waits out the plain request and the second + // one then breaks out of the bounded wait while the first is still in flight. + const noCacheFetch = store + .getState() + .fetchPRChecks('/repo', 1, 'main', 'sha', undefined, { noCache: true }) + const forcedFetch = store + .getState() + .fetchPRChecks('/repo', 1, 'main', 'sha', undefined, { force: true }) + plain.resolve([]) + await first + await vi.waitFor(() => expect(mockApi.gh.prChecks).toHaveBeenCalledTimes(3)) + + const newer: PRCheckDetail[] = [ + { name: 'build', status: 'completed', conclusion: 'success', url: null } + ] + const older: PRCheckDetail[] = [ + { name: 'build', status: 'in_progress', conclusion: null, url: null } + ] + forcedOnly.resolve(newer) + await expect(forcedFetch).resolves.toEqual(newer) + noCacheOnly.resolve(older) + await expect(noCacheFetch).resolves.toEqual(older) + + expect(Object.values(store.getState().checksCache).map((entry) => entry.data)).toEqual([newer]) + }) + + it('keeps a superseded project failure from stamping the known view key', async () => { + const store = createTestStore() + const weak = Promise.withResolvers() + const replacementWeak = Promise.withResolvers() + const forcedResult = Promise.withResolvers() + mockApi.gh.getProjectViewTable + .mockReturnValueOnce(weak.promise) + .mockReturnValueOnce(replacementWeak.promise) + .mockReturnValue(forcedResult.promise) + + const first = store.getState().fetchProjectViewTable(projectViewRequest) + const forcedFetch = store.getState().fetchProjectViewTable(projectViewRequest, { force: true }) + // Why: only a non-forced entry can be superseded mid-flight, and it has to appear in the + // microtask after the first request clears the key — before the forced waiter re-checks it. + const pending = inflightProjectViewRequests.get( + projectViewRequestKey(projectViewRequest, 'local') + ) + expect(pending).toBeDefined() + let replacementFetch: Promise | undefined + void pending?.promise.then(() => { + replacementFetch = store.getState().fetchProjectViewTable(projectViewRequest) + }) + + const staleError = { type: 'network_error' as const, message: 'first attempt offline' } + weak.resolve({ ok: false, error: staleError }) + await first + await vi.waitFor(() => expect(mockApi.gh.getProjectViewTable).toHaveBeenCalledTimes(3)) + expect(store.getState().projectViewCache[projectViewCacheKey1]).toMatchObject({ + error: staleError + }) + + forcedResult.resolve({ ok: true, data: projectTable('forced') }) + await expect(forcedFetch).resolves.toEqual({ ok: true, data: projectTable('forced') }) + const error = { type: 'network_error' as const, message: 'offline' } + replacementWeak.resolve({ ok: false, error }) + await expect(replacementFetch).resolves.toEqual({ ok: false, error }) + + expect(store.getState().projectViewCache[projectViewCacheKey1]).toMatchObject({ + data: projectTable('forced') + }) + expect(store.getState().projectViewCache[projectViewCacheKey1].error).toBeUndefined() + }) +}) diff --git a/src/renderer/src/store/github/request-coordination.ts b/src/renderer/src/store/github/request-coordination.ts index fc6f3cb0b7c..7aecfcd2b44 100644 --- a/src/renderer/src/store/github/request-coordination.ts +++ b/src/renderer/src/store/github/request-coordination.ts @@ -13,31 +13,56 @@ export type InflightPR = { } export type InflightChecks = { promise: Promise + requestId: number force: boolean noCache: boolean } export type InflightWorkItems = { promise: Promise + requestId: number force: boolean noCache: boolean requireComplete: boolean } +export type InflightProjectView = { + promise: Promise + requestId: number + force: boolean +} export const inflightPRRequests = new Map() export const inflightIssueRequests = new Map>() export const inflightChecksRequests = new Map() export const inflightCommentsRequests = new Map>() export const inflightWorkItemsRequests = new Map() -export const inflightProjectViewRequests = new Map< - string, - { promise: Promise; force: boolean } ->() +export const inflightProjectViewRequests = new Map() export const prRequestGenerations = new Map() export const prRefreshStartedHostedReviewEntries = new Map< string, AppState['hostedReviewCache'][string] | undefined >() +let providerRequestSequence = 0 + +/** Stamp identifying one provider request, captured before it awaits so it can recheck ownership after. */ +export function nextProviderRequestId(): number { + providerRequestSequence += 1 + return providerRequestSequence +} + +/** + * Why: the upgrade wait is bounded, so a stronger request can run beside a weaker one for the same + * key. Only the request the key currently resolves to may write that key's cache — otherwise a late + * weaker reply overwrites the stronger request's fresher result under a brand-new `fetchedAt`. + */ +export function ownsInflightRequest( + registry: ReadonlyMap, + key: string, + requestId: number +): boolean { + return registry.get(key)?.requestId === requestId +} + export function _getGitHubPRRequestGenerationCountForTest(): number { return prRequestGenerations.size } diff --git a/src/renderer/src/store/github/work-item-fetch-actions.ts b/src/renderer/src/store/github/work-item-fetch-actions.ts index 722556ee0ff..5d865741931 100644 --- a/src/renderer/src/store/github/work-item-fetch-actions.ts +++ b/src/renderer/src/store/github/work-item-fetch-actions.ts @@ -13,6 +13,8 @@ import { isFresh, withBoundedCacheEntry, WORK_ITEMS_CACHE_TTL } from './cache-po import { acquireProviderRequestSlot as acquireWorkItemSlot, inflightWorkItemsRequests, + nextProviderRequestId, + ownsInflightRequest, releaseProviderRequestSlot as releaseWorkItemSlot } from './request-coordination' import { findRepoForGitHubOwner } from './repository-routing' @@ -109,20 +111,29 @@ export const createWorkItemFetchActions = ( options?.sourceContext ) const inflightKey = workItemsInflightRequestKey(key, requestContext.target) - const existing = inflightWorkItemsRequests.get(inflightKey) - if (existing) { + let waitedForUpgrade = false + for (;;) { + const existing = inflightWorkItemsRequests.get(inflightKey) + if (!existing) { + break + } // Why: a forcing/noCache caller must not dedupe to a weaker in-flight fetch (noCache is stricter — it must bypass gh api's cache too). - if ( + const weakerThanRequested = (options?.force && !existing.force) || (options?.noCache && !existing.noCache) || (options?.requireComplete && !existing.requireComplete) - ) { - await existing.promise.catch(() => {}) - } else { + if (!weakerThanRequested) { return existing.promise } + // Why: wait out one weaker request so peers can share the upgrade, but never twice — a steady stream of weaker callers would otherwise starve this one forever. + if (waitedForUpgrade) { + break + } + waitedForUpgrade = true + await existing.promise.catch(() => {}) } + const requestId = nextProviderRequestId() const request = (async () => { await acquireWorkItemSlot() try { @@ -163,6 +174,12 @@ export const createWorkItemFetchActions = ( if (get().workItemsInvalidationNonce !== requestInvalidationNonce) { return items } + // Why: a stronger request may have replaced us on this key while we were awaiting (a + // force-only caller still lets gh's own cache answer), so writing here would bury its + // fresher rows under a new fetchedAt and keep isFresh serving them for the whole TTL. + if (!ownsInflightRequest(inflightWorkItemsRequests, inflightKey, requestId)) { + return items + } // Why: TaskPage useShallow-selects cache entry refs. A new { ...entry, fetchedAt } // still remaps every visible row. IPC structuredClone rebuilds nested records, so // data === previous.data never holds — reconcile structurally, then either mutate @@ -220,12 +237,16 @@ export const createWorkItemFetchActions = ( throw err } finally { releaseWorkItemSlot() + } + })().finally(() => { + if (ownsInflightRequest(inflightWorkItemsRequests, inflightKey, requestId)) { inflightWorkItemsRequests.delete(inflightKey) } - })() + }) inflightWorkItemsRequests.set(inflightKey, { promise: request, + requestId, force: Boolean(options?.force), noCache: Boolean(options?.noCache), requireComplete: Boolean(options?.requireComplete) diff --git a/src/renderer/src/store/slices/github-project-request-coordination.test.ts b/src/renderer/src/store/slices/github-project-request-coordination.test.ts index 96e4632e909..40a64f77ce7 100644 --- a/src/renderer/src/store/slices/github-project-request-coordination.test.ts +++ b/src/renderer/src/store/slices/github-project-request-coordination.test.ts @@ -73,7 +73,7 @@ describe('createGitHubSlice.fetchProjectViewTable coordination', () => { expect(mockApi.gh.getProjectViewTable).toHaveBeenCalledTimes(1) }) - it('lets each forced waiter start after a weaker request settles', async () => { + it('shares one fresh request across twenty forced waiters after a weaker request', async () => { const store = createTestStore() const weak = Promise.withResolvers() mockApi.gh.getProjectViewTable @@ -81,12 +81,16 @@ describe('createGitHubSlice.fetchProjectViewTable coordination', () => { .mockResolvedValue({ ok: true, data: makeTable('forced') }) const first = store.getState().fetchProjectViewTable(request) - const forcedOne = store.getState().fetchProjectViewTable(request, { force: true }) - const forcedTwo = store.getState().fetchProjectViewTable(request, { force: true }) + const forced = Array.from({ length: 20 }, () => + store.getState().fetchProjectViewTable(request, { force: true }) + ) weak.resolve({ ok: true, data: makeTable('weak') }) - await expect(Promise.all([first, forcedOne, forcedTwo])).resolves.toHaveLength(3) - expect(mockApi.gh.getProjectViewTable).toHaveBeenCalledTimes(3) + await expect(first).resolves.toEqual({ ok: true, data: makeTable('weak') }) + await expect(Promise.all(forced)).resolves.toEqual( + Array.from({ length: 20 }, () => ({ ok: true, data: makeTable('forced') })) + ) + expect(mockApi.gh.getProjectViewTable).toHaveBeenCalledTimes(2) }) it('stamps a classified failure onto stale data only when the view key is known', async () => {