Skip to content

feat: add some new changes #58

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
afuhflynn merged 1 commit into from
Dec 29, 2025
Merged

feat: add some new changes #58

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion app/api/webhooks/github/route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,6 @@ export async function POST(request: NextRequest) {
owner,
repoName,
prNumber,
body?.installation?.id,
pull_request?.base?.sha,
pull_request?.head?.sha
)
Expand Down
24 changes: 22 additions & 2 deletions lib/ai/actions/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ export async function reviewPullRequest(
owner: string,
repo: string,
prNumber: number,
installationId: number,
baseSha: string,
headSha: string
) {
Expand Down Expand Up @@ -87,6 +86,27 @@ export async function reviewPullRequest(
},
});

const account = await prisma.account.findFirst({
where: {
accountId: String(githubAccount.accountId),
},
include: {
user: true,
},
});

if (!account) {
throw new Error("Invalid account");
}
const installation = await prisma.installation.findFirst({
where: {
userId: account.user.id,
},
select: {
installationId: true,
},
});
Comment on lines +89 to +108

@coderabbitai coderabbitai Bot Dec 29, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

# First, let's get the full context of the file around lines 38-120
head -n 150 lib/ai/actions/index.ts | tail -n 120

Repository: afuhflynn/code-reverb

Length of output: 2976

🏁 Script executed:

# Search for the pr.summary.requested event handler and installationId usage
rg -n "pr\.summary\.requested" --type ts -A 10 -B 2

Repository: afuhflynn/code-reverb

Length of output: 1450

🏁 Script executed:

# Look for how installationId is used in the codebase
rg -n "installationId" --type ts -C 5 | head -n 100

Repository: afuhflynn/code-reverb

Length of output: 5747

🏁 Script executed:

# Check the database schema/Prisma models to understand type consistency
find . -name "*.prisma" -o -name "schema.prisma" | head -5

Repository: afuhflynn/code-reverb

Length of output: 87

🏁 Script executed:

# Check the Prisma schema to understand the type of accountId and userId relationships
cat prisma/schema.prisma | grep -A 20 "model Account"

Repository: afuhflynn/code-reverb

Length of output: 741

🏁 Script executed:

# Check the Installation model schema
cat prisma/schema.prisma | grep -A 20 "model Installation"

Repository: afuhflynn/code-reverb

Length of output: 729

🏁 Script executed:

# Check how getOctokitForInstallation handles null installationId
rg -n "getOctokitForInstallation" --type ts -A 10 -B 2

Repository: afuhflynn/code-reverb

Length of output: 5122

🏁 Script executed:

# Check if there's any null handling for installationId in the summarizePr event handler
rg -n "summarizePr" --type ts -A 30 | grep -A 30 "installationId"

Repository: afuhflynn/code-reverb

Length of output: 1639

🏁 Script executed:

# Check the full implementation of getOctokitForInstallation to see if it handles null
cat config/octokit-instance.ts

Repository: afuhflynn/code-reverb

Length of output: 870

🏁 Script executed:

# Check if there's any null checking for installationId in the summarizePr event handler before calling getOctokitForInstallation
rg -n "summarizePr" --type ts -A 50 | head -80

Repository: afuhflynn/code-reverb

Length of output: 3714

🏁 Script executed:

# Get the full summarizePr function implementation to see how installationId is used
sed -n '322,450p' inngest/functions/index.ts

Repository: afuhflynn/code-reverb

Length of output: 3446

Address error handling inconsistency and prevent potential null reference errors in downstream code.

The account and installation lookup pattern is not redundant—it's a necessary two-step flow to get account.user.id which is required to find the installation. However, the code has two issues:

  1. Error handling inconsistency (lines 98-100, 101-108): An error is thrown when the account is not found, but null is allowed when the installation is not found. This inconsistency is compounded by the fact that the null installationId is then passed directly to postSummaryAsUser() → getOctokitForInstallation() at line 119. The getOctokitForInstallation function expects a number but receives number | null, with no null checks, which will cause a runtime error.

  2. Missing null check: Before sending the pr.summary.requested event with a potentially null installationId, validate that the installation was found or decide whether this is a critical failure. If it's optional, ensure downstream handlers in summarizePr (lines 388-391 in inngest/functions/index.ts) properly guard against null before calling getOctokitForInstallation.

🤖 Prompt for AI Agents 
In lib/ai/actions/index.ts around lines 89 to 108, the account lookup throws
when missing but the installation lookup allows null and its installationId
(possibly null) is passed to postSummaryAsUser/getOctokitForInstallation; make
the behavior consistent by validating the installation result and handling null:
either throw an error when no installation is found (recommended if the
operation requires an installation) or explicitly handle the optional case
before calling postSummaryAsUser (e.g., early-return, send an event without
installationId, or branch logic) and ensure any call to
getOctokitForInstallation only receives a non-null number after a check; update
downstream callers or add guards so getOctokitForInstallation never receives
number | null.


await inngest.send({
name: "pr.summary.requested",
data: {
Expand All @@ -96,7 +116,7 @@ export async function reviewPullRequest(
title: title ?? "",
description: description ?? "",
accountId: githubAccount.accountId,
installationId: installationId ?? null,
installationId: installation?.installationId ?? null,
baseSha,
headSha,
changedFiles: changed_files ?? 0,
Expand Down