From 6fbfeeb2e4b08463cf2fd01260d07d4765bc85d9 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Thu, 9 Jul 2026 18:47:07 -0700 Subject: [PATCH] Resolve PR repo probe without active branch to avoid forever spinner Previously, checking if a repository is a GitHub remote required an active branch. In a detached HEAD or mid-rebase state (where the branch is null), the probe never resolved, leaving the PR panel on a forever spinner. Decouple the repository probe from the branch presence so the panel can correctly display the "Current branch unavailable" state. Also, hide the PR status chip when no branch is active to avoid a spinner on the chip. --- ...mobile-pr-sidebar-controller-probe.test.ts | 100 ++++++++++++++++++ .../use-mobile-pr-sidebar-controller.ts | 10 +- .../MobileSourceControlPanel.tsx | 7 +- 3 files changed, 112 insertions(+), 5 deletions(-) create mode 100644 mobile/src/session/use-mobile-pr-sidebar-controller-probe.test.ts diff --git a/mobile/src/session/use-mobile-pr-sidebar-controller-probe.test.ts b/mobile/src/session/use-mobile-pr-sidebar-controller-probe.test.ts new file mode 100644 index 00000000000..0645b6659c4 --- /dev/null +++ b/mobile/src/session/use-mobile-pr-sidebar-controller-probe.test.ts @@ -0,0 +1,100 @@ +import { createElement } from 'react' +import { act, create, type ReactTestRenderer } from 'react-test-renderer' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { RpcClient } from '../transport/rpc-client' + +const fetchGithubRepoSlugMock = vi.fn() + +// Only the repo probe matters here; the load path never runs without a branch. +vi.mock('./github-pr-rpc', () => ({ + fetchGithubRepoSlug: (...args: unknown[]) => fetchGithubRepoSlugMock(...args), + fetchHostedReviewForBranch: vi.fn(), + fetchPRChecks: vi.fn(), + fetchPRForBranch: vi.fn(), + fetchWorkItemDetails: vi.fn() +})) +vi.mock('../source-control/mobile-pr-link', () => ({ + fetchWorktreeLinkedPR: vi.fn(async () => null) +})) + +import { + useMobilePrSidebarController, + type MobilePrSidebarController +} from './use-mobile-pr-sidebar-controller' + +let captured: MobilePrSidebarController | null = null + +function Harness(props: Parameters[0]) { + captured = useMobilePrSidebarController(props) + return null +} + +async function flush(): Promise { + // Two ticks: the probe's resolved promise (.then) plus its setState commit. + await act(async () => { + await Promise.resolve() + await Promise.resolve() + }) +} + +describe('useMobilePrSidebarController repo probe', () => { + const client = { sendRequest: vi.fn() } as unknown as RpcClient + let renderer: ReactTestRenderer | null = null + + beforeEach(() => { + captured = null + fetchGithubRepoSlugMock.mockReset() + }) + + afterEach(() => { + act(() => { + renderer?.unmount() + }) + renderer = null + }) + + // Regression: repo eligibility must not be gated on a branch. A detached HEAD / + // mid-rebase worktree (branch === null) still has to resolve the probe, or the + // Pull Request segment gets stranded on a forever spinner instead of the + // "Current branch unavailable" state. + it('resolves the probe when connected without a branch (detached HEAD)', async () => { + fetchGithubRepoSlugMock.mockResolvedValue({ ok: true, result: { owner: 'o', repo: 'r' } }) + await act(async () => { + renderer = create( + createElement(Harness, { + client, + connState: 'connected', + worktreeId: 'w', + branch: null, + headSha: 'sha' + }) + ) + }) + await flush() + + expect(fetchGithubRepoSlugMock).toHaveBeenCalledWith(client, 'w') + expect(captured?.prSidebarRepoProbeLoaded).toBe(true) + expect(captured?.prSidebarIsGithubRepo).toBe(true) + // No branch means no PR load ran — state stays hidden, never a spinner. + expect(captured?.prSidebarState.kind).toBe('hidden') + }) + + it('does not probe until the client is connected', async () => { + fetchGithubRepoSlugMock.mockResolvedValue({ ok: true, result: { owner: 'o', repo: 'r' } }) + await act(async () => { + renderer = create( + createElement(Harness, { + client, + connState: 'connecting', + worktreeId: 'w', + branch: null, + headSha: 'sha' + }) + ) + }) + await flush() + + expect(fetchGithubRepoSlugMock).not.toHaveBeenCalled() + expect(captured?.prSidebarRepoProbeLoaded).toBe(false) + }) +}) diff --git a/mobile/src/session/use-mobile-pr-sidebar-controller.ts b/mobile/src/session/use-mobile-pr-sidebar-controller.ts index f59242a0583..415a7581b7d 100644 --- a/mobile/src/session/use-mobile-pr-sidebar-controller.ts +++ b/mobile/src/session/use-mobile-pr-sidebar-controller.ts @@ -70,7 +70,11 @@ export function useMobilePrSidebarController(input: PrSidebarControllerInput) { stateRef.current = state const headShaRef = useRef(headSha) - const ready = client !== null && connState === 'connected' && !!branch + // Repo eligibility (a GitHub remote) is independent of the branch, so the probe + // must not require one: a detached HEAD / mid-rebase worktree (branch === null) + // would otherwise never set repoProbeLoaded, stranding the PR segment on a + // forever spinner instead of the "Current branch unavailable" state. + const probeReady = client !== null && connState === 'connected' const identity = buildMobilePrSidebarIdentity({ worktreeId, branch }) const buildDeps = useCallback((): PrSidebarLoadDeps | null => { @@ -96,7 +100,7 @@ export function useMobilePrSidebarController(input: PrSidebarControllerInput) { useEffect(() => { let cancelled = false - if (!ready || !client) { + if (!probeReady || !client) { return } void fetchGithubRepoSlug(client, worktreeId).then((outcome) => { @@ -108,7 +112,7 @@ export function useMobilePrSidebarController(input: PrSidebarControllerInput) { return () => { cancelled = true } - }, [ready, client, worktreeId]) + }, [probeReady, client, worktreeId]) useEffect(() => { if (!identity) { diff --git a/mobile/src/source-control/MobileSourceControlPanel.tsx b/mobile/src/source-control/MobileSourceControlPanel.tsx index 6936041a44a..332f62312f1 100644 --- a/mobile/src/source-control/MobileSourceControlPanel.tsx +++ b/mobile/src/source-control/MobileSourceControlPanel.tsx @@ -200,7 +200,10 @@ export function MobileSourceControlPanel({ }, [activeTab, isHostedRepo, prBranch, prSidebarKind, prDetailsMissingFor]) const prChip = useMemo(() => { - if (!isHostedRepo) { + // No branch (detached HEAD / mid-rebase) never runs a PR load, so the shared + // state stays 'hidden' — which the chip would render as a forever spinner. + // Hide the chip instead; the Pull Request segment shows "branch unavailable". + if (!isHostedRepo || !prBranch) { return null } const commentCount = @@ -208,7 +211,7 @@ export function MobileSourceControlPanel({ ? countUnresolvedReviewThreads(prController.prSidebarState.data.details?.comments) : null return buildMobilePrChipSummary(prController.prSidebarState, commentCount) - }, [isHostedRepo, prController.prSidebarState]) + }, [isHostedRepo, prBranch, prController.prSidebarState]) // Design: refresh the active segment's body work, plus git.status for the shared // branch card (counts/sync stay honest even while on History). Preserve ready