mirror of
https://github.com/langbot-app/LangBot.git
synced 2026-06-27 07:54:19 +00:00
feat(skill): unify skill activation as authorized tools
Expose skill tools (activate/register_skill/native exec) like native tools instead of gating them behind the skill_authoring capability: - toolmgr.get_all_tools drops include_skill_authoring; SkillToolLoader self-gates on sandbox + skill_mgr - preproc drops the include_skill_authoring branch; pipeline-bound skills and the skills resource gate on skill_mgr presence Persist activated skills into host.activated_skills conversation state so they survive across runs (host writes at activate; last-write-wins); drop the dead restore_activated_skills helper. Prefill ToolResource.parameters host-side (tool_mgr.get_tool_schema) so runners build LLM tools without per-tool get_tool_detail round-trips. Align agent-runner-pluginization design docs to the all-tool model.
This commit is contained in:
@@ -97,6 +97,9 @@ def app():
|
||||
mock_app.model_mgr = Mock()
|
||||
mock_app.rag_mgr = Mock()
|
||||
mock_app.rag_mgr.get_knowledge_base_by_uuid = AsyncMock(return_value=None)
|
||||
mock_app.skill_mgr = None
|
||||
mock_app.tool_mgr = Mock()
|
||||
mock_app.tool_mgr.get_tool_schema = AsyncMock(return_value=(None, None))
|
||||
return mock_app
|
||||
|
||||
|
||||
@@ -278,7 +281,16 @@ async def test_build_models_deduplicates_query_and_config_models(app):
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_build_tools_authorizes_query_declared_tools(app):
|
||||
"""Tools discovered by Pipeline preprocessing become run-scoped authorized resources."""
|
||||
"""Tools discovered by Pipeline preprocessing become run-scoped authorized
|
||||
resources, with full parameters schema prefilled by the host."""
|
||||
app.tool_mgr.get_tool_schema = AsyncMock(
|
||||
side_effect=lambda name: {
|
||||
'qa_plugin_echo': (
|
||||
'Echo test tool',
|
||||
{'type': 'object', 'properties': {'text': {'type': 'string'}}},
|
||||
),
|
||||
}.get(name, (None, None))
|
||||
)
|
||||
descriptor = make_descriptor(
|
||||
capabilities={'tool_calling': True},
|
||||
)
|
||||
@@ -296,14 +308,16 @@ async def test_build_tools_authorizes_query_declared_tools(app):
|
||||
{
|
||||
'tool_name': 'qa_plugin_echo',
|
||||
'tool_type': None,
|
||||
'description': None,
|
||||
'description': 'Echo test tool',
|
||||
'operations': ['detail', 'call'],
|
||||
'parameters': {'type': 'object', 'properties': {'text': {'type': 'string'}}},
|
||||
},
|
||||
{
|
||||
'tool_name': 'qa_mcp_echo',
|
||||
'tool_type': None,
|
||||
'description': None,
|
||||
'operations': ['detail', 'call'],
|
||||
'parameters': None,
|
||||
},
|
||||
]
|
||||
|
||||
|
||||
@@ -176,6 +176,63 @@ class TestSkillActivationHelper:
|
||||
assert register_activated_skill(ap, query, 'primary') is False
|
||||
|
||||
|
||||
class TestPersistActivatedSkill:
|
||||
"""Host-side persistence of activated skills into conversation state (S-01/S-02)."""
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_persist_writes_conversation_state(self):
|
||||
from unittest.mock import patch
|
||||
from langbot.pkg.provider.tools.loaders.skill import (
|
||||
persist_activated_skill,
|
||||
ACTIVATED_SKILLS_KEY,
|
||||
ACTIVATED_SKILL_NAMES_STATE_KEY,
|
||||
)
|
||||
|
||||
ap = _make_ap()
|
||||
ap.persistence_mgr.get_db_engine = Mock(return_value=Mock())
|
||||
|
||||
query = SimpleNamespace(variables={ACTIVATED_SKILLS_KEY: {'pdf': {'name': 'pdf'}}})
|
||||
query._agent_run_session = {
|
||||
'runner_id': 'plugin:langbot/local-agent/default',
|
||||
'state_context': {
|
||||
'scope_keys': {'conversation': 'conv-scope-key'},
|
||||
'binding_identity': 'binding-1',
|
||||
'conversation_id': 'c1',
|
||||
},
|
||||
}
|
||||
|
||||
store = SimpleNamespace(state_set=AsyncMock(return_value=(True, None)))
|
||||
with patch(
|
||||
'langbot.pkg.agent.runner.persistent_state_store.get_persistent_state_store',
|
||||
return_value=store,
|
||||
):
|
||||
await persist_activated_skill(ap, query, 'pdf')
|
||||
|
||||
store.state_set.assert_awaited_once()
|
||||
kwargs = store.state_set.await_args.kwargs
|
||||
assert kwargs['scope_key'] == 'conv-scope-key'
|
||||
assert kwargs['state_key'] == ACTIVATED_SKILL_NAMES_STATE_KEY
|
||||
assert kwargs['value'] == ['pdf']
|
||||
assert kwargs['scope'] == 'conversation'
|
||||
assert kwargs['runner_id'] == 'plugin:langbot/local-agent/default'
|
||||
assert kwargs['binding_identity'] == 'binding-1'
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_persist_noop_without_run_session(self):
|
||||
from unittest.mock import patch
|
||||
from langbot.pkg.provider.tools.loaders.skill import persist_activated_skill
|
||||
|
||||
ap = _make_ap()
|
||||
query = SimpleNamespace(variables={'_activated_skills': {'pdf': {'name': 'pdf'}}})
|
||||
|
||||
with patch(
|
||||
'langbot.pkg.agent.runner.persistent_state_store.get_persistent_state_store',
|
||||
) as mock_factory:
|
||||
await persist_activated_skill(ap, query, 'pdf')
|
||||
|
||||
mock_factory.assert_not_called()
|
||||
|
||||
|
||||
class TestSkillPathHelpers:
|
||||
def test_get_visible_skills_filters_by_bound_names(self):
|
||||
from langbot.pkg.provider.tools.loaders.skill import PIPELINE_BOUND_SKILLS_KEY, get_visible_skills
|
||||
@@ -193,12 +250,13 @@ class TestSkillPathHelpers:
|
||||
|
||||
assert list(result.keys()) == ['visible']
|
||||
|
||||
def test_restore_activated_skills_uses_caller_provided_names_and_visibility(self):
|
||||
def test_restore_activated_skills_from_state_filters_by_visibility(self):
|
||||
from langbot.pkg.provider.tools.loaders.skill import (
|
||||
ACTIVATED_SKILLS_KEY,
|
||||
ACTIVATED_SKILL_NAMES_STATE_KEY,
|
||||
PIPELINE_BOUND_SKILLS_KEY,
|
||||
get_activated_skill_names,
|
||||
restore_activated_skills,
|
||||
restore_activated_skills_from_state,
|
||||
)
|
||||
|
||||
ap = _make_ap()
|
||||
@@ -209,8 +267,9 @@ class TestSkillPathHelpers:
|
||||
}
|
||||
)
|
||||
query = SimpleNamespace(variables={PIPELINE_BOUND_SKILLS_KEY: ['visible']})
|
||||
state = {'conversation': {ACTIVATED_SKILL_NAMES_STATE_KEY: ['visible', 'hidden', 'visible', '']}}
|
||||
|
||||
restored = restore_activated_skills(ap, query, ['visible', 'hidden', 'visible', ''])
|
||||
restored = restore_activated_skills_from_state(ap, query, state)
|
||||
|
||||
assert restored == ['visible']
|
||||
assert list(query.variables[ACTIVATED_SKILLS_KEY].keys()) == ['visible']
|
||||
|
||||
@@ -43,7 +43,8 @@ def make_tool(name: str) -> resource_tool.LLMTool:
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_tool_manager_omits_skill_authoring_tools_by_default():
|
||||
async def test_tool_manager_includes_skill_tools_by_default():
|
||||
"""Skill tools are exposed like native tools; the SkillToolLoader self-gates."""
|
||||
manager = ToolManager(SimpleNamespace())
|
||||
manager.native_tool_loader = StubLoader([make_tool('exec')])
|
||||
manager.skill_tool_loader = StubLoader([make_tool('activate')])
|
||||
@@ -52,20 +53,21 @@ async def test_tool_manager_omits_skill_authoring_tools_by_default():
|
||||
|
||||
tools = await manager.get_all_tools()
|
||||
|
||||
assert [tool.name for tool in tools] == ['exec', 'plugin_tool', 'mcp_tool']
|
||||
assert [tool.name for tool in tools] == ['exec', 'activate', 'plugin_tool', 'mcp_tool']
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_tool_manager_includes_skill_authoring_tools_when_requested():
|
||||
async def test_tool_manager_omits_skill_tools_when_loader_unavailable():
|
||||
"""When the SkillToolLoader gate is closed (no sandbox / skill_mgr) it returns no tools."""
|
||||
manager = ToolManager(SimpleNamespace())
|
||||
manager.native_tool_loader = StubLoader([make_tool('exec')])
|
||||
manager.skill_tool_loader = StubLoader([make_tool('activate')])
|
||||
manager.skill_tool_loader = StubLoader([])
|
||||
manager.plugin_tool_loader = StubLoader([make_tool('plugin_tool')])
|
||||
manager.mcp_tool_loader = StubLoader([make_tool('mcp_tool')])
|
||||
|
||||
tools = await manager.get_all_tools(include_skill_authoring=True)
|
||||
tools = await manager.get_all_tools()
|
||||
|
||||
assert [tool.name for tool in tools] == ['exec', 'activate', 'plugin_tool', 'mcp_tool']
|
||||
assert [tool.name for tool in tools] == ['exec', 'plugin_tool', 'mcp_tool']
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
|
||||
@@ -150,7 +150,7 @@ def _import_preproc_modules():
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_preproc_enables_skill_authoring_tools_when_skill_service_available():
|
||||
async def test_preproc_loads_host_tools_for_runner():
|
||||
preproc_module, entities_module = _import_preproc_modules()
|
||||
|
||||
app = _make_app(skill_service=SimpleNamespace())
|
||||
@@ -159,7 +159,7 @@ async def test_preproc_enables_skill_authoring_tools_when_skill_service_availabl
|
||||
result = await stage.process(_make_query(), 'PreProcessor')
|
||||
|
||||
assert result.result_type == entities_module.ResultType.CONTINUE
|
||||
app.tool_mgr.get_all_tools.assert_awaited_once_with(None, None, include_skill_authoring=True)
|
||||
app.tool_mgr.get_all_tools.assert_awaited_once_with(None, None)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@@ -180,12 +180,13 @@ async def test_preproc_puts_host_skill_tools_into_query_scope():
|
||||
result = await stage.process(query, 'PreProcessor')
|
||||
|
||||
assert result.result_type == entities_module.ResultType.CONTINUE
|
||||
app.tool_mgr.get_all_tools.assert_awaited_once_with(None, None, include_skill_authoring=True)
|
||||
app.tool_mgr.get_all_tools.assert_awaited_once_with(None, None)
|
||||
assert [tool.name for tool in query.use_funcs] == ['activate', 'register_skill']
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_preproc_disables_skill_authoring_tools_when_skill_service_missing():
|
||||
async def test_preproc_loads_host_tools_regardless_of_skill_service():
|
||||
"""Skill tooling no longer gates on skill_service at the preproc layer."""
|
||||
preproc_module, entities_module = _import_preproc_modules()
|
||||
|
||||
app = _make_app(skill_service=None)
|
||||
@@ -194,7 +195,7 @@ async def test_preproc_disables_skill_authoring_tools_when_skill_service_missing
|
||||
result = await stage.process(_make_query(), 'PreProcessor')
|
||||
|
||||
assert result.result_type == entities_module.ResultType.CONTINUE
|
||||
app.tool_mgr.get_all_tools.assert_awaited_once_with(None, None, include_skill_authoring=False)
|
||||
app.tool_mgr.get_all_tools.assert_awaited_once_with(None, None)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@@ -237,10 +238,11 @@ async def test_preproc_respects_pipeline_bound_skills_subset():
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_preproc_does_not_load_skill_preferences_without_skill_authoring_service():
|
||||
async def test_preproc_does_not_load_skill_preferences_without_skill_mgr():
|
||||
preproc_module, entities_module = _import_preproc_modules()
|
||||
|
||||
app = _make_app(skill_service=None)
|
||||
app = _make_app(skill_service=SimpleNamespace())
|
||||
app.skill_mgr = None # no skill manager -> skill tooling unavailable
|
||||
|
||||
query = _make_query()
|
||||
result = await stage_process_capture(preproc_module, app, query)
|
||||
|
||||
Reference in New Issue
Block a user