From eba96b6e10d88d939d14a1507c98dd351bca8f2c Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Thu, 16 Jul 2026 19:16:36 -0700 Subject: [PATCH] fix(github): preserve Tasks outage attribution --- src/main/github/client-issue-source.test.ts | 16 ++++++++ src/main/github/client.ts | 11 ++++++ src/renderer/src/components/TaskPage.tsx | 6 +-- src/renderer/src/store/slices/github.test.ts | 39 ++++++++++++++++++++ src/renderer/src/store/slices/github.ts | 24 ++++++------ 5 files changed, 81 insertions(+), 15 deletions(-) diff --git a/src/main/github/client-issue-source.test.ts b/src/main/github/client-issue-source.test.ts index df658007fe5..e8132c5b797 100644 --- a/src/main/github/client-issue-source.test.ts +++ b/src/main/github/client-issue-source.test.ts @@ -228,6 +228,22 @@ describe('GitHub issue source split', () => { ) }) + it.each(['is:issue', 'is:pr'])( + 'propagates GitHub outages for scoped %s queries', + async (query) => { + getIssueOwnerRepoMock.mockResolvedValueOnce({ owner: 'stablyai', repo: 'orca' }) + getOwnerRepoMock.mockResolvedValueOnce({ owner: 'fork', repo: 'orca' }) + ghExecFileAsyncMock.mockRejectedValueOnce(new Error('HTTP 503: Service Unavailable')) + + await expect(listWorkItems('/repo-root', 10, query)).rejects.toThrow( + 'HTTP 503: Service Unavailable' + ) + + // The outage signal reuses the failed request; it must not add retry subprocesses. + expect(ghExecFileAsyncMock).toHaveBeenCalledTimes(1) + } + ) + it("uses upstream for recent PRs when preference='upstream'", async () => { resolveIssueSourceMock.mockResolvedValueOnce({ source: { owner: 'stablyai', repo: 'orca' }, diff --git a/src/main/github/client.ts b/src/main/github/client.ts index f4932bc7d3c..aa137a971ea 100644 --- a/src/main/github/client.ts +++ b/src/main/github/client.ts @@ -1288,6 +1288,11 @@ async function listQueriedWorkItems( } } catch (err) { const stderr = err instanceof Error ? err.message : String(err) + if (!prScope && classifyGitHubUnavailable(stderr)) { + // Why: a scoped issue query has no successful side to preserve. Let the + // cross-repo caller attribute the empty result to GitHub instead. + throw err + } return { items: [], issuesError: classifyListIssuesError(stderr) } } })() @@ -1320,6 +1325,12 @@ async function listQueriedWorkItems( return hydrated } catch (err) { console.warn('listQueriedWorkItems PRs partial failure:', err) + const stderr = err instanceof Error ? err.message : String(err) + if (!issueScope && classifyGitHubUnavailable(stderr)) { + // Why: Tasks scopes every query to one work-item kind. Propagate an + // outage instead of converting the only failed source into an empty list. + throw err + } return [] } })() diff --git a/src/renderer/src/components/TaskPage.tsx b/src/renderer/src/components/TaskPage.tsx index 553fd3a0ef4..0762c02b7f1 100644 --- a/src/renderer/src/components/TaskPage.tsx +++ b/src/renderer/src/components/TaskPage.tsx @@ -3788,9 +3788,9 @@ export default function TaskPage(): React.JSX.Element { // rejections populate tasksError instead — the two are mutually exclusive so // a successful-with-partial-failure read and a hard-reject don't double-show. const [failedCount, setFailedCount] = useState(0) - // Why: when the whole list fails because GitHub is unreachable (outage / - // network / rate limit), attribute it to GitHub instead of the vague "N of M - // projects failed to load" — an outage shouldn't read as an Orca bug. + // Why: when every selected refresh fails because GitHub is unreachable + // (outage / network / rate limit), attribute it to GitHub instead of making + // an empty or stale cached list look current. const [githubUnavailable, setGithubUnavailable] = useState(false) const [taskRefreshNonce, setTaskRefreshNonce] = useState(0) // Why: the fetch effect uses this to detect when a nonce bump is from the diff --git a/src/renderer/src/store/slices/github.test.ts b/src/renderer/src/store/slices/github.test.ts index 62ac741f85d..d5c1a4bc507 100644 --- a/src/renderer/src/store/slices/github.test.ts +++ b/src/renderer/src/store/slices/github.test.ts @@ -6322,6 +6322,45 @@ describe('createGitHubSlice.fetchWorkItems source/error envelope', () => { } }) + it('flags githubUnavailable while serving stale cached rows after a failed refresh', async () => { + const store = createTestStore() + const consoleWarn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + const item = { + type: 'pr', + number: 8, + title: 'Cached PR', + url: 'https://example.test/8', + updatedAt: '2026-05-21T00:00:00Z' + } as GitHubWorkItem + mockApi.gh.listWorkItems + .mockResolvedValueOnce({ + items: [item], + sources: { + issues: null, + prs: { owner: 'up', repo: 'r' }, + originCandidate: { owner: 'up', repo: 'r' }, + upstreamCandidate: null + } + }) + .mockRejectedValueOnce(new Error('HTTP 503: Service Unavailable')) + + try { + const repos = [{ repoId: 'github-repo', path: '/server/github-repo' }] + await store.getState().fetchWorkItemsAcrossRepos(repos, 24, 100, '') + + const result = await store + .getState() + .fetchWorkItemsAcrossRepos(repos, 24, 100, '', { force: true }) + + expect(result.items).toEqual([{ ...item, repoId: 'github-repo' }]) + expect(result.failedCount).toBe(0) + expect(result.githubUnavailable).toBe(true) + expect(mockApi.gh.listWorkItems).toHaveBeenCalledTimes(2) + } finally { + consoleWarn.mockRestore() + } + }) + it('ignores an ineligible SSH repo when every GitHub source is unavailable', async () => { const store = createTestStore() const consoleWarn = vi.spyOn(console, 'warn').mockImplementation(() => {}) diff --git a/src/renderer/src/store/slices/github.ts b/src/renderer/src/store/slices/github.ts index 5c39a5e5b23..fd9351dd5ee 100644 --- a/src/renderer/src/store/slices/github.ts +++ b/src/renderer/src/store/slices/github.ts @@ -2037,10 +2037,10 @@ export type GitHubSlice = { * served from stale cache on rejection is NOT counted as failed — matching * the single-repo behavior of quietly serving stale data. * - * `githubUnavailable` is true when every selected GitHub source failed (with - * no cache) because GitHub was unreachable/unavailable (5xx outage, network, - * or rate limit). The caller uses it to attribute the empty list to a global - * GitHub availability failure instead of the generic partial-failure banner. + * `githubUnavailable` is true when every selected GitHub source refresh failed + * because GitHub was unreachable/unavailable (5xx outage, network, or rate + * limit), even when stale cache data remains available. The caller uses it to + * attribute the stale/empty list instead of showing a normal-looking result. */ fetchWorkItemsAcrossRepos: ( repos: { @@ -2865,6 +2865,7 @@ export const createGitHubSlice: StateCreator = (s } const state = get() let failedCount = 0 + let requestFailureCount = 0 let unavailableFailureCount = 0 let skippedSourceCount = 0 const perProjectResults = await Promise.all( @@ -2885,6 +2886,10 @@ export const createGitHubSlice: StateCreator = (s skippedSourceCount += 1 return [] as GitHubWorkItem[] } + requestFailureCount += 1 + if (classifyGitHubUnavailable(err instanceof Error ? err.message : String(err))) { + unavailableFailureCount += 1 + } const key = r.sourceContext?.provider === 'github' ? workItemsCacheKey( @@ -2901,11 +2906,6 @@ export const createGitHubSlice: StateCreator = (s } console.warn(`[workItems] ${r.repoId} failed:`, err) failedCount += 1 - // Why: track reachability separately so the aggregate below can - // distinguish a global GitHub failure from a partial repo failure. - if (classifyGitHubUnavailable(err instanceof Error ? err.message : String(err))) { - unavailableFailureCount += 1 - } return [] as GitHubWorkItem[] } }) @@ -2916,9 +2916,9 @@ export const createGitHubSlice: StateCreator = (s // eligible source failed for a GitHub reachability reason; intentionally // skipped SSH repos are not GitHub sources for this calculation. const githubUnavailable = - failedCount > 0 && - failedCount === repos.length - skippedSourceCount && - unavailableFailureCount === failedCount + requestFailureCount > 0 && + requestFailureCount === repos.length - skippedSourceCount && + unavailableFailureCount === requestFailureCount return { items: merged, failedCount, githubUnavailable } },