From a5cb35418e589de1bbf019331a2200ceaa7f37f8 Mon Sep 17 00:00:00 2001 From: mrT23 Date: Mon, 26 Feb 2024 14:20:50 +0200 Subject: [PATCH 01/16] remove 'review_requested' (can trigger rate limit, and doesnt make sense algorighmically) --- pr_agent/settings/configuration.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pr_agent/settings/configuration.toml b/pr_agent/settings/configuration.toml index 9ac46b79..5df59210 100644 --- a/pr_agent/settings/configuration.toml +++ b/pr_agent/settings/configuration.toml @@ -136,7 +136,7 @@ try_fix_invalid_inline_comments = true # these toggles allows running the github app from custom deployments override_deployment_type = true # settings for "pull_request" event -handle_pr_actions = ['opened', 'reopened', 'ready_for_review', 'review_requested'] +handle_pr_actions = ['opened', 'reopened', 'ready_for_review'] pr_commands = [ "/describe --pr_description.add_original_user_description=true --pr_description.keep_original_user_title=true", "/review --pr_reviewer.num_code_suggestions=0", From ffe4512b7de81f889f0869cc312009d4372dc11e Mon Sep 17 00:00:00 2001 From: mrT23 Date: Mon, 26 Feb 2024 15:16:59 +0200 Subject: [PATCH 02/16] small log improvement --- pr_agent/servers/github_app.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pr_agent/servers/github_app.py b/pr_agent/servers/github_app.py index b365bfd6..83783942 100644 --- a/pr_agent/servers/github_app.py +++ b/pr_agent/servers/github_app.py @@ -273,7 +273,7 @@ async def _perform_auto_commands_github(commands_conf: str, agent: PRAgent, body other_args = update_settings_from_args(args) new_command = ' '.join([command] + other_args) with get_logger().contextualize(**log_context): - get_logger().info(f"New PR opened. Performing auto command '{new_command}', for {api_url=}") + get_logger().info(f"{commands_conf}. Performing auto command '{new_command}', for {api_url=}") await agent.handle_request(api_url, new_command) From 8843f7bc8bff7f311c52de4b60b6445e4d49536f Mon Sep 17 00:00:00 2001 From: Ori Kotek Date: Mon, 26 Feb 2024 16:15:23 +0200 Subject: [PATCH 03/16] Add analytics logging --- pr_agent/agent/pr_agent.py | 1 + pr_agent/log/__init__.py | 7 ++++++- pr_agent/servers/github_app.py | 3 ++- 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/pr_agent/agent/pr_agent.py b/pr_agent/agent/pr_agent.py index 55c3c267..5be0f779 100644 --- a/pr_agent/agent/pr_agent.py +++ b/pr_agent/agent/pr_agent.py @@ -74,6 +74,7 @@ class PRAgent: action = action.lstrip("/").lower() with get_logger().contextualize(command=action): + get_logger().info("PR-Agent request handler started", analytics=True) if action == "reflect_and_review": get_settings().pr_reviewer.ask_and_reflect = True if action == "answer": diff --git a/pr_agent/log/__init__.py b/pr_agent/log/__init__.py index f618cb76..53430ea1 100644 --- a/pr_agent/log/__init__.py +++ b/pr_agent/log/__init__.py @@ -22,6 +22,10 @@ def analytics_filter(record: dict) -> bool: return record.get("extra", {}).get("analytics", False) +def inv_analytics_filter(record: dict) -> bool: + return not record.get("extra", {}).get("analytics", False) + + def setup_logger(level: str = "INFO", fmt: LoggingFormat = LoggingFormat.CONSOLE): level: int = logging.getLevelName(level.upper()) if type(level) is not int: @@ -31,6 +35,7 @@ def setup_logger(level: str = "INFO", fmt: LoggingFormat = LoggingFormat.CONSOLE logger.remove(None) logger.add( sys.stdout, + filter=inv_analytics_filter, level=level, format="{message}", colorize=False, @@ -38,7 +43,7 @@ def setup_logger(level: str = "INFO", fmt: LoggingFormat = LoggingFormat.CONSOLE ) elif fmt == LoggingFormat.CONSOLE: # does not print the 'extra' fields logger.remove(None) - logger.add(sys.stdout, level=level, colorize=True) + logger.add(sys.stdout, level=level, colorize=True, filter=inv_analytics_filter) log_folder = get_settings().get("CONFIG.ANALYTICS_FOLDER", "") if log_folder: diff --git a/pr_agent/servers/github_app.py b/pr_agent/servers/github_app.py index 83783942..892a2952 100644 --- a/pr_agent/servers/github_app.py +++ b/pr_agent/servers/github_app.py @@ -205,8 +205,9 @@ async def handle_request(body: Dict[str, Any], event: str): return {} agent = PRAgent() sender = body.get("sender", {}).get("login") + app_name = get_settings().get("CONFIG.APP_NAME", "Unknown") log_context = {"action": action, "event": event, "sender": sender, "server_type": "github_app", - "request_id": uuid.uuid4().hex} + "request_id": uuid.uuid4().hex, "app_name": app_name} # handle comments on PRs if action == 'created': From 86c7c495f296be06a192cd7590ab06d7012a3f73 Mon Sep 17 00:00:00 2001 From: mrT23 Date: Mon, 26 Feb 2024 16:29:40 +0200 Subject: [PATCH 04/16] Add documentation for 'ignore_pr_titles' parameter in Usage.md --- Usage.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/Usage.md b/Usage.md index c2f91bd0..bd4857e4 100644 --- a/Usage.md +++ b/Usage.md @@ -174,6 +174,13 @@ To cancel the automatic run of all the tools, set: handle_pr_actions = [] ``` +You can also disable automatic runs for PRs with specific titles, by setting the `ignore_pr_titles` parameter with the relevant regex. For example: +``` +[github_app] +ignore_pr_title = ["^[Auto]", ".*ignore.*"] +``` +will ignore PRs with titles that start with "Auto" or contain the word "ignore". + ##### GitHub app automatic tools for push actions (commits to an open PR) In addition to running automatic tools when a PR is opened, the GitHub app can also respond to new code that is pushed to an open PR. From 77831c793dabaae5b087a15fabfc07629098f7bf Mon Sep 17 00:00:00 2001 From: Ori Kotek Date: Mon, 26 Feb 2024 18:31:12 +0200 Subject: [PATCH 05/16] Identity provider --- pr_agent/identity_providers/__init__.py | 13 +++++++++++++ .../default_identity_provider.py | 9 +++++++++ .../identity_providers/identity_provider.py | 18 ++++++++++++++++++ 3 files changed, 40 insertions(+) create mode 100644 pr_agent/identity_providers/__init__.py create mode 100644 pr_agent/identity_providers/default_identity_provider.py create mode 100644 pr_agent/identity_providers/identity_provider.py diff --git a/pr_agent/identity_providers/__init__.py b/pr_agent/identity_providers/__init__.py new file mode 100644 index 00000000..6df37ecb --- /dev/null +++ b/pr_agent/identity_providers/__init__.py @@ -0,0 +1,13 @@ +from pr_agent.config_loader import get_settings +from pr_agent.identity_providers.default_identity_provider import DefaultIdentityProvider + +_IDENTITY_PROVIDERS = { + 'default': DefaultIdentityProvider +} + + +def get_identity_provider(): + identity_provider_id = get_settings().get("CONFIG.IDENTITY_PROVIDER", "default") + if identity_provider_id not in _IDENTITY_PROVIDERS: + raise ValueError(f"Unknown identity provider: {identity_provider_id}") + return _IDENTITY_PROVIDERS[identity_provider_id]() \ No newline at end of file diff --git a/pr_agent/identity_providers/default_identity_provider.py b/pr_agent/identity_providers/default_identity_provider.py new file mode 100644 index 00000000..c542e1c2 --- /dev/null +++ b/pr_agent/identity_providers/default_identity_provider.py @@ -0,0 +1,9 @@ +from pr_agent.identity_providers.identity_provider import Eligibility, IdentityProvider + + +class DefaultIdentityProvider(IdentityProvider): + def verify_eligibility(self, git_provider, git_provider_id, pr_url): + return Eligibility.ELIGIBLE + + def inc_invocation_count(self, git_provider, git_provider_id): + pass diff --git a/pr_agent/identity_providers/identity_provider.py b/pr_agent/identity_providers/identity_provider.py new file mode 100644 index 00000000..58e5f6c6 --- /dev/null +++ b/pr_agent/identity_providers/identity_provider.py @@ -0,0 +1,18 @@ +from abc import ABC, abstractmethod +from enum import Enum + + +class Eligibility(Enum): + NOT_ELIGIBLE = 0 + ELIGIBLE = 1 + TRIAL = 2 + + +class IdentityProvider(ABC): + @abstractmethod + def verify_eligibility(self, git_provider, git_provier_id, pr_url): + pass + + @abstractmethod + def inc_invocation_count(self, git_provider, git_provider_id): + pass From 8bdc90c0f746e1c62deba90fb07992dceeb98eb9 Mon Sep 17 00:00:00 2001 From: Ori Kotek Date: Mon, 26 Feb 2024 19:00:21 +0200 Subject: [PATCH 06/16] Identity provider --- pr_agent/servers/bitbucket_app.py | 49 +++++++++++++++++++++++-------- pr_agent/servers/github_app.py | 41 ++++++++++++++++++-------- 2 files changed, 64 insertions(+), 26 deletions(-) diff --git a/pr_agent/servers/bitbucket_app.py b/pr_agent/servers/bitbucket_app.py index d2cae362..df513f2b 100644 --- a/pr_agent/servers/bitbucket_app.py +++ b/pr_agent/servers/bitbucket_app.py @@ -1,3 +1,4 @@ +import base64 import copy import hashlib import json @@ -17,6 +18,8 @@ from starlette_context.middleware import RawContextMiddleware from pr_agent.agent.pr_agent import PRAgent from pr_agent.config_loader import get_settings, global_settings from pr_agent.git_providers.utils import apply_repo_settings +from pr_agent.identity_providers import get_identity_provider +from pr_agent.identity_providers.identity_provider import Eligibility from pr_agent.log import LoggingFormat, get_logger, setup_logger from pr_agent.secret_providers import get_secret_provider from pr_agent.servers.github_action_runner import get_setting_or_env, is_true @@ -80,11 +83,27 @@ async def handle_github_webhooks(background_tasks: BackgroundTasks, request: Req get_logger().debug(data) async def inner(): try: - owner = data["data"]["repository"]["owner"]["username"] + try: + if data["data"]["actor"]["type"] != "user": + return "OK" + except KeyError: + get_logger().error("Failed to get actor type, check previous logs, this shouldn't happen.") + try: + owner = data["data"]["repository"]["owner"]["username"] + except Exception as e: + get_logger().error(f"Failed to get owner, will continue: {e}") + owner = "unknown" + sender_id = data["data"]["actor"]["account_id"] log_context["sender"] = owner - secrets = json.loads(secret_provider.get_secret(owner)) + log_context["sender_id"] = sender_id + jwt_parts = input_jwt.split(".") + claim_part = jwt_parts[1] + claim_part += "=" * (-len(claim_part) % 4) + decoded_claims = base64.urlsafe_b64decode(claim_part) + claims = json.loads(decoded_claims) + client_key = claims["iss"] + secrets = json.loads(secret_provider.get_secret(client_key)) shared_secret = secrets["shared_secret"] - client_key = secrets["client_key"] jwt.decode(input_jwt, shared_secret, audience=client_key, algorithms=["HS256"]) bearer_token = await get_bearer_token(shared_secret, client_key) context['bitbucket_bearer_token'] = bearer_token @@ -98,15 +117,17 @@ async def handle_github_webhooks(background_tasks: BackgroundTasks, request: Req if pr_url: with get_logger().contextualize(**log_context): apply_repo_settings(pr_url) - auto_review = get_setting_or_env("BITBUCKET_APP.AUTO_REVIEW", None) - if auto_review is None or is_true(auto_review): # by default, auto review is enabled - await PRReviewer(pr_url).run() - auto_improve = get_setting_or_env("BITBUCKET_APP.AUTO_IMPROVE", None) - if is_true(auto_improve): # by default, auto improve is disabled - await PRCodeSuggestions(pr_url).run() - auto_describe = get_setting_or_env("BITBUCKET_APP.AUTO_DESCRIBE", None) - if is_true(auto_describe): # by default, auto describe is disabled - await PRDescription(pr_url).run() + if get_identity_provider().verify_eligibility("bitbucket", + sender_id, pr_url) is not Eligibility.NOT_ELIGIBLE: + auto_review = get_setting_or_env("BITBUCKET_APP.AUTO_REVIEW", None) + if auto_review is None or is_true(auto_review): # by default, auto review is enabled + await PRReviewer(pr_url).run() + auto_improve = get_setting_or_env("BITBUCKET_APP.AUTO_IMPROVE", None) + if is_true(auto_improve): # by default, auto improve is disabled + await PRCodeSuggestions(pr_url).run() + auto_describe = get_setting_or_env("BITBUCKET_APP.AUTO_DESCRIBE", None) + if is_true(auto_describe): # by default, auto describe is disabled + await PRDescription(pr_url).run() # with get_logger().contextualize(**log_context): # await agent.handle_request(pr_url, "review") elif event == "pullrequest:comment_created": @@ -115,7 +136,9 @@ async def handle_github_webhooks(background_tasks: BackgroundTasks, request: Req log_context["event"] = "comment" comment_body = data["data"]["comment"]["content"]["raw"] with get_logger().contextualize(**log_context): - await agent.handle_request(pr_url, comment_body) + if get_identity_provider().verify_eligibility("bitbucket", + sender_id, pr_url) is not Eligibility.NOT_ELIGIBLE: + await agent.handle_request(pr_url, comment_body) except Exception as e: get_logger().error(f"Failed to handle webhook: {e}") background_tasks.add_task(inner) diff --git a/pr_agent/servers/github_app.py b/pr_agent/servers/github_app.py index 892a2952..445facba 100644 --- a/pr_agent/servers/github_app.py +++ b/pr_agent/servers/github_app.py @@ -3,7 +3,7 @@ import copy import os import re import uuid -from typing import Any, Dict, List, Tuple +from typing import Any, Dict, Tuple import uvicorn from fastapi import APIRouter, FastAPI, HTTPException, Request, Response @@ -17,11 +17,19 @@ from pr_agent.config_loader import get_settings, global_settings from pr_agent.git_providers import get_git_provider from pr_agent.git_providers.git_provider import IncrementalPR from pr_agent.git_providers.utils import apply_repo_settings +from pr_agent.identity_providers import get_identity_provider +from pr_agent.identity_providers.identity_provider import Eligibility from pr_agent.log import LoggingFormat, get_logger, setup_logger from pr_agent.servers.utils import DefaultDictWithTimeout, verify_signature setup_logger(fmt=LoggingFormat.JSON) - +base_path = os.path.dirname(os.path.dirname(os.path.realpath(__file__))) +build_number_path = os.path.join(base_path, "build_number.txt") +if os.path.exists(build_number_path): + with open(build_number_path) as f: + build_number = f.read().strip() +else: + build_number = "unknown" router = APIRouter() @@ -70,6 +78,7 @@ _pending_task_duplicate_push_conditions = DefaultDictWithTimeout(asyncio.locks.C async def handle_comments_on_pr(body: Dict[str, Any], event: str, sender: str, + sender_id: str, action: str, log_context: Dict[str, Any], agent: PRAgent): @@ -98,13 +107,15 @@ async def handle_comments_on_pr(body: Dict[str, Any], comment_id = body.get("comment", {}).get("id") provider = get_git_provider()(pr_url=api_url) with get_logger().contextualize(**log_context): - get_logger().info(f"Processing comment on PR {api_url=}, comment_body={comment_body}") - await agent.handle_request(api_url, comment_body, - notify=lambda: provider.add_eyes_reaction(comment_id, disable_eyes=disable_eyes)) + if get_identity_provider().verify_eligibility("github", sender_id, api_url) is not Eligibility.NOT_ELIGIBLE: + get_logger().info(f"Processing comment on PR {api_url=}, comment_body={comment_body}") + await agent.handle_request(api_url, comment_body, + notify=lambda: provider.add_eyes_reaction(comment_id, disable_eyes=disable_eyes)) async def handle_new_pr_opened(body: Dict[str, Any], event: str, sender: str, + sender_id: str, action: str, log_context: Dict[str, Any], agent: PRAgent): @@ -123,11 +134,13 @@ async def handle_new_pr_opened(body: Dict[str, Any], get_logger().info(f"Invalid PR event: {action=} {api_url=}") return {} if action in get_settings().github_app.handle_pr_actions: # ['opened', 'reopened', 'ready_for_review', 'review_requested'] - await _perform_auto_commands_github("pr_commands", agent, body, api_url, log_context) + if get_identity_provider().verify_eligibility("github", sender_id, api_url) is not Eligibility.NOT_ELIGIBLE: + await _perform_auto_commands_github("pr_commands", agent, body, api_url, log_context) async def handle_push_trigger_for_new_commits(body: Dict[str, Any], event: str, sender: str, + sender_id: str, action: str, log_context: Dict[str, Any], agent: PRAgent): @@ -180,8 +193,9 @@ async def handle_push_trigger_for_new_commits(body: Dict[str, Any], if get_settings().github_app.push_trigger_wait_for_initial_review and not get_git_provider()(api_url, incremental=IncrementalPR( True)).previous_review: - get_logger().info(f"Skipping incremental review because there was no initial review for {api_url=} yet") - return {} + if get_identity_provider().verify_eligibility("github", sender_id, api_url) is not Eligibility.NOT_ELIGIBLE: + get_logger().info(f"Skipping incremental review because there was no initial review for {api_url=} yet") + return {} get_logger().info(f"Performing incremental review for {api_url=} because of {event=} and {action=}") await _perform_auto_commands_github("push_commands", agent, body, api_url, log_context) @@ -205,25 +219,26 @@ async def handle_request(body: Dict[str, Any], event: str): return {} agent = PRAgent() sender = body.get("sender", {}).get("login") + sender_id = body.get("sender", {}).get("id") app_name = get_settings().get("CONFIG.APP_NAME", "Unknown") log_context = {"action": action, "event": event, "sender": sender, "server_type": "github_app", - "request_id": uuid.uuid4().hex, "app_name": app_name} + "request_id": uuid.uuid4().hex, "build_number": build_number, "app_name": app_name} # handle comments on PRs if action == 'created': get_logger().debug(f'Request body', artifact=body) - await handle_comments_on_pr(body, event, sender, action, log_context, agent) + await handle_comments_on_pr(body, event, sender, sender_id, action, log_context, agent) # handle new PRs elif event == 'pull_request' and action != 'synchronize': get_logger().debug(f'Request body', artifact=body) - await handle_new_pr_opened(body, event, sender, action, log_context, agent) + await handle_new_pr_opened(body, event, sender, sender_id, action, log_context, agent) # handle pull_request event with synchronize action - "push trigger" for new commits elif event == 'pull_request' and action == 'synchronize': get_logger().debug(f'Request body', artifact=body) - await handle_push_trigger_for_new_commits(body, event, sender, action, log_context, agent) + await handle_push_trigger_for_new_commits(body, event, sender, sender_id, action, log_context, agent) else: get_logger().info(f"event {event=} action {action=} does not require any handling") - return {} + return {} def handle_line_comments(body: Dict, comment_body: [str, Any]) -> str: From 4921c26432c37e3bbdd6f25a7385e4e205436186 Mon Sep 17 00:00:00 2001 From: mrT23 Date: Mon, 26 Feb 2024 20:02:11 +0200 Subject: [PATCH 07/16] Add functionality to calculate and log PR statistics on closure --- pr_agent/git_providers/git_provider.py | 2 ++ pr_agent/git_providers/github_provider.py | 23 ++++++++++++++++++++++- pr_agent/servers/github_app.py | 13 ++++++++++++- 3 files changed, 36 insertions(+), 2 deletions(-) diff --git a/pr_agent/git_providers/git_provider.py b/pr_agent/git_providers/git_provider.py index ec4c8c54..3ef86709 100644 --- a/pr_agent/git_providers/git_provider.py +++ b/pr_agent/git_providers/git_provider.py @@ -190,6 +190,8 @@ class GitProvider(ABC): def auto_approve(self) -> bool: return False + def calc_pr_statistics(self, pull_request_data: dict): + return {} def get_main_pr_language(languages, files) -> str: diff --git a/pr_agent/git_providers/github_provider.py b/pr_agent/git_providers/github_provider.py index 1c23b328..ea152b9e 100644 --- a/pr_agent/git_providers/github_provider.py +++ b/pr_agent/git_providers/github_provider.py @@ -700,4 +700,25 @@ class GithubProvider(GitProvider): return False except Exception as e: get_logger().exception(f"Failed to auto-approve, error: {e}") - return False \ No newline at end of file + return False + + def calc_pr_statistics(self, pull_request_data: dict): + try: + out = {} + from datetime import datetime + created_at = pull_request_data['created_at'] + closed_at = pull_request_data['closed_at'] + closed_at_datetime = datetime.strptime(closed_at, "%Y-%m-%dT%H:%M:%SZ") + created_at_datetime = datetime.strptime(created_at, "%Y-%m-%dT%H:%M:%SZ") + difference = closed_at_datetime - created_at_datetime + out['hours'] = difference.total_seconds() / 3600 + out['commits'] = pull_request_data['commits'] + out['comments'] = pull_request_data['comments'] + out['review_comments'] = pull_request_data['review_comments'] + out['changed_files'] = pull_request_data['changed_files'] + out['additions'] = pull_request_data['additions'] + out['deletions'] = pull_request_data['deletions'] + except Exception as e: + get_logger().exception(f"Failed to calculate PR statistics, error: {e}") + return {} + return out diff --git a/pr_agent/servers/github_app.py b/pr_agent/servers/github_app.py index 445facba..777e3b13 100644 --- a/pr_agent/servers/github_app.py +++ b/pr_agent/servers/github_app.py @@ -206,6 +206,14 @@ async def handle_push_trigger_for_new_commits(body: Dict[str, Any], _duplicate_push_triggers[api_url] -= 1 +def handle_closed_pr(body, event, action, log_context): + pull_request = body.get("pull_request") + api_url = pull_request.get("url") + pr_statistics = get_git_provider()(pr_url=api_url).calc_pr_statistics(pull_request) + with get_logger().contextualize(pr_statistics=pr_statistics): + get_logger().info("PR-Agent closed pr statistics", analytics=True) + + async def handle_request(body: Dict[str, Any], event: str): """ Handle incoming GitHub webhook requests. @@ -229,13 +237,16 @@ async def handle_request(body: Dict[str, Any], event: str): get_logger().debug(f'Request body', artifact=body) await handle_comments_on_pr(body, event, sender, sender_id, action, log_context, agent) # handle new PRs - elif event == 'pull_request' and action != 'synchronize': + elif event == 'pull_request' and action != 'synchronize' and action != 'closed': get_logger().debug(f'Request body', artifact=body) await handle_new_pr_opened(body, event, sender, sender_id, action, log_context, agent) # handle pull_request event with synchronize action - "push trigger" for new commits elif event == 'pull_request' and action == 'synchronize': get_logger().debug(f'Request body', artifact=body) await handle_push_trigger_for_new_commits(body, event, sender, sender_id, action, log_context, agent) + elif event == 'pull_request' and action == 'closed': + if get_settings().get("CONFIG.ANALYTICS_FOLDER", ""): + handle_closed_pr(body, event, action, log_context) else: get_logger().info(f"event {event=} action {action=} does not require any handling") return {} From 60a37158b1763871d0031ca7735b9a067732552c Mon Sep 17 00:00:00 2001 From: mrT23 Date: Mon, 26 Feb 2024 20:09:01 +0200 Subject: [PATCH 08/16] Add functionality to calculate and log PR statistics on closure --- pr_agent/servers/github_app.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pr_agent/servers/github_app.py b/pr_agent/servers/github_app.py index 777e3b13..0b379777 100644 --- a/pr_agent/servers/github_app.py +++ b/pr_agent/servers/github_app.py @@ -211,7 +211,7 @@ def handle_closed_pr(body, event, action, log_context): api_url = pull_request.get("url") pr_statistics = get_git_provider()(pr_url=api_url).calc_pr_statistics(pull_request) with get_logger().contextualize(pr_statistics=pr_statistics): - get_logger().info("PR-Agent closed pr statistics", analytics=True) + get_logger().info("PR-Agent statistics for closed PR", analytics=True) async def handle_request(body: Dict[str, Any], event: str): From ab29cf2b309bc42993b2a8c0765d81b7c4c5a190 Mon Sep 17 00:00:00 2001 From: Ori Kotek Date: Mon, 26 Feb 2024 20:21:20 +0200 Subject: [PATCH 09/16] Identity provider --- pr_agent/servers/github_app.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pr_agent/servers/github_app.py b/pr_agent/servers/github_app.py index 445facba..a765a3be 100644 --- a/pr_agent/servers/github_app.py +++ b/pr_agent/servers/github_app.py @@ -193,11 +193,11 @@ async def handle_push_trigger_for_new_commits(body: Dict[str, Any], if get_settings().github_app.push_trigger_wait_for_initial_review and not get_git_provider()(api_url, incremental=IncrementalPR( True)).previous_review: - if get_identity_provider().verify_eligibility("github", sender_id, api_url) is not Eligibility.NOT_ELIGIBLE: - get_logger().info(f"Skipping incremental review because there was no initial review for {api_url=} yet") - return {} - get_logger().info(f"Performing incremental review for {api_url=} because of {event=} and {action=}") - await _perform_auto_commands_github("push_commands", agent, body, api_url, log_context) + get_logger().info(f"Skipping incremental review because there was no initial review for {api_url=} yet") + return {} + if get_identity_provider().verify_eligibility("github", sender_id, api_url) is not Eligibility.NOT_ELIGIBLE: + get_logger().info(f"Performing incremental review for {api_url=} because of {event=} and {action=}") + await _perform_auto_commands_github("push_commands", agent, body, api_url, log_context) finally: # release the waiting task block From edc9d8944eac25e0a3074011117299fae5bd8ba6 Mon Sep 17 00:00:00 2001 From: mrT23 Date: Mon, 26 Feb 2024 20:56:43 +0200 Subject: [PATCH 10/16] Refactor handle_closed_pr function to check for merged PRs --- pr_agent/servers/github_app.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/pr_agent/servers/github_app.py b/pr_agent/servers/github_app.py index f67e4818..34504dde 100644 --- a/pr_agent/servers/github_app.py +++ b/pr_agent/servers/github_app.py @@ -207,8 +207,11 @@ async def handle_push_trigger_for_new_commits(body: Dict[str, Any], def handle_closed_pr(body, event, action, log_context): - pull_request = body.get("pull_request") - api_url = pull_request.get("url") + pull_request = body.get("pull_request", {}) + is_merged = pull_request.get("merged", False) + if not is_merged: + return + api_url = pull_request.get("url", "") pr_statistics = get_git_provider()(pr_url=api_url).calc_pr_statistics(pull_request) with get_logger().contextualize(pr_statistics=pr_statistics): get_logger().info("PR-Agent statistics for closed PR", analytics=True) From 4316d00941cf6c39bd1c8740a348dbf585ddbb15 Mon Sep 17 00:00:00 2001 From: mrT23 Date: Mon, 26 Feb 2024 21:12:28 +0200 Subject: [PATCH 11/16] log_context --- pr_agent/servers/github_app.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pr_agent/servers/github_app.py b/pr_agent/servers/github_app.py index 34504dde..5f59969b 100644 --- a/pr_agent/servers/github_app.py +++ b/pr_agent/servers/github_app.py @@ -213,8 +213,9 @@ def handle_closed_pr(body, event, action, log_context): return api_url = pull_request.get("url", "") pr_statistics = get_git_provider()(pr_url=api_url).calc_pr_statistics(pull_request) - with get_logger().contextualize(pr_statistics=pr_statistics): - get_logger().info("PR-Agent statistics for closed PR", analytics=True) + with get_logger().contextualize(**log_context): + with get_logger().contextualize(pr_statistics=pr_statistics): + get_logger().info("PR-Agent statistics for closed PR", analytics=True) async def handle_request(body: Dict[str, Any], event: str): From 92d3627de0b284285ee9b01d24c4a561b49c1eba Mon Sep 17 00:00:00 2001 From: Tal Date: Tue, 27 Feb 2024 23:10:44 +0200 Subject: [PATCH 12/16] Update IMPROVE.md --- docs/IMPROVE.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/docs/IMPROVE.md b/docs/IMPROVE.md index 3f14450b..e1917696 100644 --- a/docs/IMPROVE.md +++ b/docs/IMPROVE.md @@ -3,7 +3,6 @@ ## Table of Contents - [Overview](#overview) - [Configuration options](#configuration-options) - - [Summarize mode](#summarize-mode) - [Usage Tips](#usage-tips) - [Extra instructions](#extra-instructions) - [PR footprint - regular vs summarize mode](#pr-footprint---regular-vs-summarize-mode) @@ -16,7 +15,7 @@ The tool can be triggered automatically every time a new PR is [opened](https:// /improve ``` -### Summarized vs commitable code suggestions +### Summarized vs committable code suggestions The code suggestions can be presented as a single comment (via `pr_code_suggestions.summarize=true`): ___ From 0c654b3b64a917ab5f01401f76a3c69bae1fc497 Mon Sep 17 00:00:00 2001 From: mrT23 Date: Wed, 28 Feb 2024 08:41:25 +0200 Subject: [PATCH 13/16] handle unsupported platforms for update changelog --- pr_agent/tools/pr_update_changelog.py | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/pr_agent/tools/pr_update_changelog.py b/pr_agent/tools/pr_update_changelog.py index dc9d72e4..3d2ad2c7 100644 --- a/pr_agent/tools/pr_update_changelog.py +++ b/pr_agent/tools/pr_update_changelog.py @@ -9,7 +9,7 @@ from pr_agent.algo.ai_handlers.litellm_ai_handler import LiteLLMAIHandler from pr_agent.algo.pr_processing import get_pr_diff, retry_with_fallback_models from pr_agent.algo.token_handler import TokenHandler from pr_agent.config_loader import get_settings -from pr_agent.git_providers import get_git_provider +from pr_agent.git_providers import get_git_provider, GithubProvider from pr_agent.git_providers.git_provider import get_main_pr_language from pr_agent.log import get_logger @@ -46,12 +46,19 @@ class PRUpdateChangelog: get_settings().pr_update_changelog_prompt.user) async def run(self): - # assert type(self.git_provider) == GithubProvider, "Currently only Github is supported" - get_logger().info('Updating the changelog...') relevant_configs = {'pr_update_changelog': dict(get_settings().pr_update_changelog), 'config': dict(get_settings().config)} get_logger().debug("Relevant configs", artifacts=relevant_configs) + + # currently only GitHub is supported for pushing changelog changes + if get_settings().pr_update_changelog.push_changelog_changes and type(self.git_provider) != GithubProvider: + get_logger().error("Pushing changelog changes is not currently supported for this code platform") + if get_settings().config.publish_output: + self.git_provider.publish_comment( + "Pushing changelog changes is not currently supported for this code platform") + return + if get_settings().config.publish_output: self.git_provider.publish_comment("Preparing changelog updates...", is_temporary=True) From 39538c5356c202af49ac9aa24fb1119b8c7c80b8 Mon Sep 17 00:00:00 2001 From: mrT23 Date: Wed, 28 Feb 2024 09:01:39 +0200 Subject: [PATCH 14/16] cleaning --- pr_agent/tools/pr_generate_labels.py | 7 ------- pr_agent/tools/pr_reviewer.py | 1 - 2 files changed, 8 deletions(-) diff --git a/pr_agent/tools/pr_generate_labels.py b/pr_agent/tools/pr_generate_labels.py index a606ab4a..1d91d5e0 100644 --- a/pr_agent/tools/pr_generate_labels.py +++ b/pr_agent/tools/pr_generate_labels.py @@ -139,10 +139,6 @@ class PRGenerateLabels: system_prompt = environment.from_string(get_settings().pr_custom_labels_prompt.system).render(variables) user_prompt = environment.from_string(get_settings().pr_custom_labels_prompt.user).render(variables) - if get_settings().config.verbosity_level >= 2: - get_logger().info(f"\nSystem prompt:\n{system_prompt}") - get_logger().info(f"\nUser prompt:\n{user_prompt}") - response, finish_reason = await self.ai_handler.chat_completion( model=model, temperature=0.2, @@ -150,9 +146,6 @@ class PRGenerateLabels: user=user_prompt ) - if get_settings().config.verbosity_level >= 2: - get_logger().info(f"\nAI response:\n{response}") - return response def _prepare_data(self): diff --git a/pr_agent/tools/pr_reviewer.py b/pr_agent/tools/pr_reviewer.py index 960b0149..0eabc59b 100644 --- a/pr_agent/tools/pr_reviewer.py +++ b/pr_agent/tools/pr_reviewer.py @@ -170,7 +170,6 @@ class PRReviewer: user=user_prompt ) - get_logger().debug(f"\nAI response:\n{response}") return response def _prepare_pr_review(self) -> str: From 07f507c442d591595eab14fb3610a2bac8c23fd2 Mon Sep 17 00:00:00 2001 From: mrT23 Date: Wed, 28 Feb 2024 09:08:48 +0200 Subject: [PATCH 15/16] remove_initial_comment --- pr_agent/tools/pr_code_suggestions.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/pr_agent/tools/pr_code_suggestions.py b/pr_agent/tools/pr_code_suggestions.py index a6a12a3a..b10f1367 100644 --- a/pr_agent/tools/pr_code_suggestions.py +++ b/pr_agent/tools/pr_code_suggestions.py @@ -124,6 +124,12 @@ class PRCodeSuggestions: get_logger().error(f"Failed to generate code suggestions for PR, error: {e}") if self.progress_response: self.progress_response.delete() + else: + try: + self.git_provider.remove_initial_comment() + self.git_provider.publish_comment(f"Failed to generate code suggestions for PR") + except Exception as e: + pass async def _prepare_prediction(self, model: str): self.patches_diff = get_pr_diff(self.git_provider, From 047c3706838cb0f0c88159f757deaae543a00e62 Mon Sep 17 00:00:00 2001 From: mrT23 Date: Wed, 28 Feb 2024 09:20:14 +0200 Subject: [PATCH 16/16] Update README.md and add gfm markdown support check in pr_help_message.py --- README.md | 4 ++-- pr_agent/tools/pr_help_message.py | 5 +++++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index dcd6e583..78eff3bd 100644 --- a/README.md +++ b/README.md @@ -74,11 +74,11 @@ CodiumAI PR-Agent is an open-source tool to help efficiently review and handle p | | Ask | :white_check_mark: | :white_check_mark: | :white_check_mark: | :white_check_mark: | | | ⮑ [Ask on code lines](./docs/ASK.md#ask-lines) | :white_check_mark: | :white_check_mark: | | | | | [Custom Suggestions](https://github.com/Codium-ai/pr-agent/blob/main/docs/CUSTOM_SUGGESTIONS.md) 💎 | :white_check_mark: | :white_check_mark: | :white_check_mark: | :white_check_mark: | -| | [Test](https://github.com/Codium-ai/pr-agent/blob/main/docs/TEST.md) 💎 | :white_check_mark: | :white_check_mark: | :white_check_mark: | :white_check_mark: | +| | [Test](https://github.com/Codium-ai/pr-agent/blob/main/docs/TEST.md) 💎 | :white_check_mark: | :white_check_mark: | | :white_check_mark: | | | Reflect and Review | :white_check_mark: | :white_check_mark: | :white_check_mark: | :white_check_mark: | | | Update CHANGELOG.md | :white_check_mark: | :white_check_mark: | :white_check_mark: | :white_check_mark: | | | Find Similar Issue | :white_check_mark: | | | | -| | [Add PR Documentation](https://github.com/Codium-ai/pr-agent/blob/main/docs/ADD_DOCUMENTATION.md) 💎 | :white_check_mark: | :white_check_mark: | :white_check_mark: | :white_check_mark: | +| | [Add PR Documentation](https://github.com/Codium-ai/pr-agent/blob/main/docs/ADD_DOCUMENTATION.md) 💎 | :white_check_mark: | :white_check_mark: | | :white_check_mark: | | | [Custom Labels](https://github.com/Codium-ai/pr-agent/blob/main/docs/DESCRIBE.md#handle-custom-labels-from-the-repos-labels-page-gem) 💎 | :white_check_mark: | :white_check_mark: | | :white_check_mark: | | | [Analyze](https://github.com/Codium-ai/pr-agent/blob/main/docs/Analyze.md) 💎 | :white_check_mark: | :white_check_mark: | | :white_check_mark: | | | [CI Feedback](https://github.com/Codium-ai/pr-agent/blob/main/docs/CI_FEEDBACK.md) 💎 | :white_check_mark: | | | | diff --git a/pr_agent/tools/pr_help_message.py b/pr_agent/tools/pr_help_message.py index 9095a8f1..357a9112 100644 --- a/pr_agent/tools/pr_help_message.py +++ b/pr_agent/tools/pr_help_message.py @@ -9,6 +9,11 @@ class PRHelpMessage: async def run(self): try: + if not self.git_provider.is_supported("gfm_markdown"): + self.git_provider.publish_comment( + "The `Help` tool requires gfm markdown, which is not supported by your code platform.") + return + get_logger().info('Getting PR Help Message...') relevant_configs = {'pr_help': dict(get_settings().pr_help), 'config': dict(get_settings().config)}