From 4b3ae52c4153f8dcc5fb9ae06acae0ca0b0c8142 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= Date: Tue, 8 Sep 2026 00:28:48 +0200 Subject: [PATCH 1/2] chore: UX: In the permanent branch-load failure state, the recovery gui (kwf bring-mobile-gitlab-and-bitb-3792/ux1) --- ...epository-branch-selector.mounted.test.tsx | 24 ++ .../agents/repository-branch-selector.tsx | 21 +- .../pr-review-merge-screen.mounted.test.tsx | 216 ++++++++++++++++++ .../pr-review/pr-review-merge-screen.tsx | 32 ++- 4 files changed, 279 insertions(+), 14 deletions(-) create mode 100644 apps/mobile/src/components/pr-review/pr-review-merge-screen.mounted.test.tsx diff --git a/apps/mobile/src/components/agents/repository-branch-selector.mounted.test.tsx b/apps/mobile/src/components/agents/repository-branch-selector.mounted.test.tsx index ebc429363e..65e408f275 100644 --- a/apps/mobile/src/components/agents/repository-branch-selector.mounted.test.tsx +++ b/apps/mobile/src/components/agents/repository-branch-selector.mounted.test.tsx @@ -210,6 +210,30 @@ describe('RepositoryBranchSelector', () => { expect(renderer.root.findAllByType('Button' as never)).toHaveLength(0); }); + it('sizes a note row to its full message instead of clipping it', () => { + // The permanent-failure copy runs past two lines at phone width, so the + // row must grow with the text: no line cap, and a minimum height rather + // than the fixed trigger height. + const renderer = mountSelector(githubRow, branchesState({ isPermanentError: true })); + const message = i18n.t('agentChat.newSession.branchUnavailable'); + + const note = renderer.root.findAll( + node => node.type === ('Text' as never) && node.children.includes(message) + )[0]; + if (note === undefined) { + throw new Error('the permanent-failure note did not render'); + } + expect(note.props.numberOfLines).toBeUndefined(); + + const row = note.parent; + if (row === null) { + throw new Error('the permanent-failure note rendered without a row'); + } + const classes = (row.props.className as string).split(' '); + expect(classes).toContain('min-h-12'); + expect(classes).not.toContain('h-12'); + }); + it('explains the organizations-only restriction for a personal Bitbucket row', () => { const renderer = mountSelector( bitbucketRow, diff --git a/apps/mobile/src/components/agents/repository-branch-selector.tsx b/apps/mobile/src/components/agents/repository-branch-selector.tsx index c697b46262..6bed318e26 100644 --- a/apps/mobile/src/components/agents/repository-branch-selector.tsx +++ b/apps/mobile/src/components/agents/repository-branch-selector.tsx @@ -26,14 +26,22 @@ type RepositoryBranchSelectorProps = { /** One height for every state, so the row never moves as branches arrive. */ const ROW_HEIGHT = 'h-12'; +/** + * Note rows carry full sentences, so they start at the trigger height and + * grow with their text instead of clipping it — the same guidance must fit + * in every language, not just English. + */ +const NOTE_MIN_HEIGHT = 'min-h-12'; + /** * Branch row under the repository selector. The provider's default branch is * preselected and marked; picking another one records a checkout override for * exactly this repository (see `setSelectedBranchOverride`), which * `useNewSessionCreator` sends as `upstreamBranch`. * - * Every state renders at the same height as the trigger row, so the section - * below never jumps between loading, branches, an error, and an empty list. + * The interactive states render at the same height as the trigger row, so the + * section below never jumps between loading, branches, and a retryable error. + * A note row is at least that tall and grows to show its whole message. */ export function RepositoryBranchSelector({ repository, @@ -120,11 +128,12 @@ export function RepositoryBranchSelector({ function renderNote(message: string) { return ( - - {message} - + {message} ); } diff --git a/apps/mobile/src/components/pr-review/pr-review-merge-screen.mounted.test.tsx b/apps/mobile/src/components/pr-review/pr-review-merge-screen.mounted.test.tsx new file mode 100644 index 0000000000..1f9a51524f --- /dev/null +++ b/apps/mobile/src/components/pr-review/pr-review-merge-screen.mounted.test.tsx @@ -0,0 +1,216 @@ +// The merge screen gates its sheet on the provider reads' SUCCESS (ux2): an +// errored `providerReview.getMergeState` must not mount the GitLab sheet +// without its restrictions list, and an errored `providerReview.getCapabilities` +// must not mount the Bitbucket auto-merge sheet without its capability banner. +// The failure body's Retry refetches the failed provider reads alongside the +// overview. Mounted with a keyed `useQuery` mock so each read can settle into a +// different state than its siblings. + +import type * as ReactQuery from '@tanstack/react-query'; +import { type ReactNode, createElement } from 'react'; +import { type ReactTestRenderer } from 'react-test-renderer'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import '@/i18n'; +import { PrReviewMergeScreen } from './pr-review-merge-screen'; +import { type ProviderPrScope, ProviderPrScopeProvider } from '@/lib/pr-review/provider-pr-ref'; +import { renderWithProviders } from '@/test/render-with-providers'; + +type MockQueryResult = { + data: unknown; + isLoading: boolean; + isPending: boolean; + isSuccess: boolean; + isError: boolean; + isFetching: boolean; + refetch: () => Promise; +}; + +type ResultKey = 'overview' | 'mergeState' | 'capabilities'; + +const mock = vi.hoisted(() => ({ + results: {} as Record, + params: {} as Record, + scope: null as ProviderPrScope | null, +})); + +function mockResult(overrides: Partial = {}): MockQueryResult { + return { + data: undefined, + isLoading: false, + isPending: false, + isSuccess: true, + isError: false, + isFetching: false, + refetch: vi.fn(async () => undefined), + ...overrides, + }; +} + +vi.mock('@tanstack/react-query', async importOriginal => ({ + ...(await importOriginal()), + useQuery: (options: { queryKey: readonly unknown[] }) => + mock.results[String(options.queryKey[0]) as ResultKey], +})); +vi.mock('expo-router', () => ({ + useRouter: () => ({ back: vi.fn(), push: vi.fn() }), + useLocalSearchParams: () => mock.params, +})); +vi.mock('react-native', () => ({ + View: 'View', + ActivityIndicator: 'ActivityIndicator', +})); +vi.mock('@/components/centered-state', () => ({ CenteredState: 'CenteredState' })); +vi.mock('@/components/query-error', () => ({ QueryError: 'QueryError' })); +vi.mock('@/components/pr-review/pr-form-sheet-chrome', () => ({ + PrFormSheetHeader: 'PrFormSheetHeader', +})); +vi.mock('@/components/pr-review/merge/pr-merge-sheet', () => ({ + PrMergeSheet: 'PrMergeSheet', + providerPrNounKey: (platform: string) => + platform === 'gitlab' ? 'common.mergeRequest' : 'common.pullRequest', +})); +vi.mock('@/lib/hooks/use-theme-colors', () => ({ useThemeColors: () => ({}) })); +vi.mock('@/lib/trpc', () => ({ + trpcClient: {}, + useTRPC: () => ({ + githubPrReview: { getPullRequest: { queryOptions: () => ({ queryKey: ['overview'] }) } }, + providerReview: { + getPullRequest: { queryOptions: () => ({ queryKey: ['overview'] }) }, + getMergeState: { queryOptions: () => ({ queryKey: ['mergeState'] }) }, + getCapabilities: { queryOptions: () => ({ queryKey: ['capabilities'] }) }, + }, + }), +})); + +function ScopeWrapper({ children }: Readonly<{ children: ReactNode }>) { + return createElement(ProviderPrScopeProvider, { value: mock.scope!, children }); +} + +const gitlabMergeState = { + canMerge: false, + approvalsRequired: 2, + pipelineMustSucceed: true, + conflicts: false, + blockedReasons: [{ code: 'approvals', message: '2 approvals required' }], +}; + +const overviewData = { + headSha: 'abc123', + headRef: 'feature', + isCrossRepo: false, + prNodeId: 'gitlab:group/repo!12', + title: 'Ship it', + bodyMarkdown: '', + baseRef: 'main', + repo: { allowMergeCommit: true, allowSquashMerge: true, allowRebaseMerge: false }, +}; + +const autoMergeUnsupported = { supported: false, reason: 'Workspace has no auto-merge' }; + +function gitlabMergeParams() { + mock.params = { platform: 'gitlab', identity: ['group', 'repo', '12'] }; + mock.scope = { + ref: { platform: 'gitlab', projectPath: 'group/repo', mrIid: 12 }, + organizationId: null, + }; +} + +function bitbucketAutoMergeParams() { + mock.params = { platform: 'bitbucket', identity: ['ws', 'repo', '7'], mode: 'enable-auto-merge' }; + mock.scope = { + ref: { platform: 'bitbucket', workspace: 'ws', repoSlug: 'repo', prId: 7 }, + organizationId: 'org-1', + }; +} + +async function renderScreen() { + return renderWithProviders(createElement(PrReviewMergeScreen), { wrapper: ScopeWrapper }); +} + +function findSheet(renderer: ReactTestRenderer) { + return renderer.root.findAll(node => String(node.type) === 'PrMergeSheet'); +} + +function findError(renderer: ReactTestRenderer) { + return renderer.root.findAll(node => String(node.type) === 'QueryError'); +} + +beforeEach(() => { + vi.clearAllMocks(); + mock.results = { + overview: mockResult({ data: overviewData }), + mergeState: mockResult({ data: gitlabMergeState }), + capabilities: mockResult({ data: { autoMerge: autoMergeUnsupported } }), + }; +}); + +describe('PrReviewMergeScreen provider-read gating', () => { + it('keeps the GitLab sheet unmounted when getMergeState fails while the overview succeeds', async () => { + gitlabMergeParams(); + mock.results.mergeState = mockResult({ data: undefined, isSuccess: false, isError: true }); + const { renderer, unmount } = await renderScreen(); + expect(findSheet(renderer)).toHaveLength(0); + expect(findError(renderer)).toHaveLength(1); + unmount(); + }); + + it('refetches the failed merge-state read alongside the overview on Retry', async () => { + gitlabMergeParams(); + mock.results.mergeState = mockResult({ data: undefined, isSuccess: false, isError: true }); + const { renderer, unmount } = await renderScreen(); + const error = findError(renderer)[0]!; + (error.props.onRetry as () => void)(); + await vi.waitFor(() => { + expect(mock.results.overview.refetch).toHaveBeenCalledOnce(); + expect(mock.results.mergeState.refetch).toHaveBeenCalledOnce(); + }); + expect(mock.results.capabilities.refetch).not.toHaveBeenCalled(); + unmount(); + }); + + it('keeps the Bitbucket auto-merge sheet unmounted when getCapabilities fails', async () => { + bitbucketAutoMergeParams(); + mock.results.mergeState = mockResult({ data: { ...gitlabMergeState, canMerge: true } }); + mock.results.capabilities = mockResult({ data: undefined, isSuccess: false, isError: true }); + const { renderer, unmount } = await renderScreen(); + expect(findSheet(renderer)).toHaveLength(0); + expect(findError(renderer)).toHaveLength(1); + const error = findError(renderer)[0]!; + (error.props.onRetry as () => void)(); + await vi.waitFor(() => { + expect(mock.results.overview.refetch).toHaveBeenCalledOnce(); + expect(mock.results.capabilities.refetch).toHaveBeenCalledOnce(); + }); + // The merge-state read succeeded, so Retry does not refetch it. + expect(mock.results.mergeState.refetch).not.toHaveBeenCalled(); + unmount(); + }); + + it('mounts the sheet with the restrictions list and the capability banner once both reads succeed', async () => { + bitbucketAutoMergeParams(); + mock.results.mergeState = mockResult({ data: { ...gitlabMergeState, canMerge: true } }); + const { renderer, unmount } = await renderScreen(); + const sheets = findSheet(renderer); + expect(sheets).toHaveLength(1); + expect(sheets[0]!.props.mergeState).toEqual({ ...gitlabMergeState, canMerge: true }); + expect(sheets[0]!.props.autoMergeCapability).toEqual(autoMergeUnsupported); + expect(findError(renderer)).toHaveLength(0); + unmount(); + }); + + it('shows one loading body while the merge-state read is in flight, not a sheet without it', async () => { + gitlabMergeParams(); + mock.results.mergeState = mockResult({ + data: undefined, + isSuccess: false, + isPending: true, + isLoading: true, + }); + const { renderer, unmount } = await renderScreen(); + expect(findSheet(renderer)).toHaveLength(0); + expect(findError(renderer)).toHaveLength(0); + expect(renderer.root.findAll(node => String(node.type) === 'CenteredState')).toHaveLength(1); + unmount(); + }); +}); diff --git a/apps/mobile/src/components/pr-review/pr-review-merge-screen.tsx b/apps/mobile/src/components/pr-review/pr-review-merge-screen.tsx index 3352b86fcc..effda73edd 100644 --- a/apps/mobile/src/components/pr-review/pr-review-merge-screen.tsx +++ b/apps/mobile/src/components/pr-review/pr-review-merge-screen.tsx @@ -142,14 +142,16 @@ export function PrReviewMergeScreen() { ); const mergeStateQuery = useQuery(mergeStateOptions); - // The sheet mounts only once its reads settle, so its content never shifts: - // loading → content happens in the screen body, not inside the sheet. + // The sheet mounts only once its reads SUCCEED, so its content never shifts + // and a doomed submit is never offered: loading → content happens in the + // screen body, not inside the sheet, and an errored provider read keeps the + // sheet unmounted rather than losing the restrictions list (getMergeState) + // or the capability banner (getCapabilities). const isProviderArm = scope.ref.platform !== 'github'; - const mergeStateSettled = - !isProviderArm || (!mergeStateQuery.isLoading && !mergeStateQuery.isPending); - const capabilitiesSettled = !needsAutoMergeCapability || !capabilitiesQuery.isPending; + const mergeStateReady = !isProviderArm || mergeStateQuery.isSuccess; + const capabilitiesReady = !needsAutoMergeCapability || capabilitiesQuery.isSuccess; - if (pr.data && mergeStateSettled && capabilitiesSettled) { + if (pr.data && mergeStateReady && capabilitiesReady) { return ( { - void pr.refetch(); + // Retry recovers every read the screen is waiting on: the overview + // and, on the provider arms, the errored merge gate / capability + // read — a Retry that refetched only the overview could never + // clear the failure that kept the sheet from mounting. + void Promise.all([ + pr.refetch(), + ...(isProviderArm && mergeStateQuery.isError ? [mergeStateQuery.refetch()] : []), + ...(needsAutoMergeCapability && capabilitiesQuery.isError + ? [capabilitiesQuery.refetch()] + : []), + ]); }} - isRetrying={pr.isFetching} + isRetrying={ + pr.isFetching || + (isProviderArm && mergeStateQuery.isFetching) || + (needsAutoMergeCapability && capabilitiesQuery.isFetching) + } /> ); From 516ca12f61281a45f09859775a59f170b67985d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= Date: Tue, 8 Sep 2026 01:00:20 +0200 Subject: [PATCH 2/2] chore: UX: When providerReview.getMergeState or getCapabilities fails w (kwf bring-mobile-gitlab-and-bitb-3792/ux2) --- .../pr-review-merge-screen.mounted.test.tsx | 118 ++++++++++++++---- .../pr-review/pr-review-merge-screen.tsx | 35 +++--- 2 files changed, 117 insertions(+), 36 deletions(-) diff --git a/apps/mobile/src/components/pr-review/pr-review-merge-screen.mounted.test.tsx b/apps/mobile/src/components/pr-review/pr-review-merge-screen.mounted.test.tsx index 1f9a51524f..172cdb4dea 100644 --- a/apps/mobile/src/components/pr-review/pr-review-merge-screen.mounted.test.tsx +++ b/apps/mobile/src/components/pr-review/pr-review-merge-screen.mounted.test.tsx @@ -1,14 +1,20 @@ -// The merge screen gates its sheet on the provider reads' SUCCESS (ux2): an +// The merge screen gates its sheet on the provider reads' DATA (ux2): an // errored `providerReview.getMergeState` must not mount the GitLab sheet // without its restrictions list, and an errored `providerReview.getCapabilities` // must not mount the Bitbucket auto-merge sheet without its capability banner. +// The gate is data presence, not `isSuccess`: a foreground refresh whose +// refetch fails retains the last good read, and the open sheet must stay +// mounted on that retained data rather than drop the user's typed message. // The failure body's Retry refetches the failed provider reads alongside the // overview. Mounted with a keyed `useQuery` mock so each read can settle into a // different state than its siblings. +/* eslint-disable typescript-eslint/no-deprecated -- react-test-renderer is the DOM-free renderer used to mount React/RN trees under vitest (node env, no jsdom); see src/components/pr-review/pr-review-submit.test.tsx */ +/* eslint-disable require-await, @typescript-eslint/require-await -- the fake refetch factories settle without await because they resolve immediately */ + import type * as ReactQuery from '@tanstack/react-query'; -import { type ReactNode, createElement } from 'react'; -import { type ReactTestRenderer } from 'react-test-renderer'; +import { type ReactNode } from 'react'; +import { type ReactTestInstance, type ReactTestRenderer } from 'react-test-renderer'; import { beforeEach, describe, expect, it, vi } from 'vitest'; import '@/i18n'; @@ -28,10 +34,16 @@ type MockQueryResult = { type ResultKey = 'overview' | 'mergeState' | 'capabilities'; -const mock = vi.hoisted(() => ({ - results: {} as Record, - params: {} as Record, - scope: null as ProviderPrScope | null, +type MockState = { + results: Partial>; + params: Record; + scope: ProviderPrScope | null; +}; + +const mock = vi.hoisted((): MockState => ({ + results: {}, + params: {}, + scope: null, })); function mockResult(overrides: Partial = {}): MockQueryResult { @@ -47,6 +59,14 @@ function mockResult(overrides: Partial = {}): MockQueryResult { }; } +function resultOf(key: ResultKey): MockQueryResult { + const query = mock.results[key]; + if (query === undefined) { + throw new Error(`the test did not set a mock result for the ${key} read`); + } + return query; +} + vi.mock('@tanstack/react-query', async importOriginal => ({ ...(await importOriginal()), useQuery: (options: { queryKey: readonly unknown[] }) => @@ -84,7 +104,11 @@ vi.mock('@/lib/trpc', () => ({ })); function ScopeWrapper({ children }: Readonly<{ children: ReactNode }>) { - return createElement(ProviderPrScopeProvider, { value: mock.scope!, children }); + const { scope } = mock; + if (scope === null) { + throw new Error('the test did not set a provider scope'); + } + return {children}; } const gitlabMergeState = { @@ -125,17 +149,33 @@ function bitbucketAutoMergeParams() { } async function renderScreen() { - return renderWithProviders(createElement(PrReviewMergeScreen), { wrapper: ScopeWrapper }); + return renderWithProviders(, { wrapper: ScopeWrapper }); } function findSheet(renderer: ReactTestRenderer) { return renderer.root.findAll(node => String(node.type) === 'PrMergeSheet'); } +function oneSheet(renderer: ReactTestRenderer): ReactTestInstance { + const [sheet] = findSheet(renderer); + if (sheet === undefined) { + throw new Error('the merge sheet did not mount'); + } + return sheet; +} + function findError(renderer: ReactTestRenderer) { return renderer.root.findAll(node => String(node.type) === 'QueryError'); } +function oneError(renderer: ReactTestRenderer): ReactTestInstance { + const [error] = findError(renderer); + if (error === undefined) { + throw new Error('the failure body did not render'); + } + return error; +} + beforeEach(() => { vi.clearAllMocks(); mock.results = { @@ -159,13 +199,13 @@ describe('PrReviewMergeScreen provider-read gating', () => { gitlabMergeParams(); mock.results.mergeState = mockResult({ data: undefined, isSuccess: false, isError: true }); const { renderer, unmount } = await renderScreen(); - const error = findError(renderer)[0]!; + const error = oneError(renderer); (error.props.onRetry as () => void)(); await vi.waitFor(() => { - expect(mock.results.overview.refetch).toHaveBeenCalledOnce(); - expect(mock.results.mergeState.refetch).toHaveBeenCalledOnce(); + expect(resultOf('overview').refetch).toHaveBeenCalledOnce(); + expect(resultOf('mergeState').refetch).toHaveBeenCalledOnce(); }); - expect(mock.results.capabilities.refetch).not.toHaveBeenCalled(); + expect(resultOf('capabilities').refetch).not.toHaveBeenCalled(); unmount(); }); @@ -176,14 +216,14 @@ describe('PrReviewMergeScreen provider-read gating', () => { const { renderer, unmount } = await renderScreen(); expect(findSheet(renderer)).toHaveLength(0); expect(findError(renderer)).toHaveLength(1); - const error = findError(renderer)[0]!; + const error = oneError(renderer); (error.props.onRetry as () => void)(); await vi.waitFor(() => { - expect(mock.results.overview.refetch).toHaveBeenCalledOnce(); - expect(mock.results.capabilities.refetch).toHaveBeenCalledOnce(); + expect(resultOf('overview').refetch).toHaveBeenCalledOnce(); + expect(resultOf('capabilities').refetch).toHaveBeenCalledOnce(); }); // The merge-state read succeeded, so Retry does not refetch it. - expect(mock.results.mergeState.refetch).not.toHaveBeenCalled(); + expect(resultOf('mergeState').refetch).not.toHaveBeenCalled(); unmount(); }); @@ -191,10 +231,10 @@ describe('PrReviewMergeScreen provider-read gating', () => { bitbucketAutoMergeParams(); mock.results.mergeState = mockResult({ data: { ...gitlabMergeState, canMerge: true } }); const { renderer, unmount } = await renderScreen(); - const sheets = findSheet(renderer); - expect(sheets).toHaveLength(1); - expect(sheets[0]!.props.mergeState).toEqual({ ...gitlabMergeState, canMerge: true }); - expect(sheets[0]!.props.autoMergeCapability).toEqual(autoMergeUnsupported); + expect(findSheet(renderer)).toHaveLength(1); + const sheet = oneSheet(renderer); + expect(sheet.props.mergeState).toEqual({ ...gitlabMergeState, canMerge: true }); + expect(sheet.props.autoMergeCapability).toEqual(autoMergeUnsupported); expect(findError(renderer)).toHaveLength(0); unmount(); }); @@ -213,4 +253,40 @@ describe('PrReviewMergeScreen provider-read gating', () => { expect(renderer.root.findAll(node => String(node.type) === 'CenteredState')).toHaveLength(1); unmount(); }); + + it('keeps the open GitLab sheet mounted when a refresh refetch of getMergeState fails on retained data', async () => { + // A foreground refresh invalidates the provider reads; the refetch errors + // but the query keeps its last good data. Gating on `isSuccess` would + // unmount the sheet and drop the commit message the user typed. + gitlabMergeParams(); + mock.results.mergeState = mockResult({ + data: gitlabMergeState, + isSuccess: false, + isError: true, + isFetching: true, + }); + const { renderer, unmount } = await renderScreen(); + expect(findSheet(renderer)).toHaveLength(1); + const sheet = oneSheet(renderer); + expect(sheet.props.mergeState).toEqual(gitlabMergeState); + expect(findError(renderer)).toHaveLength(0); + unmount(); + }); + + it('keeps the open Bitbucket auto-merge sheet mounted when a refresh refetch of getCapabilities fails on retained data', async () => { + bitbucketAutoMergeParams(); + mock.results.mergeState = mockResult({ data: { ...gitlabMergeState, canMerge: true } }); + mock.results.capabilities = mockResult({ + data: { autoMerge: autoMergeUnsupported }, + isSuccess: false, + isError: true, + isFetching: true, + }); + const { renderer, unmount } = await renderScreen(); + expect(findSheet(renderer)).toHaveLength(1); + const sheet = oneSheet(renderer); + expect(sheet.props.autoMergeCapability).toEqual(autoMergeUnsupported); + expect(findError(renderer)).toHaveLength(0); + unmount(); + }); }); diff --git a/apps/mobile/src/components/pr-review/pr-review-merge-screen.tsx b/apps/mobile/src/components/pr-review/pr-review-merge-screen.tsx index effda73edd..bc02c2070f 100644 --- a/apps/mobile/src/components/pr-review/pr-review-merge-screen.tsx +++ b/apps/mobile/src/components/pr-review/pr-review-merge-screen.tsx @@ -142,14 +142,18 @@ export function PrReviewMergeScreen() { ); const mergeStateQuery = useQuery(mergeStateOptions); - // The sheet mounts only once its reads SUCCEED, so its content never shifts - // and a doomed submit is never offered: loading → content happens in the - // screen body, not inside the sheet, and an errored provider read keeps the - // sheet unmounted rather than losing the restrictions list (getMergeState) - // or the capability banner (getCapabilities). + // The sheet mounts only once its reads HAVE DATA, so its content never + // shifts and a doomed submit is never offered: loading → content happens in + // the screen body, not inside the sheet, and a first-load failure of a + // provider read (no data yet) keeps the sheet unmounted rather than losing + // the restrictions list (getMergeState) or the capability banner + // (getCapabilities). Data presence, not `isSuccess`: a foreground refresh + // whose refetch fails RETAINS the last good read, and gating on success + // would unmount an open sheet — dropping the commit message the user typed + // — over data the sheet still has. const isProviderArm = scope.ref.platform !== 'github'; - const mergeStateReady = !isProviderArm || mergeStateQuery.isSuccess; - const capabilitiesReady = !needsAutoMergeCapability || capabilitiesQuery.isSuccess; + const mergeStateReady = !isProviderArm || mergeStateQuery.data !== undefined; + const capabilitiesReady = !needsAutoMergeCapability || capabilitiesQuery.data !== undefined; if (pr.data && mergeStateReady && capabilitiesReady) { return ( @@ -197,14 +201,15 @@ export function PrReviewMergeScreen() { // Retry recovers every read the screen is waiting on: the overview // and, on the provider arms, the errored merge gate / capability // read — a Retry that refetched only the overview could never - // clear the failure that kept the sheet from mounting. - void Promise.all([ - pr.refetch(), - ...(isProviderArm && mergeStateQuery.isError ? [mergeStateQuery.refetch()] : []), - ...(needsAutoMergeCapability && capabilitiesQuery.isError - ? [capabilitiesQuery.refetch()] - : []), - ]); + // clear the failure that kept the sheet from mounting. Each + // refetch starts as it is called, so the reads run concurrently. + void pr.refetch(); + if (isProviderArm && mergeStateQuery.isError) { + void mergeStateQuery.refetch(); + } + if (needsAutoMergeCapability && capabilitiesQuery.isError) { + void capabilitiesQuery.refetch(); + } }} isRetrying={ pr.isFetching ||