Revamp github import flow #3029
Conversation
WalkthroughThis PR transitions the GitHub import flow from context-driven multi-step state management to an OAuth-based repository discovery system. Key changes include removing the centralized Changes
Sequence DiagramssequenceDiagram
actor User
participant Web as Web Client
participant Server as Server
participant Supabase as Supabase
participant GitHub as GitHub
rect rgba(100, 150, 255, 0.1)
note over Web,GitHub: Old Flow (Removed)
User->>Web: Visit /import/github
Web->>Web: Check installation via hook
Web->>GitHub: Redirect to install app
GitHub->>Web: Callback with installation_id
Web->>Web: Multi-step UI (Connect → Select → Finalize)
Web->>Web: Create sandbox & project
end
rect rgba(150, 200, 100, 0.1)
note over Web,GitHub: New Flow (OAuth-based)
User->>Web: Visit /import/github
Server->>Supabase: Check session
alt No OAuth Token
Web->>User: Show OAuthConnect UI
User->>Web: Click Connect
Web->>Server: connectGitHubOAuth()
Server->>GitHub: Request OAuth
GitHub->>User: Authorize scopes
GitHub->>Server: Return code
Server->>Supabase: Link OAuth identity
Supabase->>Server: Confirm linked
end
Server->>Web: Prefetch repos with OAuth token
Web->>Web: Show setup.tsx (repository selection)
User->>Web: Select repo & branch
Web->>Server: validateWithOAuth(owner, repo)
Server->>GitHub: Check access with OAuth token
GitHub->>Server: Return branch & access info
Web->>Web: Navigate to importing page
Web->>Server: createSandbox() + createProject()
Server->>Web: Success, redirect to project
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes This refactor introduces significant structural changes: removal of context-based state management across multiple files, introduction of OAuth-based flows requiring tRPC mutations, restructuring of page layouts and components, and server-side session validation. While individual file changes follow consistent patterns (context removal, hook consolidation into tRPC calls), the breadth of affected files, interdependencies between removed and added components, and the density of logic changes in setup.tsx and the github router necessitate careful review of control flow, error handling, and data flow across the OAuth and repository validation paths. Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
|
Updates to Preview Branch (github-revamp) ↗︎
Tasks are run on every commit but only new migration files are pushed.
View logs for this Workflow Run ↗︎. |
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
| setImportError(null); // Clear any previous errors | ||
| }; | ||
|
|
||
| const handleImport = async () => { |
There was a problem hiding this comment.
The function handleImport is defined but never used; remove it or integrate its repository validation logic to avoid dead code.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
apps/web/client/src/app/projects/import/local/_components/select-folder.tsx (1)
1-16: Add 'use client' directive.This component uses client-only features (useState, useCallback, useRef, onClick handlers, useRouter) but is missing the 'use client' directive at the top of the file.
As per coding guidelines
Apply this diff to add the directive:
+ +'use client'; + import { ProcessedFileType, type NextJsProjectValidation,apps/web/client/src/app/callback/github/install/page.tsx (2)
22-79: Add missing dependencies to useEffect or justify the empty array.The useEffect depends on
searchParamsandhandleInstallationCallbackbut has an empty dependency array. This violates the React exhaustive-deps rule and could cause stale closures or unexpected behavior if the component re-renders with different values.If running only once on mount is intentional, add an ESLint disable comment with justification:
- useEffect(() => { + // eslint-disable-next-line react-hooks/exhaustive-deps -- Only run once on mount to process callback + useEffect(() => {Otherwise, include the dependencies:
- }, []); + }, [searchParams, handleInstallationCallback]);
67-70: window.close() will silently fail because the callback page is reached via server-side redirect, not window.open().The GitHub OAuth flow uses
redirect(data.url)inapps/web/client/src/app/projects/import/github/actions.ts, which is a server-side redirect. This means the callback page loads in the normal browser navigation flow, not via a programmatically opened window. Browsers only allow closing windows that were opened viawindow.open(), so this call will fail silently.The suggested fallback approach is appropriate—check the return value and redirect to a completion page if closing fails:
setTimeout(() => { - // Close the tab since we are using a new tab - window.close(); + // Try to close the tab/window + const closed = window.close(); + // If closing failed, redirect to a completion page + if (!closed) { + router.push(Routes.IMPORT_GITHUB); + } }, 3000);apps/web/client/src/server/api/routers/github.ts (1)
171-186: State must be unguessable and single-use; don’t use user ID.Using
ctx.user.idasstateis predictable and replayable. Switch to a signed, expiring nonce (HMAC with server secret) and verify on callback. Also avoid logging PII in state mismatch.- generateInstallationUrl: protectedProcedure + generateInstallationUrl: protectedProcedure .input( z .object({ - redirectUrl: z.string().optional(), + redirectUrl: z.string().url().optional(), }) .optional(), ) .mutation(async ({ input, ctx }) => { - const { url, state } = generateInstallationUrl({ - redirectUrl: input?.redirectUrl, - state: ctx.user.id, // Use user ID as state for CSRF protection - }); - return { url, state }; + const state = createSignedState({ uid: ctx.user.id, ttlMs: 10 * 60 * 1000 }); + const { url } = generateInstallationUrl({ redirectUrl: input?.redirectUrl, state }); + return { url, state }; }), @@ - // Validate state parameter matches current user ID for CSRF protection - if (input.state && input.state !== ctx.user.id) { - console.error('State mismatch:', { expected: ctx.user.id, received: input.state }); - throw new TRPCError({ - code: 'BAD_REQUEST', - message: 'Invalid state parameter', - }); - } + // Validate signed/expiring state + const payload = verifySignedState(input.state); + if (!payload || payload.uid !== ctx.user.id) { + console.error('State verification failed'); + throw new TRPCError({ code: 'BAD_REQUEST', message: 'Invalid state parameter' }); + }Add helper functions (outside this block):
import { createHmac, randomBytes } from 'node:crypto'; const STATE_SECRET = process.env.GITHUB_OAUTH_STATE_SECRET!; function createSignedState({ uid, ttlMs }: { uid: string; ttlMs: number }) { const body = JSON.stringify({ uid, exp: Date.now() + ttlMs, n: randomBytes(16).toString('hex') }); const sig = createHmac('sha256', STATE_SECRET).update(body).digest('hex'); return Buffer.from(`${body}.${sig}`).toString('base64url'); } function verifySignedState(state: string) { try { const raw = Buffer.from(state, 'base64url').toString('utf8'); const [body, sig] = raw.split('.'); const expected = createHmac('sha256', STATE_SECRET).update(body).digest('hex'); if (sig !== expected) return null; const payload = JSON.parse(body) as { uid: string; exp: number; n: string }; if (Date.now() > payload.exp) return null; return payload; } catch { return null; } }Set GITHUB_OAUTH_STATE_SECRET in the environment and rotate periodically.
Also applies to: 270-277
🧹 Nitpick comments (20)
apps/web/client/src/app/projects/import/page.tsx (2)
28-32: Consider using next-intl for internationalization.The component contains hardcoded English strings in CardTitle and CardDescription. Per coding guidelines, user-facing text should use next-intl messages/hooks for proper internationalization support.
As per coding guidelines
Also applies to: 50-53
17-17: Standardize className formatting for consistency.The first Card uses an inline template literal while the second uses curly braces with a string literal. Since both cards share identical styles, consider extracting to a constant or using consistent formatting.
Example refactor:
const Page = () => { const router = useRouter(); + const cardClassName = 'bg-background/80 border-foreground-tertiary/50 h-64 w-full cursor-pointer border-[0.5px] backdrop-blur-xl transition-all duration-200 hover:scale-[1.02] hover:shadow-lg'; + const handleCardClick = (type: 'local' | 'github') => { router.push(`/projects/import/${type}`); }; return ( <div className="flex w-full max-w-4xl gap-6 p-6 select-none"> <Card - className={`bg-background/80 border-foreground-tertiary/50 h-64 w-full cursor-pointer border-[0.5px] backdrop-blur-xl transition-all duration-200 hover:scale-[1.02] hover:shadow-lg`} + className={cardClassName} onClick={() => handleCardClick('local')}<Card - className={ - 'bg-background/80 border-foreground-tertiary/50 h-64 w-full cursor-pointer border-[0.5px] backdrop-blur-xl transition-all duration-200 hover:scale-[1.02] hover:shadow-lg' - } + className={cardClassName} onClick={() => handleCardClick('github')}Also applies to: 37-39
apps/web/client/src/app/projects/import/local/_components/select-folder.tsx (1)
25-34: Remove unusedprevStepfunction.The
prevStepfunction is destructured from the context but is no longer used after the Cancel button behavior changed to route navigation.Apply this diff to remove the unused destructuring:
const { projectData, setProjectData, - prevStep, nextStep, resetProjectData, validateNextJsProject, } = useProjectCreation();apps/web/client/src/app/callback/github/install/page.tsx (3)
27-27: Remove console.log or replace with proper logging.Console statements should not be left in production code. Consider removing this debug log or using a proper logging service.
- console.log('GitHub installation callback:', { installationId, setupAction, state: stateParam });
65-65: Remove console.log or replace with proper logging.Console statements should not be left in production code.
- console.log('GitHub App installation completed:', data);
190-196: Consider a more graceful retry mechanism.Using
window.location.reload()will re-run all the initialization logic, but it's a hard refresh that might lose client-side state. Consider extracting the callback logic into a function that can be called directly on retry, or providing clearer user guidance.+ const processCallback = useCallback(() => { + const installationId = searchParams.get('installation_id'); + const setupAction = searchParams.get('setup_action'); + const stateParam = searchParams.get('state'); + // ... rest of callback logic + }, [searchParams, handleInstallationCallback]); + useEffect(() => { - const installationId = searchParams.get('installation_id'); - // ... rest of callback logic + processCallback(); }, []); ... <Button variant="default" - onClick={() => window.location.reload()} + onClick={processCallback} className="w-full" >packages/github/src/index.ts (1)
4-4: Consider the architectural implications of removing public types.Removing the type exports and directing consumers to use tRPC-inferred types couples this package to tRPC and reduces its reusability. If the GitHub package is intended to be a general-purpose library, consider maintaining minimal type exports (e.g.,
GitHubOrganization,GitHubRepository) that can be used independently of tRPC.If this is intentional and the package is tightly coupled to your tRPC architecture by design, this approach is acceptable but consider documenting the dependency relationship clearly.
Consider maintaining these commonly-used types in the export:
+export type { GitHubOrganization, GitHubRepository } from './types'; -// Types removed - use tRPC inferred types insteadAlternatively, if fully committed to tRPC inference, add documentation explaining the architectural decision and how consumers should infer types.
apps/web/client/src/app/projects/import/github/actions.ts (1)
29-36: Add fallback handling for unexpected response.While unlikely, consider adding a fallback case if both
erroris null anddata.urlis undefined. This ensures the function always provides feedback to the user.if (error) { console.error('Error linking GitHub identity:', error); redirect(`${Routes.IMPORT_GITHUB}?error=oauth_failed`); } if (data?.url) { redirect(data.url); } + +// Fallback if neither error nor URL is present +redirect(`${Routes.IMPORT_GITHUB}?error=unexpected_response`);apps/web/client/src/app/projects/import/github/_components/oauth-connect.tsx (1)
46-50: Externalize user-facing strings to next-intl.Avoid hardcoded text in apps/web/client/src/**/*.tsx per guidelines. Use useTranslations and message keys.
+import { useTranslations } from 'next-intl'; ... export const OAuthConnect = ({ error }: OAuthConnectProps) => { + const t = useTranslations('import.github.oauthConnect'); ... - <CardTitle className="text-xl font-normal">Connect to GitHub</CardTitle> + <CardTitle className="text-xl font-normal">{t('title')}</CardTitle> ... - <CardDescription className="text-center font-normal"> - Grant Onlook access to your GitHub repositories - </CardDescription> + <CardDescription className="text-center font-normal"> + {t('subtitle')} + </CardDescription> ... - {error === 'oauth_failed' - ? 'Failed to connect to GitHub. Please try again.' - : 'An error occurred. Please try again.'} + {error === 'oauth_failed' ? t('error.oauthFailed') : t('error.generic')} ... - <Button onClick={handleConnect} className="px-3 py-2"> + <Button onClick={handleConnect} className="px-3 py-2" aria-label={t('actions.connectAria')}> <Icons.GitHubLogo className="mr-2 h-4 w-4" /> - Connect to GitHub + {t('actions.connect')} </Button>As per coding guidelines.
Also applies to: 60-85, 89-99, 101-110
apps/web/client/src/app/projects/import/github/page.tsx (1)
23-28: Await the prefetch to fully hydrate client queries.Small robustness improvement.
-void api.github.getRepositoriesWithOAuth.prefetch(); +await api.github.getRepositoriesWithOAuth.prefetch();apps/web/client/src/app/projects/import/github/_components/setup.tsx (1)
148-150: Internationalize visible strings (labels, empty-states, buttons).Replace hardcoded text with next-intl messages (e.g., 'Organization', 'Repository', 'Loading repositories...', 'No repositories found', 'Import', 'Cancel').
Also applies to: 191-201, 257-268, 318-338
docs/github-import-production-plan.md (1)
23-29: Add code-fence languages and avoid bare URLs.
- Label the ASCII flow block with "text".
- Convert bare URLs to Markdown links.
-``` +```text 1. ...-Go to your GitHub App settings: https://github.com/settings/apps/onlook-test-app
+Go to your GitHub App settings: https://github.com/settings/apps/onlook-test-appAlso applies to: 37-47 </blockquote></details> <details> <summary>docs/github-oauth-setup.md (2)</summary><blockquote> `37-43`: **Avoid bare URLs in prose.** Format as Markdown autolinks. ```diff -1. Go to your GitHub App settings: https://github.com/settings/apps/onlook-test-app +1. Go to your GitHub App settings: <https://github.com/settings/apps/onlook-test-app>
76-101: Consider making OAuth env vars required when the feature is enabled.Optional envs risk runtime failures. Either gate features by flag or require these in env schema.
- GITHUB_CLIENT_ID: z.string().optional(), - GITHUB_CLIENT_SECRET: z.string().optional(), + GITHUB_CLIENT_ID: z.string(), + GITHUB_CLIENT_SECRET: z.string(),If conditional, use z.preprocess with a feature flag.
apps/web/client/src/server/api/routers/github.ts (6)
52-60: Tighten input validation for owner/repo.Trim and constrain to GitHub-safe characters to avoid empty/whitespace inputs and reduce 4xx noise.
Apply:
- .input( - z.object({ - owner: z.string(), - repo: z.string(), - }), - ) + .input( + z.object({ + owner: z.string().trim().min(1).regex(/^[A-Za-z0-9-]+$/, 'invalid owner'), + repo: z.string().trim().min(1).regex(/^[A-Za-z0-9_.-]+$/, 'invalid repo'), + }), + )As per coding guidelines.
64-66: Unify return shape across validate/validateWithOAuth.
validateWithOAuthreturnshasAccess, butvalidatedoes not. AddhasAccess: truehere for a consistent client API.return { branch: data.default_branch, - isPrivateRepo: data.private, + isPrivateRepo: data.private, + hasAccess: true, };
31-50: OAuth token retrieval: clarify provider assumptions and refresh flow.This relies on
session.provider_token. If the user didn’t sign in with GitHub or the token expired, the error is 412. Consider surfacing UNAUTHORIZED for missing/expired sessions and PRECONDITION_FAILED when the session exists but lacks a GitHub provider token. Also consider moving provider token retrieval toctxto avoid re-creating a Supabase client in hot paths.
215-222: Unused input param and pagination limits.
usernameis unused; remove it or implement filtering.- Only page 1 with 100 items; consider paginate for completeness.
- .input( - z - .object({ - username: z.string().optional(), - }) - .optional(), - ) + .input(z.void()) @@ - const { data } = await octokit.rest.apps.listReposAccessibleToInstallation({ - installation_id: parseInt(installationId, 10), - per_page: 100, - page: 1, - }); + const data = await octokit.paginate( + octokit.rest.apps.listReposAccessibleToInstallation, + { installation_id: Number(installationId), per_page: 100 }, + ); @@ - return data.repositories.map((repo) => ({ + return (Array.isArray(data.repositories) ? data.repositories : data).map((repo) => ({ id: repo.id, name: repo.name, full_name: repo.full_name, description: repo.description, private: repo.private, default_branch: repo.default_branch, clone_url: repo.clone_url, html_url: repo.html_url, updated_at: repo.updated_at, owner: { login: repo.owner.login, avatar_url: repo.owner.avatar_url, }, }));Also applies to: 230-251
262-277: Reduce PII in logs.Avoid logging
ctx.user.idand rawstatevalues. The proposed signed-state change also logs only a generic failure.Also applies to: 280-299
153-160: Minor: tightenrefandpathinputs.Ensure trimmed non-empty strings when provided.
- z.object({ - owner: z.string(), - repo: z.string(), - path: z.string().default(''), - ref: z.string().optional(), // branch, tag, or commit SHA - }), + z.object({ + owner: z.string().trim().min(1), + repo: z.string().trim().min(1), + path: z.string().trim().default(''), + ref: z.string().trim().min(1).optional(), // branch, tag, or commit SHA + }),Also applies to: 167-168
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (33)
apps/backend/supabase/config.toml(1 hunks)apps/web/client/src/app/callback/github/install/page.tsx(1 hunks)apps/web/client/src/app/login/actions.tsx(1 hunks)apps/web/client/src/app/projects/import/_components/background-wrapper.tsx(1 hunks)apps/web/client/src/app/projects/import/cancel-button.tsx(0 hunks)apps/web/client/src/app/projects/import/github/_components/connect.tsx(0 hunks)apps/web/client/src/app/projects/import/github/_components/finalizing.tsx(0 hunks)apps/web/client/src/app/projects/import/github/_components/import-flow.tsx(1 hunks)apps/web/client/src/app/projects/import/github/_components/oauth-connect.tsx(1 hunks)apps/web/client/src/app/projects/import/github/_components/setup.tsx(9 hunks)apps/web/client/src/app/projects/import/github/_context/index.tsx(0 hunks)apps/web/client/src/app/projects/import/github/_hooks/index.ts(0 hunks)apps/web/client/src/app/projects/import/github/_hooks/use-data.ts(0 hunks)apps/web/client/src/app/projects/import/github/_hooks/use-installation.ts(0 hunks)apps/web/client/src/app/projects/import/github/_hooks/use-repo-import.ts(0 hunks)apps/web/client/src/app/projects/import/github/_hooks/use-repo-validation.ts(0 hunks)apps/web/client/src/app/projects/import/github/actions.ts(1 hunks)apps/web/client/src/app/projects/import/github/importing/page.tsx(1 hunks)apps/web/client/src/app/projects/import/github/layout.tsx(0 hunks)apps/web/client/src/app/projects/import/github/loading.tsx(1 hunks)apps/web/client/src/app/projects/import/github/page.tsx(1 hunks)apps/web/client/src/app/projects/import/layout.tsx(2 hunks)apps/web/client/src/app/projects/import/local/_components/import-local-project.tsx(0 hunks)apps/web/client/src/app/projects/import/local/_components/select-folder.tsx(3 hunks)apps/web/client/src/app/projects/import/local/page.tsx(2 hunks)apps/web/client/src/app/projects/import/page.tsx(1 hunks)apps/web/client/src/server/api/routers/github.ts(9 hunks)docs/github-app-setup-testing.md(1 hunks)docs/github-import-production-plan.md(1 hunks)docs/github-oauth-setup.md(1 hunks)packages/github/README.md(1 hunks)packages/github/src/index.ts(1 hunks)packages/github/src/types.ts(0 hunks)
💤 Files with no reviewable changes (12)
- apps/web/client/src/app/projects/import/github/_hooks/index.ts
- apps/web/client/src/app/projects/import/github/_hooks/use-installation.ts
- apps/web/client/src/app/projects/import/github/_hooks/use-repo-validation.ts
- apps/web/client/src/app/projects/import/github/_context/index.tsx
- apps/web/client/src/app/projects/import/github/_components/connect.tsx
- apps/web/client/src/app/projects/import/github/_components/finalizing.tsx
- apps/web/client/src/app/projects/import/github/_hooks/use-repo-import.ts
- apps/web/client/src/app/projects/import/cancel-button.tsx
- apps/web/client/src/app/projects/import/github/_hooks/use-data.ts
- apps/web/client/src/app/projects/import/local/_components/import-local-project.tsx
- apps/web/client/src/app/projects/import/github/layout.tsx
- packages/github/src/types.ts
🧰 Additional context used
📓 Path-based instructions (7)
apps/web/client/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.{ts,tsx}: Use path aliases @/* and ~/* for imports that map to apps/web/client/src/*
Avoid hardcoded user-facing text; use next-intl messages/hooks insteadUse path aliases @/* and ~/* for imports mapping to src/*
Files:
apps/web/client/src/app/projects/import/github/actions.tsapps/web/client/src/app/callback/github/install/page.tsxapps/web/client/src/app/login/actions.tsxapps/web/client/src/app/projects/import/github/loading.tsxapps/web/client/src/app/projects/import/github/_components/oauth-connect.tsxapps/web/client/src/app/projects/import/layout.tsxapps/web/client/src/app/projects/import/local/page.tsxapps/web/client/src/app/projects/import/_components/background-wrapper.tsxapps/web/client/src/app/projects/import/github/_components/import-flow.tsxapps/web/client/src/app/projects/import/github/_components/setup.tsxapps/web/client/src/app/projects/import/github/page.tsxapps/web/client/src/app/projects/import/local/_components/select-folder.tsxapps/web/client/src/server/api/routers/github.tsapps/web/client/src/app/projects/import/github/importing/page.tsxapps/web/client/src/app/projects/import/page.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use the any type unless necessary
Files:
apps/web/client/src/app/projects/import/github/actions.tspackages/github/src/index.tsapps/web/client/src/app/callback/github/install/page.tsxapps/web/client/src/app/login/actions.tsxapps/web/client/src/app/projects/import/github/loading.tsxapps/web/client/src/app/projects/import/github/_components/oauth-connect.tsxapps/web/client/src/app/projects/import/layout.tsxapps/web/client/src/app/projects/import/local/page.tsxapps/web/client/src/app/projects/import/_components/background-wrapper.tsxapps/web/client/src/app/projects/import/github/_components/import-flow.tsxapps/web/client/src/app/projects/import/github/_components/setup.tsxapps/web/client/src/app/projects/import/github/page.tsxapps/web/client/src/app/projects/import/local/_components/select-folder.tsxapps/web/client/src/server/api/routers/github.tsapps/web/client/src/app/projects/import/github/importing/page.tsxapps/web/client/src/app/projects/import/page.tsx
apps/web/client/src/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Default to Server Components; add 'use client' only when using events, state/effects, browser APIs, or client-only libs
Files:
apps/web/client/src/app/projects/import/github/actions.tsapps/web/client/src/app/callback/github/install/page.tsxapps/web/client/src/app/login/actions.tsxapps/web/client/src/app/projects/import/github/loading.tsxapps/web/client/src/app/projects/import/github/_components/oauth-connect.tsxapps/web/client/src/app/projects/import/layout.tsxapps/web/client/src/app/projects/import/local/page.tsxapps/web/client/src/app/projects/import/_components/background-wrapper.tsxapps/web/client/src/app/projects/import/github/_components/import-flow.tsxapps/web/client/src/app/projects/import/github/_components/setup.tsxapps/web/client/src/app/projects/import/github/page.tsxapps/web/client/src/app/projects/import/local/_components/select-folder.tsxapps/web/client/src/app/projects/import/github/importing/page.tsxapps/web/client/src/app/projects/import/page.tsx
{apps,packages}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Avoid using the any type unless absolutely necessary
Files:
apps/web/client/src/app/projects/import/github/actions.tspackages/github/src/index.tsapps/web/client/src/app/callback/github/install/page.tsxapps/web/client/src/app/login/actions.tsxapps/web/client/src/app/projects/import/github/loading.tsxapps/web/client/src/app/projects/import/github/_components/oauth-connect.tsxapps/web/client/src/app/projects/import/layout.tsxapps/web/client/src/app/projects/import/local/page.tsxapps/web/client/src/app/projects/import/_components/background-wrapper.tsxapps/web/client/src/app/projects/import/github/_components/import-flow.tsxapps/web/client/src/app/projects/import/github/_components/setup.tsxapps/web/client/src/app/projects/import/github/page.tsxapps/web/client/src/app/projects/import/local/_components/select-folder.tsxapps/web/client/src/server/api/routers/github.tsapps/web/client/src/app/projects/import/github/importing/page.tsxapps/web/client/src/app/projects/import/page.tsx
apps/web/client/src/app/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/app/**/*.tsx: Default to Server Components; add 'use client' when using events, state/effects, browser APIs, or client‑only libraries
Do not use process.env in client code; import env from @/env insteadAvoid hardcoded user-facing text; use next-intl messages/hooks
Files:
apps/web/client/src/app/callback/github/install/page.tsxapps/web/client/src/app/login/actions.tsxapps/web/client/src/app/projects/import/github/loading.tsxapps/web/client/src/app/projects/import/github/_components/oauth-connect.tsxapps/web/client/src/app/projects/import/layout.tsxapps/web/client/src/app/projects/import/local/page.tsxapps/web/client/src/app/projects/import/_components/background-wrapper.tsxapps/web/client/src/app/projects/import/github/_components/import-flow.tsxapps/web/client/src/app/projects/import/github/_components/setup.tsxapps/web/client/src/app/projects/import/github/page.tsxapps/web/client/src/app/projects/import/local/_components/select-folder.tsxapps/web/client/src/app/projects/import/github/importing/page.tsxapps/web/client/src/app/projects/import/page.tsx
apps/web/client/src/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable references across renders
Keep the active MobX store in a useRef and perform async cleanup with setTimeout(() => storeRef.current?.clear(), 0) to avoid route-change races
Avoid useMemo for creating MobX store instances
Avoid putting the MobX store instance in effect dependency arrays if it causes loops; split concerns by domain
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable identities across renders
Keep the active MobX store in a useRef and clean up asynchronously with setTimeout(() => storeRef.current?.clear(), 0)
Do not use useMemo to create MobX stores
Avoid placing MobX store instances in effect dependency arrays if it causes loops; split concerns instead
observer components must be client components; place a single client boundary at the feature entry; child observers need not repeat 'use client'
Files:
apps/web/client/src/app/callback/github/install/page.tsxapps/web/client/src/app/login/actions.tsxapps/web/client/src/app/projects/import/github/loading.tsxapps/web/client/src/app/projects/import/github/_components/oauth-connect.tsxapps/web/client/src/app/projects/import/layout.tsxapps/web/client/src/app/projects/import/local/page.tsxapps/web/client/src/app/projects/import/_components/background-wrapper.tsxapps/web/client/src/app/projects/import/github/_components/import-flow.tsxapps/web/client/src/app/projects/import/github/_components/setup.tsxapps/web/client/src/app/projects/import/github/page.tsxapps/web/client/src/app/projects/import/local/_components/select-folder.tsxapps/web/client/src/app/projects/import/github/importing/page.tsxapps/web/client/src/app/projects/import/page.tsx
apps/web/client/src/server/api/routers/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/server/api/routers/**/*.ts: Place tRPC routers under apps/web/client/src/server/api/routers/**
Use publicProcedure/protectedProcedure from apps/web/client/src/server/api/trpc.ts and validate inputs with Zod
Return plain objects/arrays; rely on SuperJSON for serialization in tRPC procedures
apps/web/client/src/server/api/routers/**/*.ts: Place tRPC routers under src/server/api/routers/**
Use publicProcedure/protectedProcedure from src/server/api/trpc.ts and validate inputs with Zod
Return plain objects/arrays; rely on SuperJSON for serialization
Files:
apps/web/client/src/server/api/routers/github.ts
🧠 Learnings (2)
📚 Learning: 2025-09-16T19:22:52.461Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-16T19:22:52.461Z
Learning: Applies to apps/web/client/src/server/api/root.ts : Export all tRPC routers from src/server/api/root.ts
Applied to files:
packages/github/src/index.ts
📚 Learning: 2025-09-14T01:44:21.209Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: AGENTS.md:0-0
Timestamp: 2025-09-14T01:44:21.209Z
Learning: Applies to apps/web/client/src/server/api/root.ts : Export all tRPC routers from apps/web/client/src/server/api/root.ts
Applied to files:
packages/github/src/index.ts
🧬 Code graph analysis (11)
apps/web/client/src/app/projects/import/github/actions.ts (1)
apps/web/client/src/utils/constants/index.ts (1)
Routes(1-30)
apps/web/client/src/app/projects/import/github/_components/oauth-connect.tsx (2)
apps/web/client/src/utils/constants/index.ts (2)
LocalForageKeys(50-52)Routes(1-30)apps/web/client/src/app/projects/import/github/actions.ts (1)
connectGitHubOAuth(9-37)
apps/web/client/src/app/projects/import/layout.tsx (2)
apps/web/client/src/app/projects/import/_components/background-wrapper.tsx (1)
BackgroundWrapper(5-20)apps/web/client/src/app/projects/_components/top-bar.tsx (1)
TopBar(35-268)
apps/web/client/src/app/projects/import/_components/background-wrapper.tsx (1)
apps/web/client/src/hooks/use-get-background.tsx (1)
useGetBackground(9-28)
apps/web/client/src/app/projects/import/github/_components/import-flow.tsx (1)
apps/web/client/src/app/projects/import/github/_components/setup.tsx (1)
SetupGithub(17-342)
apps/web/client/src/app/projects/import/github/_components/setup.tsx (2)
apps/web/client/src/trpc/react.tsx (2)
RouterOutputs(37-37)api(23-23)apps/web/client/src/utils/constants/index.ts (1)
Routes(1-30)
apps/web/client/src/app/projects/import/github/page.tsx (2)
apps/web/client/src/app/projects/import/github/_components/oauth-connect.tsx (1)
OAuthConnect(20-114)apps/web/client/src/app/projects/import/github/_components/import-flow.tsx (1)
ImportFlow(7-17)
apps/web/client/src/app/projects/import/local/_components/select-folder.tsx (1)
apps/web/client/src/utils/constants/index.ts (1)
Routes(1-30)
apps/web/client/src/server/api/routers/github.ts (2)
packages/db/src/schema/user/user.ts (1)
users(12-25)apps/web/client/src/server/api/trpc.ts (1)
protectedProcedure(131-150)
apps/web/client/src/app/projects/import/github/importing/page.tsx (3)
apps/web/client/src/trpc/react.tsx (1)
api(23-23)apps/web/client/src/utils/constants/index.ts (1)
Routes(1-30)packages/ui/src/components/icons/index.tsx (1)
Icons(138-3667)
apps/web/client/src/app/projects/import/page.tsx (1)
packages/ui/src/components/icons/index.tsx (1)
Icons(138-3667)
🪛 Gitleaks (8.28.0)
packages/github/README.md
[high] 81-90: Identified a Private Key, which may compromise cryptographic security and sensitive data encryption.
(private-key)
[high] 104-107: Identified a Private Key, which may compromise cryptographic security and sensitive data encryption.
(private-key)
docs/github-oauth-setup.md
[high] 66-66: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🪛 LanguageTool
docs/github-import-production-plan.md
[uncategorized] ~16-~16: The official name of this software platform is spelled with a capital “H”.
Context: ...up/login 2. Simplified Connect UI (apps/web/client/src/app/projects/import/github/_components/connect.tsx): - Only sh...
(GITHUB)
[uncategorized] ~39-~39: The official name of this software platform is spelled with a capital “H”.
Context: ...ers with 100+ repositories - Files: github.ts:144-183, setup.tsx:59-66 ### 1.2...
(GITHUB)
[uncategorized] ~69-~69: The official name of this software platform is spelled with a capital “H”.
Context: ...ch in context - Files: setup.tsx, github.ts (add getBranches endpoint) ### 2...
(GITHUB)
[uncategorized] ~77-~77: The official name of this software platform is spelled with a capital “H”.
Context: ...- Files: New use-repo-preview.ts, github.ts (add validation endpoint) ### 2.3 ...
(GITHUB)
[uncategorized] ~92-~92: The official name of this software platform is spelled with a capital “H”.
Context: ...e repository list with TTL - Files: github.ts (modify getRepositoriesWithApp), `u...
(GITHUB)
[uncategorized] ~160-~160: The official name of this software platform is spelled with a capital “H”.
Context: ...🔴 #### 1. Hardcoded Pagination Limit (github.ts:158-159) ```typescript per_page: 10...
(GITHUB)
[uncategorized] ~169-~169: The official name of this software platform is spelled with a capital “H”.
Context: ...ll pages. #### 2. CSRF Validation Bug (github.ts:196) ```typescript if (input.state ...
(GITHUB)
[uncategorized] ~187-~187: The official name of this software platform is spelled with a capital “H”.
Context: ... getUserGitHubInstallation Design Flaw (github.ts:11-27) ```typescript if (!user?.git...
(GITHUB)
[uncategorized] ~200-~200: The official name of this software platform is spelled with a capital “H”.
Context: ... #### 5. Unnecessary GitHub API Call (github.ts:128-131`) ```typescript const { octo...
(GITHUB)
[uncategorized] ~248-~248: The official name of this software platform is spelled with a capital “H”.
Context: ...tification. #### 10. Unused Parameter (github.ts:149) ```typescript username: z.stri...
(GITHUB)
[uncategorized] ~256-~256: The official name of this software platform is spelled with a capital “H”.
Context: .... #### 11. Type Casting with as any (github.ts:75) ```typescript login: 'login' in...
(GITHUB)
[uncategorized] ~265-~265: The official name of this software platform is spelled with a capital “H”.
Context: ...og/error/warn Throughout Locations: github.ts:84, 140, 179, 197, 210 and all `_ho...
(GITHUB)
[style] ~266-~266: To elevate your writing, try using a synonym here.
Context: ...structured logging, no correlation IDs, hard to debug production. Impact: Poor o...
(HARD_TO)
docs/github-oauth-setup.md
[uncategorized] ~325-~325: The official name of this software platform is spelled with a capital “H”.
Context: ... 7: Create OAuth Callback Route Create apps/web/client/src/app/api/auth/callback/github/route.ts: ```typescript import { Next...
(GITHUB)
[uncategorized] ~355-~355: The official name of this software platform is spelled with a capital “H”.
Context: ...ode}&state=${state} ); } ``` Createapps/web/client/src/app/projects/import/github/oauth-callback/page.tsx`: ```typescrip...
(GITHUB)
🪛 markdownlint-cli2 (0.18.1)
docs/github-app-setup-testing.md
14-14: Bare URL used
(MD034, no-bare-urls)
15-15: Bare URL used
(MD034, no-bare-urls)
169-169: Bare URL used
(MD034, no-bare-urls)
170-170: Bare URL used
(MD034, no-bare-urls)
171-171: Bare URL used
(MD034, no-bare-urls)
packages/github/README.md
10-10: Bare URL used
(MD034, no-bare-urls)
11-11: Bare URL used
(MD034, no-bare-urls)
docs/github-oauth-setup.md
23-23: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
37-37: Bare URL used
(MD034, no-bare-urls)
⏰ 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)
- GitHub Check: Supabase Preview
🔇 Additional comments (18)
apps/backend/supabase/config.toml (1)
17-17: Configuration change aligns with GitHub OAuth flow revamp.The
enable_manual_linkingoption enables manual account linking functionality, which is a prerequisite for the OAuth-based GitHub import workflow described in this PR. The configuration is properly placed under the[auth]section and uses correct TOML syntax.apps/web/client/src/app/projects/import/local/page.tsx (1)
28-52: LGTM! Clean refactor to a simpler layout structure.The simplified structure effectively delegates background and navigation concerns to the parent layout while maintaining the step-based animation flow. The MotionCard-based composition with nested AnimatePresence is correct and more maintainable than the previous multi-layered approach.
apps/web/client/src/app/projects/import/layout.tsx (1)
21-30: LGTM! Well-structured layout composition.The server component correctly composes client components (BackgroundWrapper and TopBar) to establish a consistent, immersive layout for import pages. The session validation at lines 14-20 ensures proper authentication before rendering the layout.
apps/web/client/src/app/projects/import/_components/background-wrapper.tsx (1)
1-20: LGTM! Clean background wrapper implementation.The component is correctly marked as a client component and properly uses the useGetBackground hook to provide a full-screen background. The fixed positioning with inline styles is appropriate for this use case, and rendering children inside the container ensures correct layering.
apps/web/client/src/app/projects/import/page.tsx (2)
1-7: LGTM!The 'use client' directive is correctly applied since the component uses the
useRouterhook and event handlers. Imports use proper path aliases as per coding guidelines.
8-12: LGTM!The component setup and navigation handler are clean and type-safe.
apps/web/client/src/app/projects/import/local/_components/select-folder.tsx (1)
477-479: <!-- [scratchpad_start]
[task review_file_1/1 verify_navigation_behavior 3/3]
Verify Cancel button navigation behavior change from prevStep to router.push(Routes.IMPORT_PROJECT).
[observations]
- Context file viewed successfully
- prevStep() function defined in context (line ~258-265):
- If currentStep === 0: calls resetProjectData() and returns
- Else: decrements currentStep and sets direction
- resetProjectData() clears folderPath, name, files, resets currentStep to 0
- NewSelectFolder is at step 0 (first step in wizard)
- /projects/import/page.tsx: Shows local vs github import selection
- /projects/import/local/page.tsx: Wraps wizard flow with ProjectCreationProvider
- Routes.IMPORT_PROJECT = '/projects/import' (verified in constants)
[analysis]
OLD behavior (prevStep from step 0):- Calls resetProjectData() which clears state but stays on wizard page
- User can retry without navigating away
NEW behavior (router.push(Routes.IMPORT_PROJECT)):
- Navigates to /projects/import (import selection page)
- Leaves context provider boundary, loses all state
- User returns to choosing between local/github import
This is intentional architectural change:
- Moving from step-based navigation (prevStep) to route-based navigation (router.push)
- Aligns with broader PR restructuring mentioned in AI summary
- Routes destination is correct
[conclusion]
Change is verified as intentional and properly implemented. Routes.IMPORT_PROJECT is correct. UX change: cancel goes to import selection rather than clearing state on wizard.
[done]
</scratchpad_end] -->Cancel button now navigates to import selection page rather than resetting wizard state.
The change from
prevSteptorouter.push(Routes.IMPORT_PROJECT)is an intentional architectural shift from step-based to route-based navigation. Previously, cancel would reset state and keep users on the wizard; now it navigates back to/projects/import(the import method selection page). This is the correct route and aligns with the PR's restructuring, but represents a UX change where in-progress work is discarded.apps/web/client/src/app/callback/github/install/page.tsx (2)
29-34: Good addition: Handles pending approval case.The pre-validation for the 'request' setup action correctly identifies when installation requires owner approval and provides clear user feedback. The early return prevents unnecessary mutation calls.
36-40: Improved error messaging.The updated error message for missing installation_id is more descriptive and helps users understand what went wrong.
apps/web/client/src/app/login/actions.tsx (1)
30-31: LGTM! Well-implemented OAuth scope handling.Requesting repository scopes upfront during the GitHub login flow provides a better user experience by avoiding multiple authorization prompts later. The scopes are appropriately scoped for the import functionality, and the explanatory comment makes the intention clear.
apps/web/client/src/app/projects/import/github/loading.tsx (1)
1-32: LGTM! Clean loading skeleton implementation.The loading component properly implements Next.js's loading.tsx pattern with appropriate skeleton placeholders. The structure mirrors the expected content layout, providing good visual feedback during data fetching.
apps/web/client/src/app/projects/import/github/importing/page.tsx (1)
28-67: LGTM! Proper useEffect implementation with tRPC mutations.The effect correctly:
- Includes all reactive values in the dependency array
- Excludes stable mutation functions (following tRPC/React Query patterns)
- Handles the async import with proper error handling
- Navigates to the project on success
The fire-and-forget pattern with
voidis appropriate here since the mutations should complete server-side even if the user navigates away.packages/github/README.md (1)
1-148: LGTM! Comprehensive GitHub App setup documentation.The documentation provides clear, step-by-step instructions for setting up a GitHub App, including permission configuration, key generation, and environment setup. The security notes and useful links are valuable additions.
Note: The static analysis warnings about "private keys" on lines 81-90 and 104-107 are false positives—these are example/placeholder keys in documentation, not actual credentials. Similarly, the bare URL warnings are acceptable in documentation context.
apps/web/client/src/app/projects/import/github/_components/import-flow.tsx (1)
1-17: LGTM! Clean composition with proper client boundary.The component correctly:
- Uses
'use client'directive for Framer Motion animations- Provides a clean wrapper with animation configuration
- Delegates actual import logic to the
SetupGithubcomponentThis follows the coding guideline of placing a client boundary at the feature entry point.
docs/github-app-setup-testing.md (1)
1-171: LGTM! Thorough testing documentation.The testing guide provides comprehensive coverage of:
- GitHub App installation for local development
- Step-by-step testing procedures
- Common troubleshooting scenarios
- A practical testing checklist
- Production deployment considerations
This complements the main README well and will help developers test the GitHub integration flow effectively.
Note: The static analysis warnings about bare URLs are false positives—they're appropriate in documentation context.
apps/web/client/src/app/projects/import/github/_components/oauth-connect.tsx (1)
14-14: connectGitHubOAuth is already correctly configured as a Server Action.The file
apps/web/client/src/app/projects/import/github/actions.tshas'use server';at line 1 and properly uses server-only APIs (headers, redirect). The import is safe and valid.Likely an incorrect or invalid review comment.
apps/web/client/src/app/projects/import/github/page.tsx (1)
6-12: Fix Next.js PageProps: searchParams must not be a Promise.Typing it as Promise forces an unnecessary await and can break at runtime.
-type PageProps = { - searchParams: Promise<{ error?: string }>; -}; +type PageProps = { + searchParams?: { error?: string | string[] }; +}; -const Page = async (props: PageProps) => { - const searchParams = await props.searchParams; +const Page = async ({ searchParams }: PageProps) => { + const errorParam = Array.isArray(searchParams?.error) + ? searchParams!.error[0] + : searchParams?.error;Then pass errorParam below.
- return <OAuthConnect error={searchParams.error} />; + return <OAuthConnect error={errorParam} />;Likely an incorrect or invalid review comment.
apps/web/client/src/server/api/routers/github.ts (1)
188-213: No callers exist—this is a newly added procedure with no dependents.The exhaustive search found zero call sites for
checkGitHubAppInstallationanywhere in the codebase. The procedure is defined but not yet invoked. The review warning assumes downstream code relies on exceptions, but no such code exists. The null-returning behavior is safe since it will only affect callers added in the future.Likely an incorrect or invalid review comment.
| // Handle "request" action - installation pending approval | ||
| if (setupAction === 'request') { | ||
| setState('error'); | ||
| setMessage('Installation requires approval from the repository or organization owner. You can try installing on repositories you own, or wait for the owner to approve the installation.'); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Hardcoded user-facing text violates coding guidelines.
The error message string is hardcoded, but the coding guidelines require using next-intl messages/hooks for all user-facing text.
As per coding guidelines
Apply this pattern throughout the file:
+import { useTranslations } from 'next-intl';
+
export default function GitHubInstallCallbackPage() {
+ const t = useTranslations('githubInstallCallback');
const router = useRouter();
...
- setMessage('Installation requires approval from the repository or organization owner. You can try installing on repositories you own, or wait for the owner to approve the installation.');
+ setMessage(t('installationRequiresApproval'));Note: This applies to all hardcoded strings in this file (lines 32, 38, 44, 50, 64, 74, 123-127, 163-165, 174-176, 185-186).
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/web/client/src/app/callback/github/install/page.tsx around lines 29 to
34 (and also update lines 32, 38, 44, 50, 64, 74, 123-127, 163-165, 174-176,
185-186), the user-facing strings are hardcoded; replace them with next-intl
translations by importing and using the useTranslations hook (or the project's
established i18n helper), add appropriate message keys to the locale JSON files,
and call t('namespace.key') (or the equivalent) instead of setMessage('...') or
inline literals; ensure pluralization/variables are handled via translation
params and update all listed lines consistently to remove hardcoded text.
| import { api, type RouterOutputs } from '@/trpc/react'; | ||
| import { Routes } from '@/utils/constants'; | ||
| import { useRouter } from 'next/navigation'; | ||
| import { StepContent, StepFooter, StepHeader } from '../../steps'; |
There was a problem hiding this comment.
Missing 'use client' — this file uses hooks and router.
Add the client directive to avoid Next.js runtime errors.
+'use client';
+
import { Button } from '@onlook/ui/button';
...
import { useCallback, useEffect, useRef, useState } from 'react';Also applies to: 17-23
🤖 Prompt for AI Agents
In apps/web/client/src/app/projects/import/github/_components/setup.tsx around
lines 8-11 (and similarly for lines 17-23), the file uses React hooks and
Next.js router but is missing the 'use client' directive; add a top-of-file "use
client" directive as the very first line in the module so the file runs on the
client runtime and avoids Next.js server/runtime hook errors.
| const handleOrganizationSelect = (value: string) => { | ||
| if (value === 'all') { | ||
| setSelectedOrg(null); | ||
| } else { | ||
| const organization = githubData.organizations.find((org: any) => org.login === value); | ||
| setSelectedOrg(organization || null); | ||
| } | ||
| const organization = organizations.find((org: any) => org.login === value); | ||
| setSelectedOrg(organization || null); | ||
| setSelectedRepo(null); | ||
| }; | ||
|
|
||
| const handleRepositorySelect = (value: string) => { | ||
| const repository = githubData.repositories.find((repo: any) => repo.full_name === value); | ||
| const repository = repositories.find((repo: any) => repo.full_name === value); | ||
| setSelectedRepo(repository || null); | ||
| setImportError(null); // Clear any previous errors | ||
| }; | ||
|
|
There was a problem hiding this comment.
Remove any and use the declared GitHub types.
Strong typing prevents UI/runtime bugs and aligns with repo guidelines.
-const handleOrganizationSelect = (value: string) => {
- const organization = organizations.find((org: any) => org.login === value);
+const handleOrganizationSelect = (value: string) => {
+ const organization = organizations.find((org: GitHubOrganization) => org.login === value);
setSelectedOrg(organization || null);
setSelectedRepo(null);
};
-const handleRepositorySelect = (value: string) => {
- const repository = repositories.find((repo: any) => repo.full_name === value);
+const handleRepositorySelect = (value: string) => {
+ const repository = repositories.find((repo: GitHubRepository) => repo.full_name === value);
setSelectedRepo(repository || null);
setImportError(null);
};
-const filteredRepositories = repositories.filter((repo: any) => {
+const filteredRepositories = repositories.filter((repo: GitHubRepository) => {
const matchesOrg = !selectedOrg || repo.owner.login === selectedOrg.login;
const matchesSearch =
...
return matchesOrg && matchesSearch;
});
-{organizations.map((org: any) => (
+{organizations.map((org: GitHubOrganization) => (
<SelectItem key={org.id} value={org.login}>
...
</SelectItem>
))}
-{filteredRepositories.map((repo: any) => (
+{filteredRepositories.map((repo: GitHubRepository) => (
<button
key={repo.id}
onClick={() => handleRepositorySelect(repo.full_name)}
>
...
</button>
))}As per coding guidelines.
Also applies to: 99-107, 171-187, 257-269, 274-302
🤖 Prompt for AI Agents
In apps/web/client/src/app/projects/import/github/_components/setup.tsx around
lines 41 to 52 (and similarly at 99-107, 171-187, 257-269, 274-302), replace all
uses of "any" for GitHub entities with the project's declared GitHub types
(e.g., the Organization and Repository interfaces/types exported/imported in
this module or from the shared github types file); change the callbacks to type
the parameters and array items properly (e.g., organizations.find((org:
GitHubOrganization) => ...) and repositories.find((repo: GitHubRepository) =>
...)), update related state setters and function signatures to use those types,
and import the correct types at the top of the file if not already present to
ensure strong typing throughout those line ranges.
| const handleImport = async () => { | ||
| if (!selectedRepo) return; | ||
|
|
||
| setIsValidating(true); | ||
| setImportError(null); | ||
|
|
||
| try { | ||
| const [owner, repo] = selectedRepo.full_name.split('/'); | ||
| if (!owner || !repo) { | ||
| throw new Error('Invalid repository name'); | ||
| } | ||
|
|
||
| // Validate repository access | ||
| await validateRepo.mutateAsync({ owner, repo }); | ||
|
|
||
| // If validation succeeds, navigate to importing page | ||
| const params = new URLSearchParams({ | ||
| repo: selectedRepo.full_name, | ||
| branch: selectedRepo.default_branch, | ||
| clone_url: selectedRepo.clone_url, | ||
| name: selectedRepo.name, | ||
| ...(selectedRepo.description && { description: selectedRepo.description }), | ||
| }); | ||
| router.push(`${Routes.IMPORT_GITHUB}/importing?${params.toString()}`); | ||
| } catch (error) { | ||
| const errorMessage = | ||
| error instanceof Error | ||
| ? error.message | ||
| : 'Failed to validate repository access'; | ||
| setImportError(errorMessage); | ||
| } finally { | ||
| setIsValidating(false); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Wire Import button to validation flow; surface errors and loading state.
Currently the footer bypasses validateRepo; handleImport is unused and users don't see errors.
// keep handleImport as-is
...
-<StepFooter>
+<StepFooter>
<Button onClick={() => router.push(Routes.IMPORT_PROJECT)} variant="outline">
Cancel
</Button>
- <Button
- className="px-3 py-2"
- onClick={() => {
- if (!selectedRepo) return;
- const params = new URLSearchParams({
- repo: selectedRepo.full_name,
- branch: selectedRepo.default_branch,
- clone_url: selectedRepo.clone_url,
- name: selectedRepo.name,
- ...(selectedRepo.description && { description: selectedRepo.description }),
- });
- router.push(`${Routes.IMPORT_GITHUB}/importing?${params.toString()}`);
- }}
- disabled={!selectedRepo}
- >
- <Icons.Download className="w-4 h-4 mr-2" />
- <span>Import</span>
- </Button>
+ <Button
+ className="px-3 py-2"
+ onClick={handleImport}
+ disabled={!selectedRepo || isValidating}
+ >
+ {isValidating ? (
+ <>
+ <Icons.Shadow className="w-3 h-3 mr-2 animate-spin" />
+ <span>Validating…</span>
+ </>
+ ) : (
+ <>
+ <Icons.Download className="w-4 h-4 mr-2" />
+ <span>Import</span>
+ </>
+ )}
+ </Button>
</StepFooter>
+
+{/* Import error */}
+{importError && (
+ <div role="alert" className="mt-3 text-sm text-red-500">
+ {importError}
+ </div>
+)}Also applies to: 318-338
🤖 Prompt for AI Agents
In apps/web/client/src/app/projects/import/github/_components/setup.tsx around
lines 53-86 (and similarly 318-338), the Import button isn't wired to the
validation flow so handleImport is unused and users don't see validation errors
or loading state; wire the footer's Import button to call handleImport, ensure
the button shows isValidating state (disabled/spinner) during validation,
surface setImportError messages in the UI (e.g., render importError near the
footer), and ensure validateRepo.mutateAsync errors propagate into
setImportError as already implemented; update both places (53-86 and 318-338) so
they invoke handleImport instead of directly navigating.
| if (error) { | ||
| return ( | ||
| <div className="fixed inset-0"> | ||
| <div className="bg-background relative flex h-full w-full items-center justify-center"> | ||
| <Card className="bg-background/30 w-[30rem] backdrop-blur-md"> | ||
| <CardContent className="space-y-6 p-6"> | ||
| <div className="flex flex-col items-center gap-4"> | ||
| <div className="rounded-full bg-red-900/20 p-4"> | ||
| <Icons.CrossCircled className="h-8 w-8 text-red-500" /> | ||
| </div> | ||
| <CardTitle className="text-xl font-normal">Import Failed</CardTitle> | ||
| <CardDescription className="text-center">{error}</CardDescription> | ||
| </div> | ||
| <div className="flex justify-center gap-2"> | ||
| <Button onClick={handleRetry} variant="outline"> | ||
| Back to Selection | ||
| </Button> | ||
| </div> | ||
| </CardContent> | ||
| </Card> | ||
| </div> | ||
| </div> | ||
| ); | ||
| } | ||
|
|
||
| return ( | ||
| <div className="fixed inset-0"> | ||
| <div className="bg-background relative flex h-full w-full items-center justify-center"> | ||
| <Card className="bg-background/30 w-[30rem] backdrop-blur-md"> | ||
| <CardContent className="space-y-6 p-6"> | ||
| <div className="flex flex-col items-center gap-4"> | ||
| <div className="bg-primary/10 rounded-full p-4"> | ||
| <Icons.Shadow className="text-primary h-8 w-8 animate-spin" /> | ||
| </div> | ||
| <CardTitle className="text-xl font-normal"> | ||
| Importing Repository | ||
| </CardTitle> | ||
| <CardDescription className="text-center"> | ||
| {repoFullName ?? 'Setting up your project...'} | ||
| </CardDescription> | ||
| </div> | ||
| <div className="text-muted-foreground text-center text-sm"> | ||
| This may take a minute. Please don't close this page. | ||
| </div> | ||
| </CardContent> | ||
| </Card> | ||
| </div> | ||
| </div> | ||
| ); |
There was a problem hiding this comment.
Internationalize user-facing text strings.
The component contains multiple hardcoded user-facing strings that should use next-intl for internationalization. This violates the coding guideline for apps/web/client/src/**/*.tsx.
As per coding guidelines, strings like "Import Failed", "Back to Selection", "Importing Repository", etc., should be moved to internationalization messages. Example:
+import { useTranslations } from 'next-intl';
+
export default function ImportingPage() {
+ const t = useTranslations('import.github');
const router = useRouter();
// ...
if (error) {
return (
// ...
- <CardTitle className="text-xl font-normal">Import Failed</CardTitle>
+ <CardTitle className="text-xl font-normal">{t('importFailed')}</CardTitle>
// ... repeat for other stringsCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
apps/web/client/src/app/projects/import/github/importing/page.tsx lines 73-121:
several hardcoded user-facing strings ("Import Failed", "Back to Selection",
"Importing Repository", "Setting up your project...", the "This may take a
minute..." notice, and possibly repoFullName fallback) must be
internationalized; add keys to the appropriate locale messages file (e.g.
projects.import.github.importing.*), import and use next-intl (useTranslations
or formatMessage) in this component, replace each literal with the corresponding
t('key') call (use t('fallback', { repo: repoFullName }) or keep fallback string
localized), and ensure the messages file is updated for all supported locales
and that any tests/typings referencing message keys are adjusted accordingly.
| validateWithOAuth: protectedProcedure | ||
| .input( | ||
| z.object({ | ||
| owner: z.string(), | ||
| repo: z.string(), | ||
| }), | ||
| ) | ||
| .mutation(async ({ input }) => { | ||
| try { | ||
| const { octokit } = await getUserGitHubOAuth(); | ||
| const { data } = await octokit.rest.repos.get({ | ||
| owner: input.owner, | ||
| repo: input.repo, | ||
| }); | ||
| return { | ||
| branch: data.default_branch, | ||
| isPrivateRepo: data.private, | ||
| hasAccess: true, | ||
| }; | ||
| } catch (error) { | ||
| throw new TRPCError({ | ||
| code: 'FORBIDDEN', | ||
| message: 'Unable to access this repository. Please check your permissions.', | ||
| cause: error, | ||
| }); | ||
| } | ||
| }), |
There was a problem hiding this comment.
Map 404 to NOT_FOUND and align Zod with validate.
Return 404 when the repo does not exist; keep 403 for permission issues. Also mirror the stricter Zod constraints.
- validateWithOAuth: protectedProcedure
- .input(
- z.object({
- owner: z.string(),
- repo: z.string(),
- }),
- )
+ validateWithOAuth: protectedProcedure
+ .input(
+ z.object({
+ owner: z.string().trim().min(1).regex(/^[A-Za-z0-9-]+$/, 'invalid owner'),
+ repo: z.string().trim().min(1).regex(/^[A-Za-z0-9_.-]+$/, 'invalid repo'),
+ }),
+ )
.mutation(async ({ input }) => {
try {
const { octokit } = await getUserGitHubOAuth();
const { data } = await octokit.rest.repos.get({
owner: input.owner,
repo: input.repo,
});
return {
branch: data.default_branch,
isPrivateRepo: data.private,
hasAccess: true,
};
} catch (error) {
- throw new TRPCError({
- code: 'FORBIDDEN',
- message: 'Unable to access this repository. Please check your permissions.',
- cause: error,
- });
+ const status = (error as any)?.status;
+ if (status === 404) {
+ throw new TRPCError({
+ code: 'NOT_FOUND',
+ message: 'Repository not found.',
+ cause: error,
+ });
+ }
+ throw new TRPCError({
+ code: 'FORBIDDEN',
+ message: 'Unable to access this repository. Please check your permissions.',
+ cause: error,
+ });
}
}),🤖 Prompt for AI Agents
In apps/web/client/src/server/api/routers/github.ts around lines 68 to 94, the
mutation should map a 404 from GitHub to a TRPCError with code 'NOT_FOUND' while
preserving 403 for permission issues and tightening the Zod input to match the
stricter validate schema; update the input schema from z.string() to the same
constraints used by validate (e.g., z.string().min(1).regex(/^[A-Za-z0-9_.-]+$/)
for owner and repo), and in the catch inspect the Octokit error (error.status or
error.statusCode): if 404 throw new TRPCError({code: 'NOT_FOUND', message:
'Repository not found.' , cause: error}); if 403 throw the existing FORBIDDEN
error; otherwise rethrow or throw an INTERNAL_SERVER_ERROR with the original
error as cause.
| getOrganizations: protectedProcedure.query(async ({ ctx }) => { | ||
| try { | ||
| const { octokit, installationId } = await getUserGitHubInstallation( | ||
| ctx.db, | ||
| ctx.user.id, | ||
| ); | ||
|
|
||
| // Get installation details to determine account type | ||
| const installation = await octokit.rest.apps.getInstallation({ | ||
| installation_id: parseInt(installationId, 10), | ||
| }); | ||
| // Get installation details to determine account type | ||
| const installation = await octokit.rest.apps.getInstallation({ | ||
| installation_id: parseInt(installationId, 10), | ||
| }); | ||
|
|
||
| // If installed on an organization, return that organization | ||
| if (installation.data.account && 'type' in installation.data.account && installation.data.account.type === 'Organization') { | ||
| return [{ | ||
| // If installed on an organization, return that organization | ||
| if ( | ||
| installation.data.account && | ||
| 'type' in installation.data.account && | ||
| installation.data.account.type === 'Organization' | ||
| ) { | ||
| return [ | ||
| { | ||
| id: installation.data.account.id, | ||
| login: 'login' in installation.data.account ? installation.data.account.login : (installation.data.account as any).name || '', | ||
| login: | ||
| 'login' in installation.data.account | ||
| ? installation.data.account.login | ||
| : (installation.data.account as any).name || '', | ||
| avatar_url: installation.data.account.avatar_url, | ||
| description: undefined, // Organizations don't have descriptions in this context | ||
| }]; | ||
| } | ||
|
|
||
| // If installed on a user account, return empty (no organizations) | ||
| return []; | ||
| } catch (error) { | ||
| throw new TRPCError({ | ||
| code: 'FORBIDDEN', | ||
| message: 'GitHub App installation is invalid or has been revoked', | ||
| cause: error | ||
| }); | ||
| }, | ||
| ]; | ||
| } | ||
| }), | ||
|
|
||
| // If installed on a user account, return empty (no organizations) | ||
| return []; | ||
| } catch (error) { | ||
| throw new TRPCError({ | ||
| code: 'FORBIDDEN', | ||
| message: 'GitHub App installation is invalid or has been revoked', | ||
| cause: error, | ||
| }); | ||
| } | ||
| }), |
There was a problem hiding this comment.
🧩 Analysis chain
Avoid as any; check installation auth; guard installationId parsing.
as anyviolates “no any” guidance. Use property guards.apps.getInstallationtypically requires App auth; installation tokens may 403. Verify the auth mode used bycreateInstallationOctokitfor this call.- Guard
installationIdparsing to avoidNaN.
- const installation = await octokit.rest.apps.getInstallation({
- installation_id: parseInt(installationId, 10),
- });
+ const installation_id = Number(installationId);
+ if (!Number.isFinite(installation_id)) {
+ throw new TRPCError({ code: 'INTERNAL_SERVER_ERROR', message: 'Invalid installation id' });
+ }
+ const installation = await octokit.rest.apps.getInstallation({ installation_id });
@@
- return [
- {
- id: installation.data.account.id,
- login:
- 'login' in installation.data.account
- ? installation.data.account.login
- : (installation.data.account as any).name || '',
- avatar_url: installation.data.account.avatar_url,
- description: undefined, // Organizations don't have descriptions in this context
- },
- ];
+ const acc = installation.data.account;
+ const login =
+ typeof (acc as Record<string, unknown>).login === 'string'
+ ? (acc as Record<string, string>).login
+ : typeof (acc as Record<string, unknown>).name === 'string'
+ ? (acc as Record<string, string>).name
+ : '';
+ return [
+ {
+ id: acc.id,
+ login,
+ avatar_url: acc.avatar_url,
+ description: undefined,
+ },
+ ];To confirm auth requirements for this endpoint:
As per coding guidelines.
🌐 Web query:
GitHub REST API: Does GET /app/installations/{installation_id} require app authentication (JWT) rather than installation access token?
💡 Result:
Yes. GET /app/installations/{installation_id} requires app authentication (a JWT) — installation access tokens (or user tokens) are not accepted. [1][2]
Sources:
[1] GitHub REST API — "Get an installation for the authenticated app" (requires JWT).
[2] GitHub docs — Authenticating as a GitHub App installation (explains which endpoints accept installation tokens).
---
🏁 Script executed:
```shell
#!/bin/bash
# Find the createInstallationOctokit function implementation
rg -nA 15 'export.*createInstallationOctokit|function createInstallationOctokit' --type ts --type tsx
Length of output: 27
🏁 Script executed:
#!/bin/bash
# Find getUserGitHubInstallation implementation
rg -nA 20 'export.*getUserGitHubInstallation|function getUserGitHubInstallation' --type ts --type tsxLength of output: 27
🏁 Script executed:
#!/bin/bash
# Search for how Octokit is initialized in the GitHub integration
fd -e ts -e tsx -type f | xargs grep -l 'Octokit' | grep -E '(github|integration)' | head -5Length of output: 170
🏁 Script executed:
#!/bin/bash
# Look at the directory structure for github-related files
find apps/web/client/src -type f -name '*github*' | head -20Length of output: 103
---
🏁 Script executed:
```shell
#!/bin/bash
# Find the getUserGitHubInstallation function implementation
rg -n 'getUserGitHubInstallation' apps/web/client/src/server/api/routers/github.ts -A 20
Length of output: 5888
🏁 Script executed:
#!/bin/bash
# Find createInstallationOctokit function
rg -n 'createInstallationOctokit' apps/web/client/src -r . -A 15Length of output: 2672
🏁 Script executed:
#!/bin/bash
# Get full github.ts router file to understand context
cat apps/web/client/src/server/api/routers/github.tsLength of output: 12978
🏁 Script executed:
#!/bin/bash
# Search for Octokit class imports and initialization
rg -n 'from.*octokit|import.*Octokit' apps/web/client/src/server/api/routers/github.ts -B 2 -A 2Length of output: 359
---
🏁 Script executed:
```shell
#!/bin/bash
# Find the @onlook/github package location
find . -type f -name "package.json" | xargs grep -l '"name".*github' | head -5
Length of output: 30
🏁 Script executed:
#!/bin/bash
# Search for github-related packages in packages directory
find packages -type d -name "*github*" 2>/dev/nullLength of output: 15
🏁 Script executed:
#!/bin/bash
# Look for createInstallationOctokit implementation
find . -type f \( -name "*.ts" -o -name "*.tsx" \) | xargs grep -l "export.*createInstallationOctokit" 2>/dev/nullLength of output: 29
🏁 Script executed:
#!/bin/bash
# Check packages directory structure
ls -la packages/ 2>/dev/null | head -20Length of output: 1068
---
🏁 Script executed:
```shell
#!/bin/bash
# Read the createInstallationOctokit implementation
cat packages/github/src/auth.ts
Length of output: 713
🏁 Script executed:
#!/bin/bash
# Check for any related auth setup or environment variables
rg -n "installation.*token|JWT|bearer|auth" packages/github/src/auth.ts -A 3 -B 3Length of output: 601
Remove incorrect auth concern; address as any type casting violation.
The auth mechanism is correct—createInstallationOctokit uses createAppAuth with App credentials (appId/privateKey), which provides JWT-based authentication required for apps.getInstallation(). No auth issue exists here.
However, the as any cast on line 136 violates the coding guidelines. Replace it with proper type guards:
- return [
- {
- id: installation.data.account.id,
- login:
- 'login' in installation.data.account
- ? installation.data.account.login
- : (installation.data.account as any).name || '',
- avatar_url: installation.data.account.avatar_url,
- description: undefined,
- },
- ];
+ const acc = installation.data.account;
+ const login =
+ typeof (acc as Record<string, unknown>).login === 'string'
+ ? (acc as Record<string, string>).login
+ : typeof (acc as Record<string, unknown>).name === 'string'
+ ? (acc as Record<string, string>).name
+ : '';
+ return [
+ {
+ id: acc.id,
+ login,
+ avatar_url: acc.avatar_url,
+ description: undefined,
+ },
+ ];📝 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.
| getOrganizations: protectedProcedure.query(async ({ ctx }) => { | |
| try { | |
| const { octokit, installationId } = await getUserGitHubInstallation( | |
| ctx.db, | |
| ctx.user.id, | |
| ); | |
| // Get installation details to determine account type | |
| const installation = await octokit.rest.apps.getInstallation({ | |
| installation_id: parseInt(installationId, 10), | |
| }); | |
| // Get installation details to determine account type | |
| const installation = await octokit.rest.apps.getInstallation({ | |
| installation_id: parseInt(installationId, 10), | |
| }); | |
| // If installed on an organization, return that organization | |
| if (installation.data.account && 'type' in installation.data.account && installation.data.account.type === 'Organization') { | |
| return [{ | |
| // If installed on an organization, return that organization | |
| if ( | |
| installation.data.account && | |
| 'type' in installation.data.account && | |
| installation.data.account.type === 'Organization' | |
| ) { | |
| return [ | |
| { | |
| id: installation.data.account.id, | |
| login: 'login' in installation.data.account ? installation.data.account.login : (installation.data.account as any).name || '', | |
| login: | |
| 'login' in installation.data.account | |
| ? installation.data.account.login | |
| : (installation.data.account as any).name || '', | |
| avatar_url: installation.data.account.avatar_url, | |
| description: undefined, // Organizations don't have descriptions in this context | |
| }]; | |
| } | |
| // If installed on a user account, return empty (no organizations) | |
| return []; | |
| } catch (error) { | |
| throw new TRPCError({ | |
| code: 'FORBIDDEN', | |
| message: 'GitHub App installation is invalid or has been revoked', | |
| cause: error | |
| }); | |
| }, | |
| ]; | |
| } | |
| }), | |
| // If installed on a user account, return empty (no organizations) | |
| return []; | |
| } catch (error) { | |
| throw new TRPCError({ | |
| code: 'FORBIDDEN', | |
| message: 'GitHub App installation is invalid or has been revoked', | |
| cause: error, | |
| }); | |
| } | |
| }), | |
| getOrganizations: protectedProcedure.query(async ({ ctx }) => { | |
| try { | |
| const { octokit, installationId } = await getUserGitHubInstallation( | |
| ctx.db, | |
| ctx.user.id, | |
| ); | |
| // Get installation details to determine account type | |
| const installation = await octokit.rest.apps.getInstallation({ | |
| installation_id: parseInt(installationId, 10), | |
| }); | |
| // If installed on an organization, return that organization | |
| if ( | |
| installation.data.account && | |
| 'type' in installation.data.account && | |
| installation.data.account.type === 'Organization' | |
| ) { | |
| const acc = installation.data.account; | |
| const login = | |
| typeof (acc as Record<string, unknown>).login === 'string' | |
| ? (acc as Record<string, string>).login | |
| : typeof (acc as Record<string, unknown>).name === 'string' | |
| ? (acc as Record<string, string>).name | |
| : ''; | |
| return [ | |
| { | |
| id: acc.id, | |
| login, | |
| avatar_url: acc.avatar_url, | |
| description: undefined, | |
| }, | |
| ]; | |
| } | |
| // If installed on a user account, return empty (no organizations) | |
| return []; | |
| } catch (error) { | |
| throw new TRPCError({ | |
| code: 'FORBIDDEN', | |
| message: 'GitHub App installation is invalid or has been revoked', | |
| cause: error, | |
| }); | |
| } | |
| }), |
| getRepositoriesWithOAuth: protectedProcedure | ||
| .input( | ||
| z | ||
| .object({ | ||
| page: z.number().default(1), | ||
| perPage: z.number().default(100), | ||
| }) | ||
| .optional(), | ||
| ) | ||
| .query(async ({ input }) => { | ||
| try { | ||
| const { octokit } = await getUserGitHubOAuth(); | ||
|
|
||
| const { data: user } = await octokit.rest.users.getAuthenticated(); | ||
|
|
||
| const { data: allRepos } = await octokit.rest.repos.listForAuthenticatedUser({ | ||
| per_page: input?.perPage ?? 100, | ||
| page: input?.page ?? 1, | ||
| sort: 'updated', | ||
| affiliation: 'owner,collaborator,organization_member', | ||
| }); | ||
|
|
||
| // Filter to only include repos owned by the user or organizations | ||
| // Exclude repos owned by other individual users | ||
| const repos = allRepos.filter((repo) => { | ||
| const isOwnedByAuthUser = repo.owner.login === user.login; | ||
| const isOrganization = repo.owner.type === 'Organization'; | ||
| return isOwnedByAuthUser || isOrganization; | ||
| }); | ||
|
|
||
| // Extract unique organizations from repository owners | ||
| const ownerMap = new Map(); | ||
| repos.forEach((repo) => { | ||
| if (!ownerMap.has(repo.owner.login)) { | ||
| ownerMap.set(repo.owner.login, { | ||
| id: repo.owner.id, | ||
| login: repo.owner.login, | ||
| avatar_url: repo.owner.avatar_url, | ||
| }); | ||
| } | ||
| }); | ||
|
|
||
| const organizations = Array.from(ownerMap.values()); | ||
|
|
||
| return { repos, organizations }; | ||
| } catch (error) { | ||
| throw new TRPCError({ | ||
| code: 'FORBIDDEN', | ||
| message: | ||
| 'GitHub OAuth access is invalid or has been revoked. Please reconnect your GitHub account.', | ||
| cause: error, | ||
| }); | ||
| } | ||
| }), | ||
| }); |
There was a problem hiding this comment.
OAuth repos: fix organizations list, clamp inputs, and align repo shape.
organizationsincludes the current user; filter to orgs only.- Clamp
perPage(1..100) andpage(>=1). - Map repos to the same shape as App flow for consistency.
- getRepositoriesWithOAuth: protectedProcedure
- .input(
- z
- .object({
- page: z.number().default(1),
- perPage: z.number().default(100),
- })
- .optional(),
- )
+ getRepositoriesWithOAuth: protectedProcedure
+ .input(
+ z
+ .object({
+ page: z.number().int().min(1).default(1),
+ perPage: z.number().int().min(1).max(100).default(100),
+ })
+ .optional(),
+ )
.query(async ({ input }) => {
try {
const { octokit } = await getUserGitHubOAuth();
const { data: user } = await octokit.rest.users.getAuthenticated();
- const { data: allRepos } = await octokit.rest.repos.listForAuthenticatedUser({
+ const { data: allRepos } = await octokit.rest.repos.listForAuthenticatedUser({
per_page: input?.perPage ?? 100,
page: input?.page ?? 1,
sort: 'updated',
affiliation: 'owner,collaborator,organization_member',
});
- // Filter to only include repos owned by the user or organizations
- // Exclude repos owned by other individual users
+ // Keep repos owned by the user or orgs; exclude repos owned by other users
const repos = allRepos.filter((repo) => {
const isOwnedByAuthUser = repo.owner.login === user.login;
const isOrganization = repo.owner.type === 'Organization';
return isOwnedByAuthUser || isOrganization;
});
- // Extract unique organizations from repository owners
- const ownerMap = new Map();
+ // Extract unique organizations from repository owners (orgs only)
+ const ownerMap = new Map<string, { id: number; login: string; avatar_url: string }>();
repos.forEach((repo) => {
- if (!ownerMap.has(repo.owner.login)) {
+ if (repo.owner.type !== 'Organization') return;
+ if (!ownerMap.has(repo.owner.login)) {
ownerMap.set(repo.owner.login, {
id: repo.owner.id,
login: repo.owner.login,
avatar_url: repo.owner.avatar_url,
});
}
});
const organizations = Array.from(ownerMap.values());
- return { repos, organizations };
+ // Normalize repo shape to match App flow
+ const normalized = repos.map((repo) => ({
+ id: repo.id,
+ name: repo.name,
+ full_name: repo.full_name,
+ description: repo.description,
+ private: repo.private,
+ default_branch: repo.default_branch,
+ clone_url: repo.clone_url,
+ html_url: repo.html_url,
+ updated_at: repo.updated_at,
+ owner: { login: repo.owner.login, avatar_url: repo.owner.avatar_url },
+ }));
+ return { repos: normalized, organizations };
} catch (error) {
throw new TRPCError({
code: 'FORBIDDEN',
message:
'GitHub OAuth access is invalid or has been revoked. Please reconnect your GitHub account.',
cause: error,
});
}
}),As per coding guidelines.
🤖 Prompt for AI Agents
In apps/web/client/src/server/api/routers/github.ts around lines 302–356, the
handler currently returns organizations that include the authenticated user,
doesn't clamp input page/perPage, and returns raw GitHub repo objects instead of
the app's canonical repo shape. Update the input schema to enforce/clamp perPage
to 1..100 and page to >=1 (e.g. z.number().int().min(1) for page and
z.number().int().min(1).max(100) for perPage or clamp values before calling the
API), filter organizations to only entries where owner.type === 'Organization'
and exclude user.login, and map each repo to the app's expected shape (pick and
transform fields to match the App flow: id, name, fullName, ownerLogin,
ownerAvatar, private, htmlUrl, description, fork, archived, pushedAt/updatedAt,
etc.) before returning repos and organizations. Ensure types stay consistent and
preserve pagination params when calling octokit.
| **Issue**: Console.error usage, poor error context | ||
| - Replace console.error with proper logging/telemetry | ||
| - Add structured error tracking (PostHog/Sentry integration) | ||
| - Improve error messages for user troubleshooting | ||
| - **Files**: All `_hooks/*.ts` files, `github.ts` | ||
|
|
||
| ## Phase 2: Feature Enhancements (Medium Priority) | ||
|
|
||
| ### 2.1 Branch Selection | ||
| **Issue**: Always imports default branch | ||
| - Add branch selector in setup UI | ||
| - Fetch available branches via GitHub API |
There was a problem hiding this comment.
Redact example secrets to avoid false-positive secret scans.
Replace sample client secrets/private keys with placeholders.
-# New OAuth variables
-GITHUB_CLIENT_ID=Iv1.abc123def456
-GITHUB_CLIENT_SECRET=abc123def456ghi789jkl012mno345pqr678stu
+# New OAuth variables (example placeholders)
+GITHUB_CLIENT_ID=Iv1.<your_client_id>
+GITHUB_CLIENT_SECRET=<your_client_secret>Also ensure any private key blocks use obvious placeholders (e.g., -----BEGIN PRIVATE KEY-----\n\n-----END PRIVATE KEY-----).
Also applies to: 160-176
🤖 Prompt for AI Agents
In docs/github-import-production-plan.md around lines 56 to 67 (and also check
lines 160 to 176), the file contains example client secrets/private keys that
could trigger secret-scanning false positives; replace any real-looking secrets
with clear placeholders (e.g., CLIENT_ID=<redacted>, CLIENT_SECRET=<redacted>)
and for private key blocks use an obvious placeholder block such as -----BEGIN
PRIVATE KEY-----\n<redacted>\n-----END PRIVATE KEY-----; ensure all occurrences
across the document and referenced files are updated accordingly.
| # New OAuth variables | ||
| GITHUB_CLIENT_ID=Iv1.abc123def456 | ||
| GITHUB_CLIENT_SECRET=abc123def456ghi789jkl012mno345pqr678stu | ||
| ``` |
There was a problem hiding this comment.
Redact example OAuth credentials.
Prevent secret scanners from flagging docs.
-# New OAuth variables
-GITHUB_CLIENT_ID=Iv1.abc123def456
-GITHUB_CLIENT_SECRET=abc123def456ghi789jkl012mno345pqr678stu
+# New OAuth variables (examples)
+GITHUB_CLIENT_ID=Iv1.<your_client_id>
+GITHUB_CLIENT_SECRET=<your_client_secret>🧰 Tools
🪛 Gitleaks (8.28.0)
[high] 66-66: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🤖 Prompt for AI Agents
In docs/github-oauth-setup.md around lines 64 to 67, the example OAuth
credentials contain realistic-looking secrets that can trigger secret scanners;
replace those literal values with non-sensitive placeholders (e.g.
GITHUB_CLIENT_ID=YOUR_CLIENT_ID and GITHUB_CLIENT_SECRET=YOUR_CLIENT_SECRET) and
update the surrounding text to indicate they are examples/placeholders so no
real tokens appear in the repo.
1 participant
Description
Related Issues
Type of Change
Testing
Screenshots (if applicable)
Additional Notes
Important
Revamps GitHub import flow by integrating OAuth for repository discovery and maintaining GitHub App for access, with updates to UI, backend, and documentation.
login()inactions.tsxto request all scopes upfront for GitHub.connectGitHubOAuth()inactions.tsto handle OAuth connection.OAuthConnectandImportFlowcomponents for handling OAuth and import UI.SetupGithubto use OAuth for repo listing and selection.ImportingPagefor better UX.githubOAuthRouterfor handling OAuth-related API calls.githubRouterto include OAuth-based repo validation and listing.github-app-setup-testing.mdandgithub-oauth-setup.mdfor setup guidance.github-import-production-plan.mdwith new OAuth flow details.index.tsinpackages/githubin favor of tRPC inferred types.This description was created by
for c244639. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
Release Notes
New Features
Improvements
Documentation