feat(tui): Add /skill lifecycle commands for installed AgentSkills#680
feat(tui): Add /skill lifecycle commands for installed AgentSkills#680sjathin wants to merge 1 commit intoOpenHands:mainfrom
Conversation
|
Thank you! Lets see what my agent thinks too @OpenHands Do a /codereview /codereview-roasted on this PR. Post your feedback as a review with gh api and event. |
|
I'm on it! enyst can track my progress at all-hands.dev |
enyst
left a comment
There was a problem hiding this comment.
🔴 Needs improvement
[CRITICAL ISSUES]
- [
openhands_cli/tui/core/skill_commands.py] Breaking user expectation: with the SDK version currently locked in this repo,install_skill()writes to~/.openhands/skills/installed/<name>butload_user_skills()still does not load from there. I reproduced this in a temp HOME: after install,list_installed_skills()returned[("pirate-mode", True)]whileload_user_skills()returned[]. That means/skill installsucceeds on paper but the installed skill never becomes active, and the new success text (Restart your session to load/apply) is false today.
[TESTING GAPS]
- [
tests/tui/core/test_skill_commands.py] Real code path not covered: the integration test only checks the installed-skill metadata APIs with a custominstalled_dir; it never exercisesload_user_skills()or the CLI's actual loaded-resource path, so it misses the regression above.
[RISK ASSESSMENT]
- [Overall PR]
⚠️ Risk Assessment: 🟡 MEDIUM
This is a small, localized TUI change, but it exposes a new user-facing command family whose success path currently does not produce a usable installed skill with the SDK version under test. That is a behavior-risk issue, not just a test gap.
VERDICT:
❌ Needs rework: either pair this with the SDK loader fix / version bump, or make the CLI explicitly detect and warn about the current incompatibility instead of claiming restart will apply the skill.
KEY INSIGHT:
The command surface is fine; the broken piece is the end-to-end data path — installed skills are written to one place and loaded from another.
This review was generated by an AI assistant (OpenHands) on behalf of the user.
|
Done — I completed the requested PR review workflow.
Conciseness check:
Net result:
|
5cf64dc to
ccbdba6
Compare
|
Thanks for the comments. I have addressed the following:
|
c2c6139 to
dd6a13f
Compare
|
If you merge in main/rebase on top of main, it will now get even newer SDK v1.19.0. |
Add /skill command family with install, list, enable, disable, uninstall, and update subcommands wrapping the SDK's skills API. Add prefix-matching in command validation and argument parsing in InputField._parse_command(). Bump openhands-sdk and openhands-tools from 1.17.0 to 1.18.1 to fix the end-to-end path where installed skills were not loaded by load_user_skills(). Add 40 tests: unit tests for all subcommands, command validation, argument parsing, a full lifecycle integration test, and a regression test verifying load_user_skills() sees installed skills. Closes OpenHands#582
dd6a13f to
598f27d
Compare
Why
The CLI exposes /skills for viewing loaded resources but has no command surface for managing installed AgentSkills. The SDK now has a public lifecycle API (install_skill, list_installed_skills, enable_skill, disable_skill, uninstall_skill, update_skill)
that the CLI should expose.
Summary
Issue Number
Closes #582 (#582)
How to Test
uv run openhands/skilland press Enter → should show help with all subcommands-
/skill install /tmp/pirate-mode → "Installed skill 'pirate-mode'"-
/skill list → shows "✓ enabled pirate-mode"-
/skill disable pirate-mode → "Disabled skill 'pirate-mode'"-
/skill list → shows "✗ disabled pirate-mode"-
/skill enable pirate-mode → "Enabled skill 'pirate-mode'"-
/skill uninstall pirate-mode → "Uninstalled skill 'pirate-mode'"-
/skill list → "No installed skills"Video/Screenshots
Manual TUI testing performed with a local pirate-mode test skill:
Verification
make lint # passes
make test # 1068 passed (1 pre-existing failure unrelated to this PR)
uv run pytest tests/tui/core/test_skill_commands.py -v # 40/40 passed
Type
Notes
accurate.