mirror of
https://github.com/langbot-app/LangBot.git
synced 2026-07-17 01:46:07 +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:
@@ -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']
|
||||
|
||||
@@ -65,7 +65,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')])
|
||||
@@ -74,20 +75,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
|
||||
|
||||
Reference in New Issue
Block a user