diff --git a/docs/guides/installing-loopx.md b/docs/guides/installing-loopx.md index a1f6a920d..b56f3a2a7 100644 --- a/docs/guides/installing-loopx.md +++ b/docs/guides/installing-loopx.md @@ -140,10 +140,20 @@ one. Host integration changes command discovery only. It does not grant LoopX permission to write a repository, contact external systems, or bypass a user gate. -Codex installs expose only canonical `loopx-*` skills. Older managed -`loop-global-*` skill aliases are retired; their catalog entries and native -slash-host compatibility remain available. This changes the Codex picker, -not goal execution or write authority. +Every host skill root exposes only canonical `loopx-*` skills. Install and +uninstall retire older LoopX-managed `loop-global-*` skill files; unmarked +user-owned files are preserved and reported. This prevents cross-host imports +from republishing a deprecated facade beside its canonical outcome. + +**Invocation migration:** on Claude Code and Kiro, the skill is also the slash +command, so use `/loopx-global-summary`, `/loopx-global-gates`, +`/loopx-global-todos` and `/loopx-global-risks` instead of `/loop-global-*`. +Other skill-backed hosts likewise expose only the canonical names. OpenCode +alone retains independently installed `commands/loop-global-*.md` when legacy +aliases are enabled. `--no-legacy-aliases` omits those command files from new +installs; it does not retire existing native command files. The command catalog +still describes aliases, but a catalog row does not install a host invocation. +Goal execution and write authority are unchanged. Both workflow and command installation reconcile managed duplicates between `CODEX_HOME/skills` (default `~/.codex/skills`) and `~/.agents/skills`. diff --git a/examples/slash-command-install-smoke.py b/examples/slash-command-install-smoke.py index 44e3295fd..f38b66401 100644 --- a/examples/slash-command-install-smoke.py +++ b/examples/slash-command-install-smoke.py @@ -152,6 +152,37 @@ def main() -> int: assert "without mutating state" in claude_risks_text assert "This command is read-only" in claude_risks_text assert "global-summary" not in claude_risks_text + assert not (claude_home / "skills" / "loop-global-risks").exists() + + # A deprecated alias skill republished by an older install (or copied + # back in by a cross-host import) is retired on the next install, while + # a user-owned same-name skill survives untouched. + managed_alias = claude_home / "skills" / "loop-global-summary" / "SKILL.md" + managed_alias.parent.mkdir(parents=True) + managed_alias.write_text( + "\n" + "# LoopX /loop-global-summary\n", + encoding="utf-8", + ) + user_alias = claude_home / "skills" / "loop-global-risks" / "SKILL.md" + user_alias.parent.mkdir(parents=True) + user_alias.write_text("# user-owned alias skill\n", encoding="utf-8") + alias_retire = json.loads( + run_cli( + "--format", + "json", + "slash-commands", + "--install", + "--codex-home", + str(codex_home), + "--claude-home", + str(claude_home), + ).stdout + ) + assert statuses_for(alias_retire, managed_alias) == ["retired_managed_file"], alias_retire + assert not managed_alias.exists() + assert statuses_for(alias_retire, user_alias) == ["skipped_user_file"], alias_retire + assert user_alias.read_text(encoding="utf-8") == "# user-owned alias skill\n" rerun = json.loads( run_cli( diff --git a/loopx/slash_command_files.py b/loopx/slash_command_files.py index a39cc1709..9245dbe7f 100644 --- a/loopx/slash_command_files.py +++ b/loopx/slash_command_files.py @@ -1,7 +1,7 @@ from __future__ import annotations from pathlib import Path -from typing import Any +from typing import Any, NotRequired, TypedDict MANAGED_MARKER_PREFIX = "" @@ -131,7 +141,7 @@ def retire_status(path: Path, *, execute: bool) -> str: def install_skill_facade( *, - specs: list[dict[str, Any]], + specs: list[CommandFacadeSpec], installed: list[dict[str, Any]], skills_dir: Path, surface: str, @@ -142,13 +152,27 @@ def install_skill_facade( invoke_prefix: str = "", flat: bool = False, ) -> None: - """Write managed command facades in directory or flat host layouts.""" + """Install canonical skills and retire catalog aliases in every layout.""" for spec in specs: path = ( skills_dir / f"{spec['name']}.md" if flat else skills_dir / str(spec["name"]) / "SKILL.md" ) + if "alias_for" in spec: + status = retire_managed_file(path, execute=execute) + if status: + installed.append({ + "surface": surface, + "host_surfaces": list(host_surfaces), + "mechanism": f"retired_{surface.replace('-', '_')}_legacy_alias", + "command": spec["command"], + "path": str(path), + "status": status, + "invoke_as": [], + "replacement_command": spec["alias_for"], + }) + continue if uninstall: installed.append( { diff --git a/loopx/slash_command_install.py b/loopx/slash_command_install.py index 28831c856..1b2b2c48a 100644 --- a/loopx/slash_command_install.py +++ b/loopx/slash_command_install.py @@ -25,6 +25,7 @@ _pi_runtime_path, ) from .slash_command_files import ( + CommandFacadeSpec, front_matter as _front_matter, install_skill_facade as _install_skill_facade, managed_marker as _managed_marker, @@ -58,7 +59,7 @@ def _openai_skill_metadata(*, command: str, display_name: str, short_description ) -def _opencode_command_body(spec: dict[str, Any]) -> str: +def _opencode_command_body(spec: CommandFacadeSpec) -> str: return "\n\n".join( [ _front_matter( @@ -170,8 +171,8 @@ def _dsh_native_loopx_instructions(*, cli_bin: str) -> list[str]: ] -def _command_prompt_specs(*, cli_bin: str, include_legacy_aliases: bool) -> list[dict[str, Any]]: - specs: list[dict[str, Any]] = [ +def _command_prompt_specs(*, cli_bin: str, include_legacy_aliases: bool) -> list[CommandFacadeSpec]: + specs: list[CommandFacadeSpec] = [ { "command": "/loopx", "name": "loopx", @@ -273,25 +274,23 @@ def _command_prompt_specs(*, cli_bin: str, include_legacy_aliases: bool) -> list }, ] if include_legacy_aliases: - legacy_specs = [] + catalog = build_slash_command_catalog(cli_bin=cli_bin, include_legacy_aliases=True) + aliases = {row["command"]: row.get("legacy_aliases", []) for row in catalog["commands"]} + legacy_specs: list[CommandFacadeSpec] = [] for canonical in specs: - name = canonical["name"] - if not str(name).startswith("loopx-global-"): - continue - legacy_name = str(name).replace("loopx-global-", "loop-global-", 1) - legacy_specs.append( - { + for alias in aliases.get(canonical["command"], []): + legacy_specs.append({ **canonical, - "command": "/" + legacy_name, - "name": legacy_name, - "description": canonical["description"] + " Legacy alias for the canonical /loopx-global-* command.", - } - ) + "command": alias, + "name": alias.removeprefix("/"), + "description": canonical["description"] + f" Legacy alias for {canonical['command']}.", + "alias_for": canonical["command"], + }) specs.extend(legacy_specs) return specs -def _command_skill_content(spec: dict[str, Any], *, surface: str) -> str: +def _command_skill_content(spec: CommandFacadeSpec, *, surface: str) -> str: instructions = list(spec["instructions"]) if surface == "codex-skills": instructions.insert( @@ -537,9 +536,9 @@ def _opencode_direct_goal_plugin_conflicts(root: Path) -> tuple[list[str], list[ invalid.append(str(path)) continue plugin_names = [ - name + plugin_name for plugin in plugins - if (name := _opencode_plugin_name(plugin)) is not None + if (plugin_name := _opencode_plugin_name(plugin)) is not None ] if any( plugin == package or plugin.startswith(f"{package}@") @@ -824,7 +823,10 @@ def install_slash_commands( pi_scope: str = "project", pi_user_home: str | None = None, ) -> dict[str, Any]: - specs = _command_prompt_specs(cli_bin=cli_bin, include_legacy_aliases=include_legacy_aliases) + facade_specs = _command_prompt_specs(cli_bin=cli_bin, include_legacy_aliases=True) + canonical_specs = [spec for spec in facade_specs if "alias_for" not in spec] + legacy_alias_specs = [spec for spec in facade_specs if "alias_for" in spec] + specs = facade_specs if include_legacy_aliases else canonical_specs effective_surfaces = _normalize_surfaces(surfaces) codex_root = _codex_home(codex_home) claude_root = _claude_home(claude_home) @@ -859,11 +861,10 @@ def install_slash_commands( codex_reconciliation = None if "codex" in effective_surfaces: - # Keep aliases in the catalog and native slash hosts, but expose one - # canonical skill per outcome in Codex's skill picker. - codex_specs = _command_prompt_specs(cli_bin=cli_bin, include_legacy_aliases=False) - legacy_specs = [s for s in _command_prompt_specs(cli_bin=cli_bin, include_legacy_aliases=True) - if str(s["name"]).startswith("loop-global-")] + # Codex exposes canonical skills only; retire its older managed facade, + # metadata and custom-prompt paths without creating replacements. + codex_specs = canonical_specs + legacy_specs = legacy_alias_specs for spec in legacy_specs: for path in (codex_root / "skills" / spec["name"] / "SKILL.md", codex_root / "skills" / spec["name"] / "agents" / "openai.yaml", @@ -877,7 +878,7 @@ def install_slash_commands( for spec in codex_specs: prompt_path = prompt_dir / f"{spec['name']}.md" if uninstall: - retire_status = _retire_status(prompt_path, execute=execute) + retire_status: str | None = _retire_status(prompt_path, execute=execute) installed.append( { "surface": "codex", @@ -1013,42 +1014,17 @@ def install_slash_commands( ) if "claude-code" in effective_surfaces: - skills_dir = claude_root / "skills" - for spec in specs: - path = skills_dir / str(spec["name"]) / "SKILL.md" - if uninstall: - status = _retire_status(path, execute=execute) - installed.append( - { - "surface": "claude-code", - "mechanism": "claude_code_skills", - "command": spec["command"], - "path": str(path), - "status": status, - "invoke_as": [str(spec["command"])], - } - ) - continue - content = _skill_body( - command=str(spec["command"]), - title=f"LoopX {spec['command']}", - description=str(spec["description"]), - argument_hint=str(spec["argument_hint"]), - instructions=list(spec["instructions"]), - surface="claude-skills", - front_matter_name=str(spec["name"]), - ) - status = _target_status(path, content, execute=execute) - installed.append( - { - "surface": "claude-code", - "mechanism": "claude_code_skills", - "command": spec["command"], - "path": str(path), - "status": status, - "invoke_as": [str(spec["command"])], - } - ) + _install_skill_facade( + specs=facade_specs, + installed=installed, + skills_dir=claude_root / "skills", + surface="claude-code", + host_surfaces=["claude-code"], + mechanism="claude_code_skills", + execute=execute, + uninstall=uninstall, + invoke_prefix="/", + ) if "gemini" in effective_surfaces: # Gemini CLI discovers user skills from GEMINI_HOME/skills. Files are @@ -1058,7 +1034,7 @@ def install_slash_commands( # status and the dry run that every other surface reports — and it would # need the `gemini` binary on PATH to install a file it already has. _install_skill_facade( - specs=specs, + specs=facade_specs, installed=installed, skills_dir=gemini_root / "skills", surface="gemini", @@ -1076,7 +1052,7 @@ def install_slash_commands( # and the root belongs to agy alone (Gemini CLI reads ~/.gemini/skills), # so the managed skill surfaces never collide across different hosts. _install_skill_facade( - specs=specs, + specs=facade_specs, installed=installed, skills_dir=agy_root / "skills", surface="agy", @@ -1097,7 +1073,7 @@ def install_slash_commands( # .kiro/prompts wins over a skill by Kiro's own resolution order; the # installer never touches the prompt directories. _install_skill_facade( - specs=specs, + specs=facade_specs, installed=installed, skills_dir=kiro_root / "skills", surface="kiro-cli", @@ -1136,7 +1112,7 @@ def install_slash_commands( # include .claude/skills and .codex/skills, but relying on another # host's directory would break the moment that host is uninstalled). _install_skill_facade( - specs=specs, + specs=facade_specs, installed=installed, skills_dir=cursor_root / "skills", surface="cursor", @@ -1172,7 +1148,7 @@ def install_slash_commands( if "zcode" in effective_surfaces: # ZCode discovers user skills from ZCODE_HOME/skills (default ~/.zcode/skills). _install_skill_facade( - specs=specs, + specs=facade_specs, installed=installed, skills_dir=zcode_root / "skills", surface="zcode", @@ -1188,7 +1164,7 @@ def install_slash_commands( # static command facade below stays as it is — a command is something # the user types, a skill is something the model can reach for itself. _install_skill_facade( - specs=specs, + specs=facade_specs, installed=installed, skills_dir=opencode_root / "skills", surface="opencode", @@ -1507,6 +1483,7 @@ def install_slash_commands( "codex_skill_reconciliation": codex_reconciliation, "notes": [ "Codex does not currently support user-defined native top-level slash commands; use explicit skill invocation through `$loopx` or `/skills`.", + "Every host skill root installs canonical facades only and retires managed /loop-global-* alias skills. Use /loopx-global-* instead on skill-backed slash hosts, including Claude Code and Kiro. Only OpenCode retains independently installed native alias command files when legacy aliases are enabled; catalog aliases remain available.", "Explicit LoopX command-facade skills use agents/openai.yaml policy allow_implicit_invocation=false and remain distinct from richer workflow skills such as loopx-project.", "Claude Code discovers user skills from CLAUDE_HOME/skills and exposes each skill name as a slash command.", "Gemini CLI discovers user skills from GEMINI_HOME/skills with the same SKILL.md front matter; files are written directly because `gemini skills install` copies from a git URL or an existing local path and hands the copy to the host, which would lose the managed marker, per-file status and dry-run reporting every other surface has.", diff --git a/tests/test_skill_discovery_reconciliation.py b/tests/test_skill_discovery_reconciliation.py index 923f6eb41..c6bb73eb4 100644 --- a/tests/test_skill_discovery_reconciliation.py +++ b/tests/test_skill_discovery_reconciliation.py @@ -145,9 +145,15 @@ def test_custom_home_does_not_touch_another_codex_profile(roots): assert not (alternate / "loopx").exists() -def test_user_alias_is_preserved_and_native_host_aliases_still_work(tmp_path): +def test_user_alias_is_preserved_and_deprecated_host_aliases_are_retired(tmp_path): + """A user-owned alias skill is never touched, and the deprecated managed + alias is not republished to another host root: a cross-host import would + otherwise copy it into the shared ~/.agents/skills root next to the + canonical facade and make one outcome resolve twice.""" + codex, claude = tmp_path / "codex", tmp_path / "claude" custom = skill(codex / "skills", "loop-global-summary", "User skill") + deprecated = skill(claude / "skills", "loop-global-summary") install_slash_commands( execute=True, surfaces=["codex", "claude-code"], @@ -155,7 +161,8 @@ def test_user_alias_is_preserved_and_native_host_aliases_still_work(tmp_path): claude_home=str(claude), ) assert custom.read_text() == "User skill" - assert (claude / "skills/loop-global-summary/SKILL.md").exists() + assert not deprecated.exists() + assert (claude / "skills/loopx-global-summary/SKILL.md").exists() def test_retirement_updates_receipt_without_blessing_modified_survivor(tmp_path): diff --git a/tests/test_slash_command_install.py b/tests/test_slash_command_install.py index 605b9ad9b..76310c02b 100644 --- a/tests/test_slash_command_install.py +++ b/tests/test_slash_command_install.py @@ -323,12 +323,146 @@ def test_claude_install_routes_global_risks_to_focused_cli(tmp_path: Path) -> No "boundary warnings, failing checks, and whether a formally evidenced " "rollback candidate source is available, without mutating state." ) - for name_skill in ("loopx-global-risks", "loop-global-risks"): - skill = claude_home / "skills" / name_skill / "SKILL.md" - skill_text = skill.read_text(encoding="utf-8") - assert expected in skill_text - assert "This command is read-only" in skill_text - assert "global-summary" not in skill_text + skill = claude_home / "skills" / "loopx-global-risks" / "SKILL.md" + skill_text = skill.read_text(encoding="utf-8") + assert expected in skill_text + assert "This command is read-only" in skill_text + assert "global-summary" not in skill_text + # One canonical facade per outcome: the deprecated alias skill file is never + # reinstalled into a host root, so a host import cannot copy it elsewhere. + assert not (claude_home / "skills" / "loop-global-risks").exists() + + +def test_install_retires_managed_alias_facades_on_every_host_root( + tmp_path: Path, +) -> None: + """A managed /loop-global-* file is retired; a user-owned same-name skill is + preserved. The deprecated facade used to be republished to Claude Code and + OpenCode, then imported into ~/.agents/skills as a second copy of an + outcome whose canonical facade already exists.""" + + marker = "\n" + claude_home = tmp_path / "claude" + opencode_home = tmp_path / "opencode" + managed = claude_home / "skills" / "loop-global-summary" / "SKILL.md" + managed.parent.mkdir(parents=True) + managed.write_text(marker + "\n# LoopX /loop-global-summary\nold\n", encoding="utf-8") + user_owned = opencode_home / "skills" / "loop-global-summary" / "SKILL.md" + user_owned.parent.mkdir(parents=True) + user_owned.write_text("user-owned skill body\n", encoding="utf-8") + + payload = install_slash_commands( + execute=True, + surfaces=["claude-code", "opencode"], + claude_home=str(claude_home), + opencode_home=str(opencode_home), + ) + + rows = { + (item["surface"], item["command"]): item + for item in payload["installed"] + if item["command"] == "/loop-global-summary" + and str(item["mechanism"]).startswith("retired_") + } + assert not managed.exists() + assert rows[("claude-code", "/loop-global-summary")]["mechanism"] == ( + "retired_claude_code_legacy_alias" + ) + assert rows[("claude-code", "/loop-global-summary")]["status"] == "retired_managed_file" + assert user_owned.read_text(encoding="utf-8") == "user-owned skill body\n" + assert rows[("opencode", "/loop-global-summary")]["status"] == "skipped_user_file" + assert (claude_home / "skills" / "loopx-global-summary" / "SKILL.md").is_file() + assert (opencode_home / "skills" / "loopx-global-summary" / "SKILL.md").is_file() + + +def test_legacy_alias_retirement_keeps_native_slash_commands( + tmp_path: Path, +) -> None: + """OpenCode keeps the alias as a typed command; only the skill file that a + host import could copy into a shared root is retired.""" + + opencode_home = tmp_path / "opencode" + install_slash_commands( + execute=True, + surfaces=["opencode"], + opencode_home=str(opencode_home), + ) + + assert (opencode_home / "commands" / "loop-global-summary.md").is_file() + assert not (opencode_home / "skills" / "loop-global-summary").exists() + assert (opencode_home / "skills" / "loopx-global-summary" / "SKILL.md").is_file() + + +@pytest.mark.parametrize("surface,home_option,flat", [ + ("codex", "codex_home", False), + ("claude-code", "claude_home", False), + ("gemini", "gemini_home", False), + ("agy", "agy_home", True), + ("kiro-cli", "kiro_home", False), + ("cursor", "cursor_home", False), + ("zcode", "zcode_home", False), + ("opencode", "opencode_home", False), +]) +@pytest.mark.parametrize("uninstall", [False, True]) +def test_alias_retirement_is_owned_and_repeatable_across_hosts( + tmp_path, monkeypatch, surface, home_option, flat, uninstall, +): + monkeypatch.setattr(slash_command_install, "_provisioned_mcp_interpreter", lambda: None) + root = tmp_path / surface + skills = root / "skills" + managed = (skills / "loop-global-summary.md" if flat else + skills / "loop-global-summary" / "SKILL.md") + user = (skills / "loop-global-risks.md" if flat else + skills / "loop-global-risks" / "SKILL.md") + managed.parent.mkdir(parents=True) + user.parent.mkdir(parents=True, exist_ok=True) + managed.write_text(MANAGED_SKILL + "old alias\n") + user.write_text("user-owned skill\n") + options = {home_option: str(root)} + preview = install_slash_commands( + execute=False, surfaces=[surface], uninstall=uninstall, + include_legacy_aliases=False, **options, + ) + assert managed.read_text() == MANAGED_SKILL + "old alias\n" + assert user.read_text() == "user-owned skill\n" + assert next(r for r in preview["installed"] if r["path"] == str(managed))["status"] == "would_retire_managed_file" + for _ in range(2): + actual = install_slash_commands( + execute=True, surfaces=[surface], uninstall=uninstall, + include_legacy_aliases=False, **options, + ) + assert not managed.exists() + assert user.read_text() == "user-owned skill\n" + assert next(r for r in actual["installed"] if r["path"] == str(user))["status"] == "skipped_user_file" + if not uninstall: + canonical = (skills / "loopx-global-summary.md" if flat else + skills / "loopx-global-summary" / "SKILL.md") + assert canonical.is_file() + assert not any(r["command"] == "/loop-global-summary" and r["invoke_as"] + for r in actual["installed"]) + + +def test_facade_alias_role_comes_from_catalog_not_name_prefix(tmp_path, monkeypatch): + build_catalog = slash_command_install.build_slash_command_catalog + + def renamed_alias(**options): + catalog = build_catalog(**options) + for row in catalog["commands"]: + if row["command"] == "/loopx-global-summary": + row["legacy_aliases"] = ["/old-summary"] + return catalog + + monkeypatch.setattr(slash_command_install, "build_slash_command_catalog", renamed_alias) + specs = slash_command_install._command_prompt_specs(cli_bin="loopx", include_legacy_aliases=True) + alias = next(spec for spec in specs if spec["name"] == "old-summary") + assert alias["alias_for"] == "/loopx-global-summary" + root = tmp_path / "claude" + old = root / "skills" / "old-summary" / "SKILL.md" + old.parent.mkdir(parents=True) + old.write_text(MANAGED_SKILL + "old alias\n") + install_slash_commands(execute=True, surfaces=["claude-code"], claude_home=str(root)) + assert not old.exists() + assert (root / "skills" / "loopx-global-summary" / "SKILL.md").is_file() def test_opencode_static_uninstall_preserves_installed_bridge(tmp_path: Path) -> None: