diff --git a/agent/skill_commands.py b/agent/skill_commands.py index 294ca2b17543..3f1156a85929 100644 --- a/agent/skill_commands.py +++ b/agent/skill_commands.py @@ -97,7 +97,7 @@ def extract_user_instruction_from_skill_message(content: Any) -> Optional[str]: return None -def describe_skill_invocation(content: Any) -> Optional[str]: +def describe_skill_invocation(content: Any, separator: str = " — ") -> Optional[str]: """Render a slash-skill-expanded turn the way the user typed it. The expanded message embeds the whole skill body, so any surface that @@ -109,6 +109,10 @@ def describe_skill_invocation(content: Any) -> Optional[str]: Returns ``"/work — fix the title leak"``, or ``"/work"`` for a bare invocation, or ``None`` when *content* is not skill scaffolding (the caller should then summarize it as an ordinary message). + + *separator* joins the command and the instruction. Previews use the + default em dash; pass ``" "`` for the literal invocation the user typed, + which is what chat transcripts render. """ if not isinstance(content, str) or not content.startswith(_SKILL_INVOCATION_PREFIX): return None @@ -127,7 +131,7 @@ def describe_skill_invocation(content: Any) -> Optional[str]: instruction = instruction.split(SKILL_EXCERPT_JOINT)[0] instruction = " ".join(instruction.split()) if instruction: - return f"{label} — {instruction}" if name else instruction + return f"{label}{separator}{instruction}" if name else instruction return label if name else None diff --git a/tests/test_tui_gateway_server.py b/tests/test_tui_gateway_server.py index 7ba25acc55c8..849c3397ed72 100644 --- a/tests/test_tui_gateway_server.py +++ b/tests/test_tui_gateway_server.py @@ -1753,6 +1753,93 @@ def test_history_to_messages_drops_display_hidden_scaffolding(): assert all("api_content" not in m for m in projected) +def test_history_to_messages_projects_a_skill_turn_to_its_invocation(): + # A /skill invocation is persisted EXPANDED: the activation note plus the + # entire skill body. That payload is model-facing scaffolding -- this + # projection is the single display source every client reads, so it must + # hand back the invocation the user typed and never the body. Without it a + # chat bubble renders the whole skill as if the user had written it. + scaffolded = ( + '[IMPORTANT: The user has invoked the "work" skill, indicating they ' + "want you to follow its instructions. The full skill content is " + "loaded below.]\n\n" + "# /work\n\nSPIN UP A WORKTREE, never the primary checkout.\n\n" + "The user has provided the following instruction alongside the skill " + "invocation: fix the title leak" + ) + + history = [ + {"role": "user", "content": scaffolded}, + {"role": "assistant", "content": "on it"}, + ] + + assert server._history_to_messages(history) == [ + { + "role": "user", + "text": "/work fix the title leak", + "display_kind": "skill_invocation", + }, + {"role": "assistant", "text": "on it"}, + ] + + +def test_history_to_messages_projects_a_bare_skill_turn_to_the_command(): + scaffolded = ( + '[IMPORTANT: The user has invoked the "work" skill, indicating they ' + "want you to follow its instructions. The full skill content is " + "loaded below.]\n\n# /work\n\nSPIN UP A WORKTREE." + ) + + assert server._history_to_messages([{"role": "user", "content": scaffolded}]) == [ + {"role": "user", "text": "/work", "display_kind": "skill_invocation"} + ] + + +def test_expand_skill_invocation_for_replay_round_trips_the_projection( + tmp_path, monkeypatch +): + # Rewind/regenerate replays a turn from what the transcript SHOWS, and a + # skill turn shows its invocation. Re-running that verbatim would send the + # agent the literal "/work fix it" instead of the skill, so the server + # re-expands it — the exact inverse of _skill_scaffold_projection, with the + # body never leaving the server. + import agent.skill_commands as skill_commands + import agent.skill_utils as skill_utils + import tools.skills_tool as skills_tool + + skills_dir = tmp_path / "skills" + (skills_dir / "worktree-kickoff").mkdir(parents=True) + (skills_dir / "worktree-kickoff" / "SKILL.md").write_text( + "---\nname: worktree-kickoff\ndescription: Spin up a worktree\n---\n\n" + "# kickoff\n\nSPIN UP A WORKTREE, never the primary checkout.\n" + ) + monkeypatch.setattr(skills_tool, "SKILLS_DIR", skills_dir) + monkeypatch.setattr(skill_utils, "get_external_skills_dirs", lambda *a, **k: []) + monkeypatch.setattr(skill_commands, "_skill_commands", {}) + monkeypatch.setattr(skill_commands, "_skill_commands_platform", None) + skill_commands.scan_skill_commands() + + expanded = server._expand_skill_invocation_for_replay( + "/worktree-kickoff fix it", "task-1" + ) + + assert "SPIN UP A WORKTREE" in expanded + assert server._skill_scaffold_projection(expanded) == "/worktree-kickoff fix it" + + +def test_expand_skill_invocation_for_replay_leaves_ordinary_text_alone(monkeypatch): + import agent.skill_commands as skill_commands + import agent.skill_utils as skill_utils + + monkeypatch.setattr(skill_utils, "get_external_skills_dirs", lambda *a, **k: []) + monkeypatch.setattr(skill_commands, "_skill_commands", {}) + monkeypatch.setattr(skill_commands, "_skill_commands_platform", None) + + assert server._expand_skill_invocation_for_replay("just words", "t") == "just words" + # A core slash command is not a skill — nothing to expand. + assert server._expand_skill_invocation_for_replay("/status", "t") == "/status" + + def test_history_to_messages_keeps_real_user_bracket_text(): # Only role=user rows whose text OPENS with the [System: marker sentinel are # bookkeeping notices. A genuine user turn that merely mentions the token is diff --git a/tui_gateway/server.py b/tui_gateway/server.py index 14228035c434..9e78b64e8e42 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -33,6 +33,7 @@ from hermes_cli.env_loader import load_hermes_dotenv from utils import is_truthy_value from tools.environments.local import hermes_subprocess_env from agent.replay_cleanup import sanitize_replay_history +from agent.skill_commands import describe_skill_invocation from agent.conversation_loop import INTERRUPT_WAITING_FOR_MODEL_PREFIX from tui_gateway import git_probe from tui_gateway.turn_marker import ( @@ -6237,6 +6238,51 @@ def _is_display_hidden_marker(role: str | None, text: str) -> bool: return role == "user" and text.lstrip().startswith("[System:") +def _skill_scaffold_projection(content_text: str) -> str: + """Return the invocation a slash-skill-expanded turn came from, else "". + + A ``/skill`` invocation expands into a model-facing message that embeds the + whole skill body. That payload belongs to the agent — every UI renders the + invocation (``/work fix the leak``) instead, so no surface can leak the + body into a chat bubble. + """ + return describe_skill_invocation(content_text, separator=" ") or "" + + +def _expand_skill_invocation_for_replay(text: str, task_id: str) -> str: + """Re-expand a projected `/skill` invocation before re-running that turn. + + The inverse of :func:`_skill_scaffold_projection`. Because a skill turn is + displayed as its invocation, a rewind/regenerate hands us back + ``/work fix the leak`` rather than the body the agent originally saw — + re-running that verbatim would drop the skill. Re-expanding here keeps the + body server-side (no client ever holds it) and makes the replayed turn + identical to the original. + + Returns *text* unchanged when it isn't a resolvable skill invocation. + """ + head, _, arg = (text or "").strip().partition(" ") + if not head.startswith("/"): + return text + + try: + from agent.skill_commands import ( + build_skill_invocation_message, + resolve_skill_command_key, + ) + + cmd_key = resolve_skill_command_key(head.lstrip("/")) + if cmd_key is None: + return text + + return build_skill_invocation_message(cmd_key, arg.strip(), task_id=task_id) or text + except Exception: + # A skill that no longer resolves (renamed, disabled, external dir + # gone) must not break the rewind — replay the text as typed. + logger.debug("skill re-expansion failed for replay", exc_info=True) + return text + + def _history_to_messages(history: list[dict]) -> list[dict]: messages = [] tool_call_args = {} @@ -6297,6 +6343,14 @@ def _history_to_messages(history: list[dict]) -> list[dict]: if not content_text.strip() and not has_reasoning: continue msg = {"role": role, "text": content_text} + if role == "user": + invocation = _skill_scaffold_projection(content_text) + if invocation: + # Show the invocation, never the expanded skill body. The raw + # payload stays server-side: a rewind/regenerate re-sends the + # turn by ordinal, so no client needs it. + msg["text"] = invocation + msg["display_kind"] = "skill_invocation" if role == "assistant": for key in reasoning_keys: if key in m and m.get(key) is not None: @@ -10842,6 +10896,14 @@ def _(rid, params: dict) -> dict: session, err = _sess_nowait(params, rid) if err: return err + if truncate_user_ordinal is not None and isinstance(text, str): + # A rewind/regenerate replays a turn from what the transcript shows. A + # skill turn shows its invocation, so re-expand it here — otherwise + # re-running `/work fix it` sends the agent nine literal characters + # instead of the skill it originally loaded. + text = _expand_skill_invocation_for_replay( + text, str(session.get("session_key") or "") + ) isolation_cfg = _load_dashboard_process_isolation_config() turn_isolation = _session_uses_compute_host(session, isolation_cfg) # Re-bind to the current client transport for this request. This keeps @@ -15450,6 +15512,9 @@ def _(rid, params: dict) -> dict: "type": "send", "message": msg, "notice": notice, + # UIs render this, never `message` — the expanded bundle body + # is model-facing scaffolding (see _skill_scaffold_projection). + "display": _skill_scaffold_projection(msg), }, ) @@ -15472,6 +15537,9 @@ def _(rid, params: dict) -> dict: "type": "skill", "message": msg, "name": cmds[key].get("name", name), + # UIs render this, never `message` — the expanded skill + # body is model-facing scaffolding. + "display": _skill_scaffold_projection(msg), }, ) except Exception: