diff --git a/openhands-cli.spec b/openhands-cli.spec index cd85ce5b2..b9afeac06 100644 --- a/openhands-cli.spec +++ b/openhands-cli.spec @@ -36,6 +36,7 @@ a = Analysis( *collect_data_files('openhands.sdk'), *collect_data_files('openhands.tools'), *collect_data_files('browser_use'), + *collect_data_files('binaryornot'), # Include all data files from openhands_cli package *collect_data_files('openhands_cli'), # Include package metadata for importlib.metadata diff --git a/openhands_cli/tui/core/commands.py b/openhands_cli/tui/core/commands.py index 41aa2bf2a..dcc194dc0 100644 --- a/openhands_cli/tui/core/commands.py +++ b/openhands_cli/tui/core/commands.py @@ -22,11 +22,15 @@ DropdownItem(main="/settings - Open settings"), DropdownItem(main="/confirm - Configure confirmation settings"), DropdownItem(main="/condense - Condense conversation history"), + DropdownItem(main="/skill - Manage installed skills (install/enable/disable/...)"), DropdownItem(main="/skills - View loaded skills, hooks, and MCPs"), DropdownItem(main="/feedback - Send anonymous feedback about CLI"), DropdownItem(main="/exit - Exit the application"), ] +# Commands that accept subcommands/arguments (prefix-matched) +PREFIX_COMMANDS = {"/skill"} + def get_valid_commands() -> set[str]: """Extract valid command names from COMMANDS list. @@ -47,15 +51,24 @@ def get_valid_commands() -> set[str]: def is_valid_command(user_input: str) -> bool: - """Check if user input is an exact match for a valid command. + """Check if user input is a valid command. + + Supports exact matches (e.g. /help) and prefix matches for commands + that accept subcommands (e.g. /skill install foo). Args: user_input: The user's input string Returns: - True if input exactly matches a valid command, False otherwise + True if input matches a valid command, False otherwise """ - return user_input in get_valid_commands() + if user_input in get_valid_commands(): + return True + # Check prefix commands (e.g. "/skill install foo" starts with "/skill ") + for prefix in PREFIX_COMMANDS: + if user_input.startswith(prefix + " ") or user_input == prefix: + return True + return False def show_help(scroll_view: VerticalScroll) -> None: @@ -77,6 +90,7 @@ def show_help(scroll_view: VerticalScroll) -> None: [{secondary}]/settings[/{secondary}] - Open settings [{secondary}]/confirm[/{secondary}] - Configure confirmation settings [{secondary}]/condense[/{secondary}] - Condense conversation history + [{secondary}]/skill[/{secondary}] - Manage installed skills [{secondary}]/skills[/{secondary}] - View loaded skills, hooks, and MCPs [{secondary}]/feedback[/{secondary}] - Send anonymous feedback about CLI [{secondary}]/exit[/{secondary}] - Exit the application diff --git a/openhands_cli/tui/core/skill_commands.py b/openhands_cli/tui/core/skill_commands.py new file mode 100644 index 000000000..2f3ebdb9c --- /dev/null +++ b/openhands_cli/tui/core/skill_commands.py @@ -0,0 +1,208 @@ +"""Skill lifecycle command handlers for the TUI. + +Wraps the SDK's public skill management API (openhands.sdk.skills) +and renders results into the TUI scroll view. +""" + +from __future__ import annotations + +from textual.containers import VerticalScroll +from textual.widgets import Static + +from openhands_cli.theme import OPENHANDS_THEME + + +_ERR = OPENHANDS_THEME.error +_OK = OPENHANDS_THEME.success +_WARN = OPENHANDS_THEME.warning + + +def handle_skill_command(scroll_view: VerticalScroll, args: str) -> None: + """Route /skill to the appropriate handler.""" + parts = args.strip().split(None, 1) + subcommand = parts[0] if parts else "" + sub_args = parts[1].strip() if len(parts) > 1 else "" + + match subcommand: + case "install": + _skill_install(scroll_view, sub_args) + case "list": + _skill_list(scroll_view) + case "enable": + _skill_enable(scroll_view, sub_args) + case "disable": + _skill_disable(scroll_view, sub_args) + case "uninstall": + _skill_uninstall(scroll_view, sub_args) + case "update": + _skill_update(scroll_view, sub_args) + case _: + _skill_help(scroll_view) + + +def _mount(scroll_view: VerticalScroll, text: str) -> None: + scroll_view.mount(Static(text, classes="skill-command-message")) + scroll_view.scroll_end(animate=False) + + +def _skill_help(scroll_view: VerticalScroll) -> None: + s = OPENHANDS_THEME.secondary + p = OPENHANDS_THEME.primary + _mount( + scroll_view, + f""" +[bold {p}]Skill Management[/bold {p}] +[dim]Usage:[/dim] /skill + + [{s}]install [/{s}] - Install a skill + [{s}]list[/{s}] - List installed skills + [{s}]enable [/{s}] - Enable a skill + [{s}]disable [/{s}] - Disable a skill + [{s}]uninstall [/{s}] - Uninstall a skill + [{s}]update [/{s}] - Update a skill +""", + ) + + +def _skill_install(scroll_view: VerticalScroll, source: str) -> None: + if not source: + _mount( + scroll_view, + f"[{_ERR}]Usage: /skill install [/]", + ) + return + try: + from openhands.sdk.skills import install_skill + + info = install_skill(source) + _mount( + scroll_view, + f"[{_OK}]Installed skill '{info.name}'[/]\n" + "Restart your session to load the new skill.", + ) + except Exception as e: + _mount( + scroll_view, + f"[{_ERR}]Install failed: {e}[/]", + ) + + +def _skill_list(scroll_view: VerticalScroll) -> None: + try: + from openhands.sdk.skills import list_installed_skills + + skills = list_installed_skills() + if not skills: + _mount( + scroll_view, + "[dim]No installed skills.[/dim]", + ) + return + p = OPENHANDS_THEME.primary + lines = [f"\n[bold {p}]Installed Skills ({len(skills)})[/bold {p}]"] + for sk in skills: + status = "✓ enabled" if sk.enabled else "✗ disabled" + style = _OK if sk.enabled else _WARN + desc = f" - {sk.description}" if sk.description else "" + lines.append(f" [{style}]{status}[/] {sk.name}{desc}") + _mount(scroll_view, "\n".join(lines)) + except Exception as e: + _mount(scroll_view, f"[{_ERR}]Error: {e}[/]") + + +def _skill_enable(scroll_view: VerticalScroll, name: str) -> None: + if not name: + _mount( + scroll_view, + f"[{_ERR}]Usage: /skill enable [/]", + ) + return + try: + from openhands.sdk.skills import enable_skill + + if enable_skill(name): + _mount( + scroll_view, + f"[{_OK}]Enabled skill '{name}'[/]\nRestart your session to apply.", + ) + else: + _mount( + scroll_view, + f"[{_WARN}]Skill '{name}' not found.[/]", + ) + except Exception as e: + _mount(scroll_view, f"[{_ERR}]Error: {e}[/]") + + +def _skill_disable(scroll_view: VerticalScroll, name: str) -> None: + if not name: + _mount( + scroll_view, + f"[{_ERR}]Usage: /skill disable [/]", + ) + return + try: + from openhands.sdk.skills import disable_skill + + if disable_skill(name): + _mount( + scroll_view, + f"[{_OK}]Disabled skill '{name}'[/]\nRestart your session to apply.", + ) + else: + _mount( + scroll_view, + f"[{_WARN}]Skill '{name}' not found.[/]", + ) + except Exception as e: + _mount(scroll_view, f"[{_ERR}]Error: {e}[/]") + + +def _skill_uninstall(scroll_view: VerticalScroll, name: str) -> None: + if not name: + _mount( + scroll_view, + f"[{_ERR}]Usage: /skill uninstall [/]", + ) + return + try: + from openhands.sdk.skills import uninstall_skill + + if uninstall_skill(name): + _mount( + scroll_view, + f"[{_OK}]Uninstalled skill '{name}'[/]", + ) + else: + _mount( + scroll_view, + f"[{_WARN}]Skill '{name}' not found.[/]", + ) + except Exception as e: + _mount(scroll_view, f"[{_ERR}]Error: {e}[/]") + + +def _skill_update(scroll_view: VerticalScroll, name: str) -> None: + if not name: + _mount( + scroll_view, + f"[{_ERR}]Usage: /skill update [/]", + ) + return + try: + from openhands.sdk.skills import update_skill + + info = update_skill(name) + if info: + _mount( + scroll_view, + f"[{_OK}]Updated skill '{info.name}'[/]\n" + "Restart your session to apply.", + ) + else: + _mount( + scroll_view, + f"[{_WARN}]Skill '{name}' not found.[/]", + ) + except Exception as e: + _mount(scroll_view, f"[{_ERR}]Error: {e}[/]") diff --git a/openhands_cli/tui/widgets/input_area.py b/openhands_cli/tui/widgets/input_area.py index 1a38bd614..79dd2bd99 100644 --- a/openhands_cli/tui/widgets/input_area.py +++ b/openhands_cli/tui/widgets/input_area.py @@ -29,6 +29,7 @@ from textual.reactive import var from openhands_cli.tui.core.commands import show_help, show_skills +from openhands_cli.tui.core.skill_commands import handle_skill_command from openhands_cli.tui.messages import SlashCommandSubmitted @@ -88,6 +89,8 @@ def _on_slash_command_submitted(self, event: SlashCommandSubmitted) -> None: self._command_condense() case "skills": self._command_skills() + case "skill": + self._command_skill(event.args) case "feedback": self._command_feedback() case "exit": @@ -159,6 +162,10 @@ def _command_skills(self) -> None: show_skills(self.scroll_view, self.loaded_resources) self.scroll_view.scroll_end(animate=False) + def _command_skill(self, args: str) -> None: + """Handle the /skill command for skill lifecycle management.""" + handle_skill_command(self.scroll_view, args) + def _command_feedback(self) -> None: """Handle the /feedback command to open feedback form in browser.""" import webbrowser diff --git a/openhands_cli/tui/widgets/user_input/input_field.py b/openhands_cli/tui/widgets/user_input/input_field.py index 465f1d5a3..de45293e8 100644 --- a/openhands_cli/tui/widgets/user_input/input_field.py +++ b/openhands_cli/tui/widgets/user_input/input_field.py @@ -338,11 +338,25 @@ def action_submit_textarea(self) -> None: self.action_toggle_input_mode() # Use the same submission logic as single-line mode if is_valid_command(content): - command = content[1:] # Remove leading "/" - self.post_message(SlashCommandSubmitted(command=command)) + command, args = self._parse_command(content) + self.post_message(SlashCommandSubmitted(command=command, args=args)) else: self.post_message(SendMessage(content=content)) + @staticmethod + def _parse_command(content: str) -> tuple[str, str]: + """Parse a slash command into (command, args). + + Examples: + "/help" -> ("help", "") + "/skill install x" -> ("skill", "install x") + """ + without_slash = content[1:] # Remove leading "/" + parts = without_slash.split(None, 1) + command = parts[0] if parts else "" + args = parts[1] if len(parts) > 1 else "" + return command, args + def _submit_current_content(self) -> None: """Submit current content and clear input. @@ -358,9 +372,9 @@ def _submit_current_content(self) -> None: # Check if this is a valid slash command if is_valid_command(content): - # Extract command name (without the leading slash) - command = content[1:] # Remove leading "/" - self.post_message(SlashCommandSubmitted(command=command)) + # Extract command name and args (without the leading slash) + command, args = self._parse_command(content) + self.post_message(SlashCommandSubmitted(command=command, args=args)) else: # Regular user input self.post_message(SendMessage(content=content)) diff --git a/openhands_cli/tui/widgets/user_input/models.py b/openhands_cli/tui/widgets/user_input/models.py index 3358e967b..e5b4d9072 100644 --- a/openhands_cli/tui/widgets/user_input/models.py +++ b/openhands_cli/tui/widgets/user_input/models.py @@ -1,11 +1,11 @@ """Pydantic models for user input components.""" -from enum import Enum +from enum import StrEnum from pydantic import BaseModel -class CompletionType(str, Enum): +class CompletionType(StrEnum): """Type of completion being performed.""" COMMAND = "command" diff --git a/tests/tui/core/test_skill_commands.py b/tests/tui/core/test_skill_commands.py new file mode 100644 index 000000000..189ee3dcf --- /dev/null +++ b/tests/tui/core/test_skill_commands.py @@ -0,0 +1,273 @@ +"""Tests for /skill lifecycle commands.""" + +from unittest import mock + +import pytest +from textual.containers import VerticalScroll + +from openhands.sdk.skills import ( + disable_skill, + enable_skill, + install_skill, + list_installed_skills, + uninstall_skill, +) +from openhands_cli.tui.core.commands import is_valid_command +from openhands_cli.tui.core.skill_commands import handle_skill_command +from openhands_cli.tui.widgets.user_input.input_field import InputField + + +_SDK = "openhands.sdk.skills" + + +class TestIsValidCommandWithSkill: + """Test is_valid_command supports /skill prefix matching.""" + + @pytest.mark.parametrize( + "cmd,expected", + [ + ("/skill", True), + ("/skill install foo", True), + ("/skill list", True), + ("/skill enable bar", True), + ("/skill disable bar", True), + ("/skill uninstall bar", True), + ("/skill update bar", True), + ("/skills", True), + ("/skillz", False), + ("/skill_bad", False), + ], + ) + def test_skill_prefix_matching(self, cmd, expected): + assert is_valid_command(cmd) is expected + + +class TestParseCommand: + """Test InputField._parse_command splits command and args.""" + + @pytest.mark.parametrize( + "content,expected", + [ + ("/help", ("help", "")), + ("/skill install foo", ("skill", "install foo")), + ("/skill list", ("skill", "list")), + ("/skill", ("skill", "")), + ], + ) + def test_parse_command(self, content, expected): + assert InputField._parse_command(content) == expected + + +class TestHandleSkillCommand: + """Test handle_skill_command routing and output.""" + + def _call(self, args: str) -> str: + sv = mock.MagicMock(spec=VerticalScroll) + handle_skill_command(sv, args) + sv.mount.assert_called_once() + return sv.mount.call_args[0][0].content + + def test_no_subcommand_shows_help(self): + text = self._call("") + assert "Skill Management" in text + + def test_unknown_subcommand_shows_help(self): + text = self._call("bogus") + assert "Skill Management" in text + + def test_install_missing_source(self): + text = self._call("install") + assert "Usage" in text + + @mock.patch(f"{_SDK}.install_skill") + def test_install_success(self, mock_fn): + mock_fn.return_value = mock.Mock(name="my-skill") + text = self._call("install https://example.com/skill") + mock_fn.assert_called_once_with("https://example.com/skill") + assert "Installed" in text + + @mock.patch( + f"{_SDK}.install_skill", + side_effect=RuntimeError("fail"), + ) + def test_install_error(self, _): + text = self._call("install bad-source") + assert "Install failed" in text + + @mock.patch(f"{_SDK}.list_installed_skills", return_value=[]) + def test_list_empty(self, _): + text = self._call("list") + assert "No installed skills" in text + + @mock.patch(f"{_SDK}.list_installed_skills") + def test_list_with_skills(self, mock_fn): + mock_fn.return_value = [ + mock.Mock(name="a", enabled=True, description="desc-a"), + mock.Mock(name="b", enabled=False, description=""), + ] + text = self._call("list") + assert "Installed Skills (2)" in text + assert "enabled" in text + assert "disabled" in text + + def test_enable_missing_name(self): + text = self._call("enable") + assert "Usage" in text + + @mock.patch(f"{_SDK}.enable_skill", return_value=True) + def test_enable_success(self, mock_fn): + text = self._call("enable my-skill") + mock_fn.assert_called_once_with("my-skill") + assert "Enabled" in text + + @mock.patch(f"{_SDK}.enable_skill", return_value=False) + def test_enable_not_found(self, _): + text = self._call("enable missing") + assert "not found" in text + + def test_disable_missing_name(self): + text = self._call("disable") + assert "Usage" in text + + @mock.patch(f"{_SDK}.disable_skill", return_value=True) + def test_disable_success(self, mock_fn): + text = self._call("disable my-skill") + mock_fn.assert_called_once_with("my-skill") + assert "Disabled" in text + + @mock.patch(f"{_SDK}.disable_skill", return_value=False) + def test_disable_not_found(self, _): + text = self._call("disable missing") + assert "not found" in text + + def test_uninstall_missing_name(self): + text = self._call("uninstall") + assert "Usage" in text + + @mock.patch(f"{_SDK}.uninstall_skill", return_value=True) + def test_uninstall_success(self, mock_fn): + text = self._call("uninstall my-skill") + mock_fn.assert_called_once_with("my-skill") + assert "Uninstalled" in text + + @mock.patch(f"{_SDK}.uninstall_skill", return_value=False) + def test_uninstall_not_found(self, _): + text = self._call("uninstall missing") + assert "not found" in text + + def test_update_missing_name(self): + text = self._call("update") + assert "Usage" in text + + @mock.patch(f"{_SDK}.update_skill") + def test_update_success(self, mock_fn): + mock_fn.return_value = mock.Mock(name="my-skill") + text = self._call("update my-skill") + mock_fn.assert_called_once_with("my-skill") + assert "Updated" in text + + @mock.patch(f"{_SDK}.update_skill", return_value=None) + def test_update_not_found(self, _): + text = self._call("update missing") + assert "not found" in text + + # -- SDK exception handling for each subcommand -- + + @mock.patch(f"{_SDK}.list_installed_skills", side_effect=RuntimeError("boom")) + def test_list_error(self, _): + text = self._call("list") + assert "Error" in text + + @mock.patch(f"{_SDK}.enable_skill", side_effect=RuntimeError("boom")) + def test_enable_error(self, _): + text = self._call("enable my-skill") + assert "Error" in text + + @mock.patch(f"{_SDK}.disable_skill", side_effect=RuntimeError("boom")) + def test_disable_error(self, _): + text = self._call("disable my-skill") + assert "Error" in text + + @mock.patch(f"{_SDK}.uninstall_skill", side_effect=RuntimeError("boom")) + def test_uninstall_error(self, _): + text = self._call("uninstall my-skill") + assert "Error" in text + + @mock.patch(f"{_SDK}.update_skill", side_effect=RuntimeError("boom")) + def test_update_error(self, _): + text = self._call("update my-skill") + assert "Error" in text + + +class TestSkillLifecycleIntegration: + """End-to-end lifecycle test using real SDK calls with a temp directory.""" + + def test_full_lifecycle(self, tmp_path): + installed_dir = tmp_path / "installed" + installed_dir.mkdir() + + # Create a local skill + skill_dir = tmp_path / "test-lifecycle" + skill_dir.mkdir() + (skill_dir / "SKILL.md").write_text( + "---\nname: test-lifecycle\ndescription: A test skill\n---\n# Test" + ) + + # Install + info = install_skill(str(skill_dir), installed_dir=installed_dir) + assert info.name == "test-lifecycle" + + # List — should show enabled + skills = list_installed_skills(installed_dir=installed_dir) + assert len(skills) == 1 + assert skills[0].name == "test-lifecycle" + assert skills[0].enabled is True + + # Disable + assert disable_skill("test-lifecycle", installed_dir=installed_dir) is True + skills = list_installed_skills(installed_dir=installed_dir) + assert skills[0].enabled is False + + # Re-enable without reinstalling + assert enable_skill("test-lifecycle", installed_dir=installed_dir) is True + skills = list_installed_skills(installed_dir=installed_dir) + assert skills[0].enabled is True + + # Uninstall + assert uninstall_skill("test-lifecycle", installed_dir=installed_dir) is True + assert list_installed_skills(installed_dir=installed_dir) == [] + assert not (installed_dir / "test-lifecycle").exists() + + def test_load_user_skills_sees_installed_skill(self, tmp_path): + """Regression: install_skill() + load_user_skills() must find the skill. + + Verifies the end-to-end data path that was broken before SDK v1.18.1: + installed skills are written to installed/ and load_user_skills() must + include them. + """ + installed_dir = tmp_path / "installed" + installed_dir.mkdir() + + # Create and install a skill + skill_dir = tmp_path / "loader-test" + skill_dir.mkdir() + (skill_dir / "SKILL.md").write_text( + "---\nname: loader-test\ndescription: Verify loader\n---\n# Test" + ) + install_skill(str(skill_dir), installed_dir=installed_dir) + + from openhands.sdk.skills import load_user_skills + + with ( + mock.patch( + "openhands.sdk.skills.skill.USER_SKILLS_DIRS", [tmp_path / "empty"] + ), + mock.patch( + "openhands.sdk.skills.installed.DEFAULT_INSTALLED_SKILLS_DIR", + installed_dir, + ), + ): + loaded = load_user_skills() + + names = [s.name for s in loaded] + assert "loader-test" in names diff --git a/tests/tui/test_commands.py b/tests/tui/test_commands.py index 186001380..40d64c171 100644 --- a/tests/tui/test_commands.py +++ b/tests/tui/test_commands.py @@ -34,7 +34,7 @@ class TestCommands: def test_commands_list_structure(self): """Test that COMMANDS list has correct structure.""" assert isinstance(COMMANDS, list) - assert len(COMMANDS) == 9 + assert len(COMMANDS) == 10 # Check that all items are DropdownItems for command in COMMANDS: @@ -52,6 +52,7 @@ def test_commands_list_structure(self): ("/settings", "Open settings"), ("/confirm", "Configure confirmation settings"), ("/condense", "Condense conversation history"), + ("/skill", "Manage installed skills"), ("/skills", "View loaded skills, hooks, and MCPs"), ("/feedback", "Send anonymous feedback about CLI"), ("/exit", "Exit the application"), @@ -92,6 +93,7 @@ def test_show_help_function_signature(self): "/settings", "/confirm", "/condense", + "/skill", "/skills", "/feedback", "/exit", @@ -101,6 +103,7 @@ def test_show_help_function_signature(self): "Open settings", "Configure confirmation settings", "Condense conversation history", + "Manage installed skills", "View loaded skills, hooks, and MCPs", "Send anonymous feedback about CLI", "Exit the application", @@ -168,6 +171,8 @@ def test_show_help_formatting(self): ("/settings", True), ("/confirm", True), ("/condense", True), + ("/skill", True), + ("/skill install foo", True), ("/skills", True), ("/feedback", True), ("/exit", True), @@ -191,8 +196,9 @@ def test_commands_contains_history(self): assert "/help" in command_names assert "/new" in command_names assert "/settings" in command_names + assert "/skill" in command_names assert "/skills" in command_names - assert len(COMMANDS) == 9 + assert len(COMMANDS) == 10 def test_all_commands_included_in_help(self): """Test that all commands from COMMANDS list are included in help text.