Skip to content

Commit a1f1c80

Browse files
[Fix]: Fix bugs for target_branch param on resolver (#5745)
Co-authored-by: openhands <openhands@all-hands.dev>
1 parent ad2237d commit a1f1c80

File tree

2 files changed

+108
-9
lines changed

2 files changed

+108
-9
lines changed

openhands/resolver/resolve_issue.py

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -307,7 +307,6 @@ async def resolve_issue(
307307
repo_instruction: str | None,
308308
issue_number: int,
309309
comment_id: int | None,
310-
target_branch: str | None = None,
311310
reset_logger: bool = False,
312311
) -> None:
313312
"""Resolve a single github issue.
@@ -326,7 +325,7 @@ async def resolve_issue(
326325
repo_instruction: Repository instruction to use.
327326
issue_number: Issue number to resolve.
328327
comment_id: Optional ID of a specific comment to focus on.
329-
target_branch: Optional target branch to create PR against (for PRs).
328+
330329
reset_logger: Whether to reset the logger for multiprocessing.
331330
"""
332331
issue_handler = issue_handler_factory(issue_type, owner, repo, token, llm_config)
@@ -424,9 +423,9 @@ async def resolve_issue(
424423
try:
425424
# checkout to pr branch if needed
426425
if issue_type == 'pr':
427-
branch_to_use = target_branch if target_branch else issue.head_branch
426+
branch_to_use = issue.head_branch
428427
logger.info(
429-
f'Checking out to PR branch {target_branch} for issue {issue.number}'
428+
f'Checking out to PR branch {branch_to_use} for issue {issue.number}'
430429
)
431430

432431
if not branch_to_use:
@@ -446,10 +445,6 @@ async def resolve_issue(
446445
cwd=repo_dir,
447446
)
448447

449-
# Update issue's base_branch if using custom target branch
450-
if target_branch:
451-
issue.base_branch = target_branch
452-
453448
base_commit = (
454449
subprocess.check_output(['git', 'rev-parse', 'HEAD'], cwd=repo_dir)
455450
.decode('utf-8')
@@ -644,7 +639,6 @@ def int_or_none(value):
644639
repo_instruction=repo_instruction,
645640
issue_number=my_args.issue_number,
646641
comment_id=my_args.comment_id,
647-
target_branch=my_args.target_branch,
648642
)
649643
)
650644

tests/unit/resolver/test_send_pull_request.py

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -500,6 +500,111 @@ def test_send_pull_request_with_reviewer(
500500
assert result == 'https://github.com/test-owner/test-repo/pull/1'
501501

502502

503+
@patch('subprocess.run')
504+
@patch('requests.post')
505+
@patch('requests.get')
506+
def test_send_pull_request_target_branch_with_fork(
507+
mock_get, mock_post, mock_run, mock_github_issue, mock_output_dir
508+
):
509+
"""Test that target_branch works correctly when using a fork."""
510+
repo_path = os.path.join(mock_output_dir, 'repo')
511+
fork_owner = 'fork-owner'
512+
target_branch = 'custom-target'
513+
514+
# Mock API responses
515+
mock_get.side_effect = [
516+
MagicMock(status_code=404), # Branch doesn't exist
517+
MagicMock(status_code=200), # Target branch exists
518+
]
519+
520+
mock_post.return_value.json.return_value = {
521+
'html_url': 'https://github.com/test-owner/test-repo/pull/1'
522+
}
523+
524+
# Mock subprocess.run calls
525+
mock_run.side_effect = [
526+
MagicMock(returncode=0), # git checkout -b
527+
MagicMock(returncode=0), # git push
528+
]
529+
530+
# Call the function with fork_owner and target_branch
531+
result = send_pull_request(
532+
github_issue=mock_github_issue,
533+
github_token='test-token',
534+
github_username='test-user',
535+
patch_dir=repo_path,
536+
pr_type='ready',
537+
fork_owner=fork_owner,
538+
target_branch=target_branch,
539+
)
540+
541+
# Assert API calls
542+
assert mock_get.call_count == 2
543+
544+
# Verify target branch was checked in original repo, not fork
545+
target_branch_check = mock_get.call_args_list[1]
546+
assert target_branch_check[0][0] == f'https://api.github.com/repos/test-owner/test-repo/branches/{target_branch}'
547+
548+
# Check PR creation
549+
mock_post.assert_called_once()
550+
post_data = mock_post.call_args[1]['json']
551+
assert post_data['base'] == target_branch # PR should target the specified branch
552+
assert post_data['head'] == 'openhands-fix-issue-42' # Branch name should be standard
553+
554+
# Check that push was to fork
555+
push_call = mock_run.call_args_list[1]
556+
assert f'https://test-user:test-token@github.com/{fork_owner}/test-repo.git' in str(push_call)
557+
558+
559+
@patch('subprocess.run')
560+
@patch('requests.post')
561+
@patch('requests.get')
562+
def test_send_pull_request_target_branch_with_additional_message(
563+
mock_get, mock_post, mock_run, mock_github_issue, mock_output_dir
564+
):
565+
"""Test that target_branch works correctly with additional PR message."""
566+
repo_path = os.path.join(mock_output_dir, 'repo')
567+
target_branch = 'feature-branch'
568+
additional_message = 'Additional PR context'
569+
570+
# Mock API responses
571+
mock_get.side_effect = [
572+
MagicMock(status_code=404), # Branch doesn't exist
573+
MagicMock(status_code=200), # Target branch exists
574+
]
575+
576+
mock_post.return_value.json.return_value = {
577+
'html_url': 'https://github.com/test-owner/test-repo/pull/1'
578+
}
579+
580+
# Mock subprocess.run calls
581+
mock_run.side_effect = [
582+
MagicMock(returncode=0), # git checkout -b
583+
MagicMock(returncode=0), # git push
584+
]
585+
586+
# Call the function with target_branch and additional_message
587+
result = send_pull_request(
588+
github_issue=mock_github_issue,
589+
github_token='test-token',
590+
github_username='test-user',
591+
patch_dir=repo_path,
592+
pr_type='ready',
593+
target_branch=target_branch,
594+
additional_message=additional_message,
595+
)
596+
597+
# Assert API calls
598+
assert mock_get.call_count == 2
599+
600+
# Check PR creation
601+
mock_post.assert_called_once()
602+
post_data = mock_post.call_args[1]['json']
603+
assert post_data['base'] == target_branch
604+
assert additional_message in post_data['body']
605+
assert 'This pull request fixes #42' in post_data['body']
606+
607+
503608
@patch('requests.get')
504609
def test_send_pull_request_invalid_target_branch(
505610
mock_get, mock_github_issue, mock_output_dir

0 commit comments

Comments
 (0)