feat(mobile): add bottom comment CTA and conversation comment composer to PR review Discussion tab

apps/mobile/src/app/(app)/pr-review/[owner]/[repo]/[number]/_layout.tsx

@@ -74,6 +74,7 @@ export default function PrReviewNumberLayout() {
               comment-composer formSheet instead of the PR overview. */}
           <Stack.Screen name="index" />
           <Stack.Screen name="comment-composer" options={sheetOptions} />
+          <Stack.Screen name="conversation-comment" options={sheetOptions} />
           <Stack.Screen name="review-submit" options={sheetOptions} />
           <Stack.Screen name="merge" options={sheetOptions} />
           <Stack.Screen name="file-navigator" options={sheetOptions} />

apps/mobile/src/app/(app)/pr-review/[owner]/[repo]/[number]/conversation-comment.tsx

@@ -0,0 +1,37 @@
+import { type Href, useLocalSearchParams, useRouter } from 'expo-router';
+
+import { InvalidRouteState } from '@/components/invalid-route-state';
+import { PrConversationCommentComposer } from '@/components/pr-review/discussion/pr-conversation-comment-composer';
+import { parseParam, parsePositiveIntParam } from '@/lib/route-params';
+
+type Params = {
+  owner: string;
+  repo: string;
+  number: string;
+};
+
+// Conversation (issue) comment formSheet, pushed by the Discussion tab's
+// bottom CTA bar. The sheet chrome and the PendingReviewProvider hoist live
+// in the `[number]` layout; this route only parses its params.
+export default function PrConversationCommentRoute() {
+  const router = useRouter();
+  const params = useLocalSearchParams<Params>();
+  const owner = parseParam(params.owner);
+  const repo = parseParam(params.repo);
+  const number = parsePositiveIntParam(params.number);
+
+  if (!owner || !repo || number === null) {
+    return <InvalidRouteState backTo={'/(app)/pr-review' as Href} />;
+  }
+
+  return (
+    <PrConversationCommentComposer
+      owner={owner}
+      repo={repo}
+      number={number}
+      onDismiss={() => {
+        router.back();
+      }}
+    />
+  );
+}

apps/mobile/src/components/offline-banner.mounted.test-helpers.tsx

@@ -0,0 +1,86 @@
+/* eslint-disable typescript-eslint/no-deprecated -- react-test-renderer mounts React/RN trees without a DOM */
+// Shared mount harness for offline-banner.mounted.test.tsx. The mutable refs
+// below are reset by that suite's beforeEach/afterEach; keeping them here lets
+// the suite stay under the max-lines budget without duplicating the provider
+// stack. This module is imported by the test file, whose hoisted vi.mock
+// registrations are already in place when these imports evaluate.
+
+import { type ReactElement } from 'react';
+import { onlineManager, QueryClient, QueryClientProvider } from '@tanstack/react-query';
+import { createTRPCClient, httpLink } from '@trpc/client';
+import TestRenderer, { act } from 'react-test-renderer';
+import { vi } from 'vitest';
+
+import { type ConnectivityState, isOnline } from '@/lib/connectivity-online';
+import { TRPCProvider } from '@/lib/trpc';
+import { OfflineBanner } from './offline-banner';
+import { type MobileRouter } from '@kilocode/trpc/mobile';
+
+/** The fetch transport the tRPC client answers with; scripted per test. */
+export const transport = vi.fn<typeof fetch>();
+
+const settingsData = {
+  isEnabled: false,
+  repositorySelectionMode: 'all',
+  selectedRepositoryIds: [],
+  analysisMode: 'auto',
+};
+
+const trpcClient = createTRPCClient<MobileRouter>({
+  links: [httpLink({ url: 'https://settings.test/api/trpc', fetch: transport })],
+});
+
+/** Mutable harness state the owning suite resets around each test. */
+export const harness = {
+  queryClient: new QueryClient(),
+  renderers: [] as TestRenderer.ReactTestRenderer[],
+  sourceListener: undefined as ((value: ConnectivityState) => void) | undefined,
+};
+
+/** Mounts the banner (or any element) under the shared provider stack. */
+export async function mountTree(element: ReactElement = <OfflineBanner />) {
+  await act(() => {
+    harness.renderers.push(
+      TestRenderer.create(
+        <QueryClientProvider client={harness.queryClient}>
+          <TRPCProvider trpcClient={trpcClient} queryClient={harness.queryClient}>
+            {element}
+          </TRPCProvider>
+        </QueryClientProvider>
+      )
+    );
+  });
+  const renderer = harness.renderers.at(-1);
+  if (!renderer) {
+    throw new Error('renderer was not created');
+  }
+  return renderer;
+}
+
+export function findHost(root: TestRenderer.ReactTestInstance, type: string) {
+  return root.findAll(node => node.type === type);
+}
+
+/** Commits a NetInfo report into the online manager and the test source. */
+export function emit(value: ConnectivityState) {
+  act(() => {
+    onlineManager.setOnline(isOnline(value));
+    harness.sourceListener?.(value);
+  });
+}
+
+export async function advanceBy(ms: number) {
+  await act(async () => {
+    await vi.advanceTimersByTimeAsync(ms);
+  });
+}
+
+/** The paused-transport tRPC payload the settings-screen scenario reads. */
+export function settingsResponse(procedure: string): unknown {
+  const data: Record<string, unknown> = {
+    getConfig: settingsData,
+    getRepositories: [{ id: 1, full_name: 'kilo/repo' }],
+    list: [{ organizationId: 'org_123', role: 'owner' }],
+  };
+  return { result: { data: data[procedure] } };
+}

apps/mobile/src/components/offline-banner.mounted.test.tsx

@@ -1,36 +1,30 @@
 /* eslint-disable typescript-eslint/no-deprecated -- react-test-renderer mounts React/RN trees without a DOM */
-import { type ReactElement, useSyncExternalStore } from 'react';
+import { useSyncExternalStore } from 'react';
 import { type MobileRouter } from '@kilocode/trpc/mobile';
-import { onlineManager, QueryClient, QueryClientProvider } from '@tanstack/react-query';
-import { createTRPCClient, httpLink } from '@trpc/client';
-import TestRenderer, { act } from 'react-test-renderer';
+import { onlineManager, QueryClient } from '@tanstack/react-query';
+import { act } from 'react-test-renderer';
 import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
 
 import '@/i18n';
-import { type ConnectivityState, isOnline } from '@/lib/connectivity-online';
+import { type ConnectivityState } from '@/lib/connectivity-online';
 import { createOfflineBannerStore, type OfflineBannerStore } from '@/lib/offline-banner-state';
-import { TRPCProvider } from '@/lib/trpc';
-import { OfflineBanner } from './offline-banner';
+import { OFFLINE_BANNER_HEIGHT } from './offline-banner';
 import { SettingsOverviewScreen } from './security-agent/settings-overview-screen';
+import {
+  advanceBy,
+  emit,
+  findHost,
+  harness,
+  mountTree,
+  settingsResponse,
+  transport,
+} from './offline-banner.mounted.test-helpers';
 
 const state = vi.hoisted(() => ({ store: undefined as OfflineBannerStore | undefined }));
 const announceForA11y = vi.hoisted(() => vi.fn());
-const transport = vi.fn<typeof fetch>();
-const settingsData = {
-  isEnabled: false,
-  repositorySelectionMode: 'all',
-  selectedRepositoryIds: [],
-  analysisMode: 'auto',
-};
-const trpcClient = createTRPCClient<MobileRouter>({
-  links: [httpLink({ url: 'https://settings.test/api/trpc', fetch: transport })],
-});
 const offlineState: ConnectivityState = { isConnected: true, isInternetReachable: false };
 const onlineState: ConnectivityState = { isConnected: true, isInternetReachable: true };
 const probe = vi.fn<() => Promise<boolean>>();
-const renderers: TestRenderer.ReactTestRenderer[] = [];
-let sourceListener: ((value: ConnectivityState) => void) | undefined = undefined;
-let queryClient = new QueryClient();
 let previousOnline = true;
 let responseGate: Promise<undefined> | undefined = undefined;
 
@@ -98,69 +92,30 @@ vi.mock('@/components/tab-screen', () => ({
   useTabBarBottomPadding: () => 0,
 }));
 
-async function mountTree(element: ReactElement = <OfflineBanner />) {
-  await act(() => {
-    renderers.push(
-      TestRenderer.create(
-        <QueryClientProvider client={queryClient}>
-          <TRPCProvider trpcClient={trpcClient} queryClient={queryClient}>
-            {element}
-          </TRPCProvider>
-        </QueryClientProvider>
-      )
-    );
-  });
-  const renderer = renderers.at(-1);
-  if (!renderer) {
-    throw new Error('renderer was not created');
-  }
-  return renderer;
-}
-
-function findHost(root: TestRenderer.ReactTestInstance, type: string) {
-  return root.findAll(node => node.type === type);
-}
-
-function emit(value: ConnectivityState) {
-  act(() => {
-    onlineManager.setOnline(isOnline(value));
-    sourceListener?.(value);
-  });
-}
-
-async function advanceBy(ms: number) {
-  await act(async () => {
-    await vi.advanceTimersByTimeAsync(ms);
-  });
-}
-
 describe('OfflineBanner mounted with confirmed connectivity', () => {
   beforeEach(() => {
     (globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true;
     vi.useFakeTimers();
     previousOnline = onlineManager.isOnline();
     onlineManager.setOnline(false);
-    queryClient = new QueryClient({ defaultOptions: { queries: { retry: 3, gcTime: Infinity } } });
+    harness.queryClient = new QueryClient({
+      defaultOptions: { queries: { retry: 3, gcTime: Infinity } },
+    });
     responseGate = undefined;
     transport.mockReset().mockImplementation(async input => {
       await responseGate;
       const procedure =
         new URL(input instanceof Request ? input.url : input).pathname.split('.').at(-1) ?? '';
-      const data: Record<string, unknown> = {
-        getConfig: settingsData,
-        getRepositories: [{ id: 1, full_name: 'kilo/repo' }],
-        list: [{ organizationId: 'org_123', role: 'owner' }],
-      };
-      return Response.json({ result: { data: data[procedure] } });
+      return Response.json(settingsResponse(procedure));
     });
     probe.mockReset().mockResolvedValue(false);
     announceForA11y.mockClear();
     state.store = createOfflineBannerStore({
       source: {
         subscribe: listener => {
-          sourceListener = listener;
+          harness.sourceListener = listener;
           return () => {
-            sourceListener = undefined;
+            harness.sourceListener = undefined;
           };
         },
       },
@@ -179,11 +134,11 @@ describe('OfflineBanner mounted with confirmed connectivity', () => {
   });
   afterEach(() => {
     act(() => {
-      for (const renderer of renderers.splice(0)) {
+      for (const renderer of harness.renderers.splice(0)) {
         renderer.unmount();
       }
     });
-    queryClient.clear();
+    harness.queryClient.clear();
     onlineManager.setOnline(previousOnline);
     state.store?.destroy();
     state.store = undefined;
@@ -227,6 +182,10 @@ describe('OfflineBanner mounted with confirmed connectivity', () => {
     const alert = findHost(renderer.root, 'Animated.View')[0];
     expect(alert?.props.accessibilityRole).toBe('alert');
     expect(alert?.props.accessibilityLabel).toBe('No internet connection');
+    // The painted row is exactly OFFLINE_BANNER_HEIGHT tall: surfaces reserve
+    // that constant above their pinned headers so the overlay never covers a
+    // title (uxs2 spot check). Keep the height style in sync with the export.
+    expect(alert?.props.style).toEqual({ height: OFFLINE_BANNER_HEIGHT });
     expect(findHost(renderer.root, 'WifiOff')).toHaveLength(1);
     expect(announceForA11y).toHaveBeenCalledExactlyOnceWith('No internet connection');
   });
@@ -274,6 +233,42 @@ describe('OfflineBanner mounted with confirmed connectivity', () => {
     expect(announceForA11y).not.toHaveBeenCalled();
   });
 
+  it('clears a stale banner on radio-back-without-reachability via the immediate app probe', async () => {
+    // e6-after-net (uxs3 spot check): airplane mode → 3G. NetInfo reports the
+    // connection back but its external reachability probe never answers, so
+    // the event is `unknown` — the banner used to stay painted forever. The
+    // committed-offline + radio-up combination now fires the app's own probe
+    // at once; a reachable answer clears the mounted banner without any
+    // further NetInfo event or timer wait.
+    const renderer = await mountTree();
+    emit(offlineState);
+    await advanceBy(5000);
+    expect(findHost(renderer.root, 'Animated.View')).toHaveLength(1);
+
+    probe.mockResolvedValue(true);
+    emit({ isConnected: true, isInternetReachable: null });
+    // No five-second wait: the recovery probe is immediate. advanceBy(0) only
+    // flushes the probe's microtasks under act — no timer fires.
+    await advanceBy(0);
+    expect(renderer.toJSON()).toBeNull();
+    expect(announceForA11y).toHaveBeenCalledWith('Internet connection restored');
+  });
+
+  it('keeps the mounted banner when the immediate radio-up probe fails', async () => {
+    // The same event with a still-unreachable backend must NOT clear the
+    // banner or announce a restoration that did not happen.
+    const renderer = await mountTree();
+    emit(offlineState);
+    await advanceBy(5000);
+    announceForA11y.mockClear();
+
+    probe.mockResolvedValue(false);
+    emit({ isConnected: true, isInternetReachable: null });
+    await advanceBy(0);
+    expect(findHost(renderer.root, 'Animated.View')).toHaveLength(1);
+    expect(announceForA11y).not.toHaveBeenCalled();
+  });
+
   it.each(['personal', 'org_123'])(
     'retrieves complete %s settings after a successful probe without another NetInfo event',
     async scope => {
@@ -282,7 +277,7 @@ describe('OfflineBanner mounted with confirmed connectivity', () => {
       probe.mockResolvedValue(true);
       const banner = await mountTree();
       const screen = await mountTree(<SettingsOverviewScreen scope={scope} />);
-      const activeQueries = queryClient.getQueryCache().findAll({ type: 'active' });
+      const activeQueries = harness.queryClient.getQueryCache().findAll({ type: 'active' });
       expect(activeQueries).toHaveLength(scope === 'personal' ? 2 : 3);
       expect(activeQueries.every(query => query.state.fetchStatus === 'paused')).toBe(true);
       expect(activeQueries.every(query => query.state.data === undefined)).toBe(true);
@@ -296,7 +291,7 @@ describe('OfflineBanner mounted with confirmed connectivity', () => {
       const onRetry = findHost(screen.root, 'Button')[0]?.props.onPress as () => void;
       act(onRetry);
       await advanceBy(10);
-      expect(queryClient.isFetching()).toBe(activeQueries.length);
+      expect(harness.queryClient.isFetching()).toBe(activeQueries.length);
       expect(findHost(screen.root, 'Button')[0]?.props.loading).toBe(true);
       await act(() => {
         response.resolve(undefined);

apps/mobile/src/components/offline-banner.tsx

@@ -10,9 +10,21 @@ import { announceForA11y } from '@/lib/a11y/announce';
 import { useThemeColors } from '@/lib/hooks/use-theme-colors';
 import { useOfflineBannerState } from '@/lib/hooks/use-offline-banner-state';
 
+/**
+ * Fixed height of the banner row. The banner is an absolute overlay pinned at
+ * `top: insets.top`, so a surface whose header starts at the safe-area top
+ * must reserve this height while the banner is visible or the overlay covers
+ * the header title (uxs2 spot check, e6-offline-hang). The banner renders at
+ * exactly this height (no vertical padding) so the constant cannot drift from
+ * the painted row.
+ */
+export const OFFLINE_BANNER_HEIGHT = 36;
+
 /**
  * App-wide offline banner. Absolute overlay, so app content keeps its layout
  * position; `pointerEvents="none"` passes every touch to the header below.
+ * Surfaces with a pinned top header reserve `OFFLINE_BANNER_HEIGHT` above the
+ * header while the banner is visible so it never covers the title.
  */
 export function OfflineBanner() {
   const isOffline = useOfflineBannerState();
@@ -46,7 +58,8 @@ export function OfflineBanner() {
         accessible
         accessibilityRole="alert"
         accessibilityLabel={t('offline.noInternet')}
-        className="flex-row items-center justify-center gap-2 bg-warn px-4 py-2"
+        className="flex-row items-center justify-center gap-2 bg-warn px-4"
+        style={{ height: OFFLINE_BANNER_HEIGHT }}
       >
         <WifiOff size={14} color={colors.warnForeground} />
         <Text className="text-sm font-medium text-warn-foreground">{t('offline.noInternet')}</Text>