fix: add sender.id to GitHub webhook payload types#533
Conversation
GitHub webhook payloads include sender.id (numeric user ID) alongside sender.login. The type definitions only modeled login. Adding id to all four payload types so the main migration can forward it as the actorProviderUserId for consistent identity linking with the web OAuth path.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughUpdated the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Terraform Validation Results
Pushed by: @ColeMurray, Action: |
![open-inspect[bot]](https://avatars.githubusercontent.com/u/2492022?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary
This PR #533 (fix: add sender.id to GitHub webhook payload types) by @ColeMurray updates the four narrowed GitHub webhook payload interfaces in packages/github-bot/src/types.ts to include sender.id, matching the webhook schema without changing runtime behavior. I reviewed the single changed file (1 file changed, +4/-4) and did not find correctness, security, performance, or maintainability issues in the diff.
Critical Issues
None.
Suggestions
None.
Nitpicks
None.
Positive Feedback
- The change is tightly scoped to the type surface and stays aligned with the stated migration step.
- Adding the numeric GitHub user ID now reduces the chance of propagating incomplete webhook typings into later identity-linking work.
- Keeping this as a type-only update avoids unnecessary runtime churn.
Questions
None.
Verdict
Approve: Ready to merge, no blocking issues.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/github-bot/src/types.ts (1)
44-108: Optional: consider extracting a sharedSender(andRepository) type.The
sender: { login: string; id: number }shape is now duplicated across all four payload interfaces, as isrepository: { owner: { login: string }; name: string; private: boolean }. Extracting a shared alias would make future additions (e.g., forwarding moresender.*fields in the upcoming unified user model migration referenced in the PR) a single-line change rather than four.♻️ Example extraction
+interface WebhookSender { + login: string; + id: number; +} + +interface WebhookRepository { + owner: { login: string }; + name: string; + private: boolean; +} + export interface PullRequestOpenedPayload { action: "opened"; pull_request: { /* ... */ }; - repository: { owner: { login: string }; name: string; private: boolean }; - sender: { login: string; id: number }; + repository: WebhookRepository; + sender: WebhookSender; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/github-bot/src/types.ts` around lines 44 - 108, Extract a shared Sender and Repository type and use them in the four payload interfaces to eliminate duplication: define e.g. type Sender = { login: string; id: number } and type Repository = { owner: { login: string }; name: string; private: boolean } (or export interfaces) and replace the inline shapes in PullRequestOpenedPayload, ReviewRequestedPayload, IssueCommentPayload, and ReviewCommentPayload with the new Sender and Repository types so future additions require changing only those two definitions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/github-bot/src/types.ts`:
- Line 56: The test fixtures fail typecheck because the sender shape in types.ts
now requires sender: { login: string; id: number }; update all sender objects in
packages/github-bot/test/handlers.test.ts to include an id numeric field
alongside login (e.g., add id: 1 or another unique number per fixture) so they
conform to the Sender type; search for each occurrence of the sender literal in
that test file and add the id property to the object literals to resolve the
type error.
---
Nitpick comments:
In `@packages/github-bot/src/types.ts`:
- Around line 44-108: Extract a shared Sender and Repository type and use them
in the four payload interfaces to eliminate duplication: define e.g. type Sender
= { login: string; id: number } and type Repository = { owner: { login: string
}; name: string; private: boolean } (or export interfaces) and replace the
inline shapes in PullRequestOpenedPayload, ReviewRequestedPayload,
IssueCommentPayload, and ReviewCommentPayload with the new Sender and Repository
types so future additions require changing only those two definitions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 52a52421-8e58-49d1-a25b-85f5d5cadc13
📒 Files selected for processing (1)
packages/github-bot/src/types.ts
Update all six sender objects in handlers.test.ts to include the id field, matching the updated payload types.
Terraform Validation Results
Pushed by: @ColeMurray, Action: |
|
Re: the nitpick on extracting shared |
1 participant
Summary
id: numbertosenderon all 4 webhook payload types:PullRequestOpenedPayload,ReviewRequestedPayload,IssueCommentPayload,ReviewCommentPayloadsender.id(numeric user ID) alongsidesender.login— the type definitions were missing itPre-step 3 of the unified user model migration (pre-steps doc). The main migration will forward
sender.idasactorProviderUserIdfor consistent identity linking with the web OAuth path (which already uses the numeric GitHub user ID).Test plan
npm run typecheck -w @open-inspect/github-botpassesnpm test -w @open-inspect/github-bot— all 100 tests passSummary by CodeRabbit