Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
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
40 changes: 38 additions & 2 deletions frontend/apps/app/app/projects/new/page.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { getInstallations } from '@liam-hq/github'
import { getInstallationsForUsername } from '@liam-hq/github'
import { redirect } from 'next/navigation'
import { ProjectNewPage } from '../../../components/ProjectNewPage'
import { getOrganizationId } from '../../../features/organizations/services/getOrganizationId'
Expand Down Expand Up @@ -31,7 +31,43 @@ export default async function NewProjectPage() {
redirect(urlgen('login'))
}

const { installations } = await getInstallations(data.session)
// Derive GitHub username from Supabase user metadata (GitHub provider) without using `any`
const isRecord = (value: unknown): value is Record<string, unknown> =>
typeof value === 'object' && value !== null

const usernameFromUserMetadata = (() => {
const userMetadata = user.user_metadata as unknown
if (isRecord(userMetadata)) {
const userNameField = userMetadata['user_name']
if (typeof userNameField === 'string') return userNameField
}
return undefined
})()

const usernameFromIdentities = (() => {
const identities = Array.isArray(user.identities) ? user.identities : []
const githubIdentity = identities.find(
(identity) =>
identity &&
typeof identity.provider === 'string' &&
identity.provider === 'github',
)
const identityData = githubIdentity?.identity_data as unknown
if (isRecord(identityData)) {
const userNameField = identityData['user_name']
if (typeof userNameField === 'string') return userNameField
}
return undefined
})()

const githubLogin = usernameFromUserMetadata ?? usernameFromIdentities

if (!githubLogin) {
console.error('GitHub login not found on user metadata')
redirect(urlgen('login'))
}

const { installations } = await getInstallationsForUsername(githubLogin)

return (
<ProjectNewPage
Expand Down
98 changes: 97 additions & 1 deletion frontend/internal-packages/github/src/api.server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
import { createAppAuth } from '@octokit/auth-app'
import { Octokit } from '@octokit/rest'
import { err, type Result, type ResultAsync } from 'neverthrow'
import type { GitHubContentItem } from './types'
import type { GitHubContentItem, Installation } from './types'

const createOctokit = async (installationId: number) => {
const octokit = new Octokit({
Expand All @@ -21,6 +21,102 @@ const createOctokit = async (installationId: number) => {
return octokit
}

const createAppOctokit = async () => {
const octokit = new Octokit({
authStrategy: createAppAuth,
auth: {
appId: process.env['GITHUB_APP_ID'],
privateKey: process.env['GITHUB_PRIVATE_KEY']?.replace(/\\n/g, '\n'),
},
})

return octokit
}

export const getInstallationsForOwners = async (
ownerLogins: string[],
): Promise<{ installations: Installation[] }> => {
const appOctokit = await createAppOctokit()

const allInstallations = (await appOctokit.paginate(
appOctokit.request,
'GET /app/installations',
)) as Installation[]

if (!ownerLogins || ownerLogins.length === 0) {
return { installations: allInstallations as Installation[] }
}

const allowedOwnerLogins = new Set(
ownerLogins.map((login) => login.toLowerCase()),
)

const filteredInstallations = allInstallations.filter(
(installation: Installation) => {
const accountLogin = (
installation.account as { login?: string } | null
)?.login
return accountLogin
? allowedOwnerLogins.has(accountLogin.toLowerCase())
: false
},
) as Installation[]

return { installations: filteredInstallations }
}

export const getInstallationsForUsername = async (
username: string,
): Promise<{ installations: Installation[] }> => {
const appOctokit = await createAppOctokit()

const allInstallations = (await appOctokit.paginate(
appOctokit.request,
'GET /app/installations',
)) as Installation[]

const normalizedUsername = username.toLowerCase()

const matchedInstallations: Installation[] = []

for (const installation of allInstallations) {
const account = installation.account as
| { type?: string; login?: string }
| null
const accountLogin = account?.login
const accountType = account?.type

if (!accountLogin || !accountType) continue

if (accountType === 'User') {
if (accountLogin.toLowerCase() === normalizedUsername) {
matchedInstallations.push(installation)
}
continue
}

if (accountType === 'Organization') {
try {
// Authenticate as the installation to check membership for the user directly
const installationOctokit = await createOctokit(installation.id)
await installationOctokit.request(
'GET /orgs/{org}/members/{username}',
{
org: accountLogin,
username,
},
)
// If the request succeeds, the user is a member
matchedInstallations.push(installation)
} catch {
// 404 or permission issues -> treat as not a member
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Avoid serial membership checks to prevent large latency regression.

Every organization installation now triggers its own GET /orgs/{org}/members/{username} call inside a for…of with await on each iteration, so requests run strictly one-by-one. With a user belonging to dozens of org installations this turns into dozens of sequential GitHub round-trips, far slower than the previous flow (single GET /user/installations using the user token). Please batch these checks with Promise.all/Promise.allSettled (or another bounded-concurrency strategy) so the per-user latency stays near constant instead of growing linearly with the installation count.

🤖 Prompt for AI Agents
In frontend/internal-packages/github/src/api.server.ts around lines 82 to 114,
the code performs organization membership checks sequentially inside a for…of
with await, causing high latency for many org installations; instead map the org
installations to membership-check promises (each creating its
installationOctokit and calling GET /orgs/{org}/members/{username}) and run them
concurrently with Promise.all or Promise.allSettled (or use a small
bounded-concurrency pool if you need to limit parallelism), then iterate the
results to push installations that succeeded into matchedInstallations and
ignore failures (treat them as non-members) while preserving the existing
error-handling semantics.

}

return { installations: matchedInstallations }
}

export const getPullRequestDetails = async (
installationId: number,
owner: string,
Expand Down
Loading