diff --git a/extensions/python/message_loop_prompts_after/AGENTS.md b/extensions/python/message_loop_prompts_after/AGENTS.md index cfd2702b4..ba8118a2f 100644 --- a/extensions/python/message_loop_prompts_after/AGENTS.md +++ b/extensions/python/message_loop_prompts_after/AGENTS.md @@ -7,7 +7,9 @@ ## Ownership - Ordered Python files own current datetime, skill recall/load context, agent info, parallel job status, and workdir extras injection. -- Loaded and active skill instructions belong in prompt protocol, not prompt extras. +- Active skill instructions belong in prompt protocol. +- Explicitly loaded skill bodies belong in tool-result history with metadata so they can survive persistence and be reattached after compaction. +- Explicitly loaded skill IDs are chat-wide context data, not agent-local state. ## Local Contracts @@ -17,11 +19,11 @@ ## Work Guidance -- Coordinate prompt protocol and prompt-extra changes with skill, workdir, and profile contracts. +- Coordinate prompt protocol, history-reattachment, and prompt-extra changes with skill, workdir, and profile contracts. ## Verification -- Inspect rendered prompt protocol/extras or run prompt-construction tests after changes. +- Inspect rendered prompt protocol/history/extras or run prompt-construction tests after changes. ## Child DOX Index diff --git a/extensions/python/message_loop_prompts_after/_65_include_loaded_skills.py b/extensions/python/message_loop_prompts_after/_65_include_loaded_skills.py index 8f667bbe2..d7effd627 100644 --- a/extensions/python/message_loop_prompts_after/_65_include_loaded_skills.py +++ b/extensions/python/message_loop_prompts_after/_65_include_loaded_skills.py @@ -1,39 +1,81 @@ from helpers.extension import Extension -from helpers import skills -from tools.skills_tool import DATA_NAME_LOADED_SKILLS +from helpers import skills, tokens from agent import LoopData +SKILL_REATTACHMENT_TOKEN_BUDGET = 12_000 +SKILL_REATTACHMENT_HEADER = ( + "Reattached loaded skill instructions after history compaction." +) + + class IncludeLoadedSkills(Extension): async def execute(self, loop_data: LoopData = LoopData(), **kwargs): if not self.agent: return - protocol = loop_data.protocol_persistent - protocol.pop("loaded_skills", None) + loop_data.protocol_persistent.pop("loaded_skills", None) + loop_data.extras_persistent.pop("loaded_skills", None) - # Get loaded skills names - skill_names = self.agent.data.get(DATA_NAME_LOADED_SKILLS) + skill_names = skills.get_loaded_skill_names(self.agent) if not skill_names: return - # load skill text here - content = "" + # Loaded skill bodies live in tool-result history. This hook only keeps + # the ledger clean and restores bodies that compaction hid. visible_skill_names = [] + loaded_skills = [] for skill_name in skill_names: - if not skills.find_skill(skill_name, agent=self.agent): + skill = skills.find_skill(skill_name, agent=self.agent) + if not skill: continue - visible_skill_names.append(skill_name) - skill_data = skills.load_skill_for_agent(skill_name=skill_name, agent=self.agent) - content += "\n\n" + skill_data - self.agent.data[DATA_NAME_LOADED_SKILLS] = visible_skill_names - content = content.strip() - if not content: + visible_skill_names.append(skill.name) + loaded_skills.append(skill) + skills.set_loaded_skill_names(self.agent, visible_skill_names) + + self._reattach_missing_skill_bodies(loop_data, loaded_skills) + + def _reattach_missing_skill_bodies(self, loop_data: LoopData, loaded_skills): + if not self.agent or not loaded_skills: return + visible_skill_names = _visible_skill_names(loop_data.history_output) + selected = [] + used_tokens = 0 - # Inject into protocol - protocol["loaded_skills"] = self.agent.read_prompt( - "agent.system.skills.loaded.md", - skills=content, - ) + for skill in reversed(loaded_skills): + if skill.name in visible_skill_names: + continue + + skill_data = skills.load_skill_for_agent( + skill_name=skill.name, + agent=self.agent, + ) + message = f"{SKILL_REATTACHMENT_HEADER}\n\n{skill_data}" + message_tokens = tokens.approximate_tokens(message) + if used_tokens + message_tokens > SKILL_REATTACHMENT_TOKEN_BUDGET: + continue + + selected.append((skill, message)) + used_tokens += message_tokens + + for skill, message in reversed(selected): + history_message = self.agent.hist_add_tool_result( + "skills_tool", + message, + skill_instructions={ + "name": skill.name, + "path": str(skill.path), + "source": "skills_tool:reattach", + "content_included": True, + }, + ) + loop_data.history_output.extend(history_message.output()) + + +def _visible_skill_names(history_output) -> set[str]: + return { + name + for message in history_output or [] + if (name := skills.skill_instruction_name(message)) + } diff --git a/helpers/skills.py b/helpers/skills.py index 8a042fe58..09d178817 100644 --- a/helpers/skills.py +++ b/helpers/skills.py @@ -21,6 +21,7 @@ except Exception: # pragma: no cover MAX_ACTIVE_SKILLS = 20 ACTIVE_SKILLS_PLUGIN_NAME = "_skills" AGENT_DATA_NAME_LOADED_SKILLS = "loaded_skills" +CONTEXT_DATA_NAME_LOADED_SKILLS = AGENT_DATA_NAME_LOADED_SKILLS CONTEXT_DATA_NAME_CHAT_ACTIVE_SKILLS = "skills_chat_active" CONTEXT_DATA_NAME_CHAT_DISABLED_SKILLS = "skills_chat_disabled" CONTEXT_DATA_NAME_CHAT_VISIBLE_SKILLS = "skills_chat_visible" @@ -451,6 +452,20 @@ def load_skill_for_agent( return "\n".join(lines) +def skill_instruction_name(message: Any) -> str: + match message: + case { + "content": { + "skill_instructions": { + "content_included": included, + "name": name, + } + } + } if included: + return str(name or "").strip() + return "" + + def _get_skill_files(skill_dir: Path) -> str: """Get file tree for skill directory.""" if not skill_dir.exists(): @@ -830,19 +845,77 @@ def get_active_skills(agent: Agent | None) -> list[ActiveSkillEntry]: return _build_active_skills(agent, limit=get_max_active_skills(agent=agent)) -def get_loaded_skill_entries(agent: Agent | None) -> list[ActiveSkillEntry]: +def _normalize_loaded_skill_names(raw: Any) -> list[str]: + if not isinstance(raw, list): + return [] + + names: list[str] = [] + for value in raw: + name = str(value or "").strip() + if name and name not in names: + names.append(name) + return names + + +def get_loaded_skill_names(agent: Agent | None) -> list[str]: if not agent: return [] - loaded = getattr(agent, "data", {}).get(AGENT_DATA_NAME_LOADED_SKILLS) - if not isinstance(loaded, list): - return [] + context = getattr(agent, "context", None) + if context and hasattr(context, "get_data"): + names = _normalize_loaded_skill_names( + context.get_data(CONTEXT_DATA_NAME_LOADED_SKILLS) + ) + if names: + data = getattr(agent, "data", None) + if isinstance(data, dict): + data.pop(AGENT_DATA_NAME_LOADED_SKILLS, None) + return names - return [ - {"name": str(skill_name).strip()} - for skill_name in loaded - if str(skill_name).strip() - ] + legacy_names = _normalize_loaded_skill_names( + getattr(agent, "data", {}).get(AGENT_DATA_NAME_LOADED_SKILLS) + ) + if legacy_names: + set_loaded_skill_names(agent, legacy_names) + return legacy_names + + +def set_loaded_skill_names(agent: Agent | None, skill_names: Any) -> list[str]: + names = _normalize_loaded_skill_names(skill_names)[-MAX_ACTIVE_SKILLS:] + if not agent: + return names + + context = getattr(agent, "context", None) + if context and hasattr(context, "set_data"): + context.set_data(CONTEXT_DATA_NAME_LOADED_SKILLS, names or None) + data = getattr(agent, "data", None) + if isinstance(data, dict): + data.pop(AGENT_DATA_NAME_LOADED_SKILLS, None) + return names + + data = getattr(agent, "data", None) + if isinstance(data, dict): + data[AGENT_DATA_NAME_LOADED_SKILLS] = names + return names + + +def add_loaded_skill_name( + agent: Agent | None, + skill_name: str, + *, + limit: int | None = None, +) -> list[str]: + name = str(skill_name or "").strip() + if not name: + return get_loaded_skill_names(agent) + + names = [loaded for loaded in get_loaded_skill_names(agent) if loaded != name] + names.append(name) + return set_loaded_skill_names(agent, names[-(limit or MAX_ACTIVE_SKILLS):]) + + +def get_loaded_skill_entries(agent: Agent | None) -> list[ActiveSkillEntry]: + return [{"name": skill_name} for skill_name in get_loaded_skill_names(agent)] def unload_agent_skill(agent: Agent | None, entry: Any) -> bool: @@ -850,17 +923,9 @@ def unload_agent_skill(agent: Agent | None, entry: Any) -> bool: if not agent or not normalized: return False - data = getattr(agent, "data", None) - if not isinstance(data, dict): - return False - - loaded = data.get(AGENT_DATA_NAME_LOADED_SKILLS) - if not isinstance(loaded, list): - return False - next_loaded: list[str] = [] removed = False - for skill_name in loaded: + for skill_name in get_loaded_skill_names(agent): loaded_entry = _normalize_active_skill_entry(str(skill_name)) if loaded_entry and _entries_match(loaded_entry, normalized): removed = True @@ -868,7 +933,7 @@ def unload_agent_skill(agent: Agent | None, entry: Any) -> bool: next_loaded.append(skill_name) if removed: - data[AGENT_DATA_NAME_LOADED_SKILLS] = next_loaded + set_loaded_skill_names(agent, next_loaded) return removed diff --git a/helpers/skills.py.dox.md b/helpers/skills.py.dox.md index dd23b4aba..19e337c0b 100644 --- a/helpers/skills.py.dox.md +++ b/helpers/skills.py.dox.md @@ -30,6 +30,7 @@ - `delete_skill(skill_path: str) -> None`: Delete a skill directory. - `find_skill(skill_name: str, agent: Agent | None=..., include_content: bool=..., include_hidden: bool=...) -> Optional[Skill]` - `load_skill_for_agent(skill_name: str, agent: Agent | None=...) -> str`: Load skill and format it as a complete string for agent context. +- `skill_instruction_name(message: Any) -> str` - `_get_skill_files(skill_dir: Path) -> str`: Get file tree for skill directory. - `search_skills(query: str, limit: int=..., agent: Agent | None=..., include_hidden: bool=...) -> List[Skill]` - `validate_skill(skill: Skill) -> List[str]` @@ -45,13 +46,17 @@ - `get_scope_hidden_skills(agent: Agent | None) -> list[ActiveSkillEntry]` - `get_chat_active_skills(context: Any | None) -> list[ActiveSkillEntry]` - `get_chat_disabled_skills(context: Any | None) -> list[ActiveSkillEntry]` -- Notable constants/configuration names: `MAX_ACTIVE_SKILLS`, `ACTIVE_SKILLS_PLUGIN_NAME`, `AGENT_DATA_NAME_LOADED_SKILLS`, `CONTEXT_DATA_NAME_CHAT_ACTIVE_SKILLS`, `CONTEXT_DATA_NAME_CHAT_DISABLED_SKILLS`, `CONTEXT_DATA_NAME_CHAT_VISIBLE_SKILLS`, `_NAME_RE`. +- `get_loaded_skill_names(agent: Agent | None) -> list[str]` +- `set_loaded_skill_names(agent: Agent | None, skill_names: Any) -> list[str]` +- `add_loaded_skill_name(agent: Agent | None, skill_name: str, limit: int | None=...) -> list[str]` +- Notable constants/configuration names: `MAX_ACTIVE_SKILLS`, `ACTIVE_SKILLS_PLUGIN_NAME`, `AGENT_DATA_NAME_LOADED_SKILLS`, `CONTEXT_DATA_NAME_LOADED_SKILLS`, `CONTEXT_DATA_NAME_CHAT_ACTIVE_SKILLS`, `CONTEXT_DATA_NAME_CHAT_DISABLED_SKILLS`, `CONTEXT_DATA_NAME_CHAT_VISIBLE_SKILLS`, `_NAME_RE`. ## Runtime Contracts - Helper modules own reusable framework APIs and must preserve public callers unless all callers, tests, and docs are updated together. - Update this file whenever public functions, classes, persistence behavior, path/security assumptions, side effects, or cross-module contracts change. -- Observed side-effect areas: filesystem reads, filesystem deletion, plugin state, settings/state persistence, secret handling. +- Loaded skill names are chat-wide context data under `CONTEXT_DATA_NAME_LOADED_SKILLS`; legacy agent-local `loaded_skills` lists are migrated into context data and cleared when read. +- Observed side-effect areas: filesystem reads, filesystem deletion, plugin state, settings/state persistence, context data, secret handling. - Imported dependency areas include: `__future__`, `dataclasses`, `helpers`, `os`, `pathlib`, `re`, `typing`. ## Key Concepts diff --git a/plugins/_chat_compaction/AGENTS.md b/plugins/_chat_compaction/AGENTS.md index 88a4f8d45..ba56b0033 100644 --- a/plugins/_chat_compaction/AGENTS.md +++ b/plugins/_chat_compaction/AGENTS.md @@ -18,6 +18,7 @@ - Preserve chat history integrity and persistence after compaction. - Backup JSON and transcript artifacts must remain UTF-8 writable when chat content contains malformed Unicode such as lone surrogates. - Keep generated summaries bounded by configured model and token limits. +- Preserve loaded skill names from `skill_instructions` metadata without copying full skill bodies into compacted summaries. - Do not discard original context data unless the compaction flow explicitly owns that behavior. ## Work Guidance diff --git a/plugins/_chat_compaction/prompts/compact.sys.md b/plugins/_chat_compaction/prompts/compact.sys.md index 15868d6fc..49a2593be 100644 --- a/plugins/_chat_compaction/prompts/compact.sys.md +++ b/plugins/_chat_compaction/prompts/compact.sys.md @@ -6,6 +6,7 @@ Rules: - Use terse bullet points, not prose - Collapse related items into single lines - Keep exact values: file paths, config values, code identifiers, credentials, URLs +- Preserve loaded skill names from skill_instructions metadata, but do not copy full skill bodies - Omit anything that can be re-derived from context - Group by topic, not chronology - No meta-commentary about the summarization diff --git a/prompts/AGENTS.md b/prompts/AGENTS.md index 646da3d6d..a3b9fb85e 100644 --- a/prompts/AGENTS.md +++ b/prompts/AGENTS.md @@ -18,6 +18,7 @@ - Keep placeholder names, include aliases, and template assumptions synchronized with prompt-loading code and extensions. - Prompt changes can alter agent behavior; keep edits narrow and intentional. - Maintain clear separation between core behavior prompts and profile/plugin-specific customization. +- Summary prompts that compress history should preserve loaded skill names from `skill_instructions` metadata without copying full skill bodies. ## Work Guidance diff --git a/prompts/agent.system.tool.skills.md b/prompts/agent.system.tool.skills.md index 22c1ba0b4..c9e5ad4f7 100644 --- a/prompts/agent.system.tool.skills.md +++ b/prompts/agent.system.tool.skills.md @@ -5,7 +5,7 @@ common args: action skill_name query file_path workflow: - action `search`: find candidate skills by keywords or trigger phrases from the current task - action `list`: discover available skills -- action `load`: load one skill by `skill_name` +- action `load`: append one skill's full instructions to chat history by `skill_name` - action `read_file`: open one file inside a loaded skill directory if the user says "find/search a skill", call `search` before `load` even when the likely skill name seems obvious `read_file` requires both `skill_name` and `file_path`; load the skill first, then read `SKILL.md` or the named relative file diff --git a/prompts/fw.bulk_summary.sys.md b/prompts/fw.bulk_summary.sys.md index 46004b15b..3ecf10576 100644 --- a/prompts/fw.bulk_summary.sys.md +++ b/prompts/fw.bulk_summary.sys.md @@ -7,7 +7,8 @@ You must return a single summary of all records # Expected output Your output will be a text of the summary Length of the text should be one paragraph, approximately 100 words +If a tool result includes skill_instructions metadata, preserve the loaded skill name in the summary, but do not copy the full skill body No intro No conclusion No formatting -Only the summary text is returned \ No newline at end of file +Only the summary text is returned diff --git a/prompts/fw.topic_summary.sys.md b/prompts/fw.topic_summary.sys.md index b8d676647..da024f854 100644 --- a/prompts/fw.topic_summary.sys.md +++ b/prompts/fw.topic_summary.sys.md @@ -8,7 +8,8 @@ You must return a single summary of all records Your output will be a text of the summary Summary must be shorter than original messages Length of the text should be maximum one paragraph, approximately 100 words, shorter if original is shorter +If a tool result includes skill_instructions metadata, preserve the loaded skill name in the summary, but do not copy the full skill body No intro No conclusion No formatting -Only the summary text is returned \ No newline at end of file +Only the summary text is returned diff --git a/tests/test_skills_runtime.py b/tests/test_skills_runtime.py index 60193b73b..068a93c5b 100644 --- a/tests/test_skills_runtime.py +++ b/tests/test_skills_runtime.py @@ -228,7 +228,24 @@ def test_reactivating_name_only_scope_default_by_path_clears_hidden_override(mon assert runtime.get_chat_disabled_skills(agent.context) == [] -def test_loaded_skill_entries_come_from_agent_data(): +def test_loaded_skill_entries_come_from_context_data(): + agent = DummyAgent() + agent.context.set_data( + runtime.CONTEXT_DATA_NAME_LOADED_SKILLS, + [ + "host-computer-use", + "", + "a0-development", + ], + ) + + assert runtime.get_loaded_skill_entries(agent) == [ + {"name": "host-computer-use"}, + {"name": "a0-development"}, + ] + + +def test_loaded_skill_entries_migrate_legacy_agent_data(): agent = DummyAgent() agent.data[runtime.AGENT_DATA_NAME_LOADED_SKILLS] = [ "host-computer-use", @@ -240,6 +257,20 @@ def test_loaded_skill_entries_come_from_agent_data(): {"name": "host-computer-use"}, {"name": "a0-development"}, ] + assert agent.context.get_data(runtime.CONTEXT_DATA_NAME_LOADED_SKILLS) == [ + "host-computer-use", + "a0-development", + ] + assert runtime.AGENT_DATA_NAME_LOADED_SKILLS not in agent.data + + +def test_unloading_last_migrated_skill_does_not_restore_legacy_agent_data(): + agent = DummyAgent() + agent.data[runtime.AGENT_DATA_NAME_LOADED_SKILLS] = ["host-computer-use"] + + assert runtime.unload_agent_skill(agent, {"name": "host-computer-use"}) is True + assert agent.context.get_data(runtime.CONTEXT_DATA_NAME_LOADED_SKILLS) is None + assert runtime.get_loaded_skill_entries(agent) == [] def test_skill_runtime_does_not_alias_old_office_skill_references(): @@ -262,12 +293,15 @@ def test_skill_runtime_does_not_alias_old_office_skill_references(): ] agent = DummyAgent() - agent.data[runtime.AGENT_DATA_NAME_LOADED_SKILLS] = [ - "office-artifacts", - "word-documents", - "excel-workbooks", - "presentation-decks", - ] + agent.context.set_data( + runtime.CONTEXT_DATA_NAME_LOADED_SKILLS, + [ + "office-artifacts", + "word-documents", + "excel-workbooks", + "presentation-decks", + ], + ) assert runtime.get_loaded_skill_entries(agent) == [ {"name": "office-artifacts"}, @@ -277,7 +311,7 @@ def test_skill_runtime_does_not_alias_old_office_skill_references(): ] assert runtime.unload_agent_skill(agent, {"name": "office-artifacts"}) is True - assert agent.data[runtime.AGENT_DATA_NAME_LOADED_SKILLS] == [ + assert agent.context.get_data(runtime.CONTEXT_DATA_NAME_LOADED_SKILLS) == [ "word-documents", "excel-workbooks", "presentation-decks", @@ -415,10 +449,13 @@ def test_host_computer_use_ranks_before_linux_desktop_for_host_screen_queries(mo def test_unload_agent_skill_removes_loaded_skill_by_name(): agent = DummyAgent() - agent.data[runtime.AGENT_DATA_NAME_LOADED_SKILLS] = [ - "host-computer-use", - "a0-development", - ] + agent.context.set_data( + runtime.CONTEXT_DATA_NAME_LOADED_SKILLS, + [ + "host-computer-use", + "a0-development", + ], + ) removed = runtime.unload_agent_skill( agent, @@ -429,7 +466,7 @@ def test_unload_agent_skill_removes_loaded_skill_by_name(): ) assert removed is True - assert agent.data[runtime.AGENT_DATA_NAME_LOADED_SKILLS] == [ + assert agent.context.get_data(runtime.CONTEXT_DATA_NAME_LOADED_SKILLS) == [ "a0-development" ] diff --git a/tests/test_tool_action_contracts.py b/tests/test_tool_action_contracts.py index f689cc232..3dfb6de2a 100644 --- a/tests/test_tool_action_contracts.py +++ b/tests/test_tool_action_contracts.py @@ -34,10 +34,23 @@ class _FakeTool: self.loop_data = loop_data +class _FakeContext: + def __init__(self, data: dict | None = None) -> None: + self.id = "ctx" + self.data = data or {} + + def get_data(self, key, recursive=True): + return self.data.get(key) + + def set_data(self, key, value, recursive=True): + self.data[key] = value + + class _FakeAgent: def __init__(self) -> None: self.data = {} - self.context = types.SimpleNamespace(id="ctx") + self.context = _FakeContext() + self.history = types.SimpleNamespace(output=lambda: []) def read_prompt(self, _name: str, **kwargs) -> str: return f"deleted {kwargs.get('memory_count', 0)}" @@ -52,6 +65,50 @@ class _FakeSkill: tags: list[str] | None = None +def _normalize_loaded_skill_names(raw) -> list[str]: + if not isinstance(raw, list): + return [] + return [name for value in raw if (name := str(value or "").strip())] + + +def _get_loaded_skill_names(agent) -> list[str]: + names = _normalize_loaded_skill_names(agent.context.get_data("loaded_skills")) + if names: + return names + names = _normalize_loaded_skill_names(agent.data.get("loaded_skills")) + if names: + _set_loaded_skill_names(agent, names) + return names + + +def _set_loaded_skill_names(agent, names) -> list[str]: + names = _normalize_loaded_skill_names(names) + agent.context.set_data("loaded_skills", names or None) + return names + + +def _add_loaded_skill_name(agent, skill_name, *, limit=None) -> list[str]: + skill_name = str(skill_name or "").strip() + names = [name for name in _get_loaded_skill_names(agent) if name != skill_name] + if skill_name: + names.append(skill_name) + return _set_loaded_skill_names(agent, names[-(limit or 20):]) + + +def _skill_instruction_name(message) -> str: + match message: + case { + "content": { + "skill_instructions": { + "content_included": included, + "name": name, + } + } + } if included: + return str(name or "").strip() + return "" + + def _install_tool_stub(monkeypatch) -> None: tool_stub = types.ModuleType("helpers.tool") tool_stub.Tool = _FakeTool @@ -64,6 +121,7 @@ def _load_skills_tool(monkeypatch, skill_root: Path): skills_stub = types.ModuleType("helpers.skills") skills_stub.AGENT_DATA_NAME_LOADED_SKILLS = "loaded_skills" + skills_stub.CONTEXT_DATA_NAME_LOADED_SKILLS = "loaded_skills" skills_stub.MAX_ACTIVE_SKILLS = 20 fake_skill = _FakeSkill( name="browser-form-workflows", @@ -74,6 +132,13 @@ def _load_skills_tool(monkeypatch, skill_root: Path): skills_stub.list_skills = lambda *args, **kwargs: [fake_skill] skills_stub.search_skills = lambda *args, **kwargs: [fake_skill] skills_stub.find_skill = lambda *args, **kwargs: fake_skill + skills_stub.load_skill_for_agent = ( + lambda *args, **kwargs: "Skill: browser-form-workflows\n\nInstructions:\nUse labels before typing." + ) + skills_stub.add_loaded_skill_name = _add_loaded_skill_name + skills_stub.get_loaded_skill_names = _get_loaded_skill_names + skills_stub.set_loaded_skill_names = _set_loaded_skill_names + skills_stub.skill_instruction_name = _skill_instruction_name monkeypatch.setitem(sys.modules, "helpers.skills", skills_stub) print_style_stub = types.ModuleType("helpers.print_style") @@ -86,6 +151,68 @@ def _load_skills_tool(monkeypatch, skill_root: Path): return importlib.import_module("tools.skills_tool") +class _FakeExtension: + def __init__(self, agent=None): + self.agent = agent + + +class _FakeLoadedSkillAgent: + def __init__(self) -> None: + self.data = {} + self.context = _FakeContext({"loaded_skills": ["browser-form-workflows"]}) + self.added_tool_results = [] + + def hist_add_tool_result(self, tool_name: str, tool_result: str, **kwargs): + content = {"tool_name": tool_name, "tool_result": tool_result, **kwargs} + self.added_tool_results.append(content) + return types.SimpleNamespace( + output=lambda: [{"ai": False, "content": content}] + ) + + +def _load_loaded_skills_extension(monkeypatch, skill_root: Path): + extension_stub = types.ModuleType("helpers.extension") + extension_stub.Extension = _FakeExtension + monkeypatch.setitem(sys.modules, "helpers.extension", extension_stub) + + agent_stub = types.ModuleType("agent") + agent_stub.LoopData = lambda **kwargs: types.SimpleNamespace(**kwargs) + monkeypatch.setitem(sys.modules, "agent", agent_stub) + + skills_stub = types.ModuleType("helpers.skills") + fake_skill = _FakeSkill( + name="browser-form-workflows", + description="Use for complex browser forms.", + path=skill_root, + tags=[], + ) + skills_stub.find_skill = lambda *args, **kwargs: fake_skill + skills_stub.load_skill_for_agent = ( + lambda *args, **kwargs: "Skill: browser-form-workflows\n\nInstructions:\nUse labels before typing." + ) + skills_stub.get_loaded_skill_names = _get_loaded_skill_names + skills_stub.set_loaded_skill_names = _set_loaded_skill_names + skills_stub.skill_instruction_name = _skill_instruction_name + monkeypatch.setitem(sys.modules, "helpers.skills", skills_stub) + + tokens_stub = types.ModuleType("helpers.tokens") + tokens_stub.approximate_tokens = lambda text: len(str(text).split()) + monkeypatch.setitem(sys.modules, "helpers.tokens", tokens_stub) + + import helpers + + monkeypatch.setattr(helpers, "skills", skills_stub, raising=False) + monkeypatch.setattr(helpers, "tokens", tokens_stub, raising=False) + + skills_tool_stub = types.ModuleType("tools.skills_tool") + skills_tool_stub.DATA_NAME_LOADED_SKILLS = "loaded_skills" + monkeypatch.setitem(sys.modules, "tools.skills_tool", skills_tool_stub) + + module_name = "extensions.python.message_loop_prompts_after._65_include_loaded_skills" + sys.modules.pop(module_name, None) + return importlib.import_module(module_name) + + def _load_computer_use_remote_tool(monkeypatch): _install_tool_stub(monkeypatch) @@ -146,7 +273,9 @@ def test_skills_tool_accepts_action_alias_for_search(monkeypatch, tmp_path: Path assert "browser-form-workflows" in response.message -def test_skills_tool_load_reports_protocol_injection(monkeypatch, tmp_path: Path): +def test_skills_tool_load_appends_skill_instructions_as_tool_result( + monkeypatch, tmp_path: Path +): module = _load_skills_tool(monkeypatch, tmp_path) agent = _FakeAgent() tool = module.SkillsTool( @@ -160,8 +289,150 @@ def test_skills_tool_load_reports_protocol_injection(monkeypatch, tmp_path: Path response = asyncio.run(tool.execute(**tool.args)) - assert response.message == "Loaded skill 'browser-form-workflows' into Protocol." - assert agent.data["loaded_skills"] == ["browser-form-workflows"] + assert "Skill: browser-form-workflows" in response.message + assert response.additional["skill_instructions"]["name"] == "browser-form-workflows" + assert response.additional["skill_instructions"]["content_included"] is True + assert agent.context.get_data("loaded_skills") == ["browser-form-workflows"] + + +def test_skills_tool_load_omits_duplicate_visible_skill( + monkeypatch, tmp_path: Path +): + module = _load_skills_tool(monkeypatch, tmp_path) + agent = _FakeAgent() + tool = module.SkillsTool( + agent, + "skills_tool", + None, + {"action": "load", "skill_name": "browser-form-workflows"}, + "", + None, + ) + first = asyncio.run(tool.execute(**tool.args)) + loaded_message = { + "ai": False, + "content": {"skill_instructions": first.additional["skill_instructions"]}, + } + agent.history = types.SimpleNamespace(output=lambda: [loaded_message]) + + second = asyncio.run(tool.execute(**tool.args)) + + assert "already loaded in visible chat history" in second.message + assert "Instructions:\nUse labels before typing." not in second.message + assert second.additional["skill_instructions"]["content_included"] is False + assert second.additional["skill_instructions"]["already_loaded"] is True + assert agent.context.get_data("loaded_skills") == ["browser-form-workflows"] + + +def test_skills_tool_load_reloads_when_prior_skill_is_not_model_visible( + monkeypatch, tmp_path: Path +): + module = _load_skills_tool(monkeypatch, tmp_path) + agent = _FakeAgent() + tool = module.SkillsTool( + agent, + "skills_tool", + None, + {"action": "load", "skill_name": "browser-form-workflows"}, + "", + None, + ) + first = asyncio.run(tool.execute(**tool.args)) + hidden_message = types.SimpleNamespace( + summary="", + content={"skill_instructions": first.additional["skill_instructions"]}, + ) + agent.history = types.SimpleNamespace( + all_messages=lambda: [hidden_message], + output=lambda: [ + { + "ai": False, + "content": "Earlier history was summarized and no skill body is visible.", + } + ], + ) + + second = asyncio.run(tool.execute(**tool.args)) + + assert "Skill: browser-form-workflows" in second.message + assert second.additional["skill_instructions"]["content_included"] is True + assert agent.context.get_data("loaded_skills") == ["browser-form-workflows"] + + +def test_loaded_skills_extension_reattaches_missing_body_after_compaction( + monkeypatch, tmp_path: Path +): + module = _load_loaded_skills_extension(monkeypatch, tmp_path) + agent = _FakeLoadedSkillAgent() + loop_data = types.SimpleNamespace( + protocol_persistent={"loaded_skills": "legacy"}, + extras_persistent={"loaded_skills": "legacy"}, + history_output=[ + { + "ai": False, + "content": "Earlier history was summarized and no skill body is visible.", + } + ], + ) + + asyncio.run(module.IncludeLoadedSkills(agent).execute(loop_data)) + + assert "loaded_skills" not in loop_data.protocol_persistent + assert "loaded_skills" not in loop_data.extras_persistent + assert len(agent.added_tool_results) == 1 + added = agent.added_tool_results[0] + assert added["tool_name"] == "skills_tool" + assert "Skill: browser-form-workflows" in added["tool_result"] + assert added["skill_instructions"] == { + "name": "browser-form-workflows", + "path": str(tmp_path), + "source": "skills_tool:reattach", + "content_included": True, + } + assert loop_data.history_output[-1]["content"] == added + + +def test_loaded_skills_extension_does_not_reattach_visible_skill( + monkeypatch, tmp_path: Path +): + module = _load_loaded_skills_extension(monkeypatch, tmp_path) + agent = _FakeLoadedSkillAgent() + loop_data = types.SimpleNamespace( + protocol_persistent={}, + extras_persistent={}, + history_output=[ + { + "ai": False, + "content": { + "skill_instructions": { + "name": "browser-form-workflows", + "content_included": True, + } + }, + } + ], + ) + + asyncio.run(module.IncludeLoadedSkills(agent).execute(loop_data)) + + assert agent.added_tool_results == [] + + +def test_loaded_skills_extension_keeps_reattachments_under_budget( + monkeypatch, tmp_path: Path +): + module = _load_loaded_skills_extension(monkeypatch, tmp_path) + monkeypatch.setattr(module, "SKILL_REATTACHMENT_TOKEN_BUDGET", 1) + agent = _FakeLoadedSkillAgent() + loop_data = types.SimpleNamespace( + protocol_persistent={}, + extras_persistent={}, + history_output=[], + ) + + asyncio.run(module.IncludeLoadedSkills(agent).execute(loop_data)) + + assert agent.added_tool_results == [] def test_skills_tool_read_file_action_reads_inside_skill_dir( diff --git a/tools/skills_tool.py b/tools/skills_tool.py index f5ccc4474..131d416d7 100644 --- a/tools/skills_tool.py +++ b/tools/skills_tool.py @@ -8,7 +8,7 @@ from helpers import skills as skills_helper from helpers.print_style import PrintStyle -DATA_NAME_LOADED_SKILLS = skills_helper.AGENT_DATA_NAME_LOADED_SKILLS +DATA_NAME_LOADED_SKILLS = skills_helper.CONTEXT_DATA_NAME_LOADED_SKILLS class SkillsTool(Tool): @@ -111,7 +111,7 @@ class SkillsTool(Tool): skill_name = self._normalize_skill_name( str(kwargs.get("skill_name") or self.args.get("skill_name") or "") ) - return Response(message=self._load(skill_name), break_loop=False) + return self._load(skill_name) if action == "read_file": skill_name = self._normalize_skill_name( str(kwargs.get("skill_name") or self.args.get("skill_name") or "") @@ -185,11 +185,14 @@ class SkillsTool(Tool): ) return "\n".join(lines) - def _load(self, skill_name: str) -> str: + def _load(self, skill_name: str) -> Response: skill_name = self._normalize_skill_name(skill_name) if not skill_name: - return "Error: 'skill_name' is required for action=load." + return Response( + message="Error: 'skill_name' is required for action=load.", + break_loop=False, + ) # Verify skill exists skill = skills_helper.find_skill( @@ -198,18 +201,63 @@ class SkillsTool(Tool): agent=self.agent, ) if not skill: - return f"Error: skill not found: {skill_name!r}. Try skills_tool action=list or action=search." + return Response( + message=( + f"Error: skill not found: {skill_name!r}. " + "Try skills_tool action=list or action=search." + ), + break_loop=False, + ) - # Store skill name for fresh loading each turn - if not self.agent.data.get(DATA_NAME_LOADED_SKILLS): - self.agent.data[DATA_NAME_LOADED_SKILLS] = [] - loaded = self.agent.data[DATA_NAME_LOADED_SKILLS] - if skill.name in loaded: - loaded.remove(skill.name) - loaded.append(skill.name) - self.agent.data[DATA_NAME_LOADED_SKILLS] = loaded[-max_loaded_skills():] + skill_data = skills_helper.load_skill_for_agent( + skill_name=skill.name, + agent=self.agent, + ) + metadata = { + "name": skill.name, + "path": str(skill.path), + "source": "skills_tool:load", + "content_included": True, + } - return f"Loaded skill '{skill.name}' into Protocol." + skills_helper.add_loaded_skill_name( + self.agent, + skill.name, + limit=max_loaded_skills(), + ) + + if self._visible_skill_loaded(skill.name): + return Response( + message=( + f"Skill '{skill.name}' is already loaded in visible " + "chat history." + ), + break_loop=False, + additional={ + "skill_instructions": { + **metadata, + "content_included": False, + "already_loaded": True, + } + }, + ) + + return Response( + message=skill_data, + break_loop=False, + additional={"skill_instructions": metadata}, + ) + + def _visible_skill_loaded(self, skill_name: str) -> bool: + history_obj = getattr(self.agent, "history", None) + output = getattr(history_obj, "output", None) + if not callable(output): + return False + + return any( + skills_helper.skill_instruction_name(message) == skill_name + for message in output() + ) def _read_file(self, skill_name: str, file_path: str) -> str: if not skill_name: diff --git a/tools/skills_tool.py.dox.md b/tools/skills_tool.py.dox.md index d838d8302..fcc023911 100644 --- a/tools/skills_tool.py.dox.md +++ b/tools/skills_tool.py.dox.md @@ -15,6 +15,7 @@ - `get_log_object(self)` - `async before_execution(self, **kwargs)` - `async execute(self, **kwargs) -> Response` + - `_visible_skill_loaded(self, skill_name: str) -> bool` - Top-level functions: - `max_loaded_skills() -> int` - Notable constants/configuration names: `DATA_NAME_LOADED_SKILLS`. @@ -25,13 +26,15 @@ - Update this file whenever tool arguments, output shape, `break_loop` behavior, intervention handling, prompt instructions, or side effects change. - `SkillsTool` is a `Tool`. - `SkillsTool` defines `execute(...)`. -- Loading a skill stores it for prompt Protocol injection on subsequent turns. -- Observed side-effect areas: filesystem reads, filesystem deletion, settings/state persistence. +- Loading a skill appends the full skill body as a normal tool-result history message with `skill_instructions` metadata containing name, path, source, and content visibility. +- Loaded skill IDs are stored in chat-wide context data. +- Duplicate loads omit the full body when the same skill name remains visible in model history. +- Observed side-effect areas: filesystem reads, filesystem deletion, settings/state persistence, chat history persistence. - Imported dependency areas include: `__future__`, `helpers`, `helpers.print_style`, `helpers.tool`, `pathlib`, `typing`. ## Key Concepts -- Important called helpers/classes observed in the source: `str.strip.lower.replace`, `skill_name.strip`, `super.get_log_object`, `self._normalize_skill_name`, `self.get_log_object`, `skills_helper.list_skills`, `join`, `skills_helper.search_skills`, `skills_helper.find_skill`, `skill.path.resolve`, `Path`, `resolved.read_text`, `skill_name.startswith`, `skill_name.endswith`, `self._current_action`, `self.agent.context.log.log`, `Response`, `strip`, `loaded.remove`, `target.is_absolute`. +- Important called helpers/classes observed in the source: `str.strip.lower.replace`, `skill_name.strip`, `super.get_log_object`, `self._normalize_skill_name`, `self.get_log_object`, `skills_helper.list_skills`, `join`, `skills_helper.search_skills`, `skills_helper.find_skill`, `skills_helper.load_skill_for_agent`, `skills_helper.add_loaded_skill_name`, `skills_helper.skill_instruction_name`, `skill.path.resolve`, `Path`, `resolved.read_text`, `skill_name.startswith`, `skill_name.endswith`, `self._current_action`, `self.agent.context.log.log`, `Response`, `strip`, `target.is_absolute`. - Keep request/response, tool, or helper semantics documented here at the same time as source changes. ## Work Guidance