From 3c83b2eca290f824867cb51cbed3fec60de2a8cb Mon Sep 17 00:00:00 2001 From: Alessandro <155005371+3clyp50@users.noreply.github.com> Date: Tue, 16 Jun 2026 14:31:37 +0200 Subject: [PATCH] Reattach loaded skills after compaction When compression hides an explicitly loaded skill body, reattach the current missing revision as a normal skills_tool history result under one fixed budget. Preserve skill name and revision metadata in automatic and manual compaction summaries without copying full skill bodies. --- .../message_loop_prompts_after/AGENTS.md | 5 +- .../_65_include_loaded_skills.py | 76 +++++++++- helpers/skills.py | 5 + helpers/skills.py.dox.md | 5 +- plugins/_chat_compaction/AGENTS.md | 1 + .../_chat_compaction/prompts/compact.sys.md | 1 + prompts/AGENTS.md | 1 + prompts/fw.bulk_summary.sys.md | 3 +- prompts/fw.topic_summary.sys.md | 3 +- tests/test_tool_action_contracts.py | 136 ++++++++++++++++-- tools/skills_tool.py | 7 +- tools/skills_tool.py.dox.md | 4 +- 12 files changed, 222 insertions(+), 25 deletions(-) diff --git a/extensions/python/message_loop_prompts_after/AGENTS.md b/extensions/python/message_loop_prompts_after/AGENTS.md index 99b799f6c..983486fc8 100644 --- a/extensions/python/message_loop_prompts_after/AGENTS.md +++ b/extensions/python/message_loop_prompts_after/AGENTS.md @@ -2,11 +2,11 @@ ## Purpose -- Own prompt extras appended after primary message-loop prompt construction. +- Own prompt extras and history-output adjustments appended after primary message-loop prompt construction. ## Ownership -- Ordered Python files own current datetime, skill recall/load context, agent info, parallel job status, and workdir extras injection. +- Ordered Python files own current datetime, skill recall/load context, loaded-skill reattachment, agent info, parallel job status, and workdir extras injection. ## Local Contracts @@ -14,6 +14,7 @@ - Preserve ordering where later prompt extras depend on earlier recall or load results. - Do not expose secrets or private files from workdir extras. - Explicitly loaded skill instructions belong in normal tool-result history; this hook may recall candidate skills, but must not reinject loaded skill bodies through prompt extras every turn. +- If compression hides an explicitly loaded skill body, this hook may reattach the missing visible revision as a bounded normal tool-result history message. ## Work Guidance 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 261a6b611..87f582aa2 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,9 +1,15 @@ from helpers.extension import Extension -from helpers import skills +from helpers import skills, tokens from tools.skills_tool import DATA_NAME_LOADED_SKILLS 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: @@ -18,8 +24,72 @@ class IncludeLoadedSkills(Extension): # tool-result history message. Keep this legacy ledger pruned, but do # not reinject loaded skills through prompt extras every turn. 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) + visible_skill_names.append(skill.name) + loaded_skills.append(skill) self.agent.data[DATA_NAME_LOADED_SKILLS] = 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_revisions = _visible_skill_revisions(loop_data.history_output) + selected = [] + used_tokens = 0 + + for skill in reversed(loaded_skills): + skill_data = skills.load_skill_for_agent( + skill_name=skill.name, + agent=self.agent, + ) + revision = skills.skill_revision(skill_data) + if (skill.name, revision) in visible_revisions: + continue + + 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, revision, message)) + used_tokens += message_tokens + + for skill, revision, message in reversed(selected): + history_message = self.agent.hist_add_tool_result( + "skills_tool", + message, + skill_instructions={ + "name": skill.name, + "path": str(skill.path), + "revision": revision, + "source": "skills_tool:reattach", + "content_included": True, + }, + ) + loop_data.history_output.extend(history_message.output()) + + +def _visible_skill_revisions(history_output) -> set[tuple[str, str]]: + visible = set() + for message in history_output or []: + if not isinstance(message, dict): + continue + content = message.get("content") + if not isinstance(content, dict): + continue + meta = content.get("skill_instructions") + if not isinstance(meta, dict): + continue + if not meta.get("content_included"): + continue + name = str(meta.get("name") or "").strip() + revision = str(meta.get("revision") or "").strip() + if name and revision: + visible.add((name, revision)) + return visible diff --git a/helpers/skills.py b/helpers/skills.py index 8a042fe58..bff6773ef 100644 --- a/helpers/skills.py +++ b/helpers/skills.py @@ -1,5 +1,6 @@ from __future__ import annotations +import hashlib import os import re from dataclasses import dataclass, field @@ -451,6 +452,10 @@ def load_skill_for_agent( return "\n".join(lines) +def skill_revision(skill_data: str) -> str: + return hashlib.sha256(skill_data.encode("utf-8")).hexdigest()[:16] + + def _get_skill_files(skill_dir: Path) -> str: """Get file tree for skill directory.""" if not skill_dir.exists(): diff --git a/helpers/skills.py.dox.md b/helpers/skills.py.dox.md index dd23b4aba..716b50e2d 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_revision(skill_data: str) -> 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]` @@ -52,11 +53,11 @@ - 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. -- Imported dependency areas include: `__future__`, `dataclasses`, `helpers`, `os`, `pathlib`, `re`, `typing`. +- Imported dependency areas include: `__future__`, `dataclasses`, `hashlib`, `helpers`, `os`, `pathlib`, `re`, `typing`. ## Key Concepts -- Important called helpers/classes observed in the source: `dataclass`, `re.compile`, `field`, `Path`, `root.rglob`, `results.sort`, `re.sub`, `path.read_text`, `text.splitlines`, `join.strip`, `parse_frontmatter`, `frontmatter_text.splitlines`, `_parse_frontmatter_fallback`, `split_frontmatter`, `str.strip`, `_coerce_list`, `Skill`, `get_skill_roots`, `_filter_hidden_skills`, `files.get_abs_path`. +- Important called helpers/classes observed in the source: `dataclass`, `re.compile`, `field`, `Path`, `root.rglob`, `results.sort`, `re.sub`, `path.read_text`, `text.splitlines`, `join.strip`, `parse_frontmatter`, `frontmatter_text.splitlines`, `_parse_frontmatter_fallback`, `split_frontmatter`, `str.strip`, `_coerce_list`, `Skill`, `get_skill_roots`, `_filter_hidden_skills`, `files.get_abs_path`, `hashlib.sha256`. - Keep request/response, tool, or helper semantics documented here at the same time as source changes. ## Work Guidance diff --git a/plugins/_chat_compaction/AGENTS.md b/plugins/_chat_compaction/AGENTS.md index c1d7b418a..479965586 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. - Keep generated summaries bounded by configured model and token limits. - Do not discard original context data unless the compaction flow explicitly owns that behavior. +- Preserve loaded skill name/revision metadata in summaries without copying full skill bodies. ## Work Guidance diff --git a/plugins/_chat_compaction/prompts/compact.sys.md b/plugins/_chat_compaction/prompts/compact.sys.md index 15868d6fc..fd2780792 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 and revisions 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 0728bbafe..e2a8834ef 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. +- Framework summary prompts must preserve loaded skill name/revision metadata without copying full skill bodies. ## Work Guidance diff --git a/prompts/fw.bulk_summary.sys.md b/prompts/fw.bulk_summary.sys.md index 46004b15b..349bc189e 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 and revision 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..e044d9c4d 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 and revision 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_tool_action_contracts.py b/tests/test_tool_action_contracts.py index f19614b2d..c411e23a6 100644 --- a/tests/test_tool_action_contracts.py +++ b/tests/test_tool_action_contracts.py @@ -78,6 +78,7 @@ def _load_skills_tool(monkeypatch, skill_root: Path): skills_stub.load_skill_for_agent = ( lambda *args, **kwargs: "Skill: browser-form-workflows\n\nInstructions:\nUse labels before typing." ) + skills_stub.skill_revision = lambda skill_data: "rev1" monkeypatch.setitem(sys.modules, "helpers.skills", skills_stub) print_style_stub = types.ModuleType("helpers.print_style") @@ -90,6 +91,65 @@ 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 = {"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.skill_revision = lambda skill_data: "rev1" + 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) @@ -235,15 +295,75 @@ def test_skills_tool_load_reloads_when_prior_skill_is_not_model_visible( assert second.additional["skill_instructions"]["content_included"] is True -def test_loaded_skills_extension_no_longer_reinjects_skill_bodies(): - project_root = Path(__file__).resolve().parents[1] - extension = ( - project_root - / "extensions/python/message_loop_prompts_after/_65_include_loaded_skills.py" - ).read_text(encoding="utf-8") +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( + extras_persistent={"loaded_skills": "legacy"}, + history_output=[ + { + "ai": False, + "content": "Earlier history was summarized and no skill body is visible.", + } + ], + ) - assert 'extras["loaded_skills"]' not in extension - assert "load_skill_for_agent" not in extension + asyncio.run(module.IncludeLoadedSkills(agent).execute(loop_data)) + + 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), + "revision": "rev1", + "source": "skills_tool:reattach", + "content_included": True, + } + assert loop_data.history_output[-1]["content"] == added + + +def test_loaded_skills_extension_does_not_reattach_visible_revision( + monkeypatch, tmp_path: Path +): + module = _load_loaded_skills_extension(monkeypatch, tmp_path) + agent = _FakeLoadedSkillAgent() + loop_data = types.SimpleNamespace( + extras_persistent={}, + history_output=[ + { + "ai": False, + "content": { + "skill_instructions": { + "name": "browser-form-workflows", + "revision": "rev1", + "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(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 06fc20d15..bb342cccd 100644 --- a/tools/skills_tool.py +++ b/tools/skills_tool.py @@ -1,6 +1,5 @@ from __future__ import annotations -import hashlib from pathlib import Path from typing import List @@ -214,7 +213,7 @@ class SkillsTool(Tool): skill_name=skill.name, agent=self.agent, ) - revision = self._skill_revision(skill_data) + revision = skills_helper.skill_revision(skill_data) metadata = { "name": skill.name, "path": str(skill.path), @@ -255,10 +254,6 @@ class SkillsTool(Tool): additional={"skill_instructions": metadata}, ) - @staticmethod - def _skill_revision(skill_data: str) -> str: - return hashlib.sha256(skill_data.encode("utf-8")).hexdigest()[:16] - def _visible_skill_revision_loaded(self, skill_name: str, revision: str) -> bool: history_obj = getattr(self.agent, "history", None) output = getattr(history_obj, "output", None) diff --git a/tools/skills_tool.py.dox.md b/tools/skills_tool.py.dox.md index 35d014984..21e825617 100644 --- a/tools/skills_tool.py.dox.md +++ b/tools/skills_tool.py.dox.md @@ -29,11 +29,11 @@ - `load` includes a `skill_instructions` metadata sidecar in the tool-result content with skill name, path, revision, source, and whether full content was included. - Duplicate `load` calls may omit the full body only when the same skill revision is still present in model-visible `history.output()` content. - Observed side-effect areas: filesystem reads, filesystem deletion, settings/state persistence, chat history content. -- Imported dependency areas include: `__future__`, `hashlib`, `helpers`, `helpers.print_style`, `helpers.tool`, `pathlib`, `typing`. +- 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`, `skills_helper.load_skill_for_agent`, `hashlib.sha256`, `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.skill_revision`, `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`. - Keep request/response, tool, or helper semantics documented here at the same time as source changes. ## Work Guidance