Skip to content
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

[IGNORE] #1453

Closed
wants to merge 2 commits into from
Closed

[IGNORE] #1453

wants to merge 2 commits into from

Conversation

hussam789
Copy link
Collaborator

@hussam789 hussam789 commented Jan 15, 2025

PR Type

Enhancement, Documentation


Description

  • Added bot detection logic for usernames and PR descriptions.

  • Enhanced bot user identification in GitHub, GitLab, and Bitbucket.

  • Updated Jira Data Center/Server documentation with new authentication methods.

  • Improved modularity by introducing utility functions for bot detection.


Changes walkthrough 📝

Relevant files
Enhancement
utils.py
Introduced utility functions for bot detection                     

pr_agent/git_providers/utils.py

  • Added is_user_name_a_bot function to detect bot-like usernames.
  • Added is_pr_description_indicating_bot function to detect bot-like PR
    descriptions.
  • +19/-0   
    bitbucket_app.py
    Improved bot user detection in Bitbucket                                 

    pr_agent/servers/bitbucket_app.py

  • Integrated bot detection for usernames and PR descriptions.
  • Enhanced is_bot_user logic to utilize new utility functions.
  • +12/-1   
    github_app.py
    Enhanced bot user detection in GitHub                                       

    pr_agent/servers/github_app.py

  • Enhanced is_bot_user logic with PR description checks.
  • Integrated new utility functions for bot detection.
  • +16/-6   
    gitlab_webhook.py
    Enhanced bot user detection in GitLab                                       

    pr_agent/servers/gitlab_webhook.py

  • Improved bot detection logic using utility functions.
  • Added PR description checks for bot identification.
  • +7/-3     
    Documentation
    fetching_ticket_context.md
    Updated Jira Data Center/Server authentication documentation

    docs/docs/core-abilities/fetching_ticket_context.md

  • Updated Jira Data Center/Server section with new authentication
    methods.
  • Added detailed steps for Local App Authentication.
  • Retained support for Personal Access Token (PAT) Authentication.
  • +37/-2   

    💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    Copy link
    Contributor

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
    🏅 Score: 85
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Bot Detection Coverage

    The bot detection patterns might need expansion to cover more cases. Consider adding patterns for other common bot naming conventions and automated PR descriptions.

    bot_indicators = ['codium', 'bot_', 'bot-', '_bot', '-bot', 'qodo', "service", "github", "jenkins", "auto",
                      "cicd", "validator", "ci-", "assistant", "srv-"]
    return any(indicator in name.lower() for indicator in bot_indicators)
    Error Handling

    The bot detection logic could benefit from more specific error handling and logging to help diagnose issues in production.

    def is_bot_user(sender, sender_type, user_description):
        try:
            # logic to ignore PRs opened by bot
            if get_settings().get("GITHUB_APP.IGNORE_BOT_PR", False):
                if sender_type.lower() == "bot":
                    if 'pr-agent' not in sender:
                        get_logger().info(f"Ignoring PR from '{sender=}' because it is a bot")
                    return True
                if is_user_name_a_bot(sender):
                    get_logger().info(f"Ignoring PR from '{sender=}' because it is a bot")
                    return True
                # Ignore PRs opened by bot users based on their description
                if isinstance(user_description, str) and is_pr_description_indicating_bot(user_description):
                    get_logger().info(f"Description indicates a bot user: {sender}",
                                      artifact={"description": user_description})
                    return True
        except Exception as e:
            get_logger().error(f"Failed 'is_bot_user' logic: {e}")
        return False

    Copy link
    Contributor

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Possible issue
    Improve bot detection accuracy by preventing false positives from short, common substrings

    Add a length check for the bot indicators to avoid false positives with short words
    like 'auto' that might appear in legitimate usernames. Consider requiring a minimum
    length or using word boundaries.

    pr_agent/git_providers/utils.py [109-111]

    -bot_indicators = ['codium', 'bot_', 'bot-', '_bot', '-bot', 'qodo', "service", "github", "jenkins", "auto",
    +bot_indicators = ['codium', 'bot_', 'bot-', '_bot', '-bot', 'qodo', "service", "github", "jenkins",
                       "cicd", "validator", "ci-", "assistant", "srv-"]
    -return any(indicator in name.lower() for indicator in bot_indicators)
    +name_lower = name.lower()
    +return any(indicator in name_lower and (len(indicator) > 4 or indicator.startswith(('bot', 'ci-', 'srv-')))
    +           for indicator in bot_indicators)
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: The suggestion addresses a significant issue where short common words like 'auto' could lead to false positives in bot detection, potentially blocking legitimate users. The improved implementation adds length checks and word boundary considerations.

    8
    Add null check for sender type to prevent potential runtime errors

    The case-sensitive comparison of sender_type with "Bot" could miss bot detection -
    use case-insensitive comparison as done in the rest of the codebase.

    pr_agent/servers/github_app.py [245]

    -if sender_type.lower() == "bot":
    +if sender_type and sender_type.lower() == "bot":
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: Adding a null check for sender_type is important to prevent potential runtime errors, as the sender_type parameter could be None in some cases. This improves the robustness of the bot detection logic.

    7
    Add type validation to prevent runtime errors from invalid input types

    Add input validation for the description parameter to handle non-string inputs that
    might be passed from the various server implementations.

    pr_agent/git_providers/utils.py [114-116]

     def is_pr_description_indicating_bot(description: str) -> bool:
    -    if not description:
    +    if not description or not isinstance(description, str):
             return False
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: The suggestion adds important type validation to prevent runtime errors when non-string inputs are passed to the function, making the code more robust and defensive against invalid inputs.

    7
    • Author self-review: I have reviewed the PR code suggestions, and addressed the relevant ones.

    @hussam789 hussam789 closed this Jan 15, 2025
    @hussam789 hussam789 changed the title Hl/jira server docs [IGNORE] Jan 15, 2025
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    1 participant