fix(gh_pr, gh_issue): show GitHub App install link when private repo … #64
Conversation
…access fails When a user has OAuth linked but the GitHub App isn't installed on a private repo, show a targeted install URL instead of generic error. - Add getOwnerIdFromUsername() to fetch owner ID from public GitHub API - Add generateInstallUrl() and sendInstallPrompt() to installation-service - Refactor parseRepo() to return tuple [owner, repo] 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
WalkthroughThis PR standardizes repo handling by switching public API functions to accept Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Areas requiring extra attention:
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (2)
🧰 Additional context used📓 Path-based instructions (2)**/*.ts📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
🔇 Additional comments (10)
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 (4)
src/handlers/gh-issue-handler.ts (2)
85-88: Consider more granular error handling for the install prompt.The
catchblock now callssendInstallPromptwhich itself can throw (e.g., ifgetOwnerIdFromUsernamefails orhandler.sendMessagefails). IfsendInstallPromptthrows, the error is silently swallowed, leaving the user with no feedback.Consider wrapping
sendInstallPromptin a try-catch with a fallback message:} catch { - await sendInstallPrompt(handler, channelId, repo); + try { + await sendInstallPrompt(handler, channelId, repo); + } catch { + await handler.sendMessage( + channelId, + "❌ Cannot access this repository. Please check your permissions or install the GitHub App." + ); + } return; }
167-170: Same error handling consideration applies here.For consistency and robustness, consider adding the same fallback error handling suggested for
handleShowIssue(line 86).src/github-app/installation-service.ts (1)
27-41: Consider validating repo format before parsing.If
repoFullNameis malformed (e.g., missing "/"),parseRepowill return[undefined, undefined], andgetOwnerIdFromUsername(undefined)will likely fail or produce unexpected behavior.export async function sendInstallPrompt( handler: BotHandler, channelId: string, repoFullName: string ): Promise<void> { const [owner] = parseRepo(repoFullName); + if (!owner) { + await handler.sendMessage( + channelId, + `❌ Invalid repository format: ${repoFullName}` + ); + return; + } const ownerId = await getOwnerIdFromUsername(owner);src/api/github-client.ts (1)
39-42: Consider adding input validation toparseRepo.The function doesn't validate that the input contains a "/" or that both owner and repo are non-empty. This could cause downstream issues when the returned values are used in API calls.
export function parseRepo(repoFullName: string): [owner: string, repo: string] { const [owner, repo] = repoFullName.split("/"); + if (!owner || !repo) { + throw new Error(`Invalid repository format: "${repoFullName}". Expected "owner/repo".`); + } return [owner, repo]; }Alternatively, if you prefer to keep this as a utility that doesn't throw, consider returning a union type or documenting the behavior.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/api/github-client.ts(5 hunks)src/api/user-oauth-client.ts(0 hunks)src/github-app/installation-service.ts(2 hunks)src/handlers/gh-issue-handler.ts(3 hunks)src/handlers/gh-pr-handler.ts(3 hunks)src/services/subscription-service.ts(6 hunks)
💤 Files with no reviewable changes (1)
- src/api/user-oauth-client.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.ts: Store context externally - maintain stateless bot architecture with no message history, thread context, or conversation memory
Use<@{userId}>for mentions in messages AND add mentions in sendMessage options - do not use @username format
Implement event handlers for onMessage, onSlashCommand, onReaction, onTip, and onInteractionResponse to respond to Towns Protocol events
Define slash commands in src/commands.ts as a const array with name and description properties, then register handlers using bot.onSlashCommand()
Set ID in interaction requests and match ID in responses to correlate form submissions, button clicks, and transaction/signature responses
Use readContract for reading smart contract state, writeContract for SimpleAccount operations, and execute() for external contract interactions
Fund bot.appAddress (Smart Account) for on-chain operations, not bot.botId (Gas Wallet/EOA)
Use bot.* handler methods directly (outside event handlers) for unprompted messages via webhooks, timers, or tasks - requires channelId, spaceId, or other context stored externally
Always check permissions using handler.hasAdminPermission() before performing admin operations like ban, redact, or pin
User IDs are hex addresses in format 0x..., not usernames - use them consistently throughout event handling and message sending
Slash commands do not trigger onMessage - register slash command handlers using bot.onSlashCommand() instead
Use getSmartAccountFromUserId() to retrieve a user's wallet address from their userId
Include required environment variables: APP_PRIVATE_DATA (bot credentials) and JWT_SECRET (webhook security token)
Files:
src/handlers/gh-pr-handler.tssrc/api/github-client.tssrc/handlers/gh-issue-handler.tssrc/github-app/installation-service.tssrc/services/subscription-service.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Provide alt text for image attachments and use appropriate MIME types for chunked attachments (videos, screenshots)
Files:
src/handlers/gh-pr-handler.tssrc/api/github-client.tssrc/handlers/gh-issue-handler.tssrc/github-app/installation-service.tssrc/services/subscription-service.ts
🧠 Learnings (1)
📚 Learning: 2025-11-25T03:24:12.451Z
Learnt from: CR
Repo: HereNotThere/bot-github PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T03:24:12.451Z
Learning: Applies to **/*.ts : Use bot.* handler methods directly (outside event handlers) for unprompted messages via webhooks, timers, or tasks - requires channelId, spaceId, or other context stored externally
Applied to files:
src/github-app/installation-service.ts
🧬 Code graph analysis (4)
src/handlers/gh-pr-handler.ts (1)
src/github-app/installation-service.ts (1)
sendInstallPrompt(27-41)
src/handlers/gh-issue-handler.ts (1)
src/github-app/installation-service.ts (1)
sendInstallPrompt(27-41)
src/github-app/installation-service.ts (1)
src/api/github-client.ts (2)
parseRepo(39-42)getOwnerIdFromUsername(248-265)
src/services/subscription-service.ts (2)
src/api/github-client.ts (1)
getOwnerIdFromUsername(248-265)src/github-app/installation-service.ts (1)
generateInstallUrl(18-22)
🔇 Additional comments (8)
src/handlers/gh-pr-handler.ts (1)
79-82: LGTM! Consistent pattern with issue handler.The changes mirror the issue handler implementation, maintaining consistency across the codebase. The same optional error handling improvement suggested for
gh-issue-handler.tswould apply here as well.Also applies to: 156-159
src/github-app/installation-service.ts (1)
18-22: LGTM! Clean URL generation with sensible defaults.The function handles the optional
targetIdgracefully, falling back to a generic install URL when the owner ID cannot be determined. The environment variable fallback is appropriate.src/services/subscription-service.ts (2)
3-4: LGTM! Clean refactoring to centralized utilities.The imports are well-organized, bringing in
getOwnerIdFromUsernamefrom the API layer andgenerateInstallUrlfrom the installation service. This eliminates code duplication and ensures consistent URL generation across the codebase.Also applies to: 19-22
159-179: LGTM! Proper handling of owner ID lookup for installation URL.The flow correctly fetches the owner ID from the public GitHub API and generates a targeted installation URL. When
getOwnerIdFromUsernamereturnsundefined,generateInstallUrlgracefully falls back to a generic URL withouttarget_id.src/api/github-client.ts (4)
44-86: LGTM! Consistent refactoring of API functions.The
validateRepo,getIssue, andgetPullRequestfunctions now consistently useparseRepoto extract owner/repo from the full name. The logic remains unchanged otherwise.
88-149: LGTM! Well-implemented pagination with proper safeguards.The pagination logic includes sensible safeguards:
maxPageslimit prevents excessive API calls- Early termination when enough results are collected
- Proper handling of "merged" state by filtering
merged_at- Case-insensitive author filtering
151-200: LGTM! Correctly filters out pull requests from issues endpoint.The implementation properly handles the GitHub API behavior where the issues endpoint returns both issues and PRs, filtering them at line 192.
248-265: LGTM! Robust owner ID lookup with graceful fallback.The implementation correctly:
- Tries organization first (common for private repos)
- Falls back to user lookup
- Returns
undefinedon failure, allowing callers to handle gracefully- Logs a warning for debugging
When fetching a specific PR/issue fails after OAuth retry, check repo access with validateRepo() before showing install prompt. If user has access, the 404 means the item doesn't exist. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Prevents confusing downstream errors when repo string is missing "/". The error propagates to handler's catch block showing a clear message. Updates existing tests to expect the new validation error. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
…access fails
When a user has OAuth linked but the GitHub App isn't installed on a private repo, show a targeted install URL instead of generic error.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.