refactor!: support nested event handler dirs with inferred event typing - #28
Conversation
|
@calebephrem is attempting to deploy a commit to the Caleb's Projects Team on Vercel. A member of the Team first needs to authorize it. |
Summary by BeetleThis PR implements a major architectural refactoring that transforms the event handler system from a flat, event-type-based directory structure to a nested, feature-based directory structure with inferred event typing. The key innovation is introducing a
📁 File Changes Summary (Consolidated across all commits):
Total Changes: 17 files changed, +528 additions, -364 deletions🗺️ Walkthrough:graph TD
A["Event Emitted GitHub Webhook"] -->|"Probot receives"| B["index.ts Main Entry Point"]
B -->|"Recursively scans"| C["collectHandlerFiles Nested Directories"]
C -->|"Discovers handlers in"| D1["cleanup/"]
C -->|"Discovers handlers in"| D2["commands/"]
C -->|"Discovers handlers in"| D3["labeling/"]
C -->|"Discovers handlers in"| D4["standards/"]
C -->|"Discovers handlers in"| D5["linting/"]
D1 -->|"defineEventHandler"| E["EventHandlerModule events: string[] callback: function"]
D2 -->|"defineEventHandler"| E
D3 -->|"defineEventHandler"| E
D4 -->|"defineEventHandler"| E
D5 -->|"defineEventHandler"| E
E -->|"Registers with"| F["app.on Event Listener"]
F -->|"Triggers on match"| G["Handler Callback Executes Logic"]
G -->|"Returns result"| H["GitHub API Response"]
I["defineEventHandler Type-safe wrapper"] -.->|"Provides typing"| E
J["EventHandlerModule Interface"] -.->|"Defines contract"| E
🎯 Key Changes:
📊 Impact Assessment:
⚙️ SettingsSeverity Threshold: 📖 User Guide
|
![beetle-ai[bot]](https://avatars.githubusercontent.com/in/1636950?s=60&v=4){kind=small}
| } catch (err: any) { | ||
| if (err.status !== 422 && err.status !== 404) throw err; |
There was a problem hiding this comment.
The error handler silently swallows errors without logging, making it impossible to debug branch deletion failures. Additionally, using err: any bypasses TypeScript type safety. If the error object doesn't have a status property, the code could fail unexpectedly.
Confidence: 4/5
Suggested Fix
| } catch (err: any) { | |
| if (err.status !== 422 && err.status !== 404) throw err; | |
| } catch (err: any) { | |
| if (err.status === 422 || err.status === 404) { | |
| // Branch already deleted or doesn't exist - this is acceptable | |
| return; | |
| } | |
| console.error(`Failed to delete branch ${branchName}:`, err); | |
| throw err; | |
| } | |
Add logging to capture unexpected errors and make the status code check more explicit. This helps with debugging while still allowing expected errors (422, 404) to be handled gracefully.
Prompt for AI
Copy this prompt to your AI IDE to fix this issue locally:
In src/events/cleanup/branchCleanup.ts around line 19-20, the error handler silently swallows errors without logging, making debugging difficult. Additionally, the `err: any` type bypasses TypeScript safety. Add logging for unexpected errors and improve the error status check to be more explicit. Make sure to log the branch name and error details when deletion fails unexpectedly.
📍 This suggestion applies to lines 19-20
| const lintWorkflowContent = readFileSync( | ||
| join(__dirname, "../../templates/linter.yml"), | ||
| "utf-8", | ||
| ); |
There was a problem hiding this comment.
The readFileSync call is not wrapped in error handling. If the template file doesn't exist or is inaccessible, the entire handler will crash without notifying the user. This is a critical failure point that should be handled gracefully.
Confidence: 5/5
Suggested Fix
| const lintWorkflowContent = readFileSync( | |
| join(__dirname, "../../templates/linter.yml"), | |
| "utf-8", | |
| ); | |
| let lintWorkflowContent: string; | |
| try { | |
| lintWorkflowContent = readFileSync( | |
| join(__dirname, "../../templates/linter.yml"), | |
| "utf-8", | |
| ); | |
| } catch (err) { | |
| await context.octokit.rest.issues.createComment( | |
| context.issue({ | |
| body: "❌ Failed to read lint workflow template. Please contact the repository administrator.", | |
| }), | |
| ); | |
| throw err; | |
| } | |
Wrap the file read in a try-catch block to handle missing or inaccessible template files. Notify the user of the failure before throwing the error.
Prompt for AI
Copy this prompt to your AI IDE to fix this issue locally:
In src/events/commands/addLinter.ts around line 76-79, the readFileSync call is not wrapped in error handling. If the template file doesn't exist, the handler will crash silently. Add a try-catch block around the file read operation, and notify the user with a comment if the template file cannot be read.
📍 This suggestion applies to lines 76-79
| const { data: mainRef } = await context.octokit.rest.git.getRef({ | ||
| owner, | ||
| repo, | ||
| ref: "heads/main", | ||
| }); | ||
| const mainSha = mainRef.object.sha; | ||
|
|
||
| await context.octokit.rest.git.createRef({ | ||
| owner, | ||
| repo, | ||
| ref: `refs/heads/${branchName}`, | ||
| sha: mainSha, | ||
| }); |
There was a problem hiding this comment.
The getRef and createRef API calls are not wrapped in error handling. If the main branch doesn't exist or the branch creation fails (e.g., branch already exists), the handler will crash without user notification. Additionally, if createRef fails, the file creation and PR creation will still attempt to execute against a non-existent branch.
Confidence: 5/5
Suggested Fix
| const { data: mainRef } = await context.octokit.rest.git.getRef({ | |
| owner, | |
| repo, | |
| ref: "heads/main", | |
| }); | |
| const mainSha = mainRef.object.sha; | |
| await context.octokit.rest.git.createRef({ | |
| owner, | |
| repo, | |
| ref: `refs/heads/${branchName}`, | |
| sha: mainSha, | |
| }); | |
| let mainSha: string; | |
| try { | |
| const { data: mainRef } = await context.octokit.rest.git.getRef({ | |
| owner, | |
| repo, | |
| ref: "heads/main", | |
| }); | |
| mainSha = mainRef.object.sha; | |
| } catch (err) { | |
| await context.octokit.rest.issues.createComment( | |
| context.issue({ | |
| body: "❌ Failed to find the main branch. Does it exist?", | |
| }), | |
| ); | |
| throw err; | |
| } | |
| try { | |
| await context.octokit.rest.git.createRef({ | |
| owner, | |
| repo, | |
| ref: `refs/heads/${branchName}`, | |
| sha: mainSha, | |
| }); | |
| } catch (err) { | |
| await context.octokit.rest.issues.createComment( | |
| context.issue({ | |
| body: "❌ Failed to create feature branch. Please try again.", | |
| }), | |
| ); | |
| throw err; | |
| } | |
Wrap both API calls in try-catch blocks to handle failures gracefully and notify the user of what went wrong.
Prompt for AI
Copy this prompt to your AI IDE to fix this issue locally:
In src/events/commands/addLinter.ts around lines 83-95, the getRef and createRef API calls lack error handling. If the main branch doesn't exist or branch creation fails, the handler crashes without user feedback. Add separate try-catch blocks for both operations and notify the user of failures before throwing errors.
📍 This suggestion applies to lines 83-95
| await context.octokit.rest.repos.createOrUpdateFileContents({ | ||
| owner, | ||
| repo, | ||
| path: workflowPath, | ||
| message: "ci: add lint workflow", | ||
| content: encodedContent, | ||
| branch: branchName, | ||
| }); |
There was a problem hiding this comment.
The createOrUpdateFileContents API call is not wrapped in error handling. If this operation fails (e.g., due to permission issues or API rate limiting), the branch will be left orphaned in the repository with no user notification. This creates resource leaks and confusion.
Confidence: 5/5
Suggested Fix
| await context.octokit.rest.repos.createOrUpdateFileContents({ | |
| owner, | |
| repo, | |
| path: workflowPath, | |
| message: "ci: add lint workflow", | |
| content: encodedContent, | |
| branch: branchName, | |
| }); | |
| try { | |
| await context.octokit.rest.repos.createOrUpdateFileContents({ | |
| owner, | |
| repo, | |
| path: workflowPath, | |
| message: "ci: add lint workflow", | |
| content: encodedContent, | |
| branch: branchName, | |
| }); | |
| } catch (err) { | |
| // Clean up the orphaned branch | |
| try { | |
| await context.octokit.rest.git.deleteRef({ | |
| owner, | |
| repo, | |
| ref: `heads/${branchName}`, | |
| }); | |
| } catch (cleanupErr) { | |
| // Log cleanup failure but continue | |
| console.error(`Failed to clean up branch ${branchName}:`, cleanupErr); | |
| } | |
| await context.octokit.rest.issues.createComment( | |
| context.issue({ | |
| body: "❌ Failed to create workflow file. Please try again.", | |
| }), | |
| ); | |
| throw err; | |
| } | |
Wrap the file creation in a try-catch block. If it fails, attempt to clean up the orphaned branch before notifying the user.
Prompt for AI
Copy this prompt to your AI IDE to fix this issue locally:
In src/events/commands/addLinter.ts around lines 97-104, the createOrUpdateFileContents call lacks error handling. If this fails, the branch is left orphaned. Add a try-catch block that attempts to clean up the branch if file creation fails, and notify the user of the failure.
📍 This suggestion applies to lines 97-104
| const { data: pr } = await context.octokit.rest.pulls.create({ | ||
| owner, | ||
| repo, | ||
| title: "ci: add lint workflow", | ||
| head: branchName, | ||
| base: "main", | ||
| body: [ | ||
| "This PR adds a GitHub Actions lint workflow to the repository.", | ||
| "", | ||
| "It was automatically generated by **devhub-bot** in response to the `!addlinter` command.", | ||
| "", | ||
| ].join("\n"), | ||
| }); |
There was a problem hiding this comment.
The pulls.create API call is not wrapped in error handling. If PR creation fails, the user won't be notified, and they'll be left wondering if the operation succeeded. The branch and file will exist but no PR will be created.
Confidence: 5/5
Suggested Fix
| const { data: pr } = await context.octokit.rest.pulls.create({ | |
| owner, | |
| repo, | |
| title: "ci: add lint workflow", | |
| head: branchName, | |
| base: "main", | |
| body: [ | |
| "This PR adds a GitHub Actions lint workflow to the repository.", | |
| "", | |
| "It was automatically generated by **devhub-bot** in response to the `!addlinter` command.", | |
| "", | |
| ].join("\n"), | |
| }); | |
| let pr; | |
| try { | |
| const response = await context.octokit.rest.pulls.create({ | |
| owner, | |
| repo, | |
| title: "ci: add lint workflow", | |
| head: branchName, | |
| base: "main", | |
| body: [ | |
| "This PR adds a GitHub Actions lint workflow to the repository.", | |
| "", | |
| "It was automatically generated by **devhub-bot** in response to the `!addlinter` command.", | |
| "", | |
| ].join("\n"), | |
| }); | |
| pr = response.data; | |
| } catch (err) { | |
| await context.octokit.rest.issues.createComment( | |
| context.issue({ | |
| body: "❌ Failed to create pull request. The branch and workflow file were created, but the PR could not be opened. Please create the PR manually.", | |
| }), | |
| ); | |
| throw err; | |
| } | |
Wrap the PR creation in a try-catch block to handle failures and notify the user appropriately.
Prompt for AI
Copy this prompt to your AI IDE to fix this issue locally:
In src/events/commands/addLinter.ts around lines 106-118, the pulls.create call lacks error handling. If PR creation fails, the user receives no feedback. Add a try-catch block to handle failures and notify the user that the branch and file were created but the PR could not be opened.
📍 This suggestion applies to lines 106-118
| await context.octokit.rest.issues.createComment({ | ||
| owner, | ||
| repo, | ||
| issue_number: context.payload.issue.number, | ||
| body: reply, | ||
| }); |
There was a problem hiding this comment.
The createComment API call is not wrapped in error handling. If this operation fails (due to permission issues, rate limiting, or network errors), the handler will crash without notifying the user or logging the error. This leaves the user without feedback about whether their command succeeded.
Confidence: 5/5
Suggested Fix
| await context.octokit.rest.issues.createComment({ | |
| owner, | |
| repo, | |
| issue_number: context.payload.issue.number, | |
| body: reply, | |
| }); | |
| try { | |
| await context.octokit.rest.issues.createComment({ | |
| owner, | |
| repo, | |
| issue_number: context.payload.issue.number, | |
| body: reply, | |
| }); | |
| } catch (err) { | |
| console.error(`Failed to post ping response: ${err}`); | |
| throw err; | |
| } | |
Wrap the API call in a try-catch block to handle failures gracefully and log the error for debugging purposes.
Prompt for AI
Copy this prompt to your AI IDE to fix this issue locally:
In src/events/commands/ping.ts around lines 38-43, the createComment API call lacks error handling. If the comment posting fails, the user receives no feedback. Add a try-catch block to handle failures and log the error for debugging.
📍 This suggestion applies to lines 38-43
|
Note Linting checks passed successfully 🎉 All formatting and code quality checks are clean. You're good to merge 🚀 |
| await context.octokit.rest.issues.addLabels( | ||
| context.issue({ labels: labelsToApply.map((l) => l.name) }), | ||
| ); |
There was a problem hiding this comment.
The addLabels API call is not wrapped in error handling. If this operation fails (due to permission issues, rate limiting, or network errors), the handler will crash without notifying the user or logging the error.
Confidence: 5/5
Suggested Fix
| await context.octokit.rest.issues.addLabels( | |
| context.issue({ labels: labelsToApply.map((l) => l.name) }), | |
| ); | |
| try { | |
| await context.octokit.rest.issues.addLabels( | |
| context.issue({ labels: labelsToApply.map((l) => l.name) }), | |
| ); | |
| } catch (err) { | |
| console.error(`Failed to add labels to issue: ${err}`); | |
| throw err; | |
| } | |
Wrap the API call in a try-catch block to handle failures gracefully and log the error for debugging purposes.
Prompt for AI
Copy this prompt to your AI IDE to fix this issue locally:
In src/events/labeling/label-issue.ts around lines 21-23, the addLabels API call lacks error handling. If the operation fails, the handler crashes without logging. Add a try-catch block to handle failures and log the error for debugging.
📍 This suggestion applies to lines 21-23
| for (const label of labelsToApply) { | ||
| await ensureLabelExists(context, label); | ||
| } |
There was a problem hiding this comment.
The ensureLabelExists function is called in a loop without error handling. If label creation fails for any reason, the handler will crash and subsequent labels won't be processed. This could leave the issue in an inconsistent state with only some labels applied.
Confidence: 4/5
Suggested Fix
| for (const label of labelsToApply) { | |
| await ensureLabelExists(context, label); | |
| } | |
| for (const label of labelsToApply) { | |
| try { | |
| await ensureLabelExists(context, label); | |
| } catch (err) { | |
| console.error(`Failed to ensure label exists: ${label}`, err); | |
| throw err; | |
| } | |
Wrap the ensureLabelExists call in a try-catch block to handle failures gracefully and log which label failed to be created.
Prompt for AI
Copy this prompt to your AI IDE to fix this issue locally:
In src/events/labeling/label-issue.ts around lines 17-19, the ensureLabelExists call in the loop lacks error handling. If label creation fails, the handler crashes and subsequent labels won't be processed. Add a try-catch block to handle failures and log which label failed.
📍 This suggestion applies to lines 17-19
| const commits = await context.octokit.paginate( | ||
| context.octokit.rest.pulls.listCommits, | ||
| { owner, repo, pull_number, per_page: 100 }, | ||
| ); |
There was a problem hiding this comment.
The paginate API call is not wrapped in error handling. If this operation fails (due to permission issues, rate limiting, or network errors), the handler will crash without notifying the user or logging the error.
Confidence: 5/5
Suggested Fix
| const commits = await context.octokit.paginate( | |
| context.octokit.rest.pulls.listCommits, | |
| { owner, repo, pull_number, per_page: 100 }, | |
| ); | |
| let commits; | |
| try { | |
| commits = await context.octokit.paginate( | |
| context.octokit.rest.pulls.listCommits, | |
| { owner, repo, pull_number, per_page: 100 }, | |
| ); | |
| } catch (err) { | |
| console.error(`Failed to fetch commits for PR #${pull_number}: ${err}`); | |
| throw err; | |
| } | |
Wrap the API call in a try-catch block to handle failures gracefully and log the error for debugging purposes.
Prompt for AI
Copy this prompt to your AI IDE to fix this issue locally:
In src/events/labeling/label-pr.ts around lines 12-15, the paginate API call lacks error handling. If fetching commits fails, the handler crashes without logging. Add a try-catch block to handle failures and log the error for debugging.
📍 This suggestion applies to lines 12-15
| for (const label of labelsToApply) { | ||
| await ensureLabelExists(context, label); | ||
| } |
There was a problem hiding this comment.
The ensureLabelExists function is called in a loop without error handling. If label creation fails for any reason, the handler will crash and subsequent labels won't be processed. This could leave the PR in an inconsistent state with only some labels applied.
Confidence: 5/5
Suggested Fix
| for (const label of labelsToApply) { | |
| await ensureLabelExists(context, label); | |
| } | |
| for (const label of labelsToApply) { | |
| try { | |
| await ensureLabelExists(context, label); | |
| } catch (err) { | |
| console.error(`Failed to ensure label exists: ${label.name}`, err); | |
| throw err; | |
| } | |
Wrap the ensureLabelExists call in a try-catch block to handle failures gracefully and log which label failed to be created.
Prompt for AI
Copy this prompt to your AI IDE to fix this issue locally:
In src/events/labeling/label-pr.ts around lines 44-46, the ensureLabelExists call in the loop lacks error handling. If label creation fails, the handler crashes and subsequent labels won't be processed. Add a try-catch block to handle failures and log which label failed.
📍 This suggestion applies to lines 44-46
| await context.octokit.rest.issues.addLabels( | ||
| context.issue({ labels: labelsToApply.map((l) => l.name) }), | ||
| ); |
There was a problem hiding this comment.
The addLabels API call is not wrapped in error handling. If this operation fails (due to permission issues, rate limiting, or network errors), the handler will crash without notifying the user or logging the error.
Confidence: 5/5
Suggested Fix
| await context.octokit.rest.issues.addLabels( | |
| context.issue({ labels: labelsToApply.map((l) => l.name) }), | |
| ); | |
| try { | |
| await context.octokit.rest.issues.addLabels( | |
| context.issue({ labels: labelsToApply.map((l) => l.name) }), | |
| ); | |
| } catch (err) { | |
| console.error(`Failed to add labels to PR #${pull_number}: ${err}`); | |
| throw err; | |
| } | |
Wrap the API call in a try-catch block to handle failures gracefully and log the error for debugging purposes.
Prompt for AI
Copy this prompt to your AI IDE to fix this issue locally:
In src/events/labeling/label-pr.ts around lines 48-50, the addLabels API call lacks error handling. If the operation fails, the handler crashes without logging. Add a try-catch block to handle failures and log the error for debugging.
📍 This suggestion applies to lines 48-50
| await context.octokit.rest.issues.createComment({ | ||
| owner, | ||
| repo, | ||
| issue_number: pullNumber, | ||
| body, | ||
| }); |
There was a problem hiding this comment.
The createComment API call is not wrapped in error handling. If this operation fails (due to permission issues, rate limiting, or network errors), the handler will crash without logging the error or notifying the user. This leaves the PR without feedback about the lint workflow result.
Confidence: 5/5
Suggested Fix
| await context.octokit.rest.issues.createComment({ | |
| owner, | |
| repo, | |
| issue_number: pullNumber, | |
| body, | |
| }); | |
| try { | |
| await context.octokit.rest.issues.createComment({ | |
| owner, | |
| repo, | |
| issue_number: pullNumber, | |
| body, | |
| }); | |
| } catch (err) { | |
| context.log.error(`[LINTER] Failed to post lint workflow comment: ${String(err)}`); | |
| throw err; | |
| } | |
Wrap the API call in a try-catch block to handle failures gracefully and log the error for debugging purposes.
Prompt for AI
Copy this prompt to your AI IDE to fix this issue locally:
In src/events/linting/checklint.ts around lines 46-51, the createComment API call lacks error handling. If the operation fails, the handler crashes without logging. Add a try-catch block to handle failures and log the error for debugging.
📍 This suggestion applies to lines 46-51
| const commits = await context.octokit.paginate( | ||
| context.octokit.rest.pulls.listCommits, | ||
| { owner, repo, pull_number, per_page: 100 }, | ||
| ); |
There was a problem hiding this comment.
The paginate API call is not wrapped in error handling. If this operation fails (due to permission issues, rate limiting, or network errors), the handler will crash without logging the error or notifying the user.
Confidence: 5/5
Suggested Fix
| const commits = await context.octokit.paginate( | |
| context.octokit.rest.pulls.listCommits, | |
| { owner, repo, pull_number, per_page: 100 }, | |
| ); | |
| let commits; | |
| try { | |
| commits = await context.octokit.paginate( | |
| context.octokit.rest.pulls.listCommits, | |
| { owner, repo, pull_number, per_page: 100 }, | |
| ); | |
| } catch (err) { | |
| console.error(`Failed to fetch commits for PR #${pull_number}: ${err}`); | |
| throw err; | |
| } | |
Wrap the API call in a try-catch block to handle failures gracefully and log the error for debugging purposes.
Prompt for AI
Copy this prompt to your AI IDE to fix this issue locally:
In src/events/standards/conventional-commits.ts around lines 19-22, the paginate API call lacks error handling. If fetching commits fails, the handler crashes without logging. Add a try-catch block to handle failures and log the error for debugging.
📍 This suggestion applies to lines 19-22
| "", | ||
| ].join("\n"); | ||
|
|
||
| await context.octokit.rest.issues.createComment(context.issue({ body })); |
There was a problem hiding this comment.
The createComment API call is not wrapped in error handling. If this operation fails (due to permission issues, rate limiting, or network errors), the handler will crash without logging the error or notifying the user of the validation results.
Confidence: 5/5
Suggested Fix
| await context.octokit.rest.issues.createComment(context.issue({ body })); | |
| try { | |
| await context.octokit.rest.issues.createComment(context.issue({ body })); | |
| } catch (err) { | |
| console.error(`Failed to post conventional commits validation comment: ${err}`); | |
| throw err; | |
| } | |
Wrap the API call in a try-catch block to handle failures gracefully and log the error for debugging purposes.
Prompt for AI
Copy this prompt to your AI IDE to fix this issue locally:
In src/events/standards/conventional-commits.ts around line 57, the createComment API call lacks error handling. If the operation fails, the handler crashes without logging. Add a try-catch block to handle failures and log the error for debugging.
Summary by BeetleThis PR refactors the event handler naming convention across the codebase by renaming the 📁 File Changes Summary (Consolidated across all commits):
Total Changes: 8 files changed, +15 additions, -15 deletions 🎯 Key Changes:
📊 Impact Assessment:
⚙️ SettingsSeverity Threshold: 📖 User Guide
|
|
✅ You're good to merge this PR! No issues found. Great job! Settings⚙️ SettingsSeverity Threshold: 📖 User Guide
|
|
Note Linting checks passed successfully 🎉 All formatting and code quality checks are clean. You're good to merge 🚀 |
1 participant
No description provided.