fix: pass sender login as scmLogin in github-bot session creation#498
fix: pass sender login as scmLogin in github-bot session creation#498ColeMurray merged 1 commit intomainfrom
Conversation
GitHub bot sessions were created without scmLogin, causing them to appear as "unknown" in the analytics dashboard. Pass sender.login from the webhook payload to the control plane on all four handlers.
|
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 (2)
📝 WalkthroughWalkthroughThe Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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: |
There was a problem hiding this comment.
Summary
This PR updates all four github-bot webhook handlers to forward sender.login as scmLogin when creating sessions, and adds focused assertions to verify that payload on each happy path. I reviewed the changed handlers and tests, verified the control-plane session creation path already accepts scmLogin as an optional field, and did not find any correctness, security, performance, or maintainability issues.
- PR Title and number:
fix: pass sender login as scmLogin in github-bot session creation(#498) - Author:
@ColeMurray - Files changed count, additions/deletions:
2 files,+15/-0
Critical Issues
- None.
Suggestions
- None.
Nitpicks
- None.
Positive Feedback
- This change is minimal and threads the new field through every github-bot session creation path consistently.
- The tests assert the new request payload directly, which keeps the regression coverage targeted and easy to understand.
- The implementation matches the existing control-plane contract, so the fix stays scoped to the caller that was omitting the field.
Questions
- None.
Verdict
Approve: Ready to merge, no blocking issues
Summary
scmLogin, causing them to appear as "unknown" in the analytics dashboardhandleReviewRequested,handlePullRequestOpened,handleIssueComment,handleReviewComment) now passsender.loginfrom the webhook payload asscmLoginwhen creating sessions via the control planescmLoginas an optional field onPOST /sessions— no backend changes neededTest plan
scmLoginassertions on all four handler happy pathsSummary by CodeRabbit
Chores
Tests