diff --git a/.github/workflows/ghcr-build.yml b/.github/workflows/ghcr-build.yml index f57c01b6945a..1a36885c7f57 100644 --- a/.github/workflows/ghcr-build.yml +++ b/.github/workflows/ghcr-build.yml @@ -108,7 +108,7 @@ jobs: packages: write steps: - name: Login to GHCR - uses: docker/login-action@v3 + uses: docker/login-action@v4 with: registry: ghcr.io username: ${{ github.repository_owner }} @@ -202,7 +202,7 @@ jobs: echo "EOF" >> "$GITHUB_OUTPUT" - name: Build and push runtime image ${{ matrix.base_image.image }} if: github.event.pull_request.head.repo.fork != true - uses: docker/build-push-action@v6 + uses: docker/build-push-action@v7 with: push: true tags: ${{ env.DOCKER_TAGS }} @@ -215,7 +215,7 @@ jobs: # Forked repos can't push to GHCR, so we just build in order to populate the cache for rebuilding - name: Build runtime image ${{ matrix.base_image.image }} for fork if: github.event.pull_request.head.repo.fork - uses: docker/build-push-action@v6 + uses: docker/build-push-action@v7 with: tags: ghcr.io/${{ env.REPO_OWNER }}/runtime:${{ env.RELEVANT_SHA }}-${{ matrix.base_image.tag }} context: containers/runtime @@ -236,7 +236,7 @@ jobs: packages: write steps: - name: Login to GHCR - uses: docker/login-action@v3 + uses: docker/login-action@v4 with: registry: ghcr.io username: ${{ github.repository_owner }} @@ -324,7 +324,7 @@ jobs: # Also compute base tags (no arch suffix) for the merge job output - name: Extract base metadata for merge id: meta_base - uses: docker/metadata-action@v5 + uses: docker/metadata-action@v6 with: images: ghcr.io/openhands/enterprise-server tags: | @@ -350,7 +350,7 @@ jobs: # rather than a mutable branch tag like "main" which can serve stale cached layers. echo "OPENHANDS_DOCKER_TAG=${RELEVANT_SHA}" >> $GITHUB_ENV - name: Build and push Docker image - uses: docker/build-push-action@v6 + uses: docker/build-push-action@v7 with: context: . file: enterprise/Dockerfile @@ -375,7 +375,7 @@ jobs: if: github.event.pull_request.head.repo.fork != true steps: - name: Login to GHCR - uses: docker/login-action@v3 + uses: docker/login-action@v4 with: registry: ghcr.io username: ${{ github.repository_owner }} diff --git a/.github/workflows/openhands-resolver.yml b/.github/workflows/openhands-resolver.yml index 27facde4bed2..05de82c1fa1b 100644 --- a/.github/workflows/openhands-resolver.yml +++ b/.github/workflows/openhands-resolver.yml @@ -192,7 +192,7 @@ jobs: echo "TARGET_BRANCH=${{ inputs.target_branch || 'main' }}" >> $GITHUB_ENV - name: Comment on issue with start message - uses: actions/github-script@v7 + uses: actions/github-script@v9 with: github-token: ${{ secrets.PAT_TOKEN || github.token }} script: | @@ -206,7 +206,7 @@ jobs: - name: Install OpenHands id: install_openhands - uses: actions/github-script@v7 + uses: actions/github-script@v9 env: COMMENT_BODY: ${{ github.event.comment.body || '' }} REVIEW_BODY: ${{ github.event.review.body || '' }} @@ -305,7 +305,7 @@ jobs: # Step leaves comment for when agent is invoked on PR - name: Analyze Push Logs (Updated PR or No Changes) # Skip comment if PR update was successful OR leave comment if the agent made no code changes - uses: actions/github-script@v7 + uses: actions/github-script@v9 if: always() env: AGENT_RESPONDED: ${{ env.AGENT_RESPONDED || 'false' }} @@ -341,7 +341,7 @@ jobs: # Step leaves comment for when agent is invoked on issue - name: Comment on issue # Comment link to either PR or branch created by agent - uses: actions/github-script@v7 + uses: actions/github-script@v9 if: always() # Comment on issue even if the previous steps fail env: AGENT_RESPONDED: ${{ env.AGENT_RESPONDED || 'false' }} @@ -416,7 +416,7 @@ jobs: # Leave error comment when both PR/Issue comment handling fail - name: Fallback Error Comment - uses: actions/github-script@v7 + uses: actions/github-script@v9 if: ${{ env.AGENT_RESPONDED == 'false' }} # Only run if no conditions were met in previous steps env: ISSUE_NUMBER: ${{ env.ISSUE_NUMBER }} diff --git a/.github/workflows/pr-artifacts.yml b/.github/workflows/pr-artifacts.yml index f0c6ee0f80c9..0134be3757dd 100644 --- a/.github/workflows/pr-artifacts.yml +++ b/.github/workflows/pr-artifacts.yml @@ -59,7 +59,7 @@ jobs: - name: Update PR comment after cleanup if: steps.check-fork.outputs.is_fork == 'false' && steps.remove.outputs.removed == 'true' - uses: actions/github-script@v7 + uses: actions/github-script@v9 with: script: | const marker = ''; @@ -107,7 +107,7 @@ jobs: - name: Post or update PR comment if: steps.check.outputs.exists == 'true' - uses: actions/github-script@v7 + uses: actions/github-script@v9 with: script: | const marker = ''; diff --git a/.github/workflows/py-tests.yml b/.github/workflows/py-tests.yml index e0985117c7df..88945a16f8e6 100644 --- a/.github/workflows/py-tests.yml +++ b/.github/workflows/py-tests.yml @@ -115,7 +115,7 @@ jobs: steps: - uses: actions/checkout@v6 - - uses: actions/download-artifact@v7 + - uses: actions/download-artifact@v8 id: download with: pattern: coverage-* diff --git a/.github/workflows/welcome-good-first-issue.yml b/.github/workflows/welcome-good-first-issue.yml index 4bedc4a48b1a..a23edf8c08cc 100644 --- a/.github/workflows/welcome-good-first-issue.yml +++ b/.github/workflows/welcome-good-first-issue.yml @@ -14,7 +14,7 @@ jobs: steps: - name: Check if welcome comment already exists id: check_comment - uses: actions/github-script@v7 + uses: actions/github-script@v9 with: result-encoding: string script: | @@ -33,7 +33,7 @@ jobs: - name: Leave welcome comment if: steps.check_comment.outputs.result == 'false' - uses: actions/github-script@v7 + uses: actions/github-script@v9 with: script: | const repoUrl = `https://github.com/${context.repo.owner}/${context.repo.repo}`; diff --git a/AGENTS.md b/AGENTS.md index f352a51a0a02..4351a9dc5cbd 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -226,6 +226,7 @@ Each integration follows a consistent pattern with service classes, storage mode - Database changes require careful migration planning in `enterprise/migrations/` - Always test changes in both OpenHands and enterprise contexts - Use the enterprise-specific Makefile commands for development +- When the `openhands-ai` package (root project) version has been updated, run `poetry lock` in the `enterprise/` folder to update the version in the enterprise poetry lockfile. **Enterprise Testing Best Practices:** diff --git a/enterprise/poetry.lock b/enterprise/poetry.lock index cadac938cd7d..ad4ea1ac6b52 100644 --- a/enterprise/poetry.lock +++ b/enterprise/poetry.lock @@ -12131,14 +12131,14 @@ requests-toolbelt = ">=0.6.0" [[package]] name = "python-multipart" -version = "0.0.22" +version = "0.0.26" description = "A streaming multipart parser for Python" optional = false python-versions = ">=3.10" groups = ["main"] files = [ - {file = "python_multipart-0.0.22-py3-none-any.whl", hash = "sha256:2b2cd894c83d21bf49d702499531c7bafd057d730c201782048f7945d82de155"}, - {file = "python_multipart-0.0.22.tar.gz", hash = "sha256:7340bef99a7e0032613f56dc36027b959fd3b30a787ed62d310e951f7c3a3a58"}, + {file = "python_multipart-0.0.26-py3-none-any.whl", hash = "sha256:c0b169f8c4484c13b0dcf2ef0ec3a4adb255c4b7d18d8e420477d2b1dd03f185"}, + {file = "python_multipart-0.0.26.tar.gz", hash = "sha256:08fadc45918cd615e26846437f50c5d6d23304da32c341f289a617127b081f17"}, ] [[package]] diff --git a/enterprise/server/routes/users_v1.py b/enterprise/server/routes/users_v1.py index 902643ade1d7..6bc7cb7e4147 100644 --- a/enterprise/server/routes/users_v1.py +++ b/enterprise/server/routes/users_v1.py @@ -12,7 +12,10 @@ from server.auth.saas_user_auth import SaasUserAuth from server.models.user_models import SaasUserInfo -from openhands.app_server.config import depends_user_context +from openhands.app_server.config import ( + depends_user_context, + resolve_provider_llm_base_url, +) from openhands.app_server.sandbox.session_auth import validate_session_key_ownership from openhands.app_server.user.auth_user_context import AuthUserContext from openhands.app_server.user.user_context import UserContext @@ -42,8 +45,9 @@ def _inject_sdk_compat_fields( """ agent_settings = content.get('agent_settings') or {} llm = agent_settings.get('llm') or {} - content['llm_model'] = llm.get('model') - content['llm_base_url'] = llm.get('base_url') + model = llm.get('model') + content['llm_model'] = model + content['llm_base_url'] = resolve_provider_llm_base_url(model, llm.get('base_url')) if include_api_key: content['llm_api_key'] = llm.get('api_key') content['mcp_config'] = agent_settings.get('mcp_config') diff --git a/enterprise/tests/unit/test_auth_routes.py b/enterprise/tests/unit/test_auth_routes.py index 5ebda80769f8..82507c6b24db 100644 --- a/enterprise/tests/unit/test_auth_routes.py +++ b/enterprise/tests/unit/test_auth_routes.py @@ -1281,7 +1281,7 @@ class TestKeycloakCallbackRecaptcha: """Tests for reCAPTCHA integration in keycloak_callback().""" @pytest.mark.asyncio - async def test_should_verify_recaptcha_and_allow_login_when_score_is_high( + async def test_login_allows_when_recaptcha_score_is_high( self, mock_request, create_keycloak_user_info ): """Test that login proceeds when reCAPTCHA score is high.""" @@ -1369,7 +1369,7 @@ async def test_should_verify_recaptcha_and_allow_login_when_score_is_high( mock_recaptcha_service.create_assessment.assert_called_once() @pytest.mark.asyncio - async def test_should_block_login_when_recaptcha_score_is_low( + async def test_login_blocks_when_recaptcha_score_is_low( self, mock_request, create_keycloak_user_info ): """Test that login is blocked and redirected when reCAPTCHA score is low.""" @@ -1439,7 +1439,7 @@ async def test_should_block_login_when_recaptcha_score_is_low( assert 'recaptcha_blocked=true' in result.headers['location'] @pytest.mark.asyncio - async def test_should_extract_ip_from_x_forwarded_for_header( + async def test_login_extracts_ip_from_x_forwarded_for( self, mock_request, create_keycloak_user_info ): """Test that IP is extracted from X-Forwarded-For header when present.""" @@ -1528,7 +1528,7 @@ async def test_should_extract_ip_from_x_forwarded_for_header( assert call_args[1]['user_ip'] == '192.168.1.1' @pytest.mark.asyncio - async def test_should_use_client_host_when_x_forwarded_for_missing( + async def test_login_uses_client_host_when_x_forwarded_for_missing( self, mock_request, create_keycloak_user_info ): """Test that client.host is used when X-Forwarded-For is missing.""" @@ -1618,7 +1618,7 @@ async def test_should_use_client_host_when_x_forwarded_for_missing( assert call_args[1]['user_ip'] == '192.168.1.2' @pytest.mark.asyncio - async def test_should_use_unknown_ip_when_client_is_none( + async def test_login_uses_unknown_ip_when_client_is_none( self, mock_request, create_keycloak_user_info ): """Test that 'unknown' IP is used when client is None.""" @@ -1707,7 +1707,7 @@ async def test_should_use_unknown_ip_when_client_is_none( assert call_args[1]['user_ip'] == 'unknown' @pytest.mark.asyncio - async def test_should_include_email_in_assessment_when_available( + async def test_login_includes_email_in_assessment( self, mock_request, create_keycloak_user_info ): """Test that email is included in assessment when available.""" @@ -1793,7 +1793,7 @@ async def test_should_include_email_in_assessment_when_available( assert call_args[1]['email'] == 'user@example.com' @pytest.mark.asyncio - async def test_should_skip_recaptcha_when_site_key_not_configured( + async def test_login_skips_recaptcha_when_site_key_not_configured( self, mock_request, create_keycloak_user_info ): """Test that reCAPTCHA is skipped when RECAPTCHA_SITE_KEY is not configured.""" @@ -1870,7 +1870,7 @@ async def test_should_skip_recaptcha_when_site_key_not_configured( mock_recaptcha_service.create_assessment.assert_not_called() @pytest.mark.asyncio - async def test_should_skip_recaptcha_when_token_is_missing( + async def test_login_skips_recaptcha_when_token_is_missing( self, mock_request, create_keycloak_user_info ): """Test that reCAPTCHA is skipped when token is missing from state.""" @@ -1941,7 +1941,7 @@ async def test_should_skip_recaptcha_when_token_is_missing( mock_recaptcha_service.create_assessment.assert_not_called() @pytest.mark.asyncio - async def test_should_fail_open_when_recaptcha_service_throws_exception( + async def test_login_fails_open_when_recaptcha_raises( self, mock_request, create_keycloak_user_info ): """Test that login proceeds (fail open) when reCAPTCHA service throws exception.""" @@ -2029,7 +2029,7 @@ async def test_should_fail_open_when_recaptcha_service_throws_exception( assert len(recaptcha_error_calls) > 0 @pytest.mark.asyncio - async def test_should_log_warning_when_recaptcha_blocks_user( + async def test_login_logs_warning_when_recaptcha_blocks( self, mock_request, create_keycloak_user_info ): """Test that warning is logged when reCAPTCHA blocks user.""" diff --git a/enterprise/tests/unit/test_maintenance_task_runner_standalone.py b/enterprise/tests/unit/test_maintenance_task_runner_standalone.py index 6de9a2dcf138..de43ae39e44b 100644 --- a/enterprise/tests/unit/test_maintenance_task_runner_standalone.py +++ b/enterprise/tests/unit/test_maintenance_task_runner_standalone.py @@ -24,6 +24,8 @@ def test_runner_initialization(self): # Mock the runner class structure class MockMaintenanceTaskRunner: + """Minimal mock of MaintenanceTaskRunner to verify initialization state.""" + def __init__(self): self._running = False self._task = None @@ -38,6 +40,8 @@ async def test_start_stop_lifecycle(self): # Mock the runner behavior class MockMaintenanceTaskRunner: + """Mock runner for verifying start/stop lifecycle behaviour.""" + def __init__(self): self._running: bool = False self._task = None @@ -45,6 +49,7 @@ def __init__(self): self.stop_called = False async def start(self): + """Start the runner.""" if self._running: return self._running = True @@ -52,6 +57,7 @@ async def start(self): self.start_called = True async def stop(self): + """Stop the runner.""" if not self._running: return self._running = False @@ -90,12 +96,15 @@ async def test_run_loop_behavior(self): # Mock the run loop logic class MockMaintenanceTaskRunner: + """Mock runner for verifying run loop iteration behaviour.""" + def __init__(self): self._running = False self.process_calls = 0 self.sleep_calls = 0 async def _run_loop(self): + """Execute the main processing loop.""" loop_count = 0 while self._running and loop_count < 3: # Limit for testing try: @@ -113,6 +122,7 @@ async def _run_loop(self): loop_count += 1 async def _process_pending_tasks(self): + """Process pending tasks.""" # Mock processing pass @@ -131,6 +141,8 @@ async def test_run_loop_error_handling(self): """Test error handling in the run loop.""" class MockMaintenanceTaskRunner: + """Mock runner for verifying error handling in the run loop.""" + def __init__(self): self._running = False self.error_count = 0 @@ -138,6 +150,7 @@ def __init__(self): self.attempt_count = 0 async def _run_loop(self): + """Execute the main processing loop.""" loop_count = 0 while self._running and loop_count < 2: # Limit for testing try: @@ -155,6 +168,7 @@ async def _run_loop(self): loop_count += 1 async def _process_pending_tasks(self): + """Process pending tasks.""" self.attempt_count += 1 # Only fail on the first attempt if self.attempt_count == 1: @@ -231,6 +245,8 @@ async def test_task_processing_success(self): # Mock task processing logic class MockTask: + """Mock task for successful processing scenarios.""" + def __init__(self, task_id, processor_type): self.id = task_id self.processor_type = processor_type @@ -239,17 +255,21 @@ def __init__(self, task_id, processor_type): self.updated_at = None def get_processor(self): + """Return a mock processor.""" # Mock processor processor = AsyncMock() processor.return_value = {'result': 'success', 'processed_items': 5} return processor class MockMaintenanceTaskRunner: + """Mock runner for verifying task status transitions on success.""" + def __init__(self): self.status_updates = [] self.commits = [] async def _process_task(self, task): + """Process a single task.""" # Simulate updating status to WORKING task.status = 'WORKING' task.updated_at = datetime.now() @@ -297,6 +317,8 @@ async def test_task_processing_failure(self): """Test task processing with failure.""" class MockTask: + """Mock task for failure processing scenarios.""" + def __init__(self, task_id, processor_type): self.id = task_id self.processor_type = processor_type @@ -305,17 +327,21 @@ def __init__(self, task_id, processor_type): self.updated_at = None def get_processor(self): + """Return a mock processor.""" # Mock processor that fails processor = AsyncMock() processor.side_effect = ValueError('Processing failed') return processor class MockMaintenanceTaskRunner: + """Mock runner for verifying task status transitions on failure.""" + def __init__(self): self.status_updates = [] self.error_logged = None async def _process_task(self, task): + """Process a single task.""" # Simulate updating status to WORKING task.status = 'WORKING' task.updated_at = datetime.now() @@ -368,6 +394,8 @@ def test_database_session_handling_pattern(self): # Mock the session handling logic class MockSession: + """Mock database session for verifying session usage patterns.""" + def __init__(self): self.queries = [] self.merges = [] @@ -541,6 +569,8 @@ async def test_concurrent_task_processing(self): """Test handling of multiple tasks in sequence.""" class MockTask: + """Mock task with configurable success or failure behaviour.""" + def __init__(self, task_id, should_fail=False): self.id = task_id self.processor_type = f'processor_{task_id}' @@ -548,6 +578,7 @@ def __init__(self, task_id, should_fail=False): self.should_fail = should_fail def get_processor(self): + """Return a mock processor.""" processor = AsyncMock() if self.should_fail: processor.side_effect = Exception(f'Task {self.id} failed') @@ -556,12 +587,15 @@ def get_processor(self): return processor class MockMaintenanceTaskRunner: + """Mock runner for verifying sequential processing of multiple tasks.""" + def __init__(self): self.processed_tasks = [] self.successful_tasks = [] self.failed_tasks = [] async def _process_pending_tasks(self): + """Process all pending tasks.""" # Simulate finding multiple tasks tasks = [ MockTask(1, should_fail=False), @@ -573,6 +607,7 @@ async def _process_pending_tasks(self): await self._process_task(task) async def _process_task(self, task): + """Process a single task.""" self.processed_tasks.append(task.id) try: @@ -610,6 +645,8 @@ def test_global_instance_pattern(self): # Mock the global instance pattern class MockMaintenanceTaskRunner: + """Mock runner for verifying the global singleton instance pattern.""" + def __init__(self): self.instance_id = id(self) @@ -628,11 +665,14 @@ async def test_cancellation_handling(self): """Test proper handling of task cancellation.""" class MockMaintenanceTaskRunner: + """Mock runner for verifying cancellation handling in the run loop.""" + def __init__(self): self._running = False self.cancellation_handled = False async def _run_loop(self): + """Execute the main processing loop.""" try: while self._running: await asyncio.sleep(0.01) diff --git a/enterprise/tests/unit/test_recaptcha_service.py b/enterprise/tests/unit/test_recaptcha_service.py index 11ace835c4e4..c8367dbaec2b 100644 --- a/enterprise/tests/unit/test_recaptcha_service.py +++ b/enterprise/tests/unit/test_recaptcha_service.py @@ -35,7 +35,7 @@ def recaptcha_service(mock_gcp_client): class TestRecaptchaServiceHashAccountId: """Tests for RecaptchaService.hash_account_id().""" - def test_should_hash_email_with_hmac_sha256(self, recaptcha_service): + def test_hash_account_id_uses_hmac_sha256(self, recaptcha_service): """Test that hash_account_id produces correct HMAC-SHA256 hash.""" # Arrange email = 'user@example.com' @@ -56,7 +56,7 @@ def test_should_hash_email_with_hmac_sha256(self, recaptcha_service): assert result == expected_hash assert len(result) == 64 # SHA256 produces 64 hex characters - def test_should_normalize_email_to_lowercase(self, recaptcha_service): + def test_hash_account_id_normalizes_to_lowercase(self, recaptcha_service): """Test that hash_account_id normalizes email to lowercase.""" # Arrange email1 = 'User@Example.com' @@ -69,9 +69,7 @@ def test_should_normalize_email_to_lowercase(self, recaptcha_service): # Assert assert hash1 == hash2 - def test_should_produce_different_hashes_for_different_emails( - self, recaptcha_service - ): + def test_hash_account_id_produces_unique_hashes_per_email(self, recaptcha_service): """Test that different emails produce different hashes.""" # Arrange email1 = 'user1@example.com' @@ -88,7 +86,7 @@ def test_should_produce_different_hashes_for_different_emails( class TestRecaptchaServiceCreateAssessment: """Tests for RecaptchaService.create_assessment().""" - def test_should_create_assessment_and_allow_when_score_is_high( + def test_create_assessment_allows_when_score_is_high( self, recaptcha_service, mock_gcp_client ): """Test that assessment allows request when score is above threshold.""" @@ -118,7 +116,7 @@ def test_should_create_assessment_and_allow_when_score_is_high( assert result.action_valid is True mock_gcp_client.create_assessment.assert_called_once() - def test_should_block_when_score_is_below_threshold( + def test_create_assessment_blocks_when_score_is_below_threshold( self, recaptcha_service, mock_gcp_client ): """Test that assessment blocks request when score is below threshold.""" @@ -143,7 +141,7 @@ def test_should_block_when_score_is_below_threshold( assert result.allowed is False assert result.score == 0.2 - def test_should_block_when_token_is_invalid( + def test_create_assessment_blocks_when_token_is_invalid( self, recaptcha_service, mock_gcp_client ): """Test that assessment blocks request when token is invalid.""" @@ -168,7 +166,7 @@ def test_should_block_when_token_is_invalid( assert result.allowed is False assert result.valid is False - def test_should_block_when_action_does_not_match( + def test_create_assessment_blocks_when_action_does_not_match( self, recaptcha_service, mock_gcp_client ): """Test that assessment blocks request when action doesn't match.""" @@ -193,7 +191,7 @@ def test_should_block_when_action_does_not_match( assert result.allowed is False assert result.action_valid is False - def test_should_include_email_in_user_info_when_provided( + def test_create_assessment_includes_email_in_user_info( self, recaptcha_service, mock_gcp_client ): """Test that email is included in user_info when provided.""" @@ -223,7 +221,7 @@ def test_should_include_email_in_user_info_when_provided( assert len(assessment.event.user_info.user_ids) == 1 assert assessment.event.user_info.user_ids[0].email == 'user@example.com' - def test_should_not_include_user_info_when_email_is_none( + def test_create_assessment_omits_user_info_when_email_is_none( self, recaptcha_service, mock_gcp_client ): """Test that user_info is not included when email is None.""" @@ -255,7 +253,7 @@ def test_should_not_include_user_info_when_email_is_none( # If user_info exists, verify account_id is empty (not set) assert not assessment.event.user_info.account_id - def test_should_log_assessment_details_including_name( + def test_create_assessment_logs_assessment_details( self, recaptcha_service, mock_gcp_client ): """Test that assessment details including assessment name are logged.""" @@ -290,7 +288,7 @@ def test_should_log_assessment_details_including_name( assert call_kwargs[1]['extra']['action_valid'] is True assert call_kwargs[1]['extra']['user_ip'] == '192.168.1.1' - def test_should_log_user_id_and_email_when_provided( + def test_create_assessment_logs_user_id_and_email( self, recaptcha_service, mock_gcp_client ): """Test that user_id and email are included in log when provided.""" @@ -320,7 +318,7 @@ def test_should_log_user_id_and_email_when_provided( assert call_kwargs[1]['extra']['user_id'] == 'keycloak-user-123' assert call_kwargs[1]['extra']['email'] == 'test@example.com' - def test_should_log_none_for_user_id_and_email_when_not_provided( + def test_create_assessment_logs_none_when_user_info_absent( self, recaptcha_service, mock_gcp_client ): """Test that user_id and email are logged as None when not provided.""" @@ -348,7 +346,7 @@ def test_should_log_none_for_user_id_and_email_when_not_provided( assert call_kwargs[1]['extra']['user_id'] is None assert call_kwargs[1]['extra']['email'] is None - def test_should_raise_exception_when_gcp_client_fails( + def test_create_assessment_raises_when_gcp_client_fails( self, recaptcha_service, mock_gcp_client ): """Test that exceptions from GCP client are propagated.""" diff --git a/frontend/__tests__/hooks/mutation/use-add-git-providers.test.tsx b/frontend/__tests__/hooks/mutation/use-add-git-providers.test.tsx new file mode 100644 index 000000000000..0862dce1ff78 --- /dev/null +++ b/frontend/__tests__/hooks/mutation/use-add-git-providers.test.tsx @@ -0,0 +1,75 @@ +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { renderHook } from "@testing-library/react"; +import React from "react"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { SecretsService } from "#/api/secrets-service"; +import { useAddGitProviders } from "#/hooks/mutation/use-add-git-providers"; +import { Provider, ProviderToken } from "#/types/settings"; + +const mockTrackGitProviderConnected = vi.fn(); + +vi.mock("#/hooks/use-tracking", () => ({ + useTracking: () => ({ + trackGitProviderConnected: mockTrackGitProviderConnected, + }), +})); + +vi.mock("#/context/use-selected-organization", () => ({ + useSelectedOrganizationId: () => ({ organizationId: "org-1" }), +})); + +const buildProviders = ( + overrides: Partial> = {}, +): Record => ({ + github: { token: "", host: null }, + gitlab: { token: "", host: null }, + bitbucket: { token: "", host: null }, + bitbucket_data_center: { token: "", host: null }, + azure_devops: { token: "", host: null }, + forgejo: { token: "", host: null }, + enterprise_sso: { token: "", host: null }, + ...overrides, +}); + +describe("useAddGitProviders", () => { + let queryClient: QueryClient; + + beforeEach(() => { + vi.restoreAllMocks(); + mockTrackGitProviderConnected.mockReset(); + queryClient = new QueryClient({ + defaultOptions: { + queries: { retry: false }, + mutations: { retry: false }, + }, + }); + }); + + it("invalidates personal settings queries after saving providers", async () => { + vi.spyOn(SecretsService, "addGitProvider").mockResolvedValue(true); + + const personalSettingsQueryKey = ["settings", "personal", "org-1"] as const; + queryClient.setQueryData(personalSettingsQueryKey, { + provider_tokens_set: {}, + }); + + const invalidateSpy = vi.spyOn(queryClient, "invalidateQueries"); + + const wrapper = ({ children }: { children: React.ReactNode }) => ( + {children} + ); + + const { result } = renderHook(() => useAddGitProviders(), { wrapper }); + + await result.current.mutateAsync({ + providers: buildProviders({ + github: { token: "ghp_test_123", host: null }, + }), + }); + + expect(invalidateSpy).toHaveBeenCalled(); + expect(queryClient.getQueryState(personalSettingsQueryKey)?.isInvalidated).toBe( + true, + ); + }); +}); diff --git a/frontend/__tests__/router.md b/frontend/__tests__/router.md index b23b4364e745..0a9ce48da9a3 100644 --- a/frontend/__tests__/router.md +++ b/frontend/__tests__/router.md @@ -217,9 +217,9 @@ expect(screen.getByTestId("settings-screen")).toBeInTheDocument(); ### Codebase Examples -- [settings.test.tsx](__tests__/routes/settings.test.tsx) - `createRoutesStub` with nested routes and loaders -- [home-screen.test.tsx](__tests__/routes/home-screen.test.tsx) - `createRoutesStub` with navigation testing -- [chat-interface.test.tsx](__tests__/components/chat/chat-interface.test.tsx) - `MemoryRouter` usage +- [settings.test.tsx](routes/settings.test.tsx) - `createRoutesStub` with nested routes and loaders +- [home-screen.test.tsx](routes/home-screen.test.tsx) - `createRoutesStub` with navigation testing +- [chat-interface.test.tsx](components/chat/chat-interface.test.tsx) - `MemoryRouter` usage ### Official Documentation diff --git a/frontend/src/hooks/mutation/use-add-git-providers.ts b/frontend/src/hooks/mutation/use-add-git-providers.ts index a6b7d85f8df5..782020f7d896 100644 --- a/frontend/src/hooks/mutation/use-add-git-providers.ts +++ b/frontend/src/hooks/mutation/use-add-git-providers.ts @@ -28,7 +28,7 @@ export const useAddGitProviders = () => { } await queryClient.invalidateQueries({ - queryKey: ["settings", organizationId], + queryKey: ["settings", "personal", organizationId], }); }, meta: { diff --git a/frontend/src/routes/mcp-settings.tsx b/frontend/src/routes/mcp-settings.tsx index 80cd96e0ea09..ab6dfe9d6a65 100644 --- a/frontend/src/routes/mcp-settings.tsx +++ b/frontend/src/routes/mcp-settings.tsx @@ -157,7 +157,7 @@ function MCPSettingsScreen() { } return ( -
+
diff --git a/openhands/agenthub/README.md b/openhands/agenthub/README.md index 4cb588bfec2a..1c4d669e80a7 100644 --- a/openhands/agenthub/README.md +++ b/openhands/agenthub/README.md @@ -40,7 +40,8 @@ The State object stores information about: * Extra data: * additional task-specific data -The agent can add and modify subtasks through the `AddTaskAction` and `ModifyTaskAction` +The agent can manage a structured task list through [`TaskTrackingAction`](../events/action/agent.py), +which powers the `task_tracker` tool used by agents such as `CodeActAgent`. ## Actions @@ -51,8 +52,7 @@ Here is a list of available Actions, which can be returned by `agent.step()`: - [`FileReadAction`](../events/action/files.py) - Reads the content of a file - [`FileWriteAction`](../events/action/files.py) - Writes new content to a file - [`BrowseURLAction`](../events/action/browse.py) - Gets the content of a URL -- [`AddTaskAction`](../events/action/tasks.py) - Adds a subtask to the plan -- [`ModifyTaskAction`](../events/action/tasks.py) - Changes the state of a subtask. +- [`TaskTrackingAction`](../events/action/agent.py) - Views or updates the task list used for task management. - [`AgentFinishAction`](../events/action/agent.py) - Stops the control loop, allowing the user/delegator agent to enter a new task - [`AgentRejectAction`](../events/action/agent.py) - Stops the control loop, allowing the user/delegator agent to enter a new task - [`AgentFinishAction`](../events/action/agent.py) - Stops the control loop, allowing the user to enter a new task @@ -74,6 +74,7 @@ Here is a list of available Observations: - [`BrowserOutputObservation`](../events/observation/browse.py) - [`FileReadObservation`](../events/observation/files.py) - [`FileWriteObservation`](../events/observation/files.py) +- [`TaskTrackingObservation`](../events/observation/task_tracking.py) - [`ErrorObservation`](../events/observation/error.py) - [`SuccessObservation`](../events/observation/success.py) diff --git a/openhands/app_server/README.md b/openhands/app_server/README.md index eb3ef387eb39..968a2d8030ab 100644 --- a/openhands/app_server/README.md +++ b/openhands/app_server/README.md @@ -10,10 +10,18 @@ As of 2025-09-29, much of the code in the OpenHands repository can be regarded a The app server is organized into several key modules: -- **conversation/**: Manages sandboxed conversations and their lifecycle +- **app_conversation/**: Manages sandboxed conversations and their lifecycle +- **app_lifespan/**: Application startup and shutdown lifecycle management +- **config_api/**: Configuration API endpoints - **event/**: Handles event storage, retrieval, and streaming - **event_callback/**: Manages webhooks and event callbacks +- **git/**: Git integration endpoints +- **pending_messages/**: Server-side message queuing - **sandbox/**: Manages sandbox environments for agent execution -- **user/**: User management and authentication +- **secrets/**: Secrets management endpoints - **services/**: Core services like JWT authentication +- **settings/**: User and application settings endpoints +- **status/**: Server status and system stats +- **user/**: User management and authentication - **utils/**: Utility functions for common operations +- **web_client/**: Web client configuration and routing diff --git a/openhands/app_server/app_conversation/live_status_app_conversation_service.py b/openhands/app_server/app_conversation/live_status_app_conversation_service.py index bde771207ce8..bce1a0eab637 100644 --- a/openhands/app_server/app_conversation/live_status_app_conversation_service.py +++ b/openhands/app_server/app_conversation/live_status_app_conversation_service.py @@ -52,7 +52,10 @@ from openhands.app_server.app_conversation.sql_app_conversation_info_service import ( SQLAppConversationInfoService, ) -from openhands.app_server.config import get_event_callback_service +from openhands.app_server.config import ( + get_event_callback_service, + resolve_provider_llm_base_url, +) from openhands.app_server.errors import SandboxError from openhands.app_server.event.event_service import EventService from openhands.app_server.event_callback.event_callback_models import EventCallback @@ -890,24 +893,12 @@ def _configure_llm(self, user: UserInfo, llm_model: str | None) -> LLM: or user.agent_settings.llm.model or LLM.model_fields['model'].default ) - base_url = user.agent_settings.llm.base_url - if model and ( - model.startswith('openhands/') or model.startswith('litellm_proxy/') - ): - # The SDK auto-fills base_url with the default public proxy for - # openhands/ models. We need to distinguish "user explicitly set a - # custom URL" from "SDK auto-filled the default". - # - # Priority: user-explicit URL > deployment provider URL > SDK default - _SDK_DEFAULT_PROXY = 'https://llm-proxy.app.all-hands.dev/' - user_set_custom = base_url and base_url.rstrip( - '/' - ) != _SDK_DEFAULT_PROXY.rstrip('/') - if user_set_custom: - pass # keep user's explicit base_url - elif self.openhands_provider_base_url: - base_url = self.openhands_provider_base_url - # else: keep the SDK default + + base_url = resolve_provider_llm_base_url( + model, + user.agent_settings.llm.base_url, + provider_base_url=self.openhands_provider_base_url, + ) return LLM( model=model, diff --git a/openhands/app_server/config.py b/openhands/app_server/config.py index d081dce40a15..cb661582421f 100644 --- a/openhands/app_server/config.py +++ b/openhands/app_server/config.py @@ -112,6 +112,50 @@ def get_openhands_provider_base_url() -> str | None: return os.getenv('OPENHANDS_PROVIDER_BASE_URL') or os.getenv('LLM_BASE_URL') or None +# The SDK auto-fills this URL as the default for openhands/ and litellm_proxy/ +# models. Deployments (e.g. staging) may use a different LLM proxy, configured +# via OPENHANDS_PROVIDER_BASE_URL. +_SDK_DEFAULT_PROXY = 'https://llm-proxy.app.all-hands.dev/' + + +def resolve_provider_llm_base_url( + model: str | None, + base_url: str | None, + provider_base_url: str | None = None, +) -> str | None: + """Apply deployment-specific LLM proxy override when needed. + + When the model uses ``openhands/`` or ``litellm_proxy/`` prefix and the + stored ``base_url`` is the SDK default, replace it with the deployment's + provider URL. + + Priority: user-explicit URL > deployment provider URL > SDK default. + + Args: + model: LLM model name (e.g. ``litellm_proxy/gpt-4``). + base_url: The base URL from user/org settings. + provider_base_url: Deployment provider URL. Falls back to + ``get_openhands_provider_base_url()`` when *None*. + """ + if not model or not ( + model.startswith('openhands/') or model.startswith('litellm_proxy/') + ): + return base_url + + user_set_custom = base_url and base_url.rstrip('/') != _SDK_DEFAULT_PROXY.rstrip( + '/' + ) + if user_set_custom: + return base_url + + if provider_base_url is None: + provider_base_url = get_openhands_provider_base_url() + if provider_base_url: + return provider_base_url + + return base_url + + def _get_default_lifespan(): # Check legacy parameters for saas mode. If we are in SAAS mode do not apply # OpenHands alembic migrations diff --git a/poetry.lock b/poetry.lock index 5ddd90e13fa3..6d37c2f4acdc 100644 --- a/poetry.lock +++ b/poetry.lock @@ -12081,14 +12081,14 @@ dev = ["backports.zoneinfo ; python_version < \"3.9\"", "black", "build", "freez [[package]] name = "python-multipart" -version = "0.0.22" +version = "0.0.26" description = "A streaming multipart parser for Python" optional = false python-versions = ">=3.10" groups = ["main"] files = [ - {file = "python_multipart-0.0.22-py3-none-any.whl", hash = "sha256:2b2cd894c83d21bf49d702499531c7bafd057d730c201782048f7945d82de155"}, - {file = "python_multipart-0.0.22.tar.gz", hash = "sha256:7340bef99a7e0032613f56dc36027b959fd3b30a787ed62d310e951f7c3a3a58"}, + {file = "python_multipart-0.0.26-py3-none-any.whl", hash = "sha256:c0b169f8c4484c13b0dcf2ef0ec3a4adb255c4b7d18d8e420477d2b1dd03f185"}, + {file = "python_multipart-0.0.26.tar.gz", hash = "sha256:08fadc45918cd615e26846437f50c5d6d23304da32c341f289a617127b081f17"}, ] [[package]] @@ -15029,4 +15029,4 @@ third-party-runtimes = ["daytona", "e2b-code-interpreter", "modal", "runloop-api [metadata] lock-version = "2.1" python-versions = "^3.12,<3.14" -content-hash = "c9e405e6cd0edaa81b220b657624c404c34822c6c98bfbbe3228c1c4b7715256" +content-hash = "e4258278f931521439b19eabf72bdecf91ef5f62cfd74b6c57d7461aec59e99c" diff --git a/pyproject.toml b/pyproject.toml index 8969c2032314..c8448ed46e25 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -81,7 +81,7 @@ dependencies = [ "python-dotenv", "python-frontmatter>=1.1", "python-json-logger>=3.2.1", - "python-multipart>=0.0.22", + "python-multipart>=0.0.26", "python-pptx", "python-socketio==5.14", "pythonnet; sys_platform=='win32'", @@ -186,7 +186,7 @@ html2text = "*" deprecated = "*" pexpect = "*" jinja2 = "^3.1.6" -python-multipart = ">=0.0.22" +python-multipart = ">=0.0.26" tenacity = ">=8.5,<10.0" zope-interface = "7.2" pathspec = ">=0.12.1,<1.1.0" diff --git a/tests/unit/app_server/test_resolve_provider_llm_base_url.py b/tests/unit/app_server/test_resolve_provider_llm_base_url.py new file mode 100644 index 000000000000..f9a4b0ae1231 --- /dev/null +++ b/tests/unit/app_server/test_resolve_provider_llm_base_url.py @@ -0,0 +1,193 @@ +"""Tests for resolve_provider_llm_base_url in openhands.app_server.config.""" + +from openhands.app_server.config import ( + _SDK_DEFAULT_PROXY, + resolve_provider_llm_base_url, +) + +SDK_DEFAULT = _SDK_DEFAULT_PROXY # 'https://llm-proxy.app.all-hands.dev/' +STAGING_URL = 'https://llm-proxy.staging.all-hands.dev/' +CUSTOM_URL = 'https://my-own-proxy.example.com/v1' + + +class TestOpenHandsPrefixWithProviderUrl: + """openhands/ prefix + SDK default URL + provider URL set → returns provider URL.""" + + def test_openhands_prefix_replaces_sdk_default(self): + result = resolve_provider_llm_base_url( + model='openhands/gpt-4', + base_url=SDK_DEFAULT, + provider_base_url=STAGING_URL, + ) + assert result == STAGING_URL + + def test_openhands_prefix_sdk_default_no_trailing_slash(self): + result = resolve_provider_llm_base_url( + model='openhands/gpt-4', + base_url=SDK_DEFAULT.rstrip('/'), + provider_base_url=STAGING_URL, + ) + assert result == STAGING_URL + + +class TestOpenHandsPrefixWithCustomUrl: + """openhands/ prefix + custom URL (not SDK default) → returns custom URL.""" + + def test_custom_url_preserved(self): + result = resolve_provider_llm_base_url( + model='openhands/gpt-4', + base_url=CUSTOM_URL, + provider_base_url=STAGING_URL, + ) + assert result == CUSTOM_URL + + def test_custom_url_preserved_no_provider(self): + result = resolve_provider_llm_base_url( + model='openhands/gpt-4', + base_url=CUSTOM_URL, + provider_base_url=None, + ) + assert result == CUSTOM_URL + + +class TestOpenHandsPrefixNoProviderUrl: + """openhands/ prefix + SDK default URL + no provider URL → returns SDK default.""" + + def test_sdk_default_returned_when_no_provider(self): + result = resolve_provider_llm_base_url( + model='openhands/gpt-4', + base_url=SDK_DEFAULT, + provider_base_url='', # empty string = falsy + ) + assert result == SDK_DEFAULT + + def test_sdk_default_returned_when_provider_none_and_env_unset(self, monkeypatch): + monkeypatch.delenv('OPENHANDS_PROVIDER_BASE_URL', raising=False) + monkeypatch.delenv('LLM_BASE_URL', raising=False) + result = resolve_provider_llm_base_url( + model='openhands/gpt-4', + base_url=SDK_DEFAULT, + # provider_base_url defaults to None → falls back to env lookup + ) + assert result == SDK_DEFAULT + + +class TestNonMatchingModelPrefix: + """Model without openhands/ or litellm_proxy/ prefix → returns base_url unchanged.""" + + def test_plain_model_returns_base_url(self): + result = resolve_provider_llm_base_url( + model='gpt-4', + base_url='https://api.openai.com/v1', + provider_base_url=STAGING_URL, + ) + assert result == 'https://api.openai.com/v1' + + def test_anthropic_model_returns_base_url(self): + result = resolve_provider_llm_base_url( + model='anthropic/claude-3-opus', + base_url='https://api.anthropic.com', + provider_base_url=STAGING_URL, + ) + assert result == 'https://api.anthropic.com' + + def test_none_base_url_passthrough(self): + result = resolve_provider_llm_base_url( + model='gpt-4', + base_url=None, + provider_base_url=STAGING_URL, + ) + assert result is None + + +class TestLitellmProxyPrefix: + """litellm_proxy/ prefix behaves identically to openhands/ prefix.""" + + def test_replaces_sdk_default(self): + result = resolve_provider_llm_base_url( + model='litellm_proxy/gpt-4', + base_url=SDK_DEFAULT, + provider_base_url=STAGING_URL, + ) + assert result == STAGING_URL + + def test_custom_url_preserved(self): + result = resolve_provider_llm_base_url( + model='litellm_proxy/gpt-4', + base_url=CUSTOM_URL, + provider_base_url=STAGING_URL, + ) + assert result == CUSTOM_URL + + def test_sdk_default_returned_when_no_provider(self): + result = resolve_provider_llm_base_url( + model='litellm_proxy/gpt-4', + base_url=SDK_DEFAULT, + provider_base_url='', + ) + assert result == SDK_DEFAULT + + +class TestEdgeCases: + """Edge cases: None values, empty strings, trailing slash normalization.""" + + def test_none_model_returns_base_url(self): + result = resolve_provider_llm_base_url( + model=None, + base_url=SDK_DEFAULT, + provider_base_url=STAGING_URL, + ) + assert result == SDK_DEFAULT + + def test_empty_model_returns_base_url(self): + result = resolve_provider_llm_base_url( + model='', + base_url=SDK_DEFAULT, + provider_base_url=STAGING_URL, + ) + assert result == SDK_DEFAULT + + def test_none_base_url_with_openhands_model_and_provider(self): + result = resolve_provider_llm_base_url( + model='openhands/gpt-4', + base_url=None, + provider_base_url=STAGING_URL, + ) + assert result == STAGING_URL + + def test_none_base_url_with_openhands_model_no_provider(self, monkeypatch): + monkeypatch.delenv('OPENHANDS_PROVIDER_BASE_URL', raising=False) + monkeypatch.delenv('LLM_BASE_URL', raising=False) + result = resolve_provider_llm_base_url( + model='openhands/gpt-4', + base_url=None, + ) + assert result is None + + def test_trailing_slash_normalization(self): + """SDK default with and without trailing slash should both be detected.""" + for url in [SDK_DEFAULT, SDK_DEFAULT.rstrip('/')]: + result = resolve_provider_llm_base_url( + model='openhands/gpt-4', + base_url=url, + provider_base_url=STAGING_URL, + ) + assert result == STAGING_URL, f'Failed for base_url={url!r}' + + def test_env_fallback_when_provider_base_url_is_none(self, monkeypatch): + monkeypatch.setenv('OPENHANDS_PROVIDER_BASE_URL', STAGING_URL) + result = resolve_provider_llm_base_url( + model='openhands/gpt-4', + base_url=SDK_DEFAULT, + # provider_base_url defaults to None → env lookup + ) + assert result == STAGING_URL + + def test_llm_base_url_env_fallback(self, monkeypatch): + monkeypatch.delenv('OPENHANDS_PROVIDER_BASE_URL', raising=False) + monkeypatch.setenv('LLM_BASE_URL', STAGING_URL) + result = resolve_provider_llm_base_url( + model='openhands/gpt-4', + base_url=SDK_DEFAULT, + ) + assert result == STAGING_URL diff --git a/uv.lock b/uv.lock index 7d9ddf67536e..d2928af475de 100644 --- a/uv.lock +++ b/uv.lock @@ -3850,7 +3850,7 @@ requires-dist = [ { name = "python-dotenv" }, { name = "python-frontmatter", specifier = ">=1.1" }, { name = "python-json-logger", specifier = ">=3.2.1" }, - { name = "python-multipart", specifier = ">=0.0.22" }, + { name = "python-multipart", specifier = ">=0.0.26" }, { name = "python-pptx" }, { name = "python-socketio", specifier = "==5.14" }, { name = "pythonnet", marker = "sys_platform == 'win32'" }, @@ -7587,11 +7587,11 @@ wheels = [ [[package]] name = "python-multipart" -version = "0.0.22" +version = "0.0.26" source = { registry = "https://pypi.org/simple" } -sdist = { url = "https://files.pythonhosted.org/packages/94/01/979e98d542a70714b0cb2b6728ed0b7c46792b695e3eaec3e20711271ca3/python_multipart-0.0.22.tar.gz", hash = "sha256:7340bef99a7e0032613f56dc36027b959fd3b30a787ed62d310e951f7c3a3a58", size = 37612, upload-time = "2026-01-25T10:15:56.219Z" } +sdist = { url = "https://files.pythonhosted.org/packages/88/71/b145a380824a960ebd60e1014256dbb7d2253f2316ff2d73dfd8928ec2c3/python_multipart-0.0.26.tar.gz", hash = "sha256:08fadc45918cd615e26846437f50c5d6d23304da32c341f289a617127b081f17", size = 43501, upload-time = "2026-04-10T14:09:59.473Z" } wheels = [ - { url = "https://files.pythonhosted.org/packages/1b/d0/397f9626e711ff749a95d96b7af99b9c566a9bb5129b8e4c10fc4d100304/python_multipart-0.0.22-py3-none-any.whl", hash = "sha256:2b2cd894c83d21bf49d702499531c7bafd057d730c201782048f7945d82de155", size = 24579, upload-time = "2026-01-25T10:15:54.811Z" }, + { url = "https://files.pythonhosted.org/packages/9a/22/f1925cdda983ab66fc8ec6ec8014b959262747e58bdca26a4e3d1da29d56/python_multipart-0.0.26-py3-none-any.whl", hash = "sha256:c0b169f8c4484c13b0dcf2ef0ec3a4adb255c4b7d18d8e420477d2b1dd03f185", size = 28847, upload-time = "2026-04-10T14:09:58.131Z" }, ] [[package]]