Skip to content

fix: fix webhook pr body data access Error#45

Merged
afuhflynn merged 1 commit intomainfrom
chore/octokit-app-upgrade
Dec 29, 2025
Merged

fix: fix webhook pr body data access Error#45
afuhflynn merged 1 commit intomainfrom
chore/octokit-app-upgrade

Conversation

@afuhflynn
Copy link
Copy Markdown
Owner

@afuhflynn afuhflynn commented Dec 29, 2025

Summary by CodeRabbit

  • Chores

    • Removed debugging statements from webhook handling.
    • Cleaned up unused code references in internal workflows.
  • Refactor

    • Optimized authentication mechanism for PR summary generation by streamlining repository and account resolution.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Dec 29, 2025

📝 Walkthrough

Walkthrough

These changes refactor the PR summary generation flow by removing debug logging, eliminating unused destructured variables, and replacing session-based account retrieval with repository-based GitHub account resolution and validation.

Changes

Cohort / File(s) Summary
Debug and dependency cleanup
app/api/webhooks/github/route.ts, inngest/functions/index.ts
Removes debug console.log from webhook handler and eliminates unused installationId destructuring from summarizePr function event data.
Authentication flow refactoring
lib/ai/actions/index.ts
Replaces session-based account lookup with repository-name-based resolution. Adds validation for repository existence and GitHub account availability, retrieves GitHub access token from repository owner's accounts, and restructures pr.summary.requested event payload to pass owner, repoName, accountId, and installationId via resolved githubAccount.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Poem

🐰 No more sessions, just repos we seek,
Where GitHub accounts grow strong and sleek,
Debug logs gone, the code runs clean,
Validation guards what lies between,
A tidy flow from hook to team! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title check ❓ Inconclusive The title mentions fixing a 'webhook pr body data access Error', but the changes primarily refactor how PR summaries are retrieved and processed—removing debug logs, eliminating token-based session retrieval, and switching to repository-based account lookup. The title is partially related to a technical fix but does not clearly summarize the main refactoring effort. Consider a more specific title that reflects the primary change, such as: 'refactor: switch PR summary to repository-based account lookup' or 'refactor: replace token-based session with repository account resolution'.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch chore/octokit-app-upgrade

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
lib/ai/actions/index.ts (1)

195-198: Typo in return key: "sucess" → "success".

This typo also appears on line 105. Consumers expecting success will get unexpected behavior.

🔎 Proposed fix
     return {
-      sucess: true,
+      success: true,
       message: "PR Summary Queued",
     };
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bf55216 and a5b1a19.

📒 Files selected for processing (3)
  • app/api/webhooks/github/route.ts
  • inngest/functions/index.ts
  • lib/ai/actions/index.ts
💤 Files with no reviewable changes (2)
  • app/api/webhooks/github/route.ts
  • inngest/functions/index.ts
🔇 Additional comments (1)
lib/ai/actions/index.ts (1)

153-169: LGTM on the repository lookup pattern.

The Prisma query correctly includes the nested owner accounts filtered by GitHub provider.

Comment on lines +171 to +174
if (!repository)
throw new Error(
`Repository ${owner}/${repoName} not found in databse. Please reconnect the repository.`
);
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 | 🟡 Minor

Typo in error message: "databse" → "database".

This typo also appears on line 33.

🔎 Proposed fix
     if (!repository)
       throw new Error(
-        `Repository ${owner}/${repoName} not found in databse. Please reconnect the repository.`
+        `Repository ${owner}/${repoName} not found in database. Please reconnect the repository.`
       );
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (!repository)
throw new Error(
`Repository ${owner}/${repoName} not found in databse. Please reconnect the repository.`
);
if (!repository)
throw new Error(
`Repository ${owner}/${repoName} not found in database. Please reconnect the repository.`
);
🤖 Prompt for AI Agents
In lib/ai/actions/index.ts around lines 171-174 (and also update the same typo
on line 33), fix the misspelled word "databse" to "database" in the thrown Error
message(s). Update both occurrences so the message reads "...not found in
database. Please reconnect the repository." and save/compile to verify no lint
or build errors.

`Repository ${owner}/${repoName} not found in databse. Please reconnect the repository.`
);

const githubAccount = repository.owner.accounts[0];
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

Missing validation: accounts[0] may be undefined if no GitHub account is linked.

If the repository owner has no connected GitHub account, accounts will be an empty array, making githubAccount undefined. Accessing githubAccount.accountId on line 185 will throw a TypeError.

🔎 Proposed fix
     const githubAccount = repository.owner.accounts[0];
+
+    if (!githubAccount) {
+      throw new Error(
+        `No GitHub account found for the owner of repository ${owner}/${repoName}.`
+      );
+    }
+
     await inngest.send({
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const githubAccount = repository.owner.accounts[0];
const githubAccount = repository.owner.accounts[0];
if (!githubAccount) {
throw new Error(
`No GitHub account found for the owner of repository ${owner}/${repoName}.`
);
}
await inngest.send({
🤖 Prompt for AI Agents
In lib/ai/actions/index.ts around line 176, the code assumes
repository.owner.accounts[0] exists; if the owner has no linked GitHub account
this will be undefined and later access to githubAccount.accountId will throw.
Add validation: check that repository.owner.accounts is a non-empty array (or
find the account with provider === 'github'), and if no GitHub account is
present either return/throw a clear, handled error or branch to the non-GitHub
flow; ensure subsequent code uses the validated githubAccount object (or early
exits) so no property access occurs on undefined.

@afuhflynn afuhflynn merged commit 578a3ea into main Dec 29, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant