diff --git a/agent/conversation_loop.py b/agent/conversation_loop.py index 411134b41a..6eb554d965 100644 --- a/agent/conversation_loop.py +++ b/agent/conversation_loop.py @@ -34,6 +34,9 @@ from agent.runtime_cwd import resolve_agent_cwd from agent.surface_switch import ( identity_line_value, note_inert_pinned_tools, runtime_host_value, stage_surface_switch_note, ) +from agent.skills_index_delta import ( # noqa: F401 — patch seam shared with the tests + stage_skills_index_note, +) from agent.turn_context import PreflightCompressionTimedOut, build_turn_context from hermes_cli.observability.shared_metrics_efficiency import record_cache_break, record_prompt_rebuild from agent.turn_retry_state import TurnRetryState @@ -798,6 +801,9 @@ def _restore_or_build_system_prompt(agent, system_message, conversation_history) # The reused bytes may describe the surface this conversation STARTED on; correct that # at the tail of the request instead of rebuilding the prompt in front of it (#104414). announced_switch = stage_surface_switch_note(agent, stored_prompt, conversation_history) + # Same channel for a skills index that drifted since the prompt was built (a skill + # installed or removed mid-conversation); the index bytes stay, the delta rides behind them. + stage_skills_index_note(agent, stored_prompt, conversation_history) # Same contract for tools[]: pin the array to the order this session already # sent (tools freeze) instead of re-probing every check_fn on a fresh AIAgent. # The pin holds ON the announcing turn too. tools[] is serialized AHEAD of the system diff --git a/agent/skills_index_delta.py b/agent/skills_index_delta.py new file mode 100644 index 0000000000..e0bdac763b --- /dev/null +++ b/agent/skills_index_delta.py @@ -0,0 +1,130 @@ +"""Skills index drift without a prompt rebuild. + +The ```` routing index is embedded in the persisted system prompt, and a +continuing session reuses those bytes verbatim so the provider prefix cache stays warm (#104414). +A skill that appears or disappears mid-conversation (a hub install, ``skill_manage`` from another +session, an org sync, the curator) therefore never reaches the index the model actually routes on +until compaction rebuilds the prompt; ``skills_list`` is live, but the prompt tells the model to +scan the index, not to call it. The delta is delivered on the per-turn user-message channel +instead: it lands behind the cached prefix, is stamped into the byte-stable ``api_content`` +sidecar, and is re-staged only when the delta itself changes (ported from +cloudflare/cloudflare-os#267, whose chat cached its skill catalog once and never saw a new skill). +""" +from __future__ import annotations + +import logging +import re +from typing import Any, Dict, Set, Tuple + +from agent.message_content import flatten_message_text + +logger = logging.getLogger("run_agent") + +_SKILLS_INDEX_NOTE_PREFIX = "[System: The skills index in the system prompt is out of date for this conversation." +_ADDED_HEAD = "Skills added since it was built (load with skill_view(name) when relevant, like the index above):" +_REMOVED_HEAD = "Skills no longer available:" +_NOTE_END = "]" +_NOTE_SCAN_TAIL = 200 + +_BLOCK_RE = re.compile(r"\n?(.*?)", re.DOTALL) +_ENTRY_RE = re.compile(r"^\s*- ([^:\n]+?)(?::|$)", re.MULTILINE) +_NAMES_ONLY_RE = re.compile(r"\[names only\]:\s*(.+)$", re.MULTILINE) + + +def index_entries(prompt: str) -> Dict[str, str]: + """``{name: index line}`` for every skill the ```` block of ``prompt`` lists. + + Names-only category lines (focus mode) contribute the name with an empty line; ``{}`` when + the prompt has no block at all. + """ + m = _BLOCK_RE.search(prompt or "") + if not m: + return {} + body = m.group(1) + entries: Dict[str, str] = {} + for line in body.splitlines(): + em = _ENTRY_RE.match(line) + if em: + entries.setdefault(em.group(1).strip(), line.rstrip()) + continue + nm = _NAMES_ONLY_RE.search(line) + if nm: + for name in nm.group(1).split(","): + if name.strip(): + entries.setdefault(name.strip(), "") + return entries + + +def _last_announced_delta(conversation_history: Any) -> Tuple[Set[str], Set[str]]: + """``(added, removed)`` named by the NEWEST skills-index note in the transcript. + + Every note carries the FULL delta against the stored prompt, so the newest one is the + complete statement of what the model has been told; reading it back is what keeps a fresh + agent per turn (the gateway shape) from re-sending the same note every turn. + """ + for msg in reversed((conversation_history or [])[-_NOTE_SCAN_TAIL:]): + if not isinstance(msg, dict) or msg.get("role") != "user": + continue + sidecar = msg.get("api_content") + text = (sidecar if isinstance(sidecar, str) else "") + "\n" + flatten_message_text(msg.get("content")) + if _SKILLS_INDEX_NOTE_PREFIX not in text: + continue + note = text.rsplit(_SKILLS_INDEX_NOTE_PREFIX, 1)[1] + added: Set[str] = set() + removed: Set[str] = set() + if _ADDED_HEAD in note: + added_part = note.split(_ADDED_HEAD, 1)[1].split(_REMOVED_HEAD, 1)[0] + added = {m.group(1).strip() for m in _ENTRY_RE.finditer(added_part)} + if _REMOVED_HEAD in note: + removed_part = note.split(_REMOVED_HEAD, 1)[1].split(_NOTE_END, 1)[0] + removed = {n.strip() for n in removed_part.split(",") if n.strip()} + return added, removed + return set(), set() + + +def _render_note(added: Dict[str, str], removed: Set[str]) -> str: + parts = [_SKILLS_INDEX_NOTE_PREFIX] + if added: + parts.append(_ADDED_HEAD) + parts.extend(added[name] or f" - {name}" for name in sorted(added)) + if removed: + parts.append(f"{_REMOVED_HEAD} {', '.join(sorted(removed))}") + return "\n".join(parts) + _NOTE_END + + +def stage_skills_index_note(agent: Any, stored_prompt: str, conversation_history: Any) -> bool: + """Stage a one-shot note when the stored prompt's skills index no longer matches the skills + the session can load right now. Returns whether it staged. + + Only a prompt that HAS an index is compared (no baseline otherwise), and only against the + index the current build would emit — the same cached builder the prompt came from, so a turn + with no skill change costs one LRU hit. MoA and codex_app_server turns never stamp the + ``api_content`` sidecar, so the note could not be read back; those modes skip it. + """ + if getattr(agent, "provider", None) == "moa" or getattr(agent, "api_mode", None) == "codex_app_server": + return False + stored = index_entries(stored_prompt) + if not stored: + return False + try: + from agent.system_prompt import _skills_prompt + current = index_entries(_skills_prompt(agent)) + except Exception: + logger.debug("skills index delta: current index unavailable", exc_info=True) + return False + added = {name: line for name, line in current.items() if name not in stored} + removed = set(stored) - set(current) + if (set(added), removed) == _last_announced_delta(conversation_history): + return False + if not added and not removed: + # Told about a delta that has since been undone (the skill came back / went away again); + # the stored index is accurate again and the stale note has to be retired. + agent._skills_index_note = f"{_SKILLS_INDEX_NOTE_PREFIX} It is accurate again: earlier skills-index notes in this conversation are superseded.{_NOTE_END}" + return True + agent._skills_index_note = _render_note(added, removed) + logger.info( + "Session %s: skills index drifted (+%d/-%d) since the stored prompt was built; keeping the " + "prompt and delivering the delta as a turn note (prefix cache preserved).", + getattr(agent, "session_id", None), len(added), len(removed), + ) + return True diff --git a/agent/turn_context.py b/agent/turn_context.py index 32cfaeecd8..4814778e60 100644 --- a/agent/turn_context.py +++ b/agent/turn_context.py @@ -171,6 +171,11 @@ def consume_surface_switch_note(agent: Any) -> str: return _pop_turn_note(agent, "_surface_switch_note") +def consume_skills_index_note(agent: Any) -> str: + """Pop the skills-index delta staged by the system-prompt restore; same channel as above.""" + return _pop_turn_note(agent, "_skills_index_note") + + def append_notes_to_multimodal_content(content: Any, notes: Optional[str]) -> bool: """Append must-deliver notes as a durable text part on a multimodal (list) user message (the sidecar path returns ``None`` for non-string content).""" @@ -850,7 +855,8 @@ def _merge_gateway_notes( append a durable text part instead.""" _turn_notes = "\n\n".join( part for part in (consume_gateway_turn_context_notes(agent), - consume_surface_switch_note(agent)) if part + consume_surface_switch_note(agent), + consume_skills_index_note(agent)) if part ) if not _turn_notes: return plugin_user_context diff --git a/tests/agent/test_skills_index_delta.py b/tests/agent/test_skills_index_delta.py new file mode 100644 index 0000000000..b38ab6d2c0 --- /dev/null +++ b/tests/agent/test_skills_index_delta.py @@ -0,0 +1,76 @@ +"""A skill that appears or disappears mid-conversation reaches the model without a prompt rebuild. + +The persisted ```` index is reused byte-for-byte for the prefix cache, so the +delta is delivered on the per-turn note channel (cloudflare/cloudflare-os#267 port). +""" +from unittest.mock import MagicMock, patch + +from agent.skills_index_delta import ( + _ADDED_HEAD, _REMOVED_HEAD, _SKILLS_INDEX_NOTE_PREFIX, index_entries, stage_skills_index_note, +) + +_STORED = ( + "BODY\n\n## Skills\nintro\n\n\n" + " devops: ops tools\n" + " - docker-ops: Run and debug containers.\n" + " - k8s-ops: Kubernetes rollouts.\n" + " creative [names only]: ascii-art, manim-video\n" + "\n\nOnly proceed without loading a skill if none fit.\n\nPlatform: cli" +) +_CURRENT = _STORED.replace( + " - k8s-ops: Kubernetes rollouts.\n", + " - k8s-ops: Kubernetes rollouts.\n - terraform-ops: Plan and apply infra changes.\n", +).replace("ascii-art, manim-video", "manim-video") + + +def _agent(): + agent = MagicMock() + agent.provider = "openrouter" + agent.api_mode = "chat_completions" + agent._skills_index_note = "" + return agent + + +def _stage(agent, history, current=_CURRENT): + with patch("agent.system_prompt._skills_prompt", return_value=current): + return stage_skills_index_note(agent, _STORED, history) + + +def test_index_entries_reads_described_and_names_only_skills(): + assert set(index_entries(_STORED)) == {"docker-ops", "k8s-ops", "ascii-art", "manim-video"} + assert index_entries(_STORED)["docker-ops"] == " - docker-ops: Run and debug containers." + assert index_entries("no block here") == {} + + +def test_delta_is_announced_once_with_its_description_and_restaged_only_when_it_changes(): + agent = _agent() + assert _stage(agent, [{"role": "user", "content": "hi"}]) is True + note = agent._skills_index_note + assert note.startswith(_SKILLS_INDEX_NOTE_PREFIX) + assert " - terraform-ops: Plan and apply infra changes." in note # the routing signal, not just a name + assert f"{_REMOVED_HEAD} ascii-art]" in note + assert "docker-ops" not in note.split(_ADDED_HEAD, 1)[1] # unchanged skills are not re-listed + + # Gateway shape: a fresh agent next turn, the transcript already carries this exact delta. + carried = [{"role": "user", "content": "hi", "api_content": f"hi\n\n{note}"}, {"role": "assistant", "content": "ok"}] + again = _agent() + assert _stage(again, carried) is False + assert again._skills_index_note == "" + + # The delta grows (another skill lands): the cumulative note is re-staged. + grown = _CURRENT.replace("", " - ansible-ops: Configuration runs.\n") + third = _agent() + assert _stage(third, carried, current=grown) is True + assert "ansible-ops" in third._skills_index_note and "terraform-ops" in third._skills_index_note + + # Undone (index accurate again): the stale note is retired, then nothing more is staged. + fourth = _agent() + assert _stage(fourth, carried, current=_STORED) is True + assert "accurate again" in fourth._skills_index_note + retired = carried + [{"role": "user", "content": "x", "api_content": f"x\n\n{fourth._skills_index_note}"}] + fifth = _agent() + assert _stage(fifth, retired, current=_STORED) is False + + # A prompt without an index has no baseline; an unchanged index stages nothing. + assert stage_skills_index_note(_agent(), "no skills block", []) is False + assert _stage(_agent(), [], current=_STORED) is False diff --git a/website/docs/developer-guide/prompt-assembly.md b/website/docs/developer-guide/prompt-assembly.md index a7482ad24e..19c80daf7d 100644 --- a/website/docs/developer-guide/prompt-assembly.md +++ b/website/docs/developer-guide/prompt-assembly.md @@ -52,7 +52,11 @@ The runtime boundary follows all project, operator, memory and plugin text, so e blocks do not masquerade as the runtime cwd. Model/provider are read before the runtime boundary, excluding embedder descriptions. `Platform:` is deliberately not an identity field: a surface switch (desktop ↔ TUI) keeps the stored bytes and delivers the current surface's guidance as a one-shot note on -the per-turn user-message channel (`agent/surface_switch.py`), so the cached prefix survives (#104414). Legacy prompts retain their original +the per-turn user-message channel (`agent/surface_switch.py`), so the cached prefix survives (#104414). The +`` index is kept the same way: when a skill is installed, created or removed after the +prompt was built, the stored bytes stay and the delta (new index lines, removed names) rides the same +one-shot note channel (`agent/skills_index_delta.py`), re-sent only when the delta changes; the index +itself converges at the next rebuild boundary (compaction). Legacy prompts retain their original host-before-context anchor, so prompts persisted before the reorder still validate. When `skip_context_files` is set (e.g., subagent delegation), SOUL.md is not loaded and the hardcoded `DEFAULT_AGENT_IDENTITY` is used instead.