mirror of
https://github.com/OpenHands/OpenHands.git
synced 2026-03-22 05:37:20 +08:00
(Hotfix): missing provider tokens in v1 resolvers (#12453)
Co-authored-by: openhands <openhands@all-hands.dev>
This commit is contained in:
@@ -37,11 +37,12 @@ class ResolverUserContext(UserContext):
|
||||
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
|
||||
|
||||
# Return the appropriate token string from git_provider_tokens
|
||||
provider_tokens = await self.saas_user_auth.get_provider_tokens()
|
||||
if provider_tokens:
|
||||
return provider_tokens.get(provider_type)
|
||||
provider_token = provider_tokens.get(provider_type)
|
||||
if provider_token and provider_token.token:
|
||||
return provider_token.token.get_secret_value()
|
||||
return None
|
||||
|
||||
async def get_provider_tokens(self) -> PROVIDER_TOKEN_TYPE | None:
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
"""Test for ResolverUserContext get_secrets conversion logic.
|
||||
"""Test for ResolverUserContext get_secrets and get_latest_token logic.
|
||||
|
||||
This test focuses on testing the actual ResolverUserContext implementation.
|
||||
"""
|
||||
@@ -12,7 +12,8 @@ from pydantic import SecretStr
|
||||
from enterprise.integrations.resolver_context import ResolverUserContext
|
||||
|
||||
# Import the real classes we want to test
|
||||
from openhands.integrations.provider import CustomSecret
|
||||
from openhands.integrations.provider import CustomSecret, ProviderToken
|
||||
from openhands.integrations.service_types import ProviderType
|
||||
|
||||
# Import the SDK types we need for testing
|
||||
from openhands.sdk.secret import SecretSource, StaticSecret
|
||||
@@ -131,3 +132,135 @@ def test_custom_to_static_conversion():
|
||||
assert isinstance(static_secret, StaticSecret)
|
||||
assert isinstance(static_secret, SecretSource)
|
||||
assert static_secret.value.get_secret_value() == secret_value
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Tests for get_latest_token - ensuring string values are returned
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def create_provider_tokens(
|
||||
tokens_dict: dict[ProviderType, str],
|
||||
) -> dict[ProviderType, ProviderToken]:
|
||||
"""Helper to create provider tokens dictionary."""
|
||||
return {
|
||||
provider_type: ProviderToken(token=SecretStr(token_value))
|
||||
for provider_type, token_value in tokens_dict.items()
|
||||
}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_get_latest_token_returns_string(resolver_context, mock_saas_user_auth):
|
||||
"""Test that get_latest_token returns a string, not a ProviderToken object."""
|
||||
# Arrange
|
||||
token_value = 'ghp_test_github_token_123'
|
||||
provider_tokens = create_provider_tokens({ProviderType.GITHUB: token_value})
|
||||
mock_saas_user_auth.get_provider_tokens = AsyncMock(return_value=provider_tokens)
|
||||
|
||||
# Act
|
||||
result = await resolver_context.get_latest_token(ProviderType.GITHUB)
|
||||
|
||||
# Assert
|
||||
assert result is not None
|
||||
assert isinstance(result, str), (
|
||||
f'Expected str, got {type(result).__name__}. '
|
||||
'get_latest_token must return a string for StaticSecret compatibility.'
|
||||
)
|
||||
assert result == token_value
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_get_latest_token_returns_string_for_multiple_providers(
|
||||
resolver_context, mock_saas_user_auth
|
||||
):
|
||||
"""Test that get_latest_token returns strings for all provider types."""
|
||||
# Arrange
|
||||
provider_tokens = create_provider_tokens(
|
||||
{
|
||||
ProviderType.GITHUB: 'ghp_github_token',
|
||||
ProviderType.GITLAB: 'glpat_gitlab_token',
|
||||
ProviderType.BITBUCKET: 'bitbucket_token',
|
||||
}
|
||||
)
|
||||
mock_saas_user_auth.get_provider_tokens = AsyncMock(return_value=provider_tokens)
|
||||
|
||||
# Act & Assert - verify each provider returns a string
|
||||
for provider_type, expected_token in [
|
||||
(ProviderType.GITHUB, 'ghp_github_token'),
|
||||
(ProviderType.GITLAB, 'glpat_gitlab_token'),
|
||||
(ProviderType.BITBUCKET, 'bitbucket_token'),
|
||||
]:
|
||||
result = await resolver_context.get_latest_token(provider_type)
|
||||
assert isinstance(
|
||||
result, str
|
||||
), f'Expected str for {provider_type.name}, got {type(result).__name__}'
|
||||
assert result == expected_token
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_get_latest_token_returns_none_for_missing_provider(
|
||||
resolver_context, mock_saas_user_auth
|
||||
):
|
||||
"""Test that get_latest_token returns None when provider is not in tokens."""
|
||||
# Arrange - only GitHub token available
|
||||
provider_tokens = create_provider_tokens({ProviderType.GITHUB: 'ghp_token'})
|
||||
mock_saas_user_auth.get_provider_tokens = AsyncMock(return_value=provider_tokens)
|
||||
|
||||
# Act - request GitLab token which doesn't exist
|
||||
result = await resolver_context.get_latest_token(ProviderType.GITLAB)
|
||||
|
||||
# Assert
|
||||
assert result is None
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_get_latest_token_returns_none_when_no_provider_tokens(
|
||||
resolver_context, mock_saas_user_auth
|
||||
):
|
||||
"""Test that get_latest_token returns None when no provider tokens exist."""
|
||||
# Arrange
|
||||
mock_saas_user_auth.get_provider_tokens = AsyncMock(return_value=None)
|
||||
|
||||
# Act
|
||||
result = await resolver_context.get_latest_token(ProviderType.GITHUB)
|
||||
|
||||
# Assert
|
||||
assert result is None
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_get_latest_token_returns_none_for_empty_token(
|
||||
resolver_context, mock_saas_user_auth
|
||||
):
|
||||
"""Test that get_latest_token returns None when provider token has no value."""
|
||||
# Arrange - provider exists but token is None
|
||||
provider_tokens = {ProviderType.GITHUB: ProviderToken(token=None)}
|
||||
mock_saas_user_auth.get_provider_tokens = AsyncMock(return_value=provider_tokens)
|
||||
|
||||
# Act
|
||||
result = await resolver_context.get_latest_token(ProviderType.GITHUB)
|
||||
|
||||
# Assert
|
||||
assert result is None
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_get_latest_token_can_be_used_with_static_secret(
|
||||
resolver_context, mock_saas_user_auth
|
||||
):
|
||||
"""Test that get_latest_token result can be used directly with StaticSecret.
|
||||
|
||||
This is a critical integration test to ensure the return value is compatible
|
||||
with how it's used in _setup_secrets_for_git_providers.
|
||||
"""
|
||||
# Arrange
|
||||
token_value = 'ghp_integration_test_token'
|
||||
provider_tokens = create_provider_tokens({ProviderType.GITHUB: token_value})
|
||||
mock_saas_user_auth.get_provider_tokens = AsyncMock(return_value=provider_tokens)
|
||||
|
||||
# Act
|
||||
token = await resolver_context.get_latest_token(ProviderType.GITHUB)
|
||||
|
||||
# Assert - this should NOT raise a ValidationError
|
||||
static_secret = StaticSecret(value=token, description='GITHUB authentication token')
|
||||
assert static_secret.get_value() == token_value
|
||||
|
||||
@@ -79,7 +79,7 @@ from openhands.experiments.experiment_manager import ExperimentManagerImpl
|
||||
from openhands.integrations.provider import ProviderType
|
||||
from openhands.sdk import Agent, AgentContext, LocalWorkspace
|
||||
from openhands.sdk.llm import LLM
|
||||
from openhands.sdk.secret import LookupSecret, StaticSecret
|
||||
from openhands.sdk.secret import LookupSecret, SecretValue, StaticSecret
|
||||
from openhands.sdk.utils.paging import page_iterator
|
||||
from openhands.sdk.workspace.remote.async_remote_workspace import AsyncRemoteWorkspace
|
||||
from openhands.server.types import AppMode
|
||||
@@ -856,7 +856,7 @@ class LiveStatusAppConversationService(AppConversationServiceBase):
|
||||
system_message_suffix: str | None,
|
||||
mcp_config: dict,
|
||||
condenser_max_size: int | None,
|
||||
secrets: dict | None = None,
|
||||
secrets: dict[str, SecretValue] | None = None,
|
||||
) -> Agent:
|
||||
"""Create an agent with appropriate tools and context based on agent type.
|
||||
|
||||
@@ -966,7 +966,7 @@ class LiveStatusAppConversationService(AppConversationServiceBase):
|
||||
user: UserInfo,
|
||||
workspace: LocalWorkspace,
|
||||
initial_message: SendMessageRequest | None,
|
||||
secrets: dict,
|
||||
secrets: dict[str, SecretValue],
|
||||
sandbox: SandboxInfo,
|
||||
remote_workspace: AsyncRemoteWorkspace | None,
|
||||
selected_repository: str | None,
|
||||
|
||||
Reference in New Issue
Block a user