diff --git a/apps/mobile/src/components/agents/new-session-configure-form.tsx b/apps/mobile/src/components/agents/new-session-configure-form.tsx index 423ce8e113..528a5ef050 100644 --- a/apps/mobile/src/components/agents/new-session-configure-form.tsx +++ b/apps/mobile/src/components/agents/new-session-configure-form.tsx @@ -244,8 +244,10 @@ export function NewSessionConfigureForm({ ); return ( + // The root reserves the navigation-bar inset, so the keyboard-lift view + // pads from its own bottom edge and must not add the inset again. - + {body} {/* The primary action is pinned below the scroll body, never part of it. diff --git a/apps/mobile/src/components/agents/session-detail-content.tsx b/apps/mobile/src/components/agents/session-detail-content.tsx index 6311270ffb..0836275d6f 100644 --- a/apps/mobile/src/components/agents/session-detail-content.tsx +++ b/apps/mobile/src/components/agents/session-detail-content.tsx @@ -1959,7 +1959,9 @@ export function SessionDetailContent({ {keepScreenAwake ? : null} {keyboardContainerKind === 'app-aware-padding' ? ( - + // The trailing bottom-chrome spacer below reserves the navigation- + // bar inset outside this view, so the view must not add it again. + {renderKeyboardBody()} ) : ( diff --git a/apps/mobile/src/components/app-root-providers.toaster-a11y.mounted.test.tsx b/apps/mobile/src/components/app-root-providers.toaster-a11y.mounted.test.tsx index d82a8d756e..57bb972769 100644 --- a/apps/mobile/src/components/app-root-providers.toaster-a11y.mounted.test.tsx +++ b/apps/mobile/src/components/app-root-providers.toaster-a11y.mounted.test.tsx @@ -130,12 +130,40 @@ it('reads the keyboard height through the shared app-aware hook', async () => { expect(sharedKeyboardHook.calls).toBeGreaterThan(0); }); -it('keeps the toast above the software keyboard while it is up', async () => { +/** + * Android's `endCoordinates.height` stops at the navigation bar + * (`ReactRootView` sends `imeInsets.bottom − barInsets.bottom`), so the raw + * height sits below the IME's true top edge by the bar inset. The toast is + * anchored to the screen bottom, so the Toaster resolves the occlusion through + * the same rule the screens reserve padding with (`resolveKeyboardBottomPadding`) + * — a raw height left the toast's last line behind the IME's navigation row + * (2026-09-20 review finding). The mocked bottom inset is 12. + */ +it('clears the Android IME navigation row by adding the bottom inset', async () => { platform.OS = 'android'; await mount(); act(() => { - keyboard.show?.({ endCoordinates: { height: 300 } }); + keyboard.show({ endCoordinates: { height: 300 } }); + }); + + const toasters = unlockRoot().findAllByType('Toaster' as ElementType); + + expect(toasters[0]?.props.offset).toBe(300 + 12 + TOAST_BOTTOM_GAP); +}); + +/** + * iOS reports the keyboard window frame, which reaches the screen bottom and + * so already includes the home-indicator inset. Adding the bottom inset there + * would float the toast above the keyboard, so the iOS height passes through + * unchanged — the platform-parity half of the keyboard rule. + */ +it('keeps the iOS keyboard height, which already reaches the screen bottom', async () => { + platform.OS = 'ios'; + await mount(); + + act(() => { + keyboard.show({ endCoordinates: { height: 300 } }); }); const toasters = unlockRoot().findAllByType('Toaster' as ElementType); @@ -143,6 +171,40 @@ it('keeps the toast above the software keyboard while it is up', async () => { expect(toasters[0]?.props.offset).toBe(300 + TOAST_BOTTOM_GAP); }); +/** + * The harness keeps every keyboard subscriber, one set per direction (see the + * Keyboard mock in the test helpers). A second consumer — here an extra + * listener standing in for a screen on top of the Toaster — must not shadow the + * Toaster's listener, and disposing it must not detach the Toaster's. A single + * slot per direction failed both halves. + */ +it('delivers a keyboard event to every subscriber and detaches only the disposed one', async () => { + platform.OS = 'android'; + await mount(); + + const extra = vi.fn((_event: { endCoordinates: { height: number } }) => undefined); + const subscription = keyboard.addListener('keyboardDidShow', extra); + + act(() => { + keyboard.show({ endCoordinates: { height: 300 } }); + }); + // Both the Toaster's hook and the extra subscriber received the height. + expect(extra).toHaveBeenCalledWith({ endCoordinates: { height: 300 } }); + expect(unlockRoot().findAllByType('Toaster' as ElementType)[0]?.props.offset).toBe( + 300 + 12 + TOAST_BOTTOM_GAP + ); + + subscription.remove(); + act(() => { + keyboard.show({ endCoordinates: { height: 240 } }); + }); + // The disposed subscriber is gone; the Toaster's listener is still attached. + expect(extra).toHaveBeenCalledTimes(1); + expect(unlockRoot().findAllByType('Toaster' as ElementType)[0]?.props.offset).toBe( + 240 + 12 + TOAST_BOTTOM_GAP + ); +}); + /** * The same rule runs on iOS: the offset module reads no platform, so the * resting offset is the shared bottom-chrome floor plus the standard gap, not diff --git a/apps/mobile/src/components/app-root-providers.tsx b/apps/mobile/src/components/app-root-providers.tsx index 1deac7b354..973ae8e748 100644 --- a/apps/mobile/src/components/app-root-providers.tsx +++ b/apps/mobile/src/components/app-root-providers.tsx @@ -12,6 +12,7 @@ import { useTranslation } from 'react-i18next'; import { AppUnlockAnnouncements } from '@/components/app-unlock-screen'; import { useAppAwareKeyboardPadding } from '@/components/kilo-chat/app-aware-keyboard-padding'; +import { resolveKeyboardBottomPadding } from '@/components/login-screen-state'; import { OfflineBanner } from '@/components/offline-banner'; import { AppUnlockProvider } from '@/lib/app-unlock-context'; import { AuthProvider } from '@/lib/auth/auth-context'; @@ -109,15 +110,29 @@ export function AppRootProviders({ * bottom-anchored overlay has no other way to clear the keyboard and its * navigation row. * - * The resting offset is one platform-free rule (`lib/toast-offset.ts`): iOS and - * Android run the same math, and the only platform value it reads is the tab - * bar's own rendered height, which the bar's helper owns. + * The offset is one platform-free rule (`lib/toast-offset.ts`): iOS and Android + * run the same math, and the platform enters only through the values resolved + * here for it — the tab bar's own rendered height, which the bar's helper owns, + * and the keyboard occlusion's origin (`resolveKeyboardBottomPadding`). */ function AppToaster() { const colors = useThemeColors(); const { bottom } = useSafeAreaInsets(); const { fontScale } = useWindowDimensions(); const keyboardHeight = useAppAwareKeyboardPadding(); + // The hook's height is the platform's own keyboard metric, and the two + // platforms measure it from different origins: Android's stops at the + // navigation bar (`ReactRootView` reports `imeInsets.bottom − barInsets.bottom`), + // while iOS reports the keyboard frame, which reaches the screen bottom. The + // offset is anchored to the screen bottom, so the occlusion is resolved here + // with the same rule the screens reserve padding with + // (`resolveKeyboardBottomPadding`); passing the raw Android height left the + // toast's last line behind the IME's navigation row (2026-09-20 review + // finding). `lib/toast-offset.ts` stays platform-free. + const keyboardOcclusion = + keyboardHeight > 0 + ? resolveKeyboardBottomPadding({ keyboardHeight, bottomInset: bottom, platform: Platform.OS }) + : 0; const segments = useSegments(); const pathname = usePathname(); // The floating tab bar is an absolute overlay over the screen bottom, so it @@ -146,7 +161,7 @@ function AppToaster() { // covered. One platform-free rule; see `lib/toast-offset.ts`. offset={getToastBottomOffset({ safeAreaBottom: bottom, - keyboardHeight, + keyboardHeight: keyboardOcclusion, tabBarHeight, })} positionerStyle={TOAST_POSITIONER_STYLE} diff --git a/apps/mobile/src/components/app-unlock-screen.mounted.test.tsx b/apps/mobile/src/components/app-unlock-screen.mounted.test.tsx index a7bf6a2c31..2c3d8d0b7e 100644 --- a/apps/mobile/src/components/app-unlock-screen.mounted.test.tsx +++ b/apps/mobile/src/components/app-unlock-screen.mounted.test.tsx @@ -91,9 +91,9 @@ it.each([false, true])( native.authenticateAsync.mockResolvedValueOnce({ success: false, error: 'user_cancel' }); const now = vi.spyOn(Date, 'now').mockReturnValue(0); await flush(() => { - lifecycle.change?.('background'); + lifecycle.change('background'); now.mockReturnValue(300_000); - lifecycle.change?.('active'); + lifecycle.change('active'); }); expectHidden(root(), true); await flush(retry()?.props.onPress as () => void); @@ -263,9 +263,9 @@ describe.each(['ios', 'android'])('%s shared unlock announcements', os => { if (locked) { const now = vi.spyOn(Date, 'now').mockReturnValue(0); await flush(() => { - lifecycle.change?.('background'); + lifecycle.change('background'); now.mockReturnValue(300_000); - lifecycle.change?.('active'); + lifecycle.change('active'); }); expect(retry()?.props.disabled).toBe(true); } diff --git a/apps/mobile/src/components/app-unlock-screen.test-helpers.tsx b/apps/mobile/src/components/app-unlock-screen.test-helpers.tsx index 3190f8c11d..cdc085b044 100644 --- a/apps/mobile/src/components/app-unlock-screen.test-helpers.tsx +++ b/apps/mobile/src/components/app-unlock-screen.test-helpers.tsx @@ -60,15 +60,61 @@ const route = vi.hoisted(() => ({ segments: ['(app)', 'agent-chat'] as string[], pathname: '/agent-chat', })); -const lifecycle = vi.hoisted(() => ({ - change: undefined as ((state: AppStateStatus) => void) | undefined, -})); -const keyboard = vi.hoisted(() => ({ - // Both slots share one signature so either listener can be stored; `hide` - // is dispatched with an empty payload. - show: undefined as ((event: { endCoordinates: { height: number } }) => void) | undefined, - hide: undefined as ((event: { endCoordinates: { height: number } }) => void) | undefined, -})); +// AppState has more than one subscriber on this screen (the query client +// lifecycle and the keyboard-lift hooks), so the harness keeps every listener +// and broadcasts to all of them. A single slot let the last registration +// shadow the earlier ones, which hid the stale-lift defect these mounted tests +// exist to catch. Ported from the closed #6392. +const lifecycle = vi.hoisted(() => { + const listeners = new Set<(state: AppStateStatus) => void>(); + return { + listeners, + change: (state: AppStateStatus) => { + for (const listener of listeners) { + listener(state); + } + }, + }; +}); +const keyboard = vi.hoisted(() => { + // Keyboard has more than one subscriber in production: `AppRootProviders` + // mounts the Toaster's shared keyboard hook alongside a screen's, so the mock + // keeps every listener in a set per direction. One slot per direction let the + // last registration shadow the earlier ones, and a `remove()` that cleared + // both slots detached a listener it did not own. The AppState mock below is a + // set for the same reason. Same signature for both directions; `hide` is + // dispatched with an empty payload. + const showListeners = new Set<(event: { endCoordinates: { height: number } }) => void>(); + const hideListeners = new Set<(event: { endCoordinates: { height: number } }) => void>(); + const addListener = ( + event: string, + listener: (event: { endCoordinates: { height: number } }) => void + ) => { + const listeners = + event === 'keyboardDidShow' || event === 'keyboardWillShow' ? showListeners : hideListeners; + listeners.add(listener); + return { + remove: () => { + listeners.delete(listener); + }, + }; + }; + return { + addListener, + showListeners, + hideListeners, + show: (event: { endCoordinates: { height: number } }) => { + for (const listener of showListeners) { + listener(event); + } + }, + hide: () => { + for (const listener of hideListeners) { + listener({ endCoordinates: { height: 0 } }); + } + }, + }; +}); export { announcements, catalogs, keyboard, lifecycle, native, platform, route, storage }; vi.mock('@/i18n/catalogs', () => ({ CATALOG_LOADERS: catalogs })); vi.mock('expo-local-authentication', () => native); @@ -90,30 +136,15 @@ vi.mock('react-native', () => ({ I18nManager: { isRTL: false }, AccessibilityInfo: { announceForAccessibility: announcements }, Keyboard: { - addListener: ( - event: string, - listener: (event: { endCoordinates: { height: number } }) => void - ) => { - if (event === 'keyboardDidShow' || event === 'keyboardWillShow') { - keyboard.show = listener; - } else { - keyboard.hide = listener; - } - return { - remove: () => { - keyboard.show = undefined; - keyboard.hide = undefined; - }, - }; - }, + addListener: keyboard.addListener, }, AppState: { currentState: 'active', addEventListener: (_event: string, listener: (state: AppStateStatus) => void) => { - lifecycle.change = listener; + lifecycle.listeners.add(listener); return { remove: () => { - lifecycle.change = undefined; + lifecycle.listeners.delete(listener); }, }; }, @@ -327,6 +358,9 @@ export function resetUnlockMocks() { platform.OS = 'ios'; route.segments = ['(app)', 'agent-chat']; route.pathname = '/agent-chat'; + lifecycle.listeners.clear(); + keyboard.showListeners.clear(); + keyboard.hideListeners.clear(); storage.getItemAsync.mockResolvedValue('enabled'); native.hasHardwareAsync.mockResolvedValue(true); native.isEnrolledAsync.mockResolvedValue(true); diff --git a/apps/mobile/src/components/kilo-chat/app-aware-keyboard-padding-state.test.ts b/apps/mobile/src/components/kilo-chat/app-aware-keyboard-padding-state.test.ts index 4825e02e7c..d2a373f624 100644 --- a/apps/mobile/src/components/kilo-chat/app-aware-keyboard-padding-state.test.ts +++ b/apps/mobile/src/components/kilo-chat/app-aware-keyboard-padding-state.test.ts @@ -1,10 +1,39 @@ import { describe, expect, it } from 'vitest'; +import { resolveKeyboardBottomPadding } from '@/components/login-screen-state'; import { resolveAppAwareKeyboardPadding, resolveKeyboardPaddingEventsForPlatform, } from './app-aware-keyboard-padding-state'; +// Ported from the closed #6380, whose cases covered the platform-aware bottom +// occlusion the keeper #6388 resolves in `resolveKeyboardBottomPadding`: the +// reported geometry is the platform capability that differs, so Android adds +// the system-bar inset to reach the keyboard's top edge while iOS keeps the +// keyboard frame height, which already reaches the window bottom. +describe('platform-aware keyboard bottom occlusion', () => { + it('reaches the keyboard top edge on Android by adding the system-bar inset', () => { + expect( + resolveKeyboardBottomPadding({ platform: 'android', keyboardHeight: 704, bottomInset: 63 }) + ).toBe(767); + }); + + it('keeps the iOS height, which already reaches the window bottom', () => { + expect( + resolveKeyboardBottomPadding({ platform: 'ios', keyboardHeight: 300, bottomInset: 34 }) + ).toBe(300); + }); + + it('reserves the system-bar inset alone while the keyboard is hidden', () => { + expect( + resolveKeyboardBottomPadding({ platform: 'android', keyboardHeight: 0, bottomInset: 63 }) + ).toBe(63); + expect( + resolveKeyboardBottomPadding({ platform: 'ios', keyboardHeight: 0, bottomInset: 34 }) + ).toBe(34); + }); +}); + describe('app-aware keyboard padding state', () => { it('resolves Android keyboard events from did-show and did-hide notifications', () => { expect(resolveKeyboardPaddingEventsForPlatform('android')).toEqual({ @@ -20,7 +49,7 @@ describe('app-aware keyboard padding state', () => { }); }); - it('clears keyboard padding when the keyboard hides or the app leaves active state', () => { + it('clears keyboard padding when the keyboard hides or the app leaves the foreground', () => { expect( resolveAppAwareKeyboardPadding({ currentPadding: 0, @@ -40,4 +69,23 @@ describe('app-aware keyboard padding state', () => { }) ).toBe(0); }); + + it('keeps keyboard padding through a transient iOS inactive state', () => { + // iOS reports `inactive` for Control Center, the app switcher, a call + // banner, or a system alert while the keyboard stays up, and fires no new + // `keyboardWillShow` when it returns to `active`. Collapsing the padding + // there left the login action under an open keyboard. + expect( + resolveAppAwareKeyboardPadding({ + currentPadding: 320, + event: { type: 'app-state-change', appState: 'inactive' }, + }) + ).toBe(320); + expect( + resolveAppAwareKeyboardPadding({ + currentPadding: 320, + event: { type: 'app-state-change', appState: 'active' }, + }) + ).toBe(320); + }); }); diff --git a/apps/mobile/src/components/kilo-chat/app-aware-keyboard-padding-state.ts b/apps/mobile/src/components/kilo-chat/app-aware-keyboard-padding-state.ts index 4e61e53a1d..464b0a0e42 100644 --- a/apps/mobile/src/components/kilo-chat/app-aware-keyboard-padding-state.ts +++ b/apps/mobile/src/components/kilo-chat/app-aware-keyboard-padding-state.ts @@ -38,7 +38,13 @@ export function resolveAppAwareKeyboardPadding({ if (event.type === 'keyboard-hidden') { return 0; } - if (event.appState !== 'active') { + // iOS reports `inactive` for transient interruptions the keyboard survives — + // Control Center, the app-switcher preview, a call banner, a system + // permission alert — and fires no fresh `keyboardWillShow` on the way back to + // `active`. Dropping the padding there left the resolved occlusion stuck at 0 + // under an open keyboard, so only a real backgrounding (which dismisses the + // keyboard) clears it. + if (event.appState === 'background') { return 0; } return currentPadding; diff --git a/apps/mobile/src/components/kilo-chat/app-aware-keyboard-padding.mounted.test.tsx b/apps/mobile/src/components/kilo-chat/app-aware-keyboard-padding.mounted.test.tsx new file mode 100644 index 0000000000..f939cf9c6f --- /dev/null +++ b/apps/mobile/src/components/kilo-chat/app-aware-keyboard-padding.mounted.test.tsx @@ -0,0 +1,213 @@ +// Mounted coverage for the shared keyboard-lift view. When the view's bottom +// edge sits at the screen bottom, the reserved space is anchored there, so the +// view must resolve the platform's own keyboard metric through +// `resolveKeyboardBottomPadding` — the same rule the login screen and the +// Toaster use — instead of padding by the raw height. Android's raw height +// stops at the navigation bar, so reserving it left the bottom `bottomInset` +// of the content (the manual review form's Start button) behind the IME's +// navigation row (2026-09-20). +// +// Callers whose own container already reserves the bottom inset above the view +// (the session screen's trailing chrome spacer, the new-session form's parent +// padding) pass `containerReservesBottomInset`, so the inset is subtracted and +// the space is resolved once per screen instead of twice. Callers whose wrapped +// content pads the inset itself (the session composer, the discussion CTA bar) +// pass `contentReservesBottomInset`, so the screen-bottom-anchored occlusion +// does not add it a second time either. + +import { createElement } from 'react'; +import { act, TestRenderer } from '@/test/renderer'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import { AppAwareKeyboardPaddingView } from './app-aware-keyboard-padding'; + +const platform = vi.hoisted(() => ({ OS: 'android' })); +const insets = vi.hoisted(() => ({ bottom: 0 })); +const keyboard = vi.hoisted(() => ({ + show: null as ((event: { endCoordinates: { height: number } }) => void) | null, + hide: null as (() => void) | null, + appState: null as ((state: string) => void) | null, +})); + +vi.mock('react-native', () => ({ + View: 'View', + Platform: platform, + Keyboard: { + addListener: vi.fn((event: string, listener: (event?: unknown) => void) => { + if (event === 'keyboardDidShow' || event === 'keyboardWillShow') { + keyboard.show = listener as (event: { endCoordinates: { height: number } }) => void; + } + if (event === 'keyboardDidHide' || event === 'keyboardWillHide') { + keyboard.hide = listener as () => void; + } + return { remove: vi.fn() }; + }), + }, + AppState: { + addEventListener: vi.fn((_event: string, listener: (state: string) => void) => { + keyboard.appState = listener; + return { remove: vi.fn() }; + }), + }, +})); + +vi.mock('react-native-safe-area-context', () => ({ + useSafeAreaInsets: () => insets, +})); + +type MountProps = { containerReservesBottomInset?: boolean; contentReservesBottomInset?: boolean }; + +function mount(props: MountProps = {}) { + const ref: { current: TestRenderer.ReactTestRenderer | undefined } = { current: undefined }; + act(() => { + ref.current = TestRenderer.create( + createElement(AppAwareKeyboardPaddingView, props, createElement('Child', null)) + ); + }); + const renderer = ref.current; + if (!renderer) { + throw new Error('padding view was not mounted'); + } + return renderer; +} + +function paddingBottom(renderer: TestRenderer.ReactTestRenderer): number { + const view = renderer.root.find(node => String(node.type) === 'View'); + const style = view.props.style as (Record | undefined)[]; + const padding = style.find(part => part != null && 'paddingBottom' in part); + if (!padding) { + throw new Error('padding view carries no paddingBottom'); + } + return padding.paddingBottom as number; +} + +describe('AppAwareKeyboardPaddingView', () => { + beforeEach(() => { + platform.OS = 'android'; + insets.bottom = 0; + keyboard.show = null; + keyboard.hide = null; + }); + + it('reserves nothing while the keyboard is down', () => { + insets.bottom = 63; + const renderer = mount(); + + expect(paddingBottom(renderer)).toBe(0); + renderer.unmount(); + }); + + it('adds the navigation-bar inset on Android, whose metric stops at the bar', () => { + platform.OS = 'android'; + insets.bottom = 63; + const renderer = mount(); + + act(() => { + keyboard.show?.({ endCoordinates: { height: 704 } }); + }); + expect(paddingBottom(renderer)).toBe(767); + + act(() => { + keyboard.hide?.(); + }); + expect(paddingBottom(renderer)).toBe(0); + + renderer.unmount(); + }); + + it('passes the iOS frame height through, which already reaches the screen bottom', () => { + platform.OS = 'ios'; + insets.bottom = 34; + const renderer = mount(); + + act(() => { + keyboard.show?.({ endCoordinates: { height: 300 } }); + }); + expect(paddingBottom(renderer)).toBe(300); + + renderer.unmount(); + }); + + it('subtracts the container-reserved inset on Android, leaving the raw metric', () => { + // The session screen's trailing spacer and the new-session form's parent + // padding already lift the view's bottom edge `bottomInset` above the + // screen bottom; Android's metric is measured down to the navigation bar, + // so the raw height is exactly the distance from the view's bottom edge to + // the IME top. Adding the inset again floated the composer / Start button + // a nav-bar height above the keyboard (2026-09-20 review finding). + platform.OS = 'android'; + insets.bottom = 63; + const renderer = mount({ containerReservesBottomInset: true }); + + act(() => { + keyboard.show?.({ endCoordinates: { height: 704 } }); + }); + expect(paddingBottom(renderer)).toBe(704); + + renderer.unmount(); + }); + + it('subtracts the container-reserved inset on iOS, whose frame reaches the screen bottom', () => { + platform.OS = 'ios'; + insets.bottom = 34; + const renderer = mount({ containerReservesBottomInset: true }); + + act(() => { + keyboard.show?.({ endCoordinates: { height: 300 } }); + }); + expect(paddingBottom(renderer)).toBe(266); + + renderer.unmount(); + }); + + it('still reserves nothing at rest when the container reserves the inset', () => { + insets.bottom = 63; + const renderer = mount({ containerReservesBottomInset: true }); + + expect(paddingBottom(renderer)).toBe(0); + renderer.unmount(); + }); + + it('leaves the content-reserved inset to the content on Android', () => { + // The chat composer and the discussion CTA bar pad the bottom inset inside + // the view themselves, so the screen-bottom-anchored occlusion must not add + // it again — adding it floated the composer a navigation-bar height above + // the keyboard (2026-09-21 review finding). + platform.OS = 'android'; + insets.bottom = 63; + const renderer = mount({ contentReservesBottomInset: true }); + + act(() => { + keyboard.show?.({ endCoordinates: { height: 704 } }); + }); + expect(paddingBottom(renderer)).toBe(704); + + act(() => { + keyboard.hide?.(); + }); + expect(paddingBottom(renderer)).toBe(0); + + renderer.unmount(); + }); + + it('keeps the iOS frame height for content that pads the inset itself', () => { + platform.OS = 'ios'; + insets.bottom = 34; + const renderer = mount({ contentReservesBottomInset: true }); + + act(() => { + keyboard.show?.({ endCoordinates: { height: 300 } }); + }); + expect(paddingBottom(renderer)).toBe(300); + + renderer.unmount(); + }); + + it('still reserves nothing at rest when the content reserves the inset', () => { + insets.bottom = 63; + const renderer = mount({ contentReservesBottomInset: true }); + + expect(paddingBottom(renderer)).toBe(0); + renderer.unmount(); + }); +}); diff --git a/apps/mobile/src/components/kilo-chat/app-aware-keyboard-padding.test.ts b/apps/mobile/src/components/kilo-chat/app-aware-keyboard-padding.test.ts index 78cc1adbba..27e6040268 100644 --- a/apps/mobile/src/components/kilo-chat/app-aware-keyboard-padding.test.ts +++ b/apps/mobile/src/components/kilo-chat/app-aware-keyboard-padding.test.ts @@ -21,6 +21,18 @@ describe('app-aware keyboard padding', () => { ).toBe(0); }); + it('keeps the padded height through a transient inactive state', () => { + // A keyboard that stays up across Control Center or a system alert must not + // have its reserved padding collapsed: iOS fires no `keyboardWillShow` + // again on the way back to `active`. + expect( + resolveAppAwareKeyboardPadding({ + currentPadding: 320, + event: { type: 'app-state-change', appState: 'inactive' }, + }) + ).toBe(320); + }); + it('keeps padding reset on foreground until a fresh keyboard event arrives', () => { expect( resolveAppAwareKeyboardPadding({ diff --git a/apps/mobile/src/components/kilo-chat/app-aware-keyboard-padding.tsx b/apps/mobile/src/components/kilo-chat/app-aware-keyboard-padding.tsx index 50cfc43e1f..aa8049e86f 100644 --- a/apps/mobile/src/components/kilo-chat/app-aware-keyboard-padding.tsx +++ b/apps/mobile/src/components/kilo-chat/app-aware-keyboard-padding.tsx @@ -1,6 +1,8 @@ import { type ComponentProps, useEffect, useState } from 'react'; import { AppState, Keyboard, type KeyboardEvent, Platform, View } from 'react-native'; +import { useSafeAreaInsets } from 'react-native-safe-area-context'; +import { resolveKeyboardBottomPadding } from '@/components/login-screen-state'; import { resolveAppAwareKeyboardPadding, resolveKeyboardPaddingEventsForPlatform, @@ -70,9 +72,64 @@ export function useAppAwareKeyboardPadding(): number { export function AppAwareKeyboardPaddingView({ style, keyboardOffset = 0, + containerReservesBottomInset = false, + contentReservesBottomInset = false, ...props -}: ComponentProps & { keyboardOffset?: number }) { - const keyboardPadding = useAppAwareKeyboardPadding(); +}: ComponentProps & { + keyboardOffset?: number; + /** + * The caller's own container reserves the platform's bottom inset above this + * view (a trailing spacer, or a `paddingBottom` on the parent), so the + * view's bottom edge sits `bottomInset` above the screen bottom. The + * resolved occlusion is anchored to the screen bottom, so the inset the + * container already reserved is subtracted here — on Android that reduces to + * the platform's raw metric, whose origin stops at the navigation bar. + * Counting the inset twice floated the session composer and the new-session + * Start button a nav-bar height above the keyboard (2026-09-20). + */ + containerReservesBottomInset?: boolean; + /** + * The wrapped content pads the platform's bottom inset itself: the session + * composer adds `MESSAGE_INPUT_BOTTOM_CLEARANCE + bottomInset` and the + * discussion CTA bar adds `useDetailScreenBottomPadding()`. The occlusion + * resolved above is anchored to the screen bottom, so it counts that inset on + * top of the content's own padding and floats the composer / CTA a + * navigation-bar height above the keyboard. Such a caller adds the platform's + * raw keyboard metric instead: on Android the content's inset padding + * completes it, and on iOS the metric already reaches the screen bottom, so + * the lift those callers shipped with is unchanged (2026-09-21 review + * finding). + */ + contentReservesBottomInset?: boolean; +}) { + const keyboardHeight = useAppAwareKeyboardPadding(); + const { bottom } = useSafeAreaInsets(); + // The hook reports the platform's own keyboard metric, and the two platforms + // measure it from different origins: Android's stops at the navigation bar + // (`ReactRootView` reports `imeInsets.bottom − barInsets.bottom`) while iOS's + // frame reaches the screen bottom. The reserved space is anchored to the + // screen bottom, so resolve it with the same rule the login screen and the + // Toaster use; padding by the raw Android height left the bottom + // `bottomInset` of the content — the manual review form's Start button — + // behind the IME's navigation row (2026-09-20). + const keyboardOcclusion = + keyboardHeight > 0 + ? resolveKeyboardBottomPadding({ + keyboardHeight, + bottomInset: bottom, + platform: Platform.OS, + }) + : 0; + // One inset per screen: where a container outside this view (a trailing + // spacer, a parent `paddingBottom`) or the wrapped content's own bottom + // padding already reserved the bottom inset, the screen-bottom-anchored + // occlusion must not count it a second time. + let keyboardPadding = keyboardOcclusion; + if (containerReservesBottomInset) { + keyboardPadding = Math.max(keyboardOcclusion - bottom, 0); + } else if (contentReservesBottomInset) { + keyboardPadding = keyboardHeight; + } const resolvedKeyboardPadding = keyboardPadding > 0 ? keyboardPadding + keyboardOffset : 0; diff --git a/apps/mobile/src/components/kilo-chat/conversation-screen.composer-keyboard.mounted.test.tsx b/apps/mobile/src/components/kilo-chat/conversation-screen.composer-keyboard.mounted.test.tsx new file mode 100644 index 0000000000..5a3a3b66a4 --- /dev/null +++ b/apps/mobile/src/components/kilo-chat/conversation-screen.composer-keyboard.mounted.test.tsx @@ -0,0 +1,235 @@ +// Mounted coverage for the chat screen's keyboard lift. The composer's own +// bottom padding already includes the platform's safe-area inset +// (`resolveMessageInputBottomPadding`), so the screen's +// `AppAwareKeyboardPaddingView` must not count that inset a second time: doing +// so floated the composer a navigation-bar height above the keyboard on Android +// (2026-09-21 review finding). Both platforms are asserted against the metric +// the composer completes, so a caller that drops the opt-in fails here. + +import { createElement } from 'react'; +import { act, TestRenderer } from '@/test/renderer'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import '@/i18n'; +import type * as ReactI18next from 'react-i18next'; +import { ConversationScreen } from './conversation-screen'; + +const platform = vi.hoisted(() => ({ OS: 'android' })); +const insets = vi.hoisted(() => ({ bottom: 0 })); +const keyboard = vi.hoisted(() => ({ + show: null as ((event: { endCoordinates: { height: number } }) => void) | null, + hide: null as (() => void) | null, +})); + +vi.mock('react-native', () => ({ + View: 'View', + Platform: platform, + Keyboard: { + addListener: vi.fn((event: string, listener: (event?: unknown) => void) => { + if (event === 'keyboardDidShow' || event === 'keyboardWillShow') { + keyboard.show = listener as (event: { endCoordinates: { height: number } }) => void; + } + if (event === 'keyboardDidHide' || event === 'keyboardWillHide') { + keyboard.hide = listener as () => void; + } + return { remove: vi.fn() }; + }), + }, + AppState: { + addEventListener: vi.fn(() => ({ remove: vi.fn() })), + }, +})); + +vi.mock('react-native-safe-area-context', () => ({ + useSafeAreaInsets: () => insets, +})); + +vi.mock('@kilocode/kilo-chat-hooks', () => ({ + useBotStatus: () => null, + useEventServiceClient: () => ({}), +})); + +vi.mock('@kilocode/kilo-chat', () => ({ CONVERSATION_TITLE_MAX_CHARS: 100 })); + +vi.mock('expo-router', () => ({ + useFocusEffect: vi.fn(), + useRouter: () => ({ push: vi.fn() }), +})); + +vi.mock('sonner-native', () => ({ toast: { error: vi.fn() } })); +vi.mock('react-i18next', async importOriginal => { + const actual = (await importOriginal()) as typeof ReactI18next; + return { + ...actual, + useTranslation: () => { + const i18n = actual.getI18n(); + return { t: i18n.t.bind(i18n), i18n }; + }, + }; +}); + +vi.mock('@/components/rename-modal', () => ({ RenameModal: 'RenameModal' })); +vi.mock('@/lib/notifications', () => ({ setActiveChatLocation: vi.fn() })); +vi.mock('@/lib/kilo-chat-routes', () => ({ chatInstancePickerPath: () => '/instances' })); +vi.mock('@/lib/kiloclaw-display', () => ({ kiloclawConversationEyebrow: () => undefined })); +vi.mock('@/lib/hooks/use-instance-context', () => ({ + instanceOrgId: () => 'org-1', + useInstanceContext: () => ({ status: 'ready' }), + useAllKiloClawInstances: () => ({ data: undefined }), +})); +vi.mock('@/lib/hooks/use-kiloclaw-queries', () => ({ useKiloClawStatus: () => ({ data: null }) })); + +// Mocked like the repo's other mounted screens: the loading/error views and the +// heavy children are stubbed down to their element type, so the mounted tree is +// the screen's own keyboard-lift view and the composer's slot in it. This test +// keeps the history content at `ready`, so none of those views render. +vi.mock('./conversation-history-state-views', () => ({ + ConversationHistoryErrorView: 'ConversationHistoryErrorView', + ConversationHistoryLoadingView: 'ConversationHistoryLoadingView', + ConversationInlineRetryBanner: 'ConversationInlineRetryBanner', +})); +vi.mock('./conversation-header', () => ({ ConversationHeader: 'ConversationHeader' })); +vi.mock('./message-list', () => ({ MessageList: 'MessageList' })); +vi.mock('./message-input', () => ({ MessageInput: 'MessageInput' })); +vi.mock('./message-reaction-picker-sheet', () => ({ + MessageReactionPickerSheet: 'MessageReactionPickerSheet', +})); + +vi.mock('./kilo-chat-provider', () => ({ + useKiloChatTokenError: () => ({ hasError: false, retry: vi.fn() }), +})); +vi.mock('./hooks/use-app-active-and-focused', () => ({ useAppActiveAndFocused: () => true })); +vi.mock('./hooks/use-current-user-id', () => ({ useCurrentUserId: () => 'user-1' })); +vi.mock('./hooks/use-now-ticker', () => ({ useNowTicker: () => 1_800_000_000_000 })); +vi.mock('./hooks/use-kilo-chat-client', () => ({ useKiloChatClient: () => ({}) })); +vi.mock('./hooks/use-conversation-presence', () => ({ useConversationPresence: vi.fn() })); +vi.mock('./hooks/use-conversation-event-subscription', () => ({ + useConversationEventSubscription: vi.fn(), +})); +vi.mock('./hooks/use-conversation-mark-read', () => ({ useConversationMarkRead: vi.fn() })); +vi.mock('./hooks/use-messages', () => ({ + useMessageCacheUpdater: vi.fn(), + useMessages: () => ({ + data: { messages: [] }, + isPending: false, + isError: false, + hasNextPage: false, + isFetchingNextPage: false, + fetchNextPage: vi.fn(), + }), +})); +vi.mock('./hooks/use-typing', () => ({ + useMobileTypingState: () => ({ typingMembers: [], clearTypingForMember: vi.fn() }), + useTypingSender: () => vi.fn(), +})); +vi.mock('./hooks/use-conversation-options-sheet', () => ({ + useConversationOptionsSheet: () => ({ + openOptions: vi.fn(), + renaming: false, + closeRename: vi.fn(), + saveRename: vi.fn(), + }), +})); +vi.mock('./hooks/use-conversation-message-controller', () => ({ + useConversationMessageController: () => ({ + editingMessage: null, + editingText: '', + visibleEditingAttachments: [], + inputAvailability: { + disabled: false, + submitDisabled: false, + disabledReason: undefined, + showInstanceCta: false, + }, + pendingAction: null, + reactionPickerMessage: null, + recentReactions: [], + replyingTo: null, + scrollToNewestRequest: 0, + handleExecuteAction: vi.fn(), + handleLongPressMessage: vi.fn(), + handleReactionPress: vi.fn(), + handleSend: vi.fn(), + handleSwipeReplyMessage: vi.fn(), + setEditingMessage: vi.fn(), + setRemovedEditAttachmentIds: vi.fn(), + setReactionPickerMessage: vi.fn(), + setReplyingTo: vi.fn(), + }), +})); + +function mount() { + const ref: { current: TestRenderer.ReactTestRenderer | undefined } = { current: undefined }; + act(() => { + ref.current = TestRenderer.create( + createElement(ConversationScreen, { + sandboxId: 'instance-1', + conversationId: 'conversation-1', + conversationTitle: 'Title', + conversationRenameTitle: 'Title', + conversationMembers: [], + }) + ); + }); + const renderer = ref.current; + if (!renderer) { + throw new Error('conversation screen was not mounted'); + } + return renderer; +} + +/** Padding the screen's keyboard-lift view reserves (its own style slot). */ +function keyboardPadding(renderer: TestRenderer.ReactTestRenderer): number { + const view = renderer.root.find( + node => String(node.type) === 'View' && Array.isArray(node.props.style) + ); + const parts = view.props.style as (Record | undefined)[]; + const padded = parts.find(part => part != null && 'paddingBottom' in part); + return padded?.paddingBottom ?? -1; +} + +describe('ConversationScreen composer keyboard lift', () => { + beforeEach(() => { + platform.OS = 'android'; + insets.bottom = 0; + keyboard.show = null; + keyboard.hide = null; + }); + + it("adds only the raw Android metric on top of the composer's own inset padding", () => { + platform.OS = 'android'; + insets.bottom = 63; + const renderer = mount(); + + act(() => { + keyboard.show?.({ endCoordinates: { height: 704 } }); + }); + // 767 would be the screen-bottom-anchored occlusion counting the inset the + // composer already pads by a second time. + expect(keyboardPadding(renderer)).toBe(704); + + renderer.unmount(); + }); + + it('keeps the iOS frame height, which the composer completes', () => { + platform.OS = 'ios'; + insets.bottom = 34; + const renderer = mount(); + + act(() => { + keyboard.show?.({ endCoordinates: { height: 300 } }); + }); + expect(keyboardPadding(renderer)).toBe(300); + + renderer.unmount(); + }); + + it('reserves nothing while the keyboard is down', () => { + insets.bottom = 63; + const renderer = mount(); + + expect(keyboardPadding(renderer)).toBe(0); + + renderer.unmount(); + }); +}); diff --git a/apps/mobile/src/components/kilo-chat/conversation-screen.tsx b/apps/mobile/src/components/kilo-chat/conversation-screen.tsx index 173863c364..65cca84613 100644 --- a/apps/mobile/src/components/kilo-chat/conversation-screen.tsx +++ b/apps/mobile/src/components/kilo-chat/conversation-screen.tsx @@ -199,7 +199,10 @@ export function ConversationScreen({ }} /> ) : null} - + {/* The composer below already pads the platform's bottom inset inside + this view (message-input-layout), so the keyboard lift must not add it + a second time and float the composer above the keyboard. */} + { } ); + it('keeps the iOS keyboard occlusion through a transient inactive state', async () => { + Platform.OS = 'ios'; + const renderer = await mountLoginScreen(); + emitKeyboard(keyboardEventsFor('ios').show, 300); + const subscription = addAppStateListener.mock.calls[0]; + if (!subscription) { + throw new Error('missing app state listener'); + } + + // Control Center, the app switcher preview, a call banner, and system + // permission alerts report `inactive` while the keyboard stays up, and no + // fresh `keyboardWillShow` follows on the way back to `active`; collapsing + // the occlusion here left the form under an open keyboard. + act(() => { + subscription[1]('inactive'); + }); + expect(scrollViewport(renderer).props.style).toEqual({ paddingBottom: 300 }); + + act(() => { + subscription[1]('active'); + }); + expect(scrollViewport(renderer).props.style).toEqual({ paddingBottom: 300 }); + + renderer.unmount(); + }); + it('tracks bottom inset changes, including devices without a bottom bar', async () => { const renderer = await mountLoginScreen(); diff --git a/apps/mobile/src/components/pr-review/discussion/pr-comment-cta.test.tsx b/apps/mobile/src/components/pr-review/discussion/pr-comment-cta.test.tsx index 64ae3be6a9..82314e6b8d 100644 --- a/apps/mobile/src/components/pr-review/discussion/pr-comment-cta.test.tsx +++ b/apps/mobile/src/components/pr-review/discussion/pr-comment-cta.test.tsx @@ -23,6 +23,7 @@ vi.mock('react-i18next', async importOriginal => { }); const insetsState = vi.hoisted(() => ({ bottom: 0 })); +const platformState = vi.hoisted(() => ({ OS: 'ios' })); const keyboardSubscribers = vi.hoisted(() => ({ show: null as ((event: { endCoordinates: { height: number } }) => void) | null, hide: null as (() => void) | null, @@ -30,15 +31,15 @@ const keyboardSubscribers = vi.hoisted(() => ({ vi.mock('react-native', () => ({ View: 'View', - Platform: { OS: 'ios' }, + Platform: platformState, Keyboard: { addListener: vi.fn((event: string, listener: (event?: unknown) => void) => { - if (event === 'keyboardWillShow') { + if (event === 'keyboardWillShow' || event === 'keyboardDidShow') { keyboardSubscribers.show = listener as (event: { endCoordinates: { height: number }; }) => void; } - if (event === 'keyboardWillHide') { + if (event === 'keyboardWillHide' || event === 'keyboardDidHide') { keyboardSubscribers.hide = listener as () => void; } return { remove: vi.fn() }; @@ -110,6 +111,7 @@ function paddingValues(renderer: TestRenderer.ReactTestRenderer): number[] { describe('PrCommentCta', () => { beforeEach(() => { + platformState.OS = 'ios'; insetsState.bottom = 0; keyboardSubscribers.show = null; keyboardSubscribers.hide = null; @@ -156,6 +158,24 @@ describe('PrCommentCta', () => { expect(paddingValues(renderer)).toContain(336); }); + it("lifts by the raw Android metric, which the bar's own inset padding completes", () => { + // The bar's inner padding already includes the platform's bottom inset + // (`useDetailScreenBottomPadding`), so the lift must not add it a second + // time and float the button a navigation-bar height above the keyboard + // (2026-09-21 review finding). + platformState.OS = 'android'; + insetsState.bottom = 63; + const renderer = mountCta(); + if (!keyboardSubscribers.show) { + throw new Error('keyboard show listener was not registered'); + } + act(() => { + keyboardSubscribers.show?.({ endCoordinates: { height: 704 } }); + }); + expect(paddingValues(renderer)).toContain(704); + expect(paddingValues(renderer)).not.toContain(767); + }); + it('does not react to keyboard events at all while the lift is gated off', () => { // The host passes keyboardLift=false when another surface owns the // keyboard (the conversation-comment formSheet): the bar must not even diff --git a/apps/mobile/src/components/pr-review/discussion/pr-comment-cta.tsx b/apps/mobile/src/components/pr-review/discussion/pr-comment-cta.tsx index 67149e46e9..0edc9e967c 100644 --- a/apps/mobile/src/components/pr-review/discussion/pr-comment-cta.tsx +++ b/apps/mobile/src/components/pr-review/discussion/pr-comment-cta.tsx @@ -52,5 +52,13 @@ export function PrCommentCta({ onPress, keyboardLift }: PrCommentCtaProps) { ); // Unmounted (not just un-padded) while unfocused: the padding view's own // keyboard listener must not react to another surface's keyboard at all. - return keyboardLift ? {bar} : bar; + // The bar's inner padding already includes the platform's bottom inset + // (`useDetailScreenBottomPadding`), so `contentReservesBottomInset` keeps the + // lift from counting that inset a second time and floating the button a + // navigation-bar height above the keyboard. + return keyboardLift ? ( + {bar} + ) : ( + bar + ); } diff --git a/apps/mobile/src/lib/toast-offset.test.ts b/apps/mobile/src/lib/toast-offset.test.ts index cebf184fb6..c2665f67dc 100644 --- a/apps/mobile/src/lib/toast-offset.test.ts +++ b/apps/mobile/src/lib/toast-offset.test.ts @@ -41,6 +41,9 @@ describe('getToastBottomOffset', () => { }); it('raises the toast above the software keyboard', () => { + // `keyboardHeight` is the occlusion measured from the screen bottom, which + // the caller resolves with `resolveKeyboardBottomPadding`: Android's raw + // height stops at the navigation bar and must not reach this math. expect(getToastBottomOffset({ safeAreaBottom: 24, keyboardHeight: 300 })).toBe( 300 + TOAST_BOTTOM_GAP ); diff --git a/apps/mobile/src/lib/toast-offset.ts b/apps/mobile/src/lib/toast-offset.ts index df1f6b9e69..6be0241ab1 100644 --- a/apps/mobile/src/lib/toast-offset.ts +++ b/apps/mobile/src/lib/toast-offset.ts @@ -30,11 +30,19 @@ export const TOAST_BOTTOM_GAP = 8; /** * Bottom offset, in logical pixels, for the bottom-center toast container. - * The keyboard height wins while the software keyboard is up so the toast + * The keyboard occlusion wins while the software keyboard is up so the toast * cannot hide behind it; otherwise the tallest bottom chrome decides: the * floating tab bar when one is on screen, else the reported safe-area inset * floored at `MIN_BOTTOM_CHROME_HEIGHT`. The standard gap always separates the * toast from the chrome. + * + * This module reads no platform, so `keyboardHeight` is the keyboard's + * occlusion measured from the screen bottom, not a platform's raw metric: the + * caller resolves it with `resolveKeyboardBottomPadding`, because Android's + * reported height stops at the navigation bar while iOS's keyboard frame + * reaches the screen bottom. The container is anchored to the screen bottom, so + * a raw Android height would leave the toast's last line behind the IME's + * navigation row. */ export function getToastBottomOffset({ safeAreaBottom, @@ -42,6 +50,10 @@ export function getToastBottomOffset({ tabBarHeight = 0, }: { safeAreaBottom: number; + /** + * Keyboard occlusion measured from the screen bottom, `0` while the keyboard + * is down (see `resolveKeyboardBottomPadding`). + */ keyboardHeight: number; /** Rendered height of the floating tab bar while one is on screen, `0` otherwise. */ tabBarHeight?: number; diff --git a/apps/web/src/lib/code-reviews/manual-code-review-jobs.test.ts b/apps/web/src/lib/code-reviews/manual-code-review-jobs.test.ts new file mode 100644 index 0000000000..4a2449800a --- /dev/null +++ b/apps/web/src/lib/code-reviews/manual-code-review-jobs.test.ts @@ -0,0 +1,230 @@ +import { TRPCError } from '@trpc/server'; +import { getHTTPStatusCodeFromError } from '@trpc/server/http'; + +// Ported from the closed #6405 (its `manual-code-review-jobs.test.ts`) onto the +// implementation kept in #6325, which maps every public-provider round-trip +// failure to a client error instead of letting tRPC answer 500. +const mockIsLocalCodeReviewDevelopmentEnabled = jest.fn(); +const mockGetAgentConfigForOwner = jest.fn(); +const mockAssertCouncilCreationAllowed = jest.fn(); +const mockCreateCodeReview = jest.fn(); +const mockTryDispatchPendingReviews = jest.fn(); + +jest.mock('@/lib/config.server', () => ({ + isLocalCodeReviewDevelopmentEnabled: () => mockIsLocalCodeReviewDevelopmentEnabled(), +})); + +jest.mock('@/lib/agent-config/db/agent-configs', () => ({ + getAgentConfigForOwner: (...args: unknown[]) => mockGetAgentConfigForOwner(...args), +})); + +jest.mock('./core/council-entitlement', () => ({ + assertCouncilCreationAllowed: (...args: unknown[]) => mockAssertCouncilCreationAllowed(...args), +})); + +jest.mock('./db/code-reviews', () => ({ + createCodeReview: (...args: unknown[]) => mockCreateCodeReview(...args), + findActiveProviderPublishingReview: jest.fn(), +})); + +jest.mock('./dispatch/dispatch-pending-reviews', () => ({ + tryDispatchPendingReviews: (...args: unknown[]) => mockTryDispatchPendingReviews(...args), +})); + +import { createManualCodeReviewJob } from './manual-code-review-jobs'; + +const OWNER = { type: 'user' as const, id: 'user-1', userId: 'user-1' }; + +const GITHUB_PR_URL = 'https://github.com/owner/repo/pull/123'; +const GITLAB_MR_URL = 'https://gitlab.com/group/project/-/merge_requests/123'; + +function providerResponse(status: number, body: unknown): Response { + return { + ok: status >= 200 && status < 300, + status, + text: async () => JSON.stringify(body), + json: async () => body, + } as unknown as Response; +} + +function taskInput(overrides: Record = {}) { + return { + platform: 'github' as const, + url: GITHUB_PR_URL, + modelSlug: 'test-model', + ...overrides, + }; +} + +async function captureError(overrides: Record = {}): Promise { + try { + await createManualCodeReviewJob({ owner: OWNER, input: taskInput(overrides) }); + return null; + } catch (error) { + return error; + } +} + +beforeEach(() => { + jest.clearAllMocks(); + mockIsLocalCodeReviewDevelopmentEnabled.mockReturnValue(true); + mockGetAgentConfigForOwner.mockResolvedValue(null); + mockAssertCouncilCreationAllowed.mockResolvedValue(undefined); +}); + +describe('createManualCodeReviewJob provider failures', () => { + afterEach(() => { + jest.restoreAllMocks(); + }); + + // Every provider round-trip failure must surface as a mapped tRPC error. + // Before the mapping landed these escaped as raw + // ProviderFetchError/TypeError/ZodError and tRPC answered + // INTERNAL_SERVER_ERROR (HTTP 500) — the finding's defect. A provider that is + // genuinely unreachable maps to 502, which is the correct gateway status and + // not the reported internal error. + const cases: Array<{ + name: string; + fetch: () => void; + code: TRPCError['code']; + /** Input overrides for a case whose name names a provider other than GitHub. */ + input?: Record; + /** Copy that proves the case ran the provider its name names. */ + message?: string; + }> = [ + { + name: 'a missing public pull request maps to NOT_FOUND', + fetch: () => + void jest + .spyOn(global, 'fetch') + .mockResolvedValue(providerResponse(404, { message: 'Not Found' })), + code: 'NOT_FOUND', + }, + { + name: 'a rate-limited GitHub maps to TOO_MANY_REQUESTS', + fetch: () => + void jest + .spyOn(global, 'fetch') + .mockResolvedValue(providerResponse(403, { message: 'API rate limit exceeded' })), + code: 'TOO_MANY_REQUESTS', + }, + { + name: 'a rate-limited GitLab maps to TOO_MANY_REQUESTS', + // GitLab reports a rate limit with 429, so the case must run the GitLab + // path — the default input is a GitHub pull request. + input: { platform: 'gitlab', url: GITLAB_MR_URL }, + message: 'GitLab rate-limited', + fetch: () => + void jest + .spyOn(global, 'fetch') + .mockResolvedValue(providerResponse(429, { message: 'Too Many Requests' })), + code: 'TOO_MANY_REQUESTS', + }, + { + name: 'an unreachable provider maps to BAD_GATEWAY', + fetch: () => + void jest.spyOn(global, 'fetch').mockRejectedValue(new TypeError('fetch failed')), + code: 'BAD_GATEWAY', + }, + { + name: 'an unexpected provider shape maps to BAD_GATEWAY', + fetch: () => + void jest.spyOn(global, 'fetch').mockResolvedValue(providerResponse(200, { nope: true })), + code: 'BAD_GATEWAY', + }, + { + name: 'a provider error status maps to BAD_GATEWAY', + fetch: () => + void jest + .spyOn(global, 'fetch') + .mockResolvedValue(providerResponse(500, { message: 'Internal Server Error' })), + code: 'BAD_GATEWAY', + }, + ]; + + it.each(cases)('$name', async ({ fetch, code, input, message }) => { + fetch(); + + const error = await captureError(input); + + expect(error).toBeInstanceOf(TRPCError); + expect((error as TRPCError).code).toBe(code); + if (message) { + expect((error as TRPCError).message).toContain(message); + } + // The finding's defect was tRPC's unmapped INTERNAL_SERVER_ERROR / HTTP 500. + expect((error as TRPCError).code).not.toBe('INTERNAL_SERVER_ERROR'); + expect(getHTTPStatusCodeFromError(error as TRPCError)).not.toBe(500); + }); + + it('reports an unparseable provider response as unexpected, not unreachable', async () => { + jest.spyOn(global, 'fetch').mockResolvedValue({ + ok: true, + status: 200, + text: async () => 'not json', + json: async () => JSON.parse('not json'), + } as unknown as Response); + + const error = await captureError(); + + expect(error).toBeInstanceOf(TRPCError); + expect((error as TRPCError).code).toBe('BAD_GATEWAY'); + expect((error as TRPCError).message).toContain('unexpected response'); + expect((error as TRPCError).message).not.toContain('Could not reach'); + // The original parse error is kept as the cause rather than dropped. + expect(((error as TRPCError).cause as Error | undefined)?.message).toContain('JSON'); + }); + + it('names the GitLab merge request, never a pull request, in a GitLab failure', async () => { + jest.spyOn(global, 'fetch').mockResolvedValue(providerResponse(404, { message: 'Not Found' })); + + const error = await captureError({ platform: 'gitlab', url: GITLAB_MR_URL }); + + expect(error).toBeInstanceOf(TRPCError); + expect((error as TRPCError).code).toBe('NOT_FOUND'); + expect((error as TRPCError).message).toContain('GitLab'); + expect((error as TRPCError).message).toContain('merge request'); + expect((error as TRPCError).message).not.toContain('pull request'); + }); + + // GitLab returns 429 for rate limits; a 403 is a permission error and must not + // be reported to the user as a rate limit. + it('does not map a public GitLab 403 to a rate-limit error', async () => { + jest.spyOn(global, 'fetch').mockResolvedValue(providerResponse(403, { message: 'Forbidden' })); + + const error = await captureError({ platform: 'gitlab', url: GITLAB_MR_URL }); + + expect(error).toBeInstanceOf(TRPCError); + expect((error as TRPCError).code).toBe('BAD_GATEWAY'); + expect((error as TRPCError).message).toContain('unexpected response'); + expect((error as TRPCError).message).not.toContain('rate-limited'); + }); +}); + +describe('createManualCodeReviewJob happy path', () => { + afterEach(() => { + jest.restoreAllMocks(); + }); + + it('creates the job from a public pull request and dispatches pending reviews', async () => { + jest.spyOn(global, 'fetch').mockResolvedValue( + providerResponse(200, { + number: 123, + html_url: GITHUB_PR_URL, + title: 'Fix the thing', + state: 'open', + draft: false, + user: { login: 'octocat', id: 1 }, + base: { ref: 'main', repo: { full_name: 'owner/repo' } }, + head: { ref: 'feature', sha: 'abc123' }, + }) + ); + mockCreateCodeReview.mockResolvedValue('review-1'); + + await expect(createManualCodeReviewJob({ owner: OWNER, input: taskInput() })).resolves.toEqual({ + reviewId: 'review-1', + outputMode: 'kilo', + }); + expect(mockTryDispatchPendingReviews).toHaveBeenCalledWith(OWNER); + }); +});