mirror of
https://github.com/langbot-app/LangBot.git
synced 2026-10-23 08:55:44 +08:00
fix: 修复独立环境登录 404 与「添加第一个模型」500(4.11 回归) (#2755)
* 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[<runner id>].
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.
This commit is contained in:
@@ -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))
|
||||
|
||||
|
||||
@@ -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[<runner id>], 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']
|
||||
|
||||
|
||||
@@ -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()
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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()
|
||||
@@ -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'
|
||||
@@ -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='[email protected]', 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
|
||||
|
||||
@@ -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
|
||||
Reference in New Issue
Block a user