feature: repo sync to gh account connected#1
Conversation
| githubAccessToken String? | ||
| githubRefreshToken String? | ||
| lastSyncedAt DateTime? | ||
| createdAt DateTime @default(now()) | ||
| updatedAt DateTime @updatedAt | ||
|
|
||
| projects Project[] | ||
| issueClaims IssueClaim[] | ||
| gigsPosted Gig[] @relation("GigClient") | ||
| proposals Proposal[] | ||
| teamsOwned Team[] @relation("TeamOwner") | ||
| teamMemberships TeamMember[] | ||
| } | ||
|
|
||
| model Project { | ||
| id String @id @default(cuid()) | ||
| githubRepoUrl String @unique | ||
| name String | ||
| description String? | ||
| language String? | ||
| stars Int @default(0) | ||
| forks Int @default(0) | ||
| topics String[] | ||
| logoUrl String? | ||
| isCollaborative Boolean @default(false) |
There was a problem hiding this comment.
Schema adds githubAccessToken, githubRefreshToken, lastSyncedAt, isCollaborative, but no migration SQL is included in the PR. PR description says to run prisma migrate dev, but reviewers/CI cannot apply changes reliably.
Suggested Fix: Commit the generated migration:
npx prisma migrate dev --name add_github_sync_fields| return prisma.project.upsert({ | ||
| where: { githubRepoUrl }, | ||
| update: { | ||
| name: repo.full_name, | ||
| description: repo.description, | ||
| language: repo.language, | ||
| stars: repo.stargazers_count, | ||
| forks: repo.forks_count, | ||
| topics: repo.topics || [], | ||
| // Preserve isCollaborative flag on update |
There was a problem hiding this comment.
Problem: Upsert is keyed only on githubRepoUrl. If User B already registered facebook/react, User A's sync runs the update branch and refreshes metadata on User B's project — no ownerId check.
Suggesting fix:
const existing = await prisma.project.findUnique({ where: { githubRepoUrl } });
if (existing && existing.ownerId !== userId) {
return null; // skip repos owned by others
}
// OR upsert with where: { githubRepoUrl, ownerId: userId } if you add a composite unique|
|
||
| // If not requesting own projects, only show collaborative public ones | ||
| if (!req.user || req.user.id !== req.query.ownerId) { | ||
| where.isCollaborative = true; | ||
| } | ||
|
|
There was a problem hiding this comment.
Problem: GET /api/projects has no auth middleware, so req.user is always undefined. The filter isCollaborative: true always applies, including for owners trying to view their own repos. This contradicts the PR verification plan (“users can still see all their own projects”).
Suggesting Fix:
Add optional auth middleware, then:
const ownerId = req.query.ownerId as string | undefined;
const isOwnList = req.user && ownerId && req.user.id === ownerId;
if (!isOwnList) {
where.isCollaborative = true;
}
if (ownerId) where.ownerId = ownerId;Or add a dedicated authenticated route: GET /api/projects/mine.
| while (hasNext) { | ||
| const { data, headers } = await client.get(`/user/repos?per_page=100&page=${page}`); | ||
| repos = [...repos, ...data]; | ||
|
|
||
| const linkHeader = headers['link']; | ||
| hasNext = linkHeader && linkHeader.includes('rel="next"'); | ||
| page++; | ||
| } | ||
| return repos; | ||
| }; |
There was a problem hiding this comment.
Problem: No try/catch around axios. GitHub 401 (expired token), 403 (missing scope), or rate-limit → unhandled rejection → process exit.
Also add import 'express-async-errors' in app.ts so controller errors reach errorHandler.
| githubAccessToken: _accessToken, | ||
| githubRefreshToken: _refreshToken, | ||
| }, | ||
| create: { | ||
| githubId: String(profile.id), | ||
| username: profile.username, | ||
| name: profile.displayName || profile.username, | ||
| email: profile.emails?.[0]?.value, | ||
| avatarUrl: profile.photos?.[0]?.value, | ||
| githubUrl: profile.profileUrl, | ||
| githubAccessToken: _accessToken, | ||
| githubRefreshToken: _refreshToken, |
There was a problem hiding this comment.
Problem: githubAccessToken stored as plain text on User. DB leak = full GitHub account access.
Fix (minimum for review): Encrypt at rest (e.g. AES with TOKEN_ENCRYPTION_KEY env var) or store in a separate secrets table. Document risk if deferring encryption to a follow-up ticket.
|
|
||
| await Promise.all(upserts); | ||
|
|
There was a problem hiding this comment.
User with 200+ repos fires 200 parallel upserts — can exhaust Prisma connection pool.
Fix: Batch in chunks (for (const chunk of chunks(repos, 20))) or use $transaction with batch size limits.
SimonGideon
left a comment
There was a problem hiding this comment.
Good progress on SPA-13: token persistence + repo sync endpoint is the right foundation. However, OAuth scopes, upsert ownership, and listProjects filtering need fixes before merge, and GitHub failures can still crash the server. Please address the inline comments and checklist below, then re-request review.
- Prisma migration committed and tested
- OAuth scopes include repo (or documented public_repo limitation)
- Upsert ownership guard — cannot modify another user's project
- listProjects visibility works for owners vs public gallery
- fetchUserRepositories error handling + async error middleware
- Endpoint to toggle isCollaborative (or updated verification plan)
- /sync-repos route registered before /:id
- Token encryption planned or implemented
Functional gaps vs SPA-13 & PR description
No endpoint to toggle isCollaborative
Problem: Verification plan says “setting isCollaborative: true makes it visible”, but the PR adds no PATCH endpoint. Cannot complete manual QA as written.
Fix: Add e.g. PATCH /api/projects/:id/collaboration with owner auth.
Happy 💯 to re-review once those are addressed.
🚀 Feature: GitHub Repository Sync (SPA-13)
This PR implements the backend synchronization flow to fetch and store a user's GitHub repositories. This allows users to import their projects into Colabs and selectively mark public repositories for collaboration.
🛠️ Key Changes
🗄️ Database Schema
githubAccessToken,githubRefreshToken, andlastSyncedAtto support authenticated API requests and enforce sync rate limits.isCollaborative boolean flag (default: false) to control public visibility in the project gallery.🔐 Authentication
auth.strategy.ts) to capture and persist access and refresh tokens during the user login/upsert process.🌐 GitHub API Integration (
src/lib/github.ts)fetchUserRepositoriesto ensure all repositories are fetched regardless of count.🔄 Sync Logic & API
🧪 Verification Plan (For Reviewer)
This PR includes a database migration. Please ensure
npx prisma migrate devis run to apply the new columns to the User and Project tables.