From 78d2b958bf075f5301b97a0210c5367c6a84cbf2 Mon Sep 17 00:00:00 2001 From: fsdwen Date: Thu, 16 Jul 2026 09:52:37 +0800 Subject: [PATCH] fix(issues): replace cursor-based pagination with page-number Search API (#8680) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(issues): replace cursor-based pagination with page-number Search API Problem ======= Issue pagination (#8649) had two bugs: 1. Pages 6-16 were unreachable — clicking page 16 highlighted page 5; clicking 6/7 did nothing. The old cursor-based approach (updated: --- src/main/github/client-issue-source.test.ts | 175 ++++------- src/main/github/client-work-items.test.ts | 200 +++++++----- src/main/github/client.ts | 296 ++++++++---------- src/main/ipc/github.test.ts | 8 +- src/main/ipc/github.ts | 4 +- src/main/runtime/orca-runtime.test.ts | 4 +- src/main/runtime/orca-runtime.ts | 10 +- src/main/runtime/rpc/methods/github.ts | 4 +- src/preload/api-types.ts | 2 +- src/preload/index.ts | 2 +- src/renderer/src/components/TaskPage.tsx | 157 +++++----- .../task-page-cache-selectors.test.ts | 4 +- .../components/task-page-cache-selectors.ts | 16 +- .../task-page-work-item-pagination.test.ts | 22 +- .../task-page-work-item-pagination.ts | 15 + src/renderer/src/store/slices/github.test.ts | 40 ++- src/renderer/src/store/slices/github.ts | 44 +-- src/renderer/src/web/web-preload-api.test.ts | 4 +- src/shared/work-items.ts | 7 + 19 files changed, 529 insertions(+), 485 deletions(-) diff --git a/src/main/github/client-issue-source.test.ts b/src/main/github/client-issue-source.test.ts index 531234322..df658007f 100644 --- a/src/main/github/client-issue-source.test.ts +++ b/src/main/github/client-issue-source.test.ts @@ -49,6 +49,47 @@ vi.mock('./rate-limit', () => ({ import { countWorkItems, getWorkItem, listWorkItems, _resetOwnerRepoCache } from './client' +const PR_LIST_FIELDS = + 'number,title,state,url,labels,updatedAt,author,isDraft,headRefName,baseRefName,headRefOid,headRepositoryOwner,reviewRequests' + +function issueSearchArgs( + ownerRepo: string, + options: { noCache?: boolean; query?: string } = {} +): string[] { + const query = options.query ?? 'is:issue is:open' + return [ + 'api', + ...(options.noCache ? [] : ['--cache', '120s']), + `search/issues?q=${encodeURIComponent(`repo:${ownerRepo} ${query}`)}&sort=created&order=desc&per_page=10&page=1`, + '--jq', + '.items' + ] +} + +function prListArgs(ownerRepo: string, query = 'is:pr is:open'): string[] { + return [ + 'pr', + 'list', + '--limit', + '10', + '--state', + 'all', + '--json', + PR_LIST_FIELDS, + '--repo', + ownerRepo, + '--search', + `${query} sort:created-desc` + ] +} + +function decodedIssueSearchPath(callIndex: number): string { + const args = ghExecFileAsyncMock.mock.calls[callIndex]?.[0] as string[] | undefined + const apiPath = args?.find((arg) => arg.startsWith('search/issues?')) + expect(apiPath).toBeDefined() + return decodeURIComponent(apiPath ?? '') +} + describe('GitHub issue source split', () => { beforeEach(() => { execFileAsyncMock.mockReset() @@ -115,26 +156,12 @@ describe('GitHub issue source split', () => { await listWorkItems('/repo-root', 10) - expect(ghExecFileAsyncMock).toHaveBeenNthCalledWith( - 1, - [ - 'api', - '--cache', - '120s', - 'repos/stablyai/orca/issues?per_page=10&state=open&sort=updated&direction=desc' - ], - { cwd: '/repo-root' } - ) - expect(ghExecFileAsyncMock).toHaveBeenNthCalledWith( - 2, - [ - 'api', - '--cache', - '120s', - 'repos/fork/orca/pulls?per_page=10&state=open&sort=updated&direction=desc' - ], - { cwd: '/repo-root' } - ) + expect(ghExecFileAsyncMock).toHaveBeenNthCalledWith(1, issueSearchArgs('stablyai/orca'), { + cwd: '/repo-root' + }) + expect(ghExecFileAsyncMock).toHaveBeenNthCalledWith(2, prListArgs('fork/orca'), { + cwd: '/repo-root' + }) }) it('omits gh api cache args for no-cache recent work-item requests', async () => { @@ -148,14 +175,12 @@ describe('GitHub issue source split', () => { expect(ghExecFileAsyncMock).toHaveBeenNthCalledWith( 1, - ['api', 'repos/stablyai/orca/issues?per_page=10&state=open&sort=updated&direction=desc'], - { cwd: '/repo-root' } - ) - expect(ghExecFileAsyncMock).toHaveBeenNthCalledWith( - 2, - ['api', 'repos/fork/orca/pulls?per_page=10&state=open&sort=updated&direction=desc'], + issueSearchArgs('stablyai/orca', { noCache: true }), { cwd: '/repo-root' } ) + expect(ghExecFileAsyncMock).toHaveBeenNthCalledWith(2, prListArgs('fork/orca'), { + cwd: '/repo-root' + }) }) it('lists SSH repo work items with explicit owner/repo and no local cwd', async () => { @@ -177,26 +202,8 @@ describe('GitHub issue source split', () => { {} ) expect(getOwnerRepoMock).toHaveBeenCalledWith('/home/jinwoo/orca', 'openclaw-2', {}) - expect(ghExecFileAsyncMock).toHaveBeenNthCalledWith( - 1, - [ - 'api', - '--cache', - '120s', - 'repos/stablyai/orca/issues?per_page=10&state=open&sort=updated&direction=desc' - ], - {} - ) - expect(ghExecFileAsyncMock).toHaveBeenNthCalledWith( - 2, - [ - 'api', - '--cache', - '120s', - 'repos/fork/orca/pulls?per_page=10&state=open&sort=updated&direction=desc' - ], - {} - ) + expect(ghExecFileAsyncMock).toHaveBeenNthCalledWith(1, issueSearchArgs('stablyai/orca'), {}) + expect(ghExecFileAsyncMock).toHaveBeenNthCalledWith(2, prListArgs('fork/orca'), {}) }) it('uses upstream for issue-only queries and origin for PR-only queries', async () => { @@ -206,10 +213,7 @@ describe('GitHub issue source split', () => { await listWorkItems('/repo-root', 10, 'is:issue') - expect(ghExecFileAsyncMock).toHaveBeenCalledWith( - expect.arrayContaining(['--repo', 'stablyai/orca']), - { cwd: '/repo-root' } - ) + expect(decodedIssueSearchPath(0)).toContain('q=repo:stablyai/orca is:issue') ghExecFileAsyncMock.mockClear() getIssueOwnerRepoMock.mockResolvedValueOnce({ owner: 'stablyai', repo: 'orca' }) @@ -237,16 +241,9 @@ describe('GitHub issue source split', () => { 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' } - ) + expect(ghExecFileAsyncMock).toHaveBeenNthCalledWith(2, prListArgs('stablyai/orca'), { + cwd: '/repo-root' + }) }) it("uses upstream for queried PRs when preference='upstream'", async () => { @@ -515,16 +512,9 @@ describe('GitHub issue source split', () => { const result = await listWorkItems('/repo-root', 10, undefined, undefined, 'auto') expect(resolveIssueSourceMock).toHaveBeenCalledWith('/repo-root', 'auto', undefined, {}) - expect(ghExecFileAsyncMock).toHaveBeenNthCalledWith( - 1, - [ - 'api', - '--cache', - '120s', - 'repos/stablyai/orca/issues?per_page=10&state=open&sort=updated&direction=desc' - ], - { cwd: '/repo-root' } - ) + expect(ghExecFileAsyncMock).toHaveBeenNthCalledWith(1, issueSearchArgs('stablyai/orca'), { + cwd: '/repo-root' + }) expect(result.issueSourceFellBack).toBeUndefined() }) @@ -540,16 +530,9 @@ describe('GitHub issue source split', () => { await listWorkItems('/repo-root', 10, undefined, undefined, 'auto') - expect(ghExecFileAsyncMock).toHaveBeenNthCalledWith( - 1, - [ - 'api', - '--cache', - '120s', - 'repos/solo/orca/issues?per_page=10&state=open&sort=updated&direction=desc' - ], - { cwd: '/repo-root' } - ) + expect(ghExecFileAsyncMock).toHaveBeenNthCalledWith(1, issueSearchArgs('solo/orca'), { + cwd: '/repo-root' + }) }) it("preference='upstream' + upstream exists → queries upstream", async () => { @@ -564,13 +547,7 @@ describe('GitHub issue source split', () => { const result = await listWorkItems('/repo-root', 10, undefined, undefined, 'upstream') - expect(ghExecFileAsyncMock).toHaveBeenNthCalledWith( - 1, - expect.arrayContaining([ - 'repos/stablyai/orca/issues?per_page=10&state=open&sort=updated&direction=desc' - ]), - { cwd: '/repo-root' } - ) + expect(decodedIssueSearchPath(0)).toContain('q=repo:stablyai/orca is:issue is:open') expect(result.issueSourceFellBack).toBeUndefined() }) @@ -586,13 +563,7 @@ describe('GitHub issue source split', () => { const result = await listWorkItems('/repo-root', 10, undefined, undefined, 'upstream') - expect(ghExecFileAsyncMock).toHaveBeenNthCalledWith( - 1, - expect.arrayContaining([ - 'repos/solo/orca/issues?per_page=10&state=open&sort=updated&direction=desc' - ]), - { cwd: '/repo-root' } - ) + expect(decodedIssueSearchPath(0)).toContain('q=repo:solo/orca is:issue is:open') expect(result.issueSourceFellBack).toBe(true) }) @@ -608,13 +579,7 @@ describe('GitHub issue source split', () => { await listWorkItems('/repo-root', 10, undefined, undefined, 'origin') - expect(ghExecFileAsyncMock).toHaveBeenNthCalledWith( - 1, - expect.arrayContaining([ - 'repos/fork/orca/issues?per_page=10&state=open&sort=updated&direction=desc' - ]), - { cwd: '/repo-root' } - ) + expect(decodedIssueSearchPath(0)).toContain('q=repo:fork/orca is:issue is:open') }) it("preference='origin' + no upstream → queries origin", async () => { @@ -629,13 +594,7 @@ describe('GitHub issue source split', () => { await listWorkItems('/repo-root', 10, undefined, undefined, 'origin') - expect(ghExecFileAsyncMock).toHaveBeenNthCalledWith( - 1, - expect.arrayContaining([ - 'repos/solo/orca/issues?per_page=10&state=open&sort=updated&direction=desc' - ]), - { cwd: '/repo-root' } - ) + expect(decodedIssueSearchPath(0)).toContain('q=repo:solo/orca is:issue is:open') }) it('surfaces upstreamCandidate in sources regardless of effective preference', async () => { diff --git a/src/main/github/client-work-items.test.ts b/src/main/github/client-work-items.test.ts index d0e93eb30..a30db9f19 100644 --- a/src/main/github/client-work-items.test.ts +++ b/src/main/github/client-work-items.test.ts @@ -180,18 +180,12 @@ describe('listWorkItems', () => { expect(ghExecFileAsyncMock).toHaveBeenNthCalledWith( 1, [ - 'issue', - 'list', - '--limit', - '10', - '--json', - 'number,title,state,url,labels,updatedAt,author,assignees', - '--repo', - 'acme/widgets', - '--assignee', - '@me', - '--search', - 'sort:updated-desc' + 'api', + '--cache', + '120s', + `search/issues?q=${encodeURIComponent('repo:acme/widgets is:issue assignee:@me')}&sort=created&order=desc&per_page=10&page=1`, + '--jq', + '.items' ], { cwd: '/repo-root' } ) @@ -202,14 +196,14 @@ describe('listWorkItems', () => { 'list', '--limit', '10', + '--state', + 'all', '--json', 'number,title,state,url,labels,updatedAt,author,isDraft,headRefName,baseRefName,headRefOid,headRepositoryOwner,reviewRequests', '--repo', 'acme/widgets', - '--assignee', - '@me', '--search', - 'sort:updated-desc' + 'is:pr assignee:@me sort:created-desc' ], { cwd: '/repo-root' } ) @@ -218,24 +212,6 @@ describe('listWorkItems', () => { expect(prListFields).toContain('reviewRequests') expect(prListFields).not.toContain('mergeStateStatus') expect(items).toEqual([ - { - id: 'issue:12', - type: 'issue', - number: 12, - title: 'Fix bug', - state: 'open', - url: 'https://github.com/acme/widgets/issues/12', - labels: [], - updatedAt: '2026-03-29T00:00:00Z', - author: 'octocat', - assignees: [ - { - login: 'test-assignee', - name: 'Test Assignee', - avatarUrl: 'https://avatars.githubusercontent.com/u/1?v=4' - } - ] - }, { id: 'pr:42', type: 'pr', @@ -257,6 +233,24 @@ describe('listWorkItems', () => { avatarUrl: 'https://avatars.githubusercontent.com/u/1?v=4' } ] + }, + { + id: 'issue:12', + type: 'issue', + number: 12, + title: 'Fix bug', + state: 'open', + url: 'https://github.com/acme/widgets/issues/12', + labels: [], + updatedAt: '2026-03-29T00:00:00Z', + author: 'octocat', + assignees: [ + { + login: 'test-assignee', + name: 'Test Assignee', + avatarUrl: 'https://avatars.githubusercontent.com/u/1?v=4' + } + ] } ]) }) @@ -404,15 +398,14 @@ describe('listWorkItems', () => { 'list', '--limit', '10', + '--state', + 'all', '--json', 'number,title,state,url,labels,updatedAt,author,isDraft,headRefName,baseRefName,headRefOid,headRepositoryOwner,reviewRequests', '--repo', 'acme/widgets', - '--state', - 'open', - '--draft', '--search', - 'sort:updated-desc' + 'is:pr is:open draft:true sort:created-desc' ], { cwd: '/repo-root' } ) @@ -465,14 +458,14 @@ describe('listWorkItems', () => { 'list', '--limit', '10', + '--state', + 'all', '--json', 'number,title,state,url,labels,updatedAt,author,isDraft,headRefName,baseRefName,headRefOid,headRepositoryOwner,reviewRequests', '--repo', 'acme/widgets', - '--state', - 'merged', '--search', - 'sort:updated-desc' + 'is:pr is:merged sort:created-desc' ], { cwd: '/repo-root' } ) @@ -536,7 +529,12 @@ describe('listWorkItems', () => { const { items } = await listWorkItems('/repo-root', 10, 'is:pr is:closed') expect(ghExecFileAsyncMock).toHaveBeenCalledWith( - expect.arrayContaining(['--state', 'closed', '--search', '-is:merged sort:updated-desc']), + expect.arrayContaining([ + '--state', + 'all', + '--search', + 'is:pr is:closed -is:merged sort:created-desc' + ]), { cwd: '/repo-root' } ) expect(items).toMatchObject([{ id: 'pr:9', type: 'pr', state: 'closed' }]) @@ -605,7 +603,7 @@ describe('listWorkItems', () => { expect(ghExecFileAsyncMock).toHaveBeenCalledTimes(1) expect(ghExecFileAsyncMock).toHaveBeenCalledWith( - expect.arrayContaining(['--search', 'review-requested:@me sort:updated-desc']), + expect.arrayContaining(['--search', 'is:pr is:open review-requested:@me sort:created-desc']), { cwd: '/repo-root' } ) expect(ghExecFileAsyncMock).not.toHaveBeenCalledWith( @@ -614,32 +612,86 @@ describe('listWorkItems', () => { ) }) - it('pins list ordering to updated-desc so the updatedAt cursor pages consistently', async () => { + it('uses the requested numbered Search API page for issues', async () => { getIssueOwnerRepoMock.mockResolvedValueOnce({ owner: 'acme', repo: 'widgets' }) getOwnerRepoMock.mockResolvedValueOnce({ owner: 'acme', repo: 'widgets' }) ghExecFileAsyncMock.mockResolvedValueOnce({ stdout: '[]' }) - await listWorkItems('/repo-root', 10, 'is:issue is:open') + await listWorkItems('/repo-root', 10, 'is:issue is:open', 2) expect(ghExecFileAsyncMock).toHaveBeenCalledWith( - expect.arrayContaining(['--search', 'sort:updated-desc']), + [ + 'api', + '--cache', + '120s', + `search/issues?q=${encodeURIComponent('repo:acme/widgets is:issue is:open')}&sort=created&order=desc&per_page=10&page=2`, + '--jq', + '.items' + ], { cwd: '/repo-root' } ) }) - it('combines the inclusive updatedAt cursor with updated-desc ordering on later pages', async () => { + it('fetches and slices stable PR results for the requested numbered page', async () => { getIssueOwnerRepoMock.mockResolvedValueOnce({ owner: 'acme', repo: 'widgets' }) getOwnerRepoMock.mockResolvedValueOnce({ owner: 'acme', repo: 'widgets' }) - ghExecFileAsyncMock.mockResolvedValueOnce({ stdout: '[]' }) + ghExecFileAsyncMock.mockResolvedValueOnce({ + stdout: JSON.stringify( + [1, 2, 3, 4].map((number) => ({ + number, + title: `PR ${number}`, + state: 'OPEN', + url: `https://github.com/acme/widgets/pull/${number}`, + labels: [], + updatedAt: `2026-07-0${number}T00:00:00Z`, + author: { login: 'octocat' }, + isDraft: false, + headRefName: `feature/${number}`, + headRefOid: `head-${number}`, + baseRefName: 'main' + })) + ) + }) - await listWorkItems('/repo-root', 10, 'is:issue is:open', '2026-07-01T00:00:00Z') + const { items } = await listWorkItems('/repo-root', 2, 'is:pr is:open', 2) - // Why: the bound is inclusive (`<=`) so boundary items sharing the cursor's - // exact updatedAt aren't skipped; the renderer dedupes the re-fetched rows. expect(ghExecFileAsyncMock).toHaveBeenCalledWith( - expect.arrayContaining(['--search', 'updated:<=2026-07-01T00:00:00Z sort:updated-desc']), + expect.arrayContaining(['--limit', '4', '--search', 'is:pr is:open sort:created-desc']), { cwd: '/repo-root' } ) + expect(items.map((item) => item.number)).toEqual([4, 3]) + }) + + it('filters pull request rows out of issue Search API results', async () => { + getIssueOwnerRepoMock.mockResolvedValueOnce({ owner: 'acme', repo: 'widgets' }) + getOwnerRepoMock.mockResolvedValueOnce({ owner: 'acme', repo: 'widgets' }) + ghExecFileAsyncMock.mockResolvedValueOnce({ + stdout: JSON.stringify([ + { + number: 2, + title: 'PR-shaped search row', + state: 'open', + html_url: 'https://github.com/acme/widgets/pull/2', + labels: [], + updated_at: '2026-07-02T00:00:00Z', + user: { login: 'octocat' }, + pull_request: { url: 'https://api.github.com/repos/acme/widgets/pulls/2' } + }, + { + number: 1, + title: 'Issue row', + state: 'open', + html_url: 'https://github.com/acme/widgets/issues/1', + labels: [], + updated_at: '2026-07-01T00:00:00Z', + user: { login: 'octocat' } + } + ]) + }) + + const { items } = await listWorkItems('/repo-root', 10, 'is:issue is:open') + + expect(items.map((item) => item.id)).toEqual(['issue:1']) }) it('returns open issues and PRs for the all-open preset query', async () => { @@ -679,18 +731,12 @@ describe('listWorkItems', () => { const { items } = await listWorkItems('/repo-root', 10, 'is:open') expect(ghExecFileAsyncMock).toHaveBeenCalledWith( [ - 'issue', - 'list', - '--limit', - '10', - '--json', - 'number,title,state,url,labels,updatedAt,author,assignees', - '--repo', - 'acme/widgets', - '--state', - 'open', - '--search', - 'sort:updated-desc' + 'api', + '--cache', + '120s', + `search/issues?q=${encodeURIComponent('repo:acme/widgets is:issue is:open')}&sort=created&order=desc&per_page=10&page=1`, + '--jq', + '.items' ], { cwd: '/repo-root' } ) @@ -700,29 +746,18 @@ describe('listWorkItems', () => { 'list', '--limit', '10', + '--state', + 'all', '--json', 'number,title,state,url,labels,updatedAt,author,isDraft,headRefName,baseRefName,headRefOid,headRepositoryOwner,reviewRequests', '--repo', 'acme/widgets', - '--state', - 'open', '--search', - 'sort:updated-desc' + 'is:pr is:open sort:created-desc' ], { cwd: '/repo-root' } ) expect(items).toEqual([ - { - id: 'issue:1', - type: 'issue', - number: 1, - title: 'Open issue', - state: 'open', - url: 'https://github.com/acme/widgets/issues/1', - labels: [], - updatedAt: '2026-03-31T00:00:00Z', - author: 'octocat' - }, { id: 'pr:2', type: 'pr', @@ -737,6 +772,17 @@ describe('listWorkItems', () => { baseRefName: 'main', headSha: 'head-2', prRepo: { owner: 'acme', repo: 'widgets' } + }, + { + id: 'issue:1', + type: 'issue', + number: 1, + title: 'Open issue', + state: 'open', + url: 'https://github.com/acme/widgets/issues/1', + labels: [], + updatedAt: '2026-03-31T00:00:00Z', + author: 'octocat' } ]) }) diff --git a/src/main/github/client.ts b/src/main/github/client.ts index 15a2f8b34..54572bd40 100644 --- a/src/main/github/client.ts +++ b/src/main/github/client.ts @@ -32,7 +32,7 @@ import { isGitHubWorkItemsQueryTooLarge } from '../../shared/github-work-items-q import { parseTaskQuery, type ParsedTaskQuery } from '../../shared/task-query' import { GITHUB_WORK_ITEMS_SSH_REMOTE_REQUIRED_MESSAGE, - sortWorkItemsByUpdatedAt + sortWorkItemsByNumber } from '../../shared/work-items' import { mkdtemp, readFile, rm, writeFile } from 'node:fs/promises' import { join } from 'node:path' @@ -449,17 +449,13 @@ export async function getAuthenticatedViewer(): Promise { } // Why: main-process maps omit repoId because the IPC handler never receives -// a repo identifier beyond path. The renderer stamps repoId after IPC so -// single-repo and cross-repo items are uniform downstream. -type MainWorkItem = Omit +// a repo identifier beyond path. Exported because runtime consumers receive +// listWorkItems results before the renderer stamps repoId. +export type MainWorkItem = Omit -const WORK_ITEM_ISSUE_LIST_JSON_FIELDS = 'number,title,state,url,labels,updatedAt,author,assignees' - -// Why: the Tasks pager slices pages on updatedAt, so every list/search fetch -// must return rows newest-updated-first for the cursor to advance correctly. -// This is the search-qualifier spelling of the same contract the REST list -// paths express as `sort=updated&direction=desc`; keep the two in sync (#8649). -const WORK_ITEM_LIST_SORT_QUALIFIER = 'sort:updated-desc' +// Why: issue numbers follow creation order within a repository. Pinning this +// sort keeps gh's rich PR rows aligned with numbered Search API issue pages. +const WORK_ITEM_NUMBER_SORT_QUALIFIER = 'sort:created-desc' const WORK_ITEM_PR_LIST_JSON_FIELDS = 'number,title,state,url,labels,updatedAt,author,isDraft,headRefName,baseRefName,headRefOid,headRepositoryOwner,reviewRequests' @@ -956,78 +952,98 @@ async function fetchPullRequestWorkItem( return mapPullRequestWorkItem(JSON.parse(stdout) as Record) } -function buildWorkItemListArgs(args: { +type WorkItemListRequest = { + args: string[] + offset: number +} + +function normalizeWorkItemPage(page: number | undefined): number { + return typeof page === 'number' && Number.isFinite(page) && page >= 1 ? Math.floor(page) : 1 +} + +function buildWorkItemListRequest(args: { kind: 'issue' | 'pr' ownerRepo: OwnerRepo | null limit: number query: ParsedTaskQuery - before?: string -}): string[] { - const { kind, ownerRepo, limit, query, before } = args - const fields = kind === 'issue' ? WORK_ITEM_ISSUE_LIST_JSON_FIELDS : WORK_ITEM_PR_LIST_JSON_FIELDS - const command = kind === 'issue' ? ['issue', 'list'] : ['pr', 'list'] - const out = [...command, '--limit', String(limit), '--json', fields] + page: number +}): WorkItemListRequest { + const { kind, ownerRepo, limit, query, page } = args + const searchParts: string[] = [] - if (ownerRepo) { - out.push('--repo', `${ownerRepo.owner}/${ownerRepo.repo}`) + if (kind === 'issue' && ownerRepo) { + searchParts.push(`repo:${ownerRepo.owner}/${ownerRepo.repo}`) + } + searchParts.push(kind === 'issue' ? 'is:issue' : 'is:pr') + + if (query.state === 'open') { + searchParts.push('is:open') + } else if (query.state === 'closed') { + searchParts.push('is:closed') + if (kind === 'pr') { + searchParts.push('-is:merged') + } + } else if (query.state === 'merged') { + searchParts.push('is:merged') } - if (query.state) { - out.push('--state', query.state) + if (kind === 'pr' && query.draft) { + searchParts.push('draft:true') } - // Why: GitHub considers merged PRs as "closed". When the user filters for - // closed-only, exclude merged PRs via the search predicate so the displayed - // and counted results match the user's intent. - const excludeMergedFromClosed = kind === 'pr' && query.state === 'closed' if (query.assignee) { - out.push('--assignee', query.assignee) + searchParts.push(`assignee:${quoteGitHubSearchValue(query.assignee)}`) } if (query.author) { - out.push('--author', query.author) + searchParts.push(`author:${quoteGitHubSearchValue(query.author)}`) } if (query.labels.length > 0) { for (const label of query.labels) { - out.push('--label', label) + searchParts.push(`label:${quoteGitHubSearchValue(label)}`) } } - // Why: only add --draft when the user explicitly typed `is:draft`. Previously - // this fired for any PR-scoped open query, which made `is:pr is:open` (the - // "PRs" preset) silently filter to drafts-only. - if (kind === 'pr' && query.draft) { - out.push('--draft') - } - - const searchParts: string[] = [] - if (excludeMergedFromClosed) { - searchParts.push('-is:merged') - } - // Why: cursor-based pagination. GitHub search supports updated:<=DATE to - // fetch items at or older than the cursor. We use the oldest item's updatedAt - // from the previous page as the cursor. The bound is inclusive (`<=`) so items - // sharing the boundary row's exact updatedAt aren't skipped between pages; the - // renderer dedupes the re-fetched boundary rows by id (#8649). - if (before) { - searchParts.push(`updated:<=${before}`) - } if (kind === 'pr' && query.reviewRequested) { - searchParts.push(`review-requested:${query.reviewRequested}`) + searchParts.push(`review-requested:${quoteGitHubSearchValue(query.reviewRequested)}`) } if (kind === 'pr' && query.reviewedBy) { - searchParts.push(`reviewed-by:${query.reviewedBy}`) + searchParts.push(`reviewed-by:${quoteGitHubSearchValue(query.reviewedBy)}`) } if (query.freeText) { searchParts.push(query.freeText) } - // Why: pagination cursors slice on updatedAt, but `gh issue list` defaults - // to created-desc and `--search` defaults to best-match. `gh issue/pr list` - // has no --sort flag, so the only lever is the search query — which forces us - // to always emit --search. Without pinning the sort, recently-updated old - // items never appear on any page, so the pager advertises pages the fetch - // chain can never reach (#8649). - searchParts.push(WORK_ITEM_LIST_SORT_QUALIFIER) + + if (kind === 'issue') { + return { + args: [ + 'api', + '--cache', + '120s', + `search/issues?q=${encodeURIComponent(searchParts.join(' '))}&sort=created&order=desc&per_page=${limit}&page=${page}`, + '--jq', + '.items' + ], + offset: 0 + } + } + + // Why: search/issues omits the PR fields used by the Tasks management + // columns. Fetch through gh's rich PR list and slice its stable created sort. + searchParts.push(WORK_ITEM_NUMBER_SORT_QUALIFIER) + const out = [ + 'pr', + 'list', + '--limit', + String(Math.min(page * limit, 1000)), + '--state', + 'all', + '--json', + WORK_ITEM_PR_LIST_JSON_FIELDS + ] + if (ownerRepo) { + out.push('--repo', `${ownerRepo.owner}/${ownerRepo.repo}`) + } out.push('--search', searchParts.join(' ')) - return out + return { args: out, offset: (page - 1) * limit } } // Why: internal shape shared by listRecentWorkItems / listQueriedWorkItems so @@ -1105,6 +1121,7 @@ async function listRecentWorkItems( issueOwnerRepo: OwnerRepo | null, prOwnerRepo: OwnerRepo | null, limit: number, + page: number, connectionId?: string | null, noCache?: boolean, localGitOptions: LocalGitExecOptions = {} @@ -1112,72 +1129,49 @@ async function listRecentWorkItems( const repoContext = githubRepoContext(repoPath, connectionId, localGitOptions) const ghOptions = ghRepoExecOptions(repoContext) const requiresExplicitRepo = Boolean(connectionId) - const restCacheArgs = noCache ? [] : ['--cache', '120s'] assertSshRepoHasResolvedGitHubSource({ connectionId, issueOwnerRepo, prOwnerRepo }) + const recentQuery = parseTaskQuery('is:open') + const issueRequest = buildWorkItemListRequest({ + kind: 'issue', + ownerRepo: issueOwnerRepo, + limit, + query: recentQuery, + page + }) + const prRequest = buildWorkItemListRequest({ + kind: 'pr', + ownerRepo: prOwnerRepo, + limit, + query: recentQuery, + page + }) + if (noCache) { + issueRequest.args.splice(1, 2) + } if (issueOwnerRepo || prOwnerRepo || requiresExplicitRepo) { // Why: allSettled so a 403 on upstream issues doesn't zero out the origin // PR half — the UI renders partial results plus a banner for the failing // side, matching the parent design doc's partial-failure rule (§2). const [issuesSettled, prsSettled] = await Promise.allSettled([ issueOwnerRepo - ? ghExecFileAsync( - [ - 'api', - ...restCacheArgs, - `repos/${issueOwnerRepo.owner}/${issueOwnerRepo.repo}/issues?per_page=${limit}&state=open&sort=updated&direction=desc` - ], - ghOptions - ) + ? ghExecFileAsync(issueRequest.args, ghOptions) : requiresExplicitRepo ? Promise.resolve({ stdout: '[]' }) - : ghCwdResolvedExec( - repoContext, - [ - 'issue', - 'list', - '--limit', - String(limit), - '--state', - 'open', - '--json', - WORK_ITEM_ISSUE_LIST_JSON_FIELDS - ], - ghOptions - ), + : ghCwdResolvedExec(repoContext, issueRequest.args, ghOptions), prOwnerRepo - ? ghExecFileAsync( - [ - 'api', - ...restCacheArgs, - `repos/${prOwnerRepo.owner}/${prOwnerRepo.repo}/pulls?per_page=${limit}&state=open&sort=updated&direction=desc` - ], - ghOptions - ) + ? ghExecFileAsync(prRequest.args, ghOptions) : requiresExplicitRepo ? Promise.resolve({ stdout: '[]' }) - : ghCwdResolvedExec( - repoContext, - [ - 'pr', - 'list', - '--limit', - String(limit), - '--state', - 'open', - '--json', - WORK_ITEM_PR_LIST_JSON_FIELDS - ], - ghOptions - ) + : ghCwdResolvedExec(repoContext, prRequest.args, ghOptions) ]) let issues: MainWorkItem[] = [] let issuesError: ClassifiedError | undefined if (issuesSettled.status === 'fulfilled') { issues = (JSON.parse(issuesSettled.value.stdout) as Record[]) - // Why: the GitHub issues REST endpoint also returns pull requests with a - // `pull_request` marker. The new-workspace task picker needs distinct - // issue vs PR buckets, so drop PR-shaped issue rows here before merging. + // Why: the GitHub search/issues endpoint may still return PRs with a + // pull_request marker even when queried with is:issue. Filter them out + // here to keep the issue and PR buckets clean. .filter((item) => !('pull_request' in item)) .map(mapIssueWorkItem) } else { @@ -1190,9 +1184,9 @@ async function listRecentWorkItems( let prs: MainWorkItem[] = [] if (prsSettled.status === 'fulfilled') { - prs = (JSON.parse(prsSettled.value.stdout) as Record[]).map((item) => - mapPullRequestWorkItem(item, prOwnerRepo) - ) + prs = (JSON.parse(prsSettled.value.stdout) as Record[]) + .slice(prRequest.offset, prRequest.offset + limit) + .map((item) => mapPullRequestWorkItem(item, prOwnerRepo)) prs = await hydrateWorkItemRepositoryMergeMetadata(prs, prOwnerRepo, ghOptions) } else { // Why: PR-side failures must preserve the pre-diff behavior of @@ -1215,7 +1209,7 @@ async function listRecentWorkItems( } return { - items: sortWorkItemsByUpdatedAt([...issues, ...prs]).slice(0, limit), + items: sortWorkItemsByNumber([...issues, ...prs]).slice(0, limit), issuesError } } @@ -1228,45 +1222,19 @@ async function listRecentWorkItems( // effectively unusable for the feature — reject-all matches reality. If // non-GitHub remotes ever grow source metadata, revisit this symmetry. const [issuesResult, prsResult] = await Promise.all([ - ghCwdResolvedExec( - repoContext, - [ - 'issue', - 'list', - '--limit', - String(limit), - '--state', - 'open', - '--json', - WORK_ITEM_ISSUE_LIST_JSON_FIELDS - ], - ghOptions - ), - ghCwdResolvedExec( - repoContext, - [ - 'pr', - 'list', - '--limit', - String(limit), - '--state', - 'open', - '--json', - WORK_ITEM_PR_LIST_JSON_FIELDS - ], - ghOptions - ) + ghCwdResolvedExec(repoContext, issueRequest.args, ghOptions), + ghCwdResolvedExec(repoContext, prRequest.args, ghOptions) ]) - const issues = (JSON.parse(issuesResult.stdout) as Record[]).map( - mapIssueWorkItem - ) - const prs = (JSON.parse(prsResult.stdout) as Record[]).map((item) => - mapPullRequestWorkItem(item, null) - ) + const issues = (JSON.parse(issuesResult.stdout) as Record[]) + .filter((item) => !('pull_request' in item)) + .map(mapIssueWorkItem) + const prs = (JSON.parse(prsResult.stdout) as Record[]) + .slice(prRequest.offset, prRequest.offset + limit) + .map((item) => mapPullRequestWorkItem(item, null)) return { - items: sortWorkItemsByUpdatedAt([...issues, ...prs]).slice(0, limit) + items: sortWorkItemsByNumber([...issues, ...prs]).slice(0, limit) } } @@ -1276,7 +1244,7 @@ async function listQueriedWorkItems( prOwnerRepo: OwnerRepo | null, query: ParsedTaskQuery, limit: number, - before?: string, + page?: number, connectionId?: string | null, localGitOptions: LocalGitExecOptions = {} ): Promise { @@ -1302,19 +1270,21 @@ async function listQueriedWorkItems( if (requiresExplicitRepo && !issueOwnerRepo) { return { items: [] } } - const args = buildWorkItemListArgs({ + const request = buildWorkItemListRequest({ kind: 'issue', ownerRepo: issueOwnerRepo, limit, query, - before + page: page ?? 1 }) try { const { stdout } = issueOwnerRepo - ? await ghExecFileAsync(args, ghOptions) - : await ghCwdResolvedExec(repoContext, args, ghOptions) + ? await ghExecFileAsync(request.args, ghOptions) + : await ghCwdResolvedExec(repoContext, request.args, ghOptions) return { - items: (JSON.parse(stdout) as Record[]).map(mapIssueWorkItem) + items: (JSON.parse(stdout) as Record[]) + .filter((item) => !('pull_request' in item)) + .map(mapIssueWorkItem) } } catch (err) { const stderr = err instanceof Error ? err.message : String(err) @@ -1329,20 +1299,20 @@ async function listQueriedWorkItems( if (requiresExplicitRepo && !prOwnerRepo) { return [] } - const args = buildWorkItemListArgs({ + const request = buildWorkItemListRequest({ kind: 'pr', ownerRepo: prOwnerRepo, limit, query, - before + page: page ?? 1 }) try { const { stdout } = prOwnerRepo - ? await ghExecFileAsync(args, ghOptions) - : await ghCwdResolvedExec(repoContext, args, ghOptions) - const mapped = (JSON.parse(stdout) as Record[]).map((item) => - mapPullRequestWorkItem(item, prOwnerRepo) - ) + ? await ghExecFileAsync(request.args, ghOptions) + : await ghCwdResolvedExec(repoContext, request.args, ghOptions) + const mapped = (JSON.parse(stdout) as Record[]) + .slice(request.offset, request.offset + limit) + .map((item) => mapPullRequestWorkItem(item, prOwnerRepo)) const hydrated = await hydrateWorkItemRepositoryMergeMetadata(mapped, prOwnerRepo, ghOptions) if (query.state === 'closed') { return hydrated.filter((item) => item.state !== 'merged') @@ -1356,7 +1326,7 @@ async function listQueriedWorkItems( const [issueResult, prItems] = await Promise.all([issueFetch, prFetch]) return { - items: sortWorkItemsByUpdatedAt([...issueResult.items, ...prItems]).slice(0, limit), + items: sortWorkItemsByNumber([...issueResult.items, ...prItems]).slice(0, limit), issuesError: issueResult.issuesError } } @@ -1365,13 +1335,14 @@ export async function listWorkItems( repoPath: string, limit = 24, query?: string, - before?: string, + page?: number, preference?: IssueSourcePreference, connectionId?: string | null, noCache?: boolean, localGitOptions: LocalGitExecOptions = {} ): Promise> { const trimmedQuery = query?.trim() ?? '' + const requestedPage = normalizeWorkItemPage(page) if (isGitHubWorkItemsQueryTooLarge(trimmedQuery)) { return { items: [], @@ -1401,6 +1372,7 @@ export async function listWorkItems( issueOwnerRepo, prOwnerRepo, limit, + requestedPage, connectionId, noCache, localGitOptions @@ -1411,7 +1383,7 @@ export async function listWorkItems( prOwnerRepo, parseTaskQuery(trimmedQuery), limit, - before, + requestedPage, connectionId, localGitOptions ) @@ -1480,7 +1452,7 @@ function buildSearchQueryString( } function quoteGitHubSearchValue(value: string): string { - return /\s/.test(value) ? `"${value.replaceAll('"', '\\"')}"` : value + return /[\s"]/.test(value) ? `"${value.replaceAll('\\', '\\\\').replaceAll('"', '\\"')}"` : value } async function countWorkItemsForQuery( diff --git a/src/main/ipc/github.test.ts b/src/main/ipc/github.test.ts index fef0cf7aa..71f5d2866 100644 --- a/src/main/ipc/github.test.ts +++ b/src/main/ipc/github.test.ts @@ -668,7 +668,7 @@ describe('registerGitHubHandlers', () => { repoPath: '/workspace/repo', limit: 10, query: 'is:open', - before: 'cursor-1', + page: 2, noCache: true }) @@ -676,7 +676,7 @@ describe('registerGitHubHandlers', () => { '/workspace/repo', 10, 'is:open', - 'cursor-1', + 2, 'origin', null, true @@ -732,7 +732,7 @@ describe('registerGitHubHandlers', () => { repoPath: '/workspace/repo', limit: 10, query: 'is:open', - before: 'cursor-1', + page: 2, noCache: true }) await handlers['gh:countWorkItems'](null, { @@ -789,7 +789,7 @@ describe('registerGitHubHandlers', () => { '/workspace/repo', 10, 'is:open', - 'cursor-1', + 2, undefined, null, true, diff --git a/src/main/ipc/github.ts b/src/main/ipc/github.ts index a95fb887e..84afec20c 100644 --- a/src/main/ipc/github.ts +++ b/src/main/ipc/github.ts @@ -433,7 +433,7 @@ export function registerGitHubHandlers(store: Store, stats: StatsCollector): voi repoId?: string limit?: number query?: string - before?: string + page?: number noCache?: boolean } ) => { @@ -442,7 +442,7 @@ export function registerGitHubHandlers(store: Store, stats: StatsCollector): voi repo.path, args.limit, args.query, - args.before, + args.page, repo.issueSourcePreference, repoConnectionId(repo), args.noCache, diff --git a/src/main/runtime/orca-runtime.test.ts b/src/main/runtime/orca-runtime.test.ts index 2545980a8..f9e2aed30 100644 --- a/src/main/runtime/orca-runtime.test.ts +++ b/src/main/runtime/orca-runtime.test.ts @@ -5185,7 +5185,7 @@ describe('OrcaRuntimeService', () => { listGitHubLabelsMock.mockResolvedValueOnce([]) listGitHubAssignableUsersMock.mockResolvedValueOnce([]) - await runtime.listRepoWorkItems('id:repo-1', 7, 'is:open', 'cursor', true) + await runtime.listRepoWorkItems('id:repo-1', 7, 'is:open', 1, true) await runtime.countRepoWorkItems('id:repo-1', 'is:issue') await runtime.listRepoIssues('id:repo-1', 5) await runtime.getRepoIssue('id:repo-1', 12) @@ -5199,7 +5199,7 @@ describe('OrcaRuntimeService', () => { TEST_REPO_PATH, 7, 'is:open', - 'cursor', + 1, undefined, null, true, diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index 8e9be4cbb..778d5feeb 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -403,7 +403,8 @@ import { addPRReviewComment, addPRReviewCommentReply, listLabels, - listAssignableUsers + listAssignableUsers, + type MainWorkItem } from '../github/client' import type { GitHubPRBranchLookupOptions } from '../github/client' import { resolveGitHubPrStartPoint } from '../github/pr-start-point' @@ -452,6 +453,7 @@ import type { GitLabMRInlineCommentInput, GitLabProjectRef, GitLabWorkItem, + ListWorkItemsResult, MRListState } from '../../shared/types' import { inspectSetupScriptImportCandidates } from '../../shared/setup-script-imports' @@ -13185,15 +13187,15 @@ export class OrcaRuntimeService { repoSelector: string, limit?: number, query?: string, - before?: string, + page?: number, noCache?: boolean - ): Promise>> { + ): Promise> { const repo = await this.resolveRepoSelector(repoSelector) return listWorkItems( repo.path, limit, query, - before, + page, repo.issueSourcePreference, repo.connectionId ?? null, noCache, diff --git a/src/main/runtime/rpc/methods/github.ts b/src/main/runtime/rpc/methods/github.ts index c1419af16..44375181d 100644 --- a/src/main/runtime/rpc/methods/github.ts +++ b/src/main/runtime/rpc/methods/github.ts @@ -10,7 +10,7 @@ const RepoSelector = z.object({ const WorkItemsList = RepoSelector.extend({ limit: OptionalFiniteNumber, query: OptionalString, - before: OptionalString, + page: z.number().int().positive().optional(), noCache: z.boolean().optional() }) @@ -306,7 +306,7 @@ export const GITHUB_METHODS: RpcMethod[] = [ params.repo, params.limit, params.query, - params.before, + params.page, params.noCache ) }), diff --git a/src/preload/api-types.ts b/src/preload/api-types.ts index 92ba7bd92..0200555af 100644 --- a/src/preload/api-types.ts +++ b/src/preload/api-types.ts @@ -1533,7 +1533,7 @@ export type PreloadApi = { repoId?: string limit?: number query?: string - before?: string + page?: number noCache?: boolean }) => Promise>> prChecks: ( diff --git a/src/preload/index.ts b/src/preload/index.ts index 05e1d62da..42d02aec1 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -1319,7 +1319,7 @@ const api = { repoId?: string limit?: number query?: string - before?: string + page?: number noCache?: boolean }): Promise>> => ipcRenderer.invoke('gh:listWorkItems', args), diff --git a/src/renderer/src/components/TaskPage.tsx b/src/renderer/src/components/TaskPage.tsx index 99fc42d71..0ade42f28 100644 --- a/src/renderer/src/components/TaskPage.tsx +++ b/src/renderer/src/components/TaskPage.tsx @@ -205,7 +205,11 @@ import { type TaskPageRepoSourceState } from '@/components/task-page-cache-selectors' import { shouldHideTaskPageListChrome } from '@/components/task-page-list-chrome-visibility' -import { accumulateWorkItemPages } from '@/components/task-page-work-item-pagination' +import { + getTaskPagePerRepoLimit, + taskPageToGitHubApiPage +} from '@/components/task-page-work-item-pagination' +import { sortWorkItemsByNumber } from '../../../shared/work-items' import LinearIssueAttributeFilterDropdowns from '@/components/linear-issue-attribute-filter-dropdowns' import { resolveLinearIssueAttributeFilterPrimaryTeam } from '@/components/linear-issue-attribute-filter-primary-team' import { @@ -3801,15 +3805,23 @@ export default function TaskPage(): React.JSX.Element { // once, but the result is reconciled into existing rows to avoid a full // table shuffle when only status/key fields changed. const landingGitHubRefreshKeysRef = useRef>(new Set()) - // Why: pages holds all fetched pages of work items. Page 0 is seeded from - // cache for instant first paint; subsequent pages are loaded via date cursors. - const [pages, setPages] = useState(() => { + // Why: divide the display budget between repos so one provider page maps to + // one UI page without truncating rows that later provider pages cannot return. + const githubPerRepoPageLimit = getTaskPagePerRepoLimit( + selectedRepos.length, + PER_REPO_FETCH_LIMIT, + CROSS_REPO_DISPLAY_LIMIT + ) + const githubPageSize = githubPerRepoPageLimit * Math.max(1, selectedRepos.length) + // Why: null entries are pages not fetched yet. Numbered provider pages let a + // high-page click load that page directly without rate-limiting intermediate reads. + const [pages, setPages] = useState<(GitHubWorkItem[] | null)[]>(() => { const trimmed = initialTaskQuery.trim() const merged: GitHubWorkItem[] = [] for (const r of selectedRepos) { const cached = getCachedWorkItems( r.id, - PER_REPO_FETCH_LIMIT, + githubPerRepoPageLimit, trimmed, r.path, getTaskPageRepoSourceContext(r, 'github') @@ -3821,15 +3833,13 @@ export default function TaskPage(): React.JSX.Element { if (merged.length === 0) { return [[]] } - const page0 = [...merged] - .sort((a, b) => new Date(b.updatedAt).getTime() - new Date(a.updatedAt).getTime()) - .slice(0, CROSS_REPO_DISPLAY_LIMIT) + const page0 = sortWorkItemsByNumber(merged).slice(0, githubPageSize) return [page0] }) const [currentPage, setCurrentPage] = useState(0) const [paginationLoading, setPaginationLoading] = useState(false) const [loadingTargetPage, setLoadingTargetPage] = useState(null) - const [totalItemCount, setTotalItemCount] = useState(null) + const [countedTotalPages, setCountedTotalPages] = useState(null) const fetchWorkItemsNextPage = useAppStore((s) => s.fetchWorkItemsNextPage) const countWorkItemsAcrossRepos = useAppStore((s) => s.countWorkItemsAcrossRepos) @@ -3859,7 +3869,7 @@ export default function TaskPage(): React.JSX.Element { selectTaskPageWorkItemsCacheEntries( s.workItemsCache, selectedRepos.map(getTaskPageRepoCacheInput), - PER_REPO_FETCH_LIMIT, + githubPerRepoPageLimit, appliedWorkItemsCacheQuery ) ) @@ -3982,6 +3992,9 @@ export default function TaskPage(): React.JSX.Element { setPages((current) => { let changed = false const nextPages = current.map((page) => { + if (!page) { + return page + } let pageChanged = false const nextPage = page.map((item) => { if (item.id !== itemKey.id || item.repoId !== itemKey.repoId) { @@ -6156,6 +6169,9 @@ export default function TaskPage(): React.JSX.Element { const seen = new Set() const logins: string[] = [] for (const page of pages) { + if (!page) { + continue + } for (const item of page) { if ( !item.author || @@ -6229,45 +6245,31 @@ export default function TaskPage(): React.JSX.Element { } }, [ensurePRChecksLoaded, filteredWorkItems, githubMode, showPRManagementColumns, taskSource]) - // Why: each page is bounded by both the per-repo fetch budget (so a single - // repo can yield at most PER_REPO_FETCH_LIMIT rows per page) and the - // post-merge display cap. Dividing the total by CROSS_REPO_DISPLAY_LIMIT - // alone under-counts pages when the per-repo budget is the tighter bound — - // e.g. one repo with 60 open PRs yielded 36 rows on page 0 and ceil(60/100) - // = 1 total pages, hiding the pager entirely. - const effectivePageSize = Math.max( - 1, - Math.min(PER_REPO_FETCH_LIMIT * Math.max(1, selectedRepos.length), CROSS_REPO_DISPLAY_LIMIT) - ) - // Why: if the search-API count is unavailable (rate-limited, transient - // failure — `countWorkItemsAcrossRepos` swallows errors and returns 0), - // infer there's at least one more page whenever the last loaded page - // filled to the per-page capacity. Otherwise pagination would silently - // hide and trap the user on page 0 with no way to advance. - const lastPageFull = (pages.at(-1)?.length ?? 0) >= effectivePageSize + let lastLoadedPageIndex = 0 + for (let index = 0; index < pages.length; index += 1) { + if (pages[index] !== null) { + lastLoadedPageIndex = index + } + } + // Why: when counts fail, a full loaded page is enough evidence to expose one + // more page without pretending unfetched sparse entries are empty results. + const lastLoadedPageFull = + (pages[lastLoadedPageIndex]?.length ?? 0) >= Math.max(1, githubPageSize) + const fallbackTotalPages = lastLoadedPageFull + ? Math.max(pages.length, lastLoadedPageIndex + 2) + : Math.max(1, pages.length) const totalPages = - totalItemCount && totalItemCount > 0 - ? Math.max(pages.length, Math.ceil(totalItemCount / effectivePageSize)) - : lastPageFull - ? pages.length + 1 - : pages.length + countedTotalPages && countedTotalPages > 0 + ? Math.max(pages.length, countedTotalPages) + : fallbackTotalPages - // Why: loads the next page using the oldest item's updatedAt as a cursor. - // When targetPage is provided (from clicking a numbered page beyond loaded - // pages), it chains fetches until that page is loaded. + // Why: numbered provider pages support random access. Load only the clicked + // page so a high-page jump does not exhaust GitHub's Search API rate bucket. const handleLoadNextPage = useCallback( async (targetPage?: number) => { if (paginationLoading || selectedRepos.length === 0) { return } - const lastPage = pages.at(-1) - if (!lastPage || lastPage.length === 0) { - return - } - const oldestItem = lastPage.at(-1) - if (!oldestItem?.updatedAt) { - return - } const q = stripRepoQualifiers(appliedTaskSearch.trim()) const repoArgs = selectedRepos.map((r) => ({ repoId: r.id, @@ -6277,37 +6279,32 @@ export default function TaskPage(): React.JSX.Element { })) const requestGeneration = paginationGenerationRef.current - const target = targetPage ?? pages.length + const target = targetPage ?? currentPage + 1 setPaginationLoading(true) setLoadingTargetPage(target) try { - const result = await accumulateWorkItemPages({ - existingPages: pages, - initialCursor: oldestItem.updatedAt, - targetPage: target, - // Why: uniform page size keeps deduped pages from shrinking below the - // size `totalPages` (count ÷ effectivePageSize) assumes — otherwise - // the per-page overlap loss would strand the tail items. - pageSize: effectivePageSize, - isCancelled: () => paginationGenerationRef.current !== requestGeneration, - fetchPage: async (cursor) => { - const { items } = await fetchWorkItemsNextPage( - repoArgs, - PER_REPO_FETCH_LIMIT, - CROSS_REPO_DISPLAY_LIMIT, - q, - cursor - ) - return { items } - } - }) - if (result.cancelled) { + const { items } = await fetchWorkItemsNextPage( + repoArgs, + githubPerRepoPageLimit, + githubPageSize, + q, + taskPageToGitHubApiPage(target) + ) + if (paginationGenerationRef.current !== requestGeneration) { return } - if (result.newPages.length > 0) { - setPages((prev) => [...prev, ...result.newPages]) - setCurrentPage(target < result.loadedPages ? target : result.loadedPages - 1) + if (items.length === 0) { + return } + setPages((previous) => { + const next = [...previous] + while (next.length <= target) { + next.push(null) + } + next[target] = items + return next + }) + setCurrentPage(target) } catch (err) { console.error('Failed to load next page:', err) } finally { @@ -6320,10 +6317,11 @@ export default function TaskPage(): React.JSX.Element { [ paginationLoading, selectedRepos, - pages, + currentPage, appliedTaskSearch, fetchWorkItemsNextPage, - effectivePageSize + githubPageSize, + githubPerRepoPageLimit ] ) @@ -6399,7 +6397,7 @@ export default function TaskPage(): React.JSX.Element { for (const r of selectedRepos) { const cached = getCachedWorkItems( r.id, - PER_REPO_FETCH_LIMIT, + githubPerRepoPageLimit, q, r.path, getTaskPageRepoSourceContext(r, 'github') @@ -6415,14 +6413,10 @@ export default function TaskPage(): React.JSX.Element { // no repo has a cache entry for it), we clear the previous query's rows // rather than leaving them on screen under the spinner. const page0 = - preMerged.length > 0 - ? [...preMerged] - .sort((a, b) => new Date(b.updatedAt).getTime() - new Date(a.updatedAt).getTime()) - .slice(0, CROSS_REPO_DISPLAY_LIMIT) - : [] + preMerged.length > 0 ? sortWorkItemsByNumber(preMerged).slice(0, githubPageSize) : [] setPages([page0]) setCurrentPage(0) - setTotalItemCount(null) + setCountedTotalPages(null) setTasksError(null) setFailedCount(0) // reset so a prior failure banner doesn't linger setTasksLoading(anyUncached) @@ -6464,7 +6458,7 @@ export default function TaskPage(): React.JSX.Element { // newer retry's source from the set. Clearing only the keys captured // when this effect dispatched preserves later additions. const dispatchedRetrySourceKeys = retryingSourceKeys - void fetchWorkItemsAcrossRepos(repoArgs, PER_REPO_FETCH_LIMIT, CROSS_REPO_DISPLAY_LIMIT, q, { + void fetchWorkItemsAcrossRepos(repoArgs, githubPerRepoPageLimit, githubPageSize, q, { ...deriveTaskPageGitHubWorkItemsFetchOptions(forcedFetch, shouldProbeOnLanding) }) .then(({ items, failedCount: failed }) => { @@ -6542,10 +6536,11 @@ export default function TaskPage(): React.JSX.Element { executionHostId: r.executionHostId, sourceContext: getTaskPageRepoSourceContext(r, 'github') })), - q - ).then((count) => { + q, + githubPerRepoPageLimit + ).then(({ totalPages: countedPages }) => { if (!cancelled) { - setTotalItemCount(count) + setCountedTotalPages(countedPages) } }) @@ -9834,7 +9829,7 @@ export default function TaskPage(): React.JSX.Element { totalPages={totalPages} loadingTarget={loadingTargetPage} onPageChange={(page) => { - if (page < pages.length) { + if (pages[page] !== null && pages[page] !== undefined) { setCurrentPage(page) } else { void handleLoadNextPage(page) diff --git a/src/renderer/src/components/task-page-cache-selectors.test.ts b/src/renderer/src/components/task-page-cache-selectors.test.ts index 98703dd23..71b85b8ad 100644 --- a/src/renderer/src/components/task-page-cache-selectors.test.ts +++ b/src/renderer/src/components/task-page-cache-selectors.test.ts @@ -146,8 +146,8 @@ describe('task page cache selectors', () => { entry([patched]) ]) - expect(nextPages[0][0]).toBe(patched) - expect(nextPages[0][1]).toBe(otherRepoSameId) + expect(nextPages[0]?.[0]).toBe(patched) + expect(nextPages[0]?.[1]).toBe(otherRepoSameId) }) it('merges landing refresh status changes without reordering GitHub rows', () => { diff --git a/src/renderer/src/components/task-page-cache-selectors.ts b/src/renderer/src/components/task-page-cache-selectors.ts index 490db04d9..67b3a7c1c 100644 --- a/src/renderer/src/components/task-page-cache-selectors.ts +++ b/src/renderer/src/components/task-page-cache-selectors.ts @@ -37,6 +37,7 @@ type WorkItemsCache = Record> type LinearIssueCache = Record> type LinearSearchCache = Record> type LinearListCache = Record>> +export type TaskPageWorkItemPages = readonly (GitHubWorkItem[] | null)[] export function deriveTaskPageGitHubWorkItemsFetchOptions( forcedFetch: boolean, @@ -83,9 +84,9 @@ function taskPageWorkItemCacheKey(item: GitHubWorkItem): string { } export function reconcileTaskPagePagesWithWorkItemsCache( - pages: readonly GitHubWorkItem[][], + pages: TaskPageWorkItemPages, entries: readonly (CacheEntry | undefined)[] -): GitHubWorkItem[][] { +): (GitHubWorkItem[] | null)[] { const cachedItems = new Map() for (const entry of entries) { for (const item of entry?.data ?? []) { @@ -95,6 +96,9 @@ export function reconcileTaskPagePagesWithWorkItemsCache( let changed = false const nextPages = pages.map((page) => { + if (!page) { + return null + } let pageChanged = false const nextPage = page.map((item) => { const cached = cachedItems.get(taskPageWorkItemCacheKey(item)) @@ -108,7 +112,7 @@ export function reconcileTaskPagePagesWithWorkItemsCache( return pageChanged ? nextPage : page }) - return changed ? nextPages : (pages as GitHubWorkItem[][]) + return changed ? nextPages : (pages as (GitHubWorkItem[] | null)[]) } function taskPageWorkItemKey(item: GitHubWorkItem): string { @@ -221,16 +225,16 @@ export function shouldResetTaskPagePaginationAfterLandingRefresh( } export function reconcileTaskPagePagesAfterLandingRefresh( - pages: readonly GitHubWorkItem[][], + pages: TaskPageWorkItemPages, refreshedItems: readonly GitHubWorkItem[] -): GitHubWorkItem[][] { +): (GitHubWorkItem[] | null)[] { const firstPage = pages[0] ?? [] if (shouldResetTaskPagePaginationAfterLandingRefresh(firstPage, refreshedItems)) { return [[...refreshedItems]] } const nextFirstPage = reconcileTaskPageItemsAfterLandingRefresh(firstPage, refreshedItems) if (nextFirstPage === firstPage) { - return pages as GitHubWorkItem[][] + return pages as (GitHubWorkItem[] | null)[] } return [nextFirstPage, ...pages.slice(1)] } diff --git a/src/renderer/src/components/task-page-work-item-pagination.test.ts b/src/renderer/src/components/task-page-work-item-pagination.test.ts index b3bc3301d..22ad6bf6c 100644 --- a/src/renderer/src/components/task-page-work-item-pagination.test.ts +++ b/src/renderer/src/components/task-page-work-item-pagination.test.ts @@ -1,6 +1,11 @@ import { describe, expect, it, vi } from 'vitest' import type { GitHubWorkItem } from '../../../shared/types' -import { accumulateWorkItemPages, workItemIdentity } from './task-page-work-item-pagination' +import { + accumulateWorkItemPages, + getTaskPagePerRepoLimit, + taskPageToGitHubApiPage, + workItemIdentity +} from './task-page-work-item-pagination' function item(repoId: string, id: string, updatedAt: string): GitHubWorkItem { return { @@ -39,6 +44,21 @@ describe('workItemIdentity', () => { }) }) +describe('numbered GitHub pagination', () => { + it('converts zero-indexed task pages to one-indexed API pages', () => { + expect(taskPageToGitHubApiPage(0)).toBe(1) + expect(taskPageToGitHubApiPage(1)).toBe(2) + expect(taskPageToGitHubApiPage(15)).toBe(16) + }) + + it('divides the display budget before per-repo fetches can overflow it', () => { + expect(getTaskPagePerRepoLimit(1, 36, 100)).toBe(36) + expect(getTaskPagePerRepoLimit(2, 36, 100)).toBe(36) + expect(getTaskPagePerRepoLimit(3, 36, 100)).toBe(33) + expect(getTaskPagePerRepoLimit(90, 36, 100)).toBe(1) + }) +}) + describe('accumulateWorkItemPages', () => { it('drops the re-fetched boundary row that shares the previous page cursor', async () => { const boundary = item('r', 'issue:2', '2026-07-02') diff --git a/src/renderer/src/components/task-page-work-item-pagination.ts b/src/renderer/src/components/task-page-work-item-pagination.ts index a332fe675..e8f0a8749 100644 --- a/src/renderer/src/components/task-page-work-item-pagination.ts +++ b/src/renderer/src/components/task-page-work-item-pagination.ts @@ -31,6 +31,21 @@ export function workItemIdentity(item: Pick): s return `${item.repoId}:${item.id}` } +export function taskPageToGitHubApiPage(taskPage: number): number { + return Math.max(0, Math.floor(taskPage)) + 1 +} + +// Why: provider pages cannot spill truncated rows into the next page. Divide +// the display budget up front so every fetched row remains reachable. +export function getTaskPagePerRepoLimit( + repoCount: number, + maxPerRepo: number, + displayLimit: number +): number { + const normalizedRepoCount = Math.max(1, Math.floor(repoCount)) + return Math.max(1, Math.min(maxPerRepo, Math.floor(displayLimit / normalizedRepoCount))) +} + export type WorkItemPageFetchResult = { items: GitHubWorkItem[] } export type AccumulateWorkItemPagesArgs = { diff --git a/src/renderer/src/store/slices/github.test.ts b/src/renderer/src/store/slices/github.test.ts index 730a9105e..6ddba03ca 100644 --- a/src/renderer/src/store/slices/github.test.ts +++ b/src/renderer/src/store/slices/github.test.ts @@ -6331,7 +6331,7 @@ describe('createGitHubSlice.fetchWorkItems source/error envelope', () => { 24, 100, '', - '2026-05-21T00:00:00Z' + 1 ) expect(result.failedCount).toBe(0) @@ -6377,7 +6377,7 @@ describe('createGitHubSlice.fetchWorkItems source/error envelope', () => { 24, 100, 'is:open', - '2026-05-22T00:00:00Z' + 1 ) expect(mockApi.gh.listWorkItems).not.toHaveBeenCalled() @@ -6388,7 +6388,7 @@ describe('createGitHubSlice.fetchWorkItems source/error envelope', () => { repo: 'runtime-repo-id', limit: 24, query: 'is:open', - before: '2026-05-22T00:00:00Z' + page: 1 }, timeoutMs: 30_000 }) @@ -6413,9 +6413,13 @@ describe('createGitHubSlice.fetchWorkItems source/error envelope', () => { const result = await store .getState() - .countWorkItemsAcrossRepos([{ repoId: 'caller-repo-id', path: '/server/repo' }], 'is:open') + .countWorkItemsAcrossRepos( + [{ repoId: 'caller-repo-id', path: '/server/repo' }], + 'is:open', + 10 + ) - expect(result).toBe(12) + expect(result).toEqual({ totalCount: 12, totalPages: 2 }) expect(mockApi.gh.countWorkItems).not.toHaveBeenCalled() expect(runtimeEnvironmentCall).toHaveBeenCalledWith({ selector: 'env-1', @@ -6434,9 +6438,9 @@ describe('createGitHubSlice.fetchWorkItems source/error envelope', () => { const result = await store .getState() - .countWorkItemsAcrossRepos([{ repoId: 'repo-id', path: '/local/repo' }], '') + .countWorkItemsAcrossRepos([{ repoId: 'repo-id', path: '/local/repo' }], '', 10) - expect(result).toBe(7) + expect(result).toEqual({ totalCount: 7, totalPages: 1 }) expect(runtimeEnvironmentCall).not.toHaveBeenCalled() expect(mockApi.gh.countWorkItems).toHaveBeenCalledWith({ repoPath: '/local/repo', @@ -6445,6 +6449,22 @@ describe('createGitHubSlice.fetchWorkItems source/error envelope', () => { }) }) + it('derives page count from the repo with the most results', async () => { + const store = createTestStore() + mockApi.gh.countWorkItems.mockResolvedValueOnce(100).mockResolvedValueOnce(1) + + const result = await store.getState().countWorkItemsAcrossRepos( + [ + { repoId: 'large-repo', path: '/local/large' }, + { repoId: 'small-repo', path: '/local/small' } + ], + 'is:issue', + 36 + ) + + expect(result).toEqual({ totalCount: 101, totalPages: 3 }) + }) + it('rejects oversized work-item queries before cache keys or provider calls', async () => { const store = createTestStore() const secret = 'github-work-items-secret' @@ -6471,14 +6491,14 @@ describe('createGitHubSlice.fetchWorkItems source/error envelope', () => { 24, 24, oversizedQuery, - 'cursor' + 1 ) ).resolves.toEqual({ items: [], failedCount: 0 }) await expect( store .getState() - .countWorkItemsAcrossRepos([{ repoId: 'repo-id', path: '/local/repo' }], oversizedQuery) - ).resolves.toBe(0) + .countWorkItemsAcrossRepos([{ repoId: 'repo-id', path: '/local/repo' }], oversizedQuery, 24) + ).resolves.toEqual({ totalCount: 0, totalPages: 0 }) store.getState().prefetchWorkItems('repo-id', '/local/repo', 24, oversizedQuery) expect(store.getState().getCachedWorkItems('repo-id', 24, oversizedQuery, '/local/repo')).toBe( diff --git a/src/renderer/src/store/slices/github.ts b/src/renderer/src/store/slices/github.ts index 0a24e24c0..ef5e67111 100644 --- a/src/renderer/src/store/slices/github.ts +++ b/src/renderer/src/store/slices/github.ts @@ -34,7 +34,7 @@ import type { } from '../../../../shared/github-project-types' import { isGitHubWorkItemsSshRemoteRequiredError, - sortWorkItemsByUpdatedAt, + sortWorkItemsByNumber, PER_REPO_FETCH_LIMIT } from '../../../../shared/work-items' import { deriveCheckStatusFromChecks, syncPRChecksStatus } from './github-checks' @@ -206,7 +206,7 @@ type GitHubWorkItemRequestTarget = type GitHubWorkItemsListArgs = { limit: number query?: string - before?: string + page?: number noCache?: true } @@ -2043,10 +2043,7 @@ export type GitHubSlice = { query: string, options?: FetchOptions ) => Promise<{ items: GitHubWorkItem[]; failedCount: number }> - /** - * Fetch the next page of work items using a date cursor. Does not cache — - * pagination pages are ephemeral and managed by TaskPage state. - */ + /** Fetch one numbered provider page. Pagination pages remain renderer-local. */ fetchWorkItemsNextPage: ( repos: { repoId: string @@ -2057,12 +2054,9 @@ export type GitHubSlice = { perRepoLimit: number, displayLimit: number, query: string, - before: string + page: number ) => Promise<{ items: GitHubWorkItem[]; failedCount: number }> - /** - * Count total work items across repos using GitHub's search API. - * Returns the sum of per-repo counts for the given query. - */ + /** Count items and derive pages from the largest per-repo result set. */ countWorkItemsAcrossRepos: ( repos: { repoId: string @@ -2070,8 +2064,9 @@ export type GitHubSlice = { executionHostId?: string | null sourceContext?: TaskSourceContext | null }[], - query: string - ) => Promise + query: string, + perRepoLimit: number + ) => Promise<{ totalCount: number; totalPages: number }> /** * Fire-and-forget prefetch used by UI entry points (hover/focus of the * "new workspace" buttons) to warm the cache before the page mounts. @@ -2896,11 +2891,11 @@ export const createGitHubSlice: StateCreator = (s } }) ) - const merged = sortWorkItemsByUpdatedAt(perProjectResults.flat()).slice(0, displayLimit) + const merged = sortWorkItemsByNumber(perProjectResults.flat()).slice(0, displayLimit) return { items: merged, failedCount } }, - fetchWorkItemsNextPage: async (repos, perRepoLimit, displayLimit, query, before) => { + fetchWorkItemsNextPage: async (repos, perRepoLimit, displayLimit, query, page) => { if (isGitHubWorkItemsQueryTooLarge(query)) { return { items: [], failedCount: 0 } } @@ -2926,7 +2921,7 @@ export const createGitHubSlice: StateCreator = (s const envelope = await listGitHubWorkItemsForRepo(requestContext, { limit: perRepoLimit, query: query || undefined, - before + page }) // Why: page-N partial failures don't participate in the cache's per-repo // error banner (which is keyed on the initial-fetch cache entry). Log the @@ -2954,14 +2949,15 @@ export const createGitHubSlice: StateCreator = (s } }) ) - const merged = sortWorkItemsByUpdatedAt(perProjectResults.flat()).slice(0, displayLimit) + const merged = sortWorkItemsByNumber(perProjectResults.flat()).slice(0, displayLimit) return { items: merged, failedCount } }, - countWorkItemsAcrossRepos: async (repos, query) => { + countWorkItemsAcrossRepos: async (repos, query, perRepoLimit) => { if (isGitHubWorkItemsQueryTooLarge(query)) { - return 0 + return { totalCount: 0, totalPages: 0 } } + const normalizedLimit = Math.max(1, Math.floor(perRepoLimit)) const counts = await Promise.all( repos.map(async (r) => { // Why: same stampede cap as the item-fetch paths — without a slot, @@ -2991,7 +2987,15 @@ export const createGitHubSlice: StateCreator = (s } }) ) - return counts.reduce((sum, c) => sum + c, 0) + return { + totalCount: counts.reduce((sum, count) => sum + count, 0), + // Why: each repo advances independently by the same numbered page. A sum + // divided by page width undercounts when one repo owns most results. + totalPages: counts.reduce( + (maxPages, count) => Math.max(maxPages, Math.ceil(count / normalizedLimit)), + 0 + ) + } }, prefetchWorkItems: (repoId, repoPath, limit = PER_REPO_FETCH_LIMIT, query = '', options) => { diff --git a/src/renderer/src/web/web-preload-api.test.ts b/src/renderer/src/web/web-preload-api.test.ts index 749eab542..bb77e2362 100644 --- a/src/renderer/src/web/web-preload-api.test.ts +++ b/src/renderer/src/web/web-preload-api.test.ts @@ -2521,13 +2521,13 @@ describe('web GitHub preload API', () => { }, { key: 'listWorkItems', - args: { repoPath, limit: 20, query: 'is:pr', before: 'cursor', noCache: true }, + args: { repoPath, limit: 20, query: 'is:pr', page: 2, noCache: true }, expectedMethod: 'github.listWorkItems', expectedParams: withRepo({ repoPath, limit: 20, query: 'is:pr', - before: 'cursor', + page: 2, noCache: true }) }, diff --git a/src/shared/work-items.ts b/src/shared/work-items.ts index 848cde95d..0a770058c 100644 --- a/src/shared/work-items.ts +++ b/src/shared/work-items.ts @@ -26,6 +26,13 @@ export function isGitHubWorkItemsSshRemoteRequiredError(error: unknown): boolean return message.includes(GITHUB_WORK_ITEMS_SSH_REMOTE_REQUIRED_MESSAGE) } +// Why: generic over item shape for the same cross-caller reasons as +// sortWorkItemsByUpdatedAt. Sorting by number descending matches GitHub's +// default Issues view (newest issue number first). +export function sortWorkItemsByNumber(items: T[]): T[] { + return [...items].sort((left, right) => right.number - left.number) +} + // Why: generic over the item shape because main-process callers emit items // without repoId (stamped by the renderer after IPC), while renderer callers // carry the full GitHubWorkItem. Both share only the updatedAt field needed