From ed7adb335ca7e65f19418ca1f337f88c7b7f9c24 Mon Sep 17 00:00:00 2001 From: Rohit Malhotra Date: Sun, 7 Dec 2025 22:58:45 -0500 Subject: [PATCH] GitHub V1 Callbacks not trigger by v1 enabled flag (#11923) Co-authored-by: openhands --- .../integrations/github/github_manager.py | 26 +++-- enterprise/integrations/github/github_view.py | 98 +++++-------------- enterprise/integrations/resolver_context.py | 55 +++++++++++ enterprise/integrations/types.py | 2 +- enterprise/integrations/v1_utils.py | 20 ++++ enterprise/tests/unit/test_github_view.py | 5 + 6 files changed, 120 insertions(+), 86 deletions(-) create mode 100644 enterprise/integrations/resolver_context.py create mode 100644 enterprise/integrations/v1_utils.py diff --git a/enterprise/integrations/github/github_manager.py b/enterprise/integrations/github/github_manager.py index e1c1a31fb4..00ad5124ce 100644 --- a/enterprise/integrations/github/github_manager.py +++ b/enterprise/integrations/github/github_manager.py @@ -22,6 +22,7 @@ from integrations.utils import ( HOST_URL, OPENHANDS_RESOLVER_TEMPLATES_DIR, ) +from integrations.v1_utils import get_saas_user_auth from jinja2 import Environment, FileSystemLoader from pydantic import SecretStr from server.auth.constants import GITHUB_APP_CLIENT_ID, GITHUB_APP_PRIVATE_KEY @@ -164,8 +165,13 @@ class GithubManager(Manager): ) if await self.is_job_requested(message): + payload = message.message.get('payload', {}) + user_id = payload['sender']['id'] + keycloak_user_id = await self.token_manager.get_user_id_from_idp_user_id( + user_id, ProviderType.GITHUB + ) github_view = await GithubFactory.create_github_view_from_payload( - message, self.token_manager + message, keycloak_user_id ) logger.info( f'[GitHub] Creating job for {github_view.user_info.username} in {github_view.full_repo_name}#{github_view.issue_number}' @@ -282,8 +288,15 @@ class GithubManager(Manager): f'[Github]: Error summarizing issue solvability: {str(e)}' ) + saas_user_auth = await get_saas_user_auth( + github_view.user_info.keycloak_user_id, self.token_manager + ) + await github_view.create_new_conversation( - self.jinja_env, secret_store.provider_tokens, convo_metadata + self.jinja_env, + secret_store.provider_tokens, + convo_metadata, + saas_user_auth, ) conversation_id = github_view.conversation_id @@ -292,14 +305,7 @@ class GithubManager(Manager): f'[GitHub] Created conversation {conversation_id} for user {user_info.username}' ) - from openhands.server.shared import ConversationStoreImpl, config - - conversation_store = await ConversationStoreImpl.get_instance( - config, github_view.user_info.keycloak_user_id - ) - metadata = await conversation_store.get_metadata(conversation_id) - - if metadata.conversation_version != 'v1': + if not github_view.v1: # Create a GithubCallbackProcessor processor = GithubCallbackProcessor( github_view=github_view, diff --git a/enterprise/integrations/github/github_view.py b/enterprise/integrations/github/github_view.py index 97352a233f..4d15349dce 100644 --- a/enterprise/integrations/github/github_view.py +++ b/enterprise/integrations/github/github_view.py @@ -1,3 +1,4 @@ +from dataclasses import dataclass from uuid import UUID, uuid4 from github import Github, GithubIntegration @@ -8,6 +9,7 @@ from integrations.github.github_types import ( WorkflowRunStatus, ) from integrations.models import Message +from integrations.resolver_context import ResolverUserContext from integrations.types import ResolverViewInterface, UserData from integrations.utils import ( ENABLE_PROACTIVE_CONVERSATION_STARTERS, @@ -17,7 +19,6 @@ from integrations.utils import ( has_exact_mention, ) from jinja2 import Environment -from pydantic.dataclasses import dataclass from server.auth.constants import GITHUB_APP_CLIENT_ID, GITHUB_APP_PRIVATE_KEY from server.auth.token_manager import TokenManager from server.config import get_config @@ -34,18 +35,16 @@ from openhands.app_server.app_conversation.app_conversation_models import ( from openhands.app_server.config import get_app_conversation_service from openhands.app_server.services.injector import InjectorState from openhands.app_server.user.specifiy_user_context import USER_CONTEXT_ATTR -from openhands.app_server.user.user_context import UserContext -from openhands.app_server.user.user_models import UserInfo from openhands.core.logger import openhands_logger as logger from openhands.integrations.github.github_service import GithubServiceImpl from openhands.integrations.provider import PROVIDER_TOKEN_TYPE, ProviderType from openhands.integrations.service_types import Comment from openhands.sdk import TextContent -from openhands.sdk.conversation.secret_source import SecretSource from openhands.server.services.conversation_service import ( initialize_conversation, start_conversation, ) +from openhands.server.user_auth.user_auth import UserAuth from openhands.storage.data_models.conversation_metadata import ( ConversationMetadata, ConversationTrigger, @@ -55,55 +54,6 @@ from openhands.utils.async_utils import call_sync_from_async OH_LABEL, INLINE_OH_LABEL = get_oh_labels(HOST) -class GithubUserContext(UserContext): - """User context for GitHub integration that provides user info without web request.""" - - def __init__(self, keycloak_user_id: str, git_provider_tokens: PROVIDER_TOKEN_TYPE): - self.keycloak_user_id = keycloak_user_id - self.git_provider_tokens = git_provider_tokens - self.settings_store = SaasSettingsStore( - user_id=self.keycloak_user_id, - session_maker=session_maker, - config=get_config(), - ) - - self.secrets_store = SaasSecretsStore( - self.keycloak_user_id, session_maker, get_config() - ) - - async def get_user_id(self) -> str | None: - return self.keycloak_user_id - - async def get_user_info(self) -> UserInfo: - user_settings = await self.settings_store.load() - return UserInfo( - id=self.keycloak_user_id, - **user_settings.model_dump(context={'expose_secrets': True}), - ) - - async def get_authenticated_git_url(self, repository: str) -> str: - # This would need to be implemented based on the git provider tokens - # For now, return a basic HTTPS URL - return f'https://github.com/{repository}.git' - - async def get_provider_tokens(self) -> PROVIDER_TOKEN_TYPE | None: - return self.git_provider_tokens - - async def get_latest_token(self, provider_type: ProviderType) -> str | None: - # Return the appropriate token from git_provider_tokens - if provider_type == ProviderType.GITHUB and self.git_provider_tokens: - return self.git_provider_tokens.get(ProviderType.GITHUB) - return None - - async def get_secrets(self) -> dict[str, SecretSource]: - # Return empty dict for now - GitHub integration handles secrets separately - user_secrets = await self.secrets_store.load() - return dict(user_secrets.custom_secrets) if user_secrets else {} - - async def get_mcp_api_key(self) -> str | None: - raise NotImplementedError() - - async def get_user_proactive_conversation_setting(user_id: str | None) -> bool: """Get the user's proactive conversation setting. @@ -137,7 +87,7 @@ async def get_user_proactive_conversation_setting(user_id: str | None) -> bool: return settings.enable_proactive_conversation_starters -async def get_user_v1_enabled_setting(user_id: str | None) -> bool: +async def get_user_v1_enabled_setting(user_id: str) -> bool: """Get the user's V1 conversation API setting. Args: @@ -146,11 +96,6 @@ async def get_user_v1_enabled_setting(user_id: str | None) -> bool: Returns: True if V1 conversations are enabled for this user, False otherwise """ - - # If no user ID is provided, we can't check user settings - if not user_id: - return False - config = get_config() settings_store = SaasSettingsStore( user_id=user_id, session_maker=session_maker, config=config @@ -186,6 +131,7 @@ class GithubIssue(ResolverViewInterface): title: str description: str previous_comments: list[Comment] + v1: bool async def _load_resolver_context(self): github_service = GithubServiceImpl( @@ -248,14 +194,17 @@ class GithubIssue(ResolverViewInterface): jinja_env: Environment, git_provider_tokens: PROVIDER_TOKEN_TYPE, conversation_metadata: ConversationMetadata, + saas_user_auth: UserAuth, ): v1_enabled = await get_user_v1_enabled_setting(self.user_info.keycloak_user_id) - + logger.info( + f'[GitHub V1]: User flag found for {self.user_info.keycloak_user_id} is {v1_enabled}' + ) if v1_enabled: try: # Use V1 app conversation service await self._create_v1_conversation( - jinja_env, git_provider_tokens, conversation_metadata + jinja_env, saas_user_auth, conversation_metadata ) return @@ -274,6 +223,7 @@ class GithubIssue(ResolverViewInterface): conversation_metadata: ConversationMetadata, ): """Create conversation using the legacy V0 system.""" + logger.info('[GitHub V1]: Creating V0 conversation') custom_secrets = await self._get_user_secrets() user_instructions, conversation_instructions = await self._get_instructions( @@ -295,10 +245,12 @@ class GithubIssue(ResolverViewInterface): async def _create_v1_conversation( self, jinja_env: Environment, - git_provider_tokens: PROVIDER_TOKEN_TYPE, + saas_user_auth: UserAuth, conversation_metadata: ConversationMetadata, ): """Create conversation using the new V1 app conversation system.""" + logger.info('[GitHub V1]: Creating V1 conversation') + user_instructions, conversation_instructions = await self._get_instructions( jinja_env ) @@ -329,10 +281,7 @@ class GithubIssue(ResolverViewInterface): ) # Set up the GitHub user context for the V1 system - github_user_context = GithubUserContext( - keycloak_user_id=self.user_info.keycloak_user_id, - git_provider_tokens=git_provider_tokens, - ) + github_user_context = ResolverUserContext(saas_user_auth=saas_user_auth) setattr(injector_state, USER_CONTEXT_ATTR, github_user_context) async with get_app_conversation_service( @@ -347,6 +296,8 @@ class GithubIssue(ResolverViewInterface): f'Failed to start V1 conversation: {task.detail}' ) + self.v1 = True + def _create_github_v1_callback_processor(self): """Create a V1 callback processor for GitHub integration.""" from openhands.app_server.event_callback.github_v1_callback_processor import ( @@ -809,7 +760,7 @@ class GithubFactory: @staticmethod async def create_github_view_from_payload( - message: Message, token_manager: TokenManager + message: Message, keycloak_user_id: str ) -> ResolverViewInterface: """Create the appropriate class (GithubIssue or GithubPRComment) based on the payload. Also return metadata about the event (e.g., action type). @@ -819,17 +770,10 @@ class GithubFactory: user_id = payload['sender']['id'] username = payload['sender']['login'] - keyloak_user_id = await token_manager.get_user_id_from_idp_user_id( - user_id, ProviderType.GITHUB - ) - - if keyloak_user_id is None: - logger.warning(f'Got invalid keyloak user id for GitHub User {user_id} ') - selected_repo = GithubFactory.get_full_repo_name(repo_obj) is_public_repo = not repo_obj.get('private', True) user_info = UserData( - user_id=user_id, username=username, keycloak_user_id=keyloak_user_id + user_id=user_id, username=username, keycloak_user_id=keycloak_user_id ) installation_id = message.message['installation'] @@ -853,6 +797,7 @@ class GithubFactory: title='', description='', previous_comments=[], + v1=False, ) elif GithubFactory.is_issue_comment(message): @@ -878,6 +823,7 @@ class GithubFactory: title='', description='', previous_comments=[], + v1=False, ) elif GithubFactory.is_pr_comment(message): @@ -919,6 +865,7 @@ class GithubFactory: title='', description='', previous_comments=[], + v1=False, ) elif GithubFactory.is_inline_pr_comment(message): @@ -952,6 +899,7 @@ class GithubFactory: title='', description='', previous_comments=[], + v1=False, ) else: diff --git a/enterprise/integrations/resolver_context.py b/enterprise/integrations/resolver_context.py new file mode 100644 index 0000000000..29840ee9c7 --- /dev/null +++ b/enterprise/integrations/resolver_context.py @@ -0,0 +1,55 @@ +from openhands.app_server.user.user_context import UserContext +from openhands.app_server.user.user_models import UserInfo +from openhands.integrations.provider import PROVIDER_TOKEN_TYPE +from openhands.integrations.service_types import ProviderType +from openhands.server.user_auth.user_auth import UserAuth + + +class ResolverUserContext(UserContext): + """User context for resolver operations that inherits from UserContext.""" + + def __init__( + self, + saas_user_auth: UserAuth, + ): + self.saas_user_auth = saas_user_auth + + async def get_user_id(self) -> str | None: + return await self.saas_user_auth.get_user_id() + + async def get_user_info(self) -> UserInfo: + user_settings = await self.saas_user_auth.get_user_settings() + user_id = await self.saas_user_auth.get_user_id() + if user_settings: + return UserInfo( + id=user_id, + **user_settings.model_dump(context={'expose_secrets': True}), + ) + + return UserInfo(id=user_id) + + async def get_authenticated_git_url(self, repository: str) -> str: + # This would need to be implemented based on the git provider tokens + # For now, return a basic HTTPS URL + return f'https://github.com/{repository}.git' + + async def get_latest_token(self, provider_type: ProviderType) -> str | None: + # Return the appropriate token from git_provider_tokens + + provider_tokens = await self.saas_user_auth.get_provider_tokens() + if provider_tokens: + return provider_tokens.get(provider_type) + return None + + async def get_provider_tokens(self) -> PROVIDER_TOKEN_TYPE | None: + return await self.saas_user_auth.get_provider_tokens() + + async def get_secrets(self) -> dict[str, str]: + """Get secrets for the user, including custom secrets.""" + secrets = await self.saas_user_auth.get_secrets() + if secrets: + return dict(secrets.custom_secrets) + return {} + + async def get_mcp_api_key(self) -> str | None: + return await self.saas_user_auth.get_mcp_api_key() diff --git a/enterprise/integrations/types.py b/enterprise/integrations/types.py index dcbcc9b7d3..0b8d79228c 100644 --- a/enterprise/integrations/types.py +++ b/enterprise/integrations/types.py @@ -19,7 +19,7 @@ class PRStatus(Enum): class UserData(BaseModel): user_id: int username: str - keycloak_user_id: str | None + keycloak_user_id: str @dataclass diff --git a/enterprise/integrations/v1_utils.py b/enterprise/integrations/v1_utils.py new file mode 100644 index 0000000000..78953e4e93 --- /dev/null +++ b/enterprise/integrations/v1_utils.py @@ -0,0 +1,20 @@ +from pydantic import SecretStr +from server.auth.saas_user_auth import SaasUserAuth +from server.auth.token_manager import TokenManager + +from openhands.core.logger import openhands_logger as logger +from openhands.server.user_auth.user_auth import UserAuth + + +async def get_saas_user_auth( + keycloak_user_id: str, token_manager: TokenManager +) -> UserAuth: + offline_token = await token_manager.load_offline_token(keycloak_user_id) + if offline_token is None: + logger.info('no_offline_token_found') + + user_auth = SaasUserAuth( + user_id=keycloak_user_id, + refresh_token=SecretStr(offline_token), + ) + return user_auth diff --git a/enterprise/tests/unit/test_github_view.py b/enterprise/tests/unit/test_github_view.py index 20f033d157..1edc46bc2a 100644 --- a/enterprise/tests/unit/test_github_view.py +++ b/enterprise/tests/unit/test_github_view.py @@ -1,6 +1,7 @@ from unittest import TestCase, mock from unittest.mock import MagicMock, patch +import pytest from integrations.github.github_view import GithubFactory, GithubIssue, get_oh_labels from integrations.models import Message, SourceType from integrations.types import UserData @@ -114,8 +115,10 @@ class TestGithubV1ConversationRouting(TestCase): title='Test Issue', description='Test issue description', previous_comments=[], + v1=False, ) + @pytest.mark.asyncio @patch('integrations.github.github_view.get_user_v1_enabled_setting') @patch.object(GithubIssue, '_create_v0_conversation') @patch.object(GithubIssue, '_create_v1_conversation') @@ -144,6 +147,7 @@ class TestGithubV1ConversationRouting(TestCase): ) mock_create_v1.assert_not_called() + @pytest.mark.asyncio @patch('integrations.github.github_view.get_user_v1_enabled_setting') @patch.object(GithubIssue, '_create_v0_conversation') @patch.object(GithubIssue, '_create_v1_conversation') @@ -172,6 +176,7 @@ class TestGithubV1ConversationRouting(TestCase): ) mock_create_v0.assert_not_called() + @pytest.mark.asyncio @patch('integrations.github.github_view.get_user_v1_enabled_setting') @patch.object(GithubIssue, '_create_v0_conversation') @patch.object(GithubIssue, '_create_v1_conversation')