feat: add GitHub API fallback for installation check #55
Conversation
When local DB is out of sync (webhook delays, manual purges, restoration),
isRepoInstalled() now falls back to checking GitHub's API directly.
Changes:
- Add checkRepoInstallationFromAPI() to query GET /repos/{owner}/{repo}/installation
- Update isRepoInstalled() to use API fallback when DB check returns null
- Auto-sync installation data to DB when found via API
- Upgrade any polling subscriptions to webhook delivery after sync
Handles edge cases:
- Webhook delivery failures/delays
- Manual DB purges (dev/testing)
- DB restoration from backups
- Initial deployment with pre-existing installations
Rate limit: Uses app authentication (15,000/hour limit). DB-first approach
minimizes API calls - only hit API on cache miss, then sync for future lookups.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
WalkthroughisRepoInstalled in src/github-app/installation-service.ts now attempts a fast DB lookup for an installationId and, on miss, validates the repo string and calls a new private helper checkRepoInstallationFromAPI(owner, repo). That helper calls the GitHub API GET /repos/{owner}/{repo}/installation, verifies the app is enabled, synchronizes the installation record into the DB, inserts the repo mapping, and triggers subscription upgrade logic. It returns the installationId on success and null on API 404 or other failures (with non-404 errors logged). Public API remains unchanged. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/github-app/installation-service.ts (2)
134-151: Fallback flow inisRepoInstalledlooks good; consider slightly stricter repo parsingThe DB‑first path and API fallback are wired correctly and keep the hot path cheap. The only thing I’d tighten here is validation of the
repostring before calling the API so bad inputs don’t silently turn into unexpected requests:
- Right now, any string with at least one
/will be split and partially used; malformed values like"foo/bar/baz"or leading/trailing whitespace will still hit the API.- Consider normalizing and strictly enforcing the
"owner/repo"shape, and optionally logging invalid inputs to make upstream issues easier to debug.For example:
- const [owner, repoName] = repo.split("/"); - if (!owner || !repoName) { - return null; - } + const parts = repo.trim().split("/"); + if (parts.length !== 2) { + console.warn( + `[InstallationService] isRepoInstalled called with invalid repo format: ${repo}` + ); + return null; + } + const [owner, repoName] = parts;This keeps the behavior the same on valid inputs but makes misuse more visible and avoids odd partial matches.
161-198: API fallback helper is solid; consider logging when disabled and tightening error typingThe helper cleanly encapsulates the fallback behavior (guarding on
isEnabled, doing the API lookup, syncing installation + repo, and upgrading subscriptions). Two small polish points:
- When
this.githubApp.isEnabled()is false you silently returnnull. Emitting a one‑line warning here would make misconfiguration much easier to spot in logs:- if (!this.githubApp.isEnabled()) { - return null; - } + if (!this.githubApp.isEnabled()) { + console.warn( + "[InstallationService] GitHub App is disabled; skipping API installation check" + ); + return null; + }
- The 404 handling currently relies on
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-accessand a loose cast. You can usually avoid the lint suppression by first checking thaterroris an object with a numericstatus:- } catch (error) { - // 404 = app not installed on this repo - // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - if ((error as any)?.status === 404) { - return null; - } + } catch (error) { + // 404 = app not installed on this repo + const status = + error && + typeof error === "object" && + "status" in error && + typeof (error as { status?: unknown }).status === "number" + ? (error as { status: number }).status + : undefined; + if (status === 404) { + return null; + }Behavior stays the same, but you drop the
anyholes and linter suppression.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/github-app/installation-service.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/github-app/installation-service.ts (2)
src/db/index.ts (1)
db(57-57)src/db/schema.ts (1)
installationRepositories(149-167)
- Add strict validation for "owner/repo" format in repository strings - Log a warning for invalid formats and return null early - Prevent unnecessary API calls for malformed repo inputs
When local DB is out of sync (webhook delays, manual purges, restoration), isRepoInstalled() now falls back to checking GitHub's API directly.
Changes:
Handles edge cases:
Rate limit: Uses app authentication (15,000/hour limit). DB-first approach minimizes API calls - only hit API on cache miss, then sync for future lookups.
🤖 Generated with Claude Code
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.