mirror of
https://github.com/langbot-app/LangBot.git
synced 2026-06-02 03:55:55 +00:00
fix(skill): re-inject skill index into local-agent system prompt
The contributor's original PR (#1917) appended an ``Available Skills`` index to the system prompt before the LLM saw the user message, so the LLM could decide whether to activate a skill. ``7145447b`` removed the text-marker activation flow and, together with it, the entire system prompt injection — but the Tool Call replacement only put the available skills inside the ``activate`` tool's description. In practice the LLM ignores tool descriptions for selection and goes straight to native tools, so user-visible skill activation silently broke. Restore the injection, adapted for the Tool Call era: - SkillManager regains ``get_skill_index(bound_skills)`` and ``build_skill_aware_prompt_addition(bound_skills)``. The addendum carries only ``name (display_name): description`` for each pipeline-visible skill plus one instruction line pointing at the ``activate`` tool. No SKILL.md contents — KV cache stays clean - PreProcessor appends the addendum to the first system message (or inserts a new one) of ``query.prompt.messages`` for the local-agent runner. Handles plain-string and ContentElement[] bodies. Skips cleanly when no skills are visible - 3 new test_preproc cases: injection happens, bound-skills subset honoured, empty addendum touches nothing. 280 passed Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -248,30 +248,67 @@ class PreProcessor(stage.PipelineStage):
|
|||||||
query.prompt.messages = event_ctx.event.default_prompt
|
query.prompt.messages = event_ctx.event.default_prompt
|
||||||
query.messages = event_ctx.event.prompt
|
query.messages = event_ctx.event.prompt
|
||||||
|
|
||||||
# =========== Store bound skills for runtime visibility checks ===========
|
# =========== Skill awareness for the local-agent runner ===========
|
||||||
# Skills are now activated through the `activate` tool (Tool Call mechanism),
|
# The actual activation goes through the ``activate`` Tool Call so the
|
||||||
# not through system prompt injection. This aligns with Claude Code's design.
|
# 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 selected_runner == 'local-agent' and self.ap.skill_mgr:
|
if selected_runner == 'local-agent' and self.ap.skill_mgr:
|
||||||
# Get bound skills from pipeline extensions_preferences
|
|
||||||
pipeline_data = await self.ap.pipeline_service.get_pipeline(query.pipeline_uuid)
|
pipeline_data = await self.ap.pipeline_service.get_pipeline(query.pipeline_uuid)
|
||||||
extensions_prefs = (pipeline_data or {}).get('extensions_preferences', {})
|
extensions_prefs = (pipeline_data or {}).get('extensions_preferences', {})
|
||||||
enable_all_skills = extensions_prefs.get('enable_all_skills', True)
|
enable_all_skills = extensions_prefs.get('enable_all_skills', True)
|
||||||
|
|
||||||
if enable_all_skills:
|
if enable_all_skills:
|
||||||
bound_skills = None # None = all skills available
|
bound_skills = None # None = all loaded skills are visible
|
||||||
else:
|
else:
|
||||||
# Get specific bound skill names
|
|
||||||
bound_skills = extensions_prefs.get('skills', [])
|
bound_skills = extensions_prefs.get('skills', [])
|
||||||
|
|
||||||
# Store bound skills in query variables for runtime path visibility checks
|
|
||||||
query.variables['_pipeline_bound_skills'] = bound_skills
|
query.variables['_pipeline_bound_skills'] = bound_skills
|
||||||
|
|
||||||
loaded_count = len(self.ap.skill_mgr.skills)
|
skill_addition = self.ap.skill_mgr.build_skill_aware_prompt_addition(
|
||||||
self.ap.logger.debug(
|
bound_skills=bound_skills,
|
||||||
f'Skills available for activate tool: '
|
|
||||||
f'pipeline={query.pipeline_uuid} '
|
|
||||||
f'loaded_skills={loaded_count} '
|
|
||||||
f'bound_skills={bound_skills or "all"}'
|
|
||||||
)
|
)
|
||||||
|
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)
|
return entities.StageProcessResult(result_type=entities.ResultType.CONTINUE, new_query=query)
|
||||||
|
|||||||
@@ -86,3 +86,50 @@ class SkillManager:
|
|||||||
def get_skill_by_name(self, name: str) -> dict | None:
|
def get_skill_by_name(self, name: str) -> dict | None:
|
||||||
"""Get skill data by name."""
|
"""Get skill data by name."""
|
||||||
return self.skills.get(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.'
|
||||||
|
)
|
||||||
|
|||||||
@@ -132,3 +132,73 @@ async def test_preproc_disables_skill_authoring_tools_when_skill_service_missing
|
|||||||
|
|
||||||
assert result.result_type == entities_module.ResultType.CONTINUE
|
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, include_skill_authoring=False)
|
||||||
|
|
||||||
|
|
||||||
|
@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."""
|
||||||
|
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)
|
||||||
|
head = query.prompt.messages[0]
|
||||||
|
assert head.role == 'system'
|
||||||
|
assert head.content.endswith(addendum)
|
||||||
|
|
||||||
|
|
||||||
|
@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())
|
||||||
|
app.pipeline_service.get_pipeline = AsyncMock(
|
||||||
|
return_value={
|
||||||
|
'extensions_preferences': {
|
||||||
|
'enable_all_skills': False,
|
||||||
|
'skills': ['only-this'],
|
||||||
|
}
|
||||||
|
}
|
||||||
|
)
|
||||||
|
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']
|
||||||
|
|
||||||
|
|
||||||
|
@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)."""
|
||||||
|
preproc_module, entities_module = _import_preproc_modules()
|
||||||
|
|
||||||
|
app = _make_app(skill_service=SimpleNamespace())
|
||||||
|
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
|
||||||
|
if query.prompt and query.prompt.messages:
|
||||||
|
assert 'Available Skills' not in (query.prompt.messages[0].content or '')
|
||||||
|
|
||||||
|
|
||||||
|
async def stage_process_capture(preproc_module, app, query):
|
||||||
|
"""Run PreProcessor.process and return the result while keeping ``query``
|
||||||
|
accessible to the assertions (process mutates query in place)."""
|
||||||
|
stage = preproc_module.PreProcessor(app)
|
||||||
|
return await stage.process(query, 'PreProcessor')
|
||||||
|
|||||||
Reference in New Issue
Block a user