diff --git a/openhands/integrations/github/github_service.py b/openhands/integrations/github/github_service.py index 4cf5279abd..c3dc447444 100644 --- a/openhands/integrations/github/github_service.py +++ b/openhands/integrations/github/github_service.py @@ -505,10 +505,7 @@ class GitHubService(BaseGitService, GitService): ) # Return the HTML URL of the created PR - if 'html_url' in response: - return response['html_url'] - else: - return f'PR created but URL not found in response: {response}' + return response['html_url'] diff --git a/openhands/integrations/gitlab/gitlab_service.py b/openhands/integrations/gitlab/gitlab_service.py index 28b2182505..110afe6ff3 100644 --- a/openhands/integrations/gitlab/gitlab_service.py +++ b/openhands/integrations/gitlab/gitlab_service.py @@ -500,12 +500,8 @@ class GitLabService(BaseGitService, GitService): url=url, params=payload, method=RequestMethod.POST ) - # Return the web URL of the created MR - if 'web_url' in response: - return response['web_url'] - else: - return f'MR created but URL not found in response: {response}' + return response['web_url'] diff --git a/openhands/integrations/templates/resolver/github/issue_comment_conversation_instructions.j2 b/openhands/integrations/templates/resolver/github/issue_comment_conversation_instructions.j2 index 1eeb5cd8aa..2431e68034 100644 --- a/openhands/integrations/templates/resolver/github/issue_comment_conversation_instructions.j2 +++ b/openhands/integrations/templates/resolver/github/issue_comment_conversation_instructions.j2 @@ -15,4 +15,3 @@ When you're done, make sure to 2. Use the `create_pr` tool to open a new PR 3. Name the branch using `openhands/` as a prefix (e.g `openhands/update-readme`) 4. The PR description should mention that it "fixes" or "closes" the issue number -5. Make sure to leave the following sentence at the end of the PR description: `@{{ username }} can click here to [continue refining the PR]({{ conversation_url }})` diff --git a/openhands/integrations/templates/resolver/github/issue_labeled_conversation_instructions.j2 b/openhands/integrations/templates/resolver/github/issue_labeled_conversation_instructions.j2 index ddd8424df5..e28d265bc8 100644 --- a/openhands/integrations/templates/resolver/github/issue_labeled_conversation_instructions.j2 +++ b/openhands/integrations/templates/resolver/github/issue_labeled_conversation_instructions.j2 @@ -9,4 +9,3 @@ When you're done, make sure to 1. Use the `create_pr` tool to open a new PR 2. The PR description should mention that it "fixes" or "closes" the issue number -3. Make sure to leave the following sentence at the end of the PR description: `@{{ username }} can click here to [continue refining the PR]({{ conversation_url }})` diff --git a/openhands/integrations/templates/resolver/gitlab/issue_comment_conversation_instructions.j2 b/openhands/integrations/templates/resolver/gitlab/issue_comment_conversation_instructions.j2 index beb8be821e..fdfcf2b387 100644 --- a/openhands/integrations/templates/resolver/gitlab/issue_comment_conversation_instructions.j2 +++ b/openhands/integrations/templates/resolver/gitlab/issue_comment_conversation_instructions.j2 @@ -15,4 +15,3 @@ When you're done, make sure to 2. Use the `create_mr` tool to open a new MR 3. Name the branch using `openhands/` as a prefix (e.g `openhands/update-readme`) 4. The MR description should mention that it "fixes" or "closes" the issue number -5. Make sure to leave the following sentence at the end of the MR description: `@{{ username }} can click here to [continue refining the MR]({{ conversation_url }})` diff --git a/openhands/integrations/templates/resolver/gitlab/issue_labeled_conversation_instructions.j2 b/openhands/integrations/templates/resolver/gitlab/issue_labeled_conversation_instructions.j2 index 17313595fd..04fab35542 100644 --- a/openhands/integrations/templates/resolver/gitlab/issue_labeled_conversation_instructions.j2 +++ b/openhands/integrations/templates/resolver/gitlab/issue_labeled_conversation_instructions.j2 @@ -9,4 +9,3 @@ When you're done, make sure to 1. Use the `create_mr` tool to open a new MR 2. The MR description should mention that it "fixes" or "closes" the issue number -3. Make sure to leave the following sentence at the end of the MR description: `@{{ username }} can click here to [continue refining the MR]({{ conversation_url }})` diff --git a/openhands/integrations/templates/resolver/slack/user_message_conversation_instructions.j2 b/openhands/integrations/templates/resolver/slack/user_message_conversation_instructions.j2 index a6570947e5..3fa0e5bb8d 100644 --- a/openhands/integrations/templates/resolver/slack/user_message_conversation_instructions.j2 +++ b/openhands/integrations/templates/resolver/slack/user_message_conversation_instructions.j2 @@ -5,7 +5,3 @@ These are a list of text messages attached in order of most recent. {{ message }} {% if not loop.last %}\n\n{% endif %} {% endfor %} - - -If you opened a pull request, please leave the following comment at the end your summary and pull request description -`{{ username }} can click here to [continue refining the PR]({{ conversation_url }})` diff --git a/openhands/server/routes/mcp.py b/openhands/server/routes/mcp.py index 7bc8d93ee8..640e201c0f 100644 --- a/openhands/server/routes/mcp.py +++ b/openhands/server/routes/mcp.py @@ -1,3 +1,4 @@ +import os import re from typing import Annotated @@ -10,9 +11,10 @@ from openhands.core.logger import openhands_logger as logger from openhands.integrations.github.github_service import GithubServiceImpl from openhands.integrations.gitlab.gitlab_service import GitLabServiceImpl from openhands.integrations.provider import ProviderToken -from openhands.integrations.service_types import ProviderType +from openhands.integrations.service_types import GitService, ProviderType from openhands.server.dependencies import get_dependencies -from openhands.server.shared import ConversationStoreImpl, config +from openhands.server.shared import ConversationStoreImpl, config, server_config +from openhands.server.types import AppMode from openhands.server.user_auth import ( get_access_token, get_provider_tokens, @@ -20,7 +22,31 @@ from openhands.server.user_auth import ( ) from openhands.storage.data_models.conversation_metadata import ConversationMetadata -mcp_server = FastMCP('mcp', stateless_http=True, dependencies=get_dependencies(), mask_error_details=True) +mcp_server = FastMCP( + 'mcp', stateless_http=True, dependencies=get_dependencies(), mask_error_details=True +) + +HOST = f'https://{os.getenv("WEB_HOST", "app.all-hands.dev").strip()}' +CONVO_URL = HOST + '/{}' + + +async def get_convo_link(service: GitService, conversation_id: str, body: str) -> str: + """ + Appends a followup link, in the PR body, to the OpenHands conversation that opened the PR + """ + + if server_config.app_mode != AppMode.SAAS: + return body + + user = await service.get_user() + username = user.login + convo_url = CONVO_URL.format(conversation_id) + convo_link = ( + f'@{username} can click here to [continue refining the PR]({convo_url})' + ) + body += f'\n\n{convo_link}' + return body + async def save_pr_metadata( user_id: str, conversation_id: str, tool_result: str @@ -84,6 +110,11 @@ async def create_pr( base_domain=github_token.host, ) + try: + body = await get_convo_link(github_service, conversation_id, body or '') + except Exception as e: + logger.warning(f'Failed to append convo link: {e}') + try: response = await github_service.create_pr( repo_name=repo_name, @@ -97,7 +128,7 @@ async def create_pr( await save_pr_metadata(user_id, conversation_id, response) except Exception as e: - error = f"Error creating pull request: {e}" + error = f'Error creating pull request: {e}' raise ToolError(str(error)) return response @@ -132,7 +163,7 @@ async def create_mr( else ProviderToken() ) - github_service = GitLabServiceImpl( + gitlab_service = GitLabServiceImpl( user_id=github_token.user_id, external_auth_id=user_id, external_auth_token=access_token, @@ -141,7 +172,14 @@ async def create_mr( ) try: - response = await github_service.create_mr( + description = await get_convo_link( + gitlab_service, conversation_id, description or '' + ) + except Exception as e: + logger.warning(f'Failed to append convo link: {e}') + + try: + response = await gitlab_service.create_mr( id=id, source_branch=source_branch, target_branch=target_branch, @@ -153,7 +191,7 @@ async def create_mr( await save_pr_metadata(user_id, conversation_id, response) except Exception as e: - error = f"Error creating merge request: {e}" + error = f'Error creating merge request: {e}' raise ToolError(str(error)) return response diff --git a/openhands/server/services/conversation_service.py b/openhands/server/services/conversation_service.py index ccb3c75e90..eeee3d298a 100644 --- a/openhands/server/services/conversation_service.py +++ b/openhands/server/services/conversation_service.py @@ -124,8 +124,8 @@ async def create_new_conversation( image_urls=image_urls or [], ) - if attach_convo_id and conversation_instructions: - conversation_instructions = conversation_instructions.format(conversation_id) + if attach_convo_id: + logger.warning('Attaching convo ID is deprecated, skipping process') agent_loop_info = await conversation_manager.maybe_start_agent_loop( conversation_id, diff --git a/tests/unit/test_mcp_routes.py b/tests/unit/test_mcp_routes.py new file mode 100644 index 0000000000..7721d1678b --- /dev/null +++ b/tests/unit/test_mcp_routes.py @@ -0,0 +1,86 @@ +from unittest.mock import AsyncMock, patch + +import pytest + +from openhands.integrations.service_types import GitService +from openhands.server.routes.mcp import get_convo_link +from openhands.server.types import AppMode + + +@pytest.mark.asyncio +async def test_get_convo_link_non_saas_mode(): + """Test get_convo_link in non-SAAS mode.""" + # Mock GitService + mock_service = AsyncMock(spec=GitService) + + # Test with non-SAAS mode + with patch('openhands.server.routes.mcp.server_config') as mock_config: + mock_config.app_mode = AppMode.OSS + + # Call the function + result = await get_convo_link( + service=mock_service, conversation_id='test-convo-id', body='Original body' + ) + + # Verify the result + assert result == 'Original body' + # Verify that get_user was not called + mock_service.get_user.assert_not_called() + + +@pytest.mark.asyncio +async def test_get_convo_link_saas_mode(): + """Test get_convo_link in SAAS mode.""" + # Mock GitService and user + mock_service = AsyncMock(spec=GitService) + mock_user = AsyncMock() + mock_user.login = 'testuser' + mock_service.get_user.return_value = mock_user + + # Test with SAAS mode + with ( + patch('openhands.server.routes.mcp.server_config') as mock_config, + patch('openhands.server.routes.mcp.CONVO_URL', 'https://test.example.com/{}'), + ): + mock_config.app_mode = AppMode.SAAS + + # Call the function + result = await get_convo_link( + service=mock_service, conversation_id='test-convo-id', body='Original body' + ) + + # Verify the result + expected_link = '@testuser can click here to [continue refining the PR](https://test.example.com/test-convo-id)' + assert result == f'Original body\n\n{expected_link}' + + # Verify that get_user was called + mock_service.get_user.assert_called_once() + + +@pytest.mark.asyncio +async def test_get_convo_link_empty_body(): + """Test get_convo_link with an empty body.""" + # Mock GitService and user + mock_service = AsyncMock(spec=GitService) + mock_user = AsyncMock() + mock_user.login = 'testuser' + mock_service.get_user.return_value = mock_user + + # Test with SAAS mode and empty body + with ( + patch('openhands.server.routes.mcp.server_config') as mock_config, + patch('openhands.server.routes.mcp.CONVO_URL', 'https://test.example.com/{}'), + ): + mock_config.app_mode = AppMode.SAAS + + # Call the function + result = await get_convo_link( + service=mock_service, conversation_id='test-convo-id', body='' + ) + + # Verify the result + expected_link = '@testuser can click here to [continue refining the PR](https://test.example.com/test-convo-id)' + assert result == f'\n\n{expected_link}' + + # Verify that get_user was called + mock_service.get_user.assert_called_once()