mirror of
https://github.com/stablyai/orca.git
synced 2026-10-15 16:02:49 +00:00
Route task PR queries by upstream source (#5176)
* Route task PR queries by upstream source Implements the routing described in docs/tasks-pr-upstream-source.md so task PR and issue queries stay scoped to the selected source. * rm design doc
This commit is contained in:
Jinjing
Jinwoo-H
@@ -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' }
|
||||
})
|
||||
})
|
||||
|
||||
+28
-11
@@ -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<ResolvedPrWorkItemSource> {
|
||||
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<ListWorkItemsResult<MainWorkItem>> {
|
||||
// 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<number> {
|
||||
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
|
||||
|
||||
@@ -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}
|
||||
<IssueSourceSelector
|
||||
preference={repo.issueSourcePreference}
|
||||
origin={s.sources.prs}
|
||||
origin={s.sources.originCandidate}
|
||||
upstream={s.sources.upstreamCandidate}
|
||||
onChange={(next) => {
|
||||
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 (
|
||||
<div className="mt-1">
|
||||
<IssueSourceSelector
|
||||
preference={newIssueTargetRepo.issueSourcePreference}
|
||||
origin={entry.sources.prs}
|
||||
origin={entry.sources.originCandidate}
|
||||
upstream={entry.sources.upstreamCandidate}
|
||||
disabled={newIssueSubmitting}
|
||||
// Why: the composer only files issues, so the "Issues from
|
||||
|
||||
@@ -165,7 +165,7 @@ describe('createGitHubSlice.evictGitHubRepoCaches', () => {
|
||||
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<WorkItemsEnvelope>((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<WorkItemsEnvelope>((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<AppState>)
|
||||
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' }
|
||||
})
|
||||
|
||||
@@ -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<AppState, [], [], GitHubSlice> = (s
|
||||
return cached.data ?? []
|
||||
}
|
||||
|
||||
const requestInvalidationNonce = requestState.workItemsInvalidationNonce
|
||||
const requestContext = getGitHubWorkItemRequestContext(
|
||||
requestState,
|
||||
requestSettings,
|
||||
@@ -2089,6 +2093,12 @@ export const createGitHubSlice: StateCreator<AppState, [], [], GitHubSlice> = (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,
|
||||
|
||||
@@ -1637,6 +1637,11 @@ export type ListWorkItemsResult<T> = {
|
||||
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
|
||||
|
||||
Reference in New Issue
Block a user