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

Add command execution functionality to Bitbucket app and update configuration settings #745

Merged
merged 1 commit into from
Mar 6, 2024

Conversation

mrT23
Copy link
Collaborator

@mrT23 mrT23 commented Mar 6, 2024

Type

enhancement


Description

  • Introduced a new functionality to execute configured commands for Bitbucket pull requests, enhancing automation capabilities.
  • Updated the webhook handler in bitbucket_app.py to support new pr_commands configuration, allowing for more flexible and powerful automation options.
  • Documentation in Usage.md has been updated to guide users on how to configure and use the new pr_commands feature.
  • Configuration file configuration.toml now supports pr_commands for Bitbucket, replacing the old auto tool settings, providing an example configuration.

Changes walkthrough

Relevant files
Enhancement
bitbucket_app.py
Introduce Command Execution for Bitbucket PRs                       

pr_agent/servers/bitbucket_app.py

  • Added a new async function _perform_commands_bitbucket to execute
    configured commands for Bitbucket PRs.
  • Updated the webhook handler to support executing commands based on the
    pr_commands configuration.
  • Enhanced logging and error handling for command execution.
  • +32/-12 
    Documentation
    Usage.md
    Update Documentation for Bitbucket Command Execution         

    Usage.md

  • Updated documentation to reflect the new pr_commands configuration for
    Bitbucket.
  • Removed old configuration instructions related to auto_review,
    auto_describe, and auto_improve.
  • Added examples of how to configure pr_commands for automatic
    execution.
  • +7/-12   
    Configuration changes
    configuration.toml
    Update Configuration to Support Command Execution               

    pr_agent/settings/configuration.toml

  • Replaced old auto tool settings with pr_commands configuration for
    Bitbucket.
  • Provided an example of how to configure pr_commands in the
    configuration.toml file.
  • +4/-3     

    PR-Agent usage:
    Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

    Copy link
    Contributor

    PR Review

    ⏱️ Estimated effort to review [1-5]

    3, because the PR introduces a significant feature with changes across multiple files, including the core functionality, documentation, and configuration. The logic seems straightforward, but understanding the context and implications of the command execution feature requires a good grasp of the existing codebase and its architecture.

    🏅 Score

    85

    🧪 Relevant tests

    No

    🔍 Possible issues

    Error Handling: The error handling in _perform_commands_bitbucket logs the error but does not stop or alter the flow. This might lead to silent failures where commands do not execute as expected, but the process continues.

    Command Injection: The method of constructing commands from strings (' '.join([command] + other_args)) could be susceptible to command injection if user input is not properly sanitized, especially since args come from splitting a string that could be user-provided.

    Logging Context: The use of with get_logger().contextualize(**log_context): inside a loop might not work as expected if the logger's context is overwritten or not properly isolated for each command execution.

    🔒 Security concerns

    - Command Injection: The dynamic construction of shell commands based on external input could lead to command injection vulnerabilities if not properly handled. It's crucial to ensure that any user-provided data is sanitized before being used to construct or execute shell commands.


    ✨ Review tool usage guide:

    Overview:
    The review tool scans the PR code changes, and generates a PR review. The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on any PR.
    When commenting, to edit configurations related to the review tool (pr_reviewer section), use the following template:

    /review --pr_reviewer.some_config1=... --pr_reviewer.some_config2=...
    

    With a configuration file, use the following template:

    [pr_reviewer]
    some_config1=...
    some_config2=...
    
    Utilizing extra instructions

    The review tool can be configured with extra instructions, which can be used to guide the model to a feedback tailored to the needs of your project.

    Be specific, clear, and concise in the instructions. With extra instructions, you are the prompter. Specify the relevant sub-tool, and the relevant aspects of the PR that you want to emphasize.

    Examples for extra instructions:

    [pr_reviewer] # /review #
    extra_instructions="""
    In the 'possible issues' section, emphasize the following:
    - Does the code logic cover relevant edge cases?
    - Is the code logic clear and easy to understand?
    - Is the code logic efficient?
    ...
    """
    

    Use triple quotes to write multi-line instructions. Use bullet points to make the instructions more readable.

    How to enable\disable automation
    • When you first install PR-Agent app, the default mode for the review tool is:
    pr_commands = ["/review", ...]
    

    meaning the review tool will run automatically on every PR, with the default configuration.
    Edit this field to enable/disable the tool, or to change the used configurations

    Auto-labels

    The review tool can auto-generate two specific types of labels for a PR:

    • a possible security issue label, that detects possible security issues (enable_review_labels_security flag)
    • a Review effort [1-5]: x label, where x is the estimated effort to review the PR (enable_review_labels_effort flag)
    Extra sub-tools

    The review tool provides a collection of possible feedbacks about a PR.
    It is recommended to review the possible options, and choose the ones relevant for your use case.
    Some of the feature that are disabled by default are quite useful, and should be considered for enabling. For example:
    require_score_review, require_soc2_ticket, and more.

    Auto-approve PRs

    By invoking:

    /review auto_approve
    

    The tool will automatically approve the PR, and add a comment with the approval.

    To ensure safety, the auto-approval feature is disabled by default. To enable auto-approval, you need to actively set in a pre-defined configuration file the following:

    [pr_reviewer]
    enable_auto_approval = true
    

    (this specific flag cannot be set with a command line argument, only in the configuration file, committed to the repository)

    You can also enable auto-approval only if the PR meets certain requirements, such as that the estimated_review_effort is equal or below a certain threshold, by adjusting the flag:

    [pr_reviewer]
    maximal_review_effort = 5
    
    More PR-Agent commands

    To invoke the PR-Agent, add a comment using one of the following commands:

    • /review: Request a review of your Pull Request.
    • /describe: Update the PR title and description based on the contents of the PR.
    • /improve [--extended]: Suggest code improvements. Extended mode provides a higher quality feedback.
    • /ask <QUESTION>: Ask a question about the PR.
    • /update_changelog: Update the changelog based on the PR's contents.
    • /add_docs 💎: Generate docstring for new components introduced in the PR.
    • /generate_labels 💎: Generate labels for the PR based on the PR's contents.
    • /analyze 💎: Automatically analyzes the PR, and presents changes walkthrough for each component.

    See the tools guide for more details.
    To list the possible configuration parameters, add a /config comment.

    See the review usage page for a comprehensive guide on using this tool.

    Copy link
    Contributor

    codiumai-pr-agent-pro bot commented Mar 6, 2024

    PR Code Suggestions

    CategorySuggestions                                                                                                                                                       
    Enhancement
    Validate the commands_conf parameter to ensure it starts with 'bitbucket_app.'.

    Consider validating the commands_conf parameter to ensure it is a valid configuration key
    before attempting to retrieve settings. This can prevent potential errors or unexpected
    behavior if an invalid configuration key is provided.

    pr_agent/servers/bitbucket_app.py [79]

    -commands = get_settings().get(f"bitbucket_app.{commands_conf}", {})
    +if not commands_conf.startswith("bitbucket_app."):
    +    raise ValueError("Invalid configuration key. It must start with 'bitbucket_app.'")
    +commands = get_settings().get(f"{commands_conf}", {})
     
    Security
    Use subprocess-like argument lists for commands to enhance security and correctness.

    Instead of manually splitting and joining the command and its arguments, consider using
    subprocess-like argument lists directly to avoid potential issues with command injection
    or incorrect argument parsing.

    pr_agent/servers/bitbucket_app.py [82-86]

     split_command = command.split(" ")
     command = split_command[0]
     args = split_command[1:]
     other_args = update_settings_from_args(args)
    -new_command = ' '.join([command] + other_args)
    +command_with_args = [command] + other_args
     
    Best practice
    Use specific exceptions for precise error handling in command execution.

    It's recommended to use a more specific exception handling than the generic Exception.
    Catching specific exceptions related to the command execution can make the error handling
    more precise and informative.

    pr_agent/servers/bitbucket_app.py [90-91]

    -except Exception as e:
    +except (CommandExecutionError, InvalidCommandError) as e:
         get_logger().error(f"Failed to perform command {command}: {e}")
     
    Use a constant for the 'bitbucket_app.' configuration key prefix to improve code clarity.

    For better code clarity and to avoid hardcoding strings, consider defining a constant for
    the configuration key prefix 'bitbucket_app.' used in the get_settings().get call.

    pr_agent/servers/bitbucket_app.py [79]

    -commands = get_settings().get(f"bitbucket_app.{commands_conf}", {})
    +BITBUCKET_APP_PREFIX = "bitbucket_app."
    +commands = get_settings().get(f"{BITBUCKET_APP_PREFIX}{commands_conf}", {})
     
    Maintainability
    Extract command execution logic into a separate method for better maintainability.

    To improve the maintainability and readability of the code, consider extracting the
    command execution logic into a separate method. This can make the
    _perform_commands_bitbucket function cleaner and easier to understand.

    pr_agent/servers/bitbucket_app.py [81-89]

     try:
    -    split_command = command.split(" ")
    +    await execute_command(command, agent, api_url, log_context)
    +except Exception as e:
    +    get_logger().error(f"Failed to perform command {command}: {e}")
    +
    +# Elsewhere in the code
    +async def execute_command(command_str, agent, api_url, log_context):
    +    split_command = command_str.split(" ")
         command = split_command[0]
         args = split_command[1:]
         other_args = update_settings_from_args(args)
         new_command = ' '.join([command] + other_args)
         get_logger().info(f"Performing command: {new_command}")
         with get_logger().contextualize(**log_context):
             await agent.handle_request(api_url, new_command)
    -except Exception as e:
    -    get_logger().error(f"Failed to perform command {command}: {e}")
     

    ✨ Improve tool usage guide:

    Overview:
    The improve tool scans the PR code changes, and automatically generates suggestions for improving the PR code. The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on a PR.
    When commenting, to edit configurations related to the improve tool (pr_code_suggestions section), use the following template:

    /improve --pr_code_suggestions.some_config1=... --pr_code_suggestions.some_config2=...
    

    With a configuration file, use the following template:

    [pr_code_suggestions]
    some_config1=...
    some_config2=...
    
    Enabling\disabling automation

    When you first install the app, the default mode for the improve tool is:

    pr_commands = ["/improve --pr_code_suggestions.summarize=true", ...]
    

    meaning the improve tool will run automatically on every PR, with summarization enabled. Delete this line to disable the tool from running automatically.

    Utilizing extra instructions

    Extra instructions are very important for the improve tool, since they enable to guide the model to suggestions that are more relevant to the specific needs of the project.

    Be specific, clear, and concise in the instructions. With extra instructions, you are the prompter. Specify relevant aspects that you want the model to focus on.

    Examples for extra instructions:

    [pr_code_suggestions] # /improve #
    extra_instructions="""
    Emphasize the following aspects:
    - Does the code logic cover relevant edge cases?
    - Is the code logic clear and easy to understand?
    - Is the code logic efficient?
    ...
    """
    

    Use triple quotes to write multi-line instructions. Use bullet points to make the instructions more readable.

    A note on code suggestions quality
    • While the current AI for code is getting better and better (GPT-4), it's not flawless. Not all the suggestions will be perfect, and a user should not accept all of them automatically.
    • Suggestions are not meant to be simplistic. Instead, they aim to give deep feedback and raise questions, ideas and thoughts to the user, who can then use his judgment, experience, and understanding of the code base.
    • Recommended to use the 'extra_instructions' field to guide the model to suggestions that are more relevant to the specific needs of the project, or use the custom suggestions 💎 tool
    • With large PRs, best quality will be obtained by using 'improve --extended' mode.
    More PR-Agent commands

    To invoke the PR-Agent, add a comment using one of the following commands:

    • /review: Request a review of your Pull Request.
    • /describe: Update the PR title and description based on the contents of the PR.
    • /improve [--extended]: Suggest code improvements. Extended mode provides a higher quality feedback.
    • /ask <QUESTION>: Ask a question about the PR.
    • /update_changelog: Update the changelog based on the PR's contents.
    • /add_docs 💎: Generate docstring for new components introduced in the PR.
    • /generate_labels 💎: Generate labels for the PR based on the PR's contents.
    • /analyze 💎: Automatically analyzes the PR, and presents changes walkthrough for each component.

    See the tools guide for more details.
    To list the possible configuration parameters, add a /config comment.

    See the improve usage page for a more comprehensive guide on using this tool.

    @mrT23 mrT23 merged commit 2f6f1d5 into main Mar 6, 2024
    2 checks passed
    @mrT23 mrT23 deleted the tr/bitbucket_pr_commands branch March 6, 2024 09:08
    @mrT23
    Copy link
    Collaborator Author

    mrT23 commented May 12, 2024

    /Users/talrid/Git/pr-agent/venv/bin/python /Users/talrid/Git/pr-agent/format_importance.py

    PR Code Suggestions ✨

    CategorySuggestions [importance]                                                                                                                       
    Best practice
    Use shlex.split for robust command parsing [8]

    Instead of manually splitting and joining commands, consider using a more robust method to
    handle command parsing and execution to prevent potential issues with commands containing
    spaces or special characters.

    pr_agent/servers/bitbucket_app.py [82-86]

    -split_command = command.split(" ")
    +import shlex
    +split_command = shlex.split(command)
     command = split_command[0]
     args = split_command[1:]
     other_args = update_settings_from_args(args)
     new_command = ' '.join([command] + other_args)
     
    Importance[1-10]: 8

    Why: Using shlex.split for command parsing is a best practice to handle potential issues with spaces or special characters in commands, making this suggestion highly relevant and impactful.

    Possible bug
    Validate configuration keys before usage to prevent runtime errors [7]

    Consider validating the commands_conf parameter to ensure it contains valid configuration
    keys before using it to fetch settings. This can prevent runtime errors due to invalid
    configuration keys.

    pr_agent/servers/bitbucket_app.py [79]

    +if not commands_conf in get_settings().keys():
    +    raise ValueError(f"Invalid configuration key: {commands_conf}")
     commands = get_settings().get(f"bitbucket_app.{commands_conf}", {})
     
    Importance[1-10]: 7

    Why: Validating configuration keys before using them is a good practice to prevent runtime errors, making this suggestion relevant and useful.

    Maintainability
    Add error handling in update_settings_from_args to manage unexpected inputs [7]

    To ensure the update_settings_from_args function modifies the settings as expected,
    consider adding error handling or validation inside this function to manage unexpected
    inputs or errors.

    pr_agent/servers/bitbucket_app.py [85]

    -other_args = update_settings_from_args(args)
    +try:
    +    other_args = update_settings_from_args(args)
    +except Exception as e:
    +    get_logger().error(f"Error updating settings from args: {e}")
    +    return
     
    Importance[1-10]: 7

    Why: Adding error handling or validation inside the update_settings_from_args function is crucial for maintaining robustness and preventing crashes from unexpected inputs.

    Enhancement
    Improve error logging by including exception type [6]

    To enhance error handling, consider logging the type of exception along with the error
    message. This can provide more context about the error and help in debugging.

    pr_agent/servers/bitbucket_app.py [91]

    -get_logger().error(f"Failed to perform command {command}: {e}")
    +get_logger().error(f"Failed to perform command {command}: {type(e).__name__}: {e}")
     
    Importance[1-10]: 6

    Why: Including the type of exception in logs can significantly improve debugging efforts, making this a beneficial enhancement.

    Process finished with exit code 0

    @mrT23
    Copy link
    Collaborator Author

    mrT23 commented May 12, 2024

    /Users/talrid/Git/pr-agent/venv/bin/python /Users/talrid/Git/pr-agent/format_importance.py

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Best practice
    Use shlex.split for robust command parsing

    Instead of manually splitting and joining commands, consider using a more robust method to
    handle command parsing and execution to prevent potential issues with commands containing
    spaces or special characters.

    pr_agent/servers/bitbucket_app.py [82-86]

    -split_command = command.split(" ")
    +import shlex
    +split_command = shlex.split(command)
     command = split_command[0]
     args = split_command[1:]
     other_args = update_settings_from_args(args)
     new_command = ' '.join([command] + other_args)
     
    Importance[1-10]: 8

    Why: Using shlex.split for command parsing is a best practice to handle potential issues with spaces or special characters in commands, making this suggestion highly relevant and impactful.

    8

    Possible bug
    Validate configuration keys before usage to prevent runtime errors

    Consider validating the commands_conf parameter to ensure it contains valid configuration
    keys before using it to fetch settings. This can prevent runtime errors due to invalid
    configuration keys.

    pr_agent/servers/bitbucket_app.py [79]

    +if not commands_conf in get_settings().keys():
    +    raise ValueError(f"Invalid configuration key: {commands_conf}")
     commands = get_settings().get(f"bitbucket_app.{commands_conf}", {})
     
    Importance[1-10]: 7

    Why: Validating configuration keys before using them is a good practice to prevent runtime errors, making this suggestion relevant and useful.

    7

    Maintainability
    Add error handling in update_settings_from_args to manage unexpected inputs

    To ensure the update_settings_from_args function modifies the settings as expected,
    consider adding error handling or validation inside this function to manage unexpected
    inputs or errors.

    pr_agent/servers/bitbucket_app.py [85]

    -other_args = update_settings_from_args(args)
    +try:
    +    other_args = update_settings_from_args(args)
    +except Exception as e:
    +    get_logger().error(f"Error updating settings from args: {e}")
    +    return
     
    Importance[1-10]: 7

    Why: Adding error handling or validation inside the update_settings_from_args function is crucial for maintaining robustness and preventing crashes from unexpected inputs.

    7

    Enhancement
    Improve error logging by including exception type

    To enhance error handling, consider logging the type of exception along with the error
    message. This can provide more context about the error and help in debugging.

    pr_agent/servers/bitbucket_app.py [91]

    -get_logger().error(f"Failed to perform command {command}: {e}")
    +get_logger().error(f"Failed to perform command {command}: {type(e).__name__}: {e}")
     
    Importance[1-10]: 6

    Why: Including the type of exception in logs can significantly improve debugging efforts, making this a beneficial enhancement.

    6

    Process finished with exit code 0

    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.

    None yet

    2 participants