diff --git a/src/main/github/client-issue-source.test.ts b/src/main/github/client-issue-source.test.ts index d65c37841b6..f2ec208f8bd 100644 --- a/src/main/github/client-issue-source.test.ts +++ b/src/main/github/client-issue-source.test.ts @@ -222,6 +222,98 @@ describe('GitHub issue source split', () => { ) }) + it("uses upstream for recent PRs when preference='upstream'", async () => { + resolveIssueSourceMock.mockResolvedValueOnce({ + source: { owner: 'stablyai', repo: 'orca' }, + fellBack: false + }) + getOwnerRepoMock.mockResolvedValueOnce({ owner: 'fork', repo: 'orca' }) + getOwnerRepoForRemoteMock.mockResolvedValueOnce({ owner: 'stablyai', repo: 'orca' }) + ghExecFileAsyncMock.mockResolvedValueOnce({ stdout: '[]' }).mockResolvedValueOnce({ + stdout: '[]' + }) + + await listWorkItems('/repo-root', 10, undefined, undefined, 'upstream') + + expect(ghExecFileAsyncMock).toHaveBeenNthCalledWith( + 2, + [ + 'api', + '--cache', + '120s', + 'repos/stablyai/orca/pulls?per_page=10&state=open&sort=updated&direction=desc' + ], + { cwd: '/repo-root' } + ) + }) + + it("uses upstream for queried PRs when preference='upstream'", async () => { + resolveIssueSourceMock.mockResolvedValueOnce({ + source: { owner: 'stablyai', repo: 'orca' }, + fellBack: false + }) + getOwnerRepoMock.mockResolvedValueOnce({ owner: 'fork', repo: 'orca' }) + getOwnerRepoForRemoteMock.mockResolvedValueOnce({ owner: 'stablyai', repo: 'orca' }) + ghExecFileAsyncMock.mockResolvedValueOnce({ stdout: '[]' }) + + await listWorkItems('/repo-root', 10, 'is:pr is:open', undefined, 'upstream') + + expect(ghExecFileAsyncMock).toHaveBeenCalledWith( + expect.arrayContaining(['--repo', 'stablyai/orca']), + { cwd: '/repo-root' } + ) + }) + + it("uses upstream for PR counts when preference='upstream'", async () => { + resolveIssueSourceMock.mockResolvedValueOnce({ + source: { owner: 'stablyai', repo: 'orca' }, + fellBack: false + }) + getOwnerRepoMock.mockResolvedValueOnce({ owner: 'fork', repo: 'orca' }) + getOwnerRepoForRemoteMock.mockResolvedValueOnce({ owner: 'stablyai', repo: 'orca' }) + ghExecFileAsyncMock.mockResolvedValueOnce({ stdout: '9\n' }) + + const count = await countWorkItems('/repo-root', 'is:pr is:open', 'upstream') + + expect(count).toBe(9) + expect(ghExecFileAsyncMock).toHaveBeenCalledTimes(1) + expect(ghExecFileAsyncMock).toHaveBeenCalledWith( + [ + 'api', + '--cache', + '120s', + `search/issues?q=${encodeURIComponent('repo:stablyai/orca is:pull-request is:open')}&per_page=1`, + '--jq', + '.total_count' + ], + { cwd: '/repo-root' } + ) + }) + + it("falls back to origin for PRs when preference='upstream' and upstream is missing", async () => { + resolveIssueSourceMock.mockResolvedValueOnce({ + source: { owner: 'fork', repo: 'orca' }, + fellBack: true + }) + getOwnerRepoMock.mockResolvedValueOnce({ owner: 'fork', repo: 'orca' }) + getOwnerRepoForRemoteMock.mockResolvedValueOnce(null) + ghExecFileAsyncMock.mockResolvedValueOnce({ stdout: '[]' }) + + const result = await listWorkItems('/repo-root', 10, 'is:pr', undefined, 'upstream') + + expect(ghExecFileAsyncMock).toHaveBeenCalledWith( + expect.arrayContaining(['--repo', 'fork/orca']), + { cwd: '/repo-root' } + ) + expect(result.sources).toEqual({ + issues: { owner: 'fork', repo: 'orca' }, + prs: { owner: 'fork', repo: 'orca' }, + originCandidate: { owner: 'fork', repo: 'orca' }, + upstreamCandidate: null + }) + expect(result.issueSourceFellBack).toBe(true) + }) + it('counts default work items across upstream issues and origin PRs', async () => { getIssueOwnerRepoMock.mockResolvedValueOnce({ owner: 'stablyai', repo: 'orca' }) getOwnerRepoMock.mockResolvedValueOnce({ owner: 'fork', repo: 'orca' }) @@ -563,6 +655,28 @@ describe('GitHub issue source split', () => { expect(result.sources).toEqual({ issues: { owner: 'fork', repo: 'orca' }, prs: { owner: 'fork', repo: 'orca' }, + originCandidate: { owner: 'fork', repo: 'orca' }, + upstreamCandidate: { owner: 'stablyai', repo: 'orca' } + }) + }) + + it('keeps raw origin metadata when effective PR source is upstream', async () => { + resolveIssueSourceMock.mockResolvedValueOnce({ + source: { owner: 'stablyai', repo: 'orca' }, + fellBack: false + }) + getOwnerRepoMock.mockResolvedValueOnce({ owner: 'fork', repo: 'orca' }) + getOwnerRepoForRemoteMock.mockResolvedValueOnce({ owner: 'stablyai', repo: 'orca' }) + ghExecFileAsyncMock.mockResolvedValueOnce({ stdout: '[]' }).mockResolvedValueOnce({ + stdout: '[]' + }) + + const result = await listWorkItems('/repo-root', 10, undefined, undefined, 'upstream') + + expect(result.sources).toEqual({ + issues: { owner: 'stablyai', repo: 'orca' }, + prs: { owner: 'stablyai', repo: 'orca' }, + originCandidate: { owner: 'fork', repo: 'orca' }, upstreamCandidate: { owner: 'stablyai', repo: 'orca' } }) }) diff --git a/src/main/github/client.ts b/src/main/github/client.ts index c63edfeaf72..e105941361d 100644 --- a/src/main/github/client.ts +++ b/src/main/github/client.ts @@ -936,6 +936,26 @@ function assertSshRepoHasResolvedGitHubSource(args: { throw new Error(GITHUB_WORK_ITEMS_SSH_REMOTE_REQUIRED_MESSAGE) } +type ResolvedPrWorkItemSource = { + source: OwnerRepo | null + originCandidate: OwnerRepo | null + upstreamCandidate: OwnerRepo | null +} + +async function resolvePrWorkItemSource( + repoPath: string, + preference: IssueSourcePreference | undefined, + connectionId?: string | null +): Promise { + const [originCandidate, upstreamCandidate] = await Promise.all([ + getOwnerRepo(repoPath, connectionId), + getOwnerRepoForRemote(repoPath, 'upstream', connectionId) + ]) + const source = + preference === 'upstream' ? (upstreamCandidate ?? originCandidate) : originCandidate + return { source, originCandidate, upstreamCandidate } +} + async function listRecentWorkItems( repoPath: string, issueOwnerRepo: OwnerRepo | null, @@ -1194,17 +1214,12 @@ export async function listWorkItems( connectionId?: string | null, noCache?: boolean ): Promise> { - // Why: resolve the raw upstream candidate alongside the preference-aware - // issue source. The selector needs to know whether an upstream remote - // *exists* to decide whether to render — independent of whether the user - // has picked 'origin' (which would otherwise make `sources.issues` equal - // origin and hide the selector permanently). - const [issueResolved, prOwnerRepo, upstreamCandidate] = await Promise.all([ + const [issueResolved, prResolved] = await Promise.all([ resolveIssueSource(repoPath, preference, connectionId), - getOwnerRepo(repoPath, connectionId), - getOwnerRepoForRemote(repoPath, 'upstream', connectionId) + resolvePrWorkItemSource(repoPath, preference, connectionId) ]) const issueOwnerRepo = issueResolved.source + const prOwnerRepo = prResolved.source const trimmedQuery = query?.trim() ?? '' await acquire() try { @@ -1237,7 +1252,8 @@ export async function listWorkItems( sources: { issues: issueOwnerRepo, prs: prOwnerRepo, - upstreamCandidate: upstreamCandidate ?? null + originCandidate: prResolved.originCandidate, + upstreamCandidate: prResolved.upstreamCandidate }, ...(errors ? { errors } : {}), ...(issueResolved.fellBack ? { issueSourceFellBack: true } : {}) @@ -1352,11 +1368,12 @@ export async function countWorkItems( preference?: IssueSourcePreference, connectionId?: string | null ): Promise { - const [issueResolved, prOwnerRepo] = await Promise.all([ + const [issueResolved, prResolved] = await Promise.all([ resolveIssueSource(repoPath, preference, connectionId), - getOwnerRepo(repoPath, connectionId) + resolvePrWorkItemSource(repoPath, preference, connectionId) ]) const issueOwnerRepo = issueResolved.source + const prOwnerRepo = prResolved.source const ownerRepo = prOwnerRepo ?? issueOwnerRepo if (!ownerRepo) { return 0 diff --git a/src/renderer/src/components/TaskPage.tsx b/src/renderer/src/components/TaskPage.tsx index 454fe779504..40a529d8dbc 100644 --- a/src/renderer/src/components/TaskPage.tsx +++ b/src/renderer/src/components/TaskPage.tsx @@ -2429,19 +2429,19 @@ const hasDivergentSources = ( sources: { issues: GitHubOwnerRepo; prs: GitHubOwnerRepo } } => !!s.sources?.issues && !!s.sources.prs && !sameGitHubOwnerRepo(s.sources.issues, s.sources.prs) -// Why: the selector keeps rendering even after the user picks 'origin' (which -// collapses `sources.issues` onto origin). Upstream-candidate divergence is -// the right render gate — a repo that has an `upstream` remote pointing -// somewhere different from origin is always a candidate for the toggle, -// regardless of the current effective preference. +// Why: the selector keeps rendering even after the user picks 'upstream' (which +// makes effective `sources.prs` point at upstream). Raw-candidate divergence is the +// right render gate — a repo that has an `upstream` remote pointing somewhere +// different from origin is always a candidate for the toggle, regardless of +// the current effective preference. const hasUpstreamCandidateDivergence = ( s: TaskPageRepoSourceState ): s is TaskPageRepoSourceState & { - sources: { prs: GitHubOwnerRepo; upstreamCandidate: GitHubOwnerRepo } + sources: { originCandidate: GitHubOwnerRepo; upstreamCandidate: GitHubOwnerRepo } } => - !!s.sources?.prs && + !!s.sources?.originCandidate && !!s.sources.upstreamCandidate && - !sameGitHubOwnerRepo(s.sources.prs, s.sources.upstreamCandidate) + !sameGitHubOwnerRepo(s.sources.originCandidate, s.sources.upstreamCandidate) export default function TaskPage(): React.JSX.Element { useTranslation() @@ -2791,6 +2791,7 @@ export default function TaskPage(): React.JSX.Element { // collapse onto a stale in-flight request that resolved against the // pre-flip source). const lastFetchedInvalidationNonceRef = useRef(0) + const paginationGenerationRef = useRef(0) // Why: entering Tasks with fresh cache should still verify remote status // once, but the result is reconciled into existing rows to avoid a full // table shuffle when only status/key fields changed. @@ -2821,6 +2822,12 @@ export default function TaskPage(): React.JSX.Element { const fetchWorkItemsNextPage = useAppStore((s) => s.fetchWorkItemsNextPage) const countWorkItemsAcrossRepos = useAppStore((s) => s.countWorkItemsAcrossRepos) + useEffect(() => { + paginationGenerationRef.current += 1 + setPaginationLoading(false) + setLoadingTargetPage(null) + }, [selectedRepos, appliedTaskSearch, workItemsInvalidationNonce]) + // Why: clicking a GitHub row (or completing the create-issue flow) opens // this dialog for a read/review surface. The dialog's "Use" button routes // through the same direct-launch flow as the row-level "Use" CTA so @@ -4860,6 +4867,7 @@ export default function TaskPage(): React.JSX.Element { path: r.path, executionHostId: r.executionHostId })) + const requestGeneration = paginationGenerationRef.current const target = targetPage ?? pages.length setPaginationLoading(true) @@ -4877,6 +4885,9 @@ export default function TaskPage(): React.JSX.Element { q, cursor ) + if (paginationGenerationRef.current !== requestGeneration) { + return + } if (items.length === 0) { break } @@ -4892,8 +4903,10 @@ export default function TaskPage(): React.JSX.Element { } catch (err) { console.error('Failed to load next page:', err) } finally { - setPaginationLoading(false) - setLoadingTargetPage(null) + if (paginationGenerationRef.current === requestGeneration) { + setPaginationLoading(false) + setLoadingTargetPage(null) + } } }, [paginationLoading, selectedRepos, pages, appliedTaskSearch, fetchWorkItemsNextPage] @@ -7108,7 +7121,7 @@ export default function TaskPage(): React.JSX.Element { ) : null} { void setIssueSourcePreference(repo.id, repo.path, next) @@ -9713,17 +9726,19 @@ export default function TaskPage(): React.JSX.Element { return null } const entry = perRepoSourceState.find((s) => s.repoId === newIssueTargetRepo.id) - if (!entry || !entry.sources?.upstreamCandidate || !entry.sources?.prs) { + if (!entry || !entry.sources?.upstreamCandidate || !entry.sources?.originCandidate) { return null } - if (sameGitHubOwnerRepo(entry.sources.prs, entry.sources.upstreamCandidate)) { + if ( + sameGitHubOwnerRepo(entry.sources.originCandidate, entry.sources.upstreamCandidate) + ) { return null } return (
{ const store = createTestStore() type WorkItemsEnvelope = { items: [] - sources: { issues: null; prs: null; upstreamCandidate: null } + sources: { issues: null; prs: null; originCandidate: null; upstreamCandidate: null } } let resolveFirst: (value: WorkItemsEnvelope) => void = () => {} const firstRequest = new Promise((resolve) => { @@ -173,7 +173,7 @@ describe('createGitHubSlice.evictGitHubRepoCaches', () => { }) mockApi.gh.listWorkItems.mockReturnValueOnce(firstRequest).mockResolvedValueOnce({ items: [], - sources: { issues: null, prs: null, upstreamCandidate: null } + sources: { issues: null, prs: null, originCandidate: null, upstreamCandidate: null } }) const firstFetch = store.getState().fetchWorkItems('repo-1', '/repo/one', 20, '') @@ -182,13 +182,55 @@ describe('createGitHubSlice.evictGitHubRepoCaches', () => { const secondFetch = store.getState().fetchWorkItems('repo-1', '/repo/one', 20, '') resolveFirst({ items: [], - sources: { issues: null, prs: null, upstreamCandidate: null } + sources: { issues: null, prs: null, originCandidate: null, upstreamCandidate: null } }) await firstFetch await secondFetch expect(mockApi.gh.listWorkItems).toHaveBeenCalledTimes(2) }) + + it('does not let a stale pre-invalidation work-item response rewrite the cache', async () => { + const store = createTestStore() + const item = { + type: 'pr', + number: 42, + title: 'Old origin PR', + url: 'https://example.test/42', + updatedAt: '2026-05-22T00:00:00Z' + } as GitHubWorkItem + let resolveFirst: (value: { + items: GitHubWorkItem[] + sources: { + issues: null + prs: { owner: 'fork'; repo: 'r' } + originCandidate: { owner: 'fork'; repo: 'r' } + upstreamCandidate: { owner: 'up'; repo: 'r' } + } + }) => void = () => {} + mockApi.gh.listWorkItems.mockImplementationOnce( + () => + new Promise((resolve) => { + resolveFirst = resolve + }) + ) + + const firstFetch = store.getState().fetchWorkItems('repo-1', '/repo/one', 20, '') + await Promise.resolve() + store.setState((s) => ({ workItemsInvalidationNonce: s.workItemsInvalidationNonce + 1 })) + resolveFirst({ + items: [item], + sources: { + issues: null, + prs: { owner: 'fork', repo: 'r' }, + originCandidate: { owner: 'fork', repo: 'r' }, + upstreamCandidate: { owner: 'up', repo: 'r' } + } + }) + + await expect(firstFetch).resolves.toEqual([{ ...item, repoId: 'repo-1' }]) + expect(store.getState().workItemsCache[workItemsCacheKey('repo-1', 20, '')]).toBeUndefined() + }) }) describe('createGitHubSlice cache bounds', () => { @@ -3252,7 +3294,10 @@ describe('createGitHubSlice.fetchWorkItems source/error envelope', () => { runtimeEnvironmentCall.mockResolvedValue({ id: 'rpc-1', ok: true, - result: { items: [], sources: { issues: null, prs: null, upstreamCandidate: null } }, + result: { + items: [], + sources: { issues: null, prs: null, originCandidate: null, upstreamCandidate: null } + }, _meta: { runtimeId: 'remote-runtime' } }) }) @@ -3264,7 +3309,12 @@ describe('createGitHubSlice.fetchWorkItems source/error envelope', () => { const store = createTestStore() mockApi.gh.listWorkItems.mockResolvedValueOnce({ items: [], - sources: { issues: { owner: 'up', repo: 'r' }, prs: { owner: 'fork', repo: 'r' } } + sources: { + issues: { owner: 'up', repo: 'r' }, + prs: { owner: 'fork', repo: 'r' }, + originCandidate: { owner: 'fork', repo: 'r' }, + upstreamCandidate: { owner: 'up', repo: 'r' } + } }) await store.getState().fetchWorkItems('repo-id', '/repo', 24, '') @@ -3272,7 +3322,9 @@ describe('createGitHubSlice.fetchWorkItems source/error envelope', () => { const result = store.getState().getWorkItemsSourcesAndError('repo-id', 24, '') expect(result.sources).toEqual({ issues: { owner: 'up', repo: 'r' }, - prs: { owner: 'fork', repo: 'r' } + prs: { owner: 'fork', repo: 'r' }, + originCandidate: { owner: 'fork', repo: 'r' }, + upstreamCandidate: { owner: 'up', repo: 'r' } }) expect(result.error).toBeNull() }) @@ -3287,7 +3339,12 @@ describe('createGitHubSlice.fetchWorkItems source/error envelope', () => { const store = createTestStore() mockApi.gh.listWorkItems.mockResolvedValueOnce({ items: [], - sources: { issues: { owner: 'up', repo: 'r' }, prs: { owner: 'fork', repo: 'r' } }, + sources: { + issues: { owner: 'up', repo: 'r' }, + prs: { owner: 'fork', repo: 'r' }, + originCandidate: { owner: 'fork', repo: 'r' }, + upstreamCandidate: { owner: 'up', repo: 'r' } + }, errors: { issues: { type: 'permission_denied', message: 'no access' } } }) @@ -3313,7 +3370,12 @@ describe('createGitHubSlice.fetchWorkItems source/error envelope', () => { }) mockApi.gh.listWorkItems.mockReturnValueOnce(failingRequest).mockResolvedValueOnce({ items: [], - sources: { issues: { owner: 'up', repo: 'r' }, prs: { owner: 'fork', repo: 'r' } } + sources: { + issues: { owner: 'up', repo: 'r' }, + prs: { owner: 'fork', repo: 'r' }, + originCandidate: { owner: 'fork', repo: 'r' }, + upstreamCandidate: { owner: 'up', repo: 'r' } + } }) const initialFetch = store.getState().fetchWorkItems('repo-id', '/repo', 24, '') @@ -3322,7 +3384,12 @@ describe('createGitHubSlice.fetchWorkItems source/error envelope', () => { // Let the initial request settle with an error so the force path runs. resolveFailing({ items: [], - sources: { issues: { owner: 'up', repo: 'r' }, prs: { owner: 'fork', repo: 'r' } }, + sources: { + issues: { owner: 'up', repo: 'r' }, + prs: { owner: 'fork', repo: 'r' }, + originCandidate: { owner: 'fork', repo: 'r' }, + upstreamCandidate: { owner: 'up', repo: 'r' } + }, errors: { issues: { type: 'permission_denied', message: 'no access' } } }) await initialFetch.catch(() => {}) @@ -3338,15 +3405,15 @@ describe('createGitHubSlice.fetchWorkItems source/error envelope', () => { mockApi.gh.listWorkItems .mockResolvedValueOnce({ items: [], - sources: { issues: null, prs: null, upstreamCandidate: null } + sources: { issues: null, prs: null, originCandidate: null, upstreamCandidate: null } }) .mockResolvedValueOnce({ items: [], - sources: { issues: null, prs: null, upstreamCandidate: null } + sources: { issues: null, prs: null, originCandidate: null, upstreamCandidate: null } }) .mockResolvedValueOnce({ items: [], - sources: { issues: null, prs: null, upstreamCandidate: null } + sources: { issues: null, prs: null, originCandidate: null, upstreamCandidate: null } }) await store.getState().fetchWorkItems('repo-normal', '/repo/normal', 24, '') @@ -3381,7 +3448,7 @@ describe('createGitHubSlice.fetchWorkItems source/error envelope', () => { const store = createTestStore() type WorkItemsEnvelope = { items: [] - sources: { issues: null; prs: null; upstreamCandidate: null } + sources: { issues: null; prs: null; originCandidate: null; upstreamCandidate: null } } let resolveCacheable: (value: WorkItemsEnvelope) => void = () => {} const cacheableRequest = new Promise((resolve) => { @@ -3389,7 +3456,7 @@ describe('createGitHubSlice.fetchWorkItems source/error envelope', () => { }) mockApi.gh.listWorkItems.mockReturnValueOnce(cacheableRequest).mockResolvedValueOnce({ items: [], - sources: { issues: null, prs: null, upstreamCandidate: null } + sources: { issues: null, prs: null, originCandidate: null, upstreamCandidate: null } }) const landingProbe = store @@ -3403,7 +3470,7 @@ describe('createGitHubSlice.fetchWorkItems source/error envelope', () => { expect(mockApi.gh.listWorkItems).toHaveBeenCalledTimes(1) resolveCacheable({ items: [], - sources: { issues: null, prs: null, upstreamCandidate: null } + sources: { issues: null, prs: null, originCandidate: null, upstreamCandidate: null } }) await landingProbe await noCacheRefresh @@ -3424,7 +3491,12 @@ describe('createGitHubSlice.fetchWorkItems source/error envelope', () => { ok: true, result: { items: [{ type: 'issue', number: 7, title: 'Server issue', url: 'https://example.test/7' }], - sources: { issues: { owner: 'up', repo: 'r' }, prs: { owner: 'up', repo: 'r' } } + sources: { + issues: { owner: 'up', repo: 'r' }, + prs: { owner: 'up', repo: 'r' }, + originCandidate: { owner: 'up', repo: 'r' }, + upstreamCandidate: null + } }, _meta: { runtimeId: 'remote-runtime' } }) @@ -3561,7 +3633,12 @@ describe('createGitHubSlice.fetchWorkItems source/error envelope', () => { const store = createTestStore() mockApi.gh.listWorkItems.mockResolvedValueOnce({ items: [{ type: 'issue', number: 7, title: 'Local issue', url: 'https://example.test/7' }], - sources: { issues: { owner: 'up', repo: 'r' }, prs: { owner: 'up', repo: 'r' } } + sources: { + issues: { owner: 'up', repo: 'r' }, + prs: { owner: 'up', repo: 'r' }, + originCandidate: { owner: 'up', repo: 'r' }, + upstreamCandidate: null + } }) await store.getState().fetchWorkItems('repo-id', '/local/repo', 24, '') @@ -3606,7 +3683,7 @@ describe('createGitHubSlice.fetchWorkItems source/error envelope', () => { const store = createTestStore() type WorkItemsEnvelope = { items: GitHubWorkItem[] - sources: { issues: null; prs: null; upstreamCandidate: null } + sources: { issues: null; prs: null; originCandidate: null; upstreamCandidate: null } } const blockingResolvers: ((value: WorkItemsEnvelope) => void)[] = [] for (let i = 0; i < 8; i++) { @@ -3637,7 +3714,7 @@ describe('createGitHubSlice.fetchWorkItems source/error envelope', () => { ok: true, result: { items: [item], - sources: { issues: null, prs: null, upstreamCandidate: null } + sources: { issues: null, prs: null, originCandidate: null, upstreamCandidate: null } }, _meta: { runtimeId: 'remote-runtime' } }) @@ -3656,7 +3733,10 @@ describe('createGitHubSlice.fetchWorkItems source/error envelope', () => { workItemsCache: {} } as unknown as Partial) for (const resolve of blockingResolvers) { - resolve({ items: [], sources: { issues: null, prs: null, upstreamCandidate: null } }) + resolve({ + items: [], + sources: { issues: null, prs: null, originCandidate: null, upstreamCandidate: null } + }) } const result = await queued @@ -3682,7 +3762,7 @@ describe('createGitHubSlice.fetchWorkItems source/error envelope', () => { const store = createTestStore() type WorkItemsEnvelope = { items: GitHubWorkItem[] - sources: { issues: null; prs: null; upstreamCandidate: null } + sources: { issues: null; prs: null; originCandidate: null; upstreamCandidate: null } } type WorkItemsRpcResponse = { id: string @@ -3759,7 +3839,7 @@ describe('createGitHubSlice.fetchWorkItems source/error envelope', () => { ok: true, result: { items: [newRuntimeItem], - sources: { issues: null, prs: null, upstreamCandidate: null } + sources: { issues: null, prs: null, originCandidate: null, upstreamCandidate: null } }, _meta: { runtimeId: 'new-runtime' } }) @@ -3777,7 +3857,7 @@ describe('createGitHubSlice.fetchWorkItems source/error envelope', () => { ok: true, result: { items: [oldRuntimeItem], - sources: { issues: null, prs: null, upstreamCandidate: null } + sources: { issues: null, prs: null, originCandidate: null, upstreamCandidate: null } }, _meta: { runtimeId: 'old-runtime' } }) @@ -3796,7 +3876,7 @@ describe('createGitHubSlice.fetchWorkItems source/error envelope', () => { const store = createTestStore() mockApi.gh.listWorkItems.mockResolvedValue({ items: [], - sources: { issues: null, prs: null, upstreamCandidate: null } + sources: { issues: null, prs: null, originCandidate: null, upstreamCandidate: null } }) for (let i = 0; i <= 500; i++) { @@ -3829,7 +3909,12 @@ describe('createGitHubSlice.fetchWorkItems source/error envelope', () => { .mockRejectedValueOnce(new Error(GITHUB_WORK_ITEMS_SSH_REMOTE_REQUIRED_MESSAGE)) .mockResolvedValueOnce({ items: [item], - sources: { issues: { owner: 'up', repo: 'r' }, prs: { owner: 'up', repo: 'r' } } + sources: { + issues: { owner: 'up', repo: 'r' }, + prs: { owner: 'up', repo: 'r' }, + originCandidate: { owner: 'up', repo: 'r' }, + upstreamCandidate: null + } }) try { @@ -3868,7 +3953,12 @@ describe('createGitHubSlice.fetchWorkItems source/error envelope', () => { .mockRejectedValueOnce(new Error(GITHUB_WORK_ITEMS_SSH_REMOTE_REQUIRED_MESSAGE)) .mockResolvedValueOnce({ items: [item], - sources: { issues: { owner: 'up', repo: 'r' }, prs: { owner: 'up', repo: 'r' } } + sources: { + issues: { owner: 'up', repo: 'r' }, + prs: { owner: 'up', repo: 'r' }, + originCandidate: { owner: 'up', repo: 'r' }, + upstreamCandidate: null + } }) try { @@ -3904,7 +3994,12 @@ describe('createGitHubSlice.fetchWorkItems source/error envelope', () => { ok: true, result: { items: [item], - sources: { issues: null, prs: { owner: 'up', repo: 'r' }, upstreamCandidate: null } + sources: { + issues: null, + prs: { owner: 'up', repo: 'r' }, + originCandidate: { owner: 'up', repo: 'r' }, + upstreamCandidate: null + } }, _meta: { runtimeId: 'remote-runtime' } }) diff --git a/src/renderer/src/store/slices/github.ts b/src/renderer/src/store/slices/github.ts index 3e83b0c5e31..2a70b4a8534 100644 --- a/src/renderer/src/store/slices/github.ts +++ b/src/renderer/src/store/slices/github.ts @@ -431,6 +431,9 @@ function parseSlugAndNumber( export type WorkItemsCacheSources = { issues: GitHubOwnerRepo | null prs: GitHubOwnerRepo | null + /** Raw origin remote (if any). Required-nullable so selector code can + * distinguish the raw candidate from the effective PR source. */ + originCandidate: GitHubOwnerRepo | null /** Raw upstream remote (if any) — present so the selector can render * independently of the currently-effective preference. Required-nullable * (matches siblings `issues`/`prs`) so consumers only branch on `null` @@ -2030,6 +2033,7 @@ export const createGitHubSlice: StateCreator = (s return cached.data ?? [] } + const requestInvalidationNonce = requestState.workItemsInvalidationNonce const requestContext = getGitHubWorkItemRequestContext( requestState, requestSettings, @@ -2089,6 +2093,12 @@ export const createGitHubSlice: StateCreator = (s if ((currentHostId ?? null) !== (ownerHostId ?? null)) { return items } + // Why: clearing in-flight entries lets the next fetch start, but the + // old promise can still settle. Do not let pre-flip source data + // repopulate the cache after the invalidation nonce changes. + if (get().workItemsInvalidationNonce !== requestInvalidationNonce) { + return items + } set((s) => ({ workItemsCache: withBoundedCacheEntry(s.workItemsCache, key, { data: items, diff --git a/src/shared/types.ts b/src/shared/types.ts index ec2b2c3386b..a6212c4aa94 100644 --- a/src/shared/types.ts +++ b/src/shared/types.ts @@ -1637,6 +1637,11 @@ export type ListWorkItemsResult = { sources: { issues: GitHubOwnerRepo | null prs: GitHubOwnerRepo | null + /** Raw `origin` remote resolved for this repo, independent of the + * user's preference. Required-nullable so the renderer can compare raw + * remote candidates without inferring origin from the effective PR + * source. */ + originCandidate: GitHubOwnerRepo | null /** Raw `upstream` remote resolved for this repo, independent of the * user's preference. Present so the renderer's issue-source selector * can always decide whether to render (upstream exists & differs from