diff --git a/src/renderer/src/components/TaskPage.tsx b/src/renderer/src/components/TaskPage.tsx index c07d32aee9b..acbe1a6b0e1 100644 --- a/src/renderer/src/components/TaskPage.tsx +++ b/src/renderer/src/components/TaskPage.tsx @@ -205,6 +205,7 @@ import { } from '@/components/task-page-cache-selectors' import { shouldHideTaskPageListChrome } from '@/components/task-page-list-chrome-visibility' import { + buildSelectedReposKey, getTaskPagePerRepoLimit, resolveEmptyPageOutcome, taskPageToGitHubApiPage @@ -3206,20 +3207,10 @@ export default function TaskPage(): React.JSX.Element { [eligibleRepos, repoSelection] ) - // Why: stable string key for selectedRepos — the repos store installs a fresh - // array on every repos:changed event, so array-identity deps re-fire even when - // the selection is unchanged. Includes the resolved GitHub source context so a - // provider-identity change still re-keys everything the requests depend on. + // Why: see buildSelectedReposKey — array-identity deps re-fire on every + // repos:changed even when the selection is unchanged. const selectedReposKey = useMemo( - () => - selectedRepos - .map( - (r) => - `${r.id}|${r.path}|${r.connectionId ?? ''}|${r.executionHostId ?? ''}|${JSON.stringify( - getTaskPageRepoSourceContext(r, 'github') - )}` - ) - .join(','), + () => buildSelectedReposKey(selectedRepos, (r) => getTaskPageRepoSourceContext(r, 'github')), [selectedRepos] ) @@ -3830,12 +3821,21 @@ export default function TaskPage(): React.JSX.Element { // Why: keyed on selectedReposKey, not the selectedRepos array — a background // repos:changed refresh mid-flight would otherwise bump the generation and - // silently discard the user's page navigation (#11485). + // silently discard the user's page navigation (#11485). Mirrors every dep of + // the fetch effect that resets page state, so a reset always invalidates + // in-flight page requests. useEffect(() => { paginationGenerationRef.current += 1 setPaginationLoading(false) setLoadingTargetPage(null) - }, [selectedReposKey, appliedTaskSearch, workItemsInvalidationNonce]) + }, [ + selectedReposKey, + appliedTaskSearch, + workItemsInvalidationNonce, + taskRefreshNonce, + taskSource, + githubMode + ]) // Why: the dialog's "Use" button routes through the same direct-launch flow as the row-level "Use" CTA so behavior is consistent regardless of entry point. const githubTaskDrawerWorkItem = useAppStore((s) => s.githubTaskDrawerWorkItem) @@ -6190,7 +6190,12 @@ export default function TaskPage(): React.JSX.Element { if (items.length === 0) { // Why: see resolveEmptyPageOutcome — a dead click needs feedback only // when something actually failed; a clean empty probe is end-of-data. - const outcome = resolveEmptyPageOutcome({ target, failedCount, issueErrorTypes }) + const outcome = resolveEmptyPageOutcome({ + target, + failedCount, + issueErrorTypes, + countedTotalPages + }) if (outcome.reason === 'window-unreachable') { toast.error( translate( @@ -6242,6 +6247,7 @@ export default function TaskPage(): React.JSX.Element { paginationLoading, selectedRepos, currentPage, + countedTotalPages, appliedTaskSearch, fetchWorkItemsNextPage, githubPageSize, 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 26fab4b6629..a54161fd9a1 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 @@ -2,6 +2,7 @@ import { describe, expect, it, vi } from 'vitest' import type { GitHubWorkItem } from '../../../shared/types' import { accumulateWorkItemPages, + buildSelectedReposKey, getTaskPagePerRepoLimit, resolveEmptyPageOutcome, taskPageToGitHubApiPage, @@ -61,41 +62,95 @@ describe('numbered GitHub pagination', () => { }) describe('resolveEmptyPageOutcome', () => { + const base = { failedCount: 0, issueErrorTypes: [] as const, countedTotalPages: null } + it('clamps and reports unreachable when the search window 422 is present', () => { expect( - resolveEmptyPageOutcome({ target: 33, failedCount: 0, issueErrorTypes: ['validation_error'] }) + resolveEmptyPageOutcome({ ...base, target: 33, issueErrorTypes: ['validation_error'] }) ).toEqual({ reason: 'window-unreachable', clampTotalPagesTo: 33 }) - // A thrown repo alongside the 422 doesn't mask the window signal. + // A real count is still clamped — the count over-advertising is the bug. expect( - resolveEmptyPageOutcome({ target: 5, failedCount: 1, issueErrorTypes: ['validation_error'] }) - ).toEqual({ reason: 'window-unreachable', clampTotalPagesTo: 5 }) + resolveEmptyPageOutcome({ + ...base, + target: 33, + issueErrorTypes: ['validation_error'], + countedTotalPages: 39 + }) + ).toEqual({ reason: 'window-unreachable', clampTotalPagesTo: 33 }) + }) + + it('does not clamp on a window 422 when a sibling repo fetch threw', () => { + // Repos advance independently; another repo's transient failure says + // nothing about the healthy repos' remaining pages. + expect( + resolveEmptyPageOutcome({ + ...base, + target: 5, + failedCount: 1, + issueErrorTypes: ['validation_error'] + }) + ).toEqual({ reason: 'window-unreachable', clampTotalPagesTo: null }) }) it('reports a failure without clamping for transient or unclassified errors', () => { - expect(resolveEmptyPageOutcome({ target: 5, failedCount: 2, issueErrorTypes: [] })).toEqual({ + expect(resolveEmptyPageOutcome({ ...base, target: 5, failedCount: 2 })).toEqual({ reason: 'load-failed', clampTotalPagesTo: null }) for (const type of ['permission_denied', 'not_found', 'rate_limited', 'unknown'] as const) { - expect( - resolveEmptyPageOutcome({ target: 5, failedCount: 0, issueErrorTypes: [type] }) - ).toEqual({ reason: 'load-failed', clampTotalPagesTo: null }) + expect(resolveEmptyPageOutcome({ ...base, target: 5, issueErrorTypes: [type] })).toEqual({ + reason: 'load-failed', + clampTotalPagesTo: null + }) } }) - it('treats a clean empty probe as end-of-data and withdraws the speculative page', () => { - expect(resolveEmptyPageOutcome({ target: 2, failedCount: 0, issueErrorTypes: [] })).toEqual({ + it('withdraws the speculative page on clean empty only while the count is unknown', () => { + expect(resolveEmptyPageOutcome({ ...base, target: 2 })).toEqual({ reason: 'end-of-data', clampTotalPagesTo: 2 }) + // 0 means the count itself failed — treat like unknown. + expect(resolveEmptyPageOutcome({ ...base, target: 2, countedTotalPages: 0 })).toEqual({ + reason: 'end-of-data', + clampTotalPagesTo: 2 + }) + // A real count must not shrink: the PR list path swallows its own failures + // into clean-empty results, and clamping would hide healthy pages silently. + expect(resolveEmptyPageOutcome({ ...base, target: 2, countedTotalPages: 34 })).toEqual({ + reason: 'end-of-data', + clampTotalPagesTo: null + }) // Never clamps below one advertised page. - expect(resolveEmptyPageOutcome({ target: 0, failedCount: 0, issueErrorTypes: [] })).toEqual({ + expect(resolveEmptyPageOutcome({ ...base, target: 0 })).toEqual({ reason: 'end-of-data', clampTotalPagesTo: 1 }) }) }) +describe('buildSelectedReposKey', () => { + const contextFor = (r: { id: string }) => ({ provider: 'github', repoId: r.id }) + + it('is stable across fresh arrays with identical contents', () => { + const a = [{ id: 'r1', path: '/a', connectionId: null, executionHostId: null }] + const b = [{ ...a[0] }] + expect(buildSelectedReposKey(a, contextFor)).toBe(buildSelectedReposKey(b, contextFor)) + }) + + it('changes when any request-relevant field changes', () => { + const repo = { id: 'r1', path: '/a', connectionId: null, executionHostId: null } + const key = buildSelectedReposKey([repo], contextFor) + expect(buildSelectedReposKey([{ ...repo, executionHostId: 'ssh:host' }], contextFor)).not.toBe( + key + ) + expect(buildSelectedReposKey([{ ...repo, path: '/b' }], contextFor)).not.toBe(key) + expect( + buildSelectedReposKey([repo], (r) => ({ provider: 'github', repoId: r.id, owner: 'new' })) + ).not.toBe(key) + }) +}) + 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 f2811e6803d..1a7cf450a78 100644 --- a/src/renderer/src/components/task-page-work-item-pagination.ts +++ b/src/renderer/src/components/task-page-work-item-pagination.ts @@ -45,26 +45,59 @@ export type EmptyPageOutcome = { * An empty page load has three distinct meanings, and only the caller-side * error channels can tell them apart (#11485): * - a `validation_error` is GitHub's 422 for pages past its 1000-result search - * window — the page can never load, so stop advertising it; + * window — the page can never load, so stop advertising it (unless a sibling + * repo's fetch threw, which says nothing about the other repos' windows); * - any other per-repo error (thrown or issue-side envelope) may be transient * (rate limit, permissions), so surface it but keep the advertised count; - * - no error at all is the designed end-of-data signal from probing the - * speculative page `fallbackTotalPages` advertises while the count is - * unknown — withdraw the phantom page silently. + * - no error at all is end-of-data. That only warrants a clamp while the count + * is unknown (null) or failed (0), to withdraw the speculative page + * `fallbackTotalPages` advertises — a real count must not shrink, because a + * provider that swallows its own failures (the PR list path) also produces + * clean-empty results. */ export function resolveEmptyPageOutcome(args: { target: number failedCount: number issueErrorTypes: readonly ClassifiedError['type'][] + countedTotalPages: number | null }): EmptyPageOutcome { const clamp = Math.max(1, Math.floor(args.target)) if (args.issueErrorTypes.includes('validation_error')) { - return { reason: 'window-unreachable', clampTotalPagesTo: clamp } + return { + reason: 'window-unreachable', + clampTotalPagesTo: args.failedCount === 0 ? clamp : null + } } if (args.failedCount > 0 || args.issueErrorTypes.length > 0) { return { reason: 'load-failed', clampTotalPagesTo: null } } - return { reason: 'end-of-data', clampTotalPagesTo: clamp } + const countUnknown = args.countedTotalPages === null || args.countedTotalPages === 0 + return { reason: 'end-of-data', clampTotalPagesTo: countUnknown ? clamp : null } +} + +/** + * Stable identity string for a repo selection. The repos store installs a fresh + * array on every repos:changed event, so effects keyed on array identity re-fire + * with the selection unchanged — resetting pagination mid-click (#11485). The + * key must cover every repo field the work-item requests read; the caller + * supplies the resolved source context (which must stay free of timestamps). + */ +export function buildSelectedReposKey< + T extends { + id: string + path: string + connectionId?: string | null + executionHostId?: string | null + } +>(repos: readonly T[], sourceContextFor: (repo: T) => unknown): string { + return repos + .map( + (r) => + `${r.id}|${r.path}|${r.connectionId ?? ''}|${r.executionHostId ?? ''}|${JSON.stringify( + sourceContextFor(r) + )}` + ) + .join(',') } // Why: provider pages cannot spill truncated rows into the next page. Divide diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index 17bbfd3792e..5b34b6020c3 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -1890,7 +1890,7 @@ "noGithubSourceDetected": "No GitHub source detected for", "noGithubSourceDetectedHint": "it may have no GitHub remote, or the source could not be resolved.", "jiraLinkSourceUnavailable": "Couldn’t link this Jira issue. Reconnect Jira or pick the matching site, then try again.", - "loadPageUnreachable": "Page {{value0}} could not be loaded from GitHub.", + "loadPageUnreachable": "Page {{value0}} is beyond what GitHub search can return.", "loadPageFailed": "Page {{value0}} could not be loaded from GitHub." }, "Terminal": { diff --git a/src/renderer/src/store/slices/github.test.ts b/src/renderer/src/store/slices/github.test.ts index e6a9d89b894..5706e776c39 100644 --- a/src/renderer/src/store/slices/github.test.ts +++ b/src/renderer/src/store/slices/github.test.ts @@ -6661,6 +6661,39 @@ describe('createGitHubSlice.fetchWorkItems source/error envelope', () => { }) }) + it('surfaces issue-side envelope errors as issueErrorTypes on next-page fetches', async () => { + runtimeEnvironmentCall.mockResolvedValueOnce({ + id: 'rpc-work-items-page-422', + ok: true, + result: { + items: [], + sources: { + issues: { owner: 'up', repo: 'r' }, + prs: null, + originCandidate: { owner: 'up', repo: 'r' }, + upstreamCandidate: null + }, + errors: { + issues: { type: 'validation_error', message: 'only the first 1000 search results' } + } + }, + _meta: { runtimeId: 'remote-runtime' } + }) + const store = createTestStore() + store.setState({ + settings: { activeRuntimeEnvironmentId: 'env-1' }, + repos: [{ id: 'runtime-repo-id', path: '/server/repo', name: 'repo', kind: 'git' }] + } as unknown as Partial) + + const result = await store + .getState() + .fetchWorkItemsNextPage([{ repoId: 'caller-repo-id', path: '/server/repo' }], 24, 100, '', 34) + + // The window 422 travels on the envelope error channel, not failedCount — + // resolveEmptyPageOutcome keys on this exact string (#11485). + expect(result).toEqual({ items: [], failedCount: 0, issueErrorTypes: ['validation_error'] }) + }) + it('routes work-item counts through the active runtime environment', async () => { runtimeEnvironmentCall.mockResolvedValueOnce({ id: 'rpc-work-items-count',