From 82d6c28bd5701ee157e1eba6153d4a3b5d41e26f Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Sat, 28 Mar 2026 14:32:23 -0700 Subject: [PATCH] fix(skills): cache-aware /skills install and uninstall in TUI (#3586) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two fixes for /skills install and /skills uninstall slash commands: 1. input() hangs indefinitely inside prompt_toolkit's TUI event loop, soft-locking the CLI. The user typing the slash command is already implicit consent, so confirmation is now always skipped. 2. Cache invalidation was unconditional — installing or uninstalling a skill mid-session silently broke the prompt cache, increasing costs. The slash handler now defers cache invalidation by default (skill takes effect next session). Pass --now to invalidate immediately, with a message explaining the cost tradeoff. The CLI argparse path (hermes skills install) is unaffected and still invalidates. Fixes #3474 Salvaged from PR #3496 by dlkakbs. --- hermes_cli/skills_hub.py | 59 ++++++++++++------- tests/hermes_cli/test_skills_skip_confirm.py | 60 +++++++++++++++++--- 2 files changed, 89 insertions(+), 30 deletions(-) diff --git a/hermes_cli/skills_hub.py b/hermes_cli/skills_hub.py index 62f91ce9a..02082f179 100644 --- a/hermes_cli/skills_hub.py +++ b/hermes_cli/skills_hub.py @@ -304,7 +304,8 @@ def do_browse(page: int = 1, page_size: int = 20, source: str = "all", def do_install(identifier: str, category: str = "", force: bool = False, - console: Optional[Console] = None, skip_confirm: bool = False) -> None: + console: Optional[Console] = None, skip_confirm: bool = False, + invalidate_cache: bool = True) -> None: """Fetch, quarantine, scan, confirm, and install a skill.""" from tools.skills_hub import ( GitHubAuth, create_source_router, ensure_hub_dirs, @@ -417,12 +418,16 @@ def do_install(identifier: str, category: str = "", force: bool = False, c.print(f"[bold green]Installed:[/] {install_dir.relative_to(SKILLS_DIR)}") c.print(f"[dim]Files: {', '.join(bundle.files.keys())}[/]\n") - # Invalidate the skills prompt cache so the new skill appears immediately - try: - from agent.prompt_builder import clear_skills_system_prompt_cache - clear_skills_system_prompt_cache(clear_snapshot=True) - except Exception: - pass + if invalidate_cache: + # Invalidate the skills prompt cache so the new skill appears immediately + try: + from agent.prompt_builder import clear_skills_system_prompt_cache + clear_skills_system_prompt_cache(clear_snapshot=True) + except Exception: + pass + else: + c.print("[dim]Skill will be available in your next session.[/]") + c.print("[dim]Use /reset to start a new session now, or --now to activate immediately (invalidates prompt cache).[/]\n") def do_inspect(identifier: str, console: Optional[Console] = None) -> None: @@ -610,7 +615,8 @@ def do_audit(name: Optional[str] = None, console: Optional[Console] = None) -> N def do_uninstall(name: str, console: Optional[Console] = None, - skip_confirm: bool = False) -> None: + skip_confirm: bool = False, + invalidate_cache: bool = True) -> None: """Remove a hub-installed skill with confirmation.""" from tools.skills_hub import uninstall_skill @@ -630,11 +636,15 @@ def do_uninstall(name: str, console: Optional[Console] = None, success, msg = uninstall_skill(name) if success: c.print(f"[bold green]{msg}[/]\n") - try: - from agent.prompt_builder import clear_skills_system_prompt_cache - clear_skills_system_prompt_cache(clear_snapshot=True) - except Exception: - pass + if invalidate_cache: + try: + from agent.prompt_builder import clear_skills_system_prompt_cache + clear_skills_system_prompt_cache(clear_snapshot=True) + except Exception: + pass + else: + c.print("[dim]Change will take effect in your next session.[/]") + c.print("[dim]Use /reset to start a new session now, or --now to apply immediately (invalidates prompt cache).[/]\n") else: c.print(f"[bold red]Error:[/] {msg}\n") @@ -1071,19 +1081,23 @@ def handle_skills_slash(cmd: str, console: Optional[Console] = None) -> None: elif action == "install": if not args: - c.print("[bold red]Usage:[/] /skills install [--category ] [--force|--yes]\n") + c.print("[bold red]Usage:[/] /skills install [--category ] [--force] [--now]\n") return identifier = args[0] category = "" - # --yes / -y bypasses confirmation prompt (needed in TUI mode) - # --force handles reinstall override - skip_confirm = any(flag in args for flag in ("--yes", "-y")) + # Slash commands run inside prompt_toolkit where input() hangs. + # Always skip confirmation — the user typing the command is implicit consent. + skip_confirm = True force = "--force" in args + # --now invalidates prompt cache immediately (costs more money). + # Default: defer to next session to preserve cache. + invalidate_cache = "--now" in args for i, a in enumerate(args): if a == "--category" and i + 1 < len(args): category = args[i + 1] do_install(identifier, category=category, force=force, - skip_confirm=skip_confirm, console=c) + skip_confirm=skip_confirm, invalidate_cache=invalidate_cache, + console=c) elif action == "inspect": if not args: @@ -1113,10 +1127,13 @@ def handle_skills_slash(cmd: str, console: Optional[Console] = None) -> None: elif action == "uninstall": if not args: - c.print("[bold red]Usage:[/] /skills uninstall [--yes]\n") + c.print("[bold red]Usage:[/] /skills uninstall [--now]\n") return - skip_confirm = any(flag in args for flag in ("--yes", "-y")) - do_uninstall(args[0], console=c, skip_confirm=skip_confirm) + # Slash commands run inside prompt_toolkit where input() hangs. + skip_confirm = True + invalidate_cache = "--now" in args + do_uninstall(args[0], console=c, skip_confirm=skip_confirm, + invalidate_cache=invalidate_cache) elif action == "publish": if not args: diff --git a/tests/hermes_cli/test_skills_skip_confirm.py b/tests/hermes_cli/test_skills_skip_confirm.py index 7293a6b3c..fd430185f 100644 --- a/tests/hermes_cli/test_skills_skip_confirm.py +++ b/tests/hermes_cli/test_skills_skip_confirm.py @@ -1,10 +1,13 @@ """ -Tests for skip_confirm behavior in /skills install and /skills uninstall. +Tests for skip_confirm and invalidate_cache behavior in /skills install +and /skills uninstall slash commands. -Verifies that --yes / -y bypasses the interactive confirmation prompt -that hangs inside prompt_toolkit's TUI. +Slash commands always skip confirmation (input() hangs in TUI). +Cache invalidation is deferred by default; --now opts into immediate +invalidation (at the cost of breaking prompt cache mid-session). Based on PR #1595 by 333Alden333 (salvaged). +Updated for PR #3586 (cache-aware install/uninstall). """ from unittest.mock import patch, MagicMock @@ -32,23 +35,43 @@ class TestHandleSkillsSlashInstallFlags: _, kwargs = mock_install.call_args assert kwargs.get("skip_confirm") is True - def test_force_flag_sets_force_not_skip(self): + def test_force_flag_sets_force(self): from hermes_cli.skills_hub import handle_skills_slash with patch("hermes_cli.skills_hub.do_install") as mock_install: handle_skills_slash("/skills install test/skill --force") mock_install.assert_called_once() _, kwargs = mock_install.call_args assert kwargs.get("force") is True - assert kwargs.get("skip_confirm") is False + # Slash commands always skip confirmation (input() hangs in TUI) + assert kwargs.get("skip_confirm") is True - def test_no_flags(self): + def test_no_flags_still_skips_confirm(self): + """Slash commands always skip confirmation — input() hangs in TUI.""" from hermes_cli.skills_hub import handle_skills_slash with patch("hermes_cli.skills_hub.do_install") as mock_install: handle_skills_slash("/skills install test/skill") mock_install.assert_called_once() _, kwargs = mock_install.call_args assert kwargs.get("force") is False - assert kwargs.get("skip_confirm") is False + assert kwargs.get("skip_confirm") is True + + def test_default_defers_cache_invalidation(self): + """Without --now, cache invalidation is deferred to next session.""" + from hermes_cli.skills_hub import handle_skills_slash + with patch("hermes_cli.skills_hub.do_install") as mock_install: + handle_skills_slash("/skills install test/skill") + mock_install.assert_called_once() + _, kwargs = mock_install.call_args + assert kwargs.get("invalidate_cache") is False + + def test_now_flag_invalidates_cache(self): + """--now opts into immediate cache invalidation.""" + from hermes_cli.skills_hub import handle_skills_slash + with patch("hermes_cli.skills_hub.do_install") as mock_install: + handle_skills_slash("/skills install test/skill --now") + mock_install.assert_called_once() + _, kwargs = mock_install.call_args + assert kwargs.get("invalidate_cache") is True class TestHandleSkillsSlashUninstallFlags: @@ -70,13 +93,32 @@ class TestHandleSkillsSlashUninstallFlags: _, kwargs = mock_uninstall.call_args assert kwargs.get("skip_confirm") is True - def test_no_flags(self): + def test_no_flags_still_skips_confirm(self): + """Slash commands always skip confirmation — input() hangs in TUI.""" from hermes_cli.skills_hub import handle_skills_slash with patch("hermes_cli.skills_hub.do_uninstall") as mock_uninstall: handle_skills_slash("/skills uninstall test-skill") mock_uninstall.assert_called_once() _, kwargs = mock_uninstall.call_args - assert kwargs.get("skip_confirm", False) is False + assert kwargs.get("skip_confirm") is True + + def test_default_defers_cache_invalidation(self): + """Without --now, cache invalidation is deferred to next session.""" + from hermes_cli.skills_hub import handle_skills_slash + with patch("hermes_cli.skills_hub.do_uninstall") as mock_uninstall: + handle_skills_slash("/skills uninstall test-skill") + mock_uninstall.assert_called_once() + _, kwargs = mock_uninstall.call_args + assert kwargs.get("invalidate_cache") is False + + def test_now_flag_invalidates_cache(self): + """--now opts into immediate cache invalidation.""" + from hermes_cli.skills_hub import handle_skills_slash + with patch("hermes_cli.skills_hub.do_uninstall") as mock_uninstall: + handle_skills_slash("/skills uninstall test-skill --now") + mock_uninstall.assert_called_once() + _, kwargs = mock_uninstall.call_args + assert kwargs.get("invalidate_cache") is True class TestDoInstallSkipConfirm: