[Mate] Add skill lifecycle: discovery, copy install and state in extensions.php - #2370
Conversation
|
I've added the BC Break label.
|
cda3b4f to
011895e
Compare
| * Removes generated mate-* folders that no longer belong to any skill. | ||
| * | ||
| * "skills:install" already prunes what it knows about; this command exists for the leftovers it | ||
| * cannot see — a folder left behind by an interrupted run, or by hand-editing mate/extensions.php. | ||
| * Only "mate-" prefixed entries are ever touched, so skills you maintain yourself are left alone. |
There was a problem hiding this comment.
feels a bit edge-casey - would be also fine to add that in a later iteration if the issue comes up
chr-hertel
left a comment
There was a problem hiding this comment.
Looks pretty good to me 👍
few things to consider:
- would filesystem component remove a bit of load?
- is the feature set in this depth needed already
- after all PRs land we should take a look at the docs, to make sure Mate is easy to understand - maybe even waiting on the MCP removal
Feel free to go ahead please 🙏
…nsions.php Skills are the workflow/knowledge layer above tools: inert SKILL.md procedures the agent reads, managed (never executed) by the CLI. The extra.ai-mate.skills config key already existed as a list of directories; this builds the metadata, install and state layer on top of it, so extensions shipping skills need no edit. A skill is installed as a real copy under .agents/skills/mate-<name>/, with its frontmatter name rewritten to the installed name so it cannot collide with a skill the user maintains. Copies rather than symlinks into vendor/ are the point: what the agent loads is a file you can open and diff, and a package update cannot change it underneath you. Only the .claude/skills/ mirror is a relative symlink to that copy, so the two can never drift apart, falling back to a second copy where symlink() is unavailable. All state lives in mate/extensions.php, next to the intent it derives from. Per skill, `enabled` and `mode` (managed|override) are user-editable; `state`, `source`, `source_hash`, `hash` and `targets` are written by the installer and rewritten on every run. SkillStateRepository is the sole reader and writer of that path: ContainerFactory includes it independently and reads a non-array as "every extension disabled", so writes are atomic and reads are served from cache once written in the same process. Two hashes are recorded because the copy is never byte-identical to its source (the rewritten name line): `hash` detects hand-edited output, `source_hash` detects an upstream change that has not been installed yet. skills:install is an idempotent reconciler — it rebuilds from source or from the user's mate/skills/ copy, prunes skills whose source or extension vanished, and never writes into mate/skills/. discover runs it automatically. The generated folders are deliberately not git-ignored: because they are plain copies, committing them turns an upstream skill change into a reviewable diff. Commands: skills:install, skills:list (enabled/mode/state/status), skills:validate (hand-edited content, missing folders, mispointed mirror, disabled-but-present, moved-on source; --strict fails on warnings too) and skills:prune (--dry-run), which only ever touches mate- prefixed folders.
011895e to
ffbc7cb
Compare
|
@chr-hertel Thanks, all pushed. Filesystem component: yes. SkillInstaller now uses mirror(), remove(), mkdir() and dumpFile(), about 90 lines lighter. One caveat worth knowing: mirror() recreates symlinks it finds inside a skill folder, so it gets an iterator that filters those out. Mispointed mirror: good catch. isUpToDate() accepted any resolvable symlink, so skills:install never repaired what skills:validate kept reporting. There is now a shared mirrorState() that both use, plus two regression tests. Feature depth / skills:prune: kept for now, because prune is the only remedy for the stray folder warning skills:validate raises, so dropping the command means dropping that check too. And there will be more commands to manage the state of the skills in the nearer future Docs: agreed, one pass after the remaining PRs and the MCP removal beats tuning them per PR. This one only flips the config section to lead with the commands instead of the file. Also rebased onto main, the UPGRADE.md conflict is gone. |
|
Thank you @wachterjohannes. |
…annes) This PR was squashed before being merged into the main branch. Discussion ---------- [Mate] Add skills:override and skills:reset | Q | A | ------------- | --- | Bug fix? | no | New feature? | yes | Docs? | yes | Issues | - | License | MIT Follow-up to #2370. That PR added the `mode` axis (`managed` vs `override`) to `mate/extensions.php`, but switching a skill to `override` still meant hand-editing the file and copying the folder yourself. `skills:override` copies the package's version of a skill into `mate/skills/<name>/` and records `'mode' => 'override'`. The copy is taken from the declared source, so it keeps the original frontmatter name; the installed `mate-` name is applied at build time as for any managed skill. `skills:reset` switches back to `managed` and rebuilds from the package. Your copy under `mate/skills/` is kept and its path reported, with `--delete-copy` to remove it explicitly. ```terminal $ vendor/bin/mate skills:override mate-system-information $ vendor/bin/mate skills:reset system-information ``` Both accept the installed or the original name, resolve against the recorded state so a skill stays addressable while its package is absent, and reinstall plus print the resulting row so intent and recorded facts never drift apart. Commits ------- 4dddcc1 [Mate] Add skills:override and skills:reset
…to skills:validate (wachterjohannes) This PR was squashed before being merged into the main branch. Discussion ---------- [Mate] Add skills:install --dry-run and content checks to skills:validate | Q | A | ------------- | --- | Bug fix? | no | New feature? | yes | Docs? | yes | Issues | - | License | MIT Two gaps left over from #2370. Independent of #2433 and #2434. `skills:install --dry-run` answers "what would this change" before it changes it. The same reconciler runs, it just writes nothing: no generated folder is touched and `mate/extensions.php` stays as it is. The result now also carries the skills that were rebuilt, not only the newly installed ones, so a real run reports the same groups the dry run does. ```terminal $ vendor/bin/mate skills:install --dry-run Would install 1 new skill: mate-symfony-profiler Would rebuild 1 skill: mate-system-information ``` `skills:validate` so far only compared the generated folders against the record, which says nothing about whether the installed skill is any good. Two content checks are added, both warnings, so they show up under `--strict` and in CI without breaking a normal run: a Markdown link to a file that is not part of the installed skill, and a description that is very short or never says when the skill applies. That description is all an agent has when it decides whether to load the skill. Commits ------- 30f8356 [Mate] Add skills:install --dry-run and content checks to skills:validate
First of three PRs building the Mate skill lifecycle. Follow-Ups will add
skills:override/skills:resetandskills:enable/skills:disable.Skills are the workflow/knowledge layer above tools: inert
SKILL.mdprocedures the agent reads, managed — never executed — by the CLI. Theextra.ai-mate.skillsconfig key already exists as a list of directories, so extensions that ship skills need no change; this PR builds the metadata, install and state layer on top of it.Install as a copy
A skill is installed as a real copy under
.agents/skills/mate-<name>/, with its frontmatternamerewritten to the installed name so it cannot collide with a skill the user maintains.Copies rather than symlinks into
vendor/are the point: what the agent loads is a file you can open and diff, and a package update cannot change it underneath you. Only the.claude/skills/mirror is a relative symlink to that copy, so the two can never drift apart — falling back to a second copy wheresymlink()is unavailable (Windows without developer mode), which is isolated behind aLinkerInterfaceso the fallback is reachable in tests.One state file
Everything lives in
mate/extensions.php, next to the intent it derives from:SkillStateRepositoryis the sole reader and writer of that path.ContainerFactoryincludes it independently and treats a non-array result as every extension disabled, so writes go through a temp file +rename(), and reads are served from cache once written in the same process (a mutating command does write → install → read, and opcache can otherwise hand back the pre-write file).Two hashes are recorded because the copy is never byte-identical to its source — the rewritten
nameline:hashdetects hand-edited output,source_hashdetects an upstream change that has not been installed yet.Commands
skills:install— idempotent reconciler. Rebuilds from source or from the user'smate/skills/copy, prunes skills whose source or extension vanished, and never writes intomate/skills/.discoverruns it automatically.skills:list— read-only overview: enabled, mode, state, status (ok,disabled,not installed,stale,broken). Table, JSON or TOON.skills:validate [name] [--strict]— checks the generated folders against the record. Errors on hand-edited content, missing folders, a mispointed mirror, an overridden skill with no copy, and a disabled skill that still has folders; warns on a moved-on source, a never-installed skill, a copied mirror and strays. Exits non-zero on errors, or on warnings with--strict.skills:prune [--dry-run]— removes leftover generated folders. Onlymate-prefixed entries are ever touched.Deliberately not git-ignored
The generated folders are not added to
.gitignore. Because they are plain copies, committing them turns an upstream skill change into a reviewable diff instead of something that lands silently. (An earlier iteration managed a.gitignoreblock that ignored/.agents/skills/and/.claude/skills/wholesale — that would also have hidden skills the user maintains, including the five tracked.claude/skills/*files in this monorepo.)Notes for review
This is a BC break for anyone who configured skills under
0.12— theUPGRADE.mdentry covers it, and the PR needs the BC Break label (I cannot add it myself).It also touches the
0.13section ofsrc/mate/CHANGELOG.md, which #2132 touches as well; whichever lands second needs a trivial rebase. Worth noting that #2132's skills usedescription: >-block scalars, which the front matter parser in this PR handles — on0.12they parse to the literal">-".