feat: implement github profile sync and manual refresh endpoint

[truemperor]

closes #117

[devasign-app]

Merge Score: 85/100

🟢 █████████████████░░░ 85%

The PR successfully implements GitHub profile synchronization and adds a manual refresh endpoint. The database migration and tests are well-structured. However, the manual sync endpoint should return the actual database state to prevent overwriting the frontend's email with null, and the GitHub API call should include an authorization token to avoid strict rate limits and handle 404/403 errors gracefully.

Code Suggestions (3)

Medium Priority (2)

1. packages/api/src/routes/users.ts (Line 29)
   Return the actual database state to avoid overwriting the frontend's email with null.

Reasoning: If the public GitHub profile does not expose an email, githubUser.email will be null. The current code avoids updating the database with null, but still returns email: null in the response, which could cause the frontend to clear the user's email in its state. Using .returning() ensures the API returns the true updated state from the database.

Suggested Code:

        const [updatedUser] = await db.update(users)
            .set({
                avatarUrl: githubUser.avatar_url,
                ...(githubUser.email ? { email: githubUser.email } : {}),
                publicRepos: githubUser.public_repos || 0,
                updatedAt: new Date(),
            })
            .where(eq(users.id, user.id))
            .returning();

        return c.json({
            success: true,
            avatarUrl: updatedUser.avatarUrl,
            email: updatedUser.email,
            publicRepos: updatedUser.publicRepos,
        });

2. packages/api/src/services/github.ts (Line 108)
   Add authorization header to GitHub API request to avoid strict rate limits, and handle 404/403 errors gracefully.

Reasoning: The unauthenticated GitHub API is limited to 60 requests per hour per IP. If multiple users sync their profiles, the server will quickly hit this limit. Using a server-side GitHub token (if available) increases this limit to 5,000 requests per hour. Additionally, handling 404 (username changed) and 403 (rate limit exceeded) provides better error messages to the client.

Suggested Code:

    async getUserProfileByUsername(username: string): Promise<GitHubUser> {
        const headers: Record<string, string> = {
            Accept: 'application/vnd.github.v3+json',
            'User-Agent': 'Devasign-API',
        };

        if (process.env.GITHUB_TOKEN) {
            headers.Authorization = `Bearer ${process.env.GITHUB_TOKEN}`;
        }

        const response = await fetch(`https://api.github.com/users/${username}`, {
            headers,
        });

        if (!response.ok) {
            if (response.status === 404) {
                throw new Error('GitHub user not found. If your username changed, please log in again.');
            }
            if (response.status === 403 && response.headers.get('x-ratelimit-remaining') === '0') {
                throw new Error('GitHub API rate limit exceeded. Please try again later.');
            }
            throw new Error(`GitHub API error: ${response.statusText}`);
        }

        return await response.json() as GitHubUser;
    }

Low Priority (1)

1. packages/api/src/tests/users.test.ts (Line 16)
   Update the database mock to support .returning().

Reasoning: Since users.ts should be updated to use .returning() to fetch the updated user record, the mock in the test file needs to be updated to chain .returning() and return a mock user object.

Suggested Code:

        update: vi.fn(() => ({
            set: vi.fn(() => ({
                where: vi.fn(() => ({
                    returning: vi.fn(() => [{
                        avatarUrl: 'https://synced.avatar.url',
                        email: 'test@example.com',
                        publicRepos: 42
                    }])
                })),
            })),
        })),

📊 Review Metadata

• Processing Time: 106s
• Analysis Date: 4/9/2026, 1:40:35 PM

🤖 This review was generated by AI. While we strive for accuracy, please use your judgment when applying suggestions.

💬 Questions about this review? Open an issue or contact support.

[devasign-app]

Updated Merge Score: 95/100

🟢 ███████████████████░ 95%

The author successfully addressed all concerns from the previous review. The manual sync endpoint now correctly returns the updated database state using .returning(), preventing accidental null overwrites on the frontend. The GitHub API service now includes authorization headers (if configured) and gracefully handles 404 and 403 rate-limit errors. Tests were also appropriately updated. One minor edge case remains: ensuring updatedUser is defined before accessing its properties to prevent potential runtime errors.

Code Suggestions (1)

Low Priority (1)

1. packages/api/src/routes/users.ts (Line 41)
   Add a check to ensure updatedUser is defined before accessing its properties.

Reasoning: Although the user is authenticated, if the database update fails to match a row (e.g., due to a deleted user or race condition), updatedUser will be undefined. Accessing updatedUser.avatarUrl will throw a TypeError and cause an unhandled exception.

Suggested Code:

        if (!updatedUser) {
            return c.json({ error: 'User not found in database' }, 404);
        }

        // Return updated attributes
        return c.json({
            success: true,
            avatarUrl: updatedUser.avatarUrl,
            email: updatedUser.email,
            publicRepos: updatedUser.publicRepos,
        });

📊 Review Metadata

• Processing Time: 17s
• Analysis Date: 4/9/2026, 1:43:15 PM

🤖 This review was generated by AI. While we strive for accuracy, please use your judgment when applying suggestions.

💬 Questions about this review? Open an issue or contact support.

[devasign-app]

Updated Merge Score: 100/100

🟢 ████████████████████ 100%

The author successfully addressed the final edge case from the previous review by adding a null check for updatedUser before accessing its properties in the /me/sync endpoint. This prevents potential runtime errors if the database update fails to return a record. The PR is now robust, well-tested, and ready to be merged. No new issues were introduced.

Code Suggestions

Great job! No specific suggestions at this time.

📊 Review Metadata

• Processing Time: 12s
• Analysis Date: 4/9/2026, 1:44:39 PM

🤖 This review was generated by AI. While we strive for accuracy, please use your judgment when applying suggestions.

💬 Questions about this review? Open an issue or contact support.