diff --git a/docs/superpowers/plans/2026-07-03-session-skills.md b/docs/superpowers/plans/2026-07-03-session-skills.md new file mode 100644 index 00000000..cb259624 --- /dev/null +++ b/docs/superpowers/plans/2026-07-03-session-skills.md @@ -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//` → 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 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 → `/.claude/skills//`; if worktree, append + `/.claude/skills//` 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. diff --git a/docs/superpowers/specs/2026-07-03-session-skills-design.md b/docs/superpowers/specs/2026-07-03-session-skills-design.md new file mode 100644 index 00000000..70b8bfc9 --- /dev/null +++ b/docs/superpowers/specs/2026-07-03-session-skills-design.md @@ -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//` (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 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//` → + `/.claude/skills//` (overwrite → idempotent for resume/re-run). +- If the cwd is a git worktree, append `/.claude/skills//` 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.