From 2fb91690feb92c6b9841816b91d8c4a498c59801 Mon Sep 17 00:00:00 2001 From: "gfargo-horizon-agent[bot]" <294710345+gfargo-horizon-agent[bot]@users.noreply.github.com> Date: Sat, 1 Aug 2026 19:01:08 +0000 Subject: [PATCH 1/2] fix(workstation): surface failed issue/PR detail fetches instead of dropping them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The debounced issue/PR detail hydration effects discarded a failed forge fetch without writing anything to context, so the triage preview pane stayed on the "Loading…" placeholder forever and re-cursoring the row never retried (the effect's cache-skip guard only checked the success cache). Cache the failure message alongside the detail (mirroring the #1633 list-loader treatment) so the preview pane can render it, and clear the cached error on mutation invalidation so a resolved transient failure retries. --- .../chrome/previewPane.triage.test.ts | 26 ++++++++++++ src/workstation/chrome/previewPane.ts | 16 +++++++- .../runtime/hooks/useDetailHydration.ts | 41 +++++++++++++++++-- .../hooks/useForgeTriageWorkflowActions.ts | 22 ++++++++++ src/workstation/runtime/types.ts | 16 ++++++++ src/workstation/surfaces/detail/index.ts | 10 ++++- 6 files changed, 123 insertions(+), 8 deletions(-) diff --git a/src/workstation/chrome/previewPane.triage.test.ts b/src/workstation/chrome/previewPane.triage.test.ts index a692a113..fb3203b6 100644 --- a/src/workstation/chrome/previewPane.triage.test.ts +++ b/src/workstation/chrome/previewPane.triage.test.ts @@ -209,6 +209,32 @@ describe('hydrated triage preview sections (inspector hydration follow-up)', () expect(lines.some((l) => l.toLowerCase().includes('loading'))).toBe(true) }) + it('issue preview shows the failure message instead of the loading hint when hydration failed', () => { + const lines = formatIssueTriagePreview(issue, undefined, 'HTTP 401: Bad credentials').map(stripLine) + expect(lines.some((l) => l.includes('Failed to load details: HTTP 401: Bad credentials'))).toBe(true) + expect(lines.some((l) => l.toLowerCase().includes('loading'))).toBe(false) + }) + + it('issue preview ignores a cached error once detail is present', () => { + const detail = { number: 882, body: 'Body text.', comments: [] } + const lines = formatIssueTriagePreview(issue, detail, 'stale error').map(stripLine) + expect(lines.some((l) => l.includes('stale error'))).toBe(false) + expect(lines).toContain('Body text.') + }) + + it('PR preview shows the failure message instead of the loading hint when hydration failed', () => { + const lines = formatPullRequestTriagePreview(pr, undefined, 'pull request', 'rate limited').map(stripLine) + expect(lines.some((l) => l.includes('Failed to load details: rate limited'))).toBe(true) + expect(lines.some((l) => l.toLowerCase().includes('loading'))).toBe(false) + }) + + it('PR preview ignores a cached error once detail is present', () => { + const detail = { number: 962, body: 'Body text.', comments: [], reviews: [], statusCheckRollup: [] } + const lines = formatPullRequestTriagePreview(pr, detail, 'pull request', 'stale error').map(stripLine) + expect(lines.some((l) => l.includes('stale error'))).toBe(false) + expect(lines).toContain('Body text.') + }) + it('PR preview renders body + checks + reviews + comments when detail is provided', () => { const detail = { number: 962, diff --git a/src/workstation/chrome/previewPane.ts b/src/workstation/chrome/previewPane.ts index 19df8a17..907d055c 100644 --- a/src/workstation/chrome/previewPane.ts +++ b/src/workstation/chrome/previewPane.ts @@ -251,7 +251,8 @@ function statusChecksSection( */ export function formatIssueTriagePreview( issue: IssueListItem | undefined, - detail?: IssueDetail + detail?: IssueDetail, + error?: string ): PreviewLine[] { if (!issue) { return [dim('Select an issue to preview.')] @@ -292,6 +293,11 @@ export function formatIssueTriagePreview( out.push(blank()) out.push(...comments) } + } else if (error) { + // #OSS-1770 — the debounced hydration effect failed; show the + // forge's message instead of a permanent "Loading…" placeholder. + out.push(blank()) + out.push(dim(`⚠ Failed to load details: ${error}`)) } else if (typeof issue.comments === 'number' && issue.comments > 0) { // Pre-hydration affordance — tell the user the body / comments // section is coming, so a 250ms wait doesn't look like a bug. @@ -313,7 +319,8 @@ export function formatIssueTriagePreview( export function formatPullRequestTriagePreview( pr: PullRequestListItem | undefined, detail?: PullRequestDetail, - nounLower = 'pull request' + nounLower = 'pull request', + error?: string ): PreviewLine[] { if (!pr) { return [dim(`Select a ${nounLower} to preview.`)] @@ -370,6 +377,11 @@ export function formatPullRequestTriagePreview( out.push(blank()) out.push(...comments) } + } else if (error) { + // #OSS-1770 — the debounced hydration effect failed; show the + // forge's message instead of a permanent "Loading…" placeholder. + out.push(blank()) + out.push(dim(`⚠ Failed to load details: ${error}`)) } else { // Pre-hydration affordance — same as the issue preview. out.push(blank()) diff --git a/src/workstation/runtime/hooks/useDetailHydration.ts b/src/workstation/runtime/hooks/useDetailHydration.ts index 257c267f..ece2468e 100644 --- a/src/workstation/runtime/hooks/useDetailHydration.ts +++ b/src/workstation/runtime/hooks/useDetailHydration.ts @@ -190,13 +190,29 @@ export function useDetailHydration( Math.min(state.selectedIssueIndex, Math.max(0, filteredIssueList.length - 1)) ] if (!cursored) return - if (context.issueDetailByNumber?.has(cursored.number)) return + if ( + context.issueDetailByNumber?.has(cursored.number) || + context.issueDetailErrorByNumber?.has(cursored.number) + ) return const issuedAtDepth = runtimes.length - 1 let active = true const timer = setTimeout(async () => { const result = await forge.getIssueDetail(cursored.number) - if (!active || !result.ok) return + if (!active) return + if (!result.ok) { + setContext( + (current) => ({ + ...current, + issueDetailErrorByNumber: new Map(current.issueDetailErrorByNumber || []).set( + cursored.number, + result.message + ), + }), + issuedAtDepth, + ) + return + } setContext( (current) => ({ ...current, @@ -219,6 +235,7 @@ export function useDetailHydration( state.selectedIssueIndex, filteredIssueList, context.issueDetailByNumber, + context.issueDetailErrorByNumber, setContext, ]) @@ -236,13 +253,28 @@ export function useDetailHydration( ) ] if (!cursored) return - if (context.pullRequestDetailByNumber?.has(cursored.number)) return + if ( + context.pullRequestDetailByNumber?.has(cursored.number) || + context.pullRequestDetailErrorByNumber?.has(cursored.number) + ) return const issuedAtDepth = runtimes.length - 1 let active = true const timer = setTimeout(async () => { const result = await forge.getPullRequestDetail(cursored.number) - if (!active || !result.ok) return + if (!active) return + if (!result.ok) { + setContext( + (current) => ({ + ...current, + pullRequestDetailErrorByNumber: new Map( + current.pullRequestDetailErrorByNumber || [] + ).set(cursored.number, result.message), + }), + issuedAtDepth, + ) + return + } setContext( (current) => ({ ...current, @@ -268,6 +300,7 @@ export function useDetailHydration( state.selectedPullRequestTriageIndex, filteredPullRequestTriageList, context.pullRequestDetailByNumber, + context.pullRequestDetailErrorByNumber, setContext, ]) diff --git a/src/workstation/runtime/hooks/useForgeTriageWorkflowActions.ts b/src/workstation/runtime/hooks/useForgeTriageWorkflowActions.ts index 9b9950c5..5b1ba681 100644 --- a/src/workstation/runtime/hooks/useForgeTriageWorkflowActions.ts +++ b/src/workstation/runtime/hooks/useForgeTriageWorkflowActions.ts @@ -89,6 +89,17 @@ export function createForgeTriageWorkflowHandlers( next.issueDetailByNumber = undefined } } + // #OSS-1770 — clear any cached failure alongside the detail entry + // so a post-mutation re-hydration isn't blocked by a stale error. + if (current.issueDetailErrorByNumber) { + if (typeof issueNumber === 'number') { + const trimmed = new Map(current.issueDetailErrorByNumber) + trimmed.delete(issueNumber) + next.issueDetailErrorByNumber = trimmed + } else { + next.issueDetailErrorByNumber = undefined + } + } return next }, issuedAtDepth) setContextStatus( @@ -109,6 +120,17 @@ export function createForgeTriageWorkflowHandlers( next.pullRequestDetailByNumber = undefined } } + // #OSS-1770 — clear any cached failure alongside the detail entry + // so a post-mutation re-hydration isn't blocked by a stale error. + if (current.pullRequestDetailErrorByNumber) { + if (typeof pullRequestNumber === 'number') { + const trimmed = new Map(current.pullRequestDetailErrorByNumber) + trimmed.delete(pullRequestNumber) + next.pullRequestDetailErrorByNumber = trimmed + } else { + next.pullRequestDetailErrorByNumber = undefined + } + } return next }, issuedAtDepth) setContextStatus( diff --git a/src/workstation/runtime/types.ts b/src/workstation/runtime/types.ts index 4a08b636..16628c23 100644 --- a/src/workstation/runtime/types.ts +++ b/src/workstation/runtime/types.ts @@ -123,12 +123,28 @@ export type LogInkContext = { * list snappy. */ issueDetailByNumber?: Map + /** + * Per-issue detail *failure* cache keyed by issue number (OSS-1770). + * Set when `forge.getIssueDetail` resolves `{ ok: false, message }` so + * the triage preview pane can render the forge's message instead of a + * permanent "Loading…" placeholder, and so the hydration effect's + * cache-skip guard stops silently re-issuing the same failing fetch + * every time the row is re-cursored. Cleared by the mutation + * invalidation path so a resolved transient failure (e.g. rate limit) + * retries on the next hydration. + */ + issueDetailErrorByNumber?: Map /** * Per-PR detail cache keyed by pull-request number (#882 * inspector hydration). Mirrors `issueDetailByNumber` — fetched * via `gh pr view <#>` and cached per session. */ pullRequestDetailByNumber?: Map + /** + * Per-PR detail *failure* cache keyed by pull-request number + * (OSS-1770). Mirrors `issueDetailErrorByNumber`. + */ + pullRequestDetailErrorByNumber?: Map reflog?: ReflogOverview /** * Remote overview (#0.71). Carries every configured remote's name + diff --git a/src/workstation/surfaces/detail/index.ts b/src/workstation/surfaces/detail/index.ts index 79e86b60..fadc1e49 100644 --- a/src/workstation/surfaces/detail/index.ts +++ b/src/workstation/surfaces/detail/index.ts @@ -1121,8 +1121,11 @@ export function renderIssueTriagePreviewPanel( const detail = issue ? context.issueDetailByNumber?.get(issue.number) : undefined + const detailError = issue + ? context.issueDetailErrorByNumber?.get(issue.number) + : undefined return renderPreviewPanel(h, { Box, Text }, 'Issue preview', - formatIssueTriagePreview(issue, detail), width, theme, focused) + formatIssueTriagePreview(issue, detail, detailError), width, theme, focused) } /** @@ -1168,6 +1171,9 @@ export function renderPullRequestTriagePreviewPanel( const detail = pr ? context.pullRequestDetailByNumber?.get(pr.number) : undefined + const detailError = pr + ? context.pullRequestDetailErrorByNumber?.get(pr.number) + : undefined return renderPreviewPanel(h, { Box, Text }, `${nouns.singular} preview`, - formatPullRequestTriagePreview(pr, detail, nouns.singularLower), width, theme, focused) + formatPullRequestTriagePreview(pr, detail, nouns.singularLower, detailError), width, theme, focused) } From 0e35107eff9ff434819d49fe3d6978ab5911a5bd Mon Sep 17 00:00:00 2001 From: "gfargo-horizon-agent[bot]" <294710345+gfargo-horizon-agent[bot]@users.noreply.github.com> Date: Sun, 2 Aug 2026 08:44:36 +0000 Subject: [PATCH 2/2] =?UTF-8?q?fix(workstation):=20drop=20hardcoded=20?= =?UTF-8?q?=E2=9A=A0=20glyph=20from=20detail-fetch=20error=20line?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reviewer flagged that the emoji bypasses the theme.ascii fallback convention used elsewhere in this surface, and formatIssueTriagePreview/ formatPullRequestTriagePreview don't receive theme to guard it. Simplest fix is to drop the glyph — the "Failed to load details:" prefix already makes the line self-explanatory. Co-Authored-By: Claude Sonnet 5 --- src/workstation/chrome/previewPane.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/workstation/chrome/previewPane.ts b/src/workstation/chrome/previewPane.ts index 907d055c..c03ea633 100644 --- a/src/workstation/chrome/previewPane.ts +++ b/src/workstation/chrome/previewPane.ts @@ -297,7 +297,7 @@ export function formatIssueTriagePreview( // #OSS-1770 — the debounced hydration effect failed; show the // forge's message instead of a permanent "Loading…" placeholder. out.push(blank()) - out.push(dim(`⚠ Failed to load details: ${error}`)) + out.push(dim(`Failed to load details: ${error}`)) } else if (typeof issue.comments === 'number' && issue.comments > 0) { // Pre-hydration affordance — tell the user the body / comments // section is coming, so a 250ms wait doesn't look like a bug. @@ -381,7 +381,7 @@ export function formatPullRequestTriagePreview( // #OSS-1770 — the debounced hydration effect failed; show the // forge's message instead of a permanent "Loading…" placeholder. out.push(blank()) - out.push(dim(`⚠ Failed to load details: ${error}`)) + out.push(dim(`Failed to load details: ${error}`)) } else { // Pre-hydration affordance — same as the issue preview. out.push(blank())