From 7e53d4f683f7bc13264d83925cd3a1e6a9cc025f Mon Sep 17 00:00:00 2001 From: lixin Date: Thu, 26 Mar 2026 10:19:35 +0800 Subject: [PATCH] Refactor skill reminder hot reload context handling Move the skill reminder out of persisted conversation history and inject it as runtime context when assembling each model request. This keeps the reminder aligned with the latest skill snapshot instead of accumulating stale reminders across a session. Also update the skill reminder text to include the current skill count, refresh the oracle prompt to treat the current reminder as the source of truth for the active turn, and add regression coverage for runtime reminder injection and skill tool validation across hot reload changes. --- src/aish/context_manager.py | 6 +++- src/aish/llm.py | 59 +++++++++++++++++----------------- src/aish/prompts/oracle.md | 2 +- src/aish/tools/skill.py | 1 + tests/test_skill_hot_reload.py | 32 +++++++++++++----- 5 files changed, 60 insertions(+), 40 deletions(-) diff --git a/src/aish/context_manager.py b/src/aish/context_manager.py index 91ed60b..741a780 100644 --- a/src/aish/context_manager.py +++ b/src/aish/context_manager.py @@ -162,7 +162,11 @@ def _trim_by_type(self, memory_type: MemoryType, count: int): def _trim_to_token_budget(self): """Trim memories to fit within token budget.""" - while self.estimate_tokens() > self.token_budget and len(self.memories) > 0: + while ( + self.token_budget is not None + and self.estimate_tokens() > self.token_budget + and len(self.memories) > 0 + ): # Find oldest non-system memory removed = False for i, memory in enumerate(self.memories): diff --git a/src/aish/llm.py b/src/aish/llm.py index dfc6e40..f382ea3 100644 --- a/src/aish/llm.py +++ b/src/aish/llm.py @@ -333,7 +333,6 @@ def __init__( self.skill_manager = skill_manager self.prompt_manager = PromptManager() self._skills_version_for_tools = self.skill_manager.skills_version - self._skills_version_for_reminder: Optional[int] = None # Event callback for new event-driven architecture self.event_callback = event_callback @@ -1063,6 +1062,13 @@ def _sync_skill_tool_from_manager_if_needed(self) -> None: self._skills_version_for_tools = current_version self._tools_spec = None + def _reload_skills_at_safe_point(self) -> None: + try: + self.skill_manager.reload_if_dirty() + except Exception: + pass + self._sync_skill_tool_from_manager_if_needed() + def _build_skills_reminder_message(self) -> Optional[dict]: """Build a skills reminder message from current loaded skills snapshot.""" try: @@ -1078,56 +1084,51 @@ def _build_skills_reminder_message(self) -> Optional[dict]: ), } - def _append_skills_reminder_if_needed( - self, context_manager: ContextManager - ) -> None: - """Append skills reminder on first turn and whenever skill snapshot changes.""" - try: - self.skill_manager.reload_if_dirty() - except Exception: - pass - self._sync_skill_tool_from_manager_if_needed() - current_version = self.skill_manager.skills_version - if self._skills_version_for_reminder == current_version: - return + def _inject_runtime_messages( + self, messages: list[dict], runtime_messages: list[dict] + ) -> list[dict]: + if not runtime_messages: + return messages - reminder = self._build_skills_reminder_message() - if reminder is None: - return + insertion_index = 0 + while ( + insertion_index < len(messages) + and messages[insertion_index].get("role") == "system" + ): + insertion_index += 1 - context_manager.add_memory(MemoryType.LLM, reminder) - self._skills_version_for_reminder = current_version + return ( + messages[:insertion_index] + + runtime_messages + + messages[insertion_index:] + ) def _get_tools_spec(self) -> list[dict]: # Lazy reload: file changes only invalidate; next tool-spec build reloads. - try: - self.skill_manager.reload_if_dirty() - except Exception: - pass - self._sync_skill_tool_from_manager_if_needed() + self._reload_skills_at_safe_point() if self._tools_spec is None: self._tools_spec = [t.to_func_spec() for t in self.tools.values()] return self._tools_spec # TODO: refresh tools spec when skills are updated, reserved for future use def refresh_tools_spec(self) -> list[dict]: - try: - self.skill_manager.reload_if_dirty() - except Exception: - pass - self._sync_skill_tool_from_manager_if_needed() + self._reload_skills_at_safe_point() self._tools_spec = [t.to_func_spec() for t in self.tools.values()] return self._tools_spec def _get_messages_with_system( self, context_manager: ContextManager, system_message: Optional[str] ) -> list[dict]: + self._reload_skills_at_safe_point() messages = context_manager.as_messages() if system_message: if messages and messages[0]["role"] == "system": messages[0]["content"] = system_message else: messages.insert(0, {"role": "system", "content": system_message}) + reminder = self._build_skills_reminder_message() + if reminder is not None: + messages = self._inject_runtime_messages(messages, [reminder]) return messages async def _handle_tool_calls( @@ -1218,8 +1219,6 @@ async def process_input( events = _LLMEventEmitter(self, emit_events) events.emit_op_start(operation="process_input", prompt=prompt, stream=stream) - self._append_skills_reminder_if_needed(context_manager) - # Add user prompt to context manager first context_manager.add_memory(MemoryType.LLM, {"role": "user", "content": prompt}) diff --git a/src/aish/prompts/oracle.md b/src/aish/prompts/oracle.md index 9d3144e..42fac11 100644 --- a/src/aish/prompts/oracle.md +++ b/src/aish/prompts/oracle.md @@ -70,7 +70,7 @@ Tool results and user messages may include or other tags. Tags - 当用户需要修改已有文件内容时，使用 **edit_file**工具，工具名称：edit_file。（先用 read_file 读取内容，再进行精确字符串替换；old_string 必须唯一，否则需要提供更大上下文或使用 replace_all。） - 当需要读取文件内容时，使用 **read_file**工具，工具名称：read_file。 - IMPORTANT: Do not use terminal commands (cat, head, tail, etc.) to read files. Instead, use the read_file tool. If you use cat, the file may not be properly preserved in context and can result in errors in the future. -- **Skill** tool is used to invoke user-invocable skills to accomplish user's request. IMPORTANT: Only use Skill for skills listed in the latest `...` user message for the current turn - do not guess or use built-in CLI commands. Skills can be hot-reloaded (added/removed/modified) during a session, so treat that latest reminder as the source of truth for the *current* turn; always re-check that the skill exists there right before invoking it, and do not rely on memory from earlier turns. If the user asks how many skills are available, answer by counting the skill items in that latest reminder (do not guess). CAVEAT: user scope skills are stored under the app's config directory. Do NOT create or modify files inside the skill or config directories. If the skill needs to generate, create, or write any files/directories, it must write only to a dedicated subdirectory under the current working directory (recommended examples: `./tmp`, `./artifacts`); do not write directly into the cwd root. Create the subdirectory if missing. If a tool or script accepts an output path (e.g. --path/--output/--dir), you must explicitly set it to a dedicated cwd subdirectory and never rely on defaults. If you cannot set a safe output path, ask the user before continuing. +- **Skill** tool is used to invoke user-invocable skills to accomplish user's request. IMPORTANT: Only use Skill for skills listed in the current `...` user message for the current turn - do not guess or use built-in CLI commands. Skills can be hot-reloaded (added/removed/modified) during a session, and the current reminder is the single source of truth for the *current* turn; always re-check that the skill exists there right before invoking it, and do not rely on memory from earlier turns. If the user asks about the current available skills, answer from the current reminder and do not rely on memory from earlier turns. CAVEAT: user scope skills are stored under the app's config directory. Do NOT create or modify files inside the skill or config directories. If the skill needs to generate, create, or write any files/directories, it must write only to a dedicated subdirectory under the current working directory (recommended examples: `./tmp`, `./artifacts`); do not write directly into the cwd root. Create the subdirectory if missing. If a tool or script accepts an output path (e.g. --path/--output/--dir), you must explicitly set it to a dedicated cwd subdirectory and never rely on defaults. If you cannot set a safe output path, ask the user before continuing. ## 长期运行命令处理原则 当用户的意图是运行一个**长期运行**或**交互式**的命令时，**不要使用****bash_exec**工具执行。 diff --git a/src/aish/tools/skill.py b/src/aish/tools/skill.py index 987e419..539e045 100644 --- a/src/aish/tools/skill.py +++ b/src/aish/tools/skill.py @@ -49,6 +49,7 @@ def render_skills_reminder_text(skills: list[SkillMetadataInfo]) -> str: skills_list = render_skills_list_for_reminder(skills) return ( "The following skills are available for use with the Skill tool:\n" + f"Current skills count: {len(skills)}\n\n" f"{skills_list}" ) diff --git a/tests/test_skill_hot_reload.py b/tests/test_skill_hot_reload.py index 136e349..ac35788 100644 --- a/tests/test_skill_hot_reload.py +++ b/tests/test_skill_hot_reload.py @@ -1,15 +1,17 @@ import shutil from pathlib import Path +from typing import cast from unittest.mock import AsyncMock, patch import anyio import pytest from aish.config import ConfigModel -from aish.context_manager import ContextManager +from aish.context_manager import ContextManager, MemoryType from aish.llm import LLMSession from aish.skills import SkillManager from aish.skills.hotreload import SkillHotReloadService +from aish.tools.skill import SkillTool def _write_skill(path: Path, *, name: str, description: str) -> None: @@ -56,6 +58,20 @@ def _extract_skills_reminders(messages: list[dict]) -> list[tuple[int, str]]: return results +def _persisted_skills_reminders(context_manager: ContextManager) -> list[str]: + reminders: list[str] = [] + for memory in context_manager.memories: + if memory.get("memory_type") != MemoryType.LLM: + continue + content = memory.get("content") + if not isinstance(content, dict): + continue + body = content.get("content") + if isinstance(body, str) and "" in body: + reminders.append(body) + return reminders + + def test_skill_manager_invalidate_then_reload(tmp_path, monkeypatch): home = tmp_path / "home" home.mkdir() @@ -188,12 +204,12 @@ async def run_once(prompt: str, system_message: str) -> list[dict]: second_messages = await run_once("question-v2", "sys-v2") second_reminders = _extract_skills_reminders(second_messages) - assert len(second_reminders) == 2 - older_reminder = second_reminders[0][1] + assert len(second_reminders) == 1 latest_reminder = second_reminders[-1][1] - assert "- my-skill: v1" in older_reminder + assert "Current skills count: 1" in latest_reminder assert "- my-skill: v2" in latest_reminder assert "- my-skill: v1" not in latest_reminder + assert _persisted_skills_reminders(context_manager) == [] @pytest.mark.anyio @@ -338,12 +354,12 @@ async def run_once(prompt: str, system_message: str) -> str: await run_once("question-2", "sys-2") reminders_after_delete = _extract_skills_reminders(captured_messages[-1]) - assert len(reminders_after_delete) == 2 - older_reminder = reminders_after_delete[0][1] + assert len(reminders_after_delete) == 1 latest_reminder = reminders_after_delete[-1][1] - assert "- skill-2: two" in older_reminder + assert "Current skills count: 1" in latest_reminder assert "- skill-1: one" in latest_reminder assert "- skill-2: two" not in latest_reminder + assert _persisted_skills_reminders(context_manager) == [] @pytest.mark.anyio @@ -467,7 +483,7 @@ def test_skill_tool_runtime_validation_tracks_deleted_skill(tmp_path, monkeypatc # Prime sync once. session._get_tools_spec() - tool = session.tools["skill"] + tool = cast(SkillTool, session.tools["skill"]) ok_result = tool(skill_name="skill-2") assert ok_result.ok is True