Skip to content

[JENKINS-75704] Fix Copilot user null name/email handling #864

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.

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

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

IvanGasant
Copy link

@IvanGasant IvanGasant commented Jun 10, 2025

Description

This PR improves the handling of pull requests and commits authored by GitHub IA, Copilot, who may not have a real GitHub account or may lack a defined name or email.
It prevents exceptions and ensures that these contributions are not discarded during indexing or processing.


Changes

  • Special handling for the "copilot" user:

    • If a pull request is authored by "copilot", a synthetic ContributorMetadataAction is created with:
    • This ensures Copilot PRs are processed and visible, even though "copilot" is not a real GitHub user.
  • Fallback for missing user name or email:

    • For all other users, if user.getName() or user.getEmail() are null or throw exceptions, the code now defaults to:
      • Name: user.getLogin()
      • Email: user.getLogin() + "@unknown.user"
    • This prevents exceptions when processing PRs or commits from bot users or users with incomplete profiles.
  • Preserves previous logic for truly missing users:

    • If the user does not exist (e.g., API returns 404), the PR is still ignored as before.

Motivation

GitHub Copilot and other bots can appear as authors of commits and pull requests, but may not exist as real users in GitHub.
Without this special handling, such PRs would be discarded or cause exceptions, impacting the visibility and traceability of AI-generated or bot-generated contributions.

See JENKINS-75704 for further information.

Submitter checklist

  • Link to JIRA ticket in description, if appropriate.
  • Change is code complete and matches issue description
  • Automated tests have been added to exercise the changes

No automated tests were added because this is a trivial change: it only handles null values or exceptions when fetching the GitHub user's name/email for logging purposes, defaulting to safe values if missing. It does not affect business logic or the core functionality of the plugin.

  • Reviewer's manual test instructions provided in PR description. See Reviewer's first task below.

Reviewer checklist

  • Run the changes and verify that the change matches the issue description
  • Reviewed the code
  • Verified that the appropriate tests have been written or valid explanation given

Documentation changes

  • Link to jenkins.io PR, or an explanation for why no doc changes are needed

Users/aliases to notify

@IvanGasant IvanGasant requested a review from a team as a code owner June 10, 2025 08:50
@IvanGasant IvanGasant marked this pull request as draft June 10, 2025 09:51
…pdate tests

- Updated logic so that only the copilot user is allowed to bypass missing name and email checks in pull request processing.
- All other users without name/email are marked as unknown and a log message is generated.
- Added and updated tests to verify that copilot is accepted and handled as expected.
@IvanGasant IvanGasant marked this pull request as ready for review June 11, 2025 18:05
@IvanGasant IvanGasant changed the title Fix Copilot user null name/email handling [JENKINS-75704] Fix Copilot user null name/email handling Jun 12, 2025
@jglick jglick added the bug label Jun 20, 2025
@jglick
Copy link
Member

jglick commented Jun 20, 2025

Fallback for missing user name or email:

* For all other users, if `user.getName()` or `user.getEmail()` are `null` or throw exceptions, the code now defaults to:
  
  * Name: `user.getLogin()`
  * Email: `user.getLogin() + "@unknown.user"`

* This prevents exceptions when processing PRs or commits from bot users or users with incomplete profiles.

I see no evidence of such a change in the current patch; did you revert it? If so, please update the PR description accordingly.

@@ -2572,17 +2572,22 @@ public void observe(GHPullRequest pr) {
GHUser user = null;
try {
user = pr.getUser();
if (users.containsKey(user.getLogin())) {
// looked up this user already
Copy link
Member

Choose a reason for hiding this comment

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

may as well retain this comment in the else clause

ContributorMetadataAction contributor =
new ContributorMetadataAction(user.getLogin(), user.getName(), user.getEmail());
pullRequestContributorCache.put(number, contributor);
// store the populated user record now that we have it
Copy link
Member

Choose a reason for hiding this comment

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

ditto

} catch (FileNotFoundException e) {
// If file not found for user, warn but keep going
Copy link
Member

Choose a reason for hiding this comment

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

IIUC this was just false.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants