diff --git a/docs/superpowers/plans/2026-07-03-session-skills.md b/docs/superpowers/plans/2026-07-03-session-skills.md index cb259624..c6bbb185 100644 --- a/docs/superpowers/plans/2026-07-03-session-skills.md +++ b/docs/superpowers/plans/2026-07-03-session-skills.md @@ -15,10 +15,11 @@ redesign around `CLAUDE_CONFIG_DIR`. - Add nullable `SessionSkills` (string, JSON array) to `TaskEntity`, `ListConfigEntity`, `AppSettingsEntity`; map `session_skills` columns in their `*Configuration.cs`. -- New `SessionSkillEntity` (`name` PK, `source_url`, `pinned_ref`, `description`, - `added_at`) + configuration + `session_skills` table. +- New `SessionSkillEntity` (`name` PK, `source_url`, `pinned_ref`, `subpath`, + `description`, `added_at`) — one row per skill; a multi-skill repo writes N rows sharing + `source_url`/`pinned_ref` — + configuration + `session_skills` table. - New `SessionSkillRepository` (async, CancellationToken): `ListAsync`, `GetAsync(name)`, - `UpsertAsync`, `DeleteAsync`. + `UpsertAsync`, `DeleteAsync(name)`, `DeleteBySourceAsync(url)`, `ListBySourceAsync(url)`. - EF migration `AddSessionSkills` (columns + table). - **Tests (Data.Tests):** repository CRUD on real SQLite; JSON column round-trips a name list. @@ -28,13 +29,16 @@ redesign around `CLAUDE_CONFIG_DIR`. - `Skills/SessionSkillRegistry` + `Skills/Interfaces/ISessionSkillRegistry`, `IRepoCloner` (clone abstraction so tests inject a local source dir). - `GitRepoCloner` (production) does `git clone` + resolves HEAD SHA. -- Install: clone → require root `SKILL.md` → parse YAML frontmatter (`name`, - `description`) → move to `~/.todo-app/session-skills//` → upsert row. Reject - collision / missing `SKILL.md`. -- Update / Remove per spec. -- **Tests (Worker.Tests):** install from a local fixture dir (fake cloner) parses - frontmatter + writes registry + copies files; missing-`SKILL.md` rejected; collision - rejected; remove deletes dir + row. **No real network / no real claude CLI.** +- Install: clone → **detect layout** (`skills/*/SKILL.md` bundle → each subskill; else + root `SKILL.md` → single; else reject) → per skill parse YAML frontmatter (`name`, + `description`), copy its dir **flat** to `~/.todo-app/session-skills//`, upsert a + row with `subpath`. Reject collision with a skill from a different source; reinstalling + the same source refreshes. +- Update(sourceUrl) / Remove(sourceUrl) per spec (act on all of a source's skills). +- **Tests (Worker.Tests):** install a **multi-skill** fixture (fake cloner, mirrors + ponytail's `skills/*/SKILL.md`) → N rows + N flat dirs; install a root-`SKILL.md` + fixture → 1 row; neither → rejected; cross-source name collision rejected; + remove-by-source deletes all its dirs + rows. **No real network / no real claude CLI.** ## Task 3 — Resolution: union into ClaudeRunConfig diff --git a/docs/superpowers/specs/2026-07-03-session-skills-design.md b/docs/superpowers/specs/2026-07-03-session-skills-design.md index 70b8bfc9..719d3ddc 100644 --- a/docs/superpowers/specs/2026-07-03-session-skills-design.md +++ b/docs/superpowers/specs/2026-07-03-session-skills-design.md @@ -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//SKILL.md` plus `.claude-plugin/`, hooks, commands, an + MCP — none of which we consume; we take only the `skills//` dirs. + - root `SKILL.md` → single skill. + - neither → reject. + The CLI expects `.claude/skills//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//` (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//` (its + own self-contained dir with `SKILL.md` at the root of that dir), so the seeder just + copies `/` → 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//`); auto-update & update notifications; per-task *disabling* of an +inherited skill; surfacing skill invocation in the run log.