docs(skills): spec + plan for per-level session skills
This commit is contained in:
Mika Kuns
@@ -0,0 +1,90 @@
|
||||
# Session Skills — Implementation Plan
|
||||
|
||||
Spec: `docs/superpowers/specs/2026-07-03-session-skills-design.md`
|
||||
Approach: subagent-driven TDD (sonnet), build + test + commit per task, stage files by
|
||||
path (never `git add -A`).
|
||||
|
||||
**Pre-flight (do first, before building anything):** manual smoke test — drop a skill
|
||||
into a scratch worktree's `.claude/skills/` and run `claude -p` to confirm cwd skills are
|
||||
discovered in headless mode. The whole feature rests on this. If it fails, stop and
|
||||
redesign around `CLAUDE_CONFIG_DIR`.
|
||||
|
||||
---
|
||||
|
||||
## Task 1 — Data layer: columns + registry table + migration
|
||||
|
||||
- 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 `SessionSkillRepository` (async, CancellationToken): `ListAsync`, `GetAsync(name)`,
|
||||
`UpsertAsync`, `DeleteAsync`.
|
||||
- EF migration `AddSessionSkills` (columns + table).
|
||||
- **Tests (Data.Tests):** repository CRUD on real SQLite; JSON column round-trips a
|
||||
name list.
|
||||
|
||||
## Task 2 — Registry service (install / update / remove)
|
||||
|
||||
- `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/<name>/` → 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.**
|
||||
|
||||
## Task 3 — Resolution: union into ClaudeRunConfig
|
||||
|
||||
- Add `IReadOnlyList<string> SkillNames` to `ClaudeRunConfig` (default empty).
|
||||
- In `TaskRunner.ResolveConfigAsync`: parse each level's `session_skills`, union + dedup,
|
||||
filter to registry-existing names (drop + log missing).
|
||||
- **Tests (Worker.Tests):** union across the three levels; dedup; unknown name dropped.
|
||||
|
||||
## Task 4 — Seeder
|
||||
|
||||
- `Skills/SessionSkillSeeder` + interface. `SeedAsync(cwd, skillNames, isWorktree, ct)`:
|
||||
copy each installed skill dir → `<cwd>/.claude/skills/<name>/`; if worktree, append
|
||||
`/.claude/skills/<name>/` to `git rev-parse --git-path info/exclude` target if absent.
|
||||
- Wire into `TaskRunner` after run-dir resolution, before `ClaudeProcess.RunAsync`
|
||||
(both worktree and sandbox paths).
|
||||
- **Tests (Worker.Tests):** seeds into real temp dir; idempotent re-seed; worktree
|
||||
exclude line written once and not duplicated; seeded path is git-ignored (real git
|
||||
temp repo → `git status` clean for the seeded dir).
|
||||
|
||||
## Task 5 — Hub + DTOs + client
|
||||
|
||||
- `WorkerHub`: `GetSessionSkills`, `InstallSessionSkill(url)`, `UpdateSessionSkill(name)`,
|
||||
`RemoveSessionSkill(name)`.
|
||||
- New `SessionSkillDto`; extend `AppSettingsDto`, `ListConfigDto`, `UpdateListConfigDto`,
|
||||
`UpdateTaskAgentSettingsDto` with skill-name lists; map in the update handlers.
|
||||
- `IWorkerClient` + `WorkerClient` additions.
|
||||
- **Update hand-rolled fakes** in Worker.Tests + Ui.Tests (memory
|
||||
`iworkerclient_fakes_sync`).
|
||||
- **Tests:** hub method round-trip via existing hub test harness where present.
|
||||
|
||||
## Task 6 — UI: registry tab + selectors
|
||||
|
||||
- `SessionSkillsSettingsTabViewModel` + a **Skills** tab in `SettingsModalView.axaml`:
|
||||
installed list, Add (URL), Update, Remove, status line. Mirror
|
||||
`FilesSettingsTabViewModel`.
|
||||
- Global multi-select in General settings tab → `AppSettings.SessionSkills`.
|
||||
- Skills multi-select in shared `AgentConfigEditor` (covers List + Task) with inheritance
|
||||
badge, wired through `AgentConfigEditorViewModel`.
|
||||
- Localization: add EN + DE keys in parity (Localization.Tests enforces).
|
||||
- **Tests (Ui.Tests / Localization.Tests):** VM load/save of selections; locale parity.
|
||||
- **Visual verification is Mika's** — flag the gaps.
|
||||
|
||||
## Task 7 — Wiring, build, end-to-end smoke
|
||||
|
||||
- DI registration (registry, cloner, seeder) in `Program.cs`.
|
||||
- Build all touched projects `-c Release`; run Worker/Data/Ui/Localization test projects.
|
||||
- Manual E2E: install ponytail via the UI, enable per-task, run a task, confirm the skill
|
||||
is available to the agent and **not** committed and **not** in interactive sessions.
|
||||
|
||||
---
|
||||
|
||||
Commit per task with Conventional Commits (`feat(worker|ui|data): …`). Commit the
|
||||
spec + plan docs first.
|
||||
@@ -0,0 +1,146 @@
|
||||
# Session Skills — Design
|
||||
|
||||
**Date:** 2026-07-03
|
||||
**Status:** Approved (design), implementation not started
|
||||
|
||||
## Problem
|
||||
|
||||
Mika wants to give headless task agents a specific Claude skill (e.g.
|
||||
[ponytail](https://github.com/DietrichGebert/ponytail)) **without** installing it
|
||||
globally in `~/.claude/skills/`, where it would leak into every interactive session.
|
||||
Skills should be a first-class, per-level session setting alongside `model`,
|
||||
`max_turns`, and `system_prompt` — configurable **global / per-list / per-task** — and
|
||||
sourced from a GitHub URL.
|
||||
|
||||
## Key facts that shape the design
|
||||
|
||||
- The Claude CLI discovers skills from two places: the **global** `~/.claude/skills/`
|
||||
(every session — undesirable here) and the **working directory's** `.claude/skills/`
|
||||
(plus plugins). ClaudeDo fully controls each spawned session's `WorkingDirectory`
|
||||
(`ClaudeProcess.cs:30`), so a skill dropped into the session cwd is scoped to exactly
|
||||
that headless run.
|
||||
- **Auto-commit uses `git add -A`** (`WorktreeManager.CommitIfChangedAsync` →
|
||||
`_git.AddAllAsync`, `WorktreeManager.cs:142`). Anything seeded into a worktree's
|
||||
`.claude/skills/` would be committed unless explicitly excluded → the seeder must add
|
||||
the seeded paths to the worktree's `info/exclude`.
|
||||
- A skill is not just prompt text: `SKILL.md` may reference scripts that run via Bash.
|
||||
Headless agents run with `--permission-mode auto` (effectively unattended), so a skill
|
||||
pulled from an arbitrary URL is **unattended third-party code execution**. This is why
|
||||
install is a deliberate, pinned, reviewable step — not a live per-run URL fetch.
|
||||
|
||||
## Decisions (locked)
|
||||
|
||||
1. **Install-and-pin, not live fetch.** A dedicated registry screen installs a skill
|
||||
once: clone the repo, pin to the current commit, store locally. Per-level config then
|
||||
references installed skills **by name** (checkboxes), never a 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.
|
||||
4. **Public repos only** (plain `git clone` over HTTPS, no auth) for MVP.
|
||||
|
||||
## 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`.
|
||||
- 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.
|
||||
|
||||
### Resolution
|
||||
|
||||
`TaskRunner.ResolveConfigAsync` (`TaskRunner.cs:488`) already merges
|
||||
task → list → global for the other fields. Add:
|
||||
|
||||
```
|
||||
SkillNames = Union(task.SessionSkills, listConfig?.SessionSkills, global.SessionSkills)
|
||||
```
|
||||
|
||||
deduped, filtered to names that still exist in the registry (a removed skill is silently
|
||||
dropped — logged). Add `IReadOnlyList<string> SkillNames` to `ClaudeRunConfig`
|
||||
(`ClaudeArgsBuilder.cs:5`). **No CLI flag is emitted** — skills are seeded on disk, not
|
||||
passed as args. `ClaudeArgsBuilder.Build` is unchanged for skills.
|
||||
|
||||
Per-level storage: a nullable TEXT column `session_skills` (JSON array of names) on
|
||||
`tasks`, `list_config`, and `app_settings`.
|
||||
|
||||
### Seeding
|
||||
|
||||
New service `SessionSkillSeeder`, called by `TaskRunner` after the working dir is
|
||||
resolved and before `ClaudeProcess.RunAsync`:
|
||||
|
||||
- For each resolved skill name, copy `~/.todo-app/session-skills/<name>/` →
|
||||
`<cwd>/.claude/skills/<name>/` (overwrite → idempotent for resume/re-run).
|
||||
- If the cwd is a git worktree, append `/.claude/skills/<name>/` to the worktree's
|
||||
`info/exclude` (path via `git rev-parse --git-path info/exclude`, so it targets the
|
||||
per-worktree exclude), only if not already present. **Only the seeded subdirs are
|
||||
excluded** — never blanket-exclude `/.claude/`, in case the target project commits its
|
||||
own `.claude/`.
|
||||
- Sandbox runs (not a repo) skip the exclude step.
|
||||
- No separate cleanup: seeded dirs vanish with the worktree/sandbox.
|
||||
|
||||
### allowedTools caveat
|
||||
|
||||
`--allowedTools` is only emitted when set (`ClaudeArgsBuilder.cs:80`); normal task runs
|
||||
leave it null → all tools allowed → the `Skill` tool is available. If a future per-task
|
||||
allowedTools restriction is added, it must include `Skill`. Noted, not handled in MVP.
|
||||
|
||||
## UI
|
||||
|
||||
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`.
|
||||
- **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
|
||||
`AgentConfigEditor` control (`AgentConfigEditor.axaml` /
|
||||
`AgentConfigEditorViewModel.cs`), which is already reused by both List settings and the
|
||||
per-task flyout — one addition covers both levels, with the existing inheritance-badge
|
||||
pattern.
|
||||
|
||||
### Hub / client surface
|
||||
|
||||
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`,
|
||||
`UpdateListConfigDto`, `UpdateTaskAgentSettingsDto` with the selected skill-name lists.
|
||||
New `SessionSkillDto`.
|
||||
|
||||
## Edge cases
|
||||
|
||||
- **Removed skill still referenced** by a level → dropped at resolve time, logged, no
|
||||
failure.
|
||||
- **Name collision on install** → reject with a clear message; offer Update instead.
|
||||
- **Repo without root `SKILL.md`** → reject at install.
|
||||
- **Target project already has `.claude/skills/`** → additive copy; exclude only our
|
||||
subdirs.
|
||||
- **Resume / re-run** reuses the worktree → re-seed overwrites, exclude append is
|
||||
idempotent.
|
||||
|
||||
## 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.
|
||||
- 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.
|
||||
Reference in New Issue
Block a user