From 7fa9c0e426b5c7a767be866d666db9f000eaa115 Mon Sep 17 00:00:00 2001 From: RockChinQ Date: Thu, 22 Oct 2026 23:26:30 +0800 Subject: [PATCH] =?UTF-8?q?fix:=20=E4=BF=AE=E5=A4=8D=E7=8B=AC=E7=AB=8B?= =?UTF-8?q?=E7=8E=AF=E5=A2=83=E7=99=BB=E5=BD=95=20404=20=E4=B8=8E=E3=80=8C?= =?UTF-8?q?=E6=B7=BB=E5=8A=A0=E7=AC=AC=E4=B8=80=E4=B8=AA=E6=A8=A1=E5=9E=8B?= =?UTF-8?q?=E3=80=8D500=EF=BC=884.11=20=E5=9B=9E=E5=BD=92=EF=BC=89=20(#275?= =?UTF-8?q?5)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(space): keep dedicated-environment Space login working A dedicated environment runs the OSS runtime with system.edition=cloud. The process-level 'LangBot Models' compatibility provider is part of the OSS single-Workspace contract, but write_space_model_providers() skipped creating it for every non-community edition. The Space OAuth callback then unconditionally rewarded the provider row by api key, so provider_service raised WorkspaceNotFoundError('Provider not found') and the whole callback returned 404 -- after the Account and its Workspace had already been created. Every retry failed the same way, making dedicated-environment login impossible on 4.11.x. Gate the compatibility provider on the deployment shape instead of the edition (genuine multi-tenant Cloud runs in CLOUD_RUNTIME persistence and never reaches this path), and treat a missing provider row as a model-routing warning rather than an authentication failure so a data anomaly can never block login. * fix(agent): bind the first model to the 4.11 runner config create_llm_model() still wrote the auto-selected model into the pre-4.11 ai['local-agent'] container. On a 4.11 Workspace that key does not exist, so the assignment raised KeyError and POST /provider/models/llm returned 500 the first time a user added a model -- and the default pipeline kept primary='', so every Agent run failed with 'No authorized model for local-agent'. Delegate to RunnerDefaultConfigService, which resolves the selected runner and writes the schema-defined model selector into ai.runner_config[]. Also default the NOT NULL description column on pipeline and bot creation: omitting an optional description returned a 500 from the constraint instead of creating the resource. * fix(agent): keep seeding the model for pipelines that never migrated set_pipeline_llm_model_if_empty() returns False when a pipeline has no 4.11 runner selected -- that is exactly the un-migrated shape, which still carries ai.local-agent. Seed that container, but only when it exists, so the legacy path keeps its old behaviour without the KeyError. --- src/langbot/pkg/api/http/service/bot.py | 3 + src/langbot/pkg/api/http/service/model.py | 40 +++++-- src/langbot/pkg/api/http/service/pipeline.py | 3 + src/langbot/pkg/api/http/service/user.py | 22 +++- src/langbot/pkg/persistence/mgr.py | 9 +- .../test_model_default_pipeline_binding.py | 106 ++++++++++++++++++ .../test_pipeline_create_description.py | 56 +++++++++ .../api/service/test_user_service.py | 26 +++++ .../test_dedicated_space_provider.py | 75 +++++++++++++ 9 files changed, 326 insertions(+), 14 deletions(-) create mode 100644 tests/unit_tests/api/service/test_model_default_pipeline_binding.py create mode 100644 tests/unit_tests/api/service/test_pipeline_create_description.py create mode 100644 tests/unit_tests/persistence/test_dedicated_space_provider.py diff --git a/src/langbot/pkg/api/http/service/bot.py b/src/langbot/pkg/api/http/service/bot.py index b9de1c1ba..506fb4de1 100644 --- a/src/langbot/pkg/api/http/service/bot.py +++ b/src/langbot/pkg/api/http/service/bot.py @@ -779,6 +779,9 @@ class BotService: else: bot_data['event_bindings'] = await self._normalize_event_bindings(context, bot_data['event_bindings']) bot_data.setdefault('plugin_processors', []) + # The column is NOT NULL; a caller that omits the optional description + # must not get a 500 from the database constraint. + bot_data.setdefault('description', '') await self.ap.persistence_mgr.execute_async(sqlalchemy.insert(persistence_bot.Bot).values(bot_data)) diff --git a/src/langbot/pkg/api/http/service/model.py b/src/langbot/pkg/api/http/service/model.py index bfd20bfaa..ecd75f83b 100644 --- a/src/langbot/pkg/api/http/service/model.py +++ b/src/langbot/pkg/api/http/service/model.py @@ -402,15 +402,37 @@ class LLMModelsService: ) pipeline = result.first() if pipeline is not None: - model_config = pipeline.config.get('ai', {}).get('local-agent', {}).get('model', {}) - if not model_config.get('primary', ''): - pipeline_config = pipeline.config - pipeline_config['ai']['local-agent']['model'] = { - 'primary': model_data['uuid'], - 'fallbacks': [], - } - pipeline_data = {'config': pipeline_config} - await self.ap.pipeline_service.update_pipeline(context, pipeline.uuid, pipeline_data) + # 4.11 pipelines keep the model selector inside + # ai.runner_config[], not in the pre-4.11 + # ai.local-agent container, so the schema-aware default-config + # service owns this. Touching ai['local-agent'] directly raised + # KeyError and turned "add the first model" into a 500 on every + # freshly created Workspace. + updated = await self.ap.runner_default_config_service.set_pipeline_llm_model_if_empty( + context, + pipeline, + model_data['uuid'], + ) + if not updated: + # Pipelines that have not gone through the 4.11 migration still + # carry the legacy container; keep seeding it, but only when it + # actually exists. + legacy_config = (pipeline.config or {}).get('ai', {}).get('local-agent') + if isinstance(legacy_config, dict): + legacy_model = legacy_config.get('model') + if not isinstance(legacy_model, dict): + legacy_model = {} + if not legacy_model.get('primary', ''): + legacy_config['model'] = { + **legacy_model, + 'primary': model_data['uuid'], + 'fallbacks': legacy_model.get('fallbacks') or [], + } + await self.ap.pipeline_service.update_pipeline( + context, + pipeline.uuid, + {'config': pipeline.config}, + ) return model_data['uuid'] diff --git a/src/langbot/pkg/api/http/service/pipeline.py b/src/langbot/pkg/api/http/service/pipeline.py index 6df59edbf..934cf97f9 100644 --- a/src/langbot/pkg/api/http/service/pipeline.py +++ b/src/langbot/pkg/api/http/service/pipeline.py @@ -183,6 +183,9 @@ class PipelineService: raise ValueError(f'Maximum number of pipelines ({max_pipelines}) reached') pipeline_data = pipeline_data.copy() + # The column is NOT NULL; a caller that omits the optional description + # must not get a 500 from the database constraint. + pipeline_data.setdefault('description', '') pipeline_data['uuid'] = str(uuid.uuid4()) pipeline_data['workspace_uuid'] = workspace_uuid pipeline_data['for_version'] = self.ap.ver_mgr.get_current_version() diff --git a/src/langbot/pkg/api/http/service/user.py b/src/langbot/pkg/api/http/service/user.py index 60926178f..8b63af7b1 100644 --- a/src/langbot/pkg/api/http/service/user.py +++ b/src/langbot/pkg/api/http/service/user.py @@ -33,6 +33,7 @@ from ....entity.persistence.workspace import MembershipRole, MembershipStatus, W from ....utils import constants from ....entity.errors import account as account_errors from ....workspace.collaboration import normalize_email +from ....workspace.errors import WorkspaceNotFoundError from ....utils import bounded_executor if typing.TYPE_CHECKING: @@ -255,10 +256,23 @@ class UserService: access = accesses[0] if access.membership.role != MembershipRole.OWNER.value: return - await self.ap.provider_service.update_space_model_provider_api_keys( - access.workspace.uuid, - api_key, - ) + # A missing compatibility provider row is a model-routing problem, not an + # authentication one. Letting provider_service raise here aborted the whole + # Space OAuth callback after the Account had already been created, so the + # user logged in "successfully" but received 404 on every retry. + try: + await self.ap.provider_service.update_space_model_provider_api_keys( + access.workspace.uuid, + api_key, + ) + except WorkspaceNotFoundError: + logger = getattr(self.ap, 'logger', None) + if logger is not None: + logger.warning( + 'Space model provider row is missing for workspace %s; ' + 'skipping credential refresh so login can proceed', + access.workspace.uuid, + ) async def is_initialized(self) -> bool: account = await self._identity_scalar( diff --git a/src/langbot/pkg/persistence/mgr.py b/src/langbot/pkg/persistence/mgr.py index 2bf8e3209..9350f5f62 100644 --- a/src/langbot/pkg/persistence/mgr.py +++ b/src/langbot/pkg/persistence/mgr.py @@ -1608,7 +1608,14 @@ class PersistenceManager: return policies async def write_space_model_providers(self): - if constants.edition != 'community': + # Only a single-Workspace deployment may own the process-level + # compatibility provider. Genuine multi-tenant Cloud runs in CLOUD_RUNTIME + # persistence mode and never reaches this method, but a dedicated + # environment is `edition=cloud` on top of OSS persistence, so gating on + # the edition alone left it without the provider row -- and the Space OAuth + # callback then failed with "Provider not found" on every login. + deployment = getattr(self.ap, 'deployment', None) + if deployment is not None and getattr(deployment, 'multi_workspace_enabled', False): # SaaS Workspace/provider linkage is explicit control-plane state; # a process-level compatibility provider must never be projected # into an arbitrary cloud Workspace. diff --git a/tests/unit_tests/api/service/test_model_default_pipeline_binding.py b/tests/unit_tests/api/service/test_model_default_pipeline_binding.py new file mode 100644 index 000000000..46ffb6f67 --- /dev/null +++ b/tests/unit_tests/api/service/test_model_default_pipeline_binding.py @@ -0,0 +1,106 @@ +import json +import sys +import types +from unittest.mock import AsyncMock, Mock + +import pytest + +from langbot.pkg.api.http.service.model import LLMModelsService + + +def _service(default_pipeline, runner_service): + app = Mock() + app.persistence_mgr = Mock() + app.persistence_mgr.execute_async = AsyncMock() + result = Mock() + result.first.return_value = default_pipeline + app.persistence_mgr.execute_async.return_value = result + app.runner_default_config_service = runner_service + app.model_mgr = Mock() + app.model_mgr.load_llm_model_with_provider = AsyncMock(return_value=Mock()) + app.model_mgr.cache_llm_model = AsyncMock() + app.provider_service = Mock() + app.pipeline_service = Mock() + app.pipeline_service.update_pipeline = AsyncMock() + return LLMModelsService(app) + + +@pytest.mark.asyncio +async def test_model_creation_delegates_default_pipeline_binding_to_runner_schema(monkeypatch): + """The pre-4.11 ai['local-agent'] container must never be touched directly. + + Assigning into ai['local-agent'] raised KeyError on 4.11 Workspaces, which + turned "add the first model" into a 500 -- and the model never reached the + default pipeline, so every agent run failed with "No authorized model". + """ + pipeline = types.SimpleNamespace( + uuid='pipeline-a', config={'ai': {'runner': {'id': 'plugin:langbot-team/LocalAgent/default'}}} + ) + runner_service = Mock() + runner_service.set_pipeline_llm_model_if_empty = AsyncMock(return_value=True) + + service = _service(pipeline, runner_service) + + monkeypatch.setattr(sys.modules['langbot.pkg.api.http.service.model'], '_require_workspace_provider', AsyncMock()) + monkeypatch.setattr( + sys.modules['langbot.pkg.api.http.service.model'], '_assert_cloud_managed_provider_mutable', AsyncMock() + ) + monkeypatch.setattr(sys.modules['langbot.pkg.api.http.service.model'], '_validate_provider_supports', AsyncMock()) + monkeypatch.setattr(sys.modules['langbot.pkg.api.http.service.model'], '_require_runtime_provider', AsyncMock()) + monkeypatch.setattr( + sys.modules['langbot.pkg.api.http.service.model'], '_normalize_llm_reasoning', lambda data: None + ) + monkeypatch.setattr( + sys.modules['langbot.pkg.api.http.service.model'], '_validate_llm_reasoning_capability', lambda *a, **k: None + ) + + uuid_value = await service.create_llm_model( + 'workspace-a', + { + 'name': 'deepseek-chat', + 'provider_uuid': 'provider-a', + 'abilities': ['func_call'], + }, + ) + + assert uuid_value + runner_service.set_pipeline_llm_model_if_empty.assert_awaited_once() + args = runner_service.set_pipeline_llm_model_if_empty.await_args.args + assert args[0] == 'workspace-a' + assert args[1] is pipeline + assert args[2] == uuid_value + # No legacy write path should remain. + assert 'local-agent' not in json.dumps(pipeline.config) + + +@pytest.mark.asyncio +async def test_unmigrated_pipeline_still_gets_a_legacy_model(monkeypatch): + """A pipeline that has not run the 4.11 migration keeps its old shape.""" + pipeline = types.SimpleNamespace( + uuid='pipeline-b', + config={'ai': {'local-agent': {'model': {'primary': '', 'fallbacks': []}}}}, + ) + runner_service = Mock() + runner_service.set_pipeline_llm_model_if_empty = AsyncMock(return_value=False) + + service = _service(pipeline, runner_service) + monkeypatch.setattr(sys.modules['langbot.pkg.api.http.service.model'], '_require_workspace_provider', AsyncMock()) + monkeypatch.setattr( + sys.modules['langbot.pkg.api.http.service.model'], '_assert_cloud_managed_provider_mutable', AsyncMock() + ) + monkeypatch.setattr(sys.modules['langbot.pkg.api.http.service.model'], '_validate_provider_supports', AsyncMock()) + monkeypatch.setattr(sys.modules['langbot.pkg.api.http.service.model'], '_require_runtime_provider', AsyncMock()) + monkeypatch.setattr( + sys.modules['langbot.pkg.api.http.service.model'], '_normalize_llm_reasoning', lambda data: None + ) + monkeypatch.setattr( + sys.modules['langbot.pkg.api.http.service.model'], '_validate_llm_reasoning_capability', lambda *a, **k: None + ) + + uuid_value = await service.create_llm_model( + 'workspace-a', + {'name': 'deepseek-chat', 'provider_uuid': 'provider-a', 'abilities': ['func_call']}, + ) + + assert pipeline.config['ai']['local-agent']['model']['primary'] == uuid_value + service.ap.pipeline_service.update_pipeline.assert_awaited_once() diff --git a/tests/unit_tests/api/service/test_pipeline_create_description.py b/tests/unit_tests/api/service/test_pipeline_create_description.py new file mode 100644 index 000000000..7fec6bca1 --- /dev/null +++ b/tests/unit_tests/api/service/test_pipeline_create_description.py @@ -0,0 +1,56 @@ +import types + +import pytest + +from langbot.pkg.api.http.service.pipeline import PipelineService + + +async def _noop(*args, **kwargs): + return None + + +@pytest.mark.asyncio +async def test_create_pipeline_defaults_missing_description(monkeypatch): + """legacy_pipelines.description is NOT NULL; omitting it must not 500.""" + captured = {} + + app = types.SimpleNamespace() + app.persistence_mgr = types.SimpleNamespace() + app.pipeline_mgr = types.SimpleNamespace(load_pipeline=_noop) + app.ver_mgr = types.SimpleNamespace(get_current_version=lambda: 'v4.11.1') + + service = PipelineService.__new__(PipelineService) + service.ap = app + + async def fake_get_default_config(context): + runner_id = 'plugin:langbot-team/LocalAgent/default' + return { + 'ai': { + 'runner': {'id': runner_id}, + 'runner_config': {runner_id: {'model': {'primary': '', 'fallbacks': []}}}, + } + } + + async def fake_get_pipelines(context, *args, **kwargs): + return [] + + async def fake_get_pipeline(context, uuid_value, include_secret=True): + return {'uuid': uuid_value} + + async def fake_insert(statement): + captured['statement'] = statement + + async def fake_limits(*args, **kwargs): + return {'max_pipelines': -1} + + monkeypatch.setattr(service, 'get_default_pipeline_config', fake_get_default_config) + monkeypatch.setattr(service, 'get_pipelines', fake_get_pipelines) + monkeypatch.setattr(service, 'get_pipeline', fake_get_pipeline) + monkeypatch.setattr('langbot.pkg.api.http.service.workspace_limits.workspace_limits', fake_limits) + app.persistence_mgr.execute_async = fake_insert + + await service.create_pipeline('workspace-a', {'name': 'de-agent-test'}) + + params = captured['statement'].compile().params + assert params.get('description') == '' + assert params.get('workspace_uuid') == 'workspace-a' diff --git a/tests/unit_tests/api/service/test_user_service.py b/tests/unit_tests/api/service/test_user_service.py index 6d28212f4..adb0e572d 100644 --- a/tests/unit_tests/api/service/test_user_service.py +++ b/tests/unit_tests/api/service/test_user_service.py @@ -925,6 +925,32 @@ class TestUserServiceCreateOrUpdateSpaceUser: provider_service.update_space_model_provider_api_keys.assert_awaited_once_with('workspace-a', 'owner-api-key') + async def test_oss_space_provider_refresh_tolerates_missing_provider_row(self): + """A missing provider row must not abort the Space OAuth login.""" + from langbot.pkg.workspace.errors import WorkspaceNotFoundError + + owner_account = _create_mock_user(email='owner@example.com', space_account_uuid='space-owner') + access = SimpleNamespace( + workspace=SimpleNamespace(uuid='workspace-a'), + membership=SimpleNamespace(role='owner'), + ) + provider_service = SimpleNamespace( + update_space_model_provider_api_keys=AsyncMock(side_effect=WorkspaceNotFoundError('Provider not found')) + ) + logger = SimpleNamespace(warning=Mock()) + ap = SimpleNamespace( + workspace_service=SimpleNamespace(policy=SimpleNamespace(multi_workspace_enabled=False)), + workspace_collaboration_service=SimpleNamespace(list_account_workspaces=AsyncMock(return_value=[access])), + provider_service=provider_service, + logger=logger, + ) + + # Must not raise: authentication already succeeded by this point. + await UserService(ap)._update_space_provider_for_account(owner_account, 'owner-api-key') + + provider_service.update_space_model_provider_api_keys.assert_awaited_once_with('workspace-a', 'owner-api-key') + logger.warning.assert_called_once() + async def test_create_or_update_space_user_no_expiry(self): """Creates Space user without token expiry.""" # Setup diff --git a/tests/unit_tests/persistence/test_dedicated_space_provider.py b/tests/unit_tests/persistence/test_dedicated_space_provider.py new file mode 100644 index 000000000..c762c76a1 --- /dev/null +++ b/tests/unit_tests/persistence/test_dedicated_space_provider.py @@ -0,0 +1,75 @@ +"""Dedicated-environment (edition=cloud on OSS persistence) Space provider coverage. + +A dedicated environment runs the OSS runtime with ``system.edition = cloud``. The +process-level ``LangBot Models`` compatibility provider is part of the OSS +single-Workspace contract, so gating its creation on ``edition == 'community'`` +left dedicated Cores without the row and broke Space OAuth login with +"Provider not found". +""" + +from __future__ import annotations + +from importlib import import_module +from types import SimpleNamespace +from unittest.mock import AsyncMock, Mock + +import pytest + + +def get_persistence_module(): + return import_module('langbot.pkg.persistence.mgr') + + +def _manager(edition: str, *, multi_workspace_enabled: bool, workspace_uuids: list[str]): + persistence = get_persistence_module() + app = Mock() + app.persistence_mgr = None + app.instance_config = SimpleNamespace(data={'space': {'models_gateway_api_url': 'https://models.example.test/v1'}}) + app.deployment = SimpleNamespace(multi_workspace_enabled=multi_workspace_enabled) + app.logger = Mock() + + mgr = persistence.PersistenceManager(app) + mgr.execute_async = AsyncMock() + + workspace_result = Mock() + workspace_result.scalars.return_value.all.return_value = workspace_uuids + provider_result = Mock() + provider_result.first.return_value = None + # Third call is the INSERT of the compatibility provider. + mgr.execute_async.side_effect = [workspace_result, provider_result, Mock()] + return mgr + + +class TestWriteSpaceModelProviders: + @pytest.mark.asyncio + async def test_dedicated_cloud_edition_creates_compatibility_provider(self, monkeypatch): + """edition=cloud on a single Workspace must still own the provider row.""" + constants = import_module('langbot.pkg.utils.constants') + monkeypatch.setattr(constants, 'edition', 'cloud') + + mgr = _manager('cloud', multi_workspace_enabled=False, workspace_uuids=['workspace-a']) + await mgr.write_space_model_providers() + + # One select for the Workspace list, one for the existing provider, one insert. + assert mgr.execute_async.await_count == 3 + + @pytest.mark.asyncio + async def test_multi_workspace_cloud_never_projects_provider(self, monkeypatch): + """Real multi-tenant Cloud keeps the provider out of its Workspaces.""" + constants = import_module('langbot.pkg.utils.constants') + monkeypatch.setattr(constants, 'edition', 'cloud') + + mgr = _manager('cloud', multi_workspace_enabled=True, workspace_uuids=['workspace-a']) + await mgr.write_space_model_providers() + + mgr.execute_async.assert_not_awaited() + + @pytest.mark.asyncio + async def test_community_edition_still_creates_provider(self, monkeypatch): + constants = import_module('langbot.pkg.utils.constants') + monkeypatch.setattr(constants, 'edition', 'community') + + mgr = _manager('community', multi_workspace_enabled=False, workspace_uuids=['workspace-a']) + await mgr.write_space_model_providers() + + assert mgr.execute_async.await_count == 3