Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,33 @@ beforeEach(() => {
collapseState.setConnectCtaCollapsed.mockClear();
});

describe('NewSessionRepositorySection repository picker', () => {
it('renders the picker under the Repository heading when no provider has rows yet', () => {
// Findings 1 and 2: with both providers settled-empty but no rows, the
// picker was gated off (`hasRepos || anyLoading`), leaving only the
// `Repository` heading above the Start session button — a blank void with
// no control under it. The picker trigger reserves its height and already
// covers loading, empty and disabled, so it must always render.
const renderer = mountSection({
repositories: [],
groups: [group('github', 'repos'), group('gitlab', 'repos')],
});

expect(renderer.root.findAllByType('RepoSelector' as never)).toHaveLength(1);
});

it('renders the picker in its loading state while every provider is unsettled', () => {
const renderer = mountSection({
repositories: [],
groups: [group('github', 'loading'), group('gitlab', 'loading')],
});

const picker = renderer.root.findAllByType('RepoSelector' as never)[0];
expect(picker).toBeDefined();
expect(picker?.props.isLoading).toBe(true);
});
});

describe('NewSessionRepositorySection branch row', () => {
it.each([
['github:owner/repo', githubRow],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,11 +95,11 @@ function connectNoteKey(platform: RepositoryPlatform): string | undefined {

/**
* Provider-aware repository section. One group per provider renders its own
* empty/error state independently, and the picker trigger lists every
* repository plus the Recently used rows when any provider has rows. A
* provider's expanded connect prompt renders only before a repository is
* selected. Afterwards, compact actions keep other providers reachable without
* contradicting the completed selection or requiring it to be cleared.
* empty/error state independently, and the picker trigger always renders,
* listing every repository plus the Recently used rows. A provider's expanded
* connect prompt renders only before a repository is selected. Afterwards,
* compact actions keep other providers reachable without contradicting the
* completed selection or requiring it to be cleared.
*/
export function NewSessionRepositorySection({
disabled,
Expand Down Expand Up @@ -135,17 +135,25 @@ export function NewSessionRepositorySection({
{t('common.repository')}
</Text>

{(hasRepos || anyLoading) && (
<RepoSelector
value={value}
repositories={repositories}
recents={recents}
isLoading={!hasRepos && anyLoading}
organizationId={organizationId ?? null}
onChange={onChange}
disabled={disabled}
/>
)}
{/*
The picker renders unconditionally. Its trigger always reserves its
height and already covers loading ("Loading..."), empty ("Select
repository") and disabled states, so gating it off (the old
`hasRepos || anyLoading`) could leave the `Repository` heading as the
section's only child — a blank void above Start session — while a
provider query was paused (`isLoading === false`, no data) or a
connect card was waiting on the persisted-collapse read. The web panel
renders its repository trigger unconditionally too.
*/}
<RepoSelector
value={value}
repositories={repositories}
recents={recents}
isLoading={!hasRepos && anyLoading}
organizationId={organizationId ?? null}
onChange={onChange}
disabled={disabled}
/>

{isCloneEntry ? null : (
<RepositoryBranchSelector
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,22 @@ describe('resolveProviderStatus', () => {
).toBe('loading');
});

it('returns loading while the provider query has produced no data yet', () => {
// A paused query (iOS boot / NetInfo probe not settled) reports
// `isLoading === false` and leaves `data` undefined, so
// `integrationInstalled` is undefined. That provider's list is not known
// yet and must never read as settled (`'repos'`), or the section renders a
// heading with no control under it.
expect(
resolveProviderStatus({
isLoading: false,
isError: false,
integrationInstalled: undefined,
repositoryCount: 0,
})
).toBe('loading');
});

it('returns error when the query failed with no cached repos', () => {
expect(
resolveProviderStatus({
Expand All @@ -39,11 +55,14 @@ describe('resolveProviderStatus', () => {
});

it('keeps cached repos visible after a background refetch error', () => {
// Cached rows and the integration flag come from the same query data, so a
// background refetch error that keeps its rows keeps a defined
// `integrationInstalled` too; `undefined` is reserved for "no data yet".
expect(
resolveProviderStatus({
isLoading: false,
isError: true,
integrationInstalled: undefined,
integrationInstalled: true,
repositoryCount: 3,
})
).toBe('repos');
Expand Down
12 changes: 10 additions & 2 deletions apps/mobile/src/components/agents/new-session-repository-state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,18 @@ export function resolveProviderStatus({
if (isError && repositoryCount === 0) {
return 'error';
}
if (integrationInstalled === false) {
// `undefined` means the query has produced no data at all, not "not
// installed": a paused query (iOS boot / the NetInfo probe not settled yet)
// reports `isLoading === false` and leaves `data` undefined, so the flag is
// undefined too. A provider whose list is not known yet must never read as
// settled, or the section renders its heading with no control under it.
if (integrationInstalled === undefined) {
return 'loading';
}
if (!integrationInstalled) {
return 'connect';
}
if (integrationInstalled === true && repositoryCount === 0) {
if (repositoryCount === 0) {
return 'connected-empty';
}
return 'repos';
Expand Down
57 changes: 57 additions & 0 deletions apps/mobile/src/lib/auth/auth-browser.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
import * as WebBrowser from 'expo-web-browser';
import { Platform } from 'react-native';

import { PRODUCTION_HOSTS } from '@/lib/url-contract';

/**
* Which browser API presents the device-auth page. The flow ends on the poll's
* approval rather than on the page's redirect, so the page must be closed from
* here, and only the API that opened it can close it: `dismissAuthSession` does
* nothing to a plain browser.
*/
export type AuthBrowserKind = 'auth-session' | 'plain-browser';

// iOS's ASWebAuthenticationSession raises a consent alert naming the auth URL's
// host ("<App>" Wants to Use "<host>" to Sign In). On a non-product host — a
// dev or preview stack — that alert shows the user a raw developer address
// instead of a domain they can recognize as Kilo, so only open the native auth
// session when the host is a product host. Otherwise present
// SFSafariViewController via openBrowserAsync: it shares Safari's cookies (an
// existing web session still applies) and raises no consent alert. The flow
// polls the server for approval and never consumes the session redirect, so
// nothing else changes. Android keeps openBrowserAsync because
// expo-web-browser's openAuthSessionAsync polyfill can get stuck (KILO-APP-22).
function isProductAuthHost(url: string): boolean {
try {
return PRODUCTION_HOSTS.includes(new URL(url).hostname);
} catch {
// An unparseable URL cannot be recognized as a product host, so avoid the
// consent alert and fall back to the plain browser.
return false;
}
}

export function resolveAuthBrowserKind(url: string): AuthBrowserKind {
return Platform.OS === 'android' || !isProductAuthHost(url) ? 'plain-browser' : 'auth-session';
}

export async function openAuthBrowser(url: string): Promise<void> {
await (resolveAuthBrowserKind(url) === 'plain-browser'
? WebBrowser.openBrowserAsync(url)
: WebBrowser.openAuthSessionAsync(url));
}

/**
* Close the page the flow opened, with the API that matches it. Android opened
* a Chrome custom tab, which the user returns from with the back gesture, and
* the flow has never dismissed it from the app.
*/
export function dismissAuthBrowser(kind: AuthBrowserKind): void {
if (kind === 'auth-session') {
WebBrowser.dismissAuthSession();
return;
}
if (Platform.OS === 'ios') {
void WebBrowser.dismissBrowser();
}
}
60 changes: 58 additions & 2 deletions apps/mobile/src/lib/auth/device-auth-poll.test.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,17 @@
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
import { dismissAuthSession, dismissBrowser } from 'expo-web-browser';

import { startDeviceAuthPoll } from '@/lib/auth/device-auth-poll';
import { type DeviceAuthState } from '@/lib/auth/device-auth-state';

const platformMock = vi.hoisted(() => ({ os: 'ios' as 'ios' | 'android' }));

vi.mock('react-native', () => ({
Platform: { OS: 'android' },
Platform: {
get OS() {
return platformMock.os;
},
},
}));

vi.mock('expo-application', () => ({
Expand All @@ -13,6 +20,7 @@ vi.mock('expo-application', () => ({

vi.mock('expo-web-browser', () => ({
dismissAuthSession: vi.fn(),
dismissBrowser: vi.fn(),
}));

vi.mock('@/lib/config', () => ({
Expand All @@ -21,11 +29,17 @@ vi.mock('@/lib/config', () => ({

const fetchMock = vi.fn();

function approvedResponse() {
return Response.json({ status: 'approved', token: 'access', expiresIn: 3600 });
}

function pendingResponse() {
return Response.json({ status: 'pending' }, { status: 202 });
}

function makePoll(overrides: { startedAt?: number } = {}) {
function makePoll(
overrides: { startedAt?: number; browserKind?: 'auth-session' | 'plain-browser' } = {}
) {
const setState = vi.fn<(updater: (prev: DeviceAuthState) => DeviceAuthState) => void>();
const cleanup = vi.fn<() => void>();
const poll = startDeviceAuthPoll({
Expand All @@ -34,6 +48,7 @@ function makePoll(overrides: { startedAt?: number } = {}) {
signal: new AbortController().signal,
setState,
cleanup,
browserKind: 'auth-session',
...overrides,
});
return { poll, setState, cleanup };
Expand All @@ -43,6 +58,8 @@ beforeEach(() => {
vi.useFakeTimers();
vi.setSystemTime(new Date('2026-01-01T00:00:00Z'));
fetchMock.mockReset();
vi.mocked(dismissAuthSession).mockClear();
vi.mocked(dismissBrowser).mockClear();
globalThis.fetch = fetchMock;
});

Expand Down Expand Up @@ -150,4 +167,43 @@ describe('startDeviceAuthPoll', () => {
expect(cleanup).toHaveBeenCalled();
expect(setState).toHaveBeenCalled();
});

it('dismisses the plain browser it opened when approval lands', async () => {
// A non-product auth host opens SFSafariViewController, which
// `dismissAuthSession` cannot close: without the matching dismissal the page
// stays over the approved app.
fetchMock.mockResolvedValue(approvedResponse());
const { setState } = makePoll({ browserKind: 'plain-browser' });

await vi.advanceTimersByTimeAsync(3000);

expect(vi.mocked(dismissBrowser)).toHaveBeenCalledTimes(1);
expect(vi.mocked(dismissAuthSession)).not.toHaveBeenCalled();
expect(setState).toHaveBeenCalled();
});

it('dismisses the native auth session it opened when approval lands', async () => {
fetchMock.mockResolvedValue(approvedResponse());
makePoll({ browserKind: 'auth-session' });

await vi.advanceTimersByTimeAsync(3000);

expect(vi.mocked(dismissAuthSession)).toHaveBeenCalledTimes(1);
expect(vi.mocked(dismissBrowser)).not.toHaveBeenCalled();
});

it('leaves the Android custom tab to the user when approval lands', async () => {
platformMock.os = 'android';
try {
fetchMock.mockResolvedValue(approvedResponse());
makePoll({ browserKind: 'plain-browser' });

await vi.advanceTimersByTimeAsync(3000);

expect(vi.mocked(dismissBrowser)).not.toHaveBeenCalled();
expect(vi.mocked(dismissAuthSession)).not.toHaveBeenCalled();
} finally {
platformMock.os = 'ios';
}
});
});
15 changes: 8 additions & 7 deletions apps/mobile/src/lib/auth/device-auth-poll.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
import { Platform } from 'react-native';
import * as WebBrowser from 'expo-web-browser';

import { i18n } from '@/i18n';
import { API_BASE_URL } from '@/lib/config';
import { type AuthBrowserKind, dismissAuthBrowser } from '@/lib/auth/auth-browser';
import { classifyPollResponse } from '@/lib/auth/poll-response';
import { buildClientMetadataHeaders } from '@/lib/client-metadata';
import {
Expand Down Expand Up @@ -31,9 +29,14 @@ export function startDeviceAuthPoll(params: {
signal: AbortSignal;
setState: (updater: (prev: DeviceAuthState) => DeviceAuthState) => void;
cleanup: () => void;
/**
* The API that opened the verification page. Approval ends the flow without
* waiting for the page, so the poll closes it with the matching dismissal.
*/
browserKind: AuthBrowserKind;
startedAt?: number;
}): DeviceAuthPollHandle {
const { code, deviceCode, signal, setState, cleanup } = params;
const { code, deviceCode, signal, setState, cleanup, browserKind } = params;

// A resumed transaction reuses the original start clock so its overall
// budget does not restart from `Date.now()` and outlive the server code.
Expand Down Expand Up @@ -94,9 +97,7 @@ export function startDeviceAuthPoll(params: {

if (parsed?.status === 'approved') {
cleanup();
if (Platform.OS !== 'android') {
WebBrowser.dismissAuthSession();
}
dismissAuthBrowser(browserKind);
setState(previous =>
approvedDeviceAuthState({
code,
Expand Down
Loading
Loading