From b8db9ecd53e9275cd8d75cf14a42ef2268b47d73 Mon Sep 17 00:00:00 2001 From: Rohit Malhotra Date: Wed, 4 Mar 2026 11:13:16 -0500 Subject: [PATCH] Fix mypy type errors in enterprise GitLab integration (#13205) Co-authored-by: openhands --- .../gitlab/gitlab_v1_callback_processor.py | 8 ++------ enterprise/integrations/gitlab/gitlab_view.py | 6 +++++- .../server/routes/integration/gitlab.py | 4 ++-- .../test_gitlab_v1_callback_processor.py | 20 +++++++++---------- 4 files changed, 19 insertions(+), 19 deletions(-) diff --git a/enterprise/integrations/gitlab/gitlab_v1_callback_processor.py b/enterprise/integrations/gitlab/gitlab_v1_callback_processor.py index fcb3c24cb2..8fb6521d98 100644 --- a/enterprise/integrations/gitlab/gitlab_v1_callback_processor.py +++ b/enterprise/integrations/gitlab/gitlab_v1_callback_processor.py @@ -107,19 +107,15 @@ class GitlabV1CallbackProcessor(EventCallbackProcessor): # Import here to avoid circular imports from integrations.gitlab.gitlab_service import SaaSGitLabService - from openhands.integrations.gitlab.gitlab_service import GitLabServiceImpl - keycloak_user_id = self.gitlab_view_data.get('keycloak_user_id') if not keycloak_user_id: raise RuntimeError('Missing keycloak user ID for GitLab') - gitlab_service: SaaSGitLabService = GitLabServiceImpl( - external_auth_id=keycloak_user_id - ) + gitlab_service = SaaSGitLabService(external_auth_id=keycloak_user_id) project_id = self.gitlab_view_data['project_id'] issue_number = self.gitlab_view_data['issue_number'] - discussion_id = self.gitlab_view_data.get('discussion_id') + discussion_id = self.gitlab_view_data['discussion_id'] is_mr = self.gitlab_view_data.get('is_mr', False) if is_mr: diff --git a/enterprise/integrations/gitlab/gitlab_view.py b/enterprise/integrations/gitlab/gitlab_view.py index 8bb0f22868..4b71940640 100644 --- a/enterprise/integrations/gitlab/gitlab_view.py +++ b/enterprise/integrations/gitlab/gitlab_view.py @@ -461,7 +461,11 @@ class GitlabFactory: ) # Check v1_enabled at construction time - this is the source of truth - v1_enabled = await is_v1_enabled_for_gitlab_resolver(keycloak_user_id) + v1_enabled = ( + await is_v1_enabled_for_gitlab_resolver(keycloak_user_id) + if keycloak_user_id + else False + ) logger.info( f'[GitLab V1]: User flag found for {keycloak_user_id} is {v1_enabled}' ) diff --git a/enterprise/server/routes/integration/gitlab.py b/enterprise/server/routes/integration/gitlab.py index 2b6cbe6fd5..a1e001bdc8 100644 --- a/enterprise/server/routes/integration/gitlab.py +++ b/enterprise/server/routes/integration/gitlab.py @@ -69,13 +69,13 @@ async def verify_gitlab_signature( raise HTTPException(status_code=403, detail='Required payload headers missing!') if IS_LOCAL_DEPLOYMENT: - webhook_secret = 'localdeploymentwebhooktesttoken' + webhook_secret: str | None = 'localdeploymentwebhooktesttoken' else: webhook_secret = await webhook_store.get_webhook_secret( webhook_uuid=webhook_uuid, user_id=user_id ) - if header_webhook_secret != webhook_secret: + if not webhook_secret or header_webhook_secret != webhook_secret: raise HTTPException(status_code=403, detail="Request signatures didn't match!") diff --git a/enterprise/tests/unit/integrations/gitlab/test_gitlab_v1_callback_processor.py b/enterprise/tests/unit/integrations/gitlab/test_gitlab_v1_callback_processor.py index edc50f1477..252d208e08 100644 --- a/enterprise/tests/unit/integrations/gitlab/test_gitlab_v1_callback_processor.py +++ b/enterprise/tests/unit/integrations/gitlab/test_gitlab_v1_callback_processor.py @@ -199,10 +199,10 @@ class TestGitlabV1CallbackProcessor: @patch('openhands.app_server.config.get_sandbox_service') @patch('openhands.app_server.config.get_httpx_client') @patch('integrations.gitlab.gitlab_v1_callback_processor.get_summary_instruction') - @patch('openhands.integrations.gitlab.gitlab_service.GitLabServiceImpl') + @patch('integrations.gitlab.gitlab_service.SaaSGitLabService') async def test_successful_callback_execution_issue( self, - mock_gitlab_service_impl, + mock_saas_gitlab_service_cls, mock_get_summary_instruction, mock_get_httpx_client, mock_get_sandbox_service, @@ -228,7 +228,7 @@ class TestGitlabV1CallbackProcessor: # GitLab service mock mock_gitlab_service = AsyncMock() - mock_gitlab_service_impl.return_value = mock_gitlab_service + mock_saas_gitlab_service_cls.return_value = mock_gitlab_service result = await gitlab_callback_processor( conversation_id=conversation_id, @@ -245,7 +245,7 @@ class TestGitlabV1CallbackProcessor: assert gitlab_callback_processor.should_request_summary is False # Verify GitLab service was called correctly for issue - mock_gitlab_service_impl.assert_called_once_with( + mock_saas_gitlab_service_cls.assert_called_once_with( external_auth_id='test_keycloak_user' ) mock_gitlab_service.reply_to_issue.assert_called_once_with( @@ -265,10 +265,10 @@ class TestGitlabV1CallbackProcessor: @patch('openhands.app_server.config.get_sandbox_service') @patch('openhands.app_server.config.get_httpx_client') @patch('integrations.gitlab.gitlab_v1_callback_processor.get_summary_instruction') - @patch('openhands.integrations.gitlab.gitlab_service.GitLabServiceImpl') + @patch('integrations.gitlab.gitlab_service.SaaSGitLabService') async def test_successful_callback_execution_mr( self, - mock_gitlab_service_impl, + mock_saas_gitlab_service_cls, mock_get_summary_instruction, mock_get_httpx_client, mock_get_sandbox_service, @@ -293,7 +293,7 @@ class TestGitlabV1CallbackProcessor: # GitLab service mock mock_gitlab_service = AsyncMock() - mock_gitlab_service_impl.return_value = mock_gitlab_service + mock_saas_gitlab_service_cls.return_value = mock_gitlab_service result = await gitlab_callback_processor_mr( conversation_id=conversation_id, @@ -326,10 +326,10 @@ class TestGitlabV1CallbackProcessor: @patch('openhands.app_server.config.get_sandbox_service') @patch('openhands.app_server.config.get_httpx_client') @patch('integrations.gitlab.gitlab_v1_callback_processor.get_summary_instruction') - @patch('openhands.integrations.gitlab.gitlab_service.GitLabServiceImpl') + @patch('integrations.gitlab.gitlab_service.SaaSGitLabService') async def test_exception_handling_posts_error_to_gitlab( self, - mock_gitlab_service_impl, + mock_saas_gitlab_service_cls, mock_get_summary_instruction, mock_get_httpx_client, mock_get_sandbox_service, @@ -355,7 +355,7 @@ class TestGitlabV1CallbackProcessor: # GitLab service mock mock_gitlab_service = AsyncMock() - mock_gitlab_service_impl.return_value = mock_gitlab_service + mock_saas_gitlab_service_cls.return_value = mock_gitlab_service result = await gitlab_callback_processor( conversation_id=conversation_id,