From 5e1cc12df4a77ff92872bc898dba1b8e648934ca Mon Sep 17 00:00:00 2001 From: GOOD21 <41340811@qq.com> Date: Thu, 20 Feb 2025 10:47:21 +0800 Subject: [PATCH 1/7] Enhanced webhook handling for push events without object_attributes --- pr_agent/servers/gitlab_webhook.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/pr_agent/servers/gitlab_webhook.py b/pr_agent/servers/gitlab_webhook.py index 6a3d6bcb..d837dae4 100644 --- a/pr_agent/servers/gitlab_webhook.py +++ b/pr_agent/servers/gitlab_webhook.py @@ -24,7 +24,6 @@ router = APIRouter() secret_provider = get_secret_provider() if get_settings().get("CONFIG.SECRET_PROVIDER") else None - async def get_mr_url_from_commit_sha(commit_sha, gitlab_token, project_id): try: import requests @@ -64,7 +63,8 @@ async def _perform_commands_gitlab(commands_conf: str, agent: PRAgent, api_url: if commands_conf == "pr_commands" and get_settings().config.disable_auto_feedback: # auto commands for PR, and auto feedback is disabled get_logger().info(f"Auto feedback is disabled, skipping auto commands for PR {api_url=}", **log_context) return - if not should_process_pr_logic(data): # Here we already updated the configurations + if commands_conf != "push_commands" and not should_process_pr_logic(data): + get_logger().info(f"Skipping auto commands for PR {api_url=}", **log_context) return commands = get_settings().get(f"gitlab.{commands_conf}", {}) get_settings().set("config.is_auto_command", True) @@ -98,6 +98,7 @@ def is_bot_user(data) -> bool: def should_process_pr_logic(data) -> bool: try: if not data.get('object_attributes', {}): + get_logger().info("No object attributes found in the data") return False title = data['object_attributes'].get('title') sender = data.get("user", {}).get("username", "") @@ -190,8 +191,9 @@ async def gitlab_webhook(background_tasks: BackgroundTasks, request: Request): # ignore bot users if is_bot_user(data): return JSONResponse(status_code=status.HTTP_200_OK, content=jsonable_encoder({"message": "success"})) - if data.get('event_type') != 'note': # not a comment + if data.get('event_type') != 'note' and data.get('object_kind') != 'push': # not a comment # ignore MRs based on title, labels, source and target branches + get_logger().info(f"Processing {data.get('object_kind')} event") if not should_process_pr_logic(data): return JSONResponse(status_code=status.HTTP_200_OK, content=jsonable_encoder({"message": "success"})) @@ -246,7 +248,6 @@ async def gitlab_webhook(background_tasks: BackgroundTasks, request: Request): get_logger().info(f"Processing time: {end_time - start_time}", request=request_json) return JSONResponse(status_code=status.HTTP_200_OK, content=jsonable_encoder({"message": "success"})) - def handle_ask_line(body, data): try: line_range_ = data['object_attributes']['position']['line_range'] From 237a6ffb5f4c9abc469a44f1f609786283c0a35b Mon Sep 17 00:00:00 2001 From: Benedict Lee Date: Thu, 20 Feb 2025 11:52:53 +0900 Subject: [PATCH 2/7] fix : existing and improved code fields to clarify formatting requirements --- pr_agent/settings/pr_code_suggestions_prompts.toml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pr_agent/settings/pr_code_suggestions_prompts.toml b/pr_agent/settings/pr_code_suggestions_prompts.toml index 35929522..064ad8c1 100644 --- a/pr_agent/settings/pr_code_suggestions_prompts.toml +++ b/pr_agent/settings/pr_code_suggestions_prompts.toml @@ -84,8 +84,8 @@ class CodeSuggestion(BaseModel): relevant_file: str = Field(description="Full path of the relevant file") language: str = Field(description="Programming language used by the relevant file") suggestion_content: str = Field(description="An actionable suggestion to enhance, improve or fix the new code introduced in the PR. Don't present here actual code snippets, just the suggestion. Be short and concise") - existing_code: str = Field(description="A short code snippet, from a '__new hunk__' section after the PR changes, that the suggestion aims to enhance or fix. Include only complete code lines. Use ellipsis (...) for brevity if needed. This snippet should represent the specific PR code targeted for improvement.") - improved_code: str = Field(description="A refined code snippet that replaces the 'existing_code' snippet after implementing the suggestion.") + existing_code: str = Field(description="A short code snippet, from a '__new hunk__' section after the PR changes, that the suggestion aims to enhance or fix. Include only complete code lines without diff prefixes like '+', '-'. Use ellipsis (...) for brevity if needed. This snippet should represent the specific PR code targeted for improvement.") + improved_code: str = Field(description="A refined code snippet that replaces the 'existing_code' snippet after implementing the suggestion. Include only complete code lines without diff prefixes like '+', '-'.") one_sentence_summary: str = Field(description="A concise, single-sentence overview (up to 6 words) of the suggested improvement. Focus on the 'what'. Be general, and avoid method or variable names.") {%- if not focus_only_on_problems %} label: str = Field(description="A single, descriptive label that best characterizes the suggestion type. Possible labels include 'security', 'possible bug', 'possible issue', 'performance', 'enhancement', 'best practice', 'maintainability', 'typo'. Other relevant labels are also acceptable.") From 347af1dd99305149c8217dbba6daad41b405595a Mon Sep 17 00:00:00 2001 From: GOOD21 <41340811@qq.com> Date: Fri, 21 Feb 2025 13:24:20 +0800 Subject: [PATCH 3/7] use merge_request with oldrev instead of push --- pr_agent/servers/gitlab_webhook.py | 137 +++++++++++++++++------------ 1 file changed, 79 insertions(+), 58 deletions(-) diff --git a/pr_agent/servers/gitlab_webhook.py b/pr_agent/servers/gitlab_webhook.py index d837dae4..fee6710c 100644 --- a/pr_agent/servers/gitlab_webhook.py +++ b/pr_agent/servers/gitlab_webhook.py @@ -24,28 +24,29 @@ router = APIRouter() secret_provider = get_secret_provider() if get_settings().get("CONFIG.SECRET_PROVIDER") else None -async def get_mr_url_from_commit_sha(commit_sha, gitlab_token, project_id): - try: - import requests - headers = { - 'Private-Token': f'{gitlab_token}' - } - # API endpoint to find MRs containing the commit - gitlab_url = get_settings().get("GITLAB.URL", 'https://gitlab.com') - response = requests.get( - f'{gitlab_url}/api/v4/projects/{project_id}/repository/commits/{commit_sha}/merge_requests', - headers=headers - ) - merge_requests = response.json() - if merge_requests and response.status_code == 200: - pr_url = merge_requests[0]['web_url'] - return pr_url - else: - get_logger().info(f"No merge requests found for commit: {commit_sha}") - return None - except Exception as e: - get_logger().error(f"Failed to get MR url from commit sha: {e}") - return None + +#async def get_mr_url_from_commit_sha(commit_sha, gitlab_token, project_id): +# try: +# import requests +# headers = { +# 'Private-Token': f'{gitlab_token}' +# } +# # API endpoint to find MRs containing the commit +# gitlab_url = get_settings().get("GITLAB.URL", 'https://gitlab.com') +# response = requests.get( +# f'{gitlab_url}/api/v4/projects/{project_id}/repository/commits/{commit_sha}/merge_requests', +# headers=headers +# ) +# merge_requests = response.json() +# if merge_requests and response.status_code == 200: +# pr_url = merge_requests[0]['web_url'] +# return pr_url +# else: +# get_logger().info(f"No merge requests found for commit: {commit_sha}") +# return None +# except Exception as e: +# get_logger().error(f"Failed to get MR url from commit sha: {e}") +# return None async def handle_request(api_url: str, body: str, log_context: dict, sender_id: str): log_context["action"] = body @@ -63,8 +64,7 @@ async def _perform_commands_gitlab(commands_conf: str, agent: PRAgent, api_url: if commands_conf == "pr_commands" and get_settings().config.disable_auto_feedback: # auto commands for PR, and auto feedback is disabled get_logger().info(f"Auto feedback is disabled, skipping auto commands for PR {api_url=}", **log_context) return - if commands_conf != "push_commands" and not should_process_pr_logic(data): - get_logger().info(f"Skipping auto commands for PR {api_url=}", **log_context) + if not should_process_pr_logic(data): # Here we already updated the configurations return commands = get_settings().get(f"gitlab.{commands_conf}", {}) get_settings().set("config.is_auto_command", True) @@ -98,7 +98,6 @@ def is_bot_user(data) -> bool: def should_process_pr_logic(data) -> bool: try: if not data.get('object_attributes', {}): - get_logger().info("No object attributes found in the data") return False title = data['object_attributes'].get('title') sender = data.get("user", {}).get("username", "") @@ -191,23 +190,44 @@ async def gitlab_webhook(background_tasks: BackgroundTasks, request: Request): # ignore bot users if is_bot_user(data): return JSONResponse(status_code=status.HTTP_200_OK, content=jsonable_encoder({"message": "success"})) - if data.get('event_type') != 'note' and data.get('object_kind') != 'push': # not a comment + + log_context["sender"] = sender + if data.get('object_kind') == 'merge_request': # ignore MRs based on title, labels, source and target branches - get_logger().info(f"Processing {data.get('object_kind')} event") if not should_process_pr_logic(data): return JSONResponse(status_code=status.HTTP_200_OK, content=jsonable_encoder({"message": "success"})) - log_context["sender"] = sender - if data.get('object_kind') == 'merge_request' and data['object_attributes'].get('action') in ['open', 'reopen']: - title = data['object_attributes'].get('title') - url = data['object_attributes'].get('url') - draft = data['object_attributes'].get('draft') - get_logger().info(f"New merge request: {url}") - if draft: - get_logger().info(f"Skipping draft MR: {url}") - return JSONResponse(status_code=status.HTTP_200_OK, content=jsonable_encoder({"message": "success"})) + if data['object_attributes'].get('action') in ['open', 'reopen']: + url = data['object_attributes'].get('url') + draft = data['object_attributes'].get('draft') + get_logger().info(f"New merge request: {url}") + if draft: + get_logger().info(f"Skipping draft MR: {url}") + return JSONResponse(status_code=status.HTTP_200_OK, content=jsonable_encoder({"message": "success"})) + + await _perform_commands_gitlab("pr_commands", PRAgent(), url, log_context, data) + + # for push event triggered merge requests + elif data['object_attributes'].get('action') == 'update' and data['object_attributes'].get('oldrev'): + url = data['object_attributes'].get('url') + draft = data['object_attributes'].get('draft') + get_logger().info(f"New merge request: {url}") + if draft: + get_logger().info(f"Skipping draft MR: {url}") + return JSONResponse(status_code=status.HTTP_200_OK, content=jsonable_encoder({"message": "success"})) + + # we need first to apply_repo_settings + # apply_repo_settings(url) + commands_on_push = get_settings().get(f"gitlab.push_commands", {}) + handle_push_trigger = get_settings().get(f"gitlab.handle_push_trigger", False) + if not commands_on_push or not handle_push_trigger: + get_logger().info("Push event, but no push commands found or push trigger is disabled") + return JSONResponse(status_code=status.HTTP_200_OK, + content=jsonable_encoder({"message": "success"})) + + get_logger().debug(f'A push event has been received: {url}') + await _perform_commands_gitlab("push_commands", PRAgent(), url, log_context, data) - await _perform_commands_gitlab("pr_commands", PRAgent(), url, log_context, data) elif data.get('object_kind') == 'note' and data.get('event_type') == 'note': # comment on MR if 'merge_request' in data: mr = data['merge_request'] @@ -219,35 +239,36 @@ async def gitlab_webhook(background_tasks: BackgroundTasks, request: Request): body = handle_ask_line(body, data) await handle_request(url, body, log_context, sender_id) - elif data.get('object_kind') == 'push' and data.get('event_name') == 'push': - try: - project_id = data['project_id'] - commit_sha = data['checkout_sha'] - url = await get_mr_url_from_commit_sha(commit_sha, gitlab_token, project_id) - if not url: - get_logger().info(f"No MR found for commit: {commit_sha}") - return JSONResponse(status_code=status.HTTP_200_OK, - content=jsonable_encoder({"message": "success"})) + # elif data.get('object_kind') == 'push' and data.get('event_name') == 'push': + # try: + # project_id = data['project_id'] + # commit_sha = data['checkout_sha'] + # url = await get_mr_url_from_commit_sha(commit_sha, gitlab_token, project_id) + # if not url: + # get_logger().info(f"No MR found for commit: {commit_sha}") + # return JSONResponse(status_code=status.HTTP_200_OK, + # content=jsonable_encoder({"message": "success"})) - # we need first to apply_repo_settings - apply_repo_settings(url) - commands_on_push = get_settings().get(f"gitlab.push_commands", {}) - handle_push_trigger = get_settings().get(f"gitlab.handle_push_trigger", False) - if not commands_on_push or not handle_push_trigger: - get_logger().info("Push event, but no push commands found or push trigger is disabled") - return JSONResponse(status_code=status.HTTP_200_OK, - content=jsonable_encoder({"message": "success"})) + # # we need first to apply_repo_settings + # apply_repo_settings(url) + # commands_on_push = get_settings().get(f"gitlab.push_commands", {}) + # handle_push_trigger = get_settings().get(f"gitlab.handle_push_trigger", False) + # if not commands_on_push or not handle_push_trigger: + # get_logger().info("Push event, but no push commands found or push trigger is disabled") + # return JSONResponse(status_code=status.HTTP_200_OK, + # content=jsonable_encoder({"message": "success"})) - get_logger().debug(f'A push event has been received: {url}') - await _perform_commands_gitlab("push_commands", PRAgent(), url, log_context, data) - except Exception as e: - get_logger().error(f"Failed to handle push event: {e}") + # get_logger().debug(f'A push event has been received: {url}') + # await _perform_commands_gitlab("push_commands", PRAgent(), url, log_context, data) + # except Exception as e: + # get_logger().error(f"Failed to handle push event: {e}") background_tasks.add_task(inner, request_json) end_time = datetime.now() get_logger().info(f"Processing time: {end_time - start_time}", request=request_json) return JSONResponse(status_code=status.HTTP_200_OK, content=jsonable_encoder({"message": "success"})) + def handle_ask_line(body, data): try: line_range_ = data['object_attributes']['position']['line_range'] From f143a248794f597f39e190d18f1b7eb988c94b0d Mon Sep 17 00:00:00 2001 From: GOOD21 <41340811@qq.com> Date: Fri, 21 Feb 2025 13:35:48 +0800 Subject: [PATCH 4/7] remove unnecessary code --- pr_agent/servers/gitlab_webhook.py | 48 ------------------------------ 1 file changed, 48 deletions(-) diff --git a/pr_agent/servers/gitlab_webhook.py b/pr_agent/servers/gitlab_webhook.py index fee6710c..b05ddbeb 100644 --- a/pr_agent/servers/gitlab_webhook.py +++ b/pr_agent/servers/gitlab_webhook.py @@ -25,29 +25,6 @@ router = APIRouter() secret_provider = get_secret_provider() if get_settings().get("CONFIG.SECRET_PROVIDER") else None -#async def get_mr_url_from_commit_sha(commit_sha, gitlab_token, project_id): -# try: -# import requests -# headers = { -# 'Private-Token': f'{gitlab_token}' -# } -# # API endpoint to find MRs containing the commit -# gitlab_url = get_settings().get("GITLAB.URL", 'https://gitlab.com') -# response = requests.get( -# f'{gitlab_url}/api/v4/projects/{project_id}/repository/commits/{commit_sha}/merge_requests', -# headers=headers -# ) -# merge_requests = response.json() -# if merge_requests and response.status_code == 200: -# pr_url = merge_requests[0]['web_url'] -# return pr_url -# else: -# get_logger().info(f"No merge requests found for commit: {commit_sha}") -# return None -# except Exception as e: -# get_logger().error(f"Failed to get MR url from commit sha: {e}") -# return None - async def handle_request(api_url: str, body: str, log_context: dict, sender_id: str): log_context["action"] = body log_context["event"] = "pull_request" if body == "/review" else "comment" @@ -216,8 +193,6 @@ async def gitlab_webhook(background_tasks: BackgroundTasks, request: Request): get_logger().info(f"Skipping draft MR: {url}") return JSONResponse(status_code=status.HTTP_200_OK, content=jsonable_encoder({"message": "success"})) - # we need first to apply_repo_settings - # apply_repo_settings(url) commands_on_push = get_settings().get(f"gitlab.push_commands", {}) handle_push_trigger = get_settings().get(f"gitlab.handle_push_trigger", False) if not commands_on_push or not handle_push_trigger: @@ -239,29 +214,6 @@ async def gitlab_webhook(background_tasks: BackgroundTasks, request: Request): body = handle_ask_line(body, data) await handle_request(url, body, log_context, sender_id) - # elif data.get('object_kind') == 'push' and data.get('event_name') == 'push': - # try: - # project_id = data['project_id'] - # commit_sha = data['checkout_sha'] - # url = await get_mr_url_from_commit_sha(commit_sha, gitlab_token, project_id) - # if not url: - # get_logger().info(f"No MR found for commit: {commit_sha}") - # return JSONResponse(status_code=status.HTTP_200_OK, - # content=jsonable_encoder({"message": "success"})) - - # # we need first to apply_repo_settings - # apply_repo_settings(url) - # commands_on_push = get_settings().get(f"gitlab.push_commands", {}) - # handle_push_trigger = get_settings().get(f"gitlab.handle_push_trigger", False) - # if not commands_on_push or not handle_push_trigger: - # get_logger().info("Push event, but no push commands found or push trigger is disabled") - # return JSONResponse(status_code=status.HTTP_200_OK, - # content=jsonable_encoder({"message": "success"})) - - # get_logger().debug(f'A push event has been received: {url}') - # await _perform_commands_gitlab("push_commands", PRAgent(), url, log_context, data) - # except Exception as e: - # get_logger().error(f"Failed to handle push event: {e}") background_tasks.add_task(inner, request_json) end_time = datetime.now() From feb306727e9e9bd3947171cd0da09f0737a8d654 Mon Sep 17 00:00:00 2001 From: Benedict Lee Date: Mon, 24 Feb 2025 09:15:00 +0900 Subject: [PATCH 5/7] fix : refine handling of leading '+' in response text --- pr_agent/algo/utils.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pr_agent/algo/utils.py b/pr_agent/algo/utils.py index 15b33068..e6365c8f 100644 --- a/pr_agent/algo/utils.py +++ b/pr_agent/algo/utils.py @@ -782,7 +782,8 @@ def try_fix_yaml(response_text: str, # fifth fallback - try to remove leading '+' (sometimes added by AI for 'existing code' and 'improved code') response_text_lines_copy = response_text_lines.copy() for i in range(0, len(response_text_lines_copy)): - response_text_lines_copy[i] = ' ' + response_text_lines_copy[i][1:] + if response_text_lines_copy[i].startswith('+'): + response_text_lines_copy[i] = ' ' + response_text_lines_copy[i][1:] try: data = yaml.safe_load('\n'.join(response_text_lines_copy)) get_logger().info(f"Successfully parsed AI prediction after removing leading '+'") From 05960f2c3fe4031715bbf9b7562ebad2f98d21e2 Mon Sep 17 00:00:00 2001 From: Benedict Lee Date: Mon, 24 Feb 2025 09:15:51 +0900 Subject: [PATCH 6/7] rollback : pr_code_suggestions_prompts.toml --- pr_agent/settings/pr_code_suggestions_prompts.toml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pr_agent/settings/pr_code_suggestions_prompts.toml b/pr_agent/settings/pr_code_suggestions_prompts.toml index 064ad8c1..61c74058 100644 --- a/pr_agent/settings/pr_code_suggestions_prompts.toml +++ b/pr_agent/settings/pr_code_suggestions_prompts.toml @@ -84,8 +84,8 @@ class CodeSuggestion(BaseModel): relevant_file: str = Field(description="Full path of the relevant file") language: str = Field(description="Programming language used by the relevant file") suggestion_content: str = Field(description="An actionable suggestion to enhance, improve or fix the new code introduced in the PR. Don't present here actual code snippets, just the suggestion. Be short and concise") - existing_code: str = Field(description="A short code snippet, from a '__new hunk__' section after the PR changes, that the suggestion aims to enhance or fix. Include only complete code lines without diff prefixes like '+', '-'. Use ellipsis (...) for brevity if needed. This snippet should represent the specific PR code targeted for improvement.") - improved_code: str = Field(description="A refined code snippet that replaces the 'existing_code' snippet after implementing the suggestion. Include only complete code lines without diff prefixes like '+', '-'.") + existing_code: str = Field(description="A short code snippet, from a '__new hunk__' section after the PR changes, that the suggestion aims to enhance or fix. Include only complete code lines. Use ellipsis (...) for brevity if needed. This snippet should represent the specific PR code targeted for improvement.") + improved_code: str = Field(description="A refined code snippet that replaces the 'existing_code' snippet after implementing the suggestion. Include only complete code lines.") one_sentence_summary: str = Field(description="A concise, single-sentence overview (up to 6 words) of the suggested improvement. Focus on the 'what'. Be general, and avoid method or variable names.") {%- if not focus_only_on_problems %} label: str = Field(description="A single, descriptive label that best characterizes the suggestion type. Possible labels include 'security', 'possible bug', 'possible issue', 'performance', 'enhancement', 'best practice', 'maintainability', 'typo'. Other relevant labels are also acceptable.") From 7b1df82c05bdefb7a69a6837112defc39f34868a Mon Sep 17 00:00:00 2001 From: Benedict Lee Date: Mon, 24 Feb 2025 09:30:00 +0900 Subject: [PATCH 7/7] rollback : pr_code_suggestions_prompts.toml --- pr_agent/settings/pr_code_suggestions_prompts.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pr_agent/settings/pr_code_suggestions_prompts.toml b/pr_agent/settings/pr_code_suggestions_prompts.toml index 61c74058..35929522 100644 --- a/pr_agent/settings/pr_code_suggestions_prompts.toml +++ b/pr_agent/settings/pr_code_suggestions_prompts.toml @@ -85,7 +85,7 @@ class CodeSuggestion(BaseModel): language: str = Field(description="Programming language used by the relevant file") suggestion_content: str = Field(description="An actionable suggestion to enhance, improve or fix the new code introduced in the PR. Don't present here actual code snippets, just the suggestion. Be short and concise") existing_code: str = Field(description="A short code snippet, from a '__new hunk__' section after the PR changes, that the suggestion aims to enhance or fix. Include only complete code lines. Use ellipsis (...) for brevity if needed. This snippet should represent the specific PR code targeted for improvement.") - improved_code: str = Field(description="A refined code snippet that replaces the 'existing_code' snippet after implementing the suggestion. Include only complete code lines.") + improved_code: str = Field(description="A refined code snippet that replaces the 'existing_code' snippet after implementing the suggestion.") one_sentence_summary: str = Field(description="A concise, single-sentence overview (up to 6 words) of the suggested improvement. Focus on the 'what'. Be general, and avoid method or variable names.") {%- if not focus_only_on_problems %} label: str = Field(description="A single, descriptive label that best characterizes the suggestion type. Possible labels include 'security', 'possible bug', 'possible issue', 'performance', 'enhancement', 'best practice', 'maintainability', 'typo'. Other relevant labels are also acceptable.")