diff --git a/openhands/resolver/README.md b/openhands/resolver/README.md index a43470f901ae..390c78309abb 100644 --- a/openhands/resolver/README.md +++ b/openhands/resolver/README.md @@ -168,13 +168,13 @@ There are three ways you can upload: 3. `ready` - create a non-draft PR that's ready for review ```bash -python -m openhands.resolver.send_pull_request --issue-number ISSUE_NUMBER --github-username YOUR_GITHUB_USERNAME --pr-type draft +python -m openhands.resolver.send_pull_request --issue-number ISSUE_NUMBER --username YOUR_GITHUB_USERNAME --pr-type draft ``` If you want to upload to a fork, you can do so by specifying the `fork-owner`: ```bash -python -m openhands.resolver.send_pull_request --issue-number ISSUE_NUMBER --github-username YOUR_GITHUB_USERNAME --pr-type draft --fork-owner YOUR_GITHUB_USERNAME +python -m openhands.resolver.send_pull_request --issue-number ISSUE_NUMBER --username YOUR_GITHUB_USERNAME --pr-type draft --fork-owner YOUR_GITHUB_USERNAME ``` ## Providing Custom Instructions diff --git a/openhands/resolver/github.py b/openhands/resolver/github.py new file mode 100644 index 000000000000..c8645da611d2 --- /dev/null +++ b/openhands/resolver/github.py @@ -0,0 +1,481 @@ +from abc import ABC, abstractmethod +from typing import Any + +import requests + +from openhands.core.logger import openhands_logger as logger +from openhands.resolver.issue import ReviewThread +from openhands.resolver.utils import extract_issue_references + + +class IssueHandlerInterface(ABC): + @abstractmethod + def download_issues(self) -> list[Any]: + pass + + @abstractmethod + def get_issue_comments( + self, issue_number: int, comment_id: int | None = None + ) -> list[str] | None: + pass + + @abstractmethod + def get_base_url(self): + pass + + @abstractmethod + def get_branch_url(self, branch_name): + pass + + @abstractmethod + def get_download_url(self): + pass + + @abstractmethod + def get_clone_url(self): + pass + + @abstractmethod + def get_graphql_url(self): + pass + + @abstractmethod + def get_headers(self): + pass + + @abstractmethod + def get_compare_url(self, branch_name): + pass + + @abstractmethod + def get_branch_name(self, base_branch_name: str): + pass + + @abstractmethod + def branch_exists(self, branch_name: str) -> bool: + pass + + @abstractmethod + def reply_to_comment(self, token: str, comment_id: str, reply: str): + pass + + @abstractmethod + def get_authorize_url(self): + pass + + @abstractmethod + def create_pull_request(self, data=dict) -> str: + pass + + +class GithubIssueHandler(IssueHandlerInterface): + def __init__(self, owner: str, repo: str, token: str, username: str | None = None): + self.owner = owner + self.repo = repo + self.token = token + self.username = username + self.base_url = self.get_base_url() + self.download_url = self.get_download_url() + self.clone_url = self.get_clone_url() + self.headers = self.get_headers() + + def get_headers(self): + return { + 'Authorization': f'token {self.token}', + 'Accept': 'application/vnd.github.v3+json', + } + + def get_base_url(self): + return f'https://api.github.com/repos/{self.owner}/{self.repo}' + + def get_authorize_url(self): + return f'https://{self.username}:{self.token}@github.com/' + + def get_branch_url(self, branch_name: str): + return self.get_base_url() + f'/branches/{branch_name}' + + def get_download_url(self): + return f'{self.base_url}/issues' + + def get_clone_url(self): + username_and_token = ( + f'{self.username}:{self.token}' + if self.username + else f'x-auth-token:{self.token}' + ) + return f'https://{username_and_token}@github.com/{self.owner}/{self.repo}.git' + + def get_graphql_url(self): + return self.get_base_url() + '/graphql' + + def get_compare_url(self, branch_name: str): + return f'https://github.com/{self.owner}/{self.repo}/compare/{branch_name}?expand=1' + + def download_issues(self) -> list[Any]: + params: dict[str, int | str] = {'state': 'open', 'per_page': 100, 'page': 1} + all_issues = [] + + while True: + response = requests.get( + self.download_url, headers=self.headers, params=params + ) + response.raise_for_status() + issues = response.json() + + if not issues: + break + + if not isinstance(issues, list) or any( + [not isinstance(issue, dict) for issue in issues] + ): + raise ValueError( + 'Expected list of dictionaries from Service Github API.' + ) + + all_issues.extend(issues) + assert isinstance(params['page'], int) + params['page'] += 1 + + return all_issues + + def get_issue_comments( + self, issue_number: int, comment_id: int | None = None + ) -> list[str] | None: + """Download comments for a specific issue from Github.""" + url = f'{self.download_url}/{issue_number}/comments' + params = {'per_page': 100, 'page': 1} + all_comments = [] + + while True: + response = requests.get(url, headers=self.headers, params=params) + response.raise_for_status() + comments = response.json() + + if not comments: + break + + if comment_id: + matching_comment = next( + ( + comment['body'] + for comment in comments + if comment['id'] == comment_id + ), + None, + ) + if matching_comment: + return [matching_comment] + else: + all_comments.extend([comment['body'] for comment in comments]) + + params['page'] += 1 + + return all_comments if all_comments else None + + def branch_exists(self, branch_name: str) -> bool: + print(f'Checking if branch {branch_name} exists...') + response = requests.get( + f'{self.base_url}/branches/{branch_name}', headers=self.headers + ) + exists = response.status_code == 200 + print(f'Branch {branch_name} exists: {exists}') + return exists + + def get_branch_name(self, base_branch_name: str): + branch_name = base_branch_name + attempt = 1 + while self.branch_exists(branch_name): + attempt += 1 + branch_name = f'{base_branch_name}-try{attempt}' + return branch_name + + def reply_to_comment(self, token: str, comment_id: str, reply: str): + # Opting for graphql as REST API doesn't allow reply to replies in comment threads + query = """ + mutation($body: String!, $pullRequestReviewThreadId: ID!) { + addPullRequestReviewThreadReply(input: { body: $body, pullRequestReviewThreadId: $pullRequestReviewThreadId }) { + comment { + id + body + createdAt + } + } + } + """ + + comment_reply = f'Openhands fix success summary\n\n\n{reply}' + variables = {'body': comment_reply, 'pullRequestReviewThreadId': comment_id} + url = self.get_base_url() + headers = { + 'Authorization': f'Bearer {token}', + 'Content-Type': 'application/json', + } + + response = requests.post( + url, json={'query': query, 'variables': variables}, headers=headers + ) + response.raise_for_status() + + def get_pull_url(self, pr_number: int): + return f'https://github.com/{self.owner}/{self.repo}/pull/{pr_number}' + + def get_default_branch_name(self) -> str: + response = requests.get(f'{self.base_url}', headers=self.headers) + response.raise_for_status() + return response.json()['default_branch'] + + def create_pull_request(self, data=dict) -> str: + response = requests.post( + f'{self.base_url}/pulls', headers=self.headers, json=data + ) + if response.status_code == 403: + raise RuntimeError( + 'Failed to create pull request due to missing permissions. ' + 'Make sure that the provided token has push permissions for the repository.' + ) + response.raise_for_status() + pr_data = response.json() + return pr_data['html_url'] + + +class GithubPRHandler(GithubIssueHandler): + def __init__(self, owner: str, repo: str, token: str): + super().__init__(owner, repo, token) + self.download_url = 'https://api.github.com/repos/{}/{}/pulls' + + def download_pr_metadata( + self, pull_number: int, comment_id: int | None = None + ) -> tuple[list[str], list[int], list[str], list[ReviewThread], list[str]]: + """Run a GraphQL query against the GitHub API for information. + + Retrieves information about: + 1. unresolved review comments + 2. referenced issues the pull request would close + + Args: + pull_number: The number of the pull request to query. + comment_id: Optional ID of a specific comment to focus on. + query: The GraphQL query as a string. + variables: A dictionary of variables for the query. + token: Your GitHub personal access token. + + Returns: + The JSON response from the GitHub API. + """ + # Using graphql as REST API doesn't indicate resolved status for review comments + # TODO: grabbing the first 10 issues, 100 review threads, and 100 coments; add pagination to retrieve all + query = """ + query($owner: String!, $repo: String!, $pr: Int!) { + repository(owner: $owner, name: $repo) { + pullRequest(number: $pr) { + closingIssuesReferences(first: 10) { + edges { + node { + body + number + } + } + } + url + reviews(first: 100) { + nodes { + body + state + fullDatabaseId + } + } + reviewThreads(first: 100) { + edges{ + node{ + id + isResolved + comments(first: 100) { + totalCount + nodes { + body + path + fullDatabaseId + } + } + } + } + } + } + } + } + """ + + variables = {'owner': self.owner, 'repo': self.repo, 'pr': pull_number} + + url = 'https://api.github.com/graphql' + headers = { + 'Authorization': f'Bearer {self.token}', + 'Content-Type': 'application/json', + } + + response = requests.post( + url, json={'query': query, 'variables': variables}, headers=headers + ) + response.raise_for_status() + response_json = response.json() + + # Parse the response to get closing issue references and unresolved review comments + pr_data = ( + response_json.get('data', {}).get('repository', {}).get('pullRequest', {}) + ) + + # Get closing issues + closing_issues = pr_data.get('closingIssuesReferences', {}).get('edges', []) + closing_issues_bodies = [issue['node']['body'] for issue in closing_issues] + closing_issue_numbers = [ + issue['node']['number'] for issue in closing_issues + ] # Extract issue numbers + + # Get review comments + reviews = pr_data.get('reviews', {}).get('nodes', []) + if comment_id is not None: + reviews = [ + review + for review in reviews + if int(review['fullDatabaseId']) == comment_id + ] + review_bodies = [review['body'] for review in reviews] + + # Get unresolved review threads + review_threads = [] + thread_ids = [] # Store thread IDs; agent replies to the thread + raw_review_threads = pr_data.get('reviewThreads', {}).get('edges', []) + for thread in raw_review_threads: + node = thread.get('node', {}) + if not node.get( + 'isResolved', True + ): # Check if the review thread is unresolved + id = node.get('id') + thread_contains_comment_id = False + my_review_threads = node.get('comments', {}).get('nodes', []) + message = '' + files = [] + for i, review_thread in enumerate(my_review_threads): + if ( + comment_id is not None + and int(review_thread['fullDatabaseId']) == comment_id + ): + thread_contains_comment_id = True + + if ( + i == len(my_review_threads) - 1 + ): # Check if it's the last thread in the thread + if len(my_review_threads) > 1: + message += '---\n' # Add "---" before the last message if there's more than one thread + message += 'latest feedback:\n' + review_thread['body'] + '\n' + else: + message += ( + review_thread['body'] + '\n' + ) # Add each thread in a new line + + file = review_thread.get('path') + if file and file not in files: + files.append(file) + + if comment_id is None or thread_contains_comment_id: + unresolved_thread = ReviewThread(comment=message, files=files) + review_threads.append(unresolved_thread) + thread_ids.append(id) + + return ( + closing_issues_bodies, + closing_issue_numbers, + review_bodies, + review_threads, + thread_ids, + ) + + # Override processing of downloaded issues + def get_pr_comments( + self, pr_number: int, comment_id: int | None = None + ) -> list[str] | None: + """Download comments for a specific pull request from Github.""" + url = f'https://api.github.com/repos/{self.owner}/{self.repo}/issues/{pr_number}/comments' + headers = { + 'Authorization': f'token {self.token}', + 'Accept': 'application/vnd.github.v3+json', + } + params = {'per_page': 100, 'page': 1} + all_comments = [] + + while True: + response = requests.get(url, headers=headers, params=params) + response.raise_for_status() + comments = response.json() + + if not comments: + break + + if comment_id is not None: + matching_comment = next( + ( + comment['body'] + for comment in comments + if comment['id'] == comment_id + ), + None, + ) + if matching_comment: + return [matching_comment] + else: + all_comments.extend([comment['body'] for comment in comments]) + + params['page'] += 1 + + return all_comments if all_comments else None + + def get_context_from_external_issues_references( + self, + closing_issues: list[str], + closing_issue_numbers: list[int], + issue_body: str, + review_comments: list[str], + review_threads: list[ReviewThread], + thread_comments: list[str] | None, + ): + new_issue_references = [] + + if issue_body: + new_issue_references.extend(extract_issue_references(issue_body)) + + if review_comments: + for comment in review_comments: + new_issue_references.extend(extract_issue_references(comment)) + + if review_threads: + for review_thread in review_threads: + new_issue_references.extend( + extract_issue_references(review_thread.comment) + ) + + if thread_comments: + for thread_comment in thread_comments: + new_issue_references.extend(extract_issue_references(thread_comment)) + + non_duplicate_references = set(new_issue_references) + unique_issue_references = non_duplicate_references.difference( + closing_issue_numbers + ) + + for issue_number in unique_issue_references: + try: + url = f'https://api.github.com/repos/{self.owner}/{self.repo}/issues/{issue_number}' + headers = { + 'Authorization': f'Bearer {self.token}', + 'Accept': 'application/vnd.github.v3+json', + } + response = requests.get(url, headers=headers) + response.raise_for_status() + issue_data = response.json() + issue_body = issue_data.get('body', '') + if issue_body: + closing_issues.append(issue_body) + except requests.exceptions.RequestException as e: + logger.warning(f'Failed to fetch issue {issue_number}: {str(e)}') + + return closing_issues diff --git a/openhands/resolver/github_issue.py b/openhands/resolver/issue.py similarity index 94% rename from openhands/resolver/github_issue.py rename to openhands/resolver/issue.py index 9b8f58d589a4..a111d5430241 100644 --- a/openhands/resolver/github_issue.py +++ b/openhands/resolver/issue.py @@ -6,7 +6,7 @@ class ReviewThread(BaseModel): files: list[str] -class GithubIssue(BaseModel): +class Issue(BaseModel): owner: str repo: str number: int diff --git a/openhands/resolver/issue_definitions.py b/openhands/resolver/issue_definitions.py index 66e3036ffa62..cfe4ca1b1fc5 100644 --- a/openhands/resolver/issue_definitions.py +++ b/openhands/resolver/issue_definitions.py @@ -1,515 +1,97 @@ import json import os import re -from abc import ABC, abstractmethod from typing import Any, ClassVar import jinja2 -import requests from openhands.core.config import LLMConfig from openhands.core.logger import openhands_logger as logger from openhands.events.event import Event from openhands.llm.llm import LLM -from openhands.resolver.github_issue import GithubIssue, ReviewThread +from openhands.resolver.issue import Issue, ReviewThread +from openhands.resolver.utils import extract_image_urls -class IssueHandlerInterface(ABC): - issue_type: ClassVar[str] - llm: LLM - - @abstractmethod - def get_converted_issues( - self, issue_numbers: list[int] | None = None, comment_id: int | None = None - ) -> list[GithubIssue]: - """Download issues from GitHub.""" - pass - - @abstractmethod - def get_instruction( - self, - issue: GithubIssue, - prompt_template: str, - repo_instruction: str | None = None, - ) -> tuple[str, list[str]]: - """Generate instruction and image urls for the agent.""" - pass - - @abstractmethod - def guess_success( - self, issue: GithubIssue, history: list[Event] - ) -> tuple[bool, list[bool] | None, str]: - """Guess if the issue has been resolved based on the agent's output.""" - pass - - -class IssueHandler(IssueHandlerInterface): - issue_type: ClassVar[str] = 'issue' +# Strategy context interface +class ServiceContextPR: + issue_type: ClassVar[str] = 'pr' - def __init__(self, owner: str, repo: str, token: str, llm_config: LLMConfig): - self.download_url = 'https://api.github.com/repos/{}/{}/issues' - self.owner = owner - self.repo = repo - self.token = token + def __init__(self, strategy, llm_config: LLMConfig): + self._strategy = strategy self.llm = LLM(llm_config) - def _download_issues_from_github(self) -> list[Any]: - url = self.download_url.format(self.owner, self.repo) - headers = { - 'Authorization': f'token {self.token}', - 'Accept': 'application/vnd.github.v3+json', - } - params: dict[str, int | str] = {'state': 'open', 'per_page': 100, 'page': 1} - all_issues = [] - - while True: - response = requests.get(url, headers=headers, params=params) - response.raise_for_status() - issues = response.json() - - if not issues: - break - - if not isinstance(issues, list) or any( - [not isinstance(issue, dict) for issue in issues] - ): - raise ValueError('Expected list of dictionaries from Github API.') - - all_issues.extend(issues) - assert isinstance(params['page'], int) - params['page'] += 1 - - return all_issues - - def _extract_image_urls(self, issue_body: str) -> list[str]: - # Regular expression to match Markdown image syntax ![alt text](image_url) - image_pattern = r'!\[.*?\]\((https?://[^\s)]+)\)' - return re.findall(image_pattern, issue_body) - - def _extract_issue_references(self, body: str) -> list[int]: - # First, remove code blocks as they may contain false positives - body = re.sub(r'```.*?```', '', body, flags=re.DOTALL) - - # Remove inline code - body = re.sub(r'`[^`]*`', '', body) - - # Remove URLs that contain hash symbols - body = re.sub(r'https?://[^\s)]*#\d+[^\s)]*', '', body) - - # Now extract issue numbers, making sure they're not part of other text - # The pattern matches #number that: - # 1. Is at the start of text or after whitespace/punctuation - # 2. Is followed by whitespace, punctuation, or end of text - # 3. Is not part of a URL - pattern = r'(?:^|[\s\[({]|[^\w#])#(\d+)(?=[\s,.\])}]|$)' - return [int(match) for match in re.findall(pattern, body)] - - def _get_issue_comments( - self, issue_number: int, comment_id: int | None = None - ) -> list[str] | None: - """Download comments for a specific issue from Github.""" - url = f'https://api.github.com/repos/{self.owner}/{self.repo}/issues/{issue_number}/comments' - headers = { - 'Authorization': f'token {self.token}', - 'Accept': 'application/vnd.github.v3+json', - } - params = {'per_page': 100, 'page': 1} - all_comments = [] - - while True: - response = requests.get(url, headers=headers, params=params) - response.raise_for_status() - comments = response.json() - - if not comments: - break - - if comment_id: - matching_comment = next( - ( - comment['body'] - for comment in comments - if comment['id'] == comment_id - ), - None, - ) - if matching_comment: - return [matching_comment] - else: - all_comments.extend([comment['body'] for comment in comments]) - - params['page'] += 1 - - return all_comments if all_comments else None - - def get_converted_issues( - self, issue_numbers: list[int] | None = None, comment_id: int | None = None - ) -> list[GithubIssue]: - """Download issues from Github. - - Returns: - List of Github issues. - """ + def set_strategy(self, strategy): + self._strategy = strategy - if not issue_numbers: - raise ValueError('Unspecified issue number') + def get_clone_url(self): + return self._strategy.get_clone_url() - all_issues = self._download_issues_from_github() - logger.info(f'Limiting resolving to issues {issue_numbers}.') - all_issues = [ - issue - for issue in all_issues - if issue['number'] in issue_numbers and 'pull_request' not in issue - ] - - if len(issue_numbers) == 1 and not all_issues: - raise ValueError(f'Issue {issue_numbers[0]} not found') - - converted_issues = [] - for issue in all_issues: - # Check for required fields (number and title) - if any([issue.get(key) is None for key in ['number', 'title']]): - logger.warning( - f'Skipping issue {issue} as it is missing number or title.' - ) - continue - - # Handle empty body by using empty string - if issue.get('body') is None: - issue['body'] = '' - - # Get issue thread comments - thread_comments = self._get_issue_comments( - issue['number'], comment_id=comment_id - ) - # Convert empty lists to None for optional fields - issue_details = GithubIssue( - owner=self.owner, - repo=self.repo, - number=issue['number'], - title=issue['title'], - body=issue['body'], - thread_comments=thread_comments, - review_comments=None, # Initialize review comments as None for regular issues - ) - - converted_issues.append(issue_details) - - return converted_issues - - def get_instruction( - self, - issue: GithubIssue, - prompt_template: str, - repo_instruction: str | None = None, - ) -> tuple[str, list[str]]: - """Generate instruction for the agent.""" - # Format thread comments if they exist - thread_context = '' - if issue.thread_comments: - thread_context = '\n\nIssue Thread Comments:\n' + '\n---\n'.join( - issue.thread_comments - ) - - images = [] - images.extend(self._extract_image_urls(issue.body)) - images.extend(self._extract_image_urls(thread_context)) - - template = jinja2.Template(prompt_template) - return ( - template.render( - body=issue.title + '\n\n' + issue.body + thread_context, - repo_instruction=repo_instruction, - ), - images, - ) + def download_issues(self) -> list[Any]: + return self._strategy.download_issues() def guess_success( - self, issue: GithubIssue, history: list[Event] + self, issue: Issue, history: list[Event] ) -> tuple[bool, None | list[bool], str]: """Guess if the issue is fixed based on the history and the issue description.""" last_message = history[-1].message - # Include thread comments in the prompt if they exist - issue_context = issue.body - if issue.thread_comments: - issue_context += '\n\nIssue Thread Comments:\n' + '\n---\n'.join( - issue.thread_comments - ) - - with open( - os.path.join( - os.path.dirname(__file__), - 'prompts/guess_success/issue-success-check.jinja', - ), - 'r', - ) as f: - template = jinja2.Template(f.read()) - prompt = template.render(issue_context=issue_context, last_message=last_message) - - response = self.llm.completion(messages=[{'role': 'user', 'content': prompt}]) - - answer = response.choices[0].message.content.strip() - pattern = r'--- success\n*(true|false)\n*--- explanation*\n((?:.|\n)*)' - match = re.search(pattern, answer) - if match: - return match.group(1).lower() == 'true', None, match.group(2) - - return False, None, f'Failed to decode answer from LLM response: {answer}' - - -class PRHandler(IssueHandler): - issue_type: ClassVar[str] = 'pr' - - def __init__(self, owner: str, repo: str, token: str, llm_config: LLMConfig): - super().__init__(owner, repo, token, llm_config) - self.download_url = 'https://api.github.com/repos/{}/{}/pulls' - - def __download_pr_metadata( - self, pull_number: int, comment_id: int | None = None - ) -> tuple[list[str], list[int], list[str], list[ReviewThread], list[str]]: - """Run a GraphQL query against the GitHub API for information. - - Retrieves information about: - 1. unresolved review comments - 2. referenced issues the pull request would close - - Args: - pull_number: The number of the pull request to query. - comment_id: Optional ID of a specific comment to focus on. - query: The GraphQL query as a string. - variables: A dictionary of variables for the query. - token: Your GitHub personal access token. - - Returns: - The JSON response from the GitHub API. - """ - # Using graphql as REST API doesn't indicate resolved status for review comments - # TODO: grabbing the first 10 issues, 100 review threads, and 100 coments; add pagination to retrieve all - query = """ - query($owner: String!, $repo: String!, $pr: Int!) { - repository(owner: $owner, name: $repo) { - pullRequest(number: $pr) { - closingIssuesReferences(first: 10) { - edges { - node { - body - number - } - } - } - url - reviews(first: 100) { - nodes { - body - state - fullDatabaseId - } - } - reviewThreads(first: 100) { - edges{ - node{ - id - isResolved - comments(first: 100) { - totalCount - nodes { - body - path - fullDatabaseId - } - } - } - } - } - } - } - } - """ - - variables = {'owner': self.owner, 'repo': self.repo, 'pr': pull_number} - - url = 'https://api.github.com/graphql' - headers = { - 'Authorization': f'Bearer {self.token}', - 'Content-Type': 'application/json', - } - - response = requests.post( - url, json={'query': query, 'variables': variables}, headers=headers - ) - response.raise_for_status() - response_json = response.json() - - # Parse the response to get closing issue references and unresolved review comments - pr_data = ( - response_json.get('data', {}).get('repository', {}).get('pullRequest', {}) - ) - - # Get closing issues - closing_issues = pr_data.get('closingIssuesReferences', {}).get('edges', []) - closing_issues_bodies = [issue['node']['body'] for issue in closing_issues] - closing_issue_numbers = [ - issue['node']['number'] for issue in closing_issues - ] # Extract issue numbers - - # Get review comments - reviews = pr_data.get('reviews', {}).get('nodes', []) - if comment_id is not None: - reviews = [ - review - for review in reviews - if int(review['fullDatabaseId']) == comment_id - ] - review_bodies = [review['body'] for review in reviews] - - # Get unresolved review threads - review_threads = [] - thread_ids = [] # Store thread IDs; agent replies to the thread - raw_review_threads = pr_data.get('reviewThreads', {}).get('edges', []) - for thread in raw_review_threads: - node = thread.get('node', {}) - if not node.get( - 'isResolved', True - ): # Check if the review thread is unresolved - id = node.get('id') - thread_contains_comment_id = False - my_review_threads = node.get('comments', {}).get('nodes', []) - message = '' - files = [] - for i, review_thread in enumerate(my_review_threads): - if ( - comment_id is not None - and int(review_thread['fullDatabaseId']) == comment_id - ): - thread_contains_comment_id = True - - if ( - i == len(my_review_threads) - 1 - ): # Check if it's the last thread in the thread - if len(my_review_threads) > 1: - message += '---\n' # Add "---" before the last message if there's more than one thread - message += 'latest feedback:\n' + review_thread['body'] + '\n' - else: - message += ( - review_thread['body'] + '\n' - ) # Add each thread in a new line - - file = review_thread.get('path') - if file and file not in files: - files.append(file) - - if comment_id is None or thread_contains_comment_id: - unresolved_thread = ReviewThread(comment=message, files=files) - review_threads.append(unresolved_thread) - thread_ids.append(id) - - return ( - closing_issues_bodies, - closing_issue_numbers, - review_bodies, - review_threads, - thread_ids, - ) + issues_context = json.dumps(issue.closing_issues, indent=4) + success_list = [] + explanation_list = [] - # Override processing of downloaded issues - def _get_pr_comments( - self, pr_number: int, comment_id: int | None = None - ) -> list[str] | None: - """Download comments for a specific pull request from Github.""" - url = f'https://api.github.com/repos/{self.owner}/{self.repo}/issues/{pr_number}/comments' - headers = { - 'Authorization': f'token {self.token}', - 'Accept': 'application/vnd.github.v3+json', - } - params = {'per_page': 100, 'page': 1} - all_comments = [] - - while True: - response = requests.get(url, headers=headers, params=params) - response.raise_for_status() - comments = response.json() - - if not comments: - break - - if comment_id is not None: - matching_comment = next( - ( - comment['body'] - for comment in comments - if comment['id'] == comment_id - ), - None, + # Handle PRs with file-specific review comments + if issue.review_threads: + for review_thread in issue.review_threads: + if issues_context and last_message: + success, explanation = self._check_review_thread( + review_thread, issues_context, last_message + ) + else: + success, explanation = False, 'Missing context or message' + success_list.append(success) + explanation_list.append(explanation) + # Handle PRs with only thread comments (no file-specific review comments) + elif issue.thread_comments: + if issue.thread_comments and issues_context and last_message: + success, explanation = self._check_thread_comments( + issue.thread_comments, issues_context, last_message ) - if matching_comment: - return [matching_comment] else: - all_comments.extend([comment['body'] for comment in comments]) - - params['page'] += 1 - - return all_comments if all_comments else None - - def __get_context_from_external_issues_references( - self, - closing_issues: list[str], - closing_issue_numbers: list[int], - issue_body: str, - review_comments: list[str], - review_threads: list[ReviewThread], - thread_comments: list[str] | None, - ): - new_issue_references = [] - - if issue_body: - new_issue_references.extend(self._extract_issue_references(issue_body)) - - if review_comments: - for comment in review_comments: - new_issue_references.extend(self._extract_issue_references(comment)) - - if review_threads: - for review_thread in review_threads: - new_issue_references.extend( - self._extract_issue_references(review_thread.comment) + success, explanation = ( + False, + 'Missing thread comments, context or message', ) - - if thread_comments: - for thread_comment in thread_comments: - new_issue_references.extend( - self._extract_issue_references(thread_comment) + success_list.append(success) + explanation_list.append(explanation) + elif issue.review_comments: + # Handle PRs with only review comments (no file-specific review comments or thread comments) + if issue.review_comments and issues_context and last_message: + success, explanation = self._check_review_comments( + issue.review_comments, issues_context, last_message ) + else: + success, explanation = ( + False, + 'Missing review comments, context or message', + ) + success_list.append(success) + explanation_list.append(explanation) + else: + # No review comments, thread comments, or file-level review comments found + return False, None, 'No feedback was found to process' - non_duplicate_references = set(new_issue_references) - unique_issue_references = non_duplicate_references.difference( - closing_issue_numbers - ) - - for issue_number in unique_issue_references: - try: - url = f'https://api.github.com/repos/{self.owner}/{self.repo}/issues/{issue_number}' - headers = { - 'Authorization': f'Bearer {self.token}', - 'Accept': 'application/vnd.github.v3+json', - } - response = requests.get(url, headers=headers) - response.raise_for_status() - issue_data = response.json() - issue_body = issue_data.get('body', '') - if issue_body: - closing_issues.append(issue_body) - except requests.exceptions.RequestException as e: - logger.warning(f'Failed to fetch issue {issue_number}: {str(e)}') - - return closing_issues + # Return overall success (all must be true) and explanations + if not success_list: + return False, None, 'No feedback was processed' + return all(success_list), success_list, json.dumps(explanation_list) def get_converted_issues( self, issue_numbers: list[int] | None = None, comment_id: int | None = None - ) -> list[GithubIssue]: + ) -> list[Issue]: if not issue_numbers: raise ValueError('Unspecified issue numbers') - all_issues = self._download_issues_from_github() + all_issues = self.download_issues() logger.info(f'Limiting resolving to issues {issue_numbers}.') all_issues = [issue for issue in all_issues if issue['number'] in issue_numbers] @@ -545,9 +127,9 @@ def get_converted_issues( thread_comments, ) - issue_details = GithubIssue( - owner=self.owner, - repo=self.repo, + issue_details = Issue( + owner=self._strategy.owner, + repo=self._strategy.repo, number=issue['number'], title=issue['title'], body=body, @@ -565,7 +147,7 @@ def get_converted_issues( def get_instruction( self, - issue: GithubIssue, + issue: Issue, prompt_template: str, repo_instruction: str | None = None, ) -> tuple[str, list[str]]: @@ -576,13 +158,13 @@ def get_instruction( issues_str = None if issue.closing_issues: issues_str = json.dumps(issue.closing_issues, indent=4) - images.extend(self._extract_image_urls(issues_str)) + images.extend(extract_image_urls(issues_str)) # Handle PRs with review comments review_comments_str = None if issue.review_comments: review_comments_str = json.dumps(issue.review_comments, indent=4) - images.extend(self._extract_image_urls(review_comments_str)) + images.extend(extract_image_urls(review_comments_str)) # Handle PRs with file-specific review comments review_thread_str = None @@ -596,13 +178,13 @@ def get_instruction( review_thread_files.extend(review_thread.files) review_thread_str = json.dumps(review_threads, indent=4) review_thread_file_str = json.dumps(review_thread_files, indent=4) - images.extend(self._extract_image_urls(review_thread_str)) + images.extend(extract_image_urls(review_thread_str)) # Format thread comments if they exist thread_context = '' if issue.thread_comments: thread_context = '\n---\n'.join(issue.thread_comments) - images.extend(self._extract_image_urls(thread_context)) + images.extend(extract_image_urls(thread_context)) instruction = template.render( issues=issues_str, @@ -653,10 +235,7 @@ def _check_review_thread( return self._check_feedback_with_llm(prompt) def _check_thread_comments( - self, - thread_comments: list[str], - issues_context: str, - last_message: str, + self, thread_comments: list[str], issues_context: str, last_message: str ) -> tuple[bool, str]: """Check if thread comments feedback has been addressed.""" thread_context = '\n---\n'.join(thread_comments) @@ -678,10 +257,7 @@ def _check_thread_comments( return self._check_feedback_with_llm(prompt) def _check_review_comments( - self, - review_comments: list[str], - issues_context: str, - last_message: str, + self, review_comments: list[str], issues_context: str, last_message: str ) -> tuple[bool, str]: """Check if review comments feedback has been addressed.""" review_context = '\n---\n'.join(review_comments) @@ -702,57 +278,207 @@ def _check_review_comments( return self._check_feedback_with_llm(prompt) + def __download_pr_metadata( + self, pull_number: int, comment_id: int | None = None + ) -> tuple[list[str], list[int], list[str], list[ReviewThread], list[str]]: + return self._strategy.download_pr_metadata(pull_number, comment_id) + + def _get_pr_comments( + self, pr_number: int, comment_id: int | None = None + ) -> list[str] | None: + return self._strategy.get_pr_comments(pr_number, comment_id) + + def __get_context_from_external_issues_references( + self, + closing_issues: list[str], + closing_issue_numbers: list[int], + issue_body: str, + review_comments: list[str], + review_threads: list[ReviewThread], + thread_comments: list[str] | None, + ): + return self._strategy.get_context_from_external_issues_references( + closing_issues, + closing_issue_numbers, + issue_body, + review_comments, + review_threads, + thread_comments, + ) + + +class ServiceContext: + issue_type: ClassVar[str] = 'issue' + + def __init__(self, strategy, llm_config: LLMConfig): + self._strategy = strategy + self.llm = LLM(llm_config) + + def set_strategy(self, strategy): + self._strategy = strategy + + def get_base_url(self): + return self._strategy.get_base_url() + + def get_branch_url(self, branch_name): + return self._strategy.get_branch_url(branch_name) + + def get_download_url(self): + return self._strategy.get_download_url() + + def get_clone_url(self): + return self._strategy.get_clone_url() + + def get_graphql_url(self): + return self._strategy.get_graphql_url() + + def get_headers(self): + return self._strategy.get_headers() + + def get_authorize_url(self): + return self._strategy.get_authorize_url() + + def get_pull_url(self, pr_number: int): + return self._strategy.get_pull_url(pr_number) + + def get_compare_url(self, branch_name: str): + return self._strategy.get_compare_url(branch_name) + + def download_issues(self) -> list[Any]: + return self._strategy.download_issues() + + def get_branch_name( + self, + base_branch_name: str, + ): + return self._strategy.get_branch_name(base_branch_name) + + def branch_exists(self, branch_name: str): + return self._strategy.branch_exists(branch_name) + + def get_default_branch_name(self) -> str: + return self._strategy.get_default_branch_name() + + def create_pull_request(self, data=dict): + return self._strategy.create_pull_request(data) + + def reply_to_comment(self, token, comment_id, reply): + return self._strategy.reply_to_comment(token, comment_id, reply) + + def get_issue_comments( + self, issue_number: int, comment_id: int | None = None + ) -> list[str] | None: + return self._strategy.get_issue_comments(issue_number, comment_id) + + def get_instruction( + self, + issue: Issue, + prompt_template: str, + repo_instruction: str | None = None, + ) -> tuple[str, list[str]]: + """Generate instruction for the agent.""" + # Format thread comments if they exist + thread_context = '' + if issue.thread_comments: + thread_context = '\n\nIssue Thread Comments:\n' + '\n---\n'.join( + issue.thread_comments + ) + + images = [] + images.extend(extract_image_urls(issue.body)) + images.extend(extract_image_urls(thread_context)) + + template = jinja2.Template(prompt_template) + return ( + template.render( + body=issue.title + '\n\n' + issue.body + thread_context, + repo_instruction=repo_instruction, + ), + images, + ) + def guess_success( - self, issue: GithubIssue, history: list[Event] + self, issue: Issue, history: list[Event] ) -> tuple[bool, None | list[bool], str]: """Guess if the issue is fixed based on the history and the issue description.""" last_message = history[-1].message - issues_context = json.dumps(issue.closing_issues, indent=4) - success_list = [] - explanation_list = [] + # Include thread comments in the prompt if they exist + issue_context = issue.body + if issue.thread_comments: + issue_context += '\n\nIssue Thread Comments:\n' + '\n---\n'.join( + issue.thread_comments + ) - # Handle PRs with file-specific review comments - if issue.review_threads: - for review_thread in issue.review_threads: - if issues_context and last_message: - success, explanation = self._check_review_thread( - review_thread, issues_context, last_message - ) - else: - success, explanation = False, 'Missing context or message' - success_list.append(success) - explanation_list.append(explanation) - # Handle PRs with only thread comments (no file-specific review comments) - elif issue.thread_comments: - if issue.thread_comments and issues_context and last_message: - success, explanation = self._check_thread_comments( - issue.thread_comments, issues_context, last_message - ) - else: - success, explanation = ( - False, - 'Missing thread comments, context or message', - ) - success_list.append(success) - explanation_list.append(explanation) - elif issue.review_comments: - # Handle PRs with only review comments (no file-specific review comments or thread comments) - if issue.review_comments and issues_context and last_message: - success, explanation = self._check_review_comments( - issue.review_comments, issues_context, last_message - ) - else: - success, explanation = ( - False, - 'Missing review comments, context or message', + with open( + os.path.join( + os.path.dirname(__file__), + 'prompts/guess_success/issue-success-check.jinja', + ), + 'r', + ) as f: + template = jinja2.Template(f.read()) + prompt = template.render(issue_context=issue_context, last_message=last_message) + + response = self.llm.completion(messages=[{'role': 'user', 'content': prompt}]) + + answer = response.choices[0].message.content.strip() + pattern = r'--- success\n*(true|false)\n*--- explanation*\n((?:.|\n)*)' + match = re.search(pattern, answer) + if match: + return match.group(1).lower() == 'true', None, match.group(2) + + return False, None, f'Failed to decode answer from LLM response: {answer}' + + def get_converted_issues( + self, issue_numbers: list[int] | None = None, comment_id: int | None = None + ) -> list[Issue]: + """Download issues + + Returns: + List issues. + """ + + if not issue_numbers: + raise ValueError('Unspecified issue number') + + all_issues = self.download_issues() + logger.info(f'Limiting resolving to issues {issue_numbers}.') + all_issues = [ + issue + for issue in all_issues + if issue['number'] in issue_numbers and 'pull_request' not in issue + ] + + if len(issue_numbers) == 1 and not all_issues: + raise ValueError(f'Issue {issue_numbers[0]} not found') + + converted_issues = [] + for issue in all_issues: + if any([issue.get(key) is None for key in ['number', 'title']]): + logger.warning( + f'Skipping issue {issue} as it is missing number or title.' ) - success_list.append(success) - explanation_list.append(explanation) - else: - # No review comments, thread comments, or file-level review comments found - return False, None, 'No feedback was found to process' + continue - # Return overall success (all must be true) and explanations - if not success_list: - return False, None, 'No feedback was processed' - return all(success_list), success_list, '\n'.join(explanation_list) + # Handle empty body by using empty string + if issue.get('body') is None: + issue['body'] = '' + + # Get issue thread comments + thread_comments = self.get_issue_comments( + issue['number'], comment_id=comment_id + ) + # Convert empty lists to None for optional fields + issue_details = Issue( + owner=self._strategy.owner, + repo=self._strategy.repo, + number=issue['number'], + title=issue['title'], + body=issue['body'], + thread_comments=thread_comments, + review_comments=None, # Initialize review comments as None for regular issues + ) + + converted_issues.append(issue_details) + + return converted_issues diff --git a/openhands/resolver/resolve_all_issues.py b/openhands/resolver/resolve_all_issues.py index 9c44855a2dd4..ef357c60c58d 100644 --- a/openhands/resolver/resolve_all_issues.py +++ b/openhands/resolver/resolve_all_issues.py @@ -13,7 +13,7 @@ import openhands from openhands.core.config import LLMConfig from openhands.core.logger import openhands_logger as logger -from openhands.resolver.github_issue import GithubIssue +from openhands.resolver.issue import Issue from openhands.resolver.resolve_issue import ( issue_handler_factory, process_issue, @@ -83,7 +83,7 @@ async def resolve_issues( issue_handler = issue_handler_factory(issue_type, owner, repo, token, llm_config) # Load dataset - issues: list[GithubIssue] = issue_handler.get_converted_issues( + issues: list[Issue] = issue_handler.get_converted_issues( issue_numbers=issue_numbers ) diff --git a/openhands/resolver/resolve_issue.py b/openhands/resolver/resolve_issue.py index 63a9e40a05ba..e42f74f71d4b 100644 --- a/openhands/resolver/resolve_issue.py +++ b/openhands/resolver/resolve_issue.py @@ -29,11 +29,11 @@ Observation, ) from openhands.events.stream import EventStreamSubscriber -from openhands.resolver.github_issue import GithubIssue +from openhands.resolver.github import GithubIssueHandler, GithubPRHandler +from openhands.resolver.issue import Issue from openhands.resolver.issue_definitions import ( - IssueHandler, - IssueHandlerInterface, - PRHandler, + ServiceContext, + ServiceContextPR, ) from openhands.resolver.resolver_output import ResolverOutput from openhands.resolver.utils import ( @@ -151,14 +151,14 @@ async def complete_runtime( async def process_issue( - issue: GithubIssue, + issue: Issue, base_commit: str, max_iterations: int, llm_config: LLMConfig, output_dir: str, runtime_container_image: str, prompt_template: str, - issue_handler: IssueHandlerInterface, + issue_handler: ServiceContext | ServiceContextPR, repo_instruction: str | None = None, reset_logger: bool = False, ) -> ResolverOutput: @@ -291,12 +291,19 @@ async def on_event(evt): def issue_handler_factory( - issue_type: str, owner: str, repo: str, token: str, llm_config: LLMConfig -) -> IssueHandlerInterface: + issue_type: str, + owner: str, + repo: str, + token: str, + llm_config: LLMConfig, + username: str | None = None, +) -> ServiceContext | ServiceContextPR: if issue_type == 'issue': - return IssueHandler(owner, repo, token, llm_config) + return ServiceContext( + GithubIssueHandler(owner, repo, token, username), llm_config + ) elif issue_type == 'pr': - return PRHandler(owner, repo, token, llm_config) + return ServiceContextPR(GithubPRHandler(owner, repo, token), llm_config) else: raise ValueError(f'Invalid issue type: {issue_type}') @@ -318,13 +325,13 @@ async def resolve_issue( target_branch: str | None = None, reset_logger: bool = False, ) -> None: - """Resolve a single github issue. + """Resolve a single issue. Args: - owner: Github owner of the repo. - repo: Github repository to resolve issues in form of `owner/repo`. - token: Github token to access the repository. - username: Github username to access the repository. + owner: owner of the repo. + repo: repository to resolve issues in form of `owner/repo`. + token: token to access the repository. + username: username to access the repository. max_iterations: Maximum number of iterations to run. output_dir: Output directory to write the results. llm_config: Configuration for the language model. @@ -337,10 +344,12 @@ async def resolve_issue( target_branch: Optional target branch to create PR against (for PRs). reset_logger: Whether to reset the logger for multiprocessing. """ - issue_handler = issue_handler_factory(issue_type, owner, repo, token, llm_config) + issue_handler = issue_handler_factory( + issue_type, owner, repo, token, llm_config, username + ) # Load dataset - issues: list[GithubIssue] = issue_handler.get_converted_issues( + issues: list[Issue] = issue_handler.get_converted_issues( issue_numbers=[issue_number], comment_id=comment_id ) @@ -378,7 +387,7 @@ async def resolve_issue( [ 'git', 'clone', - f'https://{username}:{token}@github.com/{owner}/{repo}', + issue_handler.get_clone_url(), f'{output_dir}/repo', ] ).decode('utf-8') @@ -485,24 +494,24 @@ def int_or_none(value): else: return int(value) - parser = argparse.ArgumentParser(description='Resolve a single issue from Github.') + parser = argparse.ArgumentParser(description='Resolve a single issue.') parser.add_argument( '--repo', type=str, required=True, - help='Github repository to resolve issues in form of `owner/repo`.', + help='repository to resolve issues in form of `owner/repo`.', ) parser.add_argument( '--token', type=str, default=None, - help='Github token to access the repository.', + help='token to access the repository.', ) parser.add_argument( '--username', type=str, default=None, - help='Github username to access the repository.', + help='username to access the repository.', ) parser.add_argument( '--runtime-container-image', @@ -591,10 +600,10 @@ def int_or_none(value): token = my_args.token if my_args.token else os.getenv('GITHUB_TOKEN') username = my_args.username if my_args.username else os.getenv('GITHUB_USERNAME') if not username: - raise ValueError('Github username is required.') + raise ValueError('username is required.') if not token: - raise ValueError('Github token is required.') + raise ValueError('token is required.') llm_config = LLMConfig( model=my_args.llm_model or os.environ['LLM_MODEL'], diff --git a/openhands/resolver/resolver_output.py b/openhands/resolver/resolver_output.py index 5978620f0359..bd3fece2230c 100644 --- a/openhands/resolver/resolver_output.py +++ b/openhands/resolver/resolver_output.py @@ -2,12 +2,12 @@ from litellm import BaseModel -from openhands.resolver.github_issue import GithubIssue +from openhands.resolver.issue import Issue class ResolverOutput(BaseModel): # NOTE: User-specified - issue: GithubIssue + issue: Issue issue_type: str instruction: str base_commit: str diff --git a/openhands/resolver/send_pull_request.py b/openhands/resolver/send_pull_request.py index 8a9d6118bedd..3155f079b0a3 100644 --- a/openhands/resolver/send_pull_request.py +++ b/openhands/resolver/send_pull_request.py @@ -10,11 +10,13 @@ from openhands.core.config import LLMConfig from openhands.core.logger import openhands_logger as logger -from openhands.resolver.github_issue import GithubIssue +from openhands.resolver.github import GithubIssueHandler from openhands.resolver.io_utils import ( load_all_resolver_outputs, load_single_resolver_output, ) +from openhands.resolver.issue import Issue +from openhands.resolver.issue_definitions import ServiceContext from openhands.resolver.patching import apply_diff, parse_patch from openhands.resolver.resolver_output import ResolverOutput @@ -138,7 +140,7 @@ def initialize_repo( return dest_dir -def make_commit(repo_dir: str, issue: GithubIssue, issue_type: str) -> None: +def make_commit(repo_dir: str, issue: Issue, issue_type: str) -> None: # Check if git username is set result = subprocess.run( f'git -C {repo_dir} config user.name', @@ -186,18 +188,10 @@ def make_commit(repo_dir: str, issue: GithubIssue, issue_type: str) -> None: raise RuntimeError(f'Failed to commit changes: {result}') -def branch_exists(base_url: str, branch_name: str, headers: dict) -> bool: - print(f'Checking if branch {branch_name} exists...') - response = requests.get(f'{base_url}/branches/{branch_name}', headers=headers) - exists = response.status_code == 200 - print(f'Branch {branch_name} exists: {exists}') - return exists - - def send_pull_request( - github_issue: GithubIssue, - github_token: str, - github_username: str | None, + issue: Issue, + token: str, + username: str | None, patch_dir: str, llm_config: LLMConfig, pr_type: str, @@ -208,35 +202,25 @@ def send_pull_request( if pr_type not in ['branch', 'draft', 'ready']: raise ValueError(f'Invalid pr_type: {pr_type}') - # Set up headers and base URL for GitHub API - headers = { - 'Authorization': f'token {github_token}', - 'Accept': 'application/vnd.github.v3+json', - } - base_url = f'https://api.github.com/repos/{github_issue.owner}/{github_issue.repo}' + handler = ServiceContext( + GithubIssueHandler(issue.owner, issue.repo, token, username), llm_config + ) # Create a new branch with a unique name - base_branch_name = f'openhands-fix-issue-{github_issue.number}' - branch_name = base_branch_name - attempt = 1 - - print('Checking if branch exists...') - while branch_exists(base_url, branch_name, headers): - attempt += 1 - branch_name = f'{base_branch_name}-try{attempt}' + base_branch_name = f'openhands-fix-issue-{issue.number}' + branch_name = handler.get_branch_name( + base_branch_name=base_branch_name, + ) # Get the default branch or use specified target branch print('Getting base branch...') if target_branch: base_branch = target_branch - # Verify the target branch exists - response = requests.get(f'{base_url}/branches/{target_branch}', headers=headers) - if response.status_code != 200: + exists = handler.branch_exists(branch_name=target_branch) + if not exists: raise ValueError(f'Target branch {target_branch} does not exist') else: - response = requests.get(f'{base_url}', headers=headers) - response.raise_for_status() - base_branch = response.json()['default_branch'] + base_branch = handler.get_default_branch_name() print(f'Base branch: {base_branch}') # Create and checkout the new branch @@ -253,16 +237,12 @@ def send_pull_request( ) # Determine the repository to push to (original or fork) - push_owner = fork_owner if fork_owner else github_issue.owner - push_repo = github_issue.repo + push_owner = fork_owner if fork_owner else issue.owner + + handler._strategy.set_owner = push_owner print('Pushing changes...') - username_and_token = ( - f'{github_username}:{github_token}' - if github_username - else f'x-auth-token:{github_token}' - ) - push_url = f'https://{username_and_token}@github.com/{push_owner}/{push_repo}.git' + push_url = handler.get_clone_url() result = subprocess.run( ['git', '-C', patch_dir, 'push', push_url, branch_name], capture_output=True, @@ -272,8 +252,8 @@ def send_pull_request( print(f'Error pushing changes: {result.stderr}') raise RuntimeError('Failed to push changes to the remote repository') - pr_title = f'Fix issue #{github_issue.number}: {github_issue.title}' - pr_body = f'This pull request fixes #{github_issue.number}.' + pr_title = f'Fix issue #{issue.number}: {issue.title}' + pr_body = f'This pull request fixes #{issue.number}.' if additional_message: pr_body += f'\n\n{additional_message}' pr_body += '\n\nAutomatic fix generated by [OpenHands](https://github.com/All-Hands-AI/OpenHands/) 🙌' @@ -281,7 +261,7 @@ def send_pull_request( # If we are not sending a PR, we can finish early and return the # URL for the user to open a PR manually if pr_type == 'branch': - url = f'https://github.com/{push_owner}/{github_issue.repo}/compare/{branch_name}?expand=1' + url = handler.get_compare_url(branch_name) else: data = { 'title': pr_title, # No need to escape title for GitHub API @@ -290,54 +270,17 @@ def send_pull_request( 'base': base_branch, 'draft': pr_type == 'draft', } - response = requests.post(f'{base_url}/pulls', headers=headers, json=data) - if response.status_code == 403: - raise RuntimeError( - 'Failed to create pull request due to missing permissions. ' - 'Make sure that the provided token has push permissions for the repository.' - ) - response.raise_for_status() - pr_data = response.json() - - url = pr_data['html_url'] + url = handler.create_pull_request(data) print(f'{pr_type} created: {url}\n\n--- Title: {pr_title}\n\n--- Body:\n{pr_body}') return url -def reply_to_comment(github_token: str, comment_id: str, reply: str): - # Opting for graphql as REST API doesn't allow reply to replies in comment threads - query = """ - mutation($body: String!, $pullRequestReviewThreadId: ID!) { - addPullRequestReviewThreadReply(input: { body: $body, pullRequestReviewThreadId: $pullRequestReviewThreadId }) { - comment { - id - body - createdAt - } - } - } - """ - - comment_reply = f'Openhands fix success summary\n\n\n{reply}' - variables = {'body': comment_reply, 'pullRequestReviewThreadId': comment_id} - url = 'https://api.github.com/graphql' - headers = { - 'Authorization': f'Bearer {github_token}', - 'Content-Type': 'application/json', - } - - response = requests.post( - url, json={'query': query, 'variables': variables}, headers=headers - ) - response.raise_for_status() - - def update_existing_pull_request( - github_issue: GithubIssue, - github_token: str, - github_username: str | None, + issue: Issue, + token: str, + username: str | None, patch_dir: str, llm_config: LLMConfig, comment_message: str | None = None, @@ -346,27 +289,29 @@ def update_existing_pull_request( """Update an existing pull request with the new patches. Args: - github_issue: The issue to update. - github_token: The GitHub token to use for authentication. - github_username: The GitHub username to use for authentication. + issue: The issue to update. + token: The token to use for authentication. + username: The username to use for authentication. patch_dir: The directory containing the patches to apply. llm_config: The LLM configuration to use for summarizing changes. comment_message: The main message to post as a comment on the PR. additional_message: The additional messages to post as a comment on the PR in json list format. """ # Set up headers and base URL for GitHub API - headers = { - 'Authorization': f'token {github_token}', - 'Accept': 'application/vnd.github.v3+json', - } - base_url = f'https://api.github.com/repos/{github_issue.owner}/{github_issue.repo}' - branch_name = github_issue.head_branch + + handler = ServiceContext( + GithubIssueHandler(issue.owner, issue.repo, token, username), llm_config + ) + + headers = handler.get_headers() + base_url = handler.get_base_url() + branch_name = issue.head_branch # Push the changes to the existing branch push_command = ( f'git -C {patch_dir} push ' - f'https://{github_username}:{github_token}@github.com/' - f'{github_issue.owner}/{github_issue.repo}.git {branch_name}' + f'{handler.get_authorize_url()}' + f'{issue.owner}/{issue.repo}.git {branch_name}' ) result = subprocess.run(push_command, shell=True, capture_output=True, text=True) @@ -374,7 +319,7 @@ def update_existing_pull_request( print(f'Error pushing changes: {result.stderr}') raise RuntimeError('Failed to push changes to the remote repository') - pr_url = f'https://github.com/{github_issue.owner}/{github_issue.repo}/pull/{github_issue.number}' + pr_url = handler.get_pull_url(issue.number) print(f'Updated pull request {pr_url} with new patches.') # Generate a summary of all comment success indicators for PR message @@ -412,7 +357,7 @@ def update_existing_pull_request( # Post a comment on the PR if comment_message: - comment_url = f'{base_url}/issues/{github_issue.number}/comments' + comment_url = f'{base_url}/issues/{issue.number}/comments' comment_data = {'body': comment_message} comment_response = requests.post( comment_url, headers=headers, json=comment_data @@ -425,11 +370,11 @@ def update_existing_pull_request( print(f'Comment added to the PR: {comment_message}') # Reply to each unresolved comment thread - if additional_message and github_issue.thread_ids: + if additional_message and issue.thread_ids: explanations = json.loads(additional_message) for count, reply_comment in enumerate(explanations): - comment_id = github_issue.thread_ids[count] - reply_to_comment(github_token, comment_id, reply_comment) + comment_id = issue.thread_ids[count] + handler.reply_to_comment(token, comment_id, reply_comment) return pr_url @@ -437,8 +382,8 @@ def update_existing_pull_request( def process_single_issue( output_dir: str, resolver_output: ResolverOutput, - github_token: str, - github_username: str, + token: str, + username: str, pr_type: str, llm_config: LLMConfig, fork_owner: str | None, @@ -476,18 +421,18 @@ def process_single_issue( if issue_type == 'pr': update_existing_pull_request( - github_issue=resolver_output.issue, - github_token=github_token, - github_username=github_username, + issue=resolver_output.issue, + token=token, + username=username, patch_dir=patched_repo_dir, additional_message=resolver_output.success_explanation, llm_config=llm_config, ) else: send_pull_request( - github_issue=resolver_output.issue, - github_token=github_token, - github_username=github_username, + issue=resolver_output.issue, + token=token, + username=username, patch_dir=patched_repo_dir, pr_type=pr_type, llm_config=llm_config, @@ -499,8 +444,8 @@ def process_single_issue( def process_all_successful_issues( output_dir: str, - github_token: str, - github_username: str, + token: str, + username: str, pr_type: str, llm_config: LLMConfig, fork_owner: str | None, @@ -512,8 +457,8 @@ def process_all_successful_issues( process_single_issue( output_dir, resolver_output, - github_token, - github_username, + token, + username, pr_type, llm_config, fork_owner, @@ -525,16 +470,16 @@ def process_all_successful_issues( def main(): parser = argparse.ArgumentParser(description='Send a pull request to Github.') parser.add_argument( - '--github-token', + '--token', type=str, default=None, - help='Github token to access the repository.', + help='token to access the repository.', ) parser.add_argument( - '--github-username', + '--username', type=str, default=None, - help='Github username to access the repository.', + help='username to access the repository.', ) parser.add_argument( '--output-dir', @@ -592,18 +537,12 @@ def main(): ) my_args = parser.parse_args() - github_token = ( - my_args.github_token if my_args.github_token else os.getenv('GITHUB_TOKEN') - ) - if not github_token: + token = my_args.token if my_args.token else os.getenv('GITHUB_TOKEN') + if not token: raise ValueError( - 'Github token is not set, set via --github-token or GITHUB_TOKEN environment variable.' + 'token is not set, set via --token or GITHUB_TOKEN environment variable.' ) - github_username = ( - my_args.github_username - if my_args.github_username - else os.getenv('GITHUB_USERNAME') - ) + username = my_args.username if my_args.username else os.getenv('GITHUB_USERNAME') llm_config = LLMConfig( model=my_args.llm_model or os.environ['LLM_MODEL'], @@ -615,12 +554,12 @@ def main(): raise ValueError(f'Output directory {my_args.output_dir} does not exist.') if my_args.issue_number == 'all_successful': - if not github_username: - raise ValueError('Github username is required.') + if not username: + raise ValueError('username is required.') process_all_successful_issues( my_args.output_dir, - github_token, - github_username, + token, + username, my_args.pr_type, llm_config, my_args.fork_owner, @@ -631,13 +570,13 @@ def main(): issue_number = int(my_args.issue_number) output_path = os.path.join(my_args.output_dir, 'output.jsonl') resolver_output = load_single_resolver_output(output_path, issue_number) - if not github_username: - raise ValueError('Github username is required.') + if not username: + raise ValueError('username is required.') process_single_issue( my_args.output_dir, resolver_output, - github_token, - github_username, + token, + username, my_args.pr_type, llm_config, my_args.fork_owner, diff --git a/openhands/resolver/utils.py b/openhands/resolver/utils.py index 583026455945..fc1ad18045ee 100644 --- a/openhands/resolver/utils.py +++ b/openhands/resolver/utils.py @@ -2,6 +2,7 @@ import logging import multiprocessing as mp import os +import re from typing import Callable import pandas as pd @@ -137,3 +138,28 @@ def reset_logger_for_multiprocessing( logging.Formatter('%(asctime)s - %(levelname)s - %(message)s') ) logger.addHandler(file_handler) + + +def extract_image_urls(issue_body: str) -> list[str]: + # Regular expression to match Markdown image syntax ![alt text](image_url) + image_pattern = r'!\[.*?\]\((https?://[^\s)]+)\)' + return re.findall(image_pattern, issue_body) + + +def extract_issue_references(body: str) -> list[int]: + # First, remove code blocks as they may contain false positives + body = re.sub(r'```.*?```', '', body, flags=re.DOTALL) + + # Remove inline code + body = re.sub(r'`[^`]*`', '', body) + + # Remove URLs that contain hash symbols + body = re.sub(r'https?://[^\s)]*#\d+[^\s)]*', '', body) + + # Now extract issue numbers, making sure they're not part of other text + # The pattern matches #number that: + # 1. Is at the start of text or after whitespace/punctuation + # 2. Is followed by whitespace, punctuation, or end of text + # 3. Is not part of a URL + pattern = r'(?:^|[\s\[({]|[^\w#])#(\d+)(?=[\s,.\])}]|$)' + return [int(match) for match in re.findall(pattern, body)] diff --git a/tests/unit/resolver/test_guess_success.py b/tests/unit/resolver/test_guess_success.py index 9d1b55487118..cc8089a48ede 100644 --- a/tests/unit/resolver/test_guess_success.py +++ b/tests/unit/resolver/test_guess_success.py @@ -3,13 +3,14 @@ from openhands.core.config import LLMConfig from openhands.events.action.message import MessageAction from openhands.llm import LLM -from openhands.resolver.github_issue import GithubIssue -from openhands.resolver.issue_definitions import IssueHandler, PRHandler +from openhands.resolver.github import GithubIssueHandler, GithubPRHandler +from openhands.resolver.issue import Issue +from openhands.resolver.issue_definitions import ServiceContext, ServiceContextPR def test_guess_success_multiline_explanation(): # Mock data - issue = GithubIssue( + issue = Issue( owner='test', repo='test', number=1, @@ -43,7 +44,7 @@ def test_guess_success_multiline_explanation(): # Use patch to mock the LLM completion call with patch.object(LLM, 'completion', return_value=mock_response) as mock_completion: # Create a handler instance - handler = IssueHandler('test', 'test', 'test', llm_config) + handler = ServiceContext(GithubIssueHandler('test', 'test', 'test'), llm_config) # Call guess_success success, _, explanation = handler.guess_success(issue, history) @@ -63,10 +64,10 @@ def test_guess_success_multiline_explanation(): def test_pr_handler_guess_success_with_thread_comments(): # Create a PR handler instance llm_config = LLMConfig(model='test', api_key='test') - handler = PRHandler('test-owner', 'test-repo', 'test-token', llm_config) + handler = ServiceContextPR(GithubPRHandler('test', 'test', 'test'), llm_config) # Create a mock issue with thread comments but no review comments - issue = GithubIssue( + issue = Issue( owner='test-owner', repo='test-repo', number=1, @@ -112,10 +113,12 @@ def test_pr_handler_guess_success_with_thread_comments(): def test_pr_handler_guess_success_only_review_comments(): # Create a PR handler instance llm_config = LLMConfig(model='test', api_key='test') - handler = PRHandler('test-owner', 'test-repo', 'test-token', llm_config) + handler = ServiceContextPR( + GithubPRHandler('test-owner', 'test-repo', 'test-token'), llm_config + ) # Create a mock issue with only review comments - issue = GithubIssue( + issue = Issue( owner='test-owner', repo='test-repo', number=1, @@ -155,16 +158,18 @@ def test_pr_handler_guess_success_only_review_comments(): # Verify the results assert success is True assert success_list == [True] - assert 'successfully address' in explanation + assert ( + '["The changes successfully address the review comments."]' in explanation + ) def test_pr_handler_guess_success_no_comments(): # Create a PR handler instance llm_config = LLMConfig(model='test', api_key='test') - handler = PRHandler('test-owner', 'test-repo', 'test-token', llm_config) + handler = ServiceContextPR(GithubPRHandler('test', 'test', 'test'), llm_config) # Create a mock issue with no comments - issue = GithubIssue( + issue = Issue( owner='test-owner', repo='test-repo', number=1, diff --git a/tests/unit/resolver/test_issue_handler.py b/tests/unit/resolver/test_issue_handler.py index 56f012fd77c3..72e7be746087 100644 --- a/tests/unit/resolver/test_issue_handler.py +++ b/tests/unit/resolver/test_issue_handler.py @@ -1,8 +1,9 @@ from unittest.mock import MagicMock, patch from openhands.core.config import LLMConfig -from openhands.resolver.github_issue import ReviewThread -from openhands.resolver.issue_definitions import IssueHandler, PRHandler +from openhands.resolver.github import GithubIssueHandler, GithubPRHandler +from openhands.resolver.issue import ReviewThread +from openhands.resolver.issue_definitions import ServiceContext, ServiceContextPR def test_get_converted_issues_initializes_review_comments(): @@ -27,7 +28,9 @@ def test_get_converted_issues_initializes_review_comments(): # Create an instance of IssueHandler llm_config = LLMConfig(model='test', api_key='test') - handler = IssueHandler('test-owner', 'test-repo', 'test-token', llm_config) + handler = ServiceContext( + GithubIssueHandler('test-owner', 'test-repo', 'test-token'), llm_config + ) # Get converted issues issues = handler.get_converted_issues(issue_numbers=[1]) @@ -57,7 +60,6 @@ def test_get_converted_issues_handles_empty_body(): # Mock the response for comments mock_comments_response = MagicMock() mock_comments_response.json.return_value = [] - # Set up the mock to return different responses mock_get.side_effect = [ mock_issues_response, @@ -67,7 +69,9 @@ def test_get_converted_issues_handles_empty_body(): # Create an instance of IssueHandler llm_config = LLMConfig(model='test', api_key='test') - handler = IssueHandler('test-owner', 'test-repo', 'test-token', llm_config) + handler = ServiceContext( + GithubIssueHandler('test-owner', 'test-repo', 'test-token'), llm_config + ) # Get converted issues issues = handler.get_converted_issues(issue_numbers=[1]) @@ -148,7 +152,9 @@ def test_pr_handler_get_converted_issues_with_comments(): # Create an instance of PRHandler llm_config = LLMConfig(model='test', api_key='test') - handler = PRHandler('test-owner', 'test-repo', 'test-token', llm_config) + handler = ServiceContextPR( + GithubPRHandler('test-owner', 'test-repo', 'test-token'), llm_config + ) # Get converted issues prs = handler.get_converted_issues(issue_numbers=[1]) @@ -185,10 +191,12 @@ def test_get_issue_comments_with_specific_comment_id(): # Create an instance of IssueHandler llm_config = LLMConfig(model='test', api_key='test') - handler = IssueHandler('test-owner', 'test-repo', 'test-token', llm_config) + handler = ServiceContext( + GithubIssueHandler('test-owner', 'test-repo', 'test-token'), llm_config + ) # Get comments with a specific comment_id - specific_comment = handler._get_issue_comments(issue_number=1, comment_id=123) + specific_comment = handler.get_issue_comments(issue_number=1, comment_id=123) # Verify only the specific comment is returned assert specific_comment == ['First comment'] @@ -273,7 +281,9 @@ def test_pr_handler_get_converted_issues_with_specific_thread_comment(): # Create an instance of PRHandler llm_config = LLMConfig(model='test', api_key='test') - handler = PRHandler('test-owner', 'test-repo', 'test-token', llm_config) + handler = ServiceContextPR( + GithubPRHandler('test-owner', 'test-repo', 'test-token'), llm_config + ) # Get converted issues prs = handler.get_converted_issues( @@ -376,7 +386,9 @@ def test_pr_handler_get_converted_issues_with_specific_review_thread_comment(): # Create an instance of PRHandler llm_config = LLMConfig(model='test', api_key='test') - handler = PRHandler('test-owner', 'test-repo', 'test-token', llm_config) + handler = ServiceContextPR( + GithubPRHandler('test-owner', 'test-repo', 'test-token'), llm_config + ) # Get converted issues prs = handler.get_converted_issues( @@ -499,7 +511,9 @@ def test_pr_handler_get_converted_issues_with_specific_comment_and_issue_refs(): # Create an instance of PRHandler llm_config = LLMConfig(model='test', api_key='test') - handler = PRHandler('test-owner', 'test-repo', 'test-token', llm_config) + handler = ServiceContextPR( + GithubPRHandler('test-owner', 'test-repo', 'test-token'), llm_config + ) # Get converted issues prs = handler.get_converted_issues( @@ -599,7 +613,9 @@ def test_pr_handler_get_converted_issues_with_duplicate_issue_refs(): # Create an instance of PRHandler llm_config = LLMConfig(model='test', api_key='test') - handler = PRHandler('test-owner', 'test-repo', 'test-token', llm_config) + handler = ServiceContextPR( + GithubPRHandler('test-owner', 'test-repo', 'test-token'), llm_config + ) # Get converted issues prs = handler.get_converted_issues(issue_numbers=[1]) diff --git a/tests/unit/resolver/test_issue_handler_error_handling.py b/tests/unit/resolver/test_issue_handler_error_handling.py index 93a98437168e..83c020367309 100644 --- a/tests/unit/resolver/test_issue_handler_error_handling.py +++ b/tests/unit/resolver/test_issue_handler_error_handling.py @@ -7,8 +7,9 @@ from openhands.core.config import LLMConfig from openhands.events.action.message import MessageAction from openhands.llm.llm import LLM -from openhands.resolver.github_issue import GithubIssue -from openhands.resolver.issue_definitions import IssueHandler, PRHandler +from openhands.resolver.github import GithubIssueHandler, GithubPRHandler +from openhands.resolver.issue import Issue +from openhands.resolver.issue_definitions import ServiceContext, ServiceContextPR @pytest.fixture(autouse=True) @@ -33,7 +34,9 @@ def default_config(): def test_handle_nonexistent_issue_reference(): llm_config = LLMConfig(model='test', api_key='test') - handler = PRHandler('test-owner', 'test-repo', 'test-token', llm_config) + handler = ServiceContextPR( + GithubPRHandler('test-owner', 'test-repo', 'test-token'), llm_config + ) # Mock the requests.get to simulate a 404 error mock_response = MagicMock() @@ -43,7 +46,7 @@ def test_handle_nonexistent_issue_reference(): with patch('requests.get', return_value=mock_response): # Call the method with a non-existent issue reference - result = handler._PRHandler__get_context_from_external_issues_references( + result = handler._ServiceContextPR__get_context_from_external_issues_references( closing_issues=[], closing_issue_numbers=[], issue_body='This references #999999', # Non-existent issue @@ -58,7 +61,9 @@ def test_handle_nonexistent_issue_reference(): def test_handle_rate_limit_error(): llm_config = LLMConfig(model='test', api_key='test') - handler = PRHandler('test-owner', 'test-repo', 'test-token', llm_config) + handler = ServiceContextPR( + GithubPRHandler('test-owner', 'test-repo', 'test-token'), llm_config + ) # Mock the requests.get to simulate a rate limit error mock_response = MagicMock() @@ -68,7 +73,7 @@ def test_handle_rate_limit_error(): with patch('requests.get', return_value=mock_response): # Call the method with an issue reference - result = handler._PRHandler__get_context_from_external_issues_references( + result = handler._ServiceContextPR__get_context_from_external_issues_references( closing_issues=[], closing_issue_numbers=[], issue_body='This references #123', @@ -83,14 +88,16 @@ def test_handle_rate_limit_error(): def test_handle_network_error(): llm_config = LLMConfig(model='test', api_key='test') - handler = PRHandler('test-owner', 'test-repo', 'test-token', llm_config) + handler = ServiceContextPR( + GithubPRHandler('test-owner', 'test-repo', 'test-token'), llm_config + ) # Mock the requests.get to simulate a network error with patch( 'requests.get', side_effect=requests.exceptions.ConnectionError('Network Error') ): # Call the method with an issue reference - result = handler._PRHandler__get_context_from_external_issues_references( + result = handler._ServiceContextPR__get_context_from_external_issues_references( closing_issues=[], closing_issue_numbers=[], issue_body='This references #123', @@ -105,7 +112,9 @@ def test_handle_network_error(): def test_successful_issue_reference(): llm_config = LLMConfig(model='test', api_key='test') - handler = PRHandler('test-owner', 'test-repo', 'test-token', llm_config) + handler = ServiceContextPR( + GithubPRHandler('test-owner', 'test-repo', 'test-token'), llm_config + ) # Mock a successful response mock_response = MagicMock() @@ -114,7 +123,7 @@ def test_successful_issue_reference(): with patch('requests.get', return_value=mock_response): # Call the method with an issue reference - result = handler._PRHandler__get_context_from_external_issues_references( + result = handler._ServiceContextPR__get_context_from_external_issues_references( closing_issues=[], closing_issue_numbers=[], issue_body='This references #123', @@ -201,11 +210,13 @@ def test_guess_success_rate_limit_wait_time(mock_litellm_completion, default_con ] llm = LLM(config=default_config) - handler = IssueHandler('test-owner', 'test-repo', 'test-token', default_config) + handler = ServiceContext( + GithubIssueHandler('test-owner', 'test-repo', 'test-token'), default_config + ) handler.llm = llm # Mock issue and history - issue = GithubIssue( + issue = Issue( owner='test-owner', repo='test-repo', number=1, @@ -241,11 +252,13 @@ def test_guess_success_exhausts_retries(mock_completion, default_config): # Initialize LLM and handler llm = LLM(config=default_config) - handler = PRHandler('test-owner', 'test-repo', 'test-token', default_config) + handler = ServiceContextPR( + GithubPRHandler('test-owner', 'test-repo', 'test-token'), default_config + ) handler.llm = llm # Mock issue and history - issue = GithubIssue( + issue = Issue( owner='test-owner', repo='test-repo', number=1, diff --git a/tests/unit/resolver/test_issue_references.py b/tests/unit/resolver/test_issue_references.py index 409f276d5abc..0a117492bf01 100644 --- a/tests/unit/resolver/test_issue_references.py +++ b/tests/unit/resolver/test_issue_references.py @@ -1,19 +1,15 @@ -from openhands.core.config.llm_config import LLMConfig -from openhands.resolver.issue_definitions import IssueHandler +from openhands.resolver.utils import extract_issue_references def test_extract_issue_references(): - llm_config = LLMConfig(model='test', api_key='test') - handler = IssueHandler('test-owner', 'test-repo', 'test-token', llm_config) - # Test basic issue reference - assert handler._extract_issue_references('Fixes #123') == [123] + assert extract_issue_references('Fixes #123') == [123] # Test multiple issue references - assert handler._extract_issue_references('Fixes #123, #456') == [123, 456] + assert extract_issue_references('Fixes #123, #456') == [123, 456] # Test issue references in code blocks should be ignored - assert handler._extract_issue_references(""" + assert extract_issue_references(""" Here's a code block: ```python # This is a comment with #123 @@ -24,21 +20,37 @@ def func(): """) == [789] # Test issue references in inline code should be ignored - assert handler._extract_issue_references( + assert extract_issue_references( + 'This `#123` should be ignored but #456 should be extracted' + ) == [456] + assert extract_issue_references( 'This `#123` should be ignored but #456 should be extracted' ) == [456] # Test issue references in URLs should be ignored - assert handler._extract_issue_references( + assert extract_issue_references( + 'Check http://example.com/#123 but #456 should be extracted' + ) == [456] + assert extract_issue_references( 'Check http://example.com/#123 but #456 should be extracted' ) == [456] # Test issue references in markdown links should be extracted - assert handler._extract_issue_references( - '[Link to #123](http://example.com) and #456' - ) == [123, 456] + assert extract_issue_references('[Link to #123](http://example.com) and #456') == [ + 123, + 456, + ] + assert extract_issue_references('[Link to #123](http://example.com) and #456') == [ + 123, + 456, + ] # Test issue references with text around them - assert handler._extract_issue_references( - 'Issue #123 is fixed and #456 is pending' - ) == [123, 456] + assert extract_issue_references('Issue #123 is fixed and #456 is pending') == [ + 123, + 456, + ] + assert extract_issue_references('Issue #123 is fixed and #456 is pending') == [ + 123, + 456, + ] diff --git a/tests/unit/resolver/test_pr_handler_guess_success.py b/tests/unit/resolver/test_pr_handler_guess_success.py index de0495691529..5d7a7418fd3e 100644 --- a/tests/unit/resolver/test_pr_handler_guess_success.py +++ b/tests/unit/resolver/test_pr_handler_guess_success.py @@ -4,8 +4,9 @@ from openhands.core.config import LLMConfig from openhands.events.action.message import MessageAction from openhands.llm.llm import LLM -from openhands.resolver.github_issue import GithubIssue, ReviewThread -from openhands.resolver.issue_definitions import PRHandler +from openhands.resolver.github import GithubPRHandler +from openhands.resolver.issue import Issue, ReviewThread +from openhands.resolver.issue_definitions import ServiceContextPR def mock_llm_response(content): @@ -19,10 +20,12 @@ def test_guess_success_review_threads_litellm_call(): """Test that the litellm.completion() call for review threads contains the expected content.""" # Create a PR handler instance llm_config = LLMConfig(model='test', api_key='test') - handler = PRHandler('test-owner', 'test-repo', 'test-token', llm_config) + handler = ServiceContextPR( + GithubPRHandler('test-owner', 'test-repo', 'test-token'), llm_config + ) # Create a mock issue with review threads - issue = GithubIssue( + issue = Issue( owner='test-owner', repo='test-repo', number=1, @@ -122,10 +125,12 @@ def test_guess_success_thread_comments_litellm_call(): """Test that the litellm.completion() call for thread comments contains the expected content.""" # Create a PR handler instance llm_config = LLMConfig(model='test', api_key='test') - handler = PRHandler('test-owner', 'test-repo', 'test-token', llm_config) + handler = ServiceContextPR( + GithubPRHandler('test-owner', 'test-repo', 'test-token'), llm_config + ) # Create a mock issue with thread comments - issue = GithubIssue( + issue = Issue( owner='test-owner', repo='test-repo', number=1, @@ -193,7 +198,9 @@ def test_check_feedback_with_llm(): """Test the _check_feedback_with_llm helper function.""" # Create a PR handler instance llm_config = LLMConfig(model='test', api_key='test') - handler = PRHandler('test-owner', 'test-repo', 'test-token', llm_config) + handler = ServiceContextPR( + GithubPRHandler('test-owner', 'test-repo', 'test-token'), llm_config + ) # Test cases for different LLM responses test_cases = [ @@ -233,7 +240,9 @@ def test_check_review_thread(): """Test the _check_review_thread helper function.""" # Create a PR handler instance llm_config = LLMConfig(model='test', api_key='test') - handler = PRHandler('test-owner', 'test-repo', 'test-token', llm_config) + handler = ServiceContextPR( + GithubPRHandler('test-owner', 'test-repo', 'test-token'), llm_config + ) # Create test data review_thread = ReviewThread( @@ -288,7 +297,9 @@ def test_check_thread_comments(): """Test the _check_thread_comments helper function.""" # Create a PR handler instance llm_config = LLMConfig(model='test', api_key='test') - handler = PRHandler('test-owner', 'test-repo', 'test-token', llm_config) + handler = ServiceContextPR( + GithubPRHandler('test-owner', 'test-repo', 'test-token'), llm_config + ) # Create test data thread_comments = [ @@ -341,7 +352,9 @@ def test_check_review_comments(): """Test the _check_review_comments helper function.""" # Create a PR handler instance llm_config = LLMConfig(model='test', api_key='test') - handler = PRHandler('test-owner', 'test-repo', 'test-token', llm_config) + handler = ServiceContextPR( + GithubPRHandler('test-owner', 'test-repo', 'test-token'), llm_config + ) # Create test data review_comments = [ @@ -394,10 +407,12 @@ def test_guess_success_review_comments_litellm_call(): """Test that the litellm.completion() call for review comments contains the expected content.""" # Create a PR handler instance llm_config = LLMConfig(model='test', api_key='test') - handler = PRHandler('test-owner', 'test-repo', 'test-token', llm_config) + handler = ServiceContextPR( + GithubPRHandler('test-owner', 'test-repo', 'test-token'), llm_config + ) # Create a mock issue with review comments - issue = GithubIssue( + issue = Issue( owner='test-owner', repo='test-repo', number=1, @@ -438,7 +453,6 @@ def test_guess_success_review_comments_litellm_call(): ) ] - # Test the guess_success method with patch.object(LLM, 'completion') as mock_completion: mock_completion.return_value = mock_response success, success_list, explanation = handler.guess_success(issue, history) @@ -456,3 +470,5 @@ def test_guess_success_review_comments_litellm_call(): ) assert 'PR Review Comments:\n' + '\n---\n'.join(issue.review_comments) in prompt assert 'Last message from AI agent:\n' + history[0].content in prompt + + assert len(json.loads(explanation)) == 1 diff --git a/tests/unit/resolver/test_pr_title_escaping.py b/tests/unit/resolver/test_pr_title_escaping.py index 45dd523b036a..f5d04fd2655a 100644 --- a/tests/unit/resolver/test_pr_title_escaping.py +++ b/tests/unit/resolver/test_pr_title_escaping.py @@ -2,7 +2,7 @@ import subprocess import tempfile -from openhands.resolver.github_issue import GithubIssue +from openhands.resolver.issue import Issue from openhands.resolver.send_pull_request import make_commit @@ -19,7 +19,7 @@ def test_commit_message_with_quotes(): subprocess.run(['git', '-C', temp_dir, 'add', 'test.txt'], check=True) # Create a test issue with problematic title - issue = GithubIssue( + issue = Issue( owner='test-owner', repo='test-repo', number=123, @@ -89,7 +89,7 @@ def raise_for_status(self): monkeypatch.setattr('requests.post', mock_post) monkeypatch.setattr('requests.get', lambda *args, **kwargs: MockGetResponse()) monkeypatch.setattr( - 'openhands.resolver.send_pull_request.branch_exists', + 'openhands.resolver.github.GithubIssueHandler.branch_exists', lambda *args, **kwargs: False, ) @@ -135,7 +135,7 @@ def mock_run(*args, **kwargs): # Create a test issue with problematic title print('Creating test issue...') - issue = GithubIssue( + issue = Issue( owner='test-owner', repo='test-repo', number=123, @@ -157,9 +157,9 @@ def mock_run(*args, **kwargs): from openhands.resolver.send_pull_request import send_pull_request send_pull_request( - github_issue=issue, - github_token='dummy-token', - github_username='test-user', + issue=issue, + token='dummy-token', + username='test-user', patch_dir=temp_dir, llm_config=LLMConfig(model='test-model', api_key='test-key'), pr_type='ready', diff --git a/tests/unit/resolver/test_resolve_issues.py b/tests/unit/resolver/test_resolve_issues.py index 8d54adb87627..efc7dddff4d0 100644 --- a/tests/unit/resolver/test_resolve_issues.py +++ b/tests/unit/resolver/test_resolve_issues.py @@ -8,8 +8,9 @@ from openhands.events.action import CmdRunAction from openhands.events.observation import CmdOutputObservation, NullObservation from openhands.llm.llm import LLM -from openhands.resolver.github_issue import GithubIssue, ReviewThread -from openhands.resolver.issue_definitions import IssueHandler, PRHandler +from openhands.resolver.github import GithubIssueHandler, GithubPRHandler +from openhands.resolver.issue import Issue, ReviewThread +from openhands.resolver.issue_definitions import ServiceContext, ServiceContextPR from openhands.resolver.resolve_issue import ( complete_runtime, initialize_runtime, @@ -84,9 +85,9 @@ def test_initialize_runtime(): ) -def test_download_issues_from_github(): +def test_download_issues(): llm_config = LLMConfig(model='test', api_key='test') - handler = IssueHandler('owner', 'repo', 'token', llm_config) + handler = ServiceContext(GithubIssueHandler('owner', 'repo', 'token'), llm_config) mock_issues_response = MagicMock() mock_issues_response.json.side_effect = [ @@ -118,7 +119,7 @@ def get_mock_response(url, *args, **kwargs): assert len(issues) == 2 assert handler.issue_type == 'issue' - assert all(isinstance(issue, GithubIssue) for issue in issues) + assert all(isinstance(issue, Issue) for issue in issues) assert [issue.number for issue in issues] == [1, 3] assert [issue.title for issue in issues] == ['Issue 1', 'Issue 2'] assert [issue.review_comments for issue in issues] == [None, None] @@ -126,9 +127,9 @@ def get_mock_response(url, *args, **kwargs): assert [issue.thread_ids for issue in issues] == [None, None] -def test_download_pr_from_github(): +def test_download_pr(): llm_config = LLMConfig(model='test', api_key='test') - handler = PRHandler('owner', 'repo', 'token', llm_config) + handler = ServiceContextPR(GithubPRHandler('owner', 'repo', 'token'), llm_config) mock_pr_response = MagicMock() mock_pr_response.json.side_effect = [ [ @@ -232,7 +233,7 @@ def get_mock_response(url, *args, **kwargs): assert len(issues) == 3 assert handler.issue_type == 'pr' - assert all(isinstance(issue, GithubIssue) for issue in issues) + assert all(isinstance(issue, Issue) for issue in issues) assert [issue.number for issue in issues] == [1, 2, 3] assert [issue.title for issue in issues] == ['PR 1', 'My PR', 'PR 3'] assert [issue.head_branch for issue in issues] == ['b1', 'b2', 'b3'] @@ -298,7 +299,7 @@ async def test_process_issue(mock_output_dir, mock_prompt_template): handler_instance = MagicMock() # Set up test data - issue = GithubIssue( + issue = Issue( owner='test_owner', repo='test_repo', number=1, @@ -438,7 +439,7 @@ async def test_process_issue(mock_output_dir, mock_prompt_template): def test_get_instruction(mock_prompt_template, mock_followup_prompt_template): - issue = GithubIssue( + issue = Issue( owner='test_owner', repo='test_repo', number=123, @@ -446,7 +447,9 @@ def test_get_instruction(mock_prompt_template, mock_followup_prompt_template): body='This is a test issue refer to image ![First Image](https://sampleimage.com/image1.png)', ) mock_llm_config = LLMConfig(model='test_model', api_key='test_api_key') - issue_handler = IssueHandler('owner', 'repo', 'token', mock_llm_config) + issue_handler = ServiceContext( + GithubIssueHandler('owner', 'repo', 'token'), mock_llm_config + ) instruction, images_urls = issue_handler.get_instruction( issue, mock_prompt_template, None ) @@ -456,7 +459,7 @@ def test_get_instruction(mock_prompt_template, mock_followup_prompt_template): assert issue_handler.issue_type == 'issue' assert instruction == expected_instruction - issue = GithubIssue( + issue = Issue( owner='test_owner', repo='test_repo', number=123, @@ -474,7 +477,9 @@ def test_get_instruction(mock_prompt_template, mock_followup_prompt_template): ], ) - pr_handler = PRHandler('owner', 'repo', 'token', mock_llm_config) + pr_handler = ServiceContextPR( + GithubPRHandler('owner', 'repo', 'token'), mock_llm_config + ) instruction, images_urls = pr_handler.get_instruction( issue, mock_followup_prompt_template, None ) @@ -486,7 +491,7 @@ def test_get_instruction(mock_prompt_template, mock_followup_prompt_template): def test_file_instruction(): - issue = GithubIssue( + issue = Issue( owner='test_owner', repo='test_repo', number=123, @@ -498,7 +503,9 @@ def test_file_instruction(): prompt = f.read() # Test without thread comments mock_llm_config = LLMConfig(model='test_model', api_key='test_api_key') - issue_handler = IssueHandler('owner', 'repo', 'token', mock_llm_config) + issue_handler = ServiceContext( + GithubIssueHandler('owner', 'repo', 'token'), mock_llm_config + ) instruction, images_urls = issue_handler.get_instruction(issue, prompt, None) expected_instruction = """Please fix the following issue for the repository in /workspace. An environment has been set up for you to start working. You may assume all necessary tools are installed. @@ -518,7 +525,7 @@ def test_file_instruction(): def test_file_instruction_with_repo_instruction(): - issue = GithubIssue( + issue = Issue( owner='test_owner', repo='test_repo', number=123, @@ -536,7 +543,9 @@ def test_file_instruction_with_repo_instruction(): repo_instruction = f.read() mock_llm_config = LLMConfig(model='test_model', api_key='test_api_key') - issue_handler = IssueHandler('owner', 'repo', 'token', mock_llm_config) + issue_handler = ServiceContext( + GithubIssueHandler('owner', 'repo', 'token'), mock_llm_config + ) instruction, image_urls = issue_handler.get_instruction( issue, prompt, repo_instruction ) @@ -565,7 +574,7 @@ def test_file_instruction_with_repo_instruction(): def test_guess_success(): - mock_issue = GithubIssue( + mock_issue = Issue( owner='test_owner', repo='test_repo', number=1, @@ -587,7 +596,9 @@ def test_guess_success(): ) ) ] - issue_handler = IssueHandler('owner', 'repo', 'token', mock_llm_config) + issue_handler = ServiceContext( + GithubIssueHandler('owner', 'repo', 'token'), mock_llm_config + ) with patch.object( LLM, 'completion', MagicMock(return_value=mock_completion_response) @@ -602,7 +613,7 @@ def test_guess_success(): def test_guess_success_with_thread_comments(): - mock_issue = GithubIssue( + mock_issue = Issue( owner='test_owner', repo='test_repo', number=1, @@ -625,7 +636,9 @@ def test_guess_success_with_thread_comments(): ) ) ] - issue_handler = IssueHandler('owner', 'repo', 'token', mock_llm_config) + issue_handler = ServiceContext( + GithubIssueHandler('owner', 'repo', 'token'), mock_llm_config + ) with patch.object( LLM, 'completion', MagicMock(return_value=mock_completion_response) @@ -641,7 +654,7 @@ def test_guess_success_with_thread_comments(): def test_instruction_with_thread_comments(): # Create an issue with thread comments - issue = GithubIssue( + issue = Issue( owner='test_owner', repo='test_repo', number=123, @@ -659,7 +672,9 @@ def test_instruction_with_thread_comments(): prompt = f.read() llm_config = LLMConfig(model='test', api_key='test') - issue_handler = IssueHandler('owner', 'repo', 'token', llm_config) + issue_handler = ServiceContext( + GithubIssueHandler('owner', 'repo', 'token'), llm_config + ) instruction, images_urls = issue_handler.get_instruction(issue, prompt, None) # Verify that thread comments are included in the instruction @@ -671,7 +686,7 @@ def test_instruction_with_thread_comments(): def test_guess_success_failure(): - mock_issue = GithubIssue( + mock_issue = Issue( owner='test_owner', repo='test_repo', number=1, @@ -694,7 +709,9 @@ def test_guess_success_failure(): ) ) ] - issue_handler = IssueHandler('owner', 'repo', 'token', mock_llm_config) + issue_handler = ServiceContext( + GithubIssueHandler('owner', 'repo', 'token'), mock_llm_config + ) with patch.object( LLM, 'completion', MagicMock(return_value=mock_completion_response) @@ -709,7 +726,7 @@ def test_guess_success_failure(): def test_guess_success_negative_case(): - mock_issue = GithubIssue( + mock_issue = Issue( owner='test_owner', repo='test_repo', number=1, @@ -731,7 +748,9 @@ def test_guess_success_negative_case(): ) ) ] - issue_handler = IssueHandler('owner', 'repo', 'token', mock_llm_config) + issue_handler = ServiceContext( + GithubIssueHandler('owner', 'repo', 'token'), mock_llm_config + ) with patch.object( LLM, 'completion', MagicMock(return_value=mock_completion_response) @@ -746,7 +765,7 @@ def test_guess_success_negative_case(): def test_guess_success_invalid_output(): - mock_issue = GithubIssue( + mock_issue = Issue( owner='test_owner', repo='test_repo', number=1, @@ -764,7 +783,9 @@ def test_guess_success_invalid_output(): mock_completion_response.choices = [ MagicMock(message=MagicMock(content='This is not a valid output')) ] - issue_handler = IssueHandler('owner', 'repo', 'token', mock_llm_config) + issue_handler = ServiceContext( + GithubIssueHandler('owner', 'repo', 'token'), mock_llm_config + ) with patch.object( LLM, 'completion', MagicMock(return_value=mock_completion_response) @@ -783,7 +804,7 @@ def test_guess_success_invalid_output(): def test_download_pr_with_review_comments(): llm_config = LLMConfig(model='test', api_key='test') - handler = PRHandler('owner', 'repo', 'token', llm_config) + handler = ServiceContextPR(GithubPRHandler('owner', 'repo', 'token'), llm_config) mock_pr_response = MagicMock() mock_pr_response.json.side_effect = [ [ @@ -834,7 +855,7 @@ def get_mock_response(url, *args, **kwargs): assert len(issues) == 1 assert handler.issue_type == 'pr' - assert isinstance(issues[0], GithubIssue) + assert isinstance(issues[0], Issue) assert issues[0].number == 1 assert issues[0].title == 'PR 1' assert issues[0].head_branch == 'b1' @@ -850,7 +871,7 @@ def get_mock_response(url, *args, **kwargs): def test_download_issue_with_specific_comment(): llm_config = LLMConfig(model='test', api_key='test') - handler = IssueHandler('owner', 'repo', 'token', llm_config) + handler = ServiceContext(GithubIssueHandler('owner', 'repo', 'token'), llm_config) # Define the specific comment_id to filter specific_comment_id = 101 diff --git a/tests/unit/resolver/test_send_pull_request.py b/tests/unit/resolver/test_send_pull_request.py index f83e2e97ec2f..f970e99fa38e 100644 --- a/tests/unit/resolver/test_send_pull_request.py +++ b/tests/unit/resolver/test_send_pull_request.py @@ -1,3 +1,4 @@ + import os import tempfile from unittest.mock import MagicMock, call, patch @@ -5,8 +6,9 @@ import pytest from openhands.core.config import LLMConfig -from openhands.resolver.github_issue import ReviewThread -from openhands.resolver.resolver_output import GithubIssue, ResolverOutput +from openhands.resolver.issue import ReviewThread +from openhands.resolver.github import GithubIssueHandler +from openhands.resolver.resolver_output import Issue, ResolverOutput from openhands.resolver.send_pull_request import ( apply_patch, initialize_repo, @@ -14,7 +16,6 @@ make_commit, process_all_successful_issues, process_single_issue, - reply_to_comment, send_pull_request, update_existing_pull_request, ) @@ -36,8 +37,8 @@ def mock_output_dir(): @pytest.fixture -def mock_github_issue(): - return GithubIssue( +def mock_issue(): + return Issue( number=42, title='Test Issue', owner='test-owner', @@ -241,14 +242,14 @@ def test_initialize_repo(mock_output_dir): assert f.read() == 'hello world' -@patch('openhands.resolver.send_pull_request.reply_to_comment') +@patch('openhands.resolver.github.GithubIssueHandler.reply_to_comment') @patch('requests.post') @patch('subprocess.run') def test_update_existing_pull_request( mock_subprocess_run, mock_requests_post, mock_reply_to_comment ): # Arrange: Set up test data - github_issue = GithubIssue( + issue = Issue( owner='test-owner', repo='test-repo', number=1, @@ -257,8 +258,8 @@ def test_update_existing_pull_request( thread_ids=['comment1', 'comment2'], head_branch='test-branch', ) - github_token = 'test-token' - github_username = 'test-user' + token = 'test-token' + username = 'test-user' patch_dir = '/path/to/patch' additional_message = '["Fixed bug in function A", "Updated documentation for B"]' @@ -276,9 +277,9 @@ def test_update_existing_pull_request( # Act: Call the function without comment_message to test auto-generation with patch('litellm.completion', MagicMock(return_value=mock_completion_response)): result = update_existing_pull_request( - github_issue, - github_token, - github_username, + issue, + token, + username, patch_dir, llm_config, comment_message=None, @@ -288,20 +289,20 @@ def test_update_existing_pull_request( # Assert: Check if the git push command was executed push_command = ( f'git -C {patch_dir} push ' - f'https://{github_username}:{github_token}@github.com/' - f'{github_issue.owner}/{github_issue.repo}.git {github_issue.head_branch}' + f'https://{username}:{token}@github.com/' + f'{issue.owner}/{issue.repo}.git {issue.head_branch}' ) mock_subprocess_run.assert_called_once_with( push_command, shell=True, capture_output=True, text=True ) # Assert: Check if the auto-generated comment was posted to the PR - comment_url = f'https://api.github.com/repos/{github_issue.owner}/{github_issue.repo}/issues/{github_issue.number}/comments' + comment_url = f'https://api.github.com/repos/{issue.owner}/{issue.repo}/issues/{issue.number}/comments' expected_comment = 'This is an issue resolution.' mock_requests_post.assert_called_once_with( comment_url, headers={ - 'Authorization': f'token {github_token}', + 'Authorization': f'token {token}', 'Accept': 'application/vnd.github.v3+json', }, json={'body': expected_comment}, @@ -310,15 +311,14 @@ def test_update_existing_pull_request( # Assert: Check if the reply_to_comment function was called for each thread ID mock_reply_to_comment.assert_has_calls( [ - call(github_token, 'comment1', 'Fixed bug in function A'), - call(github_token, 'comment2', 'Updated documentation for B'), + call(token, 'comment1', 'Fixed bug in function A'), + call(token, 'comment2', 'Updated documentation for B'), ] ) # Assert: Check the returned PR URL assert ( - result - == f'https://github.com/{github_issue.owner}/{github_issue.repo}/pull/{github_issue.number}' + result == f'https://github.com/{issue.owner}/{issue.repo}/pull/{issue.number}' ) @@ -340,7 +340,7 @@ def test_send_pull_request( mock_get, mock_post, mock_run, - mock_github_issue, + mock_issue, mock_output_dir, mock_llm_config, pr_type, @@ -372,9 +372,9 @@ def test_send_pull_request( # Call the function result = send_pull_request( - github_issue=mock_github_issue, - github_token='test-token', - github_username='test-user', + issue=mock_issue, + token='test-token', + username='test-user', patch_dir=repo_path, pr_type=pr_type, llm_config=mock_llm_config, @@ -427,7 +427,7 @@ def test_send_pull_request( @patch('requests.get') def test_send_pull_request_invalid_target_branch( - mock_get, mock_github_issue, mock_output_dir, mock_llm_config + mock_get, mock_issue, mock_output_dir, mock_llm_config ): """Test that an error is raised when specifying a non-existent target branch""" repo_path = os.path.join(mock_output_dir, 'repo') @@ -443,9 +443,9 @@ def test_send_pull_request_invalid_target_branch( ValueError, match='Target branch nonexistent-branch does not exist' ): send_pull_request( - github_issue=mock_github_issue, - github_token='test-token', - github_username='test-user', + issue=mock_issue, + token='test-token', + username='test-user', patch_dir=repo_path, pr_type='ready', llm_config=mock_llm_config, @@ -460,7 +460,7 @@ def test_send_pull_request_invalid_target_branch( @patch('requests.post') @patch('requests.get') def test_send_pull_request_git_push_failure( - mock_get, mock_post, mock_run, mock_github_issue, mock_output_dir, mock_llm_config + mock_get, mock_post, mock_run, mock_issue, mock_output_dir, mock_llm_config ): repo_path = os.path.join(mock_output_dir, 'repo') @@ -478,9 +478,9 @@ def test_send_pull_request_git_push_failure( RuntimeError, match='Failed to push changes to the remote repository' ): send_pull_request( - github_issue=mock_github_issue, - github_token='test-token', - github_username='test-user', + issue=mock_issue, + token='test-token', + username='test-user', patch_dir=repo_path, pr_type='ready', llm_config=mock_llm_config, @@ -519,7 +519,7 @@ def test_send_pull_request_git_push_failure( @patch('requests.post') @patch('requests.get') def test_send_pull_request_permission_error( - mock_get, mock_post, mock_run, mock_github_issue, mock_output_dir, mock_llm_config + mock_get, mock_post, mock_run, mock_issue, mock_output_dir, mock_llm_config ): repo_path = os.path.join(mock_output_dir, 'repo') @@ -538,9 +538,9 @@ def test_send_pull_request_permission_error( RuntimeError, match='Failed to create pull request due to missing permissions.' ): send_pull_request( - github_issue=mock_github_issue, - github_token='test-token', - github_username='test-user', + issue=mock_issue, + token='test-token', + username='test-user', patch_dir=repo_path, pr_type='ready', llm_config=mock_llm_config, @@ -554,10 +554,18 @@ def test_send_pull_request_permission_error( @patch('requests.post') def test_reply_to_comment(mock_post): # Arrange: set up the test data - github_token = 'test_token' + token = 'test_token' comment_id = 'test_comment_id' reply = 'This is a test reply.' + # Create an instance of GithubIssueHandler + handler = GithubIssueHandler( + owner='test-owner', + repo='test-repo', + token=token, + username='test-user' + ) + # Mock the response from the GraphQL API mock_response = MagicMock() mock_response.status_code = 200 @@ -576,37 +584,45 @@ def test_reply_to_comment(mock_post): mock_post.return_value = mock_response # Act: call the function - reply_to_comment(github_token, comment_id, reply) + handler.reply_to_comment(token, comment_id, reply) # Assert: check that the POST request was made with the correct parameters - query = """ - mutation($body: String!, $pullRequestReviewThreadId: ID!) { - addPullRequestReviewThreadReply(input: { body: $body, pullRequestReviewThreadId: $pullRequestReviewThreadId }) { - comment { - id - body - createdAt - } + # Assert: check that the POST request was made with the correct parameters + expected_query = """ + mutation($body: String!, $pullRequestReviewThreadId: ID!) { + addPullRequestReviewThreadReply(input: { body: $body, pullRequestReviewThreadId: $pullRequestReviewThreadId }) { + comment { + id + body + createdAt } } - """ + } + """ - expected_variables = { + # Normalize whitespace in both expected and actual queries + def normalize_whitespace(s): + return ' '.join(s.split()) + + # Check if the normalized actual query matches the normalized expected query + mock_post.assert_called_once() + actual_call = mock_post.call_args + actual_query = actual_call[1]['json']['query'] + + assert normalize_whitespace(actual_query) == normalize_whitespace(expected_query) + + # Check the variables and headers separately + assert actual_call[1]['json']['variables'] == { 'body': 'Openhands fix success summary\n\n\nThis is a test reply.', 'pullRequestReviewThreadId': comment_id, } - # Check that the correct request was made to the API - mock_post.assert_called_once_with( - 'https://api.github.com/graphql', - json={'query': query, 'variables': expected_variables}, - headers={ - 'Authorization': f'Bearer {github_token}', - 'Content-Type': 'application/json', - }, - ) + assert actual_call[1]['headers'] == { + 'Authorization': f'Bearer {token}', + 'Content-Type': 'application/json', + } - # Check that the response status was checked (via response.raise_for_status) + # Check that the response status was checked mock_response.raise_for_status.assert_called_once() @@ -623,12 +639,12 @@ def test_process_single_pr_update( mock_llm_config, ): # Initialize test data - github_token = 'test_token' - github_username = 'test_user' + token = 'test_token' + username = 'test_user' pr_type = 'draft' resolver_output = ResolverOutput( - issue=GithubIssue( + issue=Issue( owner='test-owner', repo='test-repo', number=1, @@ -661,8 +677,8 @@ def test_process_single_pr_update( process_single_issue( mock_output_dir, resolver_output, - github_token, - github_username, + token, + username, pr_type, mock_llm_config, None, @@ -678,9 +694,9 @@ def test_process_single_pr_update( f'{mock_output_dir}/patches/pr_1', resolver_output.issue, 'pr' ) mock_update_existing_pull_request.assert_called_once_with( - github_issue=resolver_output.issue, - github_token=github_token, - github_username=github_username, + issue=resolver_output.issue, + token=token, + username=username, patch_dir=f'{mock_output_dir}/patches/pr_1', additional_message='[Test success 1]', llm_config=mock_llm_config, @@ -700,12 +716,12 @@ def test_process_single_issue( mock_llm_config, ): # Initialize test data - github_token = 'test_token' - github_username = 'test_user' + token = 'test_token' + username = 'test_user' pr_type = 'draft' resolver_output = ResolverOutput( - issue=GithubIssue( + issue=Issue( owner='test-owner', repo='test-repo', number=1, @@ -734,8 +750,8 @@ def test_process_single_issue( process_single_issue( mock_output_dir, resolver_output, - github_token, - github_username, + token, + username, pr_type, mock_llm_config, None, @@ -752,9 +768,9 @@ def test_process_single_issue( f'{mock_output_dir}/patches/issue_1', resolver_output.issue, 'issue' ) mock_send_pull_request.assert_called_once_with( - github_issue=resolver_output.issue, - github_token=github_token, - github_username=github_username, + issue=resolver_output.issue, + token=token, + username=username, patch_dir=f'{mock_output_dir}/patches/issue_1', pr_type=pr_type, llm_config=mock_llm_config, @@ -777,12 +793,12 @@ def test_process_single_issue_unsuccessful( mock_llm_config, ): # Initialize test data - github_token = 'test_token' - github_username = 'test_user' + token = 'test_token' + username = 'test_user' pr_type = 'draft' resolver_output = ResolverOutput( - issue=GithubIssue( + issue=Issue( owner='test-owner', repo='test-repo', number=1, @@ -805,8 +821,8 @@ def test_process_single_issue_unsuccessful( process_single_issue( mock_output_dir, resolver_output, - github_token, - github_username, + token, + username, pr_type, mock_llm_config, None, @@ -826,9 +842,9 @@ def test_process_single_issue_unsuccessful( def test_process_all_successful_issues( mock_process_single_issue, mock_load_all_resolver_outputs, mock_llm_config ): - # Create ResolverOutput objects with properly initialized GithubIssue instances + # Create ResolverOutput objects with properly initialized Issue instances resolver_output_1 = ResolverOutput( - issue=GithubIssue( + issue=Issue( owner='test-owner', repo='test-repo', number=1, @@ -848,7 +864,7 @@ def test_process_all_successful_issues( ) resolver_output_2 = ResolverOutput( - issue=GithubIssue( + issue=Issue( owner='test-owner', repo='test-repo', number=2, @@ -868,7 +884,7 @@ def test_process_all_successful_issues( ) resolver_output_3 = ResolverOutput( - issue=GithubIssue( + issue=Issue( owner='test-owner', repo='test-repo', number=3, @@ -896,8 +912,8 @@ def test_process_all_successful_issues( # Call the function process_all_successful_issues( 'output_dir', - 'github_token', - 'github_username', + 'token', + 'username', 'draft', mock_llm_config, # llm_config None, # fork_owner @@ -912,8 +928,8 @@ def test_process_all_successful_issues( call( 'output_dir', resolver_output_1, - 'github_token', - 'github_username', + 'token', + 'username', 'draft', mock_llm_config, None, @@ -923,8 +939,8 @@ def test_process_all_successful_issues( call( 'output_dir', resolver_output_3, - 'github_token', - 'github_username', + 'token', + 'username', 'draft', mock_llm_config, None, @@ -940,7 +956,7 @@ def test_process_all_successful_issues( @patch('requests.get') @patch('subprocess.run') def test_send_pull_request_branch_naming( - mock_run, mock_get, mock_github_issue, mock_output_dir, mock_llm_config + mock_run, mock_get, mock_issue, mock_output_dir, mock_llm_config ): repo_path = os.path.join(mock_output_dir, 'repo') @@ -960,9 +976,9 @@ def test_send_pull_request_branch_naming( # Call the function result = send_pull_request( - github_issue=mock_github_issue, - github_token='test-token', - github_username='test-user', + issue=mock_issue, + token='test-token', + username='test-user', patch_dir=repo_path, pr_type='branch', llm_config=mock_llm_config, @@ -1018,8 +1034,8 @@ def test_main( # Setup mock parser mock_args = MagicMock() - mock_args.github_token = None - mock_args.github_username = 'mock_username' + mock_args.token = None + mock_args.username = 'mock_username' mock_args.output_dir = '/mock/output' mock_args.pr_type = 'draft' mock_args.issue_number = '42' @@ -1093,7 +1109,7 @@ def test_main( def test_make_commit_escapes_issue_title(mock_subprocess_run): # Setup repo_dir = '/path/to/repo' - issue = GithubIssue( + issue = Issue( owner='test-owner', repo='test-repo', number=42, @@ -1133,7 +1149,7 @@ def test_make_commit_escapes_issue_title(mock_subprocess_run): def test_make_commit_no_changes(mock_subprocess_run): # Setup repo_dir = '/path/to/repo' - issue = GithubIssue( + issue = Issue( owner='test-owner', repo='test-repo', number=42,