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 @@ -211,6 +211,15 @@ describe('NewSessionRepositorySection connect card open action', () => {
expect(label?.props.numberOfLines, key).toBe(1);
}
});

it('offers no open label on the connected-empty card', () => {
// The connected-empty card replaces the open action with its guidance, so
// it must leave no empty gap where the button used to be.
const renderer = mountSection({ groups: [group('gitlab', 'connected-empty')] });

expect(renderedText(renderer)).toContain(i18n.t('agentChat.newSession.gitlabConnected'));
expect(labelNode(renderer, i18n.t('agentChat.newSession.openGitlab'))).toBeUndefined();
});
});

describe('NewSessionRepositorySection connect card collapse', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,8 @@ export function NewSessionRepositorySection({
the label the free space keeps its box wider than the text, and
`numberOfLines` pins the line. The explorer captures
new-session-filled and new-session-kb-down both showed that wrap.
The logical `me-[24px]` inset keeps the centered label clear of the
leading glyph in RTL as well, which the physical `mr-` would not.
*/}
<Text className="me-[24px] flex-1 text-center" numberOfLines={1}>
{t(selectedRepository === null ? copy.openLabel : copy.connectTitle)}
Expand Down
9 changes: 8 additions & 1 deletion apps/mobile/src/lib/screen-insets.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,9 @@ describe('screen side insets: one implementation for both platforms', () => {
expect(profile, `${PROFILE_SCREEN} imports the native safe-area module again`).not.toMatch(
SAFE_AREA_MODULE
);
const alignment = alignmentPath(profile);
// `alignmentPath` starts at the `useScreenSideInsets` line, so this also
// proves the path it scans is the screen's insets read, not an empty slice.
// The screen carries no platform branch at all: sign-out confirms through
// the one shared alert on both platforms, so the insets path — and every
// other line — stays a single implementation. Only the lines that carry the
Expand All @@ -109,7 +112,11 @@ describe('screen side insets: one implementation for both platforms', () => {
// hook call to the line that first applies a side inset. Keep both so a
// fork is caught whichever way the insets are spelled.
expect(
alignmentPath(profile),
alignment,
`${PROFILE_SCREEN} does not read its side insets from the entry point`
).toMatch(/useScreenSideInsets/);
expect(
alignment,
`${PROFILE_SCREEN}'s alignment path carries a per-platform branch`
).not.toMatch(PLATFORM_BRANCH);
});
Expand Down
228 changes: 228 additions & 0 deletions apps/web/src/app/cloud-agent-fork/review-md/route.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,228 @@
import { beforeAll, beforeEach, describe, expect, it, jest } from '@jest/globals';
import { NextRequest } from 'next/server';

type TrpcContextFixture = {
user: {
id: string;
};
};

type ReviewConfigFixture = {
customInstructions: string | null;
modelSlug: string | null;
};

type RepositoriesFixture = {
integrationInstalled: boolean;
repositories: { id: number; name: string; fullName: string; private: boolean }[];
syncedAt: string | null;
};

type PrepareSessionOutput = {
kiloSessionId: string;
};

type RouteGet = (request: NextRequest) => Promise<Response>;

const mockCreateTRPCContext = jest.fn<() => Promise<TrpcContextFixture>>();
const mockPersonalGetReviewConfig =
jest.fn<(input: { platform: string }) => Promise<ReviewConfigFixture>>();
const mockOrganizationGetReviewConfig =
jest.fn<(input: { organizationId: string; platform: string }) => Promise<ReviewConfigFixture>>();
const mockPersonalListGitHubRepositories =
jest.fn<(input: { forceRefresh: boolean }) => Promise<RepositoriesFixture>>();
const mockPersonalListGitLabRepositories =
jest.fn<(input: { forceRefresh: boolean }) => Promise<RepositoriesFixture>>();
const mockOrganizationListGitHubRepositories =
jest.fn<
(input: { organizationId: string; forceRefresh: boolean }) => Promise<RepositoriesFixture>
>();
const mockOrganizationListGitLabRepositories =
jest.fn<
(input: { organizationId: string; forceRefresh: boolean }) => Promise<RepositoriesFixture>
>();
const mockPersonalPrepareSession = jest.fn<() => Promise<PrepareSessionOutput>>();
const mockOrganizationPrepareSession = jest.fn<() => Promise<PrepareSessionOutput>>();

const mockCaller = {
organizations: {
reviewAgent: {
getReviewConfig: mockOrganizationGetReviewConfig,
listGitHubRepositories: mockOrganizationListGitHubRepositories,
listGitLabRepositories: mockOrganizationListGitLabRepositories,
},
cloudAgentNext: {
prepareSession: mockOrganizationPrepareSession,
},
},
personalReviewAgent: {
getReviewConfig: mockPersonalGetReviewConfig,
listGitHubRepositories: mockPersonalListGitHubRepositories,
listGitLabRepositories: mockPersonalListGitLabRepositories,
},
cloudAgentNext: {
prepareSession: mockPersonalPrepareSession,
},
};
const mockCreateCaller = jest.fn((_: TrpcContextFixture) => mockCaller);
const mockCreateCallerFactory = jest.fn(() => mockCreateCaller);

jest.mock('@/lib/trpc/init', () => ({
createTRPCContext: () => mockCreateTRPCContext(),
createCallerFactory: () => mockCreateCallerFactory(),
}));

jest.mock('@/routers/root-router', () => ({
rootRouter: {},
}));

jest.mock('@/lib/posthog-feature-flags', () => ({
isFeatureFlagEnabledOrDevelopment: () => Promise.resolve(true),
}));

jest.mock('@/lib/redis', () => ({
redisClient: {
incr: () => Promise.resolve(1),
expire: () => Promise.resolve(1),
},
}));

jest.mock('@/lib/ai-gateway/models', () => ({
PRIMARY_DEFAULT_MODEL: 'test/primary-default-model',
}));

let getRoute: RouteGet;

const USER_ID = 'user_1';
const ORG_ID = '11111111-1111-4111-8111-111111111111';
const PERSONAL_KILO_SESSION_ID = 'ses_12345678901234567890123456';
const ORG_KILO_SESSION_ID = 'ses_abcdefabcdefabcdefabcdefab';

const repositories: RepositoriesFixture = {
integrationInstalled: true,
repositories: [{ id: 1, name: 'repo', fullName: 'owner/repo', private: false }],
syncedAt: '2024-01-01T00:00:00Z',
};

const syncedEmptyRepositories: RepositoriesFixture = {
integrationInstalled: true,
repositories: [],
syncedAt: '2024-01-01T00:00:00Z',
};

function makeRequest(params: {
platform?: string;
repo?: string;
organizationId?: string;
secFetchSite?: string | null;
}): NextRequest {
const url = new URL('https://kilo.test/cloud-agent-fork/review-md');
if (params.platform) url.searchParams.set('platform', params.platform);
if (params.repo) url.searchParams.set('repo', params.repo);
if (params.organizationId) url.searchParams.set('organizationId', params.organizationId);

const headers = new Headers();
const secFetchSite = params.secFetchSite === undefined ? 'same-origin' : params.secFetchSite;
if (secFetchSite !== null) headers.set('sec-fetch-site', secFetchSite);

return new NextRequest(url.toString(), { headers });
}

function getRedirectUrl(response: Response): URL {
const location = response.headers.get('location');
expect(location).toBeTruthy();
// This route answers with a relative Location on purpose (the dev server's
// origin differs from the browser's), so resolve it against the request origin.
return new URL(location ?? '', 'https://kilo.test');
}

function expectErrorRedirect(response: Response, error: string) {
const redirectUrl = getRedirectUrl(response);
expect(`${redirectUrl.pathname}${redirectUrl.search}`).toBe(`/code-reviews?error=${error}`);
}

describe('GET /cloud-agent-fork/review-md', () => {
beforeAll(async () => {
({ GET: getRoute } = await import('./route'));
});

beforeEach(() => {
jest.clearAllMocks();
mockCreateTRPCContext.mockResolvedValue({ user: { id: USER_ID } });
mockPersonalGetReviewConfig.mockResolvedValue({
customInstructions: 'Follow the repository conventions.',
modelSlug: null,
});
mockOrganizationGetReviewConfig.mockResolvedValue({
customInstructions: 'Follow the repository conventions.',
modelSlug: null,
});
mockPersonalListGitHubRepositories.mockResolvedValue(repositories);
mockPersonalListGitLabRepositories.mockResolvedValue(repositories);
mockOrganizationListGitHubRepositories.mockResolvedValue(repositories);
mockOrganizationListGitLabRepositories.mockResolvedValue(repositories);
mockPersonalPrepareSession.mockResolvedValue({ kiloSessionId: PERSONAL_KILO_SESSION_ID });
mockOrganizationPrepareSession.mockResolvedValue({ kiloSessionId: ORG_KILO_SESSION_ID });
});

it('forces a provider read for the personal GitHub allowlist', async () => {
const response = await getRoute(makeRequest({ platform: 'github', repo: 'owner/repo' }));

// The allowlist must not answer from a synced (possibly empty) snapshot.
expect(mockPersonalListGitHubRepositories).toHaveBeenCalledWith({ forceRefresh: true });
expect(mockPersonalPrepareSession).toHaveBeenCalledTimes(1);
expect(response.status).toBe(303);
});

it('forces a provider read for the personal GitLab allowlist', async () => {
const response = await getRoute(makeRequest({ platform: 'gitlab', repo: 'owner/repo' }));

expect(mockPersonalListGitLabRepositories).toHaveBeenCalledWith({ forceRefresh: true });
expect(mockPersonalPrepareSession).toHaveBeenCalledTimes(1);
expect(response.status).toBe(303);
});

it('forces a provider read for the organization GitHub allowlist', async () => {
const response = await getRoute(
makeRequest({ platform: 'github', repo: 'owner/repo', organizationId: ORG_ID })
);

expect(mockOrganizationListGitHubRepositories).toHaveBeenCalledWith({
organizationId: ORG_ID,
forceRefresh: true,
});
expect(mockOrganizationPrepareSession).toHaveBeenCalledTimes(1);
expect(response.status).toBe(303);
});

it('forces a provider read for the organization GitLab allowlist', async () => {
const response = await getRoute(
makeRequest({ platform: 'gitlab', repo: 'owner/repo', organizationId: ORG_ID })
);

expect(mockOrganizationListGitLabRepositories).toHaveBeenCalledWith({
organizationId: ORG_ID,
forceRefresh: true,
});
expect(mockOrganizationPrepareSession).toHaveBeenCalledTimes(1);
expect(response.status).toBe(303);
});

it('still rejects a repository the provider does not report', async () => {
mockPersonalListGitHubRepositories.mockResolvedValue(syncedEmptyRepositories);

const response = await getRoute(makeRequest({ platform: 'github', repo: 'owner/repo' }));

expectErrorRedirect(response, 'repository_not_allowed');
expect(mockPersonalPrepareSession).not.toHaveBeenCalled();
});

it('rejects a cross-site request before reading repositories', async () => {
const response = await getRoute(
makeRequest({ platform: 'github', repo: 'owner/repo', secFetchSite: 'cross-site' })
);

expectErrorRedirect(response, 'invalid_conversion_request');
expect(mockPersonalListGitHubRepositories).not.toHaveBeenCalled();
});
});
11 changes: 7 additions & 4 deletions apps/web/src/app/cloud-agent-fork/review-md/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -174,19 +174,22 @@ export async function GET(request: NextRequest) {
// integration actually covers them), so they must NOT gate this credit-spending, PR-opening
// action — otherwise a user could add an arbitrary full_name to their config and pass this
// check. The worker enforces installation scope regardless; this keeps the fail-fast honest.
// forceRefresh keeps the allowlist off the cached snapshot: a synced-empty cache (connect-time
// sync stored zero repositories, then access was granted) would otherwise reject a repo the
// provider now reports, and GitLab never invalidates that cache.
const repositoryList = organizationId
? platform === 'gitlab'
? await caller.organizations.reviewAgent.listGitLabRepositories({
organizationId,
forceRefresh: false,
forceRefresh: true,
})
: await caller.organizations.reviewAgent.listGitHubRepositories({
organizationId,
forceRefresh: false,
forceRefresh: true,
})
: platform === 'gitlab'
? await caller.personalReviewAgent.listGitLabRepositories({ forceRefresh: false })
: await caller.personalReviewAgent.listGitHubRepositories({ forceRefresh: false });
? await caller.personalReviewAgent.listGitLabRepositories({ forceRefresh: true })
: await caller.personalReviewAgent.listGitHubRepositories({ forceRefresh: true });

const allowedRepoFullNames = buildAllowedRepositoryFullNames(repositoryList.repositories, []);
if (!allowedRepoFullNames.has(repo)) {
Expand Down
25 changes: 25 additions & 0 deletions apps/web/src/lib/cloud-agent/github-integration-helpers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,18 @@ describe('github-integration-helpers', () => {
expect(mockFetchGitHubRepositories).not.toHaveBeenCalled();
});

it('returns a synced empty list as the connected-empty snapshot', async () => {
mockGetIntegrationForOwner.mockResolvedValue(buildIntegration({ repositories: [] }));

const { fetchGitHubRepositoriesForUser } = await import('./github-integration-helpers');
const result = await fetchGitHubRepositoriesForUser('user-123');

expect(result.integrationInstalled).toBe(true);
expect(result.repositories).toEqual([]);
expect(mockFetchGitHubRepositories).not.toHaveBeenCalled();
expect(mockUpdateRepositoriesForIntegration).not.toHaveBeenCalled();
});

it('returns integrationInstalled false when no integration exists', async () => {
mockGetIntegrationForOwner.mockResolvedValue(null);

Expand Down Expand Up @@ -217,6 +229,19 @@ describe('github-integration-helpers', () => {
expect(mockFetchGitHubRepositories).not.toHaveBeenCalled();
});

it('returns a synced empty list as the connected-empty snapshot', async () => {
mockGetIntegrationsByOrganization.mockResolvedValue([buildIntegration({ repositories: [] })]);

const { fetchAllGitHubRepositoriesForOrganization } =
await import('./github-integration-helpers');
const result = await fetchAllGitHubRepositoriesForOrganization('org-123');

expect(result.integrationInstalled).toBe(true);
expect(result.repositories).toEqual([]);
expect(mockFetchGitHubRepositories).not.toHaveBeenCalled();
expect(mockUpdateRepositoriesForIntegration).not.toHaveBeenCalled();
});

it('preserves installation provenance across multiple GitHub organizations', async () => {
mockGetIntegrationsByOrganization.mockResolvedValue([
buildIntegration({
Expand Down
Loading
Loading