github/OpenHands
1
0
Fork 0
mirror of https://github.com/OpenHands/OpenHands.git synced 2026-03-22 13:47:19 +08:00
Code Issues Packages Projects Releases Wiki Activity

Remove github_user_id in favor of user_id (#8406)

Browse Source 
Co-authored-by: openhands <openhands@all-hands.dev>
This commit is contained in:
Robert Brennan
2025-05-14 10:17:35 -04:00
committed by GitHub
parent c36cbf6543
commit 1b57fd4d1e
13 changed files with 18 additions and 69 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
openhands/server/conversation_manager/conversation_manager.py
View File

@@ -53,7 +53,6 @@ class ConversationManager(ABC):
connection_id: str,
settings: Settings,
user_id: str | None,
github_user_id: str | None,
) -> EventStore | None:
"""Join a conversation and return its event stream."""
@@ -82,7 +81,6 @@ class ConversationManager(ABC):
user_id: str | None,
initial_user_msg: MessageAction | None = None,
replay_json: str | None = None,
github_user_id: str | None = None,
) -> EventStore:
"""Start an event loop if one is not already running"""

26
openhands/server/conversation_manager/standalone_conversation_manager.py
View File

@@ -119,7 +119,6 @@ class StandaloneConversationManager(ConversationManager):
connection_id: str,
settings: Settings,
user_id: str | None,
github_user_id: str | None,
) -> EventStore:
logger.info(
f'join_conversation:{sid}:{connection_id}',
@@ -127,9 +126,7 @@ class StandaloneConversationManager(ConversationManager):
)
await self.sio.enter_room(connection_id, ROOM_KEY.format(sid=sid))
self._local_connection_id_to_session_id[connection_id] = sid
event_stream = await self.maybe_start_agent_loop(
sid, settings, user_id, github_user_id=github_user_id
)
event_stream = await self.maybe_start_agent_loop(sid, settings, user_id)
if not event_stream:
logger.error(
f'No event stream after joining conversation: {sid}',
@@ -197,9 +194,7 @@ class StandaloneConversationManager(ConversationManager):
logger.error('error_cleaning_stale')
await asyncio.sleep(_CLEANUP_INTERVAL)
async def _get_conversation_store(
self, user_id: str | None, github_user_id: str | None
) -> ConversationStore:
async def _get_conversation_store(self, user_id: str | None) -> ConversationStore:
conversation_store_class = self._conversation_store_class
if not conversation_store_class:
self._conversation_store_class = conversation_store_class = get_impl(
@@ -256,12 +251,11 @@ class StandaloneConversationManager(ConversationManager):
user_id: str | None,
initial_user_msg: MessageAction | None = None,
replay_json: str | None = None,
github_user_id: str | None = None,
) -> EventStore:
logger.info(f'maybe_start_agent_loop:{sid}', extra={'session_id': sid})
if not await self.is_agent_loop_running(sid):
await self._start_agent_loop(
sid, settings, user_id, initial_user_msg, replay_json, github_user_id
sid, settings, user_id, initial_user_msg, replay_json
)
event_store = await self._get_event_store(sid, user_id)
@@ -280,7 +274,6 @@ class StandaloneConversationManager(ConversationManager):
user_id: str | None,
initial_user_msg: MessageAction | None = None,
replay_json: str | None = None,
github_user_id: str | None = None,
) -> Session:
logger.info(f'starting_agent_loop:{sid}', extra={'session_id': sid})
@@ -291,9 +284,7 @@ class StandaloneConversationManager(ConversationManager):
extra={'session_id': sid, 'user_id': user_id},
)
# Get the conversations sorted (oldest first)
conversation_store = await self._get_conversation_store(
user_id, github_user_id
)
conversation_store = await self._get_conversation_store(user_id)
conversations = await conversation_store.get_all_metadata(response_ids)
conversations.sort(key=_last_updated_at_key, reverse=True)
@@ -332,9 +323,7 @@ class StandaloneConversationManager(ConversationManager):
try:
session.agent_session.event_stream.subscribe(
EventStreamSubscriber.SERVER,
self._create_conversation_update_callback(
user_id, github_user_id, sid, settings
),
self._create_conversation_update_callback(user_id, sid, settings),
UPDATED_AT_CALLBACK_ID,
)
except ValueError:
@@ -433,7 +422,6 @@ class StandaloneConversationManager(ConversationManager):
def _create_conversation_update_callback(
self,
user_id: str | None,
github_user_id: str | None,
conversation_id: str,
settings: Settings,
) -> Callable:
@@ -442,7 +430,6 @@ class StandaloneConversationManager(ConversationManager):
self._update_conversation_for_event,
GENERAL_TIMEOUT,
user_id,
github_user_id,
conversation_id,
settings,
event,
@@ -453,12 +440,11 @@ class StandaloneConversationManager(ConversationManager):
async def _update_conversation_for_event(
self,
user_id: str,
github_user_id: str,
conversation_id: str,
settings: Settings,
event=None,
):
conversation_store = await self._get_conversation_store(user_id, github_user_id)
conversation_store = await self._get_conversation_store(user_id)
conversation = await conversation_store.get_metadata(conversation_id)
conversation.last_updated_at = datetime.now(timezone.utc)

3
openhands/server/listen_socket.py
View File

@@ -77,7 +77,7 @@ async def connect(connection_id: str, environ: dict) -> None:
# Headers in WSGI/ASGI are prefixed with 'HTTP_' and have dashes replaced with underscores
authorization_header = environ.get('HTTP_AUTHORIZATION', None)
conversation_validator = create_conversation_validator()
user_id, github_user_id = await conversation_validator.validate(
user_id = await conversation_validator.validate(
conversation_id, cookies_str, authorization_header
)
@@ -108,7 +108,6 @@ async def connect(connection_id: str, environ: dict) -> None:
connection_id,
conversation_init_data,
user_id,
github_user_id,
)
logger.info(
f'Connected to conversation {conversation_id} with connection_id {connection_id}. Replaying event stream...'

1
openhands/server/routes/manage_conversations.py
View File

@@ -129,7 +129,6 @@ async def _create_new_conversation(
conversation_id=conversation_id,
title=conversation_title,
user_id=user_id,
github_user_id=None,
selected_repository=selected_repository,
selected_branch=selected_branch,
)

11
openhands/server/user_auth/__init__.py
View File

@@ -2,7 +2,6 @@ from fastapi import Request
from pydantic import SecretStr
from openhands.integrations.provider import PROVIDER_TOKEN_TYPE
from openhands.integrations.service_types import ProviderType
from openhands.server.settings import Settings
from openhands.server.user_auth.user_auth import AuthType, get_user_auth
from openhands.storage.data_models.user_secrets import UserSecrets
@@ -28,16 +27,6 @@ async def get_user_id(request: Request) -> str | None:
return user_id
async def get_github_user_id(request: Request) -> str | None:
provider_tokens = await get_provider_tokens(request)
if not provider_tokens:
return None
github_provider = provider_tokens.get(ProviderType.GITHUB)
if github_provider:
return github_provider.user_id
return None
async def get_user_settings(request: Request) -> Settings | None:
user_auth = await get_user_auth(request)
user_settings = await user_auth.get_user_settings()

7
openhands/storage/conversation/conversation_store.py
View File

@@ -23,14 +23,11 @@ class ConversationStore(ABC):
"""Load conversation metadata."""
async def validate_metadata(
self, conversation_id: str, user_id: str, github_user_id: str
self, conversation_id: str, user_id: str
) -> bool:
"""Validate that conversation belongs to the current user."""
# TODO: remove github_user_id after transition to Keycloak is complete.
metadata = await self.get_metadata(conversation_id)
if (not metadata.user_id and not metadata.github_user_id) or (
metadata.user_id != user_id and metadata.github_user_id != github_user_id
):
if not metadata.user_id or metadata.user_id != user_id:
return False
else:
return True

4
openhands/storage/conversation/conversation_validator.py
View File

@@ -11,8 +11,8 @@ class ConversationValidator:
conversation_id: str,
cookies_str: str,
authorization_header: str | None = None,
) -> tuple[None, None]:
return None, None
) -> str | None:
return None
def create_conversation_validator() -> ConversationValidator:

8
openhands/storage/conversation/file_conversation_store.py
View File

@@ -38,12 +38,14 @@ class FileConversationStore(ConversationStore):
path = self.get_conversation_metadata_filename(conversation_id)
json_str = await call_sync_from_async(self.file_store.read, path)
# Temp: force int to str to stop pydandic being, well... pedantic
# Validate the JSON
json_obj = json.loads(json_str)
if 'created_at' not in json_obj:
raise FileNotFoundError(path)
if isinstance(json_obj.get('github_user_id'), int):
json_obj['github_user_id'] = str(json_obj.get('github_user_id'))
# Remove github_user_id if it exists
if 'github_user_id' in json_obj:
json_obj.pop('github_user_id')
result = conversation_metadata_type_adapter.validate_python(json_obj)
return result

1
openhands/storage/data_models/conversation_metadata.py
View File

@@ -14,7 +14,6 @@ class ConversationTrigger(Enum):
@dataclass
class ConversationMetadata:
conversation_id: str
github_user_id: str | None
selected_repository: str | None
user_id: str | None = None
selected_branch: str | None = None

5
tests/unit/test_auto_generate_title.py
View File

@@ -190,7 +190,6 @@ async def test_update_conversation_with_title():
# Create test conversation
conversation_id = 'test-conversation'
user_id = 'test-user'
github_user_id = 'test-github-user'
# Create test settings
settings = Settings(
@@ -223,9 +222,7 @@ async def test_update_conversation_with_title():
AsyncMock(return_value='Generated Title'),
):
# Call the method
await manager._update_conversation_for_event(
user_id, github_user_id, conversation_id, settings
)
await manager._update_conversation_for_event(user_id, conversation_id, settings)
# Verify the title was updated
assert mock_metadata.title == 'Generated Title'

4
tests/unit/test_conversation.py
View File

@@ -48,7 +48,6 @@ def _patch_store():
'title': 'Some Conversation',
'selected_repository': 'foobar',
'conversation_id': 'some_conversation_id',
'github_user_id': '12345',
'user_id': '12345',
'created_at': '2025-01-01T00:00:00+00:00',
'last_updated_at': '2025-01-01T00:01:00+00:00',
@@ -139,7 +138,6 @@ async def test_search_conversations():
'2025-01-01T00:01:00+00:00'
),
selected_repository='foobar',
github_user_id='12345',
user_id='12345',
)
]
@@ -185,7 +183,6 @@ async def test_get_conversation():
created_at=datetime.fromisoformat('2025-01-01T00:00:00+00:00'),
last_updated_at=datetime.fromisoformat('2025-01-01T00:01:00+00:00'),
selected_repository='foobar',
github_user_id='12345',
user_id='12345',
)
)
@@ -409,7 +406,6 @@ async def test_delete_conversation():
created_at=datetime.fromisoformat('2025-01-01T00:00:00+00:00'),
last_updated_at=datetime.fromisoformat('2025-01-01T00:01:00+00:00'),
selected_repository='foobar',
github_user_id='12345',
user_id='12345',
)
)

10
tests/unit/test_file_conversation_store.py
View File

@@ -14,7 +14,6 @@ async def test_load_store():
expected = ConversationMetadata(
conversation_id='some-conversation-id',
user_id='some-user-id',
github_user_id='12345',
selected_repository='some-repo',
title="Let's talk about trains",
)
@@ -31,7 +30,6 @@ async def test_load_int_user_id():
get_conversation_metadata_filename('some-conversation-id'): json.dumps(
{
'conversation_id': 'some-conversation-id',
'github_user_id': 12345,
'user_id': '67890',
'selected_repository': 'some-repo',
'title': "Let's talk about trains",
@@ -42,7 +40,6 @@ async def test_load_int_user_id():
)
)
found = await store.get_metadata('some-conversation-id')
assert found.github_user_id == '12345'
assert found.user_id == '67890'
@@ -63,7 +60,6 @@ async def test_search_basic():
get_conversation_metadata_filename('conv1'): json.dumps(
{
'conversation_id': 'conv1',
'github_user_id': '123',
'user_id': '123',
'selected_repository': 'repo1',
'title': 'First conversation',
@@ -73,7 +69,6 @@ async def test_search_basic():
get_conversation_metadata_filename('conv2'): json.dumps(
{
'conversation_id': 'conv2',
'github_user_id': '123',
'user_id': '123',
'selected_repository': 'repo1',
'title': 'Second conversation',
@@ -83,7 +78,6 @@ async def test_search_basic():
get_conversation_metadata_filename('conv3'): json.dumps(
{
'conversation_id': 'conv3',
'github_user_id': '123',
'user_id': '123',
'selected_repository': 'repo1',
'title': 'Third conversation',
@@ -112,7 +106,6 @@ async def test_search_pagination():
get_conversation_metadata_filename(f'conv{i}'): json.dumps(
{
'conversation_id': f'conv{i}',
'github_user_id': '123',
'user_id': '123',
'selected_repository': 'repo1',
'title': f'Conversation {i}',
@@ -154,7 +147,6 @@ async def test_search_with_invalid_conversation():
get_conversation_metadata_filename('conv1'): json.dumps(
{
'conversation_id': 'conv1',
'github_user_id': '123',
'user_id': '123',
'selected_repository': 'repo1',
'title': 'Valid conversation',
@@ -183,7 +175,6 @@ async def test_get_all_metadata():
get_conversation_metadata_filename('conv1'): json.dumps(
{
'conversation_id': 'conv1',
'github_user_id': '123',
'user_id': '123',
'selected_repository': 'repo1',
'title': 'First conversation',
@@ -193,7 +184,6 @@ async def test_get_all_metadata():
get_conversation_metadata_filename('conv2'): json.dumps(
{
'conversation_id': 'conv2',
'github_user_id': '123',
'user_id': '123',
'selected_repository': 'repo1',
'title': 'Second conversation',

5
tests/unit/test_standalone_conversation_manager.py
View File

@@ -74,7 +74,6 @@ async def test_init_new_local_session():
'new-session-id',
ConversationInitData(),
1,
'12345',
)
assert session_instance.initialize_agent.call_count == 1
assert sio.enter_room.await_count == 1
@@ -119,14 +118,12 @@ async def test_join_local_session():
'new-session-id',
ConversationInitData(),
None,
'12345',
)
await conversation_manager.join_conversation(
'new-session-id',
'new-session-id',
ConversationInitData(),
None,
'12345',
)
assert session_instance.initialize_agent.call_count == 1
assert sio.enter_room.await_count == 2
@@ -159,7 +156,7 @@ async def test_add_to_local_event_stream():
'new-session-id', ConversationInitData(), 1
)
await conversation_manager.join_conversation(
'new-session-id', 'connection-id', ConversationInitData(), 1, '12345'
'new-session-id', 'connection-id', ConversationInitData(), 1
)
await conversation_manager.send_to_event_stream(
'connection-id', {'event_type': 'some_event'}