GitHub V1 Callbacks not trigger by v1 enabled flag (#11923)

Co-authored-by: openhands <openhands@all-hands.dev>

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,

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:

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()

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

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

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')