Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 16 additions & 4 deletions openhands/integrations/provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -466,10 +466,22 @@ async def verify_repo_provider(
except Exception as e:
errors.append(f'{provider.value}: {str(e)}')

# Log all accumulated errors before raising AuthenticationError
logger.error(
f'Failed to access repository {repository} with all available providers. Errors: {"; ".join(errors)}'
)
# Log detailed error based on whether we had tokens or not
if not self.provider_tokens:
logger.error(
f'Failed to access repository {repository}: No provider tokens available. '
f'provider_tokens dict is empty.'
)
elif errors:
logger.error(
f'Failed to access repository {repository} with all available providers. '
f'Tried providers: {list(self.provider_tokens.keys())}. '
f'Errors: {"; ".join(errors)}'
)
else:
logger.error(
f'Failed to access repository {repository}: Unknown error (no providers tried, no errors recorded)'
)
raise AuthenticationError(f'Unable to access repo {repository}')

async def get_branches(
Expand Down
20 changes: 18 additions & 2 deletions openhands/server/routes/manage_conversations.py
Original file line number Diff line number Diff line change
Expand Up @@ -462,6 +462,7 @@ async def start_conversation(
providers_set: ProvidersSetModel,
conversation_id: str = Depends(validate_conversation_id),
user_id: str = Depends(get_user_id),
provider_tokens: PROVIDER_TOKEN_TYPE = Depends(get_provider_tokens),
settings: Settings = Depends(get_user_settings),
conversation_store: ConversationStore = Depends(get_conversation_store),
) -> ConversationResponse:
Expand All @@ -471,7 +472,22 @@ async def start_conversation(
to start a conversation. If the conversation is already running, it will
return the existing agent loop info.
"""
logger.info(f'Starting conversation: {conversation_id}')
logger.info(
f'Starting conversation: {conversation_id}',
extra={'session_id': conversation_id},
)

# Log token fetch status
if provider_tokens:
logger.info(
f'/start endpoint: Fetched provider tokens: {list(provider_tokens.keys())}',
extra={'session_id': conversation_id},
)
else:
logger.warning(
'/start endpoint: No provider tokens fetched (provider_tokens is None/empty)',
extra={'session_id': conversation_id},
)

try:
# Check that the conversation exists
Expand All @@ -488,7 +504,7 @@ async def start_conversation(

# Set up conversation init data with provider information
conversation_init_data = await setup_init_conversation_settings(
user_id, conversation_id, providers_set.providers_set or []
user_id, conversation_id, providers_set.providers_set or [], provider_tokens
)

# Start the agent loop
Expand Down
33 changes: 28 additions & 5 deletions openhands/server/services/conversation_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -215,14 +215,18 @@ def create_provider_tokens_object(


async def setup_init_conversation_settings(
user_id: str | None, conversation_id: str, providers_set: list[ProviderType]
user_id: str | None,
conversation_id: str,
providers_set: list[ProviderType],
provider_tokens: PROVIDER_TOKEN_TYPE | None = None,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this line and the following logic is the fix, everything else in this pr is logs or tests

) -> ConversationInitData:
"""Set up conversation initialization data with provider tokens.

Args:
user_id: The user ID
conversation_id: The conversation ID
providers_set: List of provider types to set up tokens for
provider_tokens: Optional provider tokens to use (for SAAS mode resume)

Returns:
ConversationInitData with provider tokens configured
Expand All @@ -243,11 +247,30 @@ async def setup_init_conversation_settings(
session_init_args: dict = {}
session_init_args = {**settings.__dict__, **session_init_args}

git_provider_tokens = create_provider_tokens_object(providers_set)
logger.info(f'Git provider scaffold: {git_provider_tokens}')
# Use provided tokens if available (for SAAS resume), otherwise create scaffold
if provider_tokens:
logger.info(
f'Using provided provider_tokens: {list(provider_tokens.keys())}',
extra={'session_id': conversation_id},
)
git_provider_tokens = provider_tokens
else:
logger.info(
f'No provider_tokens provided, creating scaffold for: {providers_set}',
extra={'session_id': conversation_id},
)
git_provider_tokens = create_provider_tokens_object(providers_set)
logger.info(
f'Git provider scaffold: {git_provider_tokens}',
extra={'session_id': conversation_id},
)

if server_config.app_mode != AppMode.SAAS and user_secrets:
git_provider_tokens = user_secrets.provider_tokens
if server_config.app_mode != AppMode.SAAS and user_secrets:
logger.info(
f'Non-SaaS mode: Overriding with user_secrets provider tokens: {list(user_secrets.provider_tokens.keys())}',
extra={'session_id': conversation_id},
)
git_provider_tokens = user_secrets.provider_tokens

session_init_args['git_provider_tokens'] = git_provider_tokens
if user_secrets:
Expand Down
150 changes: 150 additions & 0 deletions tests/unit/server/routes/test_start_endpoint.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,150 @@
"""Tests for /start endpoint provider token handling."""

from types import MappingProxyType
from unittest.mock import AsyncMock, patch

import pytest
from pydantic import SecretStr

from openhands.integrations.provider import ProviderToken, ProviderType
from openhands.server.data_models.agent_loop_info import AgentLoopInfo
from openhands.server.routes.manage_conversations import (
ProvidersSetModel,
start_conversation,
)
from openhands.server.types import AppMode
from openhands.storage.data_models.conversation_metadata import ConversationMetadata
from openhands.storage.data_models.conversation_status import ConversationStatus
from openhands.storage.data_models.settings import Settings


@pytest.fixture
def mock_settings():
"""Create a real Settings object with minimal required fields."""
return Settings(
language='en',
agent='CodeActAgent',
max_iterations=100,
llm_model='anthropic/claude-3-5-sonnet-20241022',
llm_api_key=SecretStr('test_api_key_12345'),
llm_base_url=None,
)


@pytest.fixture
def mock_provider_tokens():
"""Create real provider tokens to test with."""
return MappingProxyType(
{
ProviderType.GITHUB: ProviderToken(
token=SecretStr('ghp_real_token_test123'), user_id='test_user_456'
)
}
)


@pytest.fixture
def mock_conversation_metadata():
"""Create a real ConversationMetadata object."""
return ConversationMetadata(
conversation_id='test_conv_123',
user_id='test_user_456',
title='Test Conversation',
selected_repository='test/repo',
selected_branch='main',
git_provider=ProviderType.GITHUB,
)


@pytest.mark.asyncio
async def test_start_endpoint_passes_provider_tokens(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hooray for new tests :)

mock_settings, mock_provider_tokens, mock_conversation_metadata
):
"""Test that /start endpoint passes provider_tokens to setup_init_conversation_settings.
This test verifies the full end-to-end flow with real tokens through to ConversationInitData.
"""
conversation_id = 'test_conv_123'
user_id = 'test_user_456'
providers_set = ProvidersSetModel(providers_set=[ProviderType.GITHUB])

# Mock conversation store
mock_conversation_store = AsyncMock()
mock_conversation_store.get_metadata = AsyncMock(
return_value=mock_conversation_metadata
)

# Mock agent loop info that will be returned
mock_agent_loop_info = AgentLoopInfo(
conversation_id=conversation_id,
url=None,
session_api_key=None,
event_store=None,
status=ConversationStatus.RUNNING,
)

# Mock only infrastructure - let setup_init_conversation_settings run for real
with patch(
'openhands.server.routes.manage_conversations.conversation_manager'
) as mock_manager:
# Mock the stores that setup_init_conversation_settings needs
with patch(
'openhands.server.services.conversation_service.SettingsStoreImpl.get_instance'
) as mock_settings_store_cls:
with patch(
'openhands.server.services.conversation_service.SecretsStoreImpl.get_instance'
) as mock_secrets_store_cls:
with patch(
'openhands.server.services.conversation_service.server_config'
) as mock_server_config:
# Setup store mocks
mock_settings_store = AsyncMock()
mock_settings_store.load = AsyncMock(return_value=mock_settings)
mock_settings_store_cls.return_value = mock_settings_store

mock_secrets_store = AsyncMock()
mock_secrets_store.load = AsyncMock(return_value=None)
mock_secrets_store_cls.return_value = mock_secrets_store

mock_server_config.app_mode = AppMode.SAAS

mock_manager.maybe_start_agent_loop = AsyncMock(
return_value=mock_agent_loop_info
)

# Call endpoint with provider tokens
response = await start_conversation(
providers_set=providers_set,
conversation_id=conversation_id,
user_id=user_id,
provider_tokens=mock_provider_tokens,
settings=mock_settings,
conversation_store=mock_conversation_store,
)

# Verify ConversationInitData has real provider tokens
mock_manager.maybe_start_agent_loop.assert_called_once()
call_kwargs = mock_manager.maybe_start_agent_loop.call_args[1]
conversation_init_data = call_kwargs['settings']

assert conversation_init_data.git_provider_tokens is not None
assert (
conversation_init_data.git_provider_tokens
== mock_provider_tokens
)
assert (
ProviderType.GITHUB
in conversation_init_data.git_provider_tokens
)

github_token = conversation_init_data.git_provider_tokens[
ProviderType.GITHUB
]
assert (
github_token.token.get_secret_value()
== 'ghp_real_token_test123'
)
assert github_token.user_id == 'test_user_456'

assert response.status == 'ok'
assert response.conversation_id == conversation_id
Loading
Loading