diff --git a/docs/agent-runner-pluginization/PROTOCOL_V1.md b/docs/agent-runner-pluginization/PROTOCOL_V1.md index 15c74521..fcaeb209 100644 --- a/docs/agent-runner-pluginization/PROTOCOL_V1.md +++ b/docs/agent-runner-pluginization/PROTOCOL_V1.md @@ -90,7 +90,6 @@ class AgentRunnerCapabilities(BaseModel): knowledge_retrieval: bool = False multimodal_input: bool = False skill_authoring: bool = False - skill_injection: bool = False event_context: bool = True platform_api: bool = False interrupt: bool = False @@ -104,8 +103,7 @@ class AgentRunnerCapabilities(BaseModel): - `tool_calling`: runner 可能调用 Host tool API。 - `knowledge_retrieval`: runner 可能调用 Host knowledge API。 - `multimodal_input`: runner 可以处理非纯文本 input / artifact。 -- `skill_authoring`: runner 需要 Host 提供的 skill authoring tools。 -- `skill_injection`: runner 需要 Host 在 effective prompt 中注入 skill index。 +- `skill_authoring`: runner 需要 Host 提供 skill facts 以及 skill authoring tools，例如 `activate` / `register_skill`。 - `event_context`: runner 理解 event-first 输入。 - `platform_api`: runner 可能请求平台动作。 - `interrupt`: runner 支持取消或中断。 @@ -353,15 +351,23 @@ State 是可选 host-owned snapshot。Runner 也可以完全自管状态。 ## 6. Resources ```python +class SkillResource(BaseModel): + skill_name: str + display_name: str | None = None + description: str | None = None + class AgentResources(BaseModel): models: list[ModelResource] = [] tools: list[ToolResource] = [] knowledge_bases: list[KnowledgeBaseResource] = [] + skills: list[SkillResource] = [] files: list[FileResource] = [] storage: StorageResource = StorageResource() platform_capabilities: dict[str, Any] = {} ``` +`skills` 只包含本次 run 中 pipeline-visible 的 skill facts，例如 `skill_name`、`display_name` 和 `description`。Host 不把这些 facts 追加到 system prompt，也不把它们编排进工具描述；runner 可以自行决定是否放入 model prompt、转换成 MCP surface，或只在自己的策略层使用。 + 资源列表是本次 run 的授权结果。History / Event / Artifact 访问通过 permissions、`ctx.context.available_apis` 和 Host 侧 run session 校验控制，不作为可枚举 resource list 暴露。Runner 只能通过 `AgentRunAPIProxy` 访问这些能力。 ## 7. Result Stream diff --git a/src/langbot/pkg/agent/runner/config_schema.py b/src/langbot/pkg/agent/runner/config_schema.py index b09d25fd..3b6d69b6 100644 --- a/src/langbot/pkg/agent/runner/config_schema.py +++ b/src/langbot/pkg/agent/runner/config_schema.py @@ -71,13 +71,6 @@ def supports_skill_authoring(descriptor: AgentRunnerDescriptor | None) -> bool: return bool(descriptor.capabilities.get('skill_authoring', False)) -def supports_skill_injection(descriptor: AgentRunnerDescriptor | None) -> bool: - """Return whether the runner wants the Host skill index in the effective prompt.""" - if descriptor is None: - return False - return bool(descriptor.capabilities.get('skill_injection', False)) - - def extract_prompt_config( descriptor: AgentRunnerDescriptor | None, runner_config: dict[str, typing.Any], diff --git a/src/langbot/pkg/agent/runner/context_builder.py b/src/langbot/pkg/agent/runner/context_builder.py index 834a9c70..1ae2cd5f 100644 --- a/src/langbot/pkg/agent/runner/context_builder.py +++ b/src/langbot/pkg/agent/runner/context_builder.py @@ -84,6 +84,14 @@ class KnowledgeBaseResource(typing.TypedDict): kb_type: str | None +class SkillResource(typing.TypedDict): + """Skill resource payload.""" + + skill_name: str + display_name: str | None + description: str | None + + class FileResource(typing.TypedDict): """File resource payload.""" @@ -106,6 +114,7 @@ class AgentResources(typing.TypedDict): models: list[ModelResource] tools: list[ToolResource] knowledge_bases: list[KnowledgeBaseResource] + skills: list[SkillResource] files: list[FileResource] storage: StorageResource platform_capabilities: dict[str, typing.Any] diff --git a/src/langbot/pkg/agent/runner/host_models.py b/src/langbot/pkg/agent/runner/host_models.py index f4462c6d..42f5894b 100644 --- a/src/langbot/pkg/agent/runner/host_models.py +++ b/src/langbot/pkg/agent/runner/host_models.py @@ -95,6 +95,9 @@ class ResourcePolicy(pydantic.BaseModel): allowed_kb_uuids: list[str] | None = None """Additional knowledge base UUID grants. None means no additional KB grants.""" + allowed_skill_names: list[str] | None = None + """Allowed skill names. None means all currently visible skills are allowed.""" + allow_plugin_storage: bool = True """Whether plugin storage is allowed.""" diff --git a/src/langbot/pkg/agent/runner/orchestrator.py b/src/langbot/pkg/agent/runner/orchestrator.py index 9416f921..497a2745 100644 --- a/src/langbot/pkg/agent/runner/orchestrator.py +++ b/src/langbot/pkg/agent/runner/orchestrator.py @@ -20,6 +20,7 @@ from .result_normalizer import AgentResultNormalizer from .run_journal import AgentRunJournal, MAX_ARTIFACT_INLINE_BYTES as _MAX_ARTIFACT_INLINE_BYTES from .session_registry import AgentRunSessionRegistry, get_session_registry from .state_scope import build_state_context +from ...provider.tools.loaders import skill as skill_loader MAX_ARTIFACT_INLINE_BYTES = _MAX_ARTIFACT_INLINE_BYTES @@ -86,6 +87,13 @@ class AgentRunOrchestrator: session_query_id = None if adapter_context: + query = adapter_context.get('_query') + if query is not None: + skill_loader.restore_activated_skills_from_state( + self.ap, + query, + context.get('state', {}), + ) session_query_id = adapter_context.get('query_id') if 'params' in adapter_context: context['adapter']['extra']['params'] = adapter_context['params'] @@ -175,11 +183,13 @@ class AgentRunOrchestrator: ) -> typing.AsyncGenerator[provider_message.Message | provider_message.MessageChunk, None]: """Run an AgentRunner from the current Pipeline Query entry point.""" plan = self.query_bridge.build_plan(query) + adapter_context = dict(plan.adapter_context) + adapter_context['_query'] = query async for result in self.run( plan.event, plan.binding, bound_plugins=plan.bound_plugins, - adapter_context=plan.adapter_context, + adapter_context=adapter_context, ): yield result diff --git a/src/langbot/pkg/agent/runner/query_entry_adapter.py b/src/langbot/pkg/agent/runner/query_entry_adapter.py index b5d9a65e..fc6e2383 100644 --- a/src/langbot/pkg/agent/runner/query_entry_adapter.py +++ b/src/langbot/pkg/agent/runner/query_entry_adapter.py @@ -112,6 +112,7 @@ class QueryEntryAdapter: allowed_model_uuids=cls._extract_allowed_models(query), allowed_tool_names=cls._extract_allowed_tools(query), allowed_kb_uuids=cls._extract_allowed_kbs(query), + allowed_skill_names=cls._extract_allowed_skills(query), ) # Build state policy @@ -583,3 +584,19 @@ class QueryEntryAdapter: if kb_uuids: return kb_uuids return None + + @classmethod + def _extract_allowed_skills( + cls, + query: pipeline_query.Query, + ) -> list[str] | None: + """Extract pipeline-visible skill names from query.""" + variables = getattr(query, 'variables', None) + if not variables or '_pipeline_bound_skills' not in variables: + return None + bound_skills = variables.get('_pipeline_bound_skills') + if bound_skills is None: + return None + if not isinstance(bound_skills, list): + return [] + return [str(skill_name) for skill_name in bound_skills if skill_name] diff --git a/src/langbot/pkg/agent/runner/resource_builder.py b/src/langbot/pkg/agent/runner/resource_builder.py index 161c08fa..33ea8085 100644 --- a/src/langbot/pkg/agent/runner/resource_builder.py +++ b/src/langbot/pkg/agent/runner/resource_builder.py @@ -10,6 +10,7 @@ from .context_builder import ( ModelResource, ToolResource, KnowledgeBaseResource, + SkillResource, StorageResource, ) from . import config_schema @@ -36,6 +37,7 @@ class AgentResourceBuilder: - ModelResource: model_id, model_type, provider - ToolResource: tool_name, tool_type, description - KnowledgeBaseResource: kb_id, kb_name, kb_type + - SkillResource: skill_name, display_name, description - StorageResource: plugin_storage, workspace_storage """ @@ -81,12 +83,16 @@ class AgentResourceBuilder: knowledge_bases = await self._build_knowledge_bases_from_binding( manifest_perms, resource_policy, descriptor, runner_config ) + skills = self._build_skills_from_binding( + resource_policy, descriptor + ) storage = self._build_storage_from_binding(manifest_perms, binding) return { 'models': models, 'tools': tools, 'knowledge_bases': knowledge_bases, + 'skills': skills, 'files': [], # Files are populated at runtime 'storage': storage, 'platform_capabilities': {}, # Reserved for EBA @@ -193,6 +199,36 @@ class AgentResourceBuilder: return kb_resources + def _build_skills_from_binding( + self, + resource_policy: typing.Any, + descriptor: AgentRunnerDescriptor, + ) -> list[SkillResource]: + """Build pipeline-visible skill resource facts.""" + if not config_schema.supports_skill_authoring(descriptor): + return [] + + skill_mgr = getattr(self.ap, 'skill_mgr', None) + if skill_mgr is None: + return [] + + loaded_skills = getattr(skill_mgr, 'skills', {}) or {} + allowed_names = resource_policy.allowed_skill_names + if allowed_names is None: + names = sorted(loaded_skills.keys()) + else: + names = sorted(name for name in allowed_names if name in loaded_skills) + + skills: list[SkillResource] = [] + for skill_name in names: + skill_data = loaded_skills.get(skill_name) or {} + skills.append({ + 'skill_name': skill_name, + 'display_name': skill_data.get('display_name') or skill_data.get('name') or skill_name, + 'description': skill_data.get('description') or None, + }) + return skills + def _build_storage_from_binding( self, manifest_perms: dict[str, list[str]], diff --git a/src/langbot/pkg/agent/runner/session_registry.py b/src/langbot/pkg/agent/runner/session_registry.py index cdfc4f50..bc00a39b 100644 --- a/src/langbot/pkg/agent/runner/session_registry.py +++ b/src/langbot/pkg/agent/runner/session_registry.py @@ -140,6 +140,7 @@ class AgentRunSessionRegistry: 'model': {m.get('model_id') for m in resources.get('models', [])}, 'tool': {t.get('tool_name') for t in resources.get('tools', [])}, 'knowledge_base': {kb.get('kb_id') for kb in resources.get('knowledge_bases', [])}, + 'skill': {s.get('skill_name') for s in resources.get('skills', [])}, 'file': {f.get('file_id') for f in resources.get('files', [])}, } @@ -197,7 +198,7 @@ class AgentRunSessionRegistry: authorized_ids = authorization['authorized_ids'] resources = authorization['resources'] - if resource_type in ('model', 'tool', 'knowledge_base', 'file'): + if resource_type in ('model', 'tool', 'knowledge_base', 'skill', 'file'): return resource_id in authorized_ids.get(resource_type, set()) if resource_type == 'storage': diff --git a/src/langbot/pkg/pipeline/preproc/preproc.py b/src/langbot/pkg/pipeline/preproc/preproc.py index 8898f368..17adaf91 100644 --- a/src/langbot/pkg/pipeline/preproc/preproc.py +++ b/src/langbot/pkg/pipeline/preproc/preproc.py @@ -171,7 +171,6 @@ class PreProcessor(stage.PipelineStage): config_schema.supports_skill_authoring(descriptor) and getattr(self.ap, 'skill_service', None) is not None ) - inject_skill_context = config_schema.supports_skill_injection(descriptor) llm_model = None if uses_host_models: primary_uuid, fallback_uuids = config_schema.extract_model_selection(descriptor, runner_config) @@ -350,19 +349,7 @@ class PreProcessor(stage.PipelineStage): query.prompt.messages = event_ctx.event.default_prompt query.messages = event_ctx.event.prompt - # =========== Skill awareness for capable runners =========== - # The actual activation goes through the ``activate`` Tool Call so the - # LLM doesn't see full SKILL.md instructions until it commits to a - # skill (Claude Code's progressive disclosure). But the LLM still has - # to KNOW which skills exist to make that choice, so we: - # 1. resolve the pipeline's bound skills and stash them in - # ``query.variables['_pipeline_bound_skills']`` for downstream - # visibility checks (skill loader, native exec workdir); - # 2. inject a short ``Available Skills`` index (name + description - # only) into the system prompt. The contributor's original PR - # relied on this injection; without it the LLM never discovers - # the skills are there and just calls native tools instead. - if inject_skill_context and self.ap.skill_mgr: + if include_skill_authoring and getattr(self.ap, 'skill_mgr', None) is not None: pipeline_data = await self.ap.pipeline_service.get_pipeline(query.pipeline_uuid) extensions_prefs = (pipeline_data or {}).get('extensions_preferences', {}) enable_all_skills = extensions_prefs.get('enable_all_skills', True) @@ -374,43 +361,4 @@ class PreProcessor(stage.PipelineStage): query.variables['_pipeline_bound_skills'] = bound_skills - skill_addition = self.ap.skill_mgr.build_skill_aware_prompt_addition( - bound_skills=bound_skills, - ) - if skill_addition: - # Append to the first system message; create one if the - # prompt has none. Handles both plain-string and - # content-element (list) message bodies. - if query.prompt.messages and query.prompt.messages[0].role == 'system': - head = query.prompt.messages[0] - if isinstance(head.content, str): - head.content = head.content + skill_addition - elif isinstance(head.content, list): - appended = False - for ce in head.content: - if getattr(ce, 'type', None) == 'text': - ce.text = (ce.text or '') + skill_addition - appended = True - break - if not appended: - head.content.append(provider_message.ContentElement(type='text', text=skill_addition)) - else: - query.prompt.messages.insert( - 0, - provider_message.Message(role='system', content=skill_addition.strip()), - ) - self.ap.logger.debug( - f'Skill index injected into system prompt: ' - f'pipeline={query.pipeline_uuid} ' - f'bound_skills={bound_skills or "all"} ' - f'loaded_skills={len(self.ap.skill_mgr.skills)}' - ) - else: - self.ap.logger.debug( - f'No skills available for prompt injection: ' - f'pipeline={query.pipeline_uuid} ' - f'loaded_skills={len(self.ap.skill_mgr.skills)} ' - f'bound_skills={bound_skills}' - ) - return entities.StageProcessResult(result_type=entities.ResultType.CONTINUE, new_query=query) diff --git a/src/langbot/pkg/provider/tools/loaders/skill.py b/src/langbot/pkg/provider/tools/loaders/skill.py index 9df94fd2..fbfd72b3 100644 --- a/src/langbot/pkg/provider/tools/loaders/skill.py +++ b/src/langbot/pkg/provider/tools/loaders/skill.py @@ -10,6 +10,7 @@ if typing.TYPE_CHECKING: from langbot_plugin.api.entities.events import pipeline_query ACTIVATED_SKILLS_KEY = '_activated_skills' +ACTIVATED_SKILL_NAMES_STATE_KEY = 'host.activated_skills' PIPELINE_BOUND_SKILLS_KEY = '_pipeline_bound_skills' SKILL_MOUNT_PREFIX = '/workspace/.skills' _SKILL_MOUNT_PATTERN = re.compile(r'/workspace/\.skills/([A-Za-z0-9_-]+)') @@ -72,6 +73,116 @@ def register_activated_skill(query: pipeline_query.Query, skill_data: dict) -> N activated[skill_name] = skill_data +def _normalize_skill_names(value: typing.Any) -> list[str]: + if not isinstance(value, list): + return [] + + names: list[str] = [] + for item in value: + skill_name = str(item or '').strip() + if skill_name and skill_name not in names: + names.append(skill_name) + return names + + +def restore_activated_skills_from_state( + ap: app.Application, + query: pipeline_query.Query, + state: dict[str, dict[str, typing.Any]], +) -> list[str]: + """Restore persisted activated skill names into Query variables. + + The state value stores names only. Full skill metadata is rebuilt from the + current pipeline-visible skill cache so removed or unbound skills remain + unavailable to native exec/write/edit. + """ + conversation_state = state.get('conversation', {}) if isinstance(state, dict) else {} + skill_names = _normalize_skill_names(conversation_state.get(ACTIVATED_SKILL_NAMES_STATE_KEY)) + restored: list[str] = [] + for skill_name in skill_names: + skill_data = get_visible_skill(ap, query, skill_name) + if skill_data is None: + continue + register_activated_skill(query, skill_data) + restored.append(skill_name) + return restored + + +def _get_agent_run_authorization(query: pipeline_query.Query) -> dict[str, typing.Any] | None: + session = getattr(query, '_agent_run_session', None) + if not isinstance(session, dict): + return None + authorization = session.get('authorization') + return authorization if isinstance(authorization, dict) else None + + +def _get_conversation_state_target(query: pipeline_query.Query) -> tuple[str, str, str, dict[str, typing.Any]] | None: + session = getattr(query, '_agent_run_session', None) + if not isinstance(session, dict): + return None + + authorization = _get_agent_run_authorization(query) + if authorization is None: + return None + + state_policy = authorization.get('state_policy') or {} + if not state_policy.get('enable_state', True): + return None + state_scopes = state_policy.get('state_scopes', ['conversation', 'actor']) + if 'conversation' not in state_scopes: + return None + + state_context = authorization.get('state_context') or {} + scope_keys = state_context.get('scope_keys') or {} + scope_key = scope_keys.get('conversation') + if not scope_key: + return None + + runner_id = str(session.get('runner_id') or 'unknown') + binding_identity = str(state_context.get('binding_identity') or 'unknown') + return scope_key, runner_id, binding_identity, state_context + + +async def persist_activated_skill(ap: app.Application, query: pipeline_query.Query, skill_name: str) -> bool: + """Persist activated skill names for the current AgentRunner conversation. + + Returns False when the call is outside an AgentRunner run or state policy + does not expose a conversation scope. The in-memory Query activation still + remains valid for the current turn. + """ + target = _get_conversation_state_target(query) + if target is None: + return False + + persistence_mgr = getattr(ap, 'persistence_mgr', None) + if persistence_mgr is None or not hasattr(persistence_mgr, 'get_db_engine'): + return False + + from ....agent.runner.persistent_state_store import get_persistent_state_store + + scope_key, runner_id, binding_identity, state_context = target + store = get_persistent_state_store(persistence_mgr.get_db_engine()) + existing_names = _normalize_skill_names(await store.state_get(scope_key, ACTIVATED_SKILL_NAMES_STATE_KEY)) + if skill_name not in existing_names: + existing_names.append(skill_name) + + success, error = await store.state_set( + scope_key=scope_key, + state_key=ACTIVATED_SKILL_NAMES_STATE_KEY, + value=existing_names, + runner_id=runner_id, + binding_identity=binding_identity, + scope='conversation', + context=state_context, + logger=getattr(ap, 'logger', None), + ) + if not success: + logger = getattr(ap, 'logger', None) + if logger is not None: + logger.warning(f'Failed to persist activated skill "{skill_name}": {error}') + return success + + def parse_skill_mount_path(sandbox_path: str) -> tuple[str | None, str]: normalized_path = str(sandbox_path or '/workspace').strip() or '/workspace' if normalized_path == SKILL_MOUNT_PREFIX: diff --git a/src/langbot/pkg/provider/tools/loaders/skill_authoring.py b/src/langbot/pkg/provider/tools/loaders/skill_authoring.py index e758ccac..cde4314b 100644 --- a/src/langbot/pkg/provider/tools/loaders/skill_authoring.py +++ b/src/langbot/pkg/provider/tools/loaders/skill_authoring.py @@ -82,17 +82,17 @@ class SkillToolLoader(loader.ToolLoader): if not skill_name: raise ValueError('skill_name is required') - skill_mgr = self.ap.skill_mgr - skill_data = skill_mgr.get_skill_by_name(skill_name) + from . import skill as skill_loader + + skill_data = skill_loader.get_visible_skill(self.ap, query, skill_name) if skill_data is None: - visible_skills = getattr(skill_mgr, 'skills', {}) + visible_skills = skill_loader.get_visible_skills(self.ap, query) available_names = ', '.join(sorted(visible_skills.keys())) or 'none' raise ValueError(f'Skill "{skill_name}" not found. Available skills: {available_names}') # Register activated skill for sandbox mount path resolution - from . import skill as skill_loader - skill_loader.register_activated_skill(query, skill_data) + await skill_loader.persist_activated_skill(self.ap, query, skill_name) # Return SKILL.md content as Tool Result (injects into context) instructions = skill_data.get('instructions', '') @@ -191,13 +191,13 @@ class SkillToolLoader(loader.ToolLoader): return resource_tool.LLMTool( name=ACTIVATE_SKILL_TOOL_NAME, human_desc='Activate a skill', - description=self._build_activate_tool_description(), + description='Activate a pipeline-visible skill by name and return its instructions as a tool result.', parameters={ 'type': 'object', 'properties': { 'skill_name': { 'type': 'string', - 'description': 'The skill name to activate (no arguments). E.g., "pdf" or "data-analysis"', + 'description': 'The skill name to activate.', }, }, 'required': ['skill_name'], @@ -245,50 +245,3 @@ class SkillToolLoader(loader.ToolLoader): }, func=lambda parameters: parameters, ) - - def _build_activate_tool_description(self) -> str: - """Build tool description with embedded available_skills list.""" - skill_mgr = getattr(self.ap, 'skill_mgr', None) - if skill_mgr is None: - return 'Activate a skill. No skills are currently available.' - - skills = getattr(skill_mgr, 'skills', {}) - if not skills: - return 'Activate a skill. No skills are currently available.' - - # Build section - available_skills_lines = [''] - for skill_name, skill_data in sorted(skills.items()): - description = skill_data.get('description', '') - available_skills_lines.append('') - available_skills_lines.append(f'{skill_name}') - available_skills_lines.append(f'{description}') - available_skills_lines.append('') - available_skills_lines.append('') - - available_skills_block = '\n'.join(available_skills_lines) - - return f"""Activate a skill within the main conversation. - - -When users ask you to perform tasks, check if any of the available skills -below can help complete the task more effectively. Skills provide specialized -capabilities and domain knowledge. - -How to use skills: -- Invoke skills using this tool with the skill name only (no arguments) -- When you invoke a skill, you will see -The skill is activated - -- The skill's instructions will be provided in the tool result -- Examples: - - skill_name: "pdf" - invoke the pdf skill - - skill_name: "data-analysis" - invoke the data-analysis skill - -Important: -- Only use skills listed in below -- Do not invoke a skill that is already running -- To create a new skill: prepare it in /workspace, then use register_skill tool - - -{available_skills_block}""" diff --git a/src/langbot/pkg/skill/manager.py b/src/langbot/pkg/skill/manager.py index a053697f..92503a64 100644 --- a/src/langbot/pkg/skill/manager.py +++ b/src/langbot/pkg/skill/manager.py @@ -86,50 +86,3 @@ class SkillManager: def get_skill_by_name(self, name: str) -> dict | None: """Get skill data by name.""" return self.skills.get(name) - - def get_skill_index(self, bound_skills: list[str] | None = None) -> str: - """Render the pipeline-visible skills as a short ``name: description`` - index suitable for the system prompt. - - ``bound_skills`` follows the same convention as - ``query.variables['_pipeline_bound_skills']``: ``None`` means every - loaded skill is exposed; an explicit list filters to that subset. - Returns an empty string when no skills are visible. - """ - lines: list[str] = [] - for skill in self.skills.values(): - name = skill.get('name') - if not name: - continue - if bound_skills is not None and name not in bound_skills: - continue - display = skill.get('display_name') or name - description = (skill.get('description') or '').strip().replace('\n', ' ') - lines.append(f'- {name} ({display}): {description}') - - if not lines: - return '' - return 'Available Skills:\n' + '\n'.join(lines) - - def build_skill_aware_prompt_addition(self, bound_skills: list[str] | None = None) -> str: - """Build the system-prompt addendum that makes the LLM aware of the - pipeline-visible skills. - - Only metadata (name + description) is injected — the full SKILL.md is - loaded later via the ``activate`` Tool Call, protecting KV cache and - matching Claude Code's progressive disclosure pattern. Returns an - empty string when no skills are visible (no prompt change at all). - """ - skill_index = self.get_skill_index(bound_skills) - if not skill_index: - return '' - return ( - '\n\n' - f'{skill_index}\n\n' - "When the user's request clearly matches one or more skills " - 'based on their descriptions above, call the `activate` tool with ' - 'the skill name to load its full instructions. Only the name and ' - 'description are visible here; the actual instructions arrive as ' - 'the tool result. If no skill is a clear match, respond normally ' - 'without activating any skill.' - ) diff --git a/tests/factories/app.py b/tests/factories/app.py index d1edf56a..9b316ffe 100644 --- a/tests/factories/app.py +++ b/tests/factories/app.py @@ -122,11 +122,9 @@ class FakeApp: return cmd_mgr def _create_mock_skill_mgr(self): - """Mock SkillManager that returns no skill index addition by default.""" + """Mock SkillManager with no loaded skills by default.""" skill_mgr = Mock() skill_mgr.skills = {} - skill_mgr.build_skill_aware_prompt_addition = Mock(return_value='') - skill_mgr.get_skill_index = Mock(return_value=[]) return skill_mgr def _create_mock_pipeline_service(self): diff --git a/tests/unit_tests/agent/conftest.py b/tests/unit_tests/agent/conftest.py index f25631e0..60405bdd 100644 --- a/tests/unit_tests/agent/conftest.py +++ b/tests/unit_tests/agent/conftest.py @@ -8,6 +8,7 @@ def make_resources( models: list[dict] | None = None, tools: list[dict] | None = None, knowledge_bases: list[dict] | None = None, + skills: list[dict] | None = None, storage: dict | None = None, files: list[dict] | None = None, ) -> dict[str, typing.Any]: @@ -17,6 +18,7 @@ def make_resources( models: List of model dicts with 'model_id' key tools: List of tool dicts with 'tool_name' key knowledge_bases: List of KB dicts with 'kb_id' key + skills: List of skill dicts with 'skill_name' key storage: Storage permissions dict files: List of file dicts with 'file_id' key @@ -27,6 +29,7 @@ def make_resources( 'models': models or [], 'tools': tools or [], 'knowledge_bases': knowledge_bases or [], + 'skills': skills or [], 'files': files or [], 'storage': storage or {'plugin_storage': False, 'workspace_storage': False}, 'platform_capabilities': {}, @@ -71,6 +74,7 @@ def make_session( 'model': {m.get('model_id') for m in res.get('models', [])}, 'tool': {t.get('tool_name') for t in res.get('tools', [])}, 'knowledge_base': {kb.get('kb_id') for kb in res.get('knowledge_bases', [])}, + 'skill': {s.get('skill_name') for s in res.get('skills', [])}, 'file': {f.get('file_id') for f in res.get('files', [])}, } diff --git a/tests/unit_tests/agent/test_context_validation.py b/tests/unit_tests/agent/test_context_validation.py index ac0ef594..44276675 100644 --- a/tests/unit_tests/agent/test_context_validation.py +++ b/tests/unit_tests/agent/test_context_validation.py @@ -80,6 +80,7 @@ class TestContextValidation: 'models': [], 'tools': [], 'knowledge_bases': [], + 'skills': [], 'files': [], 'storage': {'plugin_storage': True, 'workspace_storage': True}, 'platform_capabilities': {}, diff --git a/tests/unit_tests/agent/test_orchestrator_integration.py b/tests/unit_tests/agent/test_orchestrator_integration.py index 81557f87..55d650c3 100644 --- a/tests/unit_tests/agent/test_orchestrator_integration.py +++ b/tests/unit_tests/agent/test_orchestrator_integration.py @@ -124,6 +124,20 @@ class FakeApplication: self.rag_mgr = types.SimpleNamespace( get_knowledge_base_by_uuid=AsyncMock(return_value=FakeKnowledgeBase("kb_001")) ) + self.skill_mgr = types.SimpleNamespace( + skills={ + "demo": { + "name": "demo", + "display_name": "Demo Skill", + "description": "Helps with demo tasks.", + }, + "hidden": { + "name": "hidden", + "display_name": "Hidden Skill", + "description": "Not bound to this pipeline.", + }, + } + ) class FakeConversation: @@ -140,7 +154,12 @@ def make_descriptor() -> AgentRunnerDescriptor: plugin_name="local-agent", runner_name="default", protocol_version="1", - capabilities={"streaming": True, "tool_calling": True, "knowledge_retrieval": True}, + capabilities={ + "streaming": True, + "tool_calling": True, + "knowledge_retrieval": True, + "skill_authoring": True, + }, config_schema=[ {"name": "model", "type": "model-fallback-selector"}, {"name": "knowledge-bases", "type": "knowledge-base-multi-selector", "default": []}, @@ -211,6 +230,7 @@ def make_query(): variables={ "_pipeline_bound_plugins": ["langbot/local-agent"], "_fallback_model_uuids": ["model_fallback"], + "_pipeline_bound_skills": ["demo"], "public_param": "visible", }, use_llm_model_uuid="model_primary", @@ -323,12 +343,20 @@ async def test_orchestrator_runs_fake_plugin_with_authorized_context(clean_agent assert {m["model_id"] for m in resources["models"]} == {"model_primary", "model_fallback"} assert resources["tools"][0]["tool_name"] == "langbot/test-tool/search" assert resources["knowledge_bases"][0]["kb_id"] == "kb_001" + assert resources["skills"] == [ + { + "skill_name": "demo", + "display_name": "Demo Skill", + "description": "Helps with demo tasks.", + } + ] assert resources["storage"]["plugin_storage"] is True session_during_run = plugin_connector.sessions_during_run[0] assert session_during_run is not None assert session_during_run["plugin_identity"] == "langbot/local-agent" assert session_during_run["authorization"]["authorized_ids"]["tool"] == {"langbot/test-tool/search"} + assert session_during_run["authorization"]["authorized_ids"]["skill"] == {"demo"} assert await get_session_registry().get(context["run_id"]) is None @@ -766,6 +794,50 @@ class TestQueryEntryAdapterHostCapabilities: snapshot = await persistent_store.build_snapshot_from_event(event, binding, descriptor) assert snapshot["conversation"]["external.test_key"] == "test_value" + @pytest.mark.asyncio + async def test_run_from_query_restores_activated_skills_from_state(self, clean_agent_state): + """Persisted activated skill names are restored into the next Query run.""" + from langbot.pkg.agent.runner.persistent_state_store import get_persistent_state_store + from langbot.pkg.provider.tools.loaders.skill import ( + ACTIVATED_SKILL_NAMES_STATE_KEY, + ACTIVATED_SKILLS_KEY, + ) + + db_engine = clean_agent_state + descriptor = make_descriptor() + plugin_connector = FakePluginConnector( + results=[ + { + "type": "message.completed", + "data": {"message": {"role": "assistant", "content": "restored"}}, + } + ] + ) + ap = FakeApplication(plugin_connector, db_engine) + orchestrator = AgentRunOrchestrator(ap, FakeRegistry(descriptor)) + query = make_query() + + persistent_store = get_persistent_state_store(db_engine) + event = QueryEntryAdapter.query_to_event(query) + agent_config = QueryEntryAdapter.config_to_agent_config(query, RUNNER_ID) + binding = AgentBindingResolver().resolve_one(event, [agent_config]) + success, error = await persistent_store.apply_update_from_event( + event, + binding, + descriptor, + "conversation", + ACTIVATED_SKILL_NAMES_STATE_KEY, + ["demo"], + None, + ) + assert success is True + assert error is None + + messages = [message async for message in orchestrator.run_from_query(query)] + + assert len(messages) == 1 + assert query.variables[ACTIVATED_SKILLS_KEY]["demo"]["name"] == "demo" + @pytest.mark.asyncio async def test_event_log_and_transcript_written(self, clean_agent_state): """EventLog and Transcript are written via Pipeline path.""" diff --git a/tests/unit_tests/agent/test_session_registry.py b/tests/unit_tests/agent/test_session_registry.py index c021e655..9b76c426 100644 --- a/tests/unit_tests/agent/test_session_registry.py +++ b/tests/unit_tests/agent/test_session_registry.py @@ -307,6 +307,21 @@ class TestIsResourceAllowed: assert registry.is_resource_allowed(session, 'knowledge_base', 'kb_001') is False + def test_skill_allowed(self): + """Skill in resources should be allowed.""" + registry = AgentRunSessionRegistry() + resources = make_resources( + skills=[ + {'skill_name': 'demo', 'display_name': 'Demo'}, + {'skill_name': 'writer', 'display_name': 'Writer'}, + ] + ) + session = make_session(resources=resources) + + assert registry.is_resource_allowed(session, 'skill', 'demo') is True + assert registry.is_resource_allowed(session, 'skill', 'writer') is True + assert registry.is_resource_allowed(session, 'skill', 'hidden') is False + def test_storage_plugin_allowed(self): """Plugin storage permission should be checked.""" registry = AgentRunSessionRegistry() diff --git a/tests/unit_tests/pipeline/test_preproc.py b/tests/unit_tests/pipeline/test_preproc.py index 9495b69c..e5381043 100644 --- a/tests/unit_tests/pipeline/test_preproc.py +++ b/tests/unit_tests/pipeline/test_preproc.py @@ -51,7 +51,6 @@ def make_host_model_runner_descriptor( tool_calling: bool = True, knowledge_retrieval: bool = True, skill_authoring: bool = False, - skill_injection: bool = False, ): from langbot.pkg.agent.runner.descriptor import AgentRunnerDescriptor @@ -72,7 +71,6 @@ def make_host_model_runner_descriptor( 'knowledge_retrieval': knowledge_retrieval, 'multimodal_input': multimodal_input, 'skill_authoring': skill_authoring, - 'skill_injection': skill_injection, }, permissions={ 'models': ['list', 'invoke', 'stream'], diff --git a/tests/unit_tests/provider/test_skill_tools.py b/tests/unit_tests/provider/test_skill_tools.py index d486e30a..dbd9d26a 100644 --- a/tests/unit_tests/provider/test_skill_tools.py +++ b/tests/unit_tests/provider/test_skill_tools.py @@ -274,6 +274,130 @@ class TestSkillToolLoader: SimpleNamespace(variables={}), ) + @pytest.mark.asyncio + async def test_activate_rejects_pipeline_hidden_skill(self): + from langbot.pkg.provider.tools.loaders.skill_authoring import ( + ACTIVATE_SKILL_TOOL_NAME, + SkillToolLoader, + ) + from langbot.pkg.provider.tools.loaders.skill import ( + ACTIVATED_SKILLS_KEY, + PIPELINE_BOUND_SKILLS_KEY, + ) + + demo = _make_skill_data(name='demo', package_root='/data/skills/demo', instructions='Demo instructions') + hidden = _make_skill_data( + name='hidden', + package_root='/data/skills/hidden', + instructions='Hidden instructions', + ) + ap = _make_ap() + ap.skill_mgr = SimpleNamespace( + skills={'demo': demo, 'hidden': hidden}, + ) + + loader = SkillToolLoader(ap) + query = SimpleNamespace(variables={PIPELINE_BOUND_SKILLS_KEY: ['demo']}) + + with pytest.raises(ValueError, match='Available skills: demo'): + await loader.invoke_tool(ACTIVATE_SKILL_TOOL_NAME, {'skill_name': 'hidden'}, query) + + result = await loader.invoke_tool(ACTIVATE_SKILL_TOOL_NAME, {'skill_name': 'demo'}, query) + + assert result['activated'] is True + assert result['skill_name'] == 'demo' + assert set(query.variables[ACTIVATED_SKILLS_KEY].keys()) == {'demo'} + + @pytest.mark.asyncio + async def test_activate_persists_and_restores_for_next_query_exec(self, tmp_path): + from sqlalchemy.ext.asyncio import create_async_engine + + from langbot.pkg.agent.runner.persistent_state_store import ( + get_persistent_state_store, + reset_persistent_state_store, + ) + from langbot.pkg.entity.persistence.base import Base + from langbot.pkg.provider.tools.loaders.native import NativeToolLoader + from langbot.pkg.provider.tools.loaders.skill_authoring import ( + ACTIVATE_SKILL_TOOL_NAME, + SkillToolLoader, + ) + from langbot.pkg.provider.tools.loaders.skill import ( + ACTIVATED_SKILL_NAMES_STATE_KEY, + ACTIVATED_SKILLS_KEY, + PIPELINE_BOUND_SKILLS_KEY, + restore_activated_skills_from_state, + ) + + reset_persistent_state_store() + engine = create_async_engine(f'sqlite+aiosqlite:///{tmp_path / "state.db"}') + async with engine.begin() as conn: + await conn.run_sync(Base.metadata.create_all) + + try: + skill = _make_skill_data(name='demo', package_root=str(tmp_path), instructions='Demo instructions') + ap = _make_ap() + ap.persistence_mgr.get_db_engine = Mock(return_value=engine) + ap.box_service = SimpleNamespace( + available=True, + default_workspace=str(tmp_path), + execute_tool=AsyncMock(return_value={'ok': True}), + ) + ap.skill_mgr = SimpleNamespace( + skills={'demo': skill}, + refresh_skill_from_disk=Mock(), + ) + + scope_key = 'conversation:plugin:langbot/local-agent/default:binding_001:conv_001' + query1 = SimpleNamespace(query_id='q1', variables={PIPELINE_BOUND_SKILLS_KEY: ['demo']}) + object.__setattr__( + query1, + '_agent_run_session', + { + 'runner_id': 'plugin:langbot/local-agent/default', + 'authorization': { + 'state_policy': {'enable_state': True, 'state_scopes': ['conversation']}, + 'state_context': { + 'scope_keys': {'conversation': scope_key}, + 'binding_identity': 'binding_001', + 'conversation_id': 'conv_001', + }, + }, + }, + ) + + await SkillToolLoader(ap).invoke_tool(ACTIVATE_SKILL_TOOL_NAME, {'skill_name': 'demo'}, query1) + + store = get_persistent_state_store(engine) + persisted_names = await store.state_get(scope_key, ACTIVATED_SKILL_NAMES_STATE_KEY) + assert persisted_names == ['demo'] + + query2 = SimpleNamespace(query_id='q2', variables={PIPELINE_BOUND_SKILLS_KEY: ['demo']}) + restored = restore_activated_skills_from_state( + ap, + query2, + {'conversation': {ACTIVATED_SKILL_NAMES_STATE_KEY: persisted_names}}, + ) + + assert restored == ['demo'] + assert set(query2.variables[ACTIVATED_SKILLS_KEY]) == {'demo'} + + result = await NativeToolLoader(ap).invoke_tool( + 'exec', + { + 'command': 'python /workspace/.skills/demo/scripts/run.py', + 'workdir': '/workspace/.skills/demo', + }, + query2, + ) + + assert result['ok'] is True + ap.box_service.execute_tool.assert_awaited_once() + ap.skill_mgr.refresh_skill_from_disk.assert_called_once_with('demo') + finally: + reset_persistent_state_store() + await engine.dispose() + @pytest.mark.asyncio async def test_register_skill_scans_directory_and_creates_skill(self): from langbot.pkg.provider.tools.loaders.skill_authoring import ( @@ -461,6 +585,35 @@ class TestNativeToolLoaderSkillPaths: assert tool_parameters['workdir'] == '/workspace/.skills/demo' ap.skill_mgr.refresh_skill_from_disk.assert_called_once_with('demo') + @pytest.mark.asyncio + async def test_exec_requires_skill_activation_even_when_skill_visible(self): + from langbot.pkg.provider.tools.loaders.native import NativeToolLoader + from langbot.pkg.provider.tools.loaders.skill import PIPELINE_BOUND_SKILLS_KEY + + with tempfile.TemporaryDirectory() as tmpdir: + ap = _make_ap() + ap.box_service = SimpleNamespace( + available=True, + default_workspace=tmpdir, + execute_tool=AsyncMock(return_value={'ok': True}), + ) + ap.skill_mgr = SimpleNamespace(skills={'demo': _make_skill_data(name='demo', package_root=tmpdir)}) + loader = NativeToolLoader(ap) + + query = SimpleNamespace(query_id='q1', variables={PIPELINE_BOUND_SKILLS_KEY: ['demo']}) + + with pytest.raises(ValueError, match='must be activated before exec'): + await loader.invoke_tool( + 'exec', + { + 'command': 'python /workspace/.skills/demo/scripts/run.py', + 'workdir': '/workspace', + }, + query, + ) + + ap.box_service.execute_tool.assert_not_awaited() + @pytest.mark.asyncio async def test_write_requires_skill_activation(self): from langbot.pkg.provider.tools.loaders.native import NativeToolLoader diff --git a/tests/unit_tests/test_preproc.py b/tests/unit_tests/test_preproc.py index 0b082e25..0e6a78f6 100644 --- a/tests/unit_tests/test_preproc.py +++ b/tests/unit_tests/test_preproc.py @@ -33,7 +33,6 @@ class _FakeRunnerDescriptor: 'knowledge_retrieval': True, 'multimodal_input': True, 'skill_authoring': True, - 'skill_injection': True, } def supports_tool_calling(self): @@ -122,7 +121,6 @@ def _make_app(*, skill_service) -> SimpleNamespace: ), agent_runner_registry=SimpleNamespace(get=AsyncMock(return_value=_FakeRunnerDescriptor())), skill_mgr=SimpleNamespace( - build_skill_aware_prompt_addition=Mock(return_value=''), skills={}, ), skill_service=skill_service, @@ -195,30 +193,24 @@ async def test_preproc_disables_skill_authoring_tools_when_skill_service_missing @pytest.mark.asyncio -async def test_preproc_injects_skill_index_into_system_prompt(): - """The Tool Call activation pattern still needs the LLM to know which - skills exist. PreProcessor must append the SkillManager's index - addendum to the first system message.""" +async def test_preproc_records_all_visible_skills_without_prompt_injection(): preproc_module, entities_module = _import_preproc_modules() app = _make_app(skill_service=SimpleNamespace()) - addendum = '\n\nAvailable Skills:\n- demo (demo): Demo skill.\n\nCall activate ...' - app.skill_mgr.build_skill_aware_prompt_addition = Mock(return_value=addendum) query = _make_query() result = await stage_process_capture(preproc_module, app, query) assert result.result_type == entities_module.ResultType.CONTINUE - app.skill_mgr.build_skill_aware_prompt_addition.assert_called_once_with(bound_skills=None) + app.pipeline_service.get_pipeline.assert_awaited_once_with('pipe-1') + assert query.variables.get('_pipeline_bound_skills') is None head = query.prompt.messages[0] assert head.role == 'system' - assert head.content.endswith(addendum) + assert head.content == 'system prompt' @pytest.mark.asyncio async def test_preproc_respects_pipeline_bound_skills_subset(): - """When ``enable_all_skills`` is false the bound list is passed through - so the addendum only mentions skills allowed for this pipeline.""" preproc_module, entities_module = _import_preproc_modules() app = _make_app(skill_service=SimpleNamespace()) @@ -230,31 +222,28 @@ async def test_preproc_respects_pipeline_bound_skills_subset(): } } ) - app.skill_mgr.build_skill_aware_prompt_addition = Mock(return_value='') query = _make_query() result = await stage_process_capture(preproc_module, app, query) assert result.result_type == entities_module.ResultType.CONTINUE - app.skill_mgr.build_skill_aware_prompt_addition.assert_called_once_with(bound_skills=['only-this']) assert query.variables.get('_pipeline_bound_skills') == ['only-this'] + assert query.prompt.messages[0].content == 'system prompt' @pytest.mark.asyncio -async def test_preproc_skips_injection_when_addendum_is_empty(): - """No visible skills → system prompt is left untouched (no - ``Available Skills`` block appended).""" +async def test_preproc_does_not_load_skill_preferences_without_skill_authoring_service(): preproc_module, entities_module = _import_preproc_modules() - app = _make_app(skill_service=SimpleNamespace()) - app.skill_mgr.build_skill_aware_prompt_addition = Mock(return_value='') + app = _make_app(skill_service=None) query = _make_query() result = await stage_process_capture(preproc_module, app, query) assert result.result_type == entities_module.ResultType.CONTINUE - if query.prompt and query.prompt.messages: - assert 'Available Skills' not in (query.prompt.messages[0].content or '') + app.pipeline_service.get_pipeline.assert_not_awaited() + assert '_pipeline_bound_skills' not in query.variables + assert query.prompt.messages[0].content == 'system prompt' @pytest.mark.asyncio