-
Notifications
You must be signed in to change notification settings - Fork 176
✨(github): Add username-based installation filtering #3700
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
- Add getInstallationsForUsername function to filter installations by GitHub username - Add getInstallationsForUsername helper function - Check organization membership via GitHub API for org installations - Add createAppOctokit helper for app-level authentication - Update projects/new page to use username-based installation filtering - Derive GitHub username from Supabase user metadata and identities This enables filtering GitHub App installations to show only those relevant to the authenticated user (personal account or organizations they belong to). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
Updates to Preview Branch (feat/github-installations-username-filter) ↗︎
Tasks are run on every commit but only new migration files are pushed.
View logs for this Workflow Run ↗︎. |
WalkthroughThe projects/new page now derives a GitHub username from Supabase user metadata/identities and calls getInstallationsForUsername(githubLogin). The GitHub package adds app-level Octokit creation and a new getInstallationsForUsername flow that lists and filters app installations and exports those helpers. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Page as projects/new page
participant Auth as Supabase Session
participant GitHubAPI as github api.server.ts
User->>Page: Load
Page->>Auth: getSession()
alt session present
Page->>Page: Derive githubLogin from user.identities
alt githubLogin found
Page->>GitHubAPI: getInstallationsForUsername(githubLogin)
GitHubAPI-->>Page: installations[]
Page-->>User: Render ProjectNewPage with installations
else no githubLogin
Page-->>User: Redirect to login
end
else no session / error
Page-->>User: Redirect to login
end
sequenceDiagram
autonumber
participant Caller as getInstallationsForUsername
participant API as api.server.ts
participant AppKit as createAppOctokit
participant GitHub as GitHub API
Caller->>API: request(username)
API->>AppKit: createAppOctokit()
AppKit-->>API: appOctokit
API->>GitHub: List app installations (as app)
GitHub-->>API: installations[]
loop each installation
alt account.type == "User"
API->>API: compare installation.account.login == username
else account.type == "Organization"
API->>GitHub: request org membership for username as installation
alt member (200)
API->>API: include installation
else (404)
API->>API: exclude installation
end
end
end
API-->>Caller: filtered installations[]
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
frontend/apps/app/app/projects/new/page.tsx
(2 hunks)frontend/internal-packages/github/src/api.server.ts
(2 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Name utility files in camelCase (e.g., mergeSchema.ts)
Files:
frontend/internal-packages/github/src/api.server.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript/TSX across the codebase
**/*.{ts,tsx}
: Prefer early returns for readability
Use named exports only (no default exports)
Prefer const arrow functions over function declarations for simple utilities (e.g., const toggle = () => {})
Files:
frontend/internal-packages/github/src/api.server.ts
frontend/apps/app/app/projects/new/page.tsx
frontend/internal-packages/**
📄 CodeRabbit inference engine (AGENTS.md)
Infra and tooling (e2e, configs, storybook, agent) live under frontend/internal-packages
Files:
frontend/internal-packages/github/src/api.server.ts
frontend/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Follow existing import patterns and tsconfig path aliases
Files:
frontend/internal-packages/github/src/api.server.ts
frontend/apps/app/app/projects/new/page.tsx
**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Name React component files in PascalCase and use TSX (e.g., App.tsx)
Prefix event handler functions with “handle” (e.g., handleClick)
Files:
frontend/apps/app/app/projects/new/page.tsx
frontend/apps/**
📄 CodeRabbit inference engine (AGENTS.md)
Next.js apps live under frontend/apps; target app-specific scripts and configs there
Files:
frontend/apps/app/app/projects/new/page.tsx
frontend/apps/**/app/**/page.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
Do not implement page logic directly in page.tsx; create separate page components
Files:
frontend/apps/app/app/projects/new/page.tsx
frontend/apps/**/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
frontend/apps/**/app/**/*.{ts,tsx}
: Use Server Components for server-side data fetching
Do client-side data fetching only when necessary
Align data fetching responsibilities with component roles
Use Server Actions for all data mutations (create/update/delete)
Files:
frontend/apps/app/app/projects/new/page.tsx
🧬 Code graph analysis (2)
frontend/internal-packages/github/src/api.server.ts (1)
frontend/internal-packages/github/src/types.ts (1)
Installation
(3-3)
frontend/apps/app/app/projects/new/page.tsx (1)
frontend/internal-packages/github/src/api.server.ts (1)
getInstallationsForUsername
(68-118)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: frontend-ci
- GitHub Check: Supabase Preview
- GitHub Check: security-review
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 | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
…lations - Remove username-based filtering logic temporarily for debugging - Comment out organization membership check to simplify implementation - Return all installations regardless of username match - Remove unused getInstallationsForOwners function - Simplify username derivation in page.tsx to only use identities This commit temporarily removes filtering to debug installation visibility issues. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
frontend/apps/app/app/projects/new/page.tsx
(2 hunks)frontend/internal-packages/github/src/api.server.ts
(2 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Name utility files in camelCase (e.g., mergeSchema.ts)
Files:
frontend/internal-packages/github/src/api.server.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript/TSX across the codebase
**/*.{ts,tsx}
: Prefer early returns for readability
Use named exports only (no default exports)
Prefer const arrow functions over function declarations for simple utilities (e.g., const toggle = () => {})
Files:
frontend/internal-packages/github/src/api.server.ts
frontend/apps/app/app/projects/new/page.tsx
frontend/internal-packages/**
📄 CodeRabbit inference engine (AGENTS.md)
Infra and tooling (e2e, configs, storybook, agent) live under frontend/internal-packages
Files:
frontend/internal-packages/github/src/api.server.ts
frontend/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Follow existing import patterns and tsconfig path aliases
Files:
frontend/internal-packages/github/src/api.server.ts
frontend/apps/app/app/projects/new/page.tsx
**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Name React component files in PascalCase and use TSX (e.g., App.tsx)
Prefix event handler functions with “handle” (e.g., handleClick)
Files:
frontend/apps/app/app/projects/new/page.tsx
frontend/apps/**
📄 CodeRabbit inference engine (AGENTS.md)
Next.js apps live under frontend/apps; target app-specific scripts and configs there
Files:
frontend/apps/app/app/projects/new/page.tsx
frontend/apps/**/app/**/page.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
Do not implement page logic directly in page.tsx; create separate page components
Files:
frontend/apps/app/app/projects/new/page.tsx
frontend/apps/**/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
frontend/apps/**/app/**/*.{ts,tsx}
: Use Server Components for server-side data fetching
Do client-side data fetching only when necessary
Align data fetching responsibilities with component roles
Use Server Actions for all data mutations (create/update/delete)
Files:
frontend/apps/app/app/projects/new/page.tsx
🧬 Code graph analysis (2)
frontend/internal-packages/github/src/api.server.ts (1)
frontend/internal-packages/github/src/types.ts (1)
Installation
(3-3)
frontend/apps/app/app/projects/new/page.tsx (1)
frontend/internal-packages/github/src/api.server.ts (1)
getInstallationsForUsername
(36-87)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Supabase Preview
- GitHub Check: Supabase Preview
- GitHub Check: frontend-ci
- GitHub Check: Supabase Preview
export const getInstallationsForUsername = async ( | ||
username: string, | ||
): Promise<{ installations: Installation[] }> => { | ||
const appOctokit = await createAppOctokit() | ||
const _normalizedUsername = username.toLowerCase() | ||
|
||
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') { | ||
// // Authenticate as the installation to check membership for the user directly | ||
// const installationOctokit = await createOctokit(installation.id) | ||
// const membershipResult = await fromPromise( | ||
// installationOctokit.request('GET /orgs/{org}/members/{username}', { | ||
// org: accountLogin, | ||
// username, | ||
// }), | ||
// ) | ||
|
||
// // If the request succeeds, the user is a member | ||
// if (membershipResult.isOk()) { | ||
// matchedInstallations.push(installation) | ||
// } | ||
// // Errors (e.g., 404, permission) are treated as non-membership | ||
// } | ||
// } | ||
|
||
return { installations: allInstallations } | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Filtering currently disabled — every installation leaks through
Right now getInstallationsForUsername
just returns allInstallations
and never applies any username/org membership filtering (the entire block is commented out). That defeats the purpose of this PR and exposes installations the caller shouldn’t see. Please reinstate the filtering logic and run the org membership checks with bounded concurrency (e.g., Promise.allSettled
) so we don’t regress latency while still returning only the installations tied to the requesting user.
- const appOctokit = await createAppOctokit()
- const _normalizedUsername = username.toLowerCase()
-
- const allInstallations = (await appOctokit.paginate(
- appOctokit.request,
- 'GET /app/installations',
- )) as Installation[]
-
- return { installations: allInstallations }
+ const appOctokit = await createAppOctokit()
+ const normalizedUsername = username.trim().toLowerCase()
+
+ const allInstallations = (await appOctokit.paginate(
+ appOctokit.request,
+ 'GET /app/installations',
+ )) as Installation[]
+
+ const checks = await Promise.allSettled(
+ allInstallations.map(async (installation) => {
+ const account = installation.account as {
+ type?: string
+ login?: string
+ } | null
+ const accountLogin = account?.login
+ const accountType = account?.type
+
+ if (!accountLogin || !accountType) return null
+
+ if (accountType === 'User') {
+ return accountLogin.toLowerCase() === normalizedUsername
+ ? installation
+ : null
+ }
+
+ if (accountType === 'Organization') {
+ const installationOctokit = await createOctokit(installation.id)
+ const membershipResult = await fromPromise(
+ installationOctokit.request('GET /orgs/{org}/members/{username}', {
+ org: accountLogin,
+ username,
+ }),
+ )
+ return membershipResult.isOk() ? installation : null
+ }
+
+ return null
+ }),
+ )
+
+ const matchedInstallations = checks.reduce<Installation[]>(
+ (acc, result) => {
+ if (result.status === 'fulfilled' && result.value) {
+ acc.push(result.value)
+ }
+ return acc
+ },
+ [],
+ )
+
+ return { installations: matchedInstallations }
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
export const getInstallationsForUsername = async ( | |
username: string, | |
): Promise<{ installations: Installation[] }> => { | |
const appOctokit = await createAppOctokit() | |
const _normalizedUsername = username.toLowerCase() | |
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') { | |
// // Authenticate as the installation to check membership for the user directly | |
// const installationOctokit = await createOctokit(installation.id) | |
// const membershipResult = await fromPromise( | |
// installationOctokit.request('GET /orgs/{org}/members/{username}', { | |
// org: accountLogin, | |
// username, | |
// }), | |
// ) | |
// // If the request succeeds, the user is a member | |
// if (membershipResult.isOk()) { | |
// matchedInstallations.push(installation) | |
// } | |
// // Errors (e.g., 404, permission) are treated as non-membership | |
// } | |
// } | |
return { installations: allInstallations } | |
} | |
export const getInstallationsForUsername = async ( | |
username: string, | |
): Promise<{ installations: Installation[] }> => { | |
const appOctokit = await createAppOctokit() | |
const normalizedUsername = username.trim().toLowerCase() | |
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') { | |
// // Authenticate as the installation to check membership for the user directly | |
// const installationOctokit = await createOctokit(installation.id) | |
// const membershipResult = await fromPromise( | |
// installationOctokit.request('GET /orgs/{org}/members/{username}', { | |
// org: accountLogin, | |
// username, | |
// }), | |
// ) | |
// | |
// // If the request succeeds, the user is a member | |
// if (membershipResult.isOk()) { | |
// matchedInstallations.push(installation) | |
// } | |
// // Errors (e.g., 404, permission) are treated as non-membership | |
// } | |
// } | |
const checks = await Promise.allSettled( | |
allInstallations.map(async (installation) => { | |
const account = installation.account as { | |
type?: string | |
login?: string | |
} | null | |
const accountLogin = account?.login | |
const accountType = account?.type | |
if (!accountLogin || !accountType) return null | |
if (accountType === 'User') { | |
return accountLogin.toLowerCase() === normalizedUsername | |
? installation | |
: null | |
} | |
if (accountType === 'Organization') { | |
const installationOctokit = await createOctokit(installation.id) | |
const membershipResult = await fromPromise( | |
installationOctokit.request('GET /orgs/{org}/members/{username}', { | |
org: accountLogin, | |
username, | |
}), | |
) | |
return membershipResult.isOk() ? installation : null | |
} | |
return null | |
}), | |
) | |
const matchedInstallations = checks.reduce<Installation[]>( | |
(acc, result) => { | |
if (result.status === 'fulfilled' && result.value) { | |
acc.push(result.value) | |
} | |
return acc | |
}, | |
[], | |
) | |
return { installations: matchedInstallations } | |
} |
…g logging - Change API endpoint from /app/installations to /installation/repositories - Restore filtering logic with username matching - Add console.info debug logging for matched accounts and usernames - Keep returning allInstallations for debugging (not filtered yet) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Issue
Why is this change needed?
This change adds username-based filtering for GitHub App installations to ensure users only see installations relevant to their account (personal or organizations they belong to).
Previously, all installations were displayed without filtering, potentially showing installations the user doesn't have access to. This implementation:
The implementation adds two helper functions:
getInstallationsForUsername
: Filters by specific username with org membership checkscreateAppOctokit
: Creates app-level authenticated Octokit clientSummary by CodeRabbit
New Features
Bug Fixes