Add agent-skills infrastructure and installer#940
Conversation
addie9800
left a comment
addie9800
left a comment
There was a problem hiding this comment.
This is a very promising in new step towards making fundus more skalable. I am looking forward to seeing the progress this allows us to make 🚀
| except ModuleNotFoundError as exc: | ||
| if exc.name in {"textual", "rich"}: | ||
| print( | ||
| "The skill installer dashboard needs the 'textual' package.\n" |
There was a problem hiding this comment.
nit: It might be more helpful to not selectively test for textual and one of it's dependencies here, but rather either:
- always suggest running the dev extra install or
- check for all necessary dependencies (i.e.
platformdirsis also essential) and- explicitely name the missing package or
- rephrase the error message to say needs the 'textual' package and all dependencies.
| Text(name, style="bold"), | ||
| _status(agent.is_installed("project", skill)), | ||
| _status(agent.is_installed("user", skill)), | ||
| Text(textwrap.shorten(skill.description, width=70, placeholder="…")), |
There was a problem hiding this comment.
One could consider making the width dynamic here, when supported by the screen and if it doesn't complicate the code too much. For me for example, there is a lot of unnecessary dead space:
| if dest.exists(): | ||
| shutil.rmtree(dest) | ||
| return [f"removed '{skill.name}' for {self.name} ({scope}) -> {dest}"] | ||
| return [f"nothing installed at {dest}"] |
There was a problem hiding this comment.
I would also expect all the packages, that were installed and are now obsolete to be uninstalled. When typing this out, I realized that one would have to ensure that no requirement is removed that is still required by other skills. On a similar note: not really a big issue for now, but with more skills, one might eventually encounter issues with contradicting (version) requirements for skills. As of now, only the most recently installed skill will be available, right?
|
|
||
|
|
||
| def _install_requirements(skill: Skill) -> Tuple[bool, List[str]]: | ||
| """If the skill bundles a ``requirements.txt``, pip-install it into the current interpreter. |
There was a problem hiding this comment.
Could you perhaps provide a couple more details for the use-case of the user scope? Usually the skills we install will probably be quite project specific and unlikely to be required on the user-level. In case required, the execution can still be problematic from what I can see, because the requirements are always installed from the project-level interpreter. Even if the user-level is specified, if a venv/conda env are used when running this installer, the deps will not be installed globally, if I am not mistaken.
|
|
||
| ## Adding a skill | ||
|
|
||
| Create `skills/<name>/` with a `SKILL.md` (frontmatter `name`/`description`) and a `PLAYBOOK.md` |
There was a problem hiding this comment.
Having not too much experience with Markdown, I didn't know what exactly you meant with frontmatter until looking into the review-skill PR. Maybe a template or short example might make adding the first skill simpler.
| """ | ||
| try: | ||
| text = skill_md.read_text(encoding="utf-8") | ||
| except OSError: |
There was a problem hiding this comment.
This is a bit too broad. FileNotFoundError might be a better choice to allow other exceptions to be raised.
| req_ok, req_log = _install_requirements(skill) | ||
| if not req_ok: | ||
| shutil.rmtree(dest, ignore_errors=True) # undo the copy — keep the matrix honest | ||
| return InstallResult(False, [*req_log, f" rolled back '{skill.name}' ({scope}) — nothing left installed"]) |
There was a problem hiding this comment.
Also here, when rolled back, I would also assume the successful installations to be rolled back as well.
| noise = ("A new release of pip", "To update, run") | ||
| lines = [ln.rstrip() for ln in f"{stdout}\n{stderr}".splitlines() if ln.strip()] | ||
| lines = [ln for ln in lines if not any(n in ln for n in noise)] | ||
| errors = [ln for ln in lines if ln.lstrip().startswith("ERROR")] |
There was a problem hiding this comment.
I would include WARNING level as well - might make debugging easier for the user:
WARNING: Retrying (Retry(total=0, connect=None, read=None, redirect=None, status=None)) after connection broken by 'NewConnectionError('<pip._vendor.urllib3.connection.HTTPSConnection object at 0x7fa8d8545450>: Failed to establish a new connection: [Errno -3] Temporary failure in name resolution')': /simple/numpy/
ERROR: Could not find a version that satisfies the requirement numpy (from versions: none)
ERROR: No matching distribution found for numpy
| DataTable { height: 1fr; min-height: 6; margin: 1 1 0 1; border: round $primary; } | ||
| #agent { height: auto; border: round $primary; padding: 0 1; margin: 1 1 0 1; } | ||
| #agent > RadioButton { width: auto; } | ||
| RichLog { height: 5; border: round $panel; padding: 0 1; margin: 1; } |
There was a problem hiding this comment.
I think it would be good to increase the size of the log. For longer error messages (e.g. version mismatch in numpy) it is very difficult to read and get an overview of the output.
2 participants
What
Introduces a home for agent skills — playbooks/scripts meant to be executed by AI coding agents — under
skills/, plus an installer that copies them into an agent's config directory.The repo stays agent-neutral: we track skill sources here, but never commit any agent's config dir (
.claude/is now gitignored). Each developer installs the skills they want into their own agent.Contents
skills/install.py— thin launcher for the installer dashboard.skills/installer/core.py— stdlib-only install/uninstall + skill discovery (any folder with aSKILL.mdis a skill). Install is atomic: if a skill'srequirements.txtstep fails, the copied folder is rolled back, so✓always means the skill and its deps landed.skills/installer/tui.py— Textual status dashboard: a live skills × scopes matrix (project./.claude/skills/vs user~/.claude/skills/); toggle to (un)install in place.skills/README.md— layout conventions, install flow, how to add a skill.pyproject.toml— addstextualto thedevextra (the dashboard's only dependency)..gitignore— ignores the installed-skills dir (.claude/).Notes
available_agents().review-publisher— ships as a companion PR. This PR is the framework; on its own the installer matrix is empty until a skill folder exists.Testing
python skills/install.pyopens the dashboard; toggling project/user installs and uninstalls the discovered skill folders.