From 8a2aa0821b7e7de3691bf6644966540bbe37dc83 Mon Sep 17 00:00:00 2001 From: chengyongru Date: Wed, 19 Aug 2026 16:57:58 +0800 Subject: [PATCH] fix(agent): keep explicit skills out of system prompt --- nanobot/agent/context.py | 45 +++++++++++++++------ nanobot/agent/loop.py | 5 ++- tests/agent/test_context_builder.py | 19 +++++---- tests/agent/test_loop_runner_integration.py | 17 +++++++- 4 files changed, 63 insertions(+), 23 deletions(-) diff --git a/nanobot/agent/context.py b/nanobot/agent/context.py index ebb8a6304..5c9482bda 100644 --- a/nanobot/agent/context.py +++ b/nanobot/agent/context.py @@ -94,7 +94,6 @@ class ContextBuilder: def build_system_prompt( self, *, - active_skill_names: Sequence[str] | None = None, channel: str | None = None, session_summary: SessionSummary | None = None, workspace: Path | None = None, @@ -119,11 +118,6 @@ class ContextBuilder: parts.append(f"# Memory\n\n## Long-term Memory\n{memory}") active_skills = self.skills.get_always_skills() - active_skills.extend( - name - for name in (active_skill_names or ()) - if name not in active_skills - ) if active_skills: active_content = self.skills.load_skills_for_context(active_skills) if active_content: @@ -165,6 +159,33 @@ class ContextBuilder: return "\n\n---\n\n".join(parts) + def build_runtime_context_blocks( + self, + current_message: str, + blocks: Sequence[RuntimeContextBlock] | None = None, + ) -> list[RuntimeContextBlock]: + """Add explicitly invoked skill instructions to this turn's runtime context.""" + merged = list(blocks or ()) + invoked = self.skills.get_explicitly_invoked_skills(current_message) + if not invoked: + return merged + always_active = set(self.skills.get_always_skills()) + skill_names = [name for name in invoked if name not in always_active] + skill_content = self.skills.load_skills_for_context(skill_names) + if not skill_content: + return merged + skill_block = RuntimeContextBlock( + source="explicit_skills", + content=( + "[Active Skills — instructions for this user turn]\n" + f"{skill_content}\n" + "[/Active Skills]" + ), + ) + if skill_block not in merged: + merged.append(skill_block) + return merged + @staticmethod def _without_duplicate_session_summary( entries: list[dict[str, Any]], @@ -279,16 +300,10 @@ class ContextBuilder: ) -> list[dict[str, Any]]: """Build the complete message list for an LLM call.""" root = workspace or self.workspace - active_skill_names = ( - self.skills.get_explicitly_invoked_skills(current_message) - if current_role == "user" - else [] - ) messages: list[dict[str, Any]] = [ { "role": "system", "content": self.build_system_prompt( - active_skill_names=active_skill_names, channel=channel, session_summary=session_summary, workspace=root, @@ -332,7 +347,11 @@ class ContextBuilder: ) -> dict[str, Any]: """Build only the fresh turn message without merging it into history.""" content = self.build_user_content(current_message, image_paths=media) - blocks = list(runtime_context_blocks or ()) if current_role == "user" else [] + blocks = ( + self.build_runtime_context_blocks(current_message, runtime_context_blocks) + if current_role == "user" + else [] + ) merged, runtime_context_meta = append_runtime_context(content, blocks) current: dict[str, Any] = {"role": current_role, "content": merged} if current_role == "user" and runtime_context_meta is not None: diff --git a/nanobot/agent/loop.py b/nanobot/agent/loop.py index 872b61425..945a2b9c4 100644 --- a/nanobot/agent/loop.py +++ b/nanobot/agent/loop.py @@ -797,7 +797,10 @@ class AgentLoop: ] blocks = runtime_context_blocks_from_metadata(request.metadata) blocks.extend(await resolve_runtime_context(providers, request)) - return blocks + return self.context.build_runtime_context_blocks( + request.original_user_text or "", + blocks, + ) async def _dispatch_command_inline( self, diff --git a/tests/agent/test_context_builder.py b/tests/agent/test_context_builder.py index 67e50cf4b..ce5485ad4 100644 --- a/tests/agent/test_context_builder.py +++ b/tests/agent/test_context_builder.py @@ -407,15 +407,20 @@ class TestBuildMessages: builder = _builder(tmp_path) messages = builder.build_messages([], "Please $review this patch and use $review carefully.") + plain_messages = builder.build_messages([], "Please review this patch carefully.") system_prompt = messages[0]["content"] - assert "# Active Skills" in system_prompt - assert "### Skill: review" in system_prompt - assert "Follow the unique review checklist." in system_prompt - assert system_prompt.count("### Skill: review") == 1 - assert messages[-1]["content"] == ( - "Please $review this patch and use $review carefully." - ) + user_prompt = messages[-1]["content"] + assert system_prompt == plain_messages[0]["content"] + assert "Follow the unique review checklist." not in system_prompt + assert "Please $review this patch" in user_prompt + assert "[Active Skills — instructions for this user turn]" in user_prompt + assert "### Skill: review" in user_prompt + assert "Follow the unique review checklist." in user_prompt + assert user_prompt.count("### Skill: review") == 1 + assert messages[-1]["_meta"]["runtime_context"]["sources"] == [ + "explicit_skills" + ] def test_unknown_skill_reference_does_not_change_active_skills(self, tmp_path): messages = _builder(tmp_path).build_messages([], "Keep the shell literal $HOME.") diff --git a/tests/agent/test_loop_runner_integration.py b/tests/agent/test_loop_runner_integration.py index da8a08930..8f5231363 100644 --- a/tests/agent/test_loop_runner_integration.py +++ b/tests/agent/test_loop_runner_integration.py @@ -146,6 +146,16 @@ async def test_runtime_context_is_persisted_as_next_turn_prompt_prefix(tmp_path) from nanobot.bus.events import InboundMessage from nanobot.bus.queue import MessageBus + skill_dir = tmp_path / "skills" / "review" + skill_dir.mkdir(parents=True) + (skill_dir / "SKILL.md").write_text( + "---\n" + "name: review\n" + "description: Review changes.\n" + "---\n\n" + "Follow the unique review checklist.", + encoding="utf-8", + ) provider = MagicMock() provider.get_default_model.return_value = "test-model" provider.generation = GenerationSettings() @@ -169,7 +179,7 @@ async def test_runtime_context_is_persisted_as_next_turn_prompt_prefix(tmp_path) channel="cli", sender_id="user", chat_id="direct", - content="first turn", + content="first turn $review", )) await loop._process_message(InboundMessage( channel="cli", @@ -184,6 +194,9 @@ async def test_runtime_context_is_persisted_as_next_turn_prompt_prefix(tmp_path) second_wire = LLMProvider._sanitize_empty_content(second_request) assert second_wire[: len(first_wire)] == first_wire assert first_wire[1] == second_wire[1] + assert first_wire[0] == second_wire[0] + assert "Follow the unique review checklist." not in first_wire[0]["content"] + assert "Follow the unique review checklist." in first_wire[1]["content"] assert second_wire[2]["role"] == "assistant" assert second_wire[2]["content"] == "first answer" assert second_wire[3]["content"].startswith("second turn") @@ -191,7 +204,7 @@ async def test_runtime_context_is_persisted_as_next_turn_prompt_prefix(tmp_path) persisted_first_user = session.messages[0] assert persisted_first_user["content"] == first_wire[1]["content"] - assert public_history_message(persisted_first_user)["content"] == "first turn" + assert public_history_message(persisted_first_user)["content"] == "first turn $review" @pytest.mark.asyncio