docs(skills): revise for multi-skill plugin repos (ponytail)
This commit is contained in:
@@ -36,26 +36,46 @@ sourced from a GitHub URL.
|
||||
2. **Three levels, additive union.** Effective skill set = `global ∪ list ∪ task`.
|
||||
(Unlike `model`/`prompt`, which override — skills add up. Trade-off accepted: an
|
||||
inherited skill can't be switched off for a single task in the MVP.)
|
||||
3. **One repo = one skill.** Installer accepts only a repo with `SKILL.md` at the root
|
||||
(covers ponytail). Multi-skill repos / plugin-format repos are out of scope for MVP.
|
||||
3. **A repo can contribute multiple skills.** Installer detects the layout:
|
||||
- `skills/*/SKILL.md` (plugin bundle) → import **each** subskill flat. This is
|
||||
ponytail: it ships 6 skills (`ponytail`, `-help`, `-review`, `-audit`, `-debt`,
|
||||
`-gain`) under `skills/<name>/SKILL.md` plus `.claude-plugin/`, hooks, commands, an
|
||||
MCP — none of which we consume; we take only the `skills/<name>/` dirs.
|
||||
- root `SKILL.md` → single skill.
|
||||
- neither → reject.
|
||||
The CLI expects `.claude/skills/<name>/SKILL.md` **flat**, so subskills are flattened
|
||||
on install. Selection is **per individual skill name** (enable just `ponytail` +
|
||||
`ponytail-help` if you want, not all six).
|
||||
4. **Public repos only** (plain `git clone` over HTTPS, no auth) for MVP.
|
||||
|
||||
> **Correction (2026-07-03, from the smoke test):** the original "one repo = one skill
|
||||
> at root" MVP was wrong for the very target repo — ponytail is a multi-skill plugin.
|
||||
> Decision 3 above replaces it.
|
||||
|
||||
## Architecture
|
||||
|
||||
### Storage & registry
|
||||
|
||||
- Installed skills live at `~/.todo-app/session-skills/<name>/` (the cloned repo contents,
|
||||
`SKILL.md` at root).
|
||||
- New DB table `session_skills`: `name` (PK), `source_url`, `pinned_ref` (commit SHA),
|
||||
`description`, `added_at`.
|
||||
- Each discovered skill is copied **flat** to `~/.todo-app/session-skills/<name>/` (its
|
||||
own self-contained dir with `SKILL.md` at the root of that dir), so the seeder just
|
||||
copies `<name>/` → cwd.
|
||||
- New DB table `session_skills`, one row **per skill** (a multi-skill repo writes N rows
|
||||
sharing `source_url` + `pinned_ref`): `name` (PK), `source_url`, `pinned_ref` (commit
|
||||
SHA), `subpath` (dir within the repo the skill came from, e.g. `skills/ponytail` or
|
||||
`.` for root), `description`, `added_at`. Repo-level ops act on all rows with the same
|
||||
`source_url` (no separate sources table — keep it flat).
|
||||
- New worker service `SessionSkillRegistry` (in a new `Skills/` area under the Worker):
|
||||
- `InstallAsync(url)` — clone to temp → validate `SKILL.md` at root → parse frontmatter
|
||||
(`name`, `description`) → resolve HEAD SHA as `pinned_ref` → move into place →
|
||||
upsert DB row. Name collision → error surfaced to UI. The clone step is injected
|
||||
(`IRepoCloner`) so tests use a local source dir — **no real network, no real CLI**.
|
||||
- `UpdateAsync(name)` — re-fetch, checkout latest, refresh files + `pinned_ref`.
|
||||
- `RemoveAsync(name)` — delete dir + row.
|
||||
- `ListAsync()` — registry entries for the UI.
|
||||
- `InstallAsync(url)` — clone to temp → **detect layout** (`skills/*/SKILL.md` bundle
|
||||
vs root `SKILL.md`) → for each discovered skill parse frontmatter (`name`,
|
||||
`description`), resolve HEAD SHA as `pinned_ref`, copy its dir flat into place, upsert
|
||||
a row. Returns the list of installed skill names. Name collision (a skill name from a
|
||||
*different* source) → error surfaced to UI; reinstalling the same source updates.
|
||||
Clone is injected (`IRepoCloner`) so tests use a local source dir — **no real network,
|
||||
no real CLI**.
|
||||
- `UpdateAsync(sourceUrl)` — re-clone, re-detect, refresh that source's skills +
|
||||
`pinned_ref`.
|
||||
- `RemoveAsync(sourceUrl)` — delete all its skill dirs + rows.
|
||||
- `ListAsync()` — registry entries for the UI (grouped by source for display).
|
||||
|
||||
### Resolution
|
||||
|
||||
@@ -100,9 +120,10 @@ allowedTools restriction is added, it must include `Skill`. Noted, not handled i
|
||||
Mirror the existing agent-file pattern.
|
||||
|
||||
- **Registry screen ("extra mask"):** a new **Skills** tab in the Settings modal
|
||||
(`SettingsModalView.axaml`) with `SessionSkillsSettingsTabViewModel`. Lists installed
|
||||
skills (name, description, source, short ref); **Add** (URL text box → install),
|
||||
**Update**, **Remove**. Status/error line like `FilesSettingsTabViewModel`.
|
||||
(`SettingsModalView.axaml`) with `SessionSkillsSettingsTabViewModel`. **Add** (URL text
|
||||
box → install a repo, which may yield several skills); lists installed skills grouped by
|
||||
source (name, description, source, short ref); **Update** / **Remove** act per source
|
||||
(repo). Status/error line like `FilesSettingsTabViewModel`.
|
||||
- **Global selector:** multi-select (checkbox list) of installed skills in the General
|
||||
settings tab → `AppSettings.SessionSkills`.
|
||||
- **List + Task selectors:** add a skills multi-select to the shared
|
||||
@@ -115,8 +136,8 @@ Mirror the existing agent-file pattern.
|
||||
|
||||
New `WorkerHub` methods + `IWorkerClient` entries (update hand-rolled fakes in both test
|
||||
projects — see memory `iworkerclient_fakes_sync`):
|
||||
`GetSessionSkills`, `InstallSessionSkill(url)`, `UpdateSessionSkill(name)`,
|
||||
`RemoveSessionSkill(name)`. Extend `AppSettingsDto`, `ListConfigDto`,
|
||||
`GetSessionSkills`, `InstallSessionSkill(url)`, `UpdateSessionSkill(sourceUrl)`,
|
||||
`RemoveSessionSkill(sourceUrl)`. Extend `AppSettingsDto`, `ListConfigDto`,
|
||||
`UpdateListConfigDto`, `UpdateTaskAgentSettingsDto` with the selected skill-name lists.
|
||||
New `SessionSkillDto`.
|
||||
|
||||
@@ -134,13 +155,19 @@ New `SessionSkillDto`.
|
||||
## Verification (must-check, can't be unit-tested)
|
||||
|
||||
- **Does `claude -p` actually load and invoke a skill placed in cwd `.claude/skills/`?**
|
||||
This is the load-bearing assumption. Manual smoke test with ponytail before calling the
|
||||
feature done. If headless mode does *not* surface cwd skills, fall back to
|
||||
`CLAUDE_CONFIG_DIR` isolation (heavier — needs credentials copied) and revisit.
|
||||
Load-bearing assumption. Smoke test 2026-07-03: `system:init` confirms correct `cwd` +
|
||||
`Skill` tool present; one authed run engaged the ponytail skill (12 mentions, vs zero on
|
||||
an unauthed run). **Clean repro is blocked by auth-token rotation in the bash sandbox**
|
||||
(parallel-session contention, not a design issue) — Mika confirms with one run in a
|
||||
stable terminal:
|
||||
`cd /tmp/claudedo-skill-smoke && echo "Use the ponytail-help skill to show the levels table, then stop." | claude -p --permission-mode acceptEdits --max-turns 4`.
|
||||
If headless does *not* surface cwd skills, fall back to `CLAUDE_CONFIG_DIR` isolation
|
||||
(heavier — needs credentials copied) and revisit.
|
||||
- Seeded skill is **not** committed by the auto-commit step (worktree run).
|
||||
- Skill does not appear in a normal interactive session (no global leak).
|
||||
|
||||
## Out of scope (MVP)
|
||||
|
||||
Private-repo auth; multi-skill / plugin-format repos; auto-update & update notifications;
|
||||
per-task *disabling* of an inherited skill; surfacing skill invocation in the run log.
|
||||
Private-repo auth; consuming a plugin's *non-skill* parts (hooks, commands, MCP — we take
|
||||
only `skills/<name>/`); auto-update & update notifications; per-task *disabling* of an
|
||||
inherited skill; surfacing skill invocation in the run log.
|
||||
|
||||
Reference in New Issue
Block a user