feat: skills plugin #210

Merged

k9ert merged 10 commits from David/cobot:feature/skills into main 2026-03-08 10:53:03 +00:00

Conversation 6 Commits 10 Files changed 9 +1354 -4

David commented 2026-03-06 11:24:39 +00:00

Contributor

Copy link

Adds a Skills plugin that brings markdown-based skill management to cobot — discover, install, load, and remove reusable agent skills from local paths or git repos.

• Skills plugin (cobot/plugins/skills/): Discovers SKILL.md files from configured directories, injects available skills into the system prompt as XML, and provides a load_skill tool for on-demand loading at runtime
• CLI commands: cobot skill list, skill add, skill show, skill remove — install skills from local paths, GitHub shorthand (user/repo), or full git URLs
• Trust integration: Skills with an author field are checked against the configured trusted npubs, surfacing verified/unverified status
• Precedence model: Workspace skills override user-level skills; first path wins on duplicates
• Bug fix: register_plugin_commands() was passing instances to PluginRegistry.register() instead of classes, silently breaking CLI command registration for all plugins

Includes 639 lines of tests covering discovery, frontmatter parsing, CLI commands, and edge cases.

Implements #207

Adds a Skills plugin that brings markdown-based skill management to cobot — discover, install, load, and remove reusable agent skills from local paths or git repos. - Skills plugin (cobot/plugins/skills/): Discovers SKILL.md files from configured directories, injects available skills into the system prompt as XML, and provides a load_skill tool for on-demand loading at runtime - CLI commands: cobot skill list, skill add, skill show, skill remove — install skills from local paths, GitHub shorthand (user/repo), or full git URLs - Trust integration: Skills with an author field are checked against the configured trusted npubs, surfacing verified/unverified status - Precedence model: Workspace skills override user-level skills; first path wins on duplicates - Bug fix: register_plugin_commands() was passing instances to PluginRegistry.register() instead of classes, silently breaking CLI command registration for all plugins Includes 639 lines of tests covering discovery, frontmatter parsing, CLI commands, and edge cases. Implements https://forgejo.tail593e12.ts.net/ultanio/cobot/issues/207

David added 5 commits 2026-03-06 11:24:40 +00:00

chore: update .gitignore to include Docker data and BMad command directories f142e7f7c2

feat: add Skills plugin for markdown-based skill management and CLI commands ... ef365dde3c

- Introduced a new Skills plugin that enables discovery, loading, and management of skills defined in markdown format (`SKILL.md`).
- Implemented CLI commands for adding, listing, showing, and removing skills.
- Added data structures and utility functions for skill metadata handling.
- Updated `.gitignore` to include `_bmad-output/` directory.
- Included comprehensive README documentation for usage and configuration.

refactor: remove unused imports from Skills plugin initialization ... 9eab80407b

- Cleaned up the `__init__.py` file by removing unused imports for `Skill`, `SkillsPlugin`, and `create_plugin`.
- This change simplifies the module's interface and improves readability and avoids cirular dependencies

fix: initialize default skill paths in Skills plugin ... 9c04f51d18

- Updated the `__init__` method of the SkillsPlugin to set default paths for skill loading, including a local `./skills` directory and a user-specific directory at `~/.cobot/skills`.
- This change enhances the plugin's usability by providing predefined locations for skill management.

fix: CLI plugin command registration passing instances instead of classes ... 309dc768d2

register_plugin_commands() was creating plugin instances then passing them to PluginRegistry.register(), which expects classes. The isinstance(x, type) check failed for every plugin, but the bare except silently swallowed all errors — so no plugin CLI commands (skill, etc.) ever registered.

David changed title from feature/skills to feat: skills plugin 2026-03-06 11:25:13 +00:00

doxios commented 2026-03-06 11:30:35 +00:00

Collaborator

Copy link

Review: Skills Plugin

Great work David — this is a solid, well-structured plugin. Clean separation (models, cli, plugin), proper ToolProvider pattern, good test coverage (639 lines!). Here's my detailed feedback:

✅ Architecture Strengths

• Clean data model — Skill dataclass + pure utility functions in models.py
• ToolProvider pattern — load_skill follows established cobot conventions
• System prompt injection via loop.transform_system_prompt — correct hook
• Precedence system — workspace skills override user skills, first path wins
• Author verification via trusted npubs — nice touch for self-sovereign ethos
• AgentSkills spec compatibility — aligns with OpenClaw/Vercel/Claude Code

🟡 CLI Verbosity (your question)

You're right — the click.echo lines in cli.py are verbose. Options:

1. Table formatter helper — a small _print_skill(skill) and _print_skill_list(skills) function that handles the formatting. Reduces repetition.
2. Use click.echo with f-strings is actually fine — it's explicit and readable. The verbosity is mostly in skill_show which has ~8 echo calls. That's not bad.
3. Rich/tabulate — overkill for this, don't add a dependency.

My take: extract a _format_skill_detail(skill) -> str that returns the whole block, then click.echo(_format_skill_detail(match)). One echo call instead of 8. Same for list.

🔴 Git dependency (your question)

The git-only approach for skill add is fine as-is. Here's why:

• git is ubiquitous on dev machines
• The error message when git is missing is clear: "git is required for remote skill installation"
• Local path install works without git — so Docker users can copy skills in via volume mount
• A separate git tool plugin would be overengineering for a CLI command

One improvement: Add a note to the error message:

Error: git is required for remote skill installation.
Use a local path instead: cobot skill add ./path/to/skill

This guides Docker users to the working alternative.

🟡 Code Issues

1. _discover_and_list() duplicates _discover_skills() — 90% same code. Extract shared logic into _scan_paths(skip_disabled=True) that both call.

2. register_plugin_commands() change in cli.py — registry.register(plugin_class) changed from registry.register(instance). Is the registry API actually register(class) or register(instance)? Verify this doesn't break other plugins.

3. BMad artifacts in .gitignore — _bmad/** and _bmad-output/** plus .claude/commands/** and .cursor/commands/** added. These are editor/workflow specific. Consider putting them in .git/info/exclude instead of polluting the shared .gitignore.

4. No error handling on shutil.copytree in _install_single_skill — if target dir has permission issues, it'll crash with a raw traceback.

5. resolve_relative_paths regex — only matches ./foo/bar.md patterns. Won't resolve ../ paths or paths without extensions. Fine for now, but document the limitation.

🟢 Tests

Excellent coverage — 639 lines of tests. Models, plugin, CLI helpers all covered. Only gap: no integration test for _install_from_git (understandable, requires network).

Summary

This is merge-ready with minor fixes:

1. DRY up _discover_and_list() vs _discover_skills()
2. Improve git-missing error message
3. Move editor-specific ignores out of .gitignore
4. Verify registry.register() API change

Overall: excellent plugin, well-designed. 👏

## Review: Skills Plugin Great work David — this is a solid, well-structured plugin. Clean separation (models, cli, plugin), proper ToolProvider pattern, good test coverage (639 lines!). Here's my detailed feedback: ### ✅ Architecture Strengths - **Clean data model** — `Skill` dataclass + pure utility functions in `models.py` - **ToolProvider pattern** — `load_skill` follows established cobot conventions - **System prompt injection** via `loop.transform_system_prompt` — correct hook - **Precedence system** — workspace skills override user skills, first path wins - **Author verification** via trusted npubs — nice touch for self-sovereign ethos - **AgentSkills spec compatibility** — aligns with OpenClaw/Vercel/Claude Code ### 🟡 CLI Verbosity (your question) You're right — the `click.echo` lines in `cli.py` are verbose. Options: 1. **Table formatter helper** — a small `_print_skill(skill)` and `_print_skill_list(skills)` function that handles the formatting. Reduces repetition. 2. **Use `click.echo` with f-strings is actually fine** — it's explicit and readable. The verbosity is mostly in `skill_show` which has ~8 echo calls. That's not bad. 3. **Rich/tabulate** — overkill for this, don't add a dependency. My take: extract a `_format_skill_detail(skill) -> str` that returns the whole block, then `click.echo(_format_skill_detail(match))`. One echo call instead of 8. Same for list. ### 🔴 Git dependency (your question) The git-only approach for `skill add` is **fine as-is**. Here's why: - `git` is ubiquitous on dev machines - The error message when git is missing is clear: `"git is required for remote skill installation"` - **Local path install works without git** — so Docker users can copy skills in via volume mount - A separate git tool plugin would be overengineering for a CLI command **One improvement**: Add a note to the error message: ``` Error: git is required for remote skill installation. Use a local path instead: cobot skill add ./path/to/skill ``` This guides Docker users to the working alternative. ### 🟡 Code Issues 1. **`_discover_and_list()` duplicates `_discover_skills()`** — 90% same code. Extract shared logic into `_scan_paths(skip_disabled=True)` that both call. 2. **`register_plugin_commands()` change in `cli.py`** — `registry.register(plugin_class)` changed from `registry.register(instance)`. Is the registry API actually `register(class)` or `register(instance)`? Verify this doesn't break other plugins. 3. **BMad artifacts in `.gitignore`** — `_bmad/**` and `_bmad-output/**` plus `.claude/commands/**` and `.cursor/commands/**` added. These are editor/workflow specific. Consider putting them in `.git/info/exclude` instead of polluting the shared `.gitignore`. 4. **No error handling on `shutil.copytree` in `_install_single_skill`** — if target dir has permission issues, it'll crash with a raw traceback. 5. **`resolve_relative_paths` regex** — only matches `./foo/bar.md` patterns. Won't resolve `../` paths or paths without extensions. Fine for now, but document the limitation. ### 🟢 Tests Excellent coverage — 639 lines of tests. Models, plugin, CLI helpers all covered. Only gap: no integration test for `_install_from_git` (understandable, requires network). ### Summary This is merge-ready with minor fixes: 1. DRY up `_discover_and_list()` vs `_discover_skills()` 2. Improve git-missing error message 3. Move editor-specific ignores out of `.gitignore` 4. Verify `registry.register()` API change Overall: excellent plugin, well-designed. 👏

David added 2 commits 2026-03-06 11:35:01 +00:00

chore: add git as a build dependency in Dockerfile d5886eeceb

refactor: improve error handling and skill detail formatting in CLI commands ... 04f14a7caa

- Replaced direct error messages with `click.ClickException` for better error handling in CLI commands.
- Introduced a new `_format_skill_detail` function to encapsulate skill metadata formatting, improving code readability and maintainability.
- Streamlined skill discovery and loading logic in the SkillsPlugin by consolidating path scanning and handling of disabled skills.

David commented 2026-03-06 11:39:10 +00:00

Author

Contributor

Copy link

register_plugin_commands() change in cli.py — registry.register(plugin_class) changed from registry.register(instance). Is the registry API actually register(class) or register(instance)? Verify this doesn't break other plugins.

The API is unambiguous:

• Signature: register(self, plugin_class: Type[Plugin]) -> Plugin
• Line 78: isinstance(plugin_class, type) — checks it's a class, not an instance
• Line 99: instance = plugin_class() — it creates the instance internally

The old code was instance = plugin_class() then registry.register(instance) — passing an object to a method that expects a class. Line 78 would always fail because an instance is not a type. This was a pre-existing bug affecting all plugins, not just skills — we saw every single plugin fail with the same error in Docker.

> register_plugin_commands() change in cli.py — registry.register(plugin_class) changed from registry.register(instance). Is the registry API actually register(class) or register(instance)? Verify this doesn't break other plugins. The API is unambiguous: - Signature: register(self, plugin_class: Type[Plugin]) -> Plugin - Line 78: isinstance(plugin_class, type) — checks it's a class, not an instance - Line 99: instance = plugin_class() — it creates the instance internally The old code was instance = plugin_class() then registry.register(instance) — passing an object to a method that expects a class. Line 78 would always fail because an instance is not a type. This was a pre-existing bug affecting all plugins, not just skills — we saw every single plugin fail with the same error in Docker.

David added 1 commit 2026-03-06 11:41:59 +00:00

chore: clean up .gitignore by removing obsolete BMad entries f8b8772452

doxios commented 2026-03-06 11:43:57 +00:00

Collaborator

Copy link

Re-review ✅

All previous issues addressed:

• ✅ DRY — _scan_all_paths() extracted, both _discover_skills() and _discover_and_list() use it
• ✅ Git error message — now suggests local path alternative
• ✅ BMad/editor ignores — removed from .gitignore
• ✅ Git in Dockerfile — nice, Docker users can install remote skills too
• ✅ Registry API — confirmed register(class) is the correct signature, old code was wrong

LGTM — ready to merge. 👍

## Re-review ✅ All previous issues addressed: - ✅ **DRY** — `_scan_all_paths()` extracted, both `_discover_skills()` and `_discover_and_list()` use it - ✅ **Git error message** — now suggests local path alternative - ✅ **BMad/editor ignores** — removed from `.gitignore` - ✅ **Git in Dockerfile** — nice, Docker users can install remote skills too - ✅ **Registry API** — confirmed `register(class)` is the correct signature, old code was wrong LGTM — ready to merge. 👍

doxios commented 2026-03-06 15:48:06 +00:00

Collaborator

Copy link

CI lint failure: cobot/plugins/skills/cli.py needs formatting.

Fix: ruff format cobot/plugins/skills/cli.py

CI lint failure: `cobot/plugins/skills/cli.py` needs formatting. Fix: `ruff format cobot/plugins/skills/cli.py`

doxios referenced this pull request 2026-03-06 16:25:47 +00:00

proposal: Peer Interaction Ledger #211

k9ert commented 2026-03-06 16:37:46 +00:00

Owner

Copy link

Linzer is red, though.

Linzer is red, though.

David added 1 commit 2026-03-07 03:07:30 +00:00

refactor: streamline error messages in CLI skill installation 64add749b2

David commented 2026-03-08 03:51:23 +00:00

Author

Contributor

Copy link

@k9ert can you / nazim review again, linter is fine now

@k9ert can you / nazim review again, linter is fine now

k9ert added 1 commit 2026-03-08 10:41:38 +00:00

Merge branch 'main' into feature/skills a701caba9f

k9ert merged commit b220d48fb3 into main 2026-03-08 10:53:03 +00:00

k9ert referenced this pull request from a commit 2026-03-08 10:53:03 +00:00

feat: skills plugin (#210)

doxios referenced this pull request 2026-03-08 16:28:27 +00:00

Plugin Dependency Tree Resolution #230

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

Compat/Breaking

Breaking change that won't be backward compatible

  

Kind/Bug

Something is not working

  

Kind/Competitor

Anything we found interesting at competitory

  

Kind/Documentation

Documentation changes

  

Kind/Enhancement

Improve existing functionality

  

Kind/Epic

Business requirement — groups related stories

  

Kind/Feature

New functionality

  

Kind/Security

This is security issue

  

Kind/Story

Implementable unit with AC — results in a PR

  

Kind/Testing

Issue or pull request related to testing

  

Priority

Critical

The priority is critical

  

Priority

High

The priority is high

  

Priority

Low

The priority is low

  

Priority

Medium

The priority is medium

  

Reviewed

Confirmed

Issue has been confirmed

  

Reviewed

Duplicate

This issue or pull request already exists

  

Reviewed

Invalid

Invalid issue

  

Reviewed

Won't Fix

This issue won't be fixed

  

Scope/Core

Changes to core non-plugin code (CLI, config loading, etc.).

  

Scope/Cross-Plugin

Touches multiple existing plugins. Needs extra scrutiny, may require a PRD.

  

Scope/Plugin-System

Changes to plugin infrastructure (base.py, registry.py, interfaces.py, __init__.py, agent.py plugin dispatch). PRD required before merge.

  

Scope/Single-Plugin

Changes contained to one plugin (or adds one new plugin). Standard review.

  

Status

Abandoned

Somebody has started to work on this but abandoned work

  

Status

Blocked

Something is blocking this issue or pull request

  

Status

Need More Info

Feedback is required to reproduce issue or to continue work

No labels

Compat/Breaking

Kind/Bug

Kind/Competitor

Kind/Documentation

Kind/Enhancement

Kind/Epic

Kind/Feature

Kind/Security

Kind/Story

Kind/Testing

Priority

Critical

Priority

High

Priority

Low

Priority

Medium

Reviewed

Confirmed

Reviewed

Duplicate

Reviewed

Invalid

Reviewed

Won't Fix

Scope/Core

Scope/Cross-Plugin

Scope/Plugin-System

Scope/Single-Plugin

Status

Abandoned

Status

Blocked

Status

Need More Info

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

3 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

ultanio/cobot!210

No description provided.