170 lines
8.9 KiB
Markdown
170 lines
8.9 KiB
Markdown
# 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. **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
|
||
|
||
- 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 → **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
|
||
|
||
`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`. **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
|
||
`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(sourceUrl)`,
|
||
`RemoveSessionSkill(sourceUrl)`. 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/`?**
|
||
**CONFIRMED 2026-07-03.** Mika ran, in a stable terminal, a `claude -p` invocation in a
|
||
scratch cwd holding `.claude/skills/ponytail*/SKILL.md`; the model invoked the
|
||
`ponytail-help` skill and returned its exact Lite/Full/Ultra table. Headless mode does
|
||
surface cwd skills → the whole approach holds; no `CLAUDE_CONFIG_DIR` fallback needed.
|
||
- 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; 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.
|